Re: [PATCH 1/1] iommu/vt-d: use DMA domain for real DMA devices and subdevices

["Derrick, Jonathan" <jonathan.derrick@intel.com>]

On Sun, 2020-04-12 at 11:50 +0800, Daniel Drake wrote:
> Hi Jon,
> 
> Thanks for picking this up. Apologies for my absence here - I wasn't
> able to work on this recently, but I'm back again now.
> 
> On Fri, Apr 10, 2020 at 3:32 AM Jon Derrick <jonathan.derrick@intel.com> wrote:
> > This becomes problematic if the real DMA device and the subdevices have
> > different addressing capabilities and some require translation. Instead we can
> > put the real DMA dev and any subdevices on the DMA domain. This change assigns
> > subdevices to the DMA domain, and moves the real DMA device to the DMA domain
> > if necessary.
> 
> Have you tested this with the real DMA device in identity mode?
> It is not quite working for me. (Again, I'm not using VMD here, but
> have looked closely and believe we're working under the same
> constraints)

It does work for me when real DMA device starts in Identity, but my
'real DMA device' doesn't do the DMA. It just provides the source-id.

Does your 'real DMA device' do DMA?
I suppose that could be the reason. You wouldn't want to change the
domain on the live device using the method I proposed.

> 
> First, the real DMA device gets added to the group:
>  pci 0000:00:17.0: Adding to iommu group 9
> (it's in IDENTITY mode here)
> 
> Then later, the first subdevice comes along, and these are the results:
>  pci 10000:00:00.0: [8086:02d7] type 00 class 0x010601
>  pci 10000:00:00.0: reg 0x10: [mem 0xae1a0000-0xae1a7fff]
>  pci 10000:00:00.0: reg 0x14: [mem 0xae1a8000-0xae1a80ff]
>  pci 10000:00:00.0: reg 0x18: [io  0x3090-0x3097]
>  pci 10000:00:00.0: reg 0x1c: [io  0x3080-0x3083]
>  pci 10000:00:00.0: reg 0x20: [io  0x3060-0x307f]
>  pci 10000:00:00.0: reg 0x24: [mem 0xae100000-0xae103fff]
>  pci 10000:00:00.0: PME# supported from D3hot
>  pci 10000:00:00.0: Adding to iommu group 9
>  pci 10000:00:00.0: DMAR: Failed to get a private domain.
> 
> That final message is added by your patch and indicates that it's not working.
> 
> This is because the subdevice got added to the iommu group before the
> code you added tried to change to the DMA domain.
> 
> It first gets added to the group through this call path:
>     intel_iommu_add_device
> -> iommu_group_get_for_dev
> -> iommu_group_add_device
> 
> Then, continuing within intel_iommu_add_device we get to the code you
> added, which tries to move the real DMA dev to DMA mode instead. It
> calls:
> 
>    intel_iommu_request_dma_domain_for_dev
> -> iommu_request_dma_domain_for_dev
> -> request_default_domain_for_dev
> 
> Which fails here:
>     /* Don't change mappings of existing devices */
>     ret = -EBUSY;
>     if (iommu_group_device_count(group) != 1)
>         goto out;
> 
> because we already have 2 devices in the group (the real DMA dev, plus
> the subdevice we're in the process of handling now).
> 

You're right. I see the message too, but it still works for me.

> Next I'll look into the iommu group rework that Baolu mentioned.
> 
> Thanks,
> Daniel