Re: [PATCH v9 09/10] target/riscv: Clear vstart_qe_zero flag

2024-03-09 Thread Richard Henderson

On 3/9/24 10:43, Daniel Henrique Barboza wrote:

From: Ivan Klokov 

The vstart_qe_zero flag is set at the beginning of the translation


Here and subject, s/qe/ne/.


r~



Re: [PATCH v9 06/10] trans_rvv.c.inc: set vstart = 0 in int scalar move insns

2024-03-09 Thread Richard Henderson

On 3/9/24 10:43, Daniel Henrique Barboza wrote:

trans_vmv_x_s, trans_vmv_s_x, trans_vfmv_f_s and trans_vfmv_s_f aren't
setting vstart = 0 after execution. This is usually done by a helper in
vector_helper.c but these functions don't use helpers.

We'll set vstart after any potential 'over' brconds, and that will also
mandate a mark_vs_dirty() too.

Fixes: dedc53cbc9 ("target/riscv: rvv-1.0: integer scalar move instructions")
Signed-off-by: Daniel Henrique Barboza
---
  target/riscv/insn_trans/trans_rvv.c.inc | 10 --
  1 file changed, 8 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v9 05/10] target/riscv: use vext_set_tail_elems_1s() in vcrypto insns

2024-03-09 Thread Richard Henderson

On 3/9/24 10:43, Daniel Henrique Barboza wrote:

Vcrypto insns should also use the same helper the regular vector insns
uses to update the tail elements.

Move vext_set_tail_elems_1s() to vector_internals.c and make it public.
Use it in vcrypto_helper.c to set tail elements instead of
vext_set_elems_1s(). Helpers must set env->vstart = 0 after setting the
tail.

Signed-off-by: Daniel Henrique Barboza
---
  target/riscv/vcrypto_helper.c   | 63 -
  target/riscv/vector_helper.c| 30 
  target/riscv/vector_internals.c | 29 +++
  target/riscv/vector_internals.h |  4 +++
  4 files changed, 56 insertions(+), 70 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v9 04/10] target/riscv/vector_helper.c: update tail with vext_set_tail_elems_1s()

2024-03-09 Thread Richard Henderson

On 3/9/24 10:43, Daniel Henrique Barboza wrote:

Change all code that updates tail elems to use vext_set_tail_elems_1s()
instead of vext_set_elems_1s().

Setting 'env->vstart=0' needs to be the very last thing a helper does
because env->vstart is being checked by vext_set_tail_elems_1s().


I did wonder if it would be worth doing the vstart = 0 in vext_set_tail_elems_1s, allowing 
that to be the very last thing in each helper, and could be tail called.




A side effect of this change is that a lot of 'vta' local variables got
unused. The reason is that 'vta' was being fetched to be used with
vext_set_elems_1s() but vext_set_tail_elems_1s() doesn't use it - 'vta' is
retrieve inside the helper using 'desc'.

Signed-off-by: Daniel Henrique Barboza 


Reviewed-by: Richard Henderson 


r~



Re: [PATCH v9 03/10] target/riscv/vector_helper.c: do vstart=0 after updating tail

2024-03-09 Thread Richard Henderson

On 3/9/24 10:43, Daniel Henrique Barboza wrote:

vext_vv_rm_1() and vext_vv_rm_2() are setting vstart = 0 before their
respective callers (vext_vv_rm_2 and  vext_vx_rm_2) update the tail
elements.

This is benign now, but we'll convert the tail updates to use
vext_set_tail_elems_1s(), and this function is sensitive to vstart
changes. Do vstart = 0 after vext_set_elems_1s() now to make the
conversion easier.

Signed-off-by: Daniel Henrique Barboza
---
  target/riscv/vector_helper.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v9 02/10] target/riscv: handle vstart >= vl in vext_set_tail_elems_1s()

2024-03-09 Thread Richard Henderson

On 3/9/24 10:43, Daniel Henrique Barboza wrote:

We're going to make changes that will required each helper to be
responsible for the 'vstart' management, i.e. we will relieve the
'vstart < vl' assumption that helpers have today.

To do that we'll need to deal with how we're updating tail elements
first. We can't update them if vstart >= vl, but at this moment we're
not guarding for it.

We have the vext_set_tail_elems_1s() helper to update tail elements.
Change it to accept an 'env' pointer, where we can read both vstart and
vl, and make it a no-op if vstart >= vl. Note that callers will need to
set env->start = 0 *after* the helper from now on.

The exception are three helpers: vext_ldst_stride(), vext_ldst_us() and
vext_ldst_index(). They are are incrementing env->vstart during
execution and will end up with env->vstart = vl when tail updating. For
these cases only, do an early check and exit if vstart >= vl, and set
env->vstart = 0 before updating the tail.

For everyone else we'll do vext_set_tail_elems_1s() and then clear
env->vstart. This is the case of vext_ldff() that is already using
set_tail_elems_1s(), and will be the case for the rest after the next
patches.

Let's also simplify the API a little by removing the 'nf' argument since
it can be derived from 'desc'.

Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/vector_helper.c | 59 ++--
  1 file changed, 49 insertions(+), 10 deletions(-)


Reviewed-by: Richard Henderson 


+uint32_t nf = vext_nf(desc);
  int k;
  
-if (vta == 0) {

+/*
+ * Section 5.4 of the RVV spec mentions:
+ * "When vstart ≥ vl, there are no body elements, and no
+ *  elements are updated in any destination vector register
+ *  group, including that no tail elements are updated
+ *  with agnostic values."
+ */
+if (vta == 0 || env->vstart >= env->vl) {
  return;
  }
  
  for (k = 0; k < nf; ++k) {


Existing issue, and we know nf <= 8, but bad form to mix signs on the 
comparison.


-vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
+vext_set_elems_1s(vd, vta, (k * max_elems + env->vl) * esz,


You may wish to hoist vl to a local anyway.


r~



[PATCH] usb-audio: Fix invalid values in AudioControl descriptors

2024-03-09 Thread Joonas Kankaala
This fixes the invalid bInterfaceProtocol value 0x04 in the USB audio
AudioControl descriptors. It should be zero. While Linux and Windows
forgive this error, macOS 14 Sonoma does not. The usb-audio device does
not appear in macOS sound settings even though the device is recognized
and shows up in USB system information. According to the USB audio class
specs 1.0-4.0, valid values are 0x00, 0x20, 0x30 and 0x40. (Note also
that Linux prints the warning "unknown interface protocol 0x4, assuming
v1", but then proceeds as if the value was zero.)

This also fixes the invalid wTotalLength value in the multi-channel
setup AudioControl interface header descriptor (used when multi=on
and out.mixing-engine off). The combined length of all the descriptors
there add up to 0x37, not 0x38. In Linux, "lsusb -D ..." displays
incomplete descriptor information when this length is incorrect.

Signed-off-by: Joonas Kankaala 
---
 hw/usb/dev-audio.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/usb/dev-audio.c b/hw/usb/dev-audio.c
index d5ac1f8962..1897fff9e6 100644
--- a/hw/usb/dev-audio.c
+++ b/hw/usb/dev-audio.c
@@ -124,7 +124,6 @@ static const USBDescIface desc_iface[] = {
 .bNumEndpoints = 0,
 .bInterfaceClass   = USB_CLASS_AUDIO,
 .bInterfaceSubClass= USB_SUBCLASS_AUDIO_CONTROL,
-.bInterfaceProtocol= 0x04,
 .iInterface= STRING_USBAUDIO_CONTROL,
 .ndesc = 4,
 .descs = (USBDescOther[]) {
@@ -282,7 +281,6 @@ static const USBDescIface desc_iface_multi[] = {
 .bNumEndpoints = 0,
 .bInterfaceClass   = USB_CLASS_AUDIO,
 .bInterfaceSubClass= USB_SUBCLASS_AUDIO_CONTROL,
-.bInterfaceProtocol= 0x04,
 .iInterface= STRING_USBAUDIO_CONTROL,
 .ndesc = 4,
 .descs = (USBDescOther[]) {
@@ -293,7 +291,7 @@ static const USBDescIface desc_iface_multi[] = {
 USB_DT_CS_INTERFACE,/*  u8  bDescriptorType */
 DST_AC_HEADER,  /*  u8  bDescriptorSubtype */
 U16(0x0100),/* u16  bcdADC */
-U16(0x38),  /* u16  wTotalLength */
+U16(0x37),  /* u16  wTotalLength */
 0x01,   /*  u8  bInCollection */
 0x01,   /*  u8  baInterfaceNr */
 }
-- 
2.39.2




[PATCH v9 09/10] target/riscv: Clear vstart_qe_zero flag

2024-03-09 Thread Daniel Henrique Barboza
From: Ivan Klokov 

The vstart_qe_zero flag is set at the beginning of the translation
phase from the env->vstart variable. During the execution phase all
functions will set env->vstart = 0 after a successful execution,
but the vstart_eq_zero flag remains the same as at the start of the
block. This will wrongly cause SIGILLs in translations that requires
env->vstart = 0 and might be reading vstart_eq_zero = false.

This patch adds a new finalize_rvv_inst() helper that is called at the
end of each vector instruction that will both update vstart_eq_zero and
do a mark_vs_dirty().

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1976
Signed-off-by: Ivan Klokov 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/insn_trans/trans_rvbf16.c.inc |  6 +-
 target/riscv/insn_trans/trans_rvv.c.inc| 83 --
 target/riscv/insn_trans/trans_rvvk.c.inc   | 12 ++--
 target/riscv/translate.c   |  6 ++
 4 files changed, 59 insertions(+), 48 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc 
b/target/riscv/insn_trans/trans_rvbf16.c.inc
index a842e76a6b..0a9cd1ec31 100644
--- a/target/riscv/insn_trans/trans_rvbf16.c.inc
+++ b/target/riscv/insn_trans/trans_rvbf16.c.inc
@@ -83,7 +83,7 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, 
arg_vfncvtbf16_f_f_w *a)
ctx->cfg_ptr->vlenb,
ctx->cfg_ptr->vlenb, data,
gen_helper_vfncvtbf16_f_f_w);
-mark_vs_dirty(ctx);
+finalize_rvv_inst(ctx);
 return true;
 }
 return false;
@@ -108,7 +108,7 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, 
arg_vfwcvtbf16_f_f_v *a)
ctx->cfg_ptr->vlenb,
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwcvtbf16_f_f_v);
-mark_vs_dirty(ctx);
+finalize_rvv_inst(ctx);
 return true;
 }
 return false;
@@ -135,7 +135,7 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, 
arg_vfwmaccbf16_vv *a)
ctx->cfg_ptr->vlenb,
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwmaccbf16_vv);
-mark_vs_dirty(ctx);
+finalize_rvv_inst(ctx);
 return true;
 }
 return false;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index b0f19dcd85..b3d467a874 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -167,7 +167,7 @@ static bool do_vsetvl(DisasContext *s, int rd, int rs1, 
TCGv s2)
 
 gen_helper_vsetvl(dst, tcg_env, s1, s2);
 gen_set_gpr(s, rd, dst);
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 
 gen_update_pc(s, s->cur_insn_len);
 lookup_and_goto_ptr(s);
@@ -187,7 +187,7 @@ static bool do_vsetivli(DisasContext *s, int rd, TCGv s1, 
TCGv s2)
 
 gen_helper_vsetvl(dst, tcg_env, s1, s2);
 gen_set_gpr(s, rd, dst);
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 gen_update_pc(s, s->cur_insn_len);
 lookup_and_goto_ptr(s);
 s->base.is_jmp = DISAS_NORETURN;
@@ -657,6 +657,7 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 tcg_gen_mb(TCG_MO_ALL | TCG_BAR_LDAQ);
 }
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -812,6 +813,7 @@ static bool ldst_stride_trans(uint32_t vd, uint32_t rs1, 
uint32_t rs2,
 
 fn(dest, mask, base, stride, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -913,6 +915,7 @@ static bool ldst_index_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2,
 
 fn(dest, mask, base, index, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1043,7 +1046,7 @@ static bool ldff_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 
 fn(dest, mask, base, tcg_env, desc);
 
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1100,6 +1103,7 @@ static bool ldst_whole_trans(uint32_t vd, uint32_t rs1, 
uint32_t nf,
 
 fn(dest, base, tcg_env, desc);
 
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1189,7 +1193,7 @@ do_opivv_gvec(DisasContext *s, arg_rmrr *a, GVecGen3Fn 
*gvec_fn,
tcg_env, s->cfg_ptr->vlenb,
s->cfg_ptr->vlenb, data, fn);
 }
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1240,7 +1244,7 @@ static bool opivx_trans(uint32_t vd, uint32_t rs1, 
uint32_t vs2, uint32_t vm,
 
 fn(dest, mask, src1, src2, tcg_env, desc);
 
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return true;
 }
 
@@ -1265,7 +1269,7 @@ do_opivx_gvec(DisasContext *s, arg_rmrr *a, GVecGen2sFn 
*gvec_fn,
 gvec_fn(s->sew, vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2),
 src1, MAXSZ(s), MAXSZ(s));
 
-mark_vs_dirty(s);
+finalize_rvv_inst(s);
 return t

[PATCH v9 10/10] target/riscv/vector_helper.c: optimize loops in ldst helpers

2024-03-09 Thread Daniel Henrique Barboza
Change the for loops in ldst helpers to do a single increment in the
counter, and assign it env->vstart, to avoid re-reading from vstart
every time.

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Alistair Francis 
Reviewed-by: Richard Henderson 
---
 target/riscv/vector_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 4fe8752eea..ee57300dc0 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -195,7 +195,7 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
 return;
 }
 
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
 k = 0;
 while (k < nf) {
 if (!vm && !vext_elem_mask(v0, i)) {
@@ -270,7 +270,7 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState 
*env, uint32_t desc,
 }
 
 /* load bytes from guest memory */
-for (i = env->vstart; i < evl; i++, env->vstart++) {
+for (i = env->vstart; i < evl; env->vstart = ++i) {
 k = 0;
 while (k < nf) {
 target_ulong addr = base + ((i * nf + k) << log2_esz);
@@ -393,7 +393,7 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
 }
 
 /* load bytes from guest memory */
-for (i = env->vstart; i < env->vl; i++, env->vstart++) {
+for (i = env->vstart; i < env->vl; env->vstart = ++i) {
 k = 0;
 while (k < nf) {
 if (!vm && !vext_elem_mask(v0, i)) {
-- 
2.43.2




[PATCH v9 05/10] target/riscv: use vext_set_tail_elems_1s() in vcrypto insns

2024-03-09 Thread Daniel Henrique Barboza
Vcrypto insns should also use the same helper the regular vector insns
uses to update the tail elements.

Move vext_set_tail_elems_1s() to vector_internals.c and make it public.
Use it in vcrypto_helper.c to set tail elements instead of
vext_set_elems_1s(). Helpers must set env->vstart = 0 after setting the
tail.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/vcrypto_helper.c   | 63 -
 target/riscv/vector_helper.c| 30 
 target/riscv/vector_internals.c | 29 +++
 target/riscv/vector_internals.h |  4 +++
 4 files changed, 56 insertions(+), 70 deletions(-)

diff --git a/target/riscv/vcrypto_helper.c b/target/riscv/vcrypto_helper.c
index e2d719b13b..66d449c274 100644
--- a/target/riscv/vcrypto_helper.c
+++ b/target/riscv/vcrypto_helper.c
@@ -218,9 +218,7 @@ static inline void xor_round_key(AESState *round_state, 
AESState *round_key)
 void HELPER(NAME)(void *vd, void *vs2, CPURISCVState *env,\
   uint32_t desc)  \
 { \
-uint32_t vl = env->vl;\
 uint32_t total_elems = vext_get_total_elems(env, desc, 4);\
-uint32_t vta = vext_vta(desc);\
   \
 for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {\
 AESState round_key;   \
@@ -233,18 +231,16 @@ static inline void xor_round_key(AESState *round_state, 
AESState *round_key)
 *((uint64_t *)vd + H8(i * 2 + 0)) = round_state.d[0]; \
 *((uint64_t *)vd + H8(i * 2 + 1)) = round_state.d[1]; \
 } \
-env->vstart = 0;  \
 /* set tail elements to 1s */ \
-vext_set_elems_1s(vd, vta, vl * 4, total_elems * 4);  \
+vext_set_tail_elems_1s(env, vd, desc, 4, total_elems);\
+env->vstart = 0;  \
 }
 
 #define GEN_ZVKNED_HELPER_VS(NAME, ...)   \
 void HELPER(NAME)(void *vd, void *vs2, CPURISCVState *env,\
   uint32_t desc)  \
 { \
-uint32_t vl = env->vl;\
 uint32_t total_elems = vext_get_total_elems(env, desc, 4);\
-uint32_t vta = vext_vta(desc);\
   \
 for (uint32_t i = env->vstart / 4; i < env->vl / 4; i++) {\
 AESState round_key;   \
@@ -257,9 +253,9 @@ static inline void xor_round_key(AESState *round_state, 
AESState *round_key)
 *((uint64_t *)vd + H8(i * 2 + 0)) = round_state.d[0]; \
 *((uint64_t *)vd + H8(i * 2 + 1)) = round_state.d[1]; \
 } \
-env->vstart = 0;  \
 /* set tail elements to 1s */ \
-vext_set_elems_1s(vd, vta, vl * 4, total_elems * 4);  \
+vext_set_tail_elems_1s(env, vd, desc, 4, total_elems);\
+env->vstart = 0;  \
 }
 
 GEN_ZVKNED_HELPER_VV(vaesef_vv, aesenc_SB_SR_AK(&round_state,
@@ -301,9 +297,7 @@ void HELPER(vaeskf1_vi)(void *vd_vptr, void *vs2_vptr, 
uint32_t uimm,
 {
 uint32_t *vd = vd_vptr;
 uint32_t *vs2 = vs2_vptr;
-uint32_t vl = env->vl;
 uint32_t total_elems = vext_get_total_elems(env, desc, 4);
-uint32_t vta = vext_vta(desc);
 
 uimm &= 0b;
 if (uimm > 10 || uimm == 0) {
@@ -337,9 +331,9 @@ void HELPER(vaeskf1_vi)(void *vd_vptr, void *vs2_vptr, 
uint32_t uimm,
 vd[i * 4 + H4(2)] = rk[6];
 vd[i * 4 + H4(3)] = rk[7];
 }
-env->vstart = 0;
 /* set tail elements to 1s */
-vext_set_elems_1s(vd, vta, vl * 4, total_elems * 4);
+vext_set_tail_elems_1s(env, vd, desc, 4, total_elems);
+env->vstart = 0;
 }
 
 void HELPER(vaeskf2_vi)(void *vd_vptr, void *vs2_vptr, uint32_t uimm,
@@ -347,9 +341,7 @@ void HELPER(vaeskf2_vi)(void *vd_vptr, void *vs2_vptr, 
uint32_t uimm,
 {
 uint32_t *vd = vd_vptr;
 uint32_t *vs2 = vs2_vptr;
-uint32_t vl = env->vl;
 uint32_t total_elems = vext_get_total_elems(env, desc, 4);
-uint32_t vta = vext_vta(desc);
 
 uimm &= 0b;
 if (uimm > 14 |

[PATCH v9 08/10] trans_rvv.c.inc: remove redundant mark_vs_dirty() calls

2024-03-09 Thread Daniel Henrique Barboza
trans_vmv_v_i , trans_vfmv_v_f and the trans_##NAME macro from
GEN_VMV_WHOLE_TRANS() are calling mark_vs_dirty() in both branches of
their 'ifs'. conditionals.

Call it just once in the end like other functions are doing.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
Reviewed-by: Philippe Mathieu-Daudé 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 11 +++
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 4c1a064cf6..b0f19dcd85 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -2065,7 +2065,6 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
 if (s->vl_eq_vlmax && !(s->vta && s->lmul < 0)) {
 tcg_gen_gvec_dup_imm(s->sew, vreg_ofs(s, a->rd),
  MAXSZ(s), MAXSZ(s), simm);
-mark_vs_dirty(s);
 } else {
 TCGv_i32 desc;
 TCGv_i64 s1;
@@ -2083,9 +2082,8 @@ static bool trans_vmv_v_i(DisasContext *s, arg_vmv_v_i *a)
   s->cfg_ptr->vlenb, data));
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
 fns[s->sew](dest, s1, tcg_env, desc);
-
-mark_vs_dirty(s);
 }
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -2612,7 +2610,6 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f 
*a)
 
 tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
  MAXSZ(s), MAXSZ(s), t1);
-mark_vs_dirty(s);
 } else {
 TCGv_ptr dest;
 TCGv_i32 desc;
@@ -2635,9 +2632,8 @@ static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f 
*a)
 tcg_gen_addi_ptr(dest, tcg_env, vreg_ofs(s, a->rd));
 
 fns[s->sew - 1](dest, t1, tcg_env, desc);
-
-mark_vs_dirty(s);
 }
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -3560,12 +3556,11 @@ static bool trans_##NAME(DisasContext *s, arg_##NAME * 
a)   \
 if (s->vstart_eq_zero) {\
 tcg_gen_gvec_mov(s->sew, vreg_ofs(s, a->rd),\
  vreg_ofs(s, a->rs2), maxsz, maxsz);\
-mark_vs_dirty(s);   \
 } else {\
 tcg_gen_gvec_2_ptr(vreg_ofs(s, a->rd), vreg_ofs(s, a->rs2), \
tcg_env, maxsz, maxsz, 0, gen_helper_vmvr_v); \
-mark_vs_dirty(s);   \
 }   \
+mark_vs_dirty(s);   \
 return true;\
 }   \
 return false;   \
-- 
2.43.2




[PATCH v9 07/10] target/riscv: remove 'over' brconds from vector trans

2024-03-09 Thread Daniel Henrique Barboza
Most of the vector translations has this following pattern at the start:

TCGLabel *over = gen_new_label();
tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);

And then right at the end:

 gen_set_label(over);
 return true;

This means that if vstart >= vl we'll not set vstart = 0 at the end of
the insns - this is done inside the helper that is being skipped.  The
reason why this pattern hasn't been a bigger problem is because the
conditional vstart >= vl is very rare.

Checking all the helpers in vector_helper.c we see all of them with a
pattern like this:

for (i = env->vstart; i < vl; i++) {
(...)
}
env->vstart = 0;

Thus they can handle vstart >= vl case gracefully, with the benefit of
setting env->vstart = 0 during the process.

Remove all 'over' conditionals and let the helper set env->vstart = 0
every time.

Note that not all insns uses helpers, and for those cases the 'brcond'
jump is the only way to filter vstart >= vl. This is the case of
trans_vmv_s_x() and trans_vfmv_s_f(). We won't remove the 'brcond'
conditionals from them.

While we're at it, remove the (vl == 0) brconds from trans_rvbf16.c.inc
too since they're unneeded.

Suggested-by: Richard Henderson 
Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/insn_trans/trans_rvbf16.c.inc |  12 ---
 target/riscv/insn_trans/trans_rvv.c.inc| 108 -
 target/riscv/insn_trans/trans_rvvk.c.inc   |  18 
 3 files changed, 138 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvbf16.c.inc 
b/target/riscv/insn_trans/trans_rvbf16.c.inc
index 8ee99df3f3..a842e76a6b 100644
--- a/target/riscv/insn_trans/trans_rvbf16.c.inc
+++ b/target/riscv/insn_trans/trans_rvbf16.c.inc
@@ -71,11 +71,8 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, 
arg_vfncvtbf16_f_f_w *a)
 
 if (opfv_narrow_check(ctx, a) && (ctx->sew == MO_16)) {
 uint32_t data = 0;
-TCGLabel *over = gen_new_label();
 
 gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -87,7 +84,6 @@ static bool trans_vfncvtbf16_f_f_w(DisasContext *ctx, 
arg_vfncvtbf16_f_f_w *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfncvtbf16_f_f_w);
 mark_vs_dirty(ctx);
-gen_set_label(over);
 return true;
 }
 return false;
@@ -100,11 +96,8 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, 
arg_vfwcvtbf16_f_f_v *a)
 
 if (opfv_widen_check(ctx, a) && (ctx->sew == MO_16)) {
 uint32_t data = 0;
-TCGLabel *over = gen_new_label();
 
 gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -116,7 +109,6 @@ static bool trans_vfwcvtbf16_f_f_v(DisasContext *ctx, 
arg_vfwcvtbf16_f_f_v *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwcvtbf16_f_f_v);
 mark_vs_dirty(ctx);
-gen_set_label(over);
 return true;
 }
 return false;
@@ -130,11 +122,8 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, 
arg_vfwmaccbf16_vv *a)
 if (require_rvv(ctx) && vext_check_isa_ill(ctx) && (ctx->sew == MO_16) &&
 vext_check_dss(ctx, a->rd, a->rs1, a->rs2, a->vm)) {
 uint32_t data = 0;
-TCGLabel *over = gen_new_label();
 
 gen_set_rm_chkfrm(ctx, RISCV_FRM_DYN);
-tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
 
 data = FIELD_DP32(data, VDATA, VM, a->vm);
 data = FIELD_DP32(data, VDATA, LMUL, ctx->lmul);
@@ -147,7 +136,6 @@ static bool trans_vfwmaccbf16_vv(DisasContext *ctx, 
arg_vfwmaccbf16_vv *a)
ctx->cfg_ptr->vlenb, data,
gen_helper_vfwmaccbf16_vv);
 mark_vs_dirty(ctx);
-gen_set_label(over);
 return true;
 }
 return false;
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 8c16a9f5b3..4c1a064cf6 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -616,9 +616,6 @@ static bool ldst_us_trans(uint32_t vd, uint32_t rs1, 
uint32_t data,
 TCGv base;
 TCGv_i32 desc;
 
-TCGLabel *over = gen_new_label();
-tcg_gen_brcond_tl(TCG_COND_GEU, cpu_vstart, cpu_vl, over);
-
 dest = tcg_temp_new_ptr();
 mask = tcg_temp_new_ptr();
 base = get_gpr(s, rs1, EXT_NONE);
@@ -660,7 +657,6 @@ static bool ldst_us_trans(uint32_

[PATCH v9 04/10] target/riscv/vector_helper.c: update tail with vext_set_tail_elems_1s()

2024-03-09 Thread Daniel Henrique Barboza
Change all code that updates tail elems to use vext_set_tail_elems_1s()
instead of vext_set_elems_1s().

Setting 'env->vstart=0' needs to be the very last thing a helper does
because env->vstart is being checked by vext_set_tail_elems_1s().

A side effect of this change is that a lot of 'vta' local variables got
unused. The reason is that 'vta' was being fetched to be used with
vext_set_elems_1s() but vext_set_tail_elems_1s() doesn't use it - 'vta' is
retrieve inside the helper using 'desc'.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/vector_helper.c | 130 ++-
 1 file changed, 52 insertions(+), 78 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 86b990ce03..b174ddeae8 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -913,7 +913,6 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, 
  \
 uint32_t esz = sizeof(ETYPE); \
 uint32_t total_elems =\
 vext_get_total_elems(env, desc, esz); \
-uint32_t vta = vext_vta(desc);\
 uint32_t i;   \
   \
 for (i = env->vstart; i < vl; i++) {  \
@@ -923,9 +922,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2, 
  \
   \
 *((ETYPE *)vd + H(i)) = DO_OP(s2, s1, carry); \
 } \
-env->vstart = 0;  \
 /* set tail elements to 1s */ \
-vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);  \
+vext_set_tail_elems_1s(env, vd, desc, esz, total_elems);  \
+env->vstart = 0;  \
 }
 
 GEN_VEXT_VADC_VVM(vadc_vvm_b, uint8_t,  H1, DO_VADC)
@@ -945,7 +944,6 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void 
*vs2,\
 uint32_t vl = env->vl;   \
 uint32_t esz = sizeof(ETYPE);\
 uint32_t total_elems = vext_get_total_elems(env, desc, esz); \
-uint32_t vta = vext_vta(desc);   \
 uint32_t i;  \
  \
 for (i = env->vstart; i < vl; i++) { \
@@ -954,9 +952,9 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, void 
*vs2,\
  \
 *((ETYPE *)vd + H(i)) = DO_OP(s2, (ETYPE)(target_long)s1, carry);\
 }\
-env->vstart = 0; \
 /* set tail elements to 1s */\
-vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz); \
+vext_set_tail_elems_1s(env, vd, desc, esz, total_elems); \
+env->vstart = 0; \
 }
 
 GEN_VEXT_VADC_VXM(vadc_vxm_b, uint8_t,  H1, DO_VADC)
@@ -1113,7 +,6 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,  
\
 uint32_t vl = env->vl;\
 uint32_t esz = sizeof(TS1);   \
 uint32_t total_elems = vext_get_total_elems(env, desc, esz);  \
-uint32_t vta = vext_vta(desc);\
 uint32_t vma = vext_vma(desc);\
 uint32_t i;   \
   \
@@ -1127,9 +1124,9 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1,  
\
 TS2 s2 = *((TS2 *)vs2 + HS2(i));  \
 *((TS1 *)vd + HS1(i)) = OP(s2, s1 & MASK);\
 } \
-env->vstart = 0;  \
 /* set tail elements to 1s */ \
-vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);  \
+vext_set_tail_elems_1s(env, vd, desc, esz, total_elems);  \
+env->vstart = 0;  \
 }
 
 GEN_VEXT_SHIFT_VV(vsll_vv_b, uint8_t,  uint8_t, H1, H1, DO_SLL, 0x7)
