On 11/5/26 08:12, 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). > > Michael, Tao, what are your view on this? I am not Michael, but here are my $0.02.
This feature has nothing in common with AHCI, so IMHO this code should live in a separate device, kept away from our poor legacy device emulation. As posted it also changes the ich9-ahci built into q35: the device becomes PCIe with a different config space for every guest, which breaks migration from older machine types. Den
