On 4/25/2025 5:17 PM, Konrad Dybcio wrote:
> On 4/17/25 8:12 AM, Gokul Sriram Palanisamy wrote:
>> From: Vignesh Viswanathan <[email protected]>
>>
>> Add support to bring up hexagon based WCSS using secure PIL. All IPQxxxx
>> SoCs support secure Peripheral Image Loading (PIL).
>>
>> Secure PIL image is signed firmware image which only trusted software such
>> as TrustZone (TZ) can authenticate and load. Linux kernel will send a
>> Peripheral Authentication Service (PAS) request to TZ to authenticate and
>> load the PIL images. This change also introduces secure firmware
>> authentication using Trusted Management Engine-Lite (TME-L) which is
>> supported on IPQ5424 SoC. This driver uses mailbox based PAS request to
>> TME-L for image authentication if supported, else it will fallback to use
>> SCM call based PAS request to TZ.
>>
>> In order to avoid overloading the existing WCSS driver or PAS driver, we
>> came up with this new PAS based IPQ WCSS driver.
>>
>> Signed-off-by: Vignesh Viswanathan <[email protected]>
>> Signed-off-by: Manikanta Mylavarapu <[email protected]>
>> Signed-off-by: Gokul Sriram Palanisamy <[email protected]>
>> ---
> [...]
>
>> +static int wcss_sec_start(struct rproc *rproc)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + struct device *dev = wcss->dev;
>> + int ret;
>> +
>> + ret = qcom_q6v5_prepare(&wcss->q6);
>> + if (ret)
>> + return ret;
>> +
>> + if (!IS_ERR_OR_NULL(wcss->mbox_chan)) {
> You abort probe if wcss->mbox_chan returns an errno, please rework
> this to use if (use_tmelcom) or something
Hi Konrad,
do you mean to use 'use_tmelcom' variable from driver descriptor? If
yes, what if mbox_request_channel( ) failed?
or based on wcss->mbox_chan, should I set 'use_tmeeiihcckgddglcom' to
true or false and use it?
> [...]
>
>> +static void wcss_sec_copy_segment(struct rproc *rproc,
>> + struct rproc_dump_segment *segment,
>> + void *dest, size_t offset, size_t size)
>> +{
>> + struct wcss_sec *wcss = rproc->priv;
>> + struct device *dev = wcss->dev;
>> +
>> + if (!segment->io_ptr)
>> + segment->io_ptr = ioremap_wc(segment->da, segment->size);
>> +
>> + if (!segment->io_ptr) {
>> + dev_err(dev, "Failed to ioremap segment %pad size 0x%zx\n",
>> + &segment->da, segment->size);
>> + return;
>> + }
>> +
>> + if (offset + size <= segment->size) {
> I believe this allows an off-by-one (remove '=')
ok, will check and update.
> [...]
>
>> + memcpy(dest, segment->io_ptr + offset, size);
>> + } else {
>> + iounmap(segment->io_ptr);
>> + segment->io_ptr = NULL;
>> + }
>> +}
>> +
>> +static int wcss_sec_dump_segments(struct rproc *rproc,
>> + const struct firmware *fw)
>> +{
>> + struct device *dev = rproc->dev.parent;
>> + struct reserved_mem *rmem = NULL;
>> + struct device_node *node;
>> + int num_segs, index;
>> + int ret;
>> +
>> + /*
>> + * Parse thcitejlvhvdriihtheuvhn
>> hrough additional reserved memory regions for the rproc
>> + * and add them to the coredump segments
>> + */
>> + num_segs = of_count_phandle_with_args(dev->of_node,
>> + "memory-region", NULL);
>> + for (index = 0; index < num_segs; index++) {
>> + node = of_parse_phandle(dev->of_node,
>> + "memory-region", index);
> https://lore.kernel.org/linux-arm-msm/[email protected]/
ok, will implement of_reserved_mem_region_to_resource() and
of_reserved_mem_region_count().
>
> [...]
>
>> +static const struct wcss_data wcss_sec_ipq5424_res_init = {
>> + .pasid = MPD_WCSS_PAS_ID,
>> + .ss_name = "q6wcss",
>> + .tmelcom = true,
> "bool tmelcom" is very non-descriptive.. call it something like
> use_tmelcom, or maybe flip the condition and call it e.g.
> tz_managed
oeiihcckgddgcjnfeeuhjfdfbcfcdenvfnjnuceuntbir
eiihcckgddgcgltegldivttfeldnhugbnjvtllkkrujt
eiihcckgddgchbegrjtgfnk, will call it 'use_tmelcom'.
