[PATCH RFC 07/16] virtio-pci: implement support for extended features.

Paolo Abeni posted 16 patches 5 months, 4 weeks ago
There is a newer version of this series
[PATCH RFC 07/16] virtio-pci: implement support for extended features.
Posted by Paolo Abeni 5 months, 4 weeks ago
Allow the common read/write operation to access all the
available features space.

Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 hw/virtio/virtio-pci.c         | 19 +++++++++++++------
 include/hw/virtio/virtio-pci.h |  2 +-
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 0fa8fe4955..7815ef2d9b 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -123,7 +123,8 @@ static const VMStateDescription vmstate_virtio_pci_modern_state_sub = {
     .fields = (const VMStateField[]) {
         VMSTATE_UINT32(dfselect, VirtIOPCIProxy),
         VMSTATE_UINT32(gfselect, VirtIOPCIProxy),
-        VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy, 2),
+        VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy,
+                             VIRTIO_FEATURES_WORDS),
         VMSTATE_STRUCT_ARRAY(vqs, VirtIOPCIProxy, VIRTIO_QUEUE_MAX, 0,
                              vmstate_virtio_pci_modern_queue_state,
                              VirtIOPCIQueue),
@@ -1490,10 +1491,10 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
         val = proxy->dfselect;
         break;
     case VIRTIO_PCI_COMMON_DF:
-        if (proxy->dfselect <= 1) {
+        if (proxy->dfselect < VIRTIO_FEATURES_WORDS) {
             VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
 
-            val = (vdev->host_features & ~vdc->legacy_features) >>
+            val = (vdev->host_features_ex & ~vdc->legacy_features) >>
                 (32 * proxy->dfselect);
         }
         break;
@@ -1585,10 +1586,16 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
         break;
     case VIRTIO_PCI_COMMON_GF:
         if (proxy->gfselect < ARRAY_SIZE(proxy->guest_features)) {
+            virtio_features_t features = 0;
+            int i;
+
             proxy->guest_features[proxy->gfselect] = val;
-            virtio_set_features(vdev,
-                                (((uint64_t)proxy->guest_features[1]) << 32) |
-                                proxy->guest_features[0]);
+            for (i = 0; i < VIRTIO_FEATURES_WORDS; ++i) {
+                virtio_features_t cur = proxy->guest_features[i];
+
+                features |= cur << (i * 32);
+            }
+            virtio_set_features(vdev, features);
         }
         break;
     case VIRTIO_PCI_COMMON_MSIX:
diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
index 31ec144509..c20b289e64 100644
--- a/include/hw/virtio/virtio-pci.h
+++ b/include/hw/virtio/virtio-pci.h
@@ -165,7 +165,7 @@ struct VirtIOPCIProxy {
     uint32_t nvectors;
     uint32_t dfselect;
     uint32_t gfselect;
-    uint32_t guest_features[2];
+    uint32_t guest_features[VIRTIO_FEATURES_WORDS];
     VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX];
 
     VirtIOIRQFD *vector_irqfd;
-- 
2.49.0
Re: [PATCH RFC 07/16] virtio-pci: implement support for extended features.
Posted by Akihiko Odaki 5 months, 3 weeks ago
Having a period in the subject is unusual.

On 2025/05/21 20:34, Paolo Abeni wrote:
> Allow the common read/write operation to access all the
> available features space.
> 
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
>   hw/virtio/virtio-pci.c         | 19 +++++++++++++------
>   include/hw/virtio/virtio-pci.h |  2 +-
>   2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 0fa8fe4955..7815ef2d9b 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -123,7 +123,8 @@ static const VMStateDescription vmstate_virtio_pci_modern_state_sub = {
>       .fields = (const VMStateField[]) {
>           VMSTATE_UINT32(dfselect, VirtIOPCIProxy),
>           VMSTATE_UINT32(gfselect, VirtIOPCIProxy),
> -        VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy, 2),
> +        VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy,
> +                             VIRTIO_FEATURES_WORDS),

Modifying existing fields breaks migration across versions. Please refer 
to docs/devel/migration/main.rst for details.

>           VMSTATE_STRUCT_ARRAY(vqs, VirtIOPCIProxy, VIRTIO_QUEUE_MAX, 0,
>                                vmstate_virtio_pci_modern_queue_state,
>                                VirtIOPCIQueue),
> @@ -1490,10 +1491,10 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
>           val = proxy->dfselect;
>           break;
>       case VIRTIO_PCI_COMMON_DF:
> -        if (proxy->dfselect <= 1) {
> +        if (proxy->dfselect < VIRTIO_FEATURES_WORDS) {
>               VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>   
> -            val = (vdev->host_features & ~vdc->legacy_features) >>
> +            val = (vdev->host_features_ex & ~vdc->legacy_features) >>
>                   (32 * proxy->dfselect);
>           }
>           break;
> @@ -1585,10 +1586,16 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
>           break;
>       case VIRTIO_PCI_COMMON_GF:
>           if (proxy->gfselect < ARRAY_SIZE(proxy->guest_features)) {
> +            virtio_features_t features = 0;
> +            int i;
> +
>               proxy->guest_features[proxy->gfselect] = val;
> -            virtio_set_features(vdev,
> -                                (((uint64_t)proxy->guest_features[1]) << 32) |
> -                                proxy->guest_features[0]);
> +            for (i = 0; i < VIRTIO_FEATURES_WORDS; ++i) {
> +                virtio_features_t cur = proxy->guest_features[i];
> +
> +                features |= cur << (i * 32);
> +            }
> +            virtio_set_features(vdev, features);
>           }
>           break;
>       case VIRTIO_PCI_COMMON_MSIX:
> diff --git a/include/hw/virtio/virtio-pci.h b/include/hw/virtio/virtio-pci.h
> index 31ec144509..c20b289e64 100644
> --- a/include/hw/virtio/virtio-pci.h
> +++ b/include/hw/virtio/virtio-pci.h
> @@ -165,7 +165,7 @@ struct VirtIOPCIProxy {
>       uint32_t nvectors;
>       uint32_t dfselect;
>       uint32_t gfselect;
> -    uint32_t guest_features[2];
> +    uint32_t guest_features[VIRTIO_FEATURES_WORDS];
>       VirtIOPCIQueue vqs[VIRTIO_QUEUE_MAX];
>   
>       VirtIOIRQFD *vector_irqfd;
Re: [PATCH RFC 07/16] virtio-pci: implement support for extended features.
Posted by Paolo Abeni 5 months, 3 weeks ago
On 5/23/25 9:23 AM, Akihiko Odaki wrote:
>> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>> index 0fa8fe4955..7815ef2d9b 100644
>> --- a/hw/virtio/virtio-pci.c
>> +++ b/hw/virtio/virtio-pci.c
>> @@ -123,7 +123,8 @@ static const VMStateDescription vmstate_virtio_pci_modern_state_sub = {
>>       .fields = (const VMStateField[]) {
>>           VMSTATE_UINT32(dfselect, VirtIOPCIProxy),
>>           VMSTATE_UINT32(gfselect, VirtIOPCIProxy),
>> -        VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy, 2),
>> +        VMSTATE_UINT32_ARRAY(guest_features, VirtIOPCIProxy,
>> +                             VIRTIO_FEATURES_WORDS),
> 
> Modifying existing fields breaks migration across versions. Please refer 
> to docs/devel/migration/main.rst for details.

Thanks for the pointer! I missed a lot of context. I guess I need some
trickery similar to the "virtio/64bit_features"/"virtio/128bit_features"
VMstate description.

/P