target/riscv: Check PMP rules num before propagation

[PATCH] target/riscv: Check PMP rules num before propagation

Posted by LIU Zhiwei 2 years, 5 months ago

If PMP rules number is zero, it should not influence the TLB entry for
M-mode program.

Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
---
 target/riscv/cpu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9eeed38c7e..48da872d39 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -376,7 +376,7 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot,
     }
 
     *prot = pmp_priv_to_page_prot(pmp_priv);
-    if (tlb_size != NULL) {
+    if ((tlb_size != NULL) && pmp_get_num_rules(env)) {
         if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
             *tlb_size = tlb_size_pmp;
         }
-- 
2.25.1

Re: [PATCH] target/riscv: Check PMP rules num before propagation

Posted by Alistair Francis 2 years, 5 months ago

On Wed, Nov 17, 2021 at 1:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> If PMP rules number is zero, it should not influence the TLB entry for
> M-mode program.

This doesn't sound right. From what I can tell if we have no rules
pmp_is_range_in_tlb() shouldn't have an effect on the tlb_size. What
error are you seeing?

Alistair

>
> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> ---
>  target/riscv/cpu_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9eeed38c7e..48da872d39 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -376,7 +376,7 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot,
>      }
>
>      *prot = pmp_priv_to_page_prot(pmp_priv);
> -    if (tlb_size != NULL) {
> +    if ((tlb_size != NULL) && pmp_get_num_rules(env)) {
>          if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
>              *tlb_size = tlb_size_pmp;
>          }
> --
> 2.25.1
>
>

Re: [PATCH] target/riscv: Check PMP rules num before propagation

Posted by LIU Zhiwei 2 years, 5 months ago

0

On 2021/11/17 上午8:03, Alistair Francis wrote:
> On Wed, Nov 17, 2021 at 1:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>> If PMP rules number is zero, it should not influence the TLB entry for
>> M-mode program.
> This doesn't sound right. From what I can tell if we have no rules
> pmp_is_range_in_tlb() shouldn't have an effect on the tlb_size. What
> error are you seeing?

When address is in [0-4K] and no pmp rule configured,  the tlb_size will 
be set to 1.

This  is caused by pmp_get_tlb_size return a value 1.

if (pmp_sa >= tlb_sa && pmp_ea <= tlb_ea) {
          return pmp_ea - pmp_sa + 1;

}

Here pmp_sa == 0 and pmp_ea == 0.

Thanks,
Zhiwei

> Alistair
>
>> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
>> ---
>>   target/riscv/cpu_helper.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 9eeed38c7e..48da872d39 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -376,7 +376,7 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot,
>>       }
>>
>>       *prot = pmp_priv_to_page_prot(pmp_priv);
>> -    if (tlb_size != NULL) {
>> +    if ((tlb_size != NULL) && pmp_get_num_rules(env)) {
>>           if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
>>               *tlb_size = tlb_size_pmp;
>>           }
>> --
>> 2.25.1
>>
>>

Re: [PATCH] target/riscv: Check PMP rules num before propagation

Posted by Alistair Francis 2 years, 5 months ago

On Wed, Nov 17, 2021 at 10:42 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> 0
>
> On 2021/11/17 上午8:03, Alistair Francis wrote:
> > On Wed, Nov 17, 2021 at 1:12 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
> >> If PMP rules number is zero, it should not influence the TLB entry for
> >> M-mode program.
> > This doesn't sound right. From what I can tell if we have no rules
> > pmp_is_range_in_tlb() shouldn't have an effect on the tlb_size. What
> > error are you seeing?
>
> When address is in [0-4K] and no pmp rule configured,  the tlb_size will
> be set to 1.
>
> This  is caused by pmp_get_tlb_size return a value 1.
>
> if (pmp_sa >= tlb_sa && pmp_ea <= tlb_ea) {
>           return pmp_ea - pmp_sa + 1;
>
> }
>
> Here pmp_sa == 0 and pmp_ea == 0.

Ah ok. Do you mind adding that to the commit message.

Also, do you think it would be better to add the check to
pmp_is_range_in_tlb() instead?

Alistair

>
> Thanks,
> Zhiwei
>
> > Alistair
> >
> >> Signed-off-by: LIU Zhiwei <zhiwei_liu@c-sky.com>
> >> ---
> >>   target/riscv/cpu_helper.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> index 9eeed38c7e..48da872d39 100644
> >> --- a/target/riscv/cpu_helper.c
> >> +++ b/target/riscv/cpu_helper.c
> >> @@ -376,7 +376,7 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot,
> >>       }
> >>
> >>       *prot = pmp_priv_to_page_prot(pmp_priv);
> >> -    if (tlb_size != NULL) {
> >> +    if ((tlb_size != NULL) && pmp_get_num_rules(env)) {
> >>           if (pmp_is_range_in_tlb(env, addr & ~(*tlb_size - 1), &tlb_size_pmp)) {
> >>               *tlb_size = tlb_size_pmp;
> >>           }
> >> --
> >> 2.25.1
> >>
> >>