Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
From:   Biju Das <biju.das.jz@bp.renesas.com>
To:     Marc Zyngier <maz@kernel.org>
CC:     Thomas Gleixner <tglx@linutronix.de>,
        Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>,
        Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>,
        Geert Uytterhoeven <geert+renesas@glider.be>,
        Biju Das <biju.das.au@gmail.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-renesas-soc@vger.kernel.org" 
        <linux-renesas-soc@vger.kernel.org>
Subject: RE: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
 trigger detection for TINT
Thread-Topic: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
 trigger detection for TINT
Thread-Index: AQHZ6isSnZZc/iqk1kePfAP8iH4mBLAiOWYAgAALsBCAABCYgIAAACWA
Date:   Tue, 19 Sep 2023 16:32:05 +0000
Message-ID: <OS0PR01MB59228AD24951D49F3E639BFF86FAA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
References: <20230918122411.237635-1-biju.das.jz@bp.renesas.com>
        <20230918122411.237635-4-biju.das.jz@bp.renesas.com>
        <86y1h2cjpb.wl-maz@kernel.org>
        <OS0PR01MB5922748F489467BE2539AA1886FAA@OS0PR01MB5922.jpnprd01.prod.outlook.com>
 <87cyye3zly.wl-maz@kernel.org>
In-Reply-To: <87cyye3zly.wl-maz@kernel.org>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?DD6wL2T3W+TXQH7LB6+RgJmSkbWHVFjUlSPOpkLRL1H6zoTkkABXzOYmMOZJ?=
 =?us-ascii?Q?skI1dAdL0EfF1GCllZebFF5qESvAjqufzVyXcCEVK/Ao1yF1dziuOfi2XjyR?=
 =?us-ascii?Q?CTC9uyJE4fcCH+Tv8HdUpZraeXuOS1mWlzPs0EAbeU05yFk26XzUnPzrpwhX?=
 =?us-ascii?Q?RSe462kHSqkyJi/3moHD1l7txbHy6OPeKKtwc2y6y8EcrgEjobAjqXZURAKN?=
 =?us-ascii?Q?BAG+xVNEliUQjRHVpXOq+uq0+AQTnofy9UhA9wg8pCiHY+Nmf7Dtx1mFqxl6?=
 =?us-ascii?Q?z1I1udEIK9u6lylruezxaB1TcAzjTnEyl6vIpGQtxU4aqC6jwWRyoolfBw3R?=
 =?us-ascii?Q?BsMl1IZNv0CfPwdqI6Lx7R4Q/Ph9oncdgJ/HHqblaRpstJcHCDwk/b1H8vNN?=
 =?us-ascii?Q?r1MmJqUA5P4TBdJ/uM9XpDSlFGYNRRO3PkQRb+mlilu6HM42Dheoj1LcVlwb?=
 =?us-ascii?Q?osqj0dIB0osGneqk97pqb6DrXhpd3ujQTR3OMSx+W1p5M2/rcE7Odw2oQQkD?=
 =?us-ascii?Q?gTTNNK+T4ZgFiY92Z6O+vxULpKaPV4SkfhJRvKa0IoILUwm40jdqpL9W6yZ9?=
 =?us-ascii?Q?Gn7vCoHJP/ebHZkYta00QNmSZ5gaO///NleSM9YDZTllitALde8L5AELu5UB?=
 =?us-ascii?Q?xGiwJe2bWXAJRxx4LsOuYZN/Sf8WxZHbweWYE+MItS9aRHwcBQMSuq6PPiES?=
 =?us-ascii?Q?+jiBfCFFUAlnQhqNc3EXth+rucw6HCavwmBN8Pex1gVF3esx/Xz/P5gUiNLx?=
 =?us-ascii?Q?Ub5i5rbodFhgAiYw8hHgDbkjPF8d2o14x/0dQfGOX8UYC30+PYmvI8cleJAr?=
 =?us-ascii?Q?hLv5vIJi/Ehtm9o6JmeFgNnGc8s7SPALUA9HUXBkAlcO0fAXT8znuQkTkv1Y?=
 =?us-ascii?Q?nuwpGGFsFBQ8fjpQg5NmFvCJYFxdSnFyPBLmLzr8qeQSURP5EAQ4lZW/ORpt?=
 =?us-ascii?Q?WfYOtskJvwlKdE3K9Xjda0gtPTV9OFVt3nevzEswl9h08qEfaGYvKUrNiTWE?=
 =?us-ascii?Q?6kpTsL7iMaxfhnfC0HXsHVLB38anfOBnWEUYFDuRq64mpPG3fgkO2ENCJcna?=
 =?us-ascii?Q?mUBhIRYJG22yjW+d538M57fTMIV8kzu6cNKk4jo3fqkEUcWJm37i6H+oCpi2?=
 =?us-ascii?Q?eNSPnDqgkdLHZSgPX2TCyJ+3fbnNiUrvS0bhv5rrABq6HFsZgqfPQVVjY5ql?=
 =?us-ascii?Q?auYEde3U8hT9+j/us3lCedfanymCGaU4rn3D73bTgG4PX82jrsrwchAO1/VM?=
 =?us-ascii?Q?BpJn8zutc1SiYJmUXu+CLVu1m5mnEqalbljvbBoezBGfh1abpazwYqO0HiXp?=
 =?us-ascii?Q?p0YCr47eZ2MEq4A8QlKKZVH650HF4f4A/nVglBHeZG/Of5cumzaV5Axp0Z/R?=
 =?us-ascii?Q?kL/qaY8hFYc0zpqrmsbm3JUEVLKMPoU8P5EGuuL+4Jvn/763XQ8raz5WF8Ev?=
 =?us-ascii?Q?9oLVlo1+TdVq1BDznV9AcGIcx0f5pZ95ki5o37tbzgRN9i6FIVd82FHC77Cr?=
 =?us-ascii?Q?Ye2avI02jQJ8nIRMQWhgq0AHAp9OmI3LfVQ0oWuIC9PDgvaQPV7tGV+Vpj/S?=
 =?us-ascii?Q?EJLe4p+OEN+U86yFC5JJz9JDGmL5UoZDqrn3DhZ0uIbD8CPNZddR3KsGCz+I?=
 =?us-ascii?Q?Vg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: bp.renesas.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: OS0PR01MB5922.jpnprd01.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4744c105-b093-4d52-657c-08dbb92df5c2
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 Sep 2023 16:32:05.0606
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 53d82571-da19-47e4-9cb4-625a166a4a2a
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: iBCBcxqKVciiJ2NlKywyE0eeF089mWNlbhr47vLh1yrshSRNyd0jGWPhIrq1OkR1i+KMeVR581Fyh4G6ekKoEpcYr3bytVmWzDLzDl2LahM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: TY3PR01MB11114
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Tue, 19 Sep 2023 09:32:46 -0700 (PDT)

