Re: [Qemu-devel] [PATCH v1 2/2] hw/pvrdma: add live migration support

2019-09-03 Thread Sukrit Bhatnagar
On Thu, 29 Aug 2019 at 18:23, Yuval Shaia  wrote:
>
> On Wed, Aug 28, 2019 at 07:53:28PM +0530, Sukrit Bhatnagar wrote:
> > vmstate_pvrdma describes the PCI and MSIX states as well as the dma
> > address for dsr and the gid table of device.
> > vmstate_pvrdma_gids describes each gid in the gid table.
> >
> > pvrdma_post_save() does the job of unregistering gid entries from the
> > backend device in the source host.
> >
> > pvrdma_post_load() maps to dsr using the loaded dma address, registers
> > each loaded gid into the backend device, and finally calls load_dsr()
> > to perform other mappings and ring init operations.
>
> I think it worth to mention that the dma address is kept in driver/device
> shared memory (dsr->dma) which is migrated as part of memory migration and
> it is out of the scope of this change and so we do not need to save/load
> the dma address during migration.
>
> Also you should specifically comment that this migration-support does not
> includes QP migration. This means that support for life migration *during*
> traffic is not yet supported.
>
> >
> > Cc: Marcel Apfelbaum 
> > Cc: Yuval Shaia 
> > Signed-off-by: Sukrit Bhatnagar 
> > ---
> >  hw/rdma/vmw/pvrdma_main.c | 77 +++
> >  1 file changed, 77 insertions(+)
> >
> > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > index 6c90db96f9..6f8b56dea3 100644
> > --- a/hw/rdma/vmw/pvrdma_main.c
> > +++ b/hw/rdma/vmw/pvrdma_main.c
> > @@ -28,6 +28,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "monitor/monitor.h"
> >  #include "hw/rdma/rdma.h"
> > +#include "migration/register.h"
> >
> >  #include "../rdma_rm.h"
> >  #include "../rdma_backend.h"
> > @@ -593,6 +594,81 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
> > *opaque)
> >  pvrdma_fini(pci_dev);
> >  }
> >
> > +static int pvrdma_post_save(void *opaque)
> > +{
> > +int i, rc;
> > +PVRDMADev *dev = opaque;
> > +
> > +for (i = 0; i < MAX_GIDS; i++) {
> > +
>
> Empty line is redundant here.
>
> > +if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
> > +continue;
> > +}
> > +rc = rdma_backend_del_gid(>backend_dev,
> > +   dev->backend_eth_device_name,
> > +   >rdma_dev_res.port.gid_tbl[i].gid);
> > +if (rc) {
> > +return -EINVAL;
>
> Some error report will help here i guess.

rdma_backend_del_gid() already generates an error report
when rc isn't 0.

Adding another statement for the same seems redundant.

> > +}
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int pvrdma_post_load(void *opaque, int version_id)
> > +{
> > +int i, rc;
> > +PVRDMADev *dev = opaque;
> > +PCIDevice *pci_dev = PCI_DEVICE(dev);
> > +DSRInfo *dsr_info = >dsr_info;
> > +
> > +dsr_info->dsr = rdma_pci_dma_map(pci_dev, dsr_info->dma,
> > +sizeof(struct 
> > pvrdma_device_shared_region));
> > +if (!dsr_info->dsr) {
> > +rdma_error_report("Failed to map to DSR");
> > +return -ENOMEM;
> > +}
> > +
> > +for (i = 0; i < MAX_GIDS; i++) {
> > +
>
> Empty line is redundant here.
>
> > +if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
> > +continue;
> > +}
> > +
> > +rc = rdma_backend_add_gid(>backend_dev,
> > +  dev->backend_eth_device_name,
> > +  >rdma_dev_res.port.gid_tbl[i].gid);
> > +if (rc) {
> > +return -EINVAL;
> > +}
> > +}
> > +
> > +return load_dsr(dev);

Now that I will move load_dsr() before the del_gid loop,
I can use goto jumps on exit/error paths, so that I can
undo load_dsr if any del_gid fails.

> > +}
> > +
> > +static const VMStateDescription vmstate_pvrdma_gids = {
> > +.name = "pvrdma-gids",
> > +.fields = (VMStateField[]) {
> > +VMSTATE_UINT8_ARRAY_V(gid.raw, RdmaRmGid, 16, 0),
> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > +static const VMStateDescription vmstate_pvrdma = {
> > +.name = PVRDMA_HW_NAME,
> > +.post_sa

Re: [Qemu-devel] [PATCH v1 2/2] hw/pvrdma: add live migration support

2019-09-03 Thread Sukrit Bhatnagar
On Sun, 1 Sep 2019 at 01:15, Marcel Apfelbaum
 wrote:
>
>
>
> On 8/28/19 5:23 PM, Sukrit Bhatnagar wrote:
> > vmstate_pvrdma describes the PCI and MSIX states as well as the dma
> > address for dsr and the gid table of device.
> > vmstate_pvrdma_gids describes each gid in the gid table.
> >
> > pvrdma_post_save() does the job of unregistering gid entries from the
> > backend device in the source host.
> >
> > pvrdma_post_load() maps to dsr using the loaded dma address, registers
> > each loaded gid into the backend device, and finally calls load_dsr()
> > to perform other mappings and ring init operations.
> >
> > Cc: Marcel Apfelbaum 
> > Cc: Yuval Shaia 
> > Signed-off-by: Sukrit Bhatnagar 
> > ---
> >   hw/rdma/vmw/pvrdma_main.c | 77 +++
> >   1 file changed, 77 insertions(+)
> >
> > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > index 6c90db96f9..6f8b56dea3 100644
> > --- a/hw/rdma/vmw/pvrdma_main.c
> > +++ b/hw/rdma/vmw/pvrdma_main.c
> > @@ -28,6 +28,7 @@
> >   #include "sysemu/sysemu.h"
> >   #include "monitor/monitor.h"
> >   #include "hw/rdma/rdma.h"
> > +#include "migration/register.h"
> >
> >   #include "../rdma_rm.h"
> >   #include "../rdma_backend.h"
> > @@ -593,6 +594,81 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
> > *opaque)
> >   pvrdma_fini(pci_dev);
> >   }
> >
> > +static int pvrdma_post_save(void *opaque)
> > +{
> > +int i, rc;
> > +PVRDMADev *dev = opaque;
> > +
> > +for (i = 0; i < MAX_GIDS; i++) {
> > +
>
> No need for the extra line
> > +if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
> > +continue;
> > +}
> > +rc = rdma_backend_del_gid(>backend_dev,
> > +   dev->backend_eth_device_name,
> > +   >rdma_dev_res.port.gid_tbl[i].gid);
> > +if (rc) {
> > +return -EINVAL;
> > +}
> > +}
> > +
> > +return 0;
> > +}
> > +
> > +static int pvrdma_post_load(void *opaque, int version_id)
> > +{
> > +int i, rc;
> > +PVRDMADev *dev = opaque;
> > +PCIDevice *pci_dev = PCI_DEVICE(dev);
> > +DSRInfo *dsr_info = >dsr_info;
> > +
> > +dsr_info->dsr = rdma_pci_dma_map(pci_dev, dsr_info->dma,
> > +sizeof(struct 
> > pvrdma_device_shared_region));
> > +if (!dsr_info->dsr) {
> > +rdma_error_report("Failed to map to DSR");
> > +return -ENOMEM;
> > +}
> > +
> > +for (i = 0; i < MAX_GIDS; i++) {
> > +
>
> The same here
>
> > +if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
> > +continue;
> > +}
> > +
> > +rc = rdma_backend_add_gid(>backend_dev,
> > +  dev->backend_eth_device_name,
> > +  >rdma_dev_res.port.gid_tbl[i].gid);
> > +if (rc) {
> > +return -EINVAL;
> > +}
> > +}
> > +
> > +return load_dsr(dev);
> > +}
> > +
> > +static const VMStateDescription vmstate_pvrdma_gids = {
> > +.name = "pvrdma-gids",
> > +.fields = (VMStateField[]) {
> > +VMSTATE_UINT8_ARRAY_V(gid.raw, RdmaRmGid, 16, 0),
>
> Is 16 the array length? If yes, do we have same macro definition?

16 here represents the number of bytes in a GID.
This comes from the verbs definition of ibv_gid

union ibv_gid {
uint8_t raw[16];
struct {
__be64  subnet_prefix;
__be64  interface_id;
} global;
};

I suppose there is no macro for this but we can declare
our own (something like IBV_GID_SIZE).

> > +VMSTATE_END_OF_LIST()
> > +}
> > +};
> > +
> > +static const VMStateDescription vmstate_pvrdma = {
> > +.name = PVRDMA_HW_NAME,
> > +.post_save = pvrdma_post_save,
> > +.post_load = pvrdma_post_load,
> > +.fields = (VMStateField[]) {
> > +VMSTATE_PCI_DEVICE(parent_obj, PVRDMADev),
> > +VMSTATE_MSIX(parent_obj, PVRDMADev),
> > +VMSTATE_UINT64(dsr_info.dma, PVRDMADev),
> > +VMSTATE_STRUCT_ARRAY(rdma_dev_res.port.gid_tbl, PVRDMADev,
> > + MAX

[Qemu-devel] [PATCH v1 2/2] hw/pvrdma: add live migration support

2019-08-28 Thread Sukrit Bhatnagar
vmstate_pvrdma describes the PCI and MSIX states as well as the dma
address for dsr and the gid table of device.
vmstate_pvrdma_gids describes each gid in the gid table.

pvrdma_post_save() does the job of unregistering gid entries from the
backend device in the source host.

pvrdma_post_load() maps to dsr using the loaded dma address, registers
each loaded gid into the backend device, and finally calls load_dsr()
to perform other mappings and ring init operations.

Cc: Marcel Apfelbaum 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 hw/rdma/vmw/pvrdma_main.c | 77 +++
 1 file changed, 77 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 6c90db96f9..6f8b56dea3 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "hw/rdma/rdma.h"
+#include "migration/register.h"
 
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
@@ -593,6 +594,81 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
*opaque)
 pvrdma_fini(pci_dev);
 }
 
+static int pvrdma_post_save(void *opaque)
+{
+int i, rc;
+PVRDMADev *dev = opaque;
+
+for (i = 0; i < MAX_GIDS; i++) {
+
+if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
+continue;
+}
+rc = rdma_backend_del_gid(>backend_dev,
+   dev->backend_eth_device_name,
+   >rdma_dev_res.port.gid_tbl[i].gid);
+if (rc) {
+return -EINVAL;
+}
+}
+
+return 0;
+}
+
+static int pvrdma_post_load(void *opaque, int version_id)
+{
+int i, rc;
+PVRDMADev *dev = opaque;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+DSRInfo *dsr_info = >dsr_info;
+
+dsr_info->dsr = rdma_pci_dma_map(pci_dev, dsr_info->dma,
+sizeof(struct pvrdma_device_shared_region));
+if (!dsr_info->dsr) {
+rdma_error_report("Failed to map to DSR");
+return -ENOMEM;
+}
+
+for (i = 0; i < MAX_GIDS; i++) {
+
+if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
+continue;
+}
+
+rc = rdma_backend_add_gid(>backend_dev,
+  dev->backend_eth_device_name,
+  >rdma_dev_res.port.gid_tbl[i].gid);
+if (rc) {
+return -EINVAL;
+}
+}
+
+return load_dsr(dev);
+}
+
+static const VMStateDescription vmstate_pvrdma_gids = {
+.name = "pvrdma-gids",
+.fields = (VMStateField[]) {
+VMSTATE_UINT8_ARRAY_V(gid.raw, RdmaRmGid, 16, 0),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_pvrdma = {
+.name = PVRDMA_HW_NAME,
+.post_save = pvrdma_post_save,
+.post_load = pvrdma_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(parent_obj, PVRDMADev),
+VMSTATE_MSIX(parent_obj, PVRDMADev),
+VMSTATE_UINT64(dsr_info.dma, PVRDMADev),
+VMSTATE_STRUCT_ARRAY(rdma_dev_res.port.gid_tbl, PVRDMADev,
+ MAX_PORT_GIDS, 0, vmstate_pvrdma_gids,
+ RdmaRmGid),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static void pvrdma_realize(PCIDevice *pdev, Error **errp)
 {
 int rc = 0;
@@ -688,6 +764,7 @@ static void pvrdma_class_init(ObjectClass *klass, void 
*data)
 
 dc->desc = "RDMA Device";
 dc->props = pvrdma_dev_properties;
+dc->vmsd = _pvrdma;
 set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
 
 ir->print_statistics = pvrdma_print_statistics;
-- 
2.21.0




[Qemu-devel] [PATCH v1 0/2] Add live migration support in the PVRDMA device

2019-08-28 Thread Sukrit Bhatnagar
This series enables the migration of various GIDs used by the device.
This is in addition to the successful migration of PCI and MSIX states
as well as various DMA addresses and ring page information.

We have a setup having two hosts and two VMs running atop them.
Migrations are performed over the local network.

We also have performed various ping-pong tests (ibv_rc_pingpong) in the
guest(s) after adding GID migration support and this is the current status:
- ping-pong to localhost succeeds, when performed before starting the
  migration and after the completion of migration.
- ping-pong to a peer succeeds, both before and after migration as above,
  provided that both VMs are running on/migrated to the same host.
  So, if two VMs were started on two different hosts, and one of them
  was migrated to the other host, the ping-pong was successful.
  Similarly, if two VMs are migrated to the same host, then after migration,
  the ping-pong was successful.
- ping-pong to a peer on the remote host is not working as of now.

Our next goal is to achieve successful migration with live traffic.

This is the same as the RFC v3 series posted earlier:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04752.html
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04753.html
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg04754.html


Sukrit Bhatnagar (2):
  hw/pvrdma: make DSR mapping idempotent in load_dsr()
  hw/pvrdma: add live migration support

 hw/rdma/vmw/pvrdma_main.c | 94 +++
 1 file changed, 86 insertions(+), 8 deletions(-)

-- 
2.21.0




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

2019-08-28 Thread Sukrit Bhatnagar
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 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 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




[Qemu-devel] [RFC v3 0/2] Add live migration support in the PVRDMA device

2019-07-20 Thread Sukrit Bhatnagar
In v2, we had successful migration of PCI and MSIX states as well as
various DMA addresses and ring page information.
This series enables the migration of various GIDs used by the device.

We have switched to a setup having two hosts and two VMs running atop them.
Migrations are now performed over the local network. This has settled the
same-host issue with libvirt.

We also have performed various ping-pong tests (ibv_rc_pingpong) in the
guest(s) after adding GID migration support and this is the current status:
- ping-pong to localhost succeeds, when performed before starting the
  migration and after the completion of migration.
- ping-pong to a peer succeeds, both before and after migration as above,
  provided that both VMs are running on/migrated to the same host.
  So, if two VMs were started on two different hosts, and one of them
  was migrated to the other host, the ping-pong was successful.
  Similarly, if two VMs are migrated to the same host, then after migration,
  the ping-pong was successful.
- ping-pong to a peer on the remote host is not working as of now.

Our next goal is to achieve successful migration with live traffic.

This series can be also found at:
https://github.com/skrtbhtngr/qemu/tree/gsoc19


History:

v2 -> v3:
- remove struct PVRDMAMigTmp and VMSTATE_WITH_TMP
- use predefined PVRDMA_HW_NAME for the vmsd name
- add vmsd for gids and a gid table field in pvrdma_state
- perform gid registration in pvrdma_post_load
- define pvrdma_post_save to unregister gids in the source host

v1 -> v2:
- modify load_dsr() to make it idempotent
- switch to VMStateDescription
- add fields for PCI and MSIX state
- define a temporary struct PVRDMAMigTmp to use WITH_TMP macro
- perform mappings to CQ and event notification rings at load
- vmxnet3 issue solved by Marcel's patch
- BounceBuffer issue solved automatically by switching to VMStateDescription


Link(s) to v2:
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01848.html
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01849.html
https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01850.html

Link(s) to v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04924.html
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04923.html

Sukrit Bhatnagar (2):
  hw/pvrdma: make DSR mapping idempotent in load_dsr()
  hw/pvrdma: add live migration support

 hw/rdma/vmw/pvrdma_main.c | 94 +++
 1 file changed, 86 insertions(+), 8 deletions(-)

-- 
2.21.0




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

2019-07-20 Thread Sukrit Bhatnagar
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 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 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




[Qemu-devel] [RFC v3 2/2] hw/pvrdma: add live migration support

2019-07-20 Thread Sukrit Bhatnagar
vmstate_pvrdma describes the PCI and MSIX states as well as the dma
address for dsr and the gid table of device.
vmstate_pvrdma_gids describes each gid in the gid table.

pvrdma_post_save() does the job of unregistering gid entries from the
backend device in the source host.

pvrdma_post_load() maps to dsr using the loaded dma address, registers
each loaded gid into the backend device, and finally calls load_dsr()
to perform other mappings and ring init operations.

Cc: Marcel Apfelbaum 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 hw/rdma/vmw/pvrdma_main.c | 77 +++
 1 file changed, 77 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 6c90db96f9..6f8b56dea3 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "hw/rdma/rdma.h"
+#include "migration/register.h"
 
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
@@ -593,6 +594,81 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
*opaque)
 pvrdma_fini(pci_dev);
 }
 
+static int pvrdma_post_save(void *opaque)
+{
+int i, rc;
+PVRDMADev *dev = opaque;
+
+for (i = 0; i < MAX_GIDS; i++) {
+
+if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
+continue;
+}
+rc = rdma_backend_del_gid(>backend_dev,
+   dev->backend_eth_device_name,
+   >rdma_dev_res.port.gid_tbl[i].gid);
+if (rc) {
+return -EINVAL;
+}
+}
+
+return 0;
+}
+
+static int pvrdma_post_load(void *opaque, int version_id)
+{
+int i, rc;
+PVRDMADev *dev = opaque;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+DSRInfo *dsr_info = >dsr_info;
+
+dsr_info->dsr = rdma_pci_dma_map(pci_dev, dsr_info->dma,
+sizeof(struct pvrdma_device_shared_region));
+if (!dsr_info->dsr) {
+rdma_error_report("Failed to map to DSR");
+return -ENOMEM;
+}
+
+for (i = 0; i < MAX_GIDS; i++) {
+
+if (!dev->rdma_dev_res.port.gid_tbl[i].gid.global.interface_id) {
+continue;
+}
+
+rc = rdma_backend_add_gid(>backend_dev,
+  dev->backend_eth_device_name,
+  >rdma_dev_res.port.gid_tbl[i].gid);
+if (rc) {
+return -EINVAL;
+}
+}
+
+return load_dsr(dev);
+}
+
+static const VMStateDescription vmstate_pvrdma_gids = {
+.name = "pvrdma-gids",
+.fields = (VMStateField[]) {
+VMSTATE_UINT8_ARRAY_V(gid.raw, RdmaRmGid, 16, 0),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_pvrdma = {
+.name = PVRDMA_HW_NAME,
+.post_save = pvrdma_post_save,
+.post_load = pvrdma_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_PCI_DEVICE(parent_obj, PVRDMADev),
+VMSTATE_MSIX(parent_obj, PVRDMADev),
+VMSTATE_UINT64(dsr_info.dma, PVRDMADev),
+VMSTATE_STRUCT_ARRAY(rdma_dev_res.port.gid_tbl, PVRDMADev,
+ MAX_PORT_GIDS, 0, vmstate_pvrdma_gids,
+ RdmaRmGid),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static void pvrdma_realize(PCIDevice *pdev, Error **errp)
 {
 int rc = 0;
@@ -688,6 +764,7 @@ static void pvrdma_class_init(ObjectClass *klass, void 
*data)
 
 dc->desc = "RDMA Device";
 dc->props = pvrdma_dev_properties;
+dc->vmsd = _pvrdma;
 set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
 
 ir->print_statistics = pvrdma_print_statistics;
-- 
2.21.0




Re: [Qemu-devel] [RFC v2 2/2] hw/pvrdma: add live migration support

2019-07-10 Thread Sukrit Bhatnagar
On Mon, 8 Jul 2019 at 10:43, Yuval Shaia  wrote:
>
> On Sat, Jul 06, 2019 at 09:39:40AM +0530, Sukrit Bhatnagar wrote:
> > Use VMStateDescription for migrating device state. Currently,
>
> What do you mean by 'Currently'?

I meant that 'vmstate_pvrdma' will contain more fields later. This is
how it looks till now.

> > 'vmstate_pvrdma' describes the PCI and MSIX state for pvrdma and
> > 'vmstate_pvrdma_dsr_dma' describes a temporary state containing
> > some values obtained only after mapping to dsr in the source.
> > Since the dsr will not be available on dest until we map to the
> > dma address we had on source, these values cannot be migrated
> > directly.
> >
> > Add PVRDMAMigTmp to store this temporary state which consists of
> > dma addresses and ring page information. The 'parent' member is
> > used to refer to the device state (PVRDMADev) so that parent PCI
> > device object is accessible, which is needed to remap to DSR.
> >
> > pvrdma_dsr_dma_pre_save() saves the dsr state into this temporary
> > representation and pvrdma_dsr_dma_post_load() loads it back.
> > This load function also remaps to the dsr and and calls
> > load_dsr() for further map and ring init operations.
> >
> > Please note that this call to load_dsr() can be removed from the
> > migration flow and included in pvrdma_regs_write() to perform a
> > lazy load.
>
> The lazy load was suggested to overcome a potential problem with mapping to
> addresses while still in migration process. With that been solved i would
> suggest to drop the idea of lazy load.
>
> > As of now, migration will fail if there in an error in load_dsr().
> > Also, there might be a considerable amount of pages in the rings,
> > which will have dma map operations when the init functions are
> > called.
> > If this takes noticeable time, it might be better to have lazy
> > load instead.
>
> Yeah, make sense but i hope we will not get to this.
>
> >
> > Cc: Marcel Apfelbaum 
> > Cc: Yuval Shaia 
> > Signed-off-by: Sukrit Bhatnagar 
> > ---
> >  hw/rdma/vmw/pvrdma_main.c | 87 +++
> >  1 file changed, 87 insertions(+)
> >
> > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > index 6c90db96f9..4a10bd2fc7 100644
> > --- a/hw/rdma/vmw/pvrdma_main.c
> > +++ b/hw/rdma/vmw/pvrdma_main.c
> > @@ -28,6 +28,7 @@
> >  #include "sysemu/sysemu.h"
> >  #include "monitor/monitor.h"
> >  #include "hw/rdma/rdma.h"
> > +#include "migration/register.h"
> >
> >  #include "../rdma_rm.h"
> >  #include "../rdma_backend.h"
> > @@ -593,6 +594,91 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
> > *opaque)
> >  pvrdma_fini(pci_dev);
> >  }
> >
> > +struct PVRDMAMigTmp {
> > +PVRDMADev *parent;
> > +uint64_t dma;
> > +uint64_t cmd_slot_dma;
> > +uint64_t resp_slot_dma;
> > +uint32_t cq_ring_pages_num_pages;
> > +uint64_t cq_ring_pages_pdir_dma;
> > +uint32_t async_ring_pages_num_pages;
> > +uint64_t async_ring_pages_pdir_dma;
> > +};
> > +
> > +static int pvrdma_dsr_dma_pre_save(void *opaque)
> > +{
> > +struct PVRDMAMigTmp *tmp = opaque;
>
> For me tmp sounds like a very bad name, if it is the convention then i can
> live with that, anyways suggesting something like mig_tmp_info or something
> like that.

It doesn't seem like a convention, but this is how it is named
in other places in the source. But sure, I'll change it to a more
meaningful name, if it is needed.

> > +DSRInfo *dsr_info = >parent->dsr_info;
>
> Can you shad some light on how the parent field is initialized with the
> pointer to the device object?

TL;DR
The trick is on line 567 in migration/vmstate-types.c in the
function 'put_tmp'.


Each VMStateDescription has one or more VMStateField values
which are used to define members of the device state for migration.

VMSTATE_WITH_TMP is a macro which defines a VMStateField for
those types of device state which need an intermediate representation
and cannot be migrated directly. In our case, this might apply to the
dma addresses and pdir information stored within dsr.

Each VMStateField has a corresponding VMStateInfo, which basically
is an interface declaring two functions, get and put. These functions are
used to describe how this certain type of VMStateField has to be
saved/loaded during migration.

VMSTATE_WITH_TMP has its VMStateInfo set to 'vmstate_info_tmp',
which assigns the functions 'put_tmp' and 'get_tmp' for save/load logic.

Lets consider

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

2019-07-05 Thread Sukrit Bhatnagar
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 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 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




[Qemu-devel] [RFC v2 2/2] hw/pvrdma: add live migration support

2019-07-05 Thread Sukrit Bhatnagar
Use VMStateDescription for migrating device state. Currently,
'vmstate_pvrdma' describes the PCI and MSIX state for pvrdma and
'vmstate_pvrdma_dsr_dma' describes a temporary state containing
some values obtained only after mapping to dsr in the source.
Since the dsr will not be available on dest until we map to the
dma address we had on source, these values cannot be migrated
directly.

Add PVRDMAMigTmp to store this temporary state which consists of
dma addresses and ring page information. The 'parent' member is
used to refer to the device state (PVRDMADev) so that parent PCI
device object is accessible, which is needed to remap to DSR.

pvrdma_dsr_dma_pre_save() saves the dsr state into this temporary
representation and pvrdma_dsr_dma_post_load() loads it back.
This load function also remaps to the dsr and and calls
load_dsr() for further map and ring init operations.

Please note that this call to load_dsr() can be removed from the
migration flow and included in pvrdma_regs_write() to perform a
lazy load.
As of now, migration will fail if there in an error in load_dsr().
Also, there might be a considerable amount of pages in the rings,
which will have dma map operations when the init functions are
called.
If this takes noticeable time, it might be better to have lazy
load instead.

Cc: Marcel Apfelbaum 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 hw/rdma/vmw/pvrdma_main.c | 87 +++
 1 file changed, 87 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 6c90db96f9..4a10bd2fc7 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "hw/rdma/rdma.h"
+#include "migration/register.h"
 
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
@@ -593,6 +594,91 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
*opaque)
 pvrdma_fini(pci_dev);
 }
 
+struct PVRDMAMigTmp {
+PVRDMADev *parent;
+uint64_t dma;
+uint64_t cmd_slot_dma;
+uint64_t resp_slot_dma;
+uint32_t cq_ring_pages_num_pages;
+uint64_t cq_ring_pages_pdir_dma;
+uint32_t async_ring_pages_num_pages;
+uint64_t async_ring_pages_pdir_dma;
+};
+
+static int pvrdma_dsr_dma_pre_save(void *opaque)
+{
+struct PVRDMAMigTmp *tmp = opaque;
+DSRInfo *dsr_info = >parent->dsr_info;
+struct pvrdma_device_shared_region *dsr = dsr_info->dsr;
+
+tmp->dma = dsr_info->dma;
+tmp->cmd_slot_dma = dsr->cmd_slot_dma;
+tmp->resp_slot_dma = dsr->resp_slot_dma;
+tmp->cq_ring_pages_num_pages = dsr->cq_ring_pages.num_pages;
+tmp->cq_ring_pages_pdir_dma = dsr->cq_ring_pages.pdir_dma;
+tmp->async_ring_pages_num_pages = dsr->async_ring_pages.num_pages;
+tmp->async_ring_pages_pdir_dma = dsr->async_ring_pages.pdir_dma;
+
+return 0;
+}
+
+static int pvrdma_dsr_dma_post_load(void *opaque, int version_id)
+{
+struct PVRDMAMigTmp *tmp = opaque;
+PVRDMADev *dev = tmp->parent;
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+DSRInfo *dsr_info = >dsr_info;
+struct pvrdma_device_shared_region *dsr;
+
+dsr_info->dma = tmp->dma;
+dsr_info->dsr = rdma_pci_dma_map(pci_dev, dsr_info->dma,
+sizeof(struct pvrdma_device_shared_region));
+if (!dsr_info->dsr) {
+rdma_error_report("Failed to map to DSR");
+return -ENOMEM;
+}
+
+dsr = dsr_info->dsr;
+dsr->cmd_slot_dma = tmp->cmd_slot_dma;
+dsr->resp_slot_dma = tmp->resp_slot_dma;
+dsr->cq_ring_pages.num_pages = tmp->cq_ring_pages_num_pages;
+dsr->cq_ring_pages.pdir_dma = tmp->cq_ring_pages_pdir_dma;
+dsr->async_ring_pages.num_pages = tmp->async_ring_pages_num_pages;
+dsr->async_ring_pages.pdir_dma = tmp->async_ring_pages_pdir_dma;
+
+return load_dsr(dev);
+}
+
+static const VMStateDescription vmstate_pvrdma_dsr_dma = {
+.name = "pvrdma-dsr-dma",
+.pre_save = pvrdma_dsr_dma_pre_save,
+.post_load = pvrdma_dsr_dma_post_load,
+.fields = (VMStateField[]) {
+VMSTATE_UINT64(dma, struct PVRDMAMigTmp),
+VMSTATE_UINT64(cmd_slot_dma, struct PVRDMAMigTmp),
+VMSTATE_UINT64(resp_slot_dma, struct PVRDMAMigTmp),
+VMSTATE_UINT32(async_ring_pages_num_pages, struct PVRDMAMigTmp),
+VMSTATE_UINT64(async_ring_pages_pdir_dma, struct PVRDMAMigTmp),
+VMSTATE_UINT32(cq_ring_pages_num_pages, struct PVRDMAMigTmp),
+VMSTATE_UINT64(cq_ring_pages_pdir_dma, struct PVRDMAMigTmp),
+VMSTATE_END_OF_LIST()
+}
+};
+
+static const VMStateDescription vmstate_pvrdma = {
+.name = "pvrdma",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[

[Qemu-devel] [RFC v2 0/2] Add live migration support in the PVRDMA device

2019-07-05 Thread Sukrit Bhatnagar
Changes in v2:

* Modify load_dsr() such that dsr mapping is not performed if dsr value
  is non-NULL. Also move free_dsr() out of load_dsr() and call it right
  before if needed. These two changes will allow us to call load_dsr()
  even when we have already done dsr mapping and would like to go on
  with the rest of mappings.

* Use VMStateDescription instead of SaveVMHandlers to describe migration
  state. Also add fields for parent PCI object and MSIX.

* Use a temporary structure (struct PVRDMAMigTmp) to hold some fields
  during migration. These fields, such as cmd_slot_dma and resp_slot_dma
  inside dsr, do not fit into VMSTATE macros as their container
  (dsr_info->dsr) will not be ready until it is mapped on the dest.

* Perform mappings to CQ and event notification rings after the state is
  loaded. This is an extension to the mappings performed in v1;
  following the flow of load_dsr(). All the mappings are succesfully
  done on the dest on state load.

Link(s) to v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04924.html
https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04923.html


Things working now (were not working at the time of v1):

* vmxnet3 is migrating successfully. The issue was in the migration of
  its PCI configuration space, and is solved by the patch Marcel had sent:
  https://lists.gnu.org/archive/html/qemu-devel/2019-07/msg01500.html

* There is no problem due to BounceBuffers which were failing the dma mapping
  calls in state load logic earlier. Not sure exactly how it went away. I am
  guessing that adding the PCI and MSIX state to migration solved the issue.


What is still needed:

* A workaround to get libvirt to support same-host migration. Since
  the problems faced in v1 (mentioned above) are out of the way, we
  can move further, and in doing so, we will need this.

Sukrit Bhatnagar (2):
  hw/pvrdma: make DSR mapping idempotent in load_dsr()
  hw/pvrdma: add live migration support

 hw/rdma/vmw/pvrdma_main.c | 104 +++---
 1 file changed, 96 insertions(+), 8 deletions(-)

-- 
2.21.0




Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration

2019-07-05 Thread Sukrit Bhatnagar
On Fri, 5 Jul 2019 at 16:29, Dmitry Fleytman  wrote:
>
>
> > On 5 Jul 2019, at 4:07, Marcel Apfelbaum  wrote:
> >
> > At some point vmxnet3 live migration stopped working and git-bisect
> > didn't help finding a working version.
> > The issue is the PCI configuration space is not being migrated
> > successfully and MSIX remains masked at destination.
> >
> > Remove the migration differentiation between PCI and PCIe since
> > the logic resides now inside VMSTATE_PCI_DEVICE.
> > Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> > since at 'realize' time is decided if the device is PCI or PCIe,
> > then the above macro is enough.
> >
> > Use the opportunity to move to the standard VMSTATE_MSIX
> > instead of the deprecated SaveVMHandlers.
> >
> > Signed-off-by: Marcel Apfelbaum 
>
>
> Reviewed-by: Dmitry Fleytman 
>
Tested-by: Sukrit Bhatnagar 
>
> > ---
> > hw/net/vmxnet3.c | 52 ++--
> > 1 file changed, 2 insertions(+), 50 deletions(-)
> >
> > diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> > index 10d01d0058..8b17548b02 100644
> > --- a/hw/net/vmxnet3.c
> > +++ b/hw/net/vmxnet3.c
> > @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
> > msi_uninit(d);
> > }
> >
> > -static void
> > -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> > -{
> > -PCIDevice *d = PCI_DEVICE(opaque);
> > -msix_save(d, f);
> > -}
> > -
> > -static int
> > -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> > -{
> > -PCIDevice *d = PCI_DEVICE(opaque);
> > -msix_load(d, f);
> > -return 0;
> > -}
> > -
> > static const MemoryRegionOps b0_ops = {
> > .read = vmxnet3_io_bar0_read,
> > .write = vmxnet3_io_bar0_write,
> > @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
> > },
> > };
> >
> > -static SaveVMHandlers savevm_vmxnet3_msix = {
> > -.save_state = vmxnet3_msix_save,
> > -.load_state = vmxnet3_msix_load,
> > -};
> > -
> > static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
> > {
> > uint64_t dsn_payload;
> > @@ -2203,7 +2183,6 @@ static uint64_t 
> > vmxnet3_device_serial_num(VMXNET3State *s)
> >
> > static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
> > {
> > -DeviceState *dev = DEVICE(pci_dev);
> > VMXNET3State *s = VMXNET3(pci_dev);
> > int ret;
> >
> > @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, 
> > Error **errp)
> > pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
> >   vmxnet3_device_serial_num(s));
> > }
> > -
> > -register_savevm_live(dev, "vmxnet3-msix", -1, 1, _vmxnet3_msix, 
> > s);
> > }
> >
> > static void vmxnet3_instance_init(Object *obj)
> > @@ -2440,29 +2417,6 @@ static const VMStateDescription 
> > vmstate_vmxnet3_int_state = {
> > }
> > };
> >
> > -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> > -{
> > -VMXNET3State *s = VMXNET3(opaque);
> > -
> > -return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> > -}
> > -
> > -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> > -{
> > -return !vmxnet3_vmstate_need_pcie_device(opaque);
> > -}
> > -
> > -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> > -.name = "vmxnet3/pcie",
> > -.version_id = 1,
> > -.minimum_version_id = 1,
> > -.needed = vmxnet3_vmstate_need_pcie_device,
> > -.fields = (VMStateField[]) {
> > -VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > -VMSTATE_END_OF_LIST()
> > -}
> > -};
> > -
> > static const VMStateDescription vmstate_vmxnet3 = {
> > .name = "vmxnet3",
> > .version_id = 1,
> > @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
> > .pre_save = vmxnet3_pre_save,
> > .post_load = vmxnet3_post_load,
> > .fields = (VMStateField[]) {
> > -VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> > -vmxnet3_vmstate_test_pci_device, 0,
> > -vmstate_pci_device, PCIDevice),
> > +VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> > +VMSTATE_MSIX(parent_obj, VMXNET3State),
> > VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
> > VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
> > VMSTATE_BOOL(lro_supported, VMXNET3State),
> > @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
> > },
> > .subsections = (const VMStateDescription*[]) {
> > _vmxnet3_mcast_list,
> > -_vmxnet3_pcie_device,
> > NULL
> > }
> > };
> > --
> > 2.17.1
> >
>



