netdev - Re: [PATCH v1 1/1] net: stmmac: dwmac-tegra: Read iommu stream id from device tree

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: <20241115135940.5f898781.parker@finest.io>
Date: Fri, 15 Nov 2024 13:59:40 -0500
From: Parker Newman <parker@...est.io>
To: Paolo Abeni <pabeni@...hat.com>
Cc: Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu
 <joabreu@...opsys.com>, Andrew Lunn <andrew+netdev@...n.ch>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Maxime Coquelin <mcoquelin.stm32@...il.com>,
 Thierry Reding <thierry.reding@...il.com>, Jonathan Hunter
 <jonathanh@...dia.com>, netdev@...r.kernel.org,
 linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, linux-tegra@...r.kernel.org,
 linux-kernel@...r.kernel.org, Parker Newman <pnewman@...necttech.com>
Subject: Re: [PATCH v1 1/1] net: stmmac: dwmac-tegra: Read iommu stream id
 from device tree

On Fri, 15 Nov 2024 18:17:07 +0100
Paolo Abeni <pabeni@...hat.com> wrote:

> On 11/15/24 17:31, Parker Newman wrote:
> > From: Parker Newman <pnewman@...necttech.com>
> >
> > Read the iommu stream id from device tree rather than hard coding to mgbe0.
> > Fixes kernel panics when using mgbe controllers other than mgbe0.
>
> It's better to include the full Oops backtrace, possibly decoded.
>

Will do, there are many different ones but I can add the most common.

> > Tested with Orin AGX 64GB module on Connect Tech Forge carrier board.
>
> Since this looks like a fix, you should include a suitable 'Fixes' tag
> here, and specify the 'net' target tree in the subj prefix.
>

Sorry I missed the "net" tag.

The bug has existed since dwmac-tegra.c was added. I can add a Fixes tag but
in the past I was told they aren't needed in that situation?

> > @@ -241,6 +243,12 @@ static int tegra_mgbe_probe(struct platform_device *pdev)
> >  	if (IS_ERR(mgbe->xpcs))
> >  		return PTR_ERR(mgbe->xpcs);
> >
> > +	/* get controller's stream id from iommu property in device tree */
> > +	if (!tegra_dev_iommu_get_stream_id(mgbe->dev, &mgbe->iommu_sid)) {
> > +		dev_err(mgbe->dev, "failed to get iommu stream id\n");
> > +		return -EINVAL;
> > +	}
>
> I *think* it would be better to fallback (possibly with a warning or
> notice) to the previous default value when the device tree property is
> not available, to avoid regressions.
>

I debated this as well... In theory the iommu must be setup for the
mgbe controller to work anyways. Doing it this way means the worst case is
probe() fails and you lose an ethernet port.

Having it fall back to mgbe0's SID adds the risk of the entire system crashing.

I can see arguments for both methods. I can add the fallback to mgbe0's SID
and change the message to a warning when I send V2 if you like.

Thanks!
Parker

> Thanks,
>
> Paolo
>