Hi Jan,
> -----Original Message-----
> From: Jan Beulich <jbeul...@suse.com>
> Sent: 2022年11月9日 17:30
> 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
>
> On 09.11.2022 09:51, Wei Chen wrote:
> >> -----Original Message-----
> >> From: Jan Beulich <jbeul...@suse.com>
> >> Sent: 2022年11月9日 0:55
> >>
> >>> @@ -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.
>
> As said - perhaps not move, but copy. There is an (extremely unlikely)
> case
> where srat_parse_regions() would not be called at all.
>
Got it, understand now.
Cheers,
Wei Chen
> Jan
