[PATCH ssh] ssh: Do not switch session to non-blocking mode
From: Jakub Jelen The libssh does not handle non-blocking mode in SFTP correctly. The driver code already changes the mode to blocking for the SFTP initialization, but for some reason changes to non-blocking mode. This used to work accidentally until libssh in 0.11 branch merged the patch to avoid infinite looping in case of network errors: https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498 Since then, the ssh driver in qemu fails to read files over SFTP as the first SFTP messages exchanged after switching the session to non-blocking mode return SSH_AGAIN, but that message is lost int the SFTP internals and interpretted as SSH_ERROR, which is returned to the caller: https://gitlab.com/libssh/libssh-mirror/-/issues/280 This is indeed an issue in libssh that we should address in the long term, but it will require more work on the internals. For now, the SFTP is not supported in non-blocking mode. Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280 Signed-off-by: Jakub Jelen Signed-off-by: Richard W.M. Jones --- block/ssh.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/block/ssh.c b/block/ssh.c index 9f8140bcb6..e1529cfda9 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -866,8 +866,6 @@ static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags, goto err; } -/* Go non-blocking. */ -ssh_set_blocking(s->session, 0); if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) { bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; -- 2.46.0
[PATCH ssh v2] ssh: Do not switch session to non-blocking mode
From: Jakub Jelen The libssh does not handle non-blocking mode in SFTP correctly. The driver code already changes the mode to blocking for the SFTP initialization, but for some reason changes to non-blocking mode. This used to work accidentally until libssh in 0.11 branch merged the patch to avoid infinite looping in case of network errors: https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498 Since then, the ssh driver in qemu fails to read files over SFTP as the first SFTP messages exchanged after switching the session to non-blocking mode return SSH_AGAIN, but that message is lost int the SFTP internals and interpretted as SSH_ERROR, which is returned to the caller: https://gitlab.com/libssh/libssh-mirror/-/issues/280 This is indeed an issue in libssh that we should address in the long term, but it will require more work on the internals. For now, the SFTP is not supported in non-blocking mode. Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280 Signed-off-by: Jakub Jelen Signed-off-by: Richard W.M. Jones --- block/ssh.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/block/ssh.c b/block/ssh.c index 9f8140bcb6..b9f33ec739 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -866,9 +866,6 @@ static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags, goto err; } -/* Go non-blocking. */ -ssh_set_blocking(s->session, 0); - if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) { bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; } -- 2.46.0
Re: [PATCH ssh] ssh: Do not switch session to non-blocking mode
On Wed, Nov 13, 2024 at 03:02:59PM +0300, Michael Tokarev wrote: > Heh. I was creating a qemu bug report on gitlab when this email arrived :) > > 13.11.2024 14:49, Richard W.M. Jones wrote: > >From: Jakub Jelen > > > >The libssh does not handle non-blocking mode in SFTP correctly. The > >driver code already changes the mode to blocking for the SFTP > >initialization, but for some reason changes to non-blocking mode. > > "changes to non-blocking mode LATER", I guess, - or else it's a bit > difficult to read. But this works too. > > >This used to work accidentally until libssh in 0.11 branch merged > >the patch to avoid infinite looping in case of network errors: > > > >https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498 > > > >Since then, the ssh driver in qemu fails to read files over SFTP > >as the first SFTP messages exchanged after switching the session > >to non-blocking mode return SSH_AGAIN, but that message is lost > >int the SFTP internals and interpretted as SSH_ERROR, which is > >returned to the caller: > > > >https://gitlab.com/libssh/libssh-mirror/-/issues/280 > > > >This is indeed an issue in libssh that we should address in the > >long term, but it will require more work on the internals. For > >now, the SFTP is not supported in non-blocking mode. > > The comment at init where the code sets socket to blocking mode, says: > > /* > * Make sure we are in blocking mode during the connection and > * authentication phases. > */ > ssh_set_blocking(s->session, 1); > > > There are a few other places where the code expect "some" blocking > mode, changes it to blocking, and restores the mode later, - eg, > see ssh_grow_file(). It looks all this has to be fixed too. I'll just note that I'm only forwarding on the patch from Jakub. I didn't write it. I did lightly test it, and it seems to work. However it seems also likely that it is causing qemu to block internally. Probably not noticable for light use, but not something that we'd want for serious use. However if libssh doesn't support non-blocking SFTP there's not much we can do about that in qemu. I would recommend using nbdkit-ssh-plugin instead anyway as it is much more featureful and doesn't have this problem as we use real threads instead of coroutines. > I wonder if qemu ssh driver needs to mess with blocking mode of this > socket in the first place, ever. Is there a way qemu can get non-blocking > socket in this context? I can only think of fd=NNN, but is it > possible for this socket to be non-blocking? > > >Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280 > >Signed-off-by: Jakub Jelen > >Signed-off-by: Richard W.M. Jones > >--- > > block/ssh.c | 2 -- > > 1 file changed, 2 deletions(-) > > > >diff --git a/block/ssh.c b/block/ssh.c > >index 9f8140bcb6..e1529cfda9 100644 > >--- a/block/ssh.c > >+++ b/block/ssh.c > >@@ -866,8 +866,6 @@ static int ssh_open(BlockDriverState *bs, QDict > >*options, int bdrv_flags, > > goto err; > > } > >-/* Go non-blocking. */ > >-ssh_set_blocking(s->session, 0); > > if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) { > > bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; > > Please remove the empty line too. I posted a v2 which removes the blank line but is otherwise the same. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
Re: [PATCH ssh] ssh: Do not switch session to non-blocking mode
Heh. I was creating a qemu bug report on gitlab when this email arrived :) 13.11.2024 14:49, Richard W.M. Jones wrote: From: Jakub Jelen The libssh does not handle non-blocking mode in SFTP correctly. The driver code already changes the mode to blocking for the SFTP initialization, but for some reason changes to non-blocking mode. "changes to non-blocking mode LATER", I guess, - or else it's a bit difficult to read. But this works too. This used to work accidentally until libssh in 0.11 branch merged the patch to avoid infinite looping in case of network errors: https://gitlab.com/libssh/libssh-mirror/-/merge_requests/498 Since then, the ssh driver in qemu fails to read files over SFTP as the first SFTP messages exchanged after switching the session to non-blocking mode return SSH_AGAIN, but that message is lost int the SFTP internals and interpretted as SSH_ERROR, which is returned to the caller: https://gitlab.com/libssh/libssh-mirror/-/issues/280 This is indeed an issue in libssh that we should address in the long term, but it will require more work on the internals. For now, the SFTP is not supported in non-blocking mode. The comment at init where the code sets socket to blocking mode, says: /* * Make sure we are in blocking mode during the connection and * authentication phases. */ ssh_set_blocking(s->session, 1); There are a few other places where the code expect "some" blocking mode, changes it to blocking, and restores the mode later, - eg, see ssh_grow_file(). It looks all this has to be fixed too. I wonder if qemu ssh driver needs to mess with blocking mode of this socket in the first place, ever. Is there a way qemu can get non-blocking socket in this context? I can only think of fd=NNN, but is it possible for this socket to be non-blocking? Fixes: https://gitlab.com/libssh/libssh-mirror/-/issues/280 Signed-off-by: Jakub Jelen Signed-off-by: Richard W.M. Jones --- block/ssh.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/block/ssh.c b/block/ssh.c index 9f8140bcb6..e1529cfda9 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -866,8 +866,6 @@ static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags, goto err; } -/* Go non-blocking. */ -ssh_set_blocking(s->session, 0); if (s->attrs->type == SSH_FILEXFER_TYPE_REGULAR) { bs->supported_truncate_flags = BDRV_REQ_ZERO_WRITE; Please remove the empty line too. /mjt
[PATCH v10 00/15] macOS PV Graphics and new vmapple machine type
This patch set introduces a new ARM and macOS HVF specific machine type called "vmapple", as well as a family of display devices based on the ParavirtualizedGraphics.framework in macOS. One of the display adapter variants, apple-gfx-mmio, is required for the new machine type, while apple-gfx-pci can be used to enable 3D graphics acceleration with x86-64 macOS guest OSes. Previous versions of this patch set were submitted semi-separately: the original vmapple patch set by Alexander Graf included a monolithic implementation of apple-gfx-mmio. I subsequently reviewed and reworked the latter to support the PCI variant of the device as well and submitted the result in isolation. As requested in subsequent review, I have now recombined this with the original vmapple patch set, which I have updated and improved in a few ways as well. The vmapple machine type approximates the configuration in macOS's own Virtualization.framework when running arm64 macOS guests. In addition to generic components such as a GICv3 and an XHCI USB controller, it includes nonstandard extensions to the virtio block device, a special "hardware" aes engine, a configuration device, a pvpanic variant, a "backdoor" interface, and of course the apple-gfx paravirtualised display adapter. There are currently a few limitations to this which aren't intrinsic, just imperfect emulation of the VZF, but it's good enough to be just about usable for some purposes: * macOS 12 guests only. Versions 13+ currently fail during early boot. * macOS 11+ arm64 hosts only, with hvf accel. (Perhaps some differences between Apple M series CPUs and TCG's aarch64 implementation? macOS hosts only because ParavirtualizedGraphics.framework is a black box implementing most of the logic behind the apple-gfx device.) * PCI devices use legacy IRQs, not MSI/MSI-X. As far as I can tell, we'd need to include the GICv3 ITS, but it's unclear to me what exactly needs wiring up. * Due to lack of MSI(-X), event delivery from USB devices to the guest macOS isn't working correctly. My current conclusion is that the OS's XHCI driver simply was never designed to work with legacy IRQs. The upshot is that keyboard and mouse/tablet input is very laggy. The solution would be to implement MSI(-X) support or figure out how to make hcd-xhci-sysbus work with the macOS guest, if at all possible. (EHCI and UHCI/OHCI controllers are not an option as the VMAPPLE guest kernel does not include drivers for these.) * The guest OS must first be provisioned using Virtualization.framework; the disk images can subsequently be used in Qemu. (See docs.) The apple-gfx device can be used independently from the vmapple machine type, at least in the PCI variant. It mainly targets x86-64 macOS guests from version 11 on, but also includes a UEFI bootrom for basic framebuffer mode. macOS 11 is also required on the host side, as well as a GPU that supports the Metal API. On the guest side, this provides 3D acceleration/GPGPU support with a baseline Metal feature set, irrespective of the host GPU's feature set. A few limitations in the current integration: * Although it works fine with TCG, it does not work correctly cross-architecture: x86-64 guests on arm64 hosts appear to make some boot progress, but rendering is corrupted. I suspect incompatible texture memory layouts; I have no idea if this is fixable. * ParavirtualizedGraphics.framework and the guest driver support multi-headed configurations. The current Qemu integration always connects precisely 1 display. * State serialisation and deserialisation is currently not implemented, though supported in principle by the framework. Both apple-gfx variants thus set up a migration blocker. * Rendering efficiency could be better. The GPU-rendered guest framebuffer is copied to system memory and uses Qemu's usual CPU-based drawing. For maximum efficiency, the Metal texture containing the guest framebuffer could be drawn directly to a Metal view in the host window, staying on the GPU. (Similar to the OpenGL/virgl render path on other platforms.) Some of my part of this work has been sponsored by Sauce Labs Inc. --- v2 -> v3: * Merged the apple-gfx and vmapple patchsets. * Squashed a bunch of later apple-gfx patches into the main one. (dGPU support, queried MMIO area size, host GPU picking logic.) * Rebased on latest upstream, fixing any breakages due to internal Qemu API changes. * apple-gfx: Switched to re-entrant MMIO. This is supported by the underlying framework and simplifies the MMIO forwarding code which was previously different on x86-64 vs aarch64. * vmapple: Fixes for minor bugs and comments from the last round of review. * vmapple aes, conf, apple-gfx: Switched reset methods to implement the ResettableClass base's interface. * vmapple: switched from virtio-hid to an XHCI USB controller and USB mouse and tablet devices. macOS does not pro
[PATCH v10 02/15] hw/display/apple-gfx: Introduce ParavirtualizedGraphics.Framework support
MacOS provides a framework (library) that allows any vmm to implement a paravirtualized 3d graphics passthrough to the host metal stack called ParavirtualizedGraphics.Framework (PVG). The library abstracts away almost every aspect of the paravirtualized device model and only provides and receives callbacks on MMIO access as well as to share memory address space between the VM and PVG. This patch implements a QEMU device that drives PVG for the VMApple variant of it. Signed-off-by: Alexander Graf Co-authored-by: Alexander Graf Subsequent changes: * Cherry-pick/rebase conflict fixes, API use updates. * Moved from hw/vmapple/ (useful outside that machine type) * Overhaul of threading model, many thread safety improvements. * Asynchronous rendering. * Memory and object lifetime fixes. * Refactoring to split generic and (vmapple) MMIO variant specific code. Implementation wise, most of the complexity lies in the differing threading models of ParavirtualizedGraphics.framework, which uses libdispatch and internal locks, versus QEMU, which heavily uses the BQL, especially during memory-mapped device I/O. Great care has therefore been taken to prevent deadlocks by never calling into PVG methods while holding the BQL, and similarly never acquiring the BQL in a callback from PVG. Different strategies have been used (libdispatch, blocking and non-blocking BHs, RCU, etc.) depending on the specific requirements at each framework entry and exit point. Signed-off-by: Phil Dennis-Jordan --- v2: * Cherry-pick/rebase conflict fixes * BQL function renaming * Moved from hw/vmapple/ (useful outside that machine type) * Code review comments: Switched to DEFINE_TYPES macro & little endian MMIO. * Removed some dead/superfluous code * Mad set_mode thread & memory safe * Added migration blocker due to lack of (de-)serialisation. * Fixes to ObjC refcounting and autorelease pool usage. * Fixed ObjC new/init misuse * Switched to ObjC category extension for private property. * Simplified task memory mapping and made it thread safe. * Refactoring to split generic and vmapple MMIO variant specific code. * Switched to asynchronous MMIO writes on x86-64 * Rendering and graphics update are now done asynchronously * Fixed cursor handling * Coding convention fixes * Removed software cursor compositing v3: * Rebased on latest upstream, fixed breakages including switching to Resettable methods. * Squashed patches dealing with dGPUs, MMIO area size, and GPU picking. * Allow re-entrant MMIO; this simplifies the code and solves the divergence between x86-64 and arm64 variants. v4: * Renamed '-vmapple' device variant to '-mmio' * MMIO device type now requires aarch64 host and guest * Complete overhaul of the glue code for making Qemu's and ParavirtualizedGraphics.framework's threading and synchronisation models work together. Calls into PVG are from dispatch queues while the BQL-holding initiating thread processes AIO context events; callbacks from PVG are scheduled as BHs on the BQL/main AIO context, awaiting completion where necessary. * Guest frame rendering state is covered by the BQL, with only the PVG calls outside the lock, and serialised on the named render_queue. * Simplified logic for dropping frames in-flight during mode changes, fixed bug in pending frames logic. * Addressed smaller code review notes such as: function naming, object type declarations, type names/declarations/casts, code formatting, #include order, over-cautious ObjC retain/release, what goes in init vs realize, etc. v5: * Smaller non-functional fixes in response to review comments, such as using NULL for the AIO_WAIT_WHILE context argument, type name formatting, deleting leftover debug code, logging improvements, state struct field order and documentation improvements, etc. * Instead of a single condition variable for all synchronous BH job types, there is now one for each callback block. This reduces the number of threads being awoken unnecessarily to near zero. * MMIO device variant: Unified the BH job for raising interrupts. * Use DMA APIs for PVG framework's guest memory read requests. * Thread safety improvements: ensure mutable AppleGFXState fields are not accessed outside the appropriate lock. Added dedicated mutex for the task list. * Retain references to MemoryRegions for which there exist mappings in each PGTask, and for IOSurface mappings. v6: * Switched PGTask_s's' mapped_regions from GPtrArray to GArray * Allow DisplaySurface to manage its own vram now that texture -> vram copy occurs under BQL. * Memory mapping operations now use RCU_READ_LOCK_GUARD() where possible instead of a heavy-weight BH job to acquire the BQL. * Changed PVG cursor and mode setting callbacks to kick off BHs instead of libdispatch tasks which then locked the BQL explicitly. * The single remaining callback which must wait for a BH to complete now
[PATCH v10 07/15] hw/misc/pvpanic: Add MMIO interface
From: Alexander Graf In addition to the ISA and PCI variants of pvpanic, let's add an MMIO platform device that we can use in embedded arm environments. Signed-off-by: Alexander Graf Reviewed-by: Philippe Mathieu-Daudé Tested-by: Philippe Mathieu-Daudé Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v3: * Rebased on upstream, updated a header path hw/misc/Kconfig | 4 +++ hw/misc/meson.build | 1 + hw/misc/pvpanic-mmio.c| 61 +++ include/hw/misc/pvpanic.h | 1 + 4 files changed, 67 insertions(+) create mode 100644 hw/misc/pvpanic-mmio.c diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig index 1f1baa5dde9..5a6c1603b60 100644 --- a/hw/misc/Kconfig +++ b/hw/misc/Kconfig @@ -145,6 +145,10 @@ config PVPANIC_ISA depends on ISA_BUS select PVPANIC_COMMON +config PVPANIC_MMIO +bool +select PVPANIC_COMMON + config AUX bool select I2C diff --git a/hw/misc/meson.build b/hw/misc/meson.build index d02d96e403b..4de4db0a600 100644 --- a/hw/misc/meson.build +++ b/hw/misc/meson.build @@ -122,6 +122,7 @@ system_ss.add(when: 'CONFIG_ARMSSE_MHU', if_true: files('armsse-mhu.c')) system_ss.add(when: 'CONFIG_PVPANIC_ISA', if_true: files('pvpanic-isa.c')) system_ss.add(when: 'CONFIG_PVPANIC_PCI', if_true: files('pvpanic-pci.c')) +system_ss.add(when: 'CONFIG_PVPANIC_MMIO', if_true: files('pvpanic-mmio.c')) system_ss.add(when: 'CONFIG_AUX', if_true: files('auxbus.c')) system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: files( 'aspeed_hace.c', diff --git a/hw/misc/pvpanic-mmio.c b/hw/misc/pvpanic-mmio.c new file mode 100644 index 000..56738efee53 --- /dev/null +++ b/hw/misc/pvpanic-mmio.c @@ -0,0 +1,61 @@ +/* + * QEMU simulated pvpanic device (MMIO frontend) + * + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" + +#include "hw/qdev-properties.h" +#include "hw/misc/pvpanic.h" +#include "hw/sysbus.h" +#include "standard-headers/misc/pvpanic.h" + +OBJECT_DECLARE_SIMPLE_TYPE(PVPanicMMIOState, PVPANIC_MMIO_DEVICE) + +#define PVPANIC_MMIO_SIZE 0x2 + +struct PVPanicMMIOState { +SysBusDevice parent_obj; + +PVPanicState pvpanic; +}; + +static void pvpanic_mmio_initfn(Object *obj) +{ +PVPanicMMIOState *s = PVPANIC_MMIO_DEVICE(obj); + +pvpanic_setup_io(&s->pvpanic, DEVICE(s), PVPANIC_MMIO_SIZE); +sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->pvpanic.mr); +} + +static Property pvpanic_mmio_properties[] = { +DEFINE_PROP_UINT8("events", PVPanicMMIOState, pvpanic.events, + PVPANIC_PANICKED | PVPANIC_CRASH_LOADED), +DEFINE_PROP_END_OF_LIST(), +}; + +static void pvpanic_mmio_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); + +device_class_set_props(dc, pvpanic_mmio_properties); +set_bit(DEVICE_CATEGORY_MISC, dc->categories); +} + +static const TypeInfo pvpanic_mmio_info = { +.name = TYPE_PVPANIC_MMIO_DEVICE, +.parent= TYPE_SYS_BUS_DEVICE, +.instance_size = sizeof(PVPanicMMIOState), +.instance_init = pvpanic_mmio_initfn, +.class_init= pvpanic_mmio_class_init, +}; + +static void pvpanic_register_types(void) +{ +type_register_static(&pvpanic_mmio_info); +} + +type_init(pvpanic_register_types) diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h index 9a71a5ad0d7..049a94c1125 100644 --- a/include/hw/misc/pvpanic.h +++ b/include/hw/misc/pvpanic.h @@ -26,6 +26,7 @@ #define TYPE_PVPANIC_ISA_DEVICE "pvpanic" #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci" +#define TYPE_PVPANIC_MMIO_DEVICE "pvpanic-mmio" #define PVPANIC_IOPORT_PROP "ioport" -- 2.39.3 (Apple Git-145)
[PATCH v10 13/15] hw/vmapple/virtio-blk: Add support for apple virtio-blk
From: Alexander Graf Apple has its own virtio-blk PCI device ID where it deviates from the official virtio-pci spec slightly: It puts a new "apple type" field at a static offset in config space and introduces a new barrier command. This patch first creates a mechanism for virtio-blk downstream classes to handle unknown commands. It then creates such a downstream class and a new vmapple-virtio-blk-pci class which support the additional apple type config identifier as well as the barrier command. The 'aux' or 'root' device type are selected using the 'variant' property. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v4: * Use recommended object type declaration pattern. * Correctly log unimplemented code paths. * Most header code moved to .c, type name #defines moved to vmapple.h v5: * Corrected handling of potentially unaligned writes to virtio config area. * Simplified passing through device variant type to subobject. v9: * Correctly specify class_size for VMAppleVirtIOBlkClass v10: * Folded v9 patch 16/16 into this one, changing the device type design to provide a single device type with a variant property instead of 2 different subtypes for aux and root volumes. * Tidied up error reporting for the variant property. hw/block/virtio-blk.c | 19 ++- hw/core/qdev-properties-system.c| 8 ++ hw/vmapple/Kconfig | 3 + hw/vmapple/meson.build | 1 + hw/vmapple/virtio-blk.c | 205 include/hw/pci/pci_ids.h| 1 + include/hw/qdev-properties-system.h | 5 + include/hw/virtio/virtio-blk.h | 12 +- include/hw/vmapple/vmapple.h| 2 + qapi/virtio.json| 14 ++ 10 files changed, 265 insertions(+), 5 deletions(-) create mode 100644 hw/vmapple/virtio-blk.c diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9166d7974d4..9e8337bb639 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -50,12 +50,12 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, req->mr_next = NULL; } -static void virtio_blk_free_request(VirtIOBlockReq *req) +void virtio_blk_free_request(VirtIOBlockReq *req) { g_free(req); } -static void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) +void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) { VirtIOBlock *s = req->dev; VirtIODevice *vdev = VIRTIO_DEVICE(s); @@ -966,8 +966,18 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) break; } default: -virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); -virtio_blk_free_request(req); +{ +/* + * Give subclasses a chance to handle unknown requests. This way the + * class lookup is not in the hot path. + */ +VirtIOBlkClass *vbk = VIRTIO_BLK_GET_CLASS(s); +if (!vbk->handle_unknown_request || +!vbk->handle_unknown_request(req, mrb, type)) { +virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); +virtio_blk_free_request(req); +} +} } return 0; } @@ -2044,6 +2054,7 @@ static const TypeInfo virtio_blk_info = { .instance_size = sizeof(VirtIOBlock), .instance_init = virtio_blk_instance_init, .class_init = virtio_blk_class_init, +.class_size = sizeof(VirtIOBlkClass), }; static void virtio_register_types(void) diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c index 35deef05f32..8bf8a3442d6 100644 --- a/hw/core/qdev-properties-system.c +++ b/hw/core/qdev-properties-system.c @@ -1239,3 +1239,11 @@ const PropertyInfo qdev_prop_iothread_vq_mapping_list = { .set = set_iothread_vq_mapping_list, .release = release_iothread_vq_mapping_list, }; + +const PropertyInfo qdev_prop_vmapple_virtio_blk_variant = { +.name = "VMAppleVirtioBlkVariant", +.enum_table = &VMAppleVirtioBlkVariant_lookup, +.get = qdev_propinfo_get_enum, +.set = qdev_propinfo_set_enum, +.set_default_value = qdev_propinfo_set_default_value_enum, +}; diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig index 8bbeb9a9237..bcd1be63e3c 100644 --- a/hw/vmapple/Kconfig +++ b/hw/vmapple/Kconfig @@ -7,3 +7,6 @@ config VMAPPLE_BDIF config VMAPPLE_CFG bool +config VMAPPLE_VIRTIO_BLK +bool + diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build index 64b78693a31..bf17cf906c9 100644 --- a/hw/vmapple/meson.build +++ b/hw/vmapple/meson.build @@ -1,3 +1,4 @@ system_ss.add(when: 'CONFIG_VMAPPLE_AES', if_true: files('aes.c')) system_ss.add(when: 'CONFIG_VMAPPLE_BDIF', if_true: files('bdif.c')) system_ss.add(when: 'CONFIG_VMAPPLE_CFG', if_true: files('cfg.c')) +system_ss.add(when: 'CONFIG_VMAPPLE_VIRTIO_BLK', if_true: files('virtio-blk.c')) diff --git a/hw/vmapple/virtio-blk.c b/hw/vmapple/virtio-blk.c new file mode 1006
[PATCH v10 03/15] hw/display/apple-gfx: Adds PCI implementation
This change wires up the PCI variant of the paravirtualised graphics device, mainly useful for x86-64 macOS guests, implemented by macOS's ParavirtualizedGraphics.framework. It builds on code shared with the vmapple/mmio variant of the PVG device. Signed-off-by: Phil Dennis-Jordan --- v4: * Threading improvements analogous to those in common apple-gfx code and mmio device variant. * Smaller code review issues addressed. v5: * Minor error handling improvement. v6: * Removed an unused function parameter. v9: * Fixup of changed common call. * Whitespace and comment formatting tweaks. hw/display/Kconfig | 4 + hw/display/apple-gfx-pci.m | 150 + hw/display/meson.build | 1 + 3 files changed, 155 insertions(+) create mode 100644 hw/display/apple-gfx-pci.m diff --git a/hw/display/Kconfig b/hw/display/Kconfig index 6a9b7b19ada..2b53dfd7d26 100644 --- a/hw/display/Kconfig +++ b/hw/display/Kconfig @@ -149,3 +149,7 @@ config MAC_PVG_MMIO bool depends on MAC_PVG && AARCH64 +config MAC_PVG_PCI +bool +depends on MAC_PVG && PCI +default y if PCI_DEVICES diff --git a/hw/display/apple-gfx-pci.m b/hw/display/apple-gfx-pci.m new file mode 100644 index 000..5a27b95ac79 --- /dev/null +++ b/hw/display/apple-gfx-pci.m @@ -0,0 +1,150 @@ +/* + * QEMU Apple ParavirtualizedGraphics.framework device, PCI variant + * + * Copyright © 2023-2024 Phil Dennis-Jordan + * + * SPDX-License-Identifier: GPL-2.0-or-later + * + * ParavirtualizedGraphics.framework is a set of libraries that macOS provides + * which implements 3d graphics passthrough to the host as well as a + * proprietary guest communication channel to drive it. This device model + * implements support to drive that library from within QEMU as a PCI device + * aimed primarily at x86-64 macOS VMs. + */ + +#include "apple-gfx.h" +#include "hw/pci/pci_device.h" +#include "hw/pci/msi.h" +#include "qapi/error.h" +#include "trace.h" +#import + +OBJECT_DECLARE_SIMPLE_TYPE(AppleGFXPCIState, APPLE_GFX_PCI) + +struct AppleGFXPCIState { +PCIDevice parent_obj; + +AppleGFXState common; +}; + +static const char* apple_gfx_pci_option_rom_path = NULL; + +static void apple_gfx_init_option_rom_path(void) +{ +NSURL *option_rom_url = PGCopyOptionROMURL(); +const char *option_rom_path = option_rom_url.fileSystemRepresentation; +apple_gfx_pci_option_rom_path = g_strdup(option_rom_path); +[option_rom_url release]; +} + +static void apple_gfx_pci_init(Object *obj) +{ +AppleGFXPCIState *s = APPLE_GFX_PCI(obj); + +if (!apple_gfx_pci_option_rom_path) { +/* + * The following is done on device not class init to avoid running + * ObjC code before fork() in -daemonize mode. + */ +PCIDeviceClass *pci = PCI_DEVICE_CLASS(object_get_class(obj)); +apple_gfx_init_option_rom_path(); +pci->romfile = apple_gfx_pci_option_rom_path; +} + +apple_gfx_common_init(obj, &s->common, TYPE_APPLE_GFX_PCI); +} + +typedef struct AppleGFXPCIInterruptJob { +PCIDevice *device; +uint32_t vector; +} AppleGFXPCIInterruptJob; + +static void apple_gfx_pci_raise_interrupt(void *opaque) +{ +AppleGFXPCIInterruptJob *job = opaque; + +if (msi_enabled(job->device)) { +msi_notify(job->device, job->vector); +} +g_free(job); +} + +static void apple_gfx_pci_interrupt(PCIDevice *dev, uint32_t vector) +{ +AppleGFXPCIInterruptJob *job; + +trace_apple_gfx_raise_irq(vector); +job = g_malloc0(sizeof(*job)); +job->device = dev; +job->vector = vector; +aio_bh_schedule_oneshot(qemu_get_aio_context(), +apple_gfx_pci_raise_interrupt, job); +} + +static void apple_gfx_pci_realize(PCIDevice *dev, Error **errp) +{ +AppleGFXPCIState *s = APPLE_GFX_PCI(dev); +int ret; + +pci_register_bar(dev, PG_PCI_BAR_MMIO, + PCI_BASE_ADDRESS_SPACE_MEMORY, &s->common.iomem_gfx); + +ret = msi_init(dev, 0x0 /* config offset; 0 = find space */, + PG_PCI_MAX_MSI_VECTORS, true /* msi64bit */, + false /* msi_per_vector_mask */, errp); +if (ret != 0) { +return; +} + +@autoreleasepool { +PGDeviceDescriptor *desc = [PGDeviceDescriptor new]; +desc.raiseInterrupt = ^(uint32_t vector) { +apple_gfx_pci_interrupt(dev, vector); +}; + +apple_gfx_common_realize(&s->common, DEVICE(dev), desc, errp); +[desc release]; +desc = nil; +} +} + +static void apple_gfx_pci_reset(Object *obj, ResetType type) +{ +AppleGFXPCIState *s = APPLE_GFX_PCI(obj); +[s->common.pgdev reset]; +} + +static void apple_gfx_pci_class_init(ObjectClass *klass, void *data) +{ +DeviceClass *dc = DEVICE_CLASS(klass); +PCIDeviceClass *pci = PCI_DEVICE_CLASS(klass); +ResettableClass *rc = RESETTABLE_CLASS(klass); + +rc->phases.hold = apple_gfx_pci_reset; +dc->desc = "m
[PATCH v10 06/15] hw: Add vmapple subdir
From: Alexander Graf We will introduce a number of devices that are specific to the vmapple target machine. To keep them all tidily together, let's put them into a single target directory. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- MAINTAINERS | 7 +++ hw/Kconfig | 1 + hw/meson.build | 1 + hw/vmapple/Kconfig | 1 + hw/vmapple/meson.build | 0 hw/vmapple/trace-events | 2 ++ hw/vmapple/trace.h | 1 + meson.build | 1 + 8 files changed, 14 insertions(+) create mode 100644 hw/vmapple/Kconfig create mode 100644 hw/vmapple/meson.build create mode 100644 hw/vmapple/trace-events create mode 100644 hw/vmapple/trace.h diff --git a/MAINTAINERS b/MAINTAINERS index b28267b6286..c6b41b983bf 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2762,6 +2762,13 @@ F: hw/hyperv/hv-balloon*.h F: include/hw/hyperv/dynmem-proto.h F: include/hw/hyperv/hv-balloon.h +VMapple +M: Alexander Graf +R: Phil Dennis-Jordan +S: Maintained +F: hw/vmapple/* +F: include/hw/vmapple/* + Subsystems -- Overall Audio backends diff --git a/hw/Kconfig b/hw/Kconfig index 1b4e9bb07f7..2871784cfdc 100644 --- a/hw/Kconfig +++ b/hw/Kconfig @@ -41,6 +41,7 @@ source ufs/Kconfig source usb/Kconfig source virtio/Kconfig source vfio/Kconfig +source vmapple/Kconfig source xen/Kconfig source watchdog/Kconfig diff --git a/hw/meson.build b/hw/meson.build index b827c82c5d7..9c4f6d0d636 100644 --- a/hw/meson.build +++ b/hw/meson.build @@ -39,6 +39,7 @@ subdir('ufs') subdir('usb') subdir('vfio') subdir('virtio') +subdir('vmapple') subdir('watchdog') subdir('xen') subdir('xenpv') diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig new file mode 100644 index 000..8b137891791 --- /dev/null +++ b/hw/vmapple/Kconfig @@ -0,0 +1 @@ + diff --git a/hw/vmapple/meson.build b/hw/vmapple/meson.build new file mode 100644 index 000..e69de29bb2d diff --git a/hw/vmapple/trace-events b/hw/vmapple/trace-events new file mode 100644 index 000..9ccc5790487 --- /dev/null +++ b/hw/vmapple/trace-events @@ -0,0 +1,2 @@ +# See docs/devel/tracing.rst for syntax documentation. + diff --git a/hw/vmapple/trace.h b/hw/vmapple/trace.h new file mode 100644 index 000..572adbefe04 --- /dev/null +++ b/hw/vmapple/trace.h @@ -0,0 +1 @@ +#include "trace/trace-hw_vmapple.h" diff --git a/meson.build b/meson.build index 3c61238bc77..a5f76f27c12 100644 --- a/meson.build +++ b/meson.build @@ -3592,6 +3592,7 @@ if have_system 'hw/usb', 'hw/vfio', 'hw/virtio', +'hw/vmapple', 'hw/watchdog', 'hw/xen', 'hw/gpio', -- 2.39.3 (Apple Git-145)
[PATCH v10 10/15] hw/vmapple/aes: Introduce aes engine
From: Alexander Graf VMApple contains an "aes" engine device that it uses to encrypt and decrypt its nvram. It has trivial hard coded keys it uses for that purpose. Add device emulation for this device model. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan --- v3: * Rebased on latest upstream and fixed minor breakages. * Replaced legacy device reset method with Resettable method v4: * Improved logging of unimplemented functions and guest errors. * Better adherence to naming and coding conventions. * Cleaner error handling and recovery, including using g_autoptr v5: * More logging improvements * Use xxx64_overflow() functions for hexdump buffer size calculations. v7: * Coding style tweaks. v8: * Further improved logging of guest errors. v9: * Replaced a use of cpu_physical_memory_write with dma_memory_write. * Dropped unnecessary use of ternary operator for bool -> 0/1. v10: * Code style and comment improvements. hw/vmapple/Kconfig | 2 + hw/vmapple/aes.c | 581 +++ hw/vmapple/meson.build | 1 + hw/vmapple/trace-events | 14 + include/hw/vmapple/vmapple.h | 17 + include/qemu/cutils.h| 15 + util/hexdump.c | 18 ++ 7 files changed, 648 insertions(+) create mode 100644 hw/vmapple/aes.c create mode 100644 include/hw/vmapple/vmapple.h diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig index 8b137891791..a73504d5999 100644 --- a/hw/vmapple/Kconfig +++ b/hw/vmapple/Kconfig @@ -1 +1,3 @@ +config VMAPPLE_AES +bool diff --git a/hw/vmapple/aes.c b/hw/vmapple/aes.c new file mode 100644 index 000..3759dae11ba --- /dev/null +++ b/hw/vmapple/aes.c @@ -0,0 +1,581 @@ +/* + * QEMU Apple AES device emulation + * + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "trace.h" +#include "crypto/hash.h" +#include "crypto/aes.h" +#include "crypto/cipher.h" +#include "hw/irq.h" +#include "hw/sysbus.h" +#include "hw/vmapple/vmapple.h" +#include "migration/vmstate.h" +#include "qemu/cutils.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "sysemu/dma.h" + +OBJECT_DECLARE_SIMPLE_TYPE(AESState, APPLE_AES) + +#define MAX_FIFO_SIZE 9 + +#define CMD_KEY 0x1 +#define CMD_KEY_CONTEXT_SHIFT27 +#define CMD_KEY_CONTEXT_MASK (0x1 << CMD_KEY_CONTEXT_SHIFT) +#define CMD_KEY_SELECT_MAX_IDX 0x7 +#define CMD_KEY_SELECT_SHIFT 24 +#define CMD_KEY_SELECT_MASK (CMD_KEY_SELECT_MAX_IDX << CMD_KEY_SELECT_SHIFT) +#define CMD_KEY_KEY_LEN_NUM 4u +#define CMD_KEY_KEY_LEN_SHIFT22 +#define CMD_KEY_KEY_LEN_MASK ((CMD_KEY_KEY_LEN_NUM - 1u) << CMD_KEY_KEY_LEN_SHIFT) +#define CMD_KEY_ENCRYPT_SHIFT20 +#define CMD_KEY_ENCRYPT_MASK (0x1 << CMD_KEY_ENCRYPT_SHIFT) +#define CMD_KEY_BLOCK_MODE_SHIFT 16 +#define CMD_KEY_BLOCK_MODE_MASK (0x3 << CMD_KEY_BLOCK_MODE_SHIFT) +#define CMD_IV0x2 +#define CMD_IV_CONTEXT_SHIFT 26 +#define CMD_IV_CONTEXT_MASK (0x3 << CMD_KEY_CONTEXT_SHIFT) +#define CMD_DSB 0x3 +#define CMD_SKG 0x4 +#define CMD_DATA 0x5 +#define CMD_DATA_KEY_CTX_SHIFT 27 +#define CMD_DATA_KEY_CTX_MASK(0x1 << CMD_DATA_KEY_CTX_SHIFT) +#define CMD_DATA_IV_CTX_SHIFT25 +#define CMD_DATA_IV_CTX_MASK (0x3 << CMD_DATA_IV_CTX_SHIFT) +#define CMD_DATA_LEN_MASK0xff +#define CMD_STORE_IV 0x6 +#define CMD_STORE_IV_ADDR_MASK 0xff +#define CMD_WRITE_REG 0x7 +#define CMD_FLAG 0x8 +#define CMD_FLAG_STOP_MASK BIT(26) +#define CMD_FLAG_RAISE_IRQ_MASK BIT(27) +#define CMD_FLAG_INFO_MASK 0xff +#define CMD_MAX 0x10 + +#define CMD_SHIFT 28 + +#define REG_STATUS0xc +#define REG_STATUS_DMA_READ_RUNNING BIT(0) +#define REG_STATUS_DMA_READ_PENDING BIT(1) +#define REG_STATUS_DMA_WRITE_RUNNINGBIT(2) +#define REG_STATUS_DMA_WRITE_PENDINGBIT(3) +#define REG_STATUS_BUSY BIT(4) +#define REG_STATUS_EXECUTINGBIT(5) +#define REG_STATUS_READYBIT(6) +#define REG_STATUS_TEXT_DPA_SEEDED BIT(7) +#define REG_STATUS_UNWRAP_DPA_SEEDEDBIT(8) + +#define REG_IRQ_STATUS0x18 +#define REG_IRQ_STATUS_INVALID_CMD BIT(2) +#define REG_IRQ_STATUS_FLAG BIT(5) +#define REG_IRQ_ENABLE0x1c +#define REG_WATERMARK 0x20 +#define REG_Q_STATUS 0x24 +#define REG_FLAG_INFO 0x30 +#define REG_FIFO 0x200 + +static const uint32_t key_lens[CMD_KEY_KEY_LEN_NUM] = { +[0] = 16, +[1] = 24, +[2] = 32, +[3] = 64, +}; + +typedef struct Key { +uint32_t key_len; +uint8_t key[32]; +} Key; + +typedef struct IV { +uint32_t iv[4]; +} IV; + +static Key builtin_keys[CMD_
[PATCH v10 11/15] hw/vmapple/bdif: Introduce vmapple backdoor interface
From: Alexander Graf The VMApple machine exposes AUX and ROOT block devices (as well as USB OTG emulation) via virtio-pci as well as a special, simple backdoor platform device. This patch implements this backdoor platform device to the best of my understanding. I left out any USB OTG parts; they're only needed for guest recovery and I don't understand the protocol yet. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v4: * Moved most header code to .c, rest to vmapple.h * Better compliance with coding, naming, and formatting conventions. v8: * Replaced uses of cpu_physical_memory_read with dma_memory_read. * Replaced an instance of g_free with g_autofree. v9: * Replaced uses of cpu_physical_memory_write with dma_memory_write. hw/vmapple/Kconfig | 3 + hw/vmapple/bdif.c| 275 +++ hw/vmapple/meson.build | 1 + hw/vmapple/trace-events | 5 + include/hw/vmapple/vmapple.h | 2 + 5 files changed, 286 insertions(+) create mode 100644 hw/vmapple/bdif.c diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig index a73504d5999..68f88876eb9 100644 --- a/hw/vmapple/Kconfig +++ b/hw/vmapple/Kconfig @@ -1,3 +1,6 @@ config VMAPPLE_AES bool +config VMAPPLE_BDIF +bool + diff --git a/hw/vmapple/bdif.c b/hw/vmapple/bdif.c new file mode 100644 index 000..d7b6b7a25a4 --- /dev/null +++ b/hw/vmapple/bdif.c @@ -0,0 +1,275 @@ +/* + * VMApple Backdoor Interface + * + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include "qemu/osdep.h" +#include "qemu/units.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "trace.h" +#include "hw/vmapple/vmapple.h" +#include "hw/sysbus.h" +#include "hw/block/block.h" +#include "qapi/error.h" +#include "sysemu/block-backend.h" +#include "sysemu/dma.h" + +OBJECT_DECLARE_SIMPLE_TYPE(VMAppleBdifState, VMAPPLE_BDIF) + +struct VMAppleBdifState { +SysBusDevice parent_obj; + +BlockBackend *aux; +BlockBackend *root; +MemoryRegion mmio; +}; + +#define VMAPPLE_BDIF_SIZE 0x0020 + +#define REG_DEVID_MASK 0x +#define DEVID_ROOT 0x +#define DEVID_AUX 0x0001 +#define DEVID_USB 0x0010 + +#define REG_STATUS 0x0 +#define REG_STATUS_ACTIVE BIT(0) +#define REG_CFG 0x4 +#define REG_CFG_ACTIVEBIT(1) +#define REG_UNK10x8 +#define REG_BUSY0x10 +#define REG_BUSY_READYBIT(0) +#define REG_UNK20x400 +#define REG_CMD 0x408 +#define REG_NEXT_DEVICE 0x420 +#define REG_UNK30x434 + +typedef struct VblkSector { +uint32_t pad; +uint32_t pad2; +uint32_t sector; +uint32_t pad3; +} VblkSector; + +typedef struct VblkReqCmd { +uint64_t addr; +uint32_t len; +uint32_t flags; +} VblkReqCmd; + +typedef struct VblkReq { +VblkReqCmd sector; +VblkReqCmd data; +VblkReqCmd retval; +} VblkReq; + +#define VBLK_DATA_FLAGS_READ 0x00030001 +#define VBLK_DATA_FLAGS_WRITE 0x00010001 + +#define VBLK_RET_SUCCESS 0 +#define VBLK_RET_FAILED 1 + +static uint64_t bdif_read(void *opaque, hwaddr offset, unsigned size) +{ +uint64_t ret = -1; +uint64_t devid = offset & REG_DEVID_MASK; + +switch (offset & ~REG_DEVID_MASK) { +case REG_STATUS: +ret = REG_STATUS_ACTIVE; +break; +case REG_CFG: +ret = REG_CFG_ACTIVE; +break; +case REG_UNK1: +ret = 0x420; +break; +case REG_BUSY: +ret = REG_BUSY_READY; +break; +case REG_UNK2: +ret = 0x1; +break; +case REG_UNK3: +ret = 0x0; +break; +case REG_NEXT_DEVICE: +switch (devid) { +case DEVID_ROOT: +ret = 0x800; +break; +case DEVID_AUX: +ret = 0x1; +break; +} +break; +} + +trace_bdif_read(offset, size, ret); +return ret; +} + +static void le2cpu_sector(VblkSector *sector) +{ +sector->sector = le32_to_cpu(sector->sector); +} + +static void le2cpu_reqcmd(VblkReqCmd *cmd) +{ +cmd->addr = le64_to_cpu(cmd->addr); +cmd->len = le32_to_cpu(cmd->len); +cmd->flags = le32_to_cpu(cmd->flags); +} + +static void le2cpu_req(VblkReq *req) +{ +le2cpu_reqcmd(&req->sector); +le2cpu_reqcmd(&req->data); +le2cpu_reqcmd(&req->retval); +} + +static void vblk_cmd(uint64_t devid, BlockBackend *blk, uint64_t gp_addr, + uint64_t static_off) +{ +VblkReq req; +VblkSector sector; +uint64_t off = 0; +g_autofree char *buf = NULL; +uint8_t ret = VBLK_RET_FAILED; +int r; +MemTxResult dma_result; + +dma_result = dma_memory_read(&address_s
[PATCH v10 01/15] ui & main loop: Redesign of system-specific main thread event handling
macOS's Cocoa event handling must be done on the initial (main) thread of the process. Furthermore, if library or application code uses libdispatch, the main dispatch queue must be handling events on the main thread as well. So far, this has affected Qemu in both the Cocoa and SDL UIs, although in different ways: the Cocoa UI replaces the default qemu_main function with one that spins Qemu's internal main event loop off onto a background thread. SDL (which uses Cocoa internally) on the other hand uses a polling approach within Qemu's main event loop. Events are polled during the SDL UI's dpy_refresh callback, which happens to run on the main thread by default. As UIs are mutually exclusive, this works OK as long as nothing else needs platform-native event handling. In the next patch, a new device is introduced based on the ParavirtualizedGraphics.framework in macOS. This uses libdispatch internally, and only works when events are being handled on the main runloop. With the current system, it works when using either the Cocoa or the SDL UI. However, it does not when running headless. Moreover, any attempt to install a similar scheme to the Cocoa UI's main thread replacement fails when combined with the SDL UI. This change tidies up main thread management to be more flexible. * The qemu_main global function pointer is a custom function for the main thread, and it may now be NULL. When it is, the main thread runs the main Qemu loop. This represents the traditional setup. * When non-null, spawning the main Qemu event loop on a separate thread is now done centrally rather than inside the Cocoa UI code. * For most platforms, qemu_main is indeed NULL by default, but on Darwin, it defaults to a function that runs the CFRunLoop. * The Cocoa UI sets qemu_main to a function which runs the NSApplication event handling runloop, as is usual for a Cocoa app. * The SDL UI overrides the qemu_main function to NULL, thus specifying that Qemu's main loop must run on the main thread. * The GTK UI also overrides the qemu_main function to NULL. * For other UIs, or in the absence of UIs, the platform's default behaviour is followed. This means that on macOS, the platform's runloop events are always handled, regardless of chosen UI. The new PV graphics device will thus work in all configurations. There is no functional change on other operating systems. Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v5: * Simplified the way of setting/clearing the main loop by going back to setting qemu_main directly, but narrowing the scope of what it needs to do, and it can now be NULL. v6: * Folded function qemu_run_default_main_on_new_thread's code into main() * Removed whitespace changes left over on lines near code removed between v4 and v5 v9: * Set qemu_main to NULL for GTK UI as well. v10: * Added comments clarifying the functionality and purpose of qemu_main. include/qemu-main.h | 21 ++-- include/qemu/typedefs.h | 1 + system/main.c | 50 ++ ui/cocoa.m | 54 ++--- ui/gtk.c| 8 ++ ui/sdl2.c | 4 +++ 6 files changed, 90 insertions(+), 48 deletions(-) diff --git a/include/qemu-main.h b/include/qemu-main.h index 940960a7dbc..fc70681c8b5 100644 --- a/include/qemu-main.h +++ b/include/qemu-main.h @@ -5,7 +5,24 @@ #ifndef QEMU_MAIN_H #define QEMU_MAIN_H -int qemu_default_main(void); -extern int (*qemu_main)(void); +/* + * The function to run on the main (initial) thread of the process. + * NULL means QEMU's main event loop. + * When non-NULL, QEMU's main event loop will run on a purposely created + * thread, after which the provided function pointer will be invoked on + * the initial thread. + * This is useful on platforms which treat the main thread as special + * (macOS/Darwin) and/or require all UI API calls to occur from a + * specific thread. + * Implementing this via a global function pointer variable is a bit + * ugly, but it's probably worth investigating the existing UI thread rule + * violations in the SDL (e.g. #2537) and GTK+ back-ends. Fixing those + * issues might precipitate requirements similar but not identical to those + * of the Cocoa UI; hopefully we'll see some kind of pattern emerge, which + * can then be used as a basis for an overhaul. (In fact, it may turn + * out to be simplest to split the UI/native platform event thread from the + * QEMU main event loop on all platforms, with any UI or even none at all.) + */ +extern qemu_main_fn qemu_main; #endif /* QEMU_MAIN_H */ diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 3d84efcac47..b02cfe1f328 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -131,5 +131,6 @@ typedef struct IRQState *qemu_irq; * Function types */ typedef void (*qemu_irq_handler)(void *opaque, int n, int level); +typedef int (
[PATCH v10 12/15] hw/vmapple/cfg: Introduce vmapple cfg region
From: Alexander Graf Instead of device tree or other more standardized means, VMApple passes platform configuration to the first stage boot loader in a binary encoded format that resides at a dedicated RAM region in physical address space. This patch models this configuration space as a qdev device which we can then map at the fixed location in the address space. That way, we can influence and annotate all configuration fields easily. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan --- v3: * Replaced legacy device reset method with Resettable method v4: * Fixed initialisation of default values for properties * Dropped superfluous endianness conversions * Moved most header code to .c, device name #define goes in vmapple.h v5: * Improved error reporting in case of string property buffer overflow. v7: * Changed error messages for overrun of properties with fixed-length strings to be more useful to users than developers. v8: * Consistent parenthesising of macro arguments for better safety. v10: * Slightly tidier error reporting for overlong property values. hw/vmapple/Kconfig | 3 + hw/vmapple/cfg.c | 196 +++ hw/vmapple/meson.build | 1 + include/hw/vmapple/vmapple.h | 2 + 4 files changed, 202 insertions(+) create mode 100644 hw/vmapple/cfg.c diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig index 68f88876eb9..8bbeb9a9237 100644 --- a/hw/vmapple/Kconfig +++ b/hw/vmapple/Kconfig @@ -4,3 +4,6 @@ config VMAPPLE_AES config VMAPPLE_BDIF bool +config VMAPPLE_CFG +bool + diff --git a/hw/vmapple/cfg.c b/hw/vmapple/cfg.c new file mode 100644 index 000..d854b49269b --- /dev/null +++ b/hw/vmapple/cfg.c @@ -0,0 +1,196 @@ +/* + * VMApple Configuration Region + * + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * SPDX-License-Identifier: GPL-2.0-or-later + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ + +#include "qemu/osdep.h" +#include "hw/vmapple/vmapple.h" +#include "hw/sysbus.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "qapi/error.h" +#include "net/net.h" + +OBJECT_DECLARE_SIMPLE_TYPE(VMAppleCfgState, VMAPPLE_CFG) + +#define VMAPPLE_CFG_SIZE 0x0001 + +typedef struct VMAppleCfg { +uint32_t version; /* 0x000 */ +uint32_t nr_cpus; /* 0x004 */ +uint32_t unk1;/* 0x008 */ +uint32_t unk2;/* 0x00c */ +uint32_t unk3;/* 0x010 */ +uint32_t unk4;/* 0x014 */ +uint64_t ecid;/* 0x018 */ +uint64_t ram_size;/* 0x020 */ +uint32_t run_installer1; /* 0x028 */ +uint32_t unk5;/* 0x02c */ +uint32_t unk6;/* 0x030 */ +uint32_t run_installer2; /* 0x034 */ +uint32_t rnd; /* 0x038 */ +uint32_t unk7;/* 0x03c */ +MACAddr mac_en0; /* 0x040 */ +uint8_t pad1[2]; +MACAddr mac_en1; /* 0x048 */ +uint8_t pad2[2]; +MACAddr mac_wifi0;/* 0x050 */ +uint8_t pad3[2]; +MACAddr mac_bt0; /* 0x058 */ +uint8_t pad4[2]; +uint8_t reserved[0xa0]; /* 0x060 */ +uint32_t cpu_ids[0x80]; /* 0x100 */ +uint8_t scratch[0x200]; /* 0x180 */ +char serial[32]; /* 0x380 */ +char unk8[32];/* 0x3a0 */ +char model[32]; /* 0x3c0 */ +uint8_t unk9[32]; /* 0x3e0 */ +uint32_t unk10; /* 0x400 */ +char soc_name[32];/* 0x404 */ +} VMAppleCfg; + +struct VMAppleCfgState { +SysBusDevice parent_obj; +VMAppleCfg cfg; + +MemoryRegion mem; +char *serial; +char *model; +char *soc_name; +}; + +static void vmapple_cfg_reset(Object *obj, ResetType type) +{ +VMAppleCfgState *s = VMAPPLE_CFG(obj); +VMAppleCfg *cfg; + +cfg = memory_region_get_ram_ptr(&s->mem); +memset(cfg, 0, VMAPPLE_CFG_SIZE); +*cfg = s->cfg; +} + +static bool set_fixlen_property_or_error(char *restrict dst, + const char *restrict src, + size_t dst_size, Error **errp, + const char *property_name) +{ +ERRP_GUARD(); +size_t len; + +len = g_strlcpy(dst, src, dst_size); +if (len < dst_size) { /* len does not count nul terminator */ +return true; +} + +error_setg(errp, "Provided value too long for property '%s'", property_name); +error_append_hint(errp, "length (%zu) exceeds maximum of %zu\n", + len, dst_size - 1); +return false; +} + +#define set_fixlen_property_or_return(dst_array, src, errp, property_name) \ +do { \ +if (!set_fixlen_property_or_error((dst_array), (src), \ + ARRAY_SIZE(dst_array), \ +
[PATCH v10 09/15] gpex: Allow more than 4 legacy IRQs
From: Alexander Graf Some boards such as vmapple don't do real legacy PCI IRQ swizzling. Instead, they just keep allocating more board IRQ lines for each new legacy IRQ. Let's support that mode by giving instantiators a new "nr_irqs" property they can use to support more than 4 legacy IRQ lines. In this mode, GPEX will export more IRQ lines, one for each device. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v4: * Turned pair of IRQ arrays into array of structs. * Simplified swizzling logic selection. hw/arm/sbsa-ref.c | 2 +- hw/arm/virt.c | 2 +- hw/i386/microvm.c | 2 +- hw/loongarch/virt.c| 2 +- hw/mips/loongson3_virt.c | 2 +- hw/openrisc/virt.c | 12 +-- hw/pci-host/gpex.c | 43 ++ hw/riscv/virt.c| 12 +-- hw/xtensa/virt.c | 2 +- include/hw/pci-host/gpex.h | 7 +++ 10 files changed, 55 insertions(+), 31 deletions(-) diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index e3195d54497..7e7322486c2 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -673,7 +673,7 @@ static void create_pcie(SBSAMachineState *sms) /* Map IO port space */ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio); -for (i = 0; i < GPEX_NUM_IRQS; i++) { +for (i = 0; i < PCI_NUM_PINS; i++) { sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, qdev_get_gpio_in(sms->gic, irq + i)); gpex_set_irq_num(GPEX_HOST(dev), i, irq + i); diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 1a381e9a2bd..8aa22ea3155 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -1547,7 +1547,7 @@ static void create_pcie(VirtMachineState *vms) /* Map IO port space */ sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, base_pio); -for (i = 0; i < GPEX_NUM_IRQS; i++) { +for (i = 0; i < PCI_NUM_PINS; i++) { sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, qdev_get_gpio_in(vms->gic, irq + i)); gpex_set_irq_num(GPEX_HOST(dev), i, irq + i); diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c index 86637afa0f3..ce80596c239 100644 --- a/hw/i386/microvm.c +++ b/hw/i386/microvm.c @@ -139,7 +139,7 @@ static void create_gpex(MicrovmMachineState *mms) mms->gpex.mmio64.base, mmio64_alias); } -for (i = 0; i < GPEX_NUM_IRQS; i++) { +for (i = 0; i < PCI_NUM_PINS; i++) { sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, x86ms->gsi[mms->gpex.irq + i]); } diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index 9a635d1d3d3..50056384994 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -741,7 +741,7 @@ static void virt_devices_init(DeviceState *pch_pic, memory_region_add_subregion(get_system_memory(), VIRT_PCI_IO_BASE, pio_alias); -for (i = 0; i < GPEX_NUM_IRQS; i++) { +for (i = 0; i < PCI_NUM_PINS; i++) { sysbus_connect_irq(d, i, qdev_get_gpio_in(pch_pic, 16 + i)); gpex_set_irq_num(GPEX_HOST(gpex_dev), i, 16 + i); diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c index f3b6326cc59..884b5f23a99 100644 --- a/hw/mips/loongson3_virt.c +++ b/hw/mips/loongson3_virt.c @@ -458,7 +458,7 @@ static inline void loongson3_virt_devices_init(MachineState *machine, virt_memmap[VIRT_PCIE_PIO].base, s->pio_alias); sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, virt_memmap[VIRT_PCIE_PIO].base); -for (i = 0; i < GPEX_NUM_IRQS; i++) { +for (i = 0; i < PCI_NUM_PINS; i++) { irq = qdev_get_gpio_in(pic, PCIE_IRQ_BASE + i); sysbus_connect_irq(SYS_BUS_DEVICE(dev), i, irq); gpex_set_irq_num(GPEX_HOST(dev), i, PCIE_IRQ_BASE + i); diff --git a/hw/openrisc/virt.c b/hw/openrisc/virt.c index 47d2c9bd3c7..6f053bf48e0 100644 --- a/hw/openrisc/virt.c +++ b/hw/openrisc/virt.c @@ -318,7 +318,7 @@ static void create_pcie_irq_map(void *fdt, char *nodename, int irq_base, { int pin, dev; uint32_t irq_map_stride = 0; -uint32_t full_irq_map[GPEX_NUM_IRQS * GPEX_NUM_IRQS * 6] = {}; +uint32_t full_irq_map[PCI_NUM_PINS * PCI_NUM_PINS * 6] = {}; uint32_t *irq_map = full_irq_map; /* @@ -330,11 +330,11 @@ static void create_pcie_irq_map(void *fdt, char *nodename, int irq_base, * possible slot) seeing the interrupt-map-mask will allow the table * to wrap to any number of devices. */ -for (dev = 0; dev < GPEX_NUM_IRQS; dev++) { +for (dev = 0; dev < PCI_NUM_PINS; dev++) { int devfn = dev << 3; -for (pin = 0; pin < GPEX_NUM_IRQS; pin++) { -int irq_nr = irq_base + ((pin + PCI_SLOT(devfn)) % GPEX_NUM_IRQS); +for (pin = 0; pin < PCI_NUM_PINS; pin++) { +int irq_nr = irq_base + ((pin + PCI_SLOT(devfn)) % PCI_NUM_PINS); int i = 0;
[PATCH v10 08/15] hvf: arm: Ignore writes to CNTP_CTL_EL0
From: Alexander Graf MacOS unconditionally disables interrupts of the physical timer on boot and then continues to use the virtual one. We don't really want to support a full physical timer emulation, so let's just ignore those writes. Signed-off-by: Alexander Graf Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- target/arm/hvf/hvf.c | 9 + 1 file changed, 9 insertions(+) diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 6cea483d422..b45b764dfd0 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -11,6 +11,7 @@ #include "qemu/osdep.h" #include "qemu/error-report.h" +#include "qemu/log.h" #include "sysemu/runstate.h" #include "sysemu/hvf.h" @@ -184,6 +185,7 @@ void hvf_arm_init_debug(void) #define SYSREG_OSLSR_EL1 SYSREG(2, 0, 1, 1, 4) #define SYSREG_OSDLR_EL1 SYSREG(2, 0, 1, 3, 4) #define SYSREG_CNTPCT_EL0 SYSREG(3, 3, 14, 0, 1) +#define SYSREG_CNTP_CTL_EL0 SYSREG(3, 3, 14, 2, 1) #define SYSREG_PMCR_EL0 SYSREG(3, 3, 9, 12, 0) #define SYSREG_PMUSERENR_EL0 SYSREG(3, 3, 9, 14, 0) #define SYSREG_PMCNTENSET_EL0 SYSREG(3, 3, 9, 12, 1) @@ -1620,6 +1622,13 @@ static int hvf_sysreg_write(CPUState *cpu, uint32_t reg, uint64_t val) case SYSREG_OSLAR_EL1: env->cp15.oslsr_el1 = val & 1; return 0; +case SYSREG_CNTP_CTL_EL0: +/* + * Guests should not rely on the physical counter, but macOS emits + * disable writes to it. Let it do so, but ignore the requests. + */ +qemu_log_mask(LOG_UNIMP, "Unsupported write to CNTP_CTL_EL0\n"); +return 0; case SYSREG_OSDLR_EL1: /* Dummy register */ return 0; -- 2.39.3 (Apple Git-145)
[PATCH v10 05/15] MAINTAINERS: Add myself as maintainer for apple-gfx, reviewer for HVF
I'm happy to take responsibility for the macOS PV graphics code. As HVF patches don't seem to get much attention at the moment, I'm also adding myself as designated reviewer for HVF and x86 HVF to try and improve that. I anticipate that the resulting workload should be covered by the funding I'm receiving for improving Qemu in combination with macOS. As of right now this runs out at the end of 2024; I expect the workload on apple-gfx should be relatively minor and manageable in my spare time beyond that. I may have to remove myself from more general HVF duties once the contract runs out if it's more than I can manage. Signed-off-by: Phil Dennis-Jordan Reviewed-by: Roman Bolshakov --- MAINTAINERS | 7 +++ 1 file changed, 7 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 095420f8b0e..b28267b6286 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -507,6 +507,7 @@ F: target/arm/hvf/ X86 HVF CPUs M: Cameron Esfahani M: Roman Bolshakov +R: Phil Dennis-Jordan W: https://wiki.qemu.org/Features/HVF S: Maintained F: target/i386/hvf/ @@ -514,6 +515,7 @@ F: target/i386/hvf/ HVF M: Cameron Esfahani M: Roman Bolshakov +R: Phil Dennis-Jordan W: https://wiki.qemu.org/Features/HVF S: Maintained F: accel/hvf/ @@ -2609,6 +2611,11 @@ F: hw/display/edid* F: include/hw/display/edid.h F: qemu-edid.c +macOS PV Graphics (apple-gfx) +M: Phil Dennis-Jordan +S: Maintained +F: hw/display/apple-gfx* + PIIX4 South Bridge (i82371AB) M: Hervé Poussineau M: Philippe Mathieu-Daudé -- 2.39.3 (Apple Git-145)
[PATCH v10 15/15] hw/vmapple/vmapple: Add vmapple machine type
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. v8: * Use object_property_add_uint64_ptr rather than defining custom UUID property get/set functions. v9: * Documentation improvements * Fixed variable name and struct field used during pvpanic device creation. v10: * Documentation fixup for changed virtio-blk device type. * Small improvements to shell commands in documentation. * Improved propagation of errors during cfg device instantiation. MAINTAINERS | 1 + contrib/vmapple/uuid.sh | 9 + docs/system/arm/vmapple.rst | 63 docs/system/target-arm.rst | 1 + hw/vmapple/Kconfig | 20 ++ hw/vmapple/meson.build | 1 + hw/vmapple/vmapple.c| 646 7 files changed, 741 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 c6b41b983bf..031754b1e13 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2768,6 +2768,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..5d59890c291 --- /dev/null +++ b/docs/system/arm/vmapple.rst @@ -0,0 +1,63 @@ +VMApple machine emulation + + +VMApple is the device model that the macOS built-in hypervisor called "Virtualization.framework" +exposes to Apple Silicon macOS guests. The "vmapple" machine model in QEMU implements the same +device model, but does not use any code from Virtualization.Framework. + +Prer
[PATCH v10 04/15] hw/display/apple-gfx: Adds configurable mode list
This change adds a property 'display_modes' on the graphics device which permits specifying a list of display modes. (screen resolution and refresh rate) The property is an array of a custom type to make the syntax slightly less awkward to use, for example: -device '{"driver":"apple-gfx-pci", "display-modes":["1920x1080@60", "3840x2160@60"]}' Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v4: * Switched to the native array property type, which recently gained command line support. * The property has also been added to the -mmio variant. * Tidied up the code a little. v5: * Better error handling and buffer management in property parsing and output. v6: * Switched to using NSMutableArray for the mode list to avoid need for allocating a temporary array - previously done with alloca. v7: * Simplified error handling in property parsing v8: * More consistent integer variable types. v9: * Re-ordered type definitions so we can drop a 'struct' keyword. hw/display/apple-gfx-mmio.m | 8 +++ hw/display/apple-gfx-pci.m | 9 ++- hw/display/apple-gfx.h | 11 +++ hw/display/apple-gfx.m | 135 +++- hw/display/trace-events | 2 + 5 files changed, 145 insertions(+), 20 deletions(-) diff --git a/hw/display/apple-gfx-mmio.m b/hw/display/apple-gfx-mmio.m index 2d76e7375bd..99935a155c7 100644 --- a/hw/display/apple-gfx-mmio.m +++ b/hw/display/apple-gfx-mmio.m @@ -258,6 +258,12 @@ static void apple_gfx_mmio_reset(Object *obj, ResetType type) [s->common.pgdev reset]; } +static Property apple_gfx_mmio_properties[] = { +DEFINE_PROP_ARRAY("display-modes", AppleGFXMMIOState, + common.num_display_modes, common.display_modes, + qdev_prop_display_mode, AppleGFXDisplayMode), +DEFINE_PROP_END_OF_LIST(), +}; static void apple_gfx_mmio_class_init(ObjectClass *klass, void *data) { @@ -267,6 +273,8 @@ static void apple_gfx_mmio_class_init(ObjectClass *klass, void *data) rc->phases.hold = apple_gfx_mmio_reset; dc->hotpluggable = false; dc->realize = apple_gfx_mmio_realize; + +device_class_set_props(dc, apple_gfx_mmio_properties); } static TypeInfo apple_gfx_mmio_types[] = { diff --git a/hw/display/apple-gfx-pci.m b/hw/display/apple-gfx-pci.m index 5a27b95ac79..765b210287d 100644 --- a/hw/display/apple-gfx-pci.m +++ b/hw/display/apple-gfx-pci.m @@ -114,6 +114,13 @@ static void apple_gfx_pci_reset(Object *obj, ResetType type) [s->common.pgdev reset]; } +static Property apple_gfx_pci_properties[] = { +DEFINE_PROP_ARRAY("display-modes", AppleGFXPCIState, + common.num_display_modes, common.display_modes, + qdev_prop_display_mode, AppleGFXDisplayMode), +DEFINE_PROP_END_OF_LIST(), +}; + static void apple_gfx_pci_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); @@ -130,7 +137,7 @@ static void apple_gfx_pci_class_init(ObjectClass *klass, void *data) pci->class_id = PCI_CLASS_DISPLAY_OTHER; pci->realize = apple_gfx_pci_realize; -// TODO: Property for setting mode list +device_class_set_props(dc, apple_gfx_pci_properties); } static TypeInfo apple_gfx_pci_types[] = { diff --git a/hw/display/apple-gfx.h b/hw/display/apple-gfx.h index 220276b7492..a34c53cda34 100644 --- a/hw/display/apple-gfx.h +++ b/hw/display/apple-gfx.h @@ -16,6 +16,7 @@ #import #include "qemu/typedefs.h" #include "exec/memory.h" +#include "hw/qdev-properties.h" #include "ui/surface.h" @class PGDeviceDescriptor; @@ -27,6 +28,12 @@ typedef QTAILQ_HEAD(, PGTask_s) PGTaskList; +typedef struct AppleGFXDisplayMode { +uint16_t width_px; +uint16_t height_px; +uint16_t refresh_rate_hz; +} AppleGFXDisplayMode; + typedef struct AppleGFXState { /* Initialised on init/realize() */ MemoryRegion iomem_gfx; @@ -36,6 +43,8 @@ typedef struct AppleGFXState { id mtl; id mtl_queue; dispatch_queue_t render_queue; +AppleGFXDisplayMode *display_modes; +uint32_t num_display_modes; /* List `tasks` is protected by task_mutex */ QemuMutex task_mutex; @@ -63,5 +72,7 @@ void *apple_gfx_host_ptr_for_gpa_range(uint64_t guest_physical, uint64_t length, bool read_only, MemoryRegion **mapping_in_region); +extern const PropertyInfo qdev_prop_display_mode; + #endif diff --git a/hw/display/apple-gfx.m b/hw/display/apple-gfx.m index b9f92eb6656..6aaef0452bf 100644 --- a/hw/display/apple-gfx.m +++ b/hw/display/apple-gfx.m @@ -31,9 +31,10 @@ #include "sysemu/dma.h" #include "ui/console.h" -static const PGDisplayCoord_t apple_gfx_modes[] = { -{ .x = 1440, .y = 1080 }, -{ .x = 1280, .y = 1024 }, +static const AppleGFXDisplayMode apple_gfx_default_modes[] = { +{ 1920, 1080, 60 }, +{ 1440, 1080, 60 }, +{ 1280, 1024, 60 }, }; /*
[PATCH v10 14/15] hw/block/virtio-blk: Replaces request free function with g_free
The virtio_blk_free_request() function has been a 1-liner forwarding to g_free() for a while now. We may as well call g_free on the request pointer directly. Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- hw/block/virtio-blk.c | 43 +++--- hw/vmapple/virtio-blk.c| 2 +- include/hw/virtio/virtio-blk.h | 1 - 3 files changed, 20 insertions(+), 26 deletions(-) diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index 9e8337bb639..40d2c9bc591 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -50,11 +50,6 @@ static void virtio_blk_init_request(VirtIOBlock *s, VirtQueue *vq, req->mr_next = NULL; } -void virtio_blk_free_request(VirtIOBlockReq *req) -{ -g_free(req); -} - void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) { VirtIOBlock *s = req->dev; @@ -93,7 +88,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, if (acct_failed) { block_acct_failed(blk_get_stats(s->blk), &req->acct); } -virtio_blk_free_request(req); +g_free(req); } blk_error_action(s->blk, action, is_read, error); @@ -136,7 +131,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); block_acct_done(blk_get_stats(s->blk), &req->acct); -virtio_blk_free_request(req); +g_free(req); } } @@ -151,7 +146,7 @@ static void virtio_blk_flush_complete(void *opaque, int ret) virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); block_acct_done(blk_get_stats(s->blk), &req->acct); -virtio_blk_free_request(req); +g_free(req); } static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret) @@ -169,7 +164,7 @@ static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret) if (is_write_zeroes) { block_acct_done(blk_get_stats(s->blk), &req->acct); } -virtio_blk_free_request(req); +g_free(req); } static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock *s, VirtQueue *vq) @@ -214,7 +209,7 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req) fail: virtio_blk_req_complete(req, status); -virtio_blk_free_request(req); +g_free(req); } static inline void submit_requests(VirtIOBlock *s, MultiReqBuffer *mrb, @@ -612,7 +607,7 @@ static void virtio_blk_zone_report_complete(void *opaque, int ret) out: virtio_blk_req_complete(req, err_status); -virtio_blk_free_request(req); +g_free(req); g_free(data->zone_report_data.zones); g_free(data); } @@ -661,7 +656,7 @@ static void virtio_blk_handle_zone_report(VirtIOBlockReq *req, return; out: virtio_blk_req_complete(req, err_status); -virtio_blk_free_request(req); +g_free(req); } static void virtio_blk_zone_mgmt_complete(void *opaque, int ret) @@ -677,7 +672,7 @@ static void virtio_blk_zone_mgmt_complete(void *opaque, int ret) } virtio_blk_req_complete(req, err_status); -virtio_blk_free_request(req); +g_free(req); } static int virtio_blk_handle_zone_mgmt(VirtIOBlockReq *req, BlockZoneOp op) @@ -719,7 +714,7 @@ static int virtio_blk_handle_zone_mgmt(VirtIOBlockReq *req, BlockZoneOp op) return 0; out: virtio_blk_req_complete(req, err_status); -virtio_blk_free_request(req); +g_free(req); return err_status; } @@ -750,7 +745,7 @@ static void virtio_blk_zone_append_complete(void *opaque, int ret) out: virtio_blk_req_complete(req, err_status); -virtio_blk_free_request(req); +g_free(req); g_free(data); } @@ -788,7 +783,7 @@ static int virtio_blk_handle_zone_append(VirtIOBlockReq *req, out: virtio_blk_req_complete(req, err_status); -virtio_blk_free_request(req); +g_free(req); return err_status; } @@ -855,7 +850,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); block_acct_invalid(blk_get_stats(s->blk), is_write ? BLOCK_ACCT_WRITE : BLOCK_ACCT_READ); -virtio_blk_free_request(req); +g_free(req); return 0; } @@ -911,7 +906,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) VIRTIO_BLK_ID_BYTES)); iov_from_buf(in_iov, in_num, 0, serial, size); virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); -virtio_blk_free_request(req); +g_free(req); break; } case VIRTIO_BLK_T_ZONE_APPEND & ~VIRTIO_BLK_T_OUT: @@ -943,7 +938,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, MultiReqBuffer *mrb) if (unlikely(!(type & VIRTIO_BLK_T_OUT) || out_len > sizeof(dwz_hdr))) { virtio_blk_req_complete(req, VIRTIO_BLK_S_UNSUPP); -virtio_blk_free_req
Re: [PATCH v8 12/15] hw/vmapple/cfg: Introduce vmapple cfg region
On Mon, 11 Nov 2024 at 05:21, Akihiko Odaki wrote: > On 2024/11/11 0:01, Phil Dennis-Jordan wrote: > > On Sun, 10 Nov 2024 at 08:15, Akihiko Odaki > wrote: > >> > >> On 2024/11/08 23:47, Phil Dennis-Jordan wrote: > >>> From: Alexander Graf > >>> > >>> Instead of device tree or other more standardized means, VMApple passes > >>> platform configuration to the first stage boot loader in a binary > encoded > >>> format that resides at a dedicated RAM region in physical address > space. > >>> > >>> This patch models this configuration space as a qdev device which we > can > >>> then map at the fixed location in the address space. That way, we can > >>> influence and annotate all configuration fields easily. > >>> > >>> Signed-off-by: Alexander Graf > >>> Signed-off-by: Phil Dennis-Jordan > >>> --- > >>> > >>> v3: > >>> > >>>* Replaced legacy device reset method with Resettable method > >>> > >>> v4: > >>> > >>>* Fixed initialisation of default values for properties > >>>* Dropped superfluous endianness conversions > >>>* Moved most header code to .c, device name #define goes in > vmapple.h > >>> > >>> v5: > >>> > >>>* Improved error reporting in case of string property buffer > overflow. > >>> > >>> v7: > >>> > >>>* Changed error messages for overrun of properties with > >>> fixed-length strings to be more useful to users than developers. > >>> > >>> v8: > >>> > >>>* Consistent parenthesising of macro arguments for better safety. > >>> > >>>hw/vmapple/Kconfig | 3 + > >>>hw/vmapple/cfg.c | 196 > +++ > >>>hw/vmapple/meson.build | 1 + > >>>include/hw/vmapple/vmapple.h | 2 + > >>>4 files changed, 202 insertions(+) > >>>create mode 100644 hw/vmapple/cfg.c > >>> > >>> diff --git a/hw/vmapple/Kconfig b/hw/vmapple/Kconfig > >>> index 68f88876eb9..8bbeb9a9237 100644 > >>> --- a/hw/vmapple/Kconfig > >>> +++ b/hw/vmapple/Kconfig > >>> @@ -4,3 +4,6 @@ config VMAPPLE_AES > >>>config VMAPPLE_BDIF > >>>bool > >>> > >>> +config VMAPPLE_CFG > >>> +bool > >>> + > >>> diff --git a/hw/vmapple/cfg.c b/hw/vmapple/cfg.c > >>> new file mode 100644 > >>> index 000..787e2505d57 > >>> --- /dev/null > >>> +++ b/hw/vmapple/cfg.c > >>> @@ -0,0 +1,196 @@ > >>> +/* > >>> + * VMApple Configuration Region > >>> + * > >>> + * Copyright © 2023 Amazon.com, Inc. or its affiliates. All Rights > Reserved. > >>> + * > >>> + * SPDX-License-Identifier: GPL-2.0-or-later > >>> + * > >>> + * This work is licensed under the terms of the GNU GPL, version 2 or > later. > >>> + * See the COPYING file in the top-level directory. > >>> + */ > >>> + > >>> +#include "qemu/osdep.h" > >>> +#include "hw/vmapple/vmapple.h" > >>> +#include "hw/sysbus.h" > >>> +#include "qemu/log.h" > >>> +#include "qemu/module.h" > >>> +#include "qapi/error.h" > >>> +#include "net/net.h" > >>> + > >>> +OBJECT_DECLARE_SIMPLE_TYPE(VMAppleCfgState, VMAPPLE_CFG) > >>> + > >>> +#define VMAPPLE_CFG_SIZE 0x0001 > >>> + > >>> +typedef struct VMAppleCfg { > >>> +uint32_t version; /* 0x000 */ > >>> +uint32_t nr_cpus; /* 0x004 */ > >>> +uint32_t unk1;/* 0x008 */ > >>> +uint32_t unk2;/* 0x00c */ > >>> +uint32_t unk3;/* 0x010 */ > >>> +uint32_t unk4;/* 0x014 */ > >>> +uint64_t ecid;/* 0x018 */ > >>> +uint64_t ram_size;/* 0x020 */ > >>> +uint32_t run_installer1; /* 0x028 */ > >>> +uint32_t unk5;/* 0x02c */ > >>> +uint32_t unk6;/* 0x030 */ > >>> +uint32_t run_installer2; /* 0x034 */ > >>> +uint32_t rnd; /* 0x038 */ > >>> +uint32_t unk7;/* 0x03c */ > >>> +MACAddr mac_en0; /* 0x040 */ > >>> +uint8_t pad1[2]; > >>> +MACAddr mac_en1; /* 0x048 */ > >>> +uint8_t pad2[2]; > >>> +MACAddr mac_wifi0;/* 0x050 */ > >>> +uint8_t pad3[2]; > >>> +MACAddr mac_bt0; /* 0x058 */ > >>> +uint8_t pad4[2]; > >>> +uint8_t reserved[0xa0]; /* 0x060 */ > >>> +uint32_t cpu_ids[0x80]; /* 0x100 */ > >>> +uint8_t scratch[0x200]; /* 0x180 */ > >>> +char serial[32]; /* 0x380 */ > >>> +char unk8[32];/* 0x3a0 */ > >>> +char model[32]; /* 0x3c0 */ > >>> +uint8_t unk9[32]; /* 0x3e0 */ > >>> +uint32_t unk10; /* 0x400 */ > >>> +char soc_name[32];/* 0x404 */ > >>> +} VMAppleCfg; > >>> + > >>> +struct VMAppleCfgState { > >>> +SysBusDevice parent_obj; > >>> +VMAppleCfg cfg; > >>> + > >>> +MemoryRegion mem; > >>> +char *serial; > >>> +char *model; > >>> +char *soc_name; > >>> +}; > >>> + > >>> +static void vmapple_cfg_reset(Object *obj, ResetType type) > >>> +{ > >>> +VMAppleCfgState *s = VMAPPLE_CFG(obj); > >>> +VMAppleCfg *cfg; > >>> + > >>> +cfg = memory_region_get_ram_ptr(&s->mem); > >>> +memset(
Re: [PATCH v10 01/15] ui & main loop: Redesign of system-specific main thread event handling
On 11/13/24 15:23, Phil Dennis-Jordan wrote: macOS's Cocoa event handling must be done on the initial (main) thread of the process. Furthermore, if library or application code uses libdispatch, the main dispatch queue must be handling events on the main thread as well. So far, this has affected Qemu in both the Cocoa and SDL UIs, although in different ways: the Cocoa UI replaces the default qemu_main function with one that spins Qemu's internal main event loop off onto a background thread. SDL (which uses Cocoa internally) on the other hand uses a polling approach within Qemu's main event loop. Events are polled during the SDL UI's dpy_refresh callback, which happens to run on the main thread by default. As UIs are mutually exclusive, this works OK as long as nothing else needs platform-native event handling. In the next patch, a new device is introduced based on the ParavirtualizedGraphics.framework in macOS. This uses libdispatch internally, and only works when events are being handled on the main runloop. With the current system, it works when using either the Cocoa or the SDL UI. However, it does not when running headless. Moreover, any attempt to install a similar scheme to the Cocoa UI's main thread replacement fails when combined with the SDL UI. This change tidies up main thread management to be more flexible. * The qemu_main global function pointer is a custom function for the main thread, and it may now be NULL. When it is, the main thread runs the main Qemu loop. This represents the traditional setup. * When non-null, spawning the main Qemu event loop on a separate thread is now done centrally rather than inside the Cocoa UI code. * For most platforms, qemu_main is indeed NULL by default, but on Darwin, it defaults to a function that runs the CFRunLoop. * The Cocoa UI sets qemu_main to a function which runs the NSApplication event handling runloop, as is usual for a Cocoa app. * The SDL UI overrides the qemu_main function to NULL, thus specifying that Qemu's main loop must run on the main thread. * The GTK UI also overrides the qemu_main function to NULL. * For other UIs, or in the absence of UIs, the platform's default behaviour is followed. This means that on macOS, the platform's runloop events are always handled, regardless of chosen UI. The new PV graphics device will thus work in all configurations. There is no functional change on other operating systems. Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki I checked what GTK+ does and, either way, you have to create another thread: timers are handled with a CFRunLoopTimer, but file descriptors are polled in a separate thread and sent to the main thread with a single CFRunLoopSource. It's a bit nicer that the main thread is in charge, but it's more complex and probably slower too. As long as it's clear that any handlers that go through the CFRunLoop run outside the BQL, as is already the case for the Cocoa UI, I see no problem with this approach. Acked-by: Paolo Bonzini Paolo
Re: [PATCH v9 02/16] hw/display/apple-gfx: Introduce ParavirtualizedGraphics.Framework support
On Mon, 11 Nov 2024 at 05:50, Akihiko Odaki wrote: > On 2024/11/11 6:55, Phil Dennis-Jordan wrote: > > MacOS provides a framework (library) that allows any vmm to implement a > > paravirtualized 3d graphics passthrough to the host metal stack called > > ParavirtualizedGraphics.Framework (PVG). The library abstracts away > > almost every aspect of the paravirtualized device model and only provides > > and receives callbacks on MMIO access as well as to share memory address > > space between the VM and PVG. > > > > This patch implements a QEMU device that drives PVG for the VMApple > > variant of it. > > > > Signed-off-by: Alexander Graf > > Co-authored-by: Alexander Graf > > > > Subsequent changes: > > > > * Cherry-pick/rebase conflict fixes, API use updates. > > * Moved from hw/vmapple/ (useful outside that machine type) > > * Overhaul of threading model, many thread safety improvements. > > * Asynchronous rendering. > > * Memory and object lifetime fixes. > > * Refactoring to split generic and (vmapple) MMIO variant specific > > code. > > > > Implementation wise, most of the complexity lies in the differing > threading > > models of ParavirtualizedGraphics.framework, which uses libdispatch and > > internal locks, versus QEMU, which heavily uses the BQL, especially > during > > memory-mapped device I/O. Great care has therefore been taken to prevent > > deadlocks by never calling into PVG methods while holding the BQL, and > > similarly never acquiring the BQL in a callback from PVG. Different > strategies > > have been used (libdispatch, blocking and non-blocking BHs, RCU, etc.) > > depending on the specific requirements at each framework entry and exit > point. > > > > Signed-off-by: Phil Dennis-Jordan > > --- > > > > v2: > > > > * Cherry-pick/rebase conflict fixes > > * BQL function renaming > > * Moved from hw/vmapple/ (useful outside that machine type) > > * Code review comments: Switched to DEFINE_TYPES macro & little endian > > MMIO. > > * Removed some dead/superfluous code > > * Mad set_mode thread & memory safe > > * Added migration blocker due to lack of (de-)serialisation. > > * Fixes to ObjC refcounting and autorelease pool usage. > > * Fixed ObjC new/init misuse > > * Switched to ObjC category extension for private property. > > * Simplified task memory mapping and made it thread safe. > > * Refactoring to split generic and vmapple MMIO variant specific > > code. > > * Switched to asynchronous MMIO writes on x86-64 > > * Rendering and graphics update are now done asynchronously > > * Fixed cursor handling > > * Coding convention fixes > > * Removed software cursor compositing > > > > v3: > > > > * Rebased on latest upstream, fixed breakages including switching to > Resettable methods. > > * Squashed patches dealing with dGPUs, MMIO area size, and GPU picking. > > * Allow re-entrant MMIO; this simplifies the code and solves the > divergence > > between x86-64 and arm64 variants. > > > > v4: > > > > * Renamed '-vmapple' device variant to '-mmio' > > * MMIO device type now requires aarch64 host and guest > > * Complete overhaul of the glue code for making Qemu's and > > ParavirtualizedGraphics.framework's threading and synchronisation > models > > work together. Calls into PVG are from dispatch queues while the > > BQL-holding initiating thread processes AIO context events; > callbacks from > > PVG are scheduled as BHs on the BQL/main AIO context, awaiting > completion > > where necessary. > > * Guest frame rendering state is covered by the BQL, with only the PVG > calls > > outside the lock, and serialised on the named render_queue. > > * Simplified logic for dropping frames in-flight during mode changes, > fixed > > bug in pending frames logic. > > * Addressed smaller code review notes such as: function naming, object > type > > declarations, type names/declarations/casts, code formatting, > #include > > order, over-cautious ObjC retain/release, what goes in init vs > realize, > > etc. > > > > v5: > > > > * Smaller non-functional fixes in response to review comments, such as > using > > NULL for the AIO_WAIT_WHILE context argument, type name formatting, > > deleting leftover debug code, logging improvements, state struct > field > > order and documentation improvements, etc. > > * Instead of a single condition variable for all synchronous BH job > types, > > there is now one for each callback block. This reduces the number > > of threads being awoken unnecessarily to near zero. > > * MMIO device variant: Unified the BH job for raising interrupts. > > * Use DMA APIs for PVG framework's guest memory read requests. > > * Thread safety improvements: ensure mutable AppleGFXState fields are > not > > accessed outside the appropriate lock. Added dedicated mutex for the > task > > list. > > * Retain references to MemoryRegions for which the
Re: [PATCH v10 01/15] ui & main loop: Redesign of system-specific main thread event handling
On Wed, 13 Nov 2024, Phil Dennis-Jordan wrote: macOS's Cocoa event handling must be done on the initial (main) thread of the process. Furthermore, if library or application code uses libdispatch, the main dispatch queue must be handling events on the main thread as well. So far, this has affected Qemu in both the Cocoa and SDL UIs, although in different ways: the Cocoa UI replaces the default qemu_main function with one that spins Qemu's internal main event loop off onto a background thread. SDL (which uses Cocoa internally) on the other hand uses a polling approach within Qemu's main event loop. Events are polled during the SDL UI's dpy_refresh callback, which happens to run on the main thread by default. As UIs are mutually exclusive, this works OK as long as nothing else needs platform-native event handling. In the next patch, a new device is introduced based on the ParavirtualizedGraphics.framework in macOS. This uses libdispatch internally, and only works when events are being handled on the main runloop. With the current system, it works when using either the Cocoa or the SDL UI. However, it does not when running headless. Moreover, any attempt to install a similar scheme to the Cocoa UI's main thread replacement fails when combined with the SDL UI. This change tidies up main thread management to be more flexible. * The qemu_main global function pointer is a custom function for the main thread, and it may now be NULL. When it is, the main thread runs the main Qemu loop. This represents the traditional setup. * When non-null, spawning the main Qemu event loop on a separate thread is now done centrally rather than inside the Cocoa UI code. * For most platforms, qemu_main is indeed NULL by default, but on Darwin, it defaults to a function that runs the CFRunLoop. * The Cocoa UI sets qemu_main to a function which runs the NSApplication event handling runloop, as is usual for a Cocoa app. * The SDL UI overrides the qemu_main function to NULL, thus specifying that Qemu's main loop must run on the main thread. * The GTK UI also overrides the qemu_main function to NULL. * For other UIs, or in the absence of UIs, the platform's default behaviour is followed. This means that on macOS, the platform's runloop events are always handled, regardless of chosen UI. The new PV graphics device will thus work in all configurations. There is no functional change on other operating systems. Signed-off-by: Phil Dennis-Jordan Reviewed-by: Akihiko Odaki --- v5: * Simplified the way of setting/clearing the main loop by going back to setting qemu_main directly, but narrowing the scope of what it needs to do, and it can now be NULL. v6: * Folded function qemu_run_default_main_on_new_thread's code into main() * Removed whitespace changes left over on lines near code removed between v4 and v5 v9: * Set qemu_main to NULL for GTK UI as well. v10: * Added comments clarifying the functionality and purpose of qemu_main. include/qemu-main.h | 21 ++-- include/qemu/typedefs.h | 1 + system/main.c | 50 ++ ui/cocoa.m | 54 ++--- ui/gtk.c| 8 ++ ui/sdl2.c | 4 +++ 6 files changed, 90 insertions(+), 48 deletions(-) diff --git a/include/qemu-main.h b/include/qemu-main.h index 940960a7dbc..fc70681c8b5 100644 --- a/include/qemu-main.h +++ b/include/qemu-main.h @@ -5,7 +5,24 @@ #ifndef QEMU_MAIN_H #define QEMU_MAIN_H -int qemu_default_main(void); -extern int (*qemu_main)(void); +/* + * The function to run on the main (initial) thread of the process. + * NULL means QEMU's main event loop. + * When non-NULL, QEMU's main event loop will run on a purposely created + * thread, after which the provided function pointer will be invoked on + * the initial thread. + * This is useful on platforms which treat the main thread as special + * (macOS/Darwin) and/or require all UI API calls to occur from a + * specific thread. + * Implementing this via a global function pointer variable is a bit + * ugly, but it's probably worth investigating the existing UI thread rule + * violations in the SDL (e.g. #2537) and GTK+ back-ends. Fixing those + * issues might precipitate requirements similar but not identical to those + * of the Cocoa UI; hopefully we'll see some kind of pattern emerge, which + * can then be used as a basis for an overhaul. (In fact, it may turn + * out to be simplest to split the UI/native platform event thread from the + * QEMU main event loop on all platforms, with any UI or even none at all.) + */ +extern qemu_main_fn qemu_main; qemu_main_fn is defined in typdefefs.h but not included here. Also coding style says typedefs should be CamelCase but maybe it's just not worth to define a type for this and you can just leave the existing line here only removing the qemu_default_main declaration and adding the comment. #endif /* QEMU_MAIN_H */ diff --git a/
Re: [PATCH v10 01/15] ui & main loop: Redesign of system-specific main thread event handling
On Wed, Nov 13, 2024 at 7:16 PM BALATON Zoltan wrote: > > int main(int argc, char **argv) > > { > > +QemuThread main_loop_thread; > > + > > qemu_init(argc, argv); > > -return qemu_main(); > > +if (qemu_main) { > > +qemu_thread_create(&main_loop_thread, "qemu_main", > > + call_qemu_default_main, NULL, > > QEMU_THREAD_DETACHED); > > +bql_unlock(); > > +return qemu_main(); > > +} else { > > +qemu_default_main(); > > I think you need 'return qemu_default_main()' here but I'm a bit confused > by all this wrapping of qemu_default_main in call_qemu_default_main. I see > that may be needed because qemu_thread_create takes a different function > but now that qemu_default main is static and not replaced externally could > that be changed to the right type and avoid this confusion and simplify > this a bit? Note that qemu_default_main() expects the BQL to be locked, whereas qemu_main() and call_qemu_default_main() do not (because they run in a separate thread). But you're right, we could push bql_lock()/bql_unlock() into qemu_default_main(), and do bql_unlock(); if (qemu_main) { qemu_thread_create(&main_loop_thread, "qemu_main", call_qemu_default_main, NULL, QEMU_THREAD_DETACHED); return qemu_main(); } else { return qemu_default_main(); } Paolo > Regards, > BALATON Zoltan > > > +} > > } > > diff --git a/ui/cocoa.m b/ui/cocoa.m > > index 4c2dd335323..30b8920d929 100644 > > --- a/ui/cocoa.m > > +++ b/ui/cocoa.m > > @@ -73,6 +73,8 @@ > > int height; > > } QEMUScreen; > > > > +@class QemuCocoaPasteboardTypeOwner; > > + > > static void cocoa_update(DisplayChangeListener *dcl, > > int x, int y, int w, int h); > > > > @@ -107,6 +109,7 @@ static void cocoa_switch(DisplayChangeListener *dcl, > > static NSInteger cbchangecount = -1; > > static QemuClipboardInfo *cbinfo; > > static QemuEvent cbevent; > > +static QemuCocoaPasteboardTypeOwner *cbowner; > > > > // Utility functions to run specified code block with the BQL held > > typedef void (^CodeBlock)(void); > > @@ -1321,8 +1324,10 @@ - (void) dealloc > > { > > COCOA_DEBUG("QemuCocoaAppController: dealloc\n"); > > > > -if (cocoaView) > > -[cocoaView release]; > > +[cocoaView release]; > > +[cbowner release]; > > +cbowner = nil; > > + > > [super dealloc]; > > } > > > > @@ -1938,8 +1943,6 @@ - (void)pasteboard:(NSPasteboard *)sender > > provideDataForType:(NSPasteboardType)t > > > > @end > > > > -static QemuCocoaPasteboardTypeOwner *cbowner; > > - > > static void cocoa_clipboard_notify(Notifier *notifier, void *data); > > static void cocoa_clipboard_request(QemuClipboardInfo *info, > > QemuClipboardType type); > > @@ -2002,43 +2005,8 @@ static void > > cocoa_clipboard_request(QemuClipboardInfo *info, > > } > > } > > > > -/* > > - * The startup process for the OSX/Cocoa UI is complicated, because > > - * OSX insists that the UI runs on the initial main thread, and so we > > - * need to start a second thread which runs the qemu_default_main(): > > - * in main(): > > - * in cocoa_display_init(): > > - * assign cocoa_main to qemu_main > > - * create application, menus, etc > > - * in cocoa_main(): > > - * create qemu-main thread > > - * enter OSX run loop > > - */ > > - > > -static void *call_qemu_main(void *opaque) > > -{ > > -int status; > > - > > -COCOA_DEBUG("Second thread: calling qemu_default_main()\n"); > > -bql_lock(); > > -status = qemu_default_main(); > > -bql_unlock(); > > -COCOA_DEBUG("Second thread: qemu_default_main() returned, exiting\n"); > > -[cbowner release]; > > -exit(status); > > -} > > - > > static int cocoa_main(void) > > { > > -QemuThread thread; > > - > > -COCOA_DEBUG("Entered %s()\n", __func__); > > - > > -bql_unlock(); > > -qemu_thread_create(&thread, "qemu_main", call_qemu_main, > > - NULL, QEMU_THREAD_DETACHED); > > - > > -// Start the main event loop > > COCOA_DEBUG("Main thread: entering OSX run loop\n"); > > [NSApp run]; > > COCOA_DEBUG("Main thread: left OSX run loop, which should never > > happen\n"); > > @@ -2120,8 +2088,6 @@ static void cocoa_display_init(DisplayState *ds, > > DisplayOptions *opts) > > > > COCOA_DEBUG("qemu_cocoa: cocoa_display_init\n"); > > > > -qemu_main = cocoa_main; > > - > > // Pull this console process up to being a fully-fledged graphical > > // app with a menubar and Dock icon > > ProcessSerialNumber psn = { 0, kCurrentProcess }; > > @@ -2185,6 +2151,12 @@ static void cocoa_display_init(DisplayState *ds, > > DisplayOptions *opts) > > qemu_clipboard_peer_register(&cbpeer); > > > > [pool release]; > > + > > +/* > > + * The Cocoa UI will run the NSApplication runloop on the main thread > > + * rather than the default Core Foundation
Re: [PATCH v10 01/15] ui & main loop: Redesign of system-specific main thread event handling
On Wed, 13 Nov 2024 at 19:36, Paolo Bonzini wrote: > On Wed, Nov 13, 2024 at 7:16 PM BALATON Zoltan wrote: > > > int main(int argc, char **argv) > > > { > > > +QemuThread main_loop_thread; > > > + > > > qemu_init(argc, argv); > > > -return qemu_main(); > > > +if (qemu_main) { > > > +qemu_thread_create(&main_loop_thread, "qemu_main", > > > + call_qemu_default_main, NULL, > QEMU_THREAD_DETACHED); > > > +bql_unlock(); > > > +return qemu_main(); > > > +} else { > > > +qemu_default_main(); > > > > I think you need 'return qemu_default_main()' here but I'm a bit confused > > by all this wrapping of qemu_default_main in call_qemu_default_main. I > see > > that may be needed because qemu_thread_create takes a different function > > but now that qemu_default main is static and not replaced externally > could > > that be changed to the right type and avoid this confusion and simplify > > this a bit? > > Note that qemu_default_main() expects the BQL to be locked, whereas > qemu_main() and call_qemu_default_main() do not (because they run in a > separate thread). > > But you're right, we could push bql_lock()/bql_unlock() into > qemu_default_main(), and do > > bql_unlock(); > if (qemu_main) { > qemu_thread_create(&main_loop_thread, "qemu_main", >call_qemu_default_main, NULL, > QEMU_THREAD_DETACHED); > return qemu_main(); > } else { > return qemu_default_main(); > } > > Good point, if it's safe to drop the lock on thread 0 and re-lock it on another thread before running qemu_main_loop() there, it's also safe to momentarily drop the lock on thread 0 and re-take it before calling into qemu_main_loop(). I'll take that as a starting point and see how far I can simplify things. Thanks to both of you for the feedback! Paolo > > > Regards, > > BALATON Zoltan > > > > > +} > > > } > > > diff --git a/ui/cocoa.m b/ui/cocoa.m > > > index 4c2dd335323..30b8920d929 100644 > > > --- a/ui/cocoa.m > > > +++ b/ui/cocoa.m > > > @@ -73,6 +73,8 @@ > > > int height; > > > } QEMUScreen; > > > > > > +@class QemuCocoaPasteboardTypeOwner; > > > + > > > static void cocoa_update(DisplayChangeListener *dcl, > > > int x, int y, int w, int h); > > > > > > @@ -107,6 +109,7 @@ static void cocoa_switch(DisplayChangeListener > *dcl, > > > static NSInteger cbchangecount = -1; > > > static QemuClipboardInfo *cbinfo; > > > static QemuEvent cbevent; > > > +static QemuCocoaPasteboardTypeOwner *cbowner; > > > > > > // Utility functions to run specified code block with the BQL held > > > typedef void (^CodeBlock)(void); > > > @@ -1321,8 +1324,10 @@ - (void) dealloc > > > { > > > COCOA_DEBUG("QemuCocoaAppController: dealloc\n"); > > > > > > -if (cocoaView) > > > -[cocoaView release]; > > > +[cocoaView release]; > > > +[cbowner release]; > > > +cbowner = nil; > > > + > > > [super dealloc]; > > > } > > > > > > @@ -1938,8 +1943,6 @@ - (void)pasteboard:(NSPasteboard *)sender > provideDataForType:(NSPasteboardType)t > > > > > > @end > > > > > > -static QemuCocoaPasteboardTypeOwner *cbowner; > > > - > > > static void cocoa_clipboard_notify(Notifier *notifier, void *data); > > > static void cocoa_clipboard_request(QemuClipboardInfo *info, > > > QemuClipboardType type); > > > @@ -2002,43 +2005,8 @@ static void > cocoa_clipboard_request(QemuClipboardInfo *info, > > > } > > > } > > > > > > -/* > > > - * The startup process for the OSX/Cocoa UI is complicated, because > > > - * OSX insists that the UI runs on the initial main thread, and so we > > > - * need to start a second thread which runs the qemu_default_main(): > > > - * in main(): > > > - * in cocoa_display_init(): > > > - * assign cocoa_main to qemu_main > > > - * create application, menus, etc > > > - * in cocoa_main(): > > > - * create qemu-main thread > > > - * enter OSX run loop > > > - */ > > > - > > > -static void *call_qemu_main(void *opaque) > > > -{ > > > -int status; > > > - > > > -COCOA_DEBUG("Second thread: calling qemu_default_main()\n"); > > > -bql_lock(); > > > -status = qemu_default_main(); > > > -bql_unlock(); > > > -COCOA_DEBUG("Second thread: qemu_default_main() returned, > exiting\n"); > > > -[cbowner release]; > > > -exit(status); > > > -} > > > - > > > static int cocoa_main(void) > > > { > > > -QemuThread thread; > > > - > > > -COCOA_DEBUG("Entered %s()\n", __func__); > > > - > > > -bql_unlock(); > > > -qemu_thread_create(&thread, "qemu_main", call_qemu_main, > > > - NULL, QEMU_THREAD_DETACHED); > > > - > > > -// Start the main event loop > > > COCOA_DEBUG("Main thread: entering OSX run loop\n"); > > > [NSApp run]; > > > COCOA_DEBUG("Main thread: left OSX run loop, which should never > happen\n"); > > > @@ -2120,8 +2088,6 @@ static void cocoa_di
RE: [PATCH v2 00/18] Fix write incorrect data into flash in user mode
Hi Cedric, > Subject: RE: [PATCH v2 00/18] Fix write incorrect data into flash in user mode > > Hi Cedric, > > > Subject: Re: [PATCH v2 00/18] Fix write incorrect data into flash in > > user mode > > > > Hello Jamin, > > > > On 10/22/24 11:40, Jamin Lin wrote: > > > change from v1: > > > 1. Fix write incorrect data into flash in user mode. > > > 2. Refactor aspeed smc qtest testcases to support AST2600, AST2500 > > > and AST1030. > > > 3. Add ast2700 smc qtest testcase to support AST2700. > > > > > > change from v2: > > > 1. Introduce a new aspeed-smc-utils.c to place common testcases. > > > 2. Fix hardcode attach flash model of spi controllers 3. Add > > > reviewers suggestion and fix review issue. > > I have applied 1-6,8 to aspeed-next and should send a PR with them. I > > kept the test extensions for later, to take a closer a look and also > > because I will be on PTO next week. Tests can be merged in the next PR > > if we have time in this cycle or in QEMU 10.0. > > > Got it and thanks for help. > Jamin > > Thanks, Could you please help to review patch 17 and 18 ? Do I need to re-send patch from 9 to 18 of this patch series? Thanks-Jamin > > > > C. > >
RE: [PATCH v2 0/3] Introduce a new Write Protected pin inverted property
Hi Cedric, Andrew > Subject: [PATCH v2 0/3] Introduce a new Write Protected pin inverted property > > change from v1: > 1. Support RTC for AST2700. > 2. Support SDHCI write protected pin inverted for AST2500 and AST2600. > 3. Introduce Capabilities Register 2 for SD slot 0 and 1. > 4. Support create flash devices via command line for AST1030. > > change from v2: > replace wp-invert with wp-inverted and fix review issues. > > Jamin Lin (3): > hw/sd/sdhci: Fix coding style > hw/sd/sdhci: Introduce a new Write Protected pin inverted property > hw/arm/aspeed: Invert sdhci write protected pin for AST2600 and > AST2500 EVBs > > hw/arm/aspeed.c | 8 + > hw/sd/sdhci.c | 70 - > include/hw/arm/aspeed.h | 1 + > include/hw/sd/sdhci.h | 5 +++ > 4 files changed, 62 insertions(+), 22 deletions(-) > Could you please help to review this patch series? Thanks-Jamin > -- > 2.34.1
Re: [PATCH v2 0/3] Introduce a new Write Protected pin inverted property
Hello Jamin, On 11/14/24 06:32, Jamin Lin wrote: Hi Cedric, Andrew Subject: [PATCH v2 0/3] Introduce a new Write Protected pin inverted property change from v1: 1. Support RTC for AST2700. 2. Support SDHCI write protected pin inverted for AST2500 and AST2600. 3. Introduce Capabilities Register 2 for SD slot 0 and 1. 4. Support create flash devices via command line for AST1030. change from v2: replace wp-invert with wp-inverted and fix review issues. Jamin Lin (3): hw/sd/sdhci: Fix coding style hw/sd/sdhci: Introduce a new Write Protected pin inverted property hw/arm/aspeed: Invert sdhci write protected pin for AST2600 and AST2500 EVBs hw/arm/aspeed.c | 8 + hw/sd/sdhci.c | 70 - include/hw/arm/aspeed.h | 1 + include/hw/sd/sdhci.h | 5 +++ 4 files changed, 62 insertions(+), 22 deletions(-) Could you please help to review this patch series? We would need an Ack from the sd maintainer on patch 2. Then, I can apply on the aspeed queue. That's material for QEMU 10.0. Thanks, C.
Re: [PATCH v2 2/3] hw/sd/sdhci: Introduce a new Write Protected pin inverted property
On 11/4/24 04:21, Jamin Lin wrote: The Write Protect pin of SDHCI model is default active low to match the SDHCI spec. So, write enable the bit 19 should be 1 and write protected the bit 19 should be 0 at the Present State Register (0x24). However, some boards are design Write Protected pin active high. In other words, write enable the bit 19 should be 0 and write protected the bit 19 should be 1 at the Present State Register (0x24). To support it, introduces a new "wp-inverted" property and set it false by default. Signed-off-by: Jamin Lin Acked-by: Cédric Le Goater --- hw/sd/sdhci.c | 6 ++ include/hw/sd/sdhci.h | 5 + 2 files changed, 11 insertions(+) diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c index db7d547156..c675543873 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -275,6 +275,10 @@ static void sdhci_set_readonly(DeviceState *dev, bool level) { SDHCIState *s = (SDHCIState *)dev; +if (s->wp_inverted) { +level = !level; +} + if (level) { s->prnsts &= ~SDHC_WRITE_PROTECT; } else { @@ -1551,6 +1555,8 @@ static Property sdhci_sysbus_properties[] = { false), DEFINE_PROP_LINK("dma", SDHCIState, dma_mr, TYPE_MEMORY_REGION, MemoryRegion *), +DEFINE_PROP_BOOL("wp-inverted", SDHCIState, + wp_inverted, false), DEFINE_PROP_END_OF_LIST(), }; diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h index 6cd2822f1d..25ad9ed778 100644 --- a/include/hw/sd/sdhci.h +++ b/include/hw/sd/sdhci.h @@ -100,6 +100,11 @@ struct SDHCIState { uint8_t sd_spec_version; uint8_t uhs_mode; uint8_t vendor;/* For vendor specific functionality */ +/* + * Write Protect pin default active low for detecting SD card + * to be protected. Set wp_inverted to true inverted the signal. In case you respin, may be you could change the last sentence to : "Set wp_inverted to invert the signal." Thanks, C. + */ +bool wp_inverted; }; typedef struct SDHCIState SDHCIState;
Re: [PATCH v2 00/18] Fix write incorrect data into flash in user mode
Hello Jamin, On 11/14/24 06:30, Jamin Lin wrote: Hi Cedric, Subject: RE: [PATCH v2 00/18] Fix write incorrect data into flash in user mode Hi Cedric, Subject: Re: [PATCH v2 00/18] Fix write incorrect data into flash in user mode Hello Jamin, On 10/22/24 11:40, Jamin Lin wrote: change from v1: 1. Fix write incorrect data into flash in user mode. 2. Refactor aspeed smc qtest testcases to support AST2600, AST2500 and AST1030. 3. Add ast2700 smc qtest testcase to support AST2700. change from v2: 1. Introduce a new aspeed-smc-utils.c to place common testcases. 2. Fix hardcode attach flash model of spi controllers 3. Add reviewers suggestion and fix review issue. I have applied 1-6,8 to aspeed-next and should send a PR with them. I kept the test extensions for later, to take a closer a look and also because I will be on PTO next week. Tests can be merged in the next PR if we have time in this cycle or in QEMU 10.0. Got it and thanks for help. Jamin Thanks, Could you please help to review patch 17 and 18 ? Do I need to re-send patch from 9 to 18 of this patch series? Not yet. I have some comments to send but I am busy on another topic. We have some time before QEMU 10.0. They are in my aspeed-9.2 branch, so that I don't forget about them. Thanks, C.