| Message ID | CAC41xo2LfTQZnor5haAgBg=h34qv50xf8Bs1bgSeGESfr-E2ng@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: fix check of guest pa top bits | expand |
On Fri, Apr 24, 2020 at 8:10 AM Jose Martins <josemartins90@gmail.com> 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 <josemartins90@gmail.com> Thanks for the patch. Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > --- > 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. 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 >
On Thu, Apr 30, 2020 at 12:45 PM Alistair Francis <alistair23@gmail.com> wrote: > > On Fri, Apr 24, 2020 at 8:10 AM Jose Martins <josemartins90@gmail.com> 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 <josemartins90@gmail.com> > > Thanks for the patch. > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > --- > > 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 > >
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 <alistair23@gmail.com> wrote: > > On Thu, Apr 30, 2020 at 12:45 PM Alistair Francis <alistair23@gmail.com> wrote: > > > > On Fri, Apr 24, 2020 at 8:10 AM Jose Martins <josemartins90@gmail.com> 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 <josemartins90@gmail.com> > > > > Thanks for the patch. > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > > --- > > > 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 > > >
Yeah, I've run into the same issue. Even if you ask Gmail to treat the email as plain text it still takes the liberty of hard wrapping lines without asking you. It really is a shame that there isn't a good way to submit patches via a web UI, the art of git send-email + manual SMTP isn't as widespread as it once was. Jonathan On Fri, May 1, 2020 at 2:54 PM Jose Martins <josemartins90@gmail.com> wrote: > 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 <alistair23@gmail.com> > wrote: > > > > On Thu, Apr 30, 2020 at 12:45 PM Alistair Francis <alistair23@gmail.com> > wrote: > > > > > > On Fri, Apr 24, 2020 at 8:10 AM Jose Martins <josemartins90@gmail.com> > 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 <josemartins90@gmail.com> > > > > > > Thanks for the patch. > > > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > > > > > > --- > > > > 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 > > > > > >
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, 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; + } }
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 <josemartins90@gmail.com> --- target/riscv/cpu_helper.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) int ptshift = (levels - 1) * ptidxbits;