* [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* 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
* [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* 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
* [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 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