2024-05-24 16:43:05

by Dylan Van Assche

[permalink] [raw]
Subject: [PATCH v5 0/2] misc: fastrpc: FastRPC reserved memory assignment for SDM845 SLPI

* About *

The Qualcomm SDM845 SoC has a separate SLPI (Sensor Low Power Island)
DSP for sensors connected to the SoC which is responsible for exposing
sensors to userspace, power saving, and other features.
While sensors are connected to GPIOs of the SoC, they cannot be used
because the hypervisor blocks direct access to the sensors, thus the
DSP must be used to access any sensor on this SoC. The SLPI DSP uses a
GLink edge (dsps) to communicate with the host and has a FastRPC interface
to load files from the host filesystem such as sensor configuration files.
The FastRPC interface does not use regular FastRPC Compute Banks
but instead uses an allocated CMA region through which communication happens.

* Changes *

This patchseries add support to the FastRPC for assigning a coherent memory
region to a DSP via the hypervisor with the correct permissions.
This is necessary to support the SLPI found in the Qualcomm SDM845 SoC which
does not have dedicated FastRPC Compute Banks, in contrast to newer SoCs,
but uses a memory region instead when allocating buffers.

* Related patches *

1. Remoteproc changes to support the SLPI DSP in SDM845 (v3), needs to be applied:
https://lore.kernel.org/linux-remoteproc/[email protected]
2. DTS changes (v5), already applied:
https://lore.kernel.org/linux-devicetree/[email protected]

This serie does not depend on any serie, but all of them are necessary
to enable the feature in the end.

* Changelog *

Changes in v5:
- Adjusted to FastRPC driver changes in 6.9.X.

Changes in v4:
- Fixed possible memory leak when driver encounters an error during probing.

Changes in v3:
- Dropped debug prints.
- Added Reviewed-By tags from v2.

Changes in v2:

- Removed double blank lines
- Dropped dt-bindings property as it is not needed for driver behavior
- Add additional patch to allocate buffers via CMA memory for DSPs
without dedicated FastRPC Compute Banks.

Dylan Van Assche (2):
misc: fastrpc: support complete DMA pool access to the DSP
misc: fastrpc: use coherent pool for untranslated Compute Banks

drivers/misc/fastrpc.c | 24 +++++++++++++++++++++++-
1 file changed, 23 insertions(+), 1 deletion(-)

--
2.45.1



2024-05-24 16:43:20

by Dylan Van Assche

[permalink] [raw]
Subject: [PATCH v5 1/2] misc: fastrpc: support complete DMA pool access to the DSP

To support FastRPC Context Banks which aren't mapped via the SMMU,
make the whole reserved memory region available to the DSP to allow
access to coherent buffers.

This is performed by assigning the memory to the DSP via a hypervisor
call to set the correct permissions for the Virtual Machines on the DSP.
This is only necessary when a memory region is provided for SLPI DSPs
so guard this with a domain ID check.

Signed-off-by: Dylan Van Assche <[email protected]>
Reviewed-by: Caleb Connolly <[email protected]>
---
drivers/misc/fastrpc.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 4c67e2c5a82e..c06667b29055 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -2255,6 +2255,8 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
int i, err, domain_id = -1, vmcount;
const char *domain;
bool secure_dsp;
+ struct device_node *rmem_node;
+ struct reserved_mem *rmem;
unsigned int vmids[FASTRPC_MAX_VMIDS];

err = of_property_read_string(rdev->of_node, "label", &domain);
@@ -2297,6 +2299,23 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
}
}

+ rmem_node = of_parse_phandle(rdev->of_node, "memory-region", 0);
+ if (domain_id == SDSP_DOMAIN_ID && rmem_node) {
+ u64 src_perms;
+
+ rmem = of_reserved_mem_lookup(rmem_node);
+ if (!rmem) {
+ err = -EINVAL;
+ goto fdev_error;
+ }
+
+ src_perms = BIT(QCOM_SCM_VMID_HLOS);
+
+ qcom_scm_assign_mem(rmem->base, rmem->size, &src_perms,
+ data->vmperms, data->vmcount);
+
+ }
+
secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
data->secure = secure_dsp;

--
2.45.1


2024-05-24 16:43:52

by Dylan Van Assche

[permalink] [raw]
Subject: [PATCH v5 2/2] misc: fastrpc: use coherent pool for untranslated Compute Banks

Use fastrpc_remote_heap_alloc to allocate from the FastRPC device
instead of the Compute Bank when the session ID is 0. This ensures
that the allocation is inside the coherent DMA pool which is already
accessible to the DSP. This is necessary to support FastRPC devices
which do not have dedicated Compute Banks such as the SLPI on the SDM845.
The latter uses an allocated CMA region instead of FastRPC Compute Banks.

Signed-off-by: Dylan Van Assche <[email protected]>
Reviewed-by: Caleb Connolly <[email protected]>
---
drivers/misc/fastrpc.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index c06667b29055..f53d20e2e07e 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -953,7 +953,10 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)

ctx->msg_sz = pkt_size;

- err = fastrpc_buf_alloc(ctx->fl, dev, pkt_size, &ctx->buf);
+ if (ctx->fl->sctx->sid)
+ err = fastrpc_buf_alloc(ctx->fl, dev, pkt_size, &ctx->buf);
+ else
+ err = fastrpc_remote_heap_alloc(ctx->fl, dev, pkt_size, &ctx->buf);
if (err)
return err;

--
2.45.1


2024-06-11 12:55:48

by Ekansh Gupta

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] misc: fastrpc: support complete DMA pool access to the DSP



On 5/24/2024 9:44 PM, Dylan Van Assche wrote:
> To support FastRPC Context Banks which aren't mapped via the SMMU,
> make the whole reserved memory region available to the DSP to allow
> access to coherent buffers.
>
> This is performed by assigning the memory to the DSP via a hypervisor
> call to set the correct permissions for the Virtual Machines on the DSP.
> This is only necessary when a memory region is provided for SLPI DSPs
> so guard this with a domain ID check.
>
> Signed-off-by: Dylan Van Assche <[email protected]>
> Reviewed-by: Caleb Connolly <[email protected]>
> ---
Reviewed-by: Ekansh Gupta <[email protected]>
> drivers/misc/fastrpc.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 4c67e2c5a82e..c06667b29055 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2255,6 +2255,8 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> int i, err, domain_id = -1, vmcount;
> const char *domain;
> bool secure_dsp;
> + struct device_node *rmem_node;
> + struct reserved_mem *rmem;
> unsigned int vmids[FASTRPC_MAX_VMIDS];
>
> err = of_property_read_string(rdev->of_node, "label", &domain);
> @@ -2297,6 +2299,23 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> }
> }
>
> + rmem_node = of_parse_phandle(rdev->of_node, "memory-region", 0);
> + if (domain_id == SDSP_DOMAIN_ID && rmem_node) {
> + u64 src_perms;
> +
> + rmem = of_reserved_mem_lookup(rmem_node);
> + if (!rmem) {
> + err = -EINVAL;
> + goto fdev_error;
> + }
> +
> + src_perms = BIT(QCOM_SCM_VMID_HLOS);
> +
> + qcom_scm_assign_mem(rmem->base, rmem->size, &src_perms,
> + data->vmperms, data->vmcount);
> +
> + }
> +
> secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
> data->secure = secure_dsp;
>


2024-06-11 12:57:31

by Ekansh Gupta

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] misc: fastrpc: use coherent pool for untranslated Compute Banks



On 5/24/2024 9:44 PM, Dylan Van Assche wrote:
> Use fastrpc_remote_heap_alloc to allocate from the FastRPC device
> instead of the Compute Bank when the session ID is 0. This ensures
> that the allocation is inside the coherent DMA pool which is already
> accessible to the DSP. This is necessary to support FastRPC devices
> which do not have dedicated Compute Banks such as the SLPI on the SDM845.
> The latter uses an allocated CMA region instead of FastRPC Compute Banks.
>
> Signed-off-by: Dylan Van Assche <[email protected]>
> Reviewed-by: Caleb Connolly <[email protected]>
> ---
Reviewed-by: Ekansh Gupta <[email protected]>
> drivers/misc/fastrpc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index c06667b29055..f53d20e2e07e 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -953,7 +953,10 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>
> ctx->msg_sz = pkt_size;
>
> - err = fastrpc_buf_alloc(ctx->fl, dev, pkt_size, &ctx->buf);
> + if (ctx->fl->sctx->sid)
> + err = fastrpc_buf_alloc(ctx->fl, dev, pkt_size, &ctx->buf);
> + else
> + err = fastrpc_remote_heap_alloc(ctx->fl, dev, pkt_size, &ctx->buf);
> if (err)
> return err;
>


2024-06-11 19:58:05

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v5 1/2] misc: fastrpc: support complete DMA pool access to the DSP

On Fri, May 24, 2024 at 06:14:02PM +0200, Dylan Van Assche wrote:
> To support FastRPC Context Banks which aren't mapped via the SMMU,
> make the whole reserved memory region available to the DSP to allow
> access to coherent buffers.
>
> This is performed by assigning the memory to the DSP via a hypervisor
> call to set the correct permissions for the Virtual Machines on the DSP.
> This is only necessary when a memory region is provided for SLPI DSPs
> so guard this with a domain ID check.
>
> Signed-off-by: Dylan Van Assche <[email protected]>
> Reviewed-by: Caleb Connolly <[email protected]>
> ---
> drivers/misc/fastrpc.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 4c67e2c5a82e..c06667b29055 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2255,6 +2255,8 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> int i, err, domain_id = -1, vmcount;
> const char *domain;
> bool secure_dsp;
> + struct device_node *rmem_node;
> + struct reserved_mem *rmem;
> unsigned int vmids[FASTRPC_MAX_VMIDS];
>
> err = of_property_read_string(rdev->of_node, "label", &domain);
> @@ -2297,6 +2299,23 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
> }
> }
>
> + rmem_node = of_parse_phandle(rdev->of_node, "memory-region", 0);
> + if (domain_id == SDSP_DOMAIN_ID && rmem_node) {

I think we can drop the domain_id check here.

The rest looks good to me.

Reviewed-by: Dmitry Baryshkov <[email protected]>


> + u64 src_perms;
> +
> + rmem = of_reserved_mem_lookup(rmem_node);
> + if (!rmem) {
> + err = -EINVAL;
> + goto fdev_error;
> + }
> +
> + src_perms = BIT(QCOM_SCM_VMID_HLOS);
> +
> + qcom_scm_assign_mem(rmem->base, rmem->size, &src_perms,
> + data->vmperms, data->vmcount);
> +
> + }
> +
> secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
> data->secure = secure_dsp;
>
> --
> 2.45.1
>

--
With best wishes
Dmitry

2024-06-11 20:02:48

by Dmitry Baryshkov

[permalink] [raw]
Subject: Re: [PATCH v5 2/2] misc: fastrpc: use coherent pool for untranslated Compute Banks

On Fri, May 24, 2024 at 06:14:03PM +0200, Dylan Van Assche wrote:
> Use fastrpc_remote_heap_alloc to allocate from the FastRPC device
> instead of the Compute Bank when the session ID is 0. This ensures
> that the allocation is inside the coherent DMA pool which is already
> accessible to the DSP. This is necessary to support FastRPC devices
> which do not have dedicated Compute Banks such as the SLPI on the SDM845.
> The latter uses an allocated CMA region instead of FastRPC Compute Banks.
>
> Signed-off-by: Dylan Van Assche <[email protected]>
> Reviewed-by: Caleb Connolly <[email protected]>
> ---
> drivers/misc/fastrpc.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>

Reviewed-by: Dmitry Baryshkov <[email protected]>


--
With best wishes
Dmitry

2024-06-12 14:57:36

by Srinivas Kandagatla

[permalink] [raw]
Subject: Re: [PATCH v5 0/2] misc: fastrpc: FastRPC reserved memory assignment for SDM845 SLPI


On Fri, 24 May 2024 18:14:01 +0200, Dylan Van Assche wrote:
> * About *
>
> The Qualcomm SDM845 SoC has a separate SLPI (Sensor Low Power Island)
> DSP for sensors connected to the SoC which is responsible for exposing
> sensors to userspace, power saving, and other features.
> While sensors are connected to GPIOs of the SoC, they cannot be used
> because the hypervisor blocks direct access to the sensors, thus the
> DSP must be used to access any sensor on this SoC. The SLPI DSP uses a
> GLink edge (dsps) to communicate with the host and has a FastRPC interface
> to load files from the host filesystem such as sensor configuration files.
> The FastRPC interface does not use regular FastRPC Compute Banks
> but instead uses an allocated CMA region through which communication happens.
>
> [...]

Applied, thanks!

[1/2] misc: fastrpc: support complete DMA pool access to the DSP
commit: 9cee10334bdfa9db771b006fe2eba060f8719687
[2/2] misc: fastrpc: use coherent pool for untranslated Compute Banks
commit: 0ff3de43062785462d9acede2987f35525b1c0ba

Best regards,
--
Srinivas Kandagatla <[email protected]>