On Thu, 2026-11-05 at 00:11 -0800, 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
Can you share any details on how you are testing this? > device assignment. It does not really model a real device, and nvme > was > (is?) not supported due to other limitations. What limitations? NVMe + DOE should work here Alistair > > 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. > > @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
