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:   Wed, 22 Dec 2021 12:22:11 +0800
From:   Lu Baolu <baolu.lu@...ux.intel.com>
To:     Jason Gunthorpe <jgg@...dia.com>,
        Robin Murphy <robin.murphy@....com>
Cc:     baolu.lu@...ux.intel.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 12/22/21 2:46 AM, Jason Gunthorpe wrote:
>> It's worth taking a step back and realising that overall, this is really
>> just a more generalised and finer-grained extension of what 426a273834ea
>> already did for non-group-aware code, so it makes little sense*not*  to
>> integrate it into the existing interfaces.
> This is taking 426a to it's logical conclusion and*removing*  the
> group API from the drivers entirely. This is desirable because drivers
> cannot do anything sane with the group.
> 
> The drivers have struct devices, and so we provide APIs that work in
> terms of struct devices to cover both driver use cases today, and do
> so more safely than what is already implemented.
> 
> Do not mix up VFIO with the driver interface, these are different
> things. It is better VFIO stay on its own and not complicate the
> driver world.

Per Joerg's previous comments:

https://lore.kernel.org/linux-iommu/20211119150612.jhsvsbzisvux2lga@8bytes.org/

The commit 426a273834ea came only in order to disallow attaching a
single device within a group to a different iommu_domain. So it's
reasonable to improve the existing iommu_attach/detach_device() to cover
all cases. How about below code? Did I miss anything?

int iommu_attach_device(struct iommu_domain *domain, struct device *dev)
{
         struct iommu_group *group;
         int ret = 0;

         group = iommu_group_get(dev);
         if (!group)
                 return -ENODEV;

         mutex_lock(&group->mutex);
         if (group->attach_cnt) {
                 if (group->domain != domain) {
                         ret = -EBUSY;
                         goto unlock_out;
                 }
         } else {
                 ret = __iommu_attach_group(domain, group);
                 if (ret)
                         goto unlock_out;
         }

         group->attach_cnt++;
unlock_out:
         mutex_unlock(&group->mutex);
         iommu_group_put(group);

         return ret;
}
EXPORT_SYMBOL_GPL(iommu_attach_device);

void iommu_detach_device_shared(struct iommu_domain *domain, struct 
device *dev)
{
         struct iommu_group *group;

         group = iommu_group_get(dev);
         if (WARN_ON(!group))
                 return;

         mutex_lock(&group->mutex);
         if (WARN_ON(!group->attach_cnt || group->domain != domain)
                 goto unlock_out;

         if (--group->attach_cnt == 0)
                 __iommu_detach_group(domain, group);

unlock_out:
         mutex_unlock(&group->mutex);
         iommu_group_put(group);
}
EXPORT_SYMBOL_GPL(iommu_detach_device);

Best regards,
baolu