Re: [Xen-devel] [PATCH] x86/mmcfg/drhd: Move acpi_mmcfg_init() before calling acpi_parse_dmar()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.08.18 at 11:13, <zhenzhong.duan@xxxxxxxxxx> wrote:
> On 2018/8/16 15:10, Jan Beulich wrote:
>> Have you investigated the alternative of deferring acpi_dmar_init()
>> to a later point, or at least the part of it that needs to do PCI
>> config space accesses? I'm not currently convinced the device scope
>> parsing needs to happen that early: Neither iommu_supports_eim()
>> nor iommu_enable_x2apic_IR() look to depend on that information
>> at the first glance, and I think these are the routines that require
>> (part of) the DMAR parsing to happen early.
> I moved acpi_mmcfg_init() ahead of acpi_boot_init() because below code 
> sequence depending on pci_mmcfg_virt being correctly setup.
> acpi_dmar_init
>    ->acpi_parse_dmar
>      ->acpi_parse_one_drhd
>        ->acpi_parse_dev_scope
>          ->pci_conf_read8
>            ->pci_mmcfg_read
>              ->pci_dev_base
>                ->get_virt

Have you read my previous response in full? I'm specifically asking
whether the device scope parsing (i.e. acpi_parse_dev_scope())
really needs to happen as early as it does now. Without that, the
dependency on MMCFG accesses working would go away.

>>> --- a/xen/arch/x86/setup.c
>>> +++ b/xen/arch/x86/setup.c
>>> @@ -1493,6 +1493,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>>>   
>>>       generic_apic_probe();
>>>   
>>> +    pt_pci_init();
>>> +
>>> +    acpi_mmcfg_init();
>>> +
>>>       acpi_boot_init();
>> 
>> With the dependency being _in_ acpi_boot_init(), the invocation of
>> acpi_mmcfg_init() should now be moved there.
> Yes, I feel better to move pt_pci_init() and acpi_mmcfg_init() in 
> acpi_boot_init() before acpi_mmcfg_init().

I didn't say move both, did I?

That said, now having looked at what it actually does, I think you want
to rename it if the suggested alternative route can't be used, as in
particular the pt_ prefix is quite misleading then. Once that has
happened, moving the invocation perhaps even _into_ acpi_mcfg_init()
might be reasonable.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel