Re: [PATCH v2.2 1/6][4.16?] x86/x2APIC: defer probe until after IOMMU ACPI table parsing

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, Nov 15, 2021 at 05:10:04PM +0100, Jan Beulich wrote:
> On 15.11.2021 16:07, Roger Pau Monné wrote:
> > On Mon, Nov 15, 2021 at 03:31:39PM +0100, Jan Beulich wrote:
> >> While commit 46c4061cd2bf ("x86/IOMMU: mark IOMMU / intremap not in use
> >> when ACPI tables are missing") deals with apic_x2apic_probe() as called
> >> from x2apic_bsp_setup(), the check_x2apic_preenabled() path is similarly
> >> affected: The call needs to occur after acpi_iommu_init(), such that
> >> iommu_intremap getting disabled there can be properly taken into account
> >> by apic_x2apic_probe().
> >>
> >> Note that, for the time being (further cleanup patches following),
> >> reversing the order of the calls to generic_apic_probe() and
> >> acpi_boot_init() is not an option:
> >> - acpi_process_madt() calls clustered_apic_check() and hence relies on
> >>   genapic to have got filled before,
> >> - generic_bigsmp_probe() (called from acpi_process_madt()) needs to
> >>   occur after generic_apic_probe(),
> >> - acpi_parse_madt() (called from acpi_process_madt()) calls
> >>   acpi_madt_oem_check(), which wants to be after generic_apic_probe().
> >>
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> >> --- a/xen/arch/x86/setup.c
> >> +++ b/xen/arch/x86/setup.c
> >> @@ -1700,15 +1700,30 @@ void __init noreturn __start_xen(unsigne
> >>  
> >>      dmi_scan_machine();
> >>  
> >> -    generic_apic_probe();
> >> -
> >>      mmio_ro_ranges = rangeset_new(NULL, "r/o mmio ranges",
> >>                                    RANGESETF_prettyprint_hex);
> >>  
> >>      xsm_multiboot_init(module_map, mbi);
> >>  
> >> +    /*
> >> +     * IOMMU-related ACPI table parsing may require some of the system 
> >> domains
> >> +     * to be usable.
> > 
> > I would be a bit more specific and likely add "...to be usable in
> > order to hide IOMMU PCI devices.".
> 
> Hmm, not sure. I did specifically leave out the "why" part, as dom_io
> might also become required for something. If you nevertheless think
> I should extend the comment, then I'd insist on using "e.g." just
> like I did in the comment next to the call to generic_apic_probe().

My request was mostly to make it clear where dom_io is used, it's IMO
not trivial to spot that pci_ro_device requires the dom_io to be
setup. I'm fine if you want to use 'e.g.', or if you think this is not
helpful.

Thanks, Roger.