> Subject: Re: [EXT] Re: [PATCH v3] virtio-pci: correctly set virtio pci queue
> mem
> multiplier
>
> On Wed, Feb 28, 2024 at 06:13:03AM +0000, Srujana Challa wrote:
> > > 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 <scha...@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?
>
> No, reporting an error would be better since it's user-triggerable - it is
> not nice
> to report user errors through assertions, assertions are for conditions that
> can
> not be reached.
Yes, but returning error from virtio_pci_queue_mem_mult() would need changes
In multiple places right?
>
>
> > >
> > >
> > >
> > > > ---
> > > > 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
