Re: [PATCH] target/riscv: fix check of guest pa top bits

[Jose Martins]

Just resubmitted version 2. Sorry. Not really used to this. I actually
wasn't using git send-email. I was copying the patch to my email
client which was causing the weird wrapping. I think I also fixed the
issues raised by checkpatch. Hope everything is correct now.

Jose

On Thu, 30 Apr 2020 at 22:36, Alistair Francis <address@hidden> wrote:
>
> On Thu, Apr 30, 2020 at 12:45 PM Alistair Francis <address@hidden> wrote:
> >
> > On Fri, Apr 24, 2020 at 8:10 AM Jose Martins <address@hidden> wrote:
> > >
> > > The spec states that on sv39x4 guest physical  "address bits 63:41
> > > must all be zeros, or else a guest-page-fault exception occurs.".
> > > However, the check performed for these top bits of the virtual address
> > > on the second stage is the same as the one performed for virtual
> > > addresses on the first stage except with the 2-bit extension,
> > > effectively creating the same kind of "hole" in the guest's physical
> > > address space. I believe the following patch fixes this issue:
> > >
> > > Signed-off-by: Jose Martins <address@hidden>
> >
> > Thanks for the patch.
> >
> > Reviewed-by: Alistair Francis <address@hidden>
> >
> > > ---
> > >  target/riscv/cpu_helper.c | 20 +++++++++++++-------
> > >  1 file changed, 13 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index d3ba9efb02..da879f5656 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -421,15 +421,21 @@ static int get_physical_address(CPURISCVState
> > > *env, hwaddr *physical,
> >
> > There seems to be a strange wrapping here, that corrupts the patch.
> > For future patches can you please check your git send-email setup?
> >
> > Applied to the RISC-V tree with the above line fixed up.
>
> This patch also fails checkpatch.
>
> Do you mind resending a v2 with the check patch issues fixed?
>
> Alistair
>
> >
> > Alistair
> >
> > >      int va_bits = PGSHIFT + levels * ptidxbits + widened;
> > >      target_ulong mask, masked_msbs;
> > >
> > > -    if (TARGET_LONG_BITS > (va_bits - 1)) {
> > > -        mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> > > +    if(!first_stage){
> > > +        if ((addr >> va_bits) != 0) {
> > > +            return TRANSLATE_FAIL;
> > > +        }
> > >      } else {
> > > -        mask = 0;
> > > -    }
> > > -    masked_msbs = (addr >> (va_bits - 1)) & mask;
> > > +        if (TARGET_LONG_BITS > (va_bits - 1)) {
> > > +            mask = (1L << (TARGET_LONG_BITS - (va_bits - 1))) - 1;
> > > +        } else {
> > > +            mask = 0;
> > > +        }
> > > +        masked_msbs = (addr >> (va_bits - 1)) & mask;
> > >
> > > -    if (masked_msbs != 0 && masked_msbs != mask) {
> > > -        return TRANSLATE_FAIL;
> > > +        if (masked_msbs != 0 && masked_msbs != mask) {
> > > +            return TRANSLATE_FAIL;
> > > +        }
> > >      }
> > >
> > >      int ptshift = (levels - 1) * ptidxbits;
> > > --
> > > 2.17.1
> > >
> > > Jose
> > >