[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/9] hw/arm/smmu: Simplify the IOTLB key format
From: |
Auger Eric |
Subject: |
Re: [PATCH v2 4/9] hw/arm/smmu: Simplify the IOTLB key format |
Date: |
Mon, 6 Jul 2020 16:00:33 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
Hi Peter,
On 7/6/20 3:26 PM, Peter Maydell wrote:
> On Thu, 2 Jul 2020 at 16:27, 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/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
>> index 7dc8541e8b..5e85e30bdf 100644
>> --- a/hw/arm/smmu-common.c
>> +++ b/hw/arm/smmu-common.c
>> @@ -34,34 +34,17 @@
>>
>> static guint smmu_iotlb_key_hash(gconstpointer v)
>> {
>> - SMMUIOTLBKey *key = (SMMUIOTLBKey *)v;
>> - uint32_t a, b, c;
>> -
>> - /* Jenkins hash */
>> - a = b = c = JHASH_INITVAL + sizeof(*key);
>> - a += key->asid;
>> - b += extract64(key->iova, 0, 32);
>> - c += extract64(key->iova, 32, 32);
>> -
>> - __jhash_mix(a, b, c);
>> - __jhash_final(a, b, c);
>> -
>> - return c;
>> + return (guint)*(const uint64_t *)v;
>> }
>
> So the hash value is now going to be the lower 32
> bits of the key, which is to say bits [40,12] of the IOVA,
> and won't include the ASID at all. Isn't that going to
> result in more hash collisions than would be ideal?
>
> I was going to suggest using the glib builtin g_int64_hash()
> instead, but looking at the source that seems to be the
> identical implementation to this one. I guess that's
> intended for cases where an integer key is really a
> random integer, not one where it's got internal structure
> of different bit fields within it being for different
> purposes.
That's something I did not notice. Would you recommend to keep the
Jenkins hash function then? Note the intel iommu also use the hash
function which only covers the gfn.
Thanks
Eric
>
> thanks
> -- PMM
>
- [PATCH v2 0/9] SMMUv3.2 Range-based TLB Invalidation Support, Eric Auger, 2020/07/02
- [PATCH v2 2/9] hw/arm/smmu-common: Add IOTLB helpers, Eric Auger, 2020/07/02
- [PATCH v2 1/9] hw/arm/smmu-common: Factorize some code in smmu_ptw_64(), Eric Auger, 2020/07/02
- [PATCH v2 3/9] hw/arm/smmu: Introduce smmu_get_iotlb_key(), Eric Auger, 2020/07/02
- [PATCH v2 4/9] hw/arm/smmu: Simplify the IOTLB key format, Eric Auger, 2020/07/02
- [PATCH v2 5/9] hw/arm/smmu: Introduce SMMUTLBEntry for PTW and IOTLB value, Eric Auger, 2020/07/02
- [PATCH v2 6/9] hw/arm/smmu-common: Manage IOTLB block entries, Eric Auger, 2020/07/02
- [PATCH v2 7/9] hw/arm/smmuv3: Introduce smmuv3_s1_range_inval() helper, Eric Auger, 2020/07/02
- [PATCH v2 8/9] hw/arm/smmuv3: Get prepared for range invalidation, Eric Auger, 2020/07/02
- [PATCH v2 9/9] hw/arm/smmuv3: Advertise SMMUv3.2 range invalidation, Eric Auger, 2020/07/02