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

From: Salil Mehta
Date: Mon Nov 04 2024 - 05:41:44 EST


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.

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
>