* Klaus Jensen (i...@irrelevant.dk) wrote:
> 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.
HMP is there to make life easy for Humans debugging; if it makes sense from an
NVMe perspective for test/debug then I'm OK with it from an HMP side.
Note that if it was for more common use than debug/test then you'd want
to make it go via QMP and make sure it was a stable interface that was
going to live for a longtime; but for test uses HMP is fine.
Dave
> > Signed-off-by: Minwoo Im <minwoo.im....@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.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
