linux-kernel - Re: [PATCH] genirq: Introduce irq_read

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: <CAJAp7OjQ8qRspy_ocUnp+es+HPiR1jULhKK6g4DBXXZcOsABug@mail.gmail.com>
Date:	Fri, 24 Oct 2014 16:12:55 -0700
From:	Bjorn Andersson <bjorn@...o.se>
To:	Marc Zyngier <marc.zyngier@....com>
Cc:	Stephen Boyd <sboyd@...eaurora.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Linus Walleij <linus.walleij@...aro.org>,
	linux-arm-msm <linux-arm-msm@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Abhijeet Dharmapurikar <adharmap@...eaurora.org>
Subject: Re: [PATCH] genirq: Introduce irq_read_line()

On Fri, Oct 24, 2014 at 10:59 AM, Marc Zyngier <marc.zyngier@....com> wrote:
> Hi Bjorn,
>
> On 24/10/14 18:31, Bjorn Andersson wrote:
>> On Tue, Oct 21, 2014 at 2:34 AM, Marc Zyngier <marc.zyngier@....com> wrote:
[..]
>> I would prefer to see that you had explicit functions for the various things
>> that you would like to set and get. It would in my eyes give cleaner client
>> code, at the cost of a few extra functions.
>
> I don't really like the idea of having a gazillion of accessors for each
> individual property. I'd rather have an extensible API.
>

Okay, either way will work. Feel free to proceed with your approach.

>>> Basic idea is that you can read (and possibly write back) various
>>> low-level attributes (pending, masked, active) that an interrupt
>>> controller may implement. Given your use case, we should loose the
>>> checks on the interrupt being forwarded, as this makes little sense
>>> outside of virtualization.
>>>
>>
>> Relaxing the forwarding requirement seems to solve my use cases. May I ask why
>> your accessors would only be made available for forwarded IRQs - at a framework
>> level?
>
> This was never intended for wider use, and restricting it to forwarded
> interrupts was a concious design decision, as I find the idea of a
> device driver messing with the internal state of the interrupt
> controller positively repulsive... But hey, the more the merrier.
>

Makes sense and now that it comes without the forwarding part it looks
reasonable.

[..]
>>> I'm planning to respin the series this week, as I have a number of
>>> changes (there is hardly any need for the various states to be reported
>>> atomically, for example), and a number of bugs have been found.
>>>
>>
>> Sounds good, for clarity I would like them to be explicit separate functions.
>> But as long as it's not limited to forwarded IRQs we should be able to use it.
>
> I just pushed out a branch:
> git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git
> irq/irqchip_state
>
> Please let me know if that's useful for you.
>

I think that my irq_read_line() would be equivalent of
irq_get_irqchip_state(IRQCHIP_STATE_PENDING), i.e. the state of the
interrupt ignoring masking. And the rest would be EINVAL.
So I think this would work out just fine for us!


Is ACTIVE the line level with the mask considered and setting ACTIVE,
would that be the same as acking the interrupt? What's the expected
behaviour of setting PENDING?

I would appreciate if you commented the state enum, to make it obvious
what pending and active means.

Regards,
Bjorn
--
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/