* Eric Auger (eric.au...@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.william...@redhat.com> wrote: > > > >> From: Eric Auger <eric.au...@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.au...@redhat.com> > >> Tested-by: Stefan Berger <stef...@linux.ibm.com> > >> Acked-by: Stefan Berger <stef...@linux.ibm.com> > >> [PMD: Keep tpm_crb.c in meson's softmmu_ss] > >> Signed-off-by: Philippe Mathieu-Daudé <f4...@amsat.org> > >> Link: https://lore.kernel.org/r/20220120001242.230082-2-f4...@amsat.org > >> Signed-off-by: Alex Williamson <alex.william...@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 / dgilb...@redhat.com / Manchester, UK