qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
@ 2015-02-06  5:41 Tiejun Chen
  2015-02-06 12:14 ` Cornelia Huck
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Tiejun Chen @ 2015-02-06  5:41 UTC (permalink / raw)
  To: aliguori, mst, amit.shah, cornelia.huck, borntraeger, agraf; +Cc: qemu-devel

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 <tiejun.chen@intel.com>
---
 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/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
index 2c3603a..228e05d 100644
--- a/hw/9pfs/virtio-9p.h
+++ b/hw/9pfs/virtio-9p.h
@@ -146,8 +146,6 @@ struct V9fsPDU
 
 /* from Linux's linux/virtio_9p.h */
 
-/* The ID for virtio console */
-#define VIRTIO_ID_9P    9
 #define MAX_REQ         128
 #define MAX_TAG_LEN     32
 
diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index f863bfe..6e0a775 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -24,9 +24,6 @@
 
 /* from Linux's linux/virtio_balloon.h */
 
-/* The ID for virtio_balloon */
-#define VIRTIO_ID_BALLOON 5
-
 /* The feature bitmap for virtio balloon */
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index 4652b70..6ee3e8f 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -25,9 +25,6 @@
 
 /* from Linux's linux/virtio_blk.h */
 
-/* The ID for virtio_block */
-#define VIRTIO_ID_BLOCK 2
-
 /* Feature bits */
 #define VIRTIO_BLK_F_BARRIER    0       /* Does host support barriers? */
 #define VIRTIO_BLK_F_SIZE_MAX   1       /* Indicates maximum segment size */
diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
index 14e85a5..e2bb6ce 100644
--- a/include/hw/virtio/virtio-rng.h
+++ b/include/hw/virtio/virtio-rng.h
@@ -21,9 +21,6 @@
 #define VIRTIO_RNG_GET_PARENT_CLASS(obj) \
         OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_RNG)
 
-/* The Virtio ID for the virtio rng device */
-#define VIRTIO_ID_RNG    4
-
 struct VirtIORNGConf {
     RngBackend *rng;
     uint64_t max_bytes;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index bf17cc9..9606f43 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -29,9 +29,6 @@
         OBJECT_CHECK(VirtIOSCSI, (obj), TYPE_VIRTIO_SCSI)
 
 
-/* The ID for virtio_scsi */
-#define VIRTIO_ID_SCSI  8
-
 /* Feature Bits */
 #define VIRTIO_SCSI_F_INOUT                    0
 #define VIRTIO_SCSI_F_HOTPLUG                  1
diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
index 11af978..1dcced6 100644
--- a/include/hw/virtio/virtio-serial.h
+++ b/include/hw/virtio/virtio-serial.h
@@ -20,9 +20,6 @@
 
 /* == Interface shared between the guest kernel and qemu == */
 
-/* The Virtio ID for virtio console / serial ports */
-#define VIRTIO_ID_CONSOLE		3
-
 /* Features supported */
 #define VIRTIO_CONSOLE_F_MULTIPORT	1
 
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 */
+
+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_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"
 #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,
-};
-
 struct virtio_dev_header {
     enum virtio_dev_type type : 8;
     u8  num_vq;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06  5:41 [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs Tiejun Chen
@ 2015-02-06 12:14 ` Cornelia Huck
  2015-02-08 10:48   ` Michael S. Tsirkin
  2015-02-09  6:56   ` Chen, Tiejun
  2015-02-06 16:28 ` Stefan Hajnoczi
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 17+ messages in thread
From: Cornelia Huck @ 2015-02-06 12:14 UTC (permalink / raw)
  To: Tiejun Chen; +Cc: mst, qemu-devel, agraf, borntraeger, aliguori, amit.shah

On Fri,  6 Feb 2015 13:41:26 +0800
Tiejun Chen <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>
> ---
>  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
that already have been reserved.

> +
> +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.

>  #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;

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06  5:41 [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs Tiejun Chen
  2015-02-06 12:14 ` Cornelia Huck
@ 2015-02-06 16:28 ` Stefan Hajnoczi
  2015-02-09  6:58   ` Chen, Tiejun
  2015-02-09 19:57 ` Michael S. Tsirkin
  2015-02-11 11:18 ` Amit Shah
  3 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2015-02-06 16:28 UTC (permalink / raw)
  To: Tiejun Chen
  Cc: borntraeger, mst, agraf, qemu-devel, cornelia.huck, aliguori,
	amit.shah

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Fri, Feb 06, 2015 at 01:41:26PM +0800, Tiejun Chen 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 <tiejun.chen@intel.com>
> ---
>  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(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06 12:14 ` Cornelia Huck
@ 2015-02-08 10:48   ` Michael S. Tsirkin
  2015-02-09  7:01     ` Chen, Tiejun
  2015-02-09  6:56   ` Chen, Tiejun
  1 sibling, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2015-02-08 10:48 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: qemu-devel, agraf, borntraeger, aliguori, amit.shah, Tiejun Chen

On Fri, Feb 06, 2015 at 01:14:46PM +0100, Cornelia Huck wrote:
> On Fri,  6 Feb 2015 13:41:26 +0800
> Tiejun Chen <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>

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.

-- 
MST

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06 12:14 ` Cornelia Huck
  2015-02-08 10:48   ` Michael S. Tsirkin
@ 2015-02-09  6:56   ` Chen, Tiejun
  1 sibling, 0 replies; 17+ messages in thread
From: Chen, Tiejun @ 2015-02-09  6:56 UTC (permalink / raw)
  To: Cornelia Huck; +Cc: mst, qemu-devel, agraf, borntraeger, aliguori, amit.shah

On 2015/2/6 20:14, Cornelia Huck wrote:
> On Fri,  6 Feb 2015 13:41:26 +0800
> Tiejun Chen <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>
>> ---
>>   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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06 16:28 ` Stefan Hajnoczi
@ 2015-02-09  6:58   ` Chen, Tiejun
  0 siblings, 0 replies; 17+ messages in thread
From: Chen, Tiejun @ 2015-02-09  6:58 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: borntraeger, mst, agraf, qemu-devel, cornelia.huck, aliguori,
	amit.shah

On 2015/2/7 0:28, Stefan Hajnoczi wrote:
> On Fri, Feb 06, 2015 at 01:41:26PM +0800, Tiejun Chen 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 <tiejun.chen@intel.com>
>> ---
>>   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(-)
>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Thanks for you review.

Just a little change will be introduced in next revision.

Tiejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-08 10:48   ` Michael S. Tsirkin
@ 2015-02-09  7:01     ` Chen, Tiejun
  2015-02-09  7:02       ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Chen, Tiejun @ 2015-02-09  7:01 UTC (permalink / raw)
  To: Michael S. Tsirkin, Cornelia Huck
  Cc: amit.shah, borntraeger, agraf, aliguori, qemu-devel

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

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

Thanks
Tiejun

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-09  7:01     ` Chen, Tiejun
@ 2015-02-09  7:02       ` Michael S. Tsirkin
  2015-02-09  7:10         ` Chen, Tiejun
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2015-02-09  7:02 UTC (permalink / raw)
  To: Chen, Tiejun
  Cc: borntraeger, qemu-devel, agraf, amit.shah, aliguori,
	Cornelia Huck

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 <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>
> >
> >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 */
> +
>  /* 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.
An automated script for copying headers would be much better IMHO.

-- 
MST

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-09  7:02       ` Michael S. Tsirkin
@ 2015-02-09  7:10         ` Chen, Tiejun
  2015-02-09  8:47           ` Cornelia Huck
  0 siblings, 1 reply; 17+ messages in thread
From: Chen, Tiejun @ 2015-02-09  7:10 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: borntraeger, qemu-devel, agraf, amit.shah, aliguori,
	Cornelia Huck

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 <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>
>>>
>>> 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 */
>> +
>>   /* 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.

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."

So I'm not sure we really should do this :)

Thanks
Tiejun

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-09  7:10         ` Chen, Tiejun
@ 2015-02-09  8:47           ` Cornelia Huck
  2015-02-11 12:41             ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Cornelia Huck @ 2015-02-09  8:47 UTC (permalink / raw)
  To: Chen, Tiejun
  Cc: Michael S. Tsirkin, qemu-devel, agraf, borntraeger, aliguori,
	amit.shah

On Mon, 09 Feb 2015 15:10:17 +0800
"Chen, Tiejun" <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>
> >>>
> >>> 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.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06  5:41 [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs Tiejun Chen
  2015-02-06 12:14 ` Cornelia Huck
  2015-02-06 16:28 ` Stefan Hajnoczi
@ 2015-02-09 19:57 ` Michael S. Tsirkin
  2015-02-11 11:55   ` Amit Shah
  2015-02-11 11:18 ` Amit Shah
  3 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2015-02-09 19:57 UTC (permalink / raw)
  To: Tiejun Chen
  Cc: borntraeger, qemu-devel, agraf, cornelia.huck, aliguori,
	amit.shah

On Fri, Feb 06, 2015 at 01:41:26PM +0800, Tiejun Chen 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 <tiejun.chen@intel.com>

So I just posted a script that pulls in headers from
a linux tree, doing minor changes required to
make the headers portable, such as
linux/types.h -> inttypes.h

I think it's a better idea, a virtio 1.0
work will benefit from this too.

> ---
>  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/hw/9pfs/virtio-9p.h b/hw/9pfs/virtio-9p.h
> index 2c3603a..228e05d 100644
> --- a/hw/9pfs/virtio-9p.h
> +++ b/hw/9pfs/virtio-9p.h
> @@ -146,8 +146,6 @@ struct V9fsPDU
>  
>  /* from Linux's linux/virtio_9p.h */
>  
> -/* The ID for virtio console */
> -#define VIRTIO_ID_9P    9
>  #define MAX_REQ         128
>  #define MAX_TAG_LEN     32
>  
> diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
> index f863bfe..6e0a775 100644
> --- a/include/hw/virtio/virtio-balloon.h
> +++ b/include/hw/virtio/virtio-balloon.h
> @@ -24,9 +24,6 @@
>  
>  /* from Linux's linux/virtio_balloon.h */
>  
> -/* The ID for virtio_balloon */
> -#define VIRTIO_ID_BALLOON 5
> -
>  /* The feature bitmap for virtio balloon */
>  #define VIRTIO_BALLOON_F_MUST_TELL_HOST 0 /* Tell before reclaiming pages */
>  #define VIRTIO_BALLOON_F_STATS_VQ 1       /* Memory stats virtqueue */
> diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
> index 4652b70..6ee3e8f 100644
> --- a/include/hw/virtio/virtio-blk.h
> +++ b/include/hw/virtio/virtio-blk.h
> @@ -25,9 +25,6 @@
>  
>  /* from Linux's linux/virtio_blk.h */
>  
> -/* The ID for virtio_block */
> -#define VIRTIO_ID_BLOCK 2
> -
>  /* Feature bits */
>  #define VIRTIO_BLK_F_BARRIER    0       /* Does host support barriers? */
>  #define VIRTIO_BLK_F_SIZE_MAX   1       /* Indicates maximum segment size */
> diff --git a/include/hw/virtio/virtio-rng.h b/include/hw/virtio/virtio-rng.h
> index 14e85a5..e2bb6ce 100644
> --- a/include/hw/virtio/virtio-rng.h
> +++ b/include/hw/virtio/virtio-rng.h
> @@ -21,9 +21,6 @@
>  #define VIRTIO_RNG_GET_PARENT_CLASS(obj) \
>          OBJECT_GET_PARENT_CLASS(obj, TYPE_VIRTIO_RNG)
>  
> -/* The Virtio ID for the virtio rng device */
> -#define VIRTIO_ID_RNG    4
> -
>  struct VirtIORNGConf {
>      RngBackend *rng;
>      uint64_t max_bytes;
> diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
> index bf17cc9..9606f43 100644
> --- a/include/hw/virtio/virtio-scsi.h
> +++ b/include/hw/virtio/virtio-scsi.h
> @@ -29,9 +29,6 @@
>          OBJECT_CHECK(VirtIOSCSI, (obj), TYPE_VIRTIO_SCSI)
>  
>  
> -/* The ID for virtio_scsi */
> -#define VIRTIO_ID_SCSI  8
> -
>  /* Feature Bits */
>  #define VIRTIO_SCSI_F_INOUT                    0
>  #define VIRTIO_SCSI_F_HOTPLUG                  1
> diff --git a/include/hw/virtio/virtio-serial.h b/include/hw/virtio/virtio-serial.h
> index 11af978..1dcced6 100644
> --- a/include/hw/virtio/virtio-serial.h
> +++ b/include/hw/virtio/virtio-serial.h
> @@ -20,9 +20,6 @@
>  
>  /* == Interface shared between the guest kernel and qemu == */
>  
> -/* The Virtio ID for virtio console / serial ports */
> -#define VIRTIO_ID_CONSOLE		3
> -
>  /* Features supported */
>  #define VIRTIO_CONSOLE_F_MULTIPORT	1
>  
> 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 */
> +
> +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_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"
>  #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,
> -};
> -
>  struct virtio_dev_header {
>      enum virtio_dev_type type : 8;
>      u8  num_vq;
> -- 
> 1.9.1

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-06  5:41 [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs Tiejun Chen
                   ` (2 preceding siblings ...)
  2015-02-09 19:57 ` Michael S. Tsirkin
@ 2015-02-11 11:18 ` Amit Shah
  3 siblings, 0 replies; 17+ messages in thread
From: Amit Shah @ 2015-02-11 11:18 UTC (permalink / raw)
  To: Tiejun Chen; +Cc: mst, qemu-devel, agraf, borntraeger, aliguori, cornelia.huck

On (Fri) 06 Feb 2015 [13:41:26], Tiejun Chen 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 <tiejun.chen@intel.com>

Reviewed-by: Amit Shah <amit.shah@redhat.com>

		Amit

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-09 19:57 ` Michael S. Tsirkin
@ 2015-02-11 11:55   ` Amit Shah
  2015-02-11 12:37     ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Amit Shah @ 2015-02-11 11:55 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, agraf, borntraeger, aliguori, cornelia.huck,
	Tiejun Chen

On (Mon) 09 Feb 2015 [20:57:42], Michael S. Tsirkin wrote:
> On Fri, Feb 06, 2015 at 01:41:26PM +0800, Tiejun Chen 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 <tiejun.chen@intel.com>
> 
> So I just posted a script that pulls in headers from
> a linux tree, doing minor changes required to
> make the headers portable, such as
> linux/types.h -> inttypes.h

We used to do this for kvm headers earlier..

Anyway I don't feel strongly about it.  Cornelia's objection that it
doesn't work on non-Linux hosts also holds..

		Amit

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-11 11:55   ` Amit Shah
@ 2015-02-11 12:37     ` Michael S. Tsirkin
  0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2015-02-11 12:37 UTC (permalink / raw)
  To: Amit Shah
  Cc: qemu-devel, agraf, borntraeger, aliguori, cornelia.huck,
	Tiejun Chen

On Wed, Feb 11, 2015 at 05:25:29PM +0530, Amit Shah wrote:
> On (Mon) 09 Feb 2015 [20:57:42], Michael S. Tsirkin wrote:
> > On Fri, Feb 06, 2015 at 01:41:26PM +0800, Tiejun Chen 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 <tiejun.chen@intel.com>
> > 
> > So I just posted a script that pulls in headers from
> > a linux tree, doing minor changes required to
> > make the headers portable, such as
> > linux/types.h -> inttypes.h
> 
> We used to do this for kvm headers earlier..
> 
> Anyway I don't feel strongly about it.  Cornelia's objection that it
> doesn't work on non-Linux hosts also holds..
> 
> 		Amit

Imported headers clearly can work on non linux hosts.

-- 
MST

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-09  8:47           ` Cornelia Huck
@ 2015-02-11 12:41             ` Michael S. Tsirkin
  2015-02-11 18:10               ` Cornelia Huck
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2015-02-11 12:41 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: qemu-devel, agraf, borntraeger, aliguori, amit.shah, Chen, Tiejun

On Mon, Feb 09, 2015 at 09:47:20AM +0100, Cornelia Huck wrote:
> On Mon, 09 Feb 2015 15:10:17 +0800
> "Chen, Tiejun" <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com> 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 <tiejun.chen@intel.com>
> > >>>
> > >>> 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?

This has nothing to do with host.
It's just using linux source as main basis for the file
simply because linux is a higher visibility project than qemu,
they won't borrow code from us but we can borrow from them.


> (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?)

virtio linux headers are also BSD licensed intentionally for this
purpose.
 * This header is BSD licensed so anyone can use the definitions to
 * implement compatible drivers/servers.




> > 
> > 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.

Let's worry about this one once we have fixed the duplication
for the others.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-11 12:41             ` Michael S. Tsirkin
@ 2015-02-11 18:10               ` Cornelia Huck
  2015-02-11 19:53                 ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Cornelia Huck @ 2015-02-11 18:10 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: qemu-devel, agraf, borntraeger, aliguori, amit.shah, Chen, Tiejun

On Wed, 11 Feb 2015 13:41:29 +0100
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Feb 09, 2015 at 09:47:20AM +0100, Cornelia Huck wrote:

> > 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?
> 
> This has nothing to do with host.

Hm, but all hosts and all guests need the ids, no?

> It's just using linux source as main basis for the file
> simply because linux is a higher visibility project than qemu,
> they won't borrow code from us but we can borrow from them.

It still seems restricting to use Linux as the ultimate source - and
that has nothing to do with visibililty. I'd say pulling from any
project is restricting: Why shouldn't we want to implement something in
qemu that Linux is not (yet) interested in, but other guests are?

> > (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?)
> 
> virtio linux headers are also BSD licensed intentionally for this
> purpose.
>  * This header is BSD licensed so anyone can use the definitions to
>  * implement compatible drivers/servers.

Sure, but I always took this to mean "you can freely copy the
definitions", not "this is the authorative source".

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs
  2015-02-11 18:10               ` Cornelia Huck
@ 2015-02-11 19:53                 ` Michael S. Tsirkin
  0 siblings, 0 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2015-02-11 19:53 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: qemu-devel, agraf, borntraeger, aliguori, amit.shah, Chen, Tiejun

On Wed, Feb 11, 2015 at 07:10:22PM +0100, Cornelia Huck wrote:
> On Wed, 11 Feb 2015 13:41:29 +0100
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Feb 09, 2015 at 09:47:20AM +0100, Cornelia Huck wrote:
> 
> > > 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?
> > 
> > This has nothing to do with host.
> 
> Hm, but all hosts and all guests need the ids, no?
> 
> > It's just using linux source as main basis for the file
> > simply because linux is a higher visibility project than qemu,
> > they won't borrow code from us but we can borrow from them.
> 
> It still seems restricting to use Linux as the ultimate source - and
> that has nothing to do with visibililty. I'd say pulling from any
> project is restricting: Why shouldn't we want to implement something in
> qemu that Linux is not (yet) interested in, but other guests are?

We can always add such a hypothetical feature in a qemu
specific headers.

But most things are shared, and manual duplication is bad.

> > > (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?)
> > 
> > virtio linux headers are also BSD licensed intentionally for this
> > purpose.
> >  * This header is BSD licensed so anyone can use the definitions to
> >  * implement compatible drivers/servers.
> 
> Sure, but I always took this to mean "you can freely copy the
> definitions", not "this is the authorative source".

virtio spec only has a ring header.
We could get it from there but it's easier to
get everything from one place I think.

-- 
MST

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2015-02-11 19:53 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-06  5:41 [Qemu-devel] [v2][RFC][PATCH] virtio: uniform virtio device IDs Tiejun Chen
2015-02-06 12:14 ` Cornelia Huck
2015-02-08 10:48   ` Michael S. Tsirkin
2015-02-09  7:01     ` Chen, Tiejun
2015-02-09  7:02       ` Michael S. Tsirkin
2015-02-09  7:10         ` Chen, Tiejun
2015-02-09  8:47           ` Cornelia Huck
2015-02-11 12:41             ` Michael S. Tsirkin
2015-02-11 18:10               ` Cornelia Huck
2015-02-11 19:53                 ` Michael S. Tsirkin
2015-02-09  6:56   ` Chen, Tiejun
2015-02-06 16:28 ` Stefan Hajnoczi
2015-02-09  6:58   ` Chen, Tiejun
2015-02-09 19:57 ` Michael S. Tsirkin
2015-02-11 11:55   ` Amit Shah
2015-02-11 12:37     ` Michael S. Tsirkin
2015-02-11 11:18 ` Amit Shah

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).