On Tue, Sep 14, 2021 at 11:51 PM Justin He <justin...@arm.com> wrote: [..] > > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > > > index fb775b967c52..d3a0cec635b1 100644 > > > --- a/drivers/acpi/nfit/core.c > > > +++ b/drivers/acpi/nfit/core.c > > > @@ -3005,15 +3005,8 @@ static int acpi_nfit_register_region(struct > > > acpi_nfit_desc *acpi_desc, > > > ndr_desc->res = &res; > > > ndr_desc->provider_data = nfit_spa; > > > ndr_desc->attr_groups = acpi_nfit_region_attribute_groups; > > > - if (spa->flags & ACPI_NFIT_PROXIMITY_VALID) { > > > - ndr_desc->numa_node = acpi_map_pxm_to_online_node( > > > - spa->proximity_domain); > > > - ndr_desc->target_node = acpi_map_pxm_to_node( > > > - spa->proximity_domain); > > > - } else { > > > - ndr_desc->numa_node = NUMA_NO_NODE; > > > - ndr_desc->target_node = NUMA_NO_NODE; > > > - } > > > + ndr_desc->numa_node = memory_add_physaddr_to_nid(spa->address); > > > + ndr_desc->target_node = phys_to_target_node(spa->address); > > > > > > /* > > > * Persistence domain bits are hierarchical, if > > > =================================================== > > > > > > Do you still suggest fixing like this? > > > > Are you saying that ACPI_NFIT_PROXIMITY_VALID is not set on your > > platform, or that pxm_to_node() returns NUMA_NO_NODE? > > > Latter, ACPI_NFIT_PROXIMITY_VALID is *set* in my case. > > > I would expect something like this: > > > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > > index a3ef6cce644c..95de7dc18ed8 100644 > > --- a/drivers/acpi/nfit/core.c > > +++ b/drivers/acpi/nfit/core.c > > @@ -3007,6 +3007,15 @@ static int acpi_nfit_register_region(struct > > acpi_nfit_desc *acpi_desc, > > ndr_desc->target_node = NUMA_NO_NODE; > > } > > > > + /* > > + * Fallback to address based numa information if node lookup > > + * failed > > + */ > > + if (ndr_desc->numa_node == NUMA_NO_NODE) > > + ndr_desc->numa_node = memory_add_physaddr_to_nid(spa- > > >address); > > + if (ndr_desc->target_node == NUMA_NO_NODE) > > + phys_to_target_node(spa->address); > > + > > Would it better to add a dev_info() here to report this node id changing?
Yes, given all the possibilities here, a dev_info() reporting the final result of the node mapping is justifiable.