Re: [Qemu-devel] [PATCH] hw/net: fix vmxnet3 live migration

2019-07-05 Thread Sukrit Bhatnagar
On Fri, 5 Jul 2019 at 06:38, Marcel Apfelbaum
 wrote:
>
> At some point vmxnet3 live migration stopped working and git-bisect
> didn't help finding a working version.
> The issue is the PCI configuration space is not being migrated
> successfully and MSIX remains masked at destination.
>
> Remove the migration differentiation between PCI and PCIe since
> the logic resides now inside VMSTATE_PCI_DEVICE.
> Remove also the VMXNET3_COMPAT_FLAG_DISABLE_PCIE based differentiation
> since at 'realize' time is decided if the device is PCI or PCIe,
> then the above macro is enough.
>
> Use the opportunity to move to the standard VMSTATE_MSIX
> instead of the deprecated SaveVMHandlers.
>
> Signed-off-by: Marcel Apfelbaum 
Tested-by: Sukrit Bhatnagar 

> ---
>  hw/net/vmxnet3.c | 52 ++--
>  1 file changed, 2 insertions(+), 50 deletions(-)
>
> diff --git a/hw/net/vmxnet3.c b/hw/net/vmxnet3.c
> index 10d01d0058..8b17548b02 100644
> --- a/hw/net/vmxnet3.c
> +++ b/hw/net/vmxnet3.c
> @@ -2141,21 +2141,6 @@ vmxnet3_cleanup_msi(VMXNET3State *s)
>  msi_uninit(d);
>  }
>
> -static void
> -vmxnet3_msix_save(QEMUFile *f, void *opaque)
> -{
> -PCIDevice *d = PCI_DEVICE(opaque);
> -msix_save(d, f);
> -}
> -
> -static int
> -vmxnet3_msix_load(QEMUFile *f, void *opaque, int version_id)
> -{
> -PCIDevice *d = PCI_DEVICE(opaque);
> -msix_load(d, f);
> -return 0;
> -}
> -
>  static const MemoryRegionOps b0_ops = {
>  .read = vmxnet3_io_bar0_read,
>  .write = vmxnet3_io_bar0_write,
> @@ -2176,11 +2161,6 @@ static const MemoryRegionOps b1_ops = {
>  },
>  };
>
> -static SaveVMHandlers savevm_vmxnet3_msix = {
> -.save_state = vmxnet3_msix_save,
> -.load_state = vmxnet3_msix_load,
> -};
> -
>  static uint64_t vmxnet3_device_serial_num(VMXNET3State *s)
>  {
>  uint64_t dsn_payload;
> @@ -2203,7 +2183,6 @@ static uint64_t vmxnet3_device_serial_num(VMXNET3State 
> *s)
>
>  static void vmxnet3_pci_realize(PCIDevice *pci_dev, Error **errp)
>  {
> -DeviceState *dev = DEVICE(pci_dev);
>  VMXNET3State *s = VMXNET3(pci_dev);
>  int ret;
>
> @@ -2249,8 +2228,6 @@ static void vmxnet3_pci_realize(PCIDevice *pci_dev, 
> Error **errp)
>  pcie_dev_ser_num_init(pci_dev, VMXNET3_DSN_OFFSET,
>vmxnet3_device_serial_num(s));
>  }
> -
> -register_savevm_live(dev, "vmxnet3-msix", -1, 1, _vmxnet3_msix, 
> s);
>  }
>
>  static void vmxnet3_instance_init(Object *obj)
> @@ -2440,29 +2417,6 @@ static const VMStateDescription 
> vmstate_vmxnet3_int_state = {
>  }
>  };
>
> -static bool vmxnet3_vmstate_need_pcie_device(void *opaque)
> -{
> -VMXNET3State *s = VMXNET3(opaque);
> -
> -return !(s->compat_flags & VMXNET3_COMPAT_FLAG_DISABLE_PCIE);
> -}
> -
> -static bool vmxnet3_vmstate_test_pci_device(void *opaque, int version_id)
> -{
> -return !vmxnet3_vmstate_need_pcie_device(opaque);
> -}
> -
> -static const VMStateDescription vmstate_vmxnet3_pcie_device = {
> -.name = "vmxnet3/pcie",
> -.version_id = 1,
> -.minimum_version_id = 1,
> -.needed = vmxnet3_vmstate_need_pcie_device,
> -.fields = (VMStateField[]) {
> -VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> -VMSTATE_END_OF_LIST()
> -}
> -};
> -
>  static const VMStateDescription vmstate_vmxnet3 = {
>  .name = "vmxnet3",
>  .version_id = 1,
> @@ -2470,9 +2424,8 @@ static const VMStateDescription vmstate_vmxnet3 = {
>  .pre_save = vmxnet3_pre_save,
>  .post_load = vmxnet3_post_load,
>  .fields = (VMStateField[]) {
> -VMSTATE_STRUCT_TEST(parent_obj, VMXNET3State,
> -vmxnet3_vmstate_test_pci_device, 0,
> -vmstate_pci_device, PCIDevice),
> +VMSTATE_PCI_DEVICE(parent_obj, VMXNET3State),
> +VMSTATE_MSIX(parent_obj, VMXNET3State),
>  VMSTATE_BOOL(rx_packets_compound, VMXNET3State),
>  VMSTATE_BOOL(rx_vlan_stripping, VMXNET3State),
>  VMSTATE_BOOL(lro_supported, VMXNET3State),
> @@ -2508,7 +2461,6 @@ static const VMStateDescription vmstate_vmxnet3 = {
>  },
>  .subsections = (const VMStateDescription*[]) {
>  _vmxnet3_mcast_list,
> -_vmxnet3_pcie_device,
>  NULL
>  }
>  };
> --
> 2.17.1
>



Re: [Qemu-devel] [RFC 1/1] hw/pvrdma: Add live migration support

2019-06-30 Thread Sukrit Bhatnagar
On Sun, 30 Jun 2019 at 13:43, Yuval Shaia  wrote:
>
> On Sat, Jun 29, 2019 at 06:15:21PM +0530, Sukrit Bhatnagar wrote:
> > On Fri, 28 Jun 2019 at 16:56, Dr. David Alan Gilbert
> >  wrote:
> > >
> > > * Yuval Shaia (yuval.sh...@oracle.com) wrote:
> > > > On Fri, Jun 21, 2019 at 08:15:41PM +0530, Sukrit Bhatnagar wrote:
> > > > > Define and register SaveVMHandlers pvrdma_save and
> > > > > pvrdma_load for saving and loading the device state,
> > > > > which currently includes only the dma, command slot
> > > > > and response slot addresses.
> > > > >
> > > > > Remap the DSR, command slot and response slot upon
> > > > > loading the addresses in the pvrdma_load function.
> > > > >
> > > > > Cc: Marcel Apfelbaum 
> > > > > Cc: Yuval Shaia 
> > > > > Signed-off-by: Sukrit Bhatnagar 
> > > > > ---
> > > > >  hw/rdma/vmw/pvrdma_main.c | 56 
> > > > > +++
> > > > >  1 file changed, 56 insertions(+)
> > > > >
> > > > > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > > > > index adcf79cd63..cd8573173c 100644
> > > > > --- a/hw/rdma/vmw/pvrdma_main.c
> > > > > +++ b/hw/rdma/vmw/pvrdma_main.c
> > > > > @@ -28,6 +28,7 @@
> > > > >  #include "sysemu/sysemu.h"
> > > > >  #include "monitor/monitor.h"
> > > > >  #include "hw/rdma/rdma.h"
> > > > > +#include "migration/register.h"
> > > > >
> > > > >  #include "../rdma_rm.h"
> > > > >  #include "../rdma_backend.h"
> > > > > @@ -592,9 +593,62 @@ static void pvrdma_shutdown_notifier(Notifier 
> > > > > *n, void *opaque)
> > > > >  pvrdma_fini(pci_dev);
> > > > >  }
> > > > >
> > > > > +static void pvrdma_save(QEMUFile *f, void *opaque)
> > > > > +{
> > > > > +PVRDMADev *dev = PVRDMA_DEV(opaque);
> > > > > +
> > > > > +qemu_put_be64(f, dev->dsr_info.dma);
> > > > > +qemu_put_be64(f, dev->dsr_info.dsr->cmd_slot_dma);
> > > > > +qemu_put_be64(f, dev->dsr_info.dsr->resp_slot_dma);
> > > > > +}
> > > > > +
> > > > > +static int pvrdma_load(QEMUFile *f, void *opaque, int version_id)
> > > > > +{
> > > > > +PVRDMADev *dev = PVRDMA_DEV(opaque);
> > > > > +PCIDevice *pci_dev = PCI_DEVICE(dev);
> > > > > +
> > > > > +// Remap DSR
> > > > > +dev->dsr_info.dma = qemu_get_be64(f);
> > > > > +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");
> > > > > +return -1;
> > > > > +}
> > > > > +qemu_log("pvrdma_load: successfully remapped to DSR\n");
> > > > > +
> > > > > +// Remap cmd slot
> > > > > +dev->dsr_info.dsr->cmd_slot_dma = qemu_get_be64(f);
> > > > > +dev->dsr_info.req = rdma_pci_dma_map(pci_dev, 
> > > > > dev->dsr_info.dsr->cmd_slot_dma,
> > > > > + sizeof(union pvrdma_cmd_req));
> > > > > +if (!dev->dsr_info.req) {
> > > >
> > > > So this is where you hit the error, right?
> > > > All looks good to me, i wonder why the first pci_dma_map works fine but
> > > > second fails where the global BounceBuffer is marked as used.
> > > >
> > > > Anyone?
> > >
> > > I've got a few guesses:
> > >   a) My reading of address_space_map is that it gives a bounce buffer
> > > pointer and then it has to be freed; so if it's going wrong and using a
> > > bounce buffer, then the 1st call would work and only the 2nd would fail.
> >
> > In the scenario without any migration, the device is init and load_dsr()
> > is called when the guest writes to DSR in BAR1 of pvrdma.
> >
> > Inside the load_dsr(), there are similar calls to rdma_pci_dma_map().
> >
> >

Re: [Qemu-devel] [RFC 1/1] hw/pvrdma: Add live migration support

2019-06-29 Thread Sukrit Bhatnagar
On Fri, 28 Jun 2019 at 16:56, Dr. David Alan Gilbert
 wrote:
>
> * Yuval Shaia (yuval.sh...@oracle.com) wrote:
> > On Fri, Jun 21, 2019 at 08:15:41PM +0530, Sukrit Bhatnagar wrote:
> > > Define and register SaveVMHandlers pvrdma_save and
> > > pvrdma_load for saving and loading the device state,
> > > which currently includes only the dma, command slot
> > > and response slot addresses.
> > >
> > > Remap the DSR, command slot and response slot upon
> > > loading the addresses in the pvrdma_load function.
> > >
> > > Cc: Marcel Apfelbaum 
> > > Cc: Yuval Shaia 
> > > Signed-off-by: Sukrit Bhatnagar 
> > > ---
> > >  hw/rdma/vmw/pvrdma_main.c | 56 +++
> > >  1 file changed, 56 insertions(+)
> > >
> > > diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
> > > index adcf79cd63..cd8573173c 100644
> > > --- a/hw/rdma/vmw/pvrdma_main.c
> > > +++ b/hw/rdma/vmw/pvrdma_main.c
> > > @@ -28,6 +28,7 @@
> > >  #include "sysemu/sysemu.h"
> > >  #include "monitor/monitor.h"
> > >  #include "hw/rdma/rdma.h"
> > > +#include "migration/register.h"
> > >
> > >  #include "../rdma_rm.h"
> > >  #include "../rdma_backend.h"
> > > @@ -592,9 +593,62 @@ static void pvrdma_shutdown_notifier(Notifier *n, 
> > > void *opaque)
> > >  pvrdma_fini(pci_dev);
> > >  }
> > >
> > > +static void pvrdma_save(QEMUFile *f, void *opaque)
> > > +{
> > > +PVRDMADev *dev = PVRDMA_DEV(opaque);
> > > +
> > > +qemu_put_be64(f, dev->dsr_info.dma);
> > > +qemu_put_be64(f, dev->dsr_info.dsr->cmd_slot_dma);
> > > +qemu_put_be64(f, dev->dsr_info.dsr->resp_slot_dma);
> > > +}
> > > +
> > > +static int pvrdma_load(QEMUFile *f, void *opaque, int version_id)
> > > +{
> > > +PVRDMADev *dev = PVRDMA_DEV(opaque);
> > > +PCIDevice *pci_dev = PCI_DEVICE(dev);
> > > +
> > > +// Remap DSR
> > > +dev->dsr_info.dma = qemu_get_be64(f);
> > > +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");
> > > +return -1;
> > > +}
> > > +qemu_log("pvrdma_load: successfully remapped to DSR\n");
> > > +
> > > +// Remap cmd slot
> > > +dev->dsr_info.dsr->cmd_slot_dma = qemu_get_be64(f);
> > > +dev->dsr_info.req = rdma_pci_dma_map(pci_dev, 
> > > dev->dsr_info.dsr->cmd_slot_dma,
> > > + sizeof(union pvrdma_cmd_req));
> > > +if (!dev->dsr_info.req) {
> >
> > So this is where you hit the error, right?
> > All looks good to me, i wonder why the first pci_dma_map works fine but
> > second fails where the global BounceBuffer is marked as used.
> >
> > Anyone?
>
> I've got a few guesses:
>   a) My reading of address_space_map is that it gives a bounce buffer
> pointer and then it has to be freed; so if it's going wrong and using a
> bounce buffer, then the 1st call would work and only the 2nd would fail.

In the scenario without any migration, the device is init and load_dsr()
is called when the guest writes to DSR in BAR1 of pvrdma.

Inside the load_dsr(), there are similar calls to rdma_pci_dma_map().

The difference is that when the address_space_map() is called there,
!memory_access_is_direct() condition is FALSE.
So, there is no use for BounceBuffer.


In the migration scenario, when the device at dest calls load and
subsequently rdma_pci_dma_map(), the !memory_access_is_direct()
condition is TRUE.
So, the first rdma_pci_dma_map() will set BounceBuffer from FALSE to
TRUE and succeed. But, the subsequent calls will fail at atomic_xchg().

This BounceBuffer is only freed when address_space_unmap() is called.


I am guessing that when the address is translated to one in FlatView,
the MemoryRegion returned at dest is causing the issue.
To be honest, at this point I am not sure how FlatView translations works.

I tried adding qemu_log to memory_access_is_direct(), but I guess it is
called too many times, so the vm stalls before it even starts.

>   b) Perhaps the dma address read from the stream is bad, and isn't
> pointing into RAM properly - and that's why you're getting a bounce
&

Re: [Qemu-devel] [GSoC] Help needed in implementing live migration

2019-06-26 Thread Sukrit Bhatnagar
On Tue, 25 Jun 2019 at 00:11, Dr. David Alan Gilbert
 wrote:
