linux-kernel - Re: [PATCH 2/4] PCI/MSI: Add startup/shutdown support for per device MSI[X] domains

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: <hjummqbdurgohpv2kvrnr23abirami642vrpvs4j272yhw5hna@k3rqwvzax5no>
Date: Fri, 8 Aug 2025 07:18:48 +0800
From: Inochi Amaoto <inochiama@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>, 
	Inochi Amaoto <inochiama@...il.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Marc Zyngier <maz@...nel.org>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Saurabh Sengar <ssengar@...ux.microsoft.com>, 
	Shradha Gupta <shradhagupta@...ux.microsoft.com>, Jonathan Cameron <Jonathan.Cameron@...ei.com>, 
	Nicolin Chen <nicolinc@...dia.com>, Jason Gunthorpe <jgg@...pe.ca>, 
	Chen Wang <unicorn_wang@...look.com>, linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org, 
	Yixun Lan <dlan@...too.org>, Longbin Li <looong.bin@...il.com>
Subject: Re: [PATCH 2/4] PCI/MSI: Add startup/shutdown support for per device
 MSI[X] domains

On Thu, Aug 07, 2025 at 11:25:21AM -0500, Bjorn Helgaas wrote:
> In subject, s/MSI[X]// and s/support for/for/
> 
> The "MSI[X]" notation really isn't used anywhere else, and we already
> include "PCI/MSI" in the prefix, so I don't think we need it again.
> 
> On Thu, Aug 07, 2025 at 07:23:23PM +0800, Inochi Amaoto wrote:
> > As The RISC-V PLIC can not apply affinity setting without calling
> > irq_enable(), it will make the interrupt unavaible when using as
> > an underlying irq chip for MSI controller.
> 
> s/As The/As the/
> s/unavaible/unavailable/
> s/irq chip/IRQ chip/
> 

These are good for me. I will take it.

> > Introduce the irq_startup/irq_shutdown for PCI domain template with
> > new MSI domain flag. This allow the PLIC can be properly configurated
> > when calling irq_startup().
> 
> Maybe something like:
> 
>   Implement .irq_startup() and .irq_shutdown() for the PCI MSI and
>   MSI-X templates.  For chips that specify MSI_FLAG_PCI_MSI_STARTUP_PARENT, 
>   these startup and shutdown the parent as well, which allows ...
> 
> s/This allow/This allows/
> s/can be properly configurated/to be configured/
> 

Thanks, I will update my commit log.

> Evidently PLIC depends on this "parent" connection, but that isn't
> explained at all in the commit log.
> 

When call irq_startup, the PLIC is called irq_enable() instead of 
irq_unmask(), the the irq on PLIC can be enabled.

I will add this to the commit log.

> > Suggested-by: Thomas Gleixner <tglx@...utronix.de>
> > Signed-off-by: Inochi Amaoto <inochiama@...il.com>
> > ---
> >  drivers/pci/msi/irqdomain.c | 52 +++++++++++++++++++++++++++++++++++++
> >  include/linux/msi.h         |  2 ++
> >  2 files changed, 54 insertions(+)
> > 
> > diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
> > index 0938ef7ebabf..f0d18cadbe20 100644
> > --- a/drivers/pci/msi/irqdomain.c
> > +++ b/drivers/pci/msi/irqdomain.c
> > @@ -148,6 +148,23 @@ static void pci_device_domain_set_desc(msi_alloc_info_t *arg, struct msi_desc *d
> >  	arg->hwirq = desc->msi_index;
> >  }
> >  
> > +static __always_inline void cond_shutdown_parent(struct irq_data *data)
> 
> Is there a functional reason why we need __always_inline?
> 

I am not sure for this. As I found other cond_[mask/unmask]_parent()
also have this attribute, I added this as well.

> If not, it seems like this annotation is just clutter, and the compiler
> will probably inline it all by itself.
> 

I will see if someone know the reason. If there is no other objection,
I will remove this in the next version

Regards,
Inochi