linux-kernel - Re: [PATCH 4/4] iommu/vt-d: Remove lazy allocation of domains

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]

Message-Id: <A7A831AB-A7A0-407F-A7E3-B84E354E9986@arista.com>
Date:   Sat, 9 Mar 2019 11:49:46 +0000
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 4/4] iommu/vt-d: Remove lazy allocation of domains

Hey Lu,

> On 9 Mar 2019, at 01:53, Lu Baolu <baolu.lu@...ux.intel.com> wrote:
> 
> Hi James,
> 
> On 3/9/19 12:57 AM, James Sewart wrote:
>> Hey Lu,
>>> On 8 Mar 2019, at 03:09, Lu Baolu<baolu.lu@...ux.intel.com>  wrote:
>>>>> 
>>>>> Do you mind if I work on top of your patches for further cleanups and
>>>>> sign off a v2 together with you?
>>>> Sure, sounds good. I’ll fixup patch 3 and have a go at integrating
>>>> iommu_prepare_isa into get_resv_regions. This should make the initial
>>>> domain logic here quite concise.
>>> Here attached three extra patches which I think should be added before
>>> PATCH 3/4, and some further cleanup changes which you can merge with
>>> PATCH 4/4.
>>> 
>>> ----------------
>>> 
>>> 0006-iommu-Add-ops-entry-for-vendor-specific-default-doma.patch
>>> 0007-iommu-vt-d-Add-is_identity_map-ops-entry.patch
>>> 
>>> These two patches aim to add a generic method for vendor specific iommu
>>> drivers to specify the type of the default domain for a device. Intel
>>> iommu driver will register an ops for this since it already has its own
>>> identity map adjudicator for a long time.
>> This seems like a good idea, but as domain alloc is only called for the
>> default domain on first device attached to a group, we may miss checking
>> whether a device added later should have an identity domain. Should there
>> be paths to downgrade a groups domain if one of the devices requires one?
> 
> Good catch!
> 
> This is supposed to be handled in iommu_no_mapping(). But, obviously
> current code sticks to lazy domain allocation. I'm not sure whether
> there are any real such cases, but we should handle it in a clean way.
> My idea is that we could downgrade to multiple domains per group (just
> like what we have now) in this case and print a kernel message for this.

I think if a device requires an identity domain, then it should ignore 
attempts to attach to something else. A print to warn a user about this 
would be a good idea.

I figure during attach: if iommu_no_mapping() then attach to si_domain and 
print, else continue with the given domain.

> 
> Or any other thoughts?
> 
> Best regards,
> Lu Baolu

Cheers,
James.