[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command
From: |
Klaus Jensen |
Subject: |
Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command |
Date: |
Wed, 10 Feb 2021 21:33:50 +0100 |
On Feb 11 04:52, Minwoo Im wrote:
> nvme_inject_state command is to give a controller state to be.
> Human Monitor Interface(HMP) supports users to make controller to a
> specified state of:
>
> normal: Normal state (no injection)
> cmd-interrupted: Commands will be interrupted internally
>
> This patch is just a start to give dynamic command from the HMP to the
> QEMU NVMe device model. If "cmd-interrupted" state is given, then the
> controller will return all the CQ entries with Command Interrupts status
> code.
>
> Usage:
> -device nvme,id=nvme0,....
>
> (qemu) nvme_inject_state nvme0 cmd-interrupted
>
> <All the commands will be interrupted internally>
>
> (qemu) nvme_inject_state nvme0 normal
>
> This feature is required to test Linux kernel NVMe driver for the
> command retry feature.
>
This is super cool and commands like this feel much nicer than the
qom-set approach that the SMART critical warning feature took.
But... looking at the existing commands I don't think we can "bloat" it
up with a device specific command like this, but I don't know the policy
around this.
If an HMP command is out, then we should be able to make do with the
qom-set approach just fine though.
> Signed-off-by: Minwoo Im <minwoo.im.dev@gmail.com>
> ---
> hmp-commands.hx | 13 ++++++++++++
> hw/block/nvme.c | 49 +++++++++++++++++++++++++++++++++++++++++++
> hw/block/nvme.h | 8 +++++++
> include/monitor/hmp.h | 1 +
> 4 files changed, 71 insertions(+)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index d4001f9c5dc6..ef288c567b46 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1307,6 +1307,19 @@ SRST
> Inject PCIe AER error
> ERST
>
> + {
> + .name = "nvme_inject_state",
> + .args_type = "id:s,state:s",
> + .params = "id [normal|cmd-interrupted]",
> + .help = "inject controller/namespace state",
> + .cmd = hmp_nvme_inject_state,
> + },
> +
> +SRST
> +``nvme_inject_state``
> + Inject NVMe controller/namespace state
> +ERST
> +
> {
> .name = "netdev_add",
> .args_type = "netdev:O",
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index 6d3c554a0e99..42cf5bd113e6 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -123,6 +123,7 @@
> #include "sysemu/sysemu.h"
> #include "qapi/error.h"
> #include "qapi/visitor.h"
> +#include "qapi/qmp/qdict.h"
> #include "sysemu/hostmem.h"
> #include "sysemu/block-backend.h"
> #include "exec/memory.h"
> @@ -132,6 +133,7 @@
> #include "trace.h"
> #include "nvme.h"
> #include "nvme-ns.h"
> +#include "monitor/monitor.h"
>
> #define NVME_MAX_IOQPAIRS 0xffff
> #define NVME_DB_SIZE 4
> @@ -966,6 +968,14 @@ static void nvme_enqueue_req_completion(NvmeCQueue *cq,
> NvmeRequest *req)
> {
> assert(cq->cqid == req->sq->cqid);
>
> + /*
> + * Override request status field if controller state has been injected by
> + * the QMP.
> + */
> + if (cq->ctrl->state == NVME_STATE_CMD_INTERRUPTED) {
> + req->status = NVME_COMMAND_INTERRUPTED;
> + }
> +
> if (req->status != NVME_SUCCESS) {
> if (cq->ctrl->features.acre && nvme_should_retry(req)) {
> if (cq->ctrl->params.cmd_retry_delay > 0) {
> @@ -5025,4 +5035,43 @@ static void nvme_register_types(void)
> type_register_static(&nvme_bus_info);
> }
>
> +static void nvme_inject_state(NvmeCtrl *n, NvmeState state)
> +{
> + n->state = state;
> +}
> +
> +static const char *nvme_states[] = {
> + [NVME_STATE_NORMAL] = "normal",
> + [NVME_STATE_CMD_INTERRUPTED] = "cmd-interrupted",
> +};
> +
> +void hmp_nvme_inject_state(Monitor *mon, const QDict *qdict)
> +{
> + const char *id = qdict_get_str(qdict, "id");
> + const char *state = qdict_get_str(qdict, "state");
> + PCIDevice *dev;
> + NvmeCtrl *n;
> + int ret, i;
> +
> + ret = pci_qdev_find_device(id, &dev);
> + if (ret < 0) {
> + monitor_printf(mon, "invalid device id %s\n", id);
> + return;
> + }
> +
> + n = NVME(dev);
> +
> + for (i = 0; i < ARRAY_SIZE(nvme_states); i++) {
> + if (!strcmp(nvme_states[i], state)) {
> + nvme_inject_state(n, i);
> + monitor_printf(mon,
> + "-device nvme,id=%s: state %s injected\n",
> + id, state);
> + return;
> + }
> + }
> +
> + monitor_printf(mon, "invalid state %s\n", state);
> +}
> +
> type_init(nvme_register_types)
> diff --git a/hw/block/nvme.h b/hw/block/nvme.h
> index 37940b3ac2d2..1af1e0380d9b 100644
> --- a/hw/block/nvme.h
> +++ b/hw/block/nvme.h
> @@ -128,6 +128,11 @@ typedef struct NvmeFeatureVal {
> uint8_t acre;
> } NvmeFeatureVal;
>
> +typedef enum NvmeState {
> + NVME_STATE_NORMAL,
> + NVME_STATE_CMD_INTERRUPTED,
> +} NvmeState;
> +
> typedef struct NvmeCtrl {
> PCIDevice parent_obj;
> MemoryRegion bar0;
> @@ -185,6 +190,8 @@ typedef struct NvmeCtrl {
> NvmeCQueue admin_cq;
> NvmeIdCtrl id_ctrl;
> NvmeFeatureVal features;
> +
> + NvmeState state;
> } NvmeCtrl;
>
> static inline NvmeNamespace *nvme_ns(NvmeCtrl *n, uint32_t nsid)
> @@ -212,4 +219,5 @@ static inline NvmeCtrl *nvme_ctrl(NvmeRequest *req)
>
> int nvme_register_namespace(NvmeCtrl *n, NvmeNamespace *ns, Error **errp);
>
> +void hmp_nvme_inject_state(Monitor *mon, const QDict *qdict);
> #endif /* HW_NVME_H */
> diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
> index ed2913fd18e8..668384ea2e34 100644
> --- a/include/monitor/hmp.h
> +++ b/include/monitor/hmp.h
> @@ -79,6 +79,7 @@ void hmp_migrate(Monitor *mon, const QDict *qdict);
> void hmp_device_add(Monitor *mon, const QDict *qdict);
> void hmp_device_del(Monitor *mon, const QDict *qdict);
> void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict);
> +void hmp_nvme_inject_state(Monitor *mon, const QDict *qdict);
> void hmp_netdev_add(Monitor *mon, const QDict *qdict);
> void hmp_netdev_del(Monitor *mon, const QDict *qdict);
> void hmp_getfd(Monitor *mon, const QDict *qdict);
> --
> 2.17.1
>
--
One of us - No more doubt, silence or taboo about mental illness.
signature.asc
Description: PGP signature
- [RFC PATCH 0/3] support command retry, Minwoo Im, 2021/02/10
- [RFC PATCH 1/3] hw/block/nvme: set NVME_DNR in a single place, Minwoo Im, 2021/02/10
- [RFC PATCH 2/3] hw/block/nvme: support command retry delay, Minwoo Im, 2021/02/10
- [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Minwoo Im, 2021/02/10
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command,
Klaus Jensen <=
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Keith Busch, 2021/02/10
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Minwoo Im, 2021/02/10
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Keith Busch, 2021/02/10
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Warner Losh, 2021/02/10
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Minwoo Im, 2021/02/11
- Re: [RFC PATCH 3/3] hw/block/nvme: add nvme_inject_state HMP command, Klaus Jensen, 2021/02/11