diff mbox series

[v5,3/3] hw/nvme: add nvme management interface model

Message ID 20230905-nmi-i2c-v5-3-0001d372a728@samsung.com
State New
Headers show
Series hw/{i2c,nvme}: mctp endpoint, nvme management interface model | expand

Commit Message

Klaus Jensen Sept. 5, 2023, 8:38 a.m. UTC
From: Klaus Jensen <k.jensen@samsung.com>

Add the 'nmi-i2c' device that emulates an NVMe Management Interface
controller.

Initial support is very basic (Read NMI DS, Configuration Get).

This is based on previously posted code by Padmakar Kalghatgi, Arun
Kumar Agasar and Saurav Kumar.

Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
---
 hw/nvme/Kconfig      |   4 +
 hw/nvme/meson.build  |   1 +
 hw/nvme/nmi-i2c.c    | 424 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/nvme/trace-events |   6 +
 4 files changed, 435 insertions(+)

Comments

Andrew Jeffery Sept. 12, 2023, 5:50 a.m. UTC | #1
Hi Klaus,

On Tue, 2023-09-05 at 10:38 +0200, Klaus Jensen wrote:
> > 
> > +static void nmi_handle_mi_config_get(NMIDevice *nmi, NMIRequest
> > *request)
> > +{
> > +    uint32_t dw0 = le32_to_cpu(request->dw0);
> > +    uint8_t identifier = FIELD_EX32(dw0,
> > NMI_CMD_CONFIGURATION_GET_DW0,
> > +                                    IDENTIFIER);
> > +    const uint8_t *buf;
> > +
> > +    static const uint8_t smbus_freq[4] = {
> > +        0x00,               /* success */
> > +        0x01, 0x00, 0x00,   /* 100 kHz */
> > +    };
> > +
> > +    static const uint8_t mtu[4] = {
> > +        0x00,       /* success */
> > +        0x40, 0x00, /* 64 */
> > +        0x00,       /* reserved */
> > +    };
> > +
> > +    trace_nmi_handle_mi_config_get(identifier);
> > +
> > +    switch (identifier) {
> > +    case NMI_CMD_CONFIGURATION_GET_SMBUS_FREQ:
> > +        buf = smbus_freq;
> > +        break;
> > +
> > +    case NMI_CMD_CONFIGURATION_GET_MCTP_TRANSMISSION_UNIT:
> > +        buf = mtu;
> > +        break;
> > +
> > +    default:
> > +        nmi_set_parameter_error(nmi, 0x0, offsetof(NMIRequest,
> > dw0));
> > +        return;
> > +    }
> > +
> > +    nmi_scratch_append(nmi, buf, sizeof(buf));
> > +}

When I tried to build this patch I got:

```
In file included from /usr/include/string.h:535,
                 from /home/andrew/src/qemu.org/qemu/include/qemu/osdep.h:112,
                 from ../hw/nvme/nmi-i2c.c:12:
In function ‘memcpy’,
    inlined from ‘nmi_scratch_append’ at ../hw/nvme/nmi-i2c.c:80:5,
    inlined from ‘nmi_handle_mi_config_get’ at ../hw/nvme/nmi-i2c.c:246:5,
    inlined from ‘nmi_handle_mi’ at ../hw/nvme/nmi-i2c.c:266:9,
    inlined from ‘nmi_handle’ at ../hw/nvme/nmi-i2c.c:313:9:
/usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: error: ‘__builtin_memcpy’ forming offset [4, 7] is out of the bounds [0, 4] [-Werror=array-bounds=]
   29 |   return __builtin___memcpy_chk (__dest, __src, __len,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   30 |                                  __glibc_objsize0 (__dest));
      |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~
```

It wasn't clear initially from the error that the source of the problem
was the size associated with the source buffer, especially as there is
some pointer arithmetic being done to derive `__dest`.

Anyway, what we're trying to express is that the size to copy from buf
is the size of the array pointed to by buf. However, buf is declared as
a pointer to uint8_t, which loses the length information. To fix that I
think we need:

- const uint8_t *buf;
+ const uint8_t (*buf)[4];

and then:

- nmi_scratch_append(nmi, buf, sizeof(buf));
+ nmi_scratch_append(nmi, buf, sizeof(*buf));

Andrew
Klaus Jensen Sept. 14, 2023, 6:51 a.m. UTC | #2
On Sep 12 13:50, Andrew Jeffery wrote:
> Hi Klaus,
> 
> On Tue, 2023-09-05 at 10:38 +0200, Klaus Jensen wrote:
> > > 
> > > +static void nmi_handle_mi_config_get(NMIDevice *nmi, NMIRequest
> > > *request)
> > > +{
> > > +    uint32_t dw0 = le32_to_cpu(request->dw0);
> > > +    uint8_t identifier = FIELD_EX32(dw0,
> > > NMI_CMD_CONFIGURATION_GET_DW0,
> > > +                                    IDENTIFIER);
> > > +    const uint8_t *buf;
> > > +
> > > +    static const uint8_t smbus_freq[4] = {
> > > +        0x00,               /* success */
> > > +        0x01, 0x00, 0x00,   /* 100 kHz */
> > > +    };
> > > +
> > > +    static const uint8_t mtu[4] = {
> > > +        0x00,       /* success */
> > > +        0x40, 0x00, /* 64 */
> > > +        0x00,       /* reserved */
> > > +    };
> > > +
> > > +    trace_nmi_handle_mi_config_get(identifier);
> > > +
> > > +    switch (identifier) {
> > > +    case NMI_CMD_CONFIGURATION_GET_SMBUS_FREQ:
> > > +        buf = smbus_freq;
> > > +        break;
> > > +
> > > +    case NMI_CMD_CONFIGURATION_GET_MCTP_TRANSMISSION_UNIT:
> > > +        buf = mtu;
> > > +        break;
> > > +
> > > +    default:
> > > +        nmi_set_parameter_error(nmi, 0x0, offsetof(NMIRequest,
> > > dw0));
> > > +        return;
> > > +    }
> > > +
> > > +    nmi_scratch_append(nmi, buf, sizeof(buf));
> > > +}
> 
> When I tried to build this patch I got:
> 
> ```
> In file included from /usr/include/string.h:535,
>                  from /home/andrew/src/qemu.org/qemu/include/qemu/osdep.h:112,
>                  from ../hw/nvme/nmi-i2c.c:12:
> In function ‘memcpy’,
>     inlined from ‘nmi_scratch_append’ at ../hw/nvme/nmi-i2c.c:80:5,
>     inlined from ‘nmi_handle_mi_config_get’ at ../hw/nvme/nmi-i2c.c:246:5,
>     inlined from ‘nmi_handle_mi’ at ../hw/nvme/nmi-i2c.c:266:9,
>     inlined from ‘nmi_handle’ at ../hw/nvme/nmi-i2c.c:313:9:
> /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: error: ‘__builtin_memcpy’ forming offset [4, 7] is out of the bounds [0, 4] [-Werror=array-bounds=]
>    29 |   return __builtin___memcpy_chk (__dest, __src, __len,
>       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    30 |                                  __glibc_objsize0 (__dest));
>       |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~
> ```
> 
> It wasn't clear initially from the error that the source of the problem
> was the size associated with the source buffer, especially as there is
> some pointer arithmetic being done to derive `__dest`.
> 
> Anyway, what we're trying to express is that the size to copy from buf
> is the size of the array pointed to by buf. However, buf is declared as
> a pointer to uint8_t, which loses the length information. To fix that I
> think we need:
> 
> - const uint8_t *buf;
> + const uint8_t (*buf)[4];
> 
> and then:
> 
> - nmi_scratch_append(nmi, buf, sizeof(buf));
> + nmi_scratch_append(nmi, buf, sizeof(*buf));
> 
> Andrew
> 

Hi Andrew,

Nice (and important) catch! Just curious, are you massaging QEMU's build
system into adding additional checks or how did your compiler catch
this?

Thanks,
Klaus
Andrew Jeffery Sept. 14, 2023, 7:42 a.m. UTC | #3
Hi Klaus,

On Thu, 2023-09-14 at 08:51 +0200, Klaus Jensen wrote:
> On Sep 12 13:50, Andrew Jeffery wrote:
> > Hi Klaus,
> > 
> > On Tue, 2023-09-05 at 10:38 +0200, Klaus Jensen wrote:
> > > > 
> > > > +static void nmi_handle_mi_config_get(NMIDevice *nmi, NMIRequest
> > > > *request)
> > > > +{
> > > > +    uint32_t dw0 = le32_to_cpu(request->dw0);
> > > > +    uint8_t identifier = FIELD_EX32(dw0,
> > > > NMI_CMD_CONFIGURATION_GET_DW0,
> > > > +                                    IDENTIFIER);
> > > > +    const uint8_t *buf;
> > > > +
> > > > +    static const uint8_t smbus_freq[4] = {
> > > > +        0x00,               /* success */
> > > > +        0x01, 0x00, 0x00,   /* 100 kHz */
> > > > +    };
> > > > +
> > > > +    static const uint8_t mtu[4] = {
> > > > +        0x00,       /* success */
> > > > +        0x40, 0x00, /* 64 */
> > > > +        0x00,       /* reserved */
> > > > +    };
> > > > +
> > > > +    trace_nmi_handle_mi_config_get(identifier);
> > > > +
> > > > +    switch (identifier) {
> > > > +    case NMI_CMD_CONFIGURATION_GET_SMBUS_FREQ:
> > > > +        buf = smbus_freq;
> > > > +        break;
> > > > +
> > > > +    case NMI_CMD_CONFIGURATION_GET_MCTP_TRANSMISSION_UNIT:
> > > > +        buf = mtu;
> > > > +        break;
> > > > +
> > > > +    default:
> > > > +        nmi_set_parameter_error(nmi, 0x0, offsetof(NMIRequest,
> > > > dw0));
> > > > +        return;
> > > > +    }
> > > > +
> > > > +    nmi_scratch_append(nmi, buf, sizeof(buf));
> > > > +}
> > 
> > When I tried to build this patch I got:
> > 
> > ```
> > In file included from /usr/include/string.h:535,
> >                  from /home/andrew/src/qemu.org/qemu/include/qemu/osdep.h:112,
> >                  from ../hw/nvme/nmi-i2c.c:12:
> > In function ‘memcpy’,
> >     inlined from ‘nmi_scratch_append’ at ../hw/nvme/nmi-i2c.c:80:5,
> >     inlined from ‘nmi_handle_mi_config_get’ at ../hw/nvme/nmi-i2c.c:246:5,
> >     inlined from ‘nmi_handle_mi’ at ../hw/nvme/nmi-i2c.c:266:9,
> >     inlined from ‘nmi_handle’ at ../hw/nvme/nmi-i2c.c:313:9:
> > /usr/include/x86_64-linux-gnu/bits/string_fortified.h:29:10: error: ‘__builtin_memcpy’ forming offset [4, 7] is out of the bounds [0, 4] [-Werror=array-bounds=]
> >    29 |   return __builtin___memcpy_chk (__dest, __src, __len,
> >       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    30 |                                  __glibc_objsize0 (__dest));
> >       |                                  ~~~~~~~~~~~~~~~~~~~~~~~~~~
> > ```
> > 
> > It wasn't clear initially from the error that the source of the problem
> > was the size associated with the source buffer, especially as there is
> > some pointer arithmetic being done to derive `__dest`.
> > 
> > Anyway, what we're trying to express is that the size to copy from buf
> > is the size of the array pointed to by buf. However, buf is declared as
> > a pointer to uint8_t, which loses the length information. To fix that I
> > think we need:
> > 
> > - const uint8_t *buf;
> > + const uint8_t (*buf)[4];
> > 
> > and then:
> > 
> > - nmi_scratch_append(nmi, buf, sizeof(buf));
> > + nmi_scratch_append(nmi, buf, sizeof(*buf));
> > 
> > Andrew
> > 
> 
> Hi Andrew,
> 
> Nice (and important) catch! Just curious, are you massaging QEMU's build
> system into adding additional checks or how did your compiler catch
> this?

