From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: "Tian, Kevin" To: Jean-Philippe Brucker , "linux-arm-kernel@lists.infradead.org" , "linux-pci@vger.kernel.org" , "linux-acpi@vger.kernel.org" , "devicetree@vger.kernel.org" , "iommu@lists.linux-foundation.org" , "kvm@vger.kernel.org" CC: Mark Rutland , "ilias.apalodimas@linaro.org" , Catalin Marinas , "xuzaibo@huawei.com" , Will Deacon , "okaya@codeaurora.org" , "Raj, Ashok" , "bharatku@xilinx.com" , "rfranz@cavium.com" , "lenb@kernel.org" , "robh+dt@kernel.org" , "bhelgaas@google.com" , "shunyong.yang@hxt-semitech.com" , "dwmw2@infradead.org" , "rjw@rjwysocki.net" , Sudeep Holla , "christian.koenig@amd.com" , Joerg Roedel Subject: RE: [PATCH 01/37] iommu: Introduce Shared Virtual Addressing API Date: Tue, 13 Feb 2018 23:43:08 +0000 Message-ID: References: <20180212183352.22730-1-jean-philippe.brucker@arm.com> <20180212183352.22730-2-jean-philippe.brucker@arm.com> In-Reply-To: Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Sender: linux-acpi-owner@vger.kernel.org List-ID: > From: Jean-Philippe Brucker > Sent: Tuesday, February 13, 2018 8:40 PM > > > [...] > >> + > >> +/** > >> + * iommu_sva_device_init() - Initialize Shared Virtual Addressing for a > >> device > >> + * @dev: the device > >> + * @features: bitmask of features that need to be initialized > >> + * @max_pasid: max PASID value supported by the device > >> + * > >> + * Users of the bind()/unbind() API must call this function to initialize all > >> + * features required for SVA. > >> + * > >> + * - If the device should support multiple address spaces (e.g. PCI > PASID), > >> + * IOMMU_SVA_FEAT_PASID must be requested. > > > > I think it is by default assumed when using this API, based on definition of > > SVA. Can you elaborate the situation where this flag can be cleared? > > When passing a device to userspace, you could also share its non-pasid > address space with the process. It requires a new domain type so is left > as a TODO in patch 2/37. I did get requests for this feature, though I > think it was mostly for prototyping. I guess I could remove the flag, and > reintroduce it as IOMMU_SVA_FEAT_NO_PASID later on. sorry I still didn't get the definition of non-pasid address space. Did you mean the GPA/IOVA address space and no_pasid implies actually some default PASID associated? > > [...] > >> + ret = domain->ops->sva_device_init(dev, features, &min_pasid, > >> + &max_pasid); > >> + if (ret) > >> + return ret; > >> + > >> + /* FIXME: racy. Next version should have a mutex (same as fault > >> handler) */ > >> + dev_param->sva_features = features; > >> + dev_param->min_pasid = min_pasid; > >> + dev_param->max_pasid = max_pasid; > > > > what's the point of min_pasid here? > > Arm SMMUv3 uses entry 0 of the PASID table for the default (non-pasid) > context, so it needs to set min_pasid to 1. AMD IOMMU recently added a > similar feature (GIoSup), if I understood correctly. > just for such purpose maybe we should just define a reserved_pasid otherwise there will be some waste if an implementation allows it non-zero. Thanks Kevin