virtio — archive

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

On Thu, Aug 18, 2022 at 03:39:58PM +0200, Halil Pasic wrote: > On Tue, 16 Aug 2022 11:48:43 -0400 > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > On Tue, Aug 16, 2022 at 04:48:11PM +0200, Halil Pasic wrote: > > > On Fri, 12 Aug 2022 13:19:20 -0400 > > > "Michael S. Tsirkin" <mst@redhat.com> wrote: > > > > > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > > > --- > > > > content.tex | 10 ++++++++++ > > > > 1 file changed, 10 insertions(+) > > > > > > > > diff --git a/content.tex b/content.tex > > > > index 76b5a28..53be680 100644 > > > > --- a/content.tex > > > > +++ b/content.tex > > > > @@ -2668,6 +2668,16 @@ \subsubsection{Handling Device Features}\label{sec:Virtio Transport Options / Vi > > > > uses the CCW_CMD_WRITE_FEAT command, denoting a \field{features}/\field{index} > > > > combination. > > > > > > > > +\devicenormative{\paragraph}{Handling Device Features}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Handling Device Features} > > > > + > > > > +Device MUST NOT set bit VIRTIO_F_ADMIN_VQ (bit 41) in > > > > +DeviceFeatures. > > > > + > > > > +\drivernormative{\paragraph}{Handling Device Features}{Virtio Transport Options / Virtio over channel I/O / Device Initialization / Handling Device Features} > > > > + > > > > +Driver MUST NOT set bit VIRTIO_F_ADMIN_VQ (bit 41) in > > > > +DriverFeatures even if offered by the device. > > > > + > > > > > > I'm not sure I understand the intention here. I believe what we try to > > > accomplish here is the following. The Channel I/O transport *currently* > > > does not support the VIRTIO_F_ADMIN_VQ feature. It is not like we want > > > to state that the feature VIRTIO_F_ADMIN_VQ won't ever be supported by > > > the Channel I/O transport. Or am I wrong? > > > > > > If my assumptions are right, then the old incarnation of the spec could > > > contradict the new incarnation of the spec. Thus I would prefer something > > > like. > > > > Relaxing requirenents is always okay. > > Are you telling me, that for instance a driver author may not rely on > even the MUST type device normative behavior stated by the spec, because > future incarnations of the spec could relax the requirements towards this > particular device, for example by removing that device normative > statement? > I always imagined, if the spec says the device or the driver MUST > "something", then I as the implementer of the other end (driver or > device, can rely on that "something"). If this assumption is wrong then > I'm have to re-examine my entire mental model of the spec. Generally yes. Not if we explicitly tell it not to. Like here: +Driver MUST NOT set bit VIRTIO_F_ADMIN_VQ (bit 41) in +DriverFeatures even if offered by the device. This makes sure that drivers do not make an assumption that devices do not set the bit. But yes, maybe spell it out: +Driver MUST NOT set bit VIRTIO_F_ADMIN_VQ (bit 41) in +DriverFeatures even if offered by the device. +Driver MUST NOT assume that device does not offer VIRTIO_F_ADMIN_VQ. +In particular driver MUST NOT fail feature negotiation if +device offers VIRTIO_F_ADMIN_VQ. ok now? > > > > > > > > > """ > > > Currently the following features are not supported by the Channel I/O > > > transport: > > > * VIRTIO_F_ADMIN_VQ > > > """ > > > > what I want to prevent is driver saying "oh device will not set ADMIN_VQ > > so it's ok to acknowledge it if offered, it is never offered even > > though it does not suport it". > > because then it becomes impossible to know when actually a new driver > > appears with actual support. > > Fair point! > > I would prefer a driver normative which goes like this: > > """ > A driver SHOULD NOT accept features (i.e. have code that would do so if > the feature is offered) if the feature is not supported by the driver > (e.g. because unsupported by the transport), even if the specification > implies that the device can not offer these features in the first place > (e.g. because the feature is not yet supported by the transport. > """ > > And a similar device normative as well, which just that it may not offer > such features. > > """ > Note: The rationale behind the [reference to the normative] is that > while some features can not be implemented within the boundaries of the > current virtio specification, future incarnations of the specificaton may > make such implementations possible. A most prominent example is optional > features dependent on optional virtio facilities whose transport specific > implementation is not yet specified for some transports. Should one end > gain the ability to support these features, the old implementation which > made the assumption that the other end will make sure these features are > not negotiated would end up negotiating something it can't actually > support. > """ > > > > > > > So, Maybe just add text > > > > Note: future versions of this specification will allow setting ADMIN_VQ > > for driver and device. Device MUST NOT assume driver does not > > acknowledge ADMIN_VQ if offered. > > I would not lean out of the window and promise something with regards to > future versions of this spec. > > > > > And similarly for drivers: > > > > Note: future versions of this specification will allow setting ADMIN_VQ > > for driver and device. Drivers MUST NOT assume ADMIN_VQ if not offered. > > > > I think we can then make a note which references the generic normative > for each feature affected where it suits us. > > > > > > > If we want, we can also state what needs to be done in general when > > > features are unsupported by the transport. And yes, that normative > > > material in my opinion. > > > > > > Regards, > > > Halil > > > > > > Are there other examples? I want to call out the list explicitly because > > it is so easy to enable an extra feature by mistake. > > > > I don't think CCW supports the shared memory yet... But I may be wrong. > > > > > > > \subsubsection{Device Configuration}\label{sec:Virtio Transport Options / Virtio over channel I/O / Device Initialization / Device Configuration} > > > > > > > > The device's configuration space is located in host memory. > >