| Message ID | 20171019145807.23251-10-james.morse@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 19, 2017 at 03:57:55PM +0100, James Morse wrote: > Non-VHE systems take an exception to EL2 in order to world-switch into the > guest. When returning from the guest KVM implicitly restores the DAIF > flags when it returns to the kernel at EL1. > > With VHE none of this exception-level jumping happens, so KVMs > world-switch code is exposed to the host kernel's DAIF values, and KVM > spills the guest-exit DAIF values back into the host kernel. > On entry to a guest we have Debug and SError exceptions unmasked, KVM > has switched VBAR but isn't prepared to handle these. On guest exit > Debug exceptions are left disabled once we return to the host and will > stay this way until we enter user space. > > Add a helper to mask/unmask DAIF around VHE guests. The unmask can only > happen after the hosts VBAR value has been synchronised by the isb in > __vhe_hyp_call (via kvm_call_hyp()). Masking could be as late as > setting KVMs VBAR value, but is kept here for symmetry. > > Signed-off-by: James Morse <james.morse@arm.com> > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> > --- > Give me a kick if you want this reworked as a fix (which will then > conflict with this series), or a backportable version. I don't know of any real-world issues where some more graceful handling of SErrors would make sense on older kernels, so I'm fine with just merging this together with this series. Thanks, -Christoffer > > arch/arm64/include/asm/kvm_host.h | 10 ++++++++++ > virt/kvm/arm/arm.c | 4 ++++ > 2 files changed, 14 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index e923b58606e2..a0e2f7962401 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -25,6 +25,7 @@ > #include <linux/types.h> > #include <linux/kvm_types.h> > #include <asm/cpufeature.h> > +#include <asm/daifflags.h> > #include <asm/kvm.h> > #include <asm/kvm_asm.h> > #include <asm/kvm_mmio.h> > @@ -384,4 +385,13 @@ static inline void __cpu_init_stage2(void) > "PARange is %d bits, unsupported configuration!", parange); > } > > +static inline void kvm_arm_vhe_guest_enter(void) > +{ > + local_daif_mask(); > +} > + > +static inline void kvm_arm_vhe_guest_exit(void) > +{ > + local_daif_restore(DAIF_PROCCTX_NOIRQ); > +} > #endif /* __ARM64_KVM_HOST_H__ */ > diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c > index b9f68e4add71..665529924b34 100644 > --- a/virt/kvm/arm/arm.c > +++ b/virt/kvm/arm/arm.c > @@ -698,9 +698,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) > */ > trace_kvm_entry(*vcpu_pc(vcpu)); > guest_enter_irqoff(); > + if (has_vhe()) > + kvm_arm_vhe_guest_enter(); > > ret = kvm_call_hyp(__kvm_vcpu_run, vcpu); > > + if (has_vhe()) > + kvm_arm_vhe_guest_exit(); > vcpu->mode = OUTSIDE_GUEST_MODE; > vcpu->stat.exits++; > /* > -- > 2.13.3 >
Hi Christoffer, On 30/10/17 07:40, Christoffer Dall wrote: > On Thu, Oct 19, 2017 at 03:57:55PM +0100, James Morse wrote: >> Non-VHE systems take an exception to EL2 in order to world-switch into the >> guest. When returning from the guest KVM implicitly restores the DAIF >> flags when it returns to the kernel at EL1. >> >> With VHE none of this exception-level jumping happens, so KVMs >> world-switch code is exposed to the host kernel's DAIF values, and KVM >> spills the guest-exit DAIF values back into the host kernel. >> On entry to a guest we have Debug and SError exceptions unmasked, KVM >> has switched VBAR but isn't prepared to handle these. On guest exit >> Debug exceptions are left disabled once we return to the host and will >> stay this way until we enter user space. >> >> Add a helper to mask/unmask DAIF around VHE guests. The unmask can only >> happen after the hosts VBAR value has been synchronised by the isb in >> __vhe_hyp_call (via kvm_call_hyp()). Masking could be as late as >> setting KVMs VBAR value, but is kept here for symmetry. > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> Thanks! >> --- >> Give me a kick if you want this reworked as a fix (which will then >> conflict with this series), or a backportable version. > > I don't know of any real-world issues where some more graceful handling > of SErrors would make sense on older kernels, so I'm fine with just > merging this together with this series. What about debug? > On guest exit Debug exceptions are left disabled once we return to the host > and will stay this way until we enter user space. Today VHE:KVM causes the kernel to run with SError unmasked and debug disabled until the next return to user-space, whereas previously the kernel expected SError to be masked and debug enabled. (Reposting just the SError rework without this patch changes the kernel to expect SError to be unmasked, which isn't making this any worse.) Thanks, James
On Thu, Nov 02, 2017 at 12:14:28PM +0000, James Morse wrote: > Hi Christoffer, > > On 30/10/17 07:40, Christoffer Dall wrote: > > On Thu, Oct 19, 2017 at 03:57:55PM +0100, James Morse wrote: > >> Non-VHE systems take an exception to EL2 in order to world-switch into the > >> guest. When returning from the guest KVM implicitly restores the DAIF > >> flags when it returns to the kernel at EL1. > >> > >> With VHE none of this exception-level jumping happens, so KVMs > >> world-switch code is exposed to the host kernel's DAIF values, and KVM > >> spills the guest-exit DAIF values back into the host kernel. > >> On entry to a guest we have Debug and SError exceptions unmasked, KVM > >> has switched VBAR but isn't prepared to handle these. On guest exit > >> Debug exceptions are left disabled once we return to the host and will > >> stay this way until we enter user space. > >> > >> Add a helper to mask/unmask DAIF around VHE guests. The unmask can only > >> happen after the hosts VBAR value has been synchronised by the isb in > >> __vhe_hyp_call (via kvm_call_hyp()). Masking could be as late as > >> setting KVMs VBAR value, but is kept here for symmetry. > > > Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org> > > Thanks! > > > >> --- > >> Give me a kick if you want this reworked as a fix (which will then > >> conflict with this series), or a backportable version. > > > > I don't know of any real-world issues where some more graceful handling > > of SErrors would make sense on older kernels, so I'm fine with just > > merging this together with this series. > > What about debug? Are we unmasking debug exceptions as we should with this patch? If so, I suppose that could be required for something like kgdb or when running KVM as a guest hypervisor (nested). In that case, we should probably provide a backport for stable, if we think people are going to be running older kernels on VHE systems, which they probably are. > > On guest exit Debug exceptions are left disabled once we return to the host > > and will stay this way until we enter user space. [The indentation seems to indicate I wrote this, but I don't think I did. I'm confused.] > > Today VHE:KVM causes the kernel to run with SError unmasked and debug disabled > until the next return to user-space, whereas previously the kernel expected > SError to be masked and debug enabled. > > > (Reposting just the SError rework without this patch changes the kernel to > expect SError to be unmasked, which isn't making this any worse.) > I'm sorry, I don't understand this discussion. What is today, and what is previously, and are you suggesting we drop this patch, or that the rest of this series is somehow going to be applied without this patch? Reset: I think this patch is fine in the context of this series.. I now have no idea what we need to do in terms of older kernels. Thanks, -Christoffer
Hi Christoffer, On 03/11/17 12:45, Christoffer Dall wrote: > On Thu, Nov 02, 2017 at 12:14:28PM +0000, James Morse wrote: >> On 30/10/17 07:40, Christoffer Dall wrote: >>> On Thu, Oct 19, 2017 at 03:57:55PM +0100, James Morse wrote: >>>> Non-VHE systems take an exception to EL2 in order to world-switch into the >>>> guest. When returning from the guest KVM implicitly restores the DAIF >>>> flags when it returns to the kernel at EL1. >>>> >>>> With VHE none of this exception-level jumping happens, so KVMs >>>> world-switch code is exposed to the host kernel's DAIF values, and KVM >>>> spills the guest-exit DAIF values back into the host kernel. >>>> On entry to a guest we have Debug and SError exceptions unmasked, KVM >>>> has switched VBAR but isn't prepared to handle these. On guest exit >>>> Debug exceptions are left disabled once we return to the host and will >>>> stay this way until we enter user space. >>>> Give me a kick if you want this reworked as a fix (which will then >>>> conflict with this series), or a backportable version. >>> >>> I don't know of any real-world issues where some more graceful handling >>> of SErrors would make sense on older kernels, so I'm fine with just >>> merging this together with this series. >> >> What about debug? > Are we unmasking debug exceptions as we should with this patch? With this patch, yes, it directly restores the DAIF flags the arch code wants for irq-masked process-context. Debug is re-enabled. > If so, I suppose that could be required for something like kgdb or when > running KVM as a guest hypervisor (nested). > > In that case, we should probably provide a backport for stable, if we > think people are going to be running older kernels on VHE systems, which > they probably are. Okay, I will produce a backport once this gets merged. >>> On guest exit Debug exceptions are left disabled once we return to the host >>> and will stay this way until we enter user space. > [The indentation seems to indicate I wrote this, but I don't think I > did. I'm confused.] I quoted it from the commit message, but evidently not from this depth-of-reply. Sorry for the confusion. >> Today VHE:KVM causes the kernel to run with SError unmasked and debug disabled >> until the next return to user-space, whereas previously the kernel expected >> SError to be masked and debug enabled. >> >> >> (Reposting just the SError rework without this patch changes the kernel to >> expect SError to be unmasked, which isn't making this any worse.) > I'm sorry, I don't understand this discussion. What is today, and what English has failed me. I'll try again: v4.14-rc7 with VHE causes the kernel to run after guest-exit with SError unmasked and debug disabled until the next return to user-space. The arch code expects SError masked and debug enabled. In your kgdb example, if we switch-to a new task instead of returning to user space, it won't hit any break/watchpoints. > is previously, and are you suggesting we drop this patch, or that the > rest of this series is somehow going to be applied without this patch? I reposted just the SError rework, patches 1-10 without this patch. If merged, this would change the arch code to expect SError to be unmasked from process context, leaving just the debug disabled after VHE guest-exit. I was (hurriedly) trying to work out if reposting the SError-rework without this patch made the situation worse. Sorry for the confusion! James > Reset: I think this patch is fine in the context of this series.. I now > have no idea what we need to do in terms of older kernels. > > Thanks, > -Christoffer >
On Fri, Nov 03, 2017 at 05:19:40PM +0000, James Morse wrote: > Hi Christoffer, > > On 03/11/17 12:45, Christoffer Dall wrote: > > On Thu, Nov 02, 2017 at 12:14:28PM +0000, James Morse wrote: > >> On 30/10/17 07:40, Christoffer Dall wrote: > >>> On Thu, Oct 19, 2017 at 03:57:55PM +0100, James Morse wrote: > >>>> Non-VHE systems take an exception to EL2 in order to world-switch into the > >>>> guest. When returning from the guest KVM implicitly restores the DAIF > >>>> flags when it returns to the kernel at EL1. > >>>> > >>>> With VHE none of this exception-level jumping happens, so KVMs > >>>> world-switch code is exposed to the host kernel's DAIF values, and KVM > >>>> spills the guest-exit DAIF values back into the host kernel. > >>>> On entry to a guest we have Debug and SError exceptions unmasked, KVM > >>>> has switched VBAR but isn't prepared to handle these. On guest exit > >>>> Debug exceptions are left disabled once we return to the host and will > >>>> stay this way until we enter user space. > > > >>>> Give me a kick if you want this reworked as a fix (which will then > >>>> conflict with this series), or a backportable version. > >>> > >>> I don't know of any real-world issues where some more graceful handling > >>> of SErrors would make sense on older kernels, so I'm fine with just > >>> merging this together with this series. > >> > >> What about debug? > > > Are we unmasking debug exceptions as we should with this patch? > > With this patch, yes, it directly restores the DAIF flags the arch code wants > for irq-masked process-context. Debug is re-enabled. > > > > If so, I suppose that could be required for something like kgdb or when > > running KVM as a guest hypervisor (nested). > > > > In that case, we should probably provide a backport for stable, if we > > think people are going to be running older kernels on VHE systems, which > > they probably are. > > Okay, I will produce a backport once this gets merged. > > > >>> On guest exit Debug exceptions are left disabled once we return to the host > >>> and will stay this way until we enter user space. > > > [The indentation seems to indicate I wrote this, but I don't think I > > did. I'm confused.] > > I quoted it from the commit message, but evidently not from this depth-of-reply. > Sorry for the confusion. > > > >> Today VHE:KVM causes the kernel to run with SError unmasked and debug disabled > >> until the next return to user-space, whereas previously the kernel expected > >> SError to be masked and debug enabled. > >> > >> > >> (Reposting just the SError rework without this patch changes the kernel to > >> expect SError to be unmasked, which isn't making this any worse.) > > > I'm sorry, I don't understand this discussion. What is today, and what > > English has failed me. I'll try again: > > v4.14-rc7 with VHE causes the kernel to run after guest-exit with SError > unmasked and debug disabled until the next return to user-space. > > The arch code expects SError masked and debug enabled. > > In your kgdb example, if we switch-to a new task instead of returning to user > space, it won't hit any break/watchpoints. > > > > is previously, and are you suggesting we drop this patch, or that the > > rest of this series is somehow going to be applied without this patch? > > I reposted just the SError rework, patches 1-10 without this patch. > > If merged, this would change the arch code to expect SError to be unmasked from > process context, leaving just the debug disabled after VHE guest-exit. > > I was (hurriedly) trying to work out if reposting the SError-rework without this > patch made the situation worse. > > > Sorry for the confusion! > No worries, and thanks for the explanation. -Christoffer
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index e923b58606e2..a0e2f7962401 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -25,6 +25,7 @@ #include <linux/types.h> #include <linux/kvm_types.h> #include <asm/cpufeature.h> +#include <asm/daifflags.h> #include <asm/kvm.h> #include <asm/kvm_asm.h> #include <asm/kvm_mmio.h> @@ -384,4 +385,13 @@ static inline void __cpu_init_stage2(void) "PARange is %d bits, unsupported configuration!", parange); } +static inline void kvm_arm_vhe_guest_enter(void) +{ + local_daif_mask(); +} + +static inline void kvm_arm_vhe_guest_exit(void) +{ + local_daif_restore(DAIF_PROCCTX_NOIRQ); +} #endif /* __ARM64_KVM_HOST_H__ */ diff --git a/virt/kvm/arm/arm.c b/virt/kvm/arm/arm.c index b9f68e4add71..665529924b34 100644 --- a/virt/kvm/arm/arm.c +++ b/virt/kvm/arm/arm.c @@ -698,9 +698,13 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) */ trace_kvm_entry(*vcpu_pc(vcpu)); guest_enter_irqoff(); + if (has_vhe()) + kvm_arm_vhe_guest_enter(); ret = kvm_call_hyp(__kvm_vcpu_run, vcpu); + if (has_vhe()) + kvm_arm_vhe_guest_exit(); vcpu->mode = OUTSIDE_GUEST_MODE; vcpu->stat.exits++; /*
Non-VHE systems take an exception to EL2 in order to world-switch into the guest. When returning from the guest KVM implicitly restores the DAIF flags when it returns to the kernel at EL1. With VHE none of this exception-level jumping happens, so KVMs world-switch code is exposed to the host kernel's DAIF values, and KVM spills the guest-exit DAIF values back into the host kernel. On entry to a guest we have Debug and SError exceptions unmasked, KVM has switched VBAR but isn't prepared to handle these. On guest exit Debug exceptions are left disabled once we return to the host and will stay this way until we enter user space. Add a helper to mask/unmask DAIF around VHE guests. The unmask can only happen after the hosts VBAR value has been synchronised by the isb in __vhe_hyp_call (via kvm_call_hyp()). Masking could be as late as setting KVMs VBAR value, but is kept here for symmetry. Signed-off-by: James Morse <james.morse@arm.com> --- Give me a kick if you want this reworked as a fix (which will then conflict with this series), or a backportable version. arch/arm64/include/asm/kvm_host.h | 10 ++++++++++ virt/kvm/arm/arm.c | 4 ++++ 2 files changed, 14 insertions(+)