Re: [PATCH v2] x86/xen: Add support for HVMOP_set_evtchn_upcall_vector

From: Jane Malalane
Date: Tue Jul 26 2022 - 09:23:39 EST

On 26/07/2022 13:56, Jane Malalane wrote:
> Implement support for the HVMOP_set_evtchn_upcall_vector hypercall in
> order to set the per-vCPU event channel vector callback on Linux and
> use it in preference of HVM_PARAM_CALLBACK_IRQ.
>
> If the per-VCPU vector setup is successful on BSP, use this method
> for the APs. If not, fallback to the global vector-type callback.
>
> Also register callback_irq at per-vCPU event channel setup to trick
> toolstack to think the domain is enlightened.
>
> Suggested-by: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> Signed-off-by: Jane Malalane <jane.malalane@xxxxxxxxxx>
> ---
> CC: Juergen Gross <jgross@xxxxxxxx>
> CC: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> CC: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> CC: Ingo Molnar <mingo@xxxxxxxxxx>
> CC: Borislav Petkov <bp@xxxxxxxxx>
> CC: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> CC: x86@xxxxxxxxxx
> CC: "H. Peter Anvin" <hpa@xxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Oleksandr Tyshchenko <oleksandr_tyshchenko@xxxxxxxx>
> CC: Jane Malalane <jane.malalane@xxxxxxxxxx>
> CC: Maximilian Heyne <mheyne@xxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Colin Ian King <colin.king@xxxxxxxxx>
> CC: xen-devel@xxxxxxxxxxxxxxxxxxxx
>
> v2:
> * remove no_vector_callback
> * make xen_have_vector_callback a bool
> * rename xen_ack_upcall to xen_percpu_upcall
> * fail to bring CPU up on init instead of crashing kernel
> * add and use xen_set_upcall_vector where suitable
> * xen_setup_upcall_vector -> xen_init_setup_upcall_vector for clarity
> ---
> arch/x86/include/asm/xen/cpuid.h | 2 ++
> arch/x86/include/asm/xen/events.h | 3 ++-
> arch/x86/xen/enlighten.c | 2 +-
> arch/x86/xen/enlighten_hvm.c | 24 ++++++++++++++-----
> arch/x86/xen/suspend_hvm.c | 10 +++++++-
> drivers/xen/events/events_base.c | 49 ++++++++++++++++++++++++++++++++++----
> include/xen/hvm.h | 2 ++
> include/xen/interface/hvm/hvm_op.h | 15 ++++++++++++
> 8 files changed, 93 insertions(+), 14 deletions(-)
>
> diff --git a/arch/x86/include/asm/xen/cpuid.h b/arch/x86/include/asm/xen/cpuid.h
> index 78e667a31d6c..6daa9b0c8d11 100644
> --- a/arch/x86/include/asm/xen/cpuid.h
> +++ b/arch/x86/include/asm/xen/cpuid.h
> @@ -107,6 +107,8 @@
> * ID field from 8 to 15 bits, allowing to target APIC IDs up 32768.
> */
> #define XEN_HVM_CPUID_EXT_DEST_ID (1u << 5)
> +/* Per-vCPU event channel upcalls */
> +#define XEN_HVM_CPUID_UPCALL_VECTOR (1u << 6)
>
> /*
> * Leaf 6 (0x40000x05)
> diff --git a/arch/x86/include/asm/xen/events.h b/arch/x86/include/asm/xen/events.h
> index 068d9b067c83..62bdceb594f1 100644
> --- a/arch/x86/include/asm/xen/events.h
> +++ b/arch/x86/include/asm/xen/events.h
> @@ -23,7 +23,7 @@ static inline int xen_irqs_disabled(struct pt_regs *regs)
> /* No need for a barrier -- XCHG is a barrier on x86. */
> #define xchg_xen_ulong(ptr, val) xchg((ptr), (val))
>
> -extern int xen_have_vector_callback;
> +extern bool xen_have_vector_callback;
>
> /*
> * Events delivered via platform PCI interrupts are always
> @@ -34,4 +34,5 @@ static inline bool xen_support_evtchn_rebind(void)
> return (!xen_hvm_domain() || xen_have_vector_callback);
> }
>
> +extern bool xen_percpu_upcall;
> #endif /* _ASM_X86_XEN_EVENTS_H */
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 30c6e986a6cd..b8db2148c07d 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -51,7 +51,7 @@ EXPORT_SYMBOL_GPL(xen_start_info);
>
> struct shared_info xen_dummy_shared_info;
>
> -__read_mostly int xen_have_vector_callback;
> +__read_mostly bool xen_have_vector_callback = true;
> EXPORT_SYMBOL_GPL(xen_have_vector_callback);
>
> /*
> diff --git a/arch/x86/xen/enlighten_hvm.c b/arch/x86/xen/enlighten_hvm.c
> index 8b71b1dd7639..198d3cd3e9a5 100644
> --- a/arch/x86/xen/enlighten_hvm.c
> +++ b/arch/x86/xen/enlighten_hvm.c
> @@ -7,6 +7,8 @@
>
> #include <xen/features.h>
> #include <xen/events.h>
> +#include <xen/hvm.h>
> +#include <xen/interface/hvm/hvm_op.h>
> #include <xen/interface/memory.h>
>
> #include <asm/apic.h>
> @@ -30,6 +32,9 @@
>
> static unsigned long shared_info_pfn;
>
> +__ro_after_init bool xen_percpu_upcall;
> +EXPORT_SYMBOL_GPL(xen_percpu_upcall);
> +
> void xen_hvm_init_shared_info(void)
> {
> struct xen_add_to_physmap xatp;
> @@ -125,6 +130,9 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_xen_hvm_callback)
> {
> struct pt_regs *old_regs = set_irq_regs(regs);
>
> + if (xen_percpu_upcall)
> + ack_APIC_irq();
> +
> inc_irq_stat(irq_hv_callback_count);
>
> xen_hvm_evtchn_do_upcall();
> @@ -168,6 +176,15 @@ static int xen_cpu_up_prepare_hvm(unsigned int cpu)
> if (!xen_have_vector_callback)
> return 0;
>
> + if (xen_percpu_upcall) {
> + rc = xen_set_upcall_vector(cpu);
> + if (rc) {
> + WARN(1, "HVMOP_set_evtchn_upcall_vector"
> + " for CPU %d failed: %d\n", cpu, rc);
> + return rc;
> + }
> + }
> +
> if (xen_feature(XENFEAT_hvm_safe_pvclock))
> xen_setup_timer(cpu);
>
> @@ -188,8 +205,6 @@ static int xen_cpu_dead_hvm(unsigned int cpu)
> return 0;
> }
>
> -static bool no_vector_callback __initdata;
> -
> static void __init xen_hvm_guest_init(void)
> {
> if (xen_pv_domain())
> @@ -211,9 +226,6 @@ static void __init xen_hvm_guest_init(void)
>
> xen_panic_handler_init();
>
> - if (!no_vector_callback && xen_feature(XENFEAT_hvm_callback_vector))
> - xen_have_vector_callback = 1;
> -
> xen_hvm_smp_init();
> WARN_ON(xen_cpuhp_setup(xen_cpu_up_prepare_hvm, xen_cpu_dead_hvm));
> xen_unplug_emulated_devices();
> @@ -239,7 +251,7 @@ early_param("xen_nopv", xen_parse_nopv);
>
> static __init int xen_parse_no_vector_callback(char *arg)
> {
> - no_vector_callback = true;
> + xen_have_vector_callback = false;
> return 0;
> }
> early_param("xen_no_vector_callback", xen_parse_no_vector_callback);
> diff --git a/arch/x86/xen/suspend_hvm.c b/arch/x86/xen/suspend_hvm.c
> index 9d548b0c772f..0c4f7554b7cc 100644
> --- a/arch/x86/xen/suspend_hvm.c
> +++ b/arch/x86/xen/suspend_hvm.c
> @@ -5,6 +5,7 @@
> #include <xen/hvm.h>
> #include <xen/features.h>
> #include <xen/interface/features.h>
> +#include <xen/events.h>
>
> #include "xen-ops.h"
>
> @@ -14,6 +15,13 @@ void xen_hvm_post_suspend(int suspend_cancelled)
> xen_hvm_init_shared_info();
> xen_vcpu_restore();
> }
> - xen_setup_callback_vector();
> + if (xen_percpu_upcall) {
> + unsigned int cpu;
> +
> + for_each_online_cpu(cpu)
> + BUG_ON(xen_set_upcall_vector(cpu));
> + } else {
> + xen_setup_callback_vector();
> + }
> xen_unplug_emulated_devices();
> }
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 46d9295d9a6e..2ad93595d03a 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -48,6 +48,7 @@
> #include <asm/xen/pci.h>
> #endif
> #include <asm/sync_bitops.h>
> +#include <asm/xen/cpuid.h>
> #include <asm/xen/hypercall.h>
> #include <asm/xen/hypervisor.h>
> #include <xen/page.h>
> @@ -2195,11 +2196,48 @@ void xen_setup_callback_vector(void)
> callback_via = HVM_CALLBACK_VECTOR(HYPERVISOR_CALLBACK_VECTOR);
> if (xen_set_callback_via(callback_via)) {
> pr_err("Request for Xen HVM callback vector failed\n");
> - xen_have_vector_callback = 0;
> + xen_have_vector_callback = false;
> }
> }
> }
>
> +/* Setup per-vCPU vector-type callbacks and trick toolstack to think
> + * we are enlightened. If this setup is unavailable, fallback to the
> + * global vector-type callback. */
> +static __init void xen_init_setup_upcall_vector(void)
> +{
> + unsigned int cpu = 0;
> +
> + if (!xen_have_vector_callback)
> + return;
> +
> + if ((cpuid_eax(xen_cpuid_base() + 4) & XEN_HVM_CPUID_UPCALL_VECTOR) &&
> + !xen_set_upcall_vector(cpu) && !xen_set_callback_via(1))
I should have removed xen_set_callback_via(1) here. Will send another
version, would just like to wait for any comments before I do.

Jane.