Re: [PATCH 1/3] irq: move IRQ routing into irq.c

[Will Deacon]

Hi Andre,

On Tue, Mar 01, 2016 at 04:49:36PM +0000, Andre Przywara wrote:
> The current IRQ routing code in x86/irq.c is mostly implementing a
> generic KVM interface which other architectures may use too.
> Move the code to set up an MSI route into the generic irq.c file and
> guard it with the KVM_CAP_IRQ_ROUTING capability to return an error
> if the kernel does not support interrupt routing.
> This also removes the dummy implementations for all other
> architectures and only leaves the x86 specific code in x86/irq.c.
> 
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> ---
>  Makefile          |  4 +--
>  arm/irq.c         |  9 ------
>  hw/pci-shmem.c    |  2 ++
>  include/kvm/irq.h |  5 ++++
>  irq.c             | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  mips/irq.c        | 10 -------
>  powerpc/irq.c     | 31 ---------------------
>  virtio/pci.c      | 18 ++++++++----
>  x86/irq.c         | 45 ++++--------------------------
>  9 files changed, 108 insertions(+), 99 deletions(-)
>  delete mode 100644 arm/irq.c
>  delete mode 100644 mips/irq.c
>  delete mode 100644 powerpc/irq.c

[...]

> diff --git a/x86/irq.c b/x86/irq.c
> index 72177e7..49b2e90 100644
> --- a/x86/irq.c
> +++ b/x86/irq.c
> @@ -11,20 +11,15 @@
>  #include <stddef.h>
>  #include <stdlib.h>
>  
> -#define IRQ_MAX_GSI			64
>  #define IRQCHIP_MASTER			0
>  #define IRQCHIP_SLAVE			1
>  #define IRQCHIP_IOAPIC			2
>  
> -/* First 24 GSIs are routed between IRQCHIPs and IOAPICs */
> -static u32 gsi = 24;
> -
> -struct kvm_irq_routing *irq_routing;
> -
>  static int irq__add_routing(u32 gsi, u32 type, u32 irqchip, u32 pin)
>  {
> -	if (gsi >= IRQ_MAX_GSI)
> -		return -ENOSPC;
> +	int r = irq__allocate_routing_entry();
> +	if (r)
> +		return r;
>  
>  	irq_routing->entries[irq_routing->nr++] =
>  		(struct kvm_irq_routing_entry) {
> @@ -41,11 +36,6 @@ int irq__init(struct kvm *kvm)
>  {
>  	int i, r;
>  
> -	irq_routing = calloc(sizeof(struct kvm_irq_routing) +
> -			IRQ_MAX_GSI * sizeof(struct kvm_irq_routing_entry), 1);
> -	if (irq_routing == NULL)
> -		return -ENOMEM;
> -
>  	/* Hook first 8 GSIs to master IRQCHIP */
>  	for (i = 0; i < 8; i++)
>  		if (i != 2)
> @@ -69,33 +59,8 @@ int irq__init(struct kvm *kvm)
>  		return errno;
>  	}
>  
> -	return 0;
> -}
> -dev_base_init(irq__init);
> +	next_gsi = 24;

Can we not just have an arch-specific initialiser that defaults to zero,
like we do for wired interrupts? (e.g. KVM_MSI_OFFSET). That way, we can
keep next_gsi private to the common irq routing code.

Will