linux-kernel - Re: [PATCH v4 07/13] iommu: Add iommu_at[de]tach_device

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, 24 Dec 2021 10:24:04 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Joerg Roedel <joro@...tes.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, Will Deacon <will@...nel.org>,
        Dan Williams <dan.j.williams@...el.com>, rafael@...nel.org,
        Diana Craciun <diana.craciun@....nxp.com>,
        Cornelia Huck <cohuck@...hat.com>,
        Eric Auger <eric.auger@...hat.com>,
        Liu Yi L <yi.l.liu@...el.com>,
        Jacob jun Pan <jacob.jun.pan@...el.com>,
        Chaitanya Kulkarni <kch@...dia.com>,
        Stuart Yoder <stuyoder@...il.com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Thierry Reding <thierry.reding@...il.com>,
        David Airlie <airlied@...ux.ie>,
        Daniel Vetter <daniel@...ll.ch>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Li Yang <leoyang.li@....com>,
        Dmitry Osipenko <digetx@...il.com>,
        iommu@...ts.linux-foundation.org, linux-pci@...r.kernel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 07/13] iommu: Add iommu_at[de]tach_device_shared() for
 multi-device groups

On Fri, Dec 24, 2021 at 11:19:44AM +0800, Lu Baolu wrote:

> Let me summarize what I've got from above comments.
> 
> 1. Essentially we only need below interfaces for device drivers to
>    manage the I/O address conflict in iommu layer:
> 
> int iommu_device_set/release/query_kernel_dma(struct device *dev)
> 
> - Device driver lets the iommu layer know that driver DMAs go through
>   the kernel DMA APIs. The iommu layer should use the default domain
>   for DMA remapping. No other domains could be attached.
> - Device driver lets the iommu layer know that driver doesn't do DMA
>   anymore and other domains are allowed to be attached.
> - Device driver queries "can I only do DMA through the kernel DMA API?
>   In other words, can I attach my own domain?"

I'm not sure I see the utility of a query, but OK - this is the API
family v4 has added to really_probe, basically.

> int iommu_device_set/release_private_dma(struct device *dev)
> 
> - Device driver lets the iommu layer know that it wants to use its own
>   iommu domain. The iommu layer should detach the default domain and
>   allow the driver to attach or detach its own domain through
>   iommu_attach/detach_device() interfaces.
> - Device driver lets the iommy layer know that it on longer needs a
>   private domain.

Drivers don't actually need an interface like this, they all have
domains so they can all present their domain when they want to change
the ownership mode.

The advantage of presenting the domain in the API is that it allows
the core code to support sharing. Present the same domain and your
device gets to join the group. Present a different domain and it is
rejected. Simple.

Since there is no domain the above APIs cannot support tegra, for
instance.

>   Make the iommu_attach_device() the only and generic interface for the
>   device drivers to use their own private domain (I/O address space)
>   and replace all iommu_attach_group() uses with iommu_attach_device()
>   and deprecate the former.

Certainly in the devices drivers yes, VFIO should stay with group as
I've explained.

Ideals aside, we still need to have this series to have a scope that
is achievable in a reasonable size. So, we still end up with three
interfaces:

 1) iommu_attach_device() as used by the 11 current drivers that do
    not set suppress_auto_claim_dma_owner.
    It's key property is that it is API compatible with what we have
    today and doesn't require changing the 11 drivers.

 2) iommu_attach_device_shared() which is used by tegra and requires
    that drivers set suppress_auto_claim_dma_owner.

    A followup series could replace all calls of iommu_attach_device()
    with iommu_attach_device_shared() with one patch per driver that
    also sets suppress_auto_claim_dma_owner.

 3) Unless a better idea aries the
    iommu_group_set_dma_owner()/iommu_replace_group_domain()
    API that I suggested, used only by VFIO. This API is designed to
    work without a domain and uses the 'struct file *owner' instead
    of the domain to permit sharing. It swaps the obviously confusing
    concept of _USER for the more general concept of 'replace domain'.

All three need to consistently use the owner_cnt and related to
implement their internal logic.

It is a pretty clear explanation why there are three interfaces.

Jason