Re: [PATCH] target/riscv: PMP violation due to wrong size parameter

[Dayeol Lee <dayeol@berkeley.edu>]

[-- Attachment #1: Type: text/plain, Size: 2585 bytes --]

No it doesn't mean that.
But the following code will make the size TARGET_PAGE_SIZE - (page offset)
if the address is not aligned.

pmp_size = -(address | TARGET_PAGE_MASK)


On Fri, Oct 11, 2019, 7:37 PM Jonathan Behrens <fintelia@gmail.com> wrote:

> How do you know that the access won't straddle a page boundary? Is there a
> guarantee somewhere that size=0 means that the access is naturally aligned?
>
> Jonathan
>
>
> On Fri, Oct 11, 2019 at 7:14 PM Dayeol Lee <dayeol@berkeley.edu> wrote:
>
>> riscv_cpu_tlb_fill() uses the `size` parameter to check PMP violation
>> using pmp_hart_has_privs().
>> However, if the size is unknown (=0), the ending address will be
>> `addr - 1` as it is `addr + size - 1` in `pmp_hart_has_privs()`.
>> This always causes a false PMP violation on the starting address of the
>> range, as `addr - 1` is not in the range.
>>
>> In order to fix, we just assume that all bytes from addr to the end of
>> the page will be accessed if the size is unknown.
>>
>> Signed-off-by: Dayeol Lee <dayeol@berkeley.edu>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/riscv/cpu_helper.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index e32b6126af..7d9a22b601 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -441,6 +441,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
>> int size,
>>      CPURISCVState *env = &cpu->env;
>>      hwaddr pa = 0;
>>      int prot;
>> +    int pmp_size = 0;
>>      bool pmp_violation = false;
>>      int ret = TRANSLATE_FAIL;
>>      int mode = mmu_idx;
>> @@ -460,9 +461,19 @@ 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 size is unknown (0), assume that all bytes
>> +     * from addr to the end of the page will be accessed.
>> +     */
>> +    if (size == 0) {
>> +        pmp_size = -(address | TARGET_PAGE_MASK);
>> +    } else {
>> +        pmp_size = size;
>> +    }
>> +
>>      if (riscv_feature(env, RISCV_FEATURE_PMP) &&
>>          (ret == TRANSLATE_SUCCESS) &&
>> -        !pmp_hart_has_privs(env, pa, size, 1 << access_type, mode)) {
>> +        !pmp_hart_has_privs(env, pa, pmp_size, 1 << access_type, mode)) {
>>          ret = TRANSLATE_PMP_FAIL;
>>      }
>>      if (ret == TRANSLATE_PMP_FAIL) {
>> --
>> 2.20.1
>>
>>
>>

[-- Attachment #2: Type: text/html, Size: 3830 bytes --]