[PATCH 0/2] Prevent RESV_DIRECT devices from user assignment

[PATCH 0/2] Prevent RESV_DIRECT devices from user assignment

[Lu Baolu]

These are follow-up patches on this discussion:

https://lore.kernel.org/linux-iommu/BN9PR11MB5276E84229B5BD952D78E9598C639@BN9PR11MB5276.namprd11.prod.outlook.com

I just summarized the ideas and code into a real patch series. Please
help to review and merge.

Best regards,
baolu

Lu Baolu (2):
  iommu: Prevent RESV_DIRECT devices from blocking domains
  iommu/vt-d: Remove rmrr check in domain attaching device path

 include/linux/iommu.h       |  2 ++
 drivers/iommu/intel/iommu.c | 58 -------------------------------------
 drivers/iommu/iommu.c       | 39 ++++++++++++++++++-------
 3 files changed, 31 insertions(+), 68 deletions(-)

-- 
2.34.1

[PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Lu Baolu]

The IOMMU_RESV_DIRECT flag indicates that a memory region must be mapped
1:1 at all times. This means that the region must always be accessible to
the device, even if the device is attached to a blocking domain. This is
equal to saying that IOMMU_RESV_DIRECT flag prevents devices from being
attached to blocking domains.

This also implies that devices that implement RESV_DIRECT regions will be
prevented from being assigned to user space since taking the DMA ownership
immediately switches to a blocking domain.

The rule of preventing devices with the IOMMU_RESV_DIRECT regions from
being assigned to user space has existed in the Intel IOMMU driver for
a long time. Now, this rule is being lifted up to a general core rule,
as other architectures like AMD and ARM also have RMRR-like reserved
regions. This has been discussed in the community mailing list and refer
to below link for more details.

Other places using unmanaged domains for kernel DMA must follow the
iommu_get_resv_regions() and setup IOMMU_RESV_DIRECT - we do not restrict
them in the core code.

