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