All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 0/7] firmware: Add initial support for Arm FF-A
@ 2021-03-25 14:32 ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Hi all,

This is very basic implementation for in-kernel support for Arm FF-A
specification.

iArm Firmware Framework for Armv8-A specification[1] describes a software
architecture that provides mechanism to utilise the virtualization
extension to isolate software images and describes interfaces that
standardize communication between the various software images. This
includes communication between images in the Secure and Normal world.

The main idea here is to create FFA device to establish any communication
with a secure partition. This is currently tested with OPTEE(with changes
to OPTEE driver adding FFA as transport)

The series can be fetched from [2]

--
Regards,
Sudeep

[1] https://developer.arm.com/documentation/den0077/latest
[2] git://git.kerniel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git v5.12/v5_ffa

v4->v5:
	- Fixed couple of comments in DT bindings
	- Moved to use native version of RXTX_MAP call, fixed vm_id param
	  to RXTX_UNMAP, dropped couple of unused confusingly named macros
	  (can be added back with correct name when we need to use them)

v3->v4:
	- Added support to allow partitions to set 32bit only mode
	- Addressed all the comments from Jens Wiklander

v2->v3:
	- Dropped hypervisor partitions and userspace support as it is
	  no longer in the list of requirements
	- Moved away from ioctl style interface for in-kernel users as
	  there is no need to keep in sync with userspace anymore
	- Some kerneldoc fixes as pointed out in earlier reviews

v1->v2:
	- Moved userspace code to a separate unit, will move to separate
	  module. Still working on minimizing initcall dependencies and
	  exported functions to reuse some of the code.
	- Fixed couple of minor issues pointed out
	- Dropped ASYNC send message as I haven't been able to test


Sudeep Holla (7):
  dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  arm64: smccc: Add support for SMCCCv1.2 input/output registers
  firmware: arm_ffa: Add initial FFA bus support for device enumeration
  firmware: arm_ffa: Add initial Arm FFA driver support
  firmware: arm_ffa: Add support for SMCCC as transport to FFA driver
  firmware: arm_ffa: Setup in-kernel users of FFA partitions
  firmware: arm_ffa: Add support for MEM_* interfaces

 .../devicetree/bindings/arm/arm,ffa.yaml      |  57 ++
 arch/arm64/kernel/asm-offsets.c               |   4 +
 arch/arm64/kernel/smccc-call.S                |  22 +
 drivers/firmware/Kconfig                      |   1 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/arm_ffa/Kconfig              |  21 +
 drivers/firmware/arm_ffa/Makefile             |   6 +
 drivers/firmware/arm_ffa/bus.c                | 181 +++++
 drivers/firmware/arm_ffa/common.h             |  32 +
 drivers/firmware/arm_ffa/driver.c             | 672 ++++++++++++++++++
 drivers/firmware/arm_ffa/smccc.c              |  54 ++
 include/linux/arm-smccc.h                     |  50 ++
 include/linux/arm_ffa.h                       | 266 +++++++
 13 files changed, 1367 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
 create mode 100644 drivers/firmware/arm_ffa/Kconfig
 create mode 100644 drivers/firmware/arm_ffa/Makefile
 create mode 100644 drivers/firmware/arm_ffa/bus.c
 create mode 100644 drivers/firmware/arm_ffa/common.h
 create mode 100644 drivers/firmware/arm_ffa/driver.c
 create mode 100644 drivers/firmware/arm_ffa/smccc.c
 create mode 100644 include/linux/arm_ffa.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v5 0/7] firmware: Add initial support for Arm FF-A
@ 2021-03-25 14:32 ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Hi all,

This is very basic implementation for in-kernel support for Arm FF-A
specification.

iArm Firmware Framework for Armv8-A specification[1] describes a software
architecture that provides mechanism to utilise the virtualization
extension to isolate software images and describes interfaces that
standardize communication between the various software images. This
includes communication between images in the Secure and Normal world.

The main idea here is to create FFA device to establish any communication
with a secure partition. This is currently tested with OPTEE(with changes
to OPTEE driver adding FFA as transport)

The series can be fetched from [2]

--
Regards,
Sudeep

[1] https://developer.arm.com/documentation/den0077/latest
[2] git://git.kerniel.org/pub/scm/linux/kernel/git/sudeep.holla/linux.git v5.12/v5_ffa

v4->v5:
	- Fixed couple of comments in DT bindings
	- Moved to use native version of RXTX_MAP call, fixed vm_id param
	  to RXTX_UNMAP, dropped couple of unused confusingly named macros
	  (can be added back with correct name when we need to use them)

v3->v4:
	- Added support to allow partitions to set 32bit only mode
	- Addressed all the comments from Jens Wiklander

v2->v3:
	- Dropped hypervisor partitions and userspace support as it is
	  no longer in the list of requirements
	- Moved away from ioctl style interface for in-kernel users as
	  there is no need to keep in sync with userspace anymore
	- Some kerneldoc fixes as pointed out in earlier reviews

v1->v2:
	- Moved userspace code to a separate unit, will move to separate
	  module. Still working on minimizing initcall dependencies and
	  exported functions to reuse some of the code.
	- Fixed couple of minor issues pointed out
	- Dropped ASYNC send message as I haven't been able to test


Sudeep Holla (7):
  dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  arm64: smccc: Add support for SMCCCv1.2 input/output registers
  firmware: arm_ffa: Add initial FFA bus support for device enumeration
  firmware: arm_ffa: Add initial Arm FFA driver support
  firmware: arm_ffa: Add support for SMCCC as transport to FFA driver
  firmware: arm_ffa: Setup in-kernel users of FFA partitions
  firmware: arm_ffa: Add support for MEM_* interfaces

 .../devicetree/bindings/arm/arm,ffa.yaml      |  57 ++
 arch/arm64/kernel/asm-offsets.c               |   4 +
 arch/arm64/kernel/smccc-call.S                |  22 +
 drivers/firmware/Kconfig                      |   1 +
 drivers/firmware/Makefile                     |   1 +
 drivers/firmware/arm_ffa/Kconfig              |  21 +
 drivers/firmware/arm_ffa/Makefile             |   6 +
 drivers/firmware/arm_ffa/bus.c                | 181 +++++
 drivers/firmware/arm_ffa/common.h             |  32 +
 drivers/firmware/arm_ffa/driver.c             | 672 ++++++++++++++++++
 drivers/firmware/arm_ffa/smccc.c              |  54 ++
 include/linux/arm-smccc.h                     |  50 ++
 include/linux/arm_ffa.h                       | 266 +++++++
 13 files changed, 1367 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
 create mode 100644 drivers/firmware/arm_ffa/Kconfig
 create mode 100644 drivers/firmware/arm_ffa/Makefile
 create mode 100644 drivers/firmware/arm_ffa/bus.c
 create mode 100644 drivers/firmware/arm_ffa/common.h
 create mode 100644 drivers/firmware/arm_ffa/driver.c
 create mode 100644 drivers/firmware/arm_ffa/smccc.c
 create mode 100644 include/linux/arm_ffa.h

-- 
2.25.1


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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Since the FF-A v1.0 specification doesn't list the UUID of all the
partitions in the discovery API, we need to specify the UUID of the
partitions that need to be accessed by drivers within the kernel.

This binding to provide the list of partitions that kernel drivers
may need to access.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 .../devicetree/bindings/arm/arm,ffa.yaml      | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml

diff --git a/Documentation/devicetree/bindings/arm/arm,ffa.yaml b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
new file mode 100644
index 000000000000..42ed33464ba7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/arm,ffa.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Firmware Framework for Arm v8-A (in-kernel users)
+
+maintainers:
+  - Sudeep Holla <sudeep.holla@arm.com>
+
+description: |
+  Firmware frameworks implementing partition according to the FF-A
+  specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
+  Framework for Arm v8-A") [0], providing services to be used by other
+  partitions.
+
+  [0] https://developer.arm.com/docs/den0077/latest
+
+properties:
+  $nodename:
+    const: ffa
+
+  compatible:
+    oneOf:
+      - const: arm,ffa-1.0
+
+patternProperties:
+  "^ffa-partition-[0-9]+$":
+    type: object
+    description: One or more child nodes, each describing an FFA partition.
+    properties:
+      $nodename:
+        const: ffa_partition
+
+      compatible:
+        const: arm,ffa-1.0-partition
+
+      uuid:
+        $ref: '/schemas/types.yaml#definitions/string'
+        description: |
+          The 128-bit UUID [2] of the service implemented by this partition.
+
+          [2] https://tools.ietf.org/html/rfc4122
+
+additionalProperties: false
+
+examples:
+  - |
+    ffa {
+      compatible = "arm,ffa-1.0";
+
+      ffa-partition-0 {
+        compatible = "arm,ffa-1.0-partition";
+        uuid = "12345678-9abc-def0-1234-56789abcdef0";
+      };
+    };
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Since the FF-A v1.0 specification doesn't list the UUID of all the
partitions in the discovery API, we need to specify the UUID of the
partitions that need to be accessed by drivers within the kernel.

This binding to provide the list of partitions that kernel drivers
may need to access.

Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 .../devicetree/bindings/arm/arm,ffa.yaml      | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml

diff --git a/Documentation/devicetree/bindings/arm/arm,ffa.yaml b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
new file mode 100644
index 000000000000..42ed33464ba7
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
@@ -0,0 +1,57 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/arm/arm,ffa.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Arm Firmware Framework for Arm v8-A (in-kernel users)
+
+maintainers:
+  - Sudeep Holla <sudeep.holla@arm.com>
+
+description: |
+  Firmware frameworks implementing partition according to the FF-A
+  specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
+  Framework for Arm v8-A") [0], providing services to be used by other
+  partitions.
+
+  [0] https://developer.arm.com/docs/den0077/latest
+
+properties:
+  $nodename:
+    const: ffa
+
+  compatible:
+    oneOf:
+      - const: arm,ffa-1.0
+
+patternProperties:
+  "^ffa-partition-[0-9]+$":
+    type: object
+    description: One or more child nodes, each describing an FFA partition.
+    properties:
+      $nodename:
+        const: ffa_partition
+
+      compatible:
+        const: arm,ffa-1.0-partition
+
+      uuid:
+        $ref: '/schemas/types.yaml#definitions/string'
+        description: |
+          The 128-bit UUID [2] of the service implemented by this partition.
+
+          [2] https://tools.ietf.org/html/rfc4122
+
+additionalProperties: false
+
+examples:
+  - |
+    ffa {
+      compatible = "arm,ffa-1.0";
+
+      ffa-partition-0 {
+        compatible = "arm,ffa-1.0-partition";
+        uuid = "12345678-9abc-def0-1234-56789abcdef0";
+      };
+    };
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici,
	Catalin Marinas, Will Deacon

SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
to be used as result registers in SMC64/HVC64. Arm Firmware Framework
for Armv8-A specification makes use of x0-x7 as parameter and result
registers.

Current SMCCC interface in the kernel just use x0-x7 as parameter and
x0-x3 as result registers. Let us add new interface to support x0-x7
as parameter and result registers. This can be extended to include
x8-x17 when there are users for the same.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/kernel/asm-offsets.c |  4 +++
 arch/arm64/kernel/smccc-call.S  | 22 +++++++++++++++
 include/linux/arm-smccc.h       | 50 +++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

Hi Will/Catalin,

I seemed to have missed you on these patches. I realised only when I was
collecting Acks to get this series merged. The change here is simple, it
would be good if you can review and ack if you are OK with it so that I
can get the serier merged via ARM SoC.

diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index a36e2fc330d4..57ffea2920b8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -132,6 +132,10 @@ int main(void)
   DEFINE(ARM_SMCCC_RES_X2_OFFS,		offsetof(struct arm_smccc_res, a2));
   DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,	offsetof(struct arm_smccc_quirk, id));
   DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS,	offsetof(struct arm_smccc_quirk, state));
+  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
+  DEFINE(ARM_SMCCC_V1_2_RES_X2_OFFS,	offsetof(struct arm_smccc_v1_2_res, a2));
+  DEFINE(ARM_SMCCC_V1_2_RES_X4_OFFS,	offsetof(struct arm_smccc_v1_2_res, a4));
+  DEFINE(ARM_SMCCC_V1_2_RES_X6_OFFS,	offsetof(struct arm_smccc_v1_2_res, a6));
   BLANK();
   DEFINE(HIBERN_PBE_ORIG,	offsetof(struct pbe, orig_address));
   DEFINE(HIBERN_PBE_ADDR,	offsetof(struct pbe, address));
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index d62447964ed9..0ea15c1742f3 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc)
 	SMCCC	hvc
 SYM_FUNC_END(__arm_smccc_hvc)
 EXPORT_SYMBOL(__arm_smccc_hvc)
+
+	.macro SMCCC_v1_2 instr
+	.cfi_startproc
+	\instr #0
+	ldr x8, [sp]
+	stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS]
+	stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS]
+	stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS]
+	stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS]
+	ret
+	.cfi_endproc
+.endm
+
+SYM_FUNC_START(arm_smccc_v1_2_hvc)
+	SMCCC_v1_2 hvc
+SYM_FUNC_END(arm_smccc_v1_2_hvc)
+EXPORT_SYMBOL(arm_smccc_v1_2_hvc)
+
+SYM_FUNC_START(arm_smccc_v1_2_smc)
+	SMCCC_v1_2 smc
+SYM_FUNC_END(arm_smccc_v1_2_smc)
+EXPORT_SYMBOL(arm_smccc_v1_2_smc)
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 62c54234576c..0b8fa285a054 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -186,6 +186,56 @@ struct arm_smccc_res {
 	unsigned long a3;
 };
 
+#ifdef CONFIG_ARM64
+/* TODO Need to implement for ARM too */
+/**
+ * struct arm_smccc_v1_2_res - Result from SMC/HVC call
+ * @a0-a7 result values from registers 0 to 7
+ */
+struct arm_smccc_v1_2_res {
+	unsigned long a0;
+	unsigned long a1;
+	unsigned long a2;
+	unsigned long a3;
+	unsigned long a4;
+	unsigned long a5;
+	unsigned long a6;
+	unsigned long a7;
+};
+
+/**
+ * arm_smccc_v1_2_hvc() - make HVC calls
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 7
+ *
+ * This function is used to make HVC calls following SMC Calling Convention
+ * v1.2 or above. The content of the supplied param are copied to registers
+ * 0 to 7 prior to the HVC instruction. The return values are updated with
+ * the content from register 0 to 7 on return from the HVC instruction.
+ */
+asmlinkage
+void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2,
+			unsigned long a3, unsigned long a4, unsigned long a5,
+			unsigned long a6, unsigned long a7,
+			struct arm_smccc_v1_2_res  *res);
+
+/**
+ * arm_smccc_v1_2_smc() - make SMC calls
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 7
+ *
+ * This function is used to make SMC calls following SMC Calling Convention
+ * v1.2 or above. The content of the supplied param are copied to registers
+ * 0 to 7 prior to the SMC instruction. The return values are updated with
+ * the content from register 0 to 7 on return from the SMC instruction.
+ */
+asmlinkage
+void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2,
+			unsigned long a3, unsigned long a4, unsigned long a5,
+			unsigned long a6, unsigned long a7,
+			struct arm_smccc_v1_2_res  *res);
+#endif
+
 /**
  * struct arm_smccc_quirk - Contains quirk information
  * @id: quirk identification
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici,
	Catalin Marinas, Will Deacon

SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
to be used as result registers in SMC64/HVC64. Arm Firmware Framework
for Armv8-A specification makes use of x0-x7 as parameter and result
registers.

Current SMCCC interface in the kernel just use x0-x7 as parameter and
x0-x3 as result registers. Let us add new interface to support x0-x7
as parameter and result registers. This can be extended to include
x8-x17 when there are users for the same.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm64/kernel/asm-offsets.c |  4 +++
 arch/arm64/kernel/smccc-call.S  | 22 +++++++++++++++
 include/linux/arm-smccc.h       | 50 +++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

Hi Will/Catalin,

I seemed to have missed you on these patches. I realised only when I was
collecting Acks to get this series merged. The change here is simple, it
would be good if you can review and ack if you are OK with it so that I
can get the serier merged via ARM SoC.

diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index a36e2fc330d4..57ffea2920b8 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -132,6 +132,10 @@ int main(void)
   DEFINE(ARM_SMCCC_RES_X2_OFFS,		offsetof(struct arm_smccc_res, a2));
   DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,	offsetof(struct arm_smccc_quirk, id));
   DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS,	offsetof(struct arm_smccc_quirk, state));
+  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
+  DEFINE(ARM_SMCCC_V1_2_RES_X2_OFFS,	offsetof(struct arm_smccc_v1_2_res, a2));
+  DEFINE(ARM_SMCCC_V1_2_RES_X4_OFFS,	offsetof(struct arm_smccc_v1_2_res, a4));
+  DEFINE(ARM_SMCCC_V1_2_RES_X6_OFFS,	offsetof(struct arm_smccc_v1_2_res, a6));
   BLANK();
   DEFINE(HIBERN_PBE_ORIG,	offsetof(struct pbe, orig_address));
   DEFINE(HIBERN_PBE_ADDR,	offsetof(struct pbe, address));
diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
index d62447964ed9..0ea15c1742f3 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc)
 	SMCCC	hvc
 SYM_FUNC_END(__arm_smccc_hvc)
 EXPORT_SYMBOL(__arm_smccc_hvc)
+
+	.macro SMCCC_v1_2 instr
+	.cfi_startproc
+	\instr #0
+	ldr x8, [sp]
+	stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS]
+	stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS]
+	stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS]
+	stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS]
+	ret
+	.cfi_endproc
+.endm
+
+SYM_FUNC_START(arm_smccc_v1_2_hvc)
+	SMCCC_v1_2 hvc
+SYM_FUNC_END(arm_smccc_v1_2_hvc)
+EXPORT_SYMBOL(arm_smccc_v1_2_hvc)
+
+SYM_FUNC_START(arm_smccc_v1_2_smc)
+	SMCCC_v1_2 smc
+SYM_FUNC_END(arm_smccc_v1_2_smc)
+EXPORT_SYMBOL(arm_smccc_v1_2_smc)
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
index 62c54234576c..0b8fa285a054 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -186,6 +186,56 @@ struct arm_smccc_res {
 	unsigned long a3;
 };
 
+#ifdef CONFIG_ARM64
+/* TODO Need to implement for ARM too */
+/**
+ * struct arm_smccc_v1_2_res - Result from SMC/HVC call
+ * @a0-a7 result values from registers 0 to 7
+ */
+struct arm_smccc_v1_2_res {
+	unsigned long a0;
+	unsigned long a1;
+	unsigned long a2;
+	unsigned long a3;
+	unsigned long a4;
+	unsigned long a5;
+	unsigned long a6;
+	unsigned long a7;
+};
+
+/**
+ * arm_smccc_v1_2_hvc() - make HVC calls
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 7
+ *
+ * This function is used to make HVC calls following SMC Calling Convention
+ * v1.2 or above. The content of the supplied param are copied to registers
+ * 0 to 7 prior to the HVC instruction. The return values are updated with
+ * the content from register 0 to 7 on return from the HVC instruction.
+ */
+asmlinkage
+void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2,
+			unsigned long a3, unsigned long a4, unsigned long a5,
+			unsigned long a6, unsigned long a7,
+			struct arm_smccc_v1_2_res  *res);
+
+/**
+ * arm_smccc_v1_2_smc() - make SMC calls
+ * @a0-a7: arguments passed in registers 0 to 7
+ * @res: result values from registers 0 to 7
+ *
+ * This function is used to make SMC calls following SMC Calling Convention
+ * v1.2 or above. The content of the supplied param are copied to registers
+ * 0 to 7 prior to the SMC instruction. The return values are updated with
+ * the content from register 0 to 7 on return from the SMC instruction.
+ */
+asmlinkage
+void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2,
+			unsigned long a3, unsigned long a4, unsigned long a5,
+			unsigned long a6, unsigned long a7,
+			struct arm_smccc_v1_2_res  *res);
+#endif
+
 /**
  * struct arm_smccc_quirk - Contains quirk information
  * @id: quirk identification
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 3/7] firmware: arm_ffa: Add initial FFA bus support for device enumeration
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

The Arm FF for Armv8-A specification has concept of endpoints or
partitions. In the Normal world, a partition could be a VM when
the Virtualization extension is enabled or the kernel itself.

In order to handle multiple partitions, we can create a FFA device for
each such partition on a dedicated FFA bus. Similarly, different drivers
requiring FFA transport can be registered on the same bus. We can match
the device and drivers using UUID. This is mostly for the in-kernel
users with FFA drivers.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/Kconfig          |   1 +
 drivers/firmware/Makefile         |   1 +
 drivers/firmware/arm_ffa/Kconfig  |  16 +++
 drivers/firmware/arm_ffa/Makefile |   4 +
 drivers/firmware/arm_ffa/bus.c    | 187 ++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h           |  91 +++++++++++++++
 6 files changed, 300 insertions(+)
 create mode 100644 drivers/firmware/arm_ffa/Kconfig
 create mode 100644 drivers/firmware/arm_ffa/Makefile
 create mode 100644 drivers/firmware/arm_ffa/bus.c
 create mode 100644 include/linux/arm_ffa.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 3f14dffb9669..8a11d24d3e4b 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -295,6 +295,7 @@ config TURRIS_MOX_RWTM
 	  other manufacturing data and also utilize the Entropy Bit Generator
 	  for hardware random number generation.
 
+source "drivers/firmware/arm_ffa/Kconfig"
 source "drivers/firmware/broadcom/Kconfig"
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 5e013b6a3692..546ac8e7f6d0 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 obj-$(CONFIG_TURRIS_MOX_RWTM)	+= turris-mox-rwtm.o
 
+obj-y				+= arm_ffa/
 obj-y				+= arm_scmi/
 obj-y				+= broadcom/
 obj-y				+= meson/
diff --git a/drivers/firmware/arm_ffa/Kconfig b/drivers/firmware/arm_ffa/Kconfig
new file mode 100644
index 000000000000..261a3660650a
--- /dev/null
+++ b/drivers/firmware/arm_ffa/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config ARM_FFA_TRANSPORT
+	tristate "Arm Firmware Framework for Armv8-A"
+	depends on OF
+	depends on ARM64
+	default n
+	help
+	  This Firmware Framework(FF) for Arm A-profile processors describes
+	  interfaces that standardize communication between the various
+	  software images which includes communication between images in
+	  the Secure world and Normal world. It also leverages the
+	  virtualization extension to isolate software images provided
+	  by an ecosystem of vendors from each other.
+
+	  This driver provides interface for all the client drivers making
+	  use of the features offered by ARM FF-A.
diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
new file mode 100644
index 000000000000..bfe4323a8784
--- /dev/null
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+ffa-bus-y = bus.o
+ffa-module-objs := $(ffa-bus-y)
+obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
new file mode 100644
index 000000000000..3b01a3586f7b
--- /dev/null
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/arm_ffa.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+static int ffa_device_match(struct device *dev, struct device_driver *drv)
+{
+	const struct ffa_device_id *id_table;
+	struct ffa_device *ffa_dev;
+
+	id_table = to_ffa_driver(drv)->id_table;
+	ffa_dev = to_ffa_dev(dev);
+
+	while (!uuid_is_null(&id_table->uuid)) {
+		if (uuid_equal(&ffa_dev->uuid, &id_table->uuid))
+			return 1;
+		id_table++;
+	}
+
+	return 0;
+}
+
+static int ffa_device_probe(struct device *dev)
+{
+	struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver);
+	struct ffa_device *ffa_dev = to_ffa_dev(dev);
+
+	if (!ffa_device_match(dev, dev->driver))
+		return -ENODEV;
+
+	return ffa_drv->probe(ffa_dev);
+}
+
+static int ffa_device_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	uuid_t *dev_id = &to_ffa_dev(dev)->uuid;
+
+	return add_uevent_var(env, "MODALIAS=arm_ffa:%pUb", dev_id);
+}
+
+struct bus_type ffa_bus_type = {
+	.name		= "arm_ffa",
+	.match		= ffa_device_match,
+	.probe		= ffa_device_probe,
+	.uevent		= ffa_device_uevent,
+};
+EXPORT_SYMBOL_GPL(ffa_bus_type);
+
+int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
+			const char *mod_name)
+{
+	int ret;
+
+	driver->driver.bus = &ffa_bus_type;
+	driver->driver.name = driver->name;
+	driver->driver.owner = owner;
+	driver->driver.mod_name = mod_name;
+
+	ret = driver_register(&driver->driver);
+	if (!ret)
+		pr_debug("registered new ffa driver %s\n", driver->name);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ffa_driver_register);
+
+void ffa_driver_unregister(struct ffa_driver *driver)
+{
+	driver_unregister(&driver->driver);
+}
+EXPORT_SYMBOL_GPL(ffa_driver_unregister);
+
+static void ffa_release_device(struct device *dev)
+{
+	struct ffa_device *ffa_dev = to_ffa_dev(dev);
+
+	kfree(ffa_dev);
+}
+
+static int __ffa_devices_unregister(struct device *dev, void *data)
+{
+	ffa_release_device(dev);
+
+	return 0;
+}
+
+static void ffa_devices_unregister(void)
+{
+	bus_for_each_dev(&ffa_bus_type, NULL, NULL,
+			 __ffa_devices_unregister);
+}
+
+bool ffa_device_is_valid(struct ffa_device *ffa_dev)
+{
+	bool valid = false;
+	struct device *dev;
+	struct ffa_device *tmp_dev;
+
+	do {
+		dev = bus_find_next_device(&ffa_bus_type, NULL);
+		tmp_dev = to_ffa_dev(dev);
+		if (tmp_dev == ffa_dev) {
+			valid = true;
+			break;
+		}
+		put_device(dev);
+	} while (dev);
+
+	put_device(dev);
+
+	return valid;
+}
+
+struct ffa_device *ffa_device_register(const char *name, int vm_id)
+{
+	int ret;
+	struct device *dev;
+	struct ffa_device *ffa_dev;
+	uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+
+	ffa_dev = kzalloc(sizeof(*ffa_dev), GFP_KERNEL);
+	if (!ffa_dev)
+		return NULL;
+
+	dev = &ffa_dev->dev;
+	dev->bus = &ffa_bus_type;
+	dev->release = ffa_release_device;
+	dev_set_name(&ffa_dev->dev, name);
+
+	ffa_dev->vm_id = vm_id;
+	if (uuid_parse(name, &uuid)) {
+		dev_err(dev, "invalid uuid (%s)\n", name);
+		kfree(ffa_dev);
+		return NULL;
+	}
+
+	uuid_copy(&ffa_dev->uuid, &uuid);
+
+	ret = device_register(&ffa_dev->dev);
+	if (ret) {
+		dev_err(dev, "unable to register device %s err=%d\n",
+			dev_name(dev), ret);
+		put_device(dev);
+		return NULL;
+	}
+
+	return ffa_dev;
+}
+EXPORT_SYMBOL_GPL(ffa_device_register);
+
+void ffa_device_unregister(struct ffa_device *ffa_dev)
+{
+	if (!ffa_dev)
+		return;
+
+	device_unregister(&ffa_dev->dev);
+}
+EXPORT_SYMBOL_GPL(ffa_device_unregister);
+
+static int __init arm_ffa_bus_init(void)
+{
+	return bus_register(&ffa_bus_type);
+}
+module_init(arm_ffa_bus_init);
+
+static void __exit arm_ffa_bus_exit(void)
+{
+	ffa_devices_unregister();
+	bus_unregister(&ffa_bus_type);
+}
+
+module_exit(arm_ffa_bus_exit);
+
+MODULE_ALIAS("arm-ffa-bus");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("Arm FF-A bus driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
new file mode 100644
index 000000000000..43f9c0df4114
--- /dev/null
+++ b/include/linux/arm_ffa.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#ifndef _LINUX_ARM_FFA_H
+#define _LINUX_ARM_FFA_H
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+/* FFA Bus/Device/Driver related */
+struct ffa_device {
+	int vm_id;
+	uuid_t uuid;
+	struct device dev;
+};
+
+#define to_ffa_dev(d) container_of(d, struct ffa_device, dev)
+
+struct ffa_device_id {
+	uuid_t uuid;
+};
+
+struct ffa_driver {
+	const char *name;
+	int (*probe)(struct ffa_device *sdev);
+	void (*remove)(struct ffa_device *sdev);
+	const struct ffa_device_id *id_table;
+
+	struct device_driver driver;
+};
+
+#define to_ffa_driver(d) container_of(d, struct ffa_driver, driver)
+
+static inline void ffa_dev_set_drvdata(struct ffa_device *fdev, void *data)
+{
+	fdev->dev.driver_data = data;
+}
+
+#if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
+struct ffa_device *ffa_device_register(const char *name, int vm_id);
+void ffa_device_unregister(struct ffa_device *ffa_dev);
+int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
+			const char *mod_name);
+void ffa_driver_unregister(struct ffa_driver *driver);
+bool ffa_device_is_valid(struct ffa_device *ffa_dev);
+
+#else
+static inline
+struct ffa_device *ffa_device_register(const char *name, int vm_id)
+{
+	return NULL;
+}
+
+static inline void ffa_device_unregister(struct ffa_device *dev) {}
+
+static inline int
+ffa_driver_register(struct ffa_driver *driver, struct module *owner,
+		    const char *mod_name)
+{
+	return -EINVAL;
+}
+
+static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
+
+static inline
+bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
+
+#endif /* CONFIG_ARM_FFA_TRANSPORT */
+
+#define ffa_register(driver) \
+	ffa_driver_register(driver, THIS_MODULE, KBUILD_MODNAME)
+#define ffa_unregister(driver) \
+	ffa_driver_unregister(driver)
+
+/**
+ * module_ffa_driver() - Helper macro for registering a psa_ffa driver
+ * @__ffa_driver: ffa_driver structure
+ *
+ * Helper macro for psa_ffa drivers to set up proper module init / exit
+ * functions.  Replaces module_init() and module_exit() and keeps people from
+ * printing pointless things to the kernel log when their driver is loaded.
+ */
+#define module_ffa_driver(__ffa_driver)	\
+	module_driver(__ffa_driver, ffa_register, ffa_unregister)
+
+#endif /* _LINUX_ARM_FFA_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 3/7] firmware: arm_ffa: Add initial FFA bus support for device enumeration
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