@@ -1160,7 +1157,6 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1,
  \
 uint32_t esz = sizeof(TD);   

[PATCH v9 02/10] target/riscv: handle vstart >= vl in vext_set_tail_elems_1s()

2024-03-09 Thread Daniel Henrique Barboza
We're going to make changes that will required each helper to be
responsible for the 'vstart' management, i.e. we will relieve the
'vstart < vl' assumption that helpers have today.

To do that we'll need to deal with how we're updating tail elements
first. We can't update them if vstart >= vl, but at this moment we're
not guarding for it.

We have the vext_set_tail_elems_1s() helper to update tail elements.
Change it to accept an 'env' pointer, where we can read both vstart and
vl, and make it a no-op if vstart >= vl. Note that callers will need to
set env->start = 0 *after* the helper from now on.

The exception are three helpers: vext_ldst_stride(), vext_ldst_us() and
vext_ldst_index(). They are are incrementing env->vstart during
execution and will end up with env->vstart = vl when tail updating. For
these cases only, do an early check and exit if vstart >= vl, and set
env->vstart = 0 before updating the tail.

For everyone else we'll do vext_set_tail_elems_1s() and then clear
env->vstart. This is the case of vext_ldff() that is already using
set_tail_elems_1s(), and will be the case for the rest after the next
patches.

Let's also simplify the API a little by removing the 'nf' argument since
it can be derived from 'desc'.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/vector_helper.c | 59 ++--
 1 file changed, 49 insertions(+), 10 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index ca79571ae2..a3b496b6e9 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -174,19 +174,32 @@ GEN_VEXT_ST_ELEM(ste_h, int16_t, H2, stw)
 GEN_VEXT_ST_ELEM(ste_w, int32_t, H4, stl)
 GEN_VEXT_ST_ELEM(ste_d, int64_t, H8, stq)
 
-static void vext_set_tail_elems_1s(target_ulong vl, void *vd,
-   uint32_t desc, uint32_t nf,
-   uint32_t esz, uint32_t max_elems)
+/*
+ * This function is sensitive to env->vstart changes since
+ * it'll be a no-op if vstart >= vl. Do not clear env->vstart
+ * before calling it unless you're certain that vstart < vl.
+ */
+static void vext_set_tail_elems_1s(CPURISCVState *env, void *vd,
+   uint32_t desc, uint32_t esz,
+   uint32_t max_elems)
 {
 uint32_t vta = vext_vta(desc);
+uint32_t nf = vext_nf(desc);
 int k;
 
-if (vta == 0) {
+/*
+ * Section 5.4 of the RVV spec mentions:
+ * "When vstart ≥ vl, there are no body elements, and no
+ *  elements are updated in any destination vector register
+ *  group, including that no tail elements are updated
+ *  with agnostic values."
+ */
+if (vta == 0 || env->vstart >= env->vl) {
 return;
 }
 
 for (k = 0; k < nf; ++k) {
-vext_set_elems_1s(vd, vta, (k * max_elems + vl) * esz,
+vext_set_elems_1s(vd, vta, (k * max_elems + env->vl) * esz,
   (k * max_elems + max_elems) * esz);
 }
 }
@@ -207,6 +220,11 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
 uint32_t esz = 1 << log2_esz;
 uint32_t vma = vext_vma(desc);
 
+if (env->vstart >= env->vl) {
+env->vstart = 0;
+return;
+}
+
 for (i = env->vstart; i < env->vl; i++, env->vstart++) {
 k = 0;
 while (k < nf) {
@@ -222,9 +240,13 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
 k++;
 }
 }
+/*
+ * Set vstart before tail update - vstart changed during
+ * execution and we already checked that vstart < vl.
+ */
 env->vstart = 0;
 
-vext_set_tail_elems_1s(env->vl, vd, desc, nf, esz, max_elems);
+vext_set_tail_elems_1s(env, vd, desc, esz, max_elems);
 }
 
 #define GEN_VEXT_LD_STRIDE(NAME, ETYPE, LOAD_FN)\
@@ -272,6 +294,11 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState 
*env, uint32_t desc,
 uint32_t max_elems = vext_max_elems(desc, log2_esz);
 uint32_t esz = 1 << log2_esz;
 
+if (env->vstart >= env->vl) {
+env->vstart = 0;
+return;
+}
+
 /* load bytes from guest memory */
 for (i = env->vstart; i < evl; i++, env->vstart++) {
 k = 0;
@@ -281,9 +308,13 @@ vext_ldst_us(void *vd, target_ulong base, CPURISCVState 
*env, uint32_t desc,
 k++;
 }
 }
+/*
+ * Set vstart before tail update - vstart changed during
+ * execution and we already checked that vstart < vl.
+ */
 env->vstart = 0;
 
-vext_set_tail_elems_1s(evl, vd, desc, nf, esz, max_elems);
+vext_set_tail_elems_1s(env, vd, desc, esz, max_elems);
 }
 
 /*
@@ -386,6 +417,11 @@ vext_ldst_index(void *vd, void *v0, target_ulong base,
 uint32_t esz = 1 << log2_esz;
 uint32_t vma = vext_vma(desc);
 
+if (env->vstart >= env->vl) {
+env->vstart = 0;
+return;
+}
+
 /* load bytes from guest memory */
 for (i = env->vstart; i < env->vl; i++, env-

[PATCH v9 01/10] target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX()

2024-03-09 Thread Daniel Henrique Barboza
The helper isn't setting env->vstart = 0 after its execution, as it is
expected from every vector instruction that completes successfully.

Signed-off-by: Daniel Henrique Barboza 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/vector_helper.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index fe56c007d5..ca79571ae2 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -4781,6 +4781,7 @@ void HELPER(NAME)(void *vd, void *v0, target_ulong s1, 
void *vs2, \
 } \
 *((ETYPE *)vd + H(i)) = *((ETYPE *)vs2 + H(i - offset));  \
 } \
+env->vstart = 0;  \
 /* set tail elements to 1s */ \
 vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);  \
 }
-- 
2.43.2




[PATCH v9 06/10] trans_rvv.c.inc: set vstart = 0 in int scalar move insns

2024-03-09 Thread Daniel Henrique Barboza
trans_vmv_x_s, trans_vmv_s_x, trans_vfmv_f_s and trans_vfmv_s_f aren't
setting vstart = 0 after execution. This is usually done by a helper in
vector_helper.c but these functions don't use helpers.

We'll set vstart after any potential 'over' brconds, and that will also
mandate a mark_vs_dirty() too.

Fixes: dedc53cbc9 ("target/riscv: rvv-1.0: integer scalar move instructions")
Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/insn_trans/trans_rvv.c.inc | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index e42728990e..8c16a9f5b3 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3373,6 +3373,8 @@ static bool trans_vmv_x_s(DisasContext *s, arg_vmv_x_s *a)
 vec_element_loadi(s, t1, a->rs2, 0, true);
 tcg_gen_trunc_i64_tl(dest, t1);
 gen_set_gpr(s, a->rd, dest);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -3399,8 +3401,9 @@ static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
 s1 = get_gpr(s, a->rs1, EXT_NONE);
 tcg_gen_ext_tl_i64(t1, s1);
 vec_element_storei(s, a->rd, 0, t1);
-mark_vs_dirty(s);
 gen_set_label(over);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -3427,6 +3430,8 @@ static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s 
*a)
 }
 
 mark_fs_dirty(s);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
 return true;
 }
 return false;
@@ -3452,8 +3457,9 @@ static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f 
*a)
 do_nanbox(s, t1, cpu_fpr[a->rs1]);
 
 vec_element_storei(s, a->rd, 0, t1);
-mark_vs_dirty(s);
 gen_set_label(over);
+tcg_gen_movi_tl(cpu_vstart, 0);
+mark_vs_dirty(s);
 return true;
 }
 return false;
-- 
2.43.2




[PATCH v9 03/10] target/riscv/vector_helper.c: do vstart=0 after updating tail

2024-03-09 Thread Daniel Henrique Barboza
vext_vv_rm_1() and vext_vv_rm_2() are setting vstart = 0 before their
respective callers (vext_vv_rm_2 and  vext_vx_rm_2) update the tail
elements.

This is benign now, but we'll convert the tail updates to use
vext_set_tail_elems_1s(), and this function is sensitive to vstart
changes. Do vstart = 0 after vext_set_elems_1s() now to make the
conversion easier.

Signed-off-by: Daniel Henrique Barboza 
---
 target/riscv/vector_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index a3b496b6e9..86b990ce03 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -1962,7 +1962,6 @@ vext_vv_rm_1(void *vd, void *v0, void *vs1, void *vs2,
 }
 fn(vd, vs1, vs2, i, env, vxrm);
 }
-env->vstart = 0;
 }
 
 static inline void
@@ -1997,6 +1996,7 @@ vext_vv_rm_2(void *vd, void *v0, void *vs1, void *vs2,
 }
 /* set tail elements to 1s */
 vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
+env->vstart = 0;
 }
 
 /* generate helpers for fixed point instructions with OPIVV format */
@@ -2087,7 +2087,6 @@ vext_vx_rm_1(void *vd, void *v0, target_long s1, void 
*vs2,
 }
 fn(vd, s1, vs2, i, env, vxrm);
 }
-env->vstart = 0;
 }
 
 static inline void
@@ -2122,6 +2121,7 @@ vext_vx_rm_2(void *vd, void *v0, target_long s1, void 
*vs2,
 }
 /* set tail elements to 1s */
 vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
+env->vstart = 0;
 }
 
 /* generate helpers for fixed point instructions with OPIVX format */
-- 
2.43.2




[PATCH v9 00/10] riscv: set vstart_eq_zero on mark_vs_dirty

2024-03-09 Thread Daniel Henrique Barboza
Hi,

This new version has changes suggested by Richard in patches 2 and 6.
Other patches are unchanged.

Series based on alistair/riscv-to-apply.next.

Patches missing review: 2, 3, 4, 5, 6.

Changes from v8:
- patch 2:
  - do an early exit if vstart >= vl in vext_ldst_stride(), vext_ldst_us()
and vext_ldst_index() if vstart >= vl
- patch 6:
  - vec_set_vstart_zero() removed
  - set cpu_vstart directly using tcg_gen_movi_tl()
- v8 link: 
https://lore.kernel.org/qemu-riscv/20240308215402.117405-1-dbarb...@ventanamicro.com/


Daniel Henrique Barboza (9):
  target/riscv/vector_helper.c: set vstart = 0 in GEN_VEXT_VSLIDEUP_VX()
  target/riscv: handle vstart >= vl in vext_set_tail_elems_1s()
  target/riscv/vector_helper.c: do vstart=0 after updating tail
  target/riscv/vector_helper.c: update tail with
vext_set_tail_elems_1s()
  target/riscv: use vext_set_tail_elems_1s() in vcrypto insns
  trans_rvv.c.inc: set vstart = 0 in int scalar move insns
  target/riscv: remove 'over' brconds from vector trans
  trans_rvv.c.inc: remove redundant mark_vs_dirty() calls
  target/riscv/vector_helper.c: optimize loops in ldst helpers

Ivan Klokov (1):
  target/riscv: Clear vstart_qe_zero flag

 target/riscv/insn_trans/trans_rvbf16.c.inc |  18 +-
 target/riscv/insn_trans/trans_rvv.c.inc| 198 +
 target/riscv/insn_trans/trans_rvvk.c.inc   |  30 +---
 target/riscv/translate.c   |   6 +
 target/riscv/vcrypto_helper.c  |  63 +++
 target/riscv/vector_helper.c   | 192 +---
 target/riscv/vector_internals.c|  29 +++
 target/riscv/vector_internals.h|   4 +
 8 files changed, 207 insertions(+), 333 deletions(-)

-- 
2.43.2




Re: [PATCH v8 00/10] riscv: set vstart_eq_zero on mark_vs_dirty

2024-03-09 Thread Daniel Henrique Barboza




On 3/9/24 17:14, Richard Henderson wrote:

On 3/8/24 11:53, Daniel Henrique Barboza wrote:

- patch 7 (patch 3 from v7): do not remove brconds from scalar move
   insns

   trans_vmv_s_x() and trans_vfmv_s_f() does not have a helper that will
   handle vstart >= vl for them, so they need their brcond.

- patches 4 and 5 from v7: dropped. We're not removing all brconds, so
   we can't get rid of cpu_vstart and cpu_vl.


Not important for the bug fix, but for future cleanup:

(1) Use movcond to for these moves instead of brcond.

(2) Use explicit load/store in these two places where cpu_vstart and cpu_vl are 
still in use.

(3) Now there are no tcg globals that are modified by the vector helpers, which 
means that they can all be marked TCG_CALL_NO_WG, and any that never exit via 
exception may be marked TCG_CALL_NO_RWG.  This may reduce register allocator 
spill/fill across the affected helpers.


Nice. I'll mark it as a TODO to get it done after we get this series merged.


Thanks,

Daniel




r~




Re: [PATCH v2] docs/conf.py: Remove usage of distutils

2024-03-09 Thread Peter Maydell
On Sat, 9 Mar 2024 at 18:29, Philippe Mathieu-Daudé  wrote:
>
> On 9/3/24 18:27, Peter Maydell wrote:
> > On Tue, 5 Mar 2024 at 10:39, Thomas Huth  wrote:
> >> Ok, while I was writing my mail, I was looking at https://brew.sh/ and
> >> didn't see a link to a bug tracker there ... but now I realized that they
> >> are simply using the github tracker, so I went ahead and filed a bug there:
> >>
> >>https://github.com/Homebrew/brew/issues/16823
> >>
> >> Let's see how it goes...
> >
> > Seems to be going slowly. I notice that there's a comment in there
> > saying that "brew install python-setuptools" is a workaround to
> > get glib 2.78 working -- that seems like it would be good to get
> > our CI back to green. Is there a way to test changes to the cirrus
> > config short of committing it and seeing if it helps? I don't see
> > the jobs available on a pipeline in my personal gitlab repo...
>
> Yes, but you need to grant cirrus-ci some permissions you weren't
> ready to give last time.
>
> I just tested it and it is still failing for me:
> https://gitlab.com/philmd/qemu/-/jobs/6357526794

That job doesn't seem to have tried to install python-setuptools ?
It did:

- brew install 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 swtpm tesseract usbredir
vde vte3 xorriso zlib zstd

-- PMM



Re: [PATCH v8 00/10] riscv: set vstart_eq_zero on mark_vs_dirty

2024-03-09 Thread Richard Henderson

On 3/8/24 11:53, Daniel Henrique Barboza wrote:

- patch 7 (patch 3 from v7): do not remove brconds from scalar move
   insns

   trans_vmv_s_x() and trans_vfmv_s_f() does not have a helper that will
   handle vstart >= vl for them, so they need their brcond.

- patches 4 and 5 from v7: dropped. We're not removing all brconds, so
   we can't get rid of cpu_vstart and cpu_vl.


Not important for the bug fix, but for future cleanup:

(1) Use movcond to for these moves instead of brcond.

(2) Use explicit load/store in these two places where cpu_vstart and cpu_vl are 
still in use.

(3) Now there are no tcg globals that are modified by the vector helpers, which means that 
they can all be marked TCG_CALL_NO_WG, and any that never exit via exception may be marked 
TCG_CALL_NO_RWG.  This may reduce register allocator spill/fill across the affected helpers.



r~



Re: [PULL v2 0/9] Misc fixes and coverity CI for 2024-03-08

2024-03-09 Thread Peter Maydell
On Fri, 8 Mar 2024 at 18:13, Paolo Bonzini  wrote:
>
> The following changes since commit 8f6330a807f2642dc2a3cdf33347aa28a4c00a87:
>
>   Merge tag 'pull-maintainer-updates-060324-1' of 
> https://gitlab.com/stsquad/qemu into staging (2024-03-06 16:56:20 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 83aa1baa069c8f77aa9f7d9adfdeb11d90bdf78d:
>
>   gitlab-ci: add manual job to run Coverity (2024-03-08 19:08:23 +0100)
>
> Supersedes: <20240308145554.599614-1-pbonz...@redhat.com>
> 
> * target/i386: use TSTEQ/TSTNE
> * move Coverity builds to Gitlab CI
> * fix two memory leaks
> * bug fixes


Applied, thanks.

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

-- PMM



Re: [PULL 0/3] Hyper-V Dynamic Memory and VMBus misc small patches

2024-03-09 Thread Peter Maydell
On Fri, 8 Mar 2024 at 17:03, Maciej S. Szmigiero
 wrote:
>
> From: "Maciej S. Szmigiero" 
>
> The following changes since commit 8f6330a807f2642dc2a3cdf33347aa28a4c00a87:
>
>   Merge tag 'pull-maintainer-updates-060324-1' of 
> https://gitlab.com/stsquad/qemu into staging (2024-03-06 16:56:20 +)
>
> are available in the Git repository at:
>
>   https://github.com/maciejsszmigiero/qemu.git tags/pull-hv-balloon-20240308
>
> for you to fetch changes up to 6093637b4d32875f98cd59696ffc5f26884aa0b4:
>
>   vmbus: Print a warning when enabled without the recommended set of features 
> (2024-03-08 14:18:56 +0100)
>
> 
> Hyper-V Dynamic Memory and VMBus misc small patches
>
> This pull request contains two small patches to hv-balloon:
> the first one replacing alloca() usage with g_malloc0() + g_autofree
> and the second one adding additional declaration of a protocol message
> struct with an optional field explicitly defined to avoid a Coverity
> warning.
>
> Also included is a VMBus patch to print a warning when it is enabled
> without the recommended set of Hyper-V features (enlightenments) since
> some Windows versions crash at boot in this case.
>
> 
> Maciej S. Szmigiero (3):
>   hv-balloon: avoid alloca() usage
>   hv-balloon: define dm_hot_add_with_region to avoid Coverity warning
>   vmbus: Print a warning when enabled without the recommended set of 
> features
>


Applied, thanks.

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

-- PMM



[PULL 13/43] hw/xen/hvm: Propagate page_mask to a pair of functions

2024-03-09 Thread Philippe Mathieu-Daudé
We are going to replace TARGET_PAGE_MASK by a
runtime variable. In order to reduce code duplication,
propagate TARGET_PAGE_MASK to get_physmapping() and
xen_phys_offset_to_gaddr().

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Manos Pitsidianakis 
Message-Id: <20231114163123.74888-3-phi...@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 18 ++
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 8235782ef7..844b11ae08 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -174,11 +174,12 @@ static void xen_ram_init(PCMachineState *pcms,
 }
 }
 
