RE: [PATCH] [net-next] net: hns3: add IOMMU_SUPPORT dependency

From: Salil Mehta
Date: Mon Nov 04 2024 - 05:50:52 EST


> From: Salil Mehta <salil.mehta@xxxxxxxxxx>
> Sent: Monday, November 4, 2024 10:41 AM
> To: Robin Murphy <robin.murphy@xxxxxxx>; Arnd Bergmann
> <arnd@xxxxxxxxxx>; shenjian (K) <shenjian15@xxxxxxxxxx>
>
> HI Robin,
>
> > From: Robin Murphy <robin.murphy@xxxxxxx>
> > Sent: Monday, November 4, 2024 10:29 AM
> > To: Arnd Bergmann <arnd@xxxxxxxxxx>; shenjian (K)
> > <shenjian15@xxxxxxxxxx>; Salil Mehta <salil.mehta@xxxxxxxxxx>
> > Cc: Arnd Bergmann <arnd@xxxxxxxx>; Will Deacon <will@xxxxxxxxxx>;
> > Joerg Roedel <jroedel@xxxxxxx>; iommu@xxxxxxxxxxxxxxx; Andrew Lunn
> > <andrew+netdev@xxxxxxx>; David S. Miller <davem@xxxxxxxxxxxxx>;
> Eric
> > Dumazet <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>;
> > Paolo Abeni <pabeni@xxxxxxxxxx>; shaojijie <shaojijie@xxxxxxxxxx>;
> > wangpeiyang <wangpeiyang1@xxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> > linux-kernel@xxxxxxxxxxxxxxx
> > 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@xxxxxxxx> > > 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@xxxxxxxxxx>
> > > Cc: Joerg Roedel <jroedel@xxxxxxx>
> > > Cc: Robin Murphy <robin.murphy@xxxxxxx> > Cc:
> > iommu@xxxxxxxxxxxxxxx > Fixes: f2c14899caba ("net: hns3: add sync
> > command to sync io-pgtable") > Signed-off-by: Arnd Bergmann
> > <arnd@xxxxxxxx> > --- > 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@xxxxxxxxxx/


>
> Thanks
> Salil.
>
> >
> > Thanks,
> > Robin.
> >
> > > ---
> > > drivers/net/ethernet/hisilicon/Kconfig | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/net/ethernet/hisilicon/Kconfig
> > > b/drivers/net/ethernet/hisilicon/Kconfig
> > > index 65302c41bfb1..790efc8d2de6 100644 > ---
> > a/drivers/net/ethernet/hisilicon/Kconfig
> > > +++ b/drivers/net/ethernet/hisilicon/Kconfig
> > > @@ -91,6 +91,7 @@ config HNS_ENET
> > > config HNS3
> > > tristate "Hisilicon Network Subsystem Support HNS3 (Framework)"
> > > depends on PCI
> > > + depends on IOMMU_SUPPORT
> > > select NET_DEVLINK
> > > select PAGE_POOL
> > > help
> >