RE: [RFC 0/8] virtio,vhost: Add VIRTIO_F_NOTIFICATION_DATA support

2024-03-06 Thread Xinying Yu
> On Tue, Mar 5, 2024 at 4:21 AM Xinying Yu 
> wrote:
> >
> > Of course,  I am glad to do.  And I need to clarify that our use case only
> support VIRTIO_F_NOTIFICATION_DATA  transport feature on DPDK vDPA
> framework which the backend type is NET_CLIENT_DRIVER_VHOST_USER and
> use user_feature_bits. So the new feature add on vdpa_feature_bits  will not
> under verified in our case.  Not sure this meets your expectations?
> 
> As long as the driver keeps using notification data it is not able to tell the
> difference between one scenario or another, so yes.
> 

OK, I see.  And the test result is OK, the feature negotiation correctly and 
the use case passed.

> > One more thing, I would ask how do  I get the full series patch? Do I copy 
> > the
> RFC line by line from this link[1]?
> >
> 
> lore.kernel.org is a good alternative as Thomas mentioned. Another good one
> IMO is b4, which allows you to download the series and apply with "git am"
> too using "b4 mbox <20240301134330.4191007-1-
> jonah.pal...@oracle.com>" (untested).
> 
> https://pypi.org/project/b4/
> 
> Thanks!
> 

> For getting patches that you might have missed on the mailing list, I
> recommend lore.kernel.org :
> 
> 
> https://lore.kernel.org/qemu-devel/20240301134330.4191007-1-
> jonah.pal...@oracle.com/
> 
> You can download mbox files there that you can apply locally with "git am".
> 
>   HTH,
>Thomas

Thanks to you and Thomas for the advice which works well.

> > Thanks,
> > Xinying
> >
> >
> > [1]
> > https://lists.nongnu.org/archive/html/qemu-devel/2024-
> 03/msg00090.html
> >
> > 
> > From: Eugenio Perez Martin 
> > Sent: Saturday, March 2, 2024 4:32 AM
> > To: Wentao Jia ; Rick Zhong
> > ; Xinying Yu
> 
> > Cc: Jonah Palmer ; qemu-de...@nongnu.org
> > ; m...@redhat.com ;
> > jasow...@redhat.com ; si-wei@oracle.com
> > ; boris.ostrov...@oracle.com
> > ; raph...@enfabrica.net
> > ; kw...@redhat.com ;
> > hre...@redhat.com ; pa...@linux.ibm.com
> > ; borntrae...@linux.ibm.com
> > ; far...@linux.ibm.com
> > ; th...@redhat.com ;
> > richard.hender...@linaro.org ;
> > da...@redhat.com ; i...@linux.ibm.com
> > ; coh...@redhat.com ;
> > pbonz...@redhat.com ; f...@euphon.net
> > ; stefa...@redhat.com ;
> > qemu-block@nongnu.org ; qemu-
> s3...@nongnu.org
> > ; virtio...@lists.linux.dev
> > 
> > Subject: Re: [RFC 0/8] virtio,vhost: Add VIRTIO_F_NOTIFICATION_DATA
> > support
> >
> > Hi Wentao / Rick / Xinying Yu,
> >
> > Would it work for you to test this series on your use cases, so we
> > make sure everything works as expected?
> >
> > Thanks!
> >
> > On Fri, Mar 1, 2024 at 2:44 PM Jonah Palmer 
> wrote:
> > >
> > > The goal of these patches are to add support to a variety of virtio
> > > and vhost devices for the VIRTIO_F_NOTIFICATION_DATA transport
> > > feature. This feature indicates that a driver will pass extra data
> > > (instead of just a virtqueue's index) when notifying the corresponding
> device.
> > >
> > > The data passed in by the driver when this feature is enabled varies
> > > in format depending on if the device is using a split or packed
> > > virtqueue
> > > layout:
> > >
> > >  Split VQ
> > >   - Upper 16 bits: last_avail_idx
> > >   - Lower 16 bits: virtqueue index
> > >
> > >  Packed VQ
> > >   - Upper 16 bits: 1-bit wrap counter & 15-bit last_avail_idx
> > >   - Lower 16 bits: virtqueue index
> > >
> > > Also, due to the limitations of ioeventfd not being able to carry
> > > the extra provided by the driver, ioeventfd is left disabled for any
> > > devices using this feature.
> > >
> > > A significant aspect of this effort has been to maintain
> > > compatibility across different backends. As such, the feature is
> > > offered by backend devices only when supported, with fallback
> > > mechanisms where backend support is absent.
> > >
> >
> > Hi Wentao,
> >



[PATCH 0/2] blockdev: Fix block_resize error reporting for op blockers

2024-03-06 Thread Markus Armbruster
PATCH 2 requires my "[PATCH] char: Slightly better error reporting
when chardev is in use" posted earlier today, PATCH 1 does not.

Based-on: <20240306081505.2258405-1-arm...@redhat.com>

Markus Armbruster (2):
  blockdev: Fix block_resize error reporting for op blockers
  qerror: QERR_DEVICE_IN_USE is no longer used, drop

 include/qapi/qmp/qerror.h | 3 ---
 blockdev.c| 3 +--
 2 files changed, 1 insertion(+), 5 deletions(-)

-- 
2.44.0




[PATCH 1/2] blockdev: Fix block_resize error reporting for op blockers

2024-03-06 Thread Markus Armbruster
When block_resize() runs into an op blocker, it creates an error like
this:

error_setg(errp, "Device '%s' is in use", device);

Trouble is @device can be null.  My system formats null as "(null)",
but other systems might crash.  Reproducer:

1. Create two block devices

-> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": 
"blk0", "filename": "64k.img"}}
<- {"return": {}}
-> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": 
"blk1", "filename": "m.img"}}
<- {"return": {}}

