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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160302003702.GD14022@arm.com>
Date:	Wed, 2 Mar 2016 00:37:02 +0000
From:	Will Deacon <will.deacon@....com>
To:	Andre Przywara <andre.przywara@....com>
Cc:	Sasha Levin <sasha.levin@...cle.com>,
	Pekka Enberg <penberg@...nel.org>, kvm@...r.kernel.org,
	kvmarm@...ts.cs.columbia.edu, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] irq: move IRQ routing into irq.c

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@....com>
> ---
>  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