* [PULL 0/2] VFIO fixes 2022-02-03 @ 2022-02-03 22:35 Alex Williamson 2022-02-03 22:35 ` [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region Alex Williamson ` (2 more replies) 0 siblings, 3 replies; 17+ messages in thread From: Alex Williamson @ 2022-02-03 22:35 UTC (permalink / raw) To: qemu-devel; +Cc: Eric Auger, Philippe Mathieu-Daudé, Stefan Berger 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(-) ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region 2022-02-03 22:35 [PULL 0/2] VFIO fixes 2022-02-03 Alex Williamson @ 2022-02-03 22:35 ` Alex Williamson 2022-02-04 12:08 ` Igor Mammedov 2022-02-03 22:36 ` [PULL 2/2] hw/vfio/common: Silence ram device offset alignment error traces Alex Williamson 2022-02-05 10:49 ` [PULL 0/2] VFIO fixes 2022-02-03 Peter Maydell 2 siblings, 1 reply; 17+ messages in thread From: Alex Williamson @ 2022-02-03 22:35 UTC (permalink / raw) To: qemu-devel; +Cc: Eric Auger, Philippe Mathieu-Daudé, Stefan Berger 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; ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region 2022-02-03 22:35 ` [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region Alex Williamson @ 2022-02-04 12:08 ` Igor Mammedov 2022-02-07 9:23 ` Eric Auger 2022-02-07 13:42 ` Eric Auger 0 siblings, 2 replies; 17+ messages in thread From: Igor Mammedov @ 2022-02-04 12:08 UTC (permalink / raw) To: Alex Williamson Cc: Eric Auger, Stefan Berger, qemu-devel, Philippe Mathieu-Daudé 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; > > > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region 2022-02-04 12:08 ` Igor Mammedov @ 2022-02-07 9:23 ` Eric Auger 2022-02-07 13:42 ` Eric Auger 1 sibling, 0 replies; 17+ messages in thread From: Eric Auger @ 2022-02-07 9:23 UTC (permalink / raw) To: Igor Mammedov, Alex Williamson Cc: Stefan Berger, qemu-devel, Philippe Mathieu-Daudé 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; >> >> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region 2022-02-04 12:08 ` Igor Mammedov 2022-02-07 9:23 ` Eric Auger @ 2022-02-07 13:42 ` Eric Auger 2022-02-23 13:02 ` Dr. David Alan Gilbert 1 sibling, 1 reply; 17+ messages in thread From: Eric Auger @ 2022-02-07 13:42 UTC (permalink / raw) To: Igor Mammedov, Alex Williamson Cc: Juan Quintela, Stefan Berger, qemu-devel, Dr. David Alan Gilbert, Philippe Mathieu-Daudé 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; >> >> >> ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region 2022-02-07 13:42 ` Eric Auger @ 2022-02-23 13:02 ` Dr. David Alan Gilbert 0 siblings, 0 replies; 17+ messages in thread From: Dr. David Alan Gilbert @ 2022-02-23 13:02 UTC (permalink / raw) To: Eric Auger Cc: Juan Quintela, qemu-devel, Philippe Mathieu-Daudé, Alex Williamson, Igor Mammedov, Stefan Berger * 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [PULL 2/2] hw/vfio/common: Silence ram device offset alignment error traces 2022-02-03 22:35 [PULL 0/2] VFIO fixes 2022-02-03 Alex Williamson 2022-02-03 22:35 ` [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region Alex Williamson @ 2022-02-03 22:36 ` Alex Williamson 2022-02-05 10:49 ` [PULL 0/2] VFIO fixes 2022-02-03 Peter Maydell 2 siblings, 0 replies; 17+ messages in thread From: Alex Williamson @ 2022-02-03 22:36 UTC (permalink / raw) To: qemu-devel; +Cc: Eric Auger, Philippe Mathieu-Daudé, Stefan Berger 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-03 22:35 [PULL 0/2] VFIO fixes 2022-02-03 Alex Williamson 2022-02-03 22:35 ` [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region Alex Williamson 2022-02-03 22:36 ` [PULL 2/2] hw/vfio/common: Silence ram device offset alignment error traces Alex Williamson @ 2022-02-05 10:49 ` Peter Maydell 2022-02-05 11:19 ` Philippe Mathieu-Daudé via 2022-02-07 15:50 ` Alex Williamson 2 siblings, 2 replies; 17+ messages in thread From: Peter Maydell @ 2022-02-05 10:49 UTC (permalink / raw) To: Alex Williamson Cc: Eric Auger, Stefan Berger, qemu-devel, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-05 10:49 ` [PULL 0/2] VFIO fixes 2022-02-03 Peter Maydell @ 2022-02-05 11:19 ` Philippe Mathieu-Daudé via 2022-02-07 9:10 ` Eric Auger 2022-02-07 15:50 ` Alex Williamson 1 sibling, 1 reply; 17+ messages in thread From: Philippe Mathieu-Daudé via @ 2022-02-05 11:19 UTC (permalink / raw) To: Eric Auger; +Cc: qemu-devel, Alex Williamson, Peter Maydell, Stefan Berger 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-05 11:19 ` Philippe Mathieu-Daudé via @ 2022-02-07 9:10 ` Eric Auger 0 siblings, 0 replies; 17+ messages in thread From: Eric Auger @ 2022-02-07 9:10 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Alex Williamson, qemu-devel, Stefan Berger 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. > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-05 10:49 ` [PULL 0/2] VFIO fixes 2022-02-03 Peter Maydell 2022-02-05 11:19 ` Philippe Mathieu-Daudé via @ 2022-02-07 15:50 ` Alex Williamson 2022-02-07 16:08 ` Philippe Mathieu-Daudé via 2022-02-07 16:20 ` Thomas Huth 1 sibling, 2 replies; 17+ messages in thread From: Alex Williamson @ 2022-02-07 15:50 UTC (permalink / raw) To: Peter Maydell Cc: Eric Auger, Stefan Berger, qemu-devel, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-07 15:50 ` Alex Williamson @ 2022-02-07 16:08 ` Philippe Mathieu-Daudé via 2022-02-07 16:54 ` Alex Williamson 2022-02-07 16:20 ` Thomas Huth 1 sibling, 1 reply; 17+ messages in thread From: Philippe Mathieu-Daudé via @ 2022-02-07 16:08 UTC (permalink / raw) To: Alex Williamson, Peter Maydell; +Cc: qemu-devel, Eric Auger, Stefan Berger 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-07 16:08 ` Philippe Mathieu-Daudé via @ 2022-02-07 16:54 ` Alex Williamson 2022-02-07 17:47 ` Alex Williamson 0 siblings, 1 reply; 17+ messages in thread From: Alex Williamson @ 2022-02-07 16:54 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Stefan Berger, qemu-devel, Eric Auger 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-07 16:54 ` Alex Williamson @ 2022-02-07 17:47 ` Alex Williamson 0 siblings, 0 replies; 17+ messages in thread From: Alex Williamson @ 2022-02-07 17:47 UTC (permalink / raw) To: Philippe Mathieu-Daudé Cc: Peter Maydell, Stefan Berger, qemu-devel, Eric Auger 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-07 15:50 ` Alex Williamson 2022-02-07 16:08 ` Philippe Mathieu-Daudé via @ 2022-02-07 16:20 ` Thomas Huth 2022-06-02 21:31 ` Alex Williamson 1 sibling, 1 reply; 17+ messages in thread From: Thomas Huth @ 2022-02-07 16:20 UTC (permalink / raw) To: Alex Williamson, Peter Maydell Cc: Eric Auger, Philippe Mathieu-Daudé, qemu-devel, Stefan Berger 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-02-07 16:20 ` Thomas Huth @ 2022-06-02 21:31 ` Alex Williamson 2022-06-02 22:15 ` Alex Williamson 0 siblings, 1 reply; 17+ messages in thread From: Alex Williamson @ 2022-06-02 21:31 UTC (permalink / raw) To: Thomas Huth Cc: Peter Maydell, Eric Auger, Stefan Berger, qemu-devel, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PULL 0/2] VFIO fixes 2022-02-03 2022-06-02 21:31 ` Alex Williamson @ 2022-06-02 22:15 ` Alex Williamson 0 siblings, 0 replies; 17+ messages in thread From: Alex Williamson @ 2022-06-02 22:15 UTC (permalink / raw) To: Thomas Huth Cc: Peter Maydell, Eric Auger, Stefan Berger, qemu-devel, Philippe Mathieu-Daudé 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2022-06-02 23:02 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-02-03 22:35 [PULL 0/2] VFIO fixes 2022-02-03 Alex Williamson 2022-02-03 22:35 ` [PULL 1/2] tpm: CRB: Use ram_device for "tpm-crb-cmd" region Alex Williamson 2022-02-04 12:08 ` Igor Mammedov 2022-02-07 9:23 ` Eric Auger 2022-02-07 13:42 ` Eric Auger 2022-02-23 13:02 ` Dr. David Alan Gilbert 2022-02-03 22:36 ` [PULL 2/2] hw/vfio/common: Silence ram device offset alignment error traces Alex Williamson 2022-02-05 10:49 ` [PULL 0/2] VFIO fixes 2022-02-03 Peter Maydell 2022-02-05 11:19 ` Philippe Mathieu-Daudé via 2022-02-07 9:10 ` Eric Auger 2022-02-07 15:50 ` Alex Williamson 2022-02-07 16:08 ` Philippe Mathieu-Daudé via 2022-02-07 16:54 ` Alex Williamson 2022-02-07 17:47 ` Alex Williamson 2022-02-07 16:20 ` Thomas Huth 2022-06-02 21:31 ` Alex Williamson 2022-06-02 22:15 ` Alex Williamson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.