Re: [PATCH v2 09/14] vhost: Add VhostIOVATree

[Eugenio Perez Martin]

On Fri, Mar 4, 2022 at 3:04 AM Jason Wang  wrote:
>
> On Fri, Mar 4, 2022 at 12:33 AM Eugenio Perez Martin
>  wrote:
> >
> > On Mon, Feb 28, 2022 at 8:06 AM Jason Wang  wrote:
> > >
> > >
> > > 在 2022/2/27 下午9:41, Eugenio Pérez 写道:
> > > > This tree is able to look for a translated address from an IOVA address.
> > > >
> > > > At first glance it is similar to util/iova-tree. However, SVQ working on
> > > > devices with limited IOVA space need more capabilities, like allocating
> > > > IOVA chunks or performing reverse translations (qemu addresses to iova).
> > > >
> > > > The allocation capability, as "assign a free IOVA address to this chunk
> > > > of memory in qemu's address space" allows shadow virtqueue to create a
> > > > new address space that is not restricted by guest's addressable one, so
> > > > we can allocate shadow vqs vrings outside of it.
> > > >
> > > > It duplicates the tree so it can search efficiently in both directions,
> > > > and it will signal overlap if iova or the translated address is present
> > > > in any tree.
> > > >
> > > > Signed-off-by: Eugenio Pérez 
> > > > ---
> > > >   hw/virtio/vhost-iova-tree.h |  27 +++
> > > >   hw/virtio/vhost-iova-tree.c | 155 
> > > >   hw/virtio/meson.build   |   2 +-
> > > >   3 files changed, 183 insertions(+), 1 deletion(-)
> > > >   create mode 100644 hw/virtio/vhost-iova-tree.h
> > > >   create mode 100644 hw/virtio/vhost-iova-tree.c
> > > >
> > > > diff --git a/hw/virtio/vhost-iova-tree.h b/hw/virtio/vhost-iova-tree.h
> > > > new file mode 100644
> > > > index 00..6a4f24e0f9
> > > > --- /dev/null
> > > > +++ b/hw/virtio/vhost-iova-tree.h
> > > > @@ -0,0 +1,27 @@
> > > > +/*
> > > > + * vhost software live migration iova tree
> > > > + *
> > > > + * SPDX-FileCopyrightText: Red Hat, Inc. 2021
> > > > + * SPDX-FileContributor: Author: Eugenio Pérez 
> > > > + *
> > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > + */
> > > > +
> > > > +#ifndef HW_VIRTIO_VHOST_IOVA_TREE_H
> > > > +#define HW_VIRTIO_VHOST_IOVA_TREE_H
> > > > +
> > > > +#include "qemu/iova-tree.h"
> > > > +#include "exec/memory.h"
> > > > +
> > > > +typedef struct VhostIOVATree VhostIOVATree;
> > > > +
> > > > +VhostIOVATree *vhost_iova_tree_new(uint64_t iova_first, uint64_t 
> > > > iova_last);
> > > > +void vhost_iova_tree_delete(VhostIOVATree *iova_tree);
> > > > +G_DEFINE_AUTOPTR_CLEANUP_FUNC(VhostIOVATree, vhost_iova_tree_delete);
> > > > +
> > > > +const DMAMap *vhost_iova_tree_find_iova(const VhostIOVATree *iova_tree,
> > > > +const DMAMap *map);
> > > > +int vhost_iova_tree_map_alloc(VhostIOVATree *iova_tree, DMAMap *map);
> > > > +void vhost_iova_tree_remove(VhostIOVATree *iova_tree, const DMAMap 
> > > > *map);
> > > > +
> > > > +#endif
> > > > diff --git a/hw/virtio/vhost-iova-tree.c b/hw/virtio/vhost-iova-tree.c
> > > > new file mode 100644
> > > > index 00..03496ac075
> > > > --- /dev/null
> > > > +++ b/hw/virtio/vhost-iova-tree.c
> > > > @@ -0,0 +1,155 @@
> > > > +/*
> > > > + * vhost software live migration iova tree
> > > > + *
> > > > + * SPDX-FileCopyrightText: Red Hat, Inc. 2021
> > > > + * SPDX-FileContributor: Author: Eugenio Pérez 
> > > > + *
> > > > + * SPDX-License-Identifier: GPL-2.0-or-later
> > > > + */
> > > > +
> > > > +#include "qemu/osdep.h"
> > > > +#include "qemu/iova-tree.h"
> > > > +#include "vhost-iova-tree.h"
> > > > +
> > > > +#define iova_min_addr qemu_real_host_page_size
> > > > +
> > > > +/**
> > > > + * VhostIOVATree, able to:
> > > > + * - Translate iova address
> > > > + * - Reverse translate iova address (from translated to iova)
> > > > + * - Allocate IOVA regions for translated range (linear operation)
> > > > + */
> > > > +struct VhostIOVATree {
> > > > +/* First addressable iova address in the device */
> > > > +uint64_t iova_first;
> > > > +
> > > > +/* Last addressable iova address in the device */
> > > > +uint64_t iova_last;
> > > > +
> > > > +/* IOVA address to qemu memory maps. */
> > > > +IOVATree *iova_taddr_map;
> > > > +
> > > > +/* QEMU virtual memory address to iova maps */
> > > > +GTree *taddr_iova_map;
> > > > +};
> > > > +
> > > > +static gint vhost_iova_tree_cmp_taddr(gconstpointer a, gconstpointer b,
> > > > +  gpointer data)
> > > > +{
> > > > +const DMAMap *m1 = a, *m2 = b;
> > > > +
> > > > +if (m1->translated_addr > m2->translated_addr + m2->size) {
> > > > +return 1;
> > > > +}
> > > > +
> > > > +if (m1->translated_addr + m1->size < m2->translated_addr) {
> > > > +return -1;
> > > > +}
> > > > +
> > > > +/* Overlapped */
> > > > +return 0;
> > > > +}
> > > > +
> > > > +/**
> > > > + * Create a new IOVA tree
> > > > + *
> > > > + * Returns the new IOVA tree
> > > > + */
> > > > +VhostIOVATree *vhost_iova_tree_new(hwaddr iova_first, hwaddr iova_last)
> > > > +{
> > > > +VhostIOVATree *t

Re: [PATCH 1/1] virtio: fix feature negotiation for ACCESS_PLATFORM

[Michael S. Tsirkin]

On Thu, Feb 10, 2022 at 02:29:29PM +0100, Halil Pasic wrote:
> On Thu, 10 Feb 2022 12:19:25 +0100
> Cornelia Huck  wrote:
> 
> [..]
> > 
> > Nope, that's not my problem. We make sure that the bit is persistent, we
> > fail realization if the bit got removed by the callback when required,
> > and we fail feature validation if the driver removes the bit, which is
> > in a different code path. We should not talk about FEATURES_OK in this
> > code.
> 
> I agree. I changed my mind. Expanation follows...
> 
> > 
> > We force-add the bit, and then still might fail realization. The
> > important condition is the has_iommu one, not the checks later on. I
> > find it very confusing to talk about what a potential driver might do in
> > that context.
> > 
> 
> I assumed stuff like virtiofs+SE regressed with commit 04ceb61a40
> ("virtio: Fail if iommu_platform is requested, but unsupported") but I
> think, I was wrong. It didn't work before that, because we did not
> present ACCESS_PLATFORM to the guest. I operated under the assumption
> that virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM)
> does not impact the set of features offered to the driver by the device,
> but it does.
> 
> So we need both this patch and "[PATCH v5 1/1] virtio: fix the condition
> for iommu_platform not supported" to get virtiofs to work with SE/SEV/Secure 
> VM.
> 
> I have to admit I only tested for the error message, and not with a full
> SE setup.
> 
> I agree my comment was inadequate. Can we use
> 
> /* make sure that the device did not drop a required IOMMU_PLATFORM */
> 
> I will think some more though. This is again about the dual nature
> of ACCESS_PLATFORM...

Were you going to post a new version of this patch?

> > What about moving the virtio_add_feature() after the if
> > (klass->get_dma_as) check, and adding a comment
> > 
> > /* we want to always force IOMMU_PLATFORM here */
> > 
> > [I'll withdraw from this discussion for now, I fear I might just add
> > confusion.]
> > 
> > 

Re: [PATCH v4 08/14] util: Add iova_tree_alloc_map

[Eugenio Perez Martin]

On Fri, Mar 4, 2022 at 3:11 AM Liuxiangdong  wrote:
>
>
>
> On 2022/3/4 2:51, Eugenio Pérez wrote:
> > This iova tree function allows it to look for a hole in allocated
> > regions and return a totally new translation for a given translated
> > address.
> >
> > It's usage is mainly to allow devices to access qemu address space,
> > remapping guest's one into a new iova space where qemu can add chunks of
> > addresses.
> >
> > Signed-off-by: Eugenio Pérez 
> > Reviewed-by: Peter Xu 
> > ---
> >   include/qemu/iova-tree.h |  18 ++
> >   util/iova-tree.c | 135 +++
> >   2 files changed, 153 insertions(+)
> >
> > diff --git a/include/qemu/iova-tree.h b/include/qemu/iova-tree.h
> > index 8249edd764..d066400f09 100644
> > --- a/include/qemu/iova-tree.h
> > +++ b/include/qemu/iova-tree.h
> > @@ -29,6 +29,7 @@
> >   #define  IOVA_OK   (0)
> >   #define  IOVA_ERR_INVALID  (-1) /* Invalid parameters */
> >   #define  IOVA_ERR_OVERLAP  (-2) /* IOVA range overlapped */
> > +#define  IOVA_ERR_NOMEM(-3) /* Cannot allocate */
> >
> >   typedef struct IOVATree IOVATree;
> >   typedef struct DMAMap {
> > @@ -119,6 +120,23 @@ const DMAMap *iova_tree_find_address(const IOVATree 
> > *tree, hwaddr iova);
> >*/
> >   void iova_tree_foreach(IOVATree *tree, iova_tree_iterator iterator);
> >
> > +/**
> > + * iova_tree_alloc_map:
> > + *
> > + * @tree: the iova tree to allocate from
> > + * @map: the new map (as translated addr & size) to allocate in the iova 
> > region
> > + * @iova_begin: the minimum address of the allocation
> > + * @iova_end: the maximum addressable direction of the allocation
> > + *
> > + * Allocates a new region of a given size, between iova_min and iova_max.
> > + *
> > + * Return: Same as iova_tree_insert, but cannot overlap and can return 
> > error if
> > + * iova tree is out of free contiguous range. The caller gets the assigned 
> > iova
> > + * in map->iova.
> > + */
> > +int iova_tree_alloc_map(IOVATree *tree, DMAMap *map, hwaddr iova_begin,
> > +hwaddr iova_end);
> > +
> >   /**
> >* iova_tree_destroy:
> >*
> > diff --git a/util/iova-tree.c b/util/iova-tree.c
> > index 23ea35b7a4..3160c50d3b 100644
> > --- a/util/iova-tree.c
> > +++ b/util/iova-tree.c
> > @@ -16,6 +16,39 @@ struct IOVATree {
> >   GTree *tree;
> >   };
> >
> > +/* Args to pass to iova_tree_alloc foreach function. */
> > +struct IOVATreeAllocArgs {
> > +/* Size of the desired allocation */
> > +size_t new_size;
> > +
> > +/* The minimum address allowed in the allocation */
> > +hwaddr iova_begin;
> > +
> > +/* Map at the left of the hole, can be NULL if "this" is first one */
> > +const DMAMap *prev;
> > +
> > +/* Map at the right of the hole, can be NULL if "prev" is the last one 
> > */
> > +const DMAMap *this;
> > +
> > +/* If found, we fill in the IOVA here */
> > +hwaddr iova_result;
> > +
> > +/* Whether have we found a valid IOVA */
> > +bool iova_found;
> > +};
> > +
> > +/**
> > + * Iterate args to the next hole
> > + *
> > + * @args: The alloc arguments
> > + * @next: The next mapping in the tree. Can be NULL to signal the last one
> > + */
> > +static void iova_tree_alloc_args_iterate(struct IOVATreeAllocArgs *args,
> > + const DMAMap *next) {
> > +args->prev = args->this;
> > +args->this = next;
> > +}
> > +
> >   static int iova_tree_compare(gconstpointer a, gconstpointer b, gpointer 
> > data)
> >   {
> >   const DMAMap *m1 = a, *m2 = b;
> > @@ -107,6 +140,108 @@ int iova_tree_remove(IOVATree *tree, const DMAMap 
> > *map)
> >   return IOVA_OK;
> >   }
> >
> > +/**
> > + * Try to find an unallocated IOVA range between prev and this elements.
> > + *
> > + * @args: Arguments to allocation
> > + *
> > + * Cases:
> > + *
> > + * (1) !prev, !this: No entries allocated, always succeed
> > + *
> > + * (2) !prev, this: We're iterating at the 1st element.
> > + *
> > + * (3) prev, !this: We're iterating at the last element.
> > + *
> > + * (4) prev, this: this is the most common case, we'll try to find a hole
> > + * between "prev" and "this" mapping.
> > + *
> > + * Note that this function assumes the last valid iova is HWADDR_MAX, but 
> > it
> > + * searches linearly so it's easy to discard the result if it's not the 
> > case.
> > + */
> > +static void iova_tree_alloc_map_in_hole(struct IOVATreeAllocArgs *args)
> > +{
> > +const DMAMap *prev = args->prev, *this = args->this;
> > +uint64_t hole_start, hole_last;
> > +
> > +if (this && this->iova + this->size < args->iova_begin) {
> > +return;
> > +}
> > +
>
> Hi, Eugenio.
> Is there such a condition that
>
> args->iova_begin > this->iova  and
> args->iova_begin < this->iova + this->size
>

Hi Xiangdong Liu,

With the current code we would never have such a case, since
iova_begin and iova_last are the same in all the life of the tree. So
no previous 

Re: [PATCH] hw/arm/virt: Validate memory size on the first NUMA node

[Gerd Hoffmann]

  Hi,

> because 128MB node memory size isn't used in practice. If the EDK2 error
> message can be improved, we might have something like below:
> 
> ASSERT [MemoryInit] 
> /home/lacos/src/upstream/qemu/roms/edk2/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c(93):
>  NewSize >= 0x0800
> 
> to
> 
> The first NUMA node should have more than 128MB memory

Well, except the firmware doesn't even know yet at that point
whenever it runs on a NUMA machine or not ...

take care,
  Gerd

Re: [PATCH v3 14/14] vdpa: Add x-svq to NetdevVhostVDPAOptions

[Eugenio Perez Martin]

On Fri, Mar 4, 2022 at 7:30 AM Markus Armbruster  wrote:
>
> Eugenio Perez Martin  writes:
>
> > Yes, that's right. I expressed my point poorly actually, I'll go the 
> > reverse.
> >
> > qapi-gen.py forces me to write a comment in the doc:
> > qapi/block-core.json:2971: feature 'unstable' lacks documentation
> >
> > When I add the documentation line, it's enough to add @unstable. But
> > there is no way to tell if this tag is because the whole struct is
> > unstable or if it's because individual members are unstable unless the
> > reader either checks the tag or the struct code.
> >
> > I was mostly worried about doc generators, I would not like to make
> > NetdevVhostVDPAOptions unstable at this point. But I see that there is
> > no problem with that.
> >
> > Thanks!
>
> Yes, the doc generator insists on features being documented, and it
> doesn't provide for documenting them next to members, only top-level.
> The common solution is to phrase the comment like we do in
> BlockdevOptionsFile:
>
> # @unstable: Member x-check-cache-dropped is meant for debugging.
>
> If there were multiple members so flagged, we'd enumerate them all.
>
> The generator doesn't check you do this right.  The existing check
> guards against *forgetting* to do it, not against doing it wrong.
>
> More questions?
>

Got it, it matches my impression. Thank you very much!

[PATCH] qga: Introduce disk smart

[zhenwei pi]

After assigning a NVMe/SCSI controller to guest by VFIO, we lose
everything on the host side. A guest uses these devices exclusively,
we usually don't care the actions on these devices. But there is a
low probability that hitting physical hardware warning, we need a
chance to get the basic smart log info.

Introduce disk smart, and implement NVMe smart on linux.

CC: Keith Busch 
Signed-off-by: zhenwei pi 
---
 qga/commands-posix.c | 77 
 qga/qapi-schema.json | 50 +++-
 2 files changed, 126 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 4ec83bbfbc..f08c9bea14 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -27,6 +27,7 @@
 #include "qemu/base64.h"
 #include "qemu/cutils.h"
 #include "commands-common.h"
+#include "block/nvme.h"
 
 #ifdef HAVE_UTMPX
 #include 
@@ -49,6 +50,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef CONFIG_LIBUDEV
 #include 
@@ -1390,6 +1392,80 @@ static GuestDiskInfoList *get_disk_partitions(
 return ret;
 }
 
+static void get_disk_smart(GuestDiskInfo *disk)
+{
+if (disk->has_address
+&& (disk->address->bus_type == GUEST_DISK_BUS_TYPE_NVME)) {
+int fd;
+GuestDiskSmart *smart;
+NvmeSmartLog log = {0};
+struct nvme_admin_cmd cmd = {
+.opcode = NVME_ADM_CMD_GET_LOG_PAGE,
+.nsid = NVME_NSID_BROADCAST,
+.addr = (uint64_t)&log,
+.data_len = sizeof(log),
+.cdw10 = NVME_LOG_SMART_INFO | (1 << 15) /* RAE bit */
+ | (((sizeof(log) >> 2) - 1) << 16)
+};
+
+fd = qemu_open_old(disk->name, O_RDONLY);
+if (fd == -1) {
+g_debug("Failed to open device: %s", disk->name);
+return;
+}
+if (ioctl(fd, NVME_IOCTL_ADMIN_CMD, &cmd)) {
+g_debug("Failed to get smart: %s", disk->name);
+close(fd);
+return;
+}
+
+smart = g_new0(GuestDiskSmart, 1);
+smart->type = GUEST_DISK_BUS_TYPE_NVME;
+smart->u.nvme.critical_warning = log.critical_warning;
+smart->u.nvme.temperature = le16_to_cpu(log.temperature);
+smart->u.nvme.available_spare = log.available_spare;
+smart->u.nvme.available_spare_threshold = 
log.available_spare_threshold;
+smart->u.nvme.percentage_used = log.percentage_used;
+smart->u.nvme.data_units_read_lo = le64_to_cpu(log.data_units_read[0]);
+smart->u.nvme.data_units_read_hi = le64_to_cpu(log.data_units_read[1]);
+smart->u.nvme.data_units_written_lo =
+le64_to_cpu(log.data_units_written[0]);
+smart->u.nvme.data_units_written_hi =
+le64_to_cpu(log.data_units_written[1]);
+smart->u.nvme.host_read_commands_lo =
+le64_to_cpu(log.host_read_commands[0]);
+smart->u.nvme.host_read_commands_hi =
+le64_to_cpu(log.host_read_commands[1]);
+smart->u.nvme.host_write_commands_lo =
+le64_to_cpu(log.host_write_commands[0]);
+smart->u.nvme.host_write_commands_hi =
+le64_to_cpu(log.host_write_commands[1]);
+smart->u.nvme.controller_busy_time_lo =
+le64_to_cpu(log.controller_busy_time[0]);
+smart->u.nvme.controller_busy_time_hi =
+le64_to_cpu(log.controller_busy_time[1]);
+smart->u.nvme.power_cycles_lo = le64_to_cpu(log.power_cycles[0]);
+smart->u.nvme.power_cycles_hi = le64_to_cpu(log.power_cycles[1]);
+smart->u.nvme.power_on_hours_lo = le64_to_cpu(log.power_on_hours[0]);
+smart->u.nvme.power_on_hours_hi = le64_to_cpu(log.power_on_hours[1]);
+smart->u.nvme.unsafe_shutdowns_lo =
+le64_to_cpu(log.unsafe_shutdowns[0]);
+smart->u.nvme.unsafe_shutdowns_hi =
+le64_to_cpu(log.unsafe_shutdowns[1]);
+smart->u.nvme.media_errors_lo = le64_to_cpu(log.media_errors[0]);
+smart->u.nvme.media_errors_hi = le64_to_cpu(log.media_errors[1]);
+smart->u.nvme.number_of_error_log_entries_lo =
+le64_to_cpu(log.number_of_error_log_entries[0]);
+smart->u.nvme.number_of_error_log_entries_hi =
+le64_to_cpu(log.number_of_error_log_entries[1]);
+
+disk->has_smart = true;
+disk->smart = smart;
+
+close(fd);
+}
+}
+
 GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
 {
 GuestDiskInfoList *ret = NULL;
@@ -1463,6 +1539,7 @@ GuestDiskInfoList *qmp_guest_get_disks(Error **errp)
 }
 
 get_disk_deps(disk_dir, disk);
+get_disk_smart(disk);
 ret = get_disk_partitions(ret, de->d_name, disk_dir, dev_name);
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 8f73770210..8bb8731ce4 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -888,6 +888,53 @@
'*serial': 'str', '*dev': 'str',
'*ccw-address': 'Gue

Re: [PATCH v2 1/4] os-posix: Add os_set_daemonize()

[Daniel P . Berrangé]

On Thu, Mar 03, 2022 at 05:48:11PM +0100, Hanna Reitz wrote:
> The daemonizing functions in os-posix (os_daemonize() and
> os_setup_post()) only daemonize the process if the static `daemonize`
> variable is set.  Right now, it can only be set by os_parse_cmd_args().
> 
> In order to use os_daemonize() and os_setup_post() from the storage
> daemon to have it be daemonized, we need some other way to set this
> `daemonize` variable, because I would rather not tap into the system
> emulator's arg-parsing code.  Therefore, this patch adds an
> os_set_daemonize() function, which will return an error on os-win32
> (because daemonizing is not supported there).

IMHO the real flaw here is the design of 'os_daemonize' in that it
relies on static state. If I see a call to a function 'os_daemonize()'
I expect to be daemonized on return, but with this design that is not
guaranteed which is a big surprise.

I'd suggest we push the condition into the caller instead of adding
this extra function, so we have the more sane pattern:

   if (daemonmize()) {
  os_daemonize()
   }

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

Re: [PATCH v4 14/14] vdpa: Add x-svq to NetdevVhostVDPAOptions

[Eugenio Perez Martin]

On Fri, Mar 4, 2022 at 7:35 AM Markus Armbruster  wrote:
>
> Eugenio Pérez  writes:
>
> > Finally offering the possibility to enable SVQ from the command line.
> >
> > Signed-off-by: Eugenio Pérez 
> > ---
> >  qapi/net.json|  8 +++-
> >  net/vhost-vdpa.c | 48 
> >  2 files changed, 47 insertions(+), 9 deletions(-)
> >
> > diff --git a/qapi/net.json b/qapi/net.json
> > index 7fab2e7cd8..06a74d4224 100644
> > --- a/qapi/net.json
> > +++ b/qapi/net.json
> > @@ -445,12 +445,18 @@
> >  # @queues: number of queues to be created for multiqueue vhost-vdpa
> >  #  (default: 1)
> >  #
> > +# @x-svq: Start device with (experimental) shadow virtqueue. (Since 7.0)
> > +#
> > +# Features:
> > +# @unstable: Member @x-svq could change in future revisions.
>
> Elsewhere we document "Member @foo is experimental."  Does your
> different phrasing indicate a difference in intent?
>

Not really, I can use "Member @foo is experimental." too. I'll change
for the next revision.

> > +#
> >  # Since: 5.1
> >  ##
> >  { 'struct': 'NetdevVhostVDPAOptions',
> >'data': {
> >  '*vhostdev': 'str',
> > -'*queues':   'int' } }
> > +'*queues':   'int',
> > +'*x-svq':{'type': 'bool', 'features' : [ 'unstable'] } } }
> >
> >  ##
> >  # @NetClientDriver:
>
> Do you hope to make @x-svq stable eventually?  If yes: you'll want to
> rename it to @svq then, which could be a bother.  Can be avoided by
> naming it @svq now.  Up to you.
>

Yes, I'll rename for the next revision.

Thanks!

> [...]
>

Re: [PATCH v2 3/4] qsd: Add --daemonize

[Kevin Wolf]

Am 03.03.2022 um 17:48 hat Hanna Reitz geschrieben:
> To implement this, we reuse the existing daemonizing functions from the
> system emulator, which mainly do the following:
> - Fork off a child process, and set up a pipe between parent and child
> - The parent process waits until the child sends a status byte over the
>   pipe (0 means that the child was set up successfully; anything else
>   (including errors or EOF) means that the child was not set up
>   successfully), and then exits with an appropriate exit status
> - The child process enters a new session (forking off again), changes
>   the umask, and will ignore terminal signals from then on
> - Once set-up is complete, the child will chdir to /, redirect all
>   standard I/O streams to /dev/null, and tell the parent that set-up has
>   been completed successfully
> 
> In contrast to qemu-nbd's --fork implementation, during the set up
> phase, error messages are not piped through the parent process.
> qemu-nbd mainly does this to detect errors, though (while os_daemonize()
> has the child explicitly signal success after set up); because we do not
> redirect stderr after forking, error messages continue to appear on
> whatever the parent's stderr was (until set up is complete).
> 
> Signed-off-by: Hanna Reitz 
> ---
>  docs/tools/qemu-storage-daemon.rst   |  7 +++
>  storage-daemon/qemu-storage-daemon.c | 15 +++
>  2 files changed, 22 insertions(+)
> 
> diff --git a/docs/tools/qemu-storage-daemon.rst 
> b/docs/tools/qemu-storage-daemon.rst
> index 878e6a5c5c..8b97592663 100644
> --- a/docs/tools/qemu-storage-daemon.rst
> +++ b/docs/tools/qemu-storage-daemon.rst
> @@ -154,6 +154,13 @@ Standard options:
>created but before accepting connections. The daemon has started 
> successfully
>when the pid file is written and clients may begin connecting.
>  
> +.. option:: --daemonize
> +
> +  Daemonize the process. The parent process will exit once startup is 
> complete
> +  (i.e., after the pid file has been or would have been written) or failure
> +  occurs. Its exit code reflects whether the child has started up 
> successfully
> +  or failed to do so.
> +
>  Examples
>  
>  Launch the daemon with QMP monitor socket ``qmp.sock`` so clients can execute
> diff --git a/storage-daemon/qemu-storage-daemon.c 
> b/storage-daemon/qemu-storage-daemon.c
> index b798954edb..9f2c3332bf 100644
> --- a/storage-daemon/qemu-storage-daemon.c
> +++ b/storage-daemon/qemu-storage-daemon.c
> @@ -137,6 +137,9 @@ static void help(void)
>  "\n"
>  "  --pidfilewrite process ID to a file after startup\n"
>  "\n"
> +"  --daemonizedaemonize the process, and have the parent exit\n"
> +" once startup is complete\n"
> +"\n"

So far the long options in the help text are sorted alphabetically. Do
we want to keep this?

Kevin

Re: [PATCH v4 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Eric Auger]

