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] [day] [month] [year] [list]
Message-ID: <515b8ae869b94c849d02421d8d8767fd@huawei.com>
Date: Wed, 6 Nov 2024 19:39:19 +0000
From: Salil Mehta <salil.mehta@...wei.com>
To: Robin Murphy <robin.murphy@....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

Hi Robin,

Sorry for the late reply. I was on a leave yesterday.

>  From: Robin Murphy <robin.murphy@....com>
>  Sent: Monday, November 4, 2024 4:35 PM
>  To: Salil Mehta <salil.mehta@...wei.com>; Arnd Bergmann
>  <arnd@...nel.org>; shenjian (K) <shenjian15@...wei.com>
>  
>  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@...w
>  > ei.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.


Totally agree with that.  It breaks the design of the kernel fundamentally.
This was a mistake and on Monday I've requested the stakeholders to
address it exactly in the realms of what you have suggested above. 

Rest assured, we will address that in the correct way. Sorry, if this caused
any inconvenience to anymore including the MAINTAINER of `netdev`
I would have caught this much earlier had I been following the ML closely
but unfortunately I've not been in touch for long.

Thanks for the `nudge` though 😊

Cheers
Salil.


>  
>  Thanks,
>  Robin.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