Ok, that make sense for me. Thanks for your detailed explanation, I will 
rewrite the patch next week.

Sent from mobile

2018年8月17日 20:28于 Jan Beulich <jbeul...@suse.com>写道:
>
> >>> On 17.08.18 at 09:01, <zhenzhong.d...@oracle.com> wrote: 
> > pci_conf_read8() needs pci mmcfg mapping to work on multiple pci segments 
> > system such as HPE Superdome-Flex. 
> > 
> > Move acpi_mmcfg_init() call in acpi_boot_init() before calling 
> > acpi_parse_dmar() so that when pci_conf_read8() is called in 
> > acpi_parse_dev_scope(), we already have the mapping set up. 
> > 
> > acpi_mmcfg_init() only setup mmcfg mapping and has some global variables 
> > initialized so there is no hazard to move it ahead. 
>
> I'm afraid this is a gross understatement. "Setup mappings" 
> alone is already putting such movement under question. Have 
> you checked explicitly that the initialization needed for this 
> setting up of mappings, if any, has actually occurred before 
> the new point the function gets called? 
>
> In particular, mmio_ro_ranges looks to get set up only after 
> the call to acpi_boot_init(). I guess you didn't see a crash 
> solely because you also move the invocation across the call 
> to zap_low_mappings(). 
>
> Similary pci_mmcfg_check_hostbridge() doesn't look all that 
> trivial. 
>
> Please may I ask that you be quite a bit more careful here, 
> even more so when you've been warned already? 
>
> > Meanwhile from its 
> > name, acpi_boot_init() is a proper place to call it. 
> > 
> > The alternative is moving the acpi_parse_dev_scope() call to a later point 
> > after acpi_mmcfg_init(). But acpi_parse_one_drhd(), acpi_parse_one_rmrr() 
> > and acpi_parse_one_atsr() all called acpi_parse_dev_scope() as their main 
> > job. Splitting those functions to two pieces looks less optimal and 
> > meaningless. 
>
> Certainly not meaningless; I'm also not sure why you consider 
> the device scope parsing their main job. It's perhaps their 
> most involved part, but the fact alone that for the purposes 
> here we could probably get away without that part already 
> suggests to me that this is a secondary (yet necessary) aspect. 
>
> Furthermore MMCFG will continue to not work this early (or 
> more precisely not at all until Dom0 boot has progressed far 
> enough) if the range(s) isn't/aren't marked reserved in E820. 
> It would be worthwhile saying half a sentence to this effect 
> in the description. 
>
> Jan 
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Reply via email to