linux-kernel - Re: [patch V2 43/46] genirq/msi: Provide and use msi_domain_set_default_info

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, 27 Aug 2020 09:17:13 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Joerg Roedel <joro@...tes.org>,
        iommu@...ts.linux-foundation.org, linux-hyperv@...r.kernel.org,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Jon Derrick <jonathan.derrick@...el.com>,
        Lu Baolu <baolu.lu@...ux.intel.com>,
        Wei Liu <wei.liu@...nel.org>,
        "K. Y. Srinivasan" <kys@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Steve Wahl <steve.wahl@....com>,
        Dimitri Sivanich <sivanich@....com>,
        Russ Anderson <rja@....com>, linux-pci@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
        xen-devel@...ts.xenproject.org, Juergen Gross <jgross@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Megha Dey <megha.dey@...el.com>,
        Jason Gunthorpe <jgg@...lanox.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Jacob Pan <jacob.jun.pan@...el.com>,
        Baolu Lu <baolu.lu@...el.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Dan Williams <dan.j.williams@...el.com>
Subject: Re: [patch V2 43/46] genirq/msi: Provide and use
 msi_domain_set_default_info_flags()

On 2020-08-26 12:17, Thomas Gleixner wrote:
> MSI interrupts have some common flags which should be set not only for
> PCI/MSI interrupts.
> 
> Move the PCI/MSI flag setting into a common function so it can be 
> reused.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
> V2: New patch
> ---
>  drivers/pci/msi.c   |    7 +------
>  include/linux/msi.h |    1 +
>  kernel/irq/msi.c    |   24 ++++++++++++++++++++++++
>  3 files changed, 26 insertions(+), 6 deletions(-)
> 
> --- a/drivers/pci/msi.c
> +++ b/drivers/pci/msi.c
> @@ -1469,12 +1469,7 @@ struct irq_domain *pci_msi_create_irq_do
>  	if (info->flags & MSI_FLAG_USE_DEF_CHIP_OPS)
>  		pci_msi_domain_update_chip_ops(info);
> 
> -	info->flags |= MSI_FLAG_ACTIVATE_EARLY;
> -	if (IS_ENABLED(CONFIG_GENERIC_IRQ_RESERVATION_MODE))
> -		info->flags |= MSI_FLAG_MUST_REACTIVATE;
> -
> -	/* PCI-MSI is oneshot-safe */
> -	info->chip->flags |= IRQCHIP_ONESHOT_SAFE;
> +	msi_domain_set_default_info_flags(info);
> 
>  	domain = msi_create_irq_domain(fwnode, info, parent);
>  	if (!domain)
> --- a/include/linux/msi.h
> +++ b/include/linux/msi.h
> @@ -410,6 +410,7 @@ int platform_msi_domain_alloc(struct irq
>  void platform_msi_domain_free(struct irq_domain *domain, unsigned int 
> virq,
>  			      unsigned int nvec);
>  void *platform_msi_get_host_data(struct irq_domain *domain);
> +void msi_domain_set_default_info_flags(struct msi_domain_info *info);
>  #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
> 
>  #ifdef CONFIG_PCI_MSI_IRQ_DOMAIN
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -70,6 +70,30 @@ void get_cached_msi_msg(unsigned int irq
>  EXPORT_SYMBOL_GPL(get_cached_msi_msg);
> 
>  #ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +void msi_domain_set_default_info_flags(struct msi_domain_info *info)
> +{
> +	/* Required so that a device latches a valid MSI message on startup 
> */
> +	info->flags |= MSI_FLAG_ACTIVATE_EARLY;

As far as I remember the story behind this flag (it's been a while),
it was working around a PCI-specific issue, hence being located in
the PCI code.

Now, the "program the MSI before enabling it" concept makes sense no 
matter
what bus this is on, and I wonder why we are even keeping this flag 
around.
Can't we just drop it together with the check in 
msi_domain_alloc_irqs()?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...