Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 5 Oct 2020 13:33:35 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Marc Zyngier <maz@kernel.org>
Cc:     linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-arm-kernel@lists.infradead.org,
        Jonathan Hunter <jonathanh@nvidia.com>,
        Dmitry Osipenko <digetx@gmail.com>,
        Sowjanya Komatineni <skomatineni@nvidia.com>,
        Venkat Reddy Talla <vreddytalla@nvidia.com>,
        Thomas Gleixner <tglx@linutronix.de>, kernel-team@android.com
Subject: Re: [PATCH 3/3] soc/tegra: pmc: Don't create fake interrupt
 hierarchy levels
Message-ID: <20201005113335.GT425362@ulmo>
References: <20201005111443.1390096-1-maz@kernel.org>
 <20201005111443.1390096-4-maz@kernel.org>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="D5tFrmRBv7YOLFOK"
Content-Disposition: inline
In-Reply-To: <20201005111443.1390096-4-maz@kernel.org>
User-Agent: Mutt/1.14.7 (2020-08-29)
Precedence: bulk


--D5tFrmRBv7YOLFOK
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Oct 05, 2020 at 12:14:43PM +0100, Marc Zyngier wrote:
> The Tegra PMC driver does ungodly things with the interrupt hierarchy,
> repeatedly corrupting it by pulling hwirq numbers out of thin air,
> overriding existing IRQ mappings and changing the handling flow
> of unsuspecting users.
>=20
> All of this is done in the name of preserving the interrupt hierarchy
> even when these levels do not exist in the HW. Together with the use
> of proper IRQs for IPIs, this leads to an unbootable system as the
> rescheduling IPI gets repeatedly repurposed for random drivers...
>=20
> Instead, let's allow the hierarchy to be trimmed to the level that
> actually makes sense for the HW, and not any deeper. This avoids
> having unnecessary callbacks, overriding mappings, and otherwise
> keeps the hierarchy sane.
>=20
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  drivers/soc/tegra/pmc.c | 79 +++++++++++++++--------------------------
>  1 file changed, 29 insertions(+), 50 deletions(-)
>=20
> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
> index 9960f7c18431..4eea3134fb3e 100644
> --- a/drivers/soc/tegra/pmc.c
> +++ b/drivers/soc/tegra/pmc.c
> @@ -1993,6 +1993,30 @@ static int tegra_pmc_irq_translate(struct irq_doma=
in *domain,
>  	return 0;
>  }
> =20
> +/* Trim the irq hierarchy from a particular irq domain */
> +static void trim_hierarchy(unsigned int virq, struct irq_domain *domain)
> +{
> +	struct irq_data *tail, *irq_data =3D irq_get_irq_data(virq);
> +
> +	/* The PMC doesn't generate any interrupt by itself */
> +	if (WARN_ON(!irq_data->parent_data))
> +		return;
> +
> +	/* Skip until we find the right domain */
> +	while (irq_data->parent_data && irq_data->parent_data->domain !=3D doma=
in)
> +		irq_data =3D irq_data->parent_data;
> +
> +	/* Sever the inner part of the hierarchy...  */
> +	tail =3D irq_data->parent_data;
> +	irq_data->parent_data =3D NULL;
> +
> +	/* ... and free it */
> +	for (irq_data =3D tail; irq_data; irq_data =3D tail) {
> +		tail =3D irq_data->parent_data;
> +		kfree(irq_data);
> +	};
> +}

That kind of looks like what I originally wanted to do and (naively)
thought that passing the (0, NULL, NULL) triplet would achieve.

Given that this is fairly low-level stuff that deals with the inner
workings of the IRQ infrastructure, should we eventually pull this out
of the driver and make it into a core helper? I don't seriously expect
this to be widely useful, but putting it into the core might help keep
it more maintainable.

I volunteer to do that work if you think it's a good idea.

Thierry

--D5tFrmRBv7YOLFOK
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl97BI8ACgkQ3SOs138+
s6EuNhAApCC/Y3UxL04Yc45mEeylUQ9lbVX8t5YJdJfEEoWIl9a4nCmfszoDz6Jr
0EO3/atnQLyx3E5Lm61/yKshCOrvO6AQNTx//P++eEhIiRcz/HPQQ2dZ5RKCYP/g
/cRW8F6S92TdE1hlVuvhEi3cTR2fpdBbo5+y6sNvaHppg9U3mZEXttMB7vb9Ftv9
xFzCk9vIKXfrD5trhyhqgpN8P4rIcq68XWUkJYonv34qO8FmTHziUC6NtJgKesuN
RLdDOtyLHVQrJ0h+kU492Gqki5kX7hJRlqxu0EI9tYWWIqYFZT6hylJ4Ozn5v53h
zeMLezl3AF6PxQinW0Pg7wWcyJtWb3QSBD+HGW/6jXtzQOtdwqZSCRwJ7ylA+Axz
fb5lFkxvT5LcWSZEFk75yCns759w79jSqDgnYUSZJTiG/0FoamgAG5iMXTsTdwJn
DDuqiWVpIslq3AvO0IyYNSwwmzSmQBsQkr84mnvFEcPMhrrN/u46AeqAHJRsXUQ+
uZoiUmKL5Hbrpg3GQz6DRAB4nW1wollWyhF1PTiHkImmIhacYijzsb3Y32QJKRom
wysRz/m0L9OcXt9lH1KTGjDQbvZa+yAFG2QpPkFuGqstcyI8FJkiH9y3bQd0eJeS
YgSfnwxv2zC9kjD92FdFg6iofeY9OJglDyG0oPYr4zVwEhty7SM=
=GsHC
-----END PGP SIGNATURE-----

--D5tFrmRBv7YOLFOK--