Re: [EXT] Re: [PATCH] hw/cxl: Add support for abort of background operation

[ajay.opensrc <ajay.opensrc@micron.com>]

>From: Davidlohr Bueso <dave@stgolabs.net>
>>From: Ajay Joshi <ajay.opensrc@micron.com>
>>
>>This patch adds the support for aborting the background
>>operation if one is progress(r3.1 8.2.9.1.5)
>>
>>"Request Abort Background Operation" command is requested
>>by the host to cancel an ongoing background operation. When
>>the command is sent, the device will abort the ongoing
>>background operation.  When the operation is aborted,
>>background command status register will maintain a completion
>>percentage value of less then 100. The "Request Abort
>>Background Operation" command support has to be advertised in
>>the Command Effects Log to be honoured.
>
>So I had a patch for this which I had not sent out, and compared
>to this one I think the below is more robust, would you mind
>testing this out?

Didn't realize you were working on one already.
This looks good to me as well. I tested this and it seems to be
working as expected. Feel free to use:

Tested-by: Ajay Joshi <ajay.opensrc@micron.com>
Reviewed-by: Ajay Joshi <ajay.opensrc@micron.com>

>Thanks,
>Davidlohr
>
>--8<--------
>[PATCH] hw/cxl: Support aborting background commands
>
>As of 3.1 spec, background commands can be canceled with a new
>abort command. Implement the support, which is advertised in
>the CEL. No ad-hoc context undoing is necessary as all the
>command logic of the running bg command is done upon completion.
>
>Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>---
>  hw/cxl/cxl-device-utils.c    |  2 +-
>  hw/cxl/cxl-mailbox-utils.c   | 39 ++++++++++++++++++++++++++++++++++--
>  include/hw/cxl/cxl_device.h  |  1 +
>  include/hw/cxl/cxl_mailbox.h |  1 +
>  4 files changed, 40 insertions(+), 3 deletions(-)
>
>diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
>index 035d034f6dd8..4f4ccfa92d82 100644
>--- a/hw/cxl/cxl-device-utils.c
>+++ b/hw/cxl/cxl-device-utils.c
>@@ -95,7 +95,7 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, >unsigned size)
>         }
>         if (offset == A_CXL_DEV_MAILBOX_STS) {
>             uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
>-            if (cci->bg.complete_pct) {
>+            if (cci->bg.complete_pct || cci->bg.aborted) {
>                 status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
>                                         0);
>                 cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
>diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>index 80a80f1ec29b..ae4b3cabbb86 100644
>--- a/hw/cxl/cxl-mailbox-utils.c
>+++ b/hw/cxl/cxl-mailbox-utils.c
>@@ -53,6 +53,7 @@ enum {
>      INFOSTAT    = 0x00,
>         #define IS_IDENTIFY   0x1
>         #define BACKGROUND_OPERATION_STATUS    0x2
>+        #define BACKGROUND_OPERATION_ABORT     0x5
>      EVENTS      = 0x01,
>         #define GET_RECORDS   0x0
>         #define CLEAR_RECORDS   0x1
>@@ -815,6 +816,33 @@ static CXLRetCode cmd_infostat_bg_op_sts(const struct >cxl_cmd *cmd,
>      return CXL_MBOX_SUCCESS;
>  }
>
>+/* CXL r3.1 Section 8.2.9.1.5: Request Abort Background Operation (Opcode >0005h) */
>+static CXLRetCode cmd_infostat_bg_op_abort(const struct cxl_cmd *cmd,
>+                                           uint8_t *payload_in,
>+                                           size_t len_in,
>+                                           uint8_t *payload_out,
>+                                           size_t *len_out,
>+                                           CXLCCI *cci)
>+{
>+    int bg_set = cci->bg.opcode >> 8;
>+    int bg_cmd = cci->bg.opcode & 0xff;
>+    const struct cxl_cmd *bg_c = &cci->cxl_cmd_set[bg_set][bg_cmd];
>+
>+    if (!(bg_c->effect & CXL_MBOX_BACKGROUND_OPERATION_ABORT)) {
>+        return CXL_MBOX_REQUEST_ABORT_NOTSUP;
>+    }
>+
>+    if (cci->bg.runtime) {
>+        assert(cci->bg.complete_pct < 100);
>+        timer_del(cci->bg.timer);
>+        cci->bg.ret_code = CXL_MBOX_ABORTED;
>+        cci->bg.starttime = 0;
>+        cci->bg.runtime = 0;
>+        cci->bg.aborted = true;
>+    }
>+    return CXL_MBOX_SUCCESS;
>+}
>+
>  /* CXL r3.1 Section 8.2.9.3.1: Get FW Info (Opcode 0200h) */
>  static CXLRetCode cmd_firmware_update_get_info(const struct cxl_cmd *cmd,
>                                                uint8_t *payload_in,
>@@ -2149,6 +2177,8 @@ static CXLRetCode cmd_dcd_release_dyn_cap(const struct >cxl_cmd *cmd,
>  }
>
>  static const struct cxl_cmd cxl_cmd_set[256][256] = {
>+    [INFOSTAT][BACKGROUND_OPERATION_ABORT] = { "BACKGROUND_OPERATION_ABORT",
>+        cmd_infostat_bg_op_abort, 0, 0 },
>      [EVENTS][GET_RECORDS] = { "EVENTS_GET_RECORDS",
>         cmd_events_get_records, 1, 0 },
>      [EVENTS][CLEAR_RECORDS] = { "EVENTS_CLEAR_RECORDS",
>@@ -2176,7 +2206,7 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>      [SANITIZE][OVERWRITE] = { "SANITIZE_OVERWRITE", cmd_sanitize_overwrite, 0,
>         (CXL_MBOX_IMMEDIATE_DATA_CHANGE |
>          CXL_MBOX_SECURITY_STATE_CHANGE |
>-         CXL_MBOX_BACKGROUND_OPERATION)},
>+         CXL_MBOX_BACKGROUND_OPERATION  | >CXL_MBOX_BACKGROUND_OPERATION_ABORT)},
>      [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE",
>         cmd_get_security_state, 0, 0 },
>      [MEDIA_AND_POISON][GET_POISON_LIST] = { >"MEDIA_AND_POISON_GET_POISON_LIST",
>@@ -2189,7 +2219,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>         "MEDIA_AND_POISON_GET_SCAN_MEDIA_CAPABILITIES",
>         cmd_media_get_scan_media_capabilities, 16, 0 },
>      [MEDIA_AND_POISON][SCAN_MEDIA] = { "MEDIA_AND_POISON_SCAN_MEDIA",
>-        cmd_media_scan_media, 17, CXL_MBOX_BACKGROUND_OPERATION },
>+        cmd_media_scan_media, 17,
>+        CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT},
>      [MEDIA_AND_POISON][GET_SCAN_MEDIA_RESULTS] = {
>         "MEDIA_AND_POISON_GET_SCAN_MEDIA_RESULTS",
>         cmd_media_get_scan_media_results, 0, 0 },
>@@ -2214,6 +2245,8 @@ static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
>      [INFOSTAT][IS_IDENTIFY] = { "IDENTIFY", cmd_infostat_identify, 0, 0 },
>      [INFOSTAT][BACKGROUND_OPERATION_STATUS] = { "BACKGROUND_OPERATION_STATUS",
>         cmd_infostat_bg_op_sts, 0, 0 },
>+    [INFOSTAT][BACKGROUND_OPERATION_ABORT] = { "BACKGROUND_OPERATION_ABORT",
>+        cmd_infostat_bg_op_abort, 0, 0 },
>      [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
>      [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set, 8,
>                          CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
>@@ -2295,6 +2328,7 @@ int cxl_process_cci_message(CXLCCI *cci, uint8_t set, >uint8_t cmd,
>         cci->bg.opcode = (set << 8) | cmd;
>
>         cci->bg.complete_pct = 0;
>+        cci->bg.aborted = false;
>         cci->bg.ret_code = 0;
>
>         now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
>@@ -2386,6 +2420,7 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
>      cxl_rebuild_cel(cci);
>
>      cci->bg.complete_pct = 0;
>+    cci->bg.aborted = false;
>      cci->bg.starttime = 0;
>      cci->bg.runtime = 0;
>      cci->bg.timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
>diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>index d38391b26f0e..d83b359e9994 100644
>--- a/include/hw/cxl/cxl_device.h
>+++ b/include/hw/cxl/cxl_device.h
>@@ -197,6 +197,7 @@ typedef struct CXLCCI {
>      struct {
>         uint16_t opcode;
>         uint16_t complete_pct;
>+        bool aborted;
>         uint16_t ret_code; /* Current value of retcode */
>         uint64_t starttime;
>         /* set by each bg cmd, cleared by the bg_timer when complete */
>diff --git a/include/hw/cxl/cxl_mailbox.h b/include/hw/cxl/cxl_mailbox.h
>index beb048052e1b..9008402d1c46 100644
>--- a/include/hw/cxl/cxl_mailbox.h
>+++ b/include/hw/cxl/cxl_mailbox.h
>@@ -14,5 +14,6 @@
>  #define CXL_MBOX_IMMEDIATE_LOG_CHANGE (1 << 4)
>  #define CXL_MBOX_SECURITY_STATE_CHANGE (1 << 5)
>  #define CXL_MBOX_BACKGROUND_OPERATION (1 << 6)
>+#define CXL_MBOX_BACKGROUND_OPERATION_ABORT (1 << 7)
>
>  #endif
>--
>2.44.0