On 1/13/22 09:40, Eric Auger wrote:
Hi Stefan,
On 1/13/22 3:06 PM, Stefan Berger wrote:
On 1/13/22 05:37, 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.au...@redhat.com>
---
hw/tpm/meson.build | 2 +-
hw/tpm/tpm_crb.c | 10 ++++++++--
2 files changed, 9 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..25f8e685e4 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));
+
Do we need an unrealize function now to qemu_vfree() this memory?
I would say it is needed if the device can be hot-unplugged.
tpmppi->buf is not freeed either.
Correct about PPI. My main concern would be the CRB related test cases
that likely currently run without PPI but now could complain about a
memory leak upon shutdown. I tried to compile with --enable-sanitizers
and run the tests but it doesn't compile when the sanitizers are enabled.
FAILED: libcommon.fa.p/disas_i386.c.o
cc -m64 -mcx16 -Ilibcommon.fa.p -I../capstone/include/capstone
-I../dtc/libfdt -I../slirp -I../slirp/src -I/usr/include/pixman-1
-I/usr/include/p11-kit-1 -I/usr/include/glib-2.0
-I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4
-I/usr/include/libmount -I/usr/include/blkid -I/usr/include/gio-unix-2.0
-fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g
-isystem /home/stefanb/dev/qemu/linux-headers -isystem linux-headers
-iquote . -iquote /home/stefanb/dev/qemu -iquote
/home/stefanb/dev/qemu/include -iquote
/home/stefanb/dev/qemu/disas/libvixl -iquote
/home/stefanb/dev/qemu/tcg/i386 -pthread -fsanitize=undefined
-fsanitize=address -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -D_GNU_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
-Wredundant-decls -Wundef -Wwrite-strings -Wmissing-prototypes
-fno-strict-aliasing -fno-common -fwrapv -Wold-style-declaration
-Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k
-Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs
-Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2
-Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi
-fstack-protector-strong -fPIE -D_DEFAULT_SOURCE -D_XOPEN_SOURCE=600
-DNCURSES_WIDECHAR=1 -DSTRUCT_IOVEC_DEFINED -MD -MQ
libcommon.fa.p/disas_i386.c.o -MF libcommon.fa.p/disas_i386.c.o.d -o
libcommon.fa.p/disas_i386.c.o -c ../disas/i386.c
In file included from /usr/include/string.h:519,
from /home/stefanb/dev/qemu/include/qemu/osdep.h:87,
from ../disas/i386.c:34:
In function ?strcpy?,
inlined from ?PNI_Fixup? at ../disas/i386.c:6434:4,
inlined from ?PNI_Fixup? at ../disas/i386.c:6400:1:
/usr/include/bits/string_fortified.h:79:10: error: ?__builtin_memcpy?
offset [0, 7] is out of the bounds [0, 0] [-Werror=array-bounds]
79 | return __builtin___strcpy_chk (__dest, __src, __glibc_objsize
(__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In function ?strcpy?,
inlined from ?PNI_Fixup? at ../disas/i386.c:6427:4,
inlined from ?PNI_Fixup? at ../disas/i386.c:6400:1:
/usr/include/bits/string_fortified.h:79:10: error: ?__builtin_memcpy?
offset [0, 5] is out of the bounds [0, 0] [-Werror=array-bounds]
79 | return __builtin___strcpy_chk (__dest, __src, __glibc_objsize
(__dest));
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Stefan
Thanks
Eric
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);
