On Fri, Jan 14, 2022 at 05:15:35PM +0800, Jason Wang wrote:
>
> 在 2022/1/14 上午10:32, Jason Wang 写道:
> > > > dressSpace *as)
> > > > /* GHashTable functions */
> > > > static gboolean vtd_uint64_equal(gconstpointer v1, gconstpointer v2)
> > > > {
> > > > - return *((const uint64_t *)v1) == *((const uint64_t *)v2);
> > > > + const struct vtd_as_key *key1 = v1;
> > > > + const struct vtd_as_key *key2 = v2;
> > > > +
> > > > + return (key1->bus == key2->bus) && (key1->devfn == key2->devfn);
> > > > }
> > > > static guint vtd_uint64_hash(gconstpointer v)
> > > > {
> > > > - return (guint)*(const uint64_t *)v;
> > > > + const struct vtd_as_key *key = v;
> > > > + guint value = (guint)(uintptr_t)key->bus;
> > > > +
> > > > + return (guint)(value << 8 | key->devfn);
> > > Note that value is a pointer to PCIBus*. Just want to check with
> > > you that it's
> > > intended to use this hash value (or maybe you wanted to use Source
> > > ID so it is
> > > bus number to use not the bus pointer)?
> >
> >
> > Right, SID should be used here.
>
>
> Sorry for taking too long for the context switching ...
>
> The hash and shift based the bus pointer is intended since we use bus
> pointer as part of the key. The reason is still, during vtd_find_add_as(),
> SID is not correct since guest might not finish the initialization of the
> device and PCI bridge.
Ah, right.
However here value is left-shifted 8 bits so I'm not sure how it could
guarantee no collision - logically any addresses that match lower 56 bits will
hit with the same devfn by accident.
I don't think it'll matter in reality, but... wondering whether it's easier to
simply use g_direct_hash() (the glibc provided hash function when hash==NULL is
passed over, IOW we simply pass hash_func=NULL for g_hash_table_new) so we'll
hash with struct vtd_as_key* instead; IMHO that'll suffice too.
Thanks,
--
Peter Xu
