| Message ID | 20161202084754.22860-6-rkagan@virtuozzo.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
----- Original Message ----- > From: "Roman Kagan" <rkagan@virtuozzo.com> > To: "Radim Krčmář" <rkrcmar@redhat.com>, "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org > Cc: "Denis Lunev" <den@virtuozzo.com>, "Roman Kagan" <rkagan@virtuozzo.com> > Sent: Friday, December 2, 2016 9:47:54 AM > Subject: [PATCH 5/5] kvm/svm: kick L2 guest to L1 by ready async_pf > > When async pagefault is resolved vCPU may be executing L2 guest. > > In order to allow L1 take better scheduling decisions in such cases, > make L2 exit to L1 on a fake external interupt, without actually > injecting it (unless L2 has other reasons to vmexit). > > This patch does that for x86/AMD. > > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> > --- > Note 1: I didn't have a chance to test this; I'll do when I get access to an > AMD machine and let you know if anything goes wrong > > Note 2: I mostly modelled this patch after the one for vmx but is looks not > very appealing for svm. I'd appreciate being pointed at a better location > where to stick the fake external interrupt vmexit. This should not be necessary, SVM has a different mechanism (which requires L1 cooperation) to handle async page faults. Paolo > > arch/x86/kvm/svm.c | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 8ca1eca..1f6ae15 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -3723,6 +3723,15 @@ static int mwait_interception(struct vcpu_svm *svm) > return nop_interception(svm); > } > > +static int svm_check_nested_events(struct kvm_vcpu *vcpu, bool > external_intr) > +{ > + struct vcpu_svm *svm = to_svm(vcpu); > + > + if (kvm_async_pf_has_ready(vcpu)) > + nested_svm_intr(svm); > + return 0; > +} > + > enum avic_ipi_failure_cause { > AVIC_IPI_FAILURE_INVALID_INT_TYPE, > AVIC_IPI_FAILURE_TARGET_NOT_RUNNING, > @@ -5406,6 +5415,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = > { > > .check_intercept = svm_check_intercept, > .handle_external_intr = svm_handle_external_intr, > + .check_nested_events = svm_check_nested_events, > > .sched_in = svm_sched_in, > > -- > 2.9.3 > > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 02, 2016 at 04:35:24AM -0500, Paolo Bonzini wrote: > > > ----- Original Message ----- > > From: "Roman Kagan" <rkagan@virtuozzo.com> > > To: "Radim Krčmář" <rkrcmar@redhat.com>, "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org > > Cc: "Denis Lunev" <den@virtuozzo.com>, "Roman Kagan" <rkagan@virtuozzo.com> > > Sent: Friday, December 2, 2016 9:47:54 AM > > Subject: [PATCH 5/5] kvm/svm: kick L2 guest to L1 by ready async_pf > > > > When async pagefault is resolved vCPU may be executing L2 guest. > > > > In order to allow L1 take better scheduling decisions in such cases, > > make L2 exit to L1 on a fake external interupt, without actually > > injecting it (unless L2 has other reasons to vmexit). > > > > This patch does that for x86/AMD. > > > > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> > > --- > > Note 1: I didn't have a chance to test this; I'll do when I get access to an > > AMD machine and let you know if anything goes wrong > > > > Note 2: I mostly modelled this patch after the one for vmx but is looks not > > very appealing for svm. I'd appreciate being pointed at a better location > > where to stick the fake external interrupt vmexit. > > This should not be necessary, SVM has a different mechanism (which > requires L1 cooperation) to handle async page faults. Hmm, I didn't realize that... I'm afraid we have a problem then: my patch that triggered all this, "kvm/x86: skip async_pf when in guest mode", makes async_pf's not delivered if the vcpu is executing L2 on all x86, so that mechanism will stop working. I'm also curious what L1 cooperation is assumed here. E.g. would L1 running linux (which would set up async_pf) and a non-KVM hypervisor cooperate as expected? Looks like I need another dive into svm.c... Roman. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Dec 02, 2016 at 03:33:03PM +0300, Roman Kagan wrote: > On Fri, Dec 02, 2016 at 04:35:24AM -0500, Paolo Bonzini wrote: > > > > > > ----- Original Message ----- > > > From: "Roman Kagan" <rkagan@virtuozzo.com> > > > To: "Radim Krčmář" <rkrcmar@redhat.com>, "Paolo Bonzini" <pbonzini@redhat.com>, kvm@vger.kernel.org > > > Cc: "Denis Lunev" <den@virtuozzo.com>, "Roman Kagan" <rkagan@virtuozzo.com> > > > Sent: Friday, December 2, 2016 9:47:54 AM > > > Subject: [PATCH 5/5] kvm/svm: kick L2 guest to L1 by ready async_pf > > > > > > When async pagefault is resolved vCPU may be executing L2 guest. > > > > > > In order to allow L1 take better scheduling decisions in such cases, > > > make L2 exit to L1 on a fake external interupt, without actually > > > injecting it (unless L2 has other reasons to vmexit). > > > > > > This patch does that for x86/AMD. > > > > > > Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> > > > --- > > > Note 1: I didn't have a chance to test this; I'll do when I get access to an > > > AMD machine and let you know if anything goes wrong > > > > > > Note 2: I mostly modelled this patch after the one for vmx but is looks not > > > very appealing for svm. I'd appreciate being pointed at a better location > > > where to stick the fake external interrupt vmexit. > > > > This should not be necessary, SVM has a different mechanism (which > > requires L1 cooperation) to handle async page faults. > > Hmm, I didn't realize that... I'm afraid we have a problem then: my > patch that triggered all this, "kvm/x86: skip async_pf when in guest > mode", makes async_pf's not delivered if the vcpu is executing L2 on > all x86, so that mechanism will stop working. > > I'm also curious what L1 cooperation is assumed here. E.g. would L1 > running linux (which would set up async_pf) and a non-KVM hypervisor > cooperate as expected? Looks like I need another dive into svm.c... Turns out at least VirtualBox triggers an assertion when a guest vmexits on a #PF when NPT is on, see https://www.virtualbox.org/browser/vbox/trunk/src/VBox/VMM/VMMR0/HMSVMR0.cpp#L5353 (BTW this is also what KVM does on Intel). So I tend to think that in general what this patch did -- kicking L2 into L1 by an "external interrupt" with no valid interrupt info -- was less likely to confuse L1. I've now grabbed a suitable AMD machine and it at least passed a smoke test with KVM on KVM. Unfortunately I failed to run VirtualBox in a L1 guest (I haven't figured out why) but looking at the code it should be ok with external interrupt vmexit, too. So unless there are better ideas I'll respin the series taking into account the comments accumulated so far. Roman. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 8ca1eca..1f6ae15 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -3723,6 +3723,15 @@ static int mwait_interception(struct vcpu_svm *svm) return nop_interception(svm); } +static int svm_check_nested_events(struct kvm_vcpu *vcpu, bool external_intr) +{ + struct vcpu_svm *svm = to_svm(vcpu); + + if (kvm_async_pf_has_ready(vcpu)) + nested_svm_intr(svm); + return 0; +} + enum avic_ipi_failure_cause { AVIC_IPI_FAILURE_INVALID_INT_TYPE, AVIC_IPI_FAILURE_TARGET_NOT_RUNNING, @@ -5406,6 +5415,7 @@ static struct kvm_x86_ops svm_x86_ops __ro_after_init = { .check_intercept = svm_check_intercept, .handle_external_intr = svm_handle_external_intr, + .check_nested_events = svm_check_nested_events, .sched_in = svm_sched_in,
When async pagefault is resolved vCPU may be executing L2 guest. In order to allow L1 take better scheduling decisions in such cases, make L2 exit to L1 on a fake external interupt, without actually injecting it (unless L2 has other reasons to vmexit). This patch does that for x86/AMD. Signed-off-by: Roman Kagan <rkagan@virtuozzo.com> --- Note 1: I didn't have a chance to test this; I'll do when I get access to an AMD machine and let you know if anything goes wrong Note 2: I mostly modelled this patch after the one for vmx but is looks not very appealing for svm. I'd appreciate being pointed at a better location where to stick the fake external interrupt vmexit. arch/x86/kvm/svm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)