[Stable-8.2.7 27/53] target/arm: Handle denormals correctly for FMOPA (widening)

[Michael Tokarev]

From: Peter Maydell 

The FMOPA (widening) SME instruction takes pairs of half-precision
floating point values, widens them to single-precision, does a
two-way dot product and accumulates the results into a
single-precision destination.  We don't quite correctly handle the
FPCR bits FZ and FZ16 which control flushing of denormal inputs and
outputs.  This is because at the moment we pass a single float_status
value to the helper function, which then uses that configuration for
all the fp operations it does.  However, because the inputs to this
operation are float16 and the outputs are float32 we need to use the
fp_status_f16 for the float16 input widening but the normal fp_status
for everything else.  Otherwise we will apply the flushing control
FPCR.FZ16 to the 32-bit output rather than the FPCR.FZ control, and
incorrectly flush a denormal output to zero when we should not (or
vice-versa).

(In commit 207d30b5fdb5b we tried to fix the FZ handling but
didn't get it right, switching from "use FPCR.FZ for everything" to
"use FPCR.FZ16 for everything".)
(Mjt: it is commit 4975f9fc4ea3 in stable-8.2)

Pass the CPU env to the sme_fmopa_h helper instead of an fp_status
pointer, and have the helper pass an extra fp_status into the
f16_dotadd() function so that we can use the right status for the
right parts of this operation.

Cc: qemu-sta...@nongnu.org
Fixes: 207d30b5fdb5 ("target/arm: Use FPST_F16 for SME FMOPA (widening)")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2373
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
(cherry picked from commit 55f9f4ee018c5ccea81d8c8c586756d7711ae46f)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/helper-sme.h b/target/arm/tcg/helper-sme.h
index 27eef49a11..d22bf9d21b 100644
--- a/target/arm/tcg/helper-sme.h
+++ b/target/arm/tcg/helper-sme.h
@@ -121,7 +121,7 @@ DEF_HELPER_FLAGS_5(sme_addha_d, TCG_CALL_NO_RWG, void, ptr, 
ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_5(sme_addva_d, TCG_CALL_NO_RWG, void, ptr, ptr, ptr, ptr, i32)
 
 DEF_HELPER_FLAGS_7(sme_fmopa_h, TCG_CALL_NO_RWG,
-   void, ptr, ptr, ptr, ptr, ptr, ptr, i32)
+   void, ptr, ptr, ptr, ptr, ptr, env, i32)
 DEF_HELPER_FLAGS_7(sme_fmopa_s, TCG_CALL_NO_RWG,
void, ptr, ptr, ptr, ptr, ptr, ptr, i32)
 DEF_HELPER_FLAGS_7(sme_fmopa_d, TCG_CALL_NO_RWG,
diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c
index f9001f5213..3906bb51c0 100644
--- a/target/arm/tcg/sme_helper.c
+++ b/target/arm/tcg/sme_helper.c
@@ -976,12 +976,23 @@ static inline uint32_t f16mop_adj_pair(uint32_t pair, 
uint32_t pg, uint32_t neg)
 }
 
 static float32 f16_dotadd(float32 sum, uint32_t e1, uint32_t e2,
-  float_status *s_std, float_status *s_odd)
+  float_status *s_f16, float_status *s_std,
+  float_status *s_odd)
 {
-float64 e1r = float16_to_float64(e1 & 0x, true, s_std);
-float64 e1c = float16_to_float64(e1 >> 16, true, s_std);
-float64 e2r = float16_to_float64(e2 & 0x, true, s_std);
-float64 e2c = float16_to_float64(e2 >> 16, true, s_std);
+/*
+ * We need three different float_status for different parts of this
+ * operation:
+ *  - the input conversion of the float16 values must use the
+ *f16-specific float_status, so that the FPCR.FZ16 control is applied
+ *  - operations on float32 including the final accumulation must use
+ *the normal float_status, so that FPCR.FZ is applied
+ *  - we have pre-set-up copy of s_std which is set to round-to-odd,
+ *for the multiply (see below)
+ */
+float64 e1r = float16_to_float64(e1 & 0x, true, s_f16);
+float64 e1c = float16_to_float64(e1 >> 16, true, s_f16);
+float64 e2r = float16_to_float64(e2 & 0x, true, s_f16);
+float64 e2c = float16_to_float64(e2 >> 16, true, s_f16);
 float64 t64;
 float32 t32;
 
@@ -1003,20 +1014,23 @@ static float32 f16_dotadd(float32 sum, uint32_t e1, 
uint32_t e2,
 }
 
 void HELPER(sme_fmopa_h)(void *vza, void *vzn, void *vzm, void *vpn,
- void *vpm, void *vst, uint32_t desc)
+ void *vpm, CPUARMState *env, uint32_t desc)
 {
 intptr_t row, col, oprsz = simd_maxsz(desc);
 uint32_t neg = simd_data(desc) * 0x80008000u;
 uint16_t *pn = vpn, *pm = vpm;
-float_status fpst_odd, fpst_std;
+float_status fpst_odd, fpst_std, fpst_f16;
 
 /*
- * Make a copy of float_status because this operation does not
- * update the cumulative fp exception status.  It also produces
- * default nans.  Make a second copy with round-to-odd -- see above.
+ * Make copies of fp_status and fp_status_f16, because this operation
+ * does not update the cumulative fp exception status.  It also
+ * produces default NaNs. We also need a second copy of fp_status with
+ * round-to-odd -- see above.
  */
-fpst_std = *(float_status *)vst;
+

[Stable-8.2.7 26/53] hw/arm/mps2-tz.c: fix RX/TX interrupts order

[Michael Tokarev]

From: Marco Palumbi 

The order of the RX and TX interrupts are swapped.
This commit fixes the order as per the following documents:
 * https://developer.arm.com/documentation/dai0505/latest/
 * https://developer.arm.com/documentation/dai0521/latest/
 * https://developer.arm.com/documentation/dai0524/latest/
 * https://developer.arm.com/documentation/dai0547/latest/

Cc: qemu-sta...@nongnu.org
Signed-off-by: Marco Palumbi 
Message-id: 20240730073123.72992-1-ma...@palumbi.it
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
(cherry picked from commit 5a558be93ad628e5bed6e0ee062870f49251725c)
Signed-off-by: Michael Tokarev 

diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
index 668db5ed61..9d9c263ef8 100644
--- a/hw/arm/mps2-tz.c
+++ b/hw/arm/mps2-tz.c
@@ -435,7 +435,7 @@ static MemoryRegion *make_uart(MPS2TZMachineState *mms, 
void *opaque,
const char *name, hwaddr size,
const int *irqs, const PPCExtraData *extradata)
 {
-/* The irq[] array is tx, rx, combined, in that order */
+/* The irq[] array is rx, tx, combined, in that order */
 MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
 CMSDKAPBUART *uart = opaque;
 int i = uart - &mms->uart[0];
@@ -447,8 +447,8 @@ static MemoryRegion *make_uart(MPS2TZMachineState *mms, 
void *opaque,
 qdev_prop_set_uint32(DEVICE(uart), "pclk-frq", mmc->apb_periph_frq);
 sysbus_realize(SYS_BUS_DEVICE(uart), &error_fatal);
 s = SYS_BUS_DEVICE(uart);
-sysbus_connect_irq(s, 0, get_sse_irq_in(mms, irqs[0]));
-sysbus_connect_irq(s, 1, get_sse_irq_in(mms, irqs[1]));
+sysbus_connect_irq(s, 0, get_sse_irq_in(mms, irqs[1]));
+sysbus_connect_irq(s, 1, get_sse_irq_in(mms, irqs[0]));
 sysbus_connect_irq(s, 2, qdev_get_gpio_in(orgate_dev, i * 2));
 sysbus_connect_irq(s, 3, qdev_get_gpio_in(orgate_dev, i * 2 + 1));
 sysbus_connect_irq(s, 4, get_sse_irq_in(mms, irqs[2]));
-- 
2.39.2

[Stable-8.2.7 33/53] vvfat: Fix usage of `info.file.offset`

[Michael Tokarev]

From: Amjad Alsharafi 

The field is marked as "the offset in the file (in clusters)", but it
was being used like this
`cluster_size*(nums)+mapping->info.file.offset`, which is incorrect.

Signed-off-by: Amjad Alsharafi 
Reviewed-by: Kevin Wolf 
Message-ID: 
<72f19a7903886dda1aa78bcae0e17702ee939262.1721470238.git.amjadsharaf...@gmail.com>
Signed-off-by: Kevin Wolf 
(cherry picked from commit 21b25a0e466a5bba0a45600bb8100ab564202ed1)
Signed-off-by: Michael Tokarev 

diff --git a/block/vvfat.c b/block/vvfat.c
index 19da009a5b..247b232608 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1408,7 +1408,9 @@ read_cluster_directory:
 
 assert(s->current_fd);
 
-
offset=s->cluster_size*(cluster_num-s->current_mapping->begin)+s->current_mapping->info.file.offset;
+offset = s->cluster_size *
+((cluster_num - s->current_mapping->begin)
++ s->current_mapping->info.file.offset);
 if(lseek(s->current_fd, offset, SEEK_SET)!=offset)
 return -3;
 s->cluster=s->cluster_buffer;
@@ -1929,8 +1931,9 @@ get_cluster_count_for_direntry(BDRVVVFATState* s, 
direntry_t* direntry, const ch
 (mapping->mode & MODE_DIRECTORY) == 0) {
 
 /* was modified in qcow */
-if (offset != mapping->info.file.offset + s->cluster_size
-* (cluster_num - mapping->begin)) {
+if (offset != s->cluster_size
+* ((cluster_num - mapping->begin)
++ mapping->info.file.offset)) {
 /* offset of this cluster in file chain has changed */
 abort();
 copy_it = 1;
@@ -2404,7 +2407,7 @@ static int commit_mappings(BDRVVVFATState* s,
 (mapping->end - mapping->begin);
 } else
 next_mapping->info.file.offset = mapping->info.file.offset +
-mapping->end - mapping->begin;
+(mapping->end - mapping->begin);
 
 mapping = next_mapping;
 }
-- 
2.39.2

[Stable-8.2.7 35/53] vvfat: Fix reading files with non-continuous clusters

[Michael Tokarev]

From: Amjad Alsharafi 

When reading with `read_cluster` we get the `mapping` with
`find_mapping_for_cluster` and then we call `open_file` for this
mapping.
The issue appear when its the same file, but a second cluster that is
not immediately after it, imagine clusters `500 -> 503`, this will give
us 2 mappings one has the range `500..501` and another `503..504`, both
point to the same file, but different offsets.

When we don't open the file since the path is the same, we won't assign
`s->current_mapping` and thus accessing way out of bound of the file.

>From our example above, after `open_file` (that didn't open anything) we
will get the offset into the file with
`s->cluster_size*(cluster_num-s->current_mapping->begin)`, which will
give us `0x2000 * (504-500)`, which is out of bound for this mapping and
will produce some issues.

Signed-off-by: Amjad Alsharafi 
Message-ID: 
<1f3ea115779abab62ba32c788073cdc99f9ad5dd.1721470238.git.amjadsharaf...@gmail.com>
[kwolf: Simplified the patch based on Amjad's analysis and input]
Signed-off-by: Kevin Wolf 
(cherry picked from commit 5eed3db336506b529b927ba221fe0d836e5b8819)
Signed-off-by: Michael Tokarev 

diff --git a/block/vvfat.c b/block/vvfat.c
index b63ac5d045..213d7989e4 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1369,8 +1369,9 @@ static int open_file(BDRVVVFATState* s,mapping_t* mapping)
 return -1;
 vvfat_close_current_file(s);
 s->current_fd = fd;
-s->current_mapping = mapping;
 }
+
+s->current_mapping = mapping;
 return 0;
 }
 
-- 
2.39.2

[Stable-8.2.7 52/53] hw/audio/virtio-snd: fix invalid param check

[Michael Tokarev]

From: Volker Rümelin 

Commit 9b6083465f ("virtio-snd: check for invalid param shift
operands") tries to prevent invalid parameters specified by the
guest. However, the code is not correct.

Change the code so that the parameters format and rate, which are
a bit numbers, are compared with the bit size of the data type.

Fixes: 9b6083465f ("virtio-snd: check for invalid param shift operands")
Signed-off-by: Volker Rümelin 
Message-Id: <20240802071805.7123-1-vr_q...@t-online.de>
Reviewed-by: Manos Pitsidianakis 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 7d14471a121878602cb4e748c4707f9ab9a9e3e2)
Signed-off-by: Michael Tokarev 

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index f0e7349c8a..63394cf5b0 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -282,12 +282,12 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
 error_report("Number of channels is not supported.");
 return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
 }
-if (BIT(params->format) > sizeof(supported_formats) ||
+if (params->format >= sizeof(supported_formats) * BITS_PER_BYTE ||
 !(supported_formats & BIT(params->format))) {
 error_report("Stream format is not supported.");
 return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
 }
-if (BIT(params->rate) > sizeof(supported_rates) ||
+if (params->rate >= sizeof(supported_rates) * BITS_PER_BYTE ||
 !(supported_rates & BIT(params->rate))) {
 error_report("Stream rate is not supported.");
 return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
-- 
2.39.2

[Stable-8.2.7 44/53] target/i386: Do not apply REX to MMX operands

[Michael Tokarev]

From: Richard Henderson 

Cc: qemu-sta...@nongnu.org
Fixes: b3e22b2318a ("target/i386: add core of new i386 decoder")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2495
Signed-off-by: Richard Henderson 
Link: 
https://lore.kernel.org/r/20240812025844.58956-2-richard.hender...@linaro.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 416f2b16c02c618c0f233372ebfe343f9ee667d4)
Signed-off-by: Michael Tokarev 

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index ffd3a42688..852579eef5 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -1237,7 +1237,10 @@ static bool decode_op(DisasContext *s, CPUX86State *env, 
X86DecodedInsn *decode,
 op->unit = X86_OP_SSE;
 }
 get_reg:
-op->n = ((get_modrm(s, env) >> 3) & 7) | REX_R(s);
+op->n = ((get_modrm(s, env) >> 3) & 7);
+if (op->unit != X86_OP_MMX) {
+op->n |= REX_R(s);
+}
 break;
 
 case X86_TYPE_E:  /* ALU modrm operand */
-- 
2.39.2

[Stable-8.2.7 46/53] module: Prevent crash by resetting local_err in module_load_qom_all()

[Michael Tokarev]

From: Alexander Ivanov 

Set local_err to NULL after it has been freed in error_report_err(). This
avoids triggering assert(*errp == NULL) failure in error_setv() when
local_err is reused in the loop.

Signed-off-by: Alexander Ivanov 
Reviewed-by: Claudio Fontana 
Reviewed-by: Denis V. Lunev 
Link: 
https://lore.kernel.org/r/20240809121340.992049-2-alexander.iva...@virtuozzo.com
[Do the same by moving the declaration instead. - Paolo]
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 940d802b24e63650e0eacad3714e2ce171cba17c)
Signed-off-by: Michael Tokarev 

diff --git a/util/module.c b/util/module.c
index 32e263163c..3eb0f06df1 100644
--- a/util/module.c
+++ b/util/module.c
@@ -354,13 +354,13 @@ int module_load_qom(const char *type, Error **errp)
 void module_load_qom_all(void)
 {
 const QemuModinfo *modinfo;
-Error *local_err = NULL;
 
 if (module_loaded_qom_all) {
 return;
 }
 
 for (modinfo = module_info; modinfo->name != NULL; modinfo++) {
+Error *local_err = NULL;
 if (!modinfo->objs) {
 continue;
 }
-- 
2.39.2

[Stable-8.2.7 41/53] nbd/server: CVE-2024-7409: Avoid use-after-free when closing server

[Michael Tokarev]

From: Eric Blake 

Commit 3e7ef738 plugged the use-after-free of the global nbd_server
object, but overlooked a use-after-free of nbd_server->listener.
Although this race is harder to hit, notice that our shutdown path
first drops the reference count of nbd_server->listener, then triggers
actions that can result in a pending client reaching the
nbd_blockdev_client_closed() callback, which in turn calls
qio_net_listener_set_client_func on a potentially stale object.

If we know we don't want any more clients to connect, and have already
told the listener socket to shut down, then we should not be trying to
update the listener socket's associated function.

Reproducer:

> #!/usr/bin/python3
>
> import os
> from threading import Thread
>
> def start_stop():
> while 1:
> os.system('virsh qemu-monitor-command VM \'{"execute": 
> "nbd-server-start",
+"arguments":{"addr":{"type":"unix","data":{"path":"/tmp/nbd-sock"\'')
> os.system('virsh qemu-monitor-command VM \'{"execute": 
> "nbd-server-stop"}\'')
>
> def nbd_list():
> while 1:
> os.system('/path/to/build/qemu-nbd -L -k /tmp/nbd-sock')
>
> def test():
> sst = Thread(target=start_stop)
> sst.start()
> nlt = Thread(target=nbd_list)
> nlt.start()
>
> sst.join()
> nlt.join()
>
> test()

Fixes: CVE-2024-7409
Fixes: 3e7ef738c8 ("nbd/server: CVE-2024-7409: Close stray clients at 
server-stop")
CC: qemu-sta...@nongnu.org
Reported-by: Andrey Drobyshev 
Signed-off-by: Eric Blake 
Message-ID: <20240822143617.800419-2-ebl...@redhat.com>
Reviewed-by: Stefan Hajnoczi 
(cherry picked from commit 3874f5f73c441c52f1c699c848d463b0eda01e4c)
Signed-off-by: Michael Tokarev 

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index f73409ae49..b36f41b7c5 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -92,10 +92,13 @@ static void nbd_accept(QIONetListener *listener, 
QIOChannelSocket *cioc,
 
 static void nbd_update_server_watch(NBDServerData *s)
 {
-if (!s->max_connections || s->connections < s->max_connections) {
-qio_net_listener_set_client_func(s->listener, nbd_accept, NULL, NULL);
-} else {
-qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
+if (s->listener) {
+if (!s->max_connections || s->connections < s->max_connections) {
+qio_net_listener_set_client_func(s->listener, nbd_accept, NULL,
+ NULL);
+} else {
+qio_net_listener_set_client_func(s->listener, NULL, NULL, NULL);
+}
 }
 }
 
@@ -113,6 +116,7 @@ static void nbd_server_free(NBDServerData *server)
  */
 qio_net_listener_disconnect(server->listener);
 object_unref(OBJECT(server->listener));
+server->listener = NULL;
 QLIST_FOREACH_SAFE(conn, &server->conns, next, tmp) {
 qio_channel_shutdown(QIO_CHANNEL(conn->cioc), 
QIO_CHANNEL_SHUTDOWN_BOTH,
  NULL);
-- 
2.39.2

[Stable-8.2.7 51/53] virtio-pci: Fix the use of an uninitialized irqfd

[Michael Tokarev]

From: Cindy Lu 

The crash was reported in MAC OS and NixOS, here is the link for this bug
https://gitlab.com/qemu-project/qemu/-/issues/2334
https://gitlab.com/qemu-project/qemu/-/issues/2321

In this bug, they are using the virtio_input device. The guest notifier was
not supported for this device, The function virtio_pci_set_guest_notifiers()
was not called, and the vector_irqfd was not initialized.

So the fix is adding the check for vector_irqfd in virtio_pci_get_notifier()

The function virtio_pci_get_notifier() can be used in various devices.
It could also be called when VIRTIO_CONFIG_S_DRIVER_OK is not set. In this 
situation,
the vector_irqfd being NULL is acceptable. We can allow the device continue to 
boot

If the vector_irqfd still hasn't been initialized after 
VIRTIO_CONFIG_S_DRIVER_OK
is set, it means that the function set_guest_notifiers was not called before the
driver started. This indicates that the device is not using the notifier.
At this point, we will let the check fail.

This fix is verified in vyatta,MacOS,NixOS,fedora system.

The bt tree for this bug is:
Thread 6 "CPU 0/KVM" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7c817be006c0 (LWP 1269146)]
kvm_virtio_pci_vq_vector_use () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:817
817 if (irqfd->users == 0) {
(gdb) thread apply all bt
...
Thread 6 (Thread 0x7c817be006c0 (LWP 1269146) "CPU 0/KVM"):
0  kvm_virtio_pci_vq_vector_use () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:817
1  kvm_virtio_pci_vector_use_one () at ../qemu-9.0.0/hw/virtio/virtio-pci.c:893
2  0x5983657045e2 in memory_region_write_accessor () at 
../qemu-9.0.0/system/memory.c:497
3  0x598365704ba6 in access_with_adjusted_size () at 
../qemu-9.0.0/system/memory.c:573
4  0x598365705059 in memory_region_dispatch_write () at 
../qemu-9.0.0/system/memory.c:1528
5  0x5983659b8e1f in flatview_write_continue_step.isra.0 () at 
../qemu-9.0.0/system/physmem.c:2713
6  0x59836570ba7d in flatview_write_continue () at 
../qemu-9.0.0/system/physmem.c:2743
7  flatview_write () at ../qemu-9.0.0/system/physmem.c:2774
8  0x59836570bb76 in address_space_write () at 
../qemu-9.0.0/system/physmem.c:2894
9  0x598365763afe in address_space_rw () at 
../qemu-9.0.0/system/physmem.c:2904
10 kvm_cpu_exec () at ../qemu-9.0.0/accel/kvm/kvm-all.c:2917
11 0x59836576656e in kvm_vcpu_thread_fn () at 
../qemu-9.0.0/accel/kvm/kvm-accel-ops.c:50
12 0x598365926ca8 in qemu_thread_start () at 
../qemu-9.0.0/util/qemu-thread-posix.c:541
13 0x7c8185bcd1cf in ??? () at /usr/lib/libc.so.6
14 0x7c8185c4e504 in clone () at /usr/lib/libc.so.6

Fixes: 2ce6cff94d ("virtio-pci: fix use of a released vector")
Cc: qemu-sta...@nongnu.org
Signed-off-by: Cindy Lu 
Message-Id: <20240806093715.65105-1-l...@redhat.com>
Acked-by: Jason Wang 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit a8e63ff289d137197ad7a701a587cc432872d798)
Signed-off-by: Michael Tokarev 

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 453861605e..1323e128e9 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -860,6 +860,9 @@ static int virtio_pci_get_notifier(VirtIOPCIProxy *proxy, 
int queue_no,
 VirtIODevice *vdev = virtio_bus_get_device(&proxy->bus);
 VirtQueue *vq;
 
+if (!proxy->vector_irqfd && vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)
+return -1;
+
 if (queue_no == VIRTIO_CONFIG_IRQ_IDX) {
 *n = virtio_config_get_guest_notifier(vdev);
 *vector = vdev->config_vector;
-- 
2.39.2

[Stable-8.2.7 53/53] target/hppa: Fix PSW V-bit packaging in cpu_hppa_get for hppa64

[Michael Tokarev]

From: Helge Deller 

While adding hppa64 support, the psw_v variable got extended from 32 to 64
bits.  So, when packaging the PSW-V bit from the psw_v variable for interrupt
processing, check bit 31 instead the 63th (sign) bit.

This fixes a hard to find Linux kernel boot issue where the loss of the PSW-V
bit due to an ITLB interruption in the middle of a series of ds/addc
instructions (from the divU milicode library) generated the wrong division
result and thus triggered a Linux kernel crash.

Link: 
https://lore.kernel.org/lkml/718b8afe-222f-4b3a-96d3-93af0e4ce...@roeck-us.net/
Reported-by: Guenter Roeck 
Signed-off-by: Helge Deller 
Reviewed-by: Richard Henderson 
Tested-by: Guenter Roeck 
Fixes: 931adff31478 ("target/hppa: Update cpu_hppa_get/put_psw for hppa64")
Cc: qemu-sta...@nongnu.org # v8.2+
(cherry picked from commit ead5078cf1a5f11d16e3e8462154c859620bcc7e)
Signed-off-by: Michael Tokarev 
(Mjt: context fixup in target/hppa/helper.c due to lack of
 v9.0.0-688-gebc9401a4067 "target/hppa: Split PSW X and B into their own field")

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 9556e95fab..e29e69dc31 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -188,7 +188,7 @@ typedef struct CPUArchState {
 
 target_ulong psw;/* All psw bits except the following:  */
 target_ulong psw_n;  /* boolean */
-target_long psw_v;   /* in most significant bit */
+target_long psw_v;   /* in bit 31 */
 
 /* Splitting the carry-borrow field into the MSB and "the rest", allows
  * for "the rest" to be deleted when it is unused, but the MSB is in use.
diff --git a/target/hppa/helper.c b/target/hppa/helper.c
index 859644c47a..9e35b65f29 100644
--- a/target/hppa/helper.c
+++ b/target/hppa/helper.c
@@ -53,7 +53,7 @@ target_ulong cpu_hppa_get_psw(CPUHPPAState *env)
 }
 
 psw |= env->psw_n * PSW_N;
-psw |= (env->psw_v < 0) * PSW_V;
+psw |= ((env->psw_v >> 31) & 1) * PSW_V;
 psw |= env->psw;
 
 return psw;
-- 
2.39.2

[Stable-8.2.7 47/53] target/hexagon: don't look for static glib

[Michael Tokarev]

From: Alyssa Ross 

When cross compiling QEMU configured with --static, I've been getting
configure errors like the following:

Build-time dependency glib-2.0 found: NO

../target/hexagon/meson.build:303:15: ERROR: Dependency lookup for glib-2.0 
with method 'pkgconfig' failed: Could not generate libs for glib-2.0:
Package libpcre2-8 was not found in the pkg-config search path.
Perhaps you should add the directory containing `libpcre2-8.pc'
to the PKG_CONFIG_PATH environment variable
Package 'libpcre2-8', required by 'glib-2.0', not found

This happens because --static sets the prefer_static Meson option, but
my build machine doesn't have a static libpcre2.  I don't think it
makes sense to insist that native dependencies are static, just
because I want the non-native QEMU binaries to be static.

Signed-off-by: Alyssa Ross 
Link: https://lore.kernel.org/r/20240805104921.4035256-1...@alyssa.is
Signed-off-by: Paolo Bonzini 
(cherry picked from commit fe68cc0923ebfa0c12e4176f61ec9b363a07a73a)
Signed-off-by: Michael Tokarev 

diff --git a/target/hexagon/meson.build b/target/hexagon/meson.build
index da8e608d00..436217f25a 100644
--- a/target/hexagon/meson.build
+++ b/target/hexagon/meson.build
@@ -188,7 +188,7 @@ if idef_parser_enabled and 'hexagon-linux-user' in 
target_dirs
 arguments: ['@INPUT@', '--defines=@OUTPUT1@', '--output=@OUTPUT0@']
 )
 
-glib_dep = dependency('glib-2.0', native: true)
+glib_dep = dependency('glib-2.0', native: true, static: false)
 
 idef_parser = executable(
 'idef-parser',
-- 
2.39.2

[Stable-8.2.7 36/53] iotests: Add `vvfat` tests

[Michael Tokarev]

From: Amjad Alsharafi 

Added several tests to verify the implementation of the vvfat driver.

We needed a way to interact with it, so created a basic `fat16.py` driver
that handled writing correct sectors for us.

Added `vvfat` to the non-generic formats, as its not a normal image format.

Signed-off-by: Amjad Alsharafi 
Reviewed-by: Kevin Wolf 
Tested-by: Kevin Wolf 
Message-ID: 

[kwolf: Made mypy and pylint happy to unbreak 297]
Signed-off-by: Kevin Wolf 
(cherry picked from commit c8f60bfb4345ea8343a53eaefe88d47b44c53f24)
Signed-off-by: Michael Tokarev 

diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
index 56d88ca423..545f9ec7bd 100755
--- a/tests/qemu-iotests/check
+++ b/tests/qemu-iotests/check
@@ -84,7 +84,7 @@ def make_argparser() -> argparse.ArgumentParser:
 p.set_defaults(imgfmt='raw', imgproto='file')
 
 format_list = ['raw', 'bochs', 'cloop', 'parallels', 'qcow', 'qcow2',
-   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg']
+   'qed', 'vdi', 'vpc', 'vhdx', 'vmdk', 'luks', 'dmg', 'vvfat']
 g_fmt = p.add_argument_group(
 '  image format options',
 'The following options set the IMGFMT environment variable. '
diff --git a/tests/qemu-iotests/fat16.py b/tests/qemu-iotests/fat16.py
new file mode 100644
index 00..7d2d052413
--- /dev/null
+++ b/tests/qemu-iotests/fat16.py
@@ -0,0 +1,690 @@
+# A simple FAT16 driver that is used to test the `vvfat` driver in QEMU.
+#
+# Copyright (C) 2024 Amjad Alsharafi 
+#
+# 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 .
+
+from typing import Callable, List, Optional, Protocol, Set
+import string
+
+SECTOR_SIZE = 512
+DIRENTRY_SIZE = 32
+ALLOWED_FILE_CHARS = set(
+"!#$%&'()-@^_`{}~" + string.digits + string.ascii_uppercase
+)
+
+
+class MBR:
+def __init__(self, data: bytes):
+assert len(data) == 512
+self.partition_table = []
+for i in range(4):
+partition = data[446 + i * 16 : 446 + (i + 1) * 16]
+self.partition_table.append(
+{
+"status": partition[0],
+"start_head": partition[1],
+"start_sector": partition[2] & 0x3F,
+"start_cylinder": ((partition[2] & 0xC0) << 2)
+  | partition[3],
+"type": partition[4],
+"end_head": partition[5],
+"end_sector": partition[6] & 0x3F,
+"end_cylinder": ((partition[6] & 0xC0) << 2)
+| partition[7],
+"start_lba": int.from_bytes(partition[8:12], "little"),
+"size": int.from_bytes(partition[12:16], "little"),
+}
+)
+
+def __str__(self):
+return "\n".join(
+[
+f"{i}: {partition}"
+for i, partition in enumerate(self.partition_table)
+]
+)
+
+
+class FatBootSector:
+# pylint: disable=too-many-instance-attributes
+def __init__(self, data: bytes):
+assert len(data) == 512
+self.bytes_per_sector = int.from_bytes(data[11:13], "little")
+self.sectors_per_cluster = data[13]
+self.reserved_sectors = int.from_bytes(data[14:16], "little")
+self.fat_count = data[16]
+self.root_entries = int.from_bytes(data[17:19], "little")
+total_sectors_16 = int.from_bytes(data[19:21], "little")
+self.media_descriptor = data[21]
+self.sectors_per_fat = int.from_bytes(data[22:24], "little")
+self.sectors_per_track = int.from_bytes(data[24:26], "little")
+self.heads = int.from_bytes(data[26:28], "little")
+self.hidden_sectors = int.from_bytes(data[28:32], "little")
+total_sectors_32 = int.from_bytes(data[32:36], "little")
+assert (
+total_sectors_16 == 0 or total_sectors_32 == 0
+), "Both total sectors (16 and 32) fields are non-zero"
+self.total_sectors = total_sectors_16 or total_sectors_32
+self.drive_number = data[36]
+self.volume_id = int.from_bytes(data[39:43], "little")
+self.volume_label = data[43:54].decode("ascii").strip()
+self.fs_type = data[54:62].decode("ascii").strip()
+
+def root_dir_start(self):
+"""
+Calculate the start sector of the root di

[Stable-8.2.7 16/53] hw/virtio: Fix the de-initialization of vhost-user devices

[Michael Tokarev]

From: Thomas Huth 

The unrealize functions of the various vhost-user devices are
calling the corresponding vhost_*_set_status() functions with a
status of 0 to shut down the device correctly.

Now these vhost_*_set_status() functions all follow this scheme:

bool should_start = virtio_device_should_start(vdev, status);

if (vhost_dev_is_started(&vvc->vhost_dev) == should_start) {
return;
}

if (should_start) {
/* ... do the initialization stuff ... */
} else {
/* ... do the cleanup stuff ... */
}

The problem here is virtio_device_should_start(vdev, 0) currently
always returns "true" since it internally only looks at vdev->started
instead of looking at the "status" parameter. Thus once the device
got started once, virtio_device_should_start() always returns true
and thus the vhost_*_set_status() functions return early, without
ever doing any clean-up when being called with status == 0. This
causes e.g. problems when trying to hot-plug and hot-unplug a vhost
user devices multiple times since the de-initialization step is
completely skipped during the unplug operation.

This bug has been introduced in commit 9f6bcfd99f ("hw/virtio: move
vm_running check to virtio_device_started") which replaced

 should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;

with

 should_start = virtio_device_started(vdev, status);

which later got replaced by virtio_device_should_start(). This blocked
the possibility to set should_start to false in case the status flag
VIRTIO_CONFIG_S_DRIVER_OK was not set.

Fix it by adjusting the virtio_device_should_start() function to
only consider the status flag instead of vdev->started. Since this
function is only used in the various vhost_*_set_status() functions
for exactly the same purpose, it should be fine to fix it in this
central place there without any risk to change the behavior of other
code.

Fixes: 9f6bcfd99f ("hw/virtio: move vm_running check to virtio_device_started")
Buglink: https://issues.redhat.com/browse/RHEL-40708
Signed-off-by: Thomas Huth 
Message-Id: <20240618121958.88673-1-th...@redhat.com>
Reviewed-by: Manos Pitsidianakis 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit d72479b11797c28893e1e3fc565497a9cae5ca16)
Signed-off-by: Michael Tokarev 

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..2eafad17b8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,9 +470,9 @@ static inline bool virtio_device_started(VirtIODevice 
*vdev, uint8_t status)
  * @vdev - the VirtIO device
  * @status - the devices status bits
  *
- * This is similar to virtio_device_started() but also encapsulates a
- * check on the VM status which would prevent a device starting
- * anyway.
+ * This is similar to virtio_device_started() but ignores vdev->started
+ * and also encapsulates a check on the VM status which would prevent a
+ * device from starting anyway.
  */
 static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t 
status)
 {
@@ -480,7 +480,7 @@ static inline bool virtio_device_should_start(VirtIODevice 
*vdev, uint8_t status
 return false;
 }
 
-return virtio_device_started(vdev, status);
+return status & VIRTIO_CONFIG_S_DRIVER_OK;
 }
 
 static inline void virtio_set_started(VirtIODevice *vdev, bool started)
-- 
2.39.2

Re: [RFC PATCH v2 0/5] vhost-user: Add SHMEM_MAP/UNMAP requests

[Albert Esteve]

On Thu, Sep 5, 2024 at 6:39 PM Stefan Hajnoczi  wrote:

> On Tue, Sep 03, 2024 at 10:42:34AM +0200, Albert Esteve wrote:
> > Hello all,
> >
> > Sorry, I have been a bit disconnected from this thread as I was on
> > vacations and then had to switch tasks for a while.
> >
> > I will try to go through all comments and address them for the first
> > non-RFC drop of this patch series.
> >
> > But I was discussing with some colleagues on this. So turns out
> rust-vmm's
> > vhost-user-gpu will potentially use
> > this soon, and a rust-vmm/vhost patch have been already posted:
> > https://github.com/rust-vmm/vhost/pull/251.
> > So I think it may make sense to:
> > 1. Split the vhost-user documentation patch once settled. Since it is
> taken
> > as the official spec,
> > having it upstreamed independently of the implementation will benefit
> > other projects to
> > work/integrate their own code.
> > 2. Split READ_/WRITE_MEM messages from SHMEM_MAP/_UNMAP patches.
> > If I remember correctly, this addresses a virtio-fs specific issue,
> > that will not
> > impact either virtio-gpu nor virtio-media, or any other.
>
> This is an architectural issue that arises from exposing VIRTIO Shared
> Memory Regions in vhost-user. It was first seen with Linux virtiofs but
> it could happen with other devices and/or guest operating systems.
>
> Any VIRTIO Shared Memory Region that can be mmapped into Linux userspace
> may trigger this issue. Userspace may write(2) to an O_DIRECT file with
> the mmap as the source. The vhost-user-blk device will not be able to
> access the source device's VIRTIO Shared Memory Region and will fail.
>
> > So it may make
> > sense
> > to separate them so that one does not stall the other. I will try to
> > have both
> > integrated in the mid term.
>
> If READ_/WRITE_MEM is a pain to implement (I think it is in the
> vhost-user back-end, even though I've been a proponent of it), then
> another way to deal with this issue is to specify that upon receiving
> MAP/UNMAP messages, the vhost-user front-end must update the vhost-user
> memory tables of all other vhost-user devices. That way vhost-user
> devices will be able to access VIRTIO Shared Memory Regions mapped by
> other devices.
>
> Implementing this in QEMU should be much easier than implementing
> READ_/WRITE_MEM support in device back-ends.
>
> This will be slow and scale poorly but performance is only a problem for
> devices that frequently MAP/UNMAP like virtiofs. Will virtio-gpu and
> virtio-media use MAP/UNMAP often at runtime? They might be able to get
> away with this simple solution.
>
> I'd be happy with that. If someone wants to make virtiofs DAX faster,
> they can implement READ/WRITE_MEM or another solution later, but let's
> at least make things correct from the start.
>

I agree. I want it to be correct first. If you agree on splitting the spec
bits from this
patch I'm already happy. I suggested splitting READ_/WRITE_MEM messages
because I thought that it was a virtiofs-specific issue.

The alternative that you proposed is interesting. I'll take it into
account. But I
feel I prefer to go for the better solution, and if I get too entangled,
then switch
to the easier implementation.

I think we could do this in 2 patches:
1. Split the documentation bits for SHMEM_MAP/_UNMAP. The
implementation for these messages will go into the second patch.
2. The implementation patch: keep going for the time being with
 READ_/WRITE_MEM support. And the documentation for that
is kept it within this patch. This way if we switch to the frontend
updating vhost-user memory table, we weren't set in any specific
solution if patch 1 has been already merged.

BR,
Albert.


>
> Stefan
>
> >
> > WDYT?
> >
> > BR,
> > Albert.
> >
> > On Tue, Jul 16, 2024 at 3:21 AM David Stevens 
> wrote:
> >
> > > On Fri, Jul 12, 2024 at 2:47 PM Michael S. Tsirkin 
> wrote:
> > > >
> > > > On Fri, Jul 12, 2024 at 11:06:49AM +0900, David Stevens wrote:
> > > > > On Thu, Jul 11, 2024 at 7:56 PM Alyssa Ross  wrote:
> > > > > >
> > > > > > Adding David Stevens, who implemented SHMEM_MAP and SHMEM_UNMAP
> in
> > > > > > crosvm a couple of years ago.
> > > > > >
> > > > > > David, I'd be particularly interested for your thoughts on the
> > > MEM_READ
> > > > > > and MEM_WRITE commands, since as far as I know crosvm doesn't
> > > implement
> > > > > > anything like that.  The discussion leading to those being added
> > > starts
> > > > > > here:
> > > > > >
> > > > > >
> > >
> https://lore.kernel.org/qemu-devel/20240604185416.gb90...@fedora.redhat.com/
> > > > > >
> > > > > > It would be great if this could be standardised between QEMU and
> > > crosvm
> > > > > > (and therefore have a clearer path toward being implemented in
> other
> > > VMMs)!
> > > > >
> > > > > Setting aside vhost-user for a moment, the DAX example given by
> Stefan
> > > > > won't work in crosvm today.
> > > > >
> > > > > Is universal access to virtio shared memory re

[Stable-8.2.7 42/53] hw/core/ptimer: fix timer zero period condition for freq > 1GHz

[Michael Tokarev]

From: Jianzhou Yue 

The real period is zero when both period and period_frac are zero.
Check the method ptimer_set_freq, if freq is larger than 1000 MHz,
the period is zero, but the period_frac is not, in this case, the
ptimer will work but the current code incorrectly recognizes that
the ptimer is disabled.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2306
Signed-off-by: JianZhou Yue 
Message-id: 3da024aea8b57545af1b3caa37077d0fb75e8...@shasxm03.verisilicon.com
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
(cherry picked from commit 446e5e8b4515e9a7be69ef6a29852975289bb6f0)
Signed-off-by: Michael Tokarev 

diff --git a/hw/core/ptimer.c b/hw/core/ptimer.c
index e03165febf..7177ecfab0 100644
--- a/hw/core/ptimer.c
+++ b/hw/core/ptimer.c
@@ -83,7 +83,7 @@ static void ptimer_reload(ptimer_state *s, int delta_adjust)
 delta = s->delta = s->limit;
 }
 
-if (s->period == 0) {
+if (s->period == 0 && s->period_frac == 0) {
 if (!qtest_enabled()) {
 fprintf(stderr, "Timer with period zero, disabling\n");
 }
@@ -309,7 +309,7 @@ void ptimer_run(ptimer_state *s, int oneshot)
 
 assert(s->in_transaction);
 
-if (was_disabled && s->period == 0) {
+if (was_disabled && s->period == 0 && s->period_frac == 0) {
 if (!qtest_enabled()) {
 fprintf(stderr, "Timer with period zero, disabling\n");
 }
diff --git a/tests/unit/ptimer-test.c b/tests/unit/ptimer-test.c
index 04b5f4e3d0..08240594bb 100644
--- a/tests/unit/ptimer-test.c
+++ b/tests/unit/ptimer-test.c
@@ -763,6 +763,33 @@ static void check_oneshot_with_load_0(gconstpointer arg)
 ptimer_free(ptimer);
 }
 
+static void check_freq_more_than_1000M(gconstpointer arg)
+{
+const uint8_t *policy = arg;
+ptimer_state *ptimer = ptimer_init(ptimer_trigger, NULL, *policy);
+bool no_round_down = (*policy & PTIMER_POLICY_NO_COUNTER_ROUND_DOWN);
+
+triggered = false;
+
+ptimer_transaction_begin(ptimer);
+ptimer_set_freq(ptimer, 20);
+ptimer_set_limit(ptimer, 8, 1);
+ptimer_run(ptimer, 1);
+ptimer_transaction_commit(ptimer);
+
+qemu_clock_step(3);
+
+g_assert_cmpuint(ptimer_get_count(ptimer), ==, no_round_down ? 3 : 2);
+g_assert_false(triggered);
+
+qemu_clock_step(1);
+
+g_assert_cmpuint(ptimer_get_count(ptimer), ==, 0);
+g_assert_true(triggered);
+
+ptimer_free(ptimer);
+}
+
 static void add_ptimer_tests(uint8_t policy)
 {
 char policy_name[256] = "";
@@ -857,6 +884,12 @@ static void add_ptimer_tests(uint8_t policy)
   policy_name),
 g_memdup2(&policy, 1), check_oneshot_with_load_0, g_free);
 g_free(tmp);
+
+g_test_add_data_func_full(
+tmp = g_strdup_printf("/ptimer/freq_more_than_1000M policy=%s",
+  policy_name),
+g_memdup2(&policy, 1), check_freq_more_than_1000M, g_free);
+g_free(tmp);
 }
 
 static void add_all_ptimer_policies_comb_tests(void)
-- 
2.39.2

[Stable-8.2.7 30/53] target/i386: Fix VSIB decode

[Michael Tokarev]

From: Richard Henderson 

With normal SIB, index == 4 indicates no index.
With VSIB, there is no exception for VR4/VR12.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2474
Signed-off-by: Richard Henderson 
Link: 
https://lore.kernel.org/r/20240805003130.1421051-3-richard.hender...@linaro.org
Cc: qemu-sta...@nongnu.org
Signed-off-by: Paolo Bonzini 
(cherry picked from commit ac63755b20013ec6a3d2aef4538d37dc90bc3d10)
Signed-off-by: Michael Tokarev 
(Mjt: modify the change to pre-new-decoder introduced past qemu 9.0)

diff --git a/target/i386/tcg/decode-new.c.inc b/target/i386/tcg/decode-new.c.inc
index 73aa2c42b7..ffd3a42688 100644
--- a/target/i386/tcg/decode-new.c.inc
+++ b/target/i386/tcg/decode-new.c.inc
@@ -1102,7 +1102,8 @@ static int decode_modrm(DisasContext *s, CPUX86State 
*env, X86DecodedInsn *decod
 } else {
 op->has_ea = true;
 op->n = -1;
-decode->mem = gen_lea_modrm_0(env, s, get_modrm(s, env));
+decode->mem = gen_lea_modrm_0(env, s, modrm,
+  decode->e.vex_class == 12);
 }
 return modrm;
 }
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 716a747df7..157348273e 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -2159,7 +2159,7 @@ typedef struct AddressParts {
 } AddressParts;
 
 static AddressParts gen_lea_modrm_0(CPUX86State *env, DisasContext *s,
-int modrm)
+int modrm, bool is_vsib)
 {
 int def_seg, base, index, scale, mod, rm;
 target_long disp;
@@ -2188,7 +2188,7 @@ static AddressParts gen_lea_modrm_0(CPUX86State *env, 
DisasContext *s,
 int code = x86_ldub_code(env, s);
 scale = (code >> 6) & 3;
 index = ((code >> 3) & 7) | REX_X(s);
-if (index == 4) {
+if (index == 4 && !is_vsib) {
 index = -1;  /* no index */
 }
 base = (code & 7) | REX_B(s);
@@ -2318,21 +2318,21 @@ static TCGv gen_lea_modrm_1(DisasContext *s, 
AddressParts a, bool is_vsib)
 
 static void gen_lea_modrm(CPUX86State *env, DisasContext *s, int modrm)
 {
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
+AddressParts a = gen_lea_modrm_0(env, s, modrm, false);
 TCGv ea = gen_lea_modrm_1(s, a, false);
 gen_lea_v_seg(s, s->aflag, ea, a.def_seg, s->override);
 }
 
 static void gen_nop_modrm(CPUX86State *env, DisasContext *s, int modrm)
 {
-(void)gen_lea_modrm_0(env, s, modrm);
+(void)gen_lea_modrm_0(env, s, modrm, false);
 }
 
 /* Used for BNDCL, BNDCU, BNDCN.  */
 static void gen_bndck(CPUX86State *env, DisasContext *s, int modrm,
   TCGCond cond, TCGv_i64 bndv)
 {
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
+AddressParts a = gen_lea_modrm_0(env, s, modrm, false);
 TCGv ea = gen_lea_modrm_1(s, a, false);
 
 tcg_gen_extu_tl_i64(s->tmp1_i64, ea);
@@ -4156,7 +4156,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
 goto illegal_op;
 reg = ((modrm >> 3) & 7) | REX_R(s);
 {
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
+AddressParts a = gen_lea_modrm_0(env, s, modrm, false);
 TCGv ea = gen_lea_modrm_1(s, a, false);
 gen_lea_v_seg(s, s->aflag, ea, -1, -1);
 gen_op_mov_reg_v(s, dflag, reg, s->A0);
@@ -4378,7 +4378,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
 op = ((b & 7) << 3) | ((modrm >> 3) & 7);
 if (mod != 3) {
 /* memory op */
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
+AddressParts a = gen_lea_modrm_0(env, s, modrm, false);
 TCGv ea = gen_lea_modrm_1(s, a, false);
 TCGv last_addr = tcg_temp_new();
 bool update_fdp = true;
@@ -5322,7 +5322,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
 rm = (modrm & 7) | REX_B(s);
 gen_op_mov_v_reg(s, MO_32, s->T1, reg);
 if (mod != 3) {
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
+AddressParts a = gen_lea_modrm_0(env, s, modrm, false);
 /* specific case: we need to add a displacement */
 gen_exts(ot, s->T1);
 tcg_gen_sari_tl(s->tmp0, s->T1, 3 + ot);
@@ -6318,7 +6318,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
 }
 } else if (mod != 3) {
 /* bndldx */
-AddressParts a = gen_lea_modrm_0(env, s, modrm);
+AddressParts a = gen_lea_modrm_0(env, s, modrm, false);
 if (reg >= 4
 || (prefixes & PREFIX_LOCK)
 || s->aflag == MO_16
@@ -6362,7 +6362,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu)
 || s->aflag == MO_16) {
 goto illegal_op;
 }
-AddressPar

[Stable-8.2.7 48/53] target/sparc: Restrict STQF to sparcv9

[Michael Tokarev]

From: Richard Henderson 

Prior to sparcv9, the same encoding was STDFQ.

Cc: qemu-sta...@nongnu.org
Fixes: 06c060d9e5b ("target/sparc: Move simple fp load/store to decodetree")
Signed-off-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240816072311.353234-2-richard.hender...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé 
(cherry picked from commit 12d36294a2d978faf893101862118d1ac1815e85)
Signed-off-by: Michael Tokarev 

diff --git a/target/sparc/insns.decode b/target/sparc/insns.decode
index e2d8a07dc4..d2b29de084 100644
--- a/target/sparc/insns.decode
+++ b/target/sparc/insns.decode
@@ -484,7 +484,7 @@ STF 11 . 100100 . . .  
@r_r_ri_na
 STFSR   11 0 100101 . . .  @n_r_ri
 STXFSR  11 1 100101 . . .  @n_r_ri
 {
-  STQF  11 . 100110 . . .  @q_r_ri_na
+  STQF  11 . 100110 . . .  @q_r_ri_na # v9
   STDFQ 11 - 100110 - - -
 }
 STDF11 . 100111 . . .  @d_r_ri_na
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 7058b6c2a4..94350aa588 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -4362,7 +4362,7 @@ static bool do_st_fpr(DisasContext *dc, arg_r_r_ri_asi 
*a, MemOp sz)
 
 TRANS(STF, ALL, do_st_fpr, a, MO_32)
 TRANS(STDF, ALL, do_st_fpr, a, MO_64)
-TRANS(STQF, ALL, do_st_fpr, a, MO_128)
+TRANS(STQF, 64, do_st_fpr, a, MO_128)
 
 TRANS(STFA, 64, do_st_fpr, a, MO_32)
 TRANS(STDFA, 64, do_st_fpr, a, MO_64)
-- 
2.39.2

[Stable-8.2.7 29/53] virtio-net: Fix network stall at the host side waiting for kick

[Michael Tokarev]

From: thomas 

Patch 06b12970174 ("virtio-net: fix network stall under load")
added double-check to test whether the available buffer size
can satisfy the request or not, in case the guest has added
some buffers to the avail ring simultaneously after the first
check. It will be lucky if the available buffer size becomes
okay after the double-check, then the host can send the packet
to the guest. If the buffer size still can't satisfy the request,
even if the guest has added some buffers, viritio-net would
stall at the host side forever.

The patch enables notification and checks whether the guest has
added some buffers since last check of available buffers when
the available buffers are insufficient. If no buffer is added,
return false, else recheck the available buffers in the loop.
If the available buffers are sufficient, disable notification
and return true.

Changes:
1. Change the return type of virtqueue_get_avail_bytes() from void
   to int, it returns an opaque that represents the shadow_avail_idx
   of the virtqueue on success, else -1 on error.
2. Add a new API: virtio_queue_enable_notification_and_check(),
   it takes an opaque as input arg which is returned from
   virtqueue_get_avail_bytes(). It enables notification firstly,
   then checks whether the guest has added some buffers since
   last check of available buffers or not by virtio_queue_poll(),
   return ture if yes.

The patch also reverts patch "06b12970174".

The case below can reproduce the stall.

   Guest 0
 ++
 | iperf  |
---> | server |
 Host   |++
   ++   |...
   | iperf  |
   | client |  Guest n
   ++   |++
|| iperf  |
---> | server |
 ++

Boot many guests from qemu with virtio network:
 qemu ... -netdev tap,id=net_x \
-device virtio-net-pci-non-transitional,\
iommu_platform=on,mac=xx:xx:xx:xx:xx:xx,netdev=net_x

Each guest acts as iperf server with commands below:
 iperf3 -s -D -i 10 -p 8001
 iperf3 -s -D -i 10 -p 8002

The host as iperf client:
 iperf3 -c guest_IP -p 8001 -i 30 -w 256k -P 20 -t 4
 iperf3 -c guest_IP -p 8002 -i 30 -w 256k -P 20 -t 4

After some time, the host loses connection to the guest,
the guest can send packet to the host, but can't receive
packet from the host.

It's more likely to happen if SWIOTLB is enabled in the guest,
allocating and freeing bounce buffer takes some CPU ticks,
copying from/to bounce buffer takes more CPU ticks, compared
with that there is no bounce buffer in the guest.
Once the rate of producing packets from the host approximates
the rate of receiveing packets in the guest, the guest would
loop in NAPI.

 receive packets---
   | |
   v |
   free buf  virtnet_poll
   | |
   v |
 add buf to avail ring  ---
   |
   |  need kick the host?
   |  NAPI continues
   v
 receive packets---
   | |
   v |
   free buf  virtnet_poll
   | |
   v |
 add buf to avail ring  ---
   |
   v
  ...   ...

On the other hand, the host fetches free buf from avail
ring, if the buf in the avail ring is not enough, the
host notifies the guest the event by writing the avail
idx read from avail ring to the event idx of used ring,
then the host goes to sleep, waiting for the kick signal
from the guest.

Once the guest finds the host is waiting for kick singal
(in virtqueue_kick_prepare_split()), it kicks the host.

The host may stall forever at the sequences below:

 HostGuest
  ---
 fetch buf, send packet   receive packet ---
 ...  ... |
 fetch buf, send packet add buf   |
 ...add buf   virtnet_poll
buf not enough  avail idx-> add buf   |
read avail idx  add buf   |
add buf  ---
  receive packet ---
write event idx   ... |
wait for kick   add buf   virtnet_poll
  ... |
 ---
 no more packet, exit NAPI

In the first loop of NAPI above, indicated in the range of
virtnet_poll above, the host is sending packets while the
guest is receiving pa

[Stable-8.2.7 38/53] nbd/server: CVE-2024-7409: Cap default max-connections to 100

[Michael Tokarev]

From: Eric Blake 

Allowing an unlimited number of clients to any web service is a recipe
for a rudimentary denial of service attack: the client merely needs to
open lots of sockets without closing them, until qemu no longer has
any more fds available to allocate.

For qemu-nbd, we default to allowing only 1 connection unless more are
explicitly asked for (-e or --shared); this was historically picked as
a nice default (without an explicit -t, a non-persistent qemu-nbd goes
away after a client disconnects, without needing any additional
follow-up commands), and we are not going to change that interface now
(besides, someday we want to point people towards qemu-storage-daemon
instead of qemu-nbd).

But for qemu proper, and the newer qemu-storage-daemon, the QMP
nbd-server-start command has historically had a default of unlimited
number of connections, in part because unlike qemu-nbd it is
inherently persistent until nbd-server-stop.  Allowing multiple client
sockets is particularly useful for clients that can take advantage of
MULTI_CONN (creating parallel sockets to increase throughput),
although known clients that do so (such as libnbd's nbdcopy) typically
use only 8 or 16 connections (the benefits of scaling diminish once
more sockets are competing for kernel attention).  Picking a number
large enough for typical use cases, but not unlimited, makes it
slightly harder for a malicious client to perform a denial of service
merely by opening lots of connections withot progressing through the
handshake.

This change does not eliminate CVE-2024-7409 on its own, but reduces
the chance for fd exhaustion or unlimited memory usage as an attack
surface.  On the other hand, by itself, it makes it more obvious that
with a finite limit, we have the problem of an unauthenticated client
holding 100 fds opened as a way to block out a legitimate client from
being able to connect; thus, later patches will further add timeouts
to reject clients that are not making progress.

This is an INTENTIONAL change in behavior, and will break any client
of nbd-server-start that was not passing an explicit max-connections
parameter, yet expects more than 100 simultaneous connections.  We are
not aware of any such client (as stated above, most clients aware of
MULTI_CONN get by just fine on 8 or 16 connections, and probably cope
with later connections failing by relying on the earlier connections;
libvirt has not yet been passing max-connections, but generally
creates NBD servers with the intent for a single client for the sake
of live storage migration; meanwhile, the KubeSAN project anticipates
a large cluster sharing multiple clients [up to 8 per node, and up to
100 nodes in a cluster], but it currently uses qemu-nbd with an
explicit --shared=0 rather than qemu-storage-daemon with
nbd-server-start).

We considered using a deprecation period (declare that omitting
max-parameters is deprecated, and make it mandatory in 3 releases -
then we don't need to pick an arbitrary default); that has zero risk
of breaking any apps that accidentally depended on more than 100
connections, and where such breakage might not be noticed under unit
testing but only under the larger loads of production usage.  But it
does not close the denial-of-service hole until far into the future,
and requires all apps to change to add the parameter even if 100 was
good enough.  It also has a drawback that any app (like libvirt) that
is accidentally relying on an unlimited default should seriously
consider their own CVE now, at which point they are going to change to
pass explicit max-connections sooner than waiting for 3 qemu releases.
Finally, if our changed default breaks an app, that app can always
pass in an explicit max-parameters with a larger value.

It is also intentional that the HMP interface to nbd-server-start is
not changed to expose max-connections (any client needing to fine-tune
things should be using QMP).

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Eric Blake 
Message-ID: <20240807174943.771624-12-ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
[ericb: Expand commit message to summarize Dan's argument for why we
break corner-case back-compat behavior without a deprecation period]
Signed-off-by: Eric Blake 
(cherry picked from commit c8a76dbd90c2f48df89b75bef74917f90a59b623)
Signed-off-by: Michael Tokarev 

diff --git a/block/monitor/block-hmp-cmds.c b/block/monitor/block-hmp-cmds.c
index c729cbf1eb..78a6975852 100644
--- a/block/monitor/block-hmp-cmds.c
+++ b/block/monitor/block-hmp-cmds.c
@@ -415,7 +415,8 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
 goto exit;
 }
 
-nbd_server_start(addr, NULL, NULL, 0, &local_err);
+nbd_server_start(addr, NULL, NULL, NBD_DEFAULT_MAX_CONNECTIONS,
+ &local_err);
 qapi_free_SocketAddress(addr);
 if (local_err != NULL) {
 goto exit;
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 267a1de903..24ba5382db 100644
--- a/blockdev-nbd.c

[Stable-8.2.7 37/53] nbd/server: Plumb in new args to nbd_client_add()

[Michael Tokarev]

From: Eric Blake 

Upcoming patches to fix a CVE need to track an opaque pointer passed
in by the owner of a client object, as well as request for a time
limit on how fast negotiation must complete.  Prepare for that by
changing the signature of nbd_client_new() and adding an accessor to
get at the opaque pointer, although for now the two servers
(qemu-nbd.c and blockdev-nbd.c) do not change behavior even though
they pass in a new default timeout value.

Suggested-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Eric Blake 
Message-ID: <20240807174943.771624-11-ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
[eblake: s/LIMIT/MAX_SECS/ as suggested by Dan]
Signed-off-by: Eric Blake 
(cherry picked from commit fb1c2aaa981e0a2fa6362c9985f1296b74f055ac)
Signed-off-by: Michael Tokarev 

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 213012435f..267a1de903 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -64,8 +64,10 @@ static void nbd_accept(QIONetListener *listener, 
QIOChannelSocket *cioc,
 nbd_update_server_watch(nbd_server);
 
 qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
-nbd_client_new(cioc, nbd_server->tlscreds, nbd_server->tlsauthz,
-   nbd_blockdev_client_closed);
+/* TODO - expose handshake timeout as QMP option */
+nbd_client_new(cioc, NBD_DEFAULT_HANDSHAKE_MAX_SECS,
+   nbd_server->tlscreds, nbd_server->tlsauthz,
+   nbd_blockdev_client_closed, NULL);
 }
 
 static void nbd_update_server_watch(NBDServerData *s)
diff --git a/include/block/nbd.h b/include/block/nbd.h
index 4e7bd6342f..1d4d65922d 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -33,6 +33,12 @@ typedef struct NBDMetaContexts NBDMetaContexts;
 
 extern const BlockExportDriver blk_exp_nbd;
 
+/*
+ * NBD_DEFAULT_HANDSHAKE_MAX_SECS: Number of seconds in which client must
+ * succeed at NBD_OPT_GO before being forcefully dropped as too slow.
+ */
+#define NBD_DEFAULT_HANDSHAKE_MAX_SECS 10
+
 /* Handshake phase structs - this struct is passed on the wire */
 
 typedef struct NBDOption {
@@ -403,9 +409,12 @@ AioContext *nbd_export_aio_context(NBDExport *exp);
 NBDExport *nbd_export_find(const char *name);
 
 void nbd_client_new(QIOChannelSocket *sioc,
+uint32_t handshake_max_secs,
 QCryptoTLSCreds *tlscreds,
 const char *tlsauthz,
-void (*close_fn)(NBDClient *, bool));
+void (*close_fn)(NBDClient *, bool),
+void *owner);
+void *nbd_client_owner(NBDClient *client);
 void nbd_client_get(NBDClient *client);
 void nbd_client_put(NBDClient *client);
 
diff --git a/nbd/server.c b/nbd/server.c
index 3d8ddfef06..91131883ee 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -124,12 +124,14 @@ struct NBDMetaContexts {
 struct NBDClient {
 int refcount; /* atomic */
 void (*close_fn)(NBDClient *client, bool negotiated);
+void *owner;
 
 QemuMutex lock;
 
 NBDExport *exp;
 QCryptoTLSCreds *tlscreds;
 char *tlsauthz;
+uint32_t handshake_max_secs;
 QIOChannelSocket *sioc; /* The underlying data channel */
 QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
 
@@ -3194,6 +3196,7 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
 
 qemu_co_mutex_init(&client->send_lock);
 
+/* TODO - utilize client->handshake_max_secs */
 if (nbd_negotiate(client, &local_err)) {
 if (local_err) {
 error_report_err(local_err);
@@ -3208,14 +3211,17 @@ static coroutine_fn void nbd_co_client_start(void 
*opaque)
 }
 
 /*
- * Create a new client listener using the given channel @sioc.
+ * Create a new client listener using the given channel @sioc and @owner.
  * Begin servicing it in a coroutine.  When the connection closes, call
- * @close_fn with an indication of whether the client completed negotiation.
+ * @close_fn with an indication of whether the client completed negotiation
+ * within @handshake_max_secs seconds (0 for unbounded).
  */
 void nbd_client_new(QIOChannelSocket *sioc,
+uint32_t handshake_max_secs,
 QCryptoTLSCreds *tlscreds,
 const char *tlsauthz,
-void (*close_fn)(NBDClient *, bool))
+void (*close_fn)(NBDClient *, bool),
+void *owner)
 {
 NBDClient *client;
 Coroutine *co;
@@ -3228,13 +3234,21 @@ void nbd_client_new(QIOChannelSocket *sioc,
 object_ref(OBJECT(client->tlscreds));
 }
 client->tlsauthz = g_strdup(tlsauthz);
+client->handshake_max_secs = handshake_max_secs;
 client->sioc = sioc;
 qio_channel_set_delay(QIO_CHANNEL(sioc), false);
 object_ref(OBJECT(client->sioc));
 client->ioc = QIO_CHANNEL(sioc);
 object_ref(OBJECT(client->ioc));
 client->close_fn = close_fn;
+client->owner = owner;
 
 co = qemu_coroutine_create(nbd_co_client_start, client);
 qemu_coro

[Stable-8.2.7 49/53] crypto/tlscredspsk: Free username on finalize

[Michael Tokarev]

From: Peter Maydell 

When the creds->username property is set we allocate memory
for it in qcrypto_tls_creds_psk_prop_set_username(), but
we never free this when the QCryptoTLSCredsPSK is destroyed.
Free the memory in finalize.

This fixes a LeakSanitizer complaint in migration-test:

$ (cd build/asan; ASAN_OPTIONS="fast_unwind_on_malloc=0" 
QTEST_QEMU_BINARY=./qemu-system-x86_64 ./tests/qtest/migration-test --tap -k -p 
/x86_64/migration/precopy/unix/tls/psk)

=
==3867512==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 5 byte(s) in 1 object(s) allocated from:
#0 0x5624e5c99dee in malloc 
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-x86_64+0x218edee)
 (BuildId: a9e623fa1009a9435c0142c037cd7b8c1ad04ce3)
#1 0x7fb199ae9738 in g_malloc debian/build/deb/../../../glib/gmem.c:128:13
#2 0x7fb199afe583 in g_strdup 
debian/build/deb/../../../glib/gstrfuncs.c:361:17
#3 0x5624e82ea919 in qcrypto_tls_creds_psk_prop_set_username 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../crypto/tlscredspsk.c:255:23
#4 0x5624e812c6b5 in property_set_str 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object.c:2277:5
#5 0x5624e8125ce5 in object_property_set 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object.c:1463:5
#6 0x5624e8136e7c in object_set_properties_from_qdict 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object_interfaces.c:55:14
#7 0x5624e81372d2 in user_creatable_add_type 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object_interfaces.c:112:5
#8 0x5624e8137964 in user_creatable_add_qapi 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/object_interfaces.c:157:11
#9 0x5624e891ba3c in qmp_object_add 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qom/qom-qmp-cmds.c:227:5
#10 0x5624e8af9118 in qmp_marshal_object_add 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qapi/qapi-commands-qom.c:337:5
#11 0x5624e8bd1d49 in do_qmp_dispatch_bh 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../qapi/qmp-dispatch.c:128:5
#12 0x5624e8cb2531 in aio_bh_call 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/async.c:171:5
#13 0x5624e8cb340c in aio_bh_poll 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/async.c:218:13
#14 0x5624e8c0be98 in aio_dispatch 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/aio-posix.c:423:5
#15 0x5624e8cba3ce in aio_ctx_dispatch 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/async.c:360:5
#16 0x7fb199ae0d3a in g_main_dispatch 
debian/build/deb/../../../glib/gmain.c:3419:28
#17 0x7fb199ae0d3a in g_main_context_dispatch 
debian/build/deb/../../../glib/gmain.c:4137:7
#18 0x5624e8cbe1d9 in glib_pollfds_poll 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/main-loop.c:287:9
#19 0x5624e8cbcb13 in os_host_main_loop_wait 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/main-loop.c:310:5
#20 0x5624e8cbc6dc in main_loop_wait 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../util/main-loop.c:589:11
#21 0x5624e6f3f917 in qemu_main_loop 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../system/runstate.c:801:9
#22 0x5624e893379c in qemu_default_main 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../system/main.c:37:14
#23 0x5624e89337e7 in main 
/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/../../system/main.c:48:12
#24 0x7fb197972d8f in __libc_start_call_main 
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#25 0x7fb197972e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#26 0x5624e5c16fa4 in _start 
(/mnt/nvmedisk/linaro/qemu-from-laptop/qemu/build/asan/qemu-system-x86_64+0x210bfa4)
 (BuildId: a9e623fa1009a9435c0142c037cd7b8c1ad04ce3)

SUMMARY: AddressSanitizer: 5 byte(s) leaked in 1 allocation(s).

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell 
Reviewed-by: Daniel P. Berrangé 
Message-ID: <20240819145021.38524-1-peter.mayd...@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé 
(cherry picked from commit 87e012f29f2e47dcd8c385ff8bb8188f9e06d4ea)
Signed-off-by: Michael Tokarev 

diff --git a/crypto/tlscredspsk.c b/crypto/tlscredspsk.c
index 546cad1c5a..0d6b71a37c 100644
--- a/crypto/tlscredspsk.c
+++ b/crypto/tlscredspsk.c
@@ -243,6 +243,7 @@ qcrypto_tls_creds_psk_finalize(Object *obj)
 QCryptoTLSCredsPSK *creds = QCRYPTO_TLS_CREDS_PSK(obj);
 
 qcrypto_tls_creds_psk_unload(creds);
+g_free(creds->username);
 }
 
 static void
-- 
2.39.2

[Stable-8.2.7 25/53] hw/i386/amd_iommu: Don't leak memory in amdvi_update_iotlb()

[Michael Tokarev]

From: Peter Maydell 

In amdvi_update_iotlb() we will only put a new entry in the hash
table if to_cache.perm is not IOMMU_NONE.  However we allocate the
memory for the new AMDVIIOTLBEntry and for the hash table key
regardless.  This means that in the IOMMU_NONE case we will leak the
memory we alloacted.

Move the allocations into the if() to the point where we know we're
going to add the item to the hash table.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2452
Signed-off-by: Peter Maydell 
Message-Id: <20240731170019.3590563-1-peter.mayd...@linaro.org>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 9a45b0761628cc59267b3283a85d15294464ac31)
Signed-off-by: Michael Tokarev 

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 4203144da9..12742b1433 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -346,12 +346,12 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t 
devid,
uint64_t gpa, IOMMUTLBEntry to_cache,
uint16_t domid)
 {
-AMDVIIOTLBEntry *entry = g_new(AMDVIIOTLBEntry, 1);
-uint64_t *key = g_new(uint64_t, 1);
-uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
-
 /* don't cache erroneous translations */
 if (to_cache.perm != IOMMU_NONE) {
+AMDVIIOTLBEntry *entry = g_new(AMDVIIOTLBEntry, 1);
+uint64_t *key = g_new(uint64_t, 1);
+uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
+
 trace_amdvi_cache_update(domid, PCI_BUS_NUM(devid), PCI_SLOT(devid),
 PCI_FUNC(devid), gpa, to_cache.translated_addr);
 
-- 
2.39.2

[Stable-8.2.7 39/53] nbd/server: CVE-2024-7409: Drop non-negotiating clients

[Michael Tokarev]

From: Eric Blake 

A client that opens a socket but does not negotiate is merely hogging
qemu's resources (an open fd and a small amount of memory); and a
malicious client that can access the port where NBD is listening can
attempt a denial of service attack by intentionally opening and
abandoning lots of unfinished connections.  The previous patch put a
default bound on the number of such ongoing connections, but once that
limit is hit, no more clients can connect (including legitimate ones).
The solution is to insist that clients complete handshake within a
reasonable time limit, defaulting to 10 seconds.  A client that has
not successfully completed NBD_OPT_GO by then (including the case of
where the client didn't know TLS credentials to even reach the point
of NBD_OPT_GO) is wasting our time and does not deserve to stay
connected.  Later patches will allow fine-tuning the limit away from
the default value (including disabling it for doing integration
testing of the handshake process itself).

Note that this patch in isolation actually makes it more likely to see
qemu SEGV after nbd-server-stop, as any client socket still connected
when the server shuts down will now be closed after 10 seconds rather
than at the client's whims.  That will be addressed in the next patch.

For a demo of this patch in action:
$ qemu-nbd -f raw -r -t -e 10 file &
$ nbdsh --opt-mode -c '
H = list()
for i in range(20):
  print(i)
  H.insert(i, nbd.NBD())
  H[i].set_opt_mode(True)
  H[i].connect_uri("nbd://localhost")
'
$ kill $!

where later connections get to start progressing once earlier ones are
forcefully dropped for taking too long, rather than hanging.

Suggested-by: Daniel P. Berrangé 
Signed-off-by: Eric Blake 
Message-ID: <20240807174943.771624-13-ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
[eblake: rebase to changes earlier in series, reduce scope of timer]
Signed-off-by: Eric Blake 
(cherry picked from commit b9b72cb3ce15b693148bd09cef7e50110566d8a0)
Signed-off-by: Michael Tokarev 

diff --git a/nbd/server.c b/nbd/server.c
index 91131883ee..3ed8b1df9c 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -3189,22 +3189,48 @@ static void nbd_client_receive_next_request(NBDClient 
*client)
 }
 }
 
+static void nbd_handshake_timer_cb(void *opaque)
+{
+QIOChannel *ioc = opaque;
+
+trace_nbd_handshake_timer_cb();
+qio_channel_shutdown(ioc, QIO_CHANNEL_SHUTDOWN_BOTH, NULL);
+}
+
 static coroutine_fn void nbd_co_client_start(void *opaque)
 {
 NBDClient *client = opaque;
 Error *local_err = NULL;
+QEMUTimer *handshake_timer = NULL;
 
 qemu_co_mutex_init(&client->send_lock);
 
-/* TODO - utilize client->handshake_max_secs */
+/*
+ * Create a timer to bound the time spent in negotiation. If the
+ * timer expires, it is likely nbd_negotiate will fail because the
+ * socket was shutdown.
+ */
+if (client->handshake_max_secs > 0) {
+handshake_timer = aio_timer_new(qemu_get_aio_context(),
+QEMU_CLOCK_REALTIME,
+SCALE_NS,
+nbd_handshake_timer_cb,
+client->sioc);
+timer_mod(handshake_timer,
+  qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
+  client->handshake_max_secs * NANOSECONDS_PER_SECOND);
+}
+
 if (nbd_negotiate(client, &local_err)) {
 if (local_err) {
 error_report_err(local_err);
 }
+timer_free(handshake_timer);
 client_close(client, false);
 return;
 }
 
+timer_free(handshake_timer);
 WITH_QEMU_LOCK_GUARD(&client->lock) {
 nbd_client_receive_next_request(client);
 }
diff --git a/nbd/trace-events b/nbd/trace-events
index 00ae3216a1..cbd0a4ab7e 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -76,6 +76,7 @@ nbd_co_receive_request_payload_received(uint64_t cookie, 
uint64_t len) "Payload
 nbd_co_receive_ext_payload_compliance(uint64_t from, uint64_t len) "client 
sent non-compliant write without payload flag: from=0x%" PRIx64 ", len=0x%" 
PRIx64
 nbd_co_receive_align_compliance(const char *op, uint64_t from, uint64_t len, 
uint32_t align) "client sent non-compliant unaligned %s request: from=0x%" 
PRIx64 ", len=0x%" PRIx64 ", align=0x%" PRIx32
 nbd_trip(void) "Reading request"
+nbd_handshake_timer_cb(void) "client took too long to negotiate"
 
 # client-connection.c
 nbd_connect_thread_sleep(uint64_t timeout) "timeout %" PRIu64
-- 
2.39.2

[Stable-8.2.7 18/53] hw/char/bcm2835_aux: Fix assert when receive FIFO fills up

[Michael Tokarev]

From: Frederik van Hövell 

When a bare-metal application on the raspi3 board reads the
AUX_MU_STAT_REG MMIO register while the device's buffer is
at full receive FIFO capacity
(i.e. `s->read_count == BCM2835_AUX_RX_FIFO_LEN`) the
assertion `assert(s->read_count < BCM2835_AUX_RX_FIFO_LEN)`
fails.

Reported-by: Cryptjar 
Suggested-by: Cryptjar 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/459
Signed-off-by: Frederik van Hövell 
Reviewed-by: Philippe Mathieu-Daudé 
[PMM: commit message tweaks]
Signed-off-by: Peter Maydell 
(cherry picked from commit 546d574b11e02bfd5b15cdf1564842c14516dfab)
Signed-off-by: Michael Tokarev 

diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
index 96410b1ff8..0f1b28547e 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/char/bcm2835_aux.c
@@ -138,7 +138,7 @@ static uint64_t bcm2835_aux_read(void *opaque, hwaddr 
offset, unsigned size)
 res = 0x30e; /* space in the output buffer, empty tx fifo, idle tx/rx 
*/
 if (s->read_count > 0) {
 res |= 0x1; /* data in input buffer */
-assert(s->read_count < BCM2835_AUX_RX_FIFO_LEN);
+assert(s->read_count <= BCM2835_AUX_RX_FIFO_LEN);
 res |= ((uint32_t)s->read_count) << 16; /* rx fifo fill level */
 }
 return res;
-- 
2.39.2

[Stable-8.2.7 22/53] target/arm: Avoid shifts by -1 in tszimm_shr() and tszimm_shl()

[Michael Tokarev]

From: Peter Maydell 

The function tszimm_esz() returns a shift amount, or possibly -1 in
certain cases that correspond to unallocated encodings in the
instruction set.  We catch these later in the trans_ functions
(generally with an "a-esz < 0" check), but before we do the
decodetree-generated code will also call tszimm_shr() or tszimm_sl(),
which will use the tszimm_esz() return value as a shift count without
checking that it is not negative, which is undefined behaviour.

Avoid the UB by checking the return value in tszimm_shr() and
tszimm_shl().

Cc: qemu-sta...@nongnu.org
Resolves: Coverity CID 1547617, 1547694
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20240722172957.1041231-4-peter.mayd...@linaro.org
(cherry picked from commit 76916dfa89e8900639c1055c07a295c06628a0bc)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/translate-sve.c b/target/arm/tcg/translate-sve.c
index ada05aa530..466a19c25a 100644
--- a/target/arm/tcg/translate-sve.c
+++ b/target/arm/tcg/translate-sve.c
@@ -50,13 +50,27 @@ static int tszimm_esz(DisasContext *s, int x)
 
 static int tszimm_shr(DisasContext *s, int x)
 {
-return (16 << tszimm_esz(s, x)) - x;
+/*
+ * We won't use the tszimm_shr() value if tszimm_esz() returns -1 (the
+ * trans function will check for esz < 0), so we can return any
+ * value we like from here in that case as long as we avoid UB.
+ */
+int esz = tszimm_esz(s, x);
+if (esz < 0) {
+return esz;
+}
+return (16 << esz) - x;
 }
 
 /* See e.g. LSL (immediate, predicated).  */
 static int tszimm_shl(DisasContext *s, int x)
 {
-return x - (8 << tszimm_esz(s, x));
+/* As with tszimm_shr(), value will be unused if esz < 0 */
+int esz = tszimm_esz(s, x);
+if (esz < 0) {
+return esz;
+}
+return x - (8 << esz);
 }
 
 /* The SH bit is in bit 8.  Extract the low 8 and shift.  */
-- 
2.39.2

[Stable-8.2.7 17/53] target/rx: Use target_ulong for address in LI

[Michael Tokarev]

From: Richard Henderson 

Using int32_t meant that the address was sign-extended to uint64_t
when passing to translator_ld*, triggering an assert.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2453
Signed-off-by: Richard Henderson 
Tested-by: Thomas Huth 
(cherry picked from commit 83340193b991e7a974f117baa86a04db1fd835a9)
Signed-off-by: Michael Tokarev 

diff --git a/target/rx/translate.c b/target/rx/translate.c
index c6ce717a95..d33003f3c1 100644
--- a/target/rx/translate.c
+++ b/target/rx/translate.c
@@ -86,7 +86,8 @@ static uint32_t decode_load_bytes(DisasContext *ctx, uint32_t 
insn,
 
 static uint32_t li(DisasContext *ctx, int sz)
 {
-int32_t tmp, addr;
+target_ulong addr;
+uint32_t tmp;
 CPURXState *env = ctx->env;
 addr = ctx->base.pc_next;
 
-- 
2.39.2

[Stable-8.2.7 50/53] hw/nvme: fix leak of uninitialized memory in io_mgmt_recv

[Michael Tokarev]

From: Klaus Jensen 

Yutaro Shimizu from the Cyber Defense Institute discovered a bug in the
NVMe emulation that leaks contents of an uninitialized heap buffer if
subsystem and FDP emulation are enabled.

Cc: qemu-sta...@nongnu.org
Reported-by: Yutaro Shimizu 
Signed-off-by: Klaus Jensen 
(cherry picked from commit 6a22121c4f25b181e99479f65958ecde65da1c92)
Signed-off-by: Michael Tokarev 

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index 1fa117fdff..ca54c250b2 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -4302,7 +4302,7 @@ static uint16_t nvme_io_mgmt_recv_ruhs(NvmeCtrl *n, 
NvmeRequest *req,
 
 nruhsd = ns->fdp.nphs * endgrp->fdp.nrg;
 trans_len = sizeof(NvmeRuhStatus) + nruhsd * sizeof(NvmeRuhStatusDescr);
-buf = g_malloc(trans_len);
+buf = g_malloc0(trans_len);
 
 trans_len = MIN(trans_len, len);
 
-- 
2.39.2

[Stable-8.2.7 28/53] virtio-net: Ensure queue index fits with RSS

[Michael Tokarev]

From: Akihiko Odaki 

Ensure the queue index points to a valid queue when software RSS
enabled. The new calculation matches with the behavior of Linux's TAP
device with the RSS eBPF program.

Fixes: 4474e37a5b3a ("virtio-net: implement RX RSS processing")
Reported-by: Zhibin Hu 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Akihiko Odaki 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Jason Wang 
(cherry picked from commit f1595ceb9aad36a6c1da95bcb77ab9509b38822d)
Signed-off-by: Michael Tokarev 
Fixes: CVE-2024-6505

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 0467b3bd8a..f84cff43aa 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1909,7 +1909,8 @@ static ssize_t virtio_net_receive_rcu(NetClientState *nc, 
const uint8_t *buf,
 if (!no_rss && n->rss_data.enabled && n->rss_data.enabled_software_rss) {
 int index = virtio_net_process_rss(nc, buf, size);
 if (index >= 0) {
-NetClientState *nc2 = qemu_get_subqueue(n->nic, index);
+NetClientState *nc2 =
+qemu_get_subqueue(n->nic, index % n->curr_queue_pairs);
 return virtio_net_receive_rcu(nc2, buf, size, true);
 }
 }
-- 
2.39.2

[Stable-8.2.7 23/53] target/arm: Ignore SMCR_EL2.LEN and SVCR_EL2.LEN if EL2 is not enabled

[Michael Tokarev]

From: Peter Maydell 

When determining the current vector length, the SMCR_EL2.LEN and
SVCR_EL2.LEN settings should only be considered if EL2 is enabled
(compare the pseudocode CurrentSVL and CurrentNSVL which call
EL2Enabled()).

We were checking against ARM_FEATURE_EL2 rather than calling
arm_is_el2_enabled(), which meant that we would look at
SMCR_EL2/SVCR_EL2 when in Secure EL1 or Secure EL0 even if Secure EL2
was not enabled.

Use the correct check in sve_vqm1_for_el_sm().

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20240722172957.1041231-5-peter.mayd...@linaro.org
(cherry picked from commit f573ac059ed060234fcef4299fae9e500d357c33)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ca2c6e9732..9ff266a235 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6860,7 +6860,7 @@ uint32_t sve_vqm1_for_el_sm(CPUARMState *env, int el, 
bool sm)
 if (el <= 1 && !el_is_in_host(env, el)) {
 len = MIN(len, 0xf & (uint32_t)cr[1]);
 }
-if (el <= 2 && arm_feature(env, ARM_FEATURE_EL2)) {
+if (el <= 2 && arm_is_el2_enabled(env)) {
 len = MIN(len, 0xf & (uint32_t)cr[2]);
 }
 if (arm_feature(env, ARM_FEATURE_EL3)) {
-- 
2.39.2

Re: [PULL 11/63] hw/virtio: move stubs out of stubs/

[Michael Tokarev]

05.09.2024 19:27, Paolo Bonzini wrote:

On Sat, Aug 3, 2024 at 4:29 AM Michael Tokarev  wrote:


23.04.2024 18:08, Paolo Bonzini wrote:

Since the virtio memory device stubs are needed exactly when the
Kconfig symbol is not enabled, they can be placed in hw/virtio/ and
conditionalized on CONFIG_VIRTIO_MD.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Richard Henderson 
Message-ID: <20240408155330.522792-12-pbonz...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
   stubs/virtio-md-pci.c => hw/virtio/virtio-md-stubs.c | 0
   hw/virtio/meson.build| 2 ++
   stubs/meson.build| 1 -
   3 files changed, 2 insertions(+), 1 deletion(-)
   rename stubs/virtio-md-pci.c => hw/virtio/virtio-md-stubs.c (100%)


FWIW, this broke a minimal microvm build for debian:

/usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
`pc_machine_device_pre_plug_cb':
./b/microvm/hw/i386/pc.c:1377: undefined reference to `virtio_md_pci_pre_plug'
/usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
`pc_machine_device_unplug_request_cb':
./b/microvm/hw/i386/pc.c:1427: undefined reference to 
`virtio_md_pci_unplug_request'
/usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
`pc_machine_device_unplug_cb':
./b/microvm/hw/i386/pc.c:1443: undefined reference to `virtio_md_pci_unplug'
/usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
`pc_machine_device_plug_cb':
./b/microvm/hw/i386/pc.c:1413: undefined reference to `virtio_md_pci_plug'
collect2: error: ld returned 1 exit status


Does it not link hw/virtio-virtio-md-stubs.c?


The complete link line (it's done by using `cc -m64 @qemu-system-x86_64.rsp`) 
is attached.

Yes, it does NOT link md-stubs.


  Can you send the
x86_64-softmmu-config-devices.mak file?


Yeah, it was in my first email (to which you replied), here it is again:

...with the following contents of microvm-devices.mak:
==
# see configs/devices/i386-softmmu/default.mak
# for additional devices which can be disabled
#
CONFIG_PCI_DEVICES=n

# we can't disable all machine types (boards) as of 6.1
# since the resulting binary fails to link
#CONFIG_ISAPC=y
#CONFIG_I440FX=y
CONFIG_Q35=y
CONFIG_MICROVM=y

CONFIG_VIRTIO_BLK=y
CONFIG_VIRTIO_SERIAL=y
CONFIG_VIRTIO_INPUT=y
CONFIG_VIRTIO_INPUT_HOST=y
CONFIG_VHOST_USER_INPUT=y
CONFIG_VIRTIO_NET=y
CONFIG_VIRTIO_SCSI=y
CONFIG_VIRTIO_RNG=y
CONFIG_VIRTIO_CRYPTO=y
CONFIG_VIRTIO_BALLOON=y
CONFIG_VIRTIO_MEM=y
CONFIG_VIRTIO_PMEM=y
CONFIG_VIRTIO_GPU=y
CONFIG_VHOST_USER_GPU=y
==

For now I just (temporarily) reverted the patch in question in debian,
to make the build work.

Thanks,

/mjt -o qemu-system-x86_64 libcommon.a.p/gdbstub_syscalls.c.o 
libcommon.a.p/hw_core_cpu-common.c.o libcommon.a.p/hw_core_machine-smp.c.o 
libcommon.a.p/cpu-common.c.o libcommon.a.p/page-vary-common.c.o 
libcommon.a.p/disas_disas-common.c.o libcommon.a.p/semihosting_stubs-all.c.o 
libcommon.a.p/trace_trace-hmp-cmds.c.o libcommon.a.p/qom_qom-hmp-cmds.c.o 
libcommon.a.p/ui_clipboard.c.o libcommon.a.p/ui_console.c.o 
libcommon.a.p/ui_cursor.c.o libcommon.a.p/ui_dmabuf.c.o 
libcommon.a.p/ui_input-keymap.c.o libcommon.a.p/ui_input-legacy.c.o 
libcommon.a.p/ui_input-barrier.c.o libcommon.a.p/ui_input.c.o 
libcommon.a.p/ui_kbd-state.c.o libcommon.a.p/ui_keymaps.c.o 
libcommon.a.p/ui_qemu-pixman.c.o libcommon.a.p/ui_ui-hmp-cmds.c.o 
libcommon.a.p/ui_ui-qmp-cmds.c.o libcommon.a.p/ui_util.c.o 
libcommon.a.p/ui_console-vc.c.o libcommon.a.p/ui_spice-module.c.o 
libcommon.a.p/ui_input-linux.c.o libcommon.a.p/ui_udmabuf.c.o 
libcommon.a.p/ui_vnc.c.o libcommon.a.p/ui_vnc-enc-zlib.c.o 
libcommon.a.p/ui_vnc-enc-hextile.c.o libcommon.a.p/ui_vnc-enc-tight.c.o 
libcommon.a.p/ui_vnc-palette.c.o libcommon.a.p/ui_vnc-enc-zrle.c.o 
libcommon.a.p/ui_vnc-auth-vencrypt.c.o libcommon.a.p/ui_vnc-ws.c.o 
libcommon.a.p/ui_vnc-jobs.c.o libcommon.a.p/ui_vnc-clipboard.c.o 
libcommon.a.p/hw_acpi_acpi_generic_initiator.c.o 
libcommon.a.p/hw_acpi_acpi_interface.c.o libcommon.a.p/hw_acpi_aml-build.c.o 
libcommon.a.p/hw_acpi_bios-linker-loader.c.o libcommon.a.p/hw_acpi_core.c.o 
libcommon.a.p/hw_acpi_utils.c.o libcommon.a.p/hw_acpi_cpu.c.o 
libcommon.a.p/hw_acpi_cpu_hotplug.c.o libcommon.a.p/hw_acpi_memory_hotplug.c.o 
libcommon.a.p/hw_acpi_nvdimm.c.o libcommon.a.p/hw_acpi_pci.c.o 
libcommon.a.p/hw_acpi_cxl.c.o libcommon.a.p/hw_acpi_vmgenid.c.o 
libcommon.a.p/hw_acpi_generic_event_device.c.o libcommon.a.p/hw_acpi_hmat.c.o 
libcommon.a.p/hw_acpi_ghes-stub.c.o libcommon.a.p/hw_acpi_pci-bridge.c.o 
libcommon.a.p/hw_acpi_pcihp.c.o libcommon.a.p/hw_acpi_viot.c.o 
libcommon.a.p/hw_acpi_ich9.c.o libcommon.a.p/hw_acpi_ich9_tco.c.o 
libcommon.a.p/hw_acpi_erst.c.o libcommon.a.p/hw_acpi_ipmi-stub.c.o 
libcommon.a.p/hw_acpi_acpi-qmp-cmds.c.o libcommon.a.p/hw_audio_soundhw.c.o 
libcommon.a.p/hw_audio_pcspk.c.o libcommon.a.p/hw_block_block.c.o 
libcommon.a.p/hw_block_cdrom.c.o libcommon.a.p/hw_block_hd-geometry.c.o 
libcommon.a.p/hw_bloc

Re: [PULL 11/63] hw/virtio: move stubs out of stubs/

[Paolo Bonzini]

On Fri, Sep 6, 2024 at 9:08 AM Michael Tokarev  wrote:
>
> 05.09.2024 19:27, Paolo Bonzini wrote:
> > On Sat, Aug 3, 2024 at 4:29 AM Michael Tokarev  wrote:
> >>
> >> 23.04.2024 18:08, Paolo Bonzini wrote:
> >>> Since the virtio memory device stubs are needed exactly when the
> >>> Kconfig symbol is not enabled, they can be placed in hw/virtio/ and
> >>> conditionalized on CONFIG_VIRTIO_MD.
> >>>
> >>> Signed-off-by: Paolo Bonzini 
> >>> Reviewed-by: Richard Henderson 
> >>> Message-ID: <20240408155330.522792-12-pbonz...@redhat.com>
> >>> Signed-off-by: Paolo Bonzini 
> >>> ---
> >>>stubs/virtio-md-pci.c => hw/virtio/virtio-md-stubs.c | 0
> >>>hw/virtio/meson.build| 2 ++
> >>>stubs/meson.build| 1 -
> >>>3 files changed, 2 insertions(+), 1 deletion(-)
> >>>rename stubs/virtio-md-pci.c => hw/virtio/virtio-md-stubs.c (100%)
> >>
> >> FWIW, this broke a minimal microvm build for debian:
> >>
> >> /usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
> >> `pc_machine_device_pre_plug_cb':
> >> ./b/microvm/hw/i386/pc.c:1377: undefined reference to 
> >> `virtio_md_pci_pre_plug'
> >> /usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
> >> `pc_machine_device_unplug_request_cb':
> >> ./b/microvm/hw/i386/pc.c:1427: undefined reference to 
> >> `virtio_md_pci_unplug_request'
> >> /usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
> >> `pc_machine_device_unplug_cb':
> >> ./b/microvm/hw/i386/pc.c:1443: undefined reference to 
> >> `virtio_md_pci_unplug'
> >> /usr/bin/ld: libqemu-x86_64-softmmu.a.p/hw_i386_pc.c.o: in function 
> >> `pc_machine_device_plug_cb':
> >> ./b/microvm/hw/i386/pc.c:1413: undefined reference to `virtio_md_pci_plug'
> >> collect2: error: ld returned 1 exit status
> >
> > Does it not link hw/virtio-virtio-md-stubs.c?
>
> The complete link line (it's done by using `cc -m64 @qemu-system-x86_64.rsp`) 
> is attached.
>
> Yes, it does NOT link md-stubs.
>
> >   Can you send the
> > x86_64-softmmu-config-devices.mak file?
>
> Yeah, it was in my first email (to which you replied), here it is again:

No I meant the full config in the build directory. Anyway I could
reproduce this, the issue shows up because VIRTIO_MEM and VIRTIO_PMEM
are effectively PCI only but hw/virtio/Kconfig doesn't list the
dependency.

The fix is to either add CONFIG_VIRTIO_PCI=y to microvm-devices.mak,
or to remove CONFIG_VIRTIO_MEM and CONFIG_VIRTIO_PMEM.

I'll shortly send a patch that will turn this into a compilation
failure with the following error

The following clauses were found for VIRTIO_MEM
CONFIG_VIRTIO_MEM=y
config VIRTIO_MEM depends on VIRTIO_PCI
...
KconfigDataError: contradiction between clauses when setting VIRTIO_MEM

Paolo

[PATCH] minikconf: print error entirely on stderr

[Paolo Bonzini]

While debugging an invalid configuration, I noticed that the clauses debug
ends up on stderr but the header ("The following clauses were found..."
ends up on stdout.  This makes the contents of meson-logs/meson-log.txt
a bit confusing.

Signed-off-by: Paolo Bonzini 
---
 scripts/minikconf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/minikconf.py b/scripts/minikconf.py
index bcd91015d34..6f7f43b2918 100644
--- a/scripts/minikconf.py
+++ b/scripts/minikconf.py
@@ -112,7 +112,7 @@ def has_value(self):
 def set_value(self, val, clause):
 self.clauses_for_var.append(clause)
 if self.has_value() and self.value != val:
-print("The following clauses were found for " + self.name)
+print("The following clauses were found for " + self.name, 
file=sys.stderr)
 for i in self.clauses_for_var:
 print("" + str(i), file=sys.stderr)
 raise KconfigDataError('contradiction between clauses when 
setting %s' % self)
-- 
2.46.0

[PATCH] virtio: kconfig: memory devices are PCI only

[Paolo Bonzini]

Virtio memory devices rely on PCI BARs to expose the contents of memory.
Because of this they cannot be used with virtio-mmio or virtio-ccw.  In fact
the code that is common to virtio-mem and virtio-pmem, which is in
hw/virtio/virtio-md-pci.c, is only included if CONFIG_VIRTIO_PCI is
set.  Reproduce the same condition in the Kconfig file.

Without this patch it is possible to create a configuration with
CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
compilation failure.

Cc: David Hildenbrand 
Reported-by: Michael Tokarev 
Signed-off-by: Paolo Bonzini 
---
 hw/virtio/Kconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..7c554d230d8 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -37,6 +37,7 @@ config VIRTIO_CRYPTO
 
 config VIRTIO_MD
 bool
+depends on VIRTIO_PCI
 select MEM_DEVICE
 
 config VIRTIO_PMEM_SUPPORTED
@@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED
 config VIRTIO_PMEM
 bool
 default y
-depends on VIRTIO
+depends on VIRTIO_PCI
 depends on VIRTIO_PMEM_SUPPORTED
 select VIRTIO_MD
 
@@ -55,7 +56,7 @@ config VIRTIO_MEM_SUPPORTED
 config VIRTIO_MEM
 bool
 default y
-depends on VIRTIO
+depends on VIRTIO_PCI
 depends on LINUX
 depends on VIRTIO_MEM_SUPPORTED
 select VIRTIO_MD
-- 
2.46.0

[PATCH v4 0/1] linux-user: add openat2 support in linux-user

[Michael Vogt]

Hi,

This is v4 of the openat2 support in linux-user. Thanks again for the
excellent second round of feedback from Richard Henderson.

The code is identical to the previous v3 and I only fixed two typos in
the commit message. I'm sending v4 because in v3 I forgot to add
"--threaded" when generating the coverletter/patch which makes it a bit
awkward to review and it does not show up properly on
e.g. https://patchew.org/QEMU/. My apologies for this mistake.

This version tries to be closer to the kernels behavior, i.e. now
do_openat2() uses a new copy_struct_from_user() helper that is very
similar to the kernels. This lead me to also drop incuding openat2.h
(as was originally suggested in the v1 review). It now contains it as
a copy named `struct open_how_ver0` and with that we can LOG_UNIMP if
the struct ever grows and qemu-user needs updating.

To answer the question why "maybe_do_fake_open()" uses a
"use_returned_fd" bool instead of just returning "-1": I wanted to be
as close as possible to the previous behavior and maybe_fake_open()
could in theory return "-1" for failures in memfd_create() or
mkstemp() or fake_open->fill(). In those cases the old code in
do_guest_openat() failed and returned the error but the new code would
just see a -1 and continue trying to open a special file that should
have been faked. Maybe I did overthink this as it's very
corner-case-y. Advise is welcome here, happy to change back or
simplify in other ways.

Thanks again,
 Michael

v3 -> v4:
- fix typos in the commit message

v2 -> v3:
- fix coding style (braches)
- improve argument args/naming in do_openat2()
- merge do_openat2/do_guest_openat2
- do size checks first in do_openat2
- add "copy_struct_from_user" and use in "do_openat2()"
- drop using openat2.h and create "struct open_how_v0"
- log if open_how guest struct is bigger than our supported struct

v1 -> v2:
- do not include 
- drop do_guest_openat2 from qemu.h and make static
- drop "safe" from do_guest_openat2
- ensure maybe_do_fake_open() is correct about when the result should
  be used or not
- Extract do_openat2() helper from do_syscall1()
- Call user_unlock* if a lock call fails
- Fix silly incorrect use of "target_open_how" when "open_how" is required
- Fix coding style comments
- Fix validation of arg4 in openat2
- Fix missing zero initialization of open_how
- Define target_open_how with abi_* types
- Warn about unimplemented size if "size" of openat2 is bigger than
  target_open_how


Michael Vogt (1):
  linux-user: add openat2 support in linux-user

 linux-user/syscall.c  | 116 --
 linux-user/syscall_defs.h |   7 +++
 2 files changed, 119 insertions(+), 4 deletions(-)

-- 
2.45.2

[PATCH v4 1/1] linux-user: add openat2 support in linux-user

[Michael Vogt]

This commit adds support for the `openat2()` syscall in the
`linux-user` userspace emulator.

It is implemented by extracting a new helper `maybe_do_fake_open()`
out of the exiting `do_guest_openat()` and share that with the
new `do_guest_openat2()`. Unfortunately we cannot just make
do_guest_openat2() a superset of do_guest_openat() because the
openat2() syscall is stricter with the argument checking and
will return an error for invalid flags or mode combinations (which
open()/openat() will ignore).

The implementation is similar to SYSCALL_DEFINE(openat2), i.e.
a new `copy_struct_from_user()` is used that works the same
as the kernels version to support backwards-compatibility
for struct syscall argument.

Instead of including openat2.h we create a copy of `open_how`
as `open_how_ver0` to ensure that if the structure grows we
can log a LOG_UNIMP warning.

Note that in this commit using openat2() for a "faked" file in
/proc will ignore the "resolve" flags. This is not great but it
seems similar to the exiting behavior when openat() is called
with a dirfd to "/proc". Here too the fake file lookup may
not catch the special file because "realpath()" is used to
determine if the path is in /proc. Alternatively to ignoring
we could simply fail with `-TARGET_ENOSYS` (or similar) if
`resolve` flags are passed and we found something that looks
like a file in /proc that needs faking.

Signed-off-by: Michael Vogt 
Buglink: https://github.com/osbuild/bootc-image-builder/issues/619
---
 linux-user/syscall.c  | 116 --
 linux-user/syscall_defs.h |   7 +++
 2 files changed, 119 insertions(+), 4 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 9d5415674d..83c944508b 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -602,6 +602,34 @@ static int check_zeroed_user(abi_long addr, size_t ksize, 
size_t usize)
 return 1;
 }
 
+/*
+ * Copies a target struct to a host struct, in a way that guarantees
+ * backwards-compatibility for struct syscall arguments.
+ *
+ * Similar to kernels uaccess.h:copy_struct_from_user()
+ */
+static int
+copy_struct_from_user(void *dst, size_t ksize, abi_ptr src, size_t usize)
+{
+size_t size = MIN(ksize, usize);
+size_t rest = MAX(ksize, usize) - size;
+
+/* Deal with trailing bytes. */
+if (usize < ksize) {
+memset(dst + size, 0, rest);
+} else if (usize > ksize) {
+int ret = check_zeroed_user(src, ksize, usize);
+if (ret <= 0) {
+return ret ?: -TARGET_E2BIG;
+}
+}
+/* Copy the interoperable parts of the struct. */
+if (copy_from_user(dst, src, size)) {
+return -TARGET_EFAULT;
+}
+return 0;
+}
+
 #define safe_syscall0(type, name) \
 static type safe_##name(void) \
 { \
@@ -653,6 +681,15 @@ safe_syscall3(ssize_t, read, int, fd, void *, buff, 
size_t, count)
 safe_syscall3(ssize_t, write, int, fd, const void *, buff, size_t, count)
 safe_syscall4(int, openat, int, dirfd, const char *, pathname, \
   int, flags, mode_t, mode)
+
+struct open_how_ver0 {
+__u64 flags;
+__u64 mode;
+__u64 resolve;
+};
+safe_syscall4(int, openat2, int, dirfd, const char *, pathname, \
+  const struct open_how_ver0 *, how, size_t, size)
+
 #if defined(TARGET_NR_wait4) || defined(TARGET_NR_waitpid)
 safe_syscall4(pid_t, wait4, pid_t, pid, int *, status, int, options, \
   struct rusage *, rusage)
@@ -8334,8 +8371,9 @@ static int open_net_route(CPUArchState *cpu_env, int fd)
 }
 #endif
 
-int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *fname,
-int flags, mode_t mode, bool safe)
+static int maybe_do_fake_open(CPUArchState *cpu_env, int dirfd,
+  const char *fname, int flags, mode_t mode,
+  bool safe, bool *use_returned_fd)
 {
 g_autofree char *proc_name = NULL;
 const char *pathname;
@@ -8362,6 +8400,7 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, 
const char *fname,
 #endif
 { NULL, NULL, NULL }
 };
+*use_returned_fd = true;
 
 /* if this is a file from /proc/ filesystem, expand full name */
 proc_name = realpath(fname, NULL);
@@ -8418,13 +8457,77 @@ int do_guest_openat(CPUArchState *cpu_env, int dirfd, 
const char *fname,
 return fd;
 }
 
+*use_returned_fd = false;
+return -1;
+}
+
+int do_guest_openat(CPUArchState *cpu_env, int dirfd, const char *fname,
+int flags, mode_t mode, bool safe)
+{
+bool use_returned_fd;
+int fd = maybe_do_fake_open(cpu_env, dirfd, fname, flags, mode, safe,
+&use_returned_fd);
+if (use_returned_fd) {
+return fd;
+}
+
 if (safe) {
-return safe_openat(dirfd, path(pathname), flags, mode);
+return safe_openat(dirfd, path(fname), flags, mode);
 } else {
-return openat(dirfd, path(pathname), flags, mode);
+retur

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

On 06.09.24 09:37, Paolo Bonzini wrote:

Virtio memory devices rely on PCI BARs to expose the contents of memory.
Because of this they cannot be used with virtio-mmio or virtio-ccw.  In fact


Guess what I am working on at this very the moment ;)


the code that is common to virtio-mem and virtio-pmem, which is in
hw/virtio/virtio-md-pci.c, is only included if CONFIG_VIRTIO_PCI is
set.  Reproduce the same condition in the Kconfig file.

Without this patch it is possible to create a configuration with
CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
compilation failure.


Right.



Cc: David Hildenbrand 
Reported-by: Michael Tokarev 
Signed-off-by: Paolo Bonzini 
---
  hw/virtio/Kconfig | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..7c554d230d8 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -37,6 +37,7 @@ config VIRTIO_CRYPTO
  
  config VIRTIO_MD

  bool
+depends on VIRTIO_PCI
  select MEM_DEVICE
  
  config VIRTIO_PMEM_SUPPORTED

@@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED
  config VIRTIO_PMEM
  bool
  default y
-depends on VIRTIO
+depends on VIRTIO_PCI


depends on VIRTIO_MD ?


  depends on VIRTIO_PMEM_SUPPORTED
  select VIRTIO_MD
  
@@ -55,7 +56,7 @@ config VIRTIO_MEM_SUPPORTED

  config VIRTIO_MEM
  bool
  default y
-depends on VIRTIO
+depends on VIRTIO_PCI


Same here.

With CCW support, I can unlock VIRTIO_MD and VIRTIO_MEM_SUPPORTED and it 
should fly.


--
Cheers,

David / dhildenb

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

Cc: David Hildenbrand 
Reported-by: Michael Tokarev 
Signed-off-by: Paolo Bonzini 
---
   hw/virtio/Kconfig | 5 +++--
   1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..7c554d230d8 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -37,6 +37,7 @@ config VIRTIO_CRYPTO
   
   config VIRTIO_MD

   bool
+depends on VIRTIO_PCI
   select MEM_DEVICE
   
   config VIRTIO_PMEM_SUPPORTED

@@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED
   config VIRTIO_PMEM
   bool
   default y
-depends on VIRTIO
+depends on VIRTIO_PCI


depends on VIRTIO_MD ?


(of course, removing the "select VIRTIO_MD")

--
Cheers,

David / dhildenb

Re: [SPAM] [PATCH v3 00/11] support I2C for AST2700

[Cédric Le Goater]

Hello,


On 9/4/24 11:01, Troy Lee wrote:

Hi Cédric,

On Wed, Sep 4, 2024 at 3:29 PM Cédric Le Goater  wrote:


Hello Jamin,


Just want you to know that I and Troy are working on the following tasks for 
AST2700.
1. Support boot from bootmcu(riscv32) instead of u-boot(Cortex-A35)


Oh nice. This is a good topic for heterogeneous machines !


The basic model for bootmcu(risc-v) is working now, 


May be send that first.


but we're looking for a way
to integrate sram/mmio/dram together.  

Unfortunately, QEMU is not yet ready for heterogeneous architectures.


The ivshmem requires PCI, it might be
too complicate for our use case.  There is an ivshmem-flat probably more
sutiable for us.

[PATCH 0/4] Add ivshmem-flat device - Gustavo Romero (kernel.org)
https://lore.kernel.org/qemu-devel/20231127052024.435743-1-gustavo.rom...@linaro.org/


I lack expertise in that area. Adding Gustavo.

Thanks,

C.

Re: [PATCH 4/8] tests/qtest/hd-geo-test: Check for availability of "pc" machine before using it

[Philippe Mathieu-Daudé]

On 5/9/24 21:14, Thomas Huth wrote:

In case QEMU has been configured with "--without-default-devices", the
"pc" machine type might be missing in the binary. We should check for
its availability before using it.

Signed-off-by: Thomas Huth 
---
  tests/qtest/hd-geo-test.c | 71 +--
  1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index d08bffad91..85eb8d7668 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -1074,17 +1074,26 @@ int main(int argc, char **argv)
  }
  }
  
-qtest_add_func("hd-geo/ide/none", test_ide_none);

-qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
-qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
-qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
-qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
-qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
-qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
-qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
-qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
-qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);
-if (have_qemu_img()) {
+if (qtest_has_machine("pc")) {
+qtest_add_func("hd-geo/ide/none", test_ide_none);
+qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
+qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
+qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
+qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
+qtest_add_func("hd-geo/ide/device/mbr/blank", 
test_ide_device_mbr_blank);
+qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
+qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
+qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
+qtest_add_func("hd-geo/ide/device/user/chst", 
test_ide_device_user_chst);


Just wondering, could a qtest_add_machine_func() method be helpful?
Maybe not if we want to check for multiple machines?

Re: [PATCH 4/8] tests/qtest/hd-geo-test: Check for availability of "pc" machine before using it

[Thomas Huth]

On 06/09/2024 09.50, Philippe Mathieu-Daudé wrote:

On 5/9/24 21:14, Thomas Huth wrote:

In case QEMU has been configured with "--without-default-devices", the
"pc" machine type might be missing in the binary. We should check for
its availability before using it.

Signed-off-by: Thomas Huth 
---
  tests/qtest/hd-geo-test.c | 71 +--
  1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index d08bffad91..85eb8d7668 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -1074,17 +1074,26 @@ int main(int argc, char **argv)
  }
  }
-    qtest_add_func("hd-geo/ide/none", test_ide_none);
-    qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
-    qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
-    qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
-    qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
-    qtest_add_func("hd-geo/ide/device/mbr/blank", 
test_ide_device_mbr_blank);

-    qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
-    qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
-    qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
-    qtest_add_func("hd-geo/ide/device/user/chst", 
test_ide_device_user_chst);

-    if (have_qemu_img()) {
+    if (qtest_has_machine("pc")) {
+    qtest_add_func("hd-geo/ide/none", test_ide_none);
+    qtest_add_func("hd-geo/ide/drive/mbr/blank", 
test_ide_drive_mbr_blank);

+    qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
+    qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
+    qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
+    qtest_add_func("hd-geo/ide/device/mbr/blank", 
test_ide_device_mbr_blank);
+    qtest_add_func("hd-geo/ide/device/mbr/lba", 
test_ide_device_mbr_lba);
+    qtest_add_func("hd-geo/ide/device/mbr/chs", 
test_ide_device_mbr_chs);
+    qtest_add_func("hd-geo/ide/device/user/chs", 
test_ide_device_user_chs);
+    qtest_add_func("hd-geo/ide/device/user/chst", 
test_ide_device_user_chst);


Just wondering, could a qtest_add_machine_func() method be helpful?
Maybe not if we want to check for multiple machines?


For adding multiple tests, I think a check at the beginning of the block is 
better than checking it over and over again.


 Thomas

Re: [PATCH] minikconf: print error entirely on stderr

[Philippe Mathieu-Daudé]

On 6/9/24 09:33, Paolo Bonzini wrote:

While debugging an invalid configuration, I noticed that the clauses debug
ends up on stderr but the header ("The following clauses were found..."
ends up on stdout.  This makes the contents of meson-logs/meson-log.txt
a bit confusing.

Signed-off-by: Paolo Bonzini 
---
  scripts/minikconf.py | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH] hw/i386: define _AS_LATEST() macros for machine types

[Philippe Mathieu-Daudé]

On 5/9/24 20:21, Daniel P. Berrangé wrote:

Follow the other architecture targets by adding extra macros for
defining a versioned machine type as the latest. This reduces the
size of the changes when introducing new machine types at the start
of each release cycle.

Signed-off-by: Daniel P. Berrangé 
---
  hw/i386/pc_piix.c| 11 +--
  hw/i386/pc_q35.c | 11 ++-
  include/hw/i386/pc.h |  4 +++-
  3 files changed, 14 insertions(+), 12 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH RESEND v9 7/9] rust: add crate to expose bindings and interfaces

[Paolo Bonzini]

On Mon, Sep 2, 2024 at 7:59 AM Junjie Mao  wrote:
> +  '-print-file-name=libclang-' + host_clang_major + '.so',

Note that libclang-MAJOR.so is a Debian-ism. On Fedora for example I
have libclang.so.MAJOR.MINOR instead.

Overall, this is a pain and I'd rather leave it to Meson developers to
fix it. Until then, you'll have to make sure your clang/libclang
versions match, or pass CLANG_PATH/LIBCLANG_PATH.

Paolo

Re: [PATCH 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Philippe Mathieu-Daudé]

Hi Thomas,

On 5/9/24 21:14, Thomas Huth wrote:

Let's make sure that we always pass a machine name to the test_boot_orders()
function, so we can check whether the machine is available in the binary
and skip the test in case it is not included in the build.

Signed-off-by: Thomas Huth 
---
  tests/qtest/boot-order-test.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
index 8f2b6ef05a..c67b8cfe16 100644
--- a/tests/qtest/boot-order-test.c
+++ b/tests/qtest/boot-order-test.c
@@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
  uint64_t actual;
  QTestState *qts;
  
-if (machine && !qtest_has_machine(machine)) {

+if (!qtest_has_machine(machine)) {


Should we defer the NULL check to qtest_has_machine_with_env()?
It uses g_str_equal() which is described as:

  Note that this function is primarily meant as a hash table
  comparison function. For a general-purpose, NULL-safe string
  comparison function, see g_strcmp0().

Better switch to g_strcmp0() in qtest_has_machine_with_env()?


  g_test_skip("Machine is not available");
  return;
  }
@@ -107,7 +107,7 @@ static const boot_order_test test_cases_pc[] = {
  
  static void test_pc_boot_order(void)

  {
-test_boot_orders(NULL, read_boot_order_pc, test_cases_pc);
+test_boot_orders("pc", read_boot_order_pc, test_cases_pc);
  }
  
  static uint64_t read_boot_order_pmac(QTestState *qts)

Re: [PATCH] hw/i386: define _AS_LATEST() macros for machine types

[Cornelia Huck]

On Thu, Sep 05 2024, Daniel P. Berrangé  wrote:

> Follow the other architecture targets by adding extra macros for
> defining a versioned machine type as the latest. This reduces the
> size of the changes when introducing new machine types at the start
> of each release cycle.
>
> Signed-off-by: Daniel P. Berrangé 
> ---
>  hw/i386/pc_piix.c| 11 +--
>  hw/i386/pc_q35.c | 11 ++-
>  include/hw/i386/pc.h |  4 +++-
>  3 files changed, 14 insertions(+), 12 deletions(-)

Reviewed-by: Cornelia Huck 

Re: [PATCH 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Thomas Huth]

On 06/09/2024 09.59, Philippe Mathieu-Daudé wrote:

Hi Thomas,

On 5/9/24 21:14, Thomas Huth wrote:

Let's make sure that we always pass a machine name to the test_boot_orders()
function, so we can check whether the machine is available in the binary
and skip the test in case it is not included in the build.

Signed-off-by: Thomas Huth 
---
  tests/qtest/boot-order-test.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
index 8f2b6ef05a..c67b8cfe16 100644
--- a/tests/qtest/boot-order-test.c
+++ b/tests/qtest/boot-order-test.c
@@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
  uint64_t actual;
  QTestState *qts;
-    if (machine && !qtest_has_machine(machine)) {
+    if (!qtest_has_machine(machine)) {


Should we defer the NULL check to qtest_has_machine_with_env()?
It uses g_str_equal() which is described as:

   Note that this function is primarily meant as a hash table
   comparison function. For a general-purpose, NULL-safe string
   comparison function, see g_strcmp0().

Better switch to g_strcmp0() in qtest_has_machine_with_env()?


What would be the intended meaning of the function when it is called with 
"NULL" ? Use the default machine? Skip the test? ... I think it is rather an 
error to call it with NULL, so it's OK if the test crashes here since it 
should never happen?


 Thomas

Re: [PATCH for-9.2] hw: add compat machines for 9.2

[Daniel P . Berrangé]

On Thu, Sep 05, 2024 at 08:05:14PM +0100, Peter Maydell wrote:
> On Thu, 5 Sept 2024 at 19:22, Daniel P. Berrangé  wrote:
> >
> > On Fri, Aug 16, 2024 at 11:47:16AM +0100, Daniel P. Berrangé wrote:
> > > On Fri, Aug 16, 2024 at 12:37:23PM +0200, Cornelia Huck wrote:
> > > > Add 9.2 machine types for arm/i440fx/m68k/q35/s390x/spapr.
> > > >
> > > > Signed-off-by: Cornelia Huck 
> > > > ---
> > > >  hw/arm/virt.c  |  9 -
> > > >  hw/core/machine.c  |  3 +++
> > > >  hw/i386/pc.c   |  3 +++
> > > >  hw/i386/pc_piix.c  | 15 ---
> > > >  hw/i386/pc_q35.c   | 13 +++--
> > > >  hw/m68k/virt.c |  9 -
> > > >  hw/ppc/spapr.c | 15 +--
> > > >  hw/s390x/s390-virtio-ccw.c | 14 +-
> > > >  include/hw/boards.h|  3 +++
> > > >  include/hw/i386/pc.h   |  3 +++
> > > >  10 files changed, 77 insertions(+), 10 deletions(-)
> > >
> > > Reviewed-by: Daniel P. Berrangé 
> > >
> > >
> > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > index d9e69243b4a7..746bfe05d386 100644
> > > > --- a/hw/i386/pc_piix.c
> > > > +++ b/hw/i386/pc_piix.c
> > > > @@ -479,13 +479,24 @@ static void 
> > > > pc_i440fx_machine_options(MachineClass *m)
> > > >   "Use a different south bridge 
> > > > than PIIX3");
> > > >  }
> > > >
> > > > -static void pc_i440fx_machine_9_1_options(MachineClass *m)
> > > > +static void pc_i440fx_machine_9_2_options(MachineClass *m)
> > > >  {
> > > >  pc_i440fx_machine_options(m);
> > > >  m->alias = "pc";
> > > >  m->is_default = true;
> > > >  }
> > > >
> > > > +DEFINE_I440FX_MACHINE(9, 2);
> > > > +
> > > > +static void pc_i440fx_machine_9_1_options(MachineClass *m)
> > > > +{
> > > > +pc_i440fx_machine_9_2_options(m);
> > > > +m->alias = NULL;
> > > > +m->is_default = false;
> > > > +compat_props_add(m->compat_props, hw_compat_9_1, 
> > > > hw_compat_9_1_len);
> > > > +compat_props_add(m->compat_props, pc_compat_9_1, 
> > > > pc_compat_9_1_len);
> > > > +}
> > > > +
> > > >  DEFINE_I440FX_MACHINE(9, 1);
> > > >
> > > >  static void pc_i440fx_machine_9_0_options(MachineClass *m)
> > > > @@ -493,8 +504,6 @@ static void 
> > > > pc_i440fx_machine_9_0_options(MachineClass *m)
> > > >  PCMachineClass *pcmc = PC_MACHINE_CLASS(m);
> > > >
> > > >  pc_i440fx_machine_9_1_options(m);
> > > > -m->alias = NULL;
> > > > -m->is_default = false;
> > > >  m->smbios_memory_device_size = 16 * GiB;
> > >
> > > Feels like we should be adding an "_AS_LATEST" macro
> > > variant for piix/q35 too, so it matches the pattern
> > > in other targets for handling alias & is_default.
> > >
> > > Not a thing your patch needs todo though.
> >
> > I've just a patch that does that now. If it looks good & you want to include
> > it as a pre-requisite for your patch here feel free to grab, otherwise I can
> > rebase it after your patch merges.
> 
> I have this patch in my target-arm pullreq that's currently posted
> and pending merge, by the way.

Ah ok, i'll rebase on top of that. then

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 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Philippe Mathieu-Daudé]

On 6/9/24 10:04, Thomas Huth wrote:

On 06/09/2024 09.59, Philippe Mathieu-Daudé wrote:

Hi Thomas,

On 5/9/24 21:14, Thomas Huth wrote:
Let's make sure that we always pass a machine name to the 
test_boot_orders()

function, so we can check whether the machine is available in the binary
and skip the test in case it is not included in the build.

Signed-off-by: Thomas Huth 
---
  tests/qtest/boot-order-test.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/boot-order-test.c 
b/tests/qtest/boot-order-test.c

index 8f2b6ef05a..c67b8cfe16 100644
--- a/tests/qtest/boot-order-test.c
+++ b/tests/qtest/boot-order-test.c
@@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
  uint64_t actual;
  QTestState *qts;
-    if (machine && !qtest_has_machine(machine)) {
+    if (!qtest_has_machine(machine)) {


Should we defer the NULL check to qtest_has_machine_with_env()?
It uses g_str_equal() which is described as:

   Note that this function is primarily meant as a hash table
   comparison function. For a general-purpose, NULL-safe string
   comparison function, see g_strcmp0().

Better switch to g_strcmp0() in qtest_has_machine_with_env()?


What would be the intended meaning of the function when it is called 
with "NULL" ? Use the default machine? Skip the test? ... I think it is 
rather an error to call it with NULL, so it's OK if the test crashes 
here since it should never happen?


Right.

Reviewed-by: Philippe Mathieu-Daudé 

[PATCH] target/riscv/cpu_helper: Fix linking problem with semihosting disabled

[Thomas Huth]

When QEMU has been configured with "--without-default-devices", the build
is currently failing with:

 /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o:
  in function `riscv_cpu_do_interrupt':
 .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined
  reference to `do_common_semihosting'

Avoid calling into do_common_semihosting() if the corresponding Kconfig
switch has not been set.

Signed-off-by: Thomas Huth 
---
 target/riscv/cpu_helper.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 395a1d9140..c7a6569e2d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -34,6 +34,10 @@
 #include "debug.h"
 #include "tcg/oversized-guest.h"
 
+#ifndef CONFIG_USER_ONLY
+#include CONFIG_DEVICES
+#endif
+
 int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
 {
 #ifdef CONFIG_USER_ONLY
@@ -1674,10 +1678,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 if (!async) {
 /* set tval to badaddr for traps with address information */
 switch (cause) {
+#ifdef CONFIG_ARM_COMPATIBLE_SEMIHOSTING
 case RISCV_EXCP_SEMIHOST:
 do_common_semihosting(cs);
 env->pc += 4;
 return;
+#endif
 case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
 case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
 case RISCV_EXCP_LOAD_ADDR_MIS:
-- 
2.46.0

Re: [RFC PATCH 0/2] qtest: Log verbosity changes

[Daniel P . Berrangé]

On Fri, Sep 06, 2024 at 08:16:31AM +0200, Thomas Huth wrote:
> On 05/09/2024 23.03, Fabiano Rosas wrote:
> > Hi,
> > 
> > This series silences QEMU stderr unless the QTEST_LOG variable is set
> > and silences -qtest-log unless both QTEST_LOG and gtest's --verbose
> > flag is passed.
> > 
> > This was motivated by Peter Maydell's ask to suppress deprecation
> > warn_report messages from the migration-tests and by my own
> > frustration over noisy output from qtest.
> 
> Not sure whether we want to ignore stderr by default... we might also miss
> important warnings or error messages that way...?

I would prefer if our tests were quiet by default, and just printed
clear pass/fail notices without extraneous fluff. Having an opt-in
to see full messages from stderr feels good enough for debugging cases
where you need more info from a particular test.

Probably we should set verbose mode in CI though, since it is tedious
to re-run CI on failure to gather more info

> If you just want to suppress one certain warning, I think it's maybe best to
> fence it with "if (!qtest_enabled()) { ... }" on the QEMU side - at least
> that's what we did in similar cases a couple of times, IIRC.

We're got a surprisingly large mumber of if(qtest_enabled()) conditions
in the code. I can't help feeling this is a bad idea in the long term,
as its making us take different codepaths when testing from production.

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 7/8] Revert "target/riscv: Restrict semihosting to TCG"

[Thomas Huth]

On 05/09/2024 21.53, Peter Maydell wrote:

On Thu, 5 Sept 2024 at 20:16, Thomas Huth  wrote:


This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.

Using "imply" instead of "select" is causing a build failure:

  /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o: in 
function `riscv_cpu_do_interrupt':
  .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined reference 
to `do_common_semihosting'

Thus revert to fix the build.

Signed-off-by: Thomas Huth 
---
  target/riscv/Kconfig | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index c332616d36..5f30df22f2 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
  config RISCV32
  bool
-imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
  select DEVICE_TREE # needed by boot.c

  config RISCV64
  bool
-imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
  select DEVICE_TREE # needed by boot.c


This will break the intended "we don't need semihosting if
this is a KVM-only compile", though. Can we fix the
build problem use see with
  "select ARM_COMPATIBLE_SEMIHOSTING if TCG"


I haven't tried, but I assume that this will produce the same linking issues 
when TCG is disabled. Maybe best if we fix it in the code, see the patch 
suggested here:


 https://lore.kernel.org/qemu-devel/20240906080928.710051-1-th...@redhat.com/

 Thomas

Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"

[Philippe Mathieu-Daudé]

On 5/9/24 21:53, Peter Maydell wrote:

On Thu, 5 Sept 2024 at 20:16, Thomas Huth  wrote:


This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.

Using "imply" instead of "select" is causing a build failure:


(please mention ./configure arguments besides --without-default-devices)



  /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o: in 
function `riscv_cpu_do_interrupt':
  .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined reference 
to `do_common_semihosting'

Thus revert to fix the build.

Signed-off-by: Thomas Huth 
---
  target/riscv/Kconfig | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index c332616d36..5f30df22f2 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
  config RISCV32
  bool
-imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
  select DEVICE_TREE # needed by boot.c

  config RISCV64
  bool
-imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
  select DEVICE_TREE # needed by boot.c


This will break the intended "we don't need semihosting if
this is a KVM-only compile", though. Can we fix the
build problem use see with
  "select ARM_COMPATIBLE_SEMIHOSTING if TCG"


We had this discussion with Paolo in
https://lore.kernel.org/qemu-devel/cabgobfbvjg9bbgcwm-kl+yhjhmw1qlnqdqtocekkw+v3trs...@mail.gmail.com/
Not sure this is as easy as it looks...

I feel the riscv part could be fixed by a respin of:
https://lore.kernel.org/qemu-devel/20230711121453.59138-1-phi...@linaro.org/
where semihosting is restricted to TCG and isn't an issue
anymore for other accelerators such KVM.

Let me have a try.

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[Paolo Bonzini]

On Fri, Sep 6, 2024 at 9:40 AM David Hildenbrand  wrote:
> On 06.09.24 09:37, Paolo Bonzini wrote:
> > Virtio memory devices rely on PCI BARs to expose the contents of memory.
> > Because of this they cannot be used with virtio-mmio or virtio-ccw.  In fact
>
> Guess what I am working on at this very the moment ;)

Ok, then hardcoding VIRTIO_PCI is not nice.

> > @@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED
> >   config VIRTIO_PMEM
> >   bool
> >   default y
> > -depends on VIRTIO
> > +depends on VIRTIO_PCI
>
> depends on VIRTIO_MD ?

No, because VIRTIO_MD is "default n" (and anyway you don't want to
enable it by hand in the --without-default-devices case).

But something like this could be a good alternative if you plan to
support virtio-ccw as well:

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..253e7d3f90a 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -16,6 +16,7 @@ config VIRTIO_PCI
 default y if PCI_DEVICES
 depends on PCI
 select VIRTIO
+select VIRTIO_MD_SUPPORTED

 config VIRTIO_MMIO
 bool
@@ -35,8 +36,14 @@ config VIRTIO_CRYPTO
 default y
 depends on VIRTIO

+# not all virtio transports support memory devices; if none does,
+# no need to include the code
+config VIRTIO_MD_SUPPORTED
+bool
+
 config VIRTIO_MD
 bool
+depends on VIRTIO_MD_SUPPORTED
 select MEM_DEVICE

 config VIRTIO_PMEM_SUPPORTED
@@ -46,6 +51,7 @@ config VIRTIO_PMEM
 bool
 default y
 depends on VIRTIO
+depends on VIRTIO_MD_SUPPORTED
 depends on VIRTIO_PMEM_SUPPORTED
 select VIRTIO_MD

@@ -57,6 +63,7 @@ config VIRTIO_MEM
 default y
 depends on VIRTIO
 depends on LINUX
+depends on VIRTIO_MD_SUPPORTED
 depends on VIRTIO_MEM_SUPPORTED
 select VIRTIO_MD


and then you just need to select VIRTIO_MD_SUPPORTED from VIRTIO_CCW.
In the case of PCI there is some board support code as well, which is
why VIRTIO_{MEM,PMEM}_SUPPORTED is selected from "config PC", but
perhaps in the s390 code you can select those three from VIRTIO_CCW as
well.

If this looks good I'll send it as v2.

Thanks,

Paolo

Re: [PULL 13/16] target/riscv: Restrict semihosting to TCG

[Thomas Huth]

On 05/09/2024 19.08, Thomas Huth wrote:

On 22/07/2024 13.04, Alex Bennée wrote:

From: Philippe Mathieu-Daudé 

Semihosting currently uses the TCG probe_access API. To prepare for
encoding the TCG dependency in Kconfig, do not enable it unless TCG
is available.

Suggested-by: Paolo Bonzini 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Anton Johansson 
Message-Id: <20240717105723.58965-7-phi...@linaro.org>
Signed-off-by: Alex Bennée 
Message-Id: <20240718094523.1198645-14-alex.ben...@linaro.org>

diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index 5f30df22f2..c332616d36 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
  config RISCV32
  bool
-    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
+    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
  select DEVICE_TREE # needed by boot.c
  config RISCV64
  bool
-    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
+    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
  select DEVICE_TREE # needed by boot.c


  Hi,

this patch broke compilation with "--without-default-devices":

/usr/bin/ld: libqemu-riscv64-softmmu.a.p/target_riscv_cpu_helper.c.o: in 
function `riscv_cpu_do_interrupt':
.../qemu/target/riscv/cpu_helper.c:1678:(.text+0x283c): undefined reference 
to `do_common_semihosting'


Could you please have a look? I think we either have to revert to "select" 
instead of "imply", or you might need to put some "#if 
CONFIG_ARM_COMPATIBLE_SEMIHOSTING" into the code that calls 
do_common_semihosting() ...?


Suggested a patch here now:

https://lore.kernel.org/qemu-devel/20240906080928.710051-1-th...@redhat.com/

 Thomas

Re: [PATCH v2] docs: fix vhost-user protocol doc

[Michael S. Tsirkin]

On Fri, Sep 06, 2024 at 10:10:45AM +0800, luzhixing12345 wrote:
> Hi, can someone help review this patch?
> 
> Signed-off-by: luzhixing12345 

You got comments Aug 5, pls address them.

Re: [PATCH] hw/loongarch: virt: support up to 4 serial ports

[maobibo]

On 2024/9/6 下午12:49, Jason A. Donenfeld wrote:

In order to support additional channels of communication using
`-serial`, add several serial ports, up to the standard 4 generally
supported by the 8250 driver.

Signed-off-by: Jason A. Donenfeld 
---
  hw/loongarch/virt.c| 24 ++--
  include/hw/pci-host/ls7a.h |  9 +
  2 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index 4151fc5e0c..155678b684 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -319,10 +319,10 @@ static void fdt_add_ged_reset(LoongArchVirtMachineState 
*lvms)
  }
  
  static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,

-  uint32_t *pch_pic_phandle)
+  uint32_t *pch_pic_phandle, hwaddr base,
+  int irq, bool chosen)
  {
  char *nodename;
-hwaddr base = VIRT_UART_BASE;
  hwaddr size = VIRT_UART_SIZE;
  MachineState *ms = MACHINE(lvms);
  
@@ -331,9 +331,9 @@ static void fdt_add_uart_node(LoongArchVirtMachineState *lvms,

  qemu_fdt_setprop_string(ms->fdt, nodename, "compatible", "ns16550a");
  qemu_fdt_setprop_cells(ms->fdt, nodename, "reg", 0x0, base, 0x0, size);
  qemu_fdt_setprop_cell(ms->fdt, nodename, "clock-frequency", 1);
-qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
-qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts",
-   VIRT_UART_IRQ - VIRT_GSI_BASE, 0x4);
+if (chosen)
+qemu_fdt_setprop_string(ms->fdt, "/chosen", "stdout-path", nodename);
+qemu_fdt_setprop_cells(ms->fdt, nodename, "interrupts", irq, 0x4);
  qemu_fdt_setprop_cell(ms->fdt, nodename, "interrupt-parent",
*pch_pic_phandle);
  g_free(nodename);
@@ -750,11 +750,15 @@ static void virt_devices_init(DeviceState *pch_pic,
  /* Add pcie node */
  fdt_add_pcie_node(lvms, pch_pic_phandle, pch_msi_phandle);
  
-serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,

-   qdev_get_gpio_in(pch_pic,
-VIRT_UART_IRQ - VIRT_GSI_BASE),
-   115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
-fdt_add_uart_node(lvms, pch_pic_phandle);
+for (i = 0; i < VIRT_UART_COUNT; ++i) {

How about adding serial_hd(i) checking here, such as
  for (i = 0; (i < VIRT_UART_COUNT) && serial_hd(i); ++i) {


+hwaddr base = VIRT_UART_BASE + i * VIRT_UART_SIZE;
+int irq = VIRT_UART_IRQ + i - VIRT_GSI_BASE;
+serial_mm_init(get_system_memory(), base, 0,
+   qdev_get_gpio_in(pch_pic, irq),
+   115200, serial_hd(VIRT_UART_COUNT - 1 - i),
is it serial_hd(i) here rather than serial_hd(VIRT_UART_COUNT - 1 - i)? 
In general serial_hd(0) is default serial.



+   DEVICE_LITTLE_ENDIAN);
+fdt_add_uart_node(lvms, pch_pic_phandle, base, irq, i == 0);
+}
  
  /* Network init */

  pci_init_nic_devices(pci_bus, mc->default_nic);
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index cd7c9ec7bc..79d4ea8501 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -36,17 +36,18 @@
  #define VIRT_PCH_PIC_IRQ_NUM 32
  #define VIRT_GSI_BASE64
  #define VIRT_DEVICE_IRQS 16
+#define VIRT_UART_COUNT  4
  #define VIRT_UART_IRQ(VIRT_GSI_BASE + 2)
  #define VIRT_UART_BASE   0x1fe001e0
-#define VIRT_UART_SIZE   0X100
-#define VIRT_RTC_IRQ (VIRT_GSI_BASE + 3)
+#define VIRT_UART_SIZE   0x100
+#define VIRT_RTC_IRQ (VIRT_GSI_BASE + 6)
  #define VIRT_MISC_REG_BASE   (VIRT_PCH_REG_BASE + 0x0008)
  #define VIRT_RTC_REG_BASE(VIRT_MISC_REG_BASE + 0x00050100)
  #define VIRT_RTC_LEN 0x100
-#define VIRT_SCI_IRQ (VIRT_GSI_BASE + 4)
+#define VIRT_SCI_IRQ (VIRT_GSI_BASE + 7)
  
  #define VIRT_PLATFORM_BUS_BASEADDRESS   0x1600

  #define VIRT_PLATFORM_BUS_SIZE  0x200
  #define VIRT_PLATFORM_BUS_NUM_IRQS  2
-#define VIRT_PLATFORM_BUS_IRQ   (VIRT_GSI_BASE + 5)
+#define VIRT_PLATFORM_BUS_IRQ   (VIRT_GSI_BASE + 8)
  #endif


By the way, serial port for acpi table should be refreshed also, such as
diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index 2638f87434..5bd2a9beaa 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -31,6 +31,7 @@

 #include "hw/acpi/generic_event_device.h"
 #include "hw/pci-host/gpex.h"
+#include "sysemu/sysemu.h"
 #include "sysemu/tpm.h"
 #include "hw/platform-bus.h"
 #include "hw/acpi/aml-build.h"
@@ -252,23 +253,26 @@ struct AcpiBuildState {
 MemoryRegion *linker_mr;
 } AcpiBuildState;

-static void build_uart_device_aml(Aml *table)
+static void build_uart_device_aml(Aml *table, int index)
 {
 Aml *dev;
 Aml *crs;
 Aml *pkg0, *pkg1, 

Re: [PATCH] target/riscv/cpu_helper: Fix linking problem with semihosting disabled

[Peter Maydell]

On Fri, 6 Sept 2024 at 09:09, Thomas Huth  wrote:
>
> When QEMU has been configured with "--without-default-devices", the build
> is currently failing with:
>
>  /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o:
>   in function `riscv_cpu_do_interrupt':
>  .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined
>   reference to `do_common_semihosting'
>
> Avoid calling into do_common_semihosting() if the corresponding Kconfig
> switch has not been set.

This would be inconsistent with Arm, where you always
get semihosting if you're using TCG. (For KVM, semihosting
is up to the kernel to provide, which is why we don't
want the code in that case.)

> Signed-off-by: Thomas Huth 
> ---
>  target/riscv/cpu_helper.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 395a1d9140..c7a6569e2d 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -34,6 +34,10 @@
>  #include "debug.h"
>  #include "tcg/oversized-guest.h"
>
> +#ifndef CONFIG_USER_ONLY
> +#include CONFIG_DEVICES
> +#endif
> +
>  int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
>  {
>  #ifdef CONFIG_USER_ONLY
> @@ -1674,10 +1678,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
>  if (!async) {
>  /* set tval to badaddr for traps with address information */
>  switch (cause) {
> +#ifdef CONFIG_ARM_COMPATIBLE_SEMIHOSTING
>  case RISCV_EXCP_SEMIHOST:
>  do_common_semihosting(cs);
>  env->pc += 4;
>  return;
> +#endif
>  case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
>  case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
>  case RISCV_EXCP_LOAD_ADDR_MIS:

If you do want to do this thhen this isn't sufficient, because
you would also need to change the code that generates the
RISCV_EXCP_SEMIHOST exception so that it instead generates
the "behave as if we don't have semihosting and the
semihosting-trap instruction sequence were executed "normally".
But I think the best thing is to use "select if TCG" in the Kconfig.

thanks
-- PMM

Re: [PATCH] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

On 06.09.24 10:18, Paolo Bonzini wrote:

On Fri, Sep 6, 2024 at 9:40 AM David Hildenbrand  wrote:

On 06.09.24 09:37, Paolo Bonzini wrote:

Virtio memory devices rely on PCI BARs to expose the contents of memory.
Because of this they cannot be used with virtio-mmio or virtio-ccw.  In fact


Guess what I am working on at this very the moment ;)


Ok, then hardcoding VIRTIO_PCI is not nice.


@@ -45,7 +46,7 @@ config VIRTIO_PMEM_SUPPORTED
   config VIRTIO_PMEM
   bool
   default y
-depends on VIRTIO
+depends on VIRTIO_PCI


depends on VIRTIO_MD ?


No, because VIRTIO_MD is "default n" (and anyway you don't want to
enable it by hand in the --without-default-devices case).


Right, that's what I originally tried to achieve.



But something like this could be a good alternative if you plan to
support virtio-ccw as well:

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..253e7d3f90a 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -16,6 +16,7 @@ config VIRTIO_PCI
  default y if PCI_DEVICES
  depends on PCI
  select VIRTIO
+select VIRTIO_MD_SUPPORTED

  config VIRTIO_MMIO
  bool
@@ -35,8 +36,14 @@ config VIRTIO_CRYPTO
  default y
  depends on VIRTIO

+# not all virtio transports support memory devices; if none does,
+# no need to include the code
+config VIRTIO_MD_SUPPORTED
+bool
+
  config VIRTIO_MD
  bool
+depends on VIRTIO_MD_SUPPORTED
  select MEM_DEVICE

  config VIRTIO_PMEM_SUPPORTED
@@ -46,6 +51,7 @@ config VIRTIO_PMEM
  bool
  default y
  depends on VIRTIO
+depends on VIRTIO_MD_SUPPORTED
  depends on VIRTIO_PMEM_SUPPORTED
  select VIRTIO_MD

@@ -57,6 +63,7 @@ config VIRTIO_MEM
  default y
  depends on VIRTIO
  depends on LINUX
+depends on VIRTIO_MD_SUPPORTED
  depends on VIRTIO_MEM_SUPPORTED
  select VIRTIO_MD


and then you just need to select VIRTIO_MD_SUPPORTED from VIRTIO_CCW.


Sounds good.


In the case of PCI there is some board support code as well, which is
why VIRTIO_{MEM,PMEM}_SUPPORTED is selected from "config PC", but
perhaps in the s390 code you can select those three from VIRTIO_CCW as
well.


Yes, only VIRTIO_MEM_SUPPORTED for now.



If this looks good I'll send it as v2.



Thanks!

--
Cheers,

David / dhildenb

Re: [PATCH] target/riscv/cpu_helper: Fix linking problem with semihosting disabled

[Thomas Huth]

On 06/09/2024 10.58, Peter Maydell wrote:

On Fri, 6 Sept 2024 at 09:09, Thomas Huth  wrote:


When QEMU has been configured with "--without-default-devices", the build
is currently failing with:

  /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o:
   in function `riscv_cpu_do_interrupt':
  .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined
   reference to `do_common_semihosting'

Avoid calling into do_common_semihosting() if the corresponding Kconfig
switch has not been set.


This would be inconsistent with Arm, where you always
get semihosting if you're using TCG. (For KVM, semihosting
is up to the kernel to provide, which is why we don't
want the code in that case.)


Signed-off-by: Thomas Huth 
---
  target/riscv/cpu_helper.c | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 395a1d9140..c7a6569e2d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -34,6 +34,10 @@
  #include "debug.h"
  #include "tcg/oversized-guest.h"

+#ifndef CONFIG_USER_ONLY
+#include CONFIG_DEVICES
+#endif
+
  int riscv_env_mmu_index(CPURISCVState *env, bool ifetch)
  {
  #ifdef CONFIG_USER_ONLY
@@ -1674,10 +1678,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
  if (!async) {
  /* set tval to badaddr for traps with address information */
  switch (cause) {
+#ifdef CONFIG_ARM_COMPATIBLE_SEMIHOSTING
  case RISCV_EXCP_SEMIHOST:
  do_common_semihosting(cs);
  env->pc += 4;
  return;
+#endif
  case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
  case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
  case RISCV_EXCP_LOAD_ADDR_MIS:


If you do want to do this thhen this isn't sufficient, because
you would also need to change the code that generates the
RISCV_EXCP_SEMIHOST exception so that it instead generates
the "behave as if we don't have semihosting and the
semihosting-trap instruction sequence were executed "normally".
But I think the best thing is to use "select if TCG" in the Kconfig.


Ok, but I think we then still need a #ifdef CONFIG_TCG here, otherwise 
linking will fail for KVM-only builds?


 Thomas

Re: [PATCH 0/3] hw/sh4: Remove the deprecated SHIX machine

[Yoshinori Sato]

On Wed, 04 Sep 2024 00:39:56 +0900,
Philippe Mathieu-Daudé wrote:
> 
> Remove the deprecated SH4 SHIX machine, along
> with the TC58128 NAND EEPROM.
> 
> Philippe Mathieu-Daudé (3):
>   hw/sh4: Remove the deprecated SHIX machine
>   hw/block: Remove TC58128 NAND EEPROM
>   hw/sh4: Remove sh7750_register_io_device() helper
> 
>  MAINTAINERS |  11 --
>  docs/about/deprecated.rst   |   6 -
>  docs/about/removed-features.rst |   5 +
>  configs/devices/sh4-softmmu/default.mak |   1 -
>  include/hw/sh4/sh.h |  19 ---
>  hw/block/tc58128.c  | 211 
>  hw/sh4/sh7750.c |  57 +--
>  hw/sh4/shix.c   |  86 --
>  hw/block/Kconfig|   3 -
>  hw/block/meson.build|   1 -
>  hw/sh4/Kconfig  |   7 -
>  hw/sh4/meson.build  |   1 -
>  12 files changed, 7 insertions(+), 401 deletions(-)
>  delete mode 100644 hw/block/tc58128.c
>  delete mode 100644 hw/sh4/shix.c
> 
> -- 
> 2.45.2
> 

for hw/sh4
Reviewed-by: Yoshinori Sato 

-- 
Yosinori Sato

Re: [PATCH] target/riscv/cpu_helper: Fix linking problem with semihosting disabled

[Peter Maydell]

On Fri, 6 Sept 2024 at 10:30, Thomas Huth  wrote:
>
> On 06/09/2024 10.58, Peter Maydell wrote:
> > On Fri, 6 Sept 2024 at 09:09, Thomas Huth  wrote:
> >>
> >> When QEMU has been configured with "--without-default-devices", the build
> >> is currently failing with:
> >>
> >>   /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o:
> >>in function `riscv_cpu_do_interrupt':
> >>   .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined
> >>reference to `do_common_semihosting'
> >>
> >> Avoid calling into do_common_semihosting() if the corresponding Kconfig
> >> switch has not been set.
> >
> > This would be inconsistent with Arm, where you always
> > get semihosting if you're using TCG. (For KVM, semihosting
> > is up to the kernel to provide, which is why we don't
> > want the code in that case.)

> > If you do want to do this thhen this isn't sufficient, because
> > you would also need to change the code that generates the
> > RISCV_EXCP_SEMIHOST exception so that it instead generates
> > the "behave as if we don't have semihosting and the
> > semihosting-trap instruction sequence were executed "normally".
> > But I think the best thing is to use "select if TCG" in the Kconfig.
>
> Ok, but I think we then still need a #ifdef CONFIG_TCG here, otherwise
> linking will fail for KVM-only builds?

Yes, I expect so -- the equivalent code in target/arm is
wrapped in #ifdef CONFIG_TCG.

-- PMM

[PATCH v2] target/riscv/cpu_helper: Fix linking problem with semihosting disabled

[Thomas Huth]

If QEMU has been configured with "--without-default-devices", the build
is currently failing with:

 /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o:
  in function `riscv_cpu_do_interrupt':
 .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined
  reference to `do_common_semihosting'

We always want semihosting to be enabled if TCG is available, so change
the "imply" statements in the Kconfig file to "select", and make sure to
avoid calling into do_common_semihosting() if TCG is not available.

Signed-off-by: Thomas Huth 
---
 v2: Use "select" in the Kconfig file, and "CONFIG_TCG" in the #ifdef

 target/riscv/cpu_helper.c | 2 ++
 target/riscv/Kconfig  | 4 ++--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 395a1d9140..dc147181a3 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1674,10 +1674,12 @@ void riscv_cpu_do_interrupt(CPUState *cs)
 if (!async) {
 /* set tval to badaddr for traps with address information */
 switch (cause) {
+#ifdef CONFIG_TCG
 case RISCV_EXCP_SEMIHOST:
 do_common_semihosting(cs);
 env->pc += 4;
 return;
+#endif
 case RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT:
 case RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT:
 case RISCV_EXCP_LOAD_ADDR_MIS:
diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index c332616d36..11bc09b414 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
 config RISCV32
 bool
-imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+select ARM_COMPATIBLE_SEMIHOSTING if TCG
 select DEVICE_TREE # needed by boot.c
 
 config RISCV64
 bool
-imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+select ARM_COMPATIBLE_SEMIHOSTING if TCG
 select DEVICE_TREE # needed by boot.c
-- 
2.46.0

Re: [RFC PATCH 0/2] qtest: Log verbosity changes

[Peter Maydell]

On Fri, 6 Sept 2024 at 09:14, Daniel P. Berrangé  wrote:
>
> On Fri, Sep 06, 2024 at 08:16:31AM +0200, Thomas Huth wrote:
> > On 05/09/2024 23.03, Fabiano Rosas wrote:
> > > Hi,
> > >
> > > This series silences QEMU stderr unless the QTEST_LOG variable is set
> > > and silences -qtest-log unless both QTEST_LOG and gtest's --verbose
> > > flag is passed.
> > >
> > > This was motivated by Peter Maydell's ask to suppress deprecation
> > > warn_report messages from the migration-tests and by my own
> > > frustration over noisy output from qtest.

This isn't what I want, though -- what I want is that a
qtest run should not print "warning:" messages for things
that we expect to happen when we run that test. I *do* want
warnings for things that we do not expect to happen when
we run the test.

> > Not sure whether we want to ignore stderr by default... we might also miss
> > important warnings or error messages that way...?
>
> I would prefer if our tests were quiet by default, and just printed
> clear pass/fail notices without extraneous fluff. Having an opt-in
> to see full messages from stderr feels good enough for debugging cases
> where you need more info from a particular test.

I find it is not uncommon that something fails and
you don't necessarily have the option to re-run it with
the "give me the error message this time" flag turn on.
CI is just the most obvious example; other kinds of
intermittent failure can be similar.

> Probably we should set verbose mode in CI though, since it is tedious
> to re-run CI on failure to gather more info
>
> > If you just want to suppress one certain warning, I think it's maybe best to
> > fence it with "if (!qtest_enabled()) { ... }" on the QEMU side - at least
> > that's what we did in similar cases a couple of times, IIRC.
>
> We're got a surprisingly large mumber of if(qtest_enabled()) conditions
> in the code. I can't help feeling this is a bad idea in the long term,
> as its making us take different codepaths when testing from production.

What I want from CI and from tests in general:
 * if something fails, I want all the information
 * if something unexpected happens I want the warning even
   if the test passes (this is why I grep the logs for
   "warning:" in the first place -- it is to catch the case
   of "something went wrong but it didn't result in QEMU or
   the test case exiting with a failure status")
 * if something is expected, it should be silent

That means there's a class of messages where we want to warn
the user that they're doing something that might not be what
they intended or which is deprecated and will go away soon,
but where we do not want to "warn" in the test logging because
the test is deliberately setting up that oddball corner case.

It might be useful to have a look at where we're using
if (qtest_enabled()) to see if we can make some subcategories
avoid the explicit if(), e.g. by having a warn_deprecated(...)
and hide the "don't print if qtest" inside the function.

Some categories as a starter:
 * some board models will error-and-exit if the user
   didn't provide any guest code (eg no -kernel option),
   like hw/m68k/an5206.c. When we're running with the
   qtest accelerator it's fine and expected that there's
   no guest code loaded because we'll never run any guest code
 * in some places (eg target/arm/cpu.c) we treat qtest as
   another accelerator type, so we might say
   if (tcg_enabled() || qtest_enabled()) to mean "not
   hvf or kvm"
 * sometimes we print a deprecation message only if
   not qtest, eg hw/core/numa.c or hw/core/machine.c
 * the clock related code needs to be qtest aware because
   under qtest it's the qtest protocol that advances the
   clock
 * sometimes we warn about possible user error if not
   qtest, eg hw/ppc/pnv.c or target/mips/cpu.c

thanks
-- PMM

Re: [PATCH v2 0/7] Report fatal errors from failure with pre-opened eBPF RSS FDs

[Michael S. Tsirkin]

On Thu, Sep 05, 2024 at 07:13:23PM +0100, Daniel P. Berrangé wrote:
> The virtio-net code for eBPF RSS is still ignoring errors when
> failing to load the eBPF RSS program passed in by the mgmt app
> via pre-opened FDs.
> 
> This series re-factors the eBPF common code so that it actually
> reports using "Error" objects. Then it makes virtio-net treat
> a failure to load pre-opened FDs as a fatal problem. When doing
> speculative opening of eBPF FDs, QEMU merely prints a warning,
> and allows the software fallback to continue.
> 
> Trace event coverage is significantly expanded to make this all
> much more debuggable too.


looks good
Reviewed-by: Michael S. Tsirkin 

Jason's tree.

> Changed in v2:
> 
>  - Split 'ebpf_error' probe into multiple probes
> 
> Daniel P. Berrangé (7):
>   hw/net: fix typo s/epbf/ebpf/ in virtio-net
>   ebpf: drop redundant parameter checks in static methods
>   ebpf: improve error trace events
>   ebpf: add formal error reporting to all APIs
>   hw/net: report errors from failing to use eBPF RSS FDs
>   ebpf: improve trace event coverage to all key operations
>   hw/net: improve tracing of eBPF RSS setup
> 
>  ebpf/ebpf_rss.c | 118 
>  ebpf/ebpf_rss.h |  10 ++--
>  ebpf/trace-events   |   8 ++-
>  hw/net/trace-events |   8 +--
>  hw/net/virtio-net.c |  63 +++
>  5 files changed, 137 insertions(+), 70 deletions(-)
> 
> -- 
> 2.45.2

Re: [PATCH v2 4/7] ebpf: add formal error reporting to all APIs

[Philippe Mathieu-Daudé]

On 5/9/24 20:13, Daniel P. Berrangé wrote:

The eBPF code is currently reporting error messages through trace
events. Trace events are fine for debugging, but they are not to be
considered the primary error reporting mechanism, as their output
is inaccessible to callers.

This adds an "Error **errp" parameter to all methods which have
important error scenarios to report to the caller.

Signed-off-by: Daniel P. Berrangé 
---
  ebpf/ebpf_rss.c | 59 -
  ebpf/ebpf_rss.h | 10 +---
  hw/net/virtio-net.c |  7 +++---
  3 files changed, 59 insertions(+), 17 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 5/7] hw/net: report errors from failing to use eBPF RSS FDs

[Philippe Mathieu-Daudé]

On 5/9/24 20:13, Daniel P. Berrangé wrote:

If the user/mgmt app passed in a set of pre-opened FDs for eBPF RSS,
then it is expecting QEMU to use them. Any failure to do so must be
considered a fatal error and propagated back up the stack, otherwise
deployment mistakes will not be detectable in a prompt manner. When
not using pre-opened FDs, then eBPF RSS is tried on a "best effort"
basis only and thus fallback to software RSS is valid.

Signed-off-by: Daniel P. Berrangé 
---
  hw/net/virtio-net.c | 41 +
  1 file changed, 29 insertions(+), 12 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 6/7] ebpf: improve trace event coverage to all key operations

[Philippe Mathieu-Daudé]

On 5/9/24 20:13, Daniel P. Berrangé wrote:

The existing error trace event is renamed to have a name prefix
matching its source file & to remove the redundant first arg that
adds no useful information.

Signed-off-by: Daniel P. Berrangé 
---
  ebpf/ebpf_rss.c   | 19 +++
  ebpf/trace-events |  4 
  2 files changed, 23 insertions(+)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 7/7] hw/net: improve tracing of eBPF RSS setup

[Philippe Mathieu-Daudé]

On 5/9/24 20:13, Daniel P. Berrangé wrote:

This adds more trace events to key eBPF RSS setup operations, and
also distinguishes events from multiple NIC instances.

Signed-off-by: Daniel P. Berrangé 
---
  hw/net/trace-events | 8 +---
  hw/net/virtio-net.c | 9 ++---
  2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/hw/net/trace-events b/hw/net/trace-events
index 78efa2ec2c..6cad34aba2 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -399,9 +399,11 @@ virtio_net_announce_notify(void) ""
  virtio_net_announce_timer(int round) "%d"
  virtio_net_handle_announce(int round) "%d"
  virtio_net_post_load_device(void)
-virtio_net_rss_disable(void)
-virtio_net_rss_error(const char *msg, uint32_t value) "%s, value 0x%08x"
-virtio_net_rss_enable(uint32_t p1, uint16_t p2, uint8_t p3) "hashes 0x%x, table of 
%d, key of %d"
+virtio_net_rss_load(void *nic, size_t nfds, void *fds) "nic=%p nfds=%zu fds=%p"
+virtio_net_rss_attach_ebpf(void *nic, int prog_fd) "nic=%p prog-fd=%d"
+virtio_net_rss_disable(void *nic) "nic=%p"
+virtio_net_rss_error(void *nic, const char *msg, uint32_t value) "nic=%p msg=%s, 
value 0x%08x"
+virtio_net_rss_enable(void *nic, uint32_t p1, uint16_t p2, uint8_t p3) "nic=%p 
hashes 0x%x, table of %d, key of %d"


Nitpicking for pre-existing error, unsigned format is %u ;)

Reviewed-by: Philippe Mathieu-Daudé 

[PATCH v2] virtio: kconfig: memory devices are PCI only

[Paolo Bonzini]

Virtio memory devices rely on PCI BARs to expose the contents of memory.
Because of this they cannot be used (yet) with virtio-mmio or virtio-ccw.
In fact the code that is common to virtio-mem and virtio-pmem, which
is in hw/virtio/virtio-md-pci.c, is only included if CONFIG_VIRTIO_PCI
is set.  Reproduce the same condition in the Kconfig file, only allowing
VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.

Without this patch it is possible to create a configuration with
CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
linking failure.

Cc: David Hildenbrand 
Reported-by: Michael Tokarev 
Signed-off-by: Paolo Bonzini 
---
 hw/virtio/Kconfig | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/virtio/Kconfig b/hw/virtio/Kconfig
index aa63ff7fd41..0afec2ae929 100644
--- a/hw/virtio/Kconfig
+++ b/hw/virtio/Kconfig
@@ -16,6 +16,7 @@ config VIRTIO_PCI
 default y if PCI_DEVICES
 depends on PCI
 select VIRTIO
+select VIRTIO_MD_SUPPORTED
 
 config VIRTIO_MMIO
 bool
@@ -35,10 +36,17 @@ config VIRTIO_CRYPTO
 default y
 depends on VIRTIO
 
+# not all virtio transports support memory devices; if none does,
+# no need to include the code
+config VIRTIO_MD_SUPPORTED
+bool
+
 config VIRTIO_MD
 bool
+depends on VIRTIO_MD_SUPPORTED
 select MEM_DEVICE
 
+# selected by the board if it has the required support code
 config VIRTIO_PMEM_SUPPORTED
 bool
 
@@ -46,9 +54,11 @@ config VIRTIO_PMEM
 bool
 default y
 depends on VIRTIO
+depends on VIRTIO_MD_SUPPORTED
 depends on VIRTIO_PMEM_SUPPORTED
 select VIRTIO_MD
 
+# selected by the board if it has the required support code
 config VIRTIO_MEM_SUPPORTED
 bool
 
@@ -57,6 +67,7 @@ config VIRTIO_MEM
 default y
 depends on VIRTIO
 depends on LINUX
+depends on VIRTIO_MD_SUPPORTED
 depends on VIRTIO_MEM_SUPPORTED
 select VIRTIO_MD
 
-- 
2.46.0

Re: [PATCH v2] virtio: kconfig: memory devices are PCI only

[David Hildenbrand]

On 06.09.24 12:16, Paolo Bonzini wrote:

Virtio memory devices rely on PCI BARs to expose the contents of memory.
Because of this they cannot be used (yet) with virtio-mmio or virtio-ccw.
In fact the code that is common to virtio-mem and virtio-pmem, which
is in hw/virtio/virtio-md-pci.c, is only included if CONFIG_VIRTIO_PCI
is set.  Reproduce the same condition in the Kconfig file, only allowing
VIRTIO_MEM and VIRTIO_PMEM to be defined if the transport supports it.

Without this patch it is possible to create a configuration with
CONFIG_VIRTIO_PCI=n and CONFIG_VIRTIO_MEM=y, but that causes a
linking failure.

Cc: David Hildenbrand 
Reported-by: Michael Tokarev 
Signed-off-by: Paolo Bonzini 
---


Reviewed-by: David Hildenbrand 

Thanks!

--
Cheers,

David / dhildenb

Re: [PATCH RESEND v9 9/9] rust: add PL011 device model

[Paolo Bonzini]

> +impl PL011State {
> +pub fn init(&mut self) {
> +let dev = addr_of_mut!(*self).cast::();

One small thing that I forgot about, is that the init function should be unsafe.

For a short term change after this is merged, this should take not a
&mut self, but a "obj: &mut MaybeUninit".

It needs a utility macro like

fn uninit_field_mut<'a, U>(dest: *mut U) -> &'a mut MaybeUninit {
unsafe { &mut *(dest.cast()) }
}

macro_rules! uninit_field_mut {
($container:expr, $field:ident) => {
$crate::uninit_field_mut(unsafe {
 std::ptr::addr_of_mut!((&mut *$container.as_mut_ptr()).$field)
})
}
}

and initialization code like

p_clock = uninit_field_mut!(self, clock);
p_clock.write(NonNull::new(...));

and it's clearly less pretty; but it avoids comments like

// self.clock is not initialized at this point; but since
`NonNull<_>` is Copy,
// we can overwrite the undefined value without side effects.

and it is safer because MaybeUninit<> prevents calling other functions
of PL011State too early.

It would be nice to just add a TODO here, but it's not necessary.

Paolo

On Wed, Aug 28, 2024 at 6:12 AM Manos Pitsidianakis
 wrote:
>
> This commit adds a re-implementation of hw/char/pl011.c in Rust.
>
> How to build:
>
> 1. Configure a QEMU build with:
>--enable-system --target-list=aarch64-softmmu --enable-rust
> 2. Launching a VM with qemu-system-aarch64 should use the Rust version
>of the pl011 device
>
> Co-authored-by: Junjie Mao 
> Co-authored-by: Paolo Bonzini 
> Signed-off-by: Junjie Mao 
> Signed-off-by: Paolo Bonzini 
> Signed-off-by: Manos Pitsidianakis 
> ---
>  MAINTAINERS|   5 +
>  meson.build|  24 +
>  hw/arm/Kconfig |  33 +-
>  rust/Kconfig   |   1 +
>  rust/hw/Kconfig|   2 +
>  rust/hw/char/Kconfig   |   3 +
>  rust/hw/char/meson.build   |   1 +
>  rust/hw/char/pl011/.gitignore  |   2 +
>  rust/hw/char/pl011/Cargo.lock  | 134 +
>  rust/hw/char/pl011/Cargo.toml  |  26 +
>  rust/hw/char/pl011/README.md   |  31 ++
>  rust/hw/char/pl011/meson.build |  26 +
>  rust/hw/char/pl011/src/definitions.rs  |  20 +
>  rust/hw/char/pl011/src/device.rs   | 594 
> +
>  rust/hw/char/pl011/src/device_class.rs |  59 ++
>  rust/hw/char/pl011/src/lib.rs  | 585 
>  rust/hw/char/pl011/src/memory_ops.rs   |  57 ++
>  rust/hw/meson.build|   1 +
>  rust/meson.build   |   2 +
>  scripts/archive-source.sh  |   5 +-
>  scripts/make-release   |   5 +-
>  scripts/rust/rust_root_crate.sh|  13 +
>  subprojects/.gitignore |  11 +
>  subprojects/arbitrary-int-1-rs.wrap|   7 +
>  subprojects/bilge-0.2-rs.wrap  |   7 +
>  subprojects/bilge-impl-0.2-rs.wrap |   7 +
>  subprojects/either-1-rs.wrap   |   7 +
>  subprojects/itertools-0.11-rs.wrap |   7 +
>  .../packagefiles/arbitrary-int-1-rs/meson.build|  19 +
>  subprojects/packagefiles/bilge-0.2-rs/meson.build  |  29 +
>  .../packagefiles/bilge-impl-0.2-rs/meson.build |  45 ++
>  subprojects/packagefiles/either-1-rs/meson.build   |  24 +
>  .../packagefiles/itertools-0.11-rs/meson.build |  30 ++
>  .../packagefiles/proc-macro-error-1-rs/meson.build |  40 ++
>  .../proc-macro-error-attr-1-rs/meson.build |  32 ++
>  .../packagefiles/proc-macro2-1-rs/meson.build  |  31 ++
>  subprojects/packagefiles/quote-1-rs/meson.build|  29 +
>  subprojects/packagefiles/syn-2-rs/meson.build  |  40 ++
>  .../packagefiles/unicode-ident-1-rs/meson.build|  20 +
>  subprojects/proc-macro-error-1-rs.wrap |   7 +
>  subprojects/proc-macro-error-attr-1-rs.wrap|   7 +
>  subprojects/proc-macro2-1-rs.wrap  |   7 +
>  subprojects/quote-1-rs.wrap|   7 +
>  subprojects/syn-2-rs.wrap  |   7 +
>  subprojects/unicode-ident-1-rs.wrap|   7 +
>  45 files changed, 2043 insertions(+), 13 deletions(-)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 
> 727f3a7a2cfe600ffdb861bafada7db415d020e5..cb65018b4bf3958952ecd7c8d703abee838cc265
>  100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -1189,6 +1189,11 @@ F: include/hw/*/microbit*.h
>  F: tests/qtest/microbit-test.c
>  F: docs/system/arm/nrf.rst
>
> +ARM PL011 Rust device
> +M: Manos Pitsidianakis 
> +S: Maintained
> +F: rust/hw/char/pl011/
> +
>  AVR Machines
>  -

Re: [PATCH v2 1/4] KVM: Dynamic sized kvm memslots array

[Juraj Marcin]

Hi Peter,

On Thu, Sep 5, 2024 at 6:00 PM Peter Xu  wrote:
>
> On Thu, Sep 05, 2024 at 05:32:46PM +0200, Juraj Marcin wrote:
> > Hi Peter,
>
> Hi, Juraj,
>
> [...]
>
> > >  unsigned int kvm_get_max_memslots(void)
> > >  {
> > >  KVMState *s = KVM_STATE(current_accel());
> > > @@ -193,15 +247,20 @@ unsigned int kvm_get_free_memslots(void)
> > >  /* Called with KVMMemoryListener.slots_lock held */
> > >  static KVMSlot *kvm_get_free_slot(KVMMemoryListener *kml)
> > >  {
> > > -KVMState *s = kvm_state;
> > >  int i;
> > >
> > > -for (i = 0; i < s->nr_slots; i++) {
> > > +retry:
> > > +for (i = 0; i < kml->nr_slots_allocated; i++) {
> > >  if (kml->slots[i].memory_size == 0) {
> > >  return &kml->slots[i];
> > >  }
> > >  }
> > >
> > > +/* If no free slots, try to grow first by doubling */
> > > +if (kvm_slots_double(kml)) {
> > > +goto retry;
> >
> > At this point we know all previously allocated slots were used and
> > there should be a free slot just after the last used slot (at the
> > start of the region zeroed in the grow function). Wouldn't it be
> > faster to return it here right away, instead of iterating through
> > slots that should still be used again?
>
> Good question.
>
> One trivial concern is we'll then have assumption on how kvm_slots_double()
> behaves, e.g., it must not move anything around inside, and we need to know

> that it touches nr_slots_allocated so we need to cache it.  The outcome
> looks like this:
>
> ===8<===
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 020fd16ab8..7429fe87a8 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -249,9 +249,9 @@ unsigned int kvm_get_free_memslots(void)
>  /* Called with KVMMemoryListener.slots_lock held */
>  static KVMSlot *kvm_get_free_slot(KVMMemoryListener *kml)
>  {
> +unsigned int n;
>  int i;
>
> -retry:
>  for (i = 0; i < kml->nr_slots_allocated; i++) {
>  if (kml->slots[i].memory_size == 0) {
>  return &kml->slots[i];
> @@ -259,8 +259,13 @@ retry:
>  }
>
>  /* If no free slots, try to grow first by doubling */
> +n = kml->nr_slots_allocated;
>  if (kvm_slots_double(kml)) {
> -goto retry;
> +/*
> + * If succeed, we must have n used slots, then followed by n free
> + * slots.
> + */
> +return &kml->slots[n];
>  }
>
>  return NULL;
> ===8<===
>
> It's still good to get rid of "goto", and faster indeed.  Though I wished
> we don't need those assumptions, as cons.
>
> One thing to mention that I expect this is extremely slow path, where I
> don't expect to even be reached in major uses of QEMU, and when reached
> should be only once or limited few times per VM life cycle.  The re-walks
> here shouldn't be a perf concern IMHO, because when it's a concern we'll
> hit it much more frequently elsewhere... many other hotter paths around.
>
> So far it looks slightly more readable to me to keep the old way, but I'm
> ok either way.  What do you think?

I agree that it requires this assumption of not moving slots around,
but I think it's intuitive to assume it when it comes to
doubling/increasing the size of an array, realloc() and g_renew() also
don't shuffle existing elements.

In addition, there already is such an assumption. If slots were moved
around, pointers returned by `return &kml->slots[i];` wouldn't point
to the same slot structure after doubling.

However, I realized there's also another problem with this return
statement. g_renew() could have moved the whole array to a new
address, making all the previously returned pointers invalid. This
could be solved by either adding another layer of indirection, so the
function returns a pointer to a single slot structure that never moves
and the array contains pointers to these structures, or the slots need
to be always accessed through an up-to-date pointer to the array,
probably from another structure or through a getter function. With the
first approach, pointers in the array could shuffle, but with the
second one, the index of a slot must not change during the lifetime of
the slot, keeping the assumption correct.

>
> Thanks,
>
> --
> Peter Xu
>


-- 

Juraj Marcin

[PULL 2/5] hw/ufs: minor bug fixes related to ufs-test

[Jeuk Kim]

From: Yoochan Jeong 

Minor bugs and errors related to ufs-test are resolved. Some
permissions and code implementations that are not synchronized
with the ufs spec are edited.

Signed-off-by: Yoochan Jeong 
Reviewed-by: Jeuk Kim 
Signed-off-by: Jeuk Kim 
---
 hw/ufs/ufs.c   | 19 +--
 include/block/ufs.h|  6 ++
 tests/qtest/ufs-test.c | 11 ---
 3 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index ce2c96aeea..79f786ed4e 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -,10 +,13 @@ static uint32_t ufs_read_attr_value(UfsHc *u, uint8_t 
idn)
 return 0;
 }
 
-static void ufs_write_attr_value(UfsHc *u, uint8_t idn, uint32_t value)
+static QueryRespCode ufs_write_attr_value(UfsHc *u, uint8_t idn, uint32_t 
value)
 {
 switch (idn) {
 case UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL:
+if (value > UFS_QUERY_ATTR_ACTIVE_ICC_MAXVALUE) {
+return UFS_QUERY_RESULT_INVALID_VALUE;
+}
 u->attributes.active_icc_level = value;
 break;
 case UFS_QUERY_ATTR_IDN_MAX_DATA_IN:
@@ -1142,6 +1145,7 @@ static void ufs_write_attr_value(UfsHc *u, uint8_t idn, 
uint32_t value)
 u->attributes.psa_data_size = cpu_to_be32(value);
 break;
 }
+return UFS_QUERY_RESULT_SUCCESS;
 }
 
 static QueryRespCode ufs_exec_query_attr(UfsRequest *req, int op)
@@ -1158,13 +1162,13 @@ static QueryRespCode ufs_exec_query_attr(UfsRequest 
*req, int op)
 
 if (op == UFS_QUERY_ATTR_READ) {
 value = ufs_read_attr_value(u, idn);
+ret = UFS_QUERY_RESULT_SUCCESS;
 } else {
-value = be32_to_cpu(req->req_upiu.qr.value);
-ufs_write_attr_value(u, idn, value);
+value = req->req_upiu.qr.value;
+ret = ufs_write_attr_value(u, idn, value);
 }
-
 req->rsp_upiu.qr.value = cpu_to_be32(value);
-return UFS_QUERY_RESULT_SUCCESS;
+return ret;
 }
 
 static const RpmbUnitDescriptor rpmb_unit_desc = {
@@ -1287,9 +1291,12 @@ static QueryRespCode ufs_read_desc(UfsRequest *req)
 UfsHc *u = req->hc;
 QueryRespCode status;
 uint8_t idn = req->req_upiu.qr.idn;
+uint8_t selector = req->req_upiu.qr.selector;
 uint16_t length = be16_to_cpu(req->req_upiu.qr.length);
 InterconnectDescriptor desc;
-
+if (selector != 0) {
+return UFS_QUERY_RESULT_INVALID_SELECTOR;
+}
 switch (idn) {
 case UFS_QUERY_DESC_IDN_DEVICE:
 memcpy(&req->rsp_upiu.qr.data, &u->device_desc, 
sizeof(u->device_desc));
diff --git a/include/block/ufs.h b/include/block/ufs.h
index 92da7a89b9..57f5ea3500 100644
--- a/include/block/ufs.h
+++ b/include/block/ufs.h
@@ -763,6 +763,12 @@ typedef struct QEMU_PACKED UtpTaskReqDesc {
  */
 #define UFS_WB_EXCEED_LIFETIME 0x0B
 
+/*
+ * The range of valid value of Active ICC attritbute
+ * is from 0x00 to 0x0F.
+ */
+#define UFS_QUERY_ATTR_ACTIVE_ICC_MAXVALUE 0x0F
+
 /*
  * In UFS Spec, the Extra Header Segment (EHS) starts from byte 32 in UPIU
  * request/response packet
diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 82ec3f0671..9cc9e578ff 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -119,6 +119,7 @@ static void ufs_send_nop_out(QUfs *ufs, uint8_t slot,
 
 static void ufs_send_query(QUfs *ufs, uint8_t slot, uint8_t query_function,
uint8_t query_opcode, uint8_t idn, uint8_t index,
+   uint8_t selector, uint32_t attr_value,
UtpTransferReqDesc *utrd_out, UtpUpiuRsp *rsp_out)
 {
 /* Build up utp transfer request descriptor */
@@ -136,13 +137,16 @@ static void ufs_send_query(QUfs *ufs, uint8_t slot, 
uint8_t query_function,
 req_upiu.header.query_func = query_function;
 req_upiu.header.task_tag = slot;
 /*
- * QEMU UFS does not currently support Write descriptor and Write 
attribute,
+ * QEMU UFS does not currently support Write descriptor,
  * so the value of data_segment_length is always 0.
  */
 req_upiu.header.data_segment_length = 0;
 req_upiu.qr.opcode = query_opcode;
 req_upiu.qr.idn = idn;
 req_upiu.qr.index = index;
+req_upiu.qr.selector = selector;
+req_upiu.qr.value = attr_value;
+req_upiu.qr.length = UFS_QUERY_DESC_MAX_SIZE;
 qtest_memwrite(ufs->dev.bus->qts, req_upiu_addr, &req_upiu,
sizeof(req_upiu));
 
@@ -344,7 +348,7 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc)
 /* Set fDeviceInit flag via query request */
 ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
UFS_UPIU_QUERY_OPCODE_SET_FLAG,
-   UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, &utrd, &rsp_upiu);
+   UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, &rsp_upiu);
 g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
 
 /* Wait for device to reset */
@@ -353,7 +357,8 @@ static void ufs_init(QUfs *ufs, QGuestAllocator *alloc)

[PULL 5/5] hw/ufs: ufs descriptor read test implemented

[Jeuk Kim]

From: Yoochan Jeong 

New test function "ufstest_query_desc_request" added, which can check one's
virtual UFS device can properly read and its descriptor data.
(Writing descriptors are not implemented yet.)
The testcases attempt to read all kinds of descriptors at least once,
except for configuration descriptors (which are not implemented yet.)
There are some testcases that are intended to make an error caused by
an invalid index value or an invalid selector value.

Signed-off-by: Yoochan Jeong 
Reviewed-by: Jeuk Kim 
Signed-off-by: Jeuk Kim 
---
 tests/qtest/ufs-test.c | 153 +
 1 file changed, 153 insertions(+)

diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 061371414a..60199abbee 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -755,6 +755,157 @@ static void ufstest_query_attr_request(void *obj, void 
*data,
 ufs_exit(ufs, alloc);
 }
 
+static void ufstest_query_desc_request(void *obj, void *data,
+   QGuestAllocator *alloc)
+{
+QUfs *ufs = obj;
+
+UtpTransferReqDesc utrd;
+UtpUpiuRsp rsp_upiu;
+ufs_init(ufs, alloc);
+
+/* Write Descriptor is not supported yet */
+
+/* Read Device Descriptor */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_DEVICE,
+   0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_DESC);
+g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_DESC_IDN_DEVICE);
+g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(DeviceDescriptor));
+g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_DEVICE);
+
+/* Read Configuration Descriptor is not supported yet*/
+
+/* Read Unit Descriptor */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 0,
+   0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(UnitDescriptor));
+g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT);
+g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 0);
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT, 1,
+   0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(UnitDescriptor));
+g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT);
+g_assert_cmpuint(rsp_upiu.qr.data[2], ==, 1);
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_UNIT,
+   UFS_UPIU_RPMB_WLUN, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(RpmbUnitDescriptor));
+g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_UNIT);
+g_assert_cmpuint(rsp_upiu.qr.data[2], ==, UFS_UPIU_RPMB_WLUN);
+
+/* Read Interconnect Descriptor */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC,
+   UFS_QUERY_DESC_IDN_INTERCONNECT, 0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.data[0], ==, sizeof(InterconnectDescriptor));
+g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_INTERCONNECT);
+
+/* Read String Descriptor */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING,
+   0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.data[0], ==, 0x12);
+g_assert_cmpuint(rsp_upiu.qr.data[1], ==, UFS_QUERY_DESC_IDN_STRING);
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_DESC, UFS_QUERY_DESC_IDN_STRING,
+   1, 0, 0, &utrd, &rsp_upiu)

[PULL 0/5] ufs queue

[Jeuk Kim]

From: Jeuk Kim 

The following changes since commit 7b87a25f49a301d3377f3e71e0b4a62540c6f6e4:

  Merge tag 'edgar/xen-queue-2024-09-04.for-upstream' of 
https://gitlab.com/edgar.iglesias/qemu into staging (2024-09-05 13:02:26 +0100)

are available in the Git repository at:

  https://gitlab.com/jeuk20.kim/qemu.git tags/pull-ufs-20240906

for you to fetch changes up to 9fe8e2c68ad99e503a11390b868a7dad846e7a0d:

  hw/ufs: ufs descriptor read test implemented (2024-09-06 18:04:16 +0900)


ufs queue

- Add basic info of query response upiu
- Add more qtests for the ufs query request


Kyoungrul Kim (1):
  hw/ufs: add basic info of query response upiu

Yoochan Jeong (4):
  hw/ufs: minor bug fixes related to ufs-test
  hw/ufs: ufs flag read/write test implemented
  hw/ufs: ufs attribute read/write test implemented
  hw/ufs: ufs descriptor read test implemented

 hw/ufs/ufs.c   |  32 +++--
 hw/ufs/ufs.h   |   1 +
 include/block/ufs.h|   6 +
 tests/qtest/ufs-test.c | 384 -
 4 files changed, 410 insertions(+), 13 deletions(-)

[PULL 3/5] hw/ufs: ufs flag read/write test implemented

[Jeuk Kim]

From: Yoochan Jeong 

New test function "ufstest_flag_request" added, which can check one's
virtual UFS device can properly read and write its flag data. It tests
if reading, setting, clearing and toggling flags work properly. There
are some testcases that are intended to make an error caused by
permission issues.

Signed-off-by: Yoochan Jeong 
Reviewed-by: Jeuk Kim 
Signed-off-by: Jeuk Kim 
---
 tests/qtest/ufs-test.c | 83 ++
 1 file changed, 83 insertions(+)

diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 9cc9e578ff..9f45a078e7 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -539,6 +539,87 @@ static void ufstest_read_write(void *obj, void *data, 
QGuestAllocator *alloc)
 ufs_exit(ufs, alloc);
 }
 
+static void ufstest_query_flag_request(void *obj, void *data,
+   QGuestAllocator *alloc)
+{
+QUfs *ufs = obj;
+
+UtpTransferReqDesc utrd;
+UtpUpiuRsp rsp_upiu;
+ufs_init(ufs, alloc);
+
+/* Read read-only flag */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_FLAG,
+   UFS_QUERY_FLAG_IDN_FDEVICEINIT, 0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_FLAG);
+g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_FLAG_IDN_FDEVICEINIT);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, be32_to_cpu(0));
+
+/* Flag Set, Clear, Toggle Test with fDeviceLifeSpanModeEn */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_FLAG,
+   UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, be32_to_cpu(0));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_SET_FLAG,
+   UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, be32_to_cpu(1));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_CLEAR_FLAG,
+   UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, be32_to_cpu(0));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_TOGGLE_FLAG,
+   UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, be32_to_cpu(1));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_TOGGLE_FLAG,
+   UFS_QUERY_FLAG_IDN_LIFE_SPAN_MODE_ENABLE, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, be32_to_cpu(0));
+
+/* Read Write-only Flag (Intended Failure) */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_FLAG,
+   UFS_QUERY_FLAG_IDN_PURGE_ENABLE, 0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==,
+ UFS_OCS_INVALID_CMD_TABLE_ATTR);
+g_assert_cmpuint(rsp_upiu.header.response, ==,
+ UFS_QUERY_RESULT_NOT_READABLE);
+
+/* Write Read-Only Flag (Intended Failure) */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_SET_FLAG, UFS_QUERY_FLAG_IDN_BUSY_RTC,
+   0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==,
+ UFS_OCS_INVALID_CMD_TABLE_ATTR);
+g_assert_cmpuint(rsp_upiu.header.response, ==,
+ UFS_QUERY_RESULT_NOT_WRITEABLE);
+
+ufs_exit(ufs, alloc);
+}
+
 stati

[PULL 4/5] hw/ufs: ufs attribute read/write test implemented

[Jeuk Kim]

From: Yoochan Jeong 

New test function "ufstest_query_attr_request" added, which can check one's
virtual UFS device can properly read and write its attribute data.
It tests if reading and writing attributes work properly. There are
some testcases that are intended to make an error caused by writing an
invalid value, allocating an invalid selector and permission issues.

Signed-off-by: Yoochan Jeong 
Reviewed-by: Jeuk Kim 
Signed-off-by: Jeuk Kim 
---
 tests/qtest/ufs-test.c | 137 +
 1 file changed, 137 insertions(+)

diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index 9f45a078e7..061371414a 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -620,6 +620,141 @@ static void ufstest_query_flag_request(void *obj, void 
*data,
 ufs_exit(ufs, alloc);
 }
 
+static void ufstest_query_attr_request(void *obj, void *data,
+   QGuestAllocator *alloc)
+{
+QUfs *ufs = obj;
+
+UtpTransferReqDesc utrd;
+UtpUpiuRsp rsp_upiu;
+ufs_init(ufs, alloc);
+
+/* Read Readable Attributes*/
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_ATTR,
+   UFS_QUERY_ATTR_IDN_BOOT_LU_EN, 0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.opcode, ==, UFS_UPIU_QUERY_OPCODE_READ_ATTR);
+g_assert_cmpuint(rsp_upiu.qr.idn, ==, UFS_QUERY_ATTR_IDN_BOOT_LU_EN);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_ATTR,
+   UFS_QUERY_ATTR_IDN_BKOPS_STATUS, 0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x00));
+
+/* Write Writable Attributes & Read Again */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_WRITE_ATTR,
+   UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0x03, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x03));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_WRITE_ATTR,
+   UFS_QUERY_ATTR_IDN_EE_CONTROL, 0, 0, 0x07, &utrd, 
&rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x07));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_ATTR,
+   UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x03));
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_ATTR,
+   UFS_QUERY_ATTR_IDN_EE_CONTROL, 0, 0, 0, &utrd, &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x07));
+
+/* Write Invalid Value (Intended Error) */
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_WRITE_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_WRITE_ATTR,
+   UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0x10, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==,
+ UFS_OCS_INVALID_CMD_TABLE_ATTR);
+g_assert_cmpuint(rsp_upiu.header.response, ==,
+ UFS_QUERY_RESULT_INVALID_VALUE);
+
+ufs_send_query(ufs, 0, UFS_UPIU_QUERY_FUNC_STANDARD_READ_REQUEST,
+   UFS_UPIU_QUERY_OPCODE_READ_ATTR,
+   UFS_QUERY_ATTR_IDN_ACTIVE_ICC_LVL, 0, 0, 0, &utrd,
+   &rsp_upiu);
+g_assert_cmpuint(le32_to_cpu(utrd.header.dword_2), ==, UFS_OCS_SUCCESS);
+g_assert_cmpuint(rsp_upiu.header.response, ==, UFS_COMMAND_RESULT_SUCCESS);
+g_assert_cmpuint(rsp_upiu.qr.value, ==, cpu_to_be32(0x03));
+
+/* Read Write-Only Attribute (Intended Error) */
+ufs_send_query(ufs, 0, UFS_UPIU_Q

[PULL 1/5] hw/ufs: add basic info of query response upiu

[Jeuk Kim]

From: Kyoungrul Kim 

Modify to fill the opcode, idn, index, selector information of
all Query Response UPIU. because attr and flag operation of query
response upiu need these information too.

Signed-off-by: KyoungrulKim 
Reviewed-by: Minwoo Im 
Reviewed-by: Jeuk Kim 
Signed-off-by: Jeuk Kim 
---
 hw/ufs/ufs.c | 13 +
 hw/ufs/ufs.h |  1 +
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/ufs/ufs.c b/hw/ufs/ufs.c
index 945a0ea127..ce2c96aeea 100644
--- a/hw/ufs/ufs.c
+++ b/hw/ufs/ufs.c
@@ -851,6 +851,14 @@ void ufs_build_upiu_header(UfsRequest *req, uint8_t 
trans_type, uint8_t flags,
 req->rsp_upiu.header.data_segment_length = 
cpu_to_be16(data_segment_length);
 }
 
+void ufs_build_query_response(UfsRequest *req)
+{
+req->rsp_upiu.qr.opcode = req->req_upiu.qr.opcode;
+req->rsp_upiu.qr.idn = req->req_upiu.qr.idn;
+req->rsp_upiu.qr.index = req->req_upiu.qr.index;
+req->rsp_upiu.qr.selector = req->req_upiu.qr.selector;
+}
+
 static UfsReqResult ufs_exec_scsi_cmd(UfsRequest *req)
 {
 UfsHc *u = req->hc;
@@ -1327,10 +1335,6 @@ static QueryRespCode ufs_read_desc(UfsRequest *req)
 if (length > req->rsp_upiu.qr.data[0]) {
 length = req->rsp_upiu.qr.data[0];
 }
-req->rsp_upiu.qr.opcode = req->req_upiu.qr.opcode;
-req->rsp_upiu.qr.idn = req->req_upiu.qr.idn;
-req->rsp_upiu.qr.index = req->req_upiu.qr.index;
-req->rsp_upiu.qr.selector = req->req_upiu.qr.selector;
 req->rsp_upiu.qr.length = cpu_to_be16(length);
 
 return status;
@@ -1411,6 +1415,7 @@ static UfsReqResult ufs_exec_query_cmd(UfsRequest *req)
 data_segment_length = be16_to_cpu(req->rsp_upiu.qr.length);
 ufs_build_upiu_header(req, UFS_UPIU_TRANSACTION_QUERY_RSP, 0, status, 0,
   data_segment_length);
+ufs_build_query_response(req);
 
 if (status != UFS_QUERY_RESULT_SUCCESS) {
 return UFS_REQUEST_FAIL;
diff --git a/hw/ufs/ufs.h b/hw/ufs/ufs.h
index 6c9382cbc4..4bcc41f53a 100644
--- a/hw/ufs/ufs.h
+++ b/hw/ufs/ufs.h
@@ -228,6 +228,7 @@ static inline bool is_wlun(uint8_t lun)
 void ufs_build_upiu_header(UfsRequest *req, uint8_t trans_type, uint8_t flags,
uint8_t response, uint8_t scsi_status,
uint16_t data_segment_length);
+void ufs_build_query_response(UfsRequest *req);
 void ufs_complete_req(UfsRequest *req, UfsReqResult req_result);
 void ufs_init_wlu(UfsLu *wlu, uint8_t wlun);
 #endif /* HW_UFS_UFS_H */
-- 
2.34.1

[Stable-9.0.3 10/69] hw/cxl/cxl-host: Fix segmentation fault when getting cxl-fmw property

[Michael Tokarev]

From: Zhao Liu 

QEMU crashes (Segmentation fault) when getting cxl-fmw property via
qmp:

(QEMU) qom-get path=machine property=cxl-fmw

This issue is caused by accessing wrong callback (opaque) type in
machine_get_cfmw().

cxl_machine_init() sets the callback as `CXLState *` type but
machine_get_cfmw() treats the callback as
`CXLFixedMemoryWindowOptionsList **`.

Fix this error by casting opaque to `CXLState *` type in
machine_get_cfmw().

Fixes: 03b39fcf64bc ("hw/cxl: Make the CXL fixed memory window setup a machine 
parameter.")
Signed-off-by: Zhao Liu 
Reviewed-by: Li Zhijian 
Reviewed-by: Xingtao Yao 
Link: 
https://lore.kernel.org/r/20240704093404.1848132-1-zhao1@linux.intel.com
Signed-off-by: Jonathan Cameron 
Message-Id: <20240705113956.941732-2-jonathan.came...@huawei.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit a207d5f87d66f7933b50677e047498fc4af63e1f)
Signed-off-by: Michael Tokarev 

diff --git a/hw/cxl/cxl-host.c b/hw/cxl/cxl-host.c
index c5f5fcfd64..e9f2543c43 100644
--- a/hw/cxl/cxl-host.c
+++ b/hw/cxl/cxl-host.c
@@ -315,7 +315,8 @@ static void machine_set_cxl(Object *obj, Visitor *v, const 
char *name,
 static void machine_get_cfmw(Object *obj, Visitor *v, const char *name,
  void *opaque, Error **errp)
 {
-CXLFixedMemoryWindowOptionsList **list = opaque;
+CXLState *state = opaque;
+CXLFixedMemoryWindowOptionsList **list = &state->cfmw_list;
 
 visit_type_CXLFixedMemoryWindowOptionsList(v, name, list, errp);
 }
-- 
2.39.2

[Stable-9.0.3 04/69] target/arm: Fix handling of LDAPR/STLR with negative offset

[Michael Tokarev]

From: Peter Maydell 

When we converted the LDAPR/STLR instructions to decodetree we
accidentally introduced a regression where the offset is negative.
The 9-bit immediate field is signed, and the old hand decoder
correctly used sextract32() to get it out of the insn word,
but the ldapr_stlr_i pattern in the decode file used "imm:9"
instead of "imm:s9", so it treated the field as unsigned.

Fix the pattern to treat the field as a signed immediate.

Cc: qemu-sta...@nongnu.org
Fixes: 2521b6073b7 ("target/arm: Convert LDAPR/STLR (imm) to decodetree")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2419
Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-id: 20240709134504.357-2-peter.mayd...@linaro.org
(cherry picked from commit 5669d26ec614b3f4c56cf1489b9095ed327938b1)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/a64.decode b/target/arm/tcg/a64.decode
index 8a20dce3c8..6cc29a4bce 100644
--- a/target/arm/tcg/a64.decode
+++ b/target/arm/tcg/a64.decode
@@ -467,7 +467,7 @@ LDAPR   sz:2 111 0 00 1 0 1 1 1100 00 rn:5 rt:5
 LDRA11 111 0 00 m:1 . 1 . w:1 1 rn:5 rt:5 imm=%ldra_imm
 
 &ldapr_stlr_i   rn rt imm sz sign ext
-@ldapr_stlr_i   .. .. .. . imm:9 .. rn:5 rt:5 &ldapr_stlr_i
+@ldapr_stlr_i   .. .. .. . imm:s9 .. rn:5 rt:5 &ldapr_stlr_i
 STLR_i  sz:2 011001 00 0 . 00 . . @ldapr_stlr_i sign=0 
ext=0
 LDAPR_i sz:2 011001 01 0 . 00 . . @ldapr_stlr_i sign=0 
ext=0
 LDAPR_i 00 011001 10 0 . 00 . . @ldapr_stlr_i sign=1 
ext=0 sz=0
-- 
2.39.2

[Stable-9.0.3 16/69] hw/intc/loongson_ipi: Access memory in little endian

[Michael Tokarev]

From: Bibo Mao 

Loongson IPI is only available in little-endian,
so use that to access the guest memory (in case
we run on a big-endian host).

Cc: qemu-sta...@nongnu.org
Signed-off-by: Bibo Mao 
Fixes: f6783e3438 ("hw/loongarch: Add LoongArch ipi interrupt support")
[PMD: Extracted from bigger commit, added commit description]
Co-Developed-by: Philippe Mathieu-Daudé 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Bibo Mao 
Tested-by: Bibo Mao 
Acked-by: Song Gao 
Reviewed-by: Richard Henderson 
Reviewed-by: Jiaxun Yang 
Tested-by: Jiaxun Yang 
Message-Id: <20240718133312.10324-3-phi...@linaro.org>
(cherry picked from commit 2465c89fb983eed670007742bd68c7d91b6d6f85)
Signed-off-by: Michael Tokarev 
(Mjt: fixups for 9.0, for lack of:
 v9.0.0-583-g91d0b151de4c "hw/intc/loongson_ipi: Implement IOCSR address space 
for MIPS"
 v9.0.0-582-gb4a12dfc2132 "hw/intc/loongarch_ipi: Rename as loongson_ipi")

diff --git a/hw/intc/loongarch_ipi.c b/hw/intc/loongarch_ipi.c
index a184112b09..521731342c 100644
--- a/hw/intc/loongarch_ipi.c
+++ b/hw/intc/loongarch_ipi.c
@@ -13,6 +13,7 @@
 #include "qapi/error.h"
 #include "qemu/log.h"
 #include "exec/address-spaces.h"
+#include "exec/memory.h"
 #include "hw/loongarch/virt.h"
 #include "migration/vmstate.h"
 #include "target/loongarch/internals.h"
@@ -66,8 +67,8 @@ static void send_ipi_data(CPULoongArchState *env, uint64_t 
val, hwaddr addr,
  * if the mask is 0, we need not to do anything.
  */
 if ((val >> 27) & 0xf) {
-data = address_space_ldl(env->address_space_iocsr, addr,
- attrs, NULL);
+data = address_space_ldl_le(env->address_space_iocsr, addr,
+attrs, NULL);
 for (i = 0; i < 4; i++) {
 /* get mask for byte writing */
 if (val & (0x1 << (27 + i))) {
@@ -78,8 +79,8 @@ static void send_ipi_data(CPULoongArchState *env, uint64_t 
val, hwaddr addr,
 
 data &= mask;
 data |= (val >> 32) & ~mask;
-address_space_stl(env->address_space_iocsr, addr,
-  data, attrs, NULL);
+address_space_stl_le(env->address_space_iocsr, addr,
+ data, attrs, NULL);
 }
 
 static int archid_cmp(const void *a, const void *b)
-- 
2.39.2

[Stable-9.0.3 00/69] Patch Round-up for stable 9.0.3, freeze on 2024-09-16

[Michael Tokarev]

The following patches are queued for QEMU stable v9.0.3:

  https://gitlab.com/qemu-project/qemu/-/commits/staging-9.0

Patch freeze is 2024-09-16, and the release is planned for 2024-09-18:

  https://wiki.qemu.org/Planning/9.0

Please respond here or CC qemu-sta...@nongnu.org on any additional patches
you think should (or shouldn't) be included in the release.

The changes which are staging for inclusion, with the original commit hash
from master branch, are given below the bottom line.

Thanks!

/mjt

--
01 a4975023fb13 Fiona Ebner:
   hw/scsi/lsi53c895a: bump instruction limit in scripts processing to fix 
   regression
02 57a8a80d1a5b Fiona Ebner:
   scsi: fix regression and honor bootindex again for legacy drives
03 3becc9390810 Markus Armbruster:
   qapi/qom: Document feature unstable of @x-vfio-user-server
04 5669d26ec614 Peter Maydell:
   target/arm: Fix handling of LDAPR/STLR with negative offset
05 25489b521b61 Peter Maydell:
   target/arm: LDAPR should honour SCTLR_ELx.nAA
06 31d93fedf41c Daniyal Khan:
   target/arm: Use float_status copy in sme_fmopa_s
07 207d30b5fdb5 Richard Henderson:
   target/arm: Use FPST_F16 for SME FMOPA (widening)
08 30a1690f2402 Akihiko Odaki:
   hvf: arm: Do not advance PC when raising an exception
09 c510fe78f1b7 Zheyu Ma:
   hw/nvme: fix memory leak in nvme_dsm
10 a207d5f87d66 Zhao Liu:
   hw/cxl/cxl-host: Fix segmentation fault when getting cxl-fmw property
11 98e77e3dd8dd Manos Pitsidianakis:
   virtio-snd: add max size bounds check in input cb
12 9b6083465fb8 Manos Pitsidianakis:
   virtio-snd: check for invalid param shift operands
13 a3c8d7e38550 Clément Mathieu--Drif:
   intel_iommu: fix FRCD construction macro
14 13be929aff80 Paolo Bonzini:
   target/i386: do not crash if microvm guest uses SGX CPUID leaves
15 903cc9e1173e songziming:
   chardev/char-win-stdio.c: restore old console mode
16 2465c89fb983 Bibo Mao:
   hw/intc/loongson_ipi: Access memory in little endian
17 0c2086bc7360 Philippe Mathieu-Daudé:
   hw/intc/loongson_ipi: Fix resource leak
18 a18ffbcf8b9f Song Gao:
   target/loongarch: Fix helper_lddir() a CID INTEGER_OVERFLOW issue
19 851495571d14 Peter Maydell:
   util/async.c: Forbid negative min/max in 
   aio_context_set_thread_pool_params()
20 e0bf95443ee9 Sergey Dyasli:
   Revert "qemu-char: do not operate on sources from finalize callbacks"
21 d72479b11797 Thomas Huth:
   hw/virtio: Fix the de-initialization of vhost-user devices
22 83340193b991 Richard Henderson:
   target/rx: Use target_ulong for address in LI
23 546d574b11e0 Frederik van Hövell:
   hw/char/bcm2835_aux: Fix assert when receive FIFO fills up
24 0892fffc2aba Peter Maydell:
   hw/misc/bcm2835_property: Fix handling of FRAMEBUFFER_SET_PALETTE
25 56f1c0db928a Peter Maydell:
   target/arm: Don't assert for 128-bit tile accesses when SVL is 128
26 ea3f5a90f036 Peter Maydell:
   target/arm: Fix UMOPA/UMOPS of 16-bit values
27 76916dfa89e8 Peter Maydell:
   target/arm: Avoid shifts by -1 in tszimm_shr() and tszimm_shl()
28 f573ac059ed0 Peter Maydell:
   target/arm: Ignore SMCR_EL2.LEN and SVCR_EL2.LEN if EL2 is not enabled
29 48e5b5f994bc Peter Maydell:
   docs/sphinx/depfile.py: Handle env.doc2path() returning a Path not a str
30 9a45b0761628 Peter Maydell:
   hw/i386/amd_iommu: Don't leak memory in amdvi_update_iotlb()
31 5a558be93ad6 Marco Palumbi:
   hw/arm/mps2-tz.c: fix RX/TX interrupts order
32 55f9f4ee018c Peter Maydell:
   target/arm: Handle denormals correctly for FMOPA (widening)
33 f1595ceb9aad Akihiko Odaki:
   virtio-net: Ensure queue index fits with RSS
34 f937309fbdbb thomas:
   virtio-net: Fix network stall at the host side waiting for kick
35 64f75f57f9d2 David Woodhouse:
   net: Reinstate '-net nic, model=help' output as documented in man page
36 0bd5b9284fa9 Fabiano Rosas:
   migration/multifd: Fix multifd_send_setup cleanup when channel creation 
   fails
37 5b0c2742c839 Ilya Leoshkevich:
   linux-user/elfload: Fix pr_pid values in core files
38 ac63755b2001 Richard Henderson:
   target/i386: Fix VSIB decode
39 682a05280504 Richard Henderson:
   tcg/ppc: Sync tcg_out_test and constraints
40 ed5a159c3de4 Philippe Mathieu-Daudé:
   hw/sd/sdhci: Reset @data_count index on invalid ADMA transfers
41 b881cf00c99e Amjad Alsharafi:
   vvfat: Fix bug in writing to middle of file
42 21b25a0e466a Amjad Alsharafi:
   vvfat: Fix usage of `info.file.offset`
43 f60a6f7e17bf Amjad Alsharafi:
   vvfat: Fix wrong checks for cluster mappings invariant
44 5eed3db33650 Amjad Alsharafi:
   vvfat: Fix reading files with non-continuous clusters
45 c8f60bfb4345 Amjad Alsharafi:
   iotests: Add `vvfat` tests
46 fb1c2aaa981e Eric Blake:
   nbd/server: Plumb in new args to nbd_client_add()
47 c8a76dbd90c2 Eric Blake:
   nbd/server: CVE-2024-7409: Cap default max-connections to 100
48 b9b72cb3ce15 Eric Blake:
   nbd/server: CVE-2024-7409: Drop non-negotiating clients
49 3e7ef738c846 Eric Blake:
   nbd/server: CVE-2024-7409: Close stray clients at server

[Stable-9.0.3 11/69] virtio-snd: add max size bounds check in input cb

[Michael Tokarev]

From: Manos Pitsidianakis 

When reading input audio in the virtio-snd input callback,
virtio_snd_pcm_in_cb(), we do not check whether the iov can actually fit
the data buffer. This is because we use the buffer->size field as a
total-so-far accumulator instead of byte-size-left like in TX buffers.

This triggers an out of bounds write if the size of the virtio queue
element is equal to virtio_snd_pcm_status, which makes the available
space for audio data zero. This commit adds a check for reaching the
maximum buffer size before attempting any writes.

Reported-by: Zheyu Ma 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2427
Signed-off-by: Manos Pitsidianakis 
Message-Id: 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 98e77e3dd8dd6e7aa9a7dffa60f49c8c8a49d4e3)
Signed-off-by: Michael Tokarev 

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 4a56c00ec9..541f0797ac 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -1261,7 +1261,7 @@ static void virtio_snd_pcm_in_cb(void *data, int 
available)
 {
 VirtIOSoundPCMStream *stream = data;
 VirtIOSoundPCMBuffer *buffer;
-size_t size;
+size_t size, max_size;
 
 WITH_QEMU_LOCK_GUARD(&stream->queue_mutex) {
 while (!QSIMPLEQ_EMPTY(&stream->queue)) {
@@ -1275,7 +1275,12 @@ static void virtio_snd_pcm_in_cb(void *data, int 
available)
 continue;
 }
 
+max_size = iov_size(buffer->elem->in_sg, buffer->elem->in_num);
 for (;;) {
+if (buffer->size >= max_size) {
+return_rx_buffer(stream, buffer);
+break;
+}
 size = AUD_read(stream->voice.in,
 buffer->data + buffer->size,
 MIN(available, (stream->params.period_bytes -
-- 
2.39.2

[Stable-9.0.3 02/69] scsi: fix regression and honor bootindex again for legacy drives

[Michael Tokarev]

From: Fiona Ebner 

Commit 3089637461 ("scsi: Don't ignore most usb-storage properties")
removed the call to object_property_set_int() and thus the 'set'
method for the bootindex property was also not called anymore. Here
that method is device_set_bootindex() (as configured by
scsi_dev_instance_init() -> device_add_bootindex_property()) which as
a side effect registers the device via add_boot_device_path().

As reported by a downstream user [0], the bootindex property did not
have the desired effect anymore for legacy drives. Fix the regression
by explicitly calling the add_boot_device_path() function after
checking that the bootindex is not yet used (to avoid
add_boot_device_path() calling exit()).

[0]: https://forum.proxmox.com/threads/149772/post-679433

Cc: qemu-sta...@nongnu.org
Fixes: 3089637461 ("scsi: Don't ignore most usb-storage properties")
Suggested-by: Kevin Wolf 
Signed-off-by: Fiona Ebner 
Link: https://lore.kernel.org/r/20240710152529.1737407-1-f.eb...@proxmox.com
Signed-off-by: Paolo Bonzini 
(cherry picked from commit 57a8a80d1a5b28797b21d30bfc60601945820e51)
Signed-off-by: Michael Tokarev 

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 9e40b0c920..53eff5dd3d 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -384,6 +384,7 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
BlockBackend *blk,
 DeviceState *dev;
 SCSIDevice *s;
 DriveInfo *dinfo;
+Error *local_err = NULL;
 
 if (blk_is_sg(blk)) {
 driver = "scsi-generic";
@@ -403,6 +404,14 @@ SCSIDevice *scsi_bus_legacy_add_drive(SCSIBus *bus, 
BlockBackend *blk,
 s = SCSI_DEVICE(dev);
 s->conf = *conf;
 
+check_boot_index(conf->bootindex, &local_err);
+if (local_err) {
+object_unparent(OBJECT(dev));
+error_propagate(errp, local_err);
+return NULL;
+}
+add_boot_device_path(conf->bootindex, dev, NULL);
+
 qdev_prop_set_uint32(dev, "scsi-id", unit);
 if (object_property_find(OBJECT(dev), "removable")) {
 qdev_prop_set_bit(dev, "removable", removable);
-- 
2.39.2

[Stable-9.0.3 03/69] qapi/qom: Document feature unstable of @x-vfio-user-server

[Michael Tokarev]

From: Markus Armbruster 

Commit 8f9a9259d32c added ObjectType member @x-vfio-user-server with
feature unstable, but neglected to explain why it is unstable.  Do
that now.

Fixes: 8f9a9259d32c (vfio-user: define vfio-user-server object)
Cc: Elena Ufimtseva 
Cc: John G Johnson 
Cc: Jagannathan Raman 
Cc: qemu-sta...@nongnu.org
Signed-off-by: Markus Armbruster 
Message-ID: <20240703095310.1242102-1-arm...@redhat.com>
Reviewed-by: John Snow 
[Indentation fixed]
(cherry picked from commit 3becc939081097ccfed6968771f33d65ce8551eb)
Signed-off-by: Michael Tokarev 

diff --git a/qapi/qom.json b/qapi/qom.json
index 85e6b4f84a..09f4e4b22e 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -937,7 +937,8 @@
 #
 # Features:
 #
-# @unstable: Member @x-remote-object is experimental.
+# @unstable: Members @x-remote-object and @x-vfio-user-server are
+# experimental.
 #
 # Since: 6.0
 ##
-- 
2.39.2

[Stable-9.0.3 05/69] target/arm: LDAPR should honour SCTLR_ELx.nAA

[Michael Tokarev]

From: Peter Maydell 

In commit c1a1f80518d360b when we added the FEAT_LSE2 relaxations to
the alignment requirements for atomic and ordered loads and stores,
we didn't quite get it right for LDAPR/LDAPRH/LDAPRB with no
immediate offset.  These instructions were handled in the old decoder
as part of disas_ldst_atomic(), but unlike all the other insns that
function decoded (LDADD, LDCLR, etc) these insns are "ordered", not
"atomic", so they should be using check_ordered_align() rather than
check_atomic_align().  Commit c1a1f80518d360b used
check_atomic_align() regardless for everything in
disas_ldst_atomic().  We then carried that incorrect check over in
the decodetree conversion, where LDAPR/LDAPRH/LDAPRB are now handled
by trans_LDAPR().

The effect is that when FEAT_LSE2 is implemented, these instructions
don't honour the SCTLR_ELx.nAA bit and will generate alignment
faults when they should not.

(The LDAPR insns with an immediate offset were in disas_ldst_ldapr_stlr()
and then in trans_LDAPR_i() and trans_STLR_i(), and have always used
the correct check_ordered_align().)

Use check_ordered_align() in trans_LDAPR().

Cc: qemu-sta...@nongnu.org
Fixes: c1a1f80518d360b ("target/arm: Relax ordered/atomic alignment checks for 
LSE2")
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20240709134504.357-3-peter.mayd...@linaro.org
(cherry picked from commit 25489b521b61b874c4c6583956db0012a3674e3a)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c
index 2666d52711..922a16e5d4 100644
--- a/target/arm/tcg/translate-a64.c
+++ b/target/arm/tcg/translate-a64.c
@@ -3428,7 +3428,7 @@ static bool trans_LDAPR(DisasContext *s, arg_LDAPR *a)
 if (a->rn == 31) {
 gen_check_sp_alignment(s);
 }
-mop = check_atomic_align(s, a->rn, a->sz);
+mop = check_ordered_align(s, a->rn, 0, false, a->sz);
 clean_addr = gen_mte_check1(s, cpu_reg_sp(s, a->rn), false,
 a->rn != 31, mop);
 /*
-- 
2.39.2

[Stable-9.0.3 20/69] Revert "qemu-char: do not operate on sources from finalize callbacks"

[Michael Tokarev]

From: Sergey Dyasli 

This reverts commit 2b316774f60291f57ca9ecb6a9f0712c532cae34.

After 038b4217884c ("Revert "chardev: use a child source for qio input
source"") we've been observing the "iwp->src == NULL" assertion
triggering periodically during the initial capabilities querying by
libvirtd. One of possible backtraces:

Thread 1 (Thread 0x7f16cd4f0700 (LWP 43858)):
0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
1  0x7f16c6c21e65 in __GI_abort () at abort.c:79
2  0x7f16c6c21d39 in __assert_fail_base  at assert.c:92
3  0x7f16c6c46e86 in __GI___assert_fail 
(assertion=assertion@entry=0x562e9bcdaadd "iwp->src == NULL", 
file=file@entry=0x562e9bcdaac8 "../chardev/char-io.c", line=line@entry=99, 
function=function@entry=0x562e9bcdab10 <__PRETTY_FUNCTION__.20549> 
"io_watch_poll_finalize") at assert.c:101
4  0x562e9ba20c2c in io_watch_poll_finalize (source=) at 
../chardev/char-io.c:99
5  io_watch_poll_finalize (source=) at ../chardev/char-io.c:88
6  0x7f16c904aae0 in g_source_unref_internal () from /lib64/libglib-2.0.so.0
7  0x7f16c904baf9 in g_source_destroy_internal () from 
/lib64/libglib-2.0.so.0
8  0x562e9ba20db0 in io_remove_watch_poll (source=0x562e9d6720b0) at 
../chardev/char-io.c:147
9  remove_fd_in_watch (chr=chr@entry=0x562e9d5f3800) at ../chardev/char-io.c:153
10 0x562e9ba23ffb in update_ioc_handlers (s=0x562e9d5f3800) at 
../chardev/char-socket.c:592
11 0x562e9ba2072f in qemu_chr_fe_set_handlers_full at 
../chardev/char-fe.c:279
12 0x562e9ba207a9 in qemu_chr_fe_set_handlers at ../chardev/char-fe.c:304
13 0x562e9ba2ca75 in monitor_qmp_setup_handlers_bh (opaque=0x562e9d4c2c60) 
at ../monitor/qmp.c:509
14 0x562e9bb6222e in aio_bh_poll (ctx=ctx@entry=0x562e9d4c2f20) at 
../util/async.c:216
15 0x562e9bb4de0a in aio_poll (ctx=0x562e9d4c2f20, 
blocking=blocking@entry=true) at ../util/aio-posix.c:722
16 0x562e9b99dfaa in iothread_run (opaque=0x562e9d4c26f0) at 
../iothread.c:63
17 0x562e9bb505a4 in qemu_thread_start (args=0x562e9d4c7ea0) at 
../util/qemu-thread-posix.c:543
18 0x7f16c70081ca in start_thread (arg=) at 
pthread_create.c:479
19 0x7f16c6c398d3 in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:95

io_remove_watch_poll(), which makes sure that iwp->src is NULL, calls
g_source_destroy() which finds that iwp->src is not NULL in the finalize
callback. This can only happen if another thread has managed to trigger
io_watch_poll_prepare() callback in the meantime.

Move iwp->src destruction back to the finalize callback to prevent the
described race, and also remove the stale comment. The deadlock glib bug
was fixed back in 2010 by b35820285668 ("gmain: move finalization of
GSource outside of context lock").

Suggested-by: Paolo Bonzini 
Signed-off-by: Sergey Dyasli 
Link: 
https://lore.kernel.org/r/20240712092659.216206-1-sergey.dya...@nutanix.com
Signed-off-by: Paolo Bonzini 
(cherry picked from commit e0bf95443ee9326d44031373420cf9f3513ee255)
Signed-off-by: Michael Tokarev 

diff --git a/chardev/char-io.c b/chardev/char-io.c
index dab77b112e..3be17b51ca 100644
--- a/chardev/char-io.c
+++ b/chardev/char-io.c
@@ -87,16 +87,12 @@ static gboolean io_watch_poll_dispatch(GSource *source, 
GSourceFunc callback,
 
 static void io_watch_poll_finalize(GSource *source)
 {
-/*
- * Due to a glib bug, removing the last reference to a source
- * inside a finalize callback causes recursive locking (and a
- * deadlock).  This is not a problem inside other callbacks,
- * including dispatch callbacks, so we call io_remove_watch_poll
- * to remove this source.  At this point, iwp->src must
- * be NULL, or we would leak it.
- */
 IOWatchPoll *iwp = io_watch_poll_from_source(source);
-assert(iwp->src == NULL);
+if (iwp->src) {
+g_source_destroy(iwp->src);
+g_source_unref(iwp->src);
+iwp->src = NULL;
+}
 }
 
 static GSourceFuncs io_watch_poll_funcs = {
@@ -139,11 +135,6 @@ static void io_remove_watch_poll(GSource *source)
 IOWatchPoll *iwp;
 
 iwp = io_watch_poll_from_source(source);
-if (iwp->src) {
-g_source_destroy(iwp->src);
-g_source_unref(iwp->src);
-iwp->src = NULL;
-}
 g_source_destroy(&iwp->parent);
 }
 
-- 
2.39.2

[Stable-9.0.3 09/69] hw/nvme: fix memory leak in nvme_dsm

[Michael Tokarev]

From: Zheyu Ma 

The allocated memory to hold LBA ranges leaks in the nvme_dsm function. This
happens because the allocated memory for iocb->range is not freed in all
error handling paths.

Fix this by adding a free to ensure that the allocated memory is properly freed.

ASAN log:
==3075137==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 480 byte(s) in 6 object(s) allocated from:
#0 0x55f1f8a0eddd in malloc 
llvm/compiler-rt/lib/asan/asan_malloc_linux.cpp:129:3
#1 0x7f531e0f6738 in g_malloc 
(/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5e738)
#2 0x55f1faf1f091 in blk_aio_get block/block-backend.c:2583:12
#3 0x55f1f945c74b in nvme_dsm hw/nvme/ctrl.c:2609:30
#4 0x55f1f945831b in nvme_io_cmd hw/nvme/ctrl.c:4470:16
#5 0x55f1f94561b7 in nvme_process_sq hw/nvme/ctrl.c:7039:29

Cc: qemu-sta...@nongnu.org
Fixes: d7d1474fd85d ("hw/nvme: reimplement dsm to allow cancellation")
Signed-off-by: Zheyu Ma 
Reviewed-by: Klaus Jensen 
Signed-off-by: Klaus Jensen 
(cherry picked from commit c510fe78f1b7c966524489d6ba752107423b20c8)
Signed-off-by: Michael Tokarev 

diff --git a/hw/nvme/ctrl.c b/hw/nvme/ctrl.c
index e89f9f7808..652116085e 100644
--- a/hw/nvme/ctrl.c
+++ b/hw/nvme/ctrl.c
@@ -2592,6 +2592,7 @@ next:
 done:
 iocb->aiocb = NULL;
 iocb->common.cb(iocb->common.opaque, iocb->ret);
+g_free(iocb->range);
 qemu_aio_unref(iocb);
 }
 
-- 
2.39.2

[Stable-9.0.3 07/69] target/arm: Use FPST_F16 for SME FMOPA (widening)

[Michael Tokarev]

From: Richard Henderson 

This operation has float16 inputs and thus must use
the FZ16 control not the FZ control.

Cc: qemu-sta...@nongnu.org
Fixes: 3916841ac75 ("target/arm: Implement FMOPA, FMOPS (widening)")
Reported-by: Daniyal Khan 
Signed-off-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Message-id: 20240717060149.204788-3-richard.hender...@linaro.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2374
Signed-off-by: Richard Henderson 
Reviewed-by: Alex Bennée 
Signed-off-by: Peter Maydell 
(cherry picked from commit 207d30b5fdb5b45a36f26eefcf52fe2c1714dd4f)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/translate-sme.c b/target/arm/tcg/translate-sme.c
index 46c7fce8b4..185a8a917b 100644
--- a/target/arm/tcg/translate-sme.c
+++ b/target/arm/tcg/translate-sme.c
@@ -304,6 +304,7 @@ static bool do_outprod(DisasContext *s, arg_op *a, MemOp 
esz,
 }
 
 static bool do_outprod_fpst(DisasContext *s, arg_op *a, MemOp esz,
+ARMFPStatusFlavour e_fpst,
 gen_helper_gvec_5_ptr *fn)
 {
 int svl = streaming_vec_reg_size(s);
@@ -319,15 +320,18 @@ static bool do_outprod_fpst(DisasContext *s, arg_op *a, 
MemOp esz,
 zm = vec_full_reg_ptr(s, a->zm);
 pn = pred_full_reg_ptr(s, a->pn);
 pm = pred_full_reg_ptr(s, a->pm);
-fpst = fpstatus_ptr(FPST_FPCR);
+fpst = fpstatus_ptr(e_fpst);
 
 fn(za, zn, zm, pn, pm, fpst, tcg_constant_i32(desc));
 return true;
 }
 
-TRANS_FEAT(FMOPA_h, aa64_sme, do_outprod_fpst, a, MO_32, 
gen_helper_sme_fmopa_h)
-TRANS_FEAT(FMOPA_s, aa64_sme, do_outprod_fpst, a, MO_32, 
gen_helper_sme_fmopa_s)
-TRANS_FEAT(FMOPA_d, aa64_sme_f64f64, do_outprod_fpst, a, MO_64, 
gen_helper_sme_fmopa_d)
+TRANS_FEAT(FMOPA_h, aa64_sme, do_outprod_fpst, a,
+   MO_32, FPST_FPCR_F16, gen_helper_sme_fmopa_h)
+TRANS_FEAT(FMOPA_s, aa64_sme, do_outprod_fpst, a,
+   MO_32, FPST_FPCR, gen_helper_sme_fmopa_s)
+TRANS_FEAT(FMOPA_d, aa64_sme_f64f64, do_outprod_fpst, a,
+   MO_64, FPST_FPCR, gen_helper_sme_fmopa_d)
 
 /* TODO: FEAT_EBF16 */
 TRANS_FEAT(BFMOPA, aa64_sme, do_outprod, a, MO_32, gen_helper_sme_bfmopa)
-- 
2.39.2

[Stable-9.0.3 06/69] target/arm: Use float_status copy in sme_fmopa_s

[Michael Tokarev]

From: Daniyal Khan 

We made a copy above because the fp exception flags
are not propagated back to the FPST register, but
then failed to use the copy.

Cc: qemu-sta...@nongnu.org
Fixes: 558e956c719 ("target/arm: Implement FMOPA, FMOPS (non-widening)")
Signed-off-by: Daniyal Khan 
Signed-off-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Message-id: 20240717060149.204788-2-richard.hender...@linaro.org
[rth: Split from a larger patch]
Signed-off-by: Richard Henderson 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Alex Bennée 
Signed-off-by: Peter Maydell 
(cherry picked from commit 31d93fedf41c24b0badb38cd9317590d1ef74e37)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/sme_helper.c b/target/arm/tcg/sme_helper.c
index e2e0575039..5a6dd76489 100644
--- a/target/arm/tcg/sme_helper.c
+++ b/target/arm/tcg/sme_helper.c
@@ -916,7 +916,7 @@ void HELPER(sme_fmopa_s)(void *vza, void *vzn, void *vzm, 
void *vpn,
 if (pb & 1) {
 uint32_t *a = vza_row + H1_4(col);
 uint32_t *m = vzm + H1_4(col);
-*a = float32_muladd(n, *m, *a, 0, vst);
+*a = float32_muladd(n, *m, *a, 0, &fpst);
 }
 col += 4;
 pb >>= 4;
-- 
2.39.2

[Stable-9.0.3 35/69] net: Reinstate '-net nic, model=help' output as documented in man page

[Michael Tokarev]

From: David Woodhouse 

While refactoring the NIC initialization code, I broke '-net nic,model=help'
which no longer outputs a list of available NIC models.

Fixes: 2cdeca04adab ("net: report list of available models according to 
platform")
Cc: qemu-sta...@nongnu.org
Signed-off-by: David Woodhouse 
Reviewed-by: Michael Tokarev 
Signed-off-by: Jason Wang 
(cherry picked from commit 64f75f57f9d2c8c12ac6d9355fa5d3a2af5879ca)
Signed-off-by: Michael Tokarev 

diff --git a/net/net.c b/net/net.c
index a2f0c828bb..e6ca2529bb 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1150,6 +1150,21 @@ NICInfo *qemu_find_nic_info(const char *typename, bool 
match_default,
 return NULL;
 }
 
+static bool is_nic_model_help_option(const char *model)
+{
+if (model && is_help_option(model)) {
+/*
+ * Trigger the help output by instantiating the hash table which
+ * will gather tha available models as they get registered.
+ */
+if (!nic_model_help) {
+nic_model_help = g_hash_table_new_full(g_str_hash, g_str_equal,
+   g_free, NULL);
+}
+return true;
+}
+return false;
+}
 
 /* "I have created a device. Please configure it if you can" */
 bool qemu_configure_nic_device(DeviceState *dev, bool match_default,
@@ -1733,6 +1748,12 @@ void net_check_clients(void)
 
 static int net_init_client(void *dummy, QemuOpts *opts, Error **errp)
 {
+const char *model = qemu_opt_get_del(opts, "model");
+
+if (is_nic_model_help_option(model)) {
+return 0;
+}
+
 return net_client_init(opts, false, errp);
 }
 
@@ -1789,9 +1810,7 @@ static int net_param_nic(void *dummy, QemuOpts *opts, 
Error **errp)
 memset(ni, 0, sizeof(*ni));
 ni->model = qemu_opt_get_del(opts, "model");
 
-if (!nic_model_help && !g_strcmp0(ni->model, "help")) {
-nic_model_help = g_hash_table_new_full(g_str_hash, g_str_equal,
-   g_free, NULL);
+if (is_nic_model_help_option(ni->model)) {
 return 0;
 }
 
-- 
2.39.2

[Stable-9.0.3 23/69] hw/char/bcm2835_aux: Fix assert when receive FIFO fills up

[Michael Tokarev]

From: Frederik van Hövell 

When a bare-metal application on the raspi3 board reads the
AUX_MU_STAT_REG MMIO register while the device's buffer is
at full receive FIFO capacity
(i.e. `s->read_count == BCM2835_AUX_RX_FIFO_LEN`) the
assertion `assert(s->read_count < BCM2835_AUX_RX_FIFO_LEN)`
fails.

Reported-by: Cryptjar 
Suggested-by: Cryptjar 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/459
Signed-off-by: Frederik van Hövell 
Reviewed-by: Philippe Mathieu-Daudé 
[PMM: commit message tweaks]
Signed-off-by: Peter Maydell 
(cherry picked from commit 546d574b11e02bfd5b15cdf1564842c14516dfab)
Signed-off-by: Michael Tokarev 

diff --git a/hw/char/bcm2835_aux.c b/hw/char/bcm2835_aux.c
index 83990e20f7..fca2f27a55 100644
--- a/hw/char/bcm2835_aux.c
+++ b/hw/char/bcm2835_aux.c
@@ -138,7 +138,7 @@ static uint64_t bcm2835_aux_read(void *opaque, hwaddr 
offset, unsigned size)
 res = 0x30e; /* space in the output buffer, empty tx fifo, idle tx/rx 
*/
 if (s->read_count > 0) {
 res |= 0x1; /* data in input buffer */
-assert(s->read_count < BCM2835_AUX_RX_FIFO_LEN);
+assert(s->read_count <= BCM2835_AUX_RX_FIFO_LEN);
 res |= ((uint32_t)s->read_count) << 16; /* rx fifo fill level */
 }
 return res;
-- 
2.39.2

[Stable-9.0.3 08/69] hvf: arm: Do not advance PC when raising an exception

[Michael Tokarev]

From: Akihiko Odaki 

hvf did not advance PC when raising an exception for most unhandled
system registers, but it mistakenly advanced PC when raising an
exception for GICv3 registers.

Cc: qemu-sta...@nongnu.org
Fixes: a2260983c655 ("hvf: arm: Add support for GICv3")
Signed-off-by: Akihiko Odaki 
Message-id: 20240716-pmu-v3-4-8c7c1858a...@daynix.com
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
(cherry picked from commit 30a1690f2402e6c1582d5b3ebcf7940bfe2fad4b)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ee657f455b..ddf49087ec 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1277,6 +1277,7 @@ static int hvf_sysreg_read(CPUState *cpu, uint32_t reg, 
uint32_t rt)
 /* Call the TCG sysreg handler. This is only safe for GICv3 regs. */
 if (!hvf_sysreg_read_cp(cpu, reg, &val)) {
 hvf_raise_exception(cpu, EXCP_UDEF, syn_uncategorized());
+return 1;
 }
 break;
 case SYSREG_DBGBVR0_EL1:
-- 
2.39.2

[Stable-9.0.3 01/69] hw/scsi/lsi53c895a: bump instruction limit in scripts processing to fix regression

[Michael Tokarev]

From: Fiona Ebner 

Commit 9876359990 ("hw/scsi/lsi53c895a: add timer to scripts
processing") reduced the maximum allowed instruction count by
a factor of 100 all the way down to 100.

This causes the "Check Point R81.20 Gaia" appliance [0] to fail to
boot after fully finishing the installation via the appliance's web
interface (there is already one reboot before that).

With a limit of 150, the appliance still fails to boot, while with a
limit of 200, it works. Bump to 500 to fix the regression and be on
the safe side.

Originally reported in the Proxmox community forum[1].

[0]: https://support.checkpoint.com/results/download/124397
[1]: https://forum.proxmox.com/threads/149772/post-683459

Cc: qemu-sta...@nongnu.org
Fixes: 9876359990 ("hw/scsi/lsi53c895a: add timer to scripts processing")
Signed-off-by: Fiona Ebner 
Acked-by: Sven Schnelle 
Link: https://lore.kernel.org/r/20240715131403.223239-1-f.eb...@proxmox.com
Signed-off-by: Paolo Bonzini 
(cherry picked from commit a4975023fb13cf229bd59c9ceec1b8cbdc5b9a20)
Signed-off-by: Michael Tokarev 

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index eb9828dd5e..f1935e5328 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -188,7 +188,7 @@ static const char *names[] = {
 #define LSI_TAG_VALID (1 << 16)
 
 /* Maximum instructions to process. */
-#define LSI_MAX_INSN100
+#define LSI_MAX_INSN500
 
 typedef struct lsi_request {
 SCSIRequest *req;
-- 
2.39.2

[Stable-9.0.3 12/69] virtio-snd: check for invalid param shift operands

[Michael Tokarev]

From: Manos Pitsidianakis 

When setting the parameters of a PCM stream, we compute the bit flag
with the format and rate values as shift operand to check if they are
set in supported_formats and supported_rates.

If the guest provides a format/rate value which when shifting 1 results
in a value bigger than the number of bits in
supported_formats/supported_rates, we must report an error.

Previously, this ended up triggering the not reached assertions later
when converting to internal QEMU values.

Reported-by: Zheyu Ma 
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2416
Signed-off-by: Manos Pitsidianakis 
Message-Id: 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 9b6083465fb8311f2410615f8303a41f580a2a20)
Signed-off-by: Michael Tokarev 

diff --git a/hw/audio/virtio-snd.c b/hw/audio/virtio-snd.c
index 541f0797ac..2b80072b04 100644
--- a/hw/audio/virtio-snd.c
+++ b/hw/audio/virtio-snd.c
@@ -282,11 +282,13 @@ uint32_t virtio_snd_set_pcm_params(VirtIOSound *s,
 error_report("Number of channels is not supported.");
 return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
 }
-if (!(supported_formats & BIT(params->format))) {
+if (BIT(params->format) > sizeof(supported_formats) ||
+!(supported_formats & BIT(params->format))) {
 error_report("Stream format is not supported.");
 return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
 }
-if (!(supported_rates & BIT(params->rate))) {
+if (BIT(params->rate) > sizeof(supported_rates) ||
+!(supported_rates & BIT(params->rate))) {
 error_report("Stream rate is not supported.");
 return cpu_to_le32(VIRTIO_SND_S_NOT_SUPP);
 }
-- 
2.39.2

[Stable-9.0.3 13/69] intel_iommu: fix FRCD construction macro

[Michael Tokarev]

From: Clément Mathieu--Drif 

The constant must be unsigned, otherwise the two's complement
overrides the other fields when a PASID is present.

Fixes: 1b2b12376c8a ("intel-iommu: PASID support")
Signed-off-by: Clément Mathieu--Drif 
Reviewed-by: Yi Liu 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Minwoo Im 
Message-Id: <20240709142557.317271-2-clement.mathieu--d...@eviden.com>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit a3c8d7e38550c3d5a46e6fa94ffadfa625a4861d)
Signed-off-by: Michael Tokarev 

diff --git a/hw/i386/intel_iommu_internal.h b/hw/i386/intel_iommu_internal.h
index f8cf99bddf..cbc4030031 100644
--- a/hw/i386/intel_iommu_internal.h
+++ b/hw/i386/intel_iommu_internal.h
@@ -267,7 +267,7 @@
 /* For the low 64-bit of 128-bit */
 #define VTD_FRCD_FI(val)((val) & ~0xfffULL)
 #define VTD_FRCD_PV(val)(((val) & 0xULL) << 40)
-#define VTD_FRCD_PP(val)(((val) & 0x1) << 31)
+#define VTD_FRCD_PP(val)(((val) & 0x1ULL) << 31)
 #define VTD_FRCD_IR_IDX(val)(((val) & 0xULL) << 48)
 
 /* DMA Remapping Fault Conditions */
-- 
2.39.2

[Stable-9.0.3 34/69] virtio-net: Fix network stall at the host side waiting for kick

[Michael Tokarev]

From: thomas 

Patch 06b12970174 ("virtio-net: fix network stall under load")
added double-check to test whether the available buffer size
can satisfy the request or not, in case the guest has added
some buffers to the avail ring simultaneously after the first
check. It will be lucky if the available buffer size becomes
okay after the double-check, then the host can send the packet
to the guest. If the buffer size still can't satisfy the request,
even if the guest has added some buffers, viritio-net would
stall at the host side forever.

The patch enables notification and checks whether the guest has
added some buffers since last check of available buffers when
the available buffers are insufficient. If no buffer is added,
return false, else recheck the available buffers in the loop.
If the available buffers are sufficient, disable notification
and return true.

Changes:
1. Change the return type of virtqueue_get_avail_bytes() from void
   to int, it returns an opaque that represents the shadow_avail_idx
   of the virtqueue on success, else -1 on error.
2. Add a new API: virtio_queue_enable_notification_and_check(),
   it takes an opaque as input arg which is returned from
   virtqueue_get_avail_bytes(). It enables notification firstly,
   then checks whether the guest has added some buffers since
   last check of available buffers or not by virtio_queue_poll(),
   return ture if yes.

The patch also reverts patch "06b12970174".

The case below can reproduce the stall.

   Guest 0
 ++
 | iperf  |
---> | server |
 Host   |++
   ++   |...
   | iperf  |
   | client |  Guest n
   ++   |++
|| iperf  |
---> | server |
 ++

Boot many guests from qemu with virtio network:
 qemu ... -netdev tap,id=net_x \
-device virtio-net-pci-non-transitional,\
iommu_platform=on,mac=xx:xx:xx:xx:xx:xx,netdev=net_x

Each guest acts as iperf server with commands below:
 iperf3 -s -D -i 10 -p 8001
 iperf3 -s -D -i 10 -p 8002

The host as iperf client:
 iperf3 -c guest_IP -p 8001 -i 30 -w 256k -P 20 -t 4
 iperf3 -c guest_IP -p 8002 -i 30 -w 256k -P 20 -t 4

After some time, the host loses connection to the guest,
the guest can send packet to the host, but can't receive
packet from the host.

It's more likely to happen if SWIOTLB is enabled in the guest,
allocating and freeing bounce buffer takes some CPU ticks,
copying from/to bounce buffer takes more CPU ticks, compared
with that there is no bounce buffer in the guest.
Once the rate of producing packets from the host approximates
the rate of receiveing packets in the guest, the guest would
loop in NAPI.

 receive packets---
   | |
   v |
   free buf  virtnet_poll
   | |
   v |
 add buf to avail ring  ---
   |
   |  need kick the host?
   |  NAPI continues
   v
 receive packets---
   | |
   v |
   free buf  virtnet_poll
   | |
   v |
 add buf to avail ring  ---
   |
   v
  ...   ...

On the other hand, the host fetches free buf from avail
ring, if the buf in the avail ring is not enough, the
host notifies the guest the event by writing the avail
idx read from avail ring to the event idx of used ring,
then the host goes to sleep, waiting for the kick signal
from the guest.

Once the guest finds the host is waiting for kick singal
(in virtqueue_kick_prepare_split()), it kicks the host.

The host may stall forever at the sequences below:

 HostGuest
  ---
 fetch buf, send packet   receive packet ---
 ...  ... |
 fetch buf, send packet add buf   |
 ...add buf   virtnet_poll
buf not enough  avail idx-> add buf   |
read avail idx  add buf   |
add buf  ---
  receive packet ---
write event idx   ... |
wait for kick   add buf   virtnet_poll
  ... |
 ---
 no more packet, exit NAPI

In the first loop of NAPI above, indicated in the range of
virtnet_poll above, the host is sending packets while the
guest is receiving pa

[Stable-9.0.3 30/69] hw/i386/amd_iommu: Don't leak memory in amdvi_update_iotlb()

[Michael Tokarev]

From: Peter Maydell 

In amdvi_update_iotlb() we will only put a new entry in the hash
table if to_cache.perm is not IOMMU_NONE.  However we allocate the
memory for the new AMDVIIOTLBEntry and for the hash table key
regardless.  This means that in the IOMMU_NONE case we will leak the
memory we alloacted.

Move the allocations into the if() to the point where we know we're
going to add the item to the hash table.

Cc: qemu-sta...@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2452
Signed-off-by: Peter Maydell 
Message-Id: <20240731170019.3590563-1-peter.mayd...@linaro.org>
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
(cherry picked from commit 9a45b0761628cc59267b3283a85d15294464ac31)
Signed-off-by: Michael Tokarev 

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 6d4fde72f9..87643d2891 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -357,12 +357,12 @@ static void amdvi_update_iotlb(AMDVIState *s, uint16_t 
devid,
uint64_t gpa, IOMMUTLBEntry to_cache,
uint16_t domid)
 {
-AMDVIIOTLBEntry *entry = g_new(AMDVIIOTLBEntry, 1);
-uint64_t *key = g_new(uint64_t, 1);
-uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
-
 /* don't cache erroneous translations */
 if (to_cache.perm != IOMMU_NONE) {
+AMDVIIOTLBEntry *entry = g_new(AMDVIIOTLBEntry, 1);
+uint64_t *key = g_new(uint64_t, 1);
+uint64_t gfn = gpa >> AMDVI_PAGE_SHIFT_4K;
+
 trace_amdvi_cache_update(domid, PCI_BUS_NUM(devid), PCI_SLOT(devid),
 PCI_FUNC(devid), gpa, to_cache.translated_addr);
 
-- 
2.39.2

[Stable-9.0.3 28/69] target/arm: Ignore SMCR_EL2.LEN and SVCR_EL2.LEN if EL2 is not enabled

[Michael Tokarev]

From: Peter Maydell 

When determining the current vector length, the SMCR_EL2.LEN and
SVCR_EL2.LEN settings should only be considered if EL2 is enabled
(compare the pseudocode CurrentSVL and CurrentNSVL which call
EL2Enabled()).

We were checking against ARM_FEATURE_EL2 rather than calling
arm_is_el2_enabled(), which meant that we would look at
SMCR_EL2/SVCR_EL2 when in Secure EL1 or Secure EL0 even if Secure EL2
was not enabled.

Use the correct check in sve_vqm1_for_el_sm().

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20240722172957.1041231-5-peter.mayd...@linaro.org
(cherry picked from commit f573ac059ed060234fcef4299fae9e500d357c33)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/helper.c b/target/arm/helper.c
index a620481d7c..42044ae14b 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7191,7 +7191,7 @@ uint32_t sve_vqm1_for_el_sm(CPUARMState *env, int el, 
bool sm)
 if (el <= 1 && !el_is_in_host(env, el)) {
 len = MIN(len, 0xf & (uint32_t)cr[1]);
 }
-if (el <= 2 && arm_feature(env, ARM_FEATURE_EL2)) {
+if (el <= 2 && arm_is_el2_enabled(env)) {
 len = MIN(len, 0xf & (uint32_t)cr[2]);
 }
 if (arm_feature(env, ARM_FEATURE_EL3)) {
-- 
2.39.2

[Stable-9.0.3 25/69] target/arm: Don't assert for 128-bit tile accesses when SVL is 128

[Michael Tokarev]

From: Peter Maydell 

For an instruction which accesses a 128-bit element tile when
the SVL is also 128 (for example MOV z0.Q, p0/M, ZA0H.Q[w0,0]),
we will assert in get_tile_rowcol():

qemu-system-aarch64: ../../tcg/tcg-op.c:926: tcg_gen_deposit_z_i32: Assertion 
`len > 0' failed.

This happens because we calculate
len = ctz32(streaming_vec_reg_size(s)) - esz;$
but if the SVL and the element size are the same len is 0, and
the deposit operation asserts.

In this case the ZA storage contains exactly one 128 bit
element ZA tile, and the horizontal or vertical slice is just
that tile. This means that regardless of the index value in
the Ws register, we always access that tile. (In pseudocode terms,
we calculate (index + offset) MOD 1, which is 0.)

Special case the len == 0 case to avoid hitting the assertion
in tcg_gen_deposit_z_i32().

Cc: qemu-sta...@nongnu.org
Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20240722172957.1041231-2-peter.mayd...@linaro.org
(cherry picked from commit 56f1c0db928aae0b83fd91c89ddb226b137e2b21)
Signed-off-by: Michael Tokarev 

diff --git a/target/arm/tcg/translate-sme.c b/target/arm/tcg/translate-sme.c
index 185a8a917b..a50a419af2 100644
--- a/target/arm/tcg/translate-sme.c
+++ b/target/arm/tcg/translate-sme.c
@@ -49,7 +49,15 @@ static TCGv_ptr get_tile_rowcol(DisasContext *s, int esz, 
int rs,
 /* Prepare a power-of-two modulo via extraction of @len bits. */
 len = ctz32(streaming_vec_reg_size(s)) - esz;
 
-if (vertical) {
+if (!len) {
+/*
+ * SVL is 128 and the element size is 128. There is exactly
+ * one 128x128 tile in the ZA storage, and so we calculate
+ * (Rs + imm) MOD 1, which is always 0. We need to special case
+ * this because TCG doesn't allow deposit ops with len 0.
+ */
+tcg_gen_movi_i32(tmp, 0);
+} else if (vertical) {
 /*
  * Compute the byte offset of the index within the tile:
  * (index % (svl / size)) * size
-- 
2.39.2