linux-kernel - Re: [PATCH v1 3/8] iommu: Extend iommu_at[de]tach

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:   Mon, 14 Feb 2022 09:03:13 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Joerg Roedel <joro@...tes.org>
Cc:     Lu Baolu <baolu.lu@...ux.intel.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Robin Murphy <robin.murphy@....com>,
        Christoph Hellwig <hch@...radead.org>,
        Kevin Tian <kevin.tian@...el.com>,
        Ashok Raj <ashok.raj@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Bjorn Helgaas <bhelgaas@...gle.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 v1 3/8] iommu: Extend iommu_at[de]tach_device() for
 multi-device groups

On Mon, Feb 14, 2022 at 12:39:36PM +0100, Joerg Roedel wrote:

> This extends iommu_attach_device() to behave as iommu_attach_group(),
> changing the domain for the whole group. 

Of course, the only action to take is to change the domain of a
group..

> Wouldn't it be better to scrap the iommu_attach_device() interface
> instead and only rely on iommu_attach_group()? This way it is clear
> that a call changes the whole group.

>From an API design perspective drivers should never touch groups -
they have struct devices, they should have a clean struct device based
API.

Groups should disappear into an internal implementation detail, not be
so prominent in the API.

> IIUC this work is heading towards allowing multiple domains in one group
> as long as the group is owned by one entity.

No, it isn't. This work is only about properly arbitrating which
single domain is attached to an entire group.

> 	1) Introduce a concept of a sub-group (or whatever we want to
> 	   call it), which groups devices together which must be in the
> 	   same domain because they use the same request ID and thus
> 	   look all the same to the IOMMU.
>
> 	2) Keep todays IOMMU groups to group devices together which can
> 	   bypass the IOMMU when talking to each other, like
> 	   multi-function devices and devices behind a no-ACS bridge.

We've talked about all these details before and nobody has thought
they are important enough to implement. This distinction is not the
goal of this series.

I think if someone did want to do this there is room in the API to
allow the distinction between 1 (must share) and 2 (sharing is
insecure). eg by checking owner and blocking mixing user/kernel. 

This is another reason to stick with the device centric API as if we
did someday want multi-domain groups then the device input is still
the correct input and the iommu code can figure out what sub-groups or
whatever transparently.

Jason