linux-kernel - Re: [PATCH v8 2/2] s390/pci: Migrate s390 IRQ logic to IRQ domain API

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: <0506645063efd6c1472c3b787b5195e96e18048e.camel@linux.ibm.com>
Date: Wed, 03 Dec 2025 17:14:39 +0100
From: Gerd Bayer <gbayer@...ux.ibm.com>
To: Tobias Schumacher <ts@...ux.ibm.com>, Heiko Carstens
 <hca@...ux.ibm.com>,
        Vasily Gorbik <gor@...ux.ibm.com>,
        Alexander Gordeev
 <agordeev@...ux.ibm.com>,
        Christian Borntraeger	
 <borntraeger@...ux.ibm.com>,
        Sven Schnelle <svens@...ux.ibm.com>,
        Niklas
 Schnelle <schnelle@...ux.ibm.com>,
        Gerald Schaefer
 <gerald.schaefer@...ux.ibm.com>,
        Halil Pasic	 <pasic@...ux.ibm.com>,
        Matthew Rosato <mjrosato@...ux.ibm.com>,
        Thomas Gleixner	
 <tglx@...utronix.de>
Cc: linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org,
        Farhan Ali
	 <alifm@...ux.ibm.com>
Subject: Re: [PATCH v8 2/2] s390/pci: Migrate s390 IRQ logic to IRQ domain
 API

On Wed, 2025-12-03 at 15:36 +0100, Tobias Schumacher wrote:
> s390 is one of the last architectures using the legacy API for setup and
> teardown of PCI MSI IRQs. Migrate the s390 IRQ allocation and teardown
> to the MSI parent domain API. For details, see:
> 
> https://lore.kernel.org/lkml/20221111120501.026511281@linutronix.de
> 

  [ ... snip ... ]

> +static void zpci_msi_domain_free(struct irq_domain *domain, unsigned int virq,
> +				 unsigned int nr_irqs)
> +{
> +	struct zpci_dev *zdev;
> +	struct msi_desc *desc;
> +	struct irq_data *d;
> +	unsigned long bit;
> +	unsigned int cpu;
> +	u16 msi_index;
> +	int i;
> +
> +	for (i = 0; i < nr_irqs; i++) {
> +		d = irq_domain_get_irq_data(domain, virq + i);
> +		msi_index = zpci_decode_hwirq_msi_index(d->hwirq);
> +		desc = irq_data_get_msi_desc(d);
> +		zdev = to_zpci_dev(desc->dev);
> +		bit = zdev->msi_first_bit + msi_index;
>  
> +		if (irq_delivery == DIRECTED) {
>  			for_each_possible_cpu(cpu) {
> -				for (i = 0; i < irqs_per_msi; i++)
> -					airq_iv_set_data(zpci_ibv[cpu],
> -							 hwirq + i, irq + i);
> +				airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
> +				airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);
>  			}
>  		} else {
> -			msg.address_lo = zdev->msi_addr & 0xffffffff;
> -			for (i = 0; i < irqs_per_msi; i++)
> -				airq_iv_set_data(zdev->aibv, hwirq + i, irq + i);
> +			airq_iv_set_ptr(zdev->aibv, bit + i, 0);
> +			airq_iv_set_data(zdev->aibv, bit + i, 0);
>  		}
> -		msg.address_hi = zdev->msi_addr >> 32;
> -		pci_write_msi_msg(irq, &msg);
> -		hwirq += irqs_per_msi;
> +
> +		irq_domain_reset_irq_data(d);
>  	}
> +} 

Thanks for addressing my concern about clearing the airq data!

FWIW, what you thing about abstracting out the airq clearing stuff with
something like this diff on top, so the loop body remains somewhat
short and zpci_msi_domain_free() keeps its working set of local
variables.


diff --git a/arch/s390/pci/pci_irq.c b/arch/s390/pci/pci_irq.c
index 5639789dc58f..3322d8c9aff1 100644
--- a/arch/s390/pci/pci_irq.c
+++ b/arch/s390/pci/pci_irq.c
@@ -439,34 +439,37 @@ static int zpci_msi_domain_alloc(struct
irq_domain *domain, unsigned int virq,
        return 0;
 }
 
-static void zpci_msi_domain_free(struct irq_domain *domain, unsigned
int virq,
-                                unsigned int nr_irqs)
+static void zpci_msi_clear_airq(struct irq_data *d, int i)
 {
-       struct zpci_dev *zdev;
-       struct msi_desc *desc;
-       struct irq_data *d;
+       struct msi_desc *desc = irq_data_get_msi_desc(d);
+       struct zpci_dev *zdev = to_zpci_dev(desc->dev);
        unsigned long bit;
        unsigned int cpu;
        u16 msi_index;
-       int i;
 
-       for (i = 0; i < nr_irqs; i++) {
-               d = irq_domain_get_irq_data(domain, virq + i);
-               msi_index = zpci_decode_hwirq_msi_index(d->hwirq);
-               desc = irq_data_get_msi_desc(d);
-               zdev = to_zpci_dev(desc->dev);
-               bit = zdev->msi_first_bit + msi_index;
+       msi_index = zpci_decode_hwirq_msi_index(d->hwirq);
+       bit = zdev->msi_first_bit + msi_index;
 
-               if (irq_delivery == DIRECTED) {
-                       for_each_possible_cpu(cpu) {
-                               airq_iv_set_ptr(zpci_ibv[cpu], bit + i,
0);
-                               airq_iv_set_data(zpci_ibv[cpu], bit +
i, 0);
-                       }
-               } else {
-                       airq_iv_set_ptr(zdev->aibv, bit + i, 0);
-                       airq_iv_set_data(zdev->aibv, bit + i, 0);
+       if (irq_delivery == DIRECTED) {
+               for_each_possible_cpu(cpu) {
+                       airq_iv_set_ptr(zpci_ibv[cpu], bit + i, 0);
+                       airq_iv_set_data(zpci_ibv[cpu], bit + i, 0);
                }
+       } else {
+               airq_iv_set_ptr(zdev->aibv, bit + i, 0);
+               airq_iv_set_data(zdev->aibv, bit + i, 0);
+       }
+}
 
+static void zpci_msi_domain_free(struct irq_domain *domain, unsigned
int virq,
+                                unsigned int nr_irqs)
+{
+       struct irq_data *d;
+       int i;
+
+       for (i = 0; i < nr_irqs; i++) {
+               d = irq_domain_get_irq_data(domain, virq + i);
+               zpci_msi_clear_airq(d, i);
                irq_domain_reset_irq_data(d);
        }
 }

Sorry for me keeping nagging...
Gerd