From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48515) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bu9Ex-0007QP-A4 for qemu-devel@nongnu.org; Tue, 11 Oct 2016 22:25:57 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bu9Et-0005FR-6N for qemu-devel@nongnu.org; Tue, 11 Oct 2016 22:25:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:58860) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bu9Et-0005FD-0b for qemu-devel@nongnu.org; Tue, 11 Oct 2016 22:25:51 -0400 Date: Tue, 11 Oct 2016 20:25:49 -0600 From: Alex Williamson Message-ID: <20161011202549.0af84928@t450s.home> In-Reply-To: <1476090763-25295-1-git-send-email-caoj.fnst@cn.fujitsu.com> References: <1476090763-25295-1-git-send-email-caoj.fnst@cn.fujitsu.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH RFC] vfio-pci: put device in INTx disable state in pre_reset List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin Cc: qemu-devel@nongnu.org On Mon, 10 Oct 2016 17:12:43 +0800 Cao jin wrote: > Current code cleared the PCI_COMMAND_INTX_DISABLE, which indicates > device/function could asserts its INTx# signal. > > PCI local spec says: > A value of 0 enables the assertion of its INTx# signal. > A value of 1 disables the assertion of its INTx# signal. The PCI spec also says that this bit's state is zero after reset and we're about to perform a reset, so we expect it to be zero after reset. I believe this is the reason a set it this way. If we want to set it, we should OR it in, not AND it. Are you actually seeing any issues with the current behavior or was this a code inspection discovery? Thanks, Alex > Signed-off-by: Cao jin > --- > I guess it is a mistake, clearing the bit to enable INTx violate > the intention of vfio_disable_interrupts above. > > hw/vfio/pci.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index a5a620a..cce3024 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -1898,8 +1898,8 @@ static void vfio_pci_pre_reset(VFIOPCIDevice *vdev) > * Also put INTx Disable in known state. > */ > cmd = vfio_pci_read_config(pdev, PCI_COMMAND, 2); > - cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER | > - PCI_COMMAND_INTX_DISABLE); > + cmd &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER) | > + PCI_COMMAND_INTX_DISABLE; > vfio_pci_write_config(pdev, PCI_COMMAND, cmd, 2); > } >