On 3/22/2023 8:37 PM, liweiwei wrote:
>
> On 2023/3/22 20:12, Fei Wu wrote:
>> 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.
>>
>> Signed-off-by: Fei Wu <fei2...@intel.com>
>> ---
>> target/riscv/cpu-param.h | 2 +-
>> target/riscv/cpu.h | 2 +-
>> target/riscv/cpu_bits.h | 1 +
>> target/riscv/cpu_helper.c | 11 +++++++++++
>> target/riscv/csr.c | 2 +-
>> 5 files changed, 15 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/cpu-param.h b/target/riscv/cpu-param.h
>> index ebaf26d26d..9e21b943f9 100644
>> --- a/target/riscv/cpu-param.h
>> +++ b/target/riscv/cpu-param.h
>> @@ -27,6 +27,6 @@
>> * - S mode HLV/HLVX/HSV 0b101
>> * - M mode HLV/HLVX/HSV 0b111
>> */
>> -#define NB_MMU_MODES 8
>> +#define NB_MMU_MODES 16
>> #endif
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 638e47c75a..ac8bee11a7 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -624,7 +624,7 @@ target_ulong riscv_cpu_get_fflags(CPURISCVState
>> *env);
>> void riscv_cpu_set_fflags(CPURISCVState *env, target_ulong);
>> #define TB_FLAGS_PRIV_MMU_MASK 3
>> -#define TB_FLAGS_PRIV_HYP_ACCESS_MASK (1 << 2)
>> +#define TB_FLAGS_PRIV_HYP_ACCESS_MASK (1 << 3)
>> #define TB_FLAGS_MSTATUS_FS MSTATUS_FS
>> #define TB_FLAGS_MSTATUS_VS MSTATUS_VS
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index fca7ef0cef..dd9e62b6e4 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -606,6 +606,7 @@ typedef enum {
>> #define PRV_S 1
>> #define PRV_H 2 /* Reserved */
>> #define PRV_M 3
>> +#define MMUIdx_S_SUM 5
>> /* Virtulisation Register Fields */
>> #define VIRT_ONOFF 1
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index f88c503cf4..e52e9765d0 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -36,6 +36,17 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool
>> ifetch)
>> #ifdef CONFIG_USER_ONLY
>> return 0;
>> #else
>> + if (ifetch) {
>> + return env->priv;
>> + }
>> +
>> + 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 env->priv;
>
> If we return mode here, whether tlb needn't flush for changes to
> MSTATUS_MPRV and MSTATUS_MPP?
>
Good point. Besides performance improvement, it sounds more reasonable
to return the effective privilege mode instead of the plain priv, why it
should use the M-mode tlb index when it behaves as S-mode.
Thanks,
Fei.
> Regards,
>
> Weiwei Li
>
>> #endif
>> }
>> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
>> index ab566639e5..eacc40e912 100644
>> --- a/target/riscv/csr.c
>> +++ b/target/riscv/csr.c
>> @@ -1246,7 +1246,7 @@ static RISCVException
>> write_mstatus(CPURISCVState *env, int csrno,
>> /* flush tlb on mstatus fields that affect VM */
>> if ((val ^ mstatus) & (MSTATUS_MXR | MSTATUS_MPP | MSTATUS_MPV |
>> - MSTATUS_MPRV | MSTATUS_SUM)) {
>> + MSTATUS_MPRV)) {
>> tlb_flush(env_cpu(env));
>> }
>> mask = MSTATUS_SIE | MSTATUS_SPIE | MSTATUS_MIE | MSTATUS_MPIE |
>
