Re: [PATCH 2/3] intel-iommu: drop VTDBus

[Peter Xu]

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