This change introduce a new transport channel for Qualcomm virtual
platforms. The transport is mechanically similar to ARM_SCMI_TRANSPORT_SMC.
The difference between the two transports is that a parameter is passed in
the hypervisor call to identify which doorbell to assert. This parameter is
dynamically generated at runtime on the device and insuitable to pass via
the devicetree.
The function ID and parameter are stored by firmware in the shmem region.
This has been tested on ARM64 virtual Qualcomm platform.
Nikunj Kela (2):
dt-bindings: arm: Add qcom specific hvc transport for SCMI
firmware: arm_scmi: Add qcom hvc/shmem transport
.../bindings/firmware/arm,scmi.yaml | 69 +++++
drivers/firmware/arm_scmi/Kconfig | 13 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/common.h | 3 +
drivers/firmware/arm_scmi/driver.c | 4 +
drivers/firmware/arm_scmi/qcom_hvc.c | 241 ++++++++++++++++++
6 files changed, 331 insertions(+)
create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
--
2.17.1
This change introduce a new transport channel for Qualcomm virtual
platforms. The transport is mechanically similar to ARM_SCMI_TRANSPORT_SMC.
The difference between the two transports is that a parameter is passed in
the hypervisor call to identify which doorbell to assert. This parameter is
dynamically generated at runtime on the device and insuitable to pass via
the devicetree.
The function ID and parameter are stored by firmware in the shmem region.
This has been tested on ARM64 virtual Qualcomm platform.
---
v2 -> use allOf construct in dtb schema,
remove wrappers from mutexes,
use architecture independent channel layout
v1 -> original patches
Nikunj Kela (3):
dt-bindings: arm: convert nested if-else construct to allOf
dt-bindings: arm: Add qcom specific hvc transport for SCMI
firmware: arm_scmi: Add qcom hvc/shmem transport
.../bindings/firmware/arm,scmi.yaml | 67 +++---
drivers/firmware/arm_scmi/Kconfig | 13 +
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/common.h | 3 +
drivers/firmware/arm_scmi/driver.c | 4 +
drivers/firmware/arm_scmi/qcom_hvc.c | 224 ++++++++++++++++++
6 files changed, 284 insertions(+), 28 deletions(-)
create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
--
2.17.1
Introduce compatible "qcom,scmi-hvc-shmem" for SCMI
transport channel for Qualcomm virtual platforms.
The compatible mandates a shared memory channel.
Signed-off-by: Nikunj Kela <[email protected]>
---
.../devicetree/bindings/firmware/arm,scmi.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 8d54ea768d38..4090240f45b1 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -45,6 +45,9 @@ properties:
- description: SCMI compliant firmware with OP-TEE transport
items:
- const: linaro,scmi-optee
+ - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
+ items:
+ - const: qcom,scmi-hvc-shmem
interrupts:
description:
@@ -320,6 +323,15 @@ allOf:
required:
- linaro,optee-channel-id
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,scmi-hvc-shmem
+ then:
+ required:
+ - shmem
+
examples:
- |
firmware {
--
2.17.1
Nested if-else construct is not scalable therefore, convert
it to allOf:if-else.
Signed-off-by: Nikunj Kela <[email protected]>
---
.../bindings/firmware/arm,scmi.yaml | 55 +++++++++----------
1 file changed, 27 insertions(+), 28 deletions(-)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index b138f3d23df8..8d54ea768d38 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -284,35 +284,34 @@ $defs:
required:
- compatible
-if:
- properties:
- compatible:
- contains:
- const: arm,scmi
-then:
- properties:
- interrupts: false
- interrupt-names: false
-
- required:
- - mboxes
- - shmem
-
-else:
- if:
- properties:
- compatible:
- contains:
- enum:
- - arm,scmi-smc
- - arm,scmi-smc-param
- then:
- required:
- - arm,smc-id
- - shmem
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: arm,scmi
+ then:
+ properties:
+ interrupts: false
+ interrupt-names: false
+
+ required:
+ - mboxes
+ - shmem
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - arm,scmi-smc
+ - arm,scmi-smc-param
+ then:
+ required:
+ - arm,smc-id
+ - shmem
- else:
- if:
+ - if:
properties:
compatible:
contains:
--
2.17.1
Add a new transport channel to the SCMI firmware interface driver for
SCMI message exchange on Qualcomm virtual platforms.
The hypervisor associates an object-id also known as capability-id
with each hvc doorbell object. The capability-id is used to identify the
doorbell from the VM's capability namespace, similar to a file-descriptor.
The hypervisor, in addition to the function-id, expects the capability-id
to be passed in x1 register when HVC call is invoked.
The qcom hvc doorbell/shared memory transport uses a statically defined
shared memory region that binds with "arm,scmi-shmem" device tree node.
The function-id & capability-id are allocated by the hypervisor on bootup
and are stored in the shmem region by the firmware before starting Linux.
Currently, there is no usecase for the atomic support therefore this driver
doesn't include the changes for the same.
Signed-off-by: Nikunj Kela <[email protected]>
---
drivers/firmware/arm_scmi/Kconfig | 13 ++
drivers/firmware/arm_scmi/Makefile | 1 +
drivers/firmware/arm_scmi/common.h | 3 +
drivers/firmware/arm_scmi/driver.c | 4 +
drivers/firmware/arm_scmi/qcom_hvc.c | 224 +++++++++++++++++++++++++++
5 files changed, 245 insertions(+)
create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index ea0f5083ac47..40d07329ebf7 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -99,6 +99,19 @@ config ARM_SCMI_TRANSPORT_OPTEE
If you want the ARM SCMI PROTOCOL stack to include support for a
transport based on OP-TEE SCMI service, answer Y.
+config ARM_SCMI_TRANSPORT_QCOM_HVC
+ bool "SCMI transport based on hvc doorbell & shmem for Qualcomm SoCs"
+ depends on ARCH_QCOM
+ select ARM_SCMI_HAVE_TRANSPORT
+ select ARM_SCMI_HAVE_SHMEM
+ default y
+ help
+ Enable hvc doorbell & shmem based transport for SCMI.
+
+ If you want the ARM SCMI PROTOCOL stack to include support for a
+ hvc doorbell and shmem transport on Qualcomm virtual platforms,
+ answer Y.
+
config ARM_SCMI_TRANSPORT_SMC
bool "SCMI transport based on SMC"
depends on HAVE_ARM_SMCCC_DISCOVERY
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index b31d78fa66cc..ba1ff5893ec0 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -10,6 +10,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
+scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_QCOM_HVC) += qcom_hvc.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c46dc5215af7..5c98cbb1278b 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -298,6 +298,9 @@ extern const struct scmi_desc scmi_virtio_desc;
#ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
extern const struct scmi_desc scmi_optee_desc;
#endif
+#ifdef CONFIG_ARM_SCMI_TRANSPORT_QCOM_HVC
+extern const struct scmi_desc scmi_qcom_hvc_desc;
+#endif
void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index b5957cc12fee..c54519596c29 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2918,6 +2918,10 @@ static const struct of_device_id scmi_of_match[] = {
#endif
#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
+#endif
+#ifdef CONFIG_ARM_SCMI_TRANSPORT_QCOM_HVC
+ { .compatible = "qcom,scmi-hvc-shmem",
+ .data = &scmi_qcom_hvc_desc },
#endif
{ /* Sentinel */ },
};
diff --git a/drivers/firmware/arm_scmi/qcom_hvc.c b/drivers/firmware/arm_scmi/qcom_hvc.c
new file mode 100644
index 000000000000..9aa60d6bb797
--- /dev/null
+++ b/drivers/firmware/arm_scmi/qcom_hvc.c
@@ -0,0 +1,224 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Message
+ * Qualcomm HVC/shmem Transport driver
+ *
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright 2020 NXP
+ *
+ * This is based on drivers/firmware/arm_scmi/smc.c
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+#include "common.h"
+
+/**
+ * struct scmi_qcom_hvc - Structure representing a SCMI qcom hvc transport
+ *
+ * @irq: An optional IRQ for completion
+ * @cinfo: SCMI channel info
+ * @shmem: Transmit/Receive shared memory area
+ * @shmem_lock: Lock to protect access to Tx/Rx shared memory area.
+ * @func_id: hvc call function-id
+ * @cap_id: hvc doorbell's capability id
+ */
+
+struct scmi_qcom_hvc {
+ int irq;
+ struct scmi_chan_info *cinfo;
+ struct scmi_shared_mem __iomem *shmem;
+ /* Protect access to shmem area */
+ struct mutex shmem_lock;
+ u32 func_id;
+ u64 cap_id;
+};
+
+static irqreturn_t qcom_hvc_msg_done_isr(int irq, void *data)
+{
+ struct scmi_qcom_hvc *scmi_info = data;
+
+ scmi_rx_callback(scmi_info->cinfo, shmem_read_header(scmi_info->shmem), NULL);
+
+ return IRQ_HANDLED;
+}
+
+static bool qcom_hvc_chan_available(struct device_node *of_node, int idx)
+{
+ struct device_node *np = of_parse_phandle(of_node, "shmem", 0);
+
+ if (!np)
+ return false;
+
+ of_node_put(np);
+ return true;
+}
+
+static int qcom_hvc_chan_setup(struct scmi_chan_info *cinfo,
+ struct device *dev, bool tx)
+{
+ struct device *cdev = cinfo->dev;
+ struct scmi_qcom_hvc *scmi_info;
+ struct device_node *np;
+ resource_size_t size;
+ struct resource res;
+ u32 func_id;
+ u64 cap_id;
+ int ret;
+
+ if (!tx)
+ return -ENODEV;
+
+ scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL);
+ if (!scmi_info)
+ return -ENOMEM;
+
+ np = of_parse_phandle(cdev->of_node, "shmem", 0);
+ if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
+ of_node_put(np);
+ return -ENXIO;
+ }
+
+ ret = of_address_to_resource(np, 0, &res);
+ of_node_put(np);
+ if (ret) {
+ dev_err(cdev, "failed to get SCMI Tx shared memory\n");
+ return ret;
+ }
+
+ size = resource_size(&res);
+
+ /* The func-id & capability-id are kept in last 16 bytes of shmem.
+ * +-------+
+ * | |
+ * | shmem |
+ * | |
+ * | |
+ * +-------+ <-- (size - 16)
+ * | funcId|
+ * +-------+ <-- (size - 8)
+ * | capId |
+ * +-------+ <-- size
+ */
+
+ scmi_info->shmem = devm_ioremap(dev, res.start, size);
+ if (!scmi_info->shmem) {
+ dev_err(dev, "failed to ioremap SCMI Tx shared memory\n");
+ return -EADDRNOTAVAIL;
+ }
+
+ func_id = readl((void __iomem *)(scmi_info->shmem) + size - 16);
+
+#ifdef CONFIG_ARM64
+ cap_id = readq((void __iomem *)(scmi_info->shmem) + size - 8);
+#else
+ /* capability-id is 32 bit long on 32bit machines */
+ cap_id = readl((void __iomem *)(scmi_info->shmem) + size - 8);
+#endif
+
+ /*
+ * If there is an interrupt named "a2p", then the service and
+ * completion of a message is signaled by an interrupt rather than by
+ * the return of the hvc call.
+ */
+ scmi_info->irq = of_irq_get_byname(cdev->of_node, "a2p");
+ if (scmi_info->irq > 0) {
+ ret = request_irq(scmi_info->irq, qcom_hvc_msg_done_isr,
+ IRQF_NO_SUSPEND, dev_name(dev), scmi_info);
+ if (ret) {
+ dev_err(dev, "failed to setup SCMI completion irq\n");
+ return ret;
+ }
+ } else {
+ cinfo->no_completion_irq = true;
+ }
+
+ scmi_info->func_id = func_id;
+ scmi_info->cap_id = cap_id;
+ scmi_info->cinfo = cinfo;
+ mutex_init(&scmi_info->shmem_lock);
+ cinfo->transport_info = scmi_info;
+
+ return 0;
+}
+
+static int qcom_hvc_chan_free(int id, void *p, void *data)
+{
+ struct scmi_chan_info *cinfo = p;
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ /* Ignore any possible further reception on the IRQ path */
+ if (scmi_info->irq > 0)
+ free_irq(scmi_info->irq, scmi_info);
+
+ cinfo->transport_info = NULL;
+ scmi_info->cinfo = NULL;
+
+ return 0;
+}
+
+static int qcom_hvc_send_message(struct scmi_chan_info *cinfo,
+ struct scmi_xfer *xfer)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+ struct arm_smccc_res res;
+
+ /*
+ * Channel will be released only once response has been
+ * surely fully retrieved, so after .mark_txdone()
+ */
+ mutex_lock(&scmi_info->shmem_lock);
+
+ shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
+
+ arm_smccc_1_1_hvc(scmi_info->func_id, (unsigned long)scmi_info->cap_id,
+ 0, 0, 0, 0, 0, 0, &res);
+
+ if (res.a0) {
+ mutex_unlock(&scmi_info->shmem_lock);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static void qcom_hvc_fetch_response(struct scmi_chan_info *cinfo,
+ struct scmi_xfer *xfer)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ shmem_fetch_response(scmi_info->shmem, xfer);
+}
+
+static void qcom_hvc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
+ struct scmi_xfer *__unused)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ mutex_unlock(&scmi_info->shmem_lock);
+}
+
+static const struct scmi_transport_ops scmi_qcom_hvc_ops = {
+ .chan_available = qcom_hvc_chan_available,
+ .chan_setup = qcom_hvc_chan_setup,
+ .chan_free = qcom_hvc_chan_free,
+ .send_message = qcom_hvc_send_message,
+ .mark_txdone = qcom_hvc_mark_txdone,
+ .fetch_response = qcom_hvc_fetch_response,
+};
+
+const struct scmi_desc scmi_qcom_hvc_desc = {
+ .ops = &scmi_qcom_hvc_ops,
+ .max_rx_timeout_ms = 30,
+ .max_msg = 20,
+ .max_msg_size = 128,
+ .sync_cmds_completed_on_ret = true,
+};
--
2.17.1
On 24/07/2023 18:44, Nikunj Kela wrote:
> Introduce compatible "qcom,scmi-hvc-shmem" for SCMI
> transport channel for Qualcomm virtual platforms.
> The compatible mandates a shared memory channel.
>
> Signed-off-by: Nikunj Kela <[email protected]>
> ---
Acked-by: Krzysztof Kozlowski <[email protected]>
This looks fine for me, but I did not investigate whether Sudeep's
concerns were solved/answered fully.
Best regards,
Krzysztof
On 24/07/2023 18:44, Nikunj Kela wrote:
> Nested if-else construct is not scalable therefore, convert
> it to allOf:if-else.
>
> Signed-off-by: Nikunj Kela <[email protected]>
> ---
> .../bindings/firmware/arm,scmi.yaml | 55 +++++++++----------
> 1 file changed, 27 insertions(+), 28 deletions(-)
>
Suggested-by: Krzysztof Kozlowski <[email protected]>
Reviewed-by: Krzysztof Kozlowski <[email protected]>
Best regards,
Krzysztof
On Mon, Jul 24, 2023 at 09:44:19AM -0700, Nikunj Kela wrote:
> Add a new transport channel to the SCMI firmware interface driver for
> SCMI message exchange on Qualcomm virtual platforms.
>
> The hypervisor associates an object-id also known as capability-id
> with each hvc doorbell object. The capability-id is used to identify the
> doorbell from the VM's capability namespace, similar to a file-descriptor.
>
> The hypervisor, in addition to the function-id, expects the capability-id
> to be passed in x1 register when HVC call is invoked.
>
> The qcom hvc doorbell/shared memory transport uses a statically defined
> shared memory region that binds with "arm,scmi-shmem" device tree node.
>
> The function-id & capability-id are allocated by the hypervisor on bootup
> and are stored in the shmem region by the firmware before starting Linux.
>
> Currently, there is no usecase for the atomic support therefore this driver
> doesn't include the changes for the same.
>
Hi Nikunj,
so basically this new SCMI transport that you are introducing is just
exactly like the existing SMC transport with the only difference that
you introduced even another new way to configure func_id, a new cap_id
param AND the fact that you use HVC instead of SMC... all of this tied
to a new compatible to identify this new transport mechanism....
..but all in all is just a lot of plain duplicated code to maintain...
...why can't you fit this other smc/hvc transport variant into the
existing SMC transport by properly picking and configuring func_id/cap_id
and "doorbell" method (SMC vs HVC) in the chan_setup() step ?
..I mean ... you can decide where to pick your params based on
compatibles and also you can setup your invokation method (SMC vs HVC)
based on those...while keeping all the other stuff exactly the same...
...including support for atomic exchanges...if not, when you'll need that
too in your QC_HVC transport you'll have to duplicate also that (and my
bugs too probably :P)
(... well maybe in this scenario also the transport itself should be
renamed from SMC to something more general...)
Not sure if I am missing something, or if Sudeep will be horrified by
this unifying proposal of mine, but in this series as it stands now I
just see a lot of brutally duplicated stuff that just differs by naming
and a very minimal change in logic that could be addressed changing and
generalizing the original SMC transport code instead.
Thanks,
Cristian
On 7/25/2023 10:03 AM, Cristian Marussi wrote:
> On Mon, Jul 24, 2023 at 09:44:19AM -0700, Nikunj Kela wrote:
>> Add a new transport channel to the SCMI firmware interface driver for
>> SCMI message exchange on Qualcomm virtual platforms.
>>
>> The hypervisor associates an object-id also known as capability-id
>> with each hvc doorbell object. The capability-id is used to identify the
>> doorbell from the VM's capability namespace, similar to a file-descriptor.
>>
>> The hypervisor, in addition to the function-id, expects the capability-id
>> to be passed in x1 register when HVC call is invoked.
>>
>> The qcom hvc doorbell/shared memory transport uses a statically defined
>> shared memory region that binds with "arm,scmi-shmem" device tree node.
>>
>> The function-id & capability-id are allocated by the hypervisor on bootup
>> and are stored in the shmem region by the firmware before starting Linux.
>>
>> Currently, there is no usecase for the atomic support therefore this driver
>> doesn't include the changes for the same.
>>
> Hi Nikunj,
>
> so basically this new SCMI transport that you are introducing is just
> exactly like the existing SMC transport with the only difference that
> you introduced even another new way to configure func_id, a new cap_id
> param AND the fact that you use HVC instead of SMC... all of this tied
> to a new compatible to identify this new transport mechanism....
> ..but all in all is just a lot of plain duplicated code to maintain...
>
> ...why can't you fit this other smc/hvc transport variant into the
> existing SMC transport by properly picking and configuring func_id/cap_id
> and "doorbell" method (SMC vs HVC) in the chan_setup() step ?
>
> ..I mean ... you can decide where to pick your params based on
> compatibles and also you can setup your invokation method (SMC vs HVC)
> based on those...while keeping all the other stuff exactly the same...
> ...including support for atomic exchanges...if not, when you'll need that
> too in your QC_HVC transport you'll have to duplicate also that (and my
> bugs too probably :P)
>
> (... well maybe in this scenario also the transport itself should be
> renamed from SMC to something more general...)
>
> Not sure if I am missing something, or if Sudeep will be horrified by
> this unifying proposal of mine, but in this series as it stands now I
> just see a lot of brutally duplicated stuff that just differs by naming
> and a very minimal change in logic that could be addressed changing and
> generalizing the original SMC transport code instead.
>
> Thanks,
> Cristian
Hi Christian,
I totally agree with you and will be happy to include my changes in
smc.c if Sudeep agrees with that approach.
Thanks
On 7/25/2023 10:12 AM, Nikunj Kela wrote:
>
> On 7/25/2023 10:03 AM, Cristian Marussi wrote:
>> On Mon, Jul 24, 2023 at 09:44:19AM -0700, Nikunj Kela wrote:
>>> Add a new transport channel to the SCMI firmware interface driver for
>>> SCMI message exchange on Qualcomm virtual platforms.
>>>
>>> The hypervisor associates an object-id also known as capability-id
>>> with each hvc doorbell object. The capability-id is used to identify
>>> the
>>> doorbell from the VM's capability namespace, similar to a
>>> file-descriptor.
>>>
>>> The hypervisor, in addition to the function-id, expects the
>>> capability-id
>>> to be passed in x1 register when HVC call is invoked.
>>>
>>> The qcom hvc doorbell/shared memory transport uses a statically defined
>>> shared memory region that binds with "arm,scmi-shmem" device tree node.
>>>
>>> The function-id & capability-id are allocated by the hypervisor on
>>> bootup
>>> and are stored in the shmem region by the firmware before starting
>>> Linux.
>>>
>>> Currently, there is no usecase for the atomic support therefore this
>>> driver
>>> doesn't include the changes for the same.
>>>
>> Hi Nikunj,
>>
>> so basically this new SCMI transport that you are introducing is just
>> exactly like the existing SMC transport with the only difference that
>> you introduced even another new way to configure func_id, a new cap_id
>> param AND the fact that you use HVC instead of SMC... all of this tied
>> to a new compatible to identify this new transport mechanism....
>> ..but all in all is just a lot of plain duplicated code to maintain...
>>
>> ...why can't you fit this other smc/hvc transport variant into the
>> existing SMC transport by properly picking and configuring
>> func_id/cap_id
>> and "doorbell" method (SMC vs HVC) in the chan_setup() step ?
>>
>> ..I mean ... you can decide where to pick your params based on
>> compatibles and also you can setup your invokation method (SMC vs HVC)
>> based on those...while keeping all the other stuff exactly the same...
>> ...including support for atomic exchanges...if not, when you'll need
>> that
>> too in your QC_HVC transport you'll have to duplicate also that (and my
>> bugs too probably :P)
>>
>> (... well maybe in this scenario also the transport itself should be
>> renamed from SMC to something more general...)
>>
>> Not sure if I am missing something, or if Sudeep will be horrified by
>> this unifying proposal of mine, but in this series as it stands now I
>> just see a lot of brutally duplicated stuff that just differs by naming
>> and a very minimal change in logic that could be addressed changing and
>> generalizing the original SMC transport code instead.
>>
>> Thanks,
>> Cristian
>
> Hi Christian,
>
> I totally agree with you and will be happy to include my changes in
> smc.c if Sudeep agrees with that approach.
>
> Thanks
Hi Sudeep, Could you please provide your feedback on this? Thanks
Hi Nikunj,
kernel test robot noticed the following build errors:
[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.5-rc4 next-20230801]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nikunj-Kela/dt-bindings-arm-convert-nested-if-else-construct-to-allOf/20230725-004613
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230724164419.16092-4-quic_nkela%40quicinc.com
patch subject: [PATCH v2 3/3] firmware: arm_scmi: Add qcom hvc/shmem transport
config: arm-randconfig-r004-20230731 (https://download.01.org/0day-ci/archive/20230801/[email protected]/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce: (https://download.01.org/0day-ci/archive/20230801/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> drivers/firmware/arm_scmi/qcom_hvc.c:182:2: error: write to reserved register 'R7'
arm_smccc_1_1_hvc(scmi_info->func_id, (unsigned long)scmi_info->cap_id,
^
include/linux/arm-smccc.h:536:48: note: expanded from macro 'arm_smccc_1_1_hvc'
#define arm_smccc_1_1_hvc(...) __arm_smccc_1_1(SMCCC_HVC_INST, __VA_ARGS__)
^
include/linux/arm-smccc.h:398:24: note: expanded from macro 'SMCCC_HVC_INST'
#define SMCCC_HVC_INST __HVC(0)
^
arch/arm/include/asm/opcodes-virt.h:11:22: note: expanded from macro '__HVC'
#define __HVC(imm16) __inst_arm_thumb32( \
^
arch/arm/include/asm/opcodes.h:215:2: note: expanded from macro '__inst_arm_thumb32'
__inst_thumb32(thumb_opcode)
^
arch/arm/include/asm/opcodes.h:205:27: note: expanded from macro '__inst_thumb32'
#define __inst_thumb32(x) ___inst_thumb32( \
^
arch/arm/include/asm/opcodes.h:230:2: note: expanded from macro '___inst_thumb32'
".short " __stringify(first) ", " __stringify(second) "\n\t"
^
1 error generated.
vim +/R7 +182 drivers/firmware/arm_scmi/qcom_hvc.c
167
168 static int qcom_hvc_send_message(struct scmi_chan_info *cinfo,
169 struct scmi_xfer *xfer)
170 {
171 struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
172 struct arm_smccc_res res;
173
174 /*
175 * Channel will be released only once response has been
176 * surely fully retrieved, so after .mark_txdone()
177 */
178 mutex_lock(&scmi_info->shmem_lock);
179
180 shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
181
> 182 arm_smccc_1_1_hvc(scmi_info->func_id, (unsigned long)scmi_info->cap_id,
183 0, 0, 0, 0, 0, 0, &res);
184
185 if (res.a0) {
186 mutex_unlock(&scmi_info->shmem_lock);
187 return -EOPNOTSUPP;
188 }
189
190 return 0;
191 }
192
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
This change introduce a new transport channel for Qualcomm virtual
platforms. The transport is mechanically similar to ARM_SCMI_TRANSPORT_SMC.
The difference between the two transports is that a parameter is passed in
the hypervisor call to identify which doorbell to assert. This parameter is
dynamically generated at runtime on the device and insuitable to pass via
the devicetree.
The function ID and parameter are stored by firmware in the shmem region.
This has been tested on ARM64 virtual Qualcomm platform.
---
v3 -> fix the compilation error reported by the test bot,
add support for polling based instances
v2 -> use allOf construct in dtb schema,
remove wrappers from mutexes,
use architecture independent channel layout
v1 -> original patches
Nikunj Kela (3):
dt-bindings: arm: convert nested if-else construct to allOf
dt-bindings: arm: Add qcom specific hvc transport for SCMI
firmware: arm_scmi: Add qcom hvc/shmem transport
.../bindings/firmware/arm,scmi.yaml | 67 ++---
drivers/firmware/arm_scmi/Kconfig | 13 +
drivers/firmware/arm_scmi/Makefile | 2 +
drivers/firmware/arm_scmi/common.h | 3 +
drivers/firmware/arm_scmi/driver.c | 4 +
drivers/firmware/arm_scmi/qcom_hvc.c | 232 ++++++++++++++++++
6 files changed, 293 insertions(+), 28 deletions(-)
create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
--
2.17.1
Introduce compatible "qcom,scmi-hvc-shmem" for SCMI
transport channel for Qualcomm virtual platforms.
The compatible mandates a shared memory channel.
Signed-off-by: Nikunj Kela <[email protected]>
Acked-by: Krzysztof Kozlowski <[email protected]>
---
.../devicetree/bindings/firmware/arm,scmi.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 8d54ea768d38..4090240f45b1 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -45,6 +45,9 @@ properties:
- description: SCMI compliant firmware with OP-TEE transport
items:
- const: linaro,scmi-optee
+ - description: SCMI compliant firmware with Qualcomm hvc/shmem transport
+ items:
+ - const: qcom,scmi-hvc-shmem
interrupts:
description:
@@ -320,6 +323,15 @@ allOf:
required:
- linaro,optee-channel-id
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: qcom,scmi-hvc-shmem
+ then:
+ required:
+ - shmem
+
examples:
- |
firmware {
--
2.17.1
Add a new transport channel to the SCMI firmware interface driver for
SCMI message exchange on Qualcomm virtual platforms.
The hypervisor associates an object-id also known as capability-id
with each hvc doorbell object. The capability-id is used to identify the
doorbell from the VM's capability namespace, similar to a file-descriptor.
The hypervisor, in addition to the function-id, expects the capability-id
to be passed in x1 register when HVC call is invoked.
The qcom hvc doorbell/shared memory transport uses a statically defined
shared memory region that binds with "arm,scmi-shmem" device tree node.
The function-id & capability-id are allocated by the hypervisor on bootup
and are stored in the shmem region by the firmware before starting Linux.
Currently, there is no usecase for the atomic support therefore this driver
doesn't include the changes for the same.
Signed-off-by: Nikunj Kela <[email protected]>
---
drivers/firmware/arm_scmi/Kconfig | 13 ++
drivers/firmware/arm_scmi/Makefile | 2 +
drivers/firmware/arm_scmi/common.h | 3 +
drivers/firmware/arm_scmi/driver.c | 4 +
drivers/firmware/arm_scmi/qcom_hvc.c | 232 +++++++++++++++++++++++++++
5 files changed, 254 insertions(+)
create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig
index ea0f5083ac47..40d07329ebf7 100644
--- a/drivers/firmware/arm_scmi/Kconfig
+++ b/drivers/firmware/arm_scmi/Kconfig
@@ -99,6 +99,19 @@ config ARM_SCMI_TRANSPORT_OPTEE
If you want the ARM SCMI PROTOCOL stack to include support for a
transport based on OP-TEE SCMI service, answer Y.
+config ARM_SCMI_TRANSPORT_QCOM_HVC
+ bool "SCMI transport based on hvc doorbell & shmem for Qualcomm SoCs"
+ depends on ARCH_QCOM
+ select ARM_SCMI_HAVE_TRANSPORT
+ select ARM_SCMI_HAVE_SHMEM
+ default y
+ help
+ Enable hvc doorbell & shmem based transport for SCMI.
+
+ If you want the ARM SCMI PROTOCOL stack to include support for a
+ hvc doorbell and shmem transport on Qualcomm virtual platforms,
+ answer Y.
+
config ARM_SCMI_TRANSPORT_SMC
bool "SCMI transport based on SMC"
depends on HAVE_ARM_SMCCC_DISCOVERY
diff --git a/drivers/firmware/arm_scmi/Makefile b/drivers/firmware/arm_scmi/Makefile
index b31d78fa66cc..aaeba724b5e6 100644
--- a/drivers/firmware/arm_scmi/Makefile
+++ b/drivers/firmware/arm_scmi/Makefile
@@ -10,6 +10,7 @@ scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_SMC) += smc.o
scmi-transport-$(CONFIG_ARM_SCMI_HAVE_MSG) += msg.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_VIRTIO) += virtio.o
scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_OPTEE) += optee.o
+scmi-transport-$(CONFIG_ARM_SCMI_TRANSPORT_QCOM_HVC) += qcom_hvc.o
scmi-protocols-y = base.o clock.o perf.o power.o reset.o sensors.o system.o voltage.o powercap.o
scmi-module-objs := $(scmi-driver-y) $(scmi-protocols-y) $(scmi-transport-y)
@@ -24,4 +25,5 @@ ifeq ($(CONFIG_THUMB2_KERNEL)$(CONFIG_CC_IS_CLANG),yy)
# pointer in Thumb2 mode, which is forcibly enabled by Clang when profiling
# hooks are inserted via the -pg switch.
CFLAGS_REMOVE_smc.o += $(CC_FLAGS_FTRACE)
+CFLAGS_REMOVE_qcom_hvc.o += $(CC_FLAGS_FTRACE)
endif
diff --git a/drivers/firmware/arm_scmi/common.h b/drivers/firmware/arm_scmi/common.h
index c46dc5215af7..5c98cbb1278b 100644
--- a/drivers/firmware/arm_scmi/common.h
+++ b/drivers/firmware/arm_scmi/common.h
@@ -298,6 +298,9 @@ extern const struct scmi_desc scmi_virtio_desc;
#ifdef CONFIG_ARM_SCMI_TRANSPORT_OPTEE
extern const struct scmi_desc scmi_optee_desc;
#endif
+#ifdef CONFIG_ARM_SCMI_TRANSPORT_QCOM_HVC
+extern const struct scmi_desc scmi_qcom_hvc_desc;
+#endif
void scmi_rx_callback(struct scmi_chan_info *cinfo, u32 msg_hdr, void *priv);
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index b5957cc12fee..c54519596c29 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2918,6 +2918,10 @@ static const struct of_device_id scmi_of_match[] = {
#endif
#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
+#endif
+#ifdef CONFIG_ARM_SCMI_TRANSPORT_QCOM_HVC
+ { .compatible = "qcom,scmi-hvc-shmem",
+ .data = &scmi_qcom_hvc_desc },
#endif
{ /* Sentinel */ },
};
diff --git a/drivers/firmware/arm_scmi/qcom_hvc.c b/drivers/firmware/arm_scmi/qcom_hvc.c
new file mode 100644
index 000000000000..87dc36805967
--- /dev/null
+++ b/drivers/firmware/arm_scmi/qcom_hvc.c
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * System Control and Management Interface (SCMI) Message
+ * Qualcomm HVC/shmem Transport driver
+ *
+ * Copyright (c) 2023 Qualcomm Innovation Center, Inc. All rights reserved.
+ * Copyright 2020 NXP
+ *
+ * This is based on drivers/firmware/arm_scmi/smc.c
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+#include "common.h"
+
+/**
+ * struct scmi_qcom_hvc - Structure representing a SCMI qcom hvc transport
+ *
+ * @irq: An optional IRQ for completion
+ * @cinfo: SCMI channel info
+ * @shmem: Transmit/Receive shared memory area
+ * @shmem_lock: Lock to protect access to Tx/Rx shared memory area.
+ * @func_id: hvc call function-id
+ * @cap_id: hvc doorbell's capability id
+ */
+
+struct scmi_qcom_hvc {
+ int irq;
+ struct scmi_chan_info *cinfo;
+ struct scmi_shared_mem __iomem *shmem;
+ /* Protect access to shmem area */
+ struct mutex shmem_lock;
+ u32 func_id;
+ u64 cap_id;
+};
+
+static irqreturn_t qcom_hvc_msg_done_isr(int irq, void *data)
+{
+ struct scmi_qcom_hvc *scmi_info = data;
+
+ scmi_rx_callback(scmi_info->cinfo, shmem_read_header(scmi_info->shmem), NULL);
+
+ return IRQ_HANDLED;
+}
+
+static bool qcom_hvc_chan_available(struct device_node *of_node, int idx)
+{
+ struct device_node *np = of_parse_phandle(of_node, "shmem", 0);
+
+ if (!np)
+ return false;
+
+ of_node_put(np);
+ return true;
+}
+
+static int qcom_hvc_chan_setup(struct scmi_chan_info *cinfo,
+ struct device *dev, bool tx)
+{
+ struct device *cdev = cinfo->dev;
+ struct scmi_qcom_hvc *scmi_info;
+ struct device_node *np;
+ resource_size_t size;
+ struct resource res;
+ u32 func_id;
+ u64 cap_id;
+ int ret;
+
+ if (!tx)
+ return -ENODEV;
+
+ scmi_info = devm_kzalloc(dev, sizeof(*scmi_info), GFP_KERNEL);
+ if (!scmi_info)
+ return -ENOMEM;
+
+ np = of_parse_phandle(cdev->of_node, "shmem", 0);
+ if (!of_device_is_compatible(np, "arm,scmi-shmem")) {
+ of_node_put(np);
+ return -ENXIO;
+ }
+
+ ret = of_address_to_resource(np, 0, &res);
+ of_node_put(np);
+ if (ret) {
+ dev_err(cdev, "failed to get SCMI Tx shared memory\n");
+ return ret;
+ }
+
+ size = resource_size(&res);
+
+ /* The func-id & capability-id are kept in last 16 bytes of shmem.
+ * +-------+
+ * | |
+ * | shmem |
+ * | |
+ * | |
+ * +-------+ <-- (size - 16)
+ * | funcId|
+ * +-------+ <-- (size - 8)
+ * | capId |
+ * +-------+ <-- size
+ */
+
+ scmi_info->shmem = devm_ioremap(dev, res.start, size);
+ if (!scmi_info->shmem) {
+ dev_err(dev, "failed to ioremap SCMI Tx shared memory\n");
+ return -EADDRNOTAVAIL;
+ }
+
+ func_id = readl((void __iomem *)(scmi_info->shmem) + size - 16);
+
+#ifdef CONFIG_ARM64
+ cap_id = readq((void __iomem *)(scmi_info->shmem) + size - 8);
+#else
+ /* capability-id is 32 bit long on 32bit machines */
+ cap_id = readl((void __iomem *)(scmi_info->shmem) + size - 8);
+#endif
+
+ /*
+ * If there is an interrupt named "a2p", then the service and
+ * completion of a message is signaled by an interrupt rather than by
+ * the return of the hvc call.
+ */
+ scmi_info->irq = of_irq_get_byname(cdev->of_node, "a2p");
+ if (scmi_info->irq > 0) {
+ ret = request_irq(scmi_info->irq, qcom_hvc_msg_done_isr,
+ IRQF_NO_SUSPEND, dev_name(dev), scmi_info);
+ if (ret) {
+ dev_err(dev, "failed to setup SCMI completion irq\n");
+ return ret;
+ }
+ } else {
+ cinfo->no_completion_irq = true;
+ }
+
+ scmi_info->func_id = func_id;
+ scmi_info->cap_id = cap_id;
+ scmi_info->cinfo = cinfo;
+ mutex_init(&scmi_info->shmem_lock);
+ cinfo->transport_info = scmi_info;
+
+ return 0;
+}
+
+static int qcom_hvc_chan_free(int id, void *p, void *data)
+{
+ struct scmi_chan_info *cinfo = p;
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ /* Ignore any possible further reception on the IRQ path */
+ if (scmi_info->irq > 0)
+ free_irq(scmi_info->irq, scmi_info);
+
+ cinfo->transport_info = NULL;
+ scmi_info->cinfo = NULL;
+
+ return 0;
+}
+
+static int qcom_hvc_send_message(struct scmi_chan_info *cinfo,
+ struct scmi_xfer *xfer)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+ struct arm_smccc_res res;
+
+ /*
+ * Channel will be released only once response has been
+ * surely fully retrieved, so after .mark_txdone()
+ */
+ mutex_lock(&scmi_info->shmem_lock);
+
+ shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
+
+ arm_smccc_1_1_hvc(scmi_info->func_id, (unsigned long)scmi_info->cap_id,
+ 0, 0, 0, 0, 0, 0, &res);
+
+ if (res.a0) {
+ mutex_unlock(&scmi_info->shmem_lock);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
+}
+
+static void qcom_hvc_fetch_response(struct scmi_chan_info *cinfo,
+ struct scmi_xfer *xfer)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ shmem_fetch_response(scmi_info->shmem, xfer);
+}
+
+static void qcom_hvc_mark_txdone(struct scmi_chan_info *cinfo, int ret,
+ struct scmi_xfer *__unused)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ mutex_unlock(&scmi_info->shmem_lock);
+}
+
+static bool
+qcom_hvc_poll_done(struct scmi_chan_info *cinfo, struct scmi_xfer *xfer)
+{
+ struct scmi_qcom_hvc *scmi_info = cinfo->transport_info;
+
+ return shmem_poll_done(scmi_info->shmem, xfer);
+}
+
+static const struct scmi_transport_ops scmi_qcom_hvc_ops = {
+ .chan_available = qcom_hvc_chan_available,
+ .chan_setup = qcom_hvc_chan_setup,
+ .chan_free = qcom_hvc_chan_free,
+ .send_message = qcom_hvc_send_message,
+ .mark_txdone = qcom_hvc_mark_txdone,
+ .fetch_response = qcom_hvc_fetch_response,
+ .poll_done = qcom_hvc_poll_done,
+};
+
+const struct scmi_desc scmi_qcom_hvc_desc = {
+ .ops = &scmi_qcom_hvc_ops,
+ .max_rx_timeout_ms = 30,
+ .max_msg = 20,
+ .max_msg_size = 128,
+};
--
2.17.1
On Tue, Sep 05, 2023 at 06:37:14PM +0200, Krzysztof Kozlowski wrote:
> On 05/09/2023 18:06, Nikunj Kela wrote:
> >
> > On 8/11/2023 10:57 AM, Nikunj Kela wrote:
> >> This change introduce a new transport channel for Qualcomm virtual
> >> platforms. The transport is mechanically similar to ARM_SCMI_TRANSPORT_SMC.
> >> The difference between the two transports is that a parameter is passed in
> >> the hypervisor call to identify which doorbell to assert. This parameter is
> >> dynamically generated at runtime on the device and insuitable to pass via
> >> the devicetree.
> >>
> >> The function ID and parameter are stored by firmware in the shmem region.
> >>
> >> This has been tested on ARM64 virtual Qualcomm platform.
> >>
> >> ---
> >> v3 -> fix the compilation error reported by the test bot,
> >> add support for polling based instances
> >>
> >> v2 -> use allOf construct in dtb schema,
> >> remove wrappers from mutexes,
> >> use architecture independent channel layout
> >>
> >> v1 -> original patches
> >>
> >> Nikunj Kela (3):
> >> dt-bindings: arm: convert nested if-else construct to allOf
> >> dt-bindings: arm: Add qcom specific hvc transport for SCMI
> >> firmware: arm_scmi: Add qcom hvc/shmem transport
> >>
> >> .../bindings/firmware/arm,scmi.yaml | 67 ++---
> >> drivers/firmware/arm_scmi/Kconfig | 13 +
> >> drivers/firmware/arm_scmi/Makefile | 2 +
> >> drivers/firmware/arm_scmi/common.h | 3 +
> >> drivers/firmware/arm_scmi/driver.c | 4 +
> >> drivers/firmware/arm_scmi/qcom_hvc.c | 232 ++++++++++++++++++
> >> 6 files changed, 293 insertions(+), 28 deletions(-)
> >> create mode 100644 drivers/firmware/arm_scmi/qcom_hvc.c
> > Gentle Ping!
Pong !
>
> It's third ping these two weeks from Qualcomm. Folks, it is merge
> window. What do you think will happen with your ping during this time?
>
+1
Okay, here is the deal with this patch set. As you are aware that a previous
merged solution was abandoned by Qcom in a single kernel release cycle. So
I decided to ignore this for one or 2 kernel release cycle to make sure
Qcom makes up their mind on the design and then we can see how to proceed.
Qcom must understand upstream kernel is not a playground to push their
design which they might decided to drop support for in such short period.
Please understand the upstream kernel supports platforms that are more than
few decades old. It is not like the mobile platforms that are hardly supported
for couple of years. And similarly, we push core support if and only if we
know for sure it will be used on some platform. I trusted Qcom with the
previous extension of SMC/HVC transport but I was proven wrong.
Also, I definitely don't like the way you have copied the whole smc.c
and changed it to Qcom's need and made it qcom_hvc.c. Just add the required
changes in smc.c.
--
Regards,
Sudeep
On 18.07.2023 18:08, Nikunj Kela wrote:
> This change introduce a new transport channel for Qualcomm virtual
> platforms. The transport is mechanically similar to ARM_SCMI_TRANSPORT_SMC.
> The difference between the two transports is that a parameter is passed in
> the hypervisor call to identify which doorbell to assert. This parameter is
> dynamically generated at runtime on the device and insuitable to pass via
> the devicetree.
>
> The function ID and parameter are stored by firmware in the shmem region.
>
> This has been tested on ARM64 virtual Qualcomm platform.
What can we test it on?
Konrad
This change augments smc transport to include support for Qualcomm virtual
platforms by passing a parameter(capability-id) in the hypervisor call to
identify which doorbell to assert. This parameter is dynamically generated
at runtime on the device and insuitable to pass via the devicetree.
The capability-id is stored by firmware in the shmem region.
This has been tested on ARM64 virtual Qualcomm platform.
---
v5 -> changed compatible, removed polling support patch,
make use of smc-id binding for function-id
v4 -> port the changes into smc.c
v3 -> fix the compilation error reported by the test bot,
add support for polling based instances
v2 -> use allOf construct in dtb schema,
remove wrappers from mutexes,
use architecture independent channel layout
v1 -> original patches
Nikunj Kela (2):
dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI
firmware: arm_scmi: Add qcom smc/hvc transport support
.../bindings/firmware/arm,scmi.yaml | 4 +++
drivers/firmware/arm_scmi/driver.c | 1 +
drivers/firmware/arm_scmi/smc.c | 33 +++++++++++++++++--
3 files changed, 36 insertions(+), 2 deletions(-)
--
2.17.1
This change adds the support for SCMI message exchange on Qualcomm
virtual platforms.
The hypervisor associates an object-id also known as capability-id
with each smc/hvc doorbell object. The capability-id is used to
identify the doorbell from the VM's capability namespace, similar
to a file-descriptor.
The hypervisor, in addition to the function-id, expects the capability-id
to be passed in x1 register when SMC/HVC call is invoked.
The capability-id is allocated by the hypervisor on bootup and is stored in
the shmem region by the firmware before starting Linux.
Signed-off-by: Nikunj Kela <[email protected]>
---
drivers/firmware/arm_scmi/driver.c | 1 +
drivers/firmware/arm_scmi/smc.c | 33 ++++++++++++++++++++++++++++--
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/arm_scmi/driver.c b/drivers/firmware/arm_scmi/driver.c
index 87383c05424b..09371f40d61f 100644
--- a/drivers/firmware/arm_scmi/driver.c
+++ b/drivers/firmware/arm_scmi/driver.c
@@ -2915,6 +2915,7 @@ static const struct of_device_id scmi_of_match[] = {
#ifdef CONFIG_ARM_SCMI_TRANSPORT_SMC
{ .compatible = "arm,scmi-smc", .data = &scmi_smc_desc},
{ .compatible = "arm,scmi-smc-param", .data = &scmi_smc_desc},
+ { .compatible = "qcom,scmi-smc", .data = &scmi_smc_desc},
#endif
#ifdef CONFIG_ARM_SCMI_TRANSPORT_VIRTIO
{ .compatible = "arm,scmi-virtio", .data = &scmi_virtio_desc},
diff --git a/drivers/firmware/arm_scmi/smc.c b/drivers/firmware/arm_scmi/smc.c
index c193516a254d..3d594d65ab14 100644
--- a/drivers/firmware/arm_scmi/smc.c
+++ b/drivers/firmware/arm_scmi/smc.c
@@ -50,6 +50,8 @@
* @func_id: smc/hvc call function id
* @param_page: 4K page number of the shmem channel
* @param_offset: Offset within the 4K page of the shmem channel
+ * @cap_id: smc/hvc doorbell's capability id to be used on Qualcomm virtual
+ * platforms
*/
struct scmi_smc {
@@ -63,6 +65,7 @@ struct scmi_smc {
u32 func_id;
u32 param_page;
u32 param_offset;
+ s64 cap_id;
};
static irqreturn_t smc_msg_done_isr(int irq, void *data)
@@ -128,6 +131,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
resource_size_t size;
struct resource res;
struct device_node *np;
+ s64 cap_id = -EINVAL;
u32 func_id;
int ret;
@@ -162,6 +166,25 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
if (ret < 0)
return ret;
+ if (of_device_is_compatible(dev->of_node, "qcom,scmi-smc")) {
+ /* The capability-id is kept in last 8 bytes of shmem.
+ * +-------+
+ * | |
+ * | shmem |
+ * | |
+ * | |
+ * +-------+ <-- (size - 8)
+ * | capId |
+ * +-------+ <-- size
+ */
+
+#ifdef CONFIG_64BIT
+ cap_id = ioread64((void *)scmi_info->shmem + size - 8);
+#else
+ cap_id = ioread32((void *)scmi_info->shmem + size - 8);
+#endif
+ }
+
if (of_device_is_compatible(dev->of_node, "arm,scmi-smc-param")) {
scmi_info->param_page = SHMEM_PAGE(res.start);
scmi_info->param_offset = SHMEM_OFFSET(res.start);
@@ -184,6 +207,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
}
scmi_info->func_id = func_id;
+ scmi_info->cap_id = cap_id;
scmi_info->cinfo = cinfo;
smc_channel_lock_init(scmi_info);
cinfo->transport_info = scmi_info;
@@ -213,6 +237,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
struct arm_smccc_res res;
unsigned long page = scmi_info->param_page;
unsigned long offset = scmi_info->param_offset;
+ long cap_id = (long)scmi_info->cap_id;
/*
* Channel will be released only once response has been
@@ -222,8 +247,12 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
- arm_smccc_1_1_invoke(scmi_info->func_id, page, offset, 0, 0, 0, 0, 0,
- &res);
+ if (cap_id >= 0)
+ arm_smccc_1_1_invoke(scmi_info->func_id, cap_id, 0, 0, 0, 0, 0,
+ 0, &res);
+ else
+ arm_smccc_1_1_invoke(scmi_info->func_id, page, offset, 0, 0, 0,
+ 0, 0, &res);
/* Only SMCCC_RET_NOT_SUPPORTED is valid error code */
if (res.a0) {
--
2.17.1
On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
> This change adds the support for SCMI message exchange on Qualcomm
> virtual platforms.
>
> The hypervisor associates an object-id also known as capability-id
> with each smc/hvc doorbell object. The capability-id is used to
> identify the doorbell from the VM's capability namespace, similar
> to a file-descriptor.
>
> The hypervisor, in addition to the function-id, expects the capability-id
> to be passed in x1 register when SMC/HVC call is invoked.
>
> The capability-id is allocated by the hypervisor on bootup and is stored in
> the shmem region by the firmware before starting Linux.
>
> Signed-off-by: Nikunj Kela <[email protected]>
Reviewed-by: Brian Masney <[email protected]>
On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
> This change adds the support for SCMI message exchange on Qualcomm
> virtual platforms.
>
> The hypervisor associates an object-id also known as capability-id
> with each smc/hvc doorbell object. The capability-id is used to
> identify the doorbell from the VM's capability namespace, similar
> to a file-descriptor.
>
> The hypervisor, in addition to the function-id, expects the capability-id
> to be passed in x1 register when SMC/HVC call is invoked.
>
> The capability-id is allocated by the hypervisor on bootup and is stored in
> the shmem region by the firmware before starting Linux.
>
Since you are happy to move to signed value, I assume you are happy to loose
upper half of the range values ?
Anyways after Bjorn pointed out inconsistency, I am thinking of moving
all the values to unsigned long to work with both 32bit and 64bit.
Does the below delta on top of this patch works for you and makes sense?
--
Regards,
Sudeep
-->8
diff --git c/drivers/firmware/arm_scmi/smc.c i/drivers/firmware/arm_scmi/smc.c
index bf0b7769c7b2..e00c5e81c8d9 100644
--- c/drivers/firmware/arm_scmi/smc.c
+++ i/drivers/firmware/arm_scmi/smc.c
@@ -15,6 +15,7 @@
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_irq.h>
+#include <linux/limits.h>
#include <linux/processor.h>
#include <linux/slab.h>
@@ -65,7 +66,7 @@ struct scmi_smc {
unsigned long func_id;
unsigned long param_page;
unsigned long param_offset;
- s64 cap_id;
+ unsigned long cap_id;
};
static irqreturn_t smc_msg_done_isr(int irq, void *data)
@@ -127,11 +128,11 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
bool tx)
{
struct device *cdev = cinfo->dev;
+ unsigned long cap_id = ULONG_MAX;
struct scmi_smc *scmi_info;
resource_size_t size;
struct resource res;
struct device_node *np;
- s64 cap_id = -EINVAL;
u32 func_id;
int ret;
@@ -167,6 +168,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
return ret;
if (of_device_is_compatible(dev->of_node, "qcom,scmi-smc")) {
+ void __iomem *ptr = (void __iomem *)scmi_info->shmem + size - 8;
/* The capability-id is kept in last 8 bytes of shmem.
* +-------+
* | |
@@ -177,12 +179,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
* | capId |
* +-------+ <-- size
*/
-
-#ifdef CONFIG_64BIT
- cap_id = ioread64((void *)scmi_info->shmem + size - 8);
-#else
- cap_id = ioread32((void *)scmi_info->shmem + size - 8);
-#endif
+ memcpy_fromio(&cap_id, ptr, sizeof(cap_id));
}
if (of_device_is_compatible(dev->of_node, "arm,scmi-smc-param")) {
@@ -247,7 +244,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
- if (cap_id >= 0)
+ if (cap_id != ULONG_MAX)
arm_smccc_1_1_invoke(scmi_info->func_id, cap_id, 0, 0, 0, 0, 0,
0, &res);
else
On 10/9/2023 7:47 AM, Sudeep Holla wrote:
> On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
>> This change adds the support for SCMI message exchange on Qualcomm
>> virtual platforms.
>>
>> The hypervisor associates an object-id also known as capability-id
>> with each smc/hvc doorbell object. The capability-id is used to
>> identify the doorbell from the VM's capability namespace, similar
>> to a file-descriptor.
>>
>> The hypervisor, in addition to the function-id, expects the capability-id
>> to be passed in x1 register when SMC/HVC call is invoked.
>>
>> The capability-id is allocated by the hypervisor on bootup and is stored in
>> the shmem region by the firmware before starting Linux.
>>
> Since you are happy to move to signed value, I assume you are happy to loose
> upper half of the range values ?
>
> Anyways after Bjorn pointed out inconsistency, I am thinking of moving
> all the values to unsigned long to work with both 32bit and 64bit.
>
> Does the below delta on top of this patch works for you and makes sense?
This looks good to me. Will do some testing and float v6 with the
changes you suggested below. Thanks
>
> --
> Regards,
> Sudeep
>
> -->8
> diff --git c/drivers/firmware/arm_scmi/smc.c i/drivers/firmware/arm_scmi/smc.c
> index bf0b7769c7b2..e00c5e81c8d9 100644
> --- c/drivers/firmware/arm_scmi/smc.c
> +++ i/drivers/firmware/arm_scmi/smc.c
> @@ -15,6 +15,7 @@
> #include <linux/of.h>
> #include <linux/of_address.h>
> #include <linux/of_irq.h>
> +#include <linux/limits.h>
> #include <linux/processor.h>
> #include <linux/slab.h>
>
> @@ -65,7 +66,7 @@ struct scmi_smc {
> unsigned long func_id;
> unsigned long param_page;
> unsigned long param_offset;
> - s64 cap_id;
> + unsigned long cap_id;
> };
>
> static irqreturn_t smc_msg_done_isr(int irq, void *data)
> @@ -127,11 +128,11 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> bool tx)
> {
> struct device *cdev = cinfo->dev;
> + unsigned long cap_id = ULONG_MAX;
> struct scmi_smc *scmi_info;
> resource_size_t size;
> struct resource res;
> struct device_node *np;
> - s64 cap_id = -EINVAL;
> u32 func_id;
> int ret;
>
> @@ -167,6 +168,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> return ret;
>
> if (of_device_is_compatible(dev->of_node, "qcom,scmi-smc")) {
> + void __iomem *ptr = (void __iomem *)scmi_info->shmem + size - 8;
> /* The capability-id is kept in last 8 bytes of shmem.
> * +-------+
> * | |
> @@ -177,12 +179,7 @@ static int smc_chan_setup(struct scmi_chan_info *cinfo, struct device *dev,
> * | capId |
> * +-------+ <-- size
> */
> -
> -#ifdef CONFIG_64BIT
> - cap_id = ioread64((void *)scmi_info->shmem + size - 8);
> -#else
> - cap_id = ioread32((void *)scmi_info->shmem + size - 8);
> -#endif
> + memcpy_fromio(&cap_id, ptr, sizeof(cap_id));
> }
>
> if (of_device_is_compatible(dev->of_node, "arm,scmi-smc-param")) {
> @@ -247,7 +244,7 @@ static int smc_send_message(struct scmi_chan_info *cinfo,
>
> shmem_tx_prepare(scmi_info->shmem, xfer, cinfo);
>
> - if (cap_id >= 0)
> + if (cap_id != ULONG_MAX)
> arm_smccc_1_1_invoke(scmi_info->func_id, cap_id, 0, 0, 0, 0, 0,
> 0, &res);
> else
>
On Mon, Oct 09, 2023 at 07:59:08AM -0700, Nikunj Kela wrote:
>
> On 10/9/2023 7:47 AM, Sudeep Holla wrote:
> > On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
> > > This change adds the support for SCMI message exchange on Qualcomm
> > > virtual platforms.
> > >
> > > The hypervisor associates an object-id also known as capability-id
> > > with each smc/hvc doorbell object. The capability-id is used to
> > > identify the doorbell from the VM's capability namespace, similar
> > > to a file-descriptor.
> > >
> > > The hypervisor, in addition to the function-id, expects the capability-id
> > > to be passed in x1 register when SMC/HVC call is invoked.
> > >
> > > The capability-id is allocated by the hypervisor on bootup and is stored in
> > > the shmem region by the firmware before starting Linux.
> > >
> > Since you are happy to move to signed value, I assume you are happy to loose
> > upper half of the range values ?
> >
> > Anyways after Bjorn pointed out inconsistency, I am thinking of moving
> > all the values to unsigned long to work with both 32bit and 64bit.
> >
> > Does the below delta on top of this patch works for you and makes sense?
>
> This looks good to me. Will do some testing and float v6 with the changes
> you suggested below. Thanks
>
Please refer or use the patch from [1] when reposting. I rebased on my
patch[2] that I posted few minutes back. I am trying to finalise the branch
and send PR in next couple of days, so please test and post sooner. Sorry
for the rush.
--
Regards,
Sudeep
[1] https://git.kernel.org/sudeep.holla/h/for-next/scmi/updates
[2] https://lore.kernel.org/r/[email protected]
On 10/9/2023 8:29 AM, Sudeep Holla wrote:
> On Mon, Oct 09, 2023 at 07:59:08AM -0700, Nikunj Kela wrote:
>> On 10/9/2023 7:47 AM, Sudeep Holla wrote:
>>> On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
>>>> This change adds the support for SCMI message exchange on Qualcomm
>>>> virtual platforms.
>>>>
>>>> The hypervisor associates an object-id also known as capability-id
>>>> with each smc/hvc doorbell object. The capability-id is used to
>>>> identify the doorbell from the VM's capability namespace, similar
>>>> to a file-descriptor.
>>>>
>>>> The hypervisor, in addition to the function-id, expects the capability-id
>>>> to be passed in x1 register when SMC/HVC call is invoked.
>>>>
>>>> The capability-id is allocated by the hypervisor on bootup and is stored in
>>>> the shmem region by the firmware before starting Linux.
>>>>
>>> Since you are happy to move to signed value, I assume you are happy to loose
>>> upper half of the range values ?
>>>
>>> Anyways after Bjorn pointed out inconsistency, I am thinking of moving
>>> all the values to unsigned long to work with both 32bit and 64bit.
>>>
>>> Does the below delta on top of this patch works for you and makes sense?
>> This looks good to me. Will do some testing and float v6 with the changes
>> you suggested below. Thanks
>>
> Please refer or use the patch from [1] when reposting. I rebased on my
> patch[2] that I posted few minutes back. I am trying to finalise the branch
> and send PR in next couple of days, so please test and post sooner. Sorry
> for the rush.
Validated the patch from [1] below on Qualcomm ARM64 virtual platform
using SMC64 convention. Thanks!
>
> --
> Regards,
> Sudeep
> [1] https://git.kernel.org/sudeep.holla/h/for-next/scmi/updates
> [2] https://lore.kernel.org/r/[email protected]
On Mon, Oct 09, 2023 at 10:49:44AM -0700, Nikunj Kela wrote:
>
> On 10/9/2023 8:29 AM, Sudeep Holla wrote:
> > On Mon, Oct 09, 2023 at 07:59:08AM -0700, Nikunj Kela wrote:
> > > On 10/9/2023 7:47 AM, Sudeep Holla wrote:
> > > > On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
> > > > > This change adds the support for SCMI message exchange on Qualcomm
> > > > > virtual platforms.
> > > > >
> > > > > The hypervisor associates an object-id also known as capability-id
> > > > > with each smc/hvc doorbell object. The capability-id is used to
> > > > > identify the doorbell from the VM's capability namespace, similar
> > > > > to a file-descriptor.
> > > > >
> > > > > The hypervisor, in addition to the function-id, expects the capability-id
> > > > > to be passed in x1 register when SMC/HVC call is invoked.
> > > > >
> > > > > The capability-id is allocated by the hypervisor on bootup and is stored in
> > > > > the shmem region by the firmware before starting Linux.
> > > > >
> > > > Since you are happy to move to signed value, I assume you are happy to loose
> > > > upper half of the range values ?
> > > >
> > > > Anyways after Bjorn pointed out inconsistency, I am thinking of moving
> > > > all the values to unsigned long to work with both 32bit and 64bit.
> > > >
> > > > Does the below delta on top of this patch works for you and makes sense?
> > > This looks good to me. Will do some testing and float v6 with the changes
> > > you suggested below. Thanks
> > >
> > Please refer or use the patch from [1] when reposting. I rebased on my
> > patch[2] that I posted few minutes back. I am trying to finalise the branch
> > and send PR in next couple of days, so please test and post sooner. Sorry
> > for the rush.
>
> Validated the patch from [1] below on Qualcomm ARM64 virtual platform using
> SMC64 convention. Thanks!
>
Thanks, since I have patched a bit, it is better if you post them so that
we have a link for the exact patch on the list. Just pick up the patches
from the branch[1] and post them as v6 with a change log so that all the
details are captured for reference purposes.
--
Regards,
Sudeep
[1] https://git.kernel.org/sudeep.holla/h/for-next/scmi/updates
[2] https://lore.kernel.org/r/[email protected]
This change augments smc transport to include support for Qualcomm virtual
platforms by passing a parameter(capability-id) in the hypervisor call to
identify which doorbell to assert. This parameter is dynamically generated
at runtime on the device and insuitable to pass via the devicetree.
The capability-id is stored by firmware in the shmem region.
This has been tested on ARM64 virtual Qualcomm platform.
---
v6 -> use unsigned long for cap-id
v5 -> changed compatible, removed polling support patch,
make use of smc-id binding for function-id
v4 -> port the changes into smc.c
v3 -> fix the compilation error reported by the test bot,
add support for polling based instances
v2 -> use allOf construct in dtb schema,
remove wrappers from mutexes,
use architecture independent channel layout
v1 -> original patches
Nikunj Kela (2):
dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI
firmware: arm_scmi: Add qcom smc/hvc transport support
.../bindings/firmware/arm,scmi.yaml | 4 +++
drivers/firmware/arm_scmi/driver.c | 1 +
drivers/firmware/arm_scmi/smc.c | 27 +++++++++++++++++--
3 files changed, 30 insertions(+), 2 deletions(-)
--
2.17.1
Introduce compatible "qcom,scmi-smc" for SCMI smc/hvc transport channel for
Qualcomm virtual platforms.
This compatible mandates populating an additional parameter 'capability-id'
from the last 8 bytes of the shmem channel.
Signed-off-by: Nikunj Kela <[email protected]>
Reviewed-by: Brian Masney <[email protected]>
Signed-off-by: Sudeep Holla <[email protected]>
---
Documentation/devicetree/bindings/firmware/arm,scmi.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
index 563a87dfb31a..4591523b51a0 100644
--- a/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
+++ b/Documentation/devicetree/bindings/firmware/arm,scmi.yaml
@@ -38,6 +38,9 @@ properties:
with shmem address(4KB-page, offset) as parameters
items:
- const: arm,scmi-smc-param
+ - description: SCMI compliant firmware with Qualcomm SMC/HVC transport
+ items:
+ - const: qcom,scmi-smc
- description: SCMI compliant firmware with SCMI Virtio transport.
The virtio transport only supports a single device.
items:
@@ -313,6 +316,7 @@ else:
enum:
- arm,scmi-smc
- arm,scmi-smc-param
+ - qcom,scmi-smc
then:
required:
- arm,smc-id
--
2.17.1
On 10/9/2023 12:08 PM, Sudeep Holla wrote:
> On Mon, Oct 09, 2023 at 10:49:44AM -0700, Nikunj Kela wrote:
>> On 10/9/2023 8:29 AM, Sudeep Holla wrote:
>>> On Mon, Oct 09, 2023 at 07:59:08AM -0700, Nikunj Kela wrote:
>>>> On 10/9/2023 7:47 AM, Sudeep Holla wrote:
>>>>> On Fri, Oct 06, 2023 at 09:42:06AM -0700, Nikunj Kela wrote:
>>>>>> This change adds the support for SCMI message exchange on Qualcomm
>>>>>> virtual platforms.
>>>>>>
>>>>>> The hypervisor associates an object-id also known as capability-id
>>>>>> with each smc/hvc doorbell object. The capability-id is used to
>>>>>> identify the doorbell from the VM's capability namespace, similar
>>>>>> to a file-descriptor.
>>>>>>
>>>>>> The hypervisor, in addition to the function-id, expects the capability-id
>>>>>> to be passed in x1 register when SMC/HVC call is invoked.
>>>>>>
>>>>>> The capability-id is allocated by the hypervisor on bootup and is stored in
>>>>>> the shmem region by the firmware before starting Linux.
>>>>>>
>>>>> Since you are happy to move to signed value, I assume you are happy to loose
>>>>> upper half of the range values ?
>>>>>
>>>>> Anyways after Bjorn pointed out inconsistency, I am thinking of moving
>>>>> all the values to unsigned long to work with both 32bit and 64bit.
>>>>>
>>>>> Does the below delta on top of this patch works for you and makes sense?
>>>> This looks good to me. Will do some testing and float v6 with the changes
>>>> you suggested below. Thanks
>>>>
>>> Please refer or use the patch from [1] when reposting. I rebased on my
>>> patch[2] that I posted few minutes back. I am trying to finalise the branch
>>> and send PR in next couple of days, so please test and post sooner. Sorry
>>> for the rush.
>> Validated the patch from [1] below on Qualcomm ARM64 virtual platform using
>> SMC64 convention. Thanks!
>>
> Thanks, since I have patched a bit, it is better if you post them so that
> we have a link for the exact patch on the list. Just pick up the patches
> from the branch[1] and post them as v6 with a change log so that all the
> details are captured for reference purposes.
v6 on its way, thanks!
>
On Mon, 09 Oct 2023 12:14:35 -0700, Nikunj Kela wrote:
> This change augments smc transport to include support for Qualcomm virtual
> platforms by passing a parameter(capability-id) in the hypervisor call to
> identify which doorbell to assert. This parameter is dynamically generated
> at runtime on the device and insuitable to pass via the devicetree.
>
> The capability-id is stored by firmware in the shmem region.
>
> [...]
Applied to sudeep.holla/linux (for-next/scmi/updates), thanks!
[1/2] dt-bindings: arm: Add new compatible for smc/hvc transport for SCMI
https://git.kernel.org/sudeep.holla/c/6979f88f5a8e
[2/2] firmware: arm_scmi: Add qcom smc/hvc transport support
https://git.kernel.org/sudeep.holla/c/da405477e767
--
Regards,
Sudeep