| Message ID | 20191010142852.15437-1-will@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvmtool] virtio: Ensure virt_queue is always initialised | expand |
On Thu, 10 Oct 2019 15:28:52 +0100 Will Deacon <will@kernel.org> wrote: Hi Will, > Failing to initialise the virt_queue via virtio_init_device_vq() leaves, > amongst other things, the endianness unspecified. On arm/arm64 this > results in virtio_guest_to_host_uxx() treating the queue as big-endian > and trying to translate bogus addresses: > > Warning: unable to translate guest address 0x80b8249800000000 to host Ouch, a user! ;-) > Ensure the virt_queue is always initialised by the virtio device during > setup. Indeed, this is also what the other virtio devices do. Confirmed to fix rng and balloon. Thanks for spotting this! Reviewed-by: Andre Przywara <andre.przywara@arm.com> Tested-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre. > > Cc: Marc Zyngier <maz@kernel.org> > Cc: Julien Thierry <julien.thierry.kdev@gmail.com> > Cc: Andre Przywara <andre.przywara@arm.com> > Signed-off-by: Will Deacon <will@kernel.org> > --- > virtio/balloon.c | 1 + > virtio/rng.c | 1 + > virtio/scsi.c | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/virtio/balloon.c b/virtio/balloon.c > index 15a9a46e77e0..0bd16703dfee 100644 > --- a/virtio/balloon.c > +++ b/virtio/balloon.c > @@ -212,6 +212,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align, > > thread_pool__init_job(&bdev->jobs[vq], kvm, virtio_bln_do_io, queue); > vring_init(&queue->vring, VIRTIO_BLN_QUEUE_SIZE, p, align); > + virtio_init_device_vq(&bdev->vdev, queue); > > return 0; > } > diff --git a/virtio/rng.c b/virtio/rng.c > index 9dd757b7e6e9..78eaa64bda17 100644 > --- a/virtio/rng.c > +++ b/virtio/rng.c > @@ -103,6 +103,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align, > job = &rdev->jobs[vq]; > > vring_init(&queue->vring, VIRTIO_RNG_QUEUE_SIZE, p, align); > + virtio_init_device_vq(&rdev->vdev, queue); > > *job = (struct rng_dev_job) { > .vq = queue, > diff --git a/virtio/scsi.c b/virtio/scsi.c > index a72bb2a9a206..1ec78fe0945a 100644 > --- a/virtio/scsi.c > +++ b/virtio/scsi.c > @@ -72,6 +72,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align, > p = virtio_get_vq(kvm, queue->pfn, page_size); > > vring_init(&queue->vring, VIRTIO_SCSI_QUEUE_SIZE, p, align); > + virtio_init_device_vq(&sdev->vdev, queue); > > if (sdev->vhost_fd == 0) > return 0;
On Fri, Oct 25, 2019 at 11:41:00AM +0100, Andre Przywara wrote: > On Thu, 10 Oct 2019 15:28:52 +0100 > Will Deacon <will@kernel.org> wrote: > > Failing to initialise the virt_queue via virtio_init_device_vq() leaves, > > amongst other things, the endianness unspecified. On arm/arm64 this > > results in virtio_guest_to_host_uxx() treating the queue as big-endian > > and trying to translate bogus addresses: > > > > Warning: unable to translate guest address 0x80b8249800000000 to host > > Ouch, a user! ;-) > > > Ensure the virt_queue is always initialised by the virtio device during > > setup. > > Indeed, this is also what the other virtio devices do. > Confirmed to fix rng and balloon. > > Thanks for spotting this! > > Reviewed-by: Andre Przywara <andre.przywara@arm.com> > Tested-by: Andre Przywara <andre.przywara@arm.com> Cheers, Andre. Now pushed with your tags. Will
diff --git a/virtio/balloon.c b/virtio/balloon.c index 15a9a46e77e0..0bd16703dfee 100644 --- a/virtio/balloon.c +++ b/virtio/balloon.c @@ -212,6 +212,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align, thread_pool__init_job(&bdev->jobs[vq], kvm, virtio_bln_do_io, queue); vring_init(&queue->vring, VIRTIO_BLN_QUEUE_SIZE, p, align); + virtio_init_device_vq(&bdev->vdev, queue); return 0; } diff --git a/virtio/rng.c b/virtio/rng.c index 9dd757b7e6e9..78eaa64bda17 100644 --- a/virtio/rng.c +++ b/virtio/rng.c @@ -103,6 +103,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align, job = &rdev->jobs[vq]; vring_init(&queue->vring, VIRTIO_RNG_QUEUE_SIZE, p, align); + virtio_init_device_vq(&rdev->vdev, queue); *job = (struct rng_dev_job) { .vq = queue, diff --git a/virtio/scsi.c b/virtio/scsi.c index a72bb2a9a206..1ec78fe0945a 100644 --- a/virtio/scsi.c +++ b/virtio/scsi.c @@ -72,6 +72,7 @@ static int init_vq(struct kvm *kvm, void *dev, u32 vq, u32 page_size, u32 align, p = virtio_get_vq(kvm, queue->pfn, page_size); vring_init(&queue->vring, VIRTIO_SCSI_QUEUE_SIZE, p, align); + virtio_init_device_vq(&sdev->vdev, queue); if (sdev->vhost_fd == 0) return 0;
Failing to initialise the virt_queue via virtio_init_device_vq() leaves, amongst other things, the endianness unspecified. On arm/arm64 this results in virtio_guest_to_host_uxx() treating the queue as big-endian and trying to translate bogus addresses: Warning: unable to translate guest address 0x80b8249800000000 to host Ensure the virt_queue is always initialised by the virtio device during setup. Cc: Marc Zyngier <maz@kernel.org> Cc: Julien Thierry <julien.thierry.kdev@gmail.com> Cc: Andre Przywara <andre.przywara@arm.com> Signed-off-by: Will Deacon <will@kernel.org> --- virtio/balloon.c | 1 + virtio/rng.c | 1 + virtio/scsi.c | 1 + 3 files changed, 3 insertions(+)