Re: [PATCH v6 4/5] qapi/monitor: only allow 'keep' SetPasswordAction for VNC and deprecate

[Markus Armbruster]

Stefan Reiter  writes:

> VNC only supports 'keep' here, enforce this via a seperate
> SetPasswordActionVnc enum and mark the option 'deprecated' (as it is
> useless with only one value possible).
>
> Suggested-by: Eric Blake 
> Signed-off-by: Stefan Reiter 

With the next patch squashed in:
Reviewed-by: Markus Armbruster 

Re: [PATCH v6 5/5] docs: add deprecation note about 'set_password' param 'connected'

[Markus Armbruster]

Stefan Reiter  writes:

> Signed-off-by: Stefan Reiter 
> ---
>
> Seperate patch since it read a bit unsure in the review, feel free to either
> drop or squash this.
>
>  docs/about/deprecated.rst | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 0bed6ecb1d..1ad08e57d2 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -228,6 +228,12 @@ Use the more generic commands ``block-export-add`` and 
> ``block-export-del``
>  instead.  As part of this deprecation, where ``nbd-server-add`` used a
>  single ``bitmap``, the new ``block-export-add`` uses a list of ``bitmaps``.
>  
> +``set_password`` argument ``connected`` for VNC protocol (since 6.2)
> +
> +
> +Only the value ``keep`` is and was ever supported for VNC. It is recommended 
> to
> +just drop the argument.
> +
>  System accelerators
>  ---

This is okay.  Possibly clearer:

   Only the value ``keep`` is and was ever supported for VNC.  The
   (useless) argument will be dropped in a future version of QEMU.

Either phrasing is
Reviewed-by: Markus Armbruster 

Re: [PATCH v8 39/78] target/riscv: rvv-1.0: whole register move instructions

[Alistair Francis]

On Fri, Oct 15, 2021 at 6:18 PM  wrote:
>
> From: Frank Chang 
>
> Add the following instructions:
>
> * vmv1r.v
> * vmv2r.v
> * vmv4r.v
> * vmv8r.v
>
> Signed-off-by: Frank Chang 

Acked-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn32.decode  |  4 
>  target/riscv/insn_trans/trans_rvv.c.inc | 25 +
>  2 files changed, 29 insertions(+)
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index ab5fdbf9be8..06a80763112 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -650,6 +650,10 @@ vrgatherei16_vv 001110 . . . 000 . 1010111 
> @r_vm
>  vrgather_vx 001100 . . . 100 . 1010111 @r_vm
>  vrgather_vi 001100 . . . 011 . 1010111 @r_vm
>  vcompress_vm010111 - . . 010 . 1010111 @r
> +vmv1r_v 100111 1 . 0 011 . 1010111 @r2rd
> +vmv2r_v 100111 1 . 1 011 . 1010111 @r2rd
> +vmv4r_v 100111 1 . 00011 011 . 1010111 @r2rd
> +vmv8r_v 100111 1 . 00111 011 . 1010111 @r2rd
>
>  vsetvli 0 ... . 111 . 1010111  @r2_zimm
>  vsetvl  100 . . 111 . 1010111  @r
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index aec0316fba4..5eaf978fe90 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3258,3 +3258,28 @@ static bool trans_vcompress_vm(DisasContext *s, arg_r 
> *a)
>  }
>  return false;
>  }
> +
> +/*
> + * Whole Vector Register Move Instructions ignore vtype and vl setting.
> + * Thus, we don't need to check vill bit. (Section 16.6)
> + */
> +#define GEN_VMV_WHOLE_TRANS(NAME, LEN)  \
> +static bool trans_##NAME(DisasContext *s, arg_##NAME * a)   \
> +{   \
> +if (require_rvv(s) &&   \
> +QEMU_IS_ALIGNED(a->rd, LEN) &&  \
> +QEMU_IS_ALIGNED(a->rs2, LEN)) { \
> +/* EEW = 8 */   \
> +tcg_gen_gvec_mov(MO_8, vreg_ofs(s, a->rd),  \
> + vreg_ofs(s, a->rs2),   \
> + s->vlen / 8 * LEN, s->vlen / 8 * LEN); \
> +mark_vs_dirty(s);   \
> +return true;\
> +}   \
> +return false;   \
> +}
> +
> +GEN_VMV_WHOLE_TRANS(vmv1r_v, 1)
> +GEN_VMV_WHOLE_TRANS(vmv2r_v, 2)
> +GEN_VMV_WHOLE_TRANS(vmv4r_v, 4)
> +GEN_VMV_WHOLE_TRANS(vmv8r_v, 8)
> --
> 2.25.1
>
>

Re: [PATCH v6 3/5] qapi/monitor: allow VNC display id in set/expire_password

[Markus Armbruster]

Stefan Reiter  writes:

