Re: [PATCH] migration: Check current_migration in migration_is_running()

2024-11-06 Thread Pierrick Bouvier

On 11/6/24 01:43, Thomas Huth wrote:

On 05/11/2024 19.27, Peter Xu wrote:

Report shows that commit 34a8892dec broke iotest 055:

https://lore.kernel.org/r/b8806360-a2b6-4608-83a3-db67e264c...@linaro.org


FWIW, this patch also fixes a lot of other broken iotests (vmdk and vpc)
that occur when running "make check SPEED=thorough".

Tested-by: Thomas Huth 





Good news!

I'm a bit confused by your message. I thought SPEED=slow was the most 
complete test setup, but is it SPEED=thorough instead?




Re: [PATCH v7 0/3] vhost-user-blk: live resize additional APIs

2024-11-06 Thread Michael S. Tsirkin
On Wed, Nov 06, 2024 at 02:18:34PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> v7: update QAPI version 9.1 -> 9.2



Not like this. ypur patches are merged, pls post a fix patch on top.
Thanks!

> Vladimir Sementsov-Ogievskiy (3):
>   qdev-monitor: add option to report GenericError from find_device_state
>   vhost-user-blk: split vhost_user_blk_sync_config()
>   qapi: introduce device-sync-config
> 
>  hw/block/vhost-user-blk.c | 27 ++--
>  hw/virtio/virtio-pci.c|  9 +++
>  include/hw/qdev-core.h|  6 +
>  qapi/qdev.json| 24 ++
>  system/qdev-monitor.c | 53 ---
>  5 files changed, 108 insertions(+), 11 deletions(-)
> 
> -- 
> 2.34.1




Re: [PATCH] block: fix possible int overflow

2024-11-06 Thread Kevin Wolf
[ Cc: qemu-block ]

Am 06.11.2024 um 09:04 hat Dmitry Frolov geschrieben:
> The sum "cluster_index + count" may overflow uint32_t.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Signed-off-by: Dmitry Frolov 

Thanks, applied to the block branch.

While trying to check if this can be triggered in practice, I found this
line in parallels_fill_used_bitmap():

s->used_bmap_size = DIV_ROUND_UP(payload_bytes, s->cluster_size);

s->used_bmap_size is unsigned long, payload_bytes is the int64_t result
of bdrv_getlength() for the image file, which could certainly be made
more than 4 GB * cluster_size. I think we need an overflow check there,
too.

When allocate_clusters() calculates new_usedsize, it doesn't seem to
consider the overflow case either.

Denis, can you take a look?

Kevin




Re: [PATCH 0/4] python: update linting for new mypy/pylint releases

2024-11-06 Thread Kevin Wolf
Am 01.11.2024 um 18:36 hat John Snow geschrieben:
> Various python tests in the "check-python-tox" test case on GitLab have
> begun failing due to newer package versions. This patch set corrects
> those issues and also improves the reliability of local developer tests
> which may be using these tooling versions outside of GitLab pinned
> version tests.
> 
> There are remaining issues with the "check-dev" test I have yet to
> rectify, but appear unrelated to linter versions specifically and will
> be handled separately.
> 
> As a result of this patch, the optionally-run and may-fail
> "check-python-tox" test case on GitLab will become green again, and
> local invocations of "make check-tox" in the python subdirectory will
> also pass again. "check-python-minreqs" on GitLab and "make
> check-minreqs" in the local developer environment were/are
> unaffected. local iotest invocations for test case #297 ought to now
> begin passing on developer workstations with bleeding-edge python
> packages.
> 
> John Snow (4):
>   iotests: reflow ReproducibleTestRunner arguments
>   iotests: correct resultclass type in ReproducibleTestRunner
>   python: disable too-many-positional-arguments warning
>   python: silence pylint raising-non-exception error

Thanks, applied to the block branch.

(Yes, of course I had to wait until I ran into the problem patch 2 fixes
myself, and after figuring out the fix from the incomprehensible error
message, I found that this series already contains it.)

Kevin




Re: [PATCH v2] hw/nvme: fix handling of over-committed queues

2024-11-06 Thread Klaus Jensen
On Nov  4 17:32, Waldek Kozaczuk wrote:
> I have run my tests on the OSv side with small queue sizes like 3,4,5 and I
> could NOT replicate the issue. So it looks like the V2 version of this
> patch fixes the problem.
> 

Thanks for testing Waldek!

May I add a Tested-by tag to the commit for you?


signature.asc
Description: PGP signature


Re: [PATCH] migration: Check current_migration in migration_is_running()

2024-11-06 Thread Thomas Huth

On 05/11/2024 19.27, Peter Xu wrote:

Report shows that commit 34a8892dec broke iotest 055:

https://lore.kernel.org/r/b8806360-a2b6-4608-83a3-db67e264c...@linaro.org


FWIW, this patch also fixes a lot of other broken iotests (vmdk and vpc) 
that occur when running "make check SPEED=thorough".


Tested-by: Thomas Huth 






Re: [PATCH v7 15/15] hw/vmapple/vmapple: Add vmapple machine type

2024-11-06 Thread Akihiko Odaki

On 2024/11/06 18:10, Phil Dennis-Jordan wrote:



On Wed, 6 Nov 2024 at 08:42, Akihiko Odaki > wrote:


On 2024/11/06 0:30, Phil Dennis-Jordan wrote:
 > From: Alexander Graf mailto:g...@amazon.com>>
 >
 > Apple defines a new "vmapple" machine type as part of its proprietary
 > macOS Virtualization.Framework vmm. This machine type is similar
to the
 > virt one, but with subtle differences in base devices, a few special
 > vmapple device additions and a vastly different boot chain.
 >
 > This patch reimplements this machine type in QEMU. To use it, you
 > have to have a readily installed version of macOS for VMApple,
 > run on macOS with -accel hvf, pass the Virtualization.Framework
 > boot rom (AVPBooter) in via -bios, pass the aux and root volume
as pflash
 > and pass aux and root volume as virtio drives. In addition, you also
 > need to find the machine UUID and pass that as -M vmapple,uuid=
parameter:
 >
 > $ qemu-system-aarch64 -accel hvf -M vmapple,uuid=0x1234 -m 4G \
 >      -bios /System/Library/Frameworks/Virtualization.framework/
Versions/A/Resources/AVPBooter.vmapple2.bin
 >      -drive file=aux,if=pflash,format=raw \
 >      -drive file=root,if=pflash,format=raw \
 >      -drive file=aux,if=none,id=aux,format=raw \
 >      -device vmapple-virtio-aux,drive=aux \
 >      -drive file=root,if=none,id=root,format=raw \
 >      -device vmapple-virtio-root,drive=root
 >
 > With all these in place, you should be able to see macOS booting
 > successfully.
 >
 > Known issues:
 >   - Keyboard and mouse/tablet input is laggy. The reason for this is
 >     either that macOS's XHCI driver is broken when the device/
platform
 >     does not support MSI/MSI-X, or there's some unfortunate interplay
 >     with Qemu's XHCI implementation in this scenario.
 >   - Currently only macOS 12 guests are supported. The boot
process for
 >     13+ will need further investigation and adjustment.
 >
 > Signed-off-by: Alexander Graf mailto:g...@amazon.com>>
 > Co-authored-by: Phil Dennis-Jordan mailto:p...@philjordan.eu>>
 > Signed-off-by: Phil Dennis-Jordan mailto:p...@philjordan.eu>>
 > ---
 > v3:
 >   * Rebased on latest upstream, updated affinity and NIC creation
 > API usage
 >   * Included Apple-variant virtio-blk in build dependency
 >   * Updated API usage for setting 'redist-region-count' array-
typed property on GIC.
 >   * Switched from virtio HID devices (for which macOS 12 does not
contain
 >     drivers) to an XHCI USB controller and USB HID devices.
 >
 > v4:
 >   * Fixups for v4 changes to the other patches in the set.
 >   * Corrected the assert macro to use
 >   * Removed superfluous endian conversions corresponding to cfg's.
 >   * Init error handling improvement.
 >   * No need to select CPU type on TCG, as only HVF is supported.
 >   * Machine type version bumped to 9.2
 >   * #include order improved
 >
 > v5:
 >   * Fixed memory reservation for ecam alias region.
 >   * Better error handling setting properties on devices.
 >   * Simplified the machine ECID/UUID extraction script and
actually created a
 >     file for it rather than quoting its code in documentation.
 >
 > v7:
 >   * Tiny error handling fix, un-inlined function.
 >
 >   MAINTAINERS                 |   1 +
 >   contrib/vmapple/uuid.sh     |   9 +
 >   docs/system/arm/vmapple.rst |  60 
 >   docs/system/target-arm.rst  |   1 +
 >   hw/vmapple/Kconfig          |  20 ++
 >   hw/vmapple/meson.build      |   1 +
 >   hw/vmapple/vmapple.c        | 659 +
+++
 >   7 files changed, 751 insertions(+)
 >   create mode 100755 contrib/vmapple/uuid.sh
 >   create mode 100644 docs/system/arm/vmapple.rst
 >   create mode 100644 hw/vmapple/vmapple.c
 >
 > diff --git a/MAINTAINERS b/MAINTAINERS
 > index 9d0d26cb65f..9591fd44a34 100644
 > --- a/MAINTAINERS
 > +++ b/MAINTAINERS
 > @@ -2767,6 +2767,7 @@ R: Phil Dennis-Jordan mailto:p...@philjordan.eu>>
 >   S: Maintained
 >   F: hw/vmapple/*
 >   F: include/hw/vmapple/*
 > +F: docs/system/arm/vmapple.rst
 >
 >   Subsystems
 >   --
 > diff --git a/contrib/vmapple/uuid.sh b/contrib/vmapple/uuid.sh
 > new file mode 100755
 > index 000..956e8c3afed
 > --- /dev/null
 > +++ b/contrib/vmapple/uuid.sh
 > @@ -0,0 +1,9 @@
 > +#!/bin/sh
 > +# Used for converting a guest provisioned using
Virtualization.framework
 > +# for use with the QEMU 'vmapple' aarch64 machine type.
 > +#
 > +# Extracts the Machine UUID from Virtualization.framework VM
JSON file.
 > +# (as produced by 'macosvm', passed as

[PATCH v7 0/3] vhost-user-blk: live resize additional APIs

2024-11-06 Thread Vladimir Sementsov-Ogievskiy
v7: update QAPI version 9.1 -> 9.2

Vladimir Sementsov-Ogievskiy (3):
  qdev-monitor: add option to report GenericError from find_device_state
  vhost-user-blk: split vhost_user_blk_sync_config()
  qapi: introduce device-sync-config

 hw/block/vhost-user-blk.c | 27 ++--
 hw/virtio/virtio-pci.c|  9 +++
 include/hw/qdev-core.h|  6 +
 qapi/qdev.json| 24 ++
 system/qdev-monitor.c | 53 ---
 5 files changed, 108 insertions(+), 11 deletions(-)

-- 
2.34.1




Re: [PATCH] block: fix possible int overflow

2024-11-06 Thread Kevin Wolf
Am 06.11.2024 um 16:45 hat Denis V. Lunev geschrieben:
> On 11/6/24 10:53, Kevin Wolf wrote:
> > [ Cc: qemu-block ]
> > 
> > Am 06.11.2024 um 09:04 hat Dmitry Frolov geschrieben:
> > > The sum "cluster_index + count" may overflow uint32_t.
> > > 
> > > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > > 
> > > Signed-off-by: Dmitry Frolov 
> > Thanks, applied to the block branch.
> > 
> > While trying to check if this can be triggered in practice, I found this
> > line in parallels_fill_used_bitmap():
> > 
> >  s->used_bmap_size = DIV_ROUND_UP(payload_bytes, s->cluster_size);
> > 
> > s->used_bmap_size is unsigned long, payload_bytes is the int64_t result
> > of bdrv_getlength() for the image file, which could certainly be made
> > more than 4 GB * cluster_size. I think we need an overflow check there,
> > too.
> > 
> > When allocate_clusters() calculates new_usedsize, it doesn't seem to
> > consider the overflow case either.
> > 
> > Denis, can you take a look?
> > 
> > Kevin
> > 
> Hi, Kevin, Dmitry!
> 
> In general, the situation is the following.
> 
> On-disk format heavily uses offsets from the beginning of the disk
> denominated in clusters. These offsets are saved in uint32 on disk.
> This means that the image with 4T virtual size and 1M cluster size
> will use offsets from 0 to 4 * 2^10 in different tables on disk.
> 
> There is existing problem in the format specification that we
> can not easily apply limits to the virtual size of the disk as
> we also can have arbitrary size growing metadata like CBT, which
> is kept in the same address space (cluster offsets).
> 
> Though in reality I have never seen images with non-1 Mb cluster
> size and in order to nearly overflow them we would need really
> allocated images of 4 PB.
> 
> Theoretically the problem is possible but it looks impractical
> to me in the real life so far.

It probably won't happen with normal images, but we need to consider
malicious images, and I think they could be constructed in a way that
causes integer overflows here.

At least the one that directly takes bdrv_getlength() should be trivial
to trigger, you just need to extend the file size enough outside of
QEMU.

Kevin




[PATCH v7 3/3] qapi: introduce device-sync-config

2024-11-06 Thread Vladimir Sementsov-Ogievskiy
Add command to sync config from vhost-user backend to the device. It
may be helpful when VHOST_USER_SLAVE_CONFIG_CHANGE_MSG failed or not
triggered interrupt to the guest or just not available (not supported
by vhost-user server).

Command result is racy if allow it during migration. Let's not allow
that.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Markus Armbruster 
Acked-by: Raphael Norwitz 
---
 hw/block/vhost-user-blk.c |  1 +
 hw/virtio/virtio-pci.c|  9 +
 include/hw/qdev-core.h|  6 ++
 qapi/qdev.json| 24 
 system/qdev-monitor.c | 38 ++
 5 files changed, 78 insertions(+)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 48b3dabb8d..7996e49821 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -591,6 +591,7 @@ static void vhost_user_blk_class_init(ObjectClass *klass, 
void *data)
 
 device_class_set_props(dc, vhost_user_blk_properties);
 dc->vmsd = &vmstate_vhost_user_blk;
+dc->sync_config = vhost_user_blk_sync_config;
 set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
 vdc->realize = vhost_user_blk_device_realize;
 vdc->unrealize = vhost_user_blk_device_unrealize;
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 4d832fe845..c5a809b956 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -2385,6 +2385,14 @@ static void virtio_pci_dc_realize(DeviceState *qdev, 
Error **errp)
 vpciklass->parent_dc_realize(qdev, errp);
 }
 
+static int virtio_pci_sync_config(DeviceState *dev, Error **errp)
+{
+VirtIOPCIProxy *proxy = VIRTIO_PCI(dev);
+VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
+
+return qdev_sync_config(DEVICE(vdev), errp);
+}
+
 static void virtio_pci_class_init(ObjectClass *klass, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(klass);
@@ -2401,6 +2409,7 @@ static void virtio_pci_class_init(ObjectClass *klass, 
void *data)
 device_class_set_parent_realize(dc, virtio_pci_dc_realize,
 &vpciklass->parent_dc_realize);
 rc->phases.hold = virtio_pci_bus_reset_hold;
+dc->sync_config = virtio_pci_sync_config;
 }
 
 static const TypeInfo virtio_pci_info = {
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index aa97c34a4b..94914858d8 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -95,6 +95,7 @@ typedef void (*DeviceUnrealize)(DeviceState *dev);
 typedef void (*DeviceReset)(DeviceState *dev);
 typedef void (*BusRealize)(BusState *bus, Error **errp);
 typedef void (*BusUnrealize)(BusState *bus);
+typedef int (*DeviceSyncConfig)(DeviceState *dev, Error **errp);
 
 /**
  * struct DeviceClass - The base class for all devices.
@@ -103,6 +104,9 @@ typedef void (*BusUnrealize)(BusState *bus);
  * property is changed to %true.
  * @unrealize: Callback function invoked when the #DeviceState:realized
  * property is changed to %false.
+ * @sync_config: Callback function invoked when QMP command device-sync-config
+ * is called. Should synchronize device configuration from host to guest part
+ * and notify the guest about the change.
  * @hotpluggable: indicates if #DeviceClass is hotpluggable, available
  * as readonly "hotpluggable" property of #DeviceState instance
  *
@@ -162,6 +166,7 @@ struct DeviceClass {
 DeviceReset legacy_reset;
 DeviceRealize realize;
 DeviceUnrealize unrealize;
+DeviceSyncConfig sync_config;
 
 /**
  * @vmsd: device state serialisation description for
@@ -547,6 +552,7 @@ bool qdev_hotplug_allowed(DeviceState *dev, Error **errp);
  */
 HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev);
 void qdev_unplug(DeviceState *dev, Error **errp);
+int qdev_sync_config(DeviceState *dev, Error **errp);
 void qdev_simple_device_unplug_cb(HotplugHandler *hotplug_dev,
   DeviceState *dev, Error **errp);
 void qdev_machine_creation_done(void);
diff --git a/qapi/qdev.json b/qapi/qdev.json
index 53d147c7b4..25cbcf977b 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -163,3 +163,27 @@
 ##
 { 'event': 'DEVICE_UNPLUG_GUEST_ERROR',
   'data': { '*device': 'str', 'path': 'str' } }
+
+##
+# @device-sync-config:
+#
+# Synchronize device configuration from host to guest part.  First,
+# copy the configuration from the host part (backend) to the guest
+# part (frontend).  Then notify guest software that device
+# configuration changed.
+#
+# The command may be used to notify the guest about block device
+# capcity change.  Currently only vhost-user-blk device supports
+# this.
+#
+# @id: the device's ID or QOM path
+#
+# Features:
+#
+# @unstable: The command is experimental.
+#
+# Since: 9.2
+##
+{ 'command': 'device-sync-config',
+  'features': [ 'unstable' ],
+  'data': {'id': 'str'} }
diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 2c76cef4d8..d25325c4e3 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor

Re: [PATCH] block: fix possible int overflow

2024-11-06 Thread Denis V. Lunev

On 11/6/24 10:53, Kevin Wolf wrote:

[ Cc: qemu-block ]

Am 06.11.2024 um 09:04 hat Dmitry Frolov geschrieben:

The sum "cluster_index + count" may overflow uint32_t.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov 

Thanks, applied to the block branch.

While trying to check if this can be triggered in practice, I found this
line in parallels_fill_used_bitmap():

 s->used_bmap_size = DIV_ROUND_UP(payload_bytes, s->cluster_size);

s->used_bmap_size is unsigned long, payload_bytes is the int64_t result
of bdrv_getlength() for the image file, which could certainly be made
more than 4 GB * cluster_size. I think we need an overflow check there,
too.

When allocate_clusters() calculates new_usedsize, it doesn't seem to
consider the overflow case either.

Denis, can you take a look?

Kevin


Hi, Kevin, Dmitry!

In general, the situation is the following.

On-disk format heavily uses offsets from the beginning of the disk
denominated in clusters. These offsets are saved in uint32 on disk.
This means that the image with 4T virtual size and 1M cluster size
will use offsets from 0 to 4 * 2^10 in different tables on disk.

There is existing problem in the format specification that we
can not easily apply limits to the virtual size of the disk as
we also can have arbitrary size growing metadata like CBT, which
is kept in the same address space (cluster offsets).

Though in reality I have never seen images with non-1 Mb cluster
size and in order to nearly overflow them we would need really
allocated images of 4 PB.

Theoretically the problem is possible but it looks impractical
to me in the real life so far.

Thank you in advance,
    Den



[PATCH v7 1/3] qdev-monitor: add option to report GenericError from find_device_state

2024-11-06 Thread Vladimir Sementsov-Ogievskiy
Here we just prepare for the following patch, making possible to report
GenericError as recommended.

This patch doesn't aim to prevent further use of DeviceNotFound by
future interfaces:

 - find_device_state() is used in blk_by_qdev_id() and qmp_get_blk()
   functions, which may lead to spread of DeviceNotFound anyway
 - also, nothing prevent simply copy-pasting find_device_state() calls
   with false argument

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Markus Armbruster 
Acked-by: Raphael Norwitz 
---
 system/qdev-monitor.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index 320c47b72d..2c76cef4d8 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -885,13 +885,20 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, 
