On 04.04.25 16:00, Halil Pasic wrote:
On Fri, 4 Apr 2025 15:48:49 +0200 David Hildenbrand david@redhat.com wrote:
Sounds good to me! But I'm still a little confused by the "holes". What confuses me is that i can think of at least 2 distinct types of "holes": 1) Holes that can be filled later. The queue conceptually exists, but there is no need to back it with any resources for now because it is dormant (it can be seen a hole in comparison to queues that need to materialize -- vring, notifiers, ...) 2) Holes that can not be filled without resetting the device: i.e. if certain features are not negotiated, then a queue X does not exist, but subsequent queues retain their index.
I think it is not about "negotiated", that might be the wrong terminology.
E.g., in QEMU virtio_balloon_device_realize() we define the virtqueues (virtio_add_queue()) if virtio_has_feature(s->host_features).
That is, it's independent of a feature negotiation (IIUC), it's static for the device -- "host_features"
Is that really "negotiated" or is it "the device offers the feature X" ?
It is offered. And this is precisely why I'm so keen on having a precise wording here.
Yes, me too. The current phrasing in the spec is not clear.
Linux similarly checks virtio_has_feature()->virtio_check_driver_offered_feature().
Usually for compatibility one needs negotiated. Because the feature negotiation is mostly about compatibility. I.e. the driver should be able to say, hey I don't know about that feature, and get compatible behavior. If for example VIRTIO_BALLOON_F_FREE_PAGE_HINT and VIRTIO_BALLOON_F_PAGE_REPORTING are both offered but only VIRTIO_BALLOON_F_PAGE_REPORTING is negotiated. That would make reporting_vq jump to +1 compared to the case where VIRTIO_BALLOON_F_FREE_PAGE_HINT is not offered. Which is IMHO no good, because for the features that the driver is going to reject in most of the cases it should not matter if it was offered or not.
Yes. The key part is that we may only add new features to the tail of our feature list; maybe we should document that as well.
I agree that a driver that implements VIRTIO_BALLOON_F_PAGE_REPORTING *must* be aware that VIRTIO_BALLOON_F_FREE_PAGE_HINT exists. So queue existence is not about feature negotiation but about features being offered from the device.
... which is a bit the same behavior as with fixed-assigned numbers a bit. VIRTIO_BALLOON_F_PAGE_REPORTING was documented as "4" because VIRTIO_BALLOON_F_FREE_PAGE_HINT was documented to be "3" -- IOW, it already existed in the spec.
Not perfect, but AFAIKS, not horrible.
(as Linux supports all these features, it's easy. A driver that only supports some features has to calculate the queue index manually based on the offered features)
@MST: Please correct me if I'm wrong!
Regards, Halil