Re: [PATCH RESEND 5/9] hw/arm/smmuv3: Store the starting level in SMMUTransTableInfo

2020-06-26 Thread Auger Eric
Hi Peter,

On 6/25/20 5:15 PM, Peter Maydell wrote:
> On Thu, 11 Jun 2020 at 17:15, Eric Auger  wrote:
>>
>> Compute the starting level on CD decoding and store it
>> into SMMUTransTableInfo. We will need this information
>> on IOTLB lookup so let's avoid to recompute it each time.
>>
>> Signed-off-by: Eric Auger 
> 
>> --- a/hw/arm/smmu-common.c
>> +++ b/hw/arm/smmu-common.c
>> @@ -224,7 +224,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
>>  granule_sz = tt->granule_sz;
>>  stride = granule_sz - 3;
>>  inputsize = 64 - tt->tsz;
>> -level = 4 - (inputsize - 4) / stride;
>> +level = tt->starting_level;
> 
> "4 - (x - 4) / y" doesn't really seem like it's complicated
> enough to be worth caching given everything else we do on
> a page table walk. Do you have perf figures to indicate that
> this change is worthwhile?

no I don't have any figure or any setup at the moment to perform that
bench.

I thought it could only help, was not complicated to store and was of
the same kind as the granule size, the input range, ... already stored
in SMMUTransTableInfo.

Thanks

Eric
> 
> thanks
> -- PMM
> 




Re: [PATCH RESEND 5/9] hw/arm/smmuv3: Store the starting level in SMMUTransTableInfo

2020-06-25 Thread Peter Maydell
On Thu, 11 Jun 2020 at 17:15, Eric Auger  wrote:
>
> Compute the starting level on CD decoding and store it
> into SMMUTransTableInfo. We will need this information
> on IOTLB lookup so let's avoid to recompute it each time.
>
> Signed-off-by: Eric Auger 

> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -224,7 +224,7 @@ static int smmu_ptw_64(SMMUTransCfg *cfg,
>  granule_sz = tt->granule_sz;
>  stride = granule_sz - 3;
>  inputsize = 64 - tt->tsz;
> -level = 4 - (inputsize - 4) / stride;
> +level = tt->starting_level;

"4 - (x - 4) / y" doesn't really seem like it's complicated
enough to be worth caching given everything else we do on
a page table walk. Do you have perf figures to indicate that
this change is worthwhile?

thanks
-- PMM