linux-kernel - Re: Re: [PATCH] RISC-V: KVM: Fix guest page fault within HLV* instructions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20251106132950.80534-1-fangyu.yu@linux.alibaba.com>
Date: Thu,  6 Nov 2025 21:29:50 +0800
From: fangyu.yu@...ux.alibaba.com
To: anup@...infault.org
Cc: alex@...ti.fr,
	aou@...s.berkeley.edu,
	atish.patra@...ux.dev,
	fangyu.yu@...ux.alibaba.com,
	graf@...zon.com,
	guoren@...nel.org,
	jiangyifei@...wei.com,
	kvm-riscv@...ts.infradead.org,
	kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	linux-riscv@...ts.infradead.org,
	palmer@...belt.com,
	paul.walmsley@...ive.com,
	pbonzini@...hat.com
Subject: Re: Re: [PATCH] RISC-V: KVM: Fix guest page fault within HLV* instructions 

>> From: Fangyu Yu <fangyu.yu@...ux.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: 9f7013265112 ("RISC-V: KVM: Handle MMIO exits for VCPU")
>> Signed-off-by: Fangyu Yu <fangyu.yu@...ux.alibaba.com>
>> ---
>>  arch/riscv/kvm/vcpu_insn.c | 21 ++++++++++++++++++---
>>  1 file changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/kvm/vcpu_insn.c b/arch/riscv/kvm/vcpu_insn.c
>> index 97dec18e6989..a8b93aa4d8ec 100644
>> --- a/arch/riscv/kvm/vcpu_insn.c
>> +++ b/arch/riscv/kvm/vcpu_insn.c
>> @@ -448,7 +448,12 @@ int kvm_riscv_vcpu_virtual_insn(struct kvm_vcpu *vcpu, struct kvm_run *run,
>>                         insn = kvm_riscv_vcpu_unpriv_read(vcpu, true,
>>                                                           ct->sepc,
>>                                                           &utrap);
>> -                       if (utrap.scause) {
>> +                       switch (utrap.scause) {
>> +                       case 0:
>> +                               break;
>
>This looks like an unrelated change so drop it or send a separate patch
>with proper explanation.
>
>> +                       case EXC_LOAD_GUEST_PAGE_FAULT:
>> +                               return KVM_INSN_CONTINUE_SAME_SEPC;
>
>The KVM_INSN_xyz enum values are only for functions called via
>system_opcode_insn() so return 1 over here just like the below
>default case.
>
>Also, add some comments over here about why we are simply
>continuing the guest.
>
>> +                       default:
>>                                 utrap.sepc = ct->sepc;
>>                                 kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>>                                 return 1;
>> @@ -503,7 +508,12 @@ 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) {
>> +               switch (utrap.scause) {
>> +               case 0:
>> +                       break;
>
>This looks like an unrelated change so drop it or send a separate patch
>with proper explanation.
>
>> +               case EXC_LOAD_GUEST_PAGE_FAULT:
>> +                       return KVM_INSN_CONTINUE_SAME_SEPC;
>
>Same comment as kvm_riscv_vcpu_virtual_insn().
>
>> +               default:
>>                         /* Redirect trap if we failed to read instruction */
>>                         utrap.sepc = ct->sepc;
>>                         kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>> @@ -629,7 +639,12 @@ 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) {
>> +               switch (utrap.scause) {
>> +               case 0:
>> +                       break;
>
>This looks like an unrelated change so drop it or send a separate patch
>with proper explanation.
>
>> +               case EXC_LOAD_GUEST_PAGE_FAULT:
>> +                       return KVM_INSN_CONTINUE_SAME_SEPC;
>
>Same comment as kvm_riscv_vcpu_virtual_insn().
>
>> +               default:
>>                         /* Redirect trap if we failed to read instruction */
>>                         utrap.sepc = ct->sepc;
>>                         kvm_riscv_vcpu_trap_redirect(vcpu, &utrap);
>> --
>> 2.49.0
>>
>>
>
>Regards,
>Anup

Hi Anup:

Thanks for the review.
I will follow your suggestions in the next version.

Thanks,
Fangyu