Re: [PATCH] powerpc/iommu: limit number of TCEs to 512 for H_STUFF_TCE hcall

[Michael Ellerman <mpe@ellerman.id.au>]

Gaurav Batra <gbatra@linux.vnet.ibm.com> writes:
> As of now, in tce_freemulti_pSeriesLP(), there is no limit on how many TCEs
> are passed to H_STUFF_TCE hcall. PAPR is enforcing this to be limited to
> 512 TCEs.

Did you actually hit a bug here, or just noticed via code inspection?

Can you provide a Fixes: tag ?

cheers

> Signed-off-by: Gaurav Batra <gbatra@linux.vnet.ibm.com>
> Reviewed-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/iommu.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c
> index c74b71d4733d..1b134b1b795a 100644
> --- a/arch/powerpc/platforms/pseries/iommu.c
> +++ b/arch/powerpc/platforms/pseries/iommu.c
> @@ -306,13 +306,21 @@ static void tce_free_pSeriesLP(unsigned long liobn, long tcenum, long tceshift,
>  static void tce_freemulti_pSeriesLP(struct iommu_table *tbl, long tcenum, long npages)
>  {
>  	u64 rc;
> +	long limit, rpages = npages;
  
I don't know why npages is signed, but we don't ever want limit to be
negative, so it'd be better of as unsigned long wouldn't it?

>  	if (!firmware_has_feature(FW_FEATURE_STUFF_TCE))
>  		return tce_free_pSeriesLP(tbl->it_index, tcenum,
>  					  tbl->it_page_shift, npages);
>  
> -	rc = plpar_tce_stuff((u64)tbl->it_index,
> -			     (u64)tcenum << tbl->it_page_shift, 0, npages);
> +	do {
> +		limit = min_t(long, rpages, 512);

And here'd we'd use unsigned long.

> +		rc = plpar_tce_stuff((u64)tbl->it_index,
> +			     	(u64)tcenum << tbl->it_page_shift, 0, limit);
> +
> +		rpages -= limit;
> +		tcenum += limit;
> +	} while (rpages > 0 && !rc);
>  
>  	if (rc && printk_ratelimit()) {
>  		printk("tce_freemulti_pSeriesLP: plpar_tce_stuff failed\n");
> -- 

cheers