Re: [PATCH v3 06/16] iommu/amd: Introduce helper functions for managing GCR3 table

[Vasant Hegde <vasant.hegde@amd.com>]

Hi Jason,


On 8/4/2023 6:47 PM, Jason Gunthorpe wrote:
> On Fri, Aug 04, 2023 at 06:42:06AM +0000, Vasant Hegde wrote:
>> From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>>
>> Refactor domain_enable_v2() into helper functions for managing GCR3 table
>> (i.e. setup_gcr3_table() and get_gcr3_levels()), which will be used in
>> subsequent patches. Also re-arrange code and remove forward declaration.
>>
>> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
>> Co-developed-by: Vasant Hegde <vasant.hegde@amd.com>
>> Signed-off-by: Vasant Hegde <vasant.hegde@amd.com>
>> ---
>>  drivers/iommu/amd/iommu.c | 66 +++++++++++++++++++++++----------------
>>  1 file changed, 39 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
>> index d5569eec0fe9..135fa087ce47 100644
>> --- a/drivers/iommu/amd/iommu.c
>> +++ b/drivers/iommu/amd/iommu.c
>> @@ -77,7 +77,6 @@ struct iommu_cmd {
>>  struct kmem_cache *amd_iommu_irq_cache;
>>  
>>  static void detach_device(struct device *dev);
>> -static int domain_enable_v2(struct protection_domain *domain, int pasids);
>>  
>>  /****************************************************************************
>>   *
>> @@ -1575,6 +1574,43 @@ static void free_gcr3_table(struct protection_domain *domain)
>>  	free_page((unsigned long)domain->gcr3_tbl);
>>  }
>>  
>> +static int get_gcr3_levels(int pasids)
>> +{
>> +	int levels = 0;
>> +
>> +	if (pasids == -1)
>> +		return amd_iommu_max_glx_val;
>> +
>> +	/*
>> +	 * Number of GCR3 table levels required. Level must be 4-Kbyte
>> +	 * page and can contain upto 512 entries.
>> +	 */
>> +	for ( ; get_count_order(pasids) >= 9; pasids >>= 9)
>> +		levels += 1;
>> +
>> +	return levels;
> 
> You missed to use the DIV_ROUND_UP

Sorry. I wanted to add comment to few patches separately. Somehow it got missed.

In our case,
  00b=GCR3 table is single-level - PASID size 9 bits
  01b=GCR3 table is two-level - PASID size 18 bits
  10b=GCR3 table is three-level - - PASID size 20 bits.

Hence I didn't include DIV_ROUND_UP().

-Vasant