date:20230711

On Tue, Jul 11, 2023 at 04:49:22PM +0200, Philippe Mathieu-Daudé wrote:
> Get an up-to-date package list from lcitool, that way we
> don't need to manually keep this array in sync.
> 
> Inspired-by: Thomas Huth 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/vm/freebsd | 42 ++
>  1 file changed, 2 insertions(+), 40 deletions(-)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v3 3/4] tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper

On Tue, Jul 11, 2023 at 04:49:21PM +0200, Philippe Mathieu-Daudé wrote:
> Add the get_qemu_packages_from_lcitool_json() helper which return
> such package list from a lcitool env var file in JSON format.
> 
> Suggested-by: Daniel P. Berrangé 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/vm/basevm.py | 11 +++
>  1 file changed, 11 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v3 2/4] tests/lcitool: Refresh generated files

On Tue, Jul 11, 2023 at 04:49:20PM +0200, Philippe Mathieu-Daudé wrote:
> Refresh the generated files by running:
> 
>   $ make lcitool-refresh
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/docker/dockerfiles/debian-amd64.docker |  2 -
>  tests/docker/dockerfiles/ubuntu2004.docker   |  2 -
>  tests/docker/dockerfiles/ubuntu2204.docker   |  2 -

I don't know why this would be removing xen/pmem from these files. If
I run 'lcitool-refresh' on current git master that doesn't happen

Do you have any other local changes on top ?

>  tests/vm/generated/freebsd.json  | 77 
>  4 files changed, 77 insertions(+), 6 deletions(-)
>  create mode 100644 tests/vm/generated/freebsd.json

Reviewed-by: Daniel P. Berrangé 


> 
> diff --git a/tests/docker/dockerfiles/debian-amd64.docker 
> b/tests/docker/dockerfiles/debian-amd64.docker
> index e39871c7bb..8f7521fdc2 100644
> --- a/tests/docker/dockerfiles/debian-amd64.docker
> +++ b/tests/docker/dockerfiles/debian-amd64.docker
> @@ -70,7 +70,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>libpam0g-dev \
>libpcre2-dev \
>libpixman-1-dev \
> -  libpmem-dev \
>libpng-dev \
>libpulse-dev \
>librbd-dev \
> @@ -96,7 +95,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>libvdeplug-dev \
>libvirglrenderer-dev \
>libvte-2.91-dev \
> -  libxen-dev \
>libzstd-dev \
>llvm \
>locales \
> diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
> b/tests/docker/dockerfiles/ubuntu2004.docker
> index 8f864d19e6..7f60143cbb 100644
> --- a/tests/docker/dockerfiles/ubuntu2004.docker
> +++ b/tests/docker/dockerfiles/ubuntu2004.docker
> @@ -69,7 +69,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>libpam0g-dev \
>libpcre2-dev \
>libpixman-1-dev \
> -  libpmem-dev \
>libpng-dev \
>libpulse-dev \
>librbd-dev \
> @@ -94,7 +93,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>libvdeplug-dev \
>libvirglrenderer-dev \
>libvte-2.91-dev \
> -  libxen-dev \
>libzstd-dev \
>llvm \
>locales \
> diff --git a/tests/docker/dockerfiles/ubuntu2204.docker 
> b/tests/docker/dockerfiles/ubuntu2204.docker
> index 1d442cdfe6..5162927016 100644
> --- a/tests/docker/dockerfiles/ubuntu2204.docker
> +++ b/tests/docker/dockerfiles/ubuntu2204.docker
> @@ -70,7 +70,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>libpam0g-dev \
>libpcre2-dev \
>libpixman-1-dev \
> -  libpmem-dev \
>libpng-dev \
>libpulse-dev \
>librbd-dev \
> @@ -96,7 +95,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
>libvdeplug-dev \
>libvirglrenderer-dev \
>libvte-2.91-dev \
> -  libxen-dev \
>libzstd-dev \
>llvm \
>locales \
> diff --git a/tests/vm/generated/freebsd.json b/tests/vm/generated/freebsd.json
> new file mode 100644
> index 00..7c435cf23e
> --- /dev/null
> +++ b/tests/vm/generated/freebsd.json
> @@ -0,0 +1,77 @@
> +{
> +  "ccache": "/usr/local/bin/ccache",
> +  "cpan_pkgs": [],
> +  "cross_pkgs": [],
> +  "make": "/usr/local/bin/gmake",
> +  "ninja": "/usr/local/bin/ninja",
> +  "packaging_command": "pkg",
> +  "pip3": "/usr/local/bin/pip-3.8",
> +  "pkgs": [
> +"alsa-lib",
> +"bash",
> +"bison",
> +"bzip2",
> +"ca_root_nss",
> +"capstone4",
> +"ccache",
> +"cmocka",
> +"ctags",
> +"curl",
> +"cyrus-sasl",
> +"dbus",
> +"diffutils",
> +"dtc",
> +"flex",
> +"fusefs-libs3",
> +"gettext",
> +"git",
> +"glib",
> +"gmake",
> +"gnutls",
> +"gsed",
> +"gtk3",
> +"json-c",
> +"libepoxy",
> +"libffi",
> +"libgcrypt",
> +"libjpeg-turbo",
> +"libnfs",
> +"libslirp",
> +"libspice-server",
> +"libssh",
> +"libtasn1",
> +"llvm",
> +"lzo2",
> +"meson",
> +"mtools",
> +"ncurses",
> +"nettle",
> +"ninja",
> +"opencv",
> +"pixman",
> +"pkgconf",
> +"png",
> +"py39-numpy",
> +"py39-pillow",
> +"py39-pip",
> +"py39-sphinx",
> +"py39-sphinx_rtd_theme",
> +"py39-yaml",
> +"python3",
> +"rpm2cpio",
> +"sdl2",
> +

Re: [PATCH 2/3] block/io: align requests to subcluster_size

2023-07-11 Thread Andrey Drobyshev

On 7/10/23 22:47, Eric Blake wrote:
> On Mon, Jun 26, 2023 at 07:08:33PM +0300, Andrey Drobyshev via wrote:
>> When target image is using subclusters, and we align the request during
>> copy-on-read, it makes sense to align to subcluster_size rather than
>> cluster_size.  Otherwise we end up with unnecessary allocations.
>>
>> This commit renames bdrv_round_to_clusters() to bdrv_round_to_subclusters()
>> and utilizes subcluster_size field of BlockDriverInfo to make necessary
>> alignments.  It affects copy-on-read as well as mirror job (which is
>> using bdrv_round_to_clusters()).
>>
>> This change also fixes the following bug with failing assert (covered by
>> the test in the subsequent commit):
>>
>> qemu-img create -f qcow2 base.qcow2 64K
>> qemu-img create -f qcow2 -o 
>> extended_l2=on,backing_file=base.qcow2,backing_fmt=qcow2 img.qcow2 64K
>> qemu-io -c "write -P 0xaa 0 2K" img.qcow2
>> qemu-io -C -c "read -P 0x00 2K 62K" img.qcow2
>>
>> qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion `skip_bytes 
>> < pnum' failed.
>>
>> Signed-off-by: Andrey Drobyshev 
>> ---
>>  block/io.c   | 50 
>>  block/mirror.c   |  8 +++
>>  include/block/block-io.h |  2 +-
>>  3 files changed, 30 insertions(+), 30 deletions(-)
>>
>> +++ b/include/block/block-io.h
>> @@ -189,7 +189,7 @@ bdrv_get_info(BlockDriverState *bs, BlockDriverInfo 
>> *bdi);
>>  ImageInfoSpecific *bdrv_get_specific_info(BlockDriverState *bs,
>>Error **errp);
>>  BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs);
>> -void bdrv_round_to_clusters(BlockDriverState *bs,
>> +void bdrv_round_to_subclusters(BlockDriverState *bs,
>>  int64_t offset, int64_t bytes,
>>  int64_t *cluster_offset,
>>  int64_t *cluster_bytes);
> 
> Indentation on subsequent lines should be fixed.

Thanks for pointing that out, got it fixed in v2:
https://lists.nongnu.org/archive/html/qemu-block/2023-07/msg00182.html

> 
> Reviewed-by: Eric Blake 
> 
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
>

Re: [PATCH] Revert "virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events"

2023-07-11 Thread Stefan Hajnoczi

On Tue, 11 Jul 2023 at 13:06, Stefano Garzarella  wrote:
>
> CCing `./scripts/get_maintainer.pl -f drivers/scsi/virtio_scsi.c`,
> since I found a few things in the virtio-scsi driver...
>
> FYI we have seen that Linux has problems with a QEMU patch for the
> virtio-scsi device (details at the bottom of this email in the revert
> commit message and BZ).
>
>
> This is what I found when I looked at the Linux code:
>
> In scsi_report_sense() in linux/drivers/scsi/scsi_error.c linux calls
> scsi_report_lun_change() that set `sdev_target->expecting_lun_change =
> 1` when we receive a UNIT ATTENTION with REPORT LUNS CHANGED
> (sshdr->asc == 0x3f && sshdr->ascq == 0x0e).
>
> When `sdev_target->expecting_lun_change = 1` is set and we call
> scsi_check_sense(), for example to check the next UNIT ATTENTION, it
> will return NEEDS_RETRY, that I think will cause the issues we are
> seeing.
>
> `sdev_target->expecting_lun_change` is reset only in
> scsi_decide_disposition() when `REPORT_LUNS` command returns with
> SAM_STAT_GOOD.
> That command is issued in scsi_report_lun_scan() called by
> __scsi_scan_target(), called for example by scsi_scan_target(),
> scsi_scan_host(), etc.
>
> So, checking QEMU, we send VIRTIO_SCSI_EVT_RESET_RESCAN during hotplug
> and VIRTIO_SCSI_EVT_RESET_REMOVED during hotunplug. In both cases now we
> send also the UNIT ATTENTION.
>
> In the virtio-scsi driver, when we receive VIRTIO_SCSI_EVT_RESET_RESCAN
> (hotplug) we call scsi_scan_target() or scsi_add_device(). Both of them
> will call __scsi_scan_target() at some points, sending `REPORT_LUNS`
> command to the device. This does not happen for
> VIRTIO_SCSI_EVT_RESET_REMOVED (hotunplug). Indeed if I remove the
> UNIT ATTENTION from the hotunplug in QEMU, everything works well.
>
> So, I tried to add a scan also for VIRTIO_SCSI_EVT_RESET_REMOVED:
>
> diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
> index bd5633667d01..c57658a63097 100644
> --- a/drivers/scsi/virtio_scsi.c
> +++ b/drivers/scsi/virtio_scsi.c
> @@ -291,6 +291,7 @@ static void virtscsi_handle_transport_reset(struct 
> virtio_scsi *vscsi,
>  }
>  break;
>  case VIRTIO_SCSI_EVT_RESET_REMOVED:
> +   scsi_scan_host(shost);
>  sdev = scsi_device_lookup(shost, 0, target, lun);
>  if (sdev) {
>  scsi_remove_device(sdev);
>
> This somehow helps, now linux only breaks if the plug/unplug frequency
> is really high. If I put a 5 second sleep between plug/unplug events, it
> doesn't break (at least for the duration of my test which has been
> running for about 30 minutes, before it used to break after about a
> minute).
>
> Another thing I noticed is that in QEMU maybe we should set the UNIT
> ATTENTION first and then send the event on the virtqueue, because the
> scan should happen after the unit attention, but I don't know if in any
> case the unit attention is processed before the virtqueue.
>
> I mean something like this:
>
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 45b95ea070..13db40f4f3 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -1079,8 +1079,8 @@ static void virtio_scsi_hotplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>   };
>
>   virtio_scsi_acquire(s);
> -virtio_scsi_push_event(s, );
>   scsi_bus_set_ua(>bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
> +virtio_scsi_push_event(s, );
>   virtio_scsi_release(s);
>   }
>   }
> @@ -,8 +,8 @@ static void virtio_scsi_hotunplug(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
>
>   if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
>   virtio_scsi_acquire(s);
> -virtio_scsi_push_event(s, );
>   scsi_bus_set_ua(>bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
> +virtio_scsi_push_event(s, );
>   virtio_scsi_release(s);
>   }
>   }

That is racy. It's up to the guest whether the event virtqueue or the
UNIT ATTENTION will be processed first.

If the device wants to ensure ordering then it must withhold the event
until the driver has responded to the UNIT ATTENTION. That may not be
a good idea though.

I'd like to understand the root cause before choosing a solution.

> At this point I think the problem is on the handling of the
> VIRTIO_SCSI_EVT_RESET_REMOVED event in the virtio-scsi driver, where
> somehow we have to redo the bus scan, but scsi_scan_host() doesn't seem
> to be enough when the event rate is very high.

Why is it necessary to rescan the whole bus instead of removing just
the device that has been unplugged?

> I don't know if along with this fix, we also need to limit the rate in
> QEMU somehow.

Why is a high rate problematic?

> Sorry for the length of this email, but I'm not familiar with SCSI and
> wanted some suggestions on how to proceed.
>
> Paolo, Stefan, Linux SCSI maintainers, any suggestion?

I don't know the Linux SCSI

[PATCH v2 0/3] block: align CoR requests to subclusters

v1 --> v2:
 * Fixed line indentation;
 * Fixed wording in a comment;
 * Added R-b.

v1: https://lists.nongnu.org/archive/html/qemu-block/2023-06/msg00606.html

Andrey Drobyshev (3):
  block: add subcluster_size field to BlockDriverInfo
  block/io: align requests to subcluster_size
  tests/qemu-iotests/197: add testcase for CoR with subclusters

 block.c  |  7 +
 block/io.c   | 50 ++--
 block/mirror.c   |  8 +++---
 block/qcow2.c|  1 +
 include/block/block-common.h |  5 
 include/block/block-io.h |  8 +++---
 tests/qemu-iotests/197   | 29 +
 tests/qemu-iotests/197.out   | 24 +
 8 files changed, 99 insertions(+), 33 deletions(-)

-- 
2.39.3

[PATCH v2 2/3] block/io: align requests to subcluster_size

When target image is using subclusters, and we align the request during
copy-on-read, it makes sense to align to subcluster_size rather than
cluster_size.  Otherwise we end up with unnecessary allocations.

This commit renames bdrv_round_to_clusters() to bdrv_round_to_subclusters()
and utilizes subcluster_size field of BlockDriverInfo to make necessary
alignments.  It affects copy-on-read as well as mirror job (which is
using bdrv_round_to_clusters()).

This change also fixes the following bug with failing assert (covered by
the test in the subsequent commit):

qemu-img create -f qcow2 base.qcow2 64K
qemu-img create -f qcow2 -o 
extended_l2=on,backing_file=base.qcow2,backing_fmt=qcow2 img.qcow2 64K
qemu-io -c "write -P 0xaa 0 2K" img.qcow2
qemu-io -C -c "read -P 0x00 2K 62K" img.qcow2

qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion `skip_bytes < 
pnum' failed.

Reviewed-by: Eric Blake 
Reviewed-by: Denis V. Lunev 
Signed-off-by: Andrey Drobyshev 
---
 block/io.c   | 50 
 block/mirror.c   |  8 +++
 include/block/block-io.h |  8 +++
 3 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/block/io.c b/block/io.c
index e8293d6b26..4a2fef6a77 100644
--- a/block/io.c
+++ b/block/io.c
@@ -728,21 +728,21 @@ BdrvTrackedRequest *coroutine_fn 
bdrv_co_get_self_request(BlockDriverState *bs)
 }
 
 /**
- * Round a region to cluster boundaries
+ * Round a region to subcluster (if supported) or cluster boundaries
  */
 void coroutine_fn GRAPH_RDLOCK
-bdrv_round_to_clusters(BlockDriverState *bs, int64_t offset, int64_t bytes,
-   int64_t *cluster_offset, int64_t *cluster_bytes)
+bdrv_round_to_subclusters(BlockDriverState *bs, int64_t offset, int64_t bytes,
+  int64_t *align_offset, int64_t *align_bytes)
 {
 BlockDriverInfo bdi;
 IO_CODE();
-if (bdrv_co_get_info(bs, ) < 0 || bdi.cluster_size == 0) {
-*cluster_offset = offset;
-*cluster_bytes = bytes;
+if (bdrv_co_get_info(bs, ) < 0 || bdi.subcluster_size == 0) {
+*align_offset = offset;
+*align_bytes = bytes;
 } else {
-int64_t c = bdi.cluster_size;
-*cluster_offset = QEMU_ALIGN_DOWN(offset, c);
-*cluster_bytes = QEMU_ALIGN_UP(offset - *cluster_offset + bytes, c);
+int64_t c = bdi.subcluster_size;
+*align_offset = QEMU_ALIGN_DOWN(offset, c);
+*align_bytes = QEMU_ALIGN_UP(offset - *align_offset + bytes, c);
 }
 }
 
@@ -1168,8 +1168,8 @@ bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t 
offset, int64_t bytes,
 void *bounce_buffer = NULL;
 
 BlockDriver *drv = bs->drv;
-int64_t cluster_offset;
-int64_t cluster_bytes;
+int64_t align_offset;
+int64_t align_bytes;
 int64_t skip_bytes;
 int ret;
 int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
@@ -1203,28 +1203,28 @@ bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t 
offset, int64_t bytes,
  * BDRV_REQUEST_MAX_BYTES (even when the original read did not), which
  * is one reason we loop rather than doing it all at once.
  */
-bdrv_round_to_clusters(bs, offset, bytes, _offset, _bytes);
-skip_bytes = offset - cluster_offset;
+bdrv_round_to_subclusters(bs, offset, bytes, _offset, _bytes);
+skip_bytes = offset - align_offset;
 
 trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,
-   cluster_offset, cluster_bytes);
+   align_offset, align_bytes);
 
