2020-02-19 23:57:20

by Jerry Snitselaar

[permalink] [raw]
Subject: question about iommu_need_mapping

Is it possible for a device to end up with dev->archdata.iommu == NULL
on iommu_need_mapping in the following instance:

1. iommu_group has dma domain for default
2. device gets private identity domain in intel_iommu_add_device
3. iommu_need_mapping gets called with that device.
4. dmar_remove_one_dev_info sets dev->archdata.iommu = NULL via unlink_domain_info.
5. request_default_domain_for_dev exits after checking that group->default_domain
exists, and group->default_domain->type is dma.
6. iommu_request_dma_domain_for_dev returns 0 from request_default_domain_for_dev
and a private dma domain isn't created for the device.

The case I was seeing went away with commit 9235cb13d7d1 ("iommu/vt-d:
Allow devices with RMRRs to use identity domain"), because it changed
which domain the group and devices were using, but it seems like it is
still a possibility with the code. Baolu, you mentioned possibly
removing the domain switch. Commit 98b2fffb5e27 ("iommu/vt-d: Handle
32bit device with identity default domain") makes it sound like the
domain switch is required.

Regards,
Jerry


2020-02-20 02:50:31

by Lu Baolu

[permalink] [raw]
Subject: Re: question about iommu_need_mapping

Hi Jerry,

On 2020/2/20 7:55, Jerry Snitselaar wrote:
> Is it possible for a device to end up with dev->archdata.iommu == NULL
> on iommu_need_mapping in the following instance:
>
> 1. iommu_group has dma domain for default
> 2. device gets private identity domain in intel_iommu_add_device
> 3. iommu_need_mapping gets called with that device.
> 4. dmar_remove_one_dev_info sets dev->archdata.iommu = NULL via
> unlink_domain_info.
> 5. request_default_domain_for_dev exits after checking that
> group->default_domain
>    exists, and group->default_domain->type is dma.
> 6. iommu_request_dma_domain_for_dev returns 0 from
> request_default_domain_for_dev
>    and a private dma domain isn't created for the device.
>

Yes. It's possible.

> The case I was seeing went away with commit 9235cb13d7d1 ("iommu/vt-d:
> Allow devices with RMRRs to use identity domain"), because it changed
> which domain the group and devices were using, but it seems like it is
> still a possibility with the code. Baolu, you mentioned possibly
> removing the domain switch. Commit 98b2fffb5e27 ("iommu/vt-d: Handle
> 32bit device with identity default domain") makes it sound like the
> domain switch is required.

It's more "nice to have" than "required" if the iommu driver doesn't
disable swiotlb explicitly. The device access of system memory higher
than the device's addressing capability could go through the bounced
buffer implemented in swiotlb.

Best regards,
baolu

2020-02-20 16:25:24

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: question about iommu_need_mapping

On Thu Feb 20 20, Lu Baolu wrote:
>Hi Jerry,
>
>On 2020/2/20 7:55, Jerry Snitselaar wrote:
>>Is it possible for a device to end up with dev->archdata.iommu == NULL
>>on iommu_need_mapping in the following instance:
>>
>>1. iommu_group has dma domain for default
>>2. device gets private identity domain in intel_iommu_add_device
>>3. iommu_need_mapping gets called with that device.
>>4. dmar_remove_one_dev_info sets dev->archdata.iommu = NULL via
>>unlink_domain_info.
>>5. request_default_domain_for_dev exits after checking that
>>group->default_domain
>>    exists, and group->default_domain->type is dma.
>>6. iommu_request_dma_domain_for_dev returns 0 from
>>request_default_domain_for_dev
>>    and a private dma domain isn't created for the device.
>>
>
>Yes. It's possible.
>
>>The case I was seeing went away with commit 9235cb13d7d1 ("iommu/vt-d:
>>Allow devices with RMRRs to use identity domain"), because it changed
>>which domain the group and devices were using, but it seems like it is
>>still a possibility with the code. Baolu, you mentioned possibly
>>removing the domain switch. Commit 98b2fffb5e27 ("iommu/vt-d: Handle
>>32bit device with identity default domain") makes it sound like the
>>domain switch is required.
>
>It's more "nice to have" than "required" if the iommu driver doesn't
>disable swiotlb explicitly. The device access of system memory higher
>than the device's addressing capability could go through the bounced
>buffer implemented in swiotlb.
>
>Best regards,
>baolu

Hi Baolu,

Would this mean switching to bounce_dma_ops instead?

Regards,
Jerry

>_______________________________________________
>iommu mailing list
>[email protected]
>https://lists.linuxfoundation.org/mailman/listinfo/iommu

2020-02-20 17:33:17

by Jerry Snitselaar

[permalink] [raw]
Subject: Re: question about iommu_need_mapping

On Thu Feb 20 20, Jerry Snitselaar wrote:
>On Thu Feb 20 20, Lu Baolu wrote:
>>Hi Jerry,
>>
>>On 2020/2/20 7:55, Jerry Snitselaar wrote:
>>>Is it possible for a device to end up with dev->archdata.iommu == NULL
>>>on iommu_need_mapping in the following instance:
>>>
>>>1. iommu_group has dma domain for default
>>>2. device gets private identity domain in intel_iommu_add_device
>>>3. iommu_need_mapping gets called with that device.
>>>4. dmar_remove_one_dev_info sets dev->archdata.iommu = NULL via
>>>unlink_domain_info.
>>>5. request_default_domain_for_dev exits after checking that
>>>group->default_domain
>>>   exists, and group->default_domain->type is dma.
>>>6. iommu_request_dma_domain_for_dev returns 0 from
>>>request_default_domain_for_dev
>>>   and a private dma domain isn't created for the device.
>>>
>>
>>Yes. It's possible.
>>
>>>The case I was seeing went away with commit 9235cb13d7d1 ("iommu/vt-d:
>>>Allow devices with RMRRs to use identity domain"), because it changed
>>>which domain the group and devices were using, but it seems like it is
>>>still a possibility with the code. Baolu, you mentioned possibly
>>>removing the domain switch. Commit 98b2fffb5e27 ("iommu/vt-d: Handle
>>>32bit device with identity default domain") makes it sound like the
>>>domain switch is required.
>>
>>It's more "nice to have" than "required" if the iommu driver doesn't
>>disable swiotlb explicitly. The device access of system memory higher
>>than the device's addressing capability could go through the bounced
>>buffer implemented in swiotlb.
>>
>>Best regards,
>>baolu
>
>Hi Baolu,
>
>Would this mean switching to bounce_dma_ops instead?
>

Never mind. I see that it would go into the dma_direct code.

>Regards,
>Jerry
>
>>_______________________________________________
>>iommu mailing list
>>[email protected]
>>https://lists.linuxfoundation.org/mailman/listinfo/iommu