-static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size)
+static XenPhysmap *get_physmapping(hwaddr start_addr, ram_addr_t size,
+   int page_mask)
 {
 XenPhysmap *physmap = NULL;
 
-start_addr &= TARGET_PAGE_MASK;
+start_addr &= page_mask;
 
 QLIST_FOREACH(physmap, &xen_physmap, list) {
 if (range_covers_byte(physmap->start_addr, physmap->size, start_addr)) 
{
@@ -188,9 +189,10 @@ static XenPhysmap *get_physmapping(hwaddr start_addr, 
ram_addr_t size)
 return NULL;
 }
 
-static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size)
+static hwaddr xen_phys_offset_to_gaddr(hwaddr phys_offset, ram_addr_t size,
+   int page_mask)
 {
-hwaddr addr = phys_offset & TARGET_PAGE_MASK;
+hwaddr addr = phys_offset & page_mask;
 XenPhysmap *physmap = NULL;
 
 QLIST_FOREACH(physmap, &xen_physmap, list) {
@@ -252,7 +254,7 @@ static int xen_add_to_physmap(XenIOState *state,
 hwaddr phys_offset = memory_region_get_ram_addr(mr);
 const char *mr_name;
 
-if (get_physmapping(start_addr, size)) {
+if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) {
 return 0;
 }
 if (size <= 0) {
@@ -325,7 +327,7 @@ static int xen_remove_from_physmap(XenIOState *state,
 XenPhysmap *physmap = NULL;
 hwaddr phys_offset = 0;
 
-physmap = get_physmapping(start_addr, size);
+physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
 if (physmap == NULL) {
 return -1;
 }
@@ -373,7 +375,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
 int rc, i, j;
 const XenPhysmap *physmap = NULL;
 
-physmap = get_physmapping(start_addr, size);
+physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
 if (physmap == NULL) {
 /* not handled */
 return;
@@ -633,7 +635,7 @@ void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t 
length)
 int rc;
 ram_addr_t start_pfn, nb_pages;
 
-start = xen_phys_offset_to_gaddr(start, length);
+start = xen_phys_offset_to_gaddr(start, length, TARGET_PAGE_MASK);
 
 if (length == 0) {
 length = TARGET_PAGE_SIZE;
-- 
2.41.0




[PULL 24/43] qdev: Add a granule_mode property

2024-03-09 Thread Philippe Mathieu-Daudé
From: Eric Auger 

Introduce a new enum type property allowing to set an
IOMMU granule. Values are 4k, 8k, 16k, 64k and host.
This latter indicates the vIOMMU granule will match
the host page size.

A subsequent patch will add such a property to the
virtio-iommu device.

Signed-off-by: Eric Auger 
Reviewed-by: Zhenzhong Duan 
Signed-off-by: Philippe Mathieu-Daudé 
Message-ID: <20240227165730.14099-2-eric.au...@redhat.com>
---
 qapi/virtio.json| 18 ++
 include/hw/qdev-properties-system.h |  3 +++
 hw/core/qdev-properties-system.c| 14 ++
 3 files changed, 35 insertions(+)

diff --git a/qapi/virtio.json b/qapi/virtio.json
index a79013fe89..95745fdfd7 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -957,3 +957,21 @@
 
 { 'struct': 'DummyVirtioForceArrays',
   'data': { 'unused-iothread-vq-mapping': ['IOThreadVirtQueueMapping'] } }
+
+##
+# @GranuleMode:
+#
+# @4k: granule page size of 4KiB
+#
+# @8k: granule page size of 8KiB
+#
+# @16k: granule page size of 16KiB
+#
+# @64k: granule page size of 64KiB
+#
+# @host: granule matches the host page size
+#
+# Since: 9.0
+##
+{ 'enum': 'GranuleMode',
+  'data': [ '4k', '8k', '16k', '64k', 'host' ] }
diff --git a/include/hw/qdev-properties-system.h 
b/include/hw/qdev-properties-system.h
index 06c359c190..626be87dd3 100644
--- a/include/hw/qdev-properties-system.h
+++ b/include/hw/qdev-properties-system.h
@@ -8,6 +8,7 @@ extern const PropertyInfo qdev_prop_macaddr;
 extern const PropertyInfo qdev_prop_reserved_region;
 extern const PropertyInfo qdev_prop_multifd_compression;
 extern const PropertyInfo qdev_prop_mig_mode;
+extern const PropertyInfo qdev_prop_granule_mode;
 extern const PropertyInfo qdev_prop_losttickpolicy;
 extern const PropertyInfo qdev_prop_blockdev_on_error;
 extern const PropertyInfo qdev_prop_bios_chs_trans;
@@ -47,6 +48,8 @@ extern const PropertyInfo qdev_prop_iothread_vq_mapping_list;
 #define DEFINE_PROP_MIG_MODE(_n, _s, _f, _d) \
 DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_mig_mode, \
MigMode)
+#define DEFINE_PROP_GRANULE_MODE(_n, _s, _f, _d) \
+DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_granule_mode, GranuleMode)
 #define DEFINE_PROP_LOSTTICKPOLICY(_n, _s, _f, _d) \
 DEFINE_PROP_SIGNED(_n, _s, _f, _d, qdev_prop_losttickpolicy, \
 LostTickPolicy)
diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 1a396521d5..b45e90edb2 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -679,6 +679,20 @@ const PropertyInfo qdev_prop_mig_mode = {
 .set_default_value = qdev_propinfo_set_default_value_enum,
 };
 
+/* --- GranuleMode --- */
+
+QEMU_BUILD_BUG_ON(sizeof(GranuleMode) != sizeof(int));
+
+const PropertyInfo qdev_prop_granule_mode = {
+.name = "GranuleMode",
+.description = "granule_mode values, "
+   "4k, 8k, 16k, 64k, host",
+.enum_table = &GranuleMode_lookup,
+.get = qdev_propinfo_get_enum,
+.set = qdev_propinfo_set_enum,
+.set_default_value = qdev_propinfo_set_default_value_enum,
+};
+
 /* --- Reserved Region --- */
 
 /*
-- 
2.41.0




[PULL 42/43] tests/unit/test-smp-parse: Test "parameter=0" SMP configurations

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

The support for "parameter=0" SMP configurations is removed, and QEMU
returns error for those cases.

So add the related test cases to ensure parameters can't accept 0.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-14-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 92 +
 1 file changed, 92 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index d39cfdc19b..8994337e12 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -524,6 +524,91 @@ static const struct SMPTestData data_full_topo_invalid[] = 
{
 },
 };
 
+static const struct SMPTestData data_zero_topo_invalid[] = {
+{
+/*
+ * Test "cpus=0".
+ * config: -smp 0,drawers=1,books=1,sockets=1,dies=1,\
+ *  clusters=1,cores=1,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(0, 1, 1, 1, 1, 1, 1, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "drawers=0".
+ * config: -smp 1,drawers=0,books=1,sockets=1,dies=1,\
+ *  clusters=1,cores=1,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 0, 1, 1, 1, 1, 1, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "books=0".
+ * config: -smp 1,drawers=1,books=0,sockets=1,dies=1,\
+ *  clusters=1,cores=1,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 0, 1, 1, 1, 1, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "sockets=0".
+ * config: -smp 1,drawers=1,books=1,sockets=0,dies=1,\
+ *  clusters=1,cores=1,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 0, 1, 1, 1, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "dies=0".
+ * config: -smp 1,drawers=1,books=1,sockets=1,dies=0,\
+ *  clusters=1,cores=1,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 0, 1, 1, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "clusters=0".
+ * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
+ *  clusters=0,cores=1,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 0, 1, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "cores=0".
+ * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
+ *  clusters=1,cores=0,threads=1,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 1, 0, 1, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "threads=0".
+ * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
+ *  clusters=1,cores=1,threads=0,maxcpus=1
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 1, 1, 0, 1),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+}, {
+/*
+ * Test "maxcpus=0".
+ * config: -smp 1,drawers=1,books=1,sockets=1,dies=1,\
+ *  clusters=1,cores=1,threads=1,maxcpus=0
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 1, 1, 1, 1, 1, 1, 1, 0),
+.expect_error = "Invalid CPU topology: CPU topology parameters must "
+"be greater than zero",
+},
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
 return g_strdup_printf(
@@ -1173,6 +1258,13 @@ static void test_full_topo(const void *opaque)
 smp_parse_test(ms, &data, false);
 }
 
+for (i = 0; i < ARRAY_SIZE(data_zero_topo_invalid); i++) {
+data = data_zero_topo_invalid[i];
+unsupported_params_init(mc, &data);
+
+smp_parse_test(ms, &data, false);
+}
+
 object_unref(obj);
 }
 
-- 
2.41.0




[PULL 19/43] hw/i386/pc: Remove pc_compat_1_4..1.7[] left over declarations

2024-03-09 Thread Philippe Mathieu-Daudé
These definitions were removed in commit ea985d235b
("pc_piix: remove pc-i440fx-1.4 up to pc-i440fx-1.7").

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20240301185936.95175-2-phi...@linaro.org>
---
 include/hw/i386/pc.h | 12 
 1 file changed, 12 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 5065590281..b958023187 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -288,18 +288,6 @@ extern const size_t pc_compat_2_1_len;
 extern GlobalProperty pc_compat_2_0[];
 extern const size_t pc_compat_2_0_len;
 
-extern GlobalProperty pc_compat_1_7[];
-extern const size_t pc_compat_1_7_len;
-
-extern GlobalProperty pc_compat_1_6[];
-extern const size_t pc_compat_1_6_len;
-
-extern GlobalProperty pc_compat_1_5[];
-extern const size_t pc_compat_1_5_len;
-
-extern GlobalProperty pc_compat_1_4[];
-extern const size_t pc_compat_1_4_len;
-
 #define DEFINE_PC_MACHINE(suffix, namestr, initfn, optsfn) \
 static void pc_machine_##suffix##_class_init(ObjectClass *oc, void *data) \
 { \
-- 
2.41.0




[PULL 22/43] hw/i386/pc: Have pc_init_isa() pass a NULL pci_type argument

2024-03-09 Thread Philippe Mathieu-Daudé
The "isapc" machine only provides an ISA bus, not a PCI one,
and doesn't instanciate any i440FX south bridge.
Its machine class defines PCMachineClass::pci_enabled = false,
and pc_init1() only uses the pci_type argument when pci_enabled
is true. Since for this machine the argument is not used,
passing NULL makes more sense.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Bernhard Beschow 
Message-Id: <20240301185936.95175-5-phi...@linaro.org>
---
 hw/i386/pc_piix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e14dce951d..319bc4b180 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -450,7 +450,7 @@ static void pc_compat_2_0_fn(MachineState *machine)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
-pc_init1(machine, TYPE_I440FX_PCI_DEVICE);
+pc_init1(machine, NULL);
 }
 #endif
 
-- 
2.41.0




[PULL 38/43] tests/unit/test-smp-parse: Test "drawers" parameter in -smp

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Although drawer was introduced to -smp along with book by s390 machine,
as a general topology level in QEMU that may be reused by other arches
in the future, it is desirable to cover this parameter's parsing in a
separate case.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-10-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 89 +
 1 file changed, 89 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 090238ab23..aea1b2e73a 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -329,6 +329,11 @@ static const struct SMPTestData data_generic_invalid[] = {
 .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F,
 0, F, 0, F, 0, F, 0),
 .expect_error = "books not supported by this machine's CPU topology",
+}, {
+/* config: -smp 2,drawers=2 */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, T, 2, F, 0, F,
+0, F, 0, F, 0, F, 0),
+.expect_error = "drawers not supported by this machine's CPU topology",
 }, {
 /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
 .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
@@ -420,6 +425,26 @@ static const struct SMPTestData data_with_books_invalid[] 
= {
 },
 };
 
+static const struct SMPTestData data_with_drawers_invalid[] = {
+{
+/* config: -smp 16,drawers=2,sockets=2,cores=4,threads=2,maxcpus=16 */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, T, 2, F, 1, T,
+2, T, 4, T, 2, T, 16),
+.expect_error = "Invalid CPU topology: "
+"product of the hierarchy must match maxcpus: "
+"drawers (2) * sockets (2) * cores (4) * threads (2) "
+"!= maxcpus (16)",
+}, {
+/* config: -smp 34,drawers=2,sockets=2,cores=4,threads=2,maxcpus=32 */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, T, 2, F, 1, T,
+2, T, 4, T, 2, T, 32),
+.expect_error = "Invalid CPU topology: "
+"maxcpus must be equal to or greater than smp: "
+"drawers (2) * sockets (2) * cores (4) * threads (2) "
+"== maxcpus (32) < smp_cpus (34)",
+},
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
 return g_strdup_printf(
@@ -666,6 +691,13 @@ static void machine_with_books_class_init(ObjectClass *oc, 
void *data)
 mc->smp_props.books_supported = true;
 }
 
+static void machine_with_drawers_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->smp_props.drawers_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
 const char *machine_type = opaque;
@@ -854,6 +886,56 @@ static void test_with_books(const void *opaque)
 object_unref(obj);
 }
 
+static void test_with_drawers(const void *opaque)
+{
+const char *machine_type = opaque;
+Object *obj = object_new(machine_type);
+MachineState *ms = MACHINE(obj);
+MachineClass *mc = MACHINE_GET_CLASS(obj);
+SMPTestData data = {};
+unsigned int num_drawers = 2;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+data = data_generic_valid[i];
+unsupported_params_init(mc, &data);
+
+/* when drawers parameter is omitted, it will be set as 1 */
+data.expect_prefer_sockets.drawers = 1;
+data.expect_prefer_cores.drawers = 1;
+
+smp_parse_test(ms, &data, true);
+
+/* when drawers parameter is specified */
+data.config.has_drawers = true;
+data.config.drawers = num_drawers;
+if (data.config.has_cpus) {
+data.config.cpus *= num_drawers;
+}
+if (data.config.has_maxcpus) {
+data.config.maxcpus *= num_drawers;
+}
+
+data.expect_prefer_sockets.drawers = num_drawers;
+data.expect_prefer_sockets.cpus *= num_drawers;
+data.expect_prefer_sockets.max_cpus *= num_drawers;
+data.expect_prefer_cores.drawers = num_drawers;
+data.expect_prefer_cores.cpus *= num_drawers;
+data.expect_prefer_cores.max_cpus *= num_drawers;
+
+smp_parse_test(ms, &data, true);
+}
+
+for (i = 0; i < ARRAY_SIZE(data_with_drawers_invalid); i++) {
+data = data_with_drawers_invalid[i];
+unsupported_params_init(mc, &data);
+
+smp_parse_test(ms, &data, false);
+}
+
+object_unref(obj);
+}
+
 /* Type info of the tested machine */
 static const TypeInfo smp_machine_types[] = {
 {
@@ -882,6 +964,10 @@ static const TypeInfo smp_machine_types[] = {
 .name  

[PULL 36/43] tests/unit/test-smp-parse: Make test cases aware of the book/drawer

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Currently, -smp supports 2 more new levels: book and drawer.

It is necessary to consider the effects of book and drawer in the test
cases to ensure that the calculations are correct. This is also the
preparation to add new book and drawer test cases.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-8-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 25 ++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 2eb9533bc5..f656bbb6da 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -384,6 +384,8 @@ static char *smp_config_to_string(const SMPConfiguration 
*config)
 return g_strdup_printf(
 "(SMPConfiguration) {\n"
 ".has_cpus = %5s, cpus = %" PRId64 ",\n"
+".has_drawers  = %5s, drawers  = %" PRId64 ",\n"
+".has_books= %5s, books= %" PRId64 ",\n"
 ".has_sockets  = %5s, sockets  = %" PRId64 ",\n"
 ".has_dies = %5s, dies = %" PRId64 ",\n"
 ".has_clusters = %5s, clusters = %" PRId64 ",\n"
@@ -392,6 +394,8 @@ static char *smp_config_to_string(const SMPConfiguration 
*config)
 ".has_maxcpus  = %5s, maxcpus  = %" PRId64 ",\n"
 "}",
 config->has_cpus ? "true" : "false", config->cpus,
+config->has_drawers ? "true" : "false", config->drawers,
+config->has_books ? "true" : "false", config->books,
 config->has_sockets ? "true" : "false", config->sockets,
 config->has_dies ? "true" : "false", config->dies,
 config->has_clusters ? "true" : "false", config->clusters,
@@ -404,10 +408,10 @@ static char *smp_config_to_string(const SMPConfiguration 
*config)
 static unsigned int cpu_topology_get_threads_per_socket(const CpuTopology 
*topo)
 {
 /* Check the divisor to avoid invalid topology examples causing SIGFPE. */
-if (!topo->sockets) {
+if (!topo->drawers || !topo->books || !topo->sockets) {
 return 0;
 } else {
-return topo->max_cpus / topo->sockets;
+return topo->max_cpus / topo->drawers / topo->books / topo->sockets;
 }
 }
 
@@ -429,6 +433,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
 return g_strdup_printf(
 "(CpuTopology) {\n"
 ".cpus   = %u,\n"
+".drawers= %u,\n"
+".books  = %u,\n"
 ".sockets= %u,\n"
 ".dies   = %u,\n"
 ".clusters   = %u,\n"
@@ -438,7 +444,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
 ".threads_per_socket = %u,\n"
 ".cores_per_socket   = %u,\n"
 "}",
-topo->cpus, topo->sockets, topo->dies, topo->clusters,
+topo->cpus, topo->drawers, topo->books,
+topo->sockets, topo->dies, topo->clusters,
 topo->cores, topo->threads, topo->max_cpus,
 threads_per_socket, cores_per_socket);
 }
@@ -473,6 +480,8 @@ static void check_parse(MachineState *ms, const 
SMPConfiguration *config,
 if (is_valid) {
 if ((err == NULL) &&
 (ms->smp.cpus == expect_topo->cpus) &&
+(ms->smp.drawers == expect_topo->drawers) &&
+(ms->smp.books == expect_topo->books) &&
 (ms->smp.sockets == expect_topo->sockets) &&
 (ms->smp.dies == expect_topo->dies) &&
 (ms->smp.clusters == expect_topo->clusters) &&
@@ -564,6 +573,16 @@ static void unsupported_params_init(const MachineClass 
*mc, SMPTestData *data)
 data->expect_prefer_sockets.clusters = 1;
 data->expect_prefer_cores.clusters = 1;
 }
+
+if (!mc->smp_props.books_supported) {
+data->expect_prefer_sockets.books = 1;
+data->expect_prefer_cores.books = 1;
+}
+
+if (!mc->smp_props.drawers_supported) {
+data->expect_prefer_sockets.drawers = 1;
+data->expect_prefer_cores.drawers = 1;
+}
 }
 
 static void machine_base_class_init(ObjectClass *oc, void *data)
-- 
2.41.0




[PULL 27/43] hw/intc/grlib_irqmp: abort realize when ncpus value is out of range

2024-03-09 Thread Philippe Mathieu-Daudé
From: Clément Chigot 

Even if the error is set, the build is not aborted when the ncpus value
is wrong, the return is missing.

Signed-off-by: Clément Chigot 
Reviewed-by: Peter Maydell 
Fixes: 6bf1478543 ("hw/intc/grlib_irqmp: add ncpus property")
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240308152719.591232-1-chi...@adacore.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/intc/grlib_irqmp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/intc/grlib_irqmp.c b/hw/intc/grlib_irqmp.c
index 144b121d48..c6c51a349c 100644
--- a/hw/intc/grlib_irqmp.c
+++ b/hw/intc/grlib_irqmp.c
@@ -356,6 +356,7 @@ static void grlib_irqmp_realize(DeviceState *dev, Error 
**errp)
 error_setg(errp, "Invalid ncpus properties: "
"%u, must be 0 < ncpus =< %u.", irqmp->ncpus,
IRQMP_MAX_CPU);
+return;
 }
 
 qdev_init_gpio_in(dev, grlib_irqmp_set_irq, MAX_PILS);
-- 
2.41.0




[PULL 41/43] tests/unit/test-smp-parse: Test smp_props.has_clusters

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

The smp_props.has_clusters in MachineClass is not a user configured
field, and it indicates if user specifies "clusters" in -smp.

After -smp parsing, other module could aware if the cluster level
is configured by user. This is used when the machine has only 1 cluster
since there's only 1 cluster by default.

Add the check to cover this field.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Acked-by: Thomas Huth 
Message-ID: <20240308160148.3130837-13-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 20 ++--
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 7558169171..d39cfdc19b 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -573,7 +573,8 @@ static unsigned int cpu_topology_get_cores_per_socket(const 
CpuTopology *topo)
 
 static char *cpu_topology_to_string(const CpuTopology *topo,
 unsigned int threads_per_socket,
-unsigned int cores_per_socket)
+unsigned int cores_per_socket,
+bool has_clusters)
 {
 return g_strdup_printf(
 "(CpuTopology) {\n"
@@ -588,17 +589,20 @@ static char *cpu_topology_to_string(const CpuTopology 
*topo,
 ".max_cpus   = %u,\n"
 ".threads_per_socket = %u,\n"
 ".cores_per_socket   = %u,\n"
+".has_clusters   = %s,\n"
 "}",
 topo->cpus, topo->drawers, topo->books,
 topo->sockets, topo->dies, topo->clusters,
 topo->cores, topo->threads, topo->max_cpus,
-threads_per_socket, cores_per_socket);
+threads_per_socket, cores_per_socket,
+has_clusters ? "true" : "false");
 }
 
 static void check_parse(MachineState *ms, const SMPConfiguration *config,
 const CpuTopology *expect_topo, const char *expect_err,
 bool is_valid)
 {
+MachineClass *mc = MACHINE_GET_CLASS(ms);
 g_autofree char *config_str = smp_config_to_string(config);
 g_autofree char *expect_topo_str = NULL, *output_topo_str = NULL;
 unsigned int expect_threads_per_socket, expect_cores_per_socket;
@@ -611,15 +615,18 @@ static void check_parse(MachineState *ms, const 
SMPConfiguration *config,
 cpu_topology_get_cores_per_socket(expect_topo);
 expect_topo_str = cpu_topology_to_string(expect_topo,
  expect_threads_per_socket,
- expect_cores_per_socket);
+ expect_cores_per_socket,
+ config->has_clusters);
 
 /* call the generic parser */
 machine_parse_smp_config(ms, config, &err);
 
 ms_threads_per_socket = machine_topo_get_threads_per_socket(ms);
 ms_cores_per_socket = machine_topo_get_cores_per_socket(ms);
-output_topo_str = cpu_topology_to_string(&ms->smp, ms_threads_per_socket,
- ms_cores_per_socket);
+output_topo_str = cpu_topology_to_string(&ms->smp,
+ ms_threads_per_socket,
+ ms_cores_per_socket,
+ mc->smp_props.has_clusters);
 
 /* when the configuration is supposed to be valid */
 if (is_valid) {
@@ -634,7 +641,8 @@ static void check_parse(MachineState *ms, const 
SMPConfiguration *config,
 (ms->smp.threads == expect_topo->threads) &&
 (ms->smp.max_cpus == expect_topo->max_cpus) &&
 (ms_threads_per_socket == expect_threads_per_socket) &&
-(ms_cores_per_socket == expect_cores_per_socket)) {
+(ms_cores_per_socket == expect_cores_per_socket) &&
+(mc->smp_props.has_clusters == config->has_clusters)) {
 return;
 }
 
-- 
2.41.0




[PULL 32/43] hw/core/machine-smp: Calculate total CPUs once in machine_parse_smp_config()

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

In machine_parse_smp_config(), the number of total CPUs is calculated
by:

drawers * books * sockets * dies * clusters * cores * threads

To avoid missing the future new topology level, use a local variable to
cache the calculation result so that total CPUs are only calculated
once.

Signed-off-by: Zhao Liu 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240308160148.3130837-4-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/core/machine-smp.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 50a5a40dbc..27864c9507 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -91,6 +91,7 @@ void machine_parse_smp_config(MachineState *ms,
 unsigned cores   = config->has_cores ? config->cores : 0;
 unsigned threads = config->has_threads ? config->threads : 0;
 unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0;
+unsigned total_cpus;
 
 /*
  * Specified CPU topology parameters must be greater than zero,
@@ -211,8 +212,8 @@ void machine_parse_smp_config(MachineState *ms,
 }
 }
 
-maxcpus = maxcpus > 0 ? maxcpus : drawers * books * sockets * dies *
-  clusters * cores * threads;
+total_cpus = drawers * books * sockets * dies * clusters * cores * threads;
+maxcpus = maxcpus > 0 ? maxcpus : total_cpus;
 cpus = cpus > 0 ? cpus : maxcpus;
 
 ms->smp.cpus = cpus;
@@ -228,8 +229,7 @@ void machine_parse_smp_config(MachineState *ms,
 mc->smp_props.has_clusters = config->has_clusters;
 
 /* sanity-check of the computed topology */
-if (drawers * books * sockets * dies * clusters * cores * threads !=
-maxcpus) {
+if (total_cpus != maxcpus) {
 g_autofree char *topo_msg = cpu_hierarchy_to_string(ms);
 error_setg(errp, "Invalid CPU topology: "
"product of the hierarchy must match maxcpus: "
-- 
2.41.0




[PULL 33/43] tests/unit/test-smp-parse: Drop the unsupported "dies=1" case

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Unsupported "parameter=1" SMP configurations is marked as deprecated,
so drop the related test case.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-5-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 24972666a7..1874bea086 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -607,11 +607,6 @@ static void test_generic_valid(const void *opaque)
 unsupported_params_init(mc, &data);
 
 smp_parse_test(ms, &data, true);
-
-/* Unsupported parameters can be provided with their values as 1 */
-data.config.has_dies = true;
-data.config.dies = 1;
-smp_parse_test(ms, &data, true);
 }
 
 object_unref(obj);
-- 
2.41.0




[PULL 39/43] tests/unit/test-smp-parse: Test "drawers" and "books" combination case

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Since s390 machine supports both "drawers" and "books" in -smp, add the
"drawers" and "books" combination test case to match the actual topology
usage scenario.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-11-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 103 
 1 file changed, 103 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index aea1b2e73a..0cf6115198 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -445,6 +445,33 @@ static const struct SMPTestData 
data_with_drawers_invalid[] = {
 },
 };
 
+static const struct SMPTestData data_with_drawers_books_invalid[] = {
+{
+/*
+ * config: -smp 200,drawers=2,books=2,sockets=2,cores=4,\
+ * threads=2,maxcpus=200
+ */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 200, T, 3, T, 5, T,
+2, T, 4, T, 2, T, 200),
+.expect_error = "Invalid CPU topology: "
+"product of the hierarchy must match maxcpus: "
+"drawers (3) * books (5) * sockets (2) * "
+"cores (4) * threads (2) != maxcpus (200)",
+}, {
+/*
+ * config: -smp 242,drawers=2,books=2,sockets=2,cores=4,\
+ * threads=2,maxcpus=240
+ */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 242, T, 3, T, 5, T,
+2, T, 4, T, 2, T, 240),
+.expect_error = "Invalid CPU topology: "
+"maxcpus must be equal to or greater than smp: "
+"drawers (3) * books (5) * sockets (2) * "
+"cores (4) * threads (2) "
+"== maxcpus (240) < smp_cpus (242)",
+},
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
 return g_strdup_printf(
@@ -698,6 +725,14 @@ static void machine_with_drawers_class_init(ObjectClass 
*oc, void *data)
 mc->smp_props.drawers_supported = true;
 }
 
+static void machine_with_drawers_books_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->smp_props.drawers_supported = true;
+mc->smp_props.books_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
 const char *machine_type = opaque;
@@ -936,6 +971,67 @@ static void test_with_drawers(const void *opaque)
 object_unref(obj);
 }
 
+static void test_with_drawers_books(const void *opaque)
+{
+const char *machine_type = opaque;
+Object *obj = object_new(machine_type);
+MachineState *ms = MACHINE(obj);
+MachineClass *mc = MACHINE_GET_CLASS(obj);
+SMPTestData data = {};
+unsigned int num_drawers = 5, num_books = 3;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+data = data_generic_valid[i];
+unsupported_params_init(mc, &data);
+
+/*
+ * when drawers and books parameters are omitted, they will
+ * be both set as 1.
+ */
+data.expect_prefer_sockets.drawers = 1;
+data.expect_prefer_sockets.books = 1;
+data.expect_prefer_cores.drawers = 1;
+data.expect_prefer_cores.books = 1;
+
+smp_parse_test(ms, &data, true);
+
+/* when drawers and books parameters are both specified */
+data.config.has_drawers = true;
+data.config.drawers = num_drawers;
+data.config.has_books = true;
+data.config.books = num_books;
+
+if (data.config.has_cpus) {
+data.config.cpus *= num_drawers * num_books;
+}
+if (data.config.has_maxcpus) {
+data.config.maxcpus *= num_drawers * num_books;
+}
+
+data.expect_prefer_sockets.drawers = num_drawers;
+data.expect_prefer_sockets.books = num_books;
+data.expect_prefer_sockets.cpus *= num_drawers * num_books;
+data.expect_prefer_sockets.max_cpus *= num_drawers * num_books;
+
+data.expect_prefer_cores.drawers = num_drawers;
+data.expect_prefer_cores.books = num_books;
+data.expect_prefer_cores.cpus *= num_drawers * num_books;
+data.expect_prefer_cores.max_cpus *= num_drawers * num_books;
+
+smp_parse_test(ms, &data, true);
+}
+
+for (i = 0; i < ARRAY_SIZE(data_with_drawers_books_invalid); i++) {
+data = data_with_drawers_books_invalid[i];
+unsupported_params_init(mc, &data);
+
+smp_parse_test(ms, &data, false);
+}
+
+object_unref(obj);
+}
+
 /* Type info of the tested machine */
 static const TypeInfo smp_machine_types[] = {
 {
@@ -968,6 +1064,10 @@ static const TypeInfo smp_machine_types[] = {
 .name   = MACHINE_TYPE_NAME("smp-with-drawers"),
 .parent = TYPE_MACHINE,
 .class_init

[PULL 30/43] hw/core/machine-smp: Remove deprecated "parameter=0" SMP configurations

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

The "parameter=0" SMP configurations have been marked as deprecated
since v6.2.

For these cases, -smp currently returns the warning and adjusts the
zeroed parameters to 1 by default.

Remove the above compatibility logic in v9.0, and return error directly
if any -smp parameter is set as 0.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Reviewed-by: Prasad Pandit 
Message-ID: <20240308160148.3130837-2-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/about/deprecated.rst   | 16 
 docs/about/removed-features.rst | 15 +++
 hw/core/machine-smp.c   |  5 +++--
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 8565644da6..6e2f557682 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -36,22 +36,6 @@ and will cause a warning.
 The replacement for the ``nodelay`` short-form boolean option is ``nodelay=on``
 rather than ``delay=off``.
 
-``-smp`` ("parameter=0" SMP configurations) (since 6.2)
-'''
-
-Specified CPU topology parameters must be greater than zero.
-
-In the SMP configuration, users should either provide a CPU topology
-parameter with a reasonable value (greater than zero) or just omit it
-and QEMU will compute the missing value.
-
-However, historically it was implicitly allowed for users to provide
-a parameter with zero value, which is meaningless and could also possibly
-cause unexpected results in the -smp parsing. So support for this kind of
-configurations (e.g. -smp 8,sockets=0) is deprecated since 6.2 and will
-be removed in the near future, users have to ensure that all the topology
-members described with -smp are greater than zero.
-
 Plugin argument passing through ``arg=`` (since 6.1)
 
 
diff --git a/docs/about/removed-features.rst b/docs/about/removed-features.rst
index 417a0e4fa1..f9cf874f7b 100644
--- a/docs/about/removed-features.rst
+++ b/docs/about/removed-features.rst
@@ -489,6 +489,21 @@ The ``-singlestep`` option has been turned into an 
accelerator property,
 and given a name that better reflects what it actually does.
 Use ``-accel tcg,one-insn-per-tb=on`` instead.
 
+``-smp`` ("parameter=0" SMP configurations) (removed in 9.0)
+
+
+Specified CPU topology parameters must be greater than zero.
+
+In the SMP configuration, users should either provide a CPU topology
+parameter with a reasonable value (greater than zero) or just omit it
+and QEMU will compute the missing value.
+
+However, historically it was implicitly allowed for users to provide
+a parameter with zero value, which is meaningless and could also possibly
+cause unexpected results in the -smp parsing. So support for this kind of
+configurations (e.g. -smp 8,sockets=0) is removed since 9.0, users have
+to ensure that all the topology members described with -smp are greater
+than zero.
 
 User-mode emulator command line arguments
 -
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 25019c91ee..96533886b1 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -105,8 +105,9 @@ void machine_parse_smp_config(MachineState *ms,
 (config->has_cores && config->cores == 0) ||
 (config->has_threads && config->threads == 0) ||
 (config->has_maxcpus && config->maxcpus == 0)) {
-warn_report("Deprecated CPU topology (considered invalid): "
-"CPU topology parameters must be greater than zero");
+error_setg(errp, "Invalid CPU topology: "
+   "CPU topology parameters must be greater than zero");
+return;
 }
 
 /*
-- 
2.41.0




[PULL 31/43] hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP configurations

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Currently, it was allowed for users to specify the unsupported
topology parameter as "1". For example, x86 PC machine doesn't
support drawer/book/cluster topology levels, but user could specify
"-smp drawers=1,books=1,clusters=1".

This is meaningless and confusing, so that the support for this kind of
configurations is marked deprecated since 9.0. And report warning
message for such case like:

qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported clusters parameter mustn't be specified as 1
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported books parameter mustn't be specified as 1
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported drawers parameter mustn't be specified as 1

Users have to ensure that all the topology members described with -smp
are supported by the target machine.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-3-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/about/deprecated.rst | 14 +
 hw/core/machine-smp.c | 65 +--
 2 files changed, 62 insertions(+), 17 deletions(-)

diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 6e2f557682..dfd681cd02 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -57,6 +57,20 @@ The ``-p`` option pretends to control the host page size.  
However,
 it is not possible to change the host page size, and using the
 option only causes failures.
 
+``-smp`` (Unsupported "parameter=1" SMP configurations) (since 9.0)
+'''
+
+Specified CPU topology parameters must be supported by the machine.
+
+In the SMP configuration, users should provide the CPU topology parameters that
+are supported by the target machine.
+
+However, historically it was allowed for users to specify the unsupported
+topology parameter as "1", which is meaningless. So support for this kind of
+configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is
+marked deprecated since 9.0, users have to ensure that all the topology members
+described with -smp are supported by the target machine.
+
 QEMU Machine Protocol (QMP) commands
 
 
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 96533886b1..50a5a40dbc 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -112,30 +112,61 @@ void machine_parse_smp_config(MachineState *ms,
 
 /*
  * If not supported by the machine, a topology parameter must be
- * omitted or specified equal to 1.
+ * omitted.
  */
-if (!mc->smp_props.dies_supported && dies > 1) {
-error_setg(errp, "dies not supported by this machine's CPU topology");
-return;
+if (!mc->smp_props.clusters_supported && config->has_clusters) {
+if (config->clusters > 1) {
+error_setg(errp, "clusters not supported by this "
+   "machine's CPU topology");
+return;
+} else {
+/* Here clusters only equals 1 since we've checked zero case. */
+warn_report("Deprecated CPU topology (considered invalid): "
+"Unsupported clusters parameter mustn't be "
+"specified as 1");
+}
 }
-if (!mc->smp_props.clusters_supported && clusters > 1) {
-error_setg(errp, "clusters not supported by this machine's CPU 
topology");
-return;
-}
-
-dies = dies > 0 ? dies : 1;
 clusters = clusters > 0 ? clusters : 1;
 
-if (!mc->smp_props.books_supported && books > 1) {
-error_setg(errp, "books not supported by this machine's CPU topology");
-return;
+if (!mc->smp_props.dies_supported && config->has_dies) {
+if (config->dies > 1) {
+error_setg(errp, "dies not supported by this "
+   "machine's CPU topology");
+return;
+} else {
+/* Here dies only equals 1 since we've checked zero case. */
+warn_report("Deprecated CPU topology (considered invalid): "
+"Unsupported dies parameter mustn't be "
+"specified as 1");
+}
+}
+dies = dies > 0 ? dies : 1;
+
+if (!mc->smp_props.books_supported && config->has_books) {
+if (config->books > 1) {
+error_setg(errp, "books not supported by this "
+   "machine's CPU topology");
+return;
+} else {
+/* Here books only equals 1 since we've checked zero case. */
+warn_report("Deprecated CPU topology (considered invalid): "
+"Unsupported books parameter mustn't be "
+"specified as 1");
+}
 }
 bo

[PULL 25/43] hmp: Add option to info qtree to omit details

2024-03-09 Thread Philippe Mathieu-Daudé
From: BALATON Zoltan 

The output of info qtree monitor command is very long. Add an option
to print a brief overview omitting all the details.

Signed-off-by: BALATON Zoltan 
Reviewed-by: Dr. David Alan Gilbert 
Message-ID: <20240307183812.0105d4e6...@zero.eik.bme.hu>
Signed-off-by: Philippe Mathieu-Daudé 
---
 system/qdev-monitor.c | 27 +++
 hmp-commands-info.hx  |  6 +++---
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c
index a13db763e5..ad91e74181 100644
--- a/system/qdev-monitor.c
+++ b/system/qdev-monitor.c
@@ -744,7 +744,6 @@ DeviceState *qdev_device_add(QemuOpts *opts, Error **errp)
 }
 
 #define qdev_printf(fmt, ...) monitor_printf(mon, "%*s" fmt, indent, "", ## 
__VA_ARGS__)
-static void qbus_print(Monitor *mon, BusState *bus, int indent);
 
 static void qdev_print_props(Monitor *mon, DeviceState *dev, Property *props,
  int indent)
@@ -784,13 +783,9 @@ static void bus_print_dev(BusState *bus, Monitor *mon, 
DeviceState *dev, int ind
 static void qdev_print(Monitor *mon, DeviceState *dev, int indent)
 {
 ObjectClass *class;
-BusState *child;
 NamedGPIOList *ngl;
 NamedClockList *ncl;
 
-qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
-dev->id ? dev->id : "");
-indent += 2;
 QLIST_FOREACH(ngl, &dev->gpios, node) {
 if (ngl->num_in) {
 qdev_printf("gpio-in \"%s\" %d\n", ngl->name ? ngl->name : "",
@@ -814,12 +809,9 @@ static void qdev_print(Monitor *mon, DeviceState *dev, int 
indent)
 class = object_class_get_parent(class);
 } while (class != object_class_by_name(TYPE_DEVICE));
 bus_print_dev(dev->parent_bus, mon, dev, indent);
-QLIST_FOREACH(child, &dev->child_bus, sibling) {
-qbus_print(mon, child, indent);
-}
 }
 
-static void qbus_print(Monitor *mon, BusState *bus, int indent)
+static void qbus_print(Monitor *mon, BusState *bus, int indent, bool details)
 {
 BusChild *kid;
 
@@ -827,16 +819,27 @@ static void qbus_print(Monitor *mon, BusState *bus, int 
indent)
 indent += 2;
 qdev_printf("type %s\n", object_get_typename(OBJECT(bus)));
 QTAILQ_FOREACH(kid, &bus->children, sibling) {
+BusState *child_bus;
 DeviceState *dev = kid->child;
-qdev_print(mon, dev, indent);
+qdev_printf("dev: %s, id \"%s\"\n", object_get_typename(OBJECT(dev)),
+dev->id ? dev->id : "");
+if (details) {
+qdev_print(mon, dev, indent + 2);
+}
+QLIST_FOREACH(child_bus, &dev->child_bus, sibling) {
+qbus_print(mon, child_bus, indent + 2, details);
+}
 }
 }
 #undef qdev_printf
 
 void hmp_info_qtree(Monitor *mon, const QDict *qdict)
 {
-if (sysbus_get_default())
-qbus_print(mon, sysbus_get_default(), 0);
+bool details = !qdict_get_try_bool(qdict, "brief", false);
+
+if (sysbus_get_default()) {
+qbus_print(mon, sysbus_get_default(), 0, details);
+}
 }
 
 void hmp_info_qdm(Monitor *mon, const QDict *qdict)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index da120f82a3..ad1b1306e3 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -540,9 +540,9 @@ ERST
 
 {
 .name   = "qtree",
-.args_type  = "",
-.params = "",
-.help   = "show device tree",
+.args_type  = "brief:-b",
+.params = "[-b]",
+.help   = "show device tree (-b: brief, omit properties)",
 .cmd= hmp_info_qtree,
 },
 
-- 
2.41.0




[PULL 43/43] hw/m68k/mcf5208: add support for reset

2024-03-09 Thread Philippe Mathieu-Daudé
From: Angelo Dureghello 

Add reset support for mcf5208.

Signed-off-by: Angelo Dureghello 
Reviewed-by: Thomas Huth 
Message-ID: <20240309093459.984565-1-ang...@kernel-space.org>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/m68k/mcf5208.c | 44 ++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/hw/m68k/mcf5208.c b/hw/m68k/mcf5208.c
index 0cfb806c20..ec14096aa4 100644
--- a/hw/m68k/mcf5208.c
+++ b/hw/m68k/mcf5208.c
@@ -40,6 +40,8 @@
 #define PCSR_PRE_SHIFT  8
 #define PCSR_PRE_MASK   0x0f00
 
+#define RCR_SOFTRST 0x80
+
 typedef struct {
 MemoryRegion iomem;
 qemu_irq irq;
@@ -185,12 +187,50 @@ static const MemoryRegionOps m5208_sys_ops = {
 .endianness = DEVICE_NATIVE_ENDIAN,
 };
 
-static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic)
+static uint64_t m5208_rcm_read(void *opaque, hwaddr addr,
+   unsigned size)
+{
+return 0;
+}
+
+static void m5208_rcm_write(void *opaque, hwaddr addr,
+uint64_t value, unsigned size)
+{
+M68kCPU *cpu = opaque;
+CPUState *cs = CPU(cpu);
+switch (addr) {
+case 0x0: /* RCR */
+if (value & RCR_SOFTRST) {
+cpu_reset(cs);
+cpu->env.aregs[7] = ldl_phys(cs->as, 0);
+cpu->env.pc = ldl_phys(cs->as, 4);
+}
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "%s: Bad offset 0x%" HWADDR_PRIX "\n",
+  __func__, addr);
+break;
+}
+}
+
+static const MemoryRegionOps m5208_rcm_ops = {
+.read = m5208_rcm_read,
+.write = m5208_rcm_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void mcf5208_sys_init(MemoryRegion *address_space, qemu_irq *pic,
+ M68kCPU *cpu)
 {
 MemoryRegion *iomem = g_new(MemoryRegion, 1);
+MemoryRegion *iomem_rcm = g_new(MemoryRegion, 1);
 m5208_timer_state *s;
 int i;
 
+/* RCM */
+memory_region_init_io(iomem_rcm, NULL, &m5208_rcm_ops, cpu,
+  "m5208-rcm", 0x0080);
+memory_region_add_subregion(address_space, 0xfc0a, iomem_rcm);
 /* SDRAMC.  */
 memory_region_init_io(iomem, NULL, &m5208_sys_ops, NULL, "m5208-sys", 
0x4000);
 memory_region_add_subregion(address_space, 0xfc0a8000, iomem);
@@ -265,7 +305,7 @@ static void mcf5208evb_init(MachineState *machine)
 mcf_uart_create_mmap(0xfc064000, pic[27], serial_hd(1));
 mcf_uart_create_mmap(0xfc068000, pic[28], serial_hd(2));
 
-mcf5208_sys_init(address_space_mem, pic);
+mcf5208_sys_init(address_space_mem, pic, cpu);
 
 mcf_fec_init(address_space_mem, 0xfc03, pic + 36);
 
-- 
2.41.0




[PULL 34/43] tests/unit/test-smp-parse: Use CPU number macros in invalid topology case

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Use MAX_CPUS/MIN_CPUS macros in invalid topology case. This gives us the
flexibility to change the maximum and minimum CPU limits.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-6-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 22 ++
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 1874bea086..84e3422774 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -323,15 +323,21 @@ static const struct SMPTestData data_generic_invalid[] = {
 "sockets (2) * cores (4) * threads (2) "
 "== maxcpus (16) < smp_cpus (18)",
 }, {
-/* config: -smp 1
- * should tweak the supported min CPUs to 2 for testing */
-.config = SMP_CONFIG_GENERIC(T, 1, F, 0, F, 0, F, 0, F, 0),
+/*
+ * config: -smp 1
+ * The test machine should tweak the supported min CPUs to
+ * 2 (MIN_CPUS + 1) for testing.
+ */
+.config = SMP_CONFIG_GENERIC(T, MIN_CPUS, F, 0, F, 0, F, 0, F, 0),
 .expect_error = "Invalid SMP CPUs 1. The min CPUs supported "
 "by machine '" SMP_MACHINE_NAME "' is 2",
 }, {
-/* config: -smp 512
- * should tweak the supported max CPUs to 511 for testing */
-.config = SMP_CONFIG_GENERIC(T, 512, F, 0, F, 0, F, 0, F, 0),
+/*
+ * config: -smp 512
+ * The test machine should tweak the supported max CPUs to
+ * 511 (MAX_CPUS - 1) for testing.
+ */
+.config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
 .expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
 "by machine '" SMP_MACHINE_NAME "' is 511",
 },
@@ -575,8 +581,8 @@ static void machine_generic_invalid_class_init(ObjectClass 
*oc, void *data)
 MachineClass *mc = MACHINE_CLASS(oc);
 
 /* Force invalid min CPUs and max CPUs */
-mc->min_cpus = 2;
-mc->max_cpus = 511;
+mc->min_cpus = MIN_CPUS + 1;
+mc->max_cpus = MAX_CPUS - 1;
 }
 
 static void machine_with_dies_class_init(ObjectClass *oc, void *data)
-- 
2.41.0




[PULL 17/43] hw/remote/remote-obj: hw/misc/ivshmem: Fix missing ERRP_GUARD() for error_prepend()

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

As the comment in qapi/error, passing @errp to error_prepend() requires
ERRP_GUARD():

* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
...
* - It should not be passed to error_prepend(), error_vprepend() or
*   error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.

ERRP_GUARD() could avoid the case when @errp is the pointer of
error_fatal, the user can't see this additional information, because
exit() happens in error_setg earlier than information is added [1].

The remote_object_set_fd() passes @errp to error_prepend(), and as a
PropertyInfo.set method, its @errp is so widely sourced that it is
necessary to protect it with ERRP_GUARD().

To avoid the issue like [1] said, add missing ERRP_GUARD() at the
beginning of this function.

[1]: Issue description in the commit message of commit ae7c80a7bd73
 ("error: New macro ERRP_GUARD()").

Cc: Elena Ufimtseva 
Cc: Jagannathan Raman 
Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Message-ID: <20240229143914.1977550-4-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/remote/remote-obj.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/remote/remote-obj.c b/hw/remote/remote-obj.c
index 65b6f7cc86..dc27cc8da1 100644
--- a/hw/remote/remote-obj.c
+++ b/hw/remote/remote-obj.c
@@ -49,6 +49,7 @@ struct RemoteObject {
 
 static void remote_object_set_fd(Object *obj, const char *str, Error **errp)
 {
+ERRP_GUARD();
 RemoteObject *o = REMOTE_OBJECT(obj);
 int fd = -1;
 
-- 
2.41.0




[PULL 21/43] hw/i386/pc: Remove 'host_type' argument from pc_init1()

2024-03-09 Thread Philippe Mathieu-Daudé
All callers use host_type=TYPE_I440FX_PCI_HOST_BRIDGE.
Directly use this definition within pc_init1().

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20240301185936.95175-4-phi...@linaro.org>
---
 hw/i386/pc_piix.c | 14 +-
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ed777e3d61..e14dce951d 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -101,8 +101,7 @@ static void piix_intx_routing_notifier_xen(PCIDevice *dev)
 }
 
 /* PC hardware initialisation */
-static void pc_init1(MachineState *machine,
- const char *host_type, const char *pci_type)
+static void pc_init1(MachineState *machine, const char *pci_type)
 {
 PCMachineState *pcms = PC_MACHINE(machine);
 PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
@@ -194,7 +193,7 @@ static void pc_init1(MachineState *machine,
 memory_region_init(pci_memory, NULL, "pci", UINT64_MAX);
 rom_memory = pci_memory;
 
-phb = OBJECT(qdev_new(host_type));
+phb = OBJECT(qdev_new(TYPE_I440FX_PCI_HOST_BRIDGE));
 object_property_add_child(OBJECT(machine), "i440fx", phb);
 object_property_set_link(phb, PCI_HOST_PROP_RAM_MEM,
  OBJECT(ram_memory), &error_fatal);
@@ -451,7 +450,7 @@ static void pc_compat_2_0_fn(MachineState *machine)
 #ifdef CONFIG_ISAPC
 static void pc_init_isa(MachineState *machine)
 {
-pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, TYPE_I440FX_PCI_DEVICE);
+pc_init1(machine, TYPE_I440FX_PCI_DEVICE);
 }
 #endif
 
@@ -461,9 +460,7 @@ static void pc_xen_hvm_init_pci(MachineState *machine)
 const char *pci_type = xen_igd_gfx_pt_enabled() ?
 TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE : 
TYPE_I440FX_PCI_DEVICE;
 
-pc_init1(machine,
- TYPE_I440FX_PCI_HOST_BRIDGE,
- pci_type);
+pc_init1(machine, pci_type);
 }
 
 static void pc_xen_hvm_init(MachineState *machine)
@@ -488,8 +485,7 @@ static void pc_xen_hvm_init(MachineState *machine)
 if (compat) { \
 compat(machine); \
 } \
-pc_init1(machine, TYPE_I440FX_PCI_HOST_BRIDGE, \
- TYPE_I440FX_PCI_DEVICE); \
+pc_init1(machine, TYPE_I440FX_PCI_DEVICE); \
 } \
 DEFINE_PC_MACHINE(suffix, name, pc_init_##suffix, optionfn)
 
-- 
2.41.0




[PULL 37/43] tests/unit/test-smp-parse: Test "books" parameter in -smp

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Although book was introduced to -smp along with drawer by s390 machine,
as a general topology level in QEMU that may be reused by other arches
in the future, it is desirable to cover this parameter's parsing in a
separate case.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-9-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 105 
 1 file changed, 105 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index f656bbb6da..090238ab23 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -75,6 +75,22 @@
 .has_maxcpus  = hf, .maxcpus  = f,\
 }
 
+/*
+ * Currently a 5-level topology hierarchy is supported on s390 ccw machines
+ *  -drawers/books/sockets/cores/threads
+ */
+#define SMP_CONFIG_WITH_BOOKS_DRAWERS(ha, a, hb, b, hc, c, hd, \
+  d, he, e, hf, f, hg, g)  \
+{  \
+.has_cpus = ha, .cpus = a, \
+.has_drawers  = hb, .drawers  = b, \
+.has_books= hc, .books= c, \
+.has_sockets  = hd, .sockets  = d, \
+.has_cores= he, .cores= e, \
+.has_threads  = hf, .threads  = f, \
+.has_maxcpus  = hg, .maxcpus  = g, \
+}
+
 /**
  * @config - the given SMP configuration
  * @expect_prefer_sockets - the expected parsing result for the
@@ -308,6 +324,11 @@ static const struct SMPTestData data_generic_invalid[] = {
 /* config: -smp 2,clusters=2 */
 .config = SMP_CONFIG_WITH_CLUSTERS(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
 .expect_error = "clusters not supported by this machine's CPU 
topology",
+}, {
+/* config: -smp 2,books=2 */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 2, F, 0, T, 2, F,
+0, F, 0, F, 0, F, 0),
+.expect_error = "books not supported by this machine's CPU topology",
 }, {
 /* config: -smp 8,sockets=2,cores=4,threads=2,maxcpus=8 */
 .config = SMP_CONFIG_GENERIC(T, 8, T, 2, T, 4, T, 2, T, 8),
@@ -379,6 +400,26 @@ static const struct SMPTestData 
data_with_clusters_invalid[] = {
 },
 };
 
+static const struct SMPTestData data_with_books_invalid[] = {
+{
+/* config: -smp 16,books=2,sockets=2,cores=4,threads=2,maxcpus=16 */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, F, 1, T, 2, T,
+2, T, 4, T, 2, T, 16),
+.expect_error = "Invalid CPU topology: "
+"product of the hierarchy must match maxcpus: "
+"books (2) * sockets (2) * cores (4) * threads (2) "
+"!= maxcpus (16)",
+}, {
+/* config: -smp 34,books=2,sockets=2,cores=4,threads=2,maxcpus=32 */
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, F, 1, T, 2, T,
+2, T, 4, T, 2, T, 32),
+.expect_error = "Invalid CPU topology: "
+"maxcpus must be equal to or greater than smp: "
+"books (2) * sockets (2) * cores (4) * threads (2) "
+"== maxcpus (32) < smp_cpus (34)",
+},
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
 return g_strdup_printf(
@@ -618,6 +659,13 @@ static void machine_with_clusters_class_init(ObjectClass 
*oc, void *data)
 mc->smp_props.clusters_supported = true;
 }
 
+static void machine_with_books_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->smp_props.books_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
 const char *machine_type = opaque;
@@ -756,6 +804,56 @@ static void test_with_clusters(const void *opaque)
 object_unref(obj);
 }
 
+static void test_with_books(const void *opaque)
+{
+const char *machine_type = opaque;
+Object *obj = object_new(machine_type);
+MachineState *ms = MACHINE(obj);
+MachineClass *mc = MACHINE_GET_CLASS(obj);
+SMPTestData data = {};
+unsigned int num_books = 2;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+data = data_generic_valid[i];
+unsupported_params_init(mc, &data);
+
+/* when books parameter is omitted, it will be set as 1 */
+data.expect_prefer_sockets.books = 1;
+data.expect_prefer_cores.books = 1;
+
+smp_parse_test(ms, &data, true);
+
+/* when books parameter is specified */
+data.config.has_books = true;
+data.config.books = num_books;
+if (data.config.has_cpus) {
+ 

[PULL 40/43] tests/unit/test-smp-parse: Test the full 7-levels topology hierarchy

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

Currently, -smp supports up to 7-levels topology hierarchy:
  -drawers/books/sockets/dies/clusters/cores/threads.

Though no machine supports all these 7 levels yet, these 7 levels have
the strict containment relationship and together form the generic CPU
topology representation of QEMU.

Also, note that the maxcpus is calculated by multiplying all 7 levels:

  maxcpus = drawers * books * sockets * dies * clusters *
cores * threads.

To cover this code path, it is necessary to test the full topology case
(with all 7 levels). This also helps to avoid introducing new issues by
further expanding the CPU topology in the future.

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Acked-by: Thomas Huth 
Message-ID: <20240308160148.3130837-12-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 143 
 1 file changed, 143 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 0cf6115198..7558169171 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -91,6 +91,24 @@
 .has_maxcpus  = hg, .maxcpus  = g, \
 }
 
+/*
+ * Currently QEMU supports up to a 7-level topology hierarchy, which is the
+ * QEMU's unified abstract representation of CPU topology.
+ *  -drawers/books/sockets/dies/clusters/cores/threads
+ */
+#define SMP_CONFIG_WITH_FULL_TOPO(a, b, c, d, e, f, g, h, i)\
+{   \
+.has_cpus = true, .cpus = a,\
+.has_drawers  = true, .drawers  = b,\
+.has_books= true, .books= c,\
+.has_sockets  = true, .sockets  = d,\
+.has_dies = true, .dies = e,\
+.has_clusters = true, .clusters = f,\
+.has_cores= true, .cores= g,\
+.has_threads  = true, .threads  = h,\
+.has_maxcpus  = true, .maxcpus  = i,\
+}
+
 /**
  * @config - the given SMP configuration
  * @expect_prefer_sockets - the expected parsing result for the
@@ -472,6 +490,40 @@ static const struct SMPTestData 
data_with_drawers_books_invalid[] = {
 },
 };
 
+static const struct SMPTestData data_full_topo_invalid[] = {
+{
+/*
+ * config: -smp 200,drawers=3,books=5,sockets=2,dies=4,\
+ *  clusters=2,cores=7,threads=2,maxcpus=200
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(200, 3, 5, 2, 4, 2, 7, 2, 200),
+.expect_error = "Invalid CPU topology: "
+"product of the hierarchy must match maxcpus: "
+"drawers (3) * books (5) * sockets (2) * dies (4) * "
+"clusters (2) * cores (7) * threads (2) "
+"!= maxcpus (200)",
+}, {
+/*
+ * config: -smp 3361,drawers=3,books=5,sockets=2,dies=4,\
+ *  clusters=2,cores=7,threads=2,maxcpus=3360
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 2, 3360),
+.expect_error = "Invalid CPU topology: "
+"maxcpus must be equal to or greater than smp: "
+"drawers (3) * books (5) * sockets (2) * dies (4) * "
+"clusters (2) * cores (7) * threads (2) "
+"== maxcpus (3360) < smp_cpus (3361)",
+}, {
+/*
+ * config: -smp 1,drawers=3,books=5,sockets=2,dies=4,\
+ *  clusters=2,cores=7,threads=3,maxcpus=5040
+ */
+.config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 3, 5040),
+.expect_error = "Invalid SMP CPUs 5040. The max CPUs supported "
+"by machine '" SMP_MACHINE_NAME "' is 4096",
+},
+};
+
 static char *smp_config_to_string(const SMPConfiguration *config)
 {
 return g_strdup_printf(
@@ -733,6 +785,16 @@ static void 
machine_with_drawers_books_class_init(ObjectClass *oc, void *data)
 mc->smp_props.books_supported = true;
 }
 
+static void machine_full_topo_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->smp_props.drawers_supported = true;
+mc->smp_props.books_supported = true;
+mc->smp_props.dies_supported = true;
+mc->smp_props.clusters_supported = true;
+}
+
 static void test_generic_valid(const void *opaque)
 {
 const char *machine_type = opaque;
@@ -1032,6 +1094,80 @@ static void test_with_drawers_books(const void *opaque)
 object_unref(obj);
 }
 
+static void test_full_topo(const void *opaque)
+{
+const char *machine_type = opaque;
+Object *obj = object_new(machine_type);
+MachineState *ms = MACHINE(obj);
+MachineClass *mc = MACHINE_GET_CLASS(obj);
+SMPTestData data = {};

[PULL 28/43] docs/interop/firmware.json: Align examples

2024-03-09 Thread Philippe Mathieu-Daudé
From: Thomas Weißschuh 

Adjust indentation for commit d23055b8db8 (qapi: Require descriptions
and tagged sections to be indented).

Signed-off-by: Thomas Weißschuh 
Reviewed-by: Markus Armbruster 
Message-ID: <20240307-qapi-firmware-json-v2-1-3b29eabb9...@linutronix.de>
[PMD: Reword description using Markus suggestion]
Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/interop/firmware.json | 374 ++---
 1 file changed, 187 insertions(+), 187 deletions(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index cc8f869186..a024f1b9bf 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -435,203 +435,203 @@
 #
 # Examples:
 #
-# {
-# "description": "SeaBIOS",
-# "interface-types": [
-# "bios"
-# ],
-# "mapping": {
-# "device": "memory",
-# "filename": "/usr/share/seabios/bios-256k.bin"
-# },
-# "targets": [
-# {
-# "architecture": "i386",
-# "machines": [
-# "pc-i440fx-*",
-# "pc-q35-*"
-# ]
+# {
+# "description": "SeaBIOS",
+# "interface-types": [
+# "bios"
+# ],
+# "mapping": {
+# "device": "memory",
+# "filename": "/usr/share/seabios/bios-256k.bin"
 # },
-# {
-# "architecture": "x86_64",
-# "machines": [
-# "pc-i440fx-*",
-# "pc-q35-*"
-# ]
-# }
-# ],
-# "features": [
-# "acpi-s3",
-# "acpi-s4"
-# ],
-# "tags": [
-# "CONFIG_BOOTSPLASH=n",
-# "CONFIG_ROM_SIZE=256",
-# "CONFIG_USE_SMM=n"
-# ]
-# }
+# "targets": [
+# {
+# "architecture": "i386",
+# "machines": [
+# "pc-i440fx-*",
+# "pc-q35-*"
+# ]
+# },
+# {
+# "architecture": "x86_64",
+# "machines": [
+# "pc-i440fx-*",
+# "pc-q35-*"
+# ]
+# }
+# ],
+# "features": [
+# "acpi-s3",
+# "acpi-s4"
+# ],
+# "tags": [
+# "CONFIG_BOOTSPLASH=n",
+# "CONFIG_ROM_SIZE=256",
+# "CONFIG_USE_SMM=n"
+# ]
+# }
 #
-# {
-# "description": "OVMF with SB+SMM, empty varstore",
-# "interface-types": [
-# "uefi"
-# ],
-# "mapping": {
-# "device": "flash",
-# "executable": {
-# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
-# "format": "raw"
+# {
+# "description": "OVMF with SB+SMM, empty varstore",
+# "interface-types": [
+# "uefi"
+# ],
+# "mapping": {
+# "device": "flash",
+# "executable": {
+# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
+# "format": "raw"
+# },
+# "nvram-template": {
+# "filename": "/usr/share/OVMF/OVMF_VARS.fd",
+# "format": "raw"
+# }
 # },
-# "nvram-template": {
-# "filename": "/usr/share/OVMF/OVMF_VARS.fd",
-# "format": "raw"
-# }
-# },
-# "targets": [
-# {
-# "architecture": "x86_64",
-# "machines": [
-# "pc-q35-*"
-# ]
-# }
-# ],
-# "features": [
-# "acpi-s3",
-# "amd-sev",
-# "requires-smm",
-# "secure-boot",
-# "verbose-dynamic"
-# ],
-# "tags": [
-# "-a IA32",
-# "-a X64",
-# "-p OvmfPkg/OvmfPkgIa32X64.dsc",
-# "-t GCC48",
-# "-b DEBUG",
-# "-D SMM_REQUIRE",
-# "-D SECURE_BOOT_ENABLE",
-# "-D FD_SIZE_4MB"
-# ]
-# }
+# "targets": [
+# {
+# "architecture": "x86_64",
+# "machines": [
+# "pc-q35-*"
+# ]
+# }
+# ],
+# "features": [
+# "acpi-s3",
+# "amd-sev",
+# "requires-smm",
+# "secure-boot",
+# "verbose-dynamic"
+# ],
+# "tags": [
+# "-a IA32",
+# "-a X64",
+# "-p OvmfPkg/OvmfPkgIa32X64.dsc",
+# "-t GCC48",
+# "-b DEBUG",
+# "-D SMM_REQUIRE",
+# "-D SECURE_BOOT_ENABLE",
+# "-D FD_SIZE_4MB"
+# ]
+# }
 #
-# {
-# "description": "OVMF with SB+SMM, SB enabled, MS certs enrolled",
-# "interface-types": [
-# "uefi"
-# ],
-# "mapping": {
-# "device": "flash",
-# "executable": {
-# "filename": "/usr/share/OVMF/OVMF_CODE.secboot.fd",
-# "format": "raw"
+# {
+# "de

[PULL 15/43] hw/char/xen_console: Fix missing ERRP_GUARD() for error_prepend()

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

As the comment in qapi/error, passing @errp to error_prepend() requires
ERRP_GUARD():

* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
...
* - It should not be passed to error_prepend(), error_vprepend() or
*   error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.

ERRP_GUARD() could avoid the case when @errp is the pointer of
error_fatal, the user can't see this additional information, because
exit() happens in error_setg earlier than information is added [1].

The xen_console_connect() passes @errp to error_prepend() without
ERRP_GUARD().

There're 2 places will call xen_console_connect():
 - xen_console_realize(): the @errp is from DeviceClass.realize()'s
  parameter.
 - xen_console_frontend_changed(): the @errp points its caller's
   @local_err.

To avoid the issue like [1] said, add missing ERRP_GUARD() at the
beginning of xen_console_connect().

[1]: Issue description in the commit message of commit ae7c80a7bd73
 ("error: New macro ERRP_GUARD()").

Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: "Marc-André Lureau" 
Cc: Paolo Bonzini 
Signed-off-by: Zhao Liu 
Acked-by: Anthony PERARD 
Message-ID: <20240228163723.1775791-15-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/char/xen_console.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/char/xen_console.c b/hw/char/xen_console.c
index 5cbee2f184..683c92aca1 100644
--- a/hw/char/xen_console.c
+++ b/hw/char/xen_console.c
@@ -206,6 +206,7 @@ static bool con_event(void *_xendev)
 
 static bool xen_console_connect(XenDevice *xendev, Error **errp)
 {
+ERRP_GUARD();
 XenConsole *con = XEN_CONSOLE_DEVICE(xendev);
 unsigned int port, limit;
 
-- 
2.41.0




[PULL 10/43] hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'

2024-03-09 Thread Philippe Mathieu-Daudé
"hw/xen/xen_pt.h" requires "hw/xen/xen_native.h" which is target
specific. It also declares IGD methods, which are not target
specific.

Target-agnostic code can use IGD methods. To allow that, extract
these methos into a new "hw/xen/xen_igd.h" header.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-18-phi...@linaro.org>
---
 hw/xen/xen_pt.h | 14 --
 include/hw/xen/xen_igd.h| 33 +
 accel/xen/xen-all.c |  1 +
 hw/i386/pc_piix.c   |  1 +
 hw/xen/xen_pt.c |  3 ++-
 hw/xen/xen_pt_config_init.c |  3 ++-
 hw/xen/xen_pt_graphics.c|  3 ++-
 hw/xen/xen_pt_stub.c|  2 +-
 8 files changed, 42 insertions(+), 18 deletions(-)
 create mode 100644 include/hw/xen/xen_igd.h

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index d3180bb134..095a0f0365 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -15,9 +15,6 @@
 #include "xen-host-pci-device.h"
 #include "qom/object.h"
 
-bool xen_igd_gfx_pt_enabled(void);
-void xen_igd_gfx_pt_set(bool value, Error **errp);
-
 void xen_pt_log(const PCIDevice *d, const char *f, ...) G_GNUC_PRINTF(2, 3);
 
 #define XEN_PT_ERR(d, _f, _a...) xen_pt_log(d, "%s: Error: "_f, __func__, ##_a)
@@ -62,12 +59,6 @@ typedef struct XenPTDeviceClass {
 XenPTQdevRealize pci_qdev_realize;
 } XenPTDeviceClass;
 
-uint32_t igd_read_opregion(XenPCIPassthroughState *s);
-void xen_igd_reserve_slot(PCIBus *pci_bus);
-void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
-void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
-   XenHostPCIDevice *dev);
-
 /* function type for config reg */
 typedef int (*xen_pt_conf_reg_init)
 (XenPCIPassthroughState *, XenPTRegInfo *, uint32_t real_offset,
@@ -353,11 +344,6 @@ static inline bool 
xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
 void *pci_assign_dev_load_option_rom(PCIDevice *dev, int *size,
  unsigned int domain, unsigned int bus,
  unsigned int slot, unsigned int function);
-static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
-{
-return (xen_igd_gfx_pt_enabled()
-&& ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
-}
 int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
 int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
 void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
diff --git a/include/hw/xen/xen_igd.h b/include/hw/xen/xen_igd.h
new file mode 100644
index 00..7ffca06c10
--- /dev/null
+++ b/include/hw/xen/xen_igd.h
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2007, Neocleus Corporation.
+ * Copyright (c) 2007, Intel Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Alex Novik 
+ * Allen Kay 
+ * Guy Zana 
+ */
+#ifndef XEN_IGD_H
+#define XEN_IGD_H
+
+#include "hw/xen/xen-host-pci-device.h"
+
+typedef struct XenPCIPassthroughState XenPCIPassthroughState;
+
+bool xen_igd_gfx_pt_enabled(void);
+void xen_igd_gfx_pt_set(bool value, Error **errp);
+
+uint32_t igd_read_opregion(XenPCIPassthroughState *s);
+void xen_igd_reserve_slot(PCIBus *pci_bus);
+void igd_write_opregion(XenPCIPassthroughState *s, uint32_t val);
+void xen_igd_passthrough_isa_bridge_create(XenPCIPassthroughState *s,
+   XenHostPCIDevice *dev);
+
+static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
+{
+return (xen_igd_gfx_pt_enabled()
+&& ((dev->class_code >> 0x8) == PCI_CLASS_DISPLAY_VGA));
+}
+
+#endif
diff --git a/accel/xen/xen-all.c b/accel/xen/xen-all.c
index 5ff0cb8bd9..0bdefce537 100644
--- a/accel/xen/xen-all.c
+++ b/accel/xen/xen-all.c
@@ -15,6 +15,7 @@
 #include "hw/xen/xen_native.h"
 #include "hw/xen/xen-legacy-backend.h"
 #include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "chardev/char.h"
 #include "qemu/accel.h"
 #include "sysemu/cpus.h"
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index ce6aad758d..e123458bbc 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -55,6 +55,7 @@
 #ifdef CONFIG_XEN
 #include 
 #include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #endif
 #include "hw/xen/xen-x86.h"
 #include "hw/xen/xen.h"
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 36e6f93c37..a8edabdabc 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -59,7 +59,8 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
-#include "xen_pt.h"
+#include "hw/xen/xen_pt.h"
+#include "hw/xen/xen_igd.h"
 #include "hw/xen/xen.h"
 #include "hw/xen/xen-legacy-backend.h"
 #include "qemu/range.h"
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 2b8680b112..ba4cd78238 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -15,7 +15,8 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "qemu/timer.h"
-#in

[PULL 11/43] hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS

2024-03-09 Thread Philippe Mathieu-Daudé
xen-hvm.c calls xc_set_hvm_param() from ,
so better compile it with Xen CPPFLAGS.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-19-phi...@linaro.org>
---
 hw/i386/xen/meson.build | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/i386/xen/meson.build b/hw/i386/xen/meson.build
index 3dc4c4f106..3f0df8bc07 100644
--- a/hw/i386/xen/meson.build
+++ b/hw/i386/xen/meson.build
@@ -1,8 +1,10 @@
 i386_ss.add(when: 'CONFIG_XEN', if_true: files(
-  'xen-hvm.c',
   'xen_apic.c',
   'xen_pvdevice.c',
 ))
+i386_ss.add(when: ['CONFIG_XEN', xen], if_true: files(
+  'xen-hvm.c',
+))
 
 i386_ss.add(when: 'CONFIG_XEN_BUS', if_true: files(
   'xen_platform.c',
-- 
2.41.0




[PULL 29/43] docs/interop/firmware.json: Fix doc for FirmwareFlashMode

2024-03-09 Thread Philippe Mathieu-Daudé
From: Thomas Weißschuh 

The doc title did not match the actual definition.

Fixes: 2720ceda05 ("docs: expand firmware descriptor to allow flash without 
NVRAM")
Signed-off-by: Thomas Weißschuh 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240307-qapi-firmware-json-v2-2-3b29eabb9...@linutronix.de>
Signed-off-by: Philippe Mathieu-Daudé 
---
 docs/interop/firmware.json | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/interop/firmware.json b/docs/interop/firmware.json
index a024f1b9bf..54a1fc6c10 100644
--- a/docs/interop/firmware.json
+++ b/docs/interop/firmware.json
@@ -223,7 +223,7 @@
 
 
 ##
-# @FirmwareFlashType:
+# @FirmwareFlashMode:
 #
 # Describes how the firmware build handles code versus variable
 # persistence.
-- 
2.41.0




[PULL 23/43] hw/intc/apic: fix memory leak

2024-03-09 Thread Philippe Mathieu-Daudé
From: Paolo Bonzini 

deliver_bitmask is allocated on the heap in apic_deliver(), but there
are many paths in the function that return before the corresponding
g_free() is reached.  Fix this by switching to g_autofree and, while at
it, also switch to g_new.  Do the same in apic_deliver_irq() as well
for consistency.

Fixes: b5ee0468e9d ("apic: add support for x2APIC mode", 2024-02-14)
Signed-off-by: Paolo Bonzini 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Bui Quang Minh 
Reviewed-by: Alex Bennée 
Message-ID: <20240304224133.267640-1-pbonz...@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/intc/apic.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 1d887d66b8..4186c57b34 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -291,14 +291,13 @@ static void apic_deliver_irq(uint32_t dest, uint8_t 
dest_mode,
  uint8_t delivery_mode, uint8_t vector_num,
  uint8_t trigger_mode)
 {
-uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t));
+g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
 
 trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
trigger_mode);
 
 apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
-g_free(deliver_bitmask);
 }
 
 bool is_x2apic_mode(DeviceState *dev)
@@ -662,7 +661,7 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, 
uint8_t dest_mode,
 APICCommonState *s = APIC(dev);
 APICCommonState *apic_iter;
 uint32_t deliver_bitmask_size = max_apic_words * sizeof(uint32_t);
-uint32_t *deliver_bitmask = g_malloc(deliver_bitmask_size);
+g_autofree uint32_t *deliver_bitmask = g_new(uint32_t, max_apic_words);
 uint32_t current_apic_id;
 
 if (is_x2apic_mode(dev)) {
@@ -708,7 +707,6 @@ static void apic_deliver(DeviceState *dev, uint32_t dest, 
uint8_t dest_mode,
 }
 
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
-g_free(deliver_bitmask);
 }
 
 static bool apic_check_pic(APICCommonState *s)
-- 
2.41.0




[PULL 35/43] tests/unit/test-smp-parse: Bump max_cpus to 4096

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

The q35 machine is trying to support up to 4096 vCPUs [1], so it's
necessary to bump max_cpus in test-smp-parse to 4096 to cover the
topological needs of future machines.

[1]: 
https://lore.kernel.org/qemu-devel/20240228143351.3967-1-anisi...@redhat.com/

Signed-off-by: Zhao Liu 
Tested-by: Xiaoling Song 
Reviewed-by: Thomas Huth 
Message-ID: <20240308160148.3130837-7-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 tests/unit/test-smp-parse.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 84e3422774..2eb9533bc5 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -20,8 +20,8 @@
 #define T true
 #define F false
 
-#define MIN_CPUS 1   /* set the min CPUs supported by the machine as 1 */
-#define MAX_CPUS 512 /* set the max CPUs supported by the machine as 512 */
+#define MIN_CPUS 1/* set the min CPUs supported by the machine as 1 */
+#define MAX_CPUS 4096 /* set the max CPUs supported by the machine as 4096 */
 
 #define SMP_MACHINE_NAME "TEST-SMP"
 
@@ -333,13 +333,13 @@ static const struct SMPTestData data_generic_invalid[] = {
 "by machine '" SMP_MACHINE_NAME "' is 2",
 }, {
 /*
- * config: -smp 512
+ * config: -smp 4096
  * The test machine should tweak the supported max CPUs to
- * 511 (MAX_CPUS - 1) for testing.
+ * 4095 (MAX_CPUS - 1) for testing.
  */
-.config = SMP_CONFIG_GENERIC(T, MAX_CPUS, F, 0, F, 0, F, 0, F, 0),
-.expect_error = "Invalid SMP CPUs 512. The max CPUs supported "
-"by machine '" SMP_MACHINE_NAME "' is 511",
+.config = SMP_CONFIG_GENERIC(T, 4096, F, 0, F, 0, F, 0, F, 0),
+.expect_error = "Invalid SMP CPUs 4096. The max CPUs supported "
+"by machine '" SMP_MACHINE_NAME "' is 4095",
 },
 };
 
-- 
2.41.0




[PULL 26/43] mac_newworld: change timebase frequency from 100MHz to 25MHz for mac99 machine

2024-03-09 Thread Philippe Mathieu-Daudé
From: Mark Cave-Ayland 

MacOS X uses multiple techniques for calibrating timers depending upon the 
detected
hardware. One of these calibration routines compares the change in the timebase
against the KeyLargo timer and uses this to recalculate the clock frequency,
timebase frequency and bus frequency if the calibration exceeds certain limits.
This recalibration occurs despite the correct values being passed via the device
tree, and is likely due to buggy firmware on some hardware.

The timebase frequency of 100MHz was set way back in 2005 by commit fa296b0fb4
("PIC fix - changed back TB frequency to 100 MHz") and with this value on a
mac99,via=pmu machine the OSX 10.2 timer calibration incorrectly calculates the
bus frequency as 400MHz instead of 100MHz. The most noticeable side-effect is
the UI appears sluggish and not very responsive for normal use.

Change the timebase frequency from 100MHz to 25MHz which matches that of a real
G4 AGP machine (the closest match to QEMU's mac99 machine) and allows OSX 10.2
to correctly detect all of the clock frequency, timebase frequency and bus
frequency.

Tested on various MacOS images from OS 9.2 through to OSX 10.4, along with Linux
and NetBSD and I was unable to find any regressions from this change.

Signed-off-by: Mark Cave-Ayland 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240304073548.2098806-1-mark.cave-ayl...@ilande.co.uk>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/ppc/mac_newworld.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index 3e796d2f6d..ff9e490c4e 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -77,7 +77,7 @@
 
 #define MAX_IDE_BUS 2
 #define CFG_ADDR 0xf510
-#define TBFREQ (100UL * 1000UL * 1000UL)
+#define TBFREQ (25UL * 1000UL * 1000UL)
 #define CLOCKFREQ (900UL * 1000UL * 1000UL)
 #define BUSFREQ (100UL * 1000UL * 1000UL)
 
-- 
2.41.0




[PULL 16/43] hw/net/xen_nic: Fix missing ERRP_GUARD() for error_prepend()

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

As the comment in qapi/error, passing @errp to error_prepend() requires
ERRP_GUARD():

* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
...
* - It should not be passed to error_prepend(), error_vprepend() or
*   error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.

ERRP_GUARD() could avoid the case when @errp is the pointer of
error_fatal, the user can't see this additional information, because
exit() happens in error_setg earlier than information is added [1].

The xen_netdev_connect() passes @errp to error_prepend(), and its @errp
parameter is from xen_device_frontend_changed().

Though its @errp points to @local_err of xen_device_frontend_changed(),
to follow the requirement of @errp, add missing ERRP_GUARD() at the
beginning of this function.

[1]: Issue description in the commit message of commit ae7c80a7bd73
 ("error: New macro ERRP_GUARD()").

Cc: Stefano Stabellini 
Cc: Anthony Perard 
Cc: Paul Durrant 
Cc: Jason Wang 
Signed-off-by: Zhao Liu 
Acked-by: Anthony PERARD 
Reviewed-by: Thomas Huth 
Message-ID: <20240229143914.1977550-3-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/net/xen_nic.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/net/xen_nic.c b/hw/net/xen_nic.c
index 453fdb9819..89487b49ba 100644
--- a/hw/net/xen_nic.c
+++ b/hw/net/xen_nic.c
@@ -351,6 +351,7 @@ static bool net_event(void *_xendev)
 
 static bool xen_netdev_connect(XenDevice *xendev, Error **errp)
 {
+ERRP_GUARD();
 XenNetDev *netdev = XEN_NET_DEVICE(xendev);
 unsigned int port, rx_copy;
 
-- 
2.41.0




[PULL 18/43] target/i386/sev: Fix missing ERRP_GUARD() for error_prepend()

2024-03-09 Thread Philippe Mathieu-Daudé
From: Zhao Liu 

As the comment in qapi/error, passing @errp to error_prepend() requires
ERRP_GUARD():

* = Why, when and how to use ERRP_GUARD() =
*
* Without ERRP_GUARD(), use of the @errp parameter is restricted:
...
* - It should not be passed to error_prepend(), error_vprepend() or
*   error_append_hint(), because that doesn't work with &error_fatal.
* ERRP_GUARD() lifts these restrictions.
*
* To use ERRP_GUARD(), add it right at the beginning of the function.
* @errp can then be used without worrying about the argument being
* NULL or &error_fatal.

ERRP_GUARD() could avoid the case when @errp is the pointer of
error_fatal, the user can't see this additional information, because
exit() happens in error_setg earlier than information is added [1].

The sev_inject_launch_secret() passes @errp to error_prepend(), and as
an APIs defined in target/i386/sev.h, it is necessary to protect its
@errp with ERRP_GUARD().

To avoid the issue like [1] said, add missing ERRP_GUARD() at the
beginning of this function.

[1]: Issue description in the commit message of commit ae7c80a7bd73
 ("error: New macro ERRP_GUARD()").

Cc: Paolo Bonzini 
Cc: Marcelo Tosatti 
Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Message-ID: <20240229143914.1977550-17-zhao1@linux.intel.com>
Signed-off-by: Philippe Mathieu-Daudé 
---
 target/i386/sev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 173de91afe..72930ff0dc 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1044,6 +1044,7 @@ sev_encrypt_flash(uint8_t *ptr, uint64_t len, Error 
**errp)
 int sev_inject_launch_secret(const char *packet_hdr, const char *secret,
  uint64_t gpa, Error **errp)
 {
+ERRP_GUARD();
 struct kvm_sev_launch_secret input;
 g_autofree guchar *data = NULL, *hdr = NULL;
 int error, ret = 1;
-- 
2.41.0




[PULL 03/43] sysemu/xen-mapcache: Check Xen availability with CONFIG_XEN_IS_POSSIBLE

2024-03-09 Thread Philippe Mathieu-Daudé
"sysemu/xen.h" defines CONFIG_XEN_IS_POSSIBLE as a target-agnostic
version of CONFIG_XEN accelerator.
Use it in order to use "sysemu/xen-mapcache.h" in target-agnostic files.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-4-phi...@linaro.org>
---
 include/sysemu/xen-mapcache.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/sysemu/xen-mapcache.h b/include/sysemu/xen-mapcache.h
index c8e7c2f6cf..10c2e3082a 100644
--- a/include/sysemu/xen-mapcache.h
+++ b/include/sysemu/xen-mapcache.h
@@ -10,10 +10,11 @@
 #define XEN_MAPCACHE_H
 
 #include "exec/cpu-common.h"
+#include "sysemu/xen.h"
 
 typedef hwaddr (*phys_offset_to_gaddr_t)(hwaddr phys_offset,
  ram_addr_t size);
-#ifdef CONFIG_XEN
+#ifdef CONFIG_XEN_IS_POSSIBLE
 
 void xen_map_cache_init(phys_offset_to_gaddr_t f,
 void *opaque);
-- 
2.41.0




[PULL 07/43] hw/xen: Rename 'ram_memory' global variable as 'xen_memory'

2024-03-09 Thread Philippe Mathieu-Daudé
To avoid a potential global variable shadow in
hw/i386/pc_piix.c::pc_init1(), rename Xen's
"ram_memory" as "xen_memory".

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-11-phi...@linaro.org>
---
 include/hw/xen/xen-hvm-common.h |  2 +-
 hw/arm/xen_arm.c|  6 +++---
 hw/i386/xen/xen-hvm.c   | 10 +-
 hw/xen/xen-hvm-common.c |  6 +++---
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index 4b1d728f35..65a51aac2e 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -15,7 +15,7 @@
 #include "qemu/error-report.h"
 #include 
 
-extern MemoryRegion ram_memory;
+extern MemoryRegion xen_memory;
 extern MemoryListener xen_io_listener;
 extern DeviceListener xen_device_listener;
 
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 32776d94df..15fa7dfa84 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -114,14 +114,14 @@ static void xen_init_ram(MachineState *machine)
 block_len = GUEST_RAM1_BASE + ram_size[1];
 }
 
-memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len,
+memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
&error_fatal);
 
-memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &ram_memory,
+memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo", &xen_memory,
  GUEST_RAM0_BASE, ram_size[0]);
 memory_region_add_subregion(sysmem, GUEST_RAM0_BASE, &ram_lo);
 if (ram_size[1] > 0) {
-memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &ram_memory,
+memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi", &xen_memory,
  GUEST_RAM1_BASE, ram_size[1]);
 memory_region_add_subregion(sysmem, GUEST_RAM1_BASE, &ram_hi);
 }
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index f42621e674..1ae943370b 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -149,12 +149,12 @@ static void xen_ram_init(PCMachineState *pcms,
  */
 block_len = (4 * GiB) + x86ms->above_4g_mem_size;
 }
-memory_region_init_ram(&ram_memory, NULL, "xen.ram", block_len,
+memory_region_init_ram(&xen_memory, NULL, "xen.ram", block_len,
&error_fatal);
-*ram_memory_p = &ram_memory;
+*ram_memory_p = &xen_memory;
 
 memory_region_init_alias(&ram_640k, NULL, "xen.ram.640k",
- &ram_memory, 0, 0xa);
+ &xen_memory, 0, 0xa);
 memory_region_add_subregion(sysmem, 0, &ram_640k);
 /* Skip of the VGA IO memory space, it will be registered later by the VGA
  * emulated device.
@@ -163,12 +163,12 @@ static void xen_ram_init(PCMachineState *pcms,
  * the Options ROM, so it is registered here as RAM.
  */
 memory_region_init_alias(&ram_lo, NULL, "xen.ram.lo",
- &ram_memory, 0xc,
+ &xen_memory, 0xc,
  x86ms->below_4g_mem_size - 0xc);
 memory_region_add_subregion(sysmem, 0xc, &ram_lo);
 if (x86ms->above_4g_mem_size > 0) {
 memory_region_init_alias(&ram_hi, NULL, "xen.ram.hi",
- &ram_memory, 0x1ULL,
+ &xen_memory, 0x1ULL,
  x86ms->above_4g_mem_size);
 memory_region_add_subregion(sysmem, 0x1ULL, &ram_hi);
 }
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index baa1adb9f2..dc69cada57 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -9,7 +9,7 @@
 #include "hw/boards.h"
 #include "hw/xen/arch_hvm.h"
 
-MemoryRegion ram_memory;
+MemoryRegion xen_memory;
 
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
Error **errp)
@@ -26,7 +26,7 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr,
 return;
 }
 
-if (mr == &ram_memory) {
+if (mr == &xen_memory) {
 return;
 }
 
@@ -53,7 +53,7 @@ static void xen_set_memory(struct MemoryListener *listener,
 {
 XenIOState *state = container_of(listener, XenIOState, memory_listener);
 
-if (section->mr == &ram_memory) {
+if (section->mr == &xen_memory) {
 return;
 } else {
 if (add) {
-- 
2.41.0




[PULL 06/43] hw/xen: Remove unnecessary xen_hvm_inject_msi() stub

2024-03-09 Thread Philippe Mathieu-Daudé
Since commit 04b0de0ee8 ("xen: factor out common functions")
xen_hvm_inject_msi() stub is not required.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-8-phi...@linaro.org>
---
 stubs/xen-hw-stub.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/stubs/xen-hw-stub.c b/stubs/xen-hw-stub.c
index 7d7ffe83a9..6cf0e9a4c1 100644
--- a/stubs/xen-hw-stub.c
+++ b/stubs/xen-hw-stub.c
@@ -24,10 +24,6 @@ int xen_set_pci_link_route(uint8_t link, uint8_t irq)
 return -1;
 }
 
-void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
-{
-}
-
 int xen_is_pirq_msi(uint32_t msi_data)
 {
 return 0;
-- 
2.41.0




[PULL 14/43] hw/xen/hvm: Get target page size at runtime

2024-03-09 Thread Philippe Mathieu-Daudé
In order to build this file once for all targets, replace:

  TARGET_PAGE_BITS -> qemu_target_page_bits()
  TARGET_PAGE_SIZE -> qemu_target_page_size()
  TARGET_PAGE_MASK -> -qemu_target_page_size()

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Manos Pitsidianakis 
Message-Id: <20231114163123.74888-4-phi...@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 62 +++
 1 file changed, 39 insertions(+), 23 deletions(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 844b11ae08..7745cb3963 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -23,6 +23,7 @@
 #include "hw/xen/xen-hvm-common.h"
 #include "hw/xen/arch_hvm.h"
 #include 
+#include "exec/target_page.h"
 
 static MemoryRegion ram_640k, ram_lo, ram_hi;
 static MemoryRegion *framebuffer;
@@ -247,6 +248,9 @@ static int xen_add_to_physmap(XenIOState *state,
   MemoryRegion *mr,
   hwaddr offset_within_region)
 {
+unsigned target_page_bits = qemu_target_page_bits();
+int page_size = qemu_target_page_size();
+int page_mask = -page_size;
 unsigned long nr_pages;
 int rc = 0;
 XenPhysmap *physmap = NULL;
@@ -254,7 +258,7 @@ static int xen_add_to_physmap(XenIOState *state,
 hwaddr phys_offset = memory_region_get_ram_addr(mr);
 const char *mr_name;
 
-if (get_physmapping(start_addr, size, TARGET_PAGE_MASK)) {
+if (get_physmapping(start_addr, size, page_mask)) {
 return 0;
 }
 if (size <= 0) {
@@ -294,9 +298,9 @@ go_physmap:
 return 0;
 }
 
-pfn = phys_offset >> TARGET_PAGE_BITS;
-start_gpfn = start_addr >> TARGET_PAGE_BITS;
-nr_pages = size >> TARGET_PAGE_BITS;
+pfn = phys_offset >> target_page_bits;
+start_gpfn = start_addr >> target_page_bits;
+nr_pages = size >> target_page_bits;
 rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, nr_pages, pfn,
 start_gpfn);
 if (rc) {
@@ -310,8 +314,8 @@ go_physmap:
 }
 
 rc = xendevicemodel_pin_memory_cacheattr(xen_dmod, xen_domid,
-   start_addr >> TARGET_PAGE_BITS,
-   (start_addr + size - 1) >> TARGET_PAGE_BITS,
+   start_addr >> target_page_bits,
+   (start_addr + size - 1) >> target_page_bits,
XEN_DOMCTL_MEM_CACHEATTR_WB);
 if (rc) {
 error_report("pin_memory_cacheattr failed: %s", strerror(errno));
@@ -323,11 +327,14 @@ static int xen_remove_from_physmap(XenIOState *state,
hwaddr start_addr,
ram_addr_t size)
 {
+unsigned target_page_bits = qemu_target_page_bits();
+int page_size = qemu_target_page_size();
+int page_mask = -page_size;
 int rc = 0;
 XenPhysmap *physmap = NULL;
 hwaddr phys_offset = 0;
 
-physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
+physmap = get_physmapping(start_addr, size, page_mask);
 if (physmap == NULL) {
 return -1;
 }
@@ -338,9 +345,9 @@ static int xen_remove_from_physmap(XenIOState *state,
 DPRINTF("unmapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx", at "
 "%"HWADDR_PRIx"\n", start_addr, start_addr + size, phys_offset);
 
-size >>= TARGET_PAGE_BITS;
-start_addr >>= TARGET_PAGE_BITS;
-phys_offset >>= TARGET_PAGE_BITS;
+size >>= target_page_bits;
+start_addr >>= target_page_bits;
+phys_offset >>= target_page_bits;
 rc = xendevicemodel_relocate_memory(xen_dmod, xen_domid, size, start_addr,
 phys_offset);
 if (rc) {
@@ -369,13 +376,16 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
   hwaddr start_addr,
   ram_addr_t size)
 {
-hwaddr npages = size >> TARGET_PAGE_BITS;
+unsigned target_page_bits = qemu_target_page_bits();
+int page_size = qemu_target_page_size();
+int page_mask = -page_size;
+hwaddr npages = size >> target_page_bits;
 const int width = sizeof(unsigned long) * 8;
 size_t bitmap_size = DIV_ROUND_UP(npages, width);
 int rc, i, j;
 const XenPhysmap *physmap = NULL;
 
-physmap = get_physmapping(start_addr, size, TARGET_PAGE_MASK);
+physmap = get_physmapping(start_addr, size, page_mask);
 if (physmap == NULL) {
 /* not handled */
 return;
@@ -389,7 +399,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
 return;
 }
 
-rc = xen_track_dirty_vram(xen_domid, start_addr >> TARGET_PAGE_BITS,
+rc = xen_track_dirty_vram(xen_domid, start_addr >> target_page_bits,
   npages, dirty_bitmap);
 if (rc < 0) {
 #ifndef ENODATA
@@ -410,8 +420,7 @@ static void xen_sync_dirty_bitmap(XenIOState *state,
 j = ctzl(map);
 map &= ~

[PULL 09/43] hw/xen/xen_pt: Add missing license

2024-03-09 Thread Philippe Mathieu-Daudé
Commit eaab4d60d3 ("Introduce Xen PCI Passthrough, qdevice")
introduced both xen_pt.[ch], but only added the license to
xen_pt.c. Use the same license for xen_pt.h.

Suggested-by: David Woodhouse 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-17-phi...@linaro.org>
---
 hw/xen/xen_pt.h | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 31bcfdf705..d3180bb134 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -1,3 +1,13 @@
+/*
+ * Copyright (c) 2007, Neocleus Corporation.
+ * Copyright (c) 2007, Intel Corporation.
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Alex Novik 
+ * Allen Kay 
+ * Guy Zana 
+ */
 #ifndef XEN_PT_H
 #define XEN_PT_H
 
-- 
2.41.0




[PULL 01/43] hw/i386: Rename kvmvapic.c -> vapic.c

2024-03-09 Thread Philippe Mathieu-Daudé
vAPIC isn't KVM specific, so having its name prefixed 'kvm'
is misleading. Rename it simply 'vapic'. Rename the single
function prefixed 'kvm'.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20230905145159.7898-1-phi...@linaro.org>
---
 hw/i386/{kvmvapic.c => vapic.c} | 5 ++---
 hw/i386/meson.build | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)
 rename hw/i386/{kvmvapic.c => vapic.c} (99%)

diff --git a/hw/i386/kvmvapic.c b/hw/i386/vapic.c
similarity index 99%
rename from hw/i386/kvmvapic.c
rename to hw/i386/vapic.c
index 61a65ef2ab..f5b1db7e5f 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/vapic.c
@@ -747,8 +747,7 @@ static void do_vapic_enable(CPUState *cs, run_on_cpu_data 
data)
 s->state = VAPIC_ACTIVE;
 }
 
-static void kvmvapic_vm_state_change(void *opaque, bool running,
- RunState state)
+static void vapic_vm_state_change(void *opaque, bool running, RunState state)
 {
 MachineState *ms = MACHINE(qdev_get_machine());
 VAPICROMState *s = opaque;
@@ -793,7 +792,7 @@ static int vapic_post_load(void *opaque, int version_id)
 
 if (!s->vmsentry) {
 s->vmsentry =
-qemu_add_vm_change_state_handler(kvmvapic_vm_state_change, s);
+qemu_add_vm_change_state_handler(vapic_vm_state_change, s);
 }
 return 0;
 }
diff --git a/hw/i386/meson.build b/hw/i386/meson.build
index b9c1ca39cb..d8b70ef3e9 100644
--- a/hw/i386/meson.build
+++ b/hw/i386/meson.build
@@ -1,7 +1,7 @@
 i386_ss = ss.source_set()
 i386_ss.add(files(
   'fw_cfg.c',
-  'kvmvapic.c',
+  'vapic.c',
   'e820_memory_layout.c',
   'multiboot.c',
   'x86.c',
-- 
2.41.0




[PULL 08/43] hw/xen: Use target-agnostic qemu_target_page_bits()

2024-03-09 Thread Philippe Mathieu-Daudé
Instead of the target-specific TARGET_PAGE_BITS definition,
use qemu_target_page_bits() which is target agnostic.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-15-phi...@linaro.org>
---
 hw/xen/xen-hvm-common.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index dc69cada57..1627da7398 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -1,6 +1,7 @@
 #include "qemu/osdep.h"
 #include "qemu/units.h"
 #include "qapi/error.h"
+#include "exec/target_page.h"
 #include "trace.h"
 
 #include "hw/pci/pci_host.h"
@@ -14,6 +15,7 @@ MemoryRegion xen_memory;
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
Error **errp)
 {
+unsigned target_page_bits = qemu_target_page_bits();
 unsigned long nr_pfn;
 xen_pfn_t *pfn_list;
 int i;
@@ -32,11 +34,11 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, 
MemoryRegion *mr,
 
 trace_xen_ram_alloc(ram_addr, size);
 
-nr_pfn = size >> TARGET_PAGE_BITS;
+nr_pfn = size >> target_page_bits;
 pfn_list = g_new(xen_pfn_t, nr_pfn);
 
 for (i = 0; i < nr_pfn; i++) {
-pfn_list[i] = (ram_addr >> TARGET_PAGE_BITS) + i;
+pfn_list[i] = (ram_addr >> target_page_bits) + i;
 }
 
 if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, 
pfn_list)) {
-- 
2.41.0




[PULL 04/43] system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h'

2024-03-09 Thread Philippe Mathieu-Daudé
physmem.c doesn't use any declaration from "hw/xen/xen.h",
it only requires "sysemu/xen.h" and "system/xen-mapcache.h".

Suggested-by: David Woodhouse 
Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Reviewed-by: David Hildenbrand 
Message-Id: <20231114143816.71079-5-phi...@linaro.org>
---
 system/physmem.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/system/physmem.c b/system/physmem.c
index 3adda08ebf..6e9ed97597 100644
--- a/system/physmem.c
+++ b/system/physmem.c
@@ -35,7 +35,7 @@
 #include "hw/qdev-core.h"
 #include "hw/qdev-properties.h"
 #include "hw/boards.h"
-#include "hw/xen/xen.h"
+#include "sysemu/xen.h"
 #include "sysemu/kvm.h"
 #include "sysemu/tcg.h"
 #include "sysemu/qtest.h"
-- 
2.41.0




[PULL 20/43] hw/i386/pc: Use generated NotifyVmexitOption_str()

2024-03-09 Thread Philippe Mathieu-Daudé
NotifyVmexitOption_str() is QAPI-generated in
"qapi/qapi-types-run-state.h", which "sysemu/runstate.h"
already includes.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-Id: <20240301185936.95175-3-phi...@linaro.org>
---
 hw/i386/pc_piix.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index e123458bbc..ed777e3d61 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -61,6 +61,7 @@
 #include "hw/xen/xen.h"
 #include "migration/global_state.h"
 #include "migration/misc.h"
+#include "sysemu/runstate.h"
 #include "sysemu/numa.h"
 #include "hw/hyperv/vmbus-bridge.h"
 #include "hw/mem/nvdimm.h"
@@ -383,9 +384,6 @@ static const QEnumLookup PCSouthBridgeOption_lookup = {
 .size = PC_SOUTH_BRIDGE_OPTION_MAX
 };
 
-#define NotifyVmexitOption_str(val) \
-qapi_enum_lookup(&NotifyVmexitOption_lookup, (val))
-
 static int pc_get_south_bridge(Object *obj, Error **errp)
 {
 PCMachineState *pcms = PC_MACHINE(obj);
-- 
2.41.0




[PULL 05/43] hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen

2024-03-09 Thread Philippe Mathieu-Daudé
Similarly to the restriction in hw/pci/msix.c (see commit
e1e4bf2252 "msix: fix msix_vector_masked"), restrict the
xen_is_pirq_msi() call in msi_is_masked() to Xen.

No functional change intended.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-7-phi...@linaro.org>
---
 hw/pci/msi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index 041b0bdbec..8104ac1d91 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -23,6 +23,7 @@
 #include "hw/xen/xen.h"
 #include "qemu/range.h"
 #include "qapi/error.h"
+#include "sysemu/xen.h"
 
 #include "hw/i386/kvm/xen_evtchn.h"
 
@@ -308,7 +309,7 @@ bool msi_is_masked(const PCIDevice *dev, unsigned int 
vector)
 }
 
 data = pci_get_word(dev->config + msi_data_off(dev, msi64bit));
-if (xen_is_pirq_msi(data)) {
+if (xen_enabled() && xen_is_pirq_msi(data)) {
 return false;
 }
 
-- 
2.41.0




[PULL 12/43] hw/xen/hvm: Inline TARGET_PAGE_ALIGN() macro

2024-03-09 Thread Philippe Mathieu-Daudé
Use TARGET_PAGE_SIZE to calculate TARGET_PAGE_ALIGN.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Manos Pitsidianakis 
Message-Id: <20231114163123.74888-2-phi...@linaro.org>
---
 hw/i386/xen/xen-hvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 1ae943370b..8235782ef7 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -678,7 +678,7 @@ void arch_xen_set_memory(XenIOState *state, 
MemoryRegionSection *section,
 trace_xen_client_set_memory(start_addr, size, log_dirty);
 
 start_addr &= TARGET_PAGE_MASK;
-size = TARGET_PAGE_ALIGN(size);
+size = ROUND_UP(size, TARGET_PAGE_SIZE);
 
 if (add) {
 if (!memory_region_is_rom(section->mr)) {
-- 
2.41.0




[PULL 02/43] sysemu/xen: Forbid using Xen headers in user emulation

2024-03-09 Thread Philippe Mathieu-Daudé
Xen is a system specific accelerator, it makes no sense
to include its headers in user emulation.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Reviewed-by: David Woodhouse 
Message-Id: <20231114143816.71079-3-phi...@linaro.org>
---
 include/sysemu/xen.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/sysemu/xen.h b/include/sysemu/xen.h
index bc13ad5692..a9f591f26d 100644
--- a/include/sysemu/xen.h
+++ b/include/sysemu/xen.h
@@ -10,6 +10,10 @@
 #ifndef SYSEMU_XEN_H
 #define SYSEMU_XEN_H
 
+#ifdef CONFIG_USER_ONLY
+#error Cannot include sysemu/xen.h from user emulation
+#endif
+
 #include "exec/cpu-common.h"
 
 #ifdef NEED_CPU_H
@@ -26,16 +30,13 @@ extern bool xen_allowed;
 
 #define xen_enabled()   (xen_allowed)
 
-#ifndef CONFIG_USER_ONLY
 void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length);
 void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr, Error **errp);
-#endif
 
 #else /* !CONFIG_XEN_IS_POSSIBLE */
 
 #define xen_enabled() 0
-#ifndef CONFIG_USER_ONLY
 static inline void xen_hvm_modified_memory(ram_addr_t start, ram_addr_t length)
 {
 /* nothing */
@@ -45,7 +46,6 @@ static inline void xen_ram_alloc(ram_addr_t ram_addr, 
ram_addr_t size,
 {
 g_assert_not_reached();
 }
-#endif
 
 #endif /* CONFIG_XEN_IS_POSSIBLE */
 
-- 
2.41.0




[PULL 00/43] Misc HW patches for 2024-03-09

2024-03-09 Thread Philippe Mathieu-Daudé
The following changes since commit 84644ac1b0f80d41b8a2f66547b83b2ad4a98576:

  Merge tag 'darwin-20240305' of https://github.com/philmd/qemu into staging 
(2024-03-08 18:19:25 +)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/hw-misc-20240309

for you to fetch changes up to d3c79c397484ad117063702e6246e39f22f020f6:

  hw/m68k/mcf5208: add support for reset (2024-03-09 19:17:01 +0100)


Misc HW patch queue

- hmp: Shorter 'info qtree' output (Zoltan)
- qdev: Add a granule_mode property (Eric)
- Some ERRP_GUARD() fixes (Zhao)
- Doc & style fixes in docs/interop/firmware.json (Thomas)
- hw/xen: Housekeeping (Phil)
- hw/ppc/mac99: Change timebase frequency 25 -> 100 MHz (Mark)
- hw/intc/apic: Memory leak fix (Paolo)
- hw/intc/grlib_irqmp: Ensure ncpus value is in range (Clément)
- hw/m68k/mcf5208: Add support for reset (Angelo)
- hw/i386/pc: Housekeeping (Phil)
- hw/core/smp: Remove/deprecate parameter=0,1 adapting test-smp-parse (Zhao)



Angelo Dureghello (1):
  hw/m68k/mcf5208: add support for reset

BALATON Zoltan (1):
  hmp: Add option to info qtree to omit details

Clément Chigot (1):
  hw/intc/grlib_irqmp: abort realize when ncpus value is out of range

Eric Auger (1):
  qdev: Add a granule_mode property

Mark Cave-Ayland (1):
  mac_newworld: change timebase frequency from 100MHz to 25MHz for mac99
machine

Paolo Bonzini (1):
  hw/intc/apic: fix memory leak

Philippe Mathieu-Daudé (18):
  hw/i386: Rename kvmvapic.c -> vapic.c
  sysemu/xen: Forbid using Xen headers in user emulation
  sysemu/xen-mapcache: Check Xen availability with
CONFIG_XEN_IS_POSSIBLE
  system/physmem: Do not include 'hw/xen/xen.h' but 'sysemu/xen.h'
  hw/pci/msi: Restrict xen_is_pirq_msi() call to Xen
  hw/xen: Remove unnecessary xen_hvm_inject_msi() stub
  hw/xen: Rename 'ram_memory' global variable as 'xen_memory'
  hw/xen: Use target-agnostic qemu_target_page_bits()
  hw/xen/xen_pt: Add missing license
  hw/xen: Extract 'xen_igd.h' from 'xen_pt.h'
  hw/i386/xen: Compile 'xen-hvm.c' with Xen CPPFLAGS
  hw/xen/hvm: Inline TARGET_PAGE_ALIGN() macro
  hw/xen/hvm: Propagate page_mask to a pair of functions
  hw/xen/hvm: Get target page size at runtime
  hw/i386/pc: Remove pc_compat_1_4..1.7[] left over declarations
  hw/i386/pc: Use generated NotifyVmexitOption_str()
  hw/i386/pc: Remove 'host_type' argument from pc_init1()
  hw/i386/pc: Have pc_init_isa() pass a NULL pci_type argument

Thomas Weißschuh (2):
  docs/interop/firmware.json: Align examples
  docs/interop/firmware.json: Fix doc for FirmwareFlashMode

Zhao Liu (17):
  hw/char/xen_console: Fix missing ERRP_GUARD() for error_prepend()
  hw/net/xen_nic: Fix missing ERRP_GUARD() for error_prepend()
  hw/remote/remote-obj: hw/misc/ivshmem: Fix missing ERRP_GUARD() for
error_prepend()
  target/i386/sev: Fix missing ERRP_GUARD() for error_prepend()
  hw/core/machine-smp: Remove deprecated "parameter=0" SMP
configurations
  hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP
configurations
  hw/core/machine-smp: Calculate total CPUs once in
machine_parse_smp_config()
  tests/unit/test-smp-parse: Drop the unsupported "dies=1" case
  tests/unit/test-smp-parse: Use CPU number macros in invalid topology
case
  tests/unit/test-smp-parse: Bump max_cpus to 4096
  tests/unit/test-smp-parse: Make test cases aware of the book/drawer
  tests/unit/test-smp-parse: Test "books" parameter in -smp
  tests/unit/test-smp-parse: Test "drawers" parameter in -smp
  tests/unit/test-smp-parse: Test "drawers" and "books" combination case
  tests/unit/test-smp-parse: Test the full 7-levels topology hierarchy
  tests/unit/test-smp-parse: Test smp_props.has_clusters
  tests/unit/test-smp-parse: Test "parameter=0" SMP configurations

 docs/about/deprecated.rst   |  30 +-
 docs/about/removed-features.rst |  15 +
 docs/interop/firmware.json  | 376 -
 qapi/virtio.json|  18 +
 hw/xen/xen_pt.h |  24 +-
 include/hw/i386/pc.h|  12 -
 include/hw/qdev-properties-system.h |   3 +
 include/hw/xen/xen-hvm-common.h |   2 +-
 include/hw/xen/xen_igd.h|  33 ++
 include/sysemu/xen-mapcache.h   |   3 +-
 include/sysemu/xen.h|   8 +-
 accel/xen/xen-all.c |   1 +
 hw/arm/xen_arm.c|   6 +-
 hw/char/xen_console.c   |   1 +
 hw/core/machine-smp.c   |  78 ++--
 hw/core/qdev-properties-system.c|  14 +
 hw/i386/pc_piix.c   |  19 +-
 hw/i386/{kvmvapic.c => vapic.c} |   5 +-
 hw/i386/xen/xen-hvm.c   |  82 ++--
 h

Re: [PATCH v8 06/10] trans_rvv.c.inc: set vstart = 0 in int scalar move insns

2024-03-09 Thread Richard Henderson

On 3/8/24 11:53, Daniel Henrique Barboza wrote:

trans_vmv_x_s, trans_vmv_s_x, trans_vfmv_f_s and trans_vfmv_s_f aren't
setting vstart = 0 after execution. This is usually done by a helper in
vector_helper.c but these functions don't use helpers.

We'll set vstart after any potential 'over' brconds, and that will also
mandate a mark_vs_dirty() too.

Fixes: dedc53cbc9 ("target/riscv: rvv-1.0: integer scalar move instructions")
Signed-off-by: Daniel Henrique Barboza 
---
  target/riscv/insn_trans/trans_rvv.c.inc | 17 +++--
  1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index e42728990e..f3caabc101 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -3356,6 +3356,13 @@ static void vec_element_storei(DisasContext *s, int vreg,
  store_element(val, tcg_env, endian_ofs(s, vreg, idx), s->sew);
  }
  
+static void vec_set_vstart_zero(void)

+{
+TCGv_i32 t_zero = tcg_constant_i32(0);
+
+tcg_gen_st_i32(t_zero, tcg_env, offsetof(CPURISCVState, vstart));
+}


If you have not removed cpu_vstart, then you must use tcg_gen_movi_i32.
You cannot write to the backing store behind tcg's back.


r~



Re: [PATCH 1/2] hw/pci-host/pam: Free PAMMemoryRegion from Intel-specific bit handling

2024-03-09 Thread Bernhard Beschow



Am 9. März 2024 16:29:23 UTC schrieb "Philippe Mathieu-Daudé" 
:
>Hi Bernhard,
>
>On 9/3/24 14:40, Bernhard Beschow wrote:
>> The PAM bit extraction is currently spread across pam.c and the northbridge
>> device models, making the extraction logic harder to comprehend. Also note 
>> how
>> pam_update() deals with PAM_REGIONS_COUNT, even though it handles exactly one
>> region. Fix this (at the cost of minor code duplication) by moving the bit
>> extraction into the northbridge device models. As a side effect, pam_update()
>> becomes less Intel-specific which would allow it to be reused e.g. in VIA
>> northbridges.
>> 
>> Signed-off-by: Bernhard Beschow 
>> ---
>>   include/hw/pci-host/pam.h |  7 +++
>>   hw/pci-host/i440fx.c  |  7 +--
>>   hw/pci-host/pam.c | 14 +++---
>>   hw/pci-host/q35.c |  5 +++--
>>   4 files changed, 18 insertions(+), 15 deletions(-)
>> 
>> diff --git a/include/hw/pci-host/pam.h b/include/hw/pci-host/pam.h
>> index 005916f826..b9b33aecc8 100644
>> --- a/include/hw/pci-host/pam.h
>> +++ b/include/hw/pci-host/pam.h
>> @@ -70,7 +70,6 @@
>>   /* PAM registers: log nibble and high nibble*/
>>   #define PAM_ATTR_WE ((uint8_t)2)
>>   #define PAM_ATTR_RE ((uint8_t)1)
>> -#define PAM_ATTR_MASK   ((uint8_t)3)
>
>Why not use PAM_ATTR_foo instead of MCH_HOST_BRIDGE_PAM_foo?

Could be used indeed. See also below.

>
>>   /* SMRAM register */
>>   #define SMRAM_D_OPEN   ((uint8_t)(1 << 6))
>> @@ -83,13 +82,13 @@
>>   #define PAM_REGIONS_COUNT   13
>> typedef struct PAMMemoryRegion {
>> -MemoryRegion alias[4];  /* index = PAM value */
>> -unsigned current;
>> +MemoryRegion alias[4];  /* index = mode value */
>> +uint8_t mode;
>>   } PAMMemoryRegion;
>> void init_pam(PAMMemoryRegion *mem, Object *owner, MemoryRegion *ram,
>> MemoryRegion *system, MemoryRegion *pci,
>> uint32_t start, uint32_t size);
>> -void pam_update(PAMMemoryRegion *mem, int idx, uint8_t val);
>> +void pam_update(PAMMemoryRegion *mem, uint8_t mode);
>> #endif /* QEMU_PAM_H */
>> diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
>> index 4f0a0438d7..cddd506ab0 100644
>> --- a/hw/pci-host/i440fx.c
>> +++ b/hw/pci-host/i440fx.c
>> @@ -64,6 +64,8 @@ struct I440FXState {
>>   #define I440FX_PAM_SIZE 7
>>   #define I440FX_SMRAM0x72
>>   +#define I440FX_PAM_ATTR_MASK ((uint8_t)3)
>
>or (PAM_ATTR_RE|PAM_ATTR_WE)?
>
>It is odd to have I440FX_PAM_ATTR_MASK disconnected
>from the values it masks.

PAM_ATTR_RE and PAM_ATTR_WE are swapped in case of VIA. I didn't bother about 
these constants since both are currently not used at all. Shall I remove them 
in case of a respin?

>
>> -void pam_update(PAMMemoryRegion *pam, int idx, uint8_t val)
>> +void pam_update(PAMMemoryRegion *pam, uint8_t mode)
>>   {
>> -assert(0 <= idx && idx < PAM_REGIONS_COUNT);
>> +g_assert(mode < ARRAY_SIZE(pam->alias));
>>   -memory_region_set_enabled(&pam->alias[pam->current], false);
>> -pam->current = (val >> ((!(idx & 1)) * 4)) & PAM_ATTR_MASK;
>
>Can we pass the mask by argument instead?

For VIA, each PAM region is defined by just two bits (rather than four as for 
Intel). So a byte contains attributes for four regions instead of two. 
Therefore, passing a mask alone doesn't help, one needed to pass a shift value 
as well. Furthermore, since PAM_ATTR_RE and PAM_ATTR_WE are swapped, it seems 
cleaner to just pass the final mode value.

Do we consider avoiding the redundancies more worthwhile than having the bit 
extraction logic together? If so, I'm fine with dropping the series until a VIA 
northbridge gets accepted. Perhaps what's missing is a bit extraction API which 
spans multiple bytes. Please let me know.

>
>> -memory_region_set_enabled(&pam->alias[pam->current], true);
>> +memory_region_set_enabled(&pam->alias[pam->mode], false);
>> +pam->mode = mode;
>> +memory_region_set_enabled(&pam->alias[pam->mode], true);
>>   }
>
>Are the VIA values different of the PAM_ATTR_foo ones?

They are different except for PAM_ATTR_MASK.

>
>I'm not sure this is an helpful change, I'd rather
>remove the MCH_HOST_BRIDGE_PAM_foo definitions and
>use the PAM generic ones.

PAM_ATTR_MASK could indeed be reused for VIA. I'd respin if this series made 
sense in its own right.

Best regsrds,
Bernhard

>
>Regards,
>
>Phil.



Re: [PATCH v2] target/arm: Fix 32-bit SMOPA

2024-03-09 Thread Michael Tokarev

05.03.2024 19:39, Richard Henderson wrote:

While the 8-bit input elements are sequential in the input vector,
the 32-bit output elements are not sequential in the output matrix.
Do not attempt to compute 2 32-bit outputs at the same time.

Cc: qemu-sta...@nongnu.org
Fixes: 23a5e3859f5 ("target/arm: Implement SME integer outer product")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2083
Signed-off-by: Richard Henderson 


...

diff --git a/tests/tcg/aarch64/Makefile.target 
b/tests/tcg/aarch64/Makefile.target
index cded1d01fc..ea3e232e65 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -67,7 +67,7 @@ endif
  
  # SME Tests

  ifneq ($(CROSS_AS_HAS_ARMV9_SME),)
-AARCH64_TESTS += sme-outprod1
+AARCH64_TESTS += sme-outprod1 sme-smopa-1 sme-smopa-2
  endif


I tried to pick this one up for stable-7.2 (since the fix is for older commit),
and faced a fun issue in this change to tests/tcg/aarch64/Makefile.target,
since 7.2. doesn't have CROSS_AS_HAS_ARMV9_SME yet.  I went on and found out
that the following commits:

v7.2.0-374-gbc6bd20ee3  target/arm: align exposed ID registers with Linux
v8.0.0-2358-g3dc2afeab2 tests/tcg/aarch64/sysregs.c: Use S syntax for 
id_aa64zfr0_el1
and id_aa64smfr0_el1
v8.0.0-2361-g1f51573f79 target/arm: Fix SME full tile indexing

applies to 7.2, and lets this "Fix 32-bit SMOPA" change to apply cleanly,
including this very change in Makefile.target.

Now, 1f51573f79 "Fix SME full tile indexing" is Cc'd qemu-stable already, but
it is not in 7.2.x for some reason which I don't remember anymore, so it is
a good one to pick up already.  3dc2afeab is tests-only.

And bc6bd20ee3 (from Dec-2022) seems like a good candidate too, is it not?

I'm picking up all the bunch for now, let's see..

Thanks,

/mjt



Re: [PATCH v8 02/10] target/riscv: handle vstart >= vl in vext_set_tail_elems_1s()

2024-03-09 Thread Richard Henderson

On 3/8/24 11:53, Daniel Henrique Barboza wrote:

+static void vext_set_tail_elems_1s(CPURISCVState *env, void *vd,
+   uint32_t desc, uint32_t esz,
+   uint32_t max_elems)
  {
  uint32_t vta = vext_vta(desc);
+uint32_t nf = vext_nf(desc);
  int k;
  
-if (vta == 0) {

+/*
+ * Section 5.4 of the RVV spec mentions:
+ * "When vstart ≥ vl, there are no body elements, and no
+ *  elements are updated in any destination vector register
+ *  group, including that no tail elements are updated
+ *  with agnostic values."
+ */
+if (vta == 0 || env->vstart >= env->vl) {
  return;
  }


This isn't going to work for ...


@@ -222,9 +230,8 @@ vext_ldst_stride(void *vd, void *v0, target_ulong base,
  k++;
  }
  }
+vext_set_tail_elems_1s(env, vd, desc, esz, max_elems);
  env->vstart = 0;


... ldst, because we also need to increment vstart in the load loop, for consumption by 
the exception path.


You'll need to structure the helper as

if (vstart >= vl) {
   vstart = 0;
   return;
}
for (i = vstart; i < vl; vstart = ++i) {
...
}
set_tail_elems(...);
vstart = 0;


r~



Re: [PATCH v2] docs/conf.py: Remove usage of distutils

2024-03-09 Thread Philippe Mathieu-Daudé

On 9/3/24 18:27, Peter Maydell wrote:

On Tue, 5 Mar 2024 at 10:39, Thomas Huth  wrote:


On 04/03/2024 19.04, Daniel P. Berrangé wrote:

On Mon, Mar 04, 2024 at 06:11:58PM +0100, Thomas Huth wrote:

On 04/03/2024 17.56, Peter Maydell wrote:

On Mon, 4 Mar 2024 at 13:04, Thomas Huth  wrote:


The macOS jobs in our CI recently started failing, complaining that
the distutils module is not available anymore. And indeed, according to
https://peps.python.org/pep-0632/ it's been deprecated since a while
and now likely got removed in recent Python versions.


This doesn't seem to be sufficient to fix the macos CI:
something in glib seems to still be using it.

https://gitlab.com/qemu-project/qemu/-/jobs/6313212803

[281/6553] Generating ui/dbus-display gdbus-codegen with a custom command
FAILED: ui/dbus-display1.h ui/dbus-display1.c
/opt/homebrew/Cellar/glib/2.78.4/bin/gdbus-codegen
ui/dbus-display1.xml --glib-min-required 2.64 --output-directory
/private/var/folders/xc/tpssff9959345bnqq4c6tlwwgn/T/cirrus-ci-build/build/ui
--interface-prefix org.qemu. --c-namespace QemuDBus --generate-c-code
dbus-display1

...

ModuleNotFoundError: No module named 'distutils'


Looking at the glib sources, I think this has been fixed here:

   https://gitlab.gnome.org/GNOME/glib/-/commit/6ef967a0f930ce37a8c9b5aff96969

The fix will be in glib 2.79, unfortunately homebrew still seems to use glib
2.78 ...

We could maybe temporarily work-around the problem by disabling the dbus
code in the CI job? Or just wait for homebrew to update the package?


File a bug against homebrew. IME they are very quick (1-3 days) at
putting out fixes for things like this, especially if you point them
to the upstream solution.


Ok, while I was writing my mail, I was looking at https://brew.sh/ and
didn't see a link to a bug tracker there ... but now I realized that they
are simply using the github tracker, so I went ahead and filed a bug there:

   https://github.com/Homebrew/brew/issues/16823

Let's see how it goes...


Seems to be going slowly. I notice that there's a comment in there
saying that "brew install python-setuptools" is a workaround to
get glib 2.78 working -- that seems like it would be good to get
our CI back to green. Is there a way to test changes to the cirrus
config short of committing it and seeing if it helps? I don't see
the jobs available on a pipeline in my personal gitlab repo...


Yes, but you need to grant cirrus-ci some permissions you weren't
ready to give last time.

I just tested it and it is still failing for me:
https://gitlab.com/philmd/qemu/-/jobs/6357526794



Re: [PATCH] target/riscv: raise an exception when CSRRS/CSRRC writes a read-only CSR

2024-03-09 Thread Richard Henderson

On 3/7/24 22:40, Yu-Ming Chang via wrote:

Both CSRRS and CSRRC always read the addressed CSR and cause any read side
effects regardless of rs1 and rd fields. Note that if rs1 specifies a register
holding a zero value other than x0, the instruction will still attempt to write
the unmodified value back to the CSR and will cause any attendant side effects.

So if CSRRS or CSRRC tries to write a read-only CSR with rs1 which specifies
a register holding a zero value, an illegal instruction exception should be
raised.

Signed-off-by: Yu-Ming Chang 
---
  target/riscv/cpu.h   |  3 +++
  target/riscv/csr.c   | 18 +++---
  target/riscv/op_helper.c |  2 +-
  3 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5d291a7092..087ef64889 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -710,6 +710,9 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
  void riscv_cpu_update_mask(CPURISCVState *env);
  bool riscv_cpu_is_32bit(RISCVCPU *cpu);
  
+RISCVException riscv_csrr(CPURISCVState *env, int csrno,

+  target_ulong *ret_value,
+  target_ulong new_value, target_ulong write_mask);
  RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
 target_ulong *ret_value,
 target_ulong new_value, target_ulong write_mask);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d4e8ac13b9..3e49cf0df1 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -4306,7 +4306,7 @@ static RISCVException rmw_seed(CPURISCVState *env, int 
csrno,
  
  static inline RISCVException riscv_csrrw_check(CPURISCVState *env,

 int csrno,
-   bool write_mask)
+   bool write)
  {
  /* check privileges and return RISCV_EXCP_ILLEGAL_INST if check fails */
  bool read_only = get_field(csrno, 0xC00) == 3;
@@ -4328,7 +4328,7 @@ static inline RISCVException 
riscv_csrrw_check(CPURISCVState *env,
  }
  
  /* read / write check */

-if (write_mask && read_only) {
+if (write && read_only) {
  return RISCV_EXCP_ILLEGAL_INST;
  }
  
@@ -4415,11 +4415,23 @@ static RISCVException riscv_csrrw_do64(CPURISCVState *env, int csrno,

  return RISCV_EXCP_NONE;
  }
  
+RISCVException riscv_csrr(CPURISCVState *env, int csrno,

+   target_ulong *ret_value,
+   target_ulong new_value, target_ulong write_mask)


Remove new_value and write_mask arguments, as they don't make sense for read.


+{
+RISCVException ret = riscv_csrrw_check(env, csrno, false);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+return riscv_csrrw_do64(env, csrno, ret_value, new_value, write_mask);


... and pass zeros ...


-RISCVException ret = riscv_csrrw(env, csr, &val, 0, 0);
+RISCVException ret = riscv_csrr(env, csr, &val, 0, 0);


... like you do from the only caller.


r~




Re: [PATCH 2/2] tcg/aarch64: Fix tcg_out_brcond for test comparisons

2024-03-09 Thread Philippe Mathieu-Daudé

On 9/3/24 18:51, Richard Henderson wrote:

When converting test vs UINT32_MAX to compare vs 0, we need to
adjust the condition to match.

Cc: qemu-sta...@nongnu.org
Fixes: 34aff3c2e06 ("tcg/aarch64: Generate CBNZ for TSTNE of UINT32_MAX")
Signed-off-by: Richard Henderson 
---
  tcg/aarch64/tcg-target.c.inc | 1 +
  1 file changed, 1 insertion(+)


Reviewed-by: Philippe Mathieu-Daudé 





Re: [PATCH 1/2] tcg/aarch64: Fix tcg_out_cmp for test comparisons

2024-03-09 Thread Philippe Mathieu-Daudé

On 9/3/24 18:51, Richard Henderson wrote:

Pass the type to tcg_out_logicali; remove the assert, duplicated
at the start of tcg_out_logicali.

Cc: qemu-sta...@nongnu.org
Fixes: 339adf2f38e ("tcg/aarch64: Support TCG_COND_TST{EQ,NE}")


Oops, thanks.

Reviewed-by: Philippe Mathieu-Daudé 


Signed-off-by: Richard Henderson 
---
  tcg/aarch64/tcg-target.c.inc | 3 +--
  1 file changed, 1 insertion(+), 2 deletions(-)





Re: [PATCH] mac_newworld: change timebase frequency from 100MHz to 25MHz for mac99 machine

2024-03-09 Thread Philippe Mathieu-Daudé

On 4/3/24 08:35, Mark Cave-Ayland wrote:

MacOS X uses multiple techniques for calibrating timers depending upon the 
detected
hardware. One of these calibration routines compares the change in the timebase
against the KeyLargo timer and uses this to recalculate the clock frequency,
timebase frequency and bus frequency if the calibration exceeds certain limits.
This recalibration occurs despite the correct values being passed via the device
tree, and is likely due to buggy firmware on some hardware.

The timebase frequency of 100MHz was set way back in 2005 by commit fa296b0fb4
("PIC fix - changed back TB frequency to 100 MHz") and with this value on a
mac99,via=pmu machine the OSX 10.2 timer calibration incorrectly calculates the
bus frequency as 400MHz instead of 100MHz. The most noticeable side-effect is
the UI appears sluggish and not very responsive for normal use.

Change the timebase frequency from 100MHz to 25MHz which matches that of a real
G4 AGP machine (the closest match to QEMU's mac99 machine) and allows OSX 10.2
to correctly detect all of the clock frequency, timebase frequency and bus
frequency.

Tested on various MacOS images from OS 9.2 through to OSX 10.4, along with Linux
and NetBSD and I was unable to find any regressions from this change.

Signed-off-by: Mark Cave-Ayland 
---
  hw/ppc/mac_newworld.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Philippe Mathieu-Daudé 




[PATCH 1/2] tcg/aarch64: Fix tcg_out_cmp for test comparisons

2024-03-09 Thread Richard Henderson
Pass the type to tcg_out_logicali; remove the assert, duplicated
at the start of tcg_out_logicali.

Cc: qemu-sta...@nongnu.org
Fixes: 339adf2f38e ("tcg/aarch64: Support TCG_COND_TST{EQ,NE}")
Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.c.inc | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index dec8ecc1b6..38446c167e 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -1388,8 +1388,7 @@ static void tcg_out_cmp(TCGContext *s, TCGType ext, 
TCGCond cond, TCGReg a,
 if (!const_b) {
 tcg_out_insn(s, 3510, ANDS, ext, TCG_REG_XZR, a, b);
 } else {
-tcg_debug_assert(is_limm(b));
-tcg_out_logicali(s, I3404_ANDSI, 0, TCG_REG_XZR, a, b);
+tcg_out_logicali(s, I3404_ANDSI, ext, TCG_REG_XZR, a, b);
 }
 } else {
 if (!const_b) {
-- 
2.34.1




[PATCH 0/2] tcg/aarch64: Fixes for test comparisons

2024-03-09 Thread Richard Henderson
Oopsies exposed by Paolo's target/i386 changes.

r~

Richard Henderson (2):
  tcg/aarch64: Fix tcg_out_cmp for test comparisons
  tcg/aarch64: Fix tcg_out_brcond for test comparisons

 tcg/aarch64/tcg-target.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

-- 
2.34.1




[PATCH 2/2] tcg/aarch64: Fix tcg_out_brcond for test comparisons

2024-03-09 Thread Richard Henderson
When converting test vs UINT32_MAX to compare vs 0, we need to
adjust the condition to match.

Cc: qemu-sta...@nongnu.org
Fixes: 34aff3c2e06 ("tcg/aarch64: Generate CBNZ for TSTNE of UINT32_MAX")
Signed-off-by: Richard Henderson 
---
 tcg/aarch64/tcg-target.c.inc | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tcg/aarch64/tcg-target.c.inc b/tcg/aarch64/tcg-target.c.inc
index 38446c167e..56fc9cb9e0 100644
--- a/tcg/aarch64/tcg-target.c.inc
+++ b/tcg/aarch64/tcg-target.c.inc
@@ -1464,6 +1464,7 @@ static void tcg_out_brcond(TCGContext *s, TCGType ext, 
TCGCond c, TCGArg a,
 case TCG_COND_TSTNE:
 /* tst xN,0x; b.ne L -> cbnz wN,L */
 if (b_const && b == UINT32_MAX) {
+c = tcg_tst_eqne_cond(c);
 ext = TCG_TYPE_I32;
 need_cmp = false;
 break;
-- 
2.34.1




Re: [PATCH] usb: add support for sending MSIs from EHCI PCI devices

2024-03-09 Thread BALATON Zoltan

On Sat, 9 Mar 2024, Isaac Woods wrote:

Signed-off-by: Isaac Woods 
---
hw/usb/hcd-ehci-pci.c| 27 +++
hw/usb/hcd-ehci-sysbus.c |  7 +++
hw/usb/hcd-ehci.c|  2 +-
hw/usb/hcd-ehci.h|  2 ++
4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 3ff54edf62..8a714b6733 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -21,6 +21,8 @@
#include "migration/vmstate.h"
#include "qemu/module.h"
#include "qemu/range.h"
+#include "hw/pci/msi.h"
+#include "qapi/error.h"

typedef struct EHCIPCIInfo {
const char *name;
@@ -30,11 +32,27 @@ typedef struct EHCIPCIInfo {
bool companion;
} EHCIPCIInfo;

+static void ehci_pci_intr_update(EHCIState *ehci, bool enable)
+{
+EHCIPCIState *s = container_of(ehci, EHCIPCIState, ehci);
+PCIDevice *pci_dev = PCI_DEVICE(s);
+
+if (msi_enabled(pci_dev)) {
+if (enable) {
+msi_notify(pci_dev, 0);
+}
+} else {
+pci_set_irq(pci_dev, enable);
+}
+}
+
static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
{
EHCIPCIState *i = PCI_EHCI(dev);
EHCIState *s = &i->ehci;
uint8_t *pci_conf = dev->config;
+Error *err = NULL;
+int ret;

pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);

@@ -68,8 +86,17 @@ static void usb_ehci_pci_realize(PCIDevice *dev, Error 
**errp)
s->irq = pci_allocate_irq(dev);
s->as = pci_get_address_space(dev);

+s->intr_update = ehci_pci_intr_update;
+
usb_ehci_realize(s, DEVICE(dev), NULL);
pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
+
+ret = msi_init(dev, 0x70, 1, true, false, &err);
+if (ret) {
+error_propagate(errp, err);
+} else {
+error_free(err);
+}


I could be wrong but I think you don't have to do this and can just pass 
errp to msi_init here. You might need to check if (msi_init()) { return;} 
but as this is at the end even that does not matter (althogh may worth 
adding in case something is later added after this).


Regards,
BALATON Zoltan


}

static void usb_ehci_pci_init(Object *obj)
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index fe1dabd0bb..e08c856e2b 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -38,6 +38,11 @@ static Property ehci_sysbus_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};

+static void usb_ehci_sysbus_intr_update(EHCIState *ehci, bool level)
+{
+qemu_set_irq(s->irq, level);
+}
+
static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
{
SysBusDevice *d = SYS_BUS_DEVICE(dev);
@@ -70,6 +75,8 @@ static void ehci_sysbus_init(Object *obj)
s->portnr = sec->portnr;
s->as = &address_space_memory;

+s->intr_update = usb_ehci_sysbus_intr_update;
+
usb_ehci_init(s, DEVICE(obj));
sysbus_init_mmio(d, &s->mem);
}
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 01864d4649..e1f71dc445 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -209,7 +209,7 @@ static inline void ehci_update_irq(EHCIState *s)
}

trace_usb_ehci_irq(level, s->frindex, s->usbsts, s->usbintr);
-qemu_set_irq(s->irq, level);
+(s->intr_update)(s, level);
}

/* flag interrupt condition */
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 56a1c09d1f..bc017aec79 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -305,6 +305,8 @@ struct EHCIState {
EHCIQueueHead aqueues;
EHCIQueueHead pqueues;

+void (*intr_update)(EHCIState *s, bool enable);
+
/* which address to look at next */
uint32_t a_fetch_addr;
uint32_t p_fetch_addr;





Re: [PATCH v2] docs/conf.py: Remove usage of distutils

2024-03-09 Thread Peter Maydell
On Tue, 5 Mar 2024 at 10:39, Thomas Huth  wrote:
>
> On 04/03/2024 19.04, Daniel P. Berrangé wrote:
> > On Mon, Mar 04, 2024 at 06:11:58PM +0100, Thomas Huth wrote:
> >> On 04/03/2024 17.56, Peter Maydell wrote:
> >>> On Mon, 4 Mar 2024 at 13:04, Thomas Huth  wrote:
> 
>  The macOS jobs in our CI recently started failing, complaining that
>  the distutils module is not available anymore. And indeed, according to
>  https://peps.python.org/pep-0632/ it's been deprecated since a while
>  and now likely got removed in recent Python versions.
> >>>
> >>> This doesn't seem to be sufficient to fix the macos CI:
> >>> something in glib seems to still be using it.
> >>>
> >>> https://gitlab.com/qemu-project/qemu/-/jobs/6313212803
> >>>
> >>> [281/6553] Generating ui/dbus-display gdbus-codegen with a custom command
> >>> FAILED: ui/dbus-display1.h ui/dbus-display1.c
> >>> /opt/homebrew/Cellar/glib/2.78.4/bin/gdbus-codegen
> >>> ui/dbus-display1.xml --glib-min-required 2.64 --output-directory
> >>> /private/var/folders/xc/tpssff9959345bnqq4c6tlwwgn/T/cirrus-ci-build/build/ui
> >>> --interface-prefix org.qemu. --c-namespace QemuDBus --generate-c-code
> >>> dbus-display1
> >> ...
> >>> ModuleNotFoundError: No module named 'distutils'
> >>
> >> Looking at the glib sources, I think this has been fixed here:
> >>
> >>   
> >> https://gitlab.gnome.org/GNOME/glib/-/commit/6ef967a0f930ce37a8c9b5aff96969
> >>
> >> The fix will be in glib 2.79, unfortunately homebrew still seems to use 
> >> glib
> >> 2.78 ...
> >>
> >> We could maybe temporarily work-around the problem by disabling the dbus
> >> code in the CI job? Or just wait for homebrew to update the package?
> >
> > File a bug against homebrew. IME they are very quick (1-3 days) at
> > putting out fixes for things like this, especially if you point them
> > to the upstream solution.
>
> Ok, while I was writing my mail, I was looking at https://brew.sh/ and
> didn't see a link to a bug tracker there ... but now I realized that they
> are simply using the github tracker, so I went ahead and filed a bug there:
>
>   https://github.com/Homebrew/brew/issues/16823
>
> Let's see how it goes...

Seems to be going slowly. I notice that there's a comment in there
saying that "brew install python-setuptools" is a workaround to
get glib 2.78 working -- that seems like it would be good to get
our CI back to green. Is there a way to test changes to the cirrus
config short of committing it and seeing if it helps? I don't see
the jobs available on a pipeline in my personal gitlab repo...

thanks
-- PMM



Re: [PATCH 1/2] hw/pci-host/pam: Free PAMMemoryRegion from Intel-specific bit handling

2024-03-09 Thread Philippe Mathieu-Daudé

Hi Bernhard,

On 9/3/24 14:40, Bernhard Beschow wrote:

The PAM bit extraction is currently spread across pam.c and the northbridge
device models, making the extraction logic harder to comprehend. Also note how
pam_update() deals with PAM_REGIONS_COUNT, even though it handles exactly one
region. Fix this (at the cost of minor code duplication) by moving the bit
extraction into the northbridge device models. As a side effect, pam_update()
becomes less Intel-specific which would allow it to be reused e.g. in VIA
northbridges.

Signed-off-by: Bernhard Beschow 
---
  include/hw/pci-host/pam.h |  7 +++
  hw/pci-host/i440fx.c  |  7 +--
  hw/pci-host/pam.c | 14 +++---
  hw/pci-host/q35.c |  5 +++--
  4 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/include/hw/pci-host/pam.h b/include/hw/pci-host/pam.h
index 005916f826..b9b33aecc8 100644
--- a/include/hw/pci-host/pam.h
+++ b/include/hw/pci-host/pam.h
@@ -70,7 +70,6 @@
  /* PAM registers: log nibble and high nibble*/
  #define PAM_ATTR_WE ((uint8_t)2)
  #define PAM_ATTR_RE ((uint8_t)1)
-#define PAM_ATTR_MASK   ((uint8_t)3)


Why not use PAM_ATTR_foo instead of MCH_HOST_BRIDGE_PAM_foo?


  /* SMRAM register */
  #define SMRAM_D_OPEN   ((uint8_t)(1 << 6))
@@ -83,13 +82,13 @@
  #define PAM_REGIONS_COUNT   13
  
  typedef struct PAMMemoryRegion {

-MemoryRegion alias[4];  /* index = PAM value */
-unsigned current;
+MemoryRegion alias[4];  /* index = mode value */
+uint8_t mode;
  } PAMMemoryRegion;
  
  void init_pam(PAMMemoryRegion *mem, Object *owner, MemoryRegion *ram,

MemoryRegion *system, MemoryRegion *pci,
uint32_t start, uint32_t size);
-void pam_update(PAMMemoryRegion *mem, int idx, uint8_t val);
+void pam_update(PAMMemoryRegion *mem, uint8_t mode);
  
  #endif /* QEMU_PAM_H */

diff --git a/hw/pci-host/i440fx.c b/hw/pci-host/i440fx.c
index 4f0a0438d7..cddd506ab0 100644
--- a/hw/pci-host/i440fx.c
+++ b/hw/pci-host/i440fx.c
@@ -64,6 +64,8 @@ struct I440FXState {
  #define I440FX_PAM_SIZE 7
  #define I440FX_SMRAM0x72
  
+#define I440FX_PAM_ATTR_MASK ((uint8_t)3)


or (PAM_ATTR_RE|PAM_ATTR_WE)?

It is odd to have I440FX_PAM_ATTR_MASK disconnected
from the values it masks.


-void pam_update(PAMMemoryRegion *pam, int idx, uint8_t val)
+void pam_update(PAMMemoryRegion *pam, uint8_t mode)
  {
-assert(0 <= idx && idx < PAM_REGIONS_COUNT);
+g_assert(mode < ARRAY_SIZE(pam->alias));
  
-memory_region_set_enabled(&pam->alias[pam->current], false);

-pam->current = (val >> ((!(idx & 1)) * 4)) & PAM_ATTR_MASK;


Can we pass the mask by argument instead?


-memory_region_set_enabled(&pam->alias[pam->current], true);
+memory_region_set_enabled(&pam->alias[pam->mode], false);
+pam->mode = mode;
+memory_region_set_enabled(&pam->alias[pam->mode], true);
  }


Are the VIA values different of the PAM_ATTR_foo ones?

I'm not sure this is an helpful change, I'd rather
remove the MCH_HOST_BRIDGE_PAM_foo definitions and
use the PAM generic ones.

Regards,

Phil.



[PULL 00/11] Trivial patches for 2024-03-09

2024-03-09 Thread Michael Tokarev
The following changes since commit 84644ac1b0f80d41b8a2f66547b83b2ad4a98576:

  Merge tag 'darwin-20240305' of https://github.com/philmd/qemu into staging 
(2024-03-08 18:19:25 +)

are available in the Git repository at:

  https://gitlab.com/mjt0k/qemu.git tags/pull-trivial-patches

for you to fetch changes up to d65f1ed7de1559534d0a1fabca5bdd81c594c7ca:

  docs/acpi/bits: add some clarity and details while also improving formating 
(2024-03-09 18:56:37 +0300)


trivial patches for 2024-03-09


Ani Sinha (1):
  docs/acpi/bits: add some clarity and details while also improving 
formating

BALATON Zoltan (1):
  hw/scsi/lsi53c895a: Fix typo in comment

Frediano Ziglio (1):
  hw/vfio/pci.c: Make some structure static

Markus Armbruster (4):
  replay: Improve error messages about configuration conflicts
  char: Slightly better error reporting when chardev is in use
  blockdev: Fix block_resize error reporting for op blockers
  qerror: QERR_DEVICE_IN_USE is no longer used, drop

Michael Tokarev (1):
  make-release: switch to .xz format by default

Thomas Huth (3):
  hw/cxl/cxl-cdat: Fix type of buf in ct3_load_cdat()
  hw/pci-bridge/cxl_upstream: Fix problem with g_steal_pointer()
  hw/mem/cxl_type3: Fix problem with g_steal_pointer()

 blockdev.c   |  3 +--
 chardev/char-fe.c| 13 ++-
 docs/devel/acpi-bits.rst | 55 +++-
 hw/cxl/cxl-cdat.c|  4 ++--
 hw/mem/cxl_type3.c   | 24 +--
 hw/pci-bridge/cxl_upstream.c |  8 +++
 hw/scsi/lsi53c895a.c |  2 +-
 hw/vfio/pci.c|  4 ++--
 include/hw/cxl/cxl_cdat.h| 17 +-
 include/qapi/qmp/qerror.h|  3 ---
 replay/replay.c  |  2 +-
 scripts/make-release |  2 +-
 system/vl.c  |  2 +-
 13 files changed, 82 insertions(+), 57 deletions(-)



[PULL 11/11] docs/acpi/bits: add some clarity and details while also improving formating

2024-03-09 Thread Michael Tokarev
From: Ani Sinha 

Update bios-bits docs to add more details on why a pre-OS environment for
testing bioses is useful. Add author's FOSDEM talk link. Also improve the
formating of the document while at it.

Signed-off-by: Ani Sinha 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 docs/devel/acpi-bits.rst | 55 
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/docs/devel/acpi-bits.rst b/docs/devel/acpi-bits.rst
index 9677b0098f..1ec394f5fb 100644
--- a/docs/devel/acpi-bits.rst
+++ b/docs/devel/acpi-bits.rst
@@ -1,26 +1,48 @@
 =
 ACPI/SMBIOS avocado tests using biosbits
 =
-
+
+Introduction
+
 Biosbits is a software written by Josh Triplett that can be downloaded
 from https://biosbits.org/. The github codebase can be found
-`here `__. It is a software that 
executes
-the bios components such as acpi and smbios tables directly through acpica
-bios interpreter (a freely available C based library written by Intel,
+`here `__. It is a software that
+executes the bios components such as acpi and smbios tables directly through
+acpica bios interpreter (a freely available C based library written by Intel,
 downloadable from https://acpica.org/ and is included with biosbits) without an
-operating system getting involved in between.
+operating system getting involved in between. Bios-bits has python integration
+with grub so actual routines that executes bios components can be written in
+python instead of bash-ish (grub's native scripting language).
 There are several advantages to directly testing the bios in a real physical
-machine or VM as opposed to indirectly discovering bios issues through the
-operating system. For one thing, the OSes tend to hide bios problems from the
-end user. The other is that we have more control of what we wanted to test
-and how by directly using acpica interpreter on top of the bios on a running
-system. More details on the inspiration for developing biosbits and its real
-life uses can be found in [#a]_ and [#b]_.
+machine or in a VM as opposed to indirectly discovering bios issues through the
+operating system (the OS). Operating systems tend to bypass bios problems and
+hide them from the end user. We have more control of what we wanted to test and
+how by being as close to the bios on a running system as possible without a
+complicated software component such as an operating system coming in between.
+Another issue is that we cannot exercise bios components such as ACPI and
+SMBIOS without being in the highest hardware privilege level, ring 0 for
+example in case of x86. Since the OS executes from ring 0 whereas normal user
+land software resides in unprivileged ring 3, operating system must be modified
+in order to write our test routines that exercise and test the bios. This is
+not possible in all cases. Lastly, test frameworks and routines are preferably
+written using a high level scripting language such as python. OSes and
+OS modules are generally written using low level languages such as C and
+low level assembly machine language. Writing test routines in a low level
+language makes things more cumbersome. These and other reasons makes using
+bios-bits very attractive for testing bioses. More details on the inspiration
+for developing biosbits and its real life uses can be found in [#a]_ and [#b]_.
+
 For QEMU, we maintain a fork of bios bits in gitlab along with all the
-dependent submodules here: https://gitlab.com/qemu-project/biosbits-bits
+dependent submodules `here `__.
 This fork contains numerous fixes, a newer acpica and changes specific to
 running this avocado QEMU tests using bits. The author of this document
-is the sole maintainer of the QEMU fork of bios bits repo.
+is the sole maintainer of the QEMU fork of bios bits repository. For more
+information, please see author's `FOSDEM talk on this bios-bits based test
+framework 
`__.
+
+*
+Description of the test framework
+*
 
 Under the directory ``tests/avocado/``, ``acpi-bits.py`` is a QEMU avocado
 test that drives all this.
@@ -120,8 +142,9 @@ Under ``tests/avocado/`` as the root we have:
(b) Add a SPDX license header.
(c) Perform modifications to the test.
 
-   Commits (a), (b) and (c) should go under separate commits so that the 
original
-   test script and the changes we have made are separated and clear.
+   Commits (a), (b) and (c) preferably should go under separate commits so that
+   the origina

[PULL 09/11] hw/pci-bridge/cxl_upstream: Fix problem with g_steal_pointer()

2024-03-09 Thread Michael Tokarev
From: Thomas Huth 

When setting GLIB_VERSION_MAX_ALLOWED to GLIB_VERSION_2_58 or higher,
glib adds type safety checks to the g_steal_pointer() macro. This
triggers errors in the build_cdat_table() function which uses the
g_steal_pointer() for type-casting from one pointer type to the other
(which also looks quite weird since the local pointers have all been
declared with g_autofree though they are never freed here). Let's fix
it by using a proper typecast instead. For making this possible, we
have to remove the QEMU_PACKED attribute from some structs since GCC
otherwise complains that the source and destination pointer might
have different alignment restrictions. Removing the QEMU_PACKED should
be fine here since the structs are already naturally aligned. Anyway,
add some QEMU_BUILD_BUG_ON() statements to make sure that we've got
the right sizes (without padding in the structs).

Signed-off-by: Thomas Huth 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 hw/pci-bridge/cxl_upstream.c | 8 
 include/hw/cxl/cxl_cdat.h| 8 +---
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/pci-bridge/cxl_upstream.c b/hw/pci-bridge/cxl_upstream.c
index e87eb40177..537f9affb8 100644
--- a/hw/pci-bridge/cxl_upstream.c
+++ b/hw/pci-bridge/cxl_upstream.c
@@ -192,8 +192,8 @@ enum {
 
 static int build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
 {
-g_autofree CDATSslbis *sslbis_latency = NULL;
-g_autofree CDATSslbis *sslbis_bandwidth = NULL;
+CDATSslbis *sslbis_latency;
+CDATSslbis *sslbis_bandwidth;
 CXLUpstreamPort *us = CXL_USP(priv);
 PCIBus *bus = &PCI_BRIDGE(us)->sec_bus;
 int devfn, sslbis_size, i;
@@ -270,8 +270,8 @@ static int build_cdat_table(CDATSubHeader ***cdat_table, 
void *priv)
 *cdat_table = g_new0(CDATSubHeader *, CXL_USP_CDAT_NUM_ENTRIES);
 
 /* Header always at start of structure */
-(*cdat_table)[CXL_USP_CDAT_SSLBIS_LAT] = g_steal_pointer(&sslbis_latency);
-(*cdat_table)[CXL_USP_CDAT_SSLBIS_BW] = g_steal_pointer(&sslbis_bandwidth);
+(*cdat_table)[CXL_USP_CDAT_SSLBIS_LAT] = (CDATSubHeader *)sslbis_latency;
+(*cdat_table)[CXL_USP_CDAT_SSLBIS_BW] = (CDATSubHeader *)sslbis_bandwidth;
 
 return CXL_USP_CDAT_NUM_ENTRIES;
 }
diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
index 8e3d094608..b44cefaad6 100644
--- a/include/hw/cxl/cxl_cdat.h
+++ b/include/hw/cxl/cxl_cdat.h
@@ -130,7 +130,8 @@ typedef struct CDATSslbisHeader {
 uint8_t data_type;
 uint8_t reserved[3];
 uint64_t entry_base_unit;
-} QEMU_PACKED CDATSslbisHeader;
+} CDATSslbisHeader;
+QEMU_BUILD_BUG_ON(sizeof(CDATSslbisHeader) != 16);
 
 #define CDAT_PORT_ID_USP 0x100
 /* Switch Scoped Latency and Bandwidth Entry - CDAT Table 10 */
@@ -139,12 +140,13 @@ typedef struct CDATSslbe {
 uint16_t port_y_id;
 uint16_t latency_bandwidth;
 uint16_t reserved;
-} QEMU_PACKED CDATSslbe;
+} CDATSslbe;
+QEMU_BUILD_BUG_ON(sizeof(CDATSslbe) != 8);
 
 typedef struct CDATSslbis {
 CDATSslbisHeader sslbis_header;
 CDATSslbe sslbe[];
-} QEMU_PACKED CDATSslbis;
+} CDATSslbis;
 
 typedef struct CDATEntry {
 void *base;
-- 
2.39.2




[PULL 03/11] hw/scsi/lsi53c895a: Fix typo in comment

2024-03-09 Thread Michael Tokarev
From: BALATON Zoltan 

Signed-off-by: BALATON Zoltan 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 hw/scsi/lsi53c895a.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index d607a5f9fb..f8598a17f5 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -225,7 +225,7 @@ struct LSIState {
 MemoryRegion io_io;
 AddressSpace pci_io_as;
 
-int carry; /* ??? Should this be an a visible register somewhere?  */
+int carry; /* ??? Should this be in a visible register somewhere?  */
 int status;
 int msg_action;
 int msg_len;
-- 
2.39.2




[PULL 07/11] qerror: QERR_DEVICE_IN_USE is no longer used, drop

2024-03-09 Thread Michael Tokarev
From: Markus Armbruster 

Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 include/qapi/qmp/qerror.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 8dd9fcb071..0c2689cf8a 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -23,9 +23,6 @@
 #define QERR_DEVICE_HAS_NO_MEDIUM \
 "Device '%s' has no medium"
 
-#define QERR_DEVICE_IN_USE \
-"Device '%s' is in use"
-
 #define QERR_DEVICE_NO_HOTPLUG \
 "Device '%s' does not support hotplugging"
 
-- 
2.39.2




[PULL 10/11] hw/mem/cxl_type3: Fix problem with g_steal_pointer()

2024-03-09 Thread Michael Tokarev
From: Thomas Huth 

When setting GLIB_VERSION_MAX_ALLOWED to GLIB_VERSION_2_58 or higher,
glib adds type safety checks to the g_steal_pointer() macro. This
triggers errors in the ct3_build_cdat_entries_for_mr() function which
uses the g_steal_pointer() for type-casting from one pointer type to
the other (which also looks quite weird since the local pointers have
all been declared with g_autofree though they are never freed here).
Fix it by using a proper typecast instead. For making this possible, we
have to remove the QEMU_PACKED attribute from some structs since GCC
otherwise complains that the source and destination pointer might
have different alignment restrictions. Removing the QEMU_PACKED should
be fine here since the structs are already naturally aligned. Anyway,
add some QEMU_BUILD_BUG_ON() statements to make sure that we've got
the right sizes (without padding in the structs).

Signed-off-by: Thomas Huth 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 hw/mem/cxl_type3.c| 24 
 include/hw/cxl/cxl_cdat.h |  9 ++---
 2 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index e8801805b9..b679dfae1c 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -46,12 +46,12 @@ static void ct3_build_cdat_entries_for_mr(CDATSubHeader 
**cdat_table,
   int dsmad_handle, MemoryRegion *mr,
   bool is_pmem, uint64_t dpa_base)
 {
-g_autofree CDATDsmas *dsmas = NULL;
-g_autofree CDATDslbis *dslbis0 = NULL;
-g_autofree CDATDslbis *dslbis1 = NULL;
-g_autofree CDATDslbis *dslbis2 = NULL;
-g_autofree CDATDslbis *dslbis3 = NULL;
-g_autofree CDATDsemts *dsemts = NULL;
+CDATDsmas *dsmas;
+CDATDslbis *dslbis0;
+CDATDslbis *dslbis1;
+CDATDslbis *dslbis2;
+CDATDslbis *dslbis3;
+CDATDsemts *dsemts;
 
 dsmas = g_malloc(sizeof(*dsmas));
 *dsmas = (CDATDsmas) {
@@ -135,12 +135,12 @@ static void ct3_build_cdat_entries_for_mr(CDATSubHeader 
**cdat_table,
 };
 
 /* Header always at start of structure */
-cdat_table[CT3_CDAT_DSMAS] = g_steal_pointer(&dsmas);
-cdat_table[CT3_CDAT_DSLBIS0] = g_steal_pointer(&dslbis0);
-cdat_table[CT3_CDAT_DSLBIS1] = g_steal_pointer(&dslbis1);
-cdat_table[CT3_CDAT_DSLBIS2] = g_steal_pointer(&dslbis2);
-cdat_table[CT3_CDAT_DSLBIS3] = g_steal_pointer(&dslbis3);
-cdat_table[CT3_CDAT_DSEMTS] = g_steal_pointer(&dsemts);
+cdat_table[CT3_CDAT_DSMAS] = (CDATSubHeader *)dsmas;
+cdat_table[CT3_CDAT_DSLBIS0] = (CDATSubHeader *)dslbis0;
+cdat_table[CT3_CDAT_DSLBIS1] = (CDATSubHeader *)dslbis1;
+cdat_table[CT3_CDAT_DSLBIS2] = (CDATSubHeader *)dslbis2;
+cdat_table[CT3_CDAT_DSLBIS3] = (CDATSubHeader *)dslbis3;
+cdat_table[CT3_CDAT_DSEMTS] = (CDATSubHeader *)dsemts;
 }
 
 static int ct3_build_cdat_table(CDATSubHeader ***cdat_table, void *priv)
diff --git a/include/hw/cxl/cxl_cdat.h b/include/hw/cxl/cxl_cdat.h
index b44cefaad6..17a09066dc 100644
--- a/include/hw/cxl/cxl_cdat.h
+++ b/include/hw/cxl/cxl_cdat.h
@@ -82,7 +82,8 @@ typedef struct CDATDsmas {
 uint16_t reserved;
 uint64_t DPA_base;
 uint64_t DPA_length;
-} QEMU_PACKED CDATDsmas;
+} CDATDsmas;
+QEMU_BUILD_BUG_ON(sizeof(CDATDsmas) != 24);
 
 /* Device Scoped Latency and Bandwidth Information Structure - CDAT Table 5 */
 typedef struct CDATDslbis {
@@ -95,7 +96,8 @@ typedef struct CDATDslbis {
 uint64_t entry_base_unit;
 uint16_t entry[3];
 uint16_t reserved2;
-} QEMU_PACKED CDATDslbis;
+} CDATDslbis;
+QEMU_BUILD_BUG_ON(sizeof(CDATDslbis) != 24);
 
 /* Device Scoped Memory Side Cache Information Structure - CDAT Table 6 */
 typedef struct CDATDsmscis {
@@ -122,7 +124,8 @@ typedef struct CDATDsemts {
 uint16_t reserved;
 uint64_t DPA_offset;
 uint64_t DPA_length;
-} QEMU_PACKED CDATDsemts;
+} CDATDsemts;
+QEMU_BUILD_BUG_ON(sizeof(CDATDsemts) != 24);
 
 /* Switch Scoped Latency and Bandwidth Information Structure - CDAT Table 9 */
 typedef struct CDATSslbisHeader {
-- 
2.39.2




[PULL 05/11] char: Slightly better error reporting when chardev is in use

2024-03-09 Thread Michael Tokarev
From: Markus Armbruster 

Both

$ qemu-system-x86_64 -chardev null,id=chr0,mux=on -mon chardev=chr0 -mon 
chardev=chr0 -mon chardev=chr0 -mon chardev=chr0 -mon chardev=chr0

and

$ qemu-system-x86_64 -chardev null,id=chr0 -mon chardev=chr0 -mon 
chardev=chr0
fail with

qemu-system-x86_64: -mon chardev=chr0: Device 'chr0' is in use

Improve to

qemu-system-x86_64: -mon chardev=chr0: too many uses of multiplexed chardev 
'chr0' (maximum is 4)

and

qemu-system-x86_64: -mon chardev=chr0: chardev 'chr0' is already in use

Signed-off-by: Markus Armbruster 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 chardev/char-fe.c | 13 +++--
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/chardev/char-fe.c b/chardev/char-fe.c
index 20222a4cad..66cee8475a 100644
--- a/chardev/char-fe.c
+++ b/chardev/char-fe.c
@@ -199,13 +199,18 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error 
**errp)
 MuxChardev *d = MUX_CHARDEV(s);
 
 if (d->mux_cnt >= MAX_MUX) {
-goto unavailable;
+error_setg(errp,
+   "too many uses of multiplexed chardev '%s'"
+   " (maximum is " stringify(MAX_MUX) ")",
+   s->label);
+return false;
 }
 
 d->backends[d->mux_cnt] = b;
 tag = d->mux_cnt++;
 } else if (s->be) {
-goto unavailable;
+error_setg(errp, "chardev '%s' is already in use", s->label);
+return false;
 } else {
 s->be = b;
 }
@@ -215,10 +220,6 @@ bool qemu_chr_fe_init(CharBackend *b, Chardev *s, Error 
**errp)
 b->tag = tag;
 b->chr = s;
 return true;
-
-unavailable:
-error_setg(errp, QERR_DEVICE_IN_USE, s->label);
-return false;
 }
 
 void qemu_chr_fe_deinit(CharBackend *b, bool del)
-- 
2.39.2




[PULL 01/11] replay: Improve error messages about configuration conflicts

2024-03-09 Thread Michael Tokarev
From: Markus Armbruster 

Improve

   Record/replay feature is not supported for '-rtc base=localtime'
   Record/replay feature is not supported for 'smp'
   Record/replay feature is not supported for '-snapshot'

to

   Record/replay is not supported with -rtc base=localtime
   Record/replay is not supported with multiple CPUs
   Record/replay is not supported with -snapshot

Signed-off-by: Markus Armbruster 
Reviewed-by: Pavel Dovgalyuk 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 replay/replay.c | 2 +-
 system/vl.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/replay/replay.c b/replay/replay.c
index 3fd241a4fc..a2c576c16e 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -511,7 +511,7 @@ void replay_add_blocker(const char *feature)
 {
 Error *reason = NULL;
 
-error_setg(&reason, "Record/replay feature is not supported for '%s'",
+error_setg(&reason, "Record/replay is not supported with %s",
feature);
 replay_blockers = g_slist_prepend(replay_blockers, reason);
 }
diff --git a/system/vl.c b/system/vl.c
index 48aae6e053..70f4cece7f 100644
--- a/system/vl.c
+++ b/system/vl.c
@@ -1932,7 +1932,7 @@ static void qemu_apply_machine_options(QDict *qdict)
 }
 
 if (current_machine->smp.cpus > 1) {
-replay_add_blocker("smp");
+replay_add_blocker("multiple CPUs");
 }
 }
 
-- 
2.39.2




[PULL 04/11] make-release: switch to .xz format by default

2024-03-09 Thread Michael Tokarev
For a long time, we provide two compression formats in the
download area, .bz2 and .xz.  There's absolutely no reason
to provide two in parallel, .xz compresses better, and all
the links we use points to .xz.  Downstream distributions
mostly use .xz too.

For the release maintenance providing two formats is definitely
extra burden too.

Signed-off-by: Michael Tokarev 
Reviewed-by: Daniel P. Berrangé 
Reviewed-by: Stefan Hajnoczi 
---
 scripts/make-release | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/make-release b/scripts/make-release
index 9c570b87f4..6e0433de24 100755
--- a/scripts/make-release
+++ b/scripts/make-release
@@ -47,5 +47,5 @@ meson subprojects download $SUBPROJECTS
 CryptoPkg/Library/OpensslLib/openssl \
 MdeModulePkg/Library/BrotliCustomDecompressLib/brotli)
 popd
-tar --exclude=.git -cjf ${destination}.tar.bz2 ${destination}
+tar --exclude=.git -cJf ${destination}.tar.xz ${destination}
 rm -rf ${destination}
-- 
2.39.2




[PULL 02/11] hw/vfio/pci.c: Make some structure static

2024-03-09 Thread Michael Tokarev
From: Frediano Ziglio 

Not used outside C module.

Signed-off-by: Frediano Ziglio 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Reviewed-by Tokarev 
Signed-off-by: Michael Tokarev 
---
 hw/vfio/pci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 4fa387f043..a1522a011a 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2558,7 +2558,7 @@ static bool vfio_display_migration_needed(void *opaque)
 (vdev->ramfb_migrate == ON_OFF_AUTO_AUTO && vdev->enable_ramfb);
 }
 
-const VMStateDescription vmstate_vfio_display = {
+static const VMStateDescription vmstate_vfio_display = {
 .name = "VFIOPCIDevice/VFIODisplay",
 .version_id = 1,
 .minimum_version_id = 1,
@@ -2570,7 +2570,7 @@ const VMStateDescription vmstate_vfio_display = {
 }
 };
 
-const VMStateDescription vmstate_vfio_pci_config = {
+static const VMStateDescription vmstate_vfio_pci_config = {
 .name = "VFIOPCIDevice",
 .version_id = 1,
 .minimum_version_id = 1,
-- 
2.39.2




[PULL 06/11] blockdev: Fix block_resize error reporting for op blockers

2024-03-09 Thread Michael Tokarev
From: Markus Armbruster 

When block_resize() runs into an op blocker, it creates an error like
this:

error_setg(errp, "Device '%s' is in use", device);

Trouble is @device can be null.  My system formats null as "(null)",
but other systems might crash.  Reproducer:

1. Create two block devices

-> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": 
"blk0", "filename": "64k.img"}}
<- {"return": {}}
-> {"execute": "blockdev-add", "arguments": {"driver": "file", "node-name": 
"blk1", "filename": "m.img"}}
<- {"return": {}}

2. Put a blocker on one them

-> {"execute": "blockdev-mirror", "arguments": {"job-id": "job0", "device": 
"blk0", "target": "blk1", "sync": "full"}}
{"return": {}}
-> {"execute": "job-pause", "arguments": {"id": "job0"}}
{"return": {}}
-> {"execute": "job-complete", "arguments": {"id": "job0"}}
{"return": {}}

   Note: job events elided for brevity.

3. Attempt to resize

-> {"execute": "block_resize", "arguments": {"node-name": "blk1", 
"size":32768}}
<- {"error": {"class": "GenericError", "desc": "Device '(null)' is in use"}}

Broken when commit 3b1dbd11a60 made @device optional.  Fixed in commit
ed3d2ec98a3 (block: Add errp to b{lk,drv}_truncate()), except for this
one instance.

Fix it by using the error message provided by the op blocker instead,
so it fails like this:

<- {"error": {"class": "GenericError", "desc": "Node 'blk1' is busy: block 
device is in use by block job: mirror"}}

Fixes: 3b1dbd11a60d (qmp: Allow block_resize to manipulate bs graph nodes.)
Signed-off-by: Markus Armbruster 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 blockdev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index f8bb0932f8..d8fb3399f5 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -2252,8 +2252,7 @@ void coroutine_fn qmp_block_resize(const char *device, 
const char *node_name,
 }
 
 bdrv_graph_co_rdlock();
-if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, NULL)) {
-error_setg(errp, QERR_DEVICE_IN_USE, device);
+if (bdrv_op_is_blocked(bs, BLOCK_OP_TYPE_RESIZE, errp)) {
 bdrv_graph_co_rdunlock();
 return;
 }
-- 
2.39.2




[PULL 08/11] hw/cxl/cxl-cdat: Fix type of buf in ct3_load_cdat()

2024-03-09 Thread Michael Tokarev
From: Thomas Huth 

When setting GLIB_VERSION_MAX_ALLOWED to GLIB_VERSION_2_58 or higher
(which we'll certainly do in the not too distant future), glib adds
type safety checks to the g_steal_pointer() macro. This trigger an
error in the ct3_load_cdat() function: The local char *buf variable is
assigned to uint8_t *buf in CDATObject, i.e. a pointer of a different
type. Change the local variable to the same type as buf in CDATObject
to avoid the error.

Signed-off-by: Thomas Huth 
Reviewed-by: Jonathan Cameron 
Reviewed-by: Michael Tokarev 
Signed-off-by: Michael Tokarev 
---
 hw/cxl/cxl-cdat.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/cxl/cxl-cdat.c b/hw/cxl/cxl-cdat.c
index 2fea975671..551545f782 100644
--- a/hw/cxl/cxl-cdat.c
+++ b/hw/cxl/cxl-cdat.c
@@ -114,7 +114,7 @@ static void ct3_build_cdat(CDATObject *cdat, Error **errp)
 static void ct3_load_cdat(CDATObject *cdat, Error **errp)
 {
 g_autofree CDATEntry *cdat_st = NULL;
-g_autofree char *buf = NULL;
+g_autofree uint8_t *buf = NULL;
 uint8_t sum = 0;
 int num_ent;
 int i = 0, ent = 1;
@@ -171,7 +171,7 @@ static void ct3_load_cdat(CDATObject *cdat, Error **errp)
 cdat_st[ent].base = hdr;
 cdat_st[ent].length = hdr->length;
 
-while (buf + i < (char *)cdat_st[ent].base + cdat_st[ent].length) {
+while (buf + i < (uint8_t *)cdat_st[ent].base + cdat_st[ent].length) {
 assert(i < file_size);
 sum += buf[i++];
 }
-- 
2.39.2




[PATCH] usb: add support for sending MSIs from EHCI PCI devices

2024-03-09 Thread Isaac Woods
Signed-off-by: Isaac Woods 
---
 hw/usb/hcd-ehci-pci.c| 27 +++
 hw/usb/hcd-ehci-sysbus.c |  7 +++
 hw/usb/hcd-ehci.c|  2 +-
 hw/usb/hcd-ehci.h|  2 ++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 3ff54edf62..8a714b6733 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -21,6 +21,8 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "qemu/range.h"
+#include "hw/pci/msi.h"
+#include "qapi/error.h"
 
 typedef struct EHCIPCIInfo {
 const char *name;
@@ -30,11 +32,27 @@ typedef struct EHCIPCIInfo {
 bool companion;
 } EHCIPCIInfo;
 
+static void ehci_pci_intr_update(EHCIState *ehci, bool enable)
+{
+EHCIPCIState *s = container_of(ehci, EHCIPCIState, ehci);
+PCIDevice *pci_dev = PCI_DEVICE(s);
+
+if (msi_enabled(pci_dev)) {
+if (enable) {
+msi_notify(pci_dev, 0);
+}
+} else {
+pci_set_irq(pci_dev, enable);
+}
+}
+
 static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
 {
 EHCIPCIState *i = PCI_EHCI(dev);
 EHCIState *s = &i->ehci;
 uint8_t *pci_conf = dev->config;
+Error *err = NULL;
+int ret;
 
 pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);
 
@@ -68,8 +86,17 @@ static void usb_ehci_pci_realize(PCIDevice *dev, Error 
**errp)
 s->irq = pci_allocate_irq(dev);
 s->as = pci_get_address_space(dev);
 
+s->intr_update = ehci_pci_intr_update;
+
 usb_ehci_realize(s, DEVICE(dev), NULL);
 pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
+
+ret = msi_init(dev, 0x70, 1, true, false, &err);
+if (ret) {
+error_propagate(errp, err);
+} else {
+error_free(err);
+}
 }
 
 static void usb_ehci_pci_init(Object *obj)
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index fe1dabd0bb..e08c856e2b 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -38,6 +38,11 @@ static Property ehci_sysbus_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static void usb_ehci_sysbus_intr_update(EHCIState *ehci, bool level)
+{
+qemu_set_irq(s->irq, level);
+}
+
 static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
 {
 SysBusDevice *d = SYS_BUS_DEVICE(dev);
@@ -70,6 +75,8 @@ static void ehci_sysbus_init(Object *obj)
 s->portnr = sec->portnr;
 s->as = &address_space_memory;
 
+s->intr_update = usb_ehci_sysbus_intr_update;
+
 usb_ehci_init(s, DEVICE(obj));
 sysbus_init_mmio(d, &s->mem);
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 01864d4649..e1f71dc445 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -209,7 +209,7 @@ static inline void ehci_update_irq(EHCIState *s)
 }
 
 trace_usb_ehci_irq(level, s->frindex, s->usbsts, s->usbintr);
-qemu_set_irq(s->irq, level);
+(s->intr_update)(s, level);
 }
 
 /* flag interrupt condition */
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 56a1c09d1f..bc017aec79 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -305,6 +305,8 @@ struct EHCIState {
 EHCIQueueHead aqueues;
 EHCIQueueHead pqueues;
 
+void (*intr_update)(EHCIState *s, bool enable);
+
 /* which address to look at next */
 uint32_t a_fetch_addr;
 uint32_t p_fetch_addr;
-- 
2.44.0




[PATCH] usb: add support for sending MSIs from EHCI PCI devices

2024-03-09 Thread Isaac Woods
This is a small patch that aims to add support for sending
message-signalled interrupts from EHCI controllers on PCI busses.
This was desirable for guests (in my case a hobbyist OS) that do not
have support for legacy PCI interrupt routing but still want to / have to
interface with EHCI controllers. I would guess most real EHCI
controllers would not bother to support MSI, but it doesn't seem
unreasonable to me for the QEMU one to when connected to a PCIe bus.

This is my first contribution to QEMU, and so was modelled entirely off
the XHCI controller's MSI support. I have tested it on RISC-V using the
PLIC, which correctly receives the traditionally-wired interrupt, and
using MSI, where the device is able to successfully dispatch an MSI
to a IMSIC.

This is also my first experience of `git send-mail` so please forgive
any errors with the patch email.

Signed-off-by: Isaac Woods 
---
 hw/usb/hcd-ehci-pci.c| 27 +++
 hw/usb/hcd-ehci-sysbus.c |  7 +++
 hw/usb/hcd-ehci.c|  2 +-
 hw/usb/hcd-ehci.h|  2 ++
 4 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 3ff54edf62..8a714b6733 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -21,6 +21,8 @@
 #include "migration/vmstate.h"
 #include "qemu/module.h"
 #include "qemu/range.h"
+#include "hw/pci/msi.h"
+#include "qapi/error.h"
 
 typedef struct EHCIPCIInfo {
 const char *name;
@@ -30,11 +32,27 @@ typedef struct EHCIPCIInfo {
 bool companion;
 } EHCIPCIInfo;
 
+static void ehci_pci_intr_update(EHCIState *ehci, bool enable)
+{
+EHCIPCIState *s = container_of(ehci, EHCIPCIState, ehci);
+PCIDevice *pci_dev = PCI_DEVICE(s);
+
+if (msi_enabled(pci_dev)) {
+if (enable) {
+msi_notify(pci_dev, 0);
+}
+} else {
+pci_set_irq(pci_dev, enable);
+}
+}
+
 static void usb_ehci_pci_realize(PCIDevice *dev, Error **errp)
 {
 EHCIPCIState *i = PCI_EHCI(dev);
 EHCIState *s = &i->ehci;
 uint8_t *pci_conf = dev->config;
+Error *err = NULL;
+int ret;
 
 pci_set_byte(&pci_conf[PCI_CLASS_PROG], 0x20);
 
@@ -68,8 +86,17 @@ static void usb_ehci_pci_realize(PCIDevice *dev, Error 
**errp)
 s->irq = pci_allocate_irq(dev);
 s->as = pci_get_address_space(dev);
 
+s->intr_update = ehci_pci_intr_update;
+
 usb_ehci_realize(s, DEVICE(dev), NULL);
 pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY, &s->mem);
+
+ret = msi_init(dev, 0x70, 1, true, false, &err);
+if (ret) {
+error_propagate(errp, err);
+} else {
+error_free(err);
+}
 }
 
 static void usb_ehci_pci_init(Object *obj)
diff --git a/hw/usb/hcd-ehci-sysbus.c b/hw/usb/hcd-ehci-sysbus.c
index fe1dabd0bb..e08c856e2b 100644
--- a/hw/usb/hcd-ehci-sysbus.c
+++ b/hw/usb/hcd-ehci-sysbus.c
@@ -38,6 +38,11 @@ static Property ehci_sysbus_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static void usb_ehci_sysbus_intr_update(EHCIState *ehci, bool level)
+{
+qemu_set_irq(s->irq, level);
+}
+
 static void usb_ehci_sysbus_realize(DeviceState *dev, Error **errp)
 {
 SysBusDevice *d = SYS_BUS_DEVICE(dev);
@@ -70,6 +75,8 @@ static void ehci_sysbus_init(Object *obj)
 s->portnr = sec->portnr;
 s->as = &address_space_memory;
 
+s->intr_update = usb_ehci_sysbus_intr_update;
+
 usb_ehci_init(s, DEVICE(obj));
 sysbus_init_mmio(d, &s->mem);
 }
diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c
index 01864d4649..e1f71dc445 100644
--- a/hw/usb/hcd-ehci.c
+++ b/hw/usb/hcd-ehci.c
@@ -209,7 +209,7 @@ static inline void ehci_update_irq(EHCIState *s)
 }
 
 trace_usb_ehci_irq(level, s->frindex, s->usbsts, s->usbintr);
-qemu_set_irq(s->irq, level);
+(s->intr_update)(s, level);
 }
 
 /* flag interrupt condition */
diff --git a/hw/usb/hcd-ehci.h b/hw/usb/hcd-ehci.h
index 56a1c09d1f..bc017aec79 100644
--- a/hw/usb/hcd-ehci.h
+++ b/hw/usb/hcd-ehci.h
@@ -305,6 +305,8 @@ struct EHCIState {
 EHCIQueueHead aqueues;
 EHCIQueueHead pqueues;
 
+void (*intr_update)(EHCIState *s, bool enable);
+
 /* which address to look at next */
 uint32_t a_fetch_addr;
 uint32_t p_fetch_addr;
-- 
2.44.0




Re: [PATCH 0/2] Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"

2024-03-09 Thread Alex Williamson
On Mon, 26 Feb 2024 22:59:07 +0100
Bernhard Beschow  wrote:

> As reported by Volker [1], commit 6f6ad2b24582 "hw/i386/pc: Confine system
> flash handling to pc_sysfw" causes a regression when specifying the property
> `-M pflash0` in the PCI PC machines:
>   qemu-system-x86_64: Property 'pc-q35-9.0-machine.pflash0' not found
> Revert the commit for now until a solution is found.
> 
> Best regards,
> Bernhard
> 
> [1] 
> https://lore.kernel.org/qemu-devel/9e82a04b-f2c1-4e34-b4b6-46a0581b5...@t-online.de/
> 
> Bernhard Beschow (2):
>   Revert "hw/i386/pc_sysfw: Inline pc_system_flash_create() and remove
> it"
>   Revert "hw/i386/pc: Confine system flash handling to pc_sysfw"
> 
>  include/hw/i386/pc.h |  2 ++
>  hw/i386/pc.c |  1 +
>  hw/i386/pc_piix.c|  1 +
>  hw/i386/pc_sysfw.c   | 17 +
>  4 files changed, 17 insertions(+), 4 deletions(-)
> 

Please apply this, the original commits break my existing VMs.

Tested-by: Alex Williamson 




Re: [PULL 00/14] target-arm queue

2024-03-09 Thread Peter Maydell
On Fri, 8 Mar 2024 at 15:50, Peter Maydell  wrote:
>
> The following changes since commit 8f6330a807f2642dc2a3cdf33347aa28a4c00a87:
>
>   Merge tag 'pull-maintainer-updates-060324-1' of 
> https://gitlab.com/stsquad/qemu into staging (2024-03-06 16:56:20 +)
>
> are available in the Git repository at:
>
>   https://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20240308
>
> for you to fetch changes up to bbf6c6dbead82292a20951eb1204442a6b838de9:
>
>   target/arm: Move v7m-related code from cpu32.c into a separate file 
> (2024-03-08 14:45:03 +)
>
> 
> target-arm queue:
>  * Implement FEAT_ECV
>  * STM32L4x5: Implement GPIO device
>  * Fix 32-bit SMOPA
>  * Refactor v7m related code from cpu32.c into its own file
>  * hw/rtc/sun4v-rtc: Relicense to GPLv2-or-later

Applied, thanks.

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

-- PMM



Re: [PULL 00/12] Cocoa patches for 2024-03-05

2024-03-09 Thread Peter Maydell
On Tue, 5 Mar 2024 at 11:06, Philippe Mathieu-Daudé  wrote:
>
> The following changes since commit 52e7db443bd8d233acc3977bd150bdadb62db86c:
>
>   Merge tag 'hppa-latest-pull-request' of 
> https://github.com/hdeller/qemu-hppa into staging (2024-03-04 16:01:33 +)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/darwin-20240305
>
> for you to fetch changes up to 5576663208b7c31766c580520df506375d00103e:
>
>   ui/cocoa: Remove stretch_video flag (2024-03-05 12:04:41 +0100)
>
> 
> Darwin Cocoa patches:
>
> - Add 'zoom-interpolation' to smooth scaled display with 'zoom-to-fit' 
> (Carwyn)
> - Set clipsToBounds on macOS 14 to fix window clipping (David)
> - Use NSWindow's ability to resize (Akihiko)
>
> 


Applied, thanks.

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

-- PMM



Re: [PATCH v3] hw/m68k/mcf5208: add support for reset

2024-03-09 Thread Thomas Huth
Am Sat,  9 Mar 2024 10:34:59 +0100
schrieb Angelo Dureghello :

> Add reset support for mcf5208.
> 
> Signed-off-by: Angelo Dureghello 
> ---
>  hw/m68k/mcf5208.c | 44 ++--
>  1 file changed, 42 insertions(+), 2 deletions(-)

Reviewed-by: Thomas Huth 



  1   2   >