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

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

On Tue, Mar 10, 2026 at 11:08:40PM +0900, Simon Richter wrote:
> Hi,
> 
> On 3/10/26 8:37 PM, Ville Syrjälä wrote:
> 
> > Maybe this should also have a comment or spec quote to explain
> > that it's legal behavior?
> 
> The PCI-to-PCI Bridge Specification version 1.1 has in chapter 4.5 "VGA 
> Support":
> 
>      Bridges are not required to implement the VGA support mechanisms
>      described in the following sections.
> 
> So this has always been optional.
> 
> What is a bit annoying is that I cannot find an explicit sentence 
> allowing the VGAEnable bit to be hardwired to zero in that specification.
> 
> It is explicitly allowed to hardwire the VGASnoopEnable bit to zero if 
> VGA palette snooping is not supported for downstream devices (Table 3-2 
> "Command Register"):
> 
>      Implementation of VGA palette snooping by a bridge is optional. If
>      VGA palette snooping is not supported, this bit must be implemented
>      as read-only with a value of 0
> 
> That is the command register though, not the bridge control register.
> 
> The bridge control register documentation (Table 3-9) does not 
> explicitly say what should be done if VGA support is not available, but 
> the behaviour of the VGAEnable bit is a superset of the VGASnoopEnable 
> bit, and the VGASnoopEnable bit becomes a Don't Care if VGAEnable is 
> set, so "the bit must be hardwired to zero if unsupported" appears to be 
> the only valid interpretation.
> 
> This seems to be the behaviour of all the devices I have available:
> 
>   - IBM PowerNV PCIe root bridge (POWER9)
>   - SiFive VisionFive2 PCIe root bridge (PLDA XpressRich-AXI Ref Design)
> 
> Both read back this bit as zero when written via setpci:
> 
> # setpci -s 0000:00:00.0 BRIDGE_CONTROL
> 0002
> # setpci -s 0000:00:00.0 BRIDGE_CONTROL=0xa
> # setpci -s 0000:00:00.0 BRIDGE_CONTROL
> 0002
> 
> There is also a question in the Altera support forum for their PCIe Root 
> Bridge IP at https://community.altera.com/kb/knowledge-base/-/345837 .

Yeah, looks like the exact explanation was only added in the PCIe
bridge spec. Before that it was somewhat ambiguous.

> 
> > 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.
> 
> I was thinking the same, but because we're going upwards in the 
> hierarchy, the bridges we leave enabled are downstream of the bridge 
> that refused to forward.
> 
> I can add this for completeness though, that should be fairly easy.
> 
> > So I guess this should
> > work even without unwinding. Well, not sure about
> > arch_set_vga_state() since that's 100% magic...
> 
> It's not entirely magic, because after success from arch_set_vga_state() 
> we still go through all the bridges. I believe uv uses it for some kind 
> of paravirtualization.
> 
>     Simon

-- 
Ville Syrjälä
Intel