linux-kernel - Re: [PATCH v7 2/2] irqchip: Add Qualcomm MPM controller driver

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: <87lexp211g.wl-maz@kernel.org>
Date:   Fri, 04 Mar 2022 15:24:43 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Shawn Guo <shawn.guo@...aro.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Maulik Shah <quic_mkshah@...cinc.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Sudeep Holla <sudeep.holla@....com>,
        Rob Herring <robh+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v7 2/2] irqchip: Add Qualcomm MPM controller driver

On Fri, 04 Mar 2022 08:23:42 +0000,
Shawn Guo <shawn.guo@...aro.org> wrote:
> 
> On Fri, Mar 04, 2022 at 07:59:15AM +0000, Marc Zyngier wrote:
> > On Thu, 03 Mar 2022 04:02:29 +0000,
> > Shawn Guo <shawn.guo@...aro.org> wrote:
> > > 
> > > On Wed, Mar 02, 2022 at 01:57:27PM +0000, Marc Zyngier wrote:
> > > > This code actually makes me ask more questions. Why is it programming
> > > > 2 'pins' for each IRQ?
> > > 
> > > The mapping between MPM pin and GIC IRQ is not strictly 1-1.  There are
> > > some rare case that up to 2 MPM pins map to a single GIC IRQ, for
> > > example the last two in QC2290 'qcom,mpm-pin-map' below.
> > > 
> > > 	qcom,mpm-pin-map = <2 275>,     /* tsens0_tsens_upper_lower_int */
> > > 			   <5 296>,     /* lpass_irq_out_sdc */
> > > 			   <12 422>,    /* b3_lfps_rxterm_irq */
> > > 			   <24 79>,     /* bi_px_lpi_1_aoss_mx */
> > > 			   <86 183>,    /* mpm_wake,spmi_m */
> > > 			   <90 260>,    /* eud_p0_dpse_int_mx */
> > > 			   <91 260>;    /* eud_p0_dmse_int_mx */
> > > 
> > > 
> > > The downstream uses a DT bindings that specifies GIC hwirq number in
> > > client device nodes.  In that case, d->hwirq in the driver is GIC IRQ
> > > number, and the driver will need to query mapping table, find out the
> > > possible 2 MPM pins, and set them up.
> > > 
> > > The patches I'm posting here use a different bindings that specifies MPM
> > > pin instead in client device nodes.  Thus the driver can simply get the
> > > MPM pin from d->hwirq, so that the whole look-up procedure can be saved.
> > 
> > It still remains that there is no 1:1 mapping between input and
> > output, which is the rule #1 to be able to use a hierarchical setup.
> 
> For direction of MPM pin -> GIC interrupt, it's a 1:1 mapping, i.e. for
> given MPM pin, there is only one GIC interrupt.  And that's the
> mapping MPM driver relies on.  For GIC interrupt -> MPM pin, it's not
> a strict 1:1 mapping.

Then this isn't a 1:1 mapping *AT ALL*. The hierarchical setup
mandates that the mapping is a bijective function, and that's exactly
what 1:1 means. There is no such thing a 1:1 in a single
direction. When you take an interrupt, all you see is the GIC
interrupt. How do you know which of the *two* pins interrupted you? Oh
wait, you *can't* know. You end-up never servicing one of the two
interrupts (and I suspect this results in memory corruption if you
tear a hierarchy down).

This HW deals with 2:1 mappings, i.e. it is a mux. So all the time
spent on this driver is totally lost, because you have the wrong
abstraction. And the QC driver is just as bad.

	M.

-- 
Without deviation from the norm, progress is not possible.