[PATCH 0/1] Avoid oops on illegal VGA register access

[PATCH 0/1] Avoid oops on illegal VGA register access

[Simon Richter]

Hi,

this adds a way to avoid the illegal VGA accesses on non-x86 platforms by
letting these platforms report that VGA registers cannot be mapped.

This patch alone doesn't fully solve the problem, it also needs complementary
changes in vgaarb to actually report this, but since the interface is unchanged
and it is wrong to ignore the error code anyway, little coordination is needed
here.

Needs review from Intel whether this is the correct flow, and a decision on
whether this should be backported.

   Simon

Simon Richter (1):
  drm/i915: handle failure from vga_get

 drivers/gpu/drm/i915/display/intel_vga.c | 29 ++++++++++++++++++++----
 1 file changed, 24 insertions(+), 5 deletions(-)


base-commit: 15658979e64a7c97eaa65563e27a5a65e68a0188
-- 
2.47.3

[PATCH 1/1] drm/i915: handle failure from vga_get

[Simon Richter]

This function returns an error code. If that code is non-zero, VGA decoding
is undefined, and the lock counter has not been increased, so it is not valid
to access registers or call vga_put afterwards.

Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1824
---
 drivers/gpu/drm/i915/display/intel_vga.c | 29 ++++++++++++++++++++----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
index 6fc3e3702cb8..4118c451d53c 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -112,12 +112,17 @@ static bool intel_pci_bridge_set_vga(struct pci_dev *pdev, bool enable)
 	return old & PCI_BRIDGE_CTL_VGA;
 }
 
-static bool intel_vga_get(struct intel_display *display, bool mmio)
+static int __must_check intel_vga_get(struct intel_display *display,
+	bool mmio, bool *old_io_decode)
 {
 	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
+	int err;
 
 	if (mmio)
-		return false;
+	{
+		*old_io_decode = false;
+		return 0;
+	}
 
 	/*
 	 * Bypass the VGA arbiter on the iGPU and just enable
@@ -131,9 +136,14 @@ static bool intel_vga_get(struct intel_display *display, bool mmio)
 	 * of how any other VGA routing bits are configured.
 	 */
 	if (display->platform.dgfx)
-		vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
+	{
+		err = vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
+		if (unlikely(err))
+			return err;
+	}
 
-	return intel_pci_set_io_decode(pdev, true);
+	*old_io_decode = intel_pci_set_io_decode(pdev, true);
+	return 0;
 }
 
 static void intel_vga_put(struct intel_display *display, bool io_decode, bool mmio)
@@ -175,6 +185,7 @@ void intel_vga_disable(struct intel_display *display)
 	bool io_decode;
 	u8 msr, sr1;
 	u32 tmp;
+	int err;
 
 	if (!intel_vga_decode_is_enabled(display)) {
 		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
@@ -216,7 +227,15 @@ void intel_vga_disable(struct intel_display *display)
 			goto reset_vgacntr;
 	}
 
-	io_decode = intel_vga_get(display, mmio);
+	/* This should not fail, because vga_get will only report errors for
+	 * dGPUs that are unreachable via the bridge, and cannot be made
+	 * reachable either. We shouldn't even get here for this case, but if
+	 * we do, we assume that the bridge will also refuse future requests
+	 * to forward VGA accesses.
+	 */
+	err = intel_vga_get(display, mmio, &io_decode);
+	if (unlikely(err))
+		goto reset_vgacntr;
 
 	drm_WARN_ON(display->drm, !mmio && !intel_pci_has_vga_io_decode(pdev));
 
-- 
2.47.3

[PATCH v2 1/1] drm/i915: handle failure from vga_get

[Simon Richter]

This function returns an error code. If that code is non-zero, VGA decoding
is undefined, and the lock counter has not been increased, so it is not
valid to access registers or call vga_put afterwards.

Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1824
---
 drivers/gpu/drm/i915/display/intel_vga.c | 31 ++++++++++++++++++------
 1 file changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
index 6fc3e3702cb8..24635e7b27e6 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -112,12 +112,16 @@ static bool intel_pci_bridge_set_vga(struct pci_dev *pdev, bool enable)
 	return old & PCI_BRIDGE_CTL_VGA;
 }
 
