linux-kernel - Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv

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, 15 Apr 2019 15:16:59 +0100
From:   James Sewart <jamessewart@...sta.com>
To:     Lu Baolu <baolu.lu@...ux.intel.com>
Cc:     iommu@...ts.linux-foundation.org, Tom Murphy <tmurphy@...sta.com>,
        Dmitry Safonov <dima@...sta.com>,
        Jacob Pan <jacob.jun.pan@...ux.intel.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/7] iommu/vt-d: Expose ISA direct mapping region via
 iommu_get_resv_regions

Hey Lu,

> On 10 Apr 2019, at 06:22, Lu Baolu <baolu.lu@...ux.intel.com> wrote:
> 
> Hi James,
> 
> On 4/6/19 2:02 AM, James Sewart wrote:
>> Hey Lu,
>> My bad, did some debugging on my end. The issue was swapping out
>> find_domain for iommu_get_domain_for_dev. It seems in some situations the
>> domain is not attached to the group but the device is expected to have the
>> domain still stored in its archdata.
>> I’ve attached the final patch with find_domain unremoved which seems to
>> work in my testing.
> 
> Just looked into your v3 patch set and some thoughts from my end posted
> here just for your information.
> 
> Let me post the problems we want to address.
> 
> 1. When allocating a new group for a device, how should we determine the
> type of the default domain?
> 
> 2. If we need to put a device into an existing group which uses a
> different type of domain from what the device desires to use, we might
> break the functionality of the device.
> 
> My new thought is letting the iommu generic code to determine the
> default domain type (hence my proposed vendor specific default domain
> type patches could be dropped).
> 
> If the default domain type is dynamical mapping, and later in iommu_no_mapping(), we determines that we must use an identity domain,
> we then call iommu_request_dm_for_dev(dev).
> 
> If the default domain type is identity mapping, and later in
> iommu_no_mapping(), we determined that we must use a dynamical domain,
> we then call iommu_request_dma_domain_for_dev(dev).
> 
> We already have iommu_request_dm_for_dev() in iommu.c. We only need to
> implement iommu_request_dma_domain_for_dev().
> 
> With this done, your patch titled "Create an IOMMU group for devices
> that require an identity map" could also be dropped.
> 
> Any thoughts?

Theres a couple issues I can think of. iommu_request_dm_for_dev changes 
the domain for all devices within the devices group, if we may have 
devices with different domain requirements in the same group then only the 
last initialised device will get the domain it wants. If we want to add 
iommu_request_dma_domain_for_dev then we would still need another IOMMU 
group for identity domain devices.

Both with v3 and the proposed method here, iommu_should_identity_map is 
determining whether the device requires an identity map. In v3 this is 
called once up front by the generic code to determine for the IOMMU group 
which domain type to use. In the proposed method I think this would happen 
after initial domain allocation and would trigger the domain to be 
replaced with a different domain.

I prefer the solution in v3. What do you think?

> 
>> Cheers,
>> James.
> 
> Best regards,
> Lu Baolu

Cheers,
James.