Message ID | 20221013120009.15541-5-Jonathan.Cameron@huawei.com |
---|---|
State | Superseded |
Headers | show |
Series | QEMU PCIe DOE for PCIe 4.0/5.0 and CXL 2. | expand |
Other than the nitpicks below, lgtm. Not sure if you need a sign off from me given the contributions: Signed-off-by: Gregory Price <gregory.price@memverge.com> > +/* If no cdat_table == NULL returns number of entries */ > +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, > + int dsmad_handle, MemoryRegion *mr) > +{ > + enum { > + DSMAS, > + DSLBIS0, > + DSLBIS1, > + DSLBIS2, > + DSLBIS3, > + DSEMTS, > + NUM_ENTRIES > + }; // ... > + if (!cdat_table) { > + return NUM_ENTRIES; > + } the only thing that i would recommend is making this enum global (maybe renaming them CT3_CDAT_[ENTRY_NAME]) and using CT3_CDAT_NUM_ENTRIES directly in the function that allocates the cdat buffer itself. I do understand the want to keep the enum and the code creating the entries co-located though, so i'm just nitpicking here i guess. Generally I dislike the pattern of passing a NULL into a function to get configuration data, and then calling that function again. This function wants to be named... ct3_build_cdat_entries_for_mr_or_get_table_size_if_cdat_is_null(...) to accurately describe what it does. Just kinda feels like an extra function call for no reason. But either way, it works, this is just a nitpick on my side. > +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) > +{ > + g_autofree CDATSubHeader **table = NULL; > + CXLType3Dev *ct3d = priv; > + MemoryRegion *volatile_mr; > + /* ... snip ... */ > +} s/volatile/nonvolatile
On Thu, Oct 13, 2022 at 12:26:58PM -0400, Gregory Price wrote: > Other than the nitpicks below, lgtm. Not sure if you need a sign off > from me given the contributions: > > Signed-off-by: Gregory Price <gregory.price@memverge.com> > > > +/* If no cdat_table == NULL returns number of entries */ > > +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, > > + int dsmad_handle, MemoryRegion *mr) > > +{ > > + enum { > > + DSMAS, > > + DSLBIS0, > > + DSLBIS1, > > + DSLBIS2, > > + DSLBIS3, > > + DSEMTS, > > + NUM_ENTRIES > > + }; > // ... > > + if (!cdat_table) { > > + return NUM_ENTRIES; > > + } > > the only thing that i would recommend is making this enum global (maybe > renaming them CT3_CDAT_[ENTRY_NAME]) and using CT3_CDAT_NUM_ENTRIES > directly in the function that allocates the cdat buffer itself. Yes I think I agree here. > I do > understand the want to keep the enum and the code creating the entries > co-located though, so i'm just nitpicking here i guess. > > Generally I dislike the pattern of passing a NULL into a function to get > configuration data, and then calling that function again. This function > wants to be named... > > ct3_build_cdat_entries_for_mr_or_get_table_size_if_cdat_is_null(...) > > to accurately describe what it does. Just kinda feels like an extra > function call for no reason. > > But either way, it works, this is just a nitpick on my side. > > > +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) > > +{ > > + g_autofree CDATSubHeader **table = NULL; > > + CXLType3Dev *ct3d = priv; > > + MemoryRegion *volatile_mr; > > + /* ... snip ... */ > > +} > > s/volatile/nonvolatile
On Thu, 13 Oct 2022 12:26:58 -0400 Gregory Price <gregory.price@memverge.com> wrote: > Other than the nitpicks below, lgtm. Not sure if you need a sign off > from me given the contributions: > > Signed-off-by: Gregory Price <gregory.price@memverge.com> > > > +/* If no cdat_table == NULL returns number of entries */ > > +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, > > + int dsmad_handle, MemoryRegion *mr) > > +{ > > + enum { > > + DSMAS, > > + DSLBIS0, > > + DSLBIS1, > > + DSLBIS2, > > + DSLBIS3, > > + DSEMTS, > > + NUM_ENTRIES > > + }; > // ... > > + if (!cdat_table) { > > + return NUM_ENTRIES; > > + } > > the only thing that i would recommend is making this enum global (maybe > renaming them CT3_CDAT_[ENTRY_NAME]) and using CT3_CDAT_NUM_ENTRIES > directly in the function that allocates the cdat buffer itself. I do > understand the want to keep the enum and the code creating the entries > co-located though, so i'm just nitpicking here i guess. > > Generally I dislike the pattern of passing a NULL into a function to get > configuration data, and then calling that function again. This function > wants to be named... > > ct3_build_cdat_entries_for_mr_or_get_table_size_if_cdat_is_null(...) > I agree it's a messy pattern, but as things become dynamic I'd expect we'll have a bunch of checks that really need to be in that function and would be replicated at the caller to calculate the size of the array. I'm expecting this code to get more complex over time. For example adding memory-side cache support based on commandline parameters. Once we do that, the enum will almost certainly no longer be global as we'll have a bunch of other entries (potentially different numbers for each region). Whether that is done with calls to a separate function, or by changing this one isn't clear to me yet. > to accurately describe what it does. Just kinda feels like an extra > function call for no reason. > > But either way, it works, this is just a nitpick on my side. > > > +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) > > +{ > > + g_autofree CDATSubHeader **table = NULL; > > + CXLType3Dev *ct3d = priv; > > + MemoryRegion *volatile_mr; > > + /* ... snip ... */ > > +} > > s/volatile/nonvolatile Doh. I'll update the gitlab tree in a minute for this, but leave sending out the updated version for a few days to let others take a look if they wish. In meantime, regression in mainline kernel against CXL qemu emulation. Bisection with 8 steps to go. *sigh* Jonathan
On Thu, 13 Oct 2022 13:09:26 -0400 "Michael S. Tsirkin" <mst@redhat.com> wrote: > On Thu, Oct 13, 2022 at 12:26:58PM -0400, Gregory Price wrote: > > Other than the nitpicks below, lgtm. Not sure if you need a sign off > > from me given the contributions: > > > > Signed-off-by: Gregory Price <gregory.price@memverge.com> > > > > > +/* If no cdat_table == NULL returns number of entries */ > > > +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, > > > + int dsmad_handle, MemoryRegion *mr) > > > +{ > > > + enum { > > > + DSMAS, > > > + DSLBIS0, > > > + DSLBIS1, > > > + DSLBIS2, > > > + DSLBIS3, > > > + DSEMTS, > > > + NUM_ENTRIES > > > + }; > > // ... > > > + if (!cdat_table) { > > > + return NUM_ENTRIES; > > > + } > > > > the only thing that i would recommend is making this enum global (maybe > > renaming them CT3_CDAT_[ENTRY_NAME]) and using CT3_CDAT_NUM_ENTRIES > > directly in the function that allocates the cdat buffer itself. > > > Yes I think I agree here. Ok, seems a consensus against having this local. I can do this for now and then revisit if / when things become more complex and this becomes not global. I guess a potential case of premature flexibility. Jonathan > > > I do > > understand the want to keep the enum and the code creating the entries > > co-located though, so i'm just nitpicking here i guess. > > > > Generally I dislike the pattern of passing a NULL into a function to get > > configuration data, and then calling that function again. This function > > wants to be named... > > > > ct3_build_cdat_entries_for_mr_or_get_table_size_if_cdat_is_null(...) > > > > to accurately describe what it does. Just kinda feels like an extra > > function call for no reason. > > > > But either way, it works, this is just a nitpick on my side. > > > > > +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) > > > +{ > > > + g_autofree CDATSubHeader **table = NULL; > > > + CXLType3Dev *ct3d = priv; > > > + MemoryRegion *volatile_mr; > > > + /* ... snip ... */ > > > +} > > > > s/volatile/nonvolatile >
If you're expecting this to be dynamic in size in the future, then maybe what we really want here is a function static int ct3_get_cdat_size(CXLType3Dev ct3d) { /* TODO: Dynamic sizing calculations */ return CT3_CDAT_NUM_ENTRIES; } On Thu, Oct 13, 2022 at 06:21:44PM +0100, Jonathan Cameron wrote: > On Thu, 13 Oct 2022 13:09:26 -0400 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Thu, Oct 13, 2022 at 12:26:58PM -0400, Gregory Price wrote: > > > Other than the nitpicks below, lgtm. Not sure if you need a sign off > > > from me given the contributions: > > > > > > Signed-off-by: Gregory Price <gregory.price@memverge.com> > > > > > > > +/* If no cdat_table == NULL returns number of entries */ > > > > +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, > > > > + int dsmad_handle, MemoryRegion *mr) > > > > +{ > > > > + enum { > > > > + DSMAS, > > > > + DSLBIS0, > > > > + DSLBIS1, > > > > + DSLBIS2, > > > > + DSLBIS3, > > > > + DSEMTS, > > > > + NUM_ENTRIES > > > > + }; > > > // ... > > > > + if (!cdat_table) { > > > > + return NUM_ENTRIES; > > > > + } > > > > > > the only thing that i would recommend is making this enum global (maybe > > > renaming them CT3_CDAT_[ENTRY_NAME]) and using CT3_CDAT_NUM_ENTRIES > > > directly in the function that allocates the cdat buffer itself. > > > > > > Yes I think I agree here. > > Ok, seems a consensus against having this local. > > I can do this for now and then revisit if / when things become more complex > and this becomes not global. I guess a potential case of premature flexibility. > > Jonathan > > > > > > I do > > > understand the want to keep the enum and the code creating the entries > > > co-located though, so i'm just nitpicking here i guess. > > > > > > Generally I dislike the pattern of passing a NULL into a function to get > > > configuration data, and then calling that function again. This function > > > wants to be named... > > > > > > ct3_build_cdat_entries_for_mr_or_get_table_size_if_cdat_is_null(...) > > > > > > to accurately describe what it does. Just kinda feels like an extra > > > function call for no reason. > > > > > > But either way, it works, this is just a nitpick on my side. > > > > > > > +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) > > > > +{ > > > > + g_autofree CDATSubHeader **table = NULL; > > > > + CXLType3Dev *ct3d = priv; > > > > + MemoryRegion *volatile_mr; > > > > + /* ... snip ... */ > > > > +} > > > > > > s/volatile/nonvolatile > > >
On Thu, 13 Oct 2022 13:32:26 -0400 Gregory Price <gregory.price@memverge.com> wrote: > If you're expecting this to be dynamic in size in the future, then maybe > what we really want here is a function > > static int ct3_get_cdat_size(CXLType3Dev ct3d) { > /* TODO: Dynamic sizing calculations */ > return CT3_CDAT_NUM_ENTRIES; > } Perhaps, though that suffers from lack of locality and clearly flagging that the enum is definitely not global. Anyhow, lets argue that mechanism when it's needed :) Jonathan > > > On Thu, Oct 13, 2022 at 06:21:44PM +0100, Jonathan Cameron wrote: > > On Thu, 13 Oct 2022 13:09:26 -0400 > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > On Thu, Oct 13, 2022 at 12:26:58PM -0400, Gregory Price wrote: > > > > Other than the nitpicks below, lgtm. Not sure if you need a sign off > > > > from me given the contributions: > > > > > > > > Signed-off-by: Gregory Price <gregory.price@memverge.com> > > > > > > > > > +/* If no cdat_table == NULL returns number of entries */ > > > > > +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, > > > > > + int dsmad_handle, MemoryRegion *mr) > > > > > +{ > > > > > + enum { > > > > > + DSMAS, > > > > > + DSLBIS0, > > > > > + DSLBIS1, > > > > > + DSLBIS2, > > > > > + DSLBIS3, > > > > > + DSEMTS, > > > > > + NUM_ENTRIES > > > > > + }; > > > > // ... > > > > > + if (!cdat_table) { > > > > > + return NUM_ENTRIES; > > > > > + } > > > > > > > > the only thing that i would recommend is making this enum global (maybe > > > > renaming them CT3_CDAT_[ENTRY_NAME]) and using CT3_CDAT_NUM_ENTRIES > > > > directly in the function that allocates the cdat buffer itself. > > > > > > > > > Yes I think I agree here. > > > > Ok, seems a consensus against having this local. > > > > I can do this for now and then revisit if / when things become more complex > > and this becomes not global. I guess a potential case of premature flexibility. > > > > Jonathan > > > > > > > > > I do > > > > understand the want to keep the enum and the code creating the entries > > > > co-located though, so i'm just nitpicking here i guess. > > > > > > > > Generally I dislike the pattern of passing a NULL into a function to get > > > > configuration data, and then calling that function again. This function > > > > wants to be named... > > > > > > > > ct3_build_cdat_entries_for_mr_or_get_table_size_if_cdat_is_null(...) > > > > > > > > to accurately describe what it does. Just kinda feels like an extra > > > > function call for no reason. > > > > > > > > But either way, it works, this is just a nitpick on my side. > > > > > > > > > +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) > > > > > +{ > > > > > + g_autofree CDATSubHeader **table = NULL; > > > > > + CXLType3Dev *ct3d = priv; > > > > > + MemoryRegion *volatile_mr; > > > > > + /* ... snip ... */ > > > > > +} > > > > > > > > s/volatile/nonvolatile > > > > >
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c index 568c9d62f5..8490154824 100644 --- a/hw/mem/cxl_type3.c +++ b/hw/mem/cxl_type3.c @@ -12,9 +12,258 @@ #include "qemu/range.h" #include "qemu/rcu.h" #include "sysemu/hostmem.h" +#include "sysemu/numa.h" #include "hw/cxl/cxl.h" #include "hw/pci/msix.h" +#define DWORD_BYTE 4 + +/* If no cdat_table == NULL returns number of entries */ +static int ct3_build_cdat_entries_for_mr(CDATSubHeader **cdat_table, + int dsmad_handle, MemoryRegion *mr) +{ + enum { + DSMAS, + DSLBIS0, + DSLBIS1, + DSLBIS2, + DSLBIS3, + DSEMTS, + NUM_ENTRIES + }; + g_autofree CDATDsmas *dsmas = NULL; + g_autofree CDATDslbis *dslbis0 = NULL; + g_autofree CDATDslbis *dslbis1 = NULL; + g_autofree CDATDslbis *dslbis2 = NULL; + g_autofree CDATDslbis *dslbis3 = NULL; + g_autofree CDATDsemts *dsemts = NULL; + + if (!cdat_table) { + return NUM_ENTRIES; + } + + dsmas = g_malloc(sizeof(*dsmas)); + if (!dsmas) { + return -ENOMEM; + } + *dsmas = (CDATDsmas) { + .header = { + .type = CDAT_TYPE_DSMAS, + .length = sizeof(*dsmas), + }, + .DSMADhandle = dsmad_handle, + .flags = CDAT_DSMAS_FLAG_NV, + .DPA_base = 0, + .DPA_length = int128_get64(mr->size), + }; + + /* For now, no memory side cache, plausiblish numbers */ + dslbis0 = g_malloc(sizeof(*dslbis0)); + if (!dslbis0) { + return -ENOMEM; + } + *dslbis0 = (CDATDslbis) { + .header = { + .type = CDAT_TYPE_DSLBIS, + .length = sizeof(*dslbis0), + }, + .handle = dsmad_handle, + .flags = HMAT_LB_MEM_MEMORY, + .data_type = HMAT_LB_DATA_READ_LATENCY, + .entry_base_unit = 10000, /* 10ns base */ + .entry[0] = 15, /* 150ns */ + }; + + dslbis1 = g_malloc(sizeof(*dslbis1)); + if (!dslbis1) { + return -ENOMEM; + } + *dslbis1 = (CDATDslbis) { + .header = { + .type = CDAT_TYPE_DSLBIS, + .length = sizeof(*dslbis1), + }, + .handle = dsmad_handle, + .flags = HMAT_LB_MEM_MEMORY, + .data_type = HMAT_LB_DATA_WRITE_LATENCY, + .entry_base_unit = 10000, + .entry[0] = 25, /* 250ns */ + }; + + dslbis2 = g_malloc(sizeof(*dslbis2)); + if (!dslbis2) { + return -ENOMEM; + } + *dslbis2 = (CDATDslbis) { + .header = { + .type = CDAT_TYPE_DSLBIS, + .length = sizeof(*dslbis2), + }, + .handle = dsmad_handle, + .flags = HMAT_LB_MEM_MEMORY, + .data_type = HMAT_LB_DATA_READ_BANDWIDTH, + .entry_base_unit = 1000, /* GB/s */ + .entry[0] = 16, + }; + + dslbis3 = g_malloc(sizeof(*dslbis3)); + if (!dslbis3) { + return -ENOMEM; + } + *dslbis3 = (CDATDslbis) { + .header = { + .type = CDAT_TYPE_DSLBIS, + .length = sizeof(*dslbis3), + }, + .handle = dsmad_handle, + .flags = HMAT_LB_MEM_MEMORY, + .data_type = HMAT_LB_DATA_WRITE_BANDWIDTH, + .entry_base_unit = 1000, /* GB/s */ + .entry[0] = 16, + }; + + dsemts = g_malloc(sizeof(*dsemts)); + if (!dsemts) { + return -ENOMEM; + } + *dsemts = (CDATDsemts) { + .header = { + .type = CDAT_TYPE_DSEMTS, + .length = sizeof(*dsemts), + }, + .DSMAS_handle = dsmad_handle, + /* Reserved - the non volatile from DSMAS matters */ + .EFI_memory_type_attr = 2, + .DPA_offset = 0, + .DPA_length = int128_get64(mr->size), + }; + + /* Header always at start of structure */ + cdat_table[DSMAS] = g_steal_pointer(&dsmas); + cdat_table[DSLBIS0] = g_steal_pointer(&dslbis0); + cdat_table[DSLBIS1] = g_steal_pointer(&dslbis1); + cdat_table[DSLBIS2] = g_steal_pointer(&dslbis2); + cdat_table[DSLBIS3] = g_steal_pointer(&dslbis3); + cdat_table[DSEMTS] = g_steal_pointer(&dsemts); + + return NUM_ENTRIES; +} + +static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv) +{ + g_autofree CDATSubHeader **table = NULL; + CXLType3Dev *ct3d = priv; + MemoryRegion *volatile_mr; + int dsmad_handle = 0; + int len = 0; + int rc; + + if (!ct3d->hostmem) { + return 0; + } + + volatile_mr = host_memory_backend_get_memory(ct3d->hostmem); + if (!volatile_mr) { + return -EINVAL; + } + + /* How many entries needed for non volatile mr */ + rc = ct3_build_cdat_entries_for_mr(NULL, dsmad_handle, volatile_mr); + if (rc < 0) { + return rc; + } + len = rc; + + table = g_malloc0(len * sizeof(*table)); + if (!table) { + return -ENOMEM; + } + + /* Now fill them in */ + rc = ct3_build_cdat_entries_for_mr(table, dsmad_handle++, volatile_mr); + if (rc < 0) { + return rc; + } + + *cdat_table = g_steal_pointer(&table); + + return len; +} + +static void ct3_free_cdat_table(CDATSubHeader **cdat_table, int num, void *priv) +{ + int i; + + for (i = 0; i < num; i++) { + g_free(cdat_table[i]); + } + g_free(cdat_table); +} + +static bool cxl_doe_cdat_rsp(DOECap *doe_cap) +{ + CDATObject *cdat = &CXL_TYPE3(doe_cap->pdev)->cxl_cstate.cdat; + uint16_t ent; + void *base; + uint32_t len; + CDATReq *req = pcie_doe_get_write_mbox_ptr(doe_cap); + CDATRsp rsp; + + assert(cdat->entry_len); + + /* Discard if request length mismatched */ + if (pcie_doe_get_obj_len(req) < + DIV_ROUND_UP(sizeof(CDATReq), DWORD_BYTE)) { + return false; + } + + ent = req->entry_handle; + base = cdat->entry[ent].base; + len = cdat->entry[ent].length; + + rsp = (CDATRsp) { + .header = { + .vendor_id = CXL_VENDOR_ID, + .data_obj_type = CXL_DOE_TABLE_ACCESS, + .reserved = 0x0, + .length = DIV_ROUND_UP((sizeof(rsp) + len), DWORD_BYTE), + }, + .rsp_code = CXL_DOE_TAB_RSP, + .table_type = CXL_DOE_TAB_TYPE_CDAT, + .entry_handle = (ent < cdat->entry_len - 1) ? + ent + 1 : CXL_DOE_TAB_ENT_MAX, + }; + + memcpy(doe_cap->read_mbox, &rsp, sizeof(rsp)); + memcpy(doe_cap->read_mbox + DIV_ROUND_UP(sizeof(rsp), DWORD_BYTE), + base, len); + + doe_cap->read_mbox_len += rsp.header.length; + + return true; +} + +static uint32_t ct3d_config_read(PCIDevice *pci_dev, uint32_t addr, int size) +{ + CXLType3Dev *ct3d = CXL_TYPE3(pci_dev); + uint32_t val; + + if (pcie_doe_read_config(&ct3d->doe_cdat, addr, size, &val)) { + return val; + } + + return pci_default_read_config(pci_dev, addr, size); +} + +static void ct3d_config_write(PCIDevice *pci_dev, uint32_t addr, uint32_t val, + int size) +{ + CXLType3Dev *ct3d = CXL_TYPE3(pci_dev); + + pcie_doe_write_config(&ct3d->doe_cdat, addr, val, size); + pci_default_write_config(pci_dev, addr, val, size); +} + /* * Null value of all Fs suggested by IEEE RA guidelines for use of * EU, OUI and CID @@ -140,6 +389,11 @@ static bool cxl_setup_memory(CXLType3Dev *ct3d, Error **errp) return true; } +static DOEProtocol doe_cdat_prot[] = { + { CXL_VENDOR_ID, CXL_DOE_TABLE_ACCESS, cxl_doe_cdat_rsp }, + { } +}; + static void ct3_realize(PCIDevice *pci_dev, Error **errp) { CXLType3Dev *ct3d = CXL_TYPE3(pci_dev); @@ -189,6 +443,14 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp) for (i = 0; i < msix_num; i++) { msix_vector_use(pci_dev, i); } + + /* DOE Initailization */ + pcie_doe_init(pci_dev, &ct3d->doe_cdat, 0x190, doe_cdat_prot, true, 0); + + cxl_cstate->cdat.build_cdat_table = ct3_build_cdat_table; + cxl_cstate->cdat.free_cdat_table = ct3_free_cdat_table; + cxl_cstate->cdat.private = ct3d; + cxl_doe_cdat_init(cxl_cstate, errp); } static void ct3_exit(PCIDevice *pci_dev) @@ -197,6 +459,7 @@ static void ct3_exit(PCIDevice *pci_dev) CXLComponentState *cxl_cstate = &ct3d->cxl_cstate; ComponentRegisters *regs = &cxl_cstate->crb; + cxl_doe_cdat_release(cxl_cstate); g_free(regs->special_ops); address_space_destroy(&ct3d->hostmem_as); } @@ -296,6 +559,7 @@ static Property ct3_props[] = { DEFINE_PROP_LINK("lsa", CXLType3Dev, lsa, TYPE_MEMORY_BACKEND, HostMemoryBackend *), DEFINE_PROP_UINT64("sn", CXLType3Dev, sn, UI64_NULL), + DEFINE_PROP_STRING("cdat", CXLType3Dev, cxl_cstate.cdat.filename), DEFINE_PROP_END_OF_LIST(), }; @@ -361,6 +625,9 @@ static void ct3_class_init(ObjectClass *oc, void *data) pc->device_id = 0xd93; /* LVF for now */ pc->revision = 1; + pc->config_write = ct3d_config_write; + pc->config_read = ct3d_config_read; + set_bit(DEVICE_CATEGORY_STORAGE, dc->categories); dc->desc = "CXL PMEM Device (Type 3)"; dc->reset = ct3d_reset;