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@xxxxxxxxxxxxxxxxx>
>> Reviewed-by: Caleb Connolly <caleb.connolly@xxxxxxxxxx>
>> ---
>> 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
--
// Caleb (they/them)
