On Mon, Sep 9, 2019 at 8:32 AM Andre Przywara [off-list ref] wrote:

On Fri, 30 Aug 2019 03:12:29 -0500
Jassi Brar [off-list ref] wrote:

Hi,

quoted

On Fri, Aug 30, 2019 at 3:07 AM Peng Fan [off-list ref] wrote:

quoted

quoted

Subject: Re: [PATCH v5 1/2] dt-bindings: mailbox: add binding doc for the ARM
SMC/HVC mailbox

On Fri, Aug 30, 2019 at 2:37 AM Peng Fan [off-list ref] wrote:

quoted

Hi Jassi,

quoted

Subject: Re: [PATCH v5 1/2] dt-bindings: mailbox: add binding doc
for the ARM SMC/HVC mailbox

On Fri, Aug 30, 2019 at 1:28 AM Peng Fan [off-list ref] wrote:

quoted

quoted

quoted

+examples:
+  - |
+    sram@910000 {
+      compatible = "mmio-sram";
+      reg = <0x0 0x93f000 0x0 0x1000>;
+      #address-cells = <1>;
+      #size-cells = <1>;
+      ranges = <0 0x0 0x93f000 0x1000>;
+
+      cpu_scp_lpri: scp-shmem@0 {
+        compatible = "arm,scmi-shmem";
+        reg = <0x0 0x200>;
+      };
+
+      cpu_scp_hpri: scp-shmem@200 {
+        compatible = "arm,scmi-shmem";
+        reg = <0x200 0x200>;
+      };
+    };
+
+    firmware {
+      smc_mbox: mailbox {
+        #mbox-cells = <1>;
+        compatible = "arm,smc-mbox";
+        method = "smc";
+        arm,num-chans = <0x2>;
+        transports = "mem";
+        /* Optional */
+        arm,func-ids = <0xc20000fe>, <0xc20000ff>;

SMC/HVC is synchronously(block) running in "secure mode", i.e,
there can only be one instance running platform wide. Right?

I think there could be channel for TEE, and channel for Linux.
For virtualization case, there could be dedicated channel for each VM.

I am talking from Linux pov. Functions 0xfe and 0xff above, can't
both be active at the same time, right?

If I get your point correctly,
On UP, both could not be active. On SMP, tx/rx could be both active,
anyway this depends on secure firmware and Linux firmware design.

Do you have any suggestions about arm,func-ids here?

I was thinking if this is just an instruction, why can't each channel be
represented as a controller, i.e, have exactly one func-id per controller node.
Define as many controllers as you need channels ?

I am ok, this could make driver code simpler. Something as below?

    smc_tx_mbox: tx_mbox {
      #mbox-cells = <0>;
      compatible = "arm,smc-mbox";
      method = "smc";
      transports = "mem";
      arm,func-id = <0xc20000fe>;
    };

    smc_rx_mbox: rx_mbox {
      #mbox-cells = <0>;
      compatible = "arm,smc-mbox";
      method = "smc";
      transports = "mem";
      arm,func-id = <0xc20000ff>;
    };

    firmware {
      scmi {
        compatible = "arm,scmi";
        mboxes = <&smc_tx_mbox>, <&smc_rx_mbox 1>;
        mbox-names = "tx", "rx";
        shmem = <&cpu_scp_lpri>, <&cpu_scp_hpri>;
      };
    };

Yes, the channel part is good.
But I am not convinced by the need to have SCMI specific "transport" mode.

Why would this be SCMI specific and what is the problem with having this property?
By the very nature of the SMC/HVC call you would expect to also pass parameters in registers.
However this limits the amount of data you can push, so the option of reverting to a
memory based payload sounds very reasonable.

Of course, it is very legit to pass data via mem and many platforms do
that. But as you note in your next post, the 'transport' doesn't seem
necessary doing what it does in the driver.

Cheers!

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel