* [PATCH 0/3] drivers: rpmsg: make rpmsg bus DMA capable @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon, robin.murphy, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> On Qualcomm SoCs, DSP exposes many functionalities like audio, as these drivers are child of rpmsg bus. Any dma allocations or iommu operations on these devices would fail as the rpmsg bus is not marked as dma capable bus. This patchset makes the rpmsg bus dma capable. This patchset is tested on DB410c and DB820c boards. Thanks, srini Srinivas Kandagatla (3): rpmsg: core: export rpmsg bus type rpmsg: core: make rpmsg bus DMA capable iommu: armsmmu: set iommu ops for rpmsg bus drivers/iommu/arm-smmu.c | 5 +++++ drivers/rpmsg/rpmsg_core.c | 4 +++- include/linux/rpmsg.h | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) -- 2.15.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 0/3] drivers: rpmsg: make rpmsg bus DMA capable @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla at linaro.org @ 2018-03-02 14:55 UTC (permalink / raw) To: linux-arm-kernel From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> On Qualcomm SoCs, DSP exposes many functionalities like audio, as these drivers are child of rpmsg bus. Any dma allocations or iommu operations on these devices would fail as the rpmsg bus is not marked as dma capable bus. This patchset makes the rpmsg bus dma capable. This patchset is tested on DB410c and DB820c boards. Thanks, srini Srinivas Kandagatla (3): rpmsg: core: export rpmsg bus type rpmsg: core: make rpmsg bus DMA capable iommu: armsmmu: set iommu ops for rpmsg bus drivers/iommu/arm-smmu.c | 5 +++++ drivers/rpmsg/rpmsg_core.c | 4 +++- include/linux/rpmsg.h | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) -- 2.15.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 0/3] drivers: rpmsg: make rpmsg bus DMA capable @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Srinivas Kandagatla, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> On Qualcomm SoCs, DSP exposes many functionalities like audio, as these drivers are child of rpmsg bus. Any dma allocations or iommu operations on these devices would fail as the rpmsg bus is not marked as dma capable bus. This patchset makes the rpmsg bus dma capable. This patchset is tested on DB410c and DB820c boards. Thanks, srini Srinivas Kandagatla (3): rpmsg: core: export rpmsg bus type rpmsg: core: make rpmsg bus DMA capable iommu: armsmmu: set iommu ops for rpmsg bus drivers/iommu/arm-smmu.c | 5 +++++ drivers/rpmsg/rpmsg_core.c | 4 +++- include/linux/rpmsg.h | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) -- 2.15.1 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/3] rpmsg: core: export rpmsg bus type @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon, robin.murphy, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Export rpmsg bus type so that iommu drivers iommu ops to rpmsg bus type. On Qualcomm SoCs ADSP exposes functions like audio and other which need iommu access, as these drivers are part of rpmsg bus, able to allocate memory from iommus is basic requirement. So expose this bus so that iommu drivers can add ops to this. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/rpmsg/rpmsg_core.c | 3 ++- include/linux/rpmsg.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index 5a081762afcc..e84c71f8d6ab 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -465,7 +465,7 @@ static int rpmsg_dev_remove(struct device *dev) return err; } -static struct bus_type rpmsg_bus = { +struct bus_type rpmsg_bus = { .name = "rpmsg", .match = rpmsg_dev_match, .dev_groups = rpmsg_dev_groups, @@ -473,6 +473,7 @@ static struct bus_type rpmsg_bus = { .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove, }; +EXPORT_SYMBOL(rpmsg_bus); int rpmsg_register_device(struct rpmsg_device *rpdev) { diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h index ca07366c4c33..869e5946b7df 100644 --- a/include/linux/rpmsg.h +++ b/include/linux/rpmsg.h @@ -49,7 +49,7 @@ struct rpmsg_device; struct rpmsg_endpoint; struct rpmsg_device_ops; struct rpmsg_endpoint_ops; - +extern struct bus_type rpmsg_bus; /** * struct rpmsg_channel_info - channel info representation * @name: name of service -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/3] rpmsg: core: export rpmsg bus type @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla at linaro.org @ 2018-03-02 14:55 UTC (permalink / raw) To: linux-arm-kernel From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Export rpmsg bus type so that iommu drivers iommu ops to rpmsg bus type. On Qualcomm SoCs ADSP exposes functions like audio and other which need iommu access, as these drivers are part of rpmsg bus, able to allocate memory from iommus is basic requirement. So expose this bus so that iommu drivers can add ops to this. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/rpmsg/rpmsg_core.c | 3 ++- include/linux/rpmsg.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index 5a081762afcc..e84c71f8d6ab 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -465,7 +465,7 @@ static int rpmsg_dev_remove(struct device *dev) return err; } -static struct bus_type rpmsg_bus = { +struct bus_type rpmsg_bus = { .name = "rpmsg", .match = rpmsg_dev_match, .dev_groups = rpmsg_dev_groups, @@ -473,6 +473,7 @@ static struct bus_type rpmsg_bus = { .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove, }; +EXPORT_SYMBOL(rpmsg_bus); int rpmsg_register_device(struct rpmsg_device *rpdev) { diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h index ca07366c4c33..869e5946b7df 100644 --- a/include/linux/rpmsg.h +++ b/include/linux/rpmsg.h @@ -49,7 +49,7 @@ struct rpmsg_device; struct rpmsg_endpoint; struct rpmsg_device_ops; struct rpmsg_endpoint_ops; - +extern struct bus_type rpmsg_bus; /** * struct rpmsg_channel_info - channel info representation * @name: name of service -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 1/3] rpmsg: core: export rpmsg bus type @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Srinivas Kandagatla, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Export rpmsg bus type so that iommu drivers iommu ops to rpmsg bus type. On Qualcomm SoCs ADSP exposes functions like audio and other which need iommu access, as these drivers are part of rpmsg bus, able to allocate memory from iommus is basic requirement. So expose this bus so that iommu drivers can add ops to this. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> --- drivers/rpmsg/rpmsg_core.c | 3 ++- include/linux/rpmsg.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index 5a081762afcc..e84c71f8d6ab 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -465,7 +465,7 @@ static int rpmsg_dev_remove(struct device *dev) return err; } -static struct bus_type rpmsg_bus = { +struct bus_type rpmsg_bus = { .name = "rpmsg", .match = rpmsg_dev_match, .dev_groups = rpmsg_dev_groups, @@ -473,6 +473,7 @@ static struct bus_type rpmsg_bus = { .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove, }; +EXPORT_SYMBOL(rpmsg_bus); int rpmsg_register_device(struct rpmsg_device *rpdev) { diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h index ca07366c4c33..869e5946b7df 100644 --- a/include/linux/rpmsg.h +++ b/include/linux/rpmsg.h @@ -49,7 +49,7 @@ struct rpmsg_device; struct rpmsg_endpoint; struct rpmsg_device_ops; struct rpmsg_endpoint_ops; - +extern struct bus_type rpmsg_bus; /** * struct rpmsg_channel_info - channel info representation * @name: name of service -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 1/3] rpmsg: core: export rpmsg bus type @ 2018-03-02 15:25 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 15:25 UTC (permalink / raw) To: srinivas.kandagatla, will.deacon, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Export rpmsg bus type so that iommu drivers iommu ops to > rpmsg bus type. > > On Qualcomm SoCs ADSP exposes functions like audio and > other which need iommu access, as these drivers are part > of rpmsg bus, able to allocate memory from iommus is basic > requirement. So expose this bus so that iommu drivers can > add ops to this. IOMMU drivers are built-in while rpmsg can be built as a module, so that isn't always going to work. Robin. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/rpmsg/rpmsg_core.c | 3 ++- > include/linux/rpmsg.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index 5a081762afcc..e84c71f8d6ab 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -465,7 +465,7 @@ static int rpmsg_dev_remove(struct device *dev) > return err; > } > > -static struct bus_type rpmsg_bus = { > +struct bus_type rpmsg_bus = { > .name = "rpmsg", > .match = rpmsg_dev_match, > .dev_groups = rpmsg_dev_groups, > @@ -473,6 +473,7 @@ static struct bus_type rpmsg_bus = { > .probe = rpmsg_dev_probe, > .remove = rpmsg_dev_remove, > }; > +EXPORT_SYMBOL(rpmsg_bus); > > int rpmsg_register_device(struct rpmsg_device *rpdev) > { > diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h > index ca07366c4c33..869e5946b7df 100644 > --- a/include/linux/rpmsg.h > +++ b/include/linux/rpmsg.h > @@ -49,7 +49,7 @@ struct rpmsg_device; > struct rpmsg_endpoint; > struct rpmsg_device_ops; > struct rpmsg_endpoint_ops; > - > +extern struct bus_type rpmsg_bus; > /** > * struct rpmsg_channel_info - channel info representation > * @name: name of service > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 1/3] rpmsg: core: export rpmsg bus type @ 2018-03-02 15:25 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 15:25 UTC (permalink / raw) To: linux-arm-kernel On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Export rpmsg bus type so that iommu drivers iommu ops to > rpmsg bus type. > > On Qualcomm SoCs ADSP exposes functions like audio and > other which need iommu access, as these drivers are part > of rpmsg bus, able to allocate memory from iommus is basic > requirement. So expose this bus so that iommu drivers can > add ops to this. IOMMU drivers are built-in while rpmsg can be built as a module, so that isn't always going to work. Robin. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/rpmsg/rpmsg_core.c | 3 ++- > include/linux/rpmsg.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index 5a081762afcc..e84c71f8d6ab 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -465,7 +465,7 @@ static int rpmsg_dev_remove(struct device *dev) > return err; > } > > -static struct bus_type rpmsg_bus = { > +struct bus_type rpmsg_bus = { > .name = "rpmsg", > .match = rpmsg_dev_match, > .dev_groups = rpmsg_dev_groups, > @@ -473,6 +473,7 @@ static struct bus_type rpmsg_bus = { > .probe = rpmsg_dev_probe, > .remove = rpmsg_dev_remove, > }; > +EXPORT_SYMBOL(rpmsg_bus); > > int rpmsg_register_device(struct rpmsg_device *rpdev) > { > diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h > index ca07366c4c33..869e5946b7df 100644 > --- a/include/linux/rpmsg.h > +++ b/include/linux/rpmsg.h > @@ -49,7 +49,7 @@ struct rpmsg_device; > struct rpmsg_endpoint; > struct rpmsg_device_ops; > struct rpmsg_endpoint_ops; > - > +extern struct bus_type rpmsg_bus; > /** > * struct rpmsg_channel_info - channel info representation > * @name: name of service > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 1/3] rpmsg: core: export rpmsg bus type @ 2018-03-02 15:25 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 15:25 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On 02/03/18 14:55, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > Export rpmsg bus type so that iommu drivers iommu ops to > rpmsg bus type. > > On Qualcomm SoCs ADSP exposes functions like audio and > other which need iommu access, as these drivers are part > of rpmsg bus, able to allocate memory from iommus is basic > requirement. So expose this bus so that iommu drivers can > add ops to this. IOMMU drivers are built-in while rpmsg can be built as a module, so that isn't always going to work. Robin. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > drivers/rpmsg/rpmsg_core.c | 3 ++- > include/linux/rpmsg.h | 2 +- > 2 files changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index 5a081762afcc..e84c71f8d6ab 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -465,7 +465,7 @@ static int rpmsg_dev_remove(struct device *dev) > return err; > } > > -static struct bus_type rpmsg_bus = { > +struct bus_type rpmsg_bus = { > .name = "rpmsg", > .match = rpmsg_dev_match, > .dev_groups = rpmsg_dev_groups, > @@ -473,6 +473,7 @@ static struct bus_type rpmsg_bus = { > .probe = rpmsg_dev_probe, > .remove = rpmsg_dev_remove, > }; > +EXPORT_SYMBOL(rpmsg_bus); > > int rpmsg_register_device(struct rpmsg_device *rpdev) > { > diff --git a/include/linux/rpmsg.h b/include/linux/rpmsg.h > index ca07366c4c33..869e5946b7df 100644 > --- a/include/linux/rpmsg.h > +++ b/include/linux/rpmsg.h > @@ -49,7 +49,7 @@ struct rpmsg_device; > struct rpmsg_endpoint; > struct rpmsg_device_ops; > struct rpmsg_endpoint_ops; > - > +extern struct bus_type rpmsg_bus; > /** > * struct rpmsg_channel_info - channel info representation > * @name: name of service > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon, robin.murphy, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Many of the rpmsg clients like audio drivers need to allocate dma memory. Make this bus DMA capable so that the child devices can use dma apis. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/rpmsg/rpmsg_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index e84c71f8d6ab..540a3f3567b8 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { .uevent = rpmsg_uevent, .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove, + .force_dma = true, }; EXPORT_SYMBOL(rpmsg_bus); -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla at linaro.org @ 2018-03-02 14:55 UTC (permalink / raw) To: linux-arm-kernel From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> Many of the rpmsg clients like audio drivers need to allocate dma memory. Make this bus DMA capable so that the child devices can use dma apis. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/rpmsg/rpmsg_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index e84c71f8d6ab..540a3f3567b8 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { .uevent = rpmsg_uevent, .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove, + .force_dma = true, }; EXPORT_SYMBOL(rpmsg_bus); -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Srinivas Kandagatla, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> Many of the rpmsg clients like audio drivers need to allocate dma memory. Make this bus DMA capable so that the child devices can use dma apis. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> --- drivers/rpmsg/rpmsg_core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c index e84c71f8d6ab..540a3f3567b8 100644 --- a/drivers/rpmsg/rpmsg_core.c +++ b/drivers/rpmsg/rpmsg_core.c @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { .uevent = rpmsg_uevent, .probe = rpmsg_dev_probe, .remove = rpmsg_dev_remove, + .force_dma = true, }; EXPORT_SYMBOL(rpmsg_bus); -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 16:14 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 16:14 UTC (permalink / raw) To: srinivas.kandagatla, will.deacon, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Many of the rpmsg clients like audio drivers need to allocate > dma memory. Make this bus DMA capable so that the child devices > can use dma apis. AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a virtual one based around shared-memory mailbox communication, so I don't really see how DMA exists in that context - I think maybe that abstraction needs looking at. However, from grepping through the DTs it seems at first glance like the non-trivial things under the "qcom,smd" bus mostly map to actual platform devices via the "qcom,smd-edge" property - if those platform devices are the physical DMA masters, they should be the ones used for DMA API operations. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/rpmsg/rpmsg_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index e84c71f8d6ab..540a3f3567b8 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { > .uevent = rpmsg_uevent, > .probe = rpmsg_dev_probe, > .remove = rpmsg_dev_remove, > + .force_dma = true, Regardless of the above, would you really need to use this brute force hack instead of just fixing the DTs? I'm struggling to find which drivers might currently be relying on this :/ Robin. > }; > EXPORT_SYMBOL(rpmsg_bus); > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 16:14 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 16:14 UTC (permalink / raw) To: linux-arm-kernel On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > Many of the rpmsg clients like audio drivers need to allocate > dma memory. Make this bus DMA capable so that the child devices > can use dma apis. AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a virtual one based around shared-memory mailbox communication, so I don't really see how DMA exists in that context - I think maybe that abstraction needs looking at. However, from grepping through the DTs it seems at first glance like the non-trivial things under the "qcom,smd" bus mostly map to actual platform devices via the "qcom,smd-edge" property - if those platform devices are the physical DMA masters, they should be the ones used for DMA API operations. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/rpmsg/rpmsg_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index e84c71f8d6ab..540a3f3567b8 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { > .uevent = rpmsg_uevent, > .probe = rpmsg_dev_probe, > .remove = rpmsg_dev_remove, > + .force_dma = true, Regardless of the above, would you really need to use this brute force hack instead of just fixing the DTs? I'm struggling to find which drivers might currently be relying on this :/ Robin. > }; > EXPORT_SYMBOL(rpmsg_bus); > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 16:14 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 16:14 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On 02/03/18 14:55, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > Many of the rpmsg clients like audio drivers need to allocate > dma memory. Make this bus DMA capable so that the child devices > can use dma apis. AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a virtual one based around shared-memory mailbox communication, so I don't really see how DMA exists in that context - I think maybe that abstraction needs looking at. However, from grepping through the DTs it seems at first glance like the non-trivial things under the "qcom,smd" bus mostly map to actual platform devices via the "qcom,smd-edge" property - if those platform devices are the physical DMA masters, they should be the ones used for DMA API operations. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > drivers/rpmsg/rpmsg_core.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > index e84c71f8d6ab..540a3f3567b8 100644 > --- a/drivers/rpmsg/rpmsg_core.c > +++ b/drivers/rpmsg/rpmsg_core.c > @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { > .uevent = rpmsg_uevent, > .probe = rpmsg_dev_probe, > .remove = rpmsg_dev_remove, > + .force_dma = true, Regardless of the above, would you really need to use this brute force hack instead of just fixing the DTs? I'm struggling to find which drivers might currently be relying on this :/ Robin. > }; > EXPORT_SYMBOL(rpmsg_bus); > > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable 2018-03-02 16:14 ` Robin Murphy @ 2018-03-02 16:40 ` Srinivas Kandagatla -1 siblings, 0 replies; 32+ messages in thread From: Srinivas Kandagatla @ 2018-03-02 16:40 UTC (permalink / raw) To: Robin Murphy, will.deacon, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc Thanks for your time, On 02/03/18 16:14, Robin Murphy wrote: > On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> Many of the rpmsg clients like audio drivers need to allocate >> dma memory. Make this bus DMA capable so that the child devices >> can use dma apis. > > AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a > virtual one based around shared-memory mailbox communication, so I don't > really see how DMA exists in that context - I think maybe that > abstraction needs looking at. > > However, from grepping through the DTs it seems at first glance like the > non-trivial things under the "qcom,smd" bus mostly map to actual > platform devices via the "qcom,smd-edge" property - if those platform > devices are the physical DMA masters, they should be the ones used for > DMA API operations. Currently there are very limited rpmsg devices in the mainline that use dma. Only one I can think of is wcnss WIFI driver which models up itself into another layer of platform device. Not sure if the DMA was the reason to do that. However am working on audio drivers [1] which I modeled up as children of the rpmsg bus, so the problem started. There is an IOMMU in between APPs and DSP which provides audio services. There are also other projects like FastRPC which have used similar driver model which ended up with same issues. It all depends on how you model your driver. Audio case we have a rpmsg channel which exposes audio functionality. so If we want to use the iommu/dma operations we have to add another layer of platform device. Which also means that rpmsg channel notifications have to be passed to these platform devices in some way. Am not 100% sure if this correct way to fix the issue. > >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/rpmsg/rpmsg_core.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c >> index e84c71f8d6ab..540a3f3567b8 100644 >> --- a/drivers/rpmsg/rpmsg_core.c >> +++ b/drivers/rpmsg/rpmsg_core.c >> @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { >> .uevent = rpmsg_uevent, >> .probe = rpmsg_dev_probe, >> .remove = rpmsg_dev_remove, >> + .force_dma = true, > > Regardless of the above, would you really need to use this brute force > hack instead of just fixing the DTs? I'm struggling to find which > drivers might currently be relying on this :/ This is one of the two issues. dma-ranges might work in this case, but we still have iommu case. > > Robin. > >> }; >> EXPORT_SYMBOL(rpmsg_bus); >> thanks, srini [1]: https://lkml.org/lkml/2018/2/13/719 ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-02 16:40 ` Srinivas Kandagatla 0 siblings, 0 replies; 32+ messages in thread From: Srinivas Kandagatla @ 2018-03-02 16:40 UTC (permalink / raw) To: linux-arm-kernel Thanks for your time, On 02/03/18 16:14, Robin Murphy wrote: > On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> Many of the rpmsg clients like audio drivers need to allocate >> dma memory. Make this bus DMA capable so that the child devices >> can use dma apis. > > AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a > virtual one based around shared-memory mailbox communication, so I don't > really see how DMA exists in that context - I think maybe that > abstraction needs looking at. > > However, from grepping through the DTs it seems at first glance like the > non-trivial things under the "qcom,smd" bus mostly map to actual > platform devices via the "qcom,smd-edge" property - if those platform > devices are the physical DMA masters, they should be the ones used for > DMA API operations. Currently there are very limited rpmsg devices in the mainline that use dma. Only one I can think of is wcnss WIFI driver which models up itself into another layer of platform device. Not sure if the DMA was the reason to do that. However am working on audio drivers [1] which I modeled up as children of the rpmsg bus, so the problem started. There is an IOMMU in between APPs and DSP which provides audio services. There are also other projects like FastRPC which have used similar driver model which ended up with same issues. It all depends on how you model your driver. Audio case we have a rpmsg channel which exposes audio functionality. so If we want to use the iommu/dma operations we have to add another layer of platform device. Which also means that rpmsg channel notifications have to be passed to these platform devices in some way. Am not 100% sure if this correct way to fix the issue. > >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/rpmsg/rpmsg_core.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c >> index e84c71f8d6ab..540a3f3567b8 100644 >> --- a/drivers/rpmsg/rpmsg_core.c >> +++ b/drivers/rpmsg/rpmsg_core.c >> @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { >> .uevent = rpmsg_uevent, >> .probe = rpmsg_dev_probe, >> .remove = rpmsg_dev_remove, >> + .force_dma = true, > > Regardless of the above, would you really need to use this brute force > hack instead of just fixing the DTs? I'm struggling to find which > drivers might currently be relying on this :/ This is one of the two issues. dma-ranges might work in this case, but we still have iommu case. > > Robin. > >> }; >> EXPORT_SYMBOL(rpmsg_bus); >> thanks, srini [1]: https://lkml.org/lkml/2018/2/13/719 ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-18 22:47 ` Bjorn Andersson 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Andersson @ 2018-03-18 22:47 UTC (permalink / raw) To: Robin Murphy Cc: srinivas.kandagatla, will.deacon, joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc On Fri 02 Mar 08:14 PST 2018, Robin Murphy wrote: > On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: > > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > > > Many of the rpmsg clients like audio drivers need to allocate > > dma memory. Make this bus DMA capable so that the child devices > > can use dma apis. > > AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a virtual > one based around shared-memory mailbox communication, so I don't really see > how DMA exists in that context - I think maybe that abstraction needs > looking at. > That's right, rpmsg shuffles messages back and forth to some coprocessor over shared memory, the contexts generating and receiving these messages are "rpmsg devices". The problem Srinivas is facing is that one of these rpmsg devices is trying to allocate and map a larger chunk of memory to be shared with the coprocessor, which is then going to be referenced in the messages being passed in rpmsg. > However, from grepping through the DTs it seems at first glance like the > non-trivial things under the "qcom,smd" bus mostly map to actual platform > devices via the "qcom,smd-edge" property - if those platform devices are the > physical DMA masters, they should be the ones used for DMA API operations. > One of the rpmsg implementations is virtio based and have a similar problem, there dma_alloc*() is called with dev->parent->parent as device, but this causes issues as dev->parent might not be what the original author expected it to -- so this needs to be reworked as well. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > --- > > drivers/rpmsg/rpmsg_core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > > index e84c71f8d6ab..540a3f3567b8 100644 > > --- a/drivers/rpmsg/rpmsg_core.c > > +++ b/drivers/rpmsg/rpmsg_core.c > > @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { > > .uevent = rpmsg_uevent, > > .probe = rpmsg_dev_probe, > > .remove = rpmsg_dev_remove, > > + .force_dma = true, > > Regardless of the above, would you really need to use this brute force hack > instead of just fixing the DTs? I'm struggling to find which drivers might > currently be relying on this :/ > The rpmsg devices, described as child nodes of rpmsg bus relates to specific functions in the coprocessor firmware. The fact that the firmware can be started and stopped dynamically makes the current layout quite convenient (in comparison to e.g. how we would describe a mailbox). We know for these cases that dev->parent->parent is a remoteproc instance representing the coprocessor that sits on the other side of the communication channel. So we did investigate if we could just have that to allocate and map buffers. The problem with this is that these functions has multiple iommu contexts. Regards, Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-18 22:47 ` Bjorn Andersson 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Andersson @ 2018-03-18 22:47 UTC (permalink / raw) To: linux-arm-kernel On Fri 02 Mar 08:14 PST 2018, Robin Murphy wrote: > On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: > > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > > > Many of the rpmsg clients like audio drivers need to allocate > > dma memory. Make this bus DMA capable so that the child devices > > can use dma apis. > > AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a virtual > one based around shared-memory mailbox communication, so I don't really see > how DMA exists in that context - I think maybe that abstraction needs > looking at. > That's right, rpmsg shuffles messages back and forth to some coprocessor over shared memory, the contexts generating and receiving these messages are "rpmsg devices". The problem Srinivas is facing is that one of these rpmsg devices is trying to allocate and map a larger chunk of memory to be shared with the coprocessor, which is then going to be referenced in the messages being passed in rpmsg. > However, from grepping through the DTs it seems at first glance like the > non-trivial things under the "qcom,smd" bus mostly map to actual platform > devices via the "qcom,smd-edge" property - if those platform devices are the > physical DMA masters, they should be the ones used for DMA API operations. > One of the rpmsg implementations is virtio based and have a similar problem, there dma_alloc*() is called with dev->parent->parent as device, but this causes issues as dev->parent might not be what the original author expected it to -- so this needs to be reworked as well. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > --- > > drivers/rpmsg/rpmsg_core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > > index e84c71f8d6ab..540a3f3567b8 100644 > > --- a/drivers/rpmsg/rpmsg_core.c > > +++ b/drivers/rpmsg/rpmsg_core.c > > @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { > > .uevent = rpmsg_uevent, > > .probe = rpmsg_dev_probe, > > .remove = rpmsg_dev_remove, > > + .force_dma = true, > > Regardless of the above, would you really need to use this brute force hack > instead of just fixing the DTs? I'm struggling to find which drivers might > currently be relying on this :/ > The rpmsg devices, described as child nodes of rpmsg bus relates to specific functions in the coprocessor firmware. The fact that the firmware can be started and stopped dynamically makes the current layout quite convenient (in comparison to e.g. how we would describe a mailbox). We know for these cases that dev->parent->parent is a remoteproc instance representing the coprocessor that sits on the other side of the communication channel. So we did investigate if we could just have that to allocate and map buffers. The problem with this is that these functions has multiple iommu contexts. Regards, Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable @ 2018-03-18 22:47 ` Bjorn Andersson 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Andersson @ 2018-03-18 22:47 UTC (permalink / raw) To: Robin Murphy Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On Fri 02 Mar 08:14 PST 2018, Robin Murphy wrote: > On 02/03/18 14:55, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote: > > From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > > > Many of the rpmsg clients like audio drivers need to allocate > > dma memory. Make this bus DMA capable so that the child devices > > can use dma apis. > > AFAICS after 15 minutes in the docs and code, the rpmsg "bus" is a virtual > one based around shared-memory mailbox communication, so I don't really see > how DMA exists in that context - I think maybe that abstraction needs > looking at. > That's right, rpmsg shuffles messages back and forth to some coprocessor over shared memory, the contexts generating and receiving these messages are "rpmsg devices". The problem Srinivas is facing is that one of these rpmsg devices is trying to allocate and map a larger chunk of memory to be shared with the coprocessor, which is then going to be referenced in the messages being passed in rpmsg. > However, from grepping through the DTs it seems at first glance like the > non-trivial things under the "qcom,smd" bus mostly map to actual platform > devices via the "qcom,smd-edge" property - if those platform devices are the > physical DMA masters, they should be the ones used for DMA API operations. > One of the rpmsg implementations is virtio based and have a similar problem, there dma_alloc*() is called with dev->parent->parent as device, but this causes issues as dev->parent might not be what the original author expected it to -- so this needs to be reworked as well. > > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > --- > > drivers/rpmsg/rpmsg_core.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/rpmsg/rpmsg_core.c b/drivers/rpmsg/rpmsg_core.c > > index e84c71f8d6ab..540a3f3567b8 100644 > > --- a/drivers/rpmsg/rpmsg_core.c > > +++ b/drivers/rpmsg/rpmsg_core.c > > @@ -472,6 +472,7 @@ struct bus_type rpmsg_bus = { > > .uevent = rpmsg_uevent, > > .probe = rpmsg_dev_probe, > > .remove = rpmsg_dev_remove, > > + .force_dma = true, > > Regardless of the above, would you really need to use this brute force hack > instead of just fixing the DTs? I'm struggling to find which drivers might > currently be relying on this :/ > The rpmsg devices, described as child nodes of rpmsg bus relates to specific functions in the coprocessor firmware. The fact that the firmware can be started and stopped dynamically makes the current layout quite convenient (in comparison to e.g. how we would describe a mailbox). We know for these cases that dev->parent->parent is a remoteproc instance representing the coprocessor that sits on the other side of the communication channel. So we did investigate if we could just have that to allocate and map buffers. The problem with this is that these functions has multiple iommu contexts. Regards, Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon, robin.murphy, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc, Srinivas Kandagatla From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> On Qualcomm SoCs, ADSP exposes many functions like audio and others. These services need iommu access to allocate any memory for the DSP. As these drivers are childeren of rpmsg bus, able to allocate memory from iommus is basic requirement. So set arm smmu iommu ops for this bus type. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/iommu/arm-smmu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index e6920d32ac9e..9b63489af15c 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -53,6 +53,7 @@ #include <linux/spinlock.h> #include <linux/amba/bus.h> +#include <linux/rpmsg.h> #include "io-pgtable.h" #include "arm-smmu-regs.h" @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) bus_set_iommu(&pci_bus_type, &arm_smmu_ops); } #endif +#ifdef CONFIG_RPMSG + if (!iommu_present(&rpmsg_bus)) + bus_set_iommu(&rpmsg_bus, &arm_smmu_ops); +#endif } static int arm_smmu_device_probe(struct platform_device *pdev) -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla at linaro.org @ 2018-03-02 14:55 UTC (permalink / raw) To: linux-arm-kernel From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> On Qualcomm SoCs, ADSP exposes many functions like audio and others. These services need iommu access to allocate any memory for the DSP. As these drivers are childeren of rpmsg bus, able to allocate memory from iommus is basic requirement. So set arm smmu iommu ops for this bus type. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> --- drivers/iommu/arm-smmu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index e6920d32ac9e..9b63489af15c 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -53,6 +53,7 @@ #include <linux/spinlock.h> #include <linux/amba/bus.h> +#include <linux/rpmsg.h> #include "io-pgtable.h" #include "arm-smmu-regs.h" @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) bus_set_iommu(&pci_bus_type, &arm_smmu_ops); } #endif +#ifdef CONFIG_RPMSG + if (!iommu_present(&rpmsg_bus)) + bus_set_iommu(&rpmsg_bus, &arm_smmu_ops); +#endif } static int arm_smmu_device_probe(struct platform_device *pdev) -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 0 siblings, 0 replies; 32+ messages in thread From: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A @ 2018-03-02 14:55 UTC (permalink / raw) To: will.deacon-5wv7dgnIgG8, robin.murphy-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Srinivas Kandagatla, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> On Qualcomm SoCs, ADSP exposes many functions like audio and others. These services need iommu access to allocate any memory for the DSP. As these drivers are childeren of rpmsg bus, able to allocate memory from iommus is basic requirement. So set arm smmu iommu ops for this bus type. Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> --- drivers/iommu/arm-smmu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index e6920d32ac9e..9b63489af15c 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -53,6 +53,7 @@ #include <linux/spinlock.h> #include <linux/amba/bus.h> +#include <linux/rpmsg.h> #include "io-pgtable.h" #include "arm-smmu-regs.h" @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) bus_set_iommu(&pci_bus_type, &arm_smmu_ops); } #endif +#ifdef CONFIG_RPMSG + if (!iommu_present(&rpmsg_bus)) + bus_set_iommu(&rpmsg_bus, &arm_smmu_ops); +#endif } static int arm_smmu_device_probe(struct platform_device *pdev) -- 2.15.1 ^ permalink raw reply related [flat|nested] 32+ messages in thread
* Re: [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-03-02 16:59 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 16:59 UTC (permalink / raw) To: srinivas.kandagatla, will.deacon, bjorn.andersson Cc: joro, ohad, linux-arm-kernel, iommu, linux-kernel, linux-remoteproc On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > On Qualcomm SoCs, ADSP exposes many functions like audio and > others. These services need iommu access to allocate any > memory for the DSP. As these drivers are childeren of > rpmsg bus, able to allocate memory from iommus is basic > requirement. So set arm smmu iommu ops for this bus type. Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with a remote processor (thus rpmsg devices are called channels)." I'd instinctively assume that a remote processor already has its own memory, and that a communication channel doesn't somehow go directly through an IOMMU, so that "basic requirement" seems like a pretty big assumption. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/iommu/arm-smmu.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index e6920d32ac9e..9b63489af15c 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -53,6 +53,7 @@ > #include <linux/spinlock.h> > > #include <linux/amba/bus.h> > +#include <linux/rpmsg.h> > > #include "io-pgtable.h" > #include "arm-smmu-regs.h" > @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) > bus_set_iommu(&pci_bus_type, &arm_smmu_ops); > } > #endif > +#ifdef CONFIG_RPMSG Ah, so this will at least build OK with RPMSG=m, but I doubt it does what you want it to in that case. Robin. > + if (!iommu_present(&rpmsg_bus)) > + bus_set_iommu(&rpmsg_bus, &arm_smmu_ops); > +#endif > } > > static int arm_smmu_device_probe(struct platform_device *pdev) > ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-03-02 16:59 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 16:59 UTC (permalink / raw) To: linux-arm-kernel On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > > On Qualcomm SoCs, ADSP exposes many functions like audio and > others. These services need iommu access to allocate any > memory for the DSP. As these drivers are childeren of > rpmsg bus, able to allocate memory from iommus is basic > requirement. So set arm smmu iommu ops for this bus type. Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with a remote processor (thus rpmsg devices are called channels)." I'd instinctively assume that a remote processor already has its own memory, and that a communication channel doesn't somehow go directly through an IOMMU, so that "basic requirement" seems like a pretty big assumption. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > drivers/iommu/arm-smmu.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index e6920d32ac9e..9b63489af15c 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -53,6 +53,7 @@ > #include <linux/spinlock.h> > > #include <linux/amba/bus.h> > +#include <linux/rpmsg.h> > > #include "io-pgtable.h" > #include "arm-smmu-regs.h" > @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) > bus_set_iommu(&pci_bus_type, &arm_smmu_ops); > } > #endif > +#ifdef CONFIG_RPMSG Ah, so this will at least build OK with RPMSG=m, but I doubt it does what you want it to in that case. Robin. > + if (!iommu_present(&rpmsg_bus)) > + bus_set_iommu(&rpmsg_bus, &arm_smmu_ops); > +#endif > } > > static int arm_smmu_device_probe(struct platform_device *pdev) > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-03-02 16:59 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-03-02 16:59 UTC (permalink / raw) To: srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A, will.deacon-5wv7dgnIgG8, bjorn.andersson-QSEj5FYQhm4dnm+yROfE0A Cc: ohad-Ix1uc/W3ht7QT0dZR+AlfA, linux-remoteproc-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r On 02/03/18 14:55, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote: > From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > > On Qualcomm SoCs, ADSP exposes many functions like audio and > others. These services need iommu access to allocate any > memory for the DSP. As these drivers are childeren of > rpmsg bus, able to allocate memory from iommus is basic > requirement. So set arm smmu iommu ops for this bus type. Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with a remote processor (thus rpmsg devices are called channels)." I'd instinctively assume that a remote processor already has its own memory, and that a communication channel doesn't somehow go directly through an IOMMU, so that "basic requirement" seems like a pretty big assumption. > Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> > --- > drivers/iommu/arm-smmu.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c > index e6920d32ac9e..9b63489af15c 100644 > --- a/drivers/iommu/arm-smmu.c > +++ b/drivers/iommu/arm-smmu.c > @@ -53,6 +53,7 @@ > #include <linux/spinlock.h> > > #include <linux/amba/bus.h> > +#include <linux/rpmsg.h> > > #include "io-pgtable.h" > #include "arm-smmu-regs.h" > @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) > bus_set_iommu(&pci_bus_type, &arm_smmu_ops); > } > #endif > +#ifdef CONFIG_RPMSG Ah, so this will at least build OK with RPMSG=m, but I doubt it does what you want it to in that case. Robin. > + if (!iommu_present(&rpmsg_bus)) > + bus_set_iommu(&rpmsg_bus, &arm_smmu_ops); > +#endif > } > > static int arm_smmu_device_probe(struct platform_device *pdev) > ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-05-07 19:28 ` Bjorn Andersson 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Andersson @ 2018-05-07 19:28 UTC (permalink / raw) To: Robin Murphy Cc: Srinivas Kandagatla, Will Deacon, Joerg Roedel, Ohad Ben-Cohen, LAKML, iommu, lkml, open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy@arm.com> wrote: > On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: >> >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> On Qualcomm SoCs, ADSP exposes many functions like audio and >> others. These services need iommu access to allocate any >> memory for the DSP. As these drivers are childeren of >> rpmsg bus, able to allocate memory from iommus is basic >> requirement. So set arm smmu iommu ops for this bus type. > Forgot to answer this and the dma_ops patch seems to be going in the right direction. > > Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with > a remote processor (thus rpmsg devices are called channels)." > > I'd instinctively assume that a remote processor already has its own memory, > and that a communication channel doesn't somehow go directly through an > IOMMU, so that "basic requirement" seems like a pretty big assumption. > As of today rpmsg exclusively uses system memory for implementing the communication fifos, but this memory is owned/handled by the rpmsg bus. The need here is for drivers on top of the rpmsg_bus, implementing some application-level protocol that requires indirection buffers; e.g. to achieve zero copy of audio or image buffers that the remote processor is expected to operate on. In this case the device sitting on top of the rpmsg bus will have to map the buffer to the appropriate context and can then send application specific control requests referencing this mapping. As different parts of the firmware might operate in different contexts it's not feasible to utilize the parent's (the rpmsg_bus) context for these indirection buffers. >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/iommu/arm-smmu.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index e6920d32ac9e..9b63489af15c 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -53,6 +53,7 @@ >> #include <linux/spinlock.h> >> #include <linux/amba/bus.h> >> +#include <linux/rpmsg.h> >> #include "io-pgtable.h" >> #include "arm-smmu-regs.h" >> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) >> bus_set_iommu(&pci_bus_type, &arm_smmu_ops); >> } >> #endif >> +#ifdef CONFIG_RPMSG > > > Ah, so this will at least build OK with RPMSG=m, but I doubt it does what > you want it to in that case. > Things have been refactored but the core has remained tristate, causing extra head aches in various areas. I think it's very reasonable to review the rpmsg config options and make CONFIG_RPMSG bool. So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by. That said I'm generally concerned about the first probed iommu implementation assigning itself as the sole iommu implementation for all busses, but I guess we haven't yet hit the point where there are different iommu implementations in a single SoC? Regards, Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-05-07 19:28 ` Bjorn Andersson 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Andersson @ 2018-05-07 19:28 UTC (permalink / raw) To: linux-arm-kernel On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy@arm.com> wrote: > On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: >> >> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> >> On Qualcomm SoCs, ADSP exposes many functions like audio and >> others. These services need iommu access to allocate any >> memory for the DSP. As these drivers are childeren of >> rpmsg bus, able to allocate memory from iommus is basic >> requirement. So set arm smmu iommu ops for this bus type. > Forgot to answer this and the dma_ops patch seems to be going in the right direction. > > Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with > a remote processor (thus rpmsg devices are called channels)." > > I'd instinctively assume that a remote processor already has its own memory, > and that a communication channel doesn't somehow go directly through an > IOMMU, so that "basic requirement" seems like a pretty big assumption. > As of today rpmsg exclusively uses system memory for implementing the communication fifos, but this memory is owned/handled by the rpmsg bus. The need here is for drivers on top of the rpmsg_bus, implementing some application-level protocol that requires indirection buffers; e.g. to achieve zero copy of audio or image buffers that the remote processor is expected to operate on. In this case the device sitting on top of the rpmsg bus will have to map the buffer to the appropriate context and can then send application specific control requests referencing this mapping. As different parts of the firmware might operate in different contexts it's not feasible to utilize the parent's (the rpmsg_bus) context for these indirection buffers. >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >> --- >> drivers/iommu/arm-smmu.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index e6920d32ac9e..9b63489af15c 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -53,6 +53,7 @@ >> #include <linux/spinlock.h> >> #include <linux/amba/bus.h> >> +#include <linux/rpmsg.h> >> #include "io-pgtable.h" >> #include "arm-smmu-regs.h" >> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) >> bus_set_iommu(&pci_bus_type, &arm_smmu_ops); >> } >> #endif >> +#ifdef CONFIG_RPMSG > > > Ah, so this will at least build OK with RPMSG=m, but I doubt it does what > you want it to in that case. > Things have been refactored but the core has remained tristate, causing extra head aches in various areas. I think it's very reasonable to review the rpmsg config options and make CONFIG_RPMSG bool. So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by. That said I'm generally concerned about the first probed iommu implementation assigning itself as the sole iommu implementation for all busses, but I guess we haven't yet hit the point where there are different iommu implementations in a single SoC? Regards, Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-05-07 19:28 ` Bjorn Andersson 0 siblings, 0 replies; 32+ messages in thread From: Bjorn Andersson @ 2018-05-07 19:28 UTC (permalink / raw) To: Robin Murphy Cc: Ohad Ben-Cohen, open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM, Will Deacon, lkml, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Srinivas Kandagatla, LAKML On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote: > On 02/03/18 14:55, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote: >> >> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >> >> On Qualcomm SoCs, ADSP exposes many functions like audio and >> others. These services need iommu access to allocate any >> memory for the DSP. As these drivers are childeren of >> rpmsg bus, able to allocate memory from iommus is basic >> requirement. So set arm smmu iommu ops for this bus type. > Forgot to answer this and the dma_ops patch seems to be going in the right direction. > > Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with > a remote processor (thus rpmsg devices are called channels)." > > I'd instinctively assume that a remote processor already has its own memory, > and that a communication channel doesn't somehow go directly through an > IOMMU, so that "basic requirement" seems like a pretty big assumption. > As of today rpmsg exclusively uses system memory for implementing the communication fifos, but this memory is owned/handled by the rpmsg bus. The need here is for drivers on top of the rpmsg_bus, implementing some application-level protocol that requires indirection buffers; e.g. to achieve zero copy of audio or image buffers that the remote processor is expected to operate on. In this case the device sitting on top of the rpmsg bus will have to map the buffer to the appropriate context and can then send application specific control requests referencing this mapping. As different parts of the firmware might operate in different contexts it's not feasible to utilize the parent's (the rpmsg_bus) context for these indirection buffers. >> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >> --- >> drivers/iommu/arm-smmu.c | 5 +++++ >> 1 file changed, 5 insertions(+) >> >> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >> index e6920d32ac9e..9b63489af15c 100644 >> --- a/drivers/iommu/arm-smmu.c >> +++ b/drivers/iommu/arm-smmu.c >> @@ -53,6 +53,7 @@ >> #include <linux/spinlock.h> >> #include <linux/amba/bus.h> >> +#include <linux/rpmsg.h> >> #include "io-pgtable.h" >> #include "arm-smmu-regs.h" >> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) >> bus_set_iommu(&pci_bus_type, &arm_smmu_ops); >> } >> #endif >> +#ifdef CONFIG_RPMSG > > > Ah, so this will at least build OK with RPMSG=m, but I doubt it does what > you want it to in that case. > Things have been refactored but the core has remained tristate, causing extra head aches in various areas. I think it's very reasonable to review the rpmsg config options and make CONFIG_RPMSG bool. So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by. That said I'm generally concerned about the first probed iommu implementation assigning itself as the sole iommu implementation for all busses, but I guess we haven't yet hit the point where there are different iommu implementations in a single SoC? Regards, Bjorn ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-05-11 18:24 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-05-11 18:24 UTC (permalink / raw) To: Bjorn Andersson Cc: Srinivas Kandagatla, Will Deacon, Joerg Roedel, Ohad Ben-Cohen, LAKML, iommu, lkml, open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM On 07/05/18 20:28, Bjorn Andersson wrote: > On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy@arm.com> wrote: >> On 02/03/18 14:55, srinivas.kandagatla@linaro.org wrote: >>> >>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> >>> On Qualcomm SoCs, ADSP exposes many functions like audio and >>> others. These services need iommu access to allocate any >>> memory for the DSP. As these drivers are childeren of >>> rpmsg bus, able to allocate memory from iommus is basic >>> requirement. So set arm smmu iommu ops for this bus type. >> > > Forgot to answer this and the dma_ops patch seems to be going in the > right direction. > >> >> Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with >> a remote processor (thus rpmsg devices are called channels)." >> >> I'd instinctively assume that a remote processor already has its own memory, >> and that a communication channel doesn't somehow go directly through an >> IOMMU, so that "basic requirement" seems like a pretty big assumption. >> > > As of today rpmsg exclusively uses system memory for implementing the > communication fifos, but this memory is owned/handled by the rpmsg > bus. The need here is for drivers on top of the rpmsg_bus, > implementing some application-level protocol that requires indirection > buffers; e.g. to achieve zero copy of audio or image buffers that the > remote processor is expected to operate on. In this case the device > sitting on top of the rpmsg bus will have to map the buffer to the > appropriate context and can then send application specific control > requests referencing this mapping. Right, but that's more or less what I was getting at - rpmsg can be used as a means to signal some DMA master device to start doing a thing, but that thing itself is unrelated to rpmsg, and it by no means implies that everything which rpmsg can talk to is always capable of system-wide DMA. It's no different if that communication channel is a hardware mailbox or an I2C/SPI/USB/etc. link, rather than virtio; we wouldn't automatically consider devices on the other end of those to be directly connected to an IOMMU either. IOMMU and DMA operations are highly dependent on the physical hardware topology, which is why I really don't like trying to shoehorn them into software constructs without modelling the actual hardware reasonably accurately. For instance it's not unheard of for remote processors in a SoC to see a different physical memory map from the main application processors - how would rpmsg try to describe that? What even is the address space of the rpmsg "bus"? > As different parts of the firmware might operate in different contexts > it's not feasible to utilize the parent's (the rpmsg_bus) context for > these indirection buffers. Indeed, and I maintain that that wouldn't be the right thing to do anyway. As before, I think the most accurate way to model the situation with the tools we have available is to have the actual hardware function represented by a platform device, which is associated with a corresponding rpmsg endpoint. Then the driver can manage communication in the rpmsg context, and physical DMA setup in the 'real' hardware context, and everything looks sane without questionable abstraction breakage. Since this looked to be more or less what is actually implemented anyway, it doesn't seem all that hard to refine; if there are multiple DMA master functions identified distinctly to the IOMMU, then they could either be represented as separate platform devices with explicit IOMMU specifiers, or you could model the actual DSP subsystem hardware as its own bus-like arrangement with an iommu-map arrangement translating function identifiers to IOMMU identifiers. What I don't like is forcing IOMMU drivers to pretend that some data in a shared memory buffer is itself directly capable of generating transactions on the interconnect. If other 'indirect' bus abstractions like CoreSight can get this right, I don't see why rpmsg deserves to be special. >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> drivers/iommu/arm-smmu.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >>> index e6920d32ac9e..9b63489af15c 100644 >>> --- a/drivers/iommu/arm-smmu.c >>> +++ b/drivers/iommu/arm-smmu.c >>> @@ -53,6 +53,7 @@ >>> #include <linux/spinlock.h> >>> #include <linux/amba/bus.h> >>> +#include <linux/rpmsg.h> >>> #include "io-pgtable.h" >>> #include "arm-smmu-regs.h" >>> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) >>> bus_set_iommu(&pci_bus_type, &arm_smmu_ops); >>> } >>> #endif >>> +#ifdef CONFIG_RPMSG >> >> >> Ah, so this will at least build OK with RPMSG=m, but I doubt it does what >> you want it to in that case. >> > > Things have been refactored but the core has remained tristate, > causing extra head aches in various areas. I think it's very > reasonable to review the rpmsg config options and make CONFIG_RPMSG > bool. > > So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by. > > That said I'm generally concerned about the first probed iommu > implementation assigning itself as the sole iommu implementation for > all busses, but I guess we haven't yet hit the point where there are > different iommu implementations in a single SoC? As it happens I do know of one such SoC already - Rockchip RK3288 seems to have an undocumented Arm SMMU alongside all the rockchip-iommu instances, but it's not used by Linux (and I have no idea what it was intended for; I just went and poked the intriguing "peripheral MMU" region of the memory map and found what looks an awful lot like an Arm MMU-400). More realistically, I also know of folks using the Arm Juno dev board with an MMU-600 in the add-on FPGA tile, which would have that driver-probe-order fight with the MMU-401 instances in the SoC, but I figure they were either using an older firmware which didn't enable the latter or just got lucky with not having the SMMUv2 driver enabled. But yes, the per-bus ops thing is awful and I've been complaining about it for years now. Since iommu_fwspec we at least have the foundations for per-device ops in place now, but as is often the case, getting 80% of the way there is simple[1], whilst the last 20% (like replacing iommu_domain_alloc(), and where to call iommu_{add,remove}_device() from) is really hard. Robin. [1] https://www.mail-archive.com/iommu@lists.linux-foundation.org/msg14576.html ^ permalink raw reply [flat|nested] 32+ messages in thread
* [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-05-11 18:24 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-05-11 18:24 UTC (permalink / raw) To: linux-arm-kernel On 07/05/18 20:28, Bjorn Andersson wrote: > On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy@arm.com> wrote: >> On 02/03/18 14:55, srinivas.kandagatla at linaro.org wrote: >>> >>> From: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> >>> On Qualcomm SoCs, ADSP exposes many functions like audio and >>> others. These services need iommu access to allocate any >>> memory for the DSP. As these drivers are childeren of >>> rpmsg bus, able to allocate memory from iommus is basic >>> requirement. So set arm smmu iommu ops for this bus type. >> > > Forgot to answer this and the dma_ops patch seems to be going in the > right direction. > >> >> Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with >> a remote processor (thus rpmsg devices are called channels)." >> >> I'd instinctively assume that a remote processor already has its own memory, >> and that a communication channel doesn't somehow go directly through an >> IOMMU, so that "basic requirement" seems like a pretty big assumption. >> > > As of today rpmsg exclusively uses system memory for implementing the > communication fifos, but this memory is owned/handled by the rpmsg > bus. The need here is for drivers on top of the rpmsg_bus, > implementing some application-level protocol that requires indirection > buffers; e.g. to achieve zero copy of audio or image buffers that the > remote processor is expected to operate on. In this case the device > sitting on top of the rpmsg bus will have to map the buffer to the > appropriate context and can then send application specific control > requests referencing this mapping. Right, but that's more or less what I was getting at - rpmsg can be used as a means to signal some DMA master device to start doing a thing, but that thing itself is unrelated to rpmsg, and it by no means implies that everything which rpmsg can talk to is always capable of system-wide DMA. It's no different if that communication channel is a hardware mailbox or an I2C/SPI/USB/etc. link, rather than virtio; we wouldn't automatically consider devices on the other end of those to be directly connected to an IOMMU either. IOMMU and DMA operations are highly dependent on the physical hardware topology, which is why I really don't like trying to shoehorn them into software constructs without modelling the actual hardware reasonably accurately. For instance it's not unheard of for remote processors in a SoC to see a different physical memory map from the main application processors - how would rpmsg try to describe that? What even is the address space of the rpmsg "bus"? > As different parts of the firmware might operate in different contexts > it's not feasible to utilize the parent's (the rpmsg_bus) context for > these indirection buffers. Indeed, and I maintain that that wouldn't be the right thing to do anyway. As before, I think the most accurate way to model the situation with the tools we have available is to have the actual hardware function represented by a platform device, which is associated with a corresponding rpmsg endpoint. Then the driver can manage communication in the rpmsg context, and physical DMA setup in the 'real' hardware context, and everything looks sane without questionable abstraction breakage. Since this looked to be more or less what is actually implemented anyway, it doesn't seem all that hard to refine; if there are multiple DMA master functions identified distinctly to the IOMMU, then they could either be represented as separate platform devices with explicit IOMMU specifiers, or you could model the actual DSP subsystem hardware as its own bus-like arrangement with an iommu-map arrangement translating function identifiers to IOMMU identifiers. What I don't like is forcing IOMMU drivers to pretend that some data in a shared memory buffer is itself directly capable of generating transactions on the interconnect. If other 'indirect' bus abstractions like CoreSight can get this right, I don't see why rpmsg deserves to be special. >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> >>> --- >>> drivers/iommu/arm-smmu.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >>> index e6920d32ac9e..9b63489af15c 100644 >>> --- a/drivers/iommu/arm-smmu.c >>> +++ b/drivers/iommu/arm-smmu.c >>> @@ -53,6 +53,7 @@ >>> #include <linux/spinlock.h> >>> #include <linux/amba/bus.h> >>> +#include <linux/rpmsg.h> >>> #include "io-pgtable.h" >>> #include "arm-smmu-regs.h" >>> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) >>> bus_set_iommu(&pci_bus_type, &arm_smmu_ops); >>> } >>> #endif >>> +#ifdef CONFIG_RPMSG >> >> >> Ah, so this will at least build OK with RPMSG=m, but I doubt it does what >> you want it to in that case. >> > > Things have been refactored but the core has remained tristate, > causing extra head aches in various areas. I think it's very > reasonable to review the rpmsg config options and make CONFIG_RPMSG > bool. > > So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by. > > That said I'm generally concerned about the first probed iommu > implementation assigning itself as the sole iommu implementation for > all busses, but I guess we haven't yet hit the point where there are > different iommu implementations in a single SoC? As it happens I do know of one such SoC already - Rockchip RK3288 seems to have an undocumented Arm SMMU alongside all the rockchip-iommu instances, but it's not used by Linux (and I have no idea what it was intended for; I just went and poked the intriguing "peripheral MMU" region of the memory map and found what looks an awful lot like an Arm MMU-400). More realistically, I also know of folks using the Arm Juno dev board with an MMU-600 in the add-on FPGA tile, which would have that driver-probe-order fight with the MMU-401 instances in the SoC, but I figure they were either using an older firmware which didn't enable the latter or just got lucky with not having the SMMUv2 driver enabled. But yes, the per-bus ops thing is awful and I've been complaining about it for years now. Since iommu_fwspec we at least have the foundations for per-device ops in place now, but as is often the case, getting 80% of the way there is simple[1], whilst the last 20% (like replacing iommu_domain_alloc(), and where to call iommu_{add,remove}_device() from) is really hard. Robin. [1] https://www.mail-archive.com/iommu at lists.linux-foundation.org/msg14576.html ^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus @ 2018-05-11 18:24 ` Robin Murphy 0 siblings, 0 replies; 32+ messages in thread From: Robin Murphy @ 2018-05-11 18:24 UTC (permalink / raw) To: Bjorn Andersson Cc: Ohad Ben-Cohen, open list:REMOTE PROCESSOR (REMOTEPROC) SUBSYSTEM, Will Deacon, lkml, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, Srinivas Kandagatla, LAKML On 07/05/18 20:28, Bjorn Andersson wrote: > On Fri, Mar 2, 2018 at 8:59 AM, Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org> wrote: >> On 02/03/18 14:55, srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org wrote: >>> >>> From: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >>> >>> On Qualcomm SoCs, ADSP exposes many functions like audio and >>> others. These services need iommu access to allocate any >>> memory for the DSP. As these drivers are childeren of >>> rpmsg bus, able to allocate memory from iommus is basic >>> requirement. So set arm smmu iommu ops for this bus type. >> > > Forgot to answer this and the dma_ops patch seems to be going in the > right direction. > >> >> Documentation/rpmsg.txt: "Every rpmsg device is a communication channel with >> a remote processor (thus rpmsg devices are called channels)." >> >> I'd instinctively assume that a remote processor already has its own memory, >> and that a communication channel doesn't somehow go directly through an >> IOMMU, so that "basic requirement" seems like a pretty big assumption. >> > > As of today rpmsg exclusively uses system memory for implementing the > communication fifos, but this memory is owned/handled by the rpmsg > bus. The need here is for drivers on top of the rpmsg_bus, > implementing some application-level protocol that requires indirection > buffers; e.g. to achieve zero copy of audio or image buffers that the > remote processor is expected to operate on. In this case the device > sitting on top of the rpmsg bus will have to map the buffer to the > appropriate context and can then send application specific control > requests referencing this mapping. Right, but that's more or less what I was getting at - rpmsg can be used as a means to signal some DMA master device to start doing a thing, but that thing itself is unrelated to rpmsg, and it by no means implies that everything which rpmsg can talk to is always capable of system-wide DMA. It's no different if that communication channel is a hardware mailbox or an I2C/SPI/USB/etc. link, rather than virtio; we wouldn't automatically consider devices on the other end of those to be directly connected to an IOMMU either. IOMMU and DMA operations are highly dependent on the physical hardware topology, which is why I really don't like trying to shoehorn them into software constructs without modelling the actual hardware reasonably accurately. For instance it's not unheard of for remote processors in a SoC to see a different physical memory map from the main application processors - how would rpmsg try to describe that? What even is the address space of the rpmsg "bus"? > As different parts of the firmware might operate in different contexts > it's not feasible to utilize the parent's (the rpmsg_bus) context for > these indirection buffers. Indeed, and I maintain that that wouldn't be the right thing to do anyway. As before, I think the most accurate way to model the situation with the tools we have available is to have the actual hardware function represented by a platform device, which is associated with a corresponding rpmsg endpoint. Then the driver can manage communication in the rpmsg context, and physical DMA setup in the 'real' hardware context, and everything looks sane without questionable abstraction breakage. Since this looked to be more or less what is actually implemented anyway, it doesn't seem all that hard to refine; if there are multiple DMA master functions identified distinctly to the IOMMU, then they could either be represented as separate platform devices with explicit IOMMU specifiers, or you could model the actual DSP subsystem hardware as its own bus-like arrangement with an iommu-map arrangement translating function identifiers to IOMMU identifiers. What I don't like is forcing IOMMU drivers to pretend that some data in a shared memory buffer is itself directly capable of generating transactions on the interconnect. If other 'indirect' bus abstractions like CoreSight can get this right, I don't see why rpmsg deserves to be special. >>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> >>> --- >>> drivers/iommu/arm-smmu.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c >>> index e6920d32ac9e..9b63489af15c 100644 >>> --- a/drivers/iommu/arm-smmu.c >>> +++ b/drivers/iommu/arm-smmu.c >>> @@ -53,6 +53,7 @@ >>> #include <linux/spinlock.h> >>> #include <linux/amba/bus.h> >>> +#include <linux/rpmsg.h> >>> #include "io-pgtable.h" >>> #include "arm-smmu-regs.h" >>> @@ -2168,6 +2169,10 @@ static void arm_smmu_bus_init(void) >>> bus_set_iommu(&pci_bus_type, &arm_smmu_ops); >>> } >>> #endif >>> +#ifdef CONFIG_RPMSG >> >> >> Ah, so this will at least build OK with RPMSG=m, but I doubt it does what >> you want it to in that case. >> > > Things have been refactored but the core has remained tristate, > causing extra head aches in various areas. I think it's very > reasonable to review the rpmsg config options and make CONFIG_RPMSG > bool. > > So with the addition of making CONFIG_RPMSG bool the patch has my Acked-by. > > That said I'm generally concerned about the first probed iommu > implementation assigning itself as the sole iommu implementation for > all busses, but I guess we haven't yet hit the point where there are > different iommu implementations in a single SoC? As it happens I do know of one such SoC already - Rockchip RK3288 seems to have an undocumented Arm SMMU alongside all the rockchip-iommu instances, but it's not used by Linux (and I have no idea what it was intended for; I just went and poked the intriguing "peripheral MMU" region of the memory map and found what looks an awful lot like an Arm MMU-400). More realistically, I also know of folks using the Arm Juno dev board with an MMU-600 in the add-on FPGA tile, which would have that driver-probe-order fight with the MMU-401 instances in the SoC, but I figure they were either using an older firmware which didn't enable the latter or just got lucky with not having the SMMUv2 driver enabled. But yes, the per-bus ops thing is awful and I've been complaining about it for years now. Since iommu_fwspec we at least have the foundations for per-device ops in place now, but as is often the case, getting 80% of the way there is simple[1], whilst the last 20% (like replacing iommu_domain_alloc(), and where to call iommu_{add,remove}_device() from) is really hard. Robin. [1] https://www.mail-archive.com/iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org/msg14576.html ^ permalink raw reply [flat|nested] 32+ messages in thread
end of thread, other threads:[~2018-05-11 18:24 UTC | newest] Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-02 14:55 [PATCH 0/3] drivers: rpmsg: make rpmsg bus DMA capable srinivas.kandagatla 2018-03-02 14:55 ` srinivas.kandagatla at linaro.org 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 2018-03-02 14:55 ` [PATCH 1/3] rpmsg: core: export rpmsg bus type srinivas.kandagatla 2018-03-02 14:55 ` srinivas.kandagatla at linaro.org 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 2018-03-02 15:25 ` Robin Murphy 2018-03-02 15:25 ` Robin Murphy 2018-03-02 15:25 ` Robin Murphy 2018-03-02 14:55 ` [PATCH 2/3] rpmsg: core: make rpmsg bus DMA capable srinivas.kandagatla 2018-03-02 14:55 ` srinivas.kandagatla at linaro.org 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 2018-03-02 16:14 ` Robin Murphy 2018-03-02 16:14 ` Robin Murphy 2018-03-02 16:14 ` Robin Murphy 2018-03-02 16:40 ` Srinivas Kandagatla 2018-03-02 16:40 ` Srinivas Kandagatla 2018-03-18 22:47 ` Bjorn Andersson 2018-03-18 22:47 ` Bjorn Andersson 2018-03-18 22:47 ` Bjorn Andersson 2018-03-02 14:55 ` [PATCH 3/3] iommu: armsmmu: set iommu ops for rpmsg bus srinivas.kandagatla 2018-03-02 14:55 ` srinivas.kandagatla at linaro.org 2018-03-02 14:55 ` srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A 2018-03-02 16:59 ` Robin Murphy 2018-03-02 16:59 ` Robin Murphy 2018-03-02 16:59 ` Robin Murphy 2018-05-07 19:28 ` Bjorn Andersson 2018-05-07 19:28 ` Bjorn Andersson 2018-05-07 19:28 ` Bjorn Andersson 2018-05-11 18:24 ` Robin Murphy 2018-05-11 18:24 ` Robin Murphy 2018-05-11 18:24 ` Robin Murphy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.