* [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities
@ 2021-05-12 7:04 Lu Baolu
2021-05-12 8:30 ` Tian, Kevin
0 siblings, 1 reply; 5+ messages in thread
From: Lu Baolu @ 2021-05-12 7:04 UTC (permalink / raw)
To: Joerg Roedel, Will Deacon
Cc: ashok.raj, kevin.tian, jacob.jun.pan, yi.l.liu, sanjay.k.kumar,
iommu, linux-kernel, Lu Baolu
Current VT-d implementation supports nested translation only if all
underlying IOMMUs support the nested capability. This is unnecessary
as the upper layer is allowed to create different containers and set
them with different type of iommu backend. The IOMMU driver needs to
guarantee that devices attached to a nested mode iommu_domain should
support nested capabilility.
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index f1742da42478..1cd4840e6f9f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4755,6 +4755,13 @@ static int prepare_domain_attach_device(struct iommu_domain *domain,
if (!iommu)
return -ENODEV;
+ if ((dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE) &&
+ !ecap_nest(iommu->ecap)) {
+ dev_err(dev, "%s: iommu not support nested translation\n",
+ iommu->name);
+ return -EINVAL;
+ }
+
/* check if this iommu agaw is sufficient for max mapped address */
addr_width = agaw_to_width(iommu->agaw);
if (addr_width > cap_mgaw(iommu->cap))
@@ -5451,11 +5458,21 @@ static int
intel_iommu_enable_nesting(struct iommu_domain *domain)
{
struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+ struct dmar_drhd_unit *drhd;
+ struct intel_iommu *iommu;
+ bool has_nesting = false;
unsigned long flags;
- int ret = -ENODEV;
+ int ret = -EINVAL;
+
+ for_each_active_iommu(iommu, drhd)
+ if (ecap_nest(iommu->ecap))
+ has_nesting = true;
+
+ if (!has_nesting)
+ return -ENODEV;
spin_lock_irqsave(&device_domain_lock, flags);
- if (nested_mode_support() && list_empty(&dmar_domain->devices)) {
+ if (list_empty(&dmar_domain->devices)) {
dmar_domain->flags |= DOMAIN_FLAG_NESTING_MODE;
dmar_domain->flags &= ~DOMAIN_FLAG_USE_FIRST_LEVEL;
ret = 0;
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities
2021-05-12 7:04 [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities Lu Baolu
@ 2021-05-12 8:30 ` Tian, Kevin
2021-05-12 11:31 ` Lu Baolu
0 siblings, 1 reply; 5+ messages in thread
From: Tian, Kevin @ 2021-05-12 8:30 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel, Will Deacon
Cc: Raj, Ashok, Pan, Jacob jun, Liu, Yi L, Kumar, Sanjay K, iommu,
linux-kernel
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Wednesday, May 12, 2021 3:04 PM
>
> Current VT-d implementation supports nested translation only if all
> underlying IOMMUs support the nested capability. This is unnecessary
> as the upper layer is allowed to create different containers and set
> them with different type of iommu backend. The IOMMU driver needs to
> guarantee that devices attached to a nested mode iommu_domain should
> support nested capabilility.
so the consistency check is now applied only to the IOMMUs that are
spanned by a given iommu_domain?
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index f1742da42478..1cd4840e6f9f 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -4755,6 +4755,13 @@ static int prepare_domain_attach_device(struct
> iommu_domain *domain,
> if (!iommu)
> return -ENODEV;
>
> + if ((dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE) &&
> + !ecap_nest(iommu->ecap)) {
> + dev_err(dev, "%s: iommu not support nested translation\n",
> + iommu->name);
> + return -EINVAL;
> + }
> +
> /* check if this iommu agaw is sufficient for max mapped address */
> addr_width = agaw_to_width(iommu->agaw);
> if (addr_width > cap_mgaw(iommu->cap))
> @@ -5451,11 +5458,21 @@ static int
> intel_iommu_enable_nesting(struct iommu_domain *domain)
> {
> struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> + struct dmar_drhd_unit *drhd;
> + struct intel_iommu *iommu;
> + bool has_nesting = false;
> unsigned long flags;
> - int ret = -ENODEV;
> + int ret = -EINVAL;
> +
> + for_each_active_iommu(iommu, drhd)
> + if (ecap_nest(iommu->ecap))
> + has_nesting = true;
> +
> + if (!has_nesting)
> + return -ENODEV;
Isn't above still doing global consistency check?
>
> spin_lock_irqsave(&device_domain_lock, flags);
> - if (nested_mode_support() && list_empty(&dmar_domain->devices))
> {
> + if (list_empty(&dmar_domain->devices)) {
> dmar_domain->flags |= DOMAIN_FLAG_NESTING_MODE;
> dmar_domain->flags &= ~DOMAIN_FLAG_USE_FIRST_LEVEL;
> ret = 0;
> --
> 2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities
2021-05-12 8:30 ` Tian, Kevin
@ 2021-05-12 11:31 ` Lu Baolu
2021-05-13 2:26 ` Tian, Kevin
0 siblings, 1 reply; 5+ messages in thread
From: Lu Baolu @ 2021-05-12 11:31 UTC (permalink / raw)
To: Tian, Kevin, Joerg Roedel, Will Deacon
Cc: baolu.lu, Raj, Ashok, Pan, Jacob jun, Liu, Yi L, Kumar, Sanjay K,
iommu, linux-kernel
Hi Kevin,
On 5/12/21 4:30 PM, Tian, Kevin wrote:
>> From: Lu Baolu <baolu.lu@linux.intel.com>
>> Sent: Wednesday, May 12, 2021 3:04 PM
>>
>> Current VT-d implementation supports nested translation only if all
>> underlying IOMMUs support the nested capability. This is unnecessary
>> as the upper layer is allowed to create different containers and set
>> them with different type of iommu backend. The IOMMU driver needs to
>> guarantee that devices attached to a nested mode iommu_domain should
>> support nested capabilility.
>
> so the consistency check is now applied only to the IOMMUs that are
> spanned by a given iommu_domain?
Yes.
>
>>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>> drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++--
>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index f1742da42478..1cd4840e6f9f 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -4755,6 +4755,13 @@ static int prepare_domain_attach_device(struct
>> iommu_domain *domain,
>> if (!iommu)
>> return -ENODEV;
>>
>> + if ((dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE) &&
>> + !ecap_nest(iommu->ecap)) {
>> + dev_err(dev, "%s: iommu not support nested translation\n",
>> + iommu->name);
>> + return -EINVAL;
>> + }
>> +
>> /* check if this iommu agaw is sufficient for max mapped address */
>> addr_width = agaw_to_width(iommu->agaw);
>> if (addr_width > cap_mgaw(iommu->cap))
>> @@ -5451,11 +5458,21 @@ static int
>> intel_iommu_enable_nesting(struct iommu_domain *domain)
>> {
>> struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>> + struct dmar_drhd_unit *drhd;
>> + struct intel_iommu *iommu;
>> + bool has_nesting = false;
>> unsigned long flags;
>> - int ret = -ENODEV;
>> + int ret = -EINVAL;
>> +
>> + for_each_active_iommu(iommu, drhd)
>> + if (ecap_nest(iommu->ecap))
>> + has_nesting = true;
>> +
>> + if (!has_nesting)
>> + return -ENODEV;
>
> Isn't above still doing global consistency check?
The logic is if nested mode is globally unsupported, return false.
>
>>
>> spin_lock_irqsave(&device_domain_lock, flags);
>> - if (nested_mode_support() && list_empty(&dmar_domain->devices))
>> {
>> + if (list_empty(&dmar_domain->devices)) {
>> dmar_domain->flags |= DOMAIN_FLAG_NESTING_MODE;
>> dmar_domain->flags &= ~DOMAIN_FLAG_USE_FIRST_LEVEL;
>> ret = 0;
>> --
>> 2.25.1
>
Best regards,
baolu
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities
2021-05-12 11:31 ` Lu Baolu
@ 2021-05-13 2:26 ` Tian, Kevin
2021-05-13 3:10 ` Lu Baolu
0 siblings, 1 reply; 5+ messages in thread
From: Tian, Kevin @ 2021-05-13 2:26 UTC (permalink / raw)
To: Lu Baolu, Joerg Roedel, Will Deacon
Cc: Raj, Ashok, Kumar, Sanjay K, linux-kernel, iommu, Pan, Jacob jun
> From: Lu Baolu
> Sent: Wednesday, May 12, 2021 7:31 PM
>
> Hi Kevin,
>
> On 5/12/21 4:30 PM, Tian, Kevin wrote:
> >> From: Lu Baolu <baolu.lu@linux.intel.com>
> >> Sent: Wednesday, May 12, 2021 3:04 PM
> >>
> >> Current VT-d implementation supports nested translation only if all
> >> underlying IOMMUs support the nested capability. This is unnecessary
> >> as the upper layer is allowed to create different containers and set
> >> them with different type of iommu backend. The IOMMU driver needs to
> >> guarantee that devices attached to a nested mode iommu_domain should
> >> support nested capabilility.
> >
> > so the consistency check is now applied only to the IOMMUs that are
> > spanned by a given iommu_domain?
>
> Yes.
>
> >
> >>
> >> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> >> ---
> >> drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++--
> >> 1 file changed, 19 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> >> index f1742da42478..1cd4840e6f9f 100644
> >> --- a/drivers/iommu/intel/iommu.c
> >> +++ b/drivers/iommu/intel/iommu.c
> >> @@ -4755,6 +4755,13 @@ static int
> prepare_domain_attach_device(struct
> >> iommu_domain *domain,
> >> if (!iommu)
> >> return -ENODEV;
> >>
> >> + if ((dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE) &&
> >> + !ecap_nest(iommu->ecap)) {
> >> + dev_err(dev, "%s: iommu not support nested translation\n",
> >> + iommu->name);
> >> + return -EINVAL;
> >> + }
> >> +
> >> /* check if this iommu agaw is sufficient for max mapped address */
> >> addr_width = agaw_to_width(iommu->agaw);
> >> if (addr_width > cap_mgaw(iommu->cap))
> >> @@ -5451,11 +5458,21 @@ static int
> >> intel_iommu_enable_nesting(struct iommu_domain *domain)
> >> {
> >> struct dmar_domain *dmar_domain = to_dmar_domain(domain);
> >> + struct dmar_drhd_unit *drhd;
> >> + struct intel_iommu *iommu;
> >> + bool has_nesting = false;
> >> unsigned long flags;
> >> - int ret = -ENODEV;
> >> + int ret = -EINVAL;
> >> +
> >> + for_each_active_iommu(iommu, drhd)
> >> + if (ecap_nest(iommu->ecap))
> >> + has_nesting = true;
> >> +
> >> + if (!has_nesting)
> >> + return -ENODEV;
> >
> > Isn't above still doing global consistency check?
>
> The logic is if nested mode is globally unsupported, return false.
why is this check required? anyway you already have the check when
prepare device attachment, and only at that point you have accurate
info for which iommu to be checked. This check looks meaningless
as even if some IOMMUs support nesting it doesn't mean the IOMMU
relevant to this domain support it.
>
> >
> >>
> >> spin_lock_irqsave(&device_domain_lock, flags);
> >> - if (nested_mode_support() && list_empty(&dmar_domain->devices))
> >> {
> >> + if (list_empty(&dmar_domain->devices)) {
> >> dmar_domain->flags |= DOMAIN_FLAG_NESTING_MODE;
> >> dmar_domain->flags &= ~DOMAIN_FLAG_USE_FIRST_LEVEL;
> >> ret = 0;
> >> --
> >> 2.25.1
> >
>
> Best regards,
> baolu
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities
2021-05-13 2:26 ` Tian, Kevin
@ 2021-05-13 3:10 ` Lu Baolu
0 siblings, 0 replies; 5+ messages in thread
From: Lu Baolu @ 2021-05-13 3:10 UTC (permalink / raw)
To: Tian, Kevin, Joerg Roedel, Will Deacon
Cc: baolu.lu, Raj, Ashok, Kumar, Sanjay K, linux-kernel, iommu, Pan,
Jacob jun
On 5/13/21 10:26 AM, Tian, Kevin wrote:
>> From: Lu Baolu
>> Sent: Wednesday, May 12, 2021 7:31 PM
>>
>> Hi Kevin,
>>
>> On 5/12/21 4:30 PM, Tian, Kevin wrote:
>>>> From: Lu Baolu<baolu.lu@linux.intel.com>
>>>> Sent: Wednesday, May 12, 2021 3:04 PM
>>>>
>>>> Current VT-d implementation supports nested translation only if all
>>>> underlying IOMMUs support the nested capability. This is unnecessary
>>>> as the upper layer is allowed to create different containers and set
>>>> them with different type of iommu backend. The IOMMU driver needs to
>>>> guarantee that devices attached to a nested mode iommu_domain should
>>>> support nested capabilility.
>>> so the consistency check is now applied only to the IOMMUs that are
>>> spanned by a given iommu_domain?
>> Yes.
>>
>>>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>>>> ---
>>>> drivers/iommu/intel/iommu.c | 21 +++++++++++++++++++--
>>>> 1 file changed, 19 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>>> index f1742da42478..1cd4840e6f9f 100644
>>>> --- a/drivers/iommu/intel/iommu.c
>>>> +++ b/drivers/iommu/intel/iommu.c
>>>> @@ -4755,6 +4755,13 @@ static int
>> prepare_domain_attach_device(struct
>>>> iommu_domain *domain,
>>>> if (!iommu)
>>>> return -ENODEV;
>>>>
>>>> + if ((dmar_domain->flags & DOMAIN_FLAG_NESTING_MODE) &&
>>>> + !ecap_nest(iommu->ecap)) {
>>>> + dev_err(dev, "%s: iommu not support nested translation\n",
>>>> + iommu->name);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> /* check if this iommu agaw is sufficient for max mapped address */
>>>> addr_width = agaw_to_width(iommu->agaw);
>>>> if (addr_width > cap_mgaw(iommu->cap))
>>>> @@ -5451,11 +5458,21 @@ static int
>>>> intel_iommu_enable_nesting(struct iommu_domain *domain)
>>>> {
>>>> struct dmar_domain *dmar_domain = to_dmar_domain(domain);
>>>> + struct dmar_drhd_unit *drhd;
>>>> + struct intel_iommu *iommu;
>>>> + bool has_nesting = false;
>>>> unsigned long flags;
>>>> - int ret = -ENODEV;
>>>> + int ret = -EINVAL;
>>>> +
>>>> + for_each_active_iommu(iommu, drhd)
>>>> + if (ecap_nest(iommu->ecap))
>>>> + has_nesting = true;
>>>> +
>>>> + if (!has_nesting)
>>>> + return -ENODEV;
>>> Isn't above still doing global consistency check?
>> The logic is if nested mode is globally unsupported, return false.
> why is this check required? anyway you already have the check when
> prepare device attachment, and only at that point you have accurate
> info for which iommu to be checked. This check looks meaningless
> as even if some IOMMUs support nesting it doesn't mean the IOMMU
> relevant to this domain support it.
>
Get you. It's really unnecessary. I will drop this check in the new
version.
Best regards,
baolu
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-05-13 3:11 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-12 7:04 [PATCH 1/1] iommu/vt-d: Support asynchronous IOMMU nested capabilities Lu Baolu
2021-05-12 8:30 ` Tian, Kevin
2021-05-12 11:31 ` Lu Baolu
2021-05-13 2:26 ` Tian, Kevin
2021-05-13 3:10 ` Lu Baolu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).