| Message ID | 1480465344-11862-3-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Sricharan, Robin, I gave this series a go on ACPI and apart from an SMMU v3 fix-up it seems to work, more thorough testing required though. A key question below. On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: > From: Robin Murphy <robin.murphy@arm.com> > > IOMMU configuration represents unchanging properties of the hardware, > and as such should only need happen once in a device's lifetime, but > the necessary interaction with the IOMMU device and driver complicates > exactly when that point should be. > > Since the only reasonable tool available for handling the inter-device > dependency is probe deferral, we need to prepare of_iommu_configure() > to run later than it is currently called (i.e. at driver probe rather > than device creation), to handle being retried, and to tell whether a > not-yet present IOMMU should be waited for or skipped (by virtue of > having declared a built-in driver or not). > > Signed-off-by: Robin Murphy <robin.murphy@arm.com> > --- > drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index ee49081..349bd1d 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, > int err; > > ops = iommu_get_instance(fwnode); > - if (!ops || !ops->of_xlate) > + if ((ops && !ops->of_xlate) || > + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) IIUC of_match_node() here is there to check there is a driver compiled in for this device_node (aka compatible string in OF world), correct ? If that's the case (and I think that's what Sricharan was referring to in his ACPI query) I need to cook-up something on the ACPI side to emulate the OF linker table behaviour (or anyway to detect a driver is actually in the kernel), it is not that difficult but it is key to know, I will give it some thought to make it as clean as possible. Thanks, Lorenzo > return NULL; > > err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); > if (err) > return ERR_PTR(err); > + /* > + * The otherwise-empty fwspec handily serves to indicate the specific > + * IOMMU device we're waiting for, which will be useful if we ever get > + * a proper probe-ordering dependency mechanism in future. > + */ > + if (!ops) > + return ERR_PTR(-EPROBE_DEFER); > > err = ops->of_xlate(dev, iommu_spec); > if (err) > @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, > struct device_node *master_np) > { > const struct iommu_ops *ops; > + struct iommu_fwspec *fwspec = dev->iommu_fwspec; > > if (!master_np) > return NULL; > > + if (fwspec) { > + if (fwspec->ops) > + return fwspec->ops; > + > + /* In the deferred case, start again from scratch */ > + iommu_fwspec_free(dev); > + } > + > if (dev_is_pci(dev)) > ops = of_pci_iommu_init(to_pci_dev(dev), master_np); > else > ops = of_platform_iommu_init(dev, master_np); > + /* > + * If we have reason to believe the IOMMU driver missed the initial > + * add_device callback for dev, replay it to get things in order. > + */ > + if (!IS_ERR_OR_NULL(ops) && ops->add_device && > + dev->bus && !dev->iommu_group) { > + int err = ops->add_device(dev); > + > + if (err) > + ops = ERR_PTR(err); > + } > > return IS_ERR(ops) ? NULL : ops; > } > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation >
On 30/11/16 16:17, Lorenzo Pieralisi wrote: > Sricharan, Robin, > > I gave this series a go on ACPI and apart from an SMMU v3 fix-up > it seems to work, more thorough testing required though. > > A key question below. > > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: >> From: Robin Murphy <robin.murphy@arm.com> >> >> IOMMU configuration represents unchanging properties of the hardware, >> and as such should only need happen once in a device's lifetime, but >> the necessary interaction with the IOMMU device and driver complicates >> exactly when that point should be. >> >> Since the only reasonable tool available for handling the inter-device >> dependency is probe deferral, we need to prepare of_iommu_configure() >> to run later than it is currently called (i.e. at driver probe rather >> than device creation), to handle being retried, and to tell whether a >> not-yet present IOMMU should be waited for or skipped (by virtue of >> having declared a built-in driver or not). >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> --- >> drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- >> 1 file changed, 29 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >> index ee49081..349bd1d 100644 >> --- a/drivers/iommu/of_iommu.c >> +++ b/drivers/iommu/of_iommu.c >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, >> int err; >> >> ops = iommu_get_instance(fwnode); >> - if (!ops || !ops->of_xlate) >> + if ((ops && !ops->of_xlate) || >> + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) > > IIUC of_match_node() here is there to check there is a driver compiled > in for this device_node (aka compatible string in OF world), correct ? Yes - specifically, it's checking the magic table for a matching IOMMU_OF_DECLARE entry. > If that's the case (and I think that's what Sricharan was referring to > in his ACPI query) I need to cook-up something on the ACPI side to > emulate the OF linker table behaviour (or anyway to detect a driver is > actually in the kernel), it is not that difficult but it is key to know, > I will give it some thought to make it as clean as possible. I didn't think this would be a concern for ACPI, since IORT works much the same way the current of_iommu_init_fn/of_platform_device_create() bodges in drivers so for DT. If you can only discover SMMUs from IORT, then iort_init_platform_devices() will have already created every SMMU that's going to exist before discovering other devices from wherever they come from, thus you could never get into the situation of probing a device without its SMMU being ready (if it's ever going to be). Is that not right? Robin. > > Thanks, > Lorenzo > >> return NULL; >> >> err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); >> if (err) >> return ERR_PTR(err); >> + /* >> + * The otherwise-empty fwspec handily serves to indicate the specific >> + * IOMMU device we're waiting for, which will be useful if we ever get >> + * a proper probe-ordering dependency mechanism in future. >> + */ >> + if (!ops) >> + return ERR_PTR(-EPROBE_DEFER); >> >> err = ops->of_xlate(dev, iommu_spec); >> if (err) >> @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, >> struct device_node *master_np) >> { >> const struct iommu_ops *ops; >> + struct iommu_fwspec *fwspec = dev->iommu_fwspec; >> >> if (!master_np) >> return NULL; >> >> + if (fwspec) { >> + if (fwspec->ops) >> + return fwspec->ops; >> + >> + /* In the deferred case, start again from scratch */ >> + iommu_fwspec_free(dev); >> + } >> + >> if (dev_is_pci(dev)) >> ops = of_pci_iommu_init(to_pci_dev(dev), master_np); >> else >> ops = of_platform_iommu_init(dev, master_np); >> + /* >> + * If we have reason to believe the IOMMU driver missed the initial >> + * add_device callback for dev, replay it to get things in order. >> + */ >> + if (!IS_ERR_OR_NULL(ops) && ops->add_device && >> + dev->bus && !dev->iommu_group) { >> + int err = ops->add_device(dev); >> + >> + if (err) >> + ops = ERR_PTR(err); >> + } >> >> return IS_ERR(ops) ? NULL : ops; >> } >> -- >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation >>
On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote: > On 30/11/16 16:17, Lorenzo Pieralisi wrote: > > Sricharan, Robin, > > > > I gave this series a go on ACPI and apart from an SMMU v3 fix-up > > it seems to work, more thorough testing required though. > > > > A key question below. > > > > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: > >> From: Robin Murphy <robin.murphy@arm.com> > >> > >> IOMMU configuration represents unchanging properties of the hardware, > >> and as such should only need happen once in a device's lifetime, but > >> the necessary interaction with the IOMMU device and driver complicates > >> exactly when that point should be. > >> > >> Since the only reasonable tool available for handling the inter-device > >> dependency is probe deferral, we need to prepare of_iommu_configure() > >> to run later than it is currently called (i.e. at driver probe rather > >> than device creation), to handle being retried, and to tell whether a > >> not-yet present IOMMU should be waited for or skipped (by virtue of > >> having declared a built-in driver or not). > >> > >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> > >> --- > >> drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- > >> 1 file changed, 29 insertions(+), 1 deletion(-) > >> > >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > >> index ee49081..349bd1d 100644 > >> --- a/drivers/iommu/of_iommu.c > >> +++ b/drivers/iommu/of_iommu.c > >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, > >> int err; > >> > >> ops = iommu_get_instance(fwnode); > >> - if (!ops || !ops->of_xlate) > >> + if ((ops && !ops->of_xlate) || > >> + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) > > > > IIUC of_match_node() here is there to check there is a driver compiled > > in for this device_node (aka compatible string in OF world), correct ? > > Yes - specifically, it's checking the magic table for a matching > IOMMU_OF_DECLARE entry. > > > If that's the case (and I think that's what Sricharan was referring to > > in his ACPI query) I need to cook-up something on the ACPI side to > > emulate the OF linker table behaviour (or anyway to detect a driver is > > actually in the kernel), it is not that difficult but it is key to know, > > I will give it some thought to make it as clean as possible. > > I didn't think this would be a concern for ACPI, since IORT works much > the same way the current of_iommu_init_fn/of_platform_device_create() > bodges in drivers so for DT. If you can only discover SMMUs from IORT, > then iort_init_platform_devices() will have already created every SMMU > that's going to exist before discovering other devices from wherever > they come from, thus you could never get into the situation of probing a > device without its SMMU being ready (if it's ever going to be). Is that > not right? It is right, my point and question is: we are probing a device and we have to know whether it is worth deferring its IOMMU DMA setup. On DT, through of_match_node(&__iommu_of_table, iommu_device_node) we check at once that: 1 - A device for the IOMMU exists AND 2 - A driver for the IOMMU is compiled in the kernel Is this correct ? As you said (1) is not a concern on ACPI IORT (because we create the IOMMU device before _any_ other device so either the IOMMU device is there or it will never be by the time master devices are probed), but for (2) I need to slightly change how the IORT linker entry work to make sure we can detect a driver is actually compiled in the kernel, it is easy, I was just asking if my understanding was correct and I think that was what Sricharan was referring to in his query. I will put together a patch, again, it is a simple tweak. Thanks ! Lorenzo > Robin. > > > > > Thanks, > > Lorenzo > > > >> return NULL; > >> > >> err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); > >> if (err) > >> return ERR_PTR(err); > >> + /* > >> + * The otherwise-empty fwspec handily serves to indicate the specific > >> + * IOMMU device we're waiting for, which will be useful if we ever get > >> + * a proper probe-ordering dependency mechanism in future. > >> + */ > >> + if (!ops) > >> + return ERR_PTR(-EPROBE_DEFER); > >> > >> err = ops->of_xlate(dev, iommu_spec); > >> if (err) > >> @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, > >> struct device_node *master_np) > >> { > >> const struct iommu_ops *ops; > >> + struct iommu_fwspec *fwspec = dev->iommu_fwspec; > >> > >> if (!master_np) > >> return NULL; > >> > >> + if (fwspec) { > >> + if (fwspec->ops) > >> + return fwspec->ops; > >> + > >> + /* In the deferred case, start again from scratch */ > >> + iommu_fwspec_free(dev); > >> + } > >> + > >> if (dev_is_pci(dev)) > >> ops = of_pci_iommu_init(to_pci_dev(dev), master_np); > >> else > >> ops = of_platform_iommu_init(dev, master_np); > >> + /* > >> + * If we have reason to believe the IOMMU driver missed the initial > >> + * add_device callback for dev, replay it to get things in order. > >> + */ > >> + if (!IS_ERR_OR_NULL(ops) && ops->add_device && > >> + dev->bus && !dev->iommu_group) { > >> + int err = ops->add_device(dev); > >> + > >> + if (err) > >> + ops = ERR_PTR(err); > >> + } > >> > >> return IS_ERR(ops) ? NULL : ops; > >> } > >> -- > >> QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation > >> >
Hi Robin,Lorenzo, >On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote: >> On 30/11/16 16:17, Lorenzo Pieralisi wrote: >> > Sricharan, Robin, >> > >> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up >> > it seems to work, more thorough testing required though. >> > >> > A key question below. >> > >> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: >> >> From: Robin Murphy <robin.murphy@arm.com> >> >> >> >> IOMMU configuration represents unchanging properties of the hardware, >> >> and as such should only need happen once in a device's lifetime, but >> >> the necessary interaction with the IOMMU device and driver complicates >> >> exactly when that point should be. >> >> >> >> Since the only reasonable tool available for handling the inter-device >> >> dependency is probe deferral, we need to prepare of_iommu_configure() >> >> to run later than it is currently called (i.e. at driver probe rather >> >> than device creation), to handle being retried, and to tell whether a >> >> not-yet present IOMMU should be waited for or skipped (by virtue of >> >> having declared a built-in driver or not). >> >> >> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >> >> --- >> >> drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- >> >> 1 file changed, 29 insertions(+), 1 deletion(-) >> >> >> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >> >> index ee49081..349bd1d 100644 >> >> --- a/drivers/iommu/of_iommu.c >> >> +++ b/drivers/iommu/of_iommu.c >> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, >> >> int err; >> >> >> >> ops = iommu_get_instance(fwnode); >> >> - if (!ops || !ops->of_xlate) >> >> + if ((ops && !ops->of_xlate) || >> >> + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) >> > >> > IIUC of_match_node() here is there to check there is a driver compiled >> > in for this device_node (aka compatible string in OF world), correct ? >> >> Yes - specifically, it's checking the magic table for a matching >> IOMMU_OF_DECLARE entry. >> >> > If that's the case (and I think that's what Sricharan was referring to >> > in his ACPI query) I need to cook-up something on the ACPI side to >> > emulate the OF linker table behaviour (or anyway to detect a driver is >> > actually in the kernel), it is not that difficult but it is key to know, >> > I will give it some thought to make it as clean as possible. >> >> I didn't think this would be a concern for ACPI, since IORT works much >> the same way the current of_iommu_init_fn/of_platform_device_create() >> bodges in drivers so for DT. If you can only discover SMMUs from IORT, >> then iort_init_platform_devices() will have already created every SMMU >> that's going to exist before discovering other devices from wherever >> they come from, thus you could never get into the situation of probing a >> device without its SMMU being ready (if it's ever going to be). Is that >> not right? > >It is right, my point and question is: we are probing a device and we >have to know whether it is worth deferring its IOMMU DMA setup. On DT, >through of_match_node(&__iommu_of_table, iommu_device_node) we check at >once that: > >1 - A device for the IOMMU exists > >AND > >2 - A driver for the IOMMU is compiled in the kernel > >Is this correct ? As you said (1) is not a concern on ACPI IORT (because >we create the IOMMU device before _any_ other device so either the IOMMU >device is there or it will never be by the time master devices are >probed), but for (2) I need to slightly change how the IORT linker entry >work to make sure we can detect a driver is actually compiled in the >kernel, it is easy, I was just asking if my understanding was correct >and I think that was what Sricharan was referring to in his query. > Yes right, this was what i was looking for in the ACPI case and putting this in the iort_iommu_xlate was needed to return EPROBE_DEFER when the driver is not yet been probed. Regards, Sricharan
Hi Robin/Lorenzo, >Hi Robin,Lorenzo, > >>On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote: >>> On 30/11/16 16:17, Lorenzo Pieralisi wrote: >>> > Sricharan, Robin, >>> > >>> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up >>> > it seems to work, more thorough testing required though. >>> > >>> > A key question below. >>> > >>> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: >>> >> From: Robin Murphy <robin.murphy@arm.com> >>> >> >>> >> IOMMU configuration represents unchanging properties of the hardware, >>> >> and as such should only need happen once in a device's lifetime, but >>> >> the necessary interaction with the IOMMU device and driver complicates >>> >> exactly when that point should be. >>> >> >>> >> Since the only reasonable tool available for handling the inter-device >>> >> dependency is probe deferral, we need to prepare of_iommu_configure() >>> >> to run later than it is currently called (i.e. at driver probe rather >>> >> than device creation), to handle being retried, and to tell whether a >>> >> not-yet present IOMMU should be waited for or skipped (by virtue of >>> >> having declared a built-in driver or not). >>> >> >>> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>> >> --- >>> >> drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- >>> >> 1 file changed, 29 insertions(+), 1 deletion(-) >>> >> >>> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >>> >> index ee49081..349bd1d 100644 >>> >> --- a/drivers/iommu/of_iommu.c >>> >> +++ b/drivers/iommu/of_iommu.c >>> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, >>> >> int err; >>> >> >>> >> ops = iommu_get_instance(fwnode); >>> >> - if (!ops || !ops->of_xlate) >>> >> + if ((ops && !ops->of_xlate) || >>> >> + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) >>> > >>> > IIUC of_match_node() here is there to check there is a driver compiled >>> > in for this device_node (aka compatible string in OF world), correct ? >>> >>> Yes - specifically, it's checking the magic table for a matching >>> IOMMU_OF_DECLARE entry. >>> >>> > If that's the case (and I think that's what Sricharan was referring to >>> > in his ACPI query) I need to cook-up something on the ACPI side to >>> > emulate the OF linker table behaviour (or anyway to detect a driver is >>> > actually in the kernel), it is not that difficult but it is key to know, >>> > I will give it some thought to make it as clean as possible. >>> >>> I didn't think this would be a concern for ACPI, since IORT works much >>> the same way the current of_iommu_init_fn/of_platform_device_create() >>> bodges in drivers so for DT. If you can only discover SMMUs from IORT, >>> then iort_init_platform_devices() will have already created every SMMU >>> that's going to exist before discovering other devices from wherever >>> they come from, thus you could never get into the situation of probing a >>> device without its SMMU being ready (if it's ever going to be). Is that >>> not right? >> >>It is right, my point and question is: we are probing a device and we >>have to know whether it is worth deferring its IOMMU DMA setup. On DT, >>through of_match_node(&__iommu_of_table, iommu_device_node) we check at >>once that: >> >>1 - A device for the IOMMU exists >> >>AND >> >>2 - A driver for the IOMMU is compiled in the kernel >> >>Is this correct ? As you said (1) is not a concern on ACPI IORT (because >>we create the IOMMU device before _any_ other device so either the IOMMU >>device is there or it will never be by the time master devices are >>probed), but for (2) I need to slightly change how the IORT linker entry >>work to make sure we can detect a driver is actually compiled in the >>kernel, it is easy, I was just asking if my understanding was correct >>and I think that was what Sricharan was referring to in his query. >> > >Yes right, this was what i was looking for in the ACPI case and putting this >in the iort_iommu_xlate was needed to return EPROBE_DEFER when the >driver is not yet been probed. With the thinking of taking this series through, would it be fine if i cleanup the pci configure hanging outside and push it in to of/acpi_iommu configure respectively ? This time with all neeeded for ACPI added as well. Also on the last post of V4, Lorenzo commented that it worked for him, although still the of_match_node equivalent in ACPI has to be added. If i can get that, then i will add that as well to make this complete. Regards, Sricharan
On Thu, Jan 05, 2017 at 02:04:37PM +0530, Sricharan wrote: > Hi Robin/Lorenzo, > > >Hi Robin,Lorenzo, > > > >>On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote: > >>> On 30/11/16 16:17, Lorenzo Pieralisi wrote: > >>> > Sricharan, Robin, > >>> > > >>> > I gave this series a go on ACPI and apart from an SMMU v3 fix-up > >>> > it seems to work, more thorough testing required though. > >>> > > >>> > A key question below. > >>> > > >>> > On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: > >>> >> From: Robin Murphy <robin.murphy@arm.com> > >>> >> > >>> >> IOMMU configuration represents unchanging properties of the hardware, > >>> >> and as such should only need happen once in a device's lifetime, but > >>> >> the necessary interaction with the IOMMU device and driver complicates > >>> >> exactly when that point should be. > >>> >> > >>> >> Since the only reasonable tool available for handling the inter-device > >>> >> dependency is probe deferral, we need to prepare of_iommu_configure() > >>> >> to run later than it is currently called (i.e. at driver probe rather > >>> >> than device creation), to handle being retried, and to tell whether a > >>> >> not-yet present IOMMU should be waited for or skipped (by virtue of > >>> >> having declared a built-in driver or not). > >>> >> > >>> >> Signed-off-by: Robin Murphy <robin.murphy@arm.com> > >>> >> --- > >>> >> drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- > >>> >> 1 file changed, 29 insertions(+), 1 deletion(-) > >>> >> > >>> >> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > >>> >> index ee49081..349bd1d 100644 > >>> >> --- a/drivers/iommu/of_iommu.c > >>> >> +++ b/drivers/iommu/of_iommu.c > >>> >> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, > >>> >> int err; > >>> >> > >>> >> ops = iommu_get_instance(fwnode); > >>> >> - if (!ops || !ops->of_xlate) > >>> >> + if ((ops && !ops->of_xlate) || > >>> >> + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) > >>> > > >>> > IIUC of_match_node() here is there to check there is a driver compiled > >>> > in for this device_node (aka compatible string in OF world), correct ? > >>> > >>> Yes - specifically, it's checking the magic table for a matching > >>> IOMMU_OF_DECLARE entry. > >>> > >>> > If that's the case (and I think that's what Sricharan was referring to > >>> > in his ACPI query) I need to cook-up something on the ACPI side to > >>> > emulate the OF linker table behaviour (or anyway to detect a driver is > >>> > actually in the kernel), it is not that difficult but it is key to know, > >>> > I will give it some thought to make it as clean as possible. > >>> > >>> I didn't think this would be a concern for ACPI, since IORT works much > >>> the same way the current of_iommu_init_fn/of_platform_device_create() > >>> bodges in drivers so for DT. If you can only discover SMMUs from IORT, > >>> then iort_init_platform_devices() will have already created every SMMU > >>> that's going to exist before discovering other devices from wherever > >>> they come from, thus you could never get into the situation of probing a > >>> device without its SMMU being ready (if it's ever going to be). Is that > >>> not right? > >> > >>It is right, my point and question is: we are probing a device and we > >>have to know whether it is worth deferring its IOMMU DMA setup. On DT, > >>through of_match_node(&__iommu_of_table, iommu_device_node) we check at > >>once that: > >> > >>1 - A device for the IOMMU exists > >> > >>AND > >> > >>2 - A driver for the IOMMU is compiled in the kernel > >> > >>Is this correct ? As you said (1) is not a concern on ACPI IORT (because > >>we create the IOMMU device before _any_ other device so either the IOMMU > >>device is there or it will never be by the time master devices are > >>probed), but for (2) I need to slightly change how the IORT linker entry > >>work to make sure we can detect a driver is actually compiled in the > >>kernel, it is easy, I was just asking if my understanding was correct > >>and I think that was what Sricharan was referring to in his query. > >> > > > >Yes right, this was what i was looking for in the ACPI case and putting this > >in the iort_iommu_xlate was needed to return EPROBE_DEFER when the > >driver is not yet been probed. > > With the thinking of taking this series through, would it be fine if i > cleanup the pci configure hanging outside and push it in to > of/acpi_iommu configure respectively ? This time with all neeeded for > ACPI added as well. Also on the last post of V4, Lorenzo commented > that it worked for him, although still the of_match_node equivalent in > ACPI has to be added. If i can get that, then i will add that as well > to make this complete. Question: I had a look into this and instead of fiddling about with the linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this patchset would help remove entirely), I think that the only check we need in IORT is, depending on what type of SMMU a given device is connected to, to check if the respective SMMU driver is compiled in the kernel and it will be probed, _eventually_. As Robin said, by the time a device is probed the respective SMMU devices are already created and registered with IORT kernel code or they will never be, so to understand if we should defer probing SMMU device creation is _not_ really a problem in ACPI. To check if a SMMU driver is enabled, do we really need a linker table ? Would not a check based on eg: /** * @type: IOMMU IORT node type of the IOMMU a device is connected to */ static bool iort_iommu_driver_enabled(u8 type) { switch (type) { case ACPI_IORT_SMMU_V3: return IS_ENABLED(CONFIG_ARM_SMMU_V3); case ACPI_IORT_SMMU: return IS_ENABLED(CONFIG_ARM_SMMU); default: pr_warn("Unknown IORT SMMU type\n"); return false; } } be sufficient (it is a bit gross, agreed, but it is to understand if that's all we need) ? Is there anything I am missing ? Let me know, I will put together a patch for you I really do not want to block your series for this trivial niggle. Thanks, Lorenzo
On 05/01/17 12:27, Lorenzo Pieralisi wrote: > On Thu, Jan 05, 2017 at 02:04:37PM +0530, Sricharan wrote: >> Hi Robin/Lorenzo, >> >>> Hi Robin,Lorenzo, >>> >>>> On Wed, Nov 30, 2016 at 04:42:27PM +0000, Robin Murphy wrote: >>>>> On 30/11/16 16:17, Lorenzo Pieralisi wrote: >>>>>> Sricharan, Robin, >>>>>> >>>>>> I gave this series a go on ACPI and apart from an SMMU v3 fix-up >>>>>> it seems to work, more thorough testing required though. >>>>>> >>>>>> A key question below. >>>>>> >>>>>> On Wed, Nov 30, 2016 at 05:52:16AM +0530, Sricharan R wrote: >>>>>>> From: Robin Murphy <robin.murphy@arm.com> >>>>>>> >>>>>>> IOMMU configuration represents unchanging properties of the hardware, >>>>>>> and as such should only need happen once in a device's lifetime, but >>>>>>> the necessary interaction with the IOMMU device and driver complicates >>>>>>> exactly when that point should be. >>>>>>> >>>>>>> Since the only reasonable tool available for handling the inter-device >>>>>>> dependency is probe deferral, we need to prepare of_iommu_configure() >>>>>>> to run later than it is currently called (i.e. at driver probe rather >>>>>>> than device creation), to handle being retried, and to tell whether a >>>>>>> not-yet present IOMMU should be waited for or skipped (by virtue of >>>>>>> having declared a built-in driver or not). >>>>>>> >>>>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com> >>>>>>> --- >>>>>>> drivers/iommu/of_iommu.c | 30 +++++++++++++++++++++++++++++- >>>>>>> 1 file changed, 29 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c >>>>>>> index ee49081..349bd1d 100644 >>>>>>> --- a/drivers/iommu/of_iommu.c >>>>>>> +++ b/drivers/iommu/of_iommu.c >>>>>>> @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, >>>>>>> int err; >>>>>>> >>>>>>> ops = iommu_get_instance(fwnode); >>>>>>> - if (!ops || !ops->of_xlate) >>>>>>> + if ((ops && !ops->of_xlate) || >>>>>>> + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) >>>>>> >>>>>> IIUC of_match_node() here is there to check there is a driver compiled >>>>>> in for this device_node (aka compatible string in OF world), correct ? >>>>> >>>>> Yes - specifically, it's checking the magic table for a matching >>>>> IOMMU_OF_DECLARE entry. >>>>> >>>>>> If that's the case (and I think that's what Sricharan was referring to >>>>>> in his ACPI query) I need to cook-up something on the ACPI side to >>>>>> emulate the OF linker table behaviour (or anyway to detect a driver is >>>>>> actually in the kernel), it is not that difficult but it is key to know, >>>>>> I will give it some thought to make it as clean as possible. >>>>> >>>>> I didn't think this would be a concern for ACPI, since IORT works much >>>>> the same way the current of_iommu_init_fn/of_platform_device_create() >>>>> bodges in drivers so for DT. If you can only discover SMMUs from IORT, >>>>> then iort_init_platform_devices() will have already created every SMMU >>>>> that's going to exist before discovering other devices from wherever >>>>> they come from, thus you could never get into the situation of probing a >>>>> device without its SMMU being ready (if it's ever going to be). Is that >>>>> not right? >>>> >>>> It is right, my point and question is: we are probing a device and we >>>> have to know whether it is worth deferring its IOMMU DMA setup. On DT, >>>> through of_match_node(&__iommu_of_table, iommu_device_node) we check at >>>> once that: >>>> >>>> 1 - A device for the IOMMU exists >>>> >>>> AND >>>> >>>> 2 - A driver for the IOMMU is compiled in the kernel >>>> >>>> Is this correct ? As you said (1) is not a concern on ACPI IORT (because >>>> we create the IOMMU device before _any_ other device so either the IOMMU >>>> device is there or it will never be by the time master devices are >>>> probed), but for (2) I need to slightly change how the IORT linker entry >>>> work to make sure we can detect a driver is actually compiled in the >>>> kernel, it is easy, I was just asking if my understanding was correct >>>> and I think that was what Sricharan was referring to in his query. >>>> >>> >>> Yes right, this was what i was looking for in the ACPI case and putting this >>> in the iort_iommu_xlate was needed to return EPROBE_DEFER when the >>> driver is not yet been probed. >> >> With the thinking of taking this series through, would it be fine if i >> cleanup the pci configure hanging outside and push it in to >> of/acpi_iommu configure respectively ? This time with all neeeded for >> ACPI added as well. Also on the last post of V4, Lorenzo commented >> that it worked for him, although still the of_match_node equivalent in >> ACPI has to be added. If i can get that, then i will add that as well >> to make this complete. > > Question: I had a look into this and instead of fiddling about with the > linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this > patchset would help remove entirely), I think that the only check we > need in IORT is, depending on what type of SMMU a given device is > connected to, to check if the respective SMMU driver is compiled in the > kernel and it will be probed, _eventually_. > > As Robin said, by the time a device is probed the respective SMMU > devices are already created and registered with IORT kernel code or > they will never be, so to understand if we should defer probing > SMMU device creation is _not_ really a problem in ACPI. > > To check if a SMMU driver is enabled, do we really need a linker > table ? > > Would not a check based on eg: > > /** > * @type: IOMMU IORT node type of the IOMMU a device is connected to > */ > static bool iort_iommu_driver_enabled(u8 type) > { > switch (type) { > case ACPI_IORT_SMMU_V3: > return IS_ENABLED(CONFIG_ARM_SMMU_V3); IS_BUILTIN(...) > case ACPI_IORT_SMMU: > return IS_ENABLED(CONFIG_ARM_SMMU); > default: > pr_warn("Unknown IORT SMMU type\n"); Might displaying the actual value be helfpul for debugging a broken IORT table? > return false; > } > } > > be sufficient (it is a bit gross, agreed, but it is to understand if > that's all we need) ? Is there anything I am missing ? > > Let me know, I will put together a patch for you I really do not > want to block your series for this trivial niggle. Other than that, though, I like it :) IORT has a small, strictly bounded, set of supported devices, so I really don't see the need to go overboard putting it on parity with DT when something this neat and simple will suffice. Robin. > > Thanks, > Lorenzo >
Hi, [...] >>> >>> With the thinking of taking this series through, would it be fine if i >>> cleanup the pci configure hanging outside and push it in to >>> of/acpi_iommu configure respectively ? This time with all neeeded for >>> ACPI added as well. Also on the last post of V4, Lorenzo commented >>> that it worked for him, although still the of_match_node equivalent in >>> ACPI has to be added. If i can get that, then i will add that as well >>> to make this complete. >> >> Question: I had a look into this and instead of fiddling about with the >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this >> patchset would help remove entirely), I think that the only check we >> need in IORT is, depending on what type of SMMU a given device is >> connected to, to check if the respective SMMU driver is compiled in the >> kernel and it will be probed, _eventually_. >> >> As Robin said, by the time a device is probed the respective SMMU >> devices are already created and registered with IORT kernel code or >> they will never be, so to understand if we should defer probing >> SMMU device creation is _not_ really a problem in ACPI. >> >> To check if a SMMU driver is enabled, do we really need a linker >> table ? >> >> Would not a check based on eg: >> >> /** >> * @type: IOMMU IORT node type of the IOMMU a device is connected to >> */ >> static bool iort_iommu_driver_enabled(u8 type) >> { >> switch (type) { >> case ACPI_IORT_SMMU_V3: >> return IS_ENABLED(CONFIG_ARM_SMMU_V3); > >IS_BUILTIN(...) > >> case ACPI_IORT_SMMU: >> return IS_ENABLED(CONFIG_ARM_SMMU); >> default: >> pr_warn("Unknown IORT SMMU type\n"); > >Might displaying the actual value be helfpul for debugging a broken IORT >table? > >> return false; >> } >> } >> >> be sufficient (it is a bit gross, agreed, but it is to understand if >> that's all we need) ? Is there anything I am missing ? >> >> Let me know, I will put together a patch for you I really do not >> want to block your series for this trivial niggle. > >Other than that, though, I like it :) IORT has a small, strictly >bounded, set of supported devices, so I really don't see the need to go >overboard putting it on parity with DT when something this neat and >simple will suffice. > Ok sure, looks correct for me as well in whole of the context here. Regards, Sricharan
On Thu, Jan 05, 2017 at 01:52:29PM +0000, Robin Murphy wrote: [...] > > Question: I had a look into this and instead of fiddling about with the > > linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this > > patchset would help remove entirely), I think that the only check we > > need in IORT is, depending on what type of SMMU a given device is > > connected to, to check if the respective SMMU driver is compiled in the > > kernel and it will be probed, _eventually_. > > > > As Robin said, by the time a device is probed the respective SMMU > > devices are already created and registered with IORT kernel code or > > they will never be, so to understand if we should defer probing > > SMMU device creation is _not_ really a problem in ACPI. > > > > To check if a SMMU driver is enabled, do we really need a linker > > table ? > > > > Would not a check based on eg: > > > > /** > > * @type: IOMMU IORT node type of the IOMMU a device is connected to > > */ > > static bool iort_iommu_driver_enabled(u8 type) > > { > > switch (type) { > > case ACPI_IORT_SMMU_V3: > > return IS_ENABLED(CONFIG_ARM_SMMU_V3); > > IS_BUILTIN(...) Yep right, it is currently equivalent but that does not mean it will always be. > > case ACPI_IORT_SMMU: > > return IS_ENABLED(CONFIG_ARM_SMMU); > > default: > > pr_warn("Unknown IORT SMMU type\n"); > > Might displaying the actual value be helfpul for debugging a broken IORT > table? Yes I will do. > > return false; > > } > > } > > > > be sufficient (it is a bit gross, agreed, but it is to understand if > > that's all we need) ? Is there anything I am missing ? > > > > Let me know, I will put together a patch for you I really do not > > want to block your series for this trivial niggle. > > Other than that, though, I like it :) IORT has a small, strictly > bounded, set of supported devices, so I really don't see the need to go > overboard putting it on parity with DT when something this neat and > simple will suffice. Ok, patch coming, which will also allow Sricharan to get rid of the IORT linker script infrastructure altogether (and more than that, I can write the patches on top of Sricharan series that I managed to rebase against v4.10-rc2). Thanks, Lorenzo
On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote: > Hi, > > [...] > > >>> > >>> With the thinking of taking this series through, would it be fine if i > >>> cleanup the pci configure hanging outside and push it in to > >>> of/acpi_iommu configure respectively ? This time with all neeeded for > >>> ACPI added as well. Also on the last post of V4, Lorenzo commented > >>> that it worked for him, although still the of_match_node equivalent in > >>> ACPI has to be added. If i can get that, then i will add that as well > >>> to make this complete. > >> > >> Question: I had a look into this and instead of fiddling about with the > >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this > >> patchset would help remove entirely), I think that the only check we > >> need in IORT is, depending on what type of SMMU a given device is > >> connected to, to check if the respective SMMU driver is compiled in the > >> kernel and it will be probed, _eventually_. > >> > >> As Robin said, by the time a device is probed the respective SMMU > >> devices are already created and registered with IORT kernel code or > >> they will never be, so to understand if we should defer probing > >> SMMU device creation is _not_ really a problem in ACPI. > >> > >> To check if a SMMU driver is enabled, do we really need a linker > >> table ? > >> > >> Would not a check based on eg: > >> > >> /** > >> * @type: IOMMU IORT node type of the IOMMU a device is connected to > >> */ > >> static bool iort_iommu_driver_enabled(u8 type) > >> { > >> switch (type) { > >> case ACPI_IORT_SMMU_V3: > >> return IS_ENABLED(CONFIG_ARM_SMMU_V3); > > > >IS_BUILTIN(...) > > > >> case ACPI_IORT_SMMU: > >> return IS_ENABLED(CONFIG_ARM_SMMU); > >> default: > >> pr_warn("Unknown IORT SMMU type\n"); > > > >Might displaying the actual value be helfpul for debugging a broken IORT > >table? > > > >> return false; > >> } > >> } > >> > >> be sufficient (it is a bit gross, agreed, but it is to understand if > >> that's all we need) ? Is there anything I am missing ? > >> > >> Let me know, I will put together a patch for you I really do not > >> want to block your series for this trivial niggle. > > > >Other than that, though, I like it :) IORT has a small, strictly > >bounded, set of supported devices, so I really don't see the need to go > >overboard putting it on parity with DT when something this neat and > >simple will suffice. > > > > Ok sure, looks correct for me as well in whole of the context here. Ok, I put together a branch where you can find your original series plus some ACPI patches for you to test and use: git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral Feel free to post the additional patches I added along with your series (that from what I gather you have reworked already) and please both have a look if the deferral mechanism I put in place in ACPI makes sense to you. Please CC linux-acpi upon posting, if you need help shout. Lorenzo
Hi Sricharan, On Fri, Jan 06, 2017 at 04:24:00PM +0000, Lorenzo Pieralisi wrote: > On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote: > > Hi, > > > > [...] > > > > >>> > > >>> With the thinking of taking this series through, would it be fine if i > > >>> cleanup the pci configure hanging outside and push it in to > > >>> of/acpi_iommu configure respectively ? This time with all neeeded for > > >>> ACPI added as well. Also on the last post of V4, Lorenzo commented > > >>> that it worked for him, although still the of_match_node equivalent in > > >>> ACPI has to be added. If i can get that, then i will add that as well > > >>> to make this complete. > > >> > > >> Question: I had a look into this and instead of fiddling about with the > > >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this > > >> patchset would help remove entirely), I think that the only check we > > >> need in IORT is, depending on what type of SMMU a given device is > > >> connected to, to check if the respective SMMU driver is compiled in the > > >> kernel and it will be probed, _eventually_. > > >> > > >> As Robin said, by the time a device is probed the respective SMMU > > >> devices are already created and registered with IORT kernel code or > > >> they will never be, so to understand if we should defer probing > > >> SMMU device creation is _not_ really a problem in ACPI. > > >> > > >> To check if a SMMU driver is enabled, do we really need a linker > > >> table ? > > >> > > >> Would not a check based on eg: > > >> > > >> /** > > >> * @type: IOMMU IORT node type of the IOMMU a device is connected to > > >> */ > > >> static bool iort_iommu_driver_enabled(u8 type) > > >> { > > >> switch (type) { > > >> case ACPI_IORT_SMMU_V3: > > >> return IS_ENABLED(CONFIG_ARM_SMMU_V3); > > > > > >IS_BUILTIN(...) > > > > > >> case ACPI_IORT_SMMU: > > >> return IS_ENABLED(CONFIG_ARM_SMMU); > > >> default: > > >> pr_warn("Unknown IORT SMMU type\n"); > > > > > >Might displaying the actual value be helfpul for debugging a broken IORT > > >table? > > > > > >> return false; > > >> } > > >> } > > >> > > >> be sufficient (it is a bit gross, agreed, but it is to understand if > > >> that's all we need) ? Is there anything I am missing ? > > >> > > >> Let me know, I will put together a patch for you I really do not > > >> want to block your series for this trivial niggle. > > > > > >Other than that, though, I like it :) IORT has a small, strictly > > >bounded, set of supported devices, so I really don't see the need to go > > >overboard putting it on parity with DT when something this neat and > > >simple will suffice. > > > > > > > Ok sure, looks correct for me as well in whole of the context here. > > Ok, I put together a branch where you can find your original series > plus some ACPI patches for you to test and use: > > git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral > > Feel free to post the additional patches I added along with your series > (that from what I gather you have reworked already) and please both have a > look if the deferral mechanism I put in place in ACPI makes sense to you. Did you have time to make progress on this ? I think it is time we posted the complete series to aim for 4.11 please, if you need help just let us know. Thanks ! Lorenzo
Hi Lorenzo, >-----Original Message----- >From: linux-arm-kernel [mailto:linux-arm-kernel-bounces@lists.infradead.org] On Behalf Of Lorenzo Pieralisi >Sent: Thursday, January 19, 2017 8:11 PM >To: Sricharan <sricharan@codeaurora.org> >Cc: linux-arm-msm@vger.kernel.org; joro@8bytes.org; will.deacon@arm.com; iommu@lists.linux-foundation.org; 'Robin Murphy' ><robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org >Subject: Re: [PATCH 02/10] iommu/of: Prepare for deferred IOMMU configuration > >Hi Sricharan, > >On Fri, Jan 06, 2017 at 04:24:00PM +0000, Lorenzo Pieralisi wrote: >> On Thu, Jan 05, 2017 at 08:21:53PM +0530, Sricharan wrote: >> > Hi, >> > >> > [...] >> > >> > >>> >> > >>> With the thinking of taking this series through, would it be fine if i >> > >>> cleanup the pci configure hanging outside and push it in to >> > >>> of/acpi_iommu configure respectively ? This time with all neeeded for >> > >>> ACPI added as well. Also on the last post of V4, Lorenzo commented >> > >>> that it worked for him, although still the of_match_node equivalent in >> > >>> ACPI has to be added. If i can get that, then i will add that as well >> > >>> to make this complete. >> > >> >> > >> Question: I had a look into this and instead of fiddling about with the >> > >> linker script entries in ACPI (ie IORT_ACPI_DECLARE - which I hope this >> > >> patchset would help remove entirely), I think that the only check we >> > >> need in IORT is, depending on what type of SMMU a given device is >> > >> connected to, to check if the respective SMMU driver is compiled in the >> > >> kernel and it will be probed, _eventually_. >> > >> >> > >> As Robin said, by the time a device is probed the respective SMMU >> > >> devices are already created and registered with IORT kernel code or >> > >> they will never be, so to understand if we should defer probing >> > >> SMMU device creation is _not_ really a problem in ACPI. >> > >> >> > >> To check if a SMMU driver is enabled, do we really need a linker >> > >> table ? >> > >> >> > >> Would not a check based on eg: >> > >> >> > >> /** >> > >> * @type: IOMMU IORT node type of the IOMMU a device is connected to >> > >> */ >> > >> static bool iort_iommu_driver_enabled(u8 type) >> > >> { >> > >> switch (type) { >> > >> case ACPI_IORT_SMMU_V3: >> > >> return IS_ENABLED(CONFIG_ARM_SMMU_V3); >> > > >> > >IS_BUILTIN(...) >> > > >> > >> case ACPI_IORT_SMMU: >> > >> return IS_ENABLED(CONFIG_ARM_SMMU); >> > >> default: >> > >> pr_warn("Unknown IORT SMMU type\n"); >> > > >> > >Might displaying the actual value be helfpul for debugging a broken IORT >> > >table? >> > > >> > >> return false; >> > >> } >> > >> } >> > >> >> > >> be sufficient (it is a bit gross, agreed, but it is to understand if >> > >> that's all we need) ? Is there anything I am missing ? >> > >> >> > >> Let me know, I will put together a patch for you I really do not >> > >> want to block your series for this trivial niggle. >> > > >> > >Other than that, though, I like it :) IORT has a small, strictly >> > >bounded, set of supported devices, so I really don't see the need to go >> > >overboard putting it on parity with DT when something this neat and >> > >simple will suffice. >> > > >> > >> > Ok sure, looks correct for me as well in whole of the context here. >> >> Ok, I put together a branch where you can find your original series >> plus some ACPI patches for you to test and use: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/linux.git iommu/probe-deferral >> >> Feel free to post the additional patches I added along with your series >> (that from what I gather you have reworked already) and please both have a >> look if the deferral mechanism I put in place in ACPI makes sense to you. > >Did you have time to make progress on this ? I think it is time we >posted the complete series to aim for 4.11 please, if you need help just >let us know. > I just posted V5 now. I have mainly reworked the pci dma configure code which was lying outside and having it all in one place in dma_configure. The ACPI changes from you looked fine for me. I have tested on arm64/32 with DT, and require testing help on ACPI. Thanks. Regards, Sricharan
diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index ee49081..349bd1d 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -104,12 +104,20 @@ int of_get_dma_window(struct device_node *dn, const char *prefix, int index, int err; ops = iommu_get_instance(fwnode); - if (!ops || !ops->of_xlate) + if ((ops && !ops->of_xlate) || + (!ops && !of_match_node(&__iommu_of_table, iommu_spec->np))) return NULL; err = iommu_fwspec_init(dev, &iommu_spec->np->fwnode, ops); if (err) return ERR_PTR(err); + /* + * The otherwise-empty fwspec handily serves to indicate the specific + * IOMMU device we're waiting for, which will be useful if we ever get + * a proper probe-ordering dependency mechanism in future. + */ + if (!ops) + return ERR_PTR(-EPROBE_DEFER); err = ops->of_xlate(dev, iommu_spec); if (err) @@ -186,14 +194,34 @@ const struct iommu_ops *of_iommu_configure(struct device *dev, struct device_node *master_np) { const struct iommu_ops *ops; + struct iommu_fwspec *fwspec = dev->iommu_fwspec; if (!master_np) return NULL; + if (fwspec) { + if (fwspec->ops) + return fwspec->ops; + + /* In the deferred case, start again from scratch */ + iommu_fwspec_free(dev); + } + if (dev_is_pci(dev)) ops = of_pci_iommu_init(to_pci_dev(dev), master_np); else ops = of_platform_iommu_init(dev, master_np); + /* + * If we have reason to believe the IOMMU driver missed the initial + * add_device callback for dev, replay it to get things in order. + */ + if (!IS_ERR_OR_NULL(ops) && ops->add_device && + dev->bus && !dev->iommu_group) { + int err = ops->add_device(dev); + + if (err) + ops = ERR_PTR(err); + } return IS_ERR(ops) ? NULL : ops; }