Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <1530598891-21370-1-git-send-email-wanpengli@tencent.com> <1530598891-21370-3-git-send-email-wanpengli@tencent.com>
In-Reply-To: <1530598891-21370-3-git-send-email-wanpengli@tencent.com>
From:   David Matlack <dmatlack@google.com>
Date:   Thu, 19 Jul 2018 16:05:14 -0700
Message-ID: <CALzav=fUmJZSvDgTvsB-7nyv1KiAMHgVcq-N+k0WCn3OsrVpNw@mail.gmail.com>
Subject: Re: [PATCH v3 2/6] KVM: X86: Implement PV IPIs in linux guest
To:     Wanpeng Li <kernellwp@gmail.com>
Cc:     linux-kernel@vger.kernel.org, kvm list <kvm@vger.kernel.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= <rkrcmar@redhat.com>,
        vkuznets@redhat.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, Jul 2, 2018 at 11:23 PM Wanpeng Li <kernellwp@gmail.com> wrote:
>
> From: Wanpeng Li <wanpengli@tencent.com>
>
> Implement paravirtual apic hooks to enable PV IPIs.

Very cool. Thanks for working on this!

>
> apic->send_IPI_mask
> apic->send_IPI_mask_allbutself
> apic->send_IPI_allbutself
> apic->send_IPI_all
>
> The PV IPIs supports maximal 128 vCPUs VM, it is big enough for cloud
> environment currently,

From the Cloud perspective, 128 vCPUs is already obsolete. GCE's
n1-utlramem-160 VMs have 160 vCPUs where the maximum APIC ID is 231.
I'd definitely prefer an approach that scales to higher APIC IDs, like
Paolo's offset idea.

To Radim's point of real world performance testing, do you know what
is the primary source of multi-target IPIs? If it's TLB shootdowns we
might get a bigger bang for our buck with a PV TLB Shootdown.

> supporting more vCPUs needs to introduce more
> complex logic, in the future this might be extended if needed.
>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Radim Kr=C4=8Dm=C3=A1=C5=99 <rkrcmar@redhat.com>
> Cc: Vitaly Kuznetsov <vkuznets@redhat.com>
> Signed-off-by: Wanpeng Li <wanpengli@tencent.com>
> ---
>  arch/x86/include/uapi/asm/kvm_para.h |  1 +
>  arch/x86/kernel/kvm.c                | 70 ++++++++++++++++++++++++++++++=
++++++
>  include/uapi/linux/kvm_para.h        |  1 +
>  3 files changed, 72 insertions(+)
>
> diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi=
/asm/kvm_para.h
> index 0ede697..19980ec 100644
> --- a/arch/x86/include/uapi/asm/kvm_para.h
> +++ b/arch/x86/include/uapi/asm/kvm_para.h
> @@ -28,6 +28,7 @@
>  #define KVM_FEATURE_PV_UNHALT          7
>  #define KVM_FEATURE_PV_TLB_FLUSH       9
>  #define KVM_FEATURE_ASYNC_PF_VMEXIT    10
> +#define KVM_FEATURE_PV_SEND_IPI        11
>
>  #define KVM_HINTS_REALTIME      0
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 591bcf2..2fe1420 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -454,6 +454,71 @@ static void __init sev_map_percpu_data(void)
>  }
>
>  #ifdef CONFIG_SMP
> +
> +#ifdef CONFIG_X86_64
> +static void __send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> +       unsigned long flags, ipi_bitmap_low =3D 0, ipi_bitmap_high =3D 0;
> +       int cpu, apic_id;
> +
> +       if (cpumask_empty(mask))
> +               return;
> +
> +       local_irq_save(flags);
> +
> +       for_each_cpu(cpu, mask) {
> +               apic_id =3D per_cpu(x86_cpu_to_apicid, cpu);
> +               if (apic_id < BITS_PER_LONG)
> +                       __set_bit(apic_id, &ipi_bitmap_low);
> +               else if (apic_id < 2 * BITS_PER_LONG)
> +                       __set_bit(apic_id - BITS_PER_LONG, &ipi_bitmap_hi=
gh);
> +       }
> +
> +       kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, =
vector);
> +
> +       local_irq_restore(flags);
> +}
> +
> +static void kvm_send_ipi_mask(const struct cpumask *mask, int vector)
> +{
> +       __send_ipi_mask(mask, vector);
> +}
> +
> +static void kvm_send_ipi_mask_allbutself(const struct cpumask *mask, int=
 vector)
> +{
> +       unsigned int this_cpu =3D smp_processor_id();
> +       struct cpumask new_mask;
> +       const struct cpumask *local_mask;
> +
> +       cpumask_copy(&new_mask, mask);
> +       cpumask_clear_cpu(this_cpu, &new_mask);
> +       local_mask =3D &new_mask;
> +       __send_ipi_mask(local_mask, vector);
> +}
> +
> +static void kvm_send_ipi_allbutself(int vector)
> +{
> +       kvm_send_ipi_mask_allbutself(cpu_online_mask, vector);
> +}
> +
> +static void kvm_send_ipi_all(int vector)
> +{
> +       __send_ipi_mask(cpu_online_mask, vector);
> +}
> +
> +/*
> + * Set the IPI entry points
> + */
> +static void kvm_setup_pv_ipi(void)
> +{
> +       apic->send_IPI_mask =3D kvm_send_ipi_mask;
> +       apic->send_IPI_mask_allbutself =3D kvm_send_ipi_mask_allbutself;
> +       apic->send_IPI_allbutself =3D kvm_send_ipi_allbutself;
> +       apic->send_IPI_all =3D kvm_send_ipi_all;
> +       pr_info("KVM setup pv IPIs\n");
> +}
> +#endif
> +
>  static void __init kvm_smp_prepare_cpus(unsigned int max_cpus)
>  {
>         native_smp_prepare_cpus(max_cpus);
> @@ -626,6 +691,11 @@ static uint32_t __init kvm_detect(void)
>
>  static void __init kvm_apic_init(void)
>  {
> +#if defined(CONFIG_SMP) && defined(CONFIG_X86_64)
> +       if (kvm_para_has_feature(KVM_FEATURE_PV_SEND_IPI) &&
> +               num_possible_cpus() <=3D 2 * BITS_PER_LONG)
> +               kvm_setup_pv_ipi();
> +#endif
>  }
>
>  static void __init kvm_init_platform(void)
> diff --git a/include/uapi/linux/kvm_para.h b/include/uapi/linux/kvm_para.=
h
> index dcf629d..84f8fe3 100644
> --- a/include/uapi/linux/kvm_para.h
> +++ b/include/uapi/linux/kvm_para.h
> @@ -26,6 +26,7 @@
>  #define KVM_HC_MIPS_EXIT_VM            7
>  #define KVM_HC_MIPS_CONSOLE_OUTPUT     8
>  #define KVM_HC_CLOCK_PAIRING           9
> +#define KVM_HC_SEND_IPI                10
>
>  /*
>   * hypercalls use architecture specific
> --
> 2.7.4
>