From: Dave Jiang <dave.jiang@intel.com>
To: Shiyang Ruan <ruansy.fnst@fujitsu.com>,
qemu-devel@nongnu.org, linux-cxl@vger.kernel.org
Cc: Jonathan.Cameron@huawei.com, dan.j.williams@intel.com,
dave@stgolabs.net, ira.weiny@intel.com,
alison.schofield@intel.com
Subject: Re: [PATCH v3 2/2] cxl/core: add poison creation event handler
Date: Wed, 17 Apr 2024 10:30:33 -0700 [thread overview]
Message-ID: <13652e98-3a70-4946-b8b0-be11032ca431@intel.com> (raw)
In-Reply-To: <20240417075053.3273543-3-ruansy.fnst@fujitsu.com>
On 4/17/24 12:50 AM, Shiyang Ruan wrote:
> Currently driver only traces cxl events, poison creation (for both vmem
> and pmem type) on cxl memdev is silent. OS needs to be notified then it
> could handle poison pages in time. Per CXL spec, the device error event
> could be signaled through FW-First and OS-First methods.
Please consider below for better clarity:
Currently the driver only traces CXL events. Poison creation (for both ram
and pmem type) on a CXL memdev is silent. The OS needs to be notified so it
can handle poison pages. Per CXL spec, the device error event
can be signaled through the FW-First method or the OS-First method.
>
> So, add poison creation event handler in OS-First method:
> - Qemu:
> - CXL device reports POISON creation event to OS by MSI by sending
> GMER/DER after injecting a poison record;
Can probably drop the QEMU changes and this is the kernel commit log.
> - CXL driver:
> a. parse the POISON event from GMER/DER;
> b. translate poisoned DPA to HPA (PFN);
> c. enqueue poisoned PFN to memory_failure's work queue;
>
> Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
> ---
> drivers/cxl/core/mbox.c | 119 +++++++++++++++++++++++++++++++++-----
> drivers/cxl/cxlmem.h | 8 +--
> include/linux/cxl-event.h | 18 +++++-
> 3 files changed, 125 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index f0f54aeccc87..76af0d73859d 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -837,25 +837,116 @@ int cxl_enumerate_cmds(struct cxl_memdev_state *mds)
> }
> EXPORT_SYMBOL_NS_GPL(cxl_enumerate_cmds, CXL);
>
> -void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> - enum cxl_event_log_type type,
> - enum cxl_event_type event_type,
> - const uuid_t *uuid, union cxl_event *evt)
> +static void cxl_report_poison(struct cxl_memdev *cxlmd, struct cxl_region *cxlr,
I think this needs to be changed to __cxl_report_poison() and the function below to cxl_report_poison(). Otherwise it goes against typical Linux methodology of having the __functionX() as the raw functionality function called by a functionX() wrapper.
DJ
> + u64 dpa)
> {
> - if (event_type == CXL_CPER_EVENT_GEN_MEDIA)
> + u64 hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);
> + unsigned long pfn = PHYS_PFN(hpa);
> +
> + if (!IS_ENABLED(CONFIG_MEMORY_FAILURE))
> + return;
> +
> + memory_failure_queue(pfn, MF_ACTION_REQUIRED);
> +}
> +
> +static int __cxl_report_poison(struct device *dev, void *arg)
> +{
> + struct cxl_endpoint_decoder *cxled;
> + struct cxl_memdev *cxlmd;
> + u64 dpa = *(u64 *)arg;
> +
> + cxled = to_cxl_endpoint_decoder(dev);
> + if (!cxled || !cxled->dpa_res || !resource_size(cxled->dpa_res))
> + return 0;
> +
> + if (cxled->mode == CXL_DECODER_MIXED) {
> + dev_dbg(dev, "poison list read unsupported in mixed mode\n");
> + return 0;
> + }
> +
> + if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
> + return 0;
> +
> + cxlmd = cxled_to_memdev(cxled);
> + cxl_report_poison(cxlmd, cxled->cxld.region, dpa);
> +
> + return 1;
> +}
> +
> +static void cxl_event_handle_poison(struct cxl_memdev *cxlmd, u64 dpa)
> +{
> + struct cxl_port *port = cxlmd->endpoint;
> +
> + /*
> + * No region is mapped to this endpoint, that is to say no HPA is
> + * mapped.
> + */
> + if (!port || !is_cxl_endpoint(port) ||
> + cxl_num_decoders_committed(port) == 0)
> + return;
> +
> + device_for_each_child(&port->dev, &dpa, __cxl_report_poison);
> +}
> +
> +static void cxl_event_handle_general_media(struct cxl_memdev *cxlmd,
> + enum cxl_event_log_type type,
> + struct cxl_event_gen_media *rec)
> +{
> + u64 dpa = le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK;
> +
> + if (type == CXL_EVENT_TYPE_FAIL) {
> + switch (rec->transaction_type) {
> + case CXL_EVENT_TRANSACTION_READ:
> + case CXL_EVENT_TRANSACTION_WRITE:
> + case CXL_EVENT_TRANSACTION_INJECT_POISON:
> + cxl_event_handle_poison(cxlmd, dpa);
> + break;
> + default:
> + break;
> + }
> + }
> +}
> +
> +static void cxl_event_handle_dram(struct cxl_memdev *cxlmd,
> + enum cxl_event_log_type type,
> + struct cxl_event_dram *rec)
> +{
> + u64 dpa = le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK;
> +
> + if (type == CXL_EVENT_TYPE_FAIL) {
> + switch (rec->transaction_type) {
> + case CXL_EVENT_TRANSACTION_READ:
> + case CXL_EVENT_TRANSACTION_WRITE:
> + case CXL_EVENT_TRANSACTION_INJECT_POISON:
> + cxl_event_handle_poison(cxlmd, dpa);
> + break;
> + default:
> + break;
> + }
> + }
> +}
> +
> +void cxl_event_handle_record(struct cxl_memdev *cxlmd,
> + enum cxl_event_log_type type,
> + enum cxl_event_type event_type,
> + const uuid_t *uuid, union cxl_event *evt)
> +{
> + if (event_type == CXL_CPER_EVENT_GEN_MEDIA) {
> trace_cxl_general_media(cxlmd, type, &evt->gen_media);
> - else if (event_type == CXL_CPER_EVENT_DRAM)
> + cxl_event_handle_general_media(cxlmd, type, &evt->gen_media);
> + } else if (event_type == CXL_CPER_EVENT_DRAM) {
> trace_cxl_dram(cxlmd, type, &evt->dram);
> - else if (event_type == CXL_CPER_EVENT_MEM_MODULE)
> + cxl_event_handle_dram(cxlmd, type, &evt->dram);
> + } else if (event_type == CXL_CPER_EVENT_MEM_MODULE)
> trace_cxl_memory_module(cxlmd, type, &evt->mem_module);
> else
> trace_cxl_generic_event(cxlmd, type, uuid, &evt->generic);
> }
> -EXPORT_SYMBOL_NS_GPL(cxl_event_trace_record, CXL);
> +EXPORT_SYMBOL_NS_GPL(cxl_event_handle_record, CXL);
>
> -static void __cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> - enum cxl_event_log_type type,
> - struct cxl_event_record_raw *record)
> +static void __cxl_event_handle_record(struct cxl_memdev *cxlmd,
> + enum cxl_event_log_type type,
> + struct cxl_event_record_raw *record)
> {
> enum cxl_event_type ev_type = CXL_CPER_EVENT_GENERIC;
> const uuid_t *uuid = &record->id;
> @@ -867,7 +958,7 @@ static void __cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> else if (uuid_equal(uuid, &CXL_EVENT_MEM_MODULE_UUID))
> ev_type = CXL_CPER_EVENT_MEM_MODULE;
>
> - cxl_event_trace_record(cxlmd, type, ev_type, uuid, &record->event);
> + cxl_event_handle_record(cxlmd, type, ev_type, uuid, &record->event);
> }
>
> static int cxl_clear_event_record(struct cxl_memdev_state *mds,
> @@ -979,8 +1070,8 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> break;
>
> for (i = 0; i < nr_rec; i++)
> - __cxl_event_trace_record(cxlmd, type,
> - &payload->records[i]);
> + __cxl_event_handle_record(cxlmd, type,
> + &payload->records[i]);
>
> if (payload->flags & CXL_GET_EVENT_FLAG_OVERFLOW)
> trace_cxl_overflow(cxlmd, type, payload);
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 36cee9c30ceb..ba1347de5651 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -822,10 +822,10 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> unsigned long *cmds);
> void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
> -void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
> - enum cxl_event_log_type type,
> - enum cxl_event_type event_type,
> - const uuid_t *uuid, union cxl_event *evt);
> +void cxl_event_handle_record(struct cxl_memdev *cxlmd,
> + enum cxl_event_log_type type,
> + enum cxl_event_type event_type,
> + const uuid_t *uuid, union cxl_event *evt);
> int cxl_set_timestamp(struct cxl_memdev_state *mds);
> int cxl_poison_state_init(struct cxl_memdev_state *mds);
> int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
> diff --git a/include/linux/cxl-event.h b/include/linux/cxl-event.h
> index 03fa6d50d46f..8189bed76c12 100644
> --- a/include/linux/cxl-event.h
> +++ b/include/linux/cxl-event.h
> @@ -23,6 +23,20 @@ struct cxl_event_generic {
> u8 data[CXL_EVENT_RECORD_DATA_LENGTH];
> } __packed;
>
> +/*
> + * Event transaction type
> + * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
> + */
> +enum cxl_event_transaction_type {
> + CXL_EVENT_TRANSACTION_UNKNOWN = 0X00,
> + CXL_EVENT_TRANSACTION_READ,
> + CXL_EVENT_TRANSACTION_WRITE,
> + CXL_EVENT_TRANSACTION_SCAN_MEDIA,
> + CXL_EVENT_TRANSACTION_INJECT_POISON,
> + CXL_EVENT_TRANSACTION_MEDIA_SCRUB,
> + CXL_EVENT_TRANSACTION_MEDIA_MANAGEMENT,
> +};
> +
> /*
> * General Media Event Record
> * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
> @@ -33,7 +47,7 @@ struct cxl_event_gen_media {
> __le64 phys_addr;
> u8 descriptor;
> u8 type;
> - u8 transaction_type;
> + u8 transaction_type; /* enum cxl_event_transaction_type */
> u8 validity_flags[2];
> u8 channel;
> u8 rank;
> @@ -52,7 +66,7 @@ struct cxl_event_dram {
> __le64 phys_addr;
> u8 descriptor;
> u8 type;
> - u8 transaction_type;
> + u8 transaction_type; /* enum cxl_event_transaction_type */
> u8 validity_flags[2];
> u8 channel;
> u8 rank;
next prev parent reply other threads:[~2024-04-17 17:31 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-17 7:50 [PATCH v3 0/2] cxl: add poison creation event handler Shiyang Ruan via
2024-04-17 7:50 ` [PATCH v3 1/2] cxl/core: correct length of DPA field masks Shiyang Ruan via
2024-04-23 17:35 ` Ira Weiny
2024-04-23 17:35 ` Dan Williams
2024-04-23 17:42 ` Alison Schofield
2024-04-23 21:04 ` Ira Weiny
2024-04-25 10:05 ` Shiyang Ruan via
2024-04-25 16:04 ` Ira Weiny
2024-04-30 21:00 ` Alison Schofield
2024-05-03 11:37 ` Shiyang Ruan via
2024-04-17 7:50 ` [PATCH v3 2/2] cxl/core: add poison creation event handler Shiyang Ruan via
2024-04-17 17:30 ` Dave Jiang [this message]
2024-04-18 9:01 ` Shiyang Ruan via
2024-04-21 12:14 ` kernel test robot
2024-04-23 17:57 ` Ira Weiny
2024-05-03 10:42 ` Shiyang Ruan via
2024-05-08 16:15 ` Jonathan Cameron via
2024-04-23 18:40 ` Dan Williams
2024-05-03 11:32 ` Shiyang Ruan via
2024-05-21 5:35 ` Shiyang Ruan via
2024-05-22 6:45 ` Dan Williams
2024-05-24 15:15 ` Shiyang Ruan via
2024-05-28 10:13 ` Shiyang Ruan via
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=13652e98-3a70-4946-b8b0-be11032ca431@intel.com \
--to=dave.jiang@intel.com \
--cc=Jonathan.Cameron@huawei.com \
--cc=alison.schofield@intel.com \
--cc=dan.j.williams@intel.com \
--cc=dave@stgolabs.net \
--cc=ira.weiny@intel.com \
--cc=linux-cxl@vger.kernel.org \
--cc=qemu-devel@nongnu.org \
--cc=ruansy.fnst@fujitsu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).