[PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Peng Fan]

This is a modified version from Andre Przywara's patch series
https://lore.kernel.org/patchwork/cover/812997/.
[1] is a draft implementation of i.MX8MM SCMI ATF implementation that
use smc as mailbox, power/clk is included, but only part of clk has been
implemented to work with hardware, power domain only supports get name
for now.

The traditional Linux mailbox mechanism uses some kind of dedicated hardware
IP to signal a condition to some other processing unit, typically a dedicated
management processor.
This mailbox feature is used for instance by the SCMI protocol to signal a
request for some action to be taken by the management processor.
However some SoCs does not have a dedicated management core to provide
those services. In order to service TEE and to avoid linux shutdown
power and clock that used by TEE, need let firmware to handle power
and clock, the firmware here is ARM Trusted Firmware that could also
run SCMI service.

The existing SCMI implementation uses a rather flexible shared memory
region to communicate commands and their parameters, it still requires a
mailbox to actually trigger the action.

This patch series provides a Linux mailbox compatible service which uses
smc calls to invoke firmware code, for instance taking care of SCMI requests.
The actual requests are still communicated using the standard SCMI way of
shared memory regions, but a dedicated mailbox hardware IP can be replaced via
this new driver.

This simple driver uses the architected SMC calling convention to trigger
firmware services, also allows for using "HVC" calls to call into hypervisors
or firmware layers running in the EL2 exception level.

Patch 1 contains the device tree binding documentation, patch 2 introduces
the actual mailbox driver.

Please note that this driver just provides a generic mailbox mechanism,
though this is synchronous and one-way only (triggered by the OS only,
without providing an asynchronous way of triggering request from the
firmware).
And while providing SCMI services was the reason for this exercise, this
driver is in no way bound to this use case, but can be used generically
where the OS wants to signal a mailbox condition to firmware or a
hypervisor.
Also the driver is in no way meant to replace any existing firmware
interface, but actually to complement existing interfaces.

[1] https://github.com/MrVan/arm-trusted-firmware/tree/scmi

Peng Fan (2):
  DT: mailbox: add binding doc for the ARM SMC mailbox
  mailbox: introduce ARM SMC based mailbox

 .../devicetree/bindings/mailbox/arm-smc.txt        |  96 +++++++++++++
 drivers/mailbox/Kconfig                            |   7 +
 drivers/mailbox/Makefile                           |   2 +
 drivers/mailbox/arm-smc-mailbox.c                  | 154 +++++++++++++++++++++
 include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
 5 files changed, 269 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
 create mode 100644 drivers/mailbox/arm-smc-mailbox.c
 create mode 100644 include/linux/mailbox/arm-smc-mailbox.h

-- 
2.16.4

[PATCH 1/2] DT: mailbox: add binding doc for the ARM SMC mailbox

[Peng Fan]

The ARM SMC mailbox binding describes a firmware interface to trigger
actions in software layers running in the EL2 or EL3 exception levels.
The term "ARM" here relates to the SMC instruction as part of the ARM
instruction set, not as a standard endorsed by ARM Ltd.

Signed-off-by: Peng Fan <peng.fan@nxp.com>
---

V1:
arm,func-ids is still kept as an optional property, because there is no
defined SMC funciton id passed from SCMI. So in my test, I still use
arm,func-ids for ARM SIP service.

 .../devicetree/bindings/mailbox/arm-smc.txt        | 96 ++++++++++++++++++++++
 1 file changed, 96 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt

diff --git a/Documentation/devicetree/bindings/mailbox/arm-smc.txt b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
new file mode 100644
index 000000000000..12fc5997933d
--- /dev/null
+++ b/Documentation/devicetree/bindings/mailbox/arm-smc.txt
@@ -0,0 +1,96 @@
+ARM SMC Mailbox Interface
+=========================
+
+This mailbox uses the ARM smc (secure monitor call) instruction to
+trigger a mailbox-connected activity in firmware, executing on the very same
+core as the caller. By nature this operation is synchronous and this
+mailbox provides no way for asynchronous messages to be delivered the other
+way round, from firmware to the OS. However the value of r0/w0/x0 the firmware
+returns after the smc call is delivered as a received message to the
+mailbox framework, so a synchronous communication can be established.
+The exact meaning of both the action the mailbox triggers as well as the
+return value is defined by their users and is not subject to this binding.
+
+One use case of this mailbox is the SCMI interface, which uses shared memory
+to transfer commands and parameters, and a mailbox to trigger a function
+call. This allows SoCs without a separate management processor (or
+when such a processor is not available or used) to use this standardized
+interface anyway.
+
+This binding describes no hardware, but establishes a firmware interface.
+Upon receiving an SMC using one of the described SMC function identifiers,
+the firmware is expected to trigger some mailbox connected functionality.
+The communication follows the ARM SMC calling convention[1].
+Firmware expects an SMC function identifier in r0 or w0. The supported
+identifiers are passed from consumers, or listed in the the arm,func-ids
+properties as described below. The firmware can return one value in
+the first SMC result register, it is expected to be an error value,
+which shall be propagated to the mailbox client.
+
+Any core which supports the SMC or HVC instruction can be used, as long as
+a firmware component running in EL3 or EL2 is handling these calls.
+
+Mailbox Device Node:
+====================
+
+This node is expected to be a child of the /firmware node.
+
+Required properties:
+--------------------
+- compatible:		Shall be "arm,smc-mbox"
+- #mbox-cells		Shall be 1 - the index of the channel needed.
+- arm,num-chans		The number of channels supported.
+- method:		A string, either:
+			"hvc": if the driver shall use an HVC call, or
+			"smc": if the driver shall use an SMC call.
+
+Optional properties:
+- arm,func-ids		An array of 32-bit values specifying the function
+			IDs used by each mailbox channel. Those function IDs
+			follow the ARM SMC calling convention standard [1].
+			There is one identifier per channel and the number
+			of supported channels is determined by the length
+			of this array.
+
+Example:
+--------
+
+	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>;
+		};
+	};
+
+	smc_mbox: mailbox {
+		#mbox-cells = <1>;
+		compatible = "arm,smc-mbox";
+		method = "smc";
+		arm,num-chans = <0x2>;
+		/* Optional */
+		arm,func-ids = <0xc20000fe>, <0xc20000ff>;
+	};
+
+	firmware {
+		scmi {
+			compatible = "arm,scmi";
+			mboxes = <&mailbox 0 &mailbox 1>;
+			mbox-names = "tx", "rx";
+			shmem = <&cpu_scp_lpri &cpu_scp_hpri>;
+		};
+	};
+
+
+[1]
+http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.den0028a/index.html
-- 
2.16.4

[PATCH 2/2] mailbox: introduce ARM SMC based mailbox

[Peng Fan]

This mailbox driver implements a mailbox which signals transmitted data
via an ARM smc (secure monitor call) instruction. The mailbox receiver
is implemented in firmware and can synchronously return data when it
returns execution to the non-secure world again.
An asynchronous receive path is not implemented.
This allows the usage of a mailbox to trigger firmware actions on SoCs
which either don't have a separate management processor or on which such
a core is not available. A user of this mailbox could be the SCP
interface.

Modified from Andre Przywara's v2 patch
https://lore.kernel.org/patchwork/patch/812999/

Cc: Andre Przywara <andre.przywara@arm.com>
Signed-off-by: Peng Fan <peng.fan@nxp.com>
---
 drivers/mailbox/Kconfig                 |   7 ++
 drivers/mailbox/Makefile                |   2 +
 drivers/mailbox/arm-smc-mailbox.c       | 154 ++++++++++++++++++++++++++++++++
 include/linux/mailbox/arm-smc-mailbox.h |  10 +++
 4 files changed, 173 insertions(+)
 create mode 100644 drivers/mailbox/arm-smc-mailbox.c
 create mode 100644 include/linux/mailbox/arm-smc-mailbox.h

diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig
index 595542bfae85..c3bd0f1ddcd8 100644
--- a/drivers/mailbox/Kconfig
+++ b/drivers/mailbox/Kconfig
@@ -15,6 +15,13 @@ config ARM_MHU
 	  The controller has 3 mailbox channels, the last of which can be
 	  used in Secure mode only.
 
+config ARM_SMC_MBOX
+	tristate "Generic ARM smc mailbox"
+	depends on OF && HAVE_ARM_SMCCC
+	help
+	  Generic mailbox driver which uses ARM smc calls to call into
+	  firmware for triggering mailboxes.
+
 config IMX_MBOX
 	tristate "i.MX Mailbox"
 	depends on ARCH_MXC || COMPILE_TEST
diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile
index c22fad6f696b..93918a84c91b 100644
--- a/drivers/mailbox/Makefile
+++ b/drivers/mailbox/Makefile
@@ -7,6 +7,8 @@ obj-$(CONFIG_MAILBOX_TEST)	+= mailbox-test.o
 
 obj-$(CONFIG_ARM_MHU)	+= arm_mhu.o
 
+obj-$(CONFIG_ARM_SMC_MBOX)	+= arm-smc-mailbox.o
+
 obj-$(CONFIG_IMX_MBOX)	+= imx-mailbox.o
 
 obj-$(CONFIG_ARMADA_37XX_RWTM_MBOX)	+= armada-37xx-rwtm-mailbox.o
