linux-kernel - Re: [Patch V2 12/13] irqchip: Add IMS (Interrupt Message Store) driver

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 for Android: free password hash cracker in your pocket

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

Message-ID: <87zgyrqgbm.wl-maz@kernel.org>
Date:   Thu, 25 Mar 2021 17:43:57 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Megha Dey <megha.dey@...el.com>
Cc:     tglx@...utronix.de, linux-kernel@...r.kernel.org,
        dave.jiang@...el.com, ashok.raj@...el.com, kevin.tian@...el.com,
        dwmw@...zon.co.uk, x86@...nel.org, tony.luck@...el.com,
        dan.j.williams@...el.com, jgg@...lanox.com, kvm@...r.kernel.org,
        iommu@...ts.linux-foundation.org, alex.williamson@...hat.com,
        bhelgaas@...gle.com, linux-pci@...r.kernel.org,
        baolu.lu@...ux.intel.com, ravi.v.shankar@...el.com
Subject: Re: [Patch V2 12/13] irqchip: Add IMS (Interrupt Message Store) driver

On Fri, 26 Feb 2021 20:11:16 +0000,
Megha Dey <megha.dey@...el.com> wrote:
> 
> Generic IMS(Interrupt Message Store) irq chips and irq domain
> implementations for IMS based devices which store the interrupt messages
> in an array in device memory.
> 
> Allocation and freeing of interrupts happens via the generic
> msi_domain_alloc/free_irqs() interface. No special purpose IMS magic
> required as long as the interrupt domain is stored in the underlying
> device struct. The irq_set_auxdata() is used to program the pasid into
> the IMS entry.
> 
> [Megha: Fixed compile time errors
>         Added necessary dependencies to IMS_MSI_ARRAY config
>         Fixed polarity of IMS_VECTOR_CTRL
>         Added reads after writes to flush writes to device
>         Added set_desc ops to IMS msi domain ops
>         Tested the IMS infrastructure with the IDXD driver]
> 
> Reviewed-by: Tony Luck <tony.luck@...el.com>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Signed-off-by: Megha Dey <megha.dey@...el.com>
> ---
>  drivers/irqchip/Kconfig             |  14 +++
>  drivers/irqchip/Makefile            |   1 +
>  drivers/irqchip/irq-ims-msi.c       | 211 ++++++++++++++++++++++++++++++++++++
>  include/linux/irqchip/irq-ims-msi.h |  68 ++++++++++++
>  4 files changed, 294 insertions(+)
>  create mode 100644 drivers/irqchip/irq-ims-msi.c
>  create mode 100644 include/linux/irqchip/irq-ims-msi.h
> 
> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> index e74fa20..2fb0c24 100644
> --- a/drivers/irqchip/Kconfig
> +++ b/drivers/irqchip/Kconfig
> @@ -586,4 +586,18 @@ config MST_IRQ
>  	help
>  	  Support MStar Interrupt Controller.
>  
> +config IMS_MSI
> +	depends on PCI
> +	select DEVICE_MSI
> +	bool
> +
> +config IMS_MSI_ARRAY
> +	bool "IMS Interrupt Message Store MSI controller for device memory storage arrays"
> +	depends on PCI
> +	select IMS_MSI
> +	select GENERIC_MSI_IRQ_DOMAIN
> +	help
> +	  Support for IMS Interrupt Message Store MSI controller
> +	  with IMS slot storage in a slot array in device memory
> +
>  endmenu
> diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> index c59b95a..e903201 100644
> --- a/drivers/irqchip/Makefile
> +++ b/drivers/irqchip/Makefile
> @@ -113,3 +113,4 @@ obj-$(CONFIG_LOONGSON_PCH_MSI)		+= irq-loongson-pch-msi.o
>  obj-$(CONFIG_MST_IRQ)			+= irq-mst-intc.o
>  obj-$(CONFIG_SL28CPLD_INTC)		+= irq-sl28cpld.o
>  obj-$(CONFIG_MACH_REALTEK_RTL)		+= irq-realtek-rtl.o
> +obj-$(CONFIG_IMS_MSI)			+= irq-ims-msi.o
> diff --git a/drivers/irqchip/irq-ims-msi.c b/drivers/irqchip/irq-ims-msi.c
> new file mode 100644
> index 0000000..fa23207
> --- /dev/null
> +++ b/drivers/irqchip/irq-ims-msi.c
> @@ -0,0 +1,211 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// (C) Copyright 2021 Thomas Gleixner <tglx@...utronix.de>
> +/*
> + * Shared interrupt chips and irq domains for IMS devices
> + */
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/msi.h>
> +#include <linux/irq.h>
> +#include <linux/irqdomain.h>
> +
> +#include <linux/irqchip/irq-ims-msi.h>
> +
> +#ifdef CONFIG_IMS_MSI_ARRAY

Given that this covers the whole driver, what is this #defined used
for? You might as well make the driver depend on this config option.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.