* [PATCH] hw/cxl: Support aborting background commands
@ 2024-08-13 22:12 Davidlohr Bueso
2024-08-27 15:33 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Davidlohr Bueso @ 2024-08-13 22:12 UTC (permalink / raw)
To: Jonathan.Cameron
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, dave, linux-cxl
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.
Arbitrarily, the on-going background cmd will not be aborted if
already at least 85% done;
A mutex is introduced to stabilize mbox request cancel command vs
the timer callback being fired scenarios (as well as reading the
mbox registers). While some operations under critical regions
may be unnecessary (irq notifying, cmd callbacks), this is not
a path where performance is important, so simplicity is preferred.
Tested-by: Ajay Joshi <ajay.opensrc@micron.com>
Reviewed-by: Ajay Joshi <ajay.opensrc@micron.com>
Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
Changes based on the following thread:
https://lore.kernel.org/linux-cxl/20240729102010.20996-1-ajay.opensrc@micron.com
- Added a mutex (and expanded CCI to have a destroy counterpart).
An 'initialized' flag is added for correctly handling the reset()
case.
- Added the case where cancel is not done.
- Picked up Ajay's tags but it would be good to re-review/test if
possible.
hw/cxl/cxl-device-utils.c | 5 ++-
hw/cxl/cxl-mailbox-utils.c | 63 +++++++++++++++++++++++++++++++++---
hw/mem/cxl_type3.c | 8 ++++-
include/hw/cxl/cxl_device.h | 4 +++
include/hw/cxl/cxl_mailbox.h | 1 +
5 files changed, 74 insertions(+), 7 deletions(-)
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 035d034f6dd8..1a9779ed8201 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -95,11 +95,14 @@ 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) {
+
+ qemu_mutex_lock(&cci->bg.lock);
+ if (cci->bg.complete_pct == 100 || 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;
}
+ qemu_mutex_unlock(&cci->bg.lock);
}
return cxl_dstate->mbox_reg_state64[offset / size];
default:
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index b752920ec88a..ff12dfc3dcc4 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -56,6 +56,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
@@ -624,6 +625,38 @@ 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;
+ }
+
+ qemu_mutex_lock(&cci->bg.lock);
+ if (cci->bg.runtime) {
+ /* operation is near complete, let it finish */
+ if (cci->bg.complete_pct < 85) {
+ timer_del(cci->bg.timer);
+ cci->bg.ret_code = CXL_MBOX_ABORTED;
+ cci->bg.starttime = 0;
+ cci->bg.runtime = 0;
+ cci->bg.aborted = true;
+ }
+ }
+ qemu_mutex_unlock(&cci->bg.lock);
+
+ return CXL_MBOX_SUCCESS;
+}
+
#define CXL_FW_SLOTS 2
#define CXL_FW_SIZE 0x02000000 /* 32 mb */
@@ -2665,6 +2698,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",
@@ -2708,7 +2743,8 @@ 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",
@@ -2721,7 +2757,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 },
@@ -2745,6 +2782,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 },
@@ -2831,6 +2870,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);
@@ -2844,10 +2884,12 @@ int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
static void bg_timercb(void *opaque)
{
CXLCCI *cci = opaque;
- uint64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
- uint64_t total_time = cci->bg.starttime + cci->bg.runtime;
+ uint64_t now, total_time;
+
+ qemu_mutex_lock(&cci->bg.lock);
- assert(cci->bg.runtime > 0);
+ now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
+ total_time = cci->bg.starttime + cci->bg.runtime;
if (now >= total_time) { /* we are done */
uint16_t ret = CXL_MBOX_SUCCESS;
@@ -2899,6 +2941,8 @@ static void bg_timercb(void *opaque)
msi_notify(pdev, cxl_dstate->mbox_msi_n);
}
}
+
+ qemu_mutex_unlock(&cci->bg.lock);
}
static void cxl_rebuild_cel(CXLCCI *cci)
@@ -2927,12 +2971,21 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
cci->bg.complete_pct = 0;
cci->bg.starttime = 0;
cci->bg.runtime = 0;
+ cci->bg.aborted = false;
cci->bg.timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
bg_timercb, cci);
+ qemu_mutex_init(&cci->bg.lock);
memset(&cci->fw, 0, sizeof(cci->fw));
cci->fw.active_slot = 1;
cci->fw.slot[cci->fw.active_slot - 1] = true;
+ cci->initialized = true;
+}
+
+void cxl_destroy_cci(CXLCCI *cci)
+{
+ qemu_mutex_destroy(&cci->bg.lock);
+ cci->initialized = false;
}
static void cxl_copy_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmds)[256])
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 4114163324bd..f04aa58ea85d 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -961,6 +961,7 @@ static void ct3_exit(PCIDevice *pci_dev)
pcie_aer_exit(pci_dev);
cxl_doe_cdat_release(cxl_cstate);
g_free(regs->special_ops);
+ cxl_destroy_cci(&ct3d->cci);
if (ct3d->dc.host_dc) {
cxl_destroy_dc_regions(ct3d);
address_space_destroy(&ct3d->dc.host_dc_as);
@@ -1209,12 +1210,17 @@ static void ct3d_reset(DeviceState *dev)
* Bring up an endpoint to target with MCTP over VDM.
* This device is emulating an MLD with single LD for now.
*/
+ if (ct3d->vdm_fm_owned_ld_mctp_cci.initialized) {
+ cxl_destroy_cci(&ct3d->vdm_fm_owned_ld_mctp_cci);
+ }
cxl_initialize_t3_fm_owned_ld_mctpcci(&ct3d->vdm_fm_owned_ld_mctp_cci,
DEVICE(ct3d), DEVICE(ct3d),
512); /* Max payload made up */
+ if (ct3d->ld0_cci.initialized) {
+ cxl_destroy_cci(&ct3d->ld0_cci);
+ }
cxl_initialize_t3_ld_cci(&ct3d->ld0_cci, DEVICE(ct3d), DEVICE(ct3d),
512); /* Max payload made up */
-
}
static Property ct3_props[] = {
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index e14e56ae4bc2..c0e8d40053db 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -176,10 +176,12 @@ typedef struct CXLCCI {
uint16_t opcode;
uint16_t complete_pct;
uint16_t ret_code; /* Current value of retcode */
+ bool aborted;
uint64_t starttime;
/* set by each bg cmd, cleared by the bg_timer when complete */
uint64_t runtime;
QEMUTimer *timer;
+ QemuMutex lock; /* serializes mbox abort vs timer cb */
} bg;
/* firmware update */
@@ -201,6 +203,7 @@ typedef struct CXLCCI {
DeviceState *d;
/* Pointer to the device hosting the protocol conversion */
DeviceState *intf;
+ bool initialized;
} CXLCCI;
typedef struct cxl_device_state {
@@ -316,6 +319,7 @@ void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max);
void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
DeviceState *d, size_t payload_max);
void cxl_init_cci(CXLCCI *cci, size_t payload_max);
+void cxl_destroy_cci(CXLCCI *cci);
void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
size_t payload_max);
int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
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.45.2
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] hw/cxl: Support aborting background commands
2024-08-13 22:12 [PATCH] hw/cxl: Support aborting background commands Davidlohr Bueso
@ 2024-08-27 15:33 ` Jonathan Cameron
2024-10-22 3:23 ` Davidlohr Bueso
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2024-08-27 15:33 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 13 Aug 2024 15:12:55 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> 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.
> Arbitrarily, the on-going background cmd will not be aborted if
> already at least 85% done;
>
> A mutex is introduced to stabilize mbox request cancel command vs
> the timer callback being fired scenarios (as well as reading the
> mbox registers). While some operations under critical regions
> may be unnecessary (irq notifying, cmd callbacks), this is not
> a path where performance is important, so simplicity is preferred.
>
> Tested-by: Ajay Joshi <ajay.opensrc@micron.com>
> Reviewed-by: Ajay Joshi <ajay.opensrc@micron.com>
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
+CC qemu-devel
No comments inline and LGTM. I'll queue it on my tree and push
that out on gitlab sometime soonish.
J
> ---
>
> Changes based on the following thread:
> https://lore.kernel.org/linux-cxl/20240729102010.20996-1-ajay.opensrc@micron.com
>
> - Added a mutex (and expanded CCI to have a destroy counterpart).
> An 'initialized' flag is added for correctly handling the reset()
> case.
> - Added the case where cancel is not done.
> - Picked up Ajay's tags but it would be good to re-review/test if
> possible.
>
> hw/cxl/cxl-device-utils.c | 5 ++-
> hw/cxl/cxl-mailbox-utils.c | 63 +++++++++++++++++++++++++++++++++---
> hw/mem/cxl_type3.c | 8 ++++-
> include/hw/cxl/cxl_device.h | 4 +++
> include/hw/cxl/cxl_mailbox.h | 1 +
> 5 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 035d034f6dd8..1a9779ed8201 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -95,11 +95,14 @@ 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) {
> +
> + qemu_mutex_lock(&cci->bg.lock);
> + if (cci->bg.complete_pct == 100 || 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;
> }
> + qemu_mutex_unlock(&cci->bg.lock);
> }
> return cxl_dstate->mbox_reg_state64[offset / size];
> default:
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index b752920ec88a..ff12dfc3dcc4 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -56,6 +56,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
> @@ -624,6 +625,38 @@ 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;
> + }
> +
> + qemu_mutex_lock(&cci->bg.lock);
> + if (cci->bg.runtime) {
> + /* operation is near complete, let it finish */
> + if (cci->bg.complete_pct < 85) {
> + timer_del(cci->bg.timer);
> + cci->bg.ret_code = CXL_MBOX_ABORTED;
> + cci->bg.starttime = 0;
> + cci->bg.runtime = 0;
> + cci->bg.aborted = true;
> + }
> + }
> + qemu_mutex_unlock(&cci->bg.lock);
> +
> + return CXL_MBOX_SUCCESS;
> +}
> +
> #define CXL_FW_SLOTS 2
> #define CXL_FW_SIZE 0x02000000 /* 32 mb */
>
> @@ -2665,6 +2698,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",
> @@ -2708,7 +2743,8 @@ 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",
> @@ -2721,7 +2757,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 },
> @@ -2745,6 +2782,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 },
> @@ -2831,6 +2870,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);
> @@ -2844,10 +2884,12 @@ int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
> static void bg_timercb(void *opaque)
> {
> CXLCCI *cci = opaque;
> - uint64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> - uint64_t total_time = cci->bg.starttime + cci->bg.runtime;
> + uint64_t now, total_time;
> +
> + qemu_mutex_lock(&cci->bg.lock);
>
> - assert(cci->bg.runtime > 0);
> + now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL);
> + total_time = cci->bg.starttime + cci->bg.runtime;
>
> if (now >= total_time) { /* we are done */
> uint16_t ret = CXL_MBOX_SUCCESS;
> @@ -2899,6 +2941,8 @@ static void bg_timercb(void *opaque)
> msi_notify(pdev, cxl_dstate->mbox_msi_n);
> }
> }
> +
> + qemu_mutex_unlock(&cci->bg.lock);
> }
>
> static void cxl_rebuild_cel(CXLCCI *cci)
> @@ -2927,12 +2971,21 @@ void cxl_init_cci(CXLCCI *cci, size_t payload_max)
> cci->bg.complete_pct = 0;
> cci->bg.starttime = 0;
> cci->bg.runtime = 0;
> + cci->bg.aborted = false;
> cci->bg.timer = timer_new_ms(QEMU_CLOCK_VIRTUAL,
> bg_timercb, cci);
> + qemu_mutex_init(&cci->bg.lock);
>
> memset(&cci->fw, 0, sizeof(cci->fw));
> cci->fw.active_slot = 1;
> cci->fw.slot[cci->fw.active_slot - 1] = true;
> + cci->initialized = true;
> +}
> +
> +void cxl_destroy_cci(CXLCCI *cci)
> +{
> + qemu_mutex_destroy(&cci->bg.lock);
> + cci->initialized = false;
> }
>
> static void cxl_copy_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmds)[256])
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index 4114163324bd..f04aa58ea85d 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -961,6 +961,7 @@ static void ct3_exit(PCIDevice *pci_dev)
> pcie_aer_exit(pci_dev);
> cxl_doe_cdat_release(cxl_cstate);
> g_free(regs->special_ops);
> + cxl_destroy_cci(&ct3d->cci);
> if (ct3d->dc.host_dc) {
> cxl_destroy_dc_regions(ct3d);
> address_space_destroy(&ct3d->dc.host_dc_as);
> @@ -1209,12 +1210,17 @@ static void ct3d_reset(DeviceState *dev)
> * Bring up an endpoint to target with MCTP over VDM.
> * This device is emulating an MLD with single LD for now.
> */
> + if (ct3d->vdm_fm_owned_ld_mctp_cci.initialized) {
> + cxl_destroy_cci(&ct3d->vdm_fm_owned_ld_mctp_cci);
> + }
> cxl_initialize_t3_fm_owned_ld_mctpcci(&ct3d->vdm_fm_owned_ld_mctp_cci,
> DEVICE(ct3d), DEVICE(ct3d),
> 512); /* Max payload made up */
> + if (ct3d->ld0_cci.initialized) {
> + cxl_destroy_cci(&ct3d->ld0_cci);
> + }
> cxl_initialize_t3_ld_cci(&ct3d->ld0_cci, DEVICE(ct3d), DEVICE(ct3d),
> 512); /* Max payload made up */
> -
> }
>
> static Property ct3_props[] = {
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index e14e56ae4bc2..c0e8d40053db 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -176,10 +176,12 @@ typedef struct CXLCCI {
> uint16_t opcode;
> uint16_t complete_pct;
> uint16_t ret_code; /* Current value of retcode */
> + bool aborted;
> uint64_t starttime;
> /* set by each bg cmd, cleared by the bg_timer when complete */
> uint64_t runtime;
> QEMUTimer *timer;
> + QemuMutex lock; /* serializes mbox abort vs timer cb */
> } bg;
>
> /* firmware update */
> @@ -201,6 +203,7 @@ typedef struct CXLCCI {
> DeviceState *d;
> /* Pointer to the device hosting the protocol conversion */
> DeviceState *intf;
> + bool initialized;
> } CXLCCI;
>
> typedef struct cxl_device_state {
> @@ -316,6 +319,7 @@ void cxl_initialize_mailbox_t3(CXLCCI *cci, DeviceState *d, size_t payload_max);
> void cxl_initialize_mailbox_swcci(CXLCCI *cci, DeviceState *intf,
> DeviceState *d, size_t payload_max);
> void cxl_init_cci(CXLCCI *cci, size_t payload_max);
> +void cxl_destroy_cci(CXLCCI *cci);
> void cxl_add_cci_commands(CXLCCI *cci, const struct cxl_cmd (*cxl_cmd_set)[256],
> size_t payload_max);
> int cxl_process_cci_message(CXLCCI *cci, uint8_t set, uint8_t cmd,
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] hw/cxl: Support aborting background commands
2024-08-27 15:33 ` Jonathan Cameron
@ 2024-10-22 3:23 ` Davidlohr Bueso
2024-10-22 17:00 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Davidlohr Bueso @ 2024-10-22 3:23 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
>No comments inline and LGTM. I'll queue it on my tree and push
>that out on gitlab sometime soonish.
I don't see this picked up, which is a good thing atm. While testing
the kernel side, I noticed the following is needed, will send a v2
with it folded in.
diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
index 1a9779ed8201..0d429b59aafc 100644
--- a/hw/cxl/cxl-device-utils.c
+++ b/hw/cxl/cxl-device-utils.c
@@ -94,14 +94,15 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
cxl_dstate->mbox_reg_state64[offset / size] = bg_status_reg;
}
if (offset == A_CXL_DEV_MAILBOX_STS) {
+ int bgop;
uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
qemu_mutex_lock(&cci->bg.lock);
- if (cci->bg.complete_pct == 100 || 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;
- }
+ bgop = !(cci->bg.complete_pct == 100 || cci->bg.aborted);
+
+ status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
+ bgop);
+ cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
qemu_mutex_unlock(&cci->bg.lock);
}
return cxl_dstate->mbox_reg_state64[offset / size];
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index d5b084388288..760a8571fda6 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -2731,9 +2731,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
[FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
cmd_firmware_update_get_info, 0, 0 },
[FIRMWARE_UPDATE][TRANSFER] = { "FIRMWARE_UPDATE_TRANSFER",
- cmd_firmware_update_transfer, ~0, CXL_MBOX_BACKGROUND_OPERATION },
+ cmd_firmware_update_transfer, ~0,
+ CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
[FIRMWARE_UPDATE][ACTIVATE] = { "FIRMWARE_UPDATE_ACTIVATE",
- cmd_firmware_update_activate, 2, CXL_MBOX_BACKGROUND_OPERATION },
+ cmd_firmware_update_activate, 2,
+ CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
[TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
[TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set,
8, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
Thanks,
Davidlohr
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] hw/cxl: Support aborting background commands
2024-10-22 3:23 ` Davidlohr Bueso
@ 2024-10-22 17:00 ` Jonathan Cameron
2025-02-03 15:07 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2024-10-22 17:00 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Mon, 21 Oct 2024 20:23:46 -0700
Davidlohr Bueso <dave@stgolabs.net> wrote:
> On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
> >No comments inline and LGTM. I'll queue it on my tree and push
> >that out on gitlab sometime soonish.
>
> I don't see this picked up, which is a good thing atm. While testing
> the kernel side, I noticed the following is needed, will send a v2
> with it folded in.
Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
I'll pick up your v2 and replace that.
>
> diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> index 1a9779ed8201..0d429b59aafc 100644
> --- a/hw/cxl/cxl-device-utils.c
> +++ b/hw/cxl/cxl-device-utils.c
> @@ -94,14 +94,15 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
> cxl_dstate->mbox_reg_state64[offset / size] = bg_status_reg;
> }
> if (offset == A_CXL_DEV_MAILBOX_STS) {
> + int bgop;
> uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
>
> qemu_mutex_lock(&cci->bg.lock);
> - if (cci->bg.complete_pct == 100 || 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;
> - }
> + bgop = !(cci->bg.complete_pct == 100 || cci->bg.aborted);
> +
> + status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> + bgop);
> + cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> qemu_mutex_unlock(&cci->bg.lock);
> }
> return cxl_dstate->mbox_reg_state64[offset / size];
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index d5b084388288..760a8571fda6 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -2731,9 +2731,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> cmd_firmware_update_get_info, 0, 0 },
> [FIRMWARE_UPDATE][TRANSFER] = { "FIRMWARE_UPDATE_TRANSFER",
> - cmd_firmware_update_transfer, ~0, CXL_MBOX_BACKGROUND_OPERATION },
> + cmd_firmware_update_transfer, ~0,
> + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> [FIRMWARE_UPDATE][ACTIVATE] = { "FIRMWARE_UPDATE_ACTIVATE",
> - cmd_firmware_update_activate, 2, CXL_MBOX_BACKGROUND_OPERATION },
> + cmd_firmware_update_activate, 2,
> + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set,
> 8, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
>
> Thanks,
> Davidlohr
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] hw/cxl: Support aborting background commands
2024-10-22 17:00 ` Jonathan Cameron
@ 2025-02-03 15:07 ` Jonathan Cameron
2025-02-05 4:47 ` Davidlohr Bueso
0 siblings, 1 reply; 7+ messages in thread
From: Jonathan Cameron @ 2025-02-03 15:07 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 22 Oct 2024 18:00:30 +0100
Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> On Mon, 21 Oct 2024 20:23:46 -0700
> Davidlohr Bueso <dave@stgolabs.net> wrote:
>
> > On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
> > >No comments inline and LGTM. I'll queue it on my tree and push
> > >that out on gitlab sometime soonish.
> >
> > I don't see this picked up, which is a good thing atm. While testing
> > the kernel side, I noticed the following is needed, will send a v2
> > with it folded in.
>
> Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
> I'll pick up your v2 and replace that.
Hi Davidlohr,
Did I miss v2, or still to send?
Thanks,
Jonathan
> >
> > diff --git a/hw/cxl/cxl-device-utils.c b/hw/cxl/cxl-device-utils.c
> > index 1a9779ed8201..0d429b59aafc 100644
> > --- a/hw/cxl/cxl-device-utils.c
> > +++ b/hw/cxl/cxl-device-utils.c
> > @@ -94,14 +94,15 @@ static uint64_t mailbox_reg_read(void *opaque, hwaddr offset, unsigned size)
> > cxl_dstate->mbox_reg_state64[offset / size] = bg_status_reg;
> > }
> > if (offset == A_CXL_DEV_MAILBOX_STS) {
> > + int bgop;
> > uint64_t status_reg = cxl_dstate->mbox_reg_state64[offset / size];
> >
> > qemu_mutex_lock(&cci->bg.lock);
> > - if (cci->bg.complete_pct == 100 || 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;
> > - }
> > + bgop = !(cci->bg.complete_pct == 100 || cci->bg.aborted);
> > +
> > + status_reg = FIELD_DP64(status_reg, CXL_DEV_MAILBOX_STS, BG_OP,
> > + bgop);
> > + cxl_dstate->mbox_reg_state64[offset / size] = status_reg;
> > qemu_mutex_unlock(&cci->bg.lock);
> > }
> > return cxl_dstate->mbox_reg_state64[offset / size];
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index d5b084388288..760a8571fda6 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -2731,9 +2731,11 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> > [FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> > cmd_firmware_update_get_info, 0, 0 },
> > [FIRMWARE_UPDATE][TRANSFER] = { "FIRMWARE_UPDATE_TRANSFER",
> > - cmd_firmware_update_transfer, ~0, CXL_MBOX_BACKGROUND_OPERATION },
> > + cmd_firmware_update_transfer, ~0,
> > + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> > [FIRMWARE_UPDATE][ACTIVATE] = { "FIRMWARE_UPDATE_ACTIVATE",
> > - cmd_firmware_update_activate, 2, CXL_MBOX_BACKGROUND_OPERATION },
> > + cmd_firmware_update_activate, 2,
> > + CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT },
> > [TIMESTAMP][GET] = { "TIMESTAMP_GET", cmd_timestamp_get, 0, 0 },
> > [TIMESTAMP][SET] = { "TIMESTAMP_SET", cmd_timestamp_set,
> > 8, CXL_MBOX_IMMEDIATE_POLICY_CHANGE },
> >
> > Thanks,
> > Davidlohr
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] hw/cxl: Support aborting background commands
2025-02-03 15:07 ` Jonathan Cameron
@ 2025-02-05 4:47 ` Davidlohr Bueso
2025-02-05 17:24 ` Jonathan Cameron
0 siblings, 1 reply; 7+ messages in thread
From: Davidlohr Bueso @ 2025-02-05 4:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Mon, 03 Feb 2025, Jonathan Cameron wrote:
>On Tue, 22 Oct 2024 18:00:30 +0100
>Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
>
>> On Mon, 21 Oct 2024 20:23:46 -0700
>> Davidlohr Bueso <dave@stgolabs.net> wrote:
>>
>> > On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
>> > >No comments inline and LGTM. I'll queue it on my tree and push
>> > >that out on gitlab sometime soonish.
>> >
>> > I don't see this picked up, which is a good thing atm. While testing
>> > the kernel side, I noticed the following is needed, will send a v2
>> > with it folded in.
>>
>> Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
>> I'll pick up your v2 and replace that.
>
>Hi Davidlohr,
>
>Did I miss v2, or still to send?
Ah, thanks for the reminder, I had forgotten to send it out.
Actually, considering you have been carrying v1 for a while now,
would it not be cleaner/better just rebasing the diff and sending you
that instead of a whole new v2? Otherwise, I'm not sure what to do
when working on your latest 2025-01-24 branch.
Thanks,
Davidlohr
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] hw/cxl: Support aborting background commands
2025-02-05 4:47 ` Davidlohr Bueso
@ 2025-02-05 17:24 ` Jonathan Cameron
0 siblings, 0 replies; 7+ messages in thread
From: Jonathan Cameron @ 2025-02-05 17:24 UTC (permalink / raw)
To: Davidlohr Bueso
Cc: ajay.opensrc, fan.ni, john, emirakhur, ajayjoshi, sthanneeru,
ravis.opensrc, arramesh, tmmulgund, linux-cxl, qemu-devel
On Tue, 4 Feb 2025 20:47:49 -0800
Davidlohr Bueso <dave@stgolabs.net> wrote:
> On Mon, 03 Feb 2025, Jonathan Cameron wrote:
>
> >On Tue, 22 Oct 2024 18:00:30 +0100
> >Jonathan Cameron <Jonathan.Cameron@Huawei.com> wrote:
> >
> >> On Mon, 21 Oct 2024 20:23:46 -0700
> >> Davidlohr Bueso <dave@stgolabs.net> wrote:
> >>
> >> > On Tue, 27 Aug 2024, Jonathan Cameron wrote:\n
> >> > >No comments inline and LGTM. I'll queue it on my tree and push
> >> > >that out on gitlab sometime soonish.
> >> >
> >> > I don't see this picked up, which is a good thing atm. While testing
> >> > the kernel side, I noticed the following is needed, will send a v2
> >> > with it folded in.
> >>
> >> Currently just on my cxl-2024-10-15 branch of gitlab.com/jic23/qemu.
> >> I'll pick up your v2 and replace that.
> >
> >Hi Davidlohr,
> >
> >Did I miss v2, or still to send?
>
> Ah, thanks for the reminder, I had forgotten to send it out.
>
> Actually, considering you have been carrying v1 for a while now,
> would it not be cleaner/better just rebasing the diff and sending you
> that instead of a whole new v2? Otherwise, I'm not sure what to do
> when working on your latest 2025-01-24 branch.
Pick point where I'm carrying it and post a patch based on the
next commit up. Just let me know you've done that and all
should be good.
Jonathan
>
> Thanks,
> Davidlohr
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-02-05 17:24 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-13 22:12 [PATCH] hw/cxl: Support aborting background commands Davidlohr Bueso
2024-08-27 15:33 ` Jonathan Cameron
2024-10-22 3:23 ` Davidlohr Bueso
2024-10-22 17:00 ` Jonathan Cameron
2025-02-03 15:07 ` Jonathan Cameron
2025-02-05 4:47 ` Davidlohr Bueso
2025-02-05 17:24 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox