On Wed, 23 Aug 2023 11:22:00 +0200 Klaus Jensen <i...@irrelevant.dk> wrote:
> From: Klaus Jensen <k.jen...@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.jen...@samsung.com> Hi Klaus Minor suggestions inline. Either way given how trivial they are Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com> > diff --git a/hw/nvme/nmi-i2c.c b/hw/nvme/nmi-i2c.c > new file mode 100644 > index 000000000000..9040ba28a87c > --- /dev/null > +++ b/hw/nvme/nmi-i2c.c > @@ -0,0 +1,418 @@ ... > +#define NMI_MAX_MESSAGE_LENGTH 4224 Spec reference or similar would be good for this. ... > + > +static void nmi_set_error(NMIDevice *nmi, uint8_t status) > +{ > + uint8_t buf[4] = {}; > + > + buf[0] = status; Could just do uint8_t buf[4] = { status, }; > + > + memcpy(nmi->scratch + nmi->pos, buf, 4); > + nmi->pos += 4; 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 = (dw0 >> 24) & 0xf; Maybe FIELD_EX32() with appropriate defines? > + uint8_t *buf; > + size_t len; > + > + trace_nmi_handle_mi_read_nmi_ds(dtyp); > + > + static 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 since we need to patch in the i2c address */ > + 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 */ > + 0x00, /* 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 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 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; > + > + /* patch in the i2c address of the endpoint */ > + buf[14] = i2c->address; > + > + 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; > + } > + > + memcpy(nmi->scratch + nmi->pos, buf, len); > + nmi->pos += len; > +} > + > +static void nmi_handle_mi_config_get(NMIDevice *nmi, NMIRequest *request) > +{ > + uint32_t dw0 = le32_to_cpu(request->dw0); > + uint8_t identifier = dw0 & 0xff; > + uint8_t *buf; > + > + static uint8_t smbus_freq[4] = { > + 0x00, /* success */ > + 0x01, 0x00, 0x00, /* 100 kHz */ > + }; const for these? Same for other similar buffers. > + > + static 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; > + } > + > + memcpy(nmi->scratch + nmi->pos, buf, 4); > + nmi->pos += 4; > +} > + > +static void nmi_handle(MCTPI2CEndpoint *mctp) > +{ > + NMIDevice *nmi = NMI_I2C_DEVICE(mctp); > + NMIMessage *msg = (NMIMessage *)nmi->buffer; > + uint32_t crc; > + uint8_t nmimt; > + > + uint8_t buf[] = { const? > + 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; > + } > + > + memcpy(nmi->scratch, buf, sizeof(buf)); Jonathan