[PATCH v6 24/25] target/riscv: Reorg access check in get_physical_address

[Richard Henderson]

We were effectively computing the protection bits twice,
once while performing access checks and once while returning
the valid bits to the caller.  Reorg so we do this once.

Move the computation of mxr close to its single use.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 69 ---
 1 file changed, 36 insertions(+), 33 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 82a7c5f9dd..725ca45106 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -762,7 +762,7 @@ static int get_physical_address_pmp(CPURISCVState *env, int 
*prot,
  * @is_debug: Is this access from a debugger or the monitor?
  */
 static int get_physical_address(CPURISCVState *env, hwaddr *physical,
-int *prot, target_ulong addr,
+int *ret_prot, target_ulong addr,
 target_ulong *fault_pte_addr,
 int access_type, int mmu_idx,
 bool first_stage, bool two_stage,
@@ -793,20 +793,14 @@ static int get_physical_address(CPURISCVState *env, 
hwaddr *physical,
 
 if (mode == PRV_M || !riscv_cpu_cfg(env)->mmu) {
 *physical = addr;
-*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+*ret_prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TRANSLATE_SUCCESS;
 }
 
-*prot = 0;
+*ret_prot = 0;
 
 hwaddr base;
-int levels, ptidxbits, ptesize, vm, sum, mxr, widened;
-
-if (first_stage == true) {
-mxr = get_field(env->mstatus, MSTATUS_MXR);
-} else {
-mxr = get_field(env->vsstatus, MSTATUS_MXR);
-}
+int levels, ptidxbits, ptesize, vm, sum, widened;
 
 if (first_stage == true) {
 if (use_background) {
@@ -849,7 +843,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
   levels = 5; ptidxbits = 9; ptesize = 8; break;
 case VM_1_10_MBARE:
 *physical = addr;
-*prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+*ret_prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
 return TRANSLATE_SUCCESS;
 default:
   g_assert_not_reached();
@@ -984,6 +978,27 @@ restart:
 return TRANSLATE_FAIL;
 }
 
+int prot = 0;
+if (pte & PTE_R) {
+prot |= PAGE_READ;
+}
+if (pte & PTE_W) {
+prot |= PAGE_WRITE;
+}
+if (pte & PTE_X) {
+bool mxr;
+
+if (first_stage == true) {
+mxr = get_field(env->mstatus, MSTATUS_MXR);
+} else {
+mxr = get_field(env->vsstatus, MSTATUS_MXR);
+}
+if (mxr) {
+prot |= PAGE_READ;
+}
+prot |= PAGE_EXEC;
+}
+
 if ((pte & PTE_U) &&
 ((mode != PRV_U) && (!sum || access_type == MMU_INST_FETCH))) {
 /*
@@ -996,17 +1011,9 @@ restart:
 /* Supervisor PTE flags when not S mode */
 return TRANSLATE_FAIL;
 }
-if (access_type == MMU_DATA_LOAD &&
-!((pte & PTE_R) || ((pte & PTE_X) && mxr))) {
-/* Read access check failed */
-return TRANSLATE_FAIL;
-}
-if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
-/* Write access check failed */
-return TRANSLATE_FAIL;
-}
-if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
-/* Fetch access check failed */
+
+if (!((prot >> access_type) & 1)) {
+/* Access check failed */
 return TRANSLATE_FAIL;
 }
 
@@ -1071,20 +1078,16 @@ restart:
   (vpn & (((target_ulong)1 << ptshift) - 1))
  ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
 
-/* set permissions on the TLB entry */
-if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
-*prot |= PAGE_READ;
-}
-if (pte & PTE_X) {
-*prot |= PAGE_EXEC;
-}
 /*
- * Add write permission on stores or if the page is already dirty,
- * so that we TLB miss on later writes to update the dirty bit.
+ * Remove write permission unless this is a store, or the page is
+ * already dirty, so that we TLB miss on later writes to update
+ * the dirty bit.
  */
-if ((pte & PTE_W) && (access_type == MMU_DATA_STORE || (pte & PTE_D))) {
-*prot |= PAGE_WRITE;
+if (access_type != MMU_DATA_STORE && !(pte & PTE_D)) {
+prot &= ~PAGE_WRITE;
 }
+*ret_prot = prot;
+
 return TRANSLATE_SUCCESS;
 }
 
-- 
2.34.1

[PATCH v6 08/25] accel/tcg: Add cpu_ld*_code_mmu

[Richard Henderson]

At least RISC-V has the need to be able to perform a read
using execute permissions, outside of translation.
Add helpers to facilitate this.

Signed-off-by: Richard Henderson 
---
 include/exec/cpu_ldst.h |  9 +++
 accel/tcg/cputlb.c  | 48 ++
 accel/tcg/user-exec.c   | 58 +
 3 files changed, 115 insertions(+)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index 09b55cc0ee..c141f0394f 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -445,6 +445,15 @@ static inline CPUTLBEntry *tlb_entry(CPUArchState *env, 
uintptr_t mmu_idx,
 # define cpu_stq_mmu  cpu_stq_le_mmu
 #endif
 
+uint8_t cpu_ldb_code_mmu(CPUArchState *env, abi_ptr addr,
+ MemOpIdx oi, uintptr_t ra);
+uint16_t cpu_ldw_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t ra);
+uint32_t cpu_ldl_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t ra);
+uint64_t cpu_ldq_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t ra);
+
 uint32_t cpu_ldub_code(CPUArchState *env, abi_ptr addr);
 uint32_t cpu_lduw_code(CPUArchState *env, abi_ptr addr);
 uint32_t cpu_ldl_code(CPUArchState *env, abi_ptr addr);
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index e984a98dc4..e62c8f3c3f 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -2768,3 +2768,51 @@ uint64_t cpu_ldq_code(CPUArchState *env, abi_ptr addr)
 MemOpIdx oi = make_memop_idx(MO_TEUQ, cpu_mmu_index(env, true));
 return full_ldq_code(env, addr, oi, 0);
 }
+
+uint8_t cpu_ldb_code_mmu(CPUArchState *env, abi_ptr addr,
+ MemOpIdx oi, uintptr_t retaddr)
+{
+return full_ldub_code(env, addr, oi, retaddr);
+}
+
+uint16_t cpu_ldw_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t retaddr)
+{
+MemOp mop = get_memop(oi);
+int idx = get_mmuidx(oi);
+uint16_t ret;
+
+ret = full_lduw_code(env, addr, make_memop_idx(MO_TEUW, idx), retaddr);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap16(ret);
+}
+return ret;
+}
+
+uint32_t cpu_ldl_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t retaddr)
+{
+MemOp mop = get_memop(oi);
+int idx = get_mmuidx(oi);
+uint32_t ret;
+
+ret = full_ldl_code(env, addr, make_memop_idx(MO_TEUL, idx), retaddr);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap32(ret);
+}
+return ret;
+}
+
+uint64_t cpu_ldq_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t retaddr)
+{
+MemOp mop = get_memop(oi);
+int idx = get_mmuidx(oi);
+uint64_t ret;
+
+ret = full_ldq_code(env, addr, make_memop_idx(MO_TEUQ, idx), retaddr);
+if ((mop & MO_BSWAP) != MO_TE) {
+ret = bswap64(ret);
+}
+return ret;
+}
diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 7b37fd229e..44e0ea55ba 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -1222,6 +1222,64 @@ uint64_t cpu_ldq_code(CPUArchState *env, abi_ptr ptr)
 return ret;
 }
 
+uint8_t cpu_ldb_code_mmu(CPUArchState *env, abi_ptr addr,
+ MemOpIdx oi, uintptr_t ra)
+{
+void *haddr;
+uint8_t ret;
+
+haddr = cpu_mmu_lookup(env, addr, oi, ra, MMU_INST_FETCH);
+ret = ldub_p(haddr);
+clear_helper_retaddr();
+return ret;
+}
+
+uint16_t cpu_ldw_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t ra)
+{
+void *haddr;
+uint16_t ret;
+
+haddr = cpu_mmu_lookup(env, addr, oi, ra, MMU_INST_FETCH);
+ret = lduw_p(haddr);
+clear_helper_retaddr();
+if (get_memop(oi) & MO_BSWAP) {
+ret = bswap16(ret);
+}
+return ret;
+}
+
+uint32_t cpu_ldl_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t ra)
+{
+void *haddr;
+uint32_t ret;
+
+haddr = cpu_mmu_lookup(env, addr, oi, ra, MMU_INST_FETCH);
+ret = ldl_p(haddr);
+clear_helper_retaddr();
+if (get_memop(oi) & MO_BSWAP) {
+ret = bswap32(ret);
+}
+return ret;
+}
+
+uint64_t cpu_ldq_code_mmu(CPUArchState *env, abi_ptr addr,
+  MemOpIdx oi, uintptr_t ra)
+{
+void *haddr;
+uint64_t ret;
+
+validate_memop(oi, MO_BEUQ);
+haddr = cpu_mmu_lookup(env, addr, oi, ra, MMU_DATA_LOAD);
+ret = ldq_p(haddr);
+clear_helper_retaddr();
+if (get_memop(oi) & MO_BSWAP) {
+ret = bswap64(ret);
+}
+return ret;
+}
+
 #include "ldst_common.c.inc"
 
 /*
-- 
2.34.1

[PATCH v6 05/25] target/riscv: Add a tb flags field for vstart

[Richard Henderson]

From: LIU Zhiwei 

Once we mistook the vstart directly from the env->vstart. As env->vstart is not
a constant, we should record it in the tb flags if we want to use
it in translation.

Reported-by: Richard Henderson 
Reviewed-by: Richard Henderson 
Signed-off-by: LIU Zhiwei 
Reviewed-by: Weiwei Li 
Message-Id: <20230324143031.1093-5-zhiwei_...@linux.alibaba.com>
---
 target/riscv/cpu.h  |  1 +
 target/riscv/cpu_helper.c   |  1 +
 target/riscv/translate.c|  4 ++--
 target/riscv/insn_trans/trans_rvv.c.inc | 14 +++---
 4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d9e0eaaf9b..86a82e25dc 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -657,6 +657,7 @@ FIELD(TB_FLAGS, VMA, 21, 1)
 FIELD(TB_FLAGS, ITRIGGER, 22, 1)
 /* Virtual mode enabled */
 FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
+FIELD(TB_FLAGS, VSTART_EQ_ZERO, 24, 1)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 4fdd6fe021..4f0999d50b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -74,6 +74,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 FIELD_EX64(env->vtype, VTYPE, VTA));
 flags = FIELD_DP32(flags, TB_FLAGS, VMA,
 FIELD_EX64(env->vtype, VTYPE, VMA));
+flags = FIELD_DP32(flags, TB_FLAGS, VSTART_EQ_ZERO, env->vstart == 0);
 } else {
 flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
 }
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 74d0b9889d..f8c077525c 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -97,7 +97,7 @@ typedef struct DisasContext {
 uint8_t vta;
 uint8_t vma;
 bool cfg_vta_all_1s;
-target_ulong vstart;
+bool vstart_eq_zero;
 bool vl_eq_vlmax;
 CPUState *cs;
 TCGv zero;
@@ -1155,7 +1155,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cs)
 ctx->vta = FIELD_EX32(tb_flags, TB_FLAGS, VTA) && cpu->cfg.rvv_ta_all_1s;
 ctx->vma = FIELD_EX32(tb_flags, TB_FLAGS, VMA) && cpu->cfg.rvv_ma_all_1s;
 ctx->cfg_vta_all_1s = cpu->cfg.rvv_ta_all_1s;
-ctx->vstart = env->vstart;
+ctx->vstart_eq_zero = FIELD_EX32(tb_flags, TB_FLAGS, VSTART_EQ_ZERO);
 ctx->vl_eq_vlmax = FIELD_EX32(tb_flags, TB_FLAGS, VL_EQ_VLMAX);
 ctx->misa_mxl_max = env->misa_mxl_max;
 ctx->xl = FIELD_EX32(tb_flags, TB_FLAGS, XL);
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc 
b/target/riscv/insn_trans/trans_rvv.c.inc
index 6297c3b50d..32b3b9a8e5 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -547,7 +547,7 @@ static bool vext_check_sds(DisasContext *s, int vd, int 
vs1, int vs2, int vm)
  */
 static bool vext_check_reduction(DisasContext *s, int vs2)
 {
-return require_align(vs2, s->lmul) && (s->vstart == 0);
+return require_align(vs2, s->lmul) && s->vstart_eq_zero;
 }
 
 /*
@@ -3083,7 +3083,7 @@ static bool trans_vcpop_m(DisasContext *s, arg_rmr *a)
 {
 if (require_rvv(s) &&
 vext_check_isa_ill(s) &&
-s->vstart == 0) {
+s->vstart_eq_zero) {
 TCGv_ptr src2, mask;
 TCGv dst;
 TCGv_i32 desc;
@@ -3112,7 +3112,7 @@ static bool trans_vfirst_m(DisasContext *s, arg_rmr *a)
 {
 if (require_rvv(s) &&
 vext_check_isa_ill(s) &&
-s->vstart == 0) {
+s->vstart_eq_zero) {
 TCGv_ptr src2, mask;
 TCGv dst;
 TCGv_i32 desc;
@@ -3146,7 +3146,7 @@ static bool trans_##NAME(DisasContext *s, arg_rmr *a) 
 \
 vext_check_isa_ill(s) &&   \
 require_vm(a->vm, a->rd) &&\
 (a->rd != a->rs2) &&   \
-(s->vstart == 0)) {\
+s->vstart_eq_zero) {   \
 uint32_t data = 0; \
 gen_helper_gvec_3_ptr *fn = gen_helper_##NAME; \
 TCGLabel *over = gen_new_label();  \
@@ -3187,7 +3187,7 @@ static bool trans_viota_m(DisasContext *s, arg_viota_m *a)
 !is_overlapped(a->rd, 1 << MAX(s->lmul, 0), a->rs2, 1) &&
 require_vm(a->vm, a->rd) &&
 require_align(a->rd, s->lmul) &&
-(s->vstart == 0)) {
+s->vstart_eq_zero) {
 uint32_t data = 0;
 TCGLabel *over = gen_new_label();
 tcg_gen_brcondi_tl(TCG_COND_EQ, cpu_vl, 0, over);
@@ -3636,7 +3636,7 @@ static bool vcompress_vm_check(DisasContext *s, arg_r *a)
require_align(a->rs2, s->lmul) &&
(a->rd != a->rs2) &&
!is_overlapped(a->rd, 1 << MAX(s->lmul, 0), a->rs1, 1) &&
-   (s->vstart == 0);
+   s->vstart_eq_zero;
 }

[PATCH v6 10/25] target/riscv: Handle HLV, HSV via helpers

[Richard Henderson]

Implement these instructions via helpers, in expectation
of determining the mmu_idx to use at runtime.  This allows
the permission check to also be moved out of line, which
allows HLSX to be removed from TB_FLAGS.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu.h  |   6 +-
 target/riscv/helper.h   |  12 ++-
 target/riscv/cpu_helper.c   |  26 ++---
 target/riscv/op_helper.c|  99 +++--
 target/riscv/translate.c|   2 -
 target/riscv/insn_trans/trans_rvh.c.inc | 135 ++--
 6 files changed, 169 insertions(+), 111 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 5e589db106..f03ff1f10c 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -641,8 +641,7 @@ FIELD(TB_FLAGS, LMUL, 7, 3)
 FIELD(TB_FLAGS, SEW, 10, 3)
 FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
 FIELD(TB_FLAGS, VILL, 14, 1)
-/* Is a Hypervisor instruction load/store allowed? */
-FIELD(TB_FLAGS, HLSX, 15, 1)
+FIELD(TB_FLAGS, VSTART_EQ_ZERO, 15, 1)
 /* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
 FIELD(TB_FLAGS, XL, 16, 2)
 /* If PointerMasking should be applied */
@@ -654,8 +653,7 @@ FIELD(TB_FLAGS, VMA, 21, 1)
 FIELD(TB_FLAGS, ITRIGGER, 22, 1)
 /* Virtual mode enabled */
 FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
-FIELD(TB_FLAGS, VSTART_EQ_ZERO, 24, 1)
-FIELD(TB_FLAGS, PRIV, 25, 2)
+FIELD(TB_FLAGS, PRIV, 24, 2)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/helper.h b/target/riscv/helper.h
index 37b54e0991..be60bd1525 100644
--- a/target/riscv/helper.h
+++ b/target/riscv/helper.h
@@ -123,8 +123,16 @@ DEF_HELPER_1(itrigger_match, void, env)
 #ifndef CONFIG_USER_ONLY
 DEF_HELPER_1(hyp_tlb_flush, void, env)
 DEF_HELPER_1(hyp_gvma_tlb_flush, void, env)
-DEF_HELPER_2(hyp_hlvx_hu, tl, env, tl)
-DEF_HELPER_2(hyp_hlvx_wu, tl, env, tl)
+DEF_HELPER_FLAGS_2(hyp_hlv_bu, TCG_CALL_NO_WG, tl, env, tl)
+DEF_HELPER_FLAGS_2(hyp_hlv_hu, TCG_CALL_NO_WG, tl, env, tl)
+DEF_HELPER_FLAGS_2(hyp_hlv_wu, TCG_CALL_NO_WG, tl, env, tl)
+DEF_HELPER_FLAGS_2(hyp_hlv_d, TCG_CALL_NO_WG, tl, env, tl)
+DEF_HELPER_FLAGS_2(hyp_hlvx_hu, TCG_CALL_NO_WG, tl, env, tl)
+DEF_HELPER_FLAGS_2(hyp_hlvx_wu, TCG_CALL_NO_WG, tl, env, tl)
+DEF_HELPER_FLAGS_3(hyp_hsv_b, TCG_CALL_NO_WG, void, env, tl, tl)
+DEF_HELPER_FLAGS_3(hyp_hsv_h, TCG_CALL_NO_WG, void, env, tl, tl)
+DEF_HELPER_FLAGS_3(hyp_hsv_w, TCG_CALL_NO_WG, void, env, tl, tl)
+DEF_HELPER_FLAGS_3(hyp_hsv_d, TCG_CALL_NO_WG, void, env, tl, tl)
 #endif
 
 /* Vector functions */
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 052fdd2d9d..9bb84be4e1 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -102,24 +102,16 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, 
target_ulong *pc,
 fs = get_field(env->mstatus, MSTATUS_FS);
 vs = get_field(env->mstatus, MSTATUS_VS);
 
-if (riscv_has_ext(env, RVH)) {
-if (env->priv == PRV_M ||
-(env->priv == PRV_S && !riscv_cpu_virt_enabled(env)) ||
-(env->priv == PRV_U && !riscv_cpu_virt_enabled(env) &&
-get_field(env->hstatus, HSTATUS_HU))) {
-flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
-}
-
-if (riscv_cpu_virt_enabled(env)) {
-flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED, 1);
-/*
- * Merge DISABLED and !DIRTY states using MIN.
- * We will set both fields when dirtying.
- */
-fs = MIN(fs, get_field(env->mstatus_hs, MSTATUS_FS));
-vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
-}
+if (riscv_cpu_virt_enabled(env)) {
+flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED, 1);
+/*
+ * Merge DISABLED and !DIRTY states using MIN.
+ * We will set both fields when dirtying.
+ */
+fs = MIN(fs, get_field(env->mstatus_hs, MSTATUS_FS));
+vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
 }
+
 if (cpu->cfg.debug && !icount_enabled()) {
 flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
 }
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index b2169a99ff..0f81645adf 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -427,6 +427,91 @@ void helper_hyp_gvma_tlb_flush(CPURISCVState *env)
 helper_hyp_tlb_flush(env);
 }
 
+static int check_access_hlsv(CPURISCVState *env, bool x, uintptr_t ra)
+{
+if (env->priv == PRV_M) {
+/* always allowed */
+} else if (riscv_cpu_virt_enabled(env)) {
+riscv_raise_exception(env, RISCV_EXCP_VIRT_INSTRUCTION_FAULT, ra);
+} else if (env->priv == PRV_U && !get_field(env->hstatus, HSTATUS_HU)) {
+riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
+}
+
+return cpu_mmu_index(env, x) | MMU_HYP_ACCESS_BIT;
+}
+
+target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulo

[PATCH v6 13/25] target/riscv: Introduce mmuidx_priv

[Richard Henderson]

Use the priv level encoded into the mmu_idx, rather than
starting from env->priv.  We have already checked MPRV+MPP
in riscv_cpu_mmu_index -- no need to repeat that.

Signed-off-by: Richard Henderson 
---
 target/riscv/internals.h  | 9 +
 target/riscv/cpu_helper.c | 6 +-
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 0b61f337dd..4aa1cb409f 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -37,6 +37,15 @@
 #define MMUIdx_M3
 #define MMU_2STAGE_BIT  (1 << 2)
 
+static inline int mmuidx_priv(int mmu_idx)
+{
+int ret = mmu_idx & 3;
+if (ret == MMUIdx_S_SUM) {
+ret = PRV_S;
+}
+return ret;
+}
+
 static inline bool mmuidx_sum(int mmu_idx)
 {
 return (mmu_idx & 3) == MMUIdx_S_SUM;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 7e6cd8e0fd..cb260b88ea 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -771,7 +771,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
  * (riscv_cpu_do_interrupt) is correct */
 MemTxResult res;
 MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
-int mode = env->priv;
+int mode = mmuidx_priv(mmu_idx);
 bool use_background = false;
 hwaddr ppn;
 RISCVCPU *cpu = env_archcpu(env);
@@ -793,10 +793,6 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
instructions, HLV, HLVX, and HSV. */
 if (riscv_cpu_two_stage_lookup(mmu_idx)) {
 mode = get_field(env->hstatus, HSTATUS_SPVP);
-} else if (mode == PRV_M && access_type != MMU_INST_FETCH) {
-if (get_field(env->mstatus, MSTATUS_MPRV)) {
-mode = get_field(env->mstatus, MSTATUS_MPP);
-}
 }
 
 if (first_stage == false) {
-- 
2.34.1

[PATCH v6 14/25] target/riscv: Introduce mmuidx_2stage

[Richard Henderson]

Move and rename riscv_cpu_two_stage_lookup, to match
the other mmuidx_* functions.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu.h|  1 -
 target/riscv/internals.h  |  5 +
 target/riscv/cpu_helper.c | 17 ++---
 3 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f03ff1f10c..b6bcfb3834 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -586,7 +586,6 @@ void riscv_cpu_set_geilen(CPURISCVState *env, target_ulong 
geilen);
 bool riscv_cpu_vector_enabled(CPURISCVState *env);
 bool riscv_cpu_virt_enabled(CPURISCVState *env);
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
-bool riscv_cpu_two_stage_lookup(int mmu_idx);
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
 G_NORETURN void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
MMUAccessType access_type, int 
mmu_idx,
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 4aa1cb409f..b5f823c7ec 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -51,6 +51,11 @@ static inline bool mmuidx_sum(int mmu_idx)
 return (mmu_idx & 3) == MMUIdx_S_SUM;
 }
 
+static inline bool mmuidx_2stage(int mmu_idx)
+{
+return mmu_idx & MMU_2STAGE_BIT;
+}
+
 /* share data between vector helpers and decode code */
 FIELD(VDATA, VM, 0, 1)
 FIELD(VDATA, LMUL, 1, 3)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index cb260b88ea..8a124888cd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -603,11 +603,6 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
enable)
 }
 }
 
-bool riscv_cpu_two_stage_lookup(int mmu_idx)
-{
-return mmu_idx & MMU_2STAGE_BIT;
-}
-
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
 {
 CPURISCVState *env = &cpu->env;
@@ -791,7 +786,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 
 /* MPRV does not affect the virtual-machine load/store
instructions, HLV, HLVX, and HSV. */
-if (riscv_cpu_two_stage_lookup(mmu_idx)) {
+if (mmuidx_2stage(mmu_idx)) {
 mode = get_field(env->hstatus, HSTATUS_SPVP);
 }
 
@@ -1177,7 +1172,7 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr 
physaddr,
 
 env->badaddr = addr;
 env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
-riscv_cpu_two_stage_lookup(mmu_idx);
+mmuidx_2stage(mmu_idx);
 env->two_stage_indirect_lookup = false;
 cpu_loop_exit_restore(cs, retaddr);
 }
@@ -1203,7 +1198,7 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr 
addr,
 }
 env->badaddr = addr;
 env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
-riscv_cpu_two_stage_lookup(mmu_idx);
+mmuidx_2stage(mmu_idx);
 env->two_stage_indirect_lookup = false;
 cpu_loop_exit_restore(cs, retaddr);
 }
@@ -1255,7 +1250,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 
 /* MPRV does not affect the virtual-machine load/store
instructions, HLV, HLVX, and HSV. */
-if (riscv_cpu_two_stage_lookup(mmu_idx)) {
+if (mmuidx_2stage(mmu_idx)) {
 mode = get_field(env->hstatus, HSTATUS_SPVP);
 } else if (mode == PRV_M && access_type != MMU_INST_FETCH &&
get_field(env->mstatus, MSTATUS_MPRV)) {
@@ -1267,7 +1262,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 
 pmu_tlb_fill_incr_ctr(cpu, access_type);
 if (riscv_cpu_virt_enabled(env) ||
-((riscv_cpu_two_stage_lookup(mmu_idx) || two_stage_lookup) &&
+((mmuidx_2stage(mmu_idx) || two_stage_lookup) &&
  access_type != MMU_INST_FETCH)) {
 /* Two stage lookup */
 ret = get_physical_address(env, &pa, &prot, address,
@@ -1365,7 +1360,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 raise_mmu_exception(env, address, access_type, pmp_violation,
 first_stage_error,
 riscv_cpu_virt_enabled(env) ||
-riscv_cpu_two_stage_lookup(mmu_idx),
+mmuidx_2stage(mmu_idx),
 two_stage_indirect_error);
 cpu_loop_exit_restore(cs, retaddr);
 }
-- 
2.34.1

[PATCH v6 09/25] target/riscv: Use cpu_ld*_code_mmu for HLVX

[Richard Henderson]

Use the new functions to properly check execute permission
for the read rather than read permission.

Signed-off-by: Richard Henderson 
---
 target/riscv/op_helper.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 962a061228..b2169a99ff 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -427,18 +427,27 @@ void helper_hyp_gvma_tlb_flush(CPURISCVState *env)
 helper_hyp_tlb_flush(env);
 }
 
+/*
+ * TODO: These implementations are not quite correct.  They perform the
+ * access using execute permission just fine, but the final PMP check
+ * is supposed to have read permission as well.  Without replicating
+ * a fair fraction of cputlb.c, fixing this requires adding new mmu_idx
+ * which would imply that exact check in tlb_fill.
+ */
 target_ulong helper_hyp_hlvx_hu(CPURISCVState *env, target_ulong address)
 {
 int mmu_idx = cpu_mmu_index(env, true) | MMU_HYP_ACCESS_BIT;
+MemOpIdx oi = make_memop_idx(MO_TEUW, mmu_idx);
 
-return cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
+return cpu_ldw_code_mmu(env, address, oi, GETPC());
 }
 
 target_ulong helper_hyp_hlvx_wu(CPURISCVState *env, target_ulong address)
 {
 int mmu_idx = cpu_mmu_index(env, true) | MMU_HYP_ACCESS_BIT;
+MemOpIdx oi = make_memop_idx(MO_TEUL, mmu_idx);
 
-return cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
+return cpu_ldl_code_mmu(env, address, oi, GETPC());
 }
 
 #endif /* !CONFIG_USER_ONLY */
-- 
2.34.1

[PATCH v6 12/25] target/riscv: Introduce mmuidx_sum

[Richard Henderson]

In get_physical_address, we should use the setting passed
via mmu_idx rather than checking env->mstatus directly.

Signed-off-by: Richard Henderson 
---
 target/riscv/internals.h  | 5 +
 target/riscv/cpu_helper.c | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 7b63c0f1b6..0b61f337dd 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -37,6 +37,11 @@
 #define MMUIdx_M3
 #define MMU_2STAGE_BIT  (1 << 2)
 
+static inline bool mmuidx_sum(int mmu_idx)
+{
+return (mmu_idx & 3) == MMUIdx_S_SUM;
+}
+
 /* share data between vector helpers and decode code */
 FIELD(VDATA, VM, 0, 1)
 FIELD(VDATA, LMUL, 1, 3)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 888f7ae0ef..7e6cd8e0fd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -852,7 +852,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 widened = 2;
 }
 /* status.SUM will be ignored if execute on background */
-sum = get_field(env->mstatus, MSTATUS_SUM) || use_background || is_debug;
+sum = mmuidx_sum(mmu_idx) || use_background || is_debug;
 switch (vm) {
 case VM_1_10_SV32:
   levels = 2; ptidxbits = 10; ptesize = 4; break;
-- 
2.34.1

[PATCH v6 22/25] target/riscv: Don't modify SUM with is_debug

[Richard Henderson]

If we want to give the debugger a greater view of memory than
the cpu, we should simply disable the access check entirely,
not simply for this one corner case.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b26840e46c..850817edfd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -837,7 +837,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 }
 widened = 2;
 }
-sum = mmuidx_sum(mmu_idx) || is_debug;
+sum = mmuidx_sum(mmu_idx);
 switch (vm) {
 case VM_1_10_SV32:
   levels = 2; ptidxbits = 10; ptesize = 4; break;
-- 
2.34.1

[PATCH v6 19/25] target/riscv: Hoist pbmte and hade out of the level loop

[Richard Henderson]

These values are constant for every level of pte lookup.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 833ea6d3fa..00f70a3dd5 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -870,6 +870,14 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 return TRANSLATE_FAIL;
 }
 
+bool pbmte = env->menvcfg & MENVCFG_PBMTE;
+bool hade = env->menvcfg & MENVCFG_HADE;
+
+if (first_stage && two_stage && riscv_cpu_virt_enabled(env)) {
+pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
+hade = hade && (env->henvcfg & HENVCFG_HADE);
+}
+
 int ptshift = (levels - 1) * ptidxbits;
 int i;
 
@@ -930,14 +938,6 @@ restart:
 return TRANSLATE_FAIL;
 }
 
-bool pbmte = env->menvcfg & MENVCFG_PBMTE;
-bool hade = env->menvcfg & MENVCFG_HADE;
-
-if (first_stage && two_stage && riscv_cpu_virt_enabled(env)) {
-pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
-hade = hade && (env->henvcfg & HENVCFG_HADE);
-}
-
 if (riscv_cpu_sxl(env) == MXL_RV32) {
 ppn = pte >> PTE_PPN_SHIFT;
 } else if (pbmte || cpu->cfg.ext_svnapot) {
-- 
2.34.1

[PATCH v6 11/25] target/riscv: Rename MMU_HYP_ACCESS_BIT to MMU_2STAGE_BIT

[Richard Henderson]

We will enable more uses of this bit in the future.

Signed-off-by: Richard Henderson 
---
 target/riscv/internals.h  | 6 --
 target/riscv/cpu_helper.c | 2 +-
 target/riscv/op_helper.c  | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index b55152a7dc..7b63c0f1b6 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -27,13 +27,15 @@
  *  - S 0b001
  *  - S+SUM 0b010
  *  - M 0b011
- *  - HLV/HLVX/HSV adds 0b100
+ *  - U+2STAGE  0b100
+ *  - S+2STAGE  0b101
+ *  - S+SUM+2STAGE  0b110
  */
 #define MMUIdx_U0
 #define MMUIdx_S1
 #define MMUIdx_S_SUM2
 #define MMUIdx_M3
-#define MMU_HYP_ACCESS_BIT  (1 << 2)
+#define MMU_2STAGE_BIT  (1 << 2)
 
 /* share data between vector helpers and decode code */
 FIELD(VDATA, VM, 0, 1)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9bb84be4e1..888f7ae0ef 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -605,7 +605,7 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
enable)
 
 bool riscv_cpu_two_stage_lookup(int mmu_idx)
 {
-return mmu_idx & MMU_HYP_ACCESS_BIT;
+return mmu_idx & MMU_2STAGE_BIT;
 }
 
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 0f81645adf..81362537b6 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -437,7 +437,7 @@ static int check_access_hlsv(CPURISCVState *env, bool x, 
uintptr_t ra)
 riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
 }
 
-return cpu_mmu_index(env, x) | MMU_HYP_ACCESS_BIT;
+return cpu_mmu_index(env, x) | MMU_2STAGE_BIT;
 }
 
 target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
-- 
2.34.1

[PATCH v6 15/25] target/riscv: Move hstatus.spvp check to check_access_hlsv

[Richard Henderson]

The current cpu_mmu_index value is really irrelevant to
the HLV/HSV lookup.  Provide the correct priv level directly.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 8 +---
 target/riscv/op_helper.c  | 2 +-
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 8a124888cd..0adfd4a12b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -784,12 +784,6 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 use_background = true;
 }
 
-/* MPRV does not affect the virtual-machine load/store
-   instructions, HLV, HLVX, and HSV. */
-if (mmuidx_2stage(mmu_idx)) {
-mode = get_field(env->hstatus, HSTATUS_SPVP);
-}
-
 if (first_stage == false) {
 /* We are in stage 2 translation, this is similar to stage 1. */
 /* Stage 2 is always taken as U-mode */
@@ -1251,7 +1245,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 /* MPRV does not affect the virtual-machine load/store
instructions, HLV, HLVX, and HSV. */
 if (mmuidx_2stage(mmu_idx)) {
-mode = get_field(env->hstatus, HSTATUS_SPVP);
+;
 } else if (mode == PRV_M && access_type != MMU_INST_FETCH &&
get_field(env->mstatus, MSTATUS_MPRV)) {
 mode = get_field(env->mstatus, MSTATUS_MPP);
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 81362537b6..db7252e09d 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -437,7 +437,7 @@ static int check_access_hlsv(CPURISCVState *env, bool x, 
uintptr_t ra)
 riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
 }
 
-return cpu_mmu_index(env, x) | MMU_2STAGE_BIT;
+return get_field(env->hstatus, HSTATUS_SPVP) | MMU_2STAGE_BIT;
 }
 
 target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
-- 
2.34.1

[PATCH v6 07/25] target/riscv: Reduce overhead of MSTATUS_SUM change

[Richard Henderson]

From: Fei Wu 

Kernel needs to access user mode memory e.g. during syscalls, the window
is usually opened up for a very limited time through MSTATUS.SUM, the
overhead is too much if tlb_flush() gets called for every SUM change.

This patch creates a separate MMU index for S+SUM, so that it's not
necessary to flush tlb anymore when SUM changes. This is similar to how
ARM handles Privileged Access Never (PAN).

Result of 'pipe 10' from unixbench boosts from 223656 to 1705006. Many
other syscalls benefit a lot from this too.

Reviewed-by: Richard Henderson 
Signed-off-by: Fei Wu 
Message-Id: <20230324054154.414846-3-fei2...@intel.com>
---
 target/riscv/cpu.h  |  2 --
 target/riscv/internals.h| 14 ++
 target/riscv/cpu_helper.c   | 17 +++--
 target/riscv/csr.c  |  3 +--
 target/riscv/op_helper.c|  5 +++--
 target/riscv/insn_trans/trans_rvh.c.inc |  4 ++--
 6 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 3e59dbb3fd..5e589db106 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -631,8 +631,6 @@ G_NORETURN void riscv_raise_exception(CPURISCVState *env,
 target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
 void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
 
-#define TB_FLAGS_PRIV_HYP_ACCESS_MASK   (1 << 2)
-
 #include "exec/cpu-all.h"
 
 FIELD(TB_FLAGS, MEM_IDX, 0, 3)
diff --git a/target/riscv/internals.h b/target/riscv/internals.h
index 5620fbffb6..b55152a7dc 100644
--- a/target/riscv/internals.h
+++ b/target/riscv/internals.h
@@ -21,6 +21,20 @@
 
 #include "hw/registerfields.h"
 
+/*
+ * The current MMU Modes are:
+ *  - U 0b000
+ *  - S 0b001
+ *  - S+SUM 0b010
+ *  - M 0b011
+ *  - HLV/HLVX/HSV adds 0b100
+ */
+#define MMUIdx_U0
+#define MMUIdx_S1
+#define MMUIdx_S_SUM2
+#define MMUIdx_M3
+#define MMU_HYP_ACCESS_BIT  (1 << 2)
+
 /* share data between vector helpers and decode code */
 FIELD(VDATA, VM, 0, 1)
 FIELD(VDATA, LMUL, 1, 3)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 5753126c7a..052fdd2d9d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/log.h"
 #include "qemu/main-loop.h"
 #include "cpu.h"
+#include "internals.h"
 #include "pmu.h"
 #include "exec/exec-all.h"
 #include "instmap.h"
@@ -36,7 +37,19 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 #ifdef CONFIG_USER_ONLY
 return 0;
 #else
-return env->priv;
+if (ifetch) {
+return env->priv;
+}
+
+/* All priv -> mmu_idx mapping are here */
+int mode = env->priv;
+if (mode == PRV_M && get_field(env->mstatus, MSTATUS_MPRV)) {
+mode = get_field(env->mstatus, MSTATUS_MPP);
+}
+if (mode == PRV_S && get_field(env->mstatus, MSTATUS_SUM)) {
+return MMUIdx_S_SUM;
+}
+return mode;
 #endif
 }
 
@@ -600,7 +613,7 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
enable)
 
 bool riscv_cpu_two_stage_lookup(int mmu_idx)
 {
-return mmu_idx & TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+return mmu_idx & MMU_HYP_ACCESS_BIT;
 }
 
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint64_t interrupts)
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index abea7b749e..b79758a606 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1246,8 +1246,7 @@ static RISCVException write_mstatus(CPURISCVState *env, 
int csrno,
 RISCVMXL xl = riscv_cpu_mxl(env);
 
 /* flush tlb on mstatus fields that affect VM */
-if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
-MSTATUS_MPRV | MSTATUS_SUM)) {
+if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPV)) {
 tlb_flush(env_cpu(env));
 }
 mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index 84ee018f7d..962a061228 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -20,6 +20,7 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
+#include "internals.h"
 #include "qemu/main-loop.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
@@ -428,14 +429,14 @@ void helper_hyp_gvma_tlb_flush(CPURISCVState *env)
 
 target_ulong helper_hyp_hlvx_hu(CPURISCVState *env, target_ulong address)
 {
-int mmu_idx = cpu_mmu_index(env, true) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+int mmu_idx = cpu_mmu_index(env, true) | MMU_HYP_ACCESS_BIT;
 
 return cpu_lduw_mmuidx_ra(env, address, mmu_idx, GETPC());
 }
 
 target_ulong helper_hyp_hlvx_wu(CPURISCVState *env, target_ulong address)
 {
-int mmu_idx = cpu_mmu_index(env, true) | TB_FLAGS_PRIV_HYP_ACCESS_MASK;
+int mmu_idx = cpu_mmu_index(env, true) | MMU_HYP_ACCESS_BIT;
 
 return cpu_ldl_mmuidx_ra(env, address, mmu_idx, GETPC());
 }
diff --git a/target/riscv/insn_trans/trans_rvh.c.

[PATCH v6 01/25] target/riscv: Extract virt enabled state from tb flags

[Richard Henderson]

From: LIU Zhiwei 

Virt enabled state is not a constant. So we should put it into tb flags.
Thus we can use it like a constant condition at translation phase.

Reported-by: Richard Henderson 
Reviewed-by: Richard Henderson 
Signed-off-by: LIU Zhiwei 
Reviewed-by: Weiwei Li 
Message-Id: <20230324143031.1093-2-zhiwei_...@linux.alibaba.com>
---
 target/riscv/cpu.h|  2 ++
 target/riscv/cpu_helper.c |  2 ++
 target/riscv/translate.c  | 10 +-
 3 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..12fe8d8546 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -650,6 +650,8 @@ FIELD(TB_FLAGS, VTA, 24, 1)
 FIELD(TB_FLAGS, VMA, 25, 1)
 /* Native debug itrigger */
 FIELD(TB_FLAGS, ITRIGGER, 26, 1)
+/* Virtual mode enabled */
+FIELD(TB_FLAGS, VIRT_ENABLED, 27, 1)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f88c503cf4..9d50e7bbb6 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -104,6 +104,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 
 flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
get_field(env->mstatus_hs, MSTATUS_VS));
+flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED,
+   get_field(env->virt, VIRT_ONOFF));
 }
 if (cpu->cfg.debug && !icount_enabled()) {
 flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0ee8ee147d..880f6318aa 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1156,15 +1156,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cs)
 ctx->mstatus_fs = tb_flags & TB_FLAGS_MSTATUS_FS;
 ctx->mstatus_vs = tb_flags & TB_FLAGS_MSTATUS_VS;
 ctx->priv_ver = env->priv_ver;
-#if !defined(CONFIG_USER_ONLY)
-if (riscv_has_ext(env, RVH)) {
-ctx->virt_enabled = riscv_cpu_virt_enabled(env);
-} else {
-ctx->virt_enabled = false;
-}
-#else
-ctx->virt_enabled = false;
-#endif
+ctx->virt_enabled = FIELD_EX32(tb_flags, TB_FLAGS, VIRT_ENABLED);
 ctx->misa_ext = env->misa_ext;
 ctx->frm = -1;  /* unknown rounding mode */
 ctx->cfg_ptr = &(cpu->cfg);
-- 
2.34.1

[PATCH v6 16/25] target/riscv: Set MMU_2STAGE_BIT in riscv_cpu_mmu_index

[Richard Henderson]

Incorporate the virt_enabled and MPV checks into the cpu_mmu_index
function, so we don't have to keep doing it within tlb_fill and
subroutines.  This also elides a flush on changes to MPV.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 50 +--
 target/riscv/csr.c|  6 +
 2 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 0adfd4a12b..6c42f9c6fd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -37,19 +37,21 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 #ifdef CONFIG_USER_ONLY
 return 0;
 #else
-if (ifetch) {
-return env->priv;
-}
+bool virt = riscv_cpu_virt_enabled(env);
+int mode = env->priv;
 
 /* All priv -> mmu_idx mapping are here */
-int mode = env->priv;
-if (mode == PRV_M && get_field(env->mstatus, MSTATUS_MPRV)) {
-mode = get_field(env->mstatus, MSTATUS_MPP);
+if (!ifetch) {
+if (mode == PRV_M && get_field(env->mstatus, MSTATUS_MPRV)) {
+mode = get_field(env->mstatus, MSTATUS_MPP);
+virt = get_field(env->mstatus, MSTATUS_MPV);
+}
+if (mode == PRV_S && get_field(env->mstatus, MSTATUS_SUM)) {
+mode = MMUIdx_S_SUM;
+}
 }
-if (mode == PRV_S && get_field(env->mstatus, MSTATUS_SUM)) {
-return MMUIdx_S_SUM;
-}
-return mode;
+
+return mode | (virt ? MMU_2STAGE_BIT : 0);
 #endif
 }
 
@@ -1165,8 +1167,7 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr 
physaddr,
 }
 
 env->badaddr = addr;
-env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
-mmuidx_2stage(mmu_idx);
+env->two_stage_lookup = mmuidx_2stage(mmu_idx);
 env->two_stage_indirect_lookup = false;
 cpu_loop_exit_restore(cs, retaddr);
 }
@@ -1191,8 +1192,7 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr 
addr,
 g_assert_not_reached();
 }
 env->badaddr = addr;
-env->two_stage_lookup = riscv_cpu_virt_enabled(env) ||
-mmuidx_2stage(mmu_idx);
+env->two_stage_lookup = mmuidx_2stage(mmu_idx);
 env->two_stage_indirect_lookup = false;
 cpu_loop_exit_restore(cs, retaddr);
 }
@@ -1230,7 +1230,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 int prot, prot2, prot_pmp;
 bool pmp_violation = false;
 bool first_stage_error = true;
-bool two_stage_lookup = false;
+bool two_stage_lookup = mmuidx_2stage(mmu_idx);
 bool two_stage_indirect_error = false;
 int ret = TRANSLATE_FAIL;
 int mode = mmu_idx;
@@ -1242,22 +1242,8 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
   __func__, address, access_type, mmu_idx);
 
-/* MPRV does not affect the virtual-machine load/store
-   instructions, HLV, HLVX, and HSV. */
-if (mmuidx_2stage(mmu_idx)) {
-;
-} else if (mode == PRV_M && access_type != MMU_INST_FETCH &&
-   get_field(env->mstatus, MSTATUS_MPRV)) {
-mode = get_field(env->mstatus, MSTATUS_MPP);
-if (riscv_has_ext(env, RVH) && get_field(env->mstatus, MSTATUS_MPV)) {
-two_stage_lookup = true;
-}
-}
-
 pmu_tlb_fill_incr_ctr(cpu, access_type);
-if (riscv_cpu_virt_enabled(env) ||
-((mmuidx_2stage(mmu_idx) || two_stage_lookup) &&
- access_type != MMU_INST_FETCH)) {
+if (two_stage_lookup) {
 /* Two stage lookup */
 ret = get_physical_address(env, &pa, &prot, address,
&env->guest_phys_fault_addr, access_type,
@@ -1352,9 +1338,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 return false;
 } else {
 raise_mmu_exception(env, address, access_type, pmp_violation,
-first_stage_error,
-riscv_cpu_virt_enabled(env) ||
-mmuidx_2stage(mmu_idx),
+first_stage_error, two_stage_lookup,
 two_stage_indirect_error);
 cpu_loop_exit_restore(cs, retaddr);
 }
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index b79758a606..1b635373c6 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1246,7 +1246,7 @@ static RISCVException write_mstatus(CPURISCVState *env, 
int csrno,
 RISCVMXL xl = riscv_cpu_mxl(env);
 
 /* flush tlb on mstatus fields that affect VM */
-if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPV)) {
+if ((val ^ mstatus) & MSTATUS_MXR) {
 tlb_flush(env_cpu(env));
 }
 mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
@@ -1294,10 +1294,6 @@ static RISCVException write_mstatush(CPURISCVState *env, 
int csrno,
 uint64_t valh = (uint64_t)val << 32;
 uint64_t mask = MST

[PATCH v6 02/25] target/riscv: Add a general status enum for extensions

[Richard Henderson]

From: LIU Zhiwei 

The pointer masking is the only extension that directly use status.
The vector or float extension uses the status in an indirect way.

Replace the pointer masking extension special status fields with
the general status.

Reviewed-by: Richard Henderson 
Signed-off-by: LIU Zhiwei 
Message-Id: <20230324143031.1093-3-zhiwei_...@linux.alibaba.com>
[rth: Add a typedef for the enum]
Signed-off-by: Richard Henderson 
---
 target/riscv/cpu.h  |  8 
 target/riscv/cpu_bits.h | 12 
 target/riscv/cpu.c  |  2 +-
 target/riscv/csr.c  | 14 +++---
 4 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 12fe8d8546..30d9828d59 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -99,6 +99,14 @@ enum {
 TRANSLATE_G_STAGE_FAIL
 };
 
+/* Extension context status */
+typedef enum {
+EXT_STATUS_DISABLED = 0,
+EXT_STATUS_INITIAL,
+EXT_STATUS_CLEAN,
+EXT_STATUS_DIRTY,
+} RISCVExtStatus;
+
 #define MMU_USER_IDX 3
 
 #define MAX_RISCV_PMPS (16)
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef..b84f62f8d6 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -9,6 +9,9 @@
  (((uint64_t)(val) * ((mask) & ~((mask) << 1))) & \
  (uint64_t)(mask)))
 
+/* Extension context status mask */
+#define EXT_STATUS_MASK 0x3ULL
+
 /* Floating point round mode */
 #define FSR_RD_SHIFT5
 #define FSR_RD  (0x7 << FSR_RD_SHIFT)
@@ -734,13 +737,6 @@ typedef enum RISCVException {
 #define PM_ENABLE   0x0001ULL
 #define PM_CURRENT  0x0002ULL
 #define PM_INSN 0x0004ULL
-#define PM_XS_MASK  0x0003ULL
-
-/* PointerMasking XS bits values */
-#define PM_EXT_DISABLE  0xULL
-#define PM_EXT_INITIAL  0x0001ULL
-#define PM_EXT_CLEAN0x0002ULL
-#define PM_EXT_DIRTY0x0003ULL
 
 /* Execution enviornment configuration bits */
 #define MENVCFG_FIOM   BIT(0)
@@ -780,7 +776,7 @@ typedef enum RISCVException {
 #define S_OFFSET 5ULL
 #define M_OFFSET 8ULL
 
-#define PM_XS_BITS   (PM_XS_MASK << XS_OFFSET)
+#define PM_XS_BITS   (EXT_STATUS_MASK << XS_OFFSET)
 #define U_PM_ENABLE  (PM_ENABLE  << U_OFFSET)
 #define U_PM_CURRENT (PM_CURRENT << U_OFFSET)
 #define U_PM_INSN(PM_INSN<< U_OFFSET)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1e97473af2..1135106b3e 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -764,7 +764,7 @@ static void riscv_cpu_reset_hold(Object *obj)
 i++;
 }
 /* mmte is supposed to have pm.current hardwired to 1 */
-env->mmte |= (PM_EXT_INITIAL | MMTE_M_PM_CURRENT);
+env->mmte |= (EXT_STATUS_INITIAL | MMTE_M_PM_CURRENT);
 #endif
 env->xl = riscv_cpu_mxl(env);
 riscv_cpu_update_mask(env);
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d522efc0b6..abea7b749e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -3513,7 +3513,7 @@ static RISCVException write_mmte(CPURISCVState *env, int 
csrno,
 
 /* hardwiring pm.instruction bit to 0, since it's not supported yet */
 wpri_val &= ~(MMTE_M_PM_INSN | MMTE_S_PM_INSN | MMTE_U_PM_INSN);
-env->mmte = wpri_val | PM_EXT_DIRTY;
+env->mmte = wpri_val | EXT_STATUS_DIRTY;
 riscv_cpu_update_mask(env);
 
 /* Set XS and SD bits, since PM CSRs are dirty */
@@ -3593,7 +3593,7 @@ static RISCVException write_mpmmask(CPURISCVState *env, 
int csrno,
 if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
 env->cur_pmmask = val;
 }
-env->mmte |= PM_EXT_DIRTY;
+env->mmte |= EXT_STATUS_DIRTY;
 
 /* Set XS and SD bits, since PM CSRs are dirty */
 mstatus = env->mstatus | MSTATUS_XS;
@@ -3621,7 +3621,7 @@ static RISCVException write_spmmask(CPURISCVState *env, 
int csrno,
 if ((env->priv == PRV_S) && (env->mmte & S_PM_ENABLE)) {
 env->cur_pmmask = val;
 }
-env->mmte |= PM_EXT_DIRTY;
+env->mmte |= EXT_STATUS_DIRTY;
 
 /* Set XS and SD bits, since PM CSRs are dirty */
 mstatus = env->mstatus | MSTATUS_XS;
@@ -3649,7 +3649,7 @@ static RISCVException write_upmmask(CPURISCVState *env, 
int csrno,
 if ((env->priv == PRV_U) && (env->mmte & U_PM_ENABLE)) {
 env->cur_pmmask = val;
 }
-env->mmte |= PM_EXT_DIRTY;
+env->mmte |= EXT_STATUS_DIRTY;
 
 /* Set XS and SD bits, since PM CSRs are dirty */
 mstatus = env->mstatus | MSTATUS_XS;
@@ -3673,7 +3673,7 @@ static RISCVException write_mpmbase(CPURISCVState *env, 
int csrno,
 if ((env->priv == PRV_M) && (env->mmte & M_PM_ENABLE)) {
 env->cur_pmbase = val;
 }
-env->mmte |= PM_EXT_DIRTY;
+env->mmte |= EXT_STATUS_DIRTY;
 
 /* Set XS and SD bits, since PM CSRs are dirty */
 mstatus = env->mstatus | MSTATUS_XS;
@@ -3701,7 +3701,7 @@ static RISCVException write_spmbase(CPURISCVState *env, 
int csrno,
 if ((env->priv == PRV_S) && (env->mmte & S_

[PATCH v6 00/25] target/riscv: MSTATUS_SUM + cleanups

[Richard Henderson]

This builds on Fei and Zhiwei's SUM and TB_FLAGS changes.

  * Reclaim 5 TB_FLAGS bits, since we nearly ran out.

  * Using cpu_mmu_index(env, true) is insufficient to implement
HLVX properly.  While that chooses the correct mmu_idx, it
does not perform the read with execute permission.
I add a new tcg interface to perform a read-for-execute with
an arbitrary mmu_idx.  This is still not 100% compliant, but
it's closer.

  * Handle mstatus.MPV in cpu_mmu_index.
  * Use vsstatus.SUM when required for MMUIdx_S_SUM.
  * Cleanups for get_physical_address.

While this passes check-avocado, I'm sure that's insufficient.
Please have a close look.


r~


Fei Wu (2):
  target/riscv: Separate priv from mmu_idx
  target/riscv: Reduce overhead of MSTATUS_SUM change

LIU Zhiwei (4):
  target/riscv: Extract virt enabled state from tb flags
  target/riscv: Add a general status enum for extensions
  target/riscv: Encode the FS and VS on a normal way for tb flags
  target/riscv: Add a tb flags field for vstart

Richard Henderson (19):
  target/riscv: Remove mstatus_hs_{fs,vs} from tb_flags
  accel/tcg: Add cpu_ld*_code_mmu
  target/riscv: Use cpu_ld*_code_mmu for HLVX
  target/riscv: Handle HLV, HSV via helpers
  target/riscv: Rename MMU_HYP_ACCESS_BIT to MMU_2STAGE_BIT
  target/riscv: Introduce mmuidx_sum
  target/riscv: Introduce mmuidx_priv
  target/riscv: Introduce mmuidx_2stage
  target/riscv: Move hstatus.spvp check to check_access_hlsv
  target/riscv: Set MMU_2STAGE_BIT in riscv_cpu_mmu_index
  target/riscv: Check SUM in the correct register
  target/riscv: Hoist second stage mode change to callers
  target/riscv: Hoist pbmte and hade out of the level loop
  target/riscv: Move leaf pte processing out of level loop
  target/riscv: Suppress pte update with is_debug
  target/riscv: Don't modify SUM with is_debug
  target/riscv: Merge checks for reserved pte flags
  target/riscv: Reorg access check in get_physical_address
  target/riscv: Reorg sum check in get_physical_address

 include/exec/cpu_ldst.h   |   9 +
 target/riscv/cpu.h|  47 ++-
 target/riscv/cpu_bits.h   |  12 +-
 target/riscv/helper.h |  12 +-
 target/riscv/internals.h  |  35 ++
 accel/tcg/cputlb.c|  48 +++
 accel/tcg/user-exec.c |  58 +++
 target/riscv/cpu.c|   2 +-
 target/riscv/cpu_helper.c | 393 +-
 target/riscv/csr.c|  21 +-
 target/riscv/op_helper.c  | 113 -
 target/riscv/translate.c  |  72 ++--
 .../riscv/insn_trans/trans_privileged.c.inc   |   2 +-
 target/riscv/insn_trans/trans_rvf.c.inc   |   2 +-
 target/riscv/insn_trans/trans_rvh.c.inc   | 135 +++---
 target/riscv/insn_trans/trans_rvv.c.inc   |  22 +-
 target/riscv/insn_trans/trans_xthead.c.inc|   7 +-
 17 files changed, 595 insertions(+), 395 deletions(-)

-- 
2.34.1

[PATCH v6 06/25] target/riscv: Separate priv from mmu_idx

[Richard Henderson]

From: Fei Wu 

Currently it's assumed the 2 low bits of mmu_idx map to privilege mode,
this assumption won't last as we are about to add more mmu_idx. Here an
individual priv field is added into TB_FLAGS.

Reviewed-by: Richard Henderson 
Signed-off-by: Fei Wu 
Message-Id: <20230324054154.414846-2-fei2...@intel.com>
---
 target/riscv/cpu.h | 2 +-
 target/riscv/cpu_helper.c  | 4 +++-
 target/riscv/translate.c   | 2 ++
 target/riscv/insn_trans/trans_privileged.c.inc | 2 +-
 target/riscv/insn_trans/trans_xthead.c.inc | 7 +--
 5 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 86a82e25dc..3e59dbb3fd 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -631,7 +631,6 @@ G_NORETURN void riscv_raise_exception(CPURISCVState *env,
 target_ulong riscv_cpu_get_fflags(CPURISCVState *env);
 void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
 
-#define TB_FLAGS_PRIV_MMU_MASK3
 #define TB_FLAGS_PRIV_HYP_ACCESS_MASK   (1 << 2)
 
 #include "exec/cpu-all.h"
@@ -658,6 +657,7 @@ FIELD(TB_FLAGS, ITRIGGER, 22, 1)
 /* Virtual mode enabled */
 FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
 FIELD(TB_FLAGS, VSTART_EQ_ZERO, 24, 1)
+FIELD(TB_FLAGS, PRIV, 25, 2)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 4f0999d50b..5753126c7a 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -83,6 +83,8 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 fs = EXT_STATUS_DIRTY;
 vs = EXT_STATUS_DIRTY;
 #else
+flags = FIELD_DP32(flags, TB_FLAGS, PRIV, env->priv);
+
 flags |= cpu_mmu_index(env, 0);
 fs = get_field(env->mstatus, MSTATUS_FS);
 vs = get_field(env->mstatus, MSTATUS_VS);
@@ -764,7 +766,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
  * (riscv_cpu_do_interrupt) is correct */
 MemTxResult res;
 MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
-int mode = mmu_idx & TB_FLAGS_PRIV_MMU_MASK;
+int mode = env->priv;
 bool use_background = false;
 hwaddr ppn;
 RISCVCPU *cpu = env_archcpu(env);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index f8c077525c..abfc152553 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -67,6 +67,7 @@ typedef struct DisasContext {
 RISCVExtStatus mstatus_fs;
 RISCVExtStatus mstatus_vs;
 uint32_t mem_idx;
+uint32_t priv;
 /* Remember the rounding mode encoded in the previous fp instruction,
which we have already installed into env->fp_status.  Or -1 for
no previous fp instruction.  Note that we exit the TB when writing
@@ -1140,6 +1141,7 @@ static void riscv_tr_init_disas_context(DisasContextBase 
*dcbase, CPUState *cs)
 uint32_t tb_flags = ctx->base.tb->flags;
 
 ctx->pc_succ_insn = ctx->base.pc_first;
+ctx->priv = FIELD_EX32(tb_flags, TB_FLAGS, PRIV);
 ctx->mem_idx = FIELD_EX32(tb_flags, TB_FLAGS, MEM_IDX);
 ctx->mstatus_fs = FIELD_EX32(tb_flags, TB_FLAGS, FS);
 ctx->mstatus_vs = FIELD_EX32(tb_flags, TB_FLAGS, VS);
diff --git a/target/riscv/insn_trans/trans_privileged.c.inc 
b/target/riscv/insn_trans/trans_privileged.c.inc
index 59501b2780..9305b18299 100644
--- a/target/riscv/insn_trans/trans_privileged.c.inc
+++ b/target/riscv/insn_trans/trans_privileged.c.inc
@@ -52,7 +52,7 @@ static bool trans_ebreak(DisasContext *ctx, arg_ebreak *a)
  * that no exception will be raised when fetching them.
  */
 
-if (semihosting_enabled(ctx->mem_idx < PRV_S) &&
+if (semihosting_enabled(ctx->priv < PRV_S) &&
 (pre_addr & TARGET_PAGE_MASK) == (post_addr & TARGET_PAGE_MASK)) {
 pre= opcode_at(&ctx->base, pre_addr);
 ebreak = opcode_at(&ctx->base, ebreak_addr);
diff --git a/target/riscv/insn_trans/trans_xthead.c.inc 
b/target/riscv/insn_trans/trans_xthead.c.inc
index df504c3f2c..adfb53cb4c 100644
--- a/target/riscv/insn_trans/trans_xthead.c.inc
+++ b/target/riscv/insn_trans/trans_xthead.c.inc
@@ -265,12 +265,7 @@ static bool trans_th_tst(DisasContext *ctx, arg_th_tst *a)
 
 static inline int priv_level(DisasContext *ctx)
 {
-#ifdef CONFIG_USER_ONLY
-return PRV_U;
-#else
- /* Priv level is part of mem_idx. */
-return ctx->mem_idx & TB_FLAGS_PRIV_MMU_MASK;
-#endif
+return ctx->priv;
 }
 
 /* Test if priv level is M, S, or U (cannot fail). */
-- 
2.34.1

[PATCH v6 03/25] target/riscv: Encode the FS and VS on a normal way for tb flags

[Richard Henderson]

From: LIU Zhiwei 

Reuse the MSTATUS_FS and MSTATUS_VS for the tb flags positions is not a
normal way.

It will make it hard to change the tb flags layout. And even worse, if we
want to keep tb flags for a same extension togather without a hole.

Reviewed-by: Richard Henderson 
Signed-off-by: LIU Zhiwei 
Reviewed-by: Weiwei Li 
Message-Id: <20230324143031.1093-4-zhiwei_...@linux.alibaba.com>
[rth: Adjust trans_rvf.c.inc as well; use the typedef]
Signed-off-by: Richard Henderson 
---
 target/riscv/cpu.h  | 15 +--
 target/riscv/cpu_helper.c   | 11 
 target/riscv/translate.c| 34 -
 target/riscv/insn_trans/trans_rvf.c.inc |  2 +-
 target/riscv/insn_trans/trans_rvv.c.inc |  8 +++---
 5 files changed, 34 insertions(+), 36 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 30d9828d59..f787145a21 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -633,18 +633,17 @@ void riscv_cpu_set_fflags(CPURISCVState *env, 
target_ulong);
 
 #define TB_FLAGS_PRIV_MMU_MASK3
 #define TB_FLAGS_PRIV_HYP_ACCESS_MASK   (1 << 2)
-#define TB_FLAGS_MSTATUS_FS MSTATUS_FS
-#define TB_FLAGS_MSTATUS_VS MSTATUS_VS
 
 #include "exec/cpu-all.h"
 
 FIELD(TB_FLAGS, MEM_IDX, 0, 3)
-FIELD(TB_FLAGS, LMUL, 3, 3)
-FIELD(TB_FLAGS, SEW, 6, 3)
-/* Skip MSTATUS_VS (0x600) bits */
-FIELD(TB_FLAGS, VL_EQ_VLMAX, 11, 1)
-FIELD(TB_FLAGS, VILL, 12, 1)
-/* Skip MSTATUS_FS (0x6000) bits */
+FIELD(TB_FLAGS, FS, 3, 2)
+/* Vector flags */
+FIELD(TB_FLAGS, VS, 5, 2)
+FIELD(TB_FLAGS, LMUL, 7, 3)
+FIELD(TB_FLAGS, SEW, 10, 3)
+FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
+FIELD(TB_FLAGS, VILL, 14, 1)
 /* Is a Hypervisor instruction load/store allowed? */
 FIELD(TB_FLAGS, HLSX, 15, 1)
 FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9d50e7bbb6..1e7ee9aa30 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -79,16 +79,17 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 }
 
 #ifdef CONFIG_USER_ONLY
-flags |= TB_FLAGS_MSTATUS_FS;
-flags |= TB_FLAGS_MSTATUS_VS;
+flags = FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY);
+flags = FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY);
 #else
 flags |= cpu_mmu_index(env, 0);
 if (riscv_cpu_fp_enabled(env)) {
-flags |= env->mstatus & MSTATUS_FS;
+flags =  FIELD_DP32(flags, TB_FLAGS, FS,
+get_field(env->mstatus,  MSTATUS_FS));
 }
-
 if (riscv_cpu_vector_enabled(env)) {
-flags |= env->mstatus & MSTATUS_VS;
+flags =  FIELD_DP32(flags, TB_FLAGS, VS,
+get_field(env->mstatus, MSTATUS_VS));
 }
 
 if (riscv_has_ext(env, RVH)) {
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 880f6318aa..b897bf6006 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -64,10 +64,10 @@ typedef struct DisasContext {
 RISCVMXL xl;
 uint32_t misa_ext;
 uint32_t opcode;
-uint32_t mstatus_fs;
-uint32_t mstatus_vs;
-uint32_t mstatus_hs_fs;
-uint32_t mstatus_hs_vs;
+RISCVExtStatus mstatus_fs;
+RISCVExtStatus mstatus_vs;
+RISCVExtStatus mstatus_hs_fs;
+RISCVExtStatus mstatus_hs_vs;
 uint32_t mem_idx;
 /* Remember the rounding mode encoded in the previous fp instruction,
which we have already installed into env->fp_status.  Or -1 for
@@ -598,8 +598,7 @@ static TCGv get_address_indexed(DisasContext *ctx, int rs1, 
TCGv offs)
 }
 
 #ifndef CONFIG_USER_ONLY
-/* The states of mstatus_fs are:
- * 0 = disabled, 1 = initial, 2 = clean, 3 = dirty
+/*
  * We will have already diagnosed disabled state,
  * and need to turn initial/clean into dirty.
  */
@@ -611,9 +610,9 @@ static void mark_fs_dirty(DisasContext *ctx)
 return;
 }
 
-if (ctx->mstatus_fs != MSTATUS_FS) {
+if (ctx->mstatus_fs != EXT_STATUS_DIRTY) {
 /* Remember the state change for the rest of the TB. */
-ctx->mstatus_fs = MSTATUS_FS;
+ctx->mstatus_fs = EXT_STATUS_DIRTY;
 
 tmp = tcg_temp_new();
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
@@ -621,9 +620,9 @@ static void mark_fs_dirty(DisasContext *ctx)
 tcg_gen_st_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
 }
 
-if (ctx->virt_enabled && ctx->mstatus_hs_fs != MSTATUS_FS) {
+if (ctx->virt_enabled && ctx->mstatus_hs_fs != EXT_STATUS_DIRTY) {
 /* Remember the stage change for the rest of the TB. */
-ctx->mstatus_hs_fs = MSTATUS_FS;
+ctx->mstatus_hs_fs = EXT_STATUS_DIRTY;
 
 tmp = tcg_temp_new();
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus_hs));
@@ -636,8 +635,7 @@ static inline void mark_fs_dirty(DisasContext *ctx) { }
 #endif
 
 #ifndef CONFIG_USER_ONLY
-/* The states of mstatus_vs are:
- * 0 = disabled, 1 = initial, 2 = clean, 3 = dirty
+/*
  * We

[PATCH v6 18/25] target/riscv: Hoist second stage mode change to callers

[Richard Henderson]

Move the check from the top of get_physical_address to
the two callers, where passing mmu_idx makes no sense.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 10 ++
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 0017ecbf37..833ea6d3fa 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -791,12 +791,6 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 use_background = true;
 }
 
-if (first_stage == false) {
-/* We are in stage 2 translation, this is similar to stage 1. */
-/* Stage 2 is always taken as U-mode */
-mode = PRV_U;
-}
-
 if (mode == PRV_M || !riscv_cpu_cfg(env)->mmu) {
 *physical = addr;
 *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -902,7 +896,7 @@ restart:
 /* Do the second stage translation on the base PTE address. */
 int vbase_ret = get_physical_address(env, &vbase, &vbase_prot,
  base, NULL, MMU_DATA_LOAD,
- mmu_idx, false, true,
+ MMUIdx_U, false, true,
  is_debug);
 
 if (vbase_ret != TRANSLATE_SUCCESS) {
@@ -1274,7 +1268,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 im_address = pa;
 
 ret = get_physical_address(env, &pa, &prot2, im_address, NULL,
-   access_type, mmu_idx, false, true,
+   access_type, MMUIdx_U, false, true,
false);
 
 qemu_log_mask(CPU_LOG_MMU,
-- 
2.34.1

[PATCH v6 23/25] target/riscv: Merge checks for reserved pte flags

[Richard Henderson]

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 850817edfd..82a7c5f9dd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -976,14 +976,14 @@ restart:
 /* Reserved without Svpbmt. */
 return TRANSLATE_FAIL;
 }
-if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
-/* Reserved leaf PTE flags: PTE_W */
-return TRANSLATE_FAIL;
-}
-if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
-/* Reserved leaf PTE flags: PTE_W + PTE_X */
+
+/* Check for reserved combinations of RWX flags. */
+switch (pte & (PTE_R | PTE_W | PTE_X)) {
+case PTE_W:
+case PTE_W | PTE_X:
 return TRANSLATE_FAIL;
 }
+
 if ((pte & PTE_U) &&
 ((mode != PRV_U) && (!sum || access_type == MMU_INST_FETCH))) {
 /*
-- 
2.34.1

[PATCH v6 04/25] target/riscv: Remove mstatus_hs_{fs, vs} from tb_flags

[Richard Henderson]

Merge with mstatus_{fs,vs}.  We might perform a redundant
assignment to one or the other field, but it's a trivial
and saves 4 bits from TB_FLAGS.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu.h| 16 +++-
 target/riscv/cpu_helper.c | 34 --
 target/riscv/translate.c  | 32 ++--
 3 files changed, 33 insertions(+), 49 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index f787145a21..d9e0eaaf9b 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -646,19 +646,17 @@ FIELD(TB_FLAGS, VL_EQ_VLMAX, 13, 1)
 FIELD(TB_FLAGS, VILL, 14, 1)
 /* Is a Hypervisor instruction load/store allowed? */
 FIELD(TB_FLAGS, HLSX, 15, 1)
-FIELD(TB_FLAGS, MSTATUS_HS_FS, 16, 2)
-FIELD(TB_FLAGS, MSTATUS_HS_VS, 18, 2)
 /* The combination of MXL/SXL/UXL that applies to the current cpu mode. */
-FIELD(TB_FLAGS, XL, 20, 2)
+FIELD(TB_FLAGS, XL, 16, 2)
 /* If PointerMasking should be applied */
-FIELD(TB_FLAGS, PM_MASK_ENABLED, 22, 1)
-FIELD(TB_FLAGS, PM_BASE_ENABLED, 23, 1)
-FIELD(TB_FLAGS, VTA, 24, 1)
-FIELD(TB_FLAGS, VMA, 25, 1)
+FIELD(TB_FLAGS, PM_MASK_ENABLED, 18, 1)
+FIELD(TB_FLAGS, PM_BASE_ENABLED, 19, 1)
+FIELD(TB_FLAGS, VTA, 20, 1)
+FIELD(TB_FLAGS, VMA, 21, 1)
 /* Native debug itrigger */
-FIELD(TB_FLAGS, ITRIGGER, 26, 1)
+FIELD(TB_FLAGS, ITRIGGER, 22, 1)
 /* Virtual mode enabled */
-FIELD(TB_FLAGS, VIRT_ENABLED, 27, 1)
+FIELD(TB_FLAGS, VIRT_ENABLED, 23, 1)
 
 #ifdef TARGET_RISCV32
 #define riscv_cpu_mxl(env)  ((void)(env), MXL_RV32)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1e7ee9aa30..4fdd6fe021 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -45,7 +45,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 {
 CPUState *cs = env_cpu(env);
 RISCVCPU *cpu = RISCV_CPU(cs);
-
+RISCVExtStatus fs, vs;
 uint32_t flags = 0;
 
 *pc = env->xl == MXL_RV32 ? env->pc & UINT32_MAX : env->pc;
@@ -79,18 +79,12 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 }
 
 #ifdef CONFIG_USER_ONLY
-flags = FIELD_DP32(flags, TB_FLAGS, FS, EXT_STATUS_DIRTY);
-flags = FIELD_DP32(flags, TB_FLAGS, VS, EXT_STATUS_DIRTY);
+fs = EXT_STATUS_DIRTY;
+vs = EXT_STATUS_DIRTY;
 #else
 flags |= cpu_mmu_index(env, 0);
-if (riscv_cpu_fp_enabled(env)) {
-flags =  FIELD_DP32(flags, TB_FLAGS, FS,
-get_field(env->mstatus,  MSTATUS_FS));
-}
-if (riscv_cpu_vector_enabled(env)) {
-flags =  FIELD_DP32(flags, TB_FLAGS, VS,
-get_field(env->mstatus, MSTATUS_VS));
-}
+fs = get_field(env->mstatus, MSTATUS_FS);
+vs = get_field(env->mstatus, MSTATUS_VS);
 
 if (riscv_has_ext(env, RVH)) {
 if (env->priv == PRV_M ||
@@ -100,19 +94,23 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
 flags = FIELD_DP32(flags, TB_FLAGS, HLSX, 1);
 }
 
-flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_FS,
-   get_field(env->mstatus_hs, MSTATUS_FS));
-
-flags = FIELD_DP32(flags, TB_FLAGS, MSTATUS_HS_VS,
-   get_field(env->mstatus_hs, MSTATUS_VS));
-flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED,
-   get_field(env->virt, VIRT_ONOFF));
+if (riscv_cpu_virt_enabled(env)) {
+flags = FIELD_DP32(flags, TB_FLAGS, VIRT_ENABLED, 1);
+/*
+ * Merge DISABLED and !DIRTY states using MIN.
+ * We will set both fields when dirtying.
+ */
+fs = MIN(fs, get_field(env->mstatus_hs, MSTATUS_FS));
+vs = MIN(vs, get_field(env->mstatus_hs, MSTATUS_VS));
+}
 }
 if (cpu->cfg.debug && !icount_enabled()) {
 flags = FIELD_DP32(flags, TB_FLAGS, ITRIGGER, env->itrigger_enabled);
 }
 #endif
 
+flags = FIELD_DP32(flags, TB_FLAGS, FS, fs);
+flags = FIELD_DP32(flags, TB_FLAGS, VS, vs);
 flags = FIELD_DP32(flags, TB_FLAGS, XL, env->xl);
 if (env->cur_pmmask < (env->xl == MXL_RV32 ? UINT32_MAX : UINT64_MAX)) {
 flags = FIELD_DP32(flags, TB_FLAGS, PM_MASK_ENABLED, 1);
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index b897bf6006..74d0b9889d 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -66,8 +66,6 @@ typedef struct DisasContext {
 uint32_t opcode;
 RISCVExtStatus mstatus_fs;
 RISCVExtStatus mstatus_vs;
-RISCVExtStatus mstatus_hs_fs;
-RISCVExtStatus mstatus_hs_vs;
 uint32_t mem_idx;
 /* Remember the rounding mode encoded in the previous fp instruction,
which we have already installed into env->fp_status.  Or -1 for
@@ -618,16 +616,12 @@ static void mark_fs_dirty(DisasContext *ctx)
 tcg_gen_ld_tl(tmp, cpu_env, offsetof(CPURISCVState, mstatus));
 tcg_gen_ori_tl(tmp, tmp, MSTATUS_FS);
 tcg_gen_st_tl(tmp, cpu_env, offsetof(CPUR

[PATCH v6 17/25] target/riscv: Check SUM in the correct register

[Richard Henderson]

Table 9.5 "Effect of MPRV..." specifies that MPV=1 uses VS-level
vsstatus.SUM instead of HS-level sstatus.SUM.

For HLV/HSV instructions, the HS-level register does not apply, but
the VS-level register presumably does, though this is not mentioned
explicitly in the manual.  However, it matches the behavior for MPV.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 12 
 target/riscv/op_helper.c  |  6 +-
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 6c42f9c6fd..0017ecbf37 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -42,11 +42,16 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
 
 /* All priv -> mmu_idx mapping are here */
 if (!ifetch) {
-if (mode == PRV_M && get_field(env->mstatus, MSTATUS_MPRV)) {
+uint64_t status = env->mstatus;
+
+if (mode == PRV_M && get_field(status, MSTATUS_MPRV)) {
 mode = get_field(env->mstatus, MSTATUS_MPP);
 virt = get_field(env->mstatus, MSTATUS_MPV);
+if (virt) {
+status = env->vsstatus;
+}
 }
-if (mode == PRV_S && get_field(env->mstatus, MSTATUS_SUM)) {
+if (mode == PRV_S && get_field(status, MSTATUS_SUM)) {
 mode = MMUIdx_S_SUM;
 }
 }
@@ -838,8 +843,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 }
 widened = 2;
 }
-/* status.SUM will be ignored if execute on background */
-sum = mmuidx_sum(mmu_idx) || use_background || is_debug;
+sum = mmuidx_sum(mmu_idx) || is_debug;
 switch (vm) {
 case VM_1_10_SV32:
   levels = 2; ptidxbits = 10; ptesize = 4; break;
diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
index db7252e09d..93d4ae8b3e 100644
--- a/target/riscv/op_helper.c
+++ b/target/riscv/op_helper.c
@@ -437,7 +437,11 @@ static int check_access_hlsv(CPURISCVState *env, bool x, 
uintptr_t ra)
 riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST, ra);
 }
 
-return get_field(env->hstatus, HSTATUS_SPVP) | MMU_2STAGE_BIT;
+int mode = get_field(env->hstatus, HSTATUS_SPVP);
+if (!x && mode == PRV_S && get_field(env->vsstatus, MSTATUS_SUM)) {
+mode = MMUIdx_S_SUM;
+}
+return mode | MMU_2STAGE_BIT;
 }
 
 target_ulong helper_hyp_hlv_bu(CPURISCVState *env, target_ulong addr)
-- 
2.34.1

[PATCH v6 21/25] target/riscv: Suppress pte update with is_debug

[Richard Henderson]

The debugger should not modify PTE_A or PTE_D.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index ce12dcec1d..b26840e46c 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1015,7 +1015,7 @@ restart:
 (access_type == MMU_DATA_STORE ? PTE_D : 0);
 
 /* Page table updates need to be atomic with MTTCG enabled */
-if (updated_pte != pte) {
+if (updated_pte != pte && !is_debug) {
 if (!hade) {
 return TRANSLATE_FAIL;
 }
-- 
2.34.1

[PATCH v6 25/25] target/riscv: Reorg sum check in get_physical_address

[Richard Henderson]

Implement this by adjusting prot, which reduces the set of
checks required.  This prevents exec to be set for U pages
in MMUIdx_S_SUM.  While it had been technically incorrect,
it did not manifest as a bug, because we will never attempt
to execute from MMUIdx_S_SUM.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 725ca45106..7336d1273b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -800,7 +800,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 *ret_prot = 0;
 
 hwaddr base;
-int levels, ptidxbits, ptesize, vm, sum, widened;
+int levels, ptidxbits, ptesize, vm, widened;
 
 if (first_stage == true) {
 if (use_background) {
@@ -831,7 +831,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 }
 widened = 2;
 }
-sum = mmuidx_sum(mmu_idx);
+
 switch (vm) {
 case VM_1_10_SV32:
   levels = 2; ptidxbits = 10; ptesize = 4; break;
@@ -999,15 +999,15 @@ restart:
 prot |= PAGE_EXEC;
 }
 
-if ((pte & PTE_U) &&
-((mode != PRV_U) && (!sum || access_type == MMU_INST_FETCH))) {
-/*
- * User PTE flags when not U mode and mstatus.SUM is not set,
- * or the access type is an instruction fetch.
- */
-return TRANSLATE_FAIL;
-}
-if (!(pte & PTE_U) && (mode != PRV_S)) {
+if (pte & PTE_U) {
+if (mode != PRV_U) {
+if (!mmuidx_sum(mmu_idx)) {
+return TRANSLATE_FAIL;
+}
+/* SUM allows only read+write, not execute. */
+prot &= PAGE_READ | PAGE_WRITE;
+}
+} else if (mode != PRV_S) {
 /* Supervisor PTE flags when not S mode */
 return TRANSLATE_FAIL;
 }
-- 
2.34.1

Re: [PATCH 6/8] target/riscv: Fix format for indentation

[LIU Zhiwei]

On 2023/3/24 20:38, Weiwei Li wrote:

Fix identation problems, and try to use the same indentation strategy
in the same file.

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
---
  target/riscv/arch_dump.c|   4 +-
  target/riscv/cpu.c  |   4 +-
  target/riscv/cpu_helper.c   |  15 +--
  target/riscv/insn_trans/trans_rvv.c.inc |  46 
  target/riscv/op_helper.c|   4 +-
  target/riscv/pmp.c  |  19 ++--
  target/riscv/pmp.h  |   9 +-
  target/riscv/vector_helper.c| 134 +---
  8 files changed, 122 insertions(+), 113 deletions(-)

diff --git a/target/riscv/arch_dump.c b/target/riscv/arch_dump.c
index 736a232956..573587810e 100644
--- a/target/riscv/arch_dump.c
+++ b/target/riscv/arch_dump.c
@@ -180,8 +180,8 @@ int cpu_get_dump_info(ArchDumpInfo *info,
  info->d_class = ELFCLASS32;
  #endif
  
-info->d_endian = (env->mstatus & MSTATUS_UBE) != 0

- ? ELFDATA2MSB : ELFDATA2LSB;
+info->d_endian = (env->mstatus & MSTATUS_UBE) != 0 ?
+ ELFDATA2MSB : ELFDATA2LSB;
  
  return 0;

  }
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 16e465a0ab..75dab70ba7 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -344,8 +344,8 @@ static void riscv_any_cpu_init(Object *obj)
  
  #ifndef CONFIG_USER_ONLY

  set_satp_mode_max_supported(RISCV_CPU(obj),
-riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ?
-VM_1_10_SV32 : VM_1_10_SV57);
+riscv_cpu_mxl(&RISCV_CPU(obj)->env) == MXL_RV32 ?
+VM_1_10_SV32 : VM_1_10_SV57);
  #endif
  
  set_priv_version(env, PRIV_VERSION_1_12_0);

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index de2d4a8c1d..08689ee3f6 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -68,12 +68,12 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong 
*pc,
  flags = FIELD_DP32(flags, TB_FLAGS, VILL, env->vill);
  flags = FIELD_DP32(flags, TB_FLAGS, SEW, sew);
  flags = FIELD_DP32(flags, TB_FLAGS, LMUL,
-FIELD_EX64(env->vtype, VTYPE, VLMUL));
+   FIELD_EX64(env->vtype, VTYPE, VLMUL));
  flags = FIELD_DP32(flags, TB_FLAGS, VL_EQ_VLMAX, vl_eq_vlmax);
  flags = FIELD_DP32(flags, TB_FLAGS, VTA,
-FIELD_EX64(env->vtype, VTYPE, VTA));
+   FIELD_EX64(env->vtype, VTYPE, VTA));
  flags = FIELD_DP32(flags, TB_FLAGS, VMA,
-FIELD_EX64(env->vtype, VTYPE, VMA));
+   FIELD_EX64(env->vtype, VTYPE, VMA));
  } else {
  flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1);
  }
@@ -1056,7 +1056,7 @@ restart:
  /* add write permission on stores or if the page is already dirty,
 so that we TLB miss on later writes to update the dirty bit */
  if ((pte & PTE_W) &&
-(access_type == MMU_DATA_STORE || (pte & PTE_D))) {
+(access_type == MMU_DATA_STORE || (pte & PTE_D))) {
  *prot |= PAGE_WRITE;
  }
  return TRANSLATE_SUCCESS;
@@ -1285,9 +1285,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 false);
  
  qemu_log_mask(CPU_LOG_MMU,

-"%s 2nd-stage address=%" VADDR_PRIx " ret %d physical "
-HWADDR_FMT_plx " prot %d\n",
-__func__, im_address, ret, pa, prot2);
+  "%s 2nd-stage address=%" VADDR_PRIx
+  " ret %d physical "
+  HWADDR_FMT_plx " prot %d\n",
+  __func__, im_address, ret, pa, prot2);
  
  prot &= prot2;
  
diff --git a/target/riscv/insn_trans/trans_rvv.c.inc b/target/riscv/insn_trans/trans_rvv.c.inc

index f2e3d38515..2aed66934a 100644
--- a/target/riscv/insn_trans/trans_rvv.c.inc
+++ b/target/riscv/insn_trans/trans_rvv.c.inc
@@ -238,8 +238,8 @@ static bool vext_check_store(DisasContext *s, int vd, int 
nf, uint8_t eew)
  {
  int8_t emul = eew - s->sew + s->lmul;
  return (emul >= -3 && emul <= 3) &&
-require_align(vd, emul) &&
-require_nf(vd, nf, emul);
+   require_align(vd, emul) &&
+   require_nf(vd, nf, emul);
  }
  
  /*

@@ -315,7 +315,7 @@ static bool vext_check_ld_index(DisasContext *s, int vd, 
int vs2,
  int8_t seg_vd;
  int8_t emul = eew - s->sew + s->lmul;
  bool ret = vext_check_st_index(s, vd, vs2, nf, eew) &&
-require_vm(vm, vd);
+   require_vm(vm, vd);
  
  /* Each segment register group has to follow overlap rules. */

  for (int i = 0; i < nf; ++i) {
@@ -345,8 +345,8 @@ static bool vext_check_ld_index(DisasContext *s, int vd, 
int vs2,
  static bool vext_check

Re: [PATCH v1 00/11] *** add allwinner-r40 support ***

[Strahinja Jankovic]

Hi,

It's great that you are adding support for a new SoC/Board!

On Tue, Mar 21, 2023 at 11:25 AM  wrote:
>
> From: qianfan Zhao 
>
> *** history ***
>
> # v1: 2023-03-21
>
> The first version which add allwinner-r40 support, supported features:
>
> + ccu
> + dram controller
> + uart
> + i2c and pmic(axp221)
> + sdcard
> + emac/gmac
>
> Also provide a test case under avocado, running quickly test:
>
> $ AVOCADO_ALLOW_LARGE_STORAGE=yes tests/venv/bin/avocado \
> --verbose --show=app,console run -t machine:bpim2u \
> ../tests/avocado/boot_linux_console.py

I tried running this on the latest QEMU source and compilation fails with

[1758/2912] Compiling C object libqemu-arm-softmmu.fa.p/hw_arm_bananapi_m2u.c.o
FAILED: libqemu-arm-softmmu.fa.p/hw_arm_bananapi_m2u.c.o
cc -m64 -mcx16 -Ilibqemu-arm-softmmu.fa.p -I. -I../.. -Itarget/arm
-I../../target/arm -Iqapi -Itrace -Iui -Iui/shader
-I/usr/include/pixman-1 -I/usr/include/capstone
-I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
-isystem /home/strahinja/work/qemu/linux-headers -isystem
linux-headers -iquote . -iquote /home/strahinja/work/qemu -iquote
/home/strahinja/work/qemu/include -iquote
/home/strahinja/work/qemu/tcg/i386 -pthread -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
-D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef
-Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes
-Wredundant-decls -Wold-style-declaration -Wold-style-definition
-Wtype-limits -Wformat-security -Wformat-y2k -Winit-self
-Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels
-Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wmissing-format-attribute -Wno-missing-include-dirs
-Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE
-isystem../../linux-headers -isystemlinux-headers -DNEED_CPU_H
'-DCONFIG_TARGET="arm-softmmu-config-target.h"'
'-DCONFIG_DEVICES="arm-softmmu-config-devices.h"' -MD -MQ
libqemu-arm-softmmu.fa.p/hw_arm_bananapi_m2u.c.o -MF
libqemu-arm-softmmu.fa.p/hw_arm_bananapi_m2u.c.o.d -o
libqemu-arm-softmmu.fa.p/hw_arm_bananapi_m2u.c.o -c
../../hw/arm/bananapi_m2u.c
../../hw/arm/bananapi_m2u.c: In function ‘mmc_attach_drive’:
../../hw/arm/bananapi_m2u.c:41:9: error: implicit declaration of
function ‘error_report’; did you mean ‘error_report_err’?
[-Werror=implicit-function-declaration]
   41 | error_report("No SD bus found in SOC object");
  | ^~~~
  | error_report_err
../../hw/arm/bananapi_m2u.c:41:9: error: nested extern declaration of
‘error_report’ [-Werror=nested-externs]
cc1: all warnings being treated as errors

After adding

#include "qemu/error-report.h"

to hw/arm/bananapi_m2u.c, it passed and I was able to run avocado
tests for this board, as well as for cubieboard and orangepi-pc.

* Bananapi

 AVOCADO_ALLOW_LARGE_STORAGE=yes avocado --verbose
--show=app,console run -t machine:bpim2u
tests/avocado/boot_linux_console.py
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_bpim2u
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_bpim2u_initrd
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_bpim2u_initrd
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_bpim2u_gmac
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_bpim2u_gmac
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_bpim2u_openwrt_22_03_3
JOB ID : ed47ffc59b1f9ceb8b68a87405040d3af2fb9156
JOB LOG: 
/home/strahinja/avocado/job-results/job-2023-03-25T15.24-ed47ffc/job.log
...
 RESULTS: PASS 4 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT
0 | CANCEL 0
JOB TIME   : 70.79 s

* Cubieboard

 AVOCADO_ALLOW_LARGE_STORAGE=yes avocado --verbose
--show=app,console run -t machine:cubieboard
tests/avocado/boot_linux_console.py
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_initrd
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_sata
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_sata
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_cubieboard_openwrt_22_03_2
JOB ID : 1510a9b5f36b74dc1378d50dbb40553e6fb76316
JOB LOG: 
/home/strahinja/avocado/job-results/job-2023-03-25T15.27-1510a9b/job.log
...
 RESULTS: PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT
0 | CANCEL 0
JOB TIME   : 42.69 s

* OrangePi PC

AVOCADO_ALLOW_LARGE_STORAGE=yes avocado --verbose
--show=app,console run -t machine:orangepi-pc
tests/avocado/boot_linux_console.py
Fetching asset from
tests/avocado/boot_linux_console.py:BootLinuxConsole.test_arm_o

[PATCH 1/1] target/riscv: Convert env->virt to a bool env->virt_enabled

[LIU Zhiwei]

Currently we only use the env->virt to encode the virtual mode enabled
status. Let's make it a bool type.

Signed-off-by: LIU Zhiwei 
---
 target/riscv/cpu.h| 2 +-
 target/riscv/cpu_bits.h   | 3 ---
 target/riscv/cpu_helper.c | 6 +++---
 target/riscv/machine.c| 6 +++---
 target/riscv/translate.c  | 4 ++--
 5 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 638e47c75a..3c8041c5a4 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -183,7 +183,7 @@ struct CPUArchState {
 #ifndef CONFIG_USER_ONLY
 target_ulong priv;
 /* This contains QEMU specific information about the virt state. */
-target_ulong virt;
+bool virt_enabled;
 target_ulong geilen;
 uint64_t resetvec;
 
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index fca7ef0cef..45ddb00aa5 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -607,9 +607,6 @@ typedef enum {
 #define PRV_H 2 /* Reserved */
 #define PRV_M 3
 
-/* Virtulisation Register Fields */
-#define VIRT_ONOFF  1
-
 /* RV32 satp CSR field masks */
 #define SATP32_MODE 0x8000
 #define SATP32_ASID 0x7fc0
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f88c503cf4..56f4ff9ccc 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -564,7 +564,7 @@ bool riscv_cpu_virt_enabled(CPURISCVState *env)
 return false;
 }
 
-return get_field(env->virt, VIRT_ONOFF);
+return env->virt_enabled;
 }
 
 void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable)
@@ -574,11 +574,11 @@ void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool 
enable)
 }
 
 /* Flush the TLB on all virt mode changes. */
-if (get_field(env->virt, VIRT_ONOFF) != enable) {
+if (env->virt_enabled != enable) {
 tlb_flush(env_cpu(env));
 }
 
-env->virt = set_field(env->virt, VIRT_ONOFF, enable);
+env->virt_enabled = enable;
 
 if (enable) {
 /*
diff --git a/target/riscv/machine.c b/target/riscv/machine.c
index 9c455931d8..0fb3ddda06 100644
--- a/target/riscv/machine.c
+++ b/target/riscv/machine.c
@@ -331,8 +331,8 @@ static const VMStateDescription vmstate_pmu_ctr_state = {
 
 const VMStateDescription vmstate_riscv_cpu = {
 .name = "cpu",
-.version_id = 7,
-.minimum_version_id = 7,
+.version_id = 8,
+.minimum_version_id = 8,
 .post_load = riscv_cpu_post_load,
 .fields = (VMStateField[]) {
 VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
@@ -352,7 +352,7 @@ const VMStateDescription vmstate_riscv_cpu = {
 VMSTATE_UINT32(env.misa_mxl_max, RISCVCPU),
 VMSTATE_UINT32(env.misa_ext_mask, RISCVCPU),
 VMSTATE_UINTTL(env.priv, RISCVCPU),
-VMSTATE_UINTTL(env.virt, RISCVCPU),
+VMSTATE_BOOL(env.virt_enabled, RISCVCPU),
 VMSTATE_UINT64(env.resetvec, RISCVCPU),
 VMSTATE_UINTTL(env.mhartid, RISCVCPU),
 VMSTATE_UINT64(env.mstatus, RISCVCPU),
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0ee8ee147d..c3adf30b54 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1255,8 +1255,8 @@ static void riscv_tr_disas_log(const DisasContextBase 
*dcbase,
 
 fprintf(logfile, "IN: %s\n", lookup_symbol(dcbase->pc_first));
 #ifndef CONFIG_USER_ONLY
-fprintf(logfile, "Priv: "TARGET_FMT_ld"; Virt: "TARGET_FMT_ld"\n",
-env->priv, env->virt);
+fprintf(logfile, "Priv: "TARGET_FMT_ld"; Virt: %d\n",
+env->priv, env->virt_enabled);
 #endif
 target_disas(logfile, cpu, dcbase->pc_first, dcbase->tb->size);
 }
-- 
2.17.1

Re: [PATCH 0/8] target/riscv: Simplification for RVH related check and code style fix

[LIU Zhiwei]

On 2023/3/24 20:38, Weiwei Li wrote:

This patchset tries to simplify the RVH related check and fix some code style 
problems, such as problems for indentation, multi-line comments and lines with 
over 80 characters.


This patch set looks good to me, except a small comment on patch 
6(target/riscv: Fix format for indentation).


I have sent a patch to convert the env->virt to a bool type.

https://lists.gnu.org/archive/html/qemu-devel/2023-03/msg06191.html

With this patch and your patch 3(target/riscv: Remove check on RVH for 
riscv_cpu_virt_enabled), I think we can remove the riscv_cpu_virt_enabled

which has been called so many times.

you can pick it up into this patch set if you desire.

No matter what you choose, after small fix for patch 6,  for this whole 
patch set


Reviewed-by: LIU Zhiwei 

Zhiwei



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

Weiwei Li (8):
   target/riscv: Remove redundant call to riscv_cpu_virt_enabled
   target/riscv: Remove redundant check on RVH
   target/riscv: Remove check on RVH for riscv_cpu_virt_enabled
   target/riscv: Remove check on RVH for riscv_cpu_set_virt_enabled
   target/riscv: Remove redundant parentheses
   target/riscv: Fix format for indentation
   target/riscv: Fix format for comments
   target/riscv: Fix lines with over 80 characters

  target/riscv/arch_dump.c|   7 +-
  target/riscv/cpu.c  |   6 +-
  target/riscv/cpu.h  |  26 ++-
  target/riscv/cpu_bits.h |   2 +-
  target/riscv/cpu_helper.c   |  86 ---
  target/riscv/csr.c  |   6 +-
  target/riscv/insn_trans/trans_rvv.c.inc |  54 ++---
  target/riscv/op_helper.c|   7 +-
  target/riscv/pmp.c  |  48 ++--
  target/riscv/pmp.h  |   9 +-
  target/riscv/pmu.c  |   3 +-
  target/riscv/sbi_ecall_interface.h  |   8 +-
  target/riscv/translate.c|   8 +-
  target/riscv/vector_helper.c| 292 ++--
  14 files changed, 316 insertions(+), 246 deletions(-)

[PATCH v3] block: replace TABs with space

[Yeqi Fu]

Bring the block files in line with the QEMU coding style, with spaces
for indentation. This patch partially resolves the issue 371.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/371
Signed-off-by: Yeqi Fu 
---
 block/bochs.c   |  18 +++
 block/file-posix.c  | 126 ++--
 block/file-win32.c  |  66 +++
 block/parallels.c   |  34 ++--
 block/qcow.c|  44 
 include/block/nbd.h |   2 +-
 6 files changed, 145 insertions(+), 145 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 2f5ae52c90..18f48d2f89 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -293,15 +293,15 @@ static void bochs_close(BlockDriverState *bs)
 }
 
 static BlockDriver bdrv_bochs = {
-.format_name   = "bochs",
-.instance_size = sizeof(BDRVBochsState),
-.bdrv_probe= bochs_probe,
-.bdrv_open = bochs_open,
-.bdrv_child_perm = bdrv_default_perms,
-.bdrv_refresh_limits = bochs_refresh_limits,
-.bdrv_co_preadv = bochs_co_preadv,
-.bdrv_close= bochs_close,
-.is_format  = true,
+.format_name= "bochs",
+.instance_size  = sizeof(BDRVBochsState),
+.bdrv_probe = bochs_probe,
+.bdrv_open  = bochs_open,
+.bdrv_child_perm= bdrv_default_perms,
+.bdrv_refresh_limits= bochs_refresh_limits,
+.bdrv_co_preadv = bochs_co_preadv,
+.bdrv_close = bochs_close,
+.is_format  = true,
 };
 
 static void bdrv_bochs_init(void)
diff --git a/block/file-posix.c b/block/file-posix.c
index 5760cf22d1..bdebe0c9a6 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -124,7 +124,7 @@
 #define FTYPE_FILE   0
 #define FTYPE_CD 1
 
-#define MAX_BLOCKSIZE  4096
+#define MAX_BLOCKSIZE 4096
 
 /* Posix file locking bytes. Libvirt takes byte 0, we start from higher bytes,
  * leaving a few more bytes for its future use. */
@@ -3819,42 +3819,42 @@ static void coroutine_fn 
cdrom_co_lock_medium(BlockDriverState *bs, bool locked)
 }
 
 static BlockDriver bdrv_host_cdrom = {
-.format_name= "host_cdrom",
-.protocol_name  = "host_cdrom",
-.instance_size  = sizeof(BDRVRawState),
-.bdrv_needs_filename = true,
-.bdrv_probe_device = cdrom_probe_device,
-.bdrv_parse_filename = cdrom_parse_filename,
-.bdrv_file_open = cdrom_open,
-.bdrv_close = raw_close,
-.bdrv_reopen_prepare = raw_reopen_prepare,
-.bdrv_reopen_commit  = raw_reopen_commit,
-.bdrv_reopen_abort   = raw_reopen_abort,
-.bdrv_co_create_opts = bdrv_co_create_opts_simple,
-.create_opts = &bdrv_create_opts_simple,
-.mutable_opts= mutable_opts,
-.bdrv_co_invalidate_cache = raw_co_invalidate_cache,
-
-.bdrv_co_preadv = raw_co_preadv,
-.bdrv_co_pwritev= raw_co_pwritev,
-.bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
-.bdrv_refresh_limits = raw_refresh_limits,
-.bdrv_co_io_plug= raw_co_io_plug,
-.bdrv_co_io_unplug  = raw_co_io_unplug,
-.bdrv_attach_aio_context = raw_aio_attach_aio_context,
-
-.bdrv_co_truncate   = raw_co_truncate,
-.bdrv_co_getlength  = raw_co_getlength,
-.has_variable_length= true,
-.bdrv_co_get_allocated_file_size= raw_co_get_allocated_file_size,
+.format_name = "host_cdrom",
+.protocol_name   = "host_cdrom",
+.instance_size   = sizeof(BDRVRawState),
+.bdrv_needs_filename = true,
+.bdrv_probe_device   = cdrom_probe_device,
+.bdrv_parse_filename = cdrom_parse_filename,
+.bdrv_file_open  = cdrom_open,
+.bdrv_close  = raw_close,
+.bdrv_reopen_prepare = raw_reopen_prepare,
+.bdrv_reopen_commit  = raw_reopen_commit,
+.bdrv_reopen_abort   = raw_reopen_abort,
+.bdrv_co_create_opts = bdrv_co_create_opts_simple,
+.create_opts = &bdrv_create_opts_simple,
+.mutable_opts= mutable_opts,
+.bdrv_co_invalidate_cache= raw_co_invalidate_cache,
+
+.bdrv_co_preadv  = raw_co_preadv,
+.bdrv_co_pwritev = raw_co_pwritev,
+.bdrv_co_flush_to_disk   = raw_co_flush_to_disk,
+.bdrv_refresh_limits = raw_refresh_limits,
+.bdrv_co_io_plug = raw_co_io_plug,
+.bdrv_co_io_unplug   = raw_co_io_unplug,
+.bdrv_attach_aio_context = raw_aio_attach_aio_context,
+
+.bdrv_co_truncate= raw_co_truncate,
+.bdrv_co_getlength   = raw_co_getlength,
+.has_variable_length = true,
+.bdrv_co_get_allocated_file_size = raw_co_get_allocated_file_size,
 
 /* removabl

[PATCH v6 20/25] target/riscv: Move leaf pte processing out of level loop

[Richard Henderson]

Move the code that never loops outside of the loop.
Unchain the if-return-else statements.

Signed-off-by: Richard Henderson 
---
 target/riscv/cpu_helper.c | 234 +-
 1 file changed, 127 insertions(+), 107 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 00f70a3dd5..ce12dcec1d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -879,6 +879,8 @@ static int get_physical_address(CPURISCVState *env, hwaddr 
*physical,
 }
 
 int ptshift = (levels - 1) * ptidxbits;
+target_ulong pte;
+hwaddr pte_addr;
 int i;
 
 #if !TCG_OVERSIZED_GUEST
@@ -895,7 +897,6 @@ restart:
 }
 
 /* check that physical address of PTE is legal */
-hwaddr pte_addr;
 
 if (two_stage && first_stage) {
 int vbase_prot;
@@ -927,7 +928,6 @@ restart:
 return TRANSLATE_PMP_FAIL;
 }
 
-target_ulong pte;
 if (riscv_cpu_mxl(env) == MXL_RV32) {
 pte = address_space_ldl(cs->as, pte_addr, attrs, &res);
 } else {
@@ -952,120 +952,140 @@ restart:
 if (!(pte & PTE_V)) {
 /* Invalid PTE */
 return TRANSLATE_FAIL;
-} else if (!pbmte && (pte & PTE_PBMT)) {
+}
+if (pte & (PTE_R | PTE_W | PTE_X)) {
+goto leaf;
+}
+
+/* Inner PTE, continue walking */
+if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
 return TRANSLATE_FAIL;
-} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
-/* Inner PTE, continue walking */
-if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
-return TRANSLATE_FAIL;
-}
-base = ppn << PGSHIFT;
-} else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
-/* Reserved leaf PTE flags: PTE_W */
-return TRANSLATE_FAIL;
-} else if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
-/* Reserved leaf PTE flags: PTE_W + PTE_X */
-return TRANSLATE_FAIL;
-} else if ((pte & PTE_U) && ((mode != PRV_U) &&
-   (!sum || access_type == MMU_INST_FETCH))) {
-/* User PTE flags when not U mode and mstatus.SUM is not set,
-   or the access type is an instruction fetch */
-return TRANSLATE_FAIL;
-} else if (!(pte & PTE_U) && (mode != PRV_S)) {
-/* Supervisor PTE flags when not S mode */
-return TRANSLATE_FAIL;
-} else if (ppn & ((1ULL << ptshift) - 1)) {
-/* Misaligned PPN */
-return TRANSLATE_FAIL;
-} else if (access_type == MMU_DATA_LOAD && !((pte & PTE_R) ||
-   ((pte & PTE_X) && mxr))) {
-/* Read access check failed */
-return TRANSLATE_FAIL;
-} else if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
-/* Write access check failed */
-return TRANSLATE_FAIL;
-} else if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
-/* Fetch access check failed */
-return TRANSLATE_FAIL;
-} else {
-/* if necessary, set accessed and dirty bits. */
-target_ulong updated_pte = pte | PTE_A |
+}
+base = ppn << PGSHIFT;
+}
+
+/* No leaf pte at any translation level. */
+return TRANSLATE_FAIL;
+
+ leaf:
+if (ppn & ((1ULL << ptshift) - 1)) {
+/* Misaligned PPN */
+return TRANSLATE_FAIL;
+}
+if (!pbmte && (pte & PTE_PBMT)) {
+/* Reserved without Svpbmt. */
+return TRANSLATE_FAIL;
+}
+if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
+/* Reserved leaf PTE flags: PTE_W */
+return TRANSLATE_FAIL;
+}
+if ((pte & (PTE_R | PTE_W | PTE_X)) == (PTE_W | PTE_X)) {
+/* Reserved leaf PTE flags: PTE_W + PTE_X */
+return TRANSLATE_FAIL;
+}
+if ((pte & PTE_U) &&
+((mode != PRV_U) && (!sum || access_type == MMU_INST_FETCH))) {
+/*
+ * User PTE flags when not U mode and mstatus.SUM is not set,
+ * or the access type is an instruction fetch.
+ */
+return TRANSLATE_FAIL;
+}
+if (!(pte & PTE_U) && (mode != PRV_S)) {
+/* Supervisor PTE flags when not S mode */
+return TRANSLATE_FAIL;
+}
+if (access_type == MMU_DATA_LOAD &&
+!((pte & PTE_R) || ((pte & PTE_X) && mxr))) {
+/* Read access check failed */
+return TRANSLATE_FAIL;
+}
+if (access_type == MMU_DATA_STORE && !(pte & PTE_W)) {
+/* Write access check failed */
+return TRANSLATE_FAIL;
+}
+if (access_type == MMU_INST_FETCH && !(pte & PTE_X)) {
+/* Fetch access check failed */
+return TRANSLATE_FAIL;
+}
+
+/* If necessary, set accessed and dirty bits. */
+target_ulong updated_pte = pte | PTE_A |
 (access_type == MMU_DATA_STORE ? PTE_D : 0);
 
-   

Re: [PATCH v4 1/3] docs: Add support for TPM devices over I2C bus

[Stefan Berger]

On 3/25/23 00:37, Ninad Palsule wrote:

This is a documentation change for I2C TPM device support.

Qemu already supports devices attached to ISA and sysbus.
This drop adds support for the I2C bus attached TPM devices.

Signed-off-by: Ninad Palsule 

---
V2:

Incorporated Stephen's review comments
- Added example in the document.

---
V4:
Incorporate Cedric & Stefan's comments

- Added example for ast2600-evb
- Corrected statement about arm virtual machine.
---
  docs/specs/tpm.rst | 32 
  1 file changed, 32 insertions(+)

diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..a0600e2834 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface:
   - ``hw/tpm/tpm_tis_common.c``
   - ``hw/tpm/tpm_tis_isa.c``
   - ``hw/tpm/tpm_tis_sysbus.c``
+ - ``hw/tpm/tpm_tis_i2c.c``
   - ``hw/tpm/tpm_tis.h``
  
  Both an ISA device and a sysbus device are available. The former is

  used with pc/q35 machine while the latter can be instantiated in the
  Arm virt machine.
  
+An I2C device support is also added which can be instantiated in the armadded -> provided

arm -> Arm


+based emulation machines. This device only supports the TPM 2 protocol.
+
  CRB interface
  -
  
@@ -348,6 +352,34 @@ In case an Arm virt machine is emulated, use the following command line:

  -drive if=pflash,format=raw,file=flash0.img,readonly=on \
  -drive if=pflash,format=raw,file=flash1.img
  
+In case a ast2600-evb bmc machine is emulated and want to use TPM device

+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M ast2600-evb -nographic \
+-kernel arch/arm/boot/zImage \
+-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \
+-initrd rootfs.cpio \
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
+In case a Rainier bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M rainier-bmc -nographic \
+-kernel ${IMAGEPATH}/fitImage-linux.bin \
+-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
+-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
+-drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2\
+-net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443\
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
  In case SeaBIOS is used as firmware, it should show the TPM menu item
  after entering the menu with 'ESC'.
  


With the above nits:

Reviewed-by: Stefan Berger 

Re: [PATCH v4 2/3] tpm: Extend common APIs to support TPM TIS I2C

[Stefan Berger]

On 3/25/23 00:37, Ninad Palsule wrote:

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.

This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
   the I2C support. The checksum calculation is handled in the qemu
   common code.
- Added wrapper function for read and write data so that I2C code can
   call it without MMIO interface.

Signed-off-by: Ninad Palsule 
---
V2:

Incorporated Stephen's comments.

- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
   i2c layer.

---
V3:
Incorporated review comments from Cedric and Stefan.

- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h

---
V4:

Incorporated review comments by Stefan

- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
  hw/tpm/tpm_tis.h|  3 +++
  hw/tpm/tpm_tis_common.c | 28 
  include/hw/acpi/tpm.h   | 28 
  3 files changed, 59 insertions(+)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
  void tpm_tis_reset(TPMState *s);
  enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
  void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
  
  #endif /* TPM_TPM_TIS_H */

diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c6d139943e 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
  #include "hw/irq.h"
  #include "hw/isa/isa.h"
  #include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
  #include "qemu/module.h"
  
  #include "hw/acpi/tpm.h"

@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
addr,
  return val;
  }
  
+/*

+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
  /*
   * Write a value to a register of the TIS interface
   * See specs pages 33-63 for description of the registers
@@ -767,6 +786,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
  }
  }
  
+/*

+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
  const MemoryRegionOps tpm_tis_memory_ops = {
  .read = tpm_tis_mmio_read,
  .write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..4f2424e2fe 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
  #define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
  #define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
  #define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC   (1 << 8)
  #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is mandatory */
  #define TPM_TIS_CAPABILITIES_SUPPORTED1_3 \
  (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
@@ -209,6 +210,33 @@ REG32(CRB_DATA_BUFFER, 0x80)
  #define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
  #define TPM_PPI_FUNC_MASK(7 << 0)
  
+/* TPM TIS I2C registers */

+#define TPM_I2C_REG_LOC_SEL  0x00
+#define TPM_I2C_REG_ACCESS   0x04
+#define TPM_I2C_REG_INT_ENABLE   0x08
+#define TPM_I2C_REG_INT_CAPABILITY   0x14
+#define TPM_I2C_REG_STS  0x18
+#define TPM_I2C_REG_DATA_FIFO0x24
+#define TPM_I2C_REG_INTF_CAPABILITY  0x30
+#define TPM_I2C_REG_I2C_DEV_ADDRESS  0x38
+#define TPM_I2C_REG_DATA_CSUM_ENABLE 0x40
+#define TPM_I2C_REG_DATA_CSUM_GET0x44
+#define TPM_I2C_REG_DID_VID  0x48
+#define TPM_I2C_REG_RID  0x4c
+#define TPM_I2C_REG_UNKNOWN  0xff
+
+/* I2C specific interface capabilities */
+#define TPM_I2C_CAP_INTERFACE_TYPE (0x2 << 0)   /* FIFO interface */
+#define TPM_I2C_CAP_INTERFACE_VER  (0x0 << 4)   /* TCG I2C intf 1.0 */
+#define TPM_I2C_CAP_TPM2_FAMILY(0x1 << 7)   /* TPM 2.0 family. */
+#define TPM_I2C_CAP_DEV_ADDR_CHANGE(0x0 << 27)  /* No dev addr chng */
+#define TPM_I2C_CAP_BURST_COUNT_STATIC (0x1 << 29)  /* Burst count static 
*/

Re: [PULL 0/8] Misc patches for QEMU 8.0-rc2

[Peter Maydell]

On Fri, 24 Mar 2023 at 15:36, Thomas Huth  wrote:
>
> The following changes since commit 60ca584b8af0de525656f959991a440f8c191f12:
>
>   Merge tag 'pull-for-8.0-220323-1' of https://gitlab.com/stsquad/qemu into 
> staging (2023-03-22 17:58:12 +)
>
> are available in the Git repository at:
>
>   https://gitlab.com/thuth/qemu.git tags/pull-request-2023-03-24
>
> for you to fetch changes up to 8635a3a153da3a6712c4ee249c2bf3513cbfdbf7:
>
>   Revert "docs/about/deprecated: Deprecate 32-bit arm hosts for system 
> emulation" (2023-03-24 12:10:49 +0100)
>
> 
> * Remove TABs in hw/ide and hw/block
> * Two fixes for GCC 13
> * MSYS2 CI job improvements
>


Applied, thanks.

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

-- PMM

Re: [PULL 0/1] QGA warning fix

[Peter Maydell]

On Wed, 22 Mar 2023 at 19:22, Konstantin Kostiuk  wrote:
>
> From: Kostiantyn Kostiuk 
>
>
> The following changes since commit 9832009d9dd2386664c15cc70f6e6bfe062be8bd:
>
>   Merge tag 'pull-riscv-to-apply-20230306' of 
> https://gitlab.com/palmer-dabbelt/qemu into staging (2023-03-07 12:53:00 
> +)
>
> are available in the Git repository at:
>
>   g...@github.com:kostyanf14/qemu.git tags/qga-pull-2023-03-22
>
> for you to fetch changes up to 0fcd574b025fccdf14d5140687cafe2bc30b634f:
>
>   qga/vss-win32: fix warning for clang++-15 (2023-03-22 21:02:09 +0200)
>
> 
> qga-pull-2023-03-22
>
> 


Applied, thanks.

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

-- PMM

Re: [PULL 0/2] xen queue

[Peter Maydell]

On Fri, 24 Mar 2023 at 14:56, Anthony PERARD  wrote:
>
> The following changes since commit 60ca584b8af0de525656f959991a440f8c191f12:
>
>   Merge tag 'pull-for-8.0-220323-1' of https://gitlab.com/stsquad/qemu into 
> staging (2023-03-22 17:58:12 +)
>
> are available in the Git repository at:
>
>   https://xenbits.xen.org/git-http/people/aperard/qemu-dm.git 
> tags/pull-xen-20230324
>
> for you to fetch changes up to 670d8c6ebf7a2c425575bbd6fbaeb27d21edd6c6:
>
>   hw/xenpv: Initialize Xen backend operations (2023-03-24 14:52:14 +)
>
> 
> Xen queue
>
> - fix guest creation when -xen-domid-restrict is used.
> - fix Xen PV guest creation.
>
> 


Applied, thanks.

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

-- PMM

Re: [PATCH v4 3/3] tpm: Add support for TPM device over I2C bus

[Stefan Berger]

On 3/25/23 00:37, Ninad Palsule wrote:

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.

This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
   cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. User specify this
   string on command line.


The user has to provide this string on the command line.



Testing:
   TPM I2C device modulte is tested using SWTPM (software based TPM


modulte -> module


   package). The qemu used the rainier machine and it was connected to
   swtpm over the socket interface.

Qemu uses the rainier machine and is connected to swtpm over the socker 
interface.



   The command to start swtpm is as follows:
   $ swtpm socket --tpmstate dir=/tmp/mytpm1\
  --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock  \
  --tpm2 --log level=100

   The command to start qemu is as follows:
   $ qemu-system-arm -M rainier-bmc -nographic \
 -kernel ${IMAGEPATH}/fitImage-linux.bin \
 -dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
 -initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
 -drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
 -net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
 -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
 -tpmdev emulator,id=tpm0,chardev=chrtpm \
 -device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e

   Note: Currently you need to specify the I2C bus and device address on
 command line. In future we can add a device at board level.


I would remove this not. Not sure whether this can be done due to backwards 
comaptibility issues.


I tested this series on big and little endian machines and it passed the test 
cases on noth!


Signed-off-by: Ninad Palsule 
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
   capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
   layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.

---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
   remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
   is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
   function in TIS layer to return the checksum and used that to process
   the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
   write call instead of 1 byte at a time. Added functions to convert
   between little endian stream of bytes to single 32 bit unsigned
   integer. Similarly 32  bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h

---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
   tracing.
- Removed the endian conversion functions instead used simple logic
   provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
   on command line by using "-trace "tpm_tis_i2c*" option.
---
  hw/arm/Kconfig   |   1 +
  hw/tpm/Kconfig   |   7 +
  hw/tpm/meson.build   |   1 +
  hw/tpm/tpm_tis_i2c.c | 492 +++
  hw/tpm/trace-events  |   6 +
  include/sysemu/tpm.h |   3 +
  6 files changed, 510 insertions(+)
  create mode 100644 hw/tpm/tpm_tis_i2c.c

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
  imply VFIO_PLATFORM
  imply VFIO_XGMAC
  imply TPM_TIS_SYSBUS
+imply TPM_TIS_I2C
  imply NVDIMM
  select ARM_GIC
  select ACPI
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 29e82f3c92..a46663288c 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -1,3 +1,10 @@
+config TPM_TIS_I2C
+bool
+depends on TPM
+select TPM_BACKEND
+select I2C
+select TPM_TIS
+
  config TPM_TIS_ISA
  bool
  depends on TPM && ISA_BUS
diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index 7abc2d794a..76fe3cb098 100644
--- a/hw/tpm/meson.build

Re: [PATCH v4 3/3] tpm: Add support for TPM device over I2C bus

[Stefan Berger]

On 3/25/23 00:37, Ninad Palsule wrote:

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.



Your v4 patches with my 2 patches for the test cases are here now:

https://github.com/stefanberger/qemu-tpm/commits/tpm-i2c.v4

Thanks!

  Regards,
Stefan

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

[Strahinja Jankovic]

Hi,

On Tue, Mar 21, 2023 at 11:25 AM  wrote:
>
> From: qianfan Zhao 
>
> Allwinner R40 (sun8i) SoC features a Quad-Core Cortex-A7 ARM CPU,
> and a Mali400 MP2 GPU from ARM. It's also known as the Allwinner T3
> for In-Car Entertainment usage, A40i and A40pro are variants that
> differ in applicable temperatures range (industrial and military).
>
> This patch is a draft and provides very few features that we will
> improve late.
>
> Signed-off-by: qianfan Zhao 

Please run the `scripts/checkpatch.pl` on all patches before
resubmitting, there are several coding style items that should be
fixed.

