Re: [PATCH v2 2/2] linux/xen: support pirq_eoi_map

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Jan 26, 2012 at 06:00:39PM +0000, Stefano Stabellini wrote:
> The pirq_eoi_map is a bitmap offered by Xen to check which pirqs need to
> be EOI'd without having to issue an hypercall every time.
> We use PHYSDEVOP_pirq_eoi_gmfn (v2) to map the bitmap, then, if we
> succeed, we use pirq_eoi_map to check whether pirqs need eoi.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/xen/events.c            |   17 ++++++++++++++++-
>  include/xen/interface/physdev.h |   12 ++++++++++++
>  2 files changed, 28 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/xen/events.c b/drivers/xen/events.c
> index 6e075cd..7fdc738 100644
> --- a/drivers/xen/events.c
> +++ b/drivers/xen/events.c
> @@ -37,6 +37,7 @@
>  #include <asm/idle.h>
>  #include <asm/io_apic.h>
>  #include <asm/sync_bitops.h>
> +#include <asm/xen/page.h>
>  #include <asm/xen/pci.h>
>  #include <asm/xen/hypercall.h>
>  #include <asm/xen/hypervisor.h>
> @@ -108,6 +109,7 @@ struct irq_info {
>  #define PIRQ_SHAREABLE	(1 << 1)
>  
>  static int *evtchn_to_irq;
> +static unsigned long *pirq_eoi_map;
>  
>  static DEFINE_PER_CPU(unsigned long [NR_EVENT_CHANNELS/BITS_PER_LONG],
>  		      cpu_evtchn_mask);
> @@ -274,6 +276,9 @@ static bool pirq_needs_eoi(unsigned irq)
>  
>  	BUG_ON(info->type != IRQT_PIRQ);
>  
> +	if (pirq_eoi_map != NULL)
> +		return test_bit(irq, pirq_eoi_map);
> +

How about just having a different function called
pirq_needs_eoi_v2 which will just do

 return test_bit(irq, pirq_eoi_map)?

And then set the pirq_needs_eoi_v2 in the function table?


>  	return info->u.pirq.flags & PIRQ_NEEDS_EOI;
>  }
>  
> @@ -1693,7 +1698,7 @@ void xen_callback_vector(void) {}
>  
>  void __init xen_init_IRQ(void)
>  {
> -	int i;
> +	int i, rc;
>  
>  	evtchn_to_irq = kcalloc(NR_EVENT_CHANNELS, sizeof(*evtchn_to_irq),
>  				    GFP_KERNEL);
> @@ -1714,8 +1719,18 @@ void __init xen_init_IRQ(void)
>  		 * __acpi_register_gsi can point at the right function */
>  		pci_xen_hvm_init();
>  	} else {
> +		struct physdev_pirq_eoi_gmfn eoi_gmfn;
> +
>  		irq_ctx_init(smp_processor_id());
>  		if (xen_initial_domain())
>  			pci_xen_initial_domain();
> +
> +		pirq_eoi_map = (void *)__get_free_page(GFP_KERNEL|__GFP_ZERO);
> +		eoi_gmfn.gmfn = virt_to_mfn(pirq_eoi_map);
> +		rc = HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn, &eoi_gmfn);


Don't we want the v2 version?

> +		if (rc != 0) {
> +			free_page((unsigned long) pirq_eoi_map);
> +			pirq_eoi_map = NULL;
> +		}
>  	}
>  }
> diff --git a/include/xen/interface/physdev.h b/include/xen/interface/physdev.h
> index c1080d9..132c61f 100644
> --- a/include/xen/interface/physdev.h
> +++ b/include/xen/interface/physdev.h
> @@ -39,6 +39,18 @@ struct physdev_eoi {
>  };
>  
>  /*
> + * Register a shared page for the hypervisor to indicate whether the
> + * guest must issue PHYSDEVOP_eoi. The page registered is used as a bit
> + * array indexed by Xen's PIRQ value.
> + */
> +#define PHYSDEVOP_pirq_eoi_gmfn      28

Ah, the number is right, but the name is the generic one.

We should really mention that this is different from v1 or just

#define PHYSDEVOP_pirq_eoi_gmfn_v2 28
and use that in the code?

> +struct physdev_pirq_eoi_gmfn {
> +    /* IN */
> +    unsigned long gmfn;
> +};
> +
> +
> +/*
>   * Query the status of an IRQ line.
>   * @arg == pointer to physdev_irq_status_query structure.
>   */
> -- 
> 1.7.2.5