On Tue, Sep 19, 2023 at 08:58:32AM +0200, Paolo Bonzini wrote: > On 9/19/23 02:35, Anton Yakovlev wrote: > > > > If the Linux virtio sound driver violates a specification, then there > > must be > > a conformance statement that the driver does not follow. As far as I know, > > there is no such thing at the moment. > > There is one in 2.7.13.3: "The device MAY access the descriptor chains the > driver created and the memory they refer to immediately" > > And likewise for packed virtqueues in 2.8.21.1: "The device MAY access the > descriptor and any following descriptors the driver created and the memory > they refer to immediately" > > I think it's a mistake to use MAY here, as opposed to "may". This is not an > optional feature, it's a MUST NOT requirement on the driver's part that > should be in 2.7.13.3.1 and 2.8.21.1.1. > > This does not prevent the virtio-snd spec from overriding this. If an > override is desirable (for example because other hardware behaves like > this), there should be a provision in 2.7.13.3.1 and 2.8.21.1.1. For > example: > > 2.7.13.3.1 Unless the device specification specifies otherwise, the driver > MUST NOT write to the descriptor chains and the memory they refer to, > between the /idx/ update and the time the device places the driver on the > used ring. > > 2.8.21.1.1 "Unless the device specification specifies otherwise, the driver > MUST NOT write to the descriptor, to any following descriptors the driver > created, nor to the memory the refer to, between the /flags/ update and the > time the device places the driver on the used ring. > > > In the virtio-snd there would be a normative statement like > > 5.14.6.8.1.1 The device MUST NOT read from available device-readable > buffers beyond the first buffer_bytes / period_bytes periods. > > 5.14.6.8.1.2 The driver MAY write to device-readable buffers beyond the > first buffer_bytes / period_bytes periods, even after offering them to the > device. > > > > As an aside, here are two other statements that have a similar issue: > > - 2.6.1.1.2 "the driver MAY release any resource associated with that > virtqueue" (instead 2.6.1.1.1 should have something like "After a queue has > been reset by the driver, the device MUST NOT access any resource associated > with a virtqueue"). > > - 2.7.5.1 "[the device] MAY do so for debugging or diagnostic purposes" > (this is not normative and can be just "may") The spec should not make an exception for virtio-sound because the virtqueue model was not intended as a shared memory mechanism. Allowing it would prevent message-passing implementations of virtqueues. Instead the device should use Shared Memory Regions: https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-10200010 BTW, the virtio-sound spec already has VIRTIO_SND_PCM_F_SHMEM_HOST and VIRTIO_SND_PCM_F_SHMEM_GUEST bits reserved but they currently have no meaning. I wonder what that was intended for? Stefan
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization
