[PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup

Lu Baolu posted 7 patches 1 year, 4 months ago
There is a newer version of this series
[PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup
Posted by Lu Baolu 1 year, 4 months ago
We will use a global static identity domain. Reserve a static domain ID
for it.

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

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 723ea9f3f501..c019fb3b3e78 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -1440,10 +1440,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
 	 * entry for first-level or pass-through translation modes should
 	 * be programmed with a domain id different from those used for
 	 * second-level or nested translation. We reserve a domain id for
-	 * this purpose.
+	 * this purpose. This domain id is also used for identity domain
+	 * in legacy mode.
 	 */
-	if (sm_supported(iommu))
-		set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
+	set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
 
 	return 0;
 }
-- 
2.34.1
Re: [PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup
Posted by Jerry Snitselaar 1 year, 4 months ago
On Tue, Aug 06, 2024 at 10:39:37AM GMT, Lu Baolu wrote:
> We will use a global static identity domain. Reserve a static domain ID
> for it.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>

> ---
>  drivers/iommu/intel/iommu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 723ea9f3f501..c019fb3b3e78 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1440,10 +1440,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
>  	 * entry for first-level or pass-through translation modes should
>  	 * be programmed with a domain id different from those used for
>  	 * second-level or nested translation. We reserve a domain id for
> -	 * this purpose.
> +	 * this purpose. This domain id is also used for identity domain
> +	 * in legacy mode.
>  	 */
> -	if (sm_supported(iommu))
> -		set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
> +	set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
>  
>  	return 0;
>  }
> -- 
> 2.34.1
>
Re: [PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup
Posted by Jason Gunthorpe 1 year, 4 months ago
On Tue, Aug 06, 2024 at 10:39:37AM +0800, Lu Baolu wrote:
> We will use a global static identity domain. Reserve a static domain ID
> for it.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  drivers/iommu/intel/iommu.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 723ea9f3f501..c019fb3b3e78 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1440,10 +1440,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
>  	 * entry for first-level or pass-through translation modes should
>  	 * be programmed with a domain id different from those used for
>  	 * second-level or nested translation. We reserve a domain id for
> -	 * this purpose.
> +	 * this purpose. This domain id is also used for identity domain
> +	 * in legacy mode.
>  	 */
> -	if (sm_supported(iommu))
> -		set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
> +	set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);

That should probablyturn into an IDA someday, it would likely be more
memory efficient than bitmap_zalloc()

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

Jason
Re: [PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup
Posted by Baolu Lu 1 year, 4 months ago
On 2024/8/7 1:06, Jason Gunthorpe wrote:
> On Tue, Aug 06, 2024 at 10:39:37AM +0800, Lu Baolu wrote:
>> We will use a global static identity domain. Reserve a static domain ID
>> for it.
>>
>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/intel/iommu.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>> index 723ea9f3f501..c019fb3b3e78 100644
>> --- a/drivers/iommu/intel/iommu.c
>> +++ b/drivers/iommu/intel/iommu.c
>> @@ -1440,10 +1440,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
>>   	 * entry for first-level or pass-through translation modes should
>>   	 * be programmed with a domain id different from those used for
>>   	 * second-level or nested translation. We reserve a domain id for
>> -	 * this purpose.
>> +	 * this purpose. This domain id is also used for identity domain
>> +	 * in legacy mode.
>>   	 */
>> -	if (sm_supported(iommu))
>> -		set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
>> +	set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
> That should probablyturn into an IDA someday, it would likely be more
> memory efficient than bitmap_zalloc()

I have tried to. But I failed to find a suitable ida interface to
calculate the count of allocated domain IDs to replace bitmap_weight()
in below code.

static ssize_t domains_used_show(struct device *dev,
                                  struct device_attribute *attr, char *buf)
{
         struct intel_iommu *iommu = dev_to_intel_iommu(dev);
         return sysfs_emit(buf, "%d\n",
                           bitmap_weight(iommu->domain_ids,
                                         cap_ndoms(iommu->cap)));
}
static DEVICE_ATTR_RO(domains_used);

Thanks,
baolu
Re: [PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup
Posted by Jason Gunthorpe 1 year, 4 months ago
On Wed, Aug 07, 2024 at 02:19:57PM +0800, Baolu Lu wrote:
> On 2024/8/7 1:06, Jason Gunthorpe wrote:
> > On Tue, Aug 06, 2024 at 10:39:37AM +0800, Lu Baolu wrote:
> > > We will use a global static identity domain. Reserve a static domain ID
> > > for it.
> > > 
> > > Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
> > > ---
> > >   drivers/iommu/intel/iommu.c | 6 +++---
> > >   1 file changed, 3 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > > index 723ea9f3f501..c019fb3b3e78 100644
> > > --- a/drivers/iommu/intel/iommu.c
> > > +++ b/drivers/iommu/intel/iommu.c
> > > @@ -1440,10 +1440,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
> > >   	 * entry for first-level or pass-through translation modes should
> > >   	 * be programmed with a domain id different from those used for
> > >   	 * second-level or nested translation. We reserve a domain id for
> > > -	 * this purpose.
> > > +	 * this purpose. This domain id is also used for identity domain
> > > +	 * in legacy mode.
> > >   	 */
> > > -	if (sm_supported(iommu))
> > > -		set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
> > > +	set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
> > That should probablyturn into an IDA someday, it would likely be more
> > memory efficient than bitmap_zalloc()
> 
> I have tried to. But I failed to find a suitable ida interface to
> calculate the count of allocated domain IDs to replace bitmap_weight()
> in below code.

For something debugging like that just use 
  idr_for_each_entry()
     count++;

It is a weird thing to put in sysfs in the first place...

Jason
Re: [PATCH v3 3/7] iommu/vt-d: Always reserve a domain ID for identity setup
Posted by Baolu Lu 1 year, 4 months ago
On 2024/8/7 20:09, Jason Gunthorpe wrote:
> On Wed, Aug 07, 2024 at 02:19:57PM +0800, Baolu Lu wrote:
>> On 2024/8/7 1:06, Jason Gunthorpe wrote:
>>> On Tue, Aug 06, 2024 at 10:39:37AM +0800, Lu Baolu wrote:
>>>> We will use a global static identity domain. Reserve a static domain ID
>>>> for it.
>>>>
>>>> Signed-off-by: Lu Baolu<baolu.lu@linux.intel.com>
>>>> ---
>>>>    drivers/iommu/intel/iommu.c | 6 +++---
>>>>    1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>>> index 723ea9f3f501..c019fb3b3e78 100644
>>>> --- a/drivers/iommu/intel/iommu.c
>>>> +++ b/drivers/iommu/intel/iommu.c
>>>> @@ -1440,10 +1440,10 @@ static int iommu_init_domains(struct intel_iommu *iommu)
>>>>    	 * entry for first-level or pass-through translation modes should
>>>>    	 * be programmed with a domain id different from those used for
>>>>    	 * second-level or nested translation. We reserve a domain id for
>>>> -	 * this purpose.
>>>> +	 * this purpose. This domain id is also used for identity domain
>>>> +	 * in legacy mode.
>>>>    	 */
>>>> -	if (sm_supported(iommu))
>>>> -		set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
>>>> +	set_bit(FLPT_DEFAULT_DID, iommu->domain_ids);
>>> That should probablyturn into an IDA someday, it would likely be more
>>> memory efficient than bitmap_zalloc()
>> I have tried to. But I failed to find a suitable ida interface to
>> calculate the count of allocated domain IDs to replace bitmap_weight()
>> in below code.
> For something debugging like that just use
>    idr_for_each_entry()
>       count++;

Yeah! It works.

> It is a weird thing to put in sysfs in the first place...

Agreed. But it has been there for years. :-)

Thanks,
baolu