Subject: Re: [PATCH v2 06/18] powerpc/powernv: Use it_page_shift in TCE build

From: Alexey Kardashevskiy <[email-masked]>
Date: Wed, 23 Jul 2014 14:04:16 +1000

On 07/23/2014 02:00 PM, Gavin Shan wrote:
> On Wed, Jul 23, 2014 at 01:05:52PM +1000, Alexey Kardashevskiy wrote:
>> This makes use of iommu_table::it_page_shift instead of TCE_SHIFT and
>> TCE_RPN_SHIFT hardcoded values.
>>
>> Signed-off-by: Alexey Kardashevskiy <[email-masked]>
> 
> Reviewed-by: Gavin Shan <[email-masked]>
> 
>> ---
>> arch/powerpc/platforms/powernv/pci.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
>> index f91a4e5..b6cb996 100644
>> --- a/arch/powerpc/platforms/powernv/pci.c
>> +++ b/arch/powerpc/platforms/powernv/pci.c
>> @@ -564,10 +564,11 @@ static int pnv_tce_build(struct iommu_table *tbl, long index, long npages,
>> 		proto_tce |= TCE_PCI_WRITE;
>>
>> 	tces = tcep = ((__be64 *)tbl->it_base) + index - tbl->it_offset;
>> -	rpn = __pa(uaddr) >> TCE_SHIFT;
>> +	rpn = __pa(uaddr) >> tbl->it_page_shift;
>>
> 
> I'm not sure for 100%. It might be worthy to have some check somewhere:
> 
> 	WARN_ON(uaddr & ((1ull << tbl->it_page_shift) - 1))

The calling code (KVM and SPAPR TCE VFIO driver) performs all these checks,
no need to repeat it here.

> The "uaddr" are required to be "0x1ull << tbl->it_page_shift" aligned :-)
> 
> Thanks,
> Gavin
> 
>> 	while (npages--)
>> -		*(tcep++) = cpu_to_be64(proto_tce | (rpn++ << TCE_RPN_SHIFT));
>> +		*(tcep++) = cpu_to_be64(proto_tce |
>> +				(rpn++ << tbl->it_page_shift));
>>
>> 	/* Some implementations won't cache invalid TCEs and thus may not
>> 	 * need that flush. We'll probably turn it_type into a bit mask
>> -- 
>> 2.0.0
>>
> 

-- 
Alexey