Re: [Qemu-devel] [PATCH RFC] vfio-pci: put device in INTx disable state in pre_reset

[Alex Williamson <alex.williamson@redhat.com>]

On Mon, 10 Oct 2016 17:12:43 +0800
Cao jin <caoj.fnst@cn.fujitsu.com> 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 <caoj.fnst@cn.fujitsu.com>
> ---
> 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);
>  }
>