| Message ID | 20230511141146.30465-2-me@dylanvanassche.be (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | misc: fastrpc: FastRPC reserved memory assignment for SDM845 SLPI | expand |
On 11/05/2023 15:11, 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. Mapping the whole region sounds very inefficient, and also possibly making the cma region not usable by others. > AFAIU SDM845 does not have any context banks for SDSP. All new SoCs after 865 have moved to having a context bank. For such cases (w/o cb) we can make fastrpc_session_alloc use channel context device instead of session ctx device. As this is going to be an issue when we try to allocate buffers dynamically for that cb. In the newer platforms (from 865) there is support for iommu and context banks on SDSP, so the existing code flow is identical for both ADSP and SDSP. We should be careful not to break newer platfroms while adding support to this. Both myself and Ekansh thought about this and see that the better way to add support to this is by 1. extend fastrpc_session_alloc() to support zero context banks. 2. add flags to mark this and allocate meta data using secure allocation when its required based on this flag. 3. buffer allocation can either go with 2 or with a new flag coming from userspace. > 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 <me@dylanvanassche.be> > Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> > --- > drivers/misc/fastrpc.c | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index f48466960f1b..1ced553ae959 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -2231,6 +2231,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); > @@ -2274,6 +2276,19 @@ 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) { > + rmem = of_reserved_mem_lookup(rmem_node); > + if (!rmem) { > + err = -EINVAL; > + goto fdev_error; > + } > + > + qcom_scm_assign_mem(rmem->base, rmem->size, &data->perms, > + data->vmperms, data->vmcount); vmperms need to be a bit field. > + > + } > + > secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain")); > data->secure = secure_dsp; > --srini
On 30/05/2023 11:52, Srinivas Kandagatla wrote: > On 11/05/2023 15:11, 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. > > Mapping the whole region sounds very inefficient, and also possibly > making the cma region not usable by others. Probably I'm missing something obvious here... The downstream driver maps the whole region, what are the advantages to doing it on a per-allocation basis? What are the other users? > >> > > AFAIU SDM845 does not have any context banks for SDSP. All new SoCs > after 865 have moved to having a context bank. > > For such cases (w/o cb) we can make fastrpc_session_alloc use channel > context device instead of session ctx device. As this is going to be an > issue when we try to allocate buffers dynamically for that cb. Right.. This is what patch 2 does, but presumably the ALLOC_DMA_BUF, MMAP, etc ioctls won't work with the current iteration? > > In the newer platforms (from 865) there is support for iommu and context > banks on SDSP, so the existing code flow is identical for both ADSP and > SDSP. > > > We should be careful not to break newer platfroms while adding support > to this. > > Both myself and Ekansh thought about this and see that the better way to > add support to this is by > > 1. extend fastrpc_session_alloc() to support zero context banks. > > 2. add flags to mark this and allocate meta data using secure allocation > when its required based on this flag. > > 3. buffer allocation can either go with 2 or with a new flag coming > from userspace. This sounds pretty good to me! > > > >> 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 <me@dylanvanassche.be> >> Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> >> --- >> drivers/misc/fastrpc.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >> index f48466960f1b..1ced553ae959 100644 >> --- a/drivers/misc/fastrpc.c >> +++ b/drivers/misc/fastrpc.c >> @@ -2231,6 +2231,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); >> @@ -2274,6 +2276,19 @@ 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) { >> + rmem = of_reserved_mem_lookup(rmem_node); >> + if (!rmem) { >> + err = -EINVAL; >> + goto fdev_error; >> + } >> + >> + qcom_scm_assign_mem(rmem->base, rmem->size, &data->perms, >> + data->vmperms, data->vmcount); > > vmperms need to be a bit field. > >> + >> + } >> + >> secure_dsp = !(of_property_read_bool(rdev->of_node, >> "qcom,non-secure-domain")); >> data->secure = secure_dsp; >> > > --srini
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index f48466960f1b..1ced553ae959 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -2231,6 +2231,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); @@ -2274,6 +2276,19 @@ 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) { + rmem = of_reserved_mem_lookup(rmem_node); + if (!rmem) { + err = -EINVAL; + goto fdev_error; + } + + qcom_scm_assign_mem(rmem->base, rmem->size, &data->perms, + data->vmperms, data->vmcount); + + } + secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain")); data->secure = secure_dsp;