Re: [PATCH v5 02/13] target/riscv: Move pmp_get_tlb_size apart from get_physical_address_pmp

[Alistair Francis]

On Sat, Apr 29, 2023 at 12:38 AM Weiwei Li  wrote:
>
> pmp_get_tlb_size can be separated from get_physical_address_pmp and is only
> needed when ret == TRANSLATE_SUCCESS.
>
> Signed-off-by: Weiwei Li 
> Signed-off-by: Junqiang Wang 
> Reviewed-by: LIU Zhiwei 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu_helper.c | 16 ++--
>  1 file changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 075fc0538a..83c9699a6d 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -676,14 +676,11 @@ void riscv_cpu_set_mode(CPURISCVState *env, 
> target_ulong newpriv)
>   *
>   * @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,
> +static int get_physical_address_pmp(CPURISCVState *env, int *prot, hwaddr 
> addr,
>  int size, MMUAccessType access_type,
>  int mode)
>  {
> @@ -703,9 +700,6 @@ static int get_physical_address_pmp(CPURISCVState *env, 
> int *prot,
>  }
>
>  *prot = pmp_priv_to_page_prot(pmp_priv);
> -if (tlb_size != NULL) {
> -*tlb_size = pmp_get_tlb_size(env, addr);
> -}
>
>  return TRANSLATE_SUCCESS;
>  }
> @@ -905,7 +899,7 @@ restart:
>  }
>
>  int pmp_prot;
> -int pmp_ret = get_physical_address_pmp(env, _prot, NULL, 
> pte_addr,
> +int pmp_ret = get_physical_address_pmp(env, _prot, pte_addr,
> sizeof(target_ulong),
> MMU_DATA_LOAD, PRV_S);
>  if (pmp_ret != TRANSLATE_SUCCESS) {
> @@ -1300,8 +1294,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, 
> int size,
>  prot &= prot2;
>
>  if (ret == TRANSLATE_SUCCESS) {
> -ret = get_physical_address_pmp(env, _pmp, _size, pa,
> +ret = get_physical_address_pmp(env, _pmp, pa,
> size, access_type, mode);
> +tlb_size = pmp_get_tlb_size(env, pa);
>
>  qemu_log_mask(CPU_LOG_MMU,
>"%s PMP address=" HWADDR_FMT_plx " ret %d prot"
> @@ -1333,8 +1328,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, 
> int size,
>__func__, address, ret, pa, prot);
>
>  if (ret == TRANSLATE_SUCCESS) {
> -ret = get_physical_address_pmp(env, _pmp, _size, pa,
> +ret = get_physical_address_pmp(env, _pmp, pa,
> size, access_type, mode);
> +tlb_size = pmp_get_tlb_size(env, pa);
>
>  qemu_log_mask(CPU_LOG_MMU,
>"%s PMP address=" HWADDR_FMT_plx " ret %d prot"
> --
> 2.25.1
>
>

[PATCH v5 02/13] target/riscv: Move pmp_get_tlb_size apart from get_physical_address_pmp

[Weiwei Li]

pmp_get_tlb_size can be separated from get_physical_address_pmp and is only
needed when ret == TRANSLATE_SUCCESS.

Signed-off-by: Weiwei Li 
Signed-off-by: Junqiang Wang 
Reviewed-by: LIU Zhiwei 
---
 target/riscv/cpu_helper.c | 16 ++--
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 075fc0538a..83c9699a6d 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -676,14 +676,11 @@ void riscv_cpu_set_mode(CPURISCVState *env, target_ulong 
newpriv)
  *
  * @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,
+static int get_physical_address_pmp(CPURISCVState *env, int *prot, hwaddr addr,
 int size, MMUAccessType access_type,
 int mode)
 {
@@ -703,9 +700,6 @@ static int get_physical_address_pmp(CPURISCVState *env, int 
*prot,
 }
 
 *prot = pmp_priv_to_page_prot(pmp_priv);
-if (tlb_size != NULL) {
-*tlb_size = pmp_get_tlb_size(env, addr);
-}
 
 return TRANSLATE_SUCCESS;
 }
@@ -905,7 +899,7 @@ restart:
 }
 
 int pmp_prot;
-int pmp_ret = get_physical_address_pmp(env, _prot, NULL, pte_addr,
+int pmp_ret = get_physical_address_pmp(env, _prot, pte_addr,
sizeof(target_ulong),
MMU_DATA_LOAD, PRV_S);
 if (pmp_ret != TRANSLATE_SUCCESS) {
@@ -1300,8 +1294,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
 prot &= prot2;
 
 if (ret == TRANSLATE_SUCCESS) {
-ret = get_physical_address_pmp(env, _pmp, _size, pa,
+ret = get_physical_address_pmp(env, _pmp, pa,
size, access_type, mode);
+tlb_size = pmp_get_tlb_size(env, pa);
 
 qemu_log_mask(CPU_LOG_MMU,
   "%s PMP address=" HWADDR_FMT_plx " ret %d prot"
@@ -1333,8 +1328,9 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int 
size,
   __func__, address, ret, pa, prot);
 
 if (ret == TRANSLATE_SUCCESS) {
-ret = get_physical_address_pmp(env, _pmp, _size, pa,
+ret = get_physical_address_pmp(env, _pmp, pa,
size, access_type, mode);
+tlb_size = pmp_get_tlb_size(env, pa);
 
 qemu_log_mask(CPU_LOG_MMU,
   "%s PMP address=" HWADDR_FMT_plx " ret %d prot"
-- 
2.25.1