Re: [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest

[Samuel Holland]

On 2024-09-12 4:03 AM, Quan Zhou wrote:
> 
> On 2024/8/29 14:20, zhouquan@xxxxxxxxxxx wrote:
>> From: Quan Zhou <zhouquan@xxxxxxxxxxx>
>>
>> The M-mode redirects an unhandled instruction access
>> fault trap back to S-mode when not delegating it to
>> VS-mode(hedeleg). However, KVM running in HS-mode
>> terminates the VS-mode software when back from M-mode.
>>
>> The KVM should redirect the trap back to VS-mode, and
>> let VS-mode trap handler decide the next step.
>>
>> Signed-off-by: Quan Zhou <zhouquan@xxxxxxxxxxx>
>> ---
>>   arch/riscv/kvm/vcpu_exit.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/riscv/kvm/vcpu_exit.c b/arch/riscv/kvm/vcpu_exit.c
>> index fa98e5c024b2..696b62850d0b 100644
>> --- a/arch/riscv/kvm/vcpu_exit.c
>> +++ b/arch/riscv/kvm/vcpu_exit.c
>> @@ -182,6 +182,7 @@ int kvm_riscv_vcpu_exit(struct kvm_vcpu *vcpu, struct
>> kvm_run *run,
>>       ret = -EFAULT;
>>       run->exit_reason = KVM_EXIT_UNKNOWN;
>>       switch (trap->scause) {
>> +    case EXC_INST_ACCESS:
> 
> A gentle ping, the instruction access fault should be redirected to
> VS-mode for handling, is my understanding correct?

Yes, this looks correct. However, I believe it would be equivalent (and more
efficient) to add EXC_INST_ACCESS to KVM_HEDELEG_DEFAULT in asm/kvm_host.h.

I don't understand why some exceptions are delegated with hedeleg and others are
caught and redirected here with no further processing. Maybe someone thought
that it wasn't valid to set a bit in hedeleg if the corresponding bit was
cleared in medeleg? But this doesn't make sense, as S-mode cannot know which
bits are set in medeleg (maybe none are!).

So the hypervisor must either:
 1) assume M-mode firmware checks hedeleg and redirects exceptions to VS-mode
    regardless of medeleg, in which case all four of these exceptions can be
    moved to KVM_HEDELEG_DEFAULT and removed from this switch statement, or

 2) assume M-mode might not check hedeleg and redirect exceptions to VS-mode,
    and since no bits are guaranteed to be set in medeleg, any bit set in
    hedeleg must _also_ be handled in the switch case here.

Anup, Atish, thoughts?

Regards,
Samuel

> 
>>       case EXC_INST_ILLEGAL:
>>       case EXC_LOAD_MISALIGNED:
>>       case EXC_STORE_MISALIGNED:
>>
>> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@xxxxxxxxxxxxxxxxxxx
> http://lists.infradead.org/mailman/listinfo/linux-riscv