Re: [PATCH V2 3/5] iommu: Add support to change default domain of an iommu_group

[Lu Baolu <baolu.lu@linux.intel.com>]

Hi Joerg,

On 2020/3/2 23:08, Joerg Roedel wrote:
> Hello Sai, Baolu,
> 
> On Sun, Feb 16, 2020 at 01:57:26PM -0800, Sai Praneeth Prakhya wrote:
>> Hence it will be helpful if there is some way to change the default
>> domain of a B:D.F dynamically. Since, linux iommu subsystem prefers to
>> deal at iommu_group level instead of B:D.F level, it might be helpful
>> if there is some way to change the default domain of a *iommu_group*
>> dynamically. Hence, add such support.
> 
> The question is how this plays together with the per-device private
> domains in the Intel VT-d driver. I recently debugged an issue there and
> I think there are more. The overall code for this seems to be pretty
> fragile, so I had the idea to make the private default domains more
> general.
> 
> IOMMU default domains don't necessarily need to stick to the iommu-group
> granularity, because the default domain is used by in-kernel drivers
> only, and the kernel trusts itself.
> 
> So my idea was to make the private-domain concept of the VT-d driver
> more generic and move it to the iommu core code. With that we can
> configure real per-device default domain types and don't have the race
> condition with driver probing when changing the default domain of
> multiple devices. We have to limit the ability to change default domain
> types to devices with no PCI aliases, but that should not be a problem
> for the intended use-case.
> 
> What do you think?
>

Theoretically speaking, per-device default domain is impractical. PCI
aliased devices (PCI bridge and all devices beneath it, VMD devices and
various devices quirked with pci_add_dma_alias()) must use the same
domain. It's likely that we have to introduce something like a sub-group
with all PCI aliased devices staying in it. Current private-domain
implementation in the vt-d driver was introduced for compatible purpose
and I wanted to abandon it from the first day. :-)

On Intel platforms, there are only rare devices which require a specific
default domain: some graphic devices (identity), a specific model of
AZALIA (identity) and external devices connected through thunderbolt
(dma). They are not supposed to belong to a same group. Hence, if we
are able to configure per-group default domain type, we don't need to
keep private domain anymore.

Probably, we are able to configure per-group default domain type with
below two interfaces.

- (ops->)dev_def_domain_type: Return the required default domain type
   for a device. It returns
   - IOMMU_DOMAIN_DMA (device must use a DMA domain), unlikely
   - IOMMU_DOMAIN_IDENTITY (device must use an Identity domain), unlikely
   - 0 (both are okay), likely

- iommu_group_change_def_domain: Change the default domain of a group
   Works only when all devices have no driver bond.

[Sai's patch set has already included these two interfaces.]

In iommu_probe_device(),

dev_def_type = ops->dev_def_domain_type(dev)
if (dev_def_type && dev_def_type != group->default_domain->type) {
	ret = iommu_group_change_def_domain(...)
	if (ret)
		return -EINVAL;
}

This should work during boot since iommu_probe_device() always happens
before device driver binding. We need to further consider the hot-plug
cases.

- Hardware initiated device hotplug
We should always use DMA domain for devices connected through an
external port to avoid DMA attacking from malicious devices. And
such devices shouldn't share a group with internal (trusted) devices.
Hence, I can't see any problems here.

- Software initiated device hotplug
The default domain type won't change before and after device hotplug
so there's no problem as well.

This is what I have for the private domain in vt-d driver. Just for
discussion.

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu