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

[Chen, Tiejun]

On 2015/2/6 20:14, 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>
> > ---
> >   hw/9pfs/virtio-9p.h                |  2 --
> >   include/hw/virtio/virtio-balloon.h |  3 ---
> >   include/hw/virtio/virtio-blk.h     |  3 ---
> >   include/hw/virtio/virtio-rng.h     |  3 ---
> >   include/hw/virtio/virtio-scsi.h    |  3 ---
> >   include/hw/virtio/virtio-serial.h  |  3 ---
> >   include/hw/virtio/virtio.h         | 16 ++++++++++++++++
> >   pc-bios/s390-ccw/virtio.h          |  8 +-------
> >   8 files changed, 17 insertions(+), 24 deletions(-)
>
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index f24997d..9ad6bb2 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 Linux's linux/virtio_ids.h */
>
> Why not refer to the virtio spec instead? :) And maybe add in the ids

Actually they're same now but this really is a potential risk.

> that already have been reserved.

So what about this?

@@ -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 */
+
 /* from Linux's linux/virtio_config.h */

 /* Status byte for guest to report progress, and synchronize features. */


> > +
> > +enum virtio_dev_type {
> > +    VIRTIO_ID_RESERVED  = 0, /* invalid virtio device */
> > +    VIRTIO_ID_NET = 1, /* virtio net */
> > +    VIRTIO_ID_BLOCK    = 2, /* virtio block */
> > +    VIRTIO_ID_CONSOLE = 3, /* virtio console */
> > +    VIRTIO_ID_RNG = 4, /* virtio rng */
> > +    VIRTIO_ID_BALLOON = 5, /* virtio balloon */
>
> /* virtio balloon (legacy) */
>
> > +    VIRTIO_ID_RPMSG    = 7, /* virtio remote processor messaging */
> > +    VIRTIO_ID_SCSI = 8, /* virtio scsi */
> > +    VIRTIO_ID_9P = 9, /* 9p virtio console */
> > +    VIRTIO_ID_RPROC_SERIAL = 11, /* virtio remoteproc serial link */
> > +    VIRTIO_ID_CAIF = 12, /* Virtio caif */
> > +};
> > +
> >   /* from Linux's linux/virtio_config.h */
> >
> >   /* Status byte for guest to report progress, and synchronize features. */
> > diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
> > index c23466b..2eabcb4 100644
> > --- a/pc-bios/s390-ccw/virtio.h
> > +++ b/pc-bios/s390-ccw/virtio.h
> > @@ -11,6 +11,7 @@
> >   #ifndef VIRTIO_H
> >   #define VIRTIO_H
> >
> > +#include "hw/virtio/virtio.h"
>
> This won't work, the bios can't use the common headers.

Thanks for your caught.

> >   #include "s390-ccw.h"
> >
> >   /* Status byte for guest to report progress, and synchronize features. */
> > @@ -23,13 +24,6 @@
> >   /* We've given up on this device. */
> >   #define VIRTIO_CONFIG_S_FAILED          0x80
> >
> > -enum virtio_dev_type {
> > -    VIRTIO_ID_NET = 1,
> > -    VIRTIO_ID_BLOCK = 2,
> > -    VIRTIO_ID_CONSOLE = 3,
> > -    VIRTIO_ID_BALLOON = 5,
> > -};
>
> Even though this one is incomplete; but we don't need anything but the
> block id anyway.
>
> > -
> >   struct virtio_dev_header {
> >       enum virtio_dev_type type : 8;
> >       u8  num_vq;

I will remove all s390 stuff in this patch.

Thanks
Tiejun