> -----Original Message----- > From: Robin Murphy [mailto:robin.murphy@xxxxxxx] > Sent: Thursday, April 05, 2018 2:57 AM > To: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>; Wang, Dongsheng > <dongsheng.wang@xxxxxxxxxxxxxxxx> > Cc: rjw@xxxxxxxxxxxxx; gregkh@xxxxxxxxxxxxxxxxxxx; hanjun.guo@xxxxxxxxxx; > sudeep.holla@xxxxxxx; Zheng, Joey <yu.zheng@xxxxxxxxxxxxxxxx>; > linux-acpi@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > 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 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. Cheers, -Dongsheng > Robin. > > > > > Lorenzo > > > >> Signed-off-by: Wang Dongsheng <dongsheng.wang@xxxxxxxxxxxxxxxx> > >> --- > >> 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 > >> ��.n��������+%������w��{.n�����{�����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