The Arm FF for Armv8-A specification has concept of endpoints or
partitions. In the Normal world, a partition could be a VM when
the Virtualization extension is enabled or the kernel itself.

In order to handle multiple partitions, we can create a FFA device for
each such partition on a dedicated FFA bus. Similarly, different drivers
requiring FFA transport can be registered on the same bus. We can match
the device and drivers using UUID. This is mostly for the in-kernel
users with FFA drivers.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/Kconfig          |   1 +
 drivers/firmware/Makefile         |   1 +
 drivers/firmware/arm_ffa/Kconfig  |  16 +++
 drivers/firmware/arm_ffa/Makefile |   4 +
 drivers/firmware/arm_ffa/bus.c    | 187 ++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h           |  91 +++++++++++++++
 6 files changed, 300 insertions(+)
 create mode 100644 drivers/firmware/arm_ffa/Kconfig
 create mode 100644 drivers/firmware/arm_ffa/Makefile
 create mode 100644 drivers/firmware/arm_ffa/bus.c
 create mode 100644 include/linux/arm_ffa.h

diff --git a/drivers/firmware/Kconfig b/drivers/firmware/Kconfig
index 3f14dffb9669..8a11d24d3e4b 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -295,6 +295,7 @@ config TURRIS_MOX_RWTM
 	  other manufacturing data and also utilize the Entropy Bit Generator
 	  for hardware random number generation.
 
+source "drivers/firmware/arm_ffa/Kconfig"
 source "drivers/firmware/broadcom/Kconfig"
 source "drivers/firmware/google/Kconfig"
 source "drivers/firmware/efi/Kconfig"
diff --git a/drivers/firmware/Makefile b/drivers/firmware/Makefile
index 5e013b6a3692..546ac8e7f6d0 100644
--- a/drivers/firmware/Makefile
+++ b/drivers/firmware/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_TI_SCI_PROTOCOL)	+= ti_sci.o
 obj-$(CONFIG_TRUSTED_FOUNDATIONS) += trusted_foundations.o
 obj-$(CONFIG_TURRIS_MOX_RWTM)	+= turris-mox-rwtm.o
 
+obj-y				+= arm_ffa/
 obj-y				+= arm_scmi/
 obj-y				+= broadcom/
 obj-y				+= meson/
diff --git a/drivers/firmware/arm_ffa/Kconfig b/drivers/firmware/arm_ffa/Kconfig
new file mode 100644
index 000000000000..261a3660650a
--- /dev/null
+++ b/drivers/firmware/arm_ffa/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config ARM_FFA_TRANSPORT
+	tristate "Arm Firmware Framework for Armv8-A"
+	depends on OF
+	depends on ARM64
+	default n
+	help
+	  This Firmware Framework(FF) for Arm A-profile processors describes
+	  interfaces that standardize communication between the various
+	  software images which includes communication between images in
+	  the Secure world and Normal world. It also leverages the
+	  virtualization extension to isolate software images provided
+	  by an ecosystem of vendors from each other.
+
+	  This driver provides interface for all the client drivers making
+	  use of the features offered by ARM FF-A.
diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
new file mode 100644
index 000000000000..bfe4323a8784
--- /dev/null
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -0,0 +1,4 @@
+# SPDX-License-Identifier: GPL-2.0-only
+ffa-bus-y = bus.o
+ffa-module-objs := $(ffa-bus-y)
+obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
new file mode 100644
index 000000000000..3b01a3586f7b
--- /dev/null
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -0,0 +1,187 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/arm_ffa.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+static int ffa_device_match(struct device *dev, struct device_driver *drv)
+{
+	const struct ffa_device_id *id_table;
+	struct ffa_device *ffa_dev;
+
+	id_table = to_ffa_driver(drv)->id_table;
+	ffa_dev = to_ffa_dev(dev);
+
+	while (!uuid_is_null(&id_table->uuid)) {
+		if (uuid_equal(&ffa_dev->uuid, &id_table->uuid))
+			return 1;
+		id_table++;
+	}
+
+	return 0;
+}
+
+static int ffa_device_probe(struct device *dev)
+{
+	struct ffa_driver *ffa_drv = to_ffa_driver(dev->driver);
+	struct ffa_device *ffa_dev = to_ffa_dev(dev);
+
+	if (!ffa_device_match(dev, dev->driver))
+		return -ENODEV;
+
+	return ffa_drv->probe(ffa_dev);
+}
+
+static int ffa_device_uevent(struct device *dev, struct kobj_uevent_env *env)
+{
+	uuid_t *dev_id = &to_ffa_dev(dev)->uuid;
+
+	return add_uevent_var(env, "MODALIAS=arm_ffa:%pUb", dev_id);
+}
+
+struct bus_type ffa_bus_type = {
+	.name		= "arm_ffa",
+	.match		= ffa_device_match,
+	.probe		= ffa_device_probe,
+	.uevent		= ffa_device_uevent,
+};
+EXPORT_SYMBOL_GPL(ffa_bus_type);
+
+int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
+			const char *mod_name)
+{
+	int ret;
+
+	driver->driver.bus = &ffa_bus_type;
+	driver->driver.name = driver->name;
+	driver->driver.owner = owner;
+	driver->driver.mod_name = mod_name;
+
+	ret = driver_register(&driver->driver);
+	if (!ret)
+		pr_debug("registered new ffa driver %s\n", driver->name);
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(ffa_driver_register);
+
+void ffa_driver_unregister(struct ffa_driver *driver)
+{
+	driver_unregister(&driver->driver);
+}
+EXPORT_SYMBOL_GPL(ffa_driver_unregister);
+
+static void ffa_release_device(struct device *dev)
+{
+	struct ffa_device *ffa_dev = to_ffa_dev(dev);
+
+	kfree(ffa_dev);
+}
+
+static int __ffa_devices_unregister(struct device *dev, void *data)
+{
+	ffa_release_device(dev);
+
+	return 0;
+}
+
+static void ffa_devices_unregister(void)
+{
+	bus_for_each_dev(&ffa_bus_type, NULL, NULL,
+			 __ffa_devices_unregister);
+}
+
+bool ffa_device_is_valid(struct ffa_device *ffa_dev)
+{
+	bool valid = false;
+	struct device *dev;
+	struct ffa_device *tmp_dev;
+
+	do {
+		dev = bus_find_next_device(&ffa_bus_type, NULL);
+		tmp_dev = to_ffa_dev(dev);
+		if (tmp_dev == ffa_dev) {
+			valid = true;
+			break;
+		}
+		put_device(dev);
+	} while (dev);
+
+	put_device(dev);
+
+	return valid;
+}
+
+struct ffa_device *ffa_device_register(const char *name, int vm_id)
+{
+	int ret;
+	struct device *dev;
+	struct ffa_device *ffa_dev;
+	uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+
+	ffa_dev = kzalloc(sizeof(*ffa_dev), GFP_KERNEL);
+	if (!ffa_dev)
+		return NULL;
+
+	dev = &ffa_dev->dev;
+	dev->bus = &ffa_bus_type;
+	dev->release = ffa_release_device;
+	dev_set_name(&ffa_dev->dev, name);
+
+	ffa_dev->vm_id = vm_id;
+	if (uuid_parse(name, &uuid)) {
+		dev_err(dev, "invalid uuid (%s)\n", name);
+		kfree(ffa_dev);
+		return NULL;
+	}
+
+	uuid_copy(&ffa_dev->uuid, &uuid);
+
+	ret = device_register(&ffa_dev->dev);
+	if (ret) {
+		dev_err(dev, "unable to register device %s err=%d\n",
+			dev_name(dev), ret);
+		put_device(dev);
+		return NULL;
+	}
+
+	return ffa_dev;
+}
+EXPORT_SYMBOL_GPL(ffa_device_register);
+
+void ffa_device_unregister(struct ffa_device *ffa_dev)
+{
+	if (!ffa_dev)
+		return;
+
+	device_unregister(&ffa_dev->dev);
+}
+EXPORT_SYMBOL_GPL(ffa_device_unregister);
+
+static int __init arm_ffa_bus_init(void)
+{
+	return bus_register(&ffa_bus_type);
+}
+module_init(arm_ffa_bus_init);
+
+static void __exit arm_ffa_bus_exit(void)
+{
+	ffa_devices_unregister();
+	bus_unregister(&ffa_bus_type);
+}
+
+module_exit(arm_ffa_bus_exit);
+
+MODULE_ALIAS("arm-ffa-bus");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("Arm FF-A bus driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
new file mode 100644
index 000000000000..43f9c0df4114
--- /dev/null
+++ b/include/linux/arm_ffa.h
@@ -0,0 +1,91 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#ifndef _LINUX_ARM_FFA_H
+#define _LINUX_ARM_FFA_H
+
+#include <linux/cdev.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+/* FFA Bus/Device/Driver related */
+struct ffa_device {
+	int vm_id;
+	uuid_t uuid;
+	struct device dev;
+};
+
+#define to_ffa_dev(d) container_of(d, struct ffa_device, dev)
+
+struct ffa_device_id {
+	uuid_t uuid;
+};
+
+struct ffa_driver {
+	const char *name;
+	int (*probe)(struct ffa_device *sdev);
+	void (*remove)(struct ffa_device *sdev);
+	const struct ffa_device_id *id_table;
+
+	struct device_driver driver;
+};
+
+#define to_ffa_driver(d) container_of(d, struct ffa_driver, driver)
+
+static inline void ffa_dev_set_drvdata(struct ffa_device *fdev, void *data)
+{
+	fdev->dev.driver_data = data;
+}
+
+#if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
+struct ffa_device *ffa_device_register(const char *name, int vm_id);
+void ffa_device_unregister(struct ffa_device *ffa_dev);
+int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
+			const char *mod_name);
+void ffa_driver_unregister(struct ffa_driver *driver);
+bool ffa_device_is_valid(struct ffa_device *ffa_dev);
+
+#else
+static inline
+struct ffa_device *ffa_device_register(const char *name, int vm_id)
+{
+	return NULL;
+}
+
+static inline void ffa_device_unregister(struct ffa_device *dev) {}
+
+static inline int
+ffa_driver_register(struct ffa_driver *driver, struct module *owner,
+		    const char *mod_name)
+{
+	return -EINVAL;
+}
+
+static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
+
+static inline
+bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
+
+#endif /* CONFIG_ARM_FFA_TRANSPORT */
+
+#define ffa_register(driver) \
+	ffa_driver_register(driver, THIS_MODULE, KBUILD_MODNAME)
+#define ffa_unregister(driver) \
+	ffa_driver_unregister(driver)
+
+/**
+ * module_ffa_driver() - Helper macro for registering a psa_ffa driver
+ * @__ffa_driver: ffa_driver structure
+ *
+ * Helper macro for psa_ffa drivers to set up proper module init / exit
+ * functions.  Replaces module_init() and module_exit() and keeps people from
+ * printing pointless things to the kernel log when their driver is loaded.
+ */
+#define module_ffa_driver(__ffa_driver)	\
+	module_driver(__ffa_driver, ffa_register, ffa_unregister)
+
+#endif /* _LINUX_ARM_FFA_H */
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 4/7] firmware: arm_ffa: Add initial Arm FFA driver support
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

This just add a basic driver that sets up the transport(e.g. SMCCC),
checks the FFA version implemented, get the partition ID for self and
sets up the Tx/Rx buffers for communication.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/Makefile |   3 +-
 drivers/firmware/arm_ffa/bus.c    |  14 +-
 drivers/firmware/arm_ffa/common.h |  26 +++
 drivers/firmware/arm_ffa/driver.c | 300 ++++++++++++++++++++++++++++++
 4 files changed, 332 insertions(+), 11 deletions(-)
 create mode 100644 drivers/firmware/arm_ffa/common.h
 create mode 100644 drivers/firmware/arm_ffa/driver.c

diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
index bfe4323a8784..82d0d35c5324 100644
--- a/drivers/firmware/arm_ffa/Makefile
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 ffa-bus-y = bus.o
-ffa-module-objs := $(ffa-bus-y)
+ffa-driver-y = driver.o
+ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y)
 obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index 3b01a3586f7b..9debdc87111c 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -13,6 +13,8 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include "common.h"
+
 static int ffa_device_match(struct device *dev, struct device_driver *drv)
 {
 	const struct ffa_device_id *id_table;
@@ -167,21 +169,13 @@ void ffa_device_unregister(struct ffa_device *ffa_dev)
 }
 EXPORT_SYMBOL_GPL(ffa_device_unregister);
 
-static int __init arm_ffa_bus_init(void)
+int __init arm_ffa_bus_init(void)
 {
 	return bus_register(&ffa_bus_type);
 }
-module_init(arm_ffa_bus_init);
 
-static void __exit arm_ffa_bus_exit(void)
+void __exit arm_ffa_bus_exit(void)
 {
 	ffa_devices_unregister();
 	bus_unregister(&ffa_bus_type);
 }
