[v2] tpm: fix PPI RAM migration

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

Posted by Marc-André Lureau 6 years, 1 month ago

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

Posted by Dr. David Alan Gilbert 6 years, 1 month ago

* 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

Posted by Dr. David Alan Gilbert 6 years, 1 month ago

* 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

Posted by Marc-André Lureau 6 years, 1 month ago

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

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

Posted by Stefan Berger 6 years, 1 month ago

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>