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

[fangyu.yu@linux.alibaba.com]

>> 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