>
> * Sukrit Bhatnagar (skrtbht...@gmail.com) wrote:
> > Hi David,
> >
> > I am Sukrit, GSoC participant working on PVRDMA live migration.
> > We had a short chat about vmxnet3 migration about a week ago
> > on the IRC channel.
> >
> > I am facing an issue while doing migration of the pvrdma device.
> > While loading the device state, we need to perform a few dma
> > mappings on the destination. But on the destination, the migration
> > fails due a BounceBuffer being locked (in_use). This global
> > BounceBuffer is used in address_space_map/unmap functions
> > which the rdma_pci_dma_map/unmap calls.
> > Essentially, we need a way to remap guest physical address on
> > the destination after migration.
> >
> > I had posted an RFC a while ago on the list:
> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04924.html
> > https://lists.gnu.org/archive/html/qemu-devel/2019-06/msg04923.html
> >
> > My mentors (Marcel and Yuval) told me to ask you for help
> > regarding this. It would be really great if you can guide me in
> > finding a workaround for this.
>
> Hi,
>   I'll have a look; I need to get some other things finished first.

Adding cc: qemu-devel, sorry for the private email.

> Dave
> > Thanks,
> > Sukrit
> --
> Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK



[Qemu-devel] [RFC 0/1] Add live migration support to the PVRDMA device

2019-06-21 Thread Sukrit Bhatnagar
Hi,

I am a GSoC participant, trying to implement live migration for the
pvrdma device with help from my mentors Marcel and Yuval.
My current task is to save and load the various addresses that the
device uses for DMA mapping. We will be adding the device state into
live migration, incrementally. As the first step in the implementation,
we are performing migration to the same host. This will save us from
many complexities, such as GID change, at this stage, and we will
address migration across hosts at a later point when same-host migration
works.

Currently, the save and load logic uses SaveVMHandlers, which is the
legcay way, and will be ported to VMStateDescription once the
existing issues are solved.

This RFC is meant to request suggestions on the things which are
working and for help on the things which are not.


What is working:

* pvrdma device is getting initialized in a VM, its GID entry is
  getting added to the host, and rc_pingpong is successful between
  two such VMs. This is when libvirt is used to launch the VMs.

* The dma, cmd_slot_dma and resp_slot_dma addresses are saved at the
  source and loaded properly in the destination upon migration. That is,
  the values loaded at the dest during migration are the same as the
  ones saved.

  `dma` is provided by the guest device when it writes to BAR1, stored
  in dev->dsr_info.dma. A DSR is created on mapping to this address.
  `cmd_slot_dma` and `resp_slot_dma` are the dma addresses of the command
  and response buffers, respectively, which are provided by the guest
  through the DSR.

