Re: [PATCH RFC v2 11/16] vfio-user: get and set IRQs

[Stefan Hajnoczi]

On Mon, Aug 16, 2021 at 09:42:44AM -0700, Elena Ufimtseva wrote:
> From: John Johnson <john.g.johnson@oracle.com>
> 
> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
> ---
>  hw/vfio/user-protocol.h |  25 ++++++++++
>  hw/vfio/user.h          |   2 +
>  hw/vfio/common.c        |  26 ++++++++--
>  hw/vfio/pci.c           |  31 ++++++++++--
>  hw/vfio/user.c          | 106 ++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 181 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/vfio/user-protocol.h b/hw/vfio/user-protocol.h
> index 56904cf872..5614efa0a4 100644
> --- a/hw/vfio/user-protocol.h
> +++ b/hw/vfio/user-protocol.h
> @@ -109,6 +109,31 @@ typedef struct {
>      uint64_t offset;
>  } VFIOUserRegionInfo;
>  
> +/*
> + * VFIO_USER_DEVICE_GET_IRQ_INFO
> + * imported from struct vfio_irq_info
> + */
> +typedef struct {
> +    VFIOUserHdr hdr;
> +    uint32_t argsz;
> +    uint32_t flags;
> +    uint32_t index;
> +    uint32_t count;
> +} VFIOUserIRQInfo;
> +
> +/*
> + * VFIO_USER_DEVICE_SET_IRQS
> + * imported from struct vfio_irq_set
> + */
> +typedef struct {
> +    VFIOUserHdr hdr;
> +    uint32_t argsz;
> +    uint32_t flags;
> +    uint32_t index;
> +    uint32_t start;
> +    uint32_t count;
> +} VFIOUserIRQSet;
> +
>  /*
>   * VFIO_USER_REGION_READ
>   * VFIO_USER_REGION_WRITE
> diff --git a/hw/vfio/user.h b/hw/vfio/user.h
> index 02f832a173..248ad80943 100644
> --- a/hw/vfio/user.h
> +++ b/hw/vfio/user.h
> @@ -74,6 +74,8 @@ int vfio_user_validate_version(VFIODevice *vbasedev, Error 
> **errp);
>  int vfio_user_get_info(VFIODevice *vbasedev);
>  int vfio_user_get_region_info(VFIODevice *vbasedev, int index,
>                                struct vfio_region_info *info, VFIOUserFDs 
> *fds);
> +int vfio_user_get_irq_info(VFIODevice *vbasedev, struct vfio_irq_info *info);
> +int vfio_user_set_irqs(VFIODevice *vbasedev, struct vfio_irq_set *irq);
>  int vfio_user_region_read(VFIODevice *vbasedev, uint32_t index, uint64_t 
> offset,
>                            uint32_t count, void *data);
>  int vfio_user_region_write(VFIODevice *vbasedev, uint32_t index,
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index a8b1ea9358..9fe3e05dc6 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -71,7 +71,11 @@ void vfio_disable_irqindex(VFIODevice *vbasedev, int index)
>          .count = 0,
>      };
>  
> -    ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +    if (vbasedev->proxy != NULL) {
> +        vfio_user_set_irqs(vbasedev, &irq_set);
> +    } else {
> +        ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +    }
>  }
>  
>  void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index)
> @@ -84,7 +88,11 @@ void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int 
> index)
>          .count = 1,
>      };
>  
> -    ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +    if (vbasedev->proxy != NULL) {
> +        vfio_user_set_irqs(vbasedev, &irq_set);
> +    } else {
> +        ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +    }
>  }
>  
>  void vfio_mask_single_irqindex(VFIODevice *vbasedev, int index)
> @@ -97,7 +105,11 @@ void vfio_mask_single_irqindex(VFIODevice *vbasedev, int 
> index)
>          .count = 1,
>      };
>  
> -    ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +    if (vbasedev->proxy != NULL) {
> +        vfio_user_set_irqs(vbasedev, &irq_set);
> +    } else {
> +        ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +    }
>  }
>  
>  static inline const char *action_to_str(int action)
> @@ -178,8 +190,12 @@ int vfio_set_irq_signaling(VFIODevice *vbasedev, int 
> index, int subindex,
>      pfd = (int32_t *)&irq_set->data;
>      *pfd = fd;
>  
> -    if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> -        ret = -errno;
> +    if (vbasedev->proxy != NULL) {
> +        ret = vfio_user_set_irqs(vbasedev, irq_set);
> +    } else {
> +        if (ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set)) {
> +            ret = -errno;
> +        }
>      }
>      g_free(irq_set);
>  
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index ea0df8be65..282de6a30b 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -403,7 +403,11 @@ static int vfio_enable_vectors(VFIOPCIDevice *vdev, bool 
> msix)
>          fds[i] = fd;
>      }
>  
> -    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +    if (vdev->vbasedev.proxy != NULL) {
> +        ret = vfio_user_set_irqs(&vdev->vbasedev, irq_set);
> +    } else {
> +        ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_SET_IRQS, irq_set);
> +    }
>  
>      g_free(irq_set);
>  
> @@ -2675,7 +2679,13 @@ static void vfio_populate_device(VFIOPCIDevice *vdev, 
> Error **errp)
>  
>      irq_info.index = VFIO_PCI_ERR_IRQ_INDEX;
>  
> -    ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> +    if (vbasedev->proxy != NULL) {
> +        ret = vfio_user_get_irq_info(vbasedev, &irq_info);
> +    } else {
> +        ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info);
> +    }
> +
> +
>      if (ret) {
>          /* This can fail for an old kernel or legacy PCI dev */
>          trace_vfio_populate_device_get_irq_info_failure(strerror(errno));
> @@ -2794,8 +2804,16 @@ static void vfio_register_req_notifier(VFIOPCIDevice 
> *vdev)
>          return;
>      }
>  
> -    if (ioctl(vdev->vbasedev.fd,
> -              VFIO_DEVICE_GET_IRQ_INFO, &irq_info) < 0 || irq_info.count < 
> 1) {
> +    if (vdev->vbasedev.proxy != NULL) {
> +        if (vfio_user_get_irq_info(&vdev->vbasedev, &irq_info) < 0) {
> +            return;
> +        }
> +    } else {
> +        if (ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_IRQ_INFO, &irq_info) < 
> 0) {
> +            return;
> +        }
> +    }
> +    if (irq_info.count < 1) {
>          return;
>      }
>  
> @@ -3557,6 +3575,11 @@ static void vfio_user_pci_realize(PCIDevice *pdev, 
> Error **errp)
>          }
>      }
>  
> +    vfio_register_err_notifier(vdev);
> +    vfio_register_req_notifier(vdev);
> +
> +    return;
> +
>  out_deregister:
>      pci_device_set_intx_routing_notifier(&vdev->pdev, NULL);
>      kvm_irqchip_remove_change_notifier(&vdev->irqchip_change_notifier);
> diff --git a/hw/vfio/user.c b/hw/vfio/user.c
> index 83235b2411..b68ca1279d 100644
> --- a/hw/vfio/user.c
> +++ b/hw/vfio/user.c
> @@ -768,6 +768,112 @@ int vfio_user_get_region_info(VFIODevice *vbasedev, int 
> index,
>      return 0;
>  }
>  
> +int vfio_user_get_irq_info(VFIODevice *vbasedev, struct vfio_irq_info *info)
> +{
> +    VFIOUserIRQInfo msg;
> +
> +    memset(&msg, 0, sizeof(msg));
> +    vfio_user_request_msg(&msg.hdr, VFIO_USER_DEVICE_GET_IRQ_INFO,
> +                          sizeof(msg), 0);
> +    msg.argsz = info->argsz;
> +    msg.index = info->index;
> +
> +    vfio_user_send_recv(vbasedev->proxy, &msg.hdr, NULL, 0, 0);
> +    if (msg.hdr.flags & VFIO_USER_ERROR) {
> +        return -msg.hdr.error_reply;
> +    }
> +
> +    memcpy(info, &msg.argsz, sizeof(*info));

Should this be info.count = msg.count instead? Not sure why argsz is
used here.

Also, I just noticed the lack of endianness conversion in this patch
series. The spec says values are little-endian but these patches mostly
use host-endian. Did I miss something?

signature.asc
Description: PGP signature