[PATCH v2 10/23] target/riscv: Use MMU_INDEX() helper

Helge Deller posted 23 patches 2 years, 6 months ago
Maintainers: Richard Henderson <richard.henderson@linaro.org>, Paolo Bonzini <pbonzini@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, Michael Rolnik <mrolnik@gmail.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Brian Cain <bcain@quicinc.com>, Song Gao <gaosong@loongson.cn>, Xiaojuan Yang <yangxiaojuan@loongson.cn>, Laurent Vivier <laurent@vivier.eu>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, Chris Wulff <crwulff@gmail.com>, Marek Vasut <marex@denx.de>, Stafford Horne <shorne@gmail.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>, David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Nicholas Piggin <npiggin@gmail.com>, Palmer Dabbelt <palmer@dabbelt.com>, Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Weiwei Li <liweiwei@iscas.ac.cn>, Liu Zhiwei <zhiwei_liu@linux.alibaba.com>, Yoshinori Sato <ysato@users.sourceforge.jp>, David Hildenbrand <david@redhat.com>, Ilya Leoshkevich <iii@linux.ibm.com>, Thomas Huth <thuth@redhat.com>, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>, Artyom Tarasenko <atar4qemu@gmail.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Max Filippov <jcmvbkbc@gmail.com>
[PATCH v2 10/23] target/riscv: Use MMU_INDEX() helper
Posted by Helge Deller 2 years, 6 months ago
Use the new MMU_INDEX() helper to specify the index of the CPUTLB which
should be used.  Additionally, in a follow-up patch this helper allows
then to optimize the tcg code generation.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 target/riscv/cpu.h        | 4 ++--
 target/riscv/cpu_helper.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 6ea22e0eea..6aba1df64a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -88,7 +88,7 @@ typedef enum {
     EXT_STATUS_DIRTY,
 } RISCVExtStatus;

-#define MMU_USER_IDX 3
+#define MMU_USER_IDX MMU_INDEX(3)

 #define MAX_RISCV_PMPS (16)

@@ -446,7 +446,7 @@ void riscv_cpu_list(void);
 void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);

 #define cpu_list riscv_cpu_list
-#define cpu_mmu_index riscv_cpu_mmu_index
+#define cpu_mmu_index(e,i)      MMU_INDEX(riscv_cpu_mmu_index(e,i))

 #ifndef CONFIG_USER_ONLY
 void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9f611d89bb..a8e6950217 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -107,7 +107,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
 #else
     flags = FIELD_DP32(flags, TB_FLAGS, PRIV, env->priv);

-    flags |= cpu_mmu_index(env, 0);
+    flags |= riscv_cpu_mmu_index(env, 0);
     fs = get_field(env->mstatus, MSTATUS_FS);
     vs = get_field(env->mstatus, MSTATUS_VS);

--
2.41.0
Re: [PATCH v2 10/23] target/riscv: Use MMU_INDEX() helper
Posted by Richard Henderson 2 years, 6 months ago
On 8/6/23 05:17, Helge Deller wrote:
> Use the new MMU_INDEX() helper to specify the index of the CPUTLB which
> should be used.  Additionally, in a follow-up patch this helper allows
> then to optimize the tcg code generation.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   target/riscv/cpu.h        | 4 ++--
>   target/riscv/cpu_helper.c | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 6ea22e0eea..6aba1df64a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -88,7 +88,7 @@ typedef enum {
>       EXT_STATUS_DIRTY,
>   } RISCVExtStatus;
> 
> -#define MMU_USER_IDX 3
> +#define MMU_USER_IDX MMU_INDEX(3)
> 
>   #define MAX_RISCV_PMPS (16)
> 
> @@ -446,7 +446,7 @@ void riscv_cpu_list(void);
>   void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);
> 
>   #define cpu_list riscv_cpu_list
> -#define cpu_mmu_index riscv_cpu_mmu_index
> +#define cpu_mmu_index(e,i)      MMU_INDEX(riscv_cpu_mmu_index(e,i))
> 
>   #ifndef CONFIG_USER_ONLY
>   void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9f611d89bb..a8e6950217 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -107,7 +107,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>   #else
>       flags = FIELD_DP32(flags, TB_FLAGS, PRIV, env->priv);
> 
> -    flags |= cpu_mmu_index(env, 0);
> +    flags |= riscv_cpu_mmu_index(env, 0);
>       fs = get_field(env->mstatus, MSTATUS_FS);
>       vs = get_field(env->mstatus, MSTATUS_VS);

This is the sort of non-obvious changes that I hoped to avoid by restricting all changes 
to accel/tcg/cputlb.c.


r~
Re: [PATCH v2 10/23] target/riscv: Use MMU_INDEX() helper
Posted by Helge Deller 2 years, 6 months ago
On 8/6/23 16:30, Richard Henderson wrote:
> On 8/6/23 05:17, Helge Deller wrote:
>> Use the new MMU_INDEX() helper to specify the index of the CPUTLB which
>> should be used.  Additionally, in a follow-up patch this helper allows
>> then to optimize the tcg code generation.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>   target/riscv/cpu.h        | 4 ++--
>>   target/riscv/cpu_helper.c | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 6ea22e0eea..6aba1df64a 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -88,7 +88,7 @@ typedef enum {
>>       EXT_STATUS_DIRTY,
>>   } RISCVExtStatus;
>>
>> -#define MMU_USER_IDX 3
>> +#define MMU_USER_IDX MMU_INDEX(3)
>>
>>   #define MAX_RISCV_PMPS (16)
>>
>> @@ -446,7 +446,7 @@ void riscv_cpu_list(void);
>>   void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp);
>>
>>   #define cpu_list riscv_cpu_list
>> -#define cpu_mmu_index riscv_cpu_mmu_index
>> +#define cpu_mmu_index(e,i)      MMU_INDEX(riscv_cpu_mmu_index(e,i))
>>
>>   #ifndef CONFIG_USER_ONLY
>>   void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 9f611d89bb..a8e6950217 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -107,7 +107,7 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
>>   #else
>>       flags = FIELD_DP32(flags, TB_FLAGS, PRIV, env->priv);
>>
>> -    flags |= cpu_mmu_index(env, 0);
>> +    flags |= riscv_cpu_mmu_index(env, 0);
>>       fs = get_field(env->mstatus, MSTATUS_FS);
>>       vs = get_field(env->mstatus, MSTATUS_VS);
>
> This is the sort of non-obvious changes that I hoped to avoid by restricting all changes to accel/tcg/cputlb.c.

True.
And, since I've found some other missing pieces now too (e.g. in hppa)
I'm currently tempted to fully agree with you, that handling this
in accel/tcg/cputlb.c only is the better (and cleaner) solution.

I'll try you approach.

Helge