Re: [Linuxarm] Re: [RFC PATCH 3/4] hw/cxl/cxl-cdat: Initial CDAT implementation for use by CXL devices

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

On Tue, 2 Feb 2021 10:49:23 -0800
Ben Widawsky <ben@bwidawsk.net> wrote:

> On 21-02-01 23:16:28, Jonathan Cameron wrote:
> > CDAT is an ACPI like format defined by the CXL consortium. It is
> > available from
> > 
> > https://www.uefi.org/node/4093
> > 
> > Here support for managing all the entires is introduced, along with
> > an implementation of a callback for a DOE mailbox which may be
> > used to read these values from CXL hardware by either firmware or
> > an OS.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>  
> 
> I seem to be missing one critical thing, where is the CDAT header (Table 1 from
> the spec) actually populated, length, revision, checksum, etc?

An excellent question.  As far as I can see there is no actual means defined
to read that from a device.   The DOE stuff in the CXL spec just accesses 'entries'
not the header (thus as far as I can tell making a good chunk of the CDAT spec
entirely pointless).

It's on my list to raise on the SSWG list / call.  Perhaps I'm missing something...

> 
> General, probably underthought-out, comment:
> 
> With the CXLType3Class I added since you probably wrote these patches, I wonder
> if that'd be a better fit for populating some of these tables, having them
> populate dynamically when the DOE/CDAT request actually comes in.
> 
> I have no strong opinion. The one advantage I see to using the class is
> operations like managing handles, length calculations, etc can be managed by the
> helper library rather than the device implementation having to do it. OTTOH if
> you have devices that want to do weird things, they might lose some flexibility.
> 
> I think with just a few methods added to the CXLType3Class you could pretty
> trivially build up a nice sane default CDAT for any CXL device.

I'm not convinced there is such a thing as a sane default, but can certainly
look at this if we take this particular implementation forwards.
I suspect we'll also want to make a lot of this configurable from the command
line or similar on a per device basis.

Thanks,

Jonathan


