[v1] Add live migration support in the PVRDMA device

[Qemu-devel] [PATCH v1 1/2] hw/pvrdma: make DSR mapping idempotent in load_dsr()

Posted by Sukrit Bhatnagar 6 years, 5 months ago

Map to DSR only when there is no mapping done already i.e., when
dev->dsr_info.dsr is NULL. This allows the rest of mappings and
ring inits to be done by calling load_dsr() when DSR has already
been mapped to, somewhere else.

Move free_dsr() out of load_dsr() and call it before the latter
as and when needed. This aids the case where load_dsr() is called
having DSR mapping already done, but the rest of map and init
operations are pending, and prevents an unmap of the DSR.

Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Cc: Yuval Shaia <yuval.shaia@oracle.com>
Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
---
 hw/rdma/vmw/pvrdma_main.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index adcf79cd63..6c90db96f9 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -172,15 +172,15 @@ static int load_dsr(PVRDMADev *dev)
     DSRInfo *dsr_info;
     struct pvrdma_device_shared_region *dsr;
 
-    free_dsr(dev);
-
-    /* Map to DSR */
-    dev->dsr_info.dsr = rdma_pci_dma_map(pci_dev, dev->dsr_info.dma,
-                              sizeof(struct pvrdma_device_shared_region));
     if (!dev->dsr_info.dsr) {
-        rdma_error_report("Failed to map to DSR");
-        rc = -ENOMEM;
-        goto out;
+        /* Map to DSR */
+        dev->dsr_info.dsr = rdma_pci_dma_map(pci_dev, dev->dsr_info.dma,
+                                  sizeof(struct pvrdma_device_shared_region));
+        if (!dev->dsr_info.dsr) {
+            rdma_error_report("Failed to map to DSR");
+            rc = -ENOMEM;
+            goto out;
+        }
     }
 
     /* Shortcuts */
@@ -402,6 +402,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
     case PVRDMA_REG_DSRHIGH:
         trace_pvrdma_regs_write(addr, val, "DSRHIGH", "");
         dev->dsr_info.dma |= val << 32;
+        free_dsr(dev);
         load_dsr(dev);
         init_dsr_dev_caps(dev);
         break;
-- 
2.21.0

Re: [Qemu-devel] [PATCH v1 1/2] hw/pvrdma: make DSR mapping idempotent in load_dsr()

Posted by Yuval Shaia 6 years, 5 months ago

On Wed, Aug 28, 2019 at 07:53:27PM +0530, Sukrit Bhatnagar wrote:
> Map to DSR only when there is no mapping done already i.e., when
> dev->dsr_info.dsr is NULL. This allows the rest of mappings and
> ring inits to be done by calling load_dsr() when DSR has already
> been mapped to, somewhere else.
> 
> Move free_dsr() out of load_dsr() and call it before the latter
> as and when needed. This aids the case where load_dsr() is called
> having DSR mapping already done, but the rest of map and init
> operations are pending, and prevents an unmap of the DSR.
> 
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Yuval Shaia <yuval.shaia@oracle.com>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>  hw/rdma/vmw/pvrdma_main.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index adcf79cd63..6c90db96f9 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -172,15 +172,15 @@ static int load_dsr(PVRDMADev *dev)
>      DSRInfo *dsr_info;
>      struct pvrdma_device_shared_region *dsr;
>  
> -    free_dsr(dev);
> -
> -    /* Map to DSR */
> -    dev->dsr_info.dsr = rdma_pci_dma_map(pci_dev, dev->dsr_info.dma,
> -                              sizeof(struct pvrdma_device_shared_region));
>      if (!dev->dsr_info.dsr) {
> -        rdma_error_report("Failed to map to DSR");
> -        rc = -ENOMEM;
> -        goto out;
> +        /* Map to DSR */
> +        dev->dsr_info.dsr = rdma_pci_dma_map(pci_dev, dev->dsr_info.dma,
> +                                  sizeof(struct pvrdma_device_shared_region));
> +        if (!dev->dsr_info.dsr) {
> +            rdma_error_report("Failed to map to DSR");
> +            rc = -ENOMEM;
> +            goto out;
> +        }
>      }
>  
>      /* Shortcuts */
> @@ -402,6 +402,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
>      case PVRDMA_REG_DSRHIGH:
>          trace_pvrdma_regs_write(addr, val, "DSRHIGH", "");
>          dev->dsr_info.dma |= val << 32;
> +        free_dsr(dev);
>          load_dsr(dev);
>          init_dsr_dev_caps(dev);
>          break;

LGTM, thanks.

Reviewed-by: Yuval Shaia <yuval.shaia@oracle.com>

> -- 
> 2.21.0
> 
>

Re: [Qemu-devel] [PATCH v1 1/2] hw/pvrdma: make DSR mapping idempotent in load_dsr()

Posted by Marcel Apfelbaum 6 years, 5 months ago


On 8/28/19 5:23 PM, Sukrit Bhatnagar wrote:
> Map to DSR only when there is no mapping done already i.e., when
> dev->dsr_info.dsr is NULL. This allows the rest of mappings and
> ring inits to be done by calling load_dsr() when DSR has already
> been mapped to, somewhere else.
>
> Move free_dsr() out of load_dsr() and call it before the latter
> as and when needed. This aids the case where load_dsr() is called
> having DSR mapping already done, but the rest of map and init
> operations are pending, and prevents an unmap of the DSR.
>
> Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
> Cc: Yuval Shaia <yuval.shaia@oracle.com>
> Signed-off-by: Sukrit Bhatnagar <skrtbhtngr@gmail.com>
> ---
>   hw/rdma/vmw/pvrdma_main.c | 17 +++++++++--------
>   1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> index adcf79cd63..6c90db96f9 100644
> --- a/hw/rdma/vmw/pvrdma_main.c
> +++ b/hw/rdma/vmw/pvrdma_main.c
> @@ -172,15 +172,15 @@ static int load_dsr(PVRDMADev *dev)
>       DSRInfo *dsr_info;
>       struct pvrdma_device_shared_region *dsr;
>   
> -    free_dsr(dev);
> -
> -    /* Map to DSR */
> -    dev->dsr_info.dsr = rdma_pci_dma_map(pci_dev, dev->dsr_info.dma,
> -                              sizeof(struct pvrdma_device_shared_region));
>       if (!dev->dsr_info.dsr) {
> -        rdma_error_report("Failed to map to DSR");
> -        rc = -ENOMEM;
> -        goto out;
> +        /* Map to DSR */
> +        dev->dsr_info.dsr = rdma_pci_dma_map(pci_dev, dev->dsr_info.dma,
> +                                  sizeof(struct pvrdma_device_shared_region));
> +        if (!dev->dsr_info.dsr) {
> +            rdma_error_report("Failed to map to DSR");
> +            rc = -ENOMEM;
> +            goto out;
> +        }
>       }
>   
>       /* Shortcuts */
> @@ -402,6 +402,7 @@ static void pvrdma_regs_write(void *opaque, hwaddr addr, uint64_t val,
>       case PVRDMA_REG_DSRHIGH:
>           trace_pvrdma_regs_write(addr, val, "DSRHIGH", "");
>           dev->dsr_info.dma |= val << 32;
> +        free_dsr(dev);
>           load_dsr(dev);
>           init_dsr_dev_caps(dev);
>           break;

Reviewed-by: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>

Thanks,
Marcel