> It is possible to specify more than one VNC server on the command line,
> either with an explicit ID or the auto-generated ones à la "default",
> "vnc2", "vnc3", ...
>
> It is not possible to change the password on one of these extra VNC
> displays though. Fix this by adding a "display" parameter to the
> "set_password" and "expire_password" QMP and HMP commands.
>
> For HMP, the display is specified using the "-d" value flag.
>
> For QMP, the schema is updated to explicitly express the supported
> variants of the commands with protocol-discriminated unions.
>
> Suggested-by: Markus Armbruster 
> Signed-off-by: Stefan Reiter 
> ---
>  hmp-commands.hx|  24 +-
>  monitor/hmp-cmds.c |  51 +++--
>  monitor/qmp-cmds.c |  36 ++-
>  qapi/ui.json   | 112 +++--
>  4 files changed, 154 insertions(+), 69 deletions(-)
>
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index cf723c69ac..9fbb207b35 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1514,33 +1514,35 @@ ERST
>  
>  {
>  .name   = "set_password",
> -.args_type  = "protocol:s,password:s,connected:s?",
> -.params = "protocol password action-if-connected",
> +.args_type  = "protocol:s,password:s,display:-dV,connected:s?",
> +.params = "protocol password [-d display] [action-if-connected]",
>  .help   = "set spice/vnc password",
>  .cmd= hmp_set_password,
>  },
>  
>  SRST
> -``set_password [ vnc | spice ] password [ action-if-connected ]``
> -  Change spice/vnc password.  *action-if-connected* specifies what
> -  should happen in case a connection is established: *fail* makes the
> -  password change fail.  *disconnect* changes the password and
> +``set_password [ vnc | spice ] password [ -d display ] [ action-if-connected 
> ]``
> +  Change spice/vnc password.  *display* can be used with 'vnc' to specify
> +  which display to set the password on.  *action-if-connected* specifies
> +  what should happen in case a connection is established: *fail* makes
> +  the password change fail.  *disconnect* changes the password and
>disconnects the client.  *keep* changes the password and keeps the
>connection up.  *keep* is the default.
>  ERST
>  
>  {
>  .name   = "expire_password",
> -.args_type  = "protocol:s,time:s",
> -.params = "protocol time",
> +.args_type  = "protocol:s,time:s,display:-dV",
> +.params = "protocol time [-d display]",
>  .help   = "set spice/vnc password expire-time",
>  .cmd= hmp_expire_password,
>  },
>  
>  SRST
> -``expire_password [ vnc | spice ]`` *expire-time*
> -  Specify when a password for spice/vnc becomes
> -  invalid. *expire-time* accepts:
> +``expire_password [ vnc | spice ] expire-time [ -d display ]``
> +  Specify when a password for spice/vnc becomes invalid.
> +  *display* behaves the same as in ``set_password``.
> +  *expire-time* accepts:
>  
>``now``
>  Invalidate password instantly.
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index b8abe69609..daa4a8e106 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1451,26 +1451,39 @@ void hmp_set_password(Monitor *mon, const QDict 
> *qdict)
>  {
>  const char *protocol  = qdict_get_str(qdict, "protocol");
>  const char *password  = qdict_get_str(qdict, "password");
> +const char *display = qdict_get_try_str(qdict, "display");
>  const char *connected = qdict_get_try_str(qdict, "connected");
>  Error *err = NULL;
> -DisplayProtocol proto;
> -SetPasswordAction conn;
>  
> -proto = qapi_enum_parse(_lookup, protocol,
> -DISPLAY_PROTOCOL_VNC, );
> +SetPasswordOptions opts = {
> +.password = g_strdup(password),
> +.u.vnc.display = NULL,
> +};
> +
> +opts.protocol = qapi_enum_parse(_lookup, protocol,
> +DISPLAY_PROTOCOL_VNC, );
>  if (err) {
>  goto out;
>  }
>  
> -conn = qapi_enum_parse(_lookup, connected,
> -   SET_PASSWORD_ACTION_KEEP, );
> -if (err) {
> -goto out;
> +if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
> +opts.u.vnc.has_display = !!display;
> +opts.u.vnc.display = g_strdup(display);
> +} else if (opts.protocol == DISPLAY_PROTOCOL_SPICE) {
> +opts.u.spice.has_connected = !!connected;
> +opts.u.spice.connected =
> +qapi_enum_parse(_lookup, connected,
> +SET_PASSWORD_ACTION_KEEP, );
> +if (err) {
> +goto out;
> +}
>  }
>  
> -qmp_set_password(proto, password, !!connected, conn, );
> +qmp_set_password(, );
>  
>  out:
> +g_free(opts.password);
> +g_free(opts.u.vnc.display);

Uh-oh...

For DISPLAY_PROTOCOL_SPICE, we execute

   

Re: [PATCH v8 38/78] target/riscv: rvv-1.0: floating-point scalar move instructions

[Alistair Francis]

On Fri, Oct 15, 2021 at 6:15 PM  wrote:
>
> From: Frank Chang 
>
> NaN-boxed the scalar floating-point register based on RVV 1.0's rules.
>
> Signed-off-by: Frank Chang 

Acked-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn32.decode  |  4 +--
>  target/riscv/insn_trans/trans_rvv.c.inc | 38 -
>  target/riscv/internals.h|  5 
>  3 files changed, 21 insertions(+), 26 deletions(-)
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index e33ec82fdf8..ab5fdbf9be8 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -637,8 +637,8 @@ vid_v   010100 . 0 10001 010 . 1010111 
> @r1_vm
>  vmv_x_s 01 1 . 0 010 . 1010111 @r2rd
>  vmv_s_x 01 1 0 . 110 . 1010111 @r2
>  vext_x_v001100 1 . . 010 . 1010111 @r
> -vfmv_f_s001100 1 . 0 001 . 1010111 @r2rd
> -vfmv_s_f001101 1 0 . 101 . 1010111 @r2
> +vfmv_f_s01 1 . 0 001 . 1010111 @r2rd
> +vfmv_s_f01 1 0 . 101 . 1010111 @r2
>  vslideup_vx 001110 . . . 100 . 1010111 @r_vm
>  vslideup_vi 001110 . . . 011 . 1010111 @r_vm
>  vslide1up_vx001110 . . . 110 . 1010111 @r_vm
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 1340ce56806..aec0316fba4 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -3046,14 +3046,19 @@ static bool trans_vmv_s_x(DisasContext *s, 
> arg_vmv_s_x *a)
>  /* Floating-Point Scalar Move Instructions */
>  static bool trans_vfmv_f_s(DisasContext *s, arg_vfmv_f_s *a)
>  {
> -if (!s->vill && has_ext(s, RVF) &&
> -(s->mstatus_fs != 0) && (s->sew != 0)) {
> -unsigned int len = 8 << s->sew;
> +if (require_rvv(s) &&
> +require_rvf(s) &&
> +vext_check_isa_ill(s)) {
> +unsigned int ofs = (8 << s->sew);
> +unsigned int len = 64 - ofs;
> +TCGv_i64 t_nan;
>
>  vec_element_loadi(s, cpu_fpr[a->rd], a->rs2, 0, false);
> -if (len < 64) {
> -tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> -MAKE_64BIT_MASK(len, 64 - len));
> +/* NaN-box f[rd] as necessary for SEW */
> +if (len) {
> +t_nan = tcg_constant_i64(UINT64_MAX);
> +tcg_gen_deposit_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
> +t_nan, ofs, len);
>  }
>
>  mark_fs_dirty(s);
> @@ -3065,25 +3070,20 @@ static bool trans_vfmv_f_s(DisasContext *s, 
> arg_vfmv_f_s *a)
>  /* vfmv.s.f vd, rs1 # vd[0] = rs1 (vs2=0) */
>  static bool trans_vfmv_s_f(DisasContext *s, arg_vfmv_s_f *a)
>  {
> -if (!s->vill && has_ext(s, RVF) && (s->sew != 0)) {
> -TCGv_i64 t1;
> +if (require_rvv(s) &&
> +require_rvf(s) &&
> +vext_check_isa_ill(s)) {
>  /* The instructions ignore LMUL and vector register group. */
> -uint32_t vlmax = s->vlen >> 3;
> +TCGv_i64 t1;
> +TCGLabel *over = gen_new_label();
>
>  /* if vl == 0, skip vector register write back */
> -TCGLabel *over = gen_new_label();
>  tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
>
> -/* zeroed all elements */
> -tcg_gen_gvec_dup_imm(SEW64, vreg_ofs(s, a->rd), vlmax, vlmax, 0);
> -
> -/* NaN-box f[rs1] as necessary for SEW */
> +/* NaN-box f[rs1] */
>  t1 = tcg_temp_new_i64();
> -if (s->sew == MO_64 && !has_ext(s, RVD)) {
> -tcg_gen_ori_i64(t1, cpu_fpr[a->rs1], MAKE_64BIT_MASK(32, 32));
> -} else {
> -tcg_gen_mov_i64(t1, cpu_fpr[a->rs1]);
> -}
> +do_nanbox(s, t1, cpu_fpr[a->rs1]);
> +
>  vec_element_storei(s, a->rd, 0, t1);
>  tcg_temp_free_i64(t1);
>  mark_vs_dirty(s);
> diff --git a/target/riscv/internals.h b/target/riscv/internals.h
> index 81f5dfa477a..ac062dc0b4e 100644
> --- a/target/riscv/internals.h
> +++ b/target/riscv/internals.h
> @@ -32,11 +32,6 @@ target_ulong fclass_h(uint64_t frs1);
>  target_ulong fclass_s(uint64_t frs1);
>  target_ulong fclass_d(uint64_t frs1);
>
> -#define SEW8  0
> -#define SEW16 1
> -#define SEW32 2
> -#define SEW64 3
> -
>  #ifndef CONFIG_USER_ONLY
>  extern const VMStateDescription vmstate_riscv_cpu;
>  #endif
> --
> 2.25.1
>
>

Re: [PATCH v8 36/78] target/riscv: rvv-1.0: integer scalar move instructions

[Alistair Francis]

On Fri, Oct 15, 2021 at 6:13 PM  wrote:
>
> From: Frank Chang 
>
> * Remove "vmv.s.x: dothing if rs1 == 0" constraint.
> * Add vmv.x.s instruction.
>
> Signed-off-by: Frank Chang 
> Reviewed-by: Richard Henderson 

Acked-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn32.decode  |  3 +-
>  target/riscv/insn_trans/trans_rvv.c.inc | 43 -
>  2 files changed, 37 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
> index 4653a9679ef..e33ec82fdf8 100644
> --- a/target/riscv/insn32.decode
> +++ b/target/riscv/insn32.decode
> @@ -634,8 +634,9 @@ vmsif_m 010100 . . 00011 010 . 1010111 
> @r2_vm
>  vmsof_m 010100 . . 00010 010 . 1010111 @r2_vm
>  viota_m 010100 . . 1 010 . 1010111 @r2_vm
>  vid_v   010100 . 0 10001 010 . 1010111 @r1_vm
> +vmv_x_s 01 1 . 0 010 . 1010111 @r2rd
> +vmv_s_x 01 1 0 . 110 . 1010111 @r2
>  vext_x_v001100 1 . . 010 . 1010111 @r
> -vmv_s_x 001101 1 0 . 110 . 1010111 @r2
>  vfmv_f_s001100 1 . 0 001 . 1010111 @r2rd
>  vfmv_s_f001101 1 0 . 101 . 1010111 @r2
>  vslideup_vx 001110 . . . 100 . 1010111 @r_vm
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 43da36f4200..42a9f2764d5 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -2977,27 +2977,54 @@ static void vec_element_storei(DisasContext *s, int 
> vreg,
>  store_element(val, cpu_env, endian_ofs(s, vreg, idx), s->sew);
>  }
>
> +/* vmv.x.s rd, vs2 # x[rd] = vs2[0] */
> +static bool trans_vmv_x_s(DisasContext *s, arg_vmv_x_s *a)
> +{
> +if (require_rvv(s) &&
> +vext_check_isa_ill(s)) {
> +TCGv_i64 t1;
> +TCGv dest;
> +
> +t1 = tcg_temp_new_i64();
> +dest = tcg_temp_new();
> +/*
> + * load vreg and sign-extend to 64 bits,
> + * then truncate to XLEN bits before storing to gpr.
> + */
> +vec_element_loadi(s, t1, a->rs2, 0, true);
> +tcg_gen_trunc_i64_tl(dest, t1);
> +gen_set_gpr(s, a->rd, dest);
> +tcg_temp_free_i64(t1);
> +tcg_temp_free(dest);
> +
> +return true;
> +}
> +return false;
> +}
> +
>  /* vmv.s.x vd, rs1 # vd[0] = rs1 */
>  static bool trans_vmv_s_x(DisasContext *s, arg_vmv_s_x *a)
>  {
> -if (vext_check_isa_ill(s)) {
> +if (require_rvv(s) &&
> +vext_check_isa_ill(s)) {
>  /* This instruction ignores LMUL and vector register groups */
> -int maxsz = s->vlen >> 3;
>  TCGv_i64 t1;
> +TCGv s1;
>  TCGLabel *over = gen_new_label();
>
>  tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
> -tcg_gen_gvec_dup_imm(SEW64, vreg_ofs(s, a->rd), maxsz, maxsz, 0);
> -if (a->rs1 == 0) {
> -goto done;
> -}
>
>  t1 = tcg_temp_new_i64();
> -tcg_gen_extu_tl_i64(t1, cpu_gpr[a->rs1]);
> +
> +/*
> + * load gpr and sign-extend to 64 bits,
> + * then truncate to SEW bits when storing to vreg.
> + */
> +s1 = get_gpr(s, a->rs1, EXT_NONE);
> +tcg_gen_ext_tl_i64(t1, s1);
>  vec_element_storei(s, a->rd, 0, t1);
>  tcg_temp_free_i64(t1);
>  mark_vs_dirty(s);
> -done:
>  gen_set_label(over);
>  return true;
>  }
> --
> 2.25.1
>
>

[PULL 24/25] target/ppc: adding user read/write functions for PMCs

[David Gibson]

From: Daniel Henrique Barboza 

Problem state needs to be able to read and write the PMU counters,
otherwise it won't be aware of any sampling result that the PMU produces
after a Perf run.

This patch does that in a similar fashion as already done in the
previous patches. PMCs 5 and 6 have a special condition, aside from the
constraints that are common with PMCs 1-4, where they are not part of the
PMU if MMCR0_PMCC is 0b11.

Signed-off-by: Daniel Henrique Barboza 
Message-Id: <20211018010133.315842-5-danielhb...@gmail.com>
Signed-off-by: David Gibson 
---
 target/ppc/cpu_init.c| 12 +++---
 target/ppc/power8-pmu-regs.c.inc | 70 
 target/ppc/spr_tcg.h |  4 ++
 3 files changed, 80 insertions(+), 6 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index ad88e54950..65545ba9ca 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6879,27 +6879,27 @@ static void register_book3s_pmu_user_sprs(CPUPPCState 
*env)
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UPMC1, "UPMC1",
- _read_ureg, SPR_NOACCESS,
+ _read_PMC14_ureg, _write_PMC14_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UPMC2, "UPMC2",
- _read_ureg, SPR_NOACCESS,
+ _read_PMC14_ureg, _write_PMC14_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UPMC3, "UPMC3",
- _read_ureg, SPR_NOACCESS,
+ _read_PMC14_ureg, _write_PMC14_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UPMC4, "UPMC4",
- _read_ureg, SPR_NOACCESS,
+ _read_PMC14_ureg, _write_PMC14_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UPMC5, "UPMC5",
- _read_ureg, SPR_NOACCESS,
+ _read_PMC56_ureg, _write_PMC56_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UPMC6, "UPMC6",
- _read_ureg, SPR_NOACCESS,
+ _read_PMC56_ureg, _write_PMC56_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_USIAR, "USIAR",
diff --git a/target/ppc/power8-pmu-regs.c.inc b/target/ppc/power8-pmu-regs.c.inc
index fb95175183..7391851238 100644
--- a/target/ppc/power8-pmu-regs.c.inc
+++ b/target/ppc/power8-pmu-regs.c.inc
@@ -169,6 +169,56 @@ void spr_write_MMCR2_ureg(DisasContext *ctx, int sprn, int 
gprn)
 
 tcg_temp_free(masked_gprn);
 }
+
+void spr_read_PMC14_ureg(DisasContext *ctx, int gprn, int sprn)
+{
+if (!spr_groupA_read_allowed(ctx)) {
+return;
+}
+
+spr_read_ureg(ctx, gprn, sprn);
+}
+
+void spr_read_PMC56_ureg(DisasContext *ctx, int gprn, int sprn)
+{
+/*
+ * If PMCC = 0b11, PMC5 and PMC6 aren't included in the Performance
+ * Monitor, and a read attempt results in a Facility Unavailable
+ * Interrupt.
+ */
+if (ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
+gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+return;
+}
+
+/* The remaining steps are similar to PMCs 1-4 userspace read */
+spr_read_PMC14_ureg(ctx, gprn, sprn);
+}
+
+void spr_write_PMC14_ureg(DisasContext *ctx, int sprn, int gprn)
+{
+if (!spr_groupA_write_allowed(ctx)) {
+return;
+}
+
+spr_write_ureg(ctx, sprn, gprn);
+}
+
+void spr_write_PMC56_ureg(DisasContext *ctx, int sprn, int gprn)
+{
+/*
+ * If PMCC = 0b11, PMC5 and PMC6 aren't included in the Performance
+ * Monitor, and a write attempt results in a Facility Unavailable
+ * Interrupt.
+ */
+if (ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
+gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+return;
+}
+
+/* The remaining steps are similar to PMCs 1-4 userspace write */
+spr_write_PMC14_ureg(ctx, sprn, gprn);
+}
 #else
 void spr_read_MMCR0_ureg(DisasContext *ctx, int gprn, int sprn)
 {
@@ -189,4 +239,24 @@ void spr_write_MMCR2_ureg(DisasContext *ctx, int sprn, int 
gprn)
 {
 spr_noaccess(ctx, gprn, sprn);
 }
+
+void spr_read_PMC14_ureg(DisasContext *ctx, int gprn, int sprn)
+{
+spr_read_ureg(ctx, gprn, sprn);
+}
+
+void spr_read_PMC56_ureg(DisasContext *ctx, int gprn, int sprn)
+{
+spr_read_ureg(ctx, gprn, sprn);
+}
+
+void spr_write_PMC14_ureg(DisasContext *ctx, int sprn, int gprn)
+{
+spr_noaccess(ctx, gprn, sprn);
+}
+
+void spr_write_PMC56_ureg(DisasContext *ctx, int sprn, int gprn)
+{
+spr_noaccess(ctx, gprn, sprn);
+}
 #endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
index cb7f40eedf..520f1ef233 100644
--- a/target/ppc/spr_tcg.h
+++ b/target/ppc/spr_tcg.h
@@ 

[PULL 19/25] tests/acceptance: Add a test for the bamboo ppc board

[David Gibson]

From: Thomas Huth 

The kernel and initrd from the "Aboriginal Linux" project can be
used to run some tests on the bamboo ppc machine.

Signed-off-by: Thomas Huth 
Message-Id: <20211015090008.1299609-1-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
Signed-off-by: David Gibson 
---
 MAINTAINERS|  1 +
 tests/acceptance/ppc_bamboo.py | 39 ++
 2 files changed, 40 insertions(+)
 create mode 100644 tests/acceptance/ppc_bamboo.py

diff --git a/MAINTAINERS b/MAINTAINERS
index 9e9f489a41..4e77d03651 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1245,6 +1245,7 @@ Bamboo
 L: qemu-...@nongnu.org
 S: Orphan
 F: hw/ppc/ppc440_bamboo.c
+F: tests/acceptance/ppc_bamboo.py
 
 e500
 L: qemu-...@nongnu.org
diff --git a/tests/acceptance/ppc_bamboo.py b/tests/acceptance/ppc_bamboo.py
new file mode 100644
index 00..dd33bf66f3
--- /dev/null
+++ b/tests/acceptance/ppc_bamboo.py
@@ -0,0 +1,39 @@
+# Test that Linux kernel boots on the ppc bamboo board and check the console
+#
+# Copyright (c) 2021 Red Hat
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado.utils import archive
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
+from avocado_qemu import exec_command_and_wait_for_pattern
+
+class BambooMachine(Test):
+
+timeout = 90
+
+def test_ppc_bamboo(self):
+"""
+:avocado: tags=arch:ppc
+:avocado: tags=machine:bamboo
+:avocado: tags=cpu:440epb
+:avocado: tags=device:rtl8139
+"""
+tar_url = ('http://landley.net/aboriginal/downloads/binaries/'
+   'system-image-powerpc-440fp.tar.gz')
+tar_hash = '53e5f16414b195b82d2c70272f81c2eedb39bad9'
+file_path = self.fetch_asset(tar_url, asset_hash=tar_hash)
+archive.extract(file_path, self.workdir)
+self.vm.set_console()
+self.vm.add_args('-kernel', self.workdir +
+   '/system-image-powerpc-440fp/linux',
+ '-initrd', self.workdir +
+   
'/system-image-powerpc-440fp/rootfs.cpio.gz',
+ '-nic', 'user,model=rtl8139,restrict=on')
+self.vm.launch()
+wait_for_console_pattern(self, 'Type exit when done')
+exec_command_and_wait_for_pattern(self, 'ping 10.0.2.2',
+  '10.0.2.2 is alive!')
+exec_command_and_wait_for_pattern(self, 'halt', 'System Halted')
-- 
2.31.1

[PULL 25/25] hw/ppc/ppc4xx_pci: Fix ppc4xx_pci_map_irq() for recent Linux kernels

[David Gibson]

From: Thomas Huth 

Recent Linux kernels are accessing the PCI device in slot 0 that
represents the PCI host bridge. This causes ppc4xx_pci_map_irq()
to return -1 which causes an assert() later:

 hw/pci/pci.c:262: pci_bus_change_irq_level: Assertion `irq_num >= 0' failed.

Thus we should allocate an IRQ line for the device in slot 0, too.
To avoid changes to the outside of ppc4xx_pci.c, we map it to
the internal IRQ number 4 which will then happily be ignored since
ppc440_bamboo.c does not wire it up.

With these changes it is now possible again to use recent Linux
kernels for the bamboo board.

Signed-off-by: Thomas Huth 
Message-Id: <20211019091817.469003-1-th...@redhat.com>
Reviewed-by: Cédric Le Goater 
Tested-by: Cédric Le Goater 
Signed-off-by: David Gibson 
---
 hw/ppc/ppc4xx_pci.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 8147ba6f94..304a29349c 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -48,12 +48,14 @@ OBJECT_DECLARE_SIMPLE_TYPE(PPC4xxPCIState, 
PPC4xx_PCI_HOST_BRIDGE)
 #define PPC4xx_PCI_NR_PMMS 3
 #define PPC4xx_PCI_NR_PTMS 2
 
+#define PPC4xx_PCI_NUM_DEVS 5
+
 struct PPC4xxPCIState {
 PCIHostState parent_obj;
 
 struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
 struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
-qemu_irq irq[PCI_NUM_PINS];
+qemu_irq irq[PPC4xx_PCI_NUM_DEVS];
 
 MemoryRegion container;
 MemoryRegion iomem;
@@ -246,7 +248,7 @@ static int ppc4xx_pci_map_irq(PCIDevice *pci_dev, int 
irq_num)
 
 trace_ppc4xx_pci_map_irq(pci_dev->devfn, irq_num, slot);
 
-return slot - 1;
+return slot > 0 ? slot - 1 : PPC4xx_PCI_NUM_DEVS - 1;
 }
 
 static void ppc4xx_pci_set_irq(void *opaque, int irq_num, int level)
@@ -254,7 +256,7 @@ static void ppc4xx_pci_set_irq(void *opaque, int irq_num, 
int level)
 qemu_irq *pci_irqs = opaque;
 
 trace_ppc4xx_pci_set_irq(irq_num);
-assert(irq_num >= 0);
+assert(irq_num >= 0 && irq_num < PPC4xx_PCI_NUM_DEVS);
 qemu_set_irq(pci_irqs[irq_num], level);
 }
 
-- 
2.31.1

[PULL 18/25] ppc/pegasos2: Implement power-off RTAS function with VOF

[David Gibson]

From: BALATON Zoltan 

This only helps Linux guests as only that seems to use it.

Signed-off-by: BALATON Zoltan 
Message-Id: 
<1c1e030f2bbc86e950b3310fb5922facdc21ef86.1634241019.git.bala...@eik.bme.hu>
Signed-off-by: David Gibson 
---
 hw/ppc/pegasos2.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 39e96d323f..e427ac2fe0 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -22,6 +22,7 @@
 #include "hw/i2c/smbus_eeprom.h"
 #include "hw/qdev-properties.h"
 #include "sysemu/reset.h"
+#include "sysemu/runstate.h"
 #include "hw/boards.h"
 #include "hw/loader.h"
 #include "hw/fw-path-provider.h"
@@ -429,6 +430,16 @@ static target_ulong pegasos2_rtas(PowerPCCPU *cpu, 
Pegasos2MachineState *pm,
 qemu_log_mask(LOG_UNIMP, "%c", ldl_be_phys(as, args));
 stl_be_phys(as, rets, 0);
 return H_SUCCESS;
+case RTAS_POWER_OFF:
+{
+if (nargs != 2 || nrets != 1) {
+stl_be_phys(as, rets, -1);
+return H_PARAMETER;
+}
+qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+stl_be_phys(as, rets, 0);
+return H_SUCCESS;
+}
 default:
 qemu_log_mask(LOG_UNIMP, "Unknown RTAS token %u (args=%u, rets=%u)\n",
   token, nargs, nrets);
-- 
2.31.1

[PULL 23/25] target/ppc: add user read/write functions for MMCR2

[David Gibson]

From: Daniel Henrique Barboza 

Similar to the previous patch, let's add problem state read/write access to
the MMCR2 SPR, which is also a group A PMU SPR that needs to be filtered
to be read/written by userspace.

Signed-off-by: Daniel Henrique Barboza 
Message-Id: <20211018010133.315842-4-danielhb...@gmail.com>
Signed-off-by: David Gibson 
---
 target/ppc/cpu.h |  9 +++
 target/ppc/cpu_init.c|  2 +-
 target/ppc/power8-pmu-regs.c.inc | 98 
 target/ppc/spr_tcg.h |  2 +
 4 files changed, 99 insertions(+), 12 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0bd008f4b8..0472ec9154 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -353,6 +353,15 @@ typedef struct ppc_v3_pate_t {
 #define MMCR0_PMCC1  PPC_BIT(45) /* PMC Control bit 1 */
 /* MMCR0 userspace r/w mask */
 #define MMCR0_UREG_MASK (MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE)
+/* MMCR2 userspace r/w mask */
+#define MMCR2_FC1P0  PPC_BIT(1)  /* MMCR2 FCnP0 for PMC1 */
+#define MMCR2_FC2P0  PPC_BIT(10) /* MMCR2 FCnP0 for PMC2 */
+#define MMCR2_FC3P0  PPC_BIT(19) /* MMCR2 FCnP0 for PMC3 */
+#define MMCR2_FC4P0  PPC_BIT(28) /* MMCR2 FCnP0 for PMC4 */
+#define MMCR2_FC5P0  PPC_BIT(37) /* MMCR2 FCnP0 for PMC5 */
+#define MMCR2_FC6P0  PPC_BIT(46) /* MMCR2 FCnP0 for PMC6 */
+#define MMCR2_UREG_MASK (MMCR2_FC1P0 | MMCR2_FC2P0 | MMCR2_FC3P0 | \
+ MMCR2_FC4P0 | MMCR2_FC5P0 | MMCR2_FC6P0)
 
 /* LPCR bits */
 #define LPCR_VPM0 PPC_BIT(0)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 375bdca1e1..ad88e54950 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6975,7 +6975,7 @@ static void register_power8_pmu_sup_sprs(CPUPPCState *env)
 static void register_power8_pmu_user_sprs(CPUPPCState *env)
 {
 spr_register(env, SPR_POWER_UMMCR2, "UMMCR2",
- _read_ureg, SPR_NOACCESS,
+ _read_MMCR2_ureg, _write_MMCR2_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_USIER, "USIER",
diff --git a/target/ppc/power8-pmu-regs.c.inc b/target/ppc/power8-pmu-regs.c.inc
index 37c812dd4d..fb95175183 100644
--- a/target/ppc/power8-pmu-regs.c.inc
+++ b/target/ppc/power8-pmu-regs.c.inc
@@ -55,6 +55,33 @@ static bool spr_groupA_write_allowed(DisasContext *ctx)
 return false;
 }
 
+/*
+ * Helper function to avoid code repetition between MMCR0 and
+ * MMCR2 problem state write functions.
+ *
+ * 'ret' must be tcg_temp_freed() by the caller.
+ */
+static TCGv masked_gprn_for_spr_write(int gprn, int sprn,
+  uint64_t spr_mask)
+{
+TCGv ret = tcg_temp_new();
+TCGv t0 = tcg_temp_new();
+
+/* 'ret' starts with all mask bits cleared */
+gen_load_spr(ret, sprn);
+tcg_gen_andi_tl(ret, ret, ~(spr_mask));
+
+/* Apply the mask into 'gprn' in a temp var */
+tcg_gen_andi_tl(t0, cpu_gpr[gprn], spr_mask);
+
+/* Add the masked gprn bits into 'ret' */
+tcg_gen_or_tl(ret, ret, t0);
+
+tcg_temp_free(t0);
+
+return ret;
+}
+
 void spr_read_MMCR0_ureg(DisasContext *ctx, int gprn, int sprn)
 {
 TCGv t0;
@@ -79,29 +106,68 @@ void spr_read_MMCR0_ureg(DisasContext *ctx, int gprn, int 
sprn)
 
 void spr_write_MMCR0_ureg(DisasContext *ctx, int sprn, int gprn)
 {
-TCGv t0, t1;
+TCGv masked_gprn;
 
 if (!spr_groupA_write_allowed(ctx)) {
 return;
 }
 
-t0 = tcg_temp_new();
-t1 = tcg_temp_new();
-
 /*
  * Filter out all bits but FC, PMAO, and PMAE, according
  * to ISA v3.1, in 10.4.4 Monitor Mode Control Register 0,
  * fourth paragraph.
  */
-tcg_gen_andi_tl(t0, cpu_gpr[gprn], MMCR0_UREG_MASK);
-gen_load_spr(t1, SPR_POWER_MMCR0);
-tcg_gen_andi_tl(t1, t1, ~(MMCR0_UREG_MASK));
-/* Keep all other bits intact */
-tcg_gen_or_tl(t1, t1, t0);
-gen_store_spr(SPR_POWER_MMCR0, t1);
+masked_gprn = masked_gprn_for_spr_write(gprn, SPR_POWER_MMCR0,
+MMCR0_UREG_MASK);
+gen_store_spr(SPR_POWER_MMCR0, masked_gprn);
+
+tcg_temp_free(masked_gprn);
+}
+
+void spr_read_MMCR2_ureg(DisasContext *ctx, int gprn, int sprn)
+{
+TCGv t0;
+
+if (!spr_groupA_read_allowed(ctx)) {
+return;
+}
+
+t0 = tcg_temp_new();
+
+/*
+ * On read, filter out all bits that are not FCnP0 bits.
+ * When MMCR0[PMCC] is set to 0b10 or 0b11, providing
+ * problem state programs read/write access to MMCR2,
+ * only the FCnP0 bits can be accessed. All other bits are
+ * not changed when mtspr is executed in problem state, and
+ * all other bits return 0s when mfspr is executed in problem
+ * state, according to ISA v3.1, section 10.4.6 Monitor Mode
+ * Control Register 2, p. 1316, third paragraph.
+ */
+gen_load_spr(t0, SPR_POWER_MMCR2);
+tcg_gen_andi_tl(t0, t0, MMCR2_UREG_MASK);
+   

[PULL 16/25] ppc/pegasos2: Access MV64361 registers via their memory region

[David Gibson]

From: BALATON Zoltan 

Instead of relying on the mapped address of the MV64361 registers
access them via their memory region. This is not a problem at reset
time when these registers are mapped at the default address but the
guest could change this later and then the RTAS calls accessing PCI
config registers could fail. None of the guests actually do this so
this only avoids a theoretical problem not seen in practice.

Signed-off-by: BALATON Zoltan 
Message-Id: 

Signed-off-by: David Gibson 
---
 hw/pci-host/mv64361.c |   1 +
 hw/ppc/pegasos2.c | 117 --
 2 files changed, 56 insertions(+), 62 deletions(-)

diff --git a/hw/pci-host/mv64361.c b/hw/pci-host/mv64361.c
index 92b0f5d047..00b3ff7d90 100644
--- a/hw/pci-host/mv64361.c
+++ b/hw/pci-host/mv64361.c
@@ -869,6 +869,7 @@ static void mv64361_realize(DeviceState *dev, Error **errp)
 s->base_addr_enable = 0x1f;
 memory_region_init_io(>regs, OBJECT(s), _ops, s,
   TYPE_MV64361, 0x1);
+sysbus_init_mmio(SYS_BUS_DEVICE(dev), >regs);
 for (i = 0; i < 2; i++) {
 g_autofree char *name = g_strdup_printf("pcihost%d", i);
 object_initialize_child(OBJECT(dev), name, >pci[i],
diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index a9e3625f56..a861bf16b8 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -205,56 +205,49 @@ static void pegasos2_init(MachineState *machine)
 }
 }
 
-static uint32_t pegasos2_pci_config_read(AddressSpace *as, int bus,
+static uint32_t pegasos2_mv_reg_read(Pegasos2MachineState *pm,
+ uint32_t addr, uint32_t len)
+{
+MemoryRegion *r = sysbus_mmio_get_region(SYS_BUS_DEVICE(pm->mv), 0);
+uint64_t val = 0xULL;
+memory_region_dispatch_read(r, addr, , size_memop(len) | MO_LE,
+MEMTXATTRS_UNSPECIFIED);
+return val;
+}
+
+static void pegasos2_mv_reg_write(Pegasos2MachineState *pm, uint32_t addr,
+  uint32_t len, uint32_t val)
+{
+MemoryRegion *r = sysbus_mmio_get_region(SYS_BUS_DEVICE(pm->mv), 0);
+memory_region_dispatch_write(r, addr, val, size_memop(len) | MO_LE,
+ MEMTXATTRS_UNSPECIFIED);
+}
+
+static uint32_t pegasos2_pci_config_read(Pegasos2MachineState *pm, int bus,
  uint32_t addr, uint32_t len)
 {
-hwaddr pcicfg = (bus ? 0xf1000c78 : 0xf1000cf8);
-uint32_t val = 0x;
-
-stl_le_phys(as, pcicfg, addr | BIT(31));
-switch (len) {
-case 4:
-val = ldl_le_phys(as, pcicfg + 4);
-break;
-case 2:
-val = lduw_le_phys(as, pcicfg + 4);
-break;
-case 1:
-val = ldub_phys(as, pcicfg + 4);
-break;
-default:
-qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid length\n", __func__);
-break;
+hwaddr pcicfg = bus ? 0xc78 : 0xcf8;
+uint64_t val = 0xULL;
+
+if (len <= 4) {
+pegasos2_mv_reg_write(pm, pcicfg, 4, addr | BIT(31));
+val = pegasos2_mv_reg_read(pm, pcicfg + 4, len);
 }
 return val;
 }
 
-static void pegasos2_pci_config_write(AddressSpace *as, int bus, uint32_t addr,
-  uint32_t len, uint32_t val)
+static void pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
+  uint32_t addr, uint32_t len, uint32_t 
val)
 {
-hwaddr pcicfg = (bus ? 0xf1000c78 : 0xf1000cf8);
-
-stl_le_phys(as, pcicfg, addr | BIT(31));
-switch (len) {
-case 4:
-stl_le_phys(as, pcicfg + 4, val);
-break;
-case 2:
-stw_le_phys(as, pcicfg + 4, val);
-break;
-case 1:
-stb_phys(as, pcicfg + 4, val);
-break;
-default:
-qemu_log_mask(LOG_GUEST_ERROR, "%s: invalid length\n", __func__);
-break;
-}
+hwaddr pcicfg = bus ? 0xc78 : 0xcf8;
+
+pegasos2_mv_reg_write(pm, pcicfg, 4, addr | BIT(31));
+pegasos2_mv_reg_write(pm, pcicfg + 4, len, val);
 }
 
 static void pegasos2_machine_reset(MachineState *machine)
 {
 Pegasos2MachineState *pm = PEGASOS2_MACHINE(machine);
-AddressSpace *as = CPU(pm->cpu)->as;
 void *fdt;
 uint64_t d[2];
 int sz;
@@ -265,51 +258,51 @@ static void pegasos2_machine_reset(MachineState *machine)
 }
 
 /* Otherwise, set up devices that board firmware would normally do */
-stl_le_phys(as, 0xf100, 0x28020ff);
-stl_le_phys(as, 0xf1000278, 0xa31fc);
-stl_le_phys(as, 0xf100f300, 0x11ff0400);
-stl_le_phys(as, 0xf100f10c, 0x8000);
-stl_le_phys(as, 0xf11c, 0x800);
-pegasos2_pci_config_write(as, 0, PCI_COMMAND, 2, PCI_COMMAND_IO |
+pegasos2_mv_reg_write(pm, 0, 4, 0x28020ff);
+pegasos2_mv_reg_write(pm, 0x278, 4, 0xa31fc);
+pegasos2_mv_reg_write(pm, 0xf300, 4, 0x11ff0400);
+pegasos2_mv_reg_write(pm, 0xf10c, 4, 0x8000);
+pegasos2_mv_reg_write(pm, 0x1c, 4, 

[PULL 22/25] target/ppc: add user read/write functions for MMCR0

[David Gibson]

From: Gustavo Romero 

Userspace need access to PMU SPRs to be able to operate the PMU. One of
such SPRs is MMCR0.

MMCR0, as defined by PowerISA v3.1, is classified as a 'group A' PMU
register. This class of registers has common read/write rules that are
governed by MMCR0 PMCC bits. MMCR0 is also not fully exposed to problem
state: only MMCR0_FC, MMCR0_PMAO and MMCR0_PMAE bits are
readable/writable in this case.

This patch exposes MMCR0 to userspace by doing the following:

- two new callbacks, spr_read_MMCR0_ureg() and spr_write_MMCR0_ureg(),
are added to be used as problem state read/write callbacks of UMMCR0.
Both callbacks filters the amount of bits userspace is able to
read/write by using a MMCR0_UREG_MASK;

- problem state access control is done by the spr_groupA_read_allowed()
and spr_groupA_write_allowed() helpers. These helpers will read the
current PMCC bits from DisasContext and check whether the read/write
MMCR0 operation is valid or noti;

- to avoid putting exclusive PMU logic into the already loaded
translate.c file, let's create a new 'power8-pmu-regs.c.inc' file that
will hold all the spr_read/spr_write functions of PMU registers.

The 'power8' name of this new file intends to hint about the proven
support of the PMU logic to be added. The code has been tested with the
IBM POWER chip family, POWER8 being the oldest version tested. This
doesn't mean that the PMU logic will break with any other PPC64 chip
that implements Book3s, but rather that we can't assert that it works
properly with any Book3s compliant chip.

CC: Gustavo Romero 
Signed-off-by: Gustavo Romero 
Signed-off-by: Daniel Henrique Barboza 
Message-Id: <20211018010133.315842-3-danielhb...@gmail.com>
Signed-off-by: David Gibson 
---
 target/ppc/cpu.h |   7 ++
 target/ppc/cpu_init.c|   2 +-
 target/ppc/power8-pmu-regs.c.inc | 116 +++
 target/ppc/spr_tcg.h |   2 +
 target/ppc/translate.c   |   2 +
 5 files changed, 128 insertions(+), 1 deletion(-)
 create mode 100644 target/ppc/power8-pmu-regs.c.inc

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 24d1f2cf97..0bd008f4b8 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -344,8 +344,15 @@ typedef struct ppc_v3_pate_t {
 #define MSR_LE   0  /* Little-endian mode   1 hflags */
 
 /* PMU bits */
+#define MMCR0_FC PPC_BIT(32) /* Freeze Counters  */
+#define MMCR0_PMAO   PPC_BIT(56) /* Perf Monitor Alert Ocurred */
+#define MMCR0_PMAE   PPC_BIT(37) /* Perf Monitor Alert Enable */
+#define MMCR0_EBEPPC_BIT(43) /* Perf Monitor EBB Enable */
+#define MMCR0_FCECE  PPC_BIT(38) /* FC on Enabled Cond or Event */
 #define MMCR0_PMCC0  PPC_BIT(44) /* PMC Control bit 0 */
 #define MMCR0_PMCC1  PPC_BIT(45) /* PMC Control bit 1 */
+/* MMCR0 userspace r/w mask */
+#define MMCR0_UREG_MASK (MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE)
 
 /* LPCR bits */
 #define LPCR_VPM0 PPC_BIT(0)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 6aad01d1d3..375bdca1e1 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6867,7 +6867,7 @@ static void register_book3s_pmu_sup_sprs(CPUPPCState *env)
 static void register_book3s_pmu_user_sprs(CPUPPCState *env)
 {
 spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
- _read_ureg, SPR_NOACCESS,
+ _read_MMCR0_ureg, _write_MMCR0_ureg,
  _read_ureg, _write_ureg,
  0x);
 spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
diff --git a/target/ppc/power8-pmu-regs.c.inc b/target/ppc/power8-pmu-regs.c.inc
new file mode 100644
index 00..37c812dd4d
--- /dev/null
+++ b/target/ppc/power8-pmu-regs.c.inc
@@ -0,0 +1,116 @@
+/*
+ * PMU register read/write functions for TCG IBM POWER chips
+ *
+ * Copyright IBM Corp. 2021
+ *
+ * Authors:
+ *  Daniel Henrique Barboza  
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+
+/*
+ * Checks whether the Group A SPR (MMCR0, MMCR2, MMCRA, and the
+ * PMCs) has problem state read access.
+ *
+ * Read acccess is granted for all PMCC values but 0b01, where a
+ * Facility Unavailable Interrupt will occur.
+ */
+static bool spr_groupA_read_allowed(DisasContext *ctx)
+{
+if (!ctx->mmcr0_pmcc0 && ctx->mmcr0_pmcc1) {
+gen_hvpriv_exception(ctx, POWERPC_EXCP_FU);
+return false;
+}
+
+return true;
+}
+
+/*
+ * Checks whether the Group A SPR (MMCR0, MMCR2, MMCRA, and the
+ * PMCs) has problem state write access.
+ *
+ * Write acccess is granted for PMCC values 0b10 and 0b11. Userspace
+ * writing with PMCC 0b00 will generate a Hypervisor Emulation
+ * Assistance Interrupt. Userspace writing with PMCC 0b01 will
+ * generate a Facility Unavailable Interrupt.
+ */
+static bool 

Re: [PATCH v6 2/5] qapi/monitor: refactor set/expire_password with enums

[Markus Armbruster]

Stefan Reiter  writes:

> 'protocol' and 'connected' are better suited as enums than as strings,
> make use of that. No functional change intended.
>
> Suggested-by: Markus Armbruster 
> Signed-off-by: Stefan Reiter 

Reviewed-by: Markus Armbruster 

[PULL 17/25] ppc/pegasos2: Add constants for PCI config addresses

[David Gibson]

From: BALATON Zoltan 

Define a constant for PCI config addresses to make it clearer what
these numbers are.

Signed-off-by: BALATON Zoltan 
Message-Id: 
<9bd8e84d02d91693b71082a1fadeb86e6bce3025.1634241019.git.bala...@eik.bme.hu>
Signed-off-by: David Gibson 
---
 hw/ppc/pegasos2.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index a861bf16b8..39e96d323f 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -54,11 +54,13 @@
 
 #define BUS_FREQ_HZ 1
 
+#define PCI0_CFG_ADDR 0xcf8
 #define PCI0_MEM_BASE 0xc000
 #define PCI0_MEM_SIZE 0x2000
 #define PCI0_IO_BASE  0xf800
 #define PCI0_IO_SIZE  0x1
 
+#define PCI1_CFG_ADDR 0xc78
 #define PCI1_MEM_BASE 0x8000
 #define PCI1_MEM_SIZE 0x4000
 #define PCI1_IO_BASE  0xfe00
@@ -226,7 +228,7 @@ static void pegasos2_mv_reg_write(Pegasos2MachineState *pm, 
uint32_t addr,
 static uint32_t pegasos2_pci_config_read(Pegasos2MachineState *pm, int bus,
  uint32_t addr, uint32_t len)
 {
-hwaddr pcicfg = bus ? 0xc78 : 0xcf8;
+hwaddr pcicfg = bus ? PCI1_CFG_ADDR : PCI0_CFG_ADDR;
 uint64_t val = 0xULL;
 
 if (len <= 4) {
@@ -239,7 +241,7 @@ static uint32_t 
pegasos2_pci_config_read(Pegasos2MachineState *pm, int bus,
 static void pegasos2_pci_config_write(Pegasos2MachineState *pm, int bus,
   uint32_t addr, uint32_t len, uint32_t 
val)
 {
-hwaddr pcicfg = bus ? 0xc78 : 0xcf8;
+hwaddr pcicfg = bus ? PCI1_CFG_ADDR : PCI0_CFG_ADDR;
 
 pegasos2_mv_reg_write(pm, pcicfg, 4, addr | BIT(31));
 pegasos2_mv_reg_write(pm, pcicfg + 4, len, val);
-- 
2.31.1

Re: [PATCH v8 34/78] target/riscv: rvv-1.0: allow load element with sign-extended

[Alistair Francis]

On Fri, Oct 15, 2021 at 6:28 PM  wrote:
>
> From: Frank Chang 
>
> For some vector instructions (e.g. vmv.s.x), the element is loaded with
> sign-extended.
>
> Signed-off-by: Frank Chang 
> Reviewed-by: Richard Henderson 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/insn_trans/trans_rvv.c.inc | 32 +
>  1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
> b/target/riscv/insn_trans/trans_rvv.c.inc
> index 3751496676f..31ea231b3be 100644
> --- a/target/riscv/insn_trans/trans_rvv.c.inc
> +++ b/target/riscv/insn_trans/trans_rvv.c.inc
> @@ -2824,17 +2824,29 @@ static bool trans_vid_v(DisasContext *s, arg_vid_v *a)
>  /* Integer Extract Instruction */
>
>  static void load_element(TCGv_i64 dest, TCGv_ptr base,
> - int ofs, int sew)
> + int ofs, int sew, bool sign)
>  {
>  switch (sew) {
>  case MO_8:
> -tcg_gen_ld8u_i64(dest, base, ofs);
> +if (!sign) {
> +tcg_gen_ld8u_i64(dest, base, ofs);
> +} else {
> +tcg_gen_ld8s_i64(dest, base, ofs);
> +}
>  break;
>  case MO_16:
> -tcg_gen_ld16u_i64(dest, base, ofs);
> +if (!sign) {
> +tcg_gen_ld16u_i64(dest, base, ofs);
> +} else {
> +tcg_gen_ld16s_i64(dest, base, ofs);
> +}
>  break;
>  case MO_32:
> -tcg_gen_ld32u_i64(dest, base, ofs);
> +if (!sign) {
> +tcg_gen_ld32u_i64(dest, base, ofs);
> +} else {
> +tcg_gen_ld32s_i64(dest, base, ofs);
> +}
>  break;
>  case MO_64:
>  tcg_gen_ld_i64(dest, base, ofs);
> @@ -2889,7 +2901,7 @@ static void vec_element_loadx(DisasContext *s, TCGv_i64 
> dest,
>
>  /* Perform the load. */
>  load_element(dest, base,
> - vreg_ofs(s, vreg), s->sew);
> + vreg_ofs(s, vreg), s->sew, false);
>  tcg_temp_free_ptr(base);
>  tcg_temp_free_i32(ofs);
>
> @@ -2905,9 +2917,9 @@ static void vec_element_loadx(DisasContext *s, TCGv_i64 
> dest,
>  }
>
>  static void vec_element_loadi(DisasContext *s, TCGv_i64 dest,
> -  int vreg, int idx)
> +  int vreg, int idx, bool sign)
>  {
> -load_element(dest, cpu_env, endian_ofs(s, vreg, idx), s->sew);
> +load_element(dest, cpu_env, endian_ofs(s, vreg, idx), s->sew, sign);
>  }
>
>  static bool trans_vext_x_v(DisasContext *s, arg_r *a)
> @@ -2917,7 +2929,7 @@ static bool trans_vext_x_v(DisasContext *s, arg_r *a)
>
>  if (a->rs1 == 0) {
>  /* Special case vmv.x.s rd, vs2. */
> -vec_element_loadi(s, tmp, a->rs2, 0);
> +vec_element_loadi(s, tmp, a->rs2, 0, false);
>  } else {
>  /* This instruction ignores LMUL and vector register groups */
>  int vlmax = s->vlen >> (3 + s->sew);
> @@ -2999,7 +3011,7 @@ static bool trans_vfmv_f_s(DisasContext *s, 
> arg_vfmv_f_s *a)
>  (s->mstatus_fs != 0) && (s->sew != 0)) {
>  unsigned int len = 8 << s->sew;
>
> -vec_element_loadi(s, cpu_fpr[a->rd], a->rs2, 0);
> +vec_element_loadi(s, cpu_fpr[a->rd], a->rs2, 0, false);
>  if (len < 64) {
>  tcg_gen_ori_i64(cpu_fpr[a->rd], cpu_fpr[a->rd],
>  MAKE_64BIT_MASK(len, 64 - len));
> @@ -3101,7 +3113,7 @@ static bool trans_vrgather_vx(DisasContext *s, arg_rmrr 
> *a)
>  TCGv_i64 dest = tcg_temp_new_i64();
>
>  if (a->rs1 == 0) {
> -vec_element_loadi(s, dest, a->rs2, 0);
> +vec_element_loadi(s, dest, a->rs2, 0, false);
>  } else {
>  vec_element_loadx(s, dest, a->rs2, cpu_gpr[a->rs1], vlmax);
>  }
> --
> 2.25.1
>
>

[PULL 20/25] target/ppc: Filter mtmsr[d] input before setting MSR

[David Gibson]

From: Matheus Ferst 

PowerISA says that mtmsr[d] "does not alter MSR[HV], MSR[S], MSR[ME], or
MSR[LE]", but the current code only filters the GPR-provided value if
L=1. This behavior caused some problems in FreeBSD, and a build option
was added to work around the issue [1], but it seems that the bug was
not reported in launchpad/gitlab. This patch address the issue in qemu,
so the option on FreeBSD should no longer be required.

[1] 
https://cgit.freebsd.org/src/commit/?id=4efb1ca7d2a44cfb33d7f9e18bd92f8d68dcfee0

Signed-off-by: Matheus Ferst 
Message-Id: <20211015181940.197982-1-matheus.fe...@eldorado.org.br>
Signed-off-by: David Gibson 
---
 target/ppc/cpu.h   |  1 +
 target/ppc/translate.c | 73 +++---
 2 files changed, 41 insertions(+), 33 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index c6fc0043a9..cc1911bc75 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -314,6 +314,7 @@ typedef struct ppc_v3_pate_t {
 #define MSR_AP   23 /* Access privilege state on 602  hflags */
 #define MSR_VSX  23 /* Vector Scalar Extension (ISA 2.06 and later) x hflags */
 #define MSR_SA   22 /* Supervisor access mode on 602  hflags */
+#define MSR_S22 /* Secure state  */
 #define MSR_KEY  19 /* key bit on 603e   */
 #define MSR_POW  18 /* Power management  */
 #define MSR_TGPR 17 /* TGPR usage on 602/603x*/
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 98f304302e..d0d400cd8c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4934,32 +4934,40 @@ static void gen_mtmsrd(DisasContext *ctx)
 CHK_SV;
 
 #if !defined(CONFIG_USER_ONLY)
+TCGv t0, t1;
+target_ulong mask;
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+
 gen_icount_io_start(ctx);
+
 if (ctx->opcode & 0x0001) {
 /* L=1 form only updates EE and RI */
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
-(1 << MSR_RI) | (1 << MSR_EE));
-tcg_gen_andi_tl(t1, cpu_msr,
-~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
-tcg_gen_or_tl(t1, t1, t0);
-
-gen_helper_store_msr(cpu_env, t1);
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-
+mask = (1ULL << MSR_RI) | (1ULL << MSR_EE);
 } else {
+/* mtmsrd does not alter HV, S, ME, or LE */
+mask = ~((1ULL << MSR_LE) | (1ULL << MSR_ME) | (1ULL << MSR_S) |
+ (1ULL << MSR_HV));
 /*
  * XXX: we need to update nip before the store if we enter
  *  power saving mode, we will exit the loop directly from
  *  ppc_store_msr
  */
 gen_update_nip(ctx, ctx->base.pc_next);
-gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
 }
+
+tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], mask);
+tcg_gen_andi_tl(t1, cpu_msr, ~mask);
+tcg_gen_or_tl(t0, t0, t1);
+
+gen_helper_store_msr(cpu_env, t0);
+
 /* Must stop the translation as machine state (may have) changed */
 ctx->base.is_jmp = DISAS_EXIT_UPDATE;
+
+tcg_temp_free(t0);
+tcg_temp_free(t1);
 #endif /* !defined(CONFIG_USER_ONLY) */
 }
 #endif /* defined(TARGET_PPC64) */
@@ -4969,23 +4977,19 @@ static void gen_mtmsr(DisasContext *ctx)
 CHK_SV;
 
 #if !defined(CONFIG_USER_ONLY)
+TCGv t0, t1;
+target_ulong mask = 0x;
+
+t0 = tcg_temp_new();
+t1 = tcg_temp_new();
+
 gen_icount_io_start(ctx);
 if (ctx->opcode & 0x0001) {
 /* L=1 form only updates EE and RI */
-TCGv t0 = tcg_temp_new();
-TCGv t1 = tcg_temp_new();
-tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
-(1 << MSR_RI) | (1 << MSR_EE));
-tcg_gen_andi_tl(t1, cpu_msr,
-~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
-tcg_gen_or_tl(t1, t1, t0);
-
-gen_helper_store_msr(cpu_env, t1);
-tcg_temp_free(t0);
-tcg_temp_free(t1);
-
+mask &= (1ULL << MSR_RI) | (1ULL << MSR_EE);
 } else {
-TCGv msr = tcg_temp_new();
+/* mtmsr does not alter S, ME, or LE */
+mask &= ~((1ULL << MSR_LE) | (1ULL << MSR_ME) | (1ULL << MSR_S));
 
 /*
  * XXX: we need to update nip before the store if we enter
@@ -4993,16 +4997,19 @@ static void gen_mtmsr(DisasContext *ctx)
  *  ppc_store_msr
  */
 gen_update_nip(ctx, ctx->base.pc_next);
-#if defined(TARGET_PPC64)
-tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32);
-#else
-tcg_gen_mov_tl(msr, cpu_gpr[rS(ctx->opcode)]);
-#endif
-gen_helper_store_msr(cpu_env, msr);
-tcg_temp_free(msr);
 }
+
+tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)], 

[PULL 15/25] ppc/pegasos2: Implement get-time-of-day RTAS function with VOF

[David Gibson]

From: BALATON Zoltan 

This is needed for Linux to access RTC time.

Signed-off-by: BALATON Zoltan 
Message-Id: 
<6233eb07c680d6c74427e11b9641958f98d53378.1634241019.git.bala...@eik.bme.hu>
Signed-off-by: David Gibson 
---
 hw/ppc/pegasos2.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index a1dd1f6752..a9e3625f56 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -31,6 +31,8 @@
 #include "sysemu/kvm.h"
 #include "kvm_ppc.h"
 #include "exec/address-spaces.h"
+#include "qom/qom-qobject.h"
+#include "qapi/qmp/qdict.h"
 #include "trace.h"
 #include "qemu/datadir.h"
 #include "sysemu/device_tree.h"
@@ -369,6 +371,29 @@ static target_ulong pegasos2_rtas(PowerPCCPU *cpu, 
Pegasos2MachineState *pm,
 return H_PARAMETER;
 }
 switch (token) {
+case RTAS_GET_TIME_OF_DAY:
+{
+QObject *qo = object_property_get_qobject(qdev_get_machine(),
+  "rtc-time", _fatal);
+QDict *qd = qobject_to(QDict, qo);
+
+if (nargs != 0 || nrets != 8 || !qd) {
+stl_be_phys(as, rets, -1);
+qobject_unref(qo);
+return H_PARAMETER;
+}
+
+stl_be_phys(as, rets, 0);
+stl_be_phys(as, rets + 4, qdict_get_int(qd, "tm_year") + 1900);
+stl_be_phys(as, rets + 8, qdict_get_int(qd, "tm_mon") + 1);
+stl_be_phys(as, rets + 12, qdict_get_int(qd, "tm_mday"));
+stl_be_phys(as, rets + 16, qdict_get_int(qd, "tm_hour"));
+stl_be_phys(as, rets + 20, qdict_get_int(qd, "tm_min"));
+stl_be_phys(as, rets + 24, qdict_get_int(qd, "tm_sec"));
+stl_be_phys(as, rets + 28, 0);
+qobject_unref(qo);
+return H_SUCCESS;
+}
 case RTAS_READ_PCI_CONFIG:
 {
 uint32_t addr, len, val;
-- 
2.31.1

[PULL 21/25] target/ppc: add MMCR0 PMCC bits to hflags

[David Gibson]

From: Daniel Henrique Barboza 

We're going to add PMU support for TCG PPC64 chips, based on IBM POWER8+
emulation and following PowerISA v3.1. This requires several PMU related
registers to be exposed to userspace (problem state). PowerISA v3.1
dictates that the PMCC bits of the MMCR0 register controls the level of
access of the PMU registers to problem state.

This patch start things off by exposing both PMCC bits to hflags,
allowing us to access them via DisasContext in the read/write callbacks
that we're going to add next.

Signed-off-by: Daniel Henrique Barboza 
Message-Id: <20211018010133.315842-2-danielhb...@gmail.com>
Signed-off-by: David Gibson 
---
 target/ppc/cpu.h | 6 ++
 target/ppc/helper_regs.c | 6 ++
 target/ppc/translate.c   | 4 
 3 files changed, 16 insertions(+)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index cc1911bc75..24d1f2cf97 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -343,6 +343,10 @@ typedef struct ppc_v3_pate_t {
 #define MSR_RI   1  /* Recoverable interrupt1*/
 #define MSR_LE   0  /* Little-endian mode   1 hflags */
 
+/* PMU bits */
+#define MMCR0_PMCC0  PPC_BIT(44) /* PMC Control bit 0 */
+#define MMCR0_PMCC1  PPC_BIT(45) /* PMC Control bit 1 */
+
 /* LPCR bits */
 #define LPCR_VPM0 PPC_BIT(0)
 #define LPCR_VPM1 PPC_BIT(1)
@@ -608,6 +612,8 @@ enum {
 HFLAGS_SE = 10,  /* MSR_SE -- from elsewhere on embedded ppc */
 HFLAGS_FP = 13,  /* MSR_FP */
 HFLAGS_PR = 14,  /* MSR_PR */
+HFLAGS_PMCC0 = 15,  /* MMCR0 PMCC bit 0 */
+HFLAGS_PMCC1 = 16,  /* MMCR0 PMCC bit 1 */
 HFLAGS_VSX = 23, /* MSR_VSX if cpu has VSX */
 HFLAGS_VR = 25,  /* MSR_VR if cpu has VRE */
 
diff --git a/target/ppc/helper_regs.c b/target/ppc/helper_regs.c
index 1bfb480ecf..99562edd57 100644
--- a/target/ppc/helper_regs.c
+++ b/target/ppc/helper_regs.c
@@ -109,6 +109,12 @@ static uint32_t hreg_compute_hflags_value(CPUPPCState *env)
 if (env->spr[SPR_LPCR] & LPCR_HR) {
 hflags |= 1 << HFLAGS_HR;
 }
+if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC0) {
+hflags |= 1 << HFLAGS_PMCC0;
+}
+if (env->spr[SPR_POWER_MMCR0] & MMCR0_PMCC1) {
+hflags |= 1 << HFLAGS_PMCC1;
+}
 
 #ifndef CONFIG_USER_ONLY
 if (!env->has_hv_mode || (msr & (1ull << MSR_HV))) {
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index d0d400cd8c..a4c5ef3701 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -175,6 +175,8 @@ struct DisasContext {
 bool tm_enabled;
 bool gtse;
 bool hr;
+bool mmcr0_pmcc0;
+bool mmcr0_pmcc1;
 ppc_spr_t *spr_cb; /* Needed to check rights for mfspr/mtspr */
 int singlestep_enabled;
 uint32_t flags;
@@ -8552,6 +8554,8 @@ static void ppc_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cs)
 ctx->tm_enabled = (hflags >> HFLAGS_TM) & 1;
 ctx->gtse = (hflags >> HFLAGS_GTSE) & 1;
 ctx->hr = (hflags >> HFLAGS_HR) & 1;
+ctx->mmcr0_pmcc0 = (hflags >> HFLAGS_PMCC0) & 1;
+ctx->mmcr0_pmcc1 = (hflags >> HFLAGS_PMCC1) & 1;
 
 ctx->singlestep_enabled = 0;
 if ((hflags >> HFLAGS_SE) & 1) {
-- 
2.31.1

[PULL 10/25] target/ppc: Fix XER access in gdbstub

[David Gibson]

From: Matheus Ferst 

The value of XER is split in multiple fields of CPUPPCState, like
env->xer and env->so. To get/set the whole register from gdb, we should
use cpu_read_xer/cpu_write_xer.

Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst 
Message-Id: <20211014223234.127012-3-matheus.fe...@eldorado.org.br>
Reviewed-by: Richard Henderson 
Signed-off-by: David Gibson 
---
 target/ppc/gdbstub.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 1808a150e4..105c2f7dd1 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -159,7 +159,7 @@ int ppc_cpu_gdb_read_register(CPUState *cs, GByteArray 
*buf, int n)
 gdb_get_regl(buf, env->ctr);
 break;
 case 69:
-gdb_get_reg32(buf, env->xer);
+gdb_get_reg32(buf, cpu_read_xer(env));
 break;
 case 70:
 gdb_get_reg32(buf, env->fpscr);
@@ -217,7 +217,7 @@ int ppc_cpu_gdb_read_register_apple(CPUState *cs, 
GByteArray *buf, int n)
 gdb_get_reg64(buf, env->ctr);
 break;
 case 69 + 32:
-gdb_get_reg32(buf, env->xer);
+gdb_get_reg32(buf, cpu_read_xer(env));
 break;
 case 70 + 32:
 gdb_get_reg64(buf, env->fpscr);
@@ -269,7 +269,7 @@ int ppc_cpu_gdb_write_register(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->ctr = ldtul_p(mem_buf);
 break;
 case 69:
-env->xer = ldl_p(mem_buf);
+cpu_write_xer(env, ldl_p(mem_buf));
 break;
 case 70:
 /* fpscr */
@@ -319,7 +319,7 @@ int ppc_cpu_gdb_write_register_apple(CPUState *cs, uint8_t 
*mem_buf, int n)
 env->ctr = ldq_p(mem_buf);
 break;
 case 69 + 32:
-env->xer = ldl_p(mem_buf);
+cpu_write_xer(env, ldl_p(mem_buf));
 break;
 case 70 + 32:
 /* fpscr */
-- 
2.31.1

[PULL 05/25] hw/ppc/spapr_softmmu: Reduce include list

[David Gibson]

From: Philippe Mathieu-Daudé 

Commit 962104f0448 ("hw/ppc: moved hcalls that depend on softmmu")
introduced a lot of unnecessary #include directives. Remove them.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20211006170801.178023-1-phi...@redhat.com>
Signed-off-by: David Gibson 
---
 hw/ppc/spapr_softmmu.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/hw/ppc/spapr_softmmu.c b/hw/ppc/spapr_softmmu.c
index 6c6b86dd3c..f8924270ef 100644
--- a/hw/ppc/spapr_softmmu.c
+++ b/hw/ppc/spapr_softmmu.c
@@ -1,25 +1,10 @@
 #include "qemu/osdep.h"
 #include "qemu/cutils.h"
-#include "qapi/error.h"
-#include "sysemu/hw_accel.h"
-#include "sysemu/runstate.h"
-#include "qemu/log.h"
-#include "qemu/main-loop.h"
-#include "qemu/module.h"
-#include "qemu/error-report.h"
 #include "cpu.h"
-#include "exec/exec-all.h"
 #include "helper_regs.h"
 #include "hw/ppc/spapr.h"
-#include "hw/ppc/spapr_cpu_core.h"
 #include "mmu-hash64.h"
-#include "cpu-models.h"
-#include "trace.h"
-#include "kvm_ppc.h"
-#include "hw/ppc/fdt.h"
-#include "hw/ppc/spapr_ovec.h"
 #include "mmu-book3s-v3.h"
-#include "hw/mem/memory-device.h"
 
 static inline bool valid_ptex(PowerPCCPU *cpu, target_ulong ptex)
 {
-- 
2.31.1

[PULL 14/25] ppc/pegasos2: Warn when using VOF but no kernel is specified

[David Gibson]

From: BALATON Zoltan 

Issue a warning when using VOF (which is the default) but no -kernel
option given to let users know that it will likely fail as the guest
has nothing to run. It is not a hard error because it may still be
useful to start the machine without further options for testing or
inspecting it from monitor without actually booting it.

Signed-off-by: BALATON Zoltan 
Message-Id: 

Signed-off-by: David Gibson 
---
 hw/ppc/pegasos2.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index 474cfdeabf..a1dd1f6752 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -194,7 +194,10 @@ static void pegasos2_init(MachineState *machine)
 if (!pm->vof) {
 warn_report("Option -kernel may be ineffective with -bios.");
 }
+} else if (pm->vof) {
+warn_report("Using Virtual OpenFirmware but no -kernel option.");
 }
+
 if (!pm->vof && machine->kernel_cmdline && machine->kernel_cmdline[0]) {
 warn_report("Option -append may be ineffective with -bios.");
 }
-- 
2.31.1

[PULL 13/25] ppc/pegasos2: Restrict memory to 2 gigabytes

[David Gibson]

From: BALATON Zoltan 

The CHRP spec this board confirms to only allows 2 GiB of system
memory below 4 GiB as the high 2 GiB is allocated to IO and system
resources. To avoid problems with memory overlapping these areas
restrict RAM to 2 GiB similar to mac_newworld.

Signed-off-by: BALATON Zoltan 
Message-Id: 
<54f58229a69c9c1cca21bcecad700b3d7052edd5.1634241019.git.bala...@eik.bme.hu>
Signed-off-by: David Gibson 
---
 hw/ppc/pegasos2.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/pegasos2.c b/hw/ppc/pegasos2.c
index b8ce859f1a..474cfdeabf 100644
--- a/hw/ppc/pegasos2.c
+++ b/hw/ppc/pegasos2.c
@@ -117,6 +117,10 @@ static void pegasos2_init(MachineState *machine)
 qemu_register_reset(pegasos2_cpu_reset, pm->cpu);
 
 /* RAM */
+if (machine->ram_size > 2 * GiB) {
+error_report("RAM size more than 2 GiB is not supported");
+exit(1);
+}
 memory_region_add_subregion(get_system_memory(), 0, machine->ram);
 
 /* allocate and load firmware */
-- 
2.31.1

[PULL 06/25] spapr/xive: Use xive_esb_rw() to trigger interrupts

[David Gibson]

From: Cédric Le Goater 

xive_esb_rw() is the common routine used for memory accesses on ESB
page. Use it for triggers also.

Signed-off-by: Cédric Le Goater 
Message-Id: <20211006210546.641102-1-...@kaod.org>
Signed-off-by: David Gibson 
---
 hw/intc/spapr_xive_kvm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index be94cff148..61fe7bd2d3 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -301,9 +301,7 @@ static uint8_t xive_esb_read(XiveSource *xsrc, int srcno, 
uint32_t offset)
 
 static void kvmppc_xive_esb_trigger(XiveSource *xsrc, int srcno)
 {
-uint64_t *addr = xsrc->esb_mmap + xive_source_esb_page(xsrc, srcno);
-
-*addr = 0x0;
+xive_esb_rw(xsrc, srcno, 0, 0, true);
 }
 
 uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, uint32_t offset,
-- 
2.31.1

[PULL 08/25] tests/acceptance: Add tests for the ppc405 boards

[David Gibson]

From: Thomas Huth 

Using the U-Boot firmware, we can check that at least the serial console
of the ppc405 boards is still usable.

Signed-off-by: Thomas Huth 
Message-Id: <20211011125930.750217-1-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Tested-by: Philippe Mathieu-Daudé 
[dwg: Added an extra tag at Philippe's suggestion]
Signed-off-by: David Gibson 
---
 tests/acceptance/ppc_405.py | 42 +
 1 file changed, 42 insertions(+)
 create mode 100644 tests/acceptance/ppc_405.py

diff --git a/tests/acceptance/ppc_405.py b/tests/acceptance/ppc_405.py
new file mode 100644
index 00..c534d5d32f
--- /dev/null
+++ b/tests/acceptance/ppc_405.py
@@ -0,0 +1,42 @@
+# Test that the U-Boot firmware boots on ppc 405 machines and check the console
+#
+# Copyright (c) 2021 Red Hat, Inc.
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado.utils import archive
+from avocado_qemu import Test
+from avocado_qemu import wait_for_console_pattern
+from avocado_qemu import exec_command_and_wait_for_pattern
+
+class Ppc405Machine(Test):
+
+timeout = 90
+
+def do_test_ppc405(self):
+uboot_url = ('https://gitlab.com/huth/u-boot/-/raw/'
+ 'taihu-2021-10-09/u-boot-taihu.bin')
+uboot_hash = ('3208940e908a5edc7c03eab072c60f0dcfadc2ab');
+file_path = self.fetch_asset(uboot_url, asset_hash=uboot_hash)
+self.vm.set_console(console_index=1)
+self.vm.add_args('-bios', file_path)
+self.vm.launch()
+wait_for_console_pattern(self, 'AMCC PPC405EP Evaluation Board')
+exec_command_and_wait_for_pattern(self, 'reset', 'AMCC PowerPC 405EP')
+
+def test_ppc_taihu(self):
+"""
+:avocado: tags=arch:ppc
+:avocado: tags=machine:taihu
+:avocado: tags=cpu:405ep
+"""
+self.do_test_ppc405()
+
+def test_ppc_ref405ep(self):
+"""
+:avocado: tags=arch:ppc
+:avocado: tags=machine:ref405ep
+:avocado: tags=cpu:405ep
+"""
+self.do_test_ppc405()
-- 
2.31.1

[PULL 09/25] linux-user/ppc: Fix XER access in save/restore_user_regs

[David Gibson]

From: Matheus Ferst 

We should use cpu_read_xer/cpu_write_xer to save/restore the complete
register since some of its bits are in other fields of CPUPPCState. A
test is added to prevent future regressions.

Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst 
Message-Id: <20211014223234.127012-2-matheus.fe...@eldorado.org.br>
Reviewed-by: Richard Henderson 
Signed-off-by: David Gibson 
---
 linux-user/ppc/signal.c |  9 +++--
 tests/tcg/ppc64/Makefile.target |  2 +
 tests/tcg/ppc64le/Makefile.target   |  2 +
 tests/tcg/ppc64le/signal_save_restore_xer.c | 42 +
 4 files changed, 52 insertions(+), 3 deletions(-)
 create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c

diff --git a/linux-user/ppc/signal.c b/linux-user/ppc/signal.c
index c37744c8fc..90a0369632 100644
--- a/linux-user/ppc/signal.c
+++ b/linux-user/ppc/signal.c
@@ -242,7 +242,7 @@ static void save_user_regs(CPUPPCState *env, struct 
target_mcontext *frame)
 __put_user(env->nip, >mc_gregs[TARGET_PT_NIP]);
 __put_user(env->ctr, >mc_gregs[TARGET_PT_CTR]);
 __put_user(env->lr, >mc_gregs[TARGET_PT_LNK]);
-__put_user(env->xer, >mc_gregs[TARGET_PT_XER]);
+__put_user(cpu_read_xer(env), >mc_gregs[TARGET_PT_XER]);
 
 for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
 ccr |= env->crf[i] << (32 - ((i + 1) * 4));
@@ -315,6 +315,7 @@ static void restore_user_regs(CPUPPCState *env,
 {
 target_ulong save_r2 = 0;
 target_ulong msr;
+target_ulong xer;
 target_ulong ccr;
 
 int i;
@@ -330,9 +331,11 @@ static void restore_user_regs(CPUPPCState *env,
 __get_user(env->nip, >mc_gregs[TARGET_PT_NIP]);
 __get_user(env->ctr, >mc_gregs[TARGET_PT_CTR]);
 __get_user(env->lr, >mc_gregs[TARGET_PT_LNK]);
-__get_user(env->xer, >mc_gregs[TARGET_PT_XER]);
-__get_user(ccr, >mc_gregs[TARGET_PT_CCR]);
 
+__get_user(xer, >mc_gregs[TARGET_PT_XER]);
+cpu_write_xer(env, xer);
+
+__get_user(ccr, >mc_gregs[TARGET_PT_CCR]);
 for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
 env->crf[i] = (ccr >> (32 - ((i + 1) * 4))) & 0xf;
 }
diff --git a/tests/tcg/ppc64/Makefile.target b/tests/tcg/ppc64/Makefile.target
index a6a4ddaeca..6ab7934fdf 100644
--- a/tests/tcg/ppc64/Makefile.target
+++ b/tests/tcg/ppc64/Makefile.target
@@ -23,4 +23,6 @@ run-plugin-byte_reverse-with-%:
$(call skip-test, "RUN of byte_reverse ($*)", "not built")
 endif
 
+PPC64_TESTS += signal_save_restore_xer
+
 TESTS += $(PPC64_TESTS)
diff --git a/tests/tcg/ppc64le/Makefile.target 
b/tests/tcg/ppc64le/Makefile.target
index c0c14ffbad..5e65b1590d 100644
--- a/tests/tcg/ppc64le/Makefile.target
+++ b/tests/tcg/ppc64le/Makefile.target
@@ -22,4 +22,6 @@ run-plugin-byte_reverse-with-%:
$(call skip-test, "RUN of byte_reverse ($*)", "not built")
 endif
 
+PPC64LE_TESTS += signal_save_restore_xer
+
 TESTS += $(PPC64LE_TESTS)
diff --git a/tests/tcg/ppc64le/signal_save_restore_xer.c 
b/tests/tcg/ppc64le/signal_save_restore_xer.c
new file mode 100644
index 00..e4f8a07dd7
--- /dev/null
+++ b/tests/tcg/ppc64le/signal_save_restore_xer.c
@@ -0,0 +1,42 @@
+#include 
+#include 
+#include 
+#include 
+
+#define XER_SO   (1 << 31)
+#define XER_OV   (1 << 30)
+#define XER_CA   (1 << 29)
+#define XER_OV32 (1 << 19)
+#define XER_CA32 (1 << 18)
+
+uint64_t saved;
+
+void sigill_handler(int sig, siginfo_t *si, void *ucontext)
+{
+ucontext_t *uc = ucontext;
+uc->uc_mcontext.regs->nip += 4;
+saved = uc->uc_mcontext.regs->xer;
+uc->uc_mcontext.regs->xer |= XER_OV | XER_OV32;
+}
+
+int main(void)
+{
+uint64_t initial = XER_CA | XER_CA32, restored;
+struct sigaction sa = {
+.sa_sigaction = sigill_handler,
+.sa_flags = SA_SIGINFO
+};
+
+sigaction(SIGILL, , NULL);
+
+asm("mtspr 1, %1\n\t"
+".long 0x0\n\t"
+"mfspr %0, 1\n\t"
+: "=r" (restored)
+: "r" (initial));
+
+assert(saved == initial);
+assert(restored == (XER_OV | XER_OV32 | XER_CA | XER_CA32));
+
+return 0;
+}
-- 
2.31.1

[PULL 02/25] target/ppc: Use tcg_constant_i32() in gen_setb()

[David Gibson]

From: Philippe Mathieu-Daudé 

Avoid using TCG temporaries for the -1 and 8 constant values.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20211003141711.3673181-2-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: David Gibson 
---
 target/ppc/translate.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index c3c6cb9589..0258c1be16 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -5068,19 +5068,15 @@ static void gen_mtspr(DisasContext *ctx)
 static void gen_setb(DisasContext *ctx)
 {
 TCGv_i32 t0 = tcg_temp_new_i32();
-TCGv_i32 t8 = tcg_temp_new_i32();
-TCGv_i32 tm1 = tcg_temp_new_i32();
+TCGv_i32 t8 = tcg_constant_i32(8);
+TCGv_i32 tm1 = tcg_constant_i32(-1);
 int crf = crfS(ctx->opcode);
 
 tcg_gen_setcondi_i32(TCG_COND_GEU, t0, cpu_crf[crf], 4);
-tcg_gen_movi_i32(t8, 8);
-tcg_gen_movi_i32(tm1, -1);
 tcg_gen_movcond_i32(TCG_COND_GEU, t0, cpu_crf[crf], t8, tm1, t0);
 tcg_gen_ext_i32_tl(cpu_gpr[rD(ctx->opcode)], t0);
 
 tcg_temp_free_i32(t0);
-tcg_temp_free_i32(t8);
-tcg_temp_free_i32(tm1);
 }
 #endif
 
-- 
2.31.1

[PULL 07/25] hw/ppc: Fix iothread locking in the 405 code

[David Gibson]

From: Thomas Huth 

When using u-boot as firmware with the taihu board, QEMU aborts with
this assertion:

 ERROR:../accel/tcg/tcg-accel-ops.c:79:tcg_handle_interrupt: assertion failed:
  (qemu_mutex_iothread_locked())

Running QEMU with "-d in_asm" shows that the crash happens when writing
to SPR 0x3f2, so we are missing to lock the iothread in the code path
here.

Signed-off-by: Thomas Huth 
Message-Id: <20211006071140.565952-1-th...@redhat.com>
Reviewed-by: Cédric Le Goater 
Tested-by: Cédric Le Goater 
Signed-off-by: David Gibson 
---
 hw/ppc/ppc.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index a724b0bb5e..e8127599c9 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -336,6 +336,8 @@ void store_40x_dbcr0(CPUPPCState *env, uint32_t val)
 {
 PowerPCCPU *cpu = env_archcpu(env);
 
+qemu_mutex_lock_iothread();
+
 switch ((val >> 28) & 0x3) {
 case 0x0:
 /* No action */
@@ -353,6 +355,8 @@ void store_40x_dbcr0(CPUPPCState *env, uint32_t val)
 ppc40x_system_reset(cpu);
 break;
 }
+
+qemu_mutex_unlock_iothread();
 }
 
 /* PowerPC 40x internal IRQ controller */
-- 
2.31.1

[PULL 11/25] linux-user: Fix XER access in ppc version of elf_core_copy_regs

[David Gibson]

From: Matheus Ferst 

env->xer doesn't hold some bits of XER, like OV and CA. To write the
complete register in the core dump we should read XER value with
cpu_read_xer.

Reported-by: Lucas Mateus Castro (alqotel) 
Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst 
Message-Id: <20211014223234.127012-4-matheus.fe...@eldorado.org.br>
Reviewed-by: Richard Henderson 
Signed-off-by: David Gibson 
---
 linux-user/elfload.c | 2 +-
 target/ppc/cpu.c | 2 +-
 target/ppc/cpu.h | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index 2404d482ba..eb32f3e2cb 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -901,7 +901,7 @@ static void elf_core_copy_regs(target_elf_gregset_t *regs, 
const CPUPPCState *en
 (*regs)[33] = tswapreg(env->msr);
 (*regs)[35] = tswapreg(env->ctr);
 (*regs)[36] = tswapreg(env->lr);
-(*regs)[37] = tswapreg(env->xer);
+(*regs)[37] = tswapreg(cpu_read_xer(env));
 
 for (i = 0; i < ARRAY_SIZE(env->crf); i++) {
 ccr |= env->crf[i] << (32 - ((i + 1) * 4));
diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
index 7ad9bd6044..f933d9f2bd 100644
--- a/target/ppc/cpu.c
+++ b/target/ppc/cpu.c
@@ -27,7 +27,7 @@
 #include "helper_regs.h"
 #include "sysemu/tcg.h"
 
-target_ulong cpu_read_xer(CPUPPCState *env)
+target_ulong cpu_read_xer(const CPUPPCState *env)
 {
 if (is_isa300(env)) {
 return env->xer | (env->so << XER_SO) |
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index baa4e7c34d..c6fc0043a9 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2412,7 +2412,7 @@ enum {
 /*/
 
 #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
-target_ulong cpu_read_xer(CPUPPCState *env);
+target_ulong cpu_read_xer(const CPUPPCState *env);
 void cpu_write_xer(CPUPPCState *env, target_ulong xer);
 
 /*
-- 
2.31.1

[PULL 12/25] target/ppc: Fix XER access in monitor

[David Gibson]

From: Matheus Ferst 

We can't read env->xer directly, as it does not contain some bits of
XER. Instead, we should have a callback that uses cpu_read_xer to read
the complete register.

Fixes: da91a00f191f ("target-ppc: Split out SO, OV, CA fields from XER")
Signed-off-by: Matheus Ferst 
Message-Id: <20211014223234.127012-5-matheus.fe...@eldorado.org.br>
Reviewed-by: Richard Henderson 
Signed-off-by: David Gibson 
---
 target/ppc/monitor.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/ppc/monitor.c b/target/ppc/monitor.c
index a475108b2d..0b805ef6e9 100644
--- a/target/ppc/monitor.c
+++ b/target/ppc/monitor.c
@@ -44,6 +44,13 @@ static target_long monitor_get_ccr(Monitor *mon, const 
struct MonitorDef *md,
 return u;
 }
 
+static target_long monitor_get_xer(Monitor *mon, const struct MonitorDef *md,
+   int val)
+{
+CPUArchState *env = mon_get_cpu_env(mon);
+return cpu_read_xer(env);
+}
+
 static target_long monitor_get_decr(Monitor *mon, const struct MonitorDef *md,
 int val)
 {
@@ -85,7 +92,7 @@ const MonitorDef monitor_defs[] = {
 { "decr", 0, _get_decr, },
 { "ccr|cr", 0, _get_ccr, },
 /* Machine state register */
-{ "xer", offsetof(CPUPPCState, xer) },
+{ "xer", 0, _get_xer },
 { "msr", offsetof(CPUPPCState, msr) },
 { "tbu", 0, _get_tbu, },
 { "tbl", 0, _get_tbl, },
-- 
2.31.1

[PULL 03/25] target/ppc: Use tcg_constant_i64() in gen_brh()

[David Gibson]

From: Philippe Mathieu-Daudé 

The mask of the Byte-Reverse Halfword opcode is a read-only
constant. We can avoid using a TCG temporary by moving the
mask to the constant pool.

Signed-off-by: Philippe Mathieu-Daudé 
Message-Id: <20211003141711.3673181-3-f4...@amsat.org>
Reviewed-by: Richard Henderson 
Signed-off-by: David Gibson 
---
 target/ppc/translate.c | 8 +++-
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0258c1be16..98f304302e 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7569,18 +7569,16 @@ static void gen_brw(DisasContext *ctx)
 /* brh */
 static void gen_brh(DisasContext *ctx)
 {
-TCGv_i64 t0 = tcg_temp_new_i64();
+TCGv_i64 mask = tcg_constant_i64(0x00ff00ff00ff00ffull);
 TCGv_i64 t1 = tcg_temp_new_i64();
 TCGv_i64 t2 = tcg_temp_new_i64();
 
-tcg_gen_movi_i64(t0, 0x00ff00ff00ff00ffull);
 tcg_gen_shri_i64(t1, cpu_gpr[rS(ctx->opcode)], 8);
-tcg_gen_and_i64(t2, t1, t0);
-tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], t0);
+tcg_gen_and_i64(t2, t1, mask);
+tcg_gen_and_i64(t1, cpu_gpr[rS(ctx->opcode)], mask);
 tcg_gen_shli_i64(t1, t1, 8);
 tcg_gen_or_i64(cpu_gpr[rA(ctx->opcode)], t1, t2);
 
-tcg_temp_free_i64(t0);
 tcg_temp_free_i64(t1);
 tcg_temp_free_i64(t2);
 }
-- 
2.31.1

[PULL 00/25] ppc-for-6.2 queue 20211021

[David Gibson]

The following changes since commit afc9fcde55296b83f659de9da3cdf044812a6eeb:

  Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging 
(2021-10-20 06:10:51 -0700)

are available in the Git repository at:

  https://gitlab.com/dgibson/qemu.git tags/ppc-for-6.2-20211021

for you to fetch changes up to 6f9e8515c106650fbba7222c8f66234c8546c025:

  hw/ppc/ppc4xx_pci: Fix ppc4xx_pci_map_irq() for recent Linux kernels 
(2021-10-21 11:42:47 +1100)


ppc patch queue 2021-10-21

Here's the next batch of ppc target related patches for qemu-6.2.
Highlights are:
 * Some fixes and minimal tests for old embedded ppc platforms
 * The beginnings of PMU emulation in TCG from Daniel Barboza
 * Some improvements to the pegasos2 platform
 * A number of TCG bugfixes from the folks at the El Dorado Institute
 * A few other assorted bugfixes and cleanups


BALATON Zoltan (6):
  ppc/pegasos2: Restrict memory to 2 gigabytes
  ppc/pegasos2: Warn when using VOF but no kernel is specified
  ppc/pegasos2: Implement get-time-of-day RTAS function with VOF
  ppc/pegasos2: Access MV64361 registers via their memory region
  ppc/pegasos2: Add constants for PCI config addresses
  ppc/pegasos2: Implement power-off RTAS function with VOF

Cédric Le Goater (3):
  spapr/xive: Add source status helpers
  target/ppc: Fix the test raising the decrementer exception
  spapr/xive: Use xive_esb_rw() to trigger interrupts

Daniel Henrique Barboza (3):
  target/ppc: add MMCR0 PMCC bits to hflags
  target/ppc: add user read/write functions for MMCR2
  target/ppc: adding user read/write functions for PMCs

Gustavo Romero (1):
  target/ppc: add user read/write functions for MMCR0

Matheus Ferst (5):
  linux-user/ppc: Fix XER access in save/restore_user_regs
  target/ppc: Fix XER access in gdbstub
  linux-user: Fix XER access in ppc version of elf_core_copy_regs
  target/ppc: Fix XER access in monitor
  target/ppc: Filter mtmsr[d] input before setting MSR

Philippe Mathieu-Daudé (3):
  target/ppc: Use tcg_constant_i32() in gen_setb()
  target/ppc: Use tcg_constant_i64() in gen_brh()
  hw/ppc/spapr_softmmu: Reduce include list

Thomas Huth (4):
  hw/ppc: Fix iothread locking in the 405 code
  tests/acceptance: Add tests for the ppc405 boards
  tests/acceptance: Add a test for the bamboo ppc board
  hw/ppc/ppc4xx_pci: Fix ppc4xx_pci_map_irq() for recent Linux kernels

 MAINTAINERS |   1 +
 hw/intc/spapr_xive.c|   2 +-
 hw/intc/spapr_xive_kvm.c|  14 +-
 hw/intc/xive.c  |   8 +-
 hw/pci-host/mv64361.c   |   1 +
 hw/ppc/pegasos2.c   | 162 ++---
 hw/ppc/ppc.c|   6 +-
 hw/ppc/ppc4xx_pci.c |   8 +-
 hw/ppc/spapr_softmmu.c  |  15 --
 include/hw/ppc/xive.h   |  24 +++
 linux-user/elfload.c|   2 +-
 linux-user/ppc/signal.c |   9 +-
 target/ppc/cpu.c|   2 +-
 target/ppc/cpu.h|  25 ++-
 target/ppc/cpu_init.c   |  16 +-
 target/ppc/gdbstub.c|   8 +-
 target/ppc/helper_regs.c|   6 +
 target/ppc/monitor.c|   9 +-
 target/ppc/power8-pmu-regs.c.inc| 262 
 target/ppc/spr_tcg.h|   8 +
 target/ppc/translate.c  |  95 +-
 tests/acceptance/ppc_405.py |  42 +
 tests/acceptance/ppc_bamboo.py  |  39 +
 tests/tcg/ppc64/Makefile.target |   2 +
 tests/tcg/ppc64le/Makefile.target   |   2 +
 tests/tcg/ppc64le/signal_save_restore_xer.c |  42 +
 26 files changed, 651 insertions(+), 159 deletions(-)
 create mode 100644 target/ppc/power8-pmu-regs.c.inc
 create mode 100644 tests/acceptance/ppc_405.py
 create mode 100644 tests/acceptance/ppc_bamboo.py
 create mode 100644 tests/tcg/ppc64le/signal_save_restore_xer.c

[PULL 01/25] spapr/xive: Add source status helpers

[David Gibson]

From: Cédric Le Goater 

and use them to set and test the ASSERTED bit of LSI sources.

Signed-off-by: Cédric Le Goater 
Message-Id: <20211004212141.432954-1-...@kaod.org>
Signed-off-by: David Gibson 
---
 hw/intc/spapr_xive.c |  2 +-
 hw/intc/spapr_xive_kvm.c | 10 +++---
 hw/intc/xive.c   |  8 
 include/hw/ppc/xive.h| 24 
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 89cfa018f5..4ec659b93e 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -185,7 +185,7 @@ static void spapr_xive_pic_print_info(SpaprXive *xive, 
Monitor *mon)
xive_source_irq_is_lsi(xsrc, i) ? "LSI" : "MSI",
pq & XIVE_ESB_VAL_P ? 'P' : '-',
pq & XIVE_ESB_VAL_Q ? 'Q' : '-',
-   xsrc->status[i] & XIVE_STATUS_ASSERTED ? 'A' : ' ',
+   xive_source_is_asserted(xsrc, i) ? 'A' : ' ',
xive_eas_is_masked(eas) ? "M" : " ",
(int) xive_get_field64(EAS_END_DATA, eas->w));
 
diff --git a/hw/intc/spapr_xive_kvm.c b/hw/intc/spapr_xive_kvm.c
index 6d4909d0a8..be94cff148 100644
--- a/hw/intc/spapr_xive_kvm.c
+++ b/hw/intc/spapr_xive_kvm.c
@@ -242,7 +242,7 @@ int kvmppc_xive_source_reset_one(XiveSource *xsrc, int 
srcno, Error **errp)
 
 if (xive_source_irq_is_lsi(xsrc, srcno)) {
 state |= KVM_XIVE_LEVEL_SENSITIVE;
-if (xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
+if (xive_source_is_asserted(xsrc, srcno)) {
 state |= KVM_XIVE_LEVEL_ASSERTED;
 }
 }
@@ -321,7 +321,7 @@ uint64_t kvmppc_xive_esb_rw(XiveSource *xsrc, int srcno, 
uint32_t offset,
 if (xive_source_irq_is_lsi(xsrc, srcno) &&
 offset == XIVE_ESB_LOAD_EOI) {
 xive_esb_read(xsrc, srcno, XIVE_ESB_SET_PQ_00);
-if (xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
+if (xive_source_is_asserted(xsrc, srcno)) {
 kvmppc_xive_esb_trigger(xsrc, srcno);
 }
 return 0;
@@ -359,11 +359,7 @@ void kvmppc_xive_source_set_irq(void *opaque, int srcno, 
int val)
 return;
 }
 } else {
-if (val) {
-xsrc->status[srcno] |= XIVE_STATUS_ASSERTED;
-} else {
-xsrc->status[srcno] &= ~XIVE_STATUS_ASSERTED;
-}
+xive_source_set_asserted(xsrc, srcno, val);
 }
 
 kvmppc_xive_esb_trigger(xsrc, srcno);
diff --git a/hw/intc/xive.c b/hw/intc/xive.c
index 6c82326ec7..190194d27f 100644
--- a/hw/intc/xive.c
+++ b/hw/intc/xive.c
@@ -875,7 +875,7 @@ static bool xive_source_lsi_trigger(XiveSource *xsrc, 
uint32_t srcno)
 {
 uint8_t old_pq = xive_source_esb_get(xsrc, srcno);
 
-xsrc->status[srcno] |= XIVE_STATUS_ASSERTED;
+xive_source_set_asserted(xsrc, srcno, true);
 
 switch (old_pq) {
 case XIVE_ESB_RESET:
@@ -923,7 +923,7 @@ static bool xive_source_esb_eoi(XiveSource *xsrc, uint32_t 
srcno)
  * notification
  */
 if (xive_source_irq_is_lsi(xsrc, srcno) &&
-xsrc->status[srcno] & XIVE_STATUS_ASSERTED) {
+xive_source_is_asserted(xsrc, srcno)) {
 ret = xive_source_lsi_trigger(xsrc, srcno);
 }
 
@@ -1104,7 +1104,7 @@ void xive_source_set_irq(void *opaque, int srcno, int val)
 if (val) {
 notify = xive_source_lsi_trigger(xsrc, srcno);
 } else {
-xsrc->status[srcno] &= ~XIVE_STATUS_ASSERTED;
+xive_source_set_asserted(xsrc, srcno, false);
 }
 } else {
 if (val) {
@@ -1133,7 +1133,7 @@ void xive_source_pic_print_info(XiveSource *xsrc, 
uint32_t offset, Monitor *mon)
xive_source_irq_is_lsi(xsrc, i) ? "LSI" : "MSI",
pq & XIVE_ESB_VAL_P ? 'P' : '-',
pq & XIVE_ESB_VAL_Q ? 'Q' : '-',
-   xsrc->status[i] & XIVE_STATUS_ASSERTED ? 'A' : ' ');
+   xive_source_is_asserted(xsrc, i) ? 'A' : ' ');
 }
 }
 
diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
index 252c58a1d6..b8ab0bf749 100644
--- a/include/hw/ppc/xive.h
+++ b/include/hw/ppc/xive.h
@@ -286,6 +286,30 @@ uint8_t xive_esb_set(uint8_t *pq, uint8_t value);
 uint8_t xive_source_esb_get(XiveSource *xsrc, uint32_t srcno);
 uint8_t xive_source_esb_set(XiveSource *xsrc, uint32_t srcno, uint8_t pq);
 
+/*
+ * Source status helpers
+ */
+static inline void xive_source_set_status(XiveSource *xsrc, uint32_t srcno,
+  uint8_t status, bool enable)
+{
+if (enable) {
+xsrc->status[srcno] |= status;
+} else {
+xsrc->status[srcno] &= ~status;
+}
+}
+
+static inline void xive_source_set_asserted(XiveSource *xsrc, uint32_t srcno,
+bool enable)
+{
+xive_source_set_status(xsrc, srcno, XIVE_STATUS_ASSERTED, enable);
+}
+
+static inline bool 

[PULL 04/25] target/ppc: Fix the test raising the decrementer exception

[David Gibson]

From: Cédric Le Goater 

Commit 4d9b8ef9b5ab ("target/ppc: Fix 64-bit decrementer") introduced
new int64t variables and broke the test triggering the decrementer
exception. Revert partially the change to evaluate both clause of the
if statement.

Reported-by: Coverity CID 1464061
Fixes: 4d9b8ef9b5ab ("target/ppc: Fix 64-bit decrementer")
Suggested-by: Peter Maydell 
Signed-off-by: Cédric Le Goater 
Message-Id: <20211005053324.441132-1-...@kaod.org>
Reviewed-by: Greg Kurz 
Signed-off-by: David Gibson 
---
 hw/ppc/ppc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index f5d012f860..a724b0bb5e 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -848,7 +848,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t 
*nextp,
  * On MSB edge based DEC implementations the MSB going from 0 -> 1 triggers
  * an edge interrupt, so raise it here too.
  */
-if ((signed_value < 3) ||
+if ((value < 3) ||
 ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) ||
 ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0
   && signed_decr >= 0)) {
-- 
2.31.1

Re: [PATCH 0/4] vl: Prioritize device realizations

[Peter Xu]

On Wed, Oct 20, 2021 at 03:44:08PM +0200, David Hildenbrand wrote:
> On 18.08.21 21:42, Peter Xu wrote:
> > This is a long pending issue that we haven't fixed.  The issue is in QEMU we
> > have implicit device ordering requirement when realizing, otherwise some of 
> > the
> > device may not work properly.
> > 
> > The initial requirement comes from when vfio-pci starts to work with 
> > vIOMMUs.
> > To make sure vfio-pci will get the correct DMA address space, the vIOMMU 
> > device
> > needs to be created before vfio-pci otherwise vfio-pci will stop working 
> > when
> > the guest enables the vIOMMU and the device at the same time.
> > 
> > AFAIU Libvirt should have code that guarantees that.  For QEMU cmdline 
> > users,
> > they need to pay attention or things will stop working at some point.
> > 
> > Recently there's a growing and similar requirement on vDPA.  It's not a hard
> > requirement so far but vDPA has patches that try to workaround this issue.
> > 
> > This patchset allows us to realize the devices in the order that e.g. 
> > platform
> > devices will be created first (bus device, IOMMU, etc.), then the rest of
> > normal devices.  It's done simply by ordering the QemuOptsList of "device"
> > entries before realization.  The priority so far comes from migration
> > priorities which could be a little bit odd, but that's really about the same
> > problem and we can clean that part up in the future.
> > 
> > Libvirt can still keep its ordering for sure so old QEMU will still work,
> > however that won't be needed for new qemus after this patchset, so with the 
> > new
> > binary we should be able to specify qemu cmdline as wish on '-device'.
> > 
> > Logically this should also work for vDPA and the workaround code can be done
> > with more straightforward approaches.
> > 
> > Please review, thanks.
> 
> Hi Peter, looks like I have another use case:

Hi, David,

> 
> vhost devices can heavily restrict the number of available memslots:
> e.g., upstream KVM ~64k, vhost-user usually 32 (!). With virtio-mem
> intending to make use of multiple memslots [1] and auto-detecting how
> many to use based on currently avilable memslots when plugging and
> realizing the virtio-mem device, this implies that realizing vhost
> devices (especially vhost-user device) after virtio-mem devices can
> similarly result in issues: when trying realization of the vhost device
> with restricted memslots, QEMU will bail out.
> 
> So similarly, we'd want to realize any vhost-* before any virtio-mem device.
> 
> Do you have any updated version of this patchset? Thanks!

Yes I should follow this up, thanks for asking.

Though after Markus and Igor pointed out to me that it's much more than types
of device and objects to order, I don't have a good way to fix the ordering
issue for good on all the problems; obviously current solution only applies to
device class ordering.

Examples that Markus provided:

https://lore.kernel.org/qemu-devel/87ilzj81q7@dusky.pond.sub.org/

Also there can be inter-dependency issue too for single device class, e.g., for
pci buses if bus pcie.2 has a parent pci bus of pcie.1, then we must speficy
the "-device" for pcie.1 before the "-device" of pcie.2, otherwise qemu will
fail to boot too.

Any of above examples means ordering based on device class can only solve
partial of the problems, not all.

And I can buy in with what people worry about on having yet another way to fix
ordering, since the root issue is still unsettled, even if the current solution
seems to work for vIOMMU/vfio, and I had a feeling it could work too with the
virtio-mem issue you're tackling with.

My plan is to move on with what Igor suggested, on using either pre_plug hook
for vIOMMU to make sure no special devices like vfio are realized before that.
I think it'll be as silly as a pci bus scan on all the pcie host bridges
looking for vfio-pci, it can even be put directly into realize() I think as I
don't see an obvious difference on failing pre_plug() or realize() so far.
Then I'll just drop this series so the new version may not really help with
virtio-mem anymore; though not sure virtio-mem can do similar thing.

One step back, OTOH, I do second on what Daniel commented in the other thread
on leaving that problem to the user; sad to know that we already have pmem
restriction so hot plugging some device already start to fail, but maybe
failing is fine as long as nothing will crash? :)

I also do think it's nice to at least allow the user to specify the exact value
of virtio-mem slot number without any smart tricks to be played when the user
wants - I think it's still okay to do automatic detection, but that's already
part of "policy" not "mechanism" to me so imho it should be better optional,
and now I had a feeling that maybe qemu should be good enough on providing
these mechanisms first then we leave the rest of the problems to libvirt, maybe
that's a better place to do all these sanity checks and doing smart things on

Re: [PATCH v7 02/21] target/loongarch: Add core definition

[WANG Xuerui]

On 10/21/21 11:21, Song Gao wrote:


BTW,
Account yangxiaoj...@loongson.cn It seems that she has been blacklisted. 
Xiaojuan sent 31 e-mails, which were not displayed since the 21st one, people 
who don't have a CC can't read all the emails,  and xiaojuan reply can't be in 
qemu-le...@nongnu.org.
   




Xuerui said:

"You may address the several review comments then just send v2. This way
the threading is less likely to be damaged (you need to exactly specify
In-Reply-To headers and such for the re-sent patches to correctly link
to this thread, I think it's not worth the effort). "

I think this will have the same problem.

Richard and Karl,  How can we solve this problem？


You and Xiaojuan seem to be in some kind of close cooperation; for 
example every patch from you has double Signed-off-by lines. So I 
suppose you could just switch to the intended branch and `git 
send-email` for her; proper "From:" lines will be prepended to every 
mail where Git author differs from git-send-email identity. Just 
remember not to commit/send blobs next time though.


As for the supposed "ban" on Xiaojuan's account, we cannot diagnose this 
without mailing list owners' help; maybe it was just some kind of 
automatic temporary ban, or even connectivity problem on Loongson's side.

Re: [PATCH v15 4/8] [RISCV_PM] Add J extension state description

[Alistair Francis]

On Wed, Oct 20, 2021 at 8:24 PM Alexey Baturo  wrote:
>
> Signed-off-by: Alexey Baturo 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/machine.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 16a08302da..ae82f82525 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -84,6 +84,14 @@ static bool vector_needed(void *opaque)
>  return riscv_has_ext(env, RVV);
>  }
>
> +static bool pointermasking_needed(void *opaque)
> +{
> +RISCVCPU *cpu = opaque;
> +CPURISCVState *env = >env;
> +
> +return riscv_has_ext(env, RVJ);
> +}
> +
>  static const VMStateDescription vmstate_vector = {
>  .name = "cpu/vector",
>  .version_id = 1,
> @@ -138,6 +146,24 @@ static const VMStateDescription vmstate_hyper = {
>  }
>  };
>
> +static const VMStateDescription vmstate_pointermasking = {
> +.name = "cpu/pointer_masking",
> +.version_id = 1,
> +.minimum_version_id = 1,
> +.needed = pointermasking_needed,
> +.fields = (VMStateField[]) {
> +VMSTATE_UINTTL(env.mmte, RISCVCPU),
> +VMSTATE_UINTTL(env.mpmmask, RISCVCPU),
> +VMSTATE_UINTTL(env.mpmbase, RISCVCPU),
> +VMSTATE_UINTTL(env.spmmask, RISCVCPU),
> +VMSTATE_UINTTL(env.spmbase, RISCVCPU),
> +VMSTATE_UINTTL(env.upmmask, RISCVCPU),
> +VMSTATE_UINTTL(env.upmbase, RISCVCPU),
> +
> +VMSTATE_END_OF_LIST()
> +}
> +};
> +
>  const VMStateDescription vmstate_riscv_cpu = {
>  .name = "cpu",
>  .version_id = 2,
> @@ -189,6 +215,7 @@ const VMStateDescription vmstate_riscv_cpu = {
>  _pmp,
>  _hyper,
>  _vector,
> +_pointermasking,
>  NULL
>  }
>  };
> --
> 2.30.2
>
>

Re: [PATCH v7 02/21] target/loongarch: Add core definition

[Song Gao]

Hi, all

On 10/20/2021 09:56 PM, Richard Henderson wrote:
> On 10/20/21 5:00 AM, WANG Xuerui wrote:
>> On 2021/10/20 16:54, Song Gao wrote:
>>
>>> On 10/19/2021 01:38 AM, Philippe Mathieu-Daudé wrote:
 On 10/18/21 18:06, WANG Xuerui wrote:

 On 10/18/21 20:47, Song Gao wrote:
>> +static void set_loongarch_cpucfg(CPULoongArchState *env)
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < 49; i++) {
>> +    env->cpucfg[i] = 0x0;
>> +    }
>> +    env->cpucfg[0] = 0x14c010;
>> +    env->cpucfg[1] = 0x3f2f2fe;
>> +    env->cpucfg[2] = 0x60c3cf;
>> +    env->cpucfg[3] = 0xcff;
>> +    env->cpucfg[4] = 0x5f5e100;
>> +    env->cpucfg[5] = 0x10001;
>> +    env->cpucfg[16] = 0x2c3d;
>> +    env->cpucfg[17] = 0x6080003;
>> +    env->cpucfg[18] = 0x6080003;
>> +    env->cpucfg[19] = 0x60800f;
>> +    env->cpucfg[20] = 0x60f000f;
> I know these values are taken from a real 3A5000 chip, since I have such
> a machine and I've done the experiment, but others likely wouldn't
> notice so quickly. Maybe put some comment on top of this function to
> illustrate this?
 Simpler: ...

>>> On linux-user emulation. We don't care about the value of cpucfg[i].
>>> I think we only need to set cpucfg[i] to 0. and set value on system 
>>> emulations, is that better?
>>
>> I'm afraid that wouldn't be better; actually it would be *wrong* to just
>> return zeroes for user-space CPUCFG accesses. CPUCFG is designed to
>> provide runtime feature probing like CPUID, and is usable from
>> user-space. So, one can only assume this data is being used, and
>> properly return data.
>>
>> I've heard that kernel could choose to not actually enable all features
>> for which CPUCFG indicate support, while not configuring CPUCFG values
>> to reflect that, thus making CPUCFG values unreliable; that's not a
>> proper reason to return zeroes. Kernel should be fixed to properly
>> configure CPUCFG instead. Because otherwise you wouldn't make such an
>> instruction an unprivileged one in the first place...
> 
> In the meantime, I think you really need to filter these values to those you 
> have implemented.  E.g. cpucfg[2].LASX here indicates support for the 256-bit 
> vector extension.  Similarly the COMPLEX and CRYPTO extensions.
> 
> I think you would do well to add some FIELD definitions so that these can be 
> set symbolically.
>
OK.


BTW,   

Account yangxiaoj...@loongson.cn It seems that she has been blacklisted. 
Xiaojuan sent 31 e-mails, which were not displayed since the 21st one, people 
who don't have a CC can't read all the emails,  and xiaojuan reply can't be in 
qemu-le...@nongnu.org. 
  

The follow is the return message:

抱歉，您的邮件被退回来了……/
Sorry, your mail is returned...
原邮件信息/
Original e-mail message :    
时间/Time :   2021-10-20 09:33:59
主题/Subject :Re: Re: [PATCH 00/31] Add Loongarch softmmu support.
收件人/To :qemu-devel@nongnu.org
退信原因/
Bounce reason : 
由于对方服务器不稳定，或者双方服务器网络连接质量不佳，或者防火墙过滤，而无法连接上对方服务器。
Can not connect to recipient's server because of unstable network or firewall 
filter.
rcpt handle timeout,last handle info: Can not connect to 
nongnu.org:2001:470:142:3::10:25
建议解决方案/
Proposed Solution : 

邮差温馨提示：请在稍后时间重新尝试投递，或者联系联系管理员、技术中心协助。/
Warm tips:Please try again later, or contact administrator or helpcenter 
for help.
如果您有其他退信问题，欢迎向客服中心联系/
If you have any other bounce problems, please contact customer service 
center

退信代码/
Bounce Code :   
can not connect to


This message is generated by Coremail.



Xuerui said: 

"You may address the several review comments then just send v2. This way
the threading is less likely to be damaged (you need to exactly specify
In-Reply-To headers and such for the re-sent patches to correctly link
to this thread, I think it's not worth the effort). "

I think this will have the same problem. 

Richard and Karl,  How can we solve this problem？


Thanks
Song Gao.
 
> 
> r~

RE: [PATCH V3] net/colo: check vnet_hdr_support flag when using virtio-net

[Zhang, Chen]

> -Original Message-
> From: Jason Wang 
> Sent: Thursday, October 21, 2021 11:02 AM
> To: Zhang, Chen 
> Cc: Eric Blake ; Markus Armbruster
> ; qemu-dev ; Li Zhijian
> ; Lukas Straub 
> Subject: Re: [PATCH V3] net/colo: check vnet_hdr_support flag when using
> virtio-net
> 
> On Wed, Oct 20, 2021 at 2:19 PM Zhang, Chen 
> wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Jason Wang 
> > > Sent: Wednesday, October 20, 2021 11:13 AM
> > > To: Zhang, Chen 
> > > Cc: Eric Blake ; Markus Armbruster
> > > ; qemu-dev ; Li
> Zhijian
> > > ; Lukas Straub 
> > > Subject: Re: [PATCH V3] net/colo: check vnet_hdr_support flag when
> > > using virtio-net
> > >
> > > On Wed, Oct 20, 2021 at 10:53 AM Zhang, Chen 
> > > wrote:
> > > >
> > > >
> > > >
> > > > > -Original Message-
> > > > > From: Jason Wang 
> > > > > Sent: Tuesday, October 19, 2021 3:39 PM
> > > > > To: Zhang, Chen ; Eric Blake
> > > > > ; Markus Armbruster 
> > > > > Cc: qemu-dev ; Li Zhijian
> > > > > ; Lukas Straub ;
> > > > > Tao Xu 
> > > > > Subject: Re: [PATCH V3] net/colo: check vnet_hdr_support flag
> > > > > when using virtio-net
> > > > >
> > > > >
> > > > > 在 2021/9/18 上午10:04, Zhang Chen 写道:
> > > > > > When COLO use only one vnet_hdr_support parameter between
> COLO
> > > > > network
> > > > > > filter(filter-mirror, filter-redirector or filter-rewriter and
> > > > > > colo-compare, packet will not be parsed correctly. Acquire
> > > > > > network driver related to COLO, if it is nirtio-net,
> > > > >
> > > > >
> > > > > Typo.
> > > >
> > > > Oh~ will fix in next version.
> > > >
> > > > >
> > > > >
> > > > > >   check vnet_hdr_support flag of COLO network filter and colo-
> compare.
> > > > > >
> > > > > > Signed-off-by: Tao Xu 
> > > > > > Signed-off-by: Zhang Chen 
> > > > > > ---
> > > > > >
> > > > > > Changelog:
> > > > > > v3:
> > > > > >  Fix some typos.
> > > > > >  Rebased for Qemu 6.2.
> > > > > >
> > > > > > v2:
> > > > > >  Detect virtio-net driver and apply vnet_hdr_support
> > > > > >  automatically. (Jason)
> > > > > > ---
> > > > > >   net/colo-compare.c| 57
> > > > > +++
> > > > > >   net/colo.c| 20 +++
> > > > > >   net/colo.h|  4 +++
> > > > > >   net/filter-mirror.c   | 21 
> > > > > >   net/filter-rewriter.c | 10 
> > > > > >   qapi/qom.json |  6 +
> > > > > >   qemu-options.hx   |  6 +++--
> > > > > >   7 files changed, 122 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > > > > > b100e7b51f..870bd05a41 100644
> > > > > > --- a/net/colo-compare.c
> > > > > > +++ b/net/colo-compare.c
> > > > > > @@ -110,6 +110,7 @@ struct CompareState {
> > > > > >   char *sec_indev;
> > > > > >   char *outdev;
> > > > > >   char *notify_dev;
> > > > > > +char *netdev;
> > > > > >   CharBackend chr_pri_in;
> > > > > >   CharBackend chr_sec_in;
> > > > > >   CharBackend chr_out;
> > > > > > @@ -838,6 +839,28 @@ static int compare_chr_can_read(void
> *opaque)
> > > > > >   return COMPARE_READ_LEN_MAX;
> > > > > >   }
> > > > > >
> > > > > > +static int colo_set_default_netdev(void *opaque, QemuOpts
> > > > > > +*opts, Error **errp) {
> > > > > > +const char *colo_obj_type, *netdev_from_filter;
> > > > > > +char **netdev = (char **)opaque;
> > > > > > +
> > > > > > +colo_obj_type = qemu_opt_get(opts, "qom-type");
> > > > > > +
> > > > > > +if (colo_obj_type &&
> > > > > > +(strcmp(colo_obj_type, "filter-mirror") == 0 ||
> > > > > > + strcmp(colo_obj_type, "filter-redirector") == 0 ||
> > > > > > + strcmp(colo_obj_type, "filter-rewriter") == 0)) {
> > > > > > +netdev_from_filter = qemu_opt_get(opts, "netdev");
> > > > > > +if (*netdev == NULL) {
> > > > > > +*netdev = g_strdup(netdev_from_filter);
> > > > > > +} else if (strcmp(*netdev, netdev_from_filter) != 0) {
> > > > > > +warn_report("%s is using a different netdev from other
> COLO "
> > > > > > +"component", colo_obj_type);
> > > > > > +}
> > > > > > +}
> > > > > > +return 0;
> > > > > > +}
> > > > > > +
> > > > > >   /*
> > > > > >* Called from the main thread on the primary for packets
> > > > > >* arriving over the socket from the primary.
> > > > > > @@ -1050,6 +1073,21 @@ static void
> compare_set_vnet_hdr(Object
> > > *obj,
> > > > > >   s->vnet_hdr = value;
> > > > > >   }
> > > > > >
> > > > > > +static char *compare_get_netdev(Object *obj, Error **errp) {
> > > > > > +CompareState *s = COLO_COMPARE(obj);
> > > > > > +
> > > > > > +return g_strdup(s->netdev); }
> > > > > > +
> > > > > > +static void compare_set_netdev(Object *obj, const char
> > > > > > +*value, Error
> > > > > > +**errp) {
> > > > > > +CompareState *s = COLO_COMPARE(obj);
> > > > > > +
> > > > > > 

Re: [PATCH V3] net/colo: check vnet_hdr_support flag when using virtio-net

[Jason Wang]

On Wed, Oct 20, 2021 at 2:19 PM Zhang, Chen  wrote:
>
>
>
> > -Original Message-
> > From: Jason Wang 
> > Sent: Wednesday, October 20, 2021 11:13 AM
> > To: Zhang, Chen 
> > Cc: Eric Blake ; Markus Armbruster
> > ; qemu-dev ; Li Zhijian
> > ; Lukas Straub 
> > Subject: Re: [PATCH V3] net/colo: check vnet_hdr_support flag when using
> > virtio-net
> >
> > On Wed, Oct 20, 2021 at 10:53 AM Zhang, Chen 
> > wrote:
> > >
> > >
> > >
> > > > -Original Message-
> > > > From: Jason Wang 
> > > > Sent: Tuesday, October 19, 2021 3:39 PM
> > > > To: Zhang, Chen ; Eric Blake
> > > > ; Markus Armbruster 
> > > > Cc: qemu-dev ; Li Zhijian
> > > > ; Lukas Straub ; Tao
> > > > Xu 
> > > > Subject: Re: [PATCH V3] net/colo: check vnet_hdr_support flag when
> > > > using virtio-net
> > > >
> > > >
> > > > 在 2021/9/18 上午10:04, Zhang Chen 写道:
> > > > > When COLO use only one vnet_hdr_support parameter between COLO
> > > > network
> > > > > filter(filter-mirror, filter-redirector or filter-rewriter and
> > > > > colo-compare, packet will not be parsed correctly. Acquire network
> > > > > driver related to COLO, if it is nirtio-net,
> > > >
> > > >
> > > > Typo.
> > >
> > > Oh~ will fix in next version.
> > >
> > > >
> > > >
> > > > >   check vnet_hdr_support flag of COLO network filter and colo-compare.
> > > > >
> > > > > Signed-off-by: Tao Xu 
> > > > > Signed-off-by: Zhang Chen 
> > > > > ---
> > > > >
> > > > > Changelog:
> > > > > v3:
> > > > >  Fix some typos.
> > > > >  Rebased for Qemu 6.2.
> > > > >
> > > > > v2:
> > > > >  Detect virtio-net driver and apply vnet_hdr_support
> > > > >  automatically. (Jason)
> > > > > ---
> > > > >   net/colo-compare.c| 57
> > > > +++
> > > > >   net/colo.c| 20 +++
> > > > >   net/colo.h|  4 +++
> > > > >   net/filter-mirror.c   | 21 
> > > > >   net/filter-rewriter.c | 10 
> > > > >   qapi/qom.json |  6 +
> > > > >   qemu-options.hx   |  6 +++--
> > > > >   7 files changed, 122 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/net/colo-compare.c b/net/colo-compare.c index
> > > > > b100e7b51f..870bd05a41 100644
> > > > > --- a/net/colo-compare.c
> > > > > +++ b/net/colo-compare.c
> > > > > @@ -110,6 +110,7 @@ struct CompareState {
> > > > >   char *sec_indev;
> > > > >   char *outdev;
> > > > >   char *notify_dev;
> > > > > +char *netdev;
> > > > >   CharBackend chr_pri_in;
> > > > >   CharBackend chr_sec_in;
> > > > >   CharBackend chr_out;
> > > > > @@ -838,6 +839,28 @@ static int compare_chr_can_read(void *opaque)
> > > > >   return COMPARE_READ_LEN_MAX;
> > > > >   }
> > > > >
> > > > > +static int colo_set_default_netdev(void *opaque, QemuOpts *opts,
> > > > > +Error **errp) {
> > > > > +const char *colo_obj_type, *netdev_from_filter;
> > > > > +char **netdev = (char **)opaque;
> > > > > +
> > > > > +colo_obj_type = qemu_opt_get(opts, "qom-type");
> > > > > +
> > > > > +if (colo_obj_type &&
> > > > > +(strcmp(colo_obj_type, "filter-mirror") == 0 ||
> > > > > + strcmp(colo_obj_type, "filter-redirector") == 0 ||
> > > > > + strcmp(colo_obj_type, "filter-rewriter") == 0)) {
> > > > > +netdev_from_filter = qemu_opt_get(opts, "netdev");
> > > > > +if (*netdev == NULL) {
> > > > > +*netdev = g_strdup(netdev_from_filter);
> > > > > +} else if (strcmp(*netdev, netdev_from_filter) != 0) {
> > > > > +warn_report("%s is using a different netdev from other 
> > > > > COLO "
> > > > > +"component", colo_obj_type);
> > > > > +}
> > > > > +}
> > > > > +return 0;
> > > > > +}
> > > > > +
> > > > >   /*
> > > > >* Called from the main thread on the primary for packets
> > > > >* arriving over the socket from the primary.
> > > > > @@ -1050,6 +1073,21 @@ static void compare_set_vnet_hdr(Object
> > *obj,
> > > > >   s->vnet_hdr = value;
> > > > >   }
> > > > >
> > > > > +static char *compare_get_netdev(Object *obj, Error **errp) {
> > > > > +CompareState *s = COLO_COMPARE(obj);
> > > > > +
> > > > > +return g_strdup(s->netdev);
> > > > > +}
> > > > > +
> > > > > +static void compare_set_netdev(Object *obj, const char *value,
> > > > > +Error
> > > > > +**errp) {
> > > > > +CompareState *s = COLO_COMPARE(obj);
> > > > > +
> > > > > +g_free(s->netdev);
> > > > > +s->netdev = g_strdup(value);
> > > > > +}
> > > > > +
> > > > >   static char *compare_get_notify_dev(Object *obj, Error **errp)
> > > > >   {
> > > > >   CompareState *s = COLO_COMPARE(obj); @@ -1274,6 +1312,12
> > @@
> > > > > static void colo_compare_complete(UserCreatable *uc, Error **errp)
> > > > >   max_queue_size = MAX_QUEUE_SIZE;
> > > > >   }
> > > > >
> > > > > +if (!s->netdev) {
> > > > > +/* Set default netdev as the first colo netfilter found */
> 

Re: [RFC PATCH v4 20/20] vdpa: Add custom IOTLB translations to SVQ

[Jason Wang]

On Wed, Oct 20, 2021 at 7:57 PM Eugenio Perez Martin
 wrote:
>
> On Wed, Oct 20, 2021 at 11:03 AM Jason Wang  wrote:
> >
> > On Wed, Oct 20, 2021 at 2:52 PM Eugenio Perez Martin
> >  wrote:
> > >
> > > On Wed, Oct 20, 2021 at 4:07 AM Jason Wang  wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 10:02 AM Jason Wang  wrote:
> > > > >
> > > > > On Tue, Oct 19, 2021 at 6:29 PM Eugenio Perez Martin
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, Oct 19, 2021 at 11:25 AM Jason Wang  
> > > > > > wrote:
> > > > > > >
> > > > > > >
> > > > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > > > > Use translations added in VhostIOVATree in SVQ.
> > > > > > > >
> > > > > > > > Now every element needs to store the previous address also, so 
> > > > > > > > VirtQueue
> > > > > > > > can consume the elements properly. This adds a little overhead 
> > > > > > > > per VQ
> > > > > > > > element, having to allocate more memory to stash them. As a 
> > > > > > > > possible
> > > > > > > > optimization, this allocation could be avoided if the 
> > > > > > > > descriptor is not
> > > > > > > > a chain but a single one, but this is left undone.
> > > > > > > >
> > > > > > > > TODO: iova range should be queried before, and add logic to 
> > > > > > > > fail when
> > > > > > > > GPA is outside of its range and memory listener or svq add it.
> > > > > > > >
> > > > > > > > Signed-off-by: Eugenio Pérez 
> > > > > > > > ---
> > > > > > > >   hw/virtio/vhost-shadow-virtqueue.h |   4 +-
> > > > > > > >   hw/virtio/vhost-shadow-virtqueue.c | 130 
> > > > > > > > -
> > > > > > > >   hw/virtio/vhost-vdpa.c |  40 -
> > > > > > > >   hw/virtio/trace-events |   1 +
> > > > > > > >   4 files changed, 152 insertions(+), 23 deletions(-)
> > > > > > > >
> > > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
> > > > > > > > b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > index b7baa424a7..a0e6b5267a 100644
> > > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.h
> > > > > > > > @@ -11,6 +11,7 @@
> > > > > > > >   #define VHOST_SHADOW_VIRTQUEUE_H
> > > > > > > >
> > > > > > > >   #include "hw/virtio/vhost.h"
> > > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > > >
> > > > > > > >   typedef struct VhostShadowVirtqueue VhostShadowVirtqueue;
> > > > > > > >
> > > > > > > > @@ -28,7 +29,8 @@ bool vhost_svq_start(struct vhost_dev *dev, 
> > > > > > > > unsigned idx,
> > > > > > > >   void vhost_svq_stop(struct vhost_dev *dev, unsigned idx,
> > > > > > > >   VhostShadowVirtqueue *svq);
> > > > > > > >
> > > > > > > > -VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int 
> > > > > > > > idx);
> > > > > > > > +VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, int 
> > > > > > > > idx,
> > > > > > > > +VhostIOVATree *iova_map);
> > > > > > > >
> > > > > > > >   void vhost_svq_free(VhostShadowVirtqueue *vq);
> > > > > > > >
> > > > > > > > diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> > > > > > > > b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > index 2fd0bab75d..9db538547e 100644
> > > > > > > > --- a/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > +++ b/hw/virtio/vhost-shadow-virtqueue.c
> > > > > > > > @@ -11,12 +11,19 @@
> > > > > > > >   #include "hw/virtio/vhost-shadow-virtqueue.h"
> > > > > > > >   #include "hw/virtio/vhost.h"
> > > > > > > >   #include "hw/virtio/virtio-access.h"
> > > > > > > > +#include "hw/virtio/vhost-iova-tree.h"
> > > > > > > >
> > > > > > > >   #include "standard-headers/linux/vhost_types.h"
> > > > > > > >
> > > > > > > >   #include "qemu/error-report.h"
> > > > > > > >   #include "qemu/main-loop.h"
> > > > > > > >
> > > > > > > > +typedef struct SVQElement {
> > > > > > > > +VirtQueueElement elem;
> > > > > > > > +void **in_sg_stash;
> > > > > > > > +void **out_sg_stash;
> > > > > > > > +} SVQElement;
> > > > > > > > +
> > > > > > > >   /* Shadow virtqueue to relay notifications */
> > > > > > > >   typedef struct VhostShadowVirtqueue {
> > > > > > > >   /* Shadow vring */
> > > > > > > > @@ -46,8 +53,11 @@ typedef struct VhostShadowVirtqueue {
> > > > > > > >   /* Virtio device */
> > > > > > > >   VirtIODevice *vdev;
> > > > > > > >
> > > > > > > > +/* IOVA mapping if used */
> > > > > > > > +VhostIOVATree *iova_map;
> > > > > > > > +
> > > > > > > >   /* Map for returning guest's descriptors */
> > > > > > > > -VirtQueueElement **ring_id_maps;
> > > > > > > > +SVQElement **ring_id_maps;
> > > > > > > >
> > > > > > > >   /* Next head to expose to device */
> > > > > > > >   uint16_t avail_idx_shadow;
> > > > > > > > @@ -79,13 +89,6 @@ bool 
> > > > > > > > vhost_svq_valid_device_features(uint64_t *dev_features)
> > > > > > > >   continue;
> > > > > > > >
> > > > > > > >   case VIRTIO_F_ACCESS_PLATFORM:
> > > > > > 

Re: [RFC PATCH v4 18/20] vhost: Add VhostIOVATree

[Jason Wang]

On Wed, Oct 20, 2021 at 8:07 PM Eugenio Perez Martin
 wrote:
>
> On Wed, Oct 20, 2021 at 11:01 AM Jason Wang  wrote:
> >
> > On Wed, Oct 20, 2021 at 3:54 PM Eugenio Perez Martin
> >  wrote:
> > >
> > > On Tue, Oct 19, 2021 at 11:23 AM Jason Wang  wrote:
> > > >
> > > > On Tue, Oct 19, 2021 at 4:32 PM Jason Wang  wrote:
> > > > >
> > > > >
> > > > > 在 2021/10/1 下午3:06, Eugenio Pérez 写道:
> > > > > > This tree is able to look for a translated address from an IOVA 
> > > > > > address.
> > > > > >
> > > > > > At first glance is similar to util/iova-tree. However, SVQ working 
> > > > > > on
> > > > > > devices with limited IOVA space need more capabilities, like 
> > > > > > allocating
> > > > > > IOVA chunks or perform reverse translations (qemu addresses to 
> > > > > > iova).
> > > > >
> > > > >
> > > > > I don't see any reverse translation is used in the shadow code. Or
> > > > > anything I missed?
> > > >
> > > > Ok, it looks to me that it is used in the iova allocator. But I think
> > > > it's better to decouple it to an independent allocator instead of
> > > > vhost iova tree.
> > > >
> > >
> > > Reverse translation is used every time a buffer is made available,
> > > since buffers content are not copied, only the descriptors to SVQ
> > > vring.
> >
> > I may miss something but I didn't see the code? Qemu knows the VA of
> > virtqueue, and the VA of the VQ is stored in the VirtQueueElem?
> >
>
> It's used in the patch 20/20, could that be the misunderstanding? The
> function calling it is vhost_svq_translate_addr.
>
> Qemu knows the VA address of the buffer, but it must offer a valid SVQ
> iova to the device. That is the translation I mean.

Ok, I get you. So if I understand correctly, what you did is:

1) allocate IOVA during region_add
2) preform VA->IOVA reverse lookup in handle_kick

This should be fine, but here're some suggestions:

1) remove the assert(map) in vhost_svq_translate_addr() since guest
can add e.g BAR address
2) we probably need a better name vhost_iova_tree_alloc(), maybe
"vhost_iova_tree_map_alloc()"

There's actually another method.

1) don't do IOVA/map allocation in region_add()
2) do the allocation in handle_kick(), then we know the IOVA so no
reverse lookup

The advantage is that this can work for the case of vIOMMU. And they
should perform the same:

1) you method avoid the iova allocation per sg
2) my method avoid the reverse lookup per sg

>
> > >
> > > At this point all the limits are copied to vhost iova tree in the next
> > > revision I will send, defined at its creation at
> > > vhost_iova_tree_new(). They are outside of util/iova-tree, only sent
> > > to the latter at allocation time.
> > >
> > > Since vhost_iova_tree has its own vhost_iova_tree_alloc(), that wraps
> > > the iova_tree_alloc() [1], limits could be kept in vhost-vdpa and make
> > > them an argument of vhost_iova_tree_alloc. But I'm not sure if it's
> > > what you are proposing or I'm missing something.
> >
> > If the reverse translation is only used in iova allocation, I meant to
> > split the logic of IOVA allocation itself.
> >
>
> Still don't understand it, sorry :). In SVQ setup we allocate an iova
> address for every guest's GPA address its driver can use. After that
> there should be no allocation unless memory is hotplugged.
>
> So the limits are only needed precisely at allocation time. Not sure
> if that is what you mean here, but to first allocate and then check if
> it is within the range could lead to false negatives, since there
> could be a valid range *in* the address but the iova allocator
> returned us another range that fell outside the range. How could we
> know the cause if it is not using the range itself?

See my above reply. And we can teach the iova allocator to return the
IOVA in the range that vhost-vDPA supports.

Thanks

>
> > >
> > > Either way, I think it is harder to talk about this specific case
> > > without code, since this one still does not address the limits. Would
> > > you prefer me to send another RFC in WIP quality, with *not* all
> > > comments addressed? I would say that there is not a lot of pending
> > > work to send the next one, but it might be easier for all of us.
> >
> > I'd prefer to try to address them all, otherwise it's not easy to see
> > what is missing.
> >
>
> Got it, I will do it that way then!
>
> Thanks!
>
> > Thanks
> >
> > >
> > > Thanks!
> > >
> > > [1] This util/iova-tree method will be proposed in the next series,
> > > and vhost_iova_tree wraps it since it needs to keep in sync both
> > > trees: iova->qemu vaddr for iova allocation and the reverse one to
> > > translate available buffers.
> > >
> > > > Thanks
> > > >
> > >
> >
>

Re: [PATCH v2 17/48] tcg/optimize: Split out fold_brcond2

[Richard Henderson]

On 10/20/21 3:27 PM, Luis Fernando Fujita Pires wrote:

From: Richard Henderson 

Reduce some code duplication by folding the NE and EQ cases.

Signed-off-by: Richard Henderson 
---
  tcg/optimize.c | 161 +
  1 file changed, 83 insertions(+), 78 deletions(-)



+case TCG_COND_NE:
+inv = 1;
+QEMU_FALLTHROUGH;
+case TCG_COND_EQ:
+/*
+ * Simplify EQ/NE comparisons where one of the pairs
+ * can be simplified.
+ */
+i = do_constant_folding_cond(INDEX_op_brcond_i32, op->args[0],
+ op->args[2], cond);
+switch (i ^ inv) {
+case 0:
+goto do_brcond_false;


I believe this should go to do_brcond_true when cond==TCG_COND_NE.


Good eyes, thanks.
This needs to be more like setcond2, with do_brcond_const.

r~

Re: [PATCH] Fix constant folding logic of setcond2_i32

[Liren Wei]

On 10/21/21 10:13, Liren Wei wrote:

For setcond2_i32 DST, A_low, A_high, B_low, B_high, TCG_COND_EQ,
DST should be 0 as long as either half of A and B are not equal.

Signed-off-by: Liren Wei 
---
  tcg/optimize.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index c239c3bd07..45a10e5e72 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -1492,7 +1492,7 @@ void tcg_optimize(TCGContext *s)
 op->args[2], op->args[4],
 TCG_COND_EQ);
  if (tmp == 0) {
-goto do_setcond_high;
+goto do_setcond_const;
  } else if (tmp != 1) {
  goto do_default;
  }
Just noticed that this has been fixed in new patch series by others, 
please ignore this. Sorry about that...

[PATCH] Fix constant folding logic of setcond2_i32

[Liren Wei]

For setcond2_i32 DST, A_low, A_high, B_low, B_high, TCG_COND_EQ,
DST should be 0 as long as either half of A and B are not equal.

Signed-off-by: Liren Wei 
---
 tcg/optimize.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index c239c3bd07..45a10e5e72 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -1492,7 +1492,7 @@ void tcg_optimize(TCGContext *s)
op->args[2], op->args[4],
TCG_COND_EQ);
 if (tmp == 0) {
-goto do_setcond_high;
+goto do_setcond_const;
 } else if (tmp != 1) {
 goto do_default;
 }
-- 
2.33.0

[Bug 1947933] [NEW] xHCI Port Status Change Event at port powered

[Benjamin David Lunt]

Public bug reported:

Per section 4.19.3 of the xHCI version 1.0 specification, when the Port
Power bit transitions from 0 to 1, if there is a connection on that
port, a Port Status Change Event should be issued.

Currently, when the port is powered, this event is not being issued.

I don't know the QEMU code base well enough to submit a patch, but I
believe that when the port is powered, check to see if there is a
connection (hence, setting the CCS and CSC bits), and then a call to:

xhci_port_notify(port, PORTSC_PLC);

will suffice.

Thank you,
Ben

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  xHCI Port Status Change Event at port powered

Status in QEMU:
  New

Bug description:
  Per section 4.19.3 of the xHCI version 1.0 specification, when the
  Port Power bit transitions from 0 to 1, if there is a connection on
  that port, a Port Status Change Event should be issued.

  Currently, when the port is powered, this event is not being issued.

  I don't know the QEMU code base well enough to submit a patch, but I
  believe that when the port is powered, check to see if there is a
  connection (hence, setting the CCS and CSC bits), and then a call to:

  xhci_port_notify(port, PORTSC_PLC);

  will suffice.

  Thank you,
  Ben

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

Re: [PATCH v2 09/48] tcg/optimize: Drop nb_oargs, nb_iargs locals

[Richard Henderson]

On 10/20/21 9:17 AM, Alex Bennée wrote:

+int nb_oargs = def->nb_oargs;
  for (i = 0; i < nb_oargs; i++) {


nit: couldn't you just do for (i = 0; i < deb->nb_oargs; i++) or is that
too much for the compiler to wrap it's head around?'


That leaves the compiler with non-invariant loop bounds, because memory clobbering inside 
the loop.



r~

Re: [PATCH v2 08/48] tcg/optimize: Split out fold_call

[Richard Henderson]

On 10/20/21 9:05 AM, Alex Bennée wrote:


Richard Henderson  writes:


Calls are special in that they have a variable number
of arguments, and need to be able to clobber globals.

Signed-off-by: Richard Henderson 
---
  tcg/optimize.c | 63 --
  1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/tcg/optimize.c b/tcg/optimize.c
index fad6f5de1f..74b9aa025a 100644
--- a/tcg/optimize.c
+++ b/tcg/optimize.c
@@ -624,10 +624,42 @@ static void copy_propagate(OptContext *ctx, TCGOp *op,
  }
  }
  
+static bool fold_call(OptContext *ctx, TCGOp *op)

+{
+TCGContext *s = ctx->tcg;
+int nb_oargs = TCGOP_CALLO(op);
+int nb_iargs = TCGOP_CALLI(op);
+int flags, i;
+
+init_arguments(ctx, op, nb_oargs + nb_iargs);
+copy_propagate(ctx, op, nb_oargs, nb_iargs);
+
+/* If the function reads or writes globals, reset temp data. */
+flags = tcg_call_flags(op);
+if (!(flags & (TCG_CALL_NO_READ_GLOBALS | TCG_CALL_NO_WRITE_GLOBALS))) {
+int nb_globals = s->nb_globals;


Aren't we missing a trick here?

If the helper is going to read global registers we need to ensure any
temps holding their data is written but we don't need to throw the
existing temps away if the helper is TCG_CALL_NO_WRITE_GLOBALS?


Hmm, if NO_WRITE_GLOBALS, then yes, we should be able to preserve the information about 
the current contents.  There must be some quirk, though; I just don't recall what it is. 
This patch preserves existing behaviour.



r~

Re: [PATCH v3 0/3] tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges for q35

[Ani Sinha]

On Wed, Oct 20, 2021 at 2:09 PM Michael S. Tsirkin  wrote:

> On Thu, Oct 07, 2021 at 07:27:47PM +0530, Ani Sinha wrote:
> > changelist:
> > v3: removed "nodefaults" from the command line and rebased the patchset.
> > v2: incorporated some of the feedbacks from Igor.
> > v1 : initial RFC patch.
>
> This seems to break on s390 hosts for people. Likely an
> endian-ness bug somewhere. Dropped for now - care tracking that down
> and fixing so I can pick up the test again?
>
> Thanks!


So I take it this patch wasn't causing the issue since this has been merged
to master now?


>
> > This patchset adds a unit test to exercize acpi hotplug support for
> multifunction
> > bridges on q35 machines. This support was added with the commit:
> >
> > d7346e614f4ec ("acpi: x86: pcihp: add support hotplug on multifunction
> bridges")
> >
> > Ani Sinha (3):
> >   tests/acpi/bios-tables-test: add and allow changes to a new q35 DSDT
> > table blob
> >   tests/acpi/pcihp: add unit tests for hotplug on multifunction bridges
> > for q35
> >   tests/acpi/bios-tables-test: update DSDT blob for multifunction bridge
> > test
> >
> >  tests/data/acpi/q35/DSDT.multi-bridge | Bin 0 -> 8583 bytes
> >  tests/qtest/bios-tables-test.c|  18 ++
> >  2 files changed, 18 insertions(+)
> >  create mode 100644 tests/data/acpi/q35/DSDT.multi-bridge
> >
> > --
> > 2.25.1
>
>

Re: [PATCH v2 00/48] tcg: optimize redundant sign extensions

[Richard Henderson]

On 10/20/21 9:13 AM, Alex Bennée wrote:


Richard Henderson  writes:


Currently, we have support for optimizing redundant zero extensions,
which I think was done with x86 and aarch64 in mind, which zero-extend
all 32-bit operations into the 64-bit register.

But targets like Alpha, MIPS, and RISC-V do sign-extensions instead.
The last 5 patches address this.

But before that, split the quite massive tcg_optimize function.


BTW this reminded me of a discussion I was having on another thread:

   Subject: Re: TCG Floating Point Support (Work in Progress)
   Date: Fri, 01 Oct 2021 09:03:41 +0100
   In-reply-to: 

   Message-ID: <87y27d5ezt@linaro.org>

about a test harness of TCG. With the changes over the years are we any
closer to being able to lift the TCG code into a unit test so we can add
test cases that exercise and validate the optimiser decisions?


Nope.

I'm not even sure true unit testing is worthwhile.
It would require inventing a "tcg front end", parser, etc.

I could imagine, perhaps, something in which we input real asm and look at the optimized 
opcode dump.  E.g. for x86_64,


_start:
mov %eax, %ebx
mov %ebx, %ecx
hlt

should contain only one ext32u_i64 opcode.


r~

Re: [PATCH v4 8/8] target/riscv: zfh: add Zfhmin cpu property

[Alistair Francis]

On Thu, Oct 21, 2021 at 9:25 AM Alistair Francis  wrote:
>
> On Wed, Oct 20, 2021 at 1:13 PM  wrote:
> >
> > From: Frank Chang 
> >
> > Signed-off-by: Frank Chang 
>
> Reviewed-by: Alistair Francis 
>
> Alistair
>
> > ---
> >  target/riscv/cpu.c | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 8c579dc297b..4c0e6532164 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -602,6 +602,7 @@ static Property riscv_cpu_properties[] = {
> >  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
> >  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> >  DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> > +DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),

Do you mind rebasing this on
https://github.com/alistair23/qemu/tree/riscv-to-apply.next

Alistair

> >  DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> >  DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
> >  DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> > --
> > 2.25.1
> >
> >

Re: [PATCH v4 8/8] target/riscv: zfh: add Zfhmin cpu property

[Alistair Francis]

On Wed, Oct 20, 2021 at 1:13 PM  wrote:
>
> From: Frank Chang 
>
> Signed-off-by: Frank Chang 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 8c579dc297b..4c0e6532164 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -602,6 +602,7 @@ static Property riscv_cpu_properties[] = {
>  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>  DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
> +DEFINE_PROP_BOOL("Zfhmin", RISCVCPU, cfg.ext_zfhmin, false),
>  DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
>  DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
>  DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> --
> 2.25.1
>
>

Re: [PATCH v4 6/8] target/riscv: zfh: add Zfh cpu property

[Alistair Francis]

On Wed, Oct 20, 2021 at 1:15 PM  wrote:
>
> From: Frank Chang 
>
> Signed-off-by: Frank Chang 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1d69d1887e6..8c579dc297b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -601,6 +601,7 @@ static Property riscv_cpu_properties[] = {
>  DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true),
>  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
> +DEFINE_PROP_BOOL("Zfh", RISCVCPU, cfg.ext_zfh, false),
>  DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
>  DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
>  DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> --
> 2.25.1
>
>

Re: [PATCH v4 2/2] target/riscv: change the api for RVF/RVD fmin/fmax

[Alistair Francis]

On Sat, Oct 16, 2021 at 6:55 PM  wrote:
>
> From: Chih-Min Chao 
>
> The sNaN propagation behavior has been changed since
> cd20cee7 in https://github.com/riscv/riscv-isa-manual.

It would be a good idea to justify why we are using the priv spec for
the version check.

>
> Signed-off-by: Chih-Min Chao 
> Signed-off-by: Frank Chang 

Acked-by: Alistair Francis 

Alistair

> ---
>  target/riscv/fpu_helper.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/fpu_helper.c b/target/riscv/fpu_helper.c
> index 8700516a14c..d62f4709002 100644
> --- a/target/riscv/fpu_helper.c
> +++ b/target/riscv/fpu_helper.c
> @@ -174,14 +174,18 @@ uint64_t helper_fmin_s(CPURISCVState *env, uint64_t 
> rs1, uint64_t rs2)
>  {
>  float32 frs1 = check_nanbox_s(rs1);
>  float32 frs2 = check_nanbox_s(rs2);
> -return nanbox_s(float32_minnum(frs1, frs2, >fp_status));
> +return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
> +float32_minnum(frs1, frs2, >fp_status) :
> +float32_minimum_number(frs1, frs2, >fp_status));
>  }
>
>  uint64_t helper_fmax_s(CPURISCVState *env, uint64_t rs1, uint64_t rs2)
>  {
>  float32 frs1 = check_nanbox_s(rs1);
>  float32 frs2 = check_nanbox_s(rs2);
> -return nanbox_s(float32_maxnum(frs1, frs2, >fp_status));
> +return nanbox_s(env->priv_ver < PRIV_VERSION_1_11_0 ?
> +float32_maxnum(frs1, frs2, >fp_status) :
> +float32_maximum_number(frs1, frs2, >fp_status));
>  }
>
>  uint64_t helper_fsqrt_s(CPURISCVState *env, uint64_t rs1)
> @@ -283,12 +287,16 @@ uint64_t helper_fdiv_d(CPURISCVState *env, uint64_t 
> frs1, uint64_t frs2)
>
>  uint64_t helper_fmin_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>  {
> -return float64_minnum(frs1, frs2, >fp_status);
> +return env->priv_ver < PRIV_VERSION_1_11_0 ?
> +float64_minnum(frs1, frs2, >fp_status) :
> +float64_minimum_number(frs1, frs2, >fp_status);
>  }
>
>  uint64_t helper_fmax_d(CPURISCVState *env, uint64_t frs1, uint64_t frs2)
>  {
> -return float64_maxnum(frs1, frs2, >fp_status);
> +return env->priv_ver < PRIV_VERSION_1_11_0 ?
> +float64_maxnum(frs1, frs2, >fp_status) :
> +float64_maximum_number(frs1, frs2, >fp_status);
>  }
>
>  uint64_t helper_fcvt_s_d(CPURISCVState *env, uint64_t rs1)
> --
> 2.25.1
>
>

Re: [PATCH v2 6/6] hw/riscv: spike: Use MachineState::ram and MachineClass::default_ram_id

[Alistair Francis]

On Wed, Oct 20, 2021 at 11:43 AM Bin Meng  wrote:
>
> Using memory_region_init_ram(), which can't possibly handle vhost-user,
> and can't work as expected with '-numa node,memdev' options.
>
> Use MachineState::ram instead of manually initializing RAM memory
> region, as well as by providing MachineClass::default_ram_id to
> opt in to memdev scheme.
>
> Signed-off-by: Bin Meng 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Igor Mammedov 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> (no changes since v1)
>
>  hw/riscv/spike.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 79ae355ae2..288d69cd9f 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -180,7 +180,6 @@ static void spike_board_init(MachineState *machine)
>  const MemMapEntry *memmap = spike_memmap;
>  SpikeState *s = SPIKE_MACHINE(machine);
>  MemoryRegion *system_memory = get_system_memory();
> -MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>  MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
>  target_ulong firmware_end_addr, kernel_start_addr;
>  uint32_t fdt_load_addr;
> @@ -239,10 +238,8 @@ static void spike_board_init(MachineState *machine)
>  }
>
>  /* register system main memory (actual RAM) */
> -memory_region_init_ram(main_mem, NULL, "riscv.spike.ram",
> -   machine->ram_size, _fatal);
>  memory_region_add_subregion(system_memory, memmap[SPIKE_DRAM].base,
> -main_mem);
> +machine->ram);
>
>  /* create device tree */
>  create_fdt(s, memmap, machine->ram_size, machine->kernel_cmdline,
> @@ -326,6 +323,7 @@ static void spike_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->cpu_index_to_instance_props = riscv_numa_cpu_index_to_props;
>  mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id;
>  mc->numa_mem_supported = true;
> +mc->default_ram_id = "riscv.spike.ram";
>  }
>
>  static const TypeInfo spike_machine_typeinfo = {
> --
> 2.25.1
>
>

Re: [PATCH v3 20/21] target/riscv: adding 128-bit access functions for some csrs

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

+static RISCVException read_mstatus_i128(CPURISCVState *env, int csrno,
+   Int128 *val)
+{
+*val = int128_make128(env->mstatus, env->mstatush);
+return RISCV_EXCP_NONE;
+}


Needs updating from split SD bit.  I suggest

uint64_t val64;
read_mstatus(env, CSR_MSTATUS, );
*val = int128_make128(val64 & MSTATUS64_SD, val64 & MSTATUS64_SD);


+static RISCVException write_mstatus_i128(CPURISCVState *env, int csrno,
+Int128 val)
+{

...

+dirty = ((int128_getlo(mstatus) & MSTATUS_FS) == MSTATUS_FS) |
+((int128_getlo(mstatus) & MSTATUS_XS) == MSTATUS_XS);
+if (dirty) {
+if (riscv_cpu_is_32bit(env)) {
+mstatus = int128_make64(int128_getlo(mstatus) | MSTATUS32_SD);
+} else if (riscv_cpu_is_64bit(env)) {
+mstatus = int128_make64(int128_getlo(mstatus) | MSTATUS64_SD);
+} else {
+mstatus = int128_or(mstatus, int128_make128(0, MSTATUSH128_SD));
+}
+}


Needs updating for change to SD.
Now you can defer everything to the 64-bit write_mstatus.


r~

Re: [PATCH v2 4/6] hw/riscv: sifive_e: Use MachineState::ram and MachineClass::default_ram_id

[Alistair Francis]

On Wed, Oct 20, 2021 at 11:42 AM Bin Meng  wrote:
>
> Using memory_region_init_ram(), which can't possibly handle vhost-user,
> and can't work as expected with '-numa node,memdev' options.
>
> Use MachineState::ram instead of manually initializing RAM memory
> region, as well as by providing MachineClass::default_ram_id to
> opt in to memdev scheme.
>
> While at it add check for user supplied RAM size and error out if it
> mismatches board expected value.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> Changes in v2:
> - add RAM size check
> - assign mc->default_ram_size
>
>  hw/riscv/sifive_e.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
> index 6e95ea5896..9b206407a6 100644
> --- a/hw/riscv/sifive_e.c
> +++ b/hw/riscv/sifive_e.c
> @@ -29,6 +29,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qemu/error-report.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
> @@ -71,22 +72,27 @@ static const MemMapEntry sifive_e_memmap[] = {
>
>  static void sifive_e_machine_init(MachineState *machine)
>  {
> +MachineClass *mc = MACHINE_GET_CLASS(machine);
>  const MemMapEntry *memmap = sifive_e_memmap;
>
>  SiFiveEState *s = RISCV_E_MACHINE(machine);
>  MemoryRegion *sys_mem = get_system_memory();
> -MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>  int i;
>
> +if (machine->ram_size != mc->default_ram_size) {
> +char *sz = size_to_str(mc->default_ram_size);
> +error_report("Invalid RAM size, should be %s", sz);
> +g_free(sz);
> +exit(EXIT_FAILURE);
> +}
> +
>  /* Initialize SoC */
>  object_initialize_child(OBJECT(machine), "soc", >soc, 
> TYPE_RISCV_E_SOC);
>  qdev_realize(DEVICE(>soc), NULL, _abort);
>
>  /* Data Tightly Integrated Memory */
> -memory_region_init_ram(main_mem, NULL, "riscv.sifive.e.ram",
> -memmap[SIFIVE_E_DEV_DTIM].size, _fatal);
>  memory_region_add_subregion(sys_mem,
> -memmap[SIFIVE_E_DEV_DTIM].base, main_mem);
> +memmap[SIFIVE_E_DEV_DTIM].base, machine->ram);
>
>  /* Mask ROM reset vector */
>  uint32_t reset_vec[4];
> @@ -142,6 +148,8 @@ static void sifive_e_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->init = sifive_e_machine_init;
>  mc->max_cpus = 1;
>  mc->default_cpu_type = SIFIVE_E_CPU;
> +mc->default_ram_id = "riscv.sifive.e.ram";
> +mc->default_ram_size = sifive_e_memmap[SIFIVE_E_DEV_DTIM].size;
>
>  object_class_property_add_bool(oc, "revb", sifive_e_machine_get_revb,
> sifive_e_machine_set_revb);
> --
> 2.25.1
>
>

Re: [PATCH v2 5/6] hw/riscv: sifive_u: Use MachineState::ram and MachineClass::default_ram_id

[Alistair Francis]

On Wed, Oct 20, 2021 at 11:48 AM Bin Meng  wrote:
>
> Using memory_region_init_ram(), which can't possibly handle vhost-user,
> and can't work as expected with '-numa node,memdev' options.
>
> Use MachineState::ram instead of manually initializing RAM memory
> region, as well as by providing MachineClass::default_ram_id to
> opt in to memdev scheme.
>
> Signed-off-by: Bin Meng 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Igor Mammedov 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
> (no changes since v1)
>
>  hw/riscv/sifive_u.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
> index fc5790b8ce..0217006c27 100644
> --- a/hw/riscv/sifive_u.c
> +++ b/hw/riscv/sifive_u.c
> @@ -528,7 +528,6 @@ static void sifive_u_machine_init(MachineState *machine)
>  const MemMapEntry *memmap = sifive_u_memmap;
>  SiFiveUState *s = RISCV_U_MACHINE(machine);
>  MemoryRegion *system_memory = get_system_memory();
> -MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>  MemoryRegion *flash0 = g_new(MemoryRegion, 1);
>  target_ulong start_addr = memmap[SIFIVE_U_DEV_DRAM].base;
>  target_ulong firmware_end_addr, kernel_start_addr;
> @@ -549,10 +548,8 @@ static void sifive_u_machine_init(MachineState *machine)
>  qdev_realize(DEVICE(>soc), NULL, _abort);
>
>  /* register RAM */
> -memory_region_init_ram(main_mem, NULL, "riscv.sifive.u.ram",
> -   machine->ram_size, _fatal);
>  memory_region_add_subregion(system_memory, 
> memmap[SIFIVE_U_DEV_DRAM].base,
> -main_mem);
> +machine->ram);
>
>  /* register QSPI0 Flash */
>  memory_region_init_ram(flash0, NULL, "riscv.sifive.u.flash0",
> @@ -748,6 +745,7 @@ static void sifive_u_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->min_cpus = SIFIVE_U_MANAGEMENT_CPU_COUNT + 1;
>  mc->default_cpu_type = SIFIVE_U_CPU;
>  mc->default_cpus = mc->min_cpus;
> +mc->default_ram_id = "riscv.sifive.u.ram";
>
>  object_class_property_add_bool(oc, "start-in-flash",
> sifive_u_machine_get_start_in_flash,
> --
> 2.25.1
>
>

Re: [PATCH v3 21/21] target/riscv: support for 128-bit satp

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

Support for a 128-bit satp. This is a bit more involved than necessary
because we took the opportunity to increase the page size to 16kB, and
change the page table geometry, which makes the page walk a bit more
parametrizable (variables instead of defines).
Note that is anyway a necessary step for the merging of the 32-bit and
64-bit riscv versions in a single executable.

Signed-off-by: Frédéric Pétrot
Co-authored-by: Fabien Portas
---
  target/riscv/cpu-param.h  |   9 +++-
  target/riscv/cpu_bits.h   |  10 
  target/riscv/cpu_helper.c |  54 ++--
  target/riscv/csr.c| 105 --
  4 files changed, 144 insertions(+), 34 deletions(-)


Is there a spec for this?  I don't see anything in the 2021-10-06 draft...


r~

Re: [PATCH v2 3/6] hw/riscv: shakti_c: Use MachineState::ram and MachineClass::default_ram_id

[Alistair Francis]

On Wed, Oct 20, 2021 at 11:46 AM Bin Meng  wrote:
>
> Using memory_region_init_ram(), which can't possibly handle vhost-user,
> and can't work as expected with '-numa node,memdev' options.
>
> Use MachineState::ram instead of manually initializing RAM memory
> region, as well as by providing MachineClass::default_ram_id to
> opt in to memdev scheme.
>
> Signed-off-by: Bin Meng 
> Reviewed-by: Philippe Mathieu-Daudé 
> Reviewed-by: Igor Mammedov 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
> (no changes since v1)
>
>  hw/riscv/shakti_c.c | 6 ++
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/shakti_c.c b/hw/riscv/shakti_c.c
> index d7d1f91fa5..90e2cf609f 100644
> --- a/hw/riscv/shakti_c.c
> +++ b/hw/riscv/shakti_c.c
> @@ -45,7 +45,6 @@ static void shakti_c_machine_state_init(MachineState 
> *mstate)
>  {
>  ShaktiCMachineState *sms = RISCV_SHAKTI_MACHINE(mstate);
>  MemoryRegion *system_memory = get_system_memory();
> -MemoryRegion *main_mem = g_new(MemoryRegion, 1);
>
>  /* Allow only Shakti C CPU for this platform */
>  if (strcmp(mstate->cpu_type, TYPE_RISCV_CPU_SHAKTI_C) != 0) {
> @@ -59,11 +58,9 @@ static void shakti_c_machine_state_init(MachineState 
> *mstate)
>  qdev_realize(DEVICE(>soc), NULL, _abort);
>
>  /* register RAM */
> -memory_region_init_ram(main_mem, NULL, "riscv.shakti.c.ram",
> -   mstate->ram_size, _fatal);
>  memory_region_add_subregion(system_memory,
>  shakti_c_memmap[SHAKTI_C_RAM].base,
> -main_mem);
> +mstate->ram);
>
>  /* ROM reset vector */
>  riscv_setup_rom_reset_vec(mstate, >soc.cpus,
> @@ -88,6 +85,7 @@ static void shakti_c_machine_class_init(ObjectClass *klass, 
> void *data)
>  mc->desc = "RISC-V Board compatible with Shakti SDK";
>  mc->init = shakti_c_machine_state_init;
>  mc->default_cpu_type = TYPE_RISCV_CPU_SHAKTI_C;
> +mc->default_ram_id = "riscv.shakti.c.ram";
>  }
>
>  static const TypeInfo shakti_c_machine_type_info = {
> --
> 2.25.1
>
>

Re: [PATCH v2 2/6] hw/riscv: opentitan: Use MachineState::ram and MachineClass::default_ram_id

[Alistair Francis]

On Wed, Oct 20, 2021 at 11:44 AM Bin Meng  wrote:
>
> Using memory_region_init_ram(), which can't possibly handle vhost-user,
> and can't work as expected with '-numa node,memdev' options.
>
> Use MachineState::ram instead of manually initializing RAM memory
> region, as well as by providing MachineClass::default_ram_id to
> opt in to memdev scheme.
>
> While at it add check for user supplied RAM size and error out if it
> mismatches board expected value.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> Changes in v2:
> - add RAM size check
> - assign mc->default_ram_size
>
>  hw/riscv/opentitan.c | 16 
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 9803ae6d70..5d568ea58a 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -19,6 +19,7 @@
>   */
>
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "hw/riscv/opentitan.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
> @@ -64,20 +65,25 @@ static const MemMapEntry ibex_memmap[] = {
>
>  static void opentitan_board_init(MachineState *machine)
>  {
> +MachineClass *mc = MACHINE_GET_CLASS(machine);
>  const MemMapEntry *memmap = ibex_memmap;
>  OpenTitanState *s = g_new0(OpenTitanState, 1);
>  MemoryRegion *sys_mem = get_system_memory();
> -MemoryRegion *main_mem = g_new(MemoryRegion, 1);
> +
> +if (machine->ram_size != mc->default_ram_size) {
> +char *sz = size_to_str(mc->default_ram_size);
> +error_report("Invalid RAM size, should be %s", sz);
> +g_free(sz);
> +exit(EXIT_FAILURE);
> +}
>
>  /* Initialize SoC */
>  object_initialize_child(OBJECT(machine), "soc", >soc,
>  TYPE_RISCV_IBEX_SOC);
>  qdev_realize(DEVICE(>soc), NULL, _abort);
>
> -memory_region_init_ram(main_mem, NULL, "riscv.lowrisc.ibex.ram",
> -memmap[IBEX_DEV_RAM].size, _fatal);
>  memory_region_add_subregion(sys_mem,
> -memmap[IBEX_DEV_RAM].base, main_mem);
> +memmap[IBEX_DEV_RAM].base, machine->ram);
>
>  if (machine->firmware) {
>  riscv_load_firmware(machine->firmware, memmap[IBEX_DEV_RAM].base, 
> NULL);
> @@ -95,6 +101,8 @@ static void opentitan_machine_init(MachineClass *mc)
>  mc->init = opentitan_board_init;
>  mc->max_cpus = 1;
>  mc->default_cpu_type = TYPE_RISCV_CPU_IBEX;
> +mc->default_ram_id = "riscv.lowrisc.ibex.ram";
> +mc->default_ram_size = ibex_memmap[IBEX_DEV_RAM].size;
>  }
>
>  DEFINE_MACHINE("opentitan", opentitan_machine_init)
> --
> 2.25.1
>
>

Re: [PATCH v2 1/6] hw/riscv: microchip_pfsoc: Use MachineState::ram and MachineClass::default_ram_id

[Alistair Francis]

On Wed, Oct 20, 2021 at 11:41 AM Bin Meng  wrote:
>
> Using memory_region_init_ram(), which can't possibly handle vhost-user,
> and can't work as expected with '-numa node,memdev' options.
>
> Use MachineState::ram instead of manually initializing RAM memory
> region, as well as by providing MachineClass::default_ram_id to
> opt in to memdev scheme.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

>
> ---
>
> Changes in v2:
> - split RAM into low and high regions using aliases to machine->ram
> - rename mc->default_ram_id to "microchip.icicle.kit.ram"
>
>  hw/riscv/microchip_pfsoc.c | 36 
>  1 file changed, 20 insertions(+), 16 deletions(-)
>
> diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
> index e475b6d511..3fc8545562 100644
> --- a/hw/riscv/microchip_pfsoc.c
> +++ b/hw/riscv/microchip_pfsoc.c
> @@ -463,7 +463,7 @@ static void 
> microchip_icicle_kit_machine_init(MachineState *machine)
>  MemoryRegion *mem_low_alias = g_new(MemoryRegion, 1);
>  MemoryRegion *mem_high = g_new(MemoryRegion, 1);
>  MemoryRegion *mem_high_alias = g_new(MemoryRegion, 1);
> -uint64_t mem_high_size;
> +uint64_t mem_low_size, mem_high_size;
>  hwaddr firmware_load_addr;
>  const char *firmware_name;
>  bool kernel_as_payload = false;
> @@ -485,31 +485,34 @@ static void 
> microchip_icicle_kit_machine_init(MachineState *machine)
>  TYPE_MICROCHIP_PFSOC);
>  qdev_realize(DEVICE(>soc), NULL, _abort);
>
> +/* Split RAM into low and high regions using aliases to machine->ram */
> +mem_low_size = memmap[MICROCHIP_PFSOC_DRAM_LO].size;
> +mem_high_size = machine->ram_size - mem_low_size;
> +memory_region_init_alias(mem_low, NULL,
> + "microchip.icicle.kit.ram_low", machine->ram,
> + 0, mem_low_size);
> +memory_region_init_alias(mem_high, NULL,
> + "microchip.icicle.kit.ram_high", machine->ram,
> + mem_low_size, mem_high_size);
> +
>  /* Register RAM */
> -memory_region_init_ram(mem_low, NULL, "microchip.icicle.kit.ram_low",
> -   memmap[MICROCHIP_PFSOC_DRAM_LO].size,
> -   _fatal);
> -memory_region_init_alias(mem_low_alias, NULL,
> - "microchip.icicle.kit.ram_low.alias",
> - mem_low, 0,
> - memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].size);
>  memory_region_add_subregion(system_memory,
>  memmap[MICROCHIP_PFSOC_DRAM_LO].base,
>  mem_low);
> +memory_region_add_subregion(system_memory,
> +memmap[MICROCHIP_PFSOC_DRAM_HI].base,
> +mem_high);
> +
> +/* Create aliases for the low and high RAM regions */
> +memory_region_init_alias(mem_low_alias, NULL,
> + "microchip.icicle.kit.ram_low.alias",
> + mem_low, 0, mem_low_size);
>  memory_region_add_subregion(system_memory,
>  memmap[MICROCHIP_PFSOC_DRAM_LO_ALIAS].base,
>  mem_low_alias);
> -
> -mem_high_size = machine->ram_size - 1 * GiB;
> -
> -memory_region_init_ram(mem_high, NULL, "microchip.icicle.kit.ram_high",
> -   mem_high_size, _fatal);
>  memory_region_init_alias(mem_high_alias, NULL,
>   "microchip.icicle.kit.ram_high.alias",
>   mem_high, 0, mem_high_size);
> -memory_region_add_subregion(system_memory,
> -memmap[MICROCHIP_PFSOC_DRAM_HI].base,
> -mem_high);
>  memory_region_add_subregion(system_memory,
>  memmap[MICROCHIP_PFSOC_DRAM_HI_ALIAS].base,
>  mem_high_alias);
> @@ -606,6 +609,7 @@ static void 
> microchip_icicle_kit_machine_class_init(ObjectClass *oc, void *data)
> MICROCHIP_PFSOC_COMPUTE_CPU_COUNT;
>  mc->min_cpus = MICROCHIP_PFSOC_MANAGEMENT_CPU_COUNT + 1;
>  mc->default_cpus = mc->min_cpus;
> +mc->default_ram_id = "microchip.icicle.kit.ram";
>
>  /*
>   * Map 513 MiB high memory, the mimimum required high memory size, 
> because
> --
> 2.25.1
>
>

Re: [PATCH v3 16/21] target/riscv: adding high part of some csrs

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

Adding the high part of a minimal set of csr.

Signed-off-by: Frédéric Pétrot 
Co-authored-by: Fabien Portas 
---
  target/riscv/cpu.h | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 8b96ccb37a..27ec4fec63 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -192,6 +192,13 @@ struct CPURISCVState {
  target_ulong hgatp;
  uint64_t htimedelta;
  
+/* Upper 64-bits of 128-bit CSRs */

+uint64_t mtvech;
+uint64_t mscratchh;
+uint64_t mepch;
+uint64_t satph;
+uint64_t mstatush;


There's nothing defined for mstatush (except SD), so we might as well leave it out until 
there is.  The only thing required there is that we put SD in the correct place when we 
compute it from lower bits on read.


mepch and mtvech do not need extending until we extend pc.

I don't see a definition of how satph extends, and since you're not changing the rv64 
virtual memory routines nothing will examine it anyway.  Let's drop that.


Which leaves mscratchh and maybe sscratchh as the only "real" 128-bit csrs.
Which suggests that the support that you do add in the next patch does not need to be 
quite as complicated.  E.g. drop the op128 hook.



r~

Re: [PATCH] via-ide: Avoid expensive operations in irq handler

[BALATON Zoltan]

On Wed, 20 Oct 2021, Eduardo Habkost wrote:

On Mon, Oct 18, 2021 at 12:10:04PM +0200, Philippe Mathieu-Daudé wrote:

On 10/18/21 11:51, BALATON Zoltan wrote:

On Mon, 18 Oct 2021, Philippe Mathieu-Daudé wrote:

On 10/18/21 03:36, BALATON Zoltan wrote:

Cache the pointer to PCI function 0 (ISA bridge, that this IDE device
has to use for IRQs) in the PCIIDEState and pass that as the opaque
data for the interrupt handler to eliminate both the need to look up
function 0 at every interrupt and also a QOM type cast of the opaque
pointer as that's also expensive (mainly due to qom-debug being
enabled by default).

Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: BALATON Zoltan 
---
 hw/ide/via.c | 11 ++-
 include/hw/ide/pci.h |  1 +
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/ide/via.c b/hw/ide/via.c
index 82def819c4..30566bc409 100644
--- a/hw/ide/via.c
+++ b/hw/ide/via.c
@@ -104,15 +104,15 @@ static void bmdma_setup_bar(PCIIDEState *d)

 static void via_ide_set_irq(void *opaque, int n, int level)
 {
-    PCIDevice *d = PCI_DEVICE(opaque);
+    PCIIDEState *d = opaque;

 if (level) {
-    d->config[0x70 + n * 8] |= 0x80;
+    d->parent_obj.config[0x70 + n * 8] |= 0x80;
 } else {
-    d->config[0x70 + n * 8] &= ~0x80;
+    d->parent_obj.config[0x70 + n * 8] &= ~0x80;
 }


Better not to access parent_obj, so I'd rather keep the previous
code as it. The rest is OK, thanks. If you don't want to respin
I can fix and take via mips-next.


Why not? If it's OK to access other fields why not parent_obj? That
avoids the QOM cast PCI_DEVICE(opaque) or PCI_DEVICE(d) after this
patch. We know it's a PCIIDEState and has PCIDevice parent_obj field
because we set the opaque pointer when adding this callback so I think
this works and is the less expensive way. But feel free to change it any
way you like and use it that way. I'd keep it as it is.


My understanding of QOM is we shouldn't access internal states that
way, because 1/ this makes object refactors harder and 2/ this is
not the style/example we want in the codebase, but it doesn't seem
documented, so Cc'ing Markus/Eduardo for confirmation.


`PCI_DEVICE(d)` is preferred instead `of d.parent_obj` (parent_obj is
just a QOM implementation detail).  If there are real performance
reasons to avoid it, we need numbers to support that.

Also, note that `OBJECT_CHECK(obj)` is just `return obj` if
CONFIG_QOM_CAST_DEBUG is disabled.


I've tried to do some measurements but could be I did not come up with a 
good enough test (I was just trying to copy a few hundred megabytes and 
timed that) as I could not find any significant difference with or without 
QOM casts or even without this patch (so even caching func0 did not seem 
to make a difference). Could be there are other bigger bottlenecks 
elsewhere before this becomes critical so for now maybe just drop this 
patch and the similar one for USB (that is first and last patch in the 
series) and take the rest of the series only until somebody can do some 
better tests to see if this optimisation would help.


Gerd, Philippe which of you can take care of merging the remaining 
patches? That's still needed to fix USB interrupt on pegasos2.


Regards,
BALATON Zoltan

Re: [PATCH v2 5/5] speed/sdhci: Add trace events

[Francisco Iglesias]

Hi Cedric,

On the subject s/speed/aspeed/. Otherwise:

Reviewed-by: Francisco Iglesias 

/BR

On [2021 Oct 18] Mon 15:26:09, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/sd/aspeed_sdhci.c | 5 +
>  hw/sd/trace-events   | 4 
>  2 files changed, 9 insertions(+)
> 
> diff --git a/hw/sd/aspeed_sdhci.c b/hw/sd/aspeed_sdhci.c
> index 3299844de6dc..df1bdf1fa4ed 100644
> --- a/hw/sd/aspeed_sdhci.c
> +++ b/hw/sd/aspeed_sdhci.c
> @@ -14,6 +14,7 @@
>  #include "hw/irq.h"
>  #include "migration/vmstate.h"
>  #include "hw/qdev-properties.h"
> +#include "trace.h"
>  
>  #define ASPEED_SDHCI_INFO0x00
>  #define  ASPEED_SDHCI_INFO_SLOT1 (1 << 17)
> @@ -60,6 +61,8 @@ static uint64_t aspeed_sdhci_read(void *opaque, hwaddr 
> addr, unsigned int size)
>  }
>  }
>  
> +trace_aspeed_sdhci_read(addr, size, (uint64_t) val);
> +
>  return (uint64_t)val;
>  }
>  
> @@ -68,6 +71,8 @@ static void aspeed_sdhci_write(void *opaque, hwaddr addr, 
> uint64_t val,
>  {
>  AspeedSDHCIState *sdhci = opaque;
>  
> +trace_aspeed_sdhci_write(addr, size, val);
> +
>  switch (addr) {
>  case ASPEED_SDHCI_INFO:
>  /* The RESET bit automatically clears. */
> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
> index 3cc2ef89ba6b..94a00557b26f 100644
> --- a/hw/sd/trace-events
> +++ b/hw/sd/trace-events
> @@ -68,3 +68,7 @@ pl181_fifo_push(uint32_t data) "FIFO push 0x%08" PRIx32
>  pl181_fifo_pop(uint32_t data) "FIFO pop 0x%08" PRIx32
>  pl181_fifo_transfer_complete(void) "FIFO transfer complete"
>  pl181_data_engine_idle(void) "data engine idle"
> +
> +# aspeed_sdhci.c
> +aspeed_sdhci_read(uint64_t addr, uint32_t size, uint64_t data) "@0x%" PRIx64 
> " size %u: 0x%" PRIx64
> +aspeed_sdhci_write(uint64_t addr, uint32_t size, uint64_t data) "@0x%" 
> PRIx64 " size %u: 0x%" PRIx64
> -- 
> 2.31.1
> 
> 

RE: [PATCH v2 18/48] tcg/optimize: Split out fold_brcond

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 33 +++--
>  1 file changed, 19 insertions(+), 14 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 01/48] tcg/optimize: Rename "mask" to "z_mask"

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Prepare for tracking different masks by renaming this one.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 142 +
>  1 file changed, 72 insertions(+), 70 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 19/48] tcg/optimize: Split out fold_setcond

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 23 ++-
>  1 file changed, 14 insertions(+), 9 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 11/48] tcg/optimize: Return true from tcg_opt_gen_{mov, movi}

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This will allow callers to tail call to these functions and return true 
> indicating
> processing complete.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 17/48] tcg/optimize: Split out fold_brcond2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reduce some code duplication by folding the NE and EQ cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 161 +
>  1 file changed, 83 insertions(+), 78 deletions(-)

> +case TCG_COND_NE:
> +inv = 1;
> +QEMU_FALLTHROUGH;
> +case TCG_COND_EQ:
> +/*
> + * Simplify EQ/NE comparisons where one of the pairs
> + * can be simplified.
> + */
> +i = do_constant_folding_cond(INDEX_op_brcond_i32, op->args[0],
> + op->args[2], cond);
> +switch (i ^ inv) {
> +case 0:
> +goto do_brcond_false;

I believe this should go to do_brcond_true when cond==TCG_COND_NE.

> +i = do_constant_folding_cond(INDEX_op_brcond_i32, op->args[1],
> + op->args[3], cond);
> +switch (i ^ inv) {
> +case 0:
> +goto do_brcond_false;

Same here.

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 13/48] tcg/optimize: Use a boolean to avoid a mass of continues

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 15/48] tcg/optimize: Split out fold_const{1,2}

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Split out a whole bunch of placeholder functions, which are currently 
> identical.
> That won't last as more code gets moved.
> 
> Use CASE_32_64_VEC for some logical operators that previously missed the
> addition of vectors.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 254 +++--
>  1 file changed, 202 insertions(+), 52 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 12/48] tcg/optimize: Split out finish_folding

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Copy z_mask into OptContext, for writeback to the first output within the new
> function.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 49 +
>  1 file changed, 33 insertions(+), 16 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 16/48] tcg/optimize: Split out fold_setcond2

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Reduce some code duplication by folding the NE and EQ cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 145 -
>  1 file changed, 72 insertions(+), 73 deletions(-)

> -i = do_constant_folding_cond(INDEX_op_setcond_i32,
> - op->args[2], op->args[4],
> - TCG_COND_EQ);
> -if (i == 0) {
> -goto do_setcond_high;
^
> -} else if (i < 0) {
> -break;
> -}

I'll just note that the goto in the old code above is wrong - it should be a 
goto to do_setcond_const. But you fixed that in your new implementation.

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 06/48] tcg/optimize: Split out init_arguments

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> There was no real reason for calls to have separate code here.
> Unify init for calls vs non-calls using the call path, which handles
> TCG_CALL_DUMMY_ARG.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 25 +++--
>  1 file changed, 11 insertions(+), 14 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 07/48] tcg/optimize: Split out copy_propagate

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Continue splitting tcg_optimize.
> 
> Reviewed-by: Philippe Mathieu-Daudé 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 22 ++
>  1 file changed, 14 insertions(+), 8 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 09/48] tcg/optimize: Drop nb_oargs, nb_iargs locals

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Rather than try to keep these up-to-date across folding, re-read nb_oargs at 
> the
> end, after re-reading the opcode.
> 
> A couple of asserts need dropping, but that will take care of itself as we 
> split the
> function further.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 14 --
>  1 file changed, 4 insertions(+), 10 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 10/48] tcg/optimize: Change fail return for do_constant_folding_cond*

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Return -1 instead of 2 for failure.
> This us to use comparisons against 0 for all cases.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 145 +
>  1 file changed, 74 insertions(+), 71 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 14/48] tcg/optimize: Split out fold_mb, fold_qemu_{ld,st}

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This puts the separate mb optimization into the same framework as the others.
> While fold_qemu_{ld,st} are currently identical, that won't last as more code
> gets moved.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 89 +-
>  1 file changed, 51 insertions(+), 38 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 05/48] tcg/optimize: Move prev_mb into OptContext

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> This will expose the variable to subroutines that will be broken out of
> tcg_optimize.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 04/48] tcg/optimize: Change tcg_opt_gen_{mov, movi} interface

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Adjust the interface to take the OptContext parameter instead of TCGContext or
> both.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 67 +-
>  1 file changed, 34 insertions(+), 33 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 03/48] tcg/optimize: Remove do_default label

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Break the final cleanup clause out of the main switch statement.  When fully
> folding an opcode to mov/movi, use "continue" to process the next opcode, else
> break to fall into the final cleanup.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 190 -
>  1 file changed, 94 insertions(+), 96 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 02/48] tcg/optimize: Split out OptContext

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Provide what will become a larger context for splitting the very large
> tcg_optimize function.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 77 ++
>  1 file changed, 40 insertions(+), 37 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

RE: [PATCH v2 08/48] tcg/optimize: Split out fold_call

[Luis Fernando Fujita Pires]

From: Richard Henderson 
> Calls are special in that they have a variable number of arguments, and need 
> to
> be able to clobber globals.
> 
> Signed-off-by: Richard Henderson 
> ---
>  tcg/optimize.c | 63 --
>  1 file changed, 41 insertions(+), 22 deletions(-)

Reviewed-by: Luis Pires 

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer 

Re: [PATCH v3 19/21] target/riscv: actual functions to realize crs 128-bit insns

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

The csrs are accessed through function pointers: we set-up the table
for the 128-bit accesses, make the stub a function that does what it
should, and implement basic accesses on read-only csrs.

Signed-off-by: Frédéric Pétrot 
Co-authored-by: Fabien Portas 
---
  target/riscv/cpu.h |  16 +
  target/riscv/csr.c | 152 -
  2 files changed, 165 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index eb4f63fcbf..253e87cd92 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -474,6 +474,15 @@ RISCVException riscv_csrrw_i128(CPURISCVState *env, int 
csrno,
  Int128 *ret_value,
  Int128 new_value, Int128 write_mask);
  
+typedef RISCVException (*riscv_csr_read128_fn)(CPURISCVState *env, int csrno,

+   Int128 *ret_value);
+typedef RISCVException (*riscv_csr_write128_fn)(CPURISCVState *env, int csrno,
+ Int128 new_value);
+typedef RISCVException (*riscv_csr_op128_fn)(CPURISCVState *env, int csrno,
+ Int128 *ret_value,
+ Int128 new_value,
+ Int128 write_mask);


Do we really want all 3, or just the single rmw function?
Although I guess it's clearest to match the existing code...


+
  typedef struct {
  const char *name;
  riscv_csr_predicate_fn predicate;
@@ -482,6 +491,12 @@ typedef struct {
  riscv_csr_op_fn op;
  } riscv_csr_operations;
  
+typedef struct {

+riscv_csr_read128_fn read128;
+riscv_csr_write128_fn write128;
+riscv_csr_op128_fn op128;
+} riscv_csr_operations128;


Eh.  I think better to extend the one riscv_csr_operations structure.


+static inline RISCVException riscv_csrrw_check_i128(CPURISCVState *env,
+int csrno,
+Int128 write_mask,
+RISCVCPU *cpu)


Change "Int128 write_mask" to "bool write" and you can share this entire function with 
riscv_csrrw.


Indeed, you could split them like so:

riscv_csrrw(...)
{
ret = csrrw_check(...);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
return csrrw_do64(...);
}

riscv_csrrw_128(...)
{
ret = csrrw_check(...);
if (ret != RISCV_EXCP_NONE) {
return ret;
}
if (csr128) {
return csrrw_do128(...);
}
ret = csrrw_do64(..., old64, ...);
if (ret == RISCV_EXCP_NONE) {
*old_val = int128_make64(old64);
}
return ret;
}


+RISCVException ret = csr_ops[csrno].predicate(env, csrno);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+
+return RISCV_EXCP_NONE;


BTW, just

return csr_ops[csrno].predicate(env, csrno);


r~

Re: [PATCH 2/3] optionrom: add a DMA-enabled multiboot ROM

[Philippe Mathieu-Daudé]

On 10/20/21 16:02, Paolo Bonzini wrote:
> From: Marcus Hähnel 
> 
> Add a new option rom for the multiboot loader, using DMA transfers to copy
> data instead of "rep insb".
> 
> This significantly lowers QEMU's startup latency by a factor of about 40,
> for example, going from 30sec to 0.8sec when loading modules of 120MB
> in size.
> 
> Signed-off-by: Marcus Hähnel 
> Signed-off-by: Adam Lackorzynski 
> [Modified to keep the non-DMA code depending on #ifdef USE_FW_CFG_DMA;
>  do not write below stack. - Paolo]
> Signed-off-by: Paolo Bonzini 
> ---
>  pc-bios/meson.build   |   1 +
>  pc-bios/multiboot_dma.bin | Bin 0 -> 1024 bytes
>  pc-bios/optionrom/Makefile|   4 +-
>  pc-bios/optionrom/multiboot.S |   4 +-
>  pc-bios/optionrom/multiboot_dma.S |   2 +
>  pc-bios/optionrom/optionrom.h |  66 ++
>  6 files changed, 72 insertions(+), 5 deletions(-)
>  create mode 100644 pc-bios/multiboot_dma.bin
>  create mode 100644 pc-bios/optionrom/multiboot_dma.S

> diff --git a/pc-bios/multiboot_dma.bin b/pc-bios/multiboot_dma.bin
> new file mode 100644
> index 
> ..c0e2c3102a3358207c61d3ae113524fb6007abc3
> GIT binary patch
> literal 1024
> zcmd^-v1=1i9LIlmUTlcNU1}{N5`u&{DAHSmq6iKdix;GHRxaNnR0A zaS+5wQE+fL&*iQWa#czc94ZyL>R{3q;@~8~^LMo;4s!p158nI!-tWur_ul*P<{!&%
> z=%3>xm5f`4Bq!!)`Rkwf;(oFd*qcAb=mhXX1)X>Bw-tylkUpR_ETXkH1AnO4L$nJ|
> zWJui_M1kmKmTID)P`mI^=HM`~{VB1t1l_Ye=Lor4X5{8Gd)zzw_o4<6zLQJ!dr z;7%~ysSHL?*HHx*O%k}?fCyRL<7_RWt(4;Aqn}X}S>MolrKNZG2Z|kTOyaJIhYFc^
> zFhhuQ9`r5fQLCrm#T4^_QylQ>8kgs;ZX9bIEiXb`8AIxu;?h~d%9izBkKht%WM1G*
> z^E{W9(OT9dt5in2qF}|dPH>dL>{=sV#-U1 zrVCY?W~`3Hw@^=cHALr#9F2GOTYJ;??9d*-Vbsi+;lz|<$jMX#;lr6ov3qKNLH7(W
> z+>yFoWnOtw14D$*SUtsT%wS`ki@#`PtJm(Kg?H$Pdc0CL@YCy4R pT|LnIbg(BnP0#tqRx5M!bkkaD-nd?`H;YU4Yi6yHwD{k({tHC8FAx9#

Ideally this should be generated on CI as artifact, archived
and we commit the CI generated file.

> diff --git a/pc-bios/optionrom/optionrom.h b/pc-bios/optionrom/optionrom.h
> index a2b612f1a7..8d74c0ddf3 100644
> --- a/pc-bios/optionrom/optionrom.h
> +++ b/pc-bios/optionrom/optionrom.h
> @@ -37,6 +37,17 @@
>  #define BIOS_CFG_IOPORT_CFG  0x510
>  #define BIOS_CFG_IOPORT_DATA 0x511
>  
> +#define FW_CFG_DMA_CTL_ERROR   0x01
> +#define FW_CFG_DMA_CTL_READ0x02
> +#define FW_CFG_DMA_CTL_SKIP0x04
> +#define FW_CFG_DMA_CTL_SELECT  0x08
> +#define FW_CFG_DMA_CTL_WRITE   0x10
> +
> +#define FW_CFG_DMA_SIGNATURE 0x51454d5520434647ULL /* "QEMU CFG" */
> +
> +#define BIOS_CFG_DMA_ADDR_HIGH  0x514
> +#define BIOS_CFG_DMA_ADDR_LOW   0x518
> +
>  /* Break the translation block flow so -d cpu shows us values */
>  #define DEBUG_HERE \
>   jmp 1f; \
> @@ -62,6 +73,61 @@
>   bswap   %eax
>  .endm
>  
> +
> +/*
> + * Read data from the fw_cfg device using DMA.
> + * Clobbers: %edx, %eax, ADDR, SIZE, memory[%esp-16] to memory[%esp]
> + */
> +.macro read_fw_dma VAR, SIZE, ADDR
> +/* Address */
> + bswapl  \ADDR
> + pushl   \ADDR
> +
> + /* We only support 32 bit target addresses */
> + xorl%eax, %eax
> + pushl   %eax
> + mov $BIOS_CFG_DMA_ADDR_HIGH, %dx
> + outl%eax, (%dx)
> +
> + /* Size */
> + bswapl  \SIZE
> + pushl   \SIZE
> +
> +/* Control */

Indent off.

> + movl$(\VAR << 16) | (FW_CFG_DMA_CTL_READ | 
> FW_CFG_DMA_CTL_SELECT), %eax
> + bswapl  %eax
> + pushl   %eax
> +
> + movl%esp, %eax /* Address of the struct we generated */
> + bswapl  %eax
> + mov $BIOS_CFG_DMA_ADDR_LOW, %dx
> + outl%eax, (%dx) /* Initiate DMA */
> +
> +1:  mov  (%esp), %eax /* Wait for completion */
> + bswapl  %eax
> + testl   $~FW_CFG_DMA_CTL_ERROR, %eax
> + jnz 1b
> +   addl$16, %esp

Indent off.

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 5/5] speed/sdhci: Add trace events

[Philippe Mathieu-Daudé]

On 10/18/21 15:26, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/sd/aspeed_sdhci.c | 5 +
>  hw/sd/trace-events   | 4 
>  2 files changed, 9 insertions(+)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 4/5] aspeed/smc: Use a container for the flash mmio address space

[Philippe Mathieu-Daudé]

On 10/18/21 15:26, Cédric Le Goater wrote:
> Because AddressSpaces must not be sysbus-mapped, commit e9c568dbc225
> ("hw/arm/aspeed: Do not sysbus-map mmio flash region directly, use
> alias") introduced an alias for the flash mmio region.
> 
> Using a container is cleaner.
> 
> Cc: Philippe Mathieu-Daudé 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/ssi/aspeed_smc.h |  2 +-
>  hw/ssi/aspeed_smc.c | 11 +++
>  2 files changed, 8 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 2/5] aspeed: Initialize the watchdog device models before the FMC models

[Philippe Mathieu-Daudé]

On 10/18/21 15:26, Cédric Le Goater wrote:
> Next changes will map the WDT2 registers in the AST2600 FMC memory
> region. Make sure the MemoryRegion pointers are correctly initialized
> before setting the object links.
> 
> Do the same in the Aspeed AST2400 and AST2500 SoC models for
> consistency.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  hw/arm/aspeed_ast2600.c | 36 ++--
>  hw/arm/aspeed_soc.c | 36 ++--
>  2 files changed, 36 insertions(+), 36 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v2 1/5] aspeed/wdt: Introduce a container for the MMIO region

[Philippe Mathieu-Daudé]

On 10/18/21 15:26, Cédric Le Goater wrote:
> On the AST2600, the 2nd watchdog timer can be controlled through the
> FMC controller to disable the alternate boot function. Next changes
> will map the WDT2 registers in the AST2600 FMC memory region. Add a
> container on top of the register region for this purpose.
> 
> Signed-off-by: Cédric Le Goater 
> ---
>  include/hw/watchdog/wdt_aspeed.h | 1 +
>  hw/watchdog/wdt_aspeed.c | 6 +-
>  2 files changed, 6 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH v3 18/21] target/riscv: modification of the trans_csrxx for 128-bit support

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

As opposed to the gen_arith and gen_shift generation helpers, the csr insns
do not have a common prototype, so the choice to generate 32/64 or 128-bit
helper calls is done in the trans_csrxx functions.

Signed-off-by: Frédéric Pétrot
Co-authored-by: Fabien Portas
---
  target/riscv/insn_trans/trans_rvi.c.inc | 201 ++--
  1 file changed, 156 insertions(+), 45 deletions(-)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 17/21] target/riscv: helper functions to wrap calls to 128-bit csr insns

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

Given the side effects they have, the csr instructions are realized as
helpers. We extend this existing infrastructure for 128-bit sized csr.
We have a slight issue with returning 128-bit values: we use the globals
we added to support div/rem insns to that end.
Theses helpers all call a unique function that is currently a stub.
The trans_csrxx functions supporting 128-bit are yet to be implemented.

Signed-off-by: Frédéric Pétrot
Co-authored-by: Fabien Portas
---
  target/riscv/cpu.h   |  4 
  target/riscv/helper.h|  3 +++
  target/riscv/csr.c   |  7 +++
  target/riscv/op_helper.c | 44 
  4 files changed, 58 insertions(+)


Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 16/21] target/riscv: adding high part of some csrs

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

+/* Upper 64-bits of 128-bit CSRs */
+uint64_t mtvech;
+uint64_t mscratchh;
+uint64_t mepch;
+uint64_t satph;
+uint64_t mstatush;


Needs adding to the same machine.c subsection as the gprs.
Otherwise,

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v3 15/21] target/riscv: support for 128-bit M extension

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

  struct CPURISCVState {
  target_ulong gpr[32];
  target_ulong gprh[32]; /* 64 top bits of the 128-bit registers */
+target_ulong hlpr[2];  /* scratch registers for 128-bit div/rem helpers */


We have something similar for s390x, but we make use of the helper return value to return 
one part of the result and only store the other part of the result in env->retxl.



+cpu_hlpr[0] = tcg_global_mem_new(cpu_env,
+offsetof(CPURISCVState, hlpr[0]), "helper_reg0");
+cpu_hlpr[1] = tcg_global_mem_new(cpu_env,
+offsetof(CPURISCVState, hlpr[1]), "helper_reg1");


You very much do not want to make these global temps.

This requires the helpers to indicate that they clobber temps, which will flush all cached 
register state across the helper.  Just perform the load of the result explicitly after 
the helper.



+static void gen_mulu2_i128(TCGv rll, TCGv rlh, TCGv rhl, TCGv rhh,
+   TCGv al, TCGv ah, TCGv bl, TCGv bh)
+{
+TCGv tmpl = tcg_temp_new(),
+ tmph = tcg_temp_new(),
+ cnst_zero = tcg_constant_tl(0);
+
+tcg_gen_mulu2_tl(rll, rlh, al, bl);
+
+tcg_gen_mulu2_tl(tmpl, tmph, al, bh);
+tcg_gen_add2_tl(rlh, rhl, rlh, cnst_zero, tmpl, tmph);
+tcg_gen_mulu2_tl(tmpl, tmph, ah, bl);
+tcg_gen_add2_tl(rlh, tmph, rlh, rhl, tmpl, tmph);
+/* Overflow detection into rhh */
+tcg_gen_setcond_tl(TCG_COND_LTU, rhh, tmph, rhl);
+
+tcg_gen_mov_tl(rhl, tmph);
+
+tcg_gen_mulu2_tl(tmpl, tmph, ah, bh);
+tcg_gen_add2_tl(rhl, rhh, rhl, rhh, tmpl, tmph);


It might be clearer to number these 0-3 rather than permute [lh].

I think you don't need to return all 4 words of results; just have gen_mulhu_i128 with 6 
parameters, since there's no RV128 instruction that returns the entire result.



+static void gen_mul_i128(TCGv rll, TCGv rlh,
+ TCGv rs1l, TCGv rs1h, TCGv rs2l, TCGv rs2h)
+{
+TCGv rhl = tcg_temp_new(),
+ rhh = tcg_temp_new();
+
+gen_mulu2_i128(rll, rlh, rhl, rhh, rs1l, rs1h, rs2l, rs2h);
+
+tcg_temp_free(rhl);
+tcg_temp_free(rhh);
+}


This is much simpler than gen_mulu2_i128.


+static void gen_mulh_i128(TCGv rhl, TCGv rhh,
+  TCGv rs1l, TCGv rs1h, TCGv rs2l, TCGv rs2h)
+{
+TCGv rll = tcg_temp_new(),
+ rlh = tcg_temp_new(),
+ rlln = tcg_temp_new(),
+ rlhn = tcg_temp_new(),
+ rhln = tcg_temp_new(),
+ rhhn = tcg_temp_new(),
+ sgnres = tcg_temp_new(),
+ tmp = tcg_temp_new(),
+ cnst_one = tcg_constant_tl(1),
+ cnst_zero = tcg_constant_tl(0);
+
+/* Extract sign of result (=> sgn(a) xor sgn(b)) */
+tcg_gen_setcondi_tl(TCG_COND_LT, sgnres, rs1h, 0);
+tcg_gen_setcondi_tl(TCG_COND_LT, tmp, rs2h, 0);
+tcg_gen_xor_tl(sgnres, sgnres, tmp);
+
+/* Take absolute value of operands */
+tcg_gen_sari_tl(rhl, rs1h, 63);
+tcg_gen_add2_tl(rlln, rlhn, rs1l, rs1h, rhl, rhl);
+tcg_gen_xor_tl(rlln, rlln, rhl);
+tcg_gen_xor_tl(rlhn, rlhn, rhl);
+
+tcg_gen_sari_tl(rhl, rs2h, 63);
+tcg_gen_add2_tl(rhln, rhhn, rs2l, rs2h, rhl, rhl);
+tcg_gen_xor_tl(rhln, rhln, rhl);
+tcg_gen_xor_tl(rhhn, rhhn, rhl);
+
+/* Unsigned multiplication */
+gen_mulu2_i128(rll, rlh, rhl, rhh, rlln, rlhn, rhln, rhhn);
+
+/* Negation of result (two's complement : ~res + 1) */
+tcg_gen_not_tl(rlln, rll);
+tcg_gen_not_tl(rlhn, rlh);
+tcg_gen_not_tl(rhln, rhl);
+tcg_gen_not_tl(rhhn, rhh);
+
+tcg_gen_add2_tl(rlln, tmp, rlln, cnst_zero, cnst_one, cnst_zero);
+tcg_gen_add2_tl(rlhn, tmp, rlhn, cnst_zero, tmp, cnst_zero);
+tcg_gen_add2_tl(rhln, tmp, rhln, cnst_zero, tmp, cnst_zero);
+tcg_gen_add2_tl(rhhn, tmp, rhhn, cnst_zero, tmp, cnst_zero);
+
+/* Move conditionally result or -result depending on result sign */
+tcg_gen_movcond_tl(TCG_COND_NE, rhl, sgnres, cnst_zero, rhln, rhl);
+tcg_gen_movcond_tl(TCG_COND_NE, rhh, sgnres, cnst_zero, rhhn, rhh);
+
+tcg_temp_free(rll);
+tcg_temp_free(rlh);
+tcg_temp_free(rlln);
+tcg_temp_free(rlhn);
+tcg_temp_free(rhln);
+tcg_temp_free(rhhn);
+tcg_temp_free(sgnres);
+tcg_temp_free(tmp);
 }


You don't need to compute abs or conditional negation.

See tcg_gen_muls2_i32, adjust for negative inputs. It's simply subtracting one input from 
the high part when the other input is negative.



+static void gen_mulhsu_i128(TCGv rhl, TCGv rhh,
+TCGv rs1l, TCGv rs1h, TCGv rs2l, TCGv rs2h)


Similarly, but of course only one operand may be negative.


+static void gen_div_i128(TCGv rdl, TCGv rdh,
+ TCGv rs1l, TCGv rs1h, TCGv rs2l, TCGv rs2h)
+{
+gen_helper_divs_i128(cpu_env, (TCGv_i64)rs1l, (TCGv_i64)rs1h,
+  (TCGv_i64)rs2l, (TCGv_i64)rs2h);


Do not cast, just make the arguments target_long always.

Anyway, per above, this becomes

Re: plugins: Missing Store Exclusive Memory Accesses

[Aaron Lindsay via]

On Oct 20 18:54, Alex Bennée wrote:
> Have you got a test case you are using so I can try and replicate the
> failure you are seeing? So far by inspection everything looks OK to me.

I took some time today to put together a minimal(ish) reproducer using
usermode. The source files used are below, I compiled the test binary on an
AArch64 system using:

$ gcc -g -o stxp stxp.s stxp.c

Then built the plugin from stxp_plugin.cc, and ran it all like:

qemu-aarch64 \
-cpu cortex-a57 \
-D stxp_plugin.log \
-d plugin \
-plugin 'stxp_plugin.so' \
./stxp

I observe that, for me, the objdump of stxp contains:
0040070c :
  40070c:   f9800011prfmpstl1strm, [x0]
  400710:   c87f4410ldxpx16, x17, [x0]
  400714:   c8300c02stxpw16, x2, x3, [x0]
  400718:   f1000652subsx18, x18, #0x1
  40071c:   5440b.eq400724   // b.none
  400720:   17fbb   40070c 

But the output in stxp_plugin.log looks something like:
Executing PC: 0x40070c
Executing PC: 0x400710
PC 0x400710 accessed memory at 0x550080ec70
PC 0x400710 accessed memory at 0x550080ec78
Executing PC: 0x400714
Executing PC: 0x400718
Executing PC: 0x40071c
Executing PC: 0x400720

>From this, I believe the ldxp instruction at PC 0x400710 is reporting two
memory accesses but the stxp instruction at 0x400714 is not.

-Aaron

--- stxp.c ---
void stxp_issue_demo();

int main() {
char arr[16];
stxp_issue_demo();
}

--- stxp.s ---
.align 8

stxp_issue_demo:
mov x18, 0x1000
mov x2, 0x0
mov x3, 0x0
loop:
prfm  pstl1strm, [x0]
ldxp  x16, x17, [x0]
stxp  w16, x2, x3, [x0]

subs x18, x18, 1
beq done
b loop
done:
ret

.global stxp_issue_demo

--- stxp_plugin.cc ---
#include 

extern "C" {

#include 

QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;

void qemu_logf(const char *str, ...)
{
char message[1024];
va_list args;
va_start(args, str);
vsnprintf(message, 1023, str, args);

qemu_plugin_outs(message);

va_end(args);
}

void before_insn_cb(unsigned int cpu_index, void *udata)
{
uint64_t pc = (uint64_t)udata;
qemu_logf("Executing PC: 0x%" PRIx64 "\n", pc);
}

static void mem_cb(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo, 
uint64_t va, void *udata)
{
uint64_t pc = (uint64_t)udata;
qemu_logf("PC 0x%" PRIx64 " accessed memory at 0x%" PRIx64 "\n", pc, va);
}

static void vcpu_tb_trans(qemu_plugin_id_t id, struct qemu_plugin_tb *tb)
{
size_t n = qemu_plugin_tb_n_insns(tb);

for (size_t i = 0; i < n; i++) {
struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
uint64_t pc = qemu_plugin_insn_vaddr(insn);

qemu_plugin_register_vcpu_insn_exec_cb(insn, before_insn_cb, 
QEMU_PLUGIN_CB_R_REGS, (void *)pc);
qemu_plugin_register_vcpu_mem_cb(insn, mem_cb, QEMU_PLUGIN_CB_NO_REGS, 
QEMU_PLUGIN_MEM_RW, (void*)pc);
}
}

QEMU_PLUGIN_EXPORT
int qemu_plugin_install(qemu_plugin_id_t id, const qemu_info_t *info,
int argc, char **argv)
{
qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
return 0;
}

}

Re: [PATCH v3 14/21] target/riscv: support for 128-bit arithmetic instructions

[Richard Henderson]

On 10/19/21 2:48 AM, Frédéric Pétrot wrote:

+static bool gen_setcond_i128(TCGv rl, TCGv rh,
+ TCGv al, TCGv ah,
+ TCGv bl, TCGv bh,
+ TCGCond cond)
+{
+switch (cond) {
+case TCG_COND_EQ:
+tcg_gen_setcond_tl(TCG_COND_EQ, rl, al, bl);
+tcg_gen_setcond_tl(TCG_COND_EQ, rh, ah, bh);
+tcg_gen_and_tl(rl, rl, rh);
+break;
+
+case TCG_COND_NE:
+tcg_gen_setcond_tl(TCG_COND_NE, rl, al, bl);
+tcg_gen_setcond_tl(TCG_COND_NE, rh, ah, bh);
+tcg_gen_or_tl(rl, rl, rh);
+break;


But of course setcond is more expensive than logic
Better as xor + xor + or + setcond.



+case TCG_COND_LTU:
+{
+TCGv tmp1 = tcg_temp_new(),
+ tmp2 = tcg_temp_new();
+
+tcg_gen_sub2_tl(rl, rh, al, ah, bl, bh);
+tcg_gen_eqv_tl(tmp1, ah, bh);
+tcg_gen_and_tl(tmp1, tmp1, rh);
+tcg_gen_andc_tl(tmp2, bh, ah);
+tcg_gen_or_tl(tmp1, tmp1, tmp2);
+tcg_gen_shri_tl(rl, tmp1, 63);


Hmm.  Seems like it would work.
I make that 7 operations.

Or GEU in 6 operations:

/* borrow in to second word */
setcond_tl(TCG_COND_LTU, t1, al, bl);
/* seed third word with 1, which will be result */
sub2_tl(t1, t2, ah, one, t1, zero);
sub2_tl(t1, rl, t1, t2, bh, zero);


+} else {
+gen_setcond_i128(tmpl, tmph, src1, src1h, src2, src2h, cond);
+tcg_gen_brcondi_tl(TCG_COND_NE, tmpl, 0, l);
+}


There are two setcond cases that invert their result; you should fold that in here and 
invert the branch condition.  As long as you're special casing 0, you might as well 
special case TCG_COND_LT/GE and test the sign of the high word.



r~

Re: [PATCH 10/15] hw/nvme: Make max_ioqpairs and msix_qsize configurable in runtime

[Klaus Jensen]

On Oct  7 18:24, Lukasz Maniak wrote:
> +static void nvme_update_msixcap_ts(PCIDevice *pci_dev, uint32_t table_size)
> +{
> +uint8_t *config;
> +
> +assert(pci_dev->msix_cap);

Not all platforms support msix, so an assert() is not right.



signature.asc
Description: PGP signature

Re: [PATCH 1/3] target/i386: move linuxboot_dma_enabled to X86MachineState

[Philippe Mathieu-Daudé]

On 10/20/21 16:02, Paolo Bonzini wrote:
> This removes a parameter from x86_load_linux, and will avoid code
> duplication between the linux and multiboot cases once multiboot
> starts to support DMA.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/i386/microvm.c | 5 -
>  hw/i386/pc.c  | 5 ++---
>  hw/i386/pc_piix.c | 3 ++-
>  hw/i386/pc_q35.c  | 3 ++-
>  hw/i386/x86.c | 5 +++--
>  include/hw/i386/pc.h  | 3 ---
>  include/hw/i386/x86.h | 5 +++--
>  7 files changed, 16 insertions(+), 13 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 10/15] hw/nvme: Make max_ioqpairs and msix_qsize configurable in runtime

[Klaus Jensen]

On Oct  7 18:24, Lukasz Maniak wrote:
> From: Łukasz Gieryk 
> 
> The Nvme device defines two properties: max_ioqpairs, msix_qsize. Having
> them as constants is problematic for SR-IOV support.
> 
> The SR-IOV feature introduces virtual resources (queues, interrupts)
> that can be assigned to PF and its dependent VFs. Each device, following
> a reset, should work with the configured number of queues. A single
> constant is no longer sufficient to hold the whole state.
> 
> This patch tries to solve the problem by introducing additional
> variables in NvmeCtrl’s state. The variables for, e.g., managing queues
> are therefore organized as:
> 
>  - n->params.max_ioqpairs – no changes, constant set by the user.
> 
>  - n->max_ioqpairs - (new) value derived from n->params.* in realize();
>  constant through device’s lifetime.
> 
>  - n->(mutable_state) – (not a part of this patch) user-configurable,
> specifies number of queues available _after_
> reset.
> 
>  - n->conf_ioqpairs - (new) used in all the places instead of the ‘old’
>   n->params.max_ioqpairs; initialized in realize()
>   and updated during reset() to reflect user’s
>   changes to the mutable state.
> 
> Since the number of available i/o queues and interrupts can change in
> runtime, buffers for sq/cqs and the MSIX-related structures are
> allocated big enough to handle the limits, to completely avoid the
> complicated reallocation. A helper function (nvme_update_msixcap_ts)
> updates the corresponding capability register, to signal configuration
> changes.
> 
> Signed-off-by: Łukasz Gieryk 

Instead of this, how about adding new parameters, say, sriov_vi_private
and sriov_vq_private. Then, max_ioqpairs and msix_qsize are still the
"physical" limits and the new parameters just reserve some for the
primary controller, the rest being available for flexsible resources.


signature.asc
Description: PGP signature

Re: [PATCH 3/3] target/i386: use DMA-enabled multiboot ROM for new-enough QEMU machine types

[Philippe Mathieu-Daudé]

On 10/20/21 16:02, Paolo Bonzini wrote:
> As long as fw_cfg supports DMA, the new ROM can be used also on older
> machine types because it has the same size as the existing one.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/i386/multiboot.c | 10 --
>  hw/i386/multiboot.h |  4 +++-
>  hw/i386/pc.c|  3 ++-
>  hw/i386/x86.c   |  2 +-
>  4 files changed, 14 insertions(+), 5 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé