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
| ||
|
Date: Sat, 16 May 2020 13:01:26 +0000 From: Anup Patel <Anup.Patel@....com> To: Marc Zyngier <maz@...nel.org> CC: Palmer Dabbelt <palmer@...belt.com>, Paul Walmsley <paul.walmsley@...ive.com>, Thomas Gleixner <tglx@...utronix.de>, Jason Cooper <jason@...edaemon.net>, Atish Patra <Atish.Patra@....com>, Alistair Francis <Alistair.Francis@....com>, Anup Patel <anup@...infault.org>, "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org> Subject: RE: [PATCH 3/4] irqchip/sifive-plic: Separate irq_chip for muiltiple PLIC instances > -----Original Message----- > From: Marc Zyngier <maz@...nel.org> > Sent: 16 May 2020 17:59 > To: Anup Patel <Anup.Patel@....com> > Cc: Palmer Dabbelt <palmer@...belt.com>; Paul Walmsley > <paul.walmsley@...ive.com>; Thomas Gleixner <tglx@...utronix.de>; Jason > Cooper <jason@...edaemon.net>; Atish Patra <Atish.Patra@....com>; Alistair > Francis <Alistair.Francis@....com>; Anup Patel <anup@...infault.org>; linux- > riscv@...ts.infradead.org; linux-kernel@...r.kernel.org > Subject: Re: [PATCH 3/4] irqchip/sifive-plic: Separate irq_chip for muiltiple PLIC > instances > > On 2020-05-16 07:39, Anup Patel wrote: > > To distinguish interrupts from multiple PLIC instances, we use a > > per-PLIC irq_chip instance with a different name. > > > > Signed-off-by: Anup Patel <anup.patel@....com> > > --- > > drivers/irqchip/irq-sifive-plic.c | 28 +++++++++++++++------------- > > 1 file changed, 15 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/irqchip/irq-sifive-plic.c > > b/drivers/irqchip/irq-sifive-plic.c > > index 2d3db927a551..e42fc082ad18 100644 > > --- a/drivers/irqchip/irq-sifive-plic.c > > +++ b/drivers/irqchip/irq-sifive-plic.c > > @@ -60,6 +60,7 @@ > > #define PLIC_ENABLE_THRESHOLD 0 > > > > struct plic_priv { > > + struct irq_chip chip; > > struct cpumask lmask; > > struct irq_domain *irqdomain; > > void __iomem *regs; > > @@ -76,6 +77,7 @@ struct plic_handler { > > void __iomem *enable_base; > > struct plic_priv *priv; > > }; > > +static unsigned int plic_count; > > static bool plic_cpuhp_setup_done; > > static DEFINE_PER_CPU(struct plic_handler, plic_handlers); > > > > @@ -164,20 +166,12 @@ static void plic_irq_eoi(struct irq_data *d) > > writel(d->hwirq, handler->hart_base + CONTEXT_CLAIM); } > > > > -static struct irq_chip plic_chip = { > > - .name = "SiFive PLIC", > > - .irq_mask = plic_irq_mask, > > - .irq_unmask = plic_irq_unmask, > > - .irq_eoi = plic_irq_eoi, > > -#ifdef CONFIG_SMP > > - .irq_set_affinity = plic_set_affinity, > > -#endif > > -}; > > - > > static int plic_irqdomain_map(struct irq_domain *d, unsigned int irq, > > irq_hw_number_t hwirq) > > { > > - irq_domain_set_info(d, irq, hwirq, &plic_chip, d->host_data, > > + struct plic_priv *priv = d->host_data; > > + > > + irq_domain_set_info(d, irq, hwirq, &priv->chip, d->host_data, > > handle_fasteoi_irq, NULL, NULL); > > irq_set_noprobe(irq); > > return 0; > > @@ -294,6 +288,14 @@ static int __init plic_init(struct device_node > > *node, > > if (!priv) > > return -ENOMEM; > > > > + priv->chip.name = kasprintf(GFP_KERNEL, "PLIC%d", plic_count++); > > + priv->chip.irq_mask = plic_irq_mask, > > + priv->chip.irq_unmask = plic_irq_unmask, > > + priv->chip.irq_eoi = plic_irq_eoi, > > +#ifdef CONFIG_SMP > > + priv->chip.irq_set_affinity = plic_set_affinity, #endif > > + > > priv->regs = of_iomap(node, 0); > > if (WARN_ON(!priv->regs)) { > > error = -EIO; > > @@ -383,9 +385,9 @@ static int __init plic_init(struct device_node > > *node, > > } > > > > pr_info("interrupt-controller at 0x%llx " > > - "(interrupts=%d, contexts=%d, handlers=%d)\n", > > + "(interrupts=%d, contexts=%d, handlers=%d) (%s)\n", > > (unsigned long long)iores.start, nr_irqs, > > - nr_contexts, nr_handlers); > > + nr_contexts, nr_handlers, priv->chip.name); > > set_handle_irq(plic_handle_irq); > > return 0; > > I really dislike this patch for multiple reasons: > > - Allocating a new struc irq_chip just for a string seems over the top, > specially as all the *useful* stuff stays the same. > > - Even if I hate it, /proc is API. I'm sure something, somewhere is > parsing this. Changing the string is likely to confuse it. AFAIK, we don't have scripts in RISC-V world that depend on /proc/interrupts content. May be in future such scripts will show up. For system with multiple PLICs, we are seeing same "SiFive PLIC" string for all PLIC interrupts in "cat /proc/interrupts". I am trying to assign different string based on PLIC instance. This is similar to what GICv2 driver is doing (e.g. GIC-0, GIC-1, ... in /proc/interrupts). Is there a better way to do this ? > > - If you do this for debug purposes, then CONFIG_GENERIC_IRQ_DEBUGFS > is the right way to look up the information. > > - If, for reasons that are beyond me, you actually *need* this, then > implementing irq_print_chip in your irq_chip structure is the way > to go. > > But frankly, I'd rather you drop this altogether. I just want to differentiate which interrupt belongs to which PLIC Instance in /proc/interrupts. I can take a different approach if you suggest. Regards, Anup
Powered by blists - more mailing lists