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@xxxxxxxxx>
> > Signed-off-by: Gregory Price <gourry@xxxxxxxxxx>
> > Acked-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> > Acked-by: David Hildenbrand <david@xxxxxxxxxx>
> > ---
> > 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
