Re: [PATCH 1/5] x86/xen: Move Xen upcall handler

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 6585 bytes --]

On 13.03.25 19:22, Brian Gerst wrote:
> Move the upcall handler to Xen-specific files.
> 
> No functional changes.
> 
> Signed-off-by: Brian Gerst <brgerst@gmail.com>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> ---
>   arch/x86/entry/common.c     | 72 -------------------------------------
>   arch/x86/xen/enlighten_pv.c | 46 ++++++++++++++++++++++++
>   include/xen/xen-ops.h       | 19 ++++++++++
>   3 files changed, 65 insertions(+), 72 deletions(-)
> 
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 3514bf2978ee..ce4d88eda693 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -21,11 +21,6 @@
>   #include <linux/uaccess.h>
>   #include <linux/init.h>
>   
> -#ifdef CONFIG_XEN_PV
> -#include <xen/xen-ops.h>
> -#include <xen/events.h>
> -#endif
> -
>   #include <asm/apic.h>
>   #include <asm/desc.h>
>   #include <asm/traps.h>
> @@ -455,70 +450,3 @@ SYSCALL_DEFINE0(ni_syscall)
>   {
>   	return -ENOSYS;
>   }
> -
> -#ifdef CONFIG_XEN_PV
> -#ifndef CONFIG_PREEMPTION
> -/*
> - * Some hypercalls issued by the toolstack can take many 10s of
> - * seconds. Allow tasks running hypercalls via the privcmd driver to
> - * be voluntarily preempted even if full kernel preemption is
> - * disabled.
> - *
> - * Such preemptible hypercalls are bracketed by
> - * xen_preemptible_hcall_begin() and xen_preemptible_hcall_end()
> - * calls.
> - */
> -DEFINE_PER_CPU(bool, xen_in_preemptible_hcall);
> -EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
> -
> -/*
> - * In case of scheduling the flag must be cleared and restored after
> - * returning from schedule as the task might move to a different CPU.
> - */
> -static __always_inline bool get_and_clear_inhcall(void)
> -{
> -	bool inhcall = __this_cpu_read(xen_in_preemptible_hcall);
> -
> -	__this_cpu_write(xen_in_preemptible_hcall, false);
> -	return inhcall;
> -}
> -
> -static __always_inline void restore_inhcall(bool inhcall)
> -{
> -	__this_cpu_write(xen_in_preemptible_hcall, inhcall);
> -}
> -#else
> -static __always_inline bool get_and_clear_inhcall(void) { return false; }
> -static __always_inline void restore_inhcall(bool inhcall) { }
> -#endif
> -
> -static void __xen_pv_evtchn_do_upcall(struct pt_regs *regs)
> -{
> -	struct pt_regs *old_regs = set_irq_regs(regs);
> -
> -	inc_irq_stat(irq_hv_callback_count);
> -
> -	xen_evtchn_do_upcall();
> -
> -	set_irq_regs(old_regs);
> -}
> -
> -__visible noinstr void xen_pv_evtchn_do_upcall(struct pt_regs *regs)
> -{
> -	irqentry_state_t state = irqentry_enter(regs);
> -	bool inhcall;
> -
> -	instrumentation_begin();
> -	run_sysvec_on_irqstack_cond(__xen_pv_evtchn_do_upcall, regs);
> -
> -	inhcall = get_and_clear_inhcall();
> -	if (inhcall && !WARN_ON_ONCE(state.exit_rcu)) {
> -		irqentry_exit_cond_resched();
> -		instrumentation_end();
> -		restore_inhcall(inhcall);
> -	} else {
> -		instrumentation_end();
> -		irqentry_exit(regs, state);
> -	}
> -}
> -#endif /* CONFIG_XEN_PV */
> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
> index 5e57835e999d..af9e43c47b07 100644
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -73,6 +73,7 @@
>   #include <asm/mwait.h>
>   #include <asm/pci_x86.h>
>   #include <asm/cpu.h>
> +#include <asm/irq_stack.h>
>   #ifdef CONFIG_X86_IOPL_IOPERM
>   #include <asm/io_bitmap.h>
>   #endif
> @@ -94,6 +95,21 @@ void *xen_initial_gdt;
>   static int xen_cpu_up_prepare_pv(unsigned int cpu);
>   static int xen_cpu_dead_pv(unsigned int cpu);
>   
> +#ifndef CONFIG_PREEMPTION
> +/*
> + * Some hypercalls issued by the toolstack can take many 10s of
> + * seconds. Allow tasks running hypercalls via the privcmd driver to
> + * be voluntarily preempted even if full kernel preemption is
> + * disabled.
> + *
> + * Such preemptible hypercalls are bracketed by
> + * xen_preemptible_hcall_begin() and xen_preemptible_hcall_end()
> + * calls.
> + */
> +DEFINE_PER_CPU(bool, xen_in_preemptible_hcall);
> +EXPORT_SYMBOL_GPL(xen_in_preemptible_hcall);
> +#endif
> +
>   struct tls_descs {
>   	struct desc_struct desc[3];
>   };
> @@ -687,6 +703,36 @@ DEFINE_IDTENTRY_RAW(xenpv_exc_machine_check)
>   }
>   #endif
>   
> +static void __xen_pv_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	struct pt_regs *old_regs = set_irq_regs(regs);
> +
> +	inc_irq_stat(irq_hv_callback_count);
> +
> +	xen_evtchn_do_upcall();
> +
> +	set_irq_regs(old_regs);
> +}
> +
> +__visible noinstr void xen_pv_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	irqentry_state_t state = irqentry_enter(regs);
> +	bool inhcall;
> +
> +	instrumentation_begin();
> +	run_sysvec_on_irqstack_cond(__xen_pv_evtchn_do_upcall, regs);
> +
> +	inhcall = get_and_clear_inhcall();
> +	if (inhcall && !WARN_ON_ONCE(state.exit_rcu)) {
> +		irqentry_exit_cond_resched();
> +		instrumentation_end();
> +		restore_inhcall(inhcall);
> +	} else {
> +		instrumentation_end();
> +		irqentry_exit(regs, state);
> +	}
> +}
> +
>   struct trap_array_entry {
>   	void (*orig)(void);
>   	void (*xen)(void);
> diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
> index 47f11bec5e90..174ef8e4600f 100644
> --- a/include/xen/xen-ops.h
> +++ b/include/xen/xen-ops.h
> @@ -208,10 +208,29 @@ static inline void xen_preemptible_hcall_end(void)
>   	__this_cpu_write(xen_in_preemptible_hcall, false);
>   }
>   
> +/*
> + * In case of scheduling the flag must be cleared and restored after
> + * returning from schedule as the task might move to a different CPU.
> + */
> +static __always_inline bool get_and_clear_inhcall(void)
> +{
> +	bool inhcall = __this_cpu_read(xen_in_preemptible_hcall);
> +
> +	__this_cpu_write(xen_in_preemptible_hcall, false);
> +	return inhcall;
> +}
> +
> +static __always_inline void restore_inhcall(bool inhcall)
> +{
> +	__this_cpu_write(xen_in_preemptible_hcall, inhcall);
> +}
> +
>   #else
>   
>   static inline void xen_preemptible_hcall_begin(void) { }
>   static inline void xen_preemptible_hcall_end(void) { }
> +static __always_inline bool get_and_clear_inhcall(void) { return false; }
> +static __always_inline void restore_inhcall(bool inhcall) { }
>   
>   #endif /* CONFIG_XEN_PV && !CONFIG_PREEMPTION */
>   

I don't see a reason to put those two functions into xen_ops.h, as
they are used by xen_pv_evtchn_do_upcall() only.

Please move them to enlighten_pv.c, too.


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3743 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]