linux-kernel - Re: [PATCH v2 5/9] arm64: dts: ti: k3-am62a7-sk: Enable ipc with remote processors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <469ee404-fe33-471e-8652-e9206f7f433b@ti.com>
Date: Mon, 3 Feb 2025 17:06:51 -0600
From: Judith Mendez <jm@...com>
To: Andrew Davis <afd@...com>, Nishanth Menon <nm@...com>,
        Vignesh Raghavendra
	<vigneshr@...com>
CC: Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, Hari Nagalla
	<hnagalla@...com>
Subject: Re: [PATCH v2 5/9] arm64: dts: ti: k3-am62a7-sk: Enable ipc with
 remote processors

Hi Andrew,

On 2/3/25 4:53 PM, Andrew Davis wrote:
> On 1/31/25 3:46 PM, Judith Mendez wrote:
>> From: Devarsh Thakkar <devarsht@...com>
>>
>> For each remote proc, reserve memory for IPC and bind the mailbox
>> assignments. Two memory regions are reserved for each remote processor.
>> The first region of 1MB of memory is used for Vring shared buffers
>> and the second region is used as external memory to the remote processor
>> for the resource table and for tracebuffer allocations.
>>
>> Signed-off-by: Devarsh Thakkar <devarsht@...com>
>> Signed-off-by: Hari Nagalla <hnagalla@...com>
>> Signed-off-by: Judith Mendez <jm@...com>
>> ---
>> Changes since v1:
>> - add patches 5-9 to enable ipc and complete device nodes
>> ---
>>   arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 78 +++++++++++++++++++++++++
>>   1 file changed, 78 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts 
>> b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> index a6f0d87a50d8a..6f4b037537fec 100644
>> --- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> +++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
>> @@ -61,11 +61,41 @@ secure_ddr: optee@...00000 {
>>               no-map;
>>           };
>> +        wkup_r5fss0_core0_dma_memory_region: r5f-dma-memory@...00000 {
>> +            compatible = "shared-dma-pool";
>> +            reg = <0x00 0x9c800000 0x00 0x100000>;
>> +            no-map;
>> +        };
>> +
>>           wkup_r5fss0_core0_memory_region: r5f-dma-memory@...00000 {
>>               compatible = "shared-dma-pool";
>>               reg = <0x00 0x9c900000 0x00 0x01e00000>;
>>               no-map;
>>           };
>> +
>> +        mcu_r5fss0_core0_dma_memory_region: r5f-dma-memory@...00000 {
>> +            compatible = "shared-dma-pool";
>> +            reg = <0x00 0x9b800000 0x00 0x100000>;
>> +            no-map;
>> +        };
>> +
>> +        mcu_r5fss0_core0_memory_region: r5f-dma-memory@...00000 {
>> +            compatible = "shared-dma-pool";
>> +            reg = <0x00 0x9b900000 0x00 0x0f00000>;
> 
> There is a leading 0 here and in a couple other places.
> 
> s/0x0f00000/0xf00000

Ok, will check all other memory carveouts and fix.

> 
>> +            no-map;
>> +        };
>> +
>> +        c7x_0_dma_memory_region: c7x-dma-memory@...00000 {
>> +            compatible = "shared-dma-pool";
>> +            reg = <0x00 0x99800000 0x00 0x100000>;
>> +            no-map;
>> +        };
>> +
>> +        c7x_0_memory_region: c7x-memory@...00000 {
>> +            compatible = "shared-dma-pool";
>> +            reg = <0x00 0x99900000 0x00 0x01f00000>;
> 
> 0x01f00000 ? What is this size, all other firmware in all other
> K3 use 0xf00000 (16MB), that should be enough for any sane firmware.
> If some firmware needs more it is probably doing something hacky
> like leaving space for buffer passing internally. That space should
> be provided from a shared dma-buf pool or similar mechanism.

That is fine, will change to 16MB. (:

~ Judith

> 
> Make this 0xf00000 and fix the firmware.
> 
> Andrew
> 
>> +            no-map;
>> +        };
>>       };
>>       opp-table {
>> @@ -737,3 +767,51 @@ dpi1_out: endpoint {
>>           };
>>       };
>>   };
>> +
>> +&mailbox0_cluster0 {
>> +    mbox_r5_0: mbox-r5-0 {
>> +        ti,mbox-rx = <0 0 0>;
>> +        ti,mbox-tx = <1 0 0>;
>> +    };
>> +};
>> +
>> +&mailbox0_cluster1 {
>> +    mbox_c7x_0: mbox-c7x-0 {
>> +        ti,mbox-rx = <0 0 0>;
>> +        ti,mbox-tx = <1 0 0>;
>> +    };
>> +};
>> +
>> +&mailbox0_cluster2 {
>> +    mbox_mcu_r5_0: mbox-mcu-r5-0 {
>> +        ti,mbox-rx = <0 0 0>;
>> +        ti,mbox-tx = <1 0 0>;
>> +    };
>> +};
>> +
>> +&wkup_r5fss0 {
>> +    status = "okay";
>> +};
>> +
>> +&wkup_r5fss0_core0 {
>> +    mboxes = <&mailbox0_cluster0>, <&mbox_r5_0>;
>> +    memory-region = <&wkup_r5fss0_core0_dma_memory_region>,
>> +        <&wkup_r5fss0_core0_memory_region>;
>> +};
>> +
>> +&mcu_r5fss0 {
>> +    status = "okay";
>> +};
>> +
>> +&mcu_r5fss0_core0 {
>> +    mboxes = <&mailbox0_cluster2>, <&mbox_mcu_r5_0>;
>> +    memory-region = <&mcu_r5fss0_core0_dma_memory_region>,
>> +            <&mcu_r5fss0_core0_memory_region>;
>> +};
>> +
>> +&c7x_0 {
>> +    mboxes = <&mailbox0_cluster1>, <&mbox_c7x_0>;
>> +    memory-region = <&c7x_0_dma_memory_region>,
>> +            <&c7x_0_memory_region>;
>> +    status = "okay";
>> +};