From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joby Poriyath Subject: Re: [PATCH v6] interrupts: allow guest to set/clear MSI-X mask bit Date: Fri, 16 Aug 2013 12:16:09 +0100 Message-ID: <20130816111609.GB3388@citrix.com> References: <20130815154729.GA1212@citrix.com> <520E133B02000078000EC87B@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VAI2E-00056Y-Qi for xen-devel@lists.xenproject.org; Fri, 16 Aug 2013 11:17:39 +0000 Content-Disposition: inline In-Reply-To: <520E133B02000078000EC87B@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: andrew.cooper3@citrix.com, malcolm.crossley@citrix.com, keir@xen.org, xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Fri, Aug 16, 2013 at 10:55:39AM +0100, Jan Beulich wrote: > >>> On 15.08.13 at 17:47, Joby Poriyath 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