Patchwork [1/5] virtio-blk: revert serial number support

login
register
mail settings
Submitter Christoph Hellwig
Date Feb. 10, 2010, 10:36 p.m.
Message ID <20100210223649.GA11405@lst.de>
Download mbox | patch
Permalink /patch/45061/
State New
Headers show

Comments

Christoph Hellwig - Feb. 10, 2010, 10:36 p.m.
The addition of the whole ATA IDENTIY page caused the config space to
go above the allowed size in the PCI spec, and thus the feature was
already reverted in the Linux guest driver and disabled by default in
qemu.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Anthony Liguori - Feb. 11, 2010, 2:26 p.m.
On 02/10/2010 04:36 PM, Christoph Hellwig wrote:
> The addition of the whole ATA IDENTIY page caused the config space to
> go above the allowed size in the PCI spec, and thus the feature was
> already reverted in the Linux guest driver and disabled by default in
> qemu.
>
> Signed-off-by: Christoph Hellwig<hch@lst.de>
>    

Applied all.  Thanks.

Regards,

Anthony Liguori

> Index: qemu/hw/virtio-blk.c
> ===================================================================
> --- qemu.orig/hw/virtio-blk.c	2010-02-10 20:27:57.845254656 +0100
> +++ qemu/hw/virtio-blk.c	2010-02-10 23:27:51.067254307 +0100
> @@ -25,9 +25,7 @@ typedef struct VirtIOBlock
>       BlockDriverState *bs;
>       VirtQueue *vq;
>       void *rq;
> -    char serial_str[BLOCK_SERIAL_STRLEN + 1];
>       QEMUBH *bh;
> -    size_t config_size;
>   } VirtIOBlock;
>
>   static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
> @@ -35,47 +33,6 @@ static VirtIOBlock *to_virtio_blk(VirtIO
>       return (VirtIOBlock *)vdev;
>   }
>
> -/* store identify data in little endian format
> - */
> -static inline void put_le16(uint16_t *p, unsigned int v)
> -{
> -    *p = cpu_to_le16(v);
> -}
> -
> -/* copy to *dst from *src, nul pad dst tail as needed to len bytes
> - */
> -static inline void padstr(char *dst, const char *src, int len)
> -{
> -    while (len--)
> -        *dst++ = *src ? *src++ : '\0';
> -}
> -
> -/* setup simulated identify data as appropriate for virtio block device
> - *
> - * ref: AT Attachment 8 - ATA/ATAPI Command Set (ATA8-ACS)
> - */
> -static inline void virtio_identify_template(struct virtio_blk_config *bc)
> -{
> -    uint16_t *p =&bc->identify[0];
> -    uint64_t lba_sectors = bc->capacity;
> -
> -    memset(p, 0, sizeof(bc->identify));
> -    put_le16(p + 0, 0x0);                            /* ATA device */
> -    padstr((char *)(p + 23), QEMU_VERSION, 8);       /* firmware revision */
> -    padstr((char *)(p + 27), "QEMU VIRT_BLK", 40);   /* model# */
> -    put_le16(p + 47, 0x80ff);                        /* max xfer 255 sectors */
> -    put_le16(p + 49, 0x0b00);                        /* support IORDY/LBA/DMA */
> -    put_le16(p + 59, 0x1ff);                         /* cur xfer 255 sectors */
> -    put_le16(p + 80, 0x1f0);                         /* support ATA8/7/6/5/4 */
> -    put_le16(p + 81, 0x16);
> -    put_le16(p + 82, 0x400);
> -    put_le16(p + 83, 0x400);
> -    put_le16(p + 100, lba_sectors);
> -    put_le16(p + 101, lba_sectors>>  16);
> -    put_le16(p + 102, lba_sectors>>  32);
> -    put_le16(p + 103, lba_sectors>>  48);
> -}
> -
>   typedef struct VirtIOBlockReq
>   {
>       VirtIOBlock *dev;
> @@ -448,10 +405,7 @@ static void virtio_blk_update_config(Vir
>       blkcfg.heads = heads;
>       blkcfg.sectors = secs;
>       blkcfg.size_max = 0;
> -    virtio_identify_template(&blkcfg);
> -    memcpy(&blkcfg.identify[VIRTIO_BLK_ID_SN], s->serial_str,
> -        VIRTIO_BLK_ID_SN_BYTES);
> -    memcpy(config,&blkcfg, s->config_size);
> +    memcpy(config,&blkcfg, sizeof(struct virtio_blk_config));
>   }
>
>   static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
> @@ -463,8 +417,6 @@ static uint32_t virtio_blk_get_features(
>
>       if (bdrv_enable_write_cache(s->bs))
>           features |= (1<<  VIRTIO_BLK_F_WCACHE);
> -    if (strcmp(s->serial_str, "0"))
> -        features |= 1<<  VIRTIO_BLK_F_IDENTIFY;
>
>       if (bdrv_is_read_only(s->bs))
>           features |= 1<<  VIRTIO_BLK_F_RO;
> @@ -510,24 +462,16 @@ VirtIODevice *virtio_blk_init(DeviceStat
>       VirtIOBlock *s;
>       int cylinders, heads, secs;
>       static int virtio_blk_id;
> -    char *ps = (char *)drive_get_serial(dinfo->bdrv);
> -    size_t size = strlen(ps) ? sizeof(struct virtio_blk_config) :
> -	    offsetof(struct virtio_blk_config, _blk_size);
>
>       s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
> -                                          size,
> +                                          sizeof(struct virtio_blk_config),
>                                             sizeof(VirtIOBlock));
>
> -    s->config_size = size;
>       s->vdev.get_config = virtio_blk_update_config;
>       s->vdev.get_features = virtio_blk_get_features;
>       s->vdev.reset = virtio_blk_reset;
>       s->bs = dinfo->bdrv;
>       s->rq = NULL;
> -    if (strlen(ps))
> -        strncpy(s->serial_str, ps, sizeof(s->serial_str));
> -    else
> -        snprintf(s->serial_str, sizeof(s->serial_str), "0");
>       bdrv_guess_geometry(s->bs,&cylinders,&heads,&secs);
>       bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
>
> Index: qemu/hw/virtio-blk.h
> ===================================================================
> --- qemu.orig/hw/virtio-blk.h	2010-02-10 20:27:52.756275049 +0100
> +++ qemu/hw/virtio-blk.h	2010-02-10 23:27:51.067254307 +0100
> @@ -30,13 +30,9 @@
>   #define VIRTIO_BLK_F_RO         5       /* Disk is read-only */
>   #define VIRTIO_BLK_F_BLK_SIZE   6       /* Block size of disk is available*/
>   #define VIRTIO_BLK_F_SCSI       7       /* Supports scsi command passthru */
> -#define VIRTIO_BLK_F_IDENTIFY   8       /* ATA IDENTIFY supported */
> +/* #define VIRTIO_BLK_F_IDENTIFY   8       ATA IDENTIFY supported, DEPRECATED */
>   #define VIRTIO_BLK_F_WCACHE     9       /* write cache enabled */
>
> -#define VIRTIO_BLK_ID_LEN       256     /* length of identify u16 array */
> -#define VIRTIO_BLK_ID_SN        10      /* start of char * serial# */
> -#define VIRTIO_BLK_ID_SN_BYTES  20      /* length in bytes of serial# */
> -
>   struct virtio_blk_config
>   {
>       uint64_t capacity;
> @@ -46,7 +42,6 @@ struct virtio_blk_config
>       uint8_t heads;
>       uint8_t sectors;
>       uint32_t _blk_size;    /* structure pad, currently unused */
> -    uint16_t identify[VIRTIO_BLK_ID_LEN];
>   } __attribute__((packed));
>
>   /* These two define direction. */
>
>
>
>

Patch

Index: qemu/hw/virtio-blk.c
===================================================================
--- qemu.orig/hw/virtio-blk.c	2010-02-10 20:27:57.845254656 +0100
+++ qemu/hw/virtio-blk.c	2010-02-10 23:27:51.067254307 +0100
@@ -25,9 +25,7 @@  typedef struct VirtIOBlock
     BlockDriverState *bs;
     VirtQueue *vq;
     void *rq;
-    char serial_str[BLOCK_SERIAL_STRLEN + 1];
     QEMUBH *bh;
-    size_t config_size;
 } VirtIOBlock;
 
 static VirtIOBlock *to_virtio_blk(VirtIODevice *vdev)
@@ -35,47 +33,6 @@  static VirtIOBlock *to_virtio_blk(VirtIO
     return (VirtIOBlock *)vdev;
 }
 
-/* store identify data in little endian format
- */
-static inline void put_le16(uint16_t *p, unsigned int v)
-{
-    *p = cpu_to_le16(v);
-}
-
-/* copy to *dst from *src, nul pad dst tail as needed to len bytes
- */
-static inline void padstr(char *dst, const char *src, int len)
-{
-    while (len--)
-        *dst++ = *src ? *src++ : '\0';
-}
-
-/* setup simulated identify data as appropriate for virtio block device
- *
- * ref: AT Attachment 8 - ATA/ATAPI Command Set (ATA8-ACS)
- */
-static inline void virtio_identify_template(struct virtio_blk_config *bc)
-{
-    uint16_t *p = &bc->identify[0];
-    uint64_t lba_sectors = bc->capacity;
-
-    memset(p, 0, sizeof(bc->identify));
-    put_le16(p + 0, 0x0);                            /* ATA device */
-    padstr((char *)(p + 23), QEMU_VERSION, 8);       /* firmware revision */
-    padstr((char *)(p + 27), "QEMU VIRT_BLK", 40);   /* model# */
-    put_le16(p + 47, 0x80ff);                        /* max xfer 255 sectors */
-    put_le16(p + 49, 0x0b00);                        /* support IORDY/LBA/DMA */
-    put_le16(p + 59, 0x1ff);                         /* cur xfer 255 sectors */
-    put_le16(p + 80, 0x1f0);                         /* support ATA8/7/6/5/4 */
-    put_le16(p + 81, 0x16);
-    put_le16(p + 82, 0x400);
-    put_le16(p + 83, 0x400);
-    put_le16(p + 100, lba_sectors);
-    put_le16(p + 101, lba_sectors >> 16);
-    put_le16(p + 102, lba_sectors >> 32);
-    put_le16(p + 103, lba_sectors >> 48);
-}
-
 typedef struct VirtIOBlockReq
 {
     VirtIOBlock *dev;
@@ -448,10 +405,7 @@  static void virtio_blk_update_config(Vir
     blkcfg.heads = heads;
     blkcfg.sectors = secs;
     blkcfg.size_max = 0;
-    virtio_identify_template(&blkcfg);
-    memcpy(&blkcfg.identify[VIRTIO_BLK_ID_SN], s->serial_str,
-        VIRTIO_BLK_ID_SN_BYTES);
-    memcpy(config, &blkcfg, s->config_size);
+    memcpy(config, &blkcfg, sizeof(struct virtio_blk_config));
 }
 
 static uint32_t virtio_blk_get_features(VirtIODevice *vdev, uint32_t features)
@@ -463,8 +417,6 @@  static uint32_t virtio_blk_get_features(
 
     if (bdrv_enable_write_cache(s->bs))
         features |= (1 << VIRTIO_BLK_F_WCACHE);
-    if (strcmp(s->serial_str, "0"))
-        features |= 1 << VIRTIO_BLK_F_IDENTIFY;
     
     if (bdrv_is_read_only(s->bs))
         features |= 1 << VIRTIO_BLK_F_RO;
@@ -510,24 +462,16 @@  VirtIODevice *virtio_blk_init(DeviceStat
     VirtIOBlock *s;
     int cylinders, heads, secs;
     static int virtio_blk_id;
-    char *ps = (char *)drive_get_serial(dinfo->bdrv);
-    size_t size = strlen(ps) ? sizeof(struct virtio_blk_config) :
-	    offsetof(struct virtio_blk_config, _blk_size);
 
     s = (VirtIOBlock *)virtio_common_init("virtio-blk", VIRTIO_ID_BLOCK,
-                                          size,
+                                          sizeof(struct virtio_blk_config),
                                           sizeof(VirtIOBlock));
 
-    s->config_size = size;
     s->vdev.get_config = virtio_blk_update_config;
     s->vdev.get_features = virtio_blk_get_features;
     s->vdev.reset = virtio_blk_reset;
     s->bs = dinfo->bdrv;
     s->rq = NULL;
-    if (strlen(ps))
-        strncpy(s->serial_str, ps, sizeof(s->serial_str));
-    else
-        snprintf(s->serial_str, sizeof(s->serial_str), "0");
     bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
     bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
 
Index: qemu/hw/virtio-blk.h
===================================================================
--- qemu.orig/hw/virtio-blk.h	2010-02-10 20:27:52.756275049 +0100
+++ qemu/hw/virtio-blk.h	2010-02-10 23:27:51.067254307 +0100
@@ -30,13 +30,9 @@ 
 #define VIRTIO_BLK_F_RO         5       /* Disk is read-only */
 #define VIRTIO_BLK_F_BLK_SIZE   6       /* Block size of disk is available*/
 #define VIRTIO_BLK_F_SCSI       7       /* Supports scsi command passthru */
-#define VIRTIO_BLK_F_IDENTIFY   8       /* ATA IDENTIFY supported */
+/* #define VIRTIO_BLK_F_IDENTIFY   8       ATA IDENTIFY supported, DEPRECATED */
 #define VIRTIO_BLK_F_WCACHE     9       /* write cache enabled */
 
-#define VIRTIO_BLK_ID_LEN       256     /* length of identify u16 array */
-#define VIRTIO_BLK_ID_SN        10      /* start of char * serial# */
-#define VIRTIO_BLK_ID_SN_BYTES  20      /* length in bytes of serial# */
-
 struct virtio_blk_config
 {
     uint64_t capacity;
@@ -46,7 +42,6 @@  struct virtio_blk_config
     uint8_t heads;
     uint8_t sectors;
     uint32_t _blk_size;    /* structure pad, currently unused */
-    uint16_t identify[VIRTIO_BLK_ID_LEN];
 } __attribute__((packed));
 
 /* These two define direction. */