-
-module_exit(arm_ffa_bus_exit);
-
-MODULE_ALIAS("arm-ffa-bus");
-MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
-MODULE_DESCRIPTION("Arm FF-A bus driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
new file mode 100644
index 000000000000..fc6948efe9f3
--- /dev/null
+++ b/drivers/firmware/arm_ffa/common.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#ifndef _FFA_COMMON_H
+#define _FFA_COMMON_H
+
+#include <linux/arm-smccc.h>
+#include <linux/err.h>
+
+typedef struct arm_smccc_v1_2_res ffa_res_t;
+
+typedef ffa_res_t
+(ffa_fn)(unsigned long, unsigned long, unsigned long, unsigned long,
+	 unsigned long, unsigned long, unsigned long, unsigned long);
+
+int __init arm_ffa_bus_init(void);
+void __exit arm_ffa_bus_exit(void);
+
+static inline int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
+{
+	return -EOPNOTSUPP;
+}
+
+#endif /* _FFA_COMMON_H */
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
new file mode 100644
index 000000000000..6d5094ecc0f6
--- /dev/null
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Arm Firmware Framework for ARMv8-A(FFA) interface driver
+ *
+ * The Arm FFA specification[1] describes a software architecture to
+ * leverages the virtualization extension to isolate software images
+ * provided by an ecosystem of vendors from each other and describes
+ * interfaces that standardize communication between the various software
+ * images including communication between images in the Secure world and
+ * Normal world. Any Hypervisor could use the FFA interfaces to enable
+ * communication between VMs it manages.
+ *
+ * The Hypervisor a.k.a Partition managers in FFA terminology can assign
+ * system resources(Memory regions, Devices, CPU cycles) to the partitions
+ * and manage isolation amongst them.
+ *
+ * [1] https://developer.arm.com/docs/den0077/latest
+ *
+ * Copyright (C) 2020 Arm Ltd.
+ */
+
+#define DRIVER_NAME "ARM FF-A"
+#define pr_fmt(fmt) DRIVER_NAME ": " fmt
+
+#include <linux/arm_ffa.h>
+#include <linux/bitfield.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "common.h"
+
+#define FFA_DRIVER_VERSION	FFA_VERSION_1_0
+
+#define FFA_SMC(calling_convention, func_num)				\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, (calling_convention),	\
+			   ARM_SMCCC_OWNER_STANDARD, (func_num))
+
+#define FFA_SMC_32(func_num)	FFA_SMC(ARM_SMCCC_SMC_32, (func_num))
+#define FFA_SMC_64(func_num)	FFA_SMC(ARM_SMCCC_SMC_64, (func_num))
+
+#define FFA_ERROR			FFA_SMC_32(0x60)
+#define FFA_SUCCESS			FFA_SMC_32(0x61)
+#define FFA_INTERRUPT			FFA_SMC_32(0x62)
+#define FFA_VERSION			FFA_SMC_32(0x63)
+#define FFA_FEATURES			FFA_SMC_32(0x64)
+#define FFA_RX_RELEASE			FFA_SMC_32(0x65)
+#define FFA_RXTX_MAP			FFA_SMC_32(0x66)
+#define FFA_FN64_RXTX_MAP		FFA_SMC_64(0x66)
+#define FFA_RXTX_UNMAP			FFA_SMC_32(0x67)
+#define FFA_PARTITION_INFO_GET		FFA_SMC_32(0x68)
+#define FFA_ID_GET			FFA_SMC_32(0x69)
+#define FFA_MSG_POLL			FFA_SMC_32(0x6A)
+#define FFA_MSG_WAIT			FFA_SMC_32(0x6B)
+#define FFA_YIELD			FFA_SMC_32(0x6C)
+#define FFA_RUN				FFA_SMC_32(0x6D)
+#define FFA_MSG_SEND			FFA_SMC_32(0x6E)
+#define FFA_MSG_SEND_DIRECT_REQ		FFA_SMC_32(0x6F)
+#define FFA_FN64_MSG_SEND_DIRECT_REQ	FFA_SMC_64(0x6F)
+#define FFA_MSG_SEND_DIRECT_RESP	FFA_SMC_32(0x70)
+#define FFA_FN64_MSG_SEND_DIRECT_RESP	FFA_SMC_64(0x70)
+#define FFA_MEM_DONATE			FFA_SMC_32(0x71)
+#define FFA_FN64_MEM_DONATE		FFA_SMC_64(0x71)
+#define FFA_MEM_LEND			FFA_SMC_32(0x72)
+#define FFA_FN64_MEM_LEND		FFA_SMC_64(0x72)
+#define FFA_MEM_SHARE			FFA_SMC_32(0x73)
+#define FFA_FN64_MEM_SHARE		FFA_SMC_64(0x73)
+#define FFA_MEM_RETRIEVE_REQ		FFA_SMC_32(0x74)
+#define FFA_FN64_MEM_RETRIEVE_REQ	FFA_SMC_64(0x74)
+#define FFA_MEM_RETRIEVE_RESP		FFA_SMC_32(0x75)
+#define FFA_MEM_RELINQUISH		FFA_SMC_32(0x76)
+#define FFA_MEM_RECLAIM			FFA_SMC_32(0x77)
+#define FFA_MEM_OP_PAUSE		FFA_SMC_32(0x78)
+#define FFA_MEM_OP_RESUME		FFA_SMC_32(0x79)
+#define FFA_MEM_FRAG_RX			FFA_SMC_32(0x7A)
+#define FFA_MEM_FRAG_TX			FFA_SMC_32(0x7B)
+#define FFA_NORMAL_WORLD_RESUME		FFA_SMC_32(0x7C)
+
+/*
+ * For some calls it is necessary to use SMC64 to pass or return 64-bit values.
+ * For such calls FFA_FN_NATIVE(name) will choose the appropriate
+ * (native-width) function ID.
+ */
+#ifdef CONFIG_64BIT
+#define FFA_FN_NATIVE(name)	FFA_FN64_##name
+#else
+#define FFA_FN_NATIVE(name)	FFA_##name
+#endif
+
+/* FFA error codes. */
+#define FFA_RET_SUCCESS            (0)
+#define FFA_RET_NOT_SUPPORTED      (-1)
+#define FFA_RET_INVALID_PARAMETERS (-2)
+#define FFA_RET_NO_MEMORY          (-3)
+#define FFA_RET_BUSY               (-4)
+#define FFA_RET_INTERRUPTED        (-5)
+#define FFA_RET_DENIED             (-6)
+#define FFA_RET_RETRY              (-7)
+#define FFA_RET_ABORTED            (-8)
+
+#define MAJOR_VERSION_MASK	GENMASK(30, 16)
+#define MINOR_VERSION_MASK	GENMASK(15, 0)
+#define MAJOR_VERSION(x)	(u16)(FIELD_GET(MAJOR_VERSION_MASK, (x)))
+#define MINOR_VERSION(x)	(u16)(FIELD_GET(MINOR_VERSION_MASK, (x)))
+#define PACK_VERSION_INFO(major, minor)			\
+	(FIELD_PREP(MAJOR_VERSION_MASK, (major)) |	\
+	 FIELD_PREP(MINOR_VERSION_MASK, (minor)))
+#define FFA_VERSION_1_0		PACK_VERSION_INFO(1, 0)
+#define FFA_MIN_VERSION		FFA_VERSION_1_0
+
+#define SENDER_ID_MASK		GENMASK(31, 16)
+#define RECEIVER_ID_MASK	GENMASK(15, 0)
+#define SENDER_ID(x)		(u16)(FIELD_GET(SENDER_ID_MASK, (x)))
+#define RECEIVER_ID(x)		(u16)(FIELD_GET(RECEIVER_ID_MASK, (x)))
+#define PACK_TARGET_INFO(s, r)		\
+	(FIELD_PREP(SENDER_ID_MASK, (s)) | FIELD_PREP(RECEIVER_ID_MASK, (r)))
+
+/**
+ * FF-A specification mentions explicitly about '4K pages'. This should
+ * not be confused with the kernel PAGE_SIZE, which is the translation
+ * granule kernel is configured and may be one among 4K, 16K and 64K.
+ */
+#define FFA_PAGE_SIZE		SZ_4K
+/*
+ * Keeping RX TX buffer size as 4K for now
+ * 64K may be preferred to keep it min a page in 64K PAGE_SIZE config
+ */
+#define RXTX_BUFFER_SIZE	SZ_4K
+
+static ffa_fn *invoke_ffa_fn;
+
+static const int ffa_linux_errmap[] = {
+	/* better than switch case as long as return value is continuous */
+	0,		/* FFA_RET_SUCCESS */
+	-EOPNOTSUPP,	/* FFA_RET_NOT_SUPPORTED */
+	-EINVAL,	/* FFA_RET_INVALID_PARAMETERS */
+	-ENOMEM,	/* FFA_RET_NO_MEMORY */
+	-EBUSY,		/* FFA_RET_BUSY */
+	-EINTR,		/* FFA_RET_INTERRUPTED */
+	-EACCES,	/* FFA_RET_DENIED */
+	-EAGAIN,	/* FFA_RET_RETRY */
+	-ECANCELED,	/* FFA_RET_ABORTED */
+};
+
+static inline int ffa_to_linux_errno(int errno)
+{
+	if (errno < FFA_RET_SUCCESS && errno >= -ARRAY_SIZE(ffa_linux_errmap))
+		return ffa_linux_errmap[-errno];
+	return -EINVAL;
+}
+
+struct ffa_drv_info {
+	u32 version;
+	u16 vm_id;
+	struct mutex rx_lock; /* lock to protect Rx buffer */
+	struct mutex tx_lock; /* lock to protect Tx buffer */
+	void *rx_buffer;
+	void *tx_buffer;
+};
+
+static struct ffa_drv_info *drv_info;
+
+static int ffa_version_check(u32 *version)
+{
+	ffa_res_t ver;
+
+	ver = invoke_ffa_fn(FFA_VERSION, FFA_DRIVER_VERSION, 0, 0, 0, 0, 0, 0);
+
+	if (ver.a0 == FFA_RET_NOT_SUPPORTED) {
+		pr_info("FFA_VERSION returned not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (ver.a0 < FFA_MIN_VERSION || ver.a0 > FFA_DRIVER_VERSION) {
+		pr_err("Incompatible version %d.%d found\n",
+		       MAJOR_VERSION(ver.a0), MINOR_VERSION(ver.a0));
+		return -EINVAL;
+	}
+
+	*version = ver.a0;
+	pr_info("Version %d.%d found\n", MAJOR_VERSION(ver.a0),
+		MINOR_VERSION(ver.a0));
+	return 0;
+}
+
+static int ffa_rxtx_map(phys_addr_t tx_buf, phys_addr_t rx_buf, u32 pg_cnt)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_FN_NATIVE(RXTX_MAP), tx_buf, rx_buf, pg_cnt,
+			    0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	return 0;
+}
+
+static int ffa_rxtx_unmap(u16 vm_id)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_RXTX_UNMAP, PACK_TARGET_INFO(vm_id, 0),
+			    0, 0, 0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	return 0;
+}
+
+#define VM_ID_MASK	GENMASK(15, 0)
+static int ffa_id_get(u16 *vm_id)
+{
+	ffa_res_t id;
+
+	id = invoke_ffa_fn(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0);
+
+	if (id.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)id.a2);
+
+	*vm_id = FIELD_GET(VM_ID_MASK, (id.a2));
+
+	return 0;
+}
+
+static int __init ffa_init(void)
+{
+	int ret;
+
+	ret = arm_ffa_bus_init();
+	if (ret)
+		return ret;
+
+	ret = ffa_transport_init(&invoke_ffa_fn);
+	if (ret)
+		return ret;
+
+	drv_info = kzalloc(sizeof(*drv_info), GFP_KERNEL);
+	if (!drv_info)
+		return -ENOMEM;
+
+	ret = ffa_version_check(&drv_info->version);
+	if (ret)
+		goto free_drv_info;
+
+	if (ffa_id_get(&drv_info->vm_id)) {
+		pr_err("failed to obtain VM id for self\n");
+		ret = -ENODEV;
+		goto free_drv_info;
+	}
+
+	drv_info->rx_buffer = alloc_pages_exact(RXTX_BUFFER_SIZE, GFP_KERNEL);
+	if (!drv_info->rx_buffer) {
+		ret = -ENOMEM;
+		goto free_pages;
+	}
+
+	drv_info->tx_buffer = alloc_pages_exact(RXTX_BUFFER_SIZE, GFP_KERNEL);
+	if (!drv_info->tx_buffer) {
+		ret = -ENOMEM;
+		goto free_pages;
+	}
+
+	ret = ffa_rxtx_map(virt_to_phys(drv_info->tx_buffer),
+			   virt_to_phys(drv_info->rx_buffer),
+			   RXTX_BUFFER_SIZE / FFA_PAGE_SIZE);
+	if (ret) {
+		pr_err("failed to register FFA RxTx buffers\n");
+		goto free_pages;
+	}
+
+	mutex_init(&drv_info->rx_lock);
+	mutex_init(&drv_info->tx_lock);
+
+	return 0;
+free_pages:
+	if (drv_info->tx_buffer)
+		free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE);
+	free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE);
+free_drv_info:
+	kfree(drv_info);
+	return ret;
+}
+module_init(ffa_init);
+
+static void __exit ffa_exit(void)
+{
+	ffa_rxtx_unmap(drv_info->vm_id);
+	free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE);
+	free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE);
+	kfree(drv_info);
+	arm_ffa_bus_exit();
+}
+module_exit(ffa_exit);
+
+MODULE_ALIAS("arm-ffa");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("Arm FF-A interface driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 4/7] firmware: arm_ffa: Add initial Arm FFA driver support
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

This just add a basic driver that sets up the transport(e.g. SMCCC),
checks the FFA version implemented, get the partition ID for self and
sets up the Tx/Rx buffers for communication.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/Makefile |   3 +-
 drivers/firmware/arm_ffa/bus.c    |  14 +-
 drivers/firmware/arm_ffa/common.h |  26 +++
 drivers/firmware/arm_ffa/driver.c | 300 ++++++++++++++++++++++++++++++
 4 files changed, 332 insertions(+), 11 deletions(-)
 create mode 100644 drivers/firmware/arm_ffa/common.h
 create mode 100644 drivers/firmware/arm_ffa/driver.c

diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
index bfe4323a8784..82d0d35c5324 100644
--- a/drivers/firmware/arm_ffa/Makefile
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 ffa-bus-y = bus.o
-ffa-module-objs := $(ffa-bus-y)
+ffa-driver-y = driver.o
+ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y)
 obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index 3b01a3586f7b..9debdc87111c 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -13,6 +13,8 @@
 #include <linux/slab.h>
 #include <linux/types.h>
 
+#include "common.h"
+
 static int ffa_device_match(struct device *dev, struct device_driver *drv)
 {
 	const struct ffa_device_id *id_table;
@@ -167,21 +169,13 @@ void ffa_device_unregister(struct ffa_device *ffa_dev)
 }
 EXPORT_SYMBOL_GPL(ffa_device_unregister);
 
-static int __init arm_ffa_bus_init(void)
+int __init arm_ffa_bus_init(void)
 {
 	return bus_register(&ffa_bus_type);
 }
-module_init(arm_ffa_bus_init);
 
-static void __exit arm_ffa_bus_exit(void)
+void __exit arm_ffa_bus_exit(void)
 {
 	ffa_devices_unregister();
 	bus_unregister(&ffa_bus_type);
 }
-
-module_exit(arm_ffa_bus_exit);
-
-MODULE_ALIAS("arm-ffa-bus");
-MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
-MODULE_DESCRIPTION("Arm FF-A bus driver");
-MODULE_LICENSE("GPL v2");
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
new file mode 100644
index 000000000000..fc6948efe9f3
--- /dev/null
+++ b/drivers/firmware/arm_ffa/common.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#ifndef _FFA_COMMON_H
+#define _FFA_COMMON_H
+
+#include <linux/arm-smccc.h>
+#include <linux/err.h>
+
+typedef struct arm_smccc_v1_2_res ffa_res_t;
+
+typedef ffa_res_t
+(ffa_fn)(unsigned long, unsigned long, unsigned long, unsigned long,
+	 unsigned long, unsigned long, unsigned long, unsigned long);
+
+int __init arm_ffa_bus_init(void);
+void __exit arm_ffa_bus_exit(void);
+
+static inline int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
+{
+	return -EOPNOTSUPP;
+}
+
+#endif /* _FFA_COMMON_H */
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
new file mode 100644
index 000000000000..6d5094ecc0f6
--- /dev/null
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -0,0 +1,300 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Arm Firmware Framework for ARMv8-A(FFA) interface driver
+ *
+ * The Arm FFA specification[1] describes a software architecture to
+ * leverages the virtualization extension to isolate software images
+ * provided by an ecosystem of vendors from each other and describes
+ * interfaces that standardize communication between the various software
+ * images including communication between images in the Secure world and
+ * Normal world. Any Hypervisor could use the FFA interfaces to enable
+ * communication between VMs it manages.
+ *
+ * The Hypervisor a.k.a Partition managers in FFA terminology can assign
+ * system resources(Memory regions, Devices, CPU cycles) to the partitions
+ * and manage isolation amongst them.
+ *
+ * [1] https://developer.arm.com/docs/den0077/latest
+ *
+ * Copyright (C) 2020 Arm Ltd.
+ */
+
+#define DRIVER_NAME "ARM FF-A"
+#define pr_fmt(fmt) DRIVER_NAME ": " fmt
+
+#include <linux/arm_ffa.h>
+#include <linux/bitfield.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+
+#include "common.h"
+
+#define FFA_DRIVER_VERSION	FFA_VERSION_1_0
+
+#define FFA_SMC(calling_convention, func_num)				\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, (calling_convention),	\
+			   ARM_SMCCC_OWNER_STANDARD, (func_num))
+
+#define FFA_SMC_32(func_num)	FFA_SMC(ARM_SMCCC_SMC_32, (func_num))
+#define FFA_SMC_64(func_num)	FFA_SMC(ARM_SMCCC_SMC_64, (func_num))
+
+#define FFA_ERROR			FFA_SMC_32(0x60)
+#define FFA_SUCCESS			FFA_SMC_32(0x61)
+#define FFA_INTERRUPT			FFA_SMC_32(0x62)
+#define FFA_VERSION			FFA_SMC_32(0x63)
+#define FFA_FEATURES			FFA_SMC_32(0x64)
+#define FFA_RX_RELEASE			FFA_SMC_32(0x65)
+#define FFA_RXTX_MAP			FFA_SMC_32(0x66)
+#define FFA_FN64_RXTX_MAP		FFA_SMC_64(0x66)
+#define FFA_RXTX_UNMAP			FFA_SMC_32(0x67)
+#define FFA_PARTITION_INFO_GET		FFA_SMC_32(0x68)
+#define FFA_ID_GET			FFA_SMC_32(0x69)
+#define FFA_MSG_POLL			FFA_SMC_32(0x6A)
+#define FFA_MSG_WAIT			FFA_SMC_32(0x6B)
+#define FFA_YIELD			FFA_SMC_32(0x6C)
+#define FFA_RUN				FFA_SMC_32(0x6D)
+#define FFA_MSG_SEND			FFA_SMC_32(0x6E)
+#define FFA_MSG_SEND_DIRECT_REQ		FFA_SMC_32(0x6F)
+#define FFA_FN64_MSG_SEND_DIRECT_REQ	FFA_SMC_64(0x6F)
+#define FFA_MSG_SEND_DIRECT_RESP	FFA_SMC_32(0x70)
+#define FFA_FN64_MSG_SEND_DIRECT_RESP	FFA_SMC_64(0x70)
+#define FFA_MEM_DONATE			FFA_SMC_32(0x71)
+#define FFA_FN64_MEM_DONATE		FFA_SMC_64(0x71)
+#define FFA_MEM_LEND			FFA_SMC_32(0x72)
+#define FFA_FN64_MEM_LEND		FFA_SMC_64(0x72)
+#define FFA_MEM_SHARE			FFA_SMC_32(0x73)
+#define FFA_FN64_MEM_SHARE		FFA_SMC_64(0x73)
+#define FFA_MEM_RETRIEVE_REQ		FFA_SMC_32(0x74)
+#define FFA_FN64_MEM_RETRIEVE_REQ	FFA_SMC_64(0x74)
+#define FFA_MEM_RETRIEVE_RESP		FFA_SMC_32(0x75)
+#define FFA_MEM_RELINQUISH		FFA_SMC_32(0x76)
+#define FFA_MEM_RECLAIM			FFA_SMC_32(0x77)
+#define FFA_MEM_OP_PAUSE		FFA_SMC_32(0x78)
+#define FFA_MEM_OP_RESUME		FFA_SMC_32(0x79)
+#define FFA_MEM_FRAG_RX			FFA_SMC_32(0x7A)
+#define FFA_MEM_FRAG_TX			FFA_SMC_32(0x7B)
+#define FFA_NORMAL_WORLD_RESUME		FFA_SMC_32(0x7C)
+
+/*
+ * For some calls it is necessary to use SMC64 to pass or return 64-bit values.
+ * For such calls FFA_FN_NATIVE(name) will choose the appropriate
+ * (native-width) function ID.
+ */
+#ifdef CONFIG_64BIT
+#define FFA_FN_NATIVE(name)	FFA_FN64_##name
+#else
+#define FFA_FN_NATIVE(name)	FFA_##name
+#endif
+
+/* FFA error codes. */
+#define FFA_RET_SUCCESS            (0)
+#define FFA_RET_NOT_SUPPORTED      (-1)
+#define FFA_RET_INVALID_PARAMETERS (-2)
+#define FFA_RET_NO_MEMORY          (-3)
+#define FFA_RET_BUSY               (-4)
+#define FFA_RET_INTERRUPTED        (-5)
+#define FFA_RET_DENIED             (-6)
+#define FFA_RET_RETRY              (-7)
+#define FFA_RET_ABORTED            (-8)
+
+#define MAJOR_VERSION_MASK	GENMASK(30, 16)
+#define MINOR_VERSION_MASK	GENMASK(15, 0)
+#define MAJOR_VERSION(x)	(u16)(FIELD_GET(MAJOR_VERSION_MASK, (x)))
+#define MINOR_VERSION(x)	(u16)(FIELD_GET(MINOR_VERSION_MASK, (x)))
+#define PACK_VERSION_INFO(major, minor)			\
+	(FIELD_PREP(MAJOR_VERSION_MASK, (major)) |	\
+	 FIELD_PREP(MINOR_VERSION_MASK, (minor)))
+#define FFA_VERSION_1_0		PACK_VERSION_INFO(1, 0)
+#define FFA_MIN_VERSION		FFA_VERSION_1_0
+
+#define SENDER_ID_MASK		GENMASK(31, 16)
+#define RECEIVER_ID_MASK	GENMASK(15, 0)
+#define SENDER_ID(x)		(u16)(FIELD_GET(SENDER_ID_MASK, (x)))
+#define RECEIVER_ID(x)		(u16)(FIELD_GET(RECEIVER_ID_MASK, (x)))
+#define PACK_TARGET_INFO(s, r)		\
+	(FIELD_PREP(SENDER_ID_MASK, (s)) | FIELD_PREP(RECEIVER_ID_MASK, (r)))
+
+/**
+ * FF-A specification mentions explicitly about '4K pages'. This should
+ * not be confused with the kernel PAGE_SIZE, which is the translation
+ * granule kernel is configured and may be one among 4K, 16K and 64K.
+ */
+#define FFA_PAGE_SIZE		SZ_4K
+/*
+ * Keeping RX TX buffer size as 4K for now
+ * 64K may be preferred to keep it min a page in 64K PAGE_SIZE config
+ */
+#define RXTX_BUFFER_SIZE	SZ_4K
+
+static ffa_fn *invoke_ffa_fn;
+
+static const int ffa_linux_errmap[] = {
+	/* better than switch case as long as return value is continuous */
+	0,		/* FFA_RET_SUCCESS */
+	-EOPNOTSUPP,	/* FFA_RET_NOT_SUPPORTED */
+	-EINVAL,	/* FFA_RET_INVALID_PARAMETERS */
+	-ENOMEM,	/* FFA_RET_NO_MEMORY */
+	-EBUSY,		/* FFA_RET_BUSY */
+	-EINTR,		/* FFA_RET_INTERRUPTED */
+	-EACCES,	/* FFA_RET_DENIED */
+	-EAGAIN,	/* FFA_RET_RETRY */
+	-ECANCELED,	/* FFA_RET_ABORTED */
+};
+
+static inline int ffa_to_linux_errno(int errno)
+{
+	if (errno < FFA_RET_SUCCESS && errno >= -ARRAY_SIZE(ffa_linux_errmap))
+		return ffa_linux_errmap[-errno];
+	return -EINVAL;
+}
+
+struct ffa_drv_info {
+	u32 version;
+	u16 vm_id;
+	struct mutex rx_lock; /* lock to protect Rx buffer */
+	struct mutex tx_lock; /* lock to protect Tx buffer */
+	void *rx_buffer;
+	void *tx_buffer;
+};
+
+static struct ffa_drv_info *drv_info;
+
+static int ffa_version_check(u32 *version)
+{
+	ffa_res_t ver;
+
+	ver = invoke_ffa_fn(FFA_VERSION, FFA_DRIVER_VERSION, 0, 0, 0, 0, 0, 0);
+
+	if (ver.a0 == FFA_RET_NOT_SUPPORTED) {
+		pr_info("FFA_VERSION returned not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	if (ver.a0 < FFA_MIN_VERSION || ver.a0 > FFA_DRIVER_VERSION) {
+		pr_err("Incompatible version %d.%d found\n",
+		       MAJOR_VERSION(ver.a0), MINOR_VERSION(ver.a0));
+		return -EINVAL;
+	}
+
+	*version = ver.a0;
+	pr_info("Version %d.%d found\n", MAJOR_VERSION(ver.a0),
+		MINOR_VERSION(ver.a0));
+	return 0;
+}
+
+static int ffa_rxtx_map(phys_addr_t tx_buf, phys_addr_t rx_buf, u32 pg_cnt)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_FN_NATIVE(RXTX_MAP), tx_buf, rx_buf, pg_cnt,
+			    0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	return 0;
+}
+
+static int ffa_rxtx_unmap(u16 vm_id)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_RXTX_UNMAP, PACK_TARGET_INFO(vm_id, 0),
+			    0, 0, 0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	return 0;
+}
+
+#define VM_ID_MASK	GENMASK(15, 0)
+static int ffa_id_get(u16 *vm_id)
+{
+	ffa_res_t id;
+
+	id = invoke_ffa_fn(FFA_ID_GET, 0, 0, 0, 0, 0, 0, 0);
+
+	if (id.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)id.a2);
+
+	*vm_id = FIELD_GET(VM_ID_MASK, (id.a2));
+
+	return 0;
+}
+
+static int __init ffa_init(void)
+{
+	int ret;
+
+	ret = arm_ffa_bus_init();
+	if (ret)
+		return ret;
+
+	ret = ffa_transport_init(&invoke_ffa_fn);
+	if (ret)
+		return ret;
+
+	drv_info = kzalloc(sizeof(*drv_info), GFP_KERNEL);
+	if (!drv_info)
+		return -ENOMEM;
+
+	ret = ffa_version_check(&drv_info->version);
+	if (ret)
+		goto free_drv_info;
+
+	if (ffa_id_get(&drv_info->vm_id)) {
+		pr_err("failed to obtain VM id for self\n");
+		ret = -ENODEV;
+		goto free_drv_info;
+	}
+
+	drv_info->rx_buffer = alloc_pages_exact(RXTX_BUFFER_SIZE, GFP_KERNEL);
+	if (!drv_info->rx_buffer) {
+		ret = -ENOMEM;
+		goto free_pages;
+	}
+
+	drv_info->tx_buffer = alloc_pages_exact(RXTX_BUFFER_SIZE, GFP_KERNEL);
+	if (!drv_info->tx_buffer) {
+		ret = -ENOMEM;
+		goto free_pages;
+	}
+
+	ret = ffa_rxtx_map(virt_to_phys(drv_info->tx_buffer),
+			   virt_to_phys(drv_info->rx_buffer),
+			   RXTX_BUFFER_SIZE / FFA_PAGE_SIZE);
+	if (ret) {
+		pr_err("failed to register FFA RxTx buffers\n");
+		goto free_pages;
+	}
+
+	mutex_init(&drv_info->rx_lock);
+	mutex_init(&drv_info->tx_lock);
+
+	return 0;
+free_pages:
+	if (drv_info->tx_buffer)
+		free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE);
+	free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE);
+free_drv_info:
+	kfree(drv_info);
+	return ret;
+}
+module_init(ffa_init);
+
+static void __exit ffa_exit(void)
+{
+	ffa_rxtx_unmap(drv_info->vm_id);
+	free_pages_exact(drv_info->tx_buffer, RXTX_BUFFER_SIZE);
+	free_pages_exact(drv_info->rx_buffer, RXTX_BUFFER_SIZE);
+	kfree(drv_info);
+	arm_ffa_bus_exit();
+}
+module_exit(ffa_exit);
+
+MODULE_ALIAS("arm-ffa");
+MODULE_AUTHOR("Sudeep Holla <sudeep.holla@arm.com>");
+MODULE_DESCRIPTION("Arm FF-A interface driver");
+MODULE_LICENSE("GPL v2");
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 5/7] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

There are requests to keep the transport separate in order to allow
other possible transports like virtio. So let us keep the SMCCC transport
specific routines abstracted.

It is kept simple for now. Once we add another transport, we can develop
better abstraction.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/Kconfig  |  5 +++
 drivers/firmware/arm_ffa/Makefile |  3 +-
 drivers/firmware/arm_ffa/common.h |  4 +++
 drivers/firmware/arm_ffa/smccc.c  | 54 +++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 drivers/firmware/arm_ffa/smccc.c

diff --git a/drivers/firmware/arm_ffa/Kconfig b/drivers/firmware/arm_ffa/Kconfig
index 261a3660650a..5e3ae5cf82e8 100644
--- a/drivers/firmware/arm_ffa/Kconfig
+++ b/drivers/firmware/arm_ffa/Kconfig
@@ -14,3 +14,8 @@ config ARM_FFA_TRANSPORT
 
 	  This driver provides interface for all the client drivers making
 	  use of the features offered by ARM FF-A.
+
+config ARM_FFA_SMCCC
+	bool
+	default ARM_FFA_TRANSPORT
+	depends on ARM64 && HAVE_ARM_SMCCC_DISCOVERY
diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
index 82d0d35c5324..9d9f37523200 100644
--- a/drivers/firmware/arm_ffa/Makefile
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 ffa-bus-y = bus.o
 ffa-driver-y = driver.o
-ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y)
+ffa-transport-$(CONFIG_ARM_FFA_SMCCC) += smccc.o
+ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y) $(ffa-transport-y)
 obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
index fc6948efe9f3..d019348bf67d 100644
--- a/drivers/firmware/arm_ffa/common.h
+++ b/drivers/firmware/arm_ffa/common.h
@@ -18,9 +18,13 @@ typedef ffa_res_t
 int __init arm_ffa_bus_init(void);
 void __exit arm_ffa_bus_exit(void);
 
+#ifdef CONFIG_ARM_FFA_SMCCC
+int __init ffa_transport_init(ffa_fn **invoke_ffa_fn);
+#else
 static inline int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
 {
 	return -EOPNOTSUPP;
 }
+#endif
 
 #endif /* _FFA_COMMON_H */
diff --git a/drivers/firmware/arm_ffa/smccc.c b/drivers/firmware/arm_ffa/smccc.c
new file mode 100644
index 000000000000..b93d281d2399
--- /dev/null
+++ b/drivers/firmware/arm_ffa/smccc.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#include <linux/printk.h>
+
+#include "common.h"
+
+static struct arm_smccc_v1_2_res
+__arm_ffa_fn_smc(unsigned long function_id, unsigned long arg0,
+		 unsigned long arg1, unsigned long arg2, unsigned long arg3,
+		 unsigned long arg4, unsigned long arg5, unsigned long arg6)
+{
+	struct arm_smccc_v1_2_res res;
+
+	arm_smccc_v1_2_smc(function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+			   arg6, &res);
+
+	return res;
+}
+
+static struct arm_smccc_v1_2_res
+__arm_ffa_fn_hvc(unsigned long function_id, unsigned long arg0,
+		 unsigned long arg1, unsigned long arg2, unsigned long arg3,
+		 unsigned long arg4, unsigned long arg5, unsigned long arg6)
+{
+	struct arm_smccc_v1_2_res res;
+
+	arm_smccc_v1_2_hvc(function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+			   arg6, &res);
+	return res;
+}
+
+int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
+{
+	enum arm_smccc_conduit conduit;
+
+	if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2)
+		return -EOPNOTSUPP;
+
+	conduit = arm_smccc_1_1_get_conduit();
+	if (conduit == SMCCC_CONDUIT_NONE) {
+		pr_err("%s: invalid SMCCC conduit\n", __func__);
+		return -EOPNOTSUPP;
+	}
+
+	if (conduit == SMCCC_CONDUIT_SMC)
+		*invoke_ffa_fn = __arm_ffa_fn_smc;
+	else
+		*invoke_ffa_fn = __arm_ffa_fn_hvc;
+
+	return 0;
+}
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 5/7] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

There are requests to keep the transport separate in order to allow
other possible transports like virtio. So let us keep the SMCCC transport
specific routines abstracted.

It is kept simple for now. Once we add another transport, we can develop
better abstraction.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/Kconfig  |  5 +++
 drivers/firmware/arm_ffa/Makefile |  3 +-
 drivers/firmware/arm_ffa/common.h |  4 +++
 drivers/firmware/arm_ffa/smccc.c  | 54 +++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+), 1 deletion(-)
 create mode 100644 drivers/firmware/arm_ffa/smccc.c

diff --git a/drivers/firmware/arm_ffa/Kconfig b/drivers/firmware/arm_ffa/Kconfig
index 261a3660650a..5e3ae5cf82e8 100644
--- a/drivers/firmware/arm_ffa/Kconfig
+++ b/drivers/firmware/arm_ffa/Kconfig
@@ -14,3 +14,8 @@ config ARM_FFA_TRANSPORT
 
 	  This driver provides interface for all the client drivers making
 	  use of the features offered by ARM FF-A.
+
+config ARM_FFA_SMCCC
+	bool
+	default ARM_FFA_TRANSPORT
+	depends on ARM64 && HAVE_ARM_SMCCC_DISCOVERY
diff --git a/drivers/firmware/arm_ffa/Makefile b/drivers/firmware/arm_ffa/Makefile
index 82d0d35c5324..9d9f37523200 100644
--- a/drivers/firmware/arm_ffa/Makefile
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -1,5 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 ffa-bus-y = bus.o
 ffa-driver-y = driver.o
-ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y)
+ffa-transport-$(CONFIG_ARM_FFA_SMCCC) += smccc.o
+ffa-module-objs := $(ffa-bus-y) $(ffa-driver-y) $(ffa-transport-y)
 obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-module.o
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
index fc6948efe9f3..d019348bf67d 100644
--- a/drivers/firmware/arm_ffa/common.h
+++ b/drivers/firmware/arm_ffa/common.h
@@ -18,9 +18,13 @@ typedef ffa_res_t
 int __init arm_ffa_bus_init(void);
 void __exit arm_ffa_bus_exit(void);
 
+#ifdef CONFIG_ARM_FFA_SMCCC
+int __init ffa_transport_init(ffa_fn **invoke_ffa_fn);
+#else
 static inline int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
 {
 	return -EOPNOTSUPP;
 }
+#endif
 
 #endif /* _FFA_COMMON_H */
diff --git a/drivers/firmware/arm_ffa/smccc.c b/drivers/firmware/arm_ffa/smccc.c
new file mode 100644
index 000000000000..b93d281d2399
--- /dev/null
+++ b/drivers/firmware/arm_ffa/smccc.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 ARM Ltd.
+ */
+
+#include <linux/printk.h>
+
+#include "common.h"
+
+static struct arm_smccc_v1_2_res
+__arm_ffa_fn_smc(unsigned long function_id, unsigned long arg0,
+		 unsigned long arg1, unsigned long arg2, unsigned long arg3,
+		 unsigned long arg4, unsigned long arg5, unsigned long arg6)
+{
+	struct arm_smccc_v1_2_res res;
+
+	arm_smccc_v1_2_smc(function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+			   arg6, &res);
+
+	return res;
+}
+
+static struct arm_smccc_v1_2_res
+__arm_ffa_fn_hvc(unsigned long function_id, unsigned long arg0,
+		 unsigned long arg1, unsigned long arg2, unsigned long arg3,
+		 unsigned long arg4, unsigned long arg5, unsigned long arg6)
+{
+	struct arm_smccc_v1_2_res res;
+
+	arm_smccc_v1_2_hvc(function_id, arg0, arg1, arg2, arg3, arg4, arg5,
+			   arg6, &res);
+	return res;
+}
+
+int __init ffa_transport_init(ffa_fn **invoke_ffa_fn)
+{
+	enum arm_smccc_conduit conduit;
+
+	if (arm_smccc_get_version() < ARM_SMCCC_VERSION_1_2)
+		return -EOPNOTSUPP;
+
+	conduit = arm_smccc_1_1_get_conduit();
+	if (conduit == SMCCC_CONDUIT_NONE) {
+		pr_err("%s: invalid SMCCC conduit\n", __func__);
+		return -EOPNOTSUPP;
+	}
+
+	if (conduit == SMCCC_CONDUIT_SMC)
+		*invoke_ffa_fn = __arm_ffa_fn_smc;
+	else
+		*invoke_ffa_fn = __arm_ffa_fn_hvc;
+
+	return 0;
+}
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Parse the FFA nodes from the device-tree and register all the partitions
whose services will be used in the kernel.

In order to also enable in-kernel users of FFA interface, let us add
simple set of operations for such devices.

The in-kernel users are registered without the character device interface.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/common.h |   2 +
 drivers/firmware/arm_ffa/driver.c | 188 ++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h           |  38 +++++-
 3 files changed, 227 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
index d019348bf67d..eb1371c2b2b8 100644
--- a/drivers/firmware/arm_ffa/common.h
+++ b/drivers/firmware/arm_ffa/common.h
@@ -6,6 +6,7 @@
 #ifndef _FFA_COMMON_H
 #define _FFA_COMMON_H
 
+#include <linux/arm_ffa.h>
 #include <linux/arm-smccc.h>
 #include <linux/err.h>
 
@@ -17,6 +18,7 @@ typedef ffa_res_t
 
 int __init arm_ffa_bus_init(void);
 void __exit arm_ffa_bus_exit(void);
+bool ffa_device_is_valid(struct ffa_device *ffa_dev);
 
 #ifdef CONFIG_ARM_FFA_SMCCC
 int __init ffa_transport_init(ffa_fn **invoke_ffa_fn);
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 6d5094ecc0f6..fe81f6b71b6c 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -24,9 +24,13 @@
 
 #include <linux/arm_ffa.h>
 #include <linux/bitfield.h>
+#include <linux/device.h>
 #include <linux/io.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/slab.h>
+#include <linux/uuid.h>
 
 #include "common.h"
 
@@ -183,6 +187,20 @@ static int ffa_version_check(u32 *version)
 	return 0;
 }
 
+static int ffa_rx_release(void)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_RX_RELEASE, 0, 0, 0, 0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	/* check for ret.a0 == FFA_RX_RELEASE ? */
+
+	return 0;
+}
+
 static int ffa_rxtx_map(phys_addr_t tx_buf, phys_addr_t rx_buf, u32 pg_cnt)
 {
 	ffa_res_t ret;
@@ -209,6 +227,52 @@ static int ffa_rxtx_unmap(u16 vm_id)
 	return 0;
 }
 
+/* buffer must be sizeof(struct ffa_partition_info) * num_partitions */
+static int
+__ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
+			 struct ffa_partition_info **buffer, int num_partitions)
+{
+	int count;
+	ffa_res_t partition_info;
+
+	mutex_lock(&drv_info->rx_lock);
+	partition_info = invoke_ffa_fn(FFA_PARTITION_INFO_GET, uuid0, uuid1,
+				       uuid2, uuid3, 0, 0, 0);
+
+	if (partition_info.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)partition_info.a2);
+
+	count = partition_info.a2;
+
+	if (buffer && count <= num_partitions)
+		memcpy(*buffer, drv_info->rx_buffer, sizeof(*buffer) * count);
+
+	ffa_rx_release();
+
+	mutex_unlock(&drv_info->rx_lock);
+
+	return count;
+}
+
+static int ffa_partition_probe(const char *uuid_str,
+			       struct ffa_partition_info *buffer)
+{
+	int count;
+	uuid_t uuid;
+	u32 uuid0_4[4] = { 0 };
+
+	if (uuid_parse(uuid_str, &uuid)) {
+		pr_err("invalid uuid (%s)\n", uuid_str);
+		return -ENODEV;
+	}
+
+	export_uuid((u8 *)uuid0_4, &uuid);
+	count = __ffa_partition_info_get(uuid0_4[0], uuid0_4[1], uuid0_4[2],
+					 uuid0_4[3], &buffer, 1);
+
+	return count != 1;
+}
+
 #define VM_ID_MASK	GENMASK(15, 0)
 static int ffa_id_get(u16 *vm_id)
 {
@@ -224,9 +288,125 @@ static int ffa_id_get(u16 *vm_id)
 	return 0;
 }
 
+static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool mode_32bit,
+				   struct ffa_send_direct_data *data)
+{
+	u32 req_id, resp_id, src_dst_ids = PACK_TARGET_INFO(src_id, dst_id);
+	ffa_res_t ret;
+
+	if (mode_32bit) {
+		req_id = FFA_MSG_SEND_DIRECT_REQ;
+		resp_id = FFA_MSG_SEND_DIRECT_RESP;
+	} else {
+		req_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_REQ);
+		resp_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_RESP);
+	}
+
+	ret = invoke_ffa_fn(req_id, src_dst_ids, 0, data->data0, data->data1,
+			    data->data2, data->data3, data->data4);
+
+	while (ret.a0 == FFA_INTERRUPT)
+		ret = invoke_ffa_fn(FFA_RUN, ret.a1, 0, 0, 0, 0, 0, 0);
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	if (ret.a0 == resp_id) {
+		data->data0 = ret.a3;
+		data->data1 = ret.a4;
+		data->data2 = ret.a5;
+		data->data3 = ret.a6;
+		data->data4 = ret.a7;
+	}
+
+	return 0;
+}
+
+static u32 ffa_api_version_get(void)
+{
+	return drv_info->version;
+}
+
+static int ffa_partition_info_get(const char *uuid_str,
+				  struct ffa_partition_info *buffer)
+{
+	if (ffa_partition_probe(uuid_str, buffer))
+		return -ENOENT;
+	return 0;
+}
+
+static void ffa_mode_32bit_set(struct ffa_device *dev)
+{
+	dev->mode_32bit = true;
+}
+
+static int ffa_sync_send_receive(struct ffa_device *dev,
+				 struct ffa_send_direct_data *data)
+{
+	return ffa_msg_send_direct_req(drv_info->vm_id, dev->vm_id,
+				       dev->mode_32bit, data);
+}
+
+static const struct ffa_dev_ops ffa_ops = {
+	.api_version_get = ffa_api_version_get,
+	.partition_info_get = ffa_partition_info_get,
+	.mode_32bit_set = ffa_mode_32bit_set,
+	.sync_send_receive = ffa_sync_send_receive,
+};
+
+const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
+{
+	if (ffa_device_is_valid(dev))
+		return &ffa_ops;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(ffa_dev_ops_get);
+
+static void ffa_setup_partitions(struct device_node *np)
+{
+	int ret;
+	struct device_node *child;
+	struct ffa_device *ffa_dev;
+	struct ffa_partition_info pbuf;
+	const char *p_uuid, *pfx = "Ignoring FFA partition";
+	uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+
+	for_each_child_of_node(np, child) {
+		if (!of_device_is_compatible(child, "arm,ffa-1.0-partition"))
+			continue;
+
+		if (of_property_read_string(child, "uuid", &p_uuid)) {
+			pr_err("%s: failed to parse \"uuid\" property\n", pfx);
+			continue;
+		}
+
+		if (uuid_parse(p_uuid, &uuid)) {
+			pr_err("%s: invalid \"uuid\" property (%s)\n",
+			       pfx, p_uuid);
+			continue;
+		}
+
+		ret = ffa_partition_probe(p_uuid, &pbuf);
+		if (ret) {
+			pr_err("%s: %s partition info probe failed\n",
+			       pfx, p_uuid);
+			continue;
+		}
+
+		ffa_dev = ffa_device_register(p_uuid, pbuf.id);
+		if (!ffa_dev) {
+			pr_err("%s: failed to register %s\n", pfx, p_uuid);
+			continue;
+		}
+
+		ffa_dev_set_drvdata(ffa_dev, drv_info);
+	}
+}
+
 static int __init ffa_init(void)
 {
 	int ret;
+	struct device_node *np;
 
 	ret = arm_ffa_bus_init();
 	if (ret)
@@ -273,6 +453,14 @@ static int __init ffa_init(void)
 	mutex_init(&drv_info->rx_lock);
 	mutex_init(&drv_info->tx_lock);
 
+	/* Set up all the partitions */
+	np = of_find_compatible_node(NULL, NULL, "arm,ffa-1.0");
+	if (!np)
+		return 0;
+
+	ffa_setup_partitions(np);
+	of_node_put(np);
+
 	return 0;
 free_pages:
 	if (drv_info->tx_buffer)
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 43f9c0df4114..f0c68f3f1c6a 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -6,7 +6,6 @@
 #ifndef _LINUX_ARM_FFA_H
 #define _LINUX_ARM_FFA_H
 
-#include <linux/cdev.h>
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -15,6 +14,7 @@
 /* FFA Bus/Device/Driver related */
 struct ffa_device {
 	int vm_id;
+	bool mode_32bit;
 	uuid_t uuid;
 	struct device dev;
 };
@@ -48,6 +48,7 @@ int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
 			const char *mod_name);
 void ffa_driver_unregister(struct ffa_driver *driver);
 bool ffa_device_is_valid(struct ffa_device *ffa_dev);
+const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);
 
 #else
 static inline
@@ -70,6 +71,10 @@ static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
 static inline
 bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
 
+const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
+{
+	return NULL;
+}
 #endif /* CONFIG_ARM_FFA_TRANSPORT */
 
 #define ffa_register(driver) \
@@ -88,4 +93,35 @@ bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
 #define module_ffa_driver(__ffa_driver)	\
 	module_driver(__ffa_driver, ffa_register, ffa_unregister)
 
+/* FFA transport related */
+struct ffa_partition_info {
+	u16 id;
+	u16 exec_ctxt;
+/* partition supports receipt of direct requests */
+#define FFA_PARTITION_DIRECT_RECV	BIT(0)
+/* partition can send direct requests. */
+#define FFA_PARTITION_DIRECT_SEND	BIT(1)
+/* partition can send and receive indirect messages. */
+#define FFA_PARTITION_INDIRECT_MSG	BIT(2)
+	u32 properties;
+};
+
+/* For use with FFA_MSG_SEND_DIRECT_{REQ,RESP} which pass data via registers */
+struct ffa_send_direct_data {
+	unsigned long data0; /* w3/x3 */
+	unsigned long data1; /* w4/x4 */
+	unsigned long data2; /* w5/x5 */
+	unsigned long data3; /* w6/x6 */
+	unsigned long data4; /* w7/x7 */
+};
+
+struct ffa_dev_ops {
+	u32 (*api_version_get)(void);
+	int (*partition_info_get)(const char *uuid_str,
+				  struct ffa_partition_info *buffer);
+	void (*mode_32bit_set)(struct ffa_device *dev);
+	int (*sync_send_receive)(struct ffa_device *dev,
+				 struct ffa_send_direct_data *data);
+};
+
 #endif /* _LINUX_ARM_FFA_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Parse the FFA nodes from the device-tree and register all the partitions
whose services will be used in the kernel.

In order to also enable in-kernel users of FFA interface, let us add
simple set of operations for such devices.

The in-kernel users are registered without the character device interface.

Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/common.h |   2 +
 drivers/firmware/arm_ffa/driver.c | 188 ++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h           |  38 +++++-
 3 files changed, 227 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
index d019348bf67d..eb1371c2b2b8 100644
--- a/drivers/firmware/arm_ffa/common.h
+++ b/drivers/firmware/arm_ffa/common.h
@@ -6,6 +6,7 @@
 #ifndef _FFA_COMMON_H
 #define _FFA_COMMON_H
 
+#include <linux/arm_ffa.h>
 #include <linux/arm-smccc.h>
 #include <linux/err.h>
 
@@ -17,6 +18,7 @@ typedef ffa_res_t
 
 int __init arm_ffa_bus_init(void);
 void __exit arm_ffa_bus_exit(void);
+bool ffa_device_is_valid(struct ffa_device *ffa_dev);
 
 #ifdef CONFIG_ARM_FFA_SMCCC
 int __init ffa_transport_init(ffa_fn **invoke_ffa_fn);
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 6d5094ecc0f6..fe81f6b71b6c 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -24,9 +24,13 @@
 
 #include <linux/arm_ffa.h>
 #include <linux/bitfield.h>
+#include <linux/device.h>
 #include <linux/io.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/slab.h>
+#include <linux/uuid.h>
 
 #include "common.h"
 
@@ -183,6 +187,20 @@ static int ffa_version_check(u32 *version)
 	return 0;
 }
 
+static int ffa_rx_release(void)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_RX_RELEASE, 0, 0, 0, 0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	/* check for ret.a0 == FFA_RX_RELEASE ? */
+
+	return 0;
+}
+
 static int ffa_rxtx_map(phys_addr_t tx_buf, phys_addr_t rx_buf, u32 pg_cnt)
 {
 	ffa_res_t ret;
@@ -209,6 +227,52 @@ static int ffa_rxtx_unmap(u16 vm_id)
 	return 0;
 }
 
+/* buffer must be sizeof(struct ffa_partition_info) * num_partitions */
+static int
+__ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
+			 struct ffa_partition_info **buffer, int num_partitions)
+{
+	int count;
+	ffa_res_t partition_info;
+
+	mutex_lock(&drv_info->rx_lock);
+	partition_info = invoke_ffa_fn(FFA_PARTITION_INFO_GET, uuid0, uuid1,
+				       uuid2, uuid3, 0, 0, 0);
+
+	if (partition_info.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)partition_info.a2);
+
+	count = partition_info.a2;
+
+	if (buffer && count <= num_partitions)
+		memcpy(*buffer, drv_info->rx_buffer, sizeof(*buffer) * count);
+
+	ffa_rx_release();
+
+	mutex_unlock(&drv_info->rx_lock);
+
+	return count;
+}
+
+static int ffa_partition_probe(const char *uuid_str,
+			       struct ffa_partition_info *buffer)
+{
+	int count;
+	uuid_t uuid;
+	u32 uuid0_4[4] = { 0 };
+
+	if (uuid_parse(uuid_str, &uuid)) {
+		pr_err("invalid uuid (%s)\n", uuid_str);
+		return -ENODEV;
+	}
+
+	export_uuid((u8 *)uuid0_4, &uuid);
+	count = __ffa_partition_info_get(uuid0_4[0], uuid0_4[1], uuid0_4[2],
+					 uuid0_4[3], &buffer, 1);
+
+	return count != 1;
+}
+
 #define VM_ID_MASK	GENMASK(15, 0)
 static int ffa_id_get(u16 *vm_id)
 {
@@ -224,9 +288,125 @@ static int ffa_id_get(u16 *vm_id)
 	return 0;
 }
 
+static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool mode_32bit,
+				   struct ffa_send_direct_data *data)
+{
+	u32 req_id, resp_id, src_dst_ids = PACK_TARGET_INFO(src_id, dst_id);
+	ffa_res_t ret;
+
+	if (mode_32bit) {
+		req_id = FFA_MSG_SEND_DIRECT_REQ;
+		resp_id = FFA_MSG_SEND_DIRECT_RESP;
+	} else {
+		req_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_REQ);
+		resp_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_RESP);
+	}
+
+	ret = invoke_ffa_fn(req_id, src_dst_ids, 0, data->data0, data->data1,
+			    data->data2, data->data3, data->data4);
+
+	while (ret.a0 == FFA_INTERRUPT)
+		ret = invoke_ffa_fn(FFA_RUN, ret.a1, 0, 0, 0, 0, 0, 0);
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	if (ret.a0 == resp_id) {
+		data->data0 = ret.a3;
+		data->data1 = ret.a4;
+		data->data2 = ret.a5;
+		data->data3 = ret.a6;
+		data->data4 = ret.a7;
+	}
+
+	return 0;
+}
+
+static u32 ffa_api_version_get(void)
+{
+	return drv_info->version;
+}
+
+static int ffa_partition_info_get(const char *uuid_str,
+				  struct ffa_partition_info *buffer)
+{
+	if (ffa_partition_probe(uuid_str, buffer))
+		return -ENOENT;
+	return 0;
+}
+
+static void ffa_mode_32bit_set(struct ffa_device *dev)
+{
+	dev->mode_32bit = true;
+}
+
+static int ffa_sync_send_receive(struct ffa_device *dev,
+				 struct ffa_send_direct_data *data)
+{
+	return ffa_msg_send_direct_req(drv_info->vm_id, dev->vm_id,
+				       dev->mode_32bit, data);
+}
+
+static const struct ffa_dev_ops ffa_ops = {
+	.api_version_get = ffa_api_version_get,
+	.partition_info_get = ffa_partition_info_get,
+	.mode_32bit_set = ffa_mode_32bit_set,
+	.sync_send_receive = ffa_sync_send_receive,
+};
+
+const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
+{
+	if (ffa_device_is_valid(dev))
+		return &ffa_ops;
+
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(ffa_dev_ops_get);
+
+static void ffa_setup_partitions(struct device_node *np)
+{
+	int ret;
+	struct device_node *child;
+	struct ffa_device *ffa_dev;
+	struct ffa_partition_info pbuf;
+	const char *p_uuid, *pfx = "Ignoring FFA partition";
+	uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+
+	for_each_child_of_node(np, child) {
+		if (!of_device_is_compatible(child, "arm,ffa-1.0-partition"))
+			continue;
+
+		if (of_property_read_string(child, "uuid", &p_uuid)) {
+			pr_err("%s: failed to parse \"uuid\" property\n", pfx);
+			continue;
+		}
+
+		if (uuid_parse(p_uuid, &uuid)) {
+			pr_err("%s: invalid \"uuid\" property (%s)\n",
+			       pfx, p_uuid);
+			continue;
+		}
+
+		ret = ffa_partition_probe(p_uuid, &pbuf);
+		if (ret) {
+			pr_err("%s: %s partition info probe failed\n",
+			       pfx, p_uuid);
+			continue;
+		}
+
+		ffa_dev = ffa_device_register(p_uuid, pbuf.id);
+		if (!ffa_dev) {
+			pr_err("%s: failed to register %s\n", pfx, p_uuid);
+			continue;
+		}
+
+		ffa_dev_set_drvdata(ffa_dev, drv_info);
+	}
+}
+
 static int __init ffa_init(void)
 {
 	int ret;
+	struct device_node *np;
 
 	ret = arm_ffa_bus_init();
 	if (ret)
@@ -273,6 +453,14 @@ static int __init ffa_init(void)
 	mutex_init(&drv_info->rx_lock);
 	mutex_init(&drv_info->tx_lock);
 
+	/* Set up all the partitions */
+	np = of_find_compatible_node(NULL, NULL, "arm,ffa-1.0");
+	if (!np)
+		return 0;
+
+	ffa_setup_partitions(np);
+	of_node_put(np);
+
 	return 0;
 free_pages:
 	if (drv_info->tx_buffer)
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 43f9c0df4114..f0c68f3f1c6a 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -6,7 +6,6 @@
 #ifndef _LINUX_ARM_FFA_H
 #define _LINUX_ARM_FFA_H
 
-#include <linux/cdev.h>
 #include <linux/device.h>
 #include <linux/module.h>
 #include <linux/types.h>
@@ -15,6 +14,7 @@
 /* FFA Bus/Device/Driver related */
 struct ffa_device {
 	int vm_id;
+	bool mode_32bit;
 	uuid_t uuid;
 	struct device dev;
 };
@@ -48,6 +48,7 @@ int ffa_driver_register(struct ffa_driver *driver, struct module *owner,
 			const char *mod_name);
 void ffa_driver_unregister(struct ffa_driver *driver);
 bool ffa_device_is_valid(struct ffa_device *ffa_dev);
+const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);
 
 #else
 static inline
@@ -70,6 +71,10 @@ static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
 static inline
 bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
 
+const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
+{
+	return NULL;
+}
 #endif /* CONFIG_ARM_FFA_TRANSPORT */
 
 #define ffa_register(driver) \
@@ -88,4 +93,35 @@ bool ffa_device_is_valid(struct ffa_device *ffa_dev) { return false; }
 #define module_ffa_driver(__ffa_driver)	\
 	module_driver(__ffa_driver, ffa_register, ffa_unregister)
 
+/* FFA transport related */
+struct ffa_partition_info {
+	u16 id;
+	u16 exec_ctxt;
+/* partition supports receipt of direct requests */
+#define FFA_PARTITION_DIRECT_RECV	BIT(0)
+/* partition can send direct requests. */
+#define FFA_PARTITION_DIRECT_SEND	BIT(1)
+/* partition can send and receive indirect messages. */
+#define FFA_PARTITION_INDIRECT_MSG	BIT(2)
+	u32 properties;
+};
+
+/* For use with FFA_MSG_SEND_DIRECT_{REQ,RESP} which pass data via registers */
+struct ffa_send_direct_data {
+	unsigned long data0; /* w3/x3 */
+	unsigned long data1; /* w4/x4 */
+	unsigned long data2; /* w5/x5 */
+	unsigned long data3; /* w6/x6 */
+	unsigned long data4; /* w7/x7 */
+};
+
+struct ffa_dev_ops {
+	u32 (*api_version_get)(void);
+	int (*partition_info_get)(const char *uuid_str,
+				  struct ffa_partition_info *buffer);
+	void (*mode_32bit_set)(struct ffa_device *dev);
+	int (*sync_send_receive)(struct ffa_device *dev,
+				 struct ffa_send_direct_data *data);
+};
+
 #endif /* _LINUX_ARM_FFA_H */
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 7/7] firmware: arm_ffa: Add support for MEM_* interfaces
  2021-03-25 14:32 ` Sudeep Holla
@ 2021-03-25 14:32   ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Most of the MEM_* APIs share the same parameters, so they can be
generalised. Currently only MEM_SHARE is implemented and the user space
interface for that is not added yet.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 184 ++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h           | 139 ++++++++++++++++++++++
 2 files changed, 323 insertions(+)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index fe81f6b71b6c..c65be31a1603 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -28,7 +28,9 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mm.h>
 #include <linux/of.h>
+#include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/uuid.h>
 
@@ -321,6 +323,177 @@ static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool mode_32bit,
 	return 0;
 }
 
+static int ffa_mem_first_frag(u32 func_id, phys_addr_t buf, u32 buf_sz,
+			      u32 frag_len, u32 len, u64 *handle)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(func_id, len, frag_len, buf, buf_sz, 0, 0, 0);
+
+	while (ret.a0 == FFA_MEM_OP_PAUSE)
+		ret = invoke_ffa_fn(FFA_MEM_OP_RESUME, ret.a1, ret.a2,
+				    0, 0, 0, 0, 0);
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	if (ret.a0 != FFA_SUCCESS)
+		return -EOPNOTSUPP;
+
+	if (handle)
+		*handle = PACK_HANDLE(ret.a2, ret.a3);
+
+	return frag_len;
+}
+
+static int ffa_mem_next_frag(u64 handle, u32 frag_len)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_MEM_FRAG_TX, HANDLE_LOW(handle),
+			    HANDLE_HIGH(handle), frag_len, 0, 0, 0, 0);
+
+	while (ret.a0 == FFA_MEM_OP_PAUSE)
+		ret = invoke_ffa_fn(FFA_MEM_OP_RESUME, ret.a1, ret.a2,
+				    0, 0, 0, 0, 0);
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	if (ret.a0 != FFA_MEM_FRAG_RX)
+		return -EOPNOTSUPP;
+
+	return ret.a3;
+}
+
+static int
+ffa_transmit_fragment(u32 func_id, phys_addr_t buf, u32 buf_sz, u32 frag_len,
+		      u32 len, u64 *handle, bool first)
+{
+	if (!first)
+		return ffa_mem_next_frag(*handle, frag_len);
+
+	return ffa_mem_first_frag(func_id, buf, buf_sz, frag_len, len, handle);
+}
+
+static u32 ffa_get_num_pages_sg(struct scatterlist *sg)
+{
+	u32 num_pages = 0;
+
+	do {
+		num_pages += sg->length / FFA_PAGE_SIZE;
+	} while ((sg = sg_next(sg)));
+
+	return num_pages;
+}
+
+static int
+ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
+		       struct ffa_mem_ops_args *args)
+{
+	int rc = 0;
+	bool first = true;
+	phys_addr_t addr = 0;
+	struct ffa_composite_mem_region *composite;
+	struct ffa_mem_region_addr_range *constituents;
+	struct ffa_mem_region_attributes *ep_mem_access;
+	struct ffa_mem_region *mem_region = buffer;
+	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg);
+
+	mem_region->tag = args->tag;
+	mem_region->flags = args->flags;
+	mem_region->sender_id = drv_info->vm_id;
+	mem_region->attributes = FFA_MEM_NORMAL | FFA_MEM_WRITE_BACK |
+				 FFA_MEM_INNER_SHAREABLE;
+	ep_mem_access = &mem_region->ep_mem_access[0];
+
+	for (idx = 0; idx < args->nattrs; idx++, ep_mem_access++) {
+		ep_mem_access->receiver = args->attrs[idx].receiver;
+		ep_mem_access->attrs = args->attrs[idx].attrs;
+		ep_mem_access->composite_off = COMPOSITE_OFFSET(args->nattrs);
+	}
+	mem_region->ep_count = args->nattrs;
+
+	composite = buffer + COMPOSITE_OFFSET(args->nattrs);
+	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
+	composite->addr_range_cnt = num_entries;
+
+	length = COMPOSITE_CONSTITUENTS_OFFSET(args->nattrs, num_entries);
+	frag_len = COMPOSITE_CONSTITUENTS_OFFSET(args->nattrs, 0);
+	if (frag_len > max_fragsize)
+		return -ENXIO;
+
+	if (!args->use_txbuf) {
+		addr = virt_to_phys(buffer);
+		buf_sz = max_fragsize / FFA_PAGE_SIZE;
+	}
+
+	constituents = buffer + frag_len;
+	idx = 0;
+	do {
+		if (frag_len == max_fragsize) {
+			rc = ffa_transmit_fragment(func_id, addr, buf_sz,
+						   frag_len, length,
+						   &args->g_handle, first);
+			if (rc < 0)
+				return -ENXIO;
+
+			first = false;
+			idx = 0;
+			frag_len = 0;
+			constituents = buffer;
+		}
+
+		if ((void *)constituents - buffer > max_fragsize) {
+			pr_err("Memory Region Fragment > Tx Buffer size\n");
+			return -EFAULT;
+		}
+
+		constituents->address = sg_phys(args->sg);
+		constituents->pg_cnt = args->sg->length / FFA_PAGE_SIZE;
+		constituents++;
+		frag_len += sizeof(struct ffa_mem_region_addr_range);
+	} while ((args->sg = sg_next(args->sg)));
+
+	return ffa_transmit_fragment(func_id, addr, buf_sz, frag_len,
+				     length, &args->g_handle, first);
+}
+
+static int ffa_memory_ops(u32 func_id, struct ffa_mem_ops_args *args)
+{
+	int ret;
+	void *buffer;
+
+	if (!args->use_txbuf) {
+		buffer = alloc_pages_exact(RXTX_BUFFER_SIZE, GFP_KERNEL);
+		if (!buffer)
+			return -ENOMEM;
+	} else {
+		buffer = drv_info->tx_buffer;
+		mutex_lock(&drv_info->tx_lock);
+	}
+
+	ret = ffa_setup_and_transmit(func_id, buffer, RXTX_BUFFER_SIZE, args);
+
+	if (args->use_txbuf)
+		mutex_unlock(&drv_info->tx_lock);
+	else
+		free_pages_exact(buffer, RXTX_BUFFER_SIZE);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int ffa_memory_reclaim(u64 g_handle, u32 flags)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_MEM_RECLAIM, HANDLE_LOW(g_handle),
+			    HANDLE_HIGH(g_handle), flags, 0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	return 0;
+}
+
 static u32 ffa_api_version_get(void)
 {
 	return drv_info->version;
@@ -346,11 +519,22 @@ static int ffa_sync_send_receive(struct ffa_device *dev,
 				       dev->mode_32bit, data);
 }
 
+static int
+ffa_memory_share(struct ffa_device *dev, struct ffa_mem_ops_args *args)
+{
+	if (dev->mode_32bit)
+		return ffa_memory_ops(FFA_MEM_SHARE, args);
+
+	return ffa_memory_ops(FFA_FN_NATIVE(MEM_SHARE), args);
+}
+
 static const struct ffa_dev_ops ffa_ops = {
 	.api_version_get = ffa_api_version_get,
 	.partition_info_get = ffa_partition_info_get,
 	.mode_32bit_set = ffa_mode_32bit_set,
 	.sync_send_receive = ffa_sync_send_receive,
+	.memory_reclaim = ffa_memory_reclaim,
+	.memory_share = ffa_memory_share,
 };
 
 const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index f0c68f3f1c6a..200019c20e39 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -115,6 +115,142 @@ struct ffa_send_direct_data {
 	unsigned long data4; /* w7/x7 */
 };
 
+struct ffa_mem_region_addr_range {
+	/* The base IPA of the constituent memory region, aligned to 4 kiB */
+	u64 address;
+	/* The number of 4 kiB pages in the constituent memory region. */
+	u32 pg_cnt;
+	u32 reserved;
+};
+
+struct ffa_composite_mem_region {
+	/*
+	 * The total number of 4 kiB pages included in this memory region. This
+	 * must be equal to the sum of page counts specified in each
+	 * `struct ffa_mem_region_addr_range`.
+	 */
+	u32 total_pg_cnt;
+	/* The number of constituents included in this memory region range */
+	u32 addr_range_cnt;
+	u64 reserved;
+	/** An array of `addr_range_cnt` memory region constituents. */
+	struct ffa_mem_region_addr_range constituents[];
+};
+
+struct ffa_mem_region_attributes {
+	/* The ID of the VM to which the memory is being given or shared. */
+	u16 receiver;
+	/*
+	 * The permissions with which the memory region should be mapped in the
+	 * receiver's page table.
+	 */
+#define FFA_MEM_EXEC	BIT(3)
+#define FFA_MEM_NO_EXEC	BIT(2)
+#define FFA_MEM_RW		BIT(1)
+#define FFA_MEM_RO		BIT(0)
+	u8 attrs;
+	/*
+	 * Flags used during FFA_MEM_RETRIEVE_REQ and FFA_MEM_RETRIEVE_RESP
+	 * for memory regions with multiple borrowers.
+	 */
+#define FFA_MEM_RETRIEVE_SELF_BORROWER	BIT(0)
+	u8 flag;
+	u32 composite_off;
+	/*
+	 * Offset in bytes from the start of the outer `ffa_memory_region` to
+	 * an `struct ffa_mem_region_addr_range`.
+	 */
+	u64 reserved;
+};
+
+struct ffa_mem_region {
+	/* The ID of the VM/owner which originally sent the memory region */
+	u16 sender_id;
+#define FFA_MEM_NORMAL		BIT(5)
+#define FFA_MEM_DEVICE		BIT(4)
+
+#define FFA_MEM_WRITE_BACK	(3 << 2)
+#define FFA_MEM_NON_CACHEABLE	(1 << 2)
+
+#define FFA_DEV_nGnRnE		(0 << 2)
+#define FFA_DEV_nGnRE		(1 << 2)
+#define FFA_DEV_nGRE		(2 << 2)
+#define FFA_DEV_GRE		(3 << 2)
+
+#define FFA_MEM_NON_SHAREABLE	(0)
+#define FFA_MEM_OUTER_SHAREABLE	(2)
+#define FFA_MEM_INNER_SHAREABLE	(3)
+	u8 attributes;
+	u8 reserved_0;
+/*
+ * Clear memory region contents after unmapping it from the sender and
+ * before mapping it for any receiver.
+ */
+#define FFA_MEM_CLEAR			BIT(0)
+/*
+ * Whether the hypervisor may time slice the memory sharing or retrieval
+ * operation.
+ */
+#define FFA_TIME_SLICE_ENABLE		BIT(1)
+
+#define FFA_MEM_RETRIEVE_TYPE_IN_RESP	(0 << 3)
+#define FFA_MEM_RETRIEVE_TYPE_SHARE	(1 << 3)
+#define FFA_MEM_RETRIEVE_TYPE_LEND	(2 << 3)
+#define FFA_MEM_RETRIEVE_TYPE_DONATE	(3 << 3)
+
+#define FFA_MEM_RETRIEVE_ADDR_ALIGN_HINT	BIT(9)
+#define FFA_MEM_RETRIEVE_ADDR_ALIGN(x)		((x) << 5)
+	/* Flags to control behaviour of the transaction. */
+	u32 flags;
+#define HANDLE_LOW_MASK		GENMASK_ULL(31, 0)
+#define HANDLE_HIGH_MASK	GENMASK_ULL(63, 32)
+#define HANDLE_LOW(x)		(u32)(FIELD_GET(HANDLE_LOW_MASK, (x)))
+#define	HANDLE_HIGH(x)		(u32)(FIELD_GET(HANDLE_HIGH_MASK, (x)))
+
+#define PACK_HANDLE(l, h)		\
+	(FIELD_PREP(HANDLE_LOW_MASK, (l)) | FIELD_PREP(HANDLE_HIGH_MASK, (h)))
+	/*
+	 * A globally-unique ID assigned by the hypervisor for a region
+	 * of memory being sent between VMs.
+	 */
+	u64 handle;
+	/*
+	 * An implementation defined value associated with the receiver and the
+	 * memory region.
+	 */
+	u64 tag;
+	u32 reserved_1;
+	/*
+	 * The number of `ffa_mem_region_attributes` entries included in this
+	 * transaction.
+	 */
+	u32 ep_count;
+	/*
+	 * An array of endpoint memory access descriptors.
+	 * Each one specifies a memory region offset, an endpoint and the
+	 * attributes with which this memory region should be mapped in that
+	 * endpoint's page table.
+	 */
+	struct ffa_mem_region_attributes ep_mem_access[];
+};
+
+#define	COMPOSITE_OFFSET(x)	\
+	(offsetof(struct ffa_mem_region, ep_mem_access[x]))
+#define CONSTITUENTS_OFFSET(x)	\
+	(offsetof(struct ffa_composite_mem_region, constituents[x]))
+#define COMPOSITE_CONSTITUENTS_OFFSET(x, y)	\
+	(COMPOSITE_OFFSET(x) + CONSTITUENTS_OFFSET(y))
+
+struct ffa_mem_ops_args {
+	bool use_txbuf;
+	u32 nattrs;
+	u32 flags;
+	u64 tag;
+	u64 g_handle;
+	struct scatterlist *sg;
+	struct ffa_mem_region_attributes *attrs;
+};
+
 struct ffa_dev_ops {
 	u32 (*api_version_get)(void);
 	int (*partition_info_get)(const char *uuid_str,
@@ -122,6 +258,9 @@ struct ffa_dev_ops {
 	void (*mode_32bit_set)(struct ffa_device *dev);
 	int (*sync_send_receive)(struct ffa_device *dev,
 				 struct ffa_send_direct_data *data);
+	int (*memory_reclaim)(u64 g_handle, u32 flags);
+	int (*memory_share)(struct ffa_device *dev,
+			    struct ffa_mem_ops_args *args);
 };
 
 #endif /* _LINUX_ARM_FFA_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 36+ messages in thread

* [PATCH v5 7/7] firmware: arm_ffa: Add support for MEM_* interfaces
@ 2021-03-25 14:32   ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-25 14:32 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy, Marc Bonnici

Most of the MEM_* APIs share the same parameters, so they can be
generalised. Currently only MEM_SHARE is implemented and the user space
interface for that is not added yet.

Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/firmware/arm_ffa/driver.c | 184 ++++++++++++++++++++++++++++++
 include/linux/arm_ffa.h           | 139 ++++++++++++++++++++++
 2 files changed, 323 insertions(+)

diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index fe81f6b71b6c..c65be31a1603 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -28,7 +28,9 @@
 #include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/mm.h>
 #include <linux/of.h>
+#include <linux/scatterlist.h>
 #include <linux/slab.h>
 #include <linux/uuid.h>
 
@@ -321,6 +323,177 @@ static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool mode_32bit,
 	return 0;
 }
 
+static int ffa_mem_first_frag(u32 func_id, phys_addr_t buf, u32 buf_sz,
+			      u32 frag_len, u32 len, u64 *handle)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(func_id, len, frag_len, buf, buf_sz, 0, 0, 0);
+
+	while (ret.a0 == FFA_MEM_OP_PAUSE)
+		ret = invoke_ffa_fn(FFA_MEM_OP_RESUME, ret.a1, ret.a2,
+				    0, 0, 0, 0, 0);
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	if (ret.a0 != FFA_SUCCESS)
+		return -EOPNOTSUPP;
+
+	if (handle)
+		*handle = PACK_HANDLE(ret.a2, ret.a3);
+
+	return frag_len;
+}
+
+static int ffa_mem_next_frag(u64 handle, u32 frag_len)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_MEM_FRAG_TX, HANDLE_LOW(handle),
+			    HANDLE_HIGH(handle), frag_len, 0, 0, 0, 0);
+
+	while (ret.a0 == FFA_MEM_OP_PAUSE)
+		ret = invoke_ffa_fn(FFA_MEM_OP_RESUME, ret.a1, ret.a2,
+				    0, 0, 0, 0, 0);
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	if (ret.a0 != FFA_MEM_FRAG_RX)
+		return -EOPNOTSUPP;
+
+	return ret.a3;
+}
+
+static int
+ffa_transmit_fragment(u32 func_id, phys_addr_t buf, u32 buf_sz, u32 frag_len,
+		      u32 len, u64 *handle, bool first)
+{
+	if (!first)
+		return ffa_mem_next_frag(*handle, frag_len);
+
+	return ffa_mem_first_frag(func_id, buf, buf_sz, frag_len, len, handle);
+}
+
+static u32 ffa_get_num_pages_sg(struct scatterlist *sg)
+{
+	u32 num_pages = 0;
+
+	do {
+		num_pages += sg->length / FFA_PAGE_SIZE;
+	} while ((sg = sg_next(sg)));
+
+	return num_pages;
+}
+
+static int
+ffa_setup_and_transmit(u32 func_id, void *buffer, u32 max_fragsize,
+		       struct ffa_mem_ops_args *args)
+{
+	int rc = 0;
+	bool first = true;
+	phys_addr_t addr = 0;
+	struct ffa_composite_mem_region *composite;
+	struct ffa_mem_region_addr_range *constituents;
+	struct ffa_mem_region_attributes *ep_mem_access;
+	struct ffa_mem_region *mem_region = buffer;
+	u32 idx, frag_len, length, buf_sz = 0, num_entries = sg_nents(args->sg);
+
+	mem_region->tag = args->tag;
+	mem_region->flags = args->flags;
+	mem_region->sender_id = drv_info->vm_id;
+	mem_region->attributes = FFA_MEM_NORMAL | FFA_MEM_WRITE_BACK |
+				 FFA_MEM_INNER_SHAREABLE;
+	ep_mem_access = &mem_region->ep_mem_access[0];
+
+	for (idx = 0; idx < args->nattrs; idx++, ep_mem_access++) {
+		ep_mem_access->receiver = args->attrs[idx].receiver;
+		ep_mem_access->attrs = args->attrs[idx].attrs;
+		ep_mem_access->composite_off = COMPOSITE_OFFSET(args->nattrs);
+	}
+	mem_region->ep_count = args->nattrs;
+
+	composite = buffer + COMPOSITE_OFFSET(args->nattrs);
+	composite->total_pg_cnt = ffa_get_num_pages_sg(args->sg);
+	composite->addr_range_cnt = num_entries;
+
+	length = COMPOSITE_CONSTITUENTS_OFFSET(args->nattrs, num_entries);
+	frag_len = COMPOSITE_CONSTITUENTS_OFFSET(args->nattrs, 0);
+	if (frag_len > max_fragsize)
+		return -ENXIO;
+
+	if (!args->use_txbuf) {
+		addr = virt_to_phys(buffer);
+		buf_sz = max_fragsize / FFA_PAGE_SIZE;
+	}
+
+	constituents = buffer + frag_len;
+	idx = 0;
+	do {
+		if (frag_len == max_fragsize) {
+			rc = ffa_transmit_fragment(func_id, addr, buf_sz,
+						   frag_len, length,
+						   &args->g_handle, first);
+			if (rc < 0)
+				return -ENXIO;
+
+			first = false;
+			idx = 0;
+			frag_len = 0;
+			constituents = buffer;
+		}
+
+		if ((void *)constituents - buffer > max_fragsize) {
+			pr_err("Memory Region Fragment > Tx Buffer size\n");
+			return -EFAULT;
+		}
+
+		constituents->address = sg_phys(args->sg);
+		constituents->pg_cnt = args->sg->length / FFA_PAGE_SIZE;
+		constituents++;
+		frag_len += sizeof(struct ffa_mem_region_addr_range);
+	} while ((args->sg = sg_next(args->sg)));
+
+	return ffa_transmit_fragment(func_id, addr, buf_sz, frag_len,
+				     length, &args->g_handle, first);
+}
+
+static int ffa_memory_ops(u32 func_id, struct ffa_mem_ops_args *args)
+{
+	int ret;
+	void *buffer;
+
+	if (!args->use_txbuf) {
+		buffer = alloc_pages_exact(RXTX_BUFFER_SIZE, GFP_KERNEL);
+		if (!buffer)
+			return -ENOMEM;
+	} else {
+		buffer = drv_info->tx_buffer;
+		mutex_lock(&drv_info->tx_lock);
+	}
+
+	ret = ffa_setup_and_transmit(func_id, buffer, RXTX_BUFFER_SIZE, args);
+
+	if (args->use_txbuf)
+		mutex_unlock(&drv_info->tx_lock);
+	else
+		free_pages_exact(buffer, RXTX_BUFFER_SIZE);
+
+	return ret < 0 ? ret : 0;
+}
+
+static int ffa_memory_reclaim(u64 g_handle, u32 flags)
+{
+	ffa_res_t ret;
+
+	ret = invoke_ffa_fn(FFA_MEM_RECLAIM, HANDLE_LOW(g_handle),
+			    HANDLE_HIGH(g_handle), flags, 0, 0, 0, 0);
+
+	if (ret.a0 == FFA_ERROR)
+		return ffa_to_linux_errno((int)ret.a2);
+
+	return 0;
+}
+
 static u32 ffa_api_version_get(void)
 {
 	return drv_info->version;
@@ -346,11 +519,22 @@ static int ffa_sync_send_receive(struct ffa_device *dev,
 				       dev->mode_32bit, data);
 }
 
+static int
+ffa_memory_share(struct ffa_device *dev, struct ffa_mem_ops_args *args)
+{
+	if (dev->mode_32bit)
+		return ffa_memory_ops(FFA_MEM_SHARE, args);
+
+	return ffa_memory_ops(FFA_FN_NATIVE(MEM_SHARE), args);
+}
+
 static const struct ffa_dev_ops ffa_ops = {
 	.api_version_get = ffa_api_version_get,
 	.partition_info_get = ffa_partition_info_get,
 	.mode_32bit_set = ffa_mode_32bit_set,
 	.sync_send_receive = ffa_sync_send_receive,
+	.memory_reclaim = ffa_memory_reclaim,
+	.memory_share = ffa_memory_share,
 };
 
 const struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index f0c68f3f1c6a..200019c20e39 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -115,6 +115,142 @@ struct ffa_send_direct_data {
 	unsigned long data4; /* w7/x7 */
 };
 
+struct ffa_mem_region_addr_range {
+	/* The base IPA of the constituent memory region, aligned to 4 kiB */
+	u64 address;
+	/* The number of 4 kiB pages in the constituent memory region. */
+	u32 pg_cnt;
+	u32 reserved;
+};
+
+struct ffa_composite_mem_region {
+	/*
+	 * The total number of 4 kiB pages included in this memory region. This
+	 * must be equal to the sum of page counts specified in each
+	 * `struct ffa_mem_region_addr_range`.
+	 */
+	u32 total_pg_cnt;
+	/* The number of constituents included in this memory region range */
+	u32 addr_range_cnt;
+	u64 reserved;
+	/** An array of `addr_range_cnt` memory region constituents. */
+	struct ffa_mem_region_addr_range constituents[];
+};
+
+struct ffa_mem_region_attributes {
+	/* The ID of the VM to which the memory is being given or shared. */
+	u16 receiver;
+	/*
+	 * The permissions with which the memory region should be mapped in the
+	 * receiver's page table.
+	 */
+#define FFA_MEM_EXEC	BIT(3)
+#define FFA_MEM_NO_EXEC	BIT(2)
+#define FFA_MEM_RW		BIT(1)
+#define FFA_MEM_RO		BIT(0)
+	u8 attrs;
+	/*
+	 * Flags used during FFA_MEM_RETRIEVE_REQ and FFA_MEM_RETRIEVE_RESP
+	 * for memory regions with multiple borrowers.
+	 */
+#define FFA_MEM_RETRIEVE_SELF_BORROWER	BIT(0)
+	u8 flag;
+	u32 composite_off;
+	/*
+	 * Offset in bytes from the start of the outer `ffa_memory_region` to
+	 * an `struct ffa_mem_region_addr_range`.
+	 */
+	u64 reserved;
+};
+
+struct ffa_mem_region {
+	/* The ID of the VM/owner which originally sent the memory region */
+	u16 sender_id;
+#define FFA_MEM_NORMAL		BIT(5)
+#define FFA_MEM_DEVICE		BIT(4)
+
+#define FFA_MEM_WRITE_BACK	(3 << 2)
+#define FFA_MEM_NON_CACHEABLE	(1 << 2)
+
+#define FFA_DEV_nGnRnE		(0 << 2)
+#define FFA_DEV_nGnRE		(1 << 2)
+#define FFA_DEV_nGRE		(2 << 2)
+#define FFA_DEV_GRE		(3 << 2)
+
+#define FFA_MEM_NON_SHAREABLE	(0)
+#define FFA_MEM_OUTER_SHAREABLE	(2)
+#define FFA_MEM_INNER_SHAREABLE	(3)
+	u8 attributes;
+	u8 reserved_0;
+/*
+ * Clear memory region contents after unmapping it from the sender and
+ * before mapping it for any receiver.
+ */
+#define FFA_MEM_CLEAR			BIT(0)
+/*
+ * Whether the hypervisor may time slice the memory sharing or retrieval
+ * operation.
+ */
+#define FFA_TIME_SLICE_ENABLE		BIT(1)
+
+#define FFA_MEM_RETRIEVE_TYPE_IN_RESP	(0 << 3)
+#define FFA_MEM_RETRIEVE_TYPE_SHARE	(1 << 3)
+#define FFA_MEM_RETRIEVE_TYPE_LEND	(2 << 3)
+#define FFA_MEM_RETRIEVE_TYPE_DONATE	(3 << 3)
+
+#define FFA_MEM_RETRIEVE_ADDR_ALIGN_HINT	BIT(9)
+#define FFA_MEM_RETRIEVE_ADDR_ALIGN(x)		((x) << 5)
+	/* Flags to control behaviour of the transaction. */
+	u32 flags;
+#define HANDLE_LOW_MASK		GENMASK_ULL(31, 0)
+#define HANDLE_HIGH_MASK	GENMASK_ULL(63, 32)
+#define HANDLE_LOW(x)		(u32)(FIELD_GET(HANDLE_LOW_MASK, (x)))
+#define	HANDLE_HIGH(x)		(u32)(FIELD_GET(HANDLE_HIGH_MASK, (x)))
+
+#define PACK_HANDLE(l, h)		\
+	(FIELD_PREP(HANDLE_LOW_MASK, (l)) | FIELD_PREP(HANDLE_HIGH_MASK, (h)))
+	/*
+	 * A globally-unique ID assigned by the hypervisor for a region
+	 * of memory being sent between VMs.
+	 */
+	u64 handle;
+	/*
+	 * An implementation defined value associated with the receiver and the
+	 * memory region.
+	 */
+	u64 tag;
+	u32 reserved_1;
+	/*
+	 * The number of `ffa_mem_region_attributes` entries included in this
+	 * transaction.
+	 */
+	u32 ep_count;
+	/*
+	 * An array of endpoint memory access descriptors.
+	 * Each one specifies a memory region offset, an endpoint and the
+	 * attributes with which this memory region should be mapped in that
+	 * endpoint's page table.
+	 */
+	struct ffa_mem_region_attributes ep_mem_access[];
+};
+
+#define	COMPOSITE_OFFSET(x)	\
+	(offsetof(struct ffa_mem_region, ep_mem_access[x]))
+#define CONSTITUENTS_OFFSET(x)	\
+	(offsetof(struct ffa_composite_mem_region, constituents[x]))
+#define COMPOSITE_CONSTITUENTS_OFFSET(x, y)	\
+	(COMPOSITE_OFFSET(x) + CONSTITUENTS_OFFSET(y))
+
+struct ffa_mem_ops_args {
+	bool use_txbuf;
+	u32 nattrs;
+	u32 flags;
+	u64 tag;
+	u64 g_handle;
+	struct scatterlist *sg;
+	struct ffa_mem_region_attributes *attrs;
+};
+
 struct ffa_dev_ops {
 	u32 (*api_version_get)(void);
 	int (*partition_info_get)(const char *uuid_str,
@@ -122,6 +258,9 @@ struct ffa_dev_ops {
 	void (*mode_32bit_set)(struct ffa_device *dev);
 	int (*sync_send_receive)(struct ffa_device *dev,
 				 struct ffa_send_direct_data *data);
+	int (*memory_reclaim)(u64 g_handle, u32 flags);
+	int (*memory_share)(struct ffa_device *dev,
+			    struct ffa_mem_ops_args *args);
 };
 
 #endif /* _LINUX_ARM_FFA_H */
-- 
2.25.1


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

^ permalink raw reply related	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
  2021-03-25 14:32   ` Sudeep Holla
@ 2021-03-25 14:41     ` Mark Rutland
  -1 siblings, 0 replies; 36+ messages in thread
From: Mark Rutland @ 2021-03-25 14:41 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Andrew Walbran,
	David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon, Michael Kelley

Hi Sudeep,

On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> for Armv8-A specification makes use of x0-x7 as parameter and result
> registers.
> 
> Current SMCCC interface in the kernel just use x0-x7 as parameter and
> x0-x3 as result registers. Let us add new interface to support x0-x7
> as parameter and result registers. This can be extended to include
> x8-x17 when there are users for the same.

Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
thread I'd suggested we should deal with the whole set of SMCCCv1.2
registers now to avoid future churn in this area (using struct both for
the arguments and return values).

How painful would it be to extend this patch to do that?

Thanks,
Mark.

> 
> Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/kernel/asm-offsets.c |  4 +++
>  arch/arm64/kernel/smccc-call.S  | 22 +++++++++++++++
>  include/linux/arm-smccc.h       | 50 +++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+)
> 
> Hi Will/Catalin,
> 
> I seemed to have missed you on these patches. I realised only when I was
> collecting Acks to get this series merged. The change here is simple, it
> would be good if you can review and ack if you are OK with it so that I
> can get the serier merged via ARM SoC.
> 
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index a36e2fc330d4..57ffea2920b8 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -132,6 +132,10 @@ int main(void)
>    DEFINE(ARM_SMCCC_RES_X2_OFFS,		offsetof(struct arm_smccc_res, a2));
>    DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,	offsetof(struct arm_smccc_quirk, id));
>    DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS,	offsetof(struct arm_smccc_quirk, state));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X2_OFFS,	offsetof(struct arm_smccc_v1_2_res, a2));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X4_OFFS,	offsetof(struct arm_smccc_v1_2_res, a4));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X6_OFFS,	offsetof(struct arm_smccc_v1_2_res, a6));
>    BLANK();
>    DEFINE(HIBERN_PBE_ORIG,	offsetof(struct pbe, orig_address));
>    DEFINE(HIBERN_PBE_ADDR,	offsetof(struct pbe, address));
> diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> index d62447964ed9..0ea15c1742f3 100644
> --- a/arch/arm64/kernel/smccc-call.S
> +++ b/arch/arm64/kernel/smccc-call.S
> @@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc)
>  	SMCCC	hvc
>  SYM_FUNC_END(__arm_smccc_hvc)
>  EXPORT_SYMBOL(__arm_smccc_hvc)
> +
> +	.macro SMCCC_v1_2 instr
> +	.cfi_startproc
> +	\instr #0
> +	ldr x8, [sp]
> +	stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS]
> +	stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS]
> +	stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS]
> +	stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS]
> +	ret
> +	.cfi_endproc
> +.endm
> +
> +SYM_FUNC_START(arm_smccc_v1_2_hvc)
> +	SMCCC_v1_2 hvc
> +SYM_FUNC_END(arm_smccc_v1_2_hvc)
> +EXPORT_SYMBOL(arm_smccc_v1_2_hvc)
> +
> +SYM_FUNC_START(arm_smccc_v1_2_smc)
> +	SMCCC_v1_2 smc
> +SYM_FUNC_END(arm_smccc_v1_2_smc)
> +EXPORT_SYMBOL(arm_smccc_v1_2_smc)
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 62c54234576c..0b8fa285a054 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -186,6 +186,56 @@ struct arm_smccc_res {
>  	unsigned long a3;
>  };
>  
> +#ifdef CONFIG_ARM64
> +/* TODO Need to implement for ARM too */
> +/**
> + * struct arm_smccc_v1_2_res - Result from SMC/HVC call
> + * @a0-a7 result values from registers 0 to 7
> + */
> +struct arm_smccc_v1_2_res {
> +	unsigned long a0;
> +	unsigned long a1;
> +	unsigned long a2;
> +	unsigned long a3;
> +	unsigned long a4;
> +	unsigned long a5;
> +	unsigned long a6;
> +	unsigned long a7;
> +};
> +
> +/**
> + * arm_smccc_v1_2_hvc() - make HVC calls
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 7
> + *
> + * This function is used to make HVC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied to registers
> + * 0 to 7 prior to the HVC instruction. The return values are updated with
> + * the content from register 0 to 7 on return from the HVC instruction.
> + */
> +asmlinkage
> +void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2,
> +			unsigned long a3, unsigned long a4, unsigned long a5,
> +			unsigned long a6, unsigned long a7,
> +			struct arm_smccc_v1_2_res  *res);
> +
> +/**
> + * arm_smccc_v1_2_smc() - make SMC calls
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 7
> + *
> + * This function is used to make SMC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied to registers
> + * 0 to 7 prior to the SMC instruction. The return values are updated with
> + * the content from register 0 to 7 on return from the SMC instruction.
> + */
> +asmlinkage
> +void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2,
> +			unsigned long a3, unsigned long a4, unsigned long a5,
> +			unsigned long a6, unsigned long a7,
> +			struct arm_smccc_v1_2_res  *res);
> +#endif
> +
>  /**
>   * struct arm_smccc_quirk - Contains quirk information
>   * @id: quirk identification
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
@ 2021-03-25 14:41     ` Mark Rutland
  0 siblings, 0 replies; 36+ messages in thread
From: Mark Rutland @ 2021-03-25 14:41 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Andrew Walbran,
	David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon, Michael Kelley

Hi Sudeep,

On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> for Armv8-A specification makes use of x0-x7 as parameter and result
> registers.
> 
> Current SMCCC interface in the kernel just use x0-x7 as parameter and
> x0-x3 as result registers. Let us add new interface to support x0-x7
> as parameter and result registers. This can be extended to include
> x8-x17 when there are users for the same.

Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
thread I'd suggested we should deal with the whole set of SMCCCv1.2
registers now to avoid future churn in this area (using struct both for
the arguments and return values).

How painful would it be to extend this patch to do that?

Thanks,
Mark.

> 
> Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm64/kernel/asm-offsets.c |  4 +++
>  arch/arm64/kernel/smccc-call.S  | 22 +++++++++++++++
>  include/linux/arm-smccc.h       | 50 +++++++++++++++++++++++++++++++++
>  3 files changed, 76 insertions(+)
> 
> Hi Will/Catalin,
> 
> I seemed to have missed you on these patches. I realised only when I was
> collecting Acks to get this series merged. The change here is simple, it
> would be good if you can review and ack if you are OK with it so that I
> can get the serier merged via ARM SoC.
> 
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index a36e2fc330d4..57ffea2920b8 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -132,6 +132,10 @@ int main(void)
>    DEFINE(ARM_SMCCC_RES_X2_OFFS,		offsetof(struct arm_smccc_res, a2));
>    DEFINE(ARM_SMCCC_QUIRK_ID_OFFS,	offsetof(struct arm_smccc_quirk, id));
>    DEFINE(ARM_SMCCC_QUIRK_STATE_OFFS,	offsetof(struct arm_smccc_quirk, state));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X2_OFFS,	offsetof(struct arm_smccc_v1_2_res, a2));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X4_OFFS,	offsetof(struct arm_smccc_v1_2_res, a4));
> +  DEFINE(ARM_SMCCC_V1_2_RES_X6_OFFS,	offsetof(struct arm_smccc_v1_2_res, a6));
>    BLANK();
>    DEFINE(HIBERN_PBE_ORIG,	offsetof(struct pbe, orig_address));
>    DEFINE(HIBERN_PBE_ADDR,	offsetof(struct pbe, address));
> diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> index d62447964ed9..0ea15c1742f3 100644
> --- a/arch/arm64/kernel/smccc-call.S
> +++ b/arch/arm64/kernel/smccc-call.S
> @@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc)
>  	SMCCC	hvc
>  SYM_FUNC_END(__arm_smccc_hvc)
>  EXPORT_SYMBOL(__arm_smccc_hvc)
> +
> +	.macro SMCCC_v1_2 instr
> +	.cfi_startproc
> +	\instr #0
> +	ldr x8, [sp]
> +	stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS]
> +	stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS]
> +	stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS]
> +	stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS]
> +	ret
> +	.cfi_endproc
> +.endm
> +
> +SYM_FUNC_START(arm_smccc_v1_2_hvc)
> +	SMCCC_v1_2 hvc
> +SYM_FUNC_END(arm_smccc_v1_2_hvc)
> +EXPORT_SYMBOL(arm_smccc_v1_2_hvc)
> +
> +SYM_FUNC_START(arm_smccc_v1_2_smc)
> +	SMCCC_v1_2 smc
> +SYM_FUNC_END(arm_smccc_v1_2_smc)
> +EXPORT_SYMBOL(arm_smccc_v1_2_smc)
> diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> index 62c54234576c..0b8fa285a054 100644
> --- a/include/linux/arm-smccc.h
> +++ b/include/linux/arm-smccc.h
> @@ -186,6 +186,56 @@ struct arm_smccc_res {
>  	unsigned long a3;
>  };
>  
> +#ifdef CONFIG_ARM64
> +/* TODO Need to implement for ARM too */
> +/**
> + * struct arm_smccc_v1_2_res - Result from SMC/HVC call
> + * @a0-a7 result values from registers 0 to 7
> + */
> +struct arm_smccc_v1_2_res {
> +	unsigned long a0;
> +	unsigned long a1;
> +	unsigned long a2;
> +	unsigned long a3;
> +	unsigned long a4;
> +	unsigned long a5;
> +	unsigned long a6;
> +	unsigned long a7;
> +};
> +
> +/**
> + * arm_smccc_v1_2_hvc() - make HVC calls
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 7
> + *
> + * This function is used to make HVC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied to registers
> + * 0 to 7 prior to the HVC instruction. The return values are updated with
> + * the content from register 0 to 7 on return from the HVC instruction.
> + */
> +asmlinkage
> +void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2,
> +			unsigned long a3, unsigned long a4, unsigned long a5,
> +			unsigned long a6, unsigned long a7,
> +			struct arm_smccc_v1_2_res  *res);
> +
> +/**
> + * arm_smccc_v1_2_smc() - make SMC calls
> + * @a0-a7: arguments passed in registers 0 to 7
> + * @res: result values from registers 0 to 7
> + *
> + * This function is used to make SMC calls following SMC Calling Convention
> + * v1.2 or above. The content of the supplied param are copied to registers
> + * 0 to 7 prior to the SMC instruction. The return values are updated with
> + * the content from register 0 to 7 on return from the SMC instruction.
> + */
> +asmlinkage
> +void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2,
> +			unsigned long a3, unsigned long a4, unsigned long a5,
> +			unsigned long a6, unsigned long a7,
> +			struct arm_smccc_v1_2_res  *res);
> +#endif
> +
>  /**
>   * struct arm_smccc_quirk - Contains quirk information
>   * @id: quirk identification
> -- 
> 2.25.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  2021-03-25 14:32   ` Sudeep Holla
@ 2021-03-26  5:05     ` Sumit Garg
  -1 siblings, 0 replies; 36+ messages in thread
From: Sumit Garg @ 2021-03-26  5:05 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, Devicetree List, Trilok Soni, arve,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

Hi Sudeep,

Apologies for catching up late on this patch-set.

On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Since the FF-A v1.0 specification doesn't list the UUID of all the
> partitions in the discovery API, we need to specify the UUID of the
> partitions that need to be accessed by drivers within the kernel.
>

Wouldn't we be able to implement auto-discovery of ffa partitions? I
think enumeration of ffa partitions on FFA bus should be quite similar
to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
these redundant DT entries for every ffa partition which IMHO would
bloat up device trees for every platform.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tee/optee/device.c#n153

-Sumit

> This binding to provide the list of partitions that kernel drivers
> may need to access.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  .../devicetree/bindings/arm/arm,ffa.yaml      | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,ffa.yaml b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> new file mode 100644
> index 000000000000..42ed33464ba7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/arm,ffa.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Arm Firmware Framework for Arm v8-A (in-kernel users)
> +
> +maintainers:
> +  - Sudeep Holla <sudeep.holla@arm.com>
> +
> +description: |
> +  Firmware frameworks implementing partition according to the FF-A
> +  specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
> +  Framework for Arm v8-A") [0], providing services to be used by other
> +  partitions.
> +
> +  [0] https://developer.arm.com/docs/den0077/latest
> +
> +properties:
> +  $nodename:
> +    const: ffa
> +
> +  compatible:
> +    oneOf:
> +      - const: arm,ffa-1.0
> +
> +patternProperties:
> +  "^ffa-partition-[0-9]+$":
> +    type: object
> +    description: One or more child nodes, each describing an FFA partition.
> +    properties:
> +      $nodename:
> +        const: ffa_partition
> +
> +      compatible:
> +        const: arm,ffa-1.0-partition
> +
> +      uuid:
> +        $ref: '/schemas/types.yaml#definitions/string'
> +        description: |
> +          The 128-bit UUID [2] of the service implemented by this partition.
> +
> +          [2] https://tools.ietf.org/html/rfc4122
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ffa {
> +      compatible = "arm,ffa-1.0";
> +
> +      ffa-partition-0 {
> +        compatible = "arm,ffa-1.0-partition";
> +        uuid = "12345678-9abc-def0-1234-56789abcdef0";
> +      };
> +    };
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
@ 2021-03-26  5:05     ` Sumit Garg
  0 siblings, 0 replies; 36+ messages in thread
From: Sumit Garg @ 2021-03-26  5:05 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, Devicetree List, Trilok Soni, arve,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

Hi Sudeep,

Apologies for catching up late on this patch-set.

On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> Since the FF-A v1.0 specification doesn't list the UUID of all the
> partitions in the discovery API, we need to specify the UUID of the
> partitions that need to be accessed by drivers within the kernel.
>

Wouldn't we be able to implement auto-discovery of ffa partitions? I
think enumeration of ffa partitions on FFA bus should be quite similar
to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
these redundant DT entries for every ffa partition which IMHO would
bloat up device trees for every platform.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/tee/optee/device.c#n153

-Sumit

> This binding to provide the list of partitions that kernel drivers
> may need to access.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  .../devicetree/bindings/arm/arm,ffa.yaml      | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
>
> diff --git a/Documentation/devicetree/bindings/arm/arm,ffa.yaml b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> new file mode 100644
> index 000000000000..42ed33464ba7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/arm,ffa.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Arm Firmware Framework for Arm v8-A (in-kernel users)
> +
> +maintainers:
> +  - Sudeep Holla <sudeep.holla@arm.com>
> +
> +description: |
> +  Firmware frameworks implementing partition according to the FF-A
> +  specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
> +  Framework for Arm v8-A") [0], providing services to be used by other
> +  partitions.
> +
> +  [0] https://developer.arm.com/docs/den0077/latest
> +
> +properties:
> +  $nodename:
> +    const: ffa
> +
> +  compatible:
> +    oneOf:
> +      - const: arm,ffa-1.0
> +
> +patternProperties:
> +  "^ffa-partition-[0-9]+$":
> +    type: object
> +    description: One or more child nodes, each describing an FFA partition.
> +    properties:
> +      $nodename:
> +        const: ffa_partition
> +
> +      compatible:
> +        const: arm,ffa-1.0-partition
> +
> +      uuid:
> +        $ref: '/schemas/types.yaml#definitions/string'
> +        description: |
> +          The 128-bit UUID [2] of the service implemented by this partition.
> +
> +          [2] https://tools.ietf.org/html/rfc4122
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ffa {
> +      compatible = "arm,ffa-1.0";
> +
> +      ffa-partition-0 {
> +        compatible = "arm,ffa-1.0-partition";
> +        uuid = "12345678-9abc-def0-1234-56789abcdef0";
> +      };
> +    };
> --
> 2.25.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  2021-03-26  5:05     ` Sumit Garg
@ 2021-03-26 10:55       ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-26 10:55 UTC (permalink / raw)
  To: Sumit Garg
  Cc: linux-arm-kernel, Devicetree List, Trilok Soni, arve,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> Hi Sudeep,
>
> Apologies for catching up late on this patch-set.
>
> On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > partitions in the discovery API, we need to specify the UUID of the
> > partitions that need to be accessed by drivers within the kernel.
> >
>
> Wouldn't we be able to implement auto-discovery of ffa partitions? I
> think enumeration of ffa partitions on FFA bus should be quite similar
> to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> these redundant DT entries for every ffa partition which IMHO would
> bloat up device trees for every platform.
>

Any suggestions on how to ? Clearly spec doesn't have that provision, I
had raised this point in the past. Jens has similar concern and he did
ask the same[1]. As I replied to him in that thread[2].

I am open to suggestion on how to auto-discover, currently as I see spec
doesn't support it.

--
Regards,
Sudeep

[1] https://lore.kernel.org/linux-arm-kernel/20201216134659.GA4146223@jade/
[2] https://lore.kernel.org/linux-arm-kernel/20210113092236.pnabzaufzuzwprmw@bogus/

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
@ 2021-03-26 10:55       ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-26 10:55 UTC (permalink / raw)
  To: Sumit Garg
  Cc: linux-arm-kernel, Devicetree List, Trilok Soni, arve,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> Hi Sudeep,
>
> Apologies for catching up late on this patch-set.
>
> On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > partitions in the discovery API, we need to specify the UUID of the
> > partitions that need to be accessed by drivers within the kernel.
> >
>
> Wouldn't we be able to implement auto-discovery of ffa partitions? I
> think enumeration of ffa partitions on FFA bus should be quite similar
> to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> these redundant DT entries for every ffa partition which IMHO would
> bloat up device trees for every platform.
>

Any suggestions on how to ? Clearly spec doesn't have that provision, I
had raised this point in the past. Jens has similar concern and he did
ask the same[1]. As I replied to him in that thread[2].

I am open to suggestion on how to auto-discover, currently as I see spec
doesn't support it.

--
Regards,
Sudeep

[1] https://lore.kernel.org/linux-arm-kernel/20201216134659.GA4146223@jade/
[2] https://lore.kernel.org/linux-arm-kernel/20210113092236.pnabzaufzuzwprmw@bogus/

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  2021-03-26 10:55       ` Sudeep Holla
@ 2021-03-26 11:56         ` Sumit Garg
  -1 siblings, 0 replies; 36+ messages in thread
From: Sumit Garg @ 2021-03-26 11:56 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, Devicetree List, Trilok Soni, arve,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > Hi Sudeep,
> >
> > Apologies for catching up late on this patch-set.
> >
> > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > partitions in the discovery API, we need to specify the UUID of the
> > > partitions that need to be accessed by drivers within the kernel.
> > >
> >
> > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > think enumeration of ffa partitions on FFA bus should be quite similar
> > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > these redundant DT entries for every ffa partition which IMHO would
> > bloat up device trees for every platform.
> >
>
> Any suggestions on how to ? Clearly spec doesn't have that provision, I
> had raised this point in the past. Jens has similar concern and he did
> ask the same[1]. As I replied to him in that thread[2].
>
> I am open to suggestion on how to auto-discover, currently as I see spec
> doesn't support it.
>

Thanks for sharing links to prior discussions and I can see that
currently spec doesn't support it. But from an implementation
perspective, I can't find any reason that we can't support
auto-discover. Have a look at below proposed simple FFA ABI:

FFA_LIST_PARTITIONS

- No input params.
- Returns an array of secure partition UUIDs to which this non-secure
virtual/physical FF-A instance is allowed to communicate with.

I think with auto-discovery, one of the major benefits is that if the
OEM is using a common platform to cater to multiple use-cases which
rely on different secure partitions then OEM doesn't have to bother
about shipping separate DTs.

-Sumit

> --
> Regards,
> Sudeep
>
> [1] https://lore.kernel.org/linux-arm-kernel/20201216134659.GA4146223@jade/
> [2] https://lore.kernel.org/linux-arm-kernel/20210113092236.pnabzaufzuzwprmw@bogus/

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
@ 2021-03-26 11:56         ` Sumit Garg
  0 siblings, 0 replies; 36+ messages in thread
From: Sumit Garg @ 2021-03-26 11:56 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, Devicetree List, Trilok Soni, arve,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > Hi Sudeep,
> >
> > Apologies for catching up late on this patch-set.
> >
> > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > partitions in the discovery API, we need to specify the UUID of the
> > > partitions that need to be accessed by drivers within the kernel.
> > >
> >
> > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > think enumeration of ffa partitions on FFA bus should be quite similar
> > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > these redundant DT entries for every ffa partition which IMHO would
> > bloat up device trees for every platform.
> >
>
> Any suggestions on how to ? Clearly spec doesn't have that provision, I
> had raised this point in the past. Jens has similar concern and he did
> ask the same[1]. As I replied to him in that thread[2].
>
> I am open to suggestion on how to auto-discover, currently as I see spec
> doesn't support it.
>

Thanks for sharing links to prior discussions and I can see that
currently spec doesn't support it. But from an implementation
perspective, I can't find any reason that we can't support
auto-discover. Have a look at below proposed simple FFA ABI:

FFA_LIST_PARTITIONS

- No input params.
- Returns an array of secure partition UUIDs to which this non-secure
virtual/physical FF-A instance is allowed to communicate with.

I think with auto-discovery, one of the major benefits is that if the
OEM is using a common platform to cater to multiple use-cases which
rely on different secure partitions then OEM doesn't have to bother
about shipping separate DTs.

-Sumit

> --
> Regards,
> Sudeep
>
> [1] https://lore.kernel.org/linux-arm-kernel/20201216134659.GA4146223@jade/
> [2] https://lore.kernel.org/linux-arm-kernel/20210113092236.pnabzaufzuzwprmw@bogus/

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
  2021-03-25 14:41     ` Mark Rutland
@ 2021-03-26 12:18       ` Mark Rutland
  -1 siblings, 0 replies; 36+ messages in thread
From: Mark Rutland @ 2021-03-26 12:18 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Andrew Walbran,
	David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon, Michael Kelley

On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> Hi Sudeep,
> 
> On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > for Armv8-A specification makes use of x0-x7 as parameter and result
> > registers.
> > 
> > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > x0-x3 as result registers. Let us add new interface to support x0-x7
> > as parameter and result registers. This can be extended to include
> > x8-x17 when there are users for the same.
> 
> Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> thread I'd suggested we should deal with the whole set of SMCCCv1.2
> registers now to avoid future churn in this area (using struct both for
> the arguments and return values).
> 
> How painful would it be to extend this patch to do that?

I *think* the major change with this would be making the interfaces:

void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
			     struct arm_smccc_1_2_res *res);

... and callers manipulating the structs directly, with arm64 having
more fields, e.g.

// arm64
struct arm_smccc_1_2_args {
	unsigned long a1;
	...
	unsigned long a17;
}

struct arm_smccc_1_2_res {
	unsigned long a0;
	...
	unsigned long a17;
}

// arm
struct arm_smccc_1_2_args {
	unsigned long a1;
	...
	unsigned long a7;
}

struct arm_smccc_1_2_res {
	unsigned long a0;
	...
	unsigned long a7;
}

I think that can be hidden in the FF-A wrappers, so that doesn't need to
be what FF-A drivers see. Does that sound plausible, or is that painful?
 
> > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));

As a general nit, for consistency with the existing arm_smccc_1_1 code,
could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
`arm_smccc_1_2` ?

FWIW, other than the above comments, this all looks good to me

Thanks,
Mark.

> > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> > index d62447964ed9..0ea15c1742f3 100644
> > --- a/arch/arm64/kernel/smccc-call.S
> > +++ b/arch/arm64/kernel/smccc-call.S
> > @@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc)
> >  	SMCCC	hvc
> >  SYM_FUNC_END(__arm_smccc_hvc)
> >  EXPORT_SYMBOL(__arm_smccc_hvc)
> > +
> > +	.macro SMCCC_v1_2 instr
> > +	.cfi_startproc
> > +	\instr #0
> > +	ldr x8, [sp]
> > +	stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS]
> > +	stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS]
> > +	stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS]
> > +	stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS]
> > +	ret
> > +	.cfi_endproc
> > +.endm
> > +
> > +SYM_FUNC_START(arm_smccc_v1_2_hvc)
> > +	SMCCC_v1_2 hvc
> > +SYM_FUNC_END(arm_smccc_v1_2_hvc)
> > +EXPORT_SYMBOL(arm_smccc_v1_2_hvc)
> > +
> > +SYM_FUNC_START(arm_smccc_v1_2_smc)
> > +	SMCCC_v1_2 smc
> > +SYM_FUNC_END(arm_smccc_v1_2_smc)
> > +EXPORT_SYMBOL(arm_smccc_v1_2_smc)
> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > index 62c54234576c..0b8fa285a054 100644
> > --- a/include/linux/arm-smccc.h
> > +++ b/include/linux/arm-smccc.h
> > @@ -186,6 +186,56 @@ struct arm_smccc_res {
> >  	unsigned long a3;
> >  };
> >  
> > +#ifdef CONFIG_ARM64
> > +/* TODO Need to implement for ARM too */
> > +/**
> > + * struct arm_smccc_v1_2_res - Result from SMC/HVC call
> > + * @a0-a7 result values from registers 0 to 7
> > + */
> > +struct arm_smccc_v1_2_res {
> > +	unsigned long a0;
> > +	unsigned long a1;
> > +	unsigned long a2;
> > +	unsigned long a3;
> > +	unsigned long a4;
> > +	unsigned long a5;
> > +	unsigned long a6;
> > +	unsigned long a7;
> > +};
> > +
> > +/**
> > + * arm_smccc_v1_2_hvc() - make HVC calls
> > + * @a0-a7: arguments passed in registers 0 to 7
> > + * @res: result values from registers 0 to 7
> > + *
> > + * This function is used to make HVC calls following SMC Calling Convention
> > + * v1.2 or above. The content of the supplied param are copied to registers
> > + * 0 to 7 prior to the HVC instruction. The return values are updated with
> > + * the content from register 0 to 7 on return from the HVC instruction.
> > + */
> > +asmlinkage
> > +void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2,
> > +			unsigned long a3, unsigned long a4, unsigned long a5,
> > +			unsigned long a6, unsigned long a7,
> > +			struct arm_smccc_v1_2_res  *res);
> > +
> > +/**
> > + * arm_smccc_v1_2_smc() - make SMC calls
> > + * @a0-a7: arguments passed in registers 0 to 7
> > + * @res: result values from registers 0 to 7
> > + *
> > + * This function is used to make SMC calls following SMC Calling Convention
> > + * v1.2 or above. The content of the supplied param are copied to registers
> > + * 0 to 7 prior to the SMC instruction. The return values are updated with
> > + * the content from register 0 to 7 on return from the SMC instruction.
> > + */
> > +asmlinkage
> > +void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2,
> > +			unsigned long a3, unsigned long a4, unsigned long a5,
> > +			unsigned long a6, unsigned long a7,
> > +			struct arm_smccc_v1_2_res  *res);
> > +#endif
> > +
> >  /**
> >   * struct arm_smccc_quirk - Contains quirk information
> >   * @id: quirk identification
> > -- 
> > 2.25.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
@ 2021-03-26 12:18       ` Mark Rutland
  0 siblings, 0 replies; 36+ messages in thread
From: Mark Rutland @ 2021-03-26 12:18 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Andrew Walbran,
	David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon, Michael Kelley

On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> Hi Sudeep,
> 
> On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > for Armv8-A specification makes use of x0-x7 as parameter and result
> > registers.
> > 
> > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > x0-x3 as result registers. Let us add new interface to support x0-x7
> > as parameter and result registers. This can be extended to include
> > x8-x17 when there are users for the same.
> 
> Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> thread I'd suggested we should deal with the whole set of SMCCCv1.2
> registers now to avoid future churn in this area (using struct both for
> the arguments and return values).
> 
> How painful would it be to extend this patch to do that?

I *think* the major change with this would be making the interfaces:

void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
			     struct arm_smccc_1_2_res *res);

... and callers manipulating the structs directly, with arm64 having
more fields, e.g.

// arm64
struct arm_smccc_1_2_args {
	unsigned long a1;
	...
	unsigned long a17;
}

struct arm_smccc_1_2_res {
	unsigned long a0;
	...
	unsigned long a17;
}

// arm
struct arm_smccc_1_2_args {
	unsigned long a1;
	...
	unsigned long a7;
}

struct arm_smccc_1_2_res {
	unsigned long a0;
	...
	unsigned long a7;
}

I think that can be hidden in the FF-A wrappers, so that doesn't need to
be what FF-A drivers see. Does that sound plausible, or is that painful?
 
> > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));

As a general nit, for consistency with the existing arm_smccc_1_1 code,
could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
`arm_smccc_1_2` ?

FWIW, other than the above comments, this all looks good to me

Thanks,
Mark.

> > diff --git a/arch/arm64/kernel/smccc-call.S b/arch/arm64/kernel/smccc-call.S
> > index d62447964ed9..0ea15c1742f3 100644
> > --- a/arch/arm64/kernel/smccc-call.S
> > +++ b/arch/arm64/kernel/smccc-call.S
> > @@ -43,3 +43,25 @@ SYM_FUNC_START(__arm_smccc_hvc)
> >  	SMCCC	hvc
> >  SYM_FUNC_END(__arm_smccc_hvc)
> >  EXPORT_SYMBOL(__arm_smccc_hvc)
> > +
> > +	.macro SMCCC_v1_2 instr
> > +	.cfi_startproc
> > +	\instr #0
> > +	ldr x8, [sp]
> > +	stp x0, x1, [x8, #ARM_SMCCC_V1_2_RES_X0_OFFS]
> > +	stp x2, x3, [x8, #ARM_SMCCC_V1_2_RES_X2_OFFS]
> > +	stp x4, x5, [x8, #ARM_SMCCC_V1_2_RES_X4_OFFS]
> > +	stp x6, x7, [x8, #ARM_SMCCC_V1_2_RES_X6_OFFS]
> > +	ret
> > +	.cfi_endproc
> > +.endm
> > +
> > +SYM_FUNC_START(arm_smccc_v1_2_hvc)
> > +	SMCCC_v1_2 hvc
> > +SYM_FUNC_END(arm_smccc_v1_2_hvc)
> > +EXPORT_SYMBOL(arm_smccc_v1_2_hvc)
> > +
> > +SYM_FUNC_START(arm_smccc_v1_2_smc)
> > +	SMCCC_v1_2 smc
> > +SYM_FUNC_END(arm_smccc_v1_2_smc)
> > +EXPORT_SYMBOL(arm_smccc_v1_2_smc)
> > diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h
> > index 62c54234576c..0b8fa285a054 100644
> > --- a/include/linux/arm-smccc.h
> > +++ b/include/linux/arm-smccc.h
> > @@ -186,6 +186,56 @@ struct arm_smccc_res {
> >  	unsigned long a3;
> >  };
> >  
> > +#ifdef CONFIG_ARM64
> > +/* TODO Need to implement for ARM too */
> > +/**
> > + * struct arm_smccc_v1_2_res - Result from SMC/HVC call
> > + * @a0-a7 result values from registers 0 to 7
> > + */
> > +struct arm_smccc_v1_2_res {
> > +	unsigned long a0;
> > +	unsigned long a1;
> > +	unsigned long a2;
> > +	unsigned long a3;
> > +	unsigned long a4;
> > +	unsigned long a5;
> > +	unsigned long a6;
> > +	unsigned long a7;
> > +};
> > +
> > +/**
> > + * arm_smccc_v1_2_hvc() - make HVC calls
> > + * @a0-a7: arguments passed in registers 0 to 7
> > + * @res: result values from registers 0 to 7
> > + *
> > + * This function is used to make HVC calls following SMC Calling Convention
> > + * v1.2 or above. The content of the supplied param are copied to registers
> > + * 0 to 7 prior to the HVC instruction. The return values are updated with
> > + * the content from register 0 to 7 on return from the HVC instruction.
> > + */
> > +asmlinkage
> > +void arm_smccc_v1_2_hvc(unsigned long a0, unsigned long a1, unsigned long a2,
> > +			unsigned long a3, unsigned long a4, unsigned long a5,
> > +			unsigned long a6, unsigned long a7,
> > +			struct arm_smccc_v1_2_res  *res);
> > +
> > +/**
> > + * arm_smccc_v1_2_smc() - make SMC calls
> > + * @a0-a7: arguments passed in registers 0 to 7
> > + * @res: result values from registers 0 to 7
> > + *
> > + * This function is used to make SMC calls following SMC Calling Convention
> > + * v1.2 or above. The content of the supplied param are copied to registers
> > + * 0 to 7 prior to the SMC instruction. The return values are updated with
> > + * the content from register 0 to 7 on return from the SMC instruction.
> > + */
> > +asmlinkage
> > +void arm_smccc_v1_2_smc(unsigned long a0, unsigned long a1, unsigned long a2,
> > +			unsigned long a3, unsigned long a4, unsigned long a5,
> > +			unsigned long a6, unsigned long a7,
> > +			struct arm_smccc_v1_2_res  *res);
> > +#endif
> > +
> >  /**
> >   * struct arm_smccc_quirk - Contains quirk information
> >   * @id: quirk identification
> > -- 
> > 2.25.1
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
  2021-03-26 12:18       ` Mark Rutland
@ 2021-03-26 12:51         ` Sudeep Holla
  -1 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-26 12:51 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Sudeep Holla,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon, Michael Kelley

On Fri, Mar 26, 2021 at 12:18:50PM +0000, Mark Rutland wrote:
> On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> > Hi Sudeep,
> >
> > On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > > for Armv8-A specification makes use of x0-x7 as parameter and result
> > > registers.
> > >
> > > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > > x0-x3 as result registers. Let us add new interface to support x0-x7
> > > as parameter and result registers. This can be extended to include
> > > x8-x17 when there are users for the same.
> >
> > Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> > thread I'd suggested we should deal with the whole set of SMCCCv1.2
> > registers now to avoid future churn in this area (using struct both for
> > the arguments and return values).
> >
> > How painful would it be to extend this patch to do that?
>
> I *think* the major change with this would be making the interfaces:
>
> void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
> 			     struct arm_smccc_1_2_res *res);
>
> ... and callers manipulating the structs directly, with arm64 having
> more fields, e.g.
>
> // arm64
> struct arm_smccc_1_2_args {
> 	unsigned long a1;
> 	...
> 	unsigned long a17;
> }
>
> struct arm_smccc_1_2_res {
> 	unsigned long a0;
> 	...
> 	unsigned long a17;
> }
>
> // arm
> struct arm_smccc_1_2_args {
> 	unsigned long a1;
> 	...
> 	unsigned long a7;
> }
>
> struct arm_smccc_1_2_res {
> 	unsigned long a0;
> 	...
> 	unsigned long a7;
> }
>
> I think that can be hidden in the FF-A wrappers, so that doesn't need to
> be what FF-A drivers see. Does that sound plausible, or is that painful?
>

Sounds possible, will give it a try.

> > > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
>
> As a general nit, for consistency with the existing arm_smccc_1_1 code,
> could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
> `arm_smccc_1_2` ?
>

Sure, makes sense.

> FWIW, other than the above comments, this all looks good to me
>

Thanks.

--
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
@ 2021-03-26 12:51         ` Sudeep Holla
  0 siblings, 0 replies; 36+ messages in thread
From: Sudeep Holla @ 2021-03-26 12:51 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Sudeep Holla,
	Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon, Michael Kelley

On Fri, Mar 26, 2021 at 12:18:50PM +0000, Mark Rutland wrote:
> On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> > Hi Sudeep,
> >
> > On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > > for Armv8-A specification makes use of x0-x7 as parameter and result
> > > registers.
> > >
> > > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > > x0-x3 as result registers. Let us add new interface to support x0-x7
> > > as parameter and result registers. This can be extended to include
> > > x8-x17 when there are users for the same.
> >
> > Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> > thread I'd suggested we should deal with the whole set of SMCCCv1.2
> > registers now to avoid future churn in this area (using struct both for
> > the arguments and return values).
> >
> > How painful would it be to extend this patch to do that?
>
> I *think* the major change with this would be making the interfaces:
>
> void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
> 			     struct arm_smccc_1_2_res *res);
>
> ... and callers manipulating the structs directly, with arm64 having
> more fields, e.g.
>
> // arm64
> struct arm_smccc_1_2_args {
> 	unsigned long a1;
> 	...
> 	unsigned long a17;
> }
>
> struct arm_smccc_1_2_res {
> 	unsigned long a0;
> 	...
> 	unsigned long a17;
> }
>
> // arm
> struct arm_smccc_1_2_args {
> 	unsigned long a1;
> 	...
> 	unsigned long a7;
> }
>
> struct arm_smccc_1_2_res {
> 	unsigned long a0;
> 	...
> 	unsigned long a7;
> }
>
> I think that can be hidden in the FF-A wrappers, so that doesn't need to
> be what FF-A drivers see. Does that sound plausible, or is that painful?
>

Sounds possible, will give it a try.

> > > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct arm_smccc_v1_2_res, a0));
>
> As a general nit, for consistency with the existing arm_smccc_1_1 code,
> could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
> `arm_smccc_1_2` ?
>

Sure, makes sense.

> FWIW, other than the above comments, this all looks good to me
>

Thanks.

--
Regards,
Sudeep

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  2021-03-26 11:56         ` Sumit Garg
@ 2021-03-30 15:03           ` Rob Herring
  -1 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2021-03-30 15:03 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Sudeep Holla, linux-arm-kernel, Devicetree List, Trilok Soni,
	arve, Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

On Fri, Mar 26, 2021 at 05:26:52PM +0530, Sumit Garg wrote:
> On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > > Hi Sudeep,
> > >
> > > Apologies for catching up late on this patch-set.
> > >
> > > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > > partitions in the discovery API, we need to specify the UUID of the
> > > > partitions that need to be accessed by drivers within the kernel.
> > > >
> > >
> > > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > > think enumeration of ffa partitions on FFA bus should be quite similar
> > > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > > these redundant DT entries for every ffa partition which IMHO would
> > > bloat up device trees for every platform.
> > >
> >
> > Any suggestions on how to ? Clearly spec doesn't have that provision, I
> > had raised this point in the past. Jens has similar concern and he did
> > ask the same[1]. As I replied to him in that thread[2].
> >
> > I am open to suggestion on how to auto-discover, currently as I see spec
> > doesn't support it.
> >
> 
> Thanks for sharing links to prior discussions and I can see that
> currently spec doesn't support it. But from an implementation
> perspective, I can't find any reason that we can't support
> auto-discover. Have a look at below proposed simple FFA ABI:
> 
> FFA_LIST_PARTITIONS
> 
> - No input params.
> - Returns an array of secure partition UUIDs to which this non-secure
> virtual/physical FF-A instance is allowed to communicate with.
> 
> I think with auto-discovery, one of the major benefits is that if the
> OEM is using a common platform to cater to multiple use-cases which
> rely on different secure partitions then OEM doesn't have to bother
> about shipping separate DTs.

+1

DT should not be the dumping ground for everything forgotten to be made 
discoverable. There's not much we can do about h/w, but firmware is 
different and can be changed. In other threads (e.g. PCI config space 
SMC calls), fixing in firmware is the proposed answer. So let's do that 
here.

Maybe if there are implementations shipping and changing is too late 
(yet not too late to use a new binding), then I'd feel differently. But 
being in a spec or not alone is not enough reason alone to accept this. 
It's obvious the spec did not have wide enough review.

Rob

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
@ 2021-03-30 15:03           ` Rob Herring
  0 siblings, 0 replies; 36+ messages in thread
From: Rob Herring @ 2021-03-30 15:03 UTC (permalink / raw)
  To: Sumit Garg
  Cc: Sudeep Holla, linux-arm-kernel, Devicetree List, Trilok Soni,
	arve, Andrew Walbran, David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici

On Fri, Mar 26, 2021 at 05:26:52PM +0530, Sumit Garg wrote:
> On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > > Hi Sudeep,
> > >
> > > Apologies for catching up late on this patch-set.
> > >
> > > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > > partitions in the discovery API, we need to specify the UUID of the
> > > > partitions that need to be accessed by drivers within the kernel.
> > > >
> > >
> > > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > > think enumeration of ffa partitions on FFA bus should be quite similar
> > > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > > these redundant DT entries for every ffa partition which IMHO would
> > > bloat up device trees for every platform.
> > >
> >
> > Any suggestions on how to ? Clearly spec doesn't have that provision, I
> > had raised this point in the past. Jens has similar concern and he did
> > ask the same[1]. As I replied to him in that thread[2].
> >
> > I am open to suggestion on how to auto-discover, currently as I see spec
> > doesn't support it.
> >
> 
> Thanks for sharing links to prior discussions and I can see that
> currently spec doesn't support it. But from an implementation
> perspective, I can't find any reason that we can't support
> auto-discover. Have a look at below proposed simple FFA ABI:
> 
> FFA_LIST_PARTITIONS
> 
> - No input params.
> - Returns an array of secure partition UUIDs to which this non-secure
> virtual/physical FF-A instance is allowed to communicate with.
> 
> I think with auto-discovery, one of the major benefits is that if the
> OEM is using a common platform to cater to multiple use-cases which
> rely on different secure partitions then OEM doesn't have to bother
> about shipping separate DTs.

+1

DT should not be the dumping ground for everything forgotten to be made 
discoverable. There's not much we can do about h/w, but firmware is 
different and can be changed. In other threads (e.g. PCI config space 
SMC calls), fixing in firmware is the proposed answer. So let's do that 
here.

Maybe if there are implementations shipping and changing is too late 
(yet not too late to use a new binding), then I'd feel differently. But 
being in a spec or not alone is not enough reason alone to accept this. 
It's obvious the spec did not have wide enough review.

Rob

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
  2021-03-26 12:51         ` Sudeep Holla
@ 2021-04-01 15:50           ` Michael Kelley
  -1 siblings, 0 replies; 36+ messages in thread
From: Michael Kelley @ 2021-04-01 15:50 UTC (permalink / raw)
  To: Sudeep Holla, Mark Rutland
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Andrew Walbran,
	David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon

From: Sudeep Holla <sudeep.holla@arm.com> Sent: Friday, March 26, 2021 5:51 AM
> 
> On Fri, Mar 26, 2021 at 12:18:50PM +0000, Mark Rutland wrote:
> > On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> > > Hi Sudeep,
> > >
> > > On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > > > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > > > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > > > for Armv8-A specification makes use of x0-x7 as parameter and result
> > > > registers.
> > > >
> > > > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > > > x0-x3 as result registers. Let us add new interface to support x0-x7
> > > > as parameter and result registers. This can be extended to include
> > > > x8-x17 when there are users for the same.
> > >
> > > Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> > > thread I'd suggested we should deal with the whole set of SMCCCv1.2
> > > registers now to avoid future churn in this area (using struct both for
> > > the arguments and return values).
> > >
> > > How painful would it be to extend this patch to do that?
> >
> > I *think* the major change with this would be making the interfaces:
> >
> > void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
> > 			     struct arm_smccc_1_2_res *res);
> >
> > ... and callers manipulating the structs directly, with arm64 having
> > more fields, e.g.
> >
> > // arm64
> > struct arm_smccc_1_2_args {
> > 	unsigned long a1;
> > 	...
> > 	unsigned long a17;
> > }
> >
> > struct arm_smccc_1_2_res {
> > 	unsigned long a0;
> > 	...
> > 	unsigned long a17;
> > }
> >
> > // arm
> > struct arm_smccc_1_2_args {
> > 	unsigned long a1;
> > 	...
> > 	unsigned long a7;
> > }
> >
> > struct arm_smccc_1_2_res {
> > 	unsigned long a0;
> > 	...
> > 	unsigned long a7;
> > }
> >
> > I think that can be hidden in the FF-A wrappers, so that doesn't need to
> > be what FF-A drivers see. Does that sound plausible, or is that painful?
> >
> 
> Sounds possible, will give it a try.
> 
> > > > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct
> arm_smccc_v1_2_res, a0));
> >
> > As a general nit, for consistency with the existing arm_smccc_1_1 code,
> > could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
> > `arm_smccc_1_2` ?
> >
> 
> Sure, makes sense.
> 
> > FWIW, other than the above comments, this all looks good to me
> >
> 
> Thanks.
> 
> --
> Regards,
> Sudeep

Sudeep -- let me know when you have a new version available.  I can
test it for use with Hyper-V.

Michael


^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers
@ 2021-04-01 15:50           ` Michael Kelley
  0 siblings, 0 replies; 36+ messages in thread
From: Michael Kelley @ 2021-04-01 15:50 UTC (permalink / raw)
  To: Sudeep Holla, Mark Rutland
  Cc: linux-arm-kernel, devicetree, Trilok Soni, arve, Andrew Walbran,
	David Hartley, Achin Gupta, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, Catalin Marinas,
	Will Deacon

From: Sudeep Holla <sudeep.holla@arm.com> Sent: Friday, March 26, 2021 5:51 AM
> 
> On Fri, Mar 26, 2021 at 12:18:50PM +0000, Mark Rutland wrote:
> > On Thu, Mar 25, 2021 at 02:41:13PM +0000, Mark Rutland wrote:
> > > Hi Sudeep,
> > >
> > > On Thu, Mar 25, 2021 at 02:32:50PM +0000, Sudeep Holla wrote:
> > > > SMCCC v1.2 allows x8-x17 to be used as parameter registers and x4—x17
> > > > to be used as result registers in SMC64/HVC64. Arm Firmware Framework
> > > > for Armv8-A specification makes use of x0-x7 as parameter and result
> > > > registers.
> > > >
> > > > Current SMCCC interface in the kernel just use x0-x7 as parameter and
> > > > x0-x3 as result registers. Let us add new interface to support x0-x7
> > > > as parameter and result registers. This can be extended to include
> > > > x8-x17 when there are users for the same.
> > >
> > > Michael Kelley is also looking at using SMCCCv1.2, and on his HyperV
> > > thread I'd suggested we should deal with the whole set of SMCCCv1.2
> > > registers now to avoid future churn in this area (using struct both for
> > > the arguments and return values).
> > >
> > > How painful would it be to extend this patch to do that?
> >
> > I *think* the major change with this would be making the interfaces:
> >
> > void arm_smccc_1_2_{hvc,smc}(struct arm_smccc_1_2_args *args,
> > 			     struct arm_smccc_1_2_res *res);
> >
> > ... and callers manipulating the structs directly, with arm64 having
> > more fields, e.g.
> >
> > // arm64
> > struct arm_smccc_1_2_args {
> > 	unsigned long a1;
> > 	...
> > 	unsigned long a17;
> > }
> >
> > struct arm_smccc_1_2_res {
> > 	unsigned long a0;
> > 	...
> > 	unsigned long a17;
> > }
> >
> > // arm
> > struct arm_smccc_1_2_args {
> > 	unsigned long a1;
> > 	...
> > 	unsigned long a7;
> > }
> >
> > struct arm_smccc_1_2_res {
> > 	unsigned long a0;
> > 	...
> > 	unsigned long a7;
> > }
> >
> > I think that can be hidden in the FF-A wrappers, so that doesn't need to
> > be what FF-A drivers see. Does that sound plausible, or is that painful?
> >
> 
> Sounds possible, will give it a try.
> 
> > > > +  DEFINE(ARM_SMCCC_V1_2_RES_X0_OFFS,	offsetof(struct
> arm_smccc_v1_2_res, a0));
> >
> > As a general nit, for consistency with the existing arm_smccc_1_1 code,
> > could we please drop the 'V' in these names, and use `ARM_SMCCC_1_2` or
> > `arm_smccc_1_2` ?
> >
> 
> Sure, makes sense.
> 
> > FWIW, other than the above comments, this all looks good to me
> >
> 
> Thanks.
> 
> --
> Regards,
> Sudeep

Sudeep -- let me know when you have a new version available.  I can
test it for use with Hyper-V.

Michael

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
  2021-03-30 15:03           ` Rob Herring
@ 2021-04-06 15:08             ` Achin Gupta
  -1 siblings, 0 replies; 36+ messages in thread
From: Achin Gupta @ 2021-04-06 15:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sumit Garg, Sudeep Holla, linux-arm-kernel, Devicetree List,
	Trilok Soni, arve, Andrew Walbran, David Hartley, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, nd

Hi Rob,

On Tue, Mar 30, 2021 at 10:03:12AM -0500, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 05:26:52PM +0530, Sumit Garg wrote:
> > On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > > > Hi Sudeep,
> > > >
> > > > Apologies for catching up late on this patch-set.
> > > >
> > > > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > >
> > > > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > > > partitions in the discovery API, we need to specify the UUID of the
> > > > > partitions that need to be accessed by drivers within the kernel.
> > > > >
> > > >
> > > > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > > > think enumeration of ffa partitions on FFA bus should be quite similar
> > > > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > > > these redundant DT entries for every ffa partition which IMHO would
> > > > bloat up device trees for every platform.
> > > >
> > >
> > > Any suggestions on how to ? Clearly spec doesn't have that provision, I
> > > had raised this point in the past. Jens has similar concern and he did
> > > ask the same[1]. As I replied to him in that thread[2].
> > >
> > > I am open to suggestion on how to auto-discover, currently as I see spec
> > > doesn't support it.
> > >
> >
> > Thanks for sharing links to prior discussions and I can see that
> > currently spec doesn't support it. But from an implementation
> > perspective, I can't find any reason that we can't support
> > auto-discover. Have a look at below proposed simple FFA ABI:
> >
> > FFA_LIST_PARTITIONS
> >
> > - No input params.
> > - Returns an array of secure partition UUIDs to which this non-secure
> > virtual/physical FF-A instance is allowed to communicate with.
> >
> > I think with auto-discovery, one of the major benefits is that if the
> > OEM is using a common platform to cater to multiple use-cases which
> > rely on different secure partitions then OEM doesn't have to bother
> > about shipping separate DTs.
>
> +1
>
> DT should not be the dumping ground for everything forgotten to be made
> discoverable. There's not much we can do about h/w, but firmware is
> different and can be changed. In other threads (e.g. PCI config space
> SMC calls), fixing in firmware is the proposed answer. So let's do that
> here.
>
> Maybe if there are implementations shipping and changing is too late
> (yet not too late to use a new binding), then I'd feel differently. But
> being in a spec or not alone is not enough reason alone to accept this.
> It's obvious the spec did not have wide enough review.

The v1.0 specification (released almost one year back) was reviewed by a few
Trusted OS vendors amongst other partners. Nobody provided feedback that a
FFA_LIST_PARTITIONS like discovery mechanism is a gating factor. Hindsight is a
wonderful thing! The ability of discover UUIDs of partitions would indeed be a
very useful thing and this will be added to the next version of the
specification. I think it is worth providing some background in any case.

The usage model envisioned was that a partition would export services that would
be identified by a UUID. An FF-A client that wants to use these services would
query their presence by specifying this UUID. The FF-A discovery ABI returns a
16-bit ID of the partition that implements the services. This ID is more
user-friendly and space efficient than a 16 byte UUID for register and shared
memory based IPC between the client and the server.

For example, OP-TEE would have a UUID to represent the TEE service. The OP-TEE
driver would ask the FF-A driver if there is a partition with a UUID that
provides OP-TEE's TEE service.

The assumption was that a client must identify the service(s) it wants to access
to be able to use FF-A ABIs to talk to the partition that implements them. This
was more important than a discovery mechanism where the client says: I don't
know what I want but please list all the UUIDs of all partitions in the
system. I will then see if I can find anything that I like.

In this model, it was not envisioned that the FF-A driver would carry the UUIDs
of services that its clients could want to access. The client would know the
UUID and the FF-A driver would return the properties of the partition and its ID
to enable further communication.

We did envision the need for the FF-A driver to know the IDs of all the
partitions in the system and their properties e.g. number of vCPUs etc. The
FFA_PARTITION_INFO_GET ABI when invoked with a NULL UUID returns this
information. It is a bit like "grep *.*".

The implementation choice to have an FF-A bus with partitions as devices on it
was made after the specification was released. This was not obvious at all at
the time of writing the specification. Clearly, in this model the FF-A driver
needs to the know the UUIDs. But this does not change the fact that the clients
of the FF-A driver still need to know the UUID of the services they are
interested in.

For v1.0 we are left with no choice but to carry them in the DT. This will be
addressed in v1.1. I see this as a case where implementation choices drive the
evolution of a specification.

I hope you can understand that it was a bit difficult to foresee this usage
model for v1.0 when specification and implementation are temporally separated.

I also think that this issue is different from the cluster **** the PCIe ECAM
SMC discussion is.

I do hope this helps clarify the situation to some extent. Please let me know if
you need any more info or if I have missed anything. Also, apologies for not
getting back sooner as I was on holiday.

cheers,
Achin

^ permalink raw reply	[flat|nested] 36+ messages in thread

* Re: [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
@ 2021-04-06 15:08             ` Achin Gupta
  0 siblings, 0 replies; 36+ messages in thread
From: Achin Gupta @ 2021-04-06 15:08 UTC (permalink / raw)
  To: Rob Herring
  Cc: Sumit Garg, Sudeep Holla, linux-arm-kernel, Devicetree List,
	Trilok Soni, arve, Andrew Walbran, David Hartley, Jens Wiklander,
	Arunachalam Ganapathy, Marc Bonnici, nd

Hi Rob,

On Tue, Mar 30, 2021 at 10:03:12AM -0500, Rob Herring wrote:
> On Fri, Mar 26, 2021 at 05:26:52PM +0530, Sumit Garg wrote:
> > On Fri, 26 Mar 2021 at 16:25, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On Fri, Mar 26, 2021 at 10:35:23AM +0530, Sumit Garg wrote:
> > > > Hi Sudeep,
> > > >
> > > > Apologies for catching up late on this patch-set.
> > > >
> > > > On Thu, 25 Mar 2021 at 20:05, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > > >
> > > > > Since the FF-A v1.0 specification doesn't list the UUID of all the
> > > > > partitions in the discovery API, we need to specify the UUID of the
> > > > > partitions that need to be accessed by drivers within the kernel.
> > > > >
> > > >
> > > > Wouldn't we be able to implement auto-discovery of ffa partitions? I
> > > > think enumeration of ffa partitions on FFA bus should be quite similar
> > > > to enumeration of TAs on TEE bus (see [1]). Otherwise we need to put
> > > > these redundant DT entries for every ffa partition which IMHO would
> > > > bloat up device trees for every platform.
> > > >
> > >
> > > Any suggestions on how to ? Clearly spec doesn't have that provision, I
> > > had raised this point in the past. Jens has similar concern and he did
> > > ask the same[1]. As I replied to him in that thread[2].
> > >
> > > I am open to suggestion on how to auto-discover, currently as I see spec
> > > doesn't support it.
> > >
> >
> > Thanks for sharing links to prior discussions and I can see that
> > currently spec doesn't support it. But from an implementation
> > perspective, I can't find any reason that we can't support
> > auto-discover. Have a look at below proposed simple FFA ABI:
> >
> > FFA_LIST_PARTITIONS
> >
> > - No input params.
> > - Returns an array of secure partition UUIDs to which this non-secure
> > virtual/physical FF-A instance is allowed to communicate with.
> >
> > I think with auto-discovery, one of the major benefits is that if the
> > OEM is using a common platform to cater to multiple use-cases which
> > rely on different secure partitions then OEM doesn't have to bother
> > about shipping separate DTs.
>
> +1
>
> DT should not be the dumping ground for everything forgotten to be made
> discoverable. There's not much we can do about h/w, but firmware is
> different and can be changed. In other threads (e.g. PCI config space
> SMC calls), fixing in firmware is the proposed answer. So let's do that
> here.
>
> Maybe if there are implementations shipping and changing is too late
> (yet not too late to use a new binding), then I'd feel differently. But
> being in a spec or not alone is not enough reason alone to accept this.
> It's obvious the spec did not have wide enough review.

The v1.0 specification (released almost one year back) was reviewed by a few
Trusted OS vendors amongst other partners. Nobody provided feedback that a
FFA_LIST_PARTITIONS like discovery mechanism is a gating factor. Hindsight is a
wonderful thing! The ability of discover UUIDs of partitions would indeed be a
very useful thing and this will be added to the next version of the
specification. I think it is worth providing some background in any case.

The usage model envisioned was that a partition would export services that would
be identified by a UUID. An FF-A client that wants to use these services would
query their presence by specifying this UUID. The FF-A discovery ABI returns a
16-bit ID of the partition that implements the services. This ID is more
user-friendly and space efficient than a 16 byte UUID for register and shared
memory based IPC between the client and the server.

For example, OP-TEE would have a UUID to represent the TEE service. The OP-TEE
driver would ask the FF-A driver if there is a partition with a UUID that
provides OP-TEE's TEE service.

The assumption was that a client must identify the service(s) it wants to access
to be able to use FF-A ABIs to talk to the partition that implements them. This
was more important than a discovery mechanism where the client says: I don't
know what I want but please list all the UUIDs of all partitions in the
system. I will then see if I can find anything that I like.

In this model, it was not envisioned that the FF-A driver would carry the UUIDs
of services that its clients could want to access. The client would know the
UUID and the FF-A driver would return the properties of the partition and its ID
to enable further communication.

We did envision the need for the FF-A driver to know the IDs of all the
partitions in the system and their properties e.g. number of vCPUs etc. The
FFA_PARTITION_INFO_GET ABI when invoked with a NULL UUID returns this
information. It is a bit like "grep *.*".

The implementation choice to have an FF-A bus with partitions as devices on it
was made after the specification was released. This was not obvious at all at
the time of writing the specification. Clearly, in this model the FF-A driver
needs to the know the UUIDs. But this does not change the fact that the clients
of the FF-A driver still need to know the UUID of the services they are
interested in.

For v1.0 we are left with no choice but to carry them in the DT. This will be
addressed in v1.1. I see this as a case where implementation choices drive the
evolution of a specification.

I hope you can understand that it was a bit difficult to foresee this usage
model for v1.0 when specification and implementation are temporally separated.

I also think that this issue is different from the cluster **** the PCIe ECAM
SMC discussion is.

I do hope this helps clarify the situation to some extent. Please let me know if
you need any more info or if I have missed anything. Also, apologies for not
getting back sooner as I was on holiday.

cheers,
Achin

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions
  2021-03-25 14:32   ` Sudeep Holla
@ 2021-04-09 15:45     ` Marc Bonnici
  -1 siblings, 0 replies; 36+ messages in thread
From: Marc Bonnici @ 2021-04-09 15:45 UTC (permalink / raw)
  To: Sudeep Holla, linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy

Hi Sudeep, 

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: 25 March 2021 14:33
> To: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org
> Cc: Sudeep Holla <Sudeep.Holla@arm.com>; Trilok Soni
> <tsoni@codeaurora.org>; arve@android.com; Andrew Walbran
> <qwandor@google.com>; David Hartley <dhh@qti.qualcomm.com>; Achin
> Gupta <Achin.Gupta@arm.com>; Jens Wiklander
> <jens.wiklander@linaro.org>; Arunachalam Ganapathy
> <Arunachalam.Ganapathy@arm.com>; Marc Bonnici <Marc.Bonnici@arm.com>
> Subject: [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of
> FFA partitions
> 
> Parse the FFA nodes from the device-tree and register all the
> partitions whose services will be used in the kernel.
> 
> In order to also enable in-kernel users of FFA interface, let us add
> simple set of operations for such devices.
> 
> The in-kernel users are registered without the character device
> interface.
> 
> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
> Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_ffa/common.h |   2 +
>  drivers/firmware/arm_ffa/driver.c | 188
> ++++++++++++++++++++++++++++++
>  include/linux/arm_ffa.h           |  38 +++++-
>  3 files changed, 227 insertions(+), 1 deletion(-)
> 
[snip]
> 
> +static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool
> mode_32bit,
> +				   struct ffa_send_direct_data *data) {
> +	u32 req_id, resp_id, src_dst_ids = PACK_TARGET_INFO(src_id,
> dst_id);
> +	ffa_res_t ret;
> +
> +	if (mode_32bit) {
> +		req_id = FFA_MSG_SEND_DIRECT_REQ;
> +		resp_id = FFA_MSG_SEND_DIRECT_RESP;
> +	} else {
> +		req_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_REQ);
> +		resp_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_RESP);
> +	}
> +
> +	ret = invoke_ffa_fn(req_id, src_dst_ids, 0, data->data0, data-
> >data1,
> +			    data->data2, data->data3, data->data4);
> +
> +	while (ret.a0 == FFA_INTERRUPT)
> +		ret = invoke_ffa_fn(FFA_RUN, ret.a1, 0, 0, 0, 0, 0, 0);
> +	if (ret.a0 == FFA_ERROR)
> +		return ffa_to_linux_errno((int)ret.a2);
> +
> +	if (ret.a0 == resp_id) {
> +		data->data0 = ret.a3;
> +		data->data1 = ret.a4;
> +		data->data2 = ret.a5;
> +		data->data3 = ret.a6;
> +		data->data4 = ret.a7;
> +	}
> +
> +	return 0;
> +}	
> +

If the returned function ID is not FFA_ERROR but still not what we're 
expecting here we should return an error to the calling code rather
than success.

Kind Regards
Marc Bonnici

^ permalink raw reply	[flat|nested] 36+ messages in thread

* RE: [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions
@ 2021-04-09 15:45     ` Marc Bonnici
  0 siblings, 0 replies; 36+ messages in thread
From: Marc Bonnici @ 2021-04-09 15:45 UTC (permalink / raw)
  To: Sudeep Holla, linux-arm-kernel, devicetree
  Cc: Sudeep Holla, Trilok Soni, arve, Andrew Walbran, David Hartley,
	Achin Gupta, Jens Wiklander, Arunachalam Ganapathy

Hi Sudeep, 

> -----Original Message-----
> From: Sudeep Holla <sudeep.holla@arm.com>
> Sent: 25 March 2021 14:33
> To: linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org
> Cc: Sudeep Holla <Sudeep.Holla@arm.com>; Trilok Soni
> <tsoni@codeaurora.org>; arve@android.com; Andrew Walbran
> <qwandor@google.com>; David Hartley <dhh@qti.qualcomm.com>; Achin
> Gupta <Achin.Gupta@arm.com>; Jens Wiklander
> <jens.wiklander@linaro.org>; Arunachalam Ganapathy
> <Arunachalam.Ganapathy@arm.com>; Marc Bonnici <Marc.Bonnici@arm.com>
> Subject: [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of
> FFA partitions
> 
> Parse the FFA nodes from the device-tree and register all the
> partitions whose services will be used in the kernel.
> 
> In order to also enable in-kernel users of FFA interface, let us add
> simple set of operations for such devices.
> 
> The in-kernel users are registered without the character device
> interface.
> 
> Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>
> Tested-by: Jens Wiklander <jens.wiklander@linaro.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  drivers/firmware/arm_ffa/common.h |   2 +
>  drivers/firmware/arm_ffa/driver.c | 188
> ++++++++++++++++++++++++++++++
>  include/linux/arm_ffa.h           |  38 +++++-
>  3 files changed, 227 insertions(+), 1 deletion(-)
> 
[snip]
> 
> +static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id, bool
> mode_32bit,
> +				   struct ffa_send_direct_data *data) {
> +	u32 req_id, resp_id, src_dst_ids = PACK_TARGET_INFO(src_id,
> dst_id);
> +	ffa_res_t ret;
> +
> +	if (mode_32bit) {
> +		req_id = FFA_MSG_SEND_DIRECT_REQ;
> +		resp_id = FFA_MSG_SEND_DIRECT_RESP;
> +	} else {
> +		req_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_REQ);
> +		resp_id = FFA_FN_NATIVE(MSG_SEND_DIRECT_RESP);
> +	}
> +
> +	ret = invoke_ffa_fn(req_id, src_dst_ids, 0, data->data0, data-
> >data1,
> +			    data->data2, data->data3, data->data4);
> +
> +	while (ret.a0 == FFA_INTERRUPT)
> +		ret = invoke_ffa_fn(FFA_RUN, ret.a1, 0, 0, 0, 0, 0, 0);
> +	if (ret.a0 == FFA_ERROR)
> +		return ffa_to_linux_errno((int)ret.a2);
> +
> +	if (ret.a0 == resp_id) {
> +		data->data0 = ret.a3;
> +		data->data1 = ret.a4;
> +		data->data2 = ret.a5;
> +		data->data3 = ret.a6;
> +		data->data4 = ret.a7;
> +	}
> +
> +	return 0;
> +}	
> +

If the returned function ID is not FFA_ERROR but still not what we're 
expecting here we should return an error to the calling code rather
than success.

Kind Regards
Marc Bonnici

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

^ permalink raw reply	[flat|nested] 36+ messages in thread

end of thread, other threads:[~2021-04-09 15:47 UTC | newest]

Thread overview: 36+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 14:32 [PATCH v5 0/7] firmware: Add initial support for Arm FF-A Sudeep Holla
2021-03-25 14:32 ` Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 1/7] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla
2021-03-26  5:05   ` Sumit Garg
2021-03-26  5:05     ` Sumit Garg
2021-03-26 10:55     ` Sudeep Holla
2021-03-26 10:55       ` Sudeep Holla
2021-03-26 11:56       ` Sumit Garg
2021-03-26 11:56         ` Sumit Garg
2021-03-30 15:03         ` Rob Herring
2021-03-30 15:03           ` Rob Herring
2021-04-06 15:08           ` Achin Gupta
2021-04-06 15:08             ` Achin Gupta
2021-03-25 14:32 ` [PATCH v5 2/7] arm64: smccc: Add support for SMCCCv1.2 input/output registers Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla
2021-03-25 14:41   ` Mark Rutland
2021-03-25 14:41     ` Mark Rutland
2021-03-26 12:18     ` Mark Rutland
2021-03-26 12:18       ` Mark Rutland
2021-03-26 12:51       ` Sudeep Holla
2021-03-26 12:51         ` Sudeep Holla
2021-04-01 15:50         ` Michael Kelley
2021-04-01 15:50           ` Michael Kelley
2021-03-25 14:32 ` [PATCH v5 3/7] firmware: arm_ffa: Add initial FFA bus support for device enumeration Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 4/7] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 5/7] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla
2021-03-25 14:32 ` [PATCH v5 6/7] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla
2021-04-09 15:45   ` Marc Bonnici
2021-04-09 15:45     ` Marc Bonnici
2021-03-25 14:32 ` [PATCH v5 7/7] firmware: arm_ffa: Add support for MEM_* interfaces Sudeep Holla
2021-03-25 14:32   ` Sudeep Holla

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.