RE: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

["Gonglei (Arei)" <arei.gonglei@huawei.com>]

Hi guys,

Thanks for your good comments, we should come to an agreement on droping version filed
In virtio crypto specific. And I'll update a new version soon.

Regards,
-Gonglei


> -----Original Message-----
> From: Zeng, Xin [mailto:xin.zeng@intel.com]
> Sent: Tuesday, August 09, 2016 9:42 PM
> To: Michael S. Tsirkin
> Cc: Gonglei (Arei); qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A; Ma,
> Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> Subject: RE: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
> 
> On Tuesday, August 9, 2016 6:58 PM Michael S. Tsirkin Wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Tuesday, August 9, 2016 6:58 PM
> > To: Zeng, Xin <xin.zeng@intel.com>
> > Cc: Gonglei (Arei) <arei.gonglei@huawei.com>; qemu-devel@nongnu.org;
> > virtio-dev@lists.oasis-open.org; Huangpeng (Peter)
> > <peter.huangpeng@huawei.com>; Luonengjun <luonengjun@huawei.com>;
> > cornelia.huck@de.ibm.com; stefanha@redhat.com;
> > denglingli@chinamobile.com; Jani Kokkonen <Jani.Kokkonen@huawei.com>;
> > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A
> > <brian.a.keating@intel.com>; Ma, Liang J <liang.j.ma@intel.com>; Griffin,
> > John <john.griffin@intel.com>; Hanweidong (Randy)
> > <hanweidong@huawei.com>; Huangweidong (C)
> > <weidong.huang@huawei.com>
> > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > specification
> >
> > On Mon, Aug 08, 2016 at 06:27:15AM +0000, Zeng, Xin wrote:
> > > On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > > > -----Original Message-----
> > > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > > Sent: Friday, August 05, 2016 3:56 AM
> > > > To: Gonglei (Arei)
> > > > Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > > > Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> > > > stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> > > > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A;
> > Ma,
> > > > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > > > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > > > specification
> > > >
> > > > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > > > +in future.It's currently defined for the version field:
> > > > > > > +
> > > > > > > +\begin{lstlisting}
> > > > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > > > >
> > > > > > Suggest to remove this macro,
> > > > > > Do you think a version which is composed of major version and minor
> > > > > > version is better?
> > > > > >
> > > > >
> > > > > I think we should tell the developer how to set the value of version
> > > > > field, but I have no idea about which value or form is better, so I
> > > > > used 1 as the first version. What's your opinion?
> > > >
> > > > My opinion is that you should drop this completely. We do feature bits,
> > not
> > > > version numbers in virtio. We do not want each device doing its own
> > thing for
> > > > compatibility.
> > > >
> > >
> > > But as I mentioned before,  considering the bug fix case, if each backend
> > device
> > > release need a feature bit meaning "some bugs are fixed", are the feature
> > bits
> > > enough?
> >
> > It depends on whether the bug is very entrenched and important. In most
> > cases, for minor bugs, it's better to just fix the bug in the driver or
> > the hypervisor and be done with it.  For cases where
> > that's not feasible because many drivers relied on a specific bug,
> > and the bug is very important, we can always add more
> > if we run out of feature bits.
> >
> > > Physical devices usually have a revision ID to mark its version,
> >
> > Because compatibility is one way (new devices usually need
> > new drivers).
> >
> 
> Yes, that's also why I propose to put version(revision) field into device
> configuration read-only filed instead of feature bits field.
> 
> > > could we have a
> > > revision Id field for each virtio device to distinguish the the virtio devices
> > which
> > > have the same feature sets but have different version?
> >
> > ccw has version negotiation. It was only changed once at the 0.9->1.0
> > transition so far, it's not used for bug fixes.  We could discuss adding
> > this to pci and mmio as well, but if yes this should be discussed
> > separately.
> >
> 
> Ok, that's good, I do think this is needed. I can initiate this discussion in
> another separated mail loop.
> 
> > So far no argument made here was crypto specific, so
> > let's not put this in the crypto device.
> >
> 
> Sure,  It is indeed not crypto device specific,  but probably all virtio
> devices needs. We can drop it from crypto device spec.
> Thanks!
> 
> >
> > > > --
> > > > MST