RE: [EXT] Re: [PATCH v3] virtio-pci: correctly set virtio pci queue mem multiplier

[Srujana Challa]

> Subject: [EXT] Re: [PATCH v3] virtio-pci: correctly set virtio pci queue mem
> multiplier
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Fri, Feb 23, 2024 at 10:56:17AM +0530, Srujana Challa wrote:
> > Currently, virtio_pci_queue_mem_mult function always returns 4K when
> > VIRTIO_PCI_FLAG_PAGE_PER_VQ is set. But this won't work for vhost vdpa
> > when host has page size other than 4K.
> > This patch introduces a new property(host-page-per-vq) for vdpa use
> > case to fix the same.
> >
> > Signed-off-by: Srujana Challa <schalla@marvell.com>
> 
> Looks good. I'd like to fail realize if both
>    (proxy->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ) and
>    (proxy->flags & VIRTIO_PCI_FLAG_HOST_PAGE_PER_VQ) so users do not
> start depending on this combination.
Could you confirm if we can add assertion for this case in 
virtio_pci_mem_mult() function?

> 
> 
> 
> > ---
> > v2->v3:
> > - Modified property name, page-per-vdpa-vq to host-page-per-vq.
> >
> > v1->v2:
> > - Introduced a new property to get virtqueue mem multiplier for
> >   vdpa use case.
> >
> >  hw/virtio/virtio-pci.c         | 10 ++++++++--
> >  include/hw/virtio/virtio-pci.h |  5 +++++
> >  2 files changed, 13 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index
> > 1a7039fb0c..f29e60830b 100644
> > --- a/hw/virtio/virtio-pci.c
> > +++ b/hw/virtio/virtio-pci.c
> > @@ -320,8 +320,12 @@ static bool
> > virtio_pci_ioeventfd_enabled(DeviceState *d)
> >
> >  static inline int virtio_pci_queue_mem_mult(struct VirtIOPCIProxy
> > *proxy)  {
> > -    return (proxy->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ) ?
> > -        QEMU_VIRTIO_PCI_QUEUE_MEM_MULT : 4;
> > +    if (proxy->flags & VIRTIO_PCI_FLAG_PAGE_PER_VQ)
> > +        return QEMU_VIRTIO_PCI_QUEUE_MEM_MULT;
> > +    else if (proxy->flags & VIRTIO_PCI_FLAG_HOST_PAGE_PER_VQ)
> > +        return qemu_real_host_page_size();
> > +    else
> > +        return 4;
> >  }
> >
> >  static int virtio_pci_ioeventfd_assign(DeviceState *d, EventNotifier
> > *notifier, @@ -2301,6 +2305,8 @@ static Property virtio_pci_properties[] =
> {
> >                      VIRTIO_PCI_FLAG_INIT_FLR_BIT, true),
> >      DEFINE_PROP_BIT("aer", VirtIOPCIProxy, flags,
> >                      VIRTIO_PCI_FLAG_AER_BIT, false),
> > +    DEFINE_PROP_BIT("host-page-per-vq", VirtIOPCIProxy, flags,
> > +                    VIRTIO_PCI_FLAG_HOST_PAGE_PER_VQ_BIT, false),
> >      DEFINE_PROP_END_OF_LIST(),
> >  };
> >
> > diff --git a/include/hw/virtio/virtio-pci.h
> > b/include/hw/virtio/virtio-pci.h index 59d88018c1..81b6de4291 100644
> > --- a/include/hw/virtio/virtio-pci.h
> > +++ b/include/hw/virtio/virtio-pci.h
> > @@ -43,6 +43,7 @@ enum {
> >      VIRTIO_PCI_FLAG_INIT_FLR_BIT,
> >      VIRTIO_PCI_FLAG_AER_BIT,
> >      VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT,
> > +    VIRTIO_PCI_FLAG_HOST_PAGE_PER_VQ_BIT,
> >  };
> >
> >  /* Need to activate work-arounds for buggy guests at vmstate load. */
> > @@ -89,6 +90,10 @@ enum {  #define
> VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED \
> >    (1 << VIRTIO_PCI_FLAG_ATS_PAGE_ALIGNED_BIT)
> >
> > +/* page per vdpa vq flag to be used for vhost vdpa backends */
> > +#define VIRTIO_PCI_FLAG_HOST_PAGE_PER_VQ \
> > +    (1 << VIRTIO_PCI_FLAG_HOST_PAGE_PER_VQ_BIT)
> > +
> >  typedef struct {
> >      MSIMessage msg;
> >      int virq;
> > --
> > 2.25.1