> 
> > ---
> >  hw/cxl/cxl-cdat.c         | 252 ++++++++++++++++++++++++++++++++++++++
> >  hw/cxl/meson.build        |   1 +
> >  include/hw/cxl/cxl_cdat.h | 101 +++++++++++++++
> >  3 files changed, 354 insertions(+)
> > 
> > diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
> > new file mode 100644
> > index 0000000000..6ed4c15cc0
> > --- /dev/null
> > +++ b/hw/cxl/cxl-cdat.c
> > @@ -0,0 +1,252 @@
> > +/*
> > + * Support for CDAT entires as defined in
> > + * Coherent Device Attribute Table (CDAT) Specification rev 1.02
> > + * Available from uefi.org.
> > + *
> > + * Copyright (c) 2021 Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#include "qemu/osdep.h"
> > +#include "qemu/units.h"
> > +#include "qemu/error-report.h"
> > +#include "hw/mem/memory-device.h"
> > +#include "hw/mem/pc-dimm.h"
> > +#include "hw/pci/pci.h"
> > +#include "hw/pci/doe.h"
> > +#include "hw/qdev-properties.h"
> > +#include "qapi/error.h"
> > +#include "qemu/log.h"
> > +#include "qemu/module.h"
> > +#include "qemu/range.h"
> > +#include "qemu/rcu.h"
> > +#include "sysemu/hostmem.h"
> > +#include "sysemu/numa.h"
> > +#include "hw/cxl/cxl.h"
> > +
> > +void cdat_add_dsmas(CXLCDAT *cdat, uint8_t dsmad_handle, uint8_t flags,
> > +                    uint64_t dpa_base, uint64_t dpa_length)
> > +{
> > +    struct cxl_cdat_dsmas *dsmas = g_malloc0(sizeof(*dsmas));
> > +    dsmas->dsmad_handle = dsmad_handle;
> > +    dsmas->flags = flags;
> > +    dsmas->dpa_base = dpa_base;
> > +    dsmas->dpa_length = dpa_length;
> > +    cdat->dsmas_list = g_list_append(cdat->dsmas_list, dsmas);
> > +}
> > +
> > +void cdat_add_dslbis(CXLCDAT *cdat, uint8_t handle, uint8_t flags,
> > +                     uint8_t data_type, uint64_t base_unit,
> > +                     uint16_t entry0, uint16_t entry1, uint16_t entry2)
> > +{
> > +    struct cxl_cdat_dslbis *dslbis = g_malloc0(sizeof(*dslbis));
> > +    dslbis->handle = handle;
> > +    dslbis->flags = flags;
> > +    dslbis->data_type = data_type;
> > +    dslbis->entry_base_unit = base_unit;
> > +    dslbis->entries[0] = entry0;
> > +    dslbis->entries[1] = entry1;
> > +    dslbis->entries[2] = entry2;
> > +    cdat->dslbis_list = g_list_append(cdat->dslbis_list, dslbis);
> > +}
> > +
> > +void cdat_add_dsmscis(CXLCDAT *cdat, uint8_t dsmas_handle,
> > +                      uint64_t memory_sc_size, uint64_t cache_attrs)
> > +{
> > +    struct cxl_cdat_dsmscis *dsmscis = g_malloc(sizeof(*dsmscis));
> > +    dsmscis->dsmas_handle = dsmas_handle;
> > +    dsmscis->memory_side_cache_size = memory_sc_size;
> > +    dsmscis->cache_attributes = cache_attrs;
> > +    cdat->dsmscis_list = g_list_append(cdat->dsmscis_list, dsmscis);
> > +}
> > +
> > +void cdat_add_dsis(CXLCDAT *cdat, uint8_t flags, uint8_t handle)
> > +{
> > +    struct cxl_cdat_dsis *dsis = g_malloc(sizeof(*dsis));
> > +    dsis->flags = flags;
> > +    dsis->handle = handle;
> > +    cdat->dsis_list = g_list_append(cdat->dsis_list, dsis);
> > +}
> > +
> > +void cdat_add_dsemts(CXLCDAT *cdat, uint8_t dsmas_handle,
> > +                     uint8_t efi_mem_type_attr, uint64_t dpa_offset,
> > +                     uint64_t dpa_length)
> > +{
> > +    struct cxl_cdat_dsemts *dsemts = g_malloc(sizeof(*dsemts));
> > +    dsemts->dsmas_handle = dsmas_handle;
> > +    dsemts->efi_mem_type_attr = efi_mem_type_attr;
> > +    dsemts->dpa_offset = dpa_offset;
> > +    dsemts->dpa_length = dpa_length;
> > +    cdat->dsemts_list = g_list_append(cdat->dsemts_list, dsemts);
> > +}
> > +
> > +struct cxl_cdat_sslbis *cdat_add_sslbis(CXLCDAT *cdat, uint8_t num_entries,
> > +                                        uint8_t data_type, uint64_t base_unit)
> > +{
> > +    struct cxl_cdat_sslbis *sslbis =
> > +        g_malloc(sizeof(*sslbis) + num_entries * sizeof(sslbis->entries[0]));
> > +    sslbis->num_entries = num_entries;
> > +    sslbis->data_type = data_type;
> > +    sslbis->base_unit = base_unit;
> > +    cdat->sslbis_list = g_list_append(cdat->sslbis_list, sslbis);
> > +    return sslbis;
> > +}
> > +
> > +int cdata_sslbis_set_entry(struct cxl_cdat_sslbis *sslbis, uint8_t index,
> > +                           uint16_t portx, uint16_t porty, uint16_t val)
> > +{
> > +    struct cxl_cdat_sslbis_entry *entry;
> > +    if (index >= sslbis->num_entries) {
> > +        return -1;
> > +    }
> > +    entry = &sslbis->entries[index];
> > +    entry->port_x_id = portx;
> > +    entry->port_y_id = porty;
> > +    entry->val = val;
> > +    return 0;
> > +}
> > +
> > +int cxl_table_access(PCIEDOE *doe, uint16_t vendor_id, uint8_t object_type,
> > +                     void *priv)
> > +{
> > +    uint8_t table_type;
> > +    int total_entries;
> > +    uint16_t entry_handle;
> > +    CXLCDAT *cdat = priv;
> > +    uint16_t next_entry;
> > +
> > +    if (doe->req_length != 3) {
> > +        /* optional error for unexpected command length */
> > +        return -1;
> > +    }
> > +
> > +    if ((doe->store[2] & CXL_DOE_TABLE_ACCESS_DW2_RCODE) != 0) {
> > +        /*
> > +         * Only table access code currently supported.
> > +         * Error indication is lack of Data Object Ready
> > +         */
> > +        return -1;
> > +    }
> > +
> > +    table_type = (doe->store[2] & CXL_DOE_TABLE_ACCESS_DW2_TYPE) >>
> > +        ctz32(CXL_DOE_TABLE_ACCESS_DW2_TYPE);
> > +    if (table_type != 0) {
> > +        /* Unsuported table ID so just don't set Data Object Ready */
> > +        return -1;
> > +    }
> > +    entry_handle = (doe->store[2] & CXL_DOE_TABLE_ACCESS_DW2_ENTRYHANDLE) >>
> > +        ctz32(CXL_DOE_TABLE_ACCESS_DW2_ENTRYHANDLE);
> > +
> > +    /* Assume entry handle == CDAT structure index */
> > +    total_entries = g_list_length(cdat->dsmas_list) +
> > +        g_list_length(cdat->dslbis_list) +
> > +        g_list_length(cdat->dsmscis_list) +
> > +        g_list_length(cdat->dsis_list) +
> > +        g_list_length(cdat->dsemts_list) +
> > +        g_list_length(cdat->sslbis_list);
> > +    if (entry_handle + 1 == total_entries) {
> > +        next_entry = 0xFFFF;
> > +    } else {
> > +        next_entry = entry_handle + 1;
> > +    }
> > +
> > +    if (entry_handle < g_list_length(cdat->dsmas_list)) {
> > +        const int dsmas_len = 24;
> > +        struct cxl_cdat_dsmas *dsmas =
> > +            g_list_nth_data(cdat->dsmas_list, entry_handle);
> > +
> > +        doe->store[1] = 3 + dsmas_len / sizeof(uint32_t);
> > +        doe->store[2] = next_entry << 16;
> > +        doe->store[3] = (dsmas_len << 16) | CXL_CDAT_DSMAS_TYPE;
> > +
> > +        /* cxl version of proximity domain */
> > +        doe->store[4] = dsmas->dsmad_handle;
> > +        /* flags in 2nd byte of [4] (bit 2 is non volatile) */
> > +        doe->store[4] |= (dsmas->flags << 8);
> > +        doe->store[5] = dsmas->dpa_base & 0xFFFFFFFF;
> > +        doe->store[6] = (dsmas->dpa_base >> 32) & 0xFFFFFFFF;
> > +        doe->store[7] = dsmas->dpa_length & 0xFFFFFFFF;
> > +        doe->store[8] = (dsmas->dpa_length >> 32) & 0xFFFFFFFF;
> > +        return 0;
> > +    }
> > +    entry_handle -= g_list_length(cdat->dsmas_list);
> > +    if (entry_handle < g_list_length(cdat->dslbis_list)) {
> > +        const int dslbis_len = 24;
> > +        struct cxl_cdat_dslbis *dslbis =
> > +            g_list_nth_data(cdat->dslbis_list, entry_handle);
> > +
> > +        doe->store[1] = 3 + dslbis_len / sizeof(uint32_t);
> > +        doe->store[2] = next_entry << 16;
> > +        doe->store[3] = (dslbis_len << 16) | CXL_CDAT_DSLBIS_TYPE;
> > +
> > +        doe->store[4] = (dslbis->data_type << 24) | (dslbis->flags << 8) |
> > +            dslbis->handle;
> > +        doe->store[5] = dslbis->entry_base_unit & 0xFFFFFFFF;
> > +        doe->store[6] = (dslbis->entry_base_unit >> 32) & 0xFFFFFFFF;
> > +        doe->store[7] = (dslbis->entries[1] << 16) | dslbis->entries[0];
> > +        doe->store[8] = dslbis->entries[2];
> > +        return 0;
> > +    }
> > +    entry_handle -= g_list_length(cdat->dslbis_list);
> > +    if (entry_handle < g_list_length(cdat->dsmscis_list)) {
> > +        const int dsmscis_len = 20;
> > +        struct cxl_cdat_dsmscis *dsmscis =
> > +            g_list_nth_data(cdat->dsmscis_list, entry_handle);
> > +
> > +        doe->store[1] = 3 + dsmscis_len / sizeof(uint32_t);
> > +        doe->store[2] = next_entry << 16;
> > +        doe->store[3] = (dsmscis_len << 16) | CXL_CDAT_DSMSCIS_TYPE;
> > +        doe->store[4] = dsmscis->dsmas_handle;
> > +        doe->store[5] = dsmscis->memory_side_cache_size & 0xffffffff;
> > +        doe->store[6] = (dsmscis->memory_side_cache_size >> 32) & 0xffffffff;
> > +        doe->store[7] = dsmscis->cache_attributes;
> > +    }
> > +    entry_handle -= g_list_length(cdat->dsmscis_list);
> > +    if (entry_handle < g_list_length(cdat->dsis_list)) {
> > +        const int dsis_len = 8;
> > +        struct cxl_cdat_dsis *dsis =
> > +            g_list_nth_data(cdat->dsis_list, entry_handle);
> > +
> > +        doe->store[1] = 3 + dsis_len / sizeof(uint32_t);
> > +        doe->store[2] = next_entry << 16;
> > +        doe->store[3] = (dsis_len << 16) | CXL_CDAT_DSMSCIS_TYPE;
> > +        doe->store[4] = (dsis->handle << 8) | dsis->flags;
> > +    }
> > +    entry_handle -= g_list_length(cdat->dsis_list);
> > +    if (entry_handle < g_list_length(cdat->dsemts_list)) {
> > +        const int dsemts_len = 24;
> > +        struct cxl_cdat_dsemts *dsemts =
> > +            g_list_nth_data(cdat->dsemts_list, entry_handle);
> > +
> > +        doe->store[1] = 3 + dsemts_len / sizeof(uint32_t);
> > +        doe->store[2] = next_entry << 16;
> > +        doe->store[3] = (dsemts_len << 16) | CXL_CDAT_DSEMTS_TYPE;
> > +        doe->store[4] = (dsemts->efi_mem_type_attr << 8) | dsemts->dsmas_handle;
> > +        doe->store[5] = dsemts->dpa_offset & 0xffffffff;
> > +        doe->store[6] = (dsemts->dpa_offset >> 32) & 0xffffffff;
> > +        doe->store[7] = dsemts->dpa_length & 0xffffffff;
> > +        doe->store[8] = (dsemts->dpa_length >> 32) & 0xffffffff;
> > +    }
> > +    entry_handle -= g_list_length(cdat->dsemts_list);
> > +    if (entry_handle < g_list_length(cdat->sslbis_list)) {
> > +        struct cxl_cdat_sslbis *sslbis =
> > +            g_list_nth_data(cdat->sslbis_list, entry_handle);
> > +        int sslbis_len = 16 + 8 * sslbis->num_entries;
> > +        int i;
> > +
> > +        doe->store[1] = 3 + sslbis_len / sizeof(uint32_t);
> > +        doe->store[2] = next_entry << 16;
> > +        doe->store[3] = (sslbis_len << 16) | CXL_CDAT_SSLBIS_TYPE;
> > +        doe->store[4] = sslbis->data_type;
> > +        doe->store[5] = sslbis->base_unit & 0xffffffff;
> > +        doe->store[6] = (sslbis->base_unit >> 32) & 0xffffffff;
> > +        for (i = 0; i < sslbis->num_entries; i++) {
> > +            doe->store[7 + i * 2] = (sslbis->entries[i].port_y_id << 8) |
> > +                sslbis->entries[i].port_x_id;
> > +            doe->store[7 + i * 2 + 1] = sslbis->entries[i].val;
> > +        }
> > +    }
> > +
> > +    return -1;
> > +}
> > diff --git a/hw/cxl/meson.build b/hw/cxl/meson.build
> > index 0eca715d10..9e2e5f4094 100644
> > --- a/hw/cxl/meson.build
> > +++ b/hw/cxl/meson.build
> > @@ -2,4 +2,5 @@ softmmu_ss.add(when: 'CONFIG_CXL', if_true: files(
> >    'cxl-component-utils.c',
> >    'cxl-device-utils.c',
> >    'cxl-mailbox-utils.c',
> > +  'cxl-cdat.c',
> >  ))
> > diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
> > new file mode 100644
> > index 0000000000..d0226c463c
> > --- /dev/null
> > +++ b/include/hw/cxl/cxl_cdat.h
> > @@ -0,0 +1,101 @@
> > +/*
> > + * Support for CDAT entires as defined in
> > + * Coherent Device Attribute Table (CDAT) Specification rev 1.02
> > + * Available from uefi.org.
> > + *
> > + * Copyright (c) 2021 Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#ifndef QEMU_CXL_CDAT_H
> > +#define QEMU_CXL_CDAT_H
> > +#include "hw/pci/doe.h"
> > +/* DW2 is common to the request and response */
> > +#define CXL_DOE_TABLE_ACCESS_DW2_RCODE          0x000000ff
> > +#define CXL_DOE_TABLE_ACCESS_DW2_TYPE           0x0000ff00
> > +#define CXL_DOE_TABLE_ACCESS_DW2_ENTRYHANDLE    0xffff0000
> > +
> > +#define CXL_CDAT_DSMAS_TYPE     0
> > +#define CXL_CDAT_DSLBIS_TYPE    1
> > +#define CXL_CDAT_DSMSCIS_TYPE   2
> > +#define CXL_CDAT_DSIS_TYPE      3
> > +#define CXL_CDAT_DSEMTS_TYPE    4
> > +#define CXL_CDAT_SSLBIS_TYPE    5
> > +
> > +struct cxl_cdat_dsmas {
> > +    uint8_t dsmad_handle;
> > +    uint8_t flags;
> > +#define CDAT_DSMAS_FLAG_NV (1 << 2)
> > +    uint64_t dpa_base;
> > +    uint64_t dpa_length;
> > +};
> > +
> > +struct cxl_cdat_dslbis {
> > +    uint8_t handle;
> > +    uint8_t flags;
> > +    uint8_t data_type;
> > +    uint64_t entry_base_unit;
> > +    uint16_t entries[3]; /* 6 bytes */
> > +};
> > +
> > +struct cxl_cdat_dsmscis {
> > +    uint8_t dsmas_handle;
> > +    uint64_t memory_side_cache_size;
> > +    uint32_t cache_attributes;
> > +};
> > +
> > +struct cxl_cdat_dsis {
> > +    uint8_t flags;
> > +#define CDAT_DSIS_MEMORY_ATTACHED 0x01
> > +    uint8_t handle;
> > +};
> > +
> > +struct cxl_cdat_dsemts {
> > +    uint8_t dsmas_handle;
> > +    uint8_t efi_mem_type_attr;
> > +    uint64_t dpa_offset;
> > +    uint64_t dpa_length;
> > +};
> > +
> > +struct cxl_cdat_sslbis_entry {
> > +    uint16_t port_x_id;
> > +    uint16_t port_y_id;
> > +    uint16_t val;
> > +};
> > +
> > +struct cxl_cdat_sslbis {
> > +    uint8_t num_entries; /* needed to compute length */
> > +    uint8_t data_type;
> > +    uint64_t base_unit;
> > +    struct cxl_cdat_sslbis_entry entries[];
> > +};
> > +
> > +typedef struct cxl_cdat {
> > +    GList *dsmas_list;
> > +    GList *dslbis_list;
> > +    GList *dsmscis_list;
> > +    GList *dsis_list;
> > +    GList *dsemts_list;
> > +    GList *sslbis_list;
> > +} CXLCDAT;
> > +
> > +void cdat_add_dsmas(CXLCDAT *cdat, uint8_t dsmad_handle, uint8_t flags,
> > +                    uint64_t dpa_base, uint64_t dpa_length);
> > +void cdat_add_dslbis(CXLCDAT *cdat, uint8_t handle, uint8_t flags,
> > +                     uint8_t data_type, uint64_t base_unit,
> > +                     uint16_t entry0, uint16_t entry1, uint16_t entry2);
> > +void cdat_add_dsmscis(CXLCDAT *cdat, uint8_t dsmas_handle,
> > +                      uint64_t memory_sc_size, uint64_t cache_attrs);
> > +void cdat_add_dsis(CXLCDAT *cdat, uint8_t flags, uint8_t handle);
> > +void cdat_add_dsemts(CXLCDAT *cdat, uint8_t dsmas_handle,
> > +                     uint8_t efi_mem_type_attr, uint64_t dpa_offset,
> > +                     uint64_t dpa_length);
> > +struct cxl_cdat_sslbis *cdat_add_sslbis(CXLCDAT *cdat, uint8_t num_entries,
> > +                                        uint8_t data_type, uint64_t base_unit);
> > +int cdata_sslbis_set_entry(struct cxl_cdat_sslbis *sslbis, uint8_t index,
> > +                           uint16_t portx, uint16_t pory, uint16_t val);
> > +
> > +int cxl_table_access(PCIEDOE *doe, uint16_t vendor_id, uint8_t object_type,
> > +                     void *priv);
> > +#endif
> > -- 
> > 2.19.1
> > 
> >   
> _______________________________________________
> Linuxarm mailing list -- linuxarm@openeuler.org
> To unsubscribe send an email to linuxarm-leave@openeuler.org