Re: [PATCH v3 4/5] pci: check if VGA decoding was really activated

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Sun, Mar 08, 2026 at 02:35:37AM +0900, Simon Richter wrote:
> PCI bridges are allowed to refuse activating VGA decoding, by simply
> ignoring attempts to set the bit that enables it, so after setting the bit,
> read it back to verify.
> 
> One example of such a bridge is the root bridge in IBM PowerNV, but this is
> also useful for GPU passthrough into virtual machines, where it is
> difficult to set up routing for legacy IO through IOMMU.
> 
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> ---
>  drivers/pci/pci.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8479c2e1f74f..e60b948f8576 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6197,6 +6197,12 @@ int pci_set_vga_state(struct pci_dev *dev, bool decode,
>  				cmd &= ~PCI_BRIDGE_CTL_VGA;
>  			pci_write_config_word(bridge, PCI_BRIDGE_CONTROL,
>  					      cmd);
> +			if (decode) {
> +				pci_read_config_word(bridge, PCI_BRIDGE_CONTROL,
> +						     &cmd);
> +				if(!(cmd & PCI_BRIDGE_CTL_VGA))
> +					return -EIO;
> +			}

Maybe this should also have a comment or spec quote to explain
that it's legal behavior?

The slightly bigger concern I have is whether we need to unwind
the previous steps if this fails? Looks like we don't update
vgadev->owns on failure (even though we may have partially
enabled things). But since the bridge should never forward any
VGA accesses, leaving some extra PCI_COMMAND enable(s) set on
the device shouldn't matter in practice. So I guess this should
work even without unwinding. Well, not sure about 
arch_set_vga_state() since that's 100% magic...

>  		}
>  		bus = bus->parent;
>  	}
> -- 
> 2.47.3

-- 
Ville Syrjälä
Intel