Hi Jan,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 2022年11月9日 0:55
> To: Wei Chen <wei.c...@arm.com>
> Cc: nd <n...@arm.com>; Andrew Cooper <andrew.coop...@citrix.com>; Roger Pau
> Monné <roger....@citrix.com>; Wei Liu <w...@xen.org>; George Dunlap
> <george.dun...@citrix.com>; Julien Grall <jul...@xen.org>; Stefano
> Stabellini <sstabell...@kernel.org>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v7 5/6] xen/x86: move NUMA process nodes nodes code
> from x86 to common
>
> > mem_hotplug is accessed by common code if memory hotplug is
> > activated. Even if this is only supported by x86, export the
> > variable so that other architectures could support it in the future.
> >
> > As asm/acpi.h has been removed from common/numa.c, we have to
> > move NR_NODE_MEMBLKS from asm/acpi.h to xen/numa.h in this patch
> > as well.
> >
> > Signed-off-by: Wei Chen <wei.c...@arm.com>
>
> There's just one remaining concern I have: I continue to consider ...
>
> > @@ -341,159 +247,14 @@ acpi_numa_memory_affinity_init(const struct
> acpi_srat_mem_affinity *ma)
> > pxm &= 0xff;
> > node = setup_node(pxm);
> > if (node == NUMA_NO_NODE) {
> > - bad_srat();
> > + numa_fw_bad();
> > return;
> > - }
> > - } while (found && start < end);
> > -
> > - if (start < end) {
> > - printk(KERN_ERR "NUMA: No NODE for RAM range: "
> > - "[%"PRIpaddr", %"PRIpaddr"]\n", start, end - 1);
> > - return 0;
> > - }
> > - }
> > - return 1;
> > + numa_fw_nid_name = "PXM";
>
> ... this to be happening too late. Not because I can see a way for current
> code to use the variable earlier, but because of the risk of future code
> potentially doing so. Afaics srat_parse_regions() is called quite a bit
> earlier, so perhaps the field should (also?) be set there, presumably
> after acpi_table_parse() has succeeded. I've included "(also?)" because I
> think to be on the safe side the setting here may want keeping, albeit
> perhaps moving up in the function.
>
When I was composing this patch, I also thought current place to call this
"PXM" setting would be a little late. But since there is only one function
that uses this prefix right now, I thought it was acceptable at the time.
But obviously your concerns make sense, I will move this call to
srat_parse_regions after acpi_table_parse has been done successfully.
Cheers,
Wei Chen
> Jan
