Re: [PATCH v5 1/4] cxl/trace: Correct DPA field masks for general_media & dram events

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 29 Apr 2024 17:34:21 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> The length of Physical Address in General Media and DRAM event
> records is 64-bit, so the field mask for extracting the DPA should
> be 64-bit also, otherwise the trace event reports DPA's with the
> upper 32 bits of a DPA address masked off. If userspace was doing
> DPA-to-HPA translations this could lead to incorrect page retirement
> decisions, but there is no known consumer (like rasdaemon) of this
> event today.

https://github.com/mchehab/rasdaemon/blob/master/ras-cxl-handler.c#L205
Yes there is, though there may well not have been back in at v1.

> 
> Use GENMASK_ULL() for CXL_DPA_MASK to get all the DPA address bits.
> 
> Tidy up CXL_DPA_FLAGS_MASK by using GENMASK() to only mask the exact
> flag bits.
> 
> These bits are defined as part of the event record physical address
> descriptions of General Media and DRAM events in CXL Spec 3.1
> Section 8.2.9.2 Events.
> 
> Co-developed-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.CAmeron@huawei.com>
Fixes tag would probably be appropriate I think as this should
be backported.

> ---
>  drivers/cxl/core/trace.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index e5f13260fc52..7c5cd069f10c 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -253,8 +253,8 @@ TRACE_EVENT(cxl_generic_event,
>   * DRAM Event Record
>   * CXL rev 3.0 section 8.2.9.2.1.2; Table 8-44
>   */
> -#define CXL_DPA_FLAGS_MASK			0x3F
> -#define CXL_DPA_MASK				(~CXL_DPA_FLAGS_MASK)
> +#define CXL_DPA_FLAGS_MASK			GENMASK(1, 0)
> +#define CXL_DPA_MASK				GENMASK_ULL(63, 6)
>  
>  #define CXL_DPA_VOLATILE			BIT(0)
>  #define CXL_DPA_NOT_REPAIRABLE			BIT(1)