On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistai...@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <a...@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liwei...@iscas.ac.cn> wrote:
> > >
> > > From: Guo Ren <ren_...@c-sky.com>
> > >
> > > Highest bits of PTE has been used for svpbmt, ref: [1], [2], so we
> > > need to ignore them. They cannot be a part of ppn.
> > >
> > > 1: The RISC-V Instruction Set Manual, Volume II: Privileged Architecture
> > > 4.4 Sv39: Page-Based 39-bit Virtual-Memory System
> > > 4.5 Sv48: Page-Based 48-bit Virtual-Memory System
> > >
> > > 2:
> > > https://github.com/riscv/virtual-memory/blob/main/specs/663-Svpbmt-diff.pdf
> > >
> > > Signed-off-by: Guo Ren <ren_...@c-sky.com>
> > > Tested-by: Bin Meng <bmeng...@gmail.com>
> > > Reviewed-by: Liu Zhiwei <zhiwei_...@c-sky.com>
> > > Reviewed-by: Bin Meng <bmeng...@gmail.com>
> > > Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
> > > ---
> > > target/riscv/cpu_bits.h | 7 +++++++
> > > target/riscv/cpu_helper.c | 2 +-
> > > 2 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > > index 5a6d49aa64..282cd8eecd 100644
> > > --- a/target/riscv/cpu_bits.h
> > > +++ b/target/riscv/cpu_bits.h
> > > @@ -490,6 +490,13 @@ typedef enum {
> > > /* Page table PPN shift amount */
> > > #define PTE_PPN_SHIFT 10
> > >
> > > +/* Page table PPN mask */
> > > +#if defined(TARGET_RISCV32)
> > > +#define PTE_PPN_MASK 0xffffffffUL
> > > +#elif defined(TARGET_RISCV64)
> > > +#define PTE_PPN_MASK 0x3fffffffffffffULL
> > > +#endif
> > > +
> >
> > Going forward we should avoid using target specific "#if"
> > so that we can use the same qemu-system-riscv64 for both
> > RV32 and RV64.
> >
> > > /* Leaf page shift amount */
> > > #define PGSHIFT 12
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 434a83e66a..26608ddf1c 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -619,7 +619,7 @@ restart:
> > > return TRANSLATE_FAIL;
> > > }
> > >
> > > - hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> >
> > Rather than using "#if", please use "xlen" comparison to extract
> > PPN correctly from PTE.
>
> This will need to be dynamic based on get_xl()
>
> It does look like we should check the existence of the extensions though:
>
> "Bit 63 is reserved for use by the Svnapot extension in Chapter 5. If
> Svnapot is not implemented, bit 63 remains reserved and must be zeroed
> by software for forward compatibility, or else a page-fault exception
> is raised. Bits 62–61 are reserved for use by the Svpbmt extension in
> Chapter 6. If Svpbmt is not implemented, bits 62–61 remain reserved
> and must be zeroed by software for forward compatibility, or else a
> page-fault exception is raised."
Okay, we would add some checker, such as:
+ if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot)
+ hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ else
+ hwaddr ppn = pte >> PTE_PPN_SHIFT;
>
> Alistair
>
> >
> > Regards,
> > Anup
> >
> > >
> > > if (!(pte & PTE_V)) {
> > > /* Invalid PTE */
> > > --
> > > 2.17.1
> > >
> >
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
