Re: [PATCH 3/4] accel/tcg: Handle false negative lookup in page_check_range

[Philippe Mathieu-Daudé]

On 24/12/22 16:18, Richard Henderson wrote:

As in page_get_flags, we need to try again with the mmap
lock held if we fail a page lookup.

Signed-off-by: Richard Henderson 
---
  accel/tcg/user-exec.c | 39 ---
  1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 2c5c10d2e6..c72a806203 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -525,6 +525,7 @@ void page_set_flags(target_ulong start, target_ulong end, 
int flags)
  int page_check_range(target_ulong start, target_ulong len, int flags)
  {
  target_ulong last;
+int locked, ret;


have_mmap_lock() returns a boolean, can we declare 'locked'
as such, ...

  
  if (len == 0) {

  return 0;  /* trivial length */
@@ -535,42 +536,66 @@ int page_check_range(target_ulong start, target_ulong 
len, int flags)
  return -1; /* wrap around */
  }
  
+locked = have_mmap_lock();

  while (true) {
  PageFlagsNode *p = pageflags_find(start, last);
  int missing;
  
  if (!p) {

-return -1; /* entire region invalid */
+if (!locked) {
+/*
+ * Lockless lookups have false negatives.
+ * Retry with the lock held.
+ */
+mmap_lock();
+locked = -1;


... skip this confusing assignation, ...


+p = pageflags_find(start, last);
+}
+if (!p) {
+ret = -1; /* entire region invalid */
+break;
+}
  }
  if (start < p->itree.start) {
-return -1; /* initial bytes invalid */
+ret = -1; /* initial bytes invalid */
+break;
  }
  
  missing = flags & ~p->flags;

  if (missing & PAGE_READ) {
-return -1; /* page not readable */
+ret = -1; /* page not readable */
+break;
  }
  if (missing & PAGE_WRITE) {
  if (!(p->flags & PAGE_WRITE_ORG)) {
-return -1; /* page not writable */
+ret = -1; /* page not writable */
+break;
  }
  /* Asking about writable, but has been protected: undo. */
  if (!page_unprotect(start, 0)) {
-return -1;
+ret = -1;
+break;
  }
  /* TODO: page_unprotect should take a range, not a single page. */
  if (last - start < TARGET_PAGE_SIZE) {
-return 0; /* ok */
+ret = 0; /* ok */
+break;
  }
  start += TARGET_PAGE_SIZE;
  continue;
  }
  
  if (last <= p->itree.last) {

-return 0; /* ok */
+ret = 0; /* ok */
+break;
  }
  start = p->itree.last + 1;
  }
+
+if (locked < 0) {


... and check for !locked here?


+mmap_unlock();
+}
+return ret;
  }

Re: [PATCH 2/4] accel/tcg: Use g_free_rcu for user-exec interval trees

[Philippe Mathieu-Daudé]

On 24/12/22 16:18, Richard Henderson wrote:

Because we allow lockless lookups, we have to be careful
when it is freed.  Use rcu to delay the free until safe.

Signed-off-by: Richard Henderson 
---
  accel/tcg/user-exec.c | 18 ++
  1 file changed, 10 insertions(+), 8 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 

Re: [PATCH 2/2] vdpa: harden the error path if get_iova_range failed

[Jason Wang]

On Sat, Dec 24, 2022 at 7:49 PM Longpeng(Mike)  wrote:
>
> From: Longpeng 
>
> We should stop if the GET_IOVA_RANGE ioctl failed.

Acked-by: Jason Wang 

Thanks

>
> Signed-off-by: Longpeng 
> ---
>  net/vhost-vdpa.c | 8 +++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index ffdc435d19..e65023d013 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -797,7 +797,13 @@ int net_init_vhost_vdpa(const Netdev *netdev, const char 
> *name,
>  return queue_pairs;
>  }
>
> -vhost_vdpa_get_iova_range(vdpa_device_fd, _range);
> +r = vhost_vdpa_get_iova_range(vdpa_device_fd, _range);
> +if (unlikely(r < 0)) {
> +error_setg(errp, "vhost-vdpa: get iova range failed: %s",
> +   strerror(-r));
> +goto err;
> +}
> +
>  if (opts->x_svq) {
>  if (!vhost_vdpa_net_valid_svq_features(features, errp)) {
>  goto err_svq;
> --
> 2.23.0
>

Re: [PATCH 1/2] vdpa-dev: get iova range explicitly

[Jason Wang]

On Sat, Dec 24, 2022 at 7:49 PM Longpeng(Mike)  wrote:
>
> From: Longpeng 
>
> In commit a585fad26b ("vdpa: request iova_range only once") we remove
> GET_IOVA_RANGE form vhost_vdpa_init, the generic vdpa device will start
> without iova_range populated, so the device won't work. Let's call
> GET_IOVA_RANGE ioctl explicitly.

Acked-by: Jason Wang 

Thanks

>
> Fixes: a585fad26b2e6ccc ("vdpa: request iova_range only once")
> Signed-off-by: Longpeng 
> ---
>  hw/virtio/vdpa-dev.c   | 9 +
>  hw/virtio/vhost-vdpa.c | 7 +++
>  include/hw/virtio/vhost-vdpa.h | 2 ++
>  net/vhost-vdpa.c   | 8 
>  4 files changed, 18 insertions(+), 8 deletions(-)
>
> diff --git a/hw/virtio/vdpa-dev.c b/hw/virtio/vdpa-dev.c
> index db6ba61152..01b41eb0f1 100644
> --- a/hw/virtio/vdpa-dev.c
> +++ b/hw/virtio/vdpa-dev.c
> @@ -53,6 +53,7 @@ static void vhost_vdpa_device_realize(DeviceState *dev, 
> Error **errp)
>  {
>  VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>  VhostVdpaDevice *v = VHOST_VDPA_DEVICE(vdev);
> +struct vhost_vdpa_iova_range iova_range;
>  uint16_t max_queue_size;
>  struct vhost_virtqueue *vqs;
>  int i, ret;
> @@ -108,6 +109,14 @@ static void vhost_vdpa_device_realize(DeviceState *dev, 
> Error **errp)
>  v->dev.backend_features = 0;
>  v->started = false;
>
> +ret = vhost_vdpa_get_iova_range(v->vhostfd, _range);
> +if (ret < 0) {
> +error_setg(errp, "vhost-vdpa-device: get iova range failed: %s",
> +   strerror(-ret));
> +goto free_vqs;
> +}
> +v->vdpa.iova_range = iova_range;
> +
>  ret = vhost_dev_init(>dev, >vdpa, VHOST_BACKEND_TYPE_VDPA, 0, 
> NULL);
>  if (ret < 0) {
>  error_setg(errp, "vhost-vdpa-device: vhost initialization failed: 
> %s",
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 870265188a..109a2ee3bf 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -378,6 +378,13 @@ static int vhost_vdpa_add_status(struct vhost_dev *dev, 
> uint8_t status)
>  return 0;
>  }
>
> +int vhost_vdpa_get_iova_range(int fd, struct vhost_vdpa_iova_range 
> *iova_range)
> +{
> +int ret = ioctl(fd, VHOST_VDPA_GET_IOVA_RANGE, iova_range);
> +
> +return ret < 0 ? -errno : 0;
> +}
> +
>  /*
>   * The use of this function is for requests that only need to be
>   * applied once. Typically such request occurs at the beginning
> diff --git a/include/hw/virtio/vhost-vdpa.h b/include/hw/virtio/vhost-vdpa.h
> index 45b969a311..7997f09a8d 100644
> --- a/include/hw/virtio/vhost-vdpa.h
> +++ b/include/hw/virtio/vhost-vdpa.h
> @@ -51,6 +51,8 @@ typedef struct vhost_vdpa {
>  VhostVDPAHostNotifier notifier[VIRTIO_QUEUE_MAX];
>  } VhostVDPA;
>
> +int vhost_vdpa_get_iova_range(int fd, struct vhost_vdpa_iova_range 
> *iova_range);
> +
>  int vhost_vdpa_dma_map(struct vhost_vdpa *v, uint32_t asid, hwaddr iova,
> hwaddr size, void *vaddr, bool readonly);
>  int vhost_vdpa_dma_unmap(struct vhost_vdpa *v, uint32_t asid, hwaddr iova,
> diff --git a/net/vhost-vdpa.c b/net/vhost-vdpa.c
> index d36664f33a..ffdc435d19 100644
> --- a/net/vhost-vdpa.c
> +++ b/net/vhost-vdpa.c
> @@ -702,14 +702,6 @@ static NetClientState 
> *net_vhost_vdpa_init(NetClientState *peer,
>  return nc;
>  }
>
> -static int vhost_vdpa_get_iova_range(int fd,
> - struct vhost_vdpa_iova_range 
> *iova_range)
> -{
> -int ret = ioctl(fd, VHOST_VDPA_GET_IOVA_RANGE, iova_range);
> -
> -return ret < 0 ? -errno : 0;
> -}
> -
>  static int vhost_vdpa_get_features(int fd, uint64_t *features, Error **errp)
>  {
>  int ret = ioctl(fd, VHOST_GET_FEATURES, features);
> --
> 2.23.0
>

re:Re: [PATCH] target/riscv/cpu.c: Fix elen check

[503386372]

sorryforthat,iwilltryagainwithgit.sr.th.


-- 原始邮件 --
发件人: "Alistair Francis";
发件时间: 2022-12-28 13:46
收件人: "Elta"<503386...@qq.com;
收件人: 
"qemu-devel";"palmer";"alistair.francis";"bin.meng";"qemu-riscv";
主题: Re: [PATCH] target/riscv/cpu.c: Fix elen check



On Fri, Dec 16, 2022 at 1:10 AM Elta <503386...@qq.com wrote:

 Should be cpu-cfg.elen in range [8, 64].

 Signed-off-by: Dongxue Zhang 

Thanks for the patch!

I'm seeing weird formatting issues though, it looks like the encoding
is incorrect.

Did you use `git send-email` to send the patch? If not can you try
sending it again following the instructions here
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches

Alistair

 ---
  target/riscv/cpu.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
 index d14e95c9dc..1e8032c969 100644
 --- a/target/riscv/cpu.c
 +++ b/target/riscv/cpu.c
 @@ -870,7 +870,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
  "Vector extension ELEN must be power of 2");
  return;
  }
 -if (cpu-cfg.elen  64 || cpu-cfg.vlen < 8) {
 +if (cpu-cfg.elen  64 || cpu-cfg.elen < 8) {
  error_setg(errp,
  "Vector extension implementation only supports 
ELEN "
  "in the range [8, 64]");
 --
 2.17.1

[PATCH v9 6/9] target/riscv: add support for Zcmp extension

[Weiwei Li]

Add encode, trans* functions for Zcmp instructions

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/insn16.decode|  18 +++
 target/riscv/insn_trans/trans_rvzce.c.inc | 189 +-
 target/riscv/translate.c  |   5 +
 3 files changed, 211 insertions(+), 1 deletion(-)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index 47603ec1e0..4654c23052 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -21,6 +21,8 @@
 %rs1_3 7:3!function=ex_rvc_register
 %rs2_3 2:3!function=ex_rvc_register
 %rs2_5 2:5
+%sreg1 7:3!function=ex_sreg_register
+%sreg2 2:3!function=ex_sreg_register
 
 # Immediates:
 %imm_ci12:s1 2:5
@@ -45,6 +47,8 @@
 
 %zcb_b_uimm  5:1 6:1
 %zcb_h_uimm  5:1 !function=ex_shift_1
+%zcmp_spimm  2:2 !function=ex_shift_4
+%zcmp_rlist  4:4
 
 # Argument sets imported from insn32.decode:
   !extern
@@ -56,7 +60,9 @@
  imm rd   !extern
  shamt rs1 rd !extern
 rd rs1   !extern
+_s  rs1 rs2  !extern
 
+  zcmp_rlist zcmp_spimm
 
 # Formats 16:
 @cr  . .  ..   rs2=%rs2_5   rs1=%rd %rd
@@ -98,6 +104,8 @@
 @zcb_lh   ... . .. ... .. ... ..imm=%zcb_h_uimm  rs1=%rs1_3 rd=%rs2_3
 @zcb_sb   ... . .. ... .. ... ..imm=%zcb_b_uimm  rs1=%rs1_3 
rs2=%rs2_3
 @zcb_sh   ... . .. ... .. ... ..imm=%zcb_h_uimm  rs1=%rs1_3 
rs2=%rs2_3
+@zcmp ... ...     ..%zcmp_rlist   %zcmp_spimm
+@cm_mv... ...  ... .. ... ..  _s  rs2=%sreg2rs1=%sreg1
 
 # *** RV32/64C Standard Extension (Quadrant 0) ***
 {
@@ -177,6 +185,16 @@ slli  000 .  .  . 10 @c_shift2
 {
   sq  101  ... ... .. ... 10 @c_sqsp
   c_fsd   101   ..  . 10 @c_sdsp
+
+  # *** RV64 and RV32 Zcmp Extension ***
+  [
+cm_push 101  11000   .. 10 @zcmp
+cm_pop  101  11010   .. 10 @zcmp
+cm_popret   101  0   .. 10 @zcmp
+cm_popretz  101  11100   .. 10 @zcmp
+cm_mva01s   101  011 ... 11 ... 10 @cm_mv
+cm_mvsa01   101  011 ... 01 ... 10 @cm_mv
+  ]
 }
 sw110 .  .  . 10 @c_swsp
 
diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc 
b/target/riscv/insn_trans/trans_rvzce.c.inc
index de96c4afaf..30b53a9509 100644
--- a/target/riscv/insn_trans/trans_rvzce.c.inc
+++ b/target/riscv/insn_trans/trans_rvzce.c.inc
@@ -1,5 +1,5 @@
 /*
- * RISC-V translation routines for the Zcb Standard Extension.
+ * RISC-V translation routines for the Zc[b,mp] Standard Extensions.
  *
  * Copyright (c) 2021-2022 PLCT Lab
  *
@@ -21,6 +21,11 @@
 return false;   \
 } while (0)
 
+#define REQUIRE_ZCMP(ctx) do {   \
+if (!ctx->cfg_ptr->ext_zcmp) \
+return false;\
+} while (0)
+
 static bool trans_c_zext_b(DisasContext *ctx, arg_c_zext_b *a)
 {
 REQUIRE_ZCB(ctx);
@@ -98,3 +103,185 @@ static bool trans_c_sh(DisasContext *ctx, arg_c_sh *a)
 REQUIRE_ZCB(ctx);
 return gen_store(ctx, a, MO_UW);
 }
+
+#define X_S08
+#define X_S19
+#define X_Sn16
+
+static uint32_t decode_push_pop_list(DisasContext *ctx, target_ulong rlist)
+{
+uint32_t reg_bitmap = 0;
+
+if (ctx->cfg_ptr->ext_e && rlist > 6) {
+return 0;
+}
+
+switch (rlist) {
+case 15:
+reg_bitmap |=  1 << (X_Sn + 11) ;
+reg_bitmap |=  1 << (X_Sn + 10) ;
+/* FALL THROUGH */
+case 14:
+reg_bitmap |=  1 << (X_Sn + 9) ;
+/* FALL THROUGH */
+case 13:
+reg_bitmap |=  1 << (X_Sn + 8) ;
+/* FALL THROUGH */
+case 12:
+reg_bitmap |=  1 << (X_Sn + 7) ;
+/* FALL THROUGH */
+case 11:
+reg_bitmap |=  1 << (X_Sn + 6) ;
+/* FALL THROUGH */
+case 10:
+reg_bitmap |=  1 << (X_Sn + 5) ;
+/* FALL THROUGH */
+case 9:
+reg_bitmap |=  1 << (X_Sn + 4) ;
+/* FALL THROUGH */
+case 8:
+reg_bitmap |=  1 << (X_Sn + 3) ;
+/* FALL THROUGH */
+case 7:
+reg_bitmap |=  1 << (X_Sn + 2) ;
+/* FALL THROUGH */
+case 6:
+reg_bitmap |=  1 << X_S1 ;
+/* FALL THROUGH */
+case 5:
+reg_bitmap |= 1 << X_S0;
+/* FALL THROUGH */
+case 4:
+reg_bitmap |= 1 << xRA;
+break;
+default:
+break;
+}
+
+return reg_bitmap;
+}
+
+static bool gen_pop(DisasContext *ctx, arg_zcmp *a, bool ret, bool ret_val)
+{
+REQUIRE_ZCMP(ctx);
+
+uint32_t reg_bitmap = decode_push_pop_list(ctx, a->zcmp_rlist);
+if (reg_bitmap == 0) {
+return false;
+}
+
+MemOp memop = get_ol(ctx) == MXL_RV32 ? MO_TEUL : MO_TEUQ;
+int reg_size = memop_size(memop);
+target_ulong stack_adj = 

[PATCH v9 7/9] target/riscv: add support for Zcmt extension

[Weiwei Li]

Add encode, trans* functions and helper functions support for Zcmt
instrutions
Add support for jvt csr

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/cpu.h|  4 ++
 target/riscv/cpu_bits.h   |  7 +++
 target/riscv/csr.c| 38 +++-
 target/riscv/helper.h |  3 ++
 target/riscv/insn16.decode|  7 ++-
 target/riscv/insn_trans/trans_rvzce.c.inc | 28 +++-
 target/riscv/machine.c| 19 
 target/riscv/meson.build  |  3 +-
 target/riscv/zce_helper.c | 55 +++
 9 files changed, 159 insertions(+), 5 deletions(-)
 create mode 100644 target/riscv/zce_helper.c

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 71545041a0..c7b8e7f238 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -181,6 +181,8 @@ struct CPUArchState {
 
 uint32_t features;
 
+target_ulong jvt;
+
 #ifdef CONFIG_USER_ONLY
 uint32_t elf_flags;
 #endif
@@ -600,6 +602,8 @@ void riscv_cpu_set_aia_ireg_rmw_fn(CPURISCVState *env, 
uint32_t priv,
  target_ulong new_val,
  target_ulong write_mask),
void *rmw_fn_arg);
+
+RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit);
 #endif
 void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
 
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 8b0d7e20ea..ce347e5575 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -319,6 +319,7 @@
 #define SMSTATEEN_MAX_COUNT 4
 #define SMSTATEEN0_CS   (1ULL << 0)
 #define SMSTATEEN0_FCSR (1ULL << 1)
+#define SMSTATEEN0_JVT  (1ULL << 2)
 #define SMSTATEEN0_HSCONTXT (1ULL << 57)
 #define SMSTATEEN0_IMSIC(1ULL << 58)
 #define SMSTATEEN0_AIA  (1ULL << 59)
@@ -523,6 +524,9 @@
 /* Crypto Extension */
 #define CSR_SEED0x015
 
+/* Zcmt Extension */
+#define CSR_JVT 0x017
+
 /* mstatus CSR bits */
 #define MSTATUS_UIE 0x0001
 #define MSTATUS_SIE 0x0002
@@ -894,4 +898,7 @@ typedef enum RISCVException {
 #define MHPMEVENT_IDX_MASK 0xF
 #define MHPMEVENT_SSCOF_RESVD  16
 
+/* JVT CSR bits */
+#define JVT_MODE   0x3F
+#define JVT_BASE   (~0x3F)
 #endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e6f8250929..a752e8b215 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -42,8 +42,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 
 /* Predicates */
 #if !defined(CONFIG_USER_ONLY)
-static RISCVException smstateen_acc_ok(CPURISCVState *env, int index,
-   uint64_t bit)
+RISCVException smstateen_acc_ok(CPURISCVState *env, int index, uint64_t bit)
 {
 bool virt = riscv_cpu_virt_enabled(env);
 CPUState *cs = env_cpu(env);
@@ -163,6 +162,24 @@ static RISCVException ctr32(CPURISCVState *env, int csrno)
 return ctr(env, csrno);
 }
 
+static RISCVException zcmt(CPURISCVState *env, int csrno)
+{
+RISCVCPU *cpu = env_archcpu(env);
+
+if (!cpu->cfg.ext_zcmt) {
+return RISCV_EXCP_ILLEGAL_INST;
+}
+
+#if !defined(CONFIG_USER_ONLY)
+RISCVException ret = smstateen_acc_ok(env, 0, SMSTATEEN0_JVT);
+if (ret != RISCV_EXCP_NONE) {
+return ret;
+}
+#endif
+
+return RISCV_EXCP_NONE;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 static RISCVException mctr(CPURISCVState *env, int csrno)
 {
@@ -3980,6 +3997,20 @@ RISCVException riscv_csrrw_debug(CPURISCVState *env, int 
csrno,
 return ret;
 }
 
+static RISCVException read_jvt(CPURISCVState *env, int csrno,
+   target_ulong *val)
+{
+*val = env->jvt;
+return RISCV_EXCP_NONE;
+}
+
+static RISCVException write_jvt(CPURISCVState *env, int csrno,
+target_ulong val)
+{
+env->jvt = val;
+return RISCV_EXCP_NONE;
+}
+
 /* Control and Status Register function table */
 riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 /* User Floating-Point CSRs */
@@ -4017,6 +4048,9 @@ riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
 /* Crypto Extension */
 [CSR_SEED] = { "seed", seed, NULL, NULL, rmw_seed },
 
+/* Zcmt Extension */
+[CSR_JVT] = {"jvt", zcmt, read_jvt, write_jvt},
+
 #if !defined(CONFIG_USER_ONLY)
 /* Machine Timers and Counters */
 [CSR_MCYCLE]= { "mcycle",any,   read_hpmcounter,
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 227c7122ef..d979f0bfc4 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -1136,3 +1136,6 @@ DEF_HELPER_FLAGS_1(aes64im, TCG_CALL_NO_RWG_SE, tl, tl)
 
 DEF_HELPER_FLAGS_3(sm4ed, TCG_CALL_NO_RWG_SE, tl, tl, tl, tl)
 

[PATCH v9 8/9] target/riscv: expose properties for Zc* extension

[Weiwei Li]

Expose zca,zcb,zcf,zcd,zcmp,zcmt properties

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Alistair Francis 
---
 target/riscv/cpu.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 4f421130aa..bc954d9b85 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -81,6 +81,12 @@ static const struct isa_ext_data isa_edata_arr[] = {
 ISA_EXT_DATA_ENTRY(zfhmin, true, PRIV_VERSION_1_12_0, ext_zfhmin),
 ISA_EXT_DATA_ENTRY(zfinx, true, PRIV_VERSION_1_12_0, ext_zfinx),
 ISA_EXT_DATA_ENTRY(zdinx, true, PRIV_VERSION_1_12_0, ext_zdinx),
+ISA_EXT_DATA_ENTRY(zca, true, PRIV_VERSION_1_12_0, ext_zca),
+ISA_EXT_DATA_ENTRY(zcb, true, PRIV_VERSION_1_12_0, ext_zcb),
+ISA_EXT_DATA_ENTRY(zcf, true, PRIV_VERSION_1_12_0, ext_zcf),
+ISA_EXT_DATA_ENTRY(zcd, true, PRIV_VERSION_1_12_0, ext_zcd),
+ISA_EXT_DATA_ENTRY(zcmp, true, PRIV_VERSION_1_12_0, ext_zcmp),
+ISA_EXT_DATA_ENTRY(zcmt, true, PRIV_VERSION_1_12_0, ext_zcmt),
 ISA_EXT_DATA_ENTRY(zba, true, PRIV_VERSION_1_12_0, ext_zba),
 ISA_EXT_DATA_ENTRY(zbb, true, PRIV_VERSION_1_12_0, ext_zbb),
 ISA_EXT_DATA_ENTRY(zbc, true, PRIV_VERSION_1_12_0, ext_zbc),
@@ -1118,6 +1124,13 @@ static Property riscv_cpu_extensions[] = {
 
 /* These are experimental so mark with 'x-' */
 DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false),
+
+DEFINE_PROP_BOOL("x-zca", RISCVCPU, cfg.ext_zca, false),
+DEFINE_PROP_BOOL("x-zcb", RISCVCPU, cfg.ext_zcb, false),
+DEFINE_PROP_BOOL("x-zcd", RISCVCPU, cfg.ext_zcd, false),
+DEFINE_PROP_BOOL("x-zcf", RISCVCPU, cfg.ext_zcf, false),
+DEFINE_PROP_BOOL("x-zcmp", RISCVCPU, cfg.ext_zcmp, false),
+DEFINE_PROP_BOOL("x-zcmt", RISCVCPU, cfg.ext_zcmt, false),
 /* ePMP 0.9.3 */
 DEFINE_PROP_BOOL("x-epmp", RISCVCPU, cfg.epmp, false),
 DEFINE_PROP_BOOL("x-smaia", RISCVCPU, cfg.ext_smaia, false),
-- 
2.25.1

[PATCH v9 3/9] target/riscv: add support for Zcf extension

[Weiwei Li]

Separate c_flw/c_fsw from flw/fsw to add check for Zcf extension

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/insn16.decode  |  8 
 target/riscv/insn_trans/trans_rvf.c.inc | 18 ++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index ccfe59f294..f3ea650325 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -109,11 +109,11 @@ sw110  ... ... .. ... 00 @cs_w
 # *** RV32C and RV64C specific Standard Extension (Quadrant 0) ***
 {
   ld  011  ... ... .. ... 00 @cl_d
-  flw 011  ... ... .. ... 00 @cl_w
+  c_flw   011  ... ... .. ... 00 @cl_w
 }
 {
   sd  111  ... ... .. ... 00 @cs_d
-  fsw 111  ... ... .. ... 00 @cs_w
+  c_fsw   111  ... ... .. ... 00 @cs_w
 }
 
 # *** RV32/64C Standard Extension (Quadrant 1) ***
@@ -174,9 +174,9 @@ sw110 .  .  . 10 @c_swsp
 {
   c64_illegal 011 -  0  - 10 # c.ldsp, RES rd=0
   ld  011 .  .  . 10 @c_ldsp
-  flw 011 .  .  . 10 @c_lwsp
+  c_flw   011 .  .  . 10 @c_lwsp
 }
 {
   sd  111 .  .  . 10 @c_sdsp
-  fsw 111 .  .  . 10 @c_swsp
+  c_fsw   111 .  .  . 10 @c_swsp
 }
diff --git a/target/riscv/insn_trans/trans_rvf.c.inc 
b/target/riscv/insn_trans/trans_rvf.c.inc
index 965e1f8d11..5df9c148dc 100644
--- a/target/riscv/insn_trans/trans_rvf.c.inc
+++ b/target/riscv/insn_trans/trans_rvf.c.inc
@@ -30,6 +30,12 @@
 } \
 } while (0)
 
+#define REQUIRE_ZCF(ctx) do {  \
+if (!ctx->cfg_ptr->ext_zcf) {  \
+return false;  \
+}  \
+} while (0)
+
 static bool trans_flw(DisasContext *ctx, arg_flw *a)
 {
 TCGv_i64 dest;
@@ -61,6 +67,18 @@ static bool trans_fsw(DisasContext *ctx, arg_fsw *a)
 return true;
 }
 
+static bool trans_c_flw(DisasContext *ctx, arg_flw *a)
+{
+REQUIRE_ZCF(ctx);
+return trans_flw(ctx, a);
+}
+
+static bool trans_c_fsw(DisasContext *ctx, arg_fsw *a)
+{
+REQUIRE_ZCF(ctx);
+return trans_fsw(ctx, a);
+}
+
 static bool trans_fmadd_s(DisasContext *ctx, arg_fmadd_s *a)
 {
 REQUIRE_FPU;
-- 
2.25.1

[PATCH v9 4/9] target/riscv: add support for Zcd extension

[Weiwei Li]

Separate c_fld/c_fsd from fld/fsd to add additional check for
c.fld{sp}/c.fsd{sp} which is useful for zcmp/zcmt to reuse
their encodings

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/insn16.decode  |  8 
 target/riscv/insn_trans/trans_rvd.c.inc | 18 ++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index f3ea650325..b62664b6af 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -97,12 +97,12 @@
 }
 {
   lq  001  ... ... .. ... 00 @cl_q
-  fld 001  ... ... .. ... 00 @cl_d
+  c_fld   001  ... ... .. ... 00 @cl_d
 }
 lw010  ... ... .. ... 00 @cl_w
 {
   sq  101  ... ... .. ... 00 @cs_q
-  fsd 101  ... ... .. ... 00 @cs_d
+  c_fsd   101  ... ... .. ... 00 @cs_d
 }
 sw110  ... ... .. ... 00 @cs_w
 
@@ -148,7 +148,7 @@ addw  100 1 11 ... 01 ... 01 @cs_2
 slli  000 .  .  . 10 @c_shift2
 {
   lq  001  ... ... .. ... 10 @c_lqsp
-  fld 001 .  .  . 10 @c_ldsp
+  c_fld   001 .  .  . 10 @c_ldsp
 }
 {
   illegal 010 -  0  - 10 # c.lwsp, RES rd=0
@@ -166,7 +166,7 @@ slli  000 .  .  . 10 @c_shift2
 }
 {
   sq  101  ... ... .. ... 10 @c_sqsp
-  fsd 101   ..  . 10 @c_sdsp
+  c_fsd   101   ..  . 10 @c_sdsp
 }
 sw110 .  .  . 10 @c_swsp
 
diff --git a/target/riscv/insn_trans/trans_rvd.c.inc 
b/target/riscv/insn_trans/trans_rvd.c.inc
index 6e3159b797..47849ffdfd 100644
--- a/target/riscv/insn_trans/trans_rvd.c.inc
+++ b/target/riscv/insn_trans/trans_rvd.c.inc
@@ -31,6 +31,12 @@
 } \
 } while (0)
 
+#define REQUIRE_ZCD(ctx) do { \
+if (!ctx->cfg_ptr->ext_zcd) {  \
+return false; \
+} \
+} while (0)
+
 static bool trans_fld(DisasContext *ctx, arg_fld *a)
 {
 TCGv addr;
@@ -59,6 +65,18 @@ static bool trans_fsd(DisasContext *ctx, arg_fsd *a)
 return true;
 }
 
+static bool trans_c_fld(DisasContext *ctx, arg_fld *a)
+{
+REQUIRE_ZCD(ctx);
+return trans_fld(ctx, a);
+}
+
+static bool trans_c_fsd(DisasContext *ctx, arg_fsd *a)
+{
+REQUIRE_ZCD(ctx);
+return trans_fsd(ctx, a);
+}
+
 static bool trans_fmadd_d(DisasContext *ctx, arg_fmadd_d *a)
 {
 REQUIRE_FPU;
-- 
2.25.1

[PATCH v9 0/9] support subsets of code size reduction extension

[Weiwei Li]

This patchset implements RISC-V Zc* extension v1.0.0.RC5.7 version instructions.

Specification:
https://github.com/riscv/riscv-code-size-reduction/tree/main/Zc-specification

The port is available here:
https://github.com/plctlab/plct-qemu/tree/plct-zce-upstream-v9

To test Zc* implementation, specify cpu argument with 
'x-zca=true,x-zcb=true,x-zcf=true,f=true" and "x-zcd=true,d=true" (or 
"x-zcmp=true,x-zcmt=true" with c or d=false) to enable Zca/Zcb/Zcf and Zcd(or 
Zcmp,Zcmt) extensions support.


This implementation can pass the basic zc tests from 
https://github.com/yulong-plct/zc-test

v9:
* rebase on riscv-to-apply.next

v8:
* improve disas support in Patch 9

v7:
* Fix description for Zca

v6：
* fix base address for jump table in Patch 7
* rebase on riscv-to-apply.next

v5:
* fix exception unwind problem for cpu_ld*_code in helper of cm_jalt

v4:
* improve Zcmp suggested by Richard
* fix stateen related check for Zcmt

v3:
* update the solution for Zcf to the way of Zcd
* update Zcb to reuse gen_load/store
* use trans function instead of helper for push/pop

v2:
* add check for relationship between Zca/Zcf/Zcd with C/F/D based on related 
discussion in review of Zc* spec
* separate c.fld{sp}/fsd{sp} with fld{sp}/fsd{sp} before support of zcmp/zcmt

Weiwei Li (9):
  target/riscv: add cfg properties for Zc* extension
  target/riscv: add support for Zca extension
  target/riscv: add support for Zcf extension
  target/riscv: add support for Zcd extension
  target/riscv: add support for Zcb extension
  target/riscv: add support for Zcmp extension
  target/riscv: add support for Zcmt extension
  target/riscv: expose properties for Zc* extension
  disas/riscv.c: add disasm support for Zc*

 disas/riscv.c | 228 +++-
 target/riscv/cpu.c|  56 
 target/riscv/cpu.h|  10 +
 target/riscv/cpu_bits.h   |   7 +
 target/riscv/csr.c|  38 ++-
 target/riscv/helper.h |   3 +
 target/riscv/insn16.decode|  63 -
 target/riscv/insn_trans/trans_rvd.c.inc   |  18 ++
 target/riscv/insn_trans/trans_rvf.c.inc   |  18 ++
 target/riscv/insn_trans/trans_rvi.c.inc   |   4 +-
 target/riscv/insn_trans/trans_rvzce.c.inc | 313 ++
 target/riscv/machine.c|  19 ++
 target/riscv/meson.build  |   3 +-
 target/riscv/translate.c  |  15 +-
 target/riscv/zce_helper.c |  55 
 15 files changed, 834 insertions(+), 16 deletions(-)
 create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc
 create mode 100644 target/riscv/zce_helper.c

-- 
2.25.1

[PATCH v9 1/9] target/riscv: add cfg properties for Zc* extension

[Weiwei Li]

Add properties for Zca,Zcb,Zcf,Zcd,Zcmp,Zcmt extension
Add check for these properties

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/cpu.c | 43 +++
 target/riscv/cpu.h |  6 ++
 2 files changed, 49 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index cc75ca7667..4f421130aa 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -810,6 +810,49 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 }
 }
 
+if (cpu->cfg.ext_c) {
+cpu->cfg.ext_zca = true;
+if (cpu->cfg.ext_f && env->misa_mxl_max == MXL_RV32) {
+cpu->cfg.ext_zcf = true;
+}
+if (cpu->cfg.ext_d) {
+cpu->cfg.ext_zcd = true;
+}
+}
+
+if (env->misa_mxl_max != MXL_RV32 && cpu->cfg.ext_zcf) {
+error_setg(errp, "Zcf extension is only relevant to RV32");
+return;
+}
+
+if (!cpu->cfg.ext_f && cpu->cfg.ext_zcf) {
+error_setg(errp, "Zcf extension requires F extension");
+return;
+}
+
+if (!cpu->cfg.ext_d && cpu->cfg.ext_zcd) {
+error_setg(errp, "Zcd extension requires D extension");
+return;
+}
+
+if ((cpu->cfg.ext_zcf || cpu->cfg.ext_zcd || cpu->cfg.ext_zcb ||
+ cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt) && !cpu->cfg.ext_zca) {
+error_setg(errp, "Zcf/Zcd/Zcb/Zcmp/Zcmt extensions require Zca "
+ "extension");
+return;
+}
+
+if (cpu->cfg.ext_zcd && (cpu->cfg.ext_zcmp || cpu->cfg.ext_zcmt)) {
+error_setg(errp, "Zcmp/Zcmt extensions are incompatible with "
+ "Zcd extension");
+return;
+}
+
+if (cpu->cfg.ext_zcmt && !cpu->cfg.ext_icsr) {
+error_setg(errp, "Zcmt extension requires Zicsr extension");
+return;
+}
+
 if (cpu->cfg.ext_zk) {
 cpu->cfg.ext_zkn = true;
 cpu->cfg.ext_zkr = true;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f5609b62a2..71545041a0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -434,6 +434,12 @@ struct RISCVCPUConfig {
 bool ext_zbkc;
 bool ext_zbkx;
 bool ext_zbs;
+bool ext_zca;
+bool ext_zcb;
+bool ext_zcd;
+bool ext_zcf;
+bool ext_zcmp;
+bool ext_zcmt;
 bool ext_zk;
 bool ext_zkn;
 bool ext_zknd;
-- 
2.25.1

[PATCH v9 9/9] disas/riscv.c: add disasm support for Zc*

[Weiwei Li]

Zcmp/Zcmt instructions will override disasm for c.fld*/c.fsd*
instructions currently

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Acked-by: Alistair Francis 
---
 disas/riscv.c | 228 +-
 1 file changed, 227 insertions(+), 1 deletion(-)

diff --git a/disas/riscv.c b/disas/riscv.c
index d216b9c39b..f75da98540 100644
--- a/disas/riscv.c
+++ b/disas/riscv.c
@@ -163,6 +163,13 @@ typedef enum {
 rv_codec_v_i,
 rv_codec_vsetvli,
 rv_codec_vsetivli,
+rv_codec_zcb_ext,
+rv_codec_zcb_mul,
+rv_codec_zcb_lb,
+rv_codec_zcb_lh,
+rv_codec_zcmp_cm_pushpop,
+rv_codec_zcmp_cm_mv,
+rv_codec_zcmt_jt,
 } rv_codec;
 
 typedef enum {
@@ -935,6 +942,26 @@ typedef enum {
 rv_op_vsetvli = 766,
 rv_op_vsetivli = 767,
 rv_op_vsetvl = 768,
+rv_op_c_zext_b = 769,
+rv_op_c_sext_b = 770,
+rv_op_c_zext_h = 771,
+rv_op_c_sext_h = 772,
+rv_op_c_zext_w = 773,
+rv_op_c_not = 774,
+rv_op_c_mul = 775,
+rv_op_c_lbu = 776,
+rv_op_c_lhu = 777,
+rv_op_c_lh = 778,
+rv_op_c_sb = 779,
+rv_op_c_sh = 780,
+rv_op_cm_push = 781,
+rv_op_cm_pop = 782,
+rv_op_cm_popret = 783,
+rv_op_cm_popretz = 784,
+rv_op_cm_mva01s = 785,
+rv_op_cm_mvsa01 = 786,
+rv_op_cm_jt = 787,
+rv_op_cm_jalt = 788,
 } rv_op;
 
 /* structures */
@@ -958,6 +985,7 @@ typedef struct {
 uint8_t   rnum;
 uint8_t   vm;
 uint32_t  vzimm;
+uint8_t   rlist;
 } rv_decode;
 
 typedef struct {
@@ -1070,6 +1098,10 @@ static const char rv_vreg_name_sym[32][4] = {
 #define rv_fmt_vd_vm  "O\tDm"
 #define rv_fmt_vsetvli"O\t0,1,v"
 #define rv_fmt_vsetivli   "O\t0,u,v"
+#define rv_fmt_rs1_rs2_zce_ldst   "O\t2,i(1)"
+#define rv_fmt_push_rlist "O\tx,-i"
+#define rv_fmt_pop_rlist  "O\tx,i"
+#define rv_fmt_zcmt_index "O\ti"
 
 /* pseudo-instruction constraints */
 
@@ -2065,7 +2097,27 @@ const rv_opcode_data opcode_data[] = {
 { "vsext.vf8", rv_codec_v_r, rv_fmt_vd_vs2_vm, NULL, rv_op_vsext_vf8, 
rv_op_vsext_vf8, 0 },
 { "vsetvli", rv_codec_vsetvli, rv_fmt_vsetvli, NULL, rv_op_vsetvli, 
rv_op_vsetvli, 0 },
 { "vsetivli", rv_codec_vsetivli, rv_fmt_vsetivli, NULL, rv_op_vsetivli, 
rv_op_vsetivli, 0 },
-{ "vsetvl", rv_codec_r, rv_fmt_rd_rs1_rs2, NULL, rv_op_vsetvl, 
rv_op_vsetvl, 0 }
+{ "vsetvl", rv_codec_r, rv_fmt_rd_rs1_rs2, NULL, rv_op_vsetvl, 
rv_op_vsetvl, 0 },
+{ "c.zext.b", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+{ "c.sext.b", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+{ "c.zext.h", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+{ "c.sext.h", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+{ "c.zext.w", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+{ "c.not", rv_codec_zcb_ext, rv_fmt_rd, NULL, 0 },
+{ "c.mul", rv_codec_zcb_mul, rv_fmt_rd_rs2, NULL, 0, 0 },
+{ "c.lbu", rv_codec_zcb_lb, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+{ "c.lhu", rv_codec_zcb_lh, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+{ "c.lh", rv_codec_zcb_lh, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+{ "c.sb", rv_codec_zcb_lb, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+{ "c.sh", rv_codec_zcb_lh, rv_fmt_rs1_rs2_zce_ldst, NULL, 0, 0, 0 },
+{ "cm.push", rv_codec_zcmp_cm_pushpop, rv_fmt_push_rlist, NULL, 0, 0 },
+{ "cm.pop", rv_codec_zcmp_cm_pushpop, rv_fmt_pop_rlist, NULL, 0, 0 },
+{ "cm.popret", rv_codec_zcmp_cm_pushpop, rv_fmt_pop_rlist, NULL, 0, 0, 0 },
+{ "cm.popretz", rv_codec_zcmp_cm_pushpop, rv_fmt_pop_rlist, NULL, 0, 0 },
+{ "cm.mva01s", rv_codec_zcmp_cm_mv, rv_fmt_rd_rs2, NULL, 0, 0, 0 },
+{ "cm.mvsa01", rv_codec_zcmp_cm_mv, rv_fmt_rd_rs2, NULL, 0, 0, 0 },
+{ "cm.jt", rv_codec_zcmt_jt, rv_fmt_zcmt_index, NULL, 0 },
+{ "cm.jalt", rv_codec_zcmt_jt, rv_fmt_zcmt_index, NULL, 0 },
 };
 
 /* CSR names */
@@ -2084,6 +2136,7 @@ static const char *csr_name(int csrno)
 case 0x000a: return "vxrm";
 case 0x000f: return "vcsr";
 case 0x0015: return "seed";
+case 0x0017: return "jvt";
 case 0x0040: return "uscratch";
 case 0x0041: return "uepc";
 case 0x0042: return "ucause";
@@ -2306,6 +2359,24 @@ static void decode_inst_opcode(rv_decode *dec, rv_isa 
isa)
 op = rv_op_c_ld;
 }
 break;
+case 4:
+switch ((inst >> 10) & 0b111) {
+case 0: op = rv_op_c_lbu; break;
+case 1:
+if (((inst >> 6) & 1) == 0) {
+op = rv_op_c_lhu;
+} else {
+op = rv_op_c_lh;
+}
+break;
+case 2: op = rv_op_c_sb; break;
+case 3:
+if (((inst >> 6) & 1) == 0) {
+op = rv_op_c_sh;
+}
+break;
+}
+break;
 case 5:
 if (isa == rv128) {
 op = rv_op_c_sq;
@@ 

[PATCH v9 2/9] target/riscv: add support for Zca extension

[Weiwei Li]

Modify the check for C extension to Zca (C implies Zca)

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
Reviewed-by: Wilfred Mallawa 
---
 target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
 target/riscv/translate.c| 8 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/target/riscv/insn_trans/trans_rvi.c.inc 
b/target/riscv/insn_trans/trans_rvi.c.inc
index 4496f21266..ef7c3002b0 100644
--- a/target/riscv/insn_trans/trans_rvi.c.inc
+++ b/target/riscv/insn_trans/trans_rvi.c.inc
@@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr *a)
 tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
 
 gen_set_pc(ctx, cpu_pc);
-if (!has_ext(ctx, RVC)) {
+if (!ctx->cfg_ptr->ext_zca) {
 TCGv t0 = tcg_temp_new();
 
 misaligned = gen_new_label();
@@ -178,7 +178,7 @@ static bool gen_branch(DisasContext *ctx, arg_b *a, TCGCond 
cond)
 
 gen_set_label(l); /* branch taken */
 
-if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3)) {
+if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) & 0x3)) {
 /* misaligned */
 gen_exception_inst_addr_mis(ctx);
 } else {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index df38db7553..93ec2b7c55 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -557,7 +557,7 @@ static void gen_jal(DisasContext *ctx, int rd, target_ulong 
imm)
 
 /* check misaligned: */
 next_pc = ctx->base.pc_next + imm;
-if (!has_ext(ctx, RVC)) {
+if (!ctx->cfg_ptr->ext_zca) {
 if ((next_pc & 0x3) != 0) {
 gen_exception_inst_addr_mis(ctx);
 return;
@@ -1099,7 +1099,11 @@ static void decode_opc(CPURISCVState *env, DisasContext 
*ctx, uint16_t opcode)
 if (insn_len(opcode) == 2) {
 ctx->opcode = opcode;
 ctx->pc_succ_insn = ctx->base.pc_next + 2;
-if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
+/*
+ * The Zca extension is added as way to refer to instructions in the C
+ * extension that do not include the floating-point loads and stores
+ */
+if (ctx->cfg_ptr->ext_zca && decode_insn16(ctx, opcode)) {
 return;
 }
 } else {
-- 
2.25.1

[PATCH v9 5/9] target/riscv: add support for Zcb extension

[Weiwei Li]

Add encode and trans* functions support for Zcb instructions

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: Richard Henderson 
Reviewed-by: Alistair Francis 
---
 target/riscv/insn16.decode|  24 ++
 target/riscv/insn_trans/trans_rvzce.c.inc | 100 ++
 target/riscv/translate.c  |   2 +
 3 files changed, 126 insertions(+)
 create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc

diff --git a/target/riscv/insn16.decode b/target/riscv/insn16.decode
index b62664b6af..47603ec1e0 100644
--- a/target/riscv/insn16.decode
+++ b/target/riscv/insn16.decode
@@ -43,6 +43,8 @@
 %imm_addi16sp  12:s1 3:2 5:1 2:1 6:1 !function=ex_shift_4
 %imm_lui   12:s1 2:5 !function=ex_shift_12
 
+%zcb_b_uimm  5:1 6:1
+%zcb_h_uimm  5:1 !function=ex_shift_1
 
 # Argument sets imported from insn32.decode:
   !extern
@@ -53,6 +55,7 @@
  imm rs2 rs1  !extern
  imm rd   !extern
  shamt rs1 rd !extern
+rd rs1   !extern
 
 
 # Formats 16:
@@ -89,6 +92,13 @@
 
 @c_andi ... . .. ... . ..  imm=%imm_ci rs1=%rs1_3 rd=%rs1_3
 
+@zcb_unary... ...  ... .. ... ..rs1=%rs1_3 rd=%rs1_3
+@zcb_binary   ... ...  ... .. ... ..rs2=%rs2_3   rs1=%rs1_3 rd=%rs1_3
+@zcb_lb   ... . .. ... .. ... ..imm=%zcb_b_uimm  rs1=%rs1_3 rd=%rs2_3
+@zcb_lh   ... . .. ... .. ... ..imm=%zcb_h_uimm  rs1=%rs1_3 rd=%rs2_3
+@zcb_sb   ... . .. ... .. ... ..imm=%zcb_b_uimm  rs1=%rs1_3 
rs2=%rs2_3
+@zcb_sh   ... . .. ... .. ... ..imm=%zcb_h_uimm  rs1=%rs1_3 
rs2=%rs2_3
+
 # *** RV32/64C Standard Extension (Quadrant 0) ***
 {
   # Opcode of all zeros is illegal; rd != 0, nzuimm == 0 is reserved.
@@ -180,3 +190,17 @@ sw110 .  .  . 10 @c_swsp
   sd  111 .  .  . 10 @c_sdsp
   c_fsw   111 .  .  . 10 @c_swsp
 }
+
+# *** RV64 and RV32 Zcb Extension ***
+c_zext_b  100 111  ... 11 000 01 @zcb_unary
+c_sext_b  100 111  ... 11 001 01 @zcb_unary
+c_zext_h  100 111  ... 11 010 01 @zcb_unary
+c_sext_h  100 111  ... 11 011 01 @zcb_unary
+c_zext_w  100 111  ... 11 100 01 @zcb_unary
+c_not 100 111  ... 11 101 01 @zcb_unary
+c_mul 100 111  ... 10 ... 01 @zcb_binary
+c_lbu 100 000  ... .. ... 00 @zcb_lb
+c_lhu 100 001  ... 0. ... 00 @zcb_lh
+c_lh  100 001  ... 1. ... 00 @zcb_lh
+c_sb  100 010  ... .. ... 00 @zcb_sb
+c_sh  100 011  ... 0. ... 00 @zcb_sh
diff --git a/target/riscv/insn_trans/trans_rvzce.c.inc 
b/target/riscv/insn_trans/trans_rvzce.c.inc
new file mode 100644
index 00..de96c4afaf
--- /dev/null
+++ b/target/riscv/insn_trans/trans_rvzce.c.inc
@@ -0,0 +1,100 @@
+/*
+ * RISC-V translation routines for the Zcb Standard Extension.
+ *
+ * Copyright (c) 2021-2022 PLCT Lab
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#define REQUIRE_ZCB(ctx) do {   \
+if (!ctx->cfg_ptr->ext_zcb) \
+return false;   \
+} while (0)
+
+static bool trans_c_zext_b(DisasContext *ctx, arg_c_zext_b *a)
+{
+REQUIRE_ZCB(ctx);
+return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext8u_tl);
+}
+
+static bool trans_c_zext_h(DisasContext *ctx, arg_c_zext_h *a)
+{
+REQUIRE_ZCB(ctx);
+REQUIRE_ZBB(ctx);
+return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext16u_tl);
+}
+
+static bool trans_c_sext_b(DisasContext *ctx, arg_c_sext_b *a)
+{
+REQUIRE_ZCB(ctx);
+REQUIRE_ZBB(ctx);
+return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext8s_tl);
+}
+
+static bool trans_c_sext_h(DisasContext *ctx, arg_c_sext_h *a)
+{
+REQUIRE_ZCB(ctx);
+REQUIRE_ZBB(ctx);
+return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext16s_tl);
+}
+
+static bool trans_c_zext_w(DisasContext *ctx, arg_c_zext_w *a)
+{
+REQUIRE_64BIT(ctx);
+REQUIRE_ZCB(ctx);
+REQUIRE_ZBA(ctx);
+return gen_unary(ctx, a, EXT_NONE, tcg_gen_ext32u_tl);
+}
+
+static bool trans_c_not(DisasContext *ctx, arg_c_not *a)
+{
+REQUIRE_ZCB(ctx);
+return gen_unary(ctx, a, EXT_NONE, tcg_gen_not_tl);
+}
+
+static bool trans_c_mul(DisasContext *ctx, arg_c_mul *a)
+{
+REQUIRE_ZCB(ctx);
+REQUIRE_M_OR_ZMMUL(ctx);
+return gen_arith(ctx, a, EXT_NONE, tcg_gen_mul_tl, NULL);
+}
+
+static bool trans_c_lbu(DisasContext *ctx, arg_c_lbu *a)
+{
+

Re: [PATCH] target/riscv/cpu.c: Fix elen check

[Alistair Francis]

On Fri, Dec 16, 2022 at 1:10 AM Elta <503386...@qq.com> wrote:
>
> Should be cpu->cfg.elen in range [8, 64].
>
> Signed-off-by: Dongxue Zhang 

Thanks for the patch!

I'm seeing weird formatting issues though, it looks like the encoding
is incorrect.

Did you use `git send-email` to send the patch? If not can you try
sending it again following the instructions here
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches

Alistair

> ---
>  target/riscv/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index d14e95c9dc..1e8032c969 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -870,7 +870,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  "Vector extension ELEN must be power of 2");
>  return;
>  }
> -if (cpu->cfg.elen > 64 || cpu->cfg.vlen < 8) {
> +if (cpu->cfg.elen > 64 || cpu->cfg.elen < 8) {
>  error_setg(errp,
>  "Vector extension implementation only supports ELEN "
>  "in the range [8, 64]");
> --
> 2.17.1
>

Re: [PATCH v8 0/9] support subsets of code size reduction extension

[Alistair Francis]

On Wed, Nov 30, 2022 at 5:53 PM Weiwei Li  wrote:
>
> This patchset implements RISC-V Zc* extension v1.0.0.RC5.7 version 
> instructions.
>
> Specification:
> https://github.com/riscv/riscv-code-size-reduction/tree/main/Zc-specification
>
> The port is available here:
> https://github.com/plctlab/plct-qemu/tree/plct-zce-upstream-v8
>
> To test Zc* implementation, specify cpu argument with 
> 'x-zca=true,x-zcb=true,x-zcf=true,f=true" and "x-zcd=true,d=true" (or 
> "x-zcmp=true,x-zcmt=true" with c or d=false) to enable Zca/Zcb/Zcf and Zcd(or 
> Zcmp,Zcmt) extensions support.
>
>
> This implementation can pass the basic zc tests from 
> https://github.com/yulong-plct/zc-test
>
> v8:
> * improve disas support in Patch 9
>
> v7:
> * Fix description for Zca
>
> v6：
> * fix base address for jump table in Patch 7
> * rebase on riscv-to-apply.next
>
> v5:
> * fix exception unwind problem for cpu_ld*_code in helper of cm_jalt
>
> v4:
> * improve Zcmp suggested by Richard
> * fix stateen related check for Zcmt
>
> v3:
> * update the solution for Zcf to the way of Zcd
> * update Zcb to reuse gen_load/store
> * use trans function instead of helper for push/pop
>
> v2:
> * add check for relationship between Zca/Zcf/Zcd with C/F/D based on related 
> discussion in review of Zc* spec
> * separate c.fld{sp}/fsd{sp} with fld{sp}/fsd{sp} before support of zcmp/zcmt
>
> Weiwei Li (9):
>   target/riscv: add cfg properties for Zc* extension
>   target/riscv: add support for Zca extension
>   target/riscv: add support for Zcf extension
>   target/riscv: add support for Zcd extension
>   target/riscv: add support for Zcb extension
>   target/riscv: add support for Zcmp extension
>   target/riscv: add support for Zcmt extension
>   target/riscv: expose properties for Zc* extension
>   disas/riscv.c: add disasm support for Zc*

I think this series has been fully reviewed. Do you mind rebasing it
on https://github.com/alistair23/qemu/tree/riscv-to-apply.next and
then I'll apply it

Alistair

>
>  disas/riscv.c | 228 +++-
>  target/riscv/cpu.c|  56 
>  target/riscv/cpu.h|  10 +
>  target/riscv/cpu_bits.h   |   7 +
>  target/riscv/csr.c|  38 ++-
>  target/riscv/helper.h |   3 +
>  target/riscv/insn16.decode|  63 -
>  target/riscv/insn_trans/trans_rvd.c.inc   |  18 ++
>  target/riscv/insn_trans/trans_rvf.c.inc   |  18 ++
>  target/riscv/insn_trans/trans_rvi.c.inc   |   4 +-
>  target/riscv/insn_trans/trans_rvzce.c.inc | 313 ++
>  target/riscv/machine.c|  19 ++
>  target/riscv/meson.build  |   3 +-
>  target/riscv/translate.c  |  15 +-
>  target/riscv/zce_helper.c |  55 
>  15 files changed, 834 insertions(+), 16 deletions(-)
>  create mode 100644 target/riscv/insn_trans/trans_rvzce.c.inc
>  create mode 100644 target/riscv/zce_helper.c
>
> --
> 2.25.1
>
>

Re: [PATCH v2 2/5] target/riscv: Update VS timer whenever htimedelta changes

[Alistair Francis]

On Fri, Dec 23, 2022 at 11:14 PM Anup Patel  wrote:
>
> On Thu, Dec 15, 2022 at 8:55 AM Alistair Francis  wrote:
> >
> > On Mon, Dec 12, 2022 at 9:12 PM Anup Patel  wrote:
> > >
> > > On Mon, Dec 12, 2022 at 11:23 AM Alistair Francis  
> > > wrote:
> > > >
> > > > On Thu, Dec 8, 2022 at 6:41 PM Anup Patel  
> > > > wrote:
> > > > >
> > > > > On Thu, Dec 8, 2022 at 9:00 AM Alistair Francis 
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, Nov 8, 2022 at 11:07 PM Anup Patel 
> > > > > >  wrote:
> > > > > > >
> > > > > > > The htimedelta[h] CSR has impact on the VS timer comparison so we
> > > > > > > should call riscv_timer_write_timecmp() whenever htimedelta 
> > > > > > > changes.
> > > > > > >
> > > > > > > Fixes: 3ec0fe18a31f ("target/riscv: Add vstimecmp suppor")
> > > > > > > Signed-off-by: Anup Patel 
> > > > > > > Reviewed-by: Alistair Francis 
> > > > > >
> > > > > > This patch breaks my Xvisor test. When running OpenSBI and Xvisor 
> > > > > > like this:
> > > > > >
> > > > > > qemu-system-riscv64 -machine virt \
> > > > > > -m 1G -serial mon:stdio -serial null -nographic \
> > > > > > -append 'vmm.console=uart@1000 vmm.bootcmd="vfs mount initrd
> > > > > > /;vfs run /boot.xscript;vfs cat /system/banner.txt; guest kick 
> > > > > > guest0;
> > > > > > vserial bind guest0/uart0"' \
> > > > > > -smp 4 -d guest_errors \
> > > > > > -bios none \
> > > > > > -device 
> > > > > > loader,file=./images/qemuriscv64/vmm.bin,addr=0x8020 \
> > > > > > -kernel ./images/qemuriscv64/fw_jump.elf \
> > > > > > -initrd ./images/qemuriscv64/vmm-disk-linux.img -cpu rv64,h=true
> > > > > >
> > > > > > Running:
> > > > > >
> > > > > > Xvisor v0.3.0-129-gbc33f339 (Jan  1 1970 00:00:00)
> > > > > >
> > > > > > I see this failure:
> > > > > >
> > > > > > INIT: bootcmd:  guest kick guest0
> > > > > >
> > > > > > guest0: Kicked
> > > > > >
> > > > > > INIT: bootcmd:  vserial bind guest0/uart0
> > > > > >
> > > > > > [guest0/uart0] cpu_vcpu_stage2_map: guest_phys=0x3B9AC000
> > > > > > size=0x4096 map failed
> > > > > >
> > > > > > do_error: CPU3: VCPU=guest0/vcpu0 page fault failed (error -1)
> > > > > >
> > > > > >zero=0x  ra=0x80001B4E
> > > > > >
> > > > > >  sp=0x8001CF80  gp=0x
> > > > > >
> > > > > >  tp=0x  s0=0x8001CFB0
> > > > > >
> > > > > >  s1=0x  a0=0x10001048
> > > > > >
> > > > > >  a1=0x  a2=0x00989680
> > > > > >
> > > > > >  a3=0x3B9ACA00  a4=0x0048
> > > > > >
> > > > > >  a5=0x  a6=0x00019000
> > > > > >
> > > > > >  a7=0x  s2=0x
> > > > > >
> > > > > >  s3=0x  s4=0x
> > > > > >
> > > > > >  s5=0x  s6=0x
> > > > > >
> > > > > >  s7=0x  s8=0x
> > > > > >
> > > > > >  s9=0x s10=0x
> > > > > >
> > > > > > s11=0x  t0=0x4000
> > > > > >
> > > > > >  t1=0x0100  t2=0x
> > > > > >
> > > > > >  t3=0x  t4=0x
> > > > > >
> > > > > >  t5=0x  t6=0x
> > > > > >
> > > > > >sepc=0x80001918 sstatus=0x00024120
> > > > > >
> > > > > > hstatus=0x0002002001C0 sp_exec=0x10A64000
> > > > > >
> > > > > >  scause=0x0017   stval=0x3B9ACAF8
> > > > > >
> > > > > >   htval=0x0EE6B2BE  htinst=0x00D03021
> > > > > >
> > > > > > I have tried updating to a newer Xvisor release, but with that I 
> > > > > > don't
> > > > > > get any serial output.
> > > > > >
> > > > > > Can you help get the Xvisor tests back up and running?
> > > > >
> > > > > I tried the latest Xvisor-next 
> > > > > (https://github.com/avpatel/xvisor-next)
> > > > > with your QEMU riscv-to-apply.next branch and it works fine (both
> > > > > with and without Sstc).
> > > >
> > > > Does it work with the latest release?
> > >
> > > Yes, the latest Xvisor-next repo works for QEMU v7.2.0-rc4 and
> > > your riscv-to-apply.next branch (commit 51bb9de2d188)
> >
> > I can't get anything to work with this patch. I have dropped this and
> > the patches after this.
> >
> > I'm building the latest Xvisor release with:
> >
> > export CROSS_COMPILE=riscv64-linux-gnu-
> > ARCH=riscv make generic-64b-defconfig
> > make
> >
> > and running it as above, yet nothing. What am I missing here?
>
> I tried multiple times with the latest Xvisor on different machines but
> still can't reproduce the issue you are seeing.

Odd

>
> We generally provide pre-built 

Re: [PATCH 11/12] hw/riscv/boot.c: Introduce riscv_find_firmware()

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:55 PM Bin Meng  wrote:
>
> Rename previous riscv_find_firmware() to riscv_find_bios(), and
> introduce a new riscv_find_firmware() to implement the first half
> part of the work done in riscv_find_and_load_firmware().
>
> This new API is helpful for machine that wants to know the final
> chosen firmware file name but does not want to load it.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  include/hw/riscv/boot.h |  2 ++
>  hw/riscv/boot.c | 39 +--
>  2 files changed, 27 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
> index 60cf320c88..b273ab22f7 100644
> --- a/include/hw/riscv/boot.h
> +++ b/include/hw/riscv/boot.h
> @@ -38,6 +38,8 @@ target_ulong riscv_find_and_load_firmware(MachineState 
> *machine,
>hwaddr firmware_load_addr,
>symbol_fn_t sym_cb);
>  const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
> +char *riscv_find_firmware(const char *firmware_filename,
> +  const char *default_machine_firmware);
>  target_ulong riscv_load_firmware(const char *firmware_filename,
>   hwaddr firmware_load_addr,
>   symbol_fn_t sym_cb);
> diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
> index e1a544b1d9..98b80af51b 100644
> --- a/hw/riscv/boot.c
> +++ b/hw/riscv/boot.c
> @@ -84,11 +84,11 @@ const char 
> *riscv_default_firmware_name(RISCVHartArrayState *harts)
>  return RISCV64_BIOS_BIN;
>  }
>
> -static char *riscv_find_firmware(const char *firmware_filename)
> +static char *riscv_find_bios(const char *bios_filename)
>  {
>  char *filename;
>
> -filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, firmware_filename);
> +filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_filename);
>  if (filename == NULL) {
>  if (!qtest_enabled()) {
>  /*
> @@ -97,8 +97,8 @@ static char *riscv_find_firmware(const char 
> *firmware_filename)
>   * running QEMU test will complain hence let's suppress the error
>   * report for QEMU testing.
>   */
> -error_report("Unable to load the RISC-V firmware \"%s\"",
> - firmware_filename);
> +error_report("Unable to find the RISC-V BIOS \"%s\"",
> + bios_filename);
>  exit(1);
>  }
>  }
> @@ -106,25 +106,36 @@ static char *riscv_find_firmware(const char 
> *firmware_filename)
>  return filename;
>  }
>
> -target_ulong riscv_find_and_load_firmware(MachineState *machine,
> -  const char 
> *default_machine_firmware,
> -  hwaddr firmware_load_addr,
> -  symbol_fn_t sym_cb)
> +char *riscv_find_firmware(const char *firmware_filename,
> +  const char *default_machine_firmware)
>  {
> -char *firmware_filename = NULL;
> -target_ulong firmware_end_addr = firmware_load_addr;
> +char *filename = NULL;
>
> -if ((!machine->firmware) || (!strcmp(machine->firmware, "default"))) {
> +if ((!firmware_filename) || (!strcmp(firmware_filename, "default"))) {
>  /*
>   * The user didn't specify -bios, or has specified "-bios default".
>   * That means we are going to load the OpenSBI binary included in
>   * the QEMU source.
>   */
> -firmware_filename = riscv_find_firmware(default_machine_firmware);
> -} else if (strcmp(machine->firmware, "none")) {
> -firmware_filename = riscv_find_firmware(machine->firmware);
> +filename = riscv_find_bios(default_machine_firmware);
> +} else if (strcmp(firmware_filename, "none")) {
> +filename = riscv_find_bios(firmware_filename);
>  }
>
> +return filename;
> +}
> +
> +target_ulong riscv_find_and_load_firmware(MachineState *machine,
> +  const char 
> *default_machine_firmware,
> +  hwaddr firmware_load_addr,
> +  symbol_fn_t sym_cb)
> +{
> +char *firmware_filename;
> +target_ulong firmware_end_addr = firmware_load_addr;
> +
> +firmware_filename = riscv_find_firmware(machine->firmware,
> +default_machine_firmware);
> +
>  if (firmware_filename) {
>  /* If not "none" load the firmware */
>  firmware_end_addr = riscv_load_firmware(firmware_filename,
> --
> 2.34.1
>
>

Re: [PATCH 08/12] hw/riscv: spike: Remove the out-of-date comments

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:54 PM Bin Meng  wrote:
>
> Spike machine now supports OpenSBI plain binary bios image, so the
> comments are no longer valid.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  hw/riscv/spike.c | 5 -
>  1 file changed, 5 deletions(-)
>
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 8606331f61..ab0a945f8b 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -256,11 +256,6 @@ static void spike_board_init(MachineState *machine)
>  memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
>  mask_rom);
>
> -/*
> - * Not like other RISC-V machines that use plain binary bios images,
> - * keeping ELF files here was intentional because BIN files don't work
> - * for the Spike machine as HTIF emulation depends on ELF parsing.
> - */
>  if (riscv_is_32bit(>soc[0])) {
>  firmware_end_addr = riscv_find_and_load_firmware(machine,
>  RISCV32_BIOS_BIN, 
> memmap[SPIKE_DRAM].base,
> --
> 2.34.1
>
>

Re: [PATCH 07/12] hw/char: riscv_htif: Support console output via proxy syscall

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:49 PM Bin Meng  wrote:
>
> At present the HTIF proxy syscall is unsupported. On RV32, only
> device 0 is supported so there is no console device for RV32.
> The only way to implement console funtionality on RV32 is to
> support the SYS_WRITE syscall.
>
> With this commit, the Spike machine is able to boot the 32-bit
> OpenSBI generic image.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  hw/char/riscv_htif.c | 17 -
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index 3bb0a37a3e..1477fc0090 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -48,6 +48,9 @@
>  #define HTIF_CONSOLE_CMD_GETC   0
>  #define HTIF_CONSOLE_CMD_PUTC   1
>
> +/* PK system call number */
> +#define PK_SYS_WRITE64
> +
>  static uint64_t fromhost_addr, tohost_addr;
>  static int address_symbol_set;
>
> @@ -165,7 +168,19 @@ static void htif_handle_tohost_write(HTIFState *s, 
> uint64_t val_written)
>  int exit_code = payload >> 1;
>  exit(exit_code);
>  } else {
> -qemu_log_mask(LOG_UNIMP, "pk syscall proxy not supported\n");
> +uint64_t syscall[8];
> +cpu_physical_memory_read(payload, syscall, sizeof(syscall));
> +if (syscall[0] == PK_SYS_WRITE &&
> +syscall[1] == HTIF_DEV_CONSOLE &&
> +syscall[3] == HTIF_CONSOLE_CMD_PUTC) {
> +uint8_t ch;
> +cpu_physical_memory_read(syscall[2], , 1);
> +qemu_chr_fe_write(>chr, , 1);
> +resp = 0x100 | (uint8_t)payload;
> +} else {
> +qemu_log_mask(LOG_UNIMP,
> +  "pk syscall proxy not supported\n");
> +}
>  }
>  } else {
>  qemu_log("HTIF device %d: unknown command\n", device);
> --
> 2.34.1
>
>

Re: [PATCH 06/12] hw/char: riscv_htif: Remove forward declarations for non-existent variables

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:56 PM Bin Meng  wrote:
>
> There are forward declarations for 'vmstate_htif' and 'htif_io_ops'
> in riscv_htif.h however there are no definitions in the C codes.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  include/hw/char/riscv_htif.h | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
> index 55cc352331..9e8ebbe017 100644
> --- a/include/hw/char/riscv_htif.h
> +++ b/include/hw/char/riscv_htif.h
> @@ -40,9 +40,6 @@ typedef struct HTIFState {
>  uint64_t pending_read;
>  } HTIFState;
>
> -extern const VMStateDescription vmstate_htif;
> -extern const MemoryRegionOps htif_io_ops;
> -
>  /* HTIF symbol callback */
>  void htif_symbol_callback(const char *st_name, int st_info, uint64_t 
> st_value,
>  uint64_t st_size);
> --
> 2.34.1
>
>

Re: [PATCH 00/12] hw/riscv: Improve Spike HTIF emulation fidelity

[Alistair Francis]

On Wed, Dec 28, 2022 at 1:59 PM Bin Meng  wrote:
>
> Hi Daniel,
>
> On Wed, Dec 28, 2022 at 1:52 AM Daniel Henrique Barboza
>  wrote:
> >
> >
> >
> > On 12/27/22 03:48, Bin Meng wrote:
> > > At present the 32-bit OpenSBI generic firmware image does not boot on
> > > Spike, only 64-bit image can. This is due to the HTIF emulation does
> > > not implement the proxy syscall interface which is required for the
> > > 32-bit HTIF console output.
> > >
> > > An OpenSBI bug fix [1] is also needed when booting the plain binary image.
> > >
> > > With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF
> > > images can boot on QEMU 'spike' machine.
> > >
> > > [1] 
> > > https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bm...@tinylab.org/
> >
> > Aside from a nit in patch 12/12, LGTM. I've tested with a patched version of
> > Opensbi including [1] and I can get terminal output with riscv32 spike:
> >
> > $ ./qemu-system-riscv32 -M spike -display none -nographic
> > -bios ../../opensbi/build/platform/generic/firmware/fw_payload.bin
> >
> > OpenSBI v1.1-112-g6ce00f8
> > _  _
> >/ __ \  / |  _ \_   _|
> >   | |  | |_ __   ___ _ __ | (___ | |_) || |
> >   | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
> >   | |__| | |_) |  __/ | | |) | |_) || |_
> >\/| .__/ \___|_| |_|_/|/_|
> >  | |
> >  |_|
> > (...)
> >
> >
> > Speaking of [1], it seems like the fix went a bit too late for the opensbi 
> > 1.2 release.
> > Assuming that [1] is accepted, it would be nice if we could bake in this 
> > fix on top of the
> > 1.2 release when updating the QEMU roms.
> >
>
> Thanks for the review and testing!
>
> Regarding whether we can cherry-pick the fix on top of OpenSBI 1.2, I
> am not sure if that's allowed by the policy.

It doesn't seem like a good idea. Maybe it's worth pinging Anup to see
if we can get a 1.2.1 with the fix?

Alistair

>
> Alistair, do you know?
>
> Regards,
> Bin
>

Re: [PATCH 05/12] hw/char: riscv_htif: Move registers from CPUArchState to HTIFState

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:55 PM Bin Meng  wrote:
>
> At present for some unknown reason the HTIF registers (fromhost &
> tohost) are defined in the RISC-V CPUArchState. It should really
> be put in the HTIFState struct as it is only meaningful to HTIF.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  include/hw/char/riscv_htif.h |  8 
>  target/riscv/cpu.h   |  4 
>  hw/char/riscv_htif.c | 35 +--
>  hw/riscv/spike.c |  3 +--
>  target/riscv/machine.c   |  6 ++
>  5 files changed, 24 insertions(+), 32 deletions(-)
>
> diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
> index 6d172ebd6d..55cc352331 100644
> --- a/include/hw/char/riscv_htif.h
> +++ b/include/hw/char/riscv_htif.h
> @@ -23,7 +23,6 @@
>  #include "chardev/char.h"
>  #include "chardev/char-fe.h"
>  #include "exec/memory.h"
> -#include "target/riscv/cpu.h"
>
>  #define TYPE_HTIF_UART "riscv.htif.uart"
>
> @@ -31,11 +30,12 @@ typedef struct HTIFState {
>  int allow_tohost;
>  int fromhost_inprogress;
>
> +uint64_t tohost;
> +uint64_t fromhost;
>  hwaddr tohost_offset;
>  hwaddr fromhost_offset;
>  MemoryRegion mmio;
>
> -CPURISCVState *env;
>  CharBackend chr;
>  uint64_t pending_read;
>  } HTIFState;
> @@ -51,7 +51,7 @@ void htif_symbol_callback(const char *st_name, int st_info, 
> uint64_t st_value,
>  bool htif_uses_elf_symbols(void);
>
>  /* legacy pre qom */
> -HTIFState *htif_mm_init(MemoryRegion *address_space, CPURISCVState *env,
> -Chardev *chr, uint64_t nonelf_base);
> +HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
> +uint64_t nonelf_base);
>
>  #endif
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 443d15a47c..6f04d853dd 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -309,10 +309,6 @@ struct CPUArchState {
>  target_ulong sscratch;
>  target_ulong mscratch;
>
> -/* temporary htif regs */
> -uint64_t mfromhost;
> -uint64_t mtohost;
> -
>  /* Sstc CSRs */
>  uint64_t stimecmp;
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index f28976b110..3bb0a37a3e 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -100,7 +100,7 @@ static void htif_recv(void *opaque, const uint8_t *buf, 
> int size)
>  uint64_t val_written = s->pending_read;
>  uint64_t resp = 0x100 | *buf;
>
> -s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
> +s->fromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
>  }
>
>  /*
> @@ -175,7 +175,7 @@ static void htif_handle_tohost_write(HTIFState *s, 
> uint64_t val_written)
>  if (cmd == HTIF_CONSOLE_CMD_GETC) {
>  /* this should be a queue, but not yet implemented as such */
>  s->pending_read = val_written;
> -s->env->mtohost = 0; /* clear to indicate we read */
> +s->tohost = 0; /* clear to indicate we read */
>  return;
>  } else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
>  qemu_chr_fe_write(>chr, (uint8_t *), 1);
> @@ -195,11 +195,11 @@ static void htif_handle_tohost_write(HTIFState *s, 
> uint64_t val_written)
>   * HTIF needs protocol documentation and a more complete state machine.
>   *
>   *  while (!s->fromhost_inprogress &&
> - *  s->env->mfromhost != 0x0) {
> + *  s->fromhost != 0x0) {
>   *  }
>   */
> -s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
> -s->env->mtohost = 0; /* clear to indicate we read */
> +s->fromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
> +s->tohost = 0; /* clear to indicate we read */
>  }
>
>  #define TOHOST_OFFSET1  (s->tohost_offset)
> @@ -212,13 +212,13 @@ static uint64_t htif_mm_read(void *opaque, hwaddr addr, 
> unsigned size)
>  {
>  HTIFState *s = opaque;
>  if (addr == TOHOST_OFFSET1) {
> -return s->env->mtohost & 0x;
> +return s->tohost & 0x;
>  } else if (addr == TOHOST_OFFSET2) {
> -return (s->env->mtohost >> 32) & 0x;
> +return (s->tohost >> 32) & 0x;
>  } else if (addr == FROMHOST_OFFSET1) {
> -return s->env->mfromhost & 0x;
> +return s->fromhost & 0x;
>  } else if (addr == FROMHOST_OFFSET2) {
> -return (s->env->mfromhost >> 32) & 0x;
> +return (s->fromhost >> 32) & 0x;
>  } else {
>  qemu_log("Invalid htif read: address %016" PRIx64 "\n",
>  (uint64_t)addr);
> @@ -232,22 +232,22 @@ static void htif_mm_write(void *opaque, hwaddr addr,
>  {
>  HTIFState *s = opaque;
>  if (addr == TOHOST_OFFSET1) {
> -if (s->env->mtohost == 0x0) {
> +if (s->tohost == 0x0) {
>  s->allow_tohost = 1;
> -s->env->mtohost = value & 0x;
> +  

Re: [PATCH 00/12] hw/riscv: Improve Spike HTIF emulation fidelity

[Bin Meng]

Hi Daniel,

On Wed, Dec 28, 2022 at 1:52 AM Daniel Henrique Barboza
 wrote:
>
>
>
> On 12/27/22 03:48, Bin Meng wrote:
> > At present the 32-bit OpenSBI generic firmware image does not boot on
> > Spike, only 64-bit image can. This is due to the HTIF emulation does
> > not implement the proxy syscall interface which is required for the
> > 32-bit HTIF console output.
> >
> > An OpenSBI bug fix [1] is also needed when booting the plain binary image.
> >
> > With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF
> > images can boot on QEMU 'spike' machine.
> >
> > [1] 
> > https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bm...@tinylab.org/
>
> Aside from a nit in patch 12/12, LGTM. I've tested with a patched version of
> Opensbi including [1] and I can get terminal output with riscv32 spike:
>
> $ ./qemu-system-riscv32 -M spike -display none -nographic
> -bios ../../opensbi/build/platform/generic/firmware/fw_payload.bin
>
> OpenSBI v1.1-112-g6ce00f8
> _  _
>/ __ \  / |  _ \_   _|
>   | |  | |_ __   ___ _ __ | (___ | |_) || |
>   | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
>   | |__| | |_) |  __/ | | |) | |_) || |_
>\/| .__/ \___|_| |_|_/|/_|
>  | |
>  |_|
> (...)
>
>
> Speaking of [1], it seems like the fix went a bit too late for the opensbi 
> 1.2 release.
> Assuming that [1] is accepted, it would be nice if we could bake in this fix 
> on top of the
> 1.2 release when updating the QEMU roms.
>

Thanks for the review and testing!

Regarding whether we can cherry-pick the fix on top of OpenSBI 1.2, I
am not sure if that's allowed by the policy.

Alistair, do you know?

Regards,
Bin

Re: [PATCH 04/12] hw/char: riscv_htif: Use conventional 's' for HTIFState

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:52 PM Bin Meng  wrote:
>
> QEMU source codes tend to use 's' to represent the hardware state.
> Let's use it for HTIFState.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  hw/char/riscv_htif.c | 64 ++--
>  1 file changed, 32 insertions(+), 32 deletions(-)
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index e7e319ca1d..f28976b110 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -85,7 +85,7 @@ static int htif_can_recv(void *opaque)
>   */
>  static void htif_recv(void *opaque, const uint8_t *buf, int size)
>  {
> -HTIFState *htifstate = opaque;
> +HTIFState *s = opaque;
>
>  if (size != 1) {
>  return;
> @@ -97,10 +97,10 @@ static void htif_recv(void *opaque, const uint8_t *buf, 
> int size)
>   *will drop characters
>   */
>
> -uint64_t val_written = htifstate->pending_read;
> +uint64_t val_written = s->pending_read;
>  uint64_t resp = 0x100 | *buf;
>
> -htifstate->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 
> 16);
> +s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
>  }
>
>  /*
> @@ -142,7 +142,7 @@ static int htif_be_change(void *opaque)
>   * For RV32, the tohost register is zero-extended, so only device=0 and
>   * command=0 (i.e. HTIF syscalls/exit codes) are supported.
>   */
> -static void htif_handle_tohost_write(HTIFState *htifstate, uint64_t 
> val_written)
> +static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written)
>  {
>  uint8_t device = val_written >> HTIF_DEV_SHIFT;
>  uint8_t cmd = val_written >> HTIF_CMD_SHIFT;
> @@ -174,11 +174,11 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>  /* HTIF Console */
>  if (cmd == HTIF_CONSOLE_CMD_GETC) {
>  /* this should be a queue, but not yet implemented as such */
> -htifstate->pending_read = val_written;
> -htifstate->env->mtohost = 0; /* clear to indicate we read */
> +s->pending_read = val_written;
> +s->env->mtohost = 0; /* clear to indicate we read */
>  return;
>  } else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
> -qemu_chr_fe_write(>chr, (uint8_t *), 1);
> +qemu_chr_fe_write(>chr, (uint8_t *), 1);
>  resp = 0x100 | (uint8_t)payload;
>  } else {
>  qemu_log("HTIF device %d: unknown command\n", device);
> @@ -194,31 +194,31 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>   * With this code disabled, qemu works with bbl priv v1.9.1 and v1.10.
>   * HTIF needs protocol documentation and a more complete state machine.
>   *
> - *  while (!htifstate->fromhost_inprogress &&
> - *  htifstate->env->mfromhost != 0x0) {
> + *  while (!s->fromhost_inprogress &&
> + *  s->env->mfromhost != 0x0) {
>   *  }
>   */
> -htifstate->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 
> 16);
> -htifstate->env->mtohost = 0; /* clear to indicate we read */
> +s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
> +s->env->mtohost = 0; /* clear to indicate we read */
>  }
>
> -#define TOHOST_OFFSET1 (htifstate->tohost_offset)
> -#define TOHOST_OFFSET2 (htifstate->tohost_offset + 4)
> -#define FROMHOST_OFFSET1 (htifstate->fromhost_offset)
> -#define FROMHOST_OFFSET2 (htifstate->fromhost_offset + 4)
> +#define TOHOST_OFFSET1  (s->tohost_offset)
> +#define TOHOST_OFFSET2  (s->tohost_offset + 4)
> +#define FROMHOST_OFFSET1(s->fromhost_offset)
> +#define FROMHOST_OFFSET2(s->fromhost_offset + 4)
>
>  /* CPU wants to read an HTIF register */
>  static uint64_t htif_mm_read(void *opaque, hwaddr addr, unsigned size)
>  {
> -HTIFState *htifstate = opaque;
> +HTIFState *s = opaque;
>  if (addr == TOHOST_OFFSET1) {
> -return htifstate->env->mtohost & 0x;
> +return s->env->mtohost & 0x;
>  } else if (addr == TOHOST_OFFSET2) {
> -return (htifstate->env->mtohost >> 32) & 0x;
> +return (s->env->mtohost >> 32) & 0x;
>  } else if (addr == FROMHOST_OFFSET1) {
> -return htifstate->env->mfromhost & 0x;
> +return s->env->mfromhost & 0x;
>  } else if (addr == FROMHOST_OFFSET2) {
> -return (htifstate->env->mfromhost >> 32) & 0x;
> +return (s->env->mfromhost >> 32) & 0x;
>  } else {
>  qemu_log("Invalid htif read: address %016" PRIx64 "\n",
>  (uint64_t)addr);
> @@ -230,25 +230,25 @@ static uint64_t htif_mm_read(void *opaque, hwaddr addr, 
> unsigned size)
>  static void htif_mm_write(void *opaque, hwaddr addr,
>uint64_t value, unsigned size)
>  {
> -HTIFState *htifstate = opaque;
> +HTIFState *s = opaque;
>  

Re: [PATCH 03/12] hw/char: riscv_htif: Drop useless assignment of memory region

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:50 PM Bin Meng  wrote:
>
> struct HTIFState has 3 members for address space and memory region,
> and are initialized during htif_mm_init(). But they are actually
> useless. Drop them.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  include/hw/char/riscv_htif.h | 7 ++-
>  hw/char/riscv_htif.c | 7 ++-
>  hw/riscv/spike.c | 5 ++---
>  3 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
> index 3eccc1914f..6d172ebd6d 100644
> --- a/include/hw/char/riscv_htif.h
> +++ b/include/hw/char/riscv_htif.h
> @@ -34,9 +34,6 @@ typedef struct HTIFState {
>  hwaddr tohost_offset;
>  hwaddr fromhost_offset;
>  MemoryRegion mmio;
> -MemoryRegion *address_space;
> -MemoryRegion *main_mem;
> -void *main_mem_ram_ptr;
>
>  CPURISCVState *env;
>  CharBackend chr;
> @@ -54,7 +51,7 @@ void htif_symbol_callback(const char *st_name, int st_info, 
> uint64_t st_value,
>  bool htif_uses_elf_symbols(void);
>
>  /* legacy pre qom */
> -HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
> -CPURISCVState *env, Chardev *chr, uint64_t nonelf_base);
> +HTIFState *htif_mm_init(MemoryRegion *address_space, CPURISCVState *env,
> +Chardev *chr, uint64_t nonelf_base);
>
>  #endif
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index 088556bb04..e7e319ca1d 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -265,8 +265,8 @@ bool htif_uses_elf_symbols(void)
>  return (address_symbol_set == 3) ? true : false;
>  }
>
> -HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
> -CPURISCVState *env, Chardev *chr, uint64_t nonelf_base)
> +HTIFState *htif_mm_init(MemoryRegion *address_space, CPURISCVState *env,
> +Chardev *chr, uint64_t nonelf_base)
>  {
>  uint64_t base, size, tohost_offset, fromhost_offset;
>
> @@ -281,9 +281,6 @@ HTIFState *htif_mm_init(MemoryRegion *address_space, 
> MemoryRegion *main_mem,
>  fromhost_offset = fromhost_addr - base;
>
>  HTIFState *s = g_new0(HTIFState, 1);
> -s->address_space = address_space;
> -s->main_mem = main_mem;
> -s->main_mem_ram_ptr = memory_region_get_ram_ptr(main_mem);
>  s->env = env;
>  s->tohost_offset = tohost_offset;
>  s->fromhost_offset = fromhost_offset;
> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
> index 1e1d752c00..82cf41ac27 100644
> --- a/hw/riscv/spike.c
> +++ b/hw/riscv/spike.c
> @@ -317,9 +317,8 @@ static void spike_board_init(MachineState *machine)
>fdt_load_addr);
>
>  /* initialize HTIF using symbols found in load_kernel */
> -htif_mm_init(system_memory, mask_rom,
> - >soc[0].harts[0].env, serial_hd(0),
> - memmap[SPIKE_HTIF].base);
> +htif_mm_init(system_memory, >soc[0].harts[0].env,
> + serial_hd(0), memmap[SPIKE_HTIF].base);
>  }
>
>  static void spike_machine_instance_init(Object *obj)
> --
> 2.34.1
>
>

Re: [PATCH 01/12] hw/char: riscv_htif: Avoid using magic numbers

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:50 PM Bin Meng  wrote:
>
> The Spike HTIF is poorly documented. The only relevant info we can
> get from the internet is from Andrew Waterman at [1].
>
> Add a comment block before htif_handle_tohost_write() to explain
> the tohost register format, and use meaningful macros intead of
> magic numbers in the codes.
>
> While we are here, corret 2 multi-line comment blocks that have
> wrong format.
>
> Link: https://github.com/riscv-software-src/riscv-isa-sim/issues/364 [1]
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  hw/char/riscv_htif.c | 72 
>  1 file changed, 52 insertions(+), 20 deletions(-)
>
> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> index 6577f0e640..088556bb04 100644
> --- a/hw/char/riscv_htif.c
> +++ b/hw/char/riscv_htif.c
> @@ -38,6 +38,16 @@
>  }
>   \
>  } while (0)
>
> +#define HTIF_DEV_SHIFT  56
> +#define HTIF_CMD_SHIFT  48
> +
> +#define HTIF_DEV_SYSTEM 0
> +#define HTIF_DEV_CONSOLE1
> +
> +#define HTIF_SYSTEM_CMD_SYSCALL 0
> +#define HTIF_CONSOLE_CMD_GETC   0
> +#define HTIF_CONSOLE_CMD_PUTC   1
> +
>  static uint64_t fromhost_addr, tohost_addr;
>  static int address_symbol_set;
>
> @@ -81,9 +91,11 @@ static void htif_recv(void *opaque, const uint8_t *buf, 
> int size)
>  return;
>  }
>
> -/* TODO - we need to check whether mfromhost is zero which indicates
> -  the device is ready to receive. The current implementation
> -  will drop characters */
> +/*
> + * TODO - we need to check whether mfromhost is zero which indicates
> + *the device is ready to receive. The current implementation
> + *will drop characters
> + */
>
>  uint64_t val_written = htifstate->pending_read;
>  uint64_t resp = 0x100 | *buf;
> @@ -110,10 +122,30 @@ static int htif_be_change(void *opaque)
>  return 0;
>  }
>
> +/*
> + * See below the tohost register format.
> + *
> + * Bits 63:56 indicate the "device".
> + * Bits 55:48 indicate the "command".
> + *
> + * Device 0 is the syscall device, which is used to emulate Unixy syscalls.
> + * It only implements command 0, which has two subfunctions:
> + * - If bit 0 is clear, then bits 47:0 represent a pointer to a struct
> + *   describing the syscall.
> + * - If bit 1 is set, then bits 47:1 represent an exit code, with a zero
> + *   value indicating success and other values indicating failure.
> + *
> + * Device 1 is the blocking character device.
> + * - Command 0 reads a character
> + * - Command 1 writes a character from the 8 LSBs of tohost
> + *
> + * For RV32, the tohost register is zero-extended, so only device=0 and
> + * command=0 (i.e. HTIF syscalls/exit codes) are supported.
> + */
>  static void htif_handle_tohost_write(HTIFState *htifstate, uint64_t 
> val_written)
>  {
> -uint8_t device = val_written >> 56;
> -uint8_t cmd = val_written >> 48;
> +uint8_t device = val_written >> HTIF_DEV_SHIFT;
> +uint8_t cmd = val_written >> HTIF_CMD_SHIFT;
>  uint64_t payload = val_written & 0xULL;
>  int resp = 0;
>
> @@ -125,9 +157,9 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>   * 0: riscv-tests Pass/Fail Reporting Only (no syscall proxy)
>   * 1: Console
>   */
> -if (unlikely(device == 0x0)) {
> +if (unlikely(device == HTIF_DEV_SYSTEM)) {
>  /* frontend syscall handler, shutdown and exit code support */
> -if (cmd == 0x0) {
> +if (cmd == HTIF_SYSTEM_CMD_SYSCALL) {
>  if (payload & 0x1) {
>  /* exit code */
>  int exit_code = payload >> 1;
> @@ -138,14 +170,14 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>  } else {
>  qemu_log("HTIF device %d: unknown command\n", device);
>  }
> -} else if (likely(device == 0x1)) {
> +} else if (likely(device == HTIF_DEV_CONSOLE)) {
>  /* HTIF Console */
> -if (cmd == 0x0) {
> +if (cmd == HTIF_CONSOLE_CMD_GETC) {
>  /* this should be a queue, but not yet implemented as such */
>  htifstate->pending_read = val_written;
>  htifstate->env->mtohost = 0; /* clear to indicate we read */
>  return;
> -} else if (cmd == 0x1) {
> +} else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
>  qemu_chr_fe_write(>chr, (uint8_t *), 1);
>  resp = 0x100 | (uint8_t)payload;
>  } else {
> @@ -157,15 +189,15 @@ static void htif_handle_tohost_write(HTIFState 
> *htifstate, uint64_t val_written)
>  " payload: %016" PRIx64, device, cmd, payload & 0xFF, payload);
>  }
>  /*
> - * - latest bbl does not set fromhost to 0 if there is a value in tohost
> - * - with this 

Re: [PATCH 02/12] hw/char: riscv_htif: Drop {to, from}host_size in HTIFState

[Alistair Francis]

On Tue, Dec 27, 2022 at 4:56 PM Bin Meng  wrote:
>
> These are not used anywhere. Drop them.
>
> Signed-off-by: Bin Meng 

Reviewed-by: Alistair Francis 

Alistair

> ---
>
>  include/hw/char/riscv_htif.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
> index f888ac1b30..3eccc1914f 100644
> --- a/include/hw/char/riscv_htif.h
> +++ b/include/hw/char/riscv_htif.h
> @@ -33,8 +33,6 @@ typedef struct HTIFState {
>
>  hwaddr tohost_offset;
>  hwaddr fromhost_offset;
> -uint64_t tohost_size;
> -uint64_t fromhost_size;
>  MemoryRegion mmio;
>  MemoryRegion *address_space;
>  MemoryRegion *main_mem;
> --
> 2.34.1
>
>

RE: [PATCH] target/i386/hax: Add XCR0 support

[Wang, Wenchao]

Hi, Paolo,

Sorry for mistaken expression.

Without this patch, HAXM v7.8.0 will break support for QEMU and the HAXM 
versions older than 7.8.0 still can support QEMU with this patch. It will work 
on any version besides HAXM v7.8.0.


Best Regards,
Wenchao

From: Wang, Wenchao
Sent: Wednesday, December 28, 2022 10:55
To: Paolo Bonzini 
Cc: Philippe Mathieu-Daudé ; qemu-devel 
; haxm-team 
Subject: RE: [PATCH] target/i386/hax: Add XCR0 support

Hi, Paolo,

Thanks for your reply.

The reason why the variable xcr0 must be added to the header file of QEMU is 
because HAXM needs QEMU to allocate memory from user space and pass it to the 
kernel. This patch is only used to expand the buffer size of the structure, and 
HAXM will use and maintain this variable.
Without this patch, HAXM v7.8.0 will break support for QEMU and the HAXM 
versions older than 7.8.0 cannot support QEMU with this patch, either. It will 
work on any version since HAXM v7.8.0. I know QEMU treats the structure as a 
black box, but HAXM never supported xcr0 before and the structure size is not 
enough if it has been supported. We have verified the patched QEMU and it can 
launch all guest OSes. Thanks.


Best Regards,
Wenchao

From: Paolo Bonzini mailto:pbonz...@redhat.com>>
Sent: Tuesday, December 27, 2022 23:13
To: Wang, Wenchao mailto:wenchao.w...@intel.com>>
Cc: Philippe Mathieu-Daudé mailto:phi...@linaro.org>>; 
qemu-devel mailto:qemu-devel@nongnu.org>>; haxm-team 
mailto:haxm-t...@intel.com>>
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support


Il lun 28 nov 2022, 09:12 Wang, Wenchao 
mailto:wenchao.w...@intel.com>> ha scritto:
Hi, Philippe,

It is just the full patch. Currently, the implementation of HAXM is simple, we 
did not synchronize the vCPU register for xcr0 from QEMU. HAXM will handle the 
xcr0 state within the kernel space, including initialization, update, etc. This 
patch adds the xcr0 variable for allocating extra 8-byte buffer occupation, 
which will be passed between QEMU and HAXM when hax_sync_vcpu_state() is 
invoked. We have verified the patched QEMU and it can launch all guest OSes. 
Thanks for your comments.

I don't understand the patch very well, and I am on the phone so it's hard to 
check QEMU's HAXM support sources right now. Did HAXM 7.8.0 break support for 
QEMU without this patch, and likewise will QEMU with this patch will HAXM 
versions older than 7.8.0?

Or does this work on any version because QEMU treats the struct as a black box?

Paolo




Best Regards,
Wenchao

-Original Message-
From: Philippe Mathieu-Daudé mailto:phi...@linaro.org>>
Sent: Friday, November 25, 2022 21:37
To: Wang, Wenchao mailto:wenchao.w...@intel.com>>; 
qemu-devel@nongnu.org
Cc: haxm-team mailto:haxm-t...@intel.com>>; Paolo Bonzini 
mailto:pbonz...@redhat.com>>
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support

Hi,

On 25/11/22 13:18, Wang, Wenchao wrote:
> Hi, maintainers,
>
> As HAXM v7.8.0 is released and it added XCR0 support, could you help
> to merge this patch to add corresponding support into HAX user space
> of QEMU? The patch has been included in the attachment. Thanks.

See
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches
on how to send patches to a mailing list.

>
> Best Regards,
>
> Wenchao
>
>  From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00
> 2001
>
> From: Wenchao Wang mailto:wenchao.w...@intel.com>>
>
> Date: Fri, 25 Nov 2022 18:37:34 +0800
>
> Subject: [PATCH] target/i386/hax: Add XCR0 support
>
> Introduce extended control register XCR0 to support XSAVE feature set.
>
> Note: This change requires at least HAXM v7.8.0 to support.
>
> Reviewed-by: Hang Yuan mailto:hang.y...@intel.com>>
>
> Signed-off-by: Wenchao Wang 
> mailto:wenchao.w...@intel.com>>
>
> ---
>
> target/i386/hax/hax-interface.h | 2 ++
>
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/i386/hax/hax-interface.h
> b/target/i386/hax/hax-interface.h
>
> index 537ae084e9..1d13bb2380 100644
>
> --- a/target/i386/hax/hax-interface.h
>
> +++ b/target/i386/hax/hax-interface.h
>
> @@ -201,6 +201,8 @@ struct vcpu_state_t {
>
>   uint64_t _cr3;
>
>   uint64_t _cr4;
>
> +uint64_t _xcr0;
>
> +
>
>   uint64_t _dr0;
>
>   uint64_t _dr1;
>
>   uint64_t _dr2;
>
> --
>
> 2.17.1
>

Is that the full patch? It is missing the register use in 
hax_sync_vcpu_register()...

Regards,

Phil.

[PATCH] hw/loongarch/virt: rename PCH_PIC_IRQ_OFFSET with VIRT_GSI_BASE

[Bibo Mao]

In theory gsi base can start from 0 on loongarch virt machine,
however gsi base is hard-coded in linux kernel loongarch system,
else system fails to boot.

This patch renames macro PCH_PIC_IRQ_OFFSET with VIRT_GSI_BASE,
keeps value unchanged. GSI base is common concept in acpi spec
and easy to understand.

Signed-off-by: Bibo Mao 
---
 hw/loongarch/acpi-build.c  |  2 +-
 hw/loongarch/virt.c|  8 
 include/hw/pci-host/ls7a.h | 17 +
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/loongarch/acpi-build.c b/hw/loongarch/acpi-build.c
index c2b237736d..33e04e4b76 100644
--- a/hw/loongarch/acpi-build.c
+++ b/hw/loongarch/acpi-build.c
@@ -272,7 +272,7 @@ static void build_pci_device_aml(Aml *scope, 
LoongArchMachineState *lams)
 .pio.size= VIRT_PCI_IO_SIZE,
 .ecam.base   = VIRT_PCI_CFG_BASE,
 .ecam.size   = VIRT_PCI_CFG_SIZE,
-.irq = PCH_PIC_IRQ_OFFSET + VIRT_DEVICE_IRQS,
+.irq = VIRT_GSI_BASE + VIRT_DEVICE_IRQS,
 .bus = lams->pci_bus,
 };
 
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index c8a495ea30..3754e2151f 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -432,7 +432,7 @@ static DeviceState *create_acpi_ged(DeviceState *pch_pic, 
LoongArchMachineState
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 2, VIRT_GED_REG_ADDR);
 
 sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
-   qdev_get_gpio_in(pch_pic, VIRT_SCI_IRQ - 
PCH_PIC_IRQ_OFFSET));
+   qdev_get_gpio_in(pch_pic, VIRT_SCI_IRQ - 
VIRT_GSI_BASE));
 sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), _fatal);
 return dev;
 }
@@ -452,7 +452,7 @@ static DeviceState *create_platform_bus(DeviceState 
*pch_pic)
 
 sysbus = SYS_BUS_DEVICE(dev);
 for (i = 0; i < VIRT_PLATFORM_BUS_NUM_IRQS; i++) {
-irq = VIRT_PLATFORM_BUS_IRQ - PCH_PIC_IRQ_OFFSET + i;
+irq = VIRT_PLATFORM_BUS_IRQ - VIRT_GSI_BASE + i;
 sysbus_connect_irq(sysbus, i, qdev_get_gpio_in(pch_pic, irq));
 }
 
@@ -509,7 +509,7 @@ static void loongarch_devices_init(DeviceState *pch_pic, 
LoongArchMachineState *
 
 serial_mm_init(get_system_memory(), VIRT_UART_BASE, 0,
qdev_get_gpio_in(pch_pic,
-VIRT_UART_IRQ - PCH_PIC_IRQ_OFFSET),
+VIRT_UART_IRQ - VIRT_GSI_BASE),
115200, serial_hd(0), DEVICE_LITTLE_ENDIAN);
 fdt_add_uart_node(lams);
 
@@ -531,7 +531,7 @@ static void loongarch_devices_init(DeviceState *pch_pic, 
LoongArchMachineState *
 create_unimplemented_device("pci-dma-cfg", 0x1001041c, 0x4);
 sysbus_create_simple("ls7a_rtc", VIRT_RTC_REG_BASE,
  qdev_get_gpio_in(pch_pic,
- VIRT_RTC_IRQ - PCH_PIC_IRQ_OFFSET));
+ VIRT_RTC_IRQ - VIRT_GSI_BASE));
 fdt_add_rtc_node(lams);
 
 pm_mem = g_new(MemoryRegion, 1);
diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
index df7fa55a30..194aac905e 100644
--- a/include/hw/pci-host/ls7a.h
+++ b/include/hw/pci-host/ls7a.h
@@ -28,24 +28,25 @@
 #define VIRT_PCH_MSI_ADDR_LOW0x2FF0UL
 
 /*
- * According to the kernel pch irq start from 64 offset
- * 0 ~ 16 irqs used for non-pci device while 16 ~ 64 irqs
- * used for pci device.
+ * GSI_BASE is hard-coded with 64 in linux kernel, else kernel fails to boot
+ * 0  - 15  GSI for ISA devices even if there is no ISA devices
+ * 16 - 63  GSI for CPU devices such as timers/perf monitor etc
+ * 64 - GSI for external devices
  */
-#define PCH_PIC_IRQ_OFFSET   64
+#define VIRT_GSI_BASE64
 #define VIRT_DEVICE_IRQS 16
 #define VIRT_PCI_IRQS48
-#define VIRT_UART_IRQ(PCH_PIC_IRQ_OFFSET + 2)
+#define VIRT_UART_IRQ(VIRT_GSI_BASE + 2)
 #define VIRT_UART_BASE   0x1fe001e0
 #define VIRT_UART_SIZE   0X100
-#define VIRT_RTC_IRQ (PCH_PIC_IRQ_OFFSET + 3)
+#define VIRT_RTC_IRQ (VIRT_GSI_BASE + 3)
 #define VIRT_MISC_REG_BASE   (VIRT_PCH_REG_BASE + 0x0008)
 #define VIRT_RTC_REG_BASE(VIRT_MISC_REG_BASE + 0x00050100)
 #define VIRT_RTC_LEN 0x100
-#define VIRT_SCI_IRQ (PCH_PIC_IRQ_OFFSET + 4)
+#define VIRT_SCI_IRQ (VIRT_GSI_BASE + 4)
 
 #define VIRT_PLATFORM_BUS_BASEADDRESS   0x1600
 #define VIRT_PLATFORM_BUS_SIZE  0x200
 #define VIRT_PLATFORM_BUS_NUM_IRQS  2
-#define VIRT_PLATFORM_BUS_IRQ   69
+#define VIRT_PLATFORM_BUS_IRQ   (VIRT_GSI_BASE + 5)
 #endif
-- 
2.27.0

Re: [PATCH v5 4/4] x86: re-enable rng seeding via SetupData

[Jason A. Donenfeld]

On Tue, Dec 27, 2022 at 02:36:54PM +0100, Jason A. Donenfeld wrote:
> On Mon, Dec 26, 2022 at 05:57:30PM +0100, Jason A. Donenfeld wrote:
> > On Mon, Dec 26, 2022 at 03:43:04PM +0100, Jason A. Donenfeld wrote:
> > > On Mon, Dec 26, 2022 at 03:24:07PM +0100, Jason A. Donenfeld wrote:
> > > > Hi,
> > > > 
> > > > I'm currently stumped at the moment, so adding linux-mm@ and x86@. Still
> > > > working on it though. Details of where I'm at are below the quote below.
> > > > 
> > > > On Sat, Dec 24, 2022 at 05:21:46AM +0100, Jason A. Donenfeld wrote:
> > > > > On Sat, Dec 24, 2022 at 04:09:08AM +0100, Jason A. Donenfeld wrote:
> > > > > > Hi Eric,
> > > > > > 
> > > > > > Replying to you from my telephone, and I'm traveling the next two 
> > > > > > days,
> > > > > > but I thought I should mention some preliminary results right away 
> > > > > > from
> > > > > > doing some termux compiles:
> > > > > > 
> > > > > > On Fri, Dec 23, 2022 at 04:14:00PM -0800, Eric Biggers wrote:
> > > > > > > Hi Jason,
> > > > > > > 
> > > > > > > On Wed, Sep 21, 2022 at 11:31:34AM +0200, Jason A. Donenfeld 
> > > > > > > wrote:
> > > > > > > > This reverts 3824e25db1 ("x86: disable rng seeding via 
> > > > > > > > setup_data"), but
> > > > > > > > for 7.2 rather than 7.1, now that modifying setup_data is safe 
> > > > > > > > to do.
> > > > > > > > 
> > > > > > > > Cc: Laurent Vivier 
> > > > > > > > Cc: Michael S. Tsirkin 
> > > > > > > > Cc: Paolo Bonzini 
> > > > > > > > Cc: Peter Maydell 
> > > > > > > > Cc: Philippe Mathieu-Daudé 
> > > > > > > > Cc: Richard Henderson 
> > > > > > > > Cc: Ard Biesheuvel 
> > > > > > > > Acked-by: Gerd Hoffmann 
> > > > > > > > Signed-off-by: Jason A. Donenfeld 
> > > > > > > > ---
> > > > > > > >  hw/i386/microvm.c | 2 +-
> > > > > > > >  hw/i386/pc_piix.c | 3 ++-
> > > > > > > >  hw/i386/pc_q35.c  | 3 ++-
> > > > > > > >  3 files changed, 5 insertions(+), 3 deletions(-)
> > > > > > > > 
> > > > > > > 
> > > > > > > After upgrading to QEMU 7.2, Linux 6.1 no longer boots with some 
> > > > > > > configs.  There
> > > > > > > is no output at all.  I bisected it to this commit, and I 
> > > > > > > verified that the
> > > > > > > following change to QEMU's master branch makes the problem go 
> > > > > > > away:
> > > > > > > 
> > > > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > > > > index b48047f50c..42f5b07d2f 100644
> > > > > > > --- a/hw/i386/pc_piix.c
> > > > > > > +++ b/hw/i386/pc_piix.c
> > > > > > > @@ -441,6 +441,7 @@ static void 
> > > > > > > pc_i440fx_8_0_machine_options(MachineClass *m)
> > > > > > >  pc_i440fx_machine_options(m);
> > > > > > >  m->alias = "pc";
> > > > > > >  m->is_default = true;
> > > > > > > +PC_MACHINE_CLASS(m)->legacy_no_rng_seed = true;
> > > > > > >  }
> > > > > > > 
> > > > > > > I've attached the kernel config I am seeing the problem on.
> > > > > > > 
> > > > > > > For some reason, the problem also goes away if I disable 
> > > > > > > CONFIG_KASAN.
> > > > > > > 
> > > > > > > Any idea what is causing this?
> > > > > > 
> > > > > > - Commenting out the call to parse_setup_data() doesn't fix the 
> > > > > > issue.
> > > > > >   So there's no KASAN issue with the actual parser.
> > > > > > 
> > > > > > - Using KASAN_OUTLINE rather than INLINE does fix the issue!
> > > > > > 
> > > > > > That makes me suspect that it's file size related, and QEMU or the 
> > > > > > BIOS
> > > > > > is placing setup data at an overlapping offset by accident, or 
> > > > > > something
> > > > > > similar.
> > > > > 
> > > > > I removed the file systems from your config to bring the kernel size
> > > > > back down, and voila, it works, even with KASAN_INLINE. So perhaps I'm
> > > > > on the right track here...
> > > > 
> > > > QEMU sticks setup_data after the kernel image, the same as kexec-tools
> > > > and everything else. Apparently, when the kernel image is large, the
> > > > call to early_memremap(boot_params.hdr.setup_data, ...) returns a value
> > > > that points some place bogus, and the system crashes or does something
> > > > weird. I haven't yet determined what this limit is, but in my current
> > > > test kernel, a value of 0x01327650 is enough to make it point to
> > > > rubbish.
> > > > 
> > > > Is this expected? What's going on here?
> > > 
> > > Attaching gdb to QEMU and switching it to physical memory mode
> > > (`maintenance packet Qqemu.PhyMemMode:1 `) indicates that it
> > > early_memremap is actually working fine and something *else* is at this
> > > address? That's kinda weird... Is KASAN populating physical addresses
> > > immediately after the kernel image extremely early in boot? I'm seeing
> > > the crash happen from early_reserve_memory()->
> > > memblock_x86_reserve_range_setup_data(), which should be before
> > > kasan_init() even runs. Is QEMU calculating kernel_size wrong, when it
> > > goes to determine where to put the setup_data data? But that's the same
> > > calculation as used everywhere else, 

RE: [PATCH] target/i386/hax: Add XCR0 support

[Wang, Wenchao]

Hi, Paolo,

Thanks for your reply.

The reason why the variable xcr0 must be added to the header file of QEMU is 
because HAXM needs QEMU to allocate memory from user space and pass it to the 
kernel. This patch is only used to expand the buffer size of the structure, and 
HAXM will use and maintain this variable.
Without this patch, HAXM v7.8.0 will break support for QEMU and the HAXM 
versions older than 7.8.0 cannot support QEMU with this patch, either. It will 
work on any version since HAXM v7.8.0. I know QEMU treats the structure as a 
black box, but HAXM never supported xcr0 before and the structure size is not 
enough if it has been supported. We have verified the patched QEMU and it can 
launch all guest OSes. Thanks.


Best Regards,
Wenchao

From: Paolo Bonzini 
Sent: Tuesday, December 27, 2022 23:13
To: Wang, Wenchao 
Cc: Philippe Mathieu-Daudé ; qemu-devel 
; haxm-team 
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support


Il lun 28 nov 2022, 09:12 Wang, Wenchao 
mailto:wenchao.w...@intel.com>> ha scritto:
Hi, Philippe,

It is just the full patch. Currently, the implementation of HAXM is simple, we 
did not synchronize the vCPU register for xcr0 from QEMU. HAXM will handle the 
xcr0 state within the kernel space, including initialization, update, etc. This 
patch adds the xcr0 variable for allocating extra 8-byte buffer occupation, 
which will be passed between QEMU and HAXM when hax_sync_vcpu_state() is 
invoked. We have verified the patched QEMU and it can launch all guest OSes. 
Thanks for your comments.

I don't understand the patch very well, and I am on the phone so it's hard to 
check QEMU's HAXM support sources right now. Did HAXM 7.8.0 break support for 
QEMU without this patch, and likewise will QEMU with this patch will HAXM 
versions older than 7.8.0?

Or does this work on any version because QEMU treats the struct as a black box?

Paolo




Best Regards,
Wenchao

-Original Message-
From: Philippe Mathieu-Daudé mailto:phi...@linaro.org>>
Sent: Friday, November 25, 2022 21:37
To: Wang, Wenchao mailto:wenchao.w...@intel.com>>; 
qemu-devel@nongnu.org
Cc: haxm-team mailto:haxm-t...@intel.com>>; Paolo Bonzini 
mailto:pbonz...@redhat.com>>
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support

Hi,

On 25/11/22 13:18, Wang, Wenchao wrote:
> Hi, maintainers,
>
> As HAXM v7.8.0 is released and it added XCR0 support, could you help
> to merge this patch to add corresponding support into HAX user space
> of QEMU? The patch has been included in the attachment. Thanks.

See
https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches
on how to send patches to a mailing list.

>
> Best Regards,
>
> Wenchao
>
>  From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00
> 2001
>
> From: Wenchao Wang mailto:wenchao.w...@intel.com>>
>
> Date: Fri, 25 Nov 2022 18:37:34 +0800
>
> Subject: [PATCH] target/i386/hax: Add XCR0 support
>
> Introduce extended control register XCR0 to support XSAVE feature set.
>
> Note: This change requires at least HAXM v7.8.0 to support.
>
> Reviewed-by: Hang Yuan mailto:hang.y...@intel.com>>
>
> Signed-off-by: Wenchao Wang 
> mailto:wenchao.w...@intel.com>>
>
> ---
>
> target/i386/hax/hax-interface.h | 2 ++
>
> 1 file changed, 2 insertions(+)
>
> diff --git a/target/i386/hax/hax-interface.h
> b/target/i386/hax/hax-interface.h
>
> index 537ae084e9..1d13bb2380 100644
>
> --- a/target/i386/hax/hax-interface.h
>
> +++ b/target/i386/hax/hax-interface.h
>
> @@ -201,6 +201,8 @@ struct vcpu_state_t {
>
>   uint64_t _cr3;
>
>   uint64_t _cr4;
>
> +uint64_t _xcr0;
>
> +
>
>   uint64_t _dr0;
>
>   uint64_t _dr1;
>
>   uint64_t _dr2;
>
> --
>
> 2.17.1
>

Is that the full patch? It is missing the register use in 
hax_sync_vcpu_register()...

Regards,

Phil.

Re: [RFC PATCH 01/43] target/loongarch: Add vector data type vec_t

[gaosong]

在 2022/12/25 上午1:24, Richard Henderson 写道:

On 12/24/22 00:15, Song Gao wrote:

+union fpr_t {
+    uint64_t d;
+    vec_t vec;
+};
+
  struct LoongArchTLB {
  uint64_t tlb_misc;
  /* Fields corresponding to CSR_TLBELO0/1 */
@@ -251,7 +267,7 @@ typedef struct CPUArchState {
  uint64_t gpr[32];
  uint64_t pc;
  -    uint64_t fpr[32];
+    fpr_t fpr[32];


I didn't spot it right away, because you didn't add ".d" to the tcg 
register allocation, 

Oh,    my mistake.
but if you use tcg/tcg-op-gvec.h (and you really should), then you 
will also have to remove



    for (i = 0; i < 32; i++) {
    int off = offsetof(CPULoongArchState, fpr[i]);
    cpu_fpr[i] = tcg_global_mem_new_i64(cpu_env, off, fregnames[i]);
    }


because one cannot modify global_mem variables with gvec.

The manual says "The lower 64 bits of each vector register overlap with 
the floating point register of the same number.  In other words
When the basic floating-point instruction is executed to update the 
floating-point register, the low 64 bits of the corresponding LSX register

are also updated to the same value."

So If we don't use the fpr_t.  we should:
1 Update LSX low 64 bits after floating point instruction translation;
2 Update floating-point registers after LSX instruction translation.

Should we do this  or have I misunderstood?
I strongly suggest that you introduce wrappers to load/store fpr 
values from their env slots.  I would name them similarly to 
gpr_{src,dst}, gen_set_gpr.



Got it .

Thanks.
Song Gao

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

[gaosong]

在 2022/12/24 下午11:39, Richard Henderson 写道:

On 12/24/22 00:15, Song Gao wrote:

Hi, Merry Christmas!

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


That is unfortunate, as it makes reviewing this difficult.
Is there a timeline for this being published?


Perhaps at the end of the first quarter of 2023.


In the meantime, I can at least point out some general issues.


Thank you very much.

Thanks.
Song Gao

Re: [PATCH 01/15] tests/avocado: add RISC-V opensbi boot test

[Wainer dos Santos Moschetta]

Hi Daniel,

On 12/21/22 15:22, Daniel Henrique Barboza wrote:

This test is used to do a quick sanity check to ensure that we're able
to run the existing QEMU FW image.

'sifive_u', 'spike' and 'virt' riscv64 machines, and 'sifive_u' and
'virt' 32 bit machines are able to run the default RISCV64_BIOS_BIN |
RISCV32_BIOS_BIN firmware with minimal options.

Cc: Cleber Rosa 
Cc: Philippe Mathieu-Daudé 
Cc: Wainer dos Santos Moschetta 
Cc: Beraldo Leal 
Signed-off-by: Daniel Henrique Barboza 
---
  tests/avocado/riscv_opensbi.py | 65 ++
  1 file changed, 65 insertions(+)
  create mode 100644 tests/avocado/riscv_opensbi.py


It looks good to me. Thanks!

Reviewed-by: Wainer dos Santos Moschetta 



diff --git a/tests/avocado/riscv_opensbi.py b/tests/avocado/riscv_opensbi.py
new file mode 100644
index 00..abc99ced30
--- /dev/null
+++ b/tests/avocado/riscv_opensbi.py
@@ -0,0 +1,65 @@
+# opensbi boot test for RISC-V machines
+#
+# Copyright (c) 2022, Ventana Micro
+#
+# 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_qemu import QemuSystemTest
+from avocado_qemu import wait_for_console_pattern
+
+class RiscvOpensbi(QemuSystemTest):
+"""
+:avocado: tags=accel:tcg
+"""
+timeout = 5
+
+def test_riscv64_virt(self):
+"""
+:avocado: tags=arch:riscv64
+:avocado: tags=machine:virt
+"""
+self.vm.set_console()
+self.vm.launch()
+wait_for_console_pattern(self, 'Platform Name')
+wait_for_console_pattern(self, 'Boot HART MEDELEG')
+
+def test_riscv64_spike(self):
+"""
+:avocado: tags=arch:riscv64
+:avocado: tags=machine:spike
+"""
+self.vm.set_console()
+self.vm.launch()
+wait_for_console_pattern(self, 'Platform Name')
+wait_for_console_pattern(self, 'Boot HART MEDELEG')
+
+def test_riscv64_sifive_u(self):
+"""
+:avocado: tags=arch:riscv64
+:avocado: tags=machine:sifive_u
+"""
+self.vm.set_console()
+self.vm.launch()
+wait_for_console_pattern(self, 'Platform Name')
+wait_for_console_pattern(self, 'Boot HART MEDELEG')
+
+def test_riscv32_virt(self):
+"""
+:avocado: tags=arch:riscv32
+:avocado: tags=machine:virt
+"""
+self.vm.set_console()
+self.vm.launch()
+wait_for_console_pattern(self, 'Platform Name')
+wait_for_console_pattern(self, 'Boot HART MEDELEG')
+
+def test_riscv32_sifive_u(self):
+"""
+:avocado: tags=arch:riscv32
+:avocado: tags=machine:sifive_u
+"""
+self.vm.set_console()
+self.vm.launch()
+wait_for_console_pattern(self, 'Platform Name')
+wait_for_console_pattern(self, 'Boot HART MEDELEG')

[PATCH] target/hexagon/idef-parser: fix two typos in README

[Matheus Tavares Bernardino]

Signed-off-by: Matheus Tavares Bernardino 
---
 target/hexagon/idef-parser/README.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/hexagon/idef-parser/README.rst 
b/target/hexagon/idef-parser/README.rst
index 65e6bf4ee5..ff6d14150a 100644
--- a/target/hexagon/idef-parser/README.rst
+++ b/target/hexagon/idef-parser/README.rst
@@ -552,11 +552,11 @@ to compare our buggy CPU state against, running
 
 ::
 
-meson configure -Dhexagon_idef_parser_enabled=false
+meson configure -Dhexagon_idef_parser=false
 
 will disable the idef-parser for all instructions and fallback on manual
 tinycode generator overrides, or on helper function implementations. 
Recompiling
-gives us ``qemu-hexagon`` which passes all tests. If ``qemu-heaxgon-buggy`` is
+gives us ``qemu-hexagon`` which passes all tests. If ``qemu-hexagon-buggy`` is
 our binary with the incorrect tinycode generators, we can compare the CPU state
 between the two versions
 
-- 
2.37.2

Re: [PATCH 01/15] tests/avocado: add RISC-V opensbi boot test

[Daniel Henrique Barboza]

Bin,

On 12/26/22 10:56, Bin Meng wrote:

On Sat, Dec 24, 2022 at 11:52 AM Bin Meng  wrote:

Hi,

On Fri, Dec 23, 2022 at 2:25 PM Bin Meng  wrote:

Hi Anup,

On Fri, Dec 23, 2022 at 12:56 AM Anup Patel  wrote:

On Thu, Dec 22, 2022 at 6:27 PM Bin Meng  wrote:

On Thu, Dec 22, 2022 at 6:47 PM Daniel Henrique Barboza
 wrote:



On 12/22/22 07:24, Bin Meng wrote:

On Thu, Dec 22, 2022 at 2:29 AM Daniel Henrique Barboza
 wrote:

This test is used to do a quick sanity check to ensure that we're able
to run the existing QEMU FW image.


[.]




+wait_for_console_pattern(self, 'Boot HART MEDELEG')

How about testing riscv32_spike too?


I didn't manage to make it work. This riscv64 spark command line boots opensbi:


$ ./qemu-system-riscv64 -nographic -display none -vga none -machine spike

OpenSBI v1.1
 _  _
/ __ \  / |  _ \_   _|
   | |  | |_ __   ___ _ __ | (___ | |_) || |
   | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
   | |__| | |_) |  __/ | | |) | |_) || |_
\/| .__/ \___|_| |_|_/|/_|
  | |
  |_|

(...)

The same command line doesn't boot riscv32 spark:

./qemu-system-riscv32 -nographic -display none -vga none -machine spike
(--- hangs indefinitely ---)

I debugged it a bit and, as far as boot code goes, it goes all the way and 
loads the
opensbi 32bit binary.

After that I tried to found any command line example that boots spike with 
riscv32
bit and didn't find any.  So I gave up digging it further because I became 
unsure
about whether 32-bit spike works.

If someone can verify that yes, 32-bit spike is supposed to work, then I 
believe it's
worth investigating why it's not the case ATM.


+Anup who might know if QEMU spike 32-bit machine works with opensbi
32-bit generic image.

We never got HTIF putc() working on QEMU RV32 Spike but it works
perfectly fine on QEMU RV64 Spike.

Where is the problem for the 32-bit? Is it in OpenSBI or in QEMU?


See below log of QEMU RV64 Spike ...


If we cannot get Spike 32-bit to work in QEMU, should we drop the
32-bit support? @Alistair Francis

I got a deeper look at the 32-bit spike issue and I believe it is a
problem of QEMU HTIF emulation.

I will see if I can spin a patch to fix this.


It turns out there is a bug in OpenSBI too when booting 32-bit BIN
image on Spike.

For ELF & BIN image boot on QEMU, QEMU changes are needed. I will send
the QEMU patches soon.


I reviewed the QEMU patches and sent a tested-by ack in the opensbi fix
as well. LGTM.

As I commented in your QEMU patches earlier [1], it would be good if we could
have an opensbi 1.2 rom with the fix already applied. If that's not possible 
then
what we could do, w.r.t this patch, is to add the spike 32bit test with a skip 
note
mentioning that we need an opensbi update before enabling it.

[1] https://mail.gnu.org/archive/html/qemu-devel/2022-12/msg04414.html


Thanks,


Daniel




Regards,
Bin

Re: [PATCH v3 3/3] vdpa: commit all host notifier MRs in a single MR transaction

[Michael S. Tsirkin]

On Tue, Dec 27, 2022 at 05:51:47PM +0100, Philippe Mathieu-Daudé wrote:
> On 27/12/22 08:20, Longpeng(Mike) wrote:
> > From: Longpeng 
> > 
> > This allows the vhost-vdpa device to batch the setup of all its MRs of
> > host notifiers.
> > 
> > This significantly reduces the device starting time, e.g. the time spend
> > on setup the host notifier MRs reduce from 423ms to 32ms for a VM with
> > 64 vCPUs and 3 vhost-vDPA generic devices (vdpa_sim_blk, 64vq per device).
> > 
> > Signed-off-by: Longpeng 
> > ---
> >   hw/virtio/vhost-vdpa.c | 25 +++--
> >   1 file changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> > index fd0c33b0e1..870265188a 100644
> > --- a/hw/virtio/vhost-vdpa.c
> > +++ b/hw/virtio/vhost-vdpa.c
> > @@ -512,9 +512,18 @@ static void vhost_vdpa_host_notifiers_uninit(struct 
> > vhost_dev *dev, int n)
> >   {
> >   int i;
> > +/*
> > + * Pack all the changes to the memory regions in a single
> > + * transaction to avoid a few updating of the address space
> > + * topology.
> > + */
> > +memory_region_transaction_begin();
> > +
> >   for (i = dev->vq_index; i < dev->vq_index + n; i++) {
> >   vhost_vdpa_host_notifier_uninit(dev, i);
> >   }
> > +
> > +memory_region_transaction_commit();
> >   }
> 
> Instead of optimizing one frontend, I wonder if we shouldn't expose
> a 'bool memory_region_transaction_in_progress()' helper in memory.c,
> and in virtio_queue_set_host_notifier_mr() backend, assert we are
> within a transaction. That way we'd optimize all virtio frontends.


If we are doing something like this, I'd rather pass around
a "transaction" structure so this can be checked statically.
Looks like something that can be done on top though.

-- 
MST

Re: [PATCH v3 2/3] vhost: configure all host notifiers in a single MR transaction

[Michael S. Tsirkin]

On Tue, Dec 27, 2022 at 05:43:57PM +0100, Philippe Mathieu-Daudé wrote:
> On 27/12/22 08:20, Longpeng(Mike) wrote:
> > From: Longpeng 
> > 
> > This allows the vhost device to batch the setup of all its host notifiers.
> > This significantly reduces the device starting time, e.g. the time spend
> > on enabling notifiers reduce from 376ms to 9.1ms for a VM with 64 vCPUs
> > and 3 vhost-vDPA generic devices (vdpa_sim_blk, 64vq per device)
> > 
> > Signed-off-by: Longpeng 
> > ---
> >   hw/virtio/vhost.c | 24 
> >   1 file changed, 24 insertions(+)
> > 
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index 5994559da8..064d4abe5c 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -1562,16 +1562,25 @@ int vhost_dev_enable_notifiers(struct vhost_dev 
> > *hdev, VirtIODevice *vdev)
> >   return r;
> >   }
> > +/*
> > + * Batch all the host notifiers in a single transaction to avoid
> > + * quadratic time complexity in address_space_update_ioeventfds().
> > + */
> > +memory_region_transaction_begin();
> > +
> >   for (i = 0; i < hdev->nvqs; ++i) {
> >   r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index 
> > + i,
> >true);
> >   if (r < 0) {
> >   error_report("vhost VQ %d notifier binding failed: %d", i, 
> > -r);
> > +memory_region_transaction_commit();
> >   vhost_dev_disable_notifiers(hdev, vdev);
> 
> Could we 'break' here, ...
> 
> >   return r;
> >   }
> >   }
> > +memory_region_transaction_commit();
> > +
> >   return 0;
> 
> ... and return 'r' here?


won't this commit twice? seems ugly ...

> >   }
> 
> Otherwise LGTM.

Re: [PATCH 00/12] hw/riscv: Improve Spike HTIF emulation fidelity

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

At present the 32-bit OpenSBI generic firmware image does not boot on
Spike, only 64-bit image can. This is due to the HTIF emulation does
not implement the proxy syscall interface which is required for the
32-bit HTIF console output.

An OpenSBI bug fix [1] is also needed when booting the plain binary image.

With this series plus the above OpenSBI fix, both 32-bit OpenSBI BIN & ELF
images can boot on QEMU 'spike' machine.

[1] 
https://patchwork.ozlabs.org/project/opensbi/patch/20221226033603.1860569-1-bm...@tinylab.org/


Aside from a nit in patch 12/12, LGTM. I've tested with a patched version of
Opensbi including [1] and I can get terminal output with riscv32 spike:

$ ./qemu-system-riscv32 -M spike -display none -nographic
-bios ../../opensbi/build/platform/generic/firmware/fw_payload.bin

OpenSBI v1.1-112-g6ce00f8
       _  _
  / __ \  / |  _ \_   _|
 | |  | |_ __   ___ _ __ | (___ | |_) || |
 | |  | | '_ \ / _ \ '_ \ \___ \|  _ < | |
 | |__| | |_) |  __/ | | |) | |_) || |_
  \/| .__/ \___|_| |_|_/|/_|
    | |
    |_|
(...)


Speaking of [1], it seems like the fix went a bit too late for the opensbi 1.2 
release.
Assuming that [1] is accepted, it would be nice if we could bake in this fix on 
top of the
1.2 release when updating the QEMU roms.


Thanks,

Daniel




Bin Meng (10):
   hw/char: riscv_htif: Avoid using magic numbers
   hw/char: riscv_htif: Drop {to,from}host_size in HTIFState
   hw/char: riscv_htif: Drop useless assignment of memory region
   hw/char: riscv_htif: Use conventional 's' for HTIFState
   hw/char: riscv_htif: Move registers from CPUArchState to HTIFState
   hw/char: riscv_htif: Remove forward declarations for non-existent
 variables
   hw/char: riscv_htif: Support console output via proxy syscall
   hw/riscv: spike: Remove the out-of-date comments
   hw/riscv/boot.c: Introduce riscv_find_firmware()
   hw/riscv: spike: Decouple create_fdt() dependency to ELF loading

Daniel Henrique Barboza (2):
   hw/riscv/boot.c: make riscv_find_firmware() static
   hw/riscv/boot.c: introduce riscv_default_firmware_name()

  include/hw/char/riscv_htif.h |  19 +---
  include/hw/riscv/boot.h  |   4 +-
  target/riscv/cpu.h   |   4 -
  hw/char/riscv_htif.c | 172 +--
  hw/riscv/boot.c  |  76 ++--
  hw/riscv/sifive_u.c  |  11 +--
  hw/riscv/spike.c |  59 
  hw/riscv/virt.c  |  10 +-
  target/riscv/machine.c   |   6 +-
  9 files changed, 212 insertions(+), 149 deletions(-)

Re: [PATCH 11/12] hw/riscv/boot.c: Introduce riscv_find_firmware()

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

Rename previous riscv_find_firmware() to riscv_find_bios(), and
introduce a new riscv_find_firmware() to implement the first half
part of the work done in riscv_find_and_load_firmware().

This new API is helpful for machine that wants to know the final
chosen firmware file name but does not want to load it.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  include/hw/riscv/boot.h |  2 ++
  hw/riscv/boot.c | 39 +--
  2 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/include/hw/riscv/boot.h b/include/hw/riscv/boot.h
index 60cf320c88..b273ab22f7 100644
--- a/include/hw/riscv/boot.h
+++ b/include/hw/riscv/boot.h
@@ -38,6 +38,8 @@ target_ulong riscv_find_and_load_firmware(MachineState 
*machine,
hwaddr firmware_load_addr,
symbol_fn_t sym_cb);
  const char *riscv_default_firmware_name(RISCVHartArrayState *harts);
+char *riscv_find_firmware(const char *firmware_filename,
+  const char *default_machine_firmware);
  target_ulong riscv_load_firmware(const char *firmware_filename,
   hwaddr firmware_load_addr,
   symbol_fn_t sym_cb);
diff --git a/hw/riscv/boot.c b/hw/riscv/boot.c
index e1a544b1d9..98b80af51b 100644
--- a/hw/riscv/boot.c
+++ b/hw/riscv/boot.c
@@ -84,11 +84,11 @@ const char *riscv_default_firmware_name(RISCVHartArrayState 
*harts)
  return RISCV64_BIOS_BIN;
  }
  
-static char *riscv_find_firmware(const char *firmware_filename)

+static char *riscv_find_bios(const char *bios_filename)
  {
  char *filename;
  
-filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, firmware_filename);

+filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_filename);
  if (filename == NULL) {
  if (!qtest_enabled()) {
  /*
@@ -97,8 +97,8 @@ static char *riscv_find_firmware(const char 
*firmware_filename)
   * running QEMU test will complain hence let's suppress the error
   * report for QEMU testing.
   */
-error_report("Unable to load the RISC-V firmware \"%s\"",
- firmware_filename);
+error_report("Unable to find the RISC-V BIOS \"%s\"",
+ bios_filename);
  exit(1);
  }
  }
@@ -106,25 +106,36 @@ static char *riscv_find_firmware(const char 
*firmware_filename)
  return filename;
  }
  
-target_ulong riscv_find_and_load_firmware(MachineState *machine,

-  const char *default_machine_firmware,
-  hwaddr firmware_load_addr,
-  symbol_fn_t sym_cb)
+char *riscv_find_firmware(const char *firmware_filename,
+  const char *default_machine_firmware)
  {
-char *firmware_filename = NULL;
-target_ulong firmware_end_addr = firmware_load_addr;
+char *filename = NULL;
  
-if ((!machine->firmware) || (!strcmp(machine->firmware, "default"))) {

+if ((!firmware_filename) || (!strcmp(firmware_filename, "default"))) {
  /*
   * The user didn't specify -bios, or has specified "-bios default".
   * That means we are going to load the OpenSBI binary included in
   * the QEMU source.
   */
-firmware_filename = riscv_find_firmware(default_machine_firmware);
-} else if (strcmp(machine->firmware, "none")) {
-firmware_filename = riscv_find_firmware(machine->firmware);
+filename = riscv_find_bios(default_machine_firmware);
+} else if (strcmp(firmware_filename, "none")) {
+filename = riscv_find_bios(firmware_filename);
  }
  
+return filename;

+}
+
+target_ulong riscv_find_and_load_firmware(MachineState *machine,
+  const char *default_machine_firmware,
+  hwaddr firmware_load_addr,
+  symbol_fn_t sym_cb)
+{
+char *firmware_filename;
+target_ulong firmware_end_addr = firmware_load_addr;
+
+firmware_filename = riscv_find_firmware(machine->firmware,
+default_machine_firmware);
+
  if (firmware_filename) {
  /* If not "none" load the firmware */
  firmware_end_addr = riscv_load_firmware(firmware_filename,

Re: [PATCH 08/12] hw/riscv: spike: Remove the out-of-date comments

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

Spike machine now supports OpenSBI plain binary bios image, so the
comments are no longer valid.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  hw/riscv/spike.c | 5 -
  1 file changed, 5 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 8606331f61..ab0a945f8b 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -256,11 +256,6 @@ static void spike_board_init(MachineState *machine)
  memory_region_add_subregion(system_memory, memmap[SPIKE_MROM].base,
  mask_rom);
  
-/*

- * Not like other RISC-V machines that use plain binary bios images,
- * keeping ELF files here was intentional because BIN files don't work
- * for the Spike machine as HTIF emulation depends on ELF parsing.
- */
  if (riscv_is_32bit(>soc[0])) {
  firmware_end_addr = riscv_find_and_load_firmware(machine,
  RISCV32_BIOS_BIN, memmap[SPIKE_DRAM].base,

Re: [PATCH 07/12] hw/char: riscv_htif: Support console output via proxy syscall

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

At present the HTIF proxy syscall is unsupported. On RV32, only
device 0 is supported so there is no console device for RV32.
The only way to implement console funtionality on RV32 is to
support the SYS_WRITE syscall.

With this commit, the Spike machine is able to boot the 32-bit
OpenSBI generic image.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  hw/char/riscv_htif.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 3bb0a37a3e..1477fc0090 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -48,6 +48,9 @@
  #define HTIF_CONSOLE_CMD_GETC   0
  #define HTIF_CONSOLE_CMD_PUTC   1
  
+/* PK system call number */

+#define PK_SYS_WRITE64
+
  static uint64_t fromhost_addr, tohost_addr;
  static int address_symbol_set;
  
@@ -165,7 +168,19 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written)

  int exit_code = payload >> 1;
  exit(exit_code);
  } else {
-qemu_log_mask(LOG_UNIMP, "pk syscall proxy not supported\n");
+uint64_t syscall[8];
+cpu_physical_memory_read(payload, syscall, sizeof(syscall));
+if (syscall[0] == PK_SYS_WRITE &&
+syscall[1] == HTIF_DEV_CONSOLE &&
+syscall[3] == HTIF_CONSOLE_CMD_PUTC) {
+uint8_t ch;
+cpu_physical_memory_read(syscall[2], , 1);
+qemu_chr_fe_write(>chr, , 1);
+resp = 0x100 | (uint8_t)payload;
+} else {
+qemu_log_mask(LOG_UNIMP,
+  "pk syscall proxy not supported\n");
+}
  }
  } else {
  qemu_log("HTIF device %d: unknown command\n", device);

Re: [PATCH 06/12] hw/char: riscv_htif: Remove forward declarations for non-existent variables

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

There are forward declarations for 'vmstate_htif' and 'htif_io_ops'
in riscv_htif.h however there are no definitions in the C codes.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  include/hw/char/riscv_htif.h | 3 ---
  1 file changed, 3 deletions(-)

diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
index 55cc352331..9e8ebbe017 100644
--- a/include/hw/char/riscv_htif.h
+++ b/include/hw/char/riscv_htif.h
@@ -40,9 +40,6 @@ typedef struct HTIFState {
  uint64_t pending_read;
  } HTIFState;
  
-extern const VMStateDescription vmstate_htif;

-extern const MemoryRegionOps htif_io_ops;
-
  /* HTIF symbol callback */
  void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value,
  uint64_t st_size);

Re: [PATCH 05/12] hw/char: riscv_htif: Move registers from CPUArchState to HTIFState

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

At present for some unknown reason the HTIF registers (fromhost &
tohost) are defined in the RISC-V CPUArchState. It should really
be put in the HTIFState struct as it is only meaningful to HTIF.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  include/hw/char/riscv_htif.h |  8 
  target/riscv/cpu.h   |  4 
  hw/char/riscv_htif.c | 35 +--
  hw/riscv/spike.c |  3 +--
  target/riscv/machine.c   |  6 ++
  5 files changed, 24 insertions(+), 32 deletions(-)

diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
index 6d172ebd6d..55cc352331 100644
--- a/include/hw/char/riscv_htif.h
+++ b/include/hw/char/riscv_htif.h
@@ -23,7 +23,6 @@
  #include "chardev/char.h"
  #include "chardev/char-fe.h"
  #include "exec/memory.h"
-#include "target/riscv/cpu.h"
  
  #define TYPE_HTIF_UART "riscv.htif.uart"
  
@@ -31,11 +30,12 @@ typedef struct HTIFState {

  int allow_tohost;
  int fromhost_inprogress;
  
+uint64_t tohost;

+uint64_t fromhost;
  hwaddr tohost_offset;
  hwaddr fromhost_offset;
  MemoryRegion mmio;
  
-CPURISCVState *env;

  CharBackend chr;
  uint64_t pending_read;
  } HTIFState;
@@ -51,7 +51,7 @@ void htif_symbol_callback(const char *st_name, int st_info, 
uint64_t st_value,
  bool htif_uses_elf_symbols(void);
  
  /* legacy pre qom */

-HTIFState *htif_mm_init(MemoryRegion *address_space, CPURISCVState *env,
-Chardev *chr, uint64_t nonelf_base);
+HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
+uint64_t nonelf_base);
  
  #endif

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 443d15a47c..6f04d853dd 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -309,10 +309,6 @@ struct CPUArchState {
  target_ulong sscratch;
  target_ulong mscratch;
  
-/* temporary htif regs */

-uint64_t mfromhost;
-uint64_t mtohost;
-
  /* Sstc CSRs */
  uint64_t stimecmp;
  
diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c

index f28976b110..3bb0a37a3e 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -100,7 +100,7 @@ static void htif_recv(void *opaque, const uint8_t *buf, int 
size)
  uint64_t val_written = s->pending_read;
  uint64_t resp = 0x100 | *buf;
  
-s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);

+s->fromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
  }
  
  /*

@@ -175,7 +175,7 @@ static void htif_handle_tohost_write(HTIFState *s, uint64_t 
val_written)
  if (cmd == HTIF_CONSOLE_CMD_GETC) {
  /* this should be a queue, but not yet implemented as such */
  s->pending_read = val_written;
-s->env->mtohost = 0; /* clear to indicate we read */
+s->tohost = 0; /* clear to indicate we read */
  return;
  } else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
  qemu_chr_fe_write(>chr, (uint8_t *), 1);
@@ -195,11 +195,11 @@ static void htif_handle_tohost_write(HTIFState *s, 
uint64_t val_written)
   * HTIF needs protocol documentation and a more complete state machine.
   *
   *  while (!s->fromhost_inprogress &&
- *  s->env->mfromhost != 0x0) {
+ *  s->fromhost != 0x0) {
   *  }
   */
-s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
-s->env->mtohost = 0; /* clear to indicate we read */
+s->fromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
+s->tohost = 0; /* clear to indicate we read */
  }
  
  #define TOHOST_OFFSET1  (s->tohost_offset)

@@ -212,13 +212,13 @@ static uint64_t htif_mm_read(void *opaque, hwaddr addr, 
unsigned size)
  {
  HTIFState *s = opaque;
  if (addr == TOHOST_OFFSET1) {
-return s->env->mtohost & 0x;
+return s->tohost & 0x;
  } else if (addr == TOHOST_OFFSET2) {
-return (s->env->mtohost >> 32) & 0x;
+return (s->tohost >> 32) & 0x;
  } else if (addr == FROMHOST_OFFSET1) {
-return s->env->mfromhost & 0x;
+return s->fromhost & 0x;
  } else if (addr == FROMHOST_OFFSET2) {
-return (s->env->mfromhost >> 32) & 0x;
+return (s->fromhost >> 32) & 0x;
  } else {
  qemu_log("Invalid htif read: address %016" PRIx64 "\n",
  (uint64_t)addr);
@@ -232,22 +232,22 @@ static void htif_mm_write(void *opaque, hwaddr addr,
  {
  HTIFState *s = opaque;
  if (addr == TOHOST_OFFSET1) {
-if (s->env->mtohost == 0x0) {
+if (s->tohost == 0x0) {
  s->allow_tohost = 1;
-s->env->mtohost = value & 0x;
+s->tohost = value & 0x;
  } else {
  s->allow_tohost = 0;
  }
  } else if (addr == TOHOST_OFFSET2) {
  if 

Re: [PATCH 04/12] hw/char: riscv_htif: Use conventional 's' for HTIFState

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

QEMU source codes tend to use 's' to represent the hardware state.
Let's use it for HTIFState.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  hw/char/riscv_htif.c | 64 ++--
  1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index e7e319ca1d..f28976b110 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -85,7 +85,7 @@ static int htif_can_recv(void *opaque)
   */
  static void htif_recv(void *opaque, const uint8_t *buf, int size)
  {
-HTIFState *htifstate = opaque;
+HTIFState *s = opaque;
  
  if (size != 1) {

  return;
@@ -97,10 +97,10 @@ static void htif_recv(void *opaque, const uint8_t *buf, int 
size)
   *will drop characters
   */
  
-uint64_t val_written = htifstate->pending_read;

+uint64_t val_written = s->pending_read;
  uint64_t resp = 0x100 | *buf;
  
-htifstate->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);

+s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
  }
  
  /*

@@ -142,7 +142,7 @@ static int htif_be_change(void *opaque)
   * For RV32, the tohost register is zero-extended, so only device=0 and
   * command=0 (i.e. HTIF syscalls/exit codes) are supported.
   */
-static void htif_handle_tohost_write(HTIFState *htifstate, uint64_t 
val_written)
+static void htif_handle_tohost_write(HTIFState *s, uint64_t val_written)
  {
  uint8_t device = val_written >> HTIF_DEV_SHIFT;
  uint8_t cmd = val_written >> HTIF_CMD_SHIFT;
@@ -174,11 +174,11 @@ static void htif_handle_tohost_write(HTIFState 
*htifstate, uint64_t val_written)
  /* HTIF Console */
  if (cmd == HTIF_CONSOLE_CMD_GETC) {
  /* this should be a queue, but not yet implemented as such */
-htifstate->pending_read = val_written;
-htifstate->env->mtohost = 0; /* clear to indicate we read */
+s->pending_read = val_written;
+s->env->mtohost = 0; /* clear to indicate we read */
  return;
  } else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
-qemu_chr_fe_write(>chr, (uint8_t *), 1);
+qemu_chr_fe_write(>chr, (uint8_t *), 1);
  resp = 0x100 | (uint8_t)payload;
  } else {
  qemu_log("HTIF device %d: unknown command\n", device);
@@ -194,31 +194,31 @@ static void htif_handle_tohost_write(HTIFState 
*htifstate, uint64_t val_written)
   * With this code disabled, qemu works with bbl priv v1.9.1 and v1.10.
   * HTIF needs protocol documentation and a more complete state machine.
   *
- *  while (!htifstate->fromhost_inprogress &&
- *  htifstate->env->mfromhost != 0x0) {
+ *  while (!s->fromhost_inprogress &&
+ *  s->env->mfromhost != 0x0) {
   *  }
   */
-htifstate->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
-htifstate->env->mtohost = 0; /* clear to indicate we read */
+s->env->mfromhost = (val_written >> 48 << 48) | (resp << 16 >> 16);
+s->env->mtohost = 0; /* clear to indicate we read */
  }
  
-#define TOHOST_OFFSET1 (htifstate->tohost_offset)

-#define TOHOST_OFFSET2 (htifstate->tohost_offset + 4)
-#define FROMHOST_OFFSET1 (htifstate->fromhost_offset)
-#define FROMHOST_OFFSET2 (htifstate->fromhost_offset + 4)
+#define TOHOST_OFFSET1  (s->tohost_offset)
+#define TOHOST_OFFSET2  (s->tohost_offset + 4)
+#define FROMHOST_OFFSET1(s->fromhost_offset)
+#define FROMHOST_OFFSET2(s->fromhost_offset + 4)
  
  /* CPU wants to read an HTIF register */

  static uint64_t htif_mm_read(void *opaque, hwaddr addr, unsigned size)
  {
-HTIFState *htifstate = opaque;
+HTIFState *s = opaque;
  if (addr == TOHOST_OFFSET1) {
-return htifstate->env->mtohost & 0x;
+return s->env->mtohost & 0x;
  } else if (addr == TOHOST_OFFSET2) {
-return (htifstate->env->mtohost >> 32) & 0x;
+return (s->env->mtohost >> 32) & 0x;
  } else if (addr == FROMHOST_OFFSET1) {
-return htifstate->env->mfromhost & 0x;
+return s->env->mfromhost & 0x;
  } else if (addr == FROMHOST_OFFSET2) {
-return (htifstate->env->mfromhost >> 32) & 0x;
+return (s->env->mfromhost >> 32) & 0x;
  } else {
  qemu_log("Invalid htif read: address %016" PRIx64 "\n",
  (uint64_t)addr);
@@ -230,25 +230,25 @@ static uint64_t htif_mm_read(void *opaque, hwaddr addr, 
unsigned size)
  static void htif_mm_write(void *opaque, hwaddr addr,
uint64_t value, unsigned size)
  {
-HTIFState *htifstate = opaque;
+HTIFState *s = opaque;
  if (addr == TOHOST_OFFSET1) {
-if (htifstate->env->mtohost == 0x0) {
-htifstate->allow_tohost = 1;
-htifstate->env->mtohost = value & 0x;

Re: [PATCH 03/12] hw/char: riscv_htif: Drop useless assignment of memory region

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

struct HTIFState has 3 members for address space and memory region,
and are initialized during htif_mm_init(). But they are actually
useless. Drop them.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  include/hw/char/riscv_htif.h | 7 ++-
  hw/char/riscv_htif.c | 7 ++-
  hw/riscv/spike.c | 5 ++---
  3 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
index 3eccc1914f..6d172ebd6d 100644
--- a/include/hw/char/riscv_htif.h
+++ b/include/hw/char/riscv_htif.h
@@ -34,9 +34,6 @@ typedef struct HTIFState {
  hwaddr tohost_offset;
  hwaddr fromhost_offset;
  MemoryRegion mmio;
-MemoryRegion *address_space;
-MemoryRegion *main_mem;
-void *main_mem_ram_ptr;
  
  CPURISCVState *env;

  CharBackend chr;
@@ -54,7 +51,7 @@ void htif_symbol_callback(const char *st_name, int st_info, 
uint64_t st_value,
  bool htif_uses_elf_symbols(void);
  
  /* legacy pre qom */

-HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,
-CPURISCVState *env, Chardev *chr, uint64_t nonelf_base);
+HTIFState *htif_mm_init(MemoryRegion *address_space, CPURISCVState *env,
+Chardev *chr, uint64_t nonelf_base);
  
  #endif

diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 088556bb04..e7e319ca1d 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -265,8 +265,8 @@ bool htif_uses_elf_symbols(void)
  return (address_symbol_set == 3) ? true : false;
  }
  
-HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,

-CPURISCVState *env, Chardev *chr, uint64_t nonelf_base)
+HTIFState *htif_mm_init(MemoryRegion *address_space, CPURISCVState *env,
+Chardev *chr, uint64_t nonelf_base)
  {
  uint64_t base, size, tohost_offset, fromhost_offset;
  
@@ -281,9 +281,6 @@ HTIFState *htif_mm_init(MemoryRegion *address_space, MemoryRegion *main_mem,

  fromhost_offset = fromhost_addr - base;
  
  HTIFState *s = g_new0(HTIFState, 1);

-s->address_space = address_space;
-s->main_mem = main_mem;
-s->main_mem_ram_ptr = memory_region_get_ram_ptr(main_mem);
  s->env = env;
  s->tohost_offset = tohost_offset;
  s->fromhost_offset = fromhost_offset;
diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 1e1d752c00..82cf41ac27 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -317,9 +317,8 @@ static void spike_board_init(MachineState *machine)
fdt_load_addr);
  
  /* initialize HTIF using symbols found in load_kernel */

-htif_mm_init(system_memory, mask_rom,
- >soc[0].harts[0].env, serial_hd(0),
- memmap[SPIKE_HTIF].base);
+htif_mm_init(system_memory, >soc[0].harts[0].env,
+ serial_hd(0), memmap[SPIKE_HTIF].base);
  }
  
  static void spike_machine_instance_init(Object *obj)

Re: [PATCH 02/12] hw/char: riscv_htif: Drop {to,from}host_size in HTIFState

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

These are not used anywhere. Drop them.

Signed-off-by: Bin Meng 
---


Reviewed-by: Daniel Henrique Barboza 



  include/hw/char/riscv_htif.h | 2 --
  1 file changed, 2 deletions(-)

diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
index f888ac1b30..3eccc1914f 100644
--- a/include/hw/char/riscv_htif.h
+++ b/include/hw/char/riscv_htif.h
@@ -33,8 +33,6 @@ typedef struct HTIFState {
  
  hwaddr tohost_offset;

  hwaddr fromhost_offset;
-uint64_t tohost_size;
-uint64_t fromhost_size;
  MemoryRegion mmio;
  MemoryRegion *address_space;
  MemoryRegion *main_mem;

Re: [PATCH 01/12] hw/char: riscv_htif: Avoid using magic numbers

[Daniel Henrique Barboza]

On 12/27/22 03:48, Bin Meng wrote:

The Spike HTIF is poorly documented. The only relevant info we can
get from the internet is from Andrew Waterman at [1].

Add a comment block before htif_handle_tohost_write() to explain
the tohost register format, and use meaningful macros intead of

s/intead/instead


magic numbers in the codes.

While we are here, corret 2 multi-line comment blocks that have


s/corret/correct


Reviewed-by: Daniel Henrique Barboza 


wrong format.

Link: https://github.com/riscv-software-src/riscv-isa-sim/issues/364 [1]
Signed-off-by: Bin Meng 
---

  hw/char/riscv_htif.c | 72 
  1 file changed, 52 insertions(+), 20 deletions(-)

diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 6577f0e640..088556bb04 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -38,6 +38,16 @@
  } 
 \
  } while (0)
  
+#define HTIF_DEV_SHIFT  56

+#define HTIF_CMD_SHIFT  48
+
+#define HTIF_DEV_SYSTEM 0
+#define HTIF_DEV_CONSOLE1
+
+#define HTIF_SYSTEM_CMD_SYSCALL 0
+#define HTIF_CONSOLE_CMD_GETC   0
+#define HTIF_CONSOLE_CMD_PUTC   1
+
  static uint64_t fromhost_addr, tohost_addr;
  static int address_symbol_set;
  
@@ -81,9 +91,11 @@ static void htif_recv(void *opaque, const uint8_t *buf, int size)

  return;
  }
  
-/* TODO - we need to check whether mfromhost is zero which indicates

-  the device is ready to receive. The current implementation
-  will drop characters */
+/*
+ * TODO - we need to check whether mfromhost is zero which indicates
+ *the device is ready to receive. The current implementation
+ *will drop characters
+ */
  
  uint64_t val_written = htifstate->pending_read;

  uint64_t resp = 0x100 | *buf;
@@ -110,10 +122,30 @@ static int htif_be_change(void *opaque)
  return 0;
  }
  
+/*

+ * See below the tohost register format.
+ *
+ * Bits 63:56 indicate the "device".
+ * Bits 55:48 indicate the "command".
+ *
+ * Device 0 is the syscall device, which is used to emulate Unixy syscalls.
+ * It only implements command 0, which has two subfunctions:
+ * - If bit 0 is clear, then bits 47:0 represent a pointer to a struct
+ *   describing the syscall.
+ * - If bit 1 is set, then bits 47:1 represent an exit code, with a zero
+ *   value indicating success and other values indicating failure.
+ *
+ * Device 1 is the blocking character device.
+ * - Command 0 reads a character
+ * - Command 1 writes a character from the 8 LSBs of tohost
+ *
+ * For RV32, the tohost register is zero-extended, so only device=0 and
+ * command=0 (i.e. HTIF syscalls/exit codes) are supported.
+ */
  static void htif_handle_tohost_write(HTIFState *htifstate, uint64_t 
val_written)
  {
-uint8_t device = val_written >> 56;
-uint8_t cmd = val_written >> 48;
+uint8_t device = val_written >> HTIF_DEV_SHIFT;
+uint8_t cmd = val_written >> HTIF_CMD_SHIFT;
  uint64_t payload = val_written & 0xULL;
  int resp = 0;
  
@@ -125,9 +157,9 @@ static void htif_handle_tohost_write(HTIFState *htifstate, uint64_t val_written)

   * 0: riscv-tests Pass/Fail Reporting Only (no syscall proxy)
   * 1: Console
   */
-if (unlikely(device == 0x0)) {
+if (unlikely(device == HTIF_DEV_SYSTEM)) {
  /* frontend syscall handler, shutdown and exit code support */
-if (cmd == 0x0) {
+if (cmd == HTIF_SYSTEM_CMD_SYSCALL) {
  if (payload & 0x1) {
  /* exit code */
  int exit_code = payload >> 1;
@@ -138,14 +170,14 @@ static void htif_handle_tohost_write(HTIFState 
*htifstate, uint64_t val_written)
  } else {
  qemu_log("HTIF device %d: unknown command\n", device);
  }
-} else if (likely(device == 0x1)) {
+} else if (likely(device == HTIF_DEV_CONSOLE)) {
  /* HTIF Console */
-if (cmd == 0x0) {
+if (cmd == HTIF_CONSOLE_CMD_GETC) {
  /* this should be a queue, but not yet implemented as such */
  htifstate->pending_read = val_written;
  htifstate->env->mtohost = 0; /* clear to indicate we read */
  return;
-} else if (cmd == 0x1) {
+} else if (cmd == HTIF_CONSOLE_CMD_PUTC) {
  qemu_chr_fe_write(>chr, (uint8_t *), 1);
  resp = 0x100 | (uint8_t)payload;
  } else {
@@ -157,15 +189,15 @@ static void htif_handle_tohost_write(HTIFState 
*htifstate, uint64_t val_written)
  " payload: %016" PRIx64, device, cmd, payload & 0xFF, payload);
  }
  /*
- * - latest bbl does not set fromhost to 0 if there is a value in tohost
- * - with this code enabled, qemu hangs waiting for fromhost to go to 0
- * - with this code disabled, qemu works with bbl priv v1.9.1 and v1.10
- * - HTIF needs protocol 

Re: [PATCH 4/4] tests/tcg/multiarch: add vma-pthread.c

[Alex Bennée]

Richard Henderson  writes:

> From: Ilya Leoshkevich 
>
> Add a test that locklessly changes and exercises page protection bits
> from various threads. This helps catch race conditions in the VMA
> handling.
>
> Signed-off-by: Ilya Leoshkevich 
> Message-Id: <20221223120252.513319-1-...@linux.ibm.com>
> Signed-off-by: Richard Henderson 

Acked-by: Alex Bennée 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH v3 3/3] vdpa: commit all host notifier MRs in a single MR transaction

[Philippe Mathieu-Daudé]

On 27/12/22 08:20, Longpeng(Mike) wrote:

From: Longpeng 

This allows the vhost-vdpa device to batch the setup of all its MRs of
host notifiers.

This significantly reduces the device starting time, e.g. the time spend
on setup the host notifier MRs reduce from 423ms to 32ms for a VM with
64 vCPUs and 3 vhost-vDPA generic devices (vdpa_sim_blk, 64vq per device).

Signed-off-by: Longpeng 
---
  hw/virtio/vhost-vdpa.c | 25 +++--
  1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index fd0c33b0e1..870265188a 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -512,9 +512,18 @@ static void vhost_vdpa_host_notifiers_uninit(struct 
vhost_dev *dev, int n)
  {
  int i;
  
+/*

+ * Pack all the changes to the memory regions in a single
+ * transaction to avoid a few updating of the address space
+ * topology.
+ */
+memory_region_transaction_begin();
+
  for (i = dev->vq_index; i < dev->vq_index + n; i++) {
  vhost_vdpa_host_notifier_uninit(dev, i);
  }
+
+memory_region_transaction_commit();
  }


Instead of optimizing one frontend, I wonder if we shouldn't expose
a 'bool memory_region_transaction_in_progress()' helper in memory.c,
and in virtio_queue_set_host_notifier_mr() backend, assert we are
within a transaction. That way we'd optimize all virtio frontends.

Re: [PATCH v3 2/3] vhost: configure all host notifiers in a single MR transaction

[Philippe Mathieu-Daudé]

On 27/12/22 08:20, Longpeng(Mike) wrote:

From: Longpeng 

This allows the vhost device to batch the setup of all its host notifiers.
This significantly reduces the device starting time, e.g. the time spend
on enabling notifiers reduce from 376ms to 9.1ms for a VM with 64 vCPUs
and 3 vhost-vDPA generic devices (vdpa_sim_blk, 64vq per device)

Signed-off-by: Longpeng 
---
  hw/virtio/vhost.c | 24 
  1 file changed, 24 insertions(+)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 5994559da8..064d4abe5c 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -1562,16 +1562,25 @@ int vhost_dev_enable_notifiers(struct vhost_dev *hdev, 
VirtIODevice *vdev)
  return r;
  }
  
+/*

+ * Batch all the host notifiers in a single transaction to avoid
+ * quadratic time complexity in address_space_update_ioeventfds().
+ */
+memory_region_transaction_begin();
+
  for (i = 0; i < hdev->nvqs; ++i) {
  r = virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), hdev->vq_index + i,
   true);
  if (r < 0) {
  error_report("vhost VQ %d notifier binding failed: %d", i, -r);
+memory_region_transaction_commit();
  vhost_dev_disable_notifiers(hdev, vdev);


Could we 'break' here, ...


  return r;
  }
  }
  
+memory_region_transaction_commit();

+
  return 0;


... and return 'r' here?


  }


Otherwise LGTM.

[PATCH] ui/cocoa: user friendly characters for release mouse

[Christian Schoenebeck]

While mouse is grabbed, window title contains a hint for the user what
keyboard keys to press to release the mouse. Make that hint text a bit
more user friendly for a Mac user:

 - Replace "Ctrl" and "Alt" by appropriate symbols for those keyboard
   keys typically displayed for them on a Mac (encode those symbols by
   using UTF-8 characters).

 - Drop " + " in between the keys, as that's not common on macOS for
   documenting keyboard shortcuts.

 - Convert lower case "g" to upper case "G", as that's common on macOS.

 - Add one additional space at start and end of key stroke set, to
   visually separate the key strokes from the rest of the text.

Signed-off-by: Christian Schoenebeck 
---
 ui/cocoa.m | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/ui/cocoa.m b/ui/cocoa.m
index e915c344a8..289a2b193e 100644
--- a/ui/cocoa.m
+++ b/ui/cocoa.m
@@ -72,6 +72,9 @@
 
 #define cgrect(nsrect) (*(CGRect *)&(nsrect))
 
+#define UC_CTRL_KEY "\xe2\x8c\x83"
+#define UC_ALT_KEY "\xe2\x8c\xa5"
+
 typedef struct {
 int width;
 int height;
@@ -1135,9 +1138,9 @@ - (void) grabMouse
 
 if (!isFullscreen) {
 if (qemu_name)
-[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - 
(Press ctrl + alt + g to release Mouse)", qemu_name]];
+[normalWindow setTitle:[NSString stringWithFormat:@"QEMU %s - 
(Press  " UC_CTRL_KEY " " UC_ALT_KEY " G  to release Mouse)", qemu_name]];
 else
-[normalWindow setTitle:@"QEMU - (Press ctrl + alt + g to release 
Mouse)"];
+[normalWindow setTitle:@"QEMU - (Press  " UC_CTRL_KEY " " 
UC_ALT_KEY " G  to release Mouse)"];
 }
 [self hideCursor];
 CGAssociateMouseAndMouseCursorPosition(isAbsoluteEnabled);
-- 
2.32.0 (Apple Git-132)

[PATCH 1/2] linux-user/hexagon: fix signal context save & restore

[Mukilan Thiyagarajan]

This patch fixes the issue originally reported in
this thread:

https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html

The root cause of the issue is a bug in the hexagon specific
logic for saving & restoring context during signal delivery.
The CPU state has two different representations for the
predicate registers. The current logic saves & restores only
the aliased HEX_REG_P3_O register, which is part of env->gpr[]
field in the CPU state, but not the individual byte-level
predicate registers (pO, p1, p2, p3) backed by env->pred[].

Since all predicated instructions refer only to the
indiviual registers, switching to and back from a signal handler
can clobber these registers if the signal handler writes to them
causing the normal application code to behave unpredictably when
context is restored.

In the reported issue with the 'signals' test, since the updated
hexagon toolchain had built musl with -O2, the functions called
from non_trivial_free were inlined. This meant that the code
emitted reused predicate P0 computed in the entry translation
block of the function non_trivial_free in one of the child TB
as part of an assertion. Since P0 is clobbered by the signal
handler in the signals test, the assertion in non_trivial_free
fails incorectly. Since musl for hexagon implements the 'abort'
function by deliberately writing to memory via null pointer,
this causes the test to fail with segmentation fault.

This patch modifies the signal context save & restore logic
to include the individual p0, p1, p2, p3 and excludes the
32b p3_0 register since its value is derived from the former
registers. It also adds a new test case that reliabily
reproduces the issue for all four predicate registers.

Buglink: https://github.com/quic/toolchain_for_hexagon/issues/6
Signed-off-by: Mukilan Thiyagarajan 
---
 linux-user/hexagon/signal.c| 17 +++---
 tests/tcg/hexagon/Makefile.target  |  1 +
 tests/tcg/hexagon/signal_context.c | 84 ++
 3 files changed, 96 insertions(+), 6 deletions(-)
 create mode 100644 tests/tcg/hexagon/signal_context.c

diff --git a/linux-user/hexagon/signal.c b/linux-user/hexagon/signal.c
index ad4e3822d5..60fa7e1bce 100644
--- a/linux-user/hexagon/signal.c
+++ b/linux-user/hexagon/signal.c
@@ -39,15 +39,12 @@ struct target_sigcontext {
 target_ulong m0;
 target_ulong m1;
 target_ulong usr;
-target_ulong p3_0;
 target_ulong gp;
 target_ulong ugp;
 target_ulong pc;
 target_ulong cause;
 target_ulong badva;
-target_ulong pad1;
-target_ulong pad2;
-target_ulong pad3;
+target_ulong pred[NUM_PREGS];
 };
 
 struct target_ucontext {
@@ -118,10 +115,14 @@ static void setup_sigcontext(struct target_sigcontext 
*sc, CPUHexagonState *env)
 __put_user(env->gpr[HEX_REG_M0], >m0);
 __put_user(env->gpr[HEX_REG_M1], >m1);
 __put_user(env->gpr[HEX_REG_USR], >usr);
-__put_user(env->gpr[HEX_REG_P3_0], >p3_0);
 __put_user(env->gpr[HEX_REG_GP], >gp);
 __put_user(env->gpr[HEX_REG_UGP], >ugp);
 __put_user(env->gpr[HEX_REG_PC], >pc);
+
+int i;
+for (i = 0; i < NUM_PREGS; i++) {
+__put_user(env->pred[i], &(sc->pred[i]));
+}
 }
 
 static void setup_ucontext(struct target_ucontext *uc,
@@ -230,10 +231,14 @@ static void restore_sigcontext(CPUHexagonState *env,
 __get_user(env->gpr[HEX_REG_M0], >m0);
 __get_user(env->gpr[HEX_REG_M1], >m1);
 __get_user(env->gpr[HEX_REG_USR], >usr);
-__get_user(env->gpr[HEX_REG_P3_0], >p3_0);
 __get_user(env->gpr[HEX_REG_GP], >gp);
 __get_user(env->gpr[HEX_REG_UGP], >ugp);
 __get_user(env->gpr[HEX_REG_PC], >pc);
+
+int i;
+for (i = 0; i < NUM_PREGS; i++) {
+__get_user(env->pred[i], &(sc->pred[i]));
+}
 }
 
 static void restore_ucontext(CPUHexagonState *env, struct target_ucontext *uc)
diff --git a/tests/tcg/hexagon/Makefile.target 
b/tests/tcg/hexagon/Makefile.target
index 9ee1faa1e1..f1378d86a9 100644
--- a/tests/tcg/hexagon/Makefile.target
+++ b/tests/tcg/hexagon/Makefile.target
@@ -43,6 +43,7 @@ HEX_TESTS += load_align
 HEX_TESTS += atomics
 HEX_TESTS += fpstuff
 HEX_TESTS += overflow
+HEX_TESTS += signal_context
 
 HEX_TESTS += test_abs
 HEX_TESTS += test_bitcnt
diff --git a/tests/tcg/hexagon/signal_context.c 
b/tests/tcg/hexagon/signal_context.c
new file mode 100644
index 00..297e6915a4
--- /dev/null
+++ b/tests/tcg/hexagon/signal_context.c
@@ -0,0 +1,84 @@
+/*
+ *  Copyright(c) 2019-2022 Qualcomm Innovation Center, Inc. All Rights 
Reserved.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR 

[PATCH 2/2] target/hexagon: rename aliased register HEX_REG_P3_0

[Mukilan Thiyagarajan]

The patch renames the identifier of the 32bit register
HEX_REG_P3_0 to HEX_REG_P3_0_ALIASED.

This change is to intended to provide some warning that
HEX_REG_P3_0 is an aliased register which has multiple
representations in CPU state and therefore might require
special handling in some contexts. The hope is to prevent
accidental misuse of this register e.g the issue reported
for the signals tests failure [here][1].

[1]: https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html

Signed-off-by: Mukilan Thiyagarajan 
---
 target/hexagon/cpu.c  |  6 +++---
 target/hexagon/genptr.c   | 12 ++--
 target/hexagon/hex_regs.h |  2 +-
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/hexagon/cpu.c b/target/hexagon/cpu.c
index 658ca4ff78..807037c586 100644
--- a/target/hexagon/cpu.c
+++ b/target/hexagon/cpu.c
@@ -86,7 +86,7 @@ static target_ulong adjust_stack_ptrs(CPUHexagonState *env, 
target_ulong addr)
 return addr;
 }
 
-/* HEX_REG_P3_0 (aka C4) is an alias for the predicate registers */
+/* HEX_REG_P3_0_ALIASED (aka C4) is an alias for the predicate registers */
 static target_ulong read_p3_0(CPUHexagonState *env)
 {
 int32_t control_reg = 0;
@@ -102,7 +102,7 @@ static void print_reg(FILE *f, CPUHexagonState *env, int 
regnum)
 {
 target_ulong value;
 
-if (regnum == HEX_REG_P3_0) {
+if (regnum == HEX_REG_P3_0_ALIASED) {
 value = read_p3_0(env);
 } else {
 value = regnum < 32 ? adjust_stack_ptrs(env, env->gpr[regnum])
@@ -198,7 +198,7 @@ static void hexagon_dump(CPUHexagonState *env, FILE *f, int 
flags)
 print_reg(f, env, HEX_REG_M0);
 print_reg(f, env, HEX_REG_M1);
 print_reg(f, env, HEX_REG_USR);
-print_reg(f, env, HEX_REG_P3_0);
+print_reg(f, env, HEX_REG_P3_0_ALIASED);
 print_reg(f, env, HEX_REG_GP);
 print_reg(f, env, HEX_REG_UGP);
 print_reg(f, env, HEX_REG_PC);
diff --git a/target/hexagon/genptr.c b/target/hexagon/genptr.c
index 6cf2e0ed43..66a968c884 100644
--- a/target/hexagon/genptr.c
+++ b/target/hexagon/genptr.c
@@ -163,7 +163,7 @@ static inline void gen_read_p3_0(TCGv control_reg)
 
 /*
  * Certain control registers require special handling on read
- * HEX_REG_P3_0  aliased to the predicate registers
+ * HEX_REG_P3_0_ALIASED  aliased to the predicate registers
  *   -> concat the 4 predicate registers together
  * HEX_REG_PCactual value stored in DisasContext
  *   -> assign from ctx->base.pc_next
@@ -173,7 +173,7 @@ static inline void gen_read_p3_0(TCGv control_reg)
 static inline void gen_read_ctrl_reg(DisasContext *ctx, const int reg_num,
  TCGv dest)
 {
-if (reg_num == HEX_REG_P3_0) {
+if (reg_num == HEX_REG_P3_0_ALIASED) {
 gen_read_p3_0(dest);
 } else if (reg_num == HEX_REG_PC) {
 tcg_gen_movi_tl(dest, ctx->base.pc_next);
@@ -194,7 +194,7 @@ static inline void gen_read_ctrl_reg(DisasContext *ctx, 
const int reg_num,
 static inline void gen_read_ctrl_reg_pair(DisasContext *ctx, const int reg_num,
   TCGv_i64 dest)
 {
-if (reg_num == HEX_REG_P3_0) {
+if (reg_num == HEX_REG_P3_0_ALIASED) {
 TCGv p3_0 = tcg_temp_new();
 gen_read_p3_0(p3_0);
 tcg_gen_concat_i32_i64(dest, p3_0, hex_gpr[reg_num + 1]);
@@ -238,7 +238,7 @@ static void gen_write_p3_0(DisasContext *ctx, TCGv 
control_reg)
 
 /*
  * Certain control registers require special handling on write
- * HEX_REG_P3_0  aliased to the predicate registers
+ * HEX_REG_P3_0_ALIASED  aliased to the predicate registers
  *   -> break the value across 4 predicate registers
  * HEX_REG_QEMU_*_CNTchanges in current TB in DisasContext
  *-> clear the changes
@@ -246,7 +246,7 @@ static void gen_write_p3_0(DisasContext *ctx, TCGv 
control_reg)
 static inline void gen_write_ctrl_reg(DisasContext *ctx, int reg_num,
   TCGv val)
 {
-if (reg_num == HEX_REG_P3_0) {
+if (reg_num == HEX_REG_P3_0_ALIASED) {
 gen_write_p3_0(ctx, val);
 } else {
 gen_log_reg_write(reg_num, val);
@@ -266,7 +266,7 @@ static inline void gen_write_ctrl_reg(DisasContext *ctx, 
int reg_num,
 static inline void gen_write_ctrl_reg_pair(DisasContext *ctx, int reg_num,
TCGv_i64 val)
 {
-if (reg_num == HEX_REG_P3_0) {
+if (reg_num == HEX_REG_P3_0_ALIASED) {
 TCGv val32 = tcg_temp_new();
 tcg_gen_extrl_i64_i32(val32, val);
 gen_write_p3_0(ctx, val32);
diff --git a/target/hexagon/hex_regs.h b/target/hexagon/hex_regs.h
index a63c2c0fd5..bddfc28021 100644
--- a/target/hexagon/hex_regs.h
+++ b/target/hexagon/hex_regs.h
@@ -58,7 +58,7 @@ enum {
 HEX_REG_LC0  = 33,
 HEX_REG_SA1  = 34,
 HEX_REG_LC1  = 35,
-

[PATCH 0/2] Hexagon: fix signal context save & restore bug

[Mukilan Thiyagarajan]

This patchset is to fix the issue discovered in this thread
when hexagon toolchain was upgraded to a newer version.

https://lists.gnu.org/archive/html/qemu-devel/2021-11/msg01102.html

Investigation revealed that the bug was not in the
toolchain as suspected in the above thread, but in the
hexagon specific logic in linux-user for saving & restoring
thread context during signal delivery.

The first patch contains the fix for the issue along with a
test case and the second patch contains the change to rename
the enum for HEX_REG_P3_0 register, to provide some warning
to new code that it is an aliased version of the
individual predicate registers.

- Mukilan

Mukilan Thiyagarajan (2):
  linux-user/hexagon: fix signal context save & restore
  target/hexagon: rename aliased register HEX_REG_P3_0

 linux-user/hexagon/signal.c| 17 +++---
 target/hexagon/cpu.c   |  6 +--
 target/hexagon/genptr.c| 12 ++---
 target/hexagon/hex_regs.h  |  2 +-
 tests/tcg/hexagon/Makefile.target  |  1 +
 tests/tcg/hexagon/signal_context.c | 84 ++
 6 files changed, 106 insertions(+), 16 deletions(-)
 create mode 100644 tests/tcg/hexagon/signal_context.c

-- 
2.17.1

Re: [PATCH] target/i386/hax: Add XCR0 support

[Paolo Bonzini]

Il lun 28 nov 2022, 09:12 Wang, Wenchao  ha scritto:

> Hi, Philippe,
>
> It is just the full patch. Currently, the implementation of HAXM is
> simple, we did not synchronize the vCPU register for xcr0 from QEMU. HAXM
> will handle the xcr0 state within the kernel space, including
> initialization, update, etc. This patch adds the xcr0 variable for
> allocating extra 8-byte buffer occupation, which will be passed between
> QEMU and HAXM when hax_sync_vcpu_state() is invoked. We have verified the
> patched QEMU and it can launch all guest OSes. Thanks for your comments.
>

I don't understand the patch very well, and I am on the phone so it's hard
to check QEMU's HAXM support sources right now. Did HAXM 7.8.0 break
support for QEMU without this patch, and likewise will QEMU with this patch
will HAXM versions older than 7.8.0?

Or does this work on any version because QEMU treats the struct as a black
box?

Paolo



>
> Best Regards,
> Wenchao
>
> -Original Message-
> From: Philippe Mathieu-Daudé 
> Sent: Friday, November 25, 2022 21:37
> To: Wang, Wenchao ; qemu-devel@nongnu.org
> Cc: haxm-team ; Paolo Bonzini 
> Subject: Re: [PATCH] target/i386/hax: Add XCR0 support
>
> Hi,
>
> On 25/11/22 13:18, Wang, Wenchao wrote:
> > Hi, maintainers,
> >
> > As HAXM v7.8.0 is released and it added XCR0 support, could you help
> > to merge this patch to add corresponding support into HAX user space
> > of QEMU? The patch has been included in the attachment. Thanks.
>
> See
>
> https://www.qemu.org/docs/master/devel/submitting-a-patch.html#submitting-your-patches
> on how to send patches to a mailing list.
>
> >
> > Best Regards,
> >
> > Wenchao
> >
> >  From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00
> > 2001
> >
> > From: Wenchao Wang 
> >
> > Date: Fri, 25 Nov 2022 18:37:34 +0800
> >
> > Subject: [PATCH] target/i386/hax: Add XCR0 support
> >
> > Introduce extended control register XCR0 to support XSAVE feature set.
> >
> > Note: This change requires at least HAXM v7.8.0 to support.
> >
> > Reviewed-by: Hang Yuan 
> >
> > Signed-off-by: Wenchao Wang 
> >
> > ---
> >
> > target/i386/hax/hax-interface.h | 2 ++
> >
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/target/i386/hax/hax-interface.h
> > b/target/i386/hax/hax-interface.h
> >
> > index 537ae084e9..1d13bb2380 100644
> >
> > --- a/target/i386/hax/hax-interface.h
> >
> > +++ b/target/i386/hax/hax-interface.h
> >
> > @@ -201,6 +201,8 @@ struct vcpu_state_t {
> >
> >   uint64_t _cr3;
> >
> >   uint64_t _cr4;
> >
> > +uint64_t _xcr0;
> >
> > +
> >
> >   uint64_t _dr0;
> >
> >   uint64_t _dr1;
> >
> >   uint64_t _dr2;
> >
> > --
> >
> > 2.17.1
> >
>
> Is that the full patch? It is missing the register use in
> hax_sync_vcpu_register()...
>
> Regards,
>
> Phil.
>

Re: [PATCH 12/12] hw/riscv: spike: Decouple create_fdt() dependency to ELF loading

[Daniel Henrique Barboza]

Hi Bin,

On 12/27/22 03:48, Bin Meng wrote:

At present create_fdt() calls htif_uses_elf_symbols() to determine
whether to insert a  property for the HTIF. This unfortunately
creates a hidden dependency to riscv_load_{firmware,kernel} that
create_fdt() must be called after the ELF {firmware,kernel} image
has been loaded.

Decouple such dependency be adding a new parameter to create_fdt(),
whether custom HTIF base address is used. The flag will be set if
non ELF {firmware,kernel} image is given by user.

Signed-off-by: Bin Meng 

---

  include/hw/char/riscv_htif.h |  5 +---
  hw/char/riscv_htif.c | 17 +---
  hw/riscv/spike.c | 54 ++--
  3 files changed, 53 insertions(+), 23 deletions(-)

diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h
index 9e8ebbe017..5958c5b986 100644
--- a/include/hw/char/riscv_htif.h
+++ b/include/hw/char/riscv_htif.h
@@ -44,11 +44,8 @@ typedef struct HTIFState {
  void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value,
  uint64_t st_size);
  
-/* Check if HTIF uses ELF symbols */

-bool htif_uses_elf_symbols(void);
-
  /* legacy pre qom */
  HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
-uint64_t nonelf_base);
+uint64_t nonelf_base, bool custom_base);
  
  #endif

diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
index 1477fc0090..098de50e35 100644
--- a/hw/char/riscv_htif.c
+++ b/hw/char/riscv_htif.c
@@ -52,20 +52,17 @@
  #define PK_SYS_WRITE64
  
  static uint64_t fromhost_addr, tohost_addr;

-static int address_symbol_set;
  
  void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value,

uint64_t st_size)
  {
  if (strcmp("fromhost", st_name) == 0) {
-address_symbol_set |= 1;
  fromhost_addr = st_value;
  if (st_size != 8) {
  error_report("HTIF fromhost must be 8 bytes");
  exit(1);
  }
  } else if (strcmp("tohost", st_name) == 0) {
-address_symbol_set |= 2;
  tohost_addr = st_value;
  if (st_size != 8) {
  error_report("HTIF tohost must be 8 bytes");
@@ -275,19 +272,19 @@ static const MemoryRegionOps htif_mm_ops = {
  .write = htif_mm_write,
  };
  
-bool htif_uses_elf_symbols(void)

-{
-return (address_symbol_set == 3) ? true : false;
-}
-
  HTIFState *htif_mm_init(MemoryRegion *address_space, Chardev *chr,
-uint64_t nonelf_base)
+uint64_t nonelf_base, bool custom_base)
  {
  uint64_t base, size, tohost_offset, fromhost_offset;
  
-if (!htif_uses_elf_symbols()) {

+if (custom_base) {
  fromhost_addr = nonelf_base;
  tohost_addr = nonelf_base + 8;
+} else {
+if (!fromhost_addr || !tohost_addr) {
+error_report("Invalid HTIF fromhost or tohost address");
+exit(1);
+}
  }
  
  base = MIN(tohost_addr, fromhost_addr);

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 810a18f283..90f9e581e4 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -50,7 +50,8 @@ static const MemMapEntry spike_memmap[] = {
  };
  
  static void create_fdt(SpikeState *s, const MemMapEntry *memmap,

-   uint64_t mem_size, const char *cmdline, bool is_32_bit)
+   uint64_t mem_size, const char *cmdline,
+   bool is_32_bit, bool htif_custom_base)
  {
  void *fdt;
  uint64_t addr, size;
@@ -78,7 +79,7 @@ static void create_fdt(SpikeState *s, const MemMapEntry 
*memmap,
  
  qemu_fdt_add_subnode(fdt, "/htif");

  qemu_fdt_setprop_string(fdt, "/htif", "compatible", "ucb,htif0");
-if (!htif_uses_elf_symbols()) {
+if (htif_custom_base) {
  qemu_fdt_setprop_cells(fdt, "/htif", "reg",
  0x0, memmap[SPIKE_HTIF].base, 0x0, memmap[SPIKE_HTIF].size);
  }
@@ -184,6 +185,21 @@ static void create_fdt(SpikeState *s, const MemMapEntry 
*memmap,
  }
  }
  
+static bool spike_test_elf_image(char *filename)

+{
+Error *err = NULL;
+
+if (filename) {
+load_elf_hdr(filename, NULL, NULL, );
+if (err) {
+error_free(err);
+return false;
+}
+}
+
+return true;
+}
+
  static void spike_board_init(MachineState *machine)
  {
  const MemMapEntry *memmap = spike_memmap;
@@ -191,11 +207,12 @@ static void spike_board_init(MachineState *machine)
  MemoryRegion *system_memory = get_system_memory();
  MemoryRegion *mask_rom = g_new(MemoryRegion, 1);
  target_ulong firmware_end_addr, kernel_start_addr;
-const char *firmware_name;
+char *firmware_name;
  uint32_t fdt_load_addr;
  uint64_t kernel_entry;
  char *soc_name;
  int i, base_hartid, hart_count;
+bool htif_custom_base;
  
  /* Check socket count limit */

  if (SPIKE_SOCKETS_MAX < 

Re: [PATCH] m25p80: Add the is25wp256 SFPD table

[Tudor Ambarus]

On 25.12.2022 14:18, Ben Dooks wrote:

On Wed, Dec 21, 2022 at 06:36:02PM +0100, Cédric Le Goater wrote:

On 12/21/22 13:22, Guenter Roeck wrote:

Generated from hardware using the following command and then padding
with 0xff to fill out a power-of-2:
xxd -p /sys/bus/spi/devices/spi0.0/spi-nor/sfdp

Cc: Michael Walle 
Cc: Tudor Ambarus 
Signed-off-by: Guenter Roeck 


Reviewed-by: Cédric Le Goater 


If SFDP is a standard, couldn't we have an function to generate it from
the flash parameters?



No, it's not practical as you have to specify all the flash parameters
at flash declaration.

Re: [PATCH v5 4/4] x86: re-enable rng seeding via SetupData

[Jason A. Donenfeld]

On Mon, Dec 26, 2022 at 05:57:30PM +0100, Jason A. Donenfeld wrote:
> On Mon, Dec 26, 2022 at 03:43:04PM +0100, Jason A. Donenfeld wrote:
> > On Mon, Dec 26, 2022 at 03:24:07PM +0100, Jason A. Donenfeld wrote:
> > > Hi,
> > > 
> > > I'm currently stumped at the moment, so adding linux-mm@ and x86@. Still
> > > working on it though. Details of where I'm at are below the quote below.
> > > 
> > > On Sat, Dec 24, 2022 at 05:21:46AM +0100, Jason A. Donenfeld wrote:
> > > > On Sat, Dec 24, 2022 at 04:09:08AM +0100, Jason A. Donenfeld wrote:
> > > > > Hi Eric,
> > > > > 
> > > > > Replying to you from my telephone, and I'm traveling the next two 
> > > > > days,
> > > > > but I thought I should mention some preliminary results right away 
> > > > > from
> > > > > doing some termux compiles:
> > > > > 
> > > > > On Fri, Dec 23, 2022 at 04:14:00PM -0800, Eric Biggers wrote:
> > > > > > Hi Jason,
> > > > > > 
> > > > > > On Wed, Sep 21, 2022 at 11:31:34AM +0200, Jason A. Donenfeld wrote:
> > > > > > > This reverts 3824e25db1 ("x86: disable rng seeding via 
> > > > > > > setup_data"), but
> > > > > > > for 7.2 rather than 7.1, now that modifying setup_data is safe to 
> > > > > > > do.
> > > > > > > 
> > > > > > > Cc: Laurent Vivier 
> > > > > > > Cc: Michael S. Tsirkin 
> > > > > > > Cc: Paolo Bonzini 
> > > > > > > Cc: Peter Maydell 
> > > > > > > Cc: Philippe Mathieu-Daudé 
> > > > > > > Cc: Richard Henderson 
> > > > > > > Cc: Ard Biesheuvel 
> > > > > > > Acked-by: Gerd Hoffmann 
> > > > > > > Signed-off-by: Jason A. Donenfeld 
> > > > > > > ---
> > > > > > >  hw/i386/microvm.c | 2 +-
> > > > > > >  hw/i386/pc_piix.c | 3 ++-
> > > > > > >  hw/i386/pc_q35.c  | 3 ++-
> > > > > > >  3 files changed, 5 insertions(+), 3 deletions(-)
> > > > > > > 
> > > > > > 
> > > > > > After upgrading to QEMU 7.2, Linux 6.1 no longer boots with some 
> > > > > > configs.  There
> > > > > > is no output at all.  I bisected it to this commit, and I verified 
> > > > > > that the
> > > > > > following change to QEMU's master branch makes the problem go away:
> > > > > > 
> > > > > > diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> > > > > > index b48047f50c..42f5b07d2f 100644
> > > > > > --- a/hw/i386/pc_piix.c
> > > > > > +++ b/hw/i386/pc_piix.c
> > > > > > @@ -441,6 +441,7 @@ static void 
> > > > > > pc_i440fx_8_0_machine_options(MachineClass *m)
> > > > > >  pc_i440fx_machine_options(m);
> > > > > >  m->alias = "pc";
> > > > > >  m->is_default = true;
> > > > > > +PC_MACHINE_CLASS(m)->legacy_no_rng_seed = true;
> > > > > >  }
> > > > > > 
> > > > > > I've attached the kernel config I am seeing the problem on.
> > > > > > 
> > > > > > For some reason, the problem also goes away if I disable 
> > > > > > CONFIG_KASAN.
> > > > > > 
> > > > > > Any idea what is causing this?
> > > > > 
> > > > > - Commenting out the call to parse_setup_data() doesn't fix the issue.
> > > > >   So there's no KASAN issue with the actual parser.
> > > > > 
> > > > > - Using KASAN_OUTLINE rather than INLINE does fix the issue!
> > > > > 
> > > > > That makes me suspect that it's file size related, and QEMU or the 
> > > > > BIOS
> > > > > is placing setup data at an overlapping offset by accident, or 
> > > > > something
> > > > > similar.
> > > > 
> > > > I removed the file systems from your config to bring the kernel size
> > > > back down, and voila, it works, even with KASAN_INLINE. So perhaps I'm
> > > > on the right track here...
> > > 
> > > QEMU sticks setup_data after the kernel image, the same as kexec-tools
> > > and everything else. Apparently, when the kernel image is large, the
> > > call to early_memremap(boot_params.hdr.setup_data, ...) returns a value
> > > that points some place bogus, and the system crashes or does something
> > > weird. I haven't yet determined what this limit is, but in my current
> > > test kernel, a value of 0x01327650 is enough to make it point to
> > > rubbish.
> > > 
> > > Is this expected? What's going on here?
> > 
> > Attaching gdb to QEMU and switching it to physical memory mode
> > (`maintenance packet Qqemu.PhyMemMode:1 `) indicates that it
> > early_memremap is actually working fine and something *else* is at this
> > address? That's kinda weird... Is KASAN populating physical addresses
> > immediately after the kernel image extremely early in boot? I'm seeing
> > the crash happen from early_reserve_memory()->
> > memblock_x86_reserve_range_setup_data(), which should be before
> > kasan_init() even runs. Is QEMU calculating kernel_size wrong, when it
> > goes to determine where to put the setup_data data? But that's the same
> > calculation as used everywhere else, so hmm...
> > 
> > Jason
> 
> If bzImage is 15770544 bytes, it does not boot. If bzImage is 15641776
> bytes, it does boot. So something is happening somewhat close to the
> 16MB mark?
> 

Okay, the issue is that it's being decompressed to an area that overlaps
the source. So for example in my test kernel:

RE: [PATCH] target/i386/hax: Add XCR0 support

[Wang, Wenchao]

Hi, Philippe,

As nobody made any comment on this change, could you help to merge the patch 
with a generic target cleanup series? The patch has been attached below. Thanks 
in advance.


Best Regards,
Wenchao

-

From b1789f2523d06798b8883664bfa9a9df797bfccf Mon Sep 17 00:00:00 2001
From: Wenchao Wang 
Date: Fri, 25 Nov 2022 18:37:34 +0800
Subject: [PATCH] target/i386/hax: Add XCR0 support

Introduce extended control register XCR0 to support XSAVE feature set.

Note: This change requires at least HAXM v7.8.0 to support.

Reviewed-by: Hang Yuan 
Signed-off-by: Wenchao Wang 
---
target/i386/hax/hax-interface.h | 2 ++
1 file changed, 2 insertions(+)

diff --git a/target/i386/hax/hax-interface.h b/target/i386/hax/hax-interface.h 
index 537ae084e9..1d13bb2380 100644
--- a/target/i386/hax/hax-interface.h
+++ b/target/i386/hax/hax-interface.h
@@ -201,6 +201,8 @@ struct vcpu_state_t {
 uint64_t _cr3;
 uint64_t _cr4;

+uint64_t _xcr0;
+
 uint64_t _dr0;
 uint64_t _dr1;
 uint64_t _dr2;
--
2.17.1

-Original Message-
From: Philippe Mathieu-Daudé  
Sent: Monday, December 19, 2022 17:10
To: Wang, Wenchao ; qemu-devel@nongnu.org
Cc: Paolo Bonzini 
Subject: Re: [PATCH] target/i386/hax: Add XCR0 support

Hi Wenchao,

On 19/12/22 10:01, Wang, Wenchao wrote:
> Hi, Philippe,
> 
> As Paolo did not comment with this patch, as you used to think it looks 
> correct, could you help to merge this one-line patch as no one picked it up 
> so far? Thanks a lot.

I'm pretty sure Paolo is busy with KVM stuff and will take this patch when he 
switch to QEMU (it really is within his area). If he doesn't comment I'll take 
it with a generic target/ cleanup series next week.

> Best Regards,
> Wenchao
> 
> -Original Message-
> From: Wang, Wenchao
> Sent: Monday, December 5, 2022 17:10
> To: Philippe Mathieu-Daudé ; qemu-devel@nongnu.org
> Cc: haxm-team ; Paolo Bonzini 
> 
> Subject: RE: [PATCH] target/i386/hax: Add XCR0 support
> 
> Thanks for Phillippe's reply.
> 
> Hi, Paolo,
> 
> Could you help to review the patch of HAX? If there is any concern about it, 
> feel free to discuss with me. Thanks a lot.
> 
> 
> Best Regards,
> Wenchao
> 
> -Original Message-
> From: Philippe Mathieu-Daudé 
> Sent: Monday, December 5, 2022 17:05
> To: Wang, Wenchao ; qemu-devel@nongnu.org
> Cc: haxm-team ; Paolo Bonzini 
> 
> Subject: Re: [PATCH] target/i386/hax: Add XCR0 support
> 
> Hi Wenchao,
> 
> On 5/12/22 09:35, Wang, Wenchao wrote:
>> Hi, Philippe,
>>
>> Do you agree with my opinion and is there any further process that I need to 
>> follow to get this patch merged? Thanks a lot.
> 
> I don't understand this part of HAXM enough, but per your explanation, 
> your change looks correct. I'll let Paolo decide :)
> 
> Regards,
> 
> Phil.
> 
>> Best Regards,
>> Wenchao
>>
>> -Original Message-
>> From: Wang, Wenchao
>> Sent: Monday, November 28, 2022 16:11
>> To: Philippe Mathieu-Daudé ; qemu-devel@nongnu.org
>> Cc: haxm-team ; Paolo Bonzini 
>> 
>> Subject: RE: [PATCH] target/i386/hax: Add XCR0 support
>>
>> Hi, Philippe,
>>
>> It is just the full patch. Currently, the implementation of HAXM is simple, 
>> we did not synchronize the vCPU register for xcr0 from QEMU. HAXM will 
>> handle the xcr0 state within the kernel space, including initialization, 
>> update, etc. This patch adds the xcr0 variable for allocating extra 8-byte 
>> buffer occupation, which will be passed between QEMU and HAXM when 
>> hax_sync_vcpu_state() is invoked. We have verified the patched QEMU and it 
>> can launch all guest OSes. Thanks for your comments.
>>
>>
>> Best Regards,
>> Wenchao

回复: Re: [RESEND PATCH] virtio-pci: fix vector_irqfd leak in virtio_pci_set_guest_notifiers

[雷翔]

As I found it introduced from  commit: 7d37d351dffee60fc7048bbfd8573421f15eb724
From the code you look at, g_free should belongs to the lable  config_assign_error.  this code patch has been revert, latest code lable is 'assign_error'  and we tested, it's also happend in 6.2.0. but in 7.1.0 or 7.2.0, new assert crash happend before it goes into virtio_pci_set_guest_notifiers. we can't reproduce the problem before fix the new crash problem.
crash info:>2022-12-23T02:48:32Z qemu-system-aarch64: virtio-scsi: Failed to set guest notifiers (-11), ensure -accel kvm is set.>2022-12-23T02:48:32Z qemu-system-aarch64: virtio_bus_start_ioeventfd: failed. Fallback to userspace (slower).>qemu-system-aarch64: ../hw/scsi/virtio-scsi.c:832: virtio_scsi_reset: Assertion `!s->dataplane_started' failed.>2022-12-23 02:48:53.235+: shutting down, reason=crashed

 

主　题：Re: [RESEND PATCH] virtio-pci: fix vector_irqfd leak in virtio_pci_set_guest_notifiers 日　期：2022-12-20 22:42 发件人：Michael S. Tsirkin 收件人：雷翔;



On Wed, Nov 30, 2022 at 01:56:11PM +0800, leixiang wrote:> proxy->vector_irqfd did not free when set guest notifier failed.Can you pls add a Fixes tag so people know where to backport this?> Signed-off-by: Lei Xiang > Tested-by: Zeng Chi > Suggested-by: Xie Ming Looking at the code I see this:/* Must set vector notifier after guest notifier has been assigned */if ((with_irqfd ||(vdev->use_guest_notifier_mask && k->guest_notifier_mask)) &&assign) {if (with_irqfd) {proxy->vector_irqfd =g_malloc0(sizeof(*proxy->vector_irqfd) *msix_nr_vectors_allocated(>pci_dev));r = kvm_virtio_pci_vector_vq_use(proxy, nvqs);if (r goto config_assign_error;}r = kvm_virtio_pci_vector_config_use(proxy);if (r goto config_error;}}r = msix_set_vector_notifiers(>pci_dev, virtio_pci_vector_unmask,virtio_pci_vector_mask,virtio_pci_vector_poll);if (r goto notifiers_error;}}doesn't this mean g_free belongs at the label config_assign_error?> ---> hw/virtio/virtio-pci.c | 6 ++> 1 file changed, 6 insertions(+)> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c> index c6b47a9c..4862f83b 100644> --- a/hw/virtio/virtio-pci.c> +++ b/hw/virtio/virtio-pci.c> @@ -1038,6 +1038,12 @@ assign_error:> while (--n >= 0) {> virtio_pci_set_guest_notifier(d, n, !assign, with_irqfd);> }> +> + g_free(proxy->vector_irqfd);> + proxy->vector_irqfd = NULL;> +> return r;> }> > -- > > > No virus found> Checked by Hillstone Network AntiVirusThe patch is corrupted. Line counts are wrong, and your antivirus addedtrash at the end.-- MST

[RESEND PATCH] virtio-pci: fix proxy->vector_irqfd leak in virtio_pci_set_guest_notifiers

[leixiang]

proxy->vector_irqfd did not free when kvm_virtio_pci_vector_use or
msix_set_vector_notifiers failed in virtio_pci_set_guest_notifiers.

Fixes: 7d37d351

Signed-off-by: Lei Xiang 
Tested-by: Zeng Chi 
Suggested-by: Xie Ming 
---
 hw/virtio/virtio-pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a50c5a57d7..5339be4e01 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1063,6 +1063,8 @@ assign_error:
 while (--n >= 0) {
 virtio_pci_set_guest_notifier(d, n, !assign, with_irqfd);
 }
+g_free(proxy->vector_irqfd);
+proxy->vector_irqfd = NULL;
 return r;
 }
 
-- 
2.25.1