Re: [PATCH v6] interrupts: allow guest to set/clear MSI-X mask bit

[Joby Poriyath <joby.poriyath@citrix.com>]

On Fri, Aug 16, 2013 at 10:55:39AM +0100, Jan Beulich wrote:
> >>> On 15.08.13 at 17:47, Joby Poriyath <joby.poriyath@citrix.com> wrote:
> > @@ -404,7 +436,7 @@ int msixtbl_pt_register(struct domain *d, struct pirq *pirq, uint64_t gtable)
> >  
> >      entry = new_entry;
> >      new_entry = NULL;
> > -    add_msixtbl_entry(d, pdev, gtable, entry);
> > +    add_msixtbl_entry(d, pdev, gtable, entry, pirq);
> >  
> >  found:
> >      atomic_inc(&entry->refcnt);
> 
> Just noticed this "found" label here, which made me go back and
> look at the whole function: Did you consider the case of there
> already being an entry, and hence add_msixtbl_entry() not
> getting called, and thus entry->pirq not getting set to what got
> passed in here? I'm assuming that this is only ever the case if
> for the entry found entry->pirq == pirq, but if I'm right with
> this, adding a respective ASSERT() here would seem desirable.

If there was an entry, that was there only because it was added using 
"add_msixtbl_entry" function, and hence entry->pirq would have been 
initialized with a valid pirq. And modification of msixtbl_list is 
protected with a spinlock on msixtbl_list_lock. So in any case (adding
for the first time, or finding an entry), "entry" would have been 
correctly initialized. So looks like it's safe.

Or, am I getting this wrong? 

Thanks,
Joby