[PATCH v2 3/3] savevm: check RAM is pagesize aligned

Marc-André Lureau posted 3 patches 6 years, 1 month ago
Maintainers: Juan Quintela <quintela@redhat.com>, Richard Henderson <rth@twiddle.net>, "Dr. David Alan Gilbert" <dgilbert@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, Stefan Berger <stefanb@linux.ibm.com>
[PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Marc-André Lureau 6 years, 1 month ago
Check the host pointer is correctly aligned, otherwise we may fail
during migration in ram_block_discard_range().

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 migration/savevm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index a71b930b91..bbb7e89682 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2910,6 +2910,11 @@ err_drain:
 
 void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
 {
+    RAMBlock *rb = mr->ram_block;
+
+    assert(QEMU_PTR_IS_ALIGNED(qemu_ram_get_host_addr(rb),
+                               qemu_ram_pagesize(rb)));
+
     qemu_ram_set_idstr(mr->ram_block,
                        memory_region_name(mr), dev);
     qemu_ram_set_migratable(mr->ram_block);
-- 
2.24.0.308.g228f53135a


Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Philippe Mathieu-Daudé 6 years, 1 month ago
On 1/3/20 8:40 AM, Marc-André Lureau wrote:
> Check the host pointer is correctly aligned, otherwise we may fail
> during migration in ram_block_discard_range().
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>   migration/savevm.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index a71b930b91..bbb7e89682 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2910,6 +2910,11 @@ err_drain:
>   
>   void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
>   {
> +    RAMBlock *rb = mr->ram_block;
> +
> +    assert(QEMU_PTR_IS_ALIGNED(qemu_ram_get_host_addr(rb),
> +                               qemu_ram_pagesize(rb)));
> +
>       qemu_ram_set_idstr(mr->ram_block,
>                          memory_region_name(mr), dev);
>       qemu_ram_set_migratable(mr->ram_block);
> 


Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Juan Quintela 6 years, 1 month ago
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
n> Check the host pointer is correctly aligned, otherwise we may fail
> during migration in ram_block_discard_range().
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Juan Quintela <quintela@redhat.com>

queued


Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Marc-André Lureau 5 years, 11 months ago
Hi Juan

On Wed, Jan 8, 2020 at 2:08 PM Juan Quintela <quintela@redhat.com> wrote:
>
> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> n> Check the host pointer is correctly aligned, otherwise we may fail
> > during migration in ram_block_discard_range().
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Reviewed-by: Juan Quintela <quintela@redhat.com>
>
> queued
>

Did it get lost? thanks


-- 
Marc-André Lureau

Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Juan Quintela 5 years, 11 months ago
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> Hi Juan
>
> On Wed, Jan 8, 2020 at 2:08 PM Juan Quintela <quintela@redhat.com> wrote:
>>
>> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>> n> Check the host pointer is correctly aligned, otherwise we may fail
>> > during migration in ram_block_discard_range().
>> >
>> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Reviewed-by: Juan Quintela <quintela@redhat.com>
>>
>> queued
>>
>
> Did it get lost? thanks

I dropped it in the past, because it made "make check" for mips fail.
(I put it on my ToDo list to investigate and forgot about it)

But now it pass, go figure.

Included again.  Sorry.

Later, Juan.


Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Aleksandar Markovic 5 years, 11 months ago
On Thursday, February 27, 2020, Juan Quintela <quintela@redhat.com> wrote:

> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> > Hi Juan
> >
> > On Wed, Jan 8, 2020 at 2:08 PM Juan Quintela <quintela@redhat.com>
> wrote:
> >>
> >> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >> n> Check the host pointer is correctly aligned, otherwise we may fail
> >> > during migration in ram_block_discard_range().
> >> >
> >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> Reviewed-by: Juan Quintela <quintela@redhat.com>
> >>
> >> queued
> >>
> >
> > Did it get lost? thanks
>
> I dropped it in the past, because it made "make check" for mips fail.
> (I put it on my ToDo list to investigate and forgot about it)
>
>
Thank you for caring for mips.

Do you perhaps remember what was tgevtest and environment for the failing
test?

Regards,
Aleksandar


> But now it pass, go figure.
>
> Included again.  Sorry.
>
> Later, Juan.
>
>
>
Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Juan Quintela 5 years, 11 months ago
Aleksandar Markovic <aleksandar.m.mail@gmail.com> wrote:
> On Thursday, February 27, 2020, Juan Quintela <quintela@redhat.com> wrote:
>
>  Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>  > Hi Juan
>  >
>  > On Wed, Jan 8, 2020 at 2:08 PM Juan Quintela <quintela@redhat.com> wrote:
>  >>
>  >> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>  >> n> Check the host pointer is correctly aligned, otherwise we may fail
>  >> > during migration in ram_block_discard_range().
>  >> >
>  >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>  >>
>  >> Reviewed-by: Juan Quintela <quintela@redhat.com>
>  >>
>  >> queued
>  >>
>  >
>  > Did it get lost? thanks
>
>  I dropped it in the past, because it made "make check" for mips fail.
>  (I put it on my ToDo list to investigate and forgot about it)
>
> Thank you for caring for mips.

You are welcome.
But you need to thank "make check"
It didn't pass.

> Do you perhaps remember what was tgevtest and environment for the failing test?

It was plain "make check" with everything under the sun compiled in.
Clearly it was other of the patches, or an interaction between them what
failed.
I don't remember the error, sorry.  I droped the patch to my ToDo list
of things to investigate (the patch was "obviously" correct) and forgot
about it.

Later, Juan.


Re: [PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Juan Quintela 5 years, 11 months ago
Aleksandar Markovic <aleksandar.m.mail@gmail.com> wrote:
> On Thursday, February 27, 2020, Juan Quintela <quintela@redhat.com> wrote:
>
>  Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>  > Hi Juan
>  >
>  > On Wed, Jan 8, 2020 at 2:08 PM Juan Quintela <quintela@redhat.com> wrote:
>  >>
>  >> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
>  >> n> Check the host pointer is correctly aligned, otherwise we may fail
>  >> > during migration in ram_block_discard_range().
>  >> >
>  >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>  >>
>  >> Reviewed-by: Juan Quintela <quintela@redhat.com>
>  >>
>  >> queued
>  >>
>  >
>  > Did it get lost? thanks
>
>  I dropped it in the past, because it made "make check" for mips fail.
>  (I put it on my ToDo list to investigate and forgot about it)
>
> Thank you for caring for mips.
>
> Do you perhaps remember what was tgevtest and environment for the failing test?


And here we are again.
I only compile on an x86 32bit host when I am going to do a pull
request.

qemu-system-mips64el: /mnt/code/qemu/full/migration/savevm.c:2923: vmstate_register_ram: Assertion `QEMU_PTR_IS_ALIGNED(qemu_ram
_get_host_addr(rb), qemu_ram_pagesize(rb))' failed.
Broken pipe
/mnt/code/qemu/full/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)
  TEST    check-qtest-aarch64: tests/qtest/qom-test
ERROR - too few tests run (expected 4, got 0)
make: *** [/mnt/code/qemu/full/tests/Makefile.include:632: check-qtest-mips64el] Error 1
make: *** Waiting for unfinished jobs....


As you can see, this is mips tcg running in a 32bit host.

$ export QTEST_QEMU_BINARY=./mips64el-softmmu/qemu-system-mips64el 
$ ./tests/qtest/qom-test
/mips64el/qom/pica61: OK
/mips64el/qom/mipssim: OK
/mips64el/qom/mips: OK
/mips64el/qom/fulong2e: OK
/mips64el/qom/malta: OK
/mips64el/qom/boston: OK
/mips64el/qom/none: OK
/mips64el/qom/magnum: qemu-system-mips64el: /mnt/code/qemu/full/migration/savevm.c:2923: vmstate_register_ram: Assertion `QEMU_PTR_IS_ALIGNED(qemu_ram_get_host_addr(rb), qemu_ram_pagesize(rb))' failed.
Broken pipe
/mnt/code/qemu/full/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU death from signal 6 (Aborted) (core dumped)
Aborted (core dumped)
$ 

Can you take a look at this?

mips64-softmmu also fails on the same place, mips[el]-softmmu passes,
but they don't use magnum.

Code is supposed to be right, I will expect that the problem is in the
magnum board, but this is qemu + mips + migration.  Anything can happen.

Marc, I have to drop it again.

Later, Juan.


[PATCH v2 3/3] savevm: check RAM is pagesize aligned
Posted by Aleksandar Markovic 5 years, 11 months ago
just cc-ing Herve and Philippe, related to Magnum machine support (this is
one of so-called Jazz mips machines)

On Friday, February 28, 2020, Juan Quintela <quintela@redhat.com> wrote:

> Aleksandar Markovic <aleksandar.m.mail@gmail.com> wrote:
> > On Thursday, February 27, 2020, Juan Quintela <quintela@redhat.com>
> wrote:
> >
> >  Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> >  > Hi Juan
> >  >
> >  > On Wed, Jan 8, 2020 at 2:08 PM Juan Quintela <quintela@redhat.com>
> wrote:
> >  >>
> >  >> Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> >  >> n> Check the host pointer is correctly aligned, otherwise we may fail
> >  >> > during migration in ram_block_discard_range().
> >  >> >
> >  >> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >  >>
> >  >> Reviewed-by: Juan Quintela <quintela@redhat.com>
> >  >>
> >  >> queued
> >  >>
> >  >
> >  > Did it get lost? thanks
> >
> >  I dropped it in the past, because it made "make check" for mips fail.
> >  (I put it on my ToDo list to investigate and forgot about it)
> >
> > Thank you for caring for mips.
> >
> > Do you perhaps remember what was tgevtest and environment for the
> failing test?
>
>
> And here we are again.
> I only compile on an x86 32bit host when I am going to do a pull
> request.
>
> qemu-system-mips64el: /mnt/code/qemu/full/migration/savevm.c:2923:
> vmstate_register_ram: Assertion `QEMU_PTR_IS_ALIGNED(qemu_ram
> _get_host_addr(rb), qemu_ram_pagesize(rb))' failed.
> Broken pipe
> /mnt/code/qemu/full/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU
> death from signal 6 (Aborted) (core dumped)
>   TEST    check-qtest-aarch64: tests/qtest/qom-test
> ERROR - too few tests run (expected 4, got 0)
> make: *** [/mnt/code/qemu/full/tests/Makefile.include:632:
> check-qtest-mips64el] Error 1
> make: *** Waiting for unfinished jobs....
>
>
> As you can see, this is mips tcg running in a 32bit host.
>
> $ export QTEST_QEMU_BINARY=./mips64el-softmmu/qemu-system-mips64el
> $ ./tests/qtest/qom-test
> /mips64el/qom/pica61: OK
> /mips64el/qom/mipssim: OK
> /mips64el/qom/mips: OK
> /mips64el/qom/fulong2e: OK
> /mips64el/qom/malta: OK
> /mips64el/qom/boston: OK
> /mips64el/qom/none: OK
> /mips64el/qom/magnum: qemu-system-mips64el: /mnt/code/qemu/full/migration/savevm.c:2923:
> vmstate_register_ram: Assertion `QEMU_PTR_IS_ALIGNED(qemu_ram_get_host_addr(rb),
> qemu_ram_pagesize(rb))' failed.
> Broken pipe
> /mnt/code/qemu/full/tests/qtest/libqtest.c:175: kill_qemu() detected QEMU
> death from signal 6 (Aborted) (core dumped)
> Aborted (core dumped)
> $
>
> Can you take a look at this?
>
> mips64-softmmu also fails on the same place, mips[el]-softmmu passes,
> but they don't use magnum.
>
> Code is supposed to be right, I will expect that the problem is in the
> magnum board, but this is qemu + mips + migration.  Anything can happen.
>
> Marc, I have to drop it again.
>
> Later, Juan.
>
>