linux-kernel - Re: [PATCH 3/5] iommu/arm-smmu-v3: add IOMMU_CAP

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:   Thu, 20 Jul 2017 12:17:05 +0100
From:   Will Deacon <will.deacon@....com>
To:     Anup Patel <anup.patel@...adcom.com>
Cc:     Robin Murphy <robin.murphy@....com>,
        Joerg Roedel <joro@...tes.org>,
        Baptiste Reynal <b.reynal@...tualopensystems.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Scott Branden <sbranden@...adcom.com>,
        Linux Kernel <linux-kernel@...r.kernel.org>,
        Linux ARM Kernel <linux-arm-kernel@...ts.infradead.org>,
        Linux IOMMU <iommu@...ts.linux-foundation.org>,
        kvm@...r.kernel.org,
        BCM Kernel Feedback <bcm-kernel-feedback-list@...adcom.com>
Subject: Re: [PATCH 3/5] iommu/arm-smmu-v3: add IOMMU_CAP_BYPASS to the ARM
 SMMUv3 driver

On Thu, Jul 20, 2017 at 04:38:09PM +0530, Anup Patel wrote:
> On Thu, Jul 20, 2017 at 2:40 PM, Will Deacon <will.deacon@....com> wrote:
> > On Thu, Jul 20, 2017 at 09:32:00AM +0530, Anup Patel wrote:
> >> On Wed, Jul 19, 2017 at 5:23 PM, Will Deacon <will.deacon@....com> wrote:
> >> > There are two things here:
> >> >
> >> >   1. iommu_present() is pretty useless, because it applies to a "bus" which
> >> >      doesn't actually tell you what you need to know for things like the
> >> >      platform_bus, where some masters might be upstream of an SMMU and
> >> >      others might not be.
> >>
> >> I agree with you. The iommu_present() check in vfio_iommu_group_get()
> >> is not much useful. We only reach line which checks iommu_present()
> >> when iommu_group_get() returns NULL for given "struct device *". If there
> >> is no IOMMU group for a "struct device *" then it means there is no IOMMU
> >> HW doing translations for such device.
> >>
> >> If we drop the iommu_present() check (due to above reasons) in
> >> vfio_iommu_group_get() then we don't require the IOMMU_CAP_BYPASS
> >> and we can happily drop PATCH1, PATCH2, and PATCH3.
> >>
> >> I will remove the iommu_present() check in vfio_iommu_group_get()
> >> because it is only comes into actions when VFIO_NOIOMMU is
> >> enabled. This will also help us drop PATCH1-to-PATCH3.
> >
> > I don't think that's the right answer. Whilst iommu_present has obvious
> > shortcomings, its intention is clear: it should tell you whether a given
> > *device* is upstream of an IOMMU. So the right fix is to make this
> > per-device, instead of per-bus. Removing it altogether is worse than leaving
> > it like it is.
> >
> >> >   2. If a master *is* upstream of an IOMMU and you want to use no-IOMMU,
> >> >      then the VFIO no-IOMMU code needs to be extended so that it creates
> >> >      an IDENTITY domain on that IOMMU.
> >>
> >> The VFIO no-IOMMU mode is equivalent to Linux UIO hence having
> >> IDENTITY domain for VFIO no-IOMMU is not appropriate here.
> >
> > Can you elaborate on this please? I don't understand the argument you're
> > making. It's like saying "I don't like eggs, therefore I don't drive a
> > car".
> >
> 
> Like I said, VFIO no-IOMMU mode for a device means device transactions
> will not go through any IOMMU. That's why having IDENTITY domain for
> device using VFIO no-IOMMU is not semantically correct. The analogy you
> proposed does not apply here.

If you have a master that is wired through an IOMMU, then its transactions
go through that IOMMU and you can't avoid that. What you want is a way for
the IOMMU driver to make the IOMMU appear transparent to the device. That is
achieved by creating an IDENTITY domain, which sounds perfect for what
you're trying to do.

Will