From: Robin Murphy <robin.murphy@arm.com>
To: Lu Baolu <baolu.lu@linux.intel.com>, Joerg Roedel <joro@8bytes.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Bjorn Helgaas <bhelgaas@google.com>,
ashok.raj@intel.com, jacob.jun.pan@intel.com,
kevin.tian@intel.com, Christoph Hellwig <hch@lst.de>,
iommu@lists.linux-foundation.org, linux-kernel@vger.kernel.org
Subject: Re: [RFC PATCH 3/4] iommu: Preallocate iommu group when probing devices
Date: Thu, 23 Jan 2020 14:55:37 +0000 [thread overview]
Message-ID: <c7c23d49-bd44-a78c-bb83-de665737a5f8@arm.com> (raw)
In-Reply-To: <f04c590b-d966-88e8-7309-e73b600d4e9f@linux.intel.com>
On 22/01/2020 5:39 am, Lu Baolu wrote:
> Hi Robin,
>
> On 1/21/20 8:45 PM, Robin Murphy wrote:
>> On 19/01/2020 6:29 am, Lu Baolu wrote:
>>> Hi Joerg,
>>>
>>> On 1/17/20 6:21 PM, Joerg Roedel wrote:
>>>> On Wed, Jan 01, 2020 at 01:26:47PM +0800, Lu Baolu wrote:
>>>>> This splits iommu group allocation from adding devices. This makes
>>>>> it possible to determine the default domain type for each group as
>>>>> all devices belonging to the group have been determined.
>>>>
>>>> I think its better to keep group allocation as it is and just defer
>>>> default domain allocation after each device is in its group. But take
>>>
>>> I tried defering default domain allocation, but it seems not possible.
>>>
>>> The call path of adding devices into their groups:
>>>
>>> iommu_probe_device
>>> -> ops->add_device(dev)
>>> -> (iommu vendor driver) iommu_group_get_for_dev(dev)
>>>
>>> After doing this, the vendor driver will get the default domain and
>>> apply dma_ops according to the domain type. If we defer the domain
>>> allocation, they will get a NULL default domain and cause panic in
>>> the vendor driver.
>>>
>>> Any suggestions?
>>
>> https://lore.kernel.org/linux-iommu/6dbbfc10-3247-744c-ae8d-443a336e0c50@linux.intel.com/
>>
>>
>> Haven't we been here before? ;)
>>
>> Since we can't (safely or reasonably) change a group's default domain
>> after ops->add_device() has returned, and in general it gets
>> impractical to evaluate "all device in a group" once you look beyond
>> &pci_bus_type (or consider hotplug as mentioned), then AFAICS there's
>> no reasonable way to get away from the default domain type being
>> defined by the first device to attach.
>
> Yes, agreed.
>
>> But in practice it's hardly a problem anyway - if every device in a
>> given group requests the same domain type then it doesn't matter which
>> comes first, and if they don't then we ultimately end up with an
>> impossible set of constraints, so are doomed to do the 'wrong' thing
>> regardless.
>
> The third case is, for example, three devices A, B, C in a group. The
> first device A is neutral about which type of default domain type is
> used. So the iommu framework will use a static default domain. But the
> device B requires to use a specific one which is different from the
> default. Currently, this is handled in the vendor iommu driver and one
> motivation of this patch set is to handle this in the generic layer.
Yes, I wasn't explicitly considering that particular case, but it mostly
falls out more or less the same way. Given that multi-device groups
*should* be relatively rare, for the user override it seems reasonable
to expect the user to see when devices get grouped and specify all of
them to achieve the desired result; the trusted/untrusted attribute
definitely shouldn't differ within any given group; and
opportunistically replacing passthrough domains with translation domains
for DMA-limited devices can only ever be a best-effort thing without
consistent results, since at best that still comes down to which driver
probed and called dma_set_mask() first.
Platform-specific exceptions like in device_def_domain_type() probably
do want to stay in the individual drivers, but rolling that up into
default domain allocation would be neat, and functionally no worse than
the existing process.
In principle we could fairly easily delay allocating a group's default
domain until the first driver bind event. It wouldn't help universally -
in the absolute worst case, device B might only be created at all by
device A's driver probing - and it might need careful coordination in
areas like the bus->dma_configure() flow, but it could at least help
accommodate the more common PCI case.
Robin.
next prev parent reply other threads:[~2020-01-23 14:55 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-01 5:26 [RFC PATCH 0/4] iommu: Per-group default domain type Lu Baolu
2020-01-01 5:26 ` [RFC PATCH 1/4] driver core: Add iommu_passthrough to struct device Lu Baolu
2020-01-01 5:26 ` [RFC PATCH 2/4] PCI: Add "pci=iommu_passthrough=" parameter for iommu passthrough Lu Baolu
2020-01-18 0:18 ` Bjorn Helgaas
2020-01-18 2:04 ` Lu Baolu
2020-01-21 14:17 ` Bjorn Helgaas
2020-01-22 4:49 ` Lu Baolu
2020-01-01 5:26 ` [RFC PATCH 3/4] iommu: Preallocate iommu group when probing devices Lu Baolu
2020-01-17 10:21 ` Joerg Roedel
2020-01-18 2:18 ` Lu Baolu
2020-01-19 6:29 ` Lu Baolu
2020-01-21 12:45 ` Robin Murphy
2020-01-22 5:39 ` Lu Baolu
2020-01-23 14:55 ` Robin Murphy [this message]
2020-01-01 5:26 ` [RFC PATCH 4/4] iommu: Determine default domain type before allocating domain Lu Baolu
2020-01-20 9:44 ` [RFC PATCH 0/4] iommu: Per-group default domain type John Garry
2020-01-21 0:43 ` Lu Baolu
2020-01-21 10:14 ` John Garry
2020-01-22 4:58 ` Lu Baolu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c7c23d49-bd44-a78c-bb83-de665737a5f8@arm.com \
--to=robin.murphy@arm.com \
--cc=ashok.raj@intel.com \
--cc=baolu.lu@linux.intel.com \
--cc=bhelgaas@google.com \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=iommu@lists.linux-foundation.org \
--cc=jacob.jun.pan@intel.com \
--cc=joro@8bytes.org \
--cc=kevin.tian@intel.com \
--cc=linux-kernel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).