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

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

2020-06-11 Thread Eric Auger 
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 
---
 include/hw/arm/smmu-common.h | 1 +
 hw/arm/smmu-common.c | 2 +-
 hw/arm/smmuv3.c  | 8 ++--
 hw/arm/trace-events  | 2 +-
 4 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index f6ee78e16c..676e53c086 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -50,6 +50,7 @@ typedef struct SMMUTransTableInfo {
 uint64_t ttb;  /* TT base address */
 uint8_t tsz;   /* input range, ie. 2^(64 -tsz)*/
 uint8_t granule_sz;/* granule page shift */
+uint8_t starting_level;/* starting level */
 } SMMUTransTableInfo;
 
 struct SMMUTLBEntry {
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index aa88b62efb..c2ed8346fb 100644
--- 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;
 indexmask = (1ULL << (inputsize - (stride * (4 - level - 1;
 baseaddr = extract64(tt->ttb, 0, 48);
 baseaddr &= ~indexmask;
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index db74d27add..12d3e972d6 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -482,7 +482,7 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, 
SMMUEventInfo *event)
 
 /* decode data dependent on TT */
 for (i = 0; i <= 1; i++) {
-int tg, tsz;
+int tg, tsz, input_size, stride;
 SMMUTransTableInfo *tt = >tt[i];
 
 cfg->tt[i].disabled = CD_EPD(cd, i);
@@ -502,11 +502,15 @@ static int decode_cd(SMMUTransCfg *cfg, CD *cd, 
SMMUEventInfo *event)
 }
 
 tt->tsz = tsz;
+input_size = 64 - tt->tsz;
+stride = tt->granule_sz - 3;
 tt->ttb = CD_TTB(cd, i);
 if (tt->ttb & ~(MAKE_64BIT_MASK(0, cfg->oas))) {
 goto bad_cd;
 }
-trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb, tt->granule_sz);
+tt->starting_level = 4 - (input_size - 4) / stride;
+trace_smmuv3_decode_cd_tt(i, tt->tsz, tt->ttb,
+  tt->granule_sz, tt->starting_level);
 }
 
 event->record_trans_faults = CD_R(cd);
diff --git a/hw/arm/trace-events b/hw/arm/trace-events
index b808a1bfc1..7263b9c586 100644
--- a/hw/arm/trace-events
+++ b/hw/arm/trace-events
@@ -39,7 +39,7 @@ smmuv3_translate_abort(const char *n, uint16_t sid, uint64_t 
addr, bool is_write
 smmuv3_translate_success(const char *n, uint16_t sid, uint64_t iova, uint64_t 
translated, int perm) "%s sid=%d iova=0x%"PRIx64" translated=0x%"PRIx64" 
perm=0x%x"
 smmuv3_get_cd(uint64_t addr) "CD addr: 0x%"PRIx64
 smmuv3_decode_cd(uint32_t oas) "oas=%d"
-smmuv3_decode_cd_tt(int i, uint32_t tsz, uint64_t ttb, uint32_t granule_sz) 
"TT[%d]:tsz:%d ttb:0x%"PRIx64" granule_sz:%d"
+smmuv3_decode_cd_tt(int i, uint32_t tsz, uint64_t ttb, uint32_t granule_sz, 
uint8_t starting_level) "TT[%d]:tsz:%d ttb:0x%"PRIx64" granule_sz:%d 
starting_level=%d"
 smmuv3_cmdq_cfgi_ste(int streamid) "streamid =%d"
 smmuv3_cmdq_cfgi_ste_range(int start, int end) "start=0x%d - end=0x%d"
 smmuv3_cmdq_cfgi_cd(uint32_t sid) "streamid = %d"
-- 
2.20.1