| Message ID | 20230808114711.2013842-24-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: NV trap forwarding infrastructure | expand |
Hi Marc, On 8/8/23 13:47, Marc Zyngier wrote: > HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these > traps to be forwarded. Take this opportunity to deny any 32bit activity > when NV is enabled. I can't figure out how HFGITR_EL2.{SVC_EL1, SVC_EL0 and ERET} are handled. Please could you explain. Eric > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kvm/arm.c | 4 ++++ > arch/arm64/kvm/handle_exit.c | 12 ++++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 72dc53a75d1c..8b51570a76f8 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -36,6 +36,7 @@ > #include <asm/kvm_arm.h> > #include <asm/kvm_asm.h> > #include <asm/kvm_mmu.h> > +#include <asm/kvm_nested.h> > #include <asm/kvm_pkvm.h> > #include <asm/kvm_emulate.h> > #include <asm/sections.h> > @@ -818,6 +819,9 @@ static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu) > if (likely(!vcpu_mode_is_32bit(vcpu))) > return false; > > + if (vcpu_has_nv(vcpu)) > + return true; > + > return !kvm_supports_32bit_el0(); > } > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 6dcd6604b6bc..3b86d534b995 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -226,6 +226,17 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) > return 1; > } > > +static int handle_svc(struct kvm_vcpu *vcpu) > +{ > + /* > + * So far, SVC traps only for NV via HFGITR_EL2. A SVC from a > + * 32bit guest would be caught by vpcu_mode_is_bad_32bit(), so > + * we should only have to deal with a 64 bit exception. > + */ > + kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu)); > + return 1; > +} > + > static exit_handle_fn arm_exit_handlers[] = { > [0 ... ESR_ELx_EC_MAX] = kvm_handle_unknown_ec, > [ESR_ELx_EC_WFx] = kvm_handle_wfx, > @@ -239,6 +250,7 @@ static exit_handle_fn arm_exit_handlers[] = { > [ESR_ELx_EC_SMC32] = handle_smc, > [ESR_ELx_EC_HVC64] = handle_hvc, > [ESR_ELx_EC_SMC64] = handle_smc, > + [ESR_ELx_EC_SVC64] = handle_svc, > [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, > [ESR_ELx_EC_SVE] = handle_sve, > [ESR_ELx_EC_ERET] = kvm_handle_eret,
Hi Eric, On Thu, 10 Aug 2023 09:35:41 +0100, Eric Auger <eric.auger@redhat.com> wrote: > > Hi Marc, > > On 8/8/23 13:47, Marc Zyngier wrote: > > HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these > > traps to be forwarded. Take this opportunity to deny any 32bit activity > > when NV is enabled. > > I can't figure out how HFGITR_EL2.{SVC_EL1, SVC_EL0 and ERET} are > handled. Please could you explain. - SVC: KVM itself never traps it, so any trap of SVC must be the result of a guest trap -- we don't need to do any demultiplexing. We thus directly inject the trap back. This is what the comment in handle_svc() tries to capture, but obviously fails to convey the point. - ERET: This is already handled since 6898a55ce38c ("KVM: arm64: nv: Handle trapped ERET from virtual EL2"). Similarly to SVC, KVM never traps it unless we run NV. Now, looking into it, I think I'm missing the additional case where the L2 guest runs at vEL1. I'm about to add the following patchlet: diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 3b86d534b995..617ae6dea5d5 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -222,7 +222,22 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) if (kvm_vcpu_get_esr(vcpu) & ESR_ELx_ERET_ISS_ERET) return kvm_handle_ptrauth(vcpu); - kvm_emulate_nested_eret(vcpu); + /* + * If we got here, two possibilities: + * + * - the guest is in EL2, and we need to fully emulate ERET + * + * - the guest is in EL1, and we need to reinject the + * exception into the L1 hypervisor. + * + * If KVM ever traps ERET for its own use, we'll have to + * revisit this. + */ + if (is_hyp_ctxt(vcpu)) + kvm_emulate_nested_eret(vcpu); + else + kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu)); + return 1; } Does the above help? Thanks, M.
Hi Marc, On 8/10/23 12:42, Marc Zyngier wrote: > Hi Eric, > > On Thu, 10 Aug 2023 09:35:41 +0100, > Eric Auger <eric.auger@redhat.com> wrote: >> Hi Marc, >> >> On 8/8/23 13:47, Marc Zyngier wrote: >>> HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these >>> traps to be forwarded. Take this opportunity to deny any 32bit activity >>> when NV is enabled. >> I can't figure out how HFGITR_EL2.{SVC_EL1, SVC_EL0 and ERET} are >> handled. Please could you explain. > - SVC: KVM itself never traps it, so any trap of SVC must be the > result of a guest trap -- we don't need to do any demultiplexing. We > thus directly inject the trap back. This is what the comment in > handle_svc() tries to capture, but obviously fails to convey the > point. Thank you for the explanation. Now I get it and this helps. > > - ERET: This is already handled since 6898a55ce38c ("KVM: arm64: nv: > Handle trapped ERET from virtual EL2"). Similarly to SVC, KVM never > traps it unless we run NV. OK > > Now, looking into it, I think I'm missing the additional case where > the L2 guest runs at vEL1. I'm about to add the following patchlet: > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 3b86d534b995..617ae6dea5d5 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -222,7 +222,22 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) > if (kvm_vcpu_get_esr(vcpu) & ESR_ELx_ERET_ISS_ERET) > return kvm_handle_ptrauth(vcpu); > > - kvm_emulate_nested_eret(vcpu); > + /* > + * If we got here, two possibilities: > + * > + * - the guest is in EL2, and we need to fully emulate ERET > + * > + * - the guest is in EL1, and we need to reinject the > + * exception into the L1 hypervisor. but in the case the guest was running in vEL1 are we supposed to trap and end up here? in kvm_emulate_nested_eret I can see "the current EL is always the vEL2 since we set the HCR_EL2.NV bit only when entering the vEL2". But I am still catching up on the already landed [PATCH 00/18] KVM: arm64: Prefix patches for NV support <https://lore.kernel.org/all/20230209175820.1939006-1-maz@kernel.org/> so please forgive me my confusion ;-) Thanks Eric > + * > + * If KVM ever traps ERET for its own use, we'll have to > + * revisit this. > + */ > + if (is_hyp_ctxt(vcpu)) > + kvm_emulate_nested_eret(vcpu); > + else > + kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu)); > + > return 1; > } > > > Does the above help? > > Thanks, > > M. >
On Thu, 10 Aug 2023 18:30:25 +0100, Eric Auger <eric.auger@redhat.com> wrote: > > Hi Marc, > On 8/10/23 12:42, Marc Zyngier wrote: > > Hi Eric, > > > > On Thu, 10 Aug 2023 09:35:41 +0100, > > Eric Auger <eric.auger@redhat.com> wrote: > >> Hi Marc, > >> > >> On 8/8/23 13:47, Marc Zyngier wrote: > >>> HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these > >>> traps to be forwarded. Take this opportunity to deny any 32bit activity > >>> when NV is enabled. > >> I can't figure out how HFGITR_EL2.{SVC_EL1, SVC_EL0 and ERET} are > >> handled. Please could you explain. > > - SVC: KVM itself never traps it, so any trap of SVC must be the > > result of a guest trap -- we don't need to do any demultiplexing. We > > thus directly inject the trap back. This is what the comment in > > handle_svc() tries to capture, but obviously fails to convey the > > point. > Thank you for the explanation. Now I get it and this helps. > > > > - ERET: This is already handled since 6898a55ce38c ("KVM: arm64: nv: > > Handle trapped ERET from virtual EL2"). Similarly to SVC, KVM never > > traps it unless we run NV. > OK > > > > Now, looking into it, I think I'm missing the additional case where > > the L2 guest runs at vEL1. I'm about to add the following patchlet: > > > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > > index 3b86d534b995..617ae6dea5d5 100644 > > --- a/arch/arm64/kvm/handle_exit.c > > +++ b/arch/arm64/kvm/handle_exit.c > > @@ -222,7 +222,22 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) > > if (kvm_vcpu_get_esr(vcpu) & ESR_ELx_ERET_ISS_ERET) > > return kvm_handle_ptrauth(vcpu); > > > > - kvm_emulate_nested_eret(vcpu); > > + /* > > + * If we got here, two possibilities: > > + * > > + * - the guest is in EL2, and we need to fully emulate ERET > > + * > > + * - the guest is in EL1, and we need to reinject the > > + * exception into the L1 hypervisor. > but in the case the guest was running in vEL1 are we supposed to trap > and end up here? in kvm_emulate_nested_eret I can see > "the current EL is always the vEL2 since we set the HCR_EL2.NV bit only > when entering the vEL2". If the guest is running at vEL1, the only ways to trap ERET are: - if the guest hypervisor has set HFGITR_EL2.ERET, because the host KVM never sets that bit on its own - or if the guest hypervisor has set HCR_EL2.NV (which we don't really handle so far, as we don't expose FEAT_NV to guests). If the guest is running at vEL2, then it is HCR_EL2.NV that is responsible for the trap, and we perform the ERET emulation. > But I am still catching up on the already landed > > [PATCH 00/18] KVM: arm64: Prefix patches for NV support > <https://lore.kernel.org/all/20230209175820.1939006-1-maz@kernel.org/> > so please forgive me my confusion ;-) Confusion is the whole purpose of NV, so don't worry, you're in good company here! :D Thanks, M.
Hi Marc, On 8/11/23 09:36, Marc Zyngier wrote: > On Thu, 10 Aug 2023 18:30:25 +0100, > Eric Auger <eric.auger@redhat.com> wrote: >> Hi Marc, >> On 8/10/23 12:42, Marc Zyngier wrote: >>> Hi Eric, >>> >>> On Thu, 10 Aug 2023 09:35:41 +0100, >>> Eric Auger <eric.auger@redhat.com> wrote: >>>> Hi Marc, >>>> >>>> On 8/8/23 13:47, Marc Zyngier wrote: >>>>> HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these >>>>> traps to be forwarded. Take this opportunity to deny any 32bit activity >>>>> when NV is enabled. >>>> I can't figure out how HFGITR_EL2.{SVC_EL1, SVC_EL0 and ERET} are >>>> handled. Please could you explain. >>> - SVC: KVM itself never traps it, so any trap of SVC must be the >>> result of a guest trap -- we don't need to do any demultiplexing. We >>> thus directly inject the trap back. This is what the comment in >>> handle_svc() tries to capture, but obviously fails to convey the >>> point. >> Thank you for the explanation. Now I get it and this helps. >>> - ERET: This is already handled since 6898a55ce38c ("KVM: arm64: nv: >>> Handle trapped ERET from virtual EL2"). Similarly to SVC, KVM never >>> traps it unless we run NV. >> OK >>> Now, looking into it, I think I'm missing the additional case where >>> the L2 guest runs at vEL1. I'm about to add the following patchlet: >>> >>> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >>> index 3b86d534b995..617ae6dea5d5 100644 >>> --- a/arch/arm64/kvm/handle_exit.c >>> +++ b/arch/arm64/kvm/handle_exit.c >>> @@ -222,7 +222,22 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) >>> if (kvm_vcpu_get_esr(vcpu) & ESR_ELx_ERET_ISS_ERET) >>> return kvm_handle_ptrauth(vcpu); >>> >>> - kvm_emulate_nested_eret(vcpu); >>> + /* >>> + * If we got here, two possibilities: >>> + * >>> + * - the guest is in EL2, and we need to fully emulate ERET >>> + * >>> + * - the guest is in EL1, and we need to reinject the >>> + * exception into the L1 hypervisor. >> but in the case the guest was running in vEL1 are we supposed to trap >> and end up here? in kvm_emulate_nested_eret I can see >> "the current EL is always the vEL2 since we set the HCR_EL2.NV bit only >> when entering the vEL2". > If the guest is running at vEL1, the only ways to trap ERET are: > > - if the guest hypervisor has set HFGITR_EL2.ERET, because the host > KVM never sets that bit on its own > > - or if the guest hypervisor has set HCR_EL2.NV (which we don't really > handle so far, as we don't expose FEAT_NV to guests). > > If the guest is running at vEL2, then it is HCR_EL2.NV that is > responsible for the trap, and we perform the ERET emulation. makes sense to me. Explanation about HFGITR_EL2.ERET case is helpful and may be worth to be added as a comment. > >> But I am still catching up on the already landed >> >> [PATCH 00/18] KVM: arm64: Prefix patches for NV support >> <https://lore.kernel.org/all/20230209175820.1939006-1-maz@kernel.org/> >> so please forgive me my confusion ;-) > Confusion is the whole purpose of NV, so don't worry, you're in good > company here! :D :-) Eric > > Thanks, > > M. >
Hi Marc, On 8/8/23 13:47, Marc Zyngier wrote: > HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these > traps to be forwarded. Take this opportunity to deny any 32bit activity > when NV is enabled. > > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/kvm/arm.c | 4 ++++ > arch/arm64/kvm/handle_exit.c | 12 ++++++++++++ > 2 files changed, 16 insertions(+) > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c > index 72dc53a75d1c..8b51570a76f8 100644 > --- a/arch/arm64/kvm/arm.c > +++ b/arch/arm64/kvm/arm.c > @@ -36,6 +36,7 @@ > #include <asm/kvm_arm.h> > #include <asm/kvm_asm.h> > #include <asm/kvm_mmu.h> > +#include <asm/kvm_nested.h> > #include <asm/kvm_pkvm.h> > #include <asm/kvm_emulate.h> > #include <asm/sections.h> > @@ -818,6 +819,9 @@ static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu) > if (likely(!vcpu_mode_is_32bit(vcpu))) > return false; > > + if (vcpu_has_nv(vcpu)) > + return true; > + > return !kvm_supports_32bit_el0(); > } > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 6dcd6604b6bc..3b86d534b995 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -226,6 +226,17 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) > return 1; > } > > +static int handle_svc(struct kvm_vcpu *vcpu) > +{ > + /* > + * So far, SVC traps only for NV via HFGITR_EL2. A SVC from a > + * 32bit guest would be caught by vpcu_mode_is_bad_32bit(), so > + * we should only have to deal with a 64 bit exception. > + */ > + kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu)); > + return 1; > +} > + > static exit_handle_fn arm_exit_handlers[] = { > [0 ... ESR_ELx_EC_MAX] = kvm_handle_unknown_ec, > [ESR_ELx_EC_WFx] = kvm_handle_wfx, > @@ -239,6 +250,7 @@ static exit_handle_fn arm_exit_handlers[] = { > [ESR_ELx_EC_SMC32] = handle_smc, > [ESR_ELx_EC_HVC64] = handle_hvc, > [ESR_ELx_EC_SMC64] = handle_smc, > + [ESR_ELx_EC_SVC64] = handle_svc, > [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, > [ESR_ELx_EC_SVE] = handle_sve, > [ESR_ELx_EC_ERET] = kvm_handle_eret, Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 72dc53a75d1c..8b51570a76f8 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -36,6 +36,7 @@ #include <asm/kvm_arm.h> #include <asm/kvm_asm.h> #include <asm/kvm_mmu.h> +#include <asm/kvm_nested.h> #include <asm/kvm_pkvm.h> #include <asm/kvm_emulate.h> #include <asm/sections.h> @@ -818,6 +819,9 @@ static bool vcpu_mode_is_bad_32bit(struct kvm_vcpu *vcpu) if (likely(!vcpu_mode_is_32bit(vcpu))) return false; + if (vcpu_has_nv(vcpu)) + return true; + return !kvm_supports_32bit_el0(); } diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 6dcd6604b6bc..3b86d534b995 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -226,6 +226,17 @@ static int kvm_handle_eret(struct kvm_vcpu *vcpu) return 1; } +static int handle_svc(struct kvm_vcpu *vcpu) +{ + /* + * So far, SVC traps only for NV via HFGITR_EL2. A SVC from a + * 32bit guest would be caught by vpcu_mode_is_bad_32bit(), so + * we should only have to deal with a 64 bit exception. + */ + kvm_inject_nested_sync(vcpu, kvm_vcpu_get_esr(vcpu)); + return 1; +} + static exit_handle_fn arm_exit_handlers[] = { [0 ... ESR_ELx_EC_MAX] = kvm_handle_unknown_ec, [ESR_ELx_EC_WFx] = kvm_handle_wfx, @@ -239,6 +250,7 @@ static exit_handle_fn arm_exit_handlers[] = { [ESR_ELx_EC_SMC32] = handle_smc, [ESR_ELx_EC_HVC64] = handle_hvc, [ESR_ELx_EC_SMC64] = handle_smc, + [ESR_ELx_EC_SVC64] = handle_svc, [ESR_ELx_EC_SYS64] = kvm_handle_sys_reg, [ESR_ELx_EC_SVE] = handle_sve, [ESR_ELx_EC_ERET] = kvm_handle_eret,
HFGITR_EL2 allows the trap of SVC instructions to EL2. Allow these traps to be forwarded. Take this opportunity to deny any 32bit activity when NV is enabled. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/arm.c | 4 ++++ arch/arm64/kvm/handle_exit.c | 12 ++++++++++++ 2 files changed, 16 insertions(+)