Re: [PATCH 1/6] PCI: determine whether a device has extended config space

[Stewart Hildebrand <stewart.hildebrand@xxxxxxx>]

On 1/15/26 05:43, Jan Beulich wrote:
> On 13.01.2026 11:46, Jan Beulich wrote:
>> On 12.01.2026 22:07, Stewart Hildebrand wrote:
>>> On 1/6/26 08:47, Jan Beulich wrote:
>>>> @@ -420,6 +467,19 @@ static struct pci_dev *alloc_pdev(struct
>>>>              break;
>>>>      }
>>>>  
>>>> +    if ( pdev->ext_cfg &&
>>>> +         /*
>>>> +          * Regular PCI devices have 256 bytes, but PCI-X 2 and PCI 
>>>> Express
>>>> +          * devices have 4096 bytes.  Even if the device is capable, that
>>>> +          * doesn't mean we can access it.  Maybe we don't have a way to
>>>> +          * generate extended config space accesses, or the device is 
>>>> behind a
>>>> +          * reverse Express bridge.  So we try reading the dword at
>>>> +          * PCI_CFG_SPACE_SIZE which must either be 0 or a valid extended
>>>> +          * capability header.
>>>> +          */
>>>> +         pci_conf_read32(pdev->sbdf, PCI_CFG_SPACE_SIZE) == 0xffffffffU )
>>>> +        pdev->ext_cfg = false;
>>>
>>> I'm using a machine where
>>> xen/arch/x86/x86_64/mmconfig-shared.c:is_mmconf_reserved() will initially 
>>> return
>>> false during Xen boot:
>>>
>>> (XEN) PCI: MCFG configuration 0: base f0000000 segment 0000 buses 00 - 3f
>>> (XEN) PCI: Not using MCFG for segment 0000 bus 00-3f
>>>
>>> Then, during dom0 boot, dom0 will call PHYSDEVOP_pci_mmcfg_reserved, after 
>>> which
>>> MCFG becomes enabled in Xen:
>>>
>>> (XEN) PCI: Using MCFG for segment 0000 bus 00-3f
>>>
>>> On such machines where mmcfg/ECAM is initially disabled, this will 
>>> effectively
>>> set ->ext_cfg to false for all devices discovered at Xen boot.
>>>
>>> I'm not really sure if I have any good suggestions, but perhaps we could 
>>> add a
>>> macro or small function that returns something like
>>>    ( pdev->ext_cfg && pci_conf_read32(pdev->sbdf, PCI_CFG_SPACE_SIZE) != 
>>> 0xffffffffU )
>>> to allow this checking to happen dynamically (but this still wouldn't 
>>> handle the
>>> aliasing quirk). Maybe re-run the ext_cfg detection logic at the end of
>>> PHYSDEVOP_pci_mmcfg_reserved?
>>>
>>> Regardless, I'd be happy to provide my R-b without this addressed, but I am
>>> curious if others think this as an issue?
>>
>> Hmm, no, I forgot to consider that case (which in principle I'm well aware 
>> of).
>> Will need to fix in v2.
> 
> My reply yesterday was actually not quite sufficient. On a system like yours,
> isn't it the case that PVH Dom0 then also doesn't work quite right (yet), due
> to parts of vPCI depending on extended config space accesses now? All of what
> we presently do during boot, and which requires extended config space access,
> would need re-doing once extended config space access becomes available (or
> goes away) for a (sub)set of devices.

Right, e.g. rebar will fail to initialize. One possible approach might be to do
some variation of this at the end of PHYSDEVOP_pci_mmcfg_reserved (untested):

    for_each_pdev ( hardware_domain, pdev )
        vpci_reset_device(pdev);

This would be better than nothing IMO, however, as you point out, it may only be
needed for a subset of devices.