From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1MsWgH-0007dh-UG for qemu-devel@nongnu.org; Tue, 29 Sep 2009 02:59:25 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1MsWgD-0007dV-Ba for qemu-devel@nongnu.org; Tue, 29 Sep 2009 02:59:25 -0400 Received: from [199.232.76.173] (port=34832 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1MsWgD-0007dS-64 for qemu-devel@nongnu.org; Tue, 29 Sep 2009 02:59:21 -0400 Received: from mx20.gnu.org ([199.232.41.8]:3380) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1MsWgC-0003Bn-KK for qemu-devel@nongnu.org; Tue, 29 Sep 2009 02:59:20 -0400 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1MsWgB-0001et-9N for qemu-devel@nongnu.org; Tue, 29 Sep 2009 02:59:19 -0400 Date: Tue, 29 Sep 2009 08:57:16 +0200 From: "Michael S. Tsirkin" Message-ID: <20090929065716.GB25389@redhat.com> References: <20090908075831.GA9875@redhat.com> <200909212039.01126.rusty@rustcorp.com.au> <4AB7A01A.3000206@redhat.com> <4AB8992B.7070709@redhat.com> <4AB8DD53.7070806@redhat.com> <4AB8DECA.3090908@redhat.com> <4AB8E88C.4040103@redhat.com> <4AB980E6.2070203@codemonkey.ws> <4AB9AA8E.7060800@third-harmonic.com> <4AC1A4C7.1090809@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4AC1A4C7.1090809@redhat.com> Subject: [Qemu-devel] Re: [PATCH 2/2] fix virtio_blk serial pci config breakage List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: john cooper Cc: john cooper , Rusty Russell , qemu-devel@nongnu.org, Avi Kivity , jens.axboe@oracle.com, Vadim Rozenfeld On Tue, Sep 29, 2009 at 02:10:15AM -0400, john cooper wrote: > Change virtblk_identify() to pull ATA identify > data from the bar #5 map vs. the pci config > area. Add minimal support for bar mapping external > to virtio/virtio_pci.c. > > Signed-off-by: john cooper > --- > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > index aa1a3d5..a1687f3 100644 > --- a/drivers/block/virtio_blk.c > +++ b/drivers/block/virtio_blk.c > @@ -8,6 +8,8 @@ > > #define PART_BITS 4 > > +#define VBLK_IDENTIFY_BAR 5 /* PCI BAR #5 maps identify/config area */ > + This constant is PCI specific. > static int major, index; > > struct virtio_blk > @@ -25,6 +27,9 @@ struct virtio_blk > > mempool_t *pool; > > + /* maintains mapping of pci bar #5 unique to virtio_blk */ This puts PCI specific stuff in virtio_blk. It would be better to hide this in transport-specific part. > + void __iomem *identify_ioaddr; > + > /* What host tells us, plus 2 for header & tailer. */ > unsigned int sg_elems; > > @@ -171,24 +176,30 @@ static void do_virtblk_request(struct request_queue *q) > vblk->vq->vq_ops->kick(vblk->vq); > } > > -/* return ATA identify data > +/* return ATA identify data if supported by virtio_blk device > */ > static int virtblk_identify(struct gendisk *disk, void *argp) > { > struct virtio_blk *vblk = disk->private_data; > void *opaque; > - int err = -ENOMEM; > + int err, i; > + char *p; > > + err = 0; > opaque = kmalloc(VIRTIO_BLK_ID_BYTES, GFP_KERNEL); > - if (!opaque) > + if (!opaque) { > + err = -ENOMEM; > goto out; > + } > > - err = virtio_config_buf(vblk->vdev, VIRTIO_BLK_F_IDENTIFY, > - offsetof(struct virtio_blk_config, identify), opaque, > - VIRTIO_BLK_ID_BYTES); > - > - if (err) > + if (!virtio_has_feature(vblk->vdev, VIRTIO_BLK_F_IDENTIFY) || > + !vblk->identify_ioaddr) { > + err = -ENOENT; > goto out_kfree; > + } > + > + for (p = opaque, i = 0; i < VIRTIO_BLK_ID_BYTES; ++i) > + *p++ = ioread8(vblk->identify_ioaddr + i); > > if (copy_to_user(argp, opaque, VIRTIO_BLK_ID_BYTES)) > err = -EFAULT; > @@ -383,6 +394,9 @@ static int __devinit virtblk_probe(struct virtio_device *vdev) > if (!err) > blk_queue_logical_block_size(vblk->disk->queue, blk_size); > > + vblk->identify_ioaddr = vdev->config->map ? > + vdev->config->map(vdev, VBLK_IDENTIFY_BAR, 0) : NULL; > + > add_disk(vblk->disk); > return 0; > > diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c > index 248e00e..abc6963 100644 > --- a/drivers/virtio/virtio_pci.c > +++ b/drivers/virtio/virtio_pci.c > @@ -561,6 +561,13 @@ static int vp_find_vqs(struct virtio_device *vdev, unsigned nvqs, > return err; > } > > +/* translate struct virtio_device to struct pci_dev, setup map for bar > + */ > +void __iomem *vp_map(struct virtio_device *vdev, int bar, unsigned long maxlen) > +{ > + return pci_iomap(to_vp_device(vdev)->pci_dev, bar, maxlen); > +} > + I think we need a feature bit to figure out whether device supports this feature. > static struct virtio_config_ops virtio_pci_config_ops = { > .get = vp_get, > .set = vp_set, > @@ -571,6 +578,7 @@ static struct virtio_config_ops virtio_pci_config_ops = { > .del_vqs = vp_del_vqs, > .get_features = vp_get_features, > .finalize_features = vp_finalize_features, > + .map = vp_map, > }; > > static void virtio_pci_release_dev(struct device *_d) > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h > index e547e3c..b9b3c62 100644 > --- a/include/linux/virtio_config.h > +++ b/include/linux/virtio_config.h > @@ -94,6 +94,8 @@ struct virtio_config_ops { > void (*del_vqs)(struct virtio_device *); > u32 (*get_features)(struct virtio_device *vdev); > void (*finalize_features)(struct virtio_device *vdev); > + void __iomem *(*map)(struct virtio_device *vdev, int bar, > + unsigned long maxlen); Don't we ever unmap? > }; > > /* If driver didn't advertise the feature, it will never appear. */ > > > -- > john.cooper@redhat.com