[PULL 0/2] VFIO fixes 2022-02-03

[PULL 0/2] VFIO fixes 2022-02-03

[Alex Williamson]

The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:

  Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)

are available in the Git repository at:

  git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0

for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:

  hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)

----------------------------------------------------------------
VFIO fixes 2022-02-03

 * Fix alignment warnings when using TPM CRB with vfio-pci devices
   (Eric Auger & Philippe Mathieu-Daudé)

----------------------------------------------------------------
Eric Auger (2):
      tpm: CRB: Use ram_device for "tpm-crb-cmd" region
      hw/vfio/common: Silence ram device offset alignment error traces

 hw/tpm/tpm_crb.c     | 22 ++++++++++++++++++++--
 hw/vfio/common.c     | 15 ++++++++++++++-
 hw/vfio/trace-events |  1 +
 3 files changed, 35 insertions(+), 3 deletions(-)

[PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Alex Williamson]

From: Eric Auger <eric.auger@redhat.com>

Representing the CRB cmd/response buffer as a standard
RAM region causes some trouble when the device is used
with VFIO. Indeed VFIO attempts to DMA_MAP this region
as usual RAM but this latter does not have a valid page
size alignment causing such an error report:
"vfio_listener_region_add received unaligned region".
To allow VFIO to detect that failing dma mapping
this region is not an issue, let's use a ram_device
memory region type instead.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
[PMD: Keep tpm_crb.c in meson's softmmu_ss]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Link: https://lore.kernel.org/r/20220120001242.230082-2-f4bug@amsat.org
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/tpm/tpm_crb.c |   22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 58ebd1469c35..be0884ea6031 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -25,6 +25,7 @@
 #include "sysemu/tpm_backend.h"
 #include "sysemu/tpm_util.h"
 #include "sysemu/reset.h"
+#include "exec/cpu-common.h"
 #include "tpm_prop.h"
 #include "tpm_ppi.h"
 #include "trace.h"
@@ -43,6 +44,7 @@ struct CRBState {
 
     bool ppi_enabled;
     TPMPPI ppi;
+    uint8_t *crb_cmd_buf;
 };
 typedef struct CRBState CRBState;
 
@@ -291,10 +293,14 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    s->crb_cmd_buf = qemu_memalign(qemu_real_host_page_size,
+                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
+
     memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
         "tpm-crb-mmio", sizeof(s->regs));
-    memory_region_init_ram(&s->cmdmem, OBJECT(s),
-        "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
+    memory_region_init_ram_device_ptr(&s->cmdmem, OBJECT(s), "tpm-crb-cmd",
+                                      CRB_CTRL_CMD_SIZE, s->crb_cmd_buf);
+    vmstate_register_ram(&s->cmdmem, DEVICE(s));
 
     memory_region_add_subregion(get_system_memory(),
         TPM_CRB_ADDR_BASE, &s->mmio);
@@ -309,12 +315,24 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
     qemu_register_reset(tpm_crb_reset, dev);
 }
 
+static void tpm_crb_unrealize(DeviceState *dev)
+{
+    CRBState *s = CRB(dev);
+
+    qemu_vfree(s->crb_cmd_buf);
+
+    if (s->ppi_enabled) {
+        qemu_vfree(s->ppi.buf);
+    }
+}
+
 static void tpm_crb_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
     TPMIfClass *tc = TPM_IF_CLASS(klass);
 
     dc->realize = tpm_crb_realize;
+    dc->unrealize = tpm_crb_unrealize;
     device_class_set_props(dc, tpm_crb_properties);
     dc->vmsd  = &vmstate_tpm_crb;
     dc->user_creatable = true;

[PULL 2/2] hw/vfio/common: Silence ram device offset alignment error traces

[Alex Williamson]

From: Eric Auger <eric.auger@redhat.com>

Failing to DMA MAP a ram_device should not cause an error message.
This is currently happening with the TPM CRB command region and
this is causing confusion.

We may want to keep the trace for debug purpose though.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>
Acked-by: Alex Williamson <alex.williamson@redhat.com>
Acked-by: Stefan Berger <stefanb@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Link: https://lore.kernel.org/r/20220120001242.230082-3-f4bug@amsat.org
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/common.c     |   15 ++++++++++++++-
 hw/vfio/trace-events |    1 +
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 080046e3f511..9caa560b0788 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -884,7 +884,20 @@ static void vfio_listener_region_add(MemoryListener *listener,
     if (unlikely((section->offset_within_address_space &
                   ~qemu_real_host_page_mask) !=
                  (section->offset_within_region & ~qemu_real_host_page_mask))) {
-        error_report("%s received unaligned region", __func__);
+        if (memory_region_is_ram_device(section->mr)) { /* just debug purpose */
+            trace_vfio_listener_region_add_bad_offset_alignment(
+                memory_region_name(section->mr),
+                section->offset_within_address_space,
+                section->offset_within_region, qemu_real_host_page_size);
+        } else { /* error case we don't want to be fatal */
+            error_report("%s received unaligned region %s iova=0x%"PRIx64
+                         " offset_within_region=0x%"PRIx64
+                         " qemu_real_host_page_mask=0x%"PRIx64,
+                         __func__, memory_region_name(section->mr),
+                         section->offset_within_address_space,
+                         section->offset_within_region,
+                         qemu_real_host_page_mask);
+        }
         return;
     }
 
diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events
index 0ef1b5f4a65f..ccd9d7610d69 100644
--- a/hw/vfio/trace-events
+++ b/hw/vfio/trace-events
@@ -100,6 +100,7 @@ vfio_listener_region_add_skip(uint64_t start, uint64_t end) "SKIPPING region_add
 vfio_spapr_group_attach(int groupfd, int tablefd) "Attached groupfd %d to liobn fd %d"
 vfio_listener_region_add_iommu(uint64_t start, uint64_t end) "region_add [iommu] 0x%"PRIx64" - 0x%"PRIx64
 vfio_listener_region_add_ram(uint64_t iova_start, uint64_t iova_end, void *vaddr) "region_add [ram] 0x%"PRIx64" - 0x%"PRIx64" [%p]"
+vfio_listener_region_add_bad_offset_alignment(const char *name, uint64_t iova, uint64_t offset_within_region, uint64_t page_size) "Region \"%s\" @0x%"PRIx64", offset_within_region=0x%"PRIx64", qemu_real_host_page_mask=0x%"PRIx64 " cannot be mapped for DMA"
 vfio_listener_region_add_no_dma_map(const char *name, uint64_t iova, uint64_t size, uint64_t page_size) "Region \"%s\" 0x%"PRIx64" size=0x%"PRIx64" is not aligned to 0x%"PRIx64" and cannot be mapped for DMA"
 vfio_listener_region_del_skip(uint64_t start, uint64_t end) "SKIPPING region_del 0x%"PRIx64" - 0x%"PRIx64
 vfio_listener_region_del(uint64_t start, uint64_t end) "region_del 0x%"PRIx64" - 0x%"PRIx64

Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Igor Mammedov]

On Thu, 03 Feb 2022 15:35:35 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> From: Eric Auger <eric.auger@redhat.com>
> 
> Representing the CRB cmd/response buffer as a standard
> RAM region causes some trouble when the device is used
> with VFIO. Indeed VFIO attempts to DMA_MAP this region
> as usual RAM but this latter does not have a valid page
> size alignment causing such an error report:
> "vfio_listener_region_add received unaligned region".
> To allow VFIO to detect that failing dma mapping
> this region is not an issue, let's use a ram_device
> memory region type instead.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> Acked-by: Stefan Berger <stefanb@linux.ibm.com>
> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Link: https://lore.kernel.org/r/20220120001242.230082-2-f4bug@amsat.org
> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> ---
>  hw/tpm/tpm_crb.c |   22 ++++++++++++++++++++--
>  1 file changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 58ebd1469c35..be0884ea6031 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -25,6 +25,7 @@
>  #include "sysemu/tpm_backend.h"
>  #include "sysemu/tpm_util.h"
>  #include "sysemu/reset.h"
> +#include "exec/cpu-common.h"
>  #include "tpm_prop.h"
>  #include "tpm_ppi.h"
>  #include "trace.h"
> @@ -43,6 +44,7 @@ struct CRBState {
>  
>      bool ppi_enabled;
>      TPMPPI ppi;
> +    uint8_t *crb_cmd_buf;
>  };
>  typedef struct CRBState CRBState;
>  
> @@ -291,10 +293,14 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>          return;
>      }
>  
> +    s->crb_cmd_buf = qemu_memalign(qemu_real_host_page_size,
> +                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
> +
>      memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
>          "tpm-crb-mmio", sizeof(s->regs));
> -    memory_region_init_ram(&s->cmdmem, OBJECT(s),
> -        "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
> +    memory_region_init_ram_device_ptr(&s->cmdmem, OBJECT(s), "tpm-crb-cmd",
> +                                      CRB_CTRL_CMD_SIZE, s->crb_cmd_buf);

> +    vmstate_register_ram(&s->cmdmem, DEVICE(s));

Does it need a compat knob for the case of migrating to older QEMU/machine type,
not to end-up with target aborting migration when it sees unknown section.


>      memory_region_add_subregion(get_system_memory(),
>          TPM_CRB_ADDR_BASE, &s->mmio);
> @@ -309,12 +315,24 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>      qemu_register_reset(tpm_crb_reset, dev);
>  }
>  
> +static void tpm_crb_unrealize(DeviceState *dev)
> +{
> +    CRBState *s = CRB(dev);
> +

likewise, should vmstate be unregistered here, before freeing
actually happens?

> +    qemu_vfree(s->crb_cmd_buf);
> +
> +    if (s->ppi_enabled) {
> +        qemu_vfree(s->ppi.buf);
> +    }
> +}
> +
>  static void tpm_crb_class_init(ObjectClass *klass, void *data)
>  {
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      TPMIfClass *tc = TPM_IF_CLASS(klass);
>  
>      dc->realize = tpm_crb_realize;
> +    dc->unrealize = tpm_crb_unrealize;
>      device_class_set_props(dc, tpm_crb_properties);
>      dc->vmsd  = &vmstate_tpm_crb;
>      dc->user_creatable = true;
> 
> 
> 

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Peter Maydell]

