linux-kernel - Re: [PATCH v2] drivers: irqchip: add irq-type-changer

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: <a7a6ddd2-9d40-3ac2-2b7f-bb521c8d3728@cogentembedded.com>
Date:   Tue, 25 Jan 2022 12:35:22 +0300
From:   Nikita Yushchenko <nikita.yoush@...entembedded.com>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Eugeniu Rosca <erosca@...adit-jv.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drivers: irqchip: add irq-type-changer

>>> I also don't see why you model this as the actual device that triggers
>>> the interrupt.
>>
>> Well, that somehow matches the physical reality. In the case of wl18xx
>> on KF, physically the interrupt signal indeed originates from the
>> intermediate chip - the inverting level-shifter.
> 
> Reality? By allowing something like an edge-to-level conversion? How
> can that work?

Edge to level can be problematic, but level to edge does not cause any difficulties, nor in generating 
nor in handling.

>> In contrary, an abstraction of intermediate entity that does a static
>> conversion of the trigger type and does not need any software control,
>> looks sane. And, hardware designers do strange things sometimes, I
>> won't be surprised observing a change from level to edge one day.
> 
> If you think that it can happen without a HW register that latches the
> edge and requires an ack, you need to question your understanding of
> an interrupt life cycle, and of the properties of the various trigger
> types.

There are plenty of devices capable to signal both level and edge interrupts, configurable by a 
register. Basic handling is always the same, and involves masking the interrupt at interrupt controller 
while IRQs disabled, then enabling IRQs, then programming the device to clear the interrupt condition, 
then unmasking the interrupt at interrupt controller. If the trigger type is level or edge, is only 
interesting to interrupt controller driver, but not to a wider scope.

Nothing stops hw designers from doing all sort of strange things with interrupt signals. Right now I 
have a board on my desk where interrupt signals from several chips are wired to inputs of a logical AND 
element and the output of that is wired to SoC's gpio. I don't see what stops them tomorrow from setting 
up their CPLDs to issue a short impulse at output in return to a level change on input. And that will be 
a level to edge converter.

>> In driver's init, I only call of_irq_parse_one() for interrupt defined
>> in the changer's node. This does not create a mapping for it. The
>> mapping is only created when changer's "interrupt-child" creates a
>> mapping for their interrupt - then changer's alloc() routine calls
>> irq_domain_alloc_irqs_parent() in the same way as all other
>> hierarchical irqchips do.
>>
>> I don't see where double mapping can appear here. Please explain.
> 
> Just look at your code. You start probing a device that has an
> 'interrupts' property. This triggers the allocation of an interrupt.

Where does it trigger it?

And what is this "allocation", after all? Is it allocation of virq number? That allocation happens when 
irq_create_fwspec_mapping() calls irq_domain_alloc_irqs(). But this path does not necessary gets 
executed in probing. In the current irq_tyoe_changer driver, it is not.

Nikita