| Message ID | 20251111135506.8526-1-fangyu.yu@linux.alibaba.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2] RISC-V: KVM: Fix guest page fault within HLV* instructions | expand |
On Tue, Nov 11, 2025 at 09:55:06PM +0800, fangyu.yu@linux.alibaba.com wrote: > From: Fangyu Yu <fangyu.yu@linux.alibaba.com> > > When executing HLV* instructions at the HS mode, a guest page fault > may occur when a g-stage page table migration between triggering the > virtual instruction exception and executing the HLV* instruction. > > This may be a corner case, and one simpler way to handle this is to > re-execute the instruction where the virtual instruction exception > occurred, and the guest page fault will be automatically handled. > > Fixes: b91f0e4cb8a3 ("RISC-V: KVM: Factor-out instruction emulation into separate sources") > Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com> > > --- > Changes in v2: > - Remove unnecessary modifications and add comments(suggested by Anup) > - Update Fixes tag > - Link to v1: https://lore.kernel.org/linux-riscv/20250912134332.22053-1-fangyu.yu@linux.alibaba.com/ > --- > arch/riscv/kvm/vcpu_insn.c | 39 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c > index de1f96ea6225..a8d796ef2822 100644 > --- a/arch/riscv/kvm/vcpu_insn.c > +++ b/arch/riscv/kvm/vcpu_insn.c > @@ -323,6 +323,19 @@ int kvm_riscv_vcpu_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, > ct->sepc, > &utrap); > if (utrap.scause) { > + /** > + * If a g-stage page fault occurs, the direct approach > + * is to let the g-stage page fault handler handle it > + * naturally, however, calling the g-stage page fault > + * handler here seems rather strange. > + * Considering this is an corner case, we can directly > + * return to the guest and re-execute the same PC, this > + * will trigger a g-stage page fault again and then the > + * regular g-stage page fault handler will populate > + * g-stage page table. > + */ > + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) > + return 1; > utrap.sepc = ct->sepc; > kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); > return 1; > @@ -378,6 +391,19 @@ int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run, > insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, ct->sepc, > &utrap); > if (utrap.scause) { > + /** > + * If a g-stage page fault occurs, the direct approach > + * is to let the g-stage page fault handler handle it > + * naturally, however, calling the g-stage page fault > + * handler here seems rather strange. > + * Considering this is an corner case, we can directly > + * return to the guest and re-execute the same PC, this > + * will trigger a g-stage page fault again and then the > + * regular g-stage page fault handler will populate > + * g-stage page table. > + */ > + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) > + return 1; > /* Redirect trap if we failed to read instruction */ > utrap.sepc = ct->sepc; > kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); > @@ -504,6 +530,19 @@ int kvm_riscv_vcpu_mmio_store(struct kvm_vcpu *vcpu, struct kvm_run *run, > insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, ct->sepc, > &utrap); > if (utrap.scause) { > + /** > + * If a g-stage page fault occurs, the direct approach > + * is to let the g-stage page fault handler handle it > + * naturally, however, calling the g-stage page fault > + * handler here seems rather strange. > + * Considering this is an corner case, we can directly > + * return to the guest and re-execute the same PC, this > + * will trigger a g-stage page fault again and then the > + * regular g-stage page fault handler will populate > + * g-stage page table. > + */ > + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) > + return 1; > /* Redirect trap if we failed to read instruction */ > utrap.sepc = ct->sepc; > kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); > -- > 2.50.1 > To avoid repeating the same paragraph three times I would create a helper function, kvm_riscv_check_load_guest_page_fault(), with the paragraph placed in that function along with the utrap.scause exception type check. Thanks, drew
>> From: Fangyu Yu <fangyu.yu@linux.alibaba.com> >> >> When executing HLV* instructions at the HS mode, a guest page fault >> may occur when a g-stage page table migration between triggering the >> virtual instruction exception and executing the HLV* instruction. >> >> This may be a corner case, and one simpler way to handle this is to >> re-execute the instruction where the virtual instruction exception >> occurred, and the guest page fault will be automatically handled. >> >> Fixes: b91f0e4cb8a3 ("RISC-V: KVM: Factor-out instruction emulation into separate sources") >> Signed-off-by: Fangyu Yu <fangyu.yu@linux.alibaba.com> >> >> --- >> Changes in v2: >> - Remove unnecessary modifications and add comments(suggested by Anup) >> - Update Fixes tag >> - Link to v1: https://lore.kernel.org/linux-riscv/20250912134332.22053-1-fangyu.yu@linux.alibaba.com/ >> --- >> arch/riscv/kvm/vcpu_insn.c | 39 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 39 insertions(+) >> >> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c >> index de1f96ea6225..a8d796ef2822 100644 >> --- a/arch/riscv/kvm/vcpu_insn.c >> +++ b/arch/riscv/kvm/vcpu_insn.c >> @@ -323,6 +323,19 @@ int kvm_riscv_vcpu_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, >> ct->sepc, >> &utrap); >> if (utrap.scause) { >> + /** >> + * If a g-stage page fault occurs, the direct approach >> + * is to let the g-stage page fault handler handle it >> + * naturally, however, calling the g-stage page fault >> + * handler here seems rather strange. >> + * Considering this is an corner case, we can directly >> + * return to the guest and re-execute the same PC, this >> + * will trigger a g-stage page fault again and then the >> + * regular g-stage page fault handler will populate >> + * g-stage page table. >> + */ >> + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) >> + return 1; >> utrap.sepc = ct->sepc; >> kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); >> return 1; >> @@ -378,6 +391,19 @@ int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run, >> insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, ct->sepc, >> &utrap); >> if (utrap.scause) { >> + /** >> + * If a g-stage page fault occurs, the direct approach >> + * is to let the g-stage page fault handler handle it >> + * naturally, however, calling the g-stage page fault >> + * handler here seems rather strange. >> + * Considering this is an corner case, we can directly >> + * return to the guest and re-execute the same PC, this >> + * will trigger a g-stage page fault again and then the >> + * regular g-stage page fault handler will populate >> + * g-stage page table. >> + */ >> + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) >> + return 1; >> /* Redirect trap if we failed to read instruction */ >> utrap.sepc = ct->sepc; >> kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); >> @@ -504,6 +530,19 @@ int kvm_riscv_vcpu_mmio_store(struct kvm_vcpu *vcpu, struct kvm_run *run, >> insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, ct->sepc, >> &utrap); >> if (utrap.scause) { >> + /** >> + * If a g-stage page fault occurs, the direct approach >> + * is to let the g-stage page fault handler handle it >> + * naturally, however, calling the g-stage page fault >> + * handler here seems rather strange. >> + * Considering this is an corner case, we can directly >> + * return to the guest and re-execute the same PC, this >> + * will trigger a g-stage page fault again and then the >> + * regular g-stage page fault handler will populate >> + * g-stage page table. >> + */ >> + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) >> + return 1; >> /* Redirect trap if we failed to read instruction */ >> utrap.sepc = ct->sepc; >> kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); >> -- >> 2.50.1 >> > >To avoid repeating the same paragraph three times I would create a >helper function, kvm_riscv_check_load_guest_page_fault(), with the >paragraph placed in that function along with the utrap.scause >exception type check. > >Thanks, >drew Thanks for the suggestion! I'll incorporate this change in the next version. Thanks, Fangyu
diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c index de1f96ea6225..a8d796ef2822 100644 --- a/arch/riscv/kvm/vcpu_insn.c +++ b/arch/riscv/kvm/vcpu_insn.c @@ -323,6 +323,19 @@ int kvm_riscv_vcpu_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run, ct->sepc, &utrap); if (utrap.scause) { + /** + * If a g-stage page fault occurs, the direct approach + * is to let the g-stage page fault handler handle it + * naturally, however, calling the g-stage page fault + * handler here seems rather strange. + * Considering this is an corner case, we can directly + * return to the guest and re-execute the same PC, this + * will trigger a g-stage page fault again and then the + * regular g-stage page fault handler will populate + * g-stage page table. + */ + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) + return 1; utrap.sepc = ct->sepc; kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); return 1; @@ -378,6 +391,19 @@ int kvm_riscv_vcpu_mmio_load(struct kvm_vcpu *vcpu, struct kvm_run *run, insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, ct->sepc, &utrap); if (utrap.scause) { + /** + * If a g-stage page fault occurs, the direct approach + * is to let the g-stage page fault handler handle it + * naturally, however, calling the g-stage page fault + * handler here seems rather strange. + * Considering this is an corner case, we can directly + * return to the guest and re-execute the same PC, this + * will trigger a g-stage page fault again and then the + * regular g-stage page fault handler will populate + * g-stage page table. + */ + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) + return 1; /* Redirect trap if we failed to read instruction */ utrap.sepc = ct->sepc; kvm_riscv_vcpu_trap_redirect(vcpu, &utrap); @@ -504,6 +530,19 @@ int kvm_riscv_vcpu_mmio_store(struct kvm_vcpu *vcpu, struct kvm_run *run, insn = kvm_riscv_vcpu_unpriv_read(vcpu, true, ct->sepc, &utrap); if (utrap.scause) { + /** + * If a g-stage page fault occurs, the direct approach + * is to let the g-stage page fault handler handle it + * naturally, however, calling the g-stage page fault + * handler here seems rather strange. + * Considering this is an corner case, we can directly + * return to the guest and re-execute the same PC, this + * will trigger a g-stage page fault again and then the + * regular g-stage page fault handler will populate + * g-stage page table. + */ + if (utrap.scause == EXC_LOAD_GUEST_PAGE_FAULT) + return 1; /* Redirect trap if we failed to read instruction */ utrap.sepc = ct->sepc; kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);