diff mbox series

[v2,12/12] iommu/vt-d: Add set_dev_pasid callback for nested domain

Message ID 20240412081516.31168-13-yi.l.liu@intel.com (mailing list archive)
State New, archived
Headers show
Series iommufd support pasid attach/replace | expand

Commit Message

Liu, Yi L April 12, 2024, 8:15 a.m. UTC
From: Lu Baolu <baolu.lu@linux.intel.com>

This allows the upper layers to set a nested type domain to a PASID of a
device if the PASID feature is supported by the IOMMU hardware.

The set_dev_pasid callback for non-nested domain has already be there, so
this only needs to add it for nested domains. Note that the S2 domain with
dirty tracking capability is not supported yet as no user for now.

Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
---
 drivers/iommu/intel/iommu.c  | 23 +++++++++++++++++++----
 drivers/iommu/intel/iommu.h  |  3 +++
 drivers/iommu/intel/nested.c | 15 +++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

Comments

Tian, Kevin April 17, 2024, 9:25 a.m. UTC | #1
> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Friday, April 12, 2024 4:15 PM
> 
> From: Lu Baolu <baolu.lu@linux.intel.com>
> 
> This allows the upper layers to set a nested type domain to a PASID of a
> device if the PASID feature is supported by the IOMMU hardware.
> 
> The set_dev_pasid callback for non-nested domain has already be there, so
> this only needs to add it for nested domains. Note that the S2 domain with
> dirty tracking capability is not supported yet as no user for now.

S2 domain does support dirty tracking. Do you mean the specific
check in intel_iommu_set_dev_pasid() i.e. pasid-granular dirty
tracking is not supported yet?

> +static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
> +				      struct device *dev, ioasid_t pasid,
> +				      struct iommu_domain *old)
> +{
> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> +	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> +	struct intel_iommu *iommu = info->iommu;
> +
> +	if (iommu->agaw < dmar_domain->s2_domain->agaw)
> +		return -EINVAL;
> +

this check is covered by prepare_domain_attach_device() already.
Liu, Yi L April 30, 2024, 9:19 a.m. UTC | #2
On 2024/4/17 17:25, Tian, Kevin wrote:
>> From: Liu, Yi L <yi.l.liu@intel.com>
>> Sent: Friday, April 12, 2024 4:15 PM
>>
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>
>> This allows the upper layers to set a nested type domain to a PASID of a
>> device if the PASID feature is supported by the IOMMU hardware.
>>
>> The set_dev_pasid callback for non-nested domain has already be there, so
>> this only needs to add it for nested domains. Note that the S2 domain with
>> dirty tracking capability is not supported yet as no user for now.
> 
> S2 domain does support dirty tracking. Do you mean the specific
> check in intel_iommu_set_dev_pasid() i.e. pasid-granular dirty
> tracking is not supported yet?

yes. We may remove this check when real usage comes. e.g. SIOV.

>> +static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
>> +				      struct device *dev, ioasid_t pasid,
>> +				      struct iommu_domain *old)
>> +{
>> +	struct device_domain_info *info = dev_iommu_priv_get(dev);
>> +	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>> +	struct intel_iommu *iommu = info->iommu;
>> +
>> +	if (iommu->agaw < dmar_domain->s2_domain->agaw)
>> +		return -EINVAL;
>> +
> 
> this check is covered by prepare_domain_attach_device() already.

This was added to avoid modifying the s2_domain's agaw. I'm fine to remove
it personally as the existing attach path also needs to update domain's
agaw per device attachment. @Baolu, how about your opinion?
Baolu Lu May 6, 2024, 7:42 a.m. UTC | #3
On 2024/4/30 17:19, Yi Liu wrote:
> On 2024/4/17 17:25, Tian, Kevin wrote:
>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>> Sent: Friday, April 12, 2024 4:15 PM
>>>
>>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>>
>>> This allows the upper layers to set a nested type domain to a PASID of a
>>> device if the PASID feature is supported by the IOMMU hardware.
>>>
>>> The set_dev_pasid callback for non-nested domain has already be 
>>> there, so
>>> this only needs to add it for nested domains. Note that the S2 domain 
>>> with
>>> dirty tracking capability is not supported yet as no user for now.
>>
>> S2 domain does support dirty tracking. Do you mean the specific
>> check in intel_iommu_set_dev_pasid() i.e. pasid-granular dirty
>> tracking is not supported yet?
> 
> yes. We may remove this check when real usage comes. e.g. SIOV.
> 
>>> +static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
>>> +                      struct device *dev, ioasid_t pasid,
>>> +                      struct iommu_domain *old)
>>> +{
>>> +    struct device_domain_info *info = dev_iommu_priv_get(dev);
>>> +    struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>>> +    struct intel_iommu *iommu = info->iommu;
>>> +
>>> +    if (iommu->agaw < dmar_domain->s2_domain->agaw)
>>> +        return -EINVAL;
>>> +
>>
>> this check is covered by prepare_domain_attach_device() already.
> 
> This was added to avoid modifying the s2_domain's agaw. I'm fine to remove
> it personally as the existing attach path also needs to update domain's
> agaw per device attachment. @Baolu, how about your opinion?

We still need something to do before we can safely remove this check.
All the domain allocation interfaces should eventually have the device
pointer as the input, and all domain attributions could be initialized
during domain allocation. In the attach paths, it should return -EINVAL
directly if the domain is not compatible with the iommu for the device.

Best regards,
baolu
Jason Gunthorpe May 6, 2024, 1:36 p.m. UTC | #4
On Mon, May 06, 2024 at 03:42:21PM +0800, Baolu Lu wrote:
> On 2024/4/30 17:19, Yi Liu wrote:
> > On 2024/4/17 17:25, Tian, Kevin wrote:
> > > > From: Liu, Yi L <yi.l.liu@intel.com>
> > > > Sent: Friday, April 12, 2024 4:15 PM
> > > > 
> > > > From: Lu Baolu <baolu.lu@linux.intel.com>
> > > > 
> > > > This allows the upper layers to set a nested type domain to a PASID of a
> > > > device if the PASID feature is supported by the IOMMU hardware.
> > > > 
> > > > The set_dev_pasid callback for non-nested domain has already be
> > > > there, so
> > > > this only needs to add it for nested domains. Note that the S2
> > > > domain with
> > > > dirty tracking capability is not supported yet as no user for now.
> > > 
> > > S2 domain does support dirty tracking. Do you mean the specific
> > > check in intel_iommu_set_dev_pasid() i.e. pasid-granular dirty
> > > tracking is not supported yet?
> > 
> > yes. We may remove this check when real usage comes. e.g. SIOV.
> > 
> > > > +static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
> > > > +                      struct device *dev, ioasid_t pasid,
> > > > +                      struct iommu_domain *old)
> > > > +{
> > > > +    struct device_domain_info *info = dev_iommu_priv_get(dev);
> > > > +    struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> > > > +    struct intel_iommu *iommu = info->iommu;
> > > > +
> > > > +    if (iommu->agaw < dmar_domain->s2_domain->agaw)
> > > > +        return -EINVAL;
> > > > +
> > > 
> > > this check is covered by prepare_domain_attach_device() already.
> > 
> > This was added to avoid modifying the s2_domain's agaw. I'm fine to remove
> > it personally as the existing attach path also needs to update domain's
> > agaw per device attachment. @Baolu, how about your opinion?
> 
> We still need something to do before we can safely remove this check.
> All the domain allocation interfaces should eventually have the device
> pointer as the input, and all domain attributions could be initialized
> during domain allocation. In the attach paths, it should return -EINVAL
> directly if the domain is not compatible with the iommu for the device.

Yes, and this is already true for PASID.

I feel we could reasonably insist that domanis used with PASID are
allocated with a non-NULL dev.

If so it means we need to fixup the domain allocation in iommufd as
part of the pasid series, and Intel will have to implement
alloc_domain_paging().

Jason
Liu, Yi L May 7, 2024, 2:28 a.m. UTC | #5
On 2024/5/6 21:36, Jason Gunthorpe wrote:
> On Mon, May 06, 2024 at 03:42:21PM +0800, Baolu Lu wrote:
>> On 2024/4/30 17:19, Yi Liu wrote:
>>> On 2024/4/17 17:25, Tian, Kevin wrote:
>>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>>> Sent: Friday, April 12, 2024 4:15 PM
>>>>>
>>>>> From: Lu Baolu <baolu.lu@linux.intel.com>
>>>>>
>>>>> This allows the upper layers to set a nested type domain to a PASID of a
>>>>> device if the PASID feature is supported by the IOMMU hardware.
>>>>>
>>>>> The set_dev_pasid callback for non-nested domain has already be
>>>>> there, so
>>>>> this only needs to add it for nested domains. Note that the S2
>>>>> domain with
>>>>> dirty tracking capability is not supported yet as no user for now.
>>>>
>>>> S2 domain does support dirty tracking. Do you mean the specific
>>>> check in intel_iommu_set_dev_pasid() i.e. pasid-granular dirty
>>>> tracking is not supported yet?
>>>
>>> yes. We may remove this check when real usage comes. e.g. SIOV.
>>>
>>>>> +static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
>>>>> +                      struct device *dev, ioasid_t pasid,
>>>>> +                      struct iommu_domain *old)
>>>>> +{
>>>>> +    struct device_domain_info *info = dev_iommu_priv_get(dev);
>>>>> +    struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>>>>> +    struct intel_iommu *iommu = info->iommu;
>>>>> +
>>>>> +    if (iommu->agaw < dmar_domain->s2_domain->agaw)
>>>>> +        return -EINVAL;
>>>>> +
>>>>
>>>> this check is covered by prepare_domain_attach_device() already.
>>>
>>> This was added to avoid modifying the s2_domain's agaw. I'm fine to remove
>>> it personally as the existing attach path also needs to update domain's
>>> agaw per device attachment. @Baolu, how about your opinion?
>>
>> We still need something to do before we can safely remove this check.
>> All the domain allocation interfaces should eventually have the device
>> pointer as the input, and all domain attributions could be initialized
>> during domain allocation. In the attach paths, it should return -EINVAL
>> directly if the domain is not compatible with the iommu for the device.
> 
> Yes, and this is already true for PASID.

I'm not quite get why it is already true for PASID. I think Baolu's remark
is general to domains attached to either RID or PASID.

> I feel we could reasonably insist that domanis used with PASID are
> allocated with a non-NULL dev.

Any special reason for this disclaim?

> 
> If so it means we need to fixup the domain allocation in iommufd as
> part of the pasid series, and Intel will have to implement
> alloc_domain_paging().

I agree implementing alloc_domain_paging() is the final solution to avoid
such dynamic modifications to domain's caps. If it's really needed for
PASID series now, I can add it in next version. :)
Jason Gunthorpe May 7, 2024, 3:18 p.m. UTC | #6
On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote:
> > > We still need something to do before we can safely remove this check.
> > > All the domain allocation interfaces should eventually have the device
> > > pointer as the input, and all domain attributions could be initialized
> > > during domain allocation. In the attach paths, it should return -EINVAL
> > > directly if the domain is not compatible with the iommu for the device.
> > 
> > Yes, and this is already true for PASID.
> 
> I'm not quite get why it is already true for PASID. I think Baolu's remark
> is general to domains attached to either RID or PASID.
> 
> > I feel we could reasonably insist that domanis used with PASID are
> > allocated with a non-NULL dev.
> 
> Any special reason for this disclaim?

If it makes the driver easier, why not?

PASID is special since PASID is barely used, we could insist that
new PASID users also use the new domian_alloc API.

> I agree implementing alloc_domain_paging() is the final solution to avoid
> such dynamic modifications to domain's caps. If it's really needed for
> PASID series now, I can add it in next version. :)

Well, if it is needed. If you can do this some other way that is
reasonable then sure

Jason
Liu, Yi L May 8, 2024, 6:10 a.m. UTC | #7
On 2024/5/7 23:18, Jason Gunthorpe wrote:
> On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote:
>>>> We still need something to do before we can safely remove this check.
>>>> All the domain allocation interfaces should eventually have the device
>>>> pointer as the input, and all domain attributions could be initialized
>>>> during domain allocation. In the attach paths, it should return -EINVAL
>>>> directly if the domain is not compatible with the iommu for the device.
>>>
>>> Yes, and this is already true for PASID.
>>
>> I'm not quite get why it is already true for PASID. I think Baolu's remark
>> is general to domains attached to either RID or PASID.
>>
>>> I feel we could reasonably insist that domanis used with PASID are
>>> allocated with a non-NULL dev.
>>
>> Any special reason for this disclaim?
> 
> If it makes the driver easier, why not?

yep.

> PASID is special since PASID is barely used, we could insist that
> new PASID users also use the new domian_alloc API.

Ok. I have one doubt on how to make it in iommufd core. Shall the iommufd
core call ops->domain_alloc_paging() directly or still call
ops->domain_alloc_user() while ops->domain_alloc_user() flows into the
paging domain allocation with non-null dev?

> 
>> I agree implementing alloc_domain_paging() is the final solution to avoid
>> such dynamic modifications to domain's caps. If it's really needed for
>> PASID series now, I can add it in next version. :)
> 
> Well, if it is needed. If you can do this some other way that is
> reasonable then sure

At first, I'd like to keep this agaw check here and remove it after
implementing the ops->domain_alloc_paging() and retire domain_alloc op
in intel iommu driver side. I have such thoughts as the RID and PASID
attach path have the same concern w.r.t. agaw and other caps :)
Jason Gunthorpe May 8, 2024, 12:25 p.m. UTC | #8
On Wed, May 08, 2024 at 02:10:05PM +0800, Yi Liu wrote:
> On 2024/5/7 23:18, Jason Gunthorpe wrote:
> > On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote:
> > > > > We still need something to do before we can safely remove this check.
> > > > > All the domain allocation interfaces should eventually have the device
> > > > > pointer as the input, and all domain attributions could be initialized
> > > > > during domain allocation. In the attach paths, it should return -EINVAL
> > > > > directly if the domain is not compatible with the iommu for the device.
> > > > 
> > > > Yes, and this is already true for PASID.
> > > 
> > > I'm not quite get why it is already true for PASID. I think Baolu's remark
> > > is general to domains attached to either RID or PASID.
> > > 
> > > > I feel we could reasonably insist that domanis used with PASID are
> > > > allocated with a non-NULL dev.
> > > 
> > > Any special reason for this disclaim?
> > 
> > If it makes the driver easier, why not?
> 
> yep.
> 
> > PASID is special since PASID is barely used, we could insist that
> > new PASID users also use the new domian_alloc API.
> 
> Ok. I have one doubt on how to make it in iommufd core. Shall the iommufd
> core call ops->domain_alloc_paging() directly or still call
> ops->domain_alloc_user() while ops->domain_alloc_user() flows into the
> paging domain allocation with non-null dev?

I mostly figured we'd need a new iommu_domain_alloc_dev() sort of
thing? VFIO should be changed over too.

Jason
Liu, Yi L May 8, 2024, 1:26 p.m. UTC | #9
On 2024/5/8 20:25, Jason Gunthorpe wrote:
> On Wed, May 08, 2024 at 02:10:05PM +0800, Yi Liu wrote:
>> On 2024/5/7 23:18, Jason Gunthorpe wrote:
>>> On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote:
>>>>>> We still need something to do before we can safely remove this check.
>>>>>> All the domain allocation interfaces should eventually have the device
>>>>>> pointer as the input, and all domain attributions could be initialized
>>>>>> during domain allocation. In the attach paths, it should return -EINVAL
>>>>>> directly if the domain is not compatible with the iommu for the device.
>>>>>
>>>>> Yes, and this is already true for PASID.
>>>>
>>>> I'm not quite get why it is already true for PASID. I think Baolu's remark
>>>> is general to domains attached to either RID or PASID.
>>>>
>>>>> I feel we could reasonably insist that domanis used with PASID are
>>>>> allocated with a non-NULL dev.
>>>>
>>>> Any special reason for this disclaim?
>>>
>>> If it makes the driver easier, why not?
>>
>> yep.
>>
>>> PASID is special since PASID is barely used, we could insist that
>>> new PASID users also use the new domian_alloc API.
>>
>> Ok. I have one doubt on how to make it in iommufd core. Shall the iommufd
>> core call ops->domain_alloc_paging() directly or still call
>> ops->domain_alloc_user() while ops->domain_alloc_user() flows into the
>> paging domain allocation with non-null dev?
> 
> I mostly figured we'd need a new iommu_domain_alloc_dev() sort of
> thing? VFIO should be changed over too.

Would this new iommu-domain_alloc_dev() have flags and user_data input?
As below code snippet, the existing iommufd core uses domain_alloc_user
op to allocate the s2 domain (paging domain), and will fall back to
iommu_domain_alloc() only if the domain_alloc_user op does not exist. The
typical reason is to use domain_alloc_user op is to allocate a paging
domain with NESTED_PARENT flag. I suppose the new iommu_domain_alloc_dev()
shall allow allocating s2 domain with NESTED_PARENT as well. right?


struct iommufd_hwpt_paging *
iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
			  struct iommufd_device *idev, u32 flags,
			  bool immediate_attach,
			  const struct iommu_user_data *user_data)
{

	...
	if (ops->domain_alloc_user) {
		hwpt->domain = ops->domain_alloc_user(idev->dev, flags, NULL,
						      user_data);
		if (IS_ERR(hwpt->domain)) {
			rc = PTR_ERR(hwpt->domain);
			hwpt->domain = NULL;
			goto out_abort;
		}
		hwpt->domain->owner = ops;
	} else {
		hwpt->domain = iommu_domain_alloc(idev->dev->bus);
		if (!hwpt->domain) {
			rc = -ENOMEM;
			goto out_abort;
		}
	}
	...
}
Jason Gunthorpe May 8, 2024, 2:11 p.m. UTC | #10
On Wed, May 08, 2024 at 09:26:47PM +0800, Yi Liu wrote:
> On 2024/5/8 20:25, Jason Gunthorpe wrote:
> > On Wed, May 08, 2024 at 02:10:05PM +0800, Yi Liu wrote:
> > > On 2024/5/7 23:18, Jason Gunthorpe wrote:
> > > > On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote:
> > > > > > > We still need something to do before we can safely remove this check.
> > > > > > > All the domain allocation interfaces should eventually have the device
> > > > > > > pointer as the input, and all domain attributions could be initialized
> > > > > > > during domain allocation. In the attach paths, it should return -EINVAL
> > > > > > > directly if the domain is not compatible with the iommu for the device.
> > > > > > 
> > > > > > Yes, and this is already true for PASID.
> > > > > 
> > > > > I'm not quite get why it is already true for PASID. I think Baolu's remark
> > > > > is general to domains attached to either RID or PASID.
> > > > > 
> > > > > > I feel we could reasonably insist that domanis used with PASID are
> > > > > > allocated with a non-NULL dev.
> > > > > 
> > > > > Any special reason for this disclaim?
> > > > 
> > > > If it makes the driver easier, why not?
> > > 
> > > yep.
> > > 
> > > > PASID is special since PASID is barely used, we could insist that
> > > > new PASID users also use the new domian_alloc API.
> > > 
> > > Ok. I have one doubt on how to make it in iommufd core. Shall the iommufd
> > > core call ops->domain_alloc_paging() directly or still call
> > > ops->domain_alloc_user() while ops->domain_alloc_user() flows into the
> > > paging domain allocation with non-null dev?
> > 
> > I mostly figured we'd need a new iommu_domain_alloc_dev() sort of
> > thing? VFIO should be changed over too.
> 
> Would this new iommu-domain_alloc_dev() have flags and user_data
> input?

No, it would be an in-kernel replacement for the existing API.

> As below code snippet, the existing iommufd core uses domain_alloc_user
> op to allocate the s2 domain (paging domain), and will fall back to
> iommu_domain_alloc() only if the domain_alloc_user op does not exist. The

Oh, right. Yeah we built it like that so that drivers would have
consistency that iommufd always uses the _user version if it exists.

> typical reason is to use domain_alloc_user op is to allocate a paging
> domain with NESTED_PARENT flag. I suppose the new iommu_domain_alloc_dev()
> shall allow allocating s2 domain with NESTED_PARENT as well. right?

No, it is just a simple replacement for iommu_domain_alloc() that does
exactly the same thing. We don't have any in-kernel use for anything
more fancy than a simple domain right now.

Jason
Liu, Yi L May 9, 2024, 2:22 p.m. UTC | #11
> From: Jason Gunthorpe <jgg@nvidia.com>
> 
> On Wed, May 08, 2024 at 09:26:47PM +0800, Yi Liu wrote:
> > On 2024/5/8 20:25, Jason Gunthorpe wrote:
> > > On Wed, May 08, 2024 at 02:10:05PM +0800, Yi Liu wrote:
> > > > On 2024/5/7 23:18, Jason Gunthorpe wrote:
> > > > > On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote:
> > > > > > > > We still need something to do before we can safely remove this check.
> > > > > > > > All the domain allocation interfaces should eventually have the device
> > > > > > > > pointer as the input, and all domain attributions could be initialized
> > > > > > > > during domain allocation. In the attach paths, it should return -EINVAL
> > > > > > > > directly if the domain is not compatible with the iommu for the device.
> > > > > > >
> > > > > > > Yes, and this is already true for PASID.
> > > > > >
> > > > > > I'm not quite get why it is already true for PASID. I think Baolu's remark
> > > > > > is general to domains attached to either RID or PASID.
> > > > > >
> > > > > > > I feel we could reasonably insist that domanis used with PASID are
> > > > > > > allocated with a non-NULL dev.
> > > > > >
> > > > > > Any special reason for this disclaim?
> > > > >
> > > > > If it makes the driver easier, why not?
> > > >
> > > > yep.
> > > >
> > > > > PASID is special since PASID is barely used, we could insist that
> > > > > new PASID users also use the new domian_alloc API.
> > > >
> > > > Ok. I have one doubt on how to make it in iommufd core. Shall the iommufd
> > > > core call ops->domain_alloc_paging() directly or still call
> > > > ops->domain_alloc_user() while ops->domain_alloc_user() flows into the
> > > > paging domain allocation with non-null dev?
> > >
> > > I mostly figured we'd need a new iommu_domain_alloc_dev() sort of
> > > thing? VFIO should be changed over too.
> >
> > Would this new iommu-domain_alloc_dev() have flags and user_data
> > input?
> 
> No, it would be an in-kernel replacement for the existing API.

Ok.

> > As below code snippet, the existing iommufd core uses domain_alloc_user
> > op to allocate the s2 domain (paging domain), and will fall back to
> > iommu_domain_alloc() only if the domain_alloc_user op does not exist. The
> 
> Oh, right. Yeah we built it like that so that drivers would have
> consistency that iommufd always uses the _user version if it exists.

Yep. This means for the iommu drivers that have implemented
domain_alloc_user op, it would not call into the new iommu_domain_alloc_dev(). 
So I would need to make the intel domain_alloc_user op allocate paging domain
with non-dev as well.

> > typical reason is to use domain_alloc_user op is to allocate a paging
> > domain with NESTED_PARENT flag. I suppose the new iommu_domain_alloc_dev()
> > shall allow allocating s2 domain with NESTED_PARENT as well. right?
> 
> No, it is just a simple replacement for iommu_domain_alloc() that does
> exactly the same thing. We don't have any in-kernel use for anything
> more fancy than a simple domain right now.

Yes.

Regards,
Yi Liu
diff mbox series

Patch

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 9e79ffdd47db..052b90917ced 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -319,6 +319,11 @@  static int domain_type_is_si(struct dmar_domain *domain)
 	return domain->domain.type == IOMMU_DOMAIN_IDENTITY;
 }
 
+static int domain_type_is_nested(struct dmar_domain *domain)
+{
+	return domain->domain.type == IOMMU_DOMAIN_NESTED;
+}
+
 static int domain_pfn_supported(struct dmar_domain *domain, unsigned long pfn)
 {
 	int addr_width = agaw_to_width(domain->agaw) - VTD_PAGE_SHIFT;
@@ -4626,9 +4631,9 @@  static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t pasid,
 	intel_drain_pasid_prq(dev, pasid);
 }
 
-static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
-				     struct device *dev, ioasid_t pasid,
-				     struct iommu_domain *old)
+int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+			      struct device *dev, ioasid_t pasid,
+			      struct iommu_domain *old)
 {
 	struct device_domain_info *info = dev_iommu_priv_get(dev);
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
@@ -4650,7 +4655,15 @@  static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
 	if (old)
 		intel_iommu_remove_dev_pasid(dev, pasid, old);
 
-	ret = prepare_domain_attach_device(domain, dev);
+	/*
+	 * Nested type domain should adjust its parent domain according
+	 * to iommu capability.
+	 */
+	if (domain_type_is_nested(dmar_domain))
+		ret = prepare_domain_attach_device(
+				&dmar_domain->s2_domain->domain, dev);
+	else
+		ret = prepare_domain_attach_device(domain, dev);
 	if (ret)
 		return ret;
 
@@ -4664,6 +4677,8 @@  static int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
 
 	if (domain_type_is_si(dmar_domain))
 		ret = intel_pasid_setup_pass_through(iommu, dev, pasid);
+	else if (domain_type_is_nested(dmar_domain))
+		ret = intel_pasid_setup_nested(iommu, dev, pasid, dmar_domain);
 	else if (dmar_domain->use_first_level)
 		ret = domain_setup_first_level(iommu, dmar_domain,
 					       dev, pasid);
diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
index 404d2476a877..3dfd183c9736 100644
--- a/drivers/iommu/intel/iommu.h
+++ b/drivers/iommu/intel/iommu.h
@@ -1082,6 +1082,9 @@  void device_block_translation(struct device *dev);
 int prepare_domain_attach_device(struct iommu_domain *domain,
 				 struct device *dev);
 void domain_update_iommu_cap(struct dmar_domain *domain);
+int intel_iommu_set_dev_pasid(struct iommu_domain *domain,
+			      struct device *dev, ioasid_t pasid,
+			      struct iommu_domain *old);
 
 int dmar_ir_support(void);
 
diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
index a7d68f3d518a..7cb124cc0ca0 100644
--- a/drivers/iommu/intel/nested.c
+++ b/drivers/iommu/intel/nested.c
@@ -70,6 +70,20 @@  static int intel_nested_attach_dev(struct iommu_domain *domain,
 	return 0;
 }
 
+static int intel_nested_set_dev_pasid(struct iommu_domain *domain,
+				      struct device *dev, ioasid_t pasid,
+				      struct iommu_domain *old)
+{
+	struct device_domain_info *info = dev_iommu_priv_get(dev);
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct intel_iommu *iommu = info->iommu;
+
+	if (iommu->agaw < dmar_domain->s2_domain->agaw)
+		return -EINVAL;
+
+	return intel_iommu_set_dev_pasid(domain, dev, pasid, old);
+}
+
 static void intel_nested_domain_free(struct iommu_domain *domain)
 {
 	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
@@ -170,6 +184,7 @@  static int intel_nested_cache_invalidate_user(struct iommu_domain *domain,
 
 static const struct iommu_domain_ops intel_nested_domain_ops = {
 	.attach_dev		= intel_nested_attach_dev,
+	.set_dev_pasid		= intel_nested_set_dev_pasid,
 	.free			= intel_nested_domain_free,
 	.cache_invalidate_user	= intel_nested_cache_invalidate_user,
 };