Error **errp)
 object_unref(OBJECT(dev));
 }
 
-static DeviceState *find_device_state(const char *id, Error **errp)
+/*
+ * Note that creating new APIs using error classes other than GenericError is
+ * not recommended. Set use_generic_error=true for new interfaces.
+ */
+static DeviceState *find_device_state(const char *id, bool use_generic_error,
+  Error **errp)
 {
 Object *obj = object_resolve_path_at(qdev_get_peripheral(), id);
 DeviceState *dev;
 
 if (!obj) {
-error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+error_set(errp,
+  (use_generic_error ?
+   ERROR_CLASS_GENERIC_ERROR : ERROR_CLASS_DEVICE_NOT_FOUND),
   "Device '%s' not found", id);
 return NULL;
 }
@@ -956,7 +963,7 @@ void qdev_unplug(DeviceState *dev, Error **errp)
 
 void qmp_device_del(const char *id, Error **errp)
 {
-DeviceState *dev = find_device_state(id, errp);
+DeviceState *dev = find_device_state(id, false, errp);
 if (dev != NULL) {
 if (dev->pending_deleted_event &&
 (dev->pending_deleted_expires_ms == 0 ||
@@ -1076,7 +1083,7 @@ BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
 
 GLOBAL_STATE_CODE();
 
-dev = find_device_state(id, errp);
+dev = find_device_state(id, false, errp);
 if (dev == NULL) {
 return NULL;
 }
-- 
2.34.1




Re: [PATCH v7 15/15] hw/vmapple/vmapple: Add vmapple machine type

2024-11-06 Thread Phil Dennis-Jordan
On Wed, 6 Nov 2024 at 08:42, Akihiko Odaki  wrote:

> On 2024/11/06 0:30, Phil Dennis-Jordan wrote:
> > From: Alexander Graf 
> >
> > Apple defines a new "vmapple" machine type as part of its proprietary
> > macOS Virtualization.Framework vmm. This machine type is similar to the
> > virt one, but with subtle differences in base devices, a few special
> > vmapple device additions and a vastly different boot chain.
> >
> > This patch reimplements this machine type in QEMU. To use it, you
> > have to have a readily installed version of macOS for VMApple,
> > run on macOS with -accel hvf, pass the Virtualization.Framework
> > boot rom (AVPBooter) in via -bios, pass the aux and root volume as pflash
> > and pass aux and root volume as virtio drives. In addition, you also
> > need to find the machine UUID and pass that as -M vmapple,uuid=
> parameter:
> >
> > $ qemu-system-aarch64 -accel hvf -M vmapple,uuid=0x1234 -m 4G \
> >  -bios
> /System/Library/Frameworks/Virtualization.framework/Versions/A/Resources/AVPBooter.vmapple2.bin
> >  -drive file=aux,if=pflash,format=raw \
> >  -drive file=root,if=pflash,format=raw \
> >  -drive file=aux,if=none,id=aux,format=raw \
> >  -device vmapple-virtio-aux,drive=aux \
> >  -drive file=root,if=none,id=root,format=raw \
> >  -device vmapple-virtio-root,drive=root
> >
> > With all these in place, you should be able to see macOS booting
> > successfully.
> >
> > Known issues:
> >   - Keyboard and mouse/tablet input is laggy. The reason for this is
> > either that macOS's XHCI driver is broken when the device/platform
> > does not support MSI/MSI-X, or there's some unfortunate interplay
> > with Qemu's XHCI implementation in this scenario.
> >   - Currently only macOS 12 guests are supported. The boot process for
> > 13+ will need further investigation and adjustment.
> >
> > Signed-off-by: Alexander Graf 
> > Co-authored-by: Phil Dennis-Jordan 
> > Signed-off-by: Phil Dennis-Jordan 
> > ---
> > v3:
> >   * Rebased on latest upstream, updated affinity and NIC creation
> > API usage
> >   * Included Apple-variant virtio-blk in build dependency
> >   * Updated API usage for setting 'redist-region-count' array-typed
> property on GIC.
> >   * Switched from virtio HID devices (for which macOS 12 does not contain
> > drivers) to an XHCI USB controller and USB HID devices.
> >
> > v4:
> >   * Fixups for v4 changes to the other patches in the set.
> >   * Corrected the assert macro to use
> >   * Removed superfluous endian conversions corresponding to cfg's.
> >   * Init error handling improvement.
> >   * No need to select CPU type on TCG, as only HVF is supported.
> >   * Machine type version bumped to 9.2
> >   * #include order improved
> >
> > v5:
> >   * Fixed memory reservation for ecam alias region.
> >   * Better error handling setting properties on devices.
> >   * Simplified the machine ECID/UUID extraction script and actually
> created a
> > file for it rather than quoting its code in documentation.
> >
> > v7:
> >   * Tiny error handling fix, un-inlined function.
> >
> >   MAINTAINERS |   1 +
> >   contrib/vmapple/uuid.sh |   9 +
> >   docs/system/arm/vmapple.rst |  60 
> >   docs/system/target-arm.rst  |   1 +
> >   hw/vmapple/Kconfig  |  20 ++
> >   hw/vmapple/meson.build  |   1 +
> >   hw/vmapple/vmapple.c| 659 
> >   7 files changed, 751 insertions(+)
> >   create mode 100755 contrib/vmapple/uuid.sh
> >   create mode 100644 docs/system/arm/vmapple.rst
> >   create mode 100644 hw/vmapple/vmapple.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 9d0d26cb65f..9591fd44a34 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -2767,6 +2767,7 @@ R: Phil Dennis-Jordan 
> >   S: Maintained
> >   F: hw/vmapple/*
> >   F: include/hw/vmapple/*
> > +F: docs/system/arm/vmapple.rst
> >
> >   Subsystems
> >   --
> > diff --git a/contrib/vmapple/uuid.sh b/contrib/vmapple/uuid.sh
> > new file mode 100755
> > index 000..956e8c3afed
> > --- /dev/null
> > +++ b/contrib/vmapple/uuid.sh
> > @@ -0,0 +1,9 @@
> > +#!/bin/sh
> > +# Used for converting a guest provisioned using Virtualization.framework
> > +# for use with the QEMU 'vmapple' aarch64 machine type.
> > +#
> > +# Extracts the Machine UUID from Virtualization.framework VM JSON file.
> > +# (as produced by 'macosvm', passed as command line argument)
> > +
> > +plutil -extract machineId raw "$1" | base64 -d | plutil -extract ECID
> raw -
> > +
> > diff --git a/docs/system/arm/vmapple.rst b/docs/system/arm/vmapple.rst
> > new file mode 100644
> > index 000..6a634fa4572
> > --- /dev/null
> > +++ b/docs/system/arm/vmapple.rst
> > @@ -0,0 +1,60 @@
> > +VMApple machine emulation
> >
> +
> > +
> > +VMApple is the device model that the macOS built-in hypervisor called
> "Virtualization.fram

Re: [PATCH] migration: Check current_migration in migration_is_running()

2024-11-06 Thread Thomas Huth

On 06/11/2024 18.18, Pierrick Bouvier wrote:

On 11/6/24 01:43, Thomas Huth wrote:

On 05/11/2024 19.27, Peter Xu wrote:

Report shows that commit 34a8892dec broke iotest 055:

https://lore.kernel.org/r/b8806360-a2b6-4608-83a3-db67e264c...@linaro.org


FWIW, this patch also fixes a lot of other broken iotests (vmdk and vpc)
that occur when running "make check SPEED=thorough".

Tested-by: Thomas Huth 





Good news!

I'm a bit confused by your message. I thought SPEED=slow was the most 
complete test setup, but is it SPEED=thorough instead?


It depends... for the qtests, "slow" and "thorough" is the same, but for the 
iotests, we enable even more additional formats with "thorough".


 Thomas




Re: [PATCH v3 6/7] qapi/block-core: deprecate block-job-change

2024-11-06 Thread Markus Armbruster
Vladimir Sementsov-Ogievskiy  writes:

> That's a first step to move on newer job-* APIs.
>
> The difference between block-job-change and job-change is in
> find_block_job_locked() vs find_job_locked() functions. What's
> different?
>
> 1. find_block_job_locked() finds only block jobs, whereas
>find_job_locked() finds any kind of job.  job-change is a
>compatible extension of block-job-change.
>
> 2. find_block_job_locked() reports DeviceNotActive on failure, when
>find_job_locked() reports GenericError.  Since the kind of error
>reported isn't documented for either command, and clients
>shouldn't rely on undocumented error details, job-change is a
>compatible replacement for block-job-change.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Acked-by: Markus Armbruster 




Re: [PATCH v7 15/15] hw/vmapple/vmapple: Add vmapple machine type

2024-11-06 Thread Phil Dennis-Jordan
On Wed, 6 Nov 2024 at 10:16, Akihiko Odaki  wrote:

> On 2024/11/06 18:10, Phil Dennis-Jordan wrote:
> >
> >
> > On Wed, 6 Nov 2024 at 08:42, Akihiko Odaki  > > wrote:
> >
> > On 2024/11/06 0:30, Phil Dennis-Jordan wrote:
> >  > From: Alexander Graf mailto:g...@amazon.com>>
> >  >
> >  > Apple defines a new "vmapple" machine type as part of its
> proprietary
> >  > macOS Virtualization.Framework vmm. This machine type is similar
> > to the
> >  > virt one, but with subtle differences in base devices, a few
> special
> >  > vmapple device additions and a vastly different boot chain.
> >  >
> >  > This patch reimplements this machine type in QEMU. To use it, you
> >  > have to have a readily installed version of macOS for VMApple,
> >  > run on macOS with -accel hvf, pass the Virtualization.Framework
> >  > boot rom (AVPBooter) in via -bios, pass the aux and root volume
> > as pflash
> >  > and pass aux and root volume as virtio drives. In addition, you
> also
> >  > need to find the machine UUID and pass that as -M vmapple,uuid=
> > parameter:
> >  >
> >  > $ qemu-system-aarch64 -accel hvf -M vmapple,uuid=0x1234 -m 4G \
> >  >  -bios /System/Library/Frameworks/Virtualization.framework/
> > Versions/A/Resources/AVPBooter.vmapple2.bin
> >  >  -drive file=aux,if=pflash,format=raw \
> >  >  -drive file=root,if=pflash,format=raw \
> >  >  -drive file=aux,if=none,id=aux,format=raw \
> >  >  -device vmapple-virtio-aux,drive=aux \
> >  >  -drive file=root,if=none,id=root,format=raw \
> >  >  -device vmapple-virtio-root,drive=root
> >  >
> >  > With all these in place, you should be able to see macOS booting
> >  > successfully.
> >  >
> >  > Known issues:
> >  >   - Keyboard and mouse/tablet input is laggy. The reason for this
> is
> >  > either that macOS's XHCI driver is broken when the device/
> > platform
> >  > does not support MSI/MSI-X, or there's some unfortunate
> interplay
> >  > with Qemu's XHCI implementation in this scenario.
> >  >   - Currently only macOS 12 guests are supported. The boot
> > process for
> >  > 13+ will need further investigation and adjustment.
> >  >
> >  > Signed-off-by: Alexander Graf  > >
> >  > Co-authored-by: Phil Dennis-Jordan  > >
> >  > Signed-off-by: Phil Dennis-Jordan  > >
> >  > ---
> >  > v3:
> >  >   * Rebased on latest upstream, updated affinity and NIC creation
> >  > API usage
> >  >   * Included Apple-variant virtio-blk in build dependency
> >  >   * Updated API usage for setting 'redist-region-count' array-
> > typed property on GIC.
> >  >   * Switched from virtio HID devices (for which macOS 12 does not
> > contain
> >  > drivers) to an XHCI USB controller and USB HID devices.
> >  >
> >  > v4:
> >  >   * Fixups for v4 changes to the other patches in the set.
> >  >   * Corrected the assert macro to use
> >  >   * Removed superfluous endian conversions corresponding to cfg's.
> >  >   * Init error handling improvement.
> >  >   * No need to select CPU type on TCG, as only HVF is supported.
> >  >   * Machine type version bumped to 9.2
> >  >   * #include order improved
> >  >
> >  > v5:
> >  >   * Fixed memory reservation for ecam alias region.
> >  >   * Better error handling setting properties on devices.
> >  >   * Simplified the machine ECID/UUID extraction script and
> > actually created a
> >  > file for it rather than quoting its code in documentation.
> >  >
> >  > v7:
> >  >   * Tiny error handling fix, un-inlined function.
> >  >
> >  >   MAINTAINERS |   1 +
> >  >   contrib/vmapple/uuid.sh |   9 +
> >  >   docs/system/arm/vmapple.rst |  60 
> >  >   docs/system/target-arm.rst  |   1 +
> >  >   hw/vmapple/Kconfig  |  20 ++
> >  >   hw/vmapple/meson.build  |   1 +
> >  >   hw/vmapple/vmapple.c| 659 +
> > +++
> >  >   7 files changed, 751 insertions(+)
> >  >   create mode 100755 contrib/vmapple/uuid.sh
> >  >   create mode 100644 docs/system/arm/vmapple.rst
> >  >   create mode 100644 hw/vmapple/vmapple.c
> >  >
> >  > diff --git a/MAINTAINERS b/MAINTAINERS
> >  > index 9d0d26cb65f..9591fd44a34 100644
> >  > --- a/MAINTAINERS
> >  > +++ b/MAINTAINERS
> >  > @@ -2767,6 +2767,7 @@ R: Phil Dennis-Jordan  > >
> >  >   S: Maintained
> >  >   F: hw/vmapple/*
> >  >   F: include/hw/vmapple/*
> >  > +F: docs/system/arm/vmapple.rst
> >  >
> >  >   Subsystems
> >  >   --
> >  > diff --g

Re: [PATCH 2/2] hw/sd: Remove legacy sd_enable()

2024-11-06 Thread Philippe Mathieu-Daudé

On 9/9/24 16:26, Peter Maydell wrote:

On Tue, 3 Sept 2024 at 21:04, Philippe Mathieu-Daudé  wrote:


sd_enable() was only used by omap_mmc_enable() which
got recently removed. Time to remove it.

Since the SDState::enable boolean is now always %true,
we can remove it and simplify.

Signed-off-by: Philippe Mathieu-Daudé 



@@ -2328,7 +2327,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
  {
  int i;

-if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
+if (!sd->blk || !blk_is_inserted(sd->blk))
  return;

  if (sd->state != sd_receivingdata_state) {
@@ -2460,7 +2459,7 @@ uint8_t sd_read_byte(SDState *sd)
  uint8_t ret;
  uint32_t io_len;

-if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
+if (!sd->blk || !blk_is_inserted(sd->blk))
  return dummy_byte;

  if (sd->state != sd_sendingdata_state) {


Maybe add the { } that coding style wants to these if()s
since we're editing them anyway?


Sure, I didn't notice.


Either way,
Reviewed-by: Peter Maydell 


Thanks, patch queued.



Re: [PATCH v4 00/26] E500 Cleanup

2024-11-06 Thread Bernhard Beschow



Am 5. November 2024 22:55:20 UTC schrieb "Philippe Mathieu-Daudé" 
:
>Hi Bernhard,
>
>On 3/11/24 14:33, Bernhard Beschow wrote:
>> This series is part of a bigger series exploring data-driven machine creation
>> using device tree blobs on top of the e500 machines [1]. It contains patches 
>> to
>> make this exploration easier which are also expected to provide value in
>> themselves.
>> 
>> The cleanup starts with the e500 machine class itself, then proceeds with
>> machine-specific device models and concludes with more or less loosely 
>> related
>> devices. Device cleanup mostly consists of using the DEFINE_TYPES() macro.
>> 
>> Patches still missing R-b tags: 1,2,6,8,9,15,23,26
>
>I queued most of the reviewed patches.

Thanks, Phil. Much appreciated!

Best regards,
Bernhard

>
>> Bernhard Beschow (26):
>>hw/ppc/e500: Do not leak struct boot_info
>>hw/ppc/e500: Remove firstenv variable
>>hw/ppc/e500: Prefer QOM cast
>>hw/ppc/e500: Remove unused "irqs" parameter
>>hw/ppc/e500: Add missing device tree properties to i2c controller node
>>hw/ppc/e500: Reuse TYPE_GPIO_PWR
>>hw/ppc/e500: Use SysBusDevice API to access TYPE_CCSR's internal
>>  resources
>>hw/ppc/e500: Extract ppce500_ccsr.c
>>hw/ppc/ppce500_ccsr: Trace access to CCSR region
>>hw/ppc/mpc8544_guts: Populate POR PLL ratio status register
>>hw/i2c/mpc_i2c: Convert DPRINTF to trace events for register access
>>hw/i2c/mpc_i2c: Prefer DEFINE_TYPES() macro
>>hw/pci-host/ppce500: Reuse TYPE_PPC_E500_PCI_BRIDGE define
>>hw/pci-host/ppce500: Prefer DEFINE_TYPES() macro
>>hw/net/fsl_etsec/miim: Reuse MII constants
>>hw/net/fsl_etsec/etsec: Prefer DEFINE_TYPES() macro
>>hw/gpio/mpc8xxx: Prefer DEFINE_TYPES() macro
>>hw/ppc/mpc8544_guts: Prefer DEFINE_TYPES() macro
>>hw/intc: Guard openpic_kvm.c by dedicated OPENPIC_KVM Kconfig switch
>>hw/sd/sdhci: Prefer DEFINE_TYPES() macro
>>hw/block/pflash_cfi01: Prefer DEFINE_TYPES() macro
>>hw/i2c/smbus_eeprom: Prefer DEFINE_TYPES() macro
>>hw/rtc/ds1338: Prefer DEFINE_TYPES() macro
>>hw/usb/hcd-ehci-sysbus: Prefer DEFINE_TYPES() macro
>>hw/vfio/platform: Let vfio_start_eventfd_injection() take
>>  VFIOPlatformDevice pointer
>>MAINTAINERS: Add hw/gpio/gpio_pwr.c
>



[PATCH v7 2/3] vhost-user-blk: split vhost_user_blk_sync_config()

2024-11-06 Thread Vladimir Sementsov-Ogievskiy
Split vhost_user_blk_sync_config() out from
vhost_user_blk_handle_config_change(), to be reused in the following
commit.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Acked-by: Raphael Norwitz 
Reviewed-by: Stefano Garzarella 
---
 hw/block/vhost-user-blk.c | 26 +++---
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 5b7f46bbb0..48b3dabb8d 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -90,27 +90,39 @@ static void vhost_user_blk_set_config(VirtIODevice *vdev, 
const uint8_t *config)
 s->blkcfg.wce = blkcfg->wce;
 }
 
+static int vhost_user_blk_sync_config(DeviceState *dev, Error **errp)
+{
+int ret;
+VirtIODevice *vdev = VIRTIO_DEVICE(dev);
+VHostUserBlk *s = VHOST_USER_BLK(vdev);
+
+ret = vhost_dev_get_config(&s->dev, (uint8_t *)&s->blkcfg,
+   vdev->config_len, errp);
+if (ret < 0) {
+return ret;
+}
+
+memcpy(vdev->config, &s->blkcfg, vdev->config_len);
+virtio_notify_config(vdev);
+
+return 0;
+}
+
 static int vhost_user_blk_handle_config_change(struct vhost_dev *dev)
 {
 int ret;
-VirtIODevice *vdev = dev->vdev;
-VHostUserBlk *s = VHOST_USER_BLK(dev->vdev);
 Error *local_err = NULL;
 
 if (!dev->started) {
 return 0;
 }
 
-ret = vhost_dev_get_config(dev, (uint8_t *)&s->blkcfg,
-   vdev->config_len, &local_err);
+ret = vhost_user_blk_sync_config(DEVICE(dev->vdev), &local_err);
 if (ret < 0) {
 error_report_err(local_err);
 return ret;
 }
 
-memcpy(dev->vdev->config, &s->blkcfg, vdev->config_len);
-virtio_notify_config(dev->vdev);
-
 return 0;
 }
 
-- 
2.34.1