Re: [RFC PATCH v2 00/21] QEMU gmem implemention

[Xiaoyao Li]

On 9/21/2023 5:11 PM, David Hildenbrand wrote:
3. What is KVM_X86_SW_PROTECTED_VM going to look like? and do we 
need it?




Why implement it when you have to ask others for a motivation? 😉

Personally, I'm not sure if it is really useful, especially in this 
state.


Yeah, as of today, KVM_X86_SW_PROTECTED_VM is mainly a development 
vehicle,
e.g. so that testing gmem doesn't require TDX/SNP hardware, debugging 
gmem guests

isn't brutally painful, etc.

Longer term, I have aspirations of being able to back most VMs with 
gmem, but
that's going to require quite a bit more work, e.g. gmem needs to be 
mappable
(when hardware allows it) so that gmem doesn't all but require double 
mapping,

KVM obviously needs to be able to read/write gmem, etc.

The value proposition is that having a guest-first memory type will 
allow KVM to
optimize and harden gmem in ways that wouldn't be feasible for a more 
generic
memory implementation.  E.g. memory isn't mapped into host userspace 
by default
(makes it harder to accidentally corrupt the guest), the guest can 
have *larger*
mappings than host userspace, guest memory can be served from a 
dedicated pool
(similar-ish to hugetlb), the pool can be omitted from the direct map, 
etc.


Thanks for that information. Personally, I don't believe "to back most 
VMs with gmem", but that's a different discussion.


As a development vehicle to get TDX up and running it might be very 
valuable indeed. But it doesn't necessarily have to be merged in QEMU 
for that case -- especially in a semi-finished form.


It's true and I agree with it. I'll drop the KVM_X86_SW_PROTECTED_VM 
part in next version.


How would you like this series to proceed in next version? only the 
patches of gmem support without a user? or together with next QEMU TDX 
series?

Re: [PATCH v4] hw/i386/pc: improve physical address space bound check for 32-bit x86 systems

[David Hildenbrand]

On 22.09.23 06:16, Ani Sinha wrote:

32-bit x86 systems do not have a reserved memory for hole64. On those 32-bit
systems without PSE36 or PAE CPU features, hotplugging memory devices are not
supported by QEMU as QEMU always places hotplugged memory above 4 GiB boundary
which is beyond the physical address space of the processor. Linux guests also
does not support memory hotplug on those systems. Please see Linux
kernel commit b59d02ed08690 ("mm/memory_hotplug: disable the functionality
for 32b") for more details.

Therefore, the maximum limit of the guest physical address in the absence of
additional memory devices effectively coincides with the end of
"above 4G memory space" region for 32-bit x86 without PAE/PSE36. When users
configure additional memory devices, after properly accounting for the
additional device memory region to find the maximum value of the guest
physical address, the address will be outside the range of the processor's
physical address space.

This change adds improvements to take above into consideration.

For example, previously this was allowed:

$ ./qemu-system-x86_64 -cpu pentium -m size=10G

With this change now it is no longer allowed:

$ ./qemu-system-x86_64 -cpu pentium -m size=10G
qemu-system-x86_64: Address space limit 0x < 0x2bfff phys-bits too 
low (32)

However, the following are allowed since on both cases physical address
space of the processor is 36 bits:

$ ./qemu-system-x86_64 -cpu pentium2 -m size=10G
$ ./qemu-system-x86_64 -cpu pentium,pse36=on -m size=10G

For 32-bit, without PAE/PSE36, hotplugging additional memory is no longer 
allowed.

$ ./qemu-system-i386 -m size=1G,maxmem=3G,slots=2
qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
low (32)
$ ./qemu-system-i386 -machine q35 -m size=1G,maxmem=3G,slots=2
qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
low (32)

A new compatibility flag is introduced to make sure pc_max_used_gpa() keeps
returning the old value for machines 8.1 and older.
Therefore, the above is still allowed for older machine types in order to 
support
compatibility. Hence, the following still works:

$ ./qemu-system-i386 -machine pc-i440fx-8.1 -m size=1G,maxmem=3G,slots=2
$ ./qemu-system-i386 -machine pc-q35-8.1 -m size=1G,maxmem=3G,slots=2

Further, following is also allowed as with PSE36, the processor has 36-bit
address space:

$ ./qemu-system-i386 -cpu 486,pse36=on -m size=1G,maxmem=3G,slots=2

After calling CPUID with EAX=0x8001, all AMD64 compliant processors
have the longmode-capable-bit turned on in the extended feature flags (bit 29)
in EDX. The absence of CPUID longmode can be used to differentiate between
32-bit and 64-bit processors and is the recommended approach. QEMU takes this
approach elsewhere (for example, please see x86_cpu_realizefn()), With
this change, pc_max_used_gpa() also uses the same method to detect 32-bit
processors.

Unit tests are modified to not run 32-bit x86 tests that use memory hotplug.

Suggested-by: David Hildenbrand 
Signed-off-by: Ani Sinha 
---


LGTM, thanks

Reviewed-by: David Hildenbrand 

--
Cheers,

David / dhildenb

Re: [RFC PATCH v2 02/21] RAMBlock: Add support of KVM private gmem

[David Hildenbrand]

On 22.09.23 02:22, Xiaoyao Li wrote:

On 9/21/2023 4:55 PM, David Hildenbrand wrote:

On 14.09.23 05:50, Xiaoyao Li wrote:

From: Chao Peng 

Add KVM gmem support to RAMBlock so both normal hva based memory
and kvm gmem fd based private memory can be associated in one RAMBlock.

Introduce new flag RAM_KVM_GMEM. It calls KVM ioctl to create private
gmem for the RAMBlock when it's set.



But who sets RAM_KVM_GMEM and when?


The answer is in the next patch. When `private` property of memory
backend is set to true, it will pass RAM_KVM_GMEM flag to
memory_region_init_ram_*()


Okay, assuming that patch (and property) will go away, I assume this 
flag can also go away, right?


--
Cheers,

David / dhildenb

Re: [RFC PATCH v2 00/21] QEMU gmem implemention

[David Hildenbrand]

On 22.09.23 09:03, Xiaoyao Li wrote:

On 9/21/2023 5:11 PM, David Hildenbrand wrote:

3. What is KVM_X86_SW_PROTECTED_VM going to look like? and do we
need it?



Why implement it when you have to ask others for a motivation? 😉

Personally, I'm not sure if it is really useful, especially in this
state.


Yeah, as of today, KVM_X86_SW_PROTECTED_VM is mainly a development
vehicle,
e.g. so that testing gmem doesn't require TDX/SNP hardware, debugging
gmem guests
isn't brutally painful, etc.

Longer term, I have aspirations of being able to back most VMs with
gmem, but
that's going to require quite a bit more work, e.g. gmem needs to be
mappable
(when hardware allows it) so that gmem doesn't all but require double
mapping,
KVM obviously needs to be able to read/write gmem, etc.

The value proposition is that having a guest-first memory type will
allow KVM to
optimize and harden gmem in ways that wouldn't be feasible for a more
generic
memory implementation.  E.g. memory isn't mapped into host userspace
by default
(makes it harder to accidentally corrupt the guest), the guest can
have *larger*
mappings than host userspace, guest memory can be served from a
dedicated pool
(similar-ish to hugetlb), the pool can be omitted from the direct map,
etc.


Thanks for that information. Personally, I don't believe "to back most
VMs with gmem", but that's a different discussion.

As a development vehicle to get TDX up and running it might be very
valuable indeed. But it doesn't necessarily have to be merged in QEMU
for that case -- especially in a semi-finished form.


It's true and I agree with it. I'll drop the KVM_X86_SW_PROTECTED_VM
part in next version.

How would you like this series to proceed in next version? only the
patches of gmem support without a user? or together with next QEMU TDX
series?


Makes sense to me. GMEM series can be a prereq for QEMU TDX.

--
Cheers,

David / dhildenb

Re: [RFC PATCH v2 07/21] i386/pc: Drop pc_machine_kvm_type()

[David Hildenbrand]

On 22.09.23 02:24, Xiaoyao Li wrote:

On 9/21/2023 4:51 PM, David Hildenbrand wrote:

On 14.09.23 05:51, Xiaoyao Li wrote:

pc_machine_kvm_type() was introduced by commit e21be724eaf5 ("i386/xen:
add pc_machine_kvm_type to initialize XEN_EMULATE mode") to do Xen
specific initialization by utilizing kvm_type method.

commit eeedfe6c6316 ("hw/xen: Simplify emulated Xen platform init")
moves the Xen specific initialization to pc_basic_device_init().

There is no need to keep the PC specific kvm_type() implementation
anymore.


So we'll fallback to kvm_arch_get_default_type(), which simply returns 0.


On the other hand, later patch will implement kvm_type()
method for all x86/i386 machines to support KVM_X86_SW_PROTECTED_VM.



^ I suggest dropping that and merging that patch ahead-of-time as a
simple cleanup.


I suppose the "that" here means "this patch", right?



With that I meant that paragraph "On the other hand, later patch ...".

--
Cheers,

David / dhildenb

Re: [PATCH v1 16/22] backends/iommufd: Introduce the iommufd object

[Cédric Le Goater]

On 8/30/23 12:37, Zhenzhong Duan wrote:

From: Eric Auger 

Introduce an iommufd object which allows the interaction
with the host /dev/iommu device.

The /dev/iommu can have been already pre-opened outside of qemu,
in which case the fd can be passed directly along with the
iommufd object:

This allows the iommufd object to be shared accross several
subsystems (VFIO, VDPA, ...). For example, libvirt would open
the /dev/iommu once.

If no fd is passed along with the iommufd object, the /dev/iommu
is opened by the qemu code.

The CONFIG_IOMMUFD option must be set to compile this new object.

Suggested-by: Alex Williamson 
Signed-off-by: Eric Auger 
Signed-off-by: Yi Liu 
Signed-off-by: Zhenzhong Duan 
---
  MAINTAINERS  |   7 +
  backends/Kconfig |   4 +
  backends/iommufd.c   | 291 +++
  backends/meson.build |   3 +
  backends/trace-events|  13 ++
  include/sysemu/iommufd.h |  49 +++
  qapi/qom.json|  18 ++-
  qemu-options.hx  |  13 ++
  8 files changed, 397 insertions(+), 1 deletion(-)
  create mode 100644 backends/iommufd.c
  create mode 100644 include/sysemu/iommufd.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6111b6b4d9..04663fbb6f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2079,6 +2079,13 @@ F: hw/vfio/ap.c
  F: docs/system/s390x/vfio-ap.rst
  L: qemu-s3...@nongnu.org
  
+iommufd

+M: Yi Liu 
+M: Eric Auger 
+S: Supported
+F: backends/iommufd.c
+F: include/sysemu/iommufd.h
+
  vhost
  M: Michael S. Tsirkin 
  S: Supported
diff --git a/backends/Kconfig b/backends/Kconfig
index f35abc1609..2cb23f62fa 100644
--- a/backends/Kconfig
+++ b/backends/Kconfig
@@ -1 +1,5 @@
  source tpm/Kconfig
+
+config IOMMUFD
+bool
+depends on VFIO
diff --git a/backends/iommufd.c b/backends/iommufd.c
new file mode 100644
index 00..07ea434424
--- /dev/null
+++ b/backends/iommufd.c
@@ -0,0 +1,291 @@
+/*
+ * iommufd container backend
+ *
+ * Copyright (C) 2023 Intel Corporation.
+ * Copyright Red Hat, Inc. 2023
+ *
+ * Authors: Yi Liu 
+ *  Eric Auger 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "sysemu/iommufd.h"
+#include "qapi/error.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/module.h"
+#include "qom/object_interfaces.h"
+#include "qemu/error-report.h"
+#include "monitor/monitor.h"
+#include "trace.h"
+#include 
+#include 
+
+static void iommufd_backend_init(Object *obj)
+{
+IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
+
+be->fd = -1;
+be->users = 0;
+be->owned = true;
+qemu_mutex_init(&be->lock);
+}
+
+static void iommufd_backend_finalize(Object *obj)
+{
+IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
+
+if (be->owned) {
+close(be->fd);
+be->fd = -1;
+}
+}
+
+static void iommufd_backend_set_fd(Object *obj, const char *str, Error **errp)
+{
+IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
+int fd = -1;
+
+fd = monitor_fd_param(monitor_cur(), str, errp);
+if (fd == -1) {
+error_prepend(errp, "Could not parse remote object fd %s:", str);
+return;
+}
+qemu_mutex_lock(&be->lock);
+be->fd = fd;
+be->owned = false;
+qemu_mutex_unlock(&be->lock);
+trace_iommu_backend_set_fd(be->fd);
+}
+
+static void iommufd_backend_class_init(ObjectClass *oc, void *data)
+{
+object_class_property_add_str(oc, "fd", NULL, iommufd_backend_set_fd);
+}
+
+int iommufd_backend_connect(IOMMUFDBackend *be, Error **errp)
+{
+int fd, ret = 0;
+
+qemu_mutex_lock(&be->lock);
+if (be->users == UINT32_MAX) {
+error_setg(errp, "too many connections");
+ret = -E2BIG;
+goto out;
+}
+if (be->owned && !be->users) {
+fd = qemu_open_old("/dev/iommu", O_RDWR);
+if (fd < 0) {
+error_setg_errno(errp, errno, "/dev/iommu opening failed");
+ret = fd;
+goto out;
+}
+be->fd = fd;
+}
+be->users++;
+out:
+trace_iommufd_backend_connect(be->fd, be->owned,
+  be->users, ret);
+qemu_mutex_unlock(&be->lock);
+return ret;
+}
+
+void iommufd_backend_disconnect(IOMMUFDBackend *be)
+{
+qemu_mutex_lock(&be->lock);
+if (!be->users) {
+goto out;
+}
+be->users--;
+if (!be->users && be->owned) {
+close(be->fd);
+be->fd = -1;

Re: [Bug 1819289] Re: Windows 95 and Windows 98 will not install or run

[Michael Tokarev]

21.09.2023 19:26, John M wrote:

This is happening again in 8.1. I used Windows 95 for a while in 6.1 and it
was fine, but when I tried to upgrade to 8.1, it started happening again. I
tried reducing the memory and it still happens. Not an urgent issue though.


Can you show me a way to install & run win95/98? Is it available on a CD
image somewhere?

/mjt

Re: [PATCH 1/2] migration: Fix rdma migration failed

[Zhijian Li (Fujitsu)]

On 20/09/2023 20:46, Fabiano Rosas wrote:
> Li Zhijian  writes:
> 
>> From: Li Zhijian 
>>
>> Destination will fail with:
>> qemu-system-x86_64: rdma: Too many requests in this message 
>> (3638950032).Bailing.
>>
>> migrate with RDMA is different from tcp. RDMA has its own control
>> message, and all traffic between RDMA_CONTROL_REGISTER_REQUEST and
>> RDMA_CONTROL_REGISTER_FINISHED should not be disturbed.
> 
> Yeah, this is really fragile. We need a long term solution to this. Any
> other change to multifd protocol as well as any other change to the
> migration ram handling might hit this issue again.

Yeah, it's pain point.

Another option is that let RDMA control handler to know 
RAM_SAVE_FLAG_MULTIFD_FLUSH message
and do nothing with it.


> 
> Perhaps commit 294e5a4034 ("multifd: Only flush once each full round of
> memory") should simply not have touched the stream at that point, but we
> don't have any explicit safeguards to avoid interleaving flags from
> different layers like that (assuming multifd is at another logical layer
> than the ram handling)> 
> I don't have any good suggestions at this moment, so for now:
> 
> Reviewed-by: Fabiano Rosas 

Re: [PATCH v13 6/9] gfxstream + rutabaga: add initial support for gfxstream

[Alyssa Ross]

Akihiko Odaki  writes:

> Practically there is very low chance to hit the bug. I think only 
> fuzzers and malicious actors will trigger it, and probably no one will 
> dare using virtio-gpu-rutabaga or virtio-gpu-gl in a security-sensitive 
> context.

Well, this is exactly what Chrome OS does, albiet with crosvm rather
than QEMU, right?


signature.asc
Description: PGP signature

[PATCH] Update AMD memory encryption document links.

[Jianlin Li]

The previous links for the white paper and programmer's manual 
are no longer available. Replace them with the new ones.

Signed-off-by: Jianlin Li 
---
 docs/system/i386/amd-memory-encryption.rst | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/system/i386/amd-memory-encryption.rst 
b/docs/system/i386/amd-memory-encryption.rst
index dcf4add0e7..e9bc142bc1 100644
--- a/docs/system/i386/amd-memory-encryption.rst
+++ b/docs/system/i386/amd-memory-encryption.rst
@@ -183,13 +183,13 @@ References
 --
 
 `AMD Memory Encryption whitepaper
-`_
+`_
 
 .. [SEVAPI] `Secure Encrypted Virtualization API

`_
 
 .. [APMVOL2] `AMD64 Architecture Programmer's Manual Volume 2: System 
Programming
-   `_
+   
`_
 
 KVM Forum slides:
 
@@ -199,7 +199,7 @@ KVM Forum slides:
   
`_
 
 `AMD64 Architecture Programmer's Manual:
-`_
+`_
 
 * SME is section 7.10
 * SEV is section 15.34
-- 
2.25.1

Re: [PATCH 11/52] migration/rdma: Drop rdma_add_block() error handling

[Zhijian Li (Fujitsu)]

On 21/09/2023 19:15, Markus Armbruster wrote:
> "Zhijian Li (Fujitsu)"  writes:
> 
>> On 18/09/2023 22:41, Markus Armbruster wrote:
>>> rdma_add_block() can't fail.  Return void, and drop the unreachable
>>> error handling.
>>>
>>> Signed-off-by: Markus Armbruster
>>> ---
>>>migration/rdma.c | 30 +-
>>>1 file changed, 9 insertions(+), 21 deletions(-)
>>>
>>
>> [...]
>>
>>> * during dynamic page registration.
>>> */
>>> -static int qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>>> +static void qemu_rdma_init_ram_blocks(RDMAContext *rdma)
>>>{
>>>RDMALocalBlocks *local = &rdma->local_ram_blocks;
>>>int ret;
>>> @@ -646,14 +645,11 @@ static int qemu_rdma_init_ram_blocks(RDMAContext 
>>> *rdma)
>>>assert(rdma->blockmap == NULL);
>>>memset(local, 0, sizeof *local);
>>>ret = foreach_not_ignored_block(qemu_rdma_init_one_block, rdma);
>>> -if (ret) {
>>> -return ret;
>>> -}
>>> +assert(!ret);
>>
>> Why we still need a new assert(), can we remove the ret together.
>>
>>   foreach_not_ignored_block(qemu_rdma_init_one_block, rdma);
>>   trace_qemu_rdma_init_ram_blocks(local->nb_blocks);
> 
> The "the callback doesn't fail" is a non-local argument.  The assertion
> checks it.  I'd be fine with dropping it, since the argument is
> straightforward enough.  Thoughts?
> 

Both are fine, I prefer to drop it personally. :)

Anyway,

Reviewed-by: Li Zhijian 

Re: [PATCH 12/52] migration/rdma: Drop qemu_rdma_search_ram_block() error handling

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> qemu_rdma_search_ram_block() can't fail.  Return void, and drop the
> unreachable error handling.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Li Zhijian 

Re: [PATCH 13/52] migration/rdma: Make qemu_rdma_buffer_mergable() return bool

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> qemu_rdma_buffer_mergable() is semantically a predicate.  It returns
> int 0 or 1.  Return bool instead.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Li Zhijian 

Re: [PATCH v13 6/9] gfxstream + rutabaga: add initial support for gfxstream

[Akihiko Odaki]

On 2023/09/22 16:42, Alyssa Ross wrote:

Akihiko Odaki  writes:


Practically there is very low chance to hit the bug. I think only
fuzzers and malicious actors will trigger it, and probably no one will
dare using virtio-gpu-rutabaga or virtio-gpu-gl in a security-sensitive
context.


Well, this is exactly what Chrome OS does, albiet with crosvm rather
than QEMU, right?


I think so, but QEMU's virtio-gpu-rutabaga and virtio-gpu-gl should be 
very different from crosvm in terms that it does not isolate the 
graphics stack into a separate process while I believe crosvm does so. 
Having the entire graphics stack in a VMM is a security nightmare; it 
means giving a complex shader compiler the highest privilege. We need to 
use vhost-user-gpu instead for process isolation.


Since we already have such a serious security hazard, I don't think we 
have to care much about security. But security approximately equals to 
reliability, which matters for virtio-gpu-rutabaga and virtio-gpu-gl 
too, so it's still nice to get the bug fixed.

Re: [PATCH 14/52] migration/rdma: Use bool for two RDMAContext flags

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> @error_reported and @received_error are flags.  The latter is even
> assigned bool true.  Change them from int to bool.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Li Zhijian 

> ---
>   migration/rdma.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 97715dbd78..c02a1c83b2 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -91,7 +91,7 @@ static uint32_t known_capabilities = 
> RDMA_CAPABILITY_PIN_ALL;
>   if (!rdma->error_reported) { \
>   error_report("RDMA is in an error state waiting migration" \
>   " to abort!"); \
> -rdma->error_reported = 1; \
> +rdma->error_reported = true; \
>   } \
>   return rdma->error_state; \
>   } \
> @@ -365,8 +365,8 @@ typedef struct RDMAContext {
>* and remember the error state.
>*/
>   int error_state;
> -int error_reported;
> -int received_error;
> +bool error_reported;
> +bool received_error;
>   
>   /*
>* Description of ram blocks used throughout the code.

RE: [PATCH v1 0/4] vfio: report NUMA nodes for device memory

[Ankit Agrawal]

> >>
> >> Typically, a command line for a virt machine with NUMA nodes would
> >> look like :
> >>
> >> -object memory-backend-ram,id=ram-node0,size=1G \
> >> -numa node,nodeid=0,memdev=ram-node0 \
> >> -object memory-backend-ram,id=ram-node1,size=1G \
> >> -numa node,nodeid=1,cpus=0-3,memdev=ram-node1
> >>
> >> which defines 2 nodes, one with memory and all CPUs and a second with
> >> only memory.
> >>
> >> # numactl -H
> >> available: 2 nodes (0-1)
> >> node 0 cpus: 0 1 2 3
> >> node 0 size: 1003 MB
> >> node 0 free: 734 MB
> >> node 1 cpus:
> >> node 1 size: 975 MB
> >> node 1 free: 968 MB
> >> node distances:
> >> node   0   1
> >>   0:  10  20
> >>   1:  20  10
> >>
> >>
> >> Could it be a new type of host memory backend ?  Have you considered
> >> this approach ?
> >
> > Good idea.  Fundamentally the device should not be creating NUMA
> > nodes, the VM should be configured with NUMA nodes and the device
> > memory associated with those nodes.
> 
> +1. That would also make it fly with DIMMs and virtio-mem, where you
> would want NUMA-less nodes ass well (imagine passing CXL memory to a VM
> using virtio-mem).
> 

We actually do not add the device memory on the host, instead
map it into the Qemu VMA using remap_pfn_range(). Please checkout the
mmap function in vfio-pci variant driver code managing the device.
https://lore.kernel.org/all/20230915025415.6762-1-ank...@nvidia.com/
And I think host memory backend would need memory that is added on the
host.

Moreover since we want to passthrough the entire device memory, the
-object memory-backend-ram would have to be passed a size that is equal
to the device memory. I wonder if that would be too much of a trouble
for an admin (or libvirt) triggering the Qemu process.

Both these items are avoided by exposing the device memory as BAR as in the
current  implementation (referenced above) since it lets Qemu to naturally
discover the device memory region and do mmap.

Re: [PATCH v1 0/4] vfio: report NUMA nodes for device memory

[David Hildenbrand]

On 22.09.23 10:11, Ankit Agrawal wrote:


Typically, a command line for a virt machine with NUMA nodes would
look like :

 -object memory-backend-ram,id=ram-node0,size=1G \
 -numa node,nodeid=0,memdev=ram-node0 \
 -object memory-backend-ram,id=ram-node1,size=1G \
 -numa node,nodeid=1,cpus=0-3,memdev=ram-node1

which defines 2 nodes, one with memory and all CPUs and a second with
only memory.

 # numactl -H
 available: 2 nodes (0-1)
 node 0 cpus: 0 1 2 3
 node 0 size: 1003 MB
 node 0 free: 734 MB
 node 1 cpus:
 node 1 size: 975 MB
 node 1 free: 968 MB
 node distances:
 node   0   1
   0:  10  20
   1:  20  10


Could it be a new type of host memory backend ?  Have you considered
this approach ?


Good idea.  Fundamentally the device should not be creating NUMA
nodes, the VM should be configured with NUMA nodes and the device
memory associated with those nodes.


+1. That would also make it fly with DIMMs and virtio-mem, where you
would want NUMA-less nodes ass well (imagine passing CXL memory to a VM
using virtio-mem).



We actually do not add the device memory on the host, instead
map it into the Qemu VMA using remap_pfn_range(). Please checkout the
mmap function in vfio-pci variant driver code managing the device.
https://lore.kernel.org/all/20230915025415.6762-1-ank...@nvidia.com/
And I think host memory backend would need memory that is added on the
host.

Moreover since we want to passthrough the entire device memory, the
-object memory-backend-ram would have to be passed a size that is equal
to the device memory. I wonder if that would be too much of a trouble
for an admin (or libvirt) triggering the Qemu process.

Both these items are avoided by exposing the device memory as BAR as in the
current  implementation (referenced above) since it lets Qemu to naturally
discover the device memory region and do mmap.



Just to clarify: nNUMA nodes for DIMMs/NVDIMMs/virtio-mem are configured 
on the device, not on the memory backend.


e.g., -device pc-dimm,node=3,memdev=mem1,...

--
Cheers,

David / dhildenb

Re: [PATCH v4 2/3] i386: Explicitly ignore unsupported BUS_MCEERR_AO MCE on AMD guest

[William Roche]

On 9/21/23 19:41, Yazen Ghannam wrote:

On 9/20/23 7:13 AM, Joao Martins wrote:

On 18/09/2023 23:00, William Roche wrote:

[...]
So it looks like the mechanism works fine... unless the VM has migrated
between the SRAO error and the first time it really touches the poisoned
page to get an SRAR error !  In this case, its new address space
(created on the migration destination) will have a zero-page where we
had a poisoned page, and the AMD VM Kernel (that never dealt with the
SRAO) doesn't know about the poisoned page and will access the page
finding only zeros...  We have a memory corruption !


I don't understand this. Why would the page be zero? Even so, why would
that affect poison?


The migration of a VM moves the memory content from a source platform to
a destination. This is mainly the qemu processes reading the data and
replicating it on the destination. The source qemu where a memory page
is poisoned is(will be[*]) able to skip the poisoned pages it knows
about to indicate to the destination machine to populate the associated
page(s) with zeros as there is no "poison destination page" mechanism in
place for this migration transfer.



Also, during page migration, does the data flow through the CPU core?
Sorry for the basic question. I haven't done a lot with virtualization.


Yes, in most cases (with the exception of RDMA) the data flow through
the CPU cores because the migration verifies if the area to transfer has
some empty pages.



Please note that current AMD systems use an internal poison marker on
memory. This cannot be cleared through normal memory operations. The
only exception, I think, is to use the CLZERO instruction. This will
completely wipe a cacheline including metadata like poison, etc.

So the hardware should not (by design) loose track of poisoned data.


This would be better, but virtualization migration currently looses
track of this.
Which is not a problem for VMs where the kernel took note of the poison
and keeps track of it. Because this kernel will handle the poison
locations it knows about, signaling when these poisoned locations are
touched.





It is a very rare window, but in order to fix it the most reasonable
course of action would be to make the AMD emulation deal with SRAO
errors, instead of ignoring them.

Do you agree with my analysis ?


Under the case that SRAO aren't handled well in the kernel today[*] for AMD, we
could always add a migration blocker when we hit AO sigbus, in case ignoring
is our only option. But this would be less than ideal to propagating the
SRAO into the guest.

[*] Meaning knowing that handling the SRAO would generate a crash in the guest

Perhaps as an improvement, perhaps allow qemu to choose to propagate should this
limitation be lifted via a new -action value and allow it to ignore/propagate or
not e.g.

  -action mce=none # default on Intel to propagate all MCE events to the guest
  -action mce=ignore-optional # Ignore SRAO


Yes we may need to create something like that, but missing SRAO has
technical consequences too.



I suppose the second is also useful for ARM64 considering they currently ignore
SRAO events too.


Would an AMD platform generate SRAO signal to a process
(SIGBUS/BUS_MCEERR_AO) in case of a real hardware error ?


This would be useful to confirm.



There is no SRAO signal on AMD. The closest equivalent may be a
"Deferred" error interrupt. This is an x86 APIC LVT interrupt, and it's
sent when a deferred (uncorrectable non-urgent) error is detected by a
memory controller.

In this case, the CPU will get the interrupt and log the error (in the
host).

An enhancement will be to take the MCA error information collected
during the interrupt and extract useful data. For example, we'll need to
translate the reported address to a system physical address that can be
mapped to a page.


This would be great, as it would mean that a kernel running in a VM can
get notified too.



Once we have the page, then we can decide how we want to signal the
process(es). We could get a deferred/AO error in the host, and signal the
guest with an AR. So the guest handling could be the same in both cases. >
Would this be okay? Or is it important that the guest can distinguish
between the A0/AR cases?



SIGBUS/BUS_MCEERR_AO and BUS_MCEERR_AR are not interchangeable, it is
important to distinguish them.
AO is an asynchronous signal that is only generated when the process
asked for it -- indicating that an error has been detected in its
address space but hasn't been touched yet.
Most of the processes don't care about that (and don't get notified),
they just continue to run, if the poisoned area is not touched, great.
Otherwise a BUS_MCEERR_AR signal is generated when the area is touched,
indicating that the execution thread can't access the location.



IOW, will guests have their own policies on
when to take action? Or is it more about allowing the guest to handle
the error less urgently?


Yes to both questions. Any process can indicate 

RE: [PATCH v1 16/22] backends/iommufd: Introduce the iommufd object

[Duan, Zhenzhong]

>-Original Message-
>From: Cédric Le Goater 
>Sent: Friday, September 22, 2023 3:16 PM
...
>> +int iommufd_backend_map_dma(IOMMUFDBackend *be, uint32_t ioas,
>hwaddr iova,
>> +ram_addr_t size, void *vaddr, bool readonly)
>> +{
>> +int ret;
>> +struct iommu_ioas_map map = {
>> +.size = sizeof(map),
>> +.flags = IOMMU_IOAS_MAP_READABLE |
>> + IOMMU_IOAS_MAP_FIXED_IOVA,
>> +.ioas_id = ioas,
>> +.__reserved = 0,
>> +.user_va = (int64_t)vaddr,
>
>This needs an extra cast (uintptr_t)

Will fix.

Thanks
Zhenzhong

Re: [PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> Several error messages include numeric error codes returned by failed
> functions:
> 
> * ibv_poll_cq() returns an unspecified negative value.  Useless.
> 
> * rdma_accept and rmda_get_cm_event() return -1.  Useless.


s/rmda_get_cm_event/rdma_get_cm_event

Otherwise,
Reviewed-by: Li Zhijian 


> 
> * qemu_rdma_poll() returns either -1 or an unspecified negative
>value.  Useless.
> 
> * qemu_rdma_block_for_wrid(), qemu_rdma_write_flush(),
>qemu_rdma_exchange_send(), qemu_rdma_exchange_recv(),
>qemu_rdma_write() return a negative value that may or may not be an
>errno value.  While reporting human-readable errno
>information (which a number is not) can be useful, reporting an
>error code that may or may not be an errno value is useless.
> 
> Drop these error codes from the error messages.
> 
> Signed-off-by: Markus Armbruster 
> ---
>   migration/rdma.c | 20 ++--
>   1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index c02a1c83b2..2173cb076f 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -1460,7 +1460,7 @@ static int qemu_rdma_poll(RDMAContext *rdma, struct 
> ibv_cq *cq,
>   }
>   
>   if (ret < 0) {
> -error_report("ibv_poll_cq return %d", ret);
> +error_report("ibv_poll_cq failed");
>   return ret;
>   }
>   
> @@ -2194,7 +2194,7 @@ retry:
>   ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
>   if (ret < 0) {
>   error_report("rdma migration: failed to make "
> - "room in full send queue! %d", ret);
> + "room in full send queue!");
>   return ret;
>   }
>   
> @@ -2770,7 +2770,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
>   ret = qemu_rdma_write_flush(f, rdma);
>   if (ret < 0) {
>   rdma->error_state = ret;
> -error_setg(errp, "qemu_rdma_write_flush returned %d", ret);
> +error_setg(errp, "qemu_rdma_write_flush failed");
>   return -1;
>   }
>   
> @@ -2790,7 +2790,7 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
>   
>   if (ret < 0) {
>   rdma->error_state = ret;
> -error_setg(errp, "qemu_rdma_exchange_send returned %d", ret);
> +error_setg(errp, "qemu_rdma_exchange_send failed");
>   return -1;
>   }
>   
> @@ -2880,7 +2880,7 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
>   
>   if (ret < 0) {
>   rdma->error_state = ret;
> -error_setg(errp, "qemu_rdma_exchange_recv returned %d", ret);
> +error_setg(errp, "qemu_rdma_exchange_recv failed");
>   return -1;
>   }
>   
> @@ -3222,7 +3222,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>*/
>   ret = qemu_rdma_write(f, rdma, block_offset, offset, size);
>   if (ret < 0) {
> -error_report("rdma migration: write error! %d", ret);
> +error_report("rdma migration: write error");
>   goto err;
>   }
>   
> @@ -3249,7 +3249,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>   uint64_t wr_id, wr_id_in;
>   int ret = qemu_rdma_poll(rdma, rdma->recv_cq, &wr_id_in, NULL);
>   if (ret < 0) {
> -error_report("rdma migration: polling error! %d", ret);
> +error_report("rdma migration: polling error");
>   goto err;
>   }
>   
> @@ -3264,7 +3264,7 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>   uint64_t wr_id, wr_id_in;
>   int ret = qemu_rdma_poll(rdma, rdma->send_cq, &wr_id_in, NULL);
>   if (ret < 0) {
> -error_report("rdma migration: polling error! %d", ret);
> +error_report("rdma migration: polling error");
>   goto err;
>   }
>   
> @@ -3439,13 +3439,13 @@ static int qemu_rdma_accept(RDMAContext *rdma)
>   
>   ret = rdma_accept(rdma->cm_id, &conn_param);
>   if (ret) {
> -error_report("rdma_accept returns %d", ret);
> +error_report("rdma_accept failed");
>   goto err_rdma_dest_wait;
>   }
>   
>   ret = rdma_get_cm_event(rdma->channel, &cm_event);
>   if (ret) {
> -error_report("rdma_accept get_cm_event failed %d", ret);
> +error_report("rdma_accept get_cm_event failed");
>   goto err_rdma_dest_wait;
>   }
>   

Re: [PATCH 16/52] migration/rdma: Fix io_writev(), io_readv() methods to obey contract

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> QIOChannelClass methods qio_channel_rdma_readv() and
> qio_channel_rdma_writev() violate their method contract when
> rdma->error_state is non-zero:
> 
> 1. They return whatever is in rdma->error_state then.  Only -1 will be
> fine.  -2 will be misinterpreted as "would block".  Anything less
> than -2 isn't defined in the contract.  A positive value would be
> misinterpreted as success, but I believe that's not actually
> possible.
> 
> 2. They neglect to set an error then.  If something up the call stack
> dereferences the error when failure is returned, it will crash.  If
> it ignores the return value and checks the error instead, it will
> miss the error.
> 
> Crap like this happens when return statements hide in macros,
> especially when their uses are far away from the definition.
> 
> I elected not to investigate how callers are impacted.
> 
> Expand the two bad macro uses, so we can set an error and return -1.
> The next commit will then get rid of the macro altogether.
> 
> Signed-off-by: Markus Armbruster 


Reviewed-by: Li Zhijian 


> ---
>   migration/rdma.c | 12 ++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 2173cb076f..30e6dff875 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2761,7 +2761,11 @@ static ssize_t qio_channel_rdma_writev(QIOChannel *ioc,
>   return -1;
>   }
>   
> -CHECK_ERROR_STATE();
> +if (rdma->error_state) {
> +error_setg(errp,
> +   "RDMA is in an error state waiting migration to abort!");
> +return -1;
> +}
>   
>   /*
>* Push out any writes that
> @@ -2847,7 +2851,11 @@ static ssize_t qio_channel_rdma_readv(QIOChannel *ioc,
>   return -1;
>   }
>   
> -CHECK_ERROR_STATE();
> +if (rdma->error_state) {
> +error_setg(errp,
> +   "RDMA is in an error state waiting migration to abort!");
> +return -1;
> +}
>   
>   for (i = 0; i < niov; i++) {
>   size_t want = iov[i].iov_len;

Re: [PATCH 17/52] migration/rdma: Replace dangerous macro CHECK_ERROR_STATE()

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> Hiding return statements in macros is a bad idea.  Use a function
> instead, and open code the return part.
> 
> Signed-off-by: Markus Armbruster 

Reviewed-by: Li Zhijian 


> ---
>   migration/rdma.c | 43 +++
>   1 file changed, 27 insertions(+), 16 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 30e6dff875..be66f53489 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -85,18 +85,6 @@
>*/
>   static uint32_t known_capabilities = RDMA_CAPABILITY_PIN_ALL;
>   
> -#define CHECK_ERROR_STATE() \
> -do { \
> -if (rdma->error_state) { \
> -if (!rdma->error_reported) { \
> -error_report("RDMA is in an error state waiting migration" \
> -" to abort!"); \
> -rdma->error_reported = true; \
> -} \
> -return rdma->error_state; \
> -} \
> -} while (0)
> -
>   /*
>* A work request ID is 64-bits and we split up these bits
>* into 3 parts:
> @@ -451,6 +439,16 @@ typedef struct QEMU_PACKED {
>   uint64_t chunks;/* how many sequential chunks to register */
>   } RDMARegister;
>   
> +static int check_error_state(RDMAContext *rdma)
> +{
> +if (rdma->error_state && !rdma->error_reported) {
> +error_report("RDMA is in an error state waiting migration"
> + " to abort!");
> +rdma->error_reported = true;
> +}
> +return rdma->error_state;
> +}
> +
>   static void register_to_network(RDMAContext *rdma, RDMARegister *reg)
>   {
>   RDMALocalBlock *local_block;
> @@ -3219,7 +3217,10 @@ static size_t qemu_rdma_save_page(QEMUFile *f,
>   return -EIO;
>   }
>   
> -CHECK_ERROR_STATE();
> +ret = check_error_state(rdma);
> +if (ret) {
> +return ret;
> +}
>   
>   qemu_fflush(f);
>   
> @@ -3535,7 +3536,10 @@ static int qemu_rdma_registration_handle(QEMUFile *f)
>   return -EIO;
>   }
>   
> -CHECK_ERROR_STATE();
> +ret = check_error_state(rdma);
> +if (ret) {
> +return ret;
> +}
>   
>   local = &rdma->local_ram_blocks;
>   do {
> @@ -3839,6 +3843,7 @@ static int qemu_rdma_registration_start(QEMUFile *f,
>   {
>   QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(qemu_file_get_ioc(f));
>   RDMAContext *rdma;
> +int ret;
>   
>   if (migration_in_postcopy()) {
>   return 0;
> @@ -3850,7 +3855,10 @@ static int qemu_rdma_registration_start(QEMUFile *f,
>   return -EIO;
>   }
>   
> -CHECK_ERROR_STATE();
> +ret = check_error_state(rdma);
> +if (ret) {
> +return ret;
> +}
>   
>   trace_qemu_rdma_registration_start(flags);
>   qemu_put_be64(f, RAM_SAVE_FLAG_HOOK);
> @@ -3881,7 +3889,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f,
>   return -EIO;
>   }
>   
> -CHECK_ERROR_STATE();
> +ret = check_error_state(rdma);
> +if (ret) {
> +return ret;
> +}
>   
>   qemu_fflush(f);
>   ret = qemu_rdma_drain_cq(f, rdma);

Re: [PATCH 18/52] migration/rdma: Fix qemu_rdma_broken_ipv6_kernel() to set error

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> qemu_rdma_resolve_host() and qemu_rdma_dest_init() try addresses until
> they find on that works.  If none works, they return the first Error
> set by qemu_rdma_broken_ipv6_kernel(), or else return a generic one.
> 
> qemu_rdma_broken_ipv6_kernel() neglects to set an Error when
> ibv_open_device() fails.  If a later address fails differently, we use
> that Error instead, or else the generic one.  Harmless enough, but
> needs fixing all the same.
> 
> Signed-off-by: Markus Armbruster 

Wow... IPV6 + RDMA, i have never used it, though

Reviewed-by: Li Zhijian 



> ---
>   migration/rdma.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index be66f53489..08cd186385 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -855,6 +855,8 @@ static int qemu_rdma_broken_ipv6_kernel(struct 
> ibv_context *verbs, Error **errp)
>   if (errno == EPERM) {
>   continue;
>   } else {
> +error_setg_errno(errp, errno,
> + "could not open RDMA device context");
>   return -EINVAL;
>   }
>   }

Re: [PATCH 19/52] migration/rdma: Fix qemu_get_cm_event_timeout() to always set error

[Zhijian Li (Fujitsu)]

On 18/09/2023 22:41, Markus Armbruster wrote:
> qemu_get_cm_event_timeout() neglects to set an error when it fails
> because rdma_get_cm_event() fails.  Harmless, as its caller
> qemu_rdma_connect() substitutes a generic error then.  Fix it anyway.
> 
> qemu_rdma_connect() also sets the generic error when its own call of
> rdma_get_cm_event() fails.  Make the error handling more obvious: set
> a specific error right after rdma_get_cm_event() fails.  Delete the
> generic error.
> 
> Signed-off-by: Markus Armbruster 


Reviewed-by: Li Zhijian 



> ---
>   migration/rdma.c | 10 --
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/migration/rdma.c b/migration/rdma.c
> index 08cd186385..d3dc162363 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -2509,7 +2509,11 @@ static int qemu_get_cm_event_timeout(RDMAContext *rdma,
>   ERROR(errp, "failed to poll cm event, errno=%i", errno);
>   return -1;
>   } else if (poll_fd.revents & POLLIN) {
> -return rdma_get_cm_event(rdma->channel, cm_event);
> +if (rdma_get_cm_event(rdma->channel, cm_event) < 0) {
> +ERROR(errp, "failed to get cm event");
> +return -1;
> +}
> +return 0;
>   } else {
>   ERROR(errp, "no POLLIN event, revent=%x", poll_fd.revents);
>   return -1;
> @@ -2559,10 +2563,12 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool 
> return_path,
>   ret = qemu_get_cm_event_timeout(rdma, &cm_event, 5000, errp);
>   } else {
>   ret = rdma_get_cm_event(rdma->channel, &cm_event);
> +if (ret < 0) {
> +ERROR(errp, "failed to get cm event");
> +}
>   }
>   if (ret) {
>   perror("rdma_get_cm_event after rdma_connect");
> -ERROR(errp, "connecting to destination!");
>   goto err_rdma_source_connect;
>   }
>   

Re: [PATCH 1/2] pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7

[Paolo Bonzini]

Il ven 22 set 2023, 08:43 Thomas Huth  ha scritto:

>
> While you're at it ... do we maybe want to start deprecating the next
> batch
> of machine types already? (Say pc-i440fx-2.0 up to pc-i440fx-2.2 maybe?)
>

It depends on the benefit. We would have to check the compat options that
are not needed anymore, and whether they'd be something that is useful
anyway for debugging.

Also it would be useful to check if isapc can drop some of the compat code
and realign itself to the 2.0 i440fx machine in terms of QEMU-specific
features.

Because of all this todo, I decided not to proceed further with
deprecations. The 128k ROM on the other hand does provide immediate benefit.

Paolo


> > -
> >   Backend options
> >   ---
> >
> > diff --git a/docs/about/removed-features.rst
> b/docs/about/removed-features.rst
> > index 39468b6e926..56e078ad126 100644
> > --- a/docs/about/removed-features.rst
> > +++ b/docs/about/removed-features.rst
> > @@ -730,7 +730,7 @@ mips ``fulong2e`` machine alias (removed in 6.0)
> >
> >   This machine has been renamed ``fuloong2e``.
> >
> > -``pc-0.10`` up to ``pc-1.3`` (removed in 4.0 up to 6.0)
> > +``pc-0.10`` up to ``pc-1.7`` (removed in 4.0 up to 8.2)
> >   '''
>
> The names started to change with version 1.4, so it's "pc-i440fx-1.7" and
> not "pc-1.7".
>
> >   These machine types were very old and likely could not be used for live
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index 54838c0c411..1c7898a2d34 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -359,60 +359,6 @@ GlobalProperty pc_compat_2_0[] = {
> >   };
> >   const size_t pc_compat_2_0_len = G_N_ELEMENTS(pc_compat_2_0);
> >
> > -GlobalProperty pc_compat_1_7[] = {
> > -PC_CPU_MODEL_IDS("1.7.0")
> > -{ TYPE_USB_DEVICE, "msos-desc", "no" },
> > -{ "PIIX4_PM", ACPI_PM_PROP_ACPI_PCIHP_BRIDGE, "off" },
> > -{ "hpet", HPET_INTCAP, "4" },
> > -};
> > -const size_t pc_compat_1_7_len = G_N_ELEMENTS(pc_compat_1_7);
> > -
> > -GlobalProperty pc_compat_1_6[] = {
> > -PC_CPU_MODEL_IDS("1.6.0")
> > -{ "e1000", "mitigation", "off" },
> > -{ "qemu64-" TYPE_X86_CPU, "model", "2" },
> > -{ "qemu32-" TYPE_X86_CPU, "model", "3" },
> > -{ "i440FX-pcihost", "short_root_bus", "1" },
> > -{ "q35-pcihost", "short_root_bus", "1" },
> > -};
> > -const size_t pc_compat_1_6_len = G_N_ELEMENTS(pc_compat_1_6);
> > -
> > -GlobalProperty pc_compat_1_5[] = {
> > -PC_CPU_MODEL_IDS("1.5.0")
> > -{ "Conroe-" TYPE_X86_CPU, "model", "2" },
> > -{ "Conroe-" TYPE_X86_CPU, "min-level", "2" },
> > -{ "Penryn-" TYPE_X86_CPU, "model", "2" },
> > -{ "Penryn-" TYPE_X86_CPU, "min-level", "2" },
> > -{ "Nehalem-" TYPE_X86_CPU, "model", "2" },
> > -{ "Nehalem-" TYPE_X86_CPU, "min-level", "2" },
> > -{ "virtio-net-pci", "any_layout", "off" },
> > -{ TYPE_X86_CPU, "pmu", "on" },
> > -{ "i440FX-pcihost", "short_root_bus", "0" },
> > -{ "q35-pcihost", "short_root_bus", "0" },
> > -};
> > -const size_t pc_compat_1_5_len = G_N_ELEMENTS(pc_compat_1_5);
> > -
> > -GlobalProperty pc_compat_1_4[] = {
> > -PC_CPU_MODEL_IDS("1.4.0")
> > -{ "scsi-hd", "discard_granularity", "0" },
> > -{ "scsi-cd", "discard_granularity", "0" },
> > -{ "ide-hd", "discard_granularity", "0" },
> > -{ "ide-cd", "discard_granularity", "0" },
> > -{ "virtio-blk-pci", "discard_granularity", "0" },
> > -/* DEV_NVECTORS_UNSPECIFIED as a uint32_t string: */
> > -{ "virtio-serial-pci", "vectors", "0x" },
> > -{ "virtio-net-pci", "ctrl_guest_offloads", "off" },
> > -{ "e1000", "romfile", "pxe-e1000.rom" },
> > -{ "ne2k_pci", "romfile", "pxe-ne2k_pci.rom" },
> > -{ "pcnet", "romfile", "pxe-pcnet.rom" },
> > -{ "rtl8139", "romfile", "pxe-rtl8139.rom" },
> > -{ "virtio-net-pci", "romfile", "pxe-virtio.rom" },
> > -{ "486-" TYPE_X86_CPU, "model", "0" },
> > -{ "n270" "-" TYPE_X86_CPU, "movbe", "off" },
> > -{ "Westmere" "-" TYPE_X86_CPU, "pclmulqdq", "off" },
> > -};
> > -const size_t pc_compat_1_4_len = G_N_ELEMENTS(pc_compat_1_4);
>
> It might be worth to have a closer look at the above settings in the
> various
> devices - maybe we can get rid of some compatibility handling code in the
> devices now, in case the properties are not set by other targets as well.
>
>   Thomas
>
>

Re: [PATCH 2/2] seabios: remove PCI drivers from bios.bin

[Paolo Bonzini]

Il ven 22 set 2023, 08:49 Thomas Huth  ha scritto:

> On 21/09/2023 14.10, Paolo Bonzini wrote:
> > bios.bin is now used only by ISA PC, so PCI drivers are not necessary.
> >
> > Signed-off-by: Paolo Bonzini 
> > ---
> >   pc-bios/bios.bin | Bin 131072 -> 131072 bytes
> >   roms/config.seabios-128k |  30 ++
> >   2 files changed, 22 insertions(+), 8 deletions(-)
> ...
> > diff --git a/roms/config.seabios-128k b/roms/config.seabios-128k
> > index d18c802c46e..06f4ba35bbe 100644
> > --- a/roms/config.seabios-128k
> > +++ b/roms/config.seabios-128k
> > @@ -1,21 +1,35 @@
> > -# for qemu machine types 1.7 + older
> > -# need to turn off features (xhci,uas) to make it fit into 128k
> > +# SeaBIOS Configuration for -M isapc
> > +
> > +#
> > +# General Features
> > +#
> >   CONFIG_QEMU=y
> >   CONFIG_ROM_SIZE=128
> >   CONFIG_ATA_DMA=n
> >   CONFIG_BOOTSPLASH=n
> >   CONFIG_XEN=n
> > -CONFIG_USB_OHCI=n
> > -CONFIG_USB_XHCI=n
> > -CONFIG_USB_UAS=n
> > +CONFIG_ATA_PIO32=n
> > +CONFIG_AHCI=n
> >   CONFIG_SDCARD=n
> >   CONFIG_TCGBIOS=n
> > -CONFIG_MPT_SCSI=n
> > -CONFIG_ESP_SCSI=n
> > -CONFIG_MEGASAS=n
> > +CONFIG_VIRTIO_BLK=n
> > +CONFIG_VIRTIO_SCSI=n
> >   CONFIG_PVSCSI=n
> > +CONFIG_ESP_SCSI=n
> > +CONFIG_LSI_SCSI=n
> > +CONFIG_MEGASAS=n
> > +CONFIG_MPT_SCSI=n
>
> Why did you change the order of MPT, ESP and MEGASAS?
>

Because the previous .config was written by hand while this one is
generated by "make menu config" (other than the comments at the top).

Paolo


> Apart from that, wrt to the config file changes:
> Reviewed-by: Thomas Huth 
>
> >   CONFIG_NVME=n
> >   CONFIG_USE_SMM=n
> >   CONFIG_VGAHOOKS=n
> >   CONFIG_HOST_BIOS_GEOMETRY=n
> > +CONFIG_PS2PORT=n
> > +CONFIG_USB=n
> > +CONFIG_PMTIMER=n
> > +CONFIG_PCIBIOS=n
> > +CONFIG_DISABLE_A20=n
> > +CONFIG_WRITABLE_UPPERMEMORY=n
> > +CONFIG_ACPI=n
> >   CONFIG_ACPI_PARSE=n
> > +CONFIG_DEBUG_SERIAL=n
> > +CONFIG_DEBUG_SERIAL_MMIO=n
>
>

[PULL 03/12] qemu/timer: Add host ticks function for RISC-V

[Paolo Bonzini]

From: LIU Zhiwei 

Signed-off-by: LIU Zhiwei 
Message-ID: <20230911063223.742-1-zhiwei_...@linux.alibaba.com>
Signed-off-by: Paolo Bonzini 
---
 include/qemu/timer.h | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/include/qemu/timer.h b/include/qemu/timer.h
index 9a91cb1248a..9a366e551fb 100644
--- a/include/qemu/timer.h
+++ b/include/qemu/timer.h
@@ -979,6 +979,28 @@ static inline int64_t cpu_get_host_ticks(void)
 return cur - ofs;
 }
 
+#elif defined(__riscv) && __riscv_xlen == 32
+static inline int64_t cpu_get_host_ticks(void)
+{
+uint32_t lo, hi, tmph;
+do {
+asm volatile("RDTIMEH %0\n\t"
+ "RDTIME %1\n\t"
+ "RDTIMEH %2"
+ : "=r"(hi), "=r"(lo), "=r"(tmph));
+} while (unlikely(tmph != hi));
+return lo | (uint64_t)hi << 32;
+}
+
+#elif defined(__riscv) && __riscv_xlen > 32
+static inline int64_t cpu_get_host_ticks(void)
+{
+int64_t val;
+
+asm volatile("RDTIME %0" : "=r"(val));
+return val;
+}
+
 #else
 /* The host CPU doesn't have an easily accessible cycle counter.
Just return a monotonically increasing value.  This will be
-- 
2.41.0

[PULL 00/12] Audio, i386 patches for 2022-09-22

[Paolo Bonzini]

The following changes since commit 005ad32358f12fe9313a4a01918a55e60d4f39e5:

  Merge tag 'pull-tpm-2023-09-12-3' of https://github.com/stefanberger/qemu-tpm 
into staging (2023-09-13 13:41:57 -0400)

are available in the Git repository at:

  https://gitlab.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 5930ce1943065b3d7b2687da0b53c1cd1da86cf7:

  audio: Require AudioState in AUD_add_capture (2023-09-21 10:42:43 +0200)


* add host ticks function for RISC-V
* target/i386: Export GDS_NO bit
* target/i386: add support for bit 56 of MSR_IA32_VMX_BASIC
* drop -audiodev-help and QEMU_AUDIO_* environment variables
* require -audiodev for VNC audio


LIU Zhiwei (1):
  qemu/timer: Add host ticks function for RISC-V

Martin Kletzander (8):
  hw/input/tsc210x: Extract common init code into new function
  hw/audio: Simplify hda audio init
  hw/audio/lm4549: Add errp error reporting to init function
  hw/display/xlnx_dp.c: Add audiodev property
  tests/qtest: Specify audiodev= and -audiodev
  audio/spiceaudio: Fail initialisation when not using spice
  ui/vnc: Require audiodev= to enable audio
  audio: Require AudioState in AUD_add_capture

Paolo Bonzini (2):
  target/i386: enumerate bit 56 of MSR_IA32_VMX_BASIC
  audio: remove QEMU_AUDIO_* and -audio-help support

Pawan Gupta (1):
  target/i386: Export GDS_NO bit to guests

 audio/alsaaudio.c   |   1 -
 audio/audio.c   |  63 ++--
 audio/audio.h   |   1 -
 audio/audio_int.h   |   3 -
 audio/audio_legacy.c| 591 
 audio/coreaudio.m   |   1 -
 audio/dbusaudio.c   |   1 -
 audio/dsoundaudio.c |   1 -
 audio/jackaudio.c   |   1 -
 audio/meson.build   |   1 -
 audio/noaudio.c |   1 -
 audio/ossaudio.c|   1 -
 audio/paaudio.c |   1 -
 audio/pwaudio.c |   1 -
 audio/sdlaudio.c|   1 -
 audio/sndioaudio.c  |   1 -
 audio/spiceaudio.c  |   3 +-
 audio/wavaudio.c|   1 -
 docs/about/deprecated.rst   |  16 +-
 docs/about/removed-features.rst |  12 +
 hw/audio/hda-codec.c|  32 +-
 hw/audio/intel-hda.c|   4 +-
 hw/audio/intel-hda.h|   2 +-
 hw/audio/lm4549.c   |   3 +-
 hw/audio/lm4549.h   |   3 +-
 hw/audio/pl041.c|   2 +-
 hw/display/xlnx_dp.c|   6 +
 hw/input/tsc210x.c  |  68 ++--
 include/qemu/timer.h|  22 ++
 qemu-options.hx |  10 -
 scripts/kvm/vmxcap  |   1 +
 softmmu/vl.c|   4 -
 target/i386/cpu.c   |   3 +-
 target/i386/cpu.h   |   1 +
 tests/qtest/es1370-test.c   |   3 +-
 tests/qtest/fuzz/generic_fuzz_configs.h |   6 +-
 tests/qtest/intel-hda-test.c|  15 +-
 ui/vnc.c|  10 +-
 38 files changed, 142 insertions(+), 755 deletions(-)
 delete mode 100644 audio/audio_legacy.c
-- 
2.41.0

[PULL 01/12] target/i386: enumerate bit 56 of MSR_IA32_VMX_BASIC

[Paolo Bonzini]

On parts that enumerate IA32_VMX_BASIC MSR bit as 1, any exception vector
can be delivered with or without an error code if the other consistency
checks are satisfied.

Signed-off-by: Paolo Bonzini 
---
 scripts/kvm/vmxcap | 1 +
 target/i386/cpu.c  | 1 +
 target/i386/cpu.h  | 1 +
 3 files changed, 3 insertions(+)

diff --git a/scripts/kvm/vmxcap b/scripts/kvm/vmxcap
index ce27f5e635a..3fb4d5b3425 100755
--- a/scripts/kvm/vmxcap
+++ b/scripts/kvm/vmxcap
@@ -115,6 +115,7 @@ controls = [
 (50, 53): 'VMCS memory type',
 54: 'INS/OUTS instruction information',
 55: 'IA32_VMX_TRUE_*_CTLS support',
+56: 'Skip checks on event error code',
 },
 msr = MSR_IA32_VMX_BASIC,
 ),
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b2a20365e10..d48607b4e1e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1340,6 +1340,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 .feat_names = {
 [54] = "vmx-ins-outs",
 [55] = "vmx-true-ctls",
+[56] = "vmx-any-errcode",
 },
 .msr = {
 .index = MSR_IA32_VMX_BASIC,
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index fbb05eace57..d1ffadd78be 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1039,6 +1039,7 @@ uint64_t x86_cpu_get_supported_feature_word(FeatureWord w,
 #define MSR_VMX_BASIC_DUAL_MONITOR   (1ULL << 49)
 #define MSR_VMX_BASIC_INS_OUTS   (1ULL << 54)
 #define MSR_VMX_BASIC_TRUE_CTLS  (1ULL << 55)
+#define MSR_VMX_BASIC_ANY_ERRCODE(1ULL << 56)
 
 #define MSR_VMX_MISC_PREEMPTION_TIMER_SHIFT_MASK 0x1Full
 #define MSR_VMX_MISC_STORE_LMA   (1ULL << 5)
-- 
2.41.0

[PULL 02/12] target/i386: Export GDS_NO bit to guests

[Paolo Bonzini]

From: Pawan Gupta 

Gather Data Sampling (GDS) is a side-channel attack using Gather
instructions. Some Intel processors will set ARCH_CAP_GDS_NO bit in
MSR IA32_ARCH_CAPABILITIES to report that they are not vulnerable to
GDS.

Make this bit available to guests.

Closes: 
https://lore.kernel.org/qemu-devel/camgffemg6tnq0n3+4ojagxc8j0oevy60khzekxcbs3lok9v...@mail.gmail.com/
Reported-by: Jack Wang 
Signed-off-by: Pawan Gupta 
Tested-by: Jack Wang 
Tested-by: Daniel Sneddon 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
 target/i386/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index d48607b4e1e..f9e51a9d87e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1155,7 +1155,7 @@ FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
 NULL, "sbdr-ssdp-no", "fbsdp-no", "psdp-no",
 NULL, "fb-clear", NULL, NULL,
 NULL, NULL, NULL, NULL,
-"pbrsb-no", NULL, NULL, NULL,
+"pbrsb-no", NULL, "gds-no", NULL,
 NULL, NULL, NULL, NULL,
 },
 .msr = {
-- 
2.41.0

[PULL 05/12] hw/input/tsc210x: Extract common init code into new function

[Paolo Bonzini]

From: Martin Kletzander 

This deduplicates several lines and will make future changes more
concise.

Signed-off-by: Martin Kletzander 
Reviewed-by: Daniel P. Berrangé 
Message-ID: 
<1d75877cf4cc2a38f87633ff16f9fea3e1bb0c03.1650874791.git.mklet...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 hw/input/tsc210x.c | 68 --
 1 file changed, 24 insertions(+), 44 deletions(-)

diff --git a/hw/input/tsc210x.c b/hw/input/tsc210x.c
index 7eae5989f76..f568759e05a 100644
--- a/hw/input/tsc210x.c
+++ b/hw/input/tsc210x.c
@@ -30,6 +30,7 @@
 #include "hw/input/tsc2xxx.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
+#include "qapi/error.h"
 
 #define TSC_DATA_REGISTERS_PAGE0x0
 #define TSC_CONTROL_REGISTERS_PAGE 0x1
@@ -1069,20 +1070,10 @@ static const VMStateDescription vmstate_tsc2301 = {
 .fields = vmstatefields_tsc210x,
 };
 
-uWireSlave *tsc2102_init(qemu_irq pint)
+static void tsc210x_init(TSC210xState *s,
+ const char *name,
+ const VMStateDescription *vmsd)
 {
-TSC210xState *s;
-
-s = g_new0(TSC210xState, 1);
-s->x = 160;
-s->y = 160;
-s->pressure = 0;
-s->precision = s->nextprecision = 0;
-s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tsc210x_timer_tick, s);
-s->pint = pint;
-s->model = 0x2102;
-s->name = "tsc2102";
-
 s->tr[0] = 0;
 s->tr[1] = 1;
 s->tr[2] = 1;
@@ -1104,13 +1095,29 @@ uWireSlave *tsc2102_init(qemu_irq pint)
 
 tsc210x_reset(s);
 
-qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1,
-"QEMU TSC2102-driven Touchscreen");
+qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1, name);
 
 AUD_register_card(s->name, &s->card);
 
 qemu_register_reset((void *) tsc210x_reset, s);
-vmstate_register(NULL, 0, &vmstate_tsc2102, s);
+vmstate_register(NULL, 0, vmsd, s);
+}
+
+uWireSlave *tsc2102_init(qemu_irq pint)
+{
+TSC210xState *s;
+
+s = g_new0(TSC210xState, 1);
+s->x = 160;
+s->y = 160;
+s->pressure = 0;
+s->precision = s->nextprecision = 0;
+s->timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, tsc210x_timer_tick, s);
+s->pint = pint;
+s->model = 0x2102;
+s->name = "tsc2102";
+
+tsc210x_init(s, "QEMU TSC2102-driven Touchscreen", &vmstate_tsc2102);
 
 return &s->chip;
 }
@@ -1131,34 +1138,7 @@ uWireSlave *tsc2301_init(qemu_irq penirq, qemu_irq 
kbirq, qemu_irq dav)
 s->model = 0x2301;
 s->name = "tsc2301";
 
-s->tr[0] = 0;
-s->tr[1] = 1;
-s->tr[2] = 1;
-s->tr[3] = 0;
-s->tr[4] = 1;
-s->tr[5] = 0;
-s->tr[6] = 1;
-s->tr[7] = 0;
-
-s->chip.opaque = s;
-s->chip.send = (void *) tsc210x_write;
-s->chip.receive = (void *) tsc210x_read;
-
-s->codec.opaque = s;
-s->codec.tx_swallow = (void *) tsc210x_i2s_swallow;
-s->codec.set_rate = (void *) tsc210x_i2s_set_rate;
-s->codec.in.fifo = s->in_fifo;
-s->codec.out.fifo = s->out_fifo;
-
-tsc210x_reset(s);
-
-qemu_add_mouse_event_handler(tsc210x_touchscreen_event, s, 1,
-"QEMU TSC2301-driven Touchscreen");
-
-AUD_register_card(s->name, &s->card);
-
-qemu_register_reset((void *) tsc210x_reset, s);
-vmstate_register(NULL, 0, &vmstate_tsc2301, s);
+tsc210x_init(s, "QEMU TSC2301-driven Touchscreen", &vmstate_tsc2301);
 
 return &s->chip;
 }
-- 
2.41.0

[PULL 12/12] audio: Require AudioState in AUD_add_capture

[Paolo Bonzini]

From: Martin Kletzander 

Since all callers require a valid audiodev this function can now safely
abort in case of missing AudioState.

Signed-off-by: Martin Kletzander 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
 audio/audio.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 666ef4237dd..d72e7db7fb9 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1854,8 +1854,9 @@ CaptureVoiceOut *AUD_add_capture(
 struct capture_callback *cb;
 
 if (!s) {
-dolog("Capturing without setting an audiodev is deprecated\n");
-s = audio_init(NULL, NULL);
+error_setg(&error_abort,
+   "Capturing without setting an audiodev is not supported");
+abort();
 }
 
 if (!audio_get_pdo_out(s->dev)->mixing_engine) {
-- 
2.41.0

[PULL 07/12] hw/audio/lm4549: Add errp error reporting to init function

[Paolo Bonzini]

From: Martin Kletzander 

This will be used in future commit.

Signed-off-by: Martin Kletzander 
Reviewed-by: Daniel P. Berrangé 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
 hw/audio/lm4549.c | 3 ++-
 hw/audio/lm4549.h | 3 ++-
 hw/audio/pl041.c  | 2 +-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/audio/lm4549.c b/hw/audio/lm4549.c
index 32b1481b561..418041bc9c6 100644
--- a/hw/audio/lm4549.c
+++ b/hw/audio/lm4549.c
@@ -276,7 +276,8 @@ static int lm4549_post_load(void *opaque, int version_id)
 return 0;
 }
 
-void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque)
+void lm4549_init(lm4549_state *s, lm4549_callback data_req_cb, void* opaque,
+ Error **errp)
 {
 struct audsettings as;
 
diff --git a/hw/audio/lm4549.h b/hw/audio/lm4549.h
index aba9bb5b077..61c3ab12dd3 100644
--- a/hw/audio/lm4549.h
+++ b/hw/audio/lm4549.h
@@ -36,7 +36,8 @@ typedef struct {
 extern const VMStateDescription vmstate_lm4549_state;
 
 
-void lm4549_init(lm4549_state *s, lm4549_callback data_req, void *opaque);
+void lm4549_init(lm4549_state *s, lm4549_callback data_req, void *opaque,
+ Error **errp);
 uint32_t lm4549_read(lm4549_state *s, hwaddr offset);
 void lm4549_write(lm4549_state *s, hwaddr offset, uint32_t value);
 uint32_t lm4549_write_samples(lm4549_state *s, uint32_t left, uint32_t right);
diff --git a/hw/audio/pl041.c b/hw/audio/pl041.c
index 03acd4fe344..868dffbfd32 100644
--- a/hw/audio/pl041.c
+++ b/hw/audio/pl041.c
@@ -564,7 +564,7 @@ static void pl041_realize(DeviceState *dev, Error **errp)
 }
 
 /* Init the codec */
-lm4549_init(&s->codec, &pl041_request_data, (void *)s);
+lm4549_init(&s->codec, &pl041_request_data, (void *)s, errp);
 }
 
 static const VMStateDescription vmstate_pl041_regfile = {
-- 
2.41.0

[PULL 09/12] tests/qtest: Specify audiodev= and -audiodev

[Paolo Bonzini]

From: Martin Kletzander 

This will enable removing deprecated default audiodev support.

I did not figure out how to make the audiodev represented as an
interface node, so this is a workaround.  I am not sure what would be
the proper way.

Signed-off-by: Martin Kletzander 
Reviewed-by: Daniel P. Berrangé 
Message-ID: 
<6e7f2808dd40679a415812767b88f2a411fc137f.1650874791.git.mklet...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 tests/qtest/es1370-test.c   |  3 ++-
 tests/qtest/fuzz/generic_fuzz_configs.h |  6 --
 tests/qtest/intel-hda-test.c| 15 ++-
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/tests/qtest/es1370-test.c b/tests/qtest/es1370-test.c
index 97ab65c4357..8387e74193b 100644
--- a/tests/qtest/es1370-test.c
+++ b/tests/qtest/es1370-test.c
@@ -46,7 +46,8 @@ static void *es1370_create(void *pci_bus, QGuestAllocator 
*alloc, void *addr)
 static void es1370_register_nodes(void)
 {
 QOSGraphEdgeOptions opts = {
-.extra_device_opts = "addr=04.0",
+.extra_device_opts = "addr=04.0,audiodev=audio0",
+.before_cmd_line = "-audiodev driver=none,id=audio0",
 };
 add_qpci_address(&opts, &(QPCIAddress) { .devfn = QPCI_DEVFN(4, 0) });
 
diff --git a/tests/qtest/fuzz/generic_fuzz_configs.h 
b/tests/qtest/fuzz/generic_fuzz_configs.h
index 50689da6539..4d7c8ca4ece 100644
--- a/tests/qtest/fuzz/generic_fuzz_configs.h
+++ b/tests/qtest/fuzz/generic_fuzz_configs.h
@@ -106,8 +106,10 @@ const generic_fuzz_config predefined_configs[] = {
 },{
 .name = "intel-hda",
 .args = "-machine q35 -nodefaults -device intel-hda,id=hda0 "
-"-device hda-output,bus=hda0.0 -device hda-micro,bus=hda0.0 "
-"-device hda-duplex,bus=hda0.0",
+"-audiodev driver=none,id=audio0",
+"-device hda-output,bus=hda0.0,audiodev=audio0 "
+"-device hda-micro,bus=hda0.0,audiodev=audio0 "
+"-device hda-duplex,bus=hda0.0,audiodev=audio0",
 .objects = "intel-hda",
 },{
 .name = "ide-hd",
diff --git a/tests/qtest/intel-hda-test.c b/tests/qtest/intel-hda-test.c
index d4a8db6fd60..663bb6c4854 100644
--- a/tests/qtest/intel-hda-test.c
+++ b/tests/qtest/intel-hda-test.c
@@ -11,20 +11,24 @@
 #include "libqtest-single.h"
 
 #define HDA_ID "hda0"
-#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0" \
-  " -device hda-micro,bus=" HDA_ID ".0" \
-  " -device hda-duplex,bus=" HDA_ID ".0"
+#define AUDIODEV " -audiodev driver=none,id=audio0 "
+#define AUDIODEV_REF "audiodev=audio0"
+#define CODEC_DEVICES " -device hda-output,bus=" HDA_ID ".0," AUDIODEV_REF \
+  " -device hda-micro,bus=" HDA_ID ".0," AUDIODEV_REF \
+  " -device hda-duplex,bus=" HDA_ID ".0," AUDIODEV_REF
 
 /* Tests only initialization so far. TODO: Replace with functional tests */
 static void ich6_test(void)
 {
-qtest_start("-machine pc -device intel-hda,id=" HDA_ID CODEC_DEVICES);
+qtest_start(AUDIODEV "-machine pc -device intel-hda,id=" HDA_ID 
CODEC_DEVICES);
 qtest_end();
 }
 
 static void ich9_test(void)
 {
-qtest_start("-machine q35 -device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
+qtest_start("-machine q35"
+AUDIODEV
+"-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id="
 HDA_ID CODEC_DEVICES);
 qtest_end();
 }
@@ -39,6 +43,7 @@ static void test_issue542_ich6(void)
 QTestState *s;
 
 s = qtest_init("-nographic -nodefaults -M pc-q35-6.2 "
+   AUDIODEV
"-device intel-hda,id=" HDA_ID CODEC_DEVICES);
 
 qtest_outl(s, 0xcf8, 0x8804);
-- 
2.41.0

[PULL 11/12] ui/vnc: Require audiodev= to enable audio

[Paolo Bonzini]

From: Martin Kletzander 

Signed-off-by: Martin Kletzander 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
 docs/about/deprecated.rst   |  8 +++-
 docs/about/removed-features.rst |  6 ++
 ui/vnc.c| 10 --
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index b9cdd2dd03b..40845b3fcb7 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -37,13 +37,11 @@ coverage.
 System emulator command line arguments
 --
 
-Creating sound card devices and vnc without ``audiodev=`` property (since 4.2)
-''
+Creating sound card devices without ``audiodev=`` property (since 4.2)
+''
 
 When not using the deprecated legacy audio config, each sound card
-should specify an ``audiodev=`` property.  Additionally, when using
-vnc, you should specify an ``audiodev=`` property if you plan to
-transmit audio through the VNC protocol.
+should specify an ``audiodev=`` property.
 
 Short-form boolean options (since 6.0)
 ''
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 63458736ee1..7668ddef178 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -442,6 +442,12 @@ line using a ``secret`` object instance.
 The ``-audiodev`` and ``-audio`` command line options are now the only
 way to specify audio backend settings.
 
+Creating vnc without ``audiodev=`` property (removed in 8.2)
+
+
+When using vnc, you should specify an ``audiodev=`` property if
+you plan to transmit audio through the VNC protocol.
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/ui/vnc.c b/ui/vnc.c
index 6fd86996a54..cfa18bbd3e1 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2508,11 +2508,17 @@ static int protocol_client_msg(VncState *vs, uint8_t 
*data, size_t len)
 switch (read_u16 (data, 2)) {
 case VNC_MSG_CLIENT_QEMU_AUDIO_ENABLE:
 trace_vnc_msg_client_audio_enable(vs, vs->ioc);
-audio_add(vs);
+if (vs->vd->audio_state) {
+audio_add(vs);
+} else {
+error_report("audio not available, use audiodev= option 
for vnc");
+}
 break;
 case VNC_MSG_CLIENT_QEMU_AUDIO_DISABLE:
 trace_vnc_msg_client_audio_disable(vs, vs->ioc);
-audio_del(vs);
+if (vs->vd->audio_state) {
+audio_del(vs);
+}
 break;
 case VNC_MSG_CLIENT_QEMU_AUDIO_SET_FORMAT:
 if (len == 4)
-- 
2.41.0

[PULL 10/12] audio/spiceaudio: Fail initialisation when not using spice

[Paolo Bonzini]

From: Martin Kletzander 

The caller would already fail, but this way the message can better
express the reason for the failure.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2043498

Signed-off-by: Martin Kletzander 
Message-ID: 
<5db1fdef0330f20ed6ae306b5a71dad1b5e9b44c.1650874791.git.mklet...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 audio/spiceaudio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/audio/spiceaudio.c b/audio/spiceaudio.c
index d17ef1a25ef..af9bd7d54c9 100644
--- a/audio/spiceaudio.c
+++ b/audio/spiceaudio.c
@@ -74,8 +74,9 @@ static const SpiceRecordInterface record_sif = {
 static void *spice_audio_init(Audiodev *dev)
 {
 if (!using_spice) {
-return NULL;
+error_setg(&error_fatal, "Cannot use spice audio without -spice");
 }
+
 return &spice_audio_init;
 }
 
-- 
2.41.0

[PULL 08/12] hw/display/xlnx_dp.c: Add audiodev property

[Paolo Bonzini]

From: Martin Kletzander 

There was no way to set this and we need that for it to be able to properly
initialise.

Signed-off-by: Martin Kletzander 
Message-ID: 
<16963256573fcbfa7720aa2fd000ba74a4055222.1650874791.git.mklet...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 hw/display/xlnx_dp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/display/xlnx_dp.c b/hw/display/xlnx_dp.c
index 43c7dd8e9cd..341e91e886f 100644
--- a/hw/display/xlnx_dp.c
+++ b/hw/display/xlnx_dp.c
@@ -1385,6 +1385,11 @@ static void xlnx_dp_reset(DeviceState *dev)
 xlnx_dp_update_irq(s);
 }
 
+static Property xlnx_dp_device_properties[] = {
+DEFINE_AUDIO_PROPERTIES(XlnxDPState, aud_card),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void xlnx_dp_class_init(ObjectClass *oc, void *data)
 {
 DeviceClass *dc = DEVICE_CLASS(oc);
@@ -1392,6 +1397,7 @@ static void xlnx_dp_class_init(ObjectClass *oc, void 
*data)
 dc->realize = xlnx_dp_realize;
 dc->vmsd = &vmstate_dp;
 dc->reset = xlnx_dp_reset;
+device_class_set_props(dc, xlnx_dp_device_properties);
 }
 
 static const TypeInfo xlnx_dp_info = {
-- 
2.41.0

[PULL 04/12] audio: remove QEMU_AUDIO_* and -audio-help support

[Paolo Bonzini]

These have been deprecated for a long time, and the introduction
of -audio in 7.1.0 has cemented the new way of specifying an
audio backend.  Remove all the associated baggage, including the
concept of "can_be_default" audio drivers.

Signed-off-by: Paolo Bonzini 
---
 audio/alsaaudio.c   |   1 -
 audio/audio.c   |  60 +---
 audio/audio.h   |   1 -
 audio/audio_int.h   |   3 -
 audio/audio_legacy.c| 591 
 audio/coreaudio.m   |   1 -
 audio/dbusaudio.c   |   1 -
 audio/dsoundaudio.c |   1 -
 audio/jackaudio.c   |   1 -
 audio/meson.build   |   1 -
 audio/noaudio.c |   1 -
 audio/ossaudio.c|   1 -
 audio/paaudio.c |   1 -
 audio/pwaudio.c |   1 -
 audio/sdlaudio.c|   1 -
 audio/sndioaudio.c  |   1 -
 audio/wavaudio.c|   1 -
 docs/about/deprecated.rst   |   8 -
 docs/about/removed-features.rst |   6 +
 qemu-options.hx |  10 -
 softmmu/vl.c|   4 -
 21 files changed, 24 insertions(+), 672 deletions(-)
 delete mode 100644 audio/audio_legacy.c

diff --git a/audio/alsaaudio.c b/audio/alsaaudio.c
index 057571dd1e0..5ffb39c4182 100644
--- a/audio/alsaaudio.c
+++ b/audio/alsaaudio.c
@@ -960,7 +960,6 @@ static struct audio_driver alsa_audio_driver = {
 .init   = alsa_audio_init,
 .fini   = alsa_audio_fini,
 .pcm_ops= &alsa_pcm_ops,
-.can_be_default = 1,
 .max_voices_out = INT_MAX,
 .max_voices_in  = INT_MAX,
 .voice_size_out = sizeof (ALSAVoiceOut),
diff --git a/audio/audio.c b/audio/audio.c
index 90c7c49d116..666ef4237dd 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -73,7 +73,7 @@ void audio_driver_register(audio_driver *drv)
 QLIST_INSERT_HEAD(&audio_drivers, drv, next);
 }
 
-audio_driver *audio_driver_lookup(const char *name)
+static audio_driver *audio_driver_lookup(const char *name)
 {
 struct audio_driver *d;
 Error *local_err = NULL;
@@ -111,8 +111,6 @@ const struct mixeng_volume nominal_volume = {
 #endif
 };
 
-static bool legacy_config = true;
-
 int audio_bug (const char *funcname, int cond)
 {
 if (cond) {
@@ -1712,46 +1710,26 @@ static AudioState *audio_init(Audiodev *dev, const char 
*name)
 /* silence gcc warning about uninitialized variable */
 AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
 
-if (using_spice) {
-/*
- * When using spice allow the spice audio driver being picked
- * as default.
- *
- * Temporary hack.  Using audio devices without explicit
- * audiodev= property is already deprecated.  Same goes for
- * the -soundhw switch.  Once this support gets finally
- * removed we can also drop the concept of a default audio
- * backend and this can go away.
- */
-driver = audio_driver_lookup("spice");
-if (driver) {
-driver->can_be_default = 1;
-}
-}
-
 if (dev) {
 /* -audiodev option */
-legacy_config = false;
 drvname = AudiodevDriver_str(dev->driver);
-} else if (!QTAILQ_EMPTY(&audio_states)) {
-if (!legacy_config) {
-dolog("Device %s: audiodev default parameter is deprecated, please 
"
-  "specify audiodev=%s\n", name,
-  QTAILQ_FIRST(&audio_states)->dev->id);
-}
-return QTAILQ_FIRST(&audio_states);
 } else {
-/* legacy implicit initialization */
-head = audio_handle_legacy_opts();
-/*
- * In case of legacy initialization, all Audiodevs in the list will 
have
- * the same configuration (except the driver), so it doesn't matter 
which
- * one we chose.  We need an Audiodev to set up AudioState before we 
can
- * init a driver.  Also note that dev at this point is still in the
- * list.
- */
-dev = QSIMPLEQ_FIRST(&head)->dev;
-audio_validate_opts(dev, &error_abort);
+if (!QTAILQ_EMPTY(&audio_states)) {
+dev = QTAILQ_FIRST(&audio_states)->dev;
+if (!g_str_equal(dev->id, "#none")) {
+dolog("Device %s: audiodev default parameter is deprecated, 
please "
+  "specify audiodev=%s\n", name,
+  dev->id);
+}
+return QTAILQ_FIRST(&audio_states);
+}
+
+dolog("No audio device specified\n");
+dev = g_new0(Audiodev, 1);
+dev->id = g_strdup("#none");
+dev->driver = AUDIODEV_DRIVER_NONE;
+dev->u.none.in = g_new0(AudiodevPerDirectionOptions, 1);
+dev->u.none.out = g_new0(AudiodevPerDirectionOptions, 1);
 }
 
 s = g_new0(AudioState, 1);
@@ -1876,9 +1854,7 @@ CaptureVoiceOut *AUD_add_capture(
 struct capture_callback *cb;
 
 if (!s)

[PULL 06/12] hw/audio: Simplify hda audio init

[Paolo Bonzini]

From: Martin Kletzander 

No return values are used anywhere, so switch the functions to be void
and add support for error reporting using errp for use in next patches.

Signed-off-by: Martin Kletzander 
Reviewed-by: Daniel P. Berrangé 
Message-ID: 

Signed-off-by: Paolo Bonzini 
---
 hw/audio/hda-codec.c | 32 ++--
 hw/audio/intel-hda.c |  4 +---
 hw/audio/intel-hda.h |  2 +-
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/hw/audio/hda-codec.c b/hw/audio/hda-codec.c
index c51d8ba6177..a26048cf15e 100644
--- a/hw/audio/hda-codec.c
+++ b/hw/audio/hda-codec.c
@@ -675,7 +675,9 @@ static void hda_audio_stream(HDACodecDevice *hda, uint32_t 
stnr, bool running, b
 }
 }
 
-static int hda_audio_init(HDACodecDevice *hda, const struct desc_codec *desc)
+static void hda_audio_init(HDACodecDevice *hda,
+   const struct desc_codec *desc,
+   Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
 HDAAudioStream *st;
@@ -718,7 +720,6 @@ static int hda_audio_init(HDACodecDevice *hda, const struct 
desc_codec *desc)
 break;
 }
 }
-return 0;
 }
 
 static void hda_audio_exit(HDACodecDevice *hda)
@@ -848,37 +849,40 @@ static Property hda_audio_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
-static int hda_audio_init_output(HDACodecDevice *hda)
+static void hda_audio_init_output(HDACodecDevice *hda, Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
+const struct desc_codec *desc = &output_nomixemu;
 
 if (!a->mixer) {
-return hda_audio_init(hda, &output_nomixemu);
-} else {
-return hda_audio_init(hda, &output_mixemu);
+desc = &output_mixemu;
 }
+
+hda_audio_init(hda, desc, errp);
 }
 
-static int hda_audio_init_duplex(HDACodecDevice *hda)
+static void hda_audio_init_duplex(HDACodecDevice *hda, Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
+const struct desc_codec *desc = &duplex_nomixemu;
 
 if (!a->mixer) {
-return hda_audio_init(hda, &duplex_nomixemu);
-} else {
-return hda_audio_init(hda, &duplex_mixemu);
+desc = &duplex_mixemu;
 }
+
+hda_audio_init(hda, desc, errp);
 }
 
-static int hda_audio_init_micro(HDACodecDevice *hda)
+static void hda_audio_init_micro(HDACodecDevice *hda, Error **errp)
 {
 HDAAudioState *a = HDA_AUDIO(hda);
+const struct desc_codec *desc = µ_nomixemu;
 
 if (!a->mixer) {
-return hda_audio_init(hda, µ_nomixemu);
-} else {
-return hda_audio_init(hda, µ_mixemu);
+desc = µ_mixemu;
 }
+
+hda_audio_init(hda, desc, errp);
 }
 
 static void hda_audio_base_class_init(ObjectClass *klass, void *data)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index b9ed231fe84..78ff9f9a680 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -71,9 +71,7 @@ static void hda_codec_dev_realize(DeviceState *qdev, Error 
**errp)
 return;
 }
 bus->next_cad = dev->cad + 1;
-if (cdc->init(dev) != 0) {
-error_setg(errp, "HDA audio init failed");
-}
+cdc->init(dev, errp);
 }
 
 static void hda_codec_dev_unrealize(DeviceState *qdev)
diff --git a/hw/audio/intel-hda.h b/hw/audio/intel-hda.h
index f78c1833e34..8d710eee5d6 100644
--- a/hw/audio/intel-hda.h
+++ b/hw/audio/intel-hda.h
@@ -31,7 +31,7 @@ struct HDACodecBus {
 struct HDACodecDeviceClass {
 DeviceClass parent_class;
 
-int (*init)(HDACodecDevice *dev);
+void (*init)(HDACodecDevice *dev, Error **errp);
 void (*exit)(HDACodecDevice *dev);
 void (*command)(HDACodecDevice *dev, uint32_t nid, uint32_t data);
 void (*stream)(HDACodecDevice *dev, uint32_t stnr, bool running, bool 
output);
-- 
2.41.0

Re: [PATCH for-8.1] vfio/display: Fix missing update to set backing fields

[Cédric Le Goater]

On 9/13/23 21:18, Alex Williamson wrote:


Hi Gerd,

Some consultation would be appreciated on this thread to get this patch
out of limbo.  Is there a better solution that what I've proposed?


This does fix a regression reproducible on systems with an Intel Gen 8,
my T480 laptop for instance.

Tested-by: Cédric Le Goater 

Also, queuing it in vfio-next.

Thanks,

C.



AFAICT, we don't have position fields to indicate the dmabuf plane is
relative to some scanout, so I think it represents the entire display.
Thanks,

Alex

On Tue, 5 Sep 2023 09:09:07 -0600
Alex Williamson  wrote:


On Mon, 4 Sep 2023 21:00:53 +0400
Marc-André Lureau  wrote:


Hi

On Mon, Sep 4, 2023 at 6:11 PM Alex Williamson
 wrote:


On Mon, 4 Sep 2023 15:06:21 +0400
Marc-André Lureau  wrote:


Hi

On Thu, Aug 17, 2023 at 8:29 PM Kim, Dongwon  wrote:


Ok, this regression happened not just because of renaming. Originally
width and height were representing the size of whole surface that guest
shares while scanout width and height are for the each scanout. We
realized backing_width/height are more commonly used to specify the size
of the whole guest surface so put them in the place of width/height then
replaced scanout_width/height as well with normal width/height.

On 8/16/2023 3:31 PM, Philippe Mathieu-Daudé wrote:

On 16/8/23 23:55, Alex Williamson wrote:

The below referenced commit renames scanout_width/height to
backing_width/height, but also promotes these fields in various portions
of the egl interface.  Meanwhile vfio dmabuf support has never used the
previous scanout fields and is therefore missed in the update. This
results in a black screen when transitioning from ramfb to dmabuf
display
when using Intel vGPU with these features.


Referenced commit isn't trivial. Maybe because it is too late here.
I'd have tried to split it. Anyhow, too late (again).

Is vhost-user-gpu also affected? (see VHOST_USER_GPU_DMABUF_SCANOUT
in vhost_user_gpu_handle_display()).


Yeah, backing_width/height should be programmed with plane.width/height
as well in vhost_user_gpu_handle_display().

Link: https://lists.gnu.org/archive/html/qemu-devel/2023-08/msg02726.html

Fixes: 9ac06df8b684 ("virtio-gpu-udmabuf: correct naming of
QemuDmaBuf size properties")
Signed-off-by: Alex Williamson 
---

This fixes a regression in dmabuf/EGL support for Intel GVT-g and
potentially the mbochs mdev driver as well.  Once validated by those
that understand dmabuf/EGL integration, I'd welcome QEMU maintainers to
take this directly for v8.1 or queue it as soon as possible for v8.1.1.

   hw/vfio/display.c | 2 ++
   1 file changed, 2 insertions(+)

diff --git a/hw/vfio/display.c b/hw/vfio/display.c
index bec864f482f4..837d9e6a309e 100644
--- a/hw/vfio/display.c
+++ b/hw/vfio/display.c
@@ -243,6 +243,8 @@ static VFIODMABuf
*vfio_display_get_dmabuf(VFIOPCIDevice *vdev,
   dmabuf->dmabuf_id  = plane.dmabuf_id;
   dmabuf->buf.width  = plane.width;
   dmabuf->buf.height = plane.height;


One thing to note here is the normal width and height in the QemuDmaBuf
are of a scanout, which could be just a partial area of the guest plane
here. So we should not use those as normal width and height of the
QemuDmaBuf unless it is guaranteed the given guest surface (plane in
this case) is always of single display's.

https://lists.gnu.org/archive/html/qemu-devel/2021-09/msg04737.html


+dmabuf->buf.backing_width = plane.width;
+dmabuf->buf.backing_height = plane.height;
   dmabuf->buf.stride = plane.stride;
   dmabuf->buf.fourcc = plane.drm_format;
   dmabuf->buf.modifier = plane.drm_format_mod;




I agree with what Kim said. Alex, are you sending a new patch?


What would be different?

struct vfio_device_gfx_plane_info {
 __u32 argsz;
 __u32 flags;
#define VFIO_GFX_PLANE_TYPE_PROBE (1 << 0)
#define VFIO_GFX_PLANE_TYPE_DMABUF (1 << 1)
#define VFIO_GFX_PLANE_TYPE_REGION (1 << 2)
 /* in */
 __u32 drm_plane_type;   /* type of plane: DRM_PLANE_TYPE_* */
 /* out */
 __u32 drm_format;   /* drm format of plane */
 __u64 drm_format_mod;   /* tiled mode */
 __u32 width;/* width of plane */
 __u32 height;   /* height of plane */
 __u32 stride;   /* stride of plane */
 __u32 size; /* size of plane in bytes, align on page*/
 __u32 x_pos;/* horizontal position of cursor plane */
 __u32 y_pos;/* vertical position of cursor plane*/
 __u32 x_hot;/* horizontal position of cursor hotspot */
 __u32 y_hot;/* vertical position of cursor hotspot */
 union {
 __u32 region_index; /* region index */
 __u32 dmabuf_id;/* dma-buf id */
 };
};



Perhaps VFIO is missing extra infos to set the actual x/y/w/h
region(s) of the visible monitor(s). This could be an extra message. I
am not familiar with the kernel/driver side for this, perhaps it is
always guarant

Re: [PATCH 15/52] migration/rdma: Ditch useless numeric error codes in error messages

[Markus Armbruster]

"Zhijian Li (Fujitsu)"  writes:

> On 18/09/2023 22:41, Markus Armbruster wrote:
>> Several error messages include numeric error codes returned by failed
>> functions:
>> 
>> * ibv_poll_cq() returns an unspecified negative value.  Useless.
>> 
>> * rdma_accept and rmda_get_cm_event() return -1.  Useless.
>
>
> s/rmda_get_cm_event/rdma_get_cm_event

Sharp eyes!  Will fix.

> Otherwise,
> Reviewed-by: Li Zhijian 

Thanks!

[PATCH 8/9] audio: Make AUD_register_card fallible and require audiodev=

[Paolo Bonzini]

From: Martin Kletzander 

Now that all callers support error reporting with errp and all machine-default
devices use an explicit audiodev, this can be changed.  To make the detection
easier make AUD_register_card() return false on error.

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 audio/audio.c| 7 +--
 audio/audio.h| 2 +-
 hw/arm/omap2.c   | 2 +-
 hw/audio/ac97.c  | 6 +-
 hw/audio/adlib.c | 6 --
 hw/audio/cs4231a.c   | 6 --
 hw/audio/es1370.c| 5 -
 hw/audio/gus.c   | 6 --
 hw/audio/hda-codec.c | 5 -
 hw/audio/lm4549.c| 8 +---
 hw/audio/pcspk.c | 4 +---
 hw/audio/sb16.c  | 6 --
 hw/audio/via-ac97.c  | 6 --
 hw/audio/wm8750.c| 5 -
 hw/display/xlnx_dp.c | 6 --
 hw/input/tsc210x.c   | 2 +-
 hw/usb/dev-audio.c   | 5 -
 17 files changed, 59 insertions(+), 28 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 62047ea3069..4fd27309cf8 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -1825,15 +1825,18 @@ void audio_free_audiodev_list(AudiodevListHead *head)
 }
 }
 
-void AUD_register_card (const char *name, QEMUSoundCard *card)
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp)
 {
 if (!card->state) {
-card->state = audio_init(NULL, name);
+error_setg(errp, "No audiodev specified for %s", name);
+return false;
 }
 
 card->name = g_strdup (name);
 memset (&card->entries, 0, sizeof (card->entries));
 QLIST_INSERT_HEAD(&card->state->card_head, card, entries);
+
+return true;
 }
 
 void AUD_remove_card (QEMUSoundCard *card)
diff --git a/audio/audio.h b/audio/audio.h
index 81d39526625..44b7547caba 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -94,7 +94,7 @@ typedef struct QEMUAudioTimeStamp {
 void AUD_vlog (const char *cap, const char *fmt, va_list ap) G_GNUC_PRINTF(2, 
0);
 void AUD_log (const char *cap, const char *fmt, ...) G_GNUC_PRINTF(2, 3);
 
-void AUD_register_card (const char *name, QEMUSoundCard *card);
+bool AUD_register_card (const char *name, QEMUSoundCard *card, Error **errp);
 void AUD_remove_card (QEMUSoundCard *card);
 CaptureVoiceOut *AUD_add_capture(
 AudioState *s,
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index 8b65c0b8ad6..53c741bbb3b 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -617,7 +617,7 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
s->codec.card.name);
 }
 
-AUD_register_card("OMAP EAC", &s->codec.card);
+AUD_register_card("OMAP EAC", &s->codec.card, &error_abort);
 
 memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
   omap_l4_region_size(ta, 0));
diff --git a/hw/audio/ac97.c b/hw/audio/ac97.c
index c2a5ce062a1..6a7a2dc80c4 100644
--- a/hw/audio/ac97.c
+++ b/hw/audio/ac97.c
@@ -1273,6 +1273,10 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
 AC97LinkState *s = AC97(dev);
 uint8_t *c = s->dev.config;
 
+if (!AUD_register_card ("ac97", &s->card, errp)) {
+return;
+}
+
 /* TODO: no need to override */
 c[PCI_COMMAND] = 0x00;  /* pcicmd pci command rw, ro */
 c[PCI_COMMAND + 1] = 0x00;
@@ -1306,7 +1310,7 @@ static void ac97_realize(PCIDevice *dev, Error **errp)
   "ac97-nabm", 256);
 pci_register_bar(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nam);
 pci_register_bar(&s->dev, 1, PCI_BASE_ADDRESS_SPACE_IO, &s->io_nabm);
-AUD_register_card("ac97", &s->card);
+
 ac97_on_reset(DEVICE(s));
 }
 
diff --git a/hw/audio/adlib.c b/hw/audio/adlib.c
index 5f979b1487d..bd73806d83a 100644
--- a/hw/audio/adlib.c
+++ b/hw/audio/adlib.c
@@ -255,6 +255,10 @@ static void adlib_realizefn (DeviceState *dev, Error 
**errp)
 AdlibState *s = ADLIB(dev);
 struct audsettings as;
 
+if (!AUD_register_card ("adlib", &s->card, errp)) {
+return;
+}
+
 s->opl = OPLCreate (3579545, s->freq);
 if (!s->opl) {
 error_setg (errp, "OPLCreate %d failed", s->freq);
@@ -270,8 +274,6 @@ static void adlib_realizefn (DeviceState *dev, Error **errp)
 as.fmt = AUDIO_FORMAT_S16;
 as.endianness = AUDIO_HOST_ENDIANNESS;
 
-AUD_register_card ("adlib", &s->card);
-
 s->voice = AUD_open_out (
 &s->card,
 s->voice,
diff --git a/hw/audio/cs4231a.c b/hw/audio/cs4231a.c
index 5c6d6437320..3aa105748d3 100644
--- a/hw/audio/cs4231a.c
+++ b/hw/audio/cs4231a.c
@@ -678,13 +678,15 @@ static void cs4231a_realizefn (DeviceState *dev, Error 
**errp)
 return;
 }
 
+if (!AUD_register_card ("cs4231a", &s->card, errp)) {
+return;
+}
+
 s->pic = isa_bus_get_irq(bus, s->irq);
 k = ISADMA_GET_CLASS(s->isa_dma);
 k->register_channel(s->isa_dma, s->dma, cs_dma_read, s);
 
 isa_register_ioport (d, &s->ioports, s->port);
-
-AUD_register_card ("cs4231a", &s->card);
 }
 
 static Property cs4231a_properties[] = {
diff --

[PATCH 4/9] hw/arm: Support machine-default audiodev with fallback

[Paolo Bonzini]

From: Martin Kletzander 

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 hw/arm/integratorcp.c | 10 +-
 hw/arm/musicpal.c | 11 +--
 hw/arm/nseries.c  |  4 
 hw/arm/omap2.c|  8 
 hw/arm/palm.c |  2 ++
 hw/arm/realview.c | 11 +++
 hw/arm/spitz.c| 12 +---
 hw/arm/versatilepb.c  |  7 +++
 hw/arm/vexpress.c |  4 
 hw/arm/xlnx-zcu102.c  |  5 +
 hw/arm/z2.c   | 14 +-
 hw/input/tsc210x.c|  8 
 12 files changed, 89 insertions(+), 7 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index b109ece3ae0..8b4f8a1a36a 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -27,6 +27,7 @@
 #include "hw/irq.h"
 #include "hw/sd/sd.h"
 #include "qom/object.h"
+#include "audio/audio.h"
 
 #define TYPE_INTEGRATOR_CM "integrator_core"
 OBJECT_DECLARE_SIMPLE_TYPE(IntegratorCMState, INTEGRATOR_CM)
@@ -660,7 +661,12 @@ static void integratorcp_init(MachineState *machine)
&error_fatal);
 }
 
-sysbus_create_varargs("pl041", 0x1d00, pic[25], NULL);
+dev = qdev_new("pl041");
+qdev_prop_set_string(dev, "audiodev",
+ audio_maybe_init_dummy("integrator.defaudio"));
+sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0x1d00);
+sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[25]);
 
 if (nd_table[0].used)
 smc91c111_init(&nd_table[0], 0xc800, pic[27]);
@@ -678,6 +684,8 @@ static void integratorcp_machine_init(MachineClass *mc)
 mc->ignore_memory_transaction_failures = true;
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
 mc->default_ram_id = "integrator.ram";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
diff --git a/hw/arm/musicpal.c b/hw/arm/musicpal.c
index dc4e43e0ee2..05c3f621954 100644
--- a/hw/arm/musicpal.c
+++ b/hw/arm/musicpal.c
@@ -37,9 +37,9 @@
 #include "qemu/cutils.h"
 #include "qom/object.h"
 #include "hw/net/mv88w8618_eth.h"
+#include "audio/audio.h"
 #include "qemu/error-report.h"
 
-
 #define MP_MISC_BASE0x80002000
 #define MP_MISC_SIZE0x1000
 
@@ -1326,7 +1326,12 @@ static void musicpal_init(MachineState *machine)
 qdev_connect_gpio_out(key_dev, i, qdev_get_gpio_in(dev, i + 15));
 }
 
-wm8750_dev = i2c_slave_create_simple(i2c, TYPE_WM8750, MP_WM_ADDR);
+wm8750_dev = i2c_slave_new(TYPE_WM8750, MP_WM_ADDR);
+
+qdev_prop_set_string(DEVICE(wm8750_dev), "audiodev",
+ audio_maybe_init_dummy("musicpal.defaudio"));
+i2c_slave_realize_and_unref(wm8750_dev, i2c, &error_abort);
+
 dev = qdev_new(TYPE_MV88W8618_AUDIO);
 s = SYS_BUS_DEVICE(dev);
 object_property_set_link(OBJECT(dev), "wm8750", OBJECT(wm8750_dev),
@@ -1347,6 +1352,8 @@ static void musicpal_machine_init(MachineClass *mc)
 mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
 mc->default_ram_size = MP_RAM_DEFAULT_SIZE;
 mc->default_ram_id = "musicpal.ram";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("musicpal", musicpal_machine_init)
diff --git a/hw/arm/nseries.c b/hw/arm/nseries.c
index 9e49e9e1776..35aff46b4b4 100644
--- a/hw/arm/nseries.c
+++ b/hw/arm/nseries.c
@@ -1432,6 +1432,8 @@ static void n800_class_init(ObjectClass *oc, void *data)
 /* Actually two chips of 0x400 bytes each */
 mc->default_ram_size = 0x0800;
 mc->default_ram_id = "omap2.dram";
+
+machine_add_audiodev_property(mc);
 }
 
 static const TypeInfo n800_type = {
@@ -1452,6 +1454,8 @@ static void n810_class_init(ObjectClass *oc, void *data)
 /* Actually two chips of 0x400 bytes each */
 mc->default_ram_size = 0x0800;
 mc->default_ram_id = "omap2.dram";
+
+machine_add_audiodev_property(mc);
 }
 
 static const TypeInfo n810_type = {
diff --git a/hw/arm/omap2.c b/hw/arm/omap2.c
index d5a2ae7af6e..8b65c0b8ad6 100644
--- a/hw/arm/omap2.c
+++ b/hw/arm/omap2.c
@@ -603,12 +603,20 @@ static struct omap_eac_s *omap_eac_init(struct 
omap_target_agent_s *ta,
 qemu_irq irq, qemu_irq *drq, omap_clk fclk, omap_clk iclk)
 {
 struct omap_eac_s *s = g_new0(struct omap_eac_s, 1);
+const char *audiodev_id = audio_maybe_init_dummy("eac.defaudio");
 
 s->irq = irq;
 s->codec.rxdrq = *drq ++;
 s->codec.txdrq = *drq;
 omap_eac_reset(s);
 
+s->codec.card.name = g_strdup(audiodev_id);
+s->codec.card.state = audio_state_by_name(s->codec.card.name);
+if (!s->codec.card.state) {
+error_setg(&error_fatal, "Cannot find audiodev with id '%s'",
+   s->codec.card.name);
+}
+
 AUD_register_card("OMAP EAC", &s->codec.card);
 
 memory_region_init_io(&s->iomem, NULL, &omap_eac_ops, s, "omap.eac",
diff --git a/hw/arm/palm.c b/hw/arm/palm.c
index 17c11ac4cec..b86f2c331bb 100644

[PATCH 2/9] Introduce machine property "audiodev"

[Paolo Bonzini]

From: Martin Kletzander 

Many machine types have default audio devices with no way to set the underlying
audiodev.  Instead of adding an option for each and every one of them, this new
property can be used as a default during machine initialisation when creating
such devices.

Signed-off-by: Martin Kletzander 
[Make the property optional, instead of including it in all machines. - Paolo]
Signed-off-by: Paolo Bonzini 
---
 audio/audio.c   |  4 ++--
 hw/core/machine.c   | 28 
 include/hw/boards.h |  7 +++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 8c74bc6b88c..62047ea3069 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2194,8 +2194,8 @@ const char *audio_maybe_init_dummy(const char *default_id)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
 
-if (ms->default_audiodev) {
-return ms->default_audiodev;
+if (ms->audiodev) {
+return ms->audiodev;
 }
 
 dolog("warning: No audiodev specified for implicit machine devices, "
diff --git a/hw/core/machine.c b/hw/core/machine.c
index da699cf4e14..f677e96a223 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -682,6 +682,22 @@ bool device_type_is_dynamic_sysbus(MachineClass *mc, const 
char *type)
 return allowed;
 }
 
+static char *machine_get_audiodev(Object *obj, Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+return g_strdup(ms->audiodev);
+}
+
+static void machine_set_audiodev(Object *obj, const char *value,
+ Error **errp)
+{
+MachineState *ms = MACHINE(obj);
+
+g_free(ms->audiodev);
+ms->audiodev = g_strdup(value);
+}
+
 HotpluggableCPUList *machine_query_hotpluggable_cpus(MachineState *machine)
 {
 int i;
@@ -927,6 +943,17 @@ out_free:
 qapi_free_BootConfiguration(config);
 }
 
+void machine_add_audiodev_property(MachineClass *mc)
+{
+ObjectClass *oc = OBJECT_CLASS(mc);
+
+object_class_property_add_str(oc, "audiodev",
+  machine_get_audiodev,
+  machine_set_audiodev);
+object_class_property_set_description(oc, "audiodev",
+  "Audiodev to use for default machine 
devices");
+}
+
 static void machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -1132,6 +1159,7 @@ static void machine_finalize(Object *obj)
 g_free(ms->device_memory);
 g_free(ms->nvdimms_state);
 g_free(ms->numa_state);
+g_free(ms->audiodev);
 }
 
 bool machine_usb(MachineState *machine)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 6c67af196a3..b5153f5f85b 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -24,6 +24,7 @@ OBJECT_DECLARE_TYPE(MachineState, MachineClass, MACHINE)
 
 extern MachineState *current_machine;
 
+void machine_add_audiodev_property(MachineClass *mc);
 void machine_run_board_init(MachineState *machine, const char *mem_path, Error 
**errp);
 bool machine_usb(MachineState *machine);
 int machine_phandle_start(MachineState *machine);
@@ -358,6 +359,12 @@ struct MachineState {
 MemoryRegion *ram;
 DeviceMemoryState *device_memory;
 
+/*
+ * Included in MachineState for simplicity, but not supported
+ * unless machine_add_audiodev_property is called.
+ */
+char *audiodev;
+
 ram_addr_t ram_size;
 ram_addr_t maxram_size;
 uint64_t   ram_slots;
-- 
2.41.0

[PATCH 1/9] audio: Add easy dummy audio initialiser

[Paolo Bonzini]

From: Martin Kletzander 

In case of sound devices which are created by default (some of them even with
-nodefaults) it would not be nice to users if qemu required explicit:

  -audiodev driver=none,id=audio0 -machine audiodev=audio0

when they just want to run a VM and not care about any audio output.  Instead
this little helper makes it easy to create a dummy audiodev (driver=none) in
case there is no audiodev specified for the machine.  To make sure users
are not surprised by no sound output a helping message is also printed out.

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 audio/audio.c | 34 ++
 audio/audio.h |  2 ++
 2 files changed, 36 insertions(+)

diff --git a/audio/audio.c b/audio/audio.c
index d72e7db7fb9..8c74bc6b88c 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -41,6 +41,7 @@
 #include "sysemu/runstate.h"
 #include "ui/qemu-spice.h"
 #include "trace.h"
+#include "hw/boards.h"
 
 #define AUDIO_CAP "audio"
 #include "audio_int.h"
@@ -2174,6 +2175,39 @@ bool audio_init_audiodevs(void)
 return true;
 }
 
+static void audio_init_dummy(const char *id)
+{
+Audiodev *dev = g_new0(Audiodev, 1);
+AudiodevListEntry *e = g_new0(AudiodevListEntry, 1);
+
+dev->driver = AUDIODEV_DRIVER_NONE;
+dev->id = g_strdup(id);
+
+audio_validate_opts(dev, &error_abort);
+audio_init(dev, NULL);
+
+e->dev = dev;
+QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
+}
+
+const char *audio_maybe_init_dummy(const char *default_id)
+{
+MachineState *ms = MACHINE(qdev_get_machine());
+
+if (ms->default_audiodev) {
+return ms->default_audiodev;
+}
+
+dolog("warning: No audiodev specified for implicit machine devices, "
+  "no audio output will be available for these. "
+  "For setting a backend audiodev for such devices try using "
+  "the audiodev machine option.\n");
+
+audio_init_dummy(default_id);
+
+return default_id;
+}
+
 audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
 {
 return (audsettings) {
diff --git a/audio/audio.h b/audio/audio.h
index a276ec13eba..81d39526625 100644
--- a/audio/audio.h
+++ b/audio/audio.h
@@ -176,6 +176,8 @@ void audio_help(void);
 AudioState *audio_state_by_name(const char *name);
 const char *audio_get_id(QEMUSoundCard *card);
 
+const char *audio_maybe_init_dummy(const char *default_id);
+
 #define DEFINE_AUDIO_PROPERTIES(_s, _f) \
 DEFINE_PROP_AUDIODEV("audiodev", _s, _f)
 
-- 
2.41.0

[PATCH 9/9] audio: Be more strict during audio backend initialisation

[Paolo Bonzini]

From: Martin Kletzander 

Now that audiodev= is required and audio_init() will not be called
without and AudioDev we can remove the fallback functionality and error
out in case audio drivers fail initialisation or when the driver does
not exist.

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 audio/audio.c   | 138 
 docs/about/deprecated.rst   |   6 --
 docs/about/removed-features.rst |  11 ++-
 3 files changed, 42 insertions(+), 113 deletions(-)

diff --git a/audio/audio.c b/audio/audio.c
index 4fd27309cf8..31e3195c8d7 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -74,10 +74,9 @@ void audio_driver_register(audio_driver *drv)
 QLIST_INSERT_HEAD(&audio_drivers, drv, next);
 }
 
-static audio_driver *audio_driver_lookup(const char *name)
+static audio_driver *audio_driver_lookup(const char *name, Error **errp)
 {
 struct audio_driver *d;
-Error *local_err = NULL;
 int rv;
 
 QLIST_FOREACH(d, &audio_drivers, next) {
@@ -85,15 +84,19 @@ static audio_driver *audio_driver_lookup(const char *name)
 return d;
 }
 }
-rv = audio_module_load(name, &local_err);
+rv = audio_module_load(name, errp);
 if (rv > 0) {
 QLIST_FOREACH(d, &audio_drivers, next) {
 if (strcmp(name, d->name) == 0) {
 return d;
 }
 }
-} else if (rv < 0) {
-error_report_err(local_err);
+}
+
+if (rv < 0) {
+error_prepend(errp, "Cannot load audio driver `%s': ", name);
+} else {
+error_setg(errp, "Unknown audio driver `%s'", name);
 }
 return NULL;
 }
@@ -1551,31 +1554,27 @@ size_t audio_generic_read(HWVoiceIn *hw, void *buf, 
size_t size)
 return total;
 }
 
-static int audio_driver_init(AudioState *s, struct audio_driver *drv,
- bool msg, Audiodev *dev)
+static void audio_driver_init(AudioState *s, struct audio_driver *drv,
+  Audiodev *dev)
 {
 s->drv_opaque = drv->init(dev);
 
-if (s->drv_opaque) {
-if (!drv->pcm_ops->get_buffer_in) {
-drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
-drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
-}
-if (!drv->pcm_ops->get_buffer_out) {
-drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
-drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
-}
-
-audio_init_nb_voices_out(s, drv);
-audio_init_nb_voices_in(s, drv);
-s->drv = drv;
-return 0;
-} else {
-if (msg) {
-dolog("Could not init `%s' audio driver\n", drv->name);
-}
-return -1;
+if (!s->drv_opaque) {
+error_setg(&error_fatal, "Could not init `%s' audio driver", 
drv->name);
 }
+
+if (!drv->pcm_ops->get_buffer_in) {
+drv->pcm_ops->get_buffer_in = audio_generic_get_buffer_in;
+drv->pcm_ops->put_buffer_in = audio_generic_put_buffer_in;
+}
+if (!drv->pcm_ops->get_buffer_out) {
+drv->pcm_ops->get_buffer_out = audio_generic_get_buffer_out;
+drv->pcm_ops->put_buffer_out = audio_generic_put_buffer_out;
+}
+
+audio_init_nb_voices_out(s, drv);
+audio_init_nb_voices_in(s, drv);
+s->drv = drv;
 }
 
 static void audio_vm_change_state_handler (void *opaque, bool running,
@@ -1680,59 +1679,26 @@ static const VMStateDescription vmstate_audio = {
 
 static void audio_validate_opts(Audiodev *dev, Error **errp);
 
-static AudiodevListEntry *audiodev_find(
-AudiodevListHead *head, const char *drvname)
-{
-AudiodevListEntry *e;
-QSIMPLEQ_FOREACH(e, head, next) {
-if (strcmp(AudiodevDriver_str(e->dev->driver), drvname) == 0) {
-return e;
-}
-}
-
-return NULL;
-}
-
 /*
  * if we have dev, this function was called because of an -audiodev argument =>
  *   initialize a new state with it
  * if dev == NULL => legacy implicit initialization, return the already created
  *   state or create a new one
  */
-static AudioState *audio_init(Audiodev *dev, const char *name)
+static AudioState *audio_init(Audiodev *dev)
 {
 static bool atexit_registered;
-size_t i;
-int done = 0;
 const char *drvname = NULL;
 VMChangeStateEntry *e;
 AudioState *s;
-struct audio_driver *driver;
-/* silence gcc warning about uninitialized variable */
-AudiodevListHead head = QSIMPLEQ_HEAD_INITIALIZER(head);
 
-if (dev) {
-/* -audiodev option */
-drvname = AudiodevDriver_str(dev->driver);
-} else {
-if (!QTAILQ_EMPTY(&audio_states)) {
-dev = QTAILQ_FIRST(&audio_states)->dev;
-if (!g_str_equal(dev->id, "#none")) {
-dolog("Device %s: audiodev default parameter is deprecated, 
please "
-  "specify audiodev=%s\n", name,
-  dev->id);
-}
-r

[PATCH 3/9] vl: support -audio BACKEND without model

[Paolo Bonzini]

For machines with an embedded audio device, "-audio BACKEND" will
set the audiodev property of the machine itself.

Signed-off-by: Paolo Bonzini 
---
 softmmu/vl.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/softmmu/vl.c b/softmmu/vl.c
index 5a1413da2aa..70c9eb34dcf 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2933,11 +2933,12 @@ void qemu_init(int argc, char **argv)
 if (!qdict_haskey(dict, "id")) {
 qdict_put_str(dict, "id", "audiodev0");
 }
-if (!qdict_haskey(dict, "model")) {
-error_setg(&error_fatal, "Parameter 'model' is missing");
+if (qdict_haskey(dict, "model")) {
+model = g_strdup(qdict_get_str(dict, "model"));
+qdict_del(dict, "model");
+} else {
+model = g_strdup("default");
 }
-model = g_strdup(qdict_get_str(dict, "model"));
-qdict_del(dict, "model");
 if (is_help_option(model)) {
 show_valid_soundhw();
 exit(0);
@@ -2947,7 +2948,11 @@ void qemu_init(int argc, char **argv)
 visit_type_Audiodev(v, NULL, &dev, &error_fatal);
 visit_free(v);
 audio_define(dev);
-select_soundhw(model, dev->id);
+if (g_str_equal(model, "default")) {
+qdict_put_str(machine_opts_dict, "audiodev", dev->id);
+} else {
+select_soundhw(model, dev->id);
+}
 g_free(model);
 break;
 }
-- 
2.41.0

[PATCH 6/9] vt82c686: Support machine-default audiodev with fallback

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini 
---
 hw/isa/vt82c686.c   |  2 ++
 hw/mips/fuloong2e.c | 12 +---
 hw/ppc/pegasos2.c   | 10 --
 hw/ppc/sam460ex.c   |  2 ++
 4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78c..3ec8e43708a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
 object_initialize_child(obj, "uhci2", &s->uhci[1], 
TYPE_VT82C686B_USB_UHCI);
 object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
 object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
+
+object_property_add_alias(obj, "audiodev", OBJECT(&s->ac97), "audiodev");
 }
 
 static const TypeInfo via_isa_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..d0671824f94 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -41,6 +41,7 @@
 #include "sysemu/reset.h"
 #include "sysemu/sysemu.h"
 #include "qemu/error-report.h"
+#include "audio/audio.h"
 
 #define ENVP_PADDR  0x2000
 #define ENVP_VADDR  cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
@@ -295,9 +296,12 @@ static void mips_fuloong2e_init(MachineState *machine)
 pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));
 
 /* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
-  PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
-  TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+qdev_prop_set_string(DEVICE(pci_dev), "audiodev",
+ audio_maybe_init_dummy("fuloong2e.defaudio"));
+pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
+
 object_property_add_alias(OBJECT(machine), "rtc-time",
   object_resolve_path_component(OBJECT(pci_dev),
 "rtc"),
@@ -337,6 +341,8 @@ static void mips_fuloong2e_machine_init(MachineClass *mc)
 mc->default_ram_size = 256 * MiB;
 mc->default_ram_id = "fuloong2e.ram";
 mc->minimum_page_bits = 14;
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..45b94cab73a 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -37,6 +37,7 @@
 #include "qemu/datadir.h"
 #include "sysemu/device_tree.h"
 #include "hw/ppc/vof.h"
+#include "audio/audio.h"
 
 #include 
 
@@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
 pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);
 
 /* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA));
+qdev_prop_set_string(DEVICE(via), "audiodev",
+ audio_maybe_init_dummy("pegasos2.defaudio"));
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
+
 for (i = 0; i < PCI_NUM_PINS; i++) {
 pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
 }
@@ -564,6 +568,8 @@ static void pegasos2_machine_class_init(ObjectClass *oc, 
void *data)
 vhc->encode_hpt_for_kvm_pr = vhyp_encode_hpt_for_kvm_pr;
 
 vmc->setprop = pegasos2_setprop;
+
+machine_add_audiodev_property(mc);
 }
 
 static const TypeInfo pegasos2_machine_info = {
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 1e615b8d355..8c3abc67131 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -530,6 +530,8 @@ static void sam460ex_machine_init(MachineClass *mc)
 mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
 mc->default_ram_size = 512 * MiB;
 mc->default_ram_id = "ppc4xx.sdram";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("sam460ex", sam460ex_machine_init)
-- 
2.41.0

[PATCH 7/9] vl: recognize audiodev groups in configuration files

[Paolo Bonzini]

This is necessary for the q35 configuration tests to pass,
once audiodev becomes mandatory.

Signed-off-by: Paolo Bonzini 
---
 docs/config/q35-emulated.cfg |  4 
 docs/config/q35-virtio-graphical.cfg |  4 
 softmmu/vl.c | 10 ++
 3 files changed, 18 insertions(+)

diff --git a/docs/config/q35-emulated.cfg b/docs/config/q35-emulated.cfg
index c8806e6d362..b4bd7e858a9 100644
--- a/docs/config/q35-emulated.cfg
+++ b/docs/config/q35-emulated.cfg
@@ -288,3 +288,7 @@
   driver = "hda-duplex"
   bus = "ich9-hda-audio.0"
   cad = "0"
+  audiodev = "audiodev0"
+
+[audiodev "audiodev0"]
+  driver = "none"  # CHANGE ME
diff --git a/docs/config/q35-virtio-graphical.cfg 
b/docs/config/q35-virtio-graphical.cfg
index 148b5d2c5e4..820860aefe0 100644
--- a/docs/config/q35-virtio-graphical.cfg
+++ b/docs/config/q35-virtio-graphical.cfg
@@ -248,3 +248,7 @@
   driver = "hda-duplex"
   bus = "sound.0"
   cad = "0"
+  audiodev = "audiodev0"
+
+[audiodev "audiodev0"]
+  driver = "none"  # CHANGE ME
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 70c9eb34dcf..f74fc3d3e40 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2125,6 +2125,7 @@ static int global_init_func(void *opaque, QemuOpts *opts, 
Error **errp)
 static bool is_qemuopts_group(const char *group)
 {
 if (g_str_equal(group, "object") ||
+g_str_equal(group, "audiodev") ||
 g_str_equal(group, "machine") ||
 g_str_equal(group, "smp-opts") ||
 g_str_equal(group, "boot-opts")) {
@@ -2140,6 +2141,15 @@ static void qemu_record_config_group(const char *group, 
QDict *dict,
 Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict));
 object_option_add_visitor(v);
 visit_free(v);
+
+} else if (g_str_equal(group, "audiodev")) {
+Audiodev *dev = NULL;
+Visitor *v = qobject_input_visitor_new_keyval(QOBJECT(dict));
+if (visit_type_Audiodev(v, NULL, &dev, errp)) {
+audio_define(dev);
+}
+visit_free(v);
+
 } else if (g_str_equal(group, "machine")) {
 /*
  * Cannot merge string-valued and type-safe dictionaries, so JSON
-- 
2.41.0

[PATCH 5/9] hw/ppc: Support machine-default audiodev with fallback

[Paolo Bonzini]

From: Martin Kletzander 

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
 hw/ppc/prep.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c
index f6fd35fcb9e..739ff0ea8ad 100644
--- a/hw/ppc/prep.c
+++ b/hw/ppc/prep.c
@@ -45,6 +45,7 @@
 #include "trace.h"
 #include "elf.h"
 #include "qemu/units.h"
+#include "audio/audio.h"
 
 /* SMP is not enabled, for now */
 #define MAX_CPUS 1
@@ -310,6 +311,9 @@ static void ibm_40p_init(MachineState *machine)
 dev = DEVICE(isa_dev);
 qdev_prop_set_uint32(dev, "iobase", 0x830);
 qdev_prop_set_uint32(dev, "irq", 10);
+
+qdev_prop_set_string(dev, "audiodev",
+ audio_maybe_init_dummy("ppc.defaudio"));
 isa_realize_and_unref(isa_dev, isa_bus, &error_fatal);
 
 isa_dev = isa_new("pc87312");
@@ -426,6 +430,8 @@ static void ibm_40p_machine_init(MachineClass *mc)
 mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("604");
 mc->default_display = "std";
 mc->default_nic = "pcnet";
+
+machine_add_audiodev_property(mc);
 }
 
 DEFINE_MACHINE("40p", ibm_40p_machine_init)
-- 
2.41.0

[PATCH 0/9] audio: make sound cards require the audiodev property

[Paolo Bonzini]

This is the remaining part of Martin's patches to remove the deprecations
connected to audio backend configuration.  With these patches, all sound
cards have to be configured with an audio backend; only embedded cards
get the luxury of a default backend that emits or records no audio at all.
Fortunately, this is easy to do with the existing "-audio" option.

Apart from some rebasing changes, the changes are:

- the "-M" option for the embedded audiodev is only added to machines
  that need it, and renamed from "default-audiodev" to just "audiodev".

- the embedded audiodev is also accessible by specifying "-audio DRIVER"
  without a model

- the audiodev option is also configurable with -readconfig

- MIPS fuloong2e and PPC pegasos2 need changes in order to expose
  the embedded audiodev, too

Paolo

Based-on: <20230922093126.264016-1-pbonz...@redhat.com>

Martin Kletzander (6):
  audio: Add easy dummy audio initialiser
  Introduce machine property "audiodev"
  hw/arm: Support machine-default audiodev with fallback
  hw/ppc: Support machine-default audiodev with fallback
  audio: Make AUD_register_card fallible and require audiodev=
  audio: Be more strict during audio backend initialisation

Paolo Bonzini (3):
  vl: support -audio BACKEND without model
  vt82c686: Support machine-default audiodev with fallback
  vl: recognize audiodev groups in configuration files

 audio/audio.c| 177 +++
 audio/audio.h|   4 +-
 docs/about/deprecated.rst|   6 -
 docs/about/removed-features.rst  |  11 +-
 docs/config/q35-emulated.cfg |   4 +
 docs/config/q35-virtio-graphical.cfg |   4 +
 hw/arm/integratorcp.c|  10 +-
 hw/arm/musicpal.c|  11 +-
 hw/arm/nseries.c |   4 +
 hw/arm/omap2.c   |  10 +-
 hw/arm/palm.c|   2 +
 hw/arm/realview.c|  11 ++
 hw/arm/spitz.c   |  12 +-
 hw/arm/versatilepb.c |   7 ++
 hw/arm/vexpress.c|   4 +
 hw/arm/xlnx-zcu102.c |   5 +
 hw/arm/z2.c  |  14 ++-
 hw/audio/ac97.c  |   6 +-
 hw/audio/adlib.c |   6 +-
 hw/audio/cs4231a.c   |   6 +-
 hw/audio/es1370.c|   5 +-
 hw/audio/gus.c   |   6 +-
 hw/audio/hda-codec.c |   5 +-
 hw/audio/lm4549.c|   8 +-
 hw/audio/pcspk.c |   4 +-
 hw/audio/sb16.c  |   6 +-
 hw/audio/via-ac97.c  |   6 +-
 hw/audio/wm8750.c|   5 +-
 hw/core/machine.c|  28 +
 hw/display/xlnx_dp.c |   6 +-
 hw/input/tsc210x.c   |  10 +-
 hw/isa/vt82c686.c|   2 +
 hw/mips/fuloong2e.c  |  12 +-
 hw/ppc/pegasos2.c|  10 +-
 hw/ppc/prep.c|   6 +
 hw/ppc/sam460ex.c|   2 +
 hw/usb/dev-audio.c   |   5 +-
 include/hw/boards.h  |   7 ++
 softmmu/vl.c |  25 +++-
 39 files changed, 315 insertions(+), 157 deletions(-)

-- 
2.41.0

Re: [PATCH for-8.1] vfio/display: Fix missing update to set backing fields

[Michael Tokarev]

22.09.2023 12:38, Cédric Le Goater wrote:

On 9/13/23 21:18, Alex Williamson wrote:


Hi Gerd,

Some consultation would be appreciated on this thread to get this patch
out of limbo.  Is there a better solution that what I've proposed?


This does fix a regression reproducible on systems with an Intel Gen 8,
my T480 laptop for instance.

Tested-by: Cédric Le Goater 

Also, queuing it in vfio-next.



Is it not https://gitlab.com/qemu-project/qemu/-/issues/1891 ?

/mjt

Re: [PATCH 1/2] pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7

[Thomas Huth]

On 22/09/2023 11.19, Paolo Bonzini wrote:



Il ven 22 set 2023, 08:43 Thomas Huth > ha scritto:



While you're at it ... do we maybe want to start deprecating the next batch
of machine types already? (Say pc-i440fx-2.0 up to pc-i440fx-2.2 maybe?)


It depends on the benefit. We would have to check the compat options that 
are not needed anymore, and whether they'd be something that is useful 
anyway for debugging.


Also it would be useful to check if isapc can drop some of the compat code 
and realign itself to the 2.0 i440fx machine in terms of QEMU-specific features.


Because of all this todo, I decided not to proceed further with 
deprecations. The 128k ROM on the other hand does provide immediate benefit.


FWIW, when I was working on deprecating the old pc machine types, I 
originally wanted to stop with pc-1.3 (since I was most bugged by the 
different naming between "pc" and "pc-i44fx" in the help output). I then 
only put the 1.4 to 1.7 machine types on the deprecation list without 
anything in mind. Now this became useful for the 128k bios rework... so 
maybe we should continue deprecating older machine types for other future 
reworks that we don't envision yet.


 Thomas

Re: [PATCH v2 17/22] util/vhost-user-server: Clean up local variable shadowing

[Michael S. Tsirkin]

On Mon, Sep 04, 2023 at 06:12:29PM +0200, Philippe Mathieu-Daudé wrote:
> Fix:
> 
>   util/vhost-user-server.c: In function ‘set_watch’:
>   util/vhost-user-server.c:274:20: warning: declaration of ‘vu_fd_watch’ 
> shadows a previous local [-Wshadow=compatible-local]
> 274 | VuFdWatch *vu_fd_watch = g_new0(VuFdWatch, 1);
> |^~~
>   util/vhost-user-server.c:271:16: note: shadowed declaration is here
> 271 | VuFdWatch *vu_fd_watch = find_vu_fd_watch(server, fd);
> |^~~
> 
> Signed-off-by: Philippe Mathieu-Daudé 


Reviewed-by: Michael S. Tsirkin 

feel free to merge.

> ---
>  util/vhost-user-server.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/util/vhost-user-server.c b/util/vhost-user-server.c
> index cd17fb5326..5073f775ed 100644
> --- a/util/vhost-user-server.c
> +++ b/util/vhost-user-server.c
> @@ -271,7 +271,7 @@ set_watch(VuDev *vu_dev, int fd, int vu_evt,
>  VuFdWatch *vu_fd_watch = find_vu_fd_watch(server, fd);
>  
>  if (!vu_fd_watch) {
> -VuFdWatch *vu_fd_watch = g_new0(VuFdWatch, 1);
> +vu_fd_watch = g_new0(VuFdWatch, 1);
>  
>  QTAILQ_INSERT_TAIL(&server->vu_fd_watches, vu_fd_watch, next);
>  
> -- 
> 2.41.0
> 
> 

Re: [PATCH] hw/sd/sdhci: Block Size Register bits [14:12] is lost

[Philippe Mathieu-Daudé]

On 22/12/22 09:54, Gao, Lu wrote:

Hello,

Just want to check whether we can have it merged? Any other comments?


Oops sorry, I see I had this patch tagged for a merge request
but I lost track of it (likely when switching emails).

Patch queued!



Thanks a lot!
B.R.

-Original Message-
From: Philippe Mathieu-Daudé [mailto:philippe.mathieu.da...@gmail.com] On 
Behalf Of Philippe Mathieu-Daudé
Sent: Tuesday, May 31, 2022 6:09 PM
To: Gao, Lu; qemu-devel@nongnu.org
Cc: Wen, Jianxian; Bin Meng; open list:SD (Secure Card); Alexander Bulekov; 
Prasad J Pandit
Subject: Re: [PATCH] hw/sd/sdhci: Block Size Register bits [14:12] is lost

On 21/3/22 06:56, Lu Gao wrote:

Block Size Register bits [14:12] is SDMA Buffer Boundary, it is missed
in register write, but it is needed in SDMA transfer. e.g. it will be
used in sdhci_sdma_transfer_multi_blocks to calculate boundary_ variables.

Missing this field will cause wrong operation for different SDMA Buffer
Boundary settings.


Fixes: d7dfca0807 ("hw/sdhci: introduce standard SD host controller")
Fixes: dfba99f17f ("hw/sdhci: Fix DMA Transfer Block Size field")

Reviewed-by: Philippe Mathieu-Daudé 


Signed-off-by: Lu Gao 
Signed-off-by: Jianxian Wen 
---
   hw/sd/sdhci.c | 15 +++
   1 file changed, 11 insertions(+), 4 deletions(-)

Re: Help wanted for enabling -Wshadow=local

[Ani Sinha]

> On 22-Sep-2023, at 3:07 PM, Markus Armbruster  wrote:
> 
> Devices
> ---
> ACPI/SMBIOS
> M: Michael S. Tsirkin 
> M: Igor Mammedov 
> R: Ani Sinha 
>hw/acpi/cpu_hotplug.c
>hw/i386/acpi-build.c(*3*)
>hw/smbios/smbios.c

I will look into this after enabling

diff --git a/meson.build b/meson.build
index 98e68ef0b1..9fc4c7ac9d 100644
--- a/meson.build
+++ b/meson.build
@@ -466,6 +466,9 @@ warn_flags = [
   '-Wno-tautological-type-limit-compare',
   '-Wno-psabi',
   '-Wno-gnu-variable-sized-type-not-at-end',
+  '-Wshadow=local',
+  '-Wno-error=shadow=local',
+  '-Wno-error=shadow=compatible-local',
 ]

Re: [PATCH 3/3] hw/arm/smmuv3: Advertise SMMUv3.1-XNX feature

[Mostafa Saleh]

Hi Peter,

On Thu, Sep 14, 2023 at 03:57:05PM +0100, Peter Maydell wrote:
> The SMMUv3.1-XNX feature is mandatory for an SMMUv3.1 if S2P is
> supported, so we should theoretically have implemented it as part of
> the recent S2P work.  Fortunately, for us the implementation is a
> no-op.
> 
> This feature is about interpretation of the stage 2 page table
> descriptor XN bits, which control execute permissions.
> 
> For QEMU, the permission bits passed to an IOMMU (via MemTxAttrs and
> IOMMUAccessFlags) only indicate read and write; we do not distinguish
> data reads from instruction reads outside the CPU proper.  In the
> SMMU architecture's terms, our interconnect between the client device
> and the SMMU doesn't have the ability to convey the INST attribute,
> and we therefore use the default value of "data" for this attribute.
> 
> We also do not support the bits in the Stream Table Entry that can
> override the on-the-bus transaction attribute permissions (we do not
> set SMMU_IDR1.ATTR_PERMS_OVR=1).
> 
> These two things together mean that for our implementation, it never
> has to deal with transactions with the INST attribute, and so it can
> correctly ignore the XN bits entirely.  So we already implement
> FEAT_XNX's "XN field is now 2 bits, not 1" behaviour to the extent
> that we need to.
> 
> Advertise the presence of the feature in SMMU_IDR3.XNX.
> 
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/smmuv3.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 94d388fc950..d9e639f7c41 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -279,6 +279,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
>  s->idr[1] = FIELD_DP32(s->idr[1], IDR1, CMDQS,   SMMU_CMDQS);
>  
>  s->idr[3] = FIELD_DP32(s->idr[3], IDR3, HAD, 1);
> +s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
May be this can be guarded when S2P is present? according the UM
"In SMMUv3.1 and later, support for this feature is mandatory when
stage 2 is supported, that is when SMMU_IDR0.S2P == 1."
So I am not sure what it would mean for XNX and S1P only.

>  s->idr[3] = FIELD_DP32(s->idr[3], IDR3, RIL, 1);
>  s->idr[3] = FIELD_DP32(s->idr[3], IDR3, BBML, 2);

> -- 
> 2.34.1

Thanks,
Mostafa

Re: Help wanted for enabling -Wshadow=local

[Michael S. Tsirkin]

On Fri, Sep 22, 2023 at 04:03:27PM +0530, Ani Sinha wrote:
> 
> 
> > On 22-Sep-2023, at 3:07 PM, Markus Armbruster  wrote:
> > 
> > Devices
> > ---
> > ACPI/SMBIOS
> > M: Michael S. Tsirkin 
> > M: Igor Mammedov 
> > R: Ani Sinha 
> >hw/acpi/cpu_hotplug.c
> >hw/i386/acpi-build.c(*3*)
> >hw/smbios/smbios.c
> 
> I will look into this after enabling
> 
> diff --git a/meson.build b/meson.build
> index 98e68ef0b1..9fc4c7ac9d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -466,6 +466,9 @@ warn_flags = [
>'-Wno-tautological-type-limit-compare',
>'-Wno-psabi',
>'-Wno-gnu-variable-sized-type-not-at-end',
> +  '-Wshadow=local',
> +  '-Wno-error=shadow=local',
> +  '-Wno-error=shadow=compatible-local',
>  ]


Am I right that one can equivalently do
./configure --disable-werror --extra-cflags='-Wshadow=local 
-Wno-error=shadow=local -Wno-error=shadow=compatible-local'
?

[Bug 1819289] Re: Windows 95 and Windows 98 will not install or run

[Michael Tokarev]

I found a win98 iso image and gave this a try with qemu 8.1.  It works
here just fine - either kvm or tcg mode, either qemu x86_64 or i386.
Without any extra options, just this:


  qemu-system-i386 -cdrom w98.iso -drive file=w98.img,format=raw

It also works fine with a few previous versions of qemu (tried 5.2 and
7.2).  Everything tested on debian bookworm (with various versions of
qemu debian packages).

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1819289

Title:
  Windows 95 and Windows 98 will not install or run

Status in QEMU:
  Fix Released

Bug description:
  The last version of QEMU I have been able to run Windows 95 or Windows
  98 on was 2.7 or 2.8. Recent versions since then even up to 3.1 will
  either not install or will not run 95 or 98 at all. I have tried every
  combination of options like isapc or no isapc, cpu pentium  or cpu as
  486. Tried different memory configurations, but they just don't work
  anymore.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1819289/+subscriptions

Re: [PATCH 0/3] hw/arm/smmuv3: Advertise SMMUv3.1-XNX

[Mostafa Saleh]

Hi Peter,

On Thu, Sep 14, 2023 at 03:57:02PM +0100, Peter Maydell wrote:
> The SMMUv3.1-XNX feature is mandatory for an SMMUv3.1 if S2P is
> supported, so we should theoretically have implemented it as part of
> the recent S2P work.  Fortunately, for us the implementation is a
> no-op.
Oh, I missed that, thanks for spotting it.

> This feature is about interpretation of the stage 2 page table
> descriptor XN bits, which control execute permissions.
> 
> For QEMU, the permission bits passed to an IOMMU (via MemTxAttrs and
> IOMMUAccessFlags) only indicate read and write; we do not distinguish
> data reads from instruction reads outside the CPU proper.  In the
> SMMU architecture's terms, our interconnect between the client device
> and the SMMU doesn't have the ability to convey the INST attribute,
> and we therefore use the default value of "data" for this attribute.
> 
> We also do not support the bits in the Stream Table Entry that can
> override the on-the-bus transaction attribute permissions (we do not
> set SMMU_IDR1.ATTR_PERMS_OVR=1).
> 
> These two things together mean that for our implementation, it never
> has to deal with transactions with the INST attribute, and so it can
> correctly ignore the XN bits entirely.  So we already implement
> FEAT_XNX's "XN field is now 2 bits, not 1" behaviour to the extent
> that we need to.
> 
> Patches 1 and 2 in this series do a little bit of tidy up
> on the ID register bit code. Patch 3 is the one-liner to
> advertise SMMUv3.1-XNX in the ID register.
> 
> thanks
> -- PMM
> 
> Peter Maydell (3):
>   hw/arm/smmuv3: Update ID register bit field definitions
>   hw/arm/smmuv3: Sort ID register setting into field order
>   hw/arm/smmuv3: Advertise SMMUv3.1-XNX feature
> 
>  hw/arm/smmuv3-internal.h | 38 ++
>  hw/arm/smmuv3.c  |  5 +++--
>  2 files changed, 41 insertions(+), 2 deletions(-)
> 
> -- 
> 2.34.1

I left a comment/question on the last patch, otherwise

Reviewed-by: Mostafa Saleh 

Thanks,
Mostafa

Re: [PATCH 0/9] Replace remaining target_ulong in system-mode accel

[Anton Johansson via]

On 21/09/23, Michael Tokarev wrote:
> 07.08.2023 18:56, Anton Johansson via wrote:
> > This patchset replaces the remaining uses of target_ulong in the accel/
> > directory.  Specifically, the address type of a few kvm/hvf functions
> > is widened to vaddr, and the address type of the cpu_[st|ld]*()
> > functions is changed to abi_ptr (which is re-typedef'd to vaddr in
> > system mode).
> > 
> > As a starting point, my goal is to be able to build cputlb.c once for
> > system mode, and this is a step in that direction by reducing the
> > target-dependence of accel/.
> > 
> > * Changes in v2:
> >  - Removed explicit target_ulong casts from 3rd and 4th patches.
> > 
> > Anton Johansson (9):
> >accel/kvm: Widen pc/saved_insn for kvm_sw_breakpoint
> >accel/hvf: Widen pc/saved_insn for hvf_sw_breakpoint
> >target: Use vaddr for kvm_arch_[insert|remove]_hw_breakpoint
> >target: Use vaddr for hvf_arch_[insert|remove]_hw_breakpoint
> >Replace target_ulong with abi_ptr in cpu_[st|ld]*()
> >include/exec: typedef abi_ptr to vaddr in softmmu
> >include/exec: Widen tlb_hit/tlb_hit_page()
> >accel/tcg: Widen address arg. in tlb_compare_set()
> >accel/tcg: Update run_on_cpu_data static assert
> 
> Pinging a relatively old patchset, - which fixes from here needs to
> go to stable-8.1?
> 
> The context: 
> https://lore.kernel.org/qemu-devel/20230721205827.7502-1-a...@rev.ng/
> And according to this email:
> 
> https://lore.kernel.org/qemu-devel/00e9e08eae1004ef67fe8dca3aaf5043e6863faa.ca...@gmail.com/
> 
> at least "include/exec: Widen tlb_hit/tlb_hit_page()" should go to 8.1,
> something else?
> 
> Thanks,
> 
> /mjt

If the patch above is the only one needed to fix the segfault (haven't
tested myself), pulling it in isolation is fine as it doesn't depend on 
any of the other patches.

The rest of the patches can be delayed without issue.

-- 
Anton Johansson
rev.ng Labs Srl.

Re: [PATCH 3/3] hw/arm/smmuv3: Advertise SMMUv3.1-XNX feature

[Peter Maydell]

On Fri, 22 Sept 2023 at 11:34, Mostafa Saleh  wrote:
>
> Hi Peter,
>
> On Thu, Sep 14, 2023 at 03:57:05PM +0100, Peter Maydell wrote:
> > The SMMUv3.1-XNX feature is mandatory for an SMMUv3.1 if S2P is
> > supported, so we should theoretically have implemented it as part of
> > the recent S2P work.  Fortunately, for us the implementation is a
> > no-op.
> >
> > This feature is about interpretation of the stage 2 page table
> > descriptor XN bits, which control execute permissions.
> >
> > For QEMU, the permission bits passed to an IOMMU (via MemTxAttrs and
> > IOMMUAccessFlags) only indicate read and write; we do not distinguish
> > data reads from instruction reads outside the CPU proper.  In the
> > SMMU architecture's terms, our interconnect between the client device
> > and the SMMU doesn't have the ability to convey the INST attribute,
> > and we therefore use the default value of "data" for this attribute.
> >
> > We also do not support the bits in the Stream Table Entry that can
> > override the on-the-bus transaction attribute permissions (we do not
> > set SMMU_IDR1.ATTR_PERMS_OVR=1).
> >
> > These two things together mean that for our implementation, it never
> > has to deal with transactions with the INST attribute, and so it can
> > correctly ignore the XN bits entirely.  So we already implement
> > FEAT_XNX's "XN field is now 2 bits, not 1" behaviour to the extent
> > that we need to.
> >
> > Advertise the presence of the feature in SMMU_IDR3.XNX.
> >
> > Signed-off-by: Peter Maydell 
> > ---
> >  hw/arm/smmuv3.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > index 94d388fc950..d9e639f7c41 100644
> > --- a/hw/arm/smmuv3.c
> > +++ b/hw/arm/smmuv3.c
> > @@ -279,6 +279,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
> >  s->idr[1] = FIELD_DP32(s->idr[1], IDR1, CMDQS,   SMMU_CMDQS);
> >
> >  s->idr[3] = FIELD_DP32(s->idr[3], IDR3, HAD, 1);
> > +s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
> May be this can be guarded when S2P is present? according the UM
> "In SMMUv3.1 and later, support for this feature is mandatory when
> stage 2 is supported, that is when SMMU_IDR0.S2P == 1."
> So I am not sure what it would mean for XNX and S1P only.

Mmm, I don't suppose it would confuse any guest code, but
it's probably safest to put in the if():

   if (FIELD_EX32(s->idr[0], IDR0, S2P) {
   /* XNX is a stage-2-specific feature */
   s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
   }

thanks
-- PMM

[PATCH] test-throttle: don't shadow 'index' variable in do_test_accounting()

[Alberto Garcia]

Fixes build with -Wshadow=local

Signed-off-by: Alberto Garcia 
---
 tests/unit/test-throttle.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-throttle.c b/tests/unit/test-throttle.c
index cb587e33e7..ac35d65d19 100644
--- a/tests/unit/test-throttle.c
+++ b/tests/unit/test-throttle.c
@@ -625,7 +625,7 @@ static bool do_test_accounting(bool is_ops, /* are we 
testing bps or ops */
 throttle_config_init(&cfg);
 
 for (i = 0; i < 3; i++) {
-BucketType index = to_test[is_ops][i];
+index = to_test[is_ops][i];
 cfg.buckets[index].avg = avg;
 }
 
-- 
2.39.2

Re: [PATCH for-8.1] vfio/display: Fix missing update to set backing fields

[Cédric Le Goater]

On 9/22/23 11:49, Michael Tokarev wrote:

22.09.2023 12:38, Cédric Le Goater wrote:

On 9/13/23 21:18, Alex Williamson wrote:


Hi Gerd,

Some consultation would be appreciated on this thread to get this patch
out of limbo.  Is there a better solution that what I've proposed?


This does fix a regression reproducible on systems with an Intel Gen 8,
my T480 laptop for instance.

Tested-by: Cédric Le Goater 

Also, queuing it in vfio-next.



Is it not https://gitlab.com/qemu-project/qemu/-/issues/1891 ?


It is.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1891

Thanks,

C.

Re: [PATCH v23 01/20] CPU topology: extend with s390 specifics

[Nina Schoetterl-Glausch]

On Wed, 2023-09-20 at 13:11 +0200, Markus Armbruster wrote:
> Nina Schoetterl-Glausch  writes:
> 
> > From: Pierre Morel 
> > 
> > S390 adds two new SMP levels, drawers and books to the CPU
> > topology.
> > S390 CPUs have specific topology features like dedication and
> > entitlement. These indicate to the guest information on host
> > vCPU scheduling and help the guest make better scheduling decisions.
> > 
> > Let us provide the SMP properties with books and drawers levels
> > and S390 CPU with dedication and entitlement,
> > 
> > Signed-off-by: Pierre Morel 
> > Reviewed-by: Nina Schoetterl-Glausch 
> > Co-developed-by: Nina Schoetterl-Glausch 
> > Signed-off-by: Nina Schoetterl-Glausch 
> > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
> > new file mode 100644
> > index 00..e40421bb37
> > --- /dev/null
> > +++ b/qapi/machine-common.json
> > @@ -0,0 +1,21 @@
> > +# -*- Mode: Python -*-
> > +# vim: filetype=python
> > +#
> > +# This work is licensed under the terms of the GNU GPL, version 2 or later.
> > +# See the COPYING file in the top-level directory.
> > +
> > +##
> > +# = Machines S390 data types
> > +##
> > +
> > +##
> > +# @CpuS390Entitlement:
> > +#
> > +# An enumeration of cpu entitlements that can be assumed by a virtual
> > +# S390 CPU
> 
> CPU entitlements
> 
> Would someone reasonably familiar with S390 understand this?  Because

Well, someone familiar with s390 topology would, otherwise probably not tbh.

> I'm not and I don't; I wonder what "a virtual CPU assuming an
> entitlement" means.

Basically, on s390x the OS is always running on some hypervisor.
Even without KVM or z/VM you can slice up the machine, namely into logical
partitions (LPARs). Therefore, there is a scheduling of virtual CPUs to the
actual physical ones. "Entitlement" is a statement about how that scheduling
works for a virtual CPU. The same concepts can then also be applied to KVM.
> 
> > +#
> > +# Since: 8.2
> > +##
> > +{ 'enum': 'CpuS390Entitlement',
> > +  'prefix': 'S390_CPU_ENTITLEMENT',
> > +  'data': [ 'auto', 'low', 'medium', 'high' ] }
> 
> [...]
> 

Re: Help wanted for enabling -Wshadow=local

[Markus Armbruster]

"Michael S. Tsirkin"  writes:

> On Fri, Sep 22, 2023 at 04:03:27PM +0530, Ani Sinha wrote:
>> 
>> 
>> > On 22-Sep-2023, at 3:07 PM, Markus Armbruster  wrote:
>> > 
>> > Devices
>> > ---
>> > ACPI/SMBIOS
>> > M: Michael S. Tsirkin 
>> > M: Igor Mammedov 
>> > R: Ani Sinha 
>> >hw/acpi/cpu_hotplug.c
>> >hw/i386/acpi-build.c(*3*)
>> >hw/smbios/smbios.c
>> 
>> I will look into this after enabling
>> 
>> diff --git a/meson.build b/meson.build
>> index 98e68ef0b1..9fc4c7ac9d 100644
>> --- a/meson.build
>> +++ b/meson.build
>> @@ -466,6 +466,9 @@ warn_flags = [
>>'-Wno-tautological-type-limit-compare',
>>'-Wno-psabi',
>>'-Wno-gnu-variable-sized-type-not-at-end',
>> +  '-Wshadow=local',
>> +  '-Wno-error=shadow=local',
>> +  '-Wno-error=shadow=compatible-local',
>>  ]
>
>
> Am I right that one can equivalently do
> ./configure --disable-werror --extra-cflags='-Wshadow=local 
> -Wno-error=shadow=local -Wno-error=shadow=compatible-local'
> ?

Should do, although the --disable-werror is redundant with the
-Wno-error=...

Re: [PATCH v4] hw/i386/pc: improve physical address space bound check for 32-bit x86 systems

[Philippe Mathieu-Daudé]

On 22/9/23 06:16, Ani Sinha wrote:

32-bit x86 systems do not have a reserved memory for hole64. On those 32-bit
systems without PSE36 or PAE CPU features, hotplugging memory devices are not
supported by QEMU as QEMU always places hotplugged memory above 4 GiB boundary
which is beyond the physical address space of the processor. Linux guests also
does not support memory hotplug on those systems. Please see Linux
kernel commit b59d02ed08690 ("mm/memory_hotplug: disable the functionality
for 32b") for more details.

Therefore, the maximum limit of the guest physical address in the absence of
additional memory devices effectively coincides with the end of
"above 4G memory space" region for 32-bit x86 without PAE/PSE36. When users
configure additional memory devices, after properly accounting for the
additional device memory region to find the maximum value of the guest
physical address, the address will be outside the range of the processor's
physical address space.

This change adds improvements to take above into consideration.

For example, previously this was allowed:

$ ./qemu-system-x86_64 -cpu pentium -m size=10G

With this change now it is no longer allowed:

$ ./qemu-system-x86_64 -cpu pentium -m size=10G
qemu-system-x86_64: Address space limit 0x < 0x2bfff phys-bits too 
low (32)

However, the following are allowed since on both cases physical address
space of the processor is 36 bits:

$ ./qemu-system-x86_64 -cpu pentium2 -m size=10G
$ ./qemu-system-x86_64 -cpu pentium,pse36=on -m size=10G

For 32-bit, without PAE/PSE36, hotplugging additional memory is no longer 
allowed.

$ ./qemu-system-i386 -m size=1G,maxmem=3G,slots=2
qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
low (32)
$ ./qemu-system-i386 -machine q35 -m size=1G,maxmem=3G,slots=2
qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
low (32)

A new compatibility flag is introduced to make sure pc_max_used_gpa() keeps
returning the old value for machines 8.1 and older.
Therefore, the above is still allowed for older machine types in order to 
support
compatibility. Hence, the following still works:

$ ./qemu-system-i386 -machine pc-i440fx-8.1 -m size=1G,maxmem=3G,slots=2
$ ./qemu-system-i386 -machine pc-q35-8.1 -m size=1G,maxmem=3G,slots=2

Further, following is also allowed as with PSE36, the processor has 36-bit
address space:

$ ./qemu-system-i386 -cpu 486,pse36=on -m size=1G,maxmem=3G,slots=2

After calling CPUID with EAX=0x8001, all AMD64 compliant processors
have the longmode-capable-bit turned on in the extended feature flags (bit 29)
in EDX. The absence of CPUID longmode can be used to differentiate between
32-bit and 64-bit processors and is the recommended approach. QEMU takes this
approach elsewhere (for example, please see x86_cpu_realizefn()), With
this change, pc_max_used_gpa() also uses the same method to detect 32-bit
processors.

Unit tests are modified to not run 32-bit x86 tests that use memory hotplug.

Suggested-by: David Hildenbrand 
Signed-off-by: Ani Sinha 
---
  hw/i386/pc.c   | 31 ---
  hw/i386/pc_piix.c  |  4 
  hw/i386/pc_q35.c   |  2 ++
  include/hw/i386/pc.h   |  6 ++
  tests/qtest/bios-tables-test.c | 26 ++
  tests/qtest/numa-test.c|  7 ++-
  6 files changed, 64 insertions(+), 12 deletions(-)




diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 54838c0c41..2a689cf0bd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -907,12 +907,37 @@ static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
  static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
  {
  X86CPU *cpu = X86_CPU(first_cpu);
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+MachineState *ms = MACHINE(pcms);
+uint64_t devmem_start = 0;
+ram_addr_t devmem_size = 0;
  
-/* 32-bit systems don't have hole64 thus return max CPU address */

-if (cpu->phys_bits <= 32) {
-return ((hwaddr)1 << cpu->phys_bits) - 1;
+/*
+ * 32-bit systems don't have hole64 but they might have a region for
+ * memory devices. Even if additional hotplugged memory devices might
+ * not be usable by most guest OSes, we need to still consider them for
+ * calculating the highest possible GPA so that we can properly report
+ * if someone configures them on a CPU that cannot possibly address them.
+ */
+if (!(cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
+/* 32-bit systems */
+if (!pcmc->broken_32bit_mem_addr_check) {


Nitpicking, code is simplified if you invert this condition check.


+if (pcmc->has_reserved_memory &&
+(ms->ram_size < ms->maxram_size)) {
+pc_get_device_memory_range(pcms, &devmem_start,
+   &devmem_size);
+devmem_start += devmem_size;
+return de

Re: [PATCH 1/9] audio: Add easy dummy audio initialiser

[Marc-André Lureau]

Hi

On Fri, Sep 22, 2023 at 1:46 PM Paolo Bonzini  wrote:
>
> From: Martin Kletzander 
>
> In case of sound devices which are created by default (some of them even with
> -nodefaults) it would not be nice to users if qemu required explicit:
>
>   -audiodev driver=none,id=audio0 -machine audiodev=audio0
>
> when they just want to run a VM and not care about any audio output.  Instead
> this little helper makes it easy to create a dummy audiodev (driver=none) in
> case there is no audiodev specified for the machine.  To make sure users
> are not surprised by no sound output a helping message is also printed out.
>
> Signed-off-by: Martin Kletzander 
> Signed-off-by: Paolo Bonzini 
> ---
>  audio/audio.c | 34 ++
>  audio/audio.h |  2 ++
>  2 files changed, 36 insertions(+)
>
> diff --git a/audio/audio.c b/audio/audio.c
> index d72e7db7fb9..8c74bc6b88c 100644
> --- a/audio/audio.c
> +++ b/audio/audio.c
> @@ -41,6 +41,7 @@
>  #include "sysemu/runstate.h"
>  #include "ui/qemu-spice.h"
>  #include "trace.h"
> +#include "hw/boards.h"
>
>  #define AUDIO_CAP "audio"
>  #include "audio_int.h"
> @@ -2174,6 +2175,39 @@ bool audio_init_audiodevs(void)
>  return true;
>  }
>
> +static void audio_init_dummy(const char *id)
> +{
> +Audiodev *dev = g_new0(Audiodev, 1);
> +AudiodevListEntry *e = g_new0(AudiodevListEntry, 1);
> +
> +dev->driver = AUDIODEV_DRIVER_NONE;
> +dev->id = g_strdup(id);
> +
> +audio_validate_opts(dev, &error_abort);
> +audio_init(dev, NULL);
> +
> +e->dev = dev;
> +QSIMPLEQ_INSERT_TAIL(&audiodevs, e, next);
> +}
> +
> +const char *audio_maybe_init_dummy(const char *default_id)
> +{
> +MachineState *ms = MACHINE(qdev_get_machine());
> +
> +if (ms->default_audiodev) {
> +return ms->default_audiodev;
> +}

../audio/audio.c:2197:11: error: ‘MachineState’ {aka ‘struct
MachineState’} has no member named ‘default_audiodev’
 2197 | if (ms->default_audiodev) {

introduced in next patch

> +
> +dolog("warning: No audiodev specified for implicit machine devices, "
> +  "no audio output will be available for these. "
> +  "For setting a backend audiodev for such devices try using "
> +  "the audiodev machine option.\n");
> +
> +audio_init_dummy(default_id);
> +
> +return default_id;
> +}
> +
>  audsettings audiodev_to_audsettings(AudiodevPerDirectionOptions *pdo)
>  {
>  return (audsettings) {
> diff --git a/audio/audio.h b/audio/audio.h
> index a276ec13eba..81d39526625 100644
> --- a/audio/audio.h
> +++ b/audio/audio.h
> @@ -176,6 +176,8 @@ void audio_help(void);
>  AudioState *audio_state_by_name(const char *name);
>  const char *audio_get_id(QEMUSoundCard *card);
>
> +const char *audio_maybe_init_dummy(const char *default_id);
> +
>  #define DEFINE_AUDIO_PROPERTIES(_s, _f) \
>  DEFINE_PROP_AUDIODEV("audiodev", _s, _f)
>
> --
> 2.41.0
>
>


-- 
Marc-André Lureau

[PATCH v6 4/5] vhost-user-scsi: start vhost when guest kicks

[Li Feng]

Let's keep the same behavior as vhost-user-blk.

Some old guests kick virtqueue before setting VIRTIO_CONFIG_S_DRIVER_OK.

Signed-off-by: Li Feng 
---
 hw/scsi/vhost-user-scsi.c | 48 +++
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index dc109154ad..53a62c3170 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -115,8 +115,48 @@ static void vhost_user_scsi_reset(VirtIODevice *vdev)
 }
 }
 
-static void vhost_dummy_handle_output(VirtIODevice *vdev, VirtQueue *vq)
+static void vhost_user_scsi_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 {
+VHostUserSCSI *s = (VHostUserSCSI *)vdev;
+DeviceState *dev = &s->parent_obj.parent_obj.parent_obj.parent_obj;
+VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
+VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(dev);
+
+Error *local_err = NULL;
+int i, ret;
+
+if (!vdev->start_on_kick) {
+return;
+}
+
+if (!s->connected) {
+return;
+}
+
+if (vhost_dev_is_started(&vsc->dev)) {
+return;
+}
+
+/*
+ * Some guests kick before setting VIRTIO_CONFIG_S_DRIVER_OK so start
+ * vhost here instead of waiting for .set_status().
+ */
+ret = vhost_user_scsi_start(s, &local_err);
+if (ret < 0) {
+error_reportf_err(local_err, "vhost-user-scsi: vhost start failed: ");
+qemu_chr_fe_disconnect(&vs->conf.chardev);
+return;
+}
+
+/* Kick right away to begin processing requests already in vring */
+for (i = 0; i < vsc->dev.nvqs; i++) {
+VirtQueue *kick_vq = virtio_get_queue(vdev, i);
+
+if (!virtio_queue_get_desc_addr(vdev, i)) {
+continue;
+}
+event_notifier_set(virtio_queue_get_host_notifier(kick_vq));
+}
 }
 
 static int vhost_user_scsi_connect(DeviceState *dev, Error **errp)
@@ -246,9 +286,9 @@ static void vhost_user_scsi_realize(DeviceState *dev, Error 
**errp)
 return;
 }
 
-virtio_scsi_common_realize(dev, vhost_dummy_handle_output,
-   vhost_dummy_handle_output,
-   vhost_dummy_handle_output, &err);
+virtio_scsi_common_realize(dev, vhost_user_scsi_handle_output,
+   vhost_user_scsi_handle_output,
+   vhost_user_scsi_handle_output, &err);
 if (err != NULL) {
 error_propagate(errp, err);
 return;
-- 
2.41.0

[PATCH v6 2/5] vhost: move and rename the conn retry times

[Li Feng]

Multiple devices need this macro, move it to a common header.

Signed-off-by: Li Feng 
Reviewed-by: Raphael Norwitz 
---
 hw/block/vhost-user-blk.c   | 4 +---
 hw/virtio/vhost-user-gpio.c | 3 +--
 include/hw/virtio/vhost.h   | 2 ++
 3 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index eecf3f7a81..3c69fa47d5 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -32,8 +32,6 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/runstate.h"
 
-#define REALIZE_CONNECTION_RETRIES 3
-
 static const int user_feature_bits[] = {
 VIRTIO_BLK_F_SIZE_MAX,
 VIRTIO_BLK_F_SEG_MAX,
@@ -482,7 +480,7 @@ static void vhost_user_blk_device_realize(DeviceState *dev, 
Error **errp)
 s->inflight = g_new0(struct vhost_inflight, 1);
 s->vhost_vqs = g_new0(struct vhost_virtqueue, s->num_queues);
 
-retries = REALIZE_CONNECTION_RETRIES;
+retries = VU_REALIZE_CONN_RETRIES;
 assert(!*errp);
 do {
 if (*errp) {
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index 3b013f2d0f..d9979aa5db 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -15,7 +15,6 @@
 #include "standard-headers/linux/virtio_ids.h"
 #include "trace.h"
 
-#define REALIZE_CONNECTION_RETRIES 3
 #define VHOST_NVQS 2
 
 /* Features required from VirtIO */
@@ -359,7 +358,7 @@ static void vu_gpio_device_realize(DeviceState *dev, Error 
**errp)
 qemu_chr_fe_set_handlers(&gpio->chardev, NULL, NULL, vu_gpio_event, NULL,
  dev, NULL, true);
 
-retries = REALIZE_CONNECTION_RETRIES;
+retries = VU_REALIZE_CONN_RETRIES;
 g_assert(!*errp);
 do {
 if (*errp) {
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index 6a173cb9fa..ca3131b1af 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -8,6 +8,8 @@
 #define VHOST_F_DEVICE_IOTLB 63
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 
+#define VU_REALIZE_CONN_RETRIES 3
+
 /* Generic structures common for any vhost based device. */
 
 struct vhost_inflight {
-- 
2.41.0

[PATCH v6 0/5] Implement reconnect for vhost-user-scsi

[Li Feng]

Changes for v6:
- [PATCH] vhost-user: fix lost reconnect
  - Fix missing assign event_cb.

Changes for v5:
- No logic has been changed, just move part of the code from patch 4 to patch 5.

Changes for v4:
- Merge
  https://lore.kernel.org/all/20230830045722.611224-1-fen...@smartx.com/ to
  this series.
- Add ERRP_GUARD in vhost_user_scsi_realize;
- Reword the commit messages.

Changes for v3:
- Split the vhost_user_scsi_handle_output to a separate patch;
- Move the started_vu from vhost scsi common header to vhost-user-scsi header;
- Fix a log print error;

Changes for v2:
- Split the v1 patch to small separate patchset;
- New patch for fixing fd leak, which has sent to reviewers in another
  mail;
- Implement the `vhost_user_scsi_handle_output`;
- Add the started_vu safe check;
- Fix error handler;
- Check the inflight before set/get inflight fd.

Li Feng (5):
  vhost-user-common: send get_inflight_fd once
  vhost: move and rename the conn retry times
  vhost-user-scsi: support reconnect to backend
  vhost-user-scsi: start vhost when guest kicks
  vhost-user: fix lost reconnect

 hw/block/vhost-user-blk.c |   6 +-
 hw/scsi/vhost-scsi-common.c   |  47 ++---
 hw/scsi/vhost-scsi.c  |   5 +-
 hw/scsi/vhost-user-scsi.c | 253 +++---
 hw/virtio/vhost-user-gpio.c   |   5 +-
 hw/virtio/vhost-user.c|  10 +-
 include/hw/virtio/vhost-scsi-common.h |   2 +-
 include/hw/virtio/vhost-user-scsi.h   |   4 +
 include/hw/virtio/vhost-user.h|   3 +-
 include/hw/virtio/vhost.h |   2 +
 10 files changed, 277 insertions(+), 60 deletions(-)

-- 
2.41.0

[PATCH v6 5/5] vhost-user: fix lost reconnect

[Li Feng]

When the vhost-user is reconnecting to the backend, and if the vhost-user fails
at the get_features in vhost_dev_init(), then the reconnect will fail
and it will not be retriggered forever.

The reason is:
When the vhost-user fails at get_features, the vhost_dev_cleanup will be called
immediately.

vhost_dev_cleanup calls 'memset(hdev, 0, sizeof(struct vhost_dev))'.

The reconnect path is:
vhost_user_blk_event
   vhost_user_async_close(.. vhost_user_blk_disconnect ..)
 qemu_chr_fe_set_handlers <- clear the notifier callback
   schedule vhost_user_async_close_bh

The vhost->vdev is null, so the vhost_user_blk_disconnect will not be
called, then the event fd callback will not be reinstalled.

All vhost-user devices have this issue, including vhost-user-blk/scsi.

With this patch, if the vdev->vdev is null, the fd callback will still
be reinstalled.

Fixes: 71e076a07d ("hw/virtio: generalise CHR_EVENT_CLOSED handling")

Signed-off-by: Li Feng 
---
 hw/block/vhost-user-blk.c  |  2 +-
 hw/scsi/vhost-user-scsi.c  |  3 ++-
 hw/virtio/vhost-user-gpio.c|  2 +-
 hw/virtio/vhost-user.c | 10 --
 include/hw/virtio/vhost-user.h |  3 ++-
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/hw/block/vhost-user-blk.c b/hw/block/vhost-user-blk.c
index 3c69fa47d5..95c758200d 100644
--- a/hw/block/vhost-user-blk.c
+++ b/hw/block/vhost-user-blk.c
@@ -391,7 +391,7 @@ static void vhost_user_blk_event(void *opaque, QEMUChrEvent 
event)
 case CHR_EVENT_CLOSED:
 /* defer close until later to avoid circular close */
 vhost_user_async_close(dev, &s->chardev, &s->dev,
-   vhost_user_blk_disconnect);
+   vhost_user_blk_disconnect, 
vhost_user_blk_event);
 break;
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index 53a62c3170..0effbb4787 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -238,7 +238,8 @@ static void vhost_user_scsi_event(void *opaque, 
QEMUChrEvent event)
 case CHR_EVENT_CLOSED:
 /* defer close until later to avoid circular close */
 vhost_user_async_close(dev, &vs->conf.chardev, &vsc->dev,
-   vhost_user_scsi_disconnect);
+   vhost_user_scsi_disconnect,
+   vhost_user_scsi_event);
 break;
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost-user-gpio.c b/hw/virtio/vhost-user-gpio.c
index d9979aa5db..04c2cc79f4 100644
--- a/hw/virtio/vhost-user-gpio.c
+++ b/hw/virtio/vhost-user-gpio.c
@@ -283,7 +283,7 @@ static void vu_gpio_event(void *opaque, QEMUChrEvent event)
 case CHR_EVENT_CLOSED:
 /* defer close until later to avoid circular close */
 vhost_user_async_close(dev, &gpio->chardev, &gpio->vhost_dev,
-   vu_gpio_disconnect);
+   vu_gpio_disconnect, vu_gpio_event);
 break;
 case CHR_EVENT_BREAK:
 case CHR_EVENT_MUX_IN:
diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 8dcf049d42..7344f57ba7 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2643,6 +2643,7 @@ typedef struct {
 DeviceState *dev;
 CharBackend *cd;
 struct vhost_dev *vhost;
+IOEventHandler *event_cb;
 } VhostAsyncCallback;
 
 static void vhost_user_async_close_bh(void *opaque)
@@ -2657,7 +2658,10 @@ static void vhost_user_async_close_bh(void *opaque)
  */
 if (vhost->vdev) {
 data->cb(data->dev);
-}
+} else if (data->event_cb) {
+qemu_chr_fe_set_handlers(data->cd, NULL, NULL, data->event_cb,
+ NULL, data->dev, NULL, true);
+   }
 
 g_free(data);
 }
@@ -2669,7 +2673,8 @@ static void vhost_user_async_close_bh(void *opaque)
  */
 void vhost_user_async_close(DeviceState *d,
 CharBackend *chardev, struct vhost_dev *vhost,
-vu_async_close_fn cb)
+vu_async_close_fn cb,
+IOEventHandler *event_cb)
 {
 if (!runstate_check(RUN_STATE_SHUTDOWN)) {
 /*
@@ -2685,6 +2690,7 @@ void vhost_user_async_close(DeviceState *d,
 data->dev = d;
 data->cd = chardev;
 data->vhost = vhost;
+data->event_cb = event_cb;
 
 /* Disable any further notifications on the chardev */
 qemu_chr_fe_set_handlers(chardev,
diff --git a/include/hw/virtio/vhost-user.h b/include/hw/virtio/vhost-user.h
index 191216a74f..649e9dd54f 100644
--- a/include/hw/virtio/vhost-user.h
+++ b/include/hw/virtio/vhost-user.h
@@ -84,6 +84,7 @@ typedef void (*vu_async_close_fn)(DeviceState *cb);
 
 void vhost_user_async_close(DeviceState *d,
 CharBackend *chardev, struct vhost_dev *vhost,
-vu_async_close_fn cb);
+  

[PATCH v6 1/5] vhost-user-common: send get_inflight_fd once

[Li Feng]

Currently the get_inflight_fd will be sent every time the device is started, and
the backend will allocate shared memory to save the inflight state. If the
backend finds that it receives the second get_inflight_fd, it will release the
previous shared memory, which breaks inflight working logic.

This patch is a preparation for the following patches.

Signed-off-by: Li Feng 
---
 hw/scsi/vhost-scsi-common.c | 37 ++---
 1 file changed, 18 insertions(+), 19 deletions(-)

diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c
index a06f01af26..a61cd0e907 100644
--- a/hw/scsi/vhost-scsi-common.c
+++ b/hw/scsi/vhost-scsi-common.c
@@ -52,20 +52,28 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
 
 vsc->dev.acked_features = vdev->guest_features;
 
-assert(vsc->inflight == NULL);
-vsc->inflight = g_new0(struct vhost_inflight, 1);
-ret = vhost_dev_get_inflight(&vsc->dev,
- vs->conf.virtqueue_size,
- vsc->inflight);
+ret = vhost_dev_prepare_inflight(&vsc->dev, vdev);
 if (ret < 0) {
-error_report("Error get inflight: %d", -ret);
+error_report("Error setting inflight format: %d", -ret);
 goto err_guest_notifiers;
 }
 
-ret = vhost_dev_set_inflight(&vsc->dev, vsc->inflight);
-if (ret < 0) {
-error_report("Error set inflight: %d", -ret);
-goto err_guest_notifiers;
+if (vsc->inflight) {
+if (!vsc->inflight->addr) {
+ret = vhost_dev_get_inflight(&vsc->dev,
+vs->conf.virtqueue_size,
+vsc->inflight);
+if (ret < 0) {
+error_report("Error getting inflight: %d", -ret);
+goto err_guest_notifiers;
+}
+}
+
+ret = vhost_dev_set_inflight(&vsc->dev, vsc->inflight);
+if (ret < 0) {
+error_report("Error setting inflight: %d", -ret);
+goto err_guest_notifiers;
+}
 }
 
 ret = vhost_dev_start(&vsc->dev, vdev, true);
@@ -85,9 +93,6 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
 return ret;
 
 err_guest_notifiers:
-g_free(vsc->inflight);
-vsc->inflight = NULL;
-
 k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, false);
 err_host_notifiers:
 vhost_dev_disable_notifiers(&vsc->dev, vdev);
@@ -111,12 +116,6 @@ void vhost_scsi_common_stop(VHostSCSICommon *vsc)
 }
 assert(ret >= 0);
 
-if (vsc->inflight) {
-vhost_dev_free_inflight(vsc->inflight);
-g_free(vsc->inflight);
-vsc->inflight = NULL;
-}
-
 vhost_dev_disable_notifiers(&vsc->dev, vdev);
 }
 
-- 
2.41.0

[PATCH v6 3/5] vhost-user-scsi: support reconnect to backend

[Li Feng]

If the backend crashes and restarts, the device is broken.
This patch adds reconnect for vhost-user-scsi.

This patch also improves the error messages, and reports some silent errors.

Tested with spdk backend.

Signed-off-by: Li Feng 
---
 hw/scsi/vhost-scsi-common.c   |  16 +-
 hw/scsi/vhost-scsi.c  |   5 +-
 hw/scsi/vhost-user-scsi.c | 204 +++---
 include/hw/virtio/vhost-scsi-common.h |   2 +-
 include/hw/virtio/vhost-user-scsi.h   |   4 +
 5 files changed, 201 insertions(+), 30 deletions(-)

diff --git a/hw/scsi/vhost-scsi-common.c b/hw/scsi/vhost-scsi-common.c
index a61cd0e907..4c8637045d 100644
--- a/hw/scsi/vhost-scsi-common.c
+++ b/hw/scsi/vhost-scsi-common.c
@@ -16,6 +16,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "hw/virtio/vhost.h"
@@ -25,7 +26,7 @@
 #include "hw/virtio/virtio-access.h"
 #include "hw/fw-path-provider.h"
 
-int vhost_scsi_common_start(VHostSCSICommon *vsc)
+int vhost_scsi_common_start(VHostSCSICommon *vsc, Error **errp)
 {
 int ret, i;
 VirtIODevice *vdev = VIRTIO_DEVICE(vsc);
@@ -35,18 +36,19 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
 VirtIOSCSICommon *vs = (VirtIOSCSICommon *)vsc;
 
 if (!k->set_guest_notifiers) {
-error_report("binding does not support guest notifiers");
+error_setg(errp, "binding does not support guest notifiers");
 return -ENOSYS;
 }
 
 ret = vhost_dev_enable_notifiers(&vsc->dev, vdev);
 if (ret < 0) {
+error_setg_errno(errp, -ret, "Error enabling host notifiers");
 return ret;
 }
 
 ret = k->set_guest_notifiers(qbus->parent, vsc->dev.nvqs, true);
 if (ret < 0) {
-error_report("Error binding guest notifier");
+error_setg_errno(errp, -ret, "Error binding guest notifier");
 goto err_host_notifiers;
 }
 
@@ -54,7 +56,7 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
 
 ret = vhost_dev_prepare_inflight(&vsc->dev, vdev);
 if (ret < 0) {
-error_report("Error setting inflight format: %d", -ret);
+error_setg_errno(errp, -ret, "Error setting inflight format");
 goto err_guest_notifiers;
 }
 
@@ -64,21 +66,21 @@ int vhost_scsi_common_start(VHostSCSICommon *vsc)
 vs->conf.virtqueue_size,
 vsc->inflight);
 if (ret < 0) {
-error_report("Error getting inflight: %d", -ret);
+error_setg_errno(errp, -ret, "Error getting inflight");
 goto err_guest_notifiers;
 }
 }
 
 ret = vhost_dev_set_inflight(&vsc->dev, vsc->inflight);
 if (ret < 0) {
-error_report("Error setting inflight: %d", -ret);
+error_setg_errno(errp, -ret, "Error setting inflight");
 goto err_guest_notifiers;
 }
 }
 
 ret = vhost_dev_start(&vsc->dev, vdev, true);
 if (ret < 0) {
-error_report("Error start vhost dev");
+error_setg_errno(errp, -ret, "Error starting vhost dev");
 goto err_guest_notifiers;
 }
 
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 443f67daa4..01a3ab4277 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -75,6 +75,7 @@ static int vhost_scsi_start(VHostSCSI *s)
 int ret, abi_version;
 VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
 const VhostOps *vhost_ops = vsc->dev.vhost_ops;
+Error *local_err = NULL;
 
 ret = vhost_ops->vhost_scsi_get_abi_version(&vsc->dev, &abi_version);
 if (ret < 0) {
@@ -88,14 +89,14 @@ static int vhost_scsi_start(VHostSCSI *s)
 return -ENOSYS;
 }
 
-ret = vhost_scsi_common_start(vsc);
+ret = vhost_scsi_common_start(vsc, &local_err);
 if (ret < 0) {
 return ret;
 }
 
 ret = vhost_scsi_set_endpoint(s);
 if (ret < 0) {
-error_report("Error setting vhost-scsi endpoint");
+error_reportf_err(local_err, "Error setting vhost-scsi endpoint");
 vhost_scsi_common_stop(vsc);
 }
 
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
index ee99b19e7a..dc109154ad 100644
--- a/hw/scsi/vhost-user-scsi.c
+++ b/hw/scsi/vhost-user-scsi.c
@@ -43,26 +43,56 @@ enum VhostUserProtocolFeature {
 VHOST_USER_PROTOCOL_F_RESET_DEVICE = 13,
 };
 
+static int vhost_user_scsi_start(VHostUserSCSI *s, Error **errp)
+{
+VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
+int ret;
+
+ret = vhost_scsi_common_start(vsc, errp);
+s->started_vu = (ret < 0 ? false : true);
+
+return ret;
+}
+
+static void vhost_user_scsi_stop(VHostUserSCSI *s)
+{
+VHostSCSICommon *vsc = VHOST_SCSI_COMMON(s);
+
+if (!s->started_vu) {
+return;
+}
+s->started_vu = false;
+
+vhost_scsi_common_stop(vsc);
+}
+
 static void vhost_user_scsi_set_status(VirtIODevice *vdev, uint8_t status)
 {
 VHostUser

Re: [PATCH v4] hw/i386/pc: improve physical address space bound check for 32-bit x86 systems

[Ani Sinha]

> On 22-Sep-2023, at 4:12 PM, Philippe Mathieu-Daudé  wrote:
> 
> On 22/9/23 06:16, Ani Sinha wrote:
>> 32-bit x86 systems do not have a reserved memory for hole64. On those 32-bit
>> systems without PSE36 or PAE CPU features, hotplugging memory devices are not
>> supported by QEMU as QEMU always places hotplugged memory above 4 GiB 
>> boundary
>> which is beyond the physical address space of the processor. Linux guests 
>> also
>> does not support memory hotplug on those systems. Please see Linux
>> kernel commit b59d02ed08690 ("mm/memory_hotplug: disable the functionality
>> for 32b") for more details.
>> Therefore, the maximum limit of the guest physical address in the absence of
>> additional memory devices effectively coincides with the end of
>> "above 4G memory space" region for 32-bit x86 without PAE/PSE36. When users
>> configure additional memory devices, after properly accounting for the
>> additional device memory region to find the maximum value of the guest
>> physical address, the address will be outside the range of the processor's
>> physical address space.
>> This change adds improvements to take above into consideration.
>> For example, previously this was allowed:
>> $ ./qemu-system-x86_64 -cpu pentium -m size=10G
>> With this change now it is no longer allowed:
>> $ ./qemu-system-x86_64 -cpu pentium -m size=10G
>> qemu-system-x86_64: Address space limit 0x < 0x2bfff phys-bits 
>> too low (32)
>> However, the following are allowed since on both cases physical address
>> space of the processor is 36 bits:
>> $ ./qemu-system-x86_64 -cpu pentium2 -m size=10G
>> $ ./qemu-system-x86_64 -cpu pentium,pse36=on -m size=10G
>> For 32-bit, without PAE/PSE36, hotplugging additional memory is no longer 
>> allowed.
>> $ ./qemu-system-i386 -m size=1G,maxmem=3G,slots=2
>> qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
>> low (32)
>> $ ./qemu-system-i386 -machine q35 -m size=1G,maxmem=3G,slots=2
>> qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
>> low (32)
>> A new compatibility flag is introduced to make sure pc_max_used_gpa() keeps
>> returning the old value for machines 8.1 and older.
>> Therefore, the above is still allowed for older machine types in order to 
>> support
>> compatibility. Hence, the following still works:
>> $ ./qemu-system-i386 -machine pc-i440fx-8.1 -m size=1G,maxmem=3G,slots=2
>> $ ./qemu-system-i386 -machine pc-q35-8.1 -m size=1G,maxmem=3G,slots=2
>> Further, following is also allowed as with PSE36, the processor has 36-bit
>> address space:
>> $ ./qemu-system-i386 -cpu 486,pse36=on -m size=1G,maxmem=3G,slots=2
>> After calling CPUID with EAX=0x8001, all AMD64 compliant processors
>> have the longmode-capable-bit turned on in the extended feature flags (bit 
>> 29)
>> in EDX. The absence of CPUID longmode can be used to differentiate between
>> 32-bit and 64-bit processors and is the recommended approach. QEMU takes this
>> approach elsewhere (for example, please see x86_cpu_realizefn()), With
>> this change, pc_max_used_gpa() also uses the same method to detect 32-bit
>> processors.
>> Unit tests are modified to not run 32-bit x86 tests that use memory hotplug.
>> Suggested-by: David Hildenbrand 
>> Signed-off-by: Ani Sinha 
>> ---
>>  hw/i386/pc.c   | 31 ---
>>  hw/i386/pc_piix.c  |  4 
>>  hw/i386/pc_q35.c   |  2 ++
>>  include/hw/i386/pc.h   |  6 ++
>>  tests/qtest/bios-tables-test.c | 26 ++
>>  tests/qtest/numa-test.c|  7 ++-
>>  6 files changed, 64 insertions(+), 12 deletions(-)
> 
> 
>> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
>> index 54838c0c41..2a689cf0bd 100644
>> --- a/hw/i386/pc.c
>> +++ b/hw/i386/pc.c
>> @@ -907,12 +907,37 @@ static uint64_t pc_get_cxl_range_end(PCMachineState 
>> *pcms)
>>  static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t 
>> pci_hole64_size)
>>  {
>>  X86CPU *cpu = X86_CPU(first_cpu);
>> +PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>> +MachineState *ms = MACHINE(pcms);
>> +uint64_t devmem_start = 0;
>> +ram_addr_t devmem_size = 0;
>>  -/* 32-bit systems don't have hole64 thus return max CPU address */
>> -if (cpu->phys_bits <= 32) {
>> -return ((hwaddr)1 << cpu->phys_bits) - 1;
>> +/*
>> + * 32-bit systems don't have hole64 but they might have a region for
>> + * memory devices. Even if additional hotplugged memory devices might
>> + * not be usable by most guest OSes, we need to still consider them for
>> + * calculating the highest possible GPA so that we can properly report
>> + * if someone configures them on a CPU that cannot possibly address 
>> them.
>> + */
>> +if (!(cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
>> +/* 32-bit systems */
>> +if (!pcmc->broken_32bit_mem_addr_check) {
> 
> Nitpicking, code is simplified if you in

Re: [PATCH 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough'

[Vitaly Kuznetsov]

Vitaly Kuznetsov  writes:

> Vitaly Kuznetsov  writes:
>
>> Vitaly Kuznetsov  writes:
>>
>>> Hyper-V Gen1 guests are getting stuck on boot when 'hv-passthrough' is
>>> used. While 'hv-passthrough' is a debug only feature, this significantly
>>> limit its usefullness. While debugging the problem, I found that there are
>>> two loosely connected issues:
>>> - 'hv-passthrough' enables 'hv-syndbg' and this is undesired.
>>> - 'hv-syndbg's support by KVM is detected incorrectly when !CONFIG_SYNDBG.
>>>
>>> Fix both issues; exclude 'hv-syndbg' from 'hv-passthrough' and don't allow
>>> to turn on 'hv-syndbg' for !CONFIG_SYNDBG builds. 
>>>
>>> Vitaly Kuznetsov (2):
>>>   i386: Fix conditional CONFIG_SYNDBG enablement
>>>   i386: Exclude 'hv-syndbg' from 'hv-passthrough'
>>>
>>>  docs/system/i386/hyperv.rst | 13 +
>>>  target/i386/cpu.c   |  2 ++
>>>  target/i386/kvm/kvm.c   | 18 --
>>>  3 files changed, 23 insertions(+), 10 deletions(-)
>
> Monthly ping)

Turns out these patches were never merged and honestly I forgot about
them myself. Will resend shortly.

-- 
Vitaly

Re: [PATCH 6/9] vt82c686: Support machine-default audiodev with fallback

[BALATON Zoltan]

On Fri, 22 Sep 2023, Paolo Bonzini wrote:

Signed-off-by: Paolo Bonzini 
---
hw/isa/vt82c686.c   |  2 ++
hw/mips/fuloong2e.c | 12 +---
hw/ppc/pegasos2.c   | 10 --
hw/ppc/sam460ex.c   |  2 ++
4 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 57bdfb4e78c..3ec8e43708a 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -578,6 +578,8 @@ static void via_isa_init(Object *obj)
object_initialize_child(obj, "uhci2", &s->uhci[1], TYPE_VT82C686B_USB_UHCI);
object_initialize_child(obj, "ac97", &s->ac97, TYPE_VIA_AC97);
object_initialize_child(obj, "mc97", &s->mc97, TYPE_VIA_MC97);
+
+object_property_add_alias(obj, "audiodev", OBJECT(&s->ac97), "audiodev");
}

static const TypeInfo via_isa_info = {
diff --git a/hw/mips/fuloong2e.c b/hw/mips/fuloong2e.c
index c827f615f3b..d0671824f94 100644
--- a/hw/mips/fuloong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -41,6 +41,7 @@
#include "sysemu/reset.h"
#include "sysemu/sysemu.h"
#include "qemu/error-report.h"
+#include "audio/audio.h"

#define ENVP_PADDR  0x2000
#define ENVP_VADDR  cpu_mips_phys_to_kseg0(NULL, ENVP_PADDR)
@@ -295,9 +296,12 @@ static void mips_fuloong2e_init(MachineState *machine)
pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));

/* South bridge -> IP5 */
-pci_dev = pci_create_simple_multifunction(pci_bus,
-  PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
-  TYPE_VT82C686B_ISA);
+pci_dev = pci_new_multifunction(PCI_DEVFN(FULOONG2E_VIA_SLOT, 0),
+TYPE_VT82C686B_ISA);
+qdev_prop_set_string(DEVICE(pci_dev), "audiodev",
+ audio_maybe_init_dummy("fuloong2e.defaudio"));
+pci_realize_and_unref(pci_dev, pci_bus, &error_abort);
+
object_property_add_alias(OBJECT(machine), "rtc-time",
  object_resolve_path_component(OBJECT(pci_dev),
"rtc"),
@@ -337,6 +341,8 @@ static void mips_fuloong2e_machine_init(MachineClass *mc)
mc->default_ram_size = 256 * MiB;
mc->default_ram_id = "fuloong2e.ram";
mc->minimum_page_bits = 14;
+
+machine_add_audiodev_property(mc);
}

DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index bd397cf2b5c..45b94cab73a 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -37,6 +37,7 @@
#include "qemu/datadir.h"
#include "sysemu/device_tree.h"
#include "hw/ppc/vof.h"
+#include "audio/audio.h"

#include 

@@ -180,8 +181,11 @@ static void pegasos2_init(MachineState *machine)
pci_bus_irqs(pci_bus, pegasos2_pci_irq, pm, PCI_NUM_PINS);

/* VIA VT8231 South Bridge (multifunction PCI device) */
-via = OBJECT(pci_create_simple_multifunction(pci_bus, PCI_DEVFN(12, 0),
- TYPE_VT8231_ISA));
+via = OBJECT(pci_new_multifunction(PCI_DEVFN(12, 0), TYPE_VT8231_ISA));
+qdev_prop_set_string(DEVICE(via), "audiodev",
+ audio_maybe_init_dummy("pegasos2.defaudio"));
+pci_realize_and_unref(PCI_DEVICE(via), pci_bus, &error_abort);
+


Do not add blank line here, the rest is still part of via init.


for (i = 0; i < PCI_NUM_PINS; i++) {
pm->via_pirq[i] = qdev_get_gpio_in_named(DEVICE(via), "pirq", i);
}
@@ -564,6 +568,8 @@ static void pegasos2_machine_class_init(ObjectClass *oc, 
void *data)
vhc->encode_hpt_for_kvm_pr = vhyp_encode_hpt_for_kvm_pr;

vmc->setprop = pegasos2_setprop;
+
+machine_add_audiodev_property(mc);
}

static const TypeInfo pegasos2_machine_info = {
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
index 1e615b8d355..8c3abc67131 100644
--- a/hw/ppc/sam460ex.c
+++ b/hw/ppc/sam460ex.c
@@ -530,6 +530,8 @@ static void sam460ex_machine_init(MachineClass *mc)
mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
mc->default_ram_size = 512 * MiB;
mc->default_ram_id = "ppc4xx.sdram";
+
+machine_add_audiodev_property(mc);


This hunk has nothing to do with vt82686 so probably should be in 
previoius patch. Also sam460ex embedded sound part is not emulated and can 
only use PCI sound cards. What this line is needed for? If every machine 
now needs this call, can it be added in some generic machine init func in 
hw/core/machine.c instead?


I'm not sure about this whole series. Looks like it gets rid of 71 lines 
but adding 158 and makes the users' life harder by requireing them to 
specify drivers they may not even know about. What's the point? Is there 
still a default to use the normally used audiodev for the platform or 
people will now get errors and no sound unless they change their command 
lines?


Regards,
BALATON Zoltan


}

DEFINE_MACHINE("sam460ex", sam460ex_machine_init)

[PATCH RESEND 2/2] i386: Exclude 'hv-syndbg' from 'hv-passthrough'

[Vitaly Kuznetsov]

Windows with Hyper-V role enabled doesn't boot with 'hv-passthrough' when
no debugger is configured, this significantly limits the usefulness of the
feature as there's no support for subtracting Hyper-V features from CPU
flags at this moment (e.g. "-cpu host,hv-passthrough,-hv-syndbg" does not
work). While this is also theoretically fixable, 'hv-syndbg' is likely
very special and unneeded in the default set. Genuine Hyper-V doesn't seem
to enable it either.

Introduce 'skip_passthrough' flag to 'kvm_hyperv_properties' and use it as
one-off to skip 'hv-syndbg' when enabling features in 'hv-passthrough'
mode. Note, "-cpu host,hv-passthrough,hv-syndbg" can still be used if
needed.

As both 'hv-passthrough' and 'hv-syndbg' are debug features, the change
should not have any effect on production environments.

Signed-off-by: Vitaly Kuznetsov 
---
 docs/system/i386/hyperv.rst | 13 +
 target/i386/kvm/kvm.c   |  7 +--
 2 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/docs/system/i386/hyperv.rst b/docs/system/i386/hyperv.rst
index 2505dc4c86e0..009947e39141 100644
--- a/docs/system/i386/hyperv.rst
+++ b/docs/system/i386/hyperv.rst
@@ -262,14 +262,19 @@ Supplementary features
 ``hv-passthrough``
   In some cases (e.g. during development) it may make sense to use QEMU in
   'pass-through' mode and give Windows guests all enlightenments currently
-  supported by KVM. This pass-through mode is enabled by "hv-passthrough" CPU
-  flag.
+  supported by KVM.
 
   Note: ``hv-passthrough`` flag only enables enlightenments which are known to 
QEMU
   (have corresponding 'hv-' flag) and copies ``hv-spinlocks`` and 
``hv-vendor-id``
   values from KVM to QEMU. ``hv-passthrough`` overrides all other 'hv-' 
settings on
-  the command line. Also, enabling this flag effectively prevents migration as 
the
-  list of enabled enlightenments may differ between target and destination 
hosts.
+  the command line.
+
+  Note: ``hv-passthrough`` does not enable ``hv-syndbg`` which can prevent 
certain
+  Windows guests from booting when used without proper configuration. If 
needed,
+  ``hv-syndbg`` can be enabled additionally.
+
+  Note: ``hv-passthrough`` effectively prevents migration as the list of 
enabled
+  enlightenments may differ between target and destination hosts.
 
 ``hv-enforce-cpuid``
   By default, KVM allows the guest to use all currently supported Hyper-V
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 51b381a2fbbc..cfb24ba87df5 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -871,6 +871,7 @@ static struct {
 uint32_t bits;
 } flags[2];
 uint64_t dependencies;
+bool skip_passthrough;
 } kvm_hyperv_properties[] = {
 [HYPERV_FEAT_RELAXED] = {
 .desc = "relaxed timing (hv-relaxed)",
@@ -999,7 +1000,8 @@ static struct {
 {.func = HV_CPUID_FEATURES, .reg = R_EDX,
  .bits = HV_FEATURE_DEBUG_MSRS_AVAILABLE}
 },
-.dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
+.dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED),
+.skip_passthrough = true,
 },
 [HYPERV_FEAT_MSR_BITMAP] = {
 .desc = "enlightened MSR-Bitmap (hv-emsr-bitmap)",
@@ -1408,7 +1410,8 @@ bool kvm_hyperv_expand_features(X86CPU *cpu, Error **errp)
  * hv_build_cpuid_leaf() uses this info to build guest CPUIDs.
  */
 for (feat = 0; feat < ARRAY_SIZE(kvm_hyperv_properties); feat++) {
-if (hyperv_feature_supported(cs, feat)) {
+if (hyperv_feature_supported(cs, feat) &&
+!kvm_hyperv_properties[feat].skip_passthrough) {
 cpu->hyperv_features |= BIT(feat);
 }
 }
-- 
2.41.0

[PATCH RESEND 1/2] i386: Fix conditional CONFIG_SYNDBG enablement

[Vitaly Kuznetsov]

Putting HYPERV_FEAT_SYNDBG entry under "#ifdef CONFIG_SYNDBG" in
'kvm_hyperv_properties' array is wrong: as HYPERV_FEAT_SYNDBG is not
the highest feature number, the result is an empty (zeroed) entry in
the array (and not a skipped entry!). hyperv_feature_supported() is
designed to check that all CPUID bits are set but for a zeroed
feature in 'kvm_hyperv_properties' it returns 'true' so QEMU considers
HYPERV_FEAT_SYNDBG as always supported, regardless of whether KVM host
actually supports it.

To fix the issue, leave HYPERV_FEAT_SYNDBG's definition in
'kvm_hyperv_properties' array, there's nothing wrong in having it defined
even when 'CONFIG_SYNDBG' is not set. Instead, put "hv-syndbg" CPU property
under '#ifdef CONFIG_SYNDBG' to alter the existing behavior when the flag
is silently skipped in !CONFIG_SYNDBG builds.

Leave an 'assert' sentinel in hyperv_feature_supported() making sure there
are no 'holes' or improperly defined features in 'kvm_hyperv_properties'.

Fixes: d8701185f40c ("hw: hyperv: Initial commit for Synthetic Debugging 
device")
Signed-off-by: Vitaly Kuznetsov 
---
 target/i386/cpu.c |  2 ++
 target/i386/kvm/kvm.c | 11 +++
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 2589c8e9294a..01c7e8414408 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7840,8 +7840,10 @@ static Property x86_cpu_properties[] = {
   HYPERV_FEAT_TLBFLUSH_DIRECT, 0),
 DEFINE_PROP_ON_OFF_AUTO("hv-no-nonarch-coresharing", X86CPU,
 hyperv_no_nonarch_cs, ON_OFF_AUTO_OFF),
+#ifdef CONFIG_SYNDBG
 DEFINE_PROP_BIT64("hv-syndbg", X86CPU, hyperv_features,
   HYPERV_FEAT_SYNDBG, 0),
+#endif
 DEFINE_PROP_BOOL("hv-passthrough", X86CPU, hyperv_passthrough, false),
 DEFINE_PROP_BOOL("hv-enforce-cpuid", X86CPU, hyperv_enforce_cpuid, false),
 
diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index af101fcdf6ff..51b381a2fbbc 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -993,7 +993,6 @@ static struct {
  .bits = HV_DEPRECATING_AEOI_RECOMMENDED}
 }
 },
-#ifdef CONFIG_SYNDBG
 [HYPERV_FEAT_SYNDBG] = {
 .desc = "Enable synthetic kernel debugger channel (hv-syndbg)",
 .flags = {
@@ -1002,7 +1001,6 @@ static struct {
 },
 .dependencies = BIT(HYPERV_FEAT_SYNIC) | BIT(HYPERV_FEAT_RELAXED)
 },
-#endif
 [HYPERV_FEAT_MSR_BITMAP] = {
 .desc = "enlightened MSR-Bitmap (hv-emsr-bitmap)",
 .flags = {
@@ -1254,6 +1252,13 @@ static bool hyperv_feature_supported(CPUState *cs, int 
feature)
 uint32_t func, bits;
 int i, reg;
 
+/*
+ * kvm_hyperv_properties needs to define at least one CPUID flag which
+ * must be used to detect the feature, it's hard to say whether it is
+ * supported or not otherwise.
+ */
+assert(kvm_hyperv_properties[feature].flags[0].func);
+
 for (i = 0; i < ARRAY_SIZE(kvm_hyperv_properties[feature].flags); i++) {
 
 func = kvm_hyperv_properties[feature].flags[i].func;
@@ -3483,13 +3488,11 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 kvm_msr_entry_add(cpu, HV_X64_MSR_TSC_EMULATION_STATUS,
   env->msr_hv_tsc_emulation_status);
 }
-#ifdef CONFIG_SYNDBG
 if (hyperv_feat_enabled(cpu, HYPERV_FEAT_SYNDBG) &&
 has_msr_hv_syndbg_options) {
 kvm_msr_entry_add(cpu, HV_X64_MSR_SYNDBG_OPTIONS,
   hyperv_syndbg_query_options());
 }
-#endif
 }
 if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VAPIC)) {
 kvm_msr_entry_add(cpu, HV_X64_MSR_APIC_ASSIST_PAGE,
-- 
2.41.0

[PATCH RESEND 0/2] i386: Fix Hyper-V Gen1 guests stuck on boot with 'hv-passthrough'

[Vitaly Kuznetsov]

Hyper-V Gen1 guests are getting stuck on boot when 'hv-passthrough' is
used. While 'hv-passthrough' is a debug only feature, this significantly
limit its usefullness. While debugging the problem, I found that there are
two loosely connected issues:
- 'hv-passthrough' enables 'hv-syndbg' and this is undesired.
- 'hv-syndbg's support by KVM is detected incorrectly when !CONFIG_SYNDBG.

Fix both issues; exclude 'hv-syndbg' from 'hv-passthrough' and don't allow
to turn on 'hv-syndbg' for !CONFIG_SYNDBG builds. 

Vitaly Kuznetsov (2):
  i386: Fix conditional CONFIG_SYNDBG enablement
  i386: Exclude 'hv-syndbg' from 'hv-passthrough'

 docs/system/i386/hyperv.rst | 13 +
 target/i386/cpu.c   |  2 ++
 target/i386/kvm/kvm.c   | 18 --
 3 files changed, 23 insertions(+), 10 deletions(-)

-- 
2.41.0

Re: [PATCH v3 19/19] target/riscv/cpu: move priv spec functions to tcg-cpu.c

[Philippe Mathieu-Daudé]

On 20/9/23 13:20, Daniel Henrique Barboza wrote:

Priv spec validation is TCG specific. Move it to the TCG accel class.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Andrew Jones 
---
  target/riscv/cpu.c | 38 --
  target/riscv/cpu.h |  2 --
  target/riscv/tcg/tcg-cpu.c | 38 ++
  3 files changed, 38 insertions(+), 40 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v6 2/2] tpm: add backend for mssim

[Stefan Berger]

On 9/22/23 02:00, Markus Armbruster wrote:

Found this cleaning out old mail, sorry for missing it until now!

I think we owe James a quick decision wether we're willing to take the
feature.  Stefan, thoughts?


I thought we discusses it back then. Does it handle snapshotting and 
migration correctly?


  Stefan



James Bottomley  writes:


From: James Bottomley 

The Microsoft Simulator (mssim) is the reference emulation platform
for the TCG TPM 2.0 specification.

https://github.com/Microsoft/ms-tpm-20-ref.git

It exports a fairly simple network socket based protocol on two
sockets, one for command (default 2321) and one for control (default
2322).  This patch adds a simple backend that can speak the mssim
protocol over the network.  It also allows the two sockets to be
specified on the command line.  The benefits are twofold: firstly it
gives us a backend that actually speaks a standard TPM emulation
protocol instead of the linux specific TPM driver format of the
current emulated TPM backend and secondly, using the microsoft
protocol, the end point of the emulator can be anywhere on the
network, facilitating the cloud use case where a central TPM service
can be used over a control network.

The implementation does basic control commands like power off/on, but
doesn't implement cancellation or startup.  The former because
cancellation is pretty much useless on a fast operating TPM emulator
and the latter because this emulator is designed to be used with OVMF
which itself does TPM startup and I wanted to validate that.

To run this, simply download an emulator based on the MS specification
(package ibmswtpm2 on openSUSE) and run it, then add these two lines
to the qemu command and it will use the emulator.

 -tpmdev mssim,id=tpm0 \
 -device tpm-crb,tpmdev=tpm0 \

to use a remote emulator replace the first line with

 -tpmdev 
"{'type':'mssim','id':'tpm0','command':{'type':inet,'host':'remote','port':'2321'}}"

tpm-tis also works as the backend.

Signed-off-by: James Bottomley 

[...]


diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..1398735956 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -270,6 +270,38 @@ available as a module (assuming a TPM 2 is passed through):
/sys/devices/LNXSYSTEM:00/LNXSYBUS:00/MSFT0101:00/tpm/tpm0/pcr-sha256/9
...
  
+The QEMU TPM Microsoft Simulator Device

+---
+
+The TCG provides a reference implementation for TPM 2.0 written by


Suggest to copy the cover letter's nice introductory paragraph here:

   The Microsoft Simulator (mssim) is the reference emulation platform
   for the TCG TPM 2.0 specification.

   It provides a reference implementation for TPM 2.0 written by


+Microsoft (See `ms-tpm-20-ref`_ on github).  The reference implementation
+starts a network server and listens for TPM commands on port 2321 and
+TPM Platform control commands on port 2322, although these can be
+altered.  The QEMU mssim TPM backend talks to this implementation.  By
+default it connects to the default ports on localhost:
+
+.. code-block:: console
+
+  qemu-system-x86_64  \
+-tpmdev mssim,id=tpm0 \
+-device tpm-crb,tpmdev=tpm0
+
+
+Although it can also communicate with a remote host, which must be
+specified as a SocketAddress via json on the command line for each of

Is the "via JSON" part in "must be specified ... on the command line"
correct?  I'd expect to be able to use dotted keys as well, like

 -tpmdev 
type=mssim,id=tpm0,command.type=inet,command.host=remote,command.port=2321',control.type=inet,control.host=remote,control.port=2322

Aside: I do recommend management applications stick to JSON.


+the command and control ports:
+
+.. code-block:: console
+
+  qemu-system-x86_64  \
+-tpmdev 
"{'type':'mssim','id':'tpm0','command':{'type':'inet','host':'remote','port':'2321'},'control':{'type':'inet','host':'remote','port':'2322'}}"
 \
+-device tpm-crb,tpmdev=tpm0
+
+
+The mssim backend supports snapshotting and migration, but the state
+of the Microsoft Simulator server must be preserved (or the server
+kept running) outside of QEMU for restore to be successful.
+
  The QEMU TPM emulator device
  
  
@@ -526,3 +558,6 @@ the following:
  
  .. _SWTPM protocol:

 https://github.com/stefanberger/swtpm/blob/master/man/man3/swtpm_ioctls.pod
+
+.. _ms-tpm-20-ref:
+   https://github.com/microsoft/ms-tpm-20-ref
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ed78a87ddd..12482368d0 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -731,6 +731,7 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
  unsigned int c = 0;
  TPMPassthroughOptions *tpo;
  TPMEmulatorOptions *teo;
+TPMmssimOptions *tmo;
  
  info_list = qmp_query_tpm(&err);

  if (err) {
@@ -764,6 +765,14 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict)
  teo = ti->options->u.emulator.data;
  monitor_printf(mon,

[PATCH] hw/acpi: changes towards enabling -Wshadow=local

[Ani Sinha]

Code changes in acpi that addresses all compiler complaints coming from enabling
-Wshadow flags. Enabling -Wshadow catches cases of local variables shadowing
other local variables or parameters. These makes the code confusing and/or adds
bugs that are difficult to catch.

The code is tested to build with and without the flag turned on.

CC: Markus Armbruster 
CC: Philippe Mathieu-Daude 
CC: m...@redhat.com
CC: imamm...@redhat.com
Message-Id: <87r0mqlf9x@pond.sub.org>
Signed-off-by: Ani Sinha 
---
 hw/acpi/cpu_hotplug.c | 25 +
 hw/i386/acpi-build.c  | 24 
 hw/smbios/smbios.c| 37 +++--
 3 files changed, 44 insertions(+), 42 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index ff14c3f410..634bbecb31 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -265,26 +265,27 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState 
*machine,
 
 /* build Processor object for each processor */
 for (i = 0; i < apic_ids->len; i++) {
-int apic_id = apic_ids->cpus[i].arch_id;
+int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+assert(cpu_apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
 
-dev = aml_processor(i, 0, 0, "CP%.02X", apic_id);
+dev = aml_processor(i, 0, 0, "CP%.02X", cpu_apic_id);
 
 method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call2(CPU_MAT_METHOD, aml_int(apic_id), aml_int(i))
+aml_return(aml_call2(CPU_MAT_METHOD,
+ aml_int(cpu_apic_id), aml_int(i))
 ));
 aml_append(dev, method);
 
 method = aml_method("_STA", 0, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id;
+aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(cpu_apic_id;
 aml_append(dev, method);
 
 method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(cpu_apic_id),
 aml_arg(0)))
 );
 aml_append(dev, method);
@@ -298,11 +299,11 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState 
*machine,
 /* Arg0 = APIC ID */
 method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
 for (i = 0; i < apic_ids->len; i++) {
-int apic_id = apic_ids->cpus[i].arch_id;
+int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
+if_ctx = aml_if(aml_equal(aml_arg(0), aml_int(cpu_apic_id)));
 aml_append(if_ctx,
-aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
+aml_notify(aml_name("CP%.02X", cpu_apic_id), aml_arg(1))
 );
 aml_append(method, if_ctx);
 }
@@ -319,13 +320,13 @@ void build_legacy_cpu_hotplug_aml(Aml *ctx, MachineState 
*machine,
 aml_varpackage(x86ms->apic_id_limit);
 
 for (i = 0, apic_idx = 0; i < apic_ids->len; i++) {
-int apic_id = apic_ids->cpus[i].arch_id;
+int cpu_apic_id = apic_ids->cpus[i].arch_id;
 
-for (; apic_idx < apic_id; apic_idx++) {
+for (; apic_idx < cpu_apic_id; apic_idx++) {
 aml_append(pkg, aml_int(0));
 }
 aml_append(pkg, aml_int(apic_ids->cpus[i].cpu ? 1 : 0));
-apic_idx = apic_id + 1;
+apic_idx = cpu_apic_id + 1;
 }
 aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
 aml_append(ctx, sb_scope);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4d2d40bab5..95199c8900 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1585,12 +1585,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 aml_append(dev, aml_name_decl("_UID", aml_int(bus_num)));
 aml_append(dev, aml_name_decl("_BBN", aml_int(bus_num)));
 if (pci_bus_is_cxl(bus)) {
-struct Aml *pkg = aml_package(2);
+struct Aml *aml_pkg = aml_package(2);
 
 aml_append(dev, aml_name_decl("_HID", aml_string("ACPI0016")));
-aml_append(pkg, aml_eisaid("PNP0A08"));
-aml_append(pkg, aml_eisaid("PNP0A03"));
-aml_append(dev, aml_name_decl("_CID", pkg));
+aml_append(aml_pkg, aml_eisaid("PNP0A08"));
+aml_append(aml_pkg, aml_eisaid("PNP0A03"));
+aml_append(dev, aml_name_decl("_CID", aml_pkg));
 build_cxl_osc_method(dev);
 } else if (pci_bus_is_express(bus)) {
 aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0A08")));
@@ -1783,14 +1783,14 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
 Object *pci_host = acpi_get_i38

Re: [PATCH v4] hw/i386/pc: improve physical address space bound check for 32-bit x86 systems

[Philippe Mathieu-Daudé]

On 22/9/23 14:00, Ani Sinha wrote:




On 22-Sep-2023, at 4:12 PM, Philippe Mathieu-Daudé  wrote:

On 22/9/23 06:16, Ani Sinha wrote:

32-bit x86 systems do not have a reserved memory for hole64. On those 32-bit
systems without PSE36 or PAE CPU features, hotplugging memory devices are not
supported by QEMU as QEMU always places hotplugged memory above 4 GiB boundary
which is beyond the physical address space of the processor. Linux guests also
does not support memory hotplug on those systems. Please see Linux
kernel commit b59d02ed08690 ("mm/memory_hotplug: disable the functionality
for 32b") for more details.
Therefore, the maximum limit of the guest physical address in the absence of
additional memory devices effectively coincides with the end of
"above 4G memory space" region for 32-bit x86 without PAE/PSE36. When users
configure additional memory devices, after properly accounting for the
additional device memory region to find the maximum value of the guest
physical address, the address will be outside the range of the processor's
physical address space.
This change adds improvements to take above into consideration.
For example, previously this was allowed:
$ ./qemu-system-x86_64 -cpu pentium -m size=10G
With this change now it is no longer allowed:
$ ./qemu-system-x86_64 -cpu pentium -m size=10G
qemu-system-x86_64: Address space limit 0x < 0x2bfff phys-bits too 
low (32)
However, the following are allowed since on both cases physical address
space of the processor is 36 bits:
$ ./qemu-system-x86_64 -cpu pentium2 -m size=10G
$ ./qemu-system-x86_64 -cpu pentium,pse36=on -m size=10G
For 32-bit, without PAE/PSE36, hotplugging additional memory is no longer 
allowed.
$ ./qemu-system-i386 -m size=1G,maxmem=3G,slots=2
qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
low (32)
$ ./qemu-system-i386 -machine q35 -m size=1G,maxmem=3G,slots=2
qemu-system-i386: Address space limit 0x < 0x1 phys-bits too 
low (32)
A new compatibility flag is introduced to make sure pc_max_used_gpa() keeps
returning the old value for machines 8.1 and older.
Therefore, the above is still allowed for older machine types in order to 
support
compatibility. Hence, the following still works:
$ ./qemu-system-i386 -machine pc-i440fx-8.1 -m size=1G,maxmem=3G,slots=2
$ ./qemu-system-i386 -machine pc-q35-8.1 -m size=1G,maxmem=3G,slots=2
Further, following is also allowed as with PSE36, the processor has 36-bit
address space:
$ ./qemu-system-i386 -cpu 486,pse36=on -m size=1G,maxmem=3G,slots=2
After calling CPUID with EAX=0x8001, all AMD64 compliant processors
have the longmode-capable-bit turned on in the extended feature flags (bit 29)
in EDX. The absence of CPUID longmode can be used to differentiate between
32-bit and 64-bit processors and is the recommended approach. QEMU takes this
approach elsewhere (for example, please see x86_cpu_realizefn()), With
this change, pc_max_used_gpa() also uses the same method to detect 32-bit
processors.
Unit tests are modified to not run 32-bit x86 tests that use memory hotplug.
Suggested-by: David Hildenbrand 
Signed-off-by: Ani Sinha 
---
  hw/i386/pc.c   | 31 ---
  hw/i386/pc_piix.c  |  4 
  hw/i386/pc_q35.c   |  2 ++
  include/hw/i386/pc.h   |  6 ++
  tests/qtest/bios-tables-test.c | 26 ++
  tests/qtest/numa-test.c|  7 ++-
  6 files changed, 64 insertions(+), 12 deletions(-)




diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 54838c0c41..2a689cf0bd 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -907,12 +907,37 @@ static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
  static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
  {
  X86CPU *cpu = X86_CPU(first_cpu);
+PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
+MachineState *ms = MACHINE(pcms);
+uint64_t devmem_start = 0;
+ram_addr_t devmem_size = 0;
  -/* 32-bit systems don't have hole64 thus return max CPU address */
-if (cpu->phys_bits <= 32) {
-return ((hwaddr)1 << cpu->phys_bits) - 1;
+/*
+ * 32-bit systems don't have hole64 but they might have a region for
+ * memory devices. Even if additional hotplugged memory devices might
+ * not be usable by most guest OSes, we need to still consider them for
+ * calculating the highest possible GPA so that we can properly report
+ * if someone configures them on a CPU that cannot possibly address them.
+ */
+if (!(cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
+/* 32-bit systems */
+if (!pcmc->broken_32bit_mem_addr_check) {


Nitpicking, code is simplified if you invert this condition check.


Maybe it reads better, but simplified? .. I am not so sure.


As:

-- >8 --
@@ -907,12 +907,35 @@ static uint64_t 
pc_get_cxl_range_end(PCMachineState *pcms)
 static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint

Re: [PATCH v6 2/2] tpm: add backend for mssim

[Daniel P . Berrangé]

On Fri, Sep 22, 2023 at 08:41:19AM -0400, Stefan Berger wrote:
> 
> On 9/22/23 02:00, Markus Armbruster wrote:
> > Found this cleaning out old mail, sorry for missing it until now!
> > 
> > I think we owe James a quick decision wether we're willing to take the
> > feature.  Stefan, thoughts?
> 
> I thought we discusses it back then. Does it handle snapshotting and
> migration correctly?

To quote the patch itself:

  +The mssim backend supports snapshotting and migration, but the state
  +of the Microsoft Simulator server must be preserved (or the server
  +kept running) outside of QEMU for restore to be successful.


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 v4] hw/i386/pc: improve physical address space bound check for 32-bit x86 systems

[Ani Sinha]

> On 22-Sep-2023, at 6:26 PM, Philippe Mathieu-Daudé  wrote:
> 
> On 22/9/23 14:00, Ani Sinha wrote:
>>> On 22-Sep-2023, at 4:12 PM, Philippe Mathieu-Daudé  
>>> wrote:
>>> 
>>> On 22/9/23 06:16, Ani Sinha wrote:
 32-bit x86 systems do not have a reserved memory for hole64. On those 
 32-bit
 systems without PSE36 or PAE CPU features, hotplugging memory devices are 
 not
 supported by QEMU as QEMU always places hotplugged memory above 4 GiB 
 boundary
 which is beyond the physical address space of the processor. Linux guests 
 also
 does not support memory hotplug on those systems. Please see Linux
 kernel commit b59d02ed08690 ("mm/memory_hotplug: disable the functionality
 for 32b") for more details.
 Therefore, the maximum limit of the guest physical address in the absence 
 of
 additional memory devices effectively coincides with the end of
 "above 4G memory space" region for 32-bit x86 without PAE/PSE36. When users
 configure additional memory devices, after properly accounting for the
 additional device memory region to find the maximum value of the guest
 physical address, the address will be outside the range of the processor's
 physical address space.
 This change adds improvements to take above into consideration.
 For example, previously this was allowed:
 $ ./qemu-system-x86_64 -cpu pentium -m size=10G
 With this change now it is no longer allowed:
 $ ./qemu-system-x86_64 -cpu pentium -m size=10G
 qemu-system-x86_64: Address space limit 0x < 0x2bfff phys-bits 
 too low (32)
 However, the following are allowed since on both cases physical address
 space of the processor is 36 bits:
 $ ./qemu-system-x86_64 -cpu pentium2 -m size=10G
 $ ./qemu-system-x86_64 -cpu pentium,pse36=on -m size=10G
 For 32-bit, without PAE/PSE36, hotplugging additional memory is no longer 
 allowed.
 $ ./qemu-system-i386 -m size=1G,maxmem=3G,slots=2
 qemu-system-i386: Address space limit 0x < 0x1 phys-bits 
 too low (32)
 $ ./qemu-system-i386 -machine q35 -m size=1G,maxmem=3G,slots=2
 qemu-system-i386: Address space limit 0x < 0x1 phys-bits 
 too low (32)
 A new compatibility flag is introduced to make sure pc_max_used_gpa() keeps
 returning the old value for machines 8.1 and older.
 Therefore, the above is still allowed for older machine types in order to 
 support
 compatibility. Hence, the following still works:
 $ ./qemu-system-i386 -machine pc-i440fx-8.1 -m size=1G,maxmem=3G,slots=2
 $ ./qemu-system-i386 -machine pc-q35-8.1 -m size=1G,maxmem=3G,slots=2
 Further, following is also allowed as with PSE36, the processor has 36-bit
 address space:
 $ ./qemu-system-i386 -cpu 486,pse36=on -m size=1G,maxmem=3G,slots=2
 After calling CPUID with EAX=0x8001, all AMD64 compliant processors
 have the longmode-capable-bit turned on in the extended feature flags (bit 
 29)
 in EDX. The absence of CPUID longmode can be used to differentiate between
 32-bit and 64-bit processors and is the recommended approach. QEMU takes 
 this
 approach elsewhere (for example, please see x86_cpu_realizefn()), With
 this change, pc_max_used_gpa() also uses the same method to detect 32-bit
 processors.
 Unit tests are modified to not run 32-bit x86 tests that use memory 
 hotplug.
 Suggested-by: David Hildenbrand 
 Signed-off-by: Ani Sinha 
 ---
  hw/i386/pc.c   | 31 ---
  hw/i386/pc_piix.c  |  4 
  hw/i386/pc_q35.c   |  2 ++
  include/hw/i386/pc.h   |  6 ++
  tests/qtest/bios-tables-test.c | 26 ++
  tests/qtest/numa-test.c|  7 ++-
  6 files changed, 64 insertions(+), 12 deletions(-)
>>> 
>>> 
 diff --git a/hw/i386/pc.c b/hw/i386/pc.c
 index 54838c0c41..2a689cf0bd 100644
 --- a/hw/i386/pc.c
 +++ b/hw/i386/pc.c
 @@ -907,12 +907,37 @@ static uint64_t pc_get_cxl_range_end(PCMachineState 
 *pcms)
  static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t 
 pci_hole64_size)
  {
  X86CPU *cpu = X86_CPU(first_cpu);
 +PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
 +MachineState *ms = MACHINE(pcms);
 +uint64_t devmem_start = 0;
 +ram_addr_t devmem_size = 0;
  -/* 32-bit systems don't have hole64 thus return max CPU address */
 -if (cpu->phys_bits <= 32) {
 -return ((hwaddr)1 << cpu->phys_bits) - 1;
 +/*
 + * 32-bit systems don't have hole64 but they might have a region for
 + * memory devices. Even if additional hotplugged memory devices might
 + * not be usable by most guest OSes, we need to still consider them 
 for
 + * calculating the highest possible GPA so that w

Re: [PATCH 8/9] audio: Make AUD_register_card fallible and require audiodev=

[Philippe Mathieu-Daudé]

On 22/9/23 11:44, Paolo Bonzini wrote:

From: Martin Kletzander 

Now that all callers support error reporting with errp and all machine-default
devices use an explicit audiodev, this can be changed.  To make the detection
easier make AUD_register_card() return false on error.

Signed-off-by: Martin Kletzander 
Signed-off-by: Paolo Bonzini 
---
  audio/audio.c| 7 +--
  audio/audio.h| 2 +-
  hw/arm/omap2.c   | 2 +-
  hw/audio/ac97.c  | 6 +-
  hw/audio/adlib.c | 6 --
  hw/audio/cs4231a.c   | 6 --
  hw/audio/es1370.c| 5 -
  hw/audio/gus.c   | 6 --
  hw/audio/hda-codec.c | 5 -
  hw/audio/lm4549.c| 8 +---
  hw/audio/pcspk.c | 4 +---
  hw/audio/sb16.c  | 6 --
  hw/audio/via-ac97.c  | 6 --
  hw/audio/wm8750.c| 5 -
  hw/display/xlnx_dp.c | 6 --
  hw/input/tsc210x.c   | 2 +-
  hw/usb/dev-audio.c   | 5 -
  17 files changed, 59 insertions(+), 28 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v23 01/20] CPU topology: extend with s390 specifics

[Markus Armbruster]

Nina Schoetterl-Glausch  writes:

> On Wed, 2023-09-20 at 13:11 +0200, Markus Armbruster wrote:
>> Nina Schoetterl-Glausch  writes:
>> 
>> > From: Pierre Morel 
>> > 
>> > S390 adds two new SMP levels, drawers and books to the CPU
>> > topology.
>> > S390 CPUs have specific topology features like dedication and
>> > entitlement. These indicate to the guest information on host
>> > vCPU scheduling and help the guest make better scheduling decisions.
>> > 
>> > Let us provide the SMP properties with books and drawers levels
>> > and S390 CPU with dedication and entitlement,
>> > 
>> > Signed-off-by: Pierre Morel 
>> > Reviewed-by: Nina Schoetterl-Glausch 
>> > Co-developed-by: Nina Schoetterl-Glausch 
>> > Signed-off-by: Nina Schoetterl-Glausch 
>> > diff --git a/qapi/machine-common.json b/qapi/machine-common.json
>> > new file mode 100644
>> > index 00..e40421bb37
>> > --- /dev/null
>> > +++ b/qapi/machine-common.json
>> > @@ -0,0 +1,21 @@
>> > +# -*- Mode: Python -*-
>> > +# vim: filetype=python
>> > +#
>> > +# This work is licensed under the terms of the GNU GPL, version 2 or 
>> > later.
>> > +# See the COPYING file in the top-level directory.
>> > +
>> > +##
>> > +# = Machines S390 data types
>> > +##
>> > +
>> > +##
>> > +# @CpuS390Entitlement:
>> > +#
>> > +# An enumeration of cpu entitlements that can be assumed by a virtual
>> > +# S390 CPU
>> 
>> CPU entitlements
>> 
>> Would someone reasonably familiar with S390 understand this?  Because
>
> Well, someone familiar with s390 topology would, otherwise probably not tbh.

Good enough, I guess.

>> I'm not and I don't; I wonder what "a virtual CPU assuming an
>> entitlement" means.
>
> Basically, on s390x the OS is always running on some hypervisor.
> Even without KVM or z/VM you can slice up the machine, namely into logical
> partitions (LPARs). Therefore, there is a scheduling of virtual CPUs to the
> actual physical ones. "Entitlement" is a statement about how that scheduling
> works for a virtual CPU. The same concepts can then also be applied to KVM.

Thanks!

[...]

[PATCH] target/arm: Fix CNTPCT_EL0 trapping from EL0 when HCR_EL2.E2H is 0

[Michal Orzel]

On an attempt to access CNTPCT_EL0 from EL0 using a guest running on top
of Xen, a trap from EL2 was observed which is something not reproducible
on HW (also, Xen does not trap accesses to physical counter).

This is because gt_counter_access() checks for an incorrect bit (1
instead of 0) of CNTHCTL_EL2 if HCR_EL2.E2H is 0 and access is made to
physical counter. Refer ARM ARM DDI 0487J.a, D19.12.2:
When HCR_EL2.E2H is 0:
 - EL1PCTEN, bit [0]: refers to physical counter
 - EL1PCEN, bit [1]: refers to physical timer registers

Fix it by checking for the right bit (i.e. 0) and update the comment
referring to incorrect bit name.

Fixes: 5bc8437136fb ("target/arm: Update timer access for VHE")
Signed-off-by: Michal Orzel 
---
This is now in conformance to ARM ARM CNTPCT_EL0 pseudocode:
if PSTATE.EL == EL0 then
...
elif EL2Enabled() && HCR_EL2.E2H == '0' && CNTHCTL_EL2.EL1PCTEN == '0' then
AArch64.SystemAccessTrap(EL2, 0x18);
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 3b22596eabf3..3a2d77b3f81e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -2483,9 +2483,9 @@ static CPAccessResult gt_counter_access(CPUARMState *env, 
int timeridx,
 return CP_ACCESS_TRAP_EL2;
 }
 } else {
-/* If HCR_EL2. == 0: check CNTHCTL_EL2.EL1PCEN. */
+/* If HCR_EL2. == 0: check CNTHCTL_EL2.EL1PCTEN. */
 if (has_el2 && timeridx == GTIMER_PHYS &&
-!extract32(env->cp15.cnthctl_el2, 1, 1)) {
+!extract32(env->cp15.cnthctl_el2, 0, 1)) {
 return CP_ACCESS_TRAP_EL2;
 }
 }
-- 
2.25.1

Re: [Qemu-devel] [PATCH 08/19] aspeed/timer: Fix behaviour running Linux

[Cédric Le Goater]

Joel, Andrew,

On 5/25/19 17:12, Cédric Le Goater wrote:

From: Joel Stanley 

The Linux kernel driver was updated in commit 4451d3f59f2a
("clocksource/drivers/fttmr010: Fix set_next_event handler) to fix an
issue observed on hardware:

  > RELOAD register is loaded into COUNT register when the aspeed timer
  > is enabled, which means the next event may be delayed because timer
  > interrupt won't be generated until <0x - current_count +
  > cycles>.

When running under Qemu, the system appeared "laggy". The guest is now
scheduling timer events too regularly, starving the host of CPU time.

This patch modifies the timer model to attempt to schedule the timer
expiry as the guest requests, but if we have missed the deadline we
re interrupt and try again, which allows the guest to catch up.

Provides expected behaviour with old and new guest code.

Fixes: c04bd47db6b9 ("hw/timer: Add ASPEED timer device model")
Signed-off-by: Joel Stanley 
[clg: - merged a fix from Andrew Jeffery 
 "Fire interrupt on failure to meet deadline"
 https://lists.ozlabs.org/pipermail/openbmc/2019-January/014641.html
   - adapted commit log
   - checkpatch fixes ]
Signed-off-by: Cédric Le Goater 
---
  hw/timer/aspeed_timer.c | 59 ++---
  1 file changed, 31 insertions(+), 28 deletions(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c
index 5c786e512815..9ffd8e09f670 100644
--- a/hw/timer/aspeed_timer.c
+++ b/hw/timer/aspeed_timer.c
@@ -109,37 +109,40 @@ static inline uint64_t calculate_time(struct AspeedTimer 
*t, uint32_t ticks)
  
  static uint64_t calculate_next(struct AspeedTimer *t)

  {
-uint64_t next = 0;
-uint32_t rate = calculate_rate(t);
+uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+uint64_t next;
  
-while (!next) {

-/* We don't know the relationship between the values in the match
- * registers, so sort using MAX/MIN/zero. We sort in that order as the
- * timer counts down to zero. */
-uint64_t seq[] = {
-calculate_time(t, MAX(t->match[0], t->match[1])),
-calculate_time(t, MIN(t->match[0], t->match[1])),
-calculate_time(t, 0),
-};
-uint64_t reload_ns;
-uint64_t now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
-
-if (now < seq[0]) {
-next = seq[0];
-} else if (now < seq[1]) {
-next = seq[1];
-} else if (now < seq[2]) {
-next = seq[2];
-} else if (t->reload) {
-reload_ns = muldiv64(t->reload, NANOSECONDS_PER_SECOND, rate);
-t->start = now - ((now - t->start) % reload_ns);
-} else {
-/* no reload value, return 0 */
-break;
-}
+/*
+ * We don't know the relationship between the values in the match
+ * registers, so sort using MAX/MIN/zero. We sort in that order as
+ * the timer counts down to zero.
+ */
+
+next = calculate_time(t, MAX(t->match[0], t->match[1]));
+if (now < next) {
+return next;
+}
+
+next = calculate_time(t, MIN(t->match[0], t->match[1]));
+if (now < next) {
+return next;
+}
+
+next = calculate_time(t, 0);
+if (now < next) {
+return next;
+}
+
+/* We've missed all deadlines, fire interrupt and try again */
+timer_del(&t->timer);
+
+if (timer_overflow_interrupt(t)) {
+t->level = !t->level;
+qemu_set_irq(t->irq, t->level);
  }
  
-return next;

+t->start = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
+return calculate_time(t, MAX(MAX(t->match[0], t->match[1]), 0));


This MAX(MAX(x, y), 0) looks strange to me. Would you remember where it comes
from ? Thanks,

C.



  }
  
  static void aspeed_timer_mod(AspeedTimer *t)

Re: [PATCH v6 2/2] tpm: add backend for mssim

[Stefan Berger]

On 9/22/23 09:02, Daniel P. Berrangé wrote:

On Fri, Sep 22, 2023 at 08:41:19AM -0400, Stefan Berger wrote:

On 9/22/23 02:00, Markus Armbruster wrote:

Found this cleaning out old mail, sorry for missing it until now!

I think we owe James a quick decision wether we're willing to take the
feature.  Stefan, thoughts?

I thought we discusses it back then. Does it handle snapshotting and
migration correctly?

To quote the patch itself:

   +The mssim backend supports snapshotting and migration, but the state
   +of the Microsoft Simulator server must be preserved (or the server
   +kept running) outside of QEMU for restore to be successful.


How does 'it' support snapshotting where the state of the TPM can be 
completely different depending on the snapshot?  I know what it took to 
support this feature with swtpm/libtpms but I don't see the equivalent 
here in this backend driver nor in the TCG reference code that the 
underlying TPM 2 simulator is based upon.


I do not want to stand in the way of it being merged but please 
understand that I will also neither maintain nor fix bugs related to it 
nor its related underlying simulator -- with James being the maintainer 
of it, this should be clear. I have reason why I am saying this and they 
come from dealing with the upstream TPM 2 reference code.


Thanks,

  Stefan




With regards,
Daniel

Re: [PATCH 6/9] vt82c686: Support machine-default audiodev with fallback

[Paolo Bonzini]

On Fri, Sep 22, 2023 at 2:17 PM BALATON Zoltan  wrote:
> > mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("460exb");
> > mc->default_ram_size = 512 * MiB;
> > mc->default_ram_id = "ppc4xx.sdram";
> > +
> > +machine_add_audiodev_property(mc);
>
> This hunk has nothing to do with vt82686 so probably should be in
> previoius patch. Also sam460ex embedded sound part is not emulated and can
> only use PCI sound cards. What this line is needed for?

No, this line shouldn't be there.

> If every machine
> now needs this call, can it be added in some generic machine init func in
> hw/core/machine.c instead?

It is not needed by every machine, only by every machine that has
embedded sound.

> I'm not sure about this whole series. Looks like it gets rid of 71 lines
> but adding 158 and makes the users' life harder by requireing them to
> specify drivers they may not even know about. What's the point? Is there
> still a default to use the normally used audiodev for the platform or
> people will now get errors and no sound unless they change their command
> lines?

I think you're right, I should have sent this series without the last
two patches.

The first seven add more functionality, because they let you use
-audiodev for configuration of embedded boards. This is why they add
some lines of code.

The last two patches instead are the ones that make -audio or
-audiodev mandatory. They should be separate and they may not be a
good idea without something like "-audio default". And if no "-audio
default" is added, there is more code that can go (for example the
--audio-drv-list option to configure and CONFIG_AUDIO_DRIVERS).

Paolo

Re: [PATCH 3/3] hw/arm/smmuv3: Advertise SMMUv3.1-XNX feature

[Mostafa Saleh]

Hi Peter,

On Fri, Sep 22, 2023 at 11:54:06AM +0100, Peter Maydell wrote:
> On Fri, 22 Sept 2023 at 11:34, Mostafa Saleh  wrote:
> >
> > Hi Peter,
> >
> > On Thu, Sep 14, 2023 at 03:57:05PM +0100, Peter Maydell wrote:
> > > The SMMUv3.1-XNX feature is mandatory for an SMMUv3.1 if S2P is
> > > supported, so we should theoretically have implemented it as part of
> > > the recent S2P work.  Fortunately, for us the implementation is a
> > > no-op.
> > >
> > > This feature is about interpretation of the stage 2 page table
> > > descriptor XN bits, which control execute permissions.
> > >
> > > For QEMU, the permission bits passed to an IOMMU (via MemTxAttrs and
> > > IOMMUAccessFlags) only indicate read and write; we do not distinguish
> > > data reads from instruction reads outside the CPU proper.  In the
> > > SMMU architecture's terms, our interconnect between the client device
> > > and the SMMU doesn't have the ability to convey the INST attribute,
> > > and we therefore use the default value of "data" for this attribute.
> > >
> > > We also do not support the bits in the Stream Table Entry that can
> > > override the on-the-bus transaction attribute permissions (we do not
> > > set SMMU_IDR1.ATTR_PERMS_OVR=1).
> > >
> > > These two things together mean that for our implementation, it never
> > > has to deal with transactions with the INST attribute, and so it can
> > > correctly ignore the XN bits entirely.  So we already implement
> > > FEAT_XNX's "XN field is now 2 bits, not 1" behaviour to the extent
> > > that we need to.
> > >
> > > Advertise the presence of the feature in SMMU_IDR3.XNX.
> > >
> > > Signed-off-by: Peter Maydell 
> > > ---
> > >  hw/arm/smmuv3.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> > > index 94d388fc950..d9e639f7c41 100644
> > > --- a/hw/arm/smmuv3.c
> > > +++ b/hw/arm/smmuv3.c
> > > @@ -279,6 +279,7 @@ static void smmuv3_init_regs(SMMUv3State *s)
> > >  s->idr[1] = FIELD_DP32(s->idr[1], IDR1, CMDQS,   SMMU_CMDQS);
> > >
> > >  s->idr[3] = FIELD_DP32(s->idr[3], IDR3, HAD, 1);
> > > +s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
> > May be this can be guarded when S2P is present? according the UM
> > "In SMMUv3.1 and later, support for this feature is mandatory when
> > stage 2 is supported, that is when SMMU_IDR0.S2P == 1."
> > So I am not sure what it would mean for XNX and S1P only.
> 
> Mmm, I don't suppose it would confuse any guest code, but
> it's probably safest to put in the if():
> 
>if (FIELD_EX32(s->idr[0], IDR0, S2P) {
>/* XNX is a stage-2-specific feature */
>s->idr[3] = FIELD_DP32(s->idr[3], IDR3, XNX, 1);
>}

I agree that shouldn't confuse guests. I don't have a strong opinion,
both are OK for me.

> thanks
> -- PMM

Thanks
Mostafa

Re: [PATCH v2 09/12] hw/pci-host/gpex: Define properties for MMIO ranges

[Sunil V L]

Hi Igor,

On Thu, Aug 24, 2023 at 07:59:39PM +0530, Sunil V L wrote:
> ACPI DSDT generator needs information like ECAM range, PIO range, 32-bit
> and 64-bit PCI MMIO range etc related to the PCI host bridge. Instead of
> making these values machine specific, create properties for the GPEX host
> bridge with default value 0. During initialization, the firmware can
> initialize these properties with correct values for the platform. This
> basically allows DSDT generator code independent of the machine specific
> memory map accesses.
> 
> Suggested-by: Igor Mammedov 
> Signed-off-by: Sunil V L 
> ---

Could you please help reviewing these changes? From MAINTAINERS, it is
not clear to me who maintains gpex code.

Thanks!
Sunil
>  hw/pci-host/gpex-acpi.c| 13 +
>  hw/pci-host/gpex.c | 12 
>  include/hw/pci-host/gpex.h | 28 
>  3 files changed, 45 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci-host/gpex-acpi.c b/hw/pci-host/gpex-acpi.c
> index 7c7316bc96..0983af3d9e 100644
> --- a/hw/pci-host/gpex-acpi.c
> +++ b/hw/pci-host/gpex-acpi.c
> @@ -281,3 +281,16 @@ void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig 
> *cfg)
>  
>  crs_range_set_free(&crs_range_set);
>  }
> +
> +void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq)
> +{
> +bool ambig;
> +Object *obj = object_resolve_path_type("", TYPE_GPEX_HOST, &ambig);
> +
> +if (!obj || ambig) {
> +return;
> +}
> +
> +GPEX_HOST(obj)->gpex_cfg.irq = irq;
> +acpi_dsdt_add_gpex(scope, &GPEX_HOST(obj)->gpex_cfg);
> +}
> diff --git a/hw/pci-host/gpex.c b/hw/pci-host/gpex.c
> index a6752fac5e..41f4e73f6e 100644
> --- a/hw/pci-host/gpex.c
> +++ b/hw/pci-host/gpex.c
> @@ -154,6 +154,18 @@ static Property gpex_host_properties[] = {
>   */
>  DEFINE_PROP_BOOL("allow-unmapped-accesses", GPEXHost,
>   allow_unmapped_accesses, true),
> +DEFINE_PROP_UINT64(PCI_HOST_ECAM_BASE, GPEXHost, gpex_cfg.ecam.base, 0),
> +DEFINE_PROP_SIZE(PCI_HOST_ECAM_SIZE, GPEXHost, gpex_cfg.ecam.size, 0),
> +DEFINE_PROP_UINT64(PCI_HOST_PIO_BASE, GPEXHost, gpex_cfg.pio.base, 0),
> +DEFINE_PROP_SIZE(PCI_HOST_PIO_SIZE, GPEXHost, gpex_cfg.pio.size, 0),
> +DEFINE_PROP_UINT64(PCI_HOST_BELOW_4G_MMIO_BASE, GPEXHost,
> +   gpex_cfg.mmio32.base, 0),
> +DEFINE_PROP_SIZE(PCI_HOST_BELOW_4G_MMIO_SIZE, GPEXHost,
> + gpex_cfg.mmio32.size, 0),
> +DEFINE_PROP_UINT64(PCI_HOST_ABOVE_4G_MMIO_BASE, GPEXHost,
> +   gpex_cfg.mmio64.base, 0),
> +DEFINE_PROP_SIZE(PCI_HOST_ABOVE_4G_MMIO_SIZE, GPEXHost,
> + gpex_cfg.mmio64.size, 0),
>  DEFINE_PROP_END_OF_LIST(),
>  };
>  
> diff --git a/include/hw/pci-host/gpex.h b/include/hw/pci-host/gpex.h
> index b0240bd768..441c6b8b20 100644
> --- a/include/hw/pci-host/gpex.h
> +++ b/include/hw/pci-host/gpex.h
> @@ -40,6 +40,15 @@ struct GPEXRootState {
>  /*< public >*/
>  };
>  
> +struct GPEXConfig {
> +MemMapEntry ecam;
> +MemMapEntry mmio32;
> +MemMapEntry mmio64;
> +MemMapEntry pio;
> +int irq;
> +PCIBus  *bus;
> +};
> +
>  struct GPEXHost {
>  /*< private >*/
>  PCIExpressHost parent_obj;
> @@ -55,19 +64,22 @@ struct GPEXHost {
>  int irq_num[GPEX_NUM_IRQS];
>  
>  bool allow_unmapped_accesses;
> -};
>  
> -struct GPEXConfig {
> -MemMapEntry ecam;
> -MemMapEntry mmio32;
> -MemMapEntry mmio64;
> -MemMapEntry pio;
> -int irq;
> -PCIBus  *bus;
> +struct GPEXConfig gpex_cfg;
>  };
>  
>  int gpex_set_irq_num(GPEXHost *s, int index, int gsi);
>  
>  void acpi_dsdt_add_gpex(Aml *scope, struct GPEXConfig *cfg);
> +void acpi_dsdt_add_gpex_host(Aml *scope, uint32_t irq);
> +
> +#define PCI_HOST_PIO_BASE   "pio-base"
> +#define PCI_HOST_PIO_SIZE   "pio-size"
> +#define PCI_HOST_ECAM_BASE  "ecam-base"
> +#define PCI_HOST_ECAM_SIZE  "ecam-size"
> +#define PCI_HOST_BELOW_4G_MMIO_BASE "below-4g-mmio-base"
> +#define PCI_HOST_BELOW_4G_MMIO_SIZE "below-4g-mmio-size"
> +#define PCI_HOST_ABOVE_4G_MMIO_BASE "above-4g-mmio-base"
> +#define PCI_HOST_ABOVE_4G_MMIO_SIZE "above-4g-mmio-size"
>  
>  #endif /* HW_GPEX_H */
> -- 
> 2.39.2
> 

[PATCH 1/1] configure: support passthrough of -Dxxx args to meson

[Daniel P . Berrangé]

This can be useful for setting some meson global options, such as the
optimization level or debug state, which don't have an analogous
option explicitly defined in QEMU's configure wrapper script.

Signed-off-by: Daniel P. Berrangé 
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index e08127045d..cbd7e03e9f 100755
--- a/configure
+++ b/configure
@@ -931,6 +931,8 @@ cat << EOF
   bsd-userall BSD usermode emulation targets
   pie Position Independent Executables
 
+  -Dmesonoptname=val  passthrough option to meson unmodified
+
 NOTE: The object files are built at the place where configure is launched
 EOF
 exit 0
-- 
2.41.0

[PATCH 0/1] Improve _FORTIFY_SOURCE robustness

[Daniel P . Berrangé]

This ensures that we don't turn on _FORTIFY_SOURCE when the user
turns off optimizations via CFLAGS env variable. See patch 2 for
the full details.

Daniel P. Berrangé (1):
  configure: support passthrough of -Dxxx args to meson

 configure | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.41.0

[PATCH 2/2] osdep: set _FORTIFY_SOURCE=2 when optimization is enabled

[Daniel P . Berrangé]

Currently we set _FORTIFY_SOURCE=2 as a compiler argument when the
meson 'optimization' setting is non-zero, the compiler is GCC and
the target is Linux.

While the default QEMU optimization level is 2, user could override
this by setting CFLAGS="-O0" or --extra-cflags="-O0" when running
configure and this won't be reflected in the meson 'optimization'
setting. As a result we try to enable _FORTIFY_SOURCE=2 and then the
user gets compile errors as it only works with optimization.

Rather than trying to improve detection in meson, it is simpler to
just check the __OPTIMIZE__ define from osdep.h.

The comment about being incompatible with clang appears to be
outdated, as compilation works fine without excluding clang.

In the coroutine code we must set _FORTIFY_SOURCE=0 to stop the
logic in osdep.h then enabling it.

Signed-off-by: Daniel P. Berrangé 
---
 include/qemu/osdep.h |  4 
 meson.build  | 10 --
 util/coroutine-sigaltstack.c |  4 ++--
 util/coroutine-ucontext.c|  4 ++--
 4 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 2897720fac..b317649d13 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -27,6 +27,10 @@
 #ifndef QEMU_OSDEP_H
 #define QEMU_OSDEP_H
 
+#if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ && 
defined __linux__
+# define _FORTIFY_SOURCE 2
+#endif
+
 #include "config-host.h"
 #ifdef NEED_CPU_H
 #include CONFIG_TARGET
diff --git a/meson.build b/meson.build
index f426861d90..4947ae48b3 100644
--- a/meson.build
+++ b/meson.build
@@ -479,16 +479,6 @@ if 'cpp' in all_languages
   qemu_cxxflags = ['-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', 
'-D__STDC_FORMAT_MACROS'] + qemu_cflags
 endif
 
-# clang does not support glibc + FORTIFY_SOURCE (is it still true?)
-if get_option('optimization') != '0' and targetos == 'linux'
-  if cc.get_id() == 'gcc'
-qemu_cflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2']
-  endif
-  if 'cpp' in all_languages and cxx.get_id() == 'gcc'
-qemu_cxxflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2']
-  endif
-endif
-
 add_project_arguments(qemu_cflags, native: false, language: 'c')
 add_project_arguments(cc.get_supported_arguments(warn_flags), native: false, 
language: 'c')
 if 'cpp' in all_languages
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
index e2690c5f41..037d6416c4 100644
--- a/util/coroutine-sigaltstack.c
+++ b/util/coroutine-sigaltstack.c
@@ -22,9 +22,9 @@
  */
 
 /* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
-#ifdef _FORTIFY_SOURCE
 #undef _FORTIFY_SOURCE
-#endif
+#define _FORTIFY_SOURCE 0
+
 #include "qemu/osdep.h"
 #include 
 #include "qemu/coroutine_int.h"
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
index ddc98fb4f8..7b304c79d9 100644
--- a/util/coroutine-ucontext.c
+++ b/util/coroutine-ucontext.c
@@ -19,9 +19,9 @@
  */
 
 /* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
-#ifdef _FORTIFY_SOURCE
 #undef _FORTIFY_SOURCE
-#endif
+#define _FORTIFY_SOURCE 0
+
 #include "qemu/osdep.h"
 #include 
 #include "qemu/coroutine_int.h"
-- 
2.41.0

[PATCH 0/2] Improve _FORTIFY_SOURCE robustness

[Daniel P . Berrangé]

This ensures that we don't turn on _FORTIFY_SOURCE when the user
turns off optimizations via CFLAGS env variable. See patch 2 for
the full details.

Daniel P. Berrangé (2):
  configure: support passthrough of -Dxxx args to meson
  osdep: set _FORTIFY_SOURCE=2 when optimization is enabled

 configure|  2 ++
 include/qemu/osdep.h |  4 
 meson.build  | 10 --
 util/coroutine-sigaltstack.c |  4 ++--
 util/coroutine-ucontext.c|  4 ++--
 5 files changed, 10 insertions(+), 14 deletions(-)

-- 
2.41.0

[PATCH 1/2] configure: support passthrough of -Dxxx args to meson

[Daniel P . Berrangé]

This can be useful for setting some meson global options, such as the
optimization level or debug state, which don't have an analogous
option explicitly defined in QEMU's configure wrapper script.

Signed-off-by: Daniel P. Berrangé 
---
 configure | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/configure b/configure
index e08127045d..cbd7e03e9f 100755
--- a/configure
+++ b/configure
@@ -931,6 +931,8 @@ cat << EOF
   bsd-userall BSD usermode emulation targets
   pie Position Independent Executables
 
+  -Dmesonoptname=val  passthrough option to meson unmodified
+
 NOTE: The object files are built at the place where configure is launched
 EOF
 exit 0
-- 
2.41.0

Re: [PATCH 09/11] hw/rx/rx62n: Use qdev_prop_set_array()

[Markus Armbruster]

Kevin Wolf  writes:

> Instead of manually setting "foo-len" and "foo[i]" properties, build a
> QList and use the new qdev_prop_set_array() helper to set the whole
> array property with a single call.
>
> Signed-off-by: Kevin Wolf 
> ---
>  hw/rx/rx62n.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/hw/rx/rx62n.c b/hw/rx/rx62n.c
> index 3e887a0fc7..6990096642 100644
> --- a/hw/rx/rx62n.c
> +++ b/hw/rx/rx62n.c
> @@ -28,6 +28,7 @@
>  #include "hw/sysbus.h"
>  #include "hw/qdev-properties.h"
>  #include "sysemu/sysemu.h"
> +#include "qapi/qmp/qlist.h"
>  #include "qom/object.h"
>  
>  /*
> @@ -130,22 +131,22 @@ static void register_icu(RX62NState *s)
>  {
>  int i;
>  SysBusDevice *icu;
> +QList *ipr_map, *trigger_level;
>  
>  object_initialize_child(OBJECT(s), "icu", &s->icu, TYPE_RX_ICU);
>  icu = SYS_BUS_DEVICE(&s->icu);
> -qdev_prop_set_uint32(DEVICE(icu), "len-ipr-map", NR_IRQS);
> +
> +ipr_map = qlist_new();
>  for (i = 0; i < NR_IRQS; i++) {
> -char propname[32];
> -snprintf(propname, sizeof(propname), "ipr-map[%d]", i);
> -qdev_prop_set_uint32(DEVICE(icu), propname, ipr_table[i]);
> +qlist_append_int(ipr_map, ipr_table[i]);
>  }
> -qdev_prop_set_uint32(DEVICE(icu), "len-trigger-level",
> - ARRAY_SIZE(levelirq));
> +qdev_prop_set_array(sysctl, "ipr-map", ipr_map);

../hw/rx/rx62n.c:143:25: error: ‘sysctl’ undeclared (first use in this 
function); did you mean ‘syscall’?

Should be DEVICE(icu), I guess.

> +
> +trigger_level = qlist_new();
>  for (i = 0; i < ARRAY_SIZE(levelirq); i++) {
> -char propname[32];
> -snprintf(propname, sizeof(propname), "trigger-level[%d]", i);
> -qdev_prop_set_uint32(DEVICE(icu), propname, levelirq[i]);
> +qlist_append_int(trigger_level, levelirq[i]);
>  }
> +qdev_prop_set_array(sysctl, "trigger-level", trigger_level);

Again.

>  
>  for (i = 0; i < NR_IRQS; i++) {
>  s->irq[i] = qdev_get_gpio_in(DEVICE(icu), i);