From mboxrd@z Thu Jan 1 00:00:00 1970 From: Julien Grall Subject: Re: [PATCH v3 13/13] xen/iommu: smmu: Advertise when the SMMU support coherent table walk Date: Tue, 10 Feb 2015 09:37:36 +0800 Message-ID: <54D960E0.9080407@linaro.org> References: <1422643768-23614-1-git-send-email-julien.grall@linaro.org> <1422643768-23614-14-git-send-email-julien.grall@linaro.org> <54D960AC.4050602@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1YKzll-0007Kp-W0 for xen-devel@lists.xenproject.org; Tue, 10 Feb 2015 01:37:42 +0000 Received: by iecrp18 with SMTP id rp18so16847854iec.10 for ; Mon, 09 Feb 2015 17:37:39 -0800 (PST) In-Reply-To: <54D960AC.4050602@linaro.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: xen-devel@lists.xenproject.org, tim@xen.org, ian.campbell@citrix.com, stefano.stabellini@citrix.com List-Id: xen-devel@lists.xenproject.org On 10/02/2015 09:36, Julien Grall wrote: > Hi Stefano, > > On 06/02/2015 22:06, Stefano Stabellini wrote: >> On Fri, 30 Jan 2015, Julien Grall wrote: >>> When SMMU doesn't support coherent table walk, Xen may need to clean >>> updated PT (see commit 4c5f4cb "xen/arm: p2m: Clean cache PT when the >>> IOMMU doesn't support coherent walk"). >>> >>> If one SMMU of the platform doesn't support coherent table walk, the >>> feature is disabled for the whole platform. This is because device is >>> assigned to a domain after the page table are populated. >>> >>> This could impact performance on domain which doesn't use device >>> passthrough. But, as the spec strongly recommends the support of this >>> feature for maintstream platform, I expect server will always have SMMUs >>> supporting coherent table walk. If not, we may need to enable this >>> feature >>> per-domain. >>> >>> Signed-off-by: Julien Grall >>> >>> --- >>> I've just noticed that the support on the previous driver (i.e in >>> Xen 4.5) may incorrectly expose this feature when all the SMMUs is >>> not supporting coherent table walk. >>> >>> I'm not sure, if I should send a patch for it. >>> >>> Also I didn't squash this patch into "xen/iommu: smmu: Add Xen >>> specific >>> code to be able to use the driver" to help for review and to catch >>> possible error in this patch. >>> >>> Changes in v3: >>> - Patch added >>> --- >>> xen/drivers/passthrough/arm/smmu.c | 25 +++++++++++++++++++++++++ >>> 1 file changed, 25 insertions(+) >>> >>> diff --git a/xen/drivers/passthrough/arm/smmu.c >>> b/xen/drivers/passthrough/arm/smmu.c >>> index 373eee8..f4c7e49 100644 >>> --- a/xen/drivers/passthrough/arm/smmu.c >>> +++ b/xen/drivers/passthrough/arm/smmu.c >>> @@ -577,6 +577,13 @@ struct arm_smmu_domain { >>> spinlock_t lock; >>> }; >>> >>> +/* >>> + * Xen: Platform features. It indicates the list of features support >>> by all the >>> + * SMMUs. >>> + * Actually we only care about coherent table walk. >>> + */ >>> +static u32 platform_features = ARM_SMMU_FEAT_COHERENT_WALK; >>> + >>> /* Xen: Dummy iommu_domain */ >>> struct iommu_domain >>> { >>> @@ -2810,6 +2817,13 @@ static int arm_smmu_iommu_domain_init(struct >>> domain *d) >>> >>> domain_hvm_iommu(d)->arch.priv = xen_domain; >>> >>> + /* >>> + * The feature coherent walk can be enabled only when all SMMUs >>> + * support it. >>> + */ >>> + if (platform_features & ARM_SMMU_FEAT_COHERENT_WALK) >>> + iommu_set_feature(d, IOMMU_FEAT_COHERENT_WALK); >>> + >>> return 0; >>> } >> >> As long as you are sure that arm_smmu_iommu_domain_init is called after >> all the iommus have been initialized by arm_smmu_dt_init, then this >> patch is correct. > > We don't support SMMU hotplug on Xen :). All the SMMUs are initialize > via iommu_setup before DOM0 is created. Though, I could update the commit message to explain it. -- Julien Grall