On Tuesday, August 9, 2016 6:58 PM Michael S. Tsirkin Wrote: > -----Original Message----- > From: Michael S. Tsirkin [mailto:[email protected]] > Sent: Tuesday, August 9, 2016 6:58 PM > To: Zeng, Xin <[email protected]> > Cc: Gonglei (Arei) <[email protected]>; [email protected]; > [email protected]; Huangpeng (Peter) > <[email protected]>; Luonengjun <[email protected]>; > [email protected]; [email protected]; > [email protected]; Jani Kokkonen <[email protected]>; > [email protected]; [email protected]; Keating, Brian A > <[email protected]>; Ma, Liang J <[email protected]>; Griffin, > John <[email protected]>; Hanweidong (Randy) > <[email protected]>; Huangweidong (C) > <[email protected]> > 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:[email protected]] > > > Sent: Friday, August 05, 2016 3:56 AM > > > To: Gonglei (Arei) > > > Cc: Zeng, Xin; [email protected]; [email protected]; > > > Huangpeng (Peter); Luonengjun; [email protected]; > > > [email protected]; [email protected]; Jani Kokkonen; > > > [email protected]; [email protected]; 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