diff --git a/drivers/mailbox/arm-smc-mailbox.c b/drivers/mailbox/arm-smc-mailbox.c
new file mode 100644
index 000000000000..f4da1061f7f0
--- /dev/null
+++ b/drivers/mailbox/arm-smc-mailbox.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2016,2017 ARM Ltd.
+ * Copyright 2019 NXP
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/mailbox_controller.h>
+#include <linux/mailbox/arm-smc-mailbox.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+
+#define ARM_SMC_MBOX_USE_HVC	BIT(0)
+
+struct arm_smc_chan_data {
+	u32 function_id;
+	u32 flags;
+};
+
+static int arm_smc_send_data(struct mbox_chan *link, void *data)
+{
+	struct arm_smc_chan_data *chan_data = link->con_priv;
+	struct arm_smccc_mbox_cmd *cmd = data;
+	struct arm_smccc_res res;
+	u32 function_id;
+
+	if (chan_data->function_id != UINT_MAX)
+		function_id = chan_data->function_id;
+	else
+		function_id = cmd->a0;
+
+	if (chan_data->flags & ARM_SMC_MBOX_USE_HVC)
+		arm_smccc_hvc(function_id, cmd->a1, cmd->a2, cmd->a3, cmd->a4,
+			      cmd->a5, cmd->a6, cmd->a7, &res);
+	else
+		arm_smccc_smc(function_id, cmd->a1, cmd->a2, cmd->a3, cmd->a4,
+			      cmd->a5, cmd->a6, cmd->a7, &res);
+
+	mbox_chan_received_data(link, (void *)res.a0);
+
+	return 0;
+}
+
+static const struct mbox_chan_ops arm_smc_mbox_chan_ops = {
+	.send_data	= arm_smc_send_data,
+};
+
+static int arm_smc_mbox_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct mbox_controller *mbox;
+	struct arm_smc_chan_data *chan_data;
+	const char *method;
+	bool use_hvc = false;
+	int ret, i;
+	u32 val;
+
+	if (!of_property_read_u32(dev->of_node, "arm,num-chans", &val)) {
+		if (val < 1 || val > INT_MAX) {
+			dev_err(dev, "invalid arm,num-chans value %u of %pOFn\n", val, pdev->dev.of_node);
+			return -EINVAL;
+		}
+	}
+
+	if (!of_property_read_string(dev->of_node, "method", &method)) {
+		if (!strcmp("hvc", method)) {
+			use_hvc = true;
+		} else if (!strcmp("smc", method)) {
+			use_hvc = false;
+		} else {
+			dev_warn(dev, "invalid \"method\" property: %s\n",
+				 method);
+
+			return -EINVAL;
+		}
+	}
+
+	mbox = devm_kzalloc(dev, sizeof(*mbox), GFP_KERNEL);
+	if (!mbox)
+		return -ENOMEM;
+
+	mbox->num_chans = val;
+	mbox->chans = devm_kcalloc(dev, mbox->num_chans, sizeof(*mbox->chans),
+				   GFP_KERNEL);
+	if (!mbox->chans)
+		return -ENOMEM;
+
+	chan_data = devm_kcalloc(dev, mbox->num_chans, sizeof(*chan_data),
+				 GFP_KERNEL);
+	if (!chan_data)
+		return -ENOMEM;
+
+	for (i = 0; i < mbox->num_chans; i++) {
+		u32 function_id;
+
+		ret = of_property_read_u32_index(dev->of_node,
+						 "arm,func-ids", i,
+						 &function_id);
+		if (ret)
+			chan_data[i].function_id = UINT_MAX;
+
+		else
+			chan_data[i].function_id = function_id;
+
+		if (use_hvc)
+			chan_data[i].flags |= ARM_SMC_MBOX_USE_HVC;
+		mbox->chans[i].con_priv = &chan_data[i];
+	}
+
+	mbox->txdone_poll = false;
+	mbox->txdone_irq = false;
+	mbox->ops = &arm_smc_mbox_chan_ops;
+	mbox->dev = dev;
+
+	ret = mbox_controller_register(mbox);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, mbox);
+	dev_info(dev, "ARM SMC mailbox enabled with %d chan%s.\n",
+		 mbox->num_chans, mbox->num_chans == 1 ? "" : "s");
+
+	return ret;
+}
+
+static int arm_smc_mbox_remove(struct platform_device *pdev)
+{
+	struct mbox_controller *mbox = platform_get_drvdata(pdev);
+
+	mbox_controller_unregister(mbox);
+	return 0;
+}
+
+static const struct of_device_id arm_smc_mbox_of_match[] = {
+	{ .compatible = "arm,smc-mbox", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, arm_smc_mbox_of_match);
+
+static struct platform_driver arm_smc_mbox_driver = {
+	.driver = {
+		.name = "arm-smc-mbox",
+		.of_match_table = arm_smc_mbox_of_match,
+	},
+	.probe		= arm_smc_mbox_probe,
+	.remove		= arm_smc_mbox_remove,
+};
+module_platform_driver(arm_smc_mbox_driver);
+
+MODULE_AUTHOR("Andre Przywara <andre.przywara@arm.com>");
+MODULE_DESCRIPTION("Generic ARM smc mailbox driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mailbox/arm-smc-mailbox.h b/include/linux/mailbox/arm-smc-mailbox.h
new file mode 100644
index 000000000000..ca366fe491c3
--- /dev/null
+++ b/include/linux/mailbox/arm-smc-mailbox.h
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _LINUX_ARM_SMC_MAILBOX_H_
+#define _LINUX_ARM_SMC_MAILBOX_H_
+
+struct arm_smccc_mbox_cmd {
+	unsigned long a0, a1, a2, a3, a4, a5, a6, a7;
+};
+
+#endif /* _LINUX_ARM_SMC_MAILBOX_H_ */
-- 
2.16.4

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Florian Fainelli]

Hi,

On 5/22/19 10:50 PM, Peng Fan wrote:
> This is a modified version from Andre Przywara's patch series
> https://lore.kernel.org/patchwork/cover/812997/.
> [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
> use smc as mailbox, power/clk is included, but only part of clk has been
> implemented to work with hardware, power domain only supports get name
> for now.
> 
> The traditional Linux mailbox mechanism uses some kind of dedicated hardware
> IP to signal a condition to some other processing unit, typically a dedicated
> management processor.
> This mailbox feature is used for instance by the SCMI protocol to signal a
> request for some action to be taken by the management processor.
> However some SoCs does not have a dedicated management core to provide
> those services. In order to service TEE and to avoid linux shutdown
> power and clock that used by TEE, need let firmware to handle power
> and clock, the firmware here is ARM Trusted Firmware that could also
> run SCMI service.
> 
> The existing SCMI implementation uses a rather flexible shared memory
> region to communicate commands and their parameters, it still requires a
> mailbox to actually trigger the action.

We have had something similar done internally with a couple of minor
differences:

- a SGI is used to send SCMI notifications/delayed replies to support
asynchronism (patches are in the works to actually add that to the Linux
SCMI framework). There is no good support for SGI in the kernel right
now so we hacked up something from the existing SMP code and adding the
ability to register our own IPI handlers (SHAME!). Using a PPI should
work and should allow for using request_irq() AFAICT.

- the mailbox identifier is indicated as part of the SMC call such that
we can have multiple SCMI mailboxes serving both standard protocols and
non-standard (in the 0x80 and above) range, also they may have different
throughput (in hindsight, these could simply be different channels)

Your patch series looks both good and useful to me, I would just put a
provision in the binding to support an optional interrupt such that
asynchronism gets reasonably easy to plug in when it is available (and
desirable).

> 
> This patch series provides a Linux mailbox compatible service which uses
> smc calls to invoke firmware code, for instance taking care of SCMI requests.
> The actual requests are still communicated using the standard SCMI way of
> shared memory regions, but a dedicated mailbox hardware IP can be replaced via
> this new driver.
> 
> This simple driver uses the architected SMC calling convention to trigger
> firmware services, also allows for using "HVC" calls to call into hypervisors
> or firmware layers running in the EL2 exception level.
> 
> Patch 1 contains the device tree binding documentation, patch 2 introduces
> the actual mailbox driver.
> 
> Please note that this driver just provides a generic mailbox mechanism,
> though this is synchronous and one-way only (triggered by the OS only,
> without providing an asynchronous way of triggering request from the
> firmware).
> And while providing SCMI services was the reason for this exercise, this
> driver is in no way bound to this use case, but can be used generically
> where the OS wants to signal a mailbox condition to firmware or a
> hypervisor.
> Also the driver is in no way meant to replace any existing firmware
> interface, but actually to complement existing interfaces.
> 
> [1] https://github.com/MrVan/arm-trusted-firmware/tree/scmi
> 
> Peng Fan (2):
>   DT: mailbox: add binding doc for the ARM SMC mailbox
>   mailbox: introduce ARM SMC based mailbox
> 
>  .../devicetree/bindings/mailbox/arm-smc.txt        |  96 +++++++++++++
>  drivers/mailbox/Kconfig                            |   7 +
>  drivers/mailbox/Makefile                           |   2 +
>  drivers/mailbox/arm-smc-mailbox.c                  | 154 +++++++++++++++++++++
>  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
>  5 files changed, 269 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mailbox/arm-smc.txt
>  create mode 100644 drivers/mailbox/arm-smc-mailbox.c
>  create mode 100644 include/linux/mailbox/arm-smc-mailbox.h
> 


-- 
Florian

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Sudeep Holla]

On Thu, May 23, 2019 at 10:30:50AM -0700, Florian Fainelli wrote:
> Hi,
>
> On 5/22/19 10:50 PM, Peng Fan wrote:
> > This is a modified version from Andre Przywara's patch series
> > https://lore.kernel.org/patchwork/cover/812997/.
> > [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
> > use smc as mailbox, power/clk is included, but only part of clk has been
> > implemented to work with hardware, power domain only supports get name
> > for now.
> >
> > The traditional Linux mailbox mechanism uses some kind of dedicated hardware
> > IP to signal a condition to some other processing unit, typically a dedicated
> > management processor.
> > This mailbox feature is used for instance by the SCMI protocol to signal a
> > request for some action to be taken by the management processor.
> > However some SoCs does not have a dedicated management core to provide
> > those services. In order to service TEE and to avoid linux shutdown
> > power and clock that used by TEE, need let firmware to handle power
> > and clock, the firmware here is ARM Trusted Firmware that could also
> > run SCMI service.
> >
> > The existing SCMI implementation uses a rather flexible shared memory
> > region to communicate commands and their parameters, it still requires a
> > mailbox to actually trigger the action.
>
> We have had something similar done internally with a couple of minor
> differences:
>
> - a SGI is used to send SCMI notifications/delayed replies to support
> asynchronism (patches are in the works to actually add that to the Linux
> SCMI framework). There is no good support for SGI in the kernel right
> now so we hacked up something from the existing SMP code and adding the
> ability to register our own IPI handlers (SHAME!). Using a PPI should
> work and should allow for using request_irq() AFAICT.
>

We have been thinking this since we were asked if SMC can be transport.
Generally out of 16 SGIs, 8 are reserved for secure side and non-secure
has 8. Of these 8, IIUC 7 is already being used by kernel. So unless we
manage to get the last one reserved exclusive to SCMI, it makes it
difficult to add SGI support in SCMI.

We have been telling partners/vendors about this limitation if they
use SMC as transport and need to have dedicated h/w interrupt for the
notifications.

Another issue could be with virtualisation(using HVC) and EL handling
so called SCMI SGI. We need to think about those too. I will try to get
more info on this and come back on this.

--
Regards,
Sudeep

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[André Przywara]

On 24/05/2019 18:56, Sudeep Holla wrote:
> On Thu, May 23, 2019 at 10:30:50AM -0700, Florian Fainelli wrote:

Hi,

>> On 5/22/19 10:50 PM, Peng Fan wrote:
>>> This is a modified version from Andre Przywara's patch series
>>> https://lore.kernel.org/patchwork/cover/812997/.
>>> [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
>>> use smc as mailbox, power/clk is included, but only part of clk has been
>>> implemented to work with hardware, power domain only supports get name
>>> for now.
>>>
>>> The traditional Linux mailbox mechanism uses some kind of dedicated hardware
>>> IP to signal a condition to some other processing unit, typically a dedicated
>>> management processor.
>>> This mailbox feature is used for instance by the SCMI protocol to signal a
>>> request for some action to be taken by the management processor.
>>> However some SoCs does not have a dedicated management core to provide
>>> those services. In order to service TEE and to avoid linux shutdown
>>> power and clock that used by TEE, need let firmware to handle power
>>> and clock, the firmware here is ARM Trusted Firmware that could also
>>> run SCMI service.
>>>
>>> The existing SCMI implementation uses a rather flexible shared memory
>>> region to communicate commands and their parameters, it still requires a
>>> mailbox to actually trigger the action.
>>
>> We have had something similar done internally with a couple of minor
>> differences:
>>
>> - a SGI is used to send SCMI notifications/delayed replies to support
>> asynchronism (patches are in the works to actually add that to the Linux
>> SCMI framework). There is no good support for SGI in the kernel right
>> now so we hacked up something from the existing SMP code and adding the
>> ability to register our own IPI handlers (SHAME!). Using a PPI should
>> work and should allow for using request_irq() AFAICT.
>>
> 
> We have been thinking this since we were asked if SMC can be transport.
> Generally out of 16 SGIs, 8 are reserved for secure side and non-secure
> has 8. Of these 8, IIUC 7 is already being used by kernel. So unless we
> manage to get the last one reserved exclusive to SCMI, it makes it
> difficult to add SGI support in SCMI.
> 
> We have been telling partners/vendors about this limitation if they
> use SMC as transport and need to have dedicated h/w interrupt for the
> notifications.
> 
> Another issue could be with virtualisation(using HVC) and EL handling
> so called SCMI SGI. We need to think about those too. I will try to get
> more info on this and come back on this.

I think regardless of the *current* feasibility of using SGIs in *Linux*
we should at least specify an "interrupts" property in the binding, to
allow for future usage. We might copy the pmuv3 way [1] of allowing to
specify multiple SPI interrupts as well, to give more flexibility.
After all an implementation could offload the asynchronous notification
to a separate core, and that could use SPIs, for instance.

Cheers,
Andre.

[1] Documentation/devicetree/bindings/arm/pmu.yaml:45

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Jassi Brar]

On Thu, May 23, 2019 at 12:50 AM Peng Fan <peng.fan@nxp.com> wrote:
>
> This is a modified version from Andre Przywara's patch series
> https://lore.kernel.org/patchwork/cover/812997/.
>
Can you please specify exact modifications on top of Andre's last
submission? As in "Changes since v1: ...."

Thanks.

RE: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Peng Fan]

Hi Jassi,

> Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox
> 
> On Thu, May 23, 2019 at 12:50 AM Peng Fan <peng.fan@nxp.com> wrote:
> >
> > This is a modified version from Andre Przywara's patch series
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&amp;data=02%7C01%7Cpe
> ng.fan%40nxp.com%7C40bb1632f99248f24e1308d6e2467c8b%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C1%7C636945189705620796&amp;sdat
> a=v6mLyDKLO8NaBMlX6XlOCtOs2C41TuOG9yFDY%2F7q8nU%3D&amp;reser
> ved=0.
> >
> Can you please specify exact modifications on top of Andre's last submission?
> As in "Changes since v1: ...."

Since Andre's last v2 version was sent in 2017, I thought no need to add that.
The changes are mostly.
Introduce arm,num-chans
Introduce arm_smccc_mbox_cmd
txdone_poll and txdone_irq are both set to false
arm,func-ids are kept, but as an optional property.
Rewords SCPI to SCMI, because I am trying SCMI over SMC, not SCPI.

I'll add the changes information after I collect more comments in this patchset.

Thanks,
Peng.
> 
> Thanks.

RE: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Peng Fan]

Hi Florian,

> Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox
> 
> Hi,
> 
> On 5/22/19 10:50 PM, Peng Fan wrote:
> > This is a modified version from Andre Przywara's patch series
> >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&data=02%7C01%7Cpe
> ng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&sdat
> a=BbS5ZQtzMANSwaKRDJ62NKrPrAyaED1%2BvymQaT6Qr8E%3D&rese
> rved=0.
> > [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
> > use smc as mailbox, power/clk is included, but only part of clk has
> > been implemented to work with hardware, power domain only supports get
> > name for now.
> >
> > The traditional Linux mailbox mechanism uses some kind of dedicated
> > hardware IP to signal a condition to some other processing unit,
> > typically a dedicated management processor.
> > This mailbox feature is used for instance by the SCMI protocol to
> > signal a request for some action to be taken by the management processor.
> > However some SoCs does not have a dedicated management core to
> provide
> > those services. In order to service TEE and to avoid linux shutdown
> > power and clock that used by TEE, need let firmware to handle power
> > and clock, the firmware here is ARM Trusted Firmware that could also
> > run SCMI service.
> >
> > The existing SCMI implementation uses a rather flexible shared memory
> > region to communicate commands and their parameters, it still requires
> > a mailbox to actually trigger the action.
> 
> We have had something similar done internally with a couple of minor
> differences:
> 
> - a SGI is used to send SCMI notifications/delayed replies to support
> asynchronism (patches are in the works to actually add that to the Linux SCMI
> framework). There is no good support for SGI in the kernel right now so we
> hacked up something from the existing SMP code and adding the ability to
> register our own IPI handlers (SHAME!). Using a PPI should work and should
> allow for using request_irq() AFAICT.

So you are also implementing a firmware inside ATF for SCMI usecase, right?

Introducing SGI in ATF to notify Linux will introduce complexity, there is
no good framework inside ATF for SCMI, and I use synchronization call for
simplicity for now.

> 
> - the mailbox identifier is indicated as part of the SMC call such that we can
> have multiple SCMI mailboxes serving both standard protocols and
> non-standard (in the 0x80 and above) range, also they may have different
> throughput (in hindsight, these could simply be different channels)
> 
> Your patch series looks both good and useful to me, I would just put a
> provision in the binding to support an optional interrupt such that
> asynchronism gets reasonably easy to plug in when it is available (and
> desirable).

Ok. Let me think about and add that in new version patch.

Thanks,
Peng.

> 
> >
> > This patch series provides a Linux mailbox compatible service which
> > uses smc calls to invoke firmware code, for instance taking care of SCMI
> requests.
> > The actual requests are still communicated using the standard SCMI way
> > of shared memory regions, but a dedicated mailbox hardware IP can be
> > replaced via this new driver.
> >
> > This simple driver uses the architected SMC calling convention to
> > trigger firmware services, also allows for using "HVC" calls to call
> > into hypervisors or firmware layers running in the EL2 exception level.
> >
> > Patch 1 contains the device tree binding documentation, patch 2
> > introduces the actual mailbox driver.
> >
> > Please note that this driver just provides a generic mailbox
> > mechanism, though this is synchronous and one-way only (triggered by
> > the OS only, without providing an asynchronous way of triggering
> > request from the firmware).
> > And while providing SCMI services was the reason for this exercise,
> > this driver is in no way bound to this use case, but can be used
> > generically where the OS wants to signal a mailbox condition to
> > firmware or a hypervisor.
> > Also the driver is in no way meant to replace any existing firmware
> > interface, but actually to complement existing interfaces.
> >
> > [1]
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> >
> ub.com%2FMrVan%2Farm-trusted-firmware%2Ftree%2Fscmi&data=02
> %7C01%7
> >
> Cpeng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1
> d3bc2b4
> >
> c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&sdata=kN
> 9bEFFcsZA
> > 1ePeNLLfHmONpVaG6O5ajVQvKMuaBXyk%3D&reserved=0
> >
> > Peng Fan (2):
> >   DT: mailbox: add binding doc for the ARM SMC mailbox
> >   mailbox: introduce ARM SMC based mailbox
> >
> >  .../devicetree/bindings/mailbox/arm-smc.txt        |  96
> +++++++++++++
> >  drivers/mailbox/Kconfig                            |   7 +
> >  drivers/mailbox/Makefile                           |   2 +
> >  drivers/mailbox/arm-smc-mailbox.c                  | 154
> +++++++++++++++++++++
> >  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
> >  5 files changed, 269 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> >  create mode 100644 drivers/mailbox/arm-smc-mailbox.c  create mode
> > 100644 include/linux/mailbox/arm-smc-mailbox.h
> >
> 
> 
> --
> Florian

RE: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Peng Fan]

Hi Sudeep,

> Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox
> 
> On Thu, May 23, 2019 at 10:30:50AM -0700, Florian Fainelli wrote:
> > Hi,
> >
> > On 5/22/19 10:50 PM, Peng Fan wrote:
> > > This is a modified version from Andre Przywara's patch series
> > >
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&data=02%7C01%7Cpe
> ng.fan%40nxp.com%7Cfa2ba15f479b49eb219f08d6e0713ea3%7C686ea1d3b
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636943174355592142&sdat
> a=Y94rnxDEoMm9nyRyjJrYBqRduc5XkvvQhno7zfIQ%2B04%3D&reserve
> d=0.
> > > [1] is a draft implementation of i.MX8MM SCMI ATF implementation
> > > that use smc as mailbox, power/clk is included, but only part of clk
> > > has been implemented to work with hardware, power domain only
> > > supports get name for now.
> > >
> > > The traditional Linux mailbox mechanism uses some kind of dedicated
> > > hardware IP to signal a condition to some other processing unit,
> > > typically a dedicated management processor.
> > > This mailbox feature is used for instance by the SCMI protocol to
> > > signal a request for some action to be taken by the management
> processor.
> > > However some SoCs does not have a dedicated management core to
> > > provide those services. In order to service TEE and to avoid linux
> > > shutdown power and clock that used by TEE, need let firmware to
> > > handle power and clock, the firmware here is ARM Trusted Firmware
> > > that could also run SCMI service.
> > >
> > > The existing SCMI implementation uses a rather flexible shared
> > > memory region to communicate commands and their parameters, it still
> > > requires a mailbox to actually trigger the action.
> >
> > We have had something similar done internally with a couple of minor
> > differences:
> >
> > - a SGI is used to send SCMI notifications/delayed replies to support
> > asynchronism (patches are in the works to actually add that to the
> > Linux SCMI framework). There is no good support for SGI in the kernel
> > right now so we hacked up something from the existing SMP code and
> > adding the ability to register our own IPI handlers (SHAME!). Using a
> > PPI should work and should allow for using request_irq() AFAICT.
> >
> 
> We have been thinking this since we were asked if SMC can be transport.
> Generally out of 16 SGIs, 8 are reserved for secure side and non-secure has 8.
> Of these 8, IIUC 7 is already being used by kernel. So unless we manage to get
> the last one reserved exclusive to SCMI, it makes it difficult to add SGI support
> in SCMI.
> 
> We have been telling partners/vendors about this limitation if they use SMC
> as transport and need to have dedicated h/w interrupt for the notifications.

This is an option, and we could add optional property.

> 
> Another issue could be with virtualisation(using HVC) and EL handling so called
> SCMI SGI. We need to think about those too. 

Using dedicated HW for virtualization that support vcpu scheduling might be
not a good choice, do you mean this? Dedicated pin vcpu to pcpu should be fine,
just like jailhouse hypervisor.

I will try to get more info on this
> and come back on this.

Looking forward.

Thanks,
Peng.

> 
> --
> Regards,
> Sudeep

RE: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Peng Fan]

Hi Andre,

> Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox
> 
> On 24/05/2019 18:56, Sudeep Holla wrote:
> > On Thu, May 23, 2019 at 10:30:50AM -0700, Florian Fainelli wrote:
> 
> Hi,
> 
> >> On 5/22/19 10:50 PM, Peng Fan wrote:
> >>> This is a modified version from Andre Przywara's patch series
> >>>
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&data=02%7C01%7Cpe
> ng.fan%40nxp.com%7C02ee9487370c4eb9158008d6e2363ca0%7C686ea1d3
> bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636945119959534576&sda
> ta=U8FzX3FX2PoEZhRuRMhFbkaAnb3cqjZsb9%2FTdt8OfuY%3D&reserve
> d=0.
> >>> [1] is a draft implementation of i.MX8MM SCMI ATF implementation
> >>> that use smc as mailbox, power/clk is included, but only part of clk
> >>> has been implemented to work with hardware, power domain only
> >>> supports get name for now.
> >>>
> >>> The traditional Linux mailbox mechanism uses some kind of dedicated
> >>> hardware IP to signal a condition to some other processing unit,
> >>> typically a dedicated management processor.
> >>> This mailbox feature is used for instance by the SCMI protocol to
> >>> signal a request for some action to be taken by the management
> processor.
> >>> However some SoCs does not have a dedicated management core to
> >>> provide those services. In order to service TEE and to avoid linux
> >>> shutdown power and clock that used by TEE, need let firmware to
> >>> handle power and clock, the firmware here is ARM Trusted Firmware
> >>> that could also run SCMI service.
> >>>
> >>> The existing SCMI implementation uses a rather flexible shared
> >>> memory region to communicate commands and their parameters, it still
> >>> requires a mailbox to actually trigger the action.
> >>
> >> We have had something similar done internally with a couple of minor
> >> differences:
> >>
> >> - a SGI is used to send SCMI notifications/delayed replies to support
> >> asynchronism (patches are in the works to actually add that to the
> >> Linux SCMI framework). There is no good support for SGI in the kernel
> >> right now so we hacked up something from the existing SMP code and
> >> adding the ability to register our own IPI handlers (SHAME!). Using a
> >> PPI should work and should allow for using request_irq() AFAICT.
> >>
> >
> > We have been thinking this since we were asked if SMC can be transport.
> > Generally out of 16 SGIs, 8 are reserved for secure side and
> > non-secure has 8. Of these 8, IIUC 7 is already being used by kernel.
> > So unless we manage to get the last one reserved exclusive to SCMI, it
> > makes it difficult to add SGI support in SCMI.
> >
> > We have been telling partners/vendors about this limitation if they
> > use SMC as transport and need to have dedicated h/w interrupt for the
> > notifications.
> >
> > Another issue could be with virtualisation(using HVC) and EL handling
> > so called SCMI SGI. We need to think about those too. I will try to
> > get more info on this and come back on this.
> 
> I think regardless of the *current* feasibility of using SGIs in *Linux* we
> should at least specify an "interrupts" property in the binding, to allow for
> future usage. We might copy the pmuv3 way [1] of allowing to specify
> multiple SPI interrupts as well, to give more flexibility.

This needs to go with an optional property, agree?
That means smc mailbox needs to support synchronous and asynchronous
communication. I'll try to add that and write some porotype code to
verify.

Thanks,
Peng.

> After all an implementation could offload the asynchronous notification to a
> separate core, and that could use SPIs, for instance.
> 
> Cheers,
> Andre.
> 
> [1] Documentation/devicetree/bindings/arm/pmu.yaml:45

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Florian Fainelli]

On 5/26/2019 10:19 PM, Peng Fan wrote:
> Hi Florian,
> 
>> Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox
>>
>> Hi,
>>
>> On 5/22/19 10:50 PM, Peng Fan wrote:
>>> This is a modified version from Andre Przywara's patch series
>>>
>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
>> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&data=02%7C01%7Cpe
>> ng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1d3b
>> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&sdat
>> a=BbS5ZQtzMANSwaKRDJ62NKrPrAyaED1%2BvymQaT6Qr8E%3D&rese
>> rved=0.
>>> [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
>>> use smc as mailbox, power/clk is included, but only part of clk has
>>> been implemented to work with hardware, power domain only supports get
>>> name for now.
>>>
>>> The traditional Linux mailbox mechanism uses some kind of dedicated
>>> hardware IP to signal a condition to some other processing unit,
>>> typically a dedicated management processor.
>>> This mailbox feature is used for instance by the SCMI protocol to
>>> signal a request for some action to be taken by the management processor.
>>> However some SoCs does not have a dedicated management core to
>> provide
>>> those services. In order to service TEE and to avoid linux shutdown
>>> power and clock that used by TEE, need let firmware to handle power
>>> and clock, the firmware here is ARM Trusted Firmware that could also
>>> run SCMI service.
>>>
>>> The existing SCMI implementation uses a rather flexible shared memory
>>> region to communicate commands and their parameters, it still requires
>>> a mailbox to actually trigger the action.
>>
>> We have had something similar done internally with a couple of minor
>> differences:
>>
>> - a SGI is used to send SCMI notifications/delayed replies to support
>> asynchronism (patches are in the works to actually add that to the Linux SCMI
>> framework). There is no good support for SGI in the kernel right now so we
>> hacked up something from the existing SMP code and adding the ability to
>> register our own IPI handlers (SHAME!). Using a PPI should work and should
>> allow for using request_irq() AFAICT.
> 
> So you are also implementing a firmware inside ATF for SCMI usecase, right?

Correct, SCMI is implemented in part within the trusted firmware (it is
not ATF, something custom), and in part using an external processor for
specific tasks.

> 
> Introducing SGI in ATF to notify Linux will introduce complexity, there is
> no good framework inside ATF for SCMI, and I use synchronization call for
> simplicity for now.

Sure that's fine, the point we all seem to agree upon is that putting
provision in the binding document for an optional interrupt is already
known to be desirable.

> 
>>
>> - the mailbox identifier is indicated as part of the SMC call such that we can
>> have multiple SCMI mailboxes serving both standard protocols and
>> non-standard (in the 0x80 and above) range, also they may have different
>> throughput (in hindsight, these could simply be different channels)
>>
>> Your patch series looks both good and useful to me, I would just put a
>> provision in the binding to support an optional interrupt such that
>> asynchronism gets reasonably easy to plug in when it is available (and
>> desirable).
> 
> Ok. Let me think about and add that in new version patch.
> 
> Thanks,
> Peng.
> 
>>
>>>
>>> This patch series provides a Linux mailbox compatible service which
>>> uses smc calls to invoke firmware code, for instance taking care of SCMI
>> requests.
>>> The actual requests are still communicated using the standard SCMI way
>>> of shared memory regions, but a dedicated mailbox hardware IP can be
>>> replaced via this new driver.
>>>
>>> This simple driver uses the architected SMC calling convention to
>>> trigger firmware services, also allows for using "HVC" calls to call
>>> into hypervisors or firmware layers running in the EL2 exception level.
>>>
>>> Patch 1 contains the device tree binding documentation, patch 2
>>> introduces the actual mailbox driver.
>>>
>>> Please note that this driver just provides a generic mailbox
>>> mechanism, though this is synchronous and one-way only (triggered by
>>> the OS only, without providing an asynchronous way of triggering
>>> request from the firmware).
>>> And while providing SCMI services was the reason for this exercise,
>>> this driver is in no way bound to this use case, but can be used
>>> generically where the OS wants to signal a mailbox condition to
>>> firmware or a hypervisor.
>>> Also the driver is in no way meant to replace any existing firmware
>>> interface, but actually to complement existing interfaces.
>>>
>>> [1]
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
>>>
>> ub.com%2FMrVan%2Farm-trusted-firmware%2Ftree%2Fscmi&data=02
>> %7C01%7
>>>
>> Cpeng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1
>> d3bc2b4
>>>
>> c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&sdata=kN
>> 9bEFFcsZA
>>> 1ePeNLLfHmONpVaG6O5ajVQvKMuaBXyk%3D&reserved=0
>>>
>>> Peng Fan (2):
>>>   DT: mailbox: add binding doc for the ARM SMC mailbox
>>>   mailbox: introduce ARM SMC based mailbox
>>>
>>>  .../devicetree/bindings/mailbox/arm-smc.txt        |  96
>> +++++++++++++
>>>  drivers/mailbox/Kconfig                            |   7 +
>>>  drivers/mailbox/Makefile                           |   2 +
>>>  drivers/mailbox/arm-smc-mailbox.c                  | 154
>> +++++++++++++++++++++
>>>  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
>>>  5 files changed, 269 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/mailbox/arm-smc.txt
>>>  create mode 100644 drivers/mailbox/arm-smc-mailbox.c  create mode
>>> 100644 include/linux/mailbox/arm-smc-mailbox.h
>>>
>>
>>
>> --
>> Florian

-- 
Florian

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Andre Przywara]

