Re: [PATCH RFC 06/15] arm_mpam: Fix cache ID sentinel from ~0UL to U32_MAX to match u32 return type
From: Yin Li
Date: Thu Sep 03 2026 - 23:31:45 EST
On 9/2/2026 9:49 PM, Andre Przywara wrote:
Hi,
On 8/11/26 15:30, Yin Li wrote:
cache_of_calculate_id() returns u32, but callers stored the result in
unsigned long variables and compared against ~0UL. On 64-bit systems,
U32_MAX (0xffffffff) assigned to unsigned long becomes 0x00000000ffffffff,
which does not equal ~0UL (0xffffffffffffffff), so invalid cache IDs are
silently accepted instead of being rejected.
Yes, I agree about this one, long is not right, it's u32 (acpi) or even a plain int elsewhere (get_cpu_cacheinfo_id()).
Fix by changing all cache ID and component ID variables that receive the
return value of cache_of_calculate_id() to u32, and replace all ~0 and
~0UL sentinel comparisons with U32_MAX.
... but am not so sure about the U32_MAX change. I think ~0U is a common idiom in the kernel to mean "mask of all 1's", and while U32_MAX is the same, the _MAX part is slightly misleading here, I think.
So I think this patch should focus on dropping the long and L parts, but keep the ~0U notation. Which means cacheinfo.c should not change, for instance.
And also I wonder if that should be split up: one part to fix the existing usage in v7.3-rc1, so basically the function prototype, and the other part for the newly introduced DT code, which should then be squashed.
Hi Andre,
Thanks for the review.
On the types: agreed, and the patch already changes them from unsigned
long to u32.
On the sentinel: the patch currently uses U32_MAX, but you make a fair
point that ~0U is the more common idiom for an all-ones mask and that
_MAX reads a bit misleading here. I'll switch it back to ~0U and leave
cacheinfo.c unchanged, so the patch focuses on just the long/UL type fix.
(Drew raised the opposite on patch 02/15, but as a question rather than a strong preference — Drew, please shout if you disagree.)
As discussed with Ben, I'll fold these type fixes back into the respective inherited patches.
Thx and BRs,
Yin
Cheers,
Andre
Also fix the sentinel values in cache_of_calculate_id() itself for
consistency.
Signed-off-by: Yin Li <yin.li@xxxxxxxxxxxxxxxx>
---
drivers/base/cacheinfo.c | 6 +++---
drivers/resctrl/mpam_devices.c | 16 ++++++++--------
drivers/resctrl/mpam_internal.h | 2 +-
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/base/cacheinfo.c b/drivers/base/cacheinfo.c
index f75e7f64038b..a4e0d1d47e71 100644
--- a/drivers/base/cacheinfo.c
+++ b/drivers/base/cacheinfo.c
@@ -229,7 +229,7 @@ static bool match_cache_node(struct device_node *cpu,
u32 cache_of_calculate_id(struct device_node *cache_node)
{
struct device_node *cpu;
- u32 min_id = ~0;
+ u32 min_id = U32_MAX;
for_each_of_cpu_node(cpu) {
u64 id = of_get_cpu_hwid(cpu, 0);
@@ -237,7 +237,7 @@ u32 cache_of_calculate_id(struct device_node *cache_node)
id = arch_compact_of_hwid(id);
if (FIELD_GET(GENMASK_ULL(63, 32), id)) {
of_node_put(cpu);
- return ~0;
+ return U32_MAX;
}
if (match_cache_node(cpu, cache_node))
@@ -252,7 +252,7 @@ static void cache_of_set_id(struct cacheinfo *this_leaf,
{
u32 id = cache_of_calculate_id(cache_node);
- if (id != ~0) {
+ if (id != U32_MAX) {
this_leaf->id = id;
this_leaf->attributes |= CACHE_ID;
}
diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/ mpam_devices.c
index 559fa09128b4..ddc15249ec1e 100644
--- a/drivers/resctrl/mpam_devices.c
+++ b/drivers/resctrl/mpam_devices.c
@@ -166,13 +166,13 @@ static void mpam_free_garbage(void)
/* Called recursively to walk the list of caches from a particular CPU */
static void __mpam_get_cpumask_from_cache_id(int cpu, struct device_node *cache_node,
- unsigned long cache_id,
+ u32 cache_id,
u32 cache_level,
cpumask_t *affinity)
{
int err;
u32 iter_level;
- unsigned long iter_cache_id;
+ u32 iter_cache_id;
struct device_node *iter_node __free(device_node) = of_find_next_cache_node(cache_node);
if (!iter_node)
@@ -187,7 +187,7 @@ static void __mpam_get_cpumask_from_cache_id(int cpu, struct device_node *cache_
* during device_initcall(). Use cache_of_calculate_id().
*/
iter_cache_id = cache_of_calculate_id(iter_node);
- if (iter_cache_id == ~0UL)
+ if (iter_cache_id == U32_MAX)
return;
if (iter_level == cache_level && iter_cache_id == cache_id)
@@ -202,7 +202,7 @@ static void __mpam_get_cpumask_from_cache_id(int cpu, struct device_node *cache_
* The cacheinfo structures are only populated when CPUs are online.
* This helper walks the device tree to include offline CPUs too.
*/
-int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
+int mpam_get_cpumask_from_cache_id(u32 cache_id, u32 cache_level,
cpumask_t *affinity)
{
int cpu;
@@ -229,7 +229,7 @@ static int get_cpumask_from_cache(struct device_node *cache,
{
int err;
u32 cache_level;
- unsigned long cache_id;
+ u32 cache_id;
err = of_property_read_u32(cache, "cache-level", &cache_level);
if (err) {
@@ -238,7 +238,7 @@ static int get_cpumask_from_cache(struct device_node *cache,
}
cache_id = cache_of_calculate_id(cache);
- if (cache_id == ~0UL) {
+ if (cache_id == U32_MAX) {
pr_err("Failed to calculate cache-id from cache node\n");
return -ENOENT;
}
@@ -264,7 +264,7 @@ static int mpam_dt_parse_resource(struct mpam_msc *msc, struct device_node *np,
{
int err = 0;
u32 class_id = 0;
- unsigned long component_id = 0;
+ u32 component_id = 0;
struct device *dev = &msc->pdev->dev;
enum mpam_class_types type = MPAM_CLASS_UNKNOWN;
struct device_node *cache __free(device_node) = NULL;
@@ -308,7 +308,7 @@ static int mpam_dt_parse_resource(struct mpam_msc *msc, struct device_node *np,
return err;
}
component_id = cache_of_calculate_id(cache);
- if (component_id == ~0) {
+ if (component_id == U32_MAX) {
dev_err_once(dev, "Failed to calculate cache-id\n");
return -ENOENT;
}
diff --git a/drivers/resctrl/mpam_internal.h b/drivers/resctrl/ mpam_internal.h
index def0e3a65c23..aa45d00bcd07 100644
--- a/drivers/resctrl/mpam_internal.h
+++ b/drivers/resctrl/mpam_internal.h
@@ -470,7 +470,7 @@ int mpam_msmon_read(struct mpam_component *comp, struct mon_cfg *ctx,
enum mpam_device_features, u64 *val);
void mpam_msmon_reset_mbwu(struct mpam_component *comp, struct mon_cfg *ctx);
-int mpam_get_cpumask_from_cache_id(unsigned long cache_id, u32 cache_level,
+int mpam_get_cpumask_from_cache_id(u32 cache_id, u32 cache_level,
cpumask_t *affinity);
#ifdef CONFIG_RESCTRL_FS
--
Thx and BRs,
Yin