On Sep 11 16:00, Wilfred Mallawa wrote: > From: Wilfred Mallawa <wilfred.mall...@wdc.com> > > This patch extends the existing support we have for NVMe with only DoE > to also add support to SPDM over the NVMe Security Send/Recv commands. > > With the new definition of the `spdm-trans` argument, users can specify > `spdm_trans=nvme` or `spdm_trans=doe`. This allows us to select the SPDM > transport respectively. SPDM over the NVMe Security Send/Recv commands > are defined in the DMTF DSP0286. > > Signed-off-by: Wilfred Mallawa <wilfred.mall...@wdc.com> > Reviewed-by: Jonathan Cameron <jonathan.came...@huawei.com>
LGTM. One small nit below that I can fix when merging if another revision is not rolled. Reviewed-by: Klaus Jensen <k.jen...@samsung.com> > --- > docs/specs/spdm.rst | 10 ++++++-- > hw/nvme/ctrl.c | 46 ++++++++++++++++++++++++++++--------- > include/hw/pci/pci_device.h | 2 ++ > 3 files changed, 45 insertions(+), 13 deletions(-) > > diff --git a/docs/specs/spdm.rst b/docs/specs/spdm.rst > index f7de080ff0..dd6cfbbd68 100644 > --- a/docs/specs/spdm.rst > +++ b/docs/specs/spdm.rst > @@ -98,7 +98,7 @@ Then you can add this to your QEMU command line: > .. code-block:: shell > > -drive file=blknvme,if=none,id=mynvme,format=raw \ > - -device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323 > + -device > nvme,drive=mynvme,serial=deadbeef,spdm_port=2323,spdm_trans=doe > > At which point QEMU will try to connect to the SPDM server. > > @@ -113,7 +113,13 @@ of the default. So the entire QEMU command might look > like this > -append "root=/dev/vda console=ttyS0" \ > -net none -nographic \ > -drive file=blknvme,if=none,id=mynvme,format=raw \ > - -device nvme,drive=mynvme,serial=deadbeef,spdm_port=2323 > + -device > nvme,drive=mynvme,serial=deadbeef,spdm_port=2323,spdm_trans=doe > + > +The `spdm_trans` argument defines the underlying transport type that is > emulated > +by QEMU. For an PCIe NVMe controller, both "doe" and "nvme" are supported. > Where, > +"doe" does SPDM transport over the PCIe extended capability Data Object > Exchange > +(DOE), and "nvme" uses the NVMe Admin Security Send/Receive commands to > +implement the SPDM transport. > > .. _DMTF: > https://www.dmtf.org/standards/SPDM > diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c > index ad52e8f569..8a610f57f2 100644 > --- a/hw/nvme/ctrl.c > +++ b/hw/nvme/ctrl.c > @@ -8947,19 +8947,31 @@ static bool nvme_init_pci(NvmeCtrl *n, PCIDevice > *pci_dev, Error **errp) > > pcie_cap_deverr_init(pci_dev); > > - /* DOE Initialisation */ > + /* SPDM Initialisation */ > if (pci_dev->spdm_port) { > - uint16_t doe_offset = n->params.sriov_max_vfs ? > - PCI_CONFIG_SPACE_SIZE + PCI_ARI_SIZEOF > - : PCI_CONFIG_SPACE_SIZE; > + switch (pci_dev->spdm_trans) { > + case SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE: > + uint16_t doe_offset = n->params.sriov_max_vfs ? > + PCI_CONFIG_SPACE_SIZE + PCI_ARI_SIZEOF > + : PCI_CONFIG_SPACE_SIZE; > > - pcie_doe_init(pci_dev, &pci_dev->doe_spdm, doe_offset, > - doe_spdm_prot, true, 0); > + pcie_doe_init(pci_dev, &pci_dev->doe_spdm, doe_offset, > + doe_spdm_prot, true, 0); > > - pci_dev->doe_spdm.spdm_socket = > spdm_socket_connect(pci_dev->spdm_port, > - errp); > + pci_dev->doe_spdm.spdm_socket = > + spdm_socket_connect(pci_dev->spdm_port, errp); > > - if (pci_dev->doe_spdm.spdm_socket < 0) { > + if (pci_dev->doe_spdm.spdm_socket < 0) { > + return false; > + } > + break; > + case SPDM_SOCKET_TRANSPORT_TYPE_NVME: > + n->spdm_socket = spdm_socket_connect(pci_dev->spdm_port, errp); > + if (n->spdm_socket < 0) { > + return false; > + } > + break; > + default: > return false; > } > } > @@ -9250,11 +9262,17 @@ static void nvme_exit(PCIDevice *pci_dev) > g_free(n->cmb.buf); > } > > + /* Only one of the `spdm_socket` below would have been setup */ > if (pci_dev->doe_spdm.spdm_socket > 0) { > spdm_socket_close(pci_dev->doe_spdm.spdm_socket, > SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE); > } > > + if (n->spdm_socket >= 0) { > + spdm_socket_close(pci_dev->doe_spdm.spdm_socket, > + SPDM_SOCKET_TRANSPORT_TYPE_NVME); > + } > + Might as well assert the above condition or at least use an else if here.
signature.asc
Description: PGP signature
