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

["Michael S. Tsirkin" <mst@redhat.com>]

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.



> ---
> 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