* The DSR successfully (re)maps to the dma address loaded from
  migration at the dest.


What is not working:

* In the pvrdma_load() logic, the mapping to DSR is successful at dest.
  But the mapping for cmd and resp slots fails.
  rdma_pci_dma_map() eventually calls address_space_map(). Inside the
  latter, a global BounceBuffer bounce is checked to see if it is in use
  (the atomic_xchg() primitive).
  At the dest, it is in use and the dma remapping fails there, which
  fails the whole migration process. Essentially, I am looking for a
  way to remap guest physical address after a live migration (to the
  same host). Any tips on avoiding the BounceBuffer will also be great.

  I have also tried unmapping the cmd and resp slots at the source before
  saving the dma addresses in pvrdma_save(), but the mapping fails anyway.

* It seems that vmxnet3 migration itself is not working properly, at least
  for me. The pvrdma device depends on it, vmxnet3 is function 0 and pvrdma
  is function 1. This is happening even for a build of unmodified code from
  the master branch.
  After migration, the network connectivity is lost at destination.
  Things are fine at the source before migration.
  This is the command I am using at src:

  sudo /home/skrtbhtngr/qemu/build/x86_64-softmmu/qemu-system-x86_64 \
-enable-kvm \
-m 2G -smp cpus=2 \
-hda /home/skrtbhtngr/fedora.img \
-netdev tap,id=hostnet0 \
-device vmxnet3,netdev=hostnet0,id=net0,mac=52:54:00:99:ff:bc \
-monitor telnet:127.0.0.1:,server,nowait \
-trace events=/home/skrtbhtngr/trace-events \
-vnc 0.0.0.0:0

  Similar command is used for the dest. Currently, I am trying
  same-host migration for testing purpose, without the pvrdma device.
  Two tap interfaces, for src and dest were created successfully at
  the host. Kernel logs:
  ...
  br0: port 2(tap0) entered forwarding state
  ...
  br0: port 3(tap1) entered forwarding state

  tcpdump at the dest reports only outgoing ARP packets, which ask
  for gateway: "ARP, Request who-has _gateway tell guest1".

  Tried using user (slirp) as the network backend, but no luck.
  
  Also tried git bisect to find the issue using a working commit (given
  by Marcel), but it turns out that it is very old and I faced build
  errors one after another.

  Please note that e1000 live migration is working fine in the same setup.

* Since we are aiming at trying on same-host migration first, I cannot
  use libvirt as it does not allow this. Currently, I am running the
  VMs using qemu-system commands. But libvirt is needed to add the GID
  entry of the guest device in the host. I am looking for a workaround,
  if that is possible at all.
  I started a thread few days ago for the same on libvirt-users:
  https://www.redhat.com/archives/libvirt-users/2019-June/msg00011.html


Sukrit Bhatnagar (1):
  hw/pvrdma: Add live migration support

 hw/rdma/vmw/pvrdma_main.c | 56 +++
 1 file changed, 56 insertions(+)

-- 
2.21.0




[Qemu-devel] [RFC 1/1] hw/pvrdma: Add live migration support

2019-06-21 Thread Sukrit Bhatnagar
Define and register SaveVMHandlers pvrdma_save and
pvrdma_load for saving and loading the device state,
which currently includes only the dma, command slot
and response slot addresses.

Remap the DSR, command slot and response slot upon
loading the addresses in the pvrdma_load function.

Cc: Marcel Apfelbaum 
Cc: Yuval Shaia 
Signed-off-by: Sukrit Bhatnagar 
---
 hw/rdma/vmw/pvrdma_main.c | 56 +++
 1 file changed, 56 insertions(+)

diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index adcf79cd63..cd8573173c 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -28,6 +28,7 @@
 #include "sysemu/sysemu.h"
 #include "monitor/monitor.h"
 #include "hw/rdma/rdma.h"
+#include "migration/register.h"
 
 #include "../rdma_rm.h"
 #include "../rdma_backend.h"
@@ -592,9 +593,62 @@ static void pvrdma_shutdown_notifier(Notifier *n, void 
*opaque)
 pvrdma_fini(pci_dev);
 }
 
+static void pvrdma_save(QEMUFile *f, void *opaque)
+{
+PVRDMADev *dev = PVRDMA_DEV(opaque);
+
+qemu_put_be64(f, dev->dsr_info.dma);
+qemu_put_be64(f, dev->dsr_info.dsr->cmd_slot_dma);
+qemu_put_be64(f, dev->dsr_info.dsr->resp_slot_dma);
+}
+
+static int pvrdma_load(QEMUFile *f, void *opaque, int version_id)
+{
+PVRDMADev *dev = PVRDMA_DEV(opaque);
+PCIDevice *pci_dev = PCI_DEVICE(dev);
+
+// Remap DSR
+dev->dsr_info.dma = qemu_get_be64(f);
+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");
+return -1;
+}
+qemu_log("pvrdma_load: successfully remapped to DSR\n");
+
+// Remap cmd slot
+dev->dsr_info.dsr->cmd_slot_dma = qemu_get_be64(f);
+dev->dsr_info.req = rdma_pci_dma_map(pci_dev, 
dev->dsr_info.dsr->cmd_slot_dma,
+ sizeof(union pvrdma_cmd_req));
+if (!dev->dsr_info.req) {
+rdma_error_report("Failed to map to command slot address");
+return -1;
+}
+qemu_log("pvrdma_load: successfully remapped to cmd slot\n");
+
+// Remap rsp slot
+dev->dsr_info.dsr->resp_slot_dma = qemu_get_be64(f);
+dev->dsr_info.rsp = rdma_pci_dma_map(pci_dev, 
dev->dsr_info.dsr->resp_slot_dma,
+ sizeof(union pvrdma_cmd_resp));
+if (!dev->dsr_info.rsp) {
+rdma_error_report("Failed to map to response slot address");
+return -1;
+}
+qemu_log("pvrdma_load: successfully remapped to rsp slot\n");
+
+return 0;
+}
+
+static SaveVMHandlers savevm_pvrdma = {
+.save_state = pvrdma_save,
+.load_state = pvrdma_load,
+};
+
 static void pvrdma_realize(PCIDevice *pdev, Error **errp)
 {
 int rc = 0;
+DeviceState *s = DEVICE(pdev);
 PVRDMADev *dev = PVRDMA_DEV(pdev);
 Object *memdev_root;
 bool ram_shared = false;
@@ -666,6 +720,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp)
 dev->shutdown_notifier.notify = pvrdma_shutdown_notifier;
 qemu_register_shutdown_notifier(>shutdown_notifier);
 
+register_savevm_live(s, "pvrdma", -1, 1, _pvrdma, dev);
+
 out:
 if (rc) {
 pvrdma_fini(pdev);
-- 
2.21.0




Re: [Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-04-10 Thread Sukrit Bhatnagar
On Wed, 10 Apr 2019 at 17:20, Roman Bolshakov  wrote:
>
> On Sun, Apr 07, 2019 at 05:28:39PM +0530, Sukrit Bhatnagar wrote:
> > Keep the calls made to synchronize cpu by all hypervisors in one place
> > inside cpu_synchronize_* functions in include/sysemu/hw_accel.h
> >
> > Cc: Richard Henderson 
> > Cc: Paolo Bonzini 
> > Signed-off-by: Sukrit Bhatnagar 
> > ---
> >  cpus.c| 12 
> >  include/sysemu/hw_accel.h | 10 ++
> >  2 files changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/cpus.c b/cpus.c
> > index e83f72b48b..026df0dc5f 100644
> > --- a/cpus.c
> > +++ b/cpus.c
> > @@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void)
> >
> >  CPU_FOREACH(cpu) {
> >  cpu_synchronize_state(cpu);
> > -/* TODO: move to cpu_synchronize_state() */
> > -if (hvf_enabled()) {
> > -hvf_cpu_synchronize_state(cpu);
> > -}
> >  }
> >  }
> >
> > @@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void)
> >
> >  CPU_FOREACH(cpu) {
> >  cpu_synchronize_post_reset(cpu);
> > -/* TODO: move to cpu_synchronize_post_reset() */
> > -if (hvf_enabled()) {
> > -hvf_cpu_synchronize_post_reset(cpu);
> > -}
> >  }
> >  }
> >
> > @@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void)
> >
> >  CPU_FOREACH(cpu) {
> >  cpu_synchronize_post_init(cpu);
> > -/* TODO: move to cpu_synchronize_post_init() */
> > -if (hvf_enabled()) {
> > -hvf_cpu_synchronize_post_init(cpu);
> > -}
> >  }
> >  }
> >
> > diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
> > index d2ddfb5ad0..bf081b4026 100644
> > --- a/include/sysemu/hw_accel.h
> > +++ b/include/sysemu/hw_accel.h
> > @@ -15,6 +15,7 @@
> >  #include "sysemu/hax.h"
> >  #include "sysemu/kvm.h"
> >  #include "sysemu/whpx.h"
> > +#include "sysemu/hvf.h"
> >
> >  static inline void cpu_synchronize_state(CPUState *cpu)
> >  {
> > @@ -27,6 +28,9 @@ static inline void cpu_synchronize_state(CPUState *cpu)
> >  if (whpx_enabled()) {
> >  whpx_cpu_synchronize_state(cpu);
> >  }
> > +if (hvf_enabled()) {
> > +hvf_cpu_synchronize_state(cpu);
> > +}
> >  }
> >
> >  static inline void cpu_synchronize_post_reset(CPUState *cpu)
> > @@ -40,6 +44,9 @@ static inline void cpu_synchronize_post_reset(CPUState 
> > *cpu)
> >  if (whpx_enabled()) {
> >  whpx_cpu_synchronize_post_reset(cpu);
> >  }
> > +if (hvf_enabled()) {
> > +hvf_cpu_synchronize_post_reset(cpu);
> > +}
> >  }
> >
> >  static inline void cpu_synchronize_post_init(CPUState *cpu)
> > @@ -53,6 +60,9 @@ static inline void cpu_synchronize_post_init(CPUState 
> > *cpu)
> >  if (whpx_enabled()) {
> >  whpx_cpu_synchronize_post_init(cpu);
> >  }
> > +if (hvf_enabled()) {
> > +hvf_cpu_synchronize_post_init(cpu);
> > +}
> >  }
> >
> >  static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
> > --
> > 2.20.1
> >
> >
>
> Hi Sukrit,
>
> The commit itself looks sane but qemu starts to hang during BIOS boot
> after I've applied it if I run it with hvf accel.

Hi Roman,
Thanks for reviewing the patches.
If this is happening, then I assume patches are not ready
to be merged into qemu-stable.
Do you have any idea why the problem is not detected
during tests, but rather at runtime?
I am not really sure if this patch (2/2) has anything to do with
the problem but the previous patch (1/2) might.

> Thanks,
> Roman



Re: [Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-04-07 Thread Sukrit Bhatnagar
Hi,
Are the patches in good enough shape to be merged?

Thanks,
Sukrit



[Qemu-devel] [PATCH RESEND v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-04-07 Thread Sukrit Bhatnagar
Changes made in v2:
- added a patch to declare hvf_handle_io only if NEED_CPU_H
  is defined so that the poisoned type CPUArchState does not
  produce an error when include/sysemu/hvf.h is included for
  common object compilation

Link to v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06902.html

Sukrit Bhatnagar (2):
  hvf: declare hvf_handle_io if NEED_CPU_H is defined
  cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

 cpus.c| 12 
 include/sysemu/hvf.h  |  4 
 include/sysemu/hw_accel.h | 10 ++
 3 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.20.1




[Qemu-devel] [PATCH RESEND v2 1/2] hvf: declare hvf_handle_io if NEED_CPU_H is defined

2019-04-07 Thread Sukrit Bhatnagar
hvf_handle_io needs the poisoned type CPUArchState as its argument.
Declaring it if NEED_CPU_H is defined enables include/sysemu/hvf.h
to be included for common object compilation as well.

Cc: Roman Bolshakov 
Cc: Paolo Bonzini 
Signed-off-by: Sukrit Bhatnagar 
---
 include/sysemu/hvf.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index aaa51d2c51..7eca3ec7be 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -63,8 +63,12 @@ typedef struct HVFState {
 extern HVFState *hvf_state;
 
 void hvf_set_phys_mem(MemoryRegionSection *, bool);
+
+#ifdef NEED_CPU_H
 void hvf_handle_io(CPUArchState *, uint16_t, void *,
   int, int, int);
+#endif
+
 hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
 
 /* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
-- 
2.20.1




[Qemu-devel] [PATCH RESEND v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-04-07 Thread Sukrit Bhatnagar
Keep the calls made to synchronize cpu by all hypervisors in one place
inside cpu_synchronize_* functions in include/sysemu/hw_accel.h

Cc: Richard Henderson 
Cc: Paolo Bonzini 
Signed-off-by: Sukrit Bhatnagar 
---
 cpus.c| 12 
 include/sysemu/hw_accel.h | 10 ++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/cpus.c b/cpus.c
index e83f72b48b..026df0dc5f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_state(cpu);
-/* TODO: move to cpu_synchronize_state() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_state(cpu);
-}
 }
 }
 
@@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_reset(cpu);
-/* TODO: move to cpu_synchronize_post_reset() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_reset(cpu);
-}
 }
 }
 
@@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_init(cpu);
-/* TODO: move to cpu_synchronize_post_init() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_init(cpu);
-}
 }
 }
 
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index d2ddfb5ad0..bf081b4026 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -15,6 +15,7 @@
 #include "sysemu/hax.h"
 #include "sysemu/kvm.h"
 #include "sysemu/whpx.h"
+#include "sysemu/hvf.h"
 
 static inline void cpu_synchronize_state(CPUState *cpu)
 {
@@ -27,6 +28,9 @@ static inline void cpu_synchronize_state(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_state(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_state(cpu);
+}
 }
 
 static inline void cpu_synchronize_post_reset(CPUState *cpu)
@@ -40,6 +44,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_reset(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_reset(cpu);
+}
 }
 
 static inline void cpu_synchronize_post_init(CPUState *cpu)
@@ -53,6 +60,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_init(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_init(cpu);
+}
 }
 
 static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
-- 
2.20.1




[Qemu-devel] [PATCH v2 0/2] Move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-03-27 Thread Sukrit Bhatnagar
Changes made in v2:
- added a patch to declare hvf_handle_io only if NEED_CPU_H
  is defined so that the poisoned type CPUArchState does not
  produce an error when include/sysemu/hvf.h is included for
  common object compilation

Link to v1:
https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06902.html

Sukrit Bhatnagar (2):
  hvf: declare hvf_handle_io if NEED_CPU_H is defined
  cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

 cpus.c| 12 
 include/sysemu/hvf.h  |  4 
 include/sysemu/hw_accel.h | 10 ++
 3 files changed, 14 insertions(+), 12 deletions(-)

-- 
2.20.1




[Qemu-devel] [PATCH v2 2/2] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-03-27 Thread Sukrit Bhatnagar
Keep the calls made to synchronize cpu by all hypervisors in one place
inside cpu_synchronize_* functions in include/sysemu/hw_accel.h

Cc: Richard Henderson 
Cc: Paolo Bonzini 
Signed-off-by: Sukrit Bhatnagar 
---
 cpus.c| 12 
 include/sysemu/hw_accel.h | 10 ++
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/cpus.c b/cpus.c
index e83f72b48b..026df0dc5f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_state(cpu);
-/* TODO: move to cpu_synchronize_state() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_state(cpu);
-}
 }
 }
 
@@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_reset(cpu);
-/* TODO: move to cpu_synchronize_post_reset() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_reset(cpu);
-}
 }
 }
 
@@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_init(cpu);
-/* TODO: move to cpu_synchronize_post_init() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_init(cpu);
-}
 }
 }
 
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index d2ddfb5ad0..bf081b4026 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -15,6 +15,7 @@
 #include "sysemu/hax.h"
 #include "sysemu/kvm.h"
 #include "sysemu/whpx.h"
+#include "sysemu/hvf.h"
 
 static inline void cpu_synchronize_state(CPUState *cpu)
 {
@@ -27,6 +28,9 @@ static inline void cpu_synchronize_state(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_state(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_state(cpu);
+}
 }
 
 static inline void cpu_synchronize_post_reset(CPUState *cpu)
@@ -40,6 +44,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_reset(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_reset(cpu);
+}
 }
 
 static inline void cpu_synchronize_post_init(CPUState *cpu)
@@ -53,6 +60,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_init(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_init(cpu);
+}
 }
 
 static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
-- 
2.20.1




[Qemu-devel] [PATCH v2 1/2] hvf: declare hvf_handle_io if NEED_CPU_H is defined

2019-03-27 Thread Sukrit Bhatnagar
hvf_handle_io needs the poisoned type CPUArchState as its argument.
Declaring it if NEED_CPU_H is defined enables include/sysemu/hvf.h
to be included for common object compilation as well.

Cc: Roman Bolshakov 
Cc: Paolo Bonzini 
Signed-off-by: Sukrit Bhatnagar 
---
 include/sysemu/hvf.h | 4 
 1 file changed, 4 insertions(+)

diff --git a/include/sysemu/hvf.h b/include/sysemu/hvf.h
index aaa51d2c51..7eca3ec7be 100644
--- a/include/sysemu/hvf.h
+++ b/include/sysemu/hvf.h
@@ -63,8 +63,12 @@ typedef struct HVFState {
 extern HVFState *hvf_state;
 
 void hvf_set_phys_mem(MemoryRegionSection *, bool);
+
+#ifdef NEED_CPU_H
 void hvf_handle_io(CPUArchState *, uint16_t, void *,
   int, int, int);
+#endif
+
 hvf_slot *hvf_find_overlap_slot(uint64_t, uint64_t);
 
 /* Disable HVF if |disable| is 1, otherwise, enable it iff it is supported by
-- 
2.20.1




[Qemu-devel] [PATCH v1] cpus: move hvf_cpu_synchronize* calls to cpu_synchronize* functions

2019-03-25 Thread Sukrit Bhatnagar
Keep the calls made to synchronize cpu by all hypervisors in one place
inside cpu_synchronize_* functions in include/sysemu/hw_accel.h

Cc: Richard Henderson 
Cc: Paolo Bonzini 
Signed-off-by: Sukrit Bhatnagar 
---
 cpus.c| 12 
 include/sysemu/hw_accel.h |  9 +
 2 files changed, 9 insertions(+), 12 deletions(-)

diff --git a/cpus.c b/cpus.c
index e83f72b48b..026df0dc5f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1021,10 +1021,6 @@ void cpu_synchronize_all_states(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_state(cpu);
-/* TODO: move to cpu_synchronize_state() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_state(cpu);
-}
 }
 }
 
@@ -1034,10 +1030,6 @@ void cpu_synchronize_all_post_reset(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_reset(cpu);
-/* TODO: move to cpu_synchronize_post_reset() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_reset(cpu);
-}
 }
 }
 
@@ -1047,10 +1039,6 @@ void cpu_synchronize_all_post_init(void)
 
 CPU_FOREACH(cpu) {
 cpu_synchronize_post_init(cpu);
-/* TODO: move to cpu_synchronize_post_init() */
-if (hvf_enabled()) {
-hvf_cpu_synchronize_post_init(cpu);
-}
 }
 }
 
diff --git a/include/sysemu/hw_accel.h b/include/sysemu/hw_accel.h
index d2ddfb5ad0..550ca273b9 100644
--- a/include/sysemu/hw_accel.h
+++ b/include/sysemu/hw_accel.h
@@ -27,6 +27,9 @@ static inline void cpu_synchronize_state(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_state(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_state(cpu);
+}
 }
 
 static inline void cpu_synchronize_post_reset(CPUState *cpu)
@@ -40,6 +43,9 @@ static inline void cpu_synchronize_post_reset(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_reset(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_reset(cpu);
+}
 }
 
 static inline void cpu_synchronize_post_init(CPUState *cpu)
@@ -53,6 +59,9 @@ static inline void cpu_synchronize_post_init(CPUState *cpu)
 if (whpx_enabled()) {
 whpx_cpu_synchronize_post_init(cpu);
 }
+if (hvf_enabled()) {
+hvf_cpu_synchronize_post_init(cpu);
+}
 }
 
 static inline void cpu_synchronize_pre_loadvm(CPUState *cpu)
-- 
2.20.1




[Qemu-devel] [PATCH v1 0/1] Convert qdev pointer properties to QOM links

2019-03-18 Thread Sukrit Bhatnagar
Changes made:
- the properties for dma_out and dma_in objects will be added upon
  etraxfs ethernet device initialization
- the qom links for these objects will be added upon the device
  realization and will point to the respective dma controllers
- the qdev properties for them, defined using DEFINE_PROP_PTR and
  set using qdev_prop_set_ptr, are removed


Sukrit Bhatnagar (1):
  etraxfs: convert dma_out and dma_in pointer properties from qdev to
qom links

 hw/net/etraxfs_eth.c  | 13 ++---
 include/hw/cris/etraxfs.h |  4 ++--
 2 files changed, 12 insertions(+), 5 deletions(-)

-- 
2.20.1

Hi,
I am Sukrit, currently pursuing Masters in Computer Science and Engineering
from Indian Institute of Technology Bombay, India. My field of research
includes topics such as using non-volatile memory technologies for the
operating system and using accelerated processing units for offloading
network operations.
I have also studied virtualization techniques as part of my academics, and
am looking forward to working on the same as part of my thesis/project.

In GSoC'18, I had contributed code to libvirt. My final blog for the same
can be found here:
https://skrtbhtngr.github.io/2018/08/13/gsoc18-final-blog.html

I am interested in working on PVRDMA Live Migration for QEMU in GSoC'19.
This patch is submitted towards my application for the same.

The idea for this is taken from the list of BiteSizedTasks at
https://wiki.qemu.org/BiteSizedTasks#Device_models

Reference commits: 873b4d and a8299e.

I have run `make check` and got no errors reported. Not sure how I can
test it out further.

Thanks

PS: I hope this mail is not marked as spam. I am using my local smtp
server to send mail by changing the envelope sender to my gmail id.



[Qemu-devel] [PATCH v1 1/1] etraxfs: convert dma_out and dma_in pointer properties from qdev to qom links

2019-03-18 Thread Sukrit Bhatnagar
The ETRAXFS Ethernet device needs pointers to the dma controllers to
operate. According to qdev-properties.h, properties of pointer types
should be avoided. A QOM link type property is a good substitution.

Cc: Edgar E. Iglesias 

Signed-off-by: Sukrit Bhatnagar 
---
 hw/net/etraxfs_eth.c  | 13 ++---
 include/hw/cris/etraxfs.h |  4 ++--
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/hw/net/etraxfs_eth.c b/hw/net/etraxfs_eth.c
index 36855804db..6df109069f 100644
--- a/hw/net/etraxfs_eth.c
+++ b/hw/net/etraxfs_eth.c
@@ -622,6 +622,16 @@ static void etraxfs_eth_realize(DeviceState *dev, Error 
**errp)
   "etraxfs-eth", 0x5c);
 sysbus_init_mmio(sbd, >mmio);
 
+object_property_add_link(OBJECT(dev), "dma_out", TYPE_ETRAX_FS_ETH,
+ (Object **) >dma_out,
+ qdev_prop_allow_set_link_before_realize,
+ 0, _abort);
+
+object_property_add_link(OBJECT(dev), "dma_in", TYPE_ETRAX_FS_ETH,
+ (Object **) >dma_in,
+ qdev_prop_allow_set_link_before_realize,
+ 0, _abort);
+
 qemu_macaddr_default_if_unset(>conf.macaddr);
 s->nic = qemu_new_nic(_etraxfs_info, >conf,
   object_get_typename(OBJECT(s)), dev->id, s);
@@ -634,8 +644,6 @@ static void etraxfs_eth_realize(DeviceState *dev, Error 
**errp)
 
 static Property etraxfs_eth_properties[] = {
 DEFINE_PROP_UINT32("phyaddr", ETRAXFSEthState, phyaddr, 1),
-DEFINE_PROP_PTR("dma_out", ETRAXFSEthState, vdma_out),
-DEFINE_PROP_PTR("dma_in", ETRAXFSEthState, vdma_in),
 DEFINE_NIC_PROPERTIES(ETRAXFSEthState, conf),
 DEFINE_PROP_END_OF_LIST(),
 };
@@ -647,7 +655,6 @@ static void etraxfs_eth_class_init(ObjectClass *klass, void 
*data)
 dc->realize = etraxfs_eth_realize;
 dc->reset = etraxfs_eth_reset;
 dc->props = etraxfs_eth_properties;
-/* Reason: pointer properties "dma_out", "dma_in" */
 dc->user_creatable = false;
 }
 
diff --git a/include/hw/cris/etraxfs.h b/include/hw/cris/etraxfs.h
index 8da965addb..f686cee180 100644
--- a/include/hw/cris/etraxfs.h
+++ b/include/hw/cris/etraxfs.h
@@ -39,8 +39,8 @@ etraxfs_eth_init(NICInfo *nd, hwaddr base, int phyaddr,
 dev = qdev_create(NULL, "etraxfs-eth");
 qdev_set_nic_properties(dev, nd);
 qdev_prop_set_uint32(dev, "phyaddr", phyaddr);
-qdev_prop_set_ptr(dev, "dma_out", dma_out);
-qdev_prop_set_ptr(dev, "dma_in", dma_in);
+object_property_set_link(OBJECT(dev), OBJECT(dma_out), "dma_out", NULL);
+object_property_set_link(OBJECT(dev), OBJECT(dma_in), "dma_in", NULL);
 qdev_init_nofail(dev);
 sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
 return dev;
-- 
2.20.1