qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2
@ 2019-07-30 23:35 Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 1/5] target/riscv: Don't set write permissions on dirty PTEs Alistair Francis
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Alistair Francis @ 2019-07-30 23:35 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair23, palmer, alistair.francis

The first three patches are ones that I have pulled out of my original
Hypervisor series at an attempt to reduce the number of patches in the
series.

These three patches all make sense without the Hypervisor series so can
be merged seperatley and will reduce the review burden of the next
version of the patches.

The fource patch is a prep patch for the new v0.4 Hypervisor spec.

The final patch is unreleated to Hypervisor that I'm just slipping in
here because it seems easier then sending it by itself.

v2:
 - Small corrections based on feedback
 - Remove the CSR permission check patch


Alistair Francis (4):
  target/riscv: Don't set write permissions on dirty PTEs
  riscv: plic: Remove unused interrupt functions
  target/riscv: Create function to test if FP is enabled
  target/riscv: Update the Hypervisor CSRs to v0.4

Atish Patra (1):
  target/riscv: Fix Floating Point register names

 hw/riscv/sifive_plic.c         | 12 ------------
 include/hw/riscv/sifive_plic.h |  3 ---
 target/riscv/cpu.c             |  8 ++++----
 target/riscv/cpu.h             |  6 +++++-
 target/riscv/cpu_bits.h        | 35 +++++++++++++++++-----------------
 target/riscv/cpu_helper.c      | 16 ++++++++++++----
 target/riscv/csr.c             | 20 ++++++++++---------
 7 files changed, 50 insertions(+), 50 deletions(-)

-- 
2.22.0



^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH-4.2 v2 1/5] target/riscv: Don't set write permissions on dirty PTEs
  2019-07-30 23:35 [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2 Alistair Francis
@ 2019-07-30 23:35 ` Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 2/5] riscv: plic: Remove unused interrupt functions Alistair Francis
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2019-07-30 23:35 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair23, palmer, alistair.francis

Setting write permission on dirty PTEs results in userspace inside a
Hypervisor guest (VU) becoming corrupted. This appears to be because it
ends up with write permission in the second stage translation in cases
where we aren't doing a store.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_helper.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index e32b6126af..f027be7f16 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -340,10 +340,8 @@ restart:
             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 */
-            if ((pte & PTE_W) &&
-                    (access_type == MMU_DATA_STORE || (pte & PTE_D))) {
+            /* add write permission on stores */
+            if ((pte & PTE_W) && (access_type == MMU_DATA_STORE)) {
                 *prot |= PAGE_WRITE;
             }
             return TRANSLATE_SUCCESS;
-- 
2.22.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH-4.2 v2 2/5] riscv: plic: Remove unused interrupt functions
  2019-07-30 23:35 [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2 Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 1/5] target/riscv: Don't set write permissions on dirty PTEs Alistair Francis
@ 2019-07-30 23:35 ` Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 3/5] target/riscv: Create function to test if FP is enabled Alistair Francis
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Alistair Francis @ 2019-07-30 23:35 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair23, palmer, alistair.francis

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Jonathan Behrens <fintelia@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Chih-Min Chao <chihmin.chao@sifive.com>
---
 hw/riscv/sifive_plic.c         | 12 ------------
 include/hw/riscv/sifive_plic.h |  3 ---
 2 files changed, 15 deletions(-)

diff --git a/hw/riscv/sifive_plic.c b/hw/riscv/sifive_plic.c
index 0950e89e15..864a1bed42 100644
--- a/hw/riscv/sifive_plic.c
+++ b/hw/riscv/sifive_plic.c
@@ -161,18 +161,6 @@ static void sifive_plic_update(SiFivePLICState *plic)
     }
 }
 
