| Message ID | 20240329063614.362763-2-ruansy.fnst@fujitsu.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl: add poison event handler | expand |
Shiyang Ruan wrote: > The length of Physical Address in General Media Event Record/DRAM Event > Record is 64-bit, so the field mask should be defined as such length. > Otherwise, this causes cxl_general_media and cxl_dram tracepoints to > mask off the upper-32-bits of DPA addresses. The cxl_poison event is > unaffected. > > If userspace was doing its own DPA-to-HPA translation this could lead to > incorrect page retirement decisions, but there is no known consumer > (like rasdaemon) of this event today. > > Fixes: d54a531a430b ("cxl/mem: Trace General Media Event Record") > Cc: <stable@vger.kernel.org> > Cc: Dan Williams <dan.j.williams@intel.com> > Cc: Davidlohr Bueso <dave@stgolabs.net> > Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Cc: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > --- > drivers/cxl/core/trace.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h > index e5f13260fc52..e2d1f296df97 100644 > --- a/drivers/cxl/core/trace.h > +++ b/drivers/cxl/core/trace.h > @@ -253,11 +253,11 @@ 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_FLAGS_MASK 0x3FULL This change makes sense... > #define CXL_DPA_MASK (~CXL_DPA_FLAGS_MASK) > > -#define CXL_DPA_VOLATILE BIT(0) > -#define CXL_DPA_NOT_REPAIRABLE BIT(1) > +#define CXL_DPA_VOLATILE BIT_ULL(0) > +#define CXL_DPA_NOT_REPAIRABLE BIT_ULL(1) ...these do not. The argument to __print_flags() is an unsigned long, so they will be cast down to (unsigned long), and they are never used as a mask so the generated code should not change.
在 2024/3/30 9:37, Dan Williams 写道: > Shiyang Ruan wrote: >> The length of Physical Address in General Media Event Record/DRAM Event >> Record is 64-bit, so the field mask should be defined as such length. >> Otherwise, this causes cxl_general_media and cxl_dram tracepoints to >> mask off the upper-32-bits of DPA addresses. The cxl_poison event is >> unaffected. >> >> If userspace was doing its own DPA-to-HPA translation this could lead to >> incorrect page retirement decisions, but there is no known consumer >> (like rasdaemon) of this event today. >> >> Fixes: d54a531a430b ("cxl/mem: Trace General Media Event Record") >> Cc: <stable@vger.kernel.org> >> Cc: Dan Williams <dan.j.williams@intel.com> >> Cc: Davidlohr Bueso <dave@stgolabs.net> >> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> >> Cc: Ira Weiny <ira.weiny@intel.com> >> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> >> --- >> drivers/cxl/core/trace.h | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h >> index e5f13260fc52..e2d1f296df97 100644 >> --- a/drivers/cxl/core/trace.h >> +++ b/drivers/cxl/core/trace.h >> @@ -253,11 +253,11 @@ 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_FLAGS_MASK 0x3FULL > > This change makes sense... > >> #define CXL_DPA_MASK (~CXL_DPA_FLAGS_MASK) >> >> -#define CXL_DPA_VOLATILE BIT(0) >> -#define CXL_DPA_NOT_REPAIRABLE BIT(1) >> +#define CXL_DPA_VOLATILE BIT_ULL(0) >> +#define CXL_DPA_NOT_REPAIRABLE BIT_ULL(1) > > ...these do not. The argument to __print_flags() is an unsigned long, so > they will be cast down to (unsigned long), and they are never used as a > mask so the generated code should not change. They will only used to check if such flag is set, not used as mask. So, yes, I'll remove these changes. -- Thanks, Ruan.
diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h index e5f13260fc52..e2d1f296df97 100644 --- a/drivers/cxl/core/trace.h +++ b/drivers/cxl/core/trace.h @@ -253,11 +253,11 @@ 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_FLAGS_MASK 0x3FULL #define CXL_DPA_MASK (~CXL_DPA_FLAGS_MASK) -#define CXL_DPA_VOLATILE BIT(0) -#define CXL_DPA_NOT_REPAIRABLE BIT(1) +#define CXL_DPA_VOLATILE BIT_ULL(0) +#define CXL_DPA_NOT_REPAIRABLE BIT_ULL(1) #define show_dpa_flags(flags) __print_flags(flags, "|", \ { CXL_DPA_VOLATILE, "VOLATILE" }, \ { CXL_DPA_NOT_REPAIRABLE, "NOT_REPAIRABLE" } \
The length of Physical Address in General Media Event Record/DRAM Event Record is 64-bit, so the field mask should be defined as such length. Otherwise, this causes cxl_general_media and cxl_dram tracepoints to mask off the upper-32-bits of DPA addresses. The cxl_poison event is unaffected. If userspace was doing its own DPA-to-HPA translation this could lead to incorrect page retirement decisions, but there is no known consumer (like rasdaemon) of this event today. Fixes: d54a531a430b ("cxl/mem: Trace General Media Event Record") Cc: <stable@vger.kernel.org> Cc: Dan Williams <dan.j.williams@intel.com> Cc: Davidlohr Bueso <dave@stgolabs.net> Cc: Jonathan Cameron <Jonathan.Cameron@huawei.com> Cc: Ira Weiny <ira.weiny@intel.com> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> --- drivers/cxl/core/trace.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)