Re: [Qemu-devel] [PATCH 1/5] virtio-blk: revert serial number support

[Anthony Liguori <anthony@codemonkey.ws>]

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