From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1760070AbbJ3Svw (ORCPT ); Fri, 30 Oct 2015 14:51:52 -0400 Received: from mail-ob0-f177.google.com ([209.85.214.177]:35381 "EHLO mail-ob0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752610AbbJ3Svu (ORCPT ); Fri, 30 Oct 2015 14:51:50 -0400 MIME-Version: 1.0 In-Reply-To: <56335CF7.2050101@de.ibm.com> References: <7c590bf685f5cbc3f01e42bdbc1dbe3ffd83420f.1446162273.git.luto@kernel.org> <20151030130116.52a87922.cornelia.huck@de.ibm.com> <56335CF7.2050101@de.ibm.com> From: Andy Lutomirski Date: Fri, 30 Oct 2015 11:51:29 -0700 Message-ID: Subject: Re: [PATCH v4 2/6] virtio_ring: Support DMA APIs To: Christian Borntraeger Cc: Cornelia Huck , Andy Lutomirski , "linux-kernel@vger.kernel.org" , "David S. Miller" , sparclinux@vger.kernel.org, Joerg Roedel , Sebastian Ott , Paolo Bonzini , Christoph Hellwig , Benjamin Herrenschmidt , KVM , David Woodhouse , Martin Schwidefsky , linux-s390 , "Michael S. Tsirkin" , Linux Virtualization Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Oct 30, 2015 at 5:05 AM, Christian Borntraeger wrote: > Am 30.10.2015 um 13:01 schrieb Cornelia Huck: >> On Thu, 29 Oct 2015 18:09:47 -0700 >> Andy Lutomirski wrote: >> >>> virtio_ring currently sends the device (usually a hypervisor) >>> physical addresses of its I/O buffers. This is okay when DMA >>> addresses and physical addresses are the same thing, but this isn't >>> always the case. For example, this never works on Xen guests, and >>> it is likely to fail if a physical "virtio" device ever ends up >>> behind an IOMMU or swiotlb. >>> >>> The immediate use case for me is to enable virtio on Xen guests. >>> For that to work, we need vring to support DMA address translation >>> as well as a corresponding change to virtio_pci or to another >>> driver. >>> >>> With this patch, if enabled, virtfs survives kmemleak and >>> CONFIG_DMA_API_DEBUG. >>> >>> Signed-off-by: Andy Lutomirski >>> --- >>> drivers/virtio/Kconfig | 2 +- >>> drivers/virtio/virtio_ring.c | 190 +++++++++++++++++++++++++++++++-------- >>> tools/virtio/linux/dma-mapping.h | 17 ++++ >>> 3 files changed, 172 insertions(+), 37 deletions(-) >>> create mode 100644 tools/virtio/linux/dma-mapping.h >> >>> static void detach_buf(struct vring_virtqueue *vq, unsigned int head) >>> { >>> - unsigned int i; >>> + unsigned int i, j; >>> + u16 nextflag = cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT); >>> >>> /* Clear data ptr. */ >>> - vq->data[head] = NULL; >>> + vq->desc_state[head].data = NULL; >>> >>> - /* Put back on free list: find end */ >>> + /* Put back on free list: unmap first-level descriptors and find end */ >>> i = head; >>> >>> - /* Free the indirect table */ >>> - if (vq->vring.desc[i].flags & cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT)) >>> - kfree(phys_to_virt(virtio64_to_cpu(vq->vq.vdev, vq->vring.desc[i].addr))); >>> - >>> - while (vq->vring.desc[i].flags & cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_NEXT)) { >>> + while (vq->vring.desc[i].flags & nextflag) { >>> + vring_unmap_one(vq, &vq->vring.desc[i]); >>> i = virtio16_to_cpu(vq->vq.vdev, vq->vring.desc[i].next); >>> vq->vq.num_free++; >>> } >>> >>> + vring_unmap_one(vq, &vq->vring.desc[i]); >>> vq->vring.desc[i].next = cpu_to_virtio16(vq->vq.vdev, vq->free_head); >>> vq->free_head = head; >>> + >>> /* Plus final descriptor */ >>> vq->vq.num_free++; >>> + >>> + /* Free the indirect table, if any, now that it's unmapped. */ >>> + if (vq->desc_state[head].indir_desc) { >>> + struct vring_desc *indir_desc = vq->desc_state[head].indir_desc; >>> + u32 len = vq->vring.desc[head].len; >> >> This one needs to be virtio32_to_cpu(...) as well. > > Yes, just did the exact same change > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > index f269e1c..f2249df 100644 > --- a/drivers/virtio/virtio_ring.c > +++ b/drivers/virtio/virtio_ring.c > @@ -556,7 +556,7 @@ static void detach_buf(struct vring_virtqueue *vq, unsigned int head) > /* Free the indirect table, if any, now that it's unmapped. */ > if (vq->desc_state[head].indir_desc) { > struct vring_desc *indir_desc = vq->desc_state[head].indir_desc; > - u32 len = vq->vring.desc[head].len; > + u32 len = virtio32_to_cpu(vq->vq.vdev, vq->vring.desc[head].len); > > BUG_ON(!(vq->vring.desc[head].flags & > cpu_to_virtio16(vq->vq.vdev, VRING_DESC_F_INDIRECT))); > > > now it boots. Thanks! I applied this to my tree. I won't send a new version quite yet, though, to reduce inbox load. --Andy