On Tue, Aug 19, 2025 at 07:23:57PM +0100, Andrew Cooper wrote:
> On 19/08/2025 6:18 pm, Roger Pau Monne wrote:
> > Otherwise the PCI accesses to segments different than the first one done by
> > the IOMMU initialization code would silently fail by returning all ones.
> >
> > Introduce a new helper, called pci_setup(), and move both the creation of
> > PCI segment 0 internal data structures, plus the parsing of ACPI MMCFG
> > table to it.
> >
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
>
> And moving acpi_mmcfg_init() slightly earlier from acpi_boot_init() into
> pci_setup().
>
> > diff --git a/xen/arch/x86/pci.c b/xen/arch/x86/pci.c
> > index 26bb7f6a3c3a..e75a29e851a7 100644
> > --- a/xen/arch/x86/pci.c
> > +++ b/xen/arch/x86/pci.c
> > @@ -139,6 +142,19 @@ int pci_sanitize_bar_memory(struct rangeset *r)
> > return 0;
> > }
> >
> > +void __init pci_setup(void)
> > +{
> > + /*
> > + * Ahead of any ACPI table parsing make sure we have control structures
> > + * for PCI segment 0.
> > + */
> > + if ( pci_add_segment(0) )
> > + panic("Could not initialize PCI segment 0\n");
> > +
> > + /* Parse ACPI MMCFG ahead of IOMMU, so accesses to segments > 0 is
> > setup. */
>
> "ahead of IOMMU" isn't helpful here because the relevant context is in
> the caller. Instead, I'd just say:
>
> /* Parse ACPI MMCFG to see if other segments are available. */
Sure.
> > + acpi_mmcfg_init();
> > +}
> > +
> > /*
> > * Local variables:
> > * mode: C
> > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> > index 6fb42c5a5f95..bd648323bfed 100644
> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -1938,11 +1938,10 @@ void asmlinkage __init noreturn __start_xen(void)
> > setup_system_domains();
> >
> > /*
> > - * Ahead of any ACPI table parsing make sure we have control structures
> > - * for PCI segment 0.
> > + * Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU
> > + * setup, as it requires access to the PCI config space.
> > */
>
> Again, this isn't terribly clear IMO.
>
> "ahead of IOMMU setup, as the IOMMUs might not all live on segment 0." ?
It's not just IOMMUs, but for example on VT-d we also need to poke at
the config space of bridges, and when such bridges live in segment > 0
that results in garbage being returned.
I'm not sure acpi_iommu_init() accesses the IOMMU PCI device config
space, but it does at least access the config space of bridges in
order to detect hierarchy. See how acpi_parse_dev_scope() performs
PCI reads.
What about using:
/*
* Initialize PCI (create segment 0, setup MMCFG access) ahead of IOMMU
* setup, as devices in segment > 0 must also be discoverable.
*/
Thanks, Roger.
