[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <29d5918e-cfe2-4b36-b0f1-a1379075dd05@arm.com>
Date: Mon, 4 Nov 2024 16:34:46 +0000
From: Robin Murphy <robin.murphy@....com>
To: Salil Mehta <salil.mehta@...wei.com>, Arnd Bergmann <arnd@...nel.org>,
"shenjian (K)" <shenjian15@...wei.com>
Cc: Arnd Bergmann <arnd@...db.de>, Will Deacon <will@...nel.org>,
Joerg Roedel <jroedel@...e.de>, "iommu@...ts.linux.dev"
<iommu@...ts.linux.dev>, Andrew Lunn <andrew+netdev@...n.ch>,
"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
shaojijie <shaojijie@...wei.com>, wangpeiyang <wangpeiyang1@...wei.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] [net-next] net: hns3: add IOMMU_SUPPORT dependency
On 2024-11-04 10:50 am, Salil Mehta wrote:
>> From: Salil Mehta <salil.mehta@...wei.com>
>> Sent: Monday, November 4, 2024 10:41 AM
>> To: Robin Murphy <robin.murphy@....com>; Arnd Bergmann
>> <arnd@...nel.org>; shenjian (K) <shenjian15@...wei.com>
>>
>> HI Robin,
>>
>> > From: Robin Murphy <robin.murphy@....com>
>> > Sent: Monday, November 4, 2024 10:29 AM
>> > To: Arnd Bergmann <arnd@...nel.org>; shenjian (K)
>> > <shenjian15@...wei.com>; Salil Mehta <salil.mehta@...wei.com>
>> > Cc: Arnd Bergmann <arnd@...db.de>; Will Deacon <will@...nel.org>;
>> > Joerg Roedel <jroedel@...e.de>; iommu@...ts.linux.dev; Andrew Lunn
>> > <andrew+netdev@...n.ch>; David S. Miller <davem@...emloft.net>;
>> Eric
>> > Dumazet <edumazet@...gle.com>; Jakub Kicinski <kuba@...nel.org>;
>> > Paolo Abeni <pabeni@...hat.com>; shaojijie <shaojijie@...wei.com>;
>> > wangpeiyang <wangpeiyang1@...wei.com>; netdev@...r.kernel.org;
>> > linux-kernel@...r.kernel.org
>> > Subject: Re: [PATCH] [net-next] net: hns3: add IOMMU_SUPPORT
>> > dependency
>> >
>> > On 2024-11-04 8:21 am, Arnd Bergmann wrote:
>> > > From: Arnd Bergmann <arnd@...db.de> > > The hns3 driver started
>> > filling iommu_iotlb_gather structures itself, > which requires
>> > CONFIG_IOMMU_SUPPORT is enabled:
>> > >
>> > > drivers/net/ethernet/hisilicon/hns3/hns3_enet.c: In function
>> > 'hns3_dma_map_sync':
>> > > drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:395:14: error:
>> > 'struct iommu_iotlb_gather' has no member named 'start'
>> > > 395 | iotlb_gather.start = iova;
>> > > | ^
>> > > drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:396:14: error:
>> > 'struct iommu_iotlb_gather' has no member named 'end'
>> > > 396 | iotlb_gather.end = iova + granule - 1;
>> > > | ^
>> > > drivers/net/ethernet/hisilicon/hns3/hns3_enet.c:397:14: error:
>> > 'struct iommu_iotlb_gather' has no member named 'pgsize'
>> > > 397 | iotlb_gather.pgsize = granule;
>> > > | ^
>> > >
>> > > Add a Kconfig dependency to make it build in random configurations.
>> > >
>> > > Cc: Will Deacon <will@...nel.org>
>> > > Cc: Joerg Roedel <jroedel@...e.de>
>> > > Cc: Robin Murphy <robin.murphy@....com> > Cc:
>> > iommu@...ts.linux.dev > Fixes: f2c14899caba ("net: hns3: add sync
>> > command to sync io-pgtable") > Signed-off-by: Arnd Bergmann
>> > <arnd@...db.de> > --- > I noticed that no other driver does this, so
>> > it would be good to have > a confirmation from the iommu maintainers
>> > that this is how the > interface and the dependency is intended to be
>> > used.
>> >
>> > WTF is that patch doing!? No, random device drivers should absolutely
>> > not be poking into IOMMU driver internals, this is egregiously wrong
>> > and the correct action is to drop it entirely.
>>
>>
>> Absolutely agree with it. Sorry I haven't been in touch for quite some time.
>> Let me catch the whole story. Feel free to drop this patch.
>
>
> Just to make it clear I meant the culprit patch:
> https://lore.kernel.org/netdev/20241025092938.2912958-3-shaojijie@huawei.com/
Right, if the HIP09 SMMU has a bug which requires an
iommu_domain_ops::iotlb_sync_map workaround, then the SMMU driver should
detect the HIP09 SMMU and implement that workaround for it. HNS3 trying
to reach in to the SMMU driver's data and open-code iotlb_sync_map on
its behalf is just as plain illogical as it is unacceptable.
Thanks,
Robin.
Powered by blists - more mailing lists