Re: [PATCH RFC 2/3] hw/i2c: add mctp core

[Corey Minyard]

On Wed, Nov 16, 2022 at 09:43:11AM +0100, Klaus Jensen wrote:
> From: Klaus Jensen <k.jensen@samsung.com>
> 
> Add an abstract MCTP over I2C endpoint model. This implements MCTP
> control message handling as well as handling the actual I2C transport
> (packetization).
> 
> Devices are intended to derive from this and implement the class
> methods.
> 
> Parts of this implementation is inspired by code[1] previously posted by
> Jonathan Cameron.

I have some comments inline, mostly about buffer handling.  Buffer
handling is scary to me, so you might see some paranoia here :-).

> 
>   [1]: 
> https://lore.kernel.org/qemu-devel/20220520170128.4436-1-Jonathan.Cameron@huawei.com/
> 
> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
> ---
>  hw/arm/Kconfig         |   1 +
>  hw/i2c/Kconfig         |   4 +
>  hw/i2c/mctp.c          | 365 +++++++++++++++++++++++++++++++++++++++++
>  hw/i2c/meson.build     |   1 +
>  hw/i2c/trace-events    |  12 ++
>  include/hw/i2c/mctp.h  |  83 ++++++++++
>  include/hw/misc/mctp.h |  43 +++++
>  7 files changed, 509 insertions(+)
>  create mode 100644 hw/i2c/mctp.c
>  create mode 100644 include/hw/i2c/mctp.h
>  create mode 100644 include/hw/misc/mctp.h
> 
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 17fcde8e1ccc..3233bdc193d7 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -444,6 +444,7 @@ config ASPEED_SOC
>      select DS1338
>      select FTGMAC100
>      select I2C
> +    select MCTP_I2C
>      select DPS310
>      select PCA9552
>      select SERIAL
> diff --git a/hw/i2c/Kconfig b/hw/i2c/Kconfig
> index 9bb8870517f8..5dd43d550c32 100644
> --- a/hw/i2c/Kconfig
> +++ b/hw/i2c/Kconfig
> @@ -41,3 +41,7 @@ config PCA954X
>  config PMBUS
>      bool
>      select SMBUS
> +
> +config MCTP_I2C
> +    bool
> +    select I2C
> diff --git a/hw/i2c/mctp.c b/hw/i2c/mctp.c
> new file mode 100644
> index 000000000000..46376de95a98
> --- /dev/null
> +++ b/hw/i2c/mctp.c
> @@ -0,0 +1,365 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + * SPDX-FileCopyrightText: Copyright (c) 2022 Samsung Electronics Co., Ltd.
> + * SPDX-FileContributor: Klaus Jensen <k.jensen@samsung.com>
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/main-loop.h"
> +
> +#include "hw/qdev-properties.h"
> +#include "hw/i2c/i2c.h"
> +#include "hw/i2c/mctp.h"
> +
> +#include "trace.h"
> +
> +static uint8_t crc8(uint16_t data)
> +{
> +#define POLY (0x1070U << 3)
> +    int i;
> +
> +    for (i = 0; i < 8; i++) {
> +        if (data & 0x8000) {
> +            data = data ^ POLY;
> +        }
> +
> +        data = data << 1;
> +    }
> +
> +    return (uint8_t)(data >> 8);
> +#undef POLY
> +}
> +
> +static uint8_t i2c_smbus_pec(uint8_t crc, uint8_t *buf, size_t len)
> +{
> +    int i;
> +
> +    for (i = 0; i < len; i++) {
> +        crc = crc8((crc ^ buf[i]) << 8);
> +    }
> +
> +    return crc;
> +}

The PEC calculation probably belongs in it's own smbus.c file, since
it's generic, so someone looking will find it.