> ---
>  configs/devices/arm-softmmu/default.mak |   1 +
>  hw/arm/Kconfig  |   9 +
>  hw/arm/allwinner-r40.c  | 425 
>  hw/arm/bananapi_m2u.c   | 116 +++
>  hw/arm/meson.build  |   1 +
>  include/hw/arm/allwinner-r40.h  | 111 +++
>  6 files changed, 663 insertions(+)
>  create mode 100644 hw/arm/allwinner-r40.c
>  create mode 100644 hw/arm/bananapi_m2u.c
>  create mode 100644 include/hw/arm/allwinner-r40.h
>
> diff --git a/configs/devices/arm-softmmu/default.mak 
> b/configs/devices/arm-softmmu/default.mak
> index 1b49a7830c..76a43add23 100644
> --- a/configs/devices/arm-softmmu/default.mak
> +++ b/configs/devices/arm-softmmu/default.mak
> @@ -43,3 +43,4 @@ CONFIG_FSL_IMX6UL=y
>  CONFIG_SEMIHOSTING=y
>  CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y
>  CONFIG_ALLWINNER_H3=y
> +CONFIG_ALLWINNER_R40=y
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index b5aed4aff5..9e14c3427e 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -344,6 +344,15 @@ config ALLWINNER_H3
>  select USB_EHCI_SYSBUS
>  select SD
>
> +config ALLWINNER_R40
> +bool
> +select ALLWINNER_A10_PIT
> +select SERIAL
> +select ARM_TIMER
> +select ARM_GIC
> +select UNIMP
> +select SD
> +
>  config RASPI
>  bool
>  select FRAMEBUFFER
> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> new file mode 100644
> index 00..d0516f4e96
> --- /dev/null
> +++ b/hw/arm/allwinner-r40.c
> @@ -0,0 +1,425 @@
> +/*
> + * Allwinner R40/A40i/T3 System on Chip emulation
> + *
> + * Copyright (C) 2023 qianfan Zhao 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "qemu/error-report.h"
> +#include "qemu/bswap.h"
> +#include "qemu/module.h"
> +#include "qemu/units.h"
> +#include "hw/qdev-core.h"
> +#include "hw/sysbus.h"
> +#include "hw/char/serial.h"
> +#include "hw/misc/unimp.h"
> +#include "hw/usb/hcd-ehci.h"
> +#include "hw/loader.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/arm/allwinner-r40.h"
> +
> +/* Memory map */
> +const hwaddr allwinner_r40_memmap[] = {
> +[AW_R40_DEV_SRAM_A1]= 0x,
> +[AW_R40_DEV_SRAM_A2]= 0x4000,
> +[AW_R40_DEV_SRAM_A3]= 0x8000,
> +[AW_R40_DEV_SRAM_A4]= 0xb400,
> +[AW_R40_DEV_MMC0]   = 0x01c0f000,
> +[AW_R40_DEV_MMC1]   = 0x01c1,
> +[AW_R40_DEV_MMC2]   = 0x01c11000,
> +[AW_R40_DEV_MMC3]   = 0x01c12000,
> +[AW_R40_DEV_PIT]= 0x01c20c00,
> +[AW_R40_DEV_UART0]  = 0x01c28000,
> +[AW_R40_DEV_GIC_DIST]   = 0x01c81000,
> +[AW_R40_DEV_GIC_CPU]= 0x01c82000,
> +[AW_R40_DEV_GIC_HYP]= 0x01c84000,
> +[AW_R40_DEV_GIC_VCPU]   = 0x01c86000,
> +[AW_R40_DEV_SDRAM]  = 0x4000
> +};
> +
> +/* List of unimplemented devices */
> +struct AwR40Unimplemented {
> +const char *device_name;
> +hwaddr base;
> +hwaddr size;
> +};
> +
> +static struct AwR40Unimplemented r40_unimplemented[] = {
> +{ "d-engine",   0x0100, 4 * MiB },
> +{ "d-inter",0x0140, 128 * KiB },
> +{ "sram-c", 0x01c0, 4 * KiB },
> +{ "dma",0x01c02000, 4 * KiB },
> +{ "nfdc",   0x01c03000, 4 * KiB },
> +{ "ts", 0x01c04000, 4 * KiB },
> +{ "spi0",   0x01c05000, 4 * KiB },
> +{ "spi1",   0x01c06000, 4 * KiB },
> +{ "cs0",0x01c09000, 4 * KiB },
> +{ "keymem", 0x01c0a000, 4 * KiB },
> +{ "emac",   0x01c0b000, 4 * KiB },
> +{ "usb0-otg",   0x01c13000, 4 * KiB },
> +{ "usb0-host",  0x01c14000, 4 * KiB },
> +{ "crypto", 0x01c15000, 4 * KiB },
> +{ "spi2",

Re: [PATCH v1 02/11] hw/arm/allwinner-r40: add Clock Control Unit

[Strahinja Jankovic]

Hi,

On Tue, Mar 21, 2023 at 11:25 AM  wrote:
>
> From: qianfan Zhao 
>
> The CCU provides the registers to program the PLLs and the controls
> most of the clock generation, division, distribution, synchronization
> and gating.
>
> This commit adds support for the Clock Control Unit which emulates
> a simple read/write register interface.
>
> Signed-off-by: qianfan Zhao 
> ---
>  hw/arm/allwinner-r40.c  |   7 +-
>  hw/misc/allwinner-r40-ccu.c | 207 
>  hw/misc/meson.build |   1 +
>  include/hw/arm/allwinner-r40.h  |   2 +
>  include/hw/misc/allwinner-r40-ccu.h |  65 +
>  5 files changed, 281 insertions(+), 1 deletion(-)
>  create mode 100644 hw/misc/allwinner-r40-ccu.c
>  create mode 100644 include/hw/misc/allwinner-r40-ccu.h
>
> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> index d0516f4e96..3517682aed 100644
> --- a/hw/arm/allwinner-r40.c
> +++ b/hw/arm/allwinner-r40.c
> @@ -42,6 +42,7 @@ const hwaddr allwinner_r40_memmap[] = {
>  [AW_R40_DEV_MMC1]   = 0x01c1,
>  [AW_R40_DEV_MMC2]   = 0x01c11000,
>  [AW_R40_DEV_MMC3]   = 0x01c12000,
> +[AW_R40_DEV_CCU]= 0x01c2,
>  [AW_R40_DEV_PIT]= 0x01c20c00,
>  [AW_R40_DEV_UART0]  = 0x01c28000,
>  [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
> @@ -80,7 +81,6 @@ static struct AwR40Unimplemented r40_unimplemented[] = {
>  { "usb2-host",  0x01c1c000, 4 * KiB },
>  { "cs1",0x01c1d000, 4 * KiB },
>  { "spi3",   0x01c1f000, 4 * KiB },
> -{ "ccu",0x01c2, 1 * KiB },
>  { "rtc",0x01c20400, 1 * KiB },
>  { "pio",0x01c20800, 1 * KiB },
>  { "owa",0x01c21000, 1 * KiB },
> @@ -246,6 +246,7 @@ static void allwinner_r40_init(Object *obj)
>  object_property_add_alias(obj, "clk1-freq", OBJECT(&s->timer),
>"clk1-freq");
>
> +object_initialize_child(obj, "ccu", &s->ccu, TYPE_AW_R40_CCU);
>  object_initialize_child(obj, "mmc0", &s->mmc0, TYPE_AW_SDHOST_SUN5I);
>  object_initialize_child(obj, "mmc1", &s->mmc1, TYPE_AW_SDHOST_SUN5I);
>  object_initialize_child(obj, "mmc2", &s->mmc2, TYPE_AW_SDHOST_SUN5I);
> @@ -358,6 +359,10 @@ static void allwinner_r40_realize(DeviceState *dev, 
> Error **errp)
>  memory_region_add_subregion(get_system_memory(), 
> s->memmap[AW_R40_DEV_SRAM_A4],
>  &s->sram_a4);
>
> +/* Clock Control Unit */
> +sysbus_realize(SYS_BUS_DEVICE(&s->ccu), &error_fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->ccu), 0, s->memmap[AW_R40_DEV_CCU]);
> +
>  /* SD/MMC */
>  object_property_set_link(OBJECT(&s->mmc0), "dma-memory",
>   OBJECT(get_system_memory()), &error_fatal);
> diff --git a/hw/misc/allwinner-r40-ccu.c b/hw/misc/allwinner-r40-ccu.c
> new file mode 100644
> index 00..0abe006874
> --- /dev/null
> +++ b/hw/misc/allwinner-r40-ccu.c
> @@ -0,0 +1,207 @@
> +/*
> + * Allwinner R40 Clock Control Unit emulation
> + *
> + * Copyright (C) 2023 qianfan Zhao 
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/units.h"
> +#include "hw/sysbus.h"
> +#include "migration/vmstate.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "hw/misc/allwinner-r40-ccu.h"
> +
> +/* CCU register offsets */
> +enum {
> +REG_PLL_CPUX_CTRL   = 0x,
> +REG_PLL_AUDIO_CTRL  = 0x0008,
> +REG_PLL_VIDEO0_CTRL = 0x0010,
> +REG_PLL_VE_CTRL = 0x0018,
> +REG_PLL_DDR0_CTRL   = 0x0020,
> +REG_PLL_PERIPH0_CTRL= 0x0028,
> +REG_PLL_PERIPH1_CTRL= 0x002c,
> +REG_PLL_VIDEO1_CTRL = 0x0030,
> +REG_PLL_SATA_CTRL   = 0x0034,
> +REG_PLL_GPU_CTRL= 0x0038,
> +REG_PLL_MIPI_CTRL   = 0x0040,
> +REG_PLL_DE_CTRL = 0x0048,
> +REG_PLL_DDR1_CTRL   = 0x004c,
> +REG_AHB1_APB1_CFG   = 0x0054,
> +REG_APB2_CFG= 0x0058,
> +REG_MMC0_CLK= 0x0088,
> +REG_MMC1_CLK= 0x008c,
> +REG_MMC2_CLK= 0x0090,
> +REG_MMC3_CLK= 0x0094,
> +REG_USBPHY_CFG  = 0x00cc,
> +REG_PLL_DDR_AUX = 0x00

Re: [PATCH v1 03/11] hw: allwinner-r40: Complete uart devices

[Strahinja Jankovic]

Hi,

On Tue, Mar 21, 2023 at 11:25 AM  wrote:
>
> From: qianfan Zhao 
>
> R40 has eight UARTs, support both 16450 and 16550 compatible modes.
>
> Signed-off-by: qianfan Zhao 
> ---
>  hw/arm/allwinner-r40.c | 32 
>  include/hw/arm/allwinner-r40.h |  7 +++
>  2 files changed, 39 insertions(+)
>
> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> index 3517682aed..fde01783b1 100644
> --- a/hw/arm/allwinner-r40.c
> +++ b/hw/arm/allwinner-r40.c
> @@ -45,6 +45,13 @@ const hwaddr allwinner_r40_memmap[] = {
>  [AW_R40_DEV_CCU]= 0x01c2,
>  [AW_R40_DEV_PIT]= 0x01c20c00,
>  [AW_R40_DEV_UART0]  = 0x01c28000,
> +[AW_R40_DEV_UART1]  = 0x01c28400,
> +[AW_R40_DEV_UART2]  = 0x01c28800,
> +[AW_R40_DEV_UART3]  = 0x01c28c00,
> +[AW_R40_DEV_UART4]  = 0x01c29000,
> +[AW_R40_DEV_UART5]  = 0x01c29400,
> +[AW_R40_DEV_UART6]  = 0x01c29800,
> +[AW_R40_DEV_UART7]  = 0x01c29c00,
>  [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
>  [AW_R40_DEV_GIC_CPU]= 0x01c82000,
>  [AW_R40_DEV_GIC_HYP]= 0x01c84000,
> @@ -160,6 +167,10 @@ enum {
>  AW_R40_GIC_SPI_UART1 =  2,
>  AW_R40_GIC_SPI_UART2 =  3,
>  AW_R40_GIC_SPI_UART3 =  4,
> +AW_R40_GIC_SPI_UART4 = 17,
> +AW_R40_GIC_SPI_UART5 = 18,
> +AW_R40_GIC_SPI_UART6 = 19,
> +AW_R40_GIC_SPI_UART7 = 20,
>  AW_R40_GIC_SPI_TIMER0= 22,
>  AW_R40_GIC_SPI_TIMER1= 23,
>  AW_R40_GIC_SPI_MMC0  = 32,
> @@ -396,6 +407,27 @@ static void allwinner_r40_realize(DeviceState *dev, 
> Error **errp)
>  serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART0], 2,
> qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART0),
> 115200, serial_hd(0), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART1], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART1),
> +   115200, serial_hd(1), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART2], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART2),
> +   115200, serial_hd(2), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART3], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART3),
> +   115200, serial_hd(3), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART4], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART4),
> +   115200, serial_hd(4), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART5], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART5),
> +   115200, serial_hd(5), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART6], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART6),
> +   115200, serial_hd(6), DEVICE_NATIVE_ENDIAN);
> +serial_mm_init(get_system_memory(), s->memmap[AW_R40_DEV_UART7], 2,
> +   qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART7),
> +   115200, serial_hd(7), DEVICE_NATIVE_ENDIAN);
>
>  /* Unimplemented devices */
>  for (i = 0; i < ARRAY_SIZE(r40_unimplemented); i++) {
> diff --git a/include/hw/arm/allwinner-r40.h b/include/hw/arm/allwinner-r40.h
> index b355af2c4f..dfb5eb609c 100644
> --- a/include/hw/arm/allwinner-r40.h
> +++ b/include/hw/arm/allwinner-r40.h
> @@ -41,6 +41,13 @@ enum {
>  AW_R40_DEV_CCU,
>  AW_R40_DEV_PIT,
>  AW_R40_DEV_UART0,
> +AW_R40_DEV_UART1,
> +AW_R40_DEV_UART2,
> +AW_R40_DEV_UART3,
> +AW_R40_DEV_UART4,
> +AW_R40_DEV_UART5,
> +AW_R40_DEV_UART6,
> +AW_R40_DEV_UART7,
>  AW_R40_DEV_GIC_DIST,
>  AW_R40_DEV_GIC_CPU,
>  AW_R40_DEV_GIC_HYP,
> --
> 2.25.1
>

Reviewed-by: Strahinja Jankovic 


Best regards,
Strahinja

Re: [PATCH v1 04/11] hw: arm: allwinner-r40: Add 5 TWI controllers

[Strahinja Jankovic]

Hi,

On Tue, Mar 21, 2023 at 11:25 AM  wrote:
>
> From: qianfan Zhao 
>
> TWI(i2c) is designed to be used as an interface between CPU host and the
> serial 2-Wire bus. It can support all standard 2-Wire transfer, can be
> operated in standard mode(100kbit/s) or fast-mode, supporting data rate
> up to 400kbit/s.
>
> Signed-off-by: qianfan Zhao 
> ---
>  hw/arm/allwinner-r40.c | 47 ++
>  include/hw/arm/allwinner-r40.h | 11 
>  2 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/allwinner-r40.c b/hw/arm/allwinner-r40.c
> index fde01783b1..9fa23e1f33 100644
> --- a/hw/arm/allwinner-r40.c
> +++ b/hw/arm/allwinner-r40.c
> @@ -52,6 +52,11 @@ const hwaddr allwinner_r40_memmap[] = {
>  [AW_R40_DEV_UART5]  = 0x01c29400,
>  [AW_R40_DEV_UART6]  = 0x01c29800,
>  [AW_R40_DEV_UART7]  = 0x01c29c00,
> +[AW_R40_DEV_TWI0]   = 0x01c2ac00,
> +[AW_R40_DEV_TWI1]   = 0x01c2b000,
> +[AW_R40_DEV_TWI2]   = 0x01c2b400,
> +[AW_R40_DEV_TWI3]   = 0x01c2b800,
> +[AW_R40_DEV_TWI4]   = 0x01c2c000,
>  [AW_R40_DEV_GIC_DIST]   = 0x01c81000,
>  [AW_R40_DEV_GIC_CPU]= 0x01c82000,
>  [AW_R40_DEV_GIC_HYP]= 0x01c84000,
> @@ -115,11 +120,6 @@ static struct AwR40Unimplemented r40_unimplemented[] = {
>  { "uart7",  0x01c29c00, 1 * KiB },
>  { "ps20",   0x01c2a000, 1 * KiB },
>  { "ps21",   0x01c2a400, 1 * KiB },
> -{ "twi0",   0x01c2ac00, 1 * KiB },
> -{ "twi1",   0x01c2b000, 1 * KiB },
> -{ "twi2",   0x01c2b400, 1 * KiB },
> -{ "twi3",   0x01c2b800, 1 * KiB },
> -{ "twi4",   0x01c2c000, 1 * KiB },
>  { "scr",0x01c2c400, 1 * KiB },
>  { "tvd-top",0x01c3, 4 * KiB },
>  { "tvd0",   0x01c31000, 4 * KiB },
> @@ -167,6 +167,9 @@ enum {
>  AW_R40_GIC_SPI_UART1 =  2,
>  AW_R40_GIC_SPI_UART2 =  3,
>  AW_R40_GIC_SPI_UART3 =  4,
> +AW_R40_GIC_SPI_TWI0  =  7,
> +AW_R40_GIC_SPI_TWI1  =  8,
> +AW_R40_GIC_SPI_TWI2  =  9,
>  AW_R40_GIC_SPI_UART4 = 17,
>  AW_R40_GIC_SPI_UART5 = 18,
>  AW_R40_GIC_SPI_UART6 = 19,
> @@ -177,6 +180,8 @@ enum {
>  AW_R40_GIC_SPI_MMC1  = 33,
>  AW_R40_GIC_SPI_MMC2  = 34,
>  AW_R40_GIC_SPI_MMC3  = 35,
> +AW_R40_GIC_SPI_TWI3  = 88,
> +AW_R40_GIC_SPI_TWI4  = 89,
>  };
>
>  /* Allwinner R40 general constants */
> @@ -262,6 +267,12 @@ static void allwinner_r40_init(Object *obj)
>  object_initialize_child(obj, "mmc1", &s->mmc1, TYPE_AW_SDHOST_SUN5I);
>  object_initialize_child(obj, "mmc2", &s->mmc2, TYPE_AW_SDHOST_SUN5I);
>  object_initialize_child(obj, "mmc3", &s->mmc3, TYPE_AW_SDHOST_SUN5I);
> +
> +object_initialize_child(obj, "twi0", &s->i2c0, TYPE_AW_I2C_SUN6I);
> +object_initialize_child(obj, "twi1", &s->i2c1, TYPE_AW_I2C_SUN6I);
> +object_initialize_child(obj, "twi2", &s->i2c2, TYPE_AW_I2C_SUN6I);
> +object_initialize_child(obj, "twi3", &s->i2c3, TYPE_AW_I2C_SUN6I);
> +object_initialize_child(obj, "twi4", &s->i2c4, TYPE_AW_I2C_SUN6I);
>  }
>
>  static void allwinner_r40_realize(DeviceState *dev, Error **errp)
> @@ -429,6 +440,32 @@ static void allwinner_r40_realize(DeviceState *dev, 
> Error **errp)
> qdev_get_gpio_in(DEVICE(&s->gic), AW_R40_GIC_SPI_UART7),
> 115200, serial_hd(7), DEVICE_NATIVE_ENDIAN);
>
> +/* I2C */
> +sysbus_realize(SYS_BUS_DEVICE(&s->i2c0), &error_fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c0), 0, s->memmap[AW_R40_DEV_TWI0]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c0), 0,
> +   qdev_get_gpio_in(DEVICE(&s->gic), 
> AW_R40_GIC_SPI_TWI0));
> +
> +sysbus_realize(SYS_BUS_DEVICE(&s->i2c1), &error_fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c1), 0, s->memmap[AW_R40_DEV_TWI1]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c1), 0,
> +   qdev_get_gpio_in(DEVICE(&s->gic), 
> AW_R40_GIC_SPI_TWI1));
> +
> +sysbus_realize(SYS_BUS_DEVICE(&s->i2c2), &error_fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c2), 0, s->memmap[AW_R40_DEV_TWI2]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c2), 0,
> +   qdev_get_gpio_in(DEVICE(&s->gic), 
> AW_R40_GIC_SPI_TWI2));
> +
> +sysbus_realize(SYS_BUS_DEVICE(&s->i2c3), &error_fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c3), 0, s->memmap[AW_R40_DEV_TWI3]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c3), 0,
> +   qdev_get_gpio_in(DEVICE(&s->gic), 
> AW_R40_GIC_SPI_TWI3));
> +
> +sysbus_realize(SYS_BUS_DEVICE(&s->i2c4), &error_fatal);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c4), 0, s->memmap[AW_R40_DEV_TWI4]);
> +sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c4), 0,
> +   qdev_get_gpio_in(DEVICE(&s->gic), 
> AW_R40_GIC_SPI_TWI4));
> +
>  /* Unimplemented devices */
>  for (i = 0; i < ARRAY_SIZE(r40_uni

Re: [PATCH v1 05/11] hw/misc: AXP221 PMU Emulation

[Strahinja Jankovic]

Hi,

On Tue, Mar 21, 2023 at 11:25 AM  wrote:
>
> From: qianfan Zhao 
>
> This patch adds minimal support for AXP-221 PMU and connect it to
> bananapi M2U board.
>
> Signed-off-by: qianfan Zhao 

As I wrote in the RFC patch, I would suggest renaming the axp209.c
file to axp2xx_pmu.c and extending it with support for AXP-221.
The difference in emulated AXP-209 and AXP-221 is only in registers,
so they can share the functions.

Best regards,
Strahinja


> ---
>  hw/arm/Kconfig|   1 +
>  hw/arm/bananapi_m2u.c |   5 ++
>  hw/misc/Kconfig   |   4 +
>  hw/misc/axp221.c  | 196 ++
>  hw/misc/meson.build   |   1 +
>  hw/misc/trace-events  |   5 ++
>  6 files changed, 212 insertions(+)
>  create mode 100644 hw/misc/axp221.c
>
> diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
> index 9e14c3427e..cf8fb083f8 100644
> --- a/hw/arm/Kconfig
> +++ b/hw/arm/Kconfig
> @@ -347,6 +347,7 @@ config ALLWINNER_H3
>  config ALLWINNER_R40
>  bool
>  select ALLWINNER_A10_PIT
> +select AXP221_PMU
>  select SERIAL
>  select ARM_TIMER
>  select ARM_GIC
> diff --git a/hw/arm/bananapi_m2u.c b/hw/arm/bananapi_m2u.c
> index 1b6241719d..bdee12efd3 100644
> --- a/hw/arm/bananapi_m2u.c
> +++ b/hw/arm/bananapi_m2u.c
> @@ -22,6 +22,7 @@
>  #include "exec/address-spaces.h"
>  #include "qapi/error.h"
>  #include "hw/boards.h"
> +#include "hw/i2c/i2c.h"
>  #include "hw/qdev-properties.h"
>  #include "hw/arm/allwinner-r40.h"
>
> @@ -91,6 +92,10 @@ static void bpim2u_init(MachineState *machine)
>   &bootroom_loaded);
>  mmc_attach_drive(r40, &r40->mmc3, 3, false, NULL);
>
> +/* Connect AXP221 */
> +i2c = I2C_BUS(qdev_get_child_bus(DEVICE(&r40->i2c0), "i2c"));
> +i2c_slave_create_simple(i2c, "axp221_pmu", 0x34);
> +
>  /* SDRAM */
>  memory_region_add_subregion(get_system_memory(), 
> r40->memmap[AW_R40_DEV_SDRAM],
>  machine->ram);
> diff --git a/hw/misc/Kconfig b/hw/misc/Kconfig
> index 2ef5781ef8..f66ac390b1 100644
> --- a/hw/misc/Kconfig
> +++ b/hw/misc/Kconfig
> @@ -180,4 +180,8 @@ config AXP209_PMU
>  bool
>  depends on I2C
>
> +config AXP221_PMU
> +bool
> +depends on I2C
> +
>  source macio/Kconfig
> diff --git a/hw/misc/axp221.c b/hw/misc/axp221.c
> new file mode 100644
> index 00..47784bb085
> --- /dev/null
> +++ b/hw/misc/axp221.c
> @@ -0,0 +1,196 @@
> +/*
> + * AXP-221/221s PMU Emulation
> + *
> + * Copyright (C) 2023 qianfan Zhao 
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL 
> THE
> + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
> + * DEALINGS IN THE SOFTWARE.
> + *
> + * SPDX-License-Identifier: MIT
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/bitops.h"
> +#include "trace.h"
> +#include "hw/i2c/i2c.h"
> +#include "migration/vmstate.h"
> +
> +#define TYPE_AXP221_PMU "axp221_pmu"
> +
> +#define AXP221(obj) \
> +OBJECT_CHECK(AXP221I2CState, (obj), TYPE_AXP221_PMU)
> +
> +#define NR_REGS 0xff
> +
> +/* A simple I2C slave which returns values of ID or CNT register. */
> +typedef struct AXP221I2CState {
> +/*< private >*/
> +I2CSlave i2c;
> +/*< public >*/
> +uint8_t regs[NR_REGS];  /* peripheral registers */
> +uint8_t ptr;/* current register index */
> +uint8_t count;  /* counter used for tx/rx */
> +} AXP221I2CState;
> +
> +#define AXP221_PWR_STATUS_ACIN_PRESENT  BIT(7)
> +#define AXP221_PWR_STATUS_ACIN_AVAILBIT(6)
> +#define AXP221_PWR_STATUS_VBUS_PRESENT  BIT(5)
> +#define AXP221_PWR_STATUS_VBUS_USED BIT(4)
> +#define AXP221_PWR_STATUS_BAT_CHARGING  BIT(2)
> +#define AXP221_PWR_STATUS_ACIN_VBUS_POWERED BIT(1)
> +
> +/* Reset all counters and load ID register */
> +static void axp221_reset_enter(Object *obj, ResetType type)
> +{
> +AXP221I2CState *s = AXP221(obj);
> +
> +memset(s->regs, 0, NR_REGS);
> +s->ptr = 0;
> +

Re: [PATCH v2 4/7] hw/ipmi: Refactor IPMI interface

[Corey Minyard]

On Fri, Mar 24, 2023 at 04:09:01PM -0700, Hao Wu wrote:
> This patch refactors the IPMI interface so that it can be used by both
> the BMC side and core-side simulation.

This patch is hard to review because it does so many different things
and they are all mixed up.  It looks ok, but it's hard to tell.  I know
it's a pain (I've done it many times) but can you split this up into
separate patches that each make one change?  The list you have below
tells me that 5 patches might be appropriate.

If I really had to a full review this myself I would break it into
multiple patches just to review it.  But that should really be your job.

Thanks,

-corey

> 
> Detail changes:
> (1) Split IPMIInterface into IPMIInterfaceHost (for host side
> simulation) and IPMIInterfaceClient (for BMC side simulation).
> (2) rename handle_rsp -> handle_msg so the name fits both BMC side and
> Core side.
> (3) Add a new class IPMICore. This class represents a simulator/external
> connection for both BMC and Core side emulation.
> (4) Change the original IPMIBmc to IPMIBmcHost, representing host side
> simulation.
> (5) Add a new type IPMIBmcClient representing BMC side simulation.
> (6) Appy the changes to  the entire IPMI library.
> 
> Signed-off-by: Hao Wu 
> ---
>  hw/acpi/ipmi.c |   4 +-
>  hw/ipmi/ipmi.c |  60 +
>  hw/ipmi/ipmi_bmc_extern.c  |  67 ++
>  hw/ipmi/ipmi_bmc_sim.c |  78 -
>  hw/ipmi/ipmi_bt.c  |  33 +
>  hw/ipmi/ipmi_kcs.c |  31 +
>  hw/ipmi/isa_ipmi_bt.c  |  18 ++---
>  hw/ipmi/isa_ipmi_kcs.c |  13 ++--
>  hw/ipmi/pci_ipmi_bt.c  |   8 +--
>  hw/ipmi/pci_ipmi_kcs.c |   8 +--
>  hw/ipmi/smbus_ipmi.c   |  26 +++
>  hw/ppc/pnv.c   |   4 +-
>  hw/ppc/pnv_bmc.c   |  22 +++---
>  hw/smbios/smbios_type_38.c |  11 +--
>  include/hw/ipmi/ipmi.h | 135 ++---
>  include/hw/ipmi/ipmi_bt.h  |   2 +-
>  include/hw/ipmi/ipmi_kcs.h |   2 +-
>  include/hw/ppc/pnv.h   |  12 ++--
>  18 files changed, 332 insertions(+), 202 deletions(-)
> 
> diff --git a/hw/acpi/ipmi.c b/hw/acpi/ipmi.c
> index a20e57d465..e6d2cd790b 100644
> --- a/hw/acpi/ipmi.c
> +++ b/hw/acpi/ipmi.c
> @@ -66,8 +66,8 @@ void build_ipmi_dev_aml(AcpiDevAmlIf *adev, Aml *scope)
>  {
>  Aml *dev;
>  IPMIFwInfo info = {};
> -IPMIInterface *ii = IPMI_INTERFACE(adev);
> -IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
> +IPMIInterfaceHost *ii = IPMI_INTERFACE_HOST(adev);
> +IPMIInterfaceHostClass *iic = IPMI_INTERFACE_HOST_GET_CLASS(ii);
>  uint16_t version;
>  
>  iic->get_fwinfo(ii, &info);
> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c
> index bbb07b151e..1be923ffb8 100644
> --- a/hw/ipmi/ipmi.c
> +++ b/hw/ipmi/ipmi.c
> @@ -38,7 +38,7 @@ uint32_t ipmi_next_uuid(void)
>  return ipmi_current_uuid++;
>  }
>  
> -static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly)
> +static int ipmi_do_hw_op(IPMIInterfaceHost *s, enum ipmi_op op, int 
> checkonly)
>  {
>  switch (op) {
>  case IPMI_RESET_CHASSIS:
> @@ -78,9 +78,9 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, 
> int checkonly)
>  }
>  }
>  
> -static void ipmi_interface_class_init(ObjectClass *class, void *data)
> +static void ipmi_interface_host_class_init(ObjectClass *class, void *data)
>  {
> -IPMIInterfaceClass *ik = IPMI_INTERFACE_CLASS(class);
> +IPMIInterfaceHostClass *ik = IPMI_INTERFACE_HOST_CLASS(class);
>  
>  ik->do_hw_op = ipmi_do_hw_op;
>  }
> @@ -89,27 +89,48 @@ static const TypeInfo ipmi_interface_type_info = {
>  .name = TYPE_IPMI_INTERFACE,
>  .parent = TYPE_INTERFACE,
>  .class_size = sizeof(IPMIInterfaceClass),
> -.class_init = ipmi_interface_class_init,
> +};
> +
> +static const TypeInfo ipmi_interface_host_type_info = {
> +.name = TYPE_IPMI_INTERFACE_HOST,
> +.parent = TYPE_IPMI_INTERFACE,
> +.class_size = sizeof(IPMIInterfaceHostClass),
> +.class_init = ipmi_interface_host_class_init,
> +};
> +
> +static const TypeInfo ipmi_interface_client_type_info = {
> +.name = TYPE_IPMI_INTERFACE_CLIENT,
> +.parent = TYPE_IPMI_INTERFACE,
> +.class_size = sizeof(IPMIInterfaceClientClass),
> +};
> +
> +static const TypeInfo ipmi_core_type_info = {
> +.name = TYPE_IPMI_CORE,
> +.parent = TYPE_DEVICE,
> +.instance_size = sizeof(IPMICore),
> +.class_size = sizeof(IPMICoreClass),
> +.abstract = true,
>  };
>  
>  static void isa_ipmi_bmc_check(const Object *obj, const char *name,
> Object *val, Error **errp)
>  {
> -IPMIBmc *bmc = IPMI_BMC(val);
> +IPMICore *ic = IPMI_CORE(val);
>  
> -if (bmc->intf)
> +if (ic->intf) {
>  error_setg(errp, "BMC object is already in use");
> +}
>  }
>  
>  void ipmi_bmc_find_and_link(Object *obj, Object **bmc)
>  {
> -object_property_add_link(obj, "bm

Re: [PATCH v2 2/7] docs/specs: IPMI device emulation: main processor

[Corey Minyard]

On Fri, Mar 24, 2023 at 04:08:59PM -0700, Hao Wu wrote:
> From: Havard Skinnemoen 
> 
> This document is an attempt to briefly document the existing IPMI
> emulation support on the main processor. It provides the necessary
> background for the BMC-side IPMI emulation proposed by the next patch.
> 
> Signed-off-by: Havard Skinnemoen 
> Signed-off-by: Hao Wu 
> ---
> +* An external BMC simulator or emulator, connected over a chardev
> +  (``ipmi-bmc-extern``). `ipmi_sim
> +  
> `_

Nit, you have a double slash above.

Other than that, this does a good job of explaining things.  I'm good
with these docs.

-corey

[PATCH v5 2/3] tpm: Extend common APIs to support TPM TIS I2C

[Ninad Palsule]

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.

This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
  the I2C support. The checksum calculation is handled in the qemu
  common code.
- Added wrapper function for read and write data so that I2C code can
  call it without MMIO interface.

Signed-off-by: Ninad Palsule 
---
V2:

Incorporated Stephen's comments.

- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
  i2c layer.

---
V3:
Incorporated review comments from Cedric and Stefan.

- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h

---
V4:

Incorporated review comments by Stefan

- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
 hw/tpm/tpm_tis.h|  3 +++
 hw/tpm/tpm_tis_common.c | 28 
 include/hw/acpi/tpm.h   | 28 
 3 files changed, 59 insertions(+)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
 void tpm_tis_reset(TPMState *s);
 enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
 void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
 
 #endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c6d139943e 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
 #include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
 #include "qemu/module.h"
 
 #include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
addr,
 return val;
 }
 
+/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
 /*
  * Write a value to a register of the TIS interface
  * See specs pages 33-63 for description of the registers
@@ -767,6 +786,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
 }
 }
 
+/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
 const MemoryRegionOps tpm_tis_memory_ops = {
 .read = tpm_tis_mmio_read,
 .write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..4f2424e2fe 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
 #define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
 #define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
 #define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC   (1 << 8)
 #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is mandatory */
 #define TPM_TIS_CAPABILITIES_SUPPORTED1_3 \
 (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
@@ -209,6 +210,33 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
 #define TPM_PPI_FUNC_MASK(7 << 0)
 
+/* TPM TIS I2C registers */
+#define TPM_I2C_REG_LOC_SEL  0x00
+#define TPM_I2C_REG_ACCESS   0x04
+#define TPM_I2C_REG_INT_ENABLE   0x08
+#define TPM_I2C_REG_INT_CAPABILITY   0x14
+#define TPM_I2C_REG_STS  0x18
+#define TPM_I2C_REG_DATA_FIFO0x24
+#define TPM_I2C_REG_INTF_CAPABILITY  0x30
+#define TPM_I2C_REG_I2C_DEV_ADDRESS  0x38
+#define TPM_I2C_REG_DATA_CSUM_ENABLE 0x40
+#define TPM_I2C_REG_DATA_CSUM_GET0x44
+#define TPM_I2C_REG_DID_VID  0x48
+#define TPM_I2C_REG_RID  0x4c
+#define TPM_I2C_REG_UNKNOWN  0xff
+
+/* I2C specific interface capabilities */
+#define TPM_I2C_CAP_INTERFACE_TYPE (0x2 << 0)   /* FIFO interface */
+#define TPM_I2C_CAP_INTERFACE_VER  (0x0 << 4)   /* TCG I2C intf 1.0 */
+#define TPM_I2C_CAP_TPM2_FAMILY(0x1 << 7)   /* TPM 2.0 family. */
+#define TPM_I2C_CAP_DEV_ADDR_CHANGE(0x0 << 27)  /* No dev addr chng */
+#define TPM_I2C_CAP_BURST_COUNT_STATIC (0x1 << 29)  /* Burst count static 
*/
+#define TPM_I2C_CAP_LOCALITY_CAP   (0x1 << 25)  /* 0-5 locality */
+#define TPM_

[PATCH v5 3/3] tpm: Add support for TPM device over I2C bus

[Ninad Palsule]

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.

This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
  cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. User specify this
  string on command line.

Testing:
  TPM I2C device modulte is tested using SWTPM (software based TPM
  package). The qemu used the rainier machine and it was connected to
  swtpm over the socket interface.

  The command to start swtpm is as follows:
  $ swtpm socket --tpmstate dir=/tmp/mytpm1\
 --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock  \
 --tpm2 --log level=100

  The command to start qemu is as follows:
  $ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e

  Note: Currently you need to specify the I2C bus and device address on
command line. In future we can add a device at board level.

Signed-off-by: Ninad Palsule 
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
  capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
  layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.

---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
  remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
  is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
  function in TIS layer to return the checksum and used that to process
  the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
  write call instead of 1 byte at a time. Added functions to convert
  between little endian stream of bytes to single 32 bit unsigned
  integer. Similarly 32  bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h

---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
  tracing.
- Removed the endian conversion functions instead used simple logic
  provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
  on command line by using "-trace "tpm_tis_i2c*" option.

---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
  information like tis_addr and register name in the i2cst so that we
  can only convert it once on i2c_send.
- Trace register number for unknown registers.
---
 hw/arm/Kconfig   |   1 +
 hw/tpm/Kconfig   |   7 +
 hw/tpm/meson.build   |   1 +
 hw/tpm/tpm_tis_i2c.c | 485 +++
 hw/tpm/trace-events  |   6 +
 include/sysemu/tpm.h |   3 +
 6 files changed, 503 insertions(+)
 create mode 100644 hw/tpm/tpm_tis_i2c.c

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
 imply VFIO_PLATFORM
 imply VFIO_XGMAC
 imply TPM_TIS_SYSBUS
+imply TPM_TIS_I2C
 imply NVDIMM
 select ARM_GIC
 select ACPI
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 29e82f3c92..a46663288c 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -1,3 +1,10 @@
+config TPM_TIS_I2C
+bool
+depends on TPM
+select TPM_BACKEND
+select I2C
+select TPM_TIS
+
 config TPM_TIS_ISA
 bool
 depends on TPM && ISA_BUS
diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index 7abc2d794a..76fe3cb098 100644
--- a/hw/tpm/meson.build
+++ b/hw/tpm/meson.build
@@ -1,6 +1,7 @@
 softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_t

[PATCH v5 1/3] docs: Add support for TPM devices over I2C bus

[Ninad Palsule]

This is a documentation change for I2C TPM device support.

Qemu already supports devices attached to ISA and sysbus.
This drop adds support for the I2C bus attached TPM devices.

Signed-off-by: Ninad Palsule 

---
V2:

Incorporated Stephen's review comments
- Added example in the document.

---
V4:
Incorporate Cedric & Stefan's comments

- Added example for ast2600-evb
- Corrected statement about arm virtual machine.
---
 docs/specs/tpm.rst | 32 
 1 file changed, 32 insertions(+)

diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..a0600e2834 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface:
  - ``hw/tpm/tpm_tis_common.c``
  - ``hw/tpm/tpm_tis_isa.c``
  - ``hw/tpm/tpm_tis_sysbus.c``
+ - ``hw/tpm/tpm_tis_i2c.c``
  - ``hw/tpm/tpm_tis.h``
 
 Both an ISA device and a sysbus device are available. The former is
 used with pc/q35 machine while the latter can be instantiated in the
 Arm virt machine.
 
+An I2C device support is also added which can be instantiated in the arm
+based emulation machines. This device only supports the TPM 2 protocol.
+
 CRB interface
 -
 
@@ -348,6 +352,34 @@ In case an Arm virt machine is emulated, use the following 
command line:
 -drive if=pflash,format=raw,file=flash0.img,readonly=on \
 -drive if=pflash,format=raw,file=flash1.img
 
+In case a ast2600-evb bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M ast2600-evb -nographic \
+-kernel arch/arm/boot/zImage \
+-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \
+-initrd rootfs.cpio \
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
+In case a Rainier bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M rainier-bmc -nographic \
+-kernel ${IMAGEPATH}/fitImage-linux.bin \
+-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
+-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
+-drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2\
+-net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443\
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
 In case SeaBIOS is used as firmware, it should show the TPM menu item
 after entering the menu with 'ESC'.
 
-- 
2.37.2

[PATCH v5 0/3] Add support for TPM devices over I2C bus

[Ninad Palsule]

Hello,

In V5, I have fixed issues reported by Stefan's test.

This drop adds support for the TPM devices attached to the I2C bus. It
only supports the TPM2 protocol. You need to run it with the external
TPM emulator like swtpm. I have tested it with swtpm.

I have refered to the work done by zhdan...@meta.com but at the core
level out implementation is different.
https://github.com/theopolis/qemu/commit/2e2e57cde9e419c36af8071bb85392ad1ed70966

Based-on: $MESSAGE_ID

Ninad Palsule (3):
  docs: Add support for TPM devices over I2C bus
  tpm: Extend common APIs to support TPM TIS I2C
  tpm: Add support for TPM device over I2C bus

 docs/specs/tpm.rst  |  32 +++
 hw/arm/Kconfig  |   1 +
 hw/tpm/Kconfig  |   7 +
 hw/tpm/meson.build  |   1 +
 hw/tpm/tpm_tis.h|   3 +
 hw/tpm/tpm_tis_common.c |  28 +++
 hw/tpm/tpm_tis_i2c.c| 485 
 hw/tpm/trace-events |   6 +
 include/hw/acpi/tpm.h   |  28 +++
 include/sysemu/tpm.h|   3 +
 10 files changed, 594 insertions(+)
 create mode 100644 hw/tpm/tpm_tis_i2c.c

-- 
2.37.2

Re: [PATCH v4 3/3] tpm: Add support for TPM device over I2C bus

[Ninad Palsule]

On 3/25/23 11:46 AM, Stefan Berger wrote:



On 3/25/23 00:37, Ninad Palsule wrote:

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.



Your v4 patches with my 2 patches for the test cases are here now:

https://github.com/stefanberger/qemu-tpm/commits/tpm-i2c.v4


Ok, thanks for creating tests for this.

Ninad.



Thanks!

  Regards,
    Stefan

Re: [PATCH v4 1/3] docs: Add support for TPM devices over I2C bus

[Ninad Palsule]

On 3/25/23 11:10 AM, Stefan Berger wrote:



On 3/25/23 00:37, Ninad Palsule wrote:

This is a documentation change for I2C TPM device support.

Qemu already supports devices attached to ISA and sysbus.
This drop adds support for the I2C bus attached TPM devices.

Signed-off-by: Ninad Palsule 

---
V2:

Incorporated Stephen's review comments
- Added example in the document.

---
V4:
Incorporate Cedric & Stefan's comments

- Added example for ast2600-evb
- Corrected statement about arm virtual machine.
---
  docs/specs/tpm.rst | 32 
  1 file changed, 32 insertions(+)

diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..a0600e2834 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface:
   - ``hw/tpm/tpm_tis_common.c``
   - ``hw/tpm/tpm_tis_isa.c``
   - ``hw/tpm/tpm_tis_sysbus.c``
+ - ``hw/tpm/tpm_tis_i2c.c``
   - ``hw/tpm/tpm_tis.h``
    Both an ISA device and a sysbus device are available. The former is
  used with pc/q35 machine while the latter can be instantiated in the
  Arm virt machine.
  +An I2C device support is also added which can be instantiated in 
the armadded -> provided

arm -> Arm

Done



+based emulation machines. This device only supports the TPM 2 protocol.
+
  CRB interface
  -
  @@ -348,6 +352,34 @@ In case an Arm virt machine is emulated, use 
the following command line:

  -drive if=pflash,format=raw,file=flash0.img,readonly=on \
  -drive if=pflash,format=raw,file=flash1.img
  +In case a ast2600-evb bmc machine is emulated and want to use TPM 
device

+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M ast2600-evb -nographic \
+    -kernel arch/arm/boot/zImage \
+    -dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \
+    -initrd rootfs.cpio \
+    -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+    -tpmdev emulator,id=tpm0,chardev=chrtpm \
+    -device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
+In case a Rainier bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M rainier-bmc -nographic \
+    -kernel ${IMAGEPATH}/fitImage-linux.bin \
+    -dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
+    -initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
+    -drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2\
+    -net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443\

+    -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+    -tpmdev emulator,id=tpm0,chardev=chrtpm \
+    -device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
  In case SeaBIOS is used as firmware, it should show the TPM menu item
  after entering the menu with 'ESC'.


With the above nits:

Reviewed-by: Stefan Berger 

Re: [PATCH v4 2/3] tpm: Extend common APIs to support TPM TIS I2C

[Ninad Palsule]

Hi Stefan,

On 3/25/23 11:12 AM, Stefan Berger wrote:



On 3/25/23 00:37, Ninad Palsule wrote:

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.

This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
   the I2C support. The checksum calculation is handled in the qemu
   common code.
- Added wrapper function for read and write data so that I2C code can
   call it without MMIO interface.

Signed-off-by: Ninad Palsule 
---
V2:

Incorporated Stephen's comments.

- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
   i2c layer.

---
V3:
Incorporated review comments from Cedric and Stefan.

- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h

---
V4:

Incorporated review comments by Stefan

- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
  hw/tpm/tpm_tis.h    |  3 +++
  hw/tpm/tpm_tis_common.c | 28 
  include/hw/acpi/tpm.h   | 28 
  3 files changed, 59 insertions(+)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
  void tpm_tis_reset(TPMState *s);
  enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
  void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, 
uint32_t size);

+uint16_t tpm_tis_get_checksum(TPMState *s);
    #endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c6d139943e 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
  #include "hw/irq.h"
  #include "hw/isa/isa.h"
  #include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
  #include "qemu/module.h"
    #include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, 
hwaddr addr,

  return val;
  }
  +/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+    return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+    return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
  /*
   * Write a value to a register of the TIS interface
   * See specs pages 33-63 for description of the registers
@@ -767,6 +786,15 @@ static void tpm_tis_mmio_write(void *opaque, 
hwaddr addr,

  }
  }
  +/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, 
uint32_t size)

+{
+    tpm_tis_mmio_write(s, addr, val, size);
+}
+
  const MemoryRegionOps tpm_tis_memory_ops = {
  .read = tpm_tis_mmio_read,
  .write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..4f2424e2fe 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
  #define TPM_TIS_CAP_DATA_TRANSFER_64B    (3 << 9)
  #define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
  #define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC   (1 << 8)
  #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is 
mandatory */

  #define TPM_TIS_CAPABILITIES_SUPPORTED1_3 \
  (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
@@ -209,6 +210,33 @@ REG32(CRB_DATA_BUFFER, 0x80)
  #define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
  #define TPM_PPI_FUNC_MASK    (7 << 0)
  +/* TPM TIS I2C registers */
+#define TPM_I2C_REG_LOC_SEL  0x00
+#define TPM_I2C_REG_ACCESS   0x04
+#define TPM_I2C_REG_INT_ENABLE   0x08
+#define TPM_I2C_REG_INT_CAPABILITY   0x14
+#define TPM_I2C_REG_STS  0x18
+#define TPM_I2C_REG_DATA_FIFO    0x24
+#define TPM_I2C_REG_INTF_CAPABILITY  0x30
+#define TPM_I2C_REG_I2C_DEV_ADDRESS  0x38
+#define TPM_I2C_REG_DATA_CSUM_ENABLE 0x40
+#define TPM_I2C_REG_DATA_CSUM_GET    0x44
+#define TPM_I2C_REG_DID_VID  0x48
+#define TPM_I2C_REG_RID  0x4c
+#define TPM_I2C_REG_UNKNOWN  0xff
+
+/* I2C specific interface capabilities */
+#define TPM_I2C_CAP_INTERFACE_TYPE (0x2 << 0) /* FIFO interface */
+#define TPM_I2C_CAP_INTERFACE_VER  (0x0 << 4) /* TCG I2C intf 
1.0 */

+#define TPM_I2C_CAP_TPM2_FAMILY    (0x1 << 7) /* TPM 2.0 family. */
+#define TPM_I2C_CAP_DEV_ADDR_CHANGE    (0x0 << 27) /* No dev addr 
chng */
+#define TPM_I2C_CAP_BURST_COUNT_STATIC (0x1 << 29

Re: [PATCH v4 3/3] tpm: Add support for TPM device over I2C bus

[Ninad Palsule]

Hi Stefan,


On 3/25/23 11:44 AM, Stefan Berger wrote:



On 3/25/23 00:37, Ninad Palsule wrote:

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.

This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
   cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. User specify this
   string on command line.


The user has to provide this string on the command line.

Fixed.




Testing:
   TPM I2C device modulte is tested using SWTPM (software based TPM


modulte -> module

Fixed



   package). The qemu used the rainier machine and it was connected to
   swtpm over the socket interface.
Qemu uses the rainier machine and is connected to swtpm over the 
socker interface.

Fixed




   The command to start swtpm is as follows:
   $ swtpm socket --tpmstate dir=/tmp/mytpm1    \
  --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock  \
  --tpm2 --log level=100

   The command to start qemu is as follows:
   $ qemu-system-arm -M rainier-bmc -nographic \
 -kernel ${IMAGEPATH}/fitImage-linux.bin \
 -dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
 -initrd 
${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
 -drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
 -net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \

 -chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
 -tpmdev emulator,id=tpm0,chardev=chrtpm \
 -device 
tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e


   Note: Currently you need to specify the I2C bus and device address on
 command line. In future we can add a device at board level.

I would remove this not. Not sure whether this can be done due to 
backwards comaptibility issues.

Removed



I tested this series on big and little endian machines and it passed 
the test cases on noth!

Thank you!



Signed-off-by: Ninad Palsule 
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
   capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
   layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.

---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
   remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
   is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
   function in TIS layer to return the checksum and used that to process
   the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
   write call instead of 1 byte at a time. Added functions to convert
   between little endian stream of bytes to single 32 bit unsigned
   integer. Similarly 32  bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h

---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
   tracing.
- Removed the endian conversion functions instead used simple logic
   provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
   on command line by using "-trace "tpm_tis_i2c*" option.
---
  hw/arm/Kconfig   |   1 +
  hw/tpm/Kconfig   |   7 +
  hw/tpm/meson.build   |   1 +
  hw/tpm/tpm_tis_i2c.c | 492 +++
  hw/tpm/trace-events  |   6 +
  include/sysemu/tpm.h |   3 +
  6 files changed, 510 insertions(+)
  create mode 100644 hw/tpm/tpm_tis_i2c.c

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
  imply VFIO_PLATFORM
  imply VFIO_XGMAC
  imply TPM_TIS_SYSBUS
+    imply TPM_TIS_I2C
  imply NVDIMM
  select ARM_GIC
  select ACPI
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 29e82f3c92..a46663288c 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -1,3 +1,10 @@
+config TPM_TIS_I2C
+    bool
+    depends on TPM
+    select TPM_BACKEND
+    select I2C
+    select TPM_TIS
+
  config TPM_TIS_ISA
  bool
  depends on TPM && ISA_BUS
diff --g

[PATCH v6 0/3] Add support for TPM devices over I2C bus

[Ninad Palsule]

Hello,

I have incorporated review comments from Stefan. Please review.

This drop adds support for the TPM devices attached to the I2C bus. It
only supports the TPM2 protocol. You need to run it with the external
TPM emulator like swtpm. I have tested it with swtpm.

I have refered to the work done by zhdan...@meta.com but at the core
level out implementation is different.
https://github.com/theopolis/qemu/commit/2e2e57cde9e419c36af8071bb85392ad1ed70966

Based-on: $MESSAGE_ID

Ninad Palsule (3):
  docs: Add support for TPM devices over I2C bus
  tpm: Extend common APIs to support TPM TIS I2C
  tpm: Add support for TPM device over I2C bus

 docs/specs/tpm.rst  |  32 +++
 hw/arm/Kconfig  |   1 +
 hw/tpm/Kconfig  |   7 +
 hw/tpm/meson.build  |   1 +
 hw/tpm/tpm_tis.h|   3 +
 hw/tpm/tpm_tis_common.c |  28 +++
 hw/tpm/tpm_tis_i2c.c| 533 
 hw/tpm/trace-events |   6 +
 include/hw/acpi/tpm.h   |  28 +++
 include/sysemu/tpm.h|   3 +
 10 files changed, 642 insertions(+)
 create mode 100644 hw/tpm/tpm_tis_i2c.c

-- 
2.37.2

[PATCH v6 2/3] tpm: Extend common APIs to support TPM TIS I2C

[Ninad Palsule]

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices.

This commit includes changes for the common code.
- Added support for the new checksum registers which are required for
  the I2C support. The checksum calculation is handled in the qemu
  common code.
- Added wrapper function for read and write data so that I2C code can
  call it without MMIO interface.

Signed-off-by: Ninad Palsule 
---
V2:

Incorporated Stephen's comments.

- Removed checksum enable and checksum get registers.
- Added checksum calculation function which can be called from
  i2c layer.

---
V3:
Incorporated review comments from Cedric and Stefan.

- Pass locality to the checksum calculation function and cleanup
- Moved I2C related definations in the acpi/tpm.h

---
V4:

Incorporated review comments by Stefan

- Remove the check for locality while calculating checksum
- Use bswap16 instead of cpu_ti_be16.
- Rename TPM_I2C register by dropping _TIS_ from it.
---
 hw/tpm/tpm_tis.h|  3 +++
 hw/tpm/tpm_tis_common.c | 28 
 include/hw/acpi/tpm.h   | 28 
 3 files changed, 59 insertions(+)

diff --git a/hw/tpm/tpm_tis.h b/hw/tpm/tpm_tis.h
index f6b5872ba6..6f29a508dd 100644
--- a/hw/tpm/tpm_tis.h
+++ b/hw/tpm/tpm_tis.h
@@ -86,5 +86,8 @@ int tpm_tis_pre_save(TPMState *s);
 void tpm_tis_reset(TPMState *s);
 enum TPMVersion tpm_tis_get_tpm_version(TPMState *s);
 void tpm_tis_request_completed(TPMState *s, int ret);
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size);
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size);
+uint16_t tpm_tis_get_checksum(TPMState *s);
 
 #endif /* TPM_TPM_TIS_H */