On Mon, 27 May 2019 05:19:41 +0000
Peng Fan <peng.fan@nxp.com> wrote:

Hi,

> > Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox
> > 
> > Hi,
> > 
> > On 5/22/19 10:50 PM, Peng Fan wrote:  
> > > This is a modified version from Andre Przywara's patch series
> > >  
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.ke
> > rnel.org%2Fpatchwork%2Fcover%2F812997%2F&amp;data=02%7C01%7Cpe
> > ng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1d3b
> > c2b4c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&amp;sdat
> > a=BbS5ZQtzMANSwaKRDJ62NKrPrAyaED1%2BvymQaT6Qr8E%3D&amp;rese
> > rved=0.  
> > > [1] is a draft implementation of i.MX8MM SCMI ATF implementation that
> > > use smc as mailbox, power/clk is included, but only part of clk has
> > > been implemented to work with hardware, power domain only supports get
> > > name for now.
> > >
> > > The traditional Linux mailbox mechanism uses some kind of dedicated
> > > hardware IP to signal a condition to some other processing unit,
> > > typically a dedicated management processor.
> > > This mailbox feature is used for instance by the SCMI protocol to
> > > signal a request for some action to be taken by the management processor.
> > > However some SoCs does not have a dedicated management core to  
> > provide  
> > > those services. In order to service TEE and to avoid linux shutdown
> > > power and clock that used by TEE, need let firmware to handle power
> > > and clock, the firmware here is ARM Trusted Firmware that could also
> > > run SCMI service.
> > >
> > > The existing SCMI implementation uses a rather flexible shared memory
> > > region to communicate commands and their parameters, it still requires
> > > a mailbox to actually trigger the action.  
> > 
> > We have had something similar done internally with a couple of minor
> > differences:
> > 
> > - a SGI is used to send SCMI notifications/delayed replies to support
> > asynchronism (patches are in the works to actually add that to the Linux SCMI
> > framework). There is no good support for SGI in the kernel right now so we
> > hacked up something from the existing SMP code and adding the ability to
> > register our own IPI handlers (SHAME!). Using a PPI should work and should
> > allow for using request_irq() AFAICT.  
> 
> So you are also implementing a firmware inside ATF for SCMI usecase, right?
> 
> Introducing SGI in ATF to notify Linux will introduce complexity, there is
> no good framework inside ATF for SCMI, and I use synchronization call for
> simplicity for now.

I think we don't disagree, but just to clarify on one thing:

I think we should avoid tying this driver to specific protocol or software on the other end, be it ATF or SCMI. After all it's just a mailbox driver, meant to signal some event (and parameters) to some external entity. Yes, SCMI (or SCPI back then) was the reason to push for this, but it should be independent from that. I am not even sure we should mention it too much in the documentation.

So whether the receiving end is ATF or something else it irrelevant, I think. For instance we have had discussions in Xen to provide guests some virtualised device management support, and using an HVC mailbox seems like a neat solution. This could be using the SCMI (or SCPI) protocol, but that's not a requirement. In this case the Xen hypervisor would be the one to pick up the mailbox trigger, probably forwarding the request to something else (Dom0 in this case).
Also having a generic SMC mailbox could avoid having the actual hardware mailbox drivers in the kernel, so EL3 firmware could forward the request to an external management processor, and Linux would just work, without the need to describe the actual hardware mailbox device in some firmware tables. This might help ACPI on those devices.

Cheers,
Andre.

> > 
> > - the mailbox identifier is indicated as part of the SMC call such that we can
> > have multiple SCMI mailboxes serving both standard protocols and
> > non-standard (in the 0x80 and above) range, also they may have different
> > throughput (in hindsight, these could simply be different channels)
> > 
> > Your patch series looks both good and useful to me, I would just put a
> > provision in the binding to support an optional interrupt such that
> > asynchronism gets reasonably easy to plug in when it is available (and
> > desirable).  
> 
> Ok. Let me think about and add that in new version patch.
> 
> Thanks,
> Peng.
> 
> >   
> > >
> > > This patch series provides a Linux mailbox compatible service which
> > > uses smc calls to invoke firmware code, for instance taking care of SCMI  
> > requests.  
> > > The actual requests are still communicated using the standard SCMI way
> > > of shared memory regions, but a dedicated mailbox hardware IP can be
> > > replaced via this new driver.
> > >
> > > This simple driver uses the architected SMC calling convention to
> > > trigger firmware services, also allows for using "HVC" calls to call
> > > into hypervisors or firmware layers running in the EL2 exception level.
> > >
> > > Patch 1 contains the device tree binding documentation, patch 2
> > > introduces the actual mailbox driver.
> > >
> > > Please note that this driver just provides a generic mailbox
> > > mechanism, though this is synchronous and one-way only (triggered by
> > > the OS only, without providing an asynchronous way of triggering
> > > request from the firmware).
> > > And while providing SCMI services was the reason for this exercise,
> > > this driver is in no way bound to this use case, but can be used
> > > generically where the OS wants to signal a mailbox condition to
> > > firmware or a hypervisor.
> > > Also the driver is in no way meant to replace any existing firmware
> > > interface, but actually to complement existing interfaces.
> > >
> > > [1]
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > >  
> > ub.com%2FMrVan%2Farm-trusted-firmware%2Ftree%2Fscmi&amp;data=02
> > %7C01%7  
> > >  
> > Cpeng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1
> > d3bc2b4  
> > >  
> > c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&amp;sdata=kN
> > 9bEFFcsZA  
> > > 1ePeNLLfHmONpVaG6O5ajVQvKMuaBXyk%3D&amp;reserved=0
> > >
> > > Peng Fan (2):
> > >   DT: mailbox: add binding doc for the ARM SMC mailbox
> > >   mailbox: introduce ARM SMC based mailbox
> > >
> > >  .../devicetree/bindings/mailbox/arm-smc.txt        |  96  
> > +++++++++++++  
> > >  drivers/mailbox/Kconfig                            |   7 +
> > >  drivers/mailbox/Makefile                           |   2 +
> > >  drivers/mailbox/arm-smc-mailbox.c                  | 154  
> > +++++++++++++++++++++  
> > >  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
> > >  5 files changed, 269 insertions(+)
> > >  create mode 100644
> > > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > >  create mode 100644 drivers/mailbox/arm-smc-mailbox.c  create mode
> > > 100644 include/linux/mailbox/arm-smc-mailbox.h
> > >  
> > 
> > 
> > --
> > Florian  

RE: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Peng Fan]

> 
> > > Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered
> > > mailbox
> > >
> > > Hi,
> > >
> > > On 5/22/19 10:50 PM, Peng Fan wrote:
> > > > This is a modified version from Andre Przywara's patch series
> > > >
> > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > re.ke
> rnel.org%2Fpatchwork%2Fcover%2F812997%2F&data=02%7C01%7Cpe
> > >
> ng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1d3b
> > >
> c2b4c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&sdat
> > >
> a=BbS5ZQtzMANSwaKRDJ62NKrPrAyaED1%2BvymQaT6Qr8E%3D&rese
> > > rved=0.
> > > > [1] is a draft implementation of i.MX8MM SCMI ATF implementation
> > > > that use smc as mailbox, power/clk is included, but only part of
> > > > clk has been implemented to work with hardware, power domain only
> > > > supports get name for now.
> > > >
> > > > The traditional Linux mailbox mechanism uses some kind of
> > > > dedicated hardware IP to signal a condition to some other
> > > > processing unit, typically a dedicated management processor.
> > > > This mailbox feature is used for instance by the SCMI protocol to
> > > > signal a request for some action to be taken by the management
> processor.
> > > > However some SoCs does not have a dedicated management core to
> > > provide
> > > > those services. In order to service TEE and to avoid linux
> > > > shutdown power and clock that used by TEE, need let firmware to
> > > > handle power and clock, the firmware here is ARM Trusted Firmware
> > > > that could also run SCMI service.
> > > >
> > > > The existing SCMI implementation uses a rather flexible shared
> > > > memory region to communicate commands and their parameters, it
> > > > still requires a mailbox to actually trigger the action.
> > >
> > > We have had something similar done internally with a couple of minor
> > > differences:
> > >
> > > - a SGI is used to send SCMI notifications/delayed replies to
> > > support asynchronism (patches are in the works to actually add that
> > > to the Linux SCMI framework). There is no good support for SGI in
> > > the kernel right now so we hacked up something from the existing SMP
> > > code and adding the ability to register our own IPI handlers
> > > (SHAME!). Using a PPI should work and should allow for using request_irq()
> AFAICT.
> >
> > So you are also implementing a firmware inside ATF for SCMI usecase, right?
> >
> > Introducing SGI in ATF to notify Linux will introduce complexity,
> > there is no good framework inside ATF for SCMI, and I use
> > synchronization call for simplicity for now.
> 
> I think we don't disagree, but just to clarify on one thing:
> 
> I think we should avoid tying this driver to specific protocol or software on the
> other end, be it ATF or SCMI. After all it's just a mailbox driver, meant to signal
> some event (and parameters) to some external entity. Yes, SCMI (or SCPI back
> then) was the reason to push for this, but it should be independent from that.

Thanks, I agree.

> I am not even sure we should mention it too much in the documentation.

I think we need a usecase here, so it should be fine.

> 
> So whether the receiving end is ATF or something else it irrelevant, I think. For
> instance we have had discussions in Xen to provide guests some virtualised
> device management support, and using an HVC mailbox seems like a neat
> solution. This could be using the SCMI (or SCPI) protocol, but that's not a
> requirement. In this case the Xen hypervisor would be the one to pick up the
> mailbox trigger, probably forwarding the request to something else (Dom0 in
> this case).

I do not get the point "forwarding the request", DomU HVC will trap to Xen,
so how to forward to Dom0?

Thanks,
Peng.

> Also having a generic SMC mailbox could avoid having the actual hardware
> mailbox drivers in the kernel, so EL3 firmware could forward the request to an
> external management processor, and Linux would just work, without the need
> to describe the actual hardware mailbox device in some firmware tables. This
> might help ACPI on those devices.
> 
> Cheers,
> Andre.
> 
> > >
> > > - the mailbox identifier is indicated as part of the SMC call such
> > > that we can have multiple SCMI mailboxes serving both standard
> > > protocols and non-standard (in the 0x80 and above) range, also they
> > > may have different throughput (in hindsight, these could simply be
> > > different channels)
> > >
> > > Your patch series looks both good and useful to me, I would just put
> > > a provision in the binding to support an optional interrupt such
> > > that asynchronism gets reasonably easy to plug in when it is
> > > available (and desirable).
> >
> > Ok. Let me think about and add that in new version patch.
> >
> > Thanks,
> > Peng.
> >
> > >
> > > >
> > > > This patch series provides a Linux mailbox compatible service
> > > > which uses smc calls to invoke firmware code, for instance taking
> > > > care of SCMI
> > > requests.
> > > > The actual requests are still communicated using the standard SCMI
> > > > way of shared memory regions, but a dedicated mailbox hardware IP
> > > > can be replaced via this new driver.
> > > >
> > > > This simple driver uses the architected SMC calling convention to
> > > > trigger firmware services, also allows for using "HVC" calls to
> > > > call into hypervisors or firmware layers running in the EL2 exception
> level.
> > > >
> > > > Patch 1 contains the device tree binding documentation, patch 2
> > > > introduces the actual mailbox driver.
> > > >
> > > > Please note that this driver just provides a generic mailbox
> > > > mechanism, though this is synchronous and one-way only (triggered
> > > > by the OS only, without providing an asynchronous way of
> > > > triggering request from the firmware).
> > > > And while providing SCMI services was the reason for this
> > > > exercise, this driver is in no way bound to this use case, but can
> > > > be used generically where the OS wants to signal a mailbox
> > > > condition to firmware or a hypervisor.
> > > > Also the driver is in no way meant to replace any existing
> > > > firmware interface, but actually to complement existing interfaces.
> > > >
> > > > [1]
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > gith
> > > >
> > >
> ub.com%2FMrVan%2Farm-trusted-firmware%2Ftree%2Fscmi&data=02
> > > %7C01%7
> > > >
> > >
> Cpeng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1
> > > d3bc2b4
> > > >
> > >
> c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&sdata=kN
> > > 9bEFFcsZA
> > > > 1ePeNLLfHmONpVaG6O5ajVQvKMuaBXyk%3D&reserved=0
> > > >
> > > > Peng Fan (2):
> > > >   DT: mailbox: add binding doc for the ARM SMC mailbox
> > > >   mailbox: introduce ARM SMC based mailbox
> > > >
> > > >  .../devicetree/bindings/mailbox/arm-smc.txt        |  96
> > > +++++++++++++
> > > >  drivers/mailbox/Kconfig                            |   7 +
> > > >  drivers/mailbox/Makefile                           |   2 +
> > > >  drivers/mailbox/arm-smc-mailbox.c                  | 154
> > > +++++++++++++++++++++
> > > >  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
> > > >  5 files changed, 269 insertions(+)  create mode 100644
> > > > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > >  create mode 100644 drivers/mailbox/arm-smc-mailbox.c  create
> mode
> > > > 100644 include/linux/mailbox/arm-smc-mailbox.h
> > > >
> > >
> > >
> > > --
> > > Florian

Re: [PATCH 0/2] mailbox: arm: introduce smc triggered mailbox

[Andre Przywara]

On Fri, 31 May 2019 01:39:45 +0000
Peng Fan <peng.fan@nxp.com> wrote:

Hi,

(CC:ing Julien for Xen)

> > > > Subject: Re: [PATCH 0/2] mailbox: arm: introduce smc triggered
> > > > mailbox
> > > >
> > > > Hi,
> > > >
> > > > On 5/22/19 10:50 PM, Peng Fan wrote:  
> > > > > This is a modified version from Andre Przywara's patch series
> > > > >  
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flo
> > > > re.ke  
> > rnel.org%2Fpatchwork%2Fcover%2F812997%2F&amp;data=02%7C01%7Cpe  
> > > >  
> > ng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1d3b  
> > > >  
> > c2b4c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&amp;sdat  
> > > >  
> > a=BbS5ZQtzMANSwaKRDJ62NKrPrAyaED1%2BvymQaT6Qr8E%3D&amp;rese  
> > > > rved=0.  
> > > > > [1] is a draft implementation of i.MX8MM SCMI ATF implementation
> > > > > that use smc as mailbox, power/clk is included, but only part of
> > > > > clk has been implemented to work with hardware, power domain only
> > > > > supports get name for now.
> > > > >
> > > > > The traditional Linux mailbox mechanism uses some kind of
> > > > > dedicated hardware IP to signal a condition to some other
> > > > > processing unit, typically a dedicated management processor.
> > > > > This mailbox feature is used for instance by the SCMI protocol to
> > > > > signal a request for some action to be taken by the management  
> > processor.  
> > > > > However some SoCs does not have a dedicated management core to  
> > > > provide  
> > > > > those services. In order to service TEE and to avoid linux
> > > > > shutdown power and clock that used by TEE, need let firmware to
> > > > > handle power and clock, the firmware here is ARM Trusted Firmware
> > > > > that could also run SCMI service.
> > > > >
> > > > > The existing SCMI implementation uses a rather flexible shared
> > > > > memory region to communicate commands and their parameters, it
> > > > > still requires a mailbox to actually trigger the action.  
> > > >
> > > > We have had something similar done internally with a couple of minor
> > > > differences:
> > > >
> > > > - a SGI is used to send SCMI notifications/delayed replies to
> > > > support asynchronism (patches are in the works to actually add that
> > > > to the Linux SCMI framework). There is no good support for SGI in
> > > > the kernel right now so we hacked up something from the existing SMP
> > > > code and adding the ability to register our own IPI handlers
> > > > (SHAME!). Using a PPI should work and should allow for using request_irq()  
> > AFAICT.  
> > >
> > > So you are also implementing a firmware inside ATF for SCMI usecase, right?
> > >
> > > Introducing SGI in ATF to notify Linux will introduce complexity,
> > > there is no good framework inside ATF for SCMI, and I use
> > > synchronization call for simplicity for now.  
> > 
> > I think we don't disagree, but just to clarify on one thing:
> > 
> > I think we should avoid tying this driver to specific protocol or software on the
> > other end, be it ATF or SCMI. After all it's just a mailbox driver, meant to signal
> > some event (and parameters) to some external entity. Yes, SCMI (or SCPI back
> > then) was the reason to push for this, but it should be independent from that.  
> 
> Thanks, I agree.
> 
> > I am not even sure we should mention it too much in the documentation.  
> 
> I think we need a usecase here, so it should be fine.
> 
> > 
> > So whether the receiving end is ATF or something else it irrelevant, I think. For
> > instance we have had discussions in Xen to provide guests some virtualised
> > device management support, and using an HVC mailbox seems like a neat
> > solution. This could be using the SCMI (or SCPI) protocol, but that's not a
> > requirement. In this case the Xen hypervisor would be the one to pick up the
> > mailbox trigger, probably forwarding the request to something else (Dom0 in
> > this case).  
> 
> I do not get the point "forwarding the request", DomU HVC will trap to Xen,
> so how to forward to Dom0?

I don't think there is something easily available in Xen/ARM at the moment, but we could either use something like an event channel, or utilise some planned extension to allow Dom0 to register on MMIO traps.
My point was that most likely Dom0 would be in control of the resources handled by the interface (clocks, regulators), so Xen wouldn't know how to deal with those requests.

Cheers,
Andre.


> 
> Thanks,
> Peng.
> 
> > Also having a generic SMC mailbox could avoid having the actual hardware
> > mailbox drivers in the kernel, so EL3 firmware could forward the request to an
> > external management processor, and Linux would just work, without the need
> > to describe the actual hardware mailbox device in some firmware tables. This
> > might help ACPI on those devices.
> > 
> > Cheers,
> > Andre.
> >   
> > > >
> > > > - the mailbox identifier is indicated as part of the SMC call such
> > > > that we can have multiple SCMI mailboxes serving both standard
> > > > protocols and non-standard (in the 0x80 and above) range, also they
> > > > may have different throughput (in hindsight, these could simply be
> > > > different channels)
> > > >
> > > > Your patch series looks both good and useful to me, I would just put
> > > > a provision in the binding to support an optional interrupt such
> > > > that asynchronism gets reasonably easy to plug in when it is
> > > > available (and desirable).  
> > >
> > > Ok. Let me think about and add that in new version patch.
> > >
> > > Thanks,
> > > Peng.
> > >  
> > > >  
> > > > >
> > > > > This patch series provides a Linux mailbox compatible service
> > > > > which uses smc calls to invoke firmware code, for instance taking
> > > > > care of SCMI  
> > > > requests.  
> > > > > The actual requests are still communicated using the standard SCMI
> > > > > way of shared memory regions, but a dedicated mailbox hardware IP
> > > > > can be replaced via this new driver.
> > > > >
> > > > > This simple driver uses the architected SMC calling convention to
> > > > > trigger firmware services, also allows for using "HVC" calls to
> > > > > call into hypervisors or firmware layers running in the EL2 exception  
> > level.  
> > > > >
> > > > > Patch 1 contains the device tree binding documentation, patch 2
> > > > > introduces the actual mailbox driver.
> > > > >
> > > > > Please note that this driver just provides a generic mailbox
> > > > > mechanism, though this is synchronous and one-way only (triggered
> > > > > by the OS only, without providing an asynchronous way of
> > > > > triggering request from the firmware).
> > > > > And while providing SCMI services was the reason for this
> > > > > exercise, this driver is in no way bound to this use case, but can
> > > > > be used generically where the OS wants to signal a mailbox
> > > > > condition to firmware or a hypervisor.
> > > > > Also the driver is in no way meant to replace any existing
> > > > > firmware interface, but actually to complement existing interfaces.
> > > > >
> > > > > [1]
> > > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
> > > > > gith
> > > > >  
> > > >  
> > ub.com%2FMrVan%2Farm-trusted-firmware%2Ftree%2Fscmi&amp;data=02  
> > > > %7C01%7  
> > > > >  
> > > >  
> > Cpeng.fan%40nxp.com%7C010c9ddd5df645c9c66b08d6dfa46cb2%7C686ea1  
> > > > d3bc2b4  
> > > > >  
> > > >  
> > c6fa92cd99c5c301635%7C0%7C0%7C636942294631442665&amp;sdata=kN  
> > > > 9bEFFcsZA  
> > > > > 1ePeNLLfHmONpVaG6O5ajVQvKMuaBXyk%3D&amp;reserved=0
> > > > >
> > > > > Peng Fan (2):
> > > > >   DT: mailbox: add binding doc for the ARM SMC mailbox
> > > > >   mailbox: introduce ARM SMC based mailbox
> > > > >
> > > > >  .../devicetree/bindings/mailbox/arm-smc.txt        |  96  
> > > > +++++++++++++  
> > > > >  drivers/mailbox/Kconfig                            |   7 +
> > > > >  drivers/mailbox/Makefile                           |   2 +
> > > > >  drivers/mailbox/arm-smc-mailbox.c                  | 154  
> > > > +++++++++++++++++++++  
> > > > >  include/linux/mailbox/arm-smc-mailbox.h            |  10 ++
> > > > >  5 files changed, 269 insertions(+)  create mode 100644
> > > > > Documentation/devicetree/bindings/mailbox/arm-smc.txt
> > > > >  create mode 100644 drivers/mailbox/arm-smc-mailbox.c  create  
> > mode  
> > > > > 100644 include/linux/mailbox/arm-smc-mailbox.h
> > > > >  
> > > >
> > > >
> > > > --
> > > > Florian  
>