Hi Marc Zyngier,

> Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with edge
> trigger detection for TINT
>=20
> On Tue, 19 Sep 2023 16:24:53 +0100,
> Biju Das <biju.das.jz@bp.renesas.com> wrote:
> >
> > Hi Marc Zyngier,
> >
> > Thanks for the feedback.
> >
> > > Subject: Re: [PATCH 3/3] irqchip: renesas-rzg2l: Fix irq storm with
> > > edge trigger detection for TINT
> > >
> > > On Mon, 18 Sep 2023 13:24:11 +0100,
> > > Biju Das <biju.das.jz@bp.renesas.com> wrote:
> > > >
> > > > In case of edge trigger detection, enabling the TINT source causes
> > > > a phantum interrupt that leads to irq storm. So clear the phantum
> > > > interrupt in rzg2l_irqc_irq_enable().
> > > >
> > > > This issue is observed when the irq handler disables the
> > > > interrupts using
> > > > disable_irq_nosync() and scheduling a work queue and in the work
> > > > queue, re-enabling the interrupt with enable_irq().
> > > >
> > > > Fixes: 3fed09559cd8 ("irqchip: Add RZ/G2L IA55 Interrupt
> > > > Controller
> > > > driver")
> > > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> > > > ---
> > > >  drivers/irqchip/irq-renesas-rzg2l.c | 6 ++++++
> > > >  1 file changed, 6 insertions(+)
> > > >
> > > > diff --git a/drivers/irqchip/irq-renesas-rzg2l.c
> > > > b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > index 33a22bafedcd..78a9e90512a6 100644
> > > > --- a/drivers/irqchip/irq-renesas-rzg2l.c
> > > > +++ b/drivers/irqchip/irq-renesas-rzg2l.c
> > > > @@ -144,6 +144,12 @@ static void rzg2l_irqc_irq_enable(struct
> > > > irq_data
> > > *d)
> > > >  		reg =3D readl_relaxed(priv->base + TSSR(tssr_index));
> > > >  		reg |=3D (TIEN | tint) << TSSEL_SHIFT(tssr_offset);
> > > >  		writel_relaxed(reg, priv->base + TSSR(tssr_index));
> > > > +		/*
> > > > +		 * In case of edge trigger detection, enabling the TINT
> source
> > > > +		 * cause a phantum interrupt that leads to irq storm. So
> clear
> > > > +		 * the phantum interrupt.
> > > > +		 */
> > > > +		rzg2l_tint_eoi(d);
> > >
> > > This looks incredibly unsafe. disable_irq()+enable_irq() with an
> > > interrupt being made pending in the middle, and you've lost that
> interrupt.
> >
> > In this driver that will never happen as it clears the TINT source
> > during disable(), so there won't be any TINT source for interrupt
> > detection after disable().
>=20
> So you mean that you *already* lose interrupts across a disable followed =
by
> an enable? I'm slightly puzzled...

There is no interrupt lost at all.=20

Currently this patch addresses 2 issues.

Scenario 1: Extra interrupt when we select TINT source on enable_irq()

Getting an extra interrupt, when client drivers calls enable_irq() during p=
robe()/resume(). In this case, the irq handler on the
Client driver just clear the interrupt status bit.

Issue 2: IRQ storm when we select TINT source on enable_irq()

Here as well, we are getting an extra interrupt, when client drivers calls =
enable_irq() during probe() and this Interrupts getting generated infinitel=
y, when the client driver calls disable_irq() in irq handler and in in work=
 queue calling enable_irq().

Currently we are not loosing interrupts, but we are getting additional
Interrupt(phantom) which is causing the issue.

Cheers,
Biju