2. Put a blocker on one them

-> {"execute": "blockdev-mirror", "arguments": {"job-id": "job0", "device": 
"blk0", "target": "blk1", "sync": "full"}}
{"return": {}}
-> {"execute": "job-pause", "arguments": {"id": "job0"}}
{"return": {}}
-> {"execute": "job-complete", "arguments": {"id": "job0"}}
{"return": {}}

   Note: job events elided for brevity.

3. Attempt to resize

-> {"execute": "block_resize", "arguments": {"node-name": "blk1", 
"size":32768}}
<- {"error": {"class": "GenericError", "desc": "Device '(null)' is in use"}}

Broken when commit 3b1dbd11a60 made @device optional.  Fixed in commit
ed3d2ec98a3 (block: Add errp to b{lk,drv}_truncate()), except for this
one instance.

Fix it by using the error message provided by the op blocker instead,
so it fails like this:

<- {"error": {"class": "GenericError", "desc": "Node 'blk1' is busy: block 
device is in use by block job: mirror"}}

Fixes: 3b1dbd11a60d (qmp: Allow block_resize to manipulate bs graph nodes.)
Signed-off-by: Markus Armbruster 
---
 blockdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f8bb0932f8..d8fb3399f5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2252,8 +2252,7 @@ void coroutine_fn qmp_block_resize(const char *device, 
const char *node_name,
 }
 
 bdrv_graph_co_rdlock();
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
-error_setg(errp, QERR_DEVICE_IN_USE, device);
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, errp)) {
 bdrv_graph_co_rdunlock();
 return;
 }
-- 
2.44.0




[PATCH 2/2] qerror: QERR_DEVICE_IN_USE is no longer used, drop

2024-03-06 Thread Markus Armbruster
Signed-off-by: Markus Armbruster 
---
 include/qapi/qmp/qerror.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 8dd9fcb071..0c2689cf8a 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -23,9 +23,6 @@
 #define QERR_DEVICE_HAS_NO_MEDIUM \
 "Device '%s' has no medium"
 
-#define QERR_DEVICE_IN_USE \
-"Device '%s' is in use"
-
 #define QERR_DEVICE_NO_HOTPLUG \
 "Device '%s' does not support hotplugging"
 
-- 
2.44.0




Re: [RFC 0/4] mirror: implement incremental and bitmap modes