Hi Marc-André,
On 3/3/22 5:16 PM, Marc-André Lureau wrote:
> Hi
> 
> On Thu, Mar 3, 2022 at 6:41 PM Eric Auger  > wrote:
> 
> Hi Stefan,
> 
> On 2/8/22 6:58 PM, Eric Auger wrote:
> > Hi Stefan,
> >
> > On 2/8/22 6:16 PM, Stefan Berger wrote:
> >>
> >> On 2/8/22 08:38, Eric Auger wrote:
> >>> Representing the CRB cmd/response buffer as a standard
> >>> RAM region causes some trouble when the device is used
> >>> with VFIO. Indeed VFIO attempts to DMA_MAP this region
> >>> as usual RAM but this latter does not have a valid page
> >>> size alignment causing such an error report:
> >>> "vfio_listener_region_add received unaligned region".
> >>> To allow VFIO to detect that failing dma mapping
> >>> this region is not an issue, let's use a ram_device
> >>> memory region type instead.
> >>>
> >>> Signed-off-by: Eric Auger  >
> >>> Tested-by: Stefan Berger  >
> >>> Acked-by: Stefan Berger  >
> >>> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
> >>> Signed-off-by: Philippe Mathieu-Daudé  >
> >>
> >>
> >> v4 doesn't build for me:
> >>
> >> ../hw/tpm/tpm_crb.c: In function ?tpm_crb_realize?:
> >> ../hw/tpm/tpm_crb.c:297:33: error: implicit declaration of function
> >> ?HOST_PAGE_ALIGN? [-Werror=implicit-function-declaration]
> >>   297 | HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
> >>   | ^~~
> >> ../hw/tpm/tpm_crb.c:297:33: error: nested extern declaration of
> >> ?HOST_PAGE_ALIGN? [-Werror=nested-externs]
> >> cc1: all warnings being treated as errors
> >
> > Do you have
> > b269a70810a  exec/cpu: Make host pages variables / macros 'target
> > agnostic' in your tree?
> I may have missed your reply. Did you have that dependency? Were you
> able to compile eventually?
> 
> Besides, do you have any opinion overall about the relevance of
> transforming the CRB ctrl cmd region into a RAM device wrt the TPM spec?
> 
> Again spec says:
> 
> "
> Including the control structure, the three memory areas comprise the
> entirety of the CRB. There are no constraints on how those three memory
> areas are provided. They can all be in system RAM, or all be in device
> memory, or any combination.
> "
> 
> (https://trustedcomputinggroup.org/wp-content/uploads/Mobile-Command-Response-Buffer-Interface-v2-r12-Specification_FINAL2.pdf
> 
> )
> 
> What was the rationale behind using RAM device for the PPI region?
> 
> 
> Is this the rationale you are looking for?
> https://gitlab.com/qemu-project/qemu/-/commit/3b97c01e9ccdfbd517a0fd631838d6252dbfa692
> 
> 
>     Note: bios_linker cannot be used to allocate the PPI memory region,
>     since the reserved memory should stay stable across reboots, and might
>     be needed before the ACPI tables are installed.
And did this mandate to use "ram_device" memory type instead of standard
system RAM.

As I understand the spec (statement above), the CRB areas can be
implemented as system RAM or device memory. So I want to understand why
using RAM device for the CRB is not a reasonable choice. By the way I
understand my motivation behind that change is a bit far-fetched and
aiming at fixing another issue, but well ;-)

Thanks

Eric
>  
> 
> 
> There are some spurious warnings when using CRB with VFIO and that would
> be nice to remove them one way or the other.
> 
> Thanks
> 
> Eric
> >
> > Thanks
> >
> > Eric
> >>
> >>
> >>
> >
> 
> 
> 
> 
> -- 
> Marc-André Lureau

[PATCH v4 0/4] Enable vhost-user to be used on BSD systems

[Sergio Lopez]

Since QEMU is already able to emulate ioeventfd using pipefd, we're already
pretty close to supporting vhost-user on non-Linux systems.

This two patches bridge the gap by:

1. Adding a new event_notifier_get_wfd() to return wfd on the places where
   the peer is expected to write to the notifier.

2. Modifying the build system to it allows enabling vhost-user on BSD.

v1->v2:
  - Drop: "Allow returning EventNotifier's wfd" (Alex Williamson)
  - Add: "event_notifier: add event_notifier_get_wfd()" (Alex Williamson)
  - Add: "vhost: use wfd on functions setting vring call fd"
  - Rename: "Allow building vhost-user in BSD" to "configure, meson: allow
enabling vhost-user on all POSIX systems"
  - Instead of making possible enabling vhost-user on Linux and BSD systems,
allow enabling it on all non-Windows platforms. (Paolo Bonzini)

v2->v3:
  - Add a section to docs/interop/vhost-user.rst explaining how vhost-user
is supported on non-Linux platforms. (Stefan Hajnoczi)

v3->v4:
  - Some documentation fixes. (Stefan Hajnoczi)
  - Pick up Reviewed-by tags.

Sergio Lopez (4):
  event_notifier: add event_notifier_get_wfd()
  vhost: use wfd on functions setting vring call fd
  configure, meson: allow enabling vhost-user on all POSIX systems
  docs: vhost-user: add subsection for non-Linux platforms

 configure |  4 ++--
 docs/interop/vhost-user.rst   | 20 
 hw/virtio/vhost.c |  6 +++---
 include/qemu/event_notifier.h |  1 +
 meson.build   |  2 +-
 util/event_notifier-posix.c   |  5 +
 6 files changed, 32 insertions(+), 6 deletions(-)

-- 
2.35.1

[PATCH v4 1/4] event_notifier: add event_notifier_get_wfd()

[Sergio Lopez]

event_notifier_get_fd(const EventNotifier *e) always returns
EventNotifier's read file descriptor (rfd). This is not a problem when
the EventNotifier is backed by a an eventfd, as a single file
descriptor is used both for reading and triggering events (rfd ==
wfd).

But, when EventNotifier is backed by a pipe pair, we have two file
descriptors, one that can only be used for reads (rfd), and the other
only for writes (wfd).

There's, at least, one known situation in which we need to obtain wfd
instead of rfd, which is when setting up the file that's going to be
sent to the peer in vhost's SET_VRING_CALL.

Add a new event_notifier_get_wfd(const EventNotifier *e) that can be
used to obtain wfd where needed.

Signed-off-by: Sergio Lopez 
Reviewed-by: Stefan Hajnoczi 
---
 include/qemu/event_notifier.h | 1 +
 util/event_notifier-posix.c   | 5 +
 2 files changed, 6 insertions(+)

diff --git a/include/qemu/event_notifier.h b/include/qemu/event_notifier.h
index b79add035d..8a4ff308e1 100644
--- a/include/qemu/event_notifier.h
+++ b/include/qemu/event_notifier.h
@@ -38,6 +38,7 @@ int event_notifier_test_and_clear(EventNotifier *);
 #ifdef CONFIG_POSIX
 void event_notifier_init_fd(EventNotifier *, int fd);
 int event_notifier_get_fd(const EventNotifier *);
+int event_notifier_get_wfd(const EventNotifier *);
 #else
 HANDLE event_notifier_get_handle(EventNotifier *);
 #endif
diff --git a/util/event_notifier-posix.c b/util/event_notifier-posix.c
index 8307013c5d..16294e98d4 100644
--- a/util/event_notifier-posix.c
+++ b/util/event_notifier-posix.c
@@ -99,6 +99,11 @@ int event_notifier_get_fd(const EventNotifier *e)
 return e->rfd;
 }
 
+int event_notifier_get_wfd(const EventNotifier *e)
+{
+return e->wfd;
+}
+
 int event_notifier_set(EventNotifier *e)
 {
 static const uint64_t value = 1;
-- 
2.35.1

[PATCH v4 2/4] vhost: use wfd on functions setting vring call fd

[Sergio Lopez]

When ioeventfd is emulated using qemu_pipe(), only EventNotifier's wfd
can be used for writing.

Use the recently introduced event_notifier_get_wfd() function to
obtain the fd that our peer must use to signal the vring.

Signed-off-by: Sergio Lopez 
Reviewed-by: Stefan Hajnoczi 
---
 hw/virtio/vhost.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 7b03efccec..b643f42ea4 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1287,7 +1287,7 @@ static int vhost_virtqueue_init(struct vhost_dev *dev,
 return r;
 }
 
-file.fd = event_notifier_get_fd(&vq->masked_notifier);
+file.fd = event_notifier_get_wfd(&vq->masked_notifier);
 r = dev->vhost_ops->vhost_set_vring_call(dev, &file);
 if (r) {
 VHOST_OPS_DEBUG(r, "vhost_set_vring_call failed");
@@ -1542,9 +1542,9 @@ void vhost_virtqueue_mask(struct vhost_dev *hdev, 
VirtIODevice *vdev, int n,
 
 if (mask) {
 assert(vdev->use_guest_notifier_mask);
-file.fd = event_notifier_get_fd(&hdev->vqs[index].masked_notifier);
+file.fd = event_notifier_get_wfd(&hdev->vqs[index].masked_notifier);
 } else {
-file.fd = event_notifier_get_fd(virtio_queue_get_guest_notifier(vvq));
+file.fd = event_notifier_get_wfd(virtio_queue_get_guest_notifier(vvq));
 }
 
 file.index = hdev->vhost_ops->vhost_get_vq_index(hdev, n);
-- 
2.35.1

[PATCH v4 3/4] configure, meson: allow enabling vhost-user on all POSIX systems

[Sergio Lopez]

With the possibility of using a pipe pair via qemu_pipe() as a
replacement on operating systems that doesn't support eventfd,
vhost-user can also work on all POSIX systems.

This change allows enabling vhost-user on all non-Windows platforms
and makes libvhost_user (which still depends on eventfd) a linux-only
feature.

Signed-off-by: Sergio Lopez 
Reviewed-by: Stefan Hajnoczi 
---
 configure   | 4 ++--
 meson.build | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/configure b/configure
index c56ed53ee3..daccf4be7c 100755
--- a/configure
+++ b/configure
@@ -1659,8 +1659,8 @@ fi
 # vhost interdependencies and host support
 
 # vhost backends
-if test "$vhost_user" = "yes" && test "$linux" != "yes"; then
-  error_exit "vhost-user is only available on Linux"
+if test "$vhost_user" = "yes" && test "$mingw32" = "yes"; then
+  error_exit "vhost-user is not available on Windows"
 fi
 test "$vhost_vdpa" = "" && vhost_vdpa=$linux
 if test "$vhost_vdpa" = "yes" && test "$linux" != "yes"; then
diff --git a/meson.build b/meson.build
index 8df40bfac4..f2bc439c30 100644
--- a/meson.build
+++ b/meson.build
@@ -2701,7 +2701,7 @@ if have_system or have_user
 endif
 
 vhost_user = not_found
-if 'CONFIG_VHOST_USER' in config_host
+if targetos == 'linux' and 'CONFIG_VHOST_USER' in config_host
   libvhost_user = subproject('libvhost-user')
   vhost_user = libvhost_user.get_variable('vhost_user_dep')
 endif
-- 
2.35.1

[PATCH v4 4/4] docs: vhost-user: add subsection for non-Linux platforms

[Sergio Lopez]

Add a section explaining how vhost-user is supported on platforms
other than Linux.

Signed-off-by: Sergio Lopez 
Reviewed-by: Stefan Hajnoczi 
---
 docs/interop/vhost-user.rst | 20 
 1 file changed, 20 insertions(+)

diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
index edc3ad84a3..4dbc84fd00 100644
--- a/docs/interop/vhost-user.rst
+++ b/docs/interop/vhost-user.rst
@@ -38,6 +38,26 @@ conventions `.
 *Master* and *slave* can be either a client (i.e. connecting) or
 server (listening) in the socket communication.
 
+Support for platforms other than Linux
+--
+
+While vhost-user was initially developed targeting Linux, nowadays it
+is supported on any platform that provides the following features:
+
+- A way for requesting shared memory represented by a file descriptor
+  so it can be passed over a UNIX domain socket and then mapped by the
+  other process.
+
+- AF_UNIX sockets with SCM_RIGHTS, so QEMU and the other process can
+  exchange messages through it, including ancillary data when needed.
+
+- Either eventfd or pipe/pipe2. On platforms where eventfd is not
+  available, QEMU will automatically fall back to pipe2 or, as a last
+  resort, pipe. Each file descriptor will be used for receiving or
+  sending events by reading or writing (respectively) an 8-byte value
+  to the corresponding it. The 8-value itself has no meaning and
+  should not be interpreted.
+
 Message Specification
 =
 
-- 
2.35.1

Re: [PATCH v2 01/10] macfb: add VMStateDescription for MacfbNubusState and MacfbSysBusState

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

Currently when QEMU tries to migrate the macfb framebuffer it crashes randomly
because the opaque provided by the DeviceClass vmsd property for both devices
is set to MacfbState rather than MacfbNubusState or MacfbSysBusState as
appropriate.

Resolve the issue by adding new VMStateDescriptions for MacfbNubusState and
MacfbSysBusState which embed the existing vmstate_macfb VMStateDescription
within them using VMSTATE_STRUCT.

Signed-off-by: Mark Cave-Ayland 
---
  hw/display/macfb.c | 24 ++--
  1 file changed, 22 insertions(+), 2 deletions(-)



Reviewed-by: Laurent Vivier 

Re: [PATCH v2 1/4] os-posix: Add os_set_daemonize()

[Kevin Wolf]

Am 04.03.2022 um 10:19 hat Daniel P. Berrangé geschrieben:
> On Thu, Mar 03, 2022 at 05:48:11PM +0100, Hanna Reitz wrote:
> > The daemonizing functions in os-posix (os_daemonize() and
> > os_setup_post()) only daemonize the process if the static `daemonize`
> > variable is set.  Right now, it can only be set by os_parse_cmd_args().
> > 
> > In order to use os_daemonize() and os_setup_post() from the storage
> > daemon to have it be daemonized, we need some other way to set this
> > `daemonize` variable, because I would rather not tap into the system
> > emulator's arg-parsing code.  Therefore, this patch adds an
> > os_set_daemonize() function, which will return an error on os-win32
> > (because daemonizing is not supported there).
> 
> IMHO the real flaw here is the design of 'os_daemonize' in that it
> relies on static state. If I see a call to a function 'os_daemonize()'
> I expect to be daemonized on return, but with this design that is not
> guaranteed which is a big surprise.
> 
> I'd suggest we push the condition into the caller instead of adding
> this extra function, so we have the more sane pattern:
> 
>if (daemonmize()) {
>   os_daemonize()
>}

It's not as simple, the static daemonize variable is used in more places
than just os_daemonize(). I'm not sure if it's worth changing how all of
this works, but if we did, it would be a refactoring mostly focussed on
the system emulator and an issue separate from adding the option to the
storage daemon.

Kevin

Re: [PATCH v6 00/16] Make image fleecing more usable

[Hanna Reitz]

On 03.03.22 20:43, Vladimir Sementsov-Ogievskiy wrote:

v6:
11: add comment
15: limit to qcow2 with unsupported compat
 fix style
16: fix style
 change log('Backup finished ...') to assertion and comment


Thanks a lot!

Applied to my block branch:

https://gitlab.com/hreitz/qemu/-/commits/block

Re: [PATCH vRESEND] virtio/virtio-balloon: Prefer Object* over void* parameter

[Michael S. Tsirkin]

On Tue, Mar 01, 2022 at 11:23:01PM +0100, Bernhard Beschow wrote:
> *opaque is an alias to *obj. Using the ladder makes the code consistent with
> with other devices, e.g. accel/kvm/kvm-all and accel/tcg/tcg-all. It also
> makes the cast more typesafe.
> 
> Signed-off-by: Bernhard Beschow 
> Reviewed-by: David Hildenbrand 

Acked-by: Michael S. Tsirkin 

trivial tree pls

> ---
>  hw/virtio/virtio-balloon.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index e6c1b0aa46..163d244eb4 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -242,7 +242,7 @@ static void balloon_stats_get_all(Object *obj, Visitor 
> *v, const char *name,
>void *opaque, Error **errp)
>  {
>  Error *err = NULL;
> -VirtIOBalloon *s = opaque;
> +VirtIOBalloon *s = VIRTIO_BALLOON(obj);
>  int i;
>  
>  if (!visit_start_struct(v, name, NULL, 0, &err)) {
> @@ -277,7 +277,7 @@ static void balloon_stats_get_poll_interval(Object *obj, 
> Visitor *v,
>  const char *name, void *opaque,
>  Error **errp)
>  {
> -VirtIOBalloon *s = opaque;
> +VirtIOBalloon *s = VIRTIO_BALLOON(obj);
>  visit_type_int(v, name, &s->stats_poll_interval, errp);
>  }
>  
> @@ -285,7 +285,7 @@ static void balloon_stats_set_poll_interval(Object *obj, 
> Visitor *v,
>  const char *name, void *opaque,
>  Error **errp)
>  {
> -VirtIOBalloon *s = opaque;
> +VirtIOBalloon *s = VIRTIO_BALLOON(obj);
>  int64_t value;
>  
>  if (!visit_type_int(v, name, &value, errp)) {
> @@ -1015,12 +1015,12 @@ static void virtio_balloon_instance_init(Object *obj)
>  s->free_page_hint_notify.notify = virtio_balloon_free_page_hint_notify;
>  
>  object_property_add(obj, "guest-stats", "guest statistics",
> -balloon_stats_get_all, NULL, NULL, s);
> +balloon_stats_get_all, NULL, NULL, NULL);
>  
>  object_property_add(obj, "guest-stats-polling-interval", "int",
>  balloon_stats_get_poll_interval,
>  balloon_stats_set_poll_interval,
> -NULL, s);
> +NULL, NULL);
>  }
>  
>  static const VMStateDescription vmstate_virtio_balloon = {
> -- 
> 2.35.1

Re: [PATCH v2 03/10] macfb: increase number of registers saved in MacfbState

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

The MacOS toolbox ROM accesses a number of addresses between 0x0 and 0x200 
during
initialisation and resolution changes. Whilst the function of many of these
registers is unknown, it is worth the minimal cost of saving these extra values 
as
part of migration to help future-proof the migration stream for the q800 machine
as it starts to stabilise.

Signed-off-by: Mark Cave-Ayland 
---
  hw/display/macfb.c | 8 
  include/hw/display/macfb.h | 3 ++-
  2 files changed, 10 insertions(+), 1 deletion(-)



Reviewed-by: Laurent Vivier 

Re: [PATCH 4/9] util/oslib-win32: Return NULL on qemu_try_memalign() with zero size

[Peter Maydell]

On Thu, 3 Mar 2022 at 23:02, Richard Henderson
 wrote:
>
> On 3/3/22 06:55, Peter Maydell wrote:
> >> Alternately, force size == 1, so that we always get a non-NULL value that 
> >> can be freed.
> >> That's a change on the POSIX side as well, of course.
> >
> > Yes, I had a look at what actual malloc() implementations tend
> > to do, and the answer seems to be that forcing size to 1 gives
> > less weird behaviour for the application. So here that would be
> >
> > if (size == 0) {
> > size++;
> > }
> > ptr = _aligned_malloc(size, alignment);
> >
> > We don't need to do anything on the POSIX side (unless we want to
> > enforce consistency of handling the size==0 case).
>
> I would do this unconditionally.  The POSIX manpage says that either NULL or 
> a unique
> pointer is a valid return value into *memptr here for size == 0.  What we 
> want in our
> caller is NULL if and only if error.

Mm, I guess. I was trying to avoid changing the POSIX-side behaviour,
but this seems safe enough.

-- PMM

Re: [PATCH v4 0/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Michael S. Tsirkin]

On Mon, Feb 28, 2022 at 10:17:30PM +0200, Liav Albani wrote:
> This can allow the guest OS to determine more easily if i8042 controller
> is present in the system or not, so it doesn't need to do probing of the
> controller, but just initialize it immediately, before enumerating the
> ACPI AML namespace.
> 
> To allow "flexible" indication, I don't hardcode the bit at location 1
> as on in the IA-PC boot flags, but try to search for i8042 on the ISA
> bus to verify it exists in the system.
> 
> Why this is useful you might ask - this patch allows the guest OS to
> probe and use the i8042 controller without decoding the ACPI AML blob
> at all. For example, as a developer of the SerenityOS kernel, I might
> want to allow people to not try to decode the ACPI AML namespace (for
> now, we still don't support ACPI AML as it's a work in progress), but
> still to not probe for the i8042 but just use it after looking in the
> IA-PC boot flags in the ACPI FADT table.

OK still waiting for v5.

> Liav Albani (3):
>   tests/acpi: i386: allow FACP acpi table changes
>   hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT
> table
>   tests/acpi: i386: update FACP table differences
> 
>  hw/acpi/aml-build.c|  11 ++-
>  hw/i386/acpi-build.c   |   9 +
>  hw/i386/acpi-microvm.c |   9 +
>  include/hw/acpi/acpi-defs.h|   1 +
>  tests/data/acpi/q35/FACP   | Bin 244 -> 244 bytes
>  tests/data/acpi/q35/FACP.nosmm | Bin 244 -> 244 bytes
>  tests/data/acpi/q35/FACP.slic  | Bin 244 -> 244 bytes
>  tests/data/acpi/q35/FACP.xapic | Bin 244 -> 244 bytes
>  8 files changed, 29 insertions(+), 1 deletion(-)
> 
> -- 
> 2.35.1

Re: [PATCH v2 04/10] macfb: add VMStateDescription fields for display type and VBL timer

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

These fields are required in the migration stream to restore macfb state
correctly.

Signed-off-by: Mark Cave-Ayland 
---
  hw/display/macfb.c | 2 ++
  1 file changed, 2 insertions(+)



Reviewed-by: Laurent Vivier 

Re: [PATCH v2 02/10] macfb: don't use special irq_state and irq_mask variables in MacfbState

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

The current IRQ state and IRQ mask are handled exactly the same as standard
register accesses, so store these values directly in the regs array rather
than having separate variables for them.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
  hw/display/macfb.c | 15 +++
  include/hw/display/macfb.h |  2 --
  2 files changed, 7 insertions(+), 10 deletions(-)


Reviewed-by: Laurent Vivier 

Re: [PATCH v2 05/10] macfb: set initial value of mode control registers in macfb_common_realize()

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

If booting Linux directly in the q800 machine using -kernel rather than using a
MacOS toolbox ROM, the mode control registers are never initialised,
causing macfb_mode_write() to fail to determine the current resolution after
migration. Resolve this by always setting the initial values of the mode control
registers based upon the initial macfb properties during realize.

Signed-off-by: Mark Cave-Ayland 
---
  hw/display/macfb.c | 8 
  1 file changed, 8 insertions(+)


Reviewed-by: Laurent Vivier 

Re: [PATCH v2 07/10] esp: introduce esp_pdma_cb() function

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

This function is to be used to execute the current PDMA callback rather than
dereferencing the ESPState pdma_cb function pointer directly.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
  hw/scsi/esp.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)



Reviewed-by: Laurent Vivier 

Re: [PATCH v2 08/10] esp: convert ESPState pdma_cb from a function pointer to an integer

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

This prepares for the inclusion of the current PDMA callback in the migration
stream since the callback is referenced by an integer instead of a function
pointer.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
  hw/scsi/esp.c | 44 ++-
  include/hw/scsi/esp.h | 11 ++-
  2 files changed, 41 insertions(+), 14 deletions(-)



Reviewed-by: Laurent Vivier 

Re: [PATCH v2 06/10] esp: introduce esp_set_pdma_cb() function

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

This function is to be used to set the current PDMA callback rather than
accessing the ESPState pdma_cb function pointer directly.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
---
  hw/scsi/esp.c | 19 ---
  1 file changed, 12 insertions(+), 7 deletions(-)


Reviewed-by: Laurent Vivier 

Re: [PULL 00/13] riscv-to-apply queue

[Peter Maydell]

On Thu, 3 Mar 2022 at 05:31, Alistair Francis
 wrote:
>
> From: Alistair Francis 
>
> The following changes since commit 64ada298b98a51eb2512607f6e6180cb330c47b1:
>
>   Merge remote-tracking branch 'remotes/legoater/tags/pull-ppc-20220302' into 
> staging (2022-03-02 12:38:46 +)
>
> are available in the Git repository at:
>
>   g...@github.com:alistair23/qemu.git tags/pull-riscv-to-apply-20220303
>
> for you to fetch changes up to 6b1accefd4876ea5475d55454c7d5b52c02cb73c:
>
>   target/riscv: expose zfinx, zdinx, zhinx{min} properties (2022-03-03 
> 13:14:50 +1000)
>
> 
> Fifth RISC-V PR for QEMU 7.0
>
>  * Fixup checks for ext_zb[abcs]
>  * Add AIA support for virt machine
>  * Increase maximum number of CPUs in virt machine
>  * Fixup OpenTitan SPI address
>  * Add support for zfinx, zdinx and zhinx{min} extensions
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM

Re: [PATCH v2 10/10] esp: recreate ESPState current_req after migration

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

Since PDMA reads/writes are driven by the guest, it is possible that migration
can occur whilst a SCSIRequest is still active. Fortunately active SCSIRequests
are already included in the migration stream and restarted post migration but
this still leaves the reference in ESPState uninitialised.

Implement the SCSIBusInfo .load_request callback to obtain a reference to the
currently active SCSIRequest and use it to recreate ESPState current_req
after migration.

Suggested-by: Paolo Bonzini 
Signed-off-by: Mark Cave-Ayland 
---
  hw/scsi/esp.c | 10 ++
  1 file changed, 10 insertions(+)



Reviewed-by: Laurent Vivier 

Re: [PATCH v2 09/10] esp: include the current PDMA callback in the migration stream

[Laurent Vivier]

Le 02/03/2022 à 22:27, Mark Cave-Ayland a écrit :

This involves (re)adding a PDMA-specific subsection to hold the reference to the
current PDMA callback.

Signed-off-by: Mark Cave-Ayland 
---
  hw/scsi/esp.c | 23 +++
  1 file changed, 23 insertions(+)



Reviewed-by: Laurent Vivier 

Re: [PATCH v4 0/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Ani Sinha]

On Fri, Mar 4, 2022 at 3:56 PM Michael S. Tsirkin  wrote:
>
> On Mon, Feb 28, 2022 at 10:17:30PM +0200, Liav Albani wrote:
> > This can allow the guest OS to determine more easily if i8042 controller
> > is present in the system or not, so it doesn't need to do probing of the
> > controller, but just initialize it immediately, before enumerating the
> > ACPI AML namespace.
> >
> > To allow "flexible" indication, I don't hardcode the bit at location 1
> > as on in the IA-PC boot flags, but try to search for i8042 on the ISA
> > bus to verify it exists in the system.
> >
> > Why this is useful you might ask - this patch allows the guest OS to
> > probe and use the i8042 controller without decoding the ACPI AML blob
> > at all. For example, as a developer of the SerenityOS kernel, I might
> > want to allow people to not try to decode the ACPI AML namespace (for
> > now, we still don't support ACPI AML as it's a work in progress), but
> > still to not probe for the i8042 but just use it after looking in the
> > IA-PC boot flags in the ACPI FADT table.
>
> OK still waiting for v5.

Since the time is tight, I could quickly make the changes in patch 2
and send it over. I believe 8th is the last date for new changes.

Re: [PATCH v3 4/4] docs: vhost-user: add subsection for non-Linux platforms

[Michael S. Tsirkin]

On Thu, Mar 03, 2022 at 12:59:11PM +0100, Sergio Lopez wrote:
> Add a section explaining how vhost-user is supported on platforms
> other than Linux.
> 
> Signed-off-by: Sergio Lopez 
> ---
>  docs/interop/vhost-user.rst | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> index edc3ad84a3..590a626b92 100644
> --- a/docs/interop/vhost-user.rst
> +++ b/docs/interop/vhost-user.rst
> @@ -38,6 +38,24 @@ conventions `.
>  *Master* and *slave* can be either a client (i.e. connecting) or
>  server (listening) in the socket communication.
>  
> +Support for platforms other than Linux


It's not just Linux - any platform without eventfd.

So I think we should have a section explaining that whereever
spec says eventfd it can be a pipe if system does not
support creating eventfd.

> +--
> +
> +While vhost-user was initially developed targeting Linux, nowadays is
> +supported on any platform that provides the following features:
> +
> +- The ability to share a mapping injected into the guest between
> +  multiple processes, so both QEMU and the vhost-user daemon servicing
> +  the device can access simultaneously the memory regions containing
> +  the virtqueues and the data associated with each request.
> +
> +- AF_UNIX sockets with SCM_RIGHTS, so QEMU can communicate with the
> +  vhost-user daemon and send it file descriptors when needed.
> +
> +- Either eventfd or pipe/pipe2. On platforms where eventfd is not
> +  available, QEMU will automatically fallback to pipe2 or, as a last
> +  resort, pipe.
> +
>  Message Specification
>  =
>  
> -- 
> 2.35.1

Re: [PATCH v4 0/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Michael S. Tsirkin]

On Fri, Mar 04, 2022 at 04:04:13PM +0530, Ani Sinha wrote:
> On Fri, Mar 4, 2022 at 3:56 PM Michael S. Tsirkin  wrote:
> >
> > On Mon, Feb 28, 2022 at 10:17:30PM +0200, Liav Albani wrote:
> > > This can allow the guest OS to determine more easily if i8042 controller
> > > is present in the system or not, so it doesn't need to do probing of the
> > > controller, but just initialize it immediately, before enumerating the
> > > ACPI AML namespace.
> > >
> > > To allow "flexible" indication, I don't hardcode the bit at location 1
> > > as on in the IA-PC boot flags, but try to search for i8042 on the ISA
> > > bus to verify it exists in the system.
> > >
> > > Why this is useful you might ask - this patch allows the guest OS to
> > > probe and use the i8042 controller without decoding the ACPI AML blob
> > > at all. For example, as a developer of the SerenityOS kernel, I might
> > > want to allow people to not try to decode the ACPI AML namespace (for
> > > now, we still don't support ACPI AML as it's a work in progress), but
> > > still to not probe for the i8042 but just use it after looking in the
> > > IA-PC boot flags in the ACPI FADT table.
> >
> > OK still waiting for v5.
> 
> Since the time is tight, I could quickly make the changes in patch 2
> and send it over.

Sure.

> I believe 8th is the last date for new changes.

Yes.

-- 
MST

Re: [PATCH V7 10/29] machine: memfd-alloc option

[Igor Mammedov]

On Thu, 3 Mar 2022 12:21:15 -0500
"Michael S. Tsirkin"  wrote:

> On Wed, Dec 22, 2021 at 11:05:15AM -0800, Steve Sistare wrote:
> > Allocate anonymous memory using memfd_create if the memfd-alloc machine
> > option is set.
> > 
> > Signed-off-by: Steve Sistare 
> > ---
> >  hw/core/machine.c   | 19 +++
> >  include/hw/boards.h |  1 +
> >  qemu-options.hx |  6 ++
> >  softmmu/physmem.c   | 47 ++-
> >  softmmu/vl.c|  1 +
> >  trace-events|  1 +
> >  util/qemu-config.c  |  4 
> >  7 files changed, 70 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 53a99ab..7739d88 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -392,6 +392,20 @@ static void machine_set_mem_merge(Object *obj, bool 
> > value, Error **errp)
> >  ms->mem_merge = value;
> >  }
> >  
> > +static bool machine_get_memfd_alloc(Object *obj, Error **errp)
> > +{
> > +MachineState *ms = MACHINE(obj);
> > +
> > +return ms->memfd_alloc;
> > +}
> > +
> > +static void machine_set_memfd_alloc(Object *obj, bool value, Error **errp)
> > +{
> > +MachineState *ms = MACHINE(obj);
> > +
> > +ms->memfd_alloc = value;
> > +}
> > +
> >  static bool machine_get_usb(Object *obj, Error **errp)
> >  {
> >  MachineState *ms = MACHINE(obj);
> > @@ -829,6 +843,11 @@ static void machine_class_init(ObjectClass *oc, void 
> > *data)
> >  object_class_property_set_description(oc, "mem-merge",
> >  "Enable/disable memory merge support");
> >  
> > +object_class_property_add_bool(oc, "memfd-alloc",
> > +machine_get_memfd_alloc, machine_set_memfd_alloc);
> > +object_class_property_set_description(oc, "memfd-alloc",
> > +"Enable/disable allocating anonymous memory using memfd_create");
> > +
> >  object_class_property_add_bool(oc, "usb",
> >  machine_get_usb, machine_set_usb);
> >  object_class_property_set_description(oc, "usb",
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 9c1c190..a57d7a0 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -327,6 +327,7 @@ struct MachineState {
> >  char *dt_compatible;
> >  bool dump_guest_core;
> >  bool mem_merge;
> > +bool memfd_alloc;
> >  bool usb;
> >  bool usb_disabled;
> >  char *firmware;
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 7d47510..33c8173 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -30,6 +30,7 @@ DEF("machine", HAS_ARG, QEMU_OPTION_machine, \
> >  "vmport=on|off|auto controls emulation of vmport 
> > (default: auto)\n"
> >  "dump-guest-core=on|off include guest memory in a core 
> > dump (default=on)\n"
> >  "mem-merge=on|off controls memory merge support 
> > (default: on)\n"
> > +"memfd-alloc=on|off controls allocating anonymous 
> > guest RAM using memfd_create (default: off)\n"  
> 
> Question: are there any disadvantages associated with using
> memfd_create? I guess we are using up an fd, but that seems minor.  Any
> reason not to set to on by default? maybe with a fallback option to
> disable that?
> 
> I am concerned that it's actually a kind of memory backend, this flag
> seems to instead be closer to the deprecated mem-prealloc. E.g.
> it does not work with a mem path, does it?

(mem path and mem-prealloc are transparently aliased to used memory backend
if I recall it right.)

Steve,

For allocating guest RAM, we switched exclusively to using memory-backends
including initial guest RAM (-m size option) and we have hostmem-memfd
that uses memfd_create() and I'd rather avoid adding random knobs to machine
for tweaking how RAM should be allocated, we have memory backends for this,
so this patch begs the question: why hostmem-memfd is not sufficient?
(patch description is rather lacking on rationale behind the patch)


> 
> 
> >  "aes-key-wrap=on|off controls support for AES key 
> > wrapping (default=on)\n"
> >  "dea-key-wrap=on|off controls support for DEA key 
> > wrapping (default=on)\n"
> >  "suppress-vmdesc=on|off disables self-describing 
> > migration (default=off)\n"
> > @@ -76,6 +77,11 @@ SRST
> >  supported by the host, de-duplicates identical memory pages
> >  among VMs instances (enabled by default).
> >  
> > +``memfd-alloc=on|off``
> > +Enables or disables allocation of anonymous guest RAM using
> > +memfd_create.  Any associated memory-backend objects are created 
> > with
> > +share=on.  The memfd-alloc default is off.
> > +
> >  ``aes-key-wrap=on|off``
> >  Enables or disables AES key wrapping support on s390-ccw hosts.
> >  This feature controls whether AES wrapping keys will be created
> > diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> > index 3524c04

Re: [PATCH 0/6] 9pfs: convert Doxygen -> kerneldoc format

[Christian Schoenebeck]

On Freitag, 4. März 2022 08:39:36 CET Greg Kurz wrote:
> On Thu, 3 Mar 2022 14:40:56 +0100
> 
> Christian Schoenebeck  wrote:
> > This patch set converts occurrences of API doc comments from Doxygen
> > format
> > into kerneldoc format. No behaviour change whatsoever.
> > 
> > Christian Schoenebeck (6):
> >   9pfs/9p.h: convert Doxygen -> kerneldoc format
> >   9pfs/codir.c: convert Doxygen -> kerneldoc format
> >   9pfs/9p.c: convert Doxygen -> kerneldoc format
> >   9pfs/9p-util.h: convert Doxygen -> kerneldoc format
> >   9pfs/coth.h: drop Doxygen format on v9fs_co_run_in_worker()
> >   fsdev/p9array.h: convert Doxygen -> kerneldoc format
> >  
> >  fsdev/p9array.h   | 38 -
> >  hw/9pfs/9p-util.h | 10 
> >  hw/9pfs/9p.c  | 62 ++-
> >  hw/9pfs/9p.h  | 12 -
> >  hw/9pfs/codir.c   | 30 +++
> >  hw/9pfs/coth.h|  4 +--
> >  6 files changed, 84 insertions(+), 72 deletions(-)
> 
> LGTM.
> 
> Reviewed-by: Greg Kurz 

Great, then I can take this into today's PR as well.

Thanks!

Best regards,
Christian Schoenebeck

Re: any opinion on the patch "[RFC PATCH] hw/i386/e820: remove legacy reserved entries for e820"? EOM

[Ani Sinha]

On Fri, Mar 4, 2022 at 3:48 PM Gerd Hoffmann  wrote:
>
> On Thu, Mar 03, 2022 at 03:12:51PM +0530, Ani Sinha wrote:
> > On Thu, Mar 3, 2022 at 15:11 Gerd Hoffmann  wrote:
> >
> > > Sorry, no.  Noticed the discussions but don't remember the details and
> > > didn't took the time to wade through the code to refresh my memory.
> >
> > Could you please take a look when you get time? You have the most context
> > in this space I believe.
>
> Should indeed not be needed unless you use a stone-aged seabios version.
> But I think you can't simply drop it for live migration compatibility
> reasons.  So you'll need do the compatibility dance and drop it for new
> machine types only.  I doubt the benefits outweigh that effort ..

Igor what do you think?

Re: [PATCH v4 12/14] add sysbus-mmio-map qapi command

[Damien Hedde]

On 3/3/22 15:59, Philippe Mathieu-Daudé wrote:

On 23/2/22 10:07, Damien Hedde wrote:

This command allows to map an mmio region of sysbus device onto
the system memory. Its behavior mimics the sysbus_mmio_map()
function apart from the automatic unmap (the C function unmaps
the region if it is already mapped).
For the qapi function we consider it is an error to try to map
an already mapped function. If unmapping is required, it is
probably better to add a sysbus-mmip-unmap command.


"sysbus-mmio-unmap" typo I presume.


This command is still experimental (hence the 'unstable' feature),
as it is related to the sysbus device creation through qapi commands.

This command is required to be able to dynamically build a machine
from scratch as there is no qapi-way of doing a memory mapping.

Signed-off-by: Damien Hedde 
---
Cc: Alistair Francis 

v4:
  + integrate priority parameter
  + use 'unstable' feature flag instead of 'x-' prefix
  + bump version to 7.0
  + dropped Alistair's reviewed-by as a consequence
---
  qapi/qdev.json   | 31 ++
  hw/core/sysbus.c | 49 
  2 files changed, 80 insertions(+)

diff --git a/qapi/qdev.json b/qapi/qdev.json
index 2e2de41499..4830e87a90 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -160,3 +160,34 @@
  ##
  { 'event': 'DEVICE_UNPLUG_GUEST_ERROR',
    'data': { '*device': 'str', 'path': 'str' } }
+
+##
+# @sysbus-mmio-map:
+#
+# Map a sysbus device mmio onto the main system bus.
+#
+# @device: the device's QOM path
+#
+# @mmio: The mmio number to be mapped (defaults to 0).
+#
+# @addr: The base address for the mapping.
+#
+# @priority: The priority of the mapping (defaults to 0).
+#
+# Features:
+# @unstable: Command is meant to map sysbus devices
+#    while in preconfig mode.
+#
+# Since: 7.0
+#
+# Returns: Nothing on success
+#
+##
+
+{ 'command': 'sysbus-mmio-map',
+  'data': { 'device': 'str',
+    '*mmio': 'uint8',
+    'addr': 'uint64',
+    '*priority': 'int32' },


I wonder if not providing the explicit parent container now could
be problematic later, and if we shouldn't start with a QOM MR path
(default to 'system_memory'). Anyway, sysbus are currently
restricted to system_memory so as you described, this mimics well
sysbus_mmio_map().


I think we ended-up adding such a parameter to handle complex xilinx 
machines having several cpu clusters. I wanted to stay simple in this 
series here as there are probably several way to address this issue. (we 
could also add a bus parameter, and create more sysbus).
We can still add the parameter later, with an appropriate default value 
(or even make the parameter mandatory).


If everybody agree to go for the bus-less approach. I can add the MR 
parameter right now.


CCing Igor


+void qmp_sysbus_mmio_map(const char *device,
+ bool has_mmio, uint8_t mmio,
+ uint64_t addr,
+ bool has_priority, int32_t priority,
+ Error **errp)
+{
+    Object *obj = object_resolve_path_type(device, 
TYPE_SYS_BUS_DEVICE, NULL);

+    SysBusDevice *dev;
+
+    if (phase_get() != PHASE_MACHINE_INITIALIZED) {
+    error_setg(errp, "The command is permitted only when "
+ "the machine is in initialized phase");


"command only permitted during the " #PHASE_MACHINE_INITIALIZED "phase"?


What do you mean by '#', this is not a macro parameter. 
PHASE_MACHINE_INITIALIZED is just an enum value and there is no 
human/qapi exposed name.

"when the machine is initialized/initializing" ?
I think all the machine phase error message are bit like that (not 
mentioning the phase).

Re: [PATCH v3 0/4] Enable vhost-user to be used on BSD systems

[Michael S. Tsirkin]

On Thu, Mar 03, 2022 at 12:59:07PM +0100, Sergio Lopez wrote:
> Since QEMU is already able to emulate ioeventfd using pipefd, we're already
> pretty close to supporting vhost-user on non-Linux systems.
> 
> This two patches bridge the gap by:
> 
> 1. Adding a new event_notifier_get_wfd() to return wfd on the places where
>the peer is expected to write to the notifier.
> 
> 2. Modifying the build system to it allows enabling vhost-user on BSD.
> 
> v1->v2:
>   - Drop: "Allow returning EventNotifier's wfd" (Alex Williamson)
>   - Add: "event_notifier: add event_notifier_get_wfd()" (Alex Williamson)
>   - Add: "vhost: use wfd on functions setting vring call fd"
>   - Rename: "Allow building vhost-user in BSD" to "configure, meson: allow
> enabling vhost-user on all POSIX systems"
>   - Instead of making possible enabling vhost-user on Linux and BSD systems,
> allow enabling it on all non-Windows platforms. (Paolo Bonzini)


I picked 1,2.
Waiting on updated doc patch to apply 3,4.

> v2->v3:
>   - Add a section to docs/interop/vhost-user.rst explaining how vhost-user
> is supported on non-Linux platforms. (Stefan Hajnoczi)
> 
> Sergio Lopez (4):
>   event_notifier: add event_notifier_get_wfd()
>   vhost: use wfd on functions setting vring call fd
>   configure, meson: allow enabling vhost-user on all POSIX systems
>   docs: vhost-user: add subsection for non-Linux platforms
> 
>  configure |  4 ++--
>  docs/interop/vhost-user.rst   | 18 ++
>  hw/virtio/vhost.c |  6 +++---
>  include/qemu/event_notifier.h |  1 +
>  meson.build   |  2 +-
>  util/event_notifier-posix.c   |  5 +
>  6 files changed, 30 insertions(+), 6 deletions(-)
> 
> -- 
> 2.35.1
> 

Re: [PATCH v2 0/4] qsd: Add --daemonize; and add job quit tests

[Kevin Wolf]

Am 03.03.2022 um 17:48 hat Hanna Reitz geschrieben:
> Hi,
> 
> v1 cover letter:
> 
> https://lists.nongnu.org/archive/html/qemu-block/2021-12/msg00499.html
> 
> 
> In v2, I followed Vladimir’s suggestion to look into whether we could
> reuse os_daemonize().  Indeed we can, and it makes patch 3 (formerly 2)
> much simpler!
> 
> I decided to leave patch 2 (formerly 1) largely unchanged, because it
> seems to me like the point of contention is whether it’s at all
> reasonable to introduce a second argument pass for this feature, and not
> e.g. which arguments we parse during it.
> I believe such an additional pass is a necessity for --daemonize, so
> either we really don’t want this pass and so cannot add this feature
> (and just drop this series); or we do want this feature, and then we
> have to add this pass.

Thanks, fixed up as discussed on IRC to address the two minor comments
from Eric and myself, and applied to the block branch.

Kevin

Re: [PATCH v3 4/4] docs: vhost-user: add subsection for non-Linux platforms

[Sergio Lopez]

On Fri, Mar 04, 2022 at 05:35:01AM -0500, Michael S. Tsirkin wrote:
> On Thu, Mar 03, 2022 at 12:59:11PM +0100, Sergio Lopez wrote:
> > Add a section explaining how vhost-user is supported on platforms
> > other than Linux.
> > 
> > Signed-off-by: Sergio Lopez 
> > ---
> >  docs/interop/vhost-user.rst | 18 ++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst
> > index edc3ad84a3..590a626b92 100644
> > --- a/docs/interop/vhost-user.rst
> > +++ b/docs/interop/vhost-user.rst
> > @@ -38,6 +38,24 @@ conventions `.
> >  *Master* and *slave* can be either a client (i.e. connecting) or
> >  server (listening) in the socket communication.
> >  
> > +Support for platforms other than Linux
> 
> 
> It's not just Linux - any platform without eventfd.
> 
> So I think we should have a section explaining that whereever
> spec says eventfd it can be a pipe if system does not
> support creating eventfd.

I'm confused. This is exactly what this subsection intends to do...

Thanks,
Sergio.

> > +--
> > +
> > +While vhost-user was initially developed targeting Linux, nowadays is
> > +supported on any platform that provides the following features:
> > +
> > +- The ability to share a mapping injected into the guest between
> > +  multiple processes, so both QEMU and the vhost-user daemon servicing
> > +  the device can access simultaneously the memory regions containing
> > +  the virtqueues and the data associated with each request.
> > +
> > +- AF_UNIX sockets with SCM_RIGHTS, so QEMU can communicate with the
> > +  vhost-user daemon and send it file descriptors when needed.
> > +
> > +- Either eventfd or pipe/pipe2. On platforms where eventfd is not
> > +  available, QEMU will automatically fallback to pipe2 or, as a last
> > +  resort, pipe.
> > +
> >  Message Specification
> >  =
> >  
> > -- 
> > 2.35.1
> 


signature.asc
Description: PGP signature

Re: any opinion on the patch "[RFC PATCH] hw/i386/e820: remove legacy reserved entries for e820"? EOM

[Ani Sinha]

On Fri, Mar 4, 2022 at 4:11 PM Ani Sinha  wrote:
>
> On Fri, Mar 4, 2022 at 3:48 PM Gerd Hoffmann  wrote:
> >
> > On Thu, Mar 03, 2022 at 03:12:51PM +0530, Ani Sinha wrote:
> > > On Thu, Mar 3, 2022 at 15:11 Gerd Hoffmann  wrote:
> > >
> > > > Sorry, no.  Noticed the discussions but don't remember the details and
> > > > didn't took the time to wade through the code to refresh my memory.
> > >
> > > Could you please take a look when you get time? You have the most context
> > > in this space I believe.
> >
> > Should indeed not be needed unless you use a stone-aged seabios version.
> > But I think you can't simply drop it for live migration compatibility
> > reasons.  So you'll need do the compatibility dance and drop it for new
> > machine types only.  I doubt the benefits outweigh that effort ..
>
> Igor what do you think?
Since the static entries are separate from the rom file entries, I
think we are in trouble only if the destination is using an old bios?
Otherwise, the non-existence of the static entries should be simply
ignored right?

Re: [PATCH v8 00/31] block layer: split block APIs in global state and I/O

[Kevin Wolf]

Am 03.03.2022 um 16:15 hat Emanuele Giuseppe Esposito geschrieben:
> Currently, block layer APIs like block.h contain a mix of
> functions that are either running in the main loop and under the
> BQL, or are thread-safe functions and run in iothreads performing I/O.
> The functions running under BQL also take care of modifying the
> block graph, by using drain and/or aio_context_acquire/release.
> This makes it very confusing to understand where each function
> runs, and what assumptions it provided with regards to thread
> safety.
> 
> We call the functions running under BQL "global state (GS) API", and
> distinguish them from the thread-safe "I/O API".
> 
> The aim of this series is to split the relevant block headers in
> global state and I/O sub-headers. The division will be done in
> this way:
> header.h will be split in header-global-state.h, header-io.h and
> header-common.h. The latter will just contain the data structures
> needed by header-global-state and header-io, and common helpers
> that are neither in GS nor in I/O. header.h will remain for
> legacy and to avoid changing all includes in all QEMU c files,
> but will only include the two new headers. No function shall be
> added in header.c .
> Once we split all relevant headers, it will be much easier to see what
> uses the AioContext lock and remove it, which is the overall main
> goal of this and other series that I posted/will post.
> 
> In addition to splitting the relevant headers shown in this series,
> it is also very helpful splitting the function pointers in some
> block structures, to understand what runs under AioContext lock and
> what doesn't. This is what patches 21-27 do.
> 
> Each function in the GS API will have an assertion, checking
> that it is always running under BQL.
> I/O functions are instead thread safe (or so should be), meaning
> that they *can* run under BQL, but also in an iothread in another
> AioContext. Therefore they do not provide any assertion, and
> need to be audited manually to verify the correctness.
> 
> Adding assetions has helped finding 2 bugs already, as shown in
> my series "Migration: fix missing iothread locking".
> 
> Tested this series by running unit tests, qemu-iotests and qtests
> (x86_64).
> Some functions in the GS API are used everywhere but not
> properly tested. Therefore their assertion is never actually run in
> the tests, so despite my very careful auditing, it is not impossible
> to exclude that some will trigger while actually using QEMU.
> 
> Patch 1 introduces qemu_in_main_thread(), the function used in
> all assertions. This had to be introduced otherwise all unit tests
> would fail, since they run in the main loop but use the code in
> stubs/iothread.c
> Patches 2-27 (with the exception of patch 9-10, that are an additional
> assert) are all structured in the same way: first we split the header
> and in the next (or same, if small) patch we add assertions.
> Patch 28-31 take care instead of the block layer permission API,
> fixing some bugs where they are used in I/O functions.
> 
> This serie depends on my previous serie "block layer: permission API
> refactoring in preparation to the API split"
> 
> Based-on: <20220209105452.1694545-1-eespo...@redhat.com>
> 
> Signed-off-by: Emanuele Giuseppe Esposito 
> ---
> v8:
> bdrv_get_full_backing_filename to GLOBAL_STATE_CODE
> blk_iostatus_is_enabled in IO_CODE
> blk_iostatus_set_err in IO_CODE
> bdrv_apply_auto_read_only in IO_CODE
> bdrv_can_set_read_only in IO_CODE
> blk_drain to GLOBAL_STATE_CODE

Thanks, fixed up the unintentional changes to bdrv_op_blocker_is_empty()
and bdrv_op_unblock_all() as discussed on IRC, and applied to the block
branch.

Kevin

Re: [PATCH v4 0/6] Introduce CanoKey QEMU

[Gerd Hoffmann]

On Sat, Feb 12, 2022 at 09:29:47PM +0800, Hongren (Zenithal) Zheng wrote:
> Hi,
> 
> Is there any further feedback on this patch set.

Sorry for the looong delay, I'm rather busy with edk2.

Tried to queue up this, noticed it breaks the build in case the
canokey library is not installed.

I'd suggest to run the patch series through the qemu gitlab CI
before sending out v5.

take care,
  Gerd

Re: [PATCH] hw/arm/virt: Validate memory size on the first NUMA node

[Igor Mammedov]

On Thu, 3 Mar 2022 11:25:25 +0800
Gavin Shan  wrote:

> Hi Gerd,
> 
> On 3/1/22 7:42 PM, Gerd Hoffmann wrote:
> >>> Unless it architecturally wrong thing i.e. (node size less than 128Mb)
> >>> ,in which case limiting it in QEMU would be justified, I'd prefer
> >>> firmware being fixed or it reporting more useful for user error message.  
> >>
> >> [include EDK2 developers]
> >>
> >> I don't think 128MB node memory size is architecturally required.
> >> I also thought EDK2 would be better place to provide a precise error
> >> mesage and discussed it through with EDK2 developers. Lets see what
> >> are their thoughts this time.  
> > 
> > Useful error reporting that early in the firmware initialization is a
> > rather hard problem, it's much easier for qemu to catch those problems
> > and print a useful error message.
> > 
> > Fixing the firmware would be possible.  The firmware simply uses the
> > memory of the first numa note in the early initialization phase, which
> > could be changed to look for additional numa nodes.  It's IMHO simply
> > not worth the trouble though.  numa nodes with less memory than 128M
> > simply doesn't happen in practice, except when QE does questionable
> > scaleability testing (scale up the number of numa nodes without also
> > scaling up the total amount of memory, ending up with rather tiny
> > numa nodes and a configuration nobody actually uses in practice).
> >   
> 
> I still don't think QEMU is best place to have this kind of limitation,
> which the first NUMA node should have 128MB memory at least. For example,
> the limitation exists in EDK2-version-A.0 and the limitation is removed
> in EDK2-version-A.1. The QEMU can't boot the guest using EDK2-version-A.1,
> but we should succeed.

if firmware is not an option, I wouldn't opposed to printing warning
message from QEMU if you can detect that you are running broken edk2
and under condition that no new infa/hooks are invented for this.
(assuming it's worth all the effort at all)

Maybe put it in virt-arm specific machine_done.

> If it's not worthwhile to be fixed in EDK2, it might be doable to improve
> the error message printed by EDK2. Otherwise, we would ignore this issue
> because 128MB node memory size isn't used in practice. If the EDK2 error
> message can be improved, we might have something like below:
> 
> ASSERT [MemoryInit] 
> /home/lacos/src/upstream/qemu/roms/edk2/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c(93):
>  NewSize >= 0x0800
> 
> to
> 
> The first NUMA node should have more than 128MB memory
> 
> Thanks,
> Gavin
> 

Re: [PATCH v4 2/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Igor Mammedov]

On Wed, 2 Mar 2022 17:45:58 +0200
Liav Albani  wrote:

> >>> but I feel quoting spec
> >>> and including table name is a good idea actually, but pls quote verbatim: 
> >>>  
> >> I don't do that  and don't ask it from others.
> >>
> >> The reason being that pointing where to look in spec and having
> >> verbatim copy of field name is sufficient
> >> for looking it up and
> >> QEMU does not endup with half of spec copied in (+unintentional mistakes).
> >> (As reviewer I will check if whatever written in patch actually matches
> >> spec anyways)
> >>
> >> That's why I typically use
> >>'spec ver, verbatim field name[, chapter/table name]'
> >> policy. The later optional part is usually used for pointing
> >> to values description.  
> >
> > Ok but here the field name was not listed verbatim, and table name
> > is missing. It is actually 8042 and table name is Fixed ACPI Description
> > Table Boot Architecture Flags.  
> So, in which route should I go with this? I could add a reference to the 
> ACPI spec, but can write and explain more if you want me to.
A reference to spec is sufficient, as long as it is unambiguous and lets
a reviewer easily find it within the spec

Re: [PATCH v4 11/14] softmmu/memory: add memory_region_try_add_subregion function

[Damien Hedde]

On 3/3/22 14:32, Philippe Mathieu-Daudé wrote:

On 23/2/22 10:12, Damien Hedde wrote:

Hi Philippe,

I suppose it is ok if I change your mail in the reviewed by ?


No, the email is fine (git tools should take care of using the
correct email via the .mailmap entry, see commit 90f285fd83).


Thanks,
Damien



ok.

Looks like git keeps as-is the "*-by:" entries untouched when cc-ing them.

--
Damien

Re: [PATCH] hw/arm/virt: Validate memory size on the first NUMA node

[Peter Maydell]

On Fri, 4 Mar 2022 at 10:52, Igor Mammedov  wrote:
> if firmware is not an option, I wouldn't opposed to printing warning
> message from QEMU if you can detect that you are running broken edk2
> and under condition that no new infa/hooks are invented for this.
> (assuming it's worth all the effort at all)

I am definitely not in favour of that. QEMU should provide the
emulated hardware and let the firmware deal with detecting if
it's missing important stuff. It should as far as is possible
not have special-case detection-of-broken-guests handling.

thanks
-- PMM

Re: [PATCH 1/1] virtio: fix feature negotiation for ACCESS_PLATFORM

[Halil Pasic]

On Fri, 4 Mar 2022 03:12:03 -0500
"Michael S. Tsirkin"  wrote:

> On Thu, Feb 10, 2022 at 02:29:29PM +0100, Halil Pasic wrote:
> > On Thu, 10 Feb 2022 12:19:25 +0100
> > Cornelia Huck  wrote:
> > 
> > [..]  
> > > 
> > > Nope, that's not my problem. We make sure that the bit is persistent, we
> > > fail realization if the bit got removed by the callback when required,
> > > and we fail feature validation if the driver removes the bit, which is
> > > in a different code path. We should not talk about FEATURES_OK in this
> > > code.  
> > 
> > I agree. I changed my mind. Expanation follows...
> >   
> > > 
> > > We force-add the bit, and then still might fail realization. The
> > > important condition is the has_iommu one, not the checks later on. I
> > > find it very confusing to talk about what a potential driver might do in
> > > that context.
> > >   
> > 
> > I assumed stuff like virtiofs+SE regressed with commit 04ceb61a40
> > ("virtio: Fail if iommu_platform is requested, but unsupported") but I
> > think, I was wrong. It didn't work before that, because we did not
> > present ACCESS_PLATFORM to the guest. I operated under the assumption
> > that virtio_add_feature(&vdev->host_features, VIRTIO_F_IOMMU_PLATFORM)
> > does not impact the set of features offered to the driver by the device,
> > but it does.
> > 
> > So we need both this patch and "[PATCH v5 1/1] virtio: fix the condition
> > for iommu_platform not supported" to get virtiofs to work with 
> > SE/SEV/Secure VM.
> > 
> > I have to admit I only tested for the error message, and not with a full
> > SE setup.
> > 
> > I agree my comment was inadequate. Can we use
> > 
> > /* make sure that the device did not drop a required IOMMU_PLATFORM */
> > 
> > I will think some more though. This is again about the dual nature
> > of ACCESS_PLATFORM...  
> 
> Were you going to post a new version of this patch?
> 

Sorry I got sidetracked. I will spin a new version today!

Regards,
Halil

[PATCH v2 1/9] hw/usb/redirect.c: Stop using qemu_oom_check()

[Peter Maydell]

qemu_oom_check() is a function which essentially says "if you pass me
a NULL pointer then print a message then abort()".  On POSIX systems
the message includes strerror(errno); on Windows it includes the
GetLastError() error value printed as an integer.

Other than in the implementation of qemu_memalign(), we use this
function only in hw/usb/redirect.c, for three checks:

 * on a call to usbredirparser_create()
 * on a call to usberedirparser_serialize()
 * on a call to malloc()

The usbredir library API functions make no guarantees that they will
set errno on errors, let alone that they might set the
Windows-specific GetLastError string.  malloc() is documented as
setting errno, not GetLastError -- and in any case the only thing it
might set errno to is ENOMEM.  So qemu_oom_check() isn't the right
thing for any of these.  Replace them with straightforward
error-checking code.  This will allow us to get rid of
qemu_oom_check().

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Eric Blake 
Message-id: 20220226180723.1706285-2-peter.mayd...@linaro.org
---
 hw/usb/redirect.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index 5f0ef9cb3b0..8692ea25610 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -1239,7 +1239,11 @@ static void usbredir_create_parser(USBRedirDevice *dev)
 
 DPRINTF("creating usbredirparser\n");
 
-dev->parser = qemu_oom_check(usbredirparser_create());
+dev->parser = usbredirparser_create();
+if (!dev->parser) {
+error_report("usbredirparser_create() failed");
+exit(1);
+}
 dev->parser->priv = dev;
 dev->parser->log_func = usbredir_log;
 dev->parser->read_func = usbredir_read;
@@ -2239,7 +2243,10 @@ static int usbredir_put_parser(QEMUFile *f, void *priv, 
size_t unused,
 }
 
 usbredirparser_serialize(dev->parser, &data, &len);
-qemu_oom_check(data);
+if (!data) {
+error_report("usbredirparser_serialize failed");
+exit(1);
+}
 
 qemu_put_be32(f, len);
 qemu_put_buffer(f, data, len);
@@ -2330,7 +2337,11 @@ static int usbredir_get_bufpq(QEMUFile *f, void *priv, 
size_t unused,
 bufp->len = qemu_get_be32(f);
 bufp->status = qemu_get_be32(f);
 bufp->offset = 0;
-bufp->data = qemu_oom_check(malloc(bufp->len)); /* regular malloc! */
+bufp->data = malloc(bufp->len); /* regular malloc! */
+if (!bufp->data) {
+error_report("usbredir_get_bufpq: out of memory");
+exit(1);
+}
 bufp->free_on_destroy = bufp->data;
 qemu_get_buffer(f, bufp->data, bufp->len);
 QTAILQ_INSERT_TAIL(&endp->bufpq, bufp, next);
-- 
2.25.1

[PATCH v2 6/9] util: Share qemu_try_memalign() implementation between POSIX and Windows

[Peter Maydell]

The qemu_try_memalign() functions for POSIX and Windows used to be
significantly different, but these days they are identical except for
the actual allocation function called, and the POSIX version already
has to have ifdeffery for different allocation functions.

Move to a single implementation in memalign.c, which uses the Windows
_aligned_malloc if we detect that function in meson.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20220226180723.1706285-7-peter.mayd...@linaro.org
---
 meson.build|  1 +
 util/memalign.c| 39 +++
 util/oslib-posix.c | 29 -
 util/oslib-win32.c | 17 -
 4 files changed, 40 insertions(+), 46 deletions(-)

diff --git a/meson.build b/meson.build
index 73fd17a0523..eae0e4febb3 100644
--- a/meson.build
+++ b/meson.build
@@ -1622,6 +1622,7 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE', 
cc.has_function('posix_fallocate'
 # Note that we need to specify prefix: here to avoid incorrectly
 # thinking that Windows has posix_memalign()
 config_host_data.set('CONFIG_POSIX_MEMALIGN', 
cc.has_function('posix_memalign', prefix: '#include '))
+config_host_data.set('CONFIG_ALIGNED_MALLOC', 
cc.has_function('_aligned_malloc'))
 config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: 
'#include '))
 config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait', 
dependencies: threads))
diff --git a/util/memalign.c b/util/memalign.c
index 6dfc20abbb1..c3280528d24 100644
--- a/util/memalign.c
+++ b/util/memalign.c
@@ -25,6 +25,45 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/host-utils.h"
+#include "trace.h"
+
+void *qemu_try_memalign(size_t alignment, size_t size)
+{
+void *ptr;
+
+if (alignment < sizeof(void*)) {
+alignment = sizeof(void*);
+} else {
+g_assert(is_power_of_2(alignment));
+}
+
+/*
+ * Handling of 0 allocations varies among the different
+ * platform APIs (for instance _alligned_malloc() will
+ * fail) -- ensure that we always return a valid non-NULL
+ * pointer that can be freed by qemu_vfree().
+ */
+if (size == 0) {
+size++;
+}
+#if defined(CONFIG_POSIX_MEMALIGN)
+int ret;
+ret = posix_memalign(&ptr, alignment, size);
+if (ret != 0) {
+errno = ret;
+ptr = NULL;
+}
+#elif defined(CONFIG_ALIGNED_MALLOC)
+ptr = _aligned_malloc(size, alignment);
+#elif defined(CONFIG_BSD)
+ptr = valloc(size);
+#else
+ptr = memalign(alignment, size);
+#endif
+trace_qemu_memalign(alignment, size, ptr);
+return ptr;
+}
 
 void *qemu_memalign(size_t alignment, size_t size)
 {
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f7e22f4ff9b..91798f7e504 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -199,35 +199,6 @@ fail_close:
 return false;
 }
 
-void *qemu_try_memalign(size_t alignment, size_t size)
-{
-void *ptr;
-
-if (alignment < sizeof(void*)) {
-alignment = sizeof(void*);
-} else {
-g_assert(is_power_of_2(alignment));
-}
-
-if (size == 0) {
-size++;
-}
-#if defined(CONFIG_POSIX_MEMALIGN)
-int ret;
-ret = posix_memalign(&ptr, alignment, size);
-if (ret != 0) {
-errno = ret;
-ptr = NULL;
-}
-#elif defined(CONFIG_BSD)
-ptr = valloc(size);
-#else
-ptr = memalign(alignment, size);
-#endif
-trace_qemu_memalign(alignment, size, ptr);
-return ptr;
-}
-
 /* alloc shared memory pages */
 void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment, bool shared,
   bool noreserve)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 8c28d70904d..d9768532bec 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -44,23 +44,6 @@
 /* this must come after including "trace.h" */
 #include 
 
-void *qemu_try_memalign(size_t alignment, size_t size)
-{
-void *ptr;
-
-if (alignment < sizeof(void *)) {
-alignment = sizeof(void *);
-} else {
-g_assert(is_power_of_2(alignment));
-}
-if (size == 0) {
-size++;
-}
-ptr = _aligned_malloc(size, alignment);
-trace_qemu_memalign(alignment, size, ptr);
-return ptr;
-}
-
 static int get_allocation_granularity(void)
 {
 SYSTEM_INFO system_info;
-- 
2.25.1

[PATCH v2 4/9] util: Return valid allocation for qemu_try_memalign() with zero size

[Peter Maydell]

Currently qemu_try_memalign()'s behaviour if asked to allocate
0 bytes is rather variable:
 * on Windows, we will assert
 * on POSIX platforms, we get the underlying behaviour of
   the posix_memalign() or equivalent function, which may be
   either "return a valid non-NULL pointer" or "return NULL"

Explictly check for 0 byte allocations, so we get consistent
behaviour across platforms.  We handle them by incrementing the size
so that we return a valid non-NULL pointer that can later be passed
to qemu_vfree().  This is permitted behaviour for the
posix_memalign() API and is the most usual way that underlying
malloc() etc implementations handle a zero-sized allocation request,
because it won't trip up calling code that assumes NULL means an
error.  (This includes our own qemu_memalign(), which will abort on
NULL.)

This change is a preparation for sharing the qemu_try_memalign() code
between Windows and POSIX.

Signed-off-by: Peter Maydell 
---
 util/oslib-posix.c | 3 +++
 util/oslib-win32.c | 4 +++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 0278902ee79..f7e22f4ff9b 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -209,6 +209,9 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 g_assert(is_power_of_2(alignment));
 }
 
+if (size == 0) {
+size++;
+}
 #if defined(CONFIG_POSIX_MEMALIGN)
 int ret;
 ret = posix_memalign(&ptr, alignment, size);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 05857414695..8c28d70904d 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -48,12 +48,14 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 {
 void *ptr;
 
-g_assert(size != 0);
 if (alignment < sizeof(void *)) {
 alignment = sizeof(void *);
 } else {
 g_assert(is_power_of_2(alignment));
 }
+if (size == 0) {
+size++;
+}
 ptr = _aligned_malloc(size, alignment);
 trace_qemu_memalign(alignment, size, ptr);
 return ptr;
-- 
2.25.1

[PATCH v2 2/9] util: Make qemu_oom_check() a static function

[Peter Maydell]

The qemu_oom_check() function, which we define in both oslib-posix.c
and oslib-win32.c, is now used only locally in that file; make it
static.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20220226180723.1706285-3-peter.mayd...@linaro.org
---
 include/qemu-common.h | 2 --
 util/oslib-posix.c| 2 +-
 util/oslib-win32.c| 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index 68b2e3bc109..8c0d9ab0f77 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -26,8 +26,6 @@
 int qemu_main(int argc, char **argv, char **envp);
 #endif
 
-void *qemu_oom_check(void *ptr);
-
 ssize_t qemu_write_full(int fd, const void *buf, size_t count)
 QEMU_WARN_UNUSED_RESULT;
 
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index f2be7321c59..ed5974d3845 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -199,7 +199,7 @@ fail_close:
 return false;
 }
 
-void *qemu_oom_check(void *ptr)
+static void *qemu_oom_check(void *ptr)
 {
 if (ptr == NULL) {
 fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index af559ef3398..c87e6977246 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -44,7 +44,7 @@
 /* this must come after including "trace.h" */
 #include 
 
-void *qemu_oom_check(void *ptr)
+static void *qemu_oom_check(void *ptr)
 {
 if (ptr == NULL) {
 fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
-- 
2.25.1

[PATCH v2 3/9] util: Unify implementations of qemu_memalign()

[Peter Maydell]

We implement qemu_memalign() in both oslib-posix.c and oslib-win32.c,
but the two versions are essentially the same: they call
qemu_try_memalign(), and abort() after printing an error message if
it fails.  The only difference is that the win32 version prints the
GetLastError() value whereas the POSIX version prints
strerror(errno).  However, this is a bug in the win32 version: in
commit dfbd0b873a85021 in 2020 we changed the implementation of
qemu_try_memalign() from using VirtualAlloc() (which sets the
GetLastError() value) to using _aligned_malloc() (which sets errno),
but didn't update the error message to match.

Replace the two separate functions with a single version in a
new memalign.c file, which drops the unnecessary extra qemu_oom_check()
function and instead prints a more useful message including the
requested size and alignment as well as the errno string.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20220226180723.1706285-4-peter.mayd...@linaro.org
---
 util/memalign.c| 39 +++
 util/oslib-posix.c | 14 --
 util/oslib-win32.c | 14 --
 util/meson.build   |  1 +
 4 files changed, 40 insertions(+), 28 deletions(-)
 create mode 100644 util/memalign.c

diff --git a/util/memalign.c b/util/memalign.c
new file mode 100644
index 000..6dfc20abbb1
--- /dev/null
+++ b/util/memalign.c
@@ -0,0 +1,39 @@
+/*
+ * memalign.c: Allocate an aligned memory region
+ *
+ * Copyright (c) 2003-2008 Fabrice Bellard
+ * Copyright (c) 2010-2016 Red Hat, Inc.
+ * Copyright (c) 2022 Linaro Ltd
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+
+void *qemu_memalign(size_t alignment, size_t size)
+{
+void *p = qemu_try_memalign(alignment, size);
+if (p) {
+return p;
+}
+fprintf(stderr,
+"qemu_memalign: failed to allocate %zu bytes at alignment %zu: 
%s\n",
+size, alignment, strerror(errno));
+abort();
+}
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index ed5974d3845..0278902ee79 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -199,15 +199,6 @@ fail_close:
 return false;
 }
 
-static void *qemu_oom_check(void *ptr)
-{
-if (ptr == NULL) {
-fprintf(stderr, "Failed to allocate memory: %s\n", strerror(errno));
-abort();
-}
-return ptr;
-}
-
 void *qemu_try_memalign(size_t alignment, size_t size)
 {
 void *ptr;
@@ -234,11 +225,6 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 return ptr;
 }
 
-void *qemu_memalign(size_t alignment, size_t size)
-{
-return qemu_oom_check(qemu_try_memalign(alignment, size));
-}
-
 /* alloc shared memory pages */
 void *qemu_anon_ram_alloc(size_t size, uint64_t *alignment, bool shared,
   bool noreserve)
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index c87e6977246..05857414695 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -44,15 +44,6 @@
 /* this must come after including "trace.h" */
 #include 
 
-static void *qemu_oom_check(void *ptr)
-{
-if (ptr == NULL) {
-fprintf(stderr, "Failed to allocate memory: %lu\n", GetLastError());
-abort();
-}
-return ptr;
-}
-
 void *qemu_try_memalign(size_t alignment, size_t size)
 {
 void *ptr;
@@ -68,11 +59,6 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 return ptr;
 }
 
-void *qemu_memalign(size_t alignment, size_t size)
-{
-return qemu_oom_check(qemu_try_memalign(alignment, size));
-}
-
 static int get_allocation_granularity(void)
 {
 SYSTEM_INFO system_info;
diff --git a/util/meson.build b/util/meson.build
index 3736988b9f6..f6ee74ad0c8 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -51,6 +51,7 @@ util_ss.add(when: 'CONFIG_POSIX', if_true: files('drm.c'))
 util_ss.add(files('guest-random.c'))
 util_ss.add(files('yank.c'))
 util_ss.add(files('int128.c'))
+util_ss.add(files('

[PATCH v2 7/9] util: Use meson checks for valloc() and memalign() presence

[Peter Maydell]

Instead of assuming that all CONFIG_BSD have valloc() and anything
else is memalign(), explicitly check for those functions in
meson.build and use the "is the function present" define.  Tests for
specific functionality are better than which-OS checks; this also
lets us give a helpful error message if somehow there's no usable
function present.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20220226180723.1706285-8-peter.mayd...@linaro.org
---
 meson.build | 2 ++
 util/memalign.c | 6 --
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index eae0e4febb3..74f4e05c82b 100644
--- a/meson.build
+++ b/meson.build
@@ -1623,6 +1623,8 @@ config_host_data.set('CONFIG_POSIX_FALLOCATE', 
cc.has_function('posix_fallocate'
 # thinking that Windows has posix_memalign()
 config_host_data.set('CONFIG_POSIX_MEMALIGN', 
cc.has_function('posix_memalign', prefix: '#include '))
 config_host_data.set('CONFIG_ALIGNED_MALLOC', 
cc.has_function('_aligned_malloc'))
+config_host_data.set('CONFIG_VALLOC', cc.has_function('valloc'))
+config_host_data.set('CONFIG_MEMALIGN', cc.has_function('memalign'))
 config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: 
'#include '))
 config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait', 
dependencies: threads))
diff --git a/util/memalign.c b/util/memalign.c
index c3280528d24..2a139a5695d 100644
--- a/util/memalign.c
+++ b/util/memalign.c
@@ -56,10 +56,12 @@ void *qemu_try_memalign(size_t alignment, size_t size)
 }
 #elif defined(CONFIG_ALIGNED_MALLOC)
 ptr = _aligned_malloc(size, alignment);
-#elif defined(CONFIG_BSD)
+#elif defined(CONFIG_VALLOC)
 ptr = valloc(size);
-#else
+#elif defined(CONFIG_MEMALIGN)
 ptr = memalign(alignment, size);
+#else
+#error No function to allocate aligned memory available
 #endif
 trace_qemu_memalign(alignment, size, ptr);
 return ptr;
-- 
2.25.1

[PATCH v2 0/9] Cleanup of qemu_oom_check() and qemu_memalign()

[Peter Maydell]

This series does some cleanup of the qemu_oom_check() and
qemu_memalign() functions; I started looking at the first of these and
found myself wanting to tidy some stuff relating to the second in the
process. The TLDR is that this series removes qemu_oom_check() (which
was mostly being misused), unifies the POSIX and Win32 versions of
qemu_memalign() and qemu_try_memalign(), and moves the prototypes out
of osdep.h.

Changes v1->v2:
 * Replacement patch 4, which takes the approach discussed in
   comments on v1 of making all our implementations handle
   size == 0 by doing a size 1 allocation
 * two #include lines accidentally added in patch 7 are
   moved to patch 6 where they belong (fixes compile failure
   during bisect)

Patch 4 is the only one needing review.

(When I came to make this change I decided that there was
just a bit more involved than I was happy making in passing
while assembling a pull request.)

thanks
-- PMM


Peter Maydell (9):
  hw/usb/redirect.c: Stop using qemu_oom_check()
  util: Make qemu_oom_check() a static function
  util: Unify implementations of qemu_memalign()
  util: Return valid allocation for qemu_try_memalign() with zero size
  meson.build: Don't misdetect posix_memalign() on Windows
  util: Share qemu_try_memalign() implementation between POSIX and
Windows
  util: Use meson checks for valloc() and memalign() presence
  util: Put qemu_vfree() in memalign.c
  osdep: Move memalign-related functions to their own header

 meson.build|  7 ++-
 include/qemu-common.h  |  2 -
 include/qemu/memalign.h| 61 ++
 include/qemu/osdep.h   | 18 ---
 block/blkverify.c  |  1 +
 block/block-copy.c |  1 +
 block/commit.c |  1 +
 block/crypto.c |  1 +
 block/dmg.c|  1 +
 block/export/fuse.c|  1 +
 block/file-posix.c |  1 +
 block/io.c |  1 +
 block/mirror.c |  1 +
 block/nvme.c   |  1 +
 block/parallels-ext.c  |  1 +
 block/parallels.c  |  1 +
 block/qcow.c   |  1 +
 block/qcow2-cache.c|  1 +
 block/qcow2-cluster.c  |  1 +
 block/qcow2-refcount.c |  1 +
 block/qcow2-snapshot.c |  1 +
 block/qcow2.c  |  1 +
 block/qed-l2-cache.c   |  1 +
 block/qed-table.c  |  1 +
 block/qed.c|  1 +
 block/quorum.c |  1 +
 block/raw-format.c |  1 +
 block/vdi.c|  1 +
 block/vhdx-log.c   |  1 +
 block/vhdx.c   |  1 +
 block/vmdk.c   |  1 +
 block/vpc.c|  1 +
 block/win32-aio.c  |  1 +
 hw/block/dataplane/xen-block.c |  1 +
 hw/block/fdc.c |  1 +
 hw/ide/core.c  |  1 +
 hw/ppc/spapr.c |  1 +
 hw/ppc/spapr_softmmu.c |  1 +
 hw/scsi/scsi-disk.c|  1 +
 hw/tpm/tpm_ppi.c   |  2 +-
 hw/usb/redirect.c  | 17 +--
 nbd/server.c   |  1 +
 net/l2tpv3.c   |  2 +-
 plugins/loader.c   |  1 +
 qemu-img.c |  1 +
 qemu-io-cmds.c |  1 +
 qom/object.c   |  1 +
 softmmu/physmem.c  |  1 +
 target/i386/hvf/hvf.c  |  1 +
 target/i386/kvm/kvm.c  |  1 +
 tcg/region.c   |  1 +
 tests/bench/atomic_add-bench.c |  1 +
 tests/bench/qht-bench.c|  1 +
 util/atomic64.c|  1 +
 util/memalign.c| 92 ++
 util/oslib-posix.c | 46 -
 util/oslib-win32.c | 35 -
 util/qht.c |  1 +
 util/meson.build   |  1 +
 59 files changed, 224 insertions(+), 107 deletions(-)
 create mode 100644 include/qemu/memalign.h
 create mode 100644 util/memalign.c

-- 
2.25.1

[PATCH v2 5/9] meson.build: Don't misdetect posix_memalign() on Windows

[Peter Maydell]

Currently we incorrectly think that posix_memalign() exists on
Windows.  This is because of a combination of:

 * the msys2/mingw toolchain/libc claim to have a
   __builtin_posix_memalign when there isn't a builtin of that name
 * meson will assume that if you have a __builtin_foo that
   counts for has_function('foo')

Specifying a specific include file via prefix: causes meson to not
treat builtins as sufficient and actually look for the function
itself; see this meson pull request which added that as the official
way to get the right answer:
  https://github.com/mesonbuild/meson/pull/1150

Currently this misdectection doesn't cause problems because we only
use CONFIG_POSIX_MEMALIGN in oslib-posix.c; however that will change
in a following commit.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20220226180723.1706285-6-peter.mayd...@linaro.org
---
 meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index a5b63e62cdc..73fd17a0523 100644
--- a/meson.build
+++ b/meson.build
@@ -1619,7 +1619,9 @@ config_host_data.set('CONFIG_CLOCK_ADJTIME', 
cc.has_function('clock_adjtime'))
 config_host_data.set('CONFIG_DUP3', cc.has_function('dup3'))
 config_host_data.set('CONFIG_FALLOCATE', cc.has_function('fallocate'))
 config_host_data.set('CONFIG_POSIX_FALLOCATE', 
cc.has_function('posix_fallocate'))
-config_host_data.set('CONFIG_POSIX_MEMALIGN', 
cc.has_function('posix_memalign'))
+# Note that we need to specify prefix: here to avoid incorrectly
+# thinking that Windows has posix_memalign()
+config_host_data.set('CONFIG_POSIX_MEMALIGN', 
cc.has_function('posix_memalign', prefix: '#include '))
 config_host_data.set('CONFIG_PPOLL', cc.has_function('ppoll'))
 config_host_data.set('CONFIG_PREADV', cc.has_function('preadv', prefix: 
'#include '))
 config_host_data.set('CONFIG_SEM_TIMEDWAIT', cc.has_function('sem_timedwait', 
dependencies: threads))
-- 
2.25.1

[PATCH v2 8/9] util: Put qemu_vfree() in memalign.c

[Peter Maydell]

qemu_vfree() is the companion free function to qemu_memalign(); put
it in memalign.c so the allocation and free functions are together.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20220226180723.1706285-9-peter.mayd...@linaro.org
---
 util/memalign.c| 11 +++
 util/oslib-posix.c |  6 --
 util/oslib-win32.c |  6 --
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/util/memalign.c b/util/memalign.c
index 2a139a5695d..5b812bc11fe 100644
--- a/util/memalign.c
+++ b/util/memalign.c
@@ -78,3 +78,14 @@ void *qemu_memalign(size_t alignment, size_t size)
 size, alignment, strerror(errno));
 abort();
 }
+
+void qemu_vfree(void *ptr)
+{
+trace_qemu_vfree(ptr);
+#if !defined(CONFIG_POSIX_MEMALIGN) && defined(CONFIG_ALIGNED_MALLOC)
+/* Only Windows _aligned_malloc needs a special free function */
+_aligned_free(ptr);
+#else
+free(ptr);
+#endif
+}
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 91798f7e504..2ebfb750578 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -220,12 +220,6 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t 
*alignment, bool shared,
 return ptr;
 }
 
-void qemu_vfree(void *ptr)
-{
-trace_qemu_vfree(ptr);
-free(ptr);
-}
-
 void qemu_anon_ram_free(void *ptr, size_t size)
 {
 trace_qemu_anon_ram_free(ptr, size);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index d9768532bec..4b1ce0be4b0 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -75,12 +75,6 @@ void *qemu_anon_ram_alloc(size_t size, uint64_t *align, bool 
shared,
 return ptr;
 }
 
-void qemu_vfree(void *ptr)
-{
-trace_qemu_vfree(ptr);
-_aligned_free(ptr);
-}
-
 void qemu_anon_ram_free(void *ptr, size_t size)
 {
 trace_qemu_anon_ram_free(ptr, size);
-- 
2.25.1

Re: Issue with qemu-system-ppc running OSX guests

[Lucas Mateus Martins Araujo e Castro]

On 02/03/2022 20:55, Fabiano Rosas wrote:

Howard Spoelstra  writes:


On Wed, Mar 2, 2022 at 9:11 PM BALATON Zoltan  wrote:


On Wed, 2 Mar 2022, Howard Spoelstra wrote:

Hi all,

I noticed qemu-system-ppc running OSX guests does not get to the desktop

or

does not display the menu bars.

Cc-ing the relevant people and the PPC list might help, I've added them.
Also telling which OSX guest version can reproduce the problem could help
debugging. Is it any OSX version?

Regards,
BALATON Zoltan


Oops, Qemu running on Fedora 35 host,
Reproducer:

./qemu-system-ppc \
-M mac99 \
-m 512 \
-L pc-bios \
-display sdl -serial stdio \
-boot c \
-drive file=10.1.img,format=raw,media=disk

The issue affects all supported Mac OSX guests: 10.0 to 10.5

Hi Howard,

Thanks for bisecting this. It seems we inadvertently marked some of the
Vector Multiply instructions to be ISA v2.07 only.

I can boot Mac OSX 10.4 until the desktop with this fix:

diff --git a/target/ppc/translate/vmx-impl.c.inc 
b/target/ppc/translate/vmx-impl.c.inc
index f91bee839d..c5d02d13fe 100644
--- a/target/ppc/translate/vmx-impl.c.inc
+++ b/target/ppc/translate/vmx-impl.c.inc
@@ -3141,14 +3141,14 @@ static bool trans_VMULLD(DisasContext *ctx, arg_VX *a)
  return true;
  }

-TRANS_FLAGS2(ALTIVEC_207, VMULESB, do_vx_helper, gen_helper_VMULESB)
-TRANS_FLAGS2(ALTIVEC_207, VMULOSB, do_vx_helper, gen_helper_VMULOSB)
-TRANS_FLAGS2(ALTIVEC_207, VMULEUB, do_vx_helper, gen_helper_VMULEUB)
-TRANS_FLAGS2(ALTIVEC_207, VMULOUB, do_vx_helper, gen_helper_VMULOUB)
-TRANS_FLAGS2(ALTIVEC_207, VMULESH, do_vx_helper, gen_helper_VMULESH)
-TRANS_FLAGS2(ALTIVEC_207, VMULOSH, do_vx_helper, gen_helper_VMULOSH)
-TRANS_FLAGS2(ALTIVEC_207, VMULEUH, do_vx_helper, gen_helper_VMULEUH)
-TRANS_FLAGS2(ALTIVEC_207, VMULOUH, do_vx_helper, gen_helper_VMULOUH)
+TRANS_FLAGS(ALTIVEC, VMULESB, do_vx_helper, gen_helper_VMULESB)
+TRANS_FLAGS(ALTIVEC, VMULOSB, do_vx_helper, gen_helper_VMULOSB)
+TRANS_FLAGS(ALTIVEC, VMULEUB, do_vx_helper, gen_helper_VMULEUB)
+TRANS_FLAGS(ALTIVEC, VMULOUB, do_vx_helper, gen_helper_VMULOUB)
+TRANS_FLAGS(ALTIVEC, VMULESH, do_vx_helper, gen_helper_VMULESH)
+TRANS_FLAGS(ALTIVEC, VMULOSH, do_vx_helper, gen_helper_VMULOSH)
+TRANS_FLAGS(ALTIVEC, VMULEUH, do_vx_helper, gen_helper_VMULEUH)
+TRANS_FLAGS(ALTIVEC, VMULOUH, do_vx_helper, gen_helper_VMULOUH)
  TRANS_FLAGS2(ALTIVEC_207, VMULESW, do_vx_helper, gen_helper_VMULESW)
  TRANS_FLAGS2(ALTIVEC_207, VMULOSW, do_vx_helper, gen_helper_VMULOSW)
  TRANS_FLAGS2(ALTIVEC_207, VMULEUW, do_vx_helper, gen_helper_VMULEUW)
---

I'll let Lucas comment on it and we can send a proper patch in the
morning.


Checking here it seems I misread the PowerISA appendix and marked these 
instructions (vmul[eo].[bh]) as v2.07 even though they are v2.03.


This patch seems to correct it and checking here the newer instructions 
are correct (v2.07 for vmul[eo].w and v3.1 for vmul[eo].d), so


Reviewed-by: Lucas Mateus Castro

--
Lucas Mateus M. Araujo e Castro
Instituto de Pesquisas ELDORADO 


Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer 

[PATCH v2 9/9] osdep: Move memalign-related functions to their own header

[Peter Maydell]

Move the various memalign-related functions out of osdep.h and into
their own header, which we include only where they are used.
While we're doing this, add some brief documentation comments.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20220226180723.1706285-10-peter.mayd...@linaro.org
---
 include/qemu/memalign.h| 61 ++
 include/qemu/osdep.h   | 18 --
 block/blkverify.c  |  1 +
 block/block-copy.c |  1 +
 block/commit.c |  1 +
 block/crypto.c |  1 +
 block/dmg.c|  1 +
 block/export/fuse.c|  1 +
 block/file-posix.c |  1 +
 block/io.c |  1 +
 block/mirror.c |  1 +
 block/nvme.c   |  1 +
 block/parallels-ext.c  |  1 +
 block/parallels.c  |  1 +
 block/qcow.c   |  1 +
 block/qcow2-cache.c|  1 +
 block/qcow2-cluster.c  |  1 +
 block/qcow2-refcount.c |  1 +
 block/qcow2-snapshot.c |  1 +
 block/qcow2.c  |  1 +
 block/qed-l2-cache.c   |  1 +
 block/qed-table.c  |  1 +
 block/qed.c|  1 +
 block/quorum.c |  1 +
 block/raw-format.c |  1 +
 block/vdi.c|  1 +
 block/vhdx-log.c   |  1 +
 block/vhdx.c   |  1 +
 block/vmdk.c   |  1 +
 block/vpc.c|  1 +
 block/win32-aio.c  |  1 +
 hw/block/dataplane/xen-block.c |  1 +
 hw/block/fdc.c |  1 +
 hw/ide/core.c  |  1 +
 hw/ppc/spapr.c |  1 +
 hw/ppc/spapr_softmmu.c |  1 +
 hw/scsi/scsi-disk.c|  1 +
 hw/tpm/tpm_ppi.c   |  2 +-
 nbd/server.c   |  1 +
 net/l2tpv3.c   |  2 +-
 plugins/loader.c   |  1 +
 qemu-img.c |  1 +
 qemu-io-cmds.c |  1 +
 qom/object.c   |  1 +
 softmmu/physmem.c  |  1 +
 target/i386/hvf/hvf.c  |  1 +
 target/i386/kvm/kvm.c  |  1 +
 tcg/region.c   |  1 +
 tests/bench/atomic_add-bench.c |  1 +
 tests/bench/qht-bench.c|  1 +
 util/atomic64.c|  1 +
 util/memalign.c|  1 +
 util/qht.c |  1 +
 53 files changed, 112 insertions(+), 20 deletions(-)
 create mode 100644 include/qemu/memalign.h

diff --git a/include/qemu/memalign.h b/include/qemu/memalign.h
new file mode 100644
index 000..fa299f3bf67
--- /dev/null
+++ b/include/qemu/memalign.h
@@ -0,0 +1,61 @@
+/*
+ * Allocation and free functions for aligned memory
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_MEMALIGN_H
+#define QEMU_MEMALIGN_H
+
+/**
+ * qemu_try_memalign: Allocate aligned memory
+ * @alignment: required alignment, in bytes
+ * @size: size of allocation, in bytes
+ *
+ * Allocate memory on an aligned boundary (i.e. the returned
+ * address will be an exact multiple of @alignment).
+ * @alignment must be a power of 2, or the function will assert().
+ * On success, returns allocated memory; on failure, returns NULL.
+ *
+ * The memory allocated through this function must be freed via
+ * qemu_vfree() (and not via free()).
+ */
+void *qemu_try_memalign(size_t alignment, size_t size);
+/**
+ * qemu_memalign: Allocate aligned memory, without failing
+ * @alignment: required alignment, in bytes
+ * @size: size of allocation, in bytes
+ *
+ * Allocate memory in the same way as qemu_try_memalign(), but
+ * abort() with an error message if the memory allocation fails.
+ *
+ * The memory allocated through this function must be freed via
+ * qemu_vfree() (and not via free()).
+ */
+void *qemu_memalign(size_t alignment, size_t size);
+/**
+ * qemu_vfree: Free memory allocated through qemu_memalign
+ * @ptr: memory to free
+ *
+ * This function must be used to free memory allocated via qemu_memalign()
+ * or qemu_try_memalign(). (Using the wrong free function will cause
+ * subtle bugs on Windows hosts.)
+ */
+void qemu_vfree(void *ptr);
+/*
+ * It's an analog of GLIB's g_autoptr_cleanup_generic_gfree(), used to define
+ * g_autofree macro.
+ */
+static inline void qemu_cleanup_generic_vfree(void *p)
+{
+  void **pp = (void **)p;
+  qemu_vfree(*pp);
+}
+
+/*
+ * Analog of g_autofree, but qemu_vfree is called on cleanup instead of g_free.
+ */
+#define QEMU_AUTO_VFREE __attribute__((cleanup(qemu_cleanup_generic_vfree)))
+
+#endif
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 7bcce3bceb0..bc3df26da36 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -379,28 +379,10 @@ extern "C" {
 #endif
 
 int qemu_daemon(int nochdir, int noclose);
-void *qemu_try_memalign(size_t alignment, s

Re: [PATCH] target/arm: Fix sve2 ldnt1 (64-bit unscaled offset)

[Peter Maydell]

On Fri, 4 Mar 2022 at 00:17, Richard Henderson
 wrote:
>
> We were mapping this to ld1 (32-bit unpacked unscaled offset),
> which discarded the upper 32 bits of the address coming from
> the vector argument.
>
> Fixed by setting XS=2, which is the existing translator internal
> value for no extension.  Update the comments, which matched the
> incorrect code.
>
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/826
> Signed-off-by: Richard Henderson 
> ---
>  target/arm/sve.decode | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v4 18/18] hw/arm/virt: Disable LPA2 for -machine virt-6.2

[Peter Maydell]

On Tue, 1 Mar 2022 at 22:00, Richard Henderson
 wrote:
>
> There is a Linux kernel bug present until v5.12 that prevents
> booting with FEAT_LPA2 enabled.  As a workaround for TCG,
> disable this feature for machine versions prior to 7.0.
>
> Cc: Daniel P. Berrangé 
> Signed-off-by: Richard Henderson 
> ---
>  include/hw/arm/virt.h | 1 +
>  hw/arm/virt.c | 7 +++
>  2 files changed, 8 insertions(+)

Is it not possible to implement this in the usual "change
property for older versioned machines" way of adding to
the hw_compat arrays?

diff --git a/hw/core/machine.c b/hw/core/machine.c
index d856485cb4d..dac82a709ba 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -37,7 +37,9 @@
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/virtio-pci.h"

-GlobalProperty hw_compat_6_2[] = {};
+GlobalProperty hw_compat_6_2[] = {
+{ "arm-cpu-max", "lpa2", "false" },
+};
 const size_t hw_compat_6_2_len = G_N_ELEMENTS(hw_compat_6_2);

 GlobalProperty hw_compat_6_1[] = {

thanks
-- PMM

[PATCH v5 0/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Ani Sinha]

This can allow the guest OS to determine more easily if i8042 controller
is present in the system or not, so it doesn't need to do probing of the
controller, but just initialize it immediately, before enumerating the
ACPI AML namespace.

To allow "flexible" indication, I don't hardcode the bit at location 1
as on in the IA-PC boot flags, but try to search for i8042 on the ISA
bus to verify it exists in the system.

Why this is useful you might ask - this patch allows the guest OS to
probe and use the i8042 controller without decoding the ACPI AML blob
at all. For example, as a developer of the SerenityOS kernel, I might
want to allow people to not try to decode the ACPI AML namespace (for
now, we still don't support ACPI AML as it's a work in progress), but
still to not probe for the i8042 but just use it after looking in the
IA-PC boot flags in the ACPI FADT table.

Changelog:
v5:
Addressed review comments from v4. Also got rid of microvm changes. Will send
them in a separate patch.

Liav Albani (3):
  tests/acpi: i386: allow FACP acpi table changes
  hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT
table
  tests/acpi: i386: update FACP table differences

 hw/acpi/aml-build.c|   8 +++-
 hw/i386/acpi-build.c   |   8 
 include/hw/acpi/acpi-defs.h|   1 +
 include/hw/input/i8042.h   |   6 ++
 tests/data/acpi/q35/FACP   | Bin 244 -> 244 bytes
 tests/data/acpi/q35/FACP.nosmm | Bin 244 -> 244 bytes
 tests/data/acpi/q35/FACP.slic  | Bin 244 -> 244 bytes
 tests/data/acpi/q35/FACP.xapic | Bin 244 -> 244 bytes
 8 files changed, 22 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH v5 2/3] hw/acpi: add indication for i8042 in IA-PC boot flags of the FADT table

[Ani Sinha]

From: Liav Albani 

This can allow the guest OS to determine more easily if i8042 controller
is present in the system or not, so it doesn't need to do probing of the
controller, but just initialize it immediately, before enumerating the
ACPI AML namespace.

This change only applies to the x86/q35 machine type, as it uses FACP
ACPI table with revision higher than 1, which should implement at least
ACPI 2.0 features within the table, hence it can also set the IA-PC boot
flags register according to the ACPI 2.0 specification.

Signed-off-by: Liav Albani 
Signed-off-by: Ani Sinha 
---
 hw/acpi/aml-build.c | 8 +++-
 hw/i386/acpi-build.c| 8 
 include/hw/acpi/acpi-defs.h | 1 +
 include/hw/input/i8042.h| 6 ++
 4 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 8966e16320..1773cf55f1 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2152,7 +2152,13 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const 
AcpiFadtData *f,
 build_append_int_noprefix(tbl, 0, 1); /* DAY_ALRM */
 build_append_int_noprefix(tbl, 0, 1); /* MON_ALRM */
 build_append_int_noprefix(tbl, f->rtc_century, 1); /* CENTURY */
-build_append_int_noprefix(tbl, 0, 2); /* IAPC_BOOT_ARCH */
+/* IAPC_BOOT_ARCH */
+if (f->rev == 1) {
+build_append_int_noprefix(tbl, 0, 2);
+} else {
+/* since ACPI v2.0 */
+build_append_int_noprefix(tbl, f->iapc_boot_arch, 2);
+}
 build_append_int_noprefix(tbl, 0, 1); /* Reserved */
 build_append_int_noprefix(tbl, f->flags, 4); /* Flags */
 
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebd47aa26f..28ca75fb50 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -38,6 +38,7 @@
 #include "hw/nvram/fw_cfg.h"
 #include "hw/acpi/bios-linker-loader.h"
 #include "hw/isa/isa.h"
+#include "hw/input/i8042.h"
 #include "hw/block/fdc.h"
 #include "hw/acpi/memory_hotplug.h"
 #include "sysemu/tpm.h"
@@ -192,6 +193,13 @@ static void init_common_fadt_data(MachineState *ms, Object 
*o,
 .address = object_property_get_uint(o, ACPI_PM_PROP_GPE0_BLK, NULL)
 },
 };
+
+/*
+ * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+ * Flags, bit offset 1 - 8042.
+ */
+fadt.iapc_boot_arch = i8042_present() ? 0x1 << 1 : 0x0;
+
 *data = fadt;
 }
 
diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
index c97e8633ad..2b42e4192b 100644
--- a/include/hw/acpi/acpi-defs.h
+++ b/include/hw/acpi/acpi-defs.h
@@ -77,6 +77,7 @@ typedef struct AcpiFadtData {
 uint16_t plvl2_lat;/* P_LVL2_LAT */
 uint16_t plvl3_lat;/* P_LVL3_LAT */
 uint16_t arm_boot_arch;/* ARM_BOOT_ARCH */
+uint16_t iapc_boot_arch;   /* IAPC_BOOT_ARCH */
 uint8_t minor_ver; /* FADT Minor Version */
 
 /*
diff --git a/include/hw/input/i8042.h b/include/hw/input/i8042.h
index 1d90432dae..c739f30be8 100644
--- a/include/hw/input/i8042.h
+++ b/include/hw/input/i8042.h
@@ -23,4 +23,10 @@ void i8042_mm_init(qemu_irq kbd_irq, qemu_irq mouse_irq,
 void i8042_isa_mouse_fake_event(ISAKBDState *isa);
 void i8042_setup_a20_line(ISADevice *dev, qemu_irq a20_out);
 
+static inline bool i8042_present(void)
+{
+bool amb = false;
+return object_resolve_path_type("", TYPE_I8042, &amb) || amb;
+}
+
 #endif /* HW_INPUT_I8042_H */
-- 
2.25.1

Re: [PATCH v4 17/18] target/arm: Provide cpu property for controling FEAT_LPA2

[Peter Maydell]

On Tue, 1 Mar 2022 at 22:00, Richard Henderson
 wrote:
>
> There is a Linux kernel bug present until v5.12 that prevents
> booting with FEAT_LPA2 enabled.  As a workaround for TCG, allow
> the feature to be disabled from -cpu max.
>
> Since this kernel bug is present in the Fedora 31 image that
> we test in avocado, disable lpa2 on the command-line.
>
> Signed-off-by: Richard Henderson 
> ---

Reviewed-by: Peter Maydell 

thanks
-- PMM

[PATCH v5 3/3] tests/acpi: i386: update FACP table differences

[Ani Sinha]

From: Liav Albani 

After changing the IAPC boot flags register to indicate support of i8042
in the machine chipset to help the guest OS to determine its existence
"faster", we need to have the updated FACP ACPI binary images in tree.

The ASL changes introduced are shown by the following diff:

@@ -42,35 +42,35 @@
 [059h 0089   1] PM1 Control Block Length : 02
 [05Ah 0090   1] PM2 Control Block Length : 00
 [05Bh 0091   1]PM Timer Block Length : 04
 [05Ch 0092   1]GPE0 Block Length : 10
 [05Dh 0093   1]GPE1 Block Length : 00
 [05Eh 0094   1] GPE1 Base Offset : 00
 [05Fh 0095   1] _CST Support : 00
 [060h 0096   2]   C2 Latency : 0FFF
 [062h 0098   2]   C3 Latency : 0FFF
 [064h 0100   2]   CPU Cache Size : 
 [066h 0102   2]   Cache Flush Stride : 
 [068h 0104   1]Duty Cycle Offset : 00
 [069h 0105   1] Duty Cycle Width : 00
 [06Ah 0106   1]  RTC Day Alarm Index : 00
 [06Bh 0107   1]RTC Month Alarm Index : 00
 [06Ch 0108   1]RTC Century Index : 32
-[06Dh 0109   2]   Boot Flags (decoded below) : 
+[06Dh 0109   2]   Boot Flags (decoded below) : 0002
Legacy Devices Supported (V2) : 0
-8042 Present on ports 60/64 (V2) : 0
+8042 Present on ports 60/64 (V2) : 1
 VGA Not Present (V4) : 0
   MSI Not Supported (V4) : 0
 PCIe ASPM Not Supported (V4) : 0
CMOS RTC Not Present (V5) : 0
 [06Fh 0111   1] Reserved : 00
 [070h 0112   4]Flags (decoded below) : 84A5
   WBINVD instruction is operational (V1) : 1
   WBINVD flushes all caches (V1) : 0
 All CPUs support C1 (V1) : 1
   C2 works on MP system (V1) : 0
 Control Method Power Button (V1) : 0
 Control Method Sleep Button (V1) : 1
 RTC wake not in fixed reg space (V1) : 0
 RTC can wake system from S4 (V1) : 1
 32-bit PM Timer (V1) : 0
   Docking Supported (V1) : 0

Signed-off-by: Liav Albani 
Acked-by: Ani Sinha 
---
 tests/data/acpi/q35/FACP| Bin 244 -> 244 bytes
 tests/data/acpi/q35/FACP.nosmm  | Bin 244 -> 244 bytes
 tests/data/acpi/q35/FACP.slic   | Bin 244 -> 244 bytes
 tests/data/acpi/q35/FACP.xapic  | Bin 244 -> 244 bytes
 tests/qtest/bios-tables-test-allowed-diff.h |   4 
 5 files changed, 4 deletions(-)

diff --git a/tests/data/acpi/q35/FACP b/tests/data/acpi/q35/FACP
index 
f6a864cc863c7763f6c09d3814ad184a658fa0a0..a8f6a8961109d01059aceef9f1869cde09a2f10c
 100644
GIT binary patch
delta 23
ecmeyu_=S

[PATCH v5 1/3] tests/acpi: i386: allow FACP acpi table changes

[Ani Sinha]

From: Liav Albani 

The FACP table is going to be changed for x86/q35 machines. To be sure
the following changes are not breaking any QEMU test this change follows
step 2 from the bios-tables-test.c guide on changes that affect ACPI
tables.

Signed-off-by: Liav Albani 
Acked-by: Ani Sinha 
---
 tests/qtest/bios-tables-test-allowed-diff.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8b..7570e39369 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,5 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/q35/FACP",
+"tests/data/acpi/q35/FACP.nosmm",
+"tests/data/acpi/q35/FACP.slic",
+"tests/data/acpi/q35/FACP.xapic",
-- 
2.25.1

[PATCH 0/4] softmmu: move and refactor -runas, -chroot and -daemonize

[Daniel P . Berrangé]

This small series was motivated by my thoughts on the proposals in

  https://lists.gnu.org/archive/html/qemu-devel/2022-03/msg01135.html

It demostrates the approach I mention there, and has the further
benefit of untangling and isolating the implementation of UID
changing, chrooting and daemonized, from the parsing of the
corresponding command line options.

Daniel P. Berrangé (4):
  softmmu: remove deprecated --enable-fips option
  os-posix: refactor code handling the -runas argument
  os-posix: refactor code handling the -chroot argument
  softmmu: move parsing of -runas, -chroot and -daemonize code

 docs/about/deprecated.rst   |  12 --
 docs/about/removed-features.rst |  11 ++
 include/qemu/osdep.h|   3 -
 include/sysemu/os-posix.h   |   4 +-
 include/sysemu/os-win32.h   |   1 -
 os-posix.c  | 222 ++--
 os-win32.c  |   9 --
 qemu-options.hx |  10 --
 softmmu/vl.c|  76 ++-
 ui/vnc.c|   7 -
 util/osdep.c|  28 
 11 files changed, 154 insertions(+), 229 deletions(-)

-- 
2.34.1

[PATCH 2/4] os-posix: refactor code handling the -runas argument

[Daniel P . Berrangé]

Change the change_process_uid() function so that it takes its input as
parameters instead of relying on static global variables.

Signed-off-by: Daniel P. Berrangé 
---
 os-posix.c | 83 +-
 1 file changed, 39 insertions(+), 44 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 7cd662098e..5a127feee2 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,13 +42,9 @@
 #include 
 #endif
 
-/*
- * Must set all three of these at once.
- * Legal combinations are  unset   by name   by uid
- */
-static struct passwd *user_pwd;/*   NULL   non-NULL   NULL   */
-static uid_t user_uid = (uid_t)-1; /*   -1  -1>=0*/
-static gid_t user_gid = (gid_t)-1; /*   -1  -1>=0*/
+static char *user_name;
+static uid_t user_uid = (uid_t)-1;
+static gid_t user_gid = (gid_t)-1;
 
 static const char *chroot_dir;
 static int daemonize;
@@ -100,7 +96,8 @@ void os_set_proc_name(const char *s)
 }
 
 
-static bool os_parse_runas_uid_gid(const char *optarg)
+static bool os_parse_runas_uid_gid(const char *optarg,
+   uid_t *runas_uid, gid_t *runas_gid)
 {
 unsigned long lv;
 const char *ep;
@@ -120,9 +117,8 @@ static bool os_parse_runas_uid_gid(const char *optarg)
 return false;
 }
 
-user_pwd = NULL;
-user_uid = got_uid;
-user_gid = got_gid;
+*runas_uid = got_uid;
+*runas_gid = got_gid;
 return true;
 }
 
@@ -132,13 +128,18 @@ static bool os_parse_runas_uid_gid(const char *optarg)
  */
 int os_parse_cmd_args(int index, const char *optarg)
 {
+struct passwd *user_pwd;
+
 switch (index) {
 case QEMU_OPTION_runas:
 user_pwd = getpwnam(optarg);
 if (user_pwd) {
-user_uid = -1;
-user_gid = -1;
-} else if (!os_parse_runas_uid_gid(optarg)) {
+user_uid = user_pwd->pw_uid;
+user_gid = user_pwd->pw_gid;
+user_name = g_strdup(user_pwd->pw_name);
+} else if (!os_parse_runas_uid_gid(optarg,
+   &user_uid,
+   &user_gid)) {
 error_report("User \"%s\" doesn't exist"
  " (and is not :)",
  optarg);
@@ -158,41 +159,33 @@ int os_parse_cmd_args(int index, const char *optarg)
 return 0;
 }
 
-static void change_process_uid(void)
+static void change_process_uid(uid_t uid, gid_t gid, const char *name)
 {
-assert((user_uid == (uid_t)-1) || user_pwd == NULL);
-assert((user_uid == (uid_t)-1) ==
-   (user_gid == (gid_t)-1));
-
-if (user_pwd || user_uid != (uid_t)-1) {
-gid_t intended_gid = user_pwd ? user_pwd->pw_gid : user_gid;
-uid_t intended_uid = user_pwd ? user_pwd->pw_uid : user_uid;
-if (setgid(intended_gid) < 0) {
-error_report("Failed to setgid(%d)", intended_gid);
-exit(1);
-}
-if (user_pwd) {
-if (initgroups(user_pwd->pw_name, user_pwd->pw_gid) < 0) {
-error_report("Failed to initgroups(\"%s\", %d)",
-user_pwd->pw_name, user_pwd->pw_gid);
-exit(1);
-}
-} else {
-if (setgroups(1, &user_gid) < 0) {
-error_report("Failed to setgroups(1, [%d])",
-user_gid);
-exit(1);
-}
-}
-if (setuid(intended_uid) < 0) {
-error_report("Failed to setuid(%d)", intended_uid);
+if (setgid(gid) < 0) {
+error_report("Failed to setgid(%d)", gid);
+exit(1);
+}
+if (name) {
+if (initgroups(name, gid) < 0) {
+error_report("Failed to initgroups(\"%s\", %d)",
+ name, gid);
 exit(1);
 }
-if (setuid(0) != -1) {
-error_report("Dropping privileges failed");
+} else {
+if (setgroups(1, &gid) < 0) {
+error_report("Failed to setgroups(1, [%d])",
+ gid);
 exit(1);
 }
 }
+if (setuid(uid) < 0) {
+error_report("Failed to setuid(%d)", uid);
+exit(1);
+}
+if (setuid(0) != -1) {
+error_report("Dropping privileges failed");
+exit(1);
+}
 }
 
 static void change_root(void)
@@ -275,7 +268,9 @@ void os_setup_post(void)
 }
 
 change_root();
-change_process_uid();
+if (user_uid != -1 && user_gid != -1) {
+change_process_uid(user_uid, user_gid, user_name);
+}
 
 if (daemonize) {
 uint8_t status = 0;
-- 
2.34.1

[PATCH 4/4] softmmu: move parsing of -runas, -chroot and -daemonize code

[Daniel P . Berrangé]

With the future intent to try to move to a fully QAPI driven
configuration system, we want to have any current command
parsing well isolated from logic that applies the resulting
configuration.

We also don't want os-posix.c to contain code that is specific
to the system emulators, as this file is linked to other binaries
too.

To satisfy these goals, we move parsing of the -runas, -chroot and
-daemonize code out of the os-posix.c helper code, and pass the
parsed data into APIs in os-posix.c.

As a side benefit the 'os_daemonize()' function now lives upto to
its name and will always daemonize, instead of using global state
to decide to be a no-op sometimes.

Signed-off-by: Daniel P. Berrangé 
---
 include/sysemu/os-posix.h |   4 +-
 include/sysemu/os-win32.h |   1 -
 os-posix.c| 148 +++---
 os-win32.c|   9 ---
 softmmu/vl.c  |  76 ++--
 5 files changed, 113 insertions(+), 125 deletions(-)

diff --git a/include/sysemu/os-posix.h b/include/sysemu/os-posix.h
index 2edf33658a..390f3f8396 100644
--- a/include/sysemu/os-posix.h
+++ b/include/sysemu/os-posix.h
@@ -46,7 +46,9 @@ void os_set_line_buffering(void);
 void os_set_proc_name(const char *s);
 void os_setup_signal_handling(void);
 void os_daemonize(void);
-void os_setup_post(void);
+void os_setup_post(const char *chroot_dir,
+   uid_t uid, gid_t gid,
+   const char *username);
 int os_mlock(void);
 
 #define closesocket(s) close(s)
diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
index 43f569b5c2..4879f8731d 100644
--- a/include/sysemu/os-win32.h
+++ b/include/sysemu/os-win32.h
@@ -61,7 +61,6 @@ struct tm *localtime_r(const time_t *timep, struct tm 
*result);
 
 static inline void os_setup_signal_handling(void) {}
 static inline void os_daemonize(void) {}
-static inline void os_setup_post(void) {}
 void os_set_line_buffering(void);
 static inline void os_set_proc_name(const char *dummy) {}
 
diff --git a/os-posix.c b/os-posix.c
index 30da1a1491..f598a52a4f 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -42,11 +42,6 @@
 #include 
 #endif
 
-static char *user_name;
-static uid_t user_uid = (uid_t)-1;
-static gid_t user_gid = (gid_t)-1;
-
-static const char *chroot_dir;
 static int daemonize;
 static int daemon_pipe;
 
@@ -96,69 +91,6 @@ void os_set_proc_name(const char *s)
 }
 
 
-static bool os_parse_runas_uid_gid(const char *optarg,
-   uid_t *runas_uid, gid_t *runas_gid)
-{
-unsigned long lv;
-const char *ep;
-uid_t got_uid;
-gid_t got_gid;
-int rc;
-
-rc = qemu_strtoul(optarg, &ep, 0, &lv);
-got_uid = lv; /* overflow here is ID in C99 */
-if (rc || *ep != ':' || got_uid != lv || got_uid == (uid_t)-1) {
-return false;
-}
-
-rc = qemu_strtoul(ep + 1, 0, 0, &lv);
-got_gid = lv; /* overflow here is ID in C99 */
-if (rc || got_gid != lv || got_gid == (gid_t)-1) {
-return false;
-}
-
-*runas_uid = got_uid;
-*runas_gid = got_gid;
-return true;
-}
-
-/*
- * Parse OS specific command line options.
- * return 0 if option handled, -1 otherwise
- */
-int os_parse_cmd_args(int index, const char *optarg)
-{
-struct passwd *user_pwd;
-
-switch (index) {
-case QEMU_OPTION_runas:
-user_pwd = getpwnam(optarg);
-if (user_pwd) {
-user_uid = user_pwd->pw_uid;
-user_gid = user_pwd->pw_gid;
-user_name = g_strdup(user_pwd->pw_name);
-} else if (!os_parse_runas_uid_gid(optarg,
-   &user_uid,
-   &user_gid)) {
-error_report("User \"%s\" doesn't exist"
- " (and is not :)",
- optarg);
-exit(1);
-}
-break;
-case QEMU_OPTION_chroot:
-chroot_dir = optarg;
-break;
-case QEMU_OPTION_daemonize:
-daemonize = 1;
-break;
-default:
-return -1;
-}
-
-return 0;
-}
-
 static void change_process_uid(uid_t uid, gid_t gid, const char *name)
 {
 if (setgid(gid) < 0) {
@@ -202,54 +134,56 @@ static void change_root(const char *root)
 
 void os_daemonize(void)
 {
-if (daemonize) {
-pid_t pid;
-int fds[2];
+pid_t pid;
+int fds[2];
 
-if (pipe(fds) == -1) {
-exit(1);
-}
+if (pipe(fds) == -1) {
+exit(1);
+}
 
-pid = fork();
-if (pid > 0) {
-uint8_t status;
-ssize_t len;
+pid = fork();
+if (pid > 0) {
+uint8_t status;
+ssize_t len;
 
-close(fds[1]);
+close(fds[1]);
 
-do {
-len = read(fds[0], &status, 1);
-} while (len < 0 && errno == EINTR);
+do {
+len = read(fds[0], &status, 1);
+} while (len < 0 && errno == EINTR);
 
-/* only 

[PATCH 1/4] softmmu: remove deprecated --enable-fips option

[Daniel P . Berrangé]

Users requiring FIPS support must build QEMU with either the libgcrypt
or gnutls libraries for as the crytography backend.

Signed-off-by: Daniel P. Berrangé 
---
 docs/about/deprecated.rst   | 12 
 docs/about/removed-features.rst | 11 +++
 include/qemu/osdep.h|  3 ---
 os-posix.c  |  8 
 qemu-options.hx | 10 --
 ui/vnc.c|  7 ---
 util/osdep.c| 28 
 7 files changed, 11 insertions(+), 68 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 26d00812ba..a458dd453c 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -67,18 +67,6 @@ and will cause a warning.
 The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
 rather than ``delay=off``.
 
-``--enable-fips`` (since 6.0)
-'
-
-This option restricts usage of certain cryptographic algorithms when
-the host is operating in FIPS mode.
-
-If FIPS compliance is required, QEMU should be built with the ``libgcrypt``
-library enabled as a cryptography provider.
-
-Neither the ``nettle`` library, or the built-in cryptography provider are
-supported on FIPS enabled hosts.
-
 ``-writeconfig`` (since 6.0)
 '
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index cb0575fd49..6ca66f658d 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -336,6 +336,17 @@ for the RISC-V ``virt`` machine and ``sifive_u`` machine.
 The ``-no-quit`` was a synonym for ``-display ...,window-close=off`` which
 should be used instead.
 
+``--enable-fips`` (removed in 7.0)
+''
+
+This option restricted usage of certain cryptographic algorithms when
+the host is operating in FIPS mode.
+
+If FIPS compliance is required, QEMU should be built with the ``libgcrypt``
+or ``gnutls`` library enabled as a cryptography provider.
+
+Neither the ``nettle`` library, or the built-in cryptography provider are
+supported on FIPS enabled hosts.
 
 QEMU Machine Protocol (QMP) commands
 
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 7bcce3bceb..66e70e24ff 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -534,9 +534,6 @@ static inline void qemu_timersub(const struct timeval *val1,
 
 void qemu_set_cloexec(int fd);
 
-void fips_set_state(bool requested);
-bool fips_get_state(void);
-
 /* Return a dynamically allocated pathname denoting a file or directory that is
  * appropriate for storing local state.
  *
diff --git a/os-posix.c b/os-posix.c
index ae6c9f2a5e..7cd662098e 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -151,14 +151,6 @@ int os_parse_cmd_args(int index, const char *optarg)
 case QEMU_OPTION_daemonize:
 daemonize = 1;
 break;
-#if defined(CONFIG_LINUX)
-case QEMU_OPTION_enablefips:
-warn_report("-enable-fips is deprecated, please build QEMU with "
-"the `libgcrypt` library as the cryptography provider "
-"to enable FIPS compliance");
-fips_set_state(true);
-break;
-#endif
 default:
 return -1;
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index 094a6c1d7c..cb0c58904b 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4655,16 +4655,6 @@ HXCOMM Internal use
 DEF("qtest", HAS_ARG, QEMU_OPTION_qtest, "", QEMU_ARCH_ALL)
 DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL)
 
-#ifdef __linux__
-DEF("enable-fips", 0, QEMU_OPTION_enablefips,
-"-enable-fipsenable FIPS 140-2 compliance\n",
-QEMU_ARCH_ALL)
-#endif
-SRST
-``-enable-fips``
-Enable FIPS 140-2 compliance mode.
-ERST
-
 DEF("msg", HAS_ARG, QEMU_OPTION_msg,
 "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n"
 "control error message format\n"
diff --git a/ui/vnc.c b/ui/vnc.c
index 3ccd33dedc..82b28aec95 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -4051,13 +4051,6 @@ void vnc_display_open(const char *id, Error **errp)
 password = qemu_opt_get_bool(opts, "password", false);
 }
 if (password) {
-if (fips_get_state()) {
-error_setg(errp,
-   "VNC password auth disabled due to FIPS mode, "
-   "consider using the VeNCrypt or SASL authentication "
-   "methods as an alternative");
-goto fail;
-}
 if (!qcrypto_cipher_supports(
 QCRYPTO_CIPHER_ALG_DES, QCRYPTO_CIPHER_MODE_ECB)) {
 error_setg(errp,
diff --git a/util/osdep.c b/util/osdep.c
index 723cdcb004..456df9e81a 100644
--- a/util/osdep.c
+++ b/util/osdep.c
@@ -43,8 +43,6 @@ extern int madvise(char *, size_t, int);
 #include "qemu/hw-version.h"
 #include "monitor/monitor.h"
 
-static bool fips_enabled = false;
-
 static const char *hw_version = 

[PATCH 3/4] os-posix: refactor code handling the -chroot argument

[Daniel P . Berrangé]

Change the change_root() function so that it takes its input as
parameters instead of relying on static global variables.

Signed-off-by: Daniel P. Berrangé 
---
 os-posix.c | 23 +++
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/os-posix.c b/os-posix.c
index 5a127feee2..30da1a1491 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -188,19 +188,16 @@ static void change_process_uid(uid_t uid, gid_t gid, 
const char *name)
 }
 }
 
-static void change_root(void)
+static void change_root(const char *root)
 {
-if (chroot_dir) {
-if (chroot(chroot_dir) < 0) {
-error_report("chroot failed");
-exit(1);
-}
-if (chdir("/")) {
-error_report("not able to chdir to /: %s", strerror(errno));
-exit(1);
-}
+if (chroot(root) < 0) {
+error_report("chroot failed");
+exit(1);
+}
+if (chdir("/")) {
+error_report("not able to chdir to /: %s", strerror(errno));
+exit(1);
 }
-
 }
 
 void os_daemonize(void)
@@ -267,7 +264,9 @@ void os_setup_post(void)
 }
 }
 
-change_root();
+if (chroot_dir) {
+change_root(chroot_dir);
+}
 if (user_uid != -1 && user_gid != -1) {
 change_process_uid(user_uid, user_gid, user_name);
 }
-- 
2.34.1

Re: [PATCH v2 1/4] os-posix: Add os_set_daemonize()

[Daniel P . Berrangé]

On Fri, Mar 04, 2022 at 11:20:39AM +0100, Kevin Wolf wrote:
> Am 04.03.2022 um 10:19 hat Daniel P. Berrangé geschrieben:
> > On Thu, Mar 03, 2022 at 05:48:11PM +0100, Hanna Reitz wrote:
> > > The daemonizing functions in os-posix (os_daemonize() and
> > > os_setup_post()) only daemonize the process if the static `daemonize`
> > > variable is set.  Right now, it can only be set by os_parse_cmd_args().
> > > 
> > > In order to use os_daemonize() and os_setup_post() from the storage
> > > daemon to have it be daemonized, we need some other way to set this
> > > `daemonize` variable, because I would rather not tap into the system
> > > emulator's arg-parsing code.  Therefore, this patch adds an
> > > os_set_daemonize() function, which will return an error on os-win32
> > > (because daemonizing is not supported there).
> > 
> > IMHO the real flaw here is the design of 'os_daemonize' in that it
> > relies on static state. If I see a call to a function 'os_daemonize()'
> > I expect to be daemonized on return, but with this design that is not
> > guaranteed which is a big surprise.
> > 
> > I'd suggest we push the condition into the caller instead of adding
> > this extra function, so we have the more sane pattern:
> > 
> >if (daemonmize()) {
> >   os_daemonize()
> >}
> 
> It's not as simple, the static daemonize variable is used in more places
> than just os_daemonize(). I'm not sure if it's worth changing how all of
> this works, but if we did, it would be a refactoring mostly focussed on
> the system emulator and an issue separate from adding the option to the
> storage daemon.

It isn't that difficult to do the refactoring needed, so I've just
sent a series that does the job and CC folks from this thread on it.

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

Re: [PATCH 0/2] virtiofsd: Support FUSE_SYNCFS on unannounced submounts

[Vivek Goyal]

On Thu, Mar 03, 2022 at 06:13:21PM +0100, Greg Kurz wrote:
> This is the current patches I have : one to track submounts
> and the other to call syncfs() on them. Tested on simple
> cases only.
> 
> I won't be able to work on this anymore, so I'm posting for the
> records. Anyone is welcome to pick it up as there won't be a v2
> from my side.

Thanks Greg. Hopefully somebody else will be able to pick it up.

What are TODO items to take this patch series to completion.

Vivek

> 
> Cheers,
> 
> --
> Greg
> 
> Greg Kurz (2):
>   virtiofsd: Track submounts
>   virtiofsd: Support FUSE_SYNCFS on unannounced submounts
> 
>  tools/virtiofsd/passthrough_ll.c | 61 
>  1 file changed, 55 insertions(+), 6 deletions(-)
> 
> -- 
> 2.34.1
> 
> 

Re: [PATCH v2 1/3] block: Make bdrv_refresh_limits() non-recursive

[Hanna Reitz]

On 03.03.22 17:56, Kevin Wolf wrote:

Am 16.02.2022 um 11:53 hat Hanna Reitz geschrieben:

bdrv_refresh_limits() recurses down to the node's children.  That does
not seem necessary: When we refresh limits on some node, and then
recurse down and were to change one of its children's BlockLimits, then
that would mean we noticed the changed limits by pure chance.  The fact
that we refresh the parent's limits has nothing to do with it, so the
reason for the change probably happened before this point in time, and
we should have refreshed the limits then.

On the other hand, we do not have infrastructure for noticing that block
limits change after they have been initialized for the first time (this
would require propagating the change upwards to the respective node's
parents), and so evidently we consider this case impossible.

I like your optimistic approach, but my interpretation would have been
that this is simply a bug. ;-)

blockdev-reopen allows changing options that affect the block limits
(most importantly probably request_alignment), so this should be
propagated to the parents. I think we'll actually not see failures if we
forget to do this, but parents can either advertise excessive alignment
requirements or they may run into RMW when accessing the child, so this
would only affect performance. This is probably why nobody reported it
yet.


Ah, right, I forgot this for parents of parents...  I thought the block 
limits of a node might change if its children list changes, and so we 
should bdrv_refresh_limits() when that children list changes, but forgot 
that we really do need to propagate this up, right.



If this case is impossible, then we will not need to recurse down in
bdrv_refresh_limits().  Every node's limits are initialized in
bdrv_open_driver(), and are refreshed whenever its children change.
We want to use the childrens' limits to get some initial default, but
we can just take them, we do not need to refresh them.

I think even if we need to propagate to the parents, we still don't need
to propagate to the children because the children have already been
refreshed by whatever changed their options (like bdrv_reopen_commit()).
And parent limits don't influence the child limits at all.

So this patch looks good to me, just not the reasoning.


OK, so, uh, can we just drop these two paragraphs?  (“On the other 
hand...” and “If this case is impossible…”)


Or we could replace them with a note hinting at the potential bug that 
would need to be fixed, e.g.


“
Consequently, we should actually propagate block limits changes upwards,
not downwards.  That is a separate and pre-existing issue, though, and
so will not be addressed in this patch.
”

Question is, if we at some point do propagate this upwards, won’t this 
cause exactly the same problem that this patch is trying to get around, 
i.e. that we might call bdrv_refresh_limits() on non-drained parent nodes?


Hanna


Kevin


The problem with recursing is that bdrv_refresh_limits() is not atomic.
It begins with zeroing BDS.bl, and only then sets proper, valid limits.
If we do not drain all nodes whose limits are refreshed, then concurrent
I/O requests can encounter invalid request_alignment values and crash
qemu.  Therefore, a recursing bdrv_refresh_limits() requires the whole
subtree to be drained, which is currently not ensured by most callers.

A non-recursive bdrv_refresh_limits() only requires the node in question
to not receive I/O requests, and this is done by most callers in some
way or another:
- bdrv_open_driver() deals with a new node with no parents yet
- bdrv_set_file_or_backing_noperm() acts on a drained node
- bdrv_reopen_commit() acts only on drained nodes
- bdrv_append() should in theory require the node to be drained; in
   practice most callers just lock the AioContext, which should at least
   be enough to prevent concurrent I/O requests from accessing invalid
   limits

So we can resolve the bug by making bdrv_refresh_limits() non-recursive.

Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1879437
Signed-off-by: Hanna Reitz 
Reviewed-by: Eric Blake 
---
  block/io.c | 4 
  1 file changed, 4 deletions(-)

diff --git a/block/io.c b/block/io.c
index 4e4cb556c5..c3e7301613 100644
--- a/block/io.c
+++ b/block/io.c
@@ -189,10 +189,6 @@ void bdrv_refresh_limits(BlockDriverState *bs, Transaction 
*tran, Error **errp)
  QLIST_FOREACH(c, &bs->children, next) {
  if (c->role & (BDRV_CHILD_DATA | BDRV_CHILD_FILTERED | 
BDRV_CHILD_COW))
  {
-bdrv_refresh_limits(c->bs, tran, errp);
-if (*errp) {
-return;
-}
  bdrv_merge_limits(&bs->bl, &c->bs->bl);
  have_limits = true;
  }
--
2.34.1

Re: [PATCH v3 4/5] hw/intc: Vectored Interrupt Controller (VIC)

[Peter Maydell]

On Thu, 3 Mar 2022 at 15:39, Amir Gonnen  wrote:
>
> Implement nios2 Vectored Interrupt Controller (VIC).
> VIC is connected to EIC. It needs to update rha, ril, rrs and rnmi
> fields on Nios2CPU before raising an IRQ.
> For that purpose, VIC has a "cpu" property which should refer to the
> nios2 cpu and set by the board that connects VIC.
>
> Signed-off-by: Amir Gonnen 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v3 1/5] target/nios2: Check supervisor on eret

[Peter Maydell]

On Thu, 3 Mar 2022 at 15:39, Amir Gonnen  wrote:
>
> eret instruction is only allowed in supervisor mode.
>
> Signed-off-by: Amir Gonnen 
> ---
>  target/nios2/translate.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/target/nios2/translate.c b/target/nios2/translate.c
> index ce3aacf59d..007c17e6e9 100644
> --- a/target/nios2/translate.c
> +++ b/target/nios2/translate.c
> @@ -384,6 +384,8 @@ static const Nios2Instruction i_type_instructions[] = {
>   */
>  static void eret(DisasContext *dc, uint32_t code, uint32_t flags)
>  {
> +gen_check_supervisor(dc);
> +
>  tcg_gen_mov_tl(cpu_R[CR_STATUS], cpu_R[CR_ESTATUS]);
>  tcg_gen_mov_tl(cpu_R[R_PC], cpu_R[R_EA]);
>
> --

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v3 5/5] hw/nios2: Machine with a Vectored Interrupt Controller

[Peter Maydell]

On Thu, 3 Mar 2022 at 15:39, Amir Gonnen  wrote:
>
> Demonstrate how to use nios2 VIC on a machine.
> Introduce a new machine "10m50-ghrd-vic" which is based on "10m50-ghrd"
> with a VIC attached and internal interrupt controller removed.
>
> When VIC is present, irq0 connects the VIC to the cpu, intc_present
> is set to false to disable the internal interrupt controller, and the
> devices on the machine are attached to the VIC (and not directly to cpu).
> To allow VIC update EIC fields, we set the "cpu" property of the VIC
> with a reference to the nios2 cpu.
>
> Signed-off-by: Amir Gonnen 
> ---
>  hw/nios2/10m50_devboard.c | 64 ---
>  hw/nios2/Kconfig  |  1 +
>  2 files changed, 61 insertions(+), 4 deletions(-)

My remarks about this from the earlier version of the patchset still
stand:
 (1) if this isn't an actual config of the real hardware, then
presumably there aren't any guests which will run on it with the
VIC enabled
 (2) if we do want to do this, we should have a machine property
for "vic=true" rather than a complete new machine type

thanks
-- PMM

Re: [PATCH 3/5] avocado/boot_linux_console.py: check for tcg in test_ppc_powernv8/9

[Philippe Mathieu-Daudé]

On 3/3/22 16:35, Daniel Henrique Barboza wrote:

The PowerNV8/9 machines does not work with KVM acceleration, meaning
that boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8/9 tests
will always fail when QEMU is compiled with --disable-tcg:

ERROR 1-tests/avocado/boot_linux_console.py:BootLinuxConsole.test_ppc_powernv8
-> VMLaunchFailure: ConnectError: Failed to establish session:
[Errno 104] Connection reset by peer
 Exit code: 1
 Command: ./qemu-system-ppc64 -display none -vga none -chardev 
socket,id=mon,path=/var/tmp/avo_qemu_sock_no19zg0m/qemu-1936936-7fffa77cff98-monitor.sock
 -mon chardev=mon,mode=control -machine powernv8 -chardev 
socket,id=console,path=/var/tmp/avo_qemu_sock_no19zg0m/qemu-1936936-7fffa77cff98-console.sock,server=on,wait=off
 -serial chardev:console -kernel 
/home/danielhb/avocado/data/cache/by_location/4514304e2c4ee84c5f0b5c8bacedda783891df68/zImage.epapr
 -append console=tty0 console=hvc0 -device 
pcie-pci-bridge,id=bridge1,bus=pcie.1,addr=0x0 -device 
nvme,bus=pcie.2,addr=0x0,serial=1234 -device e1000e,bus=bridge1,addr=0x3 
-device nec-usb-xhci,bus=bridge1,addr=0x2
 Output: qemu-system-ppc64: The powernv machine does not work with KVM 
acceleration

Let's add the TCG accel requirement in both tests to skip them if we
don't have TCG support available.

Signed-off-by: Daniel Henrique Barboza 
---
  tests/avocado/boot_linux_console.py | 3 +++
  1 file changed, 3 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH V7 01/29] memory: qemu_check_ram_volatile

[Philippe Mathieu-Daudé]

On 22/12/21 20:05, Steve Sistare wrote:

Add a function that returns an error if any ram_list block represents
volatile memory.

Signed-off-by: Steve Sistare 
---
  include/exec/memory.h |  8 
  softmmu/memory.c  | 26 ++
  2 files changed, 34 insertions(+)

diff --git a/include/exec/memory.h b/include/exec/memory.h
index 20f1b27..137f5f3 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -2981,6 +2981,14 @@ bool ram_block_discard_is_disabled(void);
   */
  bool ram_block_discard_is_required(void);
  
+/**

+ * qemu_ram_check_volatile: return 1 if any memory regions are writable and not
+ * backed by shared memory, else return 0.
+ *
+ * @errp: returned error message identifying the first volatile region found.


This doesn't seem a good usage of the Error API. This is not an error
actually, but the expected result. If you want to return the first
or all, better use an explicit argument for them. Returning the first
is odd however. Is it useful for the user? If so, we want to return
them all, eventually in a GArray/GPtrArray, and return the MemoryRegion
handle, not its name. Otherwise if it is only useful for developers I'd
simply log the volatile MR name in a trace event.

Then we get:

  bool ram_block_is_volatile(void);

Or

  bool qemu_ram_is_volatile(void);


+ */
+int qemu_check_ram_volatile(Error **errp);




diff --git a/softmmu/memory.c b/softmmu/memory.c
index 7340e19..30b2f68 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -2837,6 +2837,32 @@ void memory_global_dirty_log_stop(unsigned int flags)
  memory_global_dirty_log_do_stop(flags);
  }
  
+static int check_volatile(RAMBlock *rb, void *opaque)


If using the 'qemu_ram_is_volatile' name for the public API,
this one could be 'static bool ram_block_is_volatile(...)'.


+{
+MemoryRegion *mr = rb->mr;
+
+if (mr &&
+memory_region_is_ram(mr) &&
+!memory_region_is_ram_device(mr) &&
+!memory_region_is_rom(mr) &&
+(rb->fd == -1 || !qemu_ram_is_shared(rb))) {
+*(const char **)opaque = memory_region_name(mr);
+return -1;
+}
+return 0;
+}
+
+int qemu_check_ram_volatile(Error **errp)
+{
+char *name;
+
+if (qemu_ram_foreach_block(check_volatile, &name)) {
+error_setg(errp, "Memory region %s is volatile", name);
+return -1;
+}
+return 0;
+}


Regards,

Phil.

Re: [PATCH] gdbstub.c: add support for info proc mappings

[Dominik Czarnota]

Hey,

I may work on this next week but I will probably not make it until the 8th :(.

On Thu, 3 Mar 2022 at 13:34, Alex Bennée  wrote:
>
>
> Disconnect3d  writes:
>
> > This commit adds support for `info proc mappings` and a few other commands 
> > into
> > the QEMU user-mode emulation gdbstub.
> >
> > For that, support for the following GDB remote protocol commands has been 
> > added:
> > * vFile:setfs: pid
> > * vFile:open: filename, flags, mode
> > * vFile:pread: fd, count, offset
> > * vFile:close: fd
> > * qXfer:exec-file:read:annex:offset,length
> >
> > Additionally, a `REMOTE_DEBUG_PRINT` macro has been added for printing 
> > remote debug logs.
> > To enable it, set the `ENABLE_REMOTE_DEBUG` definition to 1.
> >
> > All this can be tested with the following steps (commands from Ubuntu 
> > 20.04):
> > 1. Compiling an example program, e.g. for ARM:
> > echo 'int main() {puts("Hello world");}' | arm-linux-gnueabihf-gcc -xc -
> > 2. Running qemu-arm with gdbstub:
> > qemu-arm -g 1234 -L /usr/arm-linux-gnueabihf/ ./a.out
> > 3. Connecting to the gdbstub with GDB:
> > gdb-multiarch -ex 'target remote localhost:1234'
> > 4. Executing `info proc mappings` in GDB.
>
> It would be useful to add this to the check-tcg tests we do (there are
> already other examples (see run-gdbstub-FOO tests in
> tests/tcg/multiarch/Makefile.target)).
>

Thanks, I will check it out.

> >
> > The opening of files is done on behalf of the inferior by reusing the 
> > do_openat syscall.
> > Note that the current solution is still imperfect: while it allows us to 
> > fetch procfs
> > files like /proc/$pid/maps in the same way as the inferior is seeing them, 
> > there are
> > two downsides to it. First of all, it is indeed performed on behalf of the 
> > inferior.
> > Second of all, there are some files that the GDB tries to request like 
> > /lib/libc.so.6,
> > but they are usually not available as they do not exist in those paths and 
> > may need to
> > be served from the prefix provided in the -L flag to the qemu-user binary. 
> > I may try
> > to add a support for this in another patch and maybe refactor the solution 
> > to not use
> > the do_openat function directly.
> >
> > Before this commit, one would get (except of amd64, but not i386 targets):
> >
> > ```
> > (gdb) info proc mappings
> > process 1
> > warning: unable to open /proc file '/proc/1/maps'
> > ```
> >
> >
> > And after this commit, we should get something like:
> >
> > ```
> > (gdb) info proc mappings
> > process 3167519
> > Mapped address spaces:
> >
> >   Start Addr   End Addr   Size Offset objfile
> >   0x3f7d 0x3f7d1000 0x10000x0
> >   0x3f7d1000 0x3f7ed0000x1c0000x0 
> > /usr/arm-linux-gnueabihf/lib/ld-2.33.so
> >   0x3f7ed000 0x3f7fd0000x10x0
> >   0x3f7fd000 0x3f7ff000 0x20000x1c000 
> > /usr/arm-linux-gnueabihf/lib/ld-2.33.so
> >   0x3f7ff000 0x3f80 0x10000x0
> >   0x3f80 0x4000   0x800x0 [stack]
> >   0x4000 0x40001000 0x10000x0 /home/dc/src/qemu/a.out
> >   0x40001000 0x4001 0xf0000x0
> >   0x4001 0x40012000 0x20000x0 /home/dc/src/qemu/a.out
> > ```
> >
> >
> > However, on amd64 targets we would get and still get the following on the 
> > GDB side
> > (even after this commit):
> >
> > ```
> > (gdb) info proc mappings
> > Not supported on this target.
> > ```
> >
> > The x64 behavior is related to the fact that the GDB client does not 
> > initialize
> > some of its remote handlers properly when the gdbstub does not send an 
> > "orig_rax"
> > register in the target.xml file that describes the target. This happens in 
> > GDB in the
> > amd64_linux_init_abi function in the amd64-linux-tdep.c file [0]. The GDB 
> > tries to find
> > the "org.gnu.gdb.i386.linux" feature and the "orig_rax" register in it and 
> > if it is not
> > present, then it does not proceed with the `amd64_linux_init_abi_common 
> > (info, gdbarch, 2);` call
> > which initializes whatever is needed so that GDB fetches `info proc 
> > mappings` properly.
> >
> > I tried to fix this but just adding the orig_rax registry into the 
> > target.xml did not work
> > (there was some mismatch between the expected and sent register values; I 
> > guess the QEMU stub
> > would need to know how to send this register's value). On the other hand, 
> > this could also be
> > fixed on the GDB side. I will discuss this with GDB maintainers or/and 
> > propose a patch to GDB
> > related to this.
>
> Yeah there is debate about sticking to the "official" XML for certain
> arches vs just generating our own custom register XML which GDB doesn't
> deal with the special cases.
>
> >
> > [0] 
> > https://github.com/bminor/binutils-gdb/blob/dc5483c989f29fc9c7934965071ae1bb80cff902/gdb/amd64-linux-tdep.c#L1863-L1873
> >
> > Signed-off-by: Dominik 'Disconnect3d' Czarnota 
> > 
> > ---
> >  gdbstub.c|

[PULL 13/19] 9pfs: drop Doxygen format from qemu_dirent_dup() API comment

[Christian Schoenebeck]

API doc comments in QEMU are supposed to be in kerneldoc format, so drop
occurrences of "@c" which is Doxygen format for fixed-width text.

Link: 
https://lore.kernel.org/qemu-devel/cafeaca89+enom6x19oef53kd2dwkhn5sn21va0d7yepjsa3...@mail.gmail.com/
Based-on: 
Signed-off-by: Christian Schoenebeck 
Reviewed-by: Peter Maydell 
Reviewed-by: Greg Kurz 
Message-Id: 
---
 hw/9pfs/9p-util.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 8b92614e6c..22835c5f61 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -116,8 +116,8 @@ static inline off_t qemu_dirent_off(struct dirent *dent)
  * Duplicate directory entry @dent.
  *
  * It is highly recommended to use this function instead of open coding
- * duplication of @c dirent objects, because the actual @c struct @c dirent
- * size may be bigger or shorter than @c sizeof(struct dirent) and correct
+ * duplication of dirent objects, because the actual struct dirent
+ * size may be bigger or shorter than sizeof(struct dirent) and correct
  * handling is platform specific (see gitlab issue #841).
  *
  * @dent - original directory entry to be duplicated
-- 
2.20.1

[PULL 07/19] 9p: darwin: *xattr_nofollow implementations

[Christian Schoenebeck]

From: Keno Fischer 

This implements the darwin equivalent of the functions that were
moved to 9p-util(-linux) earlier in this series in the new
9p-util-darwin file.

Signed-off-by: Keno Fischer 
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch 
Signed-off-by: Will Cohen 
Message-Id: <20220227223522.91937-8-wwco...@gmail.com>
Signed-off-by: Christian Schoenebeck 
Acked-by: Christian Schoenebeck 
---
 hw/9pfs/9p-util-darwin.c | 64 
 hw/9pfs/meson.build  |  1 +
 2 files changed, 65 insertions(+)
 create mode 100644 hw/9pfs/9p-util-darwin.c

diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
new file mode 100644
index 00..cdb4c9e24c
--- /dev/null
+++ b/hw/9pfs/9p-util-darwin.c
@@ -0,0 +1,64 @@
+/*
+ * 9p utilities (Darwin Implementation)
+ *
+ * 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 "qemu/xattr.h"
+#include "9p-util.h"
+
+ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size)
+{
+int ret;
+int fd = openat_file(dirfd, filename,
+ O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = fgetxattr(fd, name, value, size, 0, 0);
+close_preserve_errno(fd);
+return ret;
+}
+
+ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
+  char *list, size_t size)
+{
+int ret;
+int fd = openat_file(dirfd, filename,
+ O_RDONLY | O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = flistxattr(fd, list, size, 0);
+close_preserve_errno(fd);
+return ret;
+}
+
+ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
+const char *name)
+{
+int ret;
+int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = fremovexattr(fd, name, 0);
+close_preserve_errno(fd);
+return ret;
+}
+
+int fsetxattrat_nofollow(int dirfd, const char *filename, const char *name,
+ void *value, size_t size, int flags)
+{
+int ret;
+int fd = openat_file(dirfd, filename, O_PATH_9P_UTIL | O_NOFOLLOW, 0);
+if (fd == -1) {
+return -1;
+}
+ret = fsetxattr(fd, name, value, size, 0, flags);
+close_preserve_errno(fd);
+return ret;
+}
diff --git a/hw/9pfs/meson.build b/hw/9pfs/meson.build
index 1b28e70040..12443b6ad5 100644
--- a/hw/9pfs/meson.build
+++ b/hw/9pfs/meson.build
@@ -14,6 +14,7 @@ fs_ss.add(files(
   'coxattr.c',
 ))
 fs_ss.add(when: 'CONFIG_LINUX', if_true: files('9p-util-linux.c'))
+fs_ss.add(when: 'CONFIG_DARWIN', if_true: files('9p-util-darwin.c'))
 fs_ss.add(when: 'CONFIG_XEN', if_true: files('xen-9p-backend.c'))
 softmmu_ss.add_all(when: 'CONFIG_FSDEV_9P', if_true: fs_ss)
 
-- 
2.20.1

[PULL 15/19] 9pfs/codir.c: convert Doxygen -> kerneldoc format

[Christian Schoenebeck]

API doc comments in QEMU are supposed to be in kerneldoc format, so
convert API doc comments from Doxygen format to kerneldoc format.

Signed-off-by: Christian Schoenebeck 
Reviewed-by: Greg Kurz 
Message-Id: 

---
 hw/9pfs/codir.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/hw/9pfs/codir.c b/hw/9pfs/codir.c
index f96d8ac4e6..75148bc985 100644
--- a/hw/9pfs/codir.c
+++ b/hw/9pfs/codir.c
@@ -184,14 +184,25 @@ out:
 }
 
 /**
- * @brief Reads multiple directory entries in one rush.
+ * v9fs_co_readdir_many() - Reads multiple directory entries in one rush.
+ *
+ * @pdu: the causing 9p (T_readdir) client request
+ * @fidp: already opened directory where readdir shall be performed on
+ * @entries: output for directory entries (must not be NULL)
+ * @offset: initial position inside the directory the function shall
+ *  seek to before retrieving the directory entries
+ * @maxsize: maximum result message body size (in bytes)
+ * @dostat: whether a stat() should be performed and returned for
+ *  each directory entry
+ * Return: resulting response message body size (in bytes) on success,
+ * negative error code otherwise
  *
  * Retrieves the requested (max. amount of) directory entries from the fs
  * driver. This function must only be called by the main IO thread (top half).
  * Internally this function call will be dispatched to a background IO thread
  * (bottom half) where it is eventually executed by the fs driver.
  *
- * @discussion Acquiring multiple directory entries in one rush from the fs
+ * Acquiring multiple directory entries in one rush from the fs
  * driver, instead of retrieving each directory entry individually, is very
  * beneficial from performance point of view. Because for every fs driver
  * request latency is added, which in practice could lead to overall
@@ -199,20 +210,9 @@ out:
  * directory) if every directory entry was individually requested from fs
  * driver.
  *
- * @note You must @b ALWAYS call @c v9fs_free_dirents(entries) after calling
+ * NOTE: You must ALWAYS call v9fs_free_dirents(entries) after calling
  * v9fs_co_readdir_many(), both on success and on error cases of this
- * function, to avoid memory leaks once @p entries are no longer needed.
- *
- * @param pdu - the causing 9p (T_readdir) client request
- * @param fidp - already opened directory where readdir shall be performed on
- * @param entries - output for directory entries (must not be NULL)
- * @param offset - initial position inside the directory the function shall
- * seek to before retrieving the directory entries
- * @param maxsize - maximum result message body size (in bytes)
- * @param dostat - whether a stat() should be performed and returned for
- * each directory entry
- * @returns resulting response message body size (in bytes) on success,
- *  negative error code otherwise
+ * function, to avoid memory leaks once @entries are no longer needed.
  */
 int coroutine_fn v9fs_co_readdir_many(V9fsPDU *pdu, V9fsFidState *fidp,
   struct V9fsDirEnt **entries,
-- 
2.20.1

Re: [PATCH v3] ui/cocoa: Use the standard about panel

[Peter Maydell]

On Sun, 27 Feb 2022 at 04:22, Akihiko Odaki  wrote:
>
> This provides standard look and feel for the about panel and reduces
> code.
>
> Signed-off-by: Akihiko Odaki 
> ---



Applied to target-arm.next, thanks.

-- PMM

[PULL 01/19] 9p: linux: Fix a couple Linux assumptions

[Christian Schoenebeck]

From: Keno Fischer 

 - Guard Linux only headers.
 - Add qemu/statfs.h header to abstract over the which
   headers are needed for struct statfs
 - Define `ENOATTR` only if not only defined
   (it's defined in system headers on Darwin).

Signed-off-by: Keno Fischer 
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch 

While it might at first appear that fsdev/virtfs-proxy-header.c would
need similar adjustment for darwin as file-op-9p here, a later patch in
this series disables virtfs-proxy-helper for non-Linux. Allowing
virtfs-proxy-helper on darwin could potentially be an additional
optimization later.

[Will Cohen: - Fix headers for Alpine
 - Integrate statfs.h back into file-op-9p.h
 - Remove superfluous header guards from file-opt-9p
 - Add note about virtfs-proxy-helper being disabled
   on non-Linux for this patch series]
Signed-off-by: Will Cohen 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Greg Kurz 
Message-Id: <20220227223522.91937-2-wwco...@gmail.com>
Signed-off-by: Christian Schoenebeck 
---
 fsdev/file-op-9p.h   | 9 -
 hw/9pfs/9p-local.c   | 2 ++
 hw/9pfs/9p.c | 4 
 include/qemu/xattr.h | 4 +++-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/fsdev/file-op-9p.h b/fsdev/file-op-9p.h
index 8fd89f0447..4997677460 100644
--- a/fsdev/file-op-9p.h
+++ b/fsdev/file-op-9p.h
@@ -16,10 +16,17 @@
 
 #include 
 #include 
-#include 
 #include "qemu-fsdev-throttle.h"
 #include "p9array.h"
 
+#ifdef CONFIG_LINUX
+# include 
+#endif
+#ifdef CONFIG_DARWIN
+# include 
+# include 
+#endif
+
 #define SM_LOCAL_MODE_BITS0600
 #define SM_LOCAL_DIR_MODE_BITS0700
 
diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 210d9e7705..1a5e3eed73 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -32,10 +32,12 @@
 #include "qemu/error-report.h"
 #include "qemu/option.h"
 #include 
+#ifdef CONFIG_LINUX
 #include 
 #ifdef CONFIG_LINUX_MAGIC_H
 #include 
 #endif
+#endif
 #include 
 
 #ifndef XFS_SUPER_MAGIC
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 15b3f4d385..9c63e14b28 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -32,7 +32,11 @@
 #include "migration/blocker.h"
 #include "qemu/xxhash.h"
 #include 
+#ifdef CONFIG_LINUX
 #include 
+#else
+#include 
+#endif
 
 int open_fd_hw;
 int total_open_fd;
diff --git a/include/qemu/xattr.h b/include/qemu/xattr.h
index a83fe8e749..f1d0f7be74 100644
--- a/include/qemu/xattr.h
+++ b/include/qemu/xattr.h
@@ -22,7 +22,9 @@
 #ifdef CONFIG_LIBATTR
 #  include 
 #else
-#  define ENOATTR ENODATA
+#  if !defined(ENOATTR)
+#define ENOATTR ENODATA
+#  endif
 #  include 
 #endif
 
-- 
2.20.1

[PULL 17/19] 9pfs/9p-util.h: convert Doxygen -> kerneldoc format

[Christian Schoenebeck]

API doc comments in QEMU are supposed to be in kerneldoc format, so
convert API doc comments from Doxygen format to kerneldoc format.

Signed-off-by: Christian Schoenebeck 
Reviewed-by: Greg Kurz 
Message-Id: 

---
 hw/9pfs/9p-util.h | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 22835c5f61..cfa7af43c5 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -97,7 +97,7 @@ ssize_t flistxattrat_nofollow(int dirfd, const char *filename,
 ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
 const char *name);
 
-/**
+/*
  * Darwin has d_seekoff, which appears to function similarly to d_off.
  * However, it does not appear to be supported on all file systems,
  * so ensure it is manually injected earlier and call here when
@@ -113,15 +113,15 @@ static inline off_t qemu_dirent_off(struct dirent *dent)
 }
 
 /**
- * Duplicate directory entry @dent.
+ * qemu_dirent_dup() - Duplicate directory entry @dent.
+ *
+ * @dent: original directory entry to be duplicated
+ * Return: duplicated directory entry which should be freed with g_free()
  *
  * It is highly recommended to use this function instead of open coding
  * duplication of dirent objects, because the actual struct dirent
  * size may be bigger or shorter than sizeof(struct dirent) and correct
  * handling is platform specific (see gitlab issue #841).
- *
- * @dent - original directory entry to be duplicated
- * @returns duplicated directory entry which should be freed with g_free()
  */
 static inline struct dirent *qemu_dirent_dup(struct dirent *dent)
 {
-- 
2.20.1

[PULL 19/19] fsdev/p9array.h: convert Doxygen -> kerneldoc format

[Christian Schoenebeck]

API doc comments in QEMU are supposed to be in kerneldoc format, so
convert API doc comments from Doxygen format to kerneldoc format.

Signed-off-by: Christian Schoenebeck 
Reviewed-by: Greg Kurz 
Message-Id: 
<2e2d46a402560f155de322d95789ba107d728885.1646314856.git.qemu_...@crudebyte.com>
---
 fsdev/p9array.h | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/fsdev/p9array.h b/fsdev/p9array.h
index 6aa25327ca..90e83a7c7b 100644
--- a/fsdev/p9array.h
+++ b/fsdev/p9array.h
@@ -81,11 +81,11 @@
  */
 
 /**
- * Declares an array type for the passed @a scalar_type.
+ * P9ARRAY_DECLARE_TYPE() - Declares an array type for the passed @scalar_type.
  *
- * This is typically used from a shared header file.
+ * @scalar_type: type of the individual array elements
  *
- * @param scalar_type - type of the individual array elements
+ * This is typically used from a shared header file.
  */
 #define P9ARRAY_DECLARE_TYPE(scalar_type) \
 typedef struct P9Array##scalar_type { \
@@ -97,14 +97,14 @@
 void p9array_auto_free_##scalar_type(scalar_type **auto_var); \
 
 /**
- * Defines an array type for the passed @a scalar_type and appropriate
- * @a scalar_cleanup_func.
+ * P9ARRAY_DEFINE_TYPE() - Defines an array type for the passed @scalar_type
+ * and appropriate @scalar_cleanup_func.
  *
- * This is typically used from a C unit file.
+ * @scalar_type: type of the individual array elements
+ * @scalar_cleanup_func: appropriate function to free memory dynamically
+ *   allocated by individual array elements before
  *
- * @param scalar_type - type of the individual array elements
- * @param scalar_cleanup_func - appropriate function to free memory dynamically
- *  allocated by individual array elements before
+ * This is typically used from a C unit file.
  */
 #define P9ARRAY_DEFINE_TYPE(scalar_type, scalar_cleanup_func) \
 void p9array_new_##scalar_type(scalar_type **auto_var, size_t len) \
@@ -132,23 +132,27 @@
 } \
 
 /**
+ * P9ARRAY_REF() - Declare a reference variable for an array.
+ *
+ * @scalar_type: type of the individual array elements
+ *
  * Used to declare a reference variable (unique pointer) for an array. After
  * leaving the scope of the reference variable, the associated array is
  * automatically freed.
- *
- * @param scalar_type - type of the individual array elements
  */
 #define P9ARRAY_REF(scalar_type) \
 __attribute((__cleanup__(p9array_auto_free_##scalar_type))) scalar_type*
 
 /**
- * Allocates a new array of passed @a scalar_type with @a len number of array
- * elements and assigns the created array to the reference variable
- * @a auto_var.
+ * P9ARRAY_NEW() - Allocate a new array.
  *
- * @param scalar_type - type of the individual array elements
- * @param auto_var - destination reference variable
- * @param len - amount of array elements to be allocated immediately
+ * @scalar_type: type of the individual array elements
+ * @auto_var: destination reference variable
+ * @len: amount of array elements to be allocated immediately
+ *
+ * Allocates a new array of passed @scalar_type with @len number of array
+ * elements and assigns the created array to the reference variable
+ * @auto_var.
  */
 #define P9ARRAY_NEW(scalar_type, auto_var, len) \
 QEMU_BUILD_BUG_MSG( \
-- 
2.20.1

[PULL 16/19] 9pfs/9p.c: convert Doxygen -> kerneldoc format

[Christian Schoenebeck]

API doc comments in QEMU are supposed to be in kerneldoc format, so
convert API doc comments from Doxygen format to kerneldoc format.

Signed-off-by: Christian Schoenebeck 
Reviewed-by: Greg Kurz 
Message-Id: 
<4ece6ffa4465c271c6a7c42a3040f42780fcce87.1646314856.git.qemu_...@crudebyte.com>
---
 hw/9pfs/9p.c | 62 +---
 1 file changed, 35 insertions(+), 27 deletions(-)

diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 7405352c37..a6d6b3f835 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -628,8 +628,8 @@ static inline uint64_t mirror64bit(uint64_t value)
((uint64_t)mirror8bit((value >> 56) & 0xff));
 }
 
-/**
- * @brief Parameter k for the Exponential Golomb algorihm to be used.
+/*
+ * Parameter k for the Exponential Golomb algorihm to be used.
  *
  * The smaller this value, the smaller the minimum bit count for the Exp.
  * Golomb generated affixes will be (at lowest index) however for the
@@ -642,28 +642,30 @@ static inline uint64_t mirror64bit(uint64_t value)
  * should be small, for a large amount of devices k might be increased
  * instead. The default of k=0 should be fine for most users though.
  *
- * @b IMPORTANT: In case this ever becomes a runtime parameter; the value of
+ * IMPORTANT: In case this ever becomes a runtime parameter; the value of
  * k should not change as long as guest is still running! Because that would
  * cause completely different inode numbers to be generated on guest.
  */
 #define EXP_GOLOMB_K0
 
 /**
- * @brief Exponential Golomb algorithm for arbitrary k (including k=0).
+ * expGolombEncode() - Exponential Golomb algorithm for arbitrary k
+ * (including k=0).
  *
- * The Exponential Golomb algorithm generates @b prefixes (@b not suffixes!)
+ * @n: natural number (or index) of the prefix to be generated
+ * (1, 2, 3, ...)
+ * @k: parameter k of Exp. Golomb algorithm to be used
+ * (see comment on EXP_GOLOMB_K macro for details about k)
+ * Return: prefix for given @n and @k
+ *
+ * The Exponential Golomb algorithm generates prefixes (NOT suffixes!)
  * with growing length and with the mathematical property of being
  * "prefix-free". The latter means the generated prefixes can be prepended
  * in front of arbitrary numbers and the resulting concatenated numbers are
  * guaranteed to be always unique.
  *
  * This is a minor adjustment to the original Exp. Golomb algorithm in the
- * sense that lowest allowed index (@param n) starts with 1, not with zero.
- *
- * @param n - natural number (or index) of the prefix to be generated
- *(1, 2, 3, ...)
- * @param k - parameter k of Exp. Golomb algorithm to be used
- *(see comment on EXP_GOLOMB_K macro for details about k)
+ * sense that lowest allowed index (@n) starts with 1, not with zero.
  */
 static VariLenAffix expGolombEncode(uint64_t n, int k)
 {
@@ -677,7 +679,9 @@ static VariLenAffix expGolombEncode(uint64_t n, int k)
 }
 
 /**
- * @brief Converts a suffix into a prefix, or a prefix into a suffix.
+ * invertAffix() - Converts a suffix into a prefix, or a prefix into a suffix.
+ * @affix: either suffix or prefix to be inverted
+ * Return: inversion of passed @affix
  *
  * Simply mirror all bits of the affix value, for the purpose to preserve
  * respectively the mathematical "prefix-free" or "suffix-free" property
@@ -701,16 +705,16 @@ static VariLenAffix invertAffix(const VariLenAffix *affix)
 }
 
 /**
- * @brief Generates suffix numbers with "suffix-free" property.
+ * affixForIndex() - Generates suffix numbers with "suffix-free" property.
+ * @index: natural number (or index) of the suffix to be generated
+ * (1, 2, 3, ...)
+ * Return: Suffix suitable to assemble unique number.
  *
  * This is just a wrapper function on top of the Exp. Golomb algorithm.
  *
  * Since the Exp. Golomb algorithm generates prefixes, but we need suffixes,
  * this function converts the Exp. Golomb prefixes into appropriate suffixes
  * which are still suitable for generating unique numbers.
- *
- * @param n - natural number (or index) of the suffix to be generated
- *(1, 2, 3, ...)
  */
 static VariLenAffix affixForIndex(uint64_t index)
 {
@@ -810,8 +814,8 @@ static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t 
dev)
 return val->prefix_bits;
 }
 
-/**
- * @brief Slow / full mapping host inode nr -> guest inode nr.
+/*
+ * Slow / full mapping host inode nr -> guest inode nr.
  *
  * This function performs a slower and much more costly remapping of an
  * original file inode number on host to an appropriate different inode
@@ -823,7 +827,7 @@ static int qid_inode_prefix_hash_bits(V9fsPDU *pdu, dev_t 
dev)
  * qid_path_suffixmap() failed. In practice this slow / full mapping is not
  * expected ever to be used at all though.
  *
- * @see qid_path_suffixmap() for details
+ * See qid_path_suffixmap() for details
  *
  */
 static int qid_path_fullmap(V9fsPDU *pdu, const struct stat *stbuf,
@@ -864,8 +

[PULL 11/19] 9p: darwin: meson: Allow VirtFS on Darwin

[Christian Schoenebeck]

From: Keno Fischer 

To allow VirtFS on darwin, we need to check that pthread_fchdir_np is
available, which has only been available since macOS 10.12.

Additionally, virtfs_proxy_helper is disabled on Darwin. This patch
series does not currently provide an implementation of the proxy-helper,
but this functionality could be implemented later on.

Signed-off-by: Keno Fischer 
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch 
[Will Cohen: - Rebase to master]
Signed-off-by: Will Cohen 
Reviewed-by: Paolo Bonzini 
[Will Cohen: - Add check for pthread_fchdir_np to virtfs
 - Add comments to patch commit
 - Note that virtfs_proxy_helper does not work
   on macOS
 - Fully adjust meson virtfs error note to specify
   macOS
 - Rebase to master]
Signed-off-by: Will Cohen 
Message-Id: <20220227223522.91937-12-wwco...@gmail.com>
Signed-off-by: Christian Schoenebeck 
Acked-by: Christian Schoenebeck 
---
 fsdev/meson.build |  1 +
 meson.build   | 12 +++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/fsdev/meson.build b/fsdev/meson.build
index adf57cc43e..b632b66348 100644
--- a/fsdev/meson.build
+++ b/fsdev/meson.build
@@ -7,6 +7,7 @@ fsdev_ss.add(when: ['CONFIG_FSDEV_9P'], if_true: files(
   'qemu-fsdev.c',
 ), if_false: files('qemu-fsdev-dummy.c'))
 softmmu_ss.add_all(when: 'CONFIG_LINUX', if_true: fsdev_ss)
+softmmu_ss.add_all(when: 'CONFIG_DARWIN', if_true: fsdev_ss)
 
 if have_virtfs_proxy_helper
   executable('virtfs-proxy-helper',
diff --git a/meson.build b/meson.build
index 6058ea3896..32d91690ee 100644
--- a/meson.build
+++ b/meson.build
@@ -1462,14 +1462,16 @@ dbus_display = get_option('dbus_display') \
   .allowed()
 
 have_virtfs = get_option('virtfs') \
-.require(targetos == 'linux',
- error_message: 'virtio-9p (virtfs) requires Linux') \
-.require(libattr.found() and libcap_ng.found(),
- error_message: 'virtio-9p (virtfs) requires libcap-ng-devel and 
libattr-devel') \
+.require(targetos == 'linux' or targetos == 'darwin',
+ error_message: 'virtio-9p (virtfs) requires Linux or macOS') \
+.require(targetos == 'linux' or cc.has_function('pthread_fchdir_np'),
+ error_message: 'virtio-9p (virtfs) on macOS requires the presence 
of pthread_fchdir_np') \
+.require(targetos == 'darwin' or (libattr.found() and libcap_ng.found()),
+ error_message: 'virtio-9p (virtfs) on Linux requires 
libcap-ng-devel and libattr-devel') \
 .disable_auto_if(not have_tools and not have_system) \
 .allowed()
 
-have_virtfs_proxy_helper = have_virtfs and have_tools
+have_virtfs_proxy_helper = targetos != 'darwin' and have_virtfs and have_tools
 
 foreach k : get_option('trace_backends')
   config_host_data.set('CONFIG_TRACE_' + k.to_upper(), true)
-- 
2.20.1

Re: [PATCH v5 3/3] tests/acpi: i386: update FACP table differences

[Michael S. Tsirkin]

On Fri, Mar 04, 2022 at 05:22:57PM +0530, Ani Sinha wrote:
> From: Liav Albani 
> 
> After changing the IAPC boot flags register to indicate support of i8042
> in the machine chipset to help the guest OS to determine its existence
> "faster", we need to have the updated FACP ACPI binary images in tree.
> 
> The ASL changes introduced are shown by the following diff:
> 
> @@ -42,35 +42,35 @@
>  [059h 0089   1] PM1 Control Block Length : 02
>  [05Ah 0090   1] PM2 Control Block Length : 00
>  [05Bh 0091   1]PM Timer Block Length : 04
>  [05Ch 0092   1]GPE0 Block Length : 10
>  [05Dh 0093   1]GPE1 Block Length : 00
>  [05Eh 0094   1] GPE1 Base Offset : 00
>  [05Fh 0095   1] _CST Support : 00
>  [060h 0096   2]   C2 Latency : 0FFF
>  [062h 0098   2]   C3 Latency : 0FFF
>  [064h 0100   2]   CPU Cache Size : 
>  [066h 0102   2]   Cache Flush Stride : 
>  [068h 0104   1]Duty Cycle Offset : 00
>  [069h 0105   1] Duty Cycle Width : 00
>  [06Ah 0106   1]  RTC Day Alarm Index : 00
>  [06Bh 0107   1]RTC Month Alarm Index : 00
>  [06Ch 0108   1]RTC Century Index : 32
> -[06Dh 0109   2]   Boot Flags (decoded below) : 
> +[06Dh 0109   2]   Boot Flags (decoded below) : 0002
> Legacy Devices Supported (V2) : 0
> -8042 Present on ports 60/64 (V2) : 0
> +8042 Present on ports 60/64 (V2) : 1
>  VGA Not Present (V4) : 0
>MSI Not Supported (V4) : 0
>  PCIe ASPM Not Supported (V4) : 0
> CMOS RTC Not Present (V5) : 0
>  [06Fh 0111   1] Reserved : 00
>  [070h 0112   4]Flags (decoded below) : 84A5
>WBINVD instruction is operational (V1) : 1
>WBINVD flushes all caches (V1) : 0
>  All CPUs support C1 (V1) : 1
>C2 works on MP system (V1) : 0
>  Control Method Power Button (V1) : 0
>  Control Method Sleep Button (V1) : 1
>  RTC wake not in fixed reg space (V1) : 0
>  RTC can wake system from S4 (V1) : 1
>  32-bit PM Timer (V1) : 0
>Docking Supported (V1) : 0
> 
> Signed-off-by: Liav Albani 
> Acked-by: Ani Sinha 

Fails make check:
▶  3/60 ERROR:../tests/qtest/bios-tables-test.c:532:test_acpi_asl: assertion 
failed: (all_tables_match) ERROR 


> ---
>  tests/data/acpi/q35/FACP| Bin 244 -> 244 bytes
>  tests/data/acpi/q35/FACP.nosmm  | Bin 244 -> 244 bytes
>  tests/data/acpi/q35/FACP.slic   | Bin 244 -> 244 bytes
>  tests/data/acpi/q35/FACP.xapic  | Bin 244 -> 244 bytes
>  tests/qtest/bios-tables-test-allowed-diff.h |   4 
>  5 files changed, 4 deletions(-)
> 
> diff --git a/tests/data/acpi/q35/FACP b/tests/data/acpi/q35/FACP
> index 
> f6a864cc863c7763f6c09d3814ad184a658fa0a0..a8f6a8961109d01059aceef9f1869cde09a2f10c
>  100644
> GIT binary patch
> delta 23
> ecmeyu_=S 
> delta 23
> ecmeyu_=S 
> diff --git a/tests/data/acpi/q35/FACP.nosmm b/tests/data/acpi/q35/FACP.nosmm
> index 
> 6a9aa5f370eb9af6a03dc739d8a159be58fdee01..c4e6d18ee5fc64159160d4589aa96b4d648c913a
>  100644
> GIT binary patch
> delta 23
> ecmeyu_=S 
> delta 23
> ecmeyu_=S 
> diff --git a/tests/data/acpi/q35/FACP.slic b/tests/data/acpi/q35/FACP.slic
> index 
> 15986e095cf2db7ee92f7ce113c1d46d54018c62..48bbb1cf5ad0ceda1d2f6d56edf5c1e207bd1a04
>  100644
> GIT binary patch
> delta 23
> ecmeyu_=S 
> delta 23
> ecmeyu_=S 
> diff --git a/tests/data/acpi/q35/FACP.xapic b/tests/data/acpi/q35/FACP.xapic
> index 
> 2d3659c9c6753d07c3d48742343cb8e8cc034de7..31fa5dd19c213034eef4eeefa6a04e61dadd8a2a
>  100644
> GIT binary patch
> delta 23
> ecmeyu_=S 
> delta 23
> ecmeyu_=S 
> diff --git a/tests/qtest/bios-tables-test-allowed-diff.h 
> b/tests/qtest/bios-tables-test-allowed-diff.h
> index 7570e39369..dfb8523c8b 100644
> --- a/tests/qtest/bios-tables-test-allowed-diff.h
> +++ b/tests/qtest/bios-tables-test-allowed-diff.h
> @@ -1,5 +1 @@
>  /* List of comma-separated changed AML files to ignore */
> -"tests/data/acpi/q35/FACP",
> -"tests/data/acpi/q35/FACP.nosmm",
> -"tests/data/acpi/q35/FACP.slic",
> -"tests/data/acpi/q35/FACP.xapic",
> -- 
> 2.25.1

Re: [PATCH v6 20/43] hw/cxl/device: Add some trivial commands

[Jonathan Cameron via]

On Tue, 01 Mar 2022 18:46:30 +
Alex Bennée  wrote:

> Jonathan Cameron  writes:
> 
> > From: Ben Widawsky 
> >
> > GET_FW_INFO and GET_PARTITION_INFO, for this emulation, is equivalent to
> > info already returned in the IDENTIFY command. To have a more robust
> > implementation, add those.
> >
> > Signed-off-by: Ben Widawsky 
> > Signed-off-by: Jonathan Cameron 

Hi Alex,

> > ---
> >  hw/cxl/cxl-mailbox-utils.c | 69 +-
> >  1 file changed, 68 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> > index 808faec114..d022711b2a 100644
> > --- a/hw/cxl/cxl-mailbox-utils.c
> > +++ b/hw/cxl/cxl-mailbox-utils.c
> > @@ -44,6 +44,8 @@ enum {
> >  #define CLEAR_RECORDS   0x1
> >  #define GET_INTERRUPT_POLICY   0x2
> >  #define SET_INTERRUPT_POLICY   0x3
> > +FIRMWARE_UPDATE = 0x02,
> > +#define GET_INFO  0x0
> >  TIMESTAMP   = 0x03,
> >  #define GET   0x0
> >  #define SET   0x1
> > @@ -52,6 +54,8 @@ enum {
> >  #define GET_LOG   0x1
> >  IDENTIFY= 0x40,
> >  #define MEMORY_DEVICE 0x0
> > +CCLS= 0x41,
> > +#define GET_PARTITION_INFO 0x0
> >  };
> >  
> >  /* 8.2.8.4.5.1 Command Return Codes */
> > @@ -114,6 +118,39 @@ DEFINE_MAILBOX_HANDLER_NOP(events_clear_records);
> >  DEFINE_MAILBOX_HANDLER_ZEROED(events_get_interrupt_policy, 4);
> >  DEFINE_MAILBOX_HANDLER_NOP(events_set_interrupt_policy);
> >  
> > +/* 8.2.9.2.1 */
> > +static ret_code cmd_firmware_update_get_info(struct cxl_cmd *cmd,
> > + CXLDeviceState *cxl_dstate,
> > + uint16_t *len)
> > +{
> > +struct {
> > +uint8_t slots_supported;
> > +uint8_t slot_info;
> > +uint8_t caps;
> > +uint8_t rsvd[0xd];
> > +char fw_rev1[0x10];
> > +char fw_rev2[0x10];
> > +char fw_rev3[0x10];
> > +char fw_rev4[0x10];
> > +} __attribute__((packed)) *fw_info;
> > +_Static_assert(sizeof(*fw_info) == 0x50, "Bad firmware info
> > size");  
> 
> note: we have QEMU_PACKED, QEMU_BUILD_BUG_ON and friends in compiler.h which 
> are
> preferred for potential compiler portability reasons.

Replaced all instances of alignment, packed and Static_assert in the
patch set with the compiler.h equivalents. Not that has minor
affect on some earlier patch sets but feels trivial enough
that I've kept RBs etc.

> 
> > +
> > +if (cxl_dstate->pmem_size < (256 << 20)) {
> > +return CXL_MBOX_INTERNAL_ERROR;
> > +}
> > +
> > +fw_info = (void *)cmd->payload;
> > +memset(fw_info, 0, sizeof(*fw_info));
> > +
> > +fw_info->slots_supported = 2;
> > +fw_info->slot_info = BIT(0) | BIT(3);
> > +fw_info->caps = 0;
> > +snprintf(fw_info->fw_rev1, 0x10, "BWFW VERSION %02d", 0);  
> 
> Given you have a fixed string here could you not:
> 
>   pstrcpy(fw_info->fw_rev1, 0x10, "BWFW VERSION 0");

Make sense.

>   
> > +
> > +*len = sizeof(*fw_info);
> > +return CXL_MBOX_SUCCESS;
> > +}
> > +
> >  /* 8.2.9.3.1 */
> >  static ret_code cmd_timestamp_get(struct cxl_cmd *cmd,
> >CXLDeviceState *cxl_dstate,
> > @@ -260,6 +297,33 @@ static ret_code cmd_identify_memory_device(struct 
> > cxl_cmd *cmd,
> >  return CXL_MBOX_SUCCESS;
> >  }
> >  
> > +static ret_code cmd_ccls_get_partition_info(struct cxl_cmd *cmd,
> > +   CXLDeviceState *cxl_dstate,
> > +   uint16_t *len)
> > +{
> > +struct {
> > +uint64_t active_vmem;
> > +uint64_t active_pmem;
> > +uint64_t next_vmem;
> > +uint64_t next_pmem;
> > +} __attribute__((packed)) *part_info = (void *)cmd->payload;
> > +_Static_assert(sizeof(*part_info) == 0x20, "Bad get partition info 
> > size");
> > +uint64_t size = cxl_dstate->pmem_size;
> > +
> > +if (!QEMU_IS_ALIGNED(size, 256 << 20)) {
> > +return CXL_MBOX_INTERNAL_ERROR;
> > +}
> > +
> > +/* PMEM only */
> > +part_info->active_vmem = 0;
> > +part_info->next_vmem = 0;
> > +part_info->active_pmem = size / (256 << 20);
> > +part_info->next_pmem = part_info->active_pmem;
> > +
> > +*len = sizeof(*part_info);
> > +return CXL_MBOX_SUCCESS;
> > +}
> > +
> >  #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
> >  #define IMMEDIATE_POLICY_CHANGE (1 << 3)
> >  #define IMMEDIATE_LOG_CHANGE (1 << 4)
> > @@ -273,15 +337,18 @@ static struct cxl_cmd cxl_cmd_set[256][256] = {
> >  cmd_events_get_interrupt_policy, 0, 0 },
> >  [EVENTS][SET_INTERRUPT_POLICY] = { "EVENTS_SET_INTERRUPT_POLICY",
> >  cmd_events_set_interrupt_policy, 4, IMMEDIATE_CONFIG_CHANGE },
> > +[FIRMWARE_UPDATE][GET_INFO] = { "FIRMWARE_UPDATE_GET_INFO",
> > +cmd_firmware_update_get_info, 0, 0 },
> >  [TIMESTAMP]

Re: [PULL 0/7] target/nios2: Rewrite interrupt handling

[Peter Maydell]

On Thu, 3 Mar 2022 at 20:46, Richard Henderson
 wrote:
>
> The following changes since commit 36eae3a732a1f2aa81391e871ac0e9bb3233e7d7:
>
>   Merge remote-tracking branch 
> 'remotes/dgilbert-gitlab/tags/pull-migration-20220302b' into staging 
> (2022-03-02 20:55:48 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/rth7680/qemu.git tags/pull-nios-20220303
>
> for you to fetch changes up to b72c9d5951f1dfa047f545408dd9e35597e6b9d3:
>
>   target/nios2: Rewrite interrupt handling (2022-03-03 09:51:59 -1000)
>
> 
> Rewrite nios2 interrupt handling
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0
for any user-visible changes.

-- PMM

[PULL 08/19] 9p: darwin: Compatibility for f/l*xattr

[Christian Schoenebeck]

From: Keno Fischer 

On darwin `fgetxattr` takes two extra optional arguments,
and the l* variants are not defined (in favor of an extra
flag to the regular variants.

Signed-off-by: Keno Fischer 
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch 
Signed-off-by: Will Cohen 
Message-Id: <20220227223522.91937-9-wwco...@gmail.com>
Signed-off-by: Christian Schoenebeck 
Acked-by: Christian Schoenebeck 
---
 hw/9pfs/9p-local.c | 12 
 hw/9pfs/9p-util.h  | 17 +
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index f3272f0b43..a0d08e5216 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -790,16 +790,20 @@ static int local_fstat(FsContext *fs_ctx, int fid_type,
 mode_t tmp_mode;
 dev_t tmp_dev;
 
-if (fgetxattr(fd, "user.virtfs.uid", &tmp_uid, sizeof(uid_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.uid",
+   &tmp_uid, sizeof(uid_t)) > 0) {
 stbuf->st_uid = le32_to_cpu(tmp_uid);
 }
-if (fgetxattr(fd, "user.virtfs.gid", &tmp_gid, sizeof(gid_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.gid",
+   &tmp_gid, sizeof(gid_t)) > 0) {
 stbuf->st_gid = le32_to_cpu(tmp_gid);
 }
-if (fgetxattr(fd, "user.virtfs.mode", &tmp_mode, sizeof(mode_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.mode",
+   &tmp_mode, sizeof(mode_t)) > 0) {
 stbuf->st_mode = le32_to_cpu(tmp_mode);
 }
-if (fgetxattr(fd, "user.virtfs.rdev", &tmp_dev, sizeof(dev_t)) > 0) {
+if (qemu_fgetxattr(fd, "user.virtfs.rdev",
+   &tmp_dev, sizeof(dev_t)) > 0) {
 stbuf->st_rdev = le64_to_cpu(tmp_dev);
 }
 } else if (fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 2b9ac74291..9abff79884 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -19,6 +19,23 @@
 #define O_PATH_9P_UTIL 0
 #endif
 
+#ifdef CONFIG_DARWIN
+#define qemu_fgetxattr(...) fgetxattr(__VA_ARGS__, 0, 0)
+#define qemu_lgetxattr(...) getxattr(__VA_ARGS__, 0, XATTR_NOFOLLOW)
+#define qemu_llistxattr(...) listxattr(__VA_ARGS__, XATTR_NOFOLLOW)
+#define qemu_lremovexattr(...) removexattr(__VA_ARGS__, XATTR_NOFOLLOW)
+static inline int qemu_lsetxattr(const char *path, const char *name,
+ const void *value, size_t size, int flags) {
+return setxattr(path, name, value, size, 0, flags | XATTR_NOFOLLOW);
+}
+#else
+#define qemu_fgetxattr fgetxattr
+#define qemu_lgetxattr lgetxattr
+#define qemu_llistxattr llistxattr
+#define qemu_lremovexattr lremovexattr
+#define qemu_lsetxattr lsetxattr
+#endif
+
 static inline void close_preserve_errno(int fd)
 {
 int serrno = errno;
-- 
2.20.1

[PULL 18/19] 9pfs/coth.h: drop Doxygen format on v9fs_co_run_in_worker()

[Christian Schoenebeck]

API doc comments in QEMU are supposed to be in kerneldoc format, so
drop Doxygen format used on v9fs_co_run_in_worker() macro.

Signed-off-by: Christian Schoenebeck 
Reviewed-by: Greg Kurz 
Message-Id: 

---
 hw/9pfs/coth.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/coth.h b/hw/9pfs/coth.h
index f83c7dda7b..1a1edbdc2a 100644
--- a/hw/9pfs/coth.h
+++ b/hw/9pfs/coth.h
@@ -19,7 +19,7 @@
 #include "qemu/coroutine.h"
 #include "9p.h"
 
-/**
+/*
  * we want to use bottom half because we want to make sure the below
  * sequence of events.
  *
@@ -29,7 +29,7 @@
  * we cannot swap step 1 and 2, because that would imply worker thread
  * can enter coroutine while step1 is still running
  *
- * @b PERFORMANCE @b CONSIDERATIONS: As a rule of thumb, keep in mind
+ * PERFORMANCE CONSIDERATIONS: As a rule of thumb, keep in mind
  * that hopping between threads adds @b latency! So when handling a
  * 9pfs request, avoid calling v9fs_co_run_in_worker() too often, because
  * this might otherwise sum up to a significant, huge overall latency for
-- 
2.20.1

[PATCH] hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present

[Ani Sinha]

The second bit of IAPC_BOOT_ARCH in FADT table indicates the presence of
keyboard controller implemented as 8042 or equivalent micro controller. This
change enables this flag for microvms if such a device exists (for example,
when added explicitly from the QEMU commandline). Change
1f810294bb31bf6ac ("hw/acpi: add indication for i8042 in IA-PC boot flags of 
the FADT table")
enabled this flag for i386 q35 based machines. The reason for doing the same
for micro-vms is to make sure we provide the correct tables to the guest OS
uniformly in all cases when an i8042 device is present. When this bit is not
enabled, guest OSes has to find other indirect methods to detect the device
which we would like to avoid.

Signed-off-by: Ani Sinha 
---
 hw/i386/acpi-microvm.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
index 68ca7e7fc2..12452cb2e5 100644
--- a/hw/i386/acpi-microvm.c
+++ b/hw/i386/acpi-microvm.c
@@ -37,6 +37,7 @@
 #include "hw/pci/pcie_host.h"
 #include "hw/usb/xhci.h"
 #include "hw/virtio/virtio-mmio.h"
+#include "hw/input/i8042.h"
 
 #include "acpi-common.h"
 #include "acpi-microvm.h"
@@ -187,6 +188,11 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
 .address = GED_MMIO_BASE_REGS + ACPI_GED_REG_RESET,
 },
 .reset_val = ACPI_GED_RESET_VALUE,
+/*
+ * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot Architecture
+ * Flags, bit offset 1 - 8042.
+ */
+.iapc_boot_arch = i8042_present() ? 0x1 << 1 : 0x0,
 };
 
 table_offsets = g_array_new(false, true /* clear */,
-- 
2.25.1

Re: [PATCH] hw/acpi/microvm: turn on 8042 bit in FADT boot architecture flags if present

[Michael S. Tsirkin]

On Fri, Mar 04, 2022 at 06:47:41PM +0530, Ani Sinha wrote:
> The second bit of IAPC_BOOT_ARCH in FADT table indicates the presence of
> keyboard controller implemented as 8042 or equivalent micro controller. This
> change enables this flag for microvms if such a device exists (for example,
> when added explicitly from the QEMU commandline). Change
> 1f810294bb31bf6ac ("hw/acpi: add indication for i8042 in IA-PC boot flags of 
> the FADT table")
> enabled this flag for i386 q35 based machines. The reason for doing the same
> for micro-vms is to make sure we provide the correct tables to the guest OS
> uniformly in all cases when an i8042 device is present. When this bit is not
> enabled, guest OSes has to find other indirect methods to detect the device
> which we would like to avoid.
> 
> Signed-off-by: Ani Sinha 
> ---
>  hw/i386/acpi-microvm.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index 68ca7e7fc2..12452cb2e5 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -37,6 +37,7 @@
>  #include "hw/pci/pcie_host.h"
>  #include "hw/usb/xhci.h"
>  #include "hw/virtio/virtio-mmio.h"
> +#include "hw/input/i8042.h"
>  
>  #include "acpi-common.h"
>  #include "acpi-microvm.h"
> @@ -187,6 +188,11 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
>  .address = GED_MMIO_BASE_REGS + ACPI_GED_REG_RESET,
>  },
>  .reset_val = ACPI_GED_RESET_VALUE,
> +/*
> + * ACPI v2, Table 5-10 - Fixed ACPI Description Table Boot 
> Architecture
> + * Flags, bit offset 1 - 8042.
> + */
> +.iapc_boot_arch = i8042_present() ? 0x1 << 1 : 0x0,


Please, move this logic to a function, do not duplicate it.

>  };
>  
>  table_offsets = g_array_new(false, true /* clear */,
> -- 
> 2.25.1

[PULL 04/19] 9p: darwin: Handle struct dirent differences

[Christian Schoenebeck]

From: Keno Fischer 

On darwin d_seekoff exists, but is optional and does not seem to
be commonly used by file systems. Use `telldir` instead to obtain
the seek offset and inject it into d_seekoff, and create a
qemu_dirent_off helper to call it appropriately when appropriate.

Signed-off-by: Keno Fischer 
[Michael Roitzsch: - Rebase for NixOS]
Signed-off-by: Michael Roitzsch 
[Will Cohen: - Adjust to pass testing
 - Ensure that d_seekoff is filled using telldir
   on darwin, and create qemu_dirent_off helper
   to decide which to access]
[Fabian Franz: - Add telldir error handling for darwin]
Signed-off-by: Fabian Franz 
[Will Cohen: - Ensure that telldir error handling uses
   signed int
 - Cleanup of telldir error handling
 - Remove superfluous error handling for
   qemu_dirent_off
 - Adjust formatting
 - Use qemu_dirent_off in codir.c
 - Declare qemu_dirent_off as static to prevent
   linker error
 - Move qemu_dirent_off above the end-of-file
   endif to fix compilation]
Signed-off-by: Will Cohen 
Message-Id: <20220227223522.91937-5-wwco...@gmail.com>
Signed-off-by: Christian Schoenebeck 
Reviewed-by: Christian Schoenebeck 
---
 hw/9pfs/9p-local.c |  9 +
 hw/9pfs/9p-proxy.c | 16 +++-
 hw/9pfs/9p-synth.c |  4 
 hw/9pfs/9p-util.h  | 16 
 hw/9pfs/9p.c   |  7 +--
 hw/9pfs/codir.c|  4 +++-
 6 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 1a5e3eed73..f3272f0b43 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -562,6 +562,15 @@ again:
 if (!entry) {
 return NULL;
 }
+#ifdef CONFIG_DARWIN
+int off;
+off = telldir(fs->dir.stream);
+/* If telldir fails, fail the entire readdir call */
+if (off < 0) {
+return NULL;
+}
+entry->d_seekoff = off;
+#endif
 
 if (ctx->export_flags & V9FS_SM_MAPPED) {
 entry->d_type = DT_UNKNOWN;
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index b1664080d8..8b4b5cf7dc 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -706,7 +706,21 @@ static off_t proxy_telldir(FsContext *ctx, 
V9fsFidOpenState *fs)
 
 static struct dirent *proxy_readdir(FsContext *ctx, V9fsFidOpenState *fs)
 {
-return readdir(fs->dir.stream);
+struct dirent *entry;
+entry = readdir(fs->dir.stream);
+#ifdef CONFIG_DARWIN
+if (!entry) {
+return NULL;
+}
+int td;
+td = telldir(fs->dir.stream);
+/* If telldir fails, fail the entire readdir call */
+if (td < 0) {
+return NULL;
+}
+entry->d_seekoff = td;
+#endif
+return entry;
 }
 
 static void proxy_seekdir(FsContext *ctx, V9fsFidOpenState *fs, off_t off)
diff --git a/hw/9pfs/9p-synth.c b/hw/9pfs/9p-synth.c
index bf9b0c5ddd..b3080e415b 100644
--- a/hw/9pfs/9p-synth.c
+++ b/hw/9pfs/9p-synth.c
@@ -234,7 +234,11 @@ static void synth_direntry(V9fsSynthNode *node,
  offsetof(struct dirent, d_name) + sz);
 memcpy(entry->d_name, node->name, sz);
 entry->d_ino = node->attr->inode;
+#ifdef CONFIG_DARWIN
+entry->d_seekoff = off + 1;
+#else
 entry->d_off = off + 1;
+#endif
 }
 
 static struct dirent *synth_get_dentry(V9fsSynthNode *dir,
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 546f46dc7d..7449733c15 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -78,4 +78,20 @@ ssize_t flistxattrat_nofollow(int dirfd, const char 
*filename,
 ssize_t fremovexattrat_nofollow(int dirfd, const char *filename,
 const char *name);
 
+/**
+ * Darwin has d_seekoff, which appears to function similarly to d_off.
+ * However, it does not appear to be supported on all file systems,
+ * so ensure it is manually injected earlier and call here when
+ * needed.
+ */
+static inline off_t qemu_dirent_off(struct dirent *dent)
+{
+#ifdef CONFIG_DARWIN
+return dent->d_seekoff;
+#else
+return dent->d_off;
+#endif
+}
+
+
 #endif
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 1563d7b7c6..caf3b240fe 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -27,6 +27,7 @@
 #include "virtio-9p.h"
 #include "fsdev/qemu-fsdev.h"
 #include "9p-xattr.h"
+#include "9p-util.h"
 #include "coth.h"
 #include "trace.h"
 #include "migration/blocker.h"
@@ -2281,7 +2282,7 @@ static int coroutine_fn v9fs_do_readdir_with_stat(V9fsPDU 
*pdu,
 count += len;
 v9fs_stat_free(&v9stat);
 v9fs_path_free(&path);
-saved_dir_pos = dent->d_off;
+saved_dir_pos = qemu_dirent_off(dent);
 }
 
 v9fs_readdir_unlock(&fidp->fs.dir);
@@ -2420,6 +2421,7 @@ static int coroutine_fn v9fs_do_readdir(V9fsPDU *pdu, 
V9fsFidState *fidp,
 V9fsString name;
 int len, err = 0;
 int32_t count = 0;
+off_t off;
 struct dirent *dent;
 struct stat *st;
 struct V9fsDirEnt *entries = NULL;
@@ -2480,12 +2482,

[PULL 09/19] 9p: darwin: Implement compatibility for mknodat

[Christian Schoenebeck]

From: Keno Fischer 

Darwin does not support mknodat. However, to avoid race conditions
with later setting the permissions, we must avoid using mknod on
the full path instead. We could try to fchdir, but that would cause
problems if multiple threads try to call mknodat at the same time.
However, luckily there is a solution: Darwin includes a function
that sets the cwd for the current thread only.
This should suffice to use mknod safely.

This function (pthread_fchdir_np) is protected by a check in
meson in a patch later in this series.

Signed-off-by: Keno Fischer 
Signed-off-by: Michael Roitzsch 
[Will Cohen: - Adjust coding style
 - Replace clang references with gcc
 - Note radar filed with Apple for missing syscall
 - Replace direct syscall with pthread_fchdir_np and
   adjust patch notes accordingly
 - Declare pthread_fchdir_np with
 - __attribute__((weak_import)) to allow checking for
   its presence before usage
 - Move declarations above cplusplus guard
 - Add CONFIG_PTHREAD_FCHDIR_NP to meson and check for
   presence in 9p-util
 - Rebase to apply cleanly on top of the 2022-02-10
   changes to 9pfs
 - Fix line over 90 characters formatting error]
Signed-off-by: Will Cohen 
Message-Id: <20220227223522.91937-10-wwco...@gmail.com>
Signed-off-by: Christian Schoenebeck 
Reviewed-by: Christian Schoenebeck 
---
 hw/9pfs/9p-local.c   |  4 ++--
 hw/9pfs/9p-util-darwin.c | 33 +
 hw/9pfs/9p-util-linux.c  |  6 ++
 hw/9pfs/9p-util.h| 11 +++
 meson.build  |  1 +
 5 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index a0d08e5216..d42ce6d8b8 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -682,7 +682,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath 
*dir_path,
 
 if (fs_ctx->export_flags & V9FS_SM_MAPPED ||
 fs_ctx->export_flags & V9FS_SM_MAPPED_FILE) {
-err = mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
+err = qemu_mknodat(dirfd, name, fs_ctx->fmode | S_IFREG, 0);
 if (err == -1) {
 goto out;
 }
@@ -697,7 +697,7 @@ static int local_mknod(FsContext *fs_ctx, V9fsPath 
*dir_path,
 }
 } else if (fs_ctx->export_flags & V9FS_SM_PASSTHROUGH ||
fs_ctx->export_flags & V9FS_SM_NONE) {
-err = mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
+err = qemu_mknodat(dirfd, name, credp->fc_mode, credp->fc_rdev);
 if (err == -1) {
 goto out;
 }
diff --git a/hw/9pfs/9p-util-darwin.c b/hw/9pfs/9p-util-darwin.c
index cdb4c9e24c..bec0253474 100644
--- a/hw/9pfs/9p-util-darwin.c
+++ b/hw/9pfs/9p-util-darwin.c
@@ -7,6 +7,8 @@
 
 #include "qemu/osdep.h"
 #include "qemu/xattr.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
 #include "9p-util.h"
 
 ssize_t fgetxattrat_nofollow(int dirfd, const char *filename, const char *name,
@@ -62,3 +64,34 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, 
const char *name,
 close_preserve_errno(fd);
 return ret;
 }
+
+/*
+ * As long as mknodat is not available on macOS, this workaround
+ * using pthread_fchdir_np is needed.
+ *
+ * Radar filed with Apple for implementing mknodat:
+ * rdar://FB9862426 (https://openradar.appspot.com/FB9862426)
+ */
+#if defined CONFIG_PTHREAD_FCHDIR_NP
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+int preserved_errno, err;
+if (!pthread_fchdir_np) {
+error_report_once("pthread_fchdir_np() not available on this version 
of macOS");
+return -ENOTSUP;
+}
+if (pthread_fchdir_np(dirfd) < 0) {
+return -1;
+}
+err = mknod(filename, mode, dev);
+preserved_errno = errno;
+/* Stop using the thread-local cwd */
+pthread_fchdir_np(-1);
+if (err < 0) {
+errno = preserved_errno;
+}
+return err;
+}
+
+#endif
diff --git a/hw/9pfs/9p-util-linux.c b/hw/9pfs/9p-util-linux.c
index 398614a5d0..db451b0784 100644
--- a/hw/9pfs/9p-util-linux.c
+++ b/hw/9pfs/9p-util-linux.c
@@ -61,4 +61,10 @@ int fsetxattrat_nofollow(int dirfd, const char *filename, 
const char *name,
 ret = lsetxattr(proc_path, name, value, size, flags);
 g_free(proc_path);
 return ret;
+
+}
+
+int qemu_mknodat(int dirfd, const char *filename, mode_t mode, dev_t dev)
+{
+return mknodat(dirfd, filename, mode, dev);
 }
diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
index 9abff79884..1f74d37558 100644
--- a/hw/9pfs/9p-util.h
+++ b/hw/9pfs/9p-util.h
@@ -112,5 +112,16 @@ static inline off_t qemu_dirent_off(struct dirent *dent)
 #endif
 }
 
+/*
+ * As long as mknodat is not available on macOS, this workaround
+ * using pthread_fchdir_np is needed. qemu_mknodat is defined in
+ * os-posix.c. pthread_fchdir_np is weakly linked here as a guard
+ * 

Re: [PATCH v3 1/3] linux-user/ppc: deliver SIGTRAP on POWERPC_EXCP_TRAP

[Laurent Vivier]

Le 13/01/2022 à 18:04, matheus.fe...@eldorado.org.br a écrit :

From: Matheus Ferst 

Handle POWERPC_EXCP_TRAP in cpu_loop to deliver SIGTRAP on tw[i]/td[i].
The si_code comes from do_program_check in the kernel source file
arch/powerpc/kernel/traps.c

Reviewed-by: Richard Henderson 
Signed-off-by: Matheus Ferst 
---
  linux-user/ppc/cpu_loop.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/linux-user/ppc/cpu_loop.c b/linux-user/ppc/cpu_loop.c
index 46e6ffd6d3..6c99feb19b 100644
--- a/linux-user/ppc/cpu_loop.c
+++ b/linux-user/ppc/cpu_loop.c
@@ -188,7 +188,8 @@ void cpu_loop(CPUPPCState *env)
  }
  break;
  case POWERPC_EXCP_TRAP:
-cpu_abort(cs, "Tried to call a TRAP\n");
+si_signo = TARGET_SIGTRAP;
+si_code = TARGET_TRAP_BRKPT;
  break;
  default:
  /* Should not happen ! */


Applied to my linux-user-for-7.0 branch.

Thanks,
Laurent