On 5/11/26 09:11, Pierrick Bouvier wrote:
On 11/4/2026 11:12 PM, Philippe Mathieu-Daudé wrote:
On 4/11/26 17:34, Jim MacArthur wrote:
From: Pierrick Bouvier <[email protected]>

Rationale?

Signed-off-by: Pierrick Bouvier <[email protected]>
Signed-off-by: Jim MacArthur <[email protected]>
---
   hw/ide/ich.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++
++++++++--
   1 file changed, 80 insertions(+), 2 deletions(-)

diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index b00987f08d..58e2d3b998 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -70,6 +70,7 @@
   #include "hw/ide/pci.h"
   #include "hw/ide/ahci-pci.h"
   #include "ahci-internal.h"
+#include "system/spdm-socket.h"
     #define ICH9_MSI_CAP_OFFSET     0x80
   #define ICH9_SATA_CAP_OFFSET    0xA8
@@ -90,6 +91,10 @@ static const VMStateDescription vmstate_ich9_ahci = {
       },
   };
   +static const Property ich_props[] = {
+    DEFINE_PROP_UINT16("spdm_port", PCIDevice, spdm_port, 0),
+};
+
   static void pci_ich9_ahci_update_irq(void *opaque, int irq_num, int
level)
   {
       PCIDevice *pci_dev = opaque;
@@ -120,13 +125,39 @@ static void pci_ich9_ahci_init(Object *obj)
       d->ahci.irq = &d->irq;
   }
   +/* Sends an SPDM message and receives the response */
+static bool pcie_doe_spdm_rsp(DOECap *doe_cap)
+{
+    void *req = pcie_doe_get_write_mbox_ptr(doe_cap);
+    uint32_t req_len = pcie_doe_get_obj_len(req) * 4;
+    void *rsp = doe_cap->read_mbox;
+    uint32_t rsp_len = SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE;
+
+    uint32_t recvd = spdm_socket_rsp(doe_cap->spdm_socket,
+                             SPDM_TRANSPORT_TYPE_DOE,
+                             req, req_len, rsp, rsp_len);
+    doe_cap->read_mbox_len += DIV_ROUND_UP(recvd, 4);
+
+    return recvd != 0;
+}
+
+/* SPDM protocols: plain and secured SPDM messages */
+static DOEProtocol doe_spdm_prot[] = {
+    { PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_CMA, pcie_doe_spdm_rsp },
+    { PCI_VENDOR_ID_PCI_SIG, PCI_SIG_DOE_SECURED_CMA,
pcie_doe_spdm_rsp },
+    { }
+};
+
   static void pci_ich9_ahci_realize(PCIDevice *dev, Error **errp)
   {
       AHCIPCIState *d;
       int sata_cap_offset;
       uint8_t *sata_cap;
-    d = ICH9_AHCI(dev);
       int ret;
+    uint16_t cap_offset; /* Next offset for extended capabilities */
+
+    cap_offset = PCI_CONFIG_SPACE_SIZE;
+    d = ICH9_AHCI(dev);
         d->ahci.ports = 6;
       ahci_realize(&d->ahci, DEVICE(dev), pci_get_address_space(dev));
@@ -145,6 +176,7 @@ static void pci_ich9_ahci_realize(PCIDevice *dev,
Error **errp)
       pci_register_bar(dev, ICH9_MEM_BAR, PCI_BASE_ADDRESS_SPACE_MEMORY,
                        &d->ahci.mem);
   +    /* Legacy capability for ICH9_SATA_CAP */
       sata_cap_offset = pci_add_capability(dev, PCI_CAP_ID_SATA,
                                             ICH9_SATA_CAP_OFFSET,
SATA_CAP_SIZE,
                                             errp);
@@ -165,6 +197,29 @@ static void pci_ich9_ahci_realize(PCIDevice *dev,
Error **errp)
       /* Any error other than -ENOTSUP(board's MSI support is broken)
        * is a programming error.  Fall back to INTx silently on -
ENOTSUP */
       assert(!ret || ret == -ENOTSUP);
+
+    /*
+     * Initialise a capability in the legacy space after the
ICH9_SATA_CAP.
+     */
+    pcie_endpoint_cap_init(dev, ICH9_SATA_CAP_OFFSET + SATA_CAP_SIZE);
+
+    if (dev->spdm_port) {
+        /* Set up the DOE extended capability, mailboxes etc */
+        pcie_doe_init(dev, &dev->doe_spdm, cap_offset,
+                      doe_spdm_prot, true, 0);
+        cap_offset += PCI_DOE_SIZEOF;
+
+        dev->doe_spdm.spdm_socket = spdm_socket_connect(dev->spdm_port,
+                                                            errp);
+        assert(dev->doe_spdm.spdm_socket > 0);
+    }
+
+    /* IDE extended capability */
+    pcie_ide_init(dev, cap_offset);
+    cap_offset += PCI_IDE_SIZEOF;
+
+    /* TEE capability bit in legacy space*/
+    pcie_cap_tee_init(dev);

Which section of the ICH9 spec describes that? AFAICT ICH9 predates
DOE / IDE. Is there some amendment? Otherwise this might not be the
correct place to add that capability. Either use a Frankenstein ICH9
with a distinct model name, or add at the SATA layer, possibly like
a class interface (we only need the SATA_CAP_OFFSET IIUC).


It's an ad hoc addition, reproducing what Arm has been doing on fvp for
device assignment. It does not really model a real device, and nvme was
(is?) not supported due to other limitations.

IIRC, we found that in the tf-rmm branch supporting device assignment.

While it does not hurt, that's why I never sent patches before because
that's indeed a custom thing, that works for a custom software stack
that was (is?) not yet finalized.

That said, it should not hurt to add it, since it's just an extra
capability.

Well I'm maintaining MIPS board that use the ICH9 and am not keen
on having the model diverge from the spec; we already suffered from
that in the past with PIIX. That said I'm not against contributing
and committing experiments as long as they don't clutter / hinder
our existing specified models.

@Jim: Feel free to complete my answer with links to the code and a spec,
if there is one to share now. I'm sure Mathieu Poirier has all this info
also.

Michael, Tao, what are your view on this?


Regards,
Pierrick



Reply via email to