Re: [PATCH 03/12] evtchn delivery on HVM

[Jeremy Fitzhardinge]

On 05/18/2010 03:22 AM, Stefano Stabellini wrote:
> From: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
>
> Set the callback to receive evtchns from Xen, using the
> callback vector delivery mechanism.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
> Signed-off-by: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
> ---
>  arch/x86/xen/enlighten.c         |   35 +++++++++++++++++++++++++++++++++++
>  drivers/xen/events.c             |   31 ++++++++++++++++++++++++-------
>  include/xen/events.h             |    3 +++
>  include/xen/hvm.h                |    9 +++++++++
>  include/xen/interface/features.h |    3 +++
>  5 files changed, 74 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
> index 87a3b10..502c4f8 100644
> --- a/arch/x86/xen/enlighten.c
> +++ b/arch/x86/xen/enlighten.c
> @@ -37,8 +37,11 @@
>  #include <xen/interface/vcpu.h>
>  #include <xen/interface/memory.h>
>  #include <xen/interface/hvm/hvm_op.h>
> +#include <xen/interface/hvm/params.h>
>  #include <xen/features.h>
>  #include <xen/page.h>
> +#include <xen/hvm.h>
> +#include <xen/events.h>
>  #include <xen/hvc-console.h>
>  
>  #include <asm/paravirt.h>
> @@ -79,6 +82,8 @@ struct shared_info xen_dummy_shared_info;
>  
>  void *xen_initial_gdt;
>  
> +int xen_have_vector_callback;
> +
>  /*
>   * Point at some empty memory to start with. We map the real shared_info
>   * page as soon as fixmap is up and running.
> @@ -1279,6 +1284,31 @@ static void __init init_shared_info(void)
>  	per_cpu(xen_vcpu, 0) = &HYPERVISOR_shared_info->vcpu_info[0];
>  }
>  
> +int xen_set_callback_via(uint64_t via)
> +{
> +	struct xen_hvm_param a;
> +	a.domid = DOMID_SELF;
> +	a.index = HVM_PARAM_CALLBACK_IRQ;
> +	a.value = via;
> +	return HYPERVISOR_hvm_op(HVMOP_set_param, &a);
>   

Does this implicitly set the vector delivery on all vcpus, current and
future?

> +}
> +
> +void do_hvm_pv_evtchn_intr(void)
> +{
> +	xen_hvm_evtchn_do_upcall(get_irq_regs());
> +}
> +
> +static void xen_callback_vector(void)
>   

All this callback vector stuff should be in drivers/xen/events.c.  It
would also be good to give it a more descriptive name
("xen_set_callback_vector"?), and make it an init function.

> +{
> +	uint64_t callback_via;
> +	if (xen_feature(XENFEAT_hvm_callback_vector)) {
> +		callback_via = HVM_CALLBACK_VECTOR(X86_PLATFORM_IPI_VECTOR);
> +		xen_set_callback_via(callback_via);
>   

Do you need to check the return value here?  Can it possibly fail?

> +		x86_platform_ipi_callback = do_hvm_pv_evtchn_intr;
> +		xen_have_vector_callback = 1;
> +	}
> +}
> +
>  void __init xen_guest_init(void)
>  {
>  	int r;
> @@ -1292,4 +1322,9 @@ void __init xen_guest_init(void)
>  		return;
>  
>  	init_shared_info();
> +
> +	xen_callback_vector();
> +
> +	have_vcpu_info_placement = 0;
> +	x86_init.irqs.intr_init = xen_init_IRQ;
>  }
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index db8f506..3523dbb 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -36,6 +36,8 @@
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
>  
> +#include <xen/xen.h>
> +#include <xen/hvm.h>
>  #include <xen/xen-ops.h>
>  #include <xen/events.h>
>  #include <xen/interface/xen.h>
> @@ -617,17 +619,13 @@ static DEFINE_PER_CPU(unsigned, xed_nesting_count);
>   * a bitset of words which contain pending event bits.  The second
>   * level is a bitset of pending events themselves.
>   */
> -void xen_evtchn_do_upcall(struct pt_regs *regs)
> +void __xen_evtchn_do_upcall(struct pt_regs *regs)
>   