Cc: Robin Murphy <robin.murphy@arm.com>
Cc: Alex Williamson <alex.williamson@redhat.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Link: https://lore.kernel.org/linux-iommu/BN9PR11MB5276E84229B5BD952D78E9598C639@BN9PR11MB5276.namprd11.prod.outlook.com
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 include/linux/iommu.h |  2 ++
 drivers/iommu/iommu.c | 39 +++++++++++++++++++++++++++++----------
 2 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index d31642596675..fd18019ac951 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -409,6 +409,7 @@ struct iommu_fault_param {
  * @priv:	 IOMMU Driver private data
  * @max_pasids:  number of PASIDs this device can consume
  * @attach_deferred: the dma domain attachment is deferred
+ * @requires_direct: The driver requested IOMMU_RESV_DIRECT
  *
  * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
  *	struct iommu_group	*iommu_group;
@@ -422,6 +423,7 @@ struct dev_iommu {
 	void				*priv;
 	u32				max_pasids;
 	u32				attach_deferred:1;
+	u32				requires_direct:1;
 };
 
 int iommu_device_register(struct iommu_device *iommu,
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 9e0228ef612b..e59de7852067 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -959,12 +959,7 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
 	unsigned long pg_size;
 	int ret = 0;
 
-	if (!iommu_is_dma_domain(domain))
-		return 0;
-
-	BUG_ON(!domain->pgsize_bitmap);
-
-	pg_size = 1UL << __ffs(domain->pgsize_bitmap);
+	pg_size = domain->pgsize_bitmap ? 1UL << __ffs(domain->pgsize_bitmap) : 0;
 	INIT_LIST_HEAD(&mappings);
 
 	iommu_get_resv_regions(dev, &mappings);
@@ -974,13 +969,22 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
 		dma_addr_t start, end, addr;
 		size_t map_size = 0;
 
+		if (entry->type == IOMMU_RESV_DIRECT)
+			dev->iommu->requires_direct = 1;
+
+		if ((entry->type != IOMMU_RESV_DIRECT &&
+		     entry->type != IOMMU_RESV_DIRECT_RELAXABLE) ||
+		    !iommu_is_dma_domain(domain))
+			continue;
+
+		if (WARN_ON_ONCE(!pg_size)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		start = ALIGN(entry->start, pg_size);
 		end   = ALIGN(entry->start + entry->length, pg_size);
 
-		if (entry->type != IOMMU_RESV_DIRECT &&
-		    entry->type != IOMMU_RESV_DIRECT_RELAXABLE)
-			continue;
-
 		for (addr = start; addr <= end; addr += pg_size) {
 			phys_addr_t phys_addr;
 
@@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
 {
 	int ret;
 
+	/*
+	 * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
+	 * the blocking domain to be attached as it does not contain the
+	 * required 1:1 mapping. This test effectively exclusive the device from
+	 * being used with iommu_group_claim_dma_owner() which will block vfio
+	 * and iommufd as well.
+	 */
+	if (dev->iommu->requires_direct &&
+	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
+	     new_domain == group->blocking_domain)) {
+		dev_warn(dev,
+			 "Firmware has requested this device have a 1:1 IOMMU mapping, rejecting configuring the device without a 1:1 mapping. Contact your platform vendor.\n");
+		return -EINVAL;
+	}
+
 	if (dev->iommu->attach_deferred) {
 		if (new_domain == group->default_domain)
 			return 0;
-- 
2.34.1

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Liu, Jingqi]

On 6/7/2023 11:51 AM, Lu Baolu wrote:
> The IOMMU_RESV_DIRECT flag indicates that a memory region must be mapped
> 1:1 at all times. This means that the region must always be accessible to
> the device, even if the device is attached to a blocking domain. This is
> equal to saying that IOMMU_RESV_DIRECT flag prevents devices from being
> attached to blocking domains.
>
> This also implies that devices that implement RESV_DIRECT regions will be
> prevented from being assigned to user space since taking the DMA ownership
> immediately switches to a blocking domain.
>
> The rule of preventing devices with the IOMMU_RESV_DIRECT regions from
> being assigned to user space has existed in the Intel IOMMU driver for
> a long time. Now, this rule is being lifted up to a general core rule,
> as other architectures like AMD and ARM also have RMRR-like reserved
> regions. This has been discussed in the community mailing list and refer
> to below link for more details.
>
> Other places using unmanaged domains for kernel DMA must follow the
> iommu_get_resv_regions() and setup IOMMU_RESV_DIRECT - we do not restrict
> them in the core code.
>
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Link: https://lore.kernel.org/linux-iommu/BN9PR11MB5276E84229B5BD952D78E9598C639@BN9PR11MB5276.namprd11.prod.outlook.com
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   include/linux/iommu.h |  2 ++
>   drivers/iommu/iommu.c | 39 +++++++++++++++++++++++++++++----------
>   2 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index d31642596675..fd18019ac951 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -409,6 +409,7 @@ struct iommu_fault_param {
>    * @priv:	 IOMMU Driver private data
>    * @max_pasids:  number of PASIDs this device can consume
>    * @attach_deferred: the dma domain attachment is deferred
> + * @requires_direct: The driver requested IOMMU_RESV_DIRECT
>    *
>    * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
>    *	struct iommu_group	*iommu_group;
> @@ -422,6 +423,7 @@ struct dev_iommu {
>   	void				*priv;
>   	u32				max_pasids;
>   	u32				attach_deferred:1;
> +	u32				requires_direct:1;
>   };
>   
>   int iommu_device_register(struct iommu_device *iommu,
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 9e0228ef612b..e59de7852067 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -959,12 +959,7 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
>   	unsigned long pg_size;
>   	int ret = 0;
>   
> -	if (!iommu_is_dma_domain(domain))
> -		return 0;
> -
> -	BUG_ON(!domain->pgsize_bitmap);
> -
> -	pg_size = 1UL << __ffs(domain->pgsize_bitmap);
> +	pg_size = domain->pgsize_bitmap ? 1UL << __ffs(domain->pgsize_bitmap) : 0;
Would it be better to add the following check here?
     if (WARN_ON(!pg_size))
             return -EINVAL;

Instead of checking latter in the loop as follows.
     if (WARN_ON_ONCE(!pg_size)) {
             ret = -EINVAL;
             goto out;
     }

Thanks,
Jingqi
>   	INIT_LIST_HEAD(&mappings);
>   
>   	iommu_get_resv_regions(dev, &mappings);
> @@ -974,13 +969,22 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
>   		dma_addr_t start, end, addr;
>   		size_t map_size = 0;
>   
> +		if (entry->type == IOMMU_RESV_DIRECT)
> +			dev->iommu->requires_direct = 1;
> +
> +		if ((entry->type != IOMMU_RESV_DIRECT &&
> +		     entry->type != IOMMU_RESV_DIRECT_RELAXABLE) ||
> +		    !iommu_is_dma_domain(domain))
> +			continue;
> +
> +		if (WARN_ON_ONCE(!pg_size)) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
>   		start = ALIGN(entry->start, pg_size);
>   		end   = ALIGN(entry->start + entry->length, pg_size);
>   
> -		if (entry->type != IOMMU_RESV_DIRECT &&
> -		    entry->type != IOMMU_RESV_DIRECT_RELAXABLE)
> -			continue;
> -
>   		for (addr = start; addr <= end; addr += pg_size) {
>   			phys_addr_t phys_addr;
>   
> @@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
>   {
>   	int ret;
>   
> +	/*
> +	 * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
> +	 * the blocking domain to be attached as it does not contain the
> +	 * required 1:1 mapping. This test effectively exclusive the device from
> +	 * being used with iommu_group_claim_dma_owner() which will block vfio
> +	 * and iommufd as well.
> +	 */
> +	if (dev->iommu->requires_direct &&
> +	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
> +	     new_domain == group->blocking_domain)) {
> +		dev_warn(dev,
> +			 "Firmware has requested this device have a 1:1 IOMMU mapping, rejecting configuring the device without a 1:1 mapping. Contact your platform vendor.\n");
> +		return -EINVAL;
> +	}
> +
>   	if (dev->iommu->attach_deferred) {
>   		if (new_domain == group->default_domain)
>   			return 0;

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Baolu Lu]

On 6/12/23 4:28 PM, Liu, Jingqi wrote:
> On 6/7/2023 11:51 AM, Lu Baolu wrote:
>> The IOMMU_RESV_DIRECT flag indicates that a memory region must be mapped
>> 1:1 at all times. This means that the region must always be accessible to
>> the device, even if the device is attached to a blocking domain. This is
>> equal to saying that IOMMU_RESV_DIRECT flag prevents devices from being
>> attached to blocking domains.
>>
>> This also implies that devices that implement RESV_DIRECT regions will be
>> prevented from being assigned to user space since taking the DMA 
>> ownership
>> immediately switches to a blocking domain.
>>
>> The rule of preventing devices with the IOMMU_RESV_DIRECT regions from
>> being assigned to user space has existed in the Intel IOMMU driver for
>> a long time. Now, this rule is being lifted up to a general core rule,
>> as other architectures like AMD and ARM also have RMRR-like reserved
>> regions. This has been discussed in the community mailing list and refer
>> to below link for more details.
>>
>> Other places using unmanaged domains for kernel DMA must follow the
>> iommu_get_resv_regions() and setup IOMMU_RESV_DIRECT - we do not restrict
>> them in the core code.
>>
>> Cc: Robin Murphy <robin.murphy@arm.com>
>> Cc: Alex Williamson <alex.williamson@redhat.com>
>> Cc: Kevin Tian <kevin.tian@intel.com>
>> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>> Link: 
>> https://lore.kernel.org/linux-iommu/BN9PR11MB5276E84229B5BD952D78E9598C639@BN9PR11MB5276.namprd11.prod.outlook.com
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   include/linux/iommu.h |  2 ++
>>   drivers/iommu/iommu.c | 39 +++++++++++++++++++++++++++++----------
>>   2 files changed, 31 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index d31642596675..fd18019ac951 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -409,6 +409,7 @@ struct iommu_fault_param {
>>    * @priv:     IOMMU Driver private data
>>    * @max_pasids:  number of PASIDs this device can consume
>>    * @attach_deferred: the dma domain attachment is deferred
>> + * @requires_direct: The driver requested IOMMU_RESV_DIRECT
>>    *
>>    * TODO: migrate other per device data pointers under 
>> iommu_dev_data, e.g.
>>    *    struct iommu_group    *iommu_group;
>> @@ -422,6 +423,7 @@ struct dev_iommu {
>>       void                *priv;
>>       u32                max_pasids;
>>       u32                attach_deferred:1;
>> +    u32                requires_direct:1;
>>   };
>>   int iommu_device_register(struct iommu_device *iommu,
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 9e0228ef612b..e59de7852067 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -959,12 +959,7 @@ static int 
>> iommu_create_device_direct_mappings(struct iommu_domain *domain,
>>       unsigned long pg_size;
>>       int ret = 0;
>> -    if (!iommu_is_dma_domain(domain))
>> -        return 0;
>> -
>> -    BUG_ON(!domain->pgsize_bitmap);
>> -
>> -    pg_size = 1UL << __ffs(domain->pgsize_bitmap);
>> +    pg_size = domain->pgsize_bitmap ? 1UL << 
>> __ffs(domain->pgsize_bitmap) : 0;
> Would it be better to add the following check here?
>      if (WARN_ON(!pg_size))
>              return -EINVAL;
> 
> Instead of checking latter in the loop as follows.
>      if (WARN_ON_ONCE(!pg_size)) {
>              ret = -EINVAL;
>              goto out;
>      }

I am afraid no. Only the paging domains need a valid pg_size. That's the
reason why I put it after the iommu_is_dma_domain() check. The previous
code has the same behavior too.

Best regards,
baolu

RE: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 13, 2023 11:15 AM
> 
> On 6/12/23 4:28 PM, Liu, Jingqi wrote:
> > On 6/7/2023 11:51 AM, Lu Baolu wrote:
> >> -
> >> -    BUG_ON(!domain->pgsize_bitmap);
> >> -
> >> -    pg_size = 1UL << __ffs(domain->pgsize_bitmap);
> >> +    pg_size = domain->pgsize_bitmap ? 1UL <<
> >> __ffs(domain->pgsize_bitmap) : 0;
> > Would it be better to add the following check here?
> >      if (WARN_ON(!pg_size))
> >              return -EINVAL;
> >
> > Instead of checking latter in the loop as follows.
> >      if (WARN_ON_ONCE(!pg_size)) {
> >              ret = -EINVAL;
> >              goto out;
> >      }
> 
> I am afraid no. Only the paging domains need a valid pg_size. That's the
> reason why I put it after the iommu_is_dma_domain() check. The previous
> code has the same behavior too.
> 

You could also add the dma domain check here. pg_size is static
then it makes more sense to verify it once instead of in a loop.

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Baolu Lu]

On 2023/6/27 15:54, Tian, Kevin wrote:
>> From: Baolu Lu <baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 13, 2023 11:15 AM
>>
>> On 6/12/23 4:28 PM, Liu, Jingqi wrote:
>>> On 6/7/2023 11:51 AM, Lu Baolu wrote:
>>>> -
>>>> -    BUG_ON(!domain->pgsize_bitmap);
>>>> -
>>>> -    pg_size = 1UL << __ffs(domain->pgsize_bitmap);
>>>> +    pg_size = domain->pgsize_bitmap ? 1UL <<
>>>> __ffs(domain->pgsize_bitmap) : 0;
>>> Would it be better to add the following check here?
>>>       if (WARN_ON(!pg_size))
>>>               return -EINVAL;
>>>
>>> Instead of checking latter in the loop as follows.
>>>       if (WARN_ON_ONCE(!pg_size)) {
>>>               ret = -EINVAL;
>>>               goto out;
>>>       }
>>
>> I am afraid no. Only the paging domains need a valid pg_size. That's the
>> reason why I put it after the iommu_is_dma_domain() check. The previous
>> code has the same behavior too.
>>
> 
> You could also add the dma domain check here. pg_size is static
> then it makes more sense to verify it once instead of in a loop.

Agreed. Does below additional change make sense?

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index e59de7852067..3be88b5f36bb 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -962,6 +962,9 @@ static int 
iommu_create_device_direct_mappings(struct iommu_domain *domain,
         pg_size = domain->pgsize_bitmap ? 1UL << 
__ffs(domain->pgsize_bitmap) : 0;
         INIT_LIST_HEAD(&mappings);

+       if (WARN_ON_ONCE((domain->type & __IOMMU_DOMAIN_PAGING) && 
!pg_size))
+               return -EINVAL;
+
         iommu_get_resv_regions(dev, &mappings);

         /* We need to consider overlapping regions for different devices */
@@ -977,11 +980,6 @@ static int 
iommu_create_device_direct_mappings(struct iommu_domain *domain,
                     !iommu_is_dma_domain(domain))
                         continue;

-               if (WARN_ON_ONCE(!pg_size)) {
-                       ret = -EINVAL;
-                       goto out;
-               }
-
                 start = ALIGN(entry->start, pg_size);
                 end   = ALIGN(entry->start + entry->length, pg_size);

Best regards,
baolu

RE: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Tian, Kevin]

> From: Baolu Lu <baolu.lu@linux.intel.com>
> Sent: Tuesday, June 27, 2023 4:01 PM
> 
> On 2023/6/27 15:54, Tian, Kevin wrote:
> >> From: Baolu Lu <baolu.lu@linux.intel.com>
> >> Sent: Tuesday, June 13, 2023 11:15 AM
> >>
> >> On 6/12/23 4:28 PM, Liu, Jingqi wrote:
> >>> On 6/7/2023 11:51 AM, Lu Baolu wrote:
> >>>> -
> >>>> -    BUG_ON(!domain->pgsize_bitmap);
> >>>> -
> >>>> -    pg_size = 1UL << __ffs(domain->pgsize_bitmap);
> >>>> +    pg_size = domain->pgsize_bitmap ? 1UL <<
> >>>> __ffs(domain->pgsize_bitmap) : 0;
> >>> Would it be better to add the following check here?
> >>>       if (WARN_ON(!pg_size))
> >>>               return -EINVAL;
> >>>
> >>> Instead of checking latter in the loop as follows.
> >>>       if (WARN_ON_ONCE(!pg_size)) {
> >>>               ret = -EINVAL;
> >>>               goto out;
> >>>       }
> >>
> >> I am afraid no. Only the paging domains need a valid pg_size. That's the
> >> reason why I put it after the iommu_is_dma_domain() check. The
> previous
> >> code has the same behavior too.
> >>
> >
> > You could also add the dma domain check here. pg_size is static
> > then it makes more sense to verify it once instead of in a loop.
> 
> Agreed. Does below additional change make sense?
> 
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index e59de7852067..3be88b5f36bb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -962,6 +962,9 @@ static int
> iommu_create_device_direct_mappings(struct iommu_domain *domain,
>          pg_size = domain->pgsize_bitmap ? 1UL <<
> __ffs(domain->pgsize_bitmap) : 0;
>          INIT_LIST_HEAD(&mappings);
> 
> +       if (WARN_ON_ONCE((domain->type & __IOMMU_DOMAIN_PAGING)
> &&
> !pg_size))
> +               return -EINVAL;

what's the reason of not using iommu_is_dma_domain()? this is called
in the probe path only for the default domain. Otherwise if you change
like this then you also want to change the check in the loop later to be
consistent.

> +
>          iommu_get_resv_regions(dev, &mappings);
> 
>          /* We need to consider overlapping regions for different devices */
> @@ -977,11 +980,6 @@ static int
> iommu_create_device_direct_mappings(struct iommu_domain *domain,
>                      !iommu_is_dma_domain(domain))
>                          continue;
> 
> -               if (WARN_ON_ONCE(!pg_size)) {
> -                       ret = -EINVAL;
> -                       goto out;
> -               }
> -
>                  start = ALIGN(entry->start, pg_size);
>                  end   = ALIGN(entry->start + entry->length, pg_size);
> 
> Best regards,
> baolu

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Baolu Lu]

On 2023/6/27 16:15, Tian, Kevin wrote:
>> From: Baolu Lu<baolu.lu@linux.intel.com>
>> Sent: Tuesday, June 27, 2023 4:01 PM
>>
>> On 2023/6/27 15:54, Tian, Kevin wrote:
>>>> From: Baolu Lu<baolu.lu@linux.intel.com>
>>>> Sent: Tuesday, June 13, 2023 11:15 AM
>>>>
>>>> On 6/12/23 4:28 PM, Liu, Jingqi wrote:
>>>>> On 6/7/2023 11:51 AM, Lu Baolu wrote:
>>>>>> -
>>>>>> -    BUG_ON(!domain->pgsize_bitmap);
>>>>>> -
>>>>>> -    pg_size = 1UL << __ffs(domain->pgsize_bitmap);
>>>>>> +    pg_size = domain->pgsize_bitmap ? 1UL <<
>>>>>> __ffs(domain->pgsize_bitmap) : 0;
>>>>> Would it be better to add the following check here?
>>>>>        if (WARN_ON(!pg_size))
>>>>>                return -EINVAL;
>>>>>
>>>>> Instead of checking latter in the loop as follows.
>>>>>        if (WARN_ON_ONCE(!pg_size)) {
>>>>>                ret = -EINVAL;
>>>>>                goto out;
>>>>>        }
>>>> I am afraid no. Only the paging domains need a valid pg_size. That's the
>>>> reason why I put it after the iommu_is_dma_domain() check. The
>> previous
>>>> code has the same behavior too.
>>>>
>>> You could also add the dma domain check here. pg_size is static
>>> then it makes more sense to verify it once instead of in a loop.
>> Agreed. Does below additional change make sense?
>>
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index e59de7852067..3be88b5f36bb 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -962,6 +962,9 @@ static int
>> iommu_create_device_direct_mappings(struct iommu_domain *domain,
>>           pg_size = domain->pgsize_bitmap ? 1UL <<
>> __ffs(domain->pgsize_bitmap) : 0;
>>           INIT_LIST_HEAD(&mappings);
>>
>> +       if (WARN_ON_ONCE((domain->type & __IOMMU_DOMAIN_PAGING)
>> &&
>> !pg_size))
>> +               return -EINVAL;
> what's the reason of not using iommu_is_dma_domain()? this is called
> in the probe path only for the default domain. Otherwise if you change
> like this then you also want to change the check in the loop later to be
> consistent.
> 

Yes. iommu_is_dma_domain() is better.

Best regards,
baolu

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Jason Gunthorpe]

On Tue, Jun 27, 2023 at 04:01:01PM +0800, Baolu Lu wrote:

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index e59de7852067..3be88b5f36bb 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -962,6 +962,9 @@ static int iommu_create_device_direct_mappings(struct
> iommu_domain *domain,
>         pg_size = domain->pgsize_bitmap ? 1UL <<
> __ffs(domain->pgsize_bitmap) : 0;
>         INIT_LIST_HEAD(&mappings);
> 
> +       if (WARN_ON_ONCE((domain->type & __IOMMU_DOMAIN_PAGING) &&
> !pg_size))
> +               return -EINVAL;

Calling this function with an identity domain is expected, it must
return 0.

Jason

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Robin Murphy]

On 07/06/2023 4:51 am, Lu Baolu wrote:
> The IOMMU_RESV_DIRECT flag indicates that a memory region must be mapped
> 1:1 at all times. This means that the region must always be accessible to
> the device, even if the device is attached to a blocking domain. This is
> equal to saying that IOMMU_RESV_DIRECT flag prevents devices from being
> attached to blocking domains.
> 
> This also implies that devices that implement RESV_DIRECT regions will be
> prevented from being assigned to user space since taking the DMA ownership
> immediately switches to a blocking domain.
> 
> The rule of preventing devices with the IOMMU_RESV_DIRECT regions from
> being assigned to user space has existed in the Intel IOMMU driver for
> a long time. Now, this rule is being lifted up to a general core rule,
> as other architectures like AMD and ARM also have RMRR-like reserved
> regions. This has been discussed in the community mailing list and refer
> to below link for more details.
> 
> Other places using unmanaged domains for kernel DMA must follow the
> iommu_get_resv_regions() and setup IOMMU_RESV_DIRECT - we do not restrict
> them in the core code.
> 
> Cc: Robin Murphy <robin.murphy@arm.com>
> Cc: Alex Williamson <alex.williamson@redhat.com>
> Cc: Kevin Tian <kevin.tian@intel.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Link: https://lore.kernel.org/linux-iommu/BN9PR11MB5276E84229B5BD952D78E9598C639@BN9PR11MB5276.namprd11.prod.outlook.com
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>   include/linux/iommu.h |  2 ++
>   drivers/iommu/iommu.c | 39 +++++++++++++++++++++++++++++----------
>   2 files changed, 31 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index d31642596675..fd18019ac951 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -409,6 +409,7 @@ struct iommu_fault_param {
>    * @priv:	 IOMMU Driver private data
>    * @max_pasids:  number of PASIDs this device can consume
>    * @attach_deferred: the dma domain attachment is deferred
> + * @requires_direct: The driver requested IOMMU_RESV_DIRECT
>    *
>    * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
>    *	struct iommu_group	*iommu_group;
> @@ -422,6 +423,7 @@ struct dev_iommu {
>   	void				*priv;
>   	u32				max_pasids;
>   	u32				attach_deferred:1;
> +	u32				requires_direct:1;
>   };
>   
>   int iommu_device_register(struct iommu_device *iommu,
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 9e0228ef612b..e59de7852067 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -959,12 +959,7 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
>   	unsigned long pg_size;
>   	int ret = 0;
>   
> -	if (!iommu_is_dma_domain(domain))
> -		return 0;
> -
> -	BUG_ON(!domain->pgsize_bitmap);
> -
> -	pg_size = 1UL << __ffs(domain->pgsize_bitmap);
> +	pg_size = domain->pgsize_bitmap ? 1UL << __ffs(domain->pgsize_bitmap) : 0;
>   	INIT_LIST_HEAD(&mappings);
>   
>   	iommu_get_resv_regions(dev, &mappings);
> @@ -974,13 +969,22 @@ static int iommu_create_device_direct_mappings(struct iommu_domain *domain,
>   		dma_addr_t start, end, addr;
>   		size_t map_size = 0;
>   
> +		if (entry->type == IOMMU_RESV_DIRECT)
> +			dev->iommu->requires_direct = 1;
> +
> +		if ((entry->type != IOMMU_RESV_DIRECT &&
> +		     entry->type != IOMMU_RESV_DIRECT_RELAXABLE) ||
> +		    !iommu_is_dma_domain(domain))
> +			continue;
> +
> +		if (WARN_ON_ONCE(!pg_size)) {
> +			ret = -EINVAL;
> +			goto out;
> +		}
> +
>   		start = ALIGN(entry->start, pg_size);
>   		end   = ALIGN(entry->start + entry->length, pg_size);
>   
> -		if (entry->type != IOMMU_RESV_DIRECT &&
> -		    entry->type != IOMMU_RESV_DIRECT_RELAXABLE)
> -			continue;
> -
>   		for (addr = start; addr <= end; addr += pg_size) {
>   			phys_addr_t phys_addr;
>   
> @@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
>   {
>   	int ret;
>   
> +	/*
> +	 * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
> +	 * the blocking domain to be attached as it does not contain the
> +	 * required 1:1 mapping. This test effectively exclusive the device from
> +	 * being used with iommu_group_claim_dma_owner() which will block vfio
> +	 * and iommufd as well.
> +	 */
> +	if (dev->iommu->requires_direct &&
> +	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||

Given the notion elsewhere that we want to use the blocking domain as a 
last resort to handle an attach failure, at face value it looks suspect 
that failing to attach to a blocking domain could also be a thing. I 
guess technically this is failing at a slightly different level so maybe 
it does work out OK, but it's still smelly.

The main thing, though, is that not everything implements the 
IOMMU_DOMAIN_BLOCKED optimisation, so a nominal blocking domain could be 
IOMMU_DOMAIN_UNMANAGED as well. FWIW I'd prefer to make the RESV_DIRECT 
check explicit in __iommu_take_dma_ownership() rather than hide it in an 
implementation detail; that's going to be a lot clearer to reason about 
as time goes on.

Thanks,
Robin.

> +	     new_domain == group->blocking_domain)) {
> +		dev_warn(dev,
> +			 "Firmware has requested this device have a 1:1 IOMMU mapping, rejecting configuring the device without a 1:1 mapping. Contact your platform vendor.\n");
> +		return -EINVAL;
> +	}
> +
>   	if (dev->iommu->attach_deferred) {
>   		if (new_domain == group->default_domain)
>   			return 0;

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Jason Gunthorpe]

On Mon, Jun 19, 2023 at 02:33:18PM +0100, Robin Murphy wrote:
> > @@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
> >   {
> >   	int ret;
> > +	/*
> > +	 * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
> > +	 * the blocking domain to be attached as it does not contain the
> > +	 * required 1:1 mapping. This test effectively exclusive the device from
> > +	 * being used with iommu_group_claim_dma_owner() which will block vfio
> > +	 * and iommufd as well.
> > +	 */
> > +	if (dev->iommu->requires_direct &&
> > +	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
> 
> Given the notion elsewhere that we want to use the blocking domain as a last
> resort to handle an attach failure,

We shouldn't do that for cases where requires_direct is true, the last
resort will have to be the static identity domain.

> at face value it looks suspect that failing to attach to a blocking
> domain could also be a thing. I guess technically this is failing at
> a slightly different level so maybe it does work out OK, but it's
> still smelly.

It basically says that this driver doesn't support blocking domains on
this device. What we don't want is for the driver to fail blocking or
identity attaches.
 
> The main thing, though, is that not everything implements the
> IOMMU_DOMAIN_BLOCKED optimisation, so a nominal blocking domain could be
> IOMMU_DOMAIN_UNMANAGED as well. 

Yes, it should check new_domain == group->blocking_domain as well.

> FWIW I'd prefer to make the RESV_DIRECT check explicit in
> __iommu_take_dma_ownership() rather than hide it in an
> implementation detail; that's going to be a lot clearer to reason
> about as time goes on.

We want to completely forbid blocking domains at all on these devices
because they are not supported (by FW request). I don't really like
the idea that we go and assume the only users of blocking domains are
also using take_dma_ownership() - that feels like a future bug waiting
to happen.

Jason

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Robin Murphy]

On 19/06/2023 2:41 pm, Jason Gunthorpe wrote:
> On Mon, Jun 19, 2023 at 02:33:18PM +0100, Robin Murphy wrote:
>>> @@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
>>>    {
>>>    	int ret;
>>> +	/*
>>> +	 * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
>>> +	 * the blocking domain to be attached as it does not contain the
>>> +	 * required 1:1 mapping. This test effectively exclusive the device from
>>> +	 * being used with iommu_group_claim_dma_owner() which will block vfio
>>> +	 * and iommufd as well.
>>> +	 */
>>> +	if (dev->iommu->requires_direct &&
>>> +	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
>>
>> Given the notion elsewhere that we want to use the blocking domain as a last
>> resort to handle an attach failure,
> 
> We shouldn't do that for cases where requires_direct is true, the last
> resort will have to be the static identity domain.
> 
>> at face value it looks suspect that failing to attach to a blocking
>> domain could also be a thing. I guess technically this is failing at
>> a slightly different level so maybe it does work out OK, but it's
>> still smelly.
> 
> It basically says that this driver doesn't support blocking domains on
> this device. What we don't want is for the driver to fail blocking or
> identity attaches.

Is that really the relevant semantic though? I thought the point was to 
prevent userspace (or anyone else for that matter) taking ownership of a 
device with reserved regions which we can't trust them to honour. Not 
least because the series is entitled "Prevent RESV_DIRECT devices from 
user assignment", not anything about attaching to blocking domains. Plus 
the existing intel-iommu behaviour being generalised is specific to 
IOMMU_DOMAIN_UNMANAGED.

>> The main thing, though, is that not everything implements the
>> IOMMU_DOMAIN_BLOCKED optimisation, so a nominal blocking domain could be
>> IOMMU_DOMAIN_UNMANAGED as well.
> 
> Yes, it should check new_domain == group->blocking_domain as well.
> 
>> FWIW I'd prefer to make the RESV_DIRECT check explicit in
>> __iommu_take_dma_ownership() rather than hide it in an
>> implementation detail; that's going to be a lot clearer to reason
>> about as time goes on.
> 
> We want to completely forbid blocking domains at all on these devices
> because they are not supported (by FW request). I don't really like
> the idea that we go and assume the only users of blocking domains are
> also using take_dma_ownership() - that feels like a future bug waiting
> to happen.

On reflection, I don't think that aspect actually matters anyway - 
nobody can explicitly request attachment to a blocking domain, so if the 
only time they're used is when the IOMMU driver has already had a 
catastrophic internal failure such that we decide to declare the device 
toasted and deliberately put it into an unusable state, blocking its 
reserved regions doesn't seem like a big deal. In fact if anything it 
kind of feels like the right thing to do for that situation. We're 
saying that we want the device to stop accessing memory because things 
might be in an inconsistent state which we can't trust; who says that 
mappings of RESV_DIRECT regions haven't also gone wonky? Having BLOCKED 
mean that the device truly cannot access - and thus potentially corrupt 
- *any* memory anywhere seems like the most robust and useful behaviour.

Thanks,
Robin.

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Jason Gunthorpe]

On Mon, Jun 19, 2023 at 03:20:30PM +0100, Robin Murphy wrote:
> On 19/06/2023 2:41 pm, Jason Gunthorpe wrote:
> > On Mon, Jun 19, 2023 at 02:33:18PM +0100, Robin Murphy wrote:
> > > > @@ -2121,6 +2125,21 @@ static int __iommu_device_set_domain(struct iommu_group *group,
> > > >    {
> > > >    	int ret;
> > > > +	/*
> > > > +	 * If the driver has requested IOMMU_RESV_DIRECT then we cannot allow
> > > > +	 * the blocking domain to be attached as it does not contain the
> > > > +	 * required 1:1 mapping. This test effectively exclusive the device from
> > > > +	 * being used with iommu_group_claim_dma_owner() which will block vfio
> > > > +	 * and iommufd as well.
> > > > +	 */
> > > > +	if (dev->iommu->requires_direct &&
> > > > +	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
> > > 
> > > Given the notion elsewhere that we want to use the blocking domain as a last
> > > resort to handle an attach failure,
> > 
> > We shouldn't do that for cases where requires_direct is true, the last
> > resort will have to be the static identity domain.
> > 
> > > at face value it looks suspect that failing to attach to a blocking
> > > domain could also be a thing. I guess technically this is failing at
> > > a slightly different level so maybe it does work out OK, but it's
> > > still smelly.
> > 
> > It basically says that this driver doesn't support blocking domains on
> > this device. What we don't want is for the driver to fail blocking or
> > identity attaches.
> 
> Is that really the relevant semantic though?

It is the semantic I have had in mind. The end goal:

1) Driver never fails identity or blocking attachments
2) Identity and blocking domains are global static so always available
3) Core code puts restrictions on when identity and blocking domains
   can be used (eg trusted, reserved regions)
4) Catastrophic error recovery ends up moving to either blocking
   (preferred) or identity.

> I thought the point was to prevent userspace (or anyone else for
> that matter) taking ownership of a device with reserved regions
> which we can't trust them to honour.

Yes, this is the practical side effect of enforcing the rules.

The other side effect is that it removes another special case where a
driver has special behavior tied to IOMMU_DOMAIN_DMA.

> assignment", not anything about attaching to blocking domains. Plus the
> existing intel-iommu behaviour being generalised is specific to
> IOMMU_DOMAIN_UNMANAGED.

Being specific to UNMANAGED is a driver mistake, IMHO. It is a hack to
make it only work with VFIO and avoid dma-iommu.

> On reflection, I don't think that aspect actually matters anyway - nobody
> can explicitly request attachment to a blocking domain

They are used as part of the ownership mechanism, blocking domains are
effectively explicitly requested by detaching domains from owned
groups.

Yes, take_ownership is very carefully a sufficient place to put the
check with today's design, but it feels wrong because the blocking
domain compatability has conceptually nothing to do with ownership. If
we use blocking domains in the future then the check will be in the
wrong place.

> so if the only time they're used is when the IOMMU driver has
> already had a catastrophic internal failure such that we decide to
> declare the device toasted and deliberately put it into an unusable
> state, blocking its reserved regions doesn't seem like a big deal.

I think we should discuss then when we get to actually implementing
the error recovery flow we want. I do like blocking in general for the
reasons you give, and that was my first plan.. But if the BIOS will
crash or something if we don't do the reserved region maps that isn't
so good either. IDK would like to hear from the people using this BIOS
feature.

Thanks,
Jason

RE: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Monday, June 19, 2023 11:30 PM
> 
> On Mon, Jun 19, 2023 at 03:20:30PM +0100, Robin Murphy wrote:
> 
> > so if the only time they're used is when the IOMMU driver has
> > already had a catastrophic internal failure such that we decide to
> > declare the device toasted and deliberately put it into an unusable
> > state, blocking its reserved regions doesn't seem like a big deal.
> 
> I think we should discuss then when we get to actually implementing
> the error recovery flow we want. I do like blocking in general for the
> reasons you give, and that was my first plan.. But if the BIOS will
> crash or something if we don't do the reserved region maps that isn't
> so good either. IDK would like to hear from the people using this BIOS
> feature.
> 

The only devices with RMRR which I'm aware of on Intel platforms are
GPU and USB. However they are all RESV_DIRECT_RELAXABLE type.

Here is one reference from the Xen hypervisor. It has a concept called
quarantine domain (similar to blocking domain) when a device is
de-assigned w/o an owner. The quarantine domain has no mappings
except the ones identity-mapped for RMRR types. I'm not sure whether
they observed real examples of RMRR devices which are not GPU/USB.

I guess the reason of not going fully identity or fully blocked is from
the same worry that the BIOS may go insane while Xen still wants to
quarantine the device as much as possible.

Re: [PATCH 1/2] iommu: Prevent RESV_DIRECT devices from blocking domains

[Jason Gunthorpe]

On Tue, Jun 27, 2023 at 08:10:48AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Monday, June 19, 2023 11:30 PM
> > 
> > On Mon, Jun 19, 2023 at 03:20:30PM +0100, Robin Murphy wrote:
> > 
> > > so if the only time they're used is when the IOMMU driver has
> > > already had a catastrophic internal failure such that we decide to
> > > declare the device toasted and deliberately put it into an unusable
> > > state, blocking its reserved regions doesn't seem like a big deal.
> > 
> > I think we should discuss then when we get to actually implementing
> > the error recovery flow we want. I do like blocking in general for the
> > reasons you give, and that was my first plan.. But if the BIOS will
> > crash or something if we don't do the reserved region maps that isn't
> > so good either. IDK would like to hear from the people using this BIOS
> > feature.
> > 
> 
> The only devices with RMRR which I'm aware of on Intel platforms are
> GPU and USB. However they are all RESV_DIRECT_RELAXABLE type.
> 
> Here is one reference from the Xen hypervisor. It has a concept called
> quarantine domain (similar to blocking domain) when a device is
> de-assigned w/o an owner. The quarantine domain has no mappings
> except the ones identity-mapped for RMRR types. I'm not sure whether
> they observed real examples of RMRR devices which are not GPU/USB.

I guess, it seems a bit hard core, but we could spend the cost to
build such a domain during probe..

At least for our cases we already do expect that DMA is halted before
we start messing with the domains so identity may not be the same
issue as with Xen..

Jason

[PATCH 2/2] iommu/vt-d: Remove rmrr check in domain attaching device path

[Lu Baolu]

The core code now prevents devices with RMRR regions from being assigned
to user space. There is no need to check for this condition in individual
drivers. Remove it to avoid duplicate code.

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

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 8096273b034c..4efc87e74455 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2458,30 +2458,6 @@ static int dmar_domain_attach_device(struct dmar_domain *domain,
 	return 0;
 }
 
-static bool device_has_rmrr(struct device *dev)
-{
-	struct dmar_rmrr_unit *rmrr;
-	struct device *tmp;
-	int i;
-
-	rcu_read_lock();
-	for_each_rmrr_units(rmrr) {
-		/*
-		 * Return TRUE if this RMRR contains the device that
-		 * is passed in.
-		 */
-		for_each_active_dev_scope(rmrr->devices,
-					  rmrr->devices_cnt, i, tmp)
-			if (tmp == dev ||
-			    is_downstream_to_pci_bridge(dev, tmp)) {
-				rcu_read_unlock();
-				return true;
-			}
-	}
-	rcu_read_unlock();
-	return false;
-}
-
 /**
  * device_rmrr_is_relaxable - Test whether the RMRR of this device
  * is relaxable (ie. is allowed to be not enforced under some conditions)
@@ -2511,34 +2487,6 @@ static bool device_rmrr_is_relaxable(struct device *dev)
 		return false;
 }
 
-/*
- * There are a couple cases where we need to restrict the functionality of
- * devices associated with RMRRs.  The first is when evaluating a device for
- * identity mapping because problems exist when devices are moved in and out
- * of domains and their respective RMRR information is lost.  This means that
- * a device with associated RMRRs will never be in a "passthrough" domain.
- * The second is use of the device through the IOMMU API.  This interface
- * expects to have full control of the IOVA space for the device.  We cannot
- * satisfy both the requirement that RMRR access is maintained and have an
- * unencumbered IOVA space.  We also have no ability to quiesce the device's
- * use of the RMRR space or even inform the IOMMU API user of the restriction.
- * We therefore prevent devices associated with an RMRR from participating in
- * the IOMMU API, which eliminates them from device assignment.
- *
- * In both cases, devices which have relaxable RMRRs are not concerned by this
- * restriction. See device_rmrr_is_relaxable comment.
- */
-static bool device_is_rmrr_locked(struct device *dev)
-{
-	if (!device_has_rmrr(dev))
-		return false;
-
-	if (device_rmrr_is_relaxable(dev))
-		return false;
-
-	return true;
-}
-
 /*
  * Return the required default domain type for a specific device.
  *
@@ -4146,12 +4094,6 @@ static int intel_iommu_attach_device(struct iommu_domain *domain,
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	int ret;
 
-	if (domain->type == IOMMU_DOMAIN_UNMANAGED &&
-	    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 (info->domain)
 		device_block_translation(dev);
 
-- 
2.34.1

Re: [PATCH 2/2] iommu/vt-d: Remove rmrr check in domain attaching device path

[Jason Gunthorpe]

On Wed, Jun 07, 2023 at 11:51:45AM +0800, Lu Baolu wrote:
> The core code now prevents devices with RMRR regions from being assigned
> to user space. There is no need to check for this condition in individual
> drivers. Remove it to avoid duplicate code.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 58 -------------------------------------
>  1 file changed, 58 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

I'm happy to see this leave drive code!

Jason