Hi Parav,

On 11-06-24, 06:10, Parav Pandit wrote:

...

From: Viresh Kumar viresh.kumar@linaro.org +\subsection{Transport Requirements}\label{sec:Virtio Transport Options +/ Virtio Transport Requirements / Transport Requirements}

+A transport provides a mechanism for the driver to discover the device.

I would like to add a normative.

Some background on why normative statements are removed for this change:

https://lore.kernel.org/all/20240521073925-mutt-send-email-mst@kernel.org/

A transport MAY provide a mechanism to create and destroy virtio devices.

(for example PCI transport provides this).

It isn’t blocker for this patch. In case if you spin v6, please add, else will send this change after yours is merged.

...

+The device doesn't access or modify buffers on a virtqueue after it has +notified the driver about their availability.

This needs to be corrected to only say "modify".

A device may still access (read) what is already wrote, for example, used index of the split queue. Or packed queue flags.

Hmm, we are talking about "buffers on a virtqueue" here. I am not sure if the device should access them anymore. Once the control is passed to the driver, the buffers may get modified anytime and device shouldn't access them. Used index or flags are configuration things, accessing them is fine I guess.

...

+The device resets itself and the virtqueues if requested by the driver, +in a transport defined way, if the transport provides such a method.

+\subsection{Driver Requirements}\label{sec:Virtio Transport Options / +Virtio Transport Requirements / Driver Requirements}

+The driver acknowledges device notifications, as mandated by the +transport.

+The driver doesn't access virtqueue contents before the device notifies +about the readiness of the same.

This also needs fix. A driver may access (poll) for the packed queue flags or used index of the split q without device notifying it.

Maybe this (in a similar way to the device specific sentence earlier) ?

The driver doesn't access buffers on the virtqueue before the device notifies about the readiness of the same.

Configuration specific fields can of course be accessed anytime.

...

+The driver accesses queued buffers after the device has processed them +and notified the driver of their availability. This mechanism is +transport defined.

+The driver asks the device to reset itself and the virtqueues if, for +example, the driver times out waiting for a notification from the +device for a previously queued request.

Please remove "virtqueue" from above. The "device to reset itself" is enough. "and the virtqueues" is confusing, it sort of implies the vqs are something separate from the device reset flow, which is not true. So device reset is good enough.

I wonder if it will cause some confusion, i.e. about the state of the virtqueues on device reset. I understand what you are saying, but is it a given that virtqueues will be reset once device is ?

I looked at: "Basic Facilities of a Virtio Device" -> "Device Reset", and it doesn't talk about vitqueue being reset with the device.

Also there is a separate section about, "Virtqueue Reset" and it talks about the possibility of a virtqueue getting reset alone.

Maybe something like ?

The driver asks the device to reset itself (and its virtqueues) if, for example, the driver times out waiting for a notification from the device for a previously queued request.

Thanks.