[PATCH v3 0/8] iommu/vt-d: Delegate DMA domain to generic iommu

[PATCH v3 0/8] iommu/vt-d: Delegate DMA domain to generic iommu

[Lu Baolu]

Hi,

This patchset delegates the iommu DMA domain management to the
generic iommu layer. It avoids the use of find_or_alloc_domain
whose domain assignment is inconsistent with the iommu grouping
as determined by pci_device_group.

The major change is to permit domains of type IOMMU_DOMAIN_DMA
and IOMMU_DOMAIN_IDENTITY to be allocated via the iommu_ops api.
This allows the default_domain of an iommu group to be set in
iommu.c. This domain will be attached to every device that is
brought up with an iommu group, and the devices reserved regions
will be mapped using regions returned by get_resv_regions.

The default domain implementation defines a default domain type
and a domain of the default domain type will be allocated and
attached to devices which belong to a same group. Unfortunately,
this doesn't work for some quirky devices which is known to only
work with a specific domain type. PATCH 1/8 adds an iommu ops
which allows the IOMMU driver to return whether a device requires
a specific domain type, otherwise the staic defined type will be
applied.

Other changes are limited within the Intel IOMMU driver. They
mainly allow the driver to adapt to allocating domains, attaching
domains, applying and direct mapping reserved memory regions,
deferred domain attachment, and so on, via the iommu_ops api's.

This patchset was initiated by James Sewart. The v1 and v2 were
posted here [1] [2] for discussion. Lu Baolu took over the work
for testing and bug fixing with permission from James Sewart.

Reference:
[1] https://lkml.org/lkml/2019/3/4/644
[2] https://lkml.org/lkml/2019/3/14/299

Best regards,
Lu Baolu

Change log:
 v2->v3:
  - Add supported default domain type callback.
  - Make the iommu map() callback work even the domain is not
    attached.
  - Add domain deferred attach when iommu is pre-enabled in
    kdump kernel. 

 v1->v2:
  - https://lkml.org/lkml/2019/3/14/299
  - Refactored ISA direct mappings to be returned by
    iommu_get_resv_regions.
  - Integrated patch by Lu to defer turning on DMAR until iommu.c
    has mapped reserved regions.
  - Integrated patches by Lu to remove more unused code in cleanup.

 v1:
  -Original post https://lkml.org/lkml/2019/3/4/644

James Sewart (4):
  iommu/vt-d: Implement apply_resv_region iommu ops entry
  iommu/vt-d: Expose ISA direct mapping region via
    iommu_get_resv_regions
  iommu/vt-d: Allow DMA domains to be allocated by iommu ops
  iommu/vt-d: Remove lazy allocation of domains

Lu Baolu (4):
  iommu: Add ops entry for supported default domain type
  iommu/vt-d: Enable DMA remapping after rmrr mapped
  iommu/vt-d: Implement def_domain_type iommu ops entry
  iommu/vt-d: Implement is_attach_deferred iommu ops entry

 drivers/iommu/intel-iommu.c | 630 +++++++++++-------------------------
 drivers/iommu/iommu.c       |  13 +-
 include/linux/iommu.h       |  11 +
 3 files changed, 210 insertions(+), 444 deletions(-)

-- 
2.17.1

[PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Lu Baolu]

This adds an optional ops entry to query the default domain
types supported by the iommu driver for  a specific device.
This is necessary in cases where the iommu driver can only
support a specific type of default domain for a device. In
normal cases, this ops will return IOMMU_DOMAIN_ANY which
indicates that the iommu driver supports both IOMMU_DOMAIN_DMA
and IOMMU_DOMAIN_IDENTITY, hence the static default domain
type will be used.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/iommu.c | 13 ++++++++++---
 include/linux/iommu.h | 11 +++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index acd6830e6e9b..1ad9a1f2e078 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1097,15 +1097,22 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
 	 * IOMMU driver.
 	 */
 	if (!group->default_domain) {
+		unsigned int domain_type = IOMMU_DOMAIN_ANY;
 		struct iommu_domain *dom;
 
-		dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
-		if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
+		if (ops->def_domain_type)
+			domain_type = ops->def_domain_type(dev);
+
+		if (domain_type == IOMMU_DOMAIN_ANY)
+			domain_type = iommu_def_domain_type;
+
+		dom = __iommu_domain_alloc(dev->bus, domain_type);
+		if (!dom && domain_type != IOMMU_DOMAIN_DMA) {
 			dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
 			if (dom) {
 				dev_warn(dev,
 					 "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
-					 iommu_def_domain_type);
+					 domain_type);
 			}
 		}
 
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 8239ece9fdfc..ba9a5b996a63 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -79,12 +79,16 @@ struct iommu_domain_geometry {
  *	IOMMU_DOMAIN_DMA	- Internally used for DMA-API implementations.
  *				  This flag allows IOMMU drivers to implement
  *				  certain optimizations for these domains
+ *	IOMMU_DOMAIN_ANY	- All domain types defined here
  */
 #define IOMMU_DOMAIN_BLOCKED	(0U)
 #define IOMMU_DOMAIN_IDENTITY	(__IOMMU_DOMAIN_PT)
 #define IOMMU_DOMAIN_UNMANAGED	(__IOMMU_DOMAIN_PAGING)
 #define IOMMU_DOMAIN_DMA	(__IOMMU_DOMAIN_PAGING |	\
 				 __IOMMU_DOMAIN_DMA_API)
+#define IOMMU_DOMAIN_ANY	(IOMMU_DOMAIN_IDENTITY |	\
+				 IOMMU_DOMAIN_UNMANAGED |	\
+				 IOMMU_DOMAIN_DMA)
 
 struct iommu_domain {
 	unsigned type;
@@ -196,6 +200,11 @@ enum iommu_dev_features {
  * @dev_feat_enabled: check enabled feature
  * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
  * @aux_get_pasid: get the pasid given an aux-domain
+ * @def_domain_type: get per-device default domain type that the IOMMU
+ *		driver is able to support. Valid returns values:
+ *		- IOMMU_DOMAIN_DMA: only suports non-identity domain
+ *		- IOMMU_DOMAIN_IDENTITY: only supports identity domain
+ *		- IOMMU_DOMAIN_ANY: supports all
  * @pgsize_bitmap: bitmap of all possible supported page sizes
  */
 struct iommu_ops {
@@ -251,6 +260,8 @@ struct iommu_ops {
 	void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
 	int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
 
+	int (*def_domain_type)(struct device *dev);
+
 	unsigned long pgsize_bitmap;
 };
 
-- 
2.17.1

Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Tom Murphy]

The AMD driver already solves this problem and uses the generic
iommu_request_dm_for_dev function. It seems like both drivers have the
same problem and could use the same solution. Is there any reason we
can't have use the same solution for the intel and amd driver?

Could we just  copy the implementation of the AMD driver? It would be
nice to have the same behavior across both drivers especially as we
move to make both drivers use more generic code.

On Mon, Apr 29, 2019 at 3:16 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> This adds an optional ops entry to query the default domain
> types supported by the iommu driver for  a specific device.
> This is necessary in cases where the iommu driver can only
> support a specific type of default domain for a device. In
> normal cases, this ops will return IOMMU_DOMAIN_ANY which
> indicates that the iommu driver supports both IOMMU_DOMAIN_DMA
> and IOMMU_DOMAIN_IDENTITY, hence the static default domain
> type will be used.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/iommu.c | 13 ++++++++++---
>  include/linux/iommu.h | 11 +++++++++++
>  2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index acd6830e6e9b..1ad9a1f2e078 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1097,15 +1097,22 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
>          * IOMMU driver.
>          */
>         if (!group->default_domain) {
> +               unsigned int domain_type = IOMMU_DOMAIN_ANY;
>                 struct iommu_domain *dom;
>
> -               dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
> -               if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
> +               if (ops->def_domain_type)
> +                       domain_type = ops->def_domain_type(dev);
> +
> +               if (domain_type == IOMMU_DOMAIN_ANY)
> +                       domain_type = iommu_def_domain_type;
> +
> +               dom = __iommu_domain_alloc(dev->bus, domain_type);
> +               if (!dom && domain_type != IOMMU_DOMAIN_DMA) {
>                         dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
>                         if (dom) {
>                                 dev_warn(dev,
>                                          "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
> -                                        iommu_def_domain_type);
> +                                        domain_type);
>                         }
>                 }
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 8239ece9fdfc..ba9a5b996a63 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -79,12 +79,16 @@ struct iommu_domain_geometry {
>   *     IOMMU_DOMAIN_DMA        - Internally used for DMA-API implementations.
>   *                               This flag allows IOMMU drivers to implement
>   *                               certain optimizations for these domains
> + *     IOMMU_DOMAIN_ANY        - All domain types defined here
>   */
>  #define IOMMU_DOMAIN_BLOCKED   (0U)
>  #define IOMMU_DOMAIN_IDENTITY  (__IOMMU_DOMAIN_PT)
>  #define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
>  #define IOMMU_DOMAIN_DMA       (__IOMMU_DOMAIN_PAGING |        \
>                                  __IOMMU_DOMAIN_DMA_API)
> +#define IOMMU_DOMAIN_ANY       (IOMMU_DOMAIN_IDENTITY |        \
> +                                IOMMU_DOMAIN_UNMANAGED |       \
> +                                IOMMU_DOMAIN_DMA)
>
>  struct iommu_domain {
>         unsigned type;
> @@ -196,6 +200,11 @@ enum iommu_dev_features {
>   * @dev_feat_enabled: check enabled feature
>   * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
>   * @aux_get_pasid: get the pasid given an aux-domain
> + * @def_domain_type: get per-device default domain type that the IOMMU
> + *             driver is able to support. Valid returns values:
> + *             - IOMMU_DOMAIN_DMA: only suports non-identity domain
> + *             - IOMMU_DOMAIN_IDENTITY: only supports identity domain
> + *             - IOMMU_DOMAIN_ANY: supports all
>   * @pgsize_bitmap: bitmap of all possible supported page sizes
>   */
>  struct iommu_ops {
> @@ -251,6 +260,8 @@ struct iommu_ops {
>         void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
>         int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
>
> +       int (*def_domain_type)(struct device *dev);
> +
>         unsigned long pgsize_bitmap;
>  };
>
> --
> 2.17.1
>

Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Robin Murphy]

On 06/05/2019 16:32, Tom Murphy via iommu wrote:
> The AMD driver already solves this problem and uses the generic
> iommu_request_dm_for_dev function. It seems like both drivers have the
> same problem and could use the same solution. Is there any reason we
> can't have use the same solution for the intel and amd driver?
> 
> Could we just  copy the implementation of the AMD driver? It would be
> nice to have the same behavior across both drivers especially as we
> move to make both drivers use more generic code.

TBH I don't think the API really needs to be involved at all here. 
Drivers can already not provide the requested default domain type if 
they don't support it, so as long as the driver can ensure that the 
device ends up with IOMMU or direct DMA ops as appropriate, I don't see 
any great problem with drivers just returning a passthrough domain when 
a DMA domain was requested, or vice versa (and logging a message that 
the requested type was overridden). The only type that we really do have 
to honour strictly is non-default (i.e. unmanaged) domains.

Robin.

> On Mon, Apr 29, 2019 at 3:16 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>> This adds an optional ops entry to query the default domain
>> types supported by the iommu driver for  a specific device.
>> This is necessary in cases where the iommu driver can only
>> support a specific type of default domain for a device. In
>> normal cases, this ops will return IOMMU_DOMAIN_ANY which
>> indicates that the iommu driver supports both IOMMU_DOMAIN_DMA
>> and IOMMU_DOMAIN_IDENTITY, hence the static default domain
>> type will be used.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/iommu.c | 13 ++++++++++---
>>   include/linux/iommu.h | 11 +++++++++++
>>   2 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index acd6830e6e9b..1ad9a1f2e078 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1097,15 +1097,22 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
>>           * IOMMU driver.
>>           */
>>          if (!group->default_domain) {
>> +               unsigned int domain_type = IOMMU_DOMAIN_ANY;
>>                  struct iommu_domain *dom;
>>
>> -               dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
>> -               if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
>> +               if (ops->def_domain_type)
>> +                       domain_type = ops->def_domain_type(dev);
>> +
>> +               if (domain_type == IOMMU_DOMAIN_ANY)
>> +                       domain_type = iommu_def_domain_type;
>> +
>> +               dom = __iommu_domain_alloc(dev->bus, domain_type);
>> +               if (!dom && domain_type != IOMMU_DOMAIN_DMA) {
>>                          dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
>>                          if (dom) {
>>                                  dev_warn(dev,
>>                                           "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
>> -                                        iommu_def_domain_type);
>> +                                        domain_type);
>>                          }
>>                  }
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 8239ece9fdfc..ba9a5b996a63 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -79,12 +79,16 @@ struct iommu_domain_geometry {
>>    *     IOMMU_DOMAIN_DMA        - Internally used for DMA-API implementations.
>>    *                               This flag allows IOMMU drivers to implement
>>    *                               certain optimizations for these domains
>> + *     IOMMU_DOMAIN_ANY        - All domain types defined here
>>    */
>>   #define IOMMU_DOMAIN_BLOCKED   (0U)
>>   #define IOMMU_DOMAIN_IDENTITY  (__IOMMU_DOMAIN_PT)
>>   #define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
>>   #define IOMMU_DOMAIN_DMA       (__IOMMU_DOMAIN_PAGING |        \
>>                                   __IOMMU_DOMAIN_DMA_API)
>> +#define IOMMU_DOMAIN_ANY       (IOMMU_DOMAIN_IDENTITY |        \
>> +                                IOMMU_DOMAIN_UNMANAGED |       \
>> +                                IOMMU_DOMAIN_DMA)
>>
>>   struct iommu_domain {
>>          unsigned type;
>> @@ -196,6 +200,11 @@ enum iommu_dev_features {
>>    * @dev_feat_enabled: check enabled feature
>>    * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
>>    * @aux_get_pasid: get the pasid given an aux-domain
>> + * @def_domain_type: get per-device default domain type that the IOMMU
>> + *             driver is able to support. Valid returns values:
>> + *             - IOMMU_DOMAIN_DMA: only suports non-identity domain
>> + *             - IOMMU_DOMAIN_IDENTITY: only supports identity domain
>> + *             - IOMMU_DOMAIN_ANY: supports all
>>    * @pgsize_bitmap: bitmap of all possible supported page sizes
>>    */
>>   struct iommu_ops {
>> @@ -251,6 +260,8 @@ struct iommu_ops {
>>          void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
>>          int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
>>
>> +       int (*def_domain_type)(struct device *dev);
>> +
>>          unsigned long pgsize_bitmap;
>>   };
>>
>> --
>> 2.17.1
>>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
> 

Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Lu Baolu]

Hi Robin,

On 5/7/19 6:28 PM, Robin Murphy wrote:
> On 06/05/2019 16:32, Tom Murphy via iommu wrote:
>> The AMD driver already solves this problem and uses the generic
>> iommu_request_dm_for_dev function. It seems like both drivers have the
>> same problem and could use the same solution. Is there any reason we
>> can't have use the same solution for the intel and amd driver?
>>
>> Could we just  copy the implementation of the AMD driver? It would be
>> nice to have the same behavior across both drivers especially as we
>> move to make both drivers use more generic code.
> 
> TBH I don't think the API really needs to be involved at all here. 
> Drivers can already not provide the requested default domain type if 
> they don't support it, so as long as the driver can ensure that the 
> device ends up with IOMMU or direct DMA ops as appropriate, I don't see 
> any great problem with drivers just returning a passthrough domain when 
> a DMA domain was requested, or vice versa (and logging a message that 
> the requested type was overridden). The only type that we really do have 
> to honour strictly is non-default (i.e. unmanaged) domains.

I agree with you that we only have to honor strictly the non-default
domains. But domain type saved in iommu_domain is consumed in iommu.c
and exposed to user through sysfs. It's not clean if the iommu driver
silently replace the default domain.

Best regards,
Lu Baolu

> 
> Robin.
> 
>> On Mon, Apr 29, 2019 at 3:16 AM Lu Baolu <baolu.lu@linux.intel.com> 
>> wrote:
>>>
>>> This adds an optional ops entry to query the default domain
>>> types supported by the iommu driver for  a specific device.
>>> This is necessary in cases where the iommu driver can only
>>> support a specific type of default domain for a device. In
>>> normal cases, this ops will return IOMMU_DOMAIN_ANY which
>>> indicates that the iommu driver supports both IOMMU_DOMAIN_DMA
>>> and IOMMU_DOMAIN_IDENTITY, hence the static default domain
>>> type will be used.
>>>
>>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>>> ---
>>>   drivers/iommu/iommu.c | 13 ++++++++++---
>>>   include/linux/iommu.h | 11 +++++++++++
>>>   2 files changed, 21 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>>> index acd6830e6e9b..1ad9a1f2e078 100644
>>> --- a/drivers/iommu/iommu.c
>>> +++ b/drivers/iommu/iommu.c
>>> @@ -1097,15 +1097,22 @@ struct iommu_group 
>>> *iommu_group_get_for_dev(struct device *dev)
>>>           * IOMMU driver.
>>>           */
>>>          if (!group->default_domain) {
>>> +               unsigned int domain_type = IOMMU_DOMAIN_ANY;
>>>                  struct iommu_domain *dom;
>>>
>>> -               dom = __iommu_domain_alloc(dev->bus, 
>>> iommu_def_domain_type);
>>> -               if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
>>> +               if (ops->def_domain_type)
>>> +                       domain_type = ops->def_domain_type(dev);
>>> +
>>> +               if (domain_type == IOMMU_DOMAIN_ANY)
>>> +                       domain_type = iommu_def_domain_type;
>>> +
>>> +               dom = __iommu_domain_alloc(dev->bus, domain_type);
>>> +               if (!dom && domain_type != IOMMU_DOMAIN_DMA) {
>>>                          dom = __iommu_domain_alloc(dev->bus, 
>>> IOMMU_DOMAIN_DMA);
>>>                          if (dom) {
>>>                                  dev_warn(dev,
>>>                                           "failed to allocate default 
>>> IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
>>> -                                        iommu_def_domain_type);
>>> +                                        domain_type);
>>>                          }
>>>                  }
>>>
>>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>>> index 8239ece9fdfc..ba9a5b996a63 100644
>>> --- a/include/linux/iommu.h
>>> +++ b/include/linux/iommu.h
>>> @@ -79,12 +79,16 @@ struct iommu_domain_geometry {
>>>    *     IOMMU_DOMAIN_DMA        - Internally used for DMA-API 
>>> implementations.
>>>    *                               This flag allows IOMMU drivers to 
>>> implement
>>>    *                               certain optimizations for these 
>>> domains
>>> + *     IOMMU_DOMAIN_ANY        - All domain types defined here
>>>    */
>>>   #define IOMMU_DOMAIN_BLOCKED   (0U)
>>>   #define IOMMU_DOMAIN_IDENTITY  (__IOMMU_DOMAIN_PT)
>>>   #define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
>>>   #define IOMMU_DOMAIN_DMA       (__IOMMU_DOMAIN_PAGING |        \
>>>                                   __IOMMU_DOMAIN_DMA_API)
>>> +#define IOMMU_DOMAIN_ANY       (IOMMU_DOMAIN_IDENTITY |        \
>>> +                                IOMMU_DOMAIN_UNMANAGED |       \
>>> +                                IOMMU_DOMAIN_DMA)
>>>
>>>   struct iommu_domain {
>>>          unsigned type;
>>> @@ -196,6 +200,11 @@ enum iommu_dev_features {
>>>    * @dev_feat_enabled: check enabled feature
>>>    * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
>>>    * @aux_get_pasid: get the pasid given an aux-domain
>>> + * @def_domain_type: get per-device default domain type that the IOMMU
>>> + *             driver is able to support. Valid returns values:
>>> + *             - IOMMU_DOMAIN_DMA: only suports non-identity domain
>>> + *             - IOMMU_DOMAIN_IDENTITY: only supports identity domain
>>> + *             - IOMMU_DOMAIN_ANY: supports all
>>>    * @pgsize_bitmap: bitmap of all possible supported page sizes
>>>    */
>>>   struct iommu_ops {
>>> @@ -251,6 +260,8 @@ struct iommu_ops {
>>>          void (*aux_detach_dev)(struct iommu_domain *domain, struct 
>>> device *dev);
>>>          int (*aux_get_pasid)(struct iommu_domain *domain, struct 
>>> device *dev);
>>>
>>> +       int (*def_domain_type)(struct device *dev);
>>> +
>>>          unsigned long pgsize_bitmap;
>>>   };
>>>
>>> -- 
>>> 2.17.1
>>>
>> _______________________________________________
>> iommu mailing list
>> iommu@lists.linux-foundation.org
>> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>>
> 

Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Robin Murphy]

On 09/05/2019 03:30, Lu Baolu wrote:
> Hi Robin,
> 
> On 5/7/19 6:28 PM, Robin Murphy wrote:
>> On 06/05/2019 16:32, Tom Murphy via iommu wrote:
>>> The AMD driver already solves this problem and uses the generic
>>> iommu_request_dm_for_dev function. It seems like both drivers have the
>>> same problem and could use the same solution. Is there any reason we
>>> can't have use the same solution for the intel and amd driver?
>>>
>>> Could we just  copy the implementation of the AMD driver? It would be
>>> nice to have the same behavior across both drivers especially as we
>>> move to make both drivers use more generic code.
>>
>> TBH I don't think the API really needs to be involved at all here. 
>> Drivers can already not provide the requested default domain type if 
>> they don't support it, so as long as the driver can ensure that the 
>> device ends up with IOMMU or direct DMA ops as appropriate, I don't 
>> see any great problem with drivers just returning a passthrough domain 
>> when a DMA domain was requested, or vice versa (and logging a message 
>> that the requested type was overridden). The only type that we really 
>> do have to honour strictly is non-default (i.e. unmanaged) domains.
> 
> I agree with you that we only have to honor strictly the non-default
> domains. But domain type saved in iommu_domain is consumed in iommu.c
> and exposed to user through sysfs. It's not clean if the iommu driver
> silently replace the default domain.

Right, I did get a bit ahead of myself there - the implicit step before 
that is to fix default domain allocation so that the core actually 
passes the relevant device which it has to hand, such that the IOMMU 
drivers *can* make the right decision up-front.

Robin.

Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Lu Baolu]

Hi Robin,

On 5/10/19 12:11 AM, Robin Murphy wrote:
> On 09/05/2019 03:30, Lu Baolu wrote:
>> Hi Robin,
>>
>> On 5/7/19 6:28 PM, Robin Murphy wrote:
>>> On 06/05/2019 16:32, Tom Murphy via iommu wrote:
>>>> The AMD driver already solves this problem and uses the generic
>>>> iommu_request_dm_for_dev function. It seems like both drivers have the
>>>> same problem and could use the same solution. Is there any reason we
>>>> can't have use the same solution for the intel and amd driver?
>>>>
>>>> Could we just  copy the implementation of the AMD driver? It would be
>>>> nice to have the same behavior across both drivers especially as we
>>>> move to make both drivers use more generic code.
>>>
>>> TBH I don't think the API really needs to be involved at all here. 
>>> Drivers can already not provide the requested default domain type if 
>>> they don't support it, so as long as the driver can ensure that the 
>>> device ends up with IOMMU or direct DMA ops as appropriate, I don't 
>>> see any great problem with drivers just returning a passthrough 
>>> domain when a DMA domain was requested, or vice versa (and logging a 
>>> message that the requested type was overridden). The only type that 
>>> we really do have to honour strictly is non-default (i.e. unmanaged) 
>>> domains.
>>
>> I agree with you that we only have to honor strictly the non-default
>> domains. But domain type saved in iommu_domain is consumed in iommu.c
>> and exposed to user through sysfs. It's not clean if the iommu driver
>> silently replace the default domain.
> 
> Right, I did get a bit ahead of myself there - the implicit step before 
> that is to fix default domain allocation so that the core actually 
> passes the relevant device which it has to hand, such that the IOMMU 
> drivers *can* make the right decision up-front.
> 

Yes, passing the relevant device when allocating the default domain so
that the IOMMU driver could make right decision seems to be a better
solution. Somebody can come up with a patch set to bring this up for
discussion. I won't include this in this patch set since it's not for
that purpose. I will follow the existing mechanism that is using on amd
and other iommu drivers.

Best regards,
Lu Baolu

Re: [PATCH v3 1/8] iommu: Add ops entry for supported default domain type

[Lu Baolu]

Hi Tom,

On 5/6/19 11:32 PM, Tom Murphy wrote:
> The AMD driver already solves this problem and uses the generic
> iommu_request_dm_for_dev function. It seems like both drivers have the
> same problem and could use the same solution. Is there any reason we
> can't have use the same solution for the intel and amd driver?

The iommu_request_dm_for_dev() works for vt-d driver as well. But it's
insufficient. Another case is the default domain type is IDENTITY, but
the iommu driver can only support DMA type. Is it possible to add an
iommu_request_dma_map_for_dev() for this purpose?

> 
> Could we just  copy the implementation of the AMD driver? It would be
> nice to have the same behavior across both drivers especially as we
> move to make both drivers use more generic code.

Yes, agreed.

Best regards,
Lu Baolu

> 
> On Mon, Apr 29, 2019 at 3:16 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>> This adds an optional ops entry to query the default domain
>> types supported by the iommu driver for  a specific device.
>> This is necessary in cases where the iommu driver can only
>> support a specific type of default domain for a device. In
>> normal cases, this ops will return IOMMU_DOMAIN_ANY which
>> indicates that the iommu driver supports both IOMMU_DOMAIN_DMA
>> and IOMMU_DOMAIN_IDENTITY, hence the static default domain
>> type will be used.
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/iommu.c | 13 ++++++++++---
>>   include/linux/iommu.h | 11 +++++++++++
>>   2 files changed, 21 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index acd6830e6e9b..1ad9a1f2e078 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -1097,15 +1097,22 @@ struct iommu_group *iommu_group_get_for_dev(struct device *dev)
>>           * IOMMU driver.
>>           */
>>          if (!group->default_domain) {
>> +               unsigned int domain_type = IOMMU_DOMAIN_ANY;
>>                  struct iommu_domain *dom;
>>
>> -               dom = __iommu_domain_alloc(dev->bus, iommu_def_domain_type);
>> -               if (!dom && iommu_def_domain_type != IOMMU_DOMAIN_DMA) {
>> +               if (ops->def_domain_type)
>> +                       domain_type = ops->def_domain_type(dev);
>> +
>> +               if (domain_type == IOMMU_DOMAIN_ANY)
>> +                       domain_type = iommu_def_domain_type;
>> +
>> +               dom = __iommu_domain_alloc(dev->bus, domain_type);
>> +               if (!dom && domain_type != IOMMU_DOMAIN_DMA) {
>>                          dom = __iommu_domain_alloc(dev->bus, IOMMU_DOMAIN_DMA);
>>                          if (dom) {
>>                                  dev_warn(dev,
>>                                           "failed to allocate default IOMMU domain of type %u; falling back to IOMMU_DOMAIN_DMA",
>> -                                        iommu_def_domain_type);
>> +                                        domain_type);
>>                          }
>>                  }
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index 8239ece9fdfc..ba9a5b996a63 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -79,12 +79,16 @@ struct iommu_domain_geometry {
>>    *     IOMMU_DOMAIN_DMA        - Internally used for DMA-API implementations.
>>    *                               This flag allows IOMMU drivers to implement
>>    *                               certain optimizations for these domains
>> + *     IOMMU_DOMAIN_ANY        - All domain types defined here
>>    */
>>   #define IOMMU_DOMAIN_BLOCKED   (0U)
>>   #define IOMMU_DOMAIN_IDENTITY  (__IOMMU_DOMAIN_PT)
>>   #define IOMMU_DOMAIN_UNMANAGED (__IOMMU_DOMAIN_PAGING)
>>   #define IOMMU_DOMAIN_DMA       (__IOMMU_DOMAIN_PAGING |        \
>>                                   __IOMMU_DOMAIN_DMA_API)
>> +#define IOMMU_DOMAIN_ANY       (IOMMU_DOMAIN_IDENTITY |        \
>> +                                IOMMU_DOMAIN_UNMANAGED |       \
>> +                                IOMMU_DOMAIN_DMA)
>>
>>   struct iommu_domain {
>>          unsigned type;
>> @@ -196,6 +200,11 @@ enum iommu_dev_features {
>>    * @dev_feat_enabled: check enabled feature
>>    * @aux_attach/detach_dev: aux-domain specific attach/detach entries.
>>    * @aux_get_pasid: get the pasid given an aux-domain
>> + * @def_domain_type: get per-device default domain type that the IOMMU
>> + *             driver is able to support. Valid returns values:
>> + *             - IOMMU_DOMAIN_DMA: only suports non-identity domain
>> + *             - IOMMU_DOMAIN_IDENTITY: only supports identity domain
>> + *             - IOMMU_DOMAIN_ANY: supports all
>>    * @pgsize_bitmap: bitmap of all possible supported page sizes
>>    */
>>   struct iommu_ops {
>> @@ -251,6 +260,8 @@ struct iommu_ops {
>>          void (*aux_detach_dev)(struct iommu_domain *domain, struct device *dev);
>>          int (*aux_get_pasid)(struct iommu_domain *domain, struct device *dev);
>>
>> +       int (*def_domain_type)(struct device *dev);
>> +
>>          unsigned long pgsize_bitmap;
>>   };
>>
>> --
>> 2.17.1
>>
> 

[PATCH v3 2/8] iommu/vt-d: Implement apply_resv_region iommu ops entry

[Lu Baolu]

From: James Sewart <jamessewart@arista.com>

Used by iommu.c before creating identity mappings for reserved
ranges to ensure dma-ops won't ever remap these ranges.

Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/iommu/intel-iommu.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e0c0febc6fa5..af025955f1bc 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -5568,6 +5568,19 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev)
 	return ret;
 }
 
+static void intel_iommu_apply_resv_region(struct device *dev,
+					  struct iommu_domain *domain,
+					  struct iommu_resv_region *region)
+{
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	unsigned long start, end;
+
+	start = IOVA_PFN(region->start);
+	end   = IOVA_PFN(region->start + region->length - 1);
+
+	WARN_ON_ONCE(!reserve_iova(&dmar_domain->iovad, start, end));
+}
+
 #ifdef CONFIG_INTEL_IOMMU_SVM
 struct intel_iommu *intel_svm_device_to_iommu(struct device *dev)
 {
@@ -5733,6 +5746,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.remove_device		= intel_iommu_remove_device,
 	.get_resv_regions	= intel_iommu_get_resv_regions,
 	.put_resv_regions	= intel_iommu_put_resv_regions,
+	.apply_resv_region	= intel_iommu_apply_resv_region,
 	.device_group		= pci_device_group,
 	.dev_has_feat		= intel_iommu_dev_has_feat,
 	.dev_feat_enabled	= intel_iommu_dev_feat_enabled,
-- 
2.17.1

[PATCH v3 3/8] iommu/vt-d: Expose ISA direct mapping region via iommu_get_resv_regions

[Lu Baolu]

From: James Sewart <jamessewart@arista.com>

To support mapping ISA region via iommu_group_create_direct_mappings,
make sure its exposed by iommu_get_resv_regions. This allows
deduplication of reserved region mappings.

Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/iommu/intel-iommu.c | 43 +++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 9 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index af025955f1bc..e9f31b062305 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -339,6 +339,8 @@ static LIST_HEAD(dmar_rmrr_units);
 #define for_each_rmrr_units(rmrr) \
 	list_for_each_entry(rmrr, &dmar_rmrr_units, list)
 
+static struct iommu_resv_region *isa_resv_region;
+
 /* bitmap for indexing intel_iommus */
 static int g_num_of_iommus;
 
@@ -2785,26 +2787,33 @@ static inline int iommu_prepare_rmrr_dev(struct dmar_rmrr_unit *rmrr,
 					  rmrr->end_address);
 }
 
+static inline struct iommu_resv_region *iommu_get_isa_resv_region(void)
+{
+	if (!isa_resv_region)
+		isa_resv_region = iommu_alloc_resv_region(0, 1UL << 24, 0,
+							  IOMMU_RESV_DIRECT);
+
+	return isa_resv_region;
+}
+
 #ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
-static inline void iommu_prepare_isa(void)
+static inline void iommu_prepare_isa(struct pci_dev *pdev)
 {
-	struct pci_dev *pdev;
 	int ret;
+	struct iommu_resv_region *reg = iommu_get_isa_resv_region();
 
-	pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
-	if (!pdev)
+	if (!reg)
 		return;
 
 	pr_info("Prepare 0-16MiB unity mapping for LPC\n");
-	ret = iommu_prepare_identity_map(&pdev->dev, 0, 16*1024*1024 - 1);
+	ret = iommu_prepare_identity_map(&pdev->dev, reg->start,
+					 reg->start + reg->length - 1);
 
 	if (ret)
 		pr_err("Failed to create 0-16MiB identity map - floppy might not work\n");
-
-	pci_dev_put(pdev);
 }
 #else
-static inline void iommu_prepare_isa(void)
+static inline void iommu_prepare_isa(struct pci_dev *pdev)
 {
 	return;
 }
@@ -3293,6 +3302,7 @@ static int __init init_dmars(void)
 	struct dmar_rmrr_unit *rmrr;
 	bool copied_tables = false;
 	struct device *dev;
+	struct pci_dev *pdev;
 	struct intel_iommu *iommu;
 	int i, ret;
 
@@ -3473,7 +3483,11 @@ static int __init init_dmars(void)
 		}
 	}
 
-	iommu_prepare_isa();
+	pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
+	if (pdev) {
+		iommu_prepare_isa(pdev);
+		pci_dev_put(pdev);
+	}
 
 domains_done:
 
@@ -5498,6 +5512,17 @@ static void intel_iommu_get_resv_regions(struct device *device,
 	}
 	rcu_read_unlock();
 
+#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
+	if (dev_is_pci(device)) {
+		struct pci_dev *pdev = to_pci_dev(device);
+
+		if ((pdev->class >> 8) == PCI_CLASS_BRIDGE_ISA) {
+			reg = iommu_get_isa_resv_region();
+			list_add_tail(&reg->list, head);
+		}
+	}
+#endif /* CONFIG_INTEL_IOMMU_FLOPPY_WA */
+
 	reg = iommu_alloc_resv_region(IOAPIC_RANGE_START,
 				      IOAPIC_RANGE_END - IOAPIC_RANGE_START + 1,
 				      0, IOMMU_RESV_MSI);
-- 
2.17.1

[PATCH v3 4/8] iommu/vt-d: Enable DMA remapping after rmrr mapped

[Lu Baolu]

The rmrr devices require identity map of the rmrr regions before
enabling DMA remapping. Otherwise, there will be a window during
which DMA from/to the rmrr regions will be blocked. In order to
alleviate this, we move enabling DMA remapping after all rmrr
regions get mapped.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e9f31b062305..847e4a326d29 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3527,11 +3527,6 @@ static int __init init_dmars(void)
 		ret = dmar_set_interrupt(iommu);
 		if (ret)
 			goto free_iommu;
-
-		if (!translation_pre_enabled(iommu))
-			iommu_enable_translation(iommu);
-
-		iommu_disable_protect_mem_regions(iommu);
 	}
 
 	return 0;
@@ -4933,7 +4928,6 @@ int __init intel_iommu_init(void)
 		goto out_free_reserved_range;
 	}
 	up_write(&dmar_global_lock);
-	pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
 
 #if defined(CONFIG_X86) && defined(CONFIG_SWIOTLB)
 	swiotlb = 0;
@@ -4956,6 +4950,16 @@ int __init intel_iommu_init(void)
 		register_memory_notifier(&intel_iommu_memory_nb);
 	cpuhp_setup_state(CPUHP_IOMMU_INTEL_DEAD, "iommu/intel:dead", NULL,
 			  intel_iommu_cpu_dead);
+
+	/* Finally, we enable the DMA remapping hardware. */
+	for_each_iommu(iommu, drhd) {
+		if (!translation_pre_enabled(iommu))
+			iommu_enable_translation(iommu);
+
+		iommu_disable_protect_mem_regions(iommu);
+	}
+	pr_info("Intel(R) Virtualization Technology for Directed I/O\n");
+
 	intel_iommu_enabled = 1;
 	intel_iommu_debugfs_init();
 
-- 
2.17.1

[PATCH v3 5/8] iommu/vt-d: Implement def_domain_type iommu ops entry

[Lu Baolu]

It will be used by the iommu generic framework to check the
default domain types that Intel IOMMU driver supports for a
specific device. Return IOMMU_DOMAIN_DMA if device requires
a non-identity domain;  Return IOMMU_DOMAIN_IDENTITY if the
device requires to use an identity domain. Otherwise return
IOMMU_DOMAIN_ANY which indicates that both domain types are
supported, hence the static default domain type defined in
iommu generic layer will be used.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 86 +++++++++++++------------------------
 1 file changed, 29 insertions(+), 57 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 847e4a326d29..d2b51e045603 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2858,9 +2858,6 @@ static int identity_mapping(struct device *dev)
 {
 	struct device_domain_info *info;
 
-	if (likely(!iommu_identity_mapping))
-		return 0;
-
 	info = dev->archdata.iommu;
 	if (info && info != DUMMY_DEVICE_DOMAIN_INFO)
 		return (info->domain == si_domain);
@@ -2945,29 +2942,26 @@ static bool device_is_rmrr_locked(struct device *dev)
 	return true;
 }
 
-static int iommu_should_identity_map(struct device *dev, int startup)
+static int intel_iommu_def_domain_type(struct device *dev)
 {
 	if (dev_is_pci(dev)) {
 		struct pci_dev *pdev = to_pci_dev(dev);
 
 		if (device_is_rmrr_locked(dev))
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 
 		/*
 		 * Prevent any device marked as untrusted from getting
 		 * placed into the statically identity mapping domain.
 		 */
 		if (pdev->untrusted)
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 
 		if ((iommu_identity_mapping & IDENTMAP_AZALIA) && IS_AZALIA(pdev))
-			return 1;
+			return IOMMU_DOMAIN_IDENTITY;
 
 		if ((iommu_identity_mapping & IDENTMAP_GFX) && IS_GFX_DEVICE(pdev))
-			return 1;
-
-		if (!(iommu_identity_mapping & IDENTMAP_ALL))
-			return 0;
+			return IOMMU_DOMAIN_IDENTITY;
 
 		/*
 		 * We want to start off with all devices in the 1:1 domain, and
@@ -2988,43 +2982,25 @@ static int iommu_should_identity_map(struct device *dev, int startup)
 		 */
 		if (!pci_is_pcie(pdev)) {
 			if (!pci_is_root_bus(pdev->bus))
-				return 0;
+				return IOMMU_DOMAIN_DMA;
 			if (pdev->class >> 8 == PCI_CLASS_BRIDGE_PCI)
-				return 0;
+				return IOMMU_DOMAIN_DMA;
 		} else if (pci_pcie_type(pdev) == PCI_EXP_TYPE_PCI_BRIDGE)
-			return 0;
+			return IOMMU_DOMAIN_DMA;
 	} else {
 		if (device_has_rmrr(dev))
-			return 0;
-	}
-
-	/*
-	 * At boot time, we don't yet know if devices will be 64-bit capable.
-	 * Assume that they will — if they turn out not to be, then we can
-	 * take them out of the 1:1 domain later.
-	 */
-	if (!startup) {
-		/*
-		 * If the device's dma_mask is less than the system's memory
-		 * size then this is not a candidate for identity mapping.
-		 */
-		u64 dma_mask = *dev->dma_mask;
-
-		if (dev->coherent_dma_mask &&
-		    dev->coherent_dma_mask < dma_mask)
-			dma_mask = dev->coherent_dma_mask;
-
-		return dma_mask >= dma_get_required_mask(dev);
+			return IOMMU_DOMAIN_DMA;
 	}
 
-	return 1;
+	return (iommu_identity_mapping & IDENTMAP_ALL) ?
+			IOMMU_DOMAIN_IDENTITY : IOMMU_DOMAIN_ANY;
 }
 
 static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw)
 {
 	int ret;
 
-	if (!iommu_should_identity_map(dev, 1))
+	if (intel_iommu_def_domain_type(dev) != IOMMU_DOMAIN_IDENTITY)
 		return 0;
 
 	ret = domain_add_dev_info(si_domain, dev);
@@ -3631,35 +3607,30 @@ static int iommu_no_mapping(struct device *dev)
 	if (iommu_dummy(dev))
 		return 1;
 
-	if (!iommu_identity_mapping)
-		return 0;
-
 	found = identity_mapping(dev);
 	if (found) {
-		if (iommu_should_identity_map(dev, 0))
-			return 1;
-		else {
+		/*
+		 * If the device's dma_mask is less than the system's memory
+		 * size then this is not a candidate for identity mapping.
+		 */
+		u64 dma_mask = *dev->dma_mask;
+
+		if (dev->coherent_dma_mask &&
+		    dev->coherent_dma_mask < dma_mask)
+			dma_mask = dev->coherent_dma_mask;
+
+		if (dma_mask < dma_get_required_mask(dev)) {
 			/*
 			 * 32 bit DMA is removed from si_domain and fall back
 			 * to non-identity mapping.
 			 */
 			dmar_remove_one_dev_info(dev);
-			dev_info(dev, "32bit DMA uses non-identity mapping\n");
+			dev_warn(dev, "32bit DMA uses non-identity mapping\n");
+
 			return 0;
 		}
-	} else {
-		/*
-		 * In case of a detached 64 bit DMA device from vm, the device
-		 * is put into si_domain for identity mapping.
-		 */
-		if (iommu_should_identity_map(dev, 0)) {
-			int ret;
-			ret = domain_add_dev_info(si_domain, dev);
-			if (!ret) {
-				dev_info(dev, "64bit DMA uses identity mapping\n");
-				return 1;
-			}
-		}
+
+		return 1;
 	}
 
 	return 0;
@@ -4605,7 +4576,7 @@ static int device_notifier(struct notifier_block *nb,
 		    list_empty(&domain->devices))
 			domain_exit(domain);
 	} else if (action == BUS_NOTIFY_ADD_DEVICE) {
-		if (iommu_should_identity_map(dev, 1))
+		if (intel_iommu_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY)
 			domain_add_dev_info(si_domain, dev);
 	}
 
@@ -5781,6 +5752,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.dev_feat_enabled	= intel_iommu_dev_feat_enabled,
 	.dev_enable_feat	= intel_iommu_dev_enable_feat,
 	.dev_disable_feat	= intel_iommu_dev_disable_feat,
+	.def_domain_type	= intel_iommu_def_domain_type,
 	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
-- 
2.17.1

Re: [PATCH v3 5/8] iommu/vt-d: Implement def_domain_type iommu ops entry

[Christoph Hellwig]

> @@ -3631,35 +3607,30 @@ static int iommu_no_mapping(struct device *dev)
>  	if (iommu_dummy(dev))
>  		return 1;
>  
> -	if (!iommu_identity_mapping)
> -		return 0;
> -

FYI, iommu_no_mapping has been refactored in for-next:

https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/?h=x86/vt-d&id=48b2c937ea37a3bece0094b46450ed5267525289

>  	found = identity_mapping(dev);
>  	if (found) {
> +		/*
> +		 * If the device's dma_mask is less than the system's memory
> +		 * size then this is not a candidate for identity mapping.
> +		 */
> +		u64 dma_mask = *dev->dma_mask;
> +
> +		if (dev->coherent_dma_mask &&
> +		    dev->coherent_dma_mask < dma_mask)
> +			dma_mask = dev->coherent_dma_mask;
> +
> +		if (dma_mask < dma_get_required_mask(dev)) {

I know this is mostly existing code moved around, but it really needs
some fixing.  For one dma_get_required_mask is supposed to return the
required to not bounce mask for the given device.  E.g. for a device
behind an iommu it should always just return 32-bit.  If you really
want to check vs system memory please call dma_direct_get_required_mask
without the dma_ops indirection.

Second I don't even think we need to check the coherent_dma_mask,
dma_direct is pretty good at always finding memory even without
an iommu.

Third this doesn't take take the bus_dma_mask into account.

This probably should just be:

		if (min(*dev->dma_mask, dev->bus_dma_mask) <
		    dma_direct_get_required_mask(dev)) {

Re: [PATCH v3 5/8] iommu/vt-d: Implement def_domain_type iommu ops entry

[Lu Baolu]

Hi Christoph,

On 4/30/19 4:03 AM, Christoph Hellwig wrote:
>> @@ -3631,35 +3607,30 @@ static int iommu_no_mapping(struct device *dev)
>>   	if (iommu_dummy(dev))
>>   		return 1;
>>   
>> -	if (!iommu_identity_mapping)
>> -		return 0;
>> -
> 
> FYI, iommu_no_mapping has been refactored in for-next:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/?h=x86/vt-d&id=48b2c937ea37a3bece0094b46450ed5267525289

Oh, yes! Thanks for letting me know this. Will rebase the code.

> 
>>   	found = identity_mapping(dev);
>>   	if (found) {
>> +		/*
>> +		 * If the device's dma_mask is less than the system's memory
>> +		 * size then this is not a candidate for identity mapping.
>> +		 */
>> +		u64 dma_mask = *dev->dma_mask;
>> +
>> +		if (dev->coherent_dma_mask &&
>> +		    dev->coherent_dma_mask < dma_mask)
>> +			dma_mask = dev->coherent_dma_mask;
>> +
>> +		if (dma_mask < dma_get_required_mask(dev)) {
> 
> I know this is mostly existing code moved around, but it really needs
> some fixing.  For one dma_get_required_mask is supposed to return the
> required to not bounce mask for the given device.  E.g. for a device
> behind an iommu it should always just return 32-bit.  If you really
> want to check vs system memory please call dma_direct_get_required_mask
> without the dma_ops indirection.
> 
> Second I don't even think we need to check the coherent_dma_mask,
> dma_direct is pretty good at always finding memory even without
> an iommu.
> 
> Third this doesn't take take the bus_dma_mask into account.
> 
> This probably should just be:
> 
> 		if (min(*dev->dma_mask, dev->bus_dma_mask) <
> 		    dma_direct_get_required_mask(dev)) {

Agreed and will add this in the next version.

Best regards,
Lu Baolu

Re: [PATCH v3 5/8] iommu/vt-d: Implement def_domain_type iommu ops entry

[Tom Murphy]

It looks like there is a bug in this code.

The behavior before this patch in __intel_map_single was that
iommu_no_mapping would call remove the attached si_domain for 32 bit
devices  (in the  dmar_remove_one_dev_info(dev) call in
iommu_no_mapping) and then allocate a new domain in
get_valid_domain_for_dev
old:
if (iommu_no_mapping(dev))
   return paddr;
domain = get_valid_domain_for_dev(dev);
if (!domain)
   return DMA_MAPPING_ERROR;

but in the new code we remove the attached si_domain but we WON'T
allocate a new domain and instead just return an error when we call
find_domain
new:
        if (iommu_no_mapping(dev))
                return paddr;

        domain = find_domain(dev);
        if (!domain)
                return DMA_MAPPING_ERROR;

This is a bug, right?

On Tue, Apr 30, 2019 at 3:18 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> Hi Christoph,
>
> On 4/30/19 4:03 AM, Christoph Hellwig wrote:
> >> @@ -3631,35 +3607,30 @@ static int iommu_no_mapping(struct device *dev)
> >>      if (iommu_dummy(dev))
> >>              return 1;
> >>
> >> -    if (!iommu_identity_mapping)
> >> -            return 0;
> >> -
> >
> > FYI, iommu_no_mapping has been refactored in for-next:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/?h=x86/vt-d&id=48b2c937ea37a3bece0094b46450ed5267525289
>
> Oh, yes! Thanks for letting me know this. Will rebase the code.
>
> >
> >>      found = identity_mapping(dev);
> >>      if (found) {
> >> +            /*
> >> +             * If the device's dma_mask is less than the system's memory
> >> +             * size then this is not a candidate for identity mapping.
> >> +             */
> >> +            u64 dma_mask = *dev->dma_mask;
> >> +
> >> +            if (dev->coherent_dma_mask &&
> >> +                dev->coherent_dma_mask < dma_mask)
> >> +                    dma_mask = dev->coherent_dma_mask;
> >> +
> >> +            if (dma_mask < dma_get_required_mask(dev)) {
> >
> > I know this is mostly existing code moved around, but it really needs
> > some fixing.  For one dma_get_required_mask is supposed to return the
> > required to not bounce mask for the given device.  E.g. for a device
> > behind an iommu it should always just return 32-bit.  If you really
> > want to check vs system memory please call dma_direct_get_required_mask
> > without the dma_ops indirection.
> >
> > Second I don't even think we need to check the coherent_dma_mask,
> > dma_direct is pretty good at always finding memory even without
> > an iommu.
> >
> > Third this doesn't take take the bus_dma_mask into account.
> >
> > This probably should just be:
> >
> >               if (min(*dev->dma_mask, dev->bus_dma_mask) <
> >                   dma_direct_get_required_mask(dev)) {
>
> Agreed and will add this in the next version.
>
> Best regards,
> Lu Baolu

Re: [PATCH v3 5/8] iommu/vt-d: Implement def_domain_type iommu ops entry

[Lu Baolu]

Hi,

On 5/6/19 11:25 PM, Tom Murphy wrote:
> It looks like there is a bug in this code.
> 
> The behavior before this patch in __intel_map_single was that
> iommu_no_mapping would call remove the attached si_domain for 32 bit
> devices  (in the  dmar_remove_one_dev_info(dev) call in
> iommu_no_mapping) and then allocate a new domain in
> get_valid_domain_for_dev
> old:
> if (iommu_no_mapping(dev))
>     return paddr;
> domain = get_valid_domain_for_dev(dev);
> if (!domain)
>     return DMA_MAPPING_ERROR;
> 
> but in the new code we remove the attached si_domain but we WON'T
> allocate a new domain and instead just return an error when we call
> find_domain
> new:
>          if (iommu_no_mapping(dev))
>                  return paddr;
> 
>          domain = find_domain(dev);
>          if (!domain)
>                  return DMA_MAPPING_ERROR;
> 
> This is a bug, right?

When we use the old lazy creation of iommu domain, we can change the
domain for a 32bit device from identity to dma by pulling it out of the
si_domain and allocating a new one for it.

When we switch to default domain in iommu generic layer, we can't do
this anymore. The logic in above code is if we find this case (32bit
device using an identity domain), we simple return error for dma api
and warn the user "hey, this is a 32bit device, don't use the default
pass-through mode".

I believe there should be better solutions, for example, how about
letting pci core to call iommu_request_dma_map_for_dev() when it
finds a 32bit device.

Best regards,
Lu Baolu

> 
> On Tue, Apr 30, 2019 at 3:18 AM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>>
>> Hi Christoph,
>>
>> On 4/30/19 4:03 AM, Christoph Hellwig wrote:
>>>> @@ -3631,35 +3607,30 @@ static int iommu_no_mapping(struct device *dev)
>>>>       if (iommu_dummy(dev))
>>>>               return 1;
>>>>
>>>> -    if (!iommu_identity_mapping)
>>>> -            return 0;
>>>> -
>>>
>>> FYI, iommu_no_mapping has been refactored in for-next:
>>>
>>> https://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git/commit/?h=x86/vt-d&id=48b2c937ea37a3bece0094b46450ed5267525289
>>
>> Oh, yes! Thanks for letting me know this. Will rebase the code.
>>
>>>
>>>>       found = identity_mapping(dev);
>>>>       if (found) {
>>>> +            /*
>>>> +             * If the device's dma_mask is less than the system's memory
>>>> +             * size then this is not a candidate for identity mapping.
>>>> +             */
>>>> +            u64 dma_mask = *dev->dma_mask;
>>>> +
>>>> +            if (dev->coherent_dma_mask &&
>>>> +                dev->coherent_dma_mask < dma_mask)
>>>> +                    dma_mask = dev->coherent_dma_mask;
>>>> +
>>>> +            if (dma_mask < dma_get_required_mask(dev)) {
>>>
>>> I know this is mostly existing code moved around, but it really needs
>>> some fixing.  For one dma_get_required_mask is supposed to return the
>>> required to not bounce mask for the given device.  E.g. for a device
>>> behind an iommu it should always just return 32-bit.  If you really
>>> want to check vs system memory please call dma_direct_get_required_mask
>>> without the dma_ops indirection.
>>>
>>> Second I don't even think we need to check the coherent_dma_mask,
>>> dma_direct is pretty good at always finding memory even without
>>> an iommu.
>>>
>>> Third this doesn't take take the bus_dma_mask into account.
>>>
>>> This probably should just be:
>>>
>>>                if (min(*dev->dma_mask, dev->bus_dma_mask) <
>>>                    dma_direct_get_required_mask(dev)) {
>>
>> Agreed and will add this in the next version.
>>
>> Best regards,
>> Lu Baolu
> 

[PATCH v3 6/8] iommu/vt-d: Allow DMA domains to be allocated by iommu ops

[Lu Baolu]

From: James Sewart <jamessewart@arista.com>

Allowing IOMMU_DOMAIN_DMA and IOMMU_DOMAIN_IDENTITY types of
domains to be allocated through iommu ops allows the default
domain of an iommu_group to be set. This delegates domains of
Intel IOMMU driver to the generic IOMMU layer.

Signed-off-by: James Sewart <jamessewart@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 124 ++++++++++++++++++++++++++----------
 1 file changed, 90 insertions(+), 34 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d2b51e045603..ec6ac39827ab 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -311,6 +311,12 @@ static int hw_pass_through = 1;
 /* si_domain contains mulitple devices */
 #define DOMAIN_FLAG_STATIC_IDENTITY	(1 << 1)
 
+/*
+ * Domain managed externally, don't cleanup if it isn't attached
+ * to any devices.
+ */
+#define DOMAIN_FLAG_MANAGED_EXTERNALLY	(1 << 2)
+
 #define for_each_domain_iommu(idx, domain)			\
 	for (idx = 0; idx < g_num_of_iommus; idx++)		\
 		if (domain->iommu_refcnt[idx])
@@ -561,6 +567,11 @@ static inline int domain_type_is_vm_or_si(struct dmar_domain *domain)
 				DOMAIN_FLAG_STATIC_IDENTITY);
 }
 
+static inline int domain_managed_externally(struct dmar_domain *domain)
+{
+	return domain->flags & DOMAIN_FLAG_MANAGED_EXTERNALLY;
+}
+
 static inline int domain_pfn_supported(struct dmar_domain *domain,
 				       unsigned long pfn)
 {
@@ -1671,7 +1682,7 @@ static void disable_dmar_iommu(struct intel_iommu *iommu)
 
 		__dmar_remove_one_dev_info(info);
 
-		if (!domain_type_is_vm_or_si(domain)) {
+		if (!domain_managed_externally(domain)) {
 			/*
 			 * The domain_exit() function  can't be called under
 			 * device_domain_lock, as it takes this lock itself.
@@ -2366,7 +2377,12 @@ static int domain_mapping(struct dmar_domain *domain, unsigned long iov_pfn,
 	} else {
 		/* General domains only have one IOMMU */
 		iommu = domain_get_iommu(domain);
-		__mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
+		/*
+		 * There's no associated iommu if domain hasn't been attached
+		 * to any device yet.
+		 */
+		if (iommu)
+			__mapping_notify_one(iommu, domain, iov_pfn, nr_pages);
 	}
 
 	return 0;
@@ -3406,12 +3422,9 @@ static int __init init_dmars(void)
 
 	check_tylersburg_isoch();
 
-	if (iommu_identity_mapping) {
-		ret = si_domain_init(hw_pass_through);
-		if (ret)
-			goto free_iommu;
-	}
-
+	ret = si_domain_init(hw_pass_through);
+	if (ret)
+		goto free_iommu;
 
 	/*
 	 * If we copied translations from a previous kernel in the kdump
@@ -4572,7 +4585,7 @@ static int device_notifier(struct notifier_block *nb,
 			return 0;
 
 		dmar_remove_one_dev_info(dev);
-		if (!domain_type_is_vm_or_si(domain) &&
+		if (!domain_managed_externally(domain) &&
 		    list_empty(&domain->devices))
 			domain_exit(domain);
 	} else if (action == BUS_NOTIFY_ADD_DEVICE) {
@@ -5038,33 +5051,52 @@ static struct iommu_domain *intel_iommu_domain_alloc(unsigned type)
 {
 	struct dmar_domain *dmar_domain;
 	struct iommu_domain *domain;
+	int flags = DOMAIN_FLAG_MANAGED_EXTERNALLY;
 
-	if (type != IOMMU_DOMAIN_UNMANAGED)
-		return NULL;
+	switch (type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		flags |= DOMAIN_FLAG_VIRTUAL_MACHINE;
+		/* fall through */
+	case IOMMU_DOMAIN_DMA:
+		dmar_domain = alloc_domain(flags);
+		if (!dmar_domain)
+			return NULL;
 
-	dmar_domain = alloc_domain(DOMAIN_FLAG_VIRTUAL_MACHINE);
-	if (!dmar_domain) {
-		pr_err("Can't allocate dmar_domain\n");
-		return NULL;
-	}
-	if (md_domain_init(dmar_domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
-		pr_err("Domain initialization failed\n");
-		domain_exit(dmar_domain);
+		if (md_domain_init(dmar_domain, DEFAULT_DOMAIN_ADDRESS_WIDTH)) {
+			pr_err("Domain initialization failed\n");
+			domain_exit(dmar_domain);
+			return NULL;
+		}
+
+		if (type == IOMMU_DOMAIN_DMA &&
+		    init_iova_flush_queue(&dmar_domain->iovad,
+					  iommu_flush_iova, iova_entry_free)) {
+			pr_warn("iova flush queue initialization failed\n");
+			intel_iommu_strict = 1;
+		}
+
+		domain_update_iommu_cap(dmar_domain);
+		domain = &dmar_domain->domain;
+		domain->geometry.aperture_start = 0;
+		domain->geometry.aperture_end =
+			__DOMAIN_MAX_ADDR(dmar_domain->gaw);
+		domain->geometry.force_aperture = true;
+		break;
+	case IOMMU_DOMAIN_IDENTITY:
+		return &si_domain->domain;
+	default:
 		return NULL;
 	}
-	domain_update_iommu_cap(dmar_domain);
-
-	domain = &dmar_domain->domain;
-	domain->geometry.aperture_start = 0;
-	domain->geometry.aperture_end   = __DOMAIN_MAX_ADDR(dmar_domain->gaw);
-	domain->geometry.force_aperture = true;
 
-	return domain;
+	return &dmar_domain->domain;
 }
 
 static void intel_iommu_domain_free(struct iommu_domain *domain)
 {
-	domain_exit(to_dmar_domain(domain));
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+
+	if (domain_managed_externally(dmar_domain))
+		domain_exit(dmar_domain);
 }
 
 /*
@@ -5240,13 +5272,33 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 {
 	int ret;
 
-	if (device_is_rmrr_locked(dev)) {
-		dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement.  Contact your platform vendor.\n");
-		return -EPERM;
-	}
+	switch (domain->type) {
+	case IOMMU_DOMAIN_UNMANAGED:
+		if (device_is_rmrr_locked(dev)) {
+			dev_warn(dev, "Device is ineligible for IOMMU domain attach due to platform RMRR requirement. Contact your platform vendor.\n");
+			return -EPERM;
+		}
 
-	if (is_aux_domain(dev, domain))
-		return -EPERM;
+		if (is_aux_domain(dev, domain))
+			return -EPERM;
+
+		break;
+	case IOMMU_DOMAIN_DMA:
+		if (intel_iommu_def_domain_type(dev) == IOMMU_DOMAIN_IDENTITY) {
+			dev_warn(dev, "Device requires identity domain\n");
+			return -EINVAL;
+		}
+		break;
+	case IOMMU_DOMAIN_IDENTITY:
+		if (intel_iommu_def_domain_type(dev) == IOMMU_DOMAIN_DMA) {
+			dev_warn(dev, "Device requires non-identity domain\n");
+			return -EINVAL;
+		}
+		break;
+	default:
+		dev_warn(dev, "Invalid domain type %u\n", domain->type);
+		return -EINVAL;
+	}
 
 	/* normally dev is not mapped */
 	if (unlikely(domain_context_mapped(dev))) {
@@ -5258,7 +5310,7 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 			dmar_remove_one_dev_info(dev);
 			rcu_read_unlock();
 
-			if (!domain_type_is_vm_or_si(old_domain) &&
+			if (!domain_managed_externally(old_domain) &&
 			    list_empty(&old_domain->devices))
 				domain_exit(old_domain);
 		}
@@ -5307,6 +5359,10 @@ static int intel_iommu_map(struct iommu_domain *domain,
 	int prot = 0;
 	int ret;
 
+	/* Don't bother if hardware passthrough used. */
+	if (dmar_domain == si_domain && hw_pass_through)
+		return 0;
+
 	if (iommu_prot & IOMMU_READ)
 		prot |= DMA_PTE_READ;
 	if (iommu_prot & IOMMU_WRITE)
-- 
2.17.1

[PATCH v3 7/8] iommu/vt-d: Remove lazy allocation of domains

[Lu Baolu]

From: James Sewart <jamessewart@arista.com>

The generic IOMMU code will manage the life cycle of a default
domain for each device that comes online. This patch removes the
lazy domain allocation and early reserved region mapping that is
now redundant.

Signed-off-by: James Sewart <jamessewart@arista.com>
---
 drivers/iommu/intel-iommu.c | 356 +-----------------------------------
 1 file changed, 5 insertions(+), 351 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index ec6ac39827ab..dd6abd554804 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1862,63 +1862,6 @@ static inline int guestwidth_to_adjustwidth(int gaw)
 	return agaw;
 }
 
-static int domain_init(struct dmar_domain *domain, struct intel_iommu *iommu,
-		       int guest_width)
-{
-	int adjust_width, agaw;
-	unsigned long sagaw;
-	int err;
-
-	init_iova_domain(&domain->iovad, VTD_PAGE_SIZE, IOVA_START_PFN);
-
-	err = init_iova_flush_queue(&domain->iovad,
-				    iommu_flush_iova, iova_entry_free);
-	if (err)
-		return err;
-
-	domain_reserve_special_ranges(domain);
-
-	/* calculate AGAW */
-	if (guest_width > cap_mgaw(iommu->cap))
-		guest_width = cap_mgaw(iommu->cap);
-	domain->gaw = guest_width;
-	adjust_width = guestwidth_to_adjustwidth(guest_width);
-	agaw = width_to_agaw(adjust_width);
-	sagaw = cap_sagaw(iommu->cap);
-	if (!test_bit(agaw, &sagaw)) {
-		/* hardware doesn't support it, choose a bigger one */
-		pr_debug("Hardware doesn't support agaw %d\n", agaw);
-		agaw = find_next_bit(&sagaw, 5, agaw);
-		if (agaw >= 5)
-			return -ENODEV;
-	}
-	domain->agaw = agaw;
-
-	if (ecap_coherent(iommu->ecap))
-		domain->iommu_coherency = 1;
-	else
-		domain->iommu_coherency = 0;
-
-	if (ecap_sc_support(iommu->ecap))
-		domain->iommu_snooping = 1;
-	else
-		domain->iommu_snooping = 0;
-
-	if (intel_iommu_superpage)
-		domain->iommu_superpage = fls(cap_super_page_val(iommu->cap));
-	else
-		domain->iommu_superpage = 0;
-
-	domain->nid = iommu->node;
-
-	/* always allocate the top pgd */
-	domain->pgd = (struct dma_pte *)alloc_pgtable_page(domain->nid);
-	if (!domain->pgd)
-		return -ENOMEM;
-	__iommu_flush_cache(iommu, domain->pgd, PAGE_SIZE);
-	return 0;
-}
-
 static void domain_exit(struct dmar_domain *domain)
 {
 	struct page *freelist;
@@ -2600,118 +2543,6 @@ static struct dmar_domain *dmar_insert_one_dev_info(struct intel_iommu *iommu,
 	return domain;
 }
 
-static int get_last_alias(struct pci_dev *pdev, u16 alias, void *opaque)
-{
-	*(u16 *)opaque = alias;
-	return 0;
-}
-
-static struct dmar_domain *find_or_alloc_domain(struct device *dev, int gaw)
-{
-	struct device_domain_info *info;
-	struct dmar_domain *domain = NULL;
-	struct intel_iommu *iommu;
-	u16 dma_alias;
-	unsigned long flags;
-	u8 bus, devfn;
-
-	iommu = device_to_iommu(dev, &bus, &devfn);
-	if (!iommu)
-		return NULL;
-
-	if (dev_is_pci(dev)) {
-		struct pci_dev *pdev = to_pci_dev(dev);
-
-		pci_for_each_dma_alias(pdev, get_last_alias, &dma_alias);
-
-		spin_lock_irqsave(&device_domain_lock, flags);
-		info = dmar_search_domain_by_dev_info(pci_domain_nr(pdev->bus),
-						      PCI_BUS_NUM(dma_alias),
-						      dma_alias & 0xff);
-		if (info) {
-			iommu = info->iommu;
-			domain = info->domain;
-		}
-		spin_unlock_irqrestore(&device_domain_lock, flags);
-
-		/* DMA alias already has a domain, use it */
-		if (info)
-			goto out;
-	}
-
-	/* Allocate and initialize new domain for the device */
-	domain = alloc_domain(0);
-	if (!domain)
-		return NULL;
-	if (domain_init(domain, iommu, gaw)) {
-		domain_exit(domain);
-		return NULL;
-	}
-
-out:
-
-	return domain;
-}
-
-static struct dmar_domain *set_domain_for_dev(struct device *dev,
-					      struct dmar_domain *domain)
-{
-	struct intel_iommu *iommu;
-	struct dmar_domain *tmp;
-	u16 req_id, dma_alias;
-	u8 bus, devfn;
-
-	iommu = device_to_iommu(dev, &bus, &devfn);
-	if (!iommu)
-		return NULL;
-
-	req_id = ((u16)bus << 8) | devfn;
-
-	if (dev_is_pci(dev)) {
-		struct pci_dev *pdev = to_pci_dev(dev);
-
-		pci_for_each_dma_alias(pdev, get_last_alias, &dma_alias);
-
-		/* register PCI DMA alias device */
-		if (req_id != dma_alias) {
-			tmp = dmar_insert_one_dev_info(iommu, PCI_BUS_NUM(dma_alias),
-					dma_alias & 0xff, NULL, domain);
-
-			if (!tmp || tmp != domain)
-				return tmp;
-		}
-	}
-
-	tmp = dmar_insert_one_dev_info(iommu, bus, devfn, dev, domain);
-	if (!tmp || tmp != domain)
-		return tmp;
-
-	return domain;
-}
-
-static struct dmar_domain *get_domain_for_dev(struct device *dev, int gaw)
-{
-	struct dmar_domain *domain, *tmp;
-
-	domain = find_domain(dev);
-	if (domain)
-		goto out;
-
-	domain = find_or_alloc_domain(dev, gaw);
-	if (!domain)
-		goto out;
-
-	tmp = set_domain_for_dev(dev, domain);
-	if (!tmp || domain != tmp) {
-		domain_exit(domain);
-		domain = tmp;
-	}
-
-out:
-
-	return domain;
-}
-
 static int iommu_domain_identity_map(struct dmar_domain *domain,
 				     unsigned long long start,
 				     unsigned long long end)
@@ -2737,72 +2568,6 @@ static int iommu_domain_identity_map(struct dmar_domain *domain,
 				DMA_PTE_READ|DMA_PTE_WRITE);
 }
 
-static int domain_prepare_identity_map(struct device *dev,
-				       struct dmar_domain *domain,
-				       unsigned long long start,
-				       unsigned long long end)
-{
-	/* For _hardware_ passthrough, don't bother. But for software
-	   passthrough, we do it anyway -- it may indicate a memory
-	   range which is reserved in E820, so which didn't get set
-	   up to start with in si_domain */
-	if (domain == si_domain && hw_pass_through) {
-		dev_warn(dev, "Ignoring identity map for HW passthrough [0x%Lx - 0x%Lx]\n",
-			 start, end);
-		return 0;
-	}
-
-	dev_info(dev, "Setting identity map [0x%Lx - 0x%Lx]\n", start, end);
-
-	if (end < start) {
-		WARN(1, "Your BIOS is broken; RMRR ends before it starts!\n"
-			"BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-			dmi_get_system_info(DMI_BIOS_VENDOR),
-			dmi_get_system_info(DMI_BIOS_VERSION),
-		     dmi_get_system_info(DMI_PRODUCT_VERSION));
-		return -EIO;
-	}
-
-	if (end >> agaw_to_width(domain->agaw)) {
-		WARN(1, "Your BIOS is broken; RMRR exceeds permitted address width (%d bits)\n"
-		     "BIOS vendor: %s; Ver: %s; Product Version: %s\n",
-		     agaw_to_width(domain->agaw),
-		     dmi_get_system_info(DMI_BIOS_VENDOR),
-		     dmi_get_system_info(DMI_BIOS_VERSION),
-		     dmi_get_system_info(DMI_PRODUCT_VERSION));
-		return -EIO;
-	}
-
-	return iommu_domain_identity_map(domain, start, end);
-}
-
-static int iommu_prepare_identity_map(struct device *dev,
-				      unsigned long long start,
-				      unsigned long long end)
-{
-	struct dmar_domain *domain;
-	int ret;
-
-	domain = get_domain_for_dev(dev, DEFAULT_DOMAIN_ADDRESS_WIDTH);
-	if (!domain)
-		return -ENOMEM;
-
-	ret = domain_prepare_identity_map(dev, domain, start, end);
-	if (ret)
-		domain_exit(domain);
-
-	return ret;
-}
-
-static inline int iommu_prepare_rmrr_dev(struct dmar_rmrr_unit *rmrr,
-					 struct device *dev)
-{
-	if (dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO)
-		return 0;
-	return iommu_prepare_identity_map(dev, rmrr->base_address,
-					  rmrr->end_address);
-}
-
 static inline struct iommu_resv_region *iommu_get_isa_resv_region(void)
 {
 	if (!isa_resv_region)
@@ -2812,29 +2577,6 @@ static inline struct iommu_resv_region *iommu_get_isa_resv_region(void)
 	return isa_resv_region;
 }
 
-#ifdef CONFIG_INTEL_IOMMU_FLOPPY_WA
-static inline void iommu_prepare_isa(struct pci_dev *pdev)
-{
-	int ret;
-	struct iommu_resv_region *reg = iommu_get_isa_resv_region();
-
-	if (!reg)
-		return;
-
-	pr_info("Prepare 0-16MiB unity mapping for LPC\n");
-	ret = iommu_prepare_identity_map(&pdev->dev, reg->start,
-					 reg->start + reg->length - 1);
-
-	if (ret)
-		pr_err("Failed to create 0-16MiB identity map - floppy might not work\n");
-}
-#else
-static inline void iommu_prepare_isa(struct pci_dev *pdev)
-{
-	return;
-}
-#endif /* !CONFIG_INTEL_IOMMU_FLPY_WA */
-
 static int md_domain_init(struct dmar_domain *domain, int guest_width);
 
 static int __init si_domain_init(int hw)
@@ -3033,18 +2775,10 @@ static int __init dev_prepare_static_identity_mapping(struct device *dev, int hw
 
 static int __init iommu_prepare_static_identity_mapping(int hw)
 {
-	struct pci_dev *pdev = NULL;
 	struct dmar_drhd_unit *drhd;
 	struct intel_iommu *iommu;
 	struct device *dev;
-	int i;
-	int ret = 0;
-
-	for_each_pci_dev(pdev) {
-		ret = dev_prepare_static_identity_mapping(&pdev->dev, hw);
-		if (ret)
-			return ret;
-	}
+	int i, ret = 0;
 
 	for_each_active_iommu(iommu, drhd)
 		for_each_active_dev_scope(drhd->devices, drhd->devices_cnt, i, dev) {
@@ -3291,12 +3025,9 @@ static int copy_translation_tables(struct intel_iommu *iommu)
 static int __init init_dmars(void)
 {
 	struct dmar_drhd_unit *drhd;
-	struct dmar_rmrr_unit *rmrr;
 	bool copied_tables = false;
-	struct device *dev;
-	struct pci_dev *pdev;
 	struct intel_iommu *iommu;
-	int i, ret;
+	int ret;
 
 	/*
 	 * for each drhd
@@ -3447,36 +3178,6 @@ static int __init init_dmars(void)
 			goto free_iommu;
 		}
 	}
-	/*
-	 * For each rmrr
-	 *   for each dev attached to rmrr
-	 *   do
-	 *     locate drhd for dev, alloc domain for dev
-	 *     allocate free domain
-	 *     allocate page table entries for rmrr
-	 *     if context not allocated for bus
-	 *           allocate and init context
-	 *           set present in root table for this bus
-	 *     init context with domain, translation etc
-	 *    endfor
-	 * endfor
-	 */
-	pr_info("Setting RMRR:\n");
-	for_each_rmrr_units(rmrr) {
-		/* some BIOS lists non-exist devices in DMAR table. */
-		for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
-					  i, dev) {
-			ret = iommu_prepare_rmrr_dev(rmrr, dev);
-			if (ret)
-				pr_err("Mapping reserved region failed\n");
-		}
-	}
-
-	pdev = pci_get_class(PCI_CLASS_BRIDGE_ISA << 8, NULL);
-	if (pdev) {
-		iommu_prepare_isa(pdev);
-		pci_dev_put(pdev);
-	}
 
 domains_done:
 
@@ -3565,53 +3266,6 @@ static unsigned long intel_alloc_iova(struct device *dev,
 	return iova_pfn;
 }
 
-struct dmar_domain *get_valid_domain_for_dev(struct device *dev)
-{
-	struct dmar_domain *domain, *tmp;
-	struct dmar_rmrr_unit *rmrr;
-	struct device *i_dev;
-	int i, ret;
-
-	domain = find_domain(dev);
-	if (domain)
-		goto out;
-
-	domain = find_or_alloc_domain(dev, DEFAULT_DOMAIN_ADDRESS_WIDTH);
-	if (!domain)
-		goto out;
-
-	/* We have a new domain - setup possible RMRRs for the device */
-	rcu_read_lock();
-	for_each_rmrr_units(rmrr) {
-		for_each_active_dev_scope(rmrr->devices, rmrr->devices_cnt,
-					  i, i_dev) {
-			if (i_dev != dev)
-				continue;
-
-			ret = domain_prepare_identity_map(dev, domain,
-							  rmrr->base_address,
-							  rmrr->end_address);
-			if (ret)
-				dev_err(dev, "Mapping reserved region failed\n");
-		}
-	}
-	rcu_read_unlock();
-
-	tmp = set_domain_for_dev(dev, domain);
-	if (!tmp || domain != tmp) {
-		domain_exit(domain);
-		domain = tmp;
-	}
-
-out:
-
-	if (!domain)
-		dev_err(dev, "Allocating domain failed\n");
-
-
-	return domain;
-}
-
 /* Check if the dev needs to go through non-identity map and unmap process.*/
 static int iommu_no_mapping(struct device *dev)
 {
@@ -3665,7 +3319,7 @@ static dma_addr_t __intel_map_single(struct device *dev, phys_addr_t paddr,
 	if (iommu_no_mapping(dev))
 		return paddr;
 
-	domain = get_valid_domain_for_dev(dev);
+	domain = find_domain(dev);
 	if (!domain)
 		return DMA_MAPPING_ERROR;
 
@@ -3887,7 +3541,7 @@ static int intel_map_sg(struct device *dev, struct scatterlist *sglist, int nele
 	if (iommu_no_mapping(dev))
 		return intel_nontranslate_map_sg(dev, sglist, nelems, dir);
 
-	domain = get_valid_domain_for_dev(dev);
+	domain = find_domain(dev);
 	if (!domain)
 		return 0;
 
@@ -5582,7 +5236,7 @@ int intel_iommu_enable_pasid(struct intel_iommu *iommu, struct device *dev)
 	u64 ctx_lo;
 	int ret;
 
-	domain = get_valid_domain_for_dev(dev);
+	domain = find_domain(dev);
 	if (!domain)
 		return -EINVAL;
 
-- 
2.17.1

[PATCH v3 8/8] iommu/vt-d: Implement is_attach_deferred iommu ops entry

[Lu Baolu]

As a domain is now attached to a device earlier, we should
implement the is_attach_deferred call-back and use it to
defer the domain attach from iommu driver init to device
driver init when iommu is pre-enabled in kdump kernel.

Suggested-by: Tom Murphy <tmurphy@arista.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel-iommu.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index dd6abd554804..7e24f025f7a9 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -358,6 +358,8 @@ static void domain_context_clear(struct intel_iommu *iommu,
 				 struct device *dev);
 static int domain_detach_iommu(struct dmar_domain *domain,
 			       struct intel_iommu *iommu);
+static int intel_iommu_attach_device(struct iommu_domain *domain,
+				     struct device *dev);
 
 #ifdef CONFIG_INTEL_IOMMU_DEFAULT_ON
 int dmar_disabled = 0;
@@ -387,6 +389,7 @@ int intel_iommu_gfx_mapped;
 EXPORT_SYMBOL_GPL(intel_iommu_gfx_mapped);
 
 #define DUMMY_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-1))
+#define DEFER_DEVICE_DOMAIN_INFO ((struct device_domain_info *)(-2))
 static DEFINE_SPINLOCK(device_domain_lock);
 static LIST_HEAD(device_domain_list);
 
@@ -2404,8 +2407,18 @@ static struct dmar_domain *find_domain(struct device *dev)
 {
 	struct device_domain_info *info;
 
+	if (unlikely(dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO)) {
+		struct iommu_domain *domain;
+
+		dev->archdata.iommu = NULL;
+		domain = iommu_get_domain_for_dev(dev);
+		if (domain)
+			intel_iommu_attach_device(domain, dev);
+	}
+
 	/* No lock here, assumes no domain exit in normal case */
 	info = dev->archdata.iommu;
+
 	if (likely(info))
 		return info->domain;
 	return NULL;
@@ -5154,6 +5167,9 @@ static int intel_iommu_add_device(struct device *dev)
 
 	iommu_device_link(&iommu->iommu, dev);
 
+	if (translation_pre_enabled(iommu))
+		dev->archdata.iommu = DEFER_DEVICE_DOMAIN_INFO;
+
 	group = iommu_group_get_for_dev(dev);
 
 	if (IS_ERR(group))
@@ -5440,6 +5456,12 @@ intel_iommu_aux_get_pasid(struct iommu_domain *domain, struct device *dev)
 			dmar_domain->default_pasid : -EINVAL;
 }
 
+static bool intel_iommu_is_attach_deferred(struct iommu_domain *domain,
+					   struct device *dev)
+{
+	return dev->archdata.iommu == DEFER_DEVICE_DOMAIN_INFO;
+}
+
 const struct iommu_ops intel_iommu_ops = {
 	.capable		= intel_iommu_capable,
 	.domain_alloc		= intel_iommu_domain_alloc,
@@ -5463,6 +5485,7 @@ const struct iommu_ops intel_iommu_ops = {
 	.dev_enable_feat	= intel_iommu_dev_enable_feat,
 	.dev_disable_feat	= intel_iommu_dev_disable_feat,
 	.def_domain_type	= intel_iommu_def_domain_type,
+	.is_attach_deferred	= intel_iommu_is_attach_deferred,
 	.pgsize_bitmap		= INTEL_IOMMU_PGSIZES,
 };
 
-- 
2.17.1