Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <e0365591-0f03-ab3d-61b4-c016541f2157@linaro.org>
Date:   Tue, 30 May 2023 11:52:18 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH v4 1/2] misc: fastrpc: support complete DMA pool access to
 the DSP
Content-Language: en-US
To:     Dylan Van Assche <me@dylanvanassche.be>, amahesh@qti.qualcomm.com,
        arnd@arndb.de, gregkh@linuxfoundation.org
Cc:     linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org,
        ~postmarketos/upstreaming@lists.sr.ht, phone-devel@vger.kernel.org,
        dan.carpenter@linaro.org,
        Caleb Connolly <caleb.connolly@linaro.org>,
        Ekansh Gupta <quic_ekangupt@quicinc.com>
References: <20230511141146.30465-1-me@dylanvanassche.be>
 <20230511141146.30465-2-me@dylanvanassche.be>
From:   Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
In-Reply-To: <20230511141146.30465-2-me@dylanvanassche.be>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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