On Wed, Nov 23, 2022 at 08:31:52AM +0100, Eugenio Perez Martin wrote: > On Wed, Nov 23, 2022 at 8:20 AM Marc-André Lureau > <[email protected]> wrote: > > > > Hi > > > > On Wed, Nov 23, 2022 at 12:32 AM Michael S. Tsirkin <[email protected]> wrote: > >> > >> On Tue, Nov 22, 2022 at 06:53:49PM +0400, [email protected] > >> wrote: > >> > From: Marc-André Lureau <[email protected]> > >> > > >> > 851d6d1a0f ("vfio/common: remove spurious tpm-crb-cmd misalignment > >> > warning") removed the warning on vfio_listener_region_add() path. > >> > > >> > An error is reported for vhost-vdpa case: > >> > qemu-kvm: vhost_vdpa_listener_region_add received unaligned region > >> > > >> > Skip the CRB device. > >> > > >> > Fixes: > >> > https://bugzilla.redhat.com/show_bug.cgi?id=2141965 > >> > > >> > Signed-off-by: Marc-André Lureau <[email protected]> > >> > --- > >> > hw/virtio/vhost-vdpa.c | 6 ++++++ > >> > 1 file changed, 6 insertions(+) > >> > > >> > diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c > >> > index 7468e44b87..9d7206e4b8 100644 > >> > --- a/hw/virtio/vhost-vdpa.c > >> > +++ b/hw/virtio/vhost-vdpa.c > >> > @@ -19,6 +19,7 @@ > >> > #include "hw/virtio/virtio-net.h" > >> > #include "hw/virtio/vhost-shadow-virtqueue.h" > >> > #include "hw/virtio/vhost-vdpa.h" > >> > +#include "sysemu/tpm.h" > >> > #include "exec/address-spaces.h" > >> > #include "migration/blocker.h" > >> > #include "qemu/cutils.h" > >> > @@ -46,6 +47,11 @@ static bool > >> > vhost_vdpa_listener_skipped_section(MemoryRegionSection *section, > >> > { > >> > Int128 llend; > >> > > >> > + if (TPM_IS_CRB(section->mr->owner)) { > >> > + /* The CRB command buffer has its base address unaligned. */ > >> > + return true; > >> > + } > >> > + > >> > >> Quite a hack. We can't really keep adding dependency on random devices > > > > > > Agree it's not great. but it's not strictly a dependency. At least you can > > still build with !CONFIG_TPM. > > > >> to vhost. And would you add hacks like this to listeners? > >> Pls figure out what's special about this buffer. > >> Also if this section is unaligned then doesn't it break up > >> other aligned sections? > > > > > > See the original discussion: > > https://patchew.org/QEMU/[email protected]/[email protected]/ > > > > Based on that, I'd tune the comment to something like: > QEMU represents the CRB cmd/response buffer as a standard RAM region > to the listeners, but real HW would not be able to access them.
But e.g. virtio can access it now with no issues, no? Is that a problem? Given it's TPM ... > Besides, these regions may not be properly page aligned. > > Is that more accurate? > > Thanks! > > > It is not clear whether aligning the tpm-crb-cmd region would work > > (overlapping tpm-crb-mmio). > > > > Peter Maydell said: "There's nothing that guarantees alignment for memory > > regions at all, whether they're RAM, IO or anything else.". > > > > Maybe vfio/vhost should simply skip those odd regions silently. > > > > > >> > >> > >> > if ((!memory_region_is_ram(section->mr) && > >> > !memory_region_is_iommu(section->mr)) || > >> > memory_region_is_protected(section->mr) || > >> > -- > >> > 2.38.1 > >>
