* [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest @ 2024-08-29 6:20 zhouquan 2024-09-12 9:03 ` Quan Zhou 0 siblings, 1 reply; 6+ messages in thread From: zhouquan @ 2024-08-29 6:20 UTC (permalink / raw) To: anup, ajones, atishp, paul.walmsley, palmer, aou Cc: linux-kernel, linux-riscv, kvm, kvm-riscv, Quan Zhou From: Quan Zhou <zhouquan@iscas.ac.cn> 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@iscas.ac.cn> --- 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: case EXC_INST_ILLEGAL: case EXC_LOAD_MISALIGNED: case EXC_STORE_MISALIGNED: base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba -- 2.34.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest 2024-08-29 6:20 [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest zhouquan @ 2024-09-12 9:03 ` Quan Zhou 2024-09-13 0:39 ` Samuel Holland 0 siblings, 1 reply; 6+ messages in thread From: Quan Zhou @ 2024-09-12 9:03 UTC (permalink / raw) To: anup, ajones, atishp, paul.walmsley, palmer, aou Cc: linux-kernel, linux-riscv, kvm, kvm-riscv On 2024/8/29 14:20, zhouquan@iscas.ac.cn wrote: > From: Quan Zhou <zhouquan@iscas.ac.cn> > > 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@iscas.ac.cn> > --- > 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? > case EXC_INST_ILLEGAL: > case EXC_LOAD_MISALIGNED: > case EXC_STORE_MISALIGNED: > > base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest 2024-09-12 9:03 ` Quan Zhou @ 2024-09-13 0:39 ` Samuel Holland 2024-09-13 4:32 ` Anup Patel 0 siblings, 1 reply; 6+ messages in thread From: Samuel Holland @ 2024-09-13 0:39 UTC (permalink / raw) To: Quan Zhou, anup, ajones, atishp, paul.walmsley, palmer, aou Cc: linux-kernel, linux-riscv, kvm, kvm-riscv On 2024-09-12 4:03 AM, Quan Zhou wrote: > > On 2024/8/29 14:20, zhouquan@iscas.ac.cn wrote: >> From: Quan Zhou <zhouquan@iscas.ac.cn> >> >> 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@iscas.ac.cn> >> --- >> 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@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest 2024-09-13 0:39 ` Samuel Holland @ 2024-09-13 4:32 ` Anup Patel 2024-09-14 16:39 ` Samuel Holland 0 siblings, 1 reply; 6+ messages in thread From: Anup Patel @ 2024-09-13 4:32 UTC (permalink / raw) To: Samuel Holland Cc: Quan Zhou, ajones, atishp, paul.walmsley, palmer, aou, linux-kernel, linux-riscv, kvm, kvm-riscv On Fri, Sep 13, 2024 at 6:09 AM Samuel Holland <samuel.holland@sifive.com> wrote: > > On 2024-09-12 4:03 AM, Quan Zhou wrote: > > > > On 2024/8/29 14:20, zhouquan@iscas.ac.cn wrote: > >> From: Quan Zhou <zhouquan@iscas.ac.cn> > >> > >> 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@iscas.ac.cn> > >> --- > >> 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? Any exception delegated to VS-mode via hedeleg means it is directly delivered to VS-mode without any intervention of HS-mode. This aligns with the RISC-V priv specification and there is no alternate semantics assumed by KVM RISC-V. At the moment, for KVM RISC-V we are converging towards the following approach: 1) Only delegate "supervisor expected" traps to VS-mode via hedeleg which supervisor software is generally expected to directly handle such as breakpoint, user syscall, inst page fault, load page fault, and store page fault. 2) Other "supervisor unexpected" traps are redirected to VS-mode via software in HS-mode because these are not typically expected by supervisor software and KVM RISC-V should at least gather some stats for such traps. Previously, we were redirecting such unexpect traps to KVM user space where the KVM user space tool will simply dump the VCPU state and kill the Guest/VM. The inst misaligned trap was historically always set in hedeleg but we should update it based on the above approach. > > Regards, > Samuel > > > > >> case EXC_INST_ILLEGAL: > >> case EXC_LOAD_MISALIGNED: > >> case EXC_STORE_MISALIGNED: > >> > >> base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba > > > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > Regards, Anup ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest 2024-09-13 4:32 ` Anup Patel @ 2024-09-14 16:39 ` Samuel Holland 2024-12-11 8:00 ` Atish Patra 0 siblings, 1 reply; 6+ messages in thread From: Samuel Holland @ 2024-09-14 16:39 UTC (permalink / raw) To: Anup Patel Cc: Quan Zhou, ajones, atishp, paul.walmsley, palmer, aou, linux-kernel, linux-riscv, kvm, kvm-riscv Hi Anup, On 2024-09-12 11:32 PM, Anup Patel wrote: > On Fri, Sep 13, 2024 at 6:09 AM Samuel Holland > <samuel.holland@sifive.com> wrote: >> >> On 2024-09-12 4:03 AM, Quan Zhou wrote: >>> >>> On 2024/8/29 14:20, zhouquan@iscas.ac.cn wrote: >>>> From: Quan Zhou <zhouquan@iscas.ac.cn> >>>> >>>> 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@iscas.ac.cn> >>>> --- >>>> 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? > > Any exception delegated to VS-mode via hedeleg means it is directly delivered > to VS-mode without any intervention of HS-mode. This aligns with the RISC-V > priv specification and there is no alternate semantics assumed by KVM RISC-V. > > At the moment, for KVM RISC-V we are converging towards the following > approach: > > 1) Only delegate "supervisor expected" traps to VS-mode via hedeleg > which supervisor software is generally expected to directly handle such > as breakpoint, user syscall, inst page fault, load page fault, and store > page fault. > > 2) Other "supervisor unexpected" traps are redirected to VS-mode via > software in HS-mode because these are not typically expected by supervisor > software and KVM RISC-V should at least gather some stats for such traps. Can you point me to where we collect stats for these traps? I don't see any code in kvm_riscv_vcpu_exit() that does this. > Previously, we were redirecting such unexpect traps to KVM user space > where the KVM user space tool will simply dump the VCPU state and kill > the Guest/VM. Currently we have 5 exception types that go through software in HS-mode but never kill the guest: EXC_INST_ILLEGAL, EXC_LOAD_MISALIGNED, EXC_STORE_MISALIGNED, EXC_LOAD_ACCESS, and EXC_STORE_ACCESS. Are those considered "expected" or "unexpected"? > The inst misaligned trap was historically always set in hedeleg but we > should update it based on the above approach. What are the criteria for determining if a trap is "supervisor expected" or "supervisor unexpected"? Certainly any trap that can be triggered by misbehaved software in VU-mode should not kill the guest. Similarly, any trap that can be triggered by a misbehaved nested VS-mode guest should not kill the outer guest either. So the only reason I see for not delegating them is to collect stats, but I wonder if that is worth the performance cost. I would rather make misaligned loads/stores (for example) faster in the guest than have a count of them at the hypervisor level. Regards, Samuel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest 2024-09-14 16:39 ` Samuel Holland @ 2024-12-11 8:00 ` Atish Patra 0 siblings, 0 replies; 6+ messages in thread From: Atish Patra @ 2024-12-11 8:00 UTC (permalink / raw) To: Samuel Holland Cc: Anup Patel, Quan Zhou, ajones, paul.walmsley, palmer, aou, linux-kernel, linux-riscv, kvm, kvm-riscv On Sat, Sep 14, 2024 at 9:39 AM Samuel Holland <samuel.holland@sifive.com> wrote: > > Hi Anup, > > On 2024-09-12 11:32 PM, Anup Patel wrote: > > On Fri, Sep 13, 2024 at 6:09 AM Samuel Holland > > <samuel.holland@sifive.com> wrote: > >> > >> On 2024-09-12 4:03 AM, Quan Zhou wrote: > >>> > >>> On 2024/8/29 14:20, zhouquan@iscas.ac.cn wrote: > >>>> From: Quan Zhou <zhouquan@iscas.ac.cn> > >>>> > >>>> 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@iscas.ac.cn> > >>>> --- > >>>> 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? > > > > Any exception delegated to VS-mode via hedeleg means it is directly delivered > > to VS-mode without any intervention of HS-mode. This aligns with the RISC-V > > priv specification and there is no alternate semantics assumed by KVM RISC-V. > > > > At the moment, for KVM RISC-V we are converging towards the following > > approach: > > > > 1) Only delegate "supervisor expected" traps to VS-mode via hedeleg > > which supervisor software is generally expected to directly handle such > > as breakpoint, user syscall, inst page fault, load page fault, and store > > page fault. > > > > 2) Other "supervisor unexpected" traps are redirected to VS-mode via > > software in HS-mode because these are not typically expected by supervisor > > software and KVM RISC-V should at least gather some stats for such traps. > > Can you point me to where we collect stats for these traps? I don't see any code > in kvm_riscv_vcpu_exit() that does this. > It doesn't exist today. But we should add it for debug purposes IMO. > > Previously, we were redirecting such unexpect traps to KVM user space > > where the KVM user space tool will simply dump the VCPU state and kill > > the Guest/VM. > > Currently we have 5 exception types that go through software in HS-mode but > never kill the guest: EXC_INST_ILLEGAL, EXC_LOAD_MISALIGNED, > EXC_STORE_MISALIGNED, EXC_LOAD_ACCESS, and EXC_STORE_ACCESS. Are those > considered "expected" or "unexpected"? > > > The inst misaligned trap was historically always set in hedeleg but we > > should update it based on the above approach. > > What are the criteria for determining if a trap is "supervisor expected" or > "supervisor unexpected"? Certainly any trap that can be triggered by misbehaved > software in VU-mode should not kill the guest. Similarly, any trap that can be > triggered by a misbehaved nested VS-mode guest should not kill the outer guest > either. > > So the only reason I see for not delegating them is to collect stats, but I > wonder if that is worth the performance cost. I would rather make misaligned > loads/stores (for example) faster in the guest than have a count of them at the > hypervisor level. > These are not in a hot path where updating a stats counter will affect the performance. I think we should use both kvm stats (for monitoring from the host) and SBI PMU firmware counters (for monitoring within the guest) which will help both guest and the host performance bottlenecks. I will send a patch. > Regards, > Samuel > -- Regards, Atish ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-12-11 8:00 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-08-29 6:20 [PATCH] RISC-V: KVM: Redirect instruction access fault trap to guest zhouquan 2024-09-12 9:03 ` Quan Zhou 2024-09-13 0:39 ` Samuel Holland 2024-09-13 4:32 ` Anup Patel 2024-09-14 16:39 ` Samuel Holland 2024-12-11 8:00 ` Atish Patra
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®