2024-03-06 Thread Fiona Ebner
Am 29.02.24 um 13:47 schrieb Fiona Ebner:
> Am 29.02.24 um 12:48 schrieb Vladimir Sementsov-Ogievskiy:
>> On 29.02.24 13:11, Fiona Ebner wrote:
>>>
>>> The iotest creates a new target image for each incremental sync which
>>> only records the diff relative to the previous mirror and those diff
>>> images are later rebased onto each other to get the full picture.
>>>
>>> Thus, it can be that a previous mirror job (not just background process
>>> or previous write) already copied a cluster, and in particular, copied
>>> it to a different target!
>>
>> Aha understand.
>>
>> For simplicity, let's consider case, when source "cluster size" = "job
>> cluster size" = "bitmap granularity" = "target cluster size".
>>
>> Which types of clusters we should consider, when we want to handle guest
>> write?
>>
>> 1. Clusters, that should be copied by background process
>>
>> These are dirty clusters from user-given bitmap, or if we do a full-disk
>> mirror, all clusters, not yet copied by background process.
>>
>> For such clusters we simply ignore the unaligned write. We can even
>> ignore the aligned write too: less disturbing the guest by delays.
>>
> 
> Since do_sync_target_write() currently doesn't ignore aligned writes, I
> wouldn't change it. Of course they can count towards the "done_bitmap"
> you propose below.
> 
>> 2. Clusters, already copied by background process during this mirror job
>> and not dirtied by guest since this time.
>>
>> For such clusters we are safe to do unaligned write, as target cluster
>> must be allocated.
>>
> 
> Right.
> 
>> 3. Clusters, not marked initially by dirty bitmap.
>>
>> What to do with them? We can't do unaligned write. I see two variants:
>>
>> - do additional read from source, to fill the whole cluster, which seems
>> a bit too heavy
>>
> 
> Yes, I'd rather only do that as a last resort.
> 
>> - just mark the cluster as dirty for background job. So we behave like
>> in "background" mode. But why not? The maximum count of such "hacks" is
>> limited to number of "clear" clusters at start of mirror job, which
>> means that we don't seriously affect the convergence. Mirror is
>> guaranteed to converge anyway. And the whole sense of "write-blocking"
>> mode is to have a guaranteed convergence. What do you think?
>>
> 
> It could lead to a lot of flips between job->actively_synced == true and
> == false. AFAIU, currently, we only switch back from true to false when
> an error happens. While I don't see a concrete issue with it, at least
> it might be unexpected to users, so it better be documented.
> 
> I'll try going with this approach, thanks!
> 

These flips are actually a problem. When using live-migration with disk
mirroring, it's good that an actively synced image stays actively
synced. Otherwise, migration could finish at an inconvenient time and
try to inactivate the block device while mirror still got something to
do which would lead to an assertion failure [0].

The IO test added by this series is what uses the possibility to sync to
"diff images" which contain only the delta. In production, we are only
syncing to a previously mirrored target image. Non-aligned writes are
not an issue later like with a diff image. (Even if the initial
mirroring happened via ZFS replication outside of QEMU).

So copy-mode=write-blocking would work fine for our use case, but if I
go with the "mark clusters for unaligned writes dirty"-approach, it
would not work fine anymore.

Should I rather just document the limitation for the combination "target
is a diff image" and copy-mode=write-blocking?

I'd still add the check for the granularity and target cluster size.
While also only needed for diff images, it would allow using background
mode safely for those.

Best Regards,
Fiona

[0]:
https://lore.kernel.org/qemu-devel/1db7f571-cb7f-c293-04cc-cd856e060...@proxmox.com/




[PATCH] blockdev: Fix blockdev-snapshot-sync error reporting for no medium

2024-03-06 Thread Markus Armbruster
When external_snapshot_abort() rejects a BlockDriverState without a
medium, it creates an error like this:

error_setg(errp, "Device '%s' has no medium", device);

Trouble is @device can be null.  My system formats null as "(null)",
but other systems might crash.  Reproducer:

1. Create a block device without a medium

-> {"execute": "blockdev-add", "arguments": {"driver": "host_cdrom", 
"node-name": "blk0", "filename": "/dev/sr0"}}
<- {"return": {}}

3. Attempt to snapshot it

-> {"execute":"blockdev-snapshot-sync", "arguments": { "node-name": "blk0", 
"snapshot-file":"/tmp/foo.qcow2","format":"qcow2"}}
<- {"error": {"class": "GenericError", "desc": "Device '(null)' has no 
medium"}}

Broken when commit 0901f67ecdb made @device optional.

Use bdrv_get_device_or_node_name() instead.  Now it fails as it
should:

<- {"error": {"class": "GenericError", "desc": "Device 'blk0' has no 
medium"}}

Fixes: 0901f67ecdb7 (qmp: Allow to take external snapshots on bs graphs node.)
Signed-off-by: Markus Armbruster 
---
 blockdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/blockdev.c b/blockdev.c
index d8fb3399f5..057601dcf0 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1395,7 +1395,8 @@ static void external_snapshot_action(TransactionAction 
*action,
 bdrv_drained_begin(state->old_bs);
 
 if (!bdrv_is_inserted(state->old_bs)) {
-error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, device);
+error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM,
+   bdrv_get_device_or_node_name(state->old_bs));
 return;
 }
 
-- 
2.44.0




Re: [PATCH 1/2] blockdev: Fix block_resize error reporting for op blockers

2024-03-06 Thread Philippe Mathieu-Daudé

On 6/3/24 14:10, Markus Armbruster wrote:

When block_resize() runs into an op blocker, it creates an error like
this:

 error_setg(errp, "Device '%s' is in use", device);

Trouble is @device can be null.  My system formats null as "(null)",
but other systems might crash.  Reproducer:

1. Create two block devices

 -> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": "blk0", 
"filename": "64k.img"}}
 <- {"return": {}}
 -> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": "blk1", 
"filename": "m.img"}}
 <- {"return": {}}

2. Put a blocker on one them

 -> {"execute": "blockdev-mirror", "arguments": {"job-id": "job0", "device": "blk0", "target": 
"blk1", "sync": "full"}}
 {"return": {}}
 -> {"execute": "job-pause", "arguments": {"id": "job0"}}
 {"return": {}}
 -> {"execute": "job-complete", "arguments": {"id": "job0"}}
 {"return": {}}

Note: job events elided for brevity.

