linux-kernel - Re: [PATCH 11/24] C6X: interrupt handling

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:	Mon, 12 Sep 2011 10:27:02 -0400
From:	Mark Salter <msalter@...hat.com>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	linux-kernel@...r.kernel.org, grant.likely@...retlab.ca
Subject: Re: [PATCH 11/24] C6X: interrupt handling

On Fri, 2011-09-09 at 16:33 +0200, Thomas Gleixner wrote:
> On Wed, 31 Aug 2011, Mark Salter wrote:
> 
> > + *
> > + *  Large parts taken directly from powerpc.
> 
> Is it really necessary to copy that stuff instead of generalizing it ?
> I guess that's mostly about the reverse map & Co.

I have a patch that takes the code out of powerpc and puts in kernel/irq
where both powerpc and c6x can use it. That leaves us with two files in
kernel/irq trying to manage hw <--> virt mappings. So, some merging of
irqdomain.c and the former powerpc code is needed. I'm not sure how that
should go. At the moment, irqdomain doesn't support dynamic allocation
of irq_descs, so its not currently suitable for powerpc or c6x.


> > +/*
> > + * IRQ controller and virtual interrupts
> > + */
> 
> How different is this from PPC ? Looks fairly familiar to me :)

I had changed it in a few minor ways, but looking again, I probably
didn't need to change it at all. In the patch to generalize it, I
simply did s/NUM_ISA_INTERRUPTS/NR_IRQS_LEGACY/.

> > +static void megamod_irq_cascade(unsigned int irq, struct irq_desc *desc)
> > +{
> > +	struct irq_chip *chip = irq_desc_get_chip(desc);
> > +	struct irq_data *idata = irq_desc_get_irq_data(desc);
> > +	struct megamod_cascade_data *cascade;
> > +	struct megamod_pic *pic;
> > +	u32 events;
> > +	int n, idx;
> > +
> > +	cascade = irq_desc_get_handler_data(desc);
> > +
> > +	pic = cascade->pic;
> > +
> > +	raw_spin_lock(&desc->lock);
> > +
> > +	chip->irq_mask(idata);
> 
> This runs with interrupts disabled, so why the lock, mask and the
> inprogress fiddling? That interrupt cannot be reentered and it is not
> controlled by disable_irq/enable_irq and better not subject to
> free_irq, so why do you need this?

It was leftover cruft from a previous incarnation. Not needed, so
I took it out.

--Mark


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/