Re: [PATCH] riscv: Fix Stage2 SV32 page table walk

[Alistair Francis]

On Mon, Mar 30, 2020 at 9:25 AM Alistair Francis <address@hidden> wrote:
>
> On Mon, Mar 30, 2020 at 1:28 AM Anup Patel <address@hidden> wrote:
> >
> > As-per RISC-V H-Extension v0.5 draft, the Stage2 SV32 page table has
> > 12bits of VPN[1] and 10bits of VPN[0]. The additional 2bits in VPN[1]
> > is required to handle the 34bit intermediate physical address coming
> > from Stage1 SV32 page table. The 12bits of VPN[1] implies that Stage2
> > SV32 level-0 page table will be 16KB in size with total 4096 enteries
> > where each entry maps 4MB of memory (same as Stage1 SV32 page table).
> >
> > The get_physical_address() function is broken for Stage2 SV32 level-0
> > page table because it incorrectly computes output physical address for
> > Stage2 SV32 level-0 page table entry.
> >
> > The root cause of the issue is that get_physical_address() uses the
> > "widened" variable to compute level-0 physical address mapping which
> > changes level-0 mapping size (instead of 4MB). We should use the
> > "widened" variable only for computing index of Stage2 SV32 level-0
> > page table.
> >
> > Signed-off-by: Anup Patel <address@hidden>
>
> Reviewed-by: Alistair Francis <address@hidden>

Applied to the RISC-V tree for 5.1

Alistair

>
> Alistair
>
> > ---
> >  target/riscv/cpu_helper.c | 7 +------
> >  1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 50e13a064f..bc80aa87cf 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -559,12 +559,7 @@ restart:
> >              /* for superpage mappings, make a fake leaf PTE for the TLB's
> >                 benefit. */
> >              target_ulong vpn = addr >> PGSHIFT;
> > -            if (i == 0) {
> > -                *physical = (ppn | (vpn & ((1L << (ptshift + widened)) - 
> > 1))) <<
> > -                             PGSHIFT;
> > -            } else {
> > -                *physical = (ppn | (vpn & ((1L << ptshift) - 1))) << 
> > PGSHIFT;
> > -            }
> > +            *physical = (ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT;
> >
> >              /* set permissions on the TLB entry */
> >              if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> > --
> > 2.17.1
> >
> >