Re: [PATCH RFC v2 15/16] vfio-user: pci reset

[Stefan Hajnoczi <stefanha@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 4444 bytes --]

On Mon, Aug 16, 2021 at 09:42:48AM -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.h |  1 +
>  hw/vfio/pci.c  | 29 ++++++++++++++++++++++++++---
>  hw/vfio/user.c | 12 ++++++++++++
>  3 files changed, 39 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/vfio/user.h b/hw/vfio/user.h
> index 32e8b70d28..5d4d0a43ba 100644
> --- a/hw/vfio/user.h
> +++ b/hw/vfio/user.h
> @@ -86,6 +86,7 @@ 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,
>                             uint64_t offset, uint32_t count, void *data);
> +void vfio_user_reset(VFIODevice *vbasedev);
>  void vfio_user_drain_reqs(VFIOProxy *proxy);
>  
>  #endif /* VFIO_USER_H */
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 29a874c066..4b933ed10f 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -2229,8 +2229,9 @@ static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
>  
>  static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
>  {
> +    VFIODevice *vbasedev = &vdev->vbasedev;
>      Error *err = NULL;
> -    int nr;
> +    int ret, nr;
>  
>      vfio_intx_enable(vdev, &err);
>      if (err) {
> @@ -2238,11 +2239,18 @@ static void vfio_pci_post_reset(VFIOPCIDevice *vdev)
>      }
>  
>      for (nr = 0; nr < PCI_NUM_REGIONS - 1; ++nr) {
> -        off_t addr = vdev->config_offset + PCI_BASE_ADDRESS_0 + (4 * nr);
> +        off_t addr = PCI_BASE_ADDRESS_0 + (4 * nr);
>          uint32_t val = 0;
>          uint32_t len = sizeof(val);
>  
> -        if (pwrite(vdev->vbasedev.fd, &val, len, addr) != len) {
> +        if (vbasedev->proxy != NULL) {
> +            ret = vfio_user_region_write(vbasedev, VFIO_PCI_CONFIG_REGION_INDEX,
> +                                         addr, len, &val);
> +        } else {
> +            ret = pwrite(vdev->vbasedev.fd, &val, len,
> +                         vdev->config_offset + addr);
> +        }
> +        if (ret != len) {
>              error_report("%s(%s) reset bar %d failed: %m", __func__,
>                           vdev->vbasedev.name, nr);

The %m format string assumes vfio_user_region_write() sets errno. I
don't think it does. We're relying on vfio_user_region_write() never
failing here, which is true at the moment but not nice.

>          }
> @@ -3684,6 +3692,20 @@ static void vfio_user_instance_finalize(Object *obj)
>      vfio_user_disconnect(vbasedev->proxy);
>  }
>  
> +static void vfio_user_pci_reset(DeviceState *dev)
> +{
> +    VFIOPCIDevice *vdev = VFIO_PCI_BASE(dev);
> +    VFIODevice *vbasedev = &vdev->vbasedev;
> +
> +    vfio_pci_pre_reset(vdev);
> +
> +    if (vbasedev->reset_works) {
> +        vfio_user_reset(vbasedev);
> +    }
> +
> +    vfio_pci_post_reset(vdev);
> +}
> +
>  static Property vfio_user_pci_dev_properties[] = {
>      DEFINE_PROP_STRING("socket", VFIOUserPCIDevice, sock_name),
>      DEFINE_PROP_BOOL("secure-dma", VFIOUserPCIDevice, secure_dma, false),
> @@ -3695,6 +3717,7 @@ static void vfio_user_pci_dev_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *pdc = PCI_DEVICE_CLASS(klass);
>  
> +    dc->reset = vfio_user_pci_reset;
>      device_class_set_props(dc, vfio_user_pci_dev_properties);
>      dc->desc = "VFIO over socket PCI device assignment";
>      pdc->realize = vfio_user_pci_realize;
> diff --git a/hw/vfio/user.c b/hw/vfio/user.c
> index fcc041959c..7de2125346 100644
> --- a/hw/vfio/user.c
> +++ b/hw/vfio/user.c
> @@ -1045,3 +1045,15 @@ int vfio_user_region_write(VFIODevice *vbasedev, uint32_t index,
>  
>      return count;
>  }
> +
> +void vfio_user_reset(VFIODevice *vbasedev)
> +{
> +    VFIOUserHdr msg;

Maybe add "= {}" to ensure it's zero-initialized?

> +
> +    vfio_user_request_msg(&msg, VFIO_USER_DEVICE_RESET, sizeof(msg), 0);
> +
> +    vfio_user_send_recv(vbasedev->proxy, &msg, NULL, 0, 0);
> +    if (msg.flags & VFIO_USER_ERROR) {
> +        error_printf("reset reply error %d\n", msg.error_reply);
> +    }
> +}
> -- 
> 2.25.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]