From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48960) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZ9AD-0000oy-HR for qemu-devel@nongnu.org; Mon, 15 Aug 2016 00:06:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bZ9AA-0007wb-AS for qemu-devel@nongnu.org; Mon, 15 Aug 2016 00:06:13 -0400 Received: from [59.151.112.132] (port=9235 helo=heian.cn.fujitsu.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bZ9A9-0007wI-UW for qemu-devel@nongnu.org; Mon, 15 Aug 2016 00:06:10 -0400 References: <1470799131-26024-1-git-send-email-caoj.fnst@cn.fujitsu.com> From: Cao jin Message-ID: <57B141A1.2040706@cn.fujitsu.com> Date: Mon, 15 Aug 2016 12:14:25 +0800 MIME-Version: 1.0 In-Reply-To: <1470799131-26024-1-git-send-email-caoj.fnst@cn.fujitsu.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC] msix_init: input params *_offset isn't the real one List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: marcel@redhat.com, alex.williamson@redhat.com, mst@redhat.com Sorry for the noise. On 08/10/2016 11:18 AM, Cao jin wrote: > The parameter table_offset & pba_offset is kind of confusing, they shouldn't > include bir field. > > Signed-off-by: Cao jin > --- > > According to the passed arguments, I guess all the callers of msix_init() > has the same feeling with me, but I am not quite sure about this, so, RFC. > > hw/pci/msix.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/hw/pci/msix.c b/hw/pci/msix.c > index 0ec1cb1..3a16d83 100644 > --- a/hw/pci/msix.c > +++ b/hw/pci/msix.c > @@ -264,8 +264,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, > if ((table_bar_nr == pba_bar_nr && > ranges_overlap(table_offset, table_size, pba_offset, pba_size)) || > table_offset + table_size > memory_region_size(table_bar) || > - pba_offset + pba_size > memory_region_size(pba_bar) || > - (table_offset | pba_offset) & PCI_MSIX_FLAGS_BIRMASK) { > + pba_offset + pba_size > memory_region_size(pba_bar)) { > return -EINVAL; > } > > @@ -282,8 +281,8 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries, > dev->msix_entries_nr = nentries; > dev->msix_function_masked = true; > > - pci_set_long(config + PCI_MSIX_TABLE, table_offset | table_bar_nr); > - pci_set_long(config + PCI_MSIX_PBA, pba_offset | pba_bar_nr); > + pci_set_long(config + PCI_MSIX_TABLE, (table_offset << 3) | table_bar_nr); > + pci_set_long(config + PCI_MSIX_PBA, (pba_offset << 3) | pba_bar_nr); > > /* Make flags bit writable. */ > dev->wmask[cap + MSIX_CONTROL_OFFSET] |= MSIX_ENABLE_MASK | > -- Yours Sincerely, Cao jin