Re: [PATCH v3 1/2] ppc: spapr: cleanup cr get/set with helpers.

[Harsh Prateek Bora]

On 5/4/23 02:19, Daniel Henrique Barboza wrote:

I squashed in this change to fix it:


diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 241d9e27e5..424f2e1741 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -75,7 +75,7 @@ void ppc_set_cr(CPUPPCState *env, uint64_t cr)
   }
   }
-uint64_t ppc_get_cr(CPUPPCState *env)
+uint64_t ppc_get_cr(const CPUPPCState *env)
   {
   uint64_t cr = 0;
   for (int i = 0; i < 8; i++) {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0af94170d0..1c02596d9f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2774,7 +2774,7 @@ void ppc_maybe_bswap_register(CPUPPCState *env, 
uint8_t *mem_buf, int len);

   void ppc_store_vscr(CPUPPCState *env, uint32_t vscr);
   uint32_t ppc_get_vscr(CPUPPCState *env);
   void ppc_set_cr(CPUPPCState *env, uint64_t cr);
-uint64_t ppc_get_cr(CPUPPCState *env);
+uint64_t ppc_get_cr(const CPUPPCState *env);
   
/*/



Just a FYI,. No need to re-send.


Oh ok, thanks Daniel.




Daniel

[PATCH v2 2/2] tests: tcg: ppc64: Add tests for Vector Extract Mask Instructions

[Shivaprasad G Bhat]

Add test for vextractbm, vextractwm, vextractdm and vextractqm
instructions. Test works for both qemu-ppc64 and qemu-ppc64le.

Based on the test case written by John Platts posted at [1]

References:
[1] - https://gitlab.com/qemu-project/qemu/-/issues/1536

Signed-off-by: John Platts 
Signed-off-by: Shivaprasad G Bhat 
Reviewed-by: Lucas Mateus Castro 
---
 tests/tcg/ppc64/Makefile.target |5 +++-
 tests/tcg/ppc64/vector.c|   51 +++
 2 files changed, 55 insertions(+), 1 deletion(-)
 create mode 100644 tests/tcg/ppc64/vector.c

diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index 6d47d3cae6..b084963b9a 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -20,7 +20,7 @@ PPC64_TESTS += mtfsf
 PPC64_TESTS += mffsce
 
 ifneq ($(CROSS_CC_HAS_POWER10),)
-PPC64_TESTS += byte_reverse sha512-vector
+PPC64_TESTS += byte_reverse sha512-vector vector
 endif
 byte_reverse: CFLAGS += -mcpu=power10
 run-byte_reverse: QEMU_OPTS+=-cpu POWER10
@@ -31,6 +31,9 @@ sha512-vector: sha512.c
 
 run-sha512-vector: QEMU_OPTS+=-cpu POWER10
 
+vector: CFLAGS += -mcpu=power10 -I$(SRC_PATH)/include
+run-vector: QEMU_OPTS += -cpu POWER10
+
 PPC64_TESTS += signal_save_restore_xer
 PPC64_TESTS += xxspltw
 
diff --git a/tests/tcg/ppc64/vector.c b/tests/tcg/ppc64/vector.c
new file mode 100644
index 00..cbf4ae9332
--- /dev/null
+++ b/tests/tcg/ppc64/vector.c
@@ -0,0 +1,51 @@
+#include 
+#include 
+#include "qemu/compiler.h"
+
+int main(void)
+{
+unsigned int result_wi;
+vector unsigned char vbc_bi_src = { 0xFF, 0xFF, 0, 0xFF, 0xFF, 0xFF,
+0xFF, 0xFF, 0xFF, 0xFF, 0, 0, 0,
+0, 0xFF, 0xFF};
+vector unsigned short vbc_hi_src = { 0x, 0, 0, 0x,
+ 0, 0, 0x, 0x};
+vector unsigned int vbc_wi_src = {0, 0, 0x, 0x};
+vector unsigned long long vbc_di_src = {0x, 0};
+vector __uint128_t vbc_qi_src;
+
+asm("vextractbm %0, %1" : "=r" (result_wi) : "v" (vbc_bi_src));
+#if HOST_BIG_ENDIAN
+assert(result_wi == 0b11011111);
+#else
+assert(result_wi == 0b11111011);
+#endif
+
+asm("vextracthm %0, %1" : "=r" (result_wi) : "v" (vbc_hi_src));
+#if HOST_BIG_ENDIAN
+assert(result_wi == 0b10010011);
+#else
+assert(result_wi == 0b11001001);
+#endif
+
+asm("vextractwm %0, %1" : "=r" (result_wi) : "v" (vbc_wi_src));
+#if HOST_BIG_ENDIAN
+assert(result_wi == 0b0011);
+#else
+assert(result_wi == 0b1100);
+#endif
+
+asm("vextractdm %0, %1" : "=r" (result_wi) : "v" (vbc_di_src));
+#if HOST_BIG_ENDIAN
+assert(result_wi == 0b10);
+#else
+assert(result_wi == 0b01);
+#endif
+
+vbc_qi_src[0] = 0x1;
+vbc_qi_src[0] = vbc_qi_src[0] << 127;
+asm("vextractqm %0, %1" : "=r" (result_wi) : "v" (vbc_qi_src));
+assert(result_wi == 0b1);
+
+return 0;
+}

[PATCH v2 1/2] tcg: ppc64: Fix mask generation for vextractdm

[Shivaprasad G Bhat]

In function do_extractm() the mask is calculated as
dup_const(1 << (element_width - 1)). '1' being signed int
works fine for MO_8,16,32. For MO_64, on PPC64 host
this ends up becoming 0 on compilation. The vextractdm
uses MO_64, and it ends up having mask as 0.

Explicitly use 1ULL instead of signed int 1 like its
used everywhere else.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1536
Signed-off-by: Shivaprasad G Bhat 
Reviewed-by: Alex Bennée 
Reviewed-by: Lucas Mateus Castro 
Reviewed-by: Richard Henderson 
---
 target/ppc/translate/vmx-impl.c.inc |2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/translate/vmx-impl.c.inc 
b/target/ppc/translate/vmx-impl.c.inc
index 112233b541..c8712dd7d8 100644
--- a/target/ppc/translate/vmx-impl.c.inc
+++ b/target/ppc/translate/vmx-impl.c.inc
@@ -2058,7 +2058,7 @@ static bool trans_VEXPANDQM(DisasContext *ctx, arg_VX_tb 
*a)
 static bool do_vextractm(DisasContext *ctx, arg_VX_tb *a, unsigned vece)
 {
 const uint64_t elem_width = 8 << vece, elem_count_half = 8 >> vece,
-   mask = dup_const(vece, 1 << (elem_width - 1));
+   mask = dup_const(vece, 1ULL << (elem_width - 1));
 uint64_t i, j;
 TCGv_i64 lo, hi, t0, t1;

[PATCH v2 0/2] tcg: ppc64: Fix mask generation for vextractdm

[Shivaprasad G Bhat]

While debugging gitlab issue[1] 1536, I happen to try the
vextract[X]m instructions on the real hardware. The test
used in [1] is failing for vextractdm.

On debugging it is seen, in function do_extractm() the
mask is calculated as dup_const(1 << (element_width - 1)).
'1' being signed int works fine for MO_8,16,32. For MO_64,
on PPC64 host this ends up becoming 0 on compilation. The
vextractdm uses MO_64, and it ends up having mask as 0.

The first patch here fixes that by explicitly using
1ULL instead of signed int 1 like its used everywhere else.
Second patch introduces the test case from [1] into qemu
tcg/ppc64 along with fixes/tweaks to make it work for both
big and little-endian targets.

References:
[1] : https://gitlab.com/qemu-project/qemu/-/issues/1536

---
Changelog:
Since v1 : https://lists.gnu.org/archive/html/qemu-devel/2023-04/msg01958.html
 - Added "Resolves: " to first patch description
 - Rebased to top of the tree. I see with d044b7c33a5, Alex has limited the
   scope of plugin tests to just the MULTIARCH_TESTS. So, removed the plugin
   tests for the test case added in the second patch.
 - Changed the test case to use the HOST_BIG_ENDIAN from compiler.h

Shivaprasad G Bhat (2):
  tcg: ppc64: Fix mask generation for vextractdm
  tests: tcg: ppc64: Add tests for Vector Extract Mask Instructions


 target/ppc/translate/vmx-impl.c.inc |  2 +-
 tests/tcg/ppc64/Makefile.target |  6 +++-
 tests/tcg/ppc64/vector.c| 51 +
 3 files changed, 57 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/ppc64/vector.c

--
Signature

Re: [PULL v3 0/10] xenpvh3-tag

[Vikram Garhwal]

Hi Richard,

On 5/3/23 12:47 AM, Richard Henderson wrote:

On 5/3/23 01:12, Stefano Stabellini wrote:

Hi Peter,

Vikram fixed the gitlab test problem, so now all the tests should
succeed. There were no changes to the QEMU code. I am resending the pull
request (I rebased it on staging, no conflicts.)

For reference this was the previous pull request:
https://marc.info/?l=qemu-devel=167641819725964

Cheers,

Stefano


The following changes since commit 
4ebc33f3f3b656ebf62112daca6aa0f8019b4891:


   Merge tag 'pull-tcg-20230502-2' of https://gitlab.com/rth7680/qemu 
into staging (2023-05-02 21:18:45 +0100)


are available in the Git repository at:

   https://gitlab.com/sstabellini/qemu xenpvh3-tag

for you to fetch changes up to bc618c54318cbc2fcb9decf9d4c193cc336a0dbc:

   meson.build: enable xenpv machine build for ARM (2023-05-02 
17:04:54 -0700)



Stefano Stabellini (5):
   hw/i386/xen/xen-hvm: move x86-specific fields out of XenIOState
   xen-hvm: reorganize xen-hvm and move common function to 
xen-hvm-common
   include/hw/xen/xen_common: return error from 
xen_create_ioreq_server
   hw/xen/xen-hvm-common: skip ioreq creation on ioreq 
registration failure
   meson.build: do not set have_xen_pci_passthrough for aarch64 
targets


Vikram Garhwal (5):
   hw/i386/xen/: move xen-mapcache.c to hw/xen/
   hw/i386/xen: rearrange xen_hvm_init_pc
   hw/xen/xen-hvm-common: Use g_new and error_report
   hw/arm: introduce xenpvh machine
   meson.build: enable xenpv machine build for ARM


Errors in CI:

https://gitlab.com/qemu-project/qemu/-/jobs/4216392008#L2381

../hw/i386/xen/xen-hvm.c:303:9: error: implicit declaration of 
function 'error_report' is invalid in C99 
[-Werror,-Wimplicit-function-declaration]

    error_report("relocate_memory %lu pages from GFN %"HWADDR_PRIx
Thanks for notifying this. I am not sure why this particular build is 
failing. error_report() is defined in "|qemu/error-report.h" and the 
header should be included as |||it builds fine for other configs.

|Also, the same tsan-build passed when we sent the PULL for v2.

I am not sure why it's failing for this config. W||ill try to fix 
this. Meanwhile, any hints on how to fix/debug this?


Regards,
Vikram
|

^


r~

Re: [PATCH] softfloat: Fix the incorrect computation in float32_exp2()

[Shivaprasad G Bhat]

Hi Richard,


On 5/3/23 01:11, Richard Henderson wrote:

On 5/2/23 16:25, Shivaprasad G Bhat wrote:

The float32_exp2() is computing wrong exponent of 2.
For example, with the following set of values {0.1, 2.0, 2.0, -1.0},
the expected output would be {1.071773, 4.00, 4.00, 0.50}.
Instead, the function is computing {1.119102, 3.382044, 3.382044, 
-0.191022}





his is because instead of the xnp which holds the numerator,
parts_muladd is using the xp which is just 'x'. The commit 
'572c4d862ff2'
refactored this function, and it seems mistakenly using xp instead of 
xnp.


The patches fixes this possible typo.

Fixes: 572c4d862ff2 "softfloat: Convert float32_exp2 to FloatParts"
Partially-Resolves:https://gitlab.com/qemu-project/qemu/-/issues/1623
Reported-By: Luca Barbato (https://gitlab.com/lu-zero)
Signed-off-by: Shivaprasad G Bhat
Signed-off-by: Vaibhav Jain
---
  fpu/softfloat.c |    2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Whoops.  Good catch.



If you are fine with the patch, could you fix the mail id for Vaibhav 
Jain as


  while pulling ?

If you have other comments, I will fix it in the next version otherwise.


Thanks,

Shivaprasad


H


r~

Re: [PATCH 2/2] tests: tcg: ppc64: Add tests for Vector Extract Mask Instructions

[Shivaprasad G Bhat]

On 5/2/23 12:35, Cédric Le Goater wrote:

On 4/13/23 21:01, Shivaprasad G Bhat wrote:

Add test for vextractbm, vextractwm, vextractdm and vextractqm
instructions. Test works for both qemu-ppc64 and qemu-ppc64le.

Based on the test case written by John Platts posted at [1]

References:
[1]: https://gitlab.com/qemu-project/qemu/-/issues/1536


Gitlab issues should be referenced as :

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

However, this patch adds a test, not a fix. So it is the previous patch
which should be annotated as resolving the issue.

Also, I think the code should be using  HOST_BIG_ENDIAN instead of
__ORDER_BIG_ENDIAN__


Thanks for the comments Cédric.

Fixing these in v2.

Thanks,

Shivaprasad

Re: [PATCH v2] vhost-user: send SET_STATUS 0 after GET_VRING_BASE

[Yajun Wu]

On 5/2/2023 7:04 AM, Stefan Hajnoczi wrote:

Setting the VIRTIO Device Status Field to 0 resets the device. The
device's state is lost, including the vring configuration.

vhost-user.c currently sends SET_STATUS 0 before GET_VRING_BASE. This
risks confusion about the lifetime of the vhost-user state (e.g. vring
last_avail_idx) across VIRTIO device reset.

Eugenio Pérez  adjusted the order for vhost-vdpa.c
in commit c3716f260bff ("vdpa: move vhost reset after get vring base")
and in that commit description suggested doing the same for vhost-user
in the future.

Go ahead and adjust vhost-user.c now. I ran various online code searches
to identify vhost-user backends implementing SET_STATUS. It seems only
DPDK implements SET_STATUS and Yajun Wu  has
confirmed that it is safe to make this change.

Fixes: commit 923b8921d210763359e96246a58658ac0db6c645 ("vhost-user: Support 
vhost_dev_start")
Cc: Michael S. Tsirkin 
Cc: Cindy Lu 
Cc: Yajun Wu 
Signed-off-by: Stefan Hajnoczi 
---
v2:
- Added VHOST_USER_PROTOCOL_F_STATUS check [Yajun Wu]
- Added "Fixes:" tag [Michael]
---
  hw/virtio/vhost-user.c | 16 +++-
  1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e5285df4ba..40974afd06 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2677,7 +2677,20 @@ static int vhost_user_dev_start(struct vhost_dev *dev, 
bool started)
VIRTIO_CONFIG_S_DRIVER |
VIRTIO_CONFIG_S_DRIVER_OK);
  } else {
-return vhost_user_set_status(dev, 0);
+return 0;
+}
+}
+
+static void vhost_user_reset_status(struct vhost_dev *dev)
+{
+/* Set device status only for last queue pair */
+if (dev->vq_index + dev->nvqs != dev->vq_index_end) {
+return;
+}
+
+if (virtio_has_feature(dev->protocol_features,
+   VHOST_USER_PROTOCOL_F_STATUS)) {
+vhost_user_set_status(dev, 0);
  }
  }

@@ -2716,4 +2729,5 @@ const VhostOps user_ops = {
  .vhost_get_inflight_fd = vhost_user_get_inflight_fd,
  .vhost_set_inflight_fd = vhost_user_set_inflight_fd,
  .vhost_dev_start = vhost_user_dev_start,
+.vhost_reset_status = vhost_user_reset_status,
  };
--
2.40.1


Reviewed-by: Yajun Wu 

Re: [PATCH RFC v2 6/9] target/loongarch: Implement kvm_arch_init_vcpu

[Tianrui Zhao]

在 2023年05月02日 19:32, Richard Henderson 写道:

On 4/27/23 08:26, Tianrui Zhao wrote:

+static void kvm_loongarch_vm_stage_change(void *opaque, bool running,


Typo:   state


+uint64_t counter_value;


I know naming is hard, but this is so generic it is difficult to 
determine what it does. Perhaps kvm_state_counter?  A comment would 
also be helpful, even with a renaming.



r~

Thanks, I will rename this variable like kvm_state_counter.

Thanks
Tianrui Zhao

Re: [PATCH RFC v2 4/9] target/loongarch: Implement kvm get/set registers

[Tianrui Zhao]

在 2023年05月02日 19:24, Richard Henderson 写道:

On 4/27/23 08:26, Tianrui Zhao wrote:

Implement kvm_arch_get/set_registers interfaces, many regs
can be get/set in the function, such as core regs, csr regs,
fpu regs, mp state, etc.

Signed-off-by: Tianrui Zhao 
---
  meson.build   |   1 +
  target/loongarch/kvm.c| 356 +-
  target/loongarch/trace-events |  11 ++
  target/loongarch/trace.h  |   1 +
  4 files changed, 367 insertions(+), 2 deletions(-)
  create mode 100644 target/loongarch/trace-events
  create mode 100644 target/loongarch/trace.h

diff --git a/meson.build b/meson.build
index 29f8644d6d..b1b29299da 100644
--- a/meson.build
+++ b/meson.build
@@ -3039,6 +3039,7 @@ if have_system or have_user
  'target/s390x',
  'target/s390x/kvm',
  'target/sparc',
+'target/loongarch',
]


Sort before mips to keep alphabetic ordering.

Thanks, I will move it to the suitable place.

Thanks
Tianrui Zhao



+static int kvm_loongarch_get_regs_core(CPUState *cs)
+{
+int ret = 0;
+int i;
+struct kvm_regs regs;
+LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+CPULoongArchState *env = >env;
+
+/* Get the current register set as KVM seems it */
+ret = kvm_vcpu_ioctl(cs, KVM_GET_REGS, );
+if (ret < 0) {
+trace_kvm_failed_get_regs_core(strerror(errno));
+return ret;
+}
+
+for (i = 0; i < 32; i++) {
+env->gpr[i] = regs.gpr[i];


For i = 1; register 0 is 0...

Thanks,  I will fix it.

Thanks
Tianrui Zhao



+static inline int kvm_larch_getq(CPUState *cs, uint64_t reg_id,
+ uint64_t *addr)
+{
+struct kvm_one_reg csrreg = {
+.id = reg_id,
+.addr = (uintptr_t)addr
+};
+
+return kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
+}


Drop inline marker and let the compiler choose.

Thanks , I will drop the inline statement.

Thanks
Tianrui Zhao



+static inline int kvm_larch_putq(CPUState *cs, uint64_t reg_id,
+ uint64_t *addr)


Likewise.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [PATCH RFC v2 3/9] target/loongarch: Supplement vcpu env initial when vcpu reset

[Tianrui Zhao]

在 2023年05月02日 19:04, Richard Henderson 写道:

On 4/27/23 08:26, Tianrui Zhao wrote:

Supplement vcpu env initial when vcpu reset, including
init vcpu mp_state value to KVM_MP_STATE_RUNNABLE and
init vcpu CSR_CPUID,CSR_TID to cpu->cpu_index.

Signed-off-by: Tianrui Zhao
---
  target/loongarch/cpu.c | 3 +++
  target/loongarch/cpu.h | 2 ++
  2 files changed, 5 insertions(+)


Why do you need KVM_MP_STATE_RUNNABLE in loongarch/cpu.c, outside of 
kvm.c?

For Arm, we test the architectural power state of the cpu.


r~
Thanks for your reviewing, we want to set mp_state to default value when 
vcpu reset, so we add it in cpu.c. When I reference other archs, I think 
I should add a new function named kvm_arch_reset_vcpu in kvm.c which 
will be called by vcpu_reset and move the reset mp_state into it.


Thanks
Tianrui Zhao

Re: [RFC PATCH v4 00/44] Add LoongArch LSX instructions

[Song Gao]

在 2023/5/2 上午2:43, Richard Henderson 写道:

On 4/25/23 08:02, Song Gao wrote:

Hi,

This series adds LoongArch LSX instructions, Since the LoongArch
Vol2 is not open, So we use 'RFC' title.

I'm not sure when the manual will be open.
After these patches are reviewed, how about merging them?

About test:
V2 we use RISU test the LoongArch LSX instructions.

QEMU:
https://github.com/loongson/qemu/tree/tcg-old-abi-support-lsx
RISU:
 https://github.com/loongson/risu/tree/loongarch-suport-lsx

Build test:
make docker-test-build@fedora-i386-cross

The following patches need to be reviewed:
   0001-target-loongarch-Add-LSX-data-type-VReg.patch
   0014-target-loongarch-Implement-vmul-vmuh-vmulw-ev-od.patch
   0030-target-loongarch-Implement-vpcnt.patch
0034-target-loongarch-Implement-LSX-fpu-fcvt-instructions.patch
   0037-target-loongarch-Implement-vbitsel-vset.patch
   0041-target-loongarch-Implement-vld-vst.patch

V4:
   - R-b and rebase;
   - Migrate the upper half lsx regs;
   - Remove tcg_gen_mulus2_*;
   - Vsetallnez use !do_match2;
   - Use tcg_gen_concat_i64_i128/tcg_gen_extr_i128_i64 to replace
 TCGV128_LOW(val)/TCGV128_High(val);


One minor nit, everything reviewed!  Congratulations.


Thank you for your guidance and review.

Since all patches are reviewed, how about drop 'RFC' on v5?
I am  really not sure When the Vol2 will be open.

Thanks.
Song Gao

Re: [PATCH v3 01/11] hw: arm: Add bananapi M2-Ultra and allwinner-r40 support

[qianfan]

在 2023/5/3 4:01, Niek Linnenbank 写道:

Hi Qianfan,

Sorry for my late response, I had a holiday in between.

On Tue, Apr 18, 2023 at 1:21 PM  wrote:

From: qianfan Zhao 

Allwinner R40 (sun8i) SoC features a Quad-Core Cortex-A7 ARM CPU,
and a Mali400 MP2 GPU from ARM. It's also known as the Allwinner T3
for In-Car Entertainment usage, A40i and A40pro are variants that
differ in applicable temperatures range (industrial and military).

Signed-off-by: qianfan Zhao 
---
 configs/devices/arm-softmmu/default.mak |   1 +
 hw/arm/Kconfig                          |   9 +
 hw/arm/allwinner-r40.c                  | 418

 hw/arm/bananapi_m2u.c                   | 129 
 hw/arm/meson.build                      |   1 +
 include/hw/arm/allwinner-r40.h          | 110 +++
 6 files changed, 668 insertions(+)
 create mode 100644 hw/arm/allwinner-r40.c
 create mode 100644 hw/arm/bananapi_m2u.c
 create mode 100644 include/hw/arm/allwinner-r40.h

diff --git a/configs/devices/arm-softmmu/default.mak
b/configs/devices/arm-softmmu/default.mak
index 1b49a7830c..76a43add23 100644
--- a/configs/devices/arm-softmmu/default.mak
+++ b/configs/devices/arm-softmmu/default.mak
@@ -43,3 +43,4 @@ CONFIG_FSL_IMX6UL=y
 CONFIG_SEMIHOSTING=y
 CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
 CONFIG_ALLWINNER_H3=y
+CONFIG_ALLWINNER_R40=y


Looks like on latest master (c586691) this file has already been 
modified - the patch doesn't apply anymore.


Could you please rebase your patch series in V4 to make them 
compatible again?
After your rebase is done, I'll try to complete reviewing the 
remaining patches.



Sure.


diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..9e14c3427e 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -344,6 +344,15 @@ config ALLWINNER_H3
     select USB_EHCI_SYSBUS
     select SD

+config ALLWINNER_R40
+    bool
+    select ALLWINNER_A10_PIT
+    select SERIAL
+    select ARM_TIMER
+    select ARM_GIC
+    select UNIMP
+    select SD
+
 config RASPI
     bool
     select FRAMEBUFFER
diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
new file mode 100644
index 00..b743d64253
--- /dev/null
+++ b/hw/arm/allwinner-r40.c
@@ -0,0 +1,418 @@
+/*
+ * Allwinner R40/A40i/T3 System on Chip emulation
+ *
+ * Copyright (C) 2023 qianfan Zhao 
+ *
+ * This program is free software: you can redistribute it and/or
modify
+ * it under the terms of the GNU General Public License as
published by
+ * the Free Software Foundation, either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see
.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "qemu/bswap.h"
+#include "qemu/module.h"
+#include "qemu/units.h"
+#include "hw/qdev-core.h"
+#include "hw/sysbus.h"
+#include "hw/char/serial.h"
+#include "hw/misc/unimp.h"
+#include "hw/usb/hcd-ehci.h"
+#include "hw/loader.h"
+#include "sysemu/sysemu.h"
+#include "hw/arm/allwinner-r40.h"
+
+/* Memory map */
+const hwaddr allwinner_r40_memmap[] = {
+    [AW_R40_DEV_SRAM_A1]    = 0x,
+    [AW_R40_DEV_SRAM_A2]    = 0x4000,
+    [AW_R40_DEV_SRAM_A3]    = 0x8000,
+    [AW_R40_DEV_SRAM_A4]    = 0xb400,
+    [AW_R40_DEV_MMC0]       = 0x01c0f000,
+    [AW_R40_DEV_MMC1]       = 0x01c1,
+    [AW_R40_DEV_MMC2]       = 0x01c11000,
+    [AW_R40_DEV_MMC3]       = 0x01c12000,
+    [AW_R40_DEV_PIT]        = 0x01c20c00,
+    [AW_R40_DEV_UART0]      = 0x01c28000,
+    [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
+    [AW_R40_DEV_GIC_CPU]    = 0x01c82000,
+    [AW_R40_DEV_GIC_HYP]    = 0x01c84000,
+    [AW_R40_DEV_GIC_VCPU]   = 0x01c86000,
+    [AW_R40_DEV_SDRAM]      = 0x4000
+};
+
+/* List of unimplemented devices */
+struct AwR40Unimplemented {
+    const char *device_name;
+    hwaddr base;
+    hwaddr size;
+};
+
+static struct AwR40Unimplemented r40_unimplemented[] = {
+    { "d-engine",   0x0100, 4 * MiB },
+    { "d-inter",    0x0140, 128 * KiB },
+    { "sram-c",     0x01c0, 4 * KiB },
+    { "dma",        0x01c02000, 4 * KiB },
+    { "nfdc",       0x01c03000, 4 * KiB },
+    { "ts", 

Re: [PATCH v4 07/10] migration: split migration_incoming_co

[Vladimir Sementsov-Ogievskiy]

On 02.05.23 23:48, Peter Xu wrote:

On Fri, Apr 28, 2023 at 10:49:25PM +0300, Vladimir Sementsov-Ogievskiy wrote:

Originally, migration_incoming_co was introduced by
25d0c16f625feb3b6
"migration: Switch to COLO process after finishing loadvm"
to be able to enter from COLO code to one specific yield point, added
by 25d0c16f625feb3b6.

Later in 923709896b1b0
  "migration: poll the cm event for destination qemu"
we reused this variable to wake the migration incoming coroutine from
RDMA code.

That was doubtful idea. Entering coroutines is a very fragile thing:
you should be absolutely sure which yield point you are going to enter.

I don't know how much is it safe to enter during qemu_loadvm_state()
which I think what RDMA want to do. But for sure RDMA shouldn't enter
the special COLO-related yield-point. As well, COLO code doesn't want
to enter during qemu_loadvm_state(), it want to enter it's own specific
yield-point.

As well, when in 8e48ac95865ac97d
  "COLO: Add block replication into colo process" we added
bdrv_invalidate_cache_all() call (now it's called activate_all())
it became possible to enter the migration incoming coroutine during
that call which is wrong too.

So, let't make these things separate and disjoint: loadvm_co for RDMA,
non-NULL during qemu_loadvm_state(), and colo_incoming_co for COLO,
non-NULL only around specific yield.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  migration/colo.c  | 4 ++--
  migration/migration.c | 8 ++--
  migration/migration.h | 9 -
  3 files changed, 16 insertions(+), 5 deletions(-)


The idea looks right to me, but I really know mostly nothing on coroutines
and also rdma+colo..

Is the other ref in rdma.c (rdma_cm_poll_handler()) still missing?



Oops right.. I was building with rdma disabled. Will fix.

Thanks a lot for reviewing!

--
Best regards,
Vladimir

Re: [PATCH v4 03/10] build: move COLO under CONFIG_REPLICATION

[Vladimir Sementsov-Ogievskiy]

On 02.05.23 19:41, Peter Xu wrote:

  ##
  # @query-colo-status:
@@ -1674,7 +1676,8 @@
  # Since: 3.1
  ##
  { 'command': 'query-colo-status',
-  'returns': 'COLOStatus' }
+  'returns': 'COLOStatus',
+  'if': 'CONFIG_REPLICATION' }

I still see a bunch of other colo related definitions around in the qapi
schema, e.g. COLOExitReason.  Is it intended to leave them as is?



Removing them will make us to add more and more ifdefs in the code.

We decided to keep x-colo capability.
One more enum field is colo migration status - not worse than x-colo enum 
field, and is not possible with replication disabled

The others (like COLOExitReason) are just structure definitions - not a public 
API, so no reason to care about.

The exclustion is COLOStatus, we have to handle it. If not compilation fails:

qapi/qapi-commands-migration.c:821:13: error: ‘qmp_marshal_output_COLOStatus’ 
defined but not used [-Werror=unused-function]
  821 | static void qmp_marshal_output_COLOStatus(COLOStatus *ret_in,
  | ^
cc1: all warnings being treated as errors


- COLOStatus becomes unused with replication disabled, as it is used only in 
migration/colo.c

--
Best regards,
Vladimir

Re: [PATCH v3 1/2] ppc: spapr: cleanup cr get/set with helpers.

[Daniel Henrique Barboza]

This patch breaks linux-user build as follows:

[23/214] Compiling C object libqemu-ppc64-linux-user.fa.p/linux-user_elfload.c.o
FAILED: libqemu-ppc64-linux-user.fa.p/linux-user_elfload.c.o
cc -m64 -mcx16 -Ilibqemu-ppc64-linux-user.fa.p -I. -I.. -Itarget/ppc -I../target/ppc 
-I../common-user/host/x86_64 -I../linux-user/include/host/x86_64 -I../linux-user/include 
-Ilinux-user -I../linux-user -Ilinux-user/ppc -I../linux-user/ppc -Iqapi -Itrace -Iui -Iui/shader 
-I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -isystem 
/home/danielhb/powerpc/qemu/linux-headers -isystem linux-headers -iquote . -iquote 
/home/danielhb/powerpc/qemu -iquote /home/danielhb/powerpc/qemu/include -iquote 
/home/danielhb/powerpc/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE 
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef 
-Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration 
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self 
-Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined 
-Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs 
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -isystem../linux-headers 
-isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-linux-user-config-target.h"' 
'-DCONFIG_DEVICES="ppc64-linux-user-config-devices.h"' -MD -MQ 
libqemu-ppc64-linux-user.fa.p/linux-user_elfload.c.o -MF 
libqemu-ppc64-linux-user.fa.p/linux-user_elfload.c.o.d -o 
libqemu-ppc64-linux-user.fa.p/linux-user_elfload.c.o -c ../linux-user/elfload.c
../linux-user/elfload.c: In function ‘elf_core_copy_regs’:
../linux-user/elfload.c:964:22: error: passing argument 1 of ‘ppc_get_cr’ 
discards ‘const’ qualifier from pointer target type 
[-Werror=discarded-qualifiers]
   964 | ccr = ppc_get_cr(env);
   |  ^~~
In file included from ../linux-user/qemu.h:4,
  from ../linux-user/elfload.c:8:
../target/ppc/cpu.h:2777:34: note: expected ‘CPUPPCState *’ {aka ‘struct 
CPUArchState *’} but argument is of type ‘const CPUPPCState *’ {aka ‘const 
struct CPUArchState *’}
  2777 | uint64_t ppc_get_cr(CPUPPCState *env);
   | ~^~~
cc1: all warnings being treated as errors
[24/214] Compiling C object libqemu-ppc-softmmu.fa.p/target_ppc_power8-pmu.c.o


I squashed in this change to fix it:


diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 241d9e27e5..424f2e1741 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -75,7 +75,7 @@ void ppc_set_cr(CPUPPCState *env, uint64_t cr)
  }
  }
  
-uint64_t ppc_get_cr(CPUPPCState *env)

+uint64_t ppc_get_cr(const CPUPPCState *env)
  {
  uint64_t cr = 0;
  for (int i = 0; i < 8; i++) {
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0af94170d0..1c02596d9f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2774,7 +2774,7 @@ void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t 
*mem_buf, int len);
  void ppc_store_vscr(CPUPPCState *env, uint32_t vscr);
  uint32_t ppc_get_vscr(CPUPPCState *env);
  void ppc_set_cr(CPUPPCState *env, uint64_t cr);
-uint64_t ppc_get_cr(CPUPPCState *env);
+uint64_t ppc_get_cr(const CPUPPCState *env);
  
  /*/



Just a FYI,. No need to re-send.



Daniel



On 5/3/23 06:36, Harsh Prateek Bora wrote:

The bits in cr reg are grouped into eight 4-bit fields represented
by env->crf[8] and the related calculations should be abstracted to
keep the calling routines simpler to read. This is a step towards
cleaning up the related/calling code for better readability.

Signed-off-by: Harsh Prateek Bora 
Reviewed-by: Fabiano Rosas 
---
  hw/ppc/spapr_hcall.c  | 18 ++
  linux-user/elfload.c  |  4 +---
  linux-user/ppc/signal.c   |  9 ++---
  target/ppc/cpu.c  | 17 +
  target/ppc/cpu.h  |  2 ++
  target/ppc/gdbstub.c  | 22 --
  target/ppc/kvm.c  | 13 ++---
  target/ppc/ppc-qmp-cmds.c |  6 +-
  8 files changed, 31 insertions(+), 60 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index ec4def62f8..1c102c8c0d 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1566,8 +1566,6 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
  struct kvmppc_hv_guest_state hv_state;
  struct kvmppc_pt_regs *regs;
  hwaddr len;
-uint64_t cr;
-int i;
  
  if (spapr->nested_ptcr == 0) {

  return H_NOT_AVAILABLE;
@@ -1616,12 +1614,7 @@ static target_ulong h_enter_nested(PowerPCCPU *cpu,
  env->lr = regs->link;
  env->ctr = regs->ctr;
  cpu_write_xer(env, regs->xer);
-
-cr = regs->ccr;
-for (i = 7; i >= 0; i--) {
-

[PATCH v4 1/2] meson: Pass -j option to sphinx

[Fabiano Rosas]

Save a bit of build time by passing the number of jobs option to
sphinx.

We cannot use  the -j option from make because  meson does not support
setting build time parameters for custom targets. Use nproc instead or
the equivalent sphinx option "-j  auto", if that is available (version
>=1.7.0).

Also make sure our plugins support parallelism and report it properly
to sphinx. Particularly, implement the merge_domaindata method in
DBusDomain that is used to merge in data from other subprocesses.

Tested-by: Daniel P. Berrangé 
Signed-off-by: Fabiano Rosas 
---
 docs/meson.build   | 12 
 docs/sphinx/dbusdomain.py  |  4 
 docs/sphinx/fakedbusdoc.py |  5 +
 docs/sphinx/qmp_lexer.py   |  5 +
 4 files changed, 26 insertions(+)

diff --git a/docs/meson.build b/docs/meson.build
index f220800e3e..6d0986579e 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -10,6 +10,18 @@ if sphinx_build.found()
 SPHINX_ARGS += [ '-W', '-Dkerneldoc_werror=1' ]
   endif
 
+  sphinx_version = run_command(SPHINX_ARGS + ['--version'],
+   check: true).stdout().split()[1]
+  if sphinx_version.version_compare('>=1.7.0')
+SPHINX_ARGS += ['-j', 'auto']
+  else
+nproc = find_program('nproc')
+if nproc.found()
+  jobs = run_command(nproc, check: true).stdout()
+  SPHINX_ARGS += ['-j', jobs]
+endif
+  endif
+
   # This is a bit awkward but works: create a trivial document and
   # try to run it with our configuration file (which enforces a
   # version requirement). This will fail if sphinx-build is too old.
diff --git a/docs/sphinx/dbusdomain.py b/docs/sphinx/dbusdomain.py
index 2ea95af623..9872fd5bf6 100644
--- a/docs/sphinx/dbusdomain.py
+++ b/docs/sphinx/dbusdomain.py
@@ -400,6 +400,10 @@ def get_objects(self) -> Iterator[Tuple[str, str, str, 
str, str, int]]:
 for refname, obj in self.objects.items():
 yield (refname, refname, obj.objtype, obj.docname, obj.node_id, 1)
 
+def merge_domaindata(self, docnames, otherdata):
+for name, obj in otherdata['objects'].items():
+if obj.docname in docnames:
+self.data['objects'][name] = obj
 
 def setup(app):
 app.add_domain(DBusDomain)
diff --git a/docs/sphinx/fakedbusdoc.py b/docs/sphinx/fakedbusdoc.py
index d2c5079046..2d2e6ef640 100644
--- a/docs/sphinx/fakedbusdoc.py
+++ b/docs/sphinx/fakedbusdoc.py
@@ -23,3 +23,8 @@ def run(self):
 def setup(app: Sphinx) -> Dict[str, Any]:
 """Register a fake dbus-doc directive with Sphinx"""
 app.add_directive("dbus-doc", FakeDBusDocDirective)
+
+return dict(
+parallel_read_safe = True,
+parallel_write_safe = True
+)
diff --git a/docs/sphinx/qmp_lexer.py b/docs/sphinx/qmp_lexer.py
index f7e4c0e198..a59de8a079 100644
--- a/docs/sphinx/qmp_lexer.py
+++ b/docs/sphinx/qmp_lexer.py
@@ -41,3 +41,8 @@ def setup(sphinx):
 sphinx.add_lexer('QMP', QMPExampleLexer)
 except errors.VersionRequirementError:
 sphinx.add_lexer('QMP', QMPExampleLexer())
+
+return dict(
+parallel_read_safe = True,
+parallel_write_safe = True
+)
-- 
2.35.3

[PATCH v4 0/2] docs: Speedup docs build

[Fabiano Rosas]

We currently have two documentation targets to build:
- 'man' for the man pages;
- 'html' for the web page.

There are two bottlenecks in the process:

1) sphinx runs with a single process;
2) the two targets are serialized.

For (1), we can just add the "-j auto" to sphinx_build and that should
bring some speed gains.

For (2) it's a little trickier because the reason the builds are
serialized is that Sphinx keeps track of changed files, but we still
need a way to tell meson whether a target needs to re-execute, so we
added dependency tracking and timestamp checking for (only) the 'html'
build via the depfile.py extension. Since the sources for both builds
are the same, we made the 'man' build dependent on 'html' so that it
would rebuild when needed.

So patch 1 adds the -j option to Sphinx and patch 2 adds depfile
support to the 'man' build. We can now run the two in parallel (ninja
will take care of that).

On my 16 core machine,
the -j change saves about 20s
the depfile change saves about 10s

===
On master:
 $ ../configure --enable-docs ...
 $ time make -j$(nproc) html man
   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
 /usr/bin/ninja  build.ninja && touch build.ninja.stamp
 ninja: no work to do.
 /usr/bin/python3 -B .../qemu/meson/meson.py introspect --targets --tests 
--benchmarks | /usr/bin/python3 -B scripts/mtest2make.py > Makefile.mtest
   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
 [1/2] Generating docs/QEMU manual with a custom command
 [2/2] Generating docs/QEMU man pages with a custom command
 make: Nothing to be done for 'man'.

 real0m50.155s
 user0m49.759s
 sys 0m0.761s

 $ mv docs ../saved-docs

This series:
 $ ../configure --enable-docs ...
 $ time make -j$(nproc) html man
   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
 /usr/bin/ninja  build.ninja && touch build.ninja.stamp
 ninja: no work to do.
 /usr/bin/python3 -B .../qemu/meson/meson.py introspect --targets --tests 
--benchmarks | /usr/bin/python3 -B scripts/mtest2make.py > Makefile.mtest
   GIT ui/keycodemapdb meson tests/fp/berkeley-testfloat-3 
tests/fp/berkeley-softfloat-3 dtc
 [1/2] Generating docs/QEMU man pages with a custom command
 [2/2] Generating docs/QEMU manual with a custom command
 make: Nothing to be done for 'man'.

 real0m21.708s
 user1m12.317s
 sys 0m2.056s

Diff sanity check:
 $ diff -rq docs/ ../saved-docs/
 Only in ../saved-docs/: docs.d  # now manual.dep
 Only in ../saved-docs/: docs.stamp  # now manual.stamp
 Only in docs/: man.dep
 Only in docs/: man.p
 Only in docs/: man.stamp
 Only in docs/: manual.dep
 Only in docs/: manual.stamp
 Files docs/manual.p/about/build-platforms.doctree and 
../saved-docs/manual.p/about/build-platforms.doctree differ
 ...  # sphinx cache files, a bunch of these^

Rebuilding (here I show that a man file and an html file are
unchanged, change their source .rst and rebuild each target):

 $ ninja -d explain html
 ninja: no work to do.
 $ ninja -d explain man
 ninja: no work to do.
 $ man -l docs/qemu.1 | grep foobar
 $ grep foobar docs/manual/system/i386/pc.html
 $ vi ../docs/system/target-i386-desc.rst.inc#add the 'foobar' string

 $ ninja -d explain man
 ninja explain: restat of output docs/man.stamp older than most recent input 
docs/system/target-i386-desc.rst.inc (1683122999140339620 vs 
1683123032492362281)
 ninja explain: docs/man.stamp is dirty
 ninja explain: docs/qemu-block-drivers.7 is dirty
 ninja explain: docs/qemu-cpu-models.7 is dirty
 ninja explain: docs/qemu-ga-ref.7 is dirty
 ninja explain: docs/qemu-ga.8 is dirty
 ninja explain: docs/qemu-img.1 is dirty
 ninja explain: docs/qemu-nbd.8 is dirty
 ninja explain: docs/qemu-pr-helper.8 is dirty
 ninja explain: docs/qemu-qmp-ref.7 is dirty
 ninja explain: docs/qemu-storage-daemon-qmp-ref.7 is dirty
 ninja explain: docs/qemu-storage-daemon.1 is dirty
 ninja explain: docs/qemu-trace-stap.1 is dirty
 ninja explain: docs/qemu.1 is dirty
 ninja explain: docs/virtfs-proxy-helper.1 is dirty
 [1/1] Generating docs/QEMU man pages with a custom command

 $ man -l docs/qemu.1 | grep foobar
The QEMU PC System emulator simulates the following foobar peripherals:
 $ grep foobar docs/manual/system/i386/pc.html  #html files unchanged

 $ ninja -d explain html
 ninja explain: restat of output docs/manual.stamp older than most recent input 
docs/system/target-i386-desc.rst.inc (1683122995876337403 vs 
1683123032492362281)
 ninja explain: docs/manual.stamp is dirty
 [1/1] Generating docs/QEMU manual with a custom command

 $ man -l docs/qemu.1 | grep foobar
The QEMU PC System emulator simulates the following foobar peripherals:
 $ grep foobar docs/manual/system/i386/pc.html
 The QEMU PC System emulator simulates the following foobar peripherals:

===

Fabiano Rosas (2):
  meson: Pass -j option to sphinx
  meson: Deserialize the man 

[PATCH v4 2/2] meson: Deserialize the man pages and html builds

[Fabiano Rosas]

For the documentation builds (man pages & manual), we let Sphinx
decide when to rebuild and use a depfile to know when to trigger the
make target.

We currently use a trick of having the man pages custom_target take as
input the html pages custom_target object, which causes both targets
to be executed if one of the dependencies has changed. However, having
this at the custom_target level means that the two builds are
effectively serialized.

We can eliminate the dependency between the targets by adding a second
depfile for the man pages build, allowing them to be parallelized by
ninja while keeping sphinx in charge of deciding when to rebuild.

Since they can now run in parallel, separate the Sphinx cache
directory of the two builds. We need this not only for data
consistency but also because Sphinx writes builder-dependent
environment information to the cache directory (see notes under
smartquotes_excludes in sphinx docs [1]).

Note that after this patch the commands `make man` and `make html`
only build the specified target. To keep the old behavior of building
both targets, use `make man html` or `make sphinxdocs`.

1- https://www.sphinx-doc.org/en/master/usage/configuration.html

Signed-off-by: Fabiano Rosas 
---
 docs/meson.build | 36 
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/docs/meson.build b/docs/meson.build
index 6d0986579e..858e737431 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -42,7 +42,9 @@ if sphinx_build.found()
 endif
 
 if build_docs
-  SPHINX_ARGS += ['-Dversion=' + meson.project_version(), '-Drelease=' + 
get_option('pkgversion')]
+  SPHINX_ARGS += ['-Dversion=' + meson.project_version(),
+  '-Drelease=' + get_option('pkgversion'),
+  '-Ddepfile=@DEPFILE@', '-Ddepfile_stamp=@OUTPUT0@']
 
   man_pages = {
 'qemu-ga.8': (have_ga ? 'man8' : ''),
@@ -61,41 +63,43 @@ if build_docs
   }
 
   sphinxdocs = []
-  sphinxmans = []
 
   private_dir = meson.current_build_dir() / 'manual.p'
   output_dir = meson.current_build_dir() / 'manual'
   input_dir = meson.current_source_dir()
 
-  this_manual = custom_target('QEMU manual',
+  manual = custom_target('QEMU manual',
 build_by_default: build_docs,
-output: 'docs.stamp',
+output: 'manual.stamp',
 input: files('conf.py'),
-depfile: 'docs.d',
-command: [SPHINX_ARGS, '-Ddepfile=@DEPFILE@',
-  '-Ddepfile_stamp=@OUTPUT0@',
-  '-b', 'html', '-d', private_dir,
+depfile: 'manual.dep',
+command: [SPHINX_ARGS, '-b', 'html', '-d', private_dir,
   input_dir, output_dir])
-  sphinxdocs += this_manual
+  sphinxdocs += manual
   install_subdir(output_dir, install_dir: qemu_docdir, strip_directory: true)
 
-  these_man_pages = []
-  install_dirs = []
+  man_private_dir = meson.current_build_dir() / 'man.p'
+  # man.stamp is not installed
+  these_man_pages = ['man.stamp']
+  install_dirs = [false]
+
   foreach page, section : man_pages
 these_man_pages += page
 install_dirs += section == '' ? false : get_option('mandir') / section
   endforeach
 
-  sphinxmans += custom_target('QEMU man pages',
+
+  man_pages = custom_target('QEMU man pages',
   build_by_default: build_docs,
   output: these_man_pages,
-  input: this_manual,
+  depfile: 'man.dep',
   install: build_docs,
   install_dir: install_dirs,
-  command: [SPHINX_ARGS, '-b', 'man', '-d', 
private_dir,
+  command: [SPHINX_ARGS, '-b', 'man', '-d', 
man_private_dir,
 input_dir, meson.current_build_dir()])
+  sphinxdocs += man_pages
 
   alias_target('sphinxdocs', sphinxdocs)
-  alias_target('html', sphinxdocs)
-  alias_target('man', sphinxmans)
+  alias_target('html', manual)
+  alias_target('man', man_pages)
 endif
-- 
2.35.3

[RFC 1/1] migration: Update error description whenever migration fails

[tejus.gk]

There are places in the code where the migration is marked failed with
MIGRATION_STATUS_FAILED, but the failiure reason is never updated. Hence
libvirt doesn't know why the migration failed when it queries for it.

Signed-off-by: tejus.gk 
---
 migration/migration.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index feb5ab7493..0d7d34bf4d 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1665,8 +1665,11 @@ void qmp_migrate(const char *uri, bool has_blk, bool blk,
 }
 error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "uri",
"a valid migration protocol");
+error_setg(_err, QERR_INVALID_PARAMETER_VALUE, "uri",
+   "a valid migration protocol");
 migrate_set_state(>state, MIGRATION_STATUS_SETUP,
   MIGRATION_STATUS_FAILED);
+migrate_set_error(s, local_err);
 block_cleanup_parameters();
 return;
 }
@@ -2059,6 +2062,7 @@ static int postcopy_start(MigrationState *ms)
 int64_t bandwidth = migrate_max_postcopy_bandwidth();
 bool restart_block = false;
 int cur_state = MIGRATION_STATUS_ACTIVE;
+Error *local_err = NULL;
 
 if (migrate_postcopy_preempt()) {
 migration_wait_main_channel(ms);
@@ -2203,8 +2207,10 @@ static int postcopy_start(MigrationState *ms)
 ret = qemu_file_get_error(ms->to_dst_file);
 if (ret) {
 error_report("postcopy_start: Migration stream errored");
+error_setg(_err, "postcopy_start: Migration stream errored");
 migrate_set_state(>state, MIGRATION_STATUS_POSTCOPY_ACTIVE,
   MIGRATION_STATUS_FAILED);
+migrate_set_error(ms, local_err);
 }
 
 trace_postcopy_preempt_enabled(migrate_postcopy_preempt());
@@ -3233,7 +3239,9 @@ void migrate_fd_connect(MigrationState *s, Error 
*error_in)
 if (migrate_postcopy_ram() || migrate_return_path()) {
 if (open_return_path_on_source(s, !resume)) {
 error_report("Unable to open return-path for postcopy");
+error_setg(_err, "Unable to open return-path");
 migrate_set_state(>state, s->state, MIGRATION_STATUS_FAILED);
+migrate_set_error(s, local_err);
 migrate_fd_cleanup(s);
 return;
 }
-- 
2.22.3

[RFC 0/1] migration: Update error description whenever migration fails

[tejus.gk]

Hi everyone,

Currently, in QEMU, whenever a migration fails, its state is set to
MIGRATION_STATUS_FAILED
via the function migrate_set_state. However, there are places in the
code where the migration is marked as a failed migration; however, the
error description is never updated in the migration state object. This
causes problems when libvirt tries to query for the status of the
migration via a query-migrate; it never receives an error description
and hence reports the error as "unexpectedly failed". This
doesn't give us any information about what actually went wrong with
the migration.

An approach to solve this problem, which this patch explores, is to
update the migration errors through  migrate_set_error, whenever the
migration state is updated to MIGRATION_STATUS_FAILED. However,
sometimes these error descriptions can be due to various reasons or be
too vague.

An alternative approach to tackle this is to update the error
description from the point where the error actually occurred. For
instance, an error which occurs while saving the vmstate in the function
vmstate_save_state_v in the file migration/vmstate.c, results in a
failed migration, hence the error description can be updated here
itself, rather than updating it in the function migration_completion,
present in migration/migration.c.

tejus.gk (1):
  migration: Update error description whenever migration fails

 migration/migration.c | 8 
 1 file changed, 8 insertions(+)

-- 
2.22.3

Re: [PATCH 15/16] target/sparc: Remove TARGET_ALIGNED_ONLY

[Mark Cave-Ayland]

On 02/05/2023 17:08, Richard Henderson wrote:


Signed-off-by: Richard Henderson 
---
  configs/targets/sparc-linux-user.mak   | 1 -
  configs/targets/sparc-softmmu.mak  | 1 -
  configs/targets/sparc32plus-linux-user.mak | 1 -
  configs/targets/sparc64-linux-user.mak | 1 -
  configs/targets/sparc64-softmmu.mak| 1 -
  5 files changed, 5 deletions(-)

diff --git a/configs/targets/sparc-linux-user.mak 
b/configs/targets/sparc-linux-user.mak
index 00e7bc1f07..abcfb8fc62 100644
--- a/configs/targets/sparc-linux-user.mak
+++ b/configs/targets/sparc-linux-user.mak
@@ -1,5 +1,4 @@
  TARGET_ARCH=sparc
  TARGET_SYSTBL_ABI=common,32
  TARGET_SYSTBL=syscall.tbl
-TARGET_ALIGNED_ONLY=y
  TARGET_BIG_ENDIAN=y
diff --git a/configs/targets/sparc-softmmu.mak 
b/configs/targets/sparc-softmmu.mak
index a849190f01..454eb35499 100644
--- a/configs/targets/sparc-softmmu.mak
+++ b/configs/targets/sparc-softmmu.mak
@@ -1,3 +1,2 @@
  TARGET_ARCH=sparc
-TARGET_ALIGNED_ONLY=y
  TARGET_BIG_ENDIAN=y
diff --git a/configs/targets/sparc32plus-linux-user.mak 
b/configs/targets/sparc32plus-linux-user.mak
index a65c0951a1..6cc8fa516b 100644
--- a/configs/targets/sparc32plus-linux-user.mak
+++ b/configs/targets/sparc32plus-linux-user.mak
@@ -4,5 +4,4 @@ TARGET_BASE_ARCH=sparc
  TARGET_ABI_DIR=sparc
  TARGET_SYSTBL_ABI=common,32
  TARGET_SYSTBL=syscall.tbl
-TARGET_ALIGNED_ONLY=y
  TARGET_BIG_ENDIAN=y
diff --git a/configs/targets/sparc64-linux-user.mak 
b/configs/targets/sparc64-linux-user.mak
index 20fcb93fa4..52f05ec000 100644
--- a/configs/targets/sparc64-linux-user.mak
+++ b/configs/targets/sparc64-linux-user.mak
@@ -3,5 +3,4 @@ TARGET_BASE_ARCH=sparc
  TARGET_ABI_DIR=sparc
  TARGET_SYSTBL_ABI=common,64
  TARGET_SYSTBL=syscall.tbl
-TARGET_ALIGNED_ONLY=y
  TARGET_BIG_ENDIAN=y
diff --git a/configs/targets/sparc64-softmmu.mak 
b/configs/targets/sparc64-softmmu.mak
index c626ac3eae..d3f8a3b710 100644
--- a/configs/targets/sparc64-softmmu.mak
+++ b/configs/targets/sparc64-softmmu.mak
@@ -1,4 +1,3 @@
  TARGET_ARCH=sparc64
  TARGET_BASE_ARCH=sparc
-TARGET_ALIGNED_ONLY=y
  TARGET_BIG_ENDIAN=y


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 14/16] target/sparc: Use cpu_ld*_code_mmu

[Mark Cave-Ayland]

On 02/05/2023 17:08, Richard Henderson wrote:


This passes on the memop as given as argument to
helper_ld_asi to the ultimate load primitive.

Signed-off-by: Richard Henderson 
---
  target/sparc/ldst_helper.c | 10 ++
  1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/sparc/ldst_helper.c b/target/sparc/ldst_helper.c
index a53580d9e4..7972d56a72 100644
--- a/target/sparc/ldst_helper.c
+++ b/target/sparc/ldst_helper.c
@@ -593,6 +593,7 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
  #if defined(DEBUG_MXCC) || defined(DEBUG_ASI)
  uint32_t last_addr = addr;
  #endif
+MemOpIdx oi;
  
  do_check_align(env, addr, size - 1, GETPC());

  switch (asi) {
@@ -692,19 +693,20 @@ uint64_t helper_ld_asi(CPUSPARCState *env, target_ulong 
addr,
  case ASI_M_IODIAG:  /* Turbosparc IOTLB Diagnostic */
  break;
  case ASI_KERNELTXT: /* Supervisor code access */
+oi = make_memop_idx(memop, cpu_mmu_index(env, true));
  switch (size) {
  case 1:
-ret = cpu_ldub_code(env, addr);
+ret = cpu_ldb_code_mmu(env, addr, oi, GETPC());
  break;
  case 2:
-ret = cpu_lduw_code(env, addr);
+ret = cpu_ldw_code_mmu(env, addr, oi, GETPC());
  break;
  default:
  case 4:
-ret = cpu_ldl_code(env, addr);
+ret = cpu_ldl_code_mmu(env, addr, oi, GETPC());
  break;
  case 8:
-ret = cpu_ldq_code(env, addr);
+ret = cpu_ldq_code_mmu(env, addr, oi, GETPC());
  break;
  }
  break;


Reviewed-by: Mark Cave-Ayland 


ATB,

Mark.

Re: [PATCH 13/16] target/sparc: Use MO_ALIGN where required

[Mark Cave-Ayland]

On 02/05/2023 17:08, Richard Henderson wrote:


Signed-off-by: Richard Henderson 
---
  target/sparc/translate.c | 66 +---
  1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index bc71e44e66..414e014b11 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -1899,7 +1899,7 @@ static void gen_swap(DisasContext *dc, TCGv dst, TCGv src,
   TCGv addr, int mmu_idx, MemOp memop)
  {
  gen_address_mask(dc, addr);
-tcg_gen_atomic_xchg_tl(dst, addr, src, mmu_idx, memop);
+tcg_gen_atomic_xchg_tl(dst, addr, src, mmu_idx, memop | MO_ALIGN);
  }
  
  static void gen_ldstub(DisasContext *dc, TCGv dst, TCGv addr, int mmu_idx)

@@ -2155,12 +2155,12 @@ static void gen_ld_asi(DisasContext *dc, TCGv dst, TCGv 
addr,
  break;
  case GET_ASI_DIRECT:
  gen_address_mask(dc, addr);
-tcg_gen_qemu_ld_tl(dst, addr, da.mem_idx, da.memop);
+tcg_gen_qemu_ld_tl(dst, addr, da.mem_idx, da.memop | MO_ALIGN);
  break;
  default:
  {
  TCGv_i32 r_asi = tcg_constant_i32(da.asi);
-TCGv_i32 r_mop = tcg_constant_i32(memop);
+TCGv_i32 r_mop = tcg_constant_i32(memop | MO_ALIGN);
  
  save_state(dc);

  #ifdef TARGET_SPARC64
@@ -2201,7 +2201,7 @@ static void gen_st_asi(DisasContext *dc, TCGv src, TCGv 
addr,
  /* fall through */
  case GET_ASI_DIRECT:
  gen_address_mask(dc, addr);
-tcg_gen_qemu_st_tl(src, addr, da.mem_idx, da.memop);
+tcg_gen_qemu_st_tl(src, addr, da.mem_idx, da.memop | MO_ALIGN);
  break;
  #if !defined(TARGET_SPARC64) && !defined(CONFIG_USER_ONLY)
  case GET_ASI_BCOPY:
@@ -2233,7 +2233,7 @@ static void gen_st_asi(DisasContext *dc, TCGv src, TCGv 
addr,
  default:
  {
  TCGv_i32 r_asi = tcg_constant_i32(da.asi);
-TCGv_i32 r_mop = tcg_constant_i32(memop & MO_SIZE);
+TCGv_i32 r_mop = tcg_constant_i32(memop | MO_ALIGN);
  
  save_state(dc);

  #ifdef TARGET_SPARC64
@@ -2283,7 +2283,7 @@ static void gen_cas_asi(DisasContext *dc, TCGv addr, TCGv 
cmpv,
  case GET_ASI_DIRECT:
  oldv = tcg_temp_new();
  tcg_gen_atomic_cmpxchg_tl(oldv, addr, cmpv, gen_load_gpr(dc, rd),
-  da.mem_idx, da.memop);
+  da.mem_idx, da.memop | MO_ALIGN);
  gen_store_gpr(dc, rd, oldv);
  break;
  default:
@@ -2347,7 +2347,7 @@ static void gen_ldf_asi(DisasContext *dc, TCGv addr,
  switch (size) {
  case 4:
  d32 = gen_dest_fpr_F(dc);
-tcg_gen_qemu_ld_i32(d32, addr, da.mem_idx, da.memop);
+tcg_gen_qemu_ld_i32(d32, addr, da.mem_idx, da.memop | MO_ALIGN);
  gen_store_fpr_F(dc, rd, d32);
  break;
  case 8:
@@ -2397,7 +2397,8 @@ static void gen_ldf_asi(DisasContext *dc, TCGv addr,
  /* Valid for lddfa only.  */
  if (size == 8) {
  gen_address_mask(dc, addr);
-tcg_gen_qemu_ld_i64(cpu_fpr[rd / 2], addr, da.mem_idx, da.memop);
+tcg_gen_qemu_ld_i64(cpu_fpr[rd / 2], addr, da.mem_idx,
+da.memop | MO_ALIGN);
  } else {
  gen_exception(dc, TT_ILL_INSN);
  }
@@ -2406,7 +2407,7 @@ static void gen_ldf_asi(DisasContext *dc, TCGv addr,
  default:
  {
  TCGv_i32 r_asi = tcg_constant_i32(da.asi);
-TCGv_i32 r_mop = tcg_constant_i32(da.memop);
+TCGv_i32 r_mop = tcg_constant_i32(da.memop | MO_ALIGN);
  
  save_state(dc);

  /* According to the table in the UA2011 manual, the only
@@ -2454,7 +2455,7 @@ static void gen_stf_asi(DisasContext *dc, TCGv addr,
  switch (size) {
  case 4:
  d32 = gen_load_fpr_F(dc, rd);
-tcg_gen_qemu_st_i32(d32, addr, da.mem_idx, da.memop);
+tcg_gen_qemu_st_i32(d32, addr, da.mem_idx, da.memop | MO_ALIGN);
  break;
  case 8:
  tcg_gen_qemu_st_i64(cpu_fpr[rd / 2], addr, da.mem_idx,
@@ -2506,7 +2507,8 @@ static void gen_stf_asi(DisasContext *dc, TCGv addr,
  /* Valid for stdfa only.  */
  if (size == 8) {
  gen_address_mask(dc, addr);
-tcg_gen_qemu_st_i64(cpu_fpr[rd / 2], addr, da.mem_idx, da.memop);
+tcg_gen_qemu_st_i64(cpu_fpr[rd / 2], addr, da.mem_idx,
+da.memop | MO_ALIGN);
  } else {
  gen_exception(dc, TT_ILL_INSN);
  }
@@ -2543,7 +2545,7 @@ static void gen_ldda_asi(DisasContext *dc, TCGv addr, int 
insn, int rd)
  TCGv_i64 tmp = tcg_temp_new_i64();
  
  gen_address_mask(dc, addr);

-tcg_gen_qemu_ld_i64(tmp, addr, da.mem_idx, da.memop);
+tcg_gen_qemu_ld_i64(tmp, addr, da.mem_idx, da.memop | 

Re: [PATCH 11/13] hw/ide/sii3112: Reuse PCIIDEState::{cmd,data}_ops

[Mark Cave-Ayland]

On 27/04/2023 13:55, BALATON Zoltan wrote:


On Thu, 27 Apr 2023, Mark Cave-Ayland wrote:

On 27/04/2023 00:24, BALATON Zoltan wrote:

On Wed, 26 Apr 2023, Bernhard Beschow wrote:
Am 26. April 2023 11:41:54 UTC schrieb Mark Cave-Ayland 
:

On 22/04/2023 16:07, Bernhard Beschow wrote:


Allows to unexport pci_ide_{cmd,data}_le_ops and models TYPE_SII3112_PCI as a
standard-compliant PCI IDE device.

Signed-off-by: Bernhard Beschow 
---
  include/hw/ide/pci.h |  2 --
  hw/ide/pci.c |  4 ++--
  hw/ide/sii3112.c | 50 
  3 files changed, 20 insertions(+), 36 deletions(-)

diff --git a/include/hw/ide/pci.h b/include/hw/ide/pci.h
index 5025df5b82..dbb4b13161 100644
--- a/include/hw/ide/pci.h
+++ b/include/hw/ide/pci.h
@@ -62,6 +62,4 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val);
  extern MemoryRegionOps bmdma_addr_ioport_ops;
  void pci_ide_create_devs(PCIDevice *dev);
  -extern const MemoryRegionOps pci_ide_cmd_le_ops;
-extern const MemoryRegionOps pci_ide_data_le_ops;
  #endif
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index b2fcc00a64..97ccc75aa6 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -60,7 +60,7 @@ static void pci_ide_ctrl_write(void *opaque, hwaddr addr,
  ide_ctrl_write(bus, addr + 2, data);
  }
  -const MemoryRegionOps pci_ide_cmd_le_ops = {
+static const MemoryRegionOps pci_ide_cmd_le_ops = {
  .read = pci_ide_status_read,
  .write = pci_ide_ctrl_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
@@ -98,7 +98,7 @@ static void pci_ide_data_write(void *opaque, hwaddr addr,
  }
  }
  -const MemoryRegionOps pci_ide_data_le_ops = {
+static const MemoryRegionOps pci_ide_data_le_ops = {
  .read = pci_ide_data_read,
  .write = pci_ide_data_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
diff --git a/hw/ide/sii3112.c b/hw/ide/sii3112.c
index 0af897a9ef..9cf920369f 100644
--- a/hw/ide/sii3112.c
+++ b/hw/ide/sii3112.c
@@ -88,21 +88,9 @@ static uint64_t sii3112_reg_read(void *opaque, hwaddr addr,
  val |= (d->regs[1].confstat & (1UL << 11) ? (1 << 4) : 0);
  val |= (uint32_t)d->i.bmdma[1].status << 16;
  break;
-    case 0x80 ... 0x87:
-    val = pci_ide_data_le_ops.read(>i.bus[0], addr - 0x80, size);
-    break;
-    case 0x8a:
-    val = pci_ide_cmd_le_ops.read(>i.bus[0], 2, size);
-    break;
  case 0xa0:
  val = d->regs[0].confstat;
  break;
-    case 0xc0 ... 0xc7:
-    val = pci_ide_data_le_ops.read(>i.bus[1], addr - 0xc0, size);
-    break;
-    case 0xca:
-    val = pci_ide_cmd_le_ops.read(>i.bus[1], 2, size);
-    break;
  case 0xe0:
  val = d->regs[1].confstat;
  break;
@@ -171,18 +159,6 @@ static void sii3112_reg_write(void *opaque, hwaddr addr,
  case 0x0c ... 0x0f:
  bmdma_addr_ioport_ops.write(>i.bmdma[1], addr - 12, val, size);
  break;
-    case 0x80 ... 0x87:
-    pci_ide_data_le_ops.write(>i.bus[0], addr - 0x80, val, size);
-    break;
-    case 0x8a:
-    pci_ide_cmd_le_ops.write(>i.bus[0], 2, val, size);
-    break;
-    case 0xc0 ... 0xc7:
-    pci_ide_data_le_ops.write(>i.bus[1], addr - 0xc0, val, size);
-    break;
-    case 0xca:
-    pci_ide_cmd_le_ops.write(>i.bus[1], 2, val, size);
-    break;
  case 0x100:
  d->regs[0].scontrol = val & 0xfff;
  if (val & 1) {
@@ -259,6 +235,11 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
**errp)
  pci_config_set_interrupt_pin(dev->config, 1);
  pci_set_byte(dev->config + PCI_CACHE_LINE_SIZE, 8);
  +    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, >data_ops[0]);
+    pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, >cmd_ops[0]);
+    pci_register_bar(dev, 2, PCI_BASE_ADDRESS_SPACE_IO, >data_ops[1]);
+    pci_register_bar(dev, 3, PCI_BASE_ADDRESS_SPACE_IO, >cmd_ops[1]);
+
  /* BAR5 is in PCI memory space */
  memory_region_init_io(>mmio, OBJECT(d), _reg_ops, d,
   "sii3112.bar5", 0x200);
@@ -266,17 +247,22 @@ static void sii3112_pci_realize(PCIDevice *dev, Error 
**errp)

    /* BAR0-BAR4 are PCI I/O space aliases into BAR5 */
  mr = g_new(MemoryRegion, 1);
-    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar0", >mmio, 0x80, 8);
-    pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar0", >data_ops[0], 0,
+ memory_region_size(>data_ops[0]));
+    memory_region_add_subregion_overlap(>mmio, 0x80, mr, 1);
  mr = g_new(MemoryRegion, 1);
-    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar1", >mmio, 0x88, 4);
-    pci_register_bar(dev, 1, PCI_BASE_ADDRESS_SPACE_IO, mr);
+    memory_region_init_alias(mr, OBJECT(d), "sii3112.bar1", >cmd_ops[0], 0,
+ memory_region_size(>cmd_ops[0]));
+    memory_region_add_subregion_overlap(>mmio, 0x88, mr, 1);
  mr = g_new(MemoryRegion, 1);
-    

Re: [PATCH 7/9] target/sparc: Finish conversion to tcg_gen_qemu_{ld, st}_*

[Mark Cave-Ayland]

On 02/05/2023 14:57, Richard Henderson wrote:


Convert away from the old interface with the implicit
MemOp argument.

Signed-off-by: Richard Henderson 
---
  target/sparc/translate.c | 43 ++--
  1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 137bdc5159..bc71e44e66 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -5179,15 +5179,18 @@ static void disas_sparc_insn(DisasContext * dc, 
unsigned int insn)
  switch (xop) {
  case 0x0:   /* ld, V9 lduw, load unsigned word */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld32u(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_TEUL);
  break;
  case 0x1:   /* ldub, load unsigned byte */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld8u(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_UB);
  break;
  case 0x2:   /* lduh, load unsigned halfword */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld16u(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_TEUW);
  break;
  case 0x3:   /* ldd, load double word */
  if (rd & 1)
@@ -5197,7 +5200,8 @@ static void disas_sparc_insn(DisasContext * dc, unsigned 
int insn)
  
  gen_address_mask(dc, cpu_addr);

  t64 = tcg_temp_new_i64();
-tcg_gen_qemu_ld64(t64, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_i64(t64, cpu_addr,
+dc->mem_idx, MO_TEUQ);
  tcg_gen_trunc_i64_tl(cpu_val, t64);
  tcg_gen_ext32u_tl(cpu_val, cpu_val);
  gen_store_gpr(dc, rd + 1, cpu_val);
@@ -5208,11 +5212,12 @@ static void disas_sparc_insn(DisasContext * dc, 
unsigned int insn)
  break;
  case 0x9:   /* ldsb, load signed byte */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld8s(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr, dc->mem_idx, MO_SB);
  break;
  case 0xa:   /* ldsh, load signed halfword */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld16s(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_TESW);
  break;
  case 0xd:   /* ldstub */
  gen_ldstub(dc, cpu_val, cpu_addr, dc->mem_idx);
@@ -5266,11 +5271,13 @@ static void disas_sparc_insn(DisasContext * dc, 
unsigned int insn)
  #ifdef TARGET_SPARC64
  case 0x08: /* V9 ldsw */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld32s(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_TESL);
  break;
  case 0x0b: /* V9 ldx */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_ld64(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_ld_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_TEUQ);
  break;
  case 0x18: /* V9 ldswa */
  gen_ld_asi(dc, cpu_val, cpu_addr, insn, MO_TESL);
@@ -5369,15 +5376,17 @@ static void disas_sparc_insn(DisasContext * dc, 
unsigned int insn)
  switch (xop) {
  case 0x4: /* st, store word */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_st32(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_st_tl(cpu_val, cpu_addr,
+   dc->mem_idx, MO_TEUL);
  break;
  case 0x5: /* stb, store byte */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_st8(cpu_val, cpu_addr, dc->mem_idx);
+tcg_gen_qemu_st_tl(cpu_val, cpu_addr, dc->mem_idx, MO_UB);
  break;
  case 0x6: /* sth, store halfword */
  gen_address_mask(dc, cpu_addr);
-tcg_gen_qemu_st16(cpu_val, cpu_addr, dc->mem_idx);
+

Re: [PATCH 10/13] hw/ide/piix: Reuse PCIIDEState::{cmd,data}_ops

[Mark Cave-Ayland]

On 27/04/2023 19:15, Bernhard Beschow wrote:


Am 27. April 2023 10:52:17 UTC schrieb Mark Cave-Ayland 
:

On 26/04/2023 21:14, Bernhard Beschow wrote:


Am 26. April 2023 18:18:35 UTC schrieb Bernhard Beschow :



Am 26. April 2023 11:37:48 UTC schrieb Mark Cave-Ayland 
:

On 22/04/2023 16:07, Bernhard Beschow wrote:


Now that PCIIDEState::{cmd,data}_ops are initialized in the base class
constructor there is an opportunity for PIIX to reuse these attributes. This
resolves usage of ide_init_ioport() which would fall back internally to using
the isabus global due to NULL being passed as ISADevice by PIIX.

Signed-off-by: Bernhard Beschow 
---
hw/ide/piix.c | 30 +-
1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index a3a15dc7db..406a67fa0f 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -104,34 +104,32 @@ static void piix_ide_reset(DeviceState *dev)
pci_set_byte(pci_conf + 0x20, 0x01);  /* BMIBA: 20-23h */
}
-static bool pci_piix_init_bus(PCIIDEState *d, unsigned i, ISABus *isa_bus,
-  Error **errp)
+static void pci_piix_init_bus(PCIIDEState *d, unsigned i, ISABus *isa_bus)
{
static const struct {
int iobase;
int iobase2;
int isairq;
} port_info[] = {
-{0x1f0, 0x3f6, 14},
-{0x170, 0x376, 15},
+{0x1f0, 0x3f4, 14},
+{0x170, 0x374, 15},
};
-int ret;
+MemoryRegion *address_space_io = pci_address_space_io(PCI_DEVICE(d));
  ide_bus_init(>bus[i], sizeof(d->bus[i]), DEVICE(d), i, 2);
-ret = ide_init_ioport(>bus[i], NULL, port_info[i].iobase,
-  port_info[i].iobase2);
-if (ret) {
-error_setg_errno(errp, -ret, "Failed to realize %s port %u",
- object_get_typename(OBJECT(d)), i);
-return false;
-}
+memory_region_add_subregion(address_space_io, port_info[i].iobase,
+>data_ops[i]);
+/*
+ * PIIX forwards the last byte of cmd_ops to ISA. Model this using a low
+ * prio so competing memory regions take precedence.
+ */
+memory_region_add_subregion_overlap(address_space_io, port_info[i].iobase2,
+>cmd_ops[i], -1);


Interesting. Is this behaviour documented somewhere and/or used in one of your 
test images at all? If I'd have seen this myself, I probably thought that the 
addresses were a typo...


I first  stumbled upon this and wondered why this code was working with VIA_IDE (through 
my pc-via branch). Then I found the correct offsets there which are confirmed in the piix 
datasheet, e.g.: "Secondary Control Block Offset: 0374h"


In case you were wondering about the forwarding of the last byte the datasheet says: 
"Accesses to byte 3 of the Control Block are forwarded to ISA where the floppy disk 
controller responds."


Ahhh okay okay I see what's happening here: the PIIX IDE is assuming that the 
legacy ioport semantics are in operation here, which as you note above is where 
the FDC controller is also accessed via the above byte in the IDE control 
block. This is also why you need to change the address above from 0x3f6/0x376 
to 0x3f4/0x374 when trying to use the MemoryRegions used for the PCI BARs since 
the PCI IDE controller specification requires a 4 byte allocation for the 
Control Block - see sections 2.0 and 2.2.


Yes, PIIX assuming that might be the case. Why does it contradict the PCI IDE 
specification? PIIX seems to apply the apprppriate "workarounds" here.


Can you explain a bit more about where you see the contradiction? At first glance it 
looks okay to me.



And that's fine, because the portio_lists used in ide_init_ioport() set up the 
legacy IDE ioports so that FDC accesses done in this way can succeed, and the 
PIIX IDE is hard-coded to legacy mode. So in fact PIIX IDE should keep using 
ide_init_ioport() rather than trying to re-use the BAR MemoryRegions so I think 
this patch should just be dropped.


I was hoping to keep that patch...


Perhaps a different way to think about it is that from QEMU's perspective a BAR is a 
MemoryRegion that can be dynamically assigned/updated and cannot overlap, whereas the 
portio_list implementation also handles unaligned accesses and overlapping sparse 
accesses. Since the latter is the exact case here with the IDE/FDC then it seems the 
existing portio_list solution already does the "right thing" instead of having to 
manually emulate the overlapping dispatch.



ATB,

Mark.

[PATCH 3/3] tests/qtest: Don't run cdrom tests if no accelerator is present

[Fabiano Rosas]

On a build configured with: --disable-tcg --enable-xen it is possible
to produce a QEMU binary with no TCG nor KVM support. Skip the test if
that's the case.

Fixes: 0c1ae3ff9d ("tests/qtest: Fix tests when no KVM or TCG are present")
Signed-off-by: Fabiano Rosas 
---
 tests/qtest/cdrom-test.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 26a2400181..09655e6ff0 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -205,6 +205,11 @@ int main(int argc, char **argv)
 
 g_test_init(, , NULL);
 
+if (!qtest_has_accel("tcg") && !qtest_has_accel("kvm")) {
+g_test_skip("No KVM or TCG accelerator available");
+return 0;
+}
+
 if (exec_genisoimg(genisocheck)) {
 /* genisoimage not available - so can't run tests */
 return g_test_run();
-- 
2.35.3

[PATCH 2/3] target/arm: Add CONFIG_ARM_V7M back to default.mak

[Fabiano Rosas]

We cannot allow this config to be disabled at the moment as not all of
the relevant code is protected by it.

Commit 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a
KVM-only build") moved the CONFIGs of several boards to Kconfig, so it
is now possible that nothing selects ARM_V7M (e.g. when doing a
--without-default-devices build).

Return the CONFIG_ARM_V7M entry to default.mak while we don't enable
the compilation without it. Note that this goes against the intention
of commit cd43648a44 ("hw/arm: move CONFIG_V7M out of
default-devices"), but at this point this is the smallest change we
can do.

Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only 
build")
Signed-off-by: Fabiano Rosas 
---
 configs/devices/arm-softmmu/default.mak | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/devices/arm-softmmu/default.mak 
b/configs/devices/arm-softmmu/default.mak
index 647fbce88d..0c2b24d6bb 100644
--- a/configs/devices/arm-softmmu/default.mak
+++ b/configs/devices/arm-softmmu/default.mak
@@ -4,3 +4,4 @@
 # CONFIG_TEST_DEVICES=n
 
 CONFIG_ARM_VIRT=y
+CONFIG_ARM_V7M=y
-- 
2.35.3

[PATCH 1/3] target/arm: Use CONFIG_SEMIHOSTING instead of TCG for semihosting

[Fabiano Rosas]

When building --without-default-devices, the semihosting code will not
be available, so check the proper config.

Fixes: 29d9efca16 ("arm/Kconfig: Do not build TCG-only boards on a KVM-only 
build")
Signed-off-by: Fabiano Rosas 
---
 target/arm/helper.c   | 4 ++--
 target/arm/tcg/m_helper.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 2297626bfb..24bb7efb34 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10972,7 +10972,7 @@ static void arm_cpu_do_interrupt_aarch64(CPUState *cs)
  * We only see semihosting exceptions in TCG only as they are not
  * trapped to the hypervisor in KVM.
  */
-#ifdef CONFIG_TCG
+#ifdef CONFIG_SEMIHOSTING
 static void tcg_handle_semihosting(CPUState *cs)
 {
 ARMCPU *cpu = ARM_CPU(cs);
@@ -11033,7 +11033,7 @@ void arm_cpu_do_interrupt(CPUState *cs)
  * that caused the exception, not the target exception level, so
  * must be handled here.
  */
-#ifdef CONFIG_TCG
+#ifdef CONFIG_SEMIHOSTING
 if (cs->exception_index == EXCP_SEMIHOST) {
 tcg_handle_semihosting(cs);
 return;
diff --git a/target/arm/tcg/m_helper.c b/target/arm/tcg/m_helper.c
index 9758f225d6..4261f1bb1e 100644
--- a/target/arm/tcg/m_helper.c
+++ b/target/arm/tcg/m_helper.c
@@ -2345,7 +2345,7 @@ void arm_v7m_cpu_do_interrupt(CPUState *cs)
 qemu_log_mask(CPU_LOG_INT,
   "...handling as semihosting call 0x%x\n",
   env->regs[0]);
-#ifdef CONFIG_TCG
+#ifdef CONFIG_SEMIHOSTING
 do_common_semihosting(cs);
 #else
 g_assert_not_reached();
-- 
2.35.3

[PATCH 0/3] target/arm: disable-tcg and without-default-devices fixes

[Fabiano Rosas]

Here's the fix for the cdrom test failure that we discussed in the
list, plus 2 fixes for the ---without-default-devices build.

When I moved the boards CONFIGs from default.mak to Kconfig, it became
possible (due to --without-default-devices) to disable the CONFIGs for
all the boards that require ARM_V7M. That breaks the build because
ARM_V7M is required to be always set.

Fabiano Rosas (3):
  target/arm: Use CONFIG_SEMIHOSTING instead of TCG for semihosting
  target/arm: Add CONFIG_ARM_V7M back to default.mak
  tests/qtest: Don't run cdrom tests if no accelerator is present

 configs/devices/arm-softmmu/default.mak | 1 +
 target/arm/helper.c | 4 ++--
 target/arm/tcg/m_helper.c   | 2 +-
 tests/qtest/cdrom-test.c| 5 +
 4 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.35.3

Re: [PATCH] iotests/nbd-reconnect-on-open: Fix NBD socket path

[Vladimir Sementsov-Ogievskiy]

On 03.05.23 19:50, Kevin Wolf wrote:

Socket paths need to be short to avoid failures. This is why there is a
iotests.sock_dir (defaulting to /tmp) separate from the disk image base
directory.

Make use of it to fix failures in too deeply nested test directories.

Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e
Signed-off-by: Kevin Wolf



Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

Re: [PATCH] iotests/nbd-reconnect-on-open: Fix NBD socket path

[Eric Blake]

On Wed, May 03, 2023 at 06:50:19PM +0200, Kevin Wolf wrote:
> Socket paths need to be short to avoid failures. This is why there is a
> iotests.sock_dir (defaulting to /tmp) separate from the disk image base
> directory.
> 
> Make use of it to fix failures in too deeply nested test directories.
> 
> Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e
> Signed-off-by: Kevin Wolf 
> ---
>  tests/qemu-iotests/tests/nbd-reconnect-on-open | 3 ++-
>  tests/qemu-iotests/tests/nbd-reconnect-on-open.out | 4 ++--
>  2 files changed, 4 insertions(+), 3 deletions(-)

Reviewed-by: Eric Blake 

Just barely missed today's NBD pull request; I'll queue it for my next
one, or it can go in through your block tree if you beat me to it.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH] migration: Fix block_bitmap_mapping migration

[Vladimir Sementsov-Ogievskiy]

On 03.05.23 21:10, Juan Quintela wrote:

It is valid that params->has_block_bitmap_mapping is true and
params->block_bitmap_mapping is NULL.  So we can't use the trick of
having a single function.

Move to two functions one for each value and the tests are fixed.

Fixes: b804b35b1c8a0edfd127ac20819c234be55ac7fc
migration: Create migrate_block_bitmap_mapping() function

Reported-by: Kevin Wolf
Signed-off-by: Juan Quintela


Reviewed-by: Vladimir Sementsov-Ogievskiy 

--
Best regards,
Vladimir

[PULL 2/2] block/export: call blk_set_dev_ops(blk, NULL, NULL)

[Eric Blake]

From: Stefan Hajnoczi 

Most export types install BlockDeviceOps pointers. It is easy to forget
to remove them because that happens automatically via the "drive" qdev
property in hw/ but not block/export/.

Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so
the export types don't need to remember.

This fixes the nbd and vhost-user-blk export types.

Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the server")
Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize")
Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Eric Blake 
Message-Id: <2023050229.720647-1-stefa...@redhat.com>
Signed-off-by: Eric Blake 
---
 block/export/export.c| 2 ++
 block/export/vduse-blk.c | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/export/export.c b/block/export/export.c
index e3fee606116..62c7c22d45d 100644
--- a/block/export/export.c
+++ b/block/export/export.c
@@ -192,6 +192,7 @@ BlockExport *blk_exp_add(BlockExportOptions *export, Error 
**errp)
 return exp;

 fail:
+blk_set_dev_ops(exp->blk, NULL, NULL);
 blk_unref(blk);
 aio_context_release(ctx);
 if (exp) {
@@ -219,6 +220,7 @@ static void blk_exp_delete_bh(void *opaque)
 assert(exp->refcount == 0);
 QLIST_REMOVE(exp, next);
 exp->drv->delete(exp);
+blk_set_dev_ops(exp->blk, NULL, NULL);
 blk_unref(exp->blk);
 qapi_event_send_block_export_deleted(exp->id);
 g_free(exp->id);
diff --git a/block/export/vduse-blk.c b/block/export/vduse-blk.c
index f7ae44e3cea..b53ef39da02 100644
--- a/block/export/vduse-blk.c
+++ b/block/export/vduse-blk.c
@@ -346,7 +346,6 @@ static void vduse_blk_exp_delete(BlockExport *exp)

 blk_remove_aio_context_notifier(exp->blk, blk_aio_attached, blk_aio_detach,
 vblk_exp);
-blk_set_dev_ops(exp->blk, NULL, NULL);
 ret = vduse_dev_destroy(vblk_exp->dev);
 if (ret != -EBUSY) {
 unlink(vblk_exp->recon_file);
-- 
2.40.1

[PULL 1/2] systemd: Also clear LISTEN_FDNAMES during systemd socket activation

[Eric Blake]

Some time after systemd documented LISTEN_PID and LISTEN_FDS for
socket activation, they later added LISTEN_FDNAMES; now documented at:
https://www.freedesktop.org/software/systemd/man/sd_listen_fds.html

In particular, look at the implementation of sd_listen_fds_with_names():
https://github.com/systemd/systemd/blob/main/src/libsystemd/sd-daemon/sd-daemon.c

If we ever pass LISTEN_PID=xxx and LISTEN_FDS=n to a child process,
but leave LISTEN_FDNAMES=... unchanged as inherited from our parent
process, then our child process using sd_listen_fds_with_names() might
see a mismatch in the number of names (unexpected -EINVAL failure), or
even if the number of names matches the values of those names may be
unexpected (with even less predictable results).

Usually, this is not an issue - the point of LISTEN_PID is to tell
systemd socket activation to ignore all other LISTEN_* if they were
not directed to this particular pid.  But if we end up consuming a
socket directed to this qemu process, and later decide to spawn a
child process that also needs systemd socket activation, we must
ensure we are not leaking any stale systemd variables through to that
child.  The easiest way to do this is to wipe ALL LISTEN_* variables
at the time we consume a socket, even if we do not yet care about a
LISTEN_FDNAMES passed in from the parent process.

See also 
https://lists.freedesktop.org/archives/systemd-devel/2023-March/048920.html

Thanks: Laszlo Ersek 
Signed-off-by: Eric Blake 
Message-Id: <20230324153349.1123774-1-ebl...@redhat.com>
Reviewed-by: Daniel P. Berrangé 
---
 util/systemd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/systemd.c b/util/systemd.c
index 5bcac9b4016..ced518f771b 100644
--- a/util/systemd.c
+++ b/util/systemd.c
@@ -51,6 +51,7 @@ unsigned int check_socket_activation(void)
 /* So these are not passed to any child processes we might start. */
 unsetenv("LISTEN_FDS");
 unsetenv("LISTEN_PID");
+unsetenv("LISTEN_FDNAMES");

 /* So the file descriptors don't leak into child processes. */
 for (i = 0; i < nr_fds; ++i) {
-- 
2.40.1

[PULL 0/2] NBD pull request for 2023-05-03

[Eric Blake]

The following changes since commit 044f8cf70a2fdf3b9e4c4d849c66e7855d2c446a:

  Merge tag 'migration-20230428-pull-request' of 
https://gitlab.com/juan.quintela/qemu into staging (2023-05-03 10:29:30 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2023-05-03

for you to fetch changes up to de79b52604e43fdeba6cee4f5af600b62169f2d2:

  block/export: call blk_set_dev_ops(blk, NULL, NULL) (2023-05-03 14:00:08 
-0500)


nbd patches for 2023-05-03

- Eric Blake: clear LISTEN_FDNAMES when consuming systemd sockets
- Stefan Hajnoczi: clear export BlockDeviceOps in central location


Eric Blake (1):
  systemd: Also clear LISTEN_FDNAMES during systemd socket activation

Stefan Hajnoczi (1):
  block/export: call blk_set_dev_ops(blk, NULL, NULL)

 block/export/export.c| 2 ++
 block/export/vduse-blk.c | 1 -
 util/systemd.c   | 1 +
 3 files changed, 3 insertions(+), 1 deletion(-)

base-commit: 044f8cf70a2fdf3b9e4c4d849c66e7855d2c446a
-- 
2.40.1

Re: [PULL 11/18] migration: Create migrate_block_bitmap_mapping() function

[Vladimir Sementsov-Ogievskiy]

On 03.05.23 20:15, Juan Quintela wrote:

Kevin Wolf  wrote:

Am 27.04.2023 um 17:22 hat Juan Quintela geschrieben:

Notice that we changed the test of ->has_block_bitmap_mapping
for the test that block_bitmap_mapping is not NULL.

Signed-off-by: Juan Quintela 
Reviewed-by: Vladimir Sementsov-Ogievskiy 

---

Make it return const (vladimir)


(I don't think this part was actually meant for the commit message)


yeap.  My understandig has always been that this is the way to put
commenst for the email.


This commit broke qemu-iotests 300 on master. Please have a look.


 if (s->parameters.has_block_bitmap_mapping)
 return s->parameters.block_bitmap_mapping;

The test has a case where s->parameters.has_block_bitmap_mapping is true
but s->parameters.block_bitmap_mapping is false.

If that combination is right, then we need two functions becase the
asumtion that I did is false.

Vladimir?



Ah right. I forget that the field is list and for lists it's OK to be NULL with 
corresponding has_* = false, as that's the only way to distinguish specified 
empty list and unspecified list.


--
Best regards,
Vladimir

[PATCH v1] vhost-user: Remove acpi-specific memslot limit

[David Hildenbrand]

Let's just support 512 memslots on x86-64 and aarch64 as well. The maximum
number of ACPI slots (256) is no longer completely expressive ever since
we supported virtio-based memory devices. Further, we're completely
ignoring other memslots used outside of memory device context, such as
memslots used for boot memory.

Note that the vhost memslot limit in the kernel is usually configured to
be 509. With this change, we prepare vhost-user on the QEMU side to be
closer to that limit, to eventually support ~512 memslots in most vhost
implementations and have less "surprises" when cold/hotplugging vhost
devices while also consuming more memslots than we're currently used to
by memory devices (e.g., once virtio-mem starts using multiple memslots).

Note that most vhost-user implementations only support a small number of
memslots so far, which we can hopefully improve in the near future.

We'll leave the PPC special-case as is for now.

Cc: "Michael S. Tsirkin" 
Cc: Igor Mammedov 
Cc: Stefan Hajnoczi 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/vhost-user.c | 12 +---
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e5285df4ba..378bd44718 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -42,17 +42,7 @@
 #define VHOST_USER_F_PROTOCOL_FEATURES 30
 #define VHOST_USER_BACKEND_MAX_FDS 8
 
-/*
- * Set maximum number of RAM slots supported to
- * the maximum number supported by the target
- * hardware plaform.
- */
-#if defined(TARGET_X86) || defined(TARGET_X86_64) || \
-defined(TARGET_ARM) || defined(TARGET_AARCH64)
-#include "hw/acpi/acpi.h"
-#define VHOST_USER_MAX_RAM_SLOTS ACPI_MAX_RAM_SLOTS
-
-#elif defined(TARGET_PPC) || defined(TARGET_PPC64)
+#if defined(TARGET_PPC) || defined(TARGET_PPC64)
 #include "hw/ppc/spapr.h"
 #define VHOST_USER_MAX_RAM_SLOTS SPAPR_MAX_RAM_SLOTS
 
-- 
2.40.0

Re: [PATCH 05/22] hw/arm: Select VIRTIO_NET for virt machine

[Peter Maydell]

On Wed, 3 May 2023 at 18:06, Paolo Bonzini  wrote:
>
> On 5/3/23 17:35, Alex Bennée wrote:
> >> You should select a device only if you cannot even start
> >> the machine without --nodefaults.
> >
> > Which is the case here right? We could skip tests that explicitly
> > instantiate a device but these are tests failing with default devices
> > the machine tries to instantiate.
>
> I'm sorry, I meant "select" directives are needed if you cannot even
> start the machine *with* --nodefaults.
>
> Devices that are added *without* --nodefaults should use "imply"
> directives instead, as is already the case.

Do we really want to build a QEMU that then barfs unless
you pass -nodefaults, though ? That doesn't seem very useful.
Something somewhere ought to be saying "if you want the
virt board then you almost certainly want these". Or
alternatively we should fall back to "don't create a
network device we don't have", maybe ?

-- PMM

[PATCH v1] virtio-mem: Default to "unplugged-inaccessible=on" with 8.1 on x86-64

[David Hildenbrand]

Allowing guests to read unplugged memory simplified the bring-up of
virtio-mem in Linux guests -- which was limited to x86-64 only. On arm64
(which was added later), we never had legacy guests and don't even allow
to configure it, essentially always having "unplugged-inaccessible=on".

At this point, all guests we care about
should be supporting VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE, so let's
change the default for the 8.1 machine.

This change implies that also memory that supports the shared zeropage
(private anonymous memory) will now require
VIRTIO_MEM_F_UNPLUGGED_INACCESSIBLE in the driver in order to be usable by
the guest -- as default, one can still manually set the
unplugged-inaccessible property.

Disallowing the guest to read unplugged memory will be important for
some future features, such as memslot optimizations or protection of
unplugged memory, whereby we'll actually no longer allow the guest to
even read from unplugged memory.

At some point, we might want to deprecate and remove that property.

Cc: "Michael S. Tsirkin" 
Cc: Marcel Apfelbaum 
Cc: Paolo Bonzini 
Cc: Richard Henderson 
Cc: Eduardo Habkost 

Signed-off-by: David Hildenbrand 
---
 hw/i386/pc.c   | 4 +++-
 hw/virtio/virtio-mem.c | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d761c8c775..7802dc21d9 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -116,7 +116,9 @@
 { "qemu64-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },\
 { "athlon-" TYPE_X86_CPU, "model-id", "QEMU Virtual CPU version " v, },
 
-GlobalProperty pc_compat_8_0[] = {};
+GlobalProperty pc_compat_8_0[] = {
+{ "virtio-mem", "unplugged-inaccessible", "auto" },
+};
 const size_t pc_compat_8_0_len = G_N_ELEMENTS(pc_compat_8_0);
 
 GlobalProperty pc_compat_7_2[] = {
diff --git a/hw/virtio/virtio-mem.c b/hw/virtio/virtio-mem.c
index 957fe77dc0..538b695c29 100644
--- a/hw/virtio/virtio-mem.c
+++ b/hw/virtio/virtio-mem.c
@@ -1341,7 +1341,7 @@ static Property virtio_mem_properties[] = {
  TYPE_MEMORY_BACKEND, HostMemoryBackend *),
 #if defined(VIRTIO_MEM_HAS_LEGACY_GUESTS)
 DEFINE_PROP_ON_OFF_AUTO(VIRTIO_MEM_UNPLUGGED_INACCESSIBLE_PROP, VirtIOMEM,
-unplugged_inaccessible, ON_OFF_AUTO_AUTO),
+unplugged_inaccessible, ON_OFF_AUTO_ON),
 #endif
 DEFINE_PROP_BOOL(VIRTIO_MEM_EARLY_MIGRATION_PROP, VirtIOMEM,
  early_migration, true),
-- 
2.40.0

[PATCH] migration: Fix block_bitmap_mapping migration

[Juan Quintela]

It is valid that params->has_block_bitmap_mapping is true and
params->block_bitmap_mapping is NULL.  So we can't use the trick of
having a single function.

Move to two functions one for each value and the tests are fixed.

Fixes: b804b35b1c8a0edfd127ac20819c234be55ac7fc
   migration: Create migrate_block_bitmap_mapping() function

Reported-by: Kevin Wolf 
Signed-off-by: Juan Quintela 
---
 migration/block-dirty-bitmap.c | 14 +-
 migration/options.c|  7 +++
 migration/options.h|  2 ++
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/migration/block-dirty-bitmap.c b/migration/block-dirty-bitmap.c
index 6624f39bc6..20f36e6bd8 100644
--- a/migration/block-dirty-bitmap.c
+++ b/migration/block-dirty-bitmap.c
@@ -606,11 +606,9 @@ static int init_dirty_bitmap_migration(DBMSaveState *s)
 GHashTable *handled_by_blk = g_hash_table_new(NULL, NULL);
 BlockBackend *blk;
 GHashTable *alias_map = NULL;
-const BitmapMigrationNodeAliasList *block_bitmap_mapping =
-migrate_block_bitmap_mapping();
 
-if (block_bitmap_mapping) {
-alias_map = construct_alias_map(block_bitmap_mapping, true,
+if (migrate_has_block_bitmap_mapping()) {
+alias_map = construct_alias_map(migrate_block_bitmap_mapping(), true,
 _abort);
 }
 
@@ -1159,8 +1157,6 @@ static int dirty_bitmap_load_header(QEMUFile *f, 
DBMLoadState *s,
 static int dirty_bitmap_load(QEMUFile *f, void *opaque, int version_id)
 {
 GHashTable *alias_map = NULL;
-const BitmapMigrationNodeAliasList *block_bitmap_mapping =
-migrate_block_bitmap_mapping();
 DBMLoadState *s = &((DBMState *)opaque)->load;
 int ret = 0;
 
@@ -1172,9 +1168,9 @@ static int dirty_bitmap_load(QEMUFile *f, void *opaque, 
int version_id)
 return -EINVAL;
 }
 
-if (block_bitmap_mapping) {
-alias_map = construct_alias_map(block_bitmap_mapping,
-false, _abort);
+if (migrate_has_block_bitmap_mapping()) {
+alias_map = construct_alias_map(migrate_block_bitmap_mapping(), false,
+_abort);
 }
 
 do {
diff --git a/migration/options.c b/migration/options.c
index 53b7fc5d5d..7395787960 100644
--- a/migration/options.c
+++ b/migration/options.c
@@ -626,6 +626,13 @@ const BitmapMigrationNodeAliasList 
*migrate_block_bitmap_mapping(void)
 return s->parameters.block_bitmap_mapping;
 }
 
+bool migrate_has_block_bitmap_mapping(void)
+{
+MigrationState *s = migrate_get_current();
+
+return s->parameters.has_block_bitmap_mapping;
+}
+
 bool migrate_block_incremental(void)
 {
 MigrationState *s = migrate_get_current();
diff --git a/migration/options.h b/migration/options.h
index 3c322867cd..09841d6a63 100644
--- a/migration/options.h
+++ b/migration/options.h
@@ -71,6 +71,8 @@ bool migrate_cap_set(int cap, bool value, Error **errp);
 /* parameters */
 
 const BitmapMigrationNodeAliasList *migrate_block_bitmap_mapping(void);
+bool migrate_has_block_bitmap_mapping(void);
+
 bool migrate_block_incremental(void);
 uint32_t migrate_checkpoint_delay(void);
 int migrate_compress_level(void);
-- 
2.40.0

[PATCH] docs: clarify --without-default-devices

[Paolo Bonzini]

--without-default-devices is a specialized option that should only be used
when configs/devices/ is changed manually.

Explain the model towards which we should tend, with respect to failures
to start guests and to run "make check".

Signed-off-by: Paolo Bonzini 
---
 docs/devel/kconfig.rst | 16 +---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/docs/devel/kconfig.rst b/docs/devel/kconfig.rst
index a68fbc142efc..01984ae9fa9c 100644
--- a/docs/devel/kconfig.rst
+++ b/docs/devel/kconfig.rst
@@ -281,9 +281,19 @@ want to change some lines in the first group, for example 
like this::
CONFIG_PCI_DEVICES=y
#CONFIG_TEST_DEVICES=n
 
-and/or pick a subset of the devices in those device groups.  Right now
-there is no single place that lists all the optional devices for
-``CONFIG_PCI_DEVICES`` and ``CONFIG_TEST_DEVICES``.  In the future,
+and/or pick a subset of the devices in those device groups.  Without
+further modifications to ``configs/devices/``, a system emulator built
+without default devices might not do much more than start an empty
+machine, and even then only if ``--nodefaults`` is specified on the
+command line.  Starting a VM *without* ``--nodefaults`` is allowed to
+fail, but should never abort.  Failures in ``make check`` with
+``--without-default-devices`` are considered bugs in the test code:
+the tests should either use ``--nodefaults``, and should be skipped
+if a necessary device is not present in the build.  Such failures
+should not be worked around with ``select`` directives.
+
+Right now there is no single place that lists all the optional devices
+for ``CONFIG_PCI_DEVICES`` and ``CONFIG_TEST_DEVICES``.  In the future,
 we expect that ``.mak`` files will be automatically generated, so that
 they will include all these symbols and some help text on what they do.
 
-- 
2.40.0

Re: [PATCH 08/22] hw/arm: Select GICV3_TCG for sbsa-ref machine

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

From: Fabiano Rosas

This machine hardcodes the creation of the interrupt controller, so
make sure the dependency is explicitly described in the Kconfig.


gicv3_init_cpuif is used unconditionally by arm_gic_realize in 
hw/intc/arm_gicv3.c, so right now the separation between GICV3 and 
GICV3_TCG is useless.


That said, using "default y" is wrong.  The correct definition in 
hw/intc/Kconfig for these should be:


config ARM_GIC
bool
select MSI_NONBROKEN
select ARM_GICV3_TCG # if TCG
select ARM_GIC_KVM if KVM

config ARM_GICV3_TCG
bool

config ARM_GIC_KVM
bool

(where the "if TCG" is incorrect right now as explained above).

Paolo


Signed-off-by: Fabiano Rosas
Reviewed-by: Peter Maydell
Signed-off-by: Alex Bennée
Message-Id:<20230208192654.8854-9-faro...@suse.de>
---
  hw/arm/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index cec7898bb7..0cce0b8d5b 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -252,6 +252,7 @@ config SBSA_REF
  select PL061 # GPIO
  select USB_EHCI_SYSBUS
  select WDT_SBSA
+select ARM_GICV3_TCG
  

Re: [PATCH 06/22] hw/arm: Select VIRTIO_BLK for virt machine

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

From: Fabiano Rosas

The virt machine has IF_VIRTIO as block_default_type, which causes the
generic code to try to create a virtio-blk-pci device pair at
configure_blockdev()/qemu_create_cli_devices().

Select VIRTIO_BLK and VIRTIO_PCI from CONFIG_ARM_VIRT to avoid errors
when PCI_DEVICES=n (due to e.g. --without-default-devices):

$ ./qemu-system-aarch64 -M virt -accel tcg -cpu max -nodefaults -cdrom foo.qcow2
qemu-system-aarch64: -cdrom foo.qcow2: 'virtio-blk' (alias
'virtio-blk-pci') is not a valid device model name


This is also not needed.  It doesn't matter what's the default - it's 
possible to create a machine without block devices at all, therefore 
VIRTIO_BLK should not be selected.


Paolo

[PATCH v3 2/3] vhost: Remove vhost_backend_can_merge() callback

[David Hildenbrand]

Checking whether the memory regions are equal is sufficient: if they are
equal, then most certainly the contained fd is equal.

The whole vhost-user memslot handling is suboptimal and overly
complicated. We shouldn't have to lookup a RAM memory regions we got
notified about in vhost_user_get_mr_data() using a host pointer. But that
requires a bigger rework -- especially an alternative vhost_set_mem_table()
backend call that simply consumes MemoryRegionSections.

For now, let's just drop vhost_backend_can_merge().

Acked-by: Stefan Hajnoczi 
Reviewed-by: Igor Mammedov 
Acked-by: Igor Mammedov 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/vhost-user.c| 14 --
 hw/virtio/vhost-vdpa.c|  1 -
 hw/virtio/vhost.c |  6 +-
 include/hw/virtio/vhost-backend.h |  4 
 4 files changed, 1 insertion(+), 24 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index 0c3e2702b1..831375a967 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2195,19 +2195,6 @@ static int vhost_user_migration_done(struct vhost_dev 
*dev, char* mac_addr)
 return -ENOTSUP;
 }
 
-static bool vhost_user_can_merge(struct vhost_dev *dev,
- uint64_t start1, uint64_t size1,
- uint64_t start2, uint64_t size2)
-{
-ram_addr_t offset;
-int mfd, rfd;
-
-(void)vhost_user_get_mr_data(start1, , );
-(void)vhost_user_get_mr_data(start2, , );
-
-return mfd == rfd;
-}
-
 static int vhost_user_net_set_mtu(struct vhost_dev *dev, uint16_t mtu)
 {
 VhostUserMsg msg;
@@ -2704,7 +2691,6 @@ const VhostOps user_ops = {
 .vhost_set_vring_enable = vhost_user_set_vring_enable,
 .vhost_requires_shm_log = vhost_user_requires_shm_log,
 .vhost_migration_done = vhost_user_migration_done,
-.vhost_backend_can_merge = vhost_user_can_merge,
 .vhost_net_set_mtu = vhost_user_net_set_mtu,
 .vhost_set_iotlb_callback = vhost_user_set_iotlb_callback,
 .vhost_send_device_iotlb_msg = vhost_user_send_device_iotlb_msg,
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index bc6bad23d5..38d98528e7 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -1355,7 +1355,6 @@ const VhostOps vdpa_ops = {
 .vhost_set_config = vhost_vdpa_set_config,
 .vhost_requires_shm_log = NULL,
 .vhost_migration_done = NULL,
-.vhost_backend_can_merge = NULL,
 .vhost_net_set_mtu = NULL,
 .vhost_set_iotlb_callback = NULL,
 .vhost_send_device_iotlb_msg = NULL,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 4fe08c809f..6148892798 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -729,11 +729,7 @@ static void vhost_region_add_section(struct vhost_dev *dev,
 size_t offset = mrs_gpa - prev_gpa_start;
 
 if (prev_host_start + offset == mrs_host &&
-section->mr == prev_sec->mr &&
-(!dev->vhost_ops->vhost_backend_can_merge ||
- dev->vhost_ops->vhost_backend_can_merge(dev,
-mrs_host, mrs_size,
-prev_host_start, prev_size))) {
+section->mr == prev_sec->mr) {
 uint64_t max_end = MAX(prev_host_end, mrs_host + mrs_size);
 need_add = false;
 prev_sec->offset_within_address_space =
diff --git a/include/hw/virtio/vhost-backend.h 
b/include/hw/virtio/vhost-backend.h
index 2349a4a7d2..f3ba7b676b 100644
--- a/include/hw/virtio/vhost-backend.h
+++ b/include/hw/virtio/vhost-backend.h
@@ -86,9 +86,6 @@ typedef int (*vhost_set_vring_enable_op)(struct vhost_dev 
*dev,
 typedef bool (*vhost_requires_shm_log_op)(struct vhost_dev *dev);
 typedef int (*vhost_migration_done_op)(struct vhost_dev *dev,
char *mac_addr);
-typedef bool (*vhost_backend_can_merge_op)(struct vhost_dev *dev,
-   uint64_t start1, uint64_t size1,
-   uint64_t start2, uint64_t size2);
 typedef int (*vhost_vsock_set_guest_cid_op)(struct vhost_dev *dev,
 uint64_t guest_cid);
 typedef int (*vhost_vsock_set_running_op)(struct vhost_dev *dev, int start);
@@ -163,7 +160,6 @@ typedef struct VhostOps {
 vhost_set_vring_enable_op vhost_set_vring_enable;
 vhost_requires_shm_log_op vhost_requires_shm_log;
 vhost_migration_done_op vhost_migration_done;
-vhost_backend_can_merge_op vhost_backend_can_merge;
 vhost_vsock_set_guest_cid_op vhost_vsock_set_guest_cid;
 vhost_vsock_set_running_op vhost_vsock_set_running;
 vhost_set_iotlb_callback_op vhost_set_iotlb_callback;
-- 
2.40.0

[PATCH v3 1/3] vhost: Rework memslot filtering and fix "used_memslot" tracking

[David Hildenbrand]

Having multiple vhost devices, some filtering out fd-less memslots and
some not, can mess up the "used_memslot" accounting. Consequently our
"free memslot" checks become unreliable and we might run out of free
memslots at runtime later.

An example sequence which can trigger a potential issue that involves
different vhost backends (vhost-kernel and vhost-user) and hotplugged
memory devices can be found at [1].

Let's make the filtering mechanism less generic and distinguish between
backends that support private memslots (without a fd) and ones that only
support shared memslots (with a fd). Track the used_memslots for both
cases separately and use the corresponding value when required.

Note: Most probably we should filter out MAP_PRIVATE fd-based RAM regions
(for example, via memory-backend-memfd,...,shared=off or as default with
 memory-backend-file) as well. When not using MAP_SHARED, it might not work
as expected. Add a TODO for now.

[1] https://lkml.kernel.org/r/fad9136f-08d3-3fd9-71a1-502069c00...@redhat.com

Fixes: 988a27754bbb ("vhost: allow backends to filter memory sections")
Cc: Tiwei Bie 
Acked-by: Igor Mammedov 
Signed-off-by: David Hildenbrand 
---
 hw/virtio/vhost-user.c|  7 ++--
 hw/virtio/vhost.c | 56 ++-
 include/hw/virtio/vhost-backend.h |  5 ++-
 3 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
index e5285df4ba..0c3e2702b1 100644
--- a/hw/virtio/vhost-user.c
+++ b/hw/virtio/vhost-user.c
@@ -2453,10 +2453,9 @@ vhost_user_crypto_close_session(struct vhost_dev *dev, 
uint64_t session_id)
 return 0;
 }
 
-static bool vhost_user_mem_section_filter(struct vhost_dev *dev,
-  MemoryRegionSection *section)
+static bool vhost_user_no_private_memslots(struct vhost_dev *dev)
 {
-return memory_region_get_fd(section->mr) >= 0;
+return true;
 }
 
 static int vhost_user_get_inflight_fd(struct vhost_dev *dev,
@@ -2686,6 +2685,7 @@ const VhostOps user_ops = {
 .vhost_backend_init = vhost_user_backend_init,
 .vhost_backend_cleanup = vhost_user_backend_cleanup,
 .vhost_backend_memslots_limit = vhost_user_memslots_limit,
+.vhost_backend_no_private_memslots = vhost_user_no_private_memslots,
 .vhost_set_log_base = vhost_user_set_log_base,
 .vhost_set_mem_table = vhost_user_set_mem_table,
 .vhost_set_vring_addr = vhost_user_set_vring_addr,
@@ -2712,7 +2712,6 @@ const VhostOps user_ops = {
 .vhost_set_config = vhost_user_set_config,
 .vhost_crypto_create_session = vhost_user_crypto_create_session,
 .vhost_crypto_close_session = vhost_user_crypto_close_session,
-.vhost_backend_mem_section_filter = vhost_user_mem_section_filter,
 .vhost_get_inflight_fd = vhost_user_get_inflight_fd,
 .vhost_set_inflight_fd = vhost_user_set_inflight_fd,
 .vhost_dev_start = vhost_user_dev_start,
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 746d130c74..4fe08c809f 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -46,20 +46,33 @@
 static struct vhost_log *vhost_log;
 static struct vhost_log *vhost_log_shm;
 
+/* Memslots used by backends that support private memslots (without an fd). */
 static unsigned int used_memslots;
+
+/* Memslots used by backends that only support shared memslots (with an fd). */
+static unsigned int used_shared_memslots;
+
 static QLIST_HEAD(, vhost_dev) vhost_devices =
 QLIST_HEAD_INITIALIZER(vhost_devices);
 
 bool vhost_has_free_slot(void)
 {
-unsigned int slots_limit = ~0U;
+unsigned int free = UINT_MAX;
 struct vhost_dev *hdev;
 
 QLIST_FOREACH(hdev, _devices, entry) {
 unsigned int r = hdev->vhost_ops->vhost_backend_memslots_limit(hdev);
-slots_limit = MIN(slots_limit, r);
+unsigned int cur_free;
+
+if (hdev->vhost_ops->vhost_backend_no_private_memslots &&
+hdev->vhost_ops->vhost_backend_no_private_memslots(hdev)) {
+cur_free = r - used_shared_memslots;
+} else {
+cur_free = r - used_memslots;
+}
+free = MIN(free, cur_free);
 }
-return slots_limit > used_memslots;
+return free > 1;
 }
 
 static void vhost_dev_sync_region(struct vhost_dev *dev,
@@ -475,8 +488,7 @@ static int vhost_verify_ring_mappings(struct vhost_dev *dev,
  * vhost_section: identify sections needed for vhost access
  *
  * We only care about RAM sections here (where virtqueue and guest
- * internals accessed by virtio might live). If we find one we still
- * allow the backend to potentially filter it out of our list.
+ * internals accessed by virtio might live).
  */
 static bool vhost_section(struct vhost_dev *dev, MemoryRegionSection *section)
 {
@@ -503,8 +515,16 @@ static bool vhost_section(struct vhost_dev *dev, 
MemoryRegionSection *section)
 return false;
 }
 
-if 

[PATCH v3 3/3] softmmu/physmem: Fixup qemu_ram_block_from_host() documentation

[David Hildenbrand]

Let's fixup the documentation (e.g., removing traces of the ram_addr
parameter that no longer exists) and move it to the header file while at
it.

Suggested-by: Igor Mammedov 
Acked-by: Igor Mammedov 
Signed-off-by: David Hildenbrand 
---
 include/exec/cpu-common.h | 15 +++
 softmmu/physmem.c | 17 -
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 1be4a3117e..cd23a7535e 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -75,6 +75,21 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length);
 ram_addr_t qemu_ram_addr_from_host(void *ptr);
 ram_addr_t qemu_ram_addr_from_host_nofail(void *ptr);
 RAMBlock *qemu_ram_block_by_name(const char *name);
+
+/*
+ * Translates a host ptr back to a RAMBlock and an offset in that RAMBlock.
+ *
+ * @ptr: The host pointer to transalte.
+ * @round_offset: Whether to round the result offset down to a target page
+ * @offset: Will be set to the offset within the returned RAMBlock.
+ *
+ * Returns: RAMBlock (or NULL if not found)
+ *
+ * By the time this function returns, the returned pointer is not protected
+ * by RCU anymore.  If the caller is not within an RCU critical section and
+ * does not hold the iothread lock, it must have other means of protecting the
+ * pointer, such as a reference to the memory region that owns the RAMBlock.
+ */
 RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
ram_addr_t *offset);
 ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void *host);
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index 0e0182d9f2..f5b0fa5b17 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -2169,23 +2169,6 @@ ram_addr_t qemu_ram_block_host_offset(RAMBlock *rb, void 
*host)
 return res;
 }
 
-/*
- * Translates a host ptr back to a RAMBlock, a ram_addr and an offset
- * in that RAMBlock.
- *
- * ptr: Host pointer to look up
- * round_offset: If true round the result offset down to a page boundary
- * *ram_addr: set to result ram_addr
- * *offset: set to result offset within the RAMBlock
- *
- * Returns: RAMBlock (or NULL if not found)
- *
- * By the time this function returns, the returned pointer is not protected
- * by RCU anymore.  If the caller is not within an RCU critical section and
- * does not hold the iothread lock, it must have other means of protecting the
- * pointer, such as a reference to the region that includes the incoming
- * ram_addr_t.
- */
 RAMBlock *qemu_ram_block_from_host(void *ptr, bool round_offset,
ram_addr_t *offset)
 {
-- 
2.40.0

Re: [PATCH 17/22] hw/xtensa: add VIRTIO as dependencies for XTENSA_VIRT

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

These are needed for board creation so fail under "make check" with a
--without-default-devices build.

Signed-off-by: Alex Bennée
---
  hw/xtensa/Kconfig | 2 ++
  1 file changed, 2 insertions(+)

diff --git a/hw/xtensa/Kconfig b/hw/xtensa/Kconfig
index 0740657ea5..a54a9d395e 100644
--- a/hw/xtensa/Kconfig
+++ b/hw/xtensa/Kconfig
@@ -6,6 +6,8 @@ config XTENSA_VIRT
  select XTENSA_SIM
  select PCI_EXPRESS_GENERIC_BRIDGE
  select PCI_DEVICES
+select VIRTIO_PCI
+select VIRTIO_NET


Not needed.

Paolo

[PATCH v3 0/3] vhost: memslot handling improvements

[David Hildenbrand]

Following up on my previous work to make virtio-mem consume multiple
memslots dynamically [1] that requires precise accounting between used vs.
reserved memslots, I realized that vhost makes this extra hard by
filtering out some memory region sections (so they don't consume a
memslot) in the vhost-user case, which messes up the whole memslot
accounting.

This series fixes what I found to be broken and prepares for more work on
[1]. Further, it cleanes up the merge checks that I consider unnecessary.

[1] https://lkml.kernel.org/r/20211027124531.57561-8-da...@redhat.com

Cc: "Michael S. Tsirkin" 
Cc: Stefan Hajnoczi 
Cc: Igor Mammedov 
Cc: Paolo Bonzini 
Cc: Peter Xu 
Cc: "Philippe Mathieu-Daudé" 

v2 -> v3:
- Add ACKs
- "softmmu/physmem: Fixup qemu_ram_block_from_host() documentation"
-- Fix typo in description

v1 -> v2:
- "vhost: Rework memslot filtering and fix "used_memslot" tracking"
-- New approach: keep filtering, but make filtering less generic and
   track separately. This should keep any existing setups working.
- "softmmu/physmem: Fixup qemu_ram_block_from_host() documentation"
-- As requested by Igor

David Hildenbrand (3):
  vhost: Rework memslot filtering and fix "used_memslot" tracking
  vhost: Remove vhost_backend_can_merge() callback
  softmmu/physmem: Fixup qemu_ram_block_from_host() documentation

 hw/virtio/vhost-user.c| 21 ++-
 hw/virtio/vhost-vdpa.c|  1 -
 hw/virtio/vhost.c | 62 ---
 include/exec/cpu-common.h | 15 
 include/hw/virtio/vhost-backend.h |  9 +
 softmmu/physmem.c | 17 -
 6 files changed, 68 insertions(+), 57 deletions(-)

-- 
2.40.0

Re: [PATCH 16/22] hw/mips: add VIRTIO and USB dependencies for LOONGSON3V

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

These are hardwired into the board so must be selected. This shows up
as a "make check" failure with a --without-default-devices build.

Signed-off-by: Alex Bennée
---
  hw/mips/Kconfig | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index da3a37e215..0fcc3da41c 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -45,6 +45,9 @@ config LOONGSON3V
  select PCI_EXPRESS_GENERIC_BRIDGE
  select MSI_NONBROKEN
  select FW_CFG_MIPS
+select VIRTIO_PCI
+select VIRTIO_NET
+select USB_OHCI_PCI


Neither is needed by -nodefaults.

Paolo

Re: [PATCH 15/22] hw/sh4: make RTL8139 a hard dependency for RD2

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

We can't just imply it as the board will fail to create otherwise.
This shows up as a "make check" failure with a
--without-default-devices build.


Not needed, also typo in subject (should be R2D rather than RD2).

Paolo

Re: [PATCH 14/22] hw/loongarch: add VIRTIO as a dependency for LOONGARCH_VIRT

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

These are needed for board creation so fail under "make check" with a
--without-default-devices build.

Signed-off-by: Alex Bennée


Also not needed:

bus: main-system-bus
  type System
  dev: platform-bus-device, id "platform-bus-device"
  dev: acpi-ged, id ""
  dev: ls7a_rtc, id ""
  dev: unimplemented-device, id ""
  dev: serial-mm, id ""
  dev: gpex-pcihost, id ""
bus: pcie.0
  type PCIE
  dev: gpex-root, id ""
  dev: loongarch_pch_msi, id ""
  dev: loongarch_pch_pic, id ""
  dev: loongarch.extioi, id ""
  dev: loongarch_ipi, id ""
  dev: fw_cfg_mem, id ""


Paolo

Re: [PULL 11/18] migration: Create migrate_block_bitmap_mapping() function

[Juan Quintela]

Kevin Wolf  wrote:
> Am 03.05.2023 um 16:53 hat Juan Quintela geschrieben:
>> Kevin Wolf  wrote:
>> > Am 27.04.2023 um 17:22 hat Juan Quintela geschrieben:
>> >> Notice that we changed the test of ->has_block_bitmap_mapping
>> >> for the test that block_bitmap_mapping is not NULL.
>> >> 
>> >> Signed-off-by: Juan Quintela 
>> >> Reviewed-by: Vladimir Sementsov-Ogievskiy 
>> >> 
>> >> ---
>> >> 
>> >> Make it return const (vladimir)
>> >
>> > (I don't think this part was actually meant for the commit message)
>> >
>> > This commit broke qemu-iotests 300 on master. Please have a look.
>> >
>> > Kevin
>> 
>> grrr
>> 
>> selfNack
>> 
>> Just wondering, make check don't run this?
>> 
>> I run "make check" before I sent the pull request.
>
> "make check" only runs a subset of iotests because it would take too
> long otherwise (especially in the context of CI - it wasn't me who made
> this decision). It comes at the cost that sometimes we catch problems
> only after merging the patch to git master when a block developer first
> runs the full set of tests.

Ahhh.

> So I wouldn't blame your testing, it's just something that happens, and
> when it happens we need to look after it.

Thanks.  I have sent another email.  Found why it is failing.  But I am
not sure what is wrong the test of the change in the code.

Later, Juan.

Re: [PATCH 13/22] hw/sparc: add a TCX dependency for SUN4M machines

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

This is the fallback VGA devices needed for board creation so will
otherwise fail under "make check" with a --without-default-devices
build.

Signed-off-by: Alex Bennée


Not needed:

info qtree
bus: main-system-bus
  dev: apc, id ""
  dev: empty_slot, id ""
  dev: escc, id ""
  dev: fw_cfg_mem, id ""
  dev: lance, id ""
  dev: macio_idreg, id ""
  dev: memory, id ""
  dev: openprom, id ""
  dev: slavio_intctl, id ""
  dev: slavio_misc, id ""
  dev: slavio_timer, id ""
  dev: sparc32-dma, id ""
  dev: sparc32-espdma, id ""
  dev: sparc32-ledma, id ""
  dev: sun4m-iommu, id ""
  dev: sun-CS4231, id ""
  dev: sun-fdtwo, id ""
  dev: sysbus-esp, id ""
  dev: sysbus-m48t08, id ""
  dev: tcx_afx, id ""

Paolo


---
  hw/sparc/Kconfig | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/sparc/Kconfig b/hw/sparc/Kconfig
index 79d58beb7a..721b37a9ca 100644
--- a/hw/sparc/Kconfig
+++ b/hw/sparc/Kconfig
@@ -15,6 +15,7 @@ config SUN4M
  select STP2000
  select CHRP_NVRAM
  select OR_IRQ
+select TCX

Re: [PATCH 12/22] hw/hppa: add TULIP as a dependency for HPPA_B160L

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

These are needed for board creation so fail under "make check" with a
--without-default-devices build.

Signed-off-by: Alex Bennée 


Not needed.  Selecting "LSI_SCSI_PCI" however is correct:

bus: main-system-bus
  type System
  dev: ps2-mouse, id ""
  dev: ps2-kbd, id ""
  dev: lasips2, id ""
  dev: fw_cfg_mem, id ""
  dev: serial-mm, id ""
  dev: isabus-bridge, id ""
  dev: dino-pcihost, id ""
bus: pci
  type PCI
  dev: lsi53c895a, id ""
  dev: lasi-chip, id ""


Paolo

Re: [PULL 11/18] migration: Create migrate_block_bitmap_mapping() function

[Juan Quintela]

Kevin Wolf  wrote:
> Am 27.04.2023 um 17:22 hat Juan Quintela geschrieben:
>> Notice that we changed the test of ->has_block_bitmap_mapping
>> for the test that block_bitmap_mapping is not NULL.
>> 
>> Signed-off-by: Juan Quintela 
>> Reviewed-by: Vladimir Sementsov-Ogievskiy 
>> 
>> ---
>> 
>> Make it return const (vladimir)
>
> (I don't think this part was actually meant for the commit message)

yeap.  My understandig has always been that this is the way to put
commenst for the email.

> This commit broke qemu-iotests 300 on master. Please have a look.

if (s->parameters.has_block_bitmap_mapping)
return s->parameters.block_bitmap_mapping;

The test has a case where s->parameters.has_block_bitmap_mapping is true
but s->parameters.block_bitmap_mapping is false.

If that combination is right, then we need two functions becase the
asumtion that I did is false.

Vladimir?

Later, Juan.

> Kevin

Re: [PATCH 11/22] hw/alpha: make E1000_PCI a hard dependency for clipper

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

We can't just imply it as the board will fail to create otherwise.
This shows up as a "make check" failure with a
--without-default-devices build.


Not needed and a test bug.

Paolo

Re: [PATCH 09/22] hw/arm: Select e1000e for sbsa-ref machine

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

From: Fabiano Rosas 

This machine explicitly selects the e1000e network adapter if no other
option was given in the command line. Make sure e1000e is present in
the build.

Signed-off-by: Fabiano Rosas 
Signed-off-by: Alex Bennée 
Message-Id: <20230208192654.8854-10-faro...@suse.de>
---
  hw/arm/Kconfig | 1 +
  1 file changed, 1 insertion(+)


This is not necessary, and patch 10 is not as well, because neither VGA 
nor a NIC are added with --nodefaults.


Paolo

Re: [PULL 11/18] migration: Create migrate_block_bitmap_mapping() function

[Kevin Wolf]

Am 03.05.2023 um 16:53 hat Juan Quintela geschrieben:
> Kevin Wolf  wrote:
> > Am 27.04.2023 um 17:22 hat Juan Quintela geschrieben:
> >> Notice that we changed the test of ->has_block_bitmap_mapping
> >> for the test that block_bitmap_mapping is not NULL.
> >> 
> >> Signed-off-by: Juan Quintela 
> >> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> >> 
> >> ---
> >> 
> >> Make it return const (vladimir)
> >
> > (I don't think this part was actually meant for the commit message)
> >
> > This commit broke qemu-iotests 300 on master. Please have a look.
> >
> > Kevin
> 
> grrr
> 
> selfNack
> 
> Just wondering, make check don't run this?
> 
> I run "make check" before I sent the pull request.

"make check" only runs a subset of iotests because it would take too
long otherwise (especially in the context of CI - it wasn't me who made
this decision). It comes at the cost that sometimes we catch problems
only after merging the patch to git master when a block developer first
runs the full set of tests.

So I wouldn't blame your testing, it's just something that happens, and
when it happens we need to look after it.

Kevin

Re: [PATCH 07/22] hw/arm: Select XLNX_USB_SUBSYS for xlnx-zcu102 machine

[Paolo Bonzini]

On 5/3/23 11:12, Alex Bennée wrote:

From: Fabiano Rosas 

This machine hardcodes initialization of the USB device, so select the
corresponding Kconfig. It is not enough to have it as "default y if
XLNX_VERSAL" at usb/Kconfig because building --without-default-devices
disables the default selection resulting in:

$ ./qemu-system-aarch64 -M xlnx-zcu102
qemu-system-aarch64: missing object type 'usb_dwc3'
Aborted (core dumped)

Signed-off-by: Fabiano Rosas 
Signed-off-by: Alex Bennée 
Message-Id: <20230208192654.8854-8-faro...@suse.de>
---
  hw/arm/Kconfig | 1 +
  hw/usb/Kconfig | 1 -
  2 files changed, 1 insertion(+), 1 deletion(-)


This one is correct: "-M xlnx-zcu102 -nodefaults" has a usb_dwc3 device.

Acked-by: Paolo Bonzini 

Paolo


diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 168c1e8089..cec7898bb7 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -439,6 +439,7 @@ config XLNX_VERSAL
  select OR_IRQ
  select XLNX_BBRAM
  select XLNX_EFUSE_VERSAL
+select XLNX_USB_SUBSYS
  
  config NPCM7XX

  bool
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index ce4f433976..0ec6def4b8 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -136,5 +136,4 @@ config USB_DWC3
  
  config XLNX_USB_SUBSYS

  bool
-default y if XLNX_VERSAL
  select USB_DWC3

Re: [PATCH 05/22] hw/arm: Select VIRTIO_NET for virt machine

[Paolo Bonzini]

On 5/3/23 17:35, Alex Bennée wrote:

You should select a device only if you cannot even start
the machine without --nodefaults.


Which is the case here right? We could skip tests that explicitly
instantiate a device but these are tests failing with default devices
the machine tries to instantiate.


I'm sorry, I meant "select" directives are needed if you cannot even 
start the machine *with* --nodefaults.


Devices that are added *without* --nodefaults should use "imply" 
directives instead, as is already the case.


Paolo

Re: [PATCH risu] use time() as random seed and introduce --randseed option

[Alex Bennée]

Jun Sun  writes:

> By default, risu currently does not generate random instruction sequences 
> because it uses 0 as the random seed. 
> This patch uses time() as random seed and also introduces --randomseed option 
> for deterministic sequence
> generation.

I can see the benefit for being able to change the seed but I think
using time() by default means any given sequence won't be reproducible.
This is useful behaviour if you want to regenerate the same test
sequence on another machine without copying stuff about.

>
> [4. text/x-diff; 
> 0008-add-randseed-option-and-use-time-as-default-seed.patch]...


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v1 1/1] hw/pci: Disable PCI_ERR_UNCOR_MASK register for machine type < 8.0

[Juan Quintela]

Leonardo Bras  wrote:
> Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
> set for machine types < 8.0 will cause migration to fail if the target
> QEMU version is < 8.0.0 :
>
> qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10a read: 40 
> device: 0 cmask: ff wmask: 0 w1cmask:0
> qemu-system-x86_64: Failed to load PCIDevice:config
> qemu-system-x86_64: Failed to load e1000e:parent_obj
> qemu-system-x86_64: error while loading state for instance 0x0 of device 
> ':00:02.0/e1000e'
> qemu-system-x86_64: load of migration failed: Invalid argument
>
> The above test migrated a 7.2 machine type from QEMU master to QEMU 7.2.0,
> with this cmdline:
>
> ./qemu-system-x86_64 -M pc-q35-7.2 [-incoming XXX]
>
> In order to fix this, property x-pcie-err-unc-mask was introduced to
> control when PCI_ERR_UNCOR_MASK is enabled. This property is enabled by
> default, but is disabled if machine type <= 7.2.
>
> Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register")
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Leonardo Bras 

Reviewed-by: Juan Quintela 

[PATCH] iotests/nbd-reconnect-on-open: Fix NBD socket path

[Kevin Wolf]

Socket paths need to be short to avoid failures. This is why there is a
iotests.sock_dir (defaulting to /tmp) separate from the disk image base
directory.

Make use of it to fix failures in too deeply nested test directories.

Fixes: ab7f7e67a7e7b49964109501dfcde4ec29bae60e
Signed-off-by: Kevin Wolf 
---
 tests/qemu-iotests/tests/nbd-reconnect-on-open | 3 ++-
 tests/qemu-iotests/tests/nbd-reconnect-on-open.out | 4 ++--
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open 
b/tests/qemu-iotests/tests/nbd-reconnect-on-open
index d0b401b060..3ce52021c3 100755
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open
@@ -26,7 +26,8 @@ from iotests import qemu_img_create, file_path, 
qemu_io_popen, qemu_nbd, \
 
 iotests.script_initialize(supported_fmts=['qcow2'])
 
-disk, nbd_sock = file_path('disk', 'nbd-sock')
+disk = file_path('disk')
+nbd_sock = file_path('nbd-sock', base_dir=iotests.sock_dir)
 
 
 def create_args(open_timeout):
diff --git a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out 
b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
index a35ae30ea4..b3dd90f2a3 100644
--- a/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
+++ b/tests/qemu-iotests/tests/nbd-reconnect-on-open.out
@@ -2,10 +2,10 @@ read 1048576/1048576 bytes at offset 0
 1 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 Check fail to connect with 0 seconds of timeout
-qemu-io: can't open: Failed to connect to 'TEST_DIR/PID-nbd-sock': No such 
file or directory
+qemu-io: can't open: Failed to connect to 'SOCK_DIR/PID-nbd-sock': No such 
file or directory
 
 qemu_io finished in 0..0.2 seconds, OK
 Check fail to connect with 1 seconds of timeout
-qemu-io: can't open: Failed to connect to 'TEST_DIR/PID-nbd-sock': No such 
file or directory
+qemu-io: can't open: Failed to connect to 'SOCK_DIR/PID-nbd-sock': No such 
file or directory
 
 qemu_io finished in 1..1.2 seconds, OK
-- 
2.40.1

RE: [PATCH v3] Hexagon (target/hexagon) Additional instructions handled by idef-parser

[Taylor Simpson]

> -Original Message-
> From: Anton Johansson 
> Sent: Tuesday, May 2, 2023 6:12 AM
> To: Taylor Simpson ; qemu-devel@nongnu.org
> Cc: richard.hender...@linaro.org; phi...@linaro.org; a...@rev.ng; Brian Cain
> ; Matheus Bernardino (QUIC)
> 
> Subject: Re: [PATCH v3] Hexagon (target/hexagon) Additional instructions
> handled by idef-parser
> 
> On 5/1/23 22:31, Taylor Simpson wrote:
> > diff --git a/target/hexagon/idef-parser/parser-helpers.c
> > b/target/hexagon/idef-parser/parser-helpers.c
> > index 86511efb62..0ad917f591 100644
> > --- a/target/hexagon/idef-parser/parser-helpers.c
> > +++ b/target/hexagon/idef-parser/parser-helpers.c
> > -961,9 +973,16 @@ HexValue gen_cast_op(Context *c,
> >   {
> >   assert_signedness(c, locp, src->signedness);
> >   if (src->bit_width == target_width) {
> > -return *src;
> > -} else if (src->type == IMMEDIATE) {
> >   HexValue res = *src;
> > +res.signedness = signedness;
> > +return res;
> > +} else if (src->type == IMMEDIATE) {
> > +HexValue res;
> > +if (src->bit_width < target_width) {
> > +res = gen_rvalue_extend(c, locp, src);
> > +} else {
> > +res = *src;
> > +}
> >   res.bit_width = target_width;
> >   res.signedness = signedness;
> >   return res;
> 
> Ah, gen_cast_op() can be simplified a great deal here to
> 
>  HexValue gen_cast_op(Context *c,
>   YYLTYPE *locp,
>   HexValue *src,
>   unsigned target_width,
>   HexSignedness signedness)
>  {
>  HexValue res;
>  assert_signedness(c, locp, src->signedness);
>  if (src->bit_width == target_width) {
>  res = *src;
>  } else if (src->bit_width < target_width) {
>  res = gen_rvalue_extend(c, locp, src);
>  } else {
>  res = gen_rvalue_truncate(c, locp, src);
>  }
>  res.signedness = signedness;
>  return res;
>  }

Great suggestion, thanks!

> 
> Other than that this patch looks good:
> 
> Tested-by: Anton Johansson 
> Reviewed-by: Anton Johansson 
> 

[PATCH risu] Add "--not-group" option to exclude groups of instructions.

[Jun Sun]

This mirrors the "--not-pattern" option and gives complete control over
group-based
instruction selection rules.

Signed-off-by: Jun Sun 
---
 risugen | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/risugen b/risugen
index f88c22a..f441f06 100755
--- a/risugen
+++ b/risugen
@@ -39,6 +39,7 @@ my $arch = "";
 my @insn_groups;

 my @groups = ();# include groups
+my @not_groups = ();# exclude groups
 my @pattern_re = ();# include pattern
 my @not_pattern_re = ();# exclude pattern

@@ -272,6 +273,12 @@ sub select_insn_keys ()
 my $re = '\b((' . join(')|(',@pattern_re) . '))\b';
 @insn_keys = grep /$re/, @insn_keys;
 }
+if (@not_groups) {
+@insn_keys = grep {
+defined($insn_details{$_}->{groups}) &&
+0 == scalar(get_intersection([$insn_details{$_}->{groups},
\@not_groups]))
+} @insn_keys
+}
 # exclude any specifics
 if (@not_pattern_re) {
 my $re = '\b((' . join(')|(',@not_pattern_re) . '))\b';
@@ -303,6 +310,7 @@ Valid options:
the perl regex '\\b((re)|(re))\\b'). This means that
'VMULL' will match 'VMULL A1' and 'VMULL A2' but not
'VMULL_scalar A1'. This is generally what you wanted.
+--not-group name[,name..]: exclude instructions in the specified groups
 --not-pattern re[,re...] : exclude patterns matching regular
expression.
These REs are applied after the matching pattern which
is useful if you want to exclude a specific instruction
from
@@ -331,6 +339,7 @@ sub main()
 "randseed=i" => \$randseed,
 "fpscr=o" => \$fpscr,
 "group=s" => \@groups,
+"not-group=s" => \@not_groups,
 "pattern=s" => \@pattern_re,
 "not-pattern=s" => \@not_pattern_re,
 "condprob=f" => sub {
@@ -347,6 +356,7 @@ sub main()
 @pattern_re = split(/,/,join(',',@pattern_re));
 @not_pattern_re = split(/,/,join(',',@not_pattern_re));
 @groups = split(/,/,join(',',@groups));
+@not_groups = split(/,/,join(',',@not_groups));

 if ($#ARGV != 1) {
 usage();
-- 
2.34.1

[PATCH risu] --group option to allow all instructions in the specified groups.

[Jun Sun]

Current semantic is a little strange when multiple --group options are
specified.
In this case,  only instructions in *all* these groups (i.e., intersection)
are used for
generation, which is not very useful at all.  This patch changes the
semantic to
include all instructions in these groups (i.e., union) for sequence
generation.

Signed-off-by: Jun Sun 
---
 risugen| 4 ++--
 risugen_arm.pm | 1 +
 risugen_loongarch64.pm | 1 +
 risugen_m68k.pm| 1 +
 risugen_ppc64.pm   | 1 +
 5 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/risugen b/risugen
index 360112f..f88c22a 100755
--- a/risugen
+++ b/risugen
@@ -264,7 +264,7 @@ sub select_insn_keys ()
 if (@groups) {
 @insn_keys = grep {
 defined($insn_details{$_}->{groups}) &&
-scalar @groups ==
get_intersection([$insn_details{$_}->{groups}, \@groups])
+0 != scalar(get_intersection([$insn_details{$_}->{groups},
\@groups]))
 } @insn_keys
 }
 # Get a list of the insn keys which are permitted by the re patterns
@@ -297,7 +297,7 @@ Valid options:
 --fpscr n: set initial FPSCR (arm) or FPCR (aarch64) value
(default is 0)
 --condprob p : [ARM only] make instructions conditional with
probability p
(default is 0, ie all instructions are always executed)
---group name[,name..]: only use instructions in all defined groups
+--group name[,name..]: only use instructions in the specified groups
 --pattern re[,re...] : only use instructions matching regular
expression
Each re must match a full word (that is, we match on
the perl regex '\\b((re)|(re))\\b'). This means that
diff --git a/risugen_arm.pm b/risugen_arm.pm
index 2dc144d..dc08ec0 100644
--- a/risugen_arm.pm
+++ b/risugen_arm.pm
@@ -1112,6 +1112,7 @@ sub write_test_code($)
 }

 print "Generating code using patterns: @keys...\n";
+print "Total insn patterns : " . $#keys . "\n";
 progress_start(78, $numinsns);

 if ($fp_enabled) {
diff --git a/risugen_loongarch64.pm b/risugen_loongarch64.pm
index 3b1b4f9..f2a6fe7 100644
--- a/risugen_loongarch64.pm
+++ b/risugen_loongarch64.pm
@@ -482,6 +482,7 @@ sub write_test_code($)
 }

 print "Generating code using patterns: @keys...\n";
+print "Total insn patterns : " . $#keys . "\n";
 progress_start(78, $numinsns);

 if ($fp_enabled) {
diff --git a/risugen_m68k.pm b/risugen_m68k.pm
index 85fc3da..76af84b 100644
--- a/risugen_m68k.pm
+++ b/risugen_m68k.pm
@@ -181,6 +181,7 @@ sub write_test_code($)
 }

 print "Generating code using patterns: @keys...\n";
+print "Total insn patterns : " . $#keys . "\n";
 progress_start(78, $numinsns);

 if (grep { defined($insn_details{$_}->{blocks}->{"memory"}) } @keys) {
diff --git a/risugen_ppc64.pm b/risugen_ppc64.pm
index 4bc2d62..e6d0456 100644
--- a/risugen_ppc64.pm
+++ b/risugen_ppc64.pm
@@ -392,6 +392,7 @@ sub write_test_code($)
 }

 print "Generating code using patterns: @keys...\n";
+print "Total insn patterns : " . $#keys . "\n";
 progress_start(78, $numinsns);

 if (grep { defined($insn_details{$_}->{blocks}->{"memory"}) } @keys) {
-- 
2.34.1

Re: [PATCH 3/3] target/openrisc: Setup FPU for detecting tininess before rounding

[Stafford Horne]

On Wed, May 03, 2023 at 10:41:42AM +0100, Richard Henderson wrote:
> On 5/3/23 10:14, Stafford Horne wrote:
> > > > +set_default_nan_mode(1, >env.fp_status);
> > > > +set_float_detect_tininess(float_tininess_before_rounding,
> > > > +  >env.fp_status);
> > > 
> > > You don't mention the nan change in the commit message.
> > 
> > Right, and I am not sure I need it.  Let me remove it and run tests again.  
> > I
> > was just adding it as a few other architectures did who set
> > float_tininess_before_rounding.
> 
> What that does is *not* propagate NaN payloads from (some) input to the
> output.  This is certainly true of RISC-V, which specifies this in their
> architecture manual.  OpenRISC does not specify any NaN behaviour at all.

Thanks, that is what I also gathered from reading up on it.

> It's not a bad choice, really, and it almost certainly simplifies the design
> of the FPU, as you can do NaN propagation and silencing in one step.

Right, it makes sense to optimize.  It doesn't look like any of our FPU
implementation do that at the moment.

I will check with bandvig who implemented the FPU to understand his thought on
this.  It at least deserves to be discussed how nan payload is to be handled in
the architecture spec.

-Stafford

[PATCH risu] use time() as random seed and introduce --randseed option

[Jun Sun]

By default, risu currently does not generate random instruction sequences
because it uses 0 as the random seed.  This patch uses time() as random
seed and also introduces --randomseed option for deterministic sequence
generation.


0008-add-randseed-option-and-use-time-as-default-seed.patch
Description: Binary data

Re: [PATCH] block: Fix use after free in blockdev_mark_auto_del()

[Stefan Hajnoczi]

On Wed, May 03, 2023 at 04:01:42PM +0200, Kevin Wolf wrote:
> job_cancel_locked() drops the job list lock temporarily and it may call
> aio_poll(). We must assume that the list has changed after this call.
> Also, with unlucky timing, it can end up freeing the job during
> job_completed_txn_abort_locked(), making the job pointer invalid, too.
> 
> For both reasons, we can't just continue at block_job_next_locked(job).
> Instead, start at the head of the list again after job_cancel_locked()
> and skip those jobs that we already cancelled (or that are completing
> anyway).
> 
> Signed-off-by: Kevin Wolf 
> ---
>  blockdev.c | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [PATCH] block/export: call blk_set_dev_ops(blk, NULL, NULL)

[Stefan Hajnoczi]

On Wed, May 03, 2023 at 10:43:16AM -0500, Eric Blake wrote:
> On Tue, May 02, 2023 at 05:11:19PM -0400, Stefan Hajnoczi wrote:
> > Most export types install BlockDeviceOps pointers. It is easy to forget
> > to remove them because that happens automatically via the "drive" qdev
> > property in hw/ but not block/export/.
> > 
> > Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so
> > the export types don't need to remember.
> > 
> > This fixes the nbd and vhost-user-blk export types.
> > 
> > Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the 
> > server")
> > Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk 
> > resize")
> > Signed-off-by: Stefan Hajnoczi 
> > ---
> >  block/export/export.c| 2 ++
> >  block/export/vduse-blk.c | 1 -
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Eric Blake 
> 
> I'm happy to add this through my NBD queue.

Sure, go ahead!

Stefan


signature.asc
Description: PGP signature

Re: [PATCH] block/blkio: add 'fd' option to virtio-blk-vhost-vdpa driver

[Stefan Hajnoczi]

On Wed, May 03, 2023 at 11:15:56AM +0200, Stefano Garzarella wrote:
> On Tue, May 02, 2023 at 03:02:32PM -0400, Stefan Hajnoczi wrote:
> > On Tue, May 02, 2023 at 04:50:50PM +0200, Stefano Garzarella wrote:
> > > The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the new
> > > 'fd' property. Let's expose this to the user, so the management layer
> > > can pass the file descriptor of an already opened vhost-vdpa character
> > > device. This is useful especially when the device can only be accessed
> > > with certain privileges.
> > > 
> > > Signed-off-by: Stefano Garzarella 
> > > ---
> > > 
> > > Notes:
> > > As an alternative we could support passing `/dev/fdset/N` via 'path',
> > > always opening the path with qemu_open() and passing the fd to the
> > > libblkio driver.
> > > I preferred to add a new parameter though, because the code is
> > > simpler without changing how path works (alternatively we should check
> > > first if fd is supported by the driver or not).
> > > 
> > > What do you think?
> > 
> > I think the approach in this patch is fine.
> > 
> > > 
> > > Thanks,
> > > Stefano
> > > 
> > >  qapi/block-core.json |  6 +-
> > >  block/blkio.c| 45 +++-
> > >  2 files changed, 49 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > > index b57978957f..9f70777d49 100644
> > > --- a/qapi/block-core.json
> > > +++ b/qapi/block-core.json
> > > @@ -3841,10 +3841,14 @@
> > >  #
> > >  # @path: path to the vhost-vdpa character device.
> > >  #
> > > +# @fd: file descriptor of an already opened vhost-vdpa character device.
> > > +#  (Since 8.1)
> > > +#
> > >  # Since: 7.2
> > >  ##
> > >  { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa',
> > > -  'data': { 'path': 'str' },
> > > +  'data': { '*path': 'str',
> > > +'*fd': 'str' },
> > >'if': 'CONFIG_BLKIO' }
> > > 
> > >  ##
> > > diff --git a/block/blkio.c b/block/blkio.c
> > > index 0cdc99a729..98394b5745 100644
> > > --- a/block/blkio.c
> > > +++ b/block/blkio.c
> > > @@ -694,6 +694,49 @@ static int 
> > > blkio_virtio_blk_common_open(BlockDriverState *bs,
> > >  return 0;
> > >  }
> > > 
> > > +static int blkio_virtio_blk_vhost_vdpa_open(BlockDriverState *bs,
> > > +QDict *options, int flags, Error **errp)
> > > +{
> > > +const char *path = qdict_get_try_str(options, "path");
> > > +const char *fd_str = qdict_get_try_str(options, "fd");
> > > +BDRVBlkioState *s = bs->opaque;
> > > +int ret;
> > > +
> > > +if (path && fd_str) {
> > > +error_setg(errp, "'path' and 'fd' options are mutually 
> > > exclusive");
> > > +return -EINVAL;
> > > +}
> > > +
> > > +if (!path && !fd_str) {
> > > +error_setg(errp, "none of 'path' or 'fd' options was specified");
> > > +return -EINVAL;
> > > +}
> > > +
> > > +if (path) {
> > > +ret = blkio_set_str(s->blkio, "path", path);
> > > +qdict_del(options, "path");
> > > +if (ret < 0) {
> > > +error_setg_errno(errp, -ret, "failed to set path: %s",
> > > + blkio_get_error_msg());
> > > +return ret;
> > > +}
> > > +} else {
> > > +ret = blkio_set_str(s->blkio, "fd", fd_str);
> > 
> > monitor_fd_param() is used by vhost-net, vhost-vsock, vhost-scsi, etc.
> > 
> > I think QEMU should parse the fd string and resolve it to a file
> > descriptor so the fd passing syntax matches the other vhost devices.
> 
> Okay, but I have a linker issue if I use monitor_fd_param().
> IIUC because blkio is built as a module, so what about adding
> qemu_fd_param() in libqemuutil?

Modules can access any extern function in QEMU so I don't think there is
a fundamental limitation there.

Maybe it's related to the dependencies between the blkio module and
monitor/ code. monitor_get_fd_param() is in softmmu_ss, which block
drivers don't directly depend on AFAICT.

> 
> I mean something like this:
> 
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index 9eff0be95b..87360c983a 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -568,6 +568,7 @@ int qemu_lock_fd(int fd, int64_t start, int64_t len, bool 
> exclusive);
>  int qemu_unlock_fd(int fd, int64_t start, int64_t len);
>  int qemu_lock_fd_test(int fd, int64_t start, int64_t len, bool exclusive);
>  bool qemu_has_ofd_lock(void);
> +int qemu_fd_param(const char *fdname, Error **errp);
>  #endif
> 
>  #if defined(__HAIKU__) && defined(__i386__)
> diff --git a/util/osdep.c b/util/osdep.c
> index e996c4744a..ed0832810b 100644
> --- a/util/osdep.c
> +++ b/util/osdep.c
> @@ -234,6 +234,11 @@ bool qemu_has_ofd_lock(void)
>  #endif
>  }
> 
> +int qemu_fd_param(const char *fdname, Error **errp)
> +{
> +return monitor_fd_param(monitor_cur(), fdname, errp);
> +}

I'm not sure. If it works with modules enabled/disabled,
qemu-io/qemu-img/etc, 

Re: [PATCH v1 1/1] hw/pci: Disable PCI_ERR_UNCOR_MASK register for machine type < 8.0

[Leonardo Bras Soares Passos]

On Wed, May 3, 2023 at 6:49 AM Jonathan Cameron
 wrote:
>
> On Tue,  2 May 2023 21:27:02 -0300
> Leonardo Bras  wrote:
>
> > Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
> > set for machine types < 8.0 will cause migration to fail if the target
> > QEMU version is < 8.0.0 :
> >
> > qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10a read: 
> > 40 device: 0 cmask: ff wmask: 0 w1cmask:0
> > qemu-system-x86_64: Failed to load PCIDevice:config
> > qemu-system-x86_64: Failed to load e1000e:parent_obj
> > qemu-system-x86_64: error while loading state for instance 0x0 of device 
> > ':00:02.0/e1000e'
> > qemu-system-x86_64: load of migration failed: Invalid argument
> >
> > The above test migrated a 7.2 machine type from QEMU master to QEMU 7.2.0,
> > with this cmdline:
> >
> > ./qemu-system-x86_64 -M pc-q35-7.2 [-incoming XXX]
> >
> > In order to fix this, property x-pcie-err-unc-mask was introduced to
> > control when PCI_ERR_UNCOR_MASK is enabled. This property is enabled by
> > default, but is disabled if machine type <= 7.2.
> >
> > Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register")
> > Suggested-by: Michael S. Tsirkin 
> > Signed-off-by: Leonardo Bras 
>
> Thanks Leo, you are a star.
>
> LGTM
>
> Reviewed-by: Jonathan Cameron 
>

Thanks!

> > ---
> >  include/hw/pci/pci.h |  2 ++
> >  hw/core/machine.c|  1 +
> >  hw/pci/pci.c |  2 ++
> >  hw/pci/pcie_aer.c| 11 +++
> >  4 files changed, 12 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 935b4b91b4..e6d0574a29 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -207,6 +207,8 @@ enum {
> >  QEMU_PCIE_EXTCAP_INIT = (1 << QEMU_PCIE_EXTCAP_INIT_BITNR),
> >  #define QEMU_PCIE_CXL_BITNR 10
> >  QEMU_PCIE_CAP_CXL = (1 << QEMU_PCIE_CXL_BITNR),
> > +#define QEMU_PCIE_ERR_UNC_MASK_BITNR 11
> > +QEMU_PCIE_ERR_UNC_MASK = (1 << QEMU_PCIE_ERR_UNC_MASK_BITNR),
> >  };
> >
> >  typedef struct PCIINTxRoute {
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index 47a34841a5..07f763eb2e 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -48,6 +48,7 @@ GlobalProperty hw_compat_7_2[] = {
> >  { "e1000e", "migrate-timadj", "off" },
> >  { "virtio-mem", "x-early-migration", "false" },
> >  { "migration", "x-preempt-pre-7-2", "true" },
> > +{ TYPE_PCI_DEVICE, "x-pcie-err-unc-mask", "off" },
> >  };
> >  const size_t hw_compat_7_2_len = G_N_ELEMENTS(hw_compat_7_2);
> >
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index 8a87ccc8b0..5153ad63d6 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -79,6 +79,8 @@ static Property pci_props[] = {
> >  DEFINE_PROP_STRING("failover_pair_id", PCIDevice,
> > failover_pair_id),
> >  DEFINE_PROP_UINT32("acpi-index",  PCIDevice, acpi_index, 0),
> > +DEFINE_PROP_BIT("x-pcie-err-unc-mask", PCIDevice, cap_present,
> > +QEMU_PCIE_ERR_UNC_MASK_BITNR, true),
> >  DEFINE_PROP_END_OF_LIST()
> >  };
> >
> > diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
> > index 103667c368..374d593ead 100644
> > --- a/hw/pci/pcie_aer.c
> > +++ b/hw/pci/pcie_aer.c
> > @@ -112,10 +112,13 @@ int pcie_aer_init(PCIDevice *dev, uint8_t cap_ver, 
> > uint16_t offset,
> >
> >  pci_set_long(dev->w1cmask + offset + PCI_ERR_UNCOR_STATUS,
> >   PCI_ERR_UNC_SUPPORTED);
> > -pci_set_long(dev->config + offset + PCI_ERR_UNCOR_MASK,
> > - PCI_ERR_UNC_MASK_DEFAULT);
> > -pci_set_long(dev->wmask + offset + PCI_ERR_UNCOR_MASK,
> > - PCI_ERR_UNC_SUPPORTED);
> > +
> > +if (dev->cap_present & QEMU_PCIE_ERR_UNC_MASK) {
> > +pci_set_long(dev->config + offset + PCI_ERR_UNCOR_MASK,
> > + PCI_ERR_UNC_MASK_DEFAULT);
> > +pci_set_long(dev->wmask + offset + PCI_ERR_UNCOR_MASK,
> > + PCI_ERR_UNC_SUPPORTED);
> > +}
> >
> >  pci_set_long(dev->config + offset + PCI_ERR_UNCOR_SEVER,
> >   PCI_ERR_UNC_SEVERITY_DEFAULT);
>

Re: [PATCH 0/8] migration: Add precopy initial data capability and VFIO precopy support

[Peter Xu]

On Wed, May 03, 2023 at 06:22:59PM +0300, Avihai Horon wrote:
> 
> On 03/05/2023 1:49, Peter Xu wrote:
> > External email: Use caution opening links or attachments
> > 
> > 
> > On Mon, May 01, 2023 at 05:01:33PM +0300, Avihai Horon wrote:
> > > Hello everyone,
> > Hi, Avihai,
> > 
> > > === Flow of operation ===
> > > 
> > > To use precopy initial data, the capability must be enabled in the
> > > source.
> > > 
> > > As this capability must be supported also in the destination, a
> > > handshake is performed during migration setup. The purpose of the
> > > handshake is to notify the destination that precopy initial data is used
> > > and to check if it's supported.
> > > 
> > > The handshake is done in two levels. First, a general handshake is done
> > > with the destination migration code to notify that precopy initial data
> > > is used. Then, for each migration user in the source that supports
> > > precopy initial data, a handshake is done with its counterpart in the
> > > destination:
> > > If both support it, precopy initial data will be used for them.
> > > If source doesn't support it, precopy initial data will not be used for
> > > them.
> > > If source supports it and destination doesn't, migration will be failed.
> > > 
> > > Assuming the handshake succeeded, migration starts to send precopy data
> > > and as part of it also the initial precopy data. Initial precopy data is
> > > just like any other precopy data and as such, migration code is not
> > > aware of it. Therefore, it's the responsibility of the migration users
> > > (such as VFIO devices) to notify their counterparts in the destination
> > > that their initial precopy data has been sent (for example, VFIO
> > > migration does it when its initial bytes reach zero).
> > > 
> > > In the destination, migration code will query each migration user that
> > > supports precopy initial data and check if its initial data has been
> > > loaded. If initial data has been loaded by all of them, an ACK will be
> > > sent to the source which will now be able to complete migration when
> > > appropriate.
> > I can understand why this is useful, what I'm not 100% sure is whether the
> > complexity is needed.  The idea seems to be that src never switchover
> > unless it receives a READY notification from dst.
> > 
> > I'm imaging below simplified and more general workflow, not sure whether it
> > could work for you:
> > 
> >- Introduce a new cap "switchover-ready", it means whether there'll be a
> >  ready event sent from dst -> src for "being ready for switchover"
> > 
> >- When cap set, a new msg MIG_RP_MSG_SWITCHOVER_READY is defined and
> >  handled on src showing that dest is ready for switchover. It'll be sent
> >  only if dest is ready for the switchover
> > 
> >- Introduce a field SaveVMHandlers.explicit_switchover_needed.  For each
> >  special device like vfio that would like to participate in the decision
> >  making, device can set its explicit_switchover_needed=1.  This field is
> >  ignored if the new cap is not set.
> > 
> >- Dst qemu: when new cap set, remember how many special devices are there
> >  requesting explicit switchover (count of SaveVMHandlers that has the
> >  bit set during load setup) as switch_over_pending=N.
> > 
> >- Dst qemu: Once a device thinks its fine to switchover (probably in the
> >  load_state() callback), it calls migration_notify_switchover_ready().
> >  That decreases switch_over_pending and when it hits zero, one msg
> >  MIG_RP_MSG_SWITCHOVER_READY will be sent to src.
> > 
> > Only until READY msg received on src could src switchover the precopy to
> > dst.
> > 
> > Then it only needs 1 more field in SaveVMHandlers rather than 3, and only 1
> > more msg (dst->src).
> > 
> > This is based on the fact that right now we always set caps on both qemus
> > so I suppose it already means either both have or don't have the feature
> > (even if one has, not setting the cap means disabled on both).
> > 
> > Would it work for this case and cleaner?
> 
> Hi Peter, thanks for the response!
> Your approach is indeed much simpler, however I have a few concerns
> regarding compatibility.
> 
> You are saying that caps are always set both in src and dest.
> But what happens if we set the cap only on one side?
> Should we care about these scenarios?

I think it's not needed for now, but I am aware that this is a problem.
It's just that it is a more generic problem to me rather than very special
in the current feature being proposed.  At least there're a few times
Daniel showed concern on keeping this way and hoped we can have a better
handshake in general with migration framework.

I'd be perfectly fine if you want to approach this with a handshake
methodology, but I hope if so we should provide a more generic handshake.
So potentially that can make this new feature rely on the handshake work,
and slower to get into shape.  Your call on how to address this, 

Re: [PATCH] block/export: call blk_set_dev_ops(blk, NULL, NULL)

[Eric Blake]

On Tue, May 02, 2023 at 05:11:19PM -0400, Stefan Hajnoczi wrote:
> Most export types install BlockDeviceOps pointers. It is easy to forget
> to remove them because that happens automatically via the "drive" qdev
> property in hw/ but not block/export/.
> 
> Put blk_set_dev_ops(blk, NULL, NULL) calls in the core export.c code so
> the export types don't need to remember.
> 
> This fixes the nbd and vhost-user-blk export types.
> 
> Fixes: fd6afc501a01 ("nbd/server: Use drained block ops to quiesce the 
> server")
> Fixes: ca858a5fe94c ("vhost-user-blk-server: notify client about disk resize")
> Signed-off-by: Stefan Hajnoczi 
> ---
>  block/export/export.c| 2 ++
>  block/export/vduse-blk.c | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)

Reviewed-by: Eric Blake 

I'm happy to add this through my NBD queue.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [PATCH 05/22] hw/arm: Select VIRTIO_NET for virt machine

[Alex Bennée]

Paolo Bonzini  writes:

> On 5/3/23 11:12, Alex Bennée wrote:
>> From: Fabiano Rosas 
>> The 'virt' machine uses virtio-net-pci as a fallback when no other
>> network driver has been selected via command line. Select VIRTIO_NET
>> and VIRTIO_PCI from CONFIG_ARM_VIRT to avoid errors when PCI_DEVICES=n
>> (due to e.g. --without-default-devices):
>> $ ./qemu-system-aarch64 -M virt -accel tcg -cpu max
>> qemu-system-aarch64: Unsupported NIC model: virtio-net-pci
>
> With respect to patches 5-17, very few devices need to be present when
> configuring --without-default-devices, and thus need to be "select"ed
> by Kconfig.  You should select a device only if you cannot even start
> the machine without --nodefaults.

Which is the case here right? We could skip tests that explicitly
instantiate a device but these are tests failing with default devices
the machine tries to instantiate.

> Anything else should be added by hand to configs/ if you use
> --nodefaults.  In particular, failures of "make check" when configured
> --without-default-devices are *test* bugs, not configuration bugs.
>
> I didn't check if _all_ of the patches in this set should be dropped,
> but most probably do.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 3/8] migration: Add precopy initial data loaded ACK functionality

[Avihai Horon]

On 03/05/2023 1:56, Peter Xu wrote:

External email: Use caution opening links or attachments


On Mon, May 01, 2023 at 05:01:36PM +0300, Avihai Horon wrote:

Add the core functionality of precopy initial data, which allows the
destination to ACK that initial data has been loaded and the source to
wait for this ACK before completing the migration.

A new return path command MIG_RP_MSG_INITIAL_DATA_LOADED_ACK is added.
It is sent by the destination after precopy initial data is loaded to
ACK to the source that precopy initial data has been loaded.

In addition, two new SaveVMHandlers handlers are added:
1. is_initial_data_active which indicates whether precopy initial data
is used for this migration user (i.e., SaveStateEntry).
2. initial_data_loaded which indicates whether precopy initial data has
been loaded by this migration user.

Signed-off-by: Avihai Horon 
---
  include/migration/register.h |  7 ++
  migration/migration.h| 12 +++
  migration/migration.c| 41 ++--
  migration/savevm.c   | 39 ++
  migration/trace-events   |  2 ++
  5 files changed, 99 insertions(+), 2 deletions(-)

diff --git a/include/migration/register.h b/include/migration/register.h
index 0a73f3883e..297bbe9f73 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -77,6 +77,13 @@ typedef struct SaveVMHandlers {
   * true if it's supported or false otherwise. Called both in src and dest.
   */
  bool (*initial_data_advise)(void *opaque);
+/*
+ * Checks if precopy initial data is active. If it's inactive,
+ * initial_data_loaded check is skipped.
+ */
+bool (*is_initial_data_active)(void *opaque);
+/* Checks if precopy initial data has been loaded in dest */
+bool (*initial_data_loaded)(void *opaque);
  } SaveVMHandlers;

  int register_savevm_live(const char *idstr,
diff --git a/migration/migration.h b/migration/migration.h
index 4f615e9dbc..d865c23d87 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -207,6 +207,11 @@ struct MigrationIncomingState {

  /* Indicates whether precopy initial data was enabled and should be used 
*/
  bool initial_data_enabled;
+/*
+ * Indicates whether an ACK that precopy initial data was loaded has been
+ * sent to source.
+ */
+bool initial_data_loaded_ack_sent;
  };

  MigrationIncomingState *migration_incoming_get_current(void);
@@ -435,6 +440,12 @@ struct MigrationState {

  /* QEMU_VM_VMDESCRIPTION content filled for all non-iterable devices. */
  JSONWriter *vmdesc;
+
+/*
+ * Indicates whether an ACK that precopy initial data was loaded in
+ * destination has been received.
+ */
+bool initial_data_loaded_acked;
  };

  void migrate_set_state(int *state, int old_state, int new_state);
@@ -475,6 +486,7 @@ int 
migrate_send_rp_message_req_pages(MigrationIncomingState *mis,
  void migrate_send_rp_recv_bitmap(MigrationIncomingState *mis,
   char *block_name);
  void migrate_send_rp_resume_ack(MigrationIncomingState *mis, uint32_t value);
+int migrate_send_rp_initial_data_loaded_ack(MigrationIncomingState *mis);

  void dirty_bitmap_mig_before_vm_start(void);
  void dirty_bitmap_mig_cancel_outgoing(void);
diff --git a/migration/migration.c b/migration/migration.c
index 68cdf5b184..304cab2fa1 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -77,6 +77,11 @@ enum mig_rp_message_type {
  MIG_RP_MSG_RECV_BITMAP,  /* send recved_bitmap back to source */
  MIG_RP_MSG_RESUME_ACK,   /* tell source that we are ready to resume */

+MIG_RP_MSG_INITIAL_DATA_LOADED_ACK, /*
+ * Tell source precopy initial data is
+ * loaded.
+ */
+
  MIG_RP_MSG_MAX
  };

@@ -756,6 +761,12 @@ bool migration_has_all_channels(void)
  return true;
  }

+int migrate_send_rp_initial_data_loaded_ack(MigrationIncomingState *mis)
+{
+return migrate_send_rp_message(mis, MIG_RP_MSG_INITIAL_DATA_LOADED_ACK, 0,
+   NULL);
+}
+
  /*
   * Send a 'SHUT' message on the return channel with the given value
   * to indicate that we've finished with the RP.  Non-0 value indicates
@@ -1401,6 +1412,8 @@ void migrate_init(MigrationState *s)
  s->vm_was_running = false;
  s->iteration_initial_bytes = 0;
  s->threshold_size = 0;
+
+s->initial_data_loaded_acked = false;
  }

  int migrate_add_blocker_internal(Error *reason, Error **errp)
@@ -1717,6 +1730,9 @@ static struct rp_cmd_args {
  [MIG_RP_MSG_REQ_PAGES_ID]   = { .len = -1, .name = "REQ_PAGES_ID" },
  [MIG_RP_MSG_RECV_BITMAP]= { .len = -1, .name = "RECV_BITMAP" },
  [MIG_RP_MSG_RESUME_ACK] = { .len =  4, .name = "RESUME_ACK" },
+[MIG_RP_MSG_INITIAL_DATA_LOADED_ACK] = { .len = 0,
+   

Re: [PATCH v3] linux-user: Add /proc/cpuinfo handler for RISC-V

[Palmer Dabbelt]

On Wed, 03 May 2023 08:30:12 PDT (-0700), sch...@suse.de wrote:

From 912af433fa5d93ce81d2054135ed475ab7462d2d Mon Sep 17 00:00:00 2001
From: Andreas Schwab 
Date: Tue, 18 Apr 2023 11:54:01 +0200

Signed-off-by: Andreas Schwab 
---
v3: fix isa order

 linux-user/syscall.c | 55 ++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 69f740ff98..5207259b56 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8231,7 +8231,8 @@ void target_exception_dump(CPUArchState *env, const char 
*fmt, int code)
 }

 #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
-defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
+defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) || \
+defined(TARGET_RISCV)
 static int is_proc(const char *filename, const char *entry)
 {
 return strcmp(filename, entry) == 0;
@@ -8309,6 +8310,56 @@ static int open_cpuinfo(CPUArchState *cpu_env, int fd)
 }
 #endif

+#if defined(TARGET_RISCV)
+static int open_cpuinfo(CPUArchState *cpu_env, int fd)
+{
+int i, num_cpus;
+char isa[32];
+
+#if defined(TARGET_RISCV32)
+strcpy (isa, "rv32");
+#endif
+#if defined(TARGET_RISCV64)
+strcpy (isa, "rv64");
+#endif
+i = strlen (isa);
+if (riscv_has_ext (cpu_env, RVI))
+isa[i++] = 'i';
+if (riscv_has_ext (cpu_env, RVE))
+isa[i++] = 'e';
+if (riscv_has_ext (cpu_env, RVM))
+isa[i++] = 'm';
+if (riscv_has_ext (cpu_env, RVA))
+isa[i++] = 'a';
+if (riscv_has_ext (cpu_env, RVF))
+isa[i++] = 'f';
+if (riscv_has_ext (cpu_env, RVD))
+isa[i++] = 'd';
+if (riscv_has_ext (cpu_env, RVC))
+isa[i++] = 'c';
+if (riscv_has_ext (cpu_env, RVV))
+isa[i++] = 'v';
+isa[i] = 0;
+
+num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
+for (i = 0; i < num_cpus; i++) {
+dprintf(fd, "processor\t: %d\n", i);
+dprintf(fd, "hart\t\t: %d\n", i);
+dprintf(fd, "isa\t\t: %s\n", isa);
+#if defined(TARGET_RISCV32)
+dprintf(fd, "mmu\t\t: sv32\n");
+#endif
+#if defined(TARGET_RISCV64)
+dprintf(fd, "mmu\t\t: sv57\n");
+#endif
+dprintf(fd, "mvendorid\t: 0x0\n");
+dprintf(fd, "marchid\t\t: 0x0\n");
+dprintf(fd, "mimpid\t\t: 0x0\n\n");
+}
+return 0;
+}
+#endif
+
 #if defined(TARGET_M68K)
 static int open_hardware(CPUArchState *cpu_env, int fd)
 {
@@ -8333,7 +8384,7 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, 
const char *pathname, int
 #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
 { "/proc/net/route", open_net_route, is_proc },
 #endif
-#if defined(TARGET_SPARC) || defined(TARGET_HPPA)
+#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined(TARGET_RISCV)
 { "/proc/cpuinfo", open_cpuinfo, is_proc },
 #endif
 #if defined(TARGET_M68K)


Reviewed-by: Palmer Dabbelt 

Thanks!

Re: [PATCH 2/8] migration: Add precopy initial data handshake

[Avihai Horon]

On 03/05/2023 1:54, Peter Xu wrote:

External email: Use caution opening links or attachments


(I've left high level comment in cover letter, but still some quick
comments I noticed when reading)

On Mon, May 01, 2023 at 05:01:35PM +0300, Avihai Horon wrote:

Add precopy initial data handshake between source and destination upon
migration setup. The purpose of the handshake is to notify the
destination that precopy initial data is used and which migration users
(i.e., SaveStateEntry) are going to use it.

The handshake is done in two levels. First, a general enable command is
sent to notify the destination migration code that precopy initial data
is used.
Then, for each migration user in the source that supports precopy
initial data, an enable command is sent to its counterpart in the
destination:
If both support it, precopy initial data will be used for them.
If source doesn't support it, precopy initial data will not be used for
them.
If source supports it and destination doesn't, migration will be failed.

To implement it, a new migration command MIG_CMD_INITIAL_DATA_ENABLE is
added, as well as a new SaveVMHandlers handler initial_data_advise.
Calling the handler advises the migration user that precopy initial data
is used and its return value indicates whether precopy initial data is
supported by it.

Signed-off-by: Avihai Horon 
---
  include/migration/register.h |   6 +++
  migration/migration.h|   3 ++
  migration/savevm.h   |   1 +
  migration/migration.c|   4 ++
  migration/savevm.c   | 102 +++
  migration/trace-events   |   2 +
  6 files changed, 118 insertions(+)

diff --git a/include/migration/register.h b/include/migration/register.h
index a8dfd8fefd..0a73f3883e 100644
--- a/include/migration/register.h
+++ b/include/migration/register.h
@@ -71,6 +71,12 @@ typedef struct SaveVMHandlers {
  int (*load_cleanup)(void *opaque);
  /* Called when postcopy migration wants to resume from failure */
  int (*resume_prepare)(MigrationState *s, void *opaque);
+
+/*
+ * Advises that precopy initial data was requested to be enabled. Returns
+ * true if it's supported or false otherwise. Called both in src and dest.
+ */
+bool (*initial_data_advise)(void *opaque);
  } SaveVMHandlers;

  int register_savevm_live(const char *idstr,
diff --git a/migration/migration.h b/migration/migration.h
index 3a918514e7..4f615e9dbc 100644
--- a/migration/migration.h
+++ b/migration/migration.h
@@ -204,6 +204,9 @@ struct MigrationIncomingState {
   * contains valid information.
   */
  QemuMutex page_request_mutex;
+
+/* Indicates whether precopy initial data was enabled and should be used */
+bool initial_data_enabled;
  };

  MigrationIncomingState *migration_incoming_get_current(void);
diff --git a/migration/savevm.h b/migration/savevm.h
index fb636735f0..d47ab4ad18 100644
--- a/migration/savevm.h
+++ b/migration/savevm.h
@@ -58,6 +58,7 @@ void qemu_savevm_send_postcopy_ram_discard(QEMUFile *f, const 
char *name,
 uint64_t *start_list,
 uint64_t *length_list);
  void qemu_savevm_send_colo_enable(QEMUFile *f);
+void qemu_savevm_send_initial_data_enable(MigrationState *ms, QEMUFile *f);
  void qemu_savevm_live_state(QEMUFile *f);
  int qemu_save_device_state(QEMUFile *f);

diff --git a/migration/migration.c b/migration/migration.c
index abcadbb619..68cdf5b184 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2964,6 +2964,10 @@ static void *migration_thread(void *opaque)
  qemu_savevm_send_colo_enable(s->to_dst_file);
  }

+if (migrate_precopy_initial_data()) {
+qemu_savevm_send_initial_data_enable(s, s->to_dst_file);
+}
+
  qemu_savevm_state_setup(s->to_dst_file);

  qemu_savevm_wait_unplug(s, MIGRATION_STATUS_SETUP,
diff --git a/migration/savevm.c b/migration/savevm.c
index a9181b444b..2740defdf0 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -71,6 +71,13 @@

  const unsigned int postcopy_ram_discard_version;

+typedef struct {
+uint8_t general_enable;
+uint8_t reserved[7];
+uint8_t idstr[256];
+uint32_t instance_id;
+} InitialDataInfo;
+
  /* Subcommands for QEMU_VM_COMMAND */
  enum qemu_vm_cmd {
  MIG_CMD_INVALID = 0,   /* Must be 0 */
@@ -90,6 +97,8 @@ enum qemu_vm_cmd {
  MIG_CMD_ENABLE_COLO,   /* Enable COLO */
  MIG_CMD_POSTCOPY_RESUME,   /* resume postcopy on dest */
  MIG_CMD_RECV_BITMAP,   /* Request for recved bitmap on dst */
+
+MIG_CMD_INITIAL_DATA_ENABLE, /* Enable precopy initial data in dest */
  MIG_CMD_MAX
  };

@@ -109,6 +118,8 @@ static struct mig_cmd_args {
  [MIG_CMD_POSTCOPY_RESUME]  = { .len =  0, .name = "POSTCOPY_RESUME" },
  [MIG_CMD_PACKAGED] = { .len =  4, .name = "PACKAGED" },
  [MIG_CMD_RECV_BITMAP]  = { .len = -1, .name = "RECV_BITMAP" },
+

[PATCH] Add information how to fix common build error on Windows in symlink-install-tree

[Mateusz Krawczuk]

By default, Windows doesn't allow to create soft links for user account and 
only administrator is allowed to do this. To fix this problem you have to raise 
your permissions or enable Developer Mode, which available since Windows 10. 
Additional explanation when build fails will allow developer to fix the problem 
on his computer faster.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1386
Signed-off-by: Mateusz Krawczuk 
---
 scripts/symlink-install-tree.py | 5 +
 1 file changed, 5 insertions(+)

diff --git a/scripts/symlink-install-tree.py b/scripts/symlink-install-tree.py
index 67cb86dd52..bb3a2d708c 100644
--- a/scripts/symlink-install-tree.py
+++ b/scripts/symlink-install-tree.py
@@ -7,12 +7,14 @@
 import subprocess
 import sys
 
+
 def destdir_join(d1: str, d2: str) -> str:
 if not d1:
 return d2
 # c:\destdir + c:\prefix must produce c:\destdir\prefix
 return str(PurePath(d1, *PurePath(d2).parts[1:]))
 
+
 introspect = os.environ.get('MESONINTROSPECT')
 out = subprocess.run([*introspect.split(' '), '--installed'],
  stdout=subprocess.PIPE, check=True).stdout
@@ -28,5 +30,8 @@ def destdir_join(d1: str, d2: str) -> str:
 os.symlink(source, bundle_dest)
 except BaseException as e:
 if not isinstance(e, OSError) or e.errno != errno.EEXIST:
+if os.name == 'nt':
+print('\nPlease enable Developer Mode to support soft link '
+  'without Administrator permission\n')
 print(f'error making symbolic link {dest}', file=sys.stderr)
 raise e
-- 
2.40.1.windows.1

[PATCH v3] linux-user: Add /proc/cpuinfo handler for RISC-V

[Andreas Schwab]

>From 912af433fa5d93ce81d2054135ed475ab7462d2d Mon Sep 17 00:00:00 2001
From: Andreas Schwab 
Date: Tue, 18 Apr 2023 11:54:01 +0200

Signed-off-by: Andreas Schwab 
---
v3: fix isa order

 linux-user/syscall.c | 55 ++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 69f740ff98..5207259b56 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8231,7 +8231,8 @@ void target_exception_dump(CPUArchState *env, const char 
*fmt, int code)
 }
 
 #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
-defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
+defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) || \
+defined(TARGET_RISCV)
 static int is_proc(const char *filename, const char *entry)
 {
 return strcmp(filename, entry) == 0;
@@ -8309,6 +8310,56 @@ static int open_cpuinfo(CPUArchState *cpu_env, int fd)
 }
 #endif
 
+#if defined(TARGET_RISCV)
+static int open_cpuinfo(CPUArchState *cpu_env, int fd)
+{
+int i, num_cpus;
+char isa[32];
+
+#if defined(TARGET_RISCV32)
+strcpy (isa, "rv32");
+#endif
+#if defined(TARGET_RISCV64)
+strcpy (isa, "rv64");
+#endif
+i = strlen (isa);
+if (riscv_has_ext (cpu_env, RVI))
+isa[i++] = 'i';
+if (riscv_has_ext (cpu_env, RVE))
+isa[i++] = 'e';
+if (riscv_has_ext (cpu_env, RVM))
+isa[i++] = 'm';
+if (riscv_has_ext (cpu_env, RVA))
+isa[i++] = 'a';
+if (riscv_has_ext (cpu_env, RVF))
+isa[i++] = 'f';
+if (riscv_has_ext (cpu_env, RVD))
+isa[i++] = 'd';
+if (riscv_has_ext (cpu_env, RVC))
+isa[i++] = 'c';
+if (riscv_has_ext (cpu_env, RVV))
+isa[i++] = 'v';
+isa[i] = 0;
+
+num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
+for (i = 0; i < num_cpus; i++) {
+dprintf(fd, "processor\t: %d\n", i);
+dprintf(fd, "hart\t\t: %d\n", i);
+dprintf(fd, "isa\t\t: %s\n", isa);
+#if defined(TARGET_RISCV32)
+dprintf(fd, "mmu\t\t: sv32\n");
+#endif
+#if defined(TARGET_RISCV64)
+dprintf(fd, "mmu\t\t: sv57\n");
+#endif
+dprintf(fd, "mvendorid\t: 0x0\n");
+dprintf(fd, "marchid\t\t: 0x0\n");
+dprintf(fd, "mimpid\t\t: 0x0\n\n");
+}
+return 0;
+}
+#endif
+
 #if defined(TARGET_M68K)
 static int open_hardware(CPUArchState *cpu_env, int fd)
 {
@@ -8333,7 +8384,7 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, 
const char *pathname, int
 #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
 { "/proc/net/route", open_net_route, is_proc },
 #endif
-#if defined(TARGET_SPARC) || defined(TARGET_HPPA)
+#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined(TARGET_RISCV)
 { "/proc/cpuinfo", open_cpuinfo, is_proc },
 #endif
 #if defined(TARGET_M68K)
-- 
2.40.1


-- 
Andreas Schwab, SUSE Labs, sch...@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Re: [PATCH 0/8] migration: Add precopy initial data capability and VFIO precopy support

[Avihai Horon]

On 03/05/2023 1:49, Peter Xu wrote:

External email: Use caution opening links or attachments


On Mon, May 01, 2023 at 05:01:33PM +0300, Avihai Horon wrote:

Hello everyone,

Hi, Avihai,


=== Flow of operation ===

To use precopy initial data, the capability must be enabled in the
source.

As this capability must be supported also in the destination, a
handshake is performed during migration setup. The purpose of the
handshake is to notify the destination that precopy initial data is used
and to check if it's supported.

The handshake is done in two levels. First, a general handshake is done
with the destination migration code to notify that precopy initial data
is used. Then, for each migration user in the source that supports
precopy initial data, a handshake is done with its counterpart in the
destination:
If both support it, precopy initial data will be used for them.
If source doesn't support it, precopy initial data will not be used for
them.
If source supports it and destination doesn't, migration will be failed.

Assuming the handshake succeeded, migration starts to send precopy data
and as part of it also the initial precopy data. Initial precopy data is
just like any other precopy data and as such, migration code is not
aware of it. Therefore, it's the responsibility of the migration users
(such as VFIO devices) to notify their counterparts in the destination
that their initial precopy data has been sent (for example, VFIO
migration does it when its initial bytes reach zero).

In the destination, migration code will query each migration user that
supports precopy initial data and check if its initial data has been
loaded. If initial data has been loaded by all of them, an ACK will be
sent to the source which will now be able to complete migration when
appropriate.

I can understand why this is useful, what I'm not 100% sure is whether the
complexity is needed.  The idea seems to be that src never switchover
unless it receives a READY notification from dst.

I'm imaging below simplified and more general workflow, not sure whether it
could work for you:

   - Introduce a new cap "switchover-ready", it means whether there'll be a
 ready event sent from dst -> src for "being ready for switchover"

   - When cap set, a new msg MIG_RP_MSG_SWITCHOVER_READY is defined and
 handled on src showing that dest is ready for switchover. It'll be sent
 only if dest is ready for the switchover

   - Introduce a field SaveVMHandlers.explicit_switchover_needed.  For each
 special device like vfio that would like to participate in the decision
 making, device can set its explicit_switchover_needed=1.  This field is
 ignored if the new cap is not set.

   - Dst qemu: when new cap set, remember how many special devices are there
 requesting explicit switchover (count of SaveVMHandlers that has the
 bit set during load setup) as switch_over_pending=N.

   - Dst qemu: Once a device thinks its fine to switchover (probably in the
 load_state() callback), it calls migration_notify_switchover_ready().
 That decreases switch_over_pending and when it hits zero, one msg
 MIG_RP_MSG_SWITCHOVER_READY will be sent to src.

Only until READY msg received on src could src switchover the precopy to
dst.

Then it only needs 1 more field in SaveVMHandlers rather than 3, and only 1
more msg (dst->src).

This is based on the fact that right now we always set caps on both qemus
so I suppose it already means either both have or don't have the feature
(even if one has, not setting the cap means disabled on both).

Would it work for this case and cleaner?


Hi Peter, thanks for the response!
Your approach is indeed much simpler, however I have a few concerns 
regarding compatibility.


You are saying that caps are always set both in src and dest.
But what happens if we set the cap only on one side?
Should we care about these scenarios?
For example, if we set the cap only in src, then src will wait 
indefinitely for dest to notify that switchover is ready.
Would you expect migration to fail instead of just keep running 
indefinitely?
In current approach we only need to enable the cap in the source, so 
such scenario can't happen.


Let's look at some other scenario.
Src QEMU supports explicit-switchover for device X but *not* for device 
Y (i.e., src QEMU is some older version of QEMU that supports 
explicit-switchover for device X but not for Y).

Dest QEMU supports explicit-switchover for device X and device Y.
The capability is set in both src and dest.
In the destination we will have switchover_pending=2 because both X and 
Y support explicit-switchover.
We do migration, but switchover_pending will never reach 0 because only 
X supports it in the source, so the migration will run indefinitely.
The per-device handshake solves this by making device Y not use 
explicit-switchover in this case.


Thanks.

Re: [RFC PATCH 0/3] QEMU ACPI generic port support

[Dave Jiang]

On 5/3/23 3:42 AM, Jonathan Cameron wrote:

On Tue, 18 Apr 2023 15:21:36 -0700
Dave Jiang  wrote:


s small RFC patch series is really a hack on what I need from qemu rather
than a proper implementation. I'm hoping to get some guidance from the list on
how to implement this correctly for qemu upstream. Thank you!

The patch series provides support for the ACPI Generic Port support that's
defined by ACPI spec 6.5 5.2.16.7 (Generic Port Affinity Structure). The
series also adds a genport object that allows locality data to be injected via
qemu commandline to the HMAT tables. The generic port support is to allow a hot
plugged CXL memory device to calculate the locality data from the CPU to
the CXL device. The generic port related data provides the locality data from
the CPU to the CXL host bridge (latency and bandwidth). These data in
addition to the PCIe link data, CDAT from device, and CXL switch CDAT if switch
exist, provides the locality data for the entire path.

Patch1: Adds Generic Port Affinity Structure sub-tables to the SRAT. For
each CXL Host Bridge (HB) a GPAS entry is created with a unique proximity
domain. For example, if the system is created with 4 proximity domains (PXM) for
system memory, then the next GPAS will get PXM 4 and so on.


I may be going crazy but I'm not seeing an increment on the numa node. So I 
think
they all get 4 at the moment. Found it increment in patch 3.



Patch2: Add the json support for generic port. Split out because
clang-format really clobbers the json files.

Patch3: Add a generic port object. The intention here is to allow setup of
numa nodes, add hmat-lb data and node distance for the generic targets. I had to
add a hack in qemu_create_cli_devices() to realize the genport objects. I need
guidance on where and how to do this properly so the genport objects
realize at the correct place and time.

Example of genport setup:
-object genport,id=$X -numa node,genport=genport$X,nodeid=$Y,initiator=$Z
-numa 
hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-latency,latency=$latency
-numa 
hmat-lb,initiator=$Z,target=$X,hierarchy=memory,data-type=access-bandwidth,bandwidth=$bandwidthM


I think we should be using some links to the host bridges in here.
So I don't think there should be an explicit genport object at all.
Instead we should be able to point at the pxb itself.  Perhaps also
allowing other port types in future.

Something like

-device pxb-cxl,id=cxl1.1
-numa node,genport=cxl1.1,nodeid=$X
-numa hmat-lb,initiator=$Z,target=$X,...
-numa hmat-lb,initiator=$X,target=$Y,...
//as generic port goes both ways.

As we are currently using bus_nr for UID (which is admittedly a somewhat dirty 
hack that
just happened to be convenient) the ACPI building code can use that to fill in 
the SRAT
entry at appropriate point.

I haven't tried implementing it so there may well be some ordering issues that
require some late binding etc, but it should be possible to make it work.


Ok thanks for the suggestion Jonathan! I appreciate you looking over 
this. I'll take a look when I get a chance.





for ((i = 0; i < total_nodes; i++)); do
 for ((j = 0; j < cxl_hbs; j++ )); do# 2 CXL HBs
 -numa dist,src=$i,dst=$X,val=$dist
 done
done
Linux kernel support:
https://lore.kernel.org/linux-cxl/168088732996.1441063.10107817505475386072.stgit@djiang5-mobl3/T/#t

---

Dave Jiang (3):
   hw/acpi: Add support for Generic Port Affinity Structure to SRAT
   genport: Add json support for generic port
   acpi: add generic port device object


  hw/acpi/aml-build.c | 21 +
  hw/acpi/genport.c   | 61 +
  hw/acpi/meson.build |  1 +
  hw/i386/acpi-build.c| 45 +++
  include/hw/acpi/aml-build.h | 27 
  qapi/machine.json   |  3 +-
  qapi/qom.json   | 12 
  softmmu/vl.c| 26 
  8 files changed, 195 insertions(+), 1 deletion(-)
  create mode 100644 hw/acpi/genport.c

--

Re: [PATCH v3 1/2] igb: RX descriptors handling cleanup

[Akihiko Odaki]

On 2023/05/03 16:46, Sriram Yagnaraman wrote:




-Original Message-
From: Tomasz Dzieciol/VIM Integration (NC) /SRPOL/Engineer/Samsung
Electronics 
Sent: Tuesday, 2 May 2023 16:01
To: Sriram Yagnaraman ; qemu-
de...@nongnu.org; akihiko.od...@daynix.com
Cc: jasow...@redhat.com; k.kwiec...@samsung.com;
m.socha...@samsung.com
Subject: RE: [PATCH v3 1/2] igb: RX descriptors handling cleanup

Not Linux/DPDK/FreeBSD for IGB.

Change here adds additional condition (RXCSUM.IPPCSE set) to enable putting
IP ID into descriptor, besides clearing RXCSUM.PCSD (required according to
Intel 82576 datasheet) that was not present in the e1000e code.



Yes, we can't even use ethtool to set this field.
My suggestion is to not add/maintain code that we cannot test. I leave it up to 
Akhikho to decide if we really need to implement IPPCSE.
The default value of RXCSUM.IPPCSE is unset, so we could as well ignore this 
field until there is a user who sets this.


In general I won't reject a patch to implement a feature not used by a 
known guest, but I don't recommend that. It just doesn't make sense to 
spend time to write code that can turn out so buggy that it is unusable 
in practice, which is often the case with untested code.

Re: [PATCH v1 1/1] hw/pci: Disable PCI_ERR_UNCOR_MASK register for machine type < 8.0

[Peter Xu]

On Tue, May 02, 2023 at 09:27:02PM -0300, Leonardo Bras wrote:
> Since it's implementation on v8.0.0-rc0, having the PCI_ERR_UNCOR_MASK
> set for machine types < 8.0 will cause migration to fail if the target
> QEMU version is < 8.0.0 :
> 
> qemu-system-x86_64: get_pci_config_device: Bad config data: i=0x10a read: 40 
> device: 0 cmask: ff wmask: 0 w1cmask:0
> qemu-system-x86_64: Failed to load PCIDevice:config
> qemu-system-x86_64: Failed to load e1000e:parent_obj
> qemu-system-x86_64: error while loading state for instance 0x0 of device 
> ':00:02.0/e1000e'
> qemu-system-x86_64: load of migration failed: Invalid argument
> 
> The above test migrated a 7.2 machine type from QEMU master to QEMU 7.2.0,
> with this cmdline:
> 
> ./qemu-system-x86_64 -M pc-q35-7.2 [-incoming XXX]
> 
> In order to fix this, property x-pcie-err-unc-mask was introduced to
> control when PCI_ERR_UNCOR_MASK is enabled. This property is enabled by
> default, but is disabled if machine type <= 7.2.
> 
> Fixes: 010746ae1d ("hw/pci/aer: Implement PCI_ERR_UNCOR_MASK register")
> Suggested-by: Michael S. Tsirkin 
> Signed-off-by: Leonardo Bras 

Reviewed-by: Peter Xu 

-- 
Peter Xu

Re: [PATCH v2] linux-user: Add /proc/cpuinfo handler for RISC-V

[Palmer Dabbelt]

On Wed, 03 May 2023 04:20:09 PDT (-0700), sch...@suse.de wrote:

Signed-off-by: Andreas Schwab 
---
v2: dynmically compute the isa string

 linux-user/syscall.c | 55 ++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 69f740ff98..6df138c8b6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -8231,7 +8231,8 @@ void target_exception_dump(CPUArchState *env, const char 
*fmt, int code)
 }

 #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN || \
-defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA)
+defined(TARGET_SPARC) || defined(TARGET_M68K) || defined(TARGET_HPPA) || \
+defined(TARGET_RISCV)
 static int is_proc(const char *filename, const char *entry)
 {
 return strcmp(filename, entry) == 0;
@@ -8309,6 +8310,56 @@ static int open_cpuinfo(CPUArchState *cpu_env, int fd)
 }
 #endif

+#if defined(TARGET_RISCV)
+static int open_cpuinfo(CPUArchState *cpu_env, int fd)
+{
+int i, num_cpus;
+char isa[32];
+
+#if defined(TARGET_RISCV32)
+strcpy (isa, "rv32");
+#endif
+#if defined(TARGET_RISCV64)
+strcpy (isa, "rv64");
+#endif
+i = strlen (isa);
+if (riscv_has_ext (cpu_env, RVI))
+isa[i++] = 'i';
+if (riscv_has_ext (cpu_env, RVE))
+isa[i++] = 'e';
+if (riscv_has_ext (cpu_env, RVM))
+isa[i++] = 'm';
+if (riscv_has_ext (cpu_env, RVA))
+isa[i++] = 'a';
+if (riscv_has_ext (cpu_env, RVF))
+isa[i++] = 'f';
+if (riscv_has_ext (cpu_env, RVD))
+isa[i++] = 'd';
+if (riscv_has_ext (cpu_env, RVV))
+isa[i++] = 'v';
+if (riscv_has_ext (cpu_env, RVC))
+isa[i++] = 'c';


Oddly enough, pretty much the only "must" in the ISA string rules is the 
ordering of extensions and it's C before V


   \caption{Standard ISA extension names.  The table also defines the
 canonical order in which extension names must appear in the name
 string, with top-to-bottom in table indicating first-to-last in the
 name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is not.}

I guess that assumes figure captions are normative?  I'm not sure we get 
into that level of detail, though.



+isa[i] = 0;
+
+num_cpus = sysconf(_SC_NPROCESSORS_ONLN);
+for (i = 0; i < num_cpus; i++) {
+dprintf(fd, "processor\t: %d\n", i);
+dprintf(fd, "hart\t\t: %d\n", i);
+dprintf(fd, "isa\t\t: %s\n", isa);
+#if defined(TARGET_RISCV32)
+dprintf(fd, "mmu\t\t: sv32\n");
+#endif
+#if defined(TARGET_RISCV64)
+dprintf(fd, "mmu\t\t: sv57\n");
+#endif
+dprintf(fd, "mvendorid\t: 0x0\n");
+dprintf(fd, "marchid\t\t: 0x0\n");
+dprintf(fd, "mimpid\t\t: 0x0\n\n");
+}
+return 0;
+}
+#endif
+
 #if defined(TARGET_M68K)
 static int open_hardware(CPUArchState *cpu_env, int fd)
 {
@@ -8333,7 +8384,7 @@ static int do_openat(CPUArchState *cpu_env, int dirfd, 
const char *pathname, int
 #if HOST_BIG_ENDIAN != TARGET_BIG_ENDIAN
 { "/proc/net/route", open_net_route, is_proc },
 #endif
-#if defined(TARGET_SPARC) || defined(TARGET_HPPA)
+#if defined(TARGET_SPARC) || defined(TARGET_HPPA) || defined(TARGET_RISCV)
 { "/proc/cpuinfo", open_cpuinfo, is_proc },
 #endif
 #if defined(TARGET_M68K)


Aside from that,

Reviewed-by: Palmer Dabbelt 

Thanks!

Re: [PATCH 05/22] hw/arm: Select VIRTIO_NET for virt machine

[Fabiano Rosas]

Paolo Bonzini  writes:

> On 5/3/23 11:12, Alex Bennée wrote:
>> From: Fabiano Rosas 
>> 
>> The 'virt' machine uses virtio-net-pci as a fallback when no other
>> network driver has been selected via command line. Select VIRTIO_NET
>> and VIRTIO_PCI from CONFIG_ARM_VIRT to avoid errors when PCI_DEVICES=n
>> (due to e.g. --without-default-devices):
>> 
>> $ ./qemu-system-aarch64 -M virt -accel tcg -cpu max
>> qemu-system-aarch64: Unsupported NIC model: virtio-net-pci
>
> With respect to patches 5-17, very few devices need to be present when 
> configuring --without-default-devices, and thus need to be "select"ed by 
> Kconfig.  You should select a device only if you cannot even start the 
> machine without --nodefaults.
>

There are some devices that are not explicitly under the scope of
-nodefaults, i.e. they are not part of the "default" logic at vl.c, but
still some code deep within QEMU uses them as fallback in some
situations.

> Anything else should be added by hand to configs/ if you use 
> --nodefaults.  In particular, failures of "make check" when configured 
> --without-default-devices are *test* bugs, not configuration bugs.
>

Yes, that makes sense, just keep in mind that this have lead to us not
testing the --without-default-devices build and people just assuming
some devices will always be present. So there's genuine scenarios of us
providing a CONFIG that can never be turned off because everything
breaks.

Re: [PULL 0/8] Migration 20230428 patches

[Richard Henderson]

On 5/3/23 10:25, Juan Quintela wrote:

The following changes since commit 4ebc33f3f3b656ebf62112daca6aa0f8019b4891:

   Merge tag 'pull-tcg-20230502-2' ofhttps://gitlab.com/rth7680/qemu  into 
staging (2023-05-02 21:18:45 +0100)

are available in the Git repository at:

   https://gitlab.com/juan.quintela/qemu.git  
tags/migration-20230428-pull-request

for you to fetch changes up to 0deb7e9b6cfdb39d89a629e705d6176c511fa83e:

   migration: Drop unused parameter for migration_tls_client_create() 
(2023-05-03 11:24:20 +0200)


Migraiton Pull request (20230428 take 2)

Hi

Dropped the compression cleanups to see if we find what is going on.

Please apply.

Later, Juan.


Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/8.1 as 
appropriate.


r~

Re: [PATCH] linux-user: Add /proc/cpuinfo handler for RISC-V

[Palmer Dabbelt]

On Wed, 03 May 2023 04:13:55 PDT (-0700), sch...@suse.de wrote:

On Mai 02 2023, Andreas Schwab wrote:


None of the currently defined cpus are non-GC cpus (except sifive_e, but
that is not suitable for user-space anyway), and there doesn't appear to
be any properties defined for changing the supported extensions.


Actually, modifying the extensions is possible for the base cpus (rv32
and rv64).


Ya, though I think you're right about the VA width in userspace.

[PATCH 2/2] gitlab: ensure coverage job also publishes meson log

[Daniel P . Berrangé]

The coverage job wants to publish a coverage report on success, but the
tests might fail and in that case we need the meson logs for debugging.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.d/buildtest.yml | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0bb5cd56f9..b6390e3562 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -462,9 +462,12 @@ gcov:
   coverage: /^\s*lines:\s*\d+.\d+\%/
   artifacts:
 name: ${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA}
-when: on_success
+when: always
 expire_in: 2 days
+paths:
+  - build/meson-logs/testlog.txt
 reports:
+  junit: build/meson-logs/testlog.junit.xml
   coverage_report:
 coverage_format: cobertura
 path: build/coverage.xml
-- 
2.40.1

[PATCH 1/2] tests: libvirt-ci: Update to commit 'c8971e90ac' to pull in mformat and xorriso

[Ani Sinha]

Pull in the following changes from lcitool:

* tests/lcitool/libvirt-ci 85487e1...c8971e9 (18):
  > mappings: add new package mappings for mformat and xorriso
  > docs: testing: Update contents with tox
  > .gitlab-ci.yml: Always test against installed lcitool
  > gitlab-ci.yml: Start using tox for testing
  > tox: Allow running with custom pytest options with {posargs}
  > gitignore: Add the default .tox directory
  > dev-requirements: Reference VM requirements
  > requirements: Add tox to dev-requirements.txt and drop pytest and flake
  > test-requirements: Rename to dev-requirements.txt
  > Add tox.ini configuration file
  > tests: commands: Consolidate the installed package/run from git tests
  > Add a pytest.ini
  > facts: targets: Drop Fedora 36 target
  > gitlab-ci.yml: Add Fedora 38 target
  > facts: targets: Add Fedora 38
  > facts: mappings: Drop 'zstd' mapping
  > facts: projects: nbdkit: Replace zstd mapping with libzstd
  > docs: mappings: Add a section on the preferred mapping naming scheme

CC: m...@redhat.com
CC: berra...@redhat.com
Signed-off-by: Ani Sinha 
---
 tests/lcitool/libvirt-ci | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci
index 85487e1404..c8971e90ac 16
--- a/tests/lcitool/libvirt-ci
+++ b/tests/lcitool/libvirt-ci
@@ -1 +1 @@
-Subproject commit 85487e140415b2ac54b01a9a6b600fd7c21edc2f
+Subproject commit c8971e90ac169ee2b539c747f74d96c876debdf9
-- 
2.31.1

[PATCH 0/2] Add mformat and xorriso dependencies in containers

[Ani Sinha]

mformat and xorriso tools are needed by biosbits avocado tests. This patchset
adds those two tools in the docker container images.
Patch 1 pulls in the latest changes in lcitool in order to add mappings
for these packages in various distros.
Patch 2 updates all Dockerfiles in QEMU repository to add these two
tools.

CC: m...@redhat.com
CC: berra...@redhat.com

Ani Sinha (2):
  tests: libvirt-ci: Update to commit 'c8971e90ac' to pull in mformat
and xorriso
  tests/lcitool: Add mtools and xorriso as dependency for bios bits
avocado tests

 .gitlab-ci.d/cirrus/freebsd-13.vars   | 2 +-
 .gitlab-ci.d/cirrus/macos-12.vars | 2 +-
 tests/docker/dockerfiles/alpine.docker| 2 ++
 tests/docker/dockerfiles/centos8.docker   | 2 ++
 tests/docker/dockerfiles/debian-amd64-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-amd64.docker  | 2 ++
 tests/docker/dockerfiles/debian-arm64-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-armel-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-armhf-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-mips64el-cross.docker | 2 ++
 tests/docker/dockerfiles/debian-mipsel-cross.docker   | 2 ++
 tests/docker/dockerfiles/debian-ppc64el-cross.docker  | 2 ++
 tests/docker/dockerfiles/debian-s390x-cross.docker| 2 ++
 tests/docker/dockerfiles/fedora-win32-cross.docker| 2 ++
 tests/docker/dockerfiles/fedora-win64-cross.docker| 2 ++
 tests/docker/dockerfiles/fedora.docker| 2 ++
 tests/docker/dockerfiles/opensuse-leap.docker | 2 ++
 tests/docker/dockerfiles/ubuntu2004.docker| 2 ++
 tests/docker/dockerfiles/ubuntu2204.docker| 2 ++
 tests/lcitool/libvirt-ci  | 2 +-
 tests/lcitool/projects/qemu.yml   | 2 ++
 21 files changed, 39 insertions(+), 3 deletions(-)

-- 
2.31.1

[PATCH 2/2] tests/lcitool: Add mtools and xorriso as dependency for bios bits avocado tests

[Ani Sinha]

Bios bits avocado tests need mformat (provided by the mtools package) and
xorriso tools in order to run within gitlab CI containers. Add those
dependencies within the Dockerfiles so that containers can be built with
those tools present.

CC: m...@redhat.com
CC: berra...@redhat.com
Signed-off-by: Ani Sinha 
---
 .gitlab-ci.d/cirrus/freebsd-13.vars   | 2 +-
 .gitlab-ci.d/cirrus/macos-12.vars | 2 +-
 tests/docker/dockerfiles/alpine.docker| 2 ++
 tests/docker/dockerfiles/centos8.docker   | 2 ++
 tests/docker/dockerfiles/debian-amd64-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-amd64.docker  | 2 ++
 tests/docker/dockerfiles/debian-arm64-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-armel-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-armhf-cross.docker| 2 ++
 tests/docker/dockerfiles/debian-mips64el-cross.docker | 2 ++
 tests/docker/dockerfiles/debian-mipsel-cross.docker   | 2 ++
 tests/docker/dockerfiles/debian-ppc64el-cross.docker  | 2 ++
 tests/docker/dockerfiles/debian-s390x-cross.docker| 2 ++
 tests/docker/dockerfiles/fedora-win32-cross.docker| 2 ++
 tests/docker/dockerfiles/fedora-win64-cross.docker| 2 ++
 tests/docker/dockerfiles/fedora.docker| 2 ++
 tests/docker/dockerfiles/opensuse-leap.docker | 2 ++
 tests/docker/dockerfiles/ubuntu2004.docker| 2 ++
 tests/docker/dockerfiles/ubuntu2204.docker| 2 ++
 tests/lcitool/projects/qemu.yml   | 2 ++
 20 files changed, 38 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars 
b/.gitlab-ci.d/cirrus/freebsd-13.vars
index 7622c849b2..aea3a25fb3 100644
--- a/.gitlab-ci.d/cirrus/freebsd-13.vars
+++ b/.gitlab-ci.d/cirrus/freebsd-13.vars
@@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake'
 NINJA='/usr/local/bin/ninja'
 PACKAGING_COMMAND='pkg'
 PIP3='/usr/local/bin/pip-3.8'
-PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache 
cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc flex 
fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi 
libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm 
lzo2 meson ncurses nettle ninja opencv pixman pkgconf png py39-numpy 
py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 
rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir 
virglrenderer vte3 zstd'
+PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache 
cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc flex 
fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi 
libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm 
lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy 
py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 
rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir 
virglrenderer vte3 xorriso zstd'
 PYPI_PKGS=''
 PYTHON='/usr/local/bin/python3'
diff --git a/.gitlab-ci.d/cirrus/macos-12.vars 
b/.gitlab-ci.d/cirrus/macos-12.vars
index da6aa6469b..ceb294e153 100644
--- a/.gitlab-ci.d/cirrus/macos-12.vars
+++ b/.gitlab-ci.d/cirrus/macos-12.vars
@@ -11,6 +11,6 @@ MAKE='/opt/homebrew/bin/gmake'
 NINJA='/opt/homebrew/bin/ninja'
 PACKAGING_COMMAND='brew'
 PIP3='/opt/homebrew/bin/pip3'
-PKGS='bash bc bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc 
flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c 
libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 
libusb llvm lzo make meson ncurses nettle ninja pixman pkg-config python3 
rpm2cpio sdl2 sdl2_image snappy socat sparse spice-protocol tesseract usbredir 
vde vte3 zlib zstd'
+PKGS='bash bc bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc 
flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c 
libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 
libusb llvm lzo make meson mtools ncurses nettle ninja pixman pkg-config 
python3 rpm2cpio sdl2 sdl2_image snappy socat sparse spice-protocol tesseract 
usbredir vde vte3 xorriso zlib zstd'
 PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme'
 PYTHON='/opt/homebrew/bin/python3'
diff --git a/tests/docker/dockerfiles/alpine.docker 
b/tests/docker/dockerfiles/alpine.docker
index 81c70aeaf9..40ac3af096 100644
--- a/tests/docker/dockerfiles/alpine.docker
+++ b/tests/docker/dockerfiles/alpine.docker
@@ -67,6 +67,7 @@ RUN apk update && \
 make \
 mesa-dev \
 meson \
+mtools \
 multipath-tools \
 musl-dev \
 ncurses-dev \
@@ -108,6 +109,7 @@ RUN apk update && \
 which \
 xen-dev \
 xfsprogs-dev \
+xorriso \
 zlib-dev \
 zlib-static \
 zstd \
diff --git a/tests/docker/dockerfiles/centos8.docker 

[PATCH 0/2] gitlab: improve artifact handling

[Daniel P . Berrangé]

We are missing test log artifacts from various check jobs on failure,
and also missing test logs from the coverage job

Daniel P. Berrangé (2):
  gitlab: explicit set artifacts publishing criteria
  gitlab: ensure coverage job also publishes meson log

 .gitlab-ci.d/buildtest-template.yml  | 4 +++-
 .gitlab-ci.d/buildtest.yml   | 5 +
 .gitlab-ci.d/crossbuild-template.yml | 1 +
 .gitlab-ci.d/crossbuilds.yml | 2 ++
 .gitlab-ci.d/custom-runners.yml  | 1 +
 .gitlab-ci.d/opensbi.yml | 1 +
 6 files changed, 13 insertions(+), 1 deletion(-)

-- 
2.40.1

[PATCH 1/2] gitlab: explicit set artifacts publishing criteria

[Daniel P . Berrangé]

If not set explicitly, gitlab assumes 'when: on_success" as the
publishing criteria for artifacts. This is reasonable if the
artifact is an output deliverable of the job. This is useless
if the artifact is a log file to be used for debugging job
failures.

This change makes the desired criteria explicit for every job
that publishes artifacts.

Signed-off-by: Daniel P. Berrangé 
---
 .gitlab-ci.d/buildtest-template.yml  | 4 +++-
 .gitlab-ci.d/buildtest.yml   | 2 ++
 .gitlab-ci.d/crossbuild-template.yml | 1 +
 .gitlab-ci.d/crossbuilds.yml | 2 ++
 .gitlab-ci.d/custom-runners.yml  | 1 +
 .gitlab-ci.d/opensbi.yml | 1 +
 6 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.d/buildtest-template.yml 
b/.gitlab-ci.d/buildtest-template.yml
index a6cfe9be97..d90bd6419f 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -29,6 +29,7 @@
 # rebuilding all the object files we skip in the artifacts
 .native_build_artifact_template:
   artifacts:
+when: on_success
 expire_in: 2 days
 paths:
   - build
@@ -57,6 +58,7 @@
   extends: .common_test_job_template
   artifacts:
 name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
+when: always
 expire_in: 7 days
 paths:
   - build/meson-logs/testlog.txt
@@ -72,7 +74,7 @@
 policy: pull-push
   artifacts:
 name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
-when: on_failure
+when: always
 expire_in: 7 days
 paths:
   - build/tests/results/latest/results.xml
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index bb3650a51c..0bb5cd56f9 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -462,6 +462,7 @@ gcov:
   coverage: /^\s*lines:\s*\d+.\d+\%/
   artifacts:
 name: ${CI_JOB_NAME}-${CI_COMMIT_REF_NAME}-${CI_COMMIT_SHA}
+when: on_success
 expire_in: 2 days
 reports:
   coverage_report:
@@ -587,6 +588,7 @@ pages:
 - make -C build install DESTDIR=$(pwd)/temp-install
 - mv temp-install/usr/local/share/doc/qemu/* public/
   artifacts:
+when: on_success
 paths:
   - public
   variables:
diff --git a/.gitlab-ci.d/crossbuild-template.yml 
b/.gitlab-ci.d/crossbuild-template.yml
index 4f93b9e4e5..a7e34e0145 100644
--- a/.gitlab-ci.d/crossbuild-template.yml
+++ b/.gitlab-ci.d/crossbuild-template.yml
@@ -55,6 +55,7 @@
 .cross_test_artifacts:
   artifacts:
 name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
+when: always
 expire_in: 7 days
 paths:
   - build/meson-logs/testlog.txt
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 61b8ac86ee..ee4c1b74d9 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -161,6 +161,7 @@ cross-win32-system:
 CROSS_SKIP_TARGETS: alpha-softmmu avr-softmmu hppa-softmmu m68k-softmmu
 microblazeel-softmmu mips64el-softmmu nios2-softmmu
   artifacts:
+when: on_success
 paths:
   - build/qemu-setup*.exe
 
@@ -176,6 +177,7 @@ cross-win64-system:
 or1k-softmmu rx-softmmu sh4eb-softmmu sparc64-softmmu
 tricore-softmmu xtensaeb-softmmu
   artifacts:
+when: on_success
 paths:
   - build/qemu-setup*.exe
 
diff --git a/.gitlab-ci.d/custom-runners.yml b/.gitlab-ci.d/custom-runners.yml
index 34a1e6f327..a0aef96a07 100644
--- a/.gitlab-ci.d/custom-runners.yml
+++ b/.gitlab-ci.d/custom-runners.yml
@@ -20,6 +20,7 @@ variables:
   artifacts:
 name: "$CI_JOB_NAME-$CI_COMMIT_REF_SLUG"
 expire_in: 7 days
+when: always
 paths:
   - build/meson-logs/testlog.txt
 reports:
diff --git a/.gitlab-ci.d/opensbi.yml b/.gitlab-ci.d/opensbi.yml
index 9a651465d8..867e34c19f 100644
--- a/.gitlab-ci.d/opensbi.yml
+++ b/.gitlab-ci.d/opensbi.yml
@@ -65,6 +65,7 @@ build-opensbi:
   stage: build
   needs: ['docker-opensbi']
   artifacts:
+when: on_success
 paths: # 'artifacts.zip' will contains the following files:
   - pc-bios/opensbi-riscv32-generic-fw_dynamic.bin
   - pc-bios/opensbi-riscv64-generic-fw_dynamic.bin
-- 
2.40.1

Re: [PULL 11/18] migration: Create migrate_block_bitmap_mapping() function

[Juan Quintela]

Kevin Wolf  wrote:
> Am 27.04.2023 um 17:22 hat Juan Quintela geschrieben:
>> Notice that we changed the test of ->has_block_bitmap_mapping
>> for the test that block_bitmap_mapping is not NULL.
>> 
>> Signed-off-by: Juan Quintela 
>> Reviewed-by: Vladimir Sementsov-Ogievskiy 
>> 
>> ---
>> 
>> Make it return const (vladimir)
>
> (I don't think this part was actually meant for the commit message)
>
> This commit broke qemu-iotests 300 on master. Please have a look.
>
> Kevin

grrr

selfNack

Just wondering, make check don't run this?

I run "make check" before I sent the pull request.

Later, Juan.

Re: [PULL 11/18] migration: Create migrate_block_bitmap_mapping() function

[Kevin Wolf]

Am 27.04.2023 um 17:22 hat Juan Quintela geschrieben:
> Notice that we changed the test of ->has_block_bitmap_mapping
> for the test that block_bitmap_mapping is not NULL.
> 
> Signed-off-by: Juan Quintela 
> Reviewed-by: Vladimir Sementsov-Ogievskiy 
> 
> ---
> 
> Make it return const (vladimir)

(I don't think this part was actually meant for the commit message)

This commit broke qemu-iotests 300 on master. Please have a look.

Kevin

Re: [PATCH 0/2] tcg: ppc64: Fix mask generation for vextractdm

[Daniel Henrique Barboza]

Shiva,

I just queued patch 1 adding this line in the commit msg:

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

This was mentioned by Cedric in patch 2. Also, speaking of patch 2, take a look
on Cedric's review and see if it's applicable or not.

I plan to send a ppc pull request at the end of the week.


Thanks,


Daniel


On 4/13/23 16:00, Shivaprasad G Bhat wrote:

While debugging gitlab issue[1] 1536, I happen to try the
vextract[X]m instructions on the real hardware. The test
used in [1] is failing for vextractdm.

On debugging it is seen, in function do_extractm() the
mask is calculated as dup_const(1 << (element_width - 1)).
'1' being signed int works fine for MO_8,16,32. For MO_64,
on PPC64 host this ends up becoming 0 on compilation. The
vextractdm uses MO_64, and it ends up having mask as 0.

The first patch here fixes that by explicitly using
1ULL instead of signed int 1 like its used everywhere else.
Second patch introduces the test case from [1] into qemu
tcg/ppc64 along with fixes/tweaks to make it work for both
big and little-endian targets.

Let me know if both patches should be squashed into single
patch. Checkpatch flagged me to avoid use of __BYTE_ORDER__
in the test file(second patch), however I see it being
used in multiarch/sha1.c also this being arch specific
test, I think it is appropriate to use it here. Let me
know if otherwise.

References:
[1] : https://gitlab.com/qemu-project/qemu/-/issues/1536

---

Shivaprasad G Bhat (2):
   tcg: ppc64: Fix mask generation for vextractdm
   tests: tcg: ppc64: Add tests for Vector Extract Mask Instructions


  target/ppc/translate/vmx-impl.c.inc |  2 +-
  tests/tcg/ppc64/Makefile.target |  6 +++-
  tests/tcg/ppc64/vector.c| 50 +
  3 files changed, 56 insertions(+), 2 deletions(-)
  create mode 100644 tests/tcg/ppc64/vector.c

--
Signature

Re: [PATCH v3 0/2] Cleanup ppc cr get/set with helper routines

[Daniel Henrique Barboza]

Queued in gitlab.com/danielhb/qemu/tree/ppc-next. Thanks,


Daniel

On 5/3/23 06:36, Harsh Prateek Bora wrote:

This patchset introduces helper routines to get/set cr reg for better code
readability / maintenance. No functional changes intended with this patchset.

Changelog:

changes from v2:
  - addressed Nick's review comments
  - holding changes from 2/4, 3/4 for future
changes from v1:
  - addressed Fabiano's review comments
  - folding Reviewed-by tags in patch 1/4, 4/4


*** BLURB HERE ***

Harsh Prateek Bora (2):
   ppc: spapr: cleanup cr get/set with helpers.
   MAINTAINERS: Adding myself in the list for ppc/spapr

  MAINTAINERS   |  1 +
  hw/ppc/spapr_hcall.c  | 18 ++
  linux-user/elfload.c  |  4 +---
  linux-user/ppc/signal.c   |  9 ++---
  target/ppc/cpu.c  | 17 +
  target/ppc/cpu.h  |  2 ++
  target/ppc/gdbstub.c  | 22 --
  target/ppc/kvm.c  | 13 ++---
  target/ppc/ppc-qmp-cmds.c |  6 +-
  9 files changed, 32 insertions(+), 60 deletions(-)