diff --git a/hw/tpm/tpm_tis_common.c b/hw/tpm/tpm_tis_common.c
index 503be2a541..c6d139943e 100644
--- a/hw/tpm/tpm_tis_common.c
+++ b/hw/tpm/tpm_tis_common.c
@@ -26,6 +26,8 @@
 #include "hw/irq.h"
 #include "hw/isa/isa.h"
 #include "qapi/error.h"
+#include "qemu/bswap.h"
+#include "qemu/crc-ccitt.h"
 #include "qemu/module.h"
 
 #include "hw/acpi/tpm.h"
@@ -447,6 +449,23 @@ static uint64_t tpm_tis_mmio_read(void *opaque, hwaddr 
addr,
 return val;
 }
 
+/*
+ * A wrapper read function so that it can be directly called without
+ * mmio.
+ */
+uint32_t tpm_tis_read_data(TPMState *s, hwaddr addr, unsigned size)
+{
+return tpm_tis_mmio_read(s, addr, size);
+}
+
+/*
+ * Calculate current data buffer checksum
+ */
+uint16_t tpm_tis_get_checksum(TPMState *s)
+{
+return bswap16(crc_ccitt(0, s->buffer, s->rw_offset));
+}
+
 /*
  * Write a value to a register of the TIS interface
  * See specs pages 33-63 for description of the registers
@@ -767,6 +786,15 @@ static void tpm_tis_mmio_write(void *opaque, hwaddr addr,
 }
 }
 
+/*
+ * A wrapper write function so that it can be directly called without
+ * mmio.
+ */
+void tpm_tis_write_data(TPMState *s, hwaddr addr, uint64_t val, uint32_t size)
+{
+tpm_tis_mmio_write(s, addr, val, size);
+}
+
 const MemoryRegionOps tpm_tis_memory_ops = {
 .read = tpm_tis_mmio_read,
 .write = tpm_tis_mmio_write,
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 559ba6906c..4f2424e2fe 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -93,6 +93,7 @@
 #define TPM_TIS_CAP_DATA_TRANSFER_64B(3 << 9)
 #define TPM_TIS_CAP_DATA_TRANSFER_LEGACY (0 << 9)
 #define TPM_TIS_CAP_BURST_COUNT_DYNAMIC  (0 << 8)
+#define TPM_TIS_CAP_BURST_COUNT_STATIC   (1 << 8)
 #define TPM_TIS_CAP_INTERRUPT_LOW_LEVEL  (1 << 4) /* support is mandatory */
 #define TPM_TIS_CAPABILITIES_SUPPORTED1_3 \
 (TPM_TIS_CAP_INTERRUPT_LOW_LEVEL | \
@@ -209,6 +210,33 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
 #define TPM_PPI_FUNC_MASK(7 << 0)
 
+/* TPM TIS I2C registers */
+#define TPM_I2C_REG_LOC_SEL  0x00
+#define TPM_I2C_REG_ACCESS   0x04
+#define TPM_I2C_REG_INT_ENABLE   0x08
+#define TPM_I2C_REG_INT_CAPABILITY   0x14
+#define TPM_I2C_REG_STS  0x18
+#define TPM_I2C_REG_DATA_FIFO0x24
+#define TPM_I2C_REG_INTF_CAPABILITY  0x30
+#define TPM_I2C_REG_I2C_DEV_ADDRESS  0x38
+#define TPM_I2C_REG_DATA_CSUM_ENABLE 0x40
+#define TPM_I2C_REG_DATA_CSUM_GET0x44
+#define TPM_I2C_REG_DID_VID  0x48
+#define TPM_I2C_REG_RID  0x4c
+#define TPM_I2C_REG_UNKNOWN  0xff
+
+/* I2C specific interface capabilities */
+#define TPM_I2C_CAP_INTERFACE_TYPE (0x2 << 0)   /* FIFO interface */
+#define TPM_I2C_CAP_INTERFACE_VER  (0x0 << 4)   /* TCG I2C intf 1.0 */
+#define TPM_I2C_CAP_TPM2_FAMILY(0x1 << 7)   /* TPM 2.0 family. */
+#define TPM_I2C_CAP_DEV_ADDR_CHANGE(0x0 << 27)  /* No dev addr chng */
+#define TPM_I2C_CAP_BURST_COUNT_STATIC (0x1 << 29)  /* Burst count static 
*/
+#define TPM_I2C_CAP_LOCALITY_CAP   (0x1 << 25)  /* 0-5 locality */
+#define TPM_

[PATCH v6 1/3] docs: Add support for TPM devices over I2C bus

[Ninad Palsule]

This is a documentation change for I2C TPM device support.

Qemu already supports devices attached to ISA and sysbus.
This drop adds support for the I2C bus attached TPM devices.

Signed-off-by: Ninad Palsule 

---
V2:

Incorporated Stephen's review comments
- Added example in the document.

---
V4:
Incorporate Cedric & Stefan's comments

- Added example for ast2600-evb
- Corrected statement about arm virtual machine.

---
V6:
Incorporated review comments from Stefan.
---
 docs/specs/tpm.rst | 32 
 1 file changed, 32 insertions(+)

diff --git a/docs/specs/tpm.rst b/docs/specs/tpm.rst
index 535912a92b..590e670a9a 100644
--- a/docs/specs/tpm.rst
+++ b/docs/specs/tpm.rst
@@ -21,12 +21,16 @@ QEMU files related to TPM TIS interface:
  - ``hw/tpm/tpm_tis_common.c``
  - ``hw/tpm/tpm_tis_isa.c``
  - ``hw/tpm/tpm_tis_sysbus.c``
+ - ``hw/tpm/tpm_tis_i2c.c``
  - ``hw/tpm/tpm_tis.h``
 
 Both an ISA device and a sysbus device are available. The former is
 used with pc/q35 machine while the latter can be instantiated in the
 Arm virt machine.
 
+An I2C device support is also provided which can be instantiated in the Arm
+based emulation machines. This device only supports the TPM 2 protocol.
+
 CRB interface
 -
 
@@ -348,6 +352,34 @@ In case an Arm virt machine is emulated, use the following 
command line:
 -drive if=pflash,format=raw,file=flash0.img,readonly=on \
 -drive if=pflash,format=raw,file=flash1.img
 
+In case a ast2600-evb bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M ast2600-evb -nographic \
+-kernel arch/arm/boot/zImage \
+-dtb arch/arm/boot/dts/aspeed-ast2600-evb.dtb \
+-initrd rootfs.cpio \
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
+In case a Rainier bmc machine is emulated and want to use TPM device
+attached to I2C bus, use the following command line:
+
+.. code-block:: console
+
+  qemu-system-arm -M rainier-bmc -nographic \
+-kernel ${IMAGEPATH}/fitImage-linux.bin \
+-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
+-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
+-drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2\
+-net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443\
+-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
+-tpmdev emulator,id=tpm0,chardev=chrtpm \
+-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e
+
 In case SeaBIOS is used as firmware, it should show the TPM menu item
 after entering the menu with 'ESC'.
 
-- 
2.37.2

[PATCH v6 3/3] tpm: Add support for TPM device over I2C bus

[Ninad Palsule]

Qemu already supports devices attached to ISA and sysbus. This drop adds
support for the I2C bus attached TPM devices. I2C model only supports
TPM2 protocol.

This commit includes changes for the common code.
- Added I2C emulation model. Logic was added in the model to temporarily
  cache the data as I2C interface works per byte basis.
- New tpm type "tpm-tis-i2c" added for I2C support. The user has to
  provide this string on command line.

Testing:
  TPM I2C device module is tested using SWTPM (software based TPM
  package). Qemu uses the rainier machine and is connected to swtpm over
  the socket interface.

  The command to start swtpm is as follows:
  $ swtpm socket --tpmstate dir=/tmp/mytpm1\
 --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock  \
 --tpm2 --log level=100

  The command to start qemu is as follows:
  $ qemu-system-arm -M rainier-bmc -nographic \
-kernel ${IMAGEPATH}/fitImage-linux.bin \
-dtb ${IMAGEPATH}/aspeed-bmc-ibm-rainier.dtb \
-initrd ${IMAGEPATH}/obmc-phosphor-initramfs.rootfs.cpio.xz \
-drive 
file=${IMAGEPATH}/obmc-phosphor-image.rootfs.wic.qcow2,if=sd,index=2 \
-net nic -net 
user,hostfwd=:127.0.0.1:-:22,hostfwd=:127.0.0.1:2443-:443 \
-chardev socket,id=chrtpm,path=/tmp/mytpm1/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis-i2c,tpmdev=tpm0,bus=aspeed.i2c.bus.12,address=0x2e

Signed-off-by: Ninad Palsule 
---
V2:
Incorporated Stephen's review comments.
- Handled checksum related register in I2C layer
- Defined I2C interface capabilities and return those instead of
  capabilities from TPM TIS. Add required capabilities from TIS.
- Do not cache FIFO data in the I2C layer.
- Make sure that Device address change register is not passed to I2C
  layer as capability indicate that it is not supported.
- Added boundary checks.
- Make sure that bits 26-31 are zeroed for the TPM_STS register on read
- Updated Kconfig files for new define.

---
V3:
- Moved processing of register TPM_I2C_LOC_SEL in the I2C. So I2C layer
  remembers the locality and pass it to TIS on each read/write.
- The write data is no more cached in the I2C layer so the buffer size
  is reduced to 16 bytes.
- Checksum registers are now managed by the I2C layer. Added new
  function in TIS layer to return the checksum and used that to process
  the request.
- Now 2-4 byte register value will be passed to TIS layer in a single
  write call instead of 1 byte at a time. Added functions to convert
  between little endian stream of bytes to single 32 bit unsigned
  integer. Similarly 32  bit integer to stream of bytes.
- Added restriction on device change register.
- Replace few if-else statement with switch statement for clarity.
- Log warning when unknown register is received.
- Moved all register definations to acpi/tmp.h

---
V4:
Incorporated review comments from Cedric and Stefan.
- Reduced data[] size from 16 byte to 5 bytes.
- Added register name in the mapping table which can be used for
  tracing.
- Removed the endian conversion functions instead used simple logic
  provided by Stefan.
- Rename I2C registers to reduce the length.
- Added traces for send, recv and event functions. You can turn on trace
  on command line by using "-trace "tpm_tis_i2c*" option.

---
V5:
Fixed issues reported by Stefan's test.
- Added mask for the INT_ENABLE register.
- Use correct TIS register for reading interrupt capability.
- Cleanup how register is converted from I2C to TIS and also saved
  information like tis_addr and register name in the i2cst so that we
  can only convert it once on i2c_send.
- Trace register number for unknown registers.

---
V6:
Fixed review comments from Stefan.
- Fixed some variable size.
- Removed unused variables.
- Added vmstat backin to handle migration.
- Added post load phase to reload tis address and register name.
---
 hw/arm/Kconfig   |   1 +
 hw/tpm/Kconfig   |   7 +
 hw/tpm/meson.build   |   1 +
 hw/tpm/tpm_tis_i2c.c | 533 +++
 hw/tpm/trace-events  |   6 +
 include/sysemu/tpm.h |   3 +
 6 files changed, 551 insertions(+)
 create mode 100644 hw/tpm/tpm_tis_i2c.c

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index b5aed4aff5..05d6ef1a31 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -6,6 +6,7 @@ config ARM_VIRT
 imply VFIO_PLATFORM
 imply VFIO_XGMAC
 imply TPM_TIS_SYSBUS
+imply TPM_TIS_I2C
 imply NVDIMM
 select ARM_GIC
 select ACPI
diff --git a/hw/tpm/Kconfig b/hw/tpm/Kconfig
index 29e82f3c92..a46663288c 100644
--- a/hw/tpm/Kconfig
+++ b/hw/tpm/Kconfig
@@ -1,3 +1,10 @@
+config TPM_TIS_I2C
+bool
+depends on TPM
+select TPM_BACKEND
+select I2C
+select TPM_TIS
+
 config TPM_TIS_ISA
 bool
 depends on TPM && ISA_BUS
diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index 7abc2d794a..76fe3cb098 100644
--- a/hw/tpm/meson.build
+++ b/hw/tpm/mes

Re: [PATCH for-8.0 00/11] tcg patch queue

[Richard Henderson]

Ping.

On 3/22/23 08:07, Richard Henderson wrote:

Posting pre-PR because I had to adjust Emilio's QTree patch [1],
and added a new patch to avoid an assert that can be generated
with incorrect -R reserved_va values vs the ARM commpage.

r~

[1] https://gitlab.com/rth7680/qemu/-/jobs/3975817279#L92

Emilio Cota (2):
   util: import GTree as QTree
   tcg: use QTree instead of GTree

Richard Henderson (9):
   linux-user: Diagnose misaligned -R size
   include/exec: Change reserved_va semantics to last byte
   accel/tcg: Pass last not end to page_set_flags
   accel/tcg: Pass last not end to page_reset_target_data
   accel/tcg: Pass last not end to PAGE_FOR_EACH_TB
   accel/tcg: Pass last not end to page_collection_lock
   accel/tcg: Pass last not end to tb_invalidate_phys_page_range__locked
   accel/tcg: Pass last not end to tb_invalidate_phys_range
   linux-user/arm: Take more care allocating commpage

  configure   |   15 +
  meson.build |4 +
  include/exec/cpu-all.h  |   15 +-
  include/exec/exec-all.h |2 +-
  include/qemu/qtree.h|  201 +
  linux-user/arm/target_cpu.h |2 +-
  accel/tcg/tb-maint.c|  112 +--
  accel/tcg/translate-all.c   |2 +-
  accel/tcg/user-exec.c   |   25 +-
  bsd-user/main.c |   10 +-
  bsd-user/mmap.c |   10 +-
  linux-user/elfload.c|   67 +-
  linux-user/main.c   |   31 +-
  linux-user/mmap.c   |   22 +-
  linux-user/syscall.c|4 +-
  softmmu/physmem.c   |2 +-
  tcg/region.c|   19 +-
  tests/bench/qtree-bench.c   |  286 +++
  tests/unit/test-qtree.c |  333 +
  util/qtree.c| 1390 +++
  tests/bench/meson.build |4 +
  tests/unit/meson.build  |1 +
  util/meson.build|1 +
  23 files changed, 2412 insertions(+), 146 deletions(-)
  create mode 100644 include/qemu/qtree.h
  create mode 100644 tests/bench/qtree-bench.c
  create mode 100644 tests/unit/test-qtree.c
  create mode 100644 util/qtree.c

Re: [PATCH v6 00/25] target/riscv: MSTATUS_SUM + cleanups

[Richard Henderson]

On 3/25/23 03:54, Richard Henderson wrote:

This builds on Fei and Zhiwei's SUM and TB_FLAGS changes.

   * Reclaim 5 TB_FLAGS bits, since we nearly ran out.

   * Using cpu_mmu_index(env, true) is insufficient to implement
 HLVX properly.  While that chooses the correct mmu_idx, it
 does not perform the read with execute permission.
 I add a new tcg interface to perform a read-for-execute with
 an arbitrary mmu_idx.  This is still not 100% compliant, but
 it's closer.

   * Handle mstatus.MPV in cpu_mmu_index.
   * Use vsstatus.SUM when required for MMUIdx_S_SUM.
   * Cleanups for get_physical_address.

While this passes check-avocado, I'm sure that's insufficient.
Please have a close look.


Somewhere after either patch 16 or 17, when env->virt is considered in riscv_cpu_mmu_index 
and a few other bugs are fixed, we can do


--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -591,11 +591,6 @@ void riscv_cpu_set_virt_enabled
 return;
 }

-/* Flush the TLB on all virt mode changes. */
-if (get_field(env->virt, VIRT_ONOFF) != enable) {
-tlb_flush(env_cpu(env));
-}
-
 env->virt = set_field(env->virt, VIRT_ONOFF, enable);

 if (enable) {
-- %< --

Because we're no longer trying to overlap the VS and HS tlbs on the same mmuidx.

I have been pondering additional changes that would avoid flushing for MXR changes, so 
that user-lookups from M-mode firmware gets the same advantage as has just been done for 
SUM.  But this is complicated by needing 12 (!) more mmuidx, which cannot currently be 
represented, nor does it even seem like the best idea.



r~

[PATCH v2 0/5] Support x2APIC mode with TCG accelerator

[Bui Quang Minh]

Hi everyone,

This series implements x2APIC mode in userspace local APIC and the
RDMSR/WRMSR helper to access x2APIC registers in x2APIC mode. Intel iommu
and AMD iommu are adjusted to support x2APIC interrupt remapping. With this
series, we can now boot Linux kernel into x2APIC mode with TCG accelerator
using either Intel or AMD iommu.

Testing the emulated userspace APIC with kvm-unit-tests, disable test
device with this patch

diff --git a/lib/x86/fwcfg.c b/lib/x86/fwcfg.c
index 1734afb..f56fe1c 100644
--- a/lib/x86/fwcfg.c
+++ b/lib/x86/fwcfg.c
@@ -27,6 +27,7 @@ static void read_cfg_override(void)
 
if ((str = getenv("TEST_DEVICE")))
no_test_device = !atol(str);
+   no_test_device = true;
 
if ((str = getenv("MEMLIMIT")))
fw_override[FW_CFG_MAX_RAM] = atol(str) * 1024 * 1024;

~ env QEMU=/home/minh/Desktop/oss/qemu/build/qemu-system-x86_64 ACCEL=tcg \
./run_tests.sh -v -g apic 

TESTNAME=apic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/apic.flat -smp 2
-cpu qemu64,+x2apic,+tsc-deadline -machine kernel_irqchip=split FAIL
apic-split (54 tests, 8 unexpected failures, 1 skipped)
TESTNAME=ioapic-split TIMEOUT=90s ACCEL=tcg ./x86/run x86/ioapic.flat -smp
1 -cpu qemu64 -machine kernel_irqchip=split PASS ioapic-split (19 tests)
TESTNAME=x2apic TIMEOUT=30 ACCEL=tcg ./x86/run x86/apic.flat -smp 2 -cpu
qemu64,+x2apic,+tsc-deadline FAIL x2apic (54 tests, 8 unexpected failures,
1 skipped) TESTNAME=xapic TIMEOUT=60 ACCEL=tcg ./x86/run x86/apic.flat -smp
2 -cpu qemu64,-x2apic,+tsc-deadline -machine pit=off FAIL xapic (43 tests,
6 unexpected failures, 2 skipped)

FAIL: apic_disable: *0xfee00030: 50014
FAIL: apic_disable: *0xfee00080: f0
FAIL: apic_disable: *0xfee00030: 50014
FAIL: apic_disable: *0xfee00080: f0 
FAIL: apicbase: relocate apic

These errors are because we don't disable MMIO region when switching to
x2APIC and don't support relocate MMIO region yet. This is a problem
because, MMIO region is the same for all CPUs, in order to support these we
need to figure out how to allocate and manage different MMIO regions for
each CPUs. This can be an improvement in the future.

FAIL: nmi-after-sti
FAIL: multiple nmi

These errors are in the way we handle CPU_INTERRUPT_NMI in core TCG.

FAIL: TMCCT should stay at zero

This error is related to APIC timer which should be addressed in separate
patch.

Version 2 changes,
- Add support for APIC ID larger than 255
- Adjust AMD iommu for x2APIC suuport
- Reorganize and split patch 1,2 into patch 1,2,3 in version 2

Thanks,
Quang Minh.

Bui Quang Minh (5):
  i386/tcg: implement x2APIC registers MSR access
  apic: add support for x2APIC mode
  apic, i386/tcg: add x2apic transitions
  intel_iommu: allow Extended Interrupt Mode when using userspace APIC
  amd_iommu: report x2APIC support to the operating system

 hw/i386/acpi-build.c |  28 +--
 hw/i386/amd_iommu.c  |  21 +-
 hw/i386/amd_iommu.h  |  16 +-
 hw/i386/intel_iommu.c|  11 -
 hw/i386/x86.c|   8 +-
 hw/intc/apic.c   | 358 ---
 hw/intc/apic_common.c|  15 +-
 hw/intc/trace-events |   4 +-
 include/hw/i386/apic.h   |   6 +-
 include/hw/i386/apic_internal.h  |   2 +-
 target/i386/cpu-sysemu.c |  10 +
 target/i386/cpu.c|   5 +-
 target/i386/cpu.h|   9 +
 target/i386/tcg/sysemu/misc_helper.c |  31 +++
 14 files changed, 389 insertions(+), 135 deletions(-)

-- 
2.25.1

[PATCH v2 1/5] i386/tcg: implement x2APIC registers MSR access

[Bui Quang Minh]

This commit refactors apic_mem_read/write to support both MMIO access in
xAPIC and MSR access in x2APIC.

Signed-off-by: Bui Quang Minh 
---
 hw/intc/apic.c   | 79 ++--
 hw/intc/trace-events |  4 +-
 include/hw/i386/apic.h   |  3 ++
 target/i386/cpu.h|  3 ++
 target/i386/tcg/sysemu/misc_helper.c | 27 ++
 5 files changed, 86 insertions(+), 30 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 20b5a94073..61b494b20a 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -288,6 +288,13 @@ void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, 
uint8_t delivery_mode,
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
 }
 
+bool is_x2apic_mode(DeviceState *dev)
+{
+APICCommonState *s = APIC(dev);
+
+return s->apicbase & MSR_IA32_APICBASE_EXTD;
+}
+
 static void apic_set_base(APICCommonState *s, uint64_t val)
 {
 s->apicbase = (val & 0xf000) |
@@ -636,16 +643,11 @@ static void apic_timer(void *opaque)
 apic_timer_update(s, s->next_time);
 }
 
-static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
+uint64_t apic_register_read(int index)
 {
 DeviceState *dev;
 APICCommonState *s;
-uint32_t val;
-int index;
-
-if (size < 4) {
-return 0;
-}
+uint64_t val;
 
 dev = cpu_get_current_apic();
 if (!dev) {
@@ -653,7 +655,6 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, 
unsigned size)
 }
 s = APIC(dev);
 
-index = (addr >> 4) & 0xff;
 switch(index) {
 case 0x02: /* id */
 val = s->id << 24;
@@ -720,7 +721,23 @@ static uint64_t apic_mem_read(void *opaque, hwaddr addr, 
unsigned size)
 val = 0;
 break;
 }
-trace_apic_mem_readl(addr, val);
+
+trace_apic_register_read(index, val);
+return val;
+}
+
+static uint64_t apic_mem_read(void *opaque, hwaddr addr, unsigned size)
+{
+uint32_t val;
+int index;
+
+if (size < 4) {
+return 0;
+}
+
+index = (addr >> 4) & 0xff;
+val = (uint32_t)apic_register_read(index);
+
 return val;
 }
 
