On Fri, Dec 29, 2023 at 9:04 PM Alison Schofield
<alison.schofield@xxxxxxxxx> wrote:
>
> 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@xxxxxxxxx> wrote:
> > >
> > > From: Alison Schofield <alison.schofield@xxxxxxxxx>
> > >
> > > 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@xxxxxxxxx>
> > > Suggested-by: Dan Williams <dan.j.williams@xxxxxxxxx>
> > > Signed-off-by: Alison Schofield <alison.schofield@xxxxxxxxx>
> > > ---
> > >
> > > 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.
No worries, thanks for letting me know.
Dropped now.
