Re: [PATCH RESEND 3/9] hw/arm/smmu: Simplify the IOTLB key format

[Auger Eric]

Hi Peter,

On 6/25/20 5:03 PM, Peter Maydell wrote:
> On Thu, 11 Jun 2020 at 17:15, Eric Auger <eric.auger@redhat.com> wrote:
>>
>> Instead of using a Jenkins hash function to generate
>> the key let's just use a 64 bit unsigned integer that
>> contains the asid and the 40 upper bits of the iova.
>> A maximum of 52-bit IOVA is supported. This change in the
>> key format also prepares for the addition of new fields
>> in subsequent patches (granule and level).
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> 
>> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
>> index 1dceec5cb1..7b9d2f0eb7 100644
>> --- a/include/hw/arm/smmu-common.h
>> +++ b/include/hw/arm/smmu-common.h
>> @@ -88,11 +88,6 @@ typedef struct SMMUPciBus {
>>      SMMUDevice   *pbdev[]; /* Parent array is sparse, so dynamically alloc 
>> */
>>  } SMMUPciBus;
>>
>> -typedef struct SMMUIOTLBKey {
>> -    uint64_t iova;
>> -    uint16_t asid;
>> -} SMMUIOTLBKey;
> 
> I think we should keep the SMMUIOTLBKey type to abstract out what
> the key type is under the hood, so it would now be
>  typedef uint64_t SMMUIOTLBKey;

OK
> 
> (and then the code that works with SMMUIOTLBKeys should never
> directly look at it as a uint64_t. If you wanted you could
> put the abstraction layer into place with the existing
> SMMUIOTLBKey type and then change the type in a second patch.)

done this way

> 
>> +uint64_t smmu_get_iotlb_key(uint16_t asid, uint64_t iova);
> 
> This should return SMMUIOTLBKey rather than uint64_t,
> or pass in the pointer, like:
>    smmu_get_iotlb_key(SMMUIOTLBKey *key, uint16_t asid, uint64_t iova);
sure

Thanks

Eric
> 
> thanks
> -- PMM
>