@@ -737,27 +754,10 @@ static void apic_send_msi(MSIMessage *msi)
 apic_deliver_irq(dest, dest_mode, delivery, vector, trigger_mode);
 }
 
-static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
-   unsigned size)
+void apic_register_write(int index, uint64_t val)
 {
 DeviceState *dev;
 APICCommonState *s;
-int index = (addr >> 4) & 0xff;
-
-if (size < 4) {
-return;
-}
-
-if (addr > 0xfff || !index) {
-/* MSI and MMIO APIC are at the same memory location,
- * but actually not on the global bus: MSI is on PCI bus
- * APIC is connected directly to the CPU.
- * Mapping them on the global bus happens to work because
- * MSI registers are reserved in APIC MMIO and vice versa. */
-MSIMessage msi = { .address = addr, .data = val };
-apic_send_msi(&msi);
-return;
-}
 
 dev = cpu_get_current_apic();
 if (!dev) {
@@ -765,7 +765,7 @@ static void apic_mem_write(void *opaque, hwaddr addr, 
uint64_t val,
 }
 s = APIC(dev);
 
-trace_apic_mem_writel(addr, val);
+trace_apic_register_write(index, val);
 
 switch(index) {
 case 0x02:
@@ -843,6 +843,29 @@ static void apic_mem_write(void *opaque, hwaddr addr, 
uint64_t val,
 }
 }
 
+static void apic_mem_write(void *opaque, hwaddr addr, uint64_t val,
+   unsigned size)
+{
+int index = (addr >> 4) & 0xff;
+
+if (size < 4) {
+return;
+}
+
+if (addr > 0xfff || !index) {
+/* MSI and MMIO APIC are at the same memory location,
+ * but actually not on the global bus: MSI is on PCI bus
+ * APIC is connected directly to the CPU.
+ * Mapping them on the global bus happens to work because
+ * MSI registers are reserved in APIC MMIO and vice versa. */
+MSIMessage msi = { .address = addr, .data = val };
+apic_send_msi(&msi);
+return;
+}
+
+apic_register_write(index, val);
+}
+
 static void apic_pre_save(APICCommonState *s)
 {
 apic_sync_vapic(s, SYNC_FROM_VAPIC);
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 50cadfb996..9d4e7a67be 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -14,8 +14,8 @@ cpu_get_apic_base(uint64_t val) "0x%016"PRIx64
 # apic.c
 apic_local_deliver(int vector, uint32_t lvt) "vector %d delivery mode %d"
 apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode, 
uint8_t vector_num, uint8_t trigger_mode) "dest %d dest_mode %d delivery_mode 
%d vector %d trigger_mode %d"
-apic_mem_readl(uint64_t addr, uint32_t val)  "0x%"PRIx64" = 0x%08x"
-apic_mem_writel(uint64_t addr, uint32_t val) "0x%"PRIx64" = 0x%08x"
+apic_register_read(uint8_t reg, uint64_t val) "register 0x%02x = 0x%"PRIx64
+apic_register_write(uin

[PATCH v2 3/5] apic, i386/tcg: add x2apic transitions

[Bui Quang Minh]

This commit adds support for x2APIC transitions when writing to
MSR_IA32_APICBASE register and finally adds CPUID_EXT_X2APIC to
TCG_EXT_FEATURES.

Signed-off-by: Bui Quang Minh 
---
 hw/intc/apic.c   | 50 
 hw/intc/apic_common.c|  7 ++--
 target/i386/cpu-sysemu.c | 10 ++
 target/i386/cpu.c|  5 +--
 target/i386/cpu.h|  6 
 target/i386/tcg/sysemu/misc_helper.c |  4 +++
 6 files changed, 75 insertions(+), 7 deletions(-)

diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 159527af30..735412274d 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -308,8 +308,41 @@ bool is_x2apic_mode(DeviceState *dev)
 return s->apicbase & MSR_IA32_APICBASE_EXTD;
 }
 
+static void apic_set_base_check(APICCommonState *s, uint64_t val)
+{
+/* Enable x2apic when x2apic is not supported by CPU */
+if (!cpu_has_x2apic_feature(&s->cpu->env) &&
+val & MSR_IA32_APICBASE_EXTD)
+raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
+
+/*
+ * Transition into invalid state
+ * (s->apicbase & MSR_IA32_APICBASE_ENABLE == 0) &&
+ * (s->apicbase & MSR_IA32_APICBASE_EXTD) == 1
+ */
+if (!(val & MSR_IA32_APICBASE_ENABLE) &&
+(val & MSR_IA32_APICBASE_EXTD))
+raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
+
+/* Invalid transition from disabled mode to x2APIC */
+if (!(s->apicbase & MSR_IA32_APICBASE_ENABLE) &&
+!(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
+(val & MSR_IA32_APICBASE_ENABLE) &&
+(val & MSR_IA32_APICBASE_EXTD))
+raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
+
+/* Invalid transition from x2APIC to xAPIC */
+if ((s->apicbase & MSR_IA32_APICBASE_ENABLE) &&
+(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
+(val & MSR_IA32_APICBASE_ENABLE) &&
+!(val & MSR_IA32_APICBASE_EXTD))
+raise_exception_ra(&s->cpu->env, EXCP0D_GPF, GETPC());
+}
+
 static void apic_set_base(APICCommonState *s, uint64_t val)
 {
+apic_set_base_check(s, val);
+
 s->apicbase = (val & 0xf000) |
 (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
 /* if disabled, cannot be enabled again */
@@ -318,6 +351,23 @@ static void apic_set_base(APICCommonState *s, uint64_t val)
 cpu_clear_apic_feature(&s->cpu->env);
 s->spurious_vec &= ~APIC_SV_ENABLE;
 }
+
+/* Transition from disabled mode to xAPIC */
+if (!(s->apicbase & MSR_IA32_APICBASE_ENABLE) &&
+(val & MSR_IA32_APICBASE_ENABLE)) {
+s->apicbase |= MSR_IA32_APICBASE_ENABLE;
+cpu_set_apic_feature(&s->cpu->env);
+}
+
+/* Transition from xAPIC to x2APIC */
+if (cpu_has_x2apic_feature(&s->cpu->env) &&
+!(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
+(val & MSR_IA32_APICBASE_EXTD)) {
+s->apicbase |= MSR_IA32_APICBASE_EXTD;
+
+s->log_dest = ((s->initial_apic_id & 0x0) << 16) |
+  (1 << (s->initial_apic_id & 0xf));
+}
 }
 
 static void apic_set_tpr(APICCommonState *s, uint8_t val)
diff --git a/hw/intc/apic_common.c b/hw/intc/apic_common.c
index 39c207bd21..3d249838fc 100644
--- a/hw/intc/apic_common.c
+++ b/hw/intc/apic_common.c
@@ -43,11 +43,8 @@ void cpu_set_apic_base(DeviceState *dev, uint64_t val)
 if (dev) {
 APICCommonState *s = APIC_COMMON(dev);
 APICCommonClass *info = APIC_COMMON_GET_CLASS(s);
-/* switching to x2APIC, reset possibly modified xAPIC ID */
-if (!(s->apicbase & MSR_IA32_APICBASE_EXTD) &&
-(val & MSR_IA32_APICBASE_EXTD)) {
-s->id = s->initial_apic_id;
-}
+/* Reset possibly modified xAPIC ID */
+s->id = s->initial_apic_id;
 info->set_base(s, val);
 }
 }
diff --git a/target/i386/cpu-sysemu.c b/target/i386/cpu-sysemu.c
index 28115edf44..de57892c25 100644
--- a/target/i386/cpu-sysemu.c
+++ b/target/i386/cpu-sysemu.c
@@ -235,6 +235,16 @@ void cpu_clear_apic_feature(CPUX86State *env)
 env->features[FEAT_1_EDX] &= ~CPUID_APIC;
 }
 
+void cpu_set_apic_feature(CPUX86State *env)
+{
+env->features[FEAT_1_EDX] |= CPUID_APIC;
+}
+
+bool cpu_has_x2apic_feature(CPUX86State *env)
+{
+return env->features[FEAT_1_ECX] & CPUID_EXT_X2APIC;
+}
+
 bool cpu_is_bsp(X86CPU *cpu)
 {
 return cpu_get_apic_base(cpu->apic_state) & MSR_IA32_APICBASE_BSP;
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6576287e5b..6847b2ae02 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -627,12 +627,13 @@ void x86_cpu_vendor_words2str(char *dst, uint32_t vendor1,
   CPUID_EXT_XSAVE | /* CPUID_EXT_OSXSAVE is dynamic */   \
   CPUID_EXT_MOVBE | CPUID_EXT_AES | CPUID_EXT_HYPERVISOR | \
   CPUID_EXT_RDRAND | CPUID_EXT_AVX | CPUID_EXT_F16C | \
-  CPUID_EXT_FMA)
+  CPUID_EXT_FMA | CPUID_EXT_X2APIC)
   /* missing:
   CPUID_EXT_DTES64, CPUID_EXT_DSCP

[PATCH v2 5/5] amd_iommu: report x2APIC support to the operating system

[Bui Quang Minh]

This commit adds XTSup configuration to let user choose to whether enable
this feature or not. When XTSup is enabled, additional bytes in IRTE with
enabled guest virtual VAPIC are used to support 32-bit destination id.

Additionally, this commit changes to use IVHD type 0x11 in ACPI table for
feature report to operating system. This is because Linux does not use
XTSup in IOMMU Feature Reporting field of IVHD type 0x10 but only use XTSup
bit in EFR Register Image of IVHD 0x11 to indicate x2APIC support (see
init_iommu_one in linux/drivers/iommu/amd/init.c)

Signed-off-by: Bui Quang Minh 
---
 hw/i386/acpi-build.c | 28 ++--
 hw/i386/amd_iommu.c  | 21 +++--
 hw/i386/amd_iommu.h  | 16 +++-
 3 files changed, 44 insertions(+), 21 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ec857a117e..72d6bb2892 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2339,7 +2339,7 @@ static void
 build_amd_iommu(GArray *table_data, BIOSLinker *linker, const char *oem_id,
 const char *oem_table_id)
 {
-int ivhd_table_len = 24;
+int ivhd_table_len = 40;
 AMDVIState *s = AMD_IOMMU_DEVICE(x86_iommu_get_default());
 GArray *ivhd_blob = g_array_new(false, true, 1);
 AcpiTable table = { .sig = "IVRS", .rev = 1, .oem_id = oem_id,
@@ -2349,18 +2349,19 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, 
const char *oem_id,
 /* IVinfo - IO virtualization information common to all
  * IOMMU units in a system
  */
-build_append_int_noprefix(table_data, 40UL << 8/* PASize */, 4);
+build_append_int_noprefix(table_data,
+ (1UL << 0) | /* EFRSup */
+ (40UL << 8), /* PASize */
+ 4);
 /* reserved */
 build_append_int_noprefix(table_data, 0, 8);
 
-/* IVHD definition - type 10h */
-build_append_int_noprefix(table_data, 0x10, 1);
+/* IVHD definition - type 11h */
+build_append_int_noprefix(table_data, 0x11, 1);
 /* virtualization flags */
 build_append_int_noprefix(table_data,
  (1UL << 0) | /* HtTunEn  */
- (1UL << 4) | /* iotblSup */
- (1UL << 6) | /* PrefSup  */
- (1UL << 7),  /* PPRSup   */
+ (1UL << 4),  /* iotblSup */
  1);
 
 /*
@@ -2404,13 +2405,12 @@ build_amd_iommu(GArray *table_data, BIOSLinker *linker, 
const char *oem_id,
 build_append_int_noprefix(table_data, 0, 2);
 /* IOMMU info */
 build_append_int_noprefix(table_data, 0, 2);
-/* IOMMU Feature Reporting */
-build_append_int_noprefix(table_data,
- (48UL << 30) | /* HATS   */
- (48UL << 28) | /* GATS   */
- (1UL << 2)   | /* GTSup  */
- (1UL << 6),/* GASup  */
- 4);
+/* IOMMU Attributes */
+build_append_int_noprefix(table_data, 0, 4);
+/* EFR Register Image */
+build_append_int_noprefix(table_data, s->efr_reg, 8);
+/* EFR Register Image 2 */
+build_append_int_noprefix(table_data, 0, 8);
 
 /* IVHD entries as found above */
 g_array_append_vals(table_data, ivhd_blob->data, ivhd_blob->len);
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index bcd016f5c5..5dfa93d945 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -31,6 +31,7 @@
 #include "hw/i386/apic_internal.h"
 #include "trace.h"
 #include "hw/i386/apic-msidef.h"
+#include "hw/qdev-properties.h"
 
 /* used AMD-Vi MMIO registers */
 const char *amdvi_mmio_low[] = {
@@ -1155,7 +1156,12 @@ static int amdvi_int_remap_ga(AMDVIState *iommu,
 irq->vector = irte.hi.fields.vector;
 irq->dest_mode = irte.lo.fields_remap.dm;
 irq->redir_hint = irte.lo.fields_remap.rq_eoi;
-irq->dest = irte.lo.fields_remap.destination;
+if (!iommu->xtsup) {
+irq->dest = irte.lo.fields_remap.destination & 0xff;
+} else {
+irq->dest = irte.lo.fields_remap.destination |
+(irte.hi.fields.destination_hi << 24);
+}
 
 return 0;
 }
@@ -1503,10 +1509,15 @@ static void amdvi_init(AMDVIState *s)
 s->enabled = false;
 s->ats_enabled = false;
 s->cmdbuf_enabled = false;
+s->efr_reg = AMDVI_DEFAULT_EXT_FEATURES;
+
+if (s->xtsup) {
+s->efr_reg |= AMDVI_FEATURE_XT;
+}
 
 /* reset MMIO */
 memset(s->mmior, 0, AMDVI_MMIO_SIZE);
-amdvi_set_quad(s, AMDVI_MMIO_EXT_FEATURES, AMDVI_EXT_FEATURES,
+amdvi_set_quad(s, AMDVI_MMIO_EXT_FEATURES, s->efr_reg,
 0xffef, 0);
 amdvi_set_quad(s, AMDVI_MMIO_STATUS, 0, 0x98, 0x67);
 
@@ -1586,6 +1597,11 @@ static void amdvi_sysbus_realize(DeviceState *dev, Error 
**errp)
 amdvi_init(s);
 }
 
+static Property amdvi_properties[] = {
+ 

[PATCH v2 4/5] intel_iommu: allow Extended Interrupt Mode when using userspace APIC

[Bui Quang Minh]

As userspace APIC now supports x2APIC, intel interrupt remapping
hardware can be set to EIM mode when userspace local APIC is used.

Signed-off-by: Bui Quang Minh 
---
 hw/i386/intel_iommu.c | 11 ---
 1 file changed, 11 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index faade7def8..a34a4c8196 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -4045,17 +4045,6 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error 
**errp)
   && x86_iommu_ir_supported(x86_iommu) ?
   ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
 }
-if (s->intr_eim == ON_OFF_AUTO_ON && !s->buggy_eim) {
-if (!kvm_irqchip_is_split()) {
-error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split");
-return false;
-}
-if (!kvm_enable_x2apic()) {
-error_setg(errp, "eim=on requires support on the KVM side"
- "(X2APIC_API, first shipped in v4.7)");
-return false;
-}
-}
 
 /* Currently only address widths supported are 39 and 48 bits */
 if ((s->aw_bits != VTD_HOST_AW_39BIT) &&
-- 
2.25.1

[PATCH v2 2/5] apic: add support for x2APIC mode

[Bui Quang Minh]

This commit extends the APIC ID to 32-bit long and remove the 255 max APIC
ID limit in userspace APIC. The array that manages local APICs is now
dynamically allocated based on the max APIC ID of created x86 machine.
Also, new x2APIC IPI destination determination scheme, self IPI and x2APIC
mode register access are supported.

Signed-off-by: Bui Quang Minh 
---
 hw/i386/x86.c   |   8 +-
 hw/intc/apic.c  | 229 +++-
 hw/intc/apic_common.c   |   8 +-
 include/hw/i386/apic.h  |   3 +-
 include/hw/i386/apic_internal.h |   2 +-
 5 files changed, 184 insertions(+), 66 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index a88a126123..fa9b15190d 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -132,11 +132,11 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
  * Can we support APIC ID 255 or higher?
  *
  * Under Xen: yes.
- * With userspace emulated lapic: no
+ * With userspace emulated lapic: yes.
  * With KVM's in-kernel lapic: only if X2APIC API is enabled.
  */
 if (x86ms->apic_id_limit > 255 && !xen_enabled() &&
-(!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
+kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
 error_report("current -smp configuration requires kernel "
  "irqchip and X2APIC API support.");
 exit(EXIT_FAILURE);
@@ -146,6 +146,10 @@ void x86_cpus_init(X86MachineState *x86ms, int 
default_cpu_version)
 kvm_set_max_apic_id(x86ms->apic_id_limit);
 }
 
+if (!kvm_irqchip_in_kernel()) {
+apic_set_max_apic_id(x86ms->apic_id_limit);
+}
+
 possible_cpus = mc->possible_cpu_arch_ids(ms);
 for (i = 0; i < ms->smp.cpus; i++) {
 x86_cpu_new(x86ms, possible_cpus->cpus[i].arch_id, &error_fatal);
diff --git a/hw/intc/apic.c b/hw/intc/apic.c
index 61b494b20a..159527af30 100644
--- a/hw/intc/apic.c
+++ b/hw/intc/apic.c
@@ -31,15 +31,15 @@
 #include "hw/i386/apic-msidef.h"
 #include "qapi/error.h"
 #include "qom/object.h"
-
-#define MAX_APICS 255
-#define MAX_APIC_WORDS 8
+#include "tcg/helper-tcg.h"
 
 #define SYNC_FROM_VAPIC 0x1
 #define SYNC_TO_VAPIC   0x2
 #define SYNC_ISR_IRR_TO_VAPIC   0x4
 
-static APICCommonState *local_apics[MAX_APICS + 1];
+static APICCommonState **local_apics;
+static uint32_t max_apics;
+static uint32_t max_apic_words;
 
 #define TYPE_APIC "apic"
 /*This is reusing the APICCommonState typedef from APIC_COMMON */
@@ -49,7 +49,19 @@ DECLARE_INSTANCE_CHECKER(APICCommonState, APIC,
 static void apic_set_irq(APICCommonState *s, int vector_num, int trigger_mode);
 static void apic_update_irq(APICCommonState *s);
 static void apic_get_delivery_bitmask(uint32_t *deliver_bitmask,
-  uint8_t dest, uint8_t dest_mode);
+  uint32_t dest, uint8_t dest_mode);
+
+void apic_set_max_apic_id(uint32_t max_apic_id)
+{
+int word_size = 32;
+
+/* round up the max apic id to next multiple of words */
+max_apics = (max_apic_id + word_size - 1) & ~(word_size - 1);
+
+local_apics = g_malloc0(sizeof(*local_apics) * max_apics);
+max_apic_words = max_apics >> 5;
+}
+
 
 /* Find first bit starting from msb */
 static int apic_fls_bit(uint32_t value)
@@ -199,7 +211,7 @@ static void apic_external_nmi(APICCommonState *s)
 #define foreach_apic(apic, deliver_bitmask, code) \
 {\
 int __i, __j;\
-for(__i = 0; __i < MAX_APIC_WORDS; __i++) {\
+for(__i = 0; __i < max_apic_words; __i++) {\
 uint32_t __mask = deliver_bitmask[__i];\
 if (__mask) {\
 for(__j = 0; __j < 32; __j++) {\
@@ -226,7 +238,7 @@ static void apic_bus_deliver(const uint32_t 
*deliver_bitmask,
 {
 int i, d;
 d = -1;
-for(i = 0; i < MAX_APIC_WORDS; i++) {
+for(i = 0; i < max_apic_words; i++) {
 if (deliver_bitmask[i]) {
 d = i * 32 + apic_ffs_bit(deliver_bitmask[i]);
 break;
@@ -276,16 +288,17 @@ static void apic_bus_deliver(const uint32_t 
*deliver_bitmask,
  apic_set_irq(apic_iter, vector_num, trigger_mode) );
 }
 
-void apic_deliver_irq(uint8_t dest, uint8_t dest_mode, uint8_t delivery_mode,
+void apic_deliver_irq(uint32_t dest, uint8_t dest_mode, uint8_t delivery_mode,
   uint8_t vector_num, uint8_t trigger_mode)
 {
-uint32_t deliver_bitmask[MAX_APIC_WORDS];
+uint32_t *deliver_bitmask = g_malloc(max_apic_words * sizeof(uint32_t));
 
 trace_apic_deliver_irq(dest, dest_mode, delivery_mode, vector_num,
trigger_mode);
 
 apic_get_delivery_bitmask(deliver_bitmask, dest, dest_mode);
 apic_bus_deliver(deliver_bitmask, delivery_mode, vector_num, trigger_mode);
+g_free(deliver_bitmask);
 }
 
 bool is_x2apic_mode(DeviceState *dev)