[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]
Subject: Re: [PATCH v9 01/10] virtio: document forward compatibility guarantees
å 2022/11/24 20:05, Cornelia Huck åé:
On Thu, Nov 24 2022, "Michael S. Tsirkin" <mst@redhat.com> wrote:On Thu, Nov 24, 2022 at 03:34:19PM +0800, Jason Wang wrote:On Thu, Nov 24, 2022 at 2:59 PM Michael S. Tsirkin <mst@redhat.com> wrote:On Thu, Nov 24, 2022 at 12:33:52PM +0800, Jason Wang wrote:On Thu, Nov 24, 2022 at 5:08 AM Michael S. Tsirkin <mst@redhat.com> wrote:Feature negotiation forms the basis of forward compatibility guarantees of virtio but has never been properly documented. Do it now. Suggested-by: Halil Pasic <pasic@linux.ibm.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- content.tex | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/content.tex b/content.tex index 3051399..e3203be 100644 --- a/content.tex +++ b/content.tex @@ -114,21 +114,63 @@ \section{Feature Bits}\label{sec:Basic Facilities of a Virtio Device / Feature B In particular, new fields in the device configuration space are indicated by offering a new feature bit. +To keep te feature negotiation mechanism extensible, it is important +that devices \em{do not} offer any feature bits that they would not be +able to handle if the driver accepted them (even though drivers are not +supposed to accept them in the first place even if offered, according to +this version of the specification.)It looks to me if we want to clarify like this, feature negotiation is not sufficient. Do we need to do something similar in other basic facilities? Generally, we probably need to do this for facilities that are similar to features (status, virtqueue size and others).I'm not sure about "not sufficient". It's sufficient as long as you just want to extend features. What triggered this work is adding a transport specific feature.E.g: For status: Devices do not offer any status bit it would not be able to handle. For virtqueue size: Devices do not offer virtqueue size it would not be able to handle. ?Jason I think what you miss here is this part: "even though drivers are not supposed to accept them in the first place even if offered, according to this version of the specification" does not apply to status and virtqueue size. Let me clarify what all this means. It seems safe for a device to offer a reserved feature bit
This depends really on the behaviour of the drivers.
since drivers are not supposed to accept it.
So this is the case of the ADMIN_VQ.
This text says device must not rely on this. How would this apply to status or vq size? I don't see.Me neither... for the status, it's about either the driver noting its progress, or the device indicating that a reset is needed. The only case where setting something requires kind of an ack is FEATURES_OK, and there we already spell out the conditions clearly.
I basically meant something like:Assuming we have a feature like VIRTIO_RING_F_NEW and a new status bit was mapped to this feature, VIRTIO_CONFIG_S_NEW. And for some reason this feature is reserved for some transports. Should we mention device does not offer VIRTIO_CONFIG_S_NEW as well, or we assume it is implied that we don't offer VIRTIO_CONFIG_S_NEW in this case?
For the queue size, we specify that the device states what it can support, and that the driver may only reduce it, that seems clear enough to me.
Similar to the above, assuming a feature VIRTIO_R_F_MAXSIZE_XXX, and it is reserved. Should we mention that the new max virtqueue size should not be advertised or it is implied in the feature advertisement?
Thanks
[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]