On Mon, Jan 17, 2022 at 09:27:03AM +0800, Peter Xu wrote:
> 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.
Please ignore this - I think what you said should work indeed, and hash
collision should be fine with the equal function. Dangling vtd_as_key* may not
really work.
--
Peter Xu
