Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933404AbcJZPGP (ORCPT ); Wed, 26 Oct 2016 11:06:15 -0400 Received: from hqemgate16.nvidia.com ([216.228.121.65]:15280 "EHLO hqemgate16.nvidia.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752094AbcJZPGJ (ORCPT ); Wed, 26 Oct 2016 11:06:09 -0400 X-PGP-Universal: processed; by hqnvupgp08.nvidia.com on Wed, 26 Oct 2016 08:06:07 -0700 Subject: Re: [PATCH v9 04/12] vfio iommu: Add support for mediated devices To: "Tian, Kevin" , Alex Williamson References: <1476739332-4911-1-git-send-email-kwankhede@nvidia.com> <1476739332-4911-5-git-send-email-kwankhede@nvidia.com> <20161019150231.19e32b84@t450s.home> <3aee74c1-f984-74e3-502b-18b5e1ea6fc7@nvidia.com> <20161023203216.70b2fde3@t450s.home> CC: "pbonzini@redhat.com" , "kraxel@redhat.com" , "cjia@nvidia.com" , "qemu-devel@nongnu.org" , "kvm@vger.kernel.org" , "Song, Jike" , "bjsdjshi@linux.vnet.ibm.com" , "linux-kernel@vger.kernel.org" X-Nvconfidentiality: public From: Kirti Wankhede Message-ID: Date: Wed, 26 Oct 2016 20:36:01 +0530 MIME-Version: 1.0 In-Reply-To: X-Originating-IP: [10.24.216.210] X-ClientProxiedBy: BGMAIL102.nvidia.com (10.25.59.11) To bgmail102.nvidia.com (10.25.59.11) Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2002 Lines: 57 On 10/26/2016 12:49 PM, Tian, Kevin wrote: >> From: Alex Williamson [mailto:alex.williamson@redhat.com] >> Sent: Monday, October 24, 2016 10:32 AM >> >>>>> -static long vfio_unpin_pages(unsigned long pfn, long npage, >>>>> - int prot, bool do_accounting) >>>>> +static long __vfio_unpin_pages_remote(struct vfio_iommu *iommu, >>>>> + unsigned long pfn, long npage, int prot, >>>>> + bool do_accounting) >>>> >>>> Have you noticed that it's kind of confusing that >>>> __vfio_{un}pin_pages_remote() uses current, which does a >>>> get_user_pages_fast() while "local" uses a provided task_struct and >>>> uses get_user_pages_*remote*()? And also what was effectively local >>>> (ie. we're pinning for our own use here) is now "remote" and pinning >>>> for a remote, vendor driver consumer, is now "local". It's not very >>>> intuitive. >>>> > > I questioned this confusing naming in v8 too... > I do tried to address your concerns on v8. >>> >>> 'local' in local_domain was suggested to describe the domain for local >>> page tracking. Earlier suggestions to have 'mdev' or 'noimmu' in this >>> name were discarded. May be we should revisit what the name should be. >>> Any suggestion? >>> >>> For local_domain, to pin pages, flow is: >>> >>> for local_domain >>> |- vfio_pin_pages() >>> |- vfio_iommu_type1_pin_pages() >>> |- __vfio_pin_page_local() >>> |- vaddr_get_pfn(task->mm) >>> |- get_user_pages_remote() >>> >>> __vfio_pin_page_local() --> get_user_pages_remote() >> >> >> In vfio.c we have the concept of an external user, perhaps that could >> be continued here. An mdev driver would be an external, or remote >> pinning. >> > > I prefer to use remote here. It's aligned with underlying mm operations > Using 'remote' in this case is also confusing since it is already used in this file. I liked Alex's suggestion to use external and I'll have those changed in next version of patch set. Kirti