From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S967466AbeEYRpq (ORCPT ); Fri, 25 May 2018 13:45:46 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:44486 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S967349AbeEYRpp (ORCPT ); Fri, 25 May 2018 13:45:45 -0400 Date: Fri, 25 May 2018 20:45:42 +0300 From: "Michael S. Tsirkin" To: Benjamin Herrenschmidt Cc: Anshuman Khandual , virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, aik@ozlabs.ru, robh@kernel.org, joe@perches.com, elfring@users.sourceforge.net, david@gibson.dropbear.id.au, jasowang@redhat.com, mpe@ellerman.id.au, hch@infradead.org Subject: Re: [RFC V2] virtio: Add platform specific DMA API translation for virito devices Message-ID: <20180525202300-mutt-send-email-mst@kernel.org> References: <20180522063317.20956-1-khandual@linux.vnet.ibm.com> <20180523213703-mutt-send-email-mst@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, May 24, 2018 at 08:27:04AM +1000, Benjamin Herrenschmidt wrote: > On Wed, 2018-05-23 at 21:50 +0300, Michael S. Tsirkin wrote: > > > I re-read that discussion and I'm still unclear on the > > original question, since I got several apparently > > conflicting answers. > > > > I asked: > > > > Why isn't setting VIRTIO_F_IOMMU_PLATFORM on the > > hypervisor side sufficient? > > I thought I had replied to this... > > There are a couple of reasons: > > - First qemu doesn't know that the guest will switch to "secure mode" > in advance. There is no difference between a normal and a secure > partition until the partition does the magic UV call to "enter secure > mode" and qemu doesn't see any of it. So who can set the flag here ? Not sure I understand. Just set the flag e.g. on qemu command line. I might be wrong, but these secure mode things usually a. require hypervisor side tricks anyway > - Second, when using VIRTIO_F_IOMMU_PLATFORM, we also make qemu (or > vhost) go through the emulated MMIO for every access to the guest, > which adds additional overhead. > > Cheers, > Ben. Well it's not supposed to be much slower for the static case. vhost has a cache so should be fine. A while ago Paolo implemented a translation cache which should be perfect for this case - most of the code got merged but never enabled because of stability issues. If all else fails, we could teach QEMU to handle the no-iommu case as if VIRTIO_F_IOMMU_PLATFORM was off. > > > > > > > arch/powerpc/include/asm/dma-mapping.h | 6 ++++++ > > > arch/powerpc/platforms/pseries/iommu.c | 11 +++++++++++ > > > drivers/virtio/virtio_ring.c | 10 ++++++++++ > > > 3 files changed, 27 insertions(+) > > > > > > diff --git a/arch/powerpc/include/asm/dma-mapping.h b/arch/powerpc/include/asm/dma-mapping.h > > > index 8fa3945..056e578 100644 > > > --- a/arch/powerpc/include/asm/dma-mapping.h > > > +++ b/arch/powerpc/include/asm/dma-mapping.h > > > @@ -115,4 +115,10 @@ extern u64 __dma_get_required_mask(struct device *dev); > > > #define ARCH_HAS_DMA_MMAP_COHERENT > > > > > > #endif /* __KERNEL__ */ > > > + > > > +#define platform_forces_virtio_dma platform_forces_virtio_dma > > > + > > > +struct virtio_device; > > > + > > > +extern bool platform_forces_virtio_dma(struct virtio_device *vdev); > > > #endif /* _ASM_DMA_MAPPING_H */ > > > diff --git a/arch/powerpc/platforms/pseries/iommu.c b/arch/powerpc/platforms/pseries/iommu.c > > > index 06f0296..a2ec15a 100644 > > > --- a/arch/powerpc/platforms/pseries/iommu.c > > > +++ b/arch/powerpc/platforms/pseries/iommu.c > > > @@ -38,6 +38,7 @@ > > > #include > > > #include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -1396,3 +1397,13 @@ static int __init disable_multitce(char *str) > > > __setup("multitce=", disable_multitce); > > > > > > machine_subsys_initcall_sync(pseries, tce_iommu_bus_notifier_init); > > > + > > > +bool platform_forces_virtio_dma(struct virtio_device *vdev) > > > +{ > > > + /* > > > + * On protected guest platforms, force virtio core to use DMA > > > + * MAP API for all virtio devices. But there can also be some > > > + * exceptions for individual devices like virtio balloon. > > > + */ > > > + return (of_find_compatible_node(NULL, NULL, "ibm,ultravisor") != NULL); > > > +} > > > > Isn't this kind of slow? vring_use_dma_api is on > > data path and supposed to be very fast. > > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c > > > index 21d464a..47ea6c3 100644 > > > --- a/drivers/virtio/virtio_ring.c > > > +++ b/drivers/virtio/virtio_ring.c > > > @@ -141,8 +141,18 @@ struct vring_virtqueue { > > > * unconditionally on data path. > > > */ > > > > > > +#ifndef platform_forces_virtio_dma > > > +static inline bool platform_forces_virtio_dma(struct virtio_device *vdev) > > > +{ > > > + return false; > > > +} > > > +#endif > > > + > > > static bool vring_use_dma_api(struct virtio_device *vdev) > > > { > > > + if (platform_forces_virtio_dma(vdev)) > > > + return true; > > > + > > > if (!virtio_has_iommu_quirk(vdev)) > > > return true; > > > > > > -- > > > 2.9.3