-void sifive_plic_raise_irq(SiFivePLICState *plic, uint32_t irq)
-{
-    sifive_plic_set_pending(plic, irq, true);
-    sifive_plic_update(plic);
-}
-
-void sifive_plic_lower_irq(SiFivePLICState *plic, uint32_t irq)
-{
-    sifive_plic_set_pending(plic, irq, false);
-    sifive_plic_update(plic);
-}
-
 static uint32_t sifive_plic_claim(SiFivePLICState *plic, uint32_t addrid)
 {
     int i, j;
diff --git a/include/hw/riscv/sifive_plic.h b/include/hw/riscv/sifive_plic.h
index ce8907f6aa..3b8a623919 100644
--- a/include/hw/riscv/sifive_plic.h
+++ b/include/hw/riscv/sifive_plic.h
@@ -69,9 +69,6 @@ typedef struct SiFivePLICState {
     uint32_t aperture_size;
 } SiFivePLICState;
 
-void sifive_plic_raise_irq(SiFivePLICState *plic, uint32_t irq);
-void sifive_plic_lower_irq(SiFivePLICState *plic, uint32_t irq);
-
 DeviceState *sifive_plic_create(hwaddr addr, char *hart_config,
     uint32_t num_sources, uint32_t num_priorities,
     uint32_t priority_base, uint32_t pending_base,
-- 
2.22.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH-4.2 v2 3/5] target/riscv: Create function to test if FP is enabled
  2019-07-30 23:35 [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2 Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 1/5] target/riscv: Don't set write permissions on dirty PTEs Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 2/5] riscv: plic: Remove unused interrupt functions Alistair Francis
@ 2019-07-30 23:35 ` Alistair Francis
  2019-08-05  6:59   ` Chih-Min Chao
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 4/5] target/riscv: Update the Hypervisor CSRs to v0.4 Alistair Francis
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names Alistair Francis
  4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2019-07-30 23:35 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair23, palmer, alistair.francis

Let's create a function that tests if floating point support is
enabled. We can then protect all floating point operations based on if
they are enabled.

This patch so far doesn't change anything, it's just preparing for the
Hypervisor support for floating point operations.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
---
 target/riscv/cpu.h        |  6 +++++-
 target/riscv/cpu_helper.c | 10 ++++++++++
 target/riscv/csr.c        | 20 +++++++++++---------
 3 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0adb307f32..2dc9b17678 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -255,6 +255,7 @@ void riscv_cpu_do_interrupt(CPUState *cpu);
 int riscv_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
 int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
 bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
+bool riscv_cpu_fp_enabled(CPURISCVState *env);
 int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
 hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
@@ -298,7 +299,10 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc,
 #ifdef CONFIG_USER_ONLY
     *flags = TB_FLAGS_MSTATUS_FS;
 #else
-    *flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
+    *flags = cpu_mmu_index(env, 0);
+    if (riscv_cpu_fp_enabled(env)) {
+        *flags |= env->mstatus & MSTATUS_FS;
+    }
 #endif
 }
 
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index f027be7f16..225e407cff 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -71,6 +71,16 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 
 #if !defined(CONFIG_USER_ONLY)
 
