Re: [PATCH 2/4] target/riscv: Fix hstatus.GVA bit setting for traps taken from HS-mode

[Alistair Francis]

On Thu, May 5, 2022 at 12:36 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Thu, May 5, 2022 at 3:21 PM Alistair Francis <alistair23@gmail.com> wrote:
> >
> > On Fri, Apr 29, 2022 at 1:38 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > Currently, QEMU does not set hstatus.GVA bit for traps taken from
> > > HS-mode into HS-mode which breaks the Xvisor nested MMU test suite
> > > on QEMU. This was working previously.
> > >
> > > This patch updates riscv_cpu_do_interrupt() to fix the above issue.
> > >
> > > Fixes: 86d0c457396b ("target/riscv: Fixup setting GVA")
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > ---
> > >  target/riscv/cpu_helper.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index e1aa4f2097..d83579accf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -1434,7 +1434,6 @@ void riscv_cpu_do_interrupt(CPUState *cs)
> > >                  /* Trap into HS mode */
> > >                  env->hstatus = set_field(env->hstatus, HSTATUS_SPV, 
> > > false);
> > >                  htval = env->guest_phys_fault_addr;
> > > -                write_gva = false;
> >
> > This doesn't seem right.
> >
> > "Field GVA (Guest Virtual Address) is written by the implementation
> > whenever a trap is taken
> > into HS-mode. For any trap (breakpoint, address misaligned, access
> > fault, page fault, or guest-
> > page fault) that writes a guest virtual address to stval, GVA is set
> > to 1. For any other trap into
> > HS-mode, GVA is set to 0"
> >
> > So if we are trapping from HS to HS, the address in stval should not
> > be a guest virtual address, at least in general.
>
> That's not correct. The HLV/HSV instructions executed by hypervisor
> (HS-mode) take guest virtual address. These instructions can trap
> from HS-mode to HS-mode.

Ah, I forgot about those instructions, but still they are the
exception. In general we would expect a trap from HS to HS to contain
HS addresses. We should just handle the other cases specially

Alistair

>
> >
> > We probably aren't correctly setting GVA if MPRV is set though, as
> > then the page faults should be guest addresses. That's probably the
> > issue you are seeing.
>
> The Xvisor nested MMU test-suit is broken on QEMU because it
> uses HLV/HSV instructions in HS-mode.
>
> Regards,
> Anup
>
> >
> > Alistair
> >
> > >              }
> > >              env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 
> > > write_gva);
> > >          }
> > > --
> > > 2.34.1
> > >
> > >