QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0
@ 2021-03-09 20:33 Chris Browy
  2021-03-09 21:03 ` [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support Chris Browy
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chris Browy @ 2021-03-09 20:33 UTC (permalink / raw)
  To: qemu-devel, linux-cxl
  Cc: ben.widawsky, david, vishal.l.verma, jgroves, cbrowy, armbru,
	f4bug, mst, tyshao, hchkuo, Jonathan.Cameron, imammedo,
	dan.j.williams, ira.weiny

Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 implements
all planned functionality.

Based on QEMU version:
https://gitlab.com/bwidawsk/qemu/-/tree/cxl-2.0v4

Summary:
1: PCIe DOE support for Discovery
   - Support multiple DOE instances for each own protocol set 
   - MSI-X and polling supported
   - Update error and interrupt status in DOE Status register
   - Use static array to register callback function for DOE protocols 
   - Deprecate DOE_SUCCESS and DOE_DISCARD
   - Add license headers
2: CXL DOE for CDAT and Compliance Mode.
   - Device supports pre-defined CDAT or user-provided CDAT.
   - Support on reading the iASL aml file via specifying
     "cdat=<filename.aml>" property to -device cxl-type3
	 skips over the ACPI header and writes only CDAT table entries
   - Clean up CXL compliance structures
   - DOE CDAT response returns one CDAT Structure instance based on
     request EntryHandle value.

Example cdat.dat file: (compile with iasl -G cdat.dat)
CDAT file may contain any mix and number of supported CDAT Structure types
----------------------
/* Header */ 
Signature : "CDAT"
Table Length : 00000000
Revision : 01
Checksum : 00
Oem ID : AVERY 
Oem Table ID : 0 
Oem Revision : 1 
Asl Compiler ID : "INTL"
Asl Compiler Revision : 20160527

/* CDAT structures */
Label : DSMAS               // Field        Byte Length
UINT8  : 0                  // Type             1
UINT8  : 00                 // Reserved         1
UINT16 : 0018               // Length           2
UINT8  : 00                 // DSMADHandle      1
UINT8  : 00                 // Flags            1
UINT16 : 0000               // Reserved         2
UINT64 : 0000000000000000   // DPA Base         8
UINT64 : ffffffffffffffff   // DPA Length       8

Label : DSLBIS              // Field          Byte Length
UINT8  : 01                 // Type             1
UINT8  : 00                 // Reserved         1
UINT16 : 0018               // Length           2
UINT8  : 00                 // Handle           1
UINT8  : 00                 // Flags            1
UINT8  : 00                 // Data Type        1
UINT8  : 00                 // Reserved         1
UINT64 : 0000000000000000   // Entry Base Unit  8
UINT16 : 0000               // Entry[0]         2
UINT16 : 0000               // Entry[1]         2
UINT16 : 0000               // Entry[2]         2
UINT16 : 0000               // Reserved         2

Label: DSMSCIS              // Field        Byte Length
UINT8  : 02                 // Type             1
UINT8  : 00                 // Reserved         1
UINT16 : 0014               // Length           2
UINT8  : 00                 // DSMASHandle      1
UINT24 : 000000             // Reserved         3
UINT64 : 0000000000000000   // Memory Side Cache Size    8
UINT32 : 00000000           // Cache Attributes 4 

Label : DSIS                // Field        Byte Length
UINT8  : 03                 // Type             1
UINT8  : 00                 // Reserved         1
UINT16 : 0008               // Length           2
UINT8  : 00                 // Flags            1
UINT8  : 00                 // Handle           1
UINT16 : 0000               // Reserved         2

Label : DSEMTS              // Field        Byte Length
UINT8  : 04                 // Type             1
UINT8  : 00                 // Reserved         1
UINT16 : 0018               // Length           2
UINT8  : 00                 // DSMAS Handle     1
UINT8  : 00                 // EFI Memory Type and Attribute    1
UINT16 : 0000               // Reserved         2
UINT64 : 0000000000000000   // DPA Offset       8
UINT64 : 0000000000000000   // DPA Length       8

Label : SSLBIS              // Field        Byte Length
UINT8  : 05                 // Type             1
UINT8  : 00                 // Reserved         1
UINT16 : 0020               // Length           2
UINT8  : 00                 // Data Type        1
UINT24 : 000000             // Reserved         3
UINT64 : 0000000000000000   // Entry Base Unit  8
Label : SSLBE               // SSLBE[0]
UINT16 : 0000               // Port X ID        2
UINT16 : 0000               // Port Y ID        2
UINT16 : 0000               // Latency or Bandwidth    2
UINT16 : BBBB               // Reserved         2
Label : SSLBE               // SSLBE[1]
UINT16 : 0000               // Port X ID        2
UINT16 : 0000               // Port Y ID        2
UINT16 : 0000               // Latency or Bandwidth    2
UINT16 : BBBC               // Reserved         2
----

References:
1. CXL 2.0 specification
https://www.computeexpresslink.org/download-the-specification
2. PCI-SIG ECN: Data Object Exchange (DOE)
http://www.pcisig.com
3. Coherent Device Attribute Table	CDAT 1.02
https://uefi.org/sites/default/files/resources/Coherent%20Device%20Attribute%20Table_1.02.pdf

---
Chris Browy (2):
  Basic PCIe DOE support
  CXL DOE support for CDAT and Compliance Mode

 MAINTAINERS                               |  49 +--
 hw/cxl/cxl-component-utils.c              |  93 +++++
 hw/mem/cxl_type3.c                        | 184 ++++++++++
 hw/pci/meson.build                        |   1 +
 hw/pci/pci.c                              |  13 +-
 hw/pci/pcie_doe.c                         | 416 ++++++++++++++++++++++
 include/hw/cxl/cxl_cdat.h                 | 127 +++++++
 include/hw/cxl/cxl_compl.h                | 252 +++++++++++++
 include/hw/cxl/cxl_component.h            |  74 ++++
 include/hw/cxl/cxl_device.h               |   4 +
 include/hw/cxl/cxl_pci.h                  |   2 +
 include/hw/pci/pci_ids.h                  |   5 +-
 include/hw/pci/pcie.h                     |   1 +
 include/hw/pci/pcie_doe.h                 | 142 ++++++++
 include/hw/pci/pcie_regs.h                |   4 +
 include/standard-headers/linux/pci_regs.h |   3 +-
 16 files changed, 1327 insertions(+), 43 deletions(-)
 create mode 100644 hw/pci/pcie_doe.c
 create mode 100644 include/hw/cxl/cxl_cdat.h
 create mode 100644 include/hw/cxl/cxl_compl.h
 create mode 100644 include/hw/pci/pcie_doe.h

-- 
2.17.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support
  2021-03-09 20:33 [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Chris Browy
@ 2021-03-09 21:03 ` Chris Browy
  2021-03-11 19:05   ` Jonathan Cameron
  2021-03-09 21:04 ` [RFC PATCH v3 cxl-2.0-doe 2/2] CXL DOE support for CDAT and Compliance Mode Chris Browy
  2021-03-12 11:54 ` [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Jonathan Cameron
  2 siblings, 1 reply; 6+ messages in thread
From: Chris Browy @ 2021-03-09 21:03 UTC (permalink / raw)
  To: qemu-devel, linux-cxl
  Cc: ben.widawsky, david, vishal.l.verma, jgroves, cbrowy, armbru,
	f4bug, mst, tyshao, hchkuo, Jonathan.Cameron, imammedo,
	dan.j.williams, ira.weiny

---
 MAINTAINERS                               |  49 +--
 hw/pci/meson.build                        |   1 +
 hw/pci/pci.c                              |  13 +-
 hw/pci/pcie_doe.c                         | 416 ++++++++++++++++++++++
 include/hw/pci/pci_ids.h                  |   5 +-
 include/hw/pci/pcie.h                     |   1 +
 include/hw/pci/pcie_doe.h                 | 142 ++++++++
 include/hw/pci/pcie_regs.h                |   4 +
 include/standard-headers/linux/pci_regs.h |   3 +-
 9 files changed, 591 insertions(+), 43 deletions(-)
 create mode 100644 hw/pci/pcie_doe.c
 create mode 100644 include/hw/pci/pcie_doe.h

diff --git a/MAINTAINERS b/MAINTAINERS
index f9097ed9e7..8c5a9690a5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -87,7 +87,7 @@ S390 general architecture support
 M: Cornelia Huck <cohuck@redhat.com>
 M: Thomas Huth <thuth@redhat.com>
 S: Supported
-F: default-configs/*/s390x-softmmu.mak
+F: default-configs/s390x-softmmu.mak
 F: gdb-xml/s390*.xml
 F: hw/char/sclp*.[hc]
 F: hw/char/terminal3270.c
@@ -188,15 +188,6 @@ F: include/hw/cris/
 F: tests/tcg/cris/
 F: disas/cris.c
 
-Hexagon TCG CPUs
-M: Taylor Simpson <tsimpson@quicinc.com>
-S: Supported
-F: target/hexagon/
-F: linux-user/hexagon/
-F: tests/tcg/hexagon/
-F: disas/hexagon.c
-F: default-configs/targets/hexagon-linux-user.mak
-
 HPPA (PA-RISC) TCG CPUs
 M: Richard Henderson <richard.henderson@linaro.org>
 S: Maintained
@@ -239,7 +230,7 @@ R: Jiaxun Yang <jiaxun.yang@flygoat.com>
 R: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
 S: Odd Fixes
 F: target/mips/
-F: default-configs/*/*mips*
+F: default-configs/*mips*
 F: disas/mips.c
 F: docs/system/cpu-models-mips.rst.inc
 F: hw/intc/mips_gic.c
@@ -263,7 +254,7 @@ S: Maintained
 F: target/moxie/
 F: disas/moxie.c
 F: hw/moxie/
-F: default-configs/*/moxie-softmmu.mak
+F: default-configs/moxie-softmmu.mak
 
 NiosII TCG CPUs
 M: Chris Wulff <crwulff@gmail.com>
@@ -272,7 +263,7 @@ S: Maintained
 F: target/nios2/
 F: hw/nios2/
 F: disas/nios2.c
-F: default-configs/*/nios2-softmmu.mak
+F: default-configs/nios2-softmmu.mak
 
 OpenRISC TCG CPUs
 M: Stafford Horne <shorne@gmail.com>
@@ -367,7 +358,7 @@ F: hw/xtensa/
 F: tests/tcg/xtensa/
 F: disas/xtensa.c
 F: include/hw/xtensa/xtensa-isa.h
-F: default-configs/*/xtensa*.mak
+F: default-configs/xtensa*.mak
 
 TriCore TCG CPUs
 M: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
@@ -1038,7 +1029,7 @@ AVR MCUs
 M: Michael Rolnik <mrolnik@gmail.com>
 R: Sarah Harris <S.E.Harris@kent.ac.uk>
 S: Maintained
-F: default-configs/*/avr-softmmu.mak
+F: default-configs/avr-softmmu.mak
 F: hw/avr/
 F: include/hw/char/avr_usart.h
 F: hw/char/avr_usart.c
@@ -1067,7 +1058,7 @@ HP B160L
 M: Richard Henderson <richard.henderson@linaro.org>
 R: Helge Deller <deller@gmx.de>
 S: Odd Fixes
-F: default-configs/*/hppa-softmmu.mak
+F: default-configs/hppa-softmmu.mak
 F: hw/hppa/
 F: pc-bios/hppa-firmware.img
 
@@ -1183,7 +1174,6 @@ F: hw/intc/loongson_liointc.c
 F: hw/mips/loongson3_bootp.c
 F: hw/mips/loongson3_bootp.h
 F: hw/mips/loongson3_virt.c
-F: tests/acceptance/machine_mips_loongson3v.py
 
 Boston
 M: Paul Burton <paulburton@kernel.org>
@@ -1373,15 +1363,6 @@ F: include/hw/misc/mchp_pfsoc_dmc.h
 F: include/hw/misc/mchp_pfsoc_ioscb.h
 F: include/hw/misc/mchp_pfsoc_sysreg.h
 
-SiFive Machines
-M: Alistair Francis <Alistair.Francis@wdc.com>
-M: Bin Meng <bin.meng@windriver.com>
-M: Palmer Dabbelt <palmer@dabbelt.com>
-L: qemu-riscv@nongnu.org
-S: Supported
-F: hw/*/*sifive*.c
-F: include/hw/*/*sifive*.h
-
 RX Machines
 -----------
 rx-gdbsim
@@ -1468,7 +1449,7 @@ F: hw/s390x/
 F: include/hw/s390x/
 F: hw/watchdog/wdt_diag288.c
 F: include/hw/watchdog/wdt_diag288.h
-F: default-configs/*/s390x-softmmu.mak
+F: default-configs/s390x-softmmu.mak
 F: tests/acceptance/machine_s390_ccw_virtio.py
 T: git https://gitlab.com/cohuck/qemu.git s390-next
 T: git https://github.com/borntraeger/qemu.git s390-next
@@ -1681,6 +1662,13 @@ F: docs/pci*
 F: docs/specs/*pci*
 F: default-configs/pci.mak
 
+PCIE DOE
+M: Huai-Cheng Kuo <hchkuo@avery-design.com.tw>
+M: Chris Browy <cbrowy@avery-design.com>
+S: Supported
+F: include/hw/pci/pcie_doe.h
+F: hw/pci/pcie_doe.c
+
 ACPI/SMBIOS
 M: Michael S. Tsirkin <mst@redhat.com>
 M: Igor Mammedov <imammedo@redhat.com>
@@ -1764,7 +1752,6 @@ F: hw/ssi/xilinx_*
 
 SD (Secure Card)
 M: Philippe Mathieu-Daudé <f4bug@amsat.org>
-M: Bin Meng <bin.meng@windriver.com>
 L: qemu-block@nongnu.org
 S: Odd Fixes
 F: include/hw/sd/sd*
@@ -1859,7 +1846,6 @@ F: fsdev/
 F: docs/interop/virtfs-proxy-helper.rst
 F: tests/qtest/virtio-9p-test.c
 T: git https://gitlab.com/gkurz/qemu.git 9p-next
-T: git https://github.com/cschoenebeck/qemu.git 9p.next
 
 virtio-blk
 M: Stefan Hajnoczi <stefanha@redhat.com>
@@ -2904,13 +2890,13 @@ F: accel/tcg/user-exec*.c
 BSD user
 S: Orphan
 F: bsd-user/
-F: default-configs/targets/*-bsd-user.mak
+F: default-configs/*-bsd-user.mak
 
 Linux user
 M: Laurent Vivier <laurent@vivier.eu>
 S: Maintained
 F: linux-user/
-F: default-configs/targets/*linux-user.mak
+F: default-configs/*-linux-user.mak
 F: scripts/qemu-binfmt-conf.sh
 F: scripts/update-syscalltbl.sh
 F: scripts/update-mips-syscall-args.sh
@@ -2930,7 +2916,6 @@ S: Maintained
 F: docs/devel/tcg-plugins.rst
 F: plugins/
 F: tests/plugin/
-F: tests/acceptance/tcg_plugins.py
 F: contrib/plugins/
 
 AArch64 TCG target
diff --git a/hw/pci/meson.build b/hw/pci/meson.build
index 5c4bbac817..115e50222f 100644
--- a/hw/pci/meson.build
+++ b/hw/pci/meson.build
@@ -12,6 +12,7 @@ pci_ss.add(files(
 # allow plugging PCIe devices into PCI buses, include them even if
 # CONFIG_PCI_EXPRESS=n.
 pci_ss.add(files('pcie.c', 'pcie_aer.c'))
+pci_ss.add(files('pcie_doe.c'))
 softmmu_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files('pcie_port.c', 'pcie_host.c'))
 softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
 
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 9d834facba..8d95c3fd25 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -132,13 +132,8 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
 static void pcie_bus_realize(BusState *qbus, Error **errp)
 {
     PCIBus *bus = PCI_BUS(qbus);
-    Error *local_err = NULL;
 
-    pci_bus_realize(qbus, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    pci_bus_realize(qbus, errp);
 
     /*
      * A PCI-E bus can support extended config space if it's the root
@@ -2158,8 +2153,10 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
             pci_qdev_unrealize(DEVICE(pci_dev));
             return;
         }
-        if ((pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)
-            || (PCI_FUNC(pci_dev->devfn) != 0)) {
+        if (!(pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)
+            && (PCI_FUNC(pci_dev->devfn) == 0)) {
+            qdev->allow_unplug_during_migration = true;
+        } else {
             error_setg(errp, "failover: primary device must be in its own "
                               "PCI slot");
             pci_qdev_unrealize(DEVICE(pci_dev));
diff --git a/hw/pci/pcie_doe.c b/hw/pci/pcie_doe.c
new file mode 100644
index 0000000000..fab58eb897
--- /dev/null
+++ b/hw/pci/pcie_doe.c
@@ -0,0 +1,416 @@
+/*
+ * PCIe Data Object Exchange
+ *
+ * Copyright (C) 2020 Avery Design Systems, Inc.
+ *
+ * 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/log.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qemu/range.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pcie.h"
+#include "hw/pci/pcie_doe.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+
+/*
+ * DOE Default Protocols (Discovery, CMA)
+ */
+/* Discovery Request Object */
+struct doe_discovery {
+    DOEHeader header;
+    uint8_t index;
+    uint8_t reserved[3];
+} QEMU_PACKED;
+
+/* Discovery Response Object */
+struct doe_discovery_rsp {
+    DOEHeader header;
+    uint16_t vendor_id;
+    uint8_t doe_type;
+    uint8_t next_index;
+} QEMU_PACKED;
+
+/* Callback for Discovery */
+static bool pcie_doe_discovery_rsp(DOECap *doe_cap)
+{
+    struct doe_discovery *req = pcie_doe_get_req(doe_cap);
+    struct doe_discovery_rsp rsp;
+    uint8_t index = req->index;
+    DOEProtocol *prot;
+
+    /* Request length mismatch, discard */
+    if (pcie_doe_object_len(req) <
+        DIV_ROUND_UP(sizeof(struct doe_discovery), 4)) {
+        return false;
+    }
+
+    rsp.header = (DOEHeader) {
+        .vendor_id = PCI_VENDOR_ID_PCI_SIG,
+        .doe_type = PCI_SIG_DOE_DISCOVERY,
+        .length = DIV_ROUND_UP(sizeof(struct doe_discovery_rsp), 4),
+    };
+
+    /* Point to the requested protocol, index 0 must be Discovery */
+    if (index == 0) {
+        rsp.vendor_id = PCI_VENDOR_ID_PCI_SIG;
+        rsp.doe_type = PCI_SIG_DOE_DISCOVERY;
+    } else {
+        if (index < doe_cap->protocol_num) {
+            prot = &doe_cap->protocols[index - 1];
+        } else {
+            prot = NULL;
+        }
+
+        rsp.vendor_id = (prot) ? prot->vendor_id : 0xFFFF;
+        rsp.doe_type = (prot) ? prot->doe_type : 0xFF;
+    }
+
+    rsp.next_index = (index + 1) % doe_cap->protocol_num,
+
+    pcie_doe_set_rsp(doe_cap, &rsp);
+
+    return true;
+}
+
+/* Callback for CMA */
+bool pcie_doe_cma_rsp(DOECap *doe_cap)
+{
+    doe_cap->status.error = 1;
+
+    memset(doe_cap->read_mbox, 0,
+           PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
+
+    doe_cap->write_mbox_len = 0;
+
+    return false;
+}
+
+/*
+ * DOE Utilities
+ */
+static void pcie_doe_reset_mbox(DOECap *st)
+{
+    st->read_mbox_idx = 0;
+    st->read_mbox_len = 0;
+    st->write_mbox_len = 0;
+
+    memset(st->read_mbox, 0, PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
+    memset(st->write_mbox, 0, PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
+}
+
+/*
+ * PCI DOE functions
+ */
+/*
+ * Add DOE cap to a dev
+ * Initialize its protocol.
+ */
+void pcie_doe_init(PCIDevice *dev, DOECap *doe_cap, uint16_t offset,
+                   DOEProtocol *protocols, bool intr, uint16_t vec)
+{
+    pcie_add_capability(dev, PCI_EXT_CAP_ID_DOE, PCI_DOE_VER, offset,
+                        PCI_DOE_SIZEOF);
+
+    doe_cap->pdev = dev;
+    doe_cap->offset = offset;
+
+    /* Configure MSI/MSI-X */
+    if (intr && (msi_present(dev) || msix_present(dev))) {
+        doe_cap->cap.intr = intr;
+        doe_cap->cap.vec = vec;
+    }
+
+    /* Set up W/R Mailbox buffer */
+    doe_cap->write_mbox = g_malloc0(PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
+    doe_cap->read_mbox = g_malloc0(PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
+
+    pcie_doe_reset_mbox(doe_cap);
+
+    /* Register default discovery protocol */
+    doe_cap->protocols = protocols;
+    for (; protocols->vendor_id; protocols++) {
+        doe_cap->protocol_num++;
+    }
+    assert(doe_cap->protocol_num < PCI_DOE_PROTOCOL_MAX);
+
+    /* Add 1 for Discovery */
+    doe_cap->protocol_num++;
+}
+
+void pcie_doe_fini(DOECap *doe_cap)
+{
+    g_free(doe_cap->read_mbox);
+    g_free(doe_cap->write_mbox);
+    g_free(doe_cap);
+}
+
+uint32_t pcie_doe_build_protocol(DOEProtocol *p)
+{
+    return DATA_OBJ_BUILD_HEADER1(p->vendor_id, p->doe_type);
+}
+
+/* Return the pointer of DOE request in write mailbox buffer */
+void *pcie_doe_get_req(DOECap *doe_cap)
+{
+    return doe_cap->write_mbox;
+}
+
+/* Copy the response to read mailbox buffer */
+void pcie_doe_set_rsp(DOECap *doe_cap, void *rsp)
+{
+    uint32_t len = pcie_doe_object_len(rsp);
+
+    memcpy(doe_cap->read_mbox + doe_cap->read_mbox_len,
+           rsp, len * sizeof(uint32_t));
+    doe_cap->read_mbox_len += len;
+}
+
+/* Get Data Object length */
+uint32_t pcie_doe_object_len(void *obj)
+{
+    uint32_t len = (obj) ? ((DOEHeader *)obj)->length : 0;
+
+    return len & (PCI_DOE_MAX_DW_SIZE - 1);
+}
+
+static void pcie_doe_write_mbox(DOECap *doe_cap, uint32_t val)
+{
+    doe_cap->write_mbox[doe_cap->write_mbox_len] = val;
+
+    if (doe_cap->write_mbox_len == 1) {
+        DOE_DBG("(Capture DOE request DO length = %x)", val & 0x0003ffff);
+    }
+
+    doe_cap->write_mbox_len++;
+}
+
+static void pcie_doe_irq_assert(DOECap *doe_cap)
+{
+    PCIDevice *dev = doe_cap->pdev;
+
+    if (doe_cap->cap.intr && doe_cap->ctrl.intr) {
+        if (doe_cap->status.intr) {
+            return;
+        }
+        doe_cap->status.intr = 1;
+
+        /* Interrupt notify */
+        if (msix_enabled(dev)) {
+            msix_notify(dev, doe_cap->cap.vec);
+        } else if (msi_enabled(dev)) {
+            msi_notify(dev, doe_cap->cap.vec);
+        }
+        /* Not support legacy IRQ */
+    }
+}
+
+static void pcie_doe_set_ready(DOECap *doe_cap, bool rdy)
+{
+    doe_cap->status.ready = rdy;
+
+    if (rdy) {
+        pcie_doe_irq_assert(doe_cap);
+    }
+}
+
+static void pcie_doe_set_error(DOECap *doe_cap, bool err)
+{
+    doe_cap->status.error = err;
+
+    if (err) {
+        pcie_doe_irq_assert(doe_cap);
+    }
+}
+
+/*
+ * Check protocol the incoming request in write_mbox and
+ * memcpy the corresponding response to read_mbox.
+ *
+ * "discard" should be set up if the response callback
+ * function could not deal with request (e.g. length
+ * mismatch) or the protocol of request was not found.
+ */
+static void pcie_doe_prepare_rsp(DOECap *doe_cap)
+{
+    bool success = false;
+    int p;
+
+    if (doe_cap->status.error) {
+        return;
+    }
+
+    if (doe_cap->write_mbox[0] ==
+        DATA_OBJ_BUILD_HEADER1(PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_DISCOVERY)) {
+        /* Discovery */
+        success = pcie_doe_discovery_rsp(doe_cap);
+    } else {
+        /* Other protocols */
+        for (p = 0; p < doe_cap->protocol_num - 1; p++) {
+            if (doe_cap->write_mbox[0] ==
+                pcie_doe_build_protocol(&doe_cap->protocols[p])) {
+                DOE_DBG("(protocol = %x)", doe_cap->write_mbox[0]);
+                /*
+                 * Spec:
+                 * If the number of DW transferred does not match the
+                 * indicated Length for a data object, then the
+                 * data object must be silently discarded.
+                 */
+                if (doe_cap->write_mbox_len ==
+                    pcie_doe_object_len(pcie_doe_get_req(doe_cap))) {
+                    success = doe_cap->protocols[p].set_rsp(doe_cap);
+                }
+                break;
+            }
+        }
+    }
+
+    /* Set DOE Ready */
+    if (success) {
+        pcie_doe_set_ready(doe_cap, 1);
+    } else {
+        pcie_doe_reset_mbox(doe_cap);
+    }
+}
+
+/*
+ * Read from DOE config space.
+ * Return false if the address doesn't hit the range.
+ */
+bool pcie_doe_read_config(DOECap *doe_cap, uint32_t addr, int size,
+                          uint32_t *buf)
+{
+    uint32_t shift, mask = 0xFFFFFFFF;
+    uint16_t doe_offset = doe_cap->offset;
+
+    if (!range_covers_byte(doe_offset + PCI_EXP_DOE_CAP,
+                           PCI_DOE_SIZEOF - 4, addr)) {
+        return false;
+    }
+
+    DOE_DBG("%s: addr=%x, size=%x\n", __func__, addr, size);
+    addr -= doe_offset;
+    *buf = 0;
+
+    if (range_covers_byte(PCI_EXP_DOE_CAP, sizeof(uint32_t), addr)) {
+        DOE_DBG("  CAP");
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_REG, INTR_SUPP,
+                          doe_cap->cap.intr);
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_REG, DOE_INTR_MSG_NUM,
+                          doe_cap->cap.vec);
+    } else if (range_covers_byte(PCI_EXP_DOE_CTRL, sizeof(uint32_t), addr)) {
+        DOE_DBG("  CONTROL");
+        /* Must return ABORT=0 and GO=0 */
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_CONTROL, DOE_INTR_EN,
+                          doe_cap->ctrl.intr);
+    } else if (range_covers_byte(PCI_EXP_DOE_STATUS, sizeof(uint32_t), addr)) {
+        DOE_DBG("  STATUS");
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DOE_BUSY,
+                          doe_cap->status.busy);
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DOE_INTR_STATUS,
+                          doe_cap->status.intr);
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DOE_ERROR,
+                          doe_cap->status.error);
+        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DATA_OBJ_RDY,
+                          doe_cap->status.ready);
+    } else if (range_covers_byte(PCI_EXP_DOE_RD_DATA_MBOX, sizeof(uint32_t), addr)) {
+        DOE_DBG("  RD_MBOX");
+        if (doe_cap->status.ready && !doe_cap->status.error) {
+            DOE_DBG(" (ready, off=%x, len=%x)",
+                    doe_cap->read_mbox_idx, doe_cap->read_mbox_len);
+            *buf = doe_cap->read_mbox[doe_cap->read_mbox_idx];
+        }
+    } else if (range_covers_byte(PCI_EXP_DOE_WR_DATA_MBOX, sizeof(uint32_t), addr)) {
+        DOE_DBG("  WR_MBOX");
+    }
+    DOE_DBG(", val=%x\n", *buf);
+
+    /* Alignment */
+    shift = addr % sizeof(uint32_t);
+    *buf >>= shift * 8;
+    mask >>= (sizeof(uint32_t) - size) * 8;
+    *buf &= mask;
+
+    return true;
+}
+
+/*
+ * Write to DOE config space.
+ * Early return if the address doesn't hit the range or receives an abort signal.
+ */
+void pcie_doe_write_config(DOECap *doe_cap,
+                           uint32_t addr, uint32_t val, int size)
+{
+    uint16_t doe_offset = doe_cap->offset;
+    uint32_t shift;
+
+    if (!range_covers_byte(doe_offset + PCI_EXP_DOE_CAP,
+                           PCI_DOE_SIZEOF - 4, addr)) {
+        return;
+    }
+
+    DOE_DBG("%s: addr = %x, size = %x, val = %x\033[m\n", __func__, addr, size, val);
+    /* Alignment */
+    shift = addr % sizeof(uint32_t);
+    addr -= (doe_offset - shift);
+    val <<= shift * 8;
+
+    switch (addr) {
+    case PCI_EXP_DOE_CTRL:
+        DOE_DBG("  CONTROL");
+        if (FIELD_EX32(val, PCI_DOE_CAP_CONTROL, DOE_ABORT)) {
+            /* If ABORT, clear status reg DOE Error and DOE Ready */
+            DOE_DBG("-ABORT");
+            pcie_doe_set_ready(doe_cap, 0);
+            pcie_doe_set_error(doe_cap, 0);
+            pcie_doe_reset_mbox(doe_cap);
+            return;
+        }
+
+        if (FIELD_EX32(val, PCI_DOE_CAP_CONTROL, DOE_GO)) {
+            DOE_DBG("-GO");
+            pcie_doe_prepare_rsp(doe_cap);
+        }
+
+        if (FIELD_EX32(val, PCI_DOE_CAP_CONTROL, DOE_INTR_EN)) {
+            DOE_DBG("-INTR");
+            doe_cap->ctrl.intr = 1;
+        }
+        break;
+    case PCI_EXP_DOE_STATUS:
+        DOE_DBG("  STATUS");
+        if (FIELD_EX32(val, PCI_DOE_CAP_STATUS, DOE_INTR_STATUS)) {
+            DOE_DBG("-INTR");
+            doe_cap->status.intr = 0;
+        }
+        break;
+    case PCI_EXP_DOE_RD_DATA_MBOX:
+        DOE_DBG("  RD_MBOX");
+        doe_cap->read_mbox_idx += 1;
+        DOE_DBG(" (incr offset = %x)", doe_cap->read_mbox_idx);
+        if (doe_cap->read_mbox_idx == doe_cap->read_mbox_len) {
+            /* Last DW */
+            pcie_doe_reset_mbox(doe_cap);
+            pcie_doe_set_ready(doe_cap, 0);
+        } else if (doe_cap->read_mbox_idx > doe_cap->read_mbox_len) {
+            /* Underflow */
+            pcie_doe_set_error(doe_cap, 1);
+        }
+        break;
+    case PCI_EXP_DOE_WR_DATA_MBOX:
+        DOE_DBG("  WR_MBOX");
+        pcie_doe_write_mbox(doe_cap, val);
+        DOE_DBG(" (offset = %x)", doe_cap->write_mbox_len);
+        break;
+    case PCI_EXP_DOE_CAP:
+        DOE_DBG("  CAP (no effect)");
+    default:
+        break;
+    }
+    DOE_DBG("\n");
+}
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index 95f92d98e9..ffa9853247 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -157,6 +157,8 @@
 
 /* Vendors and devices.  Sort key: vendor first, device next. */
 
+#define PCI_VENDOR_ID_PCI_SIG            0x0001
+
 #define PCI_VENDOR_ID_LSI_LOGIC          0x1000
 #define PCI_DEVICE_ID_LSI_53C810         0x0001
 #define PCI_DEVICE_ID_LSI_53C895A        0x0012
@@ -208,10 +210,9 @@
 #define PCI_DEVICE_ID_VIA_ISA_BRIDGE     0x0686
 #define PCI_DEVICE_ID_VIA_IDE            0x0571
 #define PCI_DEVICE_ID_VIA_UHCI           0x3038
-#define PCI_DEVICE_ID_VIA_82C686B_PM     0x3057
+#define PCI_DEVICE_ID_VIA_ACPI           0x3057
 #define PCI_DEVICE_ID_VIA_AC97           0x3058
 #define PCI_DEVICE_ID_VIA_MC97           0x3068
-#define PCI_DEVICE_ID_VIA_8231_PM        0x8235
 
 #define PCI_VENDOR_ID_MARVELL            0x11ab
 
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 14c58ebdb6..47d6f66e52 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -25,6 +25,7 @@
 #include "hw/pci/pcie_regs.h"
 #include "hw/pci/pcie_aer.h"
 #include "hw/hotplug.h"
+#include "hw/pci/pcie_doe.h"
 
 typedef enum {
     /* for attention and power indicator */
diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h
new file mode 100644
index 0000000000..45fe2d3c88
--- /dev/null
+++ b/include/hw/pci/pcie_doe.h
@@ -0,0 +1,142 @@
+/*
+ * PCIe Data Object Exchange
+ *
+ * Copyright (C) 2020 Avery Design Systems, Inc.
+ *
+ * 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 PCIE_DOE_H
+#define PCIE_DOE_H
+
+#include "qemu/range.h"
+#include "qemu/typedefs.h"
+#include "hw/register.h"
+
+/*
+ * PCI DOE register defines 7.9.24
+ */
+/* Capabilities Register 7.9.24.2 */
+#define PCI_EXP_DOE_CAP             0x04
+REG32(PCI_DOE_CAP_REG, 0)
+    FIELD(PCI_DOE_CAP_REG, INTR_SUPP, 0, 1)
+    FIELD(PCI_DOE_CAP_REG, DOE_INTR_MSG_NUM, 1, 11)
+
+/* Control Register 7.9.24.3 */
+#define PCI_EXP_DOE_CTRL            0x08
+REG32(PCI_DOE_CAP_CONTROL, 0)
+    FIELD(PCI_DOE_CAP_CONTROL, DOE_ABORT, 0, 1)
+    FIELD(PCI_DOE_CAP_CONTROL, DOE_INTR_EN, 1, 1)
+    FIELD(PCI_DOE_CAP_CONTROL, DOE_GO, 31, 1)
+
+/* Status Register 7.9.24.4 */
+#define PCI_EXP_DOE_STATUS          0x0c
+REG32(PCI_DOE_CAP_STATUS, 0)
+    FIELD(PCI_DOE_CAP_STATUS, DOE_BUSY, 0, 1)
+    FIELD(PCI_DOE_CAP_STATUS, DOE_INTR_STATUS, 1, 1)
+    FIELD(PCI_DOE_CAP_STATUS, DOE_ERROR, 2, 1)
+    FIELD(PCI_DOE_CAP_STATUS, DATA_OBJ_RDY, 31, 1)
+
+/* Write Data Mailbox Register 7.9.24.5 */
+#define PCI_EXP_DOE_WR_DATA_MBOX    0x10
+
+/* Read Data Mailbox Register 7.9.24.6 */
+#define PCI_EXP_DOE_RD_DATA_MBOX    0x14
+
+/* Table 6-31 */
+#define PCI_SIG_DOE_DISCOVERY       0x00
+#define PCI_SIG_DOE_CMA             0x01
+
+#define DATA_OBJ_BUILD_HEADER1(v, p)  ((p << 16) | v)
+
+#define PCI_DOE_MAX_DW_SIZE (1 << 18)
+#define PCI_DOE_PROTOCOL_MAX 256
+
+/*
+ * DOE Protocol - Data Object
+ */
+typedef struct DOEHeader DOEHeader;
+typedef struct DOEProtocol DOEProtocol;
+typedef struct DOECap DOECap;
+
+struct DOEHeader {
+    uint16_t vendor_id;
+    uint8_t doe_type;
+    uint8_t reserved;
+    uint32_t length;
+} QEMU_PACKED;
+
+/* Protocol infos and rsp function callback */
+struct DOEProtocol {
+    uint16_t vendor_id;
+    uint8_t doe_type;
+    bool (*set_rsp)(DOECap *);
+};
+
+struct DOECap {
+    /* Owner */
+    PCIDevice *pdev;
+
+    /* Capability offset */
+    uint16_t offset;
+
+    /* Capability */
+    struct {
+        bool intr;
+        uint16_t vec;
+    } cap;
+
+    /* Control */
+    struct {
+        bool abort;
+        bool intr;
+        bool go;
+    } ctrl;
+
+    /* Status */
+    struct {
+        bool busy;
+        bool intr;
+        bool error;
+        bool ready;
+    } status;
+
+    /* Mailbox buffer for device */
+    uint32_t *write_mbox;
+    uint32_t *read_mbox;
+
+    /* Mailbox position indicator */
+    uint32_t read_mbox_idx;
+    uint32_t read_mbox_len;
+    uint32_t write_mbox_len;
+
+    /* Protocols and its callback response */
+    DOEProtocol *protocols;
+    uint16_t protocol_num;
+};
+
+/* Basic DOE functions */
+void pcie_doe_init(PCIDevice *pdev, DOECap *doe_cap, uint16_t offset,
+                   DOEProtocol *protocols, bool intr, uint16_t vec);
+void pcie_doe_fini(DOECap *doe_cap);
+bool pcie_doe_read_config(DOECap *doe_cap, uint32_t addr, int size,
+                          uint32_t *buf);
+void pcie_doe_write_config(DOECap *doe_cap, uint32_t addr,
+                           uint32_t val, int size);
+
+/* Utility functions for DOEProtocol */
+uint32_t pcie_doe_build_protocol(DOEProtocol *p);
+bool pcie_doe_cma_rsp(DOECap *doe_cap);
+void *pcie_doe_get_req(DOECap *doe_cap);
+void pcie_doe_set_rsp(DOECap *doe_cap, void *rsp);
+uint32_t pcie_doe_object_len(void *obj);
+
+#define DOE_DEBUG
+#ifdef DOE_DEBUG
+#define DOE_DBG(...) printf(__VA_ARGS__)
+#else
+#define DOE_DBG(...) {}
+#endif
+
+#endif /* PCIE_DOE_H */
diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
index 1db86b0ec4..963dc2e170 100644
--- a/include/hw/pci/pcie_regs.h
+++ b/include/hw/pci/pcie_regs.h
@@ -179,4 +179,8 @@ typedef enum PCIExpLinkWidth {
 #define PCI_ACS_VER                     0x1
 #define PCI_ACS_SIZEOF                  8
 
+/* DOE Capability Register Fields */
+#define PCI_DOE_VER                     0x1
+#define PCI_DOE_SIZEOF                  24
+
 #endif /* QEMU_PCIE_REGS_H */
diff --git a/include/standard-headers/linux/pci_regs.h b/include/standard-headers/linux/pci_regs.h
index e709ae8235..4a7b7a426d 100644
--- a/include/standard-headers/linux/pci_regs.h
+++ b/include/standard-headers/linux/pci_regs.h
@@ -730,7 +730,8 @@
 #define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
 #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
 #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
-#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_PL_16GT
+#define PCI_EXT_CAP_ID_DOE      0x2E    /* Data Object Exchange */
+#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE
 
 #define PCI_EXT_CAP_DSN_SIZEOF	12
 #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40
-- 
2.17.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [RFC PATCH v3 cxl-2.0-doe 2/2] CXL DOE support for CDAT and Compliance Mode
  2021-03-09 20:33 [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Chris Browy
  2021-03-09 21:03 ` [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support Chris Browy
@ 2021-03-09 21:04 ` Chris Browy
  2021-03-12 11:51   ` Jonathan Cameron
  2021-03-12 11:54 ` [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Jonathan Cameron
  2 siblings, 1 reply; 6+ messages in thread
From: Chris Browy @ 2021-03-09 21:04 UTC (permalink / raw)
  To: qemu-devel, linux-cxl
  Cc: ben.widawsky, david, vishal.l.verma, jgroves, cbrowy, armbru,
	f4bug, mst, tyshao, hchkuo, Jonathan.Cameron, imammedo,
	dan.j.williams, ira.weiny

---
 hw/cxl/cxl-component-utils.c   |  93 ++++++++++++
 hw/mem/cxl_type3.c             | 184 ++++++++++++++++++++++++
 include/hw/cxl/cxl_cdat.h      | 127 +++++++++++++++++
 include/hw/cxl/cxl_compl.h     | 252 +++++++++++++++++++++++++++++++++
 include/hw/cxl/cxl_component.h |  74 ++++++++++
 include/hw/cxl/cxl_device.h    |   4 +
 include/hw/cxl/cxl_pci.h       |   2 +
 7 files changed, 736 insertions(+)
 create mode 100644 include/hw/cxl/cxl_cdat.h
 create mode 100644 include/hw/cxl/cxl_compl.h

diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
index 41d36f802a..1a2408dc76 100644
--- a/hw/cxl/cxl-component-utils.c
+++ b/hw/cxl/cxl-component-utils.c
@@ -11,6 +11,7 @@
 #include "qemu/log.h"
 #include "hw/pci/pci.h"
 #include "hw/cxl/cxl.h"
+#include "qapi/error.h"
 
 static uint64_t cxl_cache_mem_read_reg(void *opaque, hwaddr offset,
                                        unsigned size)
@@ -206,3 +207,95 @@ void cxl_component_create_dvsec(CXLComponentState *cxl, uint16_t length,
     range_init_nofail(&cxl->dvsecs[type], cxl->dvsec_offset, length);
     cxl->dvsec_offset += length;
 }
+
+static void cdat_len_check(struct cdat_sub_header *hdr, Error **errp)
+{
+    assert(hdr->length);
+    assert(hdr->reserved == 0);
+
+    switch (hdr->type) {
+    case CDAT_TYPE_DSMAS:
+        assert(hdr->length == sizeof(struct cdat_dsmas));
+        break;
+    case CDAT_TYPE_DSLBIS:
+        assert(hdr->length == sizeof(struct cdat_dslbis));
+        break;
+    case CDAT_TYPE_DSMSCIS:
+        assert(hdr->length == sizeof(struct cdat_dsmscis));
+        break;
+    case CDAT_TYPE_DSIS:
+        assert(hdr->length == sizeof(struct cdat_dsis));
+        break;
+    case CDAT_TYPE_DSEMTS:
+        assert(hdr->length == sizeof(struct cdat_dsemts));
+        break;
+    case CDAT_TYPE_SSLBIS:
+        assert(hdr->length >= sizeof(struct cdat_sslbis_header));
+        assert((hdr->length - sizeof(struct cdat_sslbis_header)) %
+               sizeof(struct cdat_sslbe) == 0);
+        break;
+    default:
+        error_setg(errp, "Type %d is reserved", hdr->type);
+    }
+}
+
+#define IASL_HEADER_LEN 0x24
+void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp)
+{
+    uint8_t sum = 0;
+    int i, curr = 0, ent = 0;
+    CDATStruct cdat_st[1024];
+    struct cdat_sub_header hdr;
+    FILE *fp;
+    uint8_t iasl_hdr[IASL_HEADER_LEN];
+    size_t rcount;
+
+    fp = fopen(cxl_cstate->cdat_filename, "r");
+
+    if (fp) {
+        /* Read iASL header */
+        rcount= fread(&iasl_hdr, sizeof(iasl_hdr), 1, fp);
+	if (rcount == 0)
+            error_setg(errp, "File read failed");
+	    
+
+        for (i = 0; i < IASL_HEADER_LEN; i++) {
+            sum += iasl_hdr[i];
+        }
+        sum = (~sum + 1) & 0xFF;
+
+        curr = ftell(fp);
+
+        /* Read CDAT structures */
+        while (fread(&hdr, sizeof(hdr), 1, fp)) {
+            cdat_len_check(&hdr, errp);
+
+            cdat_st[ent].base2 = curr;
+            cdat_st[ent].length = hdr.length;
+            ent++;
+
+            fseek(fp, curr + hdr.length, SEEK_SET);
+            curr = ftell(fp);
+        }
+        /* Check the last structure */
+        fseek(fp, 0, SEEK_END);
+        assert(curr == ftell(fp));
+    } else {
+        error_setg(errp, "Please specify the CDAT file by using ',cdat='");
+    }
+    cxl_cstate->cdat_file = fp;
+
+    cxl_cstate->cdat_ent_len = ent;
+    cxl_cstate->cdat_ent = g_malloc0(sizeof(CDATStruct) * ent);
+    memcpy(cxl_cstate->cdat_ent, cdat_st, sizeof(CDATStruct) * ent);
+
+    /* Set CDAT header, ent = 0 */
+    cxl_cstate->cdat_header.revision = CXL_CDAT_REV;
+    cxl_cstate->cdat_header.sequence = 0;
+    cxl_cstate->cdat_header.length += curr + sizeof(cxl_cstate->cdat_header);
+
+    sum += cxl_cstate->cdat_header.revision +
+           cxl_cstate->cdat_header.sequence + cxl_cstate->cdat_header.length;
+
+    cxl_cstate->cdat_header.checksum = ~sum + 1;
+}
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index bf33ddb915..33f571e1a5 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -13,6 +13,156 @@
 #include "qemu/rcu.h"
 #include "sysemu/hostmem.h"
 #include "hw/cxl/cxl.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+
+uint32_t cxl_doe_compliance_init(DOECap *doe_cap)
+{
+    CXLComponentState *cxl_cstate = &CT3(doe_cap->pdev)->cxl_cstate;
+    uint32_t req;
+    uint32_t byte_cnt = 0;
+
+    DOE_DBG(">> %s\n",  __func__);
+
+    req = ((struct cxl_compliance_mode_cap *)pcie_doe_get_req(doe_cap))
+        ->req_header.req_code;
+    switch (req) {
+    case CXL_COMP_MODE_CAP:
+        byte_cnt = sizeof(struct cxl_compliance_mode_cap_rsp);
+        cxl_cstate->doe_resp.cap_rsp.header.vendor_id = CXL_VENDOR_ID;
+        cxl_cstate->doe_resp.cap_rsp.header.doe_type = CXL_DOE_COMPLIANCE;
+        cxl_cstate->doe_resp.cap_rsp.header.reserved = 0x0;
+        cxl_cstate->doe_resp.cap_rsp.header.length =
+            DIV_ROUND_UP(sizeof(struct cxl_compliance_mode_cap_rsp), 4);
+        cxl_cstate->doe_resp.cap_rsp.rsp_header.rsp_code = 0x0;
+        cxl_cstate->doe_resp.cap_rsp.rsp_header.version = 0x1;
+        cxl_cstate->doe_resp.cap_rsp.rsp_header.length = 0x1c;
+        cxl_cstate->doe_resp.cap_rsp.status = 0x0;
+        cxl_cstate->doe_resp.cap_rsp.available_cap_bitmask = 0x3;
+        cxl_cstate->doe_resp.cap_rsp.enabled_cap_bitmask = 0x3;
+        break;
+    case CXL_COMP_MODE_STATUS:
+        byte_cnt = sizeof(struct cxl_compliance_mode_status_rsp);
+        cxl_cstate->doe_resp.status_rsp.header.vendor_id = CXL_VENDOR_ID;
+        cxl_cstate->doe_resp.status_rsp.header.doe_type = CXL_DOE_COMPLIANCE;
+        cxl_cstate->doe_resp.status_rsp.header.reserved = 0x0;
+        cxl_cstate->doe_resp.status_rsp.header.length =
+            DIV_ROUND_UP(sizeof(struct cxl_compliance_mode_status_rsp), 4);
+        cxl_cstate->doe_resp.status_rsp.rsp_header.rsp_code = 0x1;
+        cxl_cstate->doe_resp.status_rsp.rsp_header.version = 0x1;
+        cxl_cstate->doe_resp.status_rsp.rsp_header.length = 0x14;
+        cxl_cstate->doe_resp.status_rsp.cap_bitfield = 0x3;
+        cxl_cstate->doe_resp.status_rsp.cache_size = 0;
+        cxl_cstate->doe_resp.status_rsp.cache_size_units = 0;
+        break;
+    default:
+        break;
+    }
+
+    DOE_DBG("  REQ=%x, RSP BYTE_CNT=%d\n", req, byte_cnt);
+    DOE_DBG("<< %s\n",  __func__);
+    return byte_cnt;
+}
+
+
+bool cxl_doe_compliance_rsp(DOECap *doe_cap)
+{
+    CXLComponentState *cxl_cstate = &CT3(doe_cap->pdev)->cxl_cstate;
+    uint32_t byte_cnt;
+    uint32_t dw_cnt;
+
+    DOE_DBG(">> %s\n",  __func__);
+
+    byte_cnt = cxl_doe_compliance_init(doe_cap);
+    dw_cnt = byte_cnt / 4;
+    memcpy(doe_cap->read_mbox,
+           cxl_cstate->doe_resp.data_byte, byte_cnt);
+
+    doe_cap->read_mbox_len += dw_cnt;
+
+    DOE_DBG("  LEN=%x, RD MBOX[%d]=%x\n", dw_cnt - 1,
+            doe_cap->read_mbox_len,
+            *(doe_cap->read_mbox + dw_cnt - 1));
+
+    DOE_DBG("<< %s\n",  __func__);
+
+    return true;
+}
+
+bool cxl_doe_cdat_rsp(DOECap *doe_cap)
+{
+    CXLComponentState *cxl_cstate = &CT3(doe_cap->pdev)->cxl_cstate;
+    uint16_t ent;
+    uint32_t base = 0;
+    uint32_t len = 0 ;
+    size_t rcount;
+    struct cxl_cdat *req = pcie_doe_get_req(doe_cap);
+
+    ent = req->entry_handle;
+    if (ent == 0) {
+        len = sizeof(cxl_cstate->cdat_header);
+    } else if (ent <= cxl_cstate->cdat_ent_len) {
+        base = cxl_cstate->cdat_ent[ent - 1].base2;
+        len = cxl_cstate->cdat_ent[ent - 1].length;
+    }
+
+    struct cxl_cdat_rsp rsp = {
+        .header = {
+            .vendor_id = CXL_VENDOR_ID,
+            .doe_type = CXL_DOE_TABLE_ACCESS,
+            .reserved = 0x0,
+            .length = (sizeof(struct cxl_cdat_rsp) + len) / 4,
+        },
+        .req_code = CXL_DOE_TAB_RSP,
+        .table_type = CXL_DOE_TAB_TYPE_CDAT,
+        .entry_handle = (ent < cxl_cstate->cdat_ent_len) ? ent + 1 : CXL_DOE_TAB_ENT_MAX,
+    };
+
+    memcpy(doe_cap->read_mbox, &rsp, sizeof(rsp));
+
+    if (ent == 0) {
+        memcpy(doe_cap->read_mbox + sizeof(rsp) / 4,
+               &cxl_cstate->cdat_header, len);
+    } else if (ent <= cxl_cstate->cdat_ent_len) {
+        fseek(cxl_cstate->cdat_file, base, SEEK_SET);
+        rcount= fread(doe_cap->read_mbox + sizeof(rsp) / 4, len, 1, cxl_cstate->cdat_file);
+	if (rcount == 0)
+	    DOE_DBG("fread returned 0\n");
+    }
+
+    doe_cap->read_mbox_len += rsp.header.length;
+    DOE_DBG("%s: INCR RD_MBOX OFF=%x\n", __func__, doe_cap->read_mbox_len);
+
+    for (int i = 0; i < doe_cap->read_mbox_len; i++) {
+        DOE_DBG("  RD_MBOX[%d]=%08x\n", i, doe_cap->read_mbox[i]);
+    }
+
+    return true;
+}
+
+static uint32_t ct3d_config_read(PCIDevice *pci_dev, uint32_t addr, int size)
+{
+    CXLType3Dev *ct3d = CT3(pci_dev);
+    uint32_t val;
+
+    if (pcie_doe_read_config(&ct3d->doe_comp, addr, size, &val)) {
+        return val;
+    } else 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 = CT3(pci_dev);
+
+    pcie_doe_write_config(&ct3d->doe_comp, addr, val, size);
+    pcie_doe_write_config(&ct3d->doe_cdat, addr, val, size);
+    pci_default_write_config(pci_dev, addr, val, size);
+}
 
 static void build_dvsecs(CXLType3Dev *ct3d)
 {
@@ -134,6 +284,8 @@ static void ct3_finalize(Object *obj)
 
     g_free((void *)regs->special_ops);
     g_free(ct3d->cxl_dstate.pmem);
+
+    fclose(cxl_cstate->cdat_file);
 }
 
 static void cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
@@ -203,6 +355,19 @@ static MemoryRegion *cxl_md_get_memory_region(MemoryDeviceState *md,
     return ct3d->cxl_dstate.pmem;
 }
 
+static DOEProtocol doe_comp_prot[] = {
+    {PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_CMA, pcie_doe_cma_rsp},
+    {CXL_VENDOR_ID, CXL_DOE_COMPLIANCE, cxl_doe_compliance_rsp},
+    {CXL_VENDOR_ID, CXL_DOE_TABLE_ACCESS, cxl_doe_cdat_rsp},
+    { /* End of array */ },
+};
+
+static DOEProtocol doe_cdat_prot[] = {
+    {PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_CMA, pcie_doe_cma_rsp},
+    {CXL_VENDOR_ID, CXL_DOE_TABLE_ACCESS, cxl_doe_cdat_rsp},
+    { /* End of array */ },
+};
+
 static void ct3_realize(PCIDevice *pci_dev, Error **errp)
 {
     CXLType3Dev *ct3d = CT3(pci_dev);
@@ -210,6 +375,8 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
     ComponentRegisters *regs = &cxl_cstate->crb;
     MemoryRegion *mr = &regs->component_registers;
     uint8_t *pci_conf = pci_dev->config;
+    unsigned short msix_num = 2;
+    int i;
 
     if (!ct3d->cxl_dstate.pmem) {
         cxl_setup_memory(ct3d, errp);
@@ -239,6 +406,19 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
                      PCI_BASE_ADDRESS_SPACE_MEMORY |
                          PCI_BASE_ADDRESS_MEM_TYPE_64,
                      &ct3d->cxl_dstate.device_registers);
+
+    /* MSI(-X) Initailization */
+    msix_init_exclusive_bar(pci_dev, msix_num, 4, NULL);
+    for (i = 0; i < msix_num; i++) {
+        msix_vector_use(pci_dev, i);
+    }
+    /* msi_init(pci_dev, 0x60, 16, true, false, NULL); */
+
+    /* DOE Initailization */
+    pcie_doe_init(pci_dev, &ct3d->doe_comp, 0x160, doe_comp_prot, true, 0);
+    pcie_doe_init(pci_dev, &ct3d->doe_cdat, 0x190, doe_cdat_prot, true, 1);
+
+    cxl_doe_cdat_init(cxl_cstate, errp);
 }
 
 static uint64_t cxl_md_get_addr(const MemoryDeviceState *md)
@@ -275,6 +455,7 @@ static Property ct3_props[] = {
                      HostMemoryBackend *),
     DEFINE_PROP_LINK("lsa", CXLType3Dev, lsa, TYPE_MEMORY_BACKEND,
                      HostMemoryBackend *),
+    DEFINE_PROP_STRING("cdat", CXLType3Dev, cxl_cstate.cdat_filename),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -357,6 +538,9 @@ static void ct3_class_init(ObjectClass *oc, void *data)
     DeviceClass *dc = DEVICE_CLASS(oc);
     PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
     MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
+
+    pc->config_write = ct3d_config_write;
+    pc->config_read = ct3d_config_read;
     CXLType3Class *cvc = CXL_TYPE3_DEV_CLASS(oc);
 
     pc->realize = ct3_realize;
diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
new file mode 100644
index 0000000000..edd994d410
--- /dev/null
+++ b/include/hw/cxl/cxl_cdat.h
@@ -0,0 +1,127 @@
+/*
+ * CXL CDAT Structure
+ *
+ * Copyright (C) 2020 Avery Design Systems, Inc.
+ *
+ * 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 CXL_CDAT_H
+#define CXL_CDAT_H
+
+#include "hw/cxl/cxl_pci.h"
+
+#define CXL_DOE_TABLE_ACCESS      2
+#define CXL_DOE_PROTOCOL_CDAT     ((CXL_DOE_TABLE_ACCESS << 16) | CXL_VENDOR_ID)
+
+/*
+ * DOE CDAT Table Protocol (CXL Spec)
+ */
+#define CXL_DOE_TAB_REQ 0
+#define CXL_DOE_TAB_RSP 0
+#define CXL_DOE_TAB_TYPE_CDAT 0
+#define CXL_DOE_TAB_ENT_MAX 0xFFFF
+
+/* Read Entry Request, 8.1.11.1 Table 134 */
+struct cxl_cdat {
+    DOEHeader header;
+    uint8_t req_code;
+    uint8_t table_type;
+    uint16_t entry_handle;
+} QEMU_PACKED;
+
+/* Read Entry Response, 8.1.11.1 Table 135 */
+#define cxl_cdat_rsp    cxl_cdat    /* Same as request */
+
+/*
+ * CDAT Table Structure (CDAT Spec)
+ */
+#define CXL_CDAT_REV 1
+
+/* Data object header */
+struct cdat_table_header {
+    uint32_t length;    /* Length of table in bytes, including this header */
+    uint8_t revision;   /* ACPI Specification minor version number */
+    uint8_t checksum;   /* To make sum of entire table == 0 */
+    uint8_t reserved[6];
+    uint32_t sequence;  /* ASCII table signature */
+} QEMU_PACKED;
+
+/* Values for subtable type in CDAT structures */
+enum cdat_type {
+    CDAT_TYPE_DSMAS = 0,
+    CDAT_TYPE_DSLBIS = 1,
+    CDAT_TYPE_DSMSCIS = 2,
+    CDAT_TYPE_DSIS = 3,
+    CDAT_TYPE_DSEMTS = 4,
+    CDAT_TYPE_SSLBIS = 5,
+    CDAT_TYPE_MAX
+};
+
+struct cdat_sub_header {
+    uint8_t type;
+    uint8_t reserved;
+    uint16_t length;
+};
+
+/* CDAT Structure Subtables */
+struct cdat_dsmas {
+    struct cdat_sub_header header;
+    uint8_t DSMADhandle;
+    uint8_t flags;
+    uint16_t reserved;
+    uint64_t DPA_base;
+    uint64_t DPA_length;
+} QEMU_PACKED;
+
+struct cdat_dslbis {
+    struct cdat_sub_header header;
+    uint8_t handle;
+    uint8_t flags;
+    uint8_t data_type;
+    uint8_t reserved;
+    uint64_t entry_base_unit;
+    uint16_t entry[3];
+    uint16_t reserved2;
+} QEMU_PACKED;
+
+struct cdat_dsmscis {
+    struct cdat_sub_header header;
+    uint8_t DSMAS_handle;
+    uint8_t reserved[3];
+    uint64_t memory_side_cache_size;
+    uint32_t cache_attributes;
+} QEMU_PACKED;
+
+struct cdat_dsis {
+    struct cdat_sub_header header;
+    uint8_t flags;
+    uint8_t handle;
+    uint16_t reserved;
+} QEMU_PACKED;
+
+struct cdat_dsemts {
+    struct cdat_sub_header header;
+    uint8_t DSMAS_handle;
+    uint8_t EFI_memory_type_attr;
+    uint16_t reserved;
+    uint64_t DPA_offset;
+    uint64_t DPA_length;
+} QEMU_PACKED;
+
+struct cdat_sslbe {
+    uint16_t port_x_id;
+    uint16_t port_y_id;
+    uint16_t latency_bandwidth;
+    uint16_t reserved;
+} QEMU_PACKED;
+
+struct cdat_sslbis_header {
+    struct cdat_sub_header header;
+    uint8_t data_type;
+    uint8_t reserved[3];
+    uint64_t entry_base_unit;
+} QEMU_PACKED;
+
+#endif /* CXL_CDAT_H */
diff --git a/include/hw/cxl/cxl_compl.h b/include/hw/cxl/cxl_compl.h
new file mode 100644
index 0000000000..5edeaa5f27
--- /dev/null
+++ b/include/hw/cxl/cxl_compl.h
@@ -0,0 +1,252 @@
+/*
+ * CXL Compliance Structure
+ *
+ * Copyright (C) 2020 Avery Design Systems, Inc.
+ *
+ * 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 CXL_COMPL_H
+#define CXL_COMPL_H
+
+#include "hw/cxl/cxl_pci.h"
+
+#define CXL_DOE_COMPLIANCE        0
+#define CXL_DOE_PROTOCOL_COMPLIANCE ((CXL_DOE_COMPLIANCE << 16) | CXL_VENDOR_ID)
+
+/* 14.16.4 - Compliance Mode */
+#define CXL_COMP_MODE_CAP               0x0
+#define CXL_COMP_MODE_STATUS            0x1
+#define CXL_COMP_MODE_HALT              0x2
+#define CXL_COMP_MODE_MULT_WR_STREAM    0x3
+#define CXL_COMP_MODE_PRO_CON           0x4
+#define CXL_COMP_MODE_BOGUS             0x5
+#define CXL_COMP_MODE_INJ_POISON        0x6
+#define CXL_COMP_MODE_INJ_CRC           0x7
+#define CXL_COMP_MODE_INJ_FC            0x8
+#define CXL_COMP_MODE_TOGGLE_CACHE      0x9
+#define CXL_COMP_MODE_INJ_MAC           0xa
+#define CXL_COMP_MODE_INS_UNEXP_MAC     0xb
+#define CXL_COMP_MODE_INJ_VIRAL         0xc
+#define CXL_COMP_MODE_INJ_ALMP          0xd
+#define CXL_COMP_MODE_IGN_ALMP          0xe
+
+typedef struct compliance_req_header CompReqHeader;
+typedef struct compliance_rsp_header CompRspHeader;
+
+struct compliance_req_header {
+    uint8_t req_code;
+    uint8_t version;
+    uint16_t reserved;
+} QEMU_PACKED;
+
+struct compliance_rsp_header {
+    uint8_t rsp_code;
+    uint8_t version;
+    uint8_t length;
+} QEMU_PACKED;
+
+struct status_rsp {
+    DOEHeader header;
+    CompRspHeader rsp_header;
+    uint8_t status;
+} QEMU_PACKED;
+
+struct len_rsvd_rsp {
+    DOEHeader header;
+    /* The length field in rsp_header is reserved. */
+    CompRspHeader rsp_header;
+    uint8_t reserved[5];
+} QEMU_PACKED;
+
+/*
+ * CXL Compliance Mode Protocol 14.16.4
+ */
+/* 14.16.4.1 */
+struct cxl_compliance_mode_cap {
+    DOEHeader header;
+    CompReqHeader req_header;
+} QEMU_PACKED;
+
+struct cxl_compliance_mode_cap_rsp {
+    DOEHeader header;
+    CompRspHeader rsp_header;
+    uint8_t status;
+    uint64_t available_cap_bitmask;
+    uint64_t enabled_cap_bitmask;
+} QEMU_PACKED;
+
+/* 14.16.4.2 */
+struct cxl_compliance_mode_status {
+    DOEHeader header;
+    CompReqHeader req_header;
+} QEMU_PACKED;
+
+struct cxl_compliance_mode_status_rsp {
+    DOEHeader header;
+    CompRspHeader rsp_header;
+    uint32_t cap_bitfield;
+    uint16_t cache_size;
+    uint8_t cache_size_units;
+} QEMU_PACKED;
+
+/* 14.16.4.3 */
+struct cxl_compliance_mode_halt {
+    DOEHeader header;
+    CompReqHeader req_header;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_halt_rsp status_rsp
+
+/* 14.16.4.4 */
+struct cxl_compliance_mode_multiple_write_streaming {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t protocol;
+    uint8_t virtual_addr;
+    uint8_t self_checking;
+    uint8_t verify_read_semantics;
+    uint8_t num_inc;
+    uint8_t num_sets;
+    uint8_t num_loops;
+    uint8_t reserved2;
+    uint64_t start_addr;
+    uint64_t write_addr;
+    uint64_t writeback_addr;
+    uint64_t byte_mask;
+    uint32_t addr_incr;
+    uint32_t set_offset;
+    uint32_t pattern_p;
+    uint32_t inc_pattern_b;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_multiple_write_streaming_rsp status_rsp
+
+/* 14.16.4.5 */
+struct cxl_compliance_mode_producer_consumer {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t protocol;
+    uint8_t num_inc;
+    uint8_t num_sets;
+    uint8_t num_loops;
+    uint8_t write_semantics;
+    uint8_t reserved2[3];
+    uint64_t start_addr;
+    uint64_t byte_mask;
+    uint32_t addr_incr;
+    uint32_t set_offset;
+    uint32_t pattern;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_producer_consumer_rsp status_rsp
+
+/* 14.16.4.6 */
+struct cxl_compliance_mode_inject_bogus_writes {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t count;
+    uint8_t reserved;
+    uint32_t pattern;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_bogus_writes_rsp status_rsp
+
+/* 14.16.4.7 */
+struct cxl_compliance_mode_inject_poison {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t protocol;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_poison_rsp status_rsp
+
+/* 14.16.4.8 */
+struct cxl_compliance_mode_inject_crc {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t num_bits_flip;
+    uint8_t num_flits_inj;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_crc_rsp status_rsp
+
+/* 14.16.4.9 */
+struct cxl_compliance_mode_inject_flow_control {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t inj_flow_control;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_flow_control_rsp status_rsp
+
+/* 14.16.4.10 */
+struct cxl_compliance_mode_toggle_cache_flush {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t inj_flow_control;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_toggle_cache_flush_rsp status_rsp
+
+/* 14.16.4.11 */
+struct cxl_compliance_mode_inject_mac_delay {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t enable;
+    uint8_t mode;
+    uint8_t delay;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_mac_delay_rsp status_rsp
+
+/* 14.16.4.12 */
+struct cxl_compliance_mode_insert_unexp_mac {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t opcode;
+    uint8_t mode;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_insert_unexp_mac_rsp status_rsp
+
+/* 14.16.4.13 */
+struct cxl_compliance_mode_inject_viral {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t protocol;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_viral_rsp status_rsp
+
+/* 14.16.4.14 */
+struct cxl_compliance_mode_inject_almp {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t opcode;
+    uint8_t reserved2[3];
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_almp_rsp len_rsvd_rsp
+
+/* 14.16.4.15 */
+struct cxl_compliance_mode_ignore_almp {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t opcode;
+    uint8_t reserved2[3];
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_ignore_almp_rsp len_rsvd_rsp
+
+/* 14.16.4.16 */
+struct cxl_compliance_mode_inject_bit_error {
+    DOEHeader header;
+    CompReqHeader req_header;
+    uint8_t opcode;
+} QEMU_PACKED;
+
+#define cxl_compliance_mode_inject_bit_error_rsp len_rsvd_rsp
+
+#endif /* CXL_COMPL_H */
diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
index acc0730d96..1294d74d78 100644
--- a/include/hw/cxl/cxl_component.h
+++ b/include/hw/cxl/cxl_component.h
@@ -18,6 +18,7 @@
 #include "qemu/range.h"
 #include "qemu/typedefs.h"
 #include "hw/register.h"
+#include "qapi/error.h"
 
 enum reg_type {
     CXL2_DEVICE,
@@ -160,6 +161,11 @@ typedef struct component_registers {
     MemoryRegionOps *special_ops;
 } ComponentRegisters;
 
+typedef struct cdat_struct {
+    void *base;
+    uint32_t base2;
+    uint32_t length;
+} CDATStruct;
 /*
  * A CXL component represents all entities in a CXL hierarchy. This includes,
  * host bridges, root ports, upstream/downstream switch ports, and devices
@@ -173,6 +179,70 @@ typedef struct cxl_component {
             struct PCIDevice *pdev;
         };
     };
+
+    /*
+     * SW write write mailbox and GO on last DW
+     * device sets READY of offset DW in DO types to copy
+     * to read mailbox register on subsequent cfg_read
+     * of read mailbox register and then on cfg_write to
+     * denote success read increments offset to next DW
+     */
+
+    union doe_request_u {
+        /* Compliance DOE Data Objects Type=0*/
+        struct cxl_compliance_mode_cap mode_cap;
+        struct cxl_compliance_mode_status mode_status;
+        struct cxl_compliance_mode_halt mode_halt;
+        struct cxl_compliance_mode_multiple_write_streaming
+            multiple_write_streaming;
+        struct cxl_compliance_mode_producer_consumer producer_consumer;
+        struct cxl_compliance_mode_inject_bogus_writes inject_bogus_writes;
+        struct cxl_compliance_mode_inject_poison inject_poison;
+        struct cxl_compliance_mode_inject_crc inject_crc;
+        struct cxl_compliance_mode_inject_flow_control inject_flow_control;
+        struct cxl_compliance_mode_toggle_cache_flush toggle_cache_flush;
+        struct cxl_compliance_mode_inject_mac_delay inject_mac_delay;
+        struct cxl_compliance_mode_insert_unexp_mac insert_unexp_mac;
+        struct cxl_compliance_mode_inject_viral inject_viral;
+        struct cxl_compliance_mode_inject_almp inject_almp;
+        struct cxl_compliance_mode_ignore_almp ignore_almp;
+        struct cxl_compliance_mode_inject_bit_error ignore_bit_error;
+        char data_byte[128];
+    } doe_request;
+
+    union doe_resp_u {
+        /* Compliance DOE Data Objects Type=0*/
+        struct cxl_compliance_mode_cap_rsp cap_rsp;
+        struct cxl_compliance_mode_status_rsp status_rsp;
+        struct cxl_compliance_mode_halt_rsp halt_rsp;
+        struct cxl_compliance_mode_multiple_write_streaming_rsp
+            multiple_write_streaming_rsp;
+        struct cxl_compliance_mode_producer_consumer_rsp producer_consumer_rsp;
+        struct cxl_compliance_mode_inject_bogus_writes_rsp
+            inject_bogus_writes_rsp;
+        struct cxl_compliance_mode_inject_poison_rsp inject_poison_rsp;
+        struct cxl_compliance_mode_inject_crc_rsp inject_crc_rsp;
+        struct cxl_compliance_mode_inject_flow_control_rsp
+            inject_flow_control_rsp;
+        struct cxl_compliance_mode_toggle_cache_flush_rsp
+            toggle_cache_flush_rsp;
+        struct cxl_compliance_mode_inject_mac_delay_rsp inject_mac_delay_rsp;
+        struct cxl_compliance_mode_insert_unexp_mac_rsp insert_unexp_mac_rsp;
+        struct cxl_compliance_mode_inject_viral inject_viral_rsp;
+        struct cxl_compliance_mode_inject_almp_rsp inject_almp_rsp;
+        struct cxl_compliance_mode_ignore_almp_rsp ignore_almp_rsp;
+        struct cxl_compliance_mode_inject_bit_error_rsp ignore_bit_error_rsp;
+        char data_byte[520 * 4];
+    } doe_resp;
+
+    /* Table entry types */
+    struct cdat_table_header cdat_header;
+
+    CDATStruct *cdat_ent;
+    int cdat_ent_len;
+
+    char *cdat_filename;
+    FILE *cdat_file;
 } CXLComponentState;
 
 void cxl_component_register_block_init(Object *obj,
@@ -184,4 +254,8 @@ void cxl_component_register_init_common(uint32_t *reg_state,
 void cxl_component_create_dvsec(CXLComponentState *cxl_cstate, uint16_t length,
                                 uint16_t type, uint8_t rev, uint8_t *body);
 
+uint32_t cxl_doe_compliance_init(DOECap *doe_cap);
+bool cxl_doe_compliance_rsp(DOECap *doe_cap);
+void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp);
+bool cxl_doe_cdat_rsp(DOECap *doe_cap);
 #endif
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 057c5b85c6..de006ff463 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -236,6 +236,10 @@ typedef struct cxl_type3_dev {
     /* State */
     CXLComponentState cxl_cstate;
     CXLDeviceState cxl_dstate;
+
+    /* DOE */
+    DOECap doe_comp;
+    DOECap doe_cdat;
 } CXLType3Dev;
 
 #ifndef TYPE_CXL_TYPE3_DEV
diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
index e8235b10cc..a0df324a1f 100644
--- a/include/hw/cxl/cxl_pci.h
+++ b/include/hw/cxl/cxl_pci.h
@@ -12,6 +12,8 @@
 
 #include "hw/pci/pci.h"
 #include "hw/pci/pcie.h"
+#include "hw/cxl/cxl_cdat.h"
+#include "hw/cxl/cxl_compl.h"
 
 #define CXL_VENDOR_ID 0x1e98
 
-- 
2.17.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support
  2021-03-09 21:03 ` [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support Chris Browy
@ 2021-03-11 19:05   ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-03-11 19:05 UTC (permalink / raw)
  To: Chris Browy
  Cc: ben.widawsky, david, vishal.l.verma, jgroves, qemu-devel,
	linux-cxl, armbru, mst, tyshao, hchkuo, imammedo, dan.j.williams,
	ira.weiny, f4bug

On Tue, 9 Mar 2021 16:03:55 -0500
Chris Browy <cbrowy@avery-design.com> wrote:

Hi Chris, various specific comments inline but a few important ones
to watch for next version.

1) Always read the final patch you are sending out. Last minute rebases can
   result in picking up a bunch of unrelated stuff like here.
   That basically gives a grumpy maintainer a reason not to read your patch.
   If I had maintainer hat on I'd have kicked this back to you unread.

2) Please add proper descriptions for each patch - I know it's an RFC but
   it feels like it is getting there so good to get that element in shape
   as well.

3) Split the patch up. Some stuff in here will be coming in via other paths
   (headers are picked up by a script in some cases).

4) To my eye far too much debug in here for code at this level of maturity.

From actual code point of view looks pretty good to me.
Drop CMA until you have it implemented - right now it's just unwanted noise
in this patch.

Thanks,

Jonathan
  
> ---
>  MAINTAINERS                               |  49 +--
>  hw/pci/meson.build                        |   1 +
>  hw/pci/pci.c                              |  13 +-
>  hw/pci/pcie_doe.c                         | 416 ++++++++++++++++++++++
>  include/hw/pci/pci_ids.h                  |   5 +-
>  include/hw/pci/pcie.h                     |   1 +
>  include/hw/pci/pcie_doe.h                 | 142 ++++++++
>  include/hw/pci/pcie_regs.h                |   4 +
>  include/standard-headers/linux/pci_regs.h |   3 +-
>  9 files changed, 591 insertions(+), 43 deletions(-)
>  create mode 100644 hw/pci/pcie_doe.c
>  create mode 100644 include/hw/pci/pcie_doe.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f9097ed9e7..8c5a9690a5 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -87,7 +87,7 @@ S390 general architecture support
>  M: Cornelia Huck <cohuck@redhat.com>
>  M: Thomas Huth <thuth@redhat.com>
>  S: Supported
> -F: default-configs/*/s390x-softmmu.mak
> +F: default-configs/s390x-softmmu.mak
>  F: gdb-xml/s390*.xml
>  F: hw/char/sclp*.[hc]
>  F: hw/char/terminal3270.c
> @@ -188,15 +188,6 @@ F: include/hw/cris/
>  F: tests/tcg/cris/
>  F: disas/cris.c
>  
> -Hexagon TCG CPUs
> -M: Taylor Simpson <tsimpson@quicinc.com>
> -S: Supported
> -F: target/hexagon/
> -F: linux-user/hexagon/
> -F: tests/tcg/hexagon/
> -F: disas/hexagon.c
> -F: default-configs/targets/hexagon-linux-user.mak
> -
>  HPPA (PA-RISC) TCG CPUs
>  M: Richard Henderson <richard.henderson@linaro.org>
>  S: Maintained
> @@ -239,7 +230,7 @@ R: Jiaxun Yang <jiaxun.yang@flygoat.com>
>  R: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
>  S: Odd Fixes
>  F: target/mips/
> -F: default-configs/*/*mips*
> +F: default-configs/*mips*
>  F: disas/mips.c
>  F: docs/system/cpu-models-mips.rst.inc
>  F: hw/intc/mips_gic.c
> @@ -263,7 +254,7 @@ S: Maintained
>  F: target/moxie/
>  F: disas/moxie.c
>  F: hw/moxie/
> -F: default-configs/*/moxie-softmmu.mak
> +F: default-configs/moxie-softmmu.mak
>  
>  NiosII TCG CPUs
>  M: Chris Wulff <crwulff@gmail.com>
> @@ -272,7 +263,7 @@ S: Maintained
>  F: target/nios2/
>  F: hw/nios2/
>  F: disas/nios2.c
> -F: default-configs/*/nios2-softmmu.mak
> +F: default-configs/nios2-softmmu.mak
>  
>  OpenRISC TCG CPUs
>  M: Stafford Horne <shorne@gmail.com>
> @@ -367,7 +358,7 @@ F: hw/xtensa/
>  F: tests/tcg/xtensa/
>  F: disas/xtensa.c
>  F: include/hw/xtensa/xtensa-isa.h
> -F: default-configs/*/xtensa*.mak
> +F: default-configs/xtensa*.mak
>  
>  TriCore TCG CPUs
>  M: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> @@ -1038,7 +1029,7 @@ AVR MCUs
>  M: Michael Rolnik <mrolnik@gmail.com>
>  R: Sarah Harris <S.E.Harris@kent.ac.uk>
>  S: Maintained
> -F: default-configs/*/avr-softmmu.mak
> +F: default-configs/avr-softmmu.mak
>  F: hw/avr/
>  F: include/hw/char/avr_usart.h
>  F: hw/char/avr_usart.c
> @@ -1067,7 +1058,7 @@ HP B160L
>  M: Richard Henderson <richard.henderson@linaro.org>
>  R: Helge Deller <deller@gmx.de>
>  S: Odd Fixes
> -F: default-configs/*/hppa-softmmu.mak
> +F: default-configs/hppa-softmmu.mak
>  F: hw/hppa/
>  F: pc-bios/hppa-firmware.img
>  
> @@ -1183,7 +1174,6 @@ F: hw/intc/loongson_liointc.c
>  F: hw/mips/loongson3_bootp.c
>  F: hw/mips/loongson3_bootp.h
>  F: hw/mips/loongson3_virt.c
> -F: tests/acceptance/machine_mips_loongson3v.py
>  
>  Boston
>  M: Paul Burton <paulburton@kernel.org>
> @@ -1373,15 +1363,6 @@ F: include/hw/misc/mchp_pfsoc_dmc.h
>  F: include/hw/misc/mchp_pfsoc_ioscb.h
>  F: include/hw/misc/mchp_pfsoc_sysreg.h
>  
> -SiFive Machines
> -M: Alistair Francis <Alistair.Francis@wdc.com>
> -M: Bin Meng <bin.meng@windriver.com>
> -M: Palmer Dabbelt <palmer@dabbelt.com>
> -L: qemu-riscv@nongnu.org
> -S: Supported
> -F: hw/*/*sifive*.c
> -F: include/hw/*/*sifive*.h
> -
>  RX Machines
>  -----------
>  rx-gdbsim
> @@ -1468,7 +1449,7 @@ F: hw/s390x/
>  F: include/hw/s390x/
>  F: hw/watchdog/wdt_diag288.c
>  F: include/hw/watchdog/wdt_diag288.h
> -F: default-configs/*/s390x-softmmu.mak
> +F: default-configs/s390x-softmmu.mak
>  F: tests/acceptance/machine_s390_ccw_virtio.py
>  T: git https://gitlab.com/cohuck/qemu.git s390-next
>  T: git https://github.com/borntraeger/qemu.git s390-next
> @@ -1681,6 +1662,13 @@ F: docs/pci*
>  F: docs/specs/*pci*
>  F: default-configs/pci.mak
>  
> +PCIE DOE
> +M: Huai-Cheng Kuo <hchkuo@avery-design.com.tw>
> +M: Chris Browy <cbrowy@avery-design.com>
> +S: Supported
> +F: include/hw/pci/pcie_doe.h
> +F: hw/pci/pcie_doe.c
> +
>  ACPI/SMBIOS
>  M: Michael S. Tsirkin <mst@redhat.com>
>  M: Igor Mammedov <imammedo@redhat.com>
> @@ -1764,7 +1752,6 @@ F: hw/ssi/xilinx_*
>  
>  SD (Secure Card)
>  M: Philippe Mathieu-Daudé <f4bug@amsat.org>
> -M: Bin Meng <bin.meng@windriver.com>
>  L: qemu-block@nongnu.org
>  S: Odd Fixes
>  F: include/hw/sd/sd*
> @@ -1859,7 +1846,6 @@ F: fsdev/
>  F: docs/interop/virtfs-proxy-helper.rst
>  F: tests/qtest/virtio-9p-test.c
>  T: git https://gitlab.com/gkurz/qemu.git 9p-next
> -T: git https://github.com/cschoenebeck/qemu.git 9p.next
>  
>  virtio-blk
>  M: Stefan Hajnoczi <stefanha@redhat.com>
> @@ -2904,13 +2890,13 @@ F: accel/tcg/user-exec*.c
>  BSD user
>  S: Orphan
>  F: bsd-user/
> -F: default-configs/targets/*-bsd-user.mak
> +F: default-configs/*-bsd-user.mak
>  
>  Linux user
>  M: Laurent Vivier <laurent@vivier.eu>
>  S: Maintained
>  F: linux-user/
> -F: default-configs/targets/*linux-user.mak
> +F: default-configs/*-linux-user.mak
>  F: scripts/qemu-binfmt-conf.sh
>  F: scripts/update-syscalltbl.sh
>  F: scripts/update-mips-syscall-args.sh
> @@ -2930,7 +2916,6 @@ S: Maintained
>  F: docs/devel/tcg-plugins.rst
>  F: plugins/
>  F: tests/plugin/
> -F: tests/acceptance/tcg_plugins.py
>  F: contrib/plugins/
>  
>  AArch64 TCG target
> diff --git a/hw/pci/meson.build b/hw/pci/meson.build
> index 5c4bbac817..115e50222f 100644
> --- a/hw/pci/meson.build
> +++ b/hw/pci/meson.build
> @@ -12,6 +12,7 @@ pci_ss.add(files(
>  # allow plugging PCIe devices into PCI buses, include them even if
>  # CONFIG_PCI_EXPRESS=n.
>  pci_ss.add(files('pcie.c', 'pcie_aer.c'))
> +pci_ss.add(files('pcie_doe.c'))
>  softmmu_ss.add(when: 'CONFIG_PCI_EXPRESS', if_true: files('pcie_port.c', 'pcie_host.c'))
>  softmmu_ss.add_all(when: 'CONFIG_PCI', if_true: pci_ss)
>  
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 9d834facba..8d95c3fd25 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -132,13 +132,8 @@ static void pci_bus_realize(BusState *qbus, Error **errp)
>  static void pcie_bus_realize(BusState *qbus, Error **errp)
>  {
>      PCIBus *bus = PCI_BUS(qbus);
> -    Error *local_err = NULL;
>  
> -    pci_bus_realize(qbus, &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> +    pci_bus_realize(qbus, errp);
>  
>      /*
>       * A PCI-E bus can support extended config space if it's the root
> @@ -2158,8 +2153,10 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>              pci_qdev_unrealize(DEVICE(pci_dev));
>              return;
>          }

Please do a quick review of any patch before you send it out for unrelated
stuff.

> -        if ((pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)
> -            || (PCI_FUNC(pci_dev->devfn) != 0)) {
> +        if (!(pci_dev->cap_present & QEMU_PCI_CAP_MULTIFUNCTION)
> +            && (PCI_FUNC(pci_dev->devfn) == 0)) {
> +            qdev->allow_unplug_during_migration = true;
> +        } else {
>              error_setg(errp, "failover: primary device must be in its own "
>                                "PCI slot");
>              pci_qdev_unrealize(DEVICE(pci_dev));
> diff --git a/hw/pci/pcie_doe.c b/hw/pci/pcie_doe.c
> new file mode 100644
> index 0000000000..fab58eb897
> --- /dev/null
> +++ b/hw/pci/pcie_doe.c
> @@ -0,0 +1,416 @@
> +/*
> + * PCIe Data Object Exchange
> + *
> + * Copyright (C) 2020 Avery Design Systems, Inc.
> + *
> + * 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/log.h"
> +#include "qemu/error-report.h"
> +#include "qapi/error.h"
> +#include "qemu/range.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pcie.h"
> +#include "hw/pci/pcie_doe.h"
> +#include "hw/pci/msi.h"
> +#include "hw/pci/msix.h"
> +
> +/*
> + * DOE Default Protocols (Discovery, CMA)

CMA is absolutely not a default protocol. If it is there, some other protocols
cannot be.

> + */
> +/* Discovery Request Object */
> +struct doe_discovery {
> +    DOEHeader header;
> +    uint8_t index;
> +    uint8_t reserved[3];
> +} QEMU_PACKED;
> +
> +/* Discovery Response Object */
> +struct doe_discovery_rsp {
> +    DOEHeader header;
> +    uint16_t vendor_id;
> +    uint8_t doe_type;
> +    uint8_t next_index;
> +} QEMU_PACKED;
> +
> +/* Callback for Discovery */
> +static bool pcie_doe_discovery_rsp(DOECap *doe_cap)
> +{
> +    struct doe_discovery *req = pcie_doe_get_req(doe_cap);
> +    struct doe_discovery_rsp rsp;
> +    uint8_t index = req->index;
> +    DOEProtocol *prot;
> +
> +    /* Request length mismatch, discard */
> +    if (pcie_doe_object_len(req) <
> +        DIV_ROUND_UP(sizeof(struct doe_discovery), 4)) {
> +        return false;
> +    }
> +
> +    rsp.header = (DOEHeader) {
> +        .vendor_id = PCI_VENDOR_ID_PCI_SIG,
> +        .doe_type = PCI_SIG_DOE_DISCOVERY,
> +        .length = DIV_ROUND_UP(sizeof(struct doe_discovery_rsp), 4),
> +    };
> +
> +    /* Point to the requested protocol, index 0 must be Discovery */
> +    if (index == 0) {
> +        rsp.vendor_id = PCI_VENDOR_ID_PCI_SIG;
> +        rsp.doe_type = PCI_SIG_DOE_DISCOVERY;
> +    } else {
> +        if (index < doe_cap->protocol_num) {
> +            prot = &doe_cap->protocols[index - 1];
> +        } else {
> +            prot = NULL;
> +        }
> +
> +        rsp.vendor_id = (prot) ? prot->vendor_id : 0xFFFF;
> +        rsp.doe_type = (prot) ? prot->doe_type : 0xFF;
> +    }
> +
> +    rsp.next_index = (index + 1) % doe_cap->protocol_num,
> +
> +    pcie_doe_set_rsp(doe_cap, &rsp);
> +
> +    return true;
> +}
> +
> +/* Callback for CMA */
> +bool pcie_doe_cma_rsp(DOECap *doe_cap)
> +{

Umm. Really? Please don't leave a stub like this in place. Just don't
support CMA until you are ready to do it fully.

If you can share plan on that it would be great though as it's on my
list of things to track.

> +    doe_cap->status.error = 1;
> +
> +    memset(doe_cap->read_mbox, 0,
> +           PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
> +
> +    doe_cap->write_mbox_len = 0;
> +
> +    return false;
> +}
> +
> +/*
> + * DOE Utilities

Generic comments like this always rot (become out of date) over time, so
don't bother.

> + */
> +static void pcie_doe_reset_mbox(DOECap *st)
> +{
> +    st->read_mbox_idx = 0;
> +    st->read_mbox_len = 0;
> +    st->write_mbox_len = 0;
> +
> +    memset(st->read_mbox, 0, PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
> +    memset(st->write_mbox, 0, PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
> +}
> +
> +/*
> + * PCI DOE functions

Same with this one.  Make it clear by naming of functions rather than
a comment that will probably end up in the wrong place in a few years.

> + */
> +/*
> + * Add DOE cap to a dev
> + * Initialize its protocol.
> + */
> +void pcie_doe_init(PCIDevice *dev, DOECap *doe_cap, uint16_t offset,
> +                   DOEProtocol *protocols, bool intr, uint16_t vec)
> +{
> +    pcie_add_capability(dev, PCI_EXT_CAP_ID_DOE, PCI_DOE_VER, offset,
> +                        PCI_DOE_SIZEOF);
> +
> +    doe_cap->pdev = dev;
> +    doe_cap->offset = offset;
> +
> +    /* Configure MSI/MSI-X */
> +    if (intr && (msi_present(dev) || msix_present(dev))) {
> +        doe_cap->cap.intr = intr;
> +        doe_cap->cap.vec = vec;
> +    }
> +
> +    /* Set up W/R Mailbox buffer */
> +    doe_cap->write_mbox = g_malloc0(PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
> +    doe_cap->read_mbox = g_malloc0(PCI_DOE_MAX_DW_SIZE * sizeof(uint32_t));
> +
> +    pcie_doe_reset_mbox(doe_cap);
> +
> +    /* Register default discovery protocol */
> +    doe_cap->protocols = protocols;
> +    for (; protocols->vendor_id; protocols++) {
> +        doe_cap->protocol_num++;
> +    }
> +    assert(doe_cap->protocol_num < PCI_DOE_PROTOCOL_MAX);
> +
> +    /* Add 1 for Discovery */
> +    doe_cap->protocol_num++;
> +}
> +
> +void pcie_doe_fini(DOECap *doe_cap)
> +{
> +    g_free(doe_cap->read_mbox);
> +    g_free(doe_cap->write_mbox);
> +    g_free(doe_cap);
> +}
> +
> +uint32_t pcie_doe_build_protocol(DOEProtocol *p)
> +{
> +    return DATA_OBJ_BUILD_HEADER1(p->vendor_id, p->doe_type);
> +}
> +
> +/* Return the pointer of DOE request in write mailbox buffer */
> +void *pcie_doe_get_req(DOECap *doe_cap)
> +{
> +    return doe_cap->write_mbox;
> +}
> +
> +/* Copy the response to read mailbox buffer */
> +void pcie_doe_set_rsp(DOECap *doe_cap, void *rsp)
> +{
> +    uint32_t len = pcie_doe_object_len(rsp);
> +
> +    memcpy(doe_cap->read_mbox + doe_cap->read_mbox_len,
> +           rsp, len * sizeof(uint32_t));
> +    doe_cap->read_mbox_len += len;
> +}
> +
> +/* Get Data Object length */
> +uint32_t pcie_doe_object_len(void *obj)
> +{
> +    uint32_t len = (obj) ? ((DOEHeader *)obj)->length : 0;
> +
> +    return len & (PCI_DOE_MAX_DW_SIZE - 1);
> +}
> +
> +static void pcie_doe_write_mbox(DOECap *doe_cap, uint32_t val)
> +{
> +    doe_cap->write_mbox[doe_cap->write_mbox_len] = val;
> +
> +    if (doe_cap->write_mbox_len == 1) {
> +        DOE_DBG("(Capture DOE request DO length = %x)", val & 0x0003ffff);
> +    }
> +
> +    doe_cap->write_mbox_len++;
> +}
> +
> +static void pcie_doe_irq_assert(DOECap *doe_cap)
> +{
> +    PCIDevice *dev = doe_cap->pdev;
> +
> +    if (doe_cap->cap.intr && doe_cap->ctrl.intr) {
> +        if (doe_cap->status.intr) {
> +            return;
> +        }
> +        doe_cap->status.intr = 1;
> +
> +        /* Interrupt notify */
> +        if (msix_enabled(dev)) {
> +            msix_notify(dev, doe_cap->cap.vec);
> +        } else if (msi_enabled(dev)) {
> +            msi_notify(dev, doe_cap->cap.vec);
> +        }
> +        /* Not support legacy IRQ */
> +    }
> +}
> +
> +static void pcie_doe_set_ready(DOECap *doe_cap, bool rdy)
> +{
> +    doe_cap->status.ready = rdy;
> +
> +    if (rdy) {
> +        pcie_doe_irq_assert(doe_cap);
> +    }
> +}
> +
> +static void pcie_doe_set_error(DOECap *doe_cap, bool err)
> +{
> +    doe_cap->status.error = err;
> +
> +    if (err) {
> +        pcie_doe_irq_assert(doe_cap);
> +    }
> +}
> +
> +/*
> + * Check protocol the incoming request in write_mbox and
> + * memcpy the corresponding response to read_mbox.
> + *
> + * "discard" should be set up if the response callback
> + * function could not deal with request (e.g. length
> + * mismatch) or the protocol of request was not found.

I think this comment is out of date?  What does "discard" being
setup mean?

> + */
> +static void pcie_doe_prepare_rsp(DOECap *doe_cap)
> +{
> +    bool success = false;
> +    int p;
> +
> +    if (doe_cap->status.error) {
> +        return;
> +    }
> +
> +    if (doe_cap->write_mbox[0] ==
> +        DATA_OBJ_BUILD_HEADER1(PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_DISCOVERY)) {
> +        /* Discovery */
> +        success = pcie_doe_discovery_rsp(doe_cap);
> +    } else {
> +        /* Other protocols */
> +        for (p = 0; p < doe_cap->protocol_num - 1; p++) {
> +            if (doe_cap->write_mbox[0] ==
> +                pcie_doe_build_protocol(&doe_cap->protocols[p])) {
> +                DOE_DBG("(protocol = %x)", doe_cap->write_mbox[0]);
> +                /*
> +                 * Spec:
> +                 * If the number of DW transferred does not match the
> +                 * indicated Length for a data object, then the
> +                 * data object must be silently discarded.
> +                 */
> +                if (doe_cap->write_mbox_len ==
> +                    pcie_doe_object_len(pcie_doe_get_req(doe_cap))) {
> +                    success = doe_cap->protocols[p].set_rsp(doe_cap);
> +                }
> +                break;
> +            }
> +        }
> +    }
> +
> +    /* Set DOE Ready */
> +    if (success) {
> +        pcie_doe_set_ready(doe_cap, 1);
> +    } else {
> +        pcie_doe_reset_mbox(doe_cap);
> +    }
> +}
> +
> +/*
> + * Read from DOE config space.
> + * Return false if the address doesn't hit the range.
> + */
> +bool pcie_doe_read_config(DOECap *doe_cap, uint32_t addr, int size,
> +                          uint32_t *buf)
> +{
> +    uint32_t shift, mask = 0xFFFFFFFF;
> +    uint16_t doe_offset = doe_cap->offset;
> +
> +    if (!range_covers_byte(doe_offset + PCI_EXP_DOE_CAP,
> +                           PCI_DOE_SIZEOF - 4, addr)) {
> +        return false;
> +    }
> +
> +    DOE_DBG("%s: addr=%x, size=%x\n", __func__, addr, size);
> +    addr -= doe_offset;
> +    *buf = 0;
> +
> +    if (range_covers_byte(PCI_EXP_DOE_CAP, sizeof(uint32_t), addr)) {
> +        DOE_DBG("  CAP");
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_REG, INTR_SUPP,
> +                          doe_cap->cap.intr);
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_REG, DOE_INTR_MSG_NUM,
> +                          doe_cap->cap.vec);
> +    } else if (range_covers_byte(PCI_EXP_DOE_CTRL, sizeof(uint32_t), addr)) {
> +        DOE_DBG("  CONTROL");
> +        /* Must return ABORT=0 and GO=0 */
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_CONTROL, DOE_INTR_EN,
> +                          doe_cap->ctrl.intr);
> +    } else if (range_covers_byte(PCI_EXP_DOE_STATUS, sizeof(uint32_t), addr)) {
> +        DOE_DBG("  STATUS");
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DOE_BUSY,
> +                          doe_cap->status.busy);
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DOE_INTR_STATUS,
> +                          doe_cap->status.intr);
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DOE_ERROR,
> +                          doe_cap->status.error);
> +        *buf = FIELD_DP32(*buf, PCI_DOE_CAP_STATUS, DATA_OBJ_RDY,
> +                          doe_cap->status.ready);
> +    } else if (range_covers_byte(PCI_EXP_DOE_RD_DATA_MBOX, sizeof(uint32_t), addr)) {
> +        DOE_DBG("  RD_MBOX");
> +        if (doe_cap->status.ready && !doe_cap->status.error) {
> +            DOE_DBG(" (ready, off=%x, len=%x)",
> +                    doe_cap->read_mbox_idx, doe_cap->read_mbox_len);
> +            *buf = doe_cap->read_mbox[doe_cap->read_mbox_idx];
> +        }
> +    } else if (range_covers_byte(PCI_EXP_DOE_WR_DATA_MBOX, sizeof(uint32_t), addr)) {
> +        DOE_DBG("  WR_MBOX");
> +    }
> +    DOE_DBG(", val=%x\n", *buf);
> +
> +    /* Alignment */
> +    shift = addr % sizeof(uint32_t);
> +    *buf >>= shift * 8;
> +    mask >>= (sizeof(uint32_t) - size) * 8;
> +    *buf &= mask;
> +
> +    return true;
> +}
> +
> +/*
> + * Write to DOE config space.
> + * Early return if the address doesn't hit the range or receives an abort signal.
> + */
> +void pcie_doe_write_config(DOECap *doe_cap,
> +                           uint32_t addr, uint32_t val, int size)
> +{
> +    uint16_t doe_offset = doe_cap->offset;
> +    uint32_t shift;
> +
> +    if (!range_covers_byte(doe_offset + PCI_EXP_DOE_CAP,
> +                           PCI_DOE_SIZEOF - 4, addr)) {
> +        return;
> +    }
> +
> +    DOE_DBG("%s: addr = %x, size = %x, val = %x\033[m\n", __func__, addr, size, val);
> +    /* Alignment */
> +    shift = addr % sizeof(uint32_t);
> +    addr -= (doe_offset - shift);
> +    val <<= shift * 8;
> +
> +    switch (addr) {
> +    case PCI_EXP_DOE_CTRL:
> +        DOE_DBG("  CONTROL");

The density of debug info in here feel too high for a code that is pretty close
to ready for merging.  I'd think hard about what actually matters - I'd suggest
just unexpected error paths, but perhaps a little more if it's really useful.

It absolutely makes sense to have this debug in there whilst developing, but
once you are done it's not really very useful + makes code less readable
and bloated.

> +        if (FIELD_EX32(val, PCI_DOE_CAP_CONTROL, DOE_ABORT)) {
> +            /* If ABORT, clear status reg DOE Error and DOE Ready */

Fairly obvious comment and it doesn't mention the reset mailbox, so I'd
drop it as unnecessary and incomplete.

> +            DOE_DBG("-ABORT");
> +            pcie_doe_set_ready(doe_cap, 0);
> +            pcie_doe_set_error(doe_cap, 0);
> +            pcie_doe_reset_mbox(doe_cap);
> +            return;
> +        }
> +
> +        if (FIELD_EX32(val, PCI_DOE_CAP_CONTROL, DOE_GO)) {
> +            DOE_DBG("-GO");
> +            pcie_doe_prepare_rsp(doe_cap);
> +        }
> +
> +        if (FIELD_EX32(val, PCI_DOE_CAP_CONTROL, DOE_INTR_EN)) {
> +            DOE_DBG("-INTR");
> +            doe_cap->ctrl.intr = 1;

How to disable interrupt?

> +        }
> +        break;
> +    case PCI_EXP_DOE_STATUS:
> +        DOE_DBG("  STATUS");
> +        if (FIELD_EX32(val, PCI_DOE_CAP_STATUS, DOE_INTR_STATUS)) {
> +            DOE_DBG("-INTR");
> +            doe_cap->status.intr = 0;
> +        }
> +        break;
> +    case PCI_EXP_DOE_RD_DATA_MBOX:
> +        DOE_DBG("  RD_MBOX");
> +        doe_cap->read_mbox_idx += 1;
> +        DOE_DBG(" (incr offset = %x)", doe_cap->read_mbox_idx);
> +        if (doe_cap->read_mbox_idx == doe_cap->read_mbox_len) {
> +            /* Last DW */
> +            pcie_doe_reset_mbox(doe_cap);
> +            pcie_doe_set_ready(doe_cap, 0);
> +        } else if (doe_cap->read_mbox_idx > doe_cap->read_mbox_len) {
> +            /* Underflow */
> +            pcie_doe_set_error(doe_cap, 1);
> +        }
> +        break;
> +    case PCI_EXP_DOE_WR_DATA_MBOX:
> +        DOE_DBG("  WR_MBOX");
> +        pcie_doe_write_mbox(doe_cap, val);
> +        DOE_DBG(" (offset = %x)", doe_cap->write_mbox_len);
> +        break;
> +    case PCI_EXP_DOE_CAP:
> +        DOE_DBG("  CAP (no effect)");
At least sometimes, qemu is built with checks that require explicit fallthrough markers
	   /* fallthrough */

This aids static checkers by making it clear you did this deliberately.
> +    default:
> +        break;
> +    }
> +    DOE_DBG("\n");
> +}
> diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
> index 95f92d98e9..ffa9853247 100644
> --- a/include/hw/pci/pci_ids.h
> +++ b/include/hw/pci/pci_ids.h
> @@ -157,6 +157,8 @@
>  
>  /* Vendors and devices.  Sort key: vendor first, device next. */
>  
> +#define PCI_VENDOR_ID_PCI_SIG            0x0001
> +
>  #define PCI_VENDOR_ID_LSI_LOGIC          0x1000
>  #define PCI_DEVICE_ID_LSI_53C810         0x0001
>  #define PCI_DEVICE_ID_LSI_53C895A        0x0012
> @@ -208,10 +210,9 @@
>  #define PCI_DEVICE_ID_VIA_ISA_BRIDGE     0x0686
>  #define PCI_DEVICE_ID_VIA_IDE            0x0571
>  #define PCI_DEVICE_ID_VIA_UHCI           0x3038
> -#define PCI_DEVICE_ID_VIA_82C686B_PM     0x3057