3. Attempt to resize

 -> {"execute": "block_resize", "arguments": {"node-name": "blk1", 
"size":32768}}
 <- {"error": {"class": "GenericError", "desc": "Device '(null)' is in 
use"}}

Broken when commit 3b1dbd11a60 made @device optional.  Fixed in commit
ed3d2ec98a3 (block: Add errp to b{lk,drv}_truncate()), except for this
one instance.

Fix it by using the error message provided by the op blocker instead,
so it fails like this:

 <- {"error": {"class": "GenericError", "desc": "Node 'blk1' is busy: block 
device is in use by block job: mirror"}}

Fixes: 3b1dbd11a60d (qmp: Allow block_resize to manipulate bs graph nodes.)
Signed-off-by: Markus Armbruster 
---
  blockdev.c | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH 2/2] qerror: QERR_DEVICE_IN_USE is no longer used, drop

2024-03-06 Thread Philippe Mathieu-Daudé

On 6/3/24 14:10, Markus Armbruster wrote:

Signed-off-by: Markus Armbruster 
---
  include/qapi/qmp/qerror.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 8dd9fcb071..0c2689cf8a 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -23,9 +23,6 @@
  #define QERR_DEVICE_HAS_NO_MEDIUM \
  "Device '%s' has no medium"
  
-#define QERR_DEVICE_IN_USE \

-"Device '%s' is in use"


\o/

Reviewed-by: Philippe Mathieu-Daudé 




Re: [PATCH-for-9.0 v2 10/19] hw/xen: Rename 'ram_memory' global variable as 'xen_memory'

2024-03-06 Thread Philippe Mathieu-Daudé

On 14/11/23 16:49, David Woodhouse wrote:

On 14 November 2023 09:38:06 GMT-05:00, "Philippe Mathieu-Daudé" 
 wrote:

To avoid a potential global variable shadow in
hw/i386/pc_piix.c::pc_init1(), rename Xen's
"ram_memory" as "xen_memory".

Signed-off-by: Philippe Mathieu-Daudé 


Well OK, but aren't you going to be coming back later to eliminate global 
variables which are actually per-VM?

Or is that the point, because *then* the conflicting name will actually matter?


Eh I wasn't thinking about running 2 Xen VMs in the same QEMU process,
but this is a good test case. I was thinking of running a Xen guest VM
and another TCG VM in the same process.

So IIUC xen_memory should be part of PCMachineState.



Reviewed-by: David Woodhouse 


Thanks!




Re: [RFC PATCH-for-9.0 v2 13/19] hw/xen: Remove use of 'target_ulong' in handle_ioreq()

2024-03-06 Thread Philippe Mathieu-Daudé

Cc'ing Anton.

On 14/11/23 15:38, Philippe Mathieu-Daudé wrote:

Per commit f17068c1c7 ("xen-hvm: reorganize xen-hvm and move common
function to xen-hvm-common"), handle_ioreq() is expected to be
target-agnostic. However it uses 'target_ulong', which is a target
specific definition.

Per xen/include/public/hvm/ioreq.h header:

   struct ioreq {
 uint64_t addr;  /* physical address */
 uint64_t data;  /* data (or paddr of data) */
 uint32_t count; /* for rep prefixes */
 uint32_t size;  /* size in bytes */
 uint32_t vp_eport;  /* evtchn for notifications to/from device model */
 uint16_t _pad0;
 uint8_t state:4;
 uint8_t data_is_ptr:1;  /* if 1, data above is the guest paddr
  * of the real data to use. */
 uint8_t dir:1;  /* 1=read, 0=write */
 uint8_t df:1;
 uint8_t _pad1:1;
 uint8_t type;   /* I/O type */
   };
   typedef struct ioreq ioreq_t;

If 'data' is not a pointer, it is a u64.

- In PIO / VMWARE_PORT modes, only 32-bit are used.

- In MMIO COPY mode, memory is accessed by chunks of 64-bit

- In PCI_CONFIG mode, access is u8 or u16 or u32.

- None of TIMEOFFSET / INVALIDATE use 'req'.

- Fallback is only used in x86 for VMWARE_PORT.

Masking the upper bits of 'data' to keep 'req->size' low bits
is irrelevant of the target word size. Remove the word size
check and always extract the relevant bits.

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/xen/xen-hvm-common.c | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index bb3cfb200c..fb81bd8fbc 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -1,5 +1,6 @@
  #include "qemu/osdep.h"
  #include "qemu/units.h"
+#include "qemu/bitops.h"
  #include "qapi/error.h"
  #include "trace.h"
  
@@ -426,9 +427,8 @@ static void handle_ioreq(XenIOState *state, ioreq_t *req)

  trace_handle_ioreq(req, req->type, req->dir, req->df, req->data_is_ptr,
 req->addr, req->data, req->count, req->size);
  
-if (!req->data_is_ptr && (req->dir == IOREQ_WRITE) &&

-(req->size < sizeof (target_ulong))) {
-req->data &= ((target_ulong) 1 << (8 * req->size)) - 1;
+if (!req->data_is_ptr && (req->dir == IOREQ_WRITE)) {
+req->data = extract64(req->data, 0, BITS_PER_BYTE * req->size);
  }
  
  if (req->dir == IOREQ_WRITE)





[PATCH v5 0/3] Initial support for SPDM Responders

2024-03-06 Thread Alistair Francis
The Security Protocol and Data Model (SPDM) Specification defines
messages, data objects, and sequences for performing message exchanges
over a variety of transport and physical media.
 - 
https://www.dmtf.org/sites/default/files/standards/documents/DSP0274_1.3.0.pdf

SPDM currently supports PCIe DOE and MCTP transports, but it can be
extended to support others in the future. This series adds
support to QEMU to connect to an external SPDM instance.

SPDM support can be added to any QEMU device by exposing a
TCP socket to a SPDM server. The server can then implement the SPDM
decoding/encoding support, generally using libspdm [1].

This is similar to how the current TPM implementation works and means
that the heavy lifting of setting up certificate chains, capabilities,
measurements and complex crypto can be done outside QEMU by a well
supported and tested library.

This series implements socket support and exposes SPDM for a NVMe device.

1: https://github.com/DMTF/libspdm

v5:
 - Update MAINTAINERS
v4:
 - Rebase
v3:
 - Spelling fixes
 - Support for SPDM-Utils
v2:
 - Add cover letter
 - A few code fixes based on comments
 - Document SPDM-Utils
 - A few tweaks and clarifications to the documentation

Alistair Francis (1):
  hw/pci: Add all Data Object Types defined in PCIe r6.0

Huai-Cheng Kuo (1):
  backends: Initial support for SPDM socket support

Wilfred Mallawa (1):
  hw/nvme: Add SPDM over DOE support

 MAINTAINERS  |   6 +
 docs/specs/index.rst |   1 +
 docs/specs/spdm.rst  | 122 
 include/hw/pci/pci_device.h  |   5 +
 include/hw/pci/pcie_doe.h|   5 +
 include/sysemu/spdm-socket.h |  44 +++
 backends/spdm-socket.c   | 216 +++
 hw/nvme/ctrl.c   |  53 +
 backends/Kconfig |   4 +
 backends/meson.build |   2 +
 10 files changed, 458 insertions(+)
 create mode 100644 docs/specs/spdm.rst
 create mode 100644 include/sysemu/spdm-socket.h
 create mode 100644 backends/spdm-socket.c

-- 
2.44.0




[PATCH v5 1/3] hw/pci: Add all Data Object Types defined in PCIe r6.0

2024-03-06 Thread Alistair Francis
Add all of the defined protocols/features from the PCIe-SIG r6.0
"Table 6-32 PCI-SIG defined Data Object Types (Vendor ID = 0001h)"
table.

Signed-off-by: Alistair Francis 
Reviewed-by: Jonathan Cameron 
---
 include/hw/pci/pcie_doe.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h
index 87dc17dcef..15d94661f9 100644
--- a/include/hw/pci/pcie_doe.h
+++ b/include/hw/pci/pcie_doe.h
@@ -46,6 +46,8 @@ REG32(PCI_DOE_CAP_STATUS, 0)
 
 /* PCI-SIG defined Data Object Types - r6.0 Table 6-32 */
 #define PCI_SIG_DOE_DISCOVERY   0x00
+#define PCI_SIG_DOE_CMA 0x01
+#define PCI_SIG_DOE_SECURED_CMA 0x02
 
 #define PCI_DOE_DW_SIZE_MAX (1 << 18)
 #define PCI_DOE_PROTOCOL_NUM_MAX256
-- 
2.44.0




[PATCH v5 2/3] backends: Initial support for SPDM socket support

2024-03-06 Thread Alistair Francis
From: Huai-Cheng Kuo 

SPDM enables authentication, attestation and key exchange to assist in
providing infrastructure security enablement. It's a standard published
by the DMTF [1].

SPDM supports multiple transports, including PCIe DOE and MCTP.
This patch adds support to QEMU to connect to an external SPDM
instance.

SPDM support can be added to any QEMU device by exposing a
TCP socket to a SPDM server. The server can then implement the SPDM
decoding/encoding support, generally using libspdm [2].

This is similar to how the current TPM implementation works and means
that the heavy lifting of setting up certificate chains, capabilities,
measurements and complex crypto can be done outside QEMU by a well
supported and tested library.

1: https://www.dmtf.org/standards/SPDM
2: https://github.com/DMTF/libspdm

Signed-off-by: Huai-Cheng Kuo 
Signed-off-by: Chris Browy 
Co-developed-by: Jonathan Cameron 
Signed-off-by: Jonathan Cameron 
[ Changes by WM
 - Bug fixes from testing
]
Signed-off-by: Wilfred Mallawa 
[ Changes by AF:
 - Convert to be more QEMU-ified
 - Move to backends as it isn't PCIe specific
]
Signed-off-by: Alistair Francis 
---
 MAINTAINERS  |   6 +
 include/sysemu/spdm-socket.h |  44 +++
 backends/spdm-socket.c   | 216 +++
 backends/Kconfig |   4 +
 backends/meson.build |   2 +
 5 files changed, 272 insertions(+)
 create mode 100644 include/sysemu/spdm-socket.h
 create mode 100644 backends/spdm-socket.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 4183f2f3ab..a07706c225 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3395,6 +3395,12 @@ F: tests/qtest/*tpm*
 F: docs/specs/tpm.rst
 T: git https://github.com/stefanberger/qemu-tpm.git tpm-next
 
+SPDM
+M: Alistair Francis 
+S: Maintained
+F: backends/spdm-socket.c
+F: include/sysemu/spdm-socket.h
+
 Checkpatch
 S: Odd Fixes
 F: scripts/checkpatch.pl
diff --git a/include/sysemu/spdm-socket.h b/include/sysemu/spdm-socket.h
new file mode 100644
index 00..24e6fccb83
--- /dev/null
+++ b/include/sysemu/spdm-socket.h
@@ -0,0 +1,44 @@
+/*
+ * QEMU SPDM socket support
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef SPDM_REQUESTER_H
+#define SPDM_REQUESTER_H
+
+int spdm_socket_connect(uint16_t port, Error **errp);
+uint32_t spdm_socket_rsp(const int socket, uint32_t transport_type,
+ void *req, uint32_t req_len,
+ void *rsp, uint32_t rsp_len);
+void spdm_socket_close(const int socket, uint32_t transport_type);
+
+#define SPDM_SOCKET_COMMAND_NORMAL0x0001
+#define SPDM_SOCKET_COMMAND_OOB_ENCAP_KEY_UPDATE  0x8001
+#define SPDM_SOCKET_COMMAND_CONTINUE  0xFFFD
+#define SPDM_SOCKET_COMMAND_SHUTDOWN  0xFFFE
+#define SPDM_SOCKET_COMMAND_UNKOWN0x
+#define SPDM_SOCKET_COMMAND_TEST  0xDEAD
+
+#define SPDM_SOCKET_TRANSPORT_TYPE_MCTP   0x01
+#define SPDM_SOCKET_TRANSPORT_TYPE_PCI_DOE0x02
+
+#define SPDM_SOCKET_MAX_MESSAGE_BUFFER_SIZE   0x1200
+
+#endif
diff --git a/backends/spdm-socket.c b/backends/spdm-socket.c
new file mode 100644
index 00..d0663d696c
--- /dev/null
+++ b/backends/spdm-socket.c
@@ -0,0 +1,216 @@
+/* SPDX-License-Identifier: BSD-3-Clause */
+/*
+ * QEMU SPDM socket support
+ *
+ * This is based on:
+ * 
https://github.com/DMTF/spdm-emu/blob/07c0a838bcc1c6207c656ac75885c0603e344b6f/spdm_emu/spdm_emu_common/command.c
+ * but has been re-written to match QEMU style
+ *
+ * Copyright (c) 2021, DMTF. All rights reserved.
+ * Copyright (c) 2023. Western Digital Corporation or its affiliates.
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/spdm-socket.h"
+#include "qapi/error.h"
+
+static bool read_bytes(const int socket, uint8_t *buffer,
+   size_t number_of_bytes)
+{
+ssize_t number_received = 0;
+ssize_t result;
+
+while (number_received < number_of_

[PATCH v5 3/3] hw/nvme: Add SPDM over DOE support

2024-03-06 Thread Alistair Francis
From: Wilfred Mallawa 

Setup Data Object Exchance (DOE) as an extended capability for the NVME
controller and connect SPDM to it (CMA) to it.

Signed-off-by: Wilfred Mallawa 
Signed-off-by: Alistair Francis 
Reviewed-by: Jonathan Cameron 
Acked-by: Klaus Jensen 
---
 docs/specs/index.rst|   1 +
 docs/specs/spdm.rst | 122 
 include/hw/pci/pci_device.h |   5 ++
 include/hw/pci/pcie_doe.h   |   3 +
 hw/nvme/ctrl.c  |  53 
 5 files changed, 184 insertions(+)
 create mode 100644 docs/specs/spdm.rst

diff --git a/docs/specs/index.rst b/docs/specs/index.rst
index 1484e3e760..e2d907959a 100644
--- a/docs/specs/index.rst
+++ b/docs/specs/index.rst
@@ -29,6 +29,7 @@ guest hardware that is specific to QEMU.
edu
ivshmem-spec
pvpanic
+   spdm
standard-vga
virt-ctlr
vmcoreinfo
diff --git a/docs/specs/spdm.rst b/docs/specs/spdm.rst
new file mode 100644
index 00..4d0942c1ad
--- /dev/null
+++ b/docs/specs/spdm.rst
@@ -0,0 +1,122 @@
+==
+QEMU Security Protocols and Data Models (SPDM) Support
+==
+
+SPDM enables authentication, attestation and key exchange to assist in
+providing infrastructure security enablement. It's a standard published
+by the `DMTF`_.
+
+QEMU supports connecting to a SPDM responder implementation. This allows an
+external application to emulate the SPDM responder logic for an SPDM device.
+
+Setting up a SPDM server
+
+
+When using QEMU with SPDM devices QEMU will connect to a server which
+implements the SPDM functionality.
+
+SPDM-Utils
+--
+
+You can use `SPDM Utils`_ to emulate a responder. This is the simplest method.
+
+SPDM-Utils is a Linux applications to manage, test and develop devices
+supporting DMTF Security Protocol and Data Model (SPDM). It is written in Rust
+and utilises libspdm.
+
+To use SPDM-Utils you will need to do the following steps. Details are included
+in the SPDM-Utils README.
+
+ 1. `Build libspdm`_
+ 2. `Build SPDM Utils using Cargo`_
+ 3. `Run it as a server`_
+
+spdm-emu
+
+
+You can use `spdm emu`_ to model the
+SPDM responder.
+
+.. code-block:: shell
+
+$ cd spdm-emu
+$ git submodule init; git submodule update --recursive
+$ mkdir build; cd build
+$ cmake -DARCH=x64 -DTOOLCHAIN=GCC -DTARGET=Debug -DCRYPTO=openssl ..
+$ make -j32
+$ make copy_sample_key # Build certificates, required for SPDM 
authentication.
+
+It is worth noting that the certificates should be in compliance with
+PCIe r6.1 sec 6.31.3. This means you will need to add the following to
+openssl.cnf
+
+.. code-block::
+
+subjectAltName = 
otherName:2.23.147;UTF8:Vendor=1b36:Device=0010:CC=010802:REV=02:SSVID=1af4:SSID=1100
+2.23.147 = ASN1:OID:2.23.147
+
+and then manually regenerate some certificates with:
+
+.. code-block:: shell
+
+$ openssl req -nodes -newkey ec:param.pem -keyout end_responder.key \
+-out end_responder.req -sha384 -batch \
+-subj "/CN=DMTF libspdm ECP384 responder cert"
+
+$ openssl x509 -req -in end_responder.req -out end_responder.cert \
+-CA inter.cert -CAkey inter.key -sha384 -days 3650 -set_serial 3 \
+-extensions v3_end -extfile ../openssl.cnf
+
+$ openssl asn1parse -in end_responder.cert -out end_responder.cert.der
+
+$ cat ca.cert.der inter.cert.der end_responder.cert.der > 
bundle_responder.certchain.der
+
+You can use SPDM-Utils instead as it will generate the correct certificates
+automatically.
+
+The responder can then be launched with
+
+.. code-block:: shell
+
+$ cd bin
+$ ./spdm_responder_emu --trans PCI_DOE
+
+Connecting an SPDM NVMe device
+==
+
+Once a SPDM server is running we can start QEMU and connect to the server.
+
+For an NVMe device first let's setup a block we can use
+
+.. code-block:: shell
+
+$ cd qemu-spdm/linux/image
+$ dd if=/dev/zero of=blknvme bs=1M count=2096 # 2GB NNMe Drive
+
+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=2323
+
+At which point QEMU will try to connect to the SPDM server.
+
+
+.. _DMTF:
+   https://www.dmtf.org/standards/SPDM
+
+.. _SPDM Utils:
+   https://github.com/westerndigitalcorporation/spdm-utils
+
+.. _spdm emu:
+   https://github.com/dmtf/spdm-emu
+
+.. _Build SPDM Utils:
+   https://github.com/westerndigitalcorporation/spdm-utils#building
+
+.. _Generate the certificates:
+   
https://github.com/westerndigitalcorporation/spdm-utils#generate-mutable-certificates
+
+.. _Run it as a server:
+   
https://github.com/westerndigitalcorporation/spdm-utils#qemu-spdm-device-emulation
diff --git a/include/hw/pci/pci_device.h b/include/hw/pci/pci_device.h
index d3dd0f64b2..b8379c78f1 100644
--- a/include/hw/pci/pci_de