On Thu, 3 Feb 2022 at 22:38, Alex Williamson <alex.williamson@redhat.com> wrote:
>
> The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
>
>   Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
>
> for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
>
>   hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)
>
> ----------------------------------------------------------------
> VFIO fixes 2022-02-03
>
>  * Fix alignment warnings when using TPM CRB with vfio-pci devices
>    (Eric Auger & Philippe Mathieu-Daudé)

Hi; this has a format-string issue that means it doesn't build
on 32-bit systems:

https://gitlab.com/qemu-project/qemu/-/jobs/2057116569

../hw/vfio/common.c: In function 'vfio_listener_region_add':
../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
'int'} [-Werror=format=]
error_report("%s received unaligned region %s iova=0x%"PRIx64
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../hw/vfio/common.c:899:26:
qemu_real_host_page_mask);
~~~~~~~~~~~~~~~~~~~~~~~~

For intptr_t you want PRIxPTR.

thanks
-- PMM

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Philippe Mathieu-Daudé via]

On 5/2/22 11:49, Peter Maydell wrote:
> On Thu, 3 Feb 2022 at 22:38, Alex Williamson <alex.williamson@redhat.com> wrote:
>>
>> The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
>>
>>    Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)
>>
>> are available in the Git repository at:
>>
>>    git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
>>
>> for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
>>
>>    hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)
>>
>> ----------------------------------------------------------------
>> VFIO fixes 2022-02-03
>>
>>   * Fix alignment warnings when using TPM CRB with vfio-pci devices
>>     (Eric Auger & Philippe Mathieu-Daudé)
> 
> Hi; this has a format-string issue that means it doesn't build
> on 32-bit systems:
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
> 
> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
> 'int'} [-Werror=format=]
> error_report("%s received unaligned region %s iova=0x%"PRIx64
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../hw/vfio/common.c:899:26:
> qemu_real_host_page_mask);
> ~~~~~~~~~~~~~~~~~~~~~~~~
> 
> For intptr_t you want PRIxPTR.

Thanks Peter.

Eric, can you follow up on this series, looking at Igor comments wrt
migration state?

Phil.

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Eric Auger]

Hi

On 2/5/22 12:19 PM, Philippe Mathieu-Daudé wrote:
> On 5/2/22 11:49, Peter Maydell wrote:
>> On Thu, 3 Feb 2022 at 22:38, Alex Williamson
>> <alex.williamson@redhat.com> wrote:
>>>
>>> The following changes since commit
>>> 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
>>>
>>>    Merge remote-tracking branch
>>> 'remotes/hdeller/tags/hppa-updates-pull-request' into staging
>>> (2022-02-02 19:54:30 +0000)
>>>
>>> are available in the Git repository at:
>>>
>>>    git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
>>>
>>> for you to fetch changes up to
>>> 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
>>>
>>>    hw/vfio/common: Silence ram device offset alignment error traces
>>> (2022-02-03 15:05:05 -0700)
>>>
>>> ----------------------------------------------------------------
>>> VFIO fixes 2022-02-03
>>>
>>>   * Fix alignment warnings when using TPM CRB with vfio-pci devices
>>>     (Eric Auger & Philippe Mathieu-Daudé)
>>
>> Hi; this has a format-string issue that means it doesn't build
>> on 32-bit systems:
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
>>
>> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
>> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
>> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
>> 'int'} [-Werror=format=]
>> error_report("%s received unaligned region %s iova=0x%"PRIx64
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../hw/vfio/common.c:899:26:
>> qemu_real_host_page_mask);
>> ~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> For intptr_t you want PRIxPTR.
>
> Thanks Peter.
>
> Eric, can you follow up on this series, looking at Igor comments wrt
> migration state?

Sure I will.

Eric
>
> Phil.
>

Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Eric Auger]

Hi Igor,
On 2/4/22 1:08 PM, Igor Mammedov wrote:
> On Thu, 03 Feb 2022 15:35:35 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> Representing the CRB cmd/response buffer as a standard
>> RAM region causes some trouble when the device is used
>> with VFIO. Indeed VFIO attempts to DMA_MAP this region
>> as usual RAM but this latter does not have a valid page
>> size alignment causing such an error report:
>> "vfio_listener_region_add received unaligned region".
>> To allow VFIO to detect that failing dma mapping
>> this region is not an issue, let's use a ram_device
>> memory region type instead.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
>> Acked-by: Stefan Berger <stefanb@linux.ibm.com>
>> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Link: https://lore.kernel.org/r/20220120001242.230082-2-f4bug@amsat.org
>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> ---
>>  hw/tpm/tpm_crb.c |   22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
>> index 58ebd1469c35..be0884ea6031 100644
>> --- a/hw/tpm/tpm_crb.c
>> +++ b/hw/tpm/tpm_crb.c
>> @@ -25,6 +25,7 @@
>>  #include "sysemu/tpm_backend.h"
>>  #include "sysemu/tpm_util.h"
>>  #include "sysemu/reset.h"
>> +#include "exec/cpu-common.h"
>>  #include "tpm_prop.h"
>>  #include "tpm_ppi.h"
>>  #include "trace.h"
>> @@ -43,6 +44,7 @@ struct CRBState {
>>  
>>      bool ppi_enabled;
>>      TPMPPI ppi;
>> +    uint8_t *crb_cmd_buf;
>>  };
>>  typedef struct CRBState CRBState;
>>  
>> @@ -291,10 +293,14 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>>          return;
>>      }
>>  
>> +    s->crb_cmd_buf = qemu_memalign(qemu_real_host_page_size,
>> +                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
>> +
>>      memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
>>          "tpm-crb-mmio", sizeof(s->regs));
>> -    memory_region_init_ram(&s->cmdmem, OBJECT(s),
>> -        "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
>> +    memory_region_init_ram_device_ptr(&s->cmdmem, OBJECT(s), "tpm-crb-cmd",
>> +                                      CRB_CTRL_CMD_SIZE, s->crb_cmd_buf);
>> +    vmstate_register_ram(&s->cmdmem, DEVICE(s));
> Does it need a compat knob for the case of migrating to older QEMU/machine type,
> not to end-up with target aborting migration when it sees unknown section.
Hum I did not think about this. I need to double check.

Thank you for the review.

Eric
>
>
>>      memory_region_add_subregion(get_system_memory(),
>>          TPM_CRB_ADDR_BASE, &s->mmio);
>> @@ -309,12 +315,24 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>>      qemu_register_reset(tpm_crb_reset, dev);
>>  }
>>  
>> +static void tpm_crb_unrealize(DeviceState *dev)
>> +{
>> +    CRBState *s = CRB(dev);
>> +
> likewise, should vmstate be unregistered here, before freeing
> actually happens?
>
>> +    qemu_vfree(s->crb_cmd_buf);
>> +
>> +    if (s->ppi_enabled) {
>> +        qemu_vfree(s->ppi.buf);
>> +    }
>> +}
>> +
>>  static void tpm_crb_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>      TPMIfClass *tc = TPM_IF_CLASS(klass);
>>  
>>      dc->realize = tpm_crb_realize;
>> +    dc->unrealize = tpm_crb_unrealize;
>>      device_class_set_props(dc, tpm_crb_properties);
>>      dc->vmsd  = &vmstate_tpm_crb;
>>      dc->user_creatable = true;
>>
>>
>>

Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Eric Auger]

Hi Igor,

On 2/4/22 1:08 PM, Igor Mammedov wrote:
> On Thu, 03 Feb 2022 15:35:35 -0700
> Alex Williamson <alex.williamson@redhat.com> wrote:
>
>> From: Eric Auger <eric.auger@redhat.com>
>>
>> Representing the CRB cmd/response buffer as a standard
>> RAM region causes some trouble when the device is used
>> with VFIO. Indeed VFIO attempts to DMA_MAP this region
>> as usual RAM but this latter does not have a valid page
>> size alignment causing such an error report:
>> "vfio_listener_region_add received unaligned region".
>> To allow VFIO to detect that failing dma mapping
>> this region is not an issue, let's use a ram_device
>> memory region type instead.
>>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
>> Acked-by: Stefan Berger <stefanb@linux.ibm.com>
>> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Link: https://lore.kernel.org/r/20220120001242.230082-2-f4bug@amsat.org
>> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
>> ---
>>  hw/tpm/tpm_crb.c |   22 ++++++++++++++++++++--
>>  1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
>> index 58ebd1469c35..be0884ea6031 100644
>> --- a/hw/tpm/tpm_crb.c
>> +++ b/hw/tpm/tpm_crb.c
>> @@ -25,6 +25,7 @@
>>  #include "sysemu/tpm_backend.h"
>>  #include "sysemu/tpm_util.h"
>>  #include "sysemu/reset.h"
>> +#include "exec/cpu-common.h"
>>  #include "tpm_prop.h"
>>  #include "tpm_ppi.h"
>>  #include "trace.h"
>> @@ -43,6 +44,7 @@ struct CRBState {
>>  
>>      bool ppi_enabled;
>>      TPMPPI ppi;
>> +    uint8_t *crb_cmd_buf;
>>  };
>>  typedef struct CRBState CRBState;
>>  
>> @@ -291,10 +293,14 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>>          return;
>>      }
>>  
>> +    s->crb_cmd_buf = qemu_memalign(qemu_real_host_page_size,
>> +                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
>> +
>>      memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
>>          "tpm-crb-mmio", sizeof(s->regs));
>> -    memory_region_init_ram(&s->cmdmem, OBJECT(s),
>> -        "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
>> +    memory_region_init_ram_device_ptr(&s->cmdmem, OBJECT(s), "tpm-crb-cmd",
>> +                                      CRB_CTRL_CMD_SIZE, s->crb_cmd_buf);
>> +    vmstate_register_ram(&s->cmdmem, DEVICE(s));
> Does it need a compat knob for the case of migrating to older QEMU/machine type,
> not to end-up with target aborting migration when it sees unknown section.

It does not seem to be requested. I am able to migrate between this
version and qemu 6.2, back and forth, using a pc-q35-6.2 machine type.
My guess is, as the amount of RAM that is migrated is the same, it does
not complain. Adding Dave and Juan though.

Thanks

Eric
>
>
>>      memory_region_add_subregion(get_system_memory(),
>>          TPM_CRB_ADDR_BASE, &s->mmio);
>> @@ -309,12 +315,24 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>>      qemu_register_reset(tpm_crb_reset, dev);
>>  }
>>  
>> +static void tpm_crb_unrealize(DeviceState *dev)
>> +{
>> +    CRBState *s = CRB(dev);
>> +
> likewise, should vmstate be unregistered here, before freeing
> actually happens?
>
>> +    qemu_vfree(s->crb_cmd_buf);
>> +
>> +    if (s->ppi_enabled) {
>> +        qemu_vfree(s->ppi.buf);
>> +    }
>> +}
>> +
>>  static void tpm_crb_class_init(ObjectClass *klass, void *data)
>>  {
>>      DeviceClass *dc = DEVICE_CLASS(klass);
>>      TPMIfClass *tc = TPM_IF_CLASS(klass);
>>  
>>      dc->realize = tpm_crb_realize;
>> +    dc->unrealize = tpm_crb_unrealize;
>>      device_class_set_props(dc, tpm_crb_properties);
>>      dc->vmsd  = &vmstate_tpm_crb;
>>      dc->user_creatable = true;
>>
>>
>>

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Alex Williamson]

On Sat, 5 Feb 2022 10:49:35 +0000
Peter Maydell <peter.maydell@linaro.org> wrote:

> On Thu, 3 Feb 2022 at 22:38, Alex Williamson <alex.williamson@redhat.com> wrote:
> >
> > The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
> >
> >   Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
> >
> > for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
> >
> >   hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)
> >
> > ----------------------------------------------------------------
> > VFIO fixes 2022-02-03
> >
> >  * Fix alignment warnings when using TPM CRB with vfio-pci devices
> >    (Eric Auger & Philippe Mathieu-Daudé)  
> 
> Hi; this has a format-string issue that means it doesn't build
> on 32-bit systems:
> 
> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
> 
> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
> 'int'} [-Werror=format=]
> error_report("%s received unaligned region %s iova=0x%"PRIx64
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ../hw/vfio/common.c:899:26:
> qemu_real_host_page_mask);
> ~~~~~~~~~~~~~~~~~~~~~~~~
> 
> For intptr_t you want PRIxPTR.

Darn.  Well, let me use this opportunity to ask, how are folks doing
32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
purpose, but I was eventually no longer able to maintain the build
dependencies.  Looks like this failed on a mipsel cross build, but I
don't see such a cross compiler in Fedora.  I do mingw32/64 cross
builds, but they leave a lot to be desired for code coverage.  Thanks,

Alex

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Philippe Mathieu-Daudé via]

On 7/2/22 16:50, Alex Williamson wrote:
> On Sat, 5 Feb 2022 10:49:35 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:

>> Hi; this has a format-string issue that means it doesn't build
>> on 32-bit systems:
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
>>
>> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
>> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
>> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
>> 'int'} [-Werror=format=]
>> error_report("%s received unaligned region %s iova=0x%"PRIx64
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../hw/vfio/common.c:899:26:
>> qemu_real_host_page_mask);
>> ~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> For intptr_t you want PRIxPTR.
> 
> Darn.  Well, let me use this opportunity to ask, how are folks doing
> 32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
> purpose, but I was eventually no longer able to maintain the build
> dependencies.  Looks like this failed on a mipsel cross build, but I
> don't see such a cross compiler in Fedora.  I do mingw32/64 cross
> builds, but they leave a lot to be desired for code coverage.  Thanks,

You can use docker images:
https://wiki.qemu.org/Testing/DockerBuild

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Thomas Huth]

On 07/02/2022 16.50, Alex Williamson wrote:
> On Sat, 5 Feb 2022 10:49:35 +0000
> Peter Maydell <peter.maydell@linaro.org> wrote:
> 
>> On Thu, 3 Feb 2022 at 22:38, Alex Williamson <alex.williamson@redhat.com> wrote:
>>>
>>> The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
>>>
>>>    Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)
>>>
>>> are available in the Git repository at:
>>>
>>>    git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
>>>
>>> for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
>>>
>>>    hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)
>>>
>>> ----------------------------------------------------------------
>>> VFIO fixes 2022-02-03
>>>
>>>   * Fix alignment warnings when using TPM CRB with vfio-pci devices
>>>     (Eric Auger & Philippe Mathieu-Daudé)
>>
>> Hi; this has a format-string issue that means it doesn't build
>> on 32-bit systems:
>>
>> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
>>
>> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
>> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
>> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
>> 'int'} [-Werror=format=]
>> error_report("%s received unaligned region %s iova=0x%"PRIx64
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ../hw/vfio/common.c:899:26:
>> qemu_real_host_page_mask);
>> ~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> For intptr_t you want PRIxPTR.
> 
> Darn.  Well, let me use this opportunity to ask, how are folks doing
> 32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
> purpose, but I was eventually no longer able to maintain the build
> dependencies.  Looks like this failed on a mipsel cross build, but I
> don't see such a cross compiler in Fedora.  I do mingw32/64 cross
> builds, but they leave a lot to be desired for code coverage.  Thanks,

The easiest way for getting more test coverage is likely to move your qemu 
repository from github to gitlab - then you get most of the CI for free, 
which should catch such issues before sending pull requests.

  Thomas

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Alex Williamson]

On Mon, 7 Feb 2022 17:08:01 +0100
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> On 7/2/22 16:50, Alex Williamson wrote:
> > On Sat, 5 Feb 2022 10:49:35 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:  
> 
> >> Hi; this has a format-string issue that means it doesn't build
> >> on 32-bit systems:
> >>
> >> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
> >>
> >> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
> >> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
> >> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
> >> 'int'} [-Werror=format=]
> >> error_report("%s received unaligned region %s iova=0x%"PRIx64
> >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ../hw/vfio/common.c:899:26:
> >> qemu_real_host_page_mask);
> >> ~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> For intptr_t you want PRIxPTR.  
> > 
> > Darn.  Well, let me use this opportunity to ask, how are folks doing
> > 32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
> > purpose, but I was eventually no longer able to maintain the build
> > dependencies.  Looks like this failed on a mipsel cross build, but I
> > don't see such a cross compiler in Fedora.  I do mingw32/64 cross
> > builds, but they leave a lot to be desired for code coverage.  Thanks,  
> 
> You can use docker images:
> https://wiki.qemu.org/Testing/DockerBuild

Hmm, not ideal...

Clean git clone, HEAD 55ef0b702bc2 ("Merge remote-tracking branch 'remotes/lvivier-gitlab/tags/linux-user-for-7.0-pull-request' into staging")

$ make docker-test-quick@debian-mips64el-cross J=16
...
1/1 qemu:block / qemu-iotests qcow2 RUNNING       
>>> PYTHON=/usr/bin/python3 MALLOC_PERTURB_=188 /bin/sh /tmp/qemu-test/build/../src/tests/qemu-iotests/../check-block.sh qcow2
1/1 qemu:block / qemu-iotests qcow2 ERROR           0.18s   exit status 1


Summary of Failures:

1/1 qemu:block / qemu-iotests qcow2 ERROR           0.18s   exit status 1


Ok:                 0   
Expected Fail:      0   
Fail:               1   
Unexpected Pass:    0   
Skipped:            0   
Timeout:            0   

Full log written to /tmp/qemu-test/build/meson-logs/iotestslog.txt
make: *** [/tmp/qemu-test/src/tests/Makefile.include:160: check-block] Error 1
make: *** Waiting for unfinished jobs....
130/131 qemu:qapi-schema+qapi-frontend / QAPI schema regression tests     OK              0.20s
131/131 qemu:decodetree / decodetree                                      OK              1.75s


Ok:                 3   
Expected Fail:      0   
Fail:               0   
Unexpected Pass:    0   
Skipped:            128 
Timeout:            0   

