resctrl operates on configuration bitmaps and a bitmap of allocated
CLOSID, both are stored in a u32.

MPAM supports configuration/portion bitmaps and PARTIDs larger
than will fit in a u32.

Add some preprocessor values that make it clear why MPAM clamps
some of these values. This will make it easier to find code related
to these values if this resctrl behaviour ever changes.

Signed-off-by: James Morse <james.morse@arm.com>
Tested-by: Carl Worth <carl@os.amperecomputing.com> # arm64
Tested-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
Reviewed-by: Shaopeng Tan <tan.shaopeng@jp.fujitsu.com>
---
 include/linux/resctrl.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/linux/resctrl.h b/include/linux/resctrl.h
index bbce79190b13..7af6c40764ed 100644
--- a/include/linux/resctrl.h
+++ b/include/linux/resctrl.h
@@ -27,6 +27,17 @@ int proc_resctrl_show(struct seq_file *m,
 /* max value for struct rdt_domain's mbps_val */
 #define MBA_MAX_MBPS   U32_MAX
 
+/*
+ * Resctrl uses a u32 as a closid bitmap. The maximum closid is 32.
+ */
+#define RESCTRL_MAX_CLOSID		32
+
+/*
+ * Resctrl uses u32 to hold the user-space config. The maximum bitmap size is
+ * 32.
+ */
+#define RESCTRL_MAX_CBM			32
+
 /* Walk all possible resources, with variants for only controls or monitors. */
 #define for_each_rdt_resource(_r)						\
 	for ((_r) = resctrl_arch_get_resource(0);				\
-- 
2.39.2

On Fri, Oct 04, 2024 at 06:03:37PM +0000, James Morse wrote:
> resctrl operates on configuration bitmaps and a bitmap of allocated
> CLOSID, both are stored in a u32.
> 
> MPAM supports configuration/portion bitmaps and PARTIDs larger
> than will fit in a u32.
> 
> Add some preprocessor values that make it clear why MPAM clamps
> some of these values. This will make it easier to find code related
> to these values if this resctrl behaviour ever changes.

...

> +#define RESCTRL_MAX_CLOSID		32

Do you really need to do this? Intel x86 architecture allows for more than
32 CLOSIDs, it's just expensive in h/w to get past 16 ... so I picked
that trivial bitmap allocator in ages past. But if ARM can have more,
then why would you need to clamp the value? File system code could ask
architecture code to allocate a CLOSID. On x86 that will fail when there
are no more CLOSIDs, so filesystem will fail the mkdir(2).

Or, since you put closid_alloc() into the filesystem code you could
change the closid_free_map to u64.

If you really do want to have this #define ... maybe you should use it
in place of the hard coded 32 here:

static void closid_init(void)
{
        struct resctrl_schema *s;
        u32 rdt_min_closid = 32;
}


> +#define RESCTRL_MAX_CBM		32

Intel x86 could plausibly expand the cache bitmap size (the MSRs
that store them currenly have bits 63:32 reserved, but that could be
changed). The only 32-bit limits are the CPUID field that enumerates
CBM_LEN and the CPUID field that enumerates the shared bitmap. The
length has space for expansion, the share bitfiled does not. So if
Intel did go to more than 32-bits we'd be stuck making sure any shared
bits were in the lower 32-bits.

-Tony

Hi Tony,

On 08/10/2024 19:50, Tony Luck wrote:
> On Fri, Oct 04, 2024 at 06:03:37PM +0000, James Morse wrote:
>> resctrl operates on configuration bitmaps and a bitmap of allocated
>> CLOSID, both are stored in a u32.
>>
>> MPAM supports configuration/portion bitmaps and PARTIDs larger
>> than will fit in a u32.
>>
>> Add some preprocessor values that make it clear why MPAM clamps
>> some of these values. This will make it easier to find code related
>> to these values if this resctrl behaviour ever changes.
> 
> ...
> 
>> +#define RESCTRL_MAX_CLOSID		32
> 
> Do you really need to do this? Intel x86 architecture allows for more than
> 32 CLOSIDs, it's just expensive in h/w to get past 16 ... so I picked
> that trivial bitmap allocator in ages past. But if ARM can have more,
> then why would you need to clamp the value?

Just to avoid touching this code now! For feature-parity - no-one can argue they have a
33-CLOSID use-case that works on x86, but not on arm64. My cunning plan was to flush out
the people who cared ...


> File system code could ask
> architecture code to allocate a CLOSID. On x86 that will fail when there
> are no more CLOSIDs, so filesystem will fail the mkdir(2).

I didn't have a strong reason to pull the allocator out to be arch-specific, so just left
it where it was. That avoided having to think about abstracting the limbo thread.


> Or, since you put closid_alloc() into the filesystem code you could
> change the closid_free_map to u64.

Marvell sent a patch doing exactly that - so they must have a platform with more than 32
PARTID. The version I'm carrying around removes the limit:
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/commit/?h=mpam/snapshot/v6.12-rc1&id=61e646352b8286b869f8ee0627cec81f01009e3e

(I did think it would be more invasive)

As the existing limit is not important I'll swap this patch with the one linked above that
removes the CLOSID limit into this series.

(The MAX_CBM bitmap size can be inferred from things like struct resctrl_staged_config's
new_ctrl u32)


> If you really do want to have this #define ... maybe you should use it
> in place of the hard coded 32 here:
> 
> static void closid_init(void)
> {
>         struct resctrl_schema *s;
>         u32 rdt_min_closid = 32;
> }

Oops - I'd missed that.


>> +#define RESCTRL_MAX_CBM		32
> 
> Intel x86 could plausibly expand the cache bitmap size (the MSRs
> that store them currenly have bits 63:32 reserved, but that could be
> changed).

> The only 32-bit limits are the CPUID field that enumerates
> CBM_LEN and the CPUID field that enumerates the shared bitmap. The
> length has space for expansion, the share bitfiled does not. So if
> Intel did go to more than 32-bits we'd be stuck making sure any shared
> bits were in the lower 32-bits.

Thanks for looking those up - I thought these limits were hard-and-fast!

MPAMs limit for the control values for the cache bitmaps is 4K - I suggest we ignore this!
MPAM can't describe a shared bitmap, if someone ever needed that, it would have to be
done by a firmware table. The CPU's max CLOSID limit is 2^16.


Thanks,

James