netdev - Re: [PATCH] [net-next] net: hns3: add IOMMU

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: <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.