Please sanity check patches for merge conflicts. This just broke a bunch of other
peoples drivers.

> +#define PCI_DEVICE_ID_VIA_ACPI           0x3057
>  #define PCI_DEVICE_ID_VIA_AC97           0x3058
>  #define PCI_DEVICE_ID_VIA_MC97           0x3068
> -#define PCI_DEVICE_ID_VIA_8231_PM        0x8235
>  
>  #define PCI_VENDOR_ID_MARVELL            0x11ab
>  
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index 14c58ebdb6..47d6f66e52 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -25,6 +25,7 @@
>  #include "hw/pci/pcie_regs.h"
>  #include "hw/pci/pcie_aer.h"
>  #include "hw/hotplug.h"
> +#include "hw/pci/pcie_doe.h"
>  
>  typedef enum {
>      /* for attention and power indicator */
> diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h
> new file mode 100644
> index 0000000000..45fe2d3c88
> --- /dev/null
> +++ b/include/hw/pci/pcie_doe.h
> @@ -0,0 +1,142 @@
> +/*
> + * PCIe Data Object Exchange
> + *
> + * Copyright (C) 2020 Avery Design Systems, Inc.
> + *
> + * 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 PCIE_DOE_H
> +#define PCIE_DOE_H
> +
> +#include "qemu/range.h"
> +#include "qemu/typedefs.h"
> +#include "hw/register.h"
> +
> +/*
> + * PCI DOE register defines 7.9.24
> + */
> +/* Capabilities Register 7.9.24.2 */
> +#define PCI_EXP_DOE_CAP             0x04
> +REG32(PCI_DOE_CAP_REG, 0)
> +    FIELD(PCI_DOE_CAP_REG, INTR_SUPP, 0, 1)
> +    FIELD(PCI_DOE_CAP_REG, DOE_INTR_MSG_NUM, 1, 11)
> +
> +/* Control Register 7.9.24.3 */
> +#define PCI_EXP_DOE_CTRL            0x08
> +REG32(PCI_DOE_CAP_CONTROL, 0)
> +    FIELD(PCI_DOE_CAP_CONTROL, DOE_ABORT, 0, 1)
> +    FIELD(PCI_DOE_CAP_CONTROL, DOE_INTR_EN, 1, 1)
> +    FIELD(PCI_DOE_CAP_CONTROL, DOE_GO, 31, 1)
> +
> +/* Status Register 7.9.24.4 */
> +#define PCI_EXP_DOE_STATUS          0x0c
> +REG32(PCI_DOE_CAP_STATUS, 0)
> +    FIELD(PCI_DOE_CAP_STATUS, DOE_BUSY, 0, 1)
> +    FIELD(PCI_DOE_CAP_STATUS, DOE_INTR_STATUS, 1, 1)
> +    FIELD(PCI_DOE_CAP_STATUS, DOE_ERROR, 2, 1)
> +    FIELD(PCI_DOE_CAP_STATUS, DATA_OBJ_RDY, 31, 1)
> +
> +/* Write Data Mailbox Register 7.9.24.5 */
> +#define PCI_EXP_DOE_WR_DATA_MBOX    0x10
> +
> +/* Read Data Mailbox Register 7.9.24.6 */

As below, ECN references only as that's the finalized document.

> +#define PCI_EXP_DOE_RD_DATA_MBOX    0x14
> +
> +/* Table 6-31 */

Not in a published spec I think, so use references to the ECNs that
have been released only.

> +#define PCI_SIG_DOE_DISCOVERY       0x00
> +#define PCI_SIG_DOE_CMA             0x01
> +
> +#define DATA_OBJ_BUILD_HEADER1(v, p)  ((p << 16) | v)
> +
> +#define PCI_DOE_MAX_DW_SIZE (1 << 18)
> +#define PCI_DOE_PROTOCOL_MAX 256
> +
> +/*
> + * DOE Protocol - Data Object
> + */
> +typedef struct DOEHeader DOEHeader;
> +typedef struct DOEProtocol DOEProtocol;
> +typedef struct DOECap DOECap;
> +
> +struct DOEHeader {
> +    uint16_t vendor_id;
> +    uint8_t doe_type;
> +    uint8_t reserved;
> +    uint32_t length;
> +} QEMU_PACKED;
> +
> +/* Protocol infos and rsp function callback */
> +struct DOEProtocol {
> +    uint16_t vendor_id;
> +    uint8_t doe_type;

It's a "data object type", not a "data object exchange type", so
something like do_type or data_obj_type.
Point being the DOE is the means of exchanging data objects, not
the protocol for the objects being exchanged which is simply
a "Data object protocol"
(nitpick but nice to get it all to line up with the spec)

> +    bool (*set_rsp)(DOECap *);
> +};
> +
> +struct DOECap {
> +    /* Owner */
> +    PCIDevice *pdev;
> +
> +    /* Capability offset */
> +    uint16_t offset;
> +
> +    /* Capability */

Giving naming, this comment and similar ones below don't tell us anything
and just look like opportunities for noise / bitrot in the future.
I'd drop them.  When a variable name is sufficient that's great!

> +    struct {
> +        bool intr;
> +        uint16_t vec;
> +    } cap;
> +
> +    /* Control */
> +    struct {
> +        bool abort;
> +        bool intr;
> +        bool go;
> +    } ctrl;
> +
> +    /* Status */
> +    struct {
> +        bool busy;
> +        bool intr;
> +        bool error;
> +        bool ready;
> +    } status;
> +
> +    /* Mailbox buffer for device */
> +    uint32_t *write_mbox;
> +    uint32_t *read_mbox;
> +
> +    /* Mailbox position indicator */
> +    uint32_t read_mbox_idx;
> +    uint32_t read_mbox_len;
> +    uint32_t write_mbox_len;
> +
> +    /* Protocols and its callback response */
> +    DOEProtocol *protocols;
> +    uint16_t protocol_num;
> +};
> +
> +/* Basic DOE functions */
> +void pcie_doe_init(PCIDevice *pdev, DOECap *doe_cap, uint16_t offset,
> +                   DOEProtocol *protocols, bool intr, uint16_t vec);
> +void pcie_doe_fini(DOECap *doe_cap);
> +bool pcie_doe_read_config(DOECap *doe_cap, uint32_t addr, int size,
> +                          uint32_t *buf);
> +void pcie_doe_write_config(DOECap *doe_cap, uint32_t addr,
> +                           uint32_t val, int size);
> +
> +/* Utility functions for DOEProtocol */
> +uint32_t pcie_doe_build_protocol(DOEProtocol *p);
> +bool pcie_doe_cma_rsp(DOECap *doe_cap);

cma is a user of DOE.  Great to see it, but separate patch please.
Absolutely fine for a DOE mailbox to not support the CMA protocol.
More importantly the CMA ECN makes it clear that an CMA supporting
mailbox may not support other protocols. 6.xx.3 CMA Rules
That's then relaxed to allow IDE in the IDE ECN, but that's it
currently.

Side question here. Any plans on OS side for CMA?

> +void *pcie_doe_get_req(DOECap *doe_cap);
> +void pcie_doe_set_rsp(DOECap *doe_cap, void *rsp);
> +uint32_t pcie_doe_object_len(void *obj);
> +
> +#define DOE_DEBUG
> +#ifdef DOE_DEBUG
> +#define DOE_DBG(...) printf(__VA_ARGS__)

Drop the debug or move to debug more in keeping with qemu code.
At very least copy what is in pcie_aer and print to standard error.


> +#else
> +#define DOE_DBG(...) {}
> +#endif
> +
> +#endif /* PCIE_DOE_H */
> diff --git a/include/hw/pci/pcie_regs.h b/include/hw/pci/pcie_regs.h
> index 1db86b0ec4..963dc2e170 100644
> --- a/include/hw/pci/pcie_regs.h
> +++ b/include/hw/pci/pcie_regs.h
> @@ -179,4 +179,8 @@ typedef enum PCIExpLinkWidth {
>  #define PCI_ACS_VER                     0x1
>  #define PCI_ACS_SIZEOF                  8
>  
> +/* DOE Capability Register Fields */
> +#define PCI_DOE_VER                     0x1

Clearly precedence for this, but I'm a bit confused as to why it makes
sense to have a define for a version number given any changes in future
'should' be backwards compatible anyway.

Note this approach isn't universal.   ATS for example just puts the 0x1
in the call to pcie_add_capability(). 
I'd prefer that option myself.

> +#define PCI_DOE_SIZEOF                  24

Hohum. Seems size is often int eh standard-headers  Perhaps I should probably add
it there in the kernel patch.

> +
>  #endif /* QEMU_PCIE_REGS_H */
> diff --git a/include/standard-headers/linux/pci_regs.h b/include/standard-headers/linux/pci_regs.h
> index e709ae8235..4a7b7a426d 100644
> --- a/include/standard-headers/linux/pci_regs.h
> +++ b/include/standard-headers/linux/pci_regs.h
> @@ -730,7 +730,8 @@
>  #define PCI_EXT_CAP_ID_DVSEC	0x23	/* Designated Vendor-Specific */
>  #define PCI_EXT_CAP_ID_DLF	0x25	/* Data Link Feature */
>  #define PCI_EXT_CAP_ID_PL_16GT	0x26	/* Physical Layer 16.0 GT/s */
> -#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_PL_16GT
> +#define PCI_EXT_CAP_ID_DOE      0x2E    /* Data Object Exchange */
> +#define PCI_EXT_CAP_ID_MAX	PCI_EXT_CAP_ID_DOE

As mentioned before, this header will get pulled in from linux via a script.
For purposes of reviewing patches etc, pull this out to a patch before
this one, allowing it to be dropped once the header is available.

>  
>  #define PCI_EXT_CAP_DSN_SIZEOF	12
>  #define PCI_EXT_CAP_MCAST_ENDPOINT_SIZEOF 40



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH v3 cxl-2.0-doe 2/2] CXL DOE support for CDAT and Compliance Mode
  2021-03-09 21:04 ` [RFC PATCH v3 cxl-2.0-doe 2/2] CXL DOE support for CDAT and Compliance Mode Chris Browy
@ 2021-03-12 11:51   ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-03-12 11:51 UTC (permalink / raw)
  To: Chris Browy
  Cc: ben.widawsky, david, vishal.l.verma, jgroves, qemu-devel,
	linux-cxl, armbru, mst, tyshao, hchkuo, imammedo, dan.j.williams,
	ira.weiny, f4bug

On Tue, 9 Mar 2021 16:04:36 -0500
Chris Browy <cbrowy@avery-design.com> wrote:

Hi Chris,

As for patch 1, description needed here.  Let's move this series towards
the form needed for a final submission.

2 features, 2 patches...  If nothing else makes each one small enough to be
suitable for review with morning coffee :)


> ---
>  hw/cxl/cxl-component-utils.c   |  93 ++++++++++++
>  hw/mem/cxl_type3.c             | 184 ++++++++++++++++++++++++
>  include/hw/cxl/cxl_cdat.h      | 127 +++++++++++++++++
>  include/hw/cxl/cxl_compl.h     | 252 +++++++++++++++++++++++++++++++++
>  include/hw/cxl/cxl_component.h |  74 ++++++++++
>  include/hw/cxl/cxl_device.h    |   4 +
>  include/hw/cxl/cxl_pci.h       |   2 +
>  7 files changed, 736 insertions(+)
>  create mode 100644 include/hw/cxl/cxl_cdat.h
>  create mode 100644 include/hw/cxl/cxl_compl.h
> 
> diff --git a/hw/cxl/cxl-component-utils.c b/hw/cxl/cxl-component-utils.c
> index 41d36f802a..1a2408dc76 100644
> --- a/hw/cxl/cxl-component-utils.c
> +++ b/hw/cxl/cxl-component-utils.c

...

> +void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp)
> +{
> +    uint8_t sum = 0;
> +    int i, curr = 0, ent = 0;
> +    CDATStruct cdat_st[1024];
> +    struct cdat_sub_header hdr;
> +    FILE *fp;
> +    uint8_t iasl_hdr[IASL_HEADER_LEN];
> +    size_t rcount;
> +
> +    fp = fopen(cxl_cstate->cdat_filename, "r");
> +
> +    if (fp) {
> +        /* Read iASL header */
> +        rcount= fread(&iasl_hdr, sizeof(iasl_hdr), 1, fp);
> +	if (rcount == 0)
> +            error_setg(errp, "File read failed");
> +	    
> +
> +        for (i = 0; i < IASL_HEADER_LEN; i++) {
> +            sum += iasl_hdr[i];
> +        }
> +        sum = (~sum + 1) & 0xFF;
> +
> +        curr = ftell(fp);
> +
> +        /* Read CDAT structures */
> +        while (fread(&hdr, sizeof(hdr), 1, fp)) {
> +            cdat_len_check(&hdr, errp);
> +
> +            cdat_st[ent].base2 = curr;
> +            cdat_st[ent].length = hdr.length;
> +            ent++;
> +
> +            fseek(fp, curr + hdr.length, SEEK_SET);
> +            curr = ftell(fp);
> +        }
> +        /* Check the last structure */
> +        fseek(fp, 0, SEEK_END);
> +        assert(curr == ftell(fp));
> +    } else {
> +        error_setg(errp, "Please specify the CDAT file by using ',cdat='");

It definitely wants to be optional.  Fine to not provide one.
I'd hope we can generate one that makes 'sense' for whatever memory etc
is configured though and have that as the fallback if one isn't provided.
That will be a lot more convenient if people are working on other aspects
of support and don't happen to care about exactly what is in CDAT on
a given day.

> +    }
> +    cxl_cstate->cdat_file = fp;
> +
> +    cxl_cstate->cdat_ent_len = ent;
> +    cxl_cstate->cdat_ent = g_malloc0(sizeof(CDATStruct) * ent);
> +    memcpy(cxl_cstate->cdat_ent, cdat_st, sizeof(CDATStruct) * ent);
> +
> +    /* Set CDAT header, ent = 0 */
> +    cxl_cstate->cdat_header.revision = CXL_CDAT_REV;
> +    cxl_cstate->cdat_header.sequence = 0;
> +    cxl_cstate->cdat_header.length += curr + sizeof(cxl_cstate->cdat_header);
> +
> +    sum += cxl_cstate->cdat_header.revision +
> +           cxl_cstate->cdat_header.sequence + cxl_cstate->cdat_header.length;
> +
> +    cxl_cstate->cdat_header.checksum = ~sum + 1;
> +}
> diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
> index bf33ddb915..33f571e1a5 100644
> --- a/hw/mem/cxl_type3.c
> +++ b/hw/mem/cxl_type3.c
> @@ -13,6 +13,156 @@
>  #include "qemu/rcu.h"
>  #include "sysemu/hostmem.h"
>  #include "hw/cxl/cxl.h"
> +#include "hw/pci/msi.h"
> +#include "hw/pci/msix.h"
> +
> +uint32_t cxl_doe_compliance_init(DOECap *doe_cap)
> +{
> +    CXLComponentState *cxl_cstate = &CT3(doe_cap->pdev)->cxl_cstate;
> +    uint32_t req;
> +    uint32_t byte_cnt = 0;
> +
> +    DOE_DBG(">> %s\n",  __func__);
> +
> +    req = ((struct cxl_compliance_mode_cap *)pcie_doe_get_req(doe_cap))
> +        ->req_header.req_code;
> +    switch (req) {
> +    case CXL_COMP_MODE_CAP:
> +        byte_cnt = sizeof(struct cxl_compliance_mode_cap_rsp);
> +        cxl_cstate->doe_resp.cap_rsp.header.vendor_id = CXL_VENDOR_ID;
> +        cxl_cstate->doe_resp.cap_rsp.header.doe_type = CXL_DOE_COMPLIANCE;
> +        cxl_cstate->doe_resp.cap_rsp.header.reserved = 0x0;
> +        cxl_cstate->doe_resp.cap_rsp.header.length =
> +            DIV_ROUND_UP(sizeof(struct cxl_compliance_mode_cap_rsp), 4);
> +        cxl_cstate->doe_resp.cap_rsp.rsp_header.rsp_code = 0x0;
> +        cxl_cstate->doe_resp.cap_rsp.rsp_header.version = 0x1;
> +        cxl_cstate->doe_resp.cap_rsp.rsp_header.length = 0x1c;
> +        cxl_cstate->doe_resp.cap_rsp.status = 0x0;
> +        cxl_cstate->doe_resp.cap_rsp.available_cap_bitmask = 0x3;
> +        cxl_cstate->doe_resp.cap_rsp.enabled_cap_bitmask = 0x3;
> +        break;
> +    case CXL_COMP_MODE_STATUS:
> +        byte_cnt = sizeof(struct cxl_compliance_mode_status_rsp);
> +        cxl_cstate->doe_resp.status_rsp.header.vendor_id = CXL_VENDOR_ID;
> +        cxl_cstate->doe_resp.status_rsp.header.doe_type = CXL_DOE_COMPLIANCE;
> +        cxl_cstate->doe_resp.status_rsp.header.reserved = 0x0;
> +        cxl_cstate->doe_resp.status_rsp.header.length =
> +            DIV_ROUND_UP(sizeof(struct cxl_compliance_mode_status_rsp), 4);
> +        cxl_cstate->doe_resp.status_rsp.rsp_header.rsp_code = 0x1;
> +        cxl_cstate->doe_resp.status_rsp.rsp_header.version = 0x1;
> +        cxl_cstate->doe_resp.status_rsp.rsp_header.length = 0x14;
> +        cxl_cstate->doe_resp.status_rsp.cap_bitfield = 0x3;
> +        cxl_cstate->doe_resp.status_rsp.cache_size = 0;
> +        cxl_cstate->doe_resp.status_rsp.cache_size_units = 0;
> +        break;

Given there are only two types in reality.  Can we get rid of all the others
in the big union?  Right now it looks like those aren't handled, but reality
is they are the same as the ones here.


> +    default:
> +        break;
> +    }
> +
> +    DOE_DBG("  REQ=%x, RSP BYTE_CNT=%d\n", req, byte_cnt);
> +    DOE_DBG("<< %s\n",  __func__);
> +    return byte_cnt;
> +}
> +
> +
> +bool cxl_doe_compliance_rsp(DOECap *doe_cap)
> +{
> +    CXLComponentState *cxl_cstate = &CT3(doe_cap->pdev)->cxl_cstate;
> +    uint32_t byte_cnt;
> +    uint32_t dw_cnt;
> +
> +    DOE_DBG(">> %s\n",  __func__);
> +
> +    byte_cnt = cxl_doe_compliance_init(doe_cap);
> +    dw_cnt = byte_cnt / 4;
> +    memcpy(doe_cap->read_mbox,
> +           cxl_cstate->doe_resp.data_byte, byte_cnt);
> +
> +    doe_cap->read_mbox_len += dw_cnt;
> +
> +    DOE_DBG("  LEN=%x, RD MBOX[%d]=%x\n", dw_cnt - 1,
> +            doe_cap->read_mbox_len,
> +            *(doe_cap->read_mbox + dw_cnt - 1));
> +
> +    DOE_DBG("<< %s\n",  __func__);
> +
> +    return true;
> +}
> +
> +bool cxl_doe_cdat_rsp(DOECap *doe_cap)
> +{
> +    CXLComponentState *cxl_cstate = &CT3(doe_cap->pdev)->cxl_cstate;
> +    uint16_t ent;
> +    uint32_t base = 0;
> +    uint32_t len = 0 ;
> +    size_t rcount;
> +    struct cxl_cdat *req = pcie_doe_get_req(doe_cap);
> +
> +    ent = req->entry_handle;
> +    if (ent == 0) {
> +        len = sizeof(cxl_cstate->cdat_header);
> +    } else if (ent <= cxl_cstate->cdat_ent_len) {
> +        base = cxl_cstate->cdat_ent[ent - 1].base2;
> +        len = cxl_cstate->cdat_ent[ent - 1].length;
> +    }
> +
> +    struct cxl_cdat_rsp rsp = {
> +        .header = {
> +            .vendor_id = CXL_VENDOR_ID,
> +            .doe_type = CXL_DOE_TABLE_ACCESS,
> +            .reserved = 0x0,
> +            .length = (sizeof(struct cxl_cdat_rsp) + len) / 4,
> +        },
> +        .req_code = CXL_DOE_TAB_RSP,
> +        .table_type = CXL_DOE_TAB_TYPE_CDAT,
> +        .entry_handle = (ent < cxl_cstate->cdat_ent_len) ? ent + 1 : CXL_DOE_TAB_ENT_MAX,
> +    };
> +
> +    memcpy(doe_cap->read_mbox, &rsp, sizeof(rsp));
> +
> +    if (ent == 0) {
> +        memcpy(doe_cap->read_mbox + sizeof(rsp) / 4,
> +               &cxl_cstate->cdat_header, len);
> +    } else if (ent <= cxl_cstate->cdat_ent_len) {
> +        fseek(cxl_cstate->cdat_file, base, SEEK_SET);
> +        rcount= fread(doe_cap->read_mbox + sizeof(rsp) / 4, len, 1, cxl_cstate->cdat_file);
> +	if (rcount == 0)
> +	    DOE_DBG("fread returned 0\n");
> +    }
> +
> +    doe_cap->read_mbox_len += rsp.header.length;
> +    DOE_DBG("%s: INCR RD_MBOX OFF=%x\n", __func__, doe_cap->read_mbox_len);
> +
> +    for (int i = 0; i < doe_cap->read_mbox_len; i++) {
> +        DOE_DBG("  RD_MBOX[%d]=%08x\n", i, doe_cap->read_mbox[i]);
> +    }

It's trivial to sanity check this isn't broken by just doing the read, so
I'd drop the debug now you have it working.

> +
> +    return true;
> +}
> +


>  
>  static void build_dvsecs(CXLType3Dev *ct3d)
>  {
> @@ -134,6 +284,8 @@ static void ct3_finalize(Object *obj)
>  
>      g_free((void *)regs->special_ops);
>      g_free(ct3d->cxl_dstate.pmem);
> +
> +    fclose(cxl_cstate->cdat_file);

cdat will be small, perhaps we just read it in once and cache it?
That lets us not keep the file handle around or the file open once we have read it.

>  }
>  
>  static void cxl_setup_memory(CXLType3Dev *ct3d, Error **errp)
> @@ -203,6 +355,19 @@ static MemoryRegion *cxl_md_get_memory_region(MemoryDeviceState *md,
>      return ct3d->cxl_dstate.pmem;
>  }
>  
> +static DOEProtocol doe_comp_prot[] = {
> +    {PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_CMA, pcie_doe_cma_rsp},
> +    {CXL_VENDOR_ID, CXL_DOE_COMPLIANCE, cxl_doe_compliance_rsp},
> +    {CXL_VENDOR_ID, CXL_DOE_TABLE_ACCESS, cxl_doe_cdat_rsp},

This is kind of useful for exercising software paths, but I'm not sure
why any real hardware would do this.

Generic software is likely to stop at the first DOE that supports what
it wants, so it'll never reach the CDAT only one.

> +    { /* End of array */ },
> +};
> +
> +static DOEProtocol doe_cdat_prot[] = {
> +    {PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_CMA, pcie_doe_cma_rsp},

Not allowed to do that according to CMA ECN.

> +    {CXL_VENDOR_ID, CXL_DOE_TABLE_ACCESS, cxl_doe_cdat_rsp},
> +    { /* End of array */ },
> +};
> +
>  static void ct3_realize(PCIDevice *pci_dev, Error **errp)
>  {
>      CXLType3Dev *ct3d = CT3(pci_dev);
> @@ -210,6 +375,8 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
>      ComponentRegisters *regs = &cxl_cstate->crb;
>      MemoryRegion *mr = &regs->component_registers;
>      uint8_t *pci_conf = pci_dev->config;
> +    unsigned short msix_num = 2;
> +    int i;
>  
>      if (!ct3d->cxl_dstate.pmem) {
>          cxl_setup_memory(ct3d, errp);
> @@ -239,6 +406,19 @@ static void ct3_realize(PCIDevice *pci_dev, Error **errp)
>                       PCI_BASE_ADDRESS_SPACE_MEMORY |
>                           PCI_BASE_ADDRESS_MEM_TYPE_64,
>                       &ct3d->cxl_dstate.device_registers);
> +
> +    /* MSI(-X) Initailization */
> +    msix_init_exclusive_bar(pci_dev, msix_num, 4, NULL);
> +    for (i = 0; i < msix_num; i++) {
> +        msix_vector_use(pci_dev, i);
> +    }
> +    /* msi_init(pci_dev, 0x60, 16, true, false, NULL); */
> +
> +    /* DOE Initailization */
> +    pcie_doe_init(pci_dev, &ct3d->doe_comp, 0x160, doe_comp_prot, true, 0);
> +    pcie_doe_init(pci_dev, &ct3d->doe_cdat, 0x190, doe_cdat_prot, true, 1);
> +
> +    cxl_doe_cdat_init(cxl_cstate, errp);
>  }
>  
>  static uint64_t cxl_md_get_addr(const MemoryDeviceState *md)
> @@ -275,6 +455,7 @@ static Property ct3_props[] = {
>                       HostMemoryBackend *),
>      DEFINE_PROP_LINK("lsa", CXLType3Dev, lsa, TYPE_MEMORY_BACKEND,
>                       HostMemoryBackend *),
> +    DEFINE_PROP_STRING("cdat", CXLType3Dev, cxl_cstate.cdat_filename),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -357,6 +538,9 @@ static void ct3_class_init(ObjectClass *oc, void *data)
>      DeviceClass *dc = DEVICE_CLASS(oc);
>      PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
>      MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(oc);
> +
> +    pc->config_write = ct3d_config_write;
> +    pc->config_read = ct3d_config_read;
>      CXLType3Class *cvc = CXL_TYPE3_DEV_CLASS(oc);
>  
>      pc->realize = ct3_realize;
> diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
> new file mode 100644
> index 0000000000..edd994d410
> --- /dev/null
> +++ b/include/hw/cxl/cxl_cdat.h
> @@ -0,0 +1,127 @@

...

> +
> +/* Read Entry Request, 8.1.11.1 Table 134 */
> +struct cxl_cdat {
> +    DOEHeader header;
> +    uint8_t req_code;
> +    uint8_t table_type;
> +    uint16_t entry_handle;
> +} QEMU_PACKED;
> +
> +/* Read Entry Response, 8.1.11.1 Table 135 */
> +#define cxl_cdat_rsp    cxl_cdat    /* Same as request */

The header is the same as the request, but lots of data that this
is implying doesn't exist.  Also it doesn't have a request code
so at very least just make that field code in the structure.


> +
> +/*
> + * CDAT Table Structure (CDAT Spec)
> + */
> +#define CXL_CDAT_REV 1
> +
> +/* Data object header */
> +struct cdat_table_header {
> +    uint32_t length;    /* Length of table in bytes, including this header */
> +    uint8_t revision;   /* ACPI Specification minor version number */

No.  CDAT specification revision number. This isn't an ACPI table.

> +    uint8_t checksum;   /* To make sum of entire table == 0 */
> +    uint8_t reserved[6];
> +    uint32_t sequence;  /* ASCII table signature */

Not what the sequence number is at all.


> +} QEMU_PACKED;
> +


...

> +#endif /* CXL_CDAT_H */
> diff --git a/include/hw/cxl/cxl_compl.h b/include/hw/cxl/cxl_compl.h
> new file mode 100644
> index 0000000000..5edeaa5f27
> --- /dev/null
> +++ b/include/hw/cxl/cxl_compl.h
> @@ -0,0 +1,252 @@
> +/*
> + * CXL Compliance Structure
> + *
> + * Copyright (C) 2020 Avery Design Systems, Inc.

Time for an update to include 2021.

> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */

...

> +
> +struct status_rsp {
> +    DOEHeader header;
> +    CompRspHeader rsp_header;
> +    uint8_t status;
> +} QEMU_PACKED;
> +
> +struct len_rsvd_rsp {

Prefix these so they don't look like generic things.
Also for this one, the naming made me wonder if CXL really had defined
a length field then marked it reserved (which would be odd). 
I would go with two versions of the header and name this differently.

struct compliance_rsp_header_no_len {
    uint8_t rsp_code;
    uint8_t version;
} QEMU_PACKED;

struct comp_no_data_resp {
   DOEHeader header;
   struct compliance_rsp_header_no_len resp_header; /* typedefs if you like are fine */
   uint8_t reserved[6];
} QEMU_PACKED;

> +    DOEHeader header;
> +    /* The length field in rsp_header is reserved. */
> +    CompRspHeader rsp_header;
> +    uint8_t reserved[5];
> +} QEMU_PACKED;
> +
...

> +
> +/* 14.16.4.10 */
> +struct cxl_compliance_mode_toggle_cache_flush {
> +    DOEHeader header;
> +    CompReqHeader req_header;
> +    uint8_t inj_flow_control;

?

> +} QEMU_PACKED;
> +
> +#define cxl_compliance_mode_toggle_cache_flush_rsp status_rsp
> +

...

> +
> +#endif /* CXL_COMPL_H */
> diff --git a/include/hw/cxl/cxl_component.h b/include/hw/cxl/cxl_component.h
> index acc0730d96..1294d74d78 100644
> --- a/include/hw/cxl/cxl_component.h
> +++ b/include/hw/cxl/cxl_component.h
> @@ -18,6 +18,7 @@
>  #include "qemu/range.h"
>  #include "qemu/typedefs.h"
>  #include "hw/register.h"
> +#include "qapi/error.h"
>  
>  enum reg_type {
>      CXL2_DEVICE,
> @@ -160,6 +161,11 @@ typedef struct component_registers {
>      MemoryRegionOps *special_ops;
>  } ComponentRegisters;
>  
> +typedef struct cdat_struct {
> +    void *base;
> +    uint32_t base2;

Given base vs base2 isn't obvious naming, add some comments or
give it a more meaningful name.

> +    uint32_t length;
> +} CDATStruct;
>  /*
>   * A CXL component represents all entities in a CXL hierarchy. This includes,
>   * host bridges, root ports, upstream/downstream switch ports, and devices
> @@ -173,6 +179,70 @@ typedef struct cxl_component {
>              struct PCIDevice *pdev;
>          };
>      };
> +
> +    /*
> +     * SW write write mailbox and GO on last DW
> +     * device sets READY of offset DW in DO types to copy
> +     * to read mailbox register on subsequent cfg_read
> +     * of read mailbox register and then on cfg_write to
> +     * denote success read increments offset to next DW
> +     */

Comment doesn't seem to have anything to do with any code around here, so drop it.

> +
> +    union doe_request_u {
> +        /* Compliance DOE Data Objects Type=0*/
Space before */
> +        struct cxl_compliance_mode_cap mode_cap;
> +        struct cxl_compliance_mode_status mode_status;
> +        struct cxl_compliance_mode_halt mode_halt;
> +        struct cxl_compliance_mode_multiple_write_streaming
> +            multiple_write_streaming;
> +        struct cxl_compliance_mode_producer_consumer producer_consumer;
> +        struct cxl_compliance_mode_inject_bogus_writes inject_bogus_writes;
> +        struct cxl_compliance_mode_inject_poison inject_poison;
> +        struct cxl_compliance_mode_inject_crc inject_crc;
> +        struct cxl_compliance_mode_inject_flow_control inject_flow_control;
> +        struct cxl_compliance_mode_toggle_cache_flush toggle_cache_flush;
> +        struct cxl_compliance_mode_inject_mac_delay inject_mac_delay;
> +        struct cxl_compliance_mode_insert_unexp_mac insert_unexp_mac;
> +        struct cxl_compliance_mode_inject_viral inject_viral;
> +        struct cxl_compliance_mode_inject_almp inject_almp;
> +        struct cxl_compliance_mode_ignore_almp ignore_almp;
> +        struct cxl_compliance_mode_inject_bit_error ignore_bit_error;
> +        char data_byte[128];

We aren't accessing this as a byte anywhere, so just use a pointer
to the whole structure.  That will avoid padding this to a meaningless
length. (closer inspection this one isn't even referenced, but resp below is)

> +    } doe_request;
> +
> +    union doe_resp_u {
> +        /* Compliance DOE Data Objects Type=0*/

Space before */

> +        struct cxl_compliance_mode_cap_rsp cap_rsp;

I wonder if we can make this all shorter by using an abbreviation?

cxl_cmpl_mode_stat_rsp

and similar?

It's nice to match spec names where possible, but sometimes they just
end up as stupidly long/

> +        struct cxl_compliance_mode_status_rsp status_rsp;
> +        struct cxl_compliance_mode_halt_rsp halt_rsp;
> +        struct cxl_compliance_mode_multiple_write_streaming_rsp
> +            multiple_write_streaming_rsp;
> +        struct cxl_compliance_mode_producer_consumer_rsp producer_consumer_rsp;
> +        struct cxl_compliance_mode_inject_bogus_writes_rsp
> +            inject_bogus_writes_rsp;
> +        struct cxl_compliance_mode_inject_poison_rsp inject_poison_rsp;
> +        struct cxl_compliance_mode_inject_crc_rsp inject_crc_rsp;
> +        struct cxl_compliance_mode_inject_flow_control_rsp
> +            inject_flow_control_rsp;
> +        struct cxl_compliance_mode_toggle_cache_flush_rsp
> +            toggle_cache_flush_rsp;
> +        struct cxl_compliance_mode_inject_mac_delay_rsp inject_mac_delay_rsp;
> +        struct cxl_compliance_mode_insert_unexp_mac_rsp insert_unexp_mac_rsp;
> +        struct cxl_compliance_mode_inject_viral inject_viral_rsp;
> +        struct cxl_compliance_mode_inject_almp_rsp inject_almp_rsp;
> +        struct cxl_compliance_mode_ignore_almp_rsp ignore_almp_rsp;
> +        struct cxl_compliance_mode_inject_bit_error_rsp ignore_bit_error_rsp;
> +        char data_byte[520 * 4];

As above, I can't see a reason to have this.  If individual structures can be
variable length, put the maximums in them, not here.

> +    } doe_resp;
> +
> +    /* Table entry types */
> +    struct cdat_table_header cdat_header;
> +
> +    CDATStruct *cdat_ent;
> +    int cdat_ent_len;
> +
> +    char *cdat_filename;
> +    FILE *cdat_file;
>  } CXLComponentState;
>  
>  void cxl_component_register_block_init(Object *obj,
> @@ -184,4 +254,8 @@ void cxl_component_register_init_common(uint32_t *reg_state,
>  void cxl_component_create_dvsec(CXLComponentState *cxl_cstate, uint16_t length,
>                                  uint16_t type, uint8_t rev, uint8_t *body);
>  
> +uint32_t cxl_doe_compliance_init(DOECap *doe_cap);
> +bool cxl_doe_compliance_rsp(DOECap *doe_cap);
> +void cxl_doe_cdat_init(CXLComponentState *cxl_cstate, Error **errp);
> +bool cxl_doe_cdat_rsp(DOECap *doe_cap);
>  #endif
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index 057c5b85c6..de006ff463 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -236,6 +236,10 @@ typedef struct cxl_type3_dev {
>      /* State */
>      CXLComponentState cxl_cstate;
>      CXLDeviceState cxl_dstate;
> +
> +    /* DOE */
> +    DOECap doe_comp;
> +    DOECap doe_cdat;
>  } CXLType3Dev;
>  
>  #ifndef TYPE_CXL_TYPE3_DEV
> diff --git a/include/hw/cxl/cxl_pci.h b/include/hw/cxl/cxl_pci.h
> index e8235b10cc..a0df324a1f 100644
> --- a/include/hw/cxl/cxl_pci.h
> +++ b/include/hw/cxl/cxl_pci.h
> @@ -12,6 +12,8 @@
>  
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pcie.h"
> +#include "hw/cxl/cxl_cdat.h"
> +#include "hw/cxl/cxl_compl.h"
>  
>  #define CXL_VENDOR_ID 0x1e98
>  



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0
  2021-03-09 20:33 [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Chris Browy
  2021-03-09 21:03 ` [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support Chris Browy
  2021-03-09 21:04 ` [RFC PATCH v3 cxl-2.0-doe 2/2] CXL DOE support for CDAT and Compliance Mode Chris Browy
@ 2021-03-12 11:54 ` Jonathan Cameron
  2 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2021-03-12 11:54 UTC (permalink / raw)
  To: Chris Browy
  Cc: ben.widawsky, david, vishal.l.verma, jgroves, qemu-devel,
	linux-cxl, armbru, mst, tyshao, hchkuo, imammedo, dan.j.williams,
	ira.weiny, f4bug

On Tue, 9 Mar 2021 15:33:49 -0500
Chris Browy <cbrowy@avery-design.com> wrote:

> Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 implements
> all planned functionality.
> 
> Based on QEMU version:
> https://gitlab.com/bwidawsk/qemu/-/tree/cxl-2.0v4
> 
> Summary:
> 1: PCIe DOE support for Discovery
>    - Support multiple DOE instances for each own protocol set 
>    - MSI-X and polling supported
>    - Update error and interrupt status in DOE Status register
>    - Use static array to register callback function for DOE protocols 
>    - Deprecate DOE_SUCCESS and DOE_DISCARD
>    - Add license headers
> 2: CXL DOE for CDAT and Compliance Mode.
>    - Device supports pre-defined CDAT or user-provided CDAT.
>    - Support on reading the iASL aml file via specifying
>      "cdat=<filename.aml>" property to -device cxl-type3
> 	 skips over the ACPI header and writes only CDAT table entries
>    - Clean up CXL compliance structures
>    - DOE CDAT response returns one CDAT Structure instance based on
>      request EntryHandle value.
> 
> Example cdat.dat file: (compile with iasl -G cdat.dat)
> CDAT file may contain any mix and number of supported CDAT Structure types
> ----------------------
> /* Header */ 
> Signature : "CDAT"
> Table Length : 00000000
> Revision : 01
> Checksum : 00
> Oem ID : AVERY 
> Oem Table ID : 0 
> Oem Revision : 1 
> Asl Compiler ID : "INTL"
> Asl Compiler Revision : 20160527

I think this is a non starter because it's not an ACPI table and the header
of CDAT just doesn't look like this.

We 'might' be able to add some special handling to iASL as a convenient
tool for generating cdat, or we might just write something CDAT specific
(it's pretty simple after all!)

Normally iASL will convert a static table into it's raw binary representation
anyway, so we want to do something like that but end up with a valid
CDAT table.

Jonathan


> 
> /* CDAT structures */
> Label : DSMAS               // Field        Byte Length
> UINT8  : 0                  // Type             1
> UINT8  : 00                 // Reserved         1
> UINT16 : 0018               // Length           2
> UINT8  : 00                 // DSMADHandle      1
> UINT8  : 00                 // Flags            1
> UINT16 : 0000               // Reserved         2
> UINT64 : 0000000000000000   // DPA Base         8
> UINT64 : ffffffffffffffff   // DPA Length       8
> 
> Label : DSLBIS              // Field          Byte Length
> UINT8  : 01                 // Type             1
> UINT8  : 00                 // Reserved         1
> UINT16 : 0018               // Length           2
> UINT8  : 00                 // Handle           1
> UINT8  : 00                 // Flags            1
> UINT8  : 00                 // Data Type        1
> UINT8  : 00                 // Reserved         1
> UINT64 : 0000000000000000   // Entry Base Unit  8
> UINT16 : 0000               // Entry[0]         2
> UINT16 : 0000               // Entry[1]         2
> UINT16 : 0000               // Entry[2]         2
> UINT16 : 0000               // Reserved         2
> 
> Label: DSMSCIS              // Field        Byte Length
> UINT8  : 02                 // Type             1
> UINT8  : 00                 // Reserved         1
> UINT16 : 0014               // Length           2
> UINT8  : 00                 // DSMASHandle      1
> UINT24 : 000000             // Reserved         3
> UINT64 : 0000000000000000   // Memory Side Cache Size    8
> UINT32 : 00000000           // Cache Attributes 4 
> 
> Label : DSIS                // Field        Byte Length
> UINT8  : 03                 // Type             1
> UINT8  : 00                 // Reserved         1
> UINT16 : 0008               // Length           2
> UINT8  : 00                 // Flags            1
> UINT8  : 00                 // Handle           1
> UINT16 : 0000               // Reserved         2
> 
> Label : DSEMTS              // Field        Byte Length
> UINT8  : 04                 // Type             1
> UINT8  : 00                 // Reserved         1
> UINT16 : 0018               // Length           2
> UINT8  : 00                 // DSMAS Handle     1
> UINT8  : 00                 // EFI Memory Type and Attribute    1
> UINT16 : 0000               // Reserved         2
> UINT64 : 0000000000000000   // DPA Offset       8
> UINT64 : 0000000000000000   // DPA Length       8
> 
> Label : SSLBIS              // Field        Byte Length
> UINT8  : 05                 // Type             1
> UINT8  : 00                 // Reserved         1
> UINT16 : 0020               // Length           2
> UINT8  : 00                 // Data Type        1
> UINT24 : 000000             // Reserved         3
> UINT64 : 0000000000000000   // Entry Base Unit  8
> Label : SSLBE               // SSLBE[0]
> UINT16 : 0000               // Port X ID        2
> UINT16 : 0000               // Port Y ID        2
> UINT16 : 0000               // Latency or Bandwidth    2
> UINT16 : BBBB               // Reserved         2
> Label : SSLBE               // SSLBE[1]
> UINT16 : 0000               // Port X ID        2
> UINT16 : 0000               // Port Y ID        2
> UINT16 : 0000               // Latency or Bandwidth    2
> UINT16 : BBBC               // Reserved         2
> ----
> 
> References:
> 1. CXL 2.0 specification
> https://www.computeexpresslink.org/download-the-specification
> 2. PCI-SIG ECN: Data Object Exchange (DOE)
> http://www.pcisig.com
> 3. Coherent Device Attribute Table	CDAT 1.02
> https://uefi.org/sites/default/files/resources/Coherent%20Device%20Attribute%20Table_1.02.pdf
> 
> ---
> Chris Browy (2):
>   Basic PCIe DOE support
>   CXL DOE support for CDAT and Compliance Mode
> 
>  MAINTAINERS                               |  49 +--
>  hw/cxl/cxl-component-utils.c              |  93 +++++
>  hw/mem/cxl_type3.c                        | 184 ++++++++++
>  hw/pci/meson.build                        |   1 +
>  hw/pci/pci.c                              |  13 +-
>  hw/pci/pcie_doe.c                         | 416 ++++++++++++++++++++++
>  include/hw/cxl/cxl_cdat.h                 | 127 +++++++
>  include/hw/cxl/cxl_compl.h                | 252 +++++++++++++
>  include/hw/cxl/cxl_component.h            |  74 ++++
>  include/hw/cxl/cxl_device.h               |   4 +
>  include/hw/cxl/cxl_pci.h                  |   2 +
>  include/hw/pci/pci_ids.h                  |   5 +-
>  include/hw/pci/pcie.h                     |   1 +
>  include/hw/pci/pcie_doe.h                 | 142 ++++++++
>  include/hw/pci/pcie_regs.h                |   4 +
>  include/standard-headers/linux/pci_regs.h |   3 +-
>  16 files changed, 1327 insertions(+), 43 deletions(-)
>  create mode 100644 hw/pci/pcie_doe.c
>  create mode 100644 include/hw/cxl/cxl_cdat.h
>  create mode 100644 include/hw/cxl/cxl_compl.h
>  create mode 100644 include/hw/pci/pcie_doe.h
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-09 20:33 [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Chris Browy
2021-03-09 21:03 ` [RFC PATCH v3 cxl-2.0-doe 1/2] Basic PCIe DOE support Chris Browy
2021-03-11 19:05   ` Jonathan Cameron
2021-03-09 21:04 ` [RFC PATCH v3 cxl-2.0-doe 2/2] CXL DOE support for CDAT and Compliance Mode Chris Browy
2021-03-12 11:51   ` Jonathan Cameron
2021-03-12 11:54 ` [RFC PATCH v3 cxl-2.0-doe 0/2] Version 3 patch series for PCIe DOE for PCIe and CXL 2.0 Jonathan Cameron

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git
	git clone --mirror https://lore.kernel.org/qemu-devel/2 qemu-devel/git/2.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git