Given that the regs arg is completely unused, you should drop it.

>  {
>  	int cpu = get_cpu();
> -	struct pt_regs *old_regs = set_irq_regs(regs);
>  	struct shared_info *s = HYPERVISOR_shared_info;
>  	struct vcpu_info *vcpu_info = __get_cpu_var(xen_vcpu);
>   	unsigned count;
>  
> -	exit_idle();
> -	irq_enter();
> -
>  	do {
>  		unsigned long pending_words;
>  
> @@ -667,10 +665,26 @@ void xen_evtchn_do_upcall(struct pt_regs *regs)
>  	} while(count != 1);
>  
>  out:
> +
> +	put_cpu();
> +}
> +
> +void xen_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	struct pt_regs *old_regs = set_irq_regs(regs);
> +
> +	exit_idle();
> +	irq_enter();
> +
> +	__xen_evtchn_do_upcall(regs);
> +
>  	irq_exit();
>  	set_irq_regs(old_regs);
> +}
>  
> -	put_cpu();
> +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	__xen_evtchn_do_upcall(regs);
>   

Don't you need to set_irq_regs here?

>  }
>  
>  /* Rebind a new event channel to an existing irq. */
> @@ -947,5 +961,8 @@ void __init xen_init_IRQ(void)
>  	for (i = 0; i < NR_EVENT_CHANNELS; i++)
>  		mask_evtchn(i);
>  
> -	irq_ctx_init(smp_processor_id());
> +	if (xen_hvm_domain())
> +		native_init_IRQ();
> +	else
> +		irq_ctx_init(smp_processor_id());
>  }
> diff --git a/include/xen/events.h b/include/xen/events.h
> index e68d59a..868e5d6 100644
> --- a/include/xen/events.h
> +++ b/include/xen/events.h
> @@ -56,4 +56,7 @@ void xen_poll_irq(int irq);
>  /* Determine the IRQ which is bound to an event channel */
>  unsigned irq_from_evtchn(unsigned int evtchn);
>  
> +void xen_evtchn_do_upcall(struct pt_regs *regs);
> +void xen_hvm_evtchn_do_upcall(struct pt_regs *regs);
> +
>  #endif	/* _XEN_EVENTS_H */
> diff --git a/include/xen/hvm.h b/include/xen/hvm.h
> index 6b0d418..5940ee5 100644
> --- a/include/xen/hvm.h
> +++ b/include/xen/hvm.h
> @@ -3,6 +3,7 @@
>  #define XEN_HVM_H__
>  
>  #include <xen/interface/hvm/params.h>
> +#include <asm/xen/hypercall.h>
>  
>  static inline int hvm_get_parameter(int idx, uint64_t *value)
>  {
> @@ -21,4 +22,12 @@ static inline int hvm_get_parameter(int idx, uint64_t *value)
>         return r;
>  }
>  
> +int xen_set_callback_via(uint64_t via);
> +extern int xen_have_vector_callback;
> +
> +#define HVM_CALLBACK_VIA_TYPE_VECTOR 0x2
> +#define HVM_CALLBACK_VIA_TYPE_SHIFT 56
> +#define HVM_CALLBACK_VECTOR(x) (((uint64_t)HVM_CALLBACK_VIA_TYPE_VECTOR)<<\
> +                               HVM_CALLBACK_VIA_TYPE_SHIFT | (x))
> +
>  #endif /* XEN_HVM_H__ */
> diff --git a/include/xen/interface/features.h b/include/xen/interface/features.h
> index f51b641..8ab08b9 100644
> --- a/include/xen/interface/features.h
> +++ b/include/xen/interface/features.h
> @@ -41,6 +41,9 @@
>  /* x86: Does this Xen host support the MMU_PT_UPDATE_PRESERVE_AD hypercall? */
>  #define XENFEAT_mmu_pt_update_preserve_ad  5
>  
> +/* x86: Does this Xen host support the HVM callback vector type? */
> +#define XENFEAT_hvm_callback_vector        8
> +
>  #define XENFEAT_NR_SUBMAPS 1
>  
>  #endif /* __XEN_PUBLIC_FEATURES_H__ */
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/