> +
> +void i2c_mctp_schedule_send(MCTPI2CEndpoint *mctp)
> +{
> +    I2CBus *i2c = I2C_BUS(qdev_get_parent_bus(DEVICE(mctp)));
> +
> +    mctp->tx.state = I2C_MCTP_STATE_TX_START_SEND;
> +
> +    i2c_bus_master(i2c, mctp->tx.bh);
> +}
> +
> +static void i2c_mctp_tx(void *opaque)
> +{
> +    DeviceState *dev = DEVICE(opaque);
> +    I2CBus *i2c = I2C_BUS(qdev_get_parent_bus(dev));
> +    I2CSlave *slave = I2C_SLAVE(dev);
> +    MCTPI2CEndpoint *mctp = MCTP_I2C_ENDPOINT(dev);
> +    MCTPI2CEndpointClass *mc = MCTP_I2C_ENDPOINT_GET_CLASS(mctp);
> +    MCTPI2CPacket *pkt = (MCTPI2CPacket *)mctp->buffer;
> +    uint8_t flags = 0;
> +
> +    switch (mctp->tx.state) {
> +    case I2C_MCTP_STATE_TX_SEND_BYTE:
> +        if (mctp->pos < mctp->len) {
> +            uint8_t byte = mctp->buffer[mctp->pos];
> +
> +            trace_i2c_mctp_tx_send_byte(mctp->pos, byte);
> +
> +            /* send next byte */
> +            i2c_send_async(i2c, byte);
> +
> +            mctp->pos++;
> +
> +            break;
> +        }
> +
> +        /* packet sent */
> +        i2c_end_transfer(i2c);
> +
> +        /* fall through */
> +
> +    case I2C_MCTP_STATE_TX_START_SEND:
> +        if (mctp->tx.is_control) {
> +            /* packet payload is already in buffer */
> +            flags |= MCTP_H_FLAGS_SOM | MCTP_H_FLAGS_EOM;
> +        } else {
> +            /* get message bytes from derived device */
> +            mctp->len = mc->get_message_bytes(mctp, pkt->mctp.payload,
> +                                              I2C_MCTP_MAXMTU, &flags);
> +        }
> +
> +        if (!mctp->len) {
> +            trace_i2c_mctp_tx_done();
> +
> +            /* no more packets needed; release the bus */
> +            i2c_bus_release(i2c);
> +
> +            mctp->state = I2C_MCTP_STATE_IDLE;
> +            mctp->tx.is_control = false;
> +
> +            break;
> +        }
> +
> +        mctp->state = I2C_MCTP_STATE_TX;
> +
> +        pkt->i2c = (MCTPI2CPacketHeader) {
> +            .dest = mctp->tx.addr & ~0x1,
> +            .prot = 0xf,
> +            .byte_count = 5 + mctp->len,
> +            .source = slave->address << 1 | 0x1,
> +        };
> +
> +        pkt->mctp.hdr = (MCTPPacketHeader) {
> +            .version = 0x1,
> +            .eid.dest = mctp->tx.eid,
> +            .eid.source = mctp->my_eid,
> +            .flags = flags | (mctp->tx.pktseq++ & 0x3) << 4 | mctp->tx.flags,
> +        };
> +
> +        mctp->len += sizeof(MCTPI2CPacket);

Do you need overflow checking here?  There are lots of increments of
mctp->len in the code that might or might not need overflow checks.
It does seem like you have pre-calculated everything so it fits; I worry
more about later changes that might violate those assumptions.
You could use something like i2c_mctp_send_cb() to send all data.  Not
sure, but something to think about.

> +        mctp->buffer[mctp->len] = i2c_smbus_pec(0, mctp->buffer, mctp->len);
> +        mctp->len++;
> +
> +        trace_i2c_mctp_tx_start_send(mctp->len);
> +
> +        i2c_start_send_async(i2c, pkt->i2c.dest >> 1);
> +
> +        /* already "sent" the destination slave address */
> +        mctp->pos = 1;
> +
> +        mctp->tx.state = I2C_MCTP_STATE_TX_SEND_BYTE;
> +
> +        break;
> +    }
> +}
> +
> +#define i2c_mctp_control_data(buf) \
> +    (i2c_mctp_payload(buf) + offsetof(MCTPControlMessage, data))
> +
> +static void i2c_mctp_handle_control_set_eid(MCTPI2CEndpoint *mctp, uint8_t 
> eid)
> +{
> +    mctp->my_eid = eid;
> +
> +    uint8_t buf[] = {
> +        0x0, 0x0, eid, 0x0,
> +    };
> +
> +    memcpy(i2c_mctp_control_data(mctp->buffer), buf, sizeof(buf));
> +    mctp->len += sizeof(buf);
> +}
> +
> +static void i2c_mctp_handle_control_get_eid(MCTPI2CEndpoint *mctp)
> +{
> +    uint8_t buf[] = {
> +        0x0, mctp->my_eid, 0x0, 0x0,
> +    };
> +
> +    memcpy(i2c_mctp_control_data(mctp->buffer), buf, sizeof(buf));
> +    mctp->len += sizeof(buf);
> +}
> +
> +static void i2c_mctp_handle_control_get_version(MCTPI2CEndpoint *mctp)
> +{
> +    uint8_t buf[] = {
> +        0x0, 0x1, 0x0, 0x1, 0x3, 0x1,
> +    };
> +
> +    memcpy(i2c_mctp_control_data(mctp->buffer), buf, sizeof(buf));
> +    mctp->len += sizeof(buf);
> +}
> +
> +enum {
> +    MCTP_CONTROL_SET_EID                    = 0x01,
> +    MCTP_CONTROL_GET_EID                    = 0x02,
> +    MCTP_CONTROL_GET_VERSION                = 0x04,
> +    MCTP_CONTROL_GET_MESSAGE_TYPE_SUPPORT   = 0x05,
> +};
> +
> +static void i2c_mctp_handle_control(MCTPI2CEndpoint *mctp)
> +{
> +    MCTPI2CEndpointClass *mc = MCTP_I2C_ENDPOINT_GET_CLASS(mctp);
> +    MCTPControlMessage *msg = (MCTPControlMessage 
> *)i2c_mctp_payload(mctp->buffer);
> +
> +    /* clear Rq/D */
> +    msg->flags &= 0x1f;
> +
> +    mctp->len = offsetof(MCTPControlMessage, data);
> +
> +    trace_i2c_mctp_handle_control(msg->command);
> +
> +    switch (msg->command) {
> +    case MCTP_CONTROL_SET_EID:
> +        i2c_mctp_handle_control_set_eid(mctp, msg->data[1]);
> +        break;
> +
> +    case MCTP_CONTROL_GET_EID:
> +        i2c_mctp_handle_control_get_eid(mctp);
> +        break;
> +
> +    case MCTP_CONTROL_GET_VERSION:
> +        i2c_mctp_handle_control_get_version(mctp);
> +        break;
> +
> +    case MCTP_CONTROL_GET_MESSAGE_TYPE_SUPPORT:
> +        mctp->len += mc->get_message_types(mctp, 
> i2c_mctp_control_data(mctp->buffer));

You don't pass in how much data is available for the subclass to use.
That's generally good API behavior.

> +        break;
> +
> +    default:
> +        trace_i2c_mctp_unhandled_control(msg->command);
> +
> +        msg->data[0] = MCTP_CONTROL_ERROR_UNSUPPORTED_CMD;
> +        mctp->len++;
> +
> +        break;
> +    }
> +
> +    i2c_mctp_schedule_send(mctp);
> +}
> +
> +static int i2c_mctp_event_cb(I2CSlave *i2c, enum i2c_event event)
> +{
> +    MCTPI2CEndpoint *mctp = MCTP_I2C_ENDPOINT(i2c);
> +    MCTPI2CEndpointClass *mc = MCTP_I2C_ENDPOINT_GET_CLASS(mctp);
> +    MCTPI2CPacket *pkt = (MCTPI2CPacket *)mctp->buffer;
> +    size_t payload_len;
> +    uint8_t pec;
> +
> +    switch (event) {
> +    case I2C_START_SEND:
> +        if (mctp->state != I2C_MCTP_STATE_IDLE) {
> +            return -1;
> +        }
> +
> +        /* the i2c core eats the slave address, so put it back in */
> +        pkt->i2c.dest = i2c->address << 1;

This seems like a bit of a hack since pkt->i2c.dest never seems to be
used.  I guess it's ok, since it's matching what the specifications say,
but it seems a bit odd since you don't need it.

> +        mctp->len = 1;
> +
> +        mctp->state = I2C_MCTP_STATE_RX_STARTED;
> +
> +        return 0;
> +
> +    case I2C_FINISH:
> +        payload_len = mctp->len - (1 + offsetof(MCTPI2CPacket, 
> mctp.payload));

Is there a way this can underflow?

> +
> +        if (pkt->i2c.byte_count + 3 != mctp->len - 1) {
> +            trace_i2c_mctp_drop_invalid_length(pkt->i2c.byte_count + 3,
> +                                               mctp->len - 1);
> +            goto drop;
> +        }
> +
> +        pec = i2c_smbus_pec(0, mctp->buffer, mctp->len - 1);
> +        if (mctp->buffer[mctp->len - 1] != pec) {
> +            trace_i2c_mctp_drop_invalid_pec(mctp->buffer[mctp->len - 1], 
> pec);
> +            goto drop;
> +        }
> +
> +        if (pkt->mctp.hdr.eid.dest != mctp->my_eid) {
> +            trace_i2c_mctp_drop_invalid_eid(pkt->mctp.hdr.eid.dest,
> +                                            mctp->my_eid);
> +            goto drop;
> +        }
> +
> +        if (pkt->mctp.hdr.flags & MCTP_H_FLAGS_SOM) {
> +            mctp->tx.is_control = false;
> +
> +            if (mctp->state == I2C_MCTP_STATE_RX) {
> +                mc->reset_message(mctp);
> +            }
> +
> +            mctp->state = I2C_MCTP_STATE_RX;
> +
> +            mctp->tx.addr = pkt->i2c.source;
> +            mctp->tx.eid = pkt->mctp.hdr.eid.source;
> +            mctp->tx.flags = pkt->mctp.hdr.flags & 0x7;
> +            mctp->tx.pktseq = (pkt->mctp.hdr.flags >> 4) & 0x3;
> +
> +            if ((pkt->mctp.payload[0] & 0x7f) == MCTP_MESSAGE_TYPE_CONTROL) {
> +                mctp->tx.is_control = true;
> +
> +                i2c_mctp_handle_control(mctp);
> +
> +                return 0;
> +            }
> +        } else if (mctp->state == I2C_MCTP_STATE_RX_STARTED) {
> +            trace_i2c_mctp_drop("expected SOM");
> +            goto drop;
> +        } else if (((pkt->mctp.hdr.flags >> 4) & 0x3) != (mctp->tx.pktseq++ 
> & 0x3)) {
> +            trace_i2c_mctp_drop_invalid_pktseq((pkt->mctp.hdr.flags >> 4) & 
> 0x3,
> +                                               mctp->tx.pktseq & 0x3);
> +            goto drop;
> +        }
> +
> +        mc->put_message_bytes(mctp, i2c_mctp_payload(mctp->buffer), 
> payload_len);
> +
> +        if (pkt->mctp.hdr.flags & MCTP_H_FLAGS_EOM) {
> +            mc->handle_message(mctp);
> +            mctp->state = I2C_MCTP_STATE_WAIT_TX;
> +        }
> +
> +        return 0;
> +
> +    default:
> +        return -1;
> +    }
> +
> +drop:
> +    mc->reset_message(mctp);
> +
> +    mctp->state = I2C_MCTP_STATE_IDLE;
> +
> +    return 0;
> +}
> +
> +static int i2c_mctp_send_cb(I2CSlave *i2c, uint8_t data)
> +{
> +    MCTPI2CEndpoint *mctp = MCTP_I2C_ENDPOINT(i2c);
> +
> +    if (mctp->len < I2C_MCTP_MAX_LENGTH) {
> +        mctp->buffer[mctp->len++] = data;
> +        return 0;
> +    }
> +
> +    return -1;
> +}
> +
> +static void i2c_mctp_instance_init(Object *obj)
> +{
> +    MCTPI2CEndpoint *mctp = MCTP_I2C_ENDPOINT(obj);
> +
> +    mctp->tx.bh = qemu_bh_new(i2c_mctp_tx, mctp);
> +}
> +
> +static Property mctp_i2c_props[] = {
> +    DEFINE_PROP_UINT8("eid", MCTPI2CEndpoint, my_eid, 0x9),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
> +static void i2c_mctp_class_init(ObjectClass *oc, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(oc);
> +    I2CSlaveClass *k = I2C_SLAVE_CLASS(oc);
> +
> +    k->event = i2c_mctp_event_cb;
> +    k->send = i2c_mctp_send_cb;
> +
> +    device_class_set_props(dc, mctp_i2c_props);
> +}
> +
> +static const TypeInfo i2c_mctp_info = {
> +    .name = TYPE_MCTP_I2C_ENDPOINT,
> +    .parent = TYPE_I2C_SLAVE,
> +    .abstract = true,
> +    .instance_init = i2c_mctp_instance_init,
> +    .instance_size = sizeof(MCTPI2CEndpoint),
> +    .class_init = i2c_mctp_class_init,
> +    .class_size = sizeof(MCTPI2CEndpointClass),
> +};
> +
> +static void register_types(void)
> +{
> +    type_register_static(&i2c_mctp_info);
> +}
> +
> +type_init(register_types)
> diff --git a/hw/i2c/meson.build b/hw/i2c/meson.build
> index d3df273251f7..97d0bfd9ac67 100644
> --- a/hw/i2c/meson.build
> +++ b/hw/i2c/meson.build
> @@ -1,5 +1,6 @@
>  i2c_ss = ss.source_set()
>  i2c_ss.add(when: 'CONFIG_I2C', if_true: files('core.c'))
> +i2c_ss.add(when: 'CONFIG_MCTP_I2C', if_true: files('mctp.c'))
>  i2c_ss.add(when: 'CONFIG_SMBUS', if_true: files('smbus_slave.c', 
> 'smbus_master.c'))
>  i2c_ss.add(when: 'CONFIG_ACPI_SMBUS', if_true: files('pm_smbus.c'))
>  i2c_ss.add(when: 'CONFIG_ACPI_X86_ICH', if_true: files('smbus_ich9.c'))
> diff --git a/hw/i2c/trace-events b/hw/i2c/trace-events
> index af181d43ee64..778139d08166 100644
> --- a/hw/i2c/trace-events
> +++ b/hw/i2c/trace-events
> @@ -33,3 +33,15 @@ npcm7xx_smbus_recv_fifo(const char *id, uint8_t received, 
> uint8_t expected) "%s
>  
>  pca954x_write_bytes(uint8_t value) "PCA954X write data: 0x%02x"
>  pca954x_read_data(uint8_t value) "PCA954X read data: 0x%02x"
> +
> +# mctp.c
> +i2c_mctp_tx_start_send(size_t len) "len %zu"
> +i2c_mctp_tx_send_byte(size_t pos, uint8_t byte) "pos %zu byte 0x%"PRIx8""
> +i2c_mctp_tx_done(void) "packet sent"
> +i2c_mctp_handle_control(uint8_t command) "command 0x%"PRIx8""
> +i2c_mctp_unhandled_control(uint8_t command) "command 0x%"PRIx8""
> +i2c_mctp_drop(const char *reason) "%s"
> +i2c_mctp_drop_invalid_length(unsigned byte_count, size_t expected) 
> "byte_count %u expected %zu"
> +i2c_mctp_drop_invalid_pec(uint8_t pec, uint8_t expected) "pec 0x%"PRIx8" 
> expected 0x%"PRIx8""
> +i2c_mctp_drop_invalid_eid(uint8_t eid, uint8_t expected) "eid 0x%"PRIx8" 
> expected 0x%"PRIx8""
> +i2c_mctp_drop_invalid_pktseq(uint8_t pktseq, uint8_t expected) "pktseq 
> 0x%"PRIx8" expected 0x%"PRIx8""
> diff --git a/include/hw/i2c/mctp.h b/include/hw/i2c/mctp.h
> new file mode 100644
> index 000000000000..17dae662138b
> --- /dev/null
> +++ b/include/hw/i2c/mctp.h
> @@ -0,0 +1,83 @@
> +#ifndef QEMU_I2C_MCTP_H
> +#define QEMU_I2C_MCTP_H
> +
> +#include "qom/object.h"
> +#include "hw/qdev-core.h"
> +#include "hw/misc/mctp.h"
> +
> +typedef struct MCTPI2CPacketHeader {
> +    uint8_t dest;
> +    uint8_t prot;
> +    uint8_t byte_count;
> +    uint8_t source;
> +} MCTPI2CPacketHeader;
> +
> +typedef struct MCTPI2CPacket {
> +    MCTPI2CPacketHeader i2c;
> +    MCTPPacket          mctp;
> +} MCTPI2CPacket;
> +
> +#define i2c_mctp_payload(buf) (buf + offsetof(MCTPI2CPacket, mctp.payload))
> +
> +#define TYPE_MCTP_I2C_ENDPOINT "mctp-i2c-endpoint"
> +OBJECT_DECLARE_TYPE(MCTPI2CEndpoint, MCTPI2CEndpointClass, MCTP_I2C_ENDPOINT)
> +
> +struct MCTPI2CEndpointClass {
> +    I2CSlaveClass parent_class;
> +
> +    int (*put_message_bytes)(MCTPI2CEndpoint *mctp, uint8_t *buf, size_t 
> len);
> +    size_t (*get_message_bytes)(MCTPI2CEndpoint *mctp, uint8_t *buf,
> +                                size_t maxlen, uint8_t *mctp_flags);
> +
> +    void (*handle_message)(MCTPI2CEndpoint *mctp);
> +    void (*reset_message)(MCTPI2CEndpoint *mctp);
> +
> +    size_t (*get_message_types)(MCTPI2CEndpoint *mctp, uint8_t *data);
> +};
> +
> +#define I2C_MCTP_MAXBLOCK 255
> +#define I2C_MCTP_MAXMTU (I2C_MCTP_MAXBLOCK - (sizeof(MCTPPacketHeader) + 1))
> +#define I2C_MCTP_MAX_LENGTH (3 + I2C_MCTP_MAXBLOCK + 1)
> +
> +typedef enum {
> +    I2C_MCTP_STATE_IDLE,
> +    I2C_MCTP_STATE_RX_STARTED,
> +    I2C_MCTP_STATE_RX,
> +    I2C_MCTP_STATE_WAIT_TX,
> +    I2C_MCTP_STATE_TX,
> +} MCTPState;
> +
> +typedef enum {
> +    I2C_MCTP_STATE_TX_START_SEND,
> +    I2C_MCTP_STATE_TX_SEND_BYTE,
> +} MCTPTxState;
> +
> +typedef struct MCTPI2CEndpoint {
> +    I2CSlave parent_obj;
> +    I2CBus *i2c;
> +
> +    MCTPState state;
> +
> +    /* mctp endpoint identifier */
> +    uint8_t my_eid;
> +
> +    uint8_t  buffer[I2C_MCTP_MAX_LENGTH];
> +    uint64_t pos;
> +    size_t   len;
> +
> +    struct {
> +        MCTPTxState state;
> +        bool is_control;
> +
> +        uint8_t eid;
> +        uint8_t addr;
> +        uint8_t pktseq;
> +        uint8_t flags;
> +
> +        QEMUBH *bh;
> +    } tx;
> +} MCTPI2CEndpoint;
> +
> +void i2c_mctp_schedule_send(MCTPI2CEndpoint *mctp);
> +
> +#endif /* QEMU_I2C_MCTP_H */
> diff --git a/include/hw/misc/mctp.h b/include/hw/misc/mctp.h
> new file mode 100644
> index 000000000000..c936224ecf60
> --- /dev/null
> +++ b/include/hw/misc/mctp.h
> @@ -0,0 +1,43 @@
> +#ifndef QEMU_MCTP_H
> +#define QEMU_MCTP_H
> +
> +#define MCTP_BASELINE_MTU 64
> +
> +enum {
> +    MCTP_H_FLAGS_EOM = 1 << 6,
> +    MCTP_H_FLAGS_SOM = 1 << 7,
> +};
> +
> +enum {
> +    MCTP_MESSAGE_TYPE_CONTROL   = 0x0,
> +    MCTP_MESSAGE_TYPE_NMI       = 0x4,
> +
> +    MCTP_MESSAGE_IC             = 1 << 7,
> +};
> +
> +typedef struct MCTPPacketHeader {
> +    uint8_t version;
> +    struct {
> +        uint8_t dest;
> +        uint8_t source;
> +    } eid;
> +    uint8_t flags;
> +} MCTPPacketHeader;
> +
> +typedef struct MCTPPacket {
> +    MCTPPacketHeader hdr;
> +    uint8_t          payload[];
> +} MCTPPacket;
> +
> +typedef struct MCTPControlMessage {
> +    uint8_t type;
> +    uint8_t flags;
> +    uint8_t command;
> +    uint8_t data[];
> +} MCTPControlMessage;
> +
> +enum {
> +    MCTP_CONTROL_ERROR_UNSUPPORTED_CMD = 0x5,
> +};
> +
> +#endif /* QEMU_MCTP_H */
> -- 
> 2.38.1
> 
>