Re: [PATCH] cxl/edac: Bounds-check ECS threshold index from device

[Yili Zhang]

On 2026-09-29 10:53, Alison Schofield wrote:
> 
> Hi Yili,
> 
> I see the appeal of keeping this as the smallest possible fix, but is the
> existing behavior for 0-2 something we intentionally want to preserve, or
> just how the current implementation works?
> 
> Documentation/ABI/testing/sysfs-edac-ecs lists 256, 1024, and 4096 as the
> threshold values. If 0 is meant to represent a reserved encoding, should
> that be part of the documented ABI?

Hi Alison,

You're right on both counts.

The 0-2 behavior is not something we want to preserve; it is just
an artifact of the sparse array's implicit zero-fill. The ABI documentation
only lists 256, 1024 and 4096 as supported values, so rather than documenting
0 as well, make the read fail for all reserved encodings and only ever report
the documented values.

> Does CXL_ECS_GET_ATTR() itself prevent that? It already has an int return
> path and can return errors from cxl_mem_ecs_get_attrbs().
> 
> Is the real limitation just the interface used by the cxl_get_ecs_*()
> helpers? Would something like this work:
> 
> 
> static int cxl_get_ecs_threshold(u8 log_cap, u16 config, u16 *threshold)
> {
> 	u8 index = FIELD_GET(CXL_ECS_THRESHOLD_COUNT_MASK, config);
> 
> 	if (index >= ARRAY_SIZE(ecs_supp_threshold) ||
> 	    !ecs_supp_threshold[index])
> 		return -EINVAL;
> 
> 	*threshold = ecs_supp_threshold[index];
> 	return 0;
> }

And you're correct that CXL_ECS_GET_ATTR() already propagates errors;
the only real limitation was the helper signature, as you sketched.
v2 restructures the cxl_get_ecs_*() helpers to return int with an
output parameter and updates the macro accordingly.

v2 follows shortly.

Thanks,
Yili