linux-kernel - Re: [PATCH v6 3/3] acpi,srat: give memory block size advice based on CFMWS alignment

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZzPpFzdzu-cis114@PC2K9PVX.TheFacebook.com>
Date: Tue, 12 Nov 2024 18:47:35 -0500
From: Gregory Price <gourry@...rry.net>
To: Dan Williams <dan.j.williams@...el.com>
Cc: x86@...nel.org, linux-kernel@...r.kernel.org,
	linux-acpi@...r.kernel.org, linux-mm@...ck.org,
	linux-cxl@...r.kernel.org, kernel-team@...a.com,
	Jonathan.Cameron@...wei.com, rrichter@....com, Terry.Bowman@....com,
	dave.jiang@...el.com, ira.weiny@...el.com,
	alison.schofield@...el.com, dave.hansen@...ux.intel.com,
	luto@...nel.org, peterz@...radead.org, tglx@...utronix.de,
	mingo@...hat.com, bp@...en8.de, hpa@...or.com, rafael@...nel.org,
	lenb@...nel.org, david@...hat.com, osalvador@...e.de,
	gregkh@...uxfoundation.org, akpm@...ux-foundation.org,
	rppt@...nel.org
Subject: Re: [PATCH v6 3/3] acpi,srat: give memory block size advice based on
 CFMWS alignment

On Tue, Nov 12, 2024 at 01:41:55PM -0800, Dan Williams wrote:
> Gregory Price wrote:
> > Capacity is stranded when CFMWS regions are not aligned to block size.
> > On x86, block size increases with capacity (2G blocks @ 64G capacity).
> > 
> > Use CFMWS base/size to report memory block size alignment advice.
> > 
> > Suggested-by: Dan Williams <dan.j.williams@...el.com>
> > Signed-off-by: Gregory Price <gourry@...rry.net>
> > Acked-by: Mike Rapoport (Microsoft) <rppt@...nel.org>
> > Acked-by: David Hildenbrand <david@...hat.com>
> > ---
> >  drivers/acpi/numa/srat.c | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/numa/srat.c b/drivers/acpi/numa/srat.c
> > index 44f91f2c6c5d..34b6993e7d6c 100644
> > --- a/drivers/acpi/numa/srat.c
> > +++ b/drivers/acpi/numa/srat.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/errno.h>
> >  #include <linux/acpi.h>
> >  #include <linux/memblock.h>
> > +#include <linux/memory.h>
> >  #include <linux/numa.h>
> >  #include <linux/nodemask.h>
> >  #include <linux/topology.h>
> > @@ -338,13 +339,22 @@ static int __init acpi_parse_cfmws(union acpi_subtable_headers *header,
> >  {
> >  	struct acpi_cedt_cfmws *cfmws;
> >  	int *fake_pxm = arg;
> > -	u64 start, end;
> > +	u64 start, end, align;
> >  	int node;
> >  
> >  	cfmws = (struct acpi_cedt_cfmws *)header;
> >  	start = cfmws->base_hpa;
> >  	end = cfmws->base_hpa + cfmws->window_size;
> >  
> > +	/* Align memblock size to CFMW regions if possible */
> > +	align = 1UL << __ffs(start | end);
> > +	if (align >= SZ_256M) {
> > +		if (memory_block_advise_max_size(align) < 0)
> > +			pr_warn("CFMWS: memblock size advise failed\n");
> 
> Oh, this made me go back to look at what happens if CFMWS has multiple
> alignment suggestions. Should not memory_block_advise_max_size() be
> considering the max advice?
> 
>     if (memory_block_advised_size) {
>         ...    
>     } else {
>             memory_block_advised_size = max(memory_block_advised_size, size);
>     }
> 
> For example, if region0 is an x4 region and region1 is an x1 region then
> the memory block size should be 1GB, not 256M. I.e. CFMWS alignment
> follows CXL hardware decoder alignment of "256M * InterleaveWays".

Max size to minimize capacity loss to due alignment truncation.

If CFMW-0 is aligned at 1GB and CFMW-1 is aligned at 256MB, if you select 1GB
then some portion of CFMW-1 will be unmappable.

so you want min(memory_block_advised_size, size) to ensure the hotplug memblock
size aligns to the *smallest* CFMW (or any other source) alignment.

Unless I'm misunderstanding your feedback here.


I'm not clear on why the interleave data is relevant here - that just tells us
how decoders line up with the memory region described in the CFMW.  The window
still gets chopped up into N memblocks of memory_block_advised_size.

~Gregory