[PATCH v2 0/3] tpm: fix PPI RAM migration

[PATCH v2 0/3] tpm: fix PPI RAM migration

[Marc-André Lureau]

Hi,

The following series fixes a migration issue with the TPM PPI code due
to unaligned host RAM pointer.

v2:
- use QEMU_IS_ALIGNED macros
- remove RFC status from "savevm: check RAM is pagesize aligned"

Marc-André Lureau (3):
  misc: use QEMU_IS_ALIGNED
  tpm-ppi: page-align PPI RAM
  savevm: check RAM is pagesize aligned

 exec.c             | 4 ++--
 hw/tpm/tpm_ppi.c   | 3 ++-
 migration/savevm.c | 5 +++++
 roms/SLOF          | 2 +-
 4 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.24.0.308.g228f53135a

[PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Marc-André Lureau]

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 exec.c    | 4 ++--
 roms/SLOF | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/exec.c b/exec.c
index d4b769d0d4..1feda49ca1 100644
--- a/exec.c
+++ b/exec.c
@@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
 
     uint8_t *host_startaddr = rb->host + start;
 
-    if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
+    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
         error_report("ram_block_discard_range: Unaligned start address: %p",
                      host_startaddr);
         goto err;
@@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
 
     if ((start + length) <= rb->used_length) {
         bool need_madvise, need_fallocate;
-        if (length & (rb->page_size - 1)) {
+        if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
             error_report("ram_block_discard_range: Unaligned length: %zx",
                          length);
             goto err;
diff --git a/roms/SLOF b/roms/SLOF
index 9546892a80..8ebf2f55e1 160000
--- a/roms/SLOF
+++ b/roms/SLOF
@@ -1 +1 @@
-Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
+Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
-- 
2.24.0.308.g228f53135a

Re: [PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Philippe Mathieu-Daudé]

On 1/3/20 8:39 AM, Marc-André Lureau wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   exec.c    | 4 ++--
>   roms/SLOF | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index d4b769d0d4..1feda49ca1 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
>   
>       uint8_t *host_startaddr = rb->host + start;
>   
> -    if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
> +    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
>           error_report("ram_block_discard_range: Unaligned start address: %p",
>                        host_startaddr);
>           goto err;
> @@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
>   
>       if ((start + length) <= rb->used_length) {
>           bool need_madvise, need_fallocate;
> -        if (length & (rb->page_size - 1)) {
> +        if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
>               error_report("ram_block_discard_range: Unaligned length: %zx",
>                            length);
>               goto err;
> diff --git a/roms/SLOF b/roms/SLOF
> index 9546892a80..8ebf2f55e1 160000
> --- a/roms/SLOF
> +++ b/roms/SLOF
> @@ -1 +1 @@
> -Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
> +Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98

Without the SLOF submodule update:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Marc-André Lureau]

On Fri, Jan 3, 2020 at 11:43 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 1/3/20 8:39 AM, Marc-André Lureau wrote:
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >   exec.c    | 4 ++--
> >   roms/SLOF | 2 +-
> >   2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index d4b769d0d4..1feda49ca1 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
> >
> >       uint8_t *host_startaddr = rb->host + start;
> >
> > -    if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
> > +    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
> >           error_report("ram_block_discard_range: Unaligned start address: %p",
> >                        host_startaddr);
> >           goto err;
> > @@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
> >
> >       if ((start + length) <= rb->used_length) {
> >           bool need_madvise, need_fallocate;
> > -        if (length & (rb->page_size - 1)) {
> > +        if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
> >               error_report("ram_block_discard_range: Unaligned length: %zx",
> >                            length);
> >               goto err;
> > diff --git a/roms/SLOF b/roms/SLOF
> > index 9546892a80..8ebf2f55e1 160000
> > --- a/roms/SLOF
> > +++ b/roms/SLOF
> > @@ -1 +1 @@
> > -Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
> > +Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
>
> Without the SLOF submodule update:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

oops, I think I should submit a git config proposal, for git commit -a
to exclude submodules, or warn loudly...

Or perhaps I should stop using -a.

Re: [PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Philippe Mathieu-Daudé]

On 1/3/20 8:59 AM, Marc-André Lureau wrote:
> On Fri, Jan 3, 2020 at 11:43 AM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
>>
>> On 1/3/20 8:39 AM, Marc-André Lureau wrote:
>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>> ---
>>>    exec.c    | 4 ++--
>>>    roms/SLOF | 2 +-
>>>    2 files changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/exec.c b/exec.c
>>> index d4b769d0d4..1feda49ca1 100644
>>> --- a/exec.c
>>> +++ b/exec.c
>>> @@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
>>>
>>>        uint8_t *host_startaddr = rb->host + start;
>>>
>>> -    if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
>>> +    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
>>>            error_report("ram_block_discard_range: Unaligned start address: %p",
>>>                         host_startaddr);
>>>            goto err;
>>> @@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb, uint64_t start, size_t length)
>>>
>>>        if ((start + length) <= rb->used_length) {
>>>            bool need_madvise, need_fallocate;
>>> -        if (length & (rb->page_size - 1)) {
>>> +        if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
>>>                error_report("ram_block_discard_range: Unaligned length: %zx",
>>>                             length);
>>>                goto err;
>>> diff --git a/roms/SLOF b/roms/SLOF
>>> index 9546892a80..8ebf2f55e1 160000
>>> --- a/roms/SLOF
>>> +++ b/roms/SLOF
>>> @@ -1 +1 @@
>>> -Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
>>> +Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
>>
>> Without the SLOF submodule update:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> oops, I think I should submit a git config proposal, for git commit -a
> to exclude submodules, or warn loudly...
> 
> Or perhaps I should stop using -a.

Once someone told me "never use 'git add -a' it is dangerous", but as I 
was learning git the same person told me "never use git-rebase it is 
dangerous" so I dunno :)

I have so many unstagged files in my repo that I can not use '-a'.
When doing mechanical changes, I dumbly use 'yes | git add -p'.
This worked fine so far...

Re: [PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Stefan Berger]

On 1/3/20 2:43 AM, Philippe Mathieu-Daudé wrote:
> On 1/3/20 8:39 AM, Marc-André Lureau wrote:
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>   exec.c    | 4 ++--
>>   roms/SLOF | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index d4b769d0d4..1feda49ca1 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb, 
>> uint64_t start, size_t length)
>>         uint8_t *host_startaddr = rb->host + start;
>>   -    if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
>> +    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
>>           error_report("ram_block_discard_range: Unaligned start 
>> address: %p",
>>                        host_startaddr);
>>           goto err;
>> @@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb, 
>> uint64_t start, size_t length)
>>         if ((start + length) <= rb->used_length) {
>>           bool need_madvise, need_fallocate;
>> -        if (length & (rb->page_size - 1)) {
>> +        if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
>>               error_report("ram_block_discard_range: Unaligned 
>> length: %zx",
>>                            length);
>>               goto err;
>> diff --git a/roms/SLOF b/roms/SLOF
>> index 9546892a80..8ebf2f55e1 160000
>> --- a/roms/SLOF
>> +++ b/roms/SLOF
>> @@ -1 +1 @@
>> -Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
>> +Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
>
> Without the SLOF submodule update:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Paolo Bonzini]

On 03/01/20 08:43, Philippe Mathieu-Daudé wrote:
> On 1/3/20 8:39 AM, Marc-André Lureau wrote:
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>   exec.c    | 4 ++--
>>   roms/SLOF | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/exec.c b/exec.c
>> index d4b769d0d4..1feda49ca1 100644
>> --- a/exec.c
>> +++ b/exec.c
>> @@ -3895,7 +3895,7 @@ int ram_block_discard_range(RAMBlock *rb,
>> uint64_t start, size_t length)
>>         uint8_t *host_startaddr = rb->host + start;
>>   -    if ((uintptr_t)host_startaddr & (rb->page_size - 1)) {
>> +    if (!QEMU_PTR_IS_ALIGNED(host_startaddr, rb->page_size)) {
>>           error_report("ram_block_discard_range: Unaligned start
>> address: %p",
>>                        host_startaddr);
>>           goto err;
>> @@ -3903,7 +3903,7 @@ int ram_block_discard_range(RAMBlock *rb,
>> uint64_t start, size_t length)
>>         if ((start + length) <= rb->used_length) {
>>           bool need_madvise, need_fallocate;
>> -        if (length & (rb->page_size - 1)) {
>> +        if (!QEMU_IS_ALIGNED(length, rb->page_size)) {
>>               error_report("ram_block_discard_range: Unaligned length:
>> %zx",
>>                            length);
>>               goto err;
>> diff --git a/roms/SLOF b/roms/SLOF
>> index 9546892a80..8ebf2f55e1 160000
>> --- a/roms/SLOF
>> +++ b/roms/SLOF
>> @@ -1 +1 @@
>> -Subproject commit 9546892a80d5a4c73deea6719de46372f007f4a6
>> +Subproject commit 8ebf2f55e1ba1492b942ba4b682160e644fc0f98
> 
> Without the SLOF submodule update:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Also without the SLOF update:

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [PATCH v2 1/3] misc: use QEMU_IS_ALIGNED

[Juan Quintela]

Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

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

queuing it without the SLOF bits

[PATCH v2 2/3] tpm-ppi: page-align PPI RAM

[Marc-André Lureau]

post-copy migration fails on destination with error such as:
2019-12-26T10:22:44.714644Z qemu-kvm: ram_block_discard_range:
Unaligned start address: 0x559d2afae9a0

Use qemu_memalign() to constrain the PPI RAM memory alignment.

Cc: qemu-stable@nongnu.org
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/tpm/tpm_ppi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
index ff314592b4..6d9c1a3e40 100644
--- a/hw/tpm/tpm_ppi.c
+++ b/hw/tpm/tpm_ppi.c
@@ -43,7 +43,8 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
 void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
                   hwaddr addr, Object *obj)
 {
-    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
+    tpmppi->buf = qemu_memalign(qemu_real_host_page_size,
+                                HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
     memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
                                       TPM_PPI_ADDR_SIZE, tpmppi->buf);
     vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
-- 
2.24.0.308.g228f53135a

Re: [PATCH v2 2/3] tpm-ppi: page-align PPI RAM

[Dr. David Alan Gilbert]

* Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> post-copy migration fails on destination with error such as:
> 2019-12-26T10:22:44.714644Z qemu-kvm: ram_block_discard_range:
> Unaligned start address: 0x559d2afae9a0

Ah good, we got a sane error message!

> Use qemu_memalign() to constrain the PPI RAM memory alignment.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/tpm/tpm_ppi.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> index ff314592b4..6d9c1a3e40 100644
> --- a/hw/tpm/tpm_ppi.c
> +++ b/hw/tpm/tpm_ppi.c
> @@ -43,7 +43,8 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
>  void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>                    hwaddr addr, Object *obj)
>  {
> -    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> +    tpmppi->buf = qemu_memalign(qemu_real_host_page_size,
> +                                HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

>      memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
>                                        TPM_PPI_ADDR_SIZE, tpmppi->buf);
>      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> -- 
> 2.24.0.308.g228f53135a
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 2/3] tpm-ppi: page-align PPI RAM

[Stefan Berger]

On 1/3/20 5:02 AM, Dr. David Alan Gilbert wrote:
> * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
>> post-copy migration fails on destination with error such as:
>> 2019-12-26T10:22:44.714644Z qemu-kvm: ram_block_discard_range:
>> Unaligned start address: 0x559d2afae9a0
> Ah good, we got a sane error message!
>
>> Use qemu_memalign() to constrain the PPI RAM memory alignment.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   hw/tpm/tpm_ppi.c | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
>> index ff314592b4..6d9c1a3e40 100644
>> --- a/hw/tpm/tpm_ppi.c
>> +++ b/hw/tpm/tpm_ppi.c
>> @@ -43,7 +43,8 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
>>   void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>                     hwaddr addr, Object *obj)
>>   {
>> -    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
>> +    tpmppi->buf = qemu_memalign(qemu_real_host_page_size,
>> +                                HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>

Re: [PATCH v2 2/3] tpm-ppi: page-align PPI RAM

[Dr. David Alan Gilbert]

* Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > post-copy migration fails on destination with error such as:
> > 2019-12-26T10:22:44.714644Z qemu-kvm: ram_block_discard_range:
> > Unaligned start address: 0x559d2afae9a0
> 
> Ah good, we got a sane error message!
> 
> > Use qemu_memalign() to constrain the PPI RAM memory alignment.
> > 
> > Cc: qemu-stable@nongnu.org
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > ---
> >  hw/tpm/tpm_ppi.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > index ff314592b4..6d9c1a3e40 100644
> > --- a/hw/tpm/tpm_ppi.c
> > +++ b/hw/tpm/tpm_ppi.c
> > @@ -43,7 +43,8 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
> >  void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >                    hwaddr addr, Object *obj)
> >  {
> > -    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> > +    tpmppi->buf = qemu_memalign(qemu_real_host_page_size,
> > +                                HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I just noticed a bit in the CODING_STYLE that says:

  Memory allocated by qemu_memalign or qemu_blockalign must be freed with
  qemu_vfree, since breaking this will cause problems on Win32.

so I guess this is wrong?

Dave

> 
> >      memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> >                                        TPM_PPI_ADDR_SIZE, tpmppi->buf);
> >      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> > -- 
> > 2.24.0.308.g228f53135a
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v2 2/3] tpm-ppi: page-align PPI RAM

[Marc-André Lureau]

Hi

On Wed, Jan 8, 2020 at 3:29 PM Dr. David Alan Gilbert
<dgilbert@redhat.com> wrote:
>
> * Dr. David Alan Gilbert (dgilbert@redhat.com) wrote:
> > * Marc-André Lureau (marcandre.lureau@redhat.com) wrote:
> > > post-copy migration fails on destination with error such as:
> > > 2019-12-26T10:22:44.714644Z qemu-kvm: ram_block_discard_range:
> > > Unaligned start address: 0x559d2afae9a0
> >
> > Ah good, we got a sane error message!
> >
> > > Use qemu_memalign() to constrain the PPI RAM memory alignment.
> > >
> > > Cc: qemu-stable@nongnu.org
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> > > ---
> > >  hw/tpm/tpm_ppi.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > > index ff314592b4..6d9c1a3e40 100644
> > > --- a/hw/tpm/tpm_ppi.c
> > > +++ b/hw/tpm/tpm_ppi.c
> > > @@ -43,7 +43,8 @@ void tpm_ppi_reset(TPMPPI *tpmppi)
> > >  void tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > >                    hwaddr addr, Object *obj)
> > >  {
> > > -    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> > > +    tpmppi->buf = qemu_memalign(qemu_real_host_page_size,
> > > +                                HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> >
> > Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>
> I just noticed a bit in the CODING_STYLE that says:
>
>   Memory allocated by qemu_memalign or qemu_blockalign must be freed with
>   qemu_vfree, since breaking this will cause problems on Win32.
>
> so I guess this is wrong?

The buf is not freed, TPM are not hotpluggable. For strictness, we
could have a finalize later.

>
> Dave
>
> >
> > >      memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> > >                                        TPM_PPI_ADDR_SIZE, tpmppi->buf);
> > >      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> > > --
> > > 2.24.0.308.g228f53135a
> > >
> > --
> > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
>
>


-- 
Marc-André Lureau

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

[Marc-André Lureau]

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

[Philippe Mathieu-Daudé]

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

[Juan Quintela]

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

[Marc-André Lureau]

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

[Juan Quintela]

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

[Aleksandar Markovic]

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

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.
>
>
>

[-- Attachment #2: Type: text/html, Size: 1802 bytes --]

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

[Juan Quintela]

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

[Juan Quintela]

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

[Aleksandar Markovic]

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

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.
>
>

[-- Attachment #2: Type: text/html, Size: 4051 bytes --]