Peter Maydell <peter.mayd...@linaro.org> writes:
> On Tue, 11 Apr 2023 at 15:14, Peter Xu <pet...@redhat.com> wrote:
>>
>> On Mon, Apr 10, 2023 at 11:32:08AM +0800, Jason Wang wrote:
>> > @@ -222,9 +222,9 @@ static guint vtd_iotlb_hash(gconstpointer v)
>> > {
>> > const struct vtd_iotlb_key *key = v;
>> >
>> > - return key->gfn | ((key->sid) << VTD_IOTLB_SID_SHIFT) |
>> > - (key->level) << VTD_IOTLB_LVL_SHIFT |
>> > - (key->pasid) << VTD_IOTLB_PASID_SHIFT;
>> > + return key->gfn | ((uint64_t)(key->sid) << VTD_IOTLB_SID_SHIFT) |
>> > + (uint64_t)(key->level - 1) << VTD_IOTLB_LVL_SHIFT |
>> > + (uint64_t)(key->pasid) << VTD_IOTLB_PASID_SHIFT;
>> > }
>
>> > /* The shift of source_id in the key of IOTLB hash table */
>> > -#define VTD_IOTLB_SID_SHIFT 20
>> > -#define VTD_IOTLB_LVL_SHIFT 28
>> > -#define VTD_IOTLB_PASID_SHIFT 30
>> > +#define VTD_IOTLB_SID_SHIFT 26
>> > +#define VTD_IOTLB_LVL_SHIFT 42
>> > +#define VTD_IOTLB_PASID_SHIFT 44
>>
>> This is for the hash function only, IIUC it means anything over
>> sizeof(guint) will be ignored and not contributing anything to the hash
>> value being generated due to the uint64->guint conversion.
>>
>> IOW, I think "level" and "pasid" will just be ignored.
>
> Whoops, hadn't noticed that guint type... (glib's
> g_int64_hash()'s approach to this is to XOR the top
> 32 bits with the bottom 32 bits to produce the 32-bit
> hash value.)
This is less of a hash and more just concatting a bunch of fields. BTW
if the glib built-in hash isn't suitable we also have the qemu_xxhash()
functions which claim a good distribution of values and we use in a
number of places throughout the code.
> Also, does anybody know what the requirements are on
> consistency between the hash_func and the key_equal_func
> for a GHashTable ? Is the hash_func supposed to return the
> same hash for every key that compares equal under key_equal_func ?
>
> thanks
> -- PMM
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
