Re: [PATCH V4 4/6] mdev: introduce virtio device and its device ops

From: Jason Wang
Date: Mon Oct 21 2019 - 01:37:41 EST

On 2019/10/18 äå9:30, Cornelia Huck wrote:
On Fri, 18 Oct 2019 18:55:02 +0800
Jason Wang <jasowang@xxxxxxxxxx> wrote:

On 2019/10/18 äå5:46, Cornelia Huck wrote:
On Thu, 17 Oct 2019 18:48:34 +0800
Jason Wang <jasowang@xxxxxxxxxx> wrote:
+ * @get_vendor_id: Get virtio vendor id
+ * @mdev: mediated device
+ * Returns u32: virtio vendor id
How is the vendor id defined? As for normal virtio-pci devices?

The vendor that provides this device. So something like this

I notice that MMIO also had this so it looks to me it's not pci specific.
Ok. Would be good to specify this more explicitly.


+ * @get_status: Get the device status
+ * @mdev: mediated device
+ * Returns u8: virtio device status
+ * @set_status: Set the device status
+ * @mdev: mediated device
+ * @status: virtio device status
+ * @get_config: Read from device specific configuration space
+ * @mdev: mediated device
+ * @offset: offset from the beginning of
+ * configuration space
+ * @buf: buffer used to read to
+ * @len: the length to read from
+ * configration space
+ * @set_config: Write to device specific configuration space
+ * @mdev: mediated device
+ * @offset: offset from the beginning of
+ * configuration space
+ * @buf: buffer used to write from
+ * @len: the length to write to
+ * configration space
+ * @get_mdev_features: Get the feature of virtio mdev device
+ * @mdev: mediated device
+ * Returns the mdev features (API) support by
+ * the device.
What kind of 'features' are supposed to go in there? Are these bits,
like you defined for VIRTIO_MDEV_F_VERSION_1 above?

It's the API or mdev features other than virtio features. It could be
used by driver to determine the capability of the mdev device. Besides
_F_VERSION_1, we may add dirty page tracking etc which means we need new
device ops.
Ok, so that's supposed to be distinct bits that can be or'ed together?


Makes sense, but probably needs some more documentation somewhere.

Let me add some doc above this helper.

+ * @get_generation: Get device generaton
+ * @mdev: mediated device
+ * Returns u32: device generation
Is that callback mandatory?

I think so, it's hard to emulate that completely in virtio-mdev transport.
IIRC, the generation stuff is not mandatory in the current version of
virtio, as not all transports have that concept.

It looks to me we can have workaround as what has been done by virtio.c. Return 0 if this helper is not provided.

Generally, are any of the callbacks optional, or are all of them
mandatory? From what I understand, you plan to add new things that
depend on features... would that mean non-mandatory callbacks?

Yes, not all of them were mandatory, I will explicit explain which are mandatory and which are not.