Re: [PATCH] pci: Add option to disable device level INTx masking

[Michael S. Tsirkin]

On Thu, Mar 07, 2024 at 11:46:42AM -0700, Alex Williamson wrote:
> The PCI 2.3 spec added definitions of the INTx disable and status bits,
> in the command and status registers respectively.  The command register
> bit, commonly known as DisINTx in lspci, controls whether the device
> can assert the INTx signal.
> 
> Operating systems will often write to this bit to test whether a device
> supports this style of legacy interrupt masking.  When using device
> assignment, such as with vfio-pci, the result of this test dictates
> whether the device can use a shared or exclusive interrupt (ie. generic
> INTx masking at the device via DisINTx or IRQ controller level INTx
> masking).
> 
> Add an experimental option to the base set of properties for PCI
> devices which allows the DisINTx bit to be excluded from wmask, making
> it read-only to the guest for testing purposes related to INTx masking.
> 

Could you clarify the use a bit more? It's unstable - do you
expect to experiment with it and then make it permanent down
the road?

> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/pci/pci.c         | 14 ++++++++++----
>  include/hw/pci/pci.h |  2 ++
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index 6496d027ca61..8c78326ad67f 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -85,6 +85,8 @@ static Property pci_props[] = {
>                      QEMU_PCIE_ERR_UNC_MASK_BITNR, true),
>      DEFINE_PROP_BIT("x-pcie-ari-nextfn-1", PCIDevice, cap_present,
>                      QEMU_PCIE_ARI_NEXTFN_1_BITNR, false),
> +    DEFINE_PROP_BIT("x-pci-disintx", PCIDevice, cap_present,
> +                    QEMU_PCI_DISINTX_BITNR, true),
>      DEFINE_PROP_END_OF_LIST()
>  };
>  
> @@ -861,13 +863,17 @@ static void pci_init_cmask(PCIDevice *dev)
>  static void pci_init_wmask(PCIDevice *dev)
>  {
>      int config_size = pci_config_size(dev);
> +    uint16_t cmd_wmask = PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
> +                         PCI_COMMAND_MASTER | PCI_COMMAND_SERR;
>  
>      dev->wmask[PCI_CACHE_LINE_SIZE] = 0xff;
>      dev->wmask[PCI_INTERRUPT_LINE] = 0xff;
> -    pci_set_word(dev->wmask + PCI_COMMAND,
> -                 PCI_COMMAND_IO | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER |
> -                 PCI_COMMAND_INTX_DISABLE);
> -    pci_word_test_and_set_mask(dev->wmask + PCI_COMMAND, PCI_COMMAND_SERR);
> +
> +    if (dev->cap_present & QEMU_PCI_DISINTX) {
> +        cmd_wmask |= PCI_COMMAND_INTX_DISABLE;
> +    }
> +
> +    pci_set_word(dev->wmask + PCI_COMMAND, cmd_wmask);
>  
>      memset(dev->wmask + PCI_CONFIG_HEADER_SIZE, 0xff,
>             config_size - PCI_CONFIG_HEADER_SIZE);
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index eaa3fc99d884..45f0fac435cc 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -212,6 +212,8 @@ enum {
>      QEMU_PCIE_ERR_UNC_MASK = (1 << QEMU_PCIE_ERR_UNC_MASK_BITNR),
>  #define QEMU_PCIE_ARI_NEXTFN_1_BITNR 12
>      QEMU_PCIE_ARI_NEXTFN_1 = (1 << QEMU_PCIE_ARI_NEXTFN_1_BITNR),
> +#define QEMU_PCI_DISINTX_BITNR 13
> +    QEMU_PCI_DISINTX = (1 << QEMU_PCI_DISINTX_BITNR),
>  };
>  
>  typedef struct PCIINTxRoute {
> -- 
> 2.44.0