linux-kernel - [PATCH+DISCUSSION] irqchip: armada-370-xp: Remove redundant ops assignment

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-next>] [day] [month] [year] [list]

Message-ID: <e4c7b434452775d00b6621012ad5e263076b3fcf.camel@kernel.crashing.org>
Date:   Wed, 12 Jun 2019 15:16:05 +1000
From:   Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:     Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
Cc:     Gregory CLEMENT <gregory.clement@...e-electrons.com>,
        Marc Zyngier <marc.zyngier@....com>,
        linux-arm-kernel@...ts.infradead.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: [PATCH+DISCUSSION] irqchip: armada-370-xp: Remove redundant ops
 assignment

pci_msi_create_irq_domain -> pci_msi_domain_update_chip_ops will
set those two already since the driver sets MSI_FLAG_USE_DEF_CHIP_OPS

Signed-off-by: Benjamin Herrenschmidt <benh@...nel.crashing.org>
---

[UNTESTED]

Just something I noticed while browsing through those drivers in
search of ways to factor some of the code.

That leads to a question here:

Some MSI drivers such as this one (or any using the defaults mask/unmask
provided by drivers/pci/msi.c) only call the PCI MSI mask/unmask functions.

Some other drivers call those PCI function but *also* call the parent
mask/unmask (giv-v2m for example) which generally is the inner domain
which just itself forwards to its own parent.

Is there any preference for doing it one way or the other ? I can see
that in cases where the device doesn't support MSI masking, calling the
parent could be useful but we don't know that at the moment in the
corresponding code.

It feels like something we should consolidate (and remove code from
drivers). For example, the defaults in drivers/pci/msi.c could always
call the parent if it exists and has a mask/unmask callback.

Opinions ? I'm happy to produce patches once we agree...

diff --git a/drivers/irqchip/irq-armada-370-xp.c b/drivers/irqchip/irq-armada-370-xp.c
index c9bdc5221b82..911230f28e2d 100644
--- a/drivers/irqchip/irq-armada-370-xp.c
+++ b/drivers/irqchip/irq-armada-370-xp.c
@@ -197,8 +197,6 @@ static void armada_370_xp_irq_unmask(struct irq_data *d)

 static struct irq_chip armada_370_xp_msi_irq_chip = {
 	.name = "MPIC MSI",
-	.irq_mask = pci_msi_mask_irq,
-	.irq_unmask = pci_msi_unmask_irq,
 };

 static struct msi_domain_info armada_370_xp_msi_domain_info = {