| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 20 May 2024 16:11:32 -0700
From: Dan Williams <dan.j.williams@...el.com>
To: "Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>, "Davidlohr
Bueso" <dave@...olabs.net>, Jonathan Cameron <jonathan.cameron@...wei.com>,
Dave Jiang <dave.jiang@...el.com>, Alison Schofield
<alison.schofield@...el.com>, Vishal Verma <vishal.l.verma@...el.com>, "Ira
Weiny" <ira.weiny@...el.com>, Dan Williams <dan.j.williams@...el.com>,
<linux-cxl@...r.kernel.org>, <linux-kernel@...r.kernel.org>
CC: "Fabio M. De Francesco" <fabio.m.de.francesco@...ux.intel.com>
Subject: Re: [PATCH v3] cxl/events: Use a common struct for DRAM and General
Media events
Fabio M. De Francesco wrote:
> cxl_event_common was an unfortunate naming choice and caused confusion with
> the existing Common Event Record. Furthermore, its fields didn't map all
> the common information between DRAM and General Media Events.
>
> Remove cxl_event_common and introduce cxl_event_media_hdr to record common
> information between DRAM and General Media events.
>
> cxl_event_media_hdr, which is embedded in both cxl_event_gen_media and
> cxl_event_dram, leverages the commonalities between the two events to
> simplify their respective handling.
>
> Suggested-by: Dan Williams <dan.j.williams@...el.com>
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@...ux.intel.com>
> ---
>
> - Changes for v3 -
>
> - Rework the layout of cxl_event_dram and cxl_event_gen_media
> to make a simpler change (Dan)
> - Remove a "Fixes" tag (Dan)
> - Don't use unnecessary struct_group[_tagged] (Jonathan, Ira)
> - Rewrite end extend the commit message
>
> - Link to v2 -
>
> https://lore.kernel.org/linux-cxl/20240516102116.3512377-1-fabio.m.de.francesco@linux.intel.com/
>
>
> drivers/cxl/core/mbox.c | 2 +-
> drivers/cxl/core/trace.h | 32 ++++++++++++++---------------
> include/linux/cxl-event.h | 40 +++++++++++++-----------------------
> tools/testing/cxl/test/mem.c | 30 +++++++++++++--------------
> 4 files changed, 46 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 2626f3fff201..a08f050cc1ca 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -875,7 +875,7 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> guard(rwsem_read)(&cxl_region_rwsem);
> guard(rwsem_read)(&cxl_dpa_rwsem);
>
> - dpa = le64_to_cpu(evt->common.phys_addr) & CXL_DPA_MASK;
> + dpa = le64_to_cpu(evt->media_hdr.phys_addr) & CXL_DPA_MASK;
> cxlr = cxl_dpa_to_region(cxlmd, dpa);
> if (cxlr)
> hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 07a0394b1d99..5a76f94accf4 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -340,23 +340,23 @@ TRACE_EVENT(cxl_general_media,
[..]
Changes look good.
> @@ -440,19 +440,19 @@ TRACE_EVENT(cxl_dram,
[..]
Looks good.
[..]
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index 6584443144de..63b71541d399 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -384,19 +384,19 @@ struct cxl_test_gen_media {
> struct cxl_test_gen_media gen_media = {
> .id = CXL_EVENT_GEN_MEDIA_UUID,
> .rec = {
> - .hdr = {
> + .media_hdr.hdr = {
> .length = sizeof(struct cxl_test_gen_media),
> .flags[0] = CXL_EVENT_RECORD_FLAG_PERMANENT,
> /* .handle = Set dynamically */
> .related_handle = cpu_to_le16(0),
> },
> - .phys_addr = cpu_to_le64(0x2000),
> - .descriptor = CXL_GMER_EVT_DESC_UNCORECTABLE_EVENT,
> - .type = CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR,
> - .transaction_type = CXL_GMER_TRANS_HOST_WRITE,
> + .media_hdr.phys_addr = cpu_to_le64(0x2000),
> + .media_hdr.descriptor = CXL_GMER_EVT_DESC_UNCORECTABLE_EVENT,
> + .media_hdr.type = CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR,
> + .media_hdr.transaction_type = CXL_GMER_TRANS_HOST_WRITE,
> /* .validity_flags = <set below> */
> - .channel = 1,
> - .rank = 30
> + .media_hdr.channel = 1,
> + .media_hdr.rank = 30
This looks awkward, I would have expected a conversion like:
@@ -384,19 +384,21 @@ struct cxl_test_gen_media {
struct cxl_test_gen_media gen_media = {
.id = CXL_EVENT_GEN_MEDIA_UUID,
.rec = {
- .hdr = {
- .length = sizeof(struct cxl_test_gen_media),
- .flags[0] = CXL_EVENT_RECORD_FLAG_PERMANENT,
- /* .handle = Set dynamically */
- .related_handle = cpu_to_le16(0),
+ .media_hdr = {
+ .hdr = {
+ .length = sizeof(struct cxl_test_gen_media),
+ .flags[0] = CXL_EVENT_RECORD_FLAG_PERMANENT,
+ /* .handle = Set dynamically */
+ .related_handle = cpu_to_le16(0),
+ },
+ .phys_addr = cpu_to_le64(0x2000),
+ .descriptor = CXL_GMER_EVT_DESC_UNCORECTABLE_EVENT,
+ .type = CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR,
+ .transaction_type = CXL_GMER_TRANS_HOST_WRITE,
+ /* .validity_flags = <set below> */
+ .channel = 1,
+ .rank = 30
},
- .phys_addr = cpu_to_le64(0x2000),
- .descriptor = CXL_GMER_EVT_DESC_UNCORECTABLE_EVENT,
- .type = CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR,
- .transaction_type = CXL_GMER_TRANS_HOST_WRITE,
- /* .validity_flags = <set below> */
- .channel = 1,
- .rank = 30
},
};
> },
> };
>
> @@ -408,18 +408,18 @@ struct cxl_test_dram {
> struct cxl_test_dram dram = {
> .id = CXL_EVENT_DRAM_UUID,
> .rec = {
> - .hdr = {
> + .media_hdr.hdr = {
Same comment as above, just scope the declarations with out extra
de-references.
Powered by blists - more mailing lists