| Message ID | 20230422130329.23555-2-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/riscv: Fix PMP related problem | expand |
On 2023/4/22 21:03, Weiwei Li wrote: > PMP entries before the matched PMP entry (including the matched PMP entry) > may only cover partial of the TLB page, which may make different regions in > that page allow different RWX privs, such as for PMP0 (0x80000008~0x8000000F, > R) and PMP1 (0x80001000~0x80001FFF, RWX) write access to 0x80000000 will > match PMP1. Typo here. Otherwise, Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com> Zhiwei > However we cannot cache the translation result in the TLB since > this will make the write access to 0x80000008 bypass the check of PMP0. So we > should check all of them instead of the matched PMP entry in pmp_get_tlb_size() > and set the tlb_size to 1 in this case. > Set tlb_size to TARGET_PAGE_SIZE if PMP is not support or there is no PMP rules. > > Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn> > Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn> > --- > target/riscv/cpu_helper.c | 7 ++--- > target/riscv/pmp.c | 64 ++++++++++++++++++++++++++++++--------- > target/riscv/pmp.h | 3 +- > 3 files changed, 52 insertions(+), 22 deletions(-) > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 433ea529b0..075fc0538a 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -703,11 +703,8 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot, > } > > *prot = pmp_priv_to_page_prot(pmp_priv); > - if ((tlb_size != NULL) && pmp_index != MAX_RISCV_PMPS) { > - target_ulong tlb_sa = addr & ~(TARGET_PAGE_SIZE - 1); > - target_ulong tlb_ea = tlb_sa + TARGET_PAGE_SIZE - 1; > - > - *tlb_size = pmp_get_tlb_size(env, pmp_index, tlb_sa, tlb_ea); > + if (tlb_size != NULL) { > + *tlb_size = pmp_get_tlb_size(env, addr); > } > > return TRANSLATE_SUCCESS; > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 1f5aca42e8..ad20a319c1 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -601,28 +601,62 @@ target_ulong mseccfg_csr_read(CPURISCVState *env) > } > > /* > - * Calculate the TLB size if the start address or the end address of > - * PMP entry is presented in the TLB page. > + * Calculate the TLB size. If the PMP rules may make different regions in > + * the TLB page of 'addr' allow different RWX privs, set the size to 1 > + * (to make the translation result uncached in the TLB and only be used for > + * a single translation). Set the size to TARGET_PAGE_SIZE otherwise. > */ > -target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index, > - target_ulong tlb_sa, target_ulong tlb_ea) > +target_ulong pmp_get_tlb_size(CPURISCVState *env, target_ulong addr) > { > - target_ulong pmp_sa = env->pmp_state.addr[pmp_index].sa; > - target_ulong pmp_ea = env->pmp_state.addr[pmp_index].ea; > + target_ulong pmp_sa; > + target_ulong pmp_ea; > + target_ulong tlb_sa = addr & ~(TARGET_PAGE_SIZE - 1); > + target_ulong tlb_ea = tlb_sa + TARGET_PAGE_SIZE - 1; > + int i; > > - if (pmp_sa <= tlb_sa && pmp_ea >= tlb_ea) { > + /* > + * If PMP is not supported or there is no PMP rule, which means the allowed > + * RWX privs of the page will not affected by PMP or PMP will provide the > + * same option (disallow accesses or allow default RWX privs) for all > + * addresses, set the size to TARGET_PAGE_SIZE. > + */ > + if (!riscv_cpu_cfg(env)->pmp || !pmp_get_num_rules(env)) { > return TARGET_PAGE_SIZE; > - } else { > + } > + > + for (i = 0; i < MAX_RISCV_PMPS; i++) { > + if (pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg) == PMP_AMATCH_OFF) { > + continue; > + } > + > + pmp_sa = env->pmp_state.addr[i].sa; > + pmp_ea = env->pmp_state.addr[i].ea; > + > /* > - * At this point we have a tlb_size that is the smallest possible size > - * That fits within a TARGET_PAGE_SIZE and the PMP region. > - * > - * If the size is less then TARGET_PAGE_SIZE we drop the size to 1. > - * This means the result isn't cached in the TLB and is only used for > - * a single translation. > + * Only the first PMP entry that covers (whole or partial of) the TLB > + * page really matters: > + * If it can cover the whole page, set the size to TARGET_PAGE_SIZE. > + * The following PMP entries have lower priority and will not affect > + * the allowed RWX privs of the page. > + * If it only cover partial of the TLB page, set the size to 1 since > + * the allowed RWX privs for the covered region may be different from > + * other region of the page. > */ > - return 1; > + if (pmp_sa <= tlb_sa && pmp_ea >= tlb_ea) { > + return TARGET_PAGE_SIZE; > + } else if ((pmp_sa >= tlb_sa && pmp_sa <= tlb_ea) || > + (pmp_ea >= tlb_sa && pmp_ea <= tlb_ea)) { > + return 1; > + } > } > + > + /* > + * If no PMP entry covers any region of the TLB page, similar to the above > + * case that there is no PMP rule, PMP will provide the same option > + * (disallow accesses or allow default RWX privs) for the whole page, > + * set the size to TARGET_PAGE_SIZE. > + */ > + return TARGET_PAGE_SIZE; > } > > /* > diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h > index b296ea1fc6..0a7e24750b 100644 > --- a/target/riscv/pmp.h > +++ b/target/riscv/pmp.h > @@ -76,8 +76,7 @@ int pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, > target_ulong size, pmp_priv_t privs, > pmp_priv_t *allowed_privs, > target_ulong mode); > -target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index, > - target_ulong tlb_sa, target_ulong tlb_ea); > +target_ulong pmp_get_tlb_size(CPURISCVState *env, target_ulong addr); > void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index); > void pmp_update_rule_nums(CPURISCVState *env); > uint32_t pmp_get_num_rules(CPURISCVState *env);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 433ea529b0..075fc0538a 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -703,11 +703,8 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot, } *prot = pmp_priv_to_page_prot(pmp_priv); - if ((tlb_size != NULL) && pmp_index != MAX_RISCV_PMPS) { - target_ulong tlb_sa = addr & ~(TARGET_PAGE_SIZE - 1); - target_ulong tlb_ea = tlb_sa + TARGET_PAGE_SIZE - 1; - - *tlb_size = pmp_get_tlb_size(env, pmp_index, tlb_sa, tlb_ea); + if (tlb_size != NULL) { + *tlb_size = pmp_get_tlb_size(env, addr); } return TRANSLATE_SUCCESS; diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c index 1f5aca42e8..ad20a319c1 100644 --- a/target/riscv/pmp.c +++ b/target/riscv/pmp.c @@ -601,28 +601,62 @@ target_ulong mseccfg_csr_read(CPURISCVState *env) } /* - * Calculate the TLB size if the start address or the end address of - * PMP entry is presented in the TLB page. + * Calculate the TLB size. If the PMP rules may make different regions in + * the TLB page of 'addr' allow different RWX privs, set the size to 1 + * (to make the translation result uncached in the TLB and only be used for + * a single translation). Set the size to TARGET_PAGE_SIZE otherwise. */ -target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index, - target_ulong tlb_sa, target_ulong tlb_ea) +target_ulong pmp_get_tlb_size(CPURISCVState *env, target_ulong addr) { - target_ulong pmp_sa = env->pmp_state.addr[pmp_index].sa; - target_ulong pmp_ea = env->pmp_state.addr[pmp_index].ea; + target_ulong pmp_sa; + target_ulong pmp_ea; + target_ulong tlb_sa = addr & ~(TARGET_PAGE_SIZE - 1); + target_ulong tlb_ea = tlb_sa + TARGET_PAGE_SIZE - 1; + int i; - if (pmp_sa <= tlb_sa && pmp_ea >= tlb_ea) { + /* + * If PMP is not supported or there is no PMP rule, which means the allowed + * RWX privs of the page will not affected by PMP or PMP will provide the + * same option (disallow accesses or allow default RWX privs) for all + * addresses, set the size to TARGET_PAGE_SIZE. + */ + if (!riscv_cpu_cfg(env)->pmp || !pmp_get_num_rules(env)) { return TARGET_PAGE_SIZE; - } else { + } + + for (i = 0; i < MAX_RISCV_PMPS; i++) { + if (pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg) == PMP_AMATCH_OFF) { + continue; + } + + pmp_sa = env->pmp_state.addr[i].sa; + pmp_ea = env->pmp_state.addr[i].ea; + /* - * At this point we have a tlb_size that is the smallest possible size - * That fits within a TARGET_PAGE_SIZE and the PMP region. - * - * If the size is less then TARGET_PAGE_SIZE we drop the size to 1. - * This means the result isn't cached in the TLB and is only used for - * a single translation. + * Only the first PMP entry that covers (whole or partial of) the TLB + * page really matters: + * If it can cover the whole page, set the size to TARGET_PAGE_SIZE. + * The following PMP entries have lower priority and will not affect + * the allowed RWX privs of the page. + * If it only cover partial of the TLB page, set the size to 1 since + * the allowed RWX privs for the covered region may be different from + * other region of the page. */ - return 1; + if (pmp_sa <= tlb_sa && pmp_ea >= tlb_ea) { + return TARGET_PAGE_SIZE; + } else if ((pmp_sa >= tlb_sa && pmp_sa <= tlb_ea) || + (pmp_ea >= tlb_sa && pmp_ea <= tlb_ea)) { + return 1; + } } + + /* + * If no PMP entry covers any region of the TLB page, similar to the above + * case that there is no PMP rule, PMP will provide the same option + * (disallow accesses or allow default RWX privs) for the whole page, + * set the size to TARGET_PAGE_SIZE. + */ + return TARGET_PAGE_SIZE; } /* diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h index b296ea1fc6..0a7e24750b 100644 --- a/target/riscv/pmp.h +++ b/target/riscv/pmp.h @@ -76,8 +76,7 @@ int pmp_hart_has_privs(CPURISCVState *env, target_ulong addr, target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs, target_ulong mode); -target_ulong pmp_get_tlb_size(CPURISCVState *env, int pmp_index, - target_ulong tlb_sa, target_ulong tlb_ea); +target_ulong pmp_get_tlb_size(CPURISCVState *env, target_ulong addr); void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index); void pmp_update_rule_nums(CPURISCVState *env); uint32_t pmp_get_num_rules(CPURISCVState *env);