Re: [PATCH] vhost-vdpa: skip TPM CRB memory section

[Michael S. Tsirkin]

On Wed, Nov 23, 2022 at 11:20:41AM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Nov 23, 2022 at 12:32 AM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Tue, Nov 22, 2022 at 06:53:49PM +0400, marcandre.lureau@redhat.com
>     wrote:
>     > From: Marc-André Lureau <marcandre.lureau@redhat.com>
>     >
>     > 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 <marcandre.lureau@redhat.com>
>     > ---
>     >  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.

but what does it have to do with 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/20220208133842.112017-1-eric.auger@redhat.com/
> 20220208133842.112017-2-eric.auger@redhat.com/
> 
> 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.


How do we detect them? Are these regions not DMA-able on real hardware?

> 
> 
> 
> 
>     >      if ((!memory_region_is_ram(section->mr) &&
>     >           !memory_region_is_iommu(section->mr)) ||
>     >          memory_region_is_protected(section->mr) ||
>     > --
>     > 2.38.1
> 
>