Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs

[Cornelia Huck]

On Mon, 09 Feb 2015 15:10:17 +0800
"Chen, Tiejun" <address@hidden> wrote:

> On 2015/2/9 15:02, Michael S. Tsirkin wrote:
> > On Mon, Feb 09, 2015 at 03:01:15PM +0800, Chen, Tiejun wrote:
> >> On 2015/2/8 18:48, Michael S. Tsirkin wrote:
> >>> On Fri, Feb 06, 2015 at 01:14:46PM +0100, Cornelia Huck wrote:
> >>>> On Fri,  6 Feb 2015 13:41:26 +0800
> >>>> Tiejun Chen <address@hidden> wrote:
> >>>>
> >>>>> Actually we define these device IDs in virtio standard, so
> >>>>> we'd better put them into one common place to manage conveniently.
> >>>>> Here I also add VIRTIO_ID_RESERVE according to virtio spec.
> >>>>>
> >>>>> Signed-off-by: Tiejun Chen <address@hidden>
> >>>
> >>> We really should just write a script to import the headers
> >> >from the linux kernel.
> >>> They will need some tweaks to avoid dependencies on
> >>> linux/types, but this seems easy to do - better than
> >>> trying to keep things in sync manually.
> >>
> >> I prefer Cornelia's comment since actually we're trying to define a little
> >> bit according to a spec, so the following may be enough?
> >>
> >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> >> index f24997d..4afb0b7 100644
> >> --- a/include/hw/virtio/virtio.h
> >> +++ b/include/hw/virtio/virtio.h
> >> @@ -23,6 +23,22 @@
> >>   #include "hw/virtio/virtio-9p.h"
> >>   #endif
> >>
> >> +/* Refer to VirtIO Spec 1.0. */
> >> +
> >> +#define VIRTIO_ID_RESERVED  0           /* reserved (invalid)*/
> >> +#define VIRTIO_ID_NET       1           /* network card */
> >> +#define VIRTIO_ID_BLOCK     2           /* block device */
> >> +#define VIRTIO_ID_CONSOLE   3           /* console */
> >> +#define VIRTIO_ID_RNG       4           /* entropy source */
> >> +#define VIRTIO_ID_BALLOON   5           /* memory ballooning */
> >> +#define VIRTIO_ID_IOMEMORY  6           /* ioMemory */
> >> +#define VIRTIO_ID_RPMSG     7           /* rpmsg */
> >> +#define VIRTIO_ID_SCSI      8           /* SCSI host */
> >> +#define VIRTIO_ID_9P        9           /* 9P transport */
> >> +#define VIRTIO_ID_MAC80211_WALN 10      /* mac80211 wlan */
> >> +#define VIRTIO_ID_RPROC_SERIAL  11      /* rproc seria */
> >> +#define VIRTIO_ID_CAIF      12          /* virtio CAIF */

I like that.

> >> +
> >>   /* from Linux's linux/virtio_config.h */
> >>
> >>   /* Status byte for guest to report progress, and synchronize features. */
> >>
> >> Thanks
> >> Tiejun
> >
> > This still means each change has to be done in two places.

Well, we do need the changes in way more than two places, as every host
or guest has to collect the definitions on its own, no? (Granted, with
Linux and qemu you get most of the users; but it feels a bit strange
for a host implementation to collect information from one of its
guests. I really think that we should go back to the common root.
Didn't we have a BSD-licenced header in the spec?)

> 
> Are you saying another head file, pc-bios/s390-ccw/virtio.h?
> 
> But seems Cornelia thought in case of s390-ccw, -quote-
> 
> "Even though this one is incomplete; but we don't need anything but the
> block id anyway."

Note that this is the s390-ccw _bios_, which is a very incomplete
implementation only containing the bare minimum needed to access a
virtio-blk root device for booting.