No tricks to be honest, I just applied your patches on top of
9ef497755afc ("Merge tag 'pull-vfio-20230911' of
https://github.com/legoater/qemu into staging") using `b4 shazam ...`.

I'm building on Debian Bookworm:

$ gcc --version | head -n1
gcc (Debian 12.2.0-14) 12.2.0

Andrew
diff mbox series

Patch

diff --git a/hw/nvme/Kconfig b/hw/nvme/Kconfig
index 8ac90942e55e..1d89a4f4ecea 100644
--- a/hw/nvme/Kconfig
+++ b/hw/nvme/Kconfig
@@ -2,3 +2,7 @@  config NVME_PCI
     bool
     default y if PCI_DEVICES
     depends on PCI
+
+config NVME_NMI_I2C
+    bool
+    default y if I2C_MCTP
diff --git a/hw/nvme/meson.build b/hw/nvme/meson.build
index 1a6a2ca2f307..7bc85f31c149 100644
--- a/hw/nvme/meson.build
+++ b/hw/nvme/meson.build
@@ -1 +1,2 @@ 
 system_ss.add(when: 'CONFIG_NVME_PCI', if_true: files('ctrl.c', 'dif.c', 'ns.c', 'subsys.c'))
+system_ss.add(when: 'CONFIG_NVME_NMI_I2C', if_true: files('nmi-i2c.c'))
diff --git a/hw/nvme/nmi-i2c.c b/hw/nvme/nmi-i2c.c
new file mode 100644
index 000000000000..6e0ba7bd2a37
--- /dev/null
+++ b/hw/nvme/nmi-i2c.c
@@ -0,0 +1,424 @@ 
+/*
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * SPDX-FileCopyrightText: Copyright (c) 2023 Samsung Electronics Co., Ltd.
+ *
+ * SPDX-FileContributor: Padmakar Kalghatgi <p.kalghatgi@samsung.com>
+ * SPDX-FileContributor: Arun Kumar Agasar <arun.kka@samsung.com>
+ * SPDX-FileContributor: Saurav Kumar <saurav.29@partner.samsung.com>
+ * SPDX-FileContributor: Klaus Jensen <k.jensen@samsung.com>
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/crc32c.h"
+#include "hw/registerfields.h"
+#include "hw/i2c/i2c.h"
+#include "hw/i2c/mctp.h"
+#include "net/mctp.h"
+#include "trace.h"
+
+/* NVM Express Management Interface 1.2c, Section 3.1 */
+#define NMI_MAX_MESSAGE_LENGTH 4224
+
+#define TYPE_NMI_I2C_DEVICE "nmi-i2c"
+OBJECT_DECLARE_SIMPLE_TYPE(NMIDevice, NMI_I2C_DEVICE)
+
+typedef struct NMIDevice {
+    MCTPI2CEndpoint mctp;
+
+    uint8_t buffer[NMI_MAX_MESSAGE_LENGTH];
+    uint8_t scratch[NMI_MAX_MESSAGE_LENGTH];
+
+    size_t  len;
+    int64_t pos;
+} NMIDevice;
+
+FIELD(NMI_MCTPD, MT, 0, 7)
+FIELD(NMI_MCTPD, IC, 7, 1)
+
+#define NMI_MCTPD_MT_NMI 0x4
+#define NMI_MCTPD_IC_ENABLED 0x1
+
+FIELD(NMI_NMP, ROR, 7, 1)
+FIELD(NMI_NMP, NMIMT, 3, 4)
+
+#define NMI_NMP_NMIMT_NVME_MI 0x1
+#define NMI_NMP_NMIMT_NVME_ADMIN 0x2
+
+typedef struct NMIMessage {
+    uint8_t mctpd;
+    uint8_t nmp;
+    uint8_t rsvd2[2];
+    uint8_t payload[]; /* includes the Message Integrity Check */
+} NMIMessage;
+
+typedef struct NMIRequest {
+   uint8_t opc;
+   uint8_t rsvd1[3];
+   uint32_t dw0;
+   uint32_t dw1;
+   uint32_t mic;
+} NMIRequest;
+
+FIELD(NMI_CMD_READ_NMI_DS_DW0, DTYP, 24, 8)
+
+typedef enum NMIReadDSType {
+    NMI_CMD_READ_NMI_DS_SUBSYSTEM       = 0x0,
+    NMI_CMD_READ_NMI_DS_PORTS           = 0x1,
+    NMI_CMD_READ_NMI_DS_CTRL_LIST       = 0x2,
+    NMI_CMD_READ_NMI_DS_CTRL_INFO       = 0x3,
+    NMI_CMD_READ_NMI_DS_OPT_CMD_SUPPORT = 0x4,
+    NMI_CMD_READ_NMI_DS_MEB_CMD_SUPPORT = 0x5,
+} NMIReadDSType;
+
+#define NMI_STATUS_INVALID_PARAMETER 0x4
+
+static void nmi_scratch_append(NMIDevice *nmi, const void *buf, size_t count)
+{
+    assert(nmi->pos + count <= NMI_MAX_MESSAGE_LENGTH);
+
+    memcpy(nmi->scratch + nmi->pos, buf, count);
+    nmi->pos += count;
+}
+
+static void nmi_set_parameter_error(NMIDevice *nmi, uint8_t bit, uint16_t byte)
+{
+    /* NVM Express Management Interface 1.2c, Figure 30 */
+    struct resp {
+        uint8_t  status;
+        uint8_t  bit;
+        uint16_t byte;
+    };
+
+    struct resp buf = {
+        .status = NMI_STATUS_INVALID_PARAMETER,
+        .bit = bit & 0x3,
+        .byte = byte,
+    };
+
+    nmi_scratch_append(nmi, &buf, sizeof(buf));
+}
+
+static void nmi_set_error(NMIDevice *nmi, uint8_t status)
+{
+    const uint8_t buf[4] = {status,};
+
+    nmi_scratch_append(nmi, buf, sizeof(buf));
+}
+
+static void nmi_handle_mi_read_nmi_ds(NMIDevice *nmi, NMIRequest *request)
+{
+    I2CSlave *i2c = I2C_SLAVE(nmi);
+
+    uint32_t dw0 = le32_to_cpu(request->dw0);
+    uint8_t dtyp = FIELD_EX32(dw0, NMI_CMD_READ_NMI_DS_DW0, DTYP);
+    const uint8_t *buf;
+    size_t len;
+
+    trace_nmi_handle_mi_read_nmi_ds(dtyp);
+
+    static const uint8_t nmi_ds_subsystem[36] = {
+        0x00,       /* success */
+        0x20, 0x00, /* response data length */
+        0x00,       /* reserved */
+        0x00,       /* number of ports */
+        0x01,       /* major version */
+        0x01,       /* minor version */
+    };
+
+    /*
+     * Cannot be static (or const) since we need to patch in the i2c
+     * address.
+     */
+    const uint8_t nmi_ds_ports[36] = {
+        0x00,       /* success */
+        0x20, 0x00, /* response data length */
+        0x00,       /* reserved */
+        0x02,       /* port type (smbus) */
+        0x00,       /* reserved */
+        0x40, 0x00, /* maximum mctp transission unit size (64 bytes) */
+        0x00, 0x00, 0x00, 0x00, /* management endpoint buffer size */
+        0x00,       /* vpd i2c address */
+        0x00,       /* vpd i2c frequency */
+        i2c->address, /* management endpoint i2c address */
+        0x01,       /* management endpoint i2c frequency */
+        0x00,       /* nvme basic management command NOT supported */
+    };
+
+    /**
+     * Controller Information is zeroed, since there are no associated
+     * controllers at this point.
+     */
+    static const uint8_t nmi_ds_ctrl[36] = {};
+
+    /**
+     * For the Controller List, Optionally Supported Command List and
+     * Management Endpoint Buffer Supported Command List data structures.
+     *
+     * The Controller List data structure is defined in the NVM Express Base
+     * Specification, revision 2.0b, Figure 134.
+     */
+    static const uint8_t nmi_ds_empty[6] = {
+        0x00,       /* success */
+        0x02,       /* response data length */
+        0x00, 0x00, /* reserved */
+        0x00, 0x00, /* number of entries (zero) */
+    };
+
+    switch (dtyp) {
+    case NMI_CMD_READ_NMI_DS_SUBSYSTEM:
+        len = sizeof(nmi_ds_subsystem);
+        buf = nmi_ds_subsystem;
+
+        break;
+
+    case NMI_CMD_READ_NMI_DS_PORTS:
+        len = sizeof(nmi_ds_ports);
+        buf = nmi_ds_ports;
+
+        break;
+
+    case NMI_CMD_READ_NMI_DS_CTRL_INFO:
+        len = sizeof(nmi_ds_ctrl);
+        buf = nmi_ds_ctrl;
+
+        break;
+
+    case NMI_CMD_READ_NMI_DS_CTRL_LIST:
+    case NMI_CMD_READ_NMI_DS_OPT_CMD_SUPPORT:
+    case NMI_CMD_READ_NMI_DS_MEB_CMD_SUPPORT:
+        len = sizeof(nmi_ds_empty);
+        buf = nmi_ds_empty;
+
+        break;
+
+    default:
+        nmi_set_parameter_error(nmi, offsetof(NMIRequest, dw0) + 4, 0);
+
+        return;
+    }
+
+    nmi_scratch_append(nmi, buf, len);
+}
+
+FIELD(NMI_CMD_CONFIGURATION_GET_DW0, IDENTIFIER, 0, 8)
+
+enum {
+    NMI_CMD_CONFIGURATION_GET_SMBUS_FREQ                = 0x1,
+    NMI_CMD_CONFIGURATION_GET_HEALTH_STATUS_CHANGE      = 0x2,
+    NMI_CMD_CONFIGURATION_GET_MCTP_TRANSMISSION_UNIT    = 0x3,
+};
+
+static void nmi_handle_mi_config_get(NMIDevice *nmi, NMIRequest *request)
+{
+    uint32_t dw0 = le32_to_cpu(request->dw0);
+    uint8_t identifier = FIELD_EX32(dw0, NMI_CMD_CONFIGURATION_GET_DW0,
+                                    IDENTIFIER);
+    const uint8_t *buf;
+
+    static const uint8_t smbus_freq[4] = {
+        0x00,               /* success */
+        0x01, 0x00, 0x00,   /* 100 kHz */
+    };
+
+    static const uint8_t mtu[4] = {
+        0x00,       /* success */
+        0x40, 0x00, /* 64 */
+        0x00,       /* reserved */
+    };
+
+    trace_nmi_handle_mi_config_get(identifier);
+
+    switch (identifier) {
+    case NMI_CMD_CONFIGURATION_GET_SMBUS_FREQ:
+        buf = smbus_freq;
+        break;
+
+    case NMI_CMD_CONFIGURATION_GET_MCTP_TRANSMISSION_UNIT:
+        buf = mtu;
+        break;
+
+    default:
+        nmi_set_parameter_error(nmi, 0x0, offsetof(NMIRequest, dw0));
+        return;
+    }
+
+    nmi_scratch_append(nmi, buf, sizeof(buf));
+}
+
+enum {
+    NMI_CMD_READ_NMI_DS         = 0x0,
+    NMI_CMD_CONFIGURATION_GET   = 0x4,
+};
+
+static void nmi_handle_mi(NMIDevice *nmi, NMIMessage *msg)
+{
+    NMIRequest *request = (NMIRequest *)msg->payload;
+
+    trace_nmi_handle_mi(request->opc);
+
+    switch (request->opc) {
+    case NMI_CMD_READ_NMI_DS:
+        nmi_handle_mi_read_nmi_ds(nmi, request);
+        break;
+
+    case NMI_CMD_CONFIGURATION_GET:
+        nmi_handle_mi_config_get(nmi, request);
+        break;
+
+    default:
+        nmi_set_parameter_error(nmi, 0x0, 0x0);
+        fprintf(stderr, "nmi command 0x%x not handled\n", request->opc);
+
+        break;
+    }
+}
+
+static void nmi_reset(MCTPI2CEndpoint *mctp)
+{
+    NMIDevice *nmi = NMI_I2C_DEVICE(mctp);
+    nmi->len = 0;
+}
+
+static void nmi_handle(MCTPI2CEndpoint *mctp)
+{
+    NMIDevice *nmi = NMI_I2C_DEVICE(mctp);
+    NMIMessage *msg = (NMIMessage *)nmi->buffer;
+    uint32_t crc;
+    uint8_t nmimt;
+
+    const uint8_t buf[] = {
+        msg->mctpd,
+        FIELD_DP8(msg->nmp, NMI_NMP, ROR, 1),
+        0x0, 0x0,
+    };
+
+    if (FIELD_EX8(msg->mctpd, NMI_MCTPD, MT) != NMI_MCTPD_MT_NMI) {
+        goto drop;
+    }
+
+    if (FIELD_EX8(msg->mctpd, NMI_MCTPD, IC) != NMI_MCTPD_IC_ENABLED) {
+        goto drop;
+    }
+
+    nmi->pos = 0;
+    nmi_scratch_append(nmi, buf, sizeof(buf));
+
+    nmimt = FIELD_EX8(msg->nmp, NMI_NMP, NMIMT);
+
+    trace_nmi_handle_msg(nmimt);
+
+    switch (nmimt) {
+    case NMI_NMP_NMIMT_NVME_MI:
+        nmi_handle_mi(nmi, msg);
+        break;
+
+    default:
+        fprintf(stderr, "nmi message type 0x%x not handled\n", nmimt);
+
+        nmi_set_error(nmi, 0x3);
+
+        break;
+    }
+
+    crc = crc32c(0xffffffff, nmi->scratch, nmi->pos);
+    nmi_scratch_append(nmi, &crc, sizeof(crc));
+
+    nmi->len = nmi->pos;
+    nmi->pos = 0;
+
+    i2c_mctp_schedule_send(mctp);
+
+    return;
+
+drop:
+    nmi_reset(mctp);
+}
+
+static size_t nmi_get_buf(MCTPI2CEndpoint *mctp, const uint8_t **buf,
+                          size_t maxlen, uint8_t *mctp_flags)
+{
+    NMIDevice *nmi = NMI_I2C_DEVICE(mctp);
+    size_t len;
+
+    len = MIN(maxlen, nmi->len - nmi->pos);
+
+    if (len == 0) {
+        return 0;
+    }
+
+    if (nmi->pos == 0) {
+        *mctp_flags = FIELD_DP8(*mctp_flags, MCTP_H_FLAGS, SOM, 1);
+    }
+
+    *buf = nmi->scratch + nmi->pos;
+    nmi->pos += len;
+
+    if (nmi->pos == nmi->len) {
+        *mctp_flags = FIELD_DP8(*mctp_flags, MCTP_H_FLAGS, EOM, 1);
+
+        nmi->pos = nmi->len = 0;
+    }
+
+    return len;
+}
+
+static int nmi_put_buf(MCTPI2CEndpoint *mctp, uint8_t *buf, size_t len)
+{
+    NMIDevice *nmi = NMI_I2C_DEVICE(mctp);
+
+    if (nmi->len + len > NMI_MAX_MESSAGE_LENGTH) {
+        return -1;
+    }
+
+    memcpy(nmi->buffer + nmi->len, buf, len);
+    nmi->len += len;
+
+    return 0;
+}
+
+static size_t nmi_get_types(MCTPI2CEndpoint *mctp, const uint8_t **data)
+{
+    /**
+     * DSP0236 1.3.0, Table 19.
+     *
+     * This only includes message types that are supported *in addition* to the
+     * MCTP control message type.
+     */
+    static const uint8_t buf[] = {
+        0x0,                /* success */
+        0x1,                /* number of message types in list (supported) */
+        NMI_MCTPD_MT_NMI,
+    };
+
+    *data = buf;
+
+    return sizeof(buf);
+}
+
+static void nvme_mi_class_init(ObjectClass *oc, void *data)
+{
+    MCTPI2CEndpointClass *mc = MCTP_I2C_ENDPOINT_CLASS(oc);
+
+    mc->get_types = nmi_get_types;
+
+    mc->get_buf = nmi_get_buf;
+    mc->put_buf = nmi_put_buf;
+
+    mc->handle = nmi_handle;
+    mc->reset = nmi_reset;
+}
+
+static const TypeInfo nvme_mi = {
+    .name = TYPE_NMI_I2C_DEVICE,
+    .parent = TYPE_MCTP_I2C_ENDPOINT,
+    .instance_size = sizeof(NMIDevice),
+    .class_init = nvme_mi_class_init,
+};
+
+static void register_types(void)
+{
+    type_register_static(&nvme_mi);
+}
+
+type_init(register_types);
diff --git a/hw/nvme/trace-events b/hw/nvme/trace-events
index 3a67680c6ad1..41e2c540dcf2 100644
--- a/hw/nvme/trace-events
+++ b/hw/nvme/trace-events
@@ -216,3 +216,9 @@  pci_nvme_ub_db_wr_invalid_sq(uint32_t qid) "submission queue doorbell write for
 pci_nvme_ub_db_wr_invalid_sqtail(uint32_t qid, uint16_t new_tail) "submission queue doorbell write value beyond queue size, sqid=%"PRIu32", new_head=%"PRIu16", ignoring"
 pci_nvme_ub_unknown_css_value(void) "unknown value in cc.css field"
 pci_nvme_ub_too_many_mappings(void) "too many prp/sgl mappings"
+
+# nmi-i2c
+nmi_handle_mi_read_nmi_ds(uint8_t dtyp) "dtyp 0x%"PRIx8""
+nmi_handle_mi_config_get(uint8_t identifier) "identifier 0x%"PRIx8""
+nmi_handle_mi(uint8_t opc) "opc 0x%"PRIx8""
+nmi_handle_msg(uint8_t nmint) "nmint 0x%"PRIx8""