-static bool intel_vga_get(struct intel_display *display, bool mmio)
+static int __must_check intel_vga_get(struct intel_display *display, bool mmio,
+				      bool *old_io_decode)
 {
 	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
+	int err;
 
-	if (mmio)
-		return false;
+	if (mmio) {
+		*old_io_decode = false;
+		return 0;
+	}
 
 	/*
 	 * Bypass the VGA arbiter on the iGPU and just enable
@@ -130,10 +134,14 @@ static bool intel_vga_get(struct intel_display *display, bool mmio)
 	 * grab any VGA IO access when IO decode is enabled, regardless
 	 * of how any other VGA routing bits are configured.
 	 */
-	if (display->platform.dgfx)
-		vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
+	if (display->platform.dgfx) {
+		err = vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
+		if (unlikely(err))
+			return err;
+	}
 
-	return intel_pci_set_io_decode(pdev, true);
+	*old_io_decode = intel_pci_set_io_decode(pdev, true);
+	return 0;
 }
 
 static void intel_vga_put(struct intel_display *display, bool io_decode, bool mmio)
@@ -175,6 +183,7 @@ void intel_vga_disable(struct intel_display *display)
 	bool io_decode;
 	u8 msr, sr1;
 	u32 tmp;
+	int err;
 
 	if (!intel_vga_decode_is_enabled(display)) {
 		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
@@ -216,7 +225,15 @@ void intel_vga_disable(struct intel_display *display)
 			goto reset_vgacntr;
 	}
 
-	io_decode = intel_vga_get(display, mmio);
+	/* This should not fail, because vga_get will only report errors for
+	 * dGPUs that are unreachable via the bridge, and cannot be made
+	 * reachable either. We shouldn't even get here for this case, but if
+	 * we do, we assume that the bridge will also refuse future requests
+	 * to forward VGA accesses.
+	 */
+	err = intel_vga_get(display, mmio, &io_decode);
+	if (unlikely(err))
+		goto reset_vgacntr;
 
 	drm_WARN_ON(display->drm, !mmio && !intel_pci_has_vga_io_decode(pdev));
 
-- 
2.47.3

Re: [PATCH 1/1] drm/i915: handle failure from vga_get

[Jani Nikula]

On Wed, 18 Feb 2026, Simon Richter <Simon.Richter@hogyros.de> wrote:
> This function returns an error code.

Which function? Please make commit messages independent of the Subject
line.

Also, the function we call is vga_get_uninterruptible().

> If that code is non-zero, VGA decoding
> is undefined, and the lock counter has not been increased, so it is not valid
> to access registers or call vga_put afterwards.
>
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1824
> ---
>  drivers/gpu/drm/i915/display/intel_vga.c | 29 ++++++++++++++++++++----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> index 6fc3e3702cb8..4118c451d53c 100644
> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> @@ -112,12 +112,17 @@ static bool intel_pci_bridge_set_vga(struct pci_dev *pdev, bool enable)
>  	return old & PCI_BRIDGE_CTL_VGA;
>  }
>  
> -static bool intel_vga_get(struct intel_display *display, bool mmio)
> +static int __must_check intel_vga_get(struct intel_display *display,

I'm not convinced by __must_check annotations for static
functions. Global stuff, sure, but we'd be adding tons and tons of these
annotations if we decided to go this path.

> +	bool mmio, bool *old_io_decode)
>  {
>  	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
> +	int err;
>  
>  	if (mmio)
> -		return false;
> +	{

Brace location, please see checkpatch.

> +		*old_io_decode = false;
> +		return 0;
> +	}
>  
>  	/*
>  	 * Bypass the VGA arbiter on the iGPU and just enable
> @@ -131,9 +136,14 @@ static bool intel_vga_get(struct intel_display *display, bool mmio)
>  	 * of how any other VGA routing bits are configured.
>  	 */
>  	if (display->platform.dgfx)
> -		vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> +	{

Brace location.

> +		err = vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> +		if (unlikely(err))

I'm not convinced by unlikely annotations in cases like this. It's just
more stuff to read, for no clear benefit. Again, if we set the example,
we'll be adding these absolutely everywhere.

> +			return err;
> +	}
>  
> -	return intel_pci_set_io_decode(pdev, true);
> +	*old_io_decode = intel_pci_set_io_decode(pdev, true);

Blank line before return.

> +	return 0;
>  }
>  
>  static void intel_vga_put(struct intel_display *display, bool io_decode, bool mmio)
> @@ -175,6 +185,7 @@ void intel_vga_disable(struct intel_display *display)
>  	bool io_decode;
>  	u8 msr, sr1;
>  	u32 tmp;
> +	int err;
>  
>  	if (!intel_vga_decode_is_enabled(display)) {
>  		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
> @@ -216,7 +227,15 @@ void intel_vga_disable(struct intel_display *display)
>  			goto reset_vgacntr;
>  	}
>  
> -	io_decode = intel_vga_get(display, mmio);
> +	/* This should not fail, because vga_get will only report errors for

/* on its own line please.

> +	 * dGPUs that are unreachable via the bridge, and cannot be made
> +	 * reachable either. We shouldn't even get here for this case, but if
> +	 * we do, we assume that the bridge will also refuse future requests
> +	 * to forward VGA accesses.
> +	 */
> +	err = intel_vga_get(display, mmio, &io_decode);
> +	if (unlikely(err))

