Re: [PATCH v3 2/2] cxl/edac: Mask DPA flags in the event record cache key

[Richard Cheng <icheng@nvidia.com>]

On Sun, Nov 01, 2026 at 10:34:20AM +0800, Jongwon Lee wrote:
> The low bits of the DPA in a General Media or DRAM event record are
> flags: bit 0 is VOLATILE and bit 1 is NOT_REPAIRABLE. The trace events
> report the DPA with CXL_DPA_MASK applied, but cxl_store_rec_gen_media()
> and cxl_store_rec_dram() use the raw value as the xarray key.
> 
> User space reads the DPA from the trace event and writes it to the
> mem_repair dpa attribute. If the record had any flag set, xa_load()
> misses it and the repair fails with -EINVAL. The raw key also lets a
> repair go through for a record the device marked NOT_REPAIRABLE, if the
> DPA is written with bit 1 set.
> 
> Key the records by the masked DPA and skip records marked
> NOT_REPAIRABLE. cxl_find_rec_gen_media() now reads the record, so do
> the lookup under the xarray lock as cxl_find_rec_dram() does, and
> return a bool since its only caller tests the result.
> 
> Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
> Signed-off-by: Jongwon Lee <ai@linux.com>
> ---
>  drivers/cxl/core/edac.c | 43 ++++++++++++++++++++++++++---------------
>  1 file changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index f0c12871bd..94d44a3dc1 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -918,24 +918,37 @@ struct cxl_mem_repair_attrbs {
>  	enum cxl_mem_repair_type repair_type;
>  };
>  
> -static struct cxl_event_gen_media *
> -cxl_find_rec_gen_media(struct cxl_memdev *cxlmd,
> -		       struct cxl_mem_repair_attrbs *attrbs)
> +/*
> + * The low bits of the event record DPA are flags (VOLATILE, NOT_REPAIRABLE).
> + * Records are looked up by the DPA user space writes back, which comes from
> + * the trace event and has the flags masked off, so key them the same way.
> + */
> +static unsigned long cxl_rec_key(const struct cxl_event_media_hdr *hdr)
> +{
> +	return le64_to_cpu(hdr->phys_addr) & CXL_DPA_MASK;
> +}
> +
> +static bool cxl_rec_not_repairable(const struct cxl_event_media_hdr *hdr)
> +{
> +	return le64_to_cpu(hdr->phys_addr) & CXL_DPA_NOT_REPAIRABLE;
> +}
> +

Hi Jongwon,

Maybe something like the following to follow the pattern of rb_parent
hiding the packed address and metadata encoding ?

```
#define CXL_EVENT_DPA(hdr) \
    (le64_to_cpu((hdr)->phys_addr) & CXL_DPA_MASK)
```

Or at least make it static inline.

Best regards,
Richard Cheng.



> +static bool cxl_find_rec_gen_media(struct cxl_memdev *cxlmd,
> +				   struct cxl_mem_repair_attrbs *attrbs)
>  {
>  	struct cxl_mem_err_rec *array_rec = cxlmd->err_rec_array;
>  	struct cxl_event_gen_media *rec;
> +	bool found;
>  
> -	if (!array_rec)
> -		return NULL;
> +	if (!array_rec || attrbs->repair_type != CXL_PPR)
> +		return false;
>  
> +	xa_lock(&array_rec->rec_gen_media);
>  	rec = xa_load(&array_rec->rec_gen_media, attrbs->dpa);
> -	if (!rec)
> -		return NULL;
> +	found = rec && !cxl_rec_not_repairable(&rec->media_hdr);
> +	xa_unlock(&array_rec->rec_gen_media);
>  
> -	if (attrbs->repair_type == CXL_PPR)
> -		return rec;
> -
> -	return NULL;
> +	return found;
>  }
>  
>  /* Caller holds the xarray lock. The record may be freed once it is dropped. */
> @@ -949,7 +962,7 @@ __cxl_find_rec_dram(struct xarray *rec_xarray,
>  	lockdep_assert_held(&rec_xarray->xa_lock);
>  
>  	rec = xa_load(rec_xarray, attrbs->dpa);
> -	if (!rec)
> +	if (!rec || cxl_rec_not_repairable(&rec->media_hdr))
>  		return NULL;
>  
>  	validity_flags = get_unaligned_le16(rec->media_hdr.validity_flags);
> @@ -1130,8 +1143,7 @@ int cxl_store_rec_gen_media(struct cxl_memdev *cxlmd, union cxl_event *evt)
>  		return -ENOMEM;
>  
>  	old_rec = xa_store(&array_rec->rec_gen_media,
> -			   le64_to_cpu(rec->media_hdr.phys_addr), rec,
> -			   GFP_KERNEL);
> +			   cxl_rec_key(&rec->media_hdr), rec, GFP_KERNEL);
>  	if (xa_is_err(old_rec)) {
>  		kfree(rec);
>  		return xa_err(old_rec);
> @@ -1160,8 +1172,7 @@ int cxl_store_rec_dram(struct cxl_memdev *cxlmd, union cxl_event *evt)
>  		return -ENOMEM;
>  
>  	old_rec = xa_store(&array_rec->rec_dram,
> -			   le64_to_cpu(rec->media_hdr.phys_addr), rec,
> -			   GFP_KERNEL);
> +			   cxl_rec_key(&rec->media_hdr), rec, GFP_KERNEL);
>  	if (xa_is_err(old_rec)) {
>  		kfree(rec);
>  		return xa_err(old_rec);
> -- 
> 2.55.0
>