diff mbox series

[RESEND] KVM: X86: Use IPI shorthands in kvm guest when support

Message ID 1564121727-30020-1-git-send-email-wanpengli@tencent.com (mailing list archive)
State New, archived
Headers show
Series [RESEND] KVM: X86: Use IPI shorthands in kvm guest when support | expand

Commit Message

Wanpeng Li July 26, 2019, 6:15 a.m. UTC
From: Wanpeng Li <wanpengli@tencent.com>

IPI shorthand is supported now by linux apic/x2apic driver, switch to 
IPI shorthand for all excluding self and all including self destination 
shorthand in kvm guest, to avoid splitting the target mask into serveral 
PV IPI hypercalls.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Cc: Sean Christopherson <sean.j.christopherson@intel.com>
Cc: Nadav Amit <namit@vmware.com>
Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
---
Note: rebase against tip tree's x86/apic branch

 arch/x86/kernel/kvm.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Sean Christopherson July 29, 2019, 5:50 p.m. UTC | #1
On Fri, Jul 26, 2019 at 02:15:27PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <wanpengli@tencent.com>
> 
> IPI shorthand is supported now by linux apic/x2apic driver, switch to 
> IPI shorthand for all excluding self and all including self destination 
> shorthand in kvm guest, to avoid splitting the target mask into serveral 
> PV IPI hypercalls.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Nadav Amit <namit@vmware.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
> Note: rebase against tip tree's x86/apic branch
> 
>  arch/x86/kernel/kvm.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index b7f34fe..87b73b8 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -34,7 +34,9 @@
>  #include <asm/hypervisor.h>
>  #include <asm/tlb.h>
>  
> +static struct apic orig_apic;

Copying the entire struct apic to snapshot two functions is funky,
explicitly capturing the two functions would be more intuitive.

Tangentially related, kvm_setup_pv_ipi() can be annotated __init,
which means the function snapshots can be __ro_after_init.

That being said, can't we just remove kvm_send_ipi_all() and
kvm_send_ipi_allbutself()?  AFAICT, the variations that lead to shorthand
are only used when apic_use_ipi_shorthand is true.  If that isn't the
case, fixing the callers in the APIC code seems like the correct thing to
do.

>  static int kvmapf = 1;
> +DECLARE_STATIC_KEY_FALSE(apic_use_ipi_shorthand);
>  static int __init parse_no_kvmapf(char *arg)
>  {
> @@ -507,12 +509,18 @@ static void kvm_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
>  
>  static void kvm_send_ipi_allbutself(int vector)
>  {
> -	kvm_send_ipi_mask_allbutself(cpu_online_mask, vector);
> +	if (static_branch_likely(&apic_use_ipi_shorthand))
> +		orig_apic.send_IPI_allbutself(vector);
> +	else
> +		kvm_send_ipi_mask_allbutself(cpu_online_mask, vector);
>  }
>  
>  static void kvm_send_ipi_all(int vector)
>  {
> -	__send_ipi_mask(cpu_online_mask, vector);
> +	if (static_branch_likely(&apic_use_ipi_shorthand))
> +		orig_apic.send_IPI_all(vector);
> +	else
> +		__send_ipi_mask(cpu_online_mask, vector);
>  }
>  
>  /*
> @@ -520,6 +528,8 @@ static void kvm_send_ipi_all(int vector)
>   */
>  static void kvm_setup_pv_ipi(void)
>  {
> +	orig_apic = *apic;
> +
>  	apic->send_IPI_mask = kvm_send_ipi_mask;
>  	apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
>  	apic->send_IPI_allbutself = kvm_send_ipi_allbutself;
> -- 
> 2.7.4
>
Wanpeng Li July 30, 2019, 2:17 a.m. UTC | #2
On Tue, 30 Jul 2019 at 01:50, Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Fri, Jul 26, 2019 at 02:15:27PM +0800, Wanpeng Li wrote:
> > From: Wanpeng Li <wanpengli@tencent.com>
> >
> > IPI shorthand is supported now by linux apic/x2apic driver, switch to
> > IPI shorthand for all excluding self and all including self destination
> > shorthand in kvm guest, to avoid splitting the target mask into serveral
> > PV IPI hypercalls.
> >
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Radim Krčmář <rkrcmar@redhat.com>
> > Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> > Cc: Nadav Amit <namit@vmware.com>
> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> > ---
> > Note: rebase against tip tree's x86/apic branch
> >
> >  arch/x86/kernel/kvm.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> > index b7f34fe..87b73b8 100644
> > --- a/arch/x86/kernel/kvm.c
> > +++ b/arch/x86/kernel/kvm.c
> > @@ -34,7 +34,9 @@
> >  #include <asm/hypervisor.h>
> >  #include <asm/tlb.h>
> >
> > +static struct apic orig_apic;
>
> Copying the entire struct apic to snapshot two functions is funky,
> explicitly capturing the two functions would be more intuitive.
>
> Tangentially related, kvm_setup_pv_ipi() can be annotated __init,
> which means the function snapshots can be __ro_after_init.
>
> That being said, can't we just remove kvm_send_ipi_all() and
> kvm_send_ipi_allbutself()?  AFAICT, the variations that lead to shorthand
> are only used when apic_use_ipi_shorthand is true.  If that isn't the
> case, fixing the callers in the APIC code seems like the correct thing to
> do.

The callers in the APIC codes have already done this, so I just remove
the two hooks in v2.

Regards,
Wanpeng Li
diff mbox series

Patch

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index b7f34fe..87b73b8 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -34,7 +34,9 @@ 
 #include <asm/hypervisor.h>
 #include <asm/tlb.h>
 
+static struct apic orig_apic;
 static int kvmapf = 1;
+DECLARE_STATIC_KEY_FALSE(apic_use_ipi_shorthand);
 
 static int __init parse_no_kvmapf(char *arg)
 {
@@ -507,12 +509,18 @@  static void kvm_send_ipi_mask_allbutself(const struct cpumask *mask, int vector)
 
 static void kvm_send_ipi_allbutself(int vector)
 {
-	kvm_send_ipi_mask_allbutself(cpu_online_mask, vector);
+	if (static_branch_likely(&apic_use_ipi_shorthand))
+		orig_apic.send_IPI_allbutself(vector);
+	else
+		kvm_send_ipi_mask_allbutself(cpu_online_mask, vector);
 }
 
 static void kvm_send_ipi_all(int vector)
 {
-	__send_ipi_mask(cpu_online_mask, vector);
+	if (static_branch_likely(&apic_use_ipi_shorthand))
+		orig_apic.send_IPI_all(vector);
+	else
+		__send_ipi_mask(cpu_online_mask, vector);
 }
 
 /*
@@ -520,6 +528,8 @@  static void kvm_send_ipi_all(int vector)
  */
 static void kvm_setup_pv_ipi(void)
 {
+	orig_apic = *apic;
+
 	apic->send_IPI_mask = kvm_send_ipi_mask;
 	apic->send_IPI_mask_allbutself = kvm_send_ipi_mask_allbutself;
 	apic->send_IPI_allbutself = kvm_send_ipi_allbutself;