Re: [PATCH v6 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV

From: Nicolin Chen <nicolinc@nvidia.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: <will@kernel.org>, <robin.murphy@arm.com>, <joro@8bytes.org>,
	<thierry.reding@gmail.com>, <vdumpa@nvidia.com>,
	<jonathanh@nvidia.com>, <linux-kernel@vger.kernel.org>,
	<iommu@lists.linux.dev>, <linux-arm-kernel@lists.infradead.org>,
	<linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v6 5/6] iommu/arm-smmu-v3: Add in-kernel support for NVIDIA Tegra241 (Grace) CMDQV
Date: Thu, 2 May 2024 12:26:26 -0700	[thread overview]
Message-ID: <ZjPo4qUXOZzLsHW8@Asurada-Nvidia> (raw)
In-Reply-To: <20240502124103.GA3341011@nvidia.com>

On Thu, May 02, 2024 at 09:41:03AM -0300, Jason Gunthorpe wrote:
> On Wed, May 01, 2024 at 10:43:39AM -0700, Nicolin Chen wrote:
> > > It doesn't fix any real race, I'm not sure what this is supposed to be
> > > doing. The cmdq becomes broken and you get an ISR, so before the ISR
> > > it will still post but get stuck, during the ISR it will avoid
> > > posting, and after it will go back to posting?
> > > 
> > > Why? Just always post to the Q and let the ISR fix it?
> > 
> > Yes, we could do so. I was thinking of the worst case by giving
> > the guest OS a chance to continue (though in a slower mode), if
> > something unrecoverable happens to the VINTF/VCMDQ part.
> 
> Does that happn? The stuck vcmdq will have stuck entries on it no
> matter what, can we actually fully recover from that? Ie re-issue the
> commands on another queue?

Well, the handle_vintf0_error() should fix that and recover. And
rethinking about this, if this happens it's likely a SW bug that
we shouldn't ignore.

With that being said, the viommu infrastructure still needs an irq
forwarding that is currently missing. I'd need to draft something
likely on top of Baolu's work.

> > > So just don't use it. There is no value if the places where it should
> > > work automatically are not functioning.
> > 
> > I thought devm could work when rmmod too, not only when the probe
> > fails..
> 
> It is limited to cases when the probing driver of the passed struct
> device unbinds, including probe failure.

OK. I'll drop all devm_ and add tegra241_cmdqv_device_remove()
instead.

Thanks
Nicolin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel