Re: [PATCH 5/5] pci-assign: Use PCI-2.3-based shared legacy interrupts

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

On Tue, 2010-12-14 at 00:25 +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Enable the new KVM feature that allows legacy interrupt sharing for
> PCI-2.3-compliant devices. This requires to synchronize any guest
> change of the INTx mask bit to the kernel.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/device-assignment.c |   38 +++++++++++++++++++++++++++++++++-----
>  qemu-kvm.c             |    8 ++++++++
>  qemu-kvm.h             |    3 +++
>  3 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 26d3bd7..cf75c52 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -423,12 +423,21 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start)
>      return 0;
>  }
>  
> +static uint32_t calc_assigned_dev_id(uint16_t seg, uint8_t bus, uint8_t devfn)
> +{
> +    return (uint32_t)seg << 16 | (uint32_t)bus << 8 | (uint32_t)devfn;
> +}
> +
>  static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address,
>                                            uint32_t val, int len)
>  {
>      int fd;
>      ssize_t ret;
>      AssignedDevice *pci_dev = container_of(d, AssignedDevice, dev);
> +    struct kvm_assigned_pci_dev assigned_dev_data;
> +#ifdef KVM_CAP_PCI_2_3
> +    bool intx_masked, update_intx_mask;
> +#endif /* KVM_CAP_PCI_2_3 */
>  
>      DEBUG("(%x.%x): address=%04x val=0x%08x len=%d\n",
>            ((d->devfn >> 3) & 0x1F), (d->devfn & 0x7),
> @@ -439,6 +448,26 @@ static void assigned_dev_pci_write_config(PCIDevice *d, uint32_t address,
>      }
>  
>      if (ranges_overlap(address, len, PCI_COMMAND, 2)) {
> +#ifdef KVM_CAP_PCI_2_3
> +        update_intx_mask = false;
> +        if (address == PCI_COMMAND+1) {
> +            intx_masked = val & (PCI_COMMAND_INTX_DISABLE >> 8);
> +            update_intx_mask = true;
> +        } else if (len >= 2) {
> +            intx_masked = val & PCI_COMMAND_INTX_DISABLE;
> +            update_intx_mask = true;
> +        }

I wonder if this might be a little cleaner as something like this.

    if (ranges_overlap(address, len, PCI_COMMAND + 1, 1) {
        update_intx_mask = true;
        intx_masked = (len == 1 ? val << 8 : val) & PCI_COMMAND_INTX_DISABLE;
    }

> +        if (update_intx_mask) {
> +            memset(&assigned_dev_data, 0, sizeof(assigned_dev_data));
> +            assigned_dev_data.assigned_dev_id  =
> +                calc_assigned_dev_id(pci_dev->h_segnr, pci_dev->h_busnr,
> +                                     pci_dev->h_devfn);
> +            if (intx_masked) {
> +                assigned_dev_data.flags = KVM_DEV_ASSIGN_MASK_INTX;
> +            }
> +            kvm_assign_set_intx_mask(kvm_context, &assigned_dev_data);
> +        }
> +#endif /* KVM_CAP_PCI_2_3 */
>          pci_default_write_config(d, address, val, len);
>          /* Continue to program the card */
>      }
> @@ -876,11 +905,6 @@ static void free_assigned_device(AssignedDevice *dev)
>      }
>  }
>  
> -static uint32_t calc_assigned_dev_id(uint16_t seg, uint8_t bus, uint8_t devfn)
> -{
> -    return (uint32_t)seg << 16 | (uint32_t)bus << 8 | (uint32_t)devfn;
> -}
> -
>  static void assign_failed_examine(AssignedDevice *dev)
>  {
>      char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns;
> @@ -971,6 +995,10 @@ static int assign_device(AssignedDevice *dev)
>                  "cause host memory corruption if the device issues DMA write "
>                  "requests!\n");
>      }
> +#ifdef KVM_CAP_PCI_2_3
> +    assigned_dev_data.flags |= KVM_DEV_ASSIGN_PCI_2_3;
> +    dev->emulate_cmd_mask |= PCI_COMMAND_INTX_DISABLE;
> +#endif /* KVM_CAP_PCI_2_3 */
>  
>      r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
>      if (r < 0) {
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 471306b..8157b4f 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -740,6 +740,14 @@ int kvm_deassign_pci_device(kvm_context_t kvm,
>  }
>  #endif
>  
> +#ifdef KVM_CAP_PCI_2_3
> +int kvm_assign_set_intx_mask(kvm_context_t kvm,
> +                             struct kvm_assigned_pci_dev *assigned_dev)
> +{
> +    return kvm_vm_ioctl(kvm_state, KVM_ASSIGN_SET_INTX_MASK, assigned_dev);
> +}
> +#endif
> +
>  int kvm_reinject_control(kvm_context_t kvm, int pit_reinject)
>  {
>  #ifdef KVM_CAP_REINJECT_CONTROL
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 7e6edfb..522b1b2 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -602,6 +602,9 @@ int kvm_assign_set_msix_entry(kvm_context_t kvm,
>                                struct kvm_assigned_msix_entry *entry);
>  #endif
>  
> +int kvm_assign_set_intx_mask(kvm_context_t kvm,
> +                             struct kvm_assigned_pci_dev *assigned_dev);
> +
>  #else                           /* !CONFIG_KVM */
>  
>  typedef struct kvm_context *kvm_context_t;