Re: [PATCH] riscv: Raise an exception if pte reserved bits are not cleared

[Alexandre Ghiti]

Hi Alistair,

Sorry for the late reply, I was on PTO.

On Tue, Apr 18, 2023 at 4:22 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Mon, Apr 17, 2023 at 8:47 PM Andrea Parri <andrea@rivosinc.com> wrote:
> >
> > Hi Alistair,
> >
> > > > @@ -936,6 +936,11 @@ restart:
> > > >              return TRANSLATE_FAIL;
> > > >          }
> > > >
> > > > +        /* PTE reserved bits must be cleared otherwise an exception is 
> > > > raised */
> > > > +        if (riscv_cpu_mxl(env) == MXL_RV64 && (pte & PTE_RESERVED)) {
> > > > +            return TRANSLATE_FAIL;
> > > > +        }
> > >
> > > Isn't this caught by our existing check?
> > >
> > >             if ((pte & ~(target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT) {
> > >                 return TRANSLATE_FAIL;
> > >             }
> >
> > Thanks for checking this out.  AFAICS, the existing check/code doesn't
> > work if either svnapot or svpbmt are active.
>
> svpbmt uses some of the reserved fields right?

No, pbmt uses bits 61-62 and napot uses bit 63, this patchset deals
with bits 54-60.

>
> I'm not sure why svnapot excludes the check. The correct fix should be
> to change this check as required (instead of adding a new check).

napot and pbmt exclude the check because PTE_PPN_MASK only deals with
PPN bits, and then ~PTE_PPN_MASK is too large as it tests the
PBMT/NAPOT bits AND the reserved bits.
But you made me notice that the pbmt/napot check isn't right either,
as the napot bit could be set if !napot and then an exception would
not be triggered (the same for pbmt).

Let me check this completely and come back with a proper fix for that too.

Thanks!

Alex

>
> Alistair
>
> >
> > Please let me know if you need other information.
> >
> >   Andrea