Received: by 10.213.65.68 with SMTP id h4csp2535985imn; Mon, 9 Apr 2018 05:13:57 -0700 (PDT) X-Google-Smtp-Source: AIpwx49wLwegdaq+nNIsy1XSG42kryyegtwMACqTnRPpe66Xo3RPkYfOSmB/F09R5fzmZYwaW/X9 X-Received: by 10.99.95.86 with SMTP id t83mr24471785pgb.183.1523276037164; Mon, 09 Apr 2018 05:13:57 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1523276037; cv=none; d=google.com; s=arc-20160816; b=GFZQusXFAlJ3hZ9ed5dNB7mPGZdM+ZzKCGRLPUkKIldZK6kdkuMO4fwtWaVmhBq1Qd kiUSBPQ0wcdKUH8r8Zuoxr7s2sUe++GJanlUMscUnt+z0tXnKKhvTzAf0unNQUVCpGRD g8pvddgVeMVcmjVJBhB/zwBQJEcFXXKXlOpqSKzEkwDfIpe0F7UyryMgGKnpnMnL/G1P D7to7LTNtYfPm7xXZQDkvwkRiTvlHVadu+k+78GW/PZ/s2YwTxr9pkSq45wbw+/xO7JM RXmLlN+DCenu/xOeeoAFmev1I5MJDnWR2p/RcbSP/fKDVbVh2StAtCRjbYEi5j9rsKrv VMVg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding :content-language:in-reply-to:mime-version:user-agent:date :message-id:from:references:cc:to:subject:arc-authentication-results; bh=Q1xujVIDNvsLz+1ny7Onm/7i2+/C4EWiu47K87U8J58=; b=w9Yji5FHPdh6oBxgl1HGfi+UHWVOMKqXjvIPYj2SIftE/QtNQUyDktSXMWfSUxa6Zz tpG08t7B3Xbk21wEw0q7RaiUcwwEUeES/LPTb5+Lx4WyvIjTECOFr7wiQzFvlJC+yHc7 l7TI9FM6/treTqzjNzgmdpVGIQjPznc+MDiQrIuoFbc4JoaqjMFic+MGkz5wPNITSVGB hq/LETRjQ9kN5//pJNerw1awGsIWkmFdBDrzl6yXbaRY2G47Ge9f/hgu7L6jLc5s1f7O ri4iigiXAQxV6r5BNk11VrZgaN1RGTywVjPlQUhGXTH5utuANGgFzXyPRRmF2QFMIVdX 6y7w== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z22si121561pgv.684.2018.04.09.05.13.19; Mon, 09 Apr 2018 05:13:57 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752311AbeDIMKh (ORCPT + 99 others); Mon, 9 Apr 2018 08:10:37 -0400 Received: from foss.arm.com ([217.140.101.70]:55584 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751633AbeDIMKf (ORCPT ); Mon, 9 Apr 2018 08:10:35 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 57576F; Mon, 9 Apr 2018 05:10:35 -0700 (PDT) Received: from [10.1.210.88] (e110467-lin.cambridge.arm.com [10.1.210.88]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B0DA83F592; Mon, 9 Apr 2018 05:10:33 -0700 (PDT) Subject: Re: [RFC PATCH 2/2] ACPI/IORT: use swiotlb_dma_ops when smmu probe failed To: "Wang, Dongsheng" , Lorenzo Pieralisi Cc: "rjw@rjwysocki.net" , "gregkh@linuxfoundation.org" , "hanjun.guo@linaro.org" , "sudeep.holla@arm.com" , "Zheng, Joey" , "linux-acpi@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <63635b42529a48d39f36112a031e3655@HXTBJIDCEMVIW02.hxtcorp.net> From: Robin Murphy Message-ID: <3fca23a7-8fe9-6832-9267-ebab73d48f68@arm.com> Date: Mon, 9 Apr 2018 13:10:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <63635b42529a48d39f36112a031e3655@HXTBJIDCEMVIW02.hxtcorp.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 08/04/18 09:10, Wang, Dongsheng wrote: > >> -----Original Message----- >> From: Robin Murphy [mailto:robin.murphy@arm.com] >> Sent: Thursday, April 05, 2018 2:57 AM >> To: Lorenzo Pieralisi ; Wang, Dongsheng >> >> Cc: rjw@rjwysocki.net; gregkh@linuxfoundation.org; hanjun.guo@linaro.org; >> sudeep.holla@arm.com; Zheng, Joey ; >> linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org >> Subject: [此邮件可能存在风险] Re: [RFC PATCH 2/2] ACPI/IORT: use >> swiotlb_dma_ops when smmu probe failed >> >> On 04/04/18 17:01, Lorenzo Pieralisi wrote: >>> [+cc Robin] >>> >>> On Thu, Mar 29, 2018 at 03:01:00AM -0700, Wang Dongsheng wrote: >>>> If SMMU probe failed, master should use swiotlb as dma ops. >>>> SMMU may probe failed with specified environment, so there >>>> are not any iommu resources in iommu_device_list. >>>> >>>> The master will always get EPROBE_DEFER from really_probe >>>> (dma_configure) but in fact SMMU has probe failed. The issue >>>> causes all of masters failed to be driven. >> >> Let's just take a step back - why is SMMU probe failing? That seems to >> be the primary issue here, because it implies that either your hardware, >> firmware or kernel is broken, any of which would make boot failure >> somewhat unsurprising anyway. >> > It's actually not a hardware issue. This is my test case, just return > -EINVAL in arm_smmu_device_probe. The HW probe(arm_smmu_device_hw_probe) > is just part of SMMU driver probe and the failure may be caused by SW. So > I design this case, just make sure even if SMMU probe failed that cause by SW, > the MASTER also can work. _Because of our SMMU default mode is bypass._ I don't think it's particularly justifiable to make core API changes for the sake of contrived testcases. On real systems, the SMMU is a fundamental system component which is no more expected to fail probe than, say, the GIC, and as such if it *does* fail then further progress is on a best-effort basis at most. Just because *your* system happens to work fine in this state doesn't make it true for every SMMU implementation and integration that Linux may ever run on. If you want the kernel to ignore an SMMU, either configure out the driver, or don't describe that SMMU in firmware in the first place. >>> I added Robin to pick his brain. An alternative would consist >>> in using a bus notifier to prevent deferred probing once the SMMU >>> driver probing failed but that seems backwards given that a major >>> reason to move to deferred probing was to remove the bus notifiers >>> dependency in the first place. >>> >>> It seems to me this is both an OF/ACPI issue - it is not an IORT >>> only problem. >> >> Yes, this is just an instance of the general probe-deferral problem, >> e.g. once you have multiple dependencies it's possible to end up in a >> stalemate where everything including the IOMMU ends up on the deferred >> probe list with nothing to kick it and make progress again. >> >> Furthermore it seems to me that the whole premise in this patch is >> flawed, > Ditto. :) > > >> since even genuine probe failure may well be transient - just >> because one attempt failed doesn't mean a later attempt can't succeed. >> Thus "the most recent probe attempt failed" cannot be considered a >> fundamentally different state from "no driver is currently bound". >> > Agree, the genuine probe failure may well be transient. But there is > depend on SMMU probe(IOMMU instance) status. There are two situations: > > 1. MASTER probing, SMMU doesn't probe yet. > This case will match "the transient failure". > really_probe get an EPROBE_DEFER from IORT and the MASTER probe will be > delayed until SMMU probe successful. > 2. MASTER probing, SMMU probe has failed. > really_probe will always get an EPROBE_DEFER from IORT, because kernel > has build in SMMU driver.(iort_iommu_driver_enabled) And the master > never cannot do probe. > > The case 2 is I want to handle. Handle it by not deliberately breaking the SMMU driver. In all other cases, either re-triggering SMMU probe might make it succeed (i.e. the DL_DEV_PROBE_FAILED state is meaningless), or things are so broken that you're probably dead in the water anyway. Robin. > > Cheers, > -Dongsheng > >> Robin. >> >>> >>> Lorenzo >>> >>>> Signed-off-by: Wang Dongsheng >>>> --- >>>> drivers/acpi/arm64/iort.c | 39 >> +++++++++++++++++++++++++++++++++------ >>>> 1 file changed, 33 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >>>> index e2f7bdd..a6f4c27 100644 >>>> --- a/drivers/acpi/arm64/iort.c >>>> +++ b/drivers/acpi/arm64/iort.c >>>> @@ -774,17 +774,45 @@ static int arm_smmu_iort_xlate(struct device >> *dev, u32 streamid, >>>> return ret; >>>> } >>>> >>>> -static inline bool iort_iommu_driver_enabled(u8 type) >>>> +static int iort_check_dev_dl_status(struct device *dev, void *data) >>>> { >>>> + struct fwnode_handle *fwnode = data; >>>> + >>>> + if (dev->fwnode != fwnode) >>>> + return 0; >>>> + >>>> + if (dev->links.status == DL_DEV_PROBE_FAILED) >>>> + return -ENODEV; >>>> + >>>> + return -EPROBE_DEFER; >>>> +} >>>> + >>>> +static int iort_iommu_driver_enabled(u8 type, struct fwnode_handle >> *fwnode) >>>> +{ >>>> + bool buildin; >>>> + int ret; >>>> + >>>> switch (type) { >>>> case ACPI_IORT_NODE_SMMU_V3: >>>> - return IS_BUILTIN(CONFIG_ARM_SMMU_V3); >>>> + buildin = IS_BUILTIN(CONFIG_ARM_SMMU_V3); >>>> + break; >>>> case ACPI_IORT_NODE_SMMU: >>>> - return IS_BUILTIN(CONFIG_ARM_SMMU); >>>> + buildin = IS_BUILTIN(CONFIG_ARM_SMMU); >>>> + break; >>>> default: >>>> pr_warn("IORT node type %u does not describe an SMMU\n", >> type); >>>> - return false; >>>> + buildin = false; >>>> } >>>> + >>>> + if (!buildin) >>>> + return -ENODEV; >>>> + >>>> + ret = bus_for_each_dev(&platform_bus_type, NULL, fwnode, >>>> + iort_check_dev_dl_status); >>>> + if (!ret) >>>> + return -EPROBE_DEFER; >>>> + >>>> + return ret; >>>> } >>>> >>>> #ifdef CONFIG_IOMMU_API >>>> @@ -919,8 +947,7 @@ static int iort_iommu_xlate(struct device *dev, >> struct acpi_iort_node *node, >>>> */ >>>> ops = iommu_ops_from_fwnode(iort_fwnode); >>>> if (!ops) >>>> - return iort_iommu_driver_enabled(node->type) ? >>>> - -EPROBE_DEFER : -ENODEV; >>>> + return iort_iommu_driver_enabled(node->type, iort_fwnode); >>>> >>>> return arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops); >>>> } >>>> -- >>>> 2.7.4 >>>>