If the IPMMU driver is compiled in the kernel it will replace the
platform bus IOMMU ops on running the ipmmu_init() function, regardless
if there is any IPMMU hardware present or not. This screws up systems
that just want to build a generic kernel that runs on multiple platforms
and use a different IOMMU implementation.
Move the bus_set_iommu() call at the end of the ipmmu_probe() function
when we know that hardware is present. With current IOMMU framework it
should be safe (at least for OF case).
Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
platform_driver_register() and platform_driver_unregister(), replace
them with the module_platform_driver() macro call.
Signed-off-by: Liviu Dudau <[email protected]>
Cc: Laurent Pinchart <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 2a38aa15be17d..c60569c74678d 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1055,10 +1055,11 @@ static int ipmmu_probe(struct platform_device *pdev)
ipmmu_device_reset(mmu);
/*
- * We can't create the ARM mapping here as it requires the bus to have
- * an IOMMU, which only happens when bus_set_iommu() is called in
- * ipmmu_init() after the probe function returns.
+ * Now that we have validated the presence of the hardware, set
+ * the bus IOMMU ops to enable future domain and device setup.
*/
+ if (!iommu_present(&platform_bus_type))
+ bus_set_iommu(&platform_bus_type, &ipmmu_ops);
spin_lock(&ipmmu_devices_lock);
list_add(&mmu->list, &ipmmu_devices);
@@ -1100,27 +1101,7 @@ static struct platform_driver ipmmu_driver = {
.remove = ipmmu_remove,
};
-static int __init ipmmu_init(void)
-{
- int ret;
-
- ret = platform_driver_register(&ipmmu_driver);
- if (ret < 0)
- return ret;
-
- if (!iommu_present(&platform_bus_type))
- bus_set_iommu(&platform_bus_type, &ipmmu_ops);
-
- return 0;
-}
-
-static void __exit ipmmu_exit(void)
-{
- return platform_driver_unregister(&ipmmu_driver);
-}
-
-subsys_initcall(ipmmu_init);
-module_exit(ipmmu_exit);
+module_platform_driver(ipmmu_driver);
MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
--
2.14.1
Hi Liviu,
Thank you for the patch.
On Monday, 18 September 2017 13:04:44 EEST Liviu Dudau wrote:
> If the IPMMU driver is compiled in the kernel it will replace the
> platform bus IOMMU ops on running the ipmmu_init() function, regardless
> if there is any IPMMU hardware present or not. This screws up systems
> that just want to build a generic kernel that runs on multiple platforms
> and use a different IOMMU implementation.
>
> Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> when we know that hardware is present. With current IOMMU framework it
> should be safe (at least for OF case).
If I recall correctly the issue is that the IPMMU might be probed after bus
master devices when using the legacy IOMMU DT integration, which the ipmmu-
vmsa driver does on ARM32. Calling bus_set_iommu() from the probe function
will then result in some devices losing IOMMU support.
> Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> platform_driver_register() and platform_driver_unregister(), replace
> them with the module_platform_driver() macro call.
>
> Signed-off-by: Liviu Dudau <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> ---
> drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> 1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 2a38aa15be17d..c60569c74678d 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -1055,10 +1055,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> ipmmu_device_reset(mmu);
>
> /*
> - * We can't create the ARM mapping here as it requires the bus to have
> - * an IOMMU, which only happens when bus_set_iommu() is called in
> - * ipmmu_init() after the probe function returns.
> + * Now that we have validated the presence of the hardware, set
> + * the bus IOMMU ops to enable future domain and device setup.
> */
> + if (!iommu_present(&platform_bus_type))
> + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
>
> spin_lock(&ipmmu_devices_lock);
> list_add(&mmu->list, &ipmmu_devices);
What branch is this patch based on ? ipmmu_devices_lock isn't in mainline.
> @@ -1100,27 +1101,7 @@ static struct platform_driver ipmmu_driver = {
> .remove = ipmmu_remove,
> };
>
> -static int __init ipmmu_init(void)
> -{
> - int ret;
> -
> - ret = platform_driver_register(&ipmmu_driver);
> - if (ret < 0)
> - return ret;
> -
> - if (!iommu_present(&platform_bus_type))
> - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> -
> - return 0;
> -}
> -
> -static void __exit ipmmu_exit(void)
> -{
> - return platform_driver_unregister(&ipmmu_driver);
> -}
> -
> -subsys_initcall(ipmmu_init);
> -module_exit(ipmmu_exit);
> +module_platform_driver(ipmmu_driver);
>
> MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
--
Regards,
Laurent Pinchart
On Tue, Sep 19, 2017 at 10:07:58AM +0300, Laurent Pinchart wrote:
> Hi Liviu,
>
> Thank you for the patch.
>
> On Monday, 18 September 2017 13:04:44 EEST Liviu Dudau wrote:
> > If the IPMMU driver is compiled in the kernel it will replace the
> > platform bus IOMMU ops on running the ipmmu_init() function, regardless
> > if there is any IPMMU hardware present or not. This screws up systems
> > that just want to build a generic kernel that runs on multiple platforms
> > and use a different IOMMU implementation.
> >
> > Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> > when we know that hardware is present. With current IOMMU framework it
> > should be safe (at least for OF case).
>
> If I recall correctly the issue is that the IPMMU might be probed after bus
> master devices when using the legacy IOMMU DT integration, which the ipmmu-
> vmsa driver does on ARM32. Calling bus_set_iommu() from the probe function
> will then result in some devices losing IOMMU support.
This is on arm64, on the Arm Juno board.
>
> > Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> > platform_driver_register() and platform_driver_unregister(), replace
> > them with the module_platform_driver() macro call.
> >
> > Signed-off-by: Liviu Dudau <[email protected]>
> > Cc: Laurent Pinchart <[email protected]>
> > ---
> > drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> > 1 file changed, 5 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > index 2a38aa15be17d..c60569c74678d 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -1055,10 +1055,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> > ipmmu_device_reset(mmu);
> >
> > /*
> > - * We can't create the ARM mapping here as it requires the bus to have
> > - * an IOMMU, which only happens when bus_set_iommu() is called in
> > - * ipmmu_init() after the probe function returns.
> > + * Now that we have validated the presence of the hardware, set
> > + * the bus IOMMU ops to enable future domain and device setup.
> > */
> > + if (!iommu_present(&platform_bus_type))
> > + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> >
> > spin_lock(&ipmmu_devices_lock);
> > list_add(&mmu->list, &ipmmu_devices);
>
> What branch is this patch based on ? ipmmu_devices_lock isn't in mainline.
drm-next. :)
Looks like things have move forward since I've made the patch before
LPC, will update and resend.
Best regards,
Liviu
>
> > @@ -1100,27 +1101,7 @@ static struct platform_driver ipmmu_driver = {
> > .remove = ipmmu_remove,
> > };
> >
> > -static int __init ipmmu_init(void)
> > -{
> > - int ret;
> > -
> > - ret = platform_driver_register(&ipmmu_driver);
> > - if (ret < 0)
> > - return ret;
> > -
> > - if (!iommu_present(&platform_bus_type))
> > - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > -
> > - return 0;
> > -}
> > -
> > -static void __exit ipmmu_exit(void)
> > -{
> > - return platform_driver_unregister(&ipmmu_driver);
> > -}
> > -
> > -subsys_initcall(ipmmu_init);
> > -module_exit(ipmmu_exit);
> > +module_platform_driver(ipmmu_driver);
> >
> > MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> > MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
>
> --
> Regards,
>
> Laurent Pinchart
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
Hi Laurent,
On 19/09/17 08:07, Laurent Pinchart wrote:
> Hi Liviu,
>
> Thank you for the patch.
>
> On Monday, 18 September 2017 13:04:44 EEST Liviu Dudau wrote:
>> If the IPMMU driver is compiled in the kernel it will replace the
>> platform bus IOMMU ops on running the ipmmu_init() function, regardless
>> if there is any IPMMU hardware present or not. This screws up systems
>> that just want to build a generic kernel that runs on multiple platforms
>> and use a different IOMMU implementation.
>>
>> Move the bus_set_iommu() call at the end of the ipmmu_probe() function
>> when we know that hardware is present. With current IOMMU framework it
>> should be safe (at least for OF case).
>
> If I recall correctly the issue is that the IPMMU might be probed after bus
> master devices when using the legacy IOMMU DT integration, which the ipmmu-
> vmsa driver does on ARM32. Calling bus_set_iommu() from the probe function
> will then result in some devices losing IOMMU support.
Yes, that used to be the problem, but since IPMMU uses the generic
bindings and iommu_device_register(), it should now benefit from client
probe-deferral even when it doesn't support of_xlate().
Robin.
>> Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
>> platform_driver_register() and platform_driver_unregister(), replace
>> them with the module_platform_driver() macro call.
>>
>> Signed-off-by: Liviu Dudau <[email protected]>
>> Cc: Laurent Pinchart <[email protected]>
>> ---
>> drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
>> 1 file changed, 5 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
>> index 2a38aa15be17d..c60569c74678d 100644
>> --- a/drivers/iommu/ipmmu-vmsa.c
>> +++ b/drivers/iommu/ipmmu-vmsa.c
>> @@ -1055,10 +1055,11 @@ static int ipmmu_probe(struct platform_device *pdev)
>> ipmmu_device_reset(mmu);
>>
>> /*
>> - * We can't create the ARM mapping here as it requires the bus to have
>> - * an IOMMU, which only happens when bus_set_iommu() is called in
>> - * ipmmu_init() after the probe function returns.
>> + * Now that we have validated the presence of the hardware, set
>> + * the bus IOMMU ops to enable future domain and device setup.
>> */
>> + if (!iommu_present(&platform_bus_type))
>> + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
>>
>> spin_lock(&ipmmu_devices_lock);
>> list_add(&mmu->list, &ipmmu_devices);
>
> What branch is this patch based on ? ipmmu_devices_lock isn't in mainline.
>
>> @@ -1100,27 +1101,7 @@ static struct platform_driver ipmmu_driver = {
>> .remove = ipmmu_remove,
>> };
>>
>> -static int __init ipmmu_init(void)
>> -{
>> - int ret;
>> -
>> - ret = platform_driver_register(&ipmmu_driver);
>> - if (ret < 0)
>> - return ret;
>> -
>> - if (!iommu_present(&platform_bus_type))
>> - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
>> -
>> - return 0;
>> -}
>> -
>> -static void __exit ipmmu_exit(void)
>> -{
>> - return platform_driver_unregister(&ipmmu_driver);
>> -}
>> -
>> -subsys_initcall(ipmmu_init);
>> -module_exit(ipmmu_exit);
>> +module_platform_driver(ipmmu_driver);
>>
>> MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
>> MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
>
If the IPMMU driver is compiled in the kernel it will replace the
platform bus IOMMU ops on running the ipmmu_init() function, regardless
if there is any IPMMU hardware present or not. This screws up systems
that just want to build a generic kernel that runs on multiple platforms
and use a different IOMMU implementation.
Move the bus_set_iommu() call at the end of the ipmmu_probe() function
when we know that hardware is present. With current IOMMU framework it
should be safe (at least for OF case).
Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
platform_driver_register() and platform_driver_unregister(), replace
them with the module_platform_driver() macro call.
Signed-off-by: Liviu Dudau <[email protected]>
Cc: Laurent Pinchart <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 195d6e93ac718..31912997bffdf 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
return ret;
/*
- * We can't create the ARM mapping here as it requires the bus to have
- * an IOMMU, which only happens when bus_set_iommu() is called in
- * ipmmu_init() after the probe function returns.
+ * Now that we have validated the presence of the hardware, set
+ * the bus IOMMU ops to enable future domain and device setup.
*/
+ if (!iommu_present(&platform_bus_type))
+ bus_set_iommu(&platform_bus_type, &ipmmu_ops);
platform_set_drvdata(pdev, mmu);
@@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
.remove = ipmmu_remove,
};
-static int __init ipmmu_init(void)
-{
- int ret;
-
- ret = platform_driver_register(&ipmmu_driver);
- if (ret < 0)
- return ret;
-
- if (!iommu_present(&platform_bus_type))
- bus_set_iommu(&platform_bus_type, &ipmmu_ops);
-
- return 0;
-}
-
-static void __exit ipmmu_exit(void)
-{
- return platform_driver_unregister(&ipmmu_driver);
-}
-
-subsys_initcall(ipmmu_init);
-module_exit(ipmmu_exit);
+module_platform_driver(ipmmu_driver);
MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
--
2.14.1
On Tue, Nov 21, 2017 at 10:21:32AM -0700, Alex Williamson wrote:
> On Tue, 21 Nov 2017 12:08:01 +0000
> Liviu Dudau <[email protected]> wrote:
>
> > On Mon, Nov 20, 2017 at 03:01:44PM -0700, Alex Williamson wrote:
> > > On Mon, 20 Nov 2017 14:25:14 +0000
> > > Liviu Dudau <[email protected]> wrote:
> > >
> > > > On Fri, Oct 13, 2017 at 04:48:45PM +0100, Robin Murphy wrote:
> > > > > Hi Joerg,
> > > >
> > > > Hi,
> > > >
> > > > >
> > > > > On 20/09/17 15:13, Liviu Dudau wrote:
> > > > > > If the IPMMU driver is compiled in the kernel it will replace the
> > > > > > platform bus IOMMU ops on running the ipmmu_init() function, regardless
> > > > > > if there is any IPMMU hardware present or not. This screws up systems
> > > > > > that just want to build a generic kernel that runs on multiple platforms
> > > > > > and use a different IOMMU implementation.
> > > > > >
> > > > > > Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> > > > > > when we know that hardware is present. With current IOMMU framework it
> > > > > > should be safe (at least for OF case).
> > > > > >
> > > > > > Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> > > > > > platform_driver_register() and platform_driver_unregister(), replace
> > > > > > them with the module_platform_driver() macro call.
> > > > >
> > > > > Are you OK with taking this patch as a fix for 4.14, or would you rather
> > > > > have something that can safely backport past 4.12 without implicit
> > > > > dependencies? This is a config/link-order dependent thing that's been
> > > > > lurking since the beginning, but only coming to light now that other
> > > > > drivers are changing their behaviour, so I don't think there's really a
> > > > > single Fixes: commit that can be singled out.
> > > >
> > > > Can someone update me on the fate of this patch? Can someone queue it
> > > > for the next release?
> > >
> > > Sorry, this is another patch that wasn't on my radar while Joerg is out
> > > on paternity leave. I didn't follow the replies to Laurent's question
> > > about ordering and perhaps this plays in to Robin asking about fixes
> > > for specific kernel versions. It seems there are some changes
> > > elsewhere that somehow defer the ordering problem or don't matter on an
> > > Arm Juno board (whatever that is). Can someone explain?
> >
> > Hi Alex,
> >
> > Thanks for replying!
> >
> > Laurent was positing that doing the bus_set_iommu() call in the probe
> > function will break setups
>
> Yes, I understood that. Replying that it was on an Arm64 Juno board
> did not in any way clarify to me how that concern is addressed. Thanks,
Ah, sorry about that! The Juno board is only an example of where the
problem can occur, but I think any arm64 board (because arm64 defines
CONFIG_IOMMU_DMA) that runs a kernel compiled with multiple IOMMU
drivers and includes the IPMMU without using it will suffer.
Best regards,
Liviu
>
> Alex
>
> > > If there's a
> > > desire for a stable tag for this, it seems like we need to know
> > > explicitly the range where it's safe to apply. Also, the patch needs
> > > to be updated and re-evaluated in the presence of:
> > >
> > > cda52fcd999f iommu/ipmmu-vmsa: Make use of IOMMU_OF_DECLARE()
> > >
> > > Thanks,
> > > Alex
> > >
> > > > > > Signed-off-by: Liviu Dudau <[email protected]>
> > > > > > Cc: Laurent Pinchart <[email protected]>
> > > > > > ---
> > > > > > drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> > > > > > 1 file changed, 5 insertions(+), 24 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > > > > > index 195d6e93ac718..31912997bffdf 100644
> > > > > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > > > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > > > > @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> > > > > > return ret;
> > > > > >
> > > > > > /*
> > > > > > - * We can't create the ARM mapping here as it requires the bus to have
> > > > > > - * an IOMMU, which only happens when bus_set_iommu() is called in
> > > > > > - * ipmmu_init() after the probe function returns.
> > > > > > + * Now that we have validated the presence of the hardware, set
> > > > > > + * the bus IOMMU ops to enable future domain and device setup.
> > > > > > */
> > > > > > + if (!iommu_present(&platform_bus_type))
> > > > > > + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > > > >
> > > > > > platform_set_drvdata(pdev, mmu);
> > > > > >
> > > > > > @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
> > > > > > .remove = ipmmu_remove,
> > > > > > };
> > > > > >
> > > > > > -static int __init ipmmu_init(void)
> > > > > > -{
> > > > > > - int ret;
> > > > > > -
> > > > > > - ret = platform_driver_register(&ipmmu_driver);
> > > > > > - if (ret < 0)
> > > > > > - return ret;
> > > > > > -
> > > > > > - if (!iommu_present(&platform_bus_type))
> > > > > > - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > > > > -
> > > > > > - return 0;
> > > > > > -}
> > > > > > -
> > > > > > -static void __exit ipmmu_exit(void)
> > > > > > -{
> > > > > > - return platform_driver_unregister(&ipmmu_driver);
> > > > > > -}
> > > > > > -
> > > > > > -subsys_initcall(ipmmu_init);
> > > > > > -module_exit(ipmmu_exit);
> > > > > > +module_platform_driver(ipmmu_driver);
> > > > > >
> > > > > > MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> > > > > > MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
From 1584697123884082076@xxx Tue Nov 21 17:22:26 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Tue, 21 Nov 2017 12:08:01 +0000
Liviu Dudau <[email protected]> wrote:
> On Mon, Nov 20, 2017 at 03:01:44PM -0700, Alex Williamson wrote:
> > On Mon, 20 Nov 2017 14:25:14 +0000
> > Liviu Dudau <[email protected]> wrote:
> >
> > > On Fri, Oct 13, 2017 at 04:48:45PM +0100, Robin Murphy wrote:
> > > > Hi Joerg,
> > >
> > > Hi,
> > >
> > > >
> > > > On 20/09/17 15:13, Liviu Dudau wrote:
> > > > > If the IPMMU driver is compiled in the kernel it will replace the
> > > > > platform bus IOMMU ops on running the ipmmu_init() function, regardless
> > > > > if there is any IPMMU hardware present or not. This screws up systems
> > > > > that just want to build a generic kernel that runs on multiple platforms
> > > > > and use a different IOMMU implementation.
> > > > >
> > > > > Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> > > > > when we know that hardware is present. With current IOMMU framework it
> > > > > should be safe (at least for OF case).
> > > > >
> > > > > Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> > > > > platform_driver_register() and platform_driver_unregister(), replace
> > > > > them with the module_platform_driver() macro call.
> > > >
> > > > Are you OK with taking this patch as a fix for 4.14, or would you rather
> > > > have something that can safely backport past 4.12 without implicit
> > > > dependencies? This is a config/link-order dependent thing that's been
> > > > lurking since the beginning, but only coming to light now that other
> > > > drivers are changing their behaviour, so I don't think there's really a
> > > > single Fixes: commit that can be singled out.
> > >
> > > Can someone update me on the fate of this patch? Can someone queue it
> > > for the next release?
> >
> > Sorry, this is another patch that wasn't on my radar while Joerg is out
> > on paternity leave. I didn't follow the replies to Laurent's question
> > about ordering and perhaps this plays in to Robin asking about fixes
> > for specific kernel versions. It seems there are some changes
> > elsewhere that somehow defer the ordering problem or don't matter on an
> > Arm Juno board (whatever that is). Can someone explain?
>
> Hi Alex,
>
> Thanks for replying!
>
> Laurent was positing that doing the bus_set_iommu() call in the probe
> function will break setups
Yes, I understood that. Replying that it was on an Arm64 Juno board
did not in any way clarify to me how that concern is addressed. Thanks,
Alex
> > If there's a
> > desire for a stable tag for this, it seems like we need to know
> > explicitly the range where it's safe to apply. Also, the patch needs
> > to be updated and re-evaluated in the presence of:
> >
> > cda52fcd999f iommu/ipmmu-vmsa: Make use of IOMMU_OF_DECLARE()
> >
> > Thanks,
> > Alex
> >
> > > > > Signed-off-by: Liviu Dudau <[email protected]>
> > > > > Cc: Laurent Pinchart <[email protected]>
> > > > > ---
> > > > > drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> > > > > 1 file changed, 5 insertions(+), 24 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > > > > index 195d6e93ac718..31912997bffdf 100644
> > > > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > > > @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> > > > > return ret;
> > > > >
> > > > > /*
> > > > > - * We can't create the ARM mapping here as it requires the bus to have
> > > > > - * an IOMMU, which only happens when bus_set_iommu() is called in
> > > > > - * ipmmu_init() after the probe function returns.
> > > > > + * Now that we have validated the presence of the hardware, set
> > > > > + * the bus IOMMU ops to enable future domain and device setup.
> > > > > */
> > > > > + if (!iommu_present(&platform_bus_type))
> > > > > + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > > >
> > > > > platform_set_drvdata(pdev, mmu);
> > > > >
> > > > > @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
> > > > > .remove = ipmmu_remove,
> > > > > };
> > > > >
> > > > > -static int __init ipmmu_init(void)
> > > > > -{
> > > > > - int ret;
> > > > > -
> > > > > - ret = platform_driver_register(&ipmmu_driver);
> > > > > - if (ret < 0)
> > > > > - return ret;
> > > > > -
> > > > > - if (!iommu_present(&platform_bus_type))
> > > > > - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > > > -
> > > > > - return 0;
> > > > > -}
> > > > > -
> > > > > -static void __exit ipmmu_exit(void)
> > > > > -{
> > > > > - return platform_driver_unregister(&ipmmu_driver);
> > > > > -}
> > > > > -
> > > > > -subsys_initcall(ipmmu_init);
> > > > > -module_exit(ipmmu_exit);
> > > > > +module_platform_driver(ipmmu_driver);
> > > > >
> > > > > MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> > > > > MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
> > > > >
> > > >
> > >
> >
>
From 1584680645441820641@xxx Tue Nov 21 13:00:31 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
Hi Alex,
On 20/11/17 22:01, Alex Williamson wrote:
> On Mon, 20 Nov 2017 14:25:14 +0000
> Liviu Dudau <[email protected]> wrote:
>
>> On Fri, Oct 13, 2017 at 04:48:45PM +0100, Robin Murphy wrote:
>>> Hi Joerg,
>>
>> Hi,
>>
>>>
>>> On 20/09/17 15:13, Liviu Dudau wrote:
>>>> If the IPMMU driver is compiled in the kernel it will replace the
>>>> platform bus IOMMU ops on running the ipmmu_init() function, regardless
>>>> if there is any IPMMU hardware present or not. This screws up systems
>>>> that just want to build a generic kernel that runs on multiple platforms
>>>> and use a different IOMMU implementation.
>>>>
>>>> Move the bus_set_iommu() call at the end of the ipmmu_probe() function
>>>> when we know that hardware is present. With current IOMMU framework it
>>>> should be safe (at least for OF case).
>>>>
>>>> Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
>>>> platform_driver_register() and platform_driver_unregister(), replace
>>>> them with the module_platform_driver() macro call.
>>>
>>> Are you OK with taking this patch as a fix for 4.14, or would you rather
>>> have something that can safely backport past 4.12 without implicit
>>> dependencies? This is a config/link-order dependent thing that's been
>>> lurking since the beginning, but only coming to light now that other
>>> drivers are changing their behaviour, so I don't think there's really a
>>> single Fixes: commit that can be singled out.
>>
>> Can someone update me on the fate of this patch? Can someone queue it
>> for the next release?
>
> Sorry, this is another patch that wasn't on my radar while Joerg is out
> on paternity leave. I didn't follow the replies to Laurent's question
> about ordering and perhaps this plays in to Robin asking about fixes
> for specific kernel versions. It seems there are some changes
> elsewhere that somehow defer the ordering problem or don't matter on an
> Arm Juno board (whatever that is). Can someone explain?
To clarify, the ipmmu-vmsa driver is not enabled in the arm64 defconfig,
but turning it on causes crashes on non-Renesas platforms with different
IOMMUs (e.g. the Juno dev board[1] which has ARM SMMUs), because it
still relies on initcalls to initialise the IOMMU before its masters,
whereas other drivers like arm-smmu have now transitioned to using the
probe-deferral mechanism. Back when everything ran off initcalls,
arm-smmu would get there first (thanks to link order) and we never saw a
problem, but since the ipmmu-vmsa initcall doesn't check whether any
IPMMU device is actually present in the system before grabbing the bus
ops, it now breaks other drivers' probe-time setup.
> If there's a
> desire for a stable tag for this, it seems like we need to know
> explicitly the range where it's safe to apply. Also, the patch needs
> to be updated and re-evaluated in the presence of:
>
> cda52fcd999f iommu/ipmmu-vmsa: Make use of IOMMU_OF_DECLARE()
Actually, I overlooked it the first time, but Liviu's patch did in fact
need an IOMMU_OF_DECLARE() in order to correctly trigger probe-deferral
and avoid Laurent's concerns. However, cda52fcd999f does now mostly
supersede it (I've checked that the arm64 case is OK; 32-bit
multiplatform kernels might possibly still be broken, but how much
anyone's relying on IOMMU support in those I don't know).
Robin.
[1]:https://developer.arm.com/products/system-design/development-boards/juno-development-board
>
> Thanks,
> Alex
>
>>>> Signed-off-by: Liviu Dudau <[email protected]>
>>>> Cc: Laurent Pinchart <[email protected]>
>>>> ---
>>>> drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
>>>> 1 file changed, 5 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
>>>> index 195d6e93ac718..31912997bffdf 100644
>>>> --- a/drivers/iommu/ipmmu-vmsa.c
>>>> +++ b/drivers/iommu/ipmmu-vmsa.c
>>>> @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
>>>> return ret;
>>>>
>>>> /*
>>>> - * We can't create the ARM mapping here as it requires the bus to have
>>>> - * an IOMMU, which only happens when bus_set_iommu() is called in
>>>> - * ipmmu_init() after the probe function returns.
>>>> + * Now that we have validated the presence of the hardware, set
>>>> + * the bus IOMMU ops to enable future domain and device setup.
>>>> */
>>>> + if (!iommu_present(&platform_bus_type))
>>>> + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
>>>>
>>>> platform_set_drvdata(pdev, mmu);
>>>>
>>>> @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
>>>> .remove = ipmmu_remove,
>>>> };
>>>>
>>>> -static int __init ipmmu_init(void)
>>>> -{
>>>> - int ret;
>>>> -
>>>> - ret = platform_driver_register(&ipmmu_driver);
>>>> - if (ret < 0)
>>>> - return ret;
>>>> -
>>>> - if (!iommu_present(&platform_bus_type))
>>>> - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
>>>> -
>>>> - return 0;
>>>> -}
>>>> -
>>>> -static void __exit ipmmu_exit(void)
>>>> -{
>>>> - return platform_driver_unregister(&ipmmu_driver);
>>>> -}
>>>> -
>>>> -subsys_initcall(ipmmu_init);
>>>> -module_exit(ipmmu_exit);
>>>> +module_platform_driver(ipmmu_driver);
>>>>
>>>> MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
>>>> MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
>>>>
>>>
>>
>
From 1584677400610513612@xxx Tue Nov 21 12:08:56 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Mon, Nov 20, 2017 at 03:01:44PM -0700, Alex Williamson wrote:
> On Mon, 20 Nov 2017 14:25:14 +0000
> Liviu Dudau <[email protected]> wrote:
>
> > On Fri, Oct 13, 2017 at 04:48:45PM +0100, Robin Murphy wrote:
> > > Hi Joerg,
> >
> > Hi,
> >
> > >
> > > On 20/09/17 15:13, Liviu Dudau wrote:
> > > > If the IPMMU driver is compiled in the kernel it will replace the
> > > > platform bus IOMMU ops on running the ipmmu_init() function, regardless
> > > > if there is any IPMMU hardware present or not. This screws up systems
> > > > that just want to build a generic kernel that runs on multiple platforms
> > > > and use a different IOMMU implementation.
> > > >
> > > > Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> > > > when we know that hardware is present. With current IOMMU framework it
> > > > should be safe (at least for OF case).
> > > >
> > > > Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> > > > platform_driver_register() and platform_driver_unregister(), replace
> > > > them with the module_platform_driver() macro call.
> > >
> > > Are you OK with taking this patch as a fix for 4.14, or would you rather
> > > have something that can safely backport past 4.12 without implicit
> > > dependencies? This is a config/link-order dependent thing that's been
> > > lurking since the beginning, but only coming to light now that other
> > > drivers are changing their behaviour, so I don't think there's really a
> > > single Fixes: commit that can be singled out.
> >
> > Can someone update me on the fate of this patch? Can someone queue it
> > for the next release?
>
> Sorry, this is another patch that wasn't on my radar while Joerg is out
> on paternity leave. I didn't follow the replies to Laurent's question
> about ordering and perhaps this plays in to Robin asking about fixes
> for specific kernel versions. It seems there are some changes
> elsewhere that somehow defer the ordering problem or don't matter on an
> Arm Juno board (whatever that is). Can someone explain?
Hi Alex,
Thanks for replying!
Laurent was positing that doing the bus_set_iommu() call in the probe
function will break setups
> If there's a
> desire for a stable tag for this, it seems like we need to know
> explicitly the range where it's safe to apply. Also, the patch needs
> to be updated and re-evaluated in the presence of:
>
> cda52fcd999f iommu/ipmmu-vmsa: Make use of IOMMU_OF_DECLARE()
>
> Thanks,
> Alex
>
> > > > Signed-off-by: Liviu Dudau <[email protected]>
> > > > Cc: Laurent Pinchart <[email protected]>
> > > > ---
> > > > drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> > > > 1 file changed, 5 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > > > index 195d6e93ac718..31912997bffdf 100644
> > > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > > @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> > > > return ret;
> > > >
> > > > /*
> > > > - * We can't create the ARM mapping here as it requires the bus to have
> > > > - * an IOMMU, which only happens when bus_set_iommu() is called in
> > > > - * ipmmu_init() after the probe function returns.
> > > > + * Now that we have validated the presence of the hardware, set
> > > > + * the bus IOMMU ops to enable future domain and device setup.
> > > > */
> > > > + if (!iommu_present(&platform_bus_type))
> > > > + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > >
> > > > platform_set_drvdata(pdev, mmu);
> > > >
> > > > @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
> > > > .remove = ipmmu_remove,
> > > > };
> > > >
> > > > -static int __init ipmmu_init(void)
> > > > -{
> > > > - int ret;
> > > > -
> > > > - ret = platform_driver_register(&ipmmu_driver);
> > > > - if (ret < 0)
> > > > - return ret;
> > > > -
> > > > - if (!iommu_present(&platform_bus_type))
> > > > - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
> > > > -static void __exit ipmmu_exit(void)
> > > > -{
> > > > - return platform_driver_unregister(&ipmmu_driver);
> > > > -}
> > > > -
> > > > -subsys_initcall(ipmmu_init);
> > > > -module_exit(ipmmu_exit);
> > > > +module_platform_driver(ipmmu_driver);
> > > >
> > > > MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> > > > MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
> > > >
> > >
> >
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
From 1584624241750037842@xxx Mon Nov 20 22:04:00 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Mon, 20 Nov 2017 14:25:14 +0000
Liviu Dudau <[email protected]> wrote:
> On Fri, Oct 13, 2017 at 04:48:45PM +0100, Robin Murphy wrote:
> > Hi Joerg,
>
> Hi,
>
> >
> > On 20/09/17 15:13, Liviu Dudau wrote:
> > > If the IPMMU driver is compiled in the kernel it will replace the
> > > platform bus IOMMU ops on running the ipmmu_init() function, regardless
> > > if there is any IPMMU hardware present or not. This screws up systems
> > > that just want to build a generic kernel that runs on multiple platforms
> > > and use a different IOMMU implementation.
> > >
> > > Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> > > when we know that hardware is present. With current IOMMU framework it
> > > should be safe (at least for OF case).
> > >
> > > Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> > > platform_driver_register() and platform_driver_unregister(), replace
> > > them with the module_platform_driver() macro call.
> >
> > Are you OK with taking this patch as a fix for 4.14, or would you rather
> > have something that can safely backport past 4.12 without implicit
> > dependencies? This is a config/link-order dependent thing that's been
> > lurking since the beginning, but only coming to light now that other
> > drivers are changing their behaviour, so I don't think there's really a
> > single Fixes: commit that can be singled out.
>
> Can someone update me on the fate of this patch? Can someone queue it
> for the next release?
Sorry, this is another patch that wasn't on my radar while Joerg is out
on paternity leave. I didn't follow the replies to Laurent's question
about ordering and perhaps this plays in to Robin asking about fixes
for specific kernel versions. It seems there are some changes
elsewhere that somehow defer the ordering problem or don't matter on an
Arm Juno board (whatever that is). Can someone explain? If there's a
desire for a stable tag for this, it seems like we need to know
explicitly the range where it's safe to apply. Also, the patch needs
to be updated and re-evaluated in the presence of:
cda52fcd999f iommu/ipmmu-vmsa: Make use of IOMMU_OF_DECLARE()
Thanks,
Alex
> > > Signed-off-by: Liviu Dudau <[email protected]>
> > > Cc: Laurent Pinchart <[email protected]>
> > > ---
> > > drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> > > 1 file changed, 5 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > > index 195d6e93ac718..31912997bffdf 100644
> > > --- a/drivers/iommu/ipmmu-vmsa.c
> > > +++ b/drivers/iommu/ipmmu-vmsa.c
> > > @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> > > return ret;
> > >
> > > /*
> > > - * We can't create the ARM mapping here as it requires the bus to have
> > > - * an IOMMU, which only happens when bus_set_iommu() is called in
> > > - * ipmmu_init() after the probe function returns.
> > > + * Now that we have validated the presence of the hardware, set
> > > + * the bus IOMMU ops to enable future domain and device setup.
> > > */
> > > + if (!iommu_present(&platform_bus_type))
> > > + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > >
> > > platform_set_drvdata(pdev, mmu);
> > >
> > > @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
> > > .remove = ipmmu_remove,
> > > };
> > >
> > > -static int __init ipmmu_init(void)
> > > -{
> > > - int ret;
> > > -
> > > - ret = platform_driver_register(&ipmmu_driver);
> > > - if (ret < 0)
> > > - return ret;
> > > -
> > > - if (!iommu_present(&platform_bus_type))
> > > - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > > -
> > > - return 0;
> > > -}
> > > -
> > > -static void __exit ipmmu_exit(void)
> > > -{
> > > - return platform_driver_unregister(&ipmmu_driver);
> > > -}
> > > -
> > > -subsys_initcall(ipmmu_init);
> > > -module_exit(ipmmu_exit);
> > > +module_platform_driver(ipmmu_driver);
> > >
> > > MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> > > MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
> > >
> >
>
From 1584595452175373974@xxx Mon Nov 20 14:26:24 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread
On Fri, Oct 13, 2017 at 04:48:45PM +0100, Robin Murphy wrote:
> Hi Joerg,
Hi,
>
> On 20/09/17 15:13, Liviu Dudau wrote:
> > If the IPMMU driver is compiled in the kernel it will replace the
> > platform bus IOMMU ops on running the ipmmu_init() function, regardless
> > if there is any IPMMU hardware present or not. This screws up systems
> > that just want to build a generic kernel that runs on multiple platforms
> > and use a different IOMMU implementation.
> >
> > Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> > when we know that hardware is present. With current IOMMU framework it
> > should be safe (at least for OF case).
> >
> > Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> > platform_driver_register() and platform_driver_unregister(), replace
> > them with the module_platform_driver() macro call.
>
> Are you OK with taking this patch as a fix for 4.14, or would you rather
> have something that can safely backport past 4.12 without implicit
> dependencies? This is a config/link-order dependent thing that's been
> lurking since the beginning, but only coming to light now that other
> drivers are changing their behaviour, so I don't think there's really a
> single Fixes: commit that can be singled out.
Can someone update me on the fate of this patch? Can someone queue it
for the next release?
Best regards,
Liviu
>
> Robin.
>
> > Signed-off-by: Liviu Dudau <[email protected]>
> > Cc: Laurent Pinchart <[email protected]>
> > ---
> > drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> > 1 file changed, 5 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > index 195d6e93ac718..31912997bffdf 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> > return ret;
> >
> > /*
> > - * We can't create the ARM mapping here as it requires the bus to have
> > - * an IOMMU, which only happens when bus_set_iommu() is called in
> > - * ipmmu_init() after the probe function returns.
> > + * Now that we have validated the presence of the hardware, set
> > + * the bus IOMMU ops to enable future domain and device setup.
> > */
> > + if (!iommu_present(&platform_bus_type))
> > + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> >
> > platform_set_drvdata(pdev, mmu);
> >
> > @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
> > .remove = ipmmu_remove,
> > };
> >
> > -static int __init ipmmu_init(void)
> > -{
> > - int ret;
> > -
> > - ret = platform_driver_register(&ipmmu_driver);
> > - if (ret < 0)
> > - return ret;
> > -
> > - if (!iommu_present(&platform_bus_type))
> > - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> > -
> > - return 0;
> > -}
> > -
> > -static void __exit ipmmu_exit(void)
> > -{
> > - return platform_driver_unregister(&ipmmu_driver);
> > -}
> > -
> > -subsys_initcall(ipmmu_init);
> > -module_exit(ipmmu_exit);
> > +module_platform_driver(ipmmu_driver);
> >
> > MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> > MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
> >
>
--
====================
| I would like to |
| fix the world, |
| but they're not |
| giving me the |
\ source code! /
---------------
¯\_(ツ)_/¯
From 1581157993679606603@xxx Fri Oct 13 15:49:28 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums
Hi Joerg,
On 20/09/17 15:13, Liviu Dudau wrote:
> If the IPMMU driver is compiled in the kernel it will replace the
> platform bus IOMMU ops on running the ipmmu_init() function, regardless
> if there is any IPMMU hardware present or not. This screws up systems
> that just want to build a generic kernel that runs on multiple platforms
> and use a different IOMMU implementation.
>
> Move the bus_set_iommu() call at the end of the ipmmu_probe() function
> when we know that hardware is present. With current IOMMU framework it
> should be safe (at least for OF case).
>
> Now that the ipmmu_init() and ipmmu_exit() functions are simple calls to
> platform_driver_register() and platform_driver_unregister(), replace
> them with the module_platform_driver() macro call.
Are you OK with taking this patch as a fix for 4.14, or would you rather
have something that can safely backport past 4.12 without implicit
dependencies? This is a config/link-order dependent thing that's been
lurking since the beginning, but only coming to light now that other
drivers are changing their behaviour, so I don't think there's really a
single Fixes: commit that can be singled out.
Robin.
> Signed-off-by: Liviu Dudau <[email protected]>
> Cc: Laurent Pinchart <[email protected]>
> ---
> drivers/iommu/ipmmu-vmsa.c | 29 +++++------------------------
> 1 file changed, 5 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> index 195d6e93ac718..31912997bffdf 100644
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -966,10 +966,11 @@ static int ipmmu_probe(struct platform_device *pdev)
> return ret;
>
> /*
> - * We can't create the ARM mapping here as it requires the bus to have
> - * an IOMMU, which only happens when bus_set_iommu() is called in
> - * ipmmu_init() after the probe function returns.
> + * Now that we have validated the presence of the hardware, set
> + * the bus IOMMU ops to enable future domain and device setup.
> */
> + if (!iommu_present(&platform_bus_type))
> + bus_set_iommu(&platform_bus_type, &ipmmu_ops);
>
> platform_set_drvdata(pdev, mmu);
>
> @@ -1006,27 +1007,7 @@ static struct platform_driver ipmmu_driver = {
> .remove = ipmmu_remove,
> };
>
> -static int __init ipmmu_init(void)
> -{
> - int ret;
> -
> - ret = platform_driver_register(&ipmmu_driver);
> - if (ret < 0)
> - return ret;
> -
> - if (!iommu_present(&platform_bus_type))
> - bus_set_iommu(&platform_bus_type, &ipmmu_ops);
> -
> - return 0;
> -}
> -
> -static void __exit ipmmu_exit(void)
> -{
> - return platform_driver_unregister(&ipmmu_driver);
> -}
> -
> -subsys_initcall(ipmmu_init);
> -module_exit(ipmmu_exit);
> +module_platform_driver(ipmmu_driver);
>
> MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
> MODULE_AUTHOR("Laurent Pinchart <[email protected]>");
>
From 1579068298221211124@xxx Wed Sep 20 14:14:39 +0000 2017
X-GM-THRID: 1578902883289057090
X-Gmail-Labels: Inbox,Category Forums
The IPMMU driver still uses initcalls to do its initialisation, while
other IOMMU drivers have moved to probe deferal mechanism. Update
the IPMMU driver so that it can use modern driver probing which allows
for it to be compiled together with other IOMMU drivers and not
trying at boot time to replace bus masters for platforms that don't
use IPMMU.
Signed-off-by: Liviu Dudau <[email protected]>
Cc: Laurent Pinchart <[email protected]>
Cc: Magnus Damm <[email protected]>
Cc: Alex Williamson <[email protected]>
---
drivers/iommu/ipmmu-vmsa.c | 47 +++-------------------------------------------
1 file changed, 3 insertions(+), 44 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 8dce3a9de9d86..6eea75ca3c92e 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1039,18 +1039,10 @@ static int ipmmu_probe(struct platform_device *pdev)
if (ret)
return ret;
-#if defined(CONFIG_IOMMU_DMA)
if (!iommu_present(&platform_bus_type))
bus_set_iommu(&platform_bus_type, &ipmmu_ops);
-#endif
}
- /*
- * We can't create the ARM mapping here as it requires the bus to have
- * an IOMMU, which only happens when bus_set_iommu() is called in
- * ipmmu_init() after the probe function returns.
- */
-
platform_set_drvdata(pdev, mmu);
return 0;
@@ -1079,46 +1071,13 @@ static struct platform_driver ipmmu_driver = {
.remove = ipmmu_remove,
};
-static int __init ipmmu_init(void)
-{
- static bool setup_done;
- int ret;
-
- if (setup_done)
- return 0;
-
- ret = platform_driver_register(&ipmmu_driver);
- if (ret < 0)
- return ret;
-
-#if defined(CONFIG_ARM) && !defined(CONFIG_IOMMU_DMA)
- if (!iommu_present(&platform_bus_type))
- bus_set_iommu(&platform_bus_type, &ipmmu_ops);
-#endif
-
- setup_done = true;
- return 0;
-}
-
-static void __exit ipmmu_exit(void)
-{
- return platform_driver_unregister(&ipmmu_driver);
-}
-
-subsys_initcall(ipmmu_init);
-module_exit(ipmmu_exit);
+module_platform_driver(ipmmu_driver);
#ifdef CONFIG_IOMMU_DMA
-static int __init ipmmu_vmsa_iommu_of_setup(struct device_node *np)
-{
- ipmmu_init();
- return 0;
-}
-IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa",
- ipmmu_vmsa_iommu_of_setup);
+IOMMU_OF_DECLARE(ipmmu_vmsa_iommu_of, "renesas,ipmmu-vmsa", NULL);
IOMMU_OF_DECLARE(ipmmu_r8a7795_iommu_of, "renesas,ipmmu-r8a7795",
- ipmmu_vmsa_iommu_of_setup);
+ NULL);
#endif
MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
--
2.15.0
From 1600359949922631784@xxx Sun May 13 14:36:21 +0000 2018
X-GM-THRID: 1585774005312271128
X-Gmail-Labels: Inbox,Category Forums,Downloaded_2018-05