Currently, PMP permission checking of TLB page is bypassed if TLB hits
Fix it by propagating PMP permission to TLB page permission.
PMP permission checking also use MMU-style API to change TLB permission
and size.
Signed-off-by: Jim Shu <cwshu@andestech.com>
---
target/riscv/cpu_helper.c | 84 +++++++++++++++++++++++++++++----------
target/riscv/pmp.c | 80 +++++++++++++++++++++++++++----------
target/riscv/pmp.h | 4 +-
3 files changed, 125 insertions(+), 43 deletions(-)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2f43939fb6..f6ac63bf0e 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -280,6 +280,49 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
env->load_res = -1;
}
+/*
+ * get_physical_address_pmp - check PMP permission for this physical address
+ *
+ * Match the PMP region and check permission for this physical address and it's
+ * TLB page. Returns 0 if the permission checking was successful
+ *
+ * @env: CPURISCVState
+ * @prot: The returned protection attributes
+ * @tlb_size: TLB page size containing addr. It could be modified after PMP
+ * permission checking. NULL if not set TLB page for addr.
+ * @addr: The physical address to be checked permission
+ * @access_type: The type of MMU access
+ * @mode: Indicates current privilege level.
+ */
+static int get_physical_address_pmp(CPURISCVState *env, int *prot,
+ target_ulong *tlb_size, hwaddr addr,
+ int size, MMUAccessType access_type,
+ int mode)
+{
+ pmp_priv_t pmp_priv;
+ target_ulong tlb_size_pmp = 0;
+
+ if (!riscv_feature(env, RISCV_FEATURE_PMP)) {
+ *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+ return TRANSLATE_SUCCESS;
+ }
+
+ if (!pmp_hart_has_privs(env, addr, size, 1 << access_type, &pmp_priv,
+ mode)) {
+ *prot = 0;
+ return TRANSLATE_PMP_FAIL;
+ }
+
+ *prot = pmp_priv_to_page_prot(pmp_priv);
+ if (tlb_size != NULL) {
+ if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
+ *tlb_size = tlb_size_pmp;
+ }
+ }
+
+ return TRANSLATE_SUCCESS;
+}
+
/* get_physical_address - get the physical address for this virtual address
*
* Do a page table walk to obtain the physical address corresponding to a
@@ -442,9 +485,11 @@ restart:
pte_addr = base + idx * ptesize;
}
- if (riscv_feature(env, RISCV_FEATURE_PMP) &&
- !pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong),
- 1 << MMU_DATA_LOAD, PRV_S)) {
+ int pmp_prot;
+ int pmp_ret = get_physical_address_pmp(env, &pmp_prot, NULL, pte_addr,
+ sizeof(target_ulong),
+ MMU_DATA_LOAD, PRV_S);
+ if (pmp_ret != TRANSLATE_SUCCESS) {
return TRANSLATE_PMP_FAIL;
}
@@ -682,13 +727,14 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
#ifndef CONFIG_USER_ONLY
vaddr im_address;
hwaddr pa = 0;
- int prot, prot2;
+ int prot, prot2, prot_pmp;
bool pmp_violation = false;
bool first_stage_error = true;
bool two_stage_lookup = false;
int ret = TRANSLATE_FAIL;
int mode = mmu_idx;
- target_ulong tlb_size = 0;
+ /* default TLB page size */
+ target_ulong tlb_size = TARGET_PAGE_SIZE;
env->guest_phys_fault_addr = 0;
@@ -745,10 +791,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
prot &= prot2;
- if (riscv_feature(env, RISCV_FEATURE_PMP) &&
- (ret == TRANSLATE_SUCCESS) &&
- !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
- ret = TRANSLATE_PMP_FAIL;
+ if (ret == TRANSLATE_SUCCESS) {
+ ret = get_physical_address_pmp(env, &prot_pmp, &tlb_size, pa,
+ size, access_type, mode);
+ prot &= prot_pmp;
}
if (ret != TRANSLATE_SUCCESS) {
@@ -771,25 +817,21 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
"%s address=%" VADDR_PRIx " ret %d physical "
TARGET_FMT_plx " prot %d\n",
__func__, address, ret, pa, prot);
- }
- if (riscv_feature(env, RISCV_FEATURE_PMP) &&
- (ret == TRANSLATE_SUCCESS) &&
- !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
- ret = TRANSLATE_PMP_FAIL;
+ if (ret == TRANSLATE_SUCCESS) {
+ ret = get_physical_address_pmp(env, &prot_pmp, &tlb_size, pa,
+ size, access_type, mode);
+ prot &= prot_pmp;
+ }
}
+
if (ret == TRANSLATE_PMP_FAIL) {
pmp_violation = true;
}
if (ret == TRANSLATE_SUCCESS) {
- if (pmp_is_range_in_tlb(env, pa & TARGET_PAGE_MASK, &tlb_size)) {
- tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
- prot, mmu_idx, tlb_size);
- } else {
- tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK,
- prot, mmu_idx, TARGET_PAGE_SIZE);
- }
+ tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
+ prot, mmu_idx, tlb_size);
return true;
} else if (probe) {
return false;
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 80d0334e1b..ebd874cde3 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -217,6 +217,35 @@ static int pmp_is_in_range(CPURISCVState *env, int pmp_index, target_ulong addr)
return result;
}
+/*
+ * Check if the address has required RWX privs when no PMP entry is matched.
+ */
+static bool pmp_hart_has_privs_default(CPURISCVState *env, target_ulong addr,
+ target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
+ target_ulong mode)
+{
+ bool ret;
+
+ if ((!riscv_feature(env, RISCV_FEATURE_PMP)) || (mode == PRV_M)) {
+ /*
+ * Privileged spec v1.10 states if HW doesn't implement any PMP entry
+ * or no PMP entry matches an M-Mode access, the access succeeds.
+ */
+ ret = true;
+ *allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
+ } else {
+ /*
+ * Other modes are not allowed to succeed if they don't * match a rule,
+ * but there are rules. We've checked for no rule earlier in this
+ * function.
+ */
+ ret = false;
+ *allowed_privs = 0;
+ }
+
+ return ret;
+}
+
/*
* Public Interface
@@ -226,18 +255,19 @@ static int pmp_is_in_range(CPURISCVState *env, int pmp_index, target_ulong addr)
* Check if the address has required RWX privs to complete desired operation
*/
bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
- target_ulong size, pmp_priv_t privs, target_ulong mode)
+ target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
+ target_ulong mode)
{
int i = 0;
int ret = -1;
int pmp_size = 0;
target_ulong s = 0;
target_ulong e = 0;
- pmp_priv_t allowed_privs = 0;
/* Short cut if no rules */
if (0 == pmp_get_num_rules(env)) {
- return (env->priv == PRV_M) ? true : false;
+ return pmp_hart_has_privs_default(env, addr, size, privs,
+ allowed_privs, mode);
}
if (size == 0) {
@@ -277,37 +307,25 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
* check
*/
if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
- allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
+ *allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
if ((mode != PRV_M) || pmp_is_locked(env, i)) {
- allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
+ *allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
}
- if ((privs & allowed_privs) == privs) {
- ret = 1;
- break;
- } else {
- ret = 0;
- break;
- }
+ ret = ((privs & *allowed_privs) == privs);
+ break;
}
}
/* No rule matched */
if (ret == -1) {
- if (mode == PRV_M) {
- ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
- * M-Mode access, the access succeeds */
- } else {
- ret = 0; /* Other modes are not allowed to succeed if they don't
- * match a rule, but there are rules. We've checked for
- * no rule earlier in this function. */
- }
+ return pmp_hart_has_privs_default(env, addr, size, privs,
+ allowed_privs, mode);
}
return ret == 1 ? true : false;
}
-
/*
* Handle a write to a pmpcfg CSP
*/
@@ -442,3 +460,23 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
return false;
}
+
+/*
+ * Convert PMP privilege to TLB page privilege.
+ */
+int pmp_priv_to_page_prot(pmp_priv_t pmp_priv)
+{
+ int prot = 0;
+
+ if (pmp_priv & PMP_READ) {
+ prot |= PAGE_READ;
+ }
+ if (pmp_priv & PMP_WRITE) {
+ prot |= PAGE_WRITE;
+ }
+ if (pmp_priv & PMP_EXEC) {
+ prot |= PAGE_EXEC;
+ }
+
+ return prot;
+}
diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
index c8d5ef4a69..b82a30f0d5 100644
--- a/target/riscv/pmp.h
+++ b/target/riscv/pmp.h
@@ -59,11 +59,13 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
target_ulong val);
target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
- target_ulong size, pmp_priv_t priv, target_ulong mode);
+ target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
+ target_ulong mode);
bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
target_ulong *tlb_size);
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);
+int pmp_priv_to_page_prot(pmp_priv_t pmp_priv);
#endif
--
2.30.1
On Sun, Feb 21, 2021 at 10:35 AM Jim Shu <cwshu@andestech.com> wrote:
>
> Currently, PMP permission checking of TLB page is bypassed if TLB hits
> Fix it by propagating PMP permission to TLB page permission.
>
> PMP permission checking also use MMU-style API to change TLB permission
> and size.
>
> Signed-off-by: Jim Shu <cwshu@andestech.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/cpu_helper.c | 84 +++++++++++++++++++++++++++++----------
> target/riscv/pmp.c | 80 +++++++++++++++++++++++++++----------
> target/riscv/pmp.h | 4 +-
> 3 files changed, 125 insertions(+), 43 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2f43939fb6..f6ac63bf0e 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -280,6 +280,49 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv)
> env->load_res = -1;
> }
>
> +/*
> + * get_physical_address_pmp - check PMP permission for this physical address
> + *
> + * Match the PMP region and check permission for this physical address and it's
> + * TLB page. Returns 0 if the permission checking was successful
> + *
> + * @env: CPURISCVState
> + * @prot: The returned protection attributes
> + * @tlb_size: TLB page size containing addr. It could be modified after PMP
> + * permission checking. NULL if not set TLB page for addr.
> + * @addr: The physical address to be checked permission
> + * @access_type: The type of MMU access
> + * @mode: Indicates current privilege level.
> + */
> +static int get_physical_address_pmp(CPURISCVState *env, int *prot,
> + target_ulong *tlb_size, hwaddr addr,
> + int size, MMUAccessType access_type,
> + int mode)
> +{
> + pmp_priv_t pmp_priv;
> + target_ulong tlb_size_pmp = 0;
> +
> + if (!riscv_feature(env, RISCV_FEATURE_PMP)) {
> + *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> + return TRANSLATE_SUCCESS;
> + }
> +
> + if (!pmp_hart_has_privs(env, addr, size, 1 << access_type, &pmp_priv,
> + mode)) {
> + *prot = 0;
> + return TRANSLATE_PMP_FAIL;
> + }
> +
> + *prot = pmp_priv_to_page_prot(pmp_priv);
> + if (tlb_size != NULL) {
> + if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
> + *tlb_size = tlb_size_pmp;
> + }
> + }
> +
> + return TRANSLATE_SUCCESS;
> +}
> +
> /* get_physical_address - get the physical address for this virtual address
> *
> * Do a page table walk to obtain the physical address corresponding to a
> @@ -442,9 +485,11 @@ restart:
> pte_addr = base + idx * ptesize;
> }
>
> - if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> - !pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong),
> - 1 << MMU_DATA_LOAD, PRV_S)) {
> + int pmp_prot;
> + int pmp_ret = get_physical_address_pmp(env, &pmp_prot, NULL, pte_addr,
> + sizeof(target_ulong),
> + MMU_DATA_LOAD, PRV_S);
> + if (pmp_ret != TRANSLATE_SUCCESS) {
> return TRANSLATE_PMP_FAIL;
> }
>
> @@ -682,13 +727,14 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> #ifndef CONFIG_USER_ONLY
> vaddr im_address;
> hwaddr pa = 0;
> - int prot, prot2;
> + int prot, prot2, prot_pmp;
> bool pmp_violation = false;
> bool first_stage_error = true;
> bool two_stage_lookup = false;
> int ret = TRANSLATE_FAIL;
> int mode = mmu_idx;
> - target_ulong tlb_size = 0;
> + /* default TLB page size */
> + target_ulong tlb_size = TARGET_PAGE_SIZE;
>
> env->guest_phys_fault_addr = 0;
>
> @@ -745,10 +791,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>
> prot &= prot2;
>
> - if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> - (ret == TRANSLATE_SUCCESS) &&
> - !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
> - ret = TRANSLATE_PMP_FAIL;
> + if (ret == TRANSLATE_SUCCESS) {
> + ret = get_physical_address_pmp(env, &prot_pmp, &tlb_size, pa,
> + size, access_type, mode);
> + prot &= prot_pmp;
> }
>
> if (ret != TRANSLATE_SUCCESS) {
> @@ -771,25 +817,21 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> "%s address=%" VADDR_PRIx " ret %d physical "
> TARGET_FMT_plx " prot %d\n",
> __func__, address, ret, pa, prot);
> - }
>
> - if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> - (ret == TRANSLATE_SUCCESS) &&
> - !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
> - ret = TRANSLATE_PMP_FAIL;
> + if (ret == TRANSLATE_SUCCESS) {
> + ret = get_physical_address_pmp(env, &prot_pmp, &tlb_size, pa,
> + size, access_type, mode);
> + prot &= prot_pmp;
> + }
> }
> +
> if (ret == TRANSLATE_PMP_FAIL) {
> pmp_violation = true;
> }
>
> if (ret == TRANSLATE_SUCCESS) {
> - if (pmp_is_range_in_tlb(env, pa & TARGET_PAGE_MASK, &tlb_size)) {
> - tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
> - prot, mmu_idx, tlb_size);
> - } else {
> - tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK,
> - prot, mmu_idx, TARGET_PAGE_SIZE);
> - }
> + tlb_set_page(cs, address & ~(tlb_size - 1), pa & ~(tlb_size - 1),
> + prot, mmu_idx, tlb_size);
> return true;
> } else if (probe) {
> return false;
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 80d0334e1b..ebd874cde3 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -217,6 +217,35 @@ static int pmp_is_in_range(CPURISCVState *env, int pmp_index, target_ulong addr)
> return result;
> }
>
> +/*
> + * Check if the address has required RWX privs when no PMP entry is matched.
> + */
> +static bool pmp_hart_has_privs_default(CPURISCVState *env, target_ulong addr,
> + target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
> + target_ulong mode)
> +{
> + bool ret;
> +
> + if ((!riscv_feature(env, RISCV_FEATURE_PMP)) || (mode == PRV_M)) {
> + /*
> + * Privileged spec v1.10 states if HW doesn't implement any PMP entry
> + * or no PMP entry matches an M-Mode access, the access succeeds.
> + */
> + ret = true;
> + *allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> + } else {
> + /*
> + * Other modes are not allowed to succeed if they don't * match a rule,
> + * but there are rules. We've checked for no rule earlier in this
> + * function.
> + */
> + ret = false;
> + *allowed_privs = 0;
> + }
> +
> + return ret;
> +}
> +
>
> /*
> * Public Interface
> @@ -226,18 +255,19 @@ static int pmp_is_in_range(CPURISCVState *env, int pmp_index, target_ulong addr)
> * Check if the address has required RWX privs to complete desired operation
> */
> bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> - target_ulong size, pmp_priv_t privs, target_ulong mode)
> + target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
> + target_ulong mode)
> {
> int i = 0;
> int ret = -1;
> int pmp_size = 0;
> target_ulong s = 0;
> target_ulong e = 0;
> - pmp_priv_t allowed_privs = 0;
>
> /* Short cut if no rules */
> if (0 == pmp_get_num_rules(env)) {
> - return (env->priv == PRV_M) ? true : false;
> + return pmp_hart_has_privs_default(env, addr, size, privs,
> + allowed_privs, mode);
> }
>
> if (size == 0) {
> @@ -277,37 +307,25 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> * check
> */
> if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> - allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> + *allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> if ((mode != PRV_M) || pmp_is_locked(env, i)) {
> - allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> + *allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> }
>
> - if ((privs & allowed_privs) == privs) {
> - ret = 1;
> - break;
> - } else {
> - ret = 0;
> - break;
> - }
> + ret = ((privs & *allowed_privs) == privs);
> + break;
> }
> }
>
> /* No rule matched */
> if (ret == -1) {
> - if (mode == PRV_M) {
> - ret = 1; /* Privileged spec v1.10 states if no PMP entry matches an
> - * M-Mode access, the access succeeds */
> - } else {
> - ret = 0; /* Other modes are not allowed to succeed if they don't
> - * match a rule, but there are rules. We've checked for
> - * no rule earlier in this function. */
> - }
> + return pmp_hart_has_privs_default(env, addr, size, privs,
> + allowed_privs, mode);
> }
>
> return ret == 1 ? true : false;
> }
>
> -
> /*
> * Handle a write to a pmpcfg CSP
> */
> @@ -442,3 +460,23 @@ bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
>
> return false;
> }
> +
> +/*
> + * Convert PMP privilege to TLB page privilege.
> + */
> +int pmp_priv_to_page_prot(pmp_priv_t pmp_priv)
> +{
> + int prot = 0;
> +
> + if (pmp_priv & PMP_READ) {
> + prot |= PAGE_READ;
> + }
> + if (pmp_priv & PMP_WRITE) {
> + prot |= PAGE_WRITE;
> + }
> + if (pmp_priv & PMP_EXEC) {
> + prot |= PAGE_EXEC;
> + }
> +
> + return prot;
> +}
> diff --git a/target/riscv/pmp.h b/target/riscv/pmp.h
> index c8d5ef4a69..b82a30f0d5 100644
> --- a/target/riscv/pmp.h
> +++ b/target/riscv/pmp.h
> @@ -59,11 +59,13 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
> target_ulong val);
> target_ulong pmpaddr_csr_read(CPURISCVState *env, uint32_t addr_index);
> bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
> - target_ulong size, pmp_priv_t priv, target_ulong mode);
> + target_ulong size, pmp_priv_t privs, pmp_priv_t *allowed_privs,
> + target_ulong mode);
> bool pmp_is_range_in_tlb(CPURISCVState *env, hwaddr tlb_sa,
> target_ulong *tlb_size);
> 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);
> +int pmp_priv_to_page_prot(pmp_priv_t pmp_priv);
>
> #endif
> --
> 2.30.1
>
>
© 2016 - 2025 Red Hat, Inc.