QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration
@ 2019-04-08  1:56 Catherine Ho
  2019-04-08  1:56 ` Catherine Ho
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Catherine Ho @ 2019-04-08  1:56 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert
  Cc: Markus Armbruster, Juan Quintela, qemu-devel, Peter Xu, Catherine Ho

Commit 18269069c310 ("migration: Introduce ignore-shared capability")
addes ignore-shared capability to bypass the shared ramblock (e,g,
membackend + numa node). It does good to live migration.

As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
until VM starts, but it does on aarch64 qemu:
Backtrace:
1  0x000055f4a296dd84 in address_space_write_rom_internal () at
exec.c:3458
2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
6  0x000055f4a2c9851d in main () at vl.c:4552

Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
druing rom_reset. In ignore-shared incoming case, this rom filling
is not required since all the data has been stored in memory backend
file.

Further more, as suggested by Peter Xu, if we do rom_reset() now with
these ROMs then the RAM data should be re-filled again too with the
migration stream coming in.

Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
capability")
Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
 hw/core/loader.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index fe5cb24122..946bb8ff00 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1087,6 +1087,13 @@ static void rom_reset(void *unused)
 {
     Rom *rom;
 
+    /*
+     * If we do rom_reset() now with these ROMs then the RAM data should be
+     * re-filled again too with the migration stream coming in.
+     */
+    if (runstate_check(RUN_STATE_INMIGRATE))
+        return;
+
     QTAILQ_FOREACH(rom, &roms, next) {
         if (rom->fw_file) {
             continue;
-- 
2.17.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration
  2019-04-08  1:56 [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration Catherine Ho
@ 2019-04-08  1:56 ` Catherine Ho
  2019-04-08  3:31 ` Peter Xu
  2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
  2 siblings, 0 replies; 14+ messages in thread
From: Catherine Ho @ 2019-04-08  1:56 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert
  Cc: qemu-devel, Catherine Ho, Markus Armbruster, Peter Xu, Juan Quintela

Commit 18269069c310 ("migration: Introduce ignore-shared capability")
addes ignore-shared capability to bypass the shared ramblock (e,g,
membackend + numa node). It does good to live migration.

As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
until VM starts, but it does on aarch64 qemu:
Backtrace:
1  0x000055f4a296dd84 in address_space_write_rom_internal () at
exec.c:3458
2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
6  0x000055f4a2c9851d in main () at vl.c:4552

Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
druing rom_reset. In ignore-shared incoming case, this rom filling
is not required since all the data has been stored in memory backend
file.

Further more, as suggested by Peter Xu, if we do rom_reset() now with
these ROMs then the RAM data should be re-filled again too with the
migration stream coming in.

Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
capability")
Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
 hw/core/loader.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index fe5cb24122..946bb8ff00 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1087,6 +1087,13 @@ static void rom_reset(void *unused)
 {
     Rom *rom;
 
+    /*
+     * If we do rom_reset() now with these ROMs then the RAM data should be
+     * re-filled again too with the migration stream coming in.
+     */
+    if (runstate_check(RUN_STATE_INMIGRATE))
+        return;
+
     QTAILQ_FOREACH(rom, &roms, next) {
         if (rom->fw_file) {
             continue;
-- 
2.17.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration
  2019-04-08  1:56 [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration Catherine Ho
  2019-04-08  1:56 ` Catherine Ho
@ 2019-04-08  3:31 ` Peter Xu
  2019-04-08  3:31   ` Peter Xu
  2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
  2 siblings, 1 reply; 14+ messages in thread
From: Peter Xu @ 2019-04-08  3:31 UTC (permalink / raw)
  To: Catherine Ho
  Cc: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert,
	Markus Armbruster, Juan Quintela, qemu-devel

On Sun, Apr 07, 2019 at 09:56:56PM -0400, Catherine Ho wrote:
> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
> 
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
> 
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
> 
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
> 
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  hw/core/loader.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index fe5cb24122..946bb8ff00 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1087,6 +1087,13 @@ static void rom_reset(void *unused)
>  {
>      Rom *rom;
>  
> +    /*
> +     * If we do rom_reset() now with these ROMs then the RAM data should be
> +     * re-filled again too with the migration stream coming in.

I'm unconfident about correcting English in patches, but it does look
a bit odd to me...  I would say:

  We don't need to fill in the RAM with ROM data because we'll fill
  the data in during the next incoming migration in all cases.  Note
  that some of those RAMs can actually be modified by the guest on ARM
  so this is probably the only right thing to do here.

> +     */
> +    if (runstate_check(RUN_STATE_INMIGRATE))
> +        return;

The change looks good to me but of course it'll be nicer if some other
people can review it.

> +
>      QTAILQ_FOREACH(rom, &roms, next) {
>          if (rom->fw_file) {
>              continue;
> -- 
> 2.17.1
> 

Regards,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration
  2019-04-08  3:31 ` Peter Xu
@ 2019-04-08  3:31   ` Peter Xu
  0 siblings, 0 replies; 14+ messages in thread
From: Peter Xu @ 2019-04-08  3:31 UTC (permalink / raw)
  To: Catherine Ho
  Cc: Juan Quintela, Markus Armbruster, qemu-devel,
	Dr. David Alan Gilbert, Paolo Bonzini, Richard Henderson

On Sun, Apr 07, 2019 at 09:56:56PM -0400, Catherine Ho wrote:
> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
> 
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
> 
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
> 
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
> 
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  hw/core/loader.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index fe5cb24122..946bb8ff00 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1087,6 +1087,13 @@ static void rom_reset(void *unused)
>  {
>      Rom *rom;
>  
> +    /*
> +     * If we do rom_reset() now with these ROMs then the RAM data should be
> +     * re-filled again too with the migration stream coming in.

I'm unconfident about correcting English in patches, but it does look
a bit odd to me...  I would say:

  We don't need to fill in the RAM with ROM data because we'll fill
  the data in during the next incoming migration in all cases.  Note
  that some of those RAMs can actually be modified by the guest on ARM
  so this is probably the only right thing to do here.

> +     */
> +    if (runstate_check(RUN_STATE_INMIGRATE))
> +        return;

The change looks good to me but of course it'll be nicer if some other
people can review it.

> +
>      QTAILQ_FOREACH(rom, &roms, next) {
>          if (rom->fw_file) {
>              continue;
> -- 
> 2.17.1
> 

Regards,

-- 
Peter Xu


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-08  1:56 [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration Catherine Ho
  2019-04-08  1:56 ` Catherine Ho
  2019-04-08  3:31 ` Peter Xu
@ 2019-04-08  8:42 ` " Catherine Ho
  2019-04-08  8:42   ` Catherine Ho
                     ` (3 more replies)
  2 siblings, 4 replies; 14+ messages in thread
From: Catherine Ho @ 2019-04-08  8:42 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert
  Cc: Markus Armbruster, Juan Quintela, qemu-devel, Peter Xu, Catherine Ho

Commit 18269069c310 ("migration: Introduce ignore-shared capability")
addes ignore-shared capability to bypass the shared ramblock (e,g,
membackend + numa node). It does good to live migration.

As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
until VM starts, but it does on aarch64 qemu:
Backtrace:
1  0x000055f4a296dd84 in address_space_write_rom_internal () at
exec.c:3458
2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
6  0x000055f4a2c9851d in main () at vl.c:4552

Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
druing rom_reset. In ignore-shared incoming case, this rom filling
is not required since all the data has been stored in memory backend
file.

Further more, as suggested by Peter Xu, if we do rom_reset() now with
these ROMs then the RAM data should be re-filled again too with the
migration stream coming in.

Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
capability")
Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
 hw/core/loader.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index fe5cb24122..040109464b 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
 {
     Rom *rom;
 
+    /*
+     * We don't need to fill in the RAM with ROM data because we'll fill
+     * the data in during the next incoming migration in all cases.  Note
+     * that some of those RAMs can actually be modified by the guest on ARM
+     * so this is probably the only right thing to do here.
+     */
+    if (runstate_check(RUN_STATE_INMIGRATE))
+        return;
+
     QTAILQ_FOREACH(rom, &roms, next) {
         if (rom->fw_file) {
             continue;
-- 
2.17.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
@ 2019-04-08  8:42   ` Catherine Ho
  2019-04-16  1:46   ` Catherine Ho
                     ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Catherine Ho @ 2019-04-08  8:42 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert
  Cc: qemu-devel, Catherine Ho, Markus Armbruster, Peter Xu, Juan Quintela

Commit 18269069c310 ("migration: Introduce ignore-shared capability")
addes ignore-shared capability to bypass the shared ramblock (e,g,
membackend + numa node). It does good to live migration.

As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
until VM starts, but it does on aarch64 qemu:
Backtrace:
1  0x000055f4a296dd84 in address_space_write_rom_internal () at
exec.c:3458
2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
6  0x000055f4a2c9851d in main () at vl.c:4552

Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
druing rom_reset. In ignore-shared incoming case, this rom filling
is not required since all the data has been stored in memory backend
file.

Further more, as suggested by Peter Xu, if we do rom_reset() now with
these ROMs then the RAM data should be re-filled again too with the
migration stream coming in.

Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
capability")
Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
---
 hw/core/loader.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index fe5cb24122..040109464b 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
 {
     Rom *rom;
 
+    /*
+     * We don't need to fill in the RAM with ROM data because we'll fill
+     * the data in during the next incoming migration in all cases.  Note
+     * that some of those RAMs can actually be modified by the guest on ARM
+     * so this is probably the only right thing to do here.
+     */
+    if (runstate_check(RUN_STATE_INMIGRATE))
+        return;
+
     QTAILQ_FOREACH(rom, &roms, next) {
         if (rom->fw_file) {
             continue;
-- 
2.17.1



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
  2019-04-08  8:42   ` Catherine Ho
@ 2019-04-16  1:46   ` Catherine Ho
  2019-04-16  1:46     ` Catherine Ho
  2019-04-16  2:51   ` Peter Xu
  2019-06-05 18:31   ` Dr. David Alan Gilbert
  3 siblings, 1 reply; 14+ messages in thread
From: Catherine Ho @ 2019-04-16  1:46 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert
  Cc: Markus Armbruster, Juan Quintela, QEMU Developers, Peter Xu

Ping, thanks

B.R.
Catherine

On Mon, 8 Apr 2019 at 16:43, Catherine Ho <catherine.hecx@gmail.com> wrote:

> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
>
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
>
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
>
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
>
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  hw/core/loader.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index fe5cb24122..040109464b 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
>  {
>      Rom *rom;
>
> +    /*
> +     * We don't need to fill in the RAM with ROM data because we'll fill
> +     * the data in during the next incoming migration in all cases.  Note
> +     * that some of those RAMs can actually be modified by the guest on
> ARM
> +     * so this is probably the only right thing to do here.
> +     */
> +    if (runstate_check(RUN_STATE_INMIGRATE))
> +        return;
> +
>      QTAILQ_FOREACH(rom, &roms, next) {
>          if (rom->fw_file) {
>              continue;
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-16  1:46   ` Catherine Ho
@ 2019-04-16  1:46     ` Catherine Ho
  0 siblings, 0 replies; 14+ messages in thread
From: Catherine Ho @ 2019-04-16  1:46 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert
  Cc: QEMU Developers, Markus Armbruster, Peter Xu, Juan Quintela

Ping, thanks

B.R.
Catherine

On Mon, 8 Apr 2019 at 16:43, Catherine Ho <catherine.hecx@gmail.com> wrote:

> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
>
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
>
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
>
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
>
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  hw/core/loader.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index fe5cb24122..040109464b 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
>  {
>      Rom *rom;
>
> +    /*
> +     * We don't need to fill in the RAM with ROM data because we'll fill
> +     * the data in during the next incoming migration in all cases.  Note
> +     * that some of those RAMs can actually be modified by the guest on
> ARM
> +     * so this is probably the only right thing to do here.
> +     */
> +    if (runstate_check(RUN_STATE_INMIGRATE))
> +        return;
> +
>      QTAILQ_FOREACH(rom, &roms, next) {
>          if (rom->fw_file) {
>              continue;
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
  2019-04-08  8:42   ` Catherine Ho
  2019-04-16  1:46   ` Catherine Ho
@ 2019-04-16  2:51   ` Peter Xu
  2019-04-16  2:51     ` Peter Xu
  2019-05-13  3:00     ` Catherine Ho
  2019-06-05 18:31   ` Dr. David Alan Gilbert
  3 siblings, 2 replies; 14+ messages in thread
From: Peter Xu @ 2019-04-16  2:51 UTC (permalink / raw)
  To: Catherine Ho
  Cc: Paolo Bonzini, Richard Henderson, Dr. David Alan Gilbert,
	Markus Armbruster, Juan Quintela, qemu-devel

On Mon, Apr 08, 2019 at 04:42:13AM -0400, Catherine Ho wrote:
> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
> 
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
> 
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
> 
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
> 
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>

Acked-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-16  2:51   ` Peter Xu
@ 2019-04-16  2:51     ` Peter Xu
  2019-05-13  3:00     ` Catherine Ho
  1 sibling, 0 replies; 14+ messages in thread
From: Peter Xu @ 2019-04-16  2:51 UTC (permalink / raw)
  To: Catherine Ho
  Cc: Juan Quintela, Markus Armbruster, qemu-devel,
	Dr. David Alan Gilbert, Paolo Bonzini, Richard Henderson

On Mon, Apr 08, 2019 at 04:42:13AM -0400, Catherine Ho wrote:
> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
> 
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
> 
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
> 
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
> 
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>

Acked-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-16  2:51   ` Peter Xu
  2019-04-16  2:51     ` Peter Xu
@ 2019-05-13  3:00     ` Catherine Ho
  1 sibling, 0 replies; 14+ messages in thread
From: Catherine Ho @ 2019-05-13  3:00 UTC (permalink / raw)
  To: Peter Xu
  Cc: Juan Quintela, Markus Armbruster, QEMU Developers,
	Dr. David Alan Gilbert, Paolo Bonzini, Richard Henderson

Sorry for the noise, is there any more comment for this patch?
Without this patch, ignore shared capabilities can not be used on arm64

B.R.
Catherine

On Tue, 16 Apr 2019 at 10:51, Peter Xu <peterx@redhat.com> wrote:

> On Mon, Apr 08, 2019 at 04:42:13AM -0400, Catherine Ho wrote:
> > Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> > addes ignore-shared capability to bypass the shared ramblock (e,g,
> > membackend + numa node). It does good to live migration.
> >
> > As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> > until VM starts, but it does on aarch64 qemu:
> > Backtrace:
> > 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> > exec.c:3458
> > 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> > 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> > 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> > 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> > 6  0x000055f4a2c9851d in main () at vl.c:4552
> >
> > Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> > druing rom_reset. In ignore-shared incoming case, this rom filling
> > is not required since all the data has been stored in memory backend
> > file.
> >
> > Further more, as suggested by Peter Xu, if we do rom_reset() now with
> > these ROMs then the RAM data should be re-filled again too with the
> > migration stream coming in.
> >
> > Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> > capability")
> > Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> > Suggested-by: Peter Xu <peterx@redhat.com>
> > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
>
> Acked-by: Peter Xu <peterx@redhat.com>
>
> --
> Peter Xu
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
                     ` (2 preceding siblings ...)
  2019-04-16  2:51   ` Peter Xu
@ 2019-06-05 18:31   ` Dr. David Alan Gilbert
  2019-08-14 10:40     ` Catherine Ho
  3 siblings, 1 reply; 14+ messages in thread
From: Dr. David Alan Gilbert @ 2019-06-05 18:31 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Juan Quintela, qemu-devel, Peter Xu, Markus Armbruster,
	Catherine Ho, Richard Henderson

Paolo, can you take this one please.

* Catherine Ho (catherine.hecx@gmail.com) wrote:
> Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> addes ignore-shared capability to bypass the shared ramblock (e,g,
> membackend + numa node). It does good to live migration.
> 
> As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> until VM starts, but it does on aarch64 qemu:
> Backtrace:
> 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> exec.c:3458
> 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> 6  0x000055f4a2c9851d in main () at vl.c:4552
> 
> Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> druing rom_reset. In ignore-shared incoming case, this rom filling
> is not required since all the data has been stored in memory backend
> file.
> 
> Further more, as suggested by Peter Xu, if we do rom_reset() now with
> these ROMs then the RAM data should be re-filled again too with the
> migration stream coming in.
> 
> Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> capability")
> Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> Suggested-by: Peter Xu <peterx@redhat.com>
> Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> ---
>  hw/core/loader.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index fe5cb24122..040109464b 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
>  {
>      Rom *rom;
>  
> +    /*
> +     * We don't need to fill in the RAM with ROM data because we'll fill
> +     * the data in during the next incoming migration in all cases.  Note
> +     * that some of those RAMs can actually be modified by the guest on ARM
> +     * so this is probably the only right thing to do here.
> +     */
> +    if (runstate_check(RUN_STATE_INMIGRATE))
> +        return;
> +
>      QTAILQ_FOREACH(rom, &roms, next) {
>          if (rom->fw_file) {
>              continue;
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-06-05 18:31   ` Dr. David Alan Gilbert
@ 2019-08-14 10:40     ` Catherine Ho
  2019-08-14 12:34       ` Paolo Bonzini
  0 siblings, 1 reply; 14+ messages in thread
From: Catherine Ho @ 2019-08-14 10:40 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Juan Quintela, Markus Armbruster, QEMU Developers, Peter Xu,
	Dr. David Alan Gilbert, Richard Henderson

Hi Paolo
Ping, is any other comment I hadn't addressed?

Cheers
Catherine

On Thu, 6 Jun 2019 at 02:31, Dr. David Alan Gilbert <dgilbert@redhat.com>
wrote:

> Paolo, can you take this one please.
>
> * Catherine Ho (catherine.hecx@gmail.com) wrote:
> > Commit 18269069c310 ("migration: Introduce ignore-shared capability")
> > addes ignore-shared capability to bypass the shared ramblock (e,g,
> > membackend + numa node). It does good to live migration.
> >
> > As told by Yury,this commit expectes that QEMU doesn't write to guest RAM
> > until VM starts, but it does on aarch64 qemu:
> > Backtrace:
> > 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
> > exec.c:3458
> > 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
> > 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
> > 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
> > 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
> > 6  0x000055f4a2c9851d in main () at vl.c:4552
> >
> > Actually, on arm64 virt marchine, ramblock "dtb" will be filled into ram
> > druing rom_reset. In ignore-shared incoming case, this rom filling
> > is not required since all the data has been stored in memory backend
> > file.
> >
> > Further more, as suggested by Peter Xu, if we do rom_reset() now with
> > these ROMs then the RAM data should be re-filled again too with the
> > migration stream coming in.
> >
> > Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
> > capability")
> > Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru>
> > Suggested-by: Peter Xu <peterx@redhat.com>
> > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com>
> > ---
> >  hw/core/loader.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/hw/core/loader.c b/hw/core/loader.c
> > index fe5cb24122..040109464b 100644
> > --- a/hw/core/loader.c
> > +++ b/hw/core/loader.c
> > @@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
> >  {
> >      Rom *rom;
> >
> > +    /*
> > +     * We don't need to fill in the RAM with ROM data because we'll fill
> > +     * the data in during the next incoming migration in all cases.
> Note
> > +     * that some of those RAMs can actually be modified by the guest on
> ARM
> > +     * so this is probably the only right thing to do here.
> > +     */
> > +    if (runstate_check(RUN_STATE_INMIGRATE))
> > +        return;
> > +
> >      QTAILQ_FOREACH(rom, &roms, next) {
> >          if (rom->fw_file) {
> >              continue;
> > --
> > 2.17.1
> >
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Qemu-devel] [PATCH v4] migration: do not rom_reset() during incoming migration
  2019-08-14 10:40     ` Catherine Ho
@ 2019-08-14 12:34       ` Paolo Bonzini
  0 siblings, 0 replies; 14+ messages in thread
From: Paolo Bonzini @ 2019-08-14 12:34 UTC (permalink / raw)
  To: Catherine Ho
  Cc: Juan Quintela, Markus Armbruster, QEMU Developers, Peter Xu,
	Dr. David Alan Gilbert, Richard Henderson

On 14/08/19 12:40, Catherine Ho wrote:
> Hi Paolo
> Ping, is any other comment I hadn't addressed?

No, I queued the patch now.

Paolo

> Cheers
> Catherine
> 
> On Thu, 6 Jun 2019 at 02:31, Dr. David Alan Gilbert <dgilbert@redhat.com
> <mailto:dgilbert@redhat.com>> wrote:
> 
>     Paolo, can you take this one please.
> 
>     * Catherine Ho (catherine.hecx@gmail.com
>     <mailto:catherine.hecx@gmail.com>) wrote:
>     > Commit 18269069c310 ("migration: Introduce ignore-shared capability")
>     > addes ignore-shared capability to bypass the shared ramblock (e,g,
>     > membackend + numa node). It does good to live migration.
>     >
>     > As told by Yury,this commit expectes that QEMU doesn't write to
>     guest RAM
>     > until VM starts, but it does on aarch64 qemu:
>     > Backtrace:
>     > 1  0x000055f4a296dd84 in address_space_write_rom_internal () at
>     > exec.c:3458
>     > 2  0x000055f4a296de3a in address_space_write_rom () at exec.c:3479
>     > 3  0x000055f4a2d519ff in rom_reset () at hw/core/loader.c:1101
>     > 4  0x000055f4a2d475ec in qemu_devices_reset () at hw/core/reset.c:69
>     > 5  0x000055f4a2c90a28 in qemu_system_reset () at vl.c:1675
>     > 6  0x000055f4a2c9851d in main () at vl.c:4552
>     >
>     > Actually, on arm64 virt marchine, ramblock "dtb" will be filled
>     into ram
>     > druing rom_reset. In ignore-shared incoming case, this rom filling
>     > is not required since all the data has been stored in memory backend
>     > file.
>     >
>     > Further more, as suggested by Peter Xu, if we do rom_reset() now with
>     > these ROMs then the RAM data should be re-filled again too with the
>     > migration stream coming in.
>     >
>     > Fixes: commit 18269069c310 ("migration: Introduce ignore-shared
>     > capability")
>     > Suggested-by: Yury Kotov <yury-kotov@yandex-team.ru
>     <mailto:yury-kotov@yandex-team.ru>>
>     > Suggested-by: Peter Xu <peterx@redhat.com <mailto:peterx@redhat.com>>
>     > Signed-off-by: Catherine Ho <catherine.hecx@gmail.com
>     <mailto:catherine.hecx@gmail.com>>
>     > ---
>     >  hw/core/loader.c | 9 +++++++++
>     >  1 file changed, 9 insertions(+)
>     >
>     > diff --git a/hw/core/loader.c b/hw/core/loader.c
>     > index fe5cb24122..040109464b 100644
>     > --- a/hw/core/loader.c
>     > +++ b/hw/core/loader.c
>     > @@ -1087,6 +1087,15 @@ static void rom_reset(void *unused)
>     >  {
>     >      Rom *rom;
>     > 
>     > +    /*
>     > +     * We don't need to fill in the RAM with ROM data because
>     we'll fill
>     > +     * the data in during the next incoming migration in all
>     cases.  Note
>     > +     * that some of those RAMs can actually be modified by the
>     guest on ARM
>     > +     * so this is probably the only right thing to do here.
>     > +     */
>     > +    if (runstate_check(RUN_STATE_INMIGRATE))
>     > +        return;
>     > +
>     >      QTAILQ_FOREACH(rom, &roms, next) {
>     >          if (rom->fw_file) {
>     >              continue;
>     > --
>     > 2.17.1
>     >
>     --
>     Dr. David Alan Gilbert / dgilbert@redhat.com
>     <mailto:dgilbert@redhat.com> / Manchester, UK
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-08  1:56 [Qemu-devel] [PATCH v3] migration: do not rom_reset() during incoming migration Catherine Ho
2019-04-08  1:56 ` Catherine Ho
2019-04-08  3:31 ` Peter Xu
2019-04-08  3:31   ` Peter Xu
2019-04-08  8:42 ` [Qemu-devel] [PATCH v4] " Catherine Ho
2019-04-08  8:42   ` Catherine Ho
2019-04-16  1:46   ` Catherine Ho
2019-04-16  1:46     ` Catherine Ho
2019-04-16  2:51   ` Peter Xu
2019-04-16  2:51     ` Peter Xu
2019-05-13  3:00     ` Catherine Ho
2019-06-05 18:31   ` Dr. David Alan Gilbert
2019-08-14 10:40     ` Catherine Ho
2019-08-14 12:34       ` Paolo Bonzini

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox