diff mbox series

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

Message ID 23671305ae3cd299aacb3be61d90504d6918a7f8.1714435815.git.alison.schofield@intel.com
State Superseded
Headers show
Series Add DPA->HPA translation to dram & general_media events | expand

Commit Message

Alison Schofield April 30, 2024, 12:34 a.m. UTC
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.

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>
---
 drivers/cxl/core/trace.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Ira Weiny April 30, 2024, 2:12 a.m. UTC | #1
alison.schofield@ 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.
> 
> 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: Ira Weiny <ira.weiny@intel.com>
Jonathan Cameron April 30, 2024, 4:27 p.m. UTC | #2
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)
diff mbox series

Patch

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)