Full log written to /tmp/qemu-test/build/meson-logs/testlog.txt
Traceback (most recent call last):
  File "/tmp/qemu.git/./tests/docker/docker.py", line 758, in <module>
    sys.exit(main())
  File "/tmp/qemu.git/./tests/docker/docker.py", line 754, in main
    return args.cmdobj.run(args, argv)
  File "/tmp/qemu.git/./tests/docker/docker.py", line 430, in run
    return Docker().run(argv, args.keep, quiet=args.quiet,
  File "/tmp/qemu.git/./tests/docker/docker.py", line 388, in run
    ret = self._do_check(["run", "--rm", "--label",
  File "/tmp/qemu.git/./tests/docker/docker.py", line 252, in _do_check
    return subprocess.check_call(self._command + cmd, **kwargs)
  File "/usr/lib64/python3.9/subprocess.py", line 373, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['podman', 'run', '--rm', '--label', 'com.qemu.instance.uuid=560d8331a06b4fd9bbb74910f3a2b436', '--userns=keep-id', '-u', '1000', '--security-opt', 'seccomp=unconfined', '--net=none', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=16', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/alwillia/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/tmp/qemu.git/docker-src.2022-02-07-09.45.59.2258561:/var/tmp/qemu:z,ro', 'qemu/debian-mips64el-cross', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=560d8331a06b4fd9bbb74910f3a2b436
make[1]: *** [tests/docker/Makefile.include:306: docker-run] Error 1
make[1]: Leaving directory '/tmp/qemu.git'
make: *** [tests/docker/Makefile.include:339: docker-run-test-quick@debian-mips64el-cross] Error 2

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Alex Williamson]

On Mon, 7 Feb 2022 09:54:59 -0700
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Mon, 7 Feb 2022 17:08:01 +0100
> Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> 
> > On 7/2/22 16:50, Alex Williamson wrote:  
> > > On Sat, 5 Feb 2022 10:49:35 +0000
> > > Peter Maydell <peter.maydell@linaro.org> wrote:    
> >   
> > >> Hi; this has a format-string issue that means it doesn't build
> > >> on 32-bit systems:
> > >>
> > >> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
> > >>
> > >> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
> > >> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
> > >> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
> > >> 'int'} [-Werror=format=]
> > >> error_report("%s received unaligned region %s iova=0x%"PRIx64
> > >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >> ../hw/vfio/common.c:899:26:
> > >> qemu_real_host_page_mask);
> > >> ~~~~~~~~~~~~~~~~~~~~~~~~
> > >>
> > >> For intptr_t you want PRIxPTR.    
> > > 
> > > Darn.  Well, let me use this opportunity to ask, how are folks doing
> > > 32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
> > > purpose, but I was eventually no longer able to maintain the build
> > > dependencies.  Looks like this failed on a mipsel cross build, but I
> > > don't see such a cross compiler in Fedora.  I do mingw32/64 cross
> > > builds, but they leave a lot to be desired for code coverage.  Thanks,    
> > 
> > You can use docker images:
> > https://wiki.qemu.org/Testing/DockerBuild  
> 
> Hmm, not ideal...
> 
> Clean git clone, HEAD 55ef0b702bc2 ("Merge remote-tracking branch 'remotes/lvivier-gitlab/tags/linux-user-for-7.0-pull-request' into staging")
> 
> $ make docker-test-quick@debian-mips64el-cross J=16

Accidentally selected the mips64el, but tests failing seems to be
common.  I can reproduce the build issue with either the mipsel or
fedora-i386-cross, so I'll include some flavor of the test-build in my
build script.  Thanks,

Alex

Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region

[Dr. David Alan Gilbert]

* Eric Auger (eric.auger@redhat.com) wrote:
> Hi Igor,
> 
> On 2/4/22 1:08 PM, Igor Mammedov wrote:
> > On Thu, 03 Feb 2022 15:35:35 -0700
> > Alex Williamson <alex.williamson@redhat.com> wrote:
> >
> >> From: Eric Auger <eric.auger@redhat.com>
> >>
> >> Representing the CRB cmd/response buffer as a standard
> >> RAM region causes some trouble when the device is used
> >> with VFIO. Indeed VFIO attempts to DMA_MAP this region
> >> as usual RAM but this latter does not have a valid page
> >> size alignment causing such an error report:
> >> "vfio_listener_region_add received unaligned region".
> >> To allow VFIO to detect that failing dma mapping
> >> this region is not an issue, let's use a ram_device
> >> memory region type instead.
> >>
> >> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> >> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> >> Acked-by: Stefan Berger <stefanb@linux.ibm.com>
> >> [PMD: Keep tpm_crb.c in meson's softmmu_ss]
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> Link: https://lore.kernel.org/r/20220120001242.230082-2-f4bug@amsat.org
> >> Signed-off-by: Alex Williamson <alex.williamson@redhat.com>
> >> ---
> >>  hw/tpm/tpm_crb.c |   22 ++++++++++++++++++++--
> >>  1 file changed, 20 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> >> index 58ebd1469c35..be0884ea6031 100644
> >> --- a/hw/tpm/tpm_crb.c
> >> +++ b/hw/tpm/tpm_crb.c
> >> @@ -25,6 +25,7 @@
> >>  #include "sysemu/tpm_backend.h"
> >>  #include "sysemu/tpm_util.h"
> >>  #include "sysemu/reset.h"
> >> +#include "exec/cpu-common.h"
> >>  #include "tpm_prop.h"
> >>  #include "tpm_ppi.h"
> >>  #include "trace.h"
> >> @@ -43,6 +44,7 @@ struct CRBState {
> >>  
> >>      bool ppi_enabled;
> >>      TPMPPI ppi;
> >> +    uint8_t *crb_cmd_buf;
> >>  };
> >>  typedef struct CRBState CRBState;
> >>  
> >> @@ -291,10 +293,14 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
> >>          return;
> >>      }
> >>  
> >> +    s->crb_cmd_buf = qemu_memalign(qemu_real_host_page_size,
> >> +                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));
> >> +
> >>      memory_region_init_io(&s->mmio, OBJECT(s), &tpm_crb_memory_ops, s,
> >>          "tpm-crb-mmio", sizeof(s->regs));
> >> -    memory_region_init_ram(&s->cmdmem, OBJECT(s),
> >> -        "tpm-crb-cmd", CRB_CTRL_CMD_SIZE, errp);
> >> +    memory_region_init_ram_device_ptr(&s->cmdmem, OBJECT(s), "tpm-crb-cmd",
> >> +                                      CRB_CTRL_CMD_SIZE, s->crb_cmd_buf);
> >> +    vmstate_register_ram(&s->cmdmem, DEVICE(s));
> > Does it need a compat knob for the case of migrating to older QEMU/machine type,
> > not to end-up with target aborting migration when it sees unknown section.
> 
> It does not seem to be requested. I am able to migrate between this
> version and qemu 6.2, back and forth, using a pc-q35-6.2 machine type.
> My guess is, as the amount of RAM that is migrated is the same, it does
> not complain. Adding Dave and Juan though.

I think that should be OK; we just rely on the RAM Block name and size.

Dave

> Thanks
> 
> Eric
> >
> >
> >>      memory_region_add_subregion(get_system_memory(),
> >>          TPM_CRB_ADDR_BASE, &s->mmio);
> >> @@ -309,12 +315,24 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
> >>      qemu_register_reset(tpm_crb_reset, dev);
> >>  }
> >>  
> >> +static void tpm_crb_unrealize(DeviceState *dev)
> >> +{
> >> +    CRBState *s = CRB(dev);
> >> +
> > likewise, should vmstate be unregistered here, before freeing
> > actually happens?
> >
> >> +    qemu_vfree(s->crb_cmd_buf);
> >> +
> >> +    if (s->ppi_enabled) {
> >> +        qemu_vfree(s->ppi.buf);
> >> +    }
> >> +}
> >> +
> >>  static void tpm_crb_class_init(ObjectClass *klass, void *data)
> >>  {
> >>      DeviceClass *dc = DEVICE_CLASS(klass);
> >>      TPMIfClass *tc = TPM_IF_CLASS(klass);
> >>  
> >>      dc->realize = tpm_crb_realize;
> >> +    dc->unrealize = tpm_crb_unrealize;
> >>      device_class_set_props(dc, tpm_crb_properties);
> >>      dc->vmsd  = &vmstate_tpm_crb;
> >>      dc->user_creatable = true;
> >>
> >>
> >>
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Alex Williamson]

On Mon, 7 Feb 2022 17:20:02 +0100
Thomas Huth <thuth@redhat.com> wrote:

> On 07/02/2022 16.50, Alex Williamson wrote:
> > On Sat, 5 Feb 2022 10:49:35 +0000
> > Peter Maydell <peter.maydell@linaro.org> wrote:
> >   
> >> On Thu, 3 Feb 2022 at 22:38, Alex Williamson <alex.williamson@redhat.com> wrote:  
> >>>
> >>> The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
> >>>
> >>>    Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)
> >>>
> >>> are available in the Git repository at:
> >>>
> >>>    git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
> >>>
> >>> for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
> >>>
> >>>    hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)
> >>>
> >>> ----------------------------------------------------------------
> >>> VFIO fixes 2022-02-03
> >>>
> >>>   * Fix alignment warnings when using TPM CRB with vfio-pci devices
> >>>     (Eric Auger & Philippe Mathieu-Daudé)  
> >>
> >> Hi; this has a format-string issue that means it doesn't build
> >> on 32-bit systems:
> >>
> >> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
> >>
> >> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
> >> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
> >> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
> >> 'int'} [-Werror=format=]
> >> error_report("%s received unaligned region %s iova=0x%"PRIx64
> >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >> ../hw/vfio/common.c:899:26:
> >> qemu_real_host_page_mask);
> >> ~~~~~~~~~~~~~~~~~~~~~~~~
> >>
> >> For intptr_t you want PRIxPTR.  
> > 
> > Darn.  Well, let me use this opportunity to ask, how are folks doing
> > 32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
> > purpose, but I was eventually no longer able to maintain the build
> > dependencies.  Looks like this failed on a mipsel cross build, but I
> > don't see such a cross compiler in Fedora.  I do mingw32/64 cross
> > builds, but they leave a lot to be desired for code coverage.  Thanks,  
> 
> The easiest way for getting more test coverage is likely to move your qemu 
> repository from github to gitlab - then you get most of the CI for free, 
> which should catch such issues before sending pull requests.

Well, it worked for a few months, but now pushing a tag to gitlab runs
a whole 4 jobs vs the 124 jobs that it previously ran, so that's
useless now :(  Thanks,

Alex

Re: [PULL 0/2] VFIO fixes 2022-02-03

[Alex Williamson]

On Thu, 2 Jun 2022 15:31:38 -0600
Alex Williamson <alex.williamson@redhat.com> wrote:

> On Mon, 7 Feb 2022 17:20:02 +0100
> Thomas Huth <thuth@redhat.com> wrote:
> 
> > On 07/02/2022 16.50, Alex Williamson wrote:  
> > > On Sat, 5 Feb 2022 10:49:35 +0000
> > > Peter Maydell <peter.maydell@linaro.org> wrote:
> > >     
> > >> On Thu, 3 Feb 2022 at 22:38, Alex Williamson <alex.williamson@redhat.com> wrote:    
> > >>>
> > >>> The following changes since commit 8f3e5ce773c62bb5c4a847f3a9a5c98bbb3b359f:
> > >>>
> > >>>    Merge remote-tracking branch 'remotes/hdeller/tags/hppa-updates-pull-request' into staging (2022-02-02 19:54:30 +0000)
> > >>>
> > >>> are available in the Git repository at:
> > >>>
> > >>>    git://github.com/awilliam/qemu-vfio.git tags/vfio-fixes-20220203.0
> > >>>
> > >>> for you to fetch changes up to 36fe5d5836c8d5d928ef6d34e999d6991a2f732e:
> > >>>
> > >>>    hw/vfio/common: Silence ram device offset alignment error traces (2022-02-03 15:05:05 -0700)
> > >>>
> > >>> ----------------------------------------------------------------
> > >>> VFIO fixes 2022-02-03
> > >>>
> > >>>   * Fix alignment warnings when using TPM CRB with vfio-pci devices
> > >>>     (Eric Auger & Philippe Mathieu-Daudé)    
> > >>
> > >> Hi; this has a format-string issue that means it doesn't build
> > >> on 32-bit systems:
> > >>
> > >> https://gitlab.com/qemu-project/qemu/-/jobs/2057116569
> > >>
> > >> ../hw/vfio/common.c: In function 'vfio_listener_region_add':
> > >> ../hw/vfio/common.c:893:26: error: format '%llx' expects argument of
> > >> type 'long long unsigned int', but argument 6 has type 'intptr_t' {aka
> > >> 'int'} [-Werror=format=]
> > >> error_report("%s received unaligned region %s iova=0x%"PRIx64
> > >> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > >> ../hw/vfio/common.c:899:26:
> > >> qemu_real_host_page_mask);
> > >> ~~~~~~~~~~~~~~~~~~~~~~~~
> > >>
> > >> For intptr_t you want PRIxPTR.    
> > > 
> > > Darn.  Well, let me use this opportunity to ask, how are folks doing
> > > 32-bit cross builds on Fedora?  I used to keep an i686 PAE VM for this
> > > purpose, but I was eventually no longer able to maintain the build
> > > dependencies.  Looks like this failed on a mipsel cross build, but I
> > > don't see such a cross compiler in Fedora.  I do mingw32/64 cross
> > > builds, but they leave a lot to be desired for code coverage.  Thanks,    
> > 
> > The easiest way for getting more test coverage is likely to move your qemu 
> > repository from github to gitlab - then you get most of the CI for free, 
> > which should catch such issues before sending pull requests.  
> 
> Well, it worked for a few months, but now pushing a tag to gitlab runs
> a whole 4 jobs vs the 124 jobs that it previously ran, so that's
> useless now :(  Thanks,

And Richard has now sent me the link to your announcement, including
the git push variables to get things back to normal:

https://lists.nongnu.org/archive/html/qemu-devel/2022-06/msg00256.html

Thanks,
Alex