Please no unlikely.

> +		goto reset_vgacntr;
>  
>  	drm_WARN_ON(display->drm, !mmio && !intel_pci_has_vga_io_decode(pdev));

-- 
Jani Nikula, Intel

[PATCH v3 1/1] drm/i915: handle failure from vga_get_uninterruptible()

[Simon Richter]

The vga_get_uninterruptible() function can return an error if it fails to
set up VGA decoding for the requested device.

If VGA decoding is unavailable, we don't need to be careful to leave the
VGA emulation in a usable state, as vgacon will also be unable to get
access later on, so just skip over the VGA accesses and the vga_put() call
matching the failed vga_get_uninterruptible().

Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1824
---
 drivers/gpu/drm/i915/display/intel_vga.c | 33 +++++++++++++++++++-----
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
index 6fc3e3702cb8..9832a4ade318 100644
--- a/drivers/gpu/drm/i915/display/intel_vga.c
+++ b/drivers/gpu/drm/i915/display/intel_vga.c
@@ -112,12 +112,16 @@ static bool intel_pci_bridge_set_vga(struct pci_dev *pdev, bool enable)
 	return old & PCI_BRIDGE_CTL_VGA;
 }
 
-static bool intel_vga_get(struct intel_display *display, bool mmio)
+static int intel_vga_get(struct intel_display *display, bool mmio,
+			 bool *old_io_decode)
 {
 	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
+	int err;
 
-	if (mmio)
-		return false;
+	if (mmio) {
+		*old_io_decode = false;
+		return 0;
+	}
 
 	/*
 	 * Bypass the VGA arbiter on the iGPU and just enable
@@ -130,10 +134,15 @@ static bool intel_vga_get(struct intel_display *display, bool mmio)
 	 * grab any VGA IO access when IO decode is enabled, regardless
 	 * of how any other VGA routing bits are configured.
 	 */
-	if (display->platform.dgfx)
-		vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
+	if (display->platform.dgfx) {
+		err = vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
+		if (err)
+			return err;
+	}
+
+	*old_io_decode = intel_pci_set_io_decode(pdev, true);
 
-	return intel_pci_set_io_decode(pdev, true);
+	return 0;
 }
 
 static void intel_vga_put(struct intel_display *display, bool io_decode, bool mmio)
@@ -175,6 +184,7 @@ void intel_vga_disable(struct intel_display *display)
 	bool io_decode;
 	u8 msr, sr1;
 	u32 tmp;
+	int err;
 
 	if (!intel_vga_decode_is_enabled(display)) {
 		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
@@ -216,7 +226,16 @@ void intel_vga_disable(struct intel_display *display)
 			goto reset_vgacntr;
 	}
 
-	io_decode = intel_vga_get(display, mmio);
+	/*
+	 * This should not fail, because the vga_get() family of functions
+	 * will only report errors for dGPUs that are unreachable via the
+	 * bridge, and cannot be made reachable either. We shouldn't even
+	 * get here for this case, but if we do, we assume that the bridge
+	 * will also refuse future requests to forward VGA accesses.
+	 */
+	err = intel_vga_get(display, mmio, &io_decode);
+	if (err)
+		goto reset_vgacntr;
 
 	drm_WARN_ON(display->drm, !mmio && !intel_pci_has_vga_io_decode(pdev));
 
-- 
2.47.3

Re: [PATCH v3 1/1] drm/i915: handle failure from vga_get_uninterruptible()

[Ville Syrjälä]

On Sat, Mar 07, 2026 at 12:13:16AM +0900, Simon Richter wrote:
> The vga_get_uninterruptible() function can return an error if it fails to
> set up VGA decoding for the requested device.
> 
> If VGA decoding is unavailable, we don't need to be careful to leave the
> VGA emulation in a usable state, as vgacon will also be unable to get
> access later on, so just skip over the VGA accesses and the vga_put() call
> matching the failed vga_get_uninterruptible().
> 
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1824

Looks good.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Do you need to take this in via some other tree, or should I just push
it to drm-intel-next?

> ---
>  drivers/gpu/drm/i915/display/intel_vga.c | 33 +++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vga.c b/drivers/gpu/drm/i915/display/intel_vga.c
> index 6fc3e3702cb8..9832a4ade318 100644
> --- a/drivers/gpu/drm/i915/display/intel_vga.c
> +++ b/drivers/gpu/drm/i915/display/intel_vga.c
> @@ -112,12 +112,16 @@ static bool intel_pci_bridge_set_vga(struct pci_dev *pdev, bool enable)
>  	return old & PCI_BRIDGE_CTL_VGA;
>  }
>  
> -static bool intel_vga_get(struct intel_display *display, bool mmio)
> +static int intel_vga_get(struct intel_display *display, bool mmio,
> +			 bool *old_io_decode)
>  {
>  	struct pci_dev *pdev = to_pci_dev(display->drm->dev);
> +	int err;
>  
> -	if (mmio)
> -		return false;
> +	if (mmio) {
> +		*old_io_decode = false;
> +		return 0;
> +	}
>  
>  	/*
>  	 * Bypass the VGA arbiter on the iGPU and just enable
> @@ -130,10 +134,15 @@ static bool intel_vga_get(struct intel_display *display, bool mmio)
>  	 * grab any VGA IO access when IO decode is enabled, regardless
>  	 * of how any other VGA routing bits are configured.
>  	 */
> -	if (display->platform.dgfx)
> -		vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> +	if (display->platform.dgfx) {
> +		err = vga_get_uninterruptible(pdev, VGA_RSRC_LEGACY_IO);
> +		if (err)
> +			return err;
> +	}
> +
> +	*old_io_decode = intel_pci_set_io_decode(pdev, true);
>  
> -	return intel_pci_set_io_decode(pdev, true);
> +	return 0;
>  }
>  
>  static void intel_vga_put(struct intel_display *display, bool io_decode, bool mmio)
> @@ -175,6 +184,7 @@ void intel_vga_disable(struct intel_display *display)
>  	bool io_decode;
>  	u8 msr, sr1;
>  	u32 tmp;
> +	int err;
>  
>  	if (!intel_vga_decode_is_enabled(display)) {
>  		drm_dbg_kms(display->drm, "VGA decode is disabled\n");
> @@ -216,7 +226,16 @@ void intel_vga_disable(struct intel_display *display)
>  			goto reset_vgacntr;
>  	}
>  
> -	io_decode = intel_vga_get(display, mmio);
> +	/*
> +	 * This should not fail, because the vga_get() family of functions
> +	 * will only report errors for dGPUs that are unreachable via the
> +	 * bridge, and cannot be made reachable either. We shouldn't even
> +	 * get here for this case, but if we do, we assume that the bridge
> +	 * will also refuse future requests to forward VGA accesses.
> +	 */
> +	err = intel_vga_get(display, mmio, &io_decode);
> +	if (err)
> +		goto reset_vgacntr;
>  
>  	drm_WARN_ON(display->drm, !mmio && !intel_pci_has_vga_io_decode(pdev));
>  
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel

Re: [PATCH v3 1/1] drm/i915: handle failure from vga_get_uninterruptible()

[Simon Richter]

Hi,

On 3/10/26 8:10 PM, Ville Syrjälä wrote:

> Do you need to take this in via some other tree, or should I just push
> it to drm-intel-next?

This is independent from the PCI changes (and I didn't Cc linux-pci on 
this one), so via drm-intel-next would be best, thanks!

I've also prepared a backported version for 5.3 to 6.19 kernels[1].

    Simon

[1] https://patchwork.freedesktop.org/patch/709684/?series=161721&rev=4

Re: [PATCH v3 1/1] drm/i915: handle failure from vga_get_uninterruptible()

[Ville Syrjälä]

On Tue, Mar 10, 2026 at 11:45:53PM +0900, Simon Richter wrote:
> Hi,
> 
> On 3/10/26 8:10 PM, Ville Syrjälä wrote:
> 
> > Do you need to take this in via some other tree, or should I just push
> > it to drm-intel-next?
> 
> This is independent from the PCI changes (and I didn't Cc linux-pci on 
> this one), so via drm-intel-next would be best, thanks!

Pushed. Thanks.

> 
> I've also prepared a backported version for 5.3 to 6.19 kernels[1].
> 
>     Simon
> 
> [1] https://patchwork.freedesktop.org/patch/709684/?series=161721&rev=4

-- 
Ville Syrjälä
Intel