Re: [PATCH v3 5/7] hw/cxl/events: Add injection of General Media Events

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Mon, 27 Feb 2023 17:34:14 +0000
Jonathan Cameron <Jonathan.Cameron@huawei.com> wrote:

> From: Ira Weiny <ira.weiny@intel.com>
> 
> To facilitate testing provide a QMP command to inject a general media
> event.  The event can be added to the log specified.
> 
> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
> Link: https://lore.kernel.org/r/20221221-ira-cxl-events-2022-11-17-v2-8-2ce2ecc06219@intel.com
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Took another look at this after some review feedback on remaining endian issues.
There are some of those + the timestamp in the header is not in device time.
I'll fix up for v4.

> +static void cxl_assign_event_header(CXLEventRecordHdr *hdr,
> +                                    const QemuUUID *uuid, uint8_t flags,
> +                                    uint8_t length)
> +{
> +    hdr->flags[0] = flags;
Should be st24_le_p() really and a type big enough to hold a 24 bit

> +    hdr->length = length;
> +    memcpy(&hdr->id, uuid, sizeof(hdr->id));
> +    hdr->timestamp = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
This isn't the right timestamp and it should be little endian. 

> +}
> +
> +/* Component ID is device specific.  Define this as a string. */
> +void qmp_cxl_inject_gen_media_event(const char *path, CxlEventLog log,
> +                                    uint8_t flags, uint64_t physaddr,
> +                                    uint8_t descriptor, uint8_t type,
> +                                    uint8_t transaction_type,
> +                                    bool has_channel, uint8_t channel,
> +                                    bool has_rank, uint8_t rank,
> +                                    bool has_device, uint32_t device,
> +                                    const char *component_id,
> +                                    Error **errp)
> +{
> +    Object *obj = object_resolve_path(path, NULL);
> +    CXLEventGenMedia gem;
> +    CXLEventRecordHdr *hdr = &gem.hdr;
> +    CXLDeviceState *cxlds;
> +    CXLType3Dev *ct3d;
> +    uint16_t valid_flags = 0;
> +    uint8_t enc_log;
> +    int rc;
> +
> +    if (!obj) {
> +        error_setg(errp, "Unable to resolve path");
> +        return;
> +    }
> +    if (!object_dynamic_cast(obj, TYPE_CXL_TYPE3)) {
> +        error_setg(errp, "Path does not point to a CXL type 3 device");
> +        return;
> +    }
> +    ct3d = CXL_TYPE3(obj);
> +    cxlds = &ct3d->cxl_dstate;
> +
> +    rc = ct3d_qmp_cxl_event_log_enc(log);
> +    if (rc < 0) {
> +        error_setg(errp, "Unhandled error log type");
> +        return;
> +    }
> +    enc_log = rc;
> +
> +    memset(&gem, 0, sizeof(gem));
> +    cxl_assign_event_header(hdr, &gen_media_uuid, flags, sizeof(gem));
> +
> +    gem.phys_addr = physaddr;
This was the one that was pointed out off list.
Thanks!
> +    gem.descriptor = descriptor;
> +    gem.type = type;
> +    gem.transaction_type = transaction_type;