+/* Return true is floating point support is currently enabled */
+bool riscv_cpu_fp_enabled(CPURISCVState *env)
+{
+    if (env->mstatus & MSTATUS_FS) {
+        return true;
+    }
+
+    return false;
+}
+
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
 {
     CPURISCVState *env = &cpu->env;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index e0d4586760..2789215b5e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -46,7 +46,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations *ops)
 static int fs(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
 #endif
@@ -108,7 +108,7 @@ static int pmp(CPURISCVState *env, int csrno)
 static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
 #endif
@@ -119,7 +119,7 @@ static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
 static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
     env->mstatus |= MSTATUS_FS;
@@ -131,7 +131,7 @@ static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
 static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
 #endif
@@ -142,7 +142,7 @@ static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
 static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
     env->mstatus |= MSTATUS_FS;
@@ -154,7 +154,7 @@ static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
 static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
 #endif
@@ -166,7 +166,7 @@ static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
 static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
 {
 #if !defined(CONFIG_USER_ONLY)
-    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
+    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
         return -1;
     }
     env->mstatus |= MSTATUS_FS;
@@ -307,6 +307,7 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
 {
     target_ulong mstatus = env->mstatus;
     target_ulong mask = 0;
+    int dirty;
 
     /* flush tlb on mstatus fields that affect VM */
     if (env->priv_ver <= PRIV_VERSION_1_09_1) {
@@ -340,8 +341,9 @@ static int write_mstatus(CPURISCVState *env, int csrno, target_ulong val)
 
     mstatus = (mstatus & ~mask) | (val & mask);
 
-    int dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
-                ((mstatus & MSTATUS_XS) == MSTATUS_XS);
+    dirty = (riscv_cpu_fp_enabled(env) &&
+             ((mstatus & MSTATUS_FS) == MSTATUS_FS)) |
+            ((mstatus & MSTATUS_XS) == MSTATUS_XS);
     mstatus = set_field(mstatus, MSTATUS_SD, dirty);
     env->mstatus = mstatus;
 
-- 
2.22.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH-4.2 v2 4/5] target/riscv: Update the Hypervisor CSRs to v0.4
  2019-07-30 23:35 [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2 Alistair Francis
                   ` (2 preceding siblings ...)
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 3/5] target/riscv: Create function to test if FP is enabled Alistair Francis
@ 2019-07-30 23:35 ` Alistair Francis
  2019-08-05  6:19   ` Chih-Min Chao
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names Alistair Francis
  4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2019-07-30 23:35 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair23, palmer, alistair.francis

Update the Hypervisor CSR addresses to match the v0.4 spec.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu_bits.h | 35 ++++++++++++++++++-----------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 11f971ad5d..e99834856c 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -173,6 +173,24 @@
 #define CSR_SPTBR           0x180
 #define CSR_SATP            0x180
 
+/* Hpervisor CSRs */
+#define CSR_HSTATUS         0x600
+#define CSR_HEDELEG         0x602
+#define CSR_HIDELEG         0x603
+#define CSR_HCOUNTERNEN     0x606
+#define CSR_HGATP           0x680
+
+#if defined(TARGET_RISCV32)
+#define HGATP_MODE           SATP32_MODE
+#define HGATP_VMID           SATP32_ASID
+#define HGATP_PPN            SATP32_PPN
+#endif
+#if defined(TARGET_RISCV64)
+#define HGATP_MODE           SATP64_MODE
+#define HGATP_VMID           SATP64_ASID
+#define HGATP_PPN            SATP64_PPN
+#endif
+
 /* Physical Memory Protection */
 #define CSR_PMPCFG0         0x3a0
 #define CSR_PMPCFG1         0x3a1
@@ -206,23 +224,6 @@
 #define CSR_DPC             0x7b1
 #define CSR_DSCRATCH        0x7b2
 
-/* Hpervisor CSRs */
-#define CSR_HSTATUS         0xa00
-#define CSR_HEDELEG         0xa02
-#define CSR_HIDELEG         0xa03
-#define CSR_HGATP           0xa80
-
-#if defined(TARGET_RISCV32)
-#define HGATP_MODE           SATP32_MODE
-#define HGATP_ASID           SATP32_ASID
-#define HGATP_PPN            SATP32_PPN
-#endif
-#if defined(TARGET_RISCV64)
-#define HGATP_MODE           SATP64_MODE
-#define HGATP_ASID           SATP64_ASID
-#define HGATP_PPN            SATP64_PPN
-#endif
-
 /* Performance Counters */
 #define CSR_MHPMCOUNTER3    0xb03
 #define CSR_MHPMCOUNTER4    0xb04
-- 
2.22.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names
  2019-07-30 23:35 [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2 Alistair Francis
                   ` (3 preceding siblings ...)
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 4/5] target/riscv: Update the Hypervisor CSRs to v0.4 Alistair Francis
@ 2019-07-30 23:35 ` Alistair Francis
  2019-08-12 23:08   ` Palmer Dabbelt
  4 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2019-07-30 23:35 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair23, palmer, alistair.francis

From: Atish Patra <atish.patra@wdc.com>

As per the RISC-V spec, Floating Point registers are named as f0..f31
so lets fix the register names accordingly.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/cpu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index f8d07bd20a..af1e9b7690 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -40,10 +40,10 @@ const char * const riscv_int_regnames[] = {
 };
 
 const char * const riscv_fpr_regnames[] = {
-  "ft0", "ft1", "ft2",  "ft3",  "ft4", "ft5", "ft6",  "ft7",
-  "fs0", "fs1", "fa0",  "fa1",  "fa2", "fa3", "fa4",  "fa5",
-  "fa6", "fa7", "fs2",  "fs3",  "fs4", "fs5", "fs6",  "fs7",
-  "fs8", "fs9", "fs10", "fs11", "ft8", "ft9", "ft10", "ft11"
+  "f0", "f1", "f2",  "f3",  "f4", "f5", "f6", "f7",
+  "f8", "f9", "f10",  "f11",  "f12", "f13", "f14", "f15",
+  "f16", "f17", "f18",  "f19",  "f20", "f21", "f22", "f23",
+  "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31"
 };
 
 const char * const riscv_excp_names[] = {
-- 
2.22.0



^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH-4.2 v2 4/5] target/riscv: Update the Hypervisor CSRs to v0.4
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 4/5] target/riscv: Update the Hypervisor CSRs to v0.4 Alistair Francis
@ 2019-08-05  6:19   ` Chih-Min Chao
  0 siblings, 0 replies; 11+ messages in thread
From: Chih-Min Chao @ 2019-08-05  6:19 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Wed, Jul 31, 2019 at 7:40 AM Alistair Francis <alistair.francis@wdc.com>
wrote:

> Update the Hypervisor CSR addresses to match the v0.4 spec.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu_bits.h | 35 ++++++++++++++++++-----------------
>  1 file changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 11f971ad5d..e99834856c 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -173,6 +173,24 @@
>  #define CSR_SPTBR           0x180
>  #define CSR_SATP            0x180
>
> +/* Hpervisor CSRs */
> +#define CSR_HSTATUS         0x600
> +#define CSR_HEDELEG         0x602
> +#define CSR_HIDELEG         0x603
> +#define CSR_HCOUNTERNEN     0x606
> +#define CSR_HGATP           0x680
> +
> +#if defined(TARGET_RISCV32)
> +#define HGATP_MODE           SATP32_MODE
> +#define HGATP_VMID           SATP32_ASID
> +#define HGATP_PPN            SATP32_PPN
> +#endif
> +#if defined(TARGET_RISCV64)
> +#define HGATP_MODE           SATP64_MODE
> +#define HGATP_VMID           SATP64_ASID
> +#define HGATP_PPN            SATP64_PPN
> +#endif
> +
>  /* Physical Memory Protection */
>  #define CSR_PMPCFG0         0x3a0
>  #define CSR_PMPCFG1         0x3a1
> @@ -206,23 +224,6 @@
>  #define CSR_DPC             0x7b1
>  #define CSR_DSCRATCH        0x7b2
>
> -/* Hpervisor CSRs */
> -#define CSR_HSTATUS         0xa00
> -#define CSR_HEDELEG         0xa02
> -#define CSR_HIDELEG         0xa03
> -#define CSR_HGATP           0xa80
> -
> -#if defined(TARGET_RISCV32)
> -#define HGATP_MODE           SATP32_MODE
> -#define HGATP_ASID           SATP32_ASID
> -#define HGATP_PPN            SATP32_PPN
> -#endif
> -#if defined(TARGET_RISCV64)
> -#define HGATP_MODE           SATP64_MODE
> -#define HGATP_ASID           SATP64_ASID
> -#define HGATP_PPN            SATP64_PPN
> -#endif
> -
>  /* Performance Counters */
>  #define CSR_MHPMCOUNTER3    0xb03
>  #define CSR_MHPMCOUNTER4    0xb04
> --
> 2.22.0
>
> Reviewed-by: Chih-Min Chao <chihmin.chao@sifive.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH-4.2 v2 3/5] target/riscv: Create function to test if FP is enabled
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 3/5] target/riscv: Create function to test if FP is enabled Alistair Francis
@ 2019-08-05  6:59   ` Chih-Min Chao
  0 siblings, 0 replies; 11+ messages in thread
From: Chih-Min Chao @ 2019-08-05  6:59 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers

On Wed, Jul 31, 2019 at 7:39 AM Alistair Francis <alistair.francis@wdc.com>
wrote:

> Let's create a function that tests if floating point support is
> enabled. We can then protect all floating point operations based on if
> they are enabled.
>
> This patch so far doesn't change anything, it's just preparing for the
> Hypervisor support for floating point operations.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Reviewed-by: Christophe de Dinechin <dinechin@redhat.com>
> ---
>  target/riscv/cpu.h        |  6 +++++-
>  target/riscv/cpu_helper.c | 10 ++++++++++
>  target/riscv/csr.c        | 20 +++++++++++---------
>  3 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0adb307f32..2dc9b17678 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -255,6 +255,7 @@ void riscv_cpu_do_interrupt(CPUState *cpu);
>  int riscv_cpu_gdb_read_register(CPUState *cpu, uint8_t *buf, int reg);
>  int riscv_cpu_gdb_write_register(CPUState *cpu, uint8_t *buf, int reg);
>  bool riscv_cpu_exec_interrupt(CPUState *cs, int interrupt_request);
> +bool riscv_cpu_fp_enabled(CPURISCVState *env);
>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
>  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
>  void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
> @@ -298,7 +299,10 @@ static inline void cpu_get_tb_cpu_state(CPURISCVState
> *env, target_ulong *pc,
>  #ifdef CONFIG_USER_ONLY
>      *flags = TB_FLAGS_MSTATUS_FS;
>  #else
> -    *flags = cpu_mmu_index(env, 0) | (env->mstatus & MSTATUS_FS);
> +    *flags = cpu_mmu_index(env, 0);
> +    if (riscv_cpu_fp_enabled(env)) {
> +        *flags |= env->mstatus & MSTATUS_FS;
> +    }
>  #endif
>  }
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index f027be7f16..225e407cff 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -71,6 +71,16 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int
> interrupt_request)
>
>  #if !defined(CONFIG_USER_ONLY)
>
> +/* Return true is floating point support is currently enabled */
> +bool riscv_cpu_fp_enabled(CPURISCVState *env)
> +{
> +    if (env->mstatus & MSTATUS_FS) {
> +        return true;
> +    }
> +
> +    return false;
> +}
> +
>  int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts)
>  {
>      CPURISCVState *env = &cpu->env;
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index e0d4586760..2789215b5e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -46,7 +46,7 @@ void riscv_set_csr_ops(int csrno, riscv_csr_operations
> *ops)
>  static int fs(CPURISCVState *env, int csrno)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>  #endif
> @@ -108,7 +108,7 @@ static int pmp(CPURISCVState *env, int csrno)
>  static int read_fflags(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>  #endif
> @@ -119,7 +119,7 @@ static int read_fflags(CPURISCVState *env, int csrno,
> target_ulong *val)
>  static int write_fflags(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>      env->mstatus |= MSTATUS_FS;
> @@ -131,7 +131,7 @@ static int write_fflags(CPURISCVState *env, int csrno,
> target_ulong val)
>  static int read_frm(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>  #endif
> @@ -142,7 +142,7 @@ static int read_frm(CPURISCVState *env, int csrno,
> target_ulong *val)
>  static int write_frm(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>      env->mstatus |= MSTATUS_FS;
> @@ -154,7 +154,7 @@ static int write_frm(CPURISCVState *env, int csrno,
> target_ulong val)
>  static int read_fcsr(CPURISCVState *env, int csrno, target_ulong *val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>  #endif
> @@ -166,7 +166,7 @@ static int read_fcsr(CPURISCVState *env, int csrno,
> target_ulong *val)
>  static int write_fcsr(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    if (!env->debugger && !(env->mstatus & MSTATUS_FS)) {
> +    if (!env->debugger && !riscv_cpu_fp_enabled(env)) {
>          return -1;
>      }
>      env->mstatus |= MSTATUS_FS;
> @@ -307,6 +307,7 @@ static int write_mstatus(CPURISCVState *env, int
> csrno, target_ulong val)
>  {
>      target_ulong mstatus = env->mstatus;
>      target_ulong mask = 0;
> +    int dirty;
>
>      /* flush tlb on mstatus fields that affect VM */
>      if (env->priv_ver <= PRIV_VERSION_1_09_1) {
> @@ -340,8 +341,9 @@ static int write_mstatus(CPURISCVState *env, int
> csrno, target_ulong val)
>
>      mstatus = (mstatus & ~mask) | (val & mask);
>
> -    int dirty = ((mstatus & MSTATUS_FS) == MSTATUS_FS) |
> -                ((mstatus & MSTATUS_XS) == MSTATUS_XS);
> +    dirty = (riscv_cpu_fp_enabled(env) &&
> +             ((mstatus & MSTATUS_FS) == MSTATUS_FS)) |
> +            ((mstatus & MSTATUS_XS) == MSTATUS_XS);
>      mstatus = set_field(mstatus, MSTATUS_SD, dirty);
>      env->mstatus = mstatus;
>
> --
> 2.22.0
>
>
> Reviewed-by: Chih-Min Chao <chihmin.chao@sifive.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names
  2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names Alistair Francis
@ 2019-08-12 23:08   ` Palmer Dabbelt
  2019-08-13 17:06     ` Alistair Francis
  0 siblings, 1 reply; 11+ messages in thread
From: Palmer Dabbelt @ 2019-08-12 23:08 UTC (permalink / raw)
  To: Alistair Francis; +Cc: Alistair Francis, qemu-riscv, qemu-devel, alistair23

On Tue, 30 Jul 2019 16:35:34 PDT (-0700), Alistair Francis wrote:
> From: Atish Patra <atish.patra@wdc.com>
>
> As per the RISC-V spec, Floating Point registers are named as f0..f31
> so lets fix the register names accordingly.
>
> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/cpu.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index f8d07bd20a..af1e9b7690 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -40,10 +40,10 @@ const char * const riscv_int_regnames[] = {
>  };
>
>  const char * const riscv_fpr_regnames[] = {
> -  "ft0", "ft1", "ft2",  "ft3",  "ft4", "ft5", "ft6",  "ft7",
> -  "fs0", "fs1", "fa0",  "fa1",  "fa2", "fa3", "fa4",  "fa5",
> -  "fa6", "fa7", "fs2",  "fs3",  "fs4", "fs5", "fs6",  "fs7",
> -  "fs8", "fs9", "fs10", "fs11", "ft8", "ft9", "ft10", "ft11"
> +  "f0", "f1", "f2",  "f3",  "f4", "f5", "f6", "f7",
> +  "f8", "f9", "f10",  "f11",  "f12", "f13", "f14", "f15",
> +  "f16", "f17", "f18",  "f19",  "f20", "f21", "f22", "f23",
> +  "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31"
>  };
>
>  const char * const riscv_excp_names[] = {

I actually don't think this one is right: riscv_int_regnames uses the ABI 
names, so this should match.  I'd be OK switching both of them, but not just 
one.

I've queued the other four patches.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names
  2019-08-12 23:08   ` Palmer Dabbelt
@ 2019-08-13 17:06     ` Alistair Francis
  2019-08-14 18:07       ` Palmer Dabbelt
  0 siblings, 1 reply; 11+ messages in thread
From: Alistair Francis @ 2019-08-13 17:06 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: open list:RISC-V, Alistair Francis, qemu-devel@nongnu.org Developers

On Mon, Aug 12, 2019 at 4:08 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>
> On Tue, 30 Jul 2019 16:35:34 PDT (-0700), Alistair Francis wrote:
> > From: Atish Patra <atish.patra@wdc.com>
> >
> > As per the RISC-V spec, Floating Point registers are named as f0..f31
> > so lets fix the register names accordingly.
> >
> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/cpu.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index f8d07bd20a..af1e9b7690 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -40,10 +40,10 @@ const char * const riscv_int_regnames[] = {
> >  };
> >
> >  const char * const riscv_fpr_regnames[] = {
> > -  "ft0", "ft1", "ft2",  "ft3",  "ft4", "ft5", "ft6",  "ft7",
> > -  "fs0", "fs1", "fa0",  "fa1",  "fa2", "fa3", "fa4",  "fa5",
> > -  "fa6", "fa7", "fs2",  "fs3",  "fs4", "fs5", "fs6",  "fs7",
> > -  "fs8", "fs9", "fs10", "fs11", "ft8", "ft9", "ft10", "ft11"
> > +  "f0", "f1", "f2",  "f3",  "f4", "f5", "f6", "f7",
> > +  "f8", "f9", "f10",  "f11",  "f12", "f13", "f14", "f15",
> > +  "f16", "f17", "f18",  "f19",  "f20", "f21", "f22", "f23",
> > +  "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31"
> >  };
> >
> >  const char * const riscv_excp_names[] = {
>
> I actually don't think this one is right: riscv_int_regnames uses the ABI
> names, so this should match.  I'd be OK switching both of them, but not just
> one.

I like that the int registers use the ABI names though, as I find that useful.

What about we change the registers to use both? As in something like
x0/zero for all registers?

The disadvantage is that it's a little longer, but it seems the most useful.

Alistair

>
> I've queued the other four patches.


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names
  2019-08-13 17:06     ` Alistair Francis
@ 2019-08-14 18:07       ` Palmer Dabbelt
  0 siblings, 0 replies; 11+ messages in thread
From: Palmer Dabbelt @ 2019-08-14 18:07 UTC (permalink / raw)
  To: alistair23; +Cc: qemu-riscv, Alistair Francis, qemu-devel

On Tue, 13 Aug 2019 10:06:58 PDT (-0700), alistair23@gmail.com wrote:
> On Mon, Aug 12, 2019 at 4:08 PM Palmer Dabbelt <palmer@sifive.com> wrote:
>>
>> On Tue, 30 Jul 2019 16:35:34 PDT (-0700), Alistair Francis wrote:
>> > From: Atish Patra <atish.patra@wdc.com>
>> >
>> > As per the RISC-V spec, Floating Point registers are named as f0..f31
>> > so lets fix the register names accordingly.
>> >
>> > Signed-off-by: Atish Patra <atish.patra@wdc.com>
>> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> > ---
>> >  target/riscv/cpu.c | 8 ++++----
>> >  1 file changed, 4 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> > index f8d07bd20a..af1e9b7690 100644
>> > --- a/target/riscv/cpu.c
>> > +++ b/target/riscv/cpu.c
>> > @@ -40,10 +40,10 @@ const char * const riscv_int_regnames[] = {
>> >  };
>> >
>> >  const char * const riscv_fpr_regnames[] = {
>> > -  "ft0", "ft1", "ft2",  "ft3",  "ft4", "ft5", "ft6",  "ft7",
>> > -  "fs0", "fs1", "fa0",  "fa1",  "fa2", "fa3", "fa4",  "fa5",
>> > -  "fa6", "fa7", "fs2",  "fs3",  "fs4", "fs5", "fs6",  "fs7",
>> > -  "fs8", "fs9", "fs10", "fs11", "ft8", "ft9", "ft10", "ft11"
>> > +  "f0", "f1", "f2",  "f3",  "f4", "f5", "f6", "f7",
>> > +  "f8", "f9", "f10",  "f11",  "f12", "f13", "f14", "f15",
>> > +  "f16", "f17", "f18",  "f19",  "f20", "f21", "f22", "f23",
>> > +  "f24", "f25", "f26", "f27", "f28", "f29", "f30", "f31"
>> >  };
>> >
>> >  const char * const riscv_excp_names[] = {
>>
>> I actually don't think this one is right: riscv_int_regnames uses the ABI
>> names, so this should match.  I'd be OK switching both of them, but not just
>> one.
>
> I like that the int registers use the ABI names though, as I find that useful.
>
> What about we change the registers to use both? As in something like
> x0/zero for all registers?
>
> The disadvantage is that it's a little longer, but it seems the most useful.

I'm fine with that, as long as it doesn't show up in the GDB XML stuff as that 
will likely cause issues.

> Alistair
>
>>
>> I've queued the other four patches.


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2019-08-14 18:08 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30 23:35 [Qemu-devel] [PATCH-4.2 v2 0/5] RISC-V: Hypervisor prep work part 2 Alistair Francis
2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 1/5] target/riscv: Don't set write permissions on dirty PTEs Alistair Francis
2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 2/5] riscv: plic: Remove unused interrupt functions Alistair Francis
2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 3/5] target/riscv: Create function to test if FP is enabled Alistair Francis
2019-08-05  6:59   ` Chih-Min Chao
2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 4/5] target/riscv: Update the Hypervisor CSRs to v0.4 Alistair Francis
2019-08-05  6:19   ` Chih-Min Chao
2019-07-30 23:35 ` [Qemu-devel] [PATCH-4.2 v2 5/5] target/riscv: Fix Floating Point register names Alistair Francis
2019-08-12 23:08   ` Palmer Dabbelt
2019-08-13 17:06     ` Alistair Francis
2019-08-14 18:07       ` Palmer Dabbelt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).