From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jean-Philippe Brucker Subject: Re: [RFC PATCH 7/8] VFIO: Add new IOCTL for IOMMU TLB invalidate propagation Date: Fri, 12 May 2017 13:11:02 +0100 Message-ID: References: <1493201525-14418-1-git-send-email-yi.l.liu@intel.com> <1493201525-14418-8-git-send-email-yi.l.liu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: jasowang@redhat.com, qemu-devel@nongnu.org, kevin.tian@intel.com, ashok.raj@intel.com, jacob.jun.pan@intel.com, tianyu.lan@intel.com, "Liu, Yi L" To: "Liu, Yi L" , kvm@vger.kernel.org, iommu@lists.linux-foundation.org, alex.williamson@redhat.com, peterx@redhat.com Return-path: Received: from foss.arm.com ([217.140.101.70]:59026 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757876AbdELMHw (ORCPT ); Fri, 12 May 2017 08:07:52 -0400 In-Reply-To: <1493201525-14418-8-git-send-email-yi.l.liu@intel.com> Sender: kvm-owner@vger.kernel.org List-ID: Hi Yi, On 26/04/17 11:12, Liu, Yi L wrote: > From: "Liu, Yi L" > > This patch adds VFIO_IOMMU_TLB_INVALIDATE to propagate IOMMU TLB > invalidate request from guest to host. > > In the case of SVM virtualization on VT-d, host IOMMU driver has > no knowledge of caching structure updates unless the guest > invalidation activities are passed down to the host. So a new > IOCTL is needed to propagate the guest cache invalidation through > VFIO. > > Signed-off-by: Liu, Yi L > --- > include/uapi/linux/vfio.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index 6b97987..50c51f8 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -564,6 +564,15 @@ struct vfio_device_svm { > > #define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22) > > +/* For IOMMU TLB Invalidation Propagation */ > +struct vfio_iommu_tlb_invalidate { > + __u32 argsz; > + __u32 length; > + __u8 data[]; > +}; We initially discussed something a little more generic than this, with most info explicitly described and only pIOMMU-specific quirks and hints in an opaque structure. Out of curiosity, why the change? I'm not against a fully opaque structure, but there seem to be a large overlap between TLB invalidations across architectures. For what it's worth, when prototyping the paravirtualized IOMMU I came up with the following. (From the paravirtualized POV, the SMMU also has to swizzle endianess after unpacking an opaque structure, since userspace doesn't know what's in it and guest might use a different endianess. So we need to force all opaque data to be e.g. little-endian.) struct vfio_iommu_tlb_invalidate { __u32 argsz; __u32 scope; __u32 flags; __u32 pasid; __u64 vaddr; __u64 size; __u8 data[]; }; Scope is a bitfields restricting the invalidation scope. By default invalidate the whole container (all PASIDs and all VAs). @pasid, @vaddr and @size are unused. Adding VFIO_IOMMU_INVALIDATE_PASID (1 << 0) restricts the invalidation scope to the pasid described by @pasid. Adding VFIO_IOMMU_INVALIDATE_VADDR (1 << 1) restricts the invalidation scope to the address range described by (@vaddr, @size). So setting scope = VFIO_IOMMU_INVALIDATE_VADDR would invalidate the VA range for *all* pasids (as well as no_pasid). Setting scope = (VFIO_IOMMU_INVALIDATE_VADDR|VFIO_IOMMU_INVALIDATE_PASID) would invalidate the VA range only for @pasid. Flags depend on the selected scope: VFIO_IOMMU_INVALIDATE_NO_PASID, indicating that invalidation (either without scope or with INVALIDATE_VADDR) targets non-pasid mappings exclusively (some architectures, e.g. SMMU, allow this) VFIO_IOMMU_INVALIDATE_VADDR_LEAF, indicating that the pIOMMU doesn't need to invalidate all intermediate tables cached as part of the PTW for vaddr, only the last-level entry (pte). This is a hint. I guess what's missing for Intel IOMMU and would go in @data is the "global" hint (which we don't have in SMMU invalidations). Do you see anything else, that the pIOMMU cannot deduce from this structure? Thanks, Jean > +#define VFIO_IOMMU_TLB_INVALIDATE _IO(VFIO_TYPE, VFIO_BASE + 23) > + > /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */ > > /* > From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38483) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d99MV-0005OF-JI for qemu-devel@nongnu.org; Fri, 12 May 2017 08:08:00 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1d99MQ-0008Ed-QK for qemu-devel@nongnu.org; Fri, 12 May 2017 08:07:59 -0400 Received: from foss.arm.com ([217.140.101.70]:38250) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1d99MQ-0008DM-JV for qemu-devel@nongnu.org; Fri, 12 May 2017 08:07:54 -0400 References: <1493201525-14418-1-git-send-email-yi.l.liu@intel.com> <1493201525-14418-8-git-send-email-yi.l.liu@intel.com> From: Jean-Philippe Brucker Message-ID: Date: Fri, 12 May 2017 13:11:02 +0100 MIME-Version: 1.0 In-Reply-To: <1493201525-14418-8-git-send-email-yi.l.liu@intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC PATCH 7/8] VFIO: Add new IOCTL for IOMMU TLB invalidate propagation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Liu, Yi L" , kvm@vger.kernel.org, iommu@lists.linux-foundation.org, alex.williamson@redhat.com, peterx@redhat.com Cc: jasowang@redhat.com, qemu-devel@nongnu.org, kevin.tian@intel.com, ashok.raj@intel.com, jacob.jun.pan@intel.com, tianyu.lan@intel.com, "Liu, Yi L" Hi Yi, On 26/04/17 11:12, Liu, Yi L wrote: > From: "Liu, Yi L" > > This patch adds VFIO_IOMMU_TLB_INVALIDATE to propagate IOMMU TLB > invalidate request from guest to host. > > In the case of SVM virtualization on VT-d, host IOMMU driver has > no knowledge of caching structure updates unless the guest > invalidation activities are passed down to the host. So a new > IOCTL is needed to propagate the guest cache invalidation through > VFIO. > > Signed-off-by: Liu, Yi L > --- > include/uapi/linux/vfio.h | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h > index 6b97987..50c51f8 100644 > --- a/include/uapi/linux/vfio.h > +++ b/include/uapi/linux/vfio.h > @@ -564,6 +564,15 @@ struct vfio_device_svm { > > #define VFIO_IOMMU_SVM_BIND_TASK _IO(VFIO_TYPE, VFIO_BASE + 22) > > +/* For IOMMU TLB Invalidation Propagation */ > +struct vfio_iommu_tlb_invalidate { > + __u32 argsz; > + __u32 length; > + __u8 data[]; > +}; We initially discussed something a little more generic than this, with most info explicitly described and only pIOMMU-specific quirks and hints in an opaque structure. Out of curiosity, why the change? I'm not against a fully opaque structure, but there seem to be a large overlap between TLB invalidations across architectures. For what it's worth, when prototyping the paravirtualized IOMMU I came up with the following. (From the paravirtualized POV, the SMMU also has to swizzle endianess after unpacking an opaque structure, since userspace doesn't know what's in it and guest might use a different endianess. So we need to force all opaque data to be e.g. little-endian.) struct vfio_iommu_tlb_invalidate { __u32 argsz; __u32 scope; __u32 flags; __u32 pasid; __u64 vaddr; __u64 size; __u8 data[]; }; Scope is a bitfields restricting the invalidation scope. By default invalidate the whole container (all PASIDs and all VAs). @pasid, @vaddr and @size are unused. Adding VFIO_IOMMU_INVALIDATE_PASID (1 << 0) restricts the invalidation scope to the pasid described by @pasid. Adding VFIO_IOMMU_INVALIDATE_VADDR (1 << 1) restricts the invalidation scope to the address range described by (@vaddr, @size). So setting scope = VFIO_IOMMU_INVALIDATE_VADDR would invalidate the VA range for *all* pasids (as well as no_pasid). Setting scope = (VFIO_IOMMU_INVALIDATE_VADDR|VFIO_IOMMU_INVALIDATE_PASID) would invalidate the VA range only for @pasid. Flags depend on the selected scope: VFIO_IOMMU_INVALIDATE_NO_PASID, indicating that invalidation (either without scope or with INVALIDATE_VADDR) targets non-pasid mappings exclusively (some architectures, e.g. SMMU, allow this) VFIO_IOMMU_INVALIDATE_VADDR_LEAF, indicating that the pIOMMU doesn't need to invalidate all intermediate tables cached as part of the PTW for vaddr, only the last-level entry (pte). This is a hint. I guess what's missing for Intel IOMMU and would go in @data is the "global" hint (which we don't have in SMMU invalidations). Do you see anything else, that the pIOMMU cannot deduce from this structure? Thanks, Jean > +#define VFIO_IOMMU_TLB_INVALIDATE _IO(VFIO_TYPE, VFIO_BASE + 23) > + > /* -------- Additional API for SPAPR TCE (Server POWERPC) IOMMU -------- */ > > /* >