I'll move the entire check into pmp_hart_has_privs as it makes more sense.
Thanks!
On Fri, Oct 18, 2019, 3:01 PM Palmer Dabbelt <pal...@sifive.com> wrote:
> On Tue, 15 Oct 2019 10:04:32 PDT (-0700), day...@berkeley.edu wrote:
> > Hi,
> >
> > Could this patch go through?
> > If not please let me know so that I can fix.
> > Thank you!
>
> Sorry, I dropped this one. It's in the patch queue now. We should also
> check
> for size==0 in pmp_hart_has_privs(), as that won't work. LMK if you want
> to
> send a patch for that.
>
> >
> > Dayeol
> >
> >
> > On Sat, Oct 12, 2019, 11:30 AM Dayeol Lee <day...@berkeley.edu> wrote:
> >
> >> 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 <finte...@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 <day...@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 <day...@berkeley.edu>
> >>>> Reviewed-by: Richard Henderson <richard.hender...@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
> >>>>
> >>>>
> >>>>
>