-while (cluster_bytes) {
+while (align_bytes) {
 int64_t pnum;
 
 if (skip_write) {
 ret = 1; /* "already allocated", so nothing will be copied */
-pnum = MIN(cluster_bytes, max_transfer);
+pnum = MIN(align_bytes, max_transfer);
 } else {
-ret = bdrv_is_allocated(bs, cluster_offset,
-MIN(cluster_bytes, max_transfer), );
+ret = bdrv_is_allocated(bs, align_offset,
+MIN(align_bytes, max_transfer), );
 if (ret < 0) {
 /*
  * Safe to treat errors in querying allocation as if
  * unallocated; we'll probably fail again soon on the
  * read, but at least that will set a decent errno.
  */
-pnum = MIN(cluster_bytes, max_transfer);
+pnum = MIN(align_bytes, max_transfer);
 }
 
 /* Stop at EOF if the image ends in the middle of the cluster */
@@ -1242,7 +1242,7 @@ bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t 
offset, int64_t bytes,
 /* Must copy-on-read; use the bounce buffer */
 pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
 if (!bounce_buffer) {
-int64_t max_we_need = MAX(pnum, cluster_bytes - pnum);
+int64_t max_we_need = MAX(pnum, align_bytes - pnum);

[PATCH v2 1/3] block: add subcluster_size field to BlockDriverInfo

This is going to be used in the subsequent commit as requests alignment
(in particular, during copy-on-read).  This value only makes sense for
the formats which support subclusters (currently QCOW2 only).  If this
field isn't set by driver's own bdrv_get_info() implementation, we
simply set it equal to the cluster size thus treating each cluster as
having a single subcluster.

Reviewed-by: Eric Blake 
Reviewed-by: Denis V. Lunev 
Signed-off-by: Andrey Drobyshev 
---
 block.c  | 7 +++
 block/qcow2.c| 1 +
 include/block/block-common.h | 5 +
 3 files changed, 13 insertions(+)

diff --git a/block.c b/block.c
index a307c151a8..0af890f647 100644
--- a/block.c
+++ b/block.c
@@ -6480,6 +6480,13 @@ int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
 }
 memset(bdi, 0, sizeof(*bdi));
 ret = drv->bdrv_co_get_info(bs, bdi);
+if (bdi->subcluster_size == 0) {
+/*
+ * If the driver left this unset, subclusters are not supported.
+ * Then it is safe to treat each cluster as having only one subcluster.
+ */
+bdi->subcluster_size = bdi->cluster_size;
+}
 if (ret < 0) {
 return ret;
 }
diff --git a/block/qcow2.c b/block/qcow2.c
index c51388e99d..b48cd9ce63 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5197,6 +5197,7 @@ qcow2_co_get_info(BlockDriverState *bs, BlockDriverInfo 
*bdi)
 {
 BDRVQcow2State *s = bs->opaque;
 bdi->cluster_size = s->cluster_size;
+bdi->subcluster_size = s->subcluster_size;
 bdi->vm_state_offset = qcow2_vm_state_offset(s);
 bdi->is_dirty = s->incompatible_features & QCOW2_INCOMPAT_DIRTY;
 return 0;
diff --git a/include/block/block-common.h b/include/block/block-common.h
index e15395f2cb..df5ffc8d09 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -132,6 +132,11 @@ typedef struct BlockZoneWps {
 typedef struct BlockDriverInfo {
 /* in bytes, 0 if irrelevant */
 int cluster_size;
+/*
+ * A fraction of cluster_size, if supported (currently QCOW2 only); if
+ * disabled or unsupported, set equal to cluster_size.
+ */
+int subcluster_size;
 /* offset at which the VM state can be saved (0 if not possible) */
 int64_t vm_state_offset;
 bool is_dirty;
-- 
2.39.3

[PATCH v2 3/3] tests/qemu-iotests/197: add testcase for CoR with subclusters

Add testcase which checks that allocations during copy-on-read are
performed on the subcluster basis when subclusters are enabled in target
image.

This testcase also triggers the following assert with previous commit
not being applied, so we check that as well:

qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion `skip_bytes < 
pnum' failed.

Reviewed-by: Eric Blake 
Reviewed-by: Denis V. Lunev 
Signed-off-by: Andrey Drobyshev 
---
 tests/qemu-iotests/197 | 29 +
 tests/qemu-iotests/197.out | 24 
 2 files changed, 53 insertions(+)

diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
index a2547bc280..f07a9da136 100755
--- a/tests/qemu-iotests/197
+++ b/tests/qemu-iotests/197
@@ -122,6 +122,35 @@ $QEMU_IO -f qcow2 -C -c 'read 0 1024' "$TEST_WRAP" | 
_filter_qemu_io
 $QEMU_IO -f qcow2 -c map "$TEST_WRAP"
 _check_test_img
 
+echo
+echo '=== Copy-on-read with subclusters ==='
+echo
+
+# Create base and top images 64K (1 cluster) each.  Make subclusters enabled
+# for the top image
+_make_test_img 64K
+IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \
+_make_test_img --no-opts -o extended_l2=true -F "$IMGFMT" -b "$TEST_IMG" \
+64K | _filter_img_create
+
+$QEMU_IO -c "write -P 0xaa 0 64k" "$TEST_IMG" | _filter_qemu_io
+
+# Allocate individual subclusters in the top image, and not the whole cluster
+$QEMU_IO -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K" "$TEST_WRAP" \
+| _filter_qemu_io
+
+# Only 2 subclusters should be allocated in the top image at this point
+$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+
+# Actual copy-on-read operation
+$QEMU_IO -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io
+
+# And here we should have 4 subclusters allocated right in the middle of the
+# top image. Make sure the whole cluster remains unallocated
+$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+
+_check_test_img
+
 # success, all done
 echo '*** done'
 status=0
diff --git a/tests/qemu-iotests/197.out b/tests/qemu-iotests/197.out
index ad414c3b0e..8f34a30afe 100644
--- a/tests/qemu-iotests/197.out
+++ b/tests/qemu-iotests/197.out
@@ -31,4 +31,28 @@ read 1024/1024 bytes at offset 0
 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 1 KiB (0x400) bytes allocated at offset 0 bytes (0x0)
 No errors were found on the image.
+
+=== Copy-on-read with subclusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
+Formatting 'TEST_DIR/t.wrap.IMGFMT', fmt=IMGFMT size=65536 
backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 28672
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 34816
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset  Length  File
+0   0x7000  TEST_DIR/t.IMGFMT
+0x7000  0x800   TEST_DIR/t.wrap.IMGFMT
+0x7800  0x1000  TEST_DIR/t.IMGFMT
+0x8800  0x800   TEST_DIR/t.wrap.IMGFMT
+0x9000  0x7000  TEST_DIR/t.IMGFMT
+read 4096/4096 bytes at offset 30720
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset  Length  File
+0   0x7000  TEST_DIR/t.IMGFMT
+0x7000  0x2000  TEST_DIR/t.wrap.IMGFMT
+0x9000  0x7000  TEST_DIR/t.IMGFMT
+No errors were found on the image.
 *** done
-- 
2.39.3

Re: [PATCH for-8.1] accel/tcg: Zero-pad PC in TCG CPU exec trace lines

2023-07-11 Thread Anton Johansson via


In commit f0a08b0913befbd we changed the type of the PC from
target_ulong to vaddr.  In doing so we inadvertently dropped the
zero-padding on the PC in trace lines (the second item inside the []
in these lines).  They used to look like this on AArch64, for
instance:

Trace 0: 0x7f226100 [/4000/0061/ff20]

and now they look like this:
Trace 0: 0x7f4f5100 [/4000/0061/ff20]

and if the PC happens to be somewhere low like 0x5000
then the field is shown as /5000/.

This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier,
depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64
with no width specifier.

Restore the zero-padding by adding an 016 width specifier to
this tracing and a couple of others that were similarly recently
changed to use VADDR_PRIx without a width specifier.

We can't unfortunately restore the "32-bit guests are padded to
8 hex digits and 64-bit guests to 16 hex digits" behaviour so
easily.

Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr")
Signed-off-by: Peter Maydell 
---
I have a workflow that parses log files to get the executed
PC values; I don't suppose I'm the only one doing that...
---
  accel/tcg/cpu-exec.c  | 4 ++--
  accel/tcg/translate-all.c | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index ba1890a373d..db1e82811fa 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -298,7 +298,7 @@ static void log_cpu_exec(vaddr pc, CPUState *cpu,
  if (qemu_log_in_addr_range(pc)) {
  qemu_log_mask(CPU_LOG_EXEC,
"Trace %d: %p [%08" PRIx64
-  "/%" VADDR_PRIx "/%08x/%08x] %s\n",
+  "/%016" VADDR_PRIx "/%08x/%08x] %s\n",
cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc,
tb->flags, tb->cflags, lookup_symbol(pc));
  
@@ -487,7 +487,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int *tb_exit)

  if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
  vaddr pc = log_pc(cpu, last_tb);
  if (qemu_log_in_addr_range(pc)) {
-qemu_log("Stopped execution of TB chain before %p [%"
+qemu_log("Stopped execution of TB chain before %p [%016"
   VADDR_PRIx "] %s\n",
   last_tb->tc.ptr, pc, lookup_symbol(pc));
  }
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index d3d4fbc1a41..bb225afa04f 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -604,7 +604,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
  if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
  vaddr pc = log_pc(cpu, tb);
  if (qemu_log_in_addr_range(pc)) {
-qemu_log("cpu_io_recompile: rewound execution of TB to %"
+qemu_log("cpu_io_recompile: rewound execution of TB to %016"
   VADDR_PRIx "\n", pc);
  }
  }

This was definitely a mistake on my part, thanks for the quick fix!

Reviewed-by: Anton Johansson

Re: [PATCH for-8.1] accel/tcg: Zero-pad PC in TCG CPU exec trace lines


On 11/7/23 18:54, Peter Maydell wrote:

In commit f0a08b0913befbd we changed the type of the PC from
target_ulong to vaddr.  In doing so we inadvertently dropped the
zero-padding on the PC in trace lines (the second item inside the []
in these lines).  They used to look like this on AArch64, for
instance:

Trace 0: 0x7f226100 [/4000/0061/ff20]

and now they look like this:
Trace 0: 0x7f4f5100 [/4000/0061/ff20]

and if the PC happens to be somewhere low like 0x5000
then the field is shown as /5000/.

This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier,
depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64
with no width specifier.

Restore the zero-padding by adding an 016 width specifier to
this tracing and a couple of others that were similarly recently
changed to use VADDR_PRIx without a width specifier.

We can't unfortunately restore the "32-bit guests are padded to
8 hex digits and 64-bit guests to 16 hex digits" behaviour so
easily.

Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr")
Signed-off-by: Peter Maydell 
---
I have a workflow that parses log files to get the executed
PC values; I don't suppose I'm the only one doing that...
---
  accel/tcg/cpu-exec.c  | 4 ++--
  accel/tcg/translate-all.c | 2 +-
  2 files changed, 3 insertions(+), 3 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé

Re: [RFC PATCH 6/9] ui/gtk: Add a new parameter to assign connectors/monitors to GFX VCs

2023-07-11 Thread Kim, Dongwon

On 7/10/2023 11:36 PM, Markus Armbruster wrote:

"Kim, Dongwon" writes:

On 7/9/2023 11:05 PM, Markus Armbruster wrote:

"Kim, Dongwon" writes:

On 7/7/2023 7:07 AM, Markus Armbruster wrote:

[...]

Ignorant questions:

1. How would I plug / unplug display cables?

I am not sure if I understood your question correctly but 1 or more guest
displays (GTK windows) are bound to a certain physical displays like HDMI or DP
monitors. So plug/unplug means we disconnect those physical HDMI or DP cables
manually. Or this manual hot plug in can be emulated by you write something to
sysfs depending on what display driver you use.

Let's see whether I understand.

A VC is placed on a *physical* monitor, i.e. a window appears on that
monitor. That monitor's plug / unplug state is passed through to the
guest, i.e. if I physically unplug / plug the monitor, the guest sees an
unplug / plug of its virtual monitor. Correct?

This is correct. When a display is disconnected, "monitor-changed" GTK event will be
triggered then it will call gd_ui_size(0,0) which makes the guest display connection status to
"disconnected".

Thanks!

Permit me another ignorant question... Say I have a single monitor. I
configured my X windows manager to show four virtual desktops. Can I
use your feature to direct on which virtual desktop each VC is placed?

Would those virtual desktops will be their own connector names like HDMI or DP?
We use the connector name for the actual physical display you see when running
xrandr.

Output of xrandr is identical on different virtual desktops for me.

I don't know how virtual desktops are created and managed but if they don't
have their own connector names that GTK API can read, it won't work within our
current implementation.

After searching around a bit... Virtual desktops, a.k.a. workspaces,
are a window manager thing. Completely different from X displays and
monitors. Programs can mess with placement (wmctrl does). No idea
whether GDK provides an interface for it. No need to discuss this
further at this time.

[...]

Old question not yet answered: Using a list for the mapping means the
mapping must be dense, e.g. I can't map #0 and #2 but not #1. Is this
what we want?

No, it doesn't have to be dense. In your example, you can just leave the place
for VC1 blank. For example, you could do connectors.0=DP-1,connectors.2=HDMI-1.
But in this case, VC1 won't be activated and stay as disconnected from guest's
perspective. I think this info is also needed in v2.

Have you tried this? I believe it'll fail with something like
"Parameter 'connectors.1' missing".

Just tested it. Yeah you are correct. I think I had a bad assumption. Let me
take a look to see if I can make it work as I assumed.

If sparse mappings make sense, we should provide for them, I think.

An array like '*connectors': ['str'] maps from integers 0, 1, ... It
can't do sparse (you can't omit integers in the middle).

Yeah, I understand this now. Despite of my initial intention was
different, I am wondering if we don't allow the sparse mapping in this
implementation. Any thought on that? The V2 patch is with that change in
the description. Another thing I think is to change QAPI design little
bit to make it fill the element with null (0) if it's not given. Would
this be a feasible option?

Instead of omitting them, we could map them to null: '*connectors':
['StrOrNull']. JSON input looks like [null, "HDMI-A-0"]. Since dotted
key syntax does not support null at this time, you'd have to use JSON.

Only an object can do sparse. However, the QAPI schema language can't
express "object where the keys are integers and the values are strings".
We'd have to use 'any', and check everything manually.

Hmm. Thoughts?

[...]

Re: [PATCH v7 12/15] target/riscv: Add Zvkg ISA extension support


On 2023/7/11 11:57 AM, Max Chou wrote:


On 2023/7/11 11:15 AM, Alistair Francis wrote:


On Mon, Jul 10, 2023 at 6:37 PM Max Chou  wrote:

On 2023/7/8 5:25 PM, Daniel Henrique Barboza wrote:


Hi,

This patch breaks some gitlab runners because of this:

On 7/2/23 12:53, Max Chou wrote:

From: Nazar Kazakov

This commit adds support for the Zvkg vector-crypto extension, which
consists of the following instructions:

* vgmul.vv
* vghsh.vv

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Lawrence Hunter
[max.c...@sifive.com: Replaced vstart checking by TCG op]
Signed-off-by: Lawrence Hunter
Signed-off-by: Nazar Kazakov
Signed-off-by: Max Chou
Reviewed-by: Daniel Henrique Barboza
[max.c...@sifive.com: Exposed x-zvkg property]
---
   target/riscv/cpu.c   |  6 +-
   target/riscv/cpu_cfg.h   |  1 +
   target/riscv/helper.h|  3 +
   target/riscv/insn32.decode   |  4 ++
   target/riscv/insn_trans/trans_rvvk.c.inc | 30 ++
   target/riscv/vcrypto_helper.c| 72 
   6 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 08b8355f52..699ab5e9fa 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -118,6 +118,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
   ISA_EXT_DATA_ENTRY(zve64d, PRIV_VERSION_1_10_0, ext_zve64d),
   ISA_EXT_DATA_ENTRY(zvfh, PRIV_VERSION_1_12_0, ext_zvfh),
   ISA_EXT_DATA_ENTRY(zvfhmin, PRIV_VERSION_1_12_0, ext_zvfhmin),
+ISA_EXT_DATA_ENTRY(zvkg, PRIV_VERSION_1_12_0, ext_zvkg),
   ISA_EXT_DATA_ENTRY(zvkned, PRIV_VERSION_1_12_0, ext_zvkned),
   ISA_EXT_DATA_ENTRY(zvknha, PRIV_VERSION_1_12_0, ext_zvknha),
   ISA_EXT_DATA_ENTRY(zvknhb, PRIV_VERSION_1_12_0, ext_zvknhb),
@@ -1194,8 +1195,8 @@ void riscv_cpu_validate_set_extensions(RISCVCPU
*cpu, Error **errp)
* In principle Zve*x would also suffice here, were they supported
* in qemu
*/
-if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned ||
cpu->cfg.ext_zvknha ||
- cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32f) {
+if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkg ||
cpu->cfg.ext_zvkned ||
+ cpu->cfg.ext_zvknha || cpu->cfg.ext_zvksh) &&
!cpu->cfg.ext_zve32f) {
   error_setg(errp,
  "Vector crypto extensions require V or Zve*
extensions");
   return;
@@ -1710,6 +1711,7 @@ static Property riscv_cpu_extensions[] = {
   /* Vector cryptography extensions */
   DEFINE_PROP_BOOL("x-zvbb", RISCVCPU, cfg.ext_zvbb, false),
   DEFINE_PROP_BOOL("x-zvbc", RISCVCPU, cfg.ext_zvbc, false),
+DEFINE_PROP_BOOL("x-zvkg", RISCVCPU, cfg.ext_zvkg, false),
   DEFINE_PROP_BOOL("x-zvkned", RISCVCPU, cfg.ext_zvkned, false),
   DEFINE_PROP_BOOL("x-zvknha", RISCVCPU, cfg.ext_zvknha, false),
   DEFINE_PROP_BOOL("x-zvknhb", RISCVCPU, cfg.ext_zvknhb, false),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 27062b12a8..960761c479 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -85,6 +85,7 @@ struct RISCVCPUConfig {
   bool ext_zve64d;
   bool ext_zvbb;
   bool ext_zvbc;
+bool ext_zvkg;
   bool ext_zvkned;
   bool ext_zvknha;
   bool ext_zvknhb;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 172c91c65c..238343cb42 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1244,3 +1244,6 @@ DEF_HELPER_5(vsha2cl64_vv, void, ptr, ptr, ptr,
env, i32)
 DEF_HELPER_5(vsm3me_vv, void, ptr, ptr, ptr, env, i32)
   DEF_HELPER_5(vsm3c_vi, void, ptr, ptr, i32, env, i32)
+
+DEF_HELPER_5(vghsh_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_4(vgmul_vv, void, ptr, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 5ca83e8462..b10497afd3 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -957,3 +957,7 @@ vsha2cl_vv  10 1 . . 010 .
1110111 @r_vm_1
   # *** Zvksh vector crypto extension ***
   vsm3me_vv   10 1 . . 010 . 1110111 @r_vm_1
   vsm3c_vi101011 1 . . 010 . 1110111 @r_vm_1
+
+# *** Zvkg vector crypto extension ***
+vghsh_vv101100 1 . . 010 . 1110111 @r_vm_1
+vgmul_vv101000 1 . 10001 010 . 1110111 @r2_vm_1
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc
b/target/riscv/insn_trans/trans_rvvk.c.inc
index 6469dd2f02..af7cd62e7d 100644
--- a/target/riscv/insn_trans/trans_rvvk.c.inc
+++ b/target/riscv/insn_trans/trans_rvvk.c.inc
@@ -531,3 +531,33 @@ static inline bool vsm3c_check(DisasContext *s,
arg_rmrr *a)
 GEN_VV_UNMASKED_TRANS(vsm3me_vv, vsm3me_check, ZVKSH_EGS)
   GEN_VI_UNMASKED_TRANS(vsm3c_vi, vsm3c_check, ZVKSH_EGS)
+
+/*
+ * Zvkg
+ */
+
+#define ZVKG_EGS 4
+
+static bool vgmul_check(DisasContext *s, arg_rmr *a)
+{
+int egw_bytes =

Re: [PATCH] Revert "virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events"

2023-07-11 Thread Stefano Garzarella


CCing `./scripts/get_maintainer.pl -f drivers/scsi/virtio_scsi.c`,
since I found a few things in the virtio-scsi driver...

FYI we have seen that Linux has problems with a QEMU patch for the
virtio-scsi device (details at the bottom of this email in the revert
commit message and BZ).


This is what I found when I looked at the Linux code:

In scsi_report_sense() in linux/drivers/scsi/scsi_error.c linux calls
scsi_report_lun_change() that set `sdev_target->expecting_lun_change =
1` when we receive a UNIT ATTENTION with REPORT LUNS CHANGED
(sshdr->asc == 0x3f && sshdr->ascq == 0x0e).

When `sdev_target->expecting_lun_change = 1` is set and we call
scsi_check_sense(), for example to check the next UNIT ATTENTION, it
will return NEEDS_RETRY, that I think will cause the issues we are
seeing.

`sdev_target->expecting_lun_change` is reset only in
scsi_decide_disposition() when `REPORT_LUNS` command returns with
SAM_STAT_GOOD.
That command is issued in scsi_report_lun_scan() called by
__scsi_scan_target(), called for example by scsi_scan_target(),
scsi_scan_host(), etc.

So, checking QEMU, we send VIRTIO_SCSI_EVT_RESET_RESCAN during hotplug
and VIRTIO_SCSI_EVT_RESET_REMOVED during hotunplug. In both cases now we
send also the UNIT ATTENTION.

In the virtio-scsi driver, when we receive VIRTIO_SCSI_EVT_RESET_RESCAN
(hotplug) we call scsi_scan_target() or scsi_add_device(). Both of them
will call __scsi_scan_target() at some points, sending `REPORT_LUNS`
command to the device. This does not happen for
VIRTIO_SCSI_EVT_RESET_REMOVED (hotunplug). Indeed if I remove the
UNIT ATTENTION from the hotunplug in QEMU, everything works well.

So, I tried to add a scan also for VIRTIO_SCSI_EVT_RESET_REMOVED:

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index bd5633667d01..c57658a63097 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -291,6 +291,7 @@ static void virtscsi_handle_transport_reset(struct 
virtio_scsi *vscsi,
}
break;
case VIRTIO_SCSI_EVT_RESET_REMOVED:
+   scsi_scan_host(shost);
sdev = scsi_device_lookup(shost, 0, target, lun);
if (sdev) {
scsi_remove_device(sdev);

This somehow helps, now linux only breaks if the plug/unplug frequency
is really high. If I put a 5 second sleep between plug/unplug events, it
doesn't break (at least for the duration of my test which has been
running for about 30 minutes, before it used to break after about a
minute).

Another thing I noticed is that in QEMU maybe we should set the UNIT
ATTENTION first and then send the event on the virtqueue, because the
scan should happen after the unit attention, but I don't know if in any
case the unit attention is processed before the virtqueue.

I mean something like this:

diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 45b95ea070..13db40f4f3 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -1079,8 +1079,8 @@ static void virtio_scsi_hotplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,
 };

 virtio_scsi_acquire(s);
-virtio_scsi_push_event(s, );
 scsi_bus_set_ua(>bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
+virtio_scsi_push_event(s, );
 virtio_scsi_release(s);
 }
 }
@@ -,8 +,8 @@ static void virtio_scsi_hotunplug(HotplugHandler 
*hotplug_dev, DeviceState *dev,

 if (virtio_vdev_has_feature(vdev, VIRTIO_SCSI_F_HOTPLUG)) {
 virtio_scsi_acquire(s);
-virtio_scsi_push_event(s, );
 scsi_bus_set_ua(>bus, SENSE_CODE(REPORTED_LUNS_CHANGED));
+virtio_scsi_push_event(s, );
 virtio_scsi_release(s);
 }
 }

At this point I think the problem is on the handling of the
VIRTIO_SCSI_EVT_RESET_REMOVED event in the virtio-scsi driver, where
somehow we have to redo the bus scan, but scsi_scan_host() doesn't seem
to be enough when the event rate is very high.

I don't know if along with this fix, we also need to limit the rate in
QEMU somehow.

Sorry for the length of this email, but I'm not familiar with SCSI and
wanted some suggestions on how to proceed.

Paolo, Stefan, Linux SCSI maintainers, any suggestion?


Thanks,
Stefano

On Wed, Jul 05, 2023 at 09:15:23AM +0200, Stefano Garzarella wrote:

This reverts commit 8cc5583abe6419e7faaebc9fbd109f34f4c850f2.

That commit causes several problems in Linux as described in the BZ.
In particular, after a while, other devices on the bus are no longer
usable even if those devices are not affected by the hotunplug.
This may be a problem in Linux, but we have not been able to identify
it so far. So better to revert this patch until we find a solution.

Also, Oracle, which initially proposed this patch for a problem with
Solaris, seems to have already reversed it downstream:
   https://linux.oracle.com/errata/ELSA-2023-12065.html

Suggested-by: Thomas Huth 
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2176702
Cc:

Re: [PATCH 3/3] tests/qemu-iotests/197: add testcase for CoR with subclusters

2023-07-11 Thread Denis V. Lunev


On 6/26/23 18:08, Andrey Drobyshev wrote:

Add testcase which checks that allocations during copy-on-read are
performed on the subcluster basis when subclusters are enabled in target
image.

This testcase also triggers the following assert with previous commit
not being applied, so we check that as well:

qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion `skip_bytes < 
pnum' failed.

Signed-off-by: Andrey Drobyshev 
---
  tests/qemu-iotests/197 | 29 +
  tests/qemu-iotests/197.out | 24 
  2 files changed, 53 insertions(+)

diff --git a/tests/qemu-iotests/197 b/tests/qemu-iotests/197
index a2547bc280..f07a9da136 100755
--- a/tests/qemu-iotests/197
+++ b/tests/qemu-iotests/197
@@ -122,6 +122,35 @@ $QEMU_IO -f qcow2 -C -c 'read 0 1024' "$TEST_WRAP" | 
_filter_qemu_io
  $QEMU_IO -f qcow2 -c map "$TEST_WRAP"
  _check_test_img
  
+echo

+echo '=== Copy-on-read with subclusters ==='
+echo
+
+# Create base and top images 64K (1 cluster) each.  Make subclusters enabled
+# for the top image
+_make_test_img 64K
+IMGPROTO=file IMGFMT=qcow2 TEST_IMG_FILE="$TEST_WRAP" \
+_make_test_img --no-opts -o extended_l2=true -F "$IMGFMT" -b "$TEST_IMG" \
+64K | _filter_img_create
+
+$QEMU_IO -c "write -P 0xaa 0 64k" "$TEST_IMG" | _filter_qemu_io
+
+# Allocate individual subclusters in the top image, and not the whole cluster
+$QEMU_IO -c "write -P 0xbb 28K 2K" -c "write -P 0xcc 34K 2K" "$TEST_WRAP" \
+| _filter_qemu_io
+
+# Only 2 subclusters should be allocated in the top image at this point
+$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+
+# Actual copy-on-read operation
+$QEMU_IO -C -c "read -P 0xaa 30K 4K" "$TEST_WRAP" | _filter_qemu_io
+
+# And here we should have 4 subclusters allocated right in the middle of the
+# top image. Make sure the whole cluster remains unallocated
+$QEMU_IMG map "$TEST_WRAP" | _filter_qemu_img_map
+
+_check_test_img
+
  # success, all done
  echo '*** done'
  status=0
diff --git a/tests/qemu-iotests/197.out b/tests/qemu-iotests/197.out
index ad414c3b0e..8f34a30afe 100644
--- a/tests/qemu-iotests/197.out
+++ b/tests/qemu-iotests/197.out
@@ -31,4 +31,28 @@ read 1024/1024 bytes at offset 0
  1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
  1 KiB (0x400) bytes allocated at offset 0 bytes (0x0)
  No errors were found on the image.
+
+=== Copy-on-read with subclusters ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
+Formatting 'TEST_DIR/t.wrap.IMGFMT', fmt=IMGFMT size=65536 
backing_file=TEST_DIR/t.IMGFMT backing_fmt=IMGFMT
+wrote 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 28672
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+wrote 2048/2048 bytes at offset 34816
+2 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset  Length  File
+0   0x7000  TEST_DIR/t.IMGFMT
+0x7000  0x800   TEST_DIR/t.wrap.IMGFMT
+0x7800  0x1000  TEST_DIR/t.IMGFMT
+0x8800  0x800   TEST_DIR/t.wrap.IMGFMT
+0x9000  0x7000  TEST_DIR/t.IMGFMT
+read 4096/4096 bytes at offset 30720
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+Offset  Length  File
+0   0x7000  TEST_DIR/t.IMGFMT
+0x7000  0x2000  TEST_DIR/t.wrap.IMGFMT
+0x9000  0x7000  TEST_DIR/t.IMGFMT
+No errors were found on the image.
  *** done

Reviewed-by: Denis V. Lunev

Re: [PATCH 2/3] block/io: align requests to subcluster_size

2023-07-11 Thread Denis V. Lunev


On 6/26/23 18:08, Andrey Drobyshev wrote:

When target image is using subclusters, and we align the request during
copy-on-read, it makes sense to align to subcluster_size rather than
cluster_size.  Otherwise we end up with unnecessary allocations.

This commit renames bdrv_round_to_clusters() to bdrv_round_to_subclusters()
and utilizes subcluster_size field of BlockDriverInfo to make necessary
alignments.  It affects copy-on-read as well as mirror job (which is
using bdrv_round_to_clusters()).

This change also fixes the following bug with failing assert (covered by
the test in the subsequent commit):

qemu-img create -f qcow2 base.qcow2 64K
qemu-img create -f qcow2 -o 
extended_l2=on,backing_file=base.qcow2,backing_fmt=qcow2 img.qcow2 64K
qemu-io -c "write -P 0xaa 0 2K" img.qcow2
qemu-io -C -c "read -P 0x00 2K 62K" img.qcow2

qemu-io: ../block/io.c:1236: bdrv_co_do_copy_on_readv: Assertion `skip_bytes < 
pnum' failed.

Signed-off-by: Andrey Drobyshev 
---
  block/io.c   | 50 
  block/mirror.c   |  8 +++
  include/block/block-io.h |  2 +-
  3 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/block/io.c b/block/io.c
index 30748f0b59..f1f8fad409 100644
--- a/block/io.c
+++ b/block/io.c
@@ -728,21 +728,21 @@ BdrvTrackedRequest *coroutine_fn 
bdrv_co_get_self_request(BlockDriverState *bs)
  }
  
  /**

- * Round a region to cluster boundaries
+ * Round a region to subcluster (if supported) or cluster boundaries
   */
  void coroutine_fn GRAPH_RDLOCK
-bdrv_round_to_clusters(BlockDriverState *bs, int64_t offset, int64_t bytes,
-   int64_t *cluster_offset, int64_t *cluster_bytes)
+bdrv_round_to_subclusters(BlockDriverState *bs, int64_t offset, int64_t bytes,
+  int64_t *align_offset, int64_t *align_bytes)
  {
  BlockDriverInfo bdi;
  IO_CODE();
-if (bdrv_co_get_info(bs, ) < 0 || bdi.cluster_size == 0) {
-*cluster_offset = offset;
-*cluster_bytes = bytes;
+if (bdrv_co_get_info(bs, ) < 0 || bdi.subcluster_size == 0) {
+*align_offset = offset;
+*align_bytes = bytes;
  } else {
-int64_t c = bdi.cluster_size;
-*cluster_offset = QEMU_ALIGN_DOWN(offset, c);
-*cluster_bytes = QEMU_ALIGN_UP(offset - *cluster_offset + bytes, c);
+int64_t c = bdi.subcluster_size;
+*align_offset = QEMU_ALIGN_DOWN(offset, c);
+*align_bytes = QEMU_ALIGN_UP(offset - *align_offset + bytes, c);
  }
  }
  
@@ -1168,8 +1168,8 @@ bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t offset, int64_t bytes,

  void *bounce_buffer = NULL;
  
  BlockDriver *drv = bs->drv;

-int64_t cluster_offset;
-int64_t cluster_bytes;
+int64_t align_offset;
+int64_t align_bytes;
  int64_t skip_bytes;
  int ret;
  int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer,
@@ -1203,28 +1203,28 @@ bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t 
offset, int64_t bytes,
   * BDRV_REQUEST_MAX_BYTES (even when the original read did not), which
   * is one reason we loop rather than doing it all at once.
   */
-bdrv_round_to_clusters(bs, offset, bytes, _offset, _bytes);
-skip_bytes = offset - cluster_offset;
+bdrv_round_to_subclusters(bs, offset, bytes, _offset, _bytes);
+skip_bytes = offset - align_offset;
  
  trace_bdrv_co_do_copy_on_readv(bs, offset, bytes,

-   cluster_offset, cluster_bytes);
+   align_offset, align_bytes);
  
-while (cluster_bytes) {

+while (align_bytes) {
  int64_t pnum;
  
  if (skip_write) {

  ret = 1; /* "already allocated", so nothing will be copied */
-pnum = MIN(cluster_bytes, max_transfer);
+pnum = MIN(align_bytes, max_transfer);
  } else {
-ret = bdrv_is_allocated(bs, cluster_offset,
-MIN(cluster_bytes, max_transfer), );
+ret = bdrv_is_allocated(bs, align_offset,
+MIN(align_bytes, max_transfer), );
  if (ret < 0) {
  /*
   * Safe to treat errors in querying allocation as if
   * unallocated; we'll probably fail again soon on the
   * read, but at least that will set a decent errno.
   */
-pnum = MIN(cluster_bytes, max_transfer);
+pnum = MIN(align_bytes, max_transfer);
  }
  
  /* Stop at EOF if the image ends in the middle of the cluster */

@@ -1242,7 +1242,7 @@ bdrv_co_do_copy_on_readv(BdrvChild *child, int64_t 
offset, int64_t bytes,
  /* Must copy-on-read; use the bounce buffer */
  pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
  if (!bounce_buffer) {
-int64_t max_we_need = MAX(pnum, cluster_bytes - pnum);
+int64_t max_we_need

[PATCH v8 12/15] target/riscv: Add Zvkg ISA extension support

From: Nazar Kazakov 

This commit adds support for the Zvkg vector-crypto extension, which
consists of the following instructions:

* vgmul.vv
* vghsh.vv

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Lawrence Hunter 
[max.c...@sifive.com: Replaced vstart checking by TCG op]
Signed-off-by: Lawrence Hunter 
Signed-off-by: Nazar Kazakov 
Signed-off-by: Max Chou 
Reviewed-by: Daniel Henrique Barboza 
[max.c...@sifive.com: Exposed x-zvkg property]
[max.c...@sifive.com: Replaced uint by int for cross win32 build]
---
 target/riscv/cpu.c   |  6 +-
 target/riscv/cpu_cfg.h   |  1 +
 target/riscv/helper.h|  3 +
 target/riscv/insn32.decode   |  4 ++
 target/riscv/insn_trans/trans_rvvk.c.inc | 30 ++
 target/riscv/vcrypto_helper.c| 72 
 6 files changed, 114 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 2546467f53b..075692582dd 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -128,6 +128,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zvfbfwma, PRIV_VERSION_1_12_0, ext_zvfbfwma),
 ISA_EXT_DATA_ENTRY(zvfh, PRIV_VERSION_1_12_0, ext_zvfh),
 ISA_EXT_DATA_ENTRY(zvfhmin, PRIV_VERSION_1_12_0, ext_zvfhmin),
+ISA_EXT_DATA_ENTRY(zvkg, PRIV_VERSION_1_12_0, ext_zvkg),
 ISA_EXT_DATA_ENTRY(zvkned, PRIV_VERSION_1_12_0, ext_zvkned),
 ISA_EXT_DATA_ENTRY(zvknha, PRIV_VERSION_1_12_0, ext_zvknha),
 ISA_EXT_DATA_ENTRY(zvknhb, PRIV_VERSION_1_12_0, ext_zvknhb),
@@ -1278,8 +1279,8 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
  * In principle Zve*x would also suffice here, were they supported
  * in qemu
  */
-if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned || cpu->cfg.ext_zvknha ||
- cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32f) {
+if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkg || cpu->cfg.ext_zvkned ||
+ cpu->cfg.ext_zvknha || cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32f) {
 error_setg(errp,
"Vector crypto extensions require V or Zve* extensions");
 return;
@@ -1872,6 +1873,7 @@ static Property riscv_cpu_extensions[] = {
 /* Vector cryptography extensions */
 DEFINE_PROP_BOOL("x-zvbb", RISCVCPU, cfg.ext_zvbb, false),
 DEFINE_PROP_BOOL("x-zvbc", RISCVCPU, cfg.ext_zvbc, false),
+DEFINE_PROP_BOOL("x-zvkg", RISCVCPU, cfg.ext_zvkg, false),
 DEFINE_PROP_BOOL("x-zvkned", RISCVCPU, cfg.ext_zvkned, false),
 DEFINE_PROP_BOOL("x-zvknha", RISCVCPU, cfg.ext_zvknha, false),
 DEFINE_PROP_BOOL("x-zvknhb", RISCVCPU, cfg.ext_zvknhb, false),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index ab2d9294db6..b754ec23446 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -87,6 +87,7 @@ struct RISCVCPUConfig {
 bool ext_zve64d;
 bool ext_zvbb;
 bool ext_zvbc;
+bool ext_zvkg;
 bool ext_zvkned;
 bool ext_zvknha;
 bool ext_zvknhb;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 6d21347c39b..ceec97e1657 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1273,3 +1273,6 @@ DEF_HELPER_5(vsha2cl64_vv, void, ptr, ptr, ptr, env, i32)
 
 DEF_HELPER_5(vsm3me_vv, void, ptr, ptr, ptr, env, i32)
 DEF_HELPER_5(vsm3c_vi, void, ptr, ptr, i32, env, i32)
+
+DEF_HELPER_5(vghsh_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_4(vgmul_vv, void, ptr, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 4050e843f7e..0fae01c6bb3 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -995,3 +995,7 @@ vsha2cl_vv  10 1 . . 010 . 1110111 @r_vm_1
 # *** Zvksh vector crypto extension ***
 vsm3me_vv   10 1 . . 010 . 1110111 @r_vm_1
 vsm3c_vi101011 1 . . 010 . 1110111 @r_vm_1
+
+# *** Zvkg vector crypto extension ***
+vghsh_vv101100 1 . . 010 . 1110111 @r_vm_1
+vgmul_vv101000 1 . 10001 010 . 1110111 @r2_vm_1
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc 
b/target/riscv/insn_trans/trans_rvvk.c.inc
index 6469dd2f024..af7cd62e7d0 100644
--- a/target/riscv/insn_trans/trans_rvvk.c.inc
+++ b/target/riscv/insn_trans/trans_rvvk.c.inc
@@ -531,3 +531,33 @@ static inline bool vsm3c_check(DisasContext *s, arg_rmrr 
*a)
 
 GEN_VV_UNMASKED_TRANS(vsm3me_vv, vsm3me_check, ZVKSH_EGS)
 GEN_VI_UNMASKED_TRANS(vsm3c_vi, vsm3c_check, ZVKSH_EGS)
+
+/*
+ * Zvkg
+ */
+
+#define ZVKG_EGS 4
+
+static bool vgmul_check(DisasContext *s, arg_rmr *a)
+{
+int egw_bytes = ZVKG_EGS << s->sew;
+return s->cfg_ptr->ext_zvkg == true &&
+   vext_check_isa_ill(s) &&
+   require_rvv(s) &&
+   MAXSZ(s) >= egw_bytes &&
+   vext_check_ss(s, a->rd, a->rs2, a->vm) &&
+   s->sew == MO_32;
+}
+

[PATCH v8 14/15] crypto: Add SM4 constant parameter CK

Adds sm4_ck constant for use in sm4 cryptography across different targets.

Signed-off-by: Max Chou 
Reviewed-by: Frank Chang 
Signed-off-by: Max Chou 
---
 crypto/sm4.c | 10 ++
 include/crypto/sm4.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/crypto/sm4.c b/crypto/sm4.c
index 9f0cd452c78..2987306cf7a 100644
--- a/crypto/sm4.c
+++ b/crypto/sm4.c
@@ -47,3 +47,13 @@ uint8_t const sm4_sbox[] = {
 0x79, 0xee, 0x5f, 0x3e, 0xd7, 0xcb, 0x39, 0x48,
 };
 
+uint32_t const sm4_ck[] = {
+0x00070e15, 0x1c232a31, 0x383f464d, 0x545b6269,
+0x70777e85, 0x8c939aa1, 0xa8afb6bd, 0xc4cbd2d9,
+0xe0e7eef5, 0xfc030a11, 0x181f262d, 0x343b4249,
+0x50575e65, 0x6c737a81, 0x888f969d, 0xa4abb2b9,
+0xc0c7ced5, 0xdce3eaf1, 0xf8ff060d, 0x141b2229,
+0x30373e45, 0x4c535a61, 0x686f767d, 0x848b9299,
+0xa0a7aeb5, 0xbcc3cad1, 0xd8dfe6ed, 0xf4fb0209,
+0x10171e25, 0x2c333a41, 0x484f565d, 0x646b7279
+};
diff --git a/include/crypto/sm4.h b/include/crypto/sm4.h
index de8245d8a71..382b26d9224 100644
--- a/include/crypto/sm4.h
+++ b/include/crypto/sm4.h
@@ -2,6 +2,7 @@
 #define QEMU_SM4_H
 
 extern const uint8_t sm4_sbox[256];
+extern const uint32_t sm4_ck[32];
 
 static inline uint32_t sm4_subword(uint32_t word)
 {
-- 
2.34.1

[PATCH v8 15/15] target/riscv: Add Zvksed ISA extension support

This commit adds support for the Zvksed vector-crypto extension, which
consists of the following instructions:

* vsm4k.vi
* vsm4r.[vv,vs]

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Signed-off-by: Max Chou 
Reviewed-by: Frank Chang 
[lawrence.hun...@codethink.co.uk: Moved SM4 functions from
crypto_helper.c to vcrypto_helper.c]
[nazar.kaza...@codethink.co.uk: Added alignment checks, refactored code to
use macros, and minor style changes]
Signed-off-by: Max Chou 
---
 target/riscv/cpu.c   |   5 +-
 target/riscv/cpu_cfg.h   |   1 +
 target/riscv/helper.h|   4 +
 target/riscv/insn32.decode   |   5 +
 target/riscv/insn_trans/trans_rvvk.c.inc |  43 
 target/riscv/vcrypto_helper.c| 127 +++
 6 files changed, 184 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 075692582dd..41f12f3d7a9 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -132,6 +132,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zvkned, PRIV_VERSION_1_12_0, ext_zvkned),
 ISA_EXT_DATA_ENTRY(zvknha, PRIV_VERSION_1_12_0, ext_zvknha),
 ISA_EXT_DATA_ENTRY(zvknhb, PRIV_VERSION_1_12_0, ext_zvknhb),
+ISA_EXT_DATA_ENTRY(zvksed, PRIV_VERSION_1_12_0, ext_zvksed),
 ISA_EXT_DATA_ENTRY(zvksh, PRIV_VERSION_1_12_0, ext_zvksh),
 ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
 ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
@@ -1280,7 +1281,8 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
  * in qemu
  */
 if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkg || cpu->cfg.ext_zvkned ||
- cpu->cfg.ext_zvknha || cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32f) {
+ cpu->cfg.ext_zvknha || cpu->cfg.ext_zvksed || cpu->cfg.ext_zvksh) &&
+!cpu->cfg.ext_zve32f) {
 error_setg(errp,
"Vector crypto extensions require V or Zve* extensions");
 return;
@@ -1877,6 +1879,7 @@ static Property riscv_cpu_extensions[] = {
 DEFINE_PROP_BOOL("x-zvkned", RISCVCPU, cfg.ext_zvkned, false),
 DEFINE_PROP_BOOL("x-zvknha", RISCVCPU, cfg.ext_zvknha, false),
 DEFINE_PROP_BOOL("x-zvknhb", RISCVCPU, cfg.ext_zvknhb, false),
+DEFINE_PROP_BOOL("x-zvksed", RISCVCPU, cfg.ext_zvksed, false),
 DEFINE_PROP_BOOL("x-zvksh", RISCVCPU, cfg.ext_zvksh, false),
 
 DEFINE_PROP_END_OF_LIST(),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index b754ec23446..61f6238756f 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -91,6 +91,7 @@ struct RISCVCPUConfig {
 bool ext_zvkned;
 bool ext_zvknha;
 bool ext_zvknhb;
+bool ext_zvksed;
 bool ext_zvksh;
 bool ext_zmmul;
 bool ext_zvfbfmin;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index ceec97e1657..8a635238514 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1276,3 +1276,7 @@ DEF_HELPER_5(vsm3c_vi, void, ptr, ptr, i32, env, i32)
 
 DEF_HELPER_5(vghsh_vv, void, ptr, ptr, ptr, env, i32)
 DEF_HELPER_4(vgmul_vv, void, ptr, ptr, env, i32)
+
+DEF_HELPER_5(vsm4k_vi, void, ptr, ptr, i32, env, i32)
+DEF_HELPER_4(vsm4r_vv, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vsm4r_vs, void, ptr, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 0fae01c6bb3..33597fe2bb1 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -999,3 +999,8 @@ vsm3c_vi101011 1 . . 010 . 1110111 @r_vm_1
 # *** Zvkg vector crypto extension ***
 vghsh_vv101100 1 . . 010 . 1110111 @r_vm_1
 vgmul_vv101000 1 . 10001 010 . 1110111 @r2_vm_1
+
+# *** Zvksed vector crypto extension ***
+vsm4k_vi11 1 . . 010 . 1110111 @r_vm_1
+vsm4r_vv101000 1 . 1 010 . 1110111 @r2_vm_1
+vsm4r_vs101001 1 . 1 010 . 1110111 @r2_vm_1
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc 
b/target/riscv/insn_trans/trans_rvvk.c.inc
index af7cd62e7d0..c00c70dfc62 100644
--- a/target/riscv/insn_trans/trans_rvvk.c.inc
+++ b/target/riscv/insn_trans/trans_rvvk.c.inc
@@ -561,3 +561,46 @@ static bool vghsh_check(DisasContext *s, arg_rmrr *a)
 }
 
 GEN_VV_UNMASKED_TRANS(vghsh_vv, vghsh_check, ZVKG_EGS)
+
+/*
+ * Zvksed
+ */
+
+#define ZVKSED_EGS 4
+
+static bool zvksed_check(DisasContext *s)
+{
+int egw_bytes = ZVKSED_EGS << s->sew;
+return s->cfg_ptr->ext_zvksed == true &&
+   require_rvv(s) &&
+   vext_check_isa_ill(s) &&
+   MAXSZ(s) >= egw_bytes &&
+   s->sew == MO_32;
+}
+
+static bool vsm4k_vi_check(DisasContext *s, arg_rmrr *a)
+{
+return zvksed_check(s) &&
+   require_align(a->rd, s->lmul) &&
+   require_align(a->rs2, s->lmul);
+}
+
+GEN_VI_UNMASKED_TRANS(vsm4k_vi, vsm4k_vi_check, ZVKSED_EGS)
+
+static

[PATCH v8 13/15] crypto: Create sm4_subword

Allows sharing of sm4_subword between different targets.

Signed-off-by: Max Chou 
Reviewed-by: Frank Chang 
Reviewed-by: Richard Henderson 
Signed-off-by: Max Chou 
---
 include/crypto/sm4.h   |  8 
 target/arm/tcg/crypto_helper.c | 10 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/crypto/sm4.h b/include/crypto/sm4.h
index 9bd3ebc62e8..de8245d8a71 100644
--- a/include/crypto/sm4.h
+++ b/include/crypto/sm4.h
@@ -3,4 +3,12 @@
 
 extern const uint8_t sm4_sbox[256];
 
+static inline uint32_t sm4_subword(uint32_t word)
+{
+return sm4_sbox[word & 0xff] |
+   sm4_sbox[(word >> 8) & 0xff] << 8 |
+   sm4_sbox[(word >> 16) & 0xff] << 16 |
+   sm4_sbox[(word >> 24) & 0xff] << 24;
+}
+
 #endif
diff --git a/target/arm/tcg/crypto_helper.c b/target/arm/tcg/crypto_helper.c
index fdd70abbfd6..7cadd61e124 100644
--- a/target/arm/tcg/crypto_helper.c
+++ b/target/arm/tcg/crypto_helper.c
@@ -614,10 +614,7 @@ static void do_crypto_sm4e(uint64_t *rd, uint64_t *rn, 
uint64_t *rm)
 CR_ST_WORD(d, (i + 3) % 4) ^
 CR_ST_WORD(n, i);
 
-t = sm4_sbox[t & 0xff] |
-sm4_sbox[(t >> 8) & 0xff] << 8 |
-sm4_sbox[(t >> 16) & 0xff] << 16 |
-sm4_sbox[(t >> 24) & 0xff] << 24;
+t = sm4_subword(t);
 
 CR_ST_WORD(d, i) ^= t ^ rol32(t, 2) ^ rol32(t, 10) ^ rol32(t, 18) ^
 rol32(t, 24);
@@ -651,10 +648,7 @@ static void do_crypto_sm4ekey(uint64_t *rd, uint64_t *rn, 
uint64_t *rm)
 CR_ST_WORD(d, (i + 3) % 4) ^
 CR_ST_WORD(m, i);
 
-t = sm4_sbox[t & 0xff] |
-sm4_sbox[(t >> 8) & 0xff] << 8 |
-sm4_sbox[(t >> 16) & 0xff] << 16 |
-sm4_sbox[(t >> 24) & 0xff] << 24;
+t = sm4_subword(t);
 
 CR_ST_WORD(d, i) ^= t ^ rol32(t, 13) ^ rol32(t, 23);
 }
-- 
2.34.1

[PATCH v8 11/15] target/riscv: Add Zvksh ISA extension support

From: Lawrence Hunter 

This commit adds support for the Zvksh vector-crypto extension, which
consists of the following instructions:

* vsm3me.vv
* vsm3c.vi

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Kiran Ostrolenk 
[max.c...@sifive.com: Replaced vstart checking by TCG op]
Signed-off-by: Kiran Ostrolenk 
Signed-off-by: Lawrence Hunter 
Signed-off-by: Max Chou 
Reviewed-by: Daniel Henrique Barboza 
[max.c...@sifive.com: Exposed x-zvksh property]
---
 target/riscv/cpu.c   |   6 +-
 target/riscv/cpu_cfg.h   |   1 +
 target/riscv/helper.h|   3 +
 target/riscv/insn32.decode   |   4 +
 target/riscv/insn_trans/trans_rvvk.c.inc |  31 ++
 target/riscv/vcrypto_helper.c| 134 +++
 6 files changed, 177 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e66d9639e20..2546467f53b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -131,6 +131,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zvkned, PRIV_VERSION_1_12_0, ext_zvkned),
 ISA_EXT_DATA_ENTRY(zvknha, PRIV_VERSION_1_12_0, ext_zvknha),
 ISA_EXT_DATA_ENTRY(zvknhb, PRIV_VERSION_1_12_0, ext_zvknhb),
+ISA_EXT_DATA_ENTRY(zvksh, PRIV_VERSION_1_12_0, ext_zvksh),
 ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
 ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
 ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
@@ -1277,8 +1278,8 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
  * In principle Zve*x would also suffice here, were they supported
  * in qemu
  */
-if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned || cpu->cfg.ext_zvknha) &&
-!cpu->cfg.ext_zve32f) {
+if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned || cpu->cfg.ext_zvknha ||
+ cpu->cfg.ext_zvksh) && !cpu->cfg.ext_zve32f) {
 error_setg(errp,
"Vector crypto extensions require V or Zve* extensions");
 return;
@@ -1874,6 +1875,7 @@ static Property riscv_cpu_extensions[] = {
 DEFINE_PROP_BOOL("x-zvkned", RISCVCPU, cfg.ext_zvkned, false),
 DEFINE_PROP_BOOL("x-zvknha", RISCVCPU, cfg.ext_zvknha, false),
 DEFINE_PROP_BOOL("x-zvknhb", RISCVCPU, cfg.ext_zvknhb, false),
+DEFINE_PROP_BOOL("x-zvksh", RISCVCPU, cfg.ext_zvksh, false),
 
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 800b8783c14..ab2d9294db6 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -90,6 +90,7 @@ struct RISCVCPUConfig {
 bool ext_zvkned;
 bool ext_zvknha;
 bool ext_zvknhb;
+bool ext_zvksh;
 bool ext_zmmul;
 bool ext_zvfbfmin;
 bool ext_zvfbfwma;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 34329b52fe1..6d21347c39b 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1270,3 +1270,6 @@ DEF_HELPER_5(vsha2ch32_vv, void, ptr, ptr, ptr, env, i32)
 DEF_HELPER_5(vsha2ch64_vv, void, ptr, ptr, ptr, env, i32)
 DEF_HELPER_5(vsha2cl32_vv, void, ptr, ptr, ptr, env, i32)
 DEF_HELPER_5(vsha2cl64_vv, void, ptr, ptr, ptr, env, i32)
+
+DEF_HELPER_5(vsm3me_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vsm3c_vi, void, ptr, ptr, i32, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index e2b83186dc7..4050e843f7e 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -991,3 +991,7 @@ vaeskf2_vi  101010 1 . . 010 . 1110111 @r_vm_1
 vsha2ms_vv  101101 1 . . 010 . 1110111 @r_vm_1
 vsha2ch_vv  101110 1 . . 010 . 1110111 @r_vm_1
 vsha2cl_vv  10 1 . . 010 . 1110111 @r_vm_1
+
+# *** Zvksh vector crypto extension ***
+vsm3me_vv   10 1 . . 010 . 1110111 @r_vm_1
+vsm3c_vi101011 1 . . 010 . 1110111 @r_vm_1
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc 
b/target/riscv/insn_trans/trans_rvvk.c.inc
index a35be11b95e..6469dd2f024 100644
--- a/target/riscv/insn_trans/trans_rvvk.c.inc
+++ b/target/riscv/insn_trans/trans_rvvk.c.inc
@@ -500,3 +500,34 @@ static bool trans_vsha2ch_vv(DisasContext *s, arg_rmrr *a)
 }
 return false;
 }
+
+/*
+ * Zvksh
+ */
+
+#define ZVKSH_EGS 8
+
+static inline bool vsm3_check(DisasContext *s, arg_rmrr *a)
+{
+int egw_bytes = ZVKSH_EGS << s->sew;
+int mult = 1 << MAX(s->lmul, 0);
+return s->cfg_ptr->ext_zvksh == true &&
+   require_rvv(s) &&
+   vext_check_isa_ill(s) &&
+   !is_overlapped(a->rd, mult, a->rs2, mult) &&
+   MAXSZ(s) >= egw_bytes &&
+   s->sew == MO_32;
+}
+
+static inline bool vsm3me_check(DisasContext *s, arg_rmrr *a)
+{
+return vsm3_check(s, a) && vext_check_sss(s, a->rd, a->rs1, a->rs2, a->vm);
+}
+
+static inline bool vsm3c_check(DisasContext *s,

[PATCH v8 07/15] target/riscv: Refactor some of the generic vector functionality

From: Kiran Ostrolenk 

Move some macros out of `vector_helper` and into `vector_internals`.
This ensures they can be used by both vector and vector-crypto helpers
(latter implemented in proceeding commits).

Signed-off-by: Kiran Ostrolenk 
Reviewed-by: Weiwei Li 
Signed-off-by: Max Chou 
---
 target/riscv/vector_helper.c| 42 --
 target/riscv/vector_internals.h | 46 +
 2 files changed, 46 insertions(+), 42 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 6434fd2f7e8..ae94576f3df 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -635,9 +635,6 @@ GEN_VEXT_ST_WHOLE(vs8r_v, int8_t, ste_b)
 #define OP_SUS_H int16_t, uint16_t, int16_t, uint16_t, int16_t
 #define OP_SUS_W int32_t, uint32_t, int32_t, uint32_t, int32_t
 #define OP_SUS_D int64_t, uint64_t, int64_t, uint64_t, int64_t
-#define WOP_UUU_B uint16_t, uint8_t, uint8_t, uint16_t, uint16_t
-#define WOP_UUU_H uint32_t, uint16_t, uint16_t, uint32_t, uint32_t
-#define WOP_UUU_W uint64_t, uint32_t, uint32_t, uint64_t, uint64_t
 #define WOP_SSS_B int16_t, int8_t, int8_t, int16_t, int16_t
 #define WOP_SSS_H int32_t, int16_t, int16_t, int32_t, int32_t
 #define WOP_SSS_W int64_t, int32_t, int32_t, int64_t, int64_t
@@ -3437,11 +3434,6 @@ GEN_VEXT_VF(vfwnmsac_vf_h, 4)
 GEN_VEXT_VF(vfwnmsac_vf_w, 8)
 
 /* Vector Floating-Point Square-Root Instruction */
-/* (TD, T2, TX2) */
-#define OP_UU_H uint16_t, uint16_t, uint16_t
-#define OP_UU_W uint32_t, uint32_t, uint32_t
-#define OP_UU_D uint64_t, uint64_t, uint64_t
-
 #define OPFVV1(NAME, TD, T2, TX2, HD, HS2, OP) \
 static void do_##NAME(void *vd, void *vs2, int i,  \
   CPURISCVState *env)  \
@@ -4138,40 +4130,6 @@ GEN_VEXT_CMP_VF(vmfge_vf_w, uint32_t, H4, vmfge32)
 GEN_VEXT_CMP_VF(vmfge_vf_d, uint64_t, H8, vmfge64)
 
 /* Vector Floating-Point Classify Instruction */
-#define OPIVV1(NAME, TD, T2, TX2, HD, HS2, OP) \
-static void do_##NAME(void *vd, void *vs2, int i)  \
-{  \
-TX2 s2 = *((T2 *)vs2 + HS2(i));\
-*((TD *)vd + HD(i)) = OP(s2);  \
-}
-
-#define GEN_VEXT_V(NAME, ESZ)  \
-void HELPER(NAME)(void *vd, void *v0, void *vs2,   \
-  CPURISCVState *env, uint32_t desc)   \
-{  \
-uint32_t vm = vext_vm(desc);   \
-uint32_t vl = env->vl; \
-uint32_t total_elems = \
-vext_get_total_elems(env, desc, ESZ);  \
-uint32_t vta = vext_vta(desc); \
-uint32_t vma = vext_vma(desc); \
-uint32_t i;\
-   \
-for (i = env->vstart; i < vl; i++) {   \
-if (!vm && !vext_elem_mask(v0, i)) {   \
-/* set masked-off elements to 1s */\
-vext_set_elems_1s(vd, vma, i * ESZ,\
-  (i + 1) * ESZ);  \
-continue;  \
-}  \
-do_##NAME(vd, vs2, i); \
-}  \
-env->vstart = 0;   \
-/* set tail elements to 1s */  \
-vext_set_elems_1s(vd, vta, vl * ESZ,   \
-  total_elems * ESZ);  \
-}
-
 target_ulong fclass_h(uint64_t frs1)
 {
 float16 f = frs1;
diff --git a/target/riscv/vector_internals.h b/target/riscv/vector_internals.h
index 749d138bebe..8133111e5f6 100644
--- a/target/riscv/vector_internals.h
+++ b/target/riscv/vector_internals.h
@@ -121,12 +121,52 @@ void vext_set_elems_1s(void *base, uint32_t is_agnostic, 
uint32_t cnt,
 /* expand macro args before macro */
 #define RVVCALL(macro, ...)  macro(__VA_ARGS__)
 
+/* (TD, T2, TX2) */
+#define OP_UU_B uint8_t, uint8_t, uint8_t
+#define OP_UU_H uint16_t, uint16_t, uint16_t
+#define OP_UU_W uint32_t, uint32_t, uint32_t
+#define OP_UU_D uint64_t, uint64_t, uint64_t
+
 /* (TD, T1, T2, TX1, TX2) */
 #define OP_UUU_B uint8_t, uint8_t, uint8_t, uint8_t, uint8_t
 #define OP_UUU_H uint16_t, uint16_t, uint16_t, uint16_t, uint16_t
 #define OP_UUU_W uint32_t, uint32_t, uint32_t, uint32_t, uint32_t
 #define OP_UUU_D uint64_t, uint64_t, uint64_t, uint64_t, uint64_t
 
+#define OPIVV1(NAME, TD, T2, TX2, HD, HS2, OP) \
+static void do_##NAME(void *vd, void *vs2, int i)  \
+{  \
+TX2 s2 = *((T2 *)vs2 + HS2(i));\
+*((TD *)vd + HD(i)) = OP(s2);  \
+}
+
+#define GEN_VEXT_V(NAME, ESZ)

Re: riscv kvm breakage

2023-07-11 Thread Daniel Henrique Barboza





On 7/11/23 13:47, Philippe Mathieu-Daudé wrote:

On 11/7/23 18:43, Richard Henderson wrote:

Hiya,

This breakage crept in while cross-riscv64-system was otherwise broken in 
configure:

https://gitlab.com/qemu-project/qemu/-/jobs/4633277557#L4165

../target/riscv/kvm.c:209:38: error: ‘KVM_RISCV_ISA_EXT_ZICBOZ’ undeclared here 
(not in a function); did you mean ‘KVM_RISCV_ISA_EXT_ZICBOM’?
   209 | KVM_EXT_CFG("zicboz", ext_icboz, KVM_RISCV_ISA_EXT_ZICBOZ),
   |  ^~~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |    ^~~
../target/riscv/kvm.c:211:33: error: ‘KVM_RISCV_ISA_EXT_ZBB’ undeclared here 
(not in a function); did you mean ‘KVM_RISCV_ISA_EXT_MAX’?
   211 | KVM_EXT_CFG("zbb", ext_zbb, KVM_RISCV_ISA_EXT_ZBB),
   | ^
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |    ^~~
../target/riscv/kvm.c:212:37: error: ‘KVM_RISCV_ISA_EXT_SSAIA’ undeclared here 
(not in a function); did you mean ‘KVM_RISCV_ISA_EXT_SSTC’?
   212 | KVM_EXT_CFG("ssaia", ext_ssaia, KVM_RISCV_ISA_EXT_SSAIA),
   | ^~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |    ^~~
In file included from /usr/riscv64-linux-gnu/include/rpc/netdb.h:42,
  from /usr/riscv64-linux-gnu/include/netdb.h:32,
  from /builds/qemu-project/qemu/include/sysemu/os-posix.h:34,
  from /builds/qemu-project/qemu/include/qemu/osdep.h:151,
  from ../target/riscv/kvm.c:19:
../target/riscv/kvm.c:288:44: error: ‘struct kvm_riscv_config’ has no member 
named ‘zicboz_block_size’; did you mean ‘zicbom_block_size’?
   288 | .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)
   |    ^

Can someone have a look asap?  Thanks,


See:
https://lore.kernel.org/qemu-devel/20230711163346.69409-1-phi...@linaro.org/


Thanks Phil!

ps: cross building stuff is hard 





:)

[PATCH v8 10/15] target/riscv: Add Zvknh ISA extension support

From: Kiran Ostrolenk 

This commit adds support for the Zvknh vector-crypto extension, which
consists of the following instructions:

* vsha2ms.vv
* vsha2c[hl].vv

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Nazar Kazakov 
Co-authored-by: Lawrence Hunter 
[max.c...@sifive.com: Replaced vstart checking by TCG op]
Signed-off-by: Nazar Kazakov 
Signed-off-by: Lawrence Hunter 
Signed-off-by: Kiran Ostrolenk 
Signed-off-by: Max Chou 
Reviewed-by: Daniel Henrique Barboza 
[max.c...@sifive.com: Exposed x-zvknha & x-zvknhb properties]
[max.c...@sifive.com: Replaced SEW selection to happened during
translation]
---
 target/riscv/cpu.c   |  13 +-
 target/riscv/cpu_cfg.h   |   2 +
 target/riscv/helper.h|   6 +
 target/riscv/insn32.decode   |   5 +
 target/riscv/insn_trans/trans_rvvk.c.inc | 129 
 target/riscv/vcrypto_helper.c| 238 +++
 6 files changed, 390 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a1a40875f95..e66d9639e20 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -129,6 +129,8 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zvfh, PRIV_VERSION_1_12_0, ext_zvfh),
 ISA_EXT_DATA_ENTRY(zvfhmin, PRIV_VERSION_1_12_0, ext_zvfhmin),
 ISA_EXT_DATA_ENTRY(zvkned, PRIV_VERSION_1_12_0, ext_zvkned),
+ISA_EXT_DATA_ENTRY(zvknha, PRIV_VERSION_1_12_0, ext_zvknha),
+ISA_EXT_DATA_ENTRY(zvknhb, PRIV_VERSION_1_12_0, ext_zvknhb),
 ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
 ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
 ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
@@ -1275,14 +1277,17 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
  * In principle Zve*x would also suffice here, were they supported
  * in qemu
  */
-if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned) && !cpu->cfg.ext_zve32f) {
+if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned || cpu->cfg.ext_zvknha) &&
+!cpu->cfg.ext_zve32f) {
 error_setg(errp,
"Vector crypto extensions require V or Zve* extensions");
 return;
 }
 
-if (cpu->cfg.ext_zvbc && !cpu->cfg.ext_zve64f) {
-error_setg(errp, "Zvbc extension requires V or Zve64{f,d} extensions");
+if ((cpu->cfg.ext_zvbc || cpu->cfg.ext_zvknhb) && !cpu->cfg.ext_zve64f) {
+error_setg(
+errp,
+"Zvbc and Zvknhb extensions require V or Zve64{f,d} extensions");
 return;
 }
 
@@ -1867,6 +1872,8 @@ static Property riscv_cpu_extensions[] = {
 DEFINE_PROP_BOOL("x-zvbb", RISCVCPU, cfg.ext_zvbb, false),
 DEFINE_PROP_BOOL("x-zvbc", RISCVCPU, cfg.ext_zvbc, false),
 DEFINE_PROP_BOOL("x-zvkned", RISCVCPU, cfg.ext_zvkned, false),
+DEFINE_PROP_BOOL("x-zvknha", RISCVCPU, cfg.ext_zvknha, false),
+DEFINE_PROP_BOOL("x-zvknhb", RISCVCPU, cfg.ext_zvknhb, false),
 
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index c7eafe27c0d..800b8783c14 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -88,6 +88,8 @@ struct RISCVCPUConfig {
 bool ext_zvbb;
 bool ext_zvbc;
 bool ext_zvkned;
+bool ext_zvknha;
+bool ext_zvknhb;
 bool ext_zmmul;
 bool ext_zvfbfmin;
 bool ext_zvfbfwma;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 02e5dbe6ee4..34329b52fe1 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1264,3 +1264,9 @@ DEF_HELPER_4(vaesdm_vs, void, ptr, ptr, env, i32)
 DEF_HELPER_4(vaesz_vs, void, ptr, ptr, env, i32)
 DEF_HELPER_5(vaeskf1_vi, void, ptr, ptr, i32, env, i32)
 DEF_HELPER_5(vaeskf2_vi, void, ptr, ptr, i32, env, i32)
+
+DEF_HELPER_5(vsha2ms_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vsha2ch32_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vsha2ch64_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vsha2cl32_vv, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vsha2cl64_vv, void, ptr, ptr, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 4f3c50f10f3..e2b83186dc7 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -986,3 +986,8 @@ vaesdm_vs   101001 1 . 0 010 . 1110111 @r2_vm_1
 vaesz_vs101001 1 . 00111 010 . 1110111 @r2_vm_1
 vaeskf1_vi  100010 1 . . 010 . 1110111 @r_vm_1
 vaeskf2_vi  101010 1 . . 010 . 1110111 @r_vm_1
+
+# *** Zvknh vector crypto extension ***
+vsha2ms_vv  101101 1 . . 010 . 1110111 @r_vm_1
+vsha2ch_vv  101110 1 . . 010 . 1110111 @r_vm_1
+vsha2cl_vv  10 1 . . 010 . 1110111 @r_vm_1
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc 
b/target/riscv/insn_trans/trans_rvvk.c.inc
index

[PATCH v8 08/15] target/riscv: Add Zvbb ISA extension support

From: Dickon Hood 

This commit adds support for the Zvbb vector-crypto extension, which
consists of the following instructions:

* vrol.[vv,vx]
* vror.[vv,vx,vi]
* vbrev8.v
* vrev8.v
* vandn.[vv,vx]
* vbrev.v
* vclz.v
* vctz.v
* vcpop.v
* vwsll.[vv,vx,vi]

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Nazar Kazakov 
Co-authored-by: William Salmon 
Co-authored-by: Kiran Ostrolenk 
[max.c...@sifive.com: Fix imm mode of vror.vi]
Signed-off-by: Nazar Kazakov 
Signed-off-by: William Salmon 
Signed-off-by: Kiran Ostrolenk 
Signed-off-by: Dickon Hood 
Signed-off-by: Max Chou 
Reviewed-by: Daniel Henrique Barboza 
[max.c...@sifive.com: Exposed x-zvbb property]
---
 target/riscv/cpu.c   |  12 ++
 target/riscv/cpu_cfg.h   |   1 +
 target/riscv/helper.h|  62 +
 target/riscv/insn32.decode   |  20 +++
 target/riscv/insn_trans/trans_rvvk.c.inc | 164 +++
 target/riscv/vcrypto_helper.c| 138 +++
 6 files changed, 397 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index d1dc78d483f..e39a6118bbc 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -119,6 +119,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zksed, PRIV_VERSION_1_12_0, ext_zksed),
 ISA_EXT_DATA_ENTRY(zksh, PRIV_VERSION_1_12_0, ext_zksh),
 ISA_EXT_DATA_ENTRY(zkt, PRIV_VERSION_1_12_0, ext_zkt),
+ISA_EXT_DATA_ENTRY(zvbb, PRIV_VERSION_1_12_0, ext_zvbb),
 ISA_EXT_DATA_ENTRY(zvbc, PRIV_VERSION_1_12_0, ext_zvbc),
 ISA_EXT_DATA_ENTRY(zve32f, PRIV_VERSION_1_10_0, ext_zve32f),
 ISA_EXT_DATA_ENTRY(zve64f, PRIV_VERSION_1_10_0, ext_zve64f),
@@ -1269,6 +1270,16 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
 return;
 }
 
+/*
+ * In principle Zve*x would also suffice here, were they supported
+ * in qemu
+ */
+if (cpu->cfg.ext_zvbb && !cpu->cfg.ext_zve32f) {
+error_setg(errp,
+   "Vector crypto extensions require V or Zve* extensions");
+return;
+}
+
 if (cpu->cfg.ext_zvbc && !cpu->cfg.ext_zve64f) {
 error_setg(errp, "Zvbc extension requires V or Zve64{f,d} extensions");
 return;
@@ -1852,6 +1863,7 @@ static Property riscv_cpu_extensions[] = {
 DEFINE_PROP_BOOL("x-zvfbfwma", RISCVCPU, cfg.ext_zvfbfwma, false),
 
 /* Vector cryptography extensions */
+DEFINE_PROP_BOOL("x-zvbb", RISCVCPU, cfg.ext_zvbb, false),
 DEFINE_PROP_BOOL("x-zvbc", RISCVCPU, cfg.ext_zvbc, false),
 
 DEFINE_PROP_END_OF_LIST(),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index d25b36a5128..0e31ebeed93 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -85,6 +85,7 @@ struct RISCVCPUConfig {
 bool ext_zve32f;
 bool ext_zve64f;
 bool ext_zve64d;
+bool ext_zvbb;
 bool ext_zvbc;
 bool ext_zmmul;
 bool ext_zvfbfmin;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 6776777c4eb..3db25ed2a20 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1188,3 +1188,65 @@ DEF_HELPER_6(vclmul_vv, void, ptr, ptr, ptr, ptr, env, 
i32)
 DEF_HELPER_6(vclmul_vx, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vclmulh_vv, void, ptr, ptr, ptr, ptr, env, i32)
 DEF_HELPER_6(vclmulh_vx, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_6(vror_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vror_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vror_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vror_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+
+DEF_HELPER_6(vror_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vror_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vror_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vror_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_6(vrol_vv_b, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrol_vv_h, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrol_vv_w, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vrol_vv_d, void, ptr, ptr, ptr, ptr, env, i32)
+
+DEF_HELPER_6(vrol_vx_b, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vrol_vx_h, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vrol_vx_w, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vrol_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_5(vrev8_v_b, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vrev8_v_h, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vrev8_v_w, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vrev8_v_d, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vbrev8_v_b, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vbrev8_v_h, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vbrev8_v_w, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vbrev8_v_d, void, ptr, ptr, ptr, env, i32)
+DEF_HELPER_5(vbrev_v_b, void, ptr, ptr, ptr, env, i32)

[PATCH v8 05/15] target/riscv: Move vector translation checks

From: Nazar Kazakov 

Move the checks out of `do_opiv{v,x,i}_gvec{,_shift}` functions
and into the corresponding macros. This enables the functions to be
reused in proceeding commits without check duplication.

Signed-off-by: Nazar Kazakov 
Reviewed-by: Richard Henderson 
Reviewed-by: Weiwei Li 
Signed-off-by: Max Chou 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 28 +++--
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 7e194aae34a..5dfd524c7d2 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1183,9 +1183,6 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn 
*gvec_fn,
   gen_helper_gvec_4_ptr *fn)
 {
 TCGLabel *over = gen_new_label();
-if (!opivv_check(s, a)) {
-return false;
-}
 
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
@@ -1218,6 +1215,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
 \
 gen_helper_##NAME##_b, gen_helper_##NAME##_h,  \
 gen_helper_##NAME##_w, gen_helper_##NAME##_d,  \
 }; \
+if (!opivv_check(s, a)) {  \
+return false;  \
+}  \
 return do_opivv_gvec(s, a, tcg_gen_gvec_##SUF, fns[s->sew]);   \
 }
 
@@ -1276,10 +1276,6 @@ static inline bool
 do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn *gvec_fn,
   gen_helper_opivx *fn)
 {
-if (!opivx_check(s, a)) {
-return false;
-}
-
 if (a->vm && s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
 TCGv_i64 src1 = tcg_temp_new_i64();
 
@@ -1301,6 +1297,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
 \
 gen_helper_##NAME##_b, gen_helper_##NAME##_h,  \
 gen_helper_##NAME##_w, gen_helper_##NAME##_d,  \
 }; \
+if (!opivx_check(s, a)) {  \
+return false;  \
+}  \
 return do_opivx_gvec(s, a, tcg_gen_gvec_##SUF, fns[s->sew]);   \
 }
 
@@ -1432,10 +1431,6 @@ static inline bool
 do_opivi_gvec(DisasContext *s, arg_rmrr *a, GVecGen2iFn *gvec_fn,
   gen_helper_opivx *fn, imm_mode_t imm_mode)
 {
-if (!opivx_check(s, a)) {
-return false;
-}
-
 if (a->vm && s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
 gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2),
 extract_imm(s, a->rs1, imm_mode), MAXSZ(s), MAXSZ(s));
@@ -1453,6 +1448,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
 \
 gen_helper_##OPIVX##_b, gen_helper_##OPIVX##_h,\
 gen_helper_##OPIVX##_w, gen_helper_##OPIVX##_d,\
 }; \
+if (!opivx_check(s, a)) {  \
+return false;  \
+}  \
 return do_opivi_gvec(s, a, tcg_gen_gvec_##SUF, \
  fns[s->sew], IMM_MODE);   \
 }
@@ -1775,10 +1773,6 @@ static inline bool
 do_opivx_gvec_shift(DisasContext *s, arg_rmrr *a, GVecGen2sFn32 *gvec_fn,
 gen_helper_opivx *fn)
 {
-if (!opivx_check(s, a)) {
-return false;
-}
-
 if (a->vm && s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
 TCGv_i32 src1 = tcg_temp_new_i32();
 
@@ -1800,7 +1794,9 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
\
 gen_helper_##NAME##_b, gen_helper_##NAME##_h, \
 gen_helper_##NAME##_w, gen_helper_##NAME##_d, \
 };\
-  \
+if (!opivx_check(s, a)) { \
+return false; \
+} \
 return do_opivx_gvec_shift(s, a, tcg_gen_gvec_##SUF, fns[s->sew]);\
 }
 
-- 
2.34.1

[PATCH v8 09/15] target/riscv: Add Zvkned ISA extension support

From: Nazar Kazakov 

This commit adds support for the Zvkned vector-crypto extension, which
consists of the following instructions:

* vaesef.[vv,vs]
* vaesdf.[vv,vs]
* vaesdm.[vv,vs]
* vaesz.vs
* vaesem.[vv,vs]
* vaeskf1.vi
* vaeskf2.vi

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Lawrence Hunter 
Co-authored-by: William Salmon 
[max.c...@sifive.com: Replaced vstart checking by TCG op]
Signed-off-by: Lawrence Hunter 
Signed-off-by: William Salmon 
Signed-off-by: Nazar Kazakov 
Signed-off-by: Max Chou 
Reviewed-by: Daniel Henrique Barboza 
[max.c...@sifive.com: Imported aes-round.h and exposed x-zvkned
property]
[max.c...@sifive.com: Fixed endian issues and replaced the vstart & vl
egs checking by helper function]
[max.c...@sifive.com: Replaced bswap32 calls in aes key expanding]
---
 target/riscv/cpu.c   |   4 +-
 target/riscv/cpu_cfg.h   |   1 +
 target/riscv/helper.h|  14 ++
 target/riscv/insn32.decode   |  14 ++
 target/riscv/insn_trans/trans_rvvk.c.inc | 147 +
 target/riscv/vcrypto_helper.c| 202 +++
 6 files changed, 381 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index e39a6118bbc..a1a40875f95 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -128,6 +128,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zvfbfwma, PRIV_VERSION_1_12_0, ext_zvfbfwma),
 ISA_EXT_DATA_ENTRY(zvfh, PRIV_VERSION_1_12_0, ext_zvfh),
 ISA_EXT_DATA_ENTRY(zvfhmin, PRIV_VERSION_1_12_0, ext_zvfhmin),
+ISA_EXT_DATA_ENTRY(zvkned, PRIV_VERSION_1_12_0, ext_zvkned),
 ISA_EXT_DATA_ENTRY(zhinx, PRIV_VERSION_1_12_0, ext_zhinx),
 ISA_EXT_DATA_ENTRY(zhinxmin, PRIV_VERSION_1_12_0, ext_zhinxmin),
 ISA_EXT_DATA_ENTRY(smaia, PRIV_VERSION_1_12_0, ext_smaia),
@@ -1274,7 +1275,7 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
  * In principle Zve*x would also suffice here, were they supported
  * in qemu
  */
-if (cpu->cfg.ext_zvbb && !cpu->cfg.ext_zve32f) {
+if ((cpu->cfg.ext_zvbb || cpu->cfg.ext_zvkned) && !cpu->cfg.ext_zve32f) {
 error_setg(errp,
"Vector crypto extensions require V or Zve* extensions");
 return;
@@ -1865,6 +1866,7 @@ static Property riscv_cpu_extensions[] = {
 /* Vector cryptography extensions */
 DEFINE_PROP_BOOL("x-zvbb", RISCVCPU, cfg.ext_zvbb, false),
 DEFINE_PROP_BOOL("x-zvbc", RISCVCPU, cfg.ext_zvbc, false),
+DEFINE_PROP_BOOL("x-zvkned", RISCVCPU, cfg.ext_zvkned, false),
 
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 0e31ebeed93..c7eafe27c0d 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -87,6 +87,7 @@ struct RISCVCPUConfig {
 bool ext_zve64d;
 bool ext_zvbb;
 bool ext_zvbc;
+bool ext_zvkned;
 bool ext_zmmul;
 bool ext_zvfbfmin;
 bool ext_zvfbfwma;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 3db25ed2a20..02e5dbe6ee4 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1250,3 +1250,17 @@ DEF_HELPER_6(vandn_vx_b, void, ptr, ptr, tl, ptr, env, 
i32)
 DEF_HELPER_6(vandn_vx_h, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vandn_vx_w, void, ptr, ptr, tl, ptr, env, i32)
 DEF_HELPER_6(vandn_vx_d, void, ptr, ptr, tl, ptr, env, i32)
+
+DEF_HELPER_2(egs_check, void, i32, env)
+
+DEF_HELPER_4(vaesef_vv, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesef_vs, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesdf_vv, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesdf_vs, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesem_vv, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesem_vs, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesdm_vv, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesdm_vs, void, ptr, ptr, env, i32)
+DEF_HELPER_4(vaesz_vs, void, ptr, ptr, env, i32)
+DEF_HELPER_5(vaeskf1_vi, void, ptr, ptr, i32, env, i32)
+DEF_HELPER_5(vaeskf2_vi, void, ptr, ptr, i32, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index b982a8325bb..4f3c50f10f3 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -75,6 +75,7 @@
 @r_rm...   . . ... . ... %rs2 %rs1 %rm %rd
 @r2_rm   ...   . . ... . ... %rs1 %rm %rd
 @r2  ...   . . ... . ...  %rs1 %rd
+@r2_vm_1 .. . . . ... . ...  vm=1 %rs2 %rd
 @r2_nfvm ... ... vm:1 . . ... . ...  %nf %rs1 %rd
 @r2_vm   .. vm:1 . . ... . ...  %rs2 %rd
 @r1_vm   .. vm:1 . . ... . ... %rd
@@ -972,3 +973,16 @@ vcpop_v 010010 . . 01110 010 . 1010111 @r2_vm
 vwsll_vv110101 . . . 000 . 1010111 @r_vm
 vwsll_vx110101 . . . 100 . 1010111 @r_vm
 vwsll_vi110101

[PATCH v8 04/15] target/riscv: Add Zvbc ISA extension support

From: Lawrence Hunter 

This commit adds support for the Zvbc vector-crypto extension, which
consists of the following instructions:

* vclmulh.[vx,vv]
* vclmul.[vx,vv]

Translation functions are defined in
`target/riscv/insn_trans/trans_rvvk.c.inc` and helpers are defined in
`target/riscv/vcrypto_helper.c`.

Co-authored-by: Nazar Kazakov 
Co-authored-by: Max Chou 
Signed-off-by: Nazar Kazakov 
Signed-off-by: Lawrence Hunter 
Signed-off-by: Max Chou 
[max.c...@sifive.com: Exposed x-zvbc property]
---
 target/riscv/cpu.c   |  9 
 target/riscv/cpu_cfg.h   |  1 +
 target/riscv/helper.h|  6 +++
 target/riscv/insn32.decode   |  6 +++
 target/riscv/insn_trans/trans_rvvk.c.inc | 62 
 target/riscv/meson.build |  3 +-
 target/riscv/translate.c |  1 +
 target/riscv/vcrypto_helper.c| 59 ++
 8 files changed, 146 insertions(+), 1 deletion(-)
 create mode 100644 target/riscv/insn_trans/trans_rvvk.c.inc
 create mode 100644 target/riscv/vcrypto_helper.c

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9339c0241d6..d1dc78d483f 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -119,6 +119,7 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zksed, PRIV_VERSION_1_12_0, ext_zksed),
 ISA_EXT_DATA_ENTRY(zksh, PRIV_VERSION_1_12_0, ext_zksh),
 ISA_EXT_DATA_ENTRY(zkt, PRIV_VERSION_1_12_0, ext_zkt),
+ISA_EXT_DATA_ENTRY(zvbc, PRIV_VERSION_1_12_0, ext_zvbc),
 ISA_EXT_DATA_ENTRY(zve32f, PRIV_VERSION_1_10_0, ext_zve32f),
 ISA_EXT_DATA_ENTRY(zve64f, PRIV_VERSION_1_10_0, ext_zve64f),
 ISA_EXT_DATA_ENTRY(zve64d, PRIV_VERSION_1_10_0, ext_zve64d),
@@ -1268,6 +1269,11 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
Error **errp)
 return;
 }
 
+if (cpu->cfg.ext_zvbc && !cpu->cfg.ext_zve64f) {
+error_setg(errp, "Zvbc extension requires V or Zve64{f,d} extensions");
+return;
+}
+
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
 cpu->cfg.ext_zkr = true;
@@ -1845,6 +1851,9 @@ static Property riscv_cpu_extensions[] = {
 DEFINE_PROP_BOOL("x-zvfbfmin", RISCVCPU, cfg.ext_zvfbfmin, false),
 DEFINE_PROP_BOOL("x-zvfbfwma", RISCVCPU, cfg.ext_zvfbfwma, false),
 
+/* Vector cryptography extensions */
+DEFINE_PROP_BOOL("x-zvbc", RISCVCPU, cfg.ext_zvbc, false),
+
 DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 2bd9510ba3e..d25b36a5128 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -85,6 +85,7 @@ struct RISCVCPUConfig {
 bool ext_zve32f;
 bool ext_zve64f;
 bool ext_zve64d;
+bool ext_zvbc;
 bool ext_zmmul;
 bool ext_zvfbfmin;
 bool ext_zvfbfwma;
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index c95adaf08ac..6776777c4eb 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1182,3 +1182,9 @@ DEF_HELPER_5(vfwcvtbf16_f_f_v, void, ptr, ptr, ptr, env, 
i32)
 
 DEF_HELPER_6(vfwmaccbf16_vv, void, ptr, ptr, ptr, ptr, env, i32)
 DEF_HELPER_6(vfwmaccbf16_vf, void, ptr, ptr, i64, ptr, env, i32)
+
+/* Vector crypto functions */
+DEF_HELPER_6(vclmul_vv, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vclmul_vx, void, ptr, ptr, tl, ptr, env, i32)
+DEF_HELPER_6(vclmulh_vv, void, ptr, ptr, ptr, ptr, env, i32)
+DEF_HELPER_6(vclmulh_vx, void, ptr, ptr, tl, ptr, env, i32)
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index e341fa92139..dd50d5a48c7 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -946,3 +946,9 @@ vfwcvtbf16_f_f_v  010010 . . 01101 001 . 1010111 
@r2_vm
 # *** Zvfbfwma Standard Extension ***
 vfwmaccbf16_vv111011 . . . 001 . 1010111 @r_vm
 vfwmaccbf16_vf111011 . . . 101 . 1010111 @r_vm
+
+# *** Zvbc vector crypto extension ***
+vclmul_vv   001100 . . . 010 . 1010111 @r_vm
+vclmul_vx   001100 . . . 110 . 1010111 @r_vm
+vclmulh_vv  001101 . . . 010 . 1010111 @r_vm
+vclmulh_vx  001101 . . . 110 . 1010111 @r_vm
diff --git a/target/riscv/insn_trans/trans_rvvk.c.inc 
b/target/riscv/insn_trans/trans_rvvk.c.inc
new file mode 100644
index 000..552b08a2fd1
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvvk.c.inc
@@ -0,0 +1,62 @@
+/*
+ * RISC-V translation routines for the vector crypto extension.
+ *
+ * Copyright (C) 2023 SiFive, Inc.
+ * Written by Codethink Ltd and SiFive.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.

[PATCH v8 06/15] target/riscv: Refactor translation of vector-widening instruction

From: Dickon Hood 

Zvbb (implemented in later commit) has a widening instruction, which
requires an extra check on the enabled extensions.  Refactor
GEN_OPIVX_WIDEN_TRANS() to take a check function to avoid reimplementing
it.

Signed-off-by: Dickon Hood 
Reviewed-by: Richard Henderson 
Reviewed-by: Weiwei Li 
Signed-off-by: Max Chou 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 52 +++--
 1 file changed, 23 insertions(+), 29 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 5dfd524c7d2..a5562505531 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1526,30 +1526,24 @@ static bool opivx_widen_check(DisasContext *s, arg_rmrr 
*a)
vext_check_ds(s, a->rd, a->rs2, a->vm);
 }
 
-static bool do_opivx_widen(DisasContext *s, arg_rmrr *a,
-   gen_helper_opivx *fn)
-{
-if (opivx_widen_check(s, a)) {
-return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fn, s);
-}
-return false;
-}
-
-#define GEN_OPIVX_WIDEN_TRANS(NAME) \
-static bool trans_##NAME(DisasContext *s, arg_rmrr *a)   \
-{\
-static gen_helper_opivx * const fns[3] = {   \
-gen_helper_##NAME##_b,   \
-gen_helper_##NAME##_h,   \
-gen_helper_##NAME##_w\
-};   \
-return do_opivx_widen(s, a, fns[s->sew]);\
+#define GEN_OPIVX_WIDEN_TRANS(NAME, CHECK) \
+static bool trans_##NAME(DisasContext *s, arg_rmrr *a)\
+{ \
+if (CHECK(s, a)) {\
+static gen_helper_opivx * const fns[3] = {\
+gen_helper_##NAME##_b,\
+gen_helper_##NAME##_h,\
+gen_helper_##NAME##_w \
+};\
+return opivx_trans(a->rd, a->rs1, a->rs2, a->vm, fns[s->sew], s); \
+} \
+return false; \
 }
 
-GEN_OPIVX_WIDEN_TRANS(vwaddu_vx)
-GEN_OPIVX_WIDEN_TRANS(vwadd_vx)
-GEN_OPIVX_WIDEN_TRANS(vwsubu_vx)
-GEN_OPIVX_WIDEN_TRANS(vwsub_vx)
+GEN_OPIVX_WIDEN_TRANS(vwaddu_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwadd_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwsubu_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwsub_vx, opivx_widen_check)
 
 /* WIDEN OPIVV with WIDEN */
 static bool opiwv_widen_check(DisasContext *s, arg_rmrr *a)
@@ -1997,9 +1991,9 @@ GEN_OPIVX_TRANS(vrem_vx, opivx_check)
 GEN_OPIVV_WIDEN_TRANS(vwmul_vv, opivv_widen_check)
 GEN_OPIVV_WIDEN_TRANS(vwmulu_vv, opivv_widen_check)
 GEN_OPIVV_WIDEN_TRANS(vwmulsu_vv, opivv_widen_check)
-GEN_OPIVX_WIDEN_TRANS(vwmul_vx)
-GEN_OPIVX_WIDEN_TRANS(vwmulu_vx)
-GEN_OPIVX_WIDEN_TRANS(vwmulsu_vx)
+GEN_OPIVX_WIDEN_TRANS(vwmul_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmulu_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmulsu_vx, opivx_widen_check)
 
 /* Vector Single-Width Integer Multiply-Add Instructions */
 GEN_OPIVV_TRANS(vmacc_vv, opivv_check)
@@ -2015,10 +2009,10 @@ GEN_OPIVX_TRANS(vnmsub_vx, opivx_check)
 GEN_OPIVV_WIDEN_TRANS(vwmaccu_vv, opivv_widen_check)
 GEN_OPIVV_WIDEN_TRANS(vwmacc_vv, opivv_widen_check)
 GEN_OPIVV_WIDEN_TRANS(vwmaccsu_vv, opivv_widen_check)
-GEN_OPIVX_WIDEN_TRANS(vwmaccu_vx)
-GEN_OPIVX_WIDEN_TRANS(vwmacc_vx)
-GEN_OPIVX_WIDEN_TRANS(vwmaccsu_vx)
-GEN_OPIVX_WIDEN_TRANS(vwmaccus_vx)
+GEN_OPIVX_WIDEN_TRANS(vwmaccu_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmacc_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmaccsu_vx, opivx_widen_check)
+GEN_OPIVX_WIDEN_TRANS(vwmaccus_vx, opivx_widen_check)
 
 /* Vector Integer Merge and Move Instructions */
 static bool trans_vmv_v_v(DisasContext *s, arg_vmv_v_v *a)
-- 
2.34.1

[PATCH v8 03/15] target/riscv: Remove redundant "cpu_vl == 0" checks

From: Nazar Kazakov 

Remove the redundant "vl == 0" check which is already included within the  
vstart >= vl check, when vl == 0.

Signed-off-by: Nazar Kazakov 
Reviewed-by: Weiwei Li 
Signed-off-by: Max Chou 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 31 +
 1 file changed, 1 insertion(+), 30 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 4a8e62a8bef..7e194aae34a 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -617,7 +617,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 TCGv_i32 desc;
 
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 dest = tcg_temp_new_ptr();
@@ -786,7 +785,6 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, 
uint32_t rs2,
 TCGv_i32 desc;
 
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 dest = tcg_temp_new_ptr();
@@ -893,7 +891,6 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2,
 TCGv_i32 desc;
 
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 dest = tcg_temp_new_ptr();
@@ -1034,7 +1031,6 @@ static bool ldff_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 TCGv_i32 desc;
 
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 dest = tcg_temp_new_ptr();
@@ -1191,7 +1187,6 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn 
*gvec_fn,
 return false;
 }
 
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 if (a->vm && s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
@@ -1241,7 +1236,6 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2, uint32_t vm,
 uint32_t data = 0;
 
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 dest = tcg_temp_new_ptr();
@@ -1405,7 +1399,6 @@ static bool opivi_trans(uint32_t vd, uint32_t imm, 
uint32_t vs2, uint32_t vm,
 uint32_t data = 0;
 
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 dest = tcg_temp_new_ptr();
@@ -1492,7 +1485,6 @@ static bool do_opivv_widen(DisasContext *s, arg_rmrr *a,
 if (checkfn(s, a)) {
 uint32_t data = 0;
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
@@ -1575,7 +1567,6 @@ static bool do_opiwv_widen(DisasContext *s, arg_rmrr *a,
 if (opiwv_widen_check(s, a)) {
 uint32_t data = 0;
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
@@ -1648,7 +1639,6 @@ static bool opivv_trans(uint32_t vd, uint32_t vs1, 
uint32_t vs2, uint32_t vm,
 {
 uint32_t data = 0;
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, vm);
@@ -1842,7 +1832,6 @@ static bool trans_##NAME(DisasContext *s, arg_rmrr *a)
 \
 gen_helper_##NAME##_w, \
 }; \
 TCGLabel *over = gen_new_label();  \
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);  \
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
\
 data = FIELD_DP32(data, VDATA, VM, a->vm); \
@@ -2054,7 +2043,6 @@ static bool trans_vmv_v_v(DisasContext *s, arg_vmv_v_v *a)
 gen_helper_vmv_v_v_w, gen_helper_vmv_v_v_d,
 };
 TCGLabel *over = gen_new_label();
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
 tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs1),
@@ -2078,7 +2066,6 @@ static bool trans_vmv_v_x(DisasContext *s, arg_vmv_v_x *a)
 vext_check_ss(s, a->rd, 0, 1)) {
 TCGv s1;
 TCGLabel *over = gen_new_label();
-

[PATCH v8 01/15] target/riscv: Refactor some of the generic vector functionality

From: Kiran Ostrolenk 

Take some functions/macros out of `vector_helper` and put them in a new
module called `vector_internals`. This ensures they can be used by both
vector and vector-crypto helpers (latter implemented in proceeding
commits).

Signed-off-by: Kiran Ostrolenk 
Reviewed-by: Weiwei Li 
Signed-off-by: Max Chou 
---
 target/riscv/meson.build|   1 +
 target/riscv/vector_helper.c| 201 +---
 target/riscv/vector_internals.c |  81 +
 target/riscv/vector_internals.h | 182 +
 4 files changed, 265 insertions(+), 200 deletions(-)
 create mode 100644 target/riscv/vector_internals.c
 create mode 100644 target/riscv/vector_internals.h

diff --git a/target/riscv/meson.build b/target/riscv/meson.build
index 7f56c5f88d4..c3801ee5e04 100644
--- a/target/riscv/meson.build
+++ b/target/riscv/meson.build
@@ -16,6 +16,7 @@ riscv_ss.add(files(
   'gdbstub.c',
   'op_helper.c',
   'vector_helper.c',
+  'vector_internals.c',
   'bitmanip_helper.c',
   'translate.c',
   'm128_helper.c',
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 71bb9b4457b..6434fd2f7e8 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -26,6 +26,7 @@
 #include "fpu/softfloat.h"
 #include "tcg/tcg-gvec-desc.h"
 #include "internals.h"
+#include "vector_internals.h"
 #include 
 
 target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong s1,
@@ -72,68 +73,6 @@ target_ulong HELPER(vsetvl)(CPURISCVState *env, target_ulong 
s1,
 return vl;
 }
 
-/*
- * Note that vector data is stored in host-endian 64-bit chunks,
- * so addressing units smaller than that needs a host-endian fixup.
- */
-#if HOST_BIG_ENDIAN
-#define H1(x)   ((x) ^ 7)
-#define H1_2(x) ((x) ^ 6)
-#define H1_4(x) ((x) ^ 4)
-#define H2(x)   ((x) ^ 3)
-#define H4(x)   ((x) ^ 1)
-#define H8(x)   ((x))
-#else
-#define H1(x)   (x)
-#define H1_2(x) (x)
-#define H1_4(x) (x)
-#define H2(x)   (x)
-#define H4(x)   (x)
-#define H8(x)   (x)
-#endif
-
-static inline uint32_t vext_nf(uint32_t desc)
-{
-return FIELD_EX32(simd_data(desc), VDATA, NF);
-}
-
-static inline uint32_t vext_vm(uint32_t desc)
-{
-return FIELD_EX32(simd_data(desc), VDATA, VM);
-}
-
-/*
- * Encode LMUL to lmul as following:
- * LMULvlmullmul
- *  1   000   0
- *  2   001   1
- *  4   010   2
- *  8   011   3
- *  -   100   -
- * 1/8  101  -3
- * 1/4  110  -2
- * 1/2  111  -1
- */
-static inline int32_t vext_lmul(uint32_t desc)
-{
-return sextract32(FIELD_EX32(simd_data(desc), VDATA, LMUL), 0, 3);
-}
-
-static inline uint32_t vext_vta(uint32_t desc)
-{
-return FIELD_EX32(simd_data(desc), VDATA, VTA);
-}
-
-static inline uint32_t vext_vma(uint32_t desc)
-{
-return FIELD_EX32(simd_data(desc), VDATA, VMA);
-}
-
-static inline uint32_t vext_vta_all_1s(uint32_t desc)
-{
-return FIELD_EX32(simd_data(desc), VDATA, VTA_ALL_1S);
-}
-
 /*
  * Get the maximum number of elements can be operated.
  *
@@ -152,21 +91,6 @@ static inline uint32_t vext_max_elems(uint32_t desc, 
uint32_t log2_esz)
 return scale < 0 ? vlenb >> -scale : vlenb << scale;
 }
 
-/*
- * Get number of total elements, including prestart, body and tail elements.
- * Note that when LMUL < 1, the tail includes the elements past VLMAX that
- * are held in the same vector register.
- */
-static inline uint32_t vext_get_total_elems(CPURISCVState *env, uint32_t desc,
-uint32_t esz)
-{
-uint32_t vlenb = simd_maxsz(desc);
-uint32_t sew = 1 << FIELD_EX64(env->vtype, VTYPE, VSEW);
-int8_t emul = ctzl(esz) - ctzl(sew) + vext_lmul(desc) < 0 ? 0 :
-  ctzl(esz) - ctzl(sew) + vext_lmul(desc);
-return (vlenb << emul) / esz;
-}
-
 static inline target_ulong adjust_addr(CPURISCVState *env, target_ulong addr)
 {
 return (addr & ~env->cur_pmmask) | env->cur_pmbase;
@@ -199,20 +123,6 @@ static void probe_pages(CPURISCVState *env, target_ulong 
addr,
 }
 }
 
-/* set agnostic elements to 1s */
-static void vext_set_elems_1s(void *base, uint32_t is_agnostic, uint32_t cnt,
-  uint32_t tot)
-{
-if (is_agnostic == 0) {
-/* policy undisturbed */
-return;
-}
-if (tot - cnt == 0) {
-return;
-}
-memset(base + cnt, -1, tot - cnt);
-}
-
 static inline void vext_set_elem_mask(void *v0, int index,
   uint8_t value)
 {
@@ -222,18 +132,6 @@ static inline void vext_set_elem_mask(void *v0, int index,
 ((uint64_t *)v0)[idx] = deposit64(old, pos, 1, value);
 }
 
-/*
- * Earlier designs (pre-0.9) had a varying number of bits
- * per mask value (MLEN). In the 0.9 design, MLEN=1.
- * (Section 4.5)
- */
-static inline int vext_elem_mask(void *v0, int index)
-{
-int idx = index / 64;
-int pos = index  % 64;
-return (((uint64_t *)v0)[idx]

[PATCH v8 02/15] target/riscv: Refactor vector-vector translation macro

From: Kiran Ostrolenk 

Refactor the non SEW-specific stuff out of `GEN_OPIVV_TRANS` into
function `opivv_trans` (similar to `opivi_trans`). `opivv_trans` will be
used in proceeding vector-crypto commits.

Signed-off-by: Kiran Ostrolenk 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
Reviewed-by: Weiwei Li 
Signed-off-by: Max Chou 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 62 +
 1 file changed, 32 insertions(+), 30 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index c2f7527f53f..4a8e62a8bef 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -1643,38 +1643,40 @@ GEN_OPIWX_WIDEN_TRANS(vwadd_wx)
 GEN_OPIWX_WIDEN_TRANS(vwsubu_wx)
 GEN_OPIWX_WIDEN_TRANS(vwsub_wx)
 
+static bool opivv_trans(uint32_t vd, uint32_t vs1, uint32_t vs2, uint32_t vm,
+gen_helper_gvec_4_ptr *fn, DisasContext *s)
+{
+uint32_t data = 0;
+TCGLabel *over = gen_new_label();
+tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
+tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
+
+data = FIELD_DP32(data, VDATA, VM, vm);
+data = FIELD_DP32(data, VDATA, LMUL, s->lmul);
+data = FIELD_DP32(data, VDATA, VTA, s->vta);
+data = FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);
+data = FIELD_DP32(data, VDATA, VMA, s->vma);
+tcg_gen_gvec_4_ptr(vreg_ofs(s, vd), vreg_ofs(s, 0), vreg_ofs(s, vs1),
+   vreg_ofs(s, vs2), cpu_env, s->cfg_ptr->vlen / 8,
+   s->cfg_ptr->vlen / 8, data, fn);
+mark_vs_dirty(s);
+gen_set_label(over);
+return true;
+}
+
 /* Vector Integer Add-with-Carry / Subtract-with-Borrow Instructions */
 /* OPIVV without GVEC IR */
-#define GEN_OPIVV_TRANS(NAME, CHECK)   \
-static bool trans_##NAME(DisasContext *s, arg_rmrr *a) \
-{  \
-if (CHECK(s, a)) { \
-uint32_t data = 0; \
-static gen_helper_gvec_4_ptr * const fns[4] = {\
-gen_helper_##NAME##_b, gen_helper_##NAME##_h,  \
-gen_helper_##NAME##_w, gen_helper_##NAME##_d,  \
-}; \
-TCGLabel *over = gen_new_label();  \
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);  \
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over); \
-   \
-data = FIELD_DP32(data, VDATA, VM, a->vm); \
-data = FIELD_DP32(data, VDATA, LMUL, s->lmul); \
-data = FIELD_DP32(data, VDATA, VTA, s->vta);   \
-data = \
-FIELD_DP32(data, VDATA, VTA_ALL_1S, s->cfg_vta_all_1s);\
-data = FIELD_DP32(data, VDATA, VMA, s->vma);   \
-tcg_gen_gvec_4_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, 0), \
-   vreg_ofs(s, a->rs1),\
-   vreg_ofs(s, a->rs2), cpu_env,   \
-   s->cfg_ptr->vlen / 8,   \
-   s->cfg_ptr->vlen / 8, data, \
-   fns[s->sew]);   \
-mark_vs_dirty(s);  \
-gen_set_label(over);   \
-return true;   \
-}  \
-return false;  \
+#define GEN_OPIVV_TRANS(NAME, CHECK) \
+static bool trans_##NAME(DisasContext *s, arg_rmrr *a)   \
+{\
+if (CHECK(s, a)) {   \
+static gen_helper_gvec_4_ptr * const fns[4] = {  \
+gen_helper_##NAME##_b, gen_helper_##NAME##_h,\
+gen_helper_##NAME##_w, gen_helper_##NAME##_d,\
+};   \
+return opivv_trans(a->rd, a->rs1, a->rs2, a->vm, fns[s->sew], s);\
+}\
+return false;\
 }
 
 /*
-- 
2.34.1

[PATCH v8 00/15] Add RISC-V vector cryptographic instruction set support

This patchset provides an implementation for Zvbb, Zvbc, Zvkned, Zvknh,
Zvksh, Zvkg, and Zvksed of the draft RISC-V vector cryptography
extensions as per the v20230620 version of the specification(1)(168e7b4).
This is an update to the patchset submitted to qemu-devel on 
Wed, 12 Jul 2023 00:31:21 +0800

v2:

Squashed commits into one commit per extension with separate
commits for each refactoring.
Unified trans_rvzvk*.c.inc files into one trans_rvvk.c.inc.
Style fixes in insn32.decode and other files.
Added macros for EGS values in translation functions.
Updated from v20230303 to v20230407 of the spec:
Zvkb has been split into Zvbb and Zvbc.
vbrev, vclz, vctz, vcpop and vwsll have been added to Zvbb.

v3:

New patch 03/19 removes redundant “cpu_vl == 0” checks from
trans_rvv.c.inc.
Introduction of new tcg ops has been factored out of patch 11/19
and into 09/19.
These ops are now added to non riscv-specific files.

v4:

New patch 08/17 fixes the tcg_gen_gvec_andcs temporary variable
issue.
Patch 09/17 fixes imm mode for vror.vi.
Rebased to riscv-to-apply.next branch (de395bb):
Replace vstart constraint checking by TCG op.
Verified by code examples provided by vector crypto spec repository
(riscv-crypto).

v5:

Imported aes-round.h for Zvkned extension.
Rebased to 20230620110758.787479-1-richard.hender...@linaro.org
Exposed the properties of v4 patch 17/17 to the patches that the
extension was added.
Removed v4 patch 08/17 that is queued to tcg-next.

v6:

Resent the same content becaue v5 was splitted by broken
git-send-mail

v7:

Fixed endian issues
Replace the TCG ops of vstart & vl EGS checking by helper function
Changed the SEW selection of vsha2c[hl].vv to TCG translation

v8:

Rebased to the riscv-to-apply.next branch
Fixed cross win32 build issue
Removed redundent swap in AES key expanding (vaeskf1 & vaeskf2)


As v20230620 is a frozen version, we are not expecting any significant
changes to the specification or this patch series.

Please note that the Zvkt data-independent execution latency extension
(and all extensions including it) has not been implemented, and we
would recommend not using these patches in an environment where timing
attacks are an issue.

Work performed by Dickon, Lawrence, Nazar, Kiran, and William from
Codethink sponsored by SiFive, as well as Max Chou and Frank Chang
from SiFive.

https://github.com/riscv/riscv-crypto/releases

Thanks to those who have already reviewed:

Daniel Henrique Barboza dbarb...@ventanamicro.com
[PATCH v4 09/17] target/riscv: Add Zvbb ISA extension support
[PATCH v4 10/17] target/riscv: Add Zvkned ISA extension support
[PATCH v4 11/17] target/riscv: Add Zvknh ISA extension support
[PATCH v4 12/17] target/riscv: Add Zvksh ISA extension support
[PATCH v4 13/17] target/riscv: Add Zvkg ISA extension support
Weiwei Li liwei...@iscas.ac.cn
[PATCH v3 01/19] target/riscv: Refactor some of the generic vector 
functionality
[PATCH v3 02/19] target/riscv: Refactor vector-vector translation macro
[PATCH v3 03/19] target/riscv: Remove redundant "cpu_vl == 0" checks
[PATCH v3 05/19] target/riscv: Move vector translation checks
[PATCH v3 06/19] target/riscv: Refactor translation of vector-widening 
instruction
[PATCH v3 07/19] target/riscv: Refactor some of the generic vector 
functionality
[PATCH v3 19/19] target/riscv: Expose Zvk* and Zvb[b, c] cpu properties
Richard Henderson richard.hender...@linaro.org
[PATCH v2 02/17] target/riscv: Refactor vector-vector translation macro
[PATCH v2 04/17] target/riscv: Move vector translation checks
[PATCH v2 05/17] target/riscv: Refactor translation of vector-widening 
instruction
[PATCH v2 07/17] qemu/bitops.h: Limit rotate amounts
[PATCH v2 08/17] qemu/host-utils.h: Add clz and ctz functions for 
lower-bit integers
[PATCH v2 14/17] crypto: Create sm4_subword
Alistair Francis alistair.fran...@wdc.com
[PATCH v2 02/17] target/riscv: Refactor vector-vector translation macro
Philipp Tomsich philipp.toms...@vrull.eu
Various v1 reviews
Christoph Müllner christoph.muell...@vrull.eu
Various v1 reviews

Dickon Hood (2):
  target/riscv: Refactor translation of vector-widening instruction
  target/riscv: Add Zvbb ISA extension support

Kiran Ostrolenk (4):
  target/riscv: Refactor some of the generic vector functionality
  target/riscv: Refactor vector-vector translation macro
  target/riscv: Refactor some of the generic vector functionality
  target/riscv: Add Zvknh ISA extension support

Lawrence Hunter (2):
  target/riscv: Add Zvbc ISA extension support
  target/riscv: Add Zvksh ISA extension support

Max Chou (3):
  crypto: Create sm4_subword
  crypto: Add SM4 constant parameter CK

Re: [PATCH 1/3] block: add subcluster_size field to BlockDriverInfo

2023-07-11 Thread Denis V. Lunev


On 6/26/23 18:08, Andrey Drobyshev wrote:

This is going to be used in the subsequent commit as requests alignment
(in particular, during copy-on-read).  This value only makes sense for
the formats which support subclusters (currently QCOW2 only).  If this
field isn't set by driver's own bdrv_get_info() implementation, we
simply set it equal to the cluster size thus treating each cluster as having
a single subcluster.

Signed-off-by: Andrey Drobyshev 
---
  block.c  | 7 +++
  block/qcow2.c| 1 +
  include/block/block-common.h | 5 +
  3 files changed, 13 insertions(+)

diff --git a/block.c b/block.c
index 0637265c26..4fe1743cfb 100644
--- a/block.c
+++ b/block.c
@@ -6392,6 +6392,13 @@ int coroutine_fn bdrv_co_get_info(BlockDriverState *bs, 
BlockDriverInfo *bdi)
  }
  memset(bdi, 0, sizeof(*bdi));
  ret = drv->bdrv_co_get_info(bs, bdi);
+if (bdi->subcluster_size == 0) {
+/*
+ * If the driver left this unset, subclusters either not supported.
+ * Then it is safe to treat each cluster as having only one subcluster.
+ */
+bdi->subcluster_size = bdi->cluster_size;
+}
  if (ret < 0) {
  return ret;
  }
diff --git a/block/qcow2.c b/block/qcow2.c
index e23edd48c2..94b8d0e1c2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -5197,6 +5197,7 @@ qcow2_co_get_info(BlockDriverState *bs, BlockDriverInfo 
*bdi)
  {
  BDRVQcow2State *s = bs->opaque;
  bdi->cluster_size = s->cluster_size;
+bdi->subcluster_size = s->subcluster_size;
  bdi->vm_state_offset = qcow2_vm_state_offset(s);
  bdi->is_dirty = s->incompatible_features & QCOW2_INCOMPAT_DIRTY;
  return 0;
diff --git a/include/block/block-common.h b/include/block/block-common.h
index e15395f2cb..df5ffc8d09 100644
--- a/include/block/block-common.h
+++ b/include/block/block-common.h
@@ -132,6 +132,11 @@ typedef struct BlockZoneWps {
  typedef struct BlockDriverInfo {
  /* in bytes, 0 if irrelevant */
  int cluster_size;
+/*
+ * A fraction of cluster_size, if supported (currently QCOW2 only); if
+ * disabled or unsupported, set equal to cluster_size.
+ */
+int subcluster_size;
  /* offset at which the VM state can be saved (0 if not possible) */
  int64_t vm_state_offset;
  bool is_dirty;

Reviewed-by: Denis V. Lunev

[PATCH for-8.1] accel/tcg: Zero-pad PC in TCG CPU exec trace lines

In commit f0a08b0913befbd we changed the type of the PC from
target_ulong to vaddr.  In doing so we inadvertently dropped the
zero-padding on the PC in trace lines (the second item inside the []
in these lines).  They used to look like this on AArch64, for
instance:

Trace 0: 0x7f226100 [/4000/0061/ff20]

and now they look like this:
Trace 0: 0x7f4f5100 [/4000/0061/ff20]

and if the PC happens to be somewhere low like 0x5000
then the field is shown as /5000/.

This is because TARGET_FMT_lx is a "%08x" or "%016x" specifier,
depending on TARGET_LONG_SIZE, whereas VADDR_PRIx is just PRIx64
with no width specifier.

Restore the zero-padding by adding an 016 width specifier to
this tracing and a couple of others that were similarly recently
changed to use VADDR_PRIx without a width specifier.

We can't unfortunately restore the "32-bit guests are padded to
8 hex digits and 64-bit guests to 16 hex digits" behaviour so
easily.

Fixes: f0a08b0913befbd ("accel/tcg/cpu-exec.c: Widen pc to vaddr")
Signed-off-by: Peter Maydell 
---
I have a workflow that parses log files to get the executed
PC values; I don't suppose I'm the only one doing that...
---
 accel/tcg/cpu-exec.c  | 4 ++--
 accel/tcg/translate-all.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index ba1890a373d..db1e82811fa 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -298,7 +298,7 @@ static void log_cpu_exec(vaddr pc, CPUState *cpu,
 if (qemu_log_in_addr_range(pc)) {
 qemu_log_mask(CPU_LOG_EXEC,
   "Trace %d: %p [%08" PRIx64
-  "/%" VADDR_PRIx "/%08x/%08x] %s\n",
+  "/%016" VADDR_PRIx "/%08x/%08x] %s\n",
   cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc,
   tb->flags, tb->cflags, lookup_symbol(pc));
 
@@ -487,7 +487,7 @@ cpu_tb_exec(CPUState *cpu, TranslationBlock *itb, int 
*tb_exit)
 if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
 vaddr pc = log_pc(cpu, last_tb);
 if (qemu_log_in_addr_range(pc)) {
-qemu_log("Stopped execution of TB chain before %p [%"
+qemu_log("Stopped execution of TB chain before %p [%016"
  VADDR_PRIx "] %s\n",
  last_tb->tc.ptr, pc, lookup_symbol(pc));
 }
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index d3d4fbc1a41..bb225afa04f 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -604,7 +604,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
 if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
 vaddr pc = log_pc(cpu, tb);
 if (qemu_log_in_addr_range(pc)) {
-qemu_log("cpu_io_recompile: rewound execution of TB to %"
+qemu_log("cpu_io_recompile: rewound execution of TB to %016"
  VADDR_PRIx "\n", pc);
 }
 }
-- 
2.34.1

Re: riscv kvm breakage

2023-07-11 Thread Palmer Dabbelt


On Tue, 11 Jul 2023 09:43:48 PDT (-0700), Richard Henderson wrote:

Hiya,

This breakage crept in while cross-riscv64-system was otherwise broken in 
configure:

https://gitlab.com/qemu-project/qemu/-/jobs/4633277557#L4165

../target/riscv/kvm.c:209:38: error: ‘KVM_RISCV_ISA_EXT_ZICBOZ’ undeclared here 
(not in a
function); did you mean ‘KVM_RISCV_ISA_EXT_ZICBOM’?
   209 | KVM_EXT_CFG("zicboz", ext_icboz, KVM_RISCV_ISA_EXT_ZICBOZ),
   |  ^~~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |^~~
../target/riscv/kvm.c:211:33: error: ‘KVM_RISCV_ISA_EXT_ZBB’ undeclared here 
(not in a
function); did you mean ‘KVM_RISCV_ISA_EXT_MAX’?
   211 | KVM_EXT_CFG("zbb", ext_zbb, KVM_RISCV_ISA_EXT_ZBB),
   | ^
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |^~~
../target/riscv/kvm.c:212:37: error: ‘KVM_RISCV_ISA_EXT_SSAIA’ undeclared here 
(not in a
function); did you mean ‘KVM_RISCV_ISA_EXT_SSTC’?
   212 | KVM_EXT_CFG("ssaia", ext_ssaia, KVM_RISCV_ISA_EXT_SSAIA),
   | ^~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |^~~
In file included from /usr/riscv64-linux-gnu/include/rpc/netdb.h:42,
  from /usr/riscv64-linux-gnu/include/netdb.h:32,
  from /builds/qemu-project/qemu/include/sysemu/os-posix.h:34,
  from /builds/qemu-project/qemu/include/qemu/osdep.h:151,
  from ../target/riscv/kvm.c:19:
../target/riscv/kvm.c:288:44: error: ‘struct kvm_riscv_config’ has no member 
named
‘zicboz_block_size’; did you mean ‘zicbom_block_size’?
   288 | .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)
   |^

Can someone have a look asap?  Thanks,


+Phillippe and Michael, there's already a discussion on IRC and sounds 
like there's a fix?





r~

Re: riscv kvm breakage


On 11/7/23 18:43, Richard Henderson wrote:

Hiya,

This breakage crept in while cross-riscv64-system was otherwise broken 
in configure:


https://gitlab.com/qemu-project/qemu/-/jobs/4633277557#L4165

../target/riscv/kvm.c:209:38: error: ‘KVM_RISCV_ISA_EXT_ZICBOZ’ 
undeclared here (not in a function); did you mean 
‘KVM_RISCV_ISA_EXT_ZICBOM’?

   209 | KVM_EXT_CFG("zicboz", ext_icboz, KVM_RISCV_ISA_EXT_ZICBOZ),
   |  ^~~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |    ^~~
../target/riscv/kvm.c:211:33: error: ‘KVM_RISCV_ISA_EXT_ZBB’ undeclared 
here (not in a function); did you mean ‘KVM_RISCV_ISA_EXT_MAX’?

   211 | KVM_EXT_CFG("zbb", ext_zbb, KVM_RISCV_ISA_EXT_ZBB),
   | ^
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |    ^~~
../target/riscv/kvm.c:212:37: error: ‘KVM_RISCV_ISA_EXT_SSAIA’ 
undeclared here (not in a function); did you mean ‘KVM_RISCV_ISA_EXT_SSTC’?

   212 | KVM_EXT_CFG("ssaia", ext_ssaia, KVM_RISCV_ISA_EXT_SSAIA),
   | ^~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
   205 |  .kvm_reg_id = _reg_id}
   |    ^~~
In file included from /usr/riscv64-linux-gnu/include/rpc/netdb.h:42,
  from /usr/riscv64-linux-gnu/include/netdb.h:32,
  from 
/builds/qemu-project/qemu/include/sysemu/os-posix.h:34,

  from /builds/qemu-project/qemu/include/qemu/osdep.h:151,
  from ../target/riscv/kvm.c:19:
../target/riscv/kvm.c:288:44: error: ‘struct kvm_riscv_config’ has no 
member named ‘zicboz_block_size’; did you mean ‘zicbom_block_size’?

   288 | .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)
   |    ^

Can someone have a look asap?  Thanks,


See:
https://lore.kernel.org/qemu-devel/20230711163346.69409-1-phi...@linaro.org/

:)

riscv kvm breakage

2023-07-11 Thread Richard Henderson


Hiya,

This breakage crept in while cross-riscv64-system was otherwise broken in 
configure:

https://gitlab.com/qemu-project/qemu/-/jobs/4633277557#L4165

../target/riscv/kvm.c:209:38: error: ‘KVM_RISCV_ISA_EXT_ZICBOZ’ undeclared here (not in a 
function); did you mean ‘KVM_RISCV_ISA_EXT_ZICBOM’?

  209 | KVM_EXT_CFG("zicboz", ext_icboz, KVM_RISCV_ISA_EXT_ZICBOZ),
  |  ^~~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
  205 |  .kvm_reg_id = _reg_id}
  |^~~
../target/riscv/kvm.c:211:33: error: ‘KVM_RISCV_ISA_EXT_ZBB’ undeclared here (not in a 
function); did you mean ‘KVM_RISCV_ISA_EXT_MAX’?

  211 | KVM_EXT_CFG("zbb", ext_zbb, KVM_RISCV_ISA_EXT_ZBB),
  | ^
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
  205 |  .kvm_reg_id = _reg_id}
  |^~~
../target/riscv/kvm.c:212:37: error: ‘KVM_RISCV_ISA_EXT_SSAIA’ undeclared here (not in a 
function); did you mean ‘KVM_RISCV_ISA_EXT_SSTC’?

  212 | KVM_EXT_CFG("ssaia", ext_ssaia, KVM_RISCV_ISA_EXT_SSAIA),
  | ^~~
../target/riscv/kvm.c:205:20: note: in definition of macro ‘KVM_EXT_CFG’
  205 |  .kvm_reg_id = _reg_id}
  |^~~
In file included from /usr/riscv64-linux-gnu/include/rpc/netdb.h:42,
 from /usr/riscv64-linux-gnu/include/netdb.h:32,
 from /builds/qemu-project/qemu/include/sysemu/os-posix.h:34,
 from /builds/qemu-project/qemu/include/qemu/osdep.h:151,
 from ../target/riscv/kvm.c:19:
../target/riscv/kvm.c:288:44: error: ‘struct kvm_riscv_config’ has no member named 
‘zicboz_block_size’; did you mean ‘zicbom_block_size’?

  288 | .kvm_reg_id = KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)
  |^

Can someone have a look asap?  Thanks,


r~

Re: [PULL 32/52] exec-all: Widen TranslationBlock pc and cs_base to 64-bits

On Mon, 5 Jun 2023 at 21:27, Richard Henderson
 wrote:
>
> This makes TranslationBlock agnostic to the address size of the guest.
> Use vaddr for pc, since that's always a virtual address.
> Use uint64_t for cs_base, since usage varies between guests.

> index 60ca9e229e..1cf4f1fa22 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -297,7 +297,7 @@ static void log_cpu_exec(target_ulong pc, CPUState *cpu,
>  {
>  if (qemu_log_in_addr_range(pc)) {
>  qemu_log_mask(CPU_LOG_EXEC,
> -  "Trace %d: %p [" TARGET_FMT_lx
> +  "Trace %d: %p [%08" PRIx64
>"/" TARGET_FMT_lx "/%08x/%08x] %s\n",
>cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc,
>tb->flags, tb->cflags, lookup_symbol(pc));

TARGET_FMT_lx zero-pads appropriately to the size of
target_ulong, which is what cs_base used to be.
Now we have an explicit %08, which will sometimes
be too small for cs_base if the guest really uses all
64 bits of it. Is that intentional ?

thanks
-- PMM

Re: [PULL 06/22] accel/tcg/cpu-exec.c: Widen pc to vaddr

On Mon, 26 Jun 2023 at 16:48, Richard Henderson
 wrote:
>
> From: Anton Johansson 
>
> Signed-off-by: Anton Johansson 
> Reviewed-by: Richard Henderson 
> Message-Id: <20230621135633.1649-7-a...@rev.ng>
> Signed-off-by: Richard Henderson 

> -static void log_cpu_exec(target_ulong pc, CPUState *cpu,
> +static void log_cpu_exec(vaddr pc, CPUState *cpu,
>   const TranslationBlock *tb)
>  {
>  if (qemu_log_in_addr_range(pc)) {
>  qemu_log_mask(CPU_LOG_EXEC,
>"Trace %d: %p [%08" PRIx64
> -  "/" TARGET_FMT_lx "/%08x/%08x] %s\n",
> +  "/%" VADDR_PRIx "/%08x/%08x] %s\n",
>cpu->cpu_index, tb->tc.ptr, tb->cs_base, pc,
>tb->flags, tb->cflags, lookup_symbol(pc));

This again has lost the zero-padding. I noticed this one because
I have a workflow where I post-process these log files to
extract the executed PC values.

We've also lost the "PC is padded to the appropriate size
depending on whether this is a 32-bit or 64-bit guest CPU",
which is a bit of a shame.

thanks
-- PMM

Re: [PULL 02/22] accel/tcg/translate-all.c: Widen pc and cs_base

On Mon, 26 Jun 2023 at 16:40, Richard Henderson
 wrote:
>
> From: Anton Johansson 
>
> Signed-off-by: Anton Johansson 
> Reviewed-by: Richard Henderson 
> Message-Id: <20230621135633.1649-3-a...@rev.ng>
> Signed-off-by: Richard Henderson 
> ---

> @@ -634,10 +634,10 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
>  cpu->cflags_next_tb = curr_cflags(cpu) | CF_MEMI_ONLY | CF_LAST_IO | n;
>
>  if (qemu_loglevel_mask(CPU_LOG_EXEC)) {
> -target_ulong pc = log_pc(cpu, tb);
> +vaddr pc = log_pc(cpu, tb);
>  if (qemu_log_in_addr_range(pc)) {
> -qemu_log("cpu_io_recompile: rewound execution of TB to "
> - TARGET_FMT_lx "\n", pc);
> +qemu_log("cpu_io_recompile: rewound execution of TB to %"
> + VADDR_PRIx "\n", pc);

TARGET_FMT_lx includes a width specifier so the value is
left-zero-padded to the appropriate width (depending
on the size of the PC on the target). VADDR_PRIx does
not zero-pad. Intentional change?

thanks
-- PMM

Re: [hotfix PATCH-for-8.1 v4] configure: Fix cross-building for RISCV host

2023-07-11 Thread Richard Henderson


On 7/11/23 12:06, Philippe Mathieu-Daudé wrote:

While when building on native Linux the host architecture
is reported as "riscv32" or "riscv64":

   Host machine cpu family: riscv64
   Host machine cpu: riscv64
   Found pkg-config: /usr/bin/pkg-config (0.29.2)

Since commit ba0e733362 ("configure: Merge riscv32 and riscv64
host architectures"), when cross-compiling it is detected as
"riscv". Meson handles the cross-detection but displays a warning:

   WARNING: Unknown CPU family riscv, please report this at 
https://github.com/mesonbuild/meson/issues/new
   Host machine cpu family: riscv
   Host machine cpu: riscv
   Target machine cpu family: riscv
   Target machine cpu: riscv
   Found pkg-config: /usr/bin/riscv64-linux-gnu-pkg-config (1.8.1)

Now since commit 278c1bcef5 ("target/riscv: Only unify 'riscv32/64'
-> 'riscv' for host cpu in meson") Meson expects the cpu to be in
[riscv32, riscv64]. So when cross-building (for example on our
cross-riscv64-system Gitlab-CI job) we get:

   WARNING: Unknown CPU family riscv, please report this at 
https://github.com/mesonbuild/meson/issues/new
   Host machine cpu family: riscv
   Host machine cpu: riscv
   Target machine cpu family: riscv
   Target machine cpu: riscv
   ../meson.build:684:6: ERROR: Problem encountered: Unsupported CPU riscv, try 
--enable-tcg-interpreter

Fix by partially revert commit ba0e733362 so when cross-building
the ./configure script passes the proper host architecture to meson.

Fixes: ba0e733362 ("configure: Merge riscv32 and riscv64 host architectures")
Fixes: 278c1bcef5 ("target/riscv: Only unify 'riscv32/64' -> 'riscv' for host cpu in 
meson")
Reported-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
v4: pass from configure


Applied to master.


r~



---
  configure | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 2b41c49c0d..dffd44c059 100755
--- a/configure
+++ b/configure
@@ -451,7 +451,11 @@ elif check_define __s390__ ; then
  cpu="s390"
fi
  elif check_define __riscv ; then
-  cpu="riscv"
+  if check_define _LP64 ; then
+cpu="riscv64"
+  else
+cpu="riscv32"
+  fi
  elif check_define __arm__ ; then
cpu="arm"
  elif check_define __aarch64__ ; then

Re: [PATCH v2] s390x: Fix QEMU abort by selecting S390_FLIC_KVM


On 11/7/23 17:14, Cédric Le Goater wrote:

If QEMU is built with --without-default-devices, the s390-flic-kvm
device is missing and QEMU aborts when started with the KVM accelerator.
Make sure it's available by selecting S390_FLIC_KVM in Kconfig.

Consequently, this also fixes an abort in tests/qtest/migration-test.

Signed-off-by: Cédric Le Goater 
---
  hw/intc/Kconfig  | 1 -
  hw/s390x/Kconfig | 1 +
  2 files changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé

[hotfix PATCH-for-8.1 v5] configure: Fix cross-building for RISCV host

While when building on native Linux the host architecture
is reported as "riscv32" or "riscv64":

  Host machine cpu family: riscv64
  Host machine cpu: riscv64
  Found pkg-config: /usr/bin/pkg-config (0.29.2)

Since commit ba0e733362 ("configure: Merge riscv32 and riscv64
host architectures"), when cross-compiling it is detected as
"riscv". Meson handles the cross-detection but displays a warning:

  WARNING: Unknown CPU family riscv, please report this at 
https://github.com/mesonbuild/meson/issues/new
  Host machine cpu family: riscv
  Host machine cpu: riscv
  Target machine cpu family: riscv
  Target machine cpu: riscv
  Found pkg-config: /usr/bin/riscv64-linux-gnu-pkg-config (1.8.1)

Now since commit 278c1bcef5 ("target/riscv: Only unify 'riscv32/64'
-> 'riscv' for host cpu in meson") Meson expects the cpu to be in
[riscv32, riscv64]. So when cross-building (for example on our
cross-riscv64-system Gitlab-CI job) we get:

  WARNING: Unknown CPU family riscv, please report this at 
https://github.com/mesonbuild/meson/issues/new
  Host machine cpu family: riscv
  Host machine cpu: riscv
  Target machine cpu family: riscv
  Target machine cpu: riscv
  ../meson.build:684:6: ERROR: Problem encountered: Unsupported CPU riscv, try 
--enable-tcg-interpreter

Fix by partially revert commit ba0e733362 so when cross-building
the ./configure script passes the proper host architecture to meson.

Update $linux_arch to keep using the shared linux-headers/asm-riscv/
include path.

Fixes: ba0e733362 ("configure: Merge riscv32 and riscv64 host architectures")
Fixes: 278c1bcef5 ("target/riscv: Only unify 'riscv32/64' -> 'riscv' for host 
cpu in meson")
Reported-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
v5: fix linux-headers path
---
 configure | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 2b41c49c0d..26ec5e4f54 100755
--- a/configure
+++ b/configure
@@ -451,7 +451,11 @@ elif check_define __s390__ ; then
 cpu="s390"
   fi
 elif check_define __riscv ; then
-  cpu="riscv"
+  if check_define _LP64 ; then
+cpu="riscv64"
+  else
+cpu="riscv32"
+  fi
 elif check_define __arm__ ; then
   cpu="arm"
 elif check_define __aarch64__ ; then
@@ -1722,6 +1726,9 @@ if test "$linux" = "yes" ; then
   mips64)
 linux_arch=mips
 ;;
+  riscv32|riscv64)
+linux_arch=riscv
+;;
   *)
 # For most CPUs the kernel architecture name and QEMU CPU name match.
 linux_arch="$cpu"
-- 
2.38.1

[PATCH 2/6] vhost-vdpa: Move vhost_vdpa_reset_status() up

The next commit is going to have vhost_vdpa_dev_start() call this, so
move it up to have the declaration where we are going to need it.

Signed-off-by: Hanna Czenczek 
---
 hw/virtio/vhost-vdpa.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 42f2a4bae9..7b7dee468e 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1286,6 +1286,20 @@ static void vhost_vdpa_suspend(struct vhost_dev *dev)
 vhost_vdpa_reset_device(dev);
 }
 
+static void vhost_vdpa_reset_status(struct vhost_dev *dev)
+{
+struct vhost_vdpa *v = dev->opaque;
+
+if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
+return;
+}
+
+vhost_vdpa_reset_device(dev);
+vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
+   VIRTIO_CONFIG_S_DRIVER);
+memory_listener_unregister(>listener);
+}
+
 static int vhost_vdpa_dev_start(struct vhost_dev *dev, bool started)
 {
 struct vhost_vdpa *v = dev->opaque;
@@ -1323,20 +1337,6 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, 
bool started)
 return 0;
 }
 
-static void vhost_vdpa_reset_status(struct vhost_dev *dev)
-{
-struct vhost_vdpa *v = dev->opaque;
-
-if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
-return;
-}
-
-vhost_vdpa_reset_device(dev);
-vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
-   VIRTIO_CONFIG_S_DRIVER);
-memory_listener_unregister(>listener);
-}
-
 static int vhost_vdpa_set_log_base(struct vhost_dev *dev, uint64_t base,
  struct vhost_log *log)
 {
-- 
2.41.0

[PATCH 6/6] vhost-user: Have reset_status fall back to reset

The only user of vhost_user_reset_status() is vhost_dev_stop(), which
only uses it as a fall-back to stop the back-end if it does not support
SUSPEND.  However, vhost-user's implementation is a no-op unless the
back-end supports SET_STATUS.

vhost-vdpa's implementation instead just calls
vhost_vdpa_reset_device(), implying that it's OK to fully reset the
device if SET_STATUS is not supported.

To be fair, vhost_vdpa_reset_device() does nothing but to set the status
to zero.  However, that may well be because vhost-vdpa has no method
besides this to reset a device.  In contrast, vhost-user has
RESET_DEVICE and a RESET_OWNER, which can be used instead.

While it is not entirely clear from documentation or git logs, from
discussions and the order of vhost-user protocol features, it appears to
me as if RESET_OWNER originally had no real meaning for vhost-user, and
was thus used to signal a device reset to the back-end.  Then,
RESET_DEVICE was introduced, to have a well-defined dedicated reset
command.  Finally, vhost-user received full STATUS support, including
SET_STATUS, so setting the device status to 0 is now the preferred way
of resetting a device.  Still, RESET_DEVICE and RESET_OWNER should
remain valid as fall-backs.

Therefore, have vhost_user_reset_status() fall back to
vhost_user_reset_device() if the back-end has no STATUS support.

Signed-off-by: Hanna Czenczek 
---
 hw/virtio/vhost-user.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 4507de5a92..53a881ec2a 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2833,6 +2833,8 @@ static void vhost_user_reset_status(struct vhost_dev *dev)
 if (virtio_has_feature(dev->protocol_features,
VHOST_USER_PROTOCOL_F_STATUS)) {
 vhost_user_set_status(dev, 0);
+} else {
+vhost_user_reset_device(dev);
 }
 }
 
-- 
2.41.0

[PATCH 5/6] vhost-vdpa: Match vhost-user's status reset

vhost-vdpa and vhost-user differ in how they reset the status in their
respective vhost_reset_status implementations: vhost-vdpa zeroes it,
then re-adds the S_ACKNOWLEDGE and S_DRIVER config bits.  S_DRIVER_OK is
then set in vhost_vdpa_dev_start().

vhost-user in contrast just zeroes the status, and does no re-add any
config bits until vhost_user_dev_start() (where it does re-add all of
S_ACKNOWLEDGE, S_DRIVER, and S_DRIVER_OK).

There is no documentation for vhost_reset_status, but its only caller is
vhost_dev_stop().  So apparently, the device is to be stopped after
vhost_reset_status, and therefore it makes more sense to keep the status
field fully cleared until the back-end is re-started, which is how
vhost-user does it.  Make vhost-vdpa do the same -- if nothing else it's
confusing to have both vhost implementations handle this differently.

Signed-off-by: Hanna Czenczek 
---
 hw/virtio/vhost-vdpa.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index f7fd19a203..0cde8b40de 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1294,8 +1294,6 @@ static void vhost_vdpa_reset_status(struct vhost_dev *dev)
 }
 
 vhost_vdpa_reset_device(dev);
-vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
-   VIRTIO_CONFIG_S_DRIVER);
 memory_listener_unregister(>listener);
 }
 
@@ -1334,7 +1332,9 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, 
bool started)
 }
 memory_listener_register(>listener, dev->vdev->dma_as);
 
-return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
+return vhost_vdpa_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
+  VIRTIO_CONFIG_S_DRIVER |
+  VIRTIO_CONFIG_S_DRIVER_OK);
 }
 
 return 0;
-- 
2.41.0

[PATCH 4/6] vhost-user: Implement suspend/resume

Implement SUSPEND/RESUME like vDPA does, by automatically using it in
vhost_user_dev_start().  (Though our vDPA code does not implement RESUME
yet, so there, the device is reset when it is to be resumed.)

Signed-off-by: Hanna Czenczek 
---
 hw/virtio/vhost-user.c | 99 +-
 1 file changed, 97 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8dcf049d42..4507de5a92 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -74,6 +74,8 @@ enum VhostUserProtocolFeature {
 /* Feature 14 reserved for VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS. */
 VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS = 15,
 VHOST_USER_PROTOCOL_F_STATUS = 16,
+/* Feature 17 reserved for VHOST_USER_PROTOCOL_F_XEN_MMAP. */
+VHOST_USER_PROTOCOL_F_SUSPEND = 18,
 VHOST_USER_PROTOCOL_F_MAX
 };
 
@@ -121,6 +123,8 @@ typedef enum VhostUserRequest {
 VHOST_USER_REM_MEM_REG = 38,
 VHOST_USER_SET_STATUS = 39,
 VHOST_USER_GET_STATUS = 40,
+VHOST_USER_SUSPEND = 41,
+VHOST_USER_RESUME = 42,
 VHOST_USER_MAX
 } VhostUserRequest;
 
@@ -1389,7 +1393,19 @@ static int vhost_user_set_u64(struct vhost_dev *dev, int 
request, uint64_t u64,
 
 static int vhost_user_set_status(struct vhost_dev *dev, uint8_t status)
 {
-return vhost_user_set_u64(dev, VHOST_USER_SET_STATUS, status, false);
+int ret;
+
+ret = vhost_user_set_u64(dev, VHOST_USER_SET_STATUS, status, false);
+if (ret < 0) {
+return ret;
+}
+
+if (!status) {
+/* reset */
+dev->suspended = false;
+}
+
+return 0;
 }
 
 static int vhost_user_get_status(struct vhost_dev *dev, uint8_t *status)
@@ -1490,6 +1506,7 @@ static int vhost_user_get_max_memslots(struct vhost_dev 
*dev,
 
 static int vhost_user_reset_device(struct vhost_dev *dev)
 {
+int ret;
 VhostUserMsg msg = {
 .hdr.flags = VHOST_USER_VERSION,
 };
@@ -1499,7 +1516,13 @@ static int vhost_user_reset_device(struct vhost_dev *dev)
 ? VHOST_USER_RESET_DEVICE
 : VHOST_USER_RESET_OWNER;
 
-return vhost_user_write(dev, , NULL, 0);
+ret = vhost_user_write(dev, , NULL, 0);
+if (ret < 0) {
+return ret;
+}
+
+dev->suspended = false;
+return 0;
 }
 
 static int vhost_user_backend_handle_config_change(struct vhost_dev *dev)
@@ -2707,8 +2730,80 @@ void vhost_user_async_close(DeviceState *d,
 }
 }
 
+static bool vhost_user_supports_suspend(struct vhost_dev *dev)
+{
+return virtio_has_feature(dev->protocol_features,
+  VHOST_USER_PROTOCOL_F_SUSPEND);
+}
+
+static int vhost_user_do_suspend_resume(struct vhost_dev *dev, bool suspend)
+{
+VhostUserMsg msg;
+bool reply_supported = virtio_has_feature(dev->protocol_features,
+  VHOST_USER_PROTOCOL_F_REPLY_ACK);
+VhostUserRequest request = suspend ? VHOST_USER_SUSPEND : 
VHOST_USER_RESUME;
+int ret;
+
+if (dev->suspended == suspend) {
+/* Nothing to do */
+return 0;
+}
+
+if (!vhost_user_supports_suspend(dev)) {
+return -ENOTSUP;
+}
+
+msg = (VhostUserMsg) {
+.hdr = {
+.request = request,
+.size = 0,
+.flags = VHOST_USER_VERSION,
+},
+};
+if (reply_supported) {
+msg.hdr.flags |= VHOST_USER_NEED_REPLY_MASK;
+}
+
+ret = vhost_user_write(dev, , NULL, 0);
+if (ret < 0) {
+return ret;
+}
+
+if (reply_supported) {
+ret = process_message_reply(dev, );
+if (ret < 0) {
+return ret;
+}
+}
+
+dev->suspended = suspend;
+return 0;
+}
+
+static int vhost_user_suspend(struct vhost_dev *dev)
+{
+return vhost_user_do_suspend_resume(dev, true);
+}
+
+static int vhost_user_resume(struct vhost_dev *dev)
+{
+return vhost_user_do_suspend_resume(dev, false);
+}
+
 static int vhost_user_dev_start(struct vhost_dev *dev, bool started)
 {
+/*
+ * Ignore results.  If the central vhost code cares, it will check
+ * dev->suspended.  (These calls will fail if the back-end does not
+ * support suspend/resume, which callers that just want to start the
+ * device do not care about.)
+ */
+if (started) {
+vhost_user_resume(dev);
+} else {
+vhost_user_suspend(dev);
+}
+
 if (!virtio_has_feature(dev->protocol_features,
 VHOST_USER_PROTOCOL_F_STATUS)) {
 return 0;
-- 
2.41.0

[PATCH 0/6] vhost-user: Add suspend/resume

Hi,

As discussed on the previous version of the virtio-fs migration series
(https://lists.nongnu.org/archive/html/qemu-devel/2023-04/msg01575.html),
we currently don’t have a good way to have a vhost-user back-end fully
cease all operations, including background operations.  To work around
this, we reset it, which is not an option for stateful devices like
virtio-fs.

Instead, we want the same SUSPEND/RESUME model that vhost-vdpa already
has, so that we can suspend back-ends when we want them to stop doing
anything (i.e. on VM stop), and resume them later (i.e. on VM resume).
This series adds these vhost-user operations to the protocol and
implements them in qemu.  Furthermore, it has vhost-user and vhost-vdpa
do roughly the same thing in their reset paths, as far as possible.
That path will still remain as a fall-back if SUSPEND/RESUME is not
implemented, and, given that qemu’s vhost-vdpa code currently does not
make use of RESUME, it is actually always used for vhost-vdpa (to take
the device out of a suspended state).


Hanna Czenczek (6):
  vhost-user.rst: Add suspend/resume
  vhost-vdpa: Move vhost_vdpa_reset_status() up
  vhost: Do not reset suspended devices on stop
  vhost-user: Implement suspend/resume
  vhost-vdpa: Match vhost-user's status reset
  vhost-user: Have reset_status fall back to reset

 docs/interop/vhost-user.rst|  35 +++-
 include/hw/virtio/vhost-vdpa.h |   2 -
 include/hw/virtio/vhost.h  |   8 +++
 hw/virtio/vhost-user.c | 101 -
 hw/virtio/vhost-vdpa.c |  41 ++---
 hw/virtio/vhost.c  |   8 ++-
 6 files changed, 169 insertions(+), 26 deletions(-)

-- 
2.41.0

[PATCH 1/6] vhost-user.rst: Add suspend/resume

When stopping the VM, qemu wants all devices to fully cease any
operation, too.  Currently, we can only have vhost-user back-ends stop
processing vrings, but not background operations.  Add the SUSPEND and
RESUME commands from vDPA, which allow the front-end (qemu) to tell
back-ends to cease all operations, including those running in the
background.

qemu's current work-around for this is to reset the back-end instead of
suspending it, which will not work for back-ends that have internal
state that must be preserved across e.g. stop/cont.

Note that the given specification requires the back-end to delay
processing kicks (i.e. starting vrings) until the device is resumed,
instead of requiring the front-end not to send kicks while suspended.
qemu starts devices (and would just resume them) only when the VM is in
a running state, so it would be difficult to have qemu delay kicks until
the device is resumed, which is why this patch specifies handling of
kicks as it does.

Signed-off-by: Hanna Czenczek 
---
 docs/interop/vhost-user.rst | 35 +--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index 5a070adbc1..ac6be34c4c 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -381,6 +381,10 @@ readable) on the descriptor specified by 
``VHOST_USER_SET_VRING_KICK``
 or receiving the in-band message ``VHOST_USER_VRING_KICK`` if negotiated,
 and stop ring upon receiving ``VHOST_USER_GET_VRING_BASE``.
 
+While the back-end is suspended (via ``VHOST_USER_SUSPEND``), it must
+never process rings, and thus also delay handling kicks until the
+back-end is resumed again.
+
 Rings can be enabled or disabled by ``VHOST_USER_SET_VRING_ENABLE``.
 
 If ``VHOST_USER_F_PROTOCOL_FEATURES`` has not been negotiated, the
@@ -479,8 +483,9 @@ supplied by ``VhostUserLog``.
 ancillary data, it may be used to inform the front-end that the log has
 been modified.
 
-Once the source has finished migration, rings will be stopped by the
-source. No further update must be done before rings are restarted.
+Once the source has finished migration, the device will be suspended and
+its rings will be stopped by the source. No further update must be done
+before the device and its rings are resumed.
 
 In postcopy migration the back-end is started before all the memory has
 been received from the source host, and care must be taken to avoid
@@ -885,6 +890,7 @@ Protocol features
   #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS  15
   #define VHOST_USER_PROTOCOL_F_STATUS   16
   #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17
+  #define VHOST_USER_PROTOCOL_F_SUSPEND  18
 
 Front-end message types
 ---
@@ -1440,6 +1446,31 @@ Front-end message types
   query the back-end for its device status as defined in the Virtio
   specification.
 
+``VHOST_USER_SUSPEND``
+  :id: 41
+  :equivalent ioctl: VHOST_VDPA_SUSPEND
+  :request payload: N/A
+  :reply payload: N/A
+
+  When the ``VHOST_USER_PROTOCOL_F_SUSPEND`` protocol feature has been
+  successfully negotiated, this message is submitted by the front-end to
+  have the back-end cease all operations except for handling vhost-user
+  requests.  The back-end must stop processing all virt queues, and it
+  must not perform any background operations.  It may not resume until a
+  subsequent ``VHOST_USER_RESUME`` call.
+
+``VHOST_USER_RESUME``
+  :id: 42
+  :equivalent ioctl: VHOST_VDPA_RESUME
+  :request payload: N/A
+  :reply payload: N/A
+
+  When the ``VHOST_USER_PROTOCOL_F_SUSPEND`` protocol feature has been
+  successfully negotiated, this message is submitted by the front-end to
+  allow the back-end to resume operations after having been suspended
+  before.  The back-end must again begin processing rings that are not
+  stopped, and it may resume background operations.
+
 
 Back-end message types
 --
-- 
2.41.0

[PATCH 3/6] vhost: Do not reset suspended devices on stop

Move the `suspended` field from vhost_vdpa into the global vhost_dev
struct, so vhost_dev_stop() can check whether the back-end has been
suspended by `vhost_ops->vhost_dev_start(hdev, false)`.  If it has,
there is no need to reset it; the reset is just a fall-back to stop
device operations for back-ends that do not support suspend.

Unfortunately, for vDPA specifically, RESUME is not yet implemented, so
when the device is re-started, we still have to do the reset to have it
un-suspend.

Signed-off-by: Hanna Czenczek 
---
 include/hw/virtio/vhost-vdpa.h |  2 --
 include/hw/virtio/vhost.h  |  8 
 hw/virtio/vhost-vdpa.c | 11 +++
 hw/virtio/vhost.c  |  8 +++-
 4 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
index e64bfc7f98..72c3686b7f 100644
--- a/include/hw/virtio/vhost-vdpa.h
+++ b/include/hw/virtio/vhost-vdpa.h
@@ -42,8 +42,6 @@ typedef struct vhost_vdpa {
 bool shadow_vqs_enabled;
 /* Vdpa must send shadow addresses as IOTLB key for data queues, not GPA */
 bool shadow_data;
-/* Device suspended successfully */
-bool suspended;
 /* IOVA mapping used by the Shadow Virtqueue */
 VhostIOVATree *iova_tree;
 GPtrArray *shadow_vqs;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 6a173cb9fa..69bf59d630 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -120,6 +120,14 @@ struct vhost_dev {
 uint64_t backend_cap;
 /* @started: is the vhost device started? */
 bool started;
+/**
+ * @suspended: Whether the vhost device is currently suspended.  Set
+ * and reset by implementations (vhost-user, vhost-vdpa, ...), which
+ * are supposed to automatically suspend/resume in their
+ * vhost_dev_start handlers as required.  Must also be cleared when
+ * the device is reset.
+ */
+bool suspended;
 bool log_enabled;
 uint64_t log_size;
 Error *migration_blocker;
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index 7b7dee468e..f7fd19a203 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -858,13 +858,12 @@ static int vhost_vdpa_get_device_id(struct vhost_dev *dev,
 
 static int vhost_vdpa_reset_device(struct vhost_dev *dev)
 {
-struct vhost_vdpa *v = dev->opaque;
 int ret;
 uint8_t status = 0;
 
 ret = vhost_vdpa_call(dev, VHOST_VDPA_SET_STATUS, );
 trace_vhost_vdpa_reset_device(dev);
-v->suspended = false;
+dev->suspended = false;
 return ret;
 }
 
@@ -1278,7 +1277,7 @@ static void vhost_vdpa_suspend(struct vhost_dev *dev)
 if (unlikely(r)) {
 error_report("Cannot suspend: %s(%d)", g_strerror(errno), errno);
 } else {
-v->suspended = true;
+dev->suspended = true;
 return;
 }
 }
@@ -1313,6 +1312,10 @@ static int vhost_vdpa_dev_start(struct vhost_dev *dev, 
bool started)
 return -1;
 }
 vhost_vdpa_set_vring_ready(dev);
+if (dev->suspended) {
+/* TODO: When RESUME is available, use it instead of resetting */
+vhost_vdpa_reset_status(dev);
+}
 } else {
 vhost_vdpa_suspend(dev);
 vhost_vdpa_svqs_stop(dev);
@@ -1400,7 +1403,7 @@ static int vhost_vdpa_get_vring_base(struct vhost_dev 
*dev,
 return 0;
 }
 
-if (!v->suspended) {
+if (!dev->suspended) {
 /*
  * Cannot trust in value returned by device, let vhost recover used
  * idx from guest.
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index abf0d03c8d..2e28e58da7 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -2059,7 +2059,13 @@ void vhost_dev_stop(struct vhost_dev *hdev, VirtIODevice 
*vdev, bool vrings)
  hdev->vqs + i,
  hdev->vq_index + i);
 }
-if (hdev->vhost_ops->vhost_reset_status) {
+
+/*
+ * If we failed to successfully stop the device via SUSPEND (should have
+ * been attempted by `vhost_dev_start(hdev, false)`), reset it to stop it.
+ * Stateful devices where this would be problematic must implement SUSPEND.
+ */
+if (!hdev->suspended && hdev->vhost_ops->vhost_reset_status) {
 hdev->vhost_ops->vhost_reset_status(hdev);
 }
 
-- 
2.41.0

[PATCH v4 7/7] virtio-mem-pci: Device unplug support

Let's support device unplug by forwarding the unplug_request_check()
callback to the virtio-mem device.

Further, disallow changing the requested-size once an unplug request is
pending.

Disallowing requested-size changes handles corner cases such as
(1) pausing the VM (2) requesting device unplug and (3) adjusting the
requested size. If the VM would plug memory (due to the requested size
change) before processing the unplug request, we would be in trouble.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-mem-pci.c | 49 +++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c
index 2ef0f07630..c4597e029e 100644
--- a/hw/virtio/virtio-mem-pci.c
+++ b/hw/virtio/virtio-mem-pci.c
@@ -93,12 +93,53 @@ static void virtio_mem_pci_size_change_notify(Notifier 
*notifier, void *data)
 g_free(qom_path);
 }
 
+static void virtio_mem_pci_unplug_request_check(VirtIOMDPCI *vmd, Error **errp)
+{
+VirtIOMEMPCI *pci_mem = VIRTIO_MEM_PCI(vmd);
+VirtIOMEM *vmem = _mem->vdev;
+VirtIOMEMClass *vpc = VIRTIO_MEM_GET_CLASS(vmem);
+
+vpc->unplug_request_check(vmem, errp);
+}
+
+static void virtio_mem_pci_get_requested_size(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+VirtIOMEMPCI *pci_mem = VIRTIO_MEM_PCI(obj);
+
+object_property_get(OBJECT(_mem->vdev), name, v, errp);
+}
+
+static void virtio_mem_pci_set_requested_size(Object *obj, Visitor *v,
+  const char *name, void *opaque,
+  Error **errp)
+{
+VirtIOMEMPCI *pci_mem = VIRTIO_MEM_PCI(obj);
+DeviceState *dev = DEVICE(obj);
+
+/*
+ * If we passed virtio_mem_pci_unplug_request_check(), making sure that
+ * the requested size is 0, don't allow modifying the requested size
+ * anymore, otherwise the VM might end up hotplugging memory before
+ * handling the unplug request.
+ */
+if (dev->pending_deleted_event) {
+error_setg(errp, "'%s' cannot be changed if the device is in the"
+   " process of unplug", name);
+return;
+}
+
+object_property_set(OBJECT(_mem->vdev), name, v, errp);
+}
+
 static void virtio_mem_pci_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
 VirtioPCIClass *k = VIRTIO_PCI_CLASS(klass);
 PCIDeviceClass *pcidev_k = PCI_DEVICE_CLASS(klass);
 MemoryDeviceClass *mdc = MEMORY_DEVICE_CLASS(klass);
+VirtIOMDPCIClass *vmdc = VIRTIO_MD_PCI_CLASS(klass);
 
 k->realize = virtio_mem_pci_realize;
 set_bit(DEVICE_CATEGORY_MISC, dc->categories);
@@ -111,6 +152,8 @@ static void virtio_mem_pci_class_init(ObjectClass *klass, 
void *data)
 mdc->get_memory_region = virtio_mem_pci_get_memory_region;
 mdc->fill_device_info = virtio_mem_pci_fill_device_info;
 mdc->get_min_alignment = virtio_mem_pci_get_min_alignment;
+
+vmdc->unplug_request_check = virtio_mem_pci_unplug_request_check;
 }
 
 static void virtio_mem_pci_instance_init(Object *obj)
@@ -135,9 +178,9 @@ static void virtio_mem_pci_instance_init(Object *obj)
   OBJECT(>vdev), VIRTIO_MEM_BLOCK_SIZE_PROP);
 object_property_add_alias(obj, VIRTIO_MEM_SIZE_PROP, OBJECT(>vdev),
   VIRTIO_MEM_SIZE_PROP);
-object_property_add_alias(obj, VIRTIO_MEM_REQUESTED_SIZE_PROP,
-  OBJECT(>vdev),
-  VIRTIO_MEM_REQUESTED_SIZE_PROP);
+object_property_add(obj, VIRTIO_MEM_REQUESTED_SIZE_PROP, "size",
+virtio_mem_pci_get_requested_size,
+virtio_mem_pci_set_requested_size, NULL, NULL);
 }
 
 static const VirtioPCIDeviceTypeInfo virtio_mem_pci_info = {
-- 
2.41.0

Re: [PATCH v4 0/7] virtio-mem: Device unplug support


On 11.07.23 17:37, Michael S. Tsirkin wrote:

On Tue, Jul 11, 2023 at 05:34:38PM +0200, David Hildenbrand wrote:

One limitation of virtio-mem is that we cannot currently unplug virtio-mem
devices that have all memory unplugged from the VM.

Let's properly handle forced unplug (as can be triggered by the VM) and
add support for ordinary unplug (requests) of virtio-mem devices that are
in a compatible state (no legacy mode, no plugged memory, no plug request).

Briefly tested on both, x86_64 and aarch64.



Reviewed-by: Michael S. Tsirkin 


feel free to merge.


Will queue tomorrow (and send a PR soonish) if there is no more 
feedback, thanks!


--
Cheers,

David / dhildenb

Re: [PATCH v4 0/7] virtio-mem: Device unplug support

2023-07-11 Thread Michael S. Tsirkin

On Tue, Jul 11, 2023 at 05:34:38PM +0200, David Hildenbrand wrote:
> One limitation of virtio-mem is that we cannot currently unplug virtio-mem
> devices that have all memory unplugged from the VM.
> 
> Let's properly handle forced unplug (as can be triggered by the VM) and
> add support for ordinary unplug (requests) of virtio-mem devices that are
> in a compatible state (no legacy mode, no plugged memory, no plug request).
> 
> Briefly tested on both, x86_64 and aarch64.


Reviewed-by: Michael S. Tsirkin 


feel free to merge.

> v3 -> v4:
> - "pc: Factor out (un)plug handling of virtio-md-pci devices"
>  -> Add stubs to fix arm64 compilation under mingw
> - Added Tested-by's
> 
> v2 -> v3:
> - "virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci"
>  -> Add MAINTAINERS entry
> 
> v1 -> v2:
> - Reduce code duplication and implement it in a cleaner way using a
>   new abstract virtio-md-pci parent class
> - "virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci"
>  -> Added, use a new aprent type like virtio-input-pci
> - "pc: Factor out (un)plug handling of virtio-md-pci devices"
>  -> Added, factor it cleanly out
> - "arm/virt: Use virtio-md-pci (un)plug functions"
>  -> Added, reduce code duplciation
> - "virtio-md-pci: Handle unplug of virtio based memory devices"
>  -> More generic without any device-specifics
> - "virtio-md-pci: Support unplug requests for compatible devices"
>  -> More generic without any device-specifics
> - "virtio-mem: Prepare for device unplug support"
>  -> Use callback, separated from virtio-mem-pci device change
> - "virtio-mem-pci: Device unplug support"
>  -> Use callback, separated from virtio-mem device change
> 
> Cc: Peter Maydell 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: "Michael S. Tsirkin" 
> Cc: Marcel Apfelbaum 
> Cc: Igor Mammedov 
> Cc: qemu-...@nongnu.org
> Cc: Gavin Shan 
> Cc: Mario Casquero 
> 
> David Hildenbrand (7):
>   virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci
>   pc: Factor out (un)plug handling of virtio-md-pci devices
>   arm/virt: Use virtio-md-pci (un)plug functions
>   virtio-md-pci: Handle unplug of virtio based memory devices
>   virtio-md-pci: Support unplug requests for compatible devices
>   virtio-mem: Prepare for device unplug support
>   virtio-mem-pci: Device unplug support
> 
>  MAINTAINERS   |   7 ++
>  hw/arm/virt.c |  81 +++-
>  hw/i386/pc.c  |  90 +++---
>  hw/virtio/Kconfig |   8 +-
>  hw/virtio/meson.build |   1 +
>  hw/virtio/virtio-md-pci.c | 151 ++
>  hw/virtio/virtio-mem-pci.c|  54 +--
>  hw/virtio/virtio-mem-pci.h|   6 +-
>  hw/virtio/virtio-mem.c|  25 +
>  hw/virtio/virtio-pmem-pci.c   |   5 +-
>  hw/virtio/virtio-pmem-pci.h   |   6 +-
>  include/hw/virtio/virtio-md-pci.h |  44 +
>  include/hw/virtio/virtio-mem.h|   1 +
>  stubs/meson.build |   1 +
>  stubs/virtio-md-pci.c |  24 +
>  15 files changed, 337 insertions(+), 167 deletions(-)
>  create mode 100644 hw/virtio/virtio-md-pci.c
>  create mode 100644 include/hw/virtio/virtio-md-pci.h
>  create mode 100644 stubs/virtio-md-pci.c
> 
> -- 
> 2.41.0

[PATCH v4 0/7] virtio-mem: Device unplug support

One limitation of virtio-mem is that we cannot currently unplug virtio-mem
devices that have all memory unplugged from the VM.

Let's properly handle forced unplug (as can be triggered by the VM) and
add support for ordinary unplug (requests) of virtio-mem devices that are
in a compatible state (no legacy mode, no plugged memory, no plug request).

Briefly tested on both, x86_64 and aarch64.

v3 -> v4:
- "pc: Factor out (un)plug handling of virtio-md-pci devices"
 -> Add stubs to fix arm64 compilation under mingw
- Added Tested-by's

v2 -> v3:
- "virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci"
 -> Add MAINTAINERS entry

v1 -> v2:
- Reduce code duplication and implement it in a cleaner way using a
  new abstract virtio-md-pci parent class
- "virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci"
 -> Added, use a new aprent type like virtio-input-pci
- "pc: Factor out (un)plug handling of virtio-md-pci devices"
 -> Added, factor it cleanly out
- "arm/virt: Use virtio-md-pci (un)plug functions"
 -> Added, reduce code duplciation
- "virtio-md-pci: Handle unplug of virtio based memory devices"
 -> More generic without any device-specifics
- "virtio-md-pci: Support unplug requests for compatible devices"
 -> More generic without any device-specifics
- "virtio-mem: Prepare for device unplug support"
 -> Use callback, separated from virtio-mem-pci device change
- "virtio-mem-pci: Device unplug support"
 -> Use callback, separated from virtio-mem device change

Cc: Peter Maydell 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 
Cc: "Michael S. Tsirkin" 
Cc: Marcel Apfelbaum 
Cc: Igor Mammedov 
Cc: qemu-...@nongnu.org
Cc: Gavin Shan 
Cc: Mario Casquero 

David Hildenbrand (7):
  virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci
  pc: Factor out (un)plug handling of virtio-md-pci devices
  arm/virt: Use virtio-md-pci (un)plug functions
  virtio-md-pci: Handle unplug of virtio based memory devices
  virtio-md-pci: Support unplug requests for compatible devices
  virtio-mem: Prepare for device unplug support
  virtio-mem-pci: Device unplug support

 MAINTAINERS   |   7 ++
 hw/arm/virt.c |  81 +++-
 hw/i386/pc.c  |  90 +++---
 hw/virtio/Kconfig |   8 +-
 hw/virtio/meson.build |   1 +
 hw/virtio/virtio-md-pci.c | 151 ++
 hw/virtio/virtio-mem-pci.c|  54 +--
 hw/virtio/virtio-mem-pci.h|   6 +-
 hw/virtio/virtio-mem.c|  25 +
 hw/virtio/virtio-pmem-pci.c   |   5 +-
 hw/virtio/virtio-pmem-pci.h   |   6 +-
 include/hw/virtio/virtio-md-pci.h |  44 +
 include/hw/virtio/virtio-mem.h|   1 +
 stubs/meson.build |   1 +
 stubs/virtio-md-pci.c |  24 +
 15 files changed, 337 insertions(+), 167 deletions(-)
 create mode 100644 hw/virtio/virtio-md-pci.c
 create mode 100644 include/hw/virtio/virtio-md-pci.h
 create mode 100644 stubs/virtio-md-pci.c

-- 
2.41.0

[PATCH v4 5/7] virtio-md-pci: Support unplug requests for compatible devices

Let's support unplug requests for virtio-md-pci devices that provide
a unplug_request_check() callback.

We'll wire that up for virtio-mem-pci next.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-md-pci.c | 38 +--
 include/hw/virtio/virtio-md-pci.h |  3 +++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/virtio-md-pci.c b/hw/virtio/virtio-md-pci.c
index a22a259e2d..62bfb7920b 100644
--- a/hw/virtio/virtio-md-pci.c
+++ b/hw/virtio/virtio-md-pci.c
@@ -69,8 +69,42 @@ void virtio_md_pci_plug(VirtIOMDPCI *vmd, MachineState *ms, 
Error **errp)
 void virtio_md_pci_unplug_request(VirtIOMDPCI *vmd, MachineState *ms,
   Error **errp)
 {
-/* We don't support hot unplug of virtio based memory devices */
-error_setg(errp, "virtio based memory devices cannot be unplugged.");
+VirtIOMDPCIClass *vmdc = VIRTIO_MD_PCI_GET_CLASS(vmd);
+DeviceState *dev = DEVICE(vmd);
+HotplugHandler *bus_handler = qdev_get_bus_hotplug_handler(dev);
+HotplugHandlerClass *hdc;
+Error *local_err = NULL;
+
+if (!vmdc->unplug_request_check) {
+error_setg(errp, "this virtio based memory devices cannot be 
unplugged");
+return;
+}
+
+if (!bus_handler) {
+error_setg(errp, "hotunplug of virtio based memory devices not"
+   "supported on this bus");
+return;
+}
+
+vmdc->unplug_request_check(vmd, _err);
+if (local_err) {
+error_propagate(errp, local_err);
+return;
+}
+
+/*
+ * Forward the async request or turn it into a sync request (handling it
+ * like qdev_unplug()).
+ */
+hdc = HOTPLUG_HANDLER_GET_CLASS(bus_handler);
+if (hdc->unplug_request) {
+hotplug_handler_unplug_request(bus_handler, dev, _err);
+} else {
+virtio_md_pci_unplug(vmd, ms, _err);
+if (!local_err) {
+object_unparent(OBJECT(dev));
+}
+}
 }
 
 void virtio_md_pci_unplug(VirtIOMDPCI *vmd, MachineState *ms, Error **errp)
diff --git a/include/hw/virtio/virtio-md-pci.h 
b/include/hw/virtio/virtio-md-pci.h
index f9fa857aec..5912e16674 100644
--- a/include/hw/virtio/virtio-md-pci.h
+++ b/include/hw/virtio/virtio-md-pci.h
@@ -26,6 +26,9 @@ OBJECT_DECLARE_TYPE(VirtIOMDPCI, VirtIOMDPCIClass, 
VIRTIO_MD_PCI)
 struct VirtIOMDPCIClass {
 /* private */
 VirtioPCIClass parent;
+
+/* public */
+void (*unplug_request_check)(VirtIOMDPCI *vmd, Error **errp);
 };
 
 struct VirtIOMDPCI {
-- 
2.41.0

[PATCH v4 3/7] arm/virt: Use virtio-md-pci (un)plug functions

Let's use our new helper functions. Note that virtio-pmem-pci is not
enabled for arm and, therefore, not compiled in.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 hw/arm/virt.c | 81 ---
 1 file changed, 12 insertions(+), 69 deletions(-)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 8a4c663735..4ae1996d37 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -73,11 +73,10 @@
 #include "hw/arm/smmuv3.h"
 #include "hw/acpi/acpi.h"
 #include "target/arm/internals.h"
-#include "hw/mem/memory-device.h"
 #include "hw/mem/pc-dimm.h"
 #include "hw/mem/nvdimm.h"
 #include "hw/acpi/generic_event_device.h"
-#include "hw/virtio/virtio-mem-pci.h"
+#include "hw/virtio/virtio-md-pci.h"
 #include "hw/virtio/virtio-iommu.h"
 #include "hw/char/pl011.h"
 #include "qemu/guest-random.h"
@@ -2740,64 +2739,6 @@ static void virt_memory_plug(HotplugHandler *hotplug_dev,
  dev, _abort);
 }
 
-static void virt_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
-DeviceState *dev, Error **errp)
-{
-HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
-Error *local_err = NULL;
-
-if (!hotplug_dev2 && dev->hotplugged) {
-/*
- * Without a bus hotplug handler, we cannot control the plug/unplug
- * order. We should never reach this point when hotplugging on ARM.
- * However, it's nice to add a safety net, similar to what we have
- * on x86.
- */
-error_setg(errp, "hotplug of virtio based memory devices not supported"
-   " on this bus.");
-return;
-}
-/*
- * First, see if we can plug this memory device at all. If that
- * succeeds, branch of to the actual hotplug handler.
- */
-memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
-   _err);
-if (!local_err && hotplug_dev2) {
-hotplug_handler_pre_plug(hotplug_dev2, dev, _err);
-}
-error_propagate(errp, local_err);
-}
-
-static void virt_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
-DeviceState *dev, Error **errp)
-{
-HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
-Error *local_err = NULL;
-
-/*
- * Plug the memory device first and then branch off to the actual
- * hotplug handler. If that one fails, we can easily undo the memory
- * device bits.
- */
-memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
-if (hotplug_dev2) {
-hotplug_handler_plug(hotplug_dev2, dev, _err);
-if (local_err) {
-memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
-}
-}
-error_propagate(errp, local_err);
-}
-
-static void virt_virtio_md_pci_unplug_request(HotplugHandler *hotplug_dev,
-  DeviceState *dev, Error **errp)
-{
-/* We don't support hot unplug of virtio based memory devices */
-error_setg(errp, "virtio based memory devices cannot be unplugged.");
-}
-
-
 static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 DeviceState *dev, Error **errp)
 {
@@ -2805,8 +2746,8 @@ static void 
virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
 virt_memory_pre_plug(hotplug_dev, dev, errp);
-} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
-virt_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
 hwaddr db_start = 0, db_end = 0;
 char *resv_prop_str;
@@ -2855,12 +2796,11 @@ static void virt_machine_device_plug_cb(HotplugHandler 
*hotplug_dev,
  SYS_BUS_DEVICE(dev));
 }
 }
+
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
 virt_memory_plug(hotplug_dev, dev, errp);
-}
-
-if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
-virt_virtio_md_pci_plug(hotplug_dev, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+virtio_md_pci_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
 }
 
 if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
@@ -2915,8 +2855,9 @@ static void 
virt_machine_device_unplug_request_cb(HotplugHandler *hotplug_dev,
 {
 if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM)) {
 virt_dimm_unplug_request(hotplug_dev, dev, errp);
-} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
-virt_virtio_md_pci_unplug_request(hotplug_dev, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev),

[PATCH v4 4/7] virtio-md-pci: Handle unplug of virtio based memory devices

While we fence unplug requests from the outside, the VM can still
trigger unplug of virtio based memory devices, for example, in Linux
doing on a virtio-mem-pci device:
# echo 0 > /sys/bus/pci/slots/3/power

While doing that is not really expected to work without harming the
guest OS (e.g., removing a virtio-mem device while it still provides
memory), let's make sure that we properly handle it on the QEMU side.

We'll add support for unplugging of virtio-mem devices in some
configurations next.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-md-pci.c | 23 ++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-md-pci.c b/hw/virtio/virtio-md-pci.c
index e849c3131d..a22a259e2d 100644
--- a/hw/virtio/virtio-md-pci.c
+++ b/hw/virtio/virtio-md-pci.c
@@ -14,6 +14,7 @@
 #include "hw/virtio/virtio-md-pci.h"
 #include "hw/mem/memory-device.h"
 #include "qapi/error.h"
+#include "qemu/error-report.h"
 
 void virtio_md_pci_pre_plug(VirtIOMDPCI *vmd, MachineState *ms, Error **errp)
 {
@@ -74,7 +75,27 @@ void virtio_md_pci_unplug_request(VirtIOMDPCI *vmd, 
MachineState *ms,
 
 void virtio_md_pci_unplug(VirtIOMDPCI *vmd, MachineState *ms, Error **errp)
 {
-/* We don't support hot unplug of virtio based memory devices */
+DeviceState *dev = DEVICE(vmd);
+HotplugHandler *bus_handler = qdev_get_bus_hotplug_handler(dev);
+MemoryDeviceState *md = MEMORY_DEVICE(vmd);
+Error *local_err = NULL;
+
+/* Unplug the memory device while it is still realized. */
+memory_device_unplug(md, ms);
+
+if (bus_handler) {
+hotplug_handler_unplug(bus_handler, dev, _err);
+if (local_err) {
+/* Not expected to fail ... but still try to recover. */
+memory_device_plug(md, ms);
+error_propagate(errp, local_err);
+return;
+}
+} else {
+/* Very unexpected, but let's just try to do the right thing. */
+warn_report("Unexpected unplug of virtio based memory device");
+qdev_unrealize(dev);
+}
 }
 
 static const TypeInfo virtio_md_pci_info = {
-- 
2.41.0

[PATCH v4 6/7] virtio-mem: Prepare for device unplug support

In many cases, blindly unplugging a virtio-mem device is problematic. We
can only safely remove a device once:
* The guest is not expecting to be able to read unplugged memory
  (unplugged-inaccessible == on)
* The virtio-mem device does not have memory plugged (size == 0)
* The virtio-mem device does not have outstanding requests to the VM to
  plug memory (requested-size == 0)

So let's add a callback to the virtio-mem device class to check for that.
We'll wire-up virtio-mem-pci next.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/virtio-mem.c | 25 +
 include/hw/virtio/virtio-mem.h |  1 +
 2 files changed, 26 insertions(+)

diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index ec0ae32589..27b3aac87c 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -1483,6 +1483,30 @@ static void 
virtio_mem_rdm_unregister_listener(RamDiscardManager *rdm,
 QLIST_REMOVE(rdl, next);
 }
 
+static void virtio_mem_unplug_request_check(VirtIOMEM *vmem, Error **errp)
+{
+if (vmem->unplugged_inaccessible == ON_OFF_AUTO_OFF) {
+/*
+ * We could allow it with a usable region size of 0, but let's just
+ * not care about that legacy setting.
+ */
+error_setg(errp, "virtio-mem device cannot get unplugged while"
+   " '" VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP "' != 'on'");
+return;
+}
+
+if (vmem->size) {
+error_setg(errp, "virtio-mem device cannot get unplugged while"
+   " '" VIRTIO_MEM_SIZE_PROP "' != '0'");
+return;
+}
+if (vmem->requested_size) {
+error_setg(errp, "virtio-mem device cannot get unplugged while"
+   " '" VIRTIO_MEM_REQUESTED_SIZE_PROP "' != '0'");
+return;
+}
+}
+
 static void virtio_mem_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -1505,6 +1529,7 @@ static void virtio_mem_class_init(ObjectClass *klass, 
void *data)
 vmc->get_memory_region = virtio_mem_get_memory_region;
 vmc->add_size_change_notifier = virtio_mem_add_size_change_notifier;
 vmc->remove_size_change_notifier = virtio_mem_remove_size_change_notifier;
+vmc->unplug_request_check = virtio_mem_unplug_request_check;
 
 rdmc->get_min_granularity = virtio_mem_rdm_get_min_granularity;
 rdmc->is_populated = virtio_mem_rdm_is_populated;
diff --git a/include/hw/virtio/virtio-mem.h b/include/hw/virtio/virtio-mem.h
index f15e561785..ab0fe2b4f2 100644
--- a/include/hw/virtio/virtio-mem.h
+++ b/include/hw/virtio/virtio-mem.h
@@ -98,6 +98,7 @@ struct VirtIOMEMClass {
 MemoryRegion *(*get_memory_region)(VirtIOMEM *vmem, Error **errp);
 void (*add_size_change_notifier)(VirtIOMEM *vmem, Notifier *notifier);
 void (*remove_size_change_notifier)(VirtIOMEM *vmem, Notifier *notifier);
+void (*unplug_request_check)(VirtIOMEM *vmem, Error **errp);
 };
 
 #endif
-- 
2.41.0

[PATCH v4 1/7] virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci

Let's add a new abstract "virtio memory device" type, and use it as
parent class of virtio-mem-pci and virtio-pmem-pci.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 MAINTAINERS   |  6 ++
 hw/virtio/Kconfig |  8 +--
 hw/virtio/meson.build |  1 +
 hw/virtio/virtio-md-pci.c | 33 +
 hw/virtio/virtio-mem-pci.c|  5 +
 hw/virtio/virtio-mem-pci.h|  6 +++---
 hw/virtio/virtio-pmem-pci.c   |  5 +
 hw/virtio/virtio-pmem-pci.h   |  6 +++---
 include/hw/virtio/virtio-md-pci.h | 35 +++
 9 files changed, 89 insertions(+), 16 deletions(-)
 create mode 100644 hw/virtio/virtio-md-pci.c
 create mode 100644 include/hw/virtio/virtio-md-pci.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e158a25cfe..bb4626faf7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2229,6 +2229,12 @@ F: hw/virtio/virtio-crypto.c
 F: hw/virtio/virtio-crypto-pci.c
 F: include/hw/virtio/virtio-crypto.h
 
+virtio based memory device
+M: David Hildenbrand 
+S: Supported
+F: hw/virtio/virtio-md-pci.c
+F: include/hw/virtio/virtio-md-pci.h
+
 virtio-mem
 M: David Hildenbrand 
 S: Supported
diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index a9ee09062f..92c9cf6c96 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -35,6 +35,10 @@ config VIRTIO_CRYPTO
 default y
 depends on VIRTIO
 
+config VIRTIO_MD
+bool
+select MEM_DEVICE
+
 config VIRTIO_PMEM_SUPPORTED
 bool
 
@@ -43,7 +47,7 @@ config VIRTIO_PMEM
 default y
 depends on VIRTIO
 depends on VIRTIO_PMEM_SUPPORTED
-select MEM_DEVICE
+select VIRTIO_MD
 
 config VIRTIO_MEM_SUPPORTED
 bool
@@ -54,7 +58,7 @@ config VIRTIO_MEM
 depends on VIRTIO
 depends on LINUX
 depends on VIRTIO_MEM_SUPPORTED
-select MEM_DEVICE
+select VIRTIO_MD
 
 config VHOST_VSOCK_COMMON
 bool
diff --git a/hw/virtio/meson.build b/hw/virtio/meson.build
index c4f4fe05fa..13e7c6c272 100644
--- a/hw/virtio/meson.build
+++ b/hw/virtio/meson.build
@@ -63,6 +63,7 @@ virtio_pci_ss.add(when: 'CONFIG_VIRTIO_PMEM', if_true: 
files('virtio-pmem-pci.c'
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_IOMMU', if_true: 
files('virtio-iommu-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MEM', if_true: 
files('virtio-mem-pci.c'))
 virtio_pci_ss.add(when: 'CONFIG_VHOST_VDPA_DEV', if_true: 
files('vdpa-dev-pci.c'))
+virtio_pci_ss.add(when: 'CONFIG_VIRTIO_MD', if_true: files('virtio-md-pci.c'))
 
 specific_virtio_ss.add_all(when: 'CONFIG_VIRTIO_PCI', if_true: virtio_pci_ss)
 
diff --git a/hw/virtio/virtio-md-pci.c b/hw/virtio/virtio-md-pci.c
new file mode 100644
index 00..6b02ff908e
--- /dev/null
+++ b/hw/virtio/virtio-md-pci.c
@@ -0,0 +1,33 @@
+/*
+ * Abstract virtio based memory device
+ *
+ * Copyright (C) 2023 Red Hat, Inc.
+ *
+ * Authors:
+ *  David Hildenbrand 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/virtio/virtio-md-pci.h"
+#include "hw/mem/memory-device.h"
+
+static const TypeInfo virtio_md_pci_info = {
+.name = TYPE_VIRTIO_MD_PCI,
+.parent = TYPE_VIRTIO_PCI,
+.instance_size = sizeof(VirtIOMDPCI),
+.class_size = sizeof(VirtIOMDPCIClass),
+.abstract = true,
+.interfaces = (InterfaceInfo[]) {
+{ TYPE_MEMORY_DEVICE },
+{ }
+},
+};
+
+static void virtio_md_pci_register(void)
+{
+type_register_static(_md_pci_info);
+}
+type_init(virtio_md_pci_register)
diff --git a/hw/virtio/virtio-mem-pci.c b/hw/virtio/virtio-mem-pci.c
index b85c12668d..2ef0f07630 100644
--- a/hw/virtio/virtio-mem-pci.c
+++ b/hw/virtio/virtio-mem-pci.c
@@ -142,14 +142,11 @@ static void virtio_mem_pci_instance_init(Object *obj)
 
 static const VirtioPCIDeviceTypeInfo virtio_mem_pci_info = {
 .base_name = TYPE_VIRTIO_MEM_PCI,
+.parent = TYPE_VIRTIO_MD_PCI,
 .generic_name = "virtio-mem-pci",
 .instance_size = sizeof(VirtIOMEMPCI),
 .instance_init = virtio_mem_pci_instance_init,
 .class_init = virtio_mem_pci_class_init,
-.interfaces = (InterfaceInfo[]) {
-{ TYPE_MEMORY_DEVICE },
-{ }
-},
 };
 
 static void virtio_mem_pci_register_types(void)
diff --git a/hw/virtio/virtio-mem-pci.h b/hw/virtio/virtio-mem-pci.h
index e636e1a48d..c50b51d608 100644
--- a/hw/virtio/virtio-mem-pci.h
+++ b/hw/virtio/virtio-mem-pci.h
@@ -13,21 +13,21 @@
 #ifndef QEMU_VIRTIO_MEM_PCI_H
 #define QEMU_VIRTIO_MEM_PCI_H
 
-#include "hw/virtio/virtio-pci.h"
+#include "hw/virtio/virtio-md-pci.h"
 #include "hw/virtio/virtio-mem.h"
 #include "qom/object.h"
 
 typedef struct VirtIOMEMPCI VirtIOMEMPCI;
 
 /*
- * virtio-mem-pci: This extends VirtioPCIProxy.
+ * virtio-mem-pci: This extends VirtIOMDPCI.
  */
 #define TYPE_VIRTIO_MEM_PCI "virtio-mem-pci-base"
 DECLARE_INSTANCE_CHECKER(VirtIOMEMPCI, VIRTIO_MEM_PCI,
  TYPE_VIRTIO_MEM_PCI)

[PATCH v4 2/7] pc: Factor out (un)plug handling of virtio-md-pci devices

Let's factor out (un)plug handling, to be reused from arm/virt code.

Provide stubs for the case that CONFIG_VIRTIO_MD is not selected because
neither virtio-mem nor virtio-pmem is enabled. While this cannot
currently happen for x86, it will be possible for arm/virt.

Tested-by: Mario Casquero 
Signed-off-by: David Hildenbrand 
---
 MAINTAINERS   |  1 +
 hw/i386/pc.c  | 90 ---
 hw/virtio/virtio-md-pci.c | 63 ++
 include/hw/virtio/virtio-md-pci.h |  6 +++
 stubs/meson.build |  1 +
 stubs/virtio-md-pci.c | 24 +
 6 files changed, 106 insertions(+), 79 deletions(-)
 create mode 100644 stubs/virtio-md-pci.c

diff --git a/MAINTAINERS b/MAINTAINERS
index bb4626faf7..12e59b6b27 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2234,6 +2234,7 @@ M: David Hildenbrand 
 S: Supported
 F: hw/virtio/virtio-md-pci.c
 F: include/hw/virtio/virtio-md-pci.h
+F: stubs/virtio-md-pci.c
 
 virtio-mem
 M: David Hildenbrand 
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index f01d7de5ad..c74a4014f0 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -88,13 +88,11 @@
 #include "hw/net/ne2000-isa.h"
 #include "standard-headers/asm-x86/bootparam.h"
 #include "hw/virtio/virtio-iommu.h"
-#include "hw/virtio/virtio-pmem-pci.h"
-#include "hw/virtio/virtio-mem-pci.h"
+#include "hw/virtio/virtio-md-pci.h"
 #include "hw/i386/kvm/xen_overlay.h"
 #include "hw/i386/kvm/xen_evtchn.h"
 #include "hw/i386/kvm/xen_gnttab.h"
 #include "hw/i386/kvm/xen_xenstore.h"
-#include "hw/mem/memory-device.h"
 #include "sysemu/replay.h"
 #include "target/i386/cpu.h"
 #include "e820_memory_layout.h"
@@ -1500,68 +1498,6 @@ static void pc_memory_unplug(HotplugHandler *hotplug_dev,
 error_propagate(errp, local_err);
 }
 
-static void pc_virtio_md_pci_pre_plug(HotplugHandler *hotplug_dev,
-  DeviceState *dev, Error **errp)
-{
-HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
-Error *local_err = NULL;
-
-if (!hotplug_dev2 && dev->hotplugged) {
-/*
- * Without a bus hotplug handler, we cannot control the plug/unplug
- * order. We should never reach this point when hotplugging on x86,
- * however, better add a safety net.
- */
-error_setg(errp, "hotplug of virtio based memory devices not supported"
-   " on this bus.");
-return;
-}
-/*
- * First, see if we can plug this memory device at all. If that
- * succeeds, branch of to the actual hotplug handler.
- */
-memory_device_pre_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev), NULL,
-   _err);
-if (!local_err && hotplug_dev2) {
-hotplug_handler_pre_plug(hotplug_dev2, dev, _err);
-}
-error_propagate(errp, local_err);
-}
-
-static void pc_virtio_md_pci_plug(HotplugHandler *hotplug_dev,
-  DeviceState *dev, Error **errp)
-{
-HotplugHandler *hotplug_dev2 = qdev_get_bus_hotplug_handler(dev);
-Error *local_err = NULL;
-
-/*
- * Plug the memory device first and then branch off to the actual
- * hotplug handler. If that one fails, we can easily undo the memory
- * device bits.
- */
-memory_device_plug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
-if (hotplug_dev2) {
-hotplug_handler_plug(hotplug_dev2, dev, _err);
-if (local_err) {
-memory_device_unplug(MEMORY_DEVICE(dev), MACHINE(hotplug_dev));
-}
-}
-error_propagate(errp, local_err);
-}
-
-static void pc_virtio_md_pci_unplug_request(HotplugHandler *hotplug_dev,
-DeviceState *dev, Error **errp)
-{
-/* We don't support hot unplug of virtio based memory devices */
-error_setg(errp, "virtio based memory devices cannot be unplugged.");
-}
-
-static void pc_virtio_md_pci_unplug(HotplugHandler *hotplug_dev,
-DeviceState *dev, Error **errp)
-{
-/* We don't support hot unplug of virtio based memory devices */
-}
-
 static void pc_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp)
 {
@@ -1569,9 +1505,8 @@ static void pc_machine_device_pre_plug_cb(HotplugHandler 
*hotplug_dev,
 pc_memory_pre_plug(hotplug_dev, dev, errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_CPU)) {
 x86_cpu_pre_plug(hotplug_dev, dev, errp);
-} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_PMEM_PCI) ||
-   object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MEM_PCI)) {
-pc_virtio_md_pci_pre_plug(hotplug_dev, dev, errp);
+} else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_MD_PCI)) {
+virtio_md_pci_pre_plug(VIRTIO_MD_PCI(dev), MACHINE(hotplug_dev), errp);
 } else if (object_dynamic_cast(OBJECT(dev), TYPE_VIRTIO_IOMMU_PCI)) {
 /* Declare the

[PATCH-for-8.1] linux-user/arm: Do not allocate a commpage at all for M-profile CPUs

Since commit fbd3c4cff6 ("linux-user/arm: Mark the commpage
executable") executing bare-metal (linked with rdimon.specs)
cortex-M code fails as:

  $ qemu-arm -cpu cortex-m3 ~/hello.exe.m3
  qemu-arm: ../../accel/tcg/user-exec.c:492: page_set_flags: Assertion `last <= 
GUEST_ADDR_MAX' failed.
  Aborted (core dumped)

Commit 4f5c67f8df ("linux-user/arm: Take more care allocating
commpage") already took care of not allocating a commpage for
M-profile CPUs, however it had to be reverted as commit 6cda41daa2.

Re-introduce the M-profile fix from commit 4f5c67f8df.

Fixes: fbd3c4cff6 ("linux-user/arm: Mark the commpage executable")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1755
Reported-by: Christophe Lyon 
Suggested-by: Richard Henderson 
Signed-off-by: Philippe Mathieu-Daudé 
---
 linux-user/elfload.c | 21 +
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index d3d1352c4e..a26200d9f3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -424,10 +424,23 @@ enum {
 
 static bool init_guest_commpage(void)
 {
-abi_ptr commpage = HI_COMMPAGE & -qemu_host_page_size;
-void *want = g2h_untagged(commpage);
-void *addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE,
-  MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
+ARMCPU *cpu = ARM_CPU(thread_cpu);
+abi_ptr commpage;
+void *want;
+void *addr;
+
+/*
+ * M-profile allocates maximum of 2GB address space, so can never
+ * allocate the commpage.  Skip it.
+ */
+if (arm_feature(>env, ARM_FEATURE_M)) {
+return true;
+}
+
+commpage = HI_COMMPAGE & -qemu_host_page_size;
+want = g2h_untagged(commpage);
+addr = mmap(want, qemu_host_page_size, PROT_READ | PROT_WRITE,
+MAP_ANONYMOUS | MAP_PRIVATE | MAP_FIXED, -1, 0);
 
 if (addr == MAP_FAILED) {
 perror("Allocating guest commpage");
-- 
2.38.1

Re: [PATCH v3 0/7] virtio-mem: Device unplug support


On 11.07.23 16:19, Mario Casquero wrote:

Tested-by: Mario Casquero


Thanks a lot Mario!

--
Cheers,

David / dhildenb

Re: [PATCH v3 1/4] tests/lcitool: Generate distribution packages list in JSON format

On Tue, Jul 11, 2023 at 04:49:19PM +0200, Philippe Mathieu-Daudé wrote:
> Add the generate_pkglist() helper to generate a list of packages
> required by a distribution to build QEMU.
> 
> Since we can not add a "THIS FILE WAS AUTO-GENERATED" comment in
> JSON, create the files under tests/vm/generated/ sub-directory;
> add a README mentioning the files are generated.
> 
> Suggested-by: Erik Skultety 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/lcitool/refresh | 11 +++
>  tests/vm/generated/README |  5 +

Or 's/generated/packages/' perhaps ?

>  2 files changed, 16 insertions(+)
>  create mode 100644 tests/vm/generated/README

Either way, at your discretion

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH v2] s390x: Fix QEMU abort by selecting S390_FLIC_KVM

2023-07-11 Thread Cédric Le Goater

If QEMU is built with --without-default-devices, the s390-flic-kvm
device is missing and QEMU aborts when started with the KVM accelerator.
Make sure it's available by selecting S390_FLIC_KVM in Kconfig.

Consequently, this also fixes an abort in tests/qtest/migration-test.

Signed-off-by: Cédric Le Goater 
---
 hw/intc/Kconfig  | 1 -
 hw/s390x/Kconfig | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 21441d0a0c43..97d550b06b77 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -49,7 +49,6 @@ config S390_FLIC
 
 config S390_FLIC_KVM
 bool
-default y
 depends on S390_FLIC && KVM
 
 config OMPIC
diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 5e7d8a2bae8b..454e0ff4b613 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -7,6 +7,7 @@ config S390_CCW_VIRTIO
 imply WDT_DIAG288
 select PCI
 select S390_FLIC
+select S390_FLIC_KVM if KVM
 select SCLPCONSOLE
 select VIRTIO_CCW
 select MSI_NONBROKEN
-- 
2.41.0

Re: [PATCH] s390x: Fix QEMU abort by selecting S390_FLIC_KVM


On 11/7/23 16:34, Cédric Le Goater wrote:

If QEMU is built with --without-default-devices, the s390-flic-kvm
device is missing and QEMU aborts when started with the KVM accelerator.
Make sure it's available by selecting S390_FLIC_KVM in Kconfig.

Consequently, this also fixes an abort in tests/qtest/migration-test.

Signed-off-by: Cédric Le Goater 
---
  hw/intc/Kconfig  | 1 -
  hw/s390x/Kconfig | 1 +
  2 files changed, 1 insertion(+), 1 deletion(-)




diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 5e7d8a2bae8b..0dad184e06d2 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -7,6 +7,7 @@ config S390_CCW_VIRTIO
  imply WDT_DIAG288
  select PCI
  select S390_FLIC
+select S390_FLIC_KVM


select S390_FLIC_KVM if KVM?


  select SCLPCONSOLE
  select VIRTIO_CCW
  select MSI_NONBROKEN

[PATCH v3 1/4] tests/lcitool: Generate distribution packages list in JSON format

Add the generate_pkglist() helper to generate a list of packages
required by a distribution to build QEMU.

Since we can not add a "THIS FILE WAS AUTO-GENERATED" comment in
JSON, create the files under tests/vm/generated/ sub-directory;
add a README mentioning the files are generated.

Suggested-by: Erik Skultety 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/lcitool/refresh | 11 +++
 tests/vm/generated/README |  5 +
 2 files changed, 16 insertions(+)
 create mode 100644 tests/vm/generated/README

diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index b54566edcc..4584870ea1 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -84,6 +84,12 @@ def generate_cirrus(target, trailer=None):
 generate(filename, cmd, trailer)
 
 
+def generate_pkglist(vm, target):
+filename = Path(src_dir, "tests", "vm", "generated", vm + ".json")
+cmd = lcitool_cmd + ["variables", "--format", "json", target, "qemu"]
+generate(filename, cmd, None)
+
+
 # Netmap still needs to be manually built as it is yet to be packaged
 # into a distro. We also add cscope and gtags which are used in the CI
 # test
@@ -191,6 +197,11 @@ try:
 generate_cirrus("freebsd-13")
 generate_cirrus("macos-12")
 
+#
+# VM packages lists
+#
+generate_pkglist("freebsd", "freebsd-13")
+
 sys.exit(0)
 except Exception as ex:
 print(str(ex), file=sys.stderr)
diff --git a/tests/vm/generated/README b/tests/vm/generated/README
new file mode 100644
index 00..7ccc6ffd3d
--- /dev/null
+++ b/tests/vm/generated/README
@@ -0,0 +1,5 @@
+# FILES IN THIS FOLDER WERE AUTO-GENERATED
+#
+#  $ make lcitool-refresh
+#
+# https://gitlab.com/libvirt/libvirt-ci
-- 
2.38.1

[PATCH v3 4/4] tests/vm/freebsd: Get up-to-date package list from lcitool vars file

Get an up-to-date package list from lcitool, that way we
don't need to manually keep this array in sync.

Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/vm/freebsd | 42 ++
 1 file changed, 2 insertions(+), 40 deletions(-)

diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 11de6473f4..ac51376c82 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -31,45 +31,6 @@ class FreeBSDVM(basevm.BaseVM):
 link = 
"https://download.freebsd.org/releases/CI-IMAGES/13.2-RELEASE/amd64/Latest/FreeBSD-13.2-RELEASE-amd64-BASIC-CI.raw.xz;
 csum = "a4fb3b6c7b75dd4d58fb0d75e4caf72844bffe0ca00e66459c028b198ffb3c0e"
 size = "20G"
-pkgs = [
-# build tools
-"git",
-"pkgconf",
-"bzip2",
-"python39",
-"ninja",
-
-# gnu tools
-"bash",
-"gmake",
-"gsed",
-"gettext",
-
-# libs: crypto
-"gnutls",
-
-# libs: images
-"jpeg-turbo",
-"png",
-
-# libs: ui
-"sdl2",
-"gtk3",
-"libxkbcommon",
-
-# libs: opengl
-"libepoxy",
-"mesa-libs",
-
-# libs: migration
-"zstd",
-
-# libs: networking
-"libslirp",
-
-# libs: sndio
-"sndio",
-]
 
 BUILD_SCRIPT = """
 set -e;
@@ -151,8 +112,9 @@ class FreeBSDVM(basevm.BaseVM):
 self.console_wait(prompt)
 self.console_send("echo 'chmod 666 /dev/vtbd1' >> /etc/rc.local\n")
 
+pkgs = self.get_qemu_packages_from_lcitool_json()
 self.print_step("Installing packages")
-self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
+self.ssh_root_check("pkg install -y %s\n" % " ".join(pkgs))
 
 # shutdown
 self.ssh_root(self.poweroff)
-- 
2.38.1

[PATCH v3 0/4] tests/vm/freebsd: Get up-to-date package list from lcitool

Inspired by this patch from Thomas:
https://lore.kernel.org/qemu-devel/20230531090415.40421-1-th...@redhat.com/

Instead of updating the package list manually, use lcitool vars file.

Since v2:
- Commit generated json (Daniel)

Since v1:
- Addressed Erik & Daniel comments (generate in JSON)

Philippe Mathieu-Daudé (4):
  tests/lcitool: Generate distribution packages list in JSON format
  tests/lcitool: Refresh generated files
  tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper
  tests/vm/freebsd: Get up-to-date package list from lcitool vars file

 tests/docker/dockerfiles/debian-amd64.docker |  2 -
 tests/docker/dockerfiles/ubuntu2004.docker   |  2 -
 tests/docker/dockerfiles/ubuntu2204.docker   |  2 -
 tests/lcitool/refresh| 11 +++
 tests/vm/basevm.py   | 11 +++
 tests/vm/freebsd | 42 +--
 tests/vm/generated/README|  5 ++
 tests/vm/generated/freebsd.json  | 77 
 8 files changed, 106 insertions(+), 46 deletions(-)
 create mode 100644 tests/vm/generated/README
 create mode 100644 tests/vm/generated/freebsd.json

-- 
2.38.1

[PATCH v3 2/4] tests/lcitool: Refresh generated files

Refresh the generated files by running:

  $ make lcitool-refresh

Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/docker/dockerfiles/debian-amd64.docker |  2 -
 tests/docker/dockerfiles/ubuntu2004.docker   |  2 -
 tests/docker/dockerfiles/ubuntu2204.docker   |  2 -
 tests/vm/generated/freebsd.json  | 77 
 4 files changed, 77 insertions(+), 6 deletions(-)
 create mode 100644 tests/vm/generated/freebsd.json

diff --git a/tests/docker/dockerfiles/debian-amd64.docker 
b/tests/docker/dockerfiles/debian-amd64.docker
index e39871c7bb..8f7521fdc2 100644
--- a/tests/docker/dockerfiles/debian-amd64.docker
+++ b/tests/docker/dockerfiles/debian-amd64.docker
@@ -70,7 +70,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libpam0g-dev \
   libpcre2-dev \
   libpixman-1-dev \
-  libpmem-dev \
   libpng-dev \
   libpulse-dev \
   librbd-dev \
@@ -96,7 +95,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libvdeplug-dev \
   libvirglrenderer-dev \
   libvte-2.91-dev \
-  libxen-dev \
   libzstd-dev \
   llvm \
   locales \
diff --git a/tests/docker/dockerfiles/ubuntu2004.docker 
b/tests/docker/dockerfiles/ubuntu2004.docker
index 8f864d19e6..7f60143cbb 100644
--- a/tests/docker/dockerfiles/ubuntu2004.docker
+++ b/tests/docker/dockerfiles/ubuntu2004.docker
@@ -69,7 +69,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libpam0g-dev \
   libpcre2-dev \
   libpixman-1-dev \
-  libpmem-dev \
   libpng-dev \
   libpulse-dev \
   librbd-dev \
@@ -94,7 +93,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libvdeplug-dev \
   libvirglrenderer-dev \
   libvte-2.91-dev \
-  libxen-dev \
   libzstd-dev \
   llvm \
   locales \
diff --git a/tests/docker/dockerfiles/ubuntu2204.docker 
b/tests/docker/dockerfiles/ubuntu2204.docker
index 1d442cdfe6..5162927016 100644
--- a/tests/docker/dockerfiles/ubuntu2204.docker
+++ b/tests/docker/dockerfiles/ubuntu2204.docker
@@ -70,7 +70,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libpam0g-dev \
   libpcre2-dev \
   libpixman-1-dev \
-  libpmem-dev \
   libpng-dev \
   libpulse-dev \
   librbd-dev \
@@ -96,7 +95,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libvdeplug-dev \
   libvirglrenderer-dev \
   libvte-2.91-dev \
-  libxen-dev \
   libzstd-dev \
   llvm \
   locales \
diff --git a/tests/vm/generated/freebsd.json b/tests/vm/generated/freebsd.json
new file mode 100644
index 00..7c435cf23e
--- /dev/null
+++ b/tests/vm/generated/freebsd.json
@@ -0,0 +1,77 @@
+{
+  "ccache": "/usr/local/bin/ccache",
+  "cpan_pkgs": [],
+  "cross_pkgs": [],
+  "make": "/usr/local/bin/gmake",
+  "ninja": "/usr/local/bin/ninja",
+  "packaging_command": "pkg",
+  "pip3": "/usr/local/bin/pip-3.8",
+  "pkgs": [
+"alsa-lib",
+"bash",
+"bison",
+"bzip2",
+"ca_root_nss",
+"capstone4",
+"ccache",
+"cmocka",
+"ctags",
+"curl",
+"cyrus-sasl",
+"dbus",
+"diffutils",
+"dtc",
+"flex",
+"fusefs-libs3",
+"gettext",
+"git",
+"glib",
+"gmake",
+"gnutls",
+"gsed",
+"gtk3",
+"json-c",
+"libepoxy",
+"libffi",
+"libgcrypt",
+"libjpeg-turbo",
+"libnfs",
+"libslirp",
+"libspice-server",
+"libssh",
+"libtasn1",
+"llvm",
+"lzo2",
+"meson",
+"mtools",
+"ncurses",
+"nettle",
+"ninja",
+"opencv",
+"pixman",
+"pkgconf",
+"png",
+"py39-numpy",
+"py39-pillow",
+"py39-pip",
+"py39-sphinx",
+"py39-sphinx_rtd_theme",
+"py39-yaml",
+"python3",
+"rpm2cpio",
+"sdl2",
+"sdl2_image",
+"snappy",
+"sndio",
+"socat",
+"spice-protocol",
+"tesseract",
+"usbredir",
+"virglrenderer",
+"vte3",
+"xorriso",
+"zstd"
+  ],
+  "pypi_pkgs": [],
+  "python": "/usr/local/bin/python3"
+}
-- 
2.38.1

[PATCH v3 3/4] tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper

Add the get_qemu_packages_from_lcitool_json() helper which return
such package list from a lcitool env var file in JSON format.

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/vm/basevm.py | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 23229e23d1..a97e23b0ce 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -27,6 +27,7 @@
 import multiprocessing
 import traceback
 import shlex
+import json
 
 from qemu.machine import QEMUMachine
 from qemu.utils import get_info_usernet_hostfwd_port, kvm_available
@@ -501,6 +502,16 @@ def gen_cloud_init_iso(self):
   stderr=self._stdout)
 return os.path.join(cidir, "cloud-init.iso")
 
+def get_qemu_packages_from_lcitool_json(self, json_path=None):
+"""Parse a lcitool variables json file and return the PKGS list."""
+if json_path is None:
+json_path = os.path.join(
+os.path.dirname(__file__), "generated", self.name + ".json"
+)
+with open(json_path, "r") as fh:
+return json.load(fh)["pkgs"]
+
+
 def get_qemu_path(arch, build_path=None):
 """Fetch the path to the qemu binary."""
 # If QEMU environment variable set, it takes precedence
-- 
2.38.1

Re: [PATCH v2 1/3] tests/lcitool: Generate distribution packages list in JSON format


On 11/7/23 16:01, Philippe Mathieu-Daudé wrote:

Add the generate_pkglist() helper to generate a list of packages
required by a distribution to build QEMU.

Generate the FreeBSD JSON file (based on FreeBSD 13).

Suggested-by: Erik Skultety 
Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/lcitool/refresh | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/tests/lcitool/refresh b/tests/lcitool/refresh
index b54566edcc..a5df096074 100755
--- a/tests/lcitool/refresh
+++ b/tests/lcitool/refresh
@@ -84,6 +84,12 @@ def generate_cirrus(target, trailer=None):
  generate(filename, cmd, trailer)
  
  
+def generate_pkglist(vm, target):

+filename = Path(src_dir, "tests", "vm", vm + ".json")
+cmd = lcitool_cmd + ["variables", "--format", "json", target, "qemu"]
+generate(filename, cmd, None)


Note since json files don't have comments, we can't have the
"THIS IS GENERATED BY A SCRIPT" in header.

[PATCH] s390x: Fix QEMU abort by selecting S390_FLIC_KVM

2023-07-11 Thread Cédric Le Goater

If QEMU is built with --without-default-devices, the s390-flic-kvm
device is missing and QEMU aborts when started with the KVM accelerator.
Make sure it's available by selecting S390_FLIC_KVM in Kconfig.

Consequently, this also fixes an abort in tests/qtest/migration-test.

Signed-off-by: Cédric Le Goater 
---
 hw/intc/Kconfig  | 1 -
 hw/s390x/Kconfig | 1 +
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig
index 21441d0a0c43..97d550b06b77 100644
--- a/hw/intc/Kconfig
+++ b/hw/intc/Kconfig
@@ -49,7 +49,6 @@ config S390_FLIC
 
 config S390_FLIC_KVM
 bool
-default y
 depends on S390_FLIC && KVM
 
 config OMPIC
diff --git a/hw/s390x/Kconfig b/hw/s390x/Kconfig
index 5e7d8a2bae8b..0dad184e06d2 100644
--- a/hw/s390x/Kconfig
+++ b/hw/s390x/Kconfig
@@ -7,6 +7,7 @@ config S390_CCW_VIRTIO
 imply WDT_DIAG288
 select PCI
 select S390_FLIC
+select S390_FLIC_KVM
 select SCLPCONSOLE
 select VIRTIO_CCW
 select MSI_NONBROKEN
-- 
2.41.0

Re: [PATCH v3 0/7] virtio-mem: Device unplug support

2023-07-11 Thread Mario Casquero

This series has been successfully tested by QE. Start a VM, plug a
virtio-mem device, resize the device (adding memory) and verify that
the virtio-mem device cannot be unplugged. Finally, resize the device
(removing all the memory) and check that it can be unplugged
seamlessly.

Tested-by: Mario Casquero 

On Mon, Jul 10, 2023 at 12:07 PM David Hildenbrand  wrote:
>
> Any further comments? IMHO this is pretty straight forward. I'll wait
> a bit longer for more feedback.
>
>
> One limitation of virtio-mem is that we cannot currently unplug virtio-mem
> devices that have all memory unplugged from the VM.
>
> Let's properly handle forced unplug (as can be triggered by the VM) and
> add support for ordinary unplug (requests) of virtio-mem devices that are
> in a compatible state (no legacy mode, no plugged memory, no plug request).
>
> Briefly tested on both, x86_64 and aarch64.
>
> v2 -> v3:
> - "virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci"
>  -> Add MAINTAINERS entry
>
> v1 -> v2:
> - Reduce code duplication and implement it in a cleaner way using a
>   new abstract virtio-md-pci parent class
> - "virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci"
>  -> Added, use a new aprent type like virtio-input-pci
> - "pc: Factor out (un)plug handling of virtio-md-pci devices"
>  -> Added, factor it cleanly out
> - "arm/virt: Use virtio-md-pci (un)plug functions"
>  -> Added, reduce code duplciation
> - "virtio-md-pci: Handle unplug of virtio based memory devices"
>  -> More generic without any device-specifics
> - "virtio-md-pci: Support unplug requests for compatible devices"
>  -> More generic without any device-specifics
> - "virtio-mem: Prepare for device unplug support"
>  -> Use callback, separated from virtio-mem-pci device change
> - "virtio-mem-pci: Device unplug support"
>  -> Use callback, separated from virtio-mem device change
>
> Cc: Peter Maydell 
> Cc: Paolo Bonzini 
> Cc: Richard Henderson 
> Cc: Eduardo Habkost 
> Cc: "Michael S. Tsirkin" 
> Cc: Marcel Apfelbaum 
> Cc: Igor Mammedov 
> Cc: qemu-...@nongnu.org
> Cc: Gavin Shan 
> Cc: Mario Casquero 
>
> David Hildenbrand (7):
>   virtio-md-pci: New parent type for virtio-mem-pci and virtio-pmem-pci
>   pc: Factor out (un)plug handling of virtio-md-pci devices
>   arm/virt: Use virtio-md-pci (un)plug functions
>   virtio-md-pci: Handle unplug of virtio based memory devices
>   virtio-md-pci: Support unplug requests for compatible devices
>   virtio-mem: Prepare for device unplug support
>   virtio-mem-pci: Device unplug support
>
>  MAINTAINERS   |   6 ++
>  hw/arm/virt.c |  81 +++-
>  hw/i386/pc.c  |  90 +++---
>  hw/virtio/Kconfig |   8 +-
>  hw/virtio/meson.build |   1 +
>  hw/virtio/virtio-md-pci.c | 151 ++
>  hw/virtio/virtio-mem-pci.c|  54 +--
>  hw/virtio/virtio-mem-pci.h|   6 +-
>  hw/virtio/virtio-mem.c|  25 +
>  hw/virtio/virtio-pmem-pci.c   |   5 +-
>  hw/virtio/virtio-pmem-pci.h   |   6 +-
>  include/hw/virtio/virtio-md-pci.h |  44 +
>  include/hw/virtio/virtio-mem.h|   1 +
>  13 files changed, 311 insertions(+), 167 deletions(-)
>  create mode 100644 hw/virtio/virtio-md-pci.c
>  create mode 100644 include/hw/virtio/virtio-md-pci.h
>
> --
> 2.41.0
>

Re: [PATCH v2 2/3] tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper


On 11/7/23 16:08, Daniel P. Berrangé wrote:

On Tue, Jul 11, 2023 at 04:01:42PM +0200, Philippe Mathieu-Daudé wrote:

Add the get_qemu_packages_from_lcitool_json() helper which return
such package list from a lcitool env var file in JSON format.

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/vm/Makefile.include | 4 
  tests/vm/basevm.py| 9 +
  2 files changed, 13 insertions(+)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index c2a8ca1c17..b021b344b5 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -106,6 +106,10 @@ $(IMAGES_DIR)/%.img:   $(SRC_PATH)/tests/vm/% \
--build-image $@, \
"  VM-IMAGE $*")
  
+$(SRC_PATH)/tests/vm/%.json:

+   $(call quiet-command, \
+   make lcitool-refresh)


'lcitool-refresh' is something we only run explicitly when we
change something about the lcitool, and also isn't provided
to end users running from tarballs IIRC. So we shouldn't need
to wire it up to build targets, just commit its output JSON
file.


Right, simpler. Thanks!

Re: [PATCH v2 2/3] tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper

On Tue, Jul 11, 2023 at 04:01:42PM +0200, Philippe Mathieu-Daudé wrote:
> Add the get_qemu_packages_from_lcitool_json() helper which return
> such package list from a lcitool env var file in JSON format.
> 
> Suggested-by: Daniel P. Berrangé 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/vm/Makefile.include | 4 
>  tests/vm/basevm.py| 9 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index c2a8ca1c17..b021b344b5 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -106,6 +106,10 @@ $(IMAGES_DIR)/%.img: $(SRC_PATH)/tests/vm/% \
>   --build-image $@, \
>   "  VM-IMAGE $*")
>  
> +$(SRC_PATH)/tests/vm/%.json:
> + $(call quiet-command, \
> + make lcitool-refresh)

'lcitool-refresh' is something we only run explicitly when we
change something about the lcitool, and also isn't provided
to end users running from tarballs IIRC. So we shouldn't need
to wire it up to build targets, just commit its output JSON
file.


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 3/3] tests/vm/freebsd: Get up-to-date package list from lcitool vars file

On Tue, Jul 11, 2023 at 04:01:43PM +0200, Philippe Mathieu-Daudé wrote:
> Get an up-to-date package list from lcitool, that way we
> don't need to manually keep this array in sync.
> 
> Inspired-by: Thomas Huth 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/vm/Makefile.include |  2 ++
>  tests/vm/freebsd  | 42 ++-
>  2 files changed, 4 insertions(+), 40 deletions(-)
> 
> diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
> index b021b344b5..84b8ad5222 100644
> --- a/tests/vm/Makefile.include
> +++ b/tests/vm/Makefile.include
> @@ -110,6 +110,8 @@ $(SRC_PATH)/tests/vm/%.json:
>   $(call quiet-command, \
>   make lcitool-refresh)
>  
> +vm-build-freebsd: $(SRC_PATH)/tests/vm/freebsd.json

I don't think we need this, as we just commit the result of
lcitool as needed.

> +
>  # Build in VM $(IMAGE)
>  vm-build-%: $(IMAGES_DIR)/%.img $(VM_VENV)
>   $(call quiet-command, \
> diff --git a/tests/vm/freebsd b/tests/vm/freebsd
> index 11de6473f4..ac51376c82 100755
> --- a/tests/vm/freebsd
> +++ b/tests/vm/freebsd
> @@ -31,45 +31,6 @@ class FreeBSDVM(basevm.BaseVM):
>  link = 
> "https://download.freebsd.org/releases/CI-IMAGES/13.2-RELEASE/amd64/Latest/FreeBSD-13.2-RELEASE-amd64-BASIC-CI.raw.xz;
>  csum = "a4fb3b6c7b75dd4d58fb0d75e4caf72844bffe0ca00e66459c028b198ffb3c0e"
>  size = "20G"
> -pkgs = [
> -# build tools
> -"git",
> -"pkgconf",
> -"bzip2",
> -"python39",
> -"ninja",
> -
> -# gnu tools
> -"bash",
> -"gmake",
> -"gsed",
> -"gettext",
> -
> -# libs: crypto
> -"gnutls",
> -
> -# libs: images
> -"jpeg-turbo",
> -"png",
> -
> -# libs: ui
> -"sdl2",
> -"gtk3",
> -"libxkbcommon",
> -
> -# libs: opengl
> -"libepoxy",
> -"mesa-libs",
> -
> -# libs: migration
> -"zstd",
> -
> -# libs: networking
> -"libslirp",
> -
> -# libs: sndio
> -"sndio",
> -]
>  
>  BUILD_SCRIPT = """
>  set -e;
> @@ -151,8 +112,9 @@ class FreeBSDVM(basevm.BaseVM):
>  self.console_wait(prompt)
>  self.console_send("echo 'chmod 666 /dev/vtbd1' >> /etc/rc.local\n")
>  
> +pkgs = self.get_qemu_packages_from_lcitool_json()
>  self.print_step("Installing packages")
> -self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
> +self.ssh_root_check("pkg install -y %s\n" % " ".join(pkgs))
>  
>  # shutdown
>  self.ssh_root(self.poweroff)
> -- 
> 2.38.1
> 

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

Re: [PATCH v2 1/3] tests/lcitool: Generate distribution packages list in JSON format

On Tue, Jul 11, 2023 at 04:01:41PM +0200, Philippe Mathieu-Daudé wrote:
> Add the generate_pkglist() helper to generate a list of packages
> required by a distribution to build QEMU.
> 
> Generate the FreeBSD JSON file (based on FreeBSD 13).
> 
> Suggested-by: Erik Skultety 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  tests/lcitool/refresh | 11 +++
>  1 file changed, 11 insertions(+)

Reviewed-by: Daniel P. Berrangé 


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|

[PATCH v2 2/3] tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper

Add the get_qemu_packages_from_lcitool_json() helper which return
such package list from a lcitool env var file in JSON format.

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/vm/Makefile.include | 4 
 tests/vm/basevm.py| 9 +
 2 files changed, 13 insertions(+)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index c2a8ca1c17..b021b344b5 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -106,6 +106,10 @@ $(IMAGES_DIR)/%.img:   $(SRC_PATH)/tests/vm/% \
--build-image $@, \
"  VM-IMAGE $*")
 
+$(SRC_PATH)/tests/vm/%.json:
+   $(call quiet-command, \
+   make lcitool-refresh)
+
 # Build in VM $(IMAGE)
 vm-build-%: $(IMAGES_DIR)/%.img $(VM_VENV)
$(call quiet-command, \
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 23229e23d1..cc4cc0a973 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -27,6 +27,7 @@
 import multiprocessing
 import traceback
 import shlex
+import json
 
 from qemu.machine import QEMUMachine
 from qemu.utils import get_info_usernet_hostfwd_port, kvm_available
@@ -501,6 +502,14 @@ def gen_cloud_init_iso(self):
   stderr=self._stdout)
 return os.path.join(cidir, "cloud-init.iso")
 
+def get_qemu_packages_from_lcitool_json(self, json_path=None):
+"""Parse a lcitool variables json file and return the PKGS list."""
+if json_path is None:
+json_path = os.path.join(os.path.dirname(__file__), self.name + 
".json")
+with open(json_path, "r") as fh:
+return json.load(fh)["pkgs"]
+
+
 def get_qemu_path(arch, build_path=None):
 """Fetch the path to the qemu binary."""
 # If QEMU environment variable set, it takes precedence
-- 
2.38.1

[PATCH v2 3/3] tests/vm/freebsd: Get up-to-date package list from lcitool vars file

Get an up-to-date package list from lcitool, that way we
don't need to manually keep this array in sync.

Inspired-by: Thomas Huth 
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/vm/Makefile.include |  2 ++
 tests/vm/freebsd  | 42 ++-
 2 files changed, 4 insertions(+), 40 deletions(-)

diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include
index b021b344b5..84b8ad5222 100644
--- a/tests/vm/Makefile.include
+++ b/tests/vm/Makefile.include
@@ -110,6 +110,8 @@ $(SRC_PATH)/tests/vm/%.json:
$(call quiet-command, \
make lcitool-refresh)
 
+vm-build-freebsd: $(SRC_PATH)/tests/vm/freebsd.json
+
 # Build in VM $(IMAGE)
 vm-build-%: $(IMAGES_DIR)/%.img $(VM_VENV)
$(call quiet-command, \
diff --git a/tests/vm/freebsd b/tests/vm/freebsd
index 11de6473f4..ac51376c82 100755
--- a/tests/vm/freebsd
+++ b/tests/vm/freebsd
@@ -31,45 +31,6 @@ class FreeBSDVM(basevm.BaseVM):
 link = 
"https://download.freebsd.org/releases/CI-IMAGES/13.2-RELEASE/amd64/Latest/FreeBSD-13.2-RELEASE-amd64-BASIC-CI.raw.xz;
 csum = "a4fb3b6c7b75dd4d58fb0d75e4caf72844bffe0ca00e66459c028b198ffb3c0e"
 size = "20G"
-pkgs = [
-# build tools
-"git",
-"pkgconf",
-"bzip2",
-"python39",
-"ninja",
-
-# gnu tools
-"bash",
-"gmake",
-"gsed",
-"gettext",
-
-# libs: crypto
-"gnutls",
-
-# libs: images
-"jpeg-turbo",
-"png",
-
-# libs: ui
-"sdl2",
-"gtk3",
-"libxkbcommon",
-
-# libs: opengl
-"libepoxy",
-"mesa-libs",
-
-# libs: migration
-"zstd",
-
-# libs: networking
-"libslirp",
-
-# libs: sndio
-"sndio",
-]
 
 BUILD_SCRIPT = """
 set -e;
@@ -151,8 +112,9 @@ class FreeBSDVM(basevm.BaseVM):
 self.console_wait(prompt)
 self.console_send("echo 'chmod 666 /dev/vtbd1' >> /etc/rc.local\n")
 
+pkgs = self.get_qemu_packages_from_lcitool_json()
 self.print_step("Installing packages")
-self.ssh_root_check("pkg install -y %s\n" % " ".join(self.pkgs))
+self.ssh_root_check("pkg install -y %s\n" % " ".join(pkgs))
 
 # shutdown
 self.ssh_root(self.poweroff)
-- 
2.38.1

[PATCH v2 0/3] tests/vm/freebsd: Get up-to-date package list from lcitool

Inspired by this patch from Thomas:
https://lore.kernel.org/qemu-devel/20230531090415.40421-1-th...@redhat.com/

Instead of updating the package list manually, use lcitool vars file.

Since v1:
- Addressed Erik & Daniel comments (generate in JSON)

Philippe Mathieu-Daudé (3):
  tests/lcitool: Generate distribution packages list in JSON format
  tests/vm: Introduce get_qemu_packages_from_lcitool_json() helper
  tests/vm/freebsd: Get up-to-date package list from lcitool vars file

 tests/lcitool/refresh | 11 ++
 tests/vm/Makefile.include |  6 ++
 tests/vm/basevm.py|  9 +
 tests/vm/freebsd  | 42 ++-
 4 files changed, 28 insertions(+), 40 deletions(-)

-- 
2.38.1

[PATCH v2 1/3] tests/lcitool: Generate distribution packages list in JSON format