From: Wanpeng Li <[email protected]>
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 <[email protected]>
Cc: Paolo Bonzini <[email protected]>
Cc: Radim Krčmář <[email protected]>
Cc: Sean Christopherson <[email protected]>
Cc: Nadav Amit <[email protected]>
Signed-off-by: Wanpeng Li <[email protected]>
---
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;
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
On Fri, Jul 26, 2019 at 02:15:27PM +0800, Wanpeng Li wrote:
> From: Wanpeng Li <[email protected]>
>
> 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 <[email protected]>
> Cc: Paolo Bonzini <[email protected]>
> Cc: Radim Krčmář <[email protected]>
> Cc: Sean Christopherson <[email protected]>
> Cc: Nadav Amit <[email protected]>
> Signed-off-by: Wanpeng Li <[email protected]>
> ---
> 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
>
On Tue, 30 Jul 2019 at 01:50, Sean Christopherson
<[email protected]> wrote:
>
> On Fri, Jul 26, 2019 at 02:15:27PM +0800, Wanpeng Li wrote:
> > From: Wanpeng Li <[email protected]>
> >
> > 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 <[email protected]>
> > Cc: Paolo Bonzini <[email protected]>
> > Cc: Radim Krčmář <[email protected]>
> > Cc: Sean Christopherson <[email protected]>
> > Cc: Nadav Amit <[email protected]>
> > Signed-off-by: Wanpeng Li <[email protected]>
> > ---
> > 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