linux-kernel - Re: [PATCH 1/8] iommu: Decouple 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]

Date:   Fri, 27 Jan 2023 15:19:55 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Baolu Lu <baolu.lu@...ux.intel.com>,
        Jason Gunthorpe <jgg@...dia.com>
Cc:     joro@...tes.org, will@...nel.org, hch@....de,
        iommu@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/8] iommu: Decouple iommu_present() from bus ops

On 2023-01-27 13:50, Baolu Lu wrote:
> On 2023/1/26 22:41, Jason Gunthorpe wrote:
>> On Thu, Jan 26, 2023 at 02:21:29PM +0000, Robin Murphy wrote:
>>
>>> The "check" is inherent in the fact that it's been called at all. VFIO
>>> noiommu*is*  an IOMMU driver in the sense that it provides a bare 
>>> minimum of
>>> IOMMU API functionality (i.e. creating groups), sufficient to support
>>> (careful) usage by VFIO drivers. There would not seem to be a legitimate
>>> reason for some*other*  driver to be specifically querying a device 
>>> while it
>>> is already bound to a VFIO driver (and thus may have a noiommu group).
>> Yes, the devices that VFIO assigns to its internal groups never leak
>> outside VFIO's control during their assignment - ie they are
>> continuously bound to VFIO never another driver.
>>
>> So no other driver can ever see the internal groups unless it is
>> messing around with devices it is not bound to 😄
> 
> Fair enough. I was thinking that probably we could make it like below:
> 
> /**
>   * device_iommu_mapped - Returns true when the device DMA is translated
>   *                       by an IOMMU
>   * @dev: Device to perform the check on
>   */
> static inline bool device_iommu_mapped(struct device *dev)
> {
>          return (dev->iommu && dev->iommu->iommu_dev);
> }
> 
> The iommu probe device code guarantees that dev->iommu->iommu_dev is
> valid only after the IOMMU driver's .probe_device returned successfully.
> 
> Any thoughts?

Heh, I actually wrote that helper yesterday for v2, but as an internal 
check for valid ops :)

The current implementation of device_iommu_mapped() just dates back to 
when dev->iommu_group was the only per-device thing we had, so in 
principle I don't have any conceptual objection to redefining it in 
terms of "device has ops" rather than "device has a group", but as 
things stand you'd still have to do something about PPC first (I know 
Jason had been pushing on that, but I've not kept track of where it got to).

Thanks,
Robin.