Re: [virtio-dev] Re: [PATCH] vdpa/mlx5: set_features should allow reset to zero

On Mon, 1 Mar 2021 11:51:08 +0800
Jason Wang [off-list ref] wrote:

On 2021/3/1 5:25 上午, Michael S. Tsirkin wrote:

quoted

On Fri, Feb 26, 2021 at 04:19:16PM +0800, Jason Wang wrote:  

quoted

On 2021/2/26 2:53 上午, Michael S. Tsirkin wrote:  

quoted

quoted

quoted

Confused. What is wrong with the above? It never reads the
field unless the feature has been offered by device.  

So the spec said:

"

The following driver-read-only field, max_virtqueue_pairs only exists if
VIRTIO_NET_F_MQ is set.

"

If I read this correctly, there will be no max_virtqueue_pairs field if the
VIRTIO_NET_F_MQ is not offered by device? If yes the offsetof() violates
what spec said.

Thanks  

I think that's a misunderstanding. This text was never intended to
imply that field offsets change beased on feature bits.
We had this pain with legacy and we never wanted to go back there.

This merely implies that without VIRTIO_NET_F_MQ the field
should not be accessed. Exists in the sense "is accessible to driver".

Let's just clarify that in the spec, job done.  

Ok, agree. That will make things more eaiser.

Yes, that makes much more sense.

What about adding the following to the "Basic Facilities of a Virtio
Device/Device Configuration Space" section of the spec:

"If an optional configuration field does not exist, the corresponding
space is still present, but reserved."

(Do we need to specify what a device needs to do if the driver tries to
access a non-existing field? We cannot really make assumptions about
config space accesses; virtio-ccw can just copy a chunk of config space
that contains non-existing fields... I guess the device could ignore
writes and return zeroes on read?)

I've opened https://github.com/oasis-tcs/virtio-spec/issues/98 for the
spec issues.