Re: [PATCH v2] Add page_per_vq flag to the 'driver' element of virtio devices

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, 2021-05-05 at 10:25 +0300, Gavi Teitz wrote:
> https://bugzilla.redhat.com/show_bug.cgi?id=1925363
> 
> Add support for setting the page-per-vq flag, which is important for
> vdpa with vhost-user performance.
> 
> Signed-off-by: Gavi Teitz <gavi@xxxxxxxxxx>
> ---
>  docs/formatdomain.rst                              | 11 +++++-
>  docs/schemas/domaincommon.rng                      |  5 +++
>  src/conf/domain_conf.c                             | 10 +++++
>  src/conf/domain_conf.h                             |  1 +
>  src/qemu/qemu_command.c                            |  5 +++
>  src/qemu/qemu_hotplug.c                            |  1 +
>  .../net-virtio-page-per-vq.x86_64-latest.args      | 38
> ++++++++++++++++++
>  tests/qemuxml2argvdata/net-virtio-page-per-vq.xml  | 29
> ++++++++++++++
>  tests/qemuxml2argvtest.c                           |  1 +
>  .../net-virtio-page-per-vq.x86_64-latest.xml       | 46
> ++++++++++++++++++++++
>  tests/qemuxml2xmltest.c                            |  1 +
>  11 files changed, 147 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/net-virtio-page-per-
> vq.x86_64-latest.args
>  create mode 100644 tests/qemuxml2argvdata/net-virtio-page-per-vq.xml
>  create mode 100644 tests/qemuxml2xmloutdata/net-virtio-page-per-
> vq.x86_64-latest.xml
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index fa5c14f..ce7a537 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -5120,7 +5120,7 @@ Setting NIC driver-specific options
>         <source network='default'/>
>         <target dev='vnet1'/>
>         <model type='virtio'/>
> -       <driver name='vhost' txmode='iothread' ioeventfd='on'
> event_idx='off' queues='5' rx_queue_size='256' tx_queue_size='256'>
> +       <driver name='vhost' txmode='iothread' ioeventfd='on'
> event_idx='off' queues='5' rx_queue_size='256' tx_queue_size='256'
> page_per_vq='on'>
>           <host csum='off' gso='off' tso4='off' tso6='off' ecn='off'
> ufo='off' mrg_rxbuf='off'/>
>           <guest csum='off' tso4='off' tso6='off' ecn='off'
> ufo='off'/>
>         </driver>
> @@ -5215,6 +5215,15 @@ following attributes are available for the
> ``"virtio"`` NIC driver:
>     only for ``vhostuser`` type. :since:`Since 3.7.0 (QEMU and KVM
> only)`
>     **In general you should leave this option alone, unless you are
> very certain
>     you know what you are doing.**
> +``page_per_vq``
> +   This optional attribute controls the layout of the notification
> capabilities
> +   exposed to the guest. When enabled, each virtio queue will have a
> dedicated
> +   page on the device BAR exposed to the guest. It is recommended to
> be used when
> +   vDPA is enabled on the hypervisor, as it enables mapping the
> notification area
> +   to the physical device, which is only supported in page
> granularity. The
> +   default is determined by QEMU; as off. :since:`Since 7.4.0`
> +   **In general you should leave this option alone, unless you are
> very certain
> +   you know what you are doing.**
>  virtio options
>     For virtio interfaces, `Virtio-specific options
> <#elementsVirtio>`__ can also
>     be set. ( :since:`Since 3.5.0` )
> diff --git a/docs/schemas/domaincommon.rng
> b/docs/schemas/domaincommon.rng
> index a2e5c50..e61ad67 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3463,6 +3463,11 @@
>                  </attribute>
>                </optional>
>                <optional>
> +                <attribute name="page_per_vq">
> +                  <ref name="virOnOff"/>
> +                </attribute>
> +              </optional>
> +              <optional>
>                  <attribute name="txmode">
>                    <choice>
>                      <value>iothread</value>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index cb668d3..0e219ed 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -10948,6 +10948,12 @@ virDomainNetDefParseXML(virDomainXMLOption
> *xmlopt,
>              def->driver.virtio.tx_queue_size = q;
>          }
>  
> +        if ((tmpNode = virXPathNode("./driver", ctxt))) {
> +            if (virXMLPropTristateSwitch(tmpNode, "page_per_vq",
> VIR_XML_PROP_NONE,
> +                                         &def-
> >driver.virtio.page_per_vq) < 0)
> +                goto error;
> +        }
> +


I didn't notice this the last time I reviewed the code, but the rest of
the driver options (ioeventfd, etc) are parsed from the xml up above
(around line 10389) and then applied to the def structure down here. I
think that for consistency it would make more sense to follow that
example for this property as well. 

Jonathon


>          if ((tmpNode = virXPathNode("./driver/host", ctxt))) {
>              if (virXMLPropTristateSwitch(tmpNode, "csum",
> VIR_XML_PROP_NONE,
>                                           &def-
> >driver.virtio.host.csum) < 0)
> @@ -25221,6 +25227,10 @@ virDomainVirtioNetDriverFormat(virBuffer
> *buf,
>      if (def->driver.virtio.tx_queue_size)
>          virBufferAsprintf(buf, " tx_queue_size='%u'",
>                            def->driver.virtio.tx_queue_size);
> +    if (def->driver.virtio.page_per_vq) {
> +        virBufferAsprintf(buf, " page_per_vq='%s'",
> +                          virTristateSwitchTypeToString(def-
> >driver.virtio.page_per_vq));
> +    }
>  
>      virDomainVirtioOptionsFormat(buf, def->virtio);
>  }
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 85c318d..7aab565 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1027,6 +1027,7 @@ struct _virDomainNetDef {
>              virDomainNetVirtioTxModeType txmode;
>              virTristateSwitch ioeventfd;
>              virTristateSwitch event_idx;
> +            virTristateSwitch page_per_vq;
>              unsigned int queues; /* Multiqueue virtio-net */
>              unsigned int rx_queue_size;
>              unsigned int tx_queue_size;
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index ca2265c..3545ee9 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3629,6 +3629,11 @@ qemuBuildNicDevStr(virDomainDef *def,
>          if (net->driver.virtio.tx_queue_size)
>              virBufferAsprintf(&buf, ",tx_queue_size=%u", net-
> >driver.virtio.tx_queue_size);
>  
> +        if (net->driver.virtio.page_per_vq) {
> +            virBufferAsprintf(&buf, ",page-per-vq=%s",
> +                              virTristateSwitchTypeToString(net-
> >driver.virtio.page_per_vq));
> +        }
> +
>          if (net->mtu)
>              virBufferAsprintf(&buf, ",host_mtu=%u", net->mtu);
>  
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 444d89d..a244709 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -3578,6 +3578,7 @@ qemuDomainChangeNet(virQEMUDriver *driver,
>           olddev->driver.virtio.queues != newdev-
> >driver.virtio.queues ||
>           olddev->driver.virtio.rx_queue_size != newdev-
> >driver.virtio.rx_queue_size ||
>           olddev->driver.virtio.tx_queue_size != newdev-
> >driver.virtio.tx_queue_size ||
> +         olddev->driver.virtio.page_per_vq != newdev-
> >driver.virtio.page_per_vq ||
>           olddev->driver.virtio.host.csum != newdev-
> >driver.virtio.host.csum ||
>           olddev->driver.virtio.host.gso != newdev-
> >driver.virtio.host.gso ||
>           olddev->driver.virtio.host.tso4 != newdev-
> >driver.virtio.host.tso4 ||
> diff --git a/tests/qemuxml2argvdata/net-virtio-page-per-vq.x86_64-
> latest.args b/tests/qemuxml2argvdata/net-virtio-page-per-vq.x86_64-
> latest.args
> new file mode 100644
> index 0000000..5a52595
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/net-virtio-page-per-vq.x86_64-
> latest.args
> @@ -0,0 +1,38 @@
> +LC_ALL=C \
> +PATH=/bin \
> +HOME=/tmp/lib/domain--1-QEMUGuest1 \
> +USER=test \
> +LOGNAME=test \
> +XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
> +XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
> +XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
> +/usr/bin/qemu-system-i386 \
> +-name guest=QEMUGuest1,debug-threads=on \
> +-S \
> +-object '{"qom-
> type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/doma
> in--1-QEMUGuest1/master-key.aes"}' \
> +-machine pc,accel=tcg,usb=off,dump-guest-core=off,memory-
> backend=pc.ram \
> +-cpu qemu64 \
> +-m 214 \
> +-object '{"qom-type":"memory-backend-
> ram","id":"pc.ram","size":224395264}' \
> +-overcommit mem-lock=off \
> +-smp 1,sockets=1,cores=1,threads=1 \
> +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
> +-display none \
> +-no-user-config \
> +-nodefaults \
> +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
> +-mon chardev=charmonitor,id=monitor,mode=control \
> +-rtc base=utc \
> +-no-shutdown \
> +-no-acpi \
> +-boot strict=on \
> +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \
> +-blockdev
> '{"driver":"host_device","filename":"/dev/HostVG/QEMUGuest1","node-
> name":"libvirt-1-storage","auto-read-only":true,"discard":"unmap"}' \
> +-blockdev '{"node-name":"libvirt-1-format","read-
> only":false,"driver":"raw","file":"libvirt-1-storage"}' \
> +-device ide-hd,bus=ide.0,unit=0,drive=libvirt-1-format,id=ide0-0-
> 0,bootindex=1 \
> +-netdev user,id=hostnet0 \
> +-device virtio-net-pci,page-per-
> vq=on,netdev=hostnet0,id=net0,mac=00:11:22:33:44:55,bus=pci.0,addr=0x
> 2 \
> +-audiodev id=audio1,driver=none \
> +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \
> +-sandbox
> on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=de
> ny \
> +-msg timestamp=on
> diff --git a/tests/qemuxml2argvdata/net-virtio-page-per-vq.xml
> b/tests/qemuxml2argvdata/net-virtio-page-per-vq.xml
> new file mode 100644
> index 0000000..e22ecd6
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/net-virtio-page-per-vq.xml
> @@ -0,0 +1,29 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i386</emulator>
> +    <disk type='block' device='disk'>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +    </disk>
> +    <controller type='usb' index='0'/>
> +    <interface type='user'>
> +      <mac address='00:11:22:33:44:55'/>
> +      <model type='virtio'/>
> +      <driver page_per_vq='on'/>
> +    </interface>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index a9dafe2..3dc866a 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -1634,6 +1634,7 @@ mymain(void)
>              QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE);
>      DO_TEST_PARSE_ERROR("net-virtio-rxqueuesize-invalid-size",
>                          QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE);
> +    DO_TEST_CAPS_LATEST("net-virtio-page-per-vq");
>      DO_TEST("net-virtio-teaming",
>              QEMU_CAPS_VIRTIO_NET_FAILOVER,
>              QEMU_CAPS_DEVICE_VFIO_PCI);
> diff --git a/tests/qemuxml2xmloutdata/net-virtio-page-per-vq.x86_64-
> latest.xml b/tests/qemuxml2xmloutdata/net-virtio-page-per-vq.x86_64-
> latest.xml
> new file mode 100644
> index 0000000..34f7ee5
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/net-virtio-page-per-vq.x86_64-
> latest.xml
> @@ -0,0 +1,46 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
> +  <memory unit='KiB'>219100</memory>
> +  <currentMemory unit='KiB'>219100</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='i686' machine='pc'>hvm</type>
> +    <boot dev='hd'/>
> +  </os>
> +  <cpu mode='custom' match='exact' check='none'>
> +    <model fallback='forbid'>qemu64</model>
> +  </cpu>
> +  <clock offset='utc'/>
> +  <on_poweroff>destroy</on_poweroff>
> +  <on_reboot>restart</on_reboot>
> +  <on_crash>destroy</on_crash>
> +  <devices>
> +    <emulator>/usr/bin/qemu-system-i386</emulator>
> +    <disk type='block' device='disk'>
> +      <driver name='qemu' type='raw'/>
> +      <source dev='/dev/HostVG/QEMUGuest1'/>
> +      <target dev='hda' bus='ide'/>
> +      <address type='drive' controller='0' bus='0' target='0'
> unit='0'/>
> +    </disk>
> +    <controller type='usb' index='0' model='piix3-uhci'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
> function='0x2'/>
> +    </controller>
> +    <controller type='pci' index='0' model='pci-root'/>
> +    <controller type='ide' index='0'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x01'
> function='0x1'/>
> +    </controller>
> +    <interface type='user'>
> +      <mac address='00:11:22:33:44:55'/>
> +      <model type='virtio'/>
> +      <driver page_per_vq='on'/>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x02'
> function='0x0'/>
> +    </interface>
> +    <input type='mouse' bus='ps2'/>
> +    <input type='keyboard' bus='ps2'/>
> +    <audio id='1' type='none'/>
> +    <memballoon model='virtio'>
> +      <address type='pci' domain='0x0000' bus='0x00' slot='0x03'
> function='0x0'/>
> +    </memballoon>
> +  </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 7af6f90..93f29d6 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -444,6 +444,7 @@ mymain(void)
>      DO_TEST("net-virtio-rxtxqueuesize",
>              QEMU_CAPS_VIRTIO_NET_RX_QUEUE_SIZE,
>              QEMU_CAPS_VIRTIO_NET_TX_QUEUE_SIZE);
> +    DO_TEST_CAPS_LATEST("net-virtio-page-per-vq");
>      DO_TEST("net-virtio-teaming",
>              QEMU_CAPS_VIRTIO_NET_FAILOVER,
>              QEMU_CAPS_DEVICE_VFIO_PCI);





[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux