linux-kernel - Re: [Patch V2 07/13] irqdomain/msi: Provide msi_alloc/free

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]

Date:   Thu, 25 Mar 2021 17:08:13 +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 07/13] irqdomain/msi: Provide msi_alloc/free_store() callbacks

On Fri, 26 Feb 2021 20:11:11 +0000,
Megha Dey <megha.dey@...el.com> wrote:
> 
> From: Thomas Gleixner <tglx@...utronix.de>
> 
> For devices which don't have a standard storage for MSI messages like the
> upcoming IMS (Interrupt Message Store) it's required to allocate storage
> space before allocating interrupts and after freeing them.
> 
> This could be achieved with the existing callbacks, but that would be
> awkward because they operate on msi_alloc_info_t which is not uniform
> across architectures. Also these callbacks are invoked per interrupt but
> the allocation might have bulk requirements depending on the device.
> 
> As such devices can operate on different architectures it is simpler to
> have separate callbacks which operate on struct device. The resulting
> storage information has to be stored in struct msi_desc so the underlying
> irq chip implementation can retrieve it for the relevant operations.
> 
> 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>
> ---
>  include/linux/msi.h |  8 ++++++++
>  kernel/irq/msi.c    | 11 +++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/include/linux/msi.h b/include/linux/msi.h
> index 46e879c..e915932 100644
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -323,6 +323,10 @@ struct msi_domain_info;
>   *			function.
>   * @domain_free_irqs:	Optional function to override the default free
>   *			function.
> + * @msi_alloc_store:	Optional callback to allocate storage in a device
> + *			specific non-standard MSI store
> + * @msi_alloc_free:	Optional callback to free storage in a device
> + *			specific non-standard MSI store
>   *
>   * @get_hwirq, @msi_init and @msi_free are callbacks used by
>   * msi_create_irq_domain() and related interfaces
> @@ -372,6 +376,10 @@ struct msi_domain_ops {
>  					     struct device *dev, int nvec);
>  	void		(*domain_free_irqs)(struct irq_domain *domain,
>  					    struct device *dev);
> +	int		(*msi_alloc_store)(struct irq_domain *domain,
> +					   struct device *dev, int nvec);
> +	void		(*msi_free_store)(struct irq_domain *domain,
> +					  struct device *dev);
>  };
>  
>  /**
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index c54316d..047b59d 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -434,6 +434,12 @@ int __msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev,
>  	if (ret)
>  		return ret;
>  
> +	if (ops->msi_alloc_store) {
> +		ret = ops->msi_alloc_store(domain, dev, nvec);

What is supposed to happen if we get aliasing devices (similar to what
we have with devices behind a PCI bridge)?

The ITS code goes through all kind of hoops to try and detect this
case when sizing the translation tables (in the .prepare callback),
and I have the feeling that sizing the message store is analogous.

Or do we all have the warm fuzzy feeling that aliasing is a thing of
the past and that we can ignore this potential problem?

Thanks,

	M.

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