Re: [PATCH v2] ACPI: NUMA: Fix overlap when extending memblks to fill CFMWS

[Alison Schofield <alison.schofield@intel.com>]

On Fri, Dec 29, 2023 at 06:43:26PM +0100, Rafael J. Wysocki wrote:
> On Sat, Dec 23, 2023 at 1:10 AM <alison.schofield@intel.com> wrote:
> >
> > From: Alison Schofield <alison.schofield@intel.com>
> >
> > When the BIOS only partially describes a CFMWS Window in the SRAT
> > the acpi driver uses numa_fill_memblks() to extend existing memblk(s)
> > to fill the entire CFMWS Window, thereby applying the proximity domain
> > to the entire CFMWS.
> >
> > The calculation of the memblks to fill has an off-by-one error, that
> > causes numa_init to fail when it sees the overlap:
> >
> > [] ACPI: SRAT: Node 0 PXM 0 [mem 0x00000000-0x7fffffff]
> > [] ACPI: SRAT: Node 0 PXM 0 [mem 0x100000000-0xffffffffff]
> > [] ACPI: SRAT: Node 1 PXM 1 [mem 0x10000000000-0x1ffffffffff]
> > [] node 0 [mem 0x100000000-0xffffffffff] overlaps with node 1 [mem 0x100000000-0x1ffffffffff]
> >
> > Fix by making the 'end' parameter to numa_fill_memblks() exclusive.
> >
> > Fixes: 8f1004679987 ("ACPI/NUMA: Apply SRAT proximity domain to entire CFMWS window")
> > Reported-by: "Huang, Ying" <ying.huang@intel.com>
> > Suggested-by: Dan Williams <dan.j.williams@intel.com>
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >
> > Changes in v2:
> > - Send to ACPI maintainer, reviewer, and mailing list.
> >
> >
> >  drivers/acpi/numa/srat.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> > index 12f330b0eac0..b99062f7c412 100644
> > --- a/drivers/acpi/numa/srat.c
> > +++ b/drivers/acpi/numa/srat.c
> > @@ -308,7 +308,7 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
> >
> >         cfmws = (struct acpi_cedt_cfmws *)header;
> >         start = cfmws->base_hpa;
> > -       end = cfmws->base_hpa + cfmws->window_size;
> > +       end = cfmws->base_hpa + cfmws->window_size - 1;
> >
> >         /*
> >          * The SRAT may have already described NUMA details for all,
> >
> > base-commit: b85ea95d086471afb4ad062012a4d73cd328fa86
> > --
> 
> Applied as 6.8 material, thanks!

Sorry Rafael - this patch is bad.  Can you back it out?

I got some feedback here that showed this breaks a subsequent call
to numa_add_memblks().
https://lore.kernel.org/linux-cxl/ZYyozZcK9g4JE11B@aschofie-mobl2/T/#m16f13cafc186c7ebbf6037b3ae0e3ae572a83f77

ATM I expect this will lead to a fix in x86/mm/numa, not ACPI.

My apologies for messing up the mailing list and causing this confusion.

Alison