[PATCH v2 0/2] TPM-CRB: Remove spurious error report when used with VFIO

[PATCH v2 0/2] TPM-CRB: Remove spurious error report when used with VFIO

[Eric Auger]

This series aims at removing a spurious error message we get when
launching a guest with a TPM-CRB device and VFIO-PCI devices.

The CRB command buffer currently is a RAM MemoryRegion and given
its base address alignment, it causes an error report on
vfio_listener_region_add(). This series proposes to use a ram-device
region instead which helps in better assessing the dma map error
failure on VFIO side.

Best Regards

Eric

This series can be found at:
https://github.com/eauger/qemu/tree/tpm-crb-ram-device-v2

History:
v1 -> v2:
- added tpm_crb_unrealize (dared to keep Stefan's T-b though)

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/meson.build   |  2 +-
 hw/tpm/tpm_crb.c     | 22 ++++++++++++++++++++--
 hw/vfio/common.c     | 15 ++++++++++++++-
 hw/vfio/trace-events |  1 +
 4 files changed, 36 insertions(+), 4 deletions(-)

-- 
2.26.3

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

[Eric Auger]

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.

The change in meson.build is required to include the
cpu.h header.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Stefan Berger <stefanb@linux.ibm.com>

---

v1 -> v2:
- Add tpm_crb_unrealize
---
 hw/tpm/meson.build |  2 +-
 hw/tpm/tpm_crb.c   | 22 ++++++++++++++++++++--
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
index 1c68d81d6a..3e74df945b 100644
--- a/hw/tpm/meson.build
+++ b/hw/tpm/meson.build
@@ -1,8 +1,8 @@
 softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_tis_common.c'))
 softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true: files('tpm_tis_isa.c'))
 softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
-softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
 
+specific_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
 specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true: files('tpm_ppi.c'))
 specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true: files('tpm_ppi.c'))
 specific_ss.add(when: 'CONFIG_TPM_SPAPR', if_true: files('tpm_spapr.c'))
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index 58ebd1469c..6ec19a9911 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 "cpu.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;
-- 
2.26.3

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

[Eric Auger]

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>
---
 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 080046e3f5..9caa560b07 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 0ef1b5f4a6..ccd9d7610d 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
-- 
2.26.3

Re: [PATCH v2 2/2] hw/vfio/common: Silence ram device offset alignment error traces

[Alex Williamson]

On Tue, 18 Jan 2022 16:33:06 +0100
Eric Auger <eric.auger@redhat.com> wrote:

> 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>
> ---
>  hw/vfio/common.c     | 15 ++++++++++++++-
>  hw/vfio/trace-events |  1 +
>  2 files changed, 15 insertions(+), 1 deletion(-)

Thanks!  Looks good to me.

Stefan, I can provide an ack here if you want to send a pull request
for both or likewise I can send a pull request with your ack on the
previous patch.  I suppose the patches themselves are technically
independent if you want to split them.  Whichever you prefer.

Acked-by: Alex Williamson <alex.williamson@redhat.com>

> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 080046e3f5..9caa560b07 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 0ef1b5f4a6..ccd9d7610d 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: [PATCH v2 2/2] hw/vfio/common: Silence ram device offset alignment error traces

[Stefan Berger]

On 1/19/22 15:13, Alex Williamson wrote:
> On Tue, 18 Jan 2022 16:33:06 +0100
> Eric Auger <eric.auger@redhat.com> wrote:
>
>> 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>
>> ---
>>   hw/vfio/common.c     | 15 ++++++++++++++-
>>   hw/vfio/trace-events |  1 +
>>   2 files changed, 15 insertions(+), 1 deletion(-)
> Thanks!  Looks good to me.
>
> Stefan, I can provide an ack here if you want to send a pull request
> for both or likewise I can send a pull request with your ack on the
> previous patch.  I suppose the patches themselves are technically
> independent if you want to split them.  Whichever you prefer.
>
> Acked-by: Alex Williamson <alex.williamson@redhat.com>

If you want to send the PR, please go ahead.

    Stefan

>
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 080046e3f5..9caa560b07 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 0ef1b5f4a6..ccd9d7610d 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: [PATCH v2 0/2] TPM-CRB: Remove spurious error report when used with VFIO

[Stefan Berger]

On 1/18/22 10:33, Eric Auger wrote:
> This series aims at removing a spurious error message we get when
> launching a guest with a TPM-CRB device and VFIO-PCI devices.
>
> The CRB command buffer currently is a RAM MemoryRegion and given
> its base address alignment, it causes an error report on
> vfio_listener_region_add(). This series proposes to use a ram-device
> region instead which helps in better assessing the dma map error
> failure on VFIO side.
>
> Best Regards
>
> Eric
>
> This series can be found at:
> https://github.com/eauger/qemu/tree/tpm-crb-ram-device-v2
>
> History:
> v1 -> v2:
> - added tpm_crb_unrealize (dared to keep Stefan's T-b though)
>
> 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/meson.build   |  2 +-
>   hw/tpm/tpm_crb.c     | 22 ++++++++++++++++++++--
>   hw/vfio/common.c     | 15 ++++++++++++++-
>   hw/vfio/trace-events |  1 +
>   4 files changed, 36 insertions(+), 4 deletions(-)
>
Acked-by: Stefan Berger <stefanb@linux.ibm.com>

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

[Philippe Mathieu-Daudé via]

On 18/1/22 16:33, Eric Auger wrote:
> 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.
> 
> The change in meson.build is required to include the
> cpu.h header.
> 
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> 
> ---
> 
> v1 -> v2:
> - Add tpm_crb_unrealize
> ---
>   hw/tpm/meson.build |  2 +-
>   hw/tpm/tpm_crb.c   | 22 ++++++++++++++++++++--
>   2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> index 1c68d81d6a..3e74df945b 100644
> --- a/hw/tpm/meson.build
> +++ b/hw/tpm/meson.build
> @@ -1,8 +1,8 @@
>   softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_tis_common.c'))
>   softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true: files('tpm_tis_isa.c'))
>   softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
> -softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
>   
> +specific_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))

We don't need to make this file target-specific.

>   specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true: files('tpm_ppi.c'))
>   specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true: files('tpm_ppi.c'))
>   specific_ss.add(when: 'CONFIG_TPM_SPAPR', if_true: files('tpm_spapr.c'))
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index 58ebd1469c..6ec19a9911 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 "cpu.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(ç,
> +                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));

HOST_PAGE_ALIGN() and qemu_real_host_page_size() actually belong
to "exec/cpu-common.h".

Alex, could you hold on a few days for this patch? I am going to send
a cleanup series. Otherwise no worry, I will clean this on top too.

Thanks,

Phil.

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

[Alex Williamson]

On Wed, 19 Jan 2022 23:46:19 +0100
Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:

> On 18/1/22 16:33, Eric Auger wrote:
> > 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.
> > 
> > The change in meson.build is required to include the
> > cpu.h header.
> > 
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Stefan Berger <stefanb@linux.ibm.com>
> > 
> > ---
> > 
> > v1 -> v2:
> > - Add tpm_crb_unrealize
> > ---
> >   hw/tpm/meson.build |  2 +-
> >   hw/tpm/tpm_crb.c   | 22 ++++++++++++++++++++--
> >   2 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/tpm/meson.build b/hw/tpm/meson.build
> > index 1c68d81d6a..3e74df945b 100644
> > --- a/hw/tpm/meson.build
> > +++ b/hw/tpm/meson.build
> > @@ -1,8 +1,8 @@
> >   softmmu_ss.add(when: 'CONFIG_TPM_TIS', if_true: files('tpm_tis_common.c'))
> >   softmmu_ss.add(when: 'CONFIG_TPM_TIS_ISA', if_true: files('tpm_tis_isa.c'))
> >   softmmu_ss.add(when: 'CONFIG_TPM_TIS_SYSBUS', if_true: files('tpm_tis_sysbus.c'))
> > -softmmu_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))
> >   
> > +specific_ss.add(when: 'CONFIG_TPM_CRB', if_true: files('tpm_crb.c'))  
> 
> We don't need to make this file target-specific.
> 
> >   specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_TIS'], if_true: files('tpm_ppi.c'))
> >   specific_ss.add(when: ['CONFIG_SOFTMMU', 'CONFIG_TPM_CRB'], if_true: files('tpm_ppi.c'))
> >   specific_ss.add(when: 'CONFIG_TPM_SPAPR', if_true: files('tpm_spapr.c'))
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > index 58ebd1469c..6ec19a9911 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 "cpu.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(ç,
> > +                                HOST_PAGE_ALIGN(CRB_CTRL_CMD_SIZE));  
> 
> HOST_PAGE_ALIGN() and qemu_real_host_page_size() actually belong
> to "exec/cpu-common.h".
> 
> Alex, could you hold on a few days for this patch? I am going to send
> a cleanup series. Otherwise no worry, I will clean this on top too.

Sure.  Thanks,

Alex