| Message ID | 20220429033409.258707-3-apatel@ventanamicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | QEMU RISC-V nested virtualization fixes | expand |
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. 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. Alistair > } > env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva); > } > -- > 2.34.1 > >
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. > > 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 > > > >
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 > > > > > >
On Mon, May 9, 2022 at 2:54 PM Alistair Francis <alistair23@gmail.com> wrote: > > 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 I see your point. Let me re-work this patch to ensure that the GVA bit is only set when we have a guest virtual address. Regards, Anup > > 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 > > > > > > > >
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; } env->hstatus = set_field(env->hstatus, HSTATUS_GVA, write_gva); }
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(-)