* [PATCH 0/9] firmware: Add initial support for Arm FF-A
@ 2020-08-29 17:09 Sudeep Holla
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
` (8 more replies)
0 siblings, 9 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
Hi all,
Let me start stating this is just initial implementation to check on
the idea of providing more in-kernel and userspace support. Lot of things
are still work in progress, I am posting just to get the early feedback
before building lot of things on this idea. Consider this more as RFC
though not tagged explicity(just to avoid it being ignored :))
Arm 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 partition(secure or normal world VM).
If it is a partition managed by hypervisor, then we will register chardev
associated with each of those partition FFA device.
/dev/arm_ffa:
self
e3a48fa5-dc54-4a8b-898b-bdc4dfeeb7b8
49f65057-d002-4ae2-b4ee-d31c7940a13d
For in-kernel usage(mostly communication with secure partitions), only
in-kernel APIs are accessible(no userspace). There may be a need to
provide userspace access instead of in-kernel, it is not yet support
in this series as we need way to identify those and I am not sure if
that belong to DT.
--
Regards,
Sudeep
[1] https://developer.arm.com/documentation/den0077/latest
Sudeep Holla (8):
dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of
partitions
arm64: smccc: Add support for SMCCCv1.2 input/output registers
firmware: smccc: export both smccc functions
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 and register all the KVM managed partitions
firmware: arm_ffa: Setup in-kernel users of FFA partitions
Will Deacon (1):
dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
.../devicetree/bindings/arm/arm,ffa.yaml | 132 +++
.../reserved-memory/arm,ffa-memory.yaml | 71 ++
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 | 5 +
drivers/firmware/arm_ffa/bus.c | 195 +++++
drivers/firmware/arm_ffa/common.h | 27 +
drivers/firmware/arm_ffa/driver.c | 755 ++++++++++++++++++
drivers/firmware/arm_ffa/smccc.c | 54 ++
drivers/firmware/smccc/smccc.c | 2 +
include/linux/arm-smccc.h | 50 ++
include/linux/arm_ffa.h | 96 +++
include/uapi/linux/arm_ffa.h | 56 ++
16 files changed, 1492 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
create mode 100644 Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.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
create mode 100644 include/uapi/linux/arm_ffa.h
--
2.17.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-09-01 16:03 ` Jonathan Cameron
2020-09-02 22:14 ` Rob Herring
2020-08-29 17:09 ` [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions Sudeep Holla
` (7 subsequent siblings)
8 siblings, 2 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
From: Will Deacon <will@kernel.org>
Add devicetree bindings for a FF-A-compliant hypervisor, its partitions
and their memory regions. The naming is ludicrous but also not by fault.
Signed-off-by: Will Deacon <will@kernel.org>
(sudeep.holla: Dropped PSA from name and elsewhere as it seem to have
disappeared mysteriously just before the final release)
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
.../devicetree/bindings/arm/arm,ffa.yaml | 102 ++++++++++++++++++
.../reserved-memory/arm,ffa-memory.yaml | 71 ++++++++++++
2 files changed, 173 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
create mode 100644 Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.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..668a5995fcab
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
@@ -0,0 +1,102 @@
+# SPDX-License-Identifier: GPL-2.0
+%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
+
+maintainers:
+ - Will Deacon <will@kernel.org>
+
+description: |
+ Firmware frameworks implementing partition setup according to the FF-A
+ specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
+ Framework for Arm v8-A") [0] must provide a "manifest and image" for each
+ partition to the "partition manager" so that the partition execution contexts
+ can be initialised.
+
+ In the case of a virtual FFA instance, the manifest and image details can be
+ passed to the hypervisor (e.g. Linux KVM) using this binding.
+
+ [0] https://developer.arm.com/docs/den0077/latest
+
+properties:
+ $nodename:
+ const: ffa_hyp
+
+ compatible:
+ oneOf:
+ - const: arm,ffa-1.0-hypervisor
+
+ memory-region:
+ $ref: '/schemas/types.yaml#/definitions/phandle'
+ description: |
+ A phandle to the reserved memory region [1] to be used by the hypervisor.
+ The reserved memory region must be compatible with
+ "arm,ffa-1.0-hypervisor-memory-region".
+
+ [1] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
+patternProperties:
+ "^ffa_partition[0-9]+$":
+ type: object
+ description: One or more child nodes, each describing an FFA partition.
+ properties:
+ $nodename:
+ const: ffa_partition
+
+ compatible:
+ oneOf:
+ - 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
+
+ nr-exec-ctxs:
+ $ref: '/schemas/types.yaml#/definitions/uint32'
+ description: |
+ The number of virtual CPUs to instantiate for this partition.
+
+ exec-state:
+ description: The execution state in which to execute the partition.
+ oneOf:
+ - const: "AArch64"
+ - const: "AArch32"
+
+ entry-point:
+ $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
+ description: |
+ The entry address of the partition specified as an Intermediate
+ Physical Address (IPA) encoded according to the '#address-cells'
+ property.
+
+ memory-region:
+ $ref: '/schemas/types.yaml#/definitions/phandle-array'
+ description: |
+ A list of phandles to FFA reserved memory regions [3] for this
+ partition.
+
+ [3] Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
+
+additionalProperties: false
+
+examples:
+ - |
+ ffa_hyp {
+ compatible = "arm,ffa-1.0-hypervisor";
+ memory-region = <&ffa_hyp_reserved>;
+
+ ffa_partition0 {
+ compatible = "arm,ffa-1.0-partition";
+ uuid = "12345678-9abc-def0-1234-56789abcdef0";
+ nr-exec-ctxs = <2>;
+ exec-state = "AArch64";
+ entry-point = <0x80000>;
+ memory-region = <&ffa_reserved0 &ffa_reserved1>;
+ };
+ };
diff --git a/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
new file mode 100644
index 000000000000..5335e07abcfc
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/arm,ffa-memory.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Memory Region for Arm Firmware Framework for Arm v8-A
+
+maintainers:
+ - Will Deacon <will@kernel.org>
+
+description: |
+ This binding allows a FF-A implementation to describe the normal memory
+ regions of a partition [1] to a hypervisor according to [2].
+
+ The physical address range reserved for the partition can be specified as a
+ static allocation using the 'reg' property or as a dynamic allocation using
+ the 'size' property. If both properties are omitted, then the hypervisor can
+ allocate physical memory for the partition however it sees fit.
+
+ [1] Documentation/devicetree/bindings/arm/arm,ffa.yaml
+ [2] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+
+properties:
+ $nodename:
+ pattern: "^ffa_mem(@[0-9a-f]+)?$"
+
+ compatible:
+ oneOf:
+ - const: arm,ffa-1.0-partition-memory-region
+
+ ipa-range:
+ $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
+ description: |
+ The Intermediate Physical Address (IPA) range (encoded in the same way as
+ a 'reg' property) at which to map the physical memory. If the IPA range is
+ larger than the physical memory region then the region is mapped starting
+ at the base of the IPA range.
+
+ read-only:
+ type: boolean
+ description: |
+ (static allocation only) The memory region has been pre-populated
+ by the firmware framework and must be mapped without write permission
+ at stage 2.
+
+ non-executable:
+ type: boolean
+ description: |
+ The memory region must be mapped without execute permission at stage 2.
+
+
+required:
+ - compatible
+
+# The "reserved-memory" binding defines additional properties.
+additionalProperties: true
+
+examples:
+ - |
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ ffa_reserved0: ffa_mem@100000000 {
+ compatible = "arm,ffa-1.0-partition-memory-region";
+ reg = <0x1 0x0 0x0 0x04000000>; // 64M @ 1GB
+ ipa-range = <0x0 0x0 0x0 0x04000000>; // 64M @ 0x0
+ read-only;
+ };
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-09-02 21:36 ` Rob Herring
2020-08-29 17:09 ` [PATCH 3/9] arm64: smccc: Add support for SMCCCv1.2 input/output registers Sudeep Holla
` (6 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
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 extends the binding to provide the list of partitions that kernel
drivers may need to access and are not part of the partitions managed
by the hypervisor.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
.../devicetree/bindings/arm/arm,ffa.yaml | 34 +++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
Hi,
I am sure this is incomplete, but I couldn't figure out how to make all
the child properties optional if it is not managed by hypervisor.
Moreover, if we don't like the idea of adding UUID of all the partitions
that in-kernel drivers may need to communicate to, one alternative I can
think of is to allow the creation of FFA device from the FFA driver
itself.
Regards,
Sudeep
diff --git a/Documentation/devicetree/bindings/arm/arm,ffa.yaml b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
index 668a5995fcab..d5c6d71c99de 100644
--- a/Documentation/devicetree/bindings/arm/arm,ffa.yaml
+++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
@@ -23,11 +23,12 @@ description: |
properties:
$nodename:
- const: ffa_hyp
+ pattern: "^(ffa|ffa_hyp)$"
compatible:
oneOf:
- const: arm,ffa-1.0-hypervisor
+ - const: arm,ffa-1.0
memory-region:
$ref: '/schemas/types.yaml#/definitions/phandle'
@@ -83,10 +84,26 @@ description: |
[3] Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
+required:
+ - compatible
+
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: arm,ffa-1.0-hypervisor
+ then:
+ required:
+ - memory-region
+
additionalProperties: false
examples:
- - |
+ - |+
+
+ // Case 1: Partitions managed by hypervisor
+
ffa_hyp {
compatible = "arm,ffa-1.0-hypervisor";
memory-region = <&ffa_hyp_reserved>;
@@ -100,3 +117,16 @@ additionalProperties: false
memory-region = <&ffa_reserved0 &ffa_reserved1>;
};
};
+
+ - |+
+
+ // Case 2: Partitions needing in-kernel usage
+
+ ffa {
+ compatible = "arm,ffa-1.0";
+
+ ffa_partition1 {
+ compatible = "arm,ffa-1.0-partition";
+ uuid = "589fc454-4502-4e66-9347-97b61e27cf73";
+ };
+ };
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/9] arm64: smccc: Add support for SMCCCv1.2 input/output registers
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
2020-08-29 17:09 ` [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 4/9] firmware: smccc: export both smccc functions Sudeep Holla
` (5 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
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.
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(+)
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 7d32fc959b1a..32bcc25337ce 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -122,6 +122,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 1f93809528a4..70e3749b44cf 100644
--- a/arch/arm64/kernel/smccc-call.S
+++ b/arch/arm64/kernel/smccc-call.S
@@ -45,3 +45,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 15c706fb0a37..b64552076b7e 100644
--- a/include/linux/arm-smccc.h
+++ b/include/linux/arm-smccc.h
@@ -152,6 +152,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-a3 result values from registers 0 to 3
+ */
+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 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_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.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/9] firmware: smccc: export both smccc functions
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
` (2 preceding siblings ...)
2020-08-29 17:09 ` [PATCH 3/9] arm64: smccc: Add support for SMCCCv1.2 input/output registers Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 5/9] firmware: arm_ffa: Add initial FFA bus support for device enumeration Sudeep Holla
` (4 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
We need to export both arm_smccc_1_1_get_conduit and arm_smccc_get_version
to allow Arm FFA modules make use of them.
Let us export them in preparation to add support for Arm FFA modules.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/smccc/smccc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
index 4e80921ee212..00c88b809c0c 100644
--- a/drivers/firmware/smccc/smccc.c
+++ b/drivers/firmware/smccc/smccc.c
@@ -24,8 +24,10 @@ enum arm_smccc_conduit arm_smccc_1_1_get_conduit(void)
return smccc_conduit;
}
+EXPORT_SYMBOL_GPL(arm_smccc_1_1_get_conduit);
u32 arm_smccc_get_version(void)
{
return smccc_version;
}
+EXPORT_SYMBOL_GPL(arm_smccc_get_version);
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/9] firmware: arm_ffa: Add initial FFA bus support for device enumeration
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
` (3 preceding siblings ...)
2020-08-29 17:09 ` [PATCH 4/9] firmware: smccc: export both smccc functions Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
` (3 subsequent siblings)
8 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
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 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.
However, to support usage of FFA transport from user-space, there is also
a provision to create character device interface for the same.
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 | 3 +
drivers/firmware/arm_ffa/bus.c | 193 ++++++++++++++++++++++++++++++
include/linux/arm_ffa.h | 81 +++++++++++++
6 files changed, 295 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 fbd785dd0513..8660014e9ec7 100644
--- a/drivers/firmware/Kconfig
+++ b/drivers/firmware/Kconfig
@@ -296,6 +296,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 99510be9f5ed..cd990d411f89 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-$(CONFIG_ARM_SCMI_PROTOCOL) += 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..fadb325ee888
--- /dev/null
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0-only
+obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-bus.o
+ffa-bus-y = bus.o
diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
new file mode 100644
index 000000000000..b5789bd4dfcd
--- /dev/null
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -0,0 +1,193 @@
+// 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>
+
+#define DEVICE_NAME "arm_ffa"
+#define FFA_MAX_CDEVS 32
+
+static DEFINE_IDA(ffa_dev_id);
+static dev_t ffa_devt;
+
+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_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,
+ .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);
+}
+
+static char *
+ffa_devnode(struct device *dev, umode_t *mode, kuid_t *uid, kgid_t *gid)
+{
+ return kasprintf(GFP_KERNEL, DEVICE_NAME "/%s", dev_name(dev));
+}
+
+static struct device_type ffa_dev_type = {
+ .devnode = ffa_devnode,
+};
+
+int ffa_device_register(struct ffa_device *ffa_dev)
+{
+ int ret;
+ struct cdev *cdev;
+ struct device *dev;
+
+ if (!ffa_dev)
+ return -EINVAL;
+
+ dev = &ffa_dev->dev;
+ cdev = &ffa_dev->cdev;
+
+ dev->bus = &ffa_bus_type;
+ dev->type = &ffa_dev_type;
+ dev->release = ffa_release_device;
+
+ device_initialize(dev);
+
+ if (cdev->ops) {
+ ret = ida_simple_get(&ffa_dev_id, 0, FFA_MAX_CDEVS, GFP_KERNEL);
+ if (ret < 0) {
+ put_device(dev);
+ return ret;
+ }
+
+ dev->devt = MKDEV(MAJOR(ffa_devt), ret);
+
+ cdev->owner = cdev->ops->owner;
+ }
+
+ ret = cdev_device_add(cdev, dev);
+ if (ret) {
+ dev_err(dev, "unable to cdev_device_add() %s, major %d, minor %d, err=%d\n",
+ dev_name(dev), MAJOR(dev->devt), MINOR(dev->devt),
+ ret);
+ put_device(dev);
+ return ret;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ffa_device_register);
+
+void ffa_device_unregister(struct ffa_device *ffa_dev)
+{
+ if (!ffa_dev)
+ return;
+
+ cdev_device_del(&ffa_dev->cdev, &ffa_dev->dev);
+
+ put_device(&ffa_dev->dev);
+}
+EXPORT_SYMBOL_GPL(ffa_device_unregister);
+
+static int __init arm_ffa_bus_init(void)
+{
+ int ret;
+
+ ret = alloc_chrdev_region(&ffa_devt, 0, FFA_MAX_CDEVS, DEVICE_NAME);
+ if (ret) {
+ pr_err("failed to allocate char dev region\n");
+ return ret;
+ }
+
+ ret = bus_register(&ffa_bus_type);
+ if (ret) {
+ pr_err("ffa bus register failed (%d)\n", ret);
+ unregister_chrdev_region(ffa_devt, FFA_MAX_CDEVS);
+ }
+
+ return ret;
+}
+subsys_initcall(arm_ffa_bus_init);
+
+static void __exit arm_ffa_bus_exit(void)
+{
+ ffa_devices_unregister();
+ bus_unregister(&ffa_bus_type);
+ unregister_chrdev_region(ffa_devt, FFA_MAX_CDEVS);
+}
+
+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..2fe16176149f
--- /dev/null
+++ b/include/linux/arm_ffa.h
@@ -0,0 +1,81 @@
+/* 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>
+
+struct ffa_device {
+ u32 vm_id;
+ uuid_t uuid;
+ struct device dev;
+ struct cdev cdev;
+};
+
+#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)
+
+#if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
+int ffa_device_register(struct ffa_device *ffa_dev);
+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);
+
+#else
+static inline int ffa_device_register(struct ffa_device *ffa_dev)
+{
+ return -EINVAL;
+}
+
+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) {}
+
+#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.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
` (4 preceding siblings ...)
2020-08-29 17:09 ` [PATCH 5/9] firmware: arm_ffa: Add initial FFA bus support for device enumeration Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-09-07 7:55 ` Fuad Tabba
2020-08-29 17:09 ` [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
` (2 subsequent siblings)
8 siblings, 1 reply; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
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.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_ffa/Makefile | 3 +-
drivers/firmware/arm_ffa/common.h | 23 +++
drivers/firmware/arm_ffa/driver.c | 288 ++++++++++++++++++++++++++++++
3 files changed, 313 insertions(+), 1 deletion(-)
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 fadb325ee888..1a9bd2bf8752 100644
--- a/drivers/firmware/arm_ffa/Makefile
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
-obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-bus.o
+obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-bus.o ffa-driver.o
ffa-bus-y = bus.o
+ffa-driver-y = driver.o
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
new file mode 100644
index 000000000000..720c8425dfd6
--- /dev/null
+++ b/drivers/firmware/arm_ffa/common.h
@@ -0,0 +1,23 @@
+/* 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);
+
+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..3670ba400f89
--- /dev/null
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -0,0 +1,288 @@
+// 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/bitfield.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/arm_ffa.h>
+
+#include "common.h"
+
+#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_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_32(0x71)
+#define FFA_MEM_LEND FFA_SMC_32(0x72)
+#define FFA_FN64_MEM_LEND FFA_SMC_32(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 FFA_DRIVER_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 64K for now */
+#define RXTX_BUFFER_SIZE SZ_64K
+
+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 >= FFA_RET_ABORTED)
+ 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_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, vm_id, 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 = 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);
+}
+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.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
` (5 preceding siblings ...)
2020-08-29 17:09 ` [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-09-01 16:23 ` Jonathan Cameron
2020-08-29 17:09 ` [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions Sudeep Holla
2020-08-29 17:09 ` [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
8 siblings, 1 reply; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
There are requests to keep the transport separate in order to allow
other possible transports like virtio. So let us keep the SMCCC transport
specifi routines abstracted.
It is kept simple for now. Once we add another transport, we can develop
better abstraction.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_ffa/Kconfig | 5 +++
drivers/firmware/arm_ffa/Makefile | 1 +
drivers/firmware/arm_ffa/common.h | 4 +++
drivers/firmware/arm_ffa/smccc.c | 54 +++++++++++++++++++++++++++++++
4 files changed, 64 insertions(+)
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 1a9bd2bf8752..1aaac512384c 100644
--- a/drivers/firmware/arm_ffa/Makefile
+++ b/drivers/firmware/arm_ffa/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-bus.o ffa-driver.o
ffa-bus-y = bus.o
ffa-driver-y = driver.o
+ffa-driver-$(CONFIG_ARM_FFA_SMCCC) += smccc.o
diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
index 720c8425dfd6..10ac3f363f52 100644
--- a/drivers/firmware/arm_ffa/common.h
+++ b/drivers/firmware/arm_ffa/common.h
@@ -15,9 +15,13 @@ typedef ffa_res_t
(ffa_fn)(unsigned long, unsigned long, unsigned long, unsigned long,
unsigned long, unsigned long, unsigned long, unsigned long);
+#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.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
` (6 preceding siblings ...)
2020-08-29 17:09 ` [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-09-01 16:34 ` Jonathan Cameron
2020-09-07 11:20 ` Achin Gupta
2020-08-29 17:09 ` [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
8 siblings, 2 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
Parse the FFA nodes from the device-tree and register all the partitions
managed by the KVM hypervisor.
All the partitions including the host(self) are registered as the
character device with a set of file operations. Most of the interface
will concentrated in the ioctl.
For now, we have a tiny set of initial ioctls implemented.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_ffa/bus.c | 2 +
drivers/firmware/arm_ffa/driver.c | 378 +++++++++++++++++++++++++++++-
include/linux/arm_ffa.h | 3 +
include/uapi/linux/arm_ffa.h | 56 +++++
4 files changed, 438 insertions(+), 1 deletion(-)
create mode 100644 include/uapi/linux/arm_ffa.h
diff --git a/drivers/firmware/arm_ffa/bus.c b/drivers/firmware/arm_ffa/bus.c
index b5789bd4dfcd..4af354e16750 100644
--- a/drivers/firmware/arm_ffa/bus.c
+++ b/drivers/firmware/arm_ffa/bus.c
@@ -78,6 +78,7 @@ static void ffa_release_device(struct device *dev)
{
struct ffa_device *ffa_dev = to_ffa_dev(dev);
+ mutex_destroy(&ffa_dev->mutex);
kfree(ffa_dev);
}
@@ -115,6 +116,7 @@ int ffa_device_register(struct ffa_device *ffa_dev)
dev = &ffa_dev->dev;
cdev = &ffa_dev->cdev;
+ mutex_init(&ffa_dev->mutex);
dev->bus = &ffa_bus_type;
dev->type = &ffa_dev_type;
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 3670ba400f89..96113e594db6 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -22,11 +22,17 @@
#define DRIVER_NAME "ARM FF-A"
#define pr_fmt(fmt) DRIVER_NAME ": " fmt
+#include <linux/arm_ffa.h>
#include <linux/bitfield.h>
+#include <linux/device.h>
+#include <linux/fs.h>
#include <linux/io.h>
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/slab.h>
-#include <linux/arm_ffa.h>
+#include <linux/uaccess.h>
+#include <linux/uuid.h>
+#include <uapi/linux/arm_ffa.h>
#include "common.h"
@@ -122,6 +128,13 @@
/* Keeping RX TX buffer size as 64K for now */
#define RXTX_BUFFER_SIZE SZ_64K
+#define list_to_ffa_dev(n) container_of(n, struct ffa_device, node)
+
+/* List of all FFA devices active in system */
+static LIST_HEAD(ffa_devs_list);
+/* Protection for the entire list */
+static DEFINE_MUTEX(ffa_devs_list_mutex);
+
static ffa_fn *invoke_ffa_fn;
static const int ffa_linux_errmap[] = {
@@ -178,6 +191,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;
@@ -202,6 +229,50 @@ static int ffa_rxtx_unmap(u16 vm_id)
return 0;
}
+static int ffa_partition_info_get(u32 uuid0, u32 uuid1, u32 uuid2, u32 uuid3,
+ struct ffa_partition_info **buffer)
+{
+ 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)
+ 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);
+
+ return count;
+}
+
#define VM_ID_MASK GENMASK(15, 0)
static int ffa_id_get(u16 *vm_id)
{
@@ -217,6 +288,296 @@ static int ffa_id_get(u16 *vm_id)
return 0;
}
+static int ffa_msg_send(u16 src_id, u16 dst_id, void *buf, u32 len)
+{
+ ffa_res_t ret;
+ u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id);
+
+ mutex_lock(&drv_info->tx_lock);
+
+ memcpy(drv_info->tx_buffer, buf, len);
+
+ ret = invoke_ffa_fn(FFA_MSG_SEND, src_dst_ids, 0, len, 0, 0, 0, 0);
+
+ mutex_unlock(&drv_info->tx_lock);
+
+ if (ret.a0 == FFA_ERROR)
+ return ffa_to_linux_errno((int)ret.a2);
+
+ return 0;
+}
+
+static int ffa_msg_send_direct_req(u16 src_id, u16 dst_id,
+ struct ffa_send_direct_data *data)
+{
+ u32 src_dst_ids = PACK_TARGET_INFO(src_id, dst_id);
+ ffa_res_t ret;
+
+ ret = invoke_ffa_fn(FFA_FN_NATIVE(MSG_SEND_DIRECT_REQ), 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 == FFA_FN_NATIVE(MSG_SEND_DIRECT_RESP)) {
+ data->data0 = ret.a3;
+ data->data1 = ret.a4;
+ data->data2 = ret.a5;
+ data->data3 = ret.a6;
+ data->data4 = ret.a7;
+ }
+
+ return 0;
+}
+
+static struct ffa_device *ffa_device_get_from_minor(int minor)
+{
+ struct list_head *p;
+ struct ffa_device *ffa_dev = NULL;
+
+ mutex_lock(&ffa_devs_list_mutex);
+
+ list_for_each(p, &ffa_devs_list) {
+ struct device *dev;
+ struct ffa_device *tmp_dev;
+
+ tmp_dev = list_to_ffa_dev(p);
+ dev = &tmp_dev->dev;
+
+ if (minor == MINOR(dev->devt)) {
+ ffa_dev = tmp_dev;
+ break;
+ }
+ }
+
+ mutex_unlock(&ffa_devs_list_mutex);
+
+ return ffa_dev;
+}
+
+static void ffa_device_get(struct ffa_device *ffa_dev)
+{
+ mutex_lock(&ffa_dev->mutex);
+ ffa_dev->num_users++;
+ mutex_unlock(&ffa_dev->mutex);
+}
+
+static void ffa_device_put(struct ffa_device *ffa_dev)
+{
+ mutex_lock(&ffa_dev->mutex);
+ ffa_dev->num_users--;
+ mutex_unlock(&ffa_dev->mutex);
+}
+
+static int ffa_open(struct inode *inode, struct file *filp)
+{
+ struct ffa_device *ffa_dev;
+
+ ffa_dev = ffa_device_get_from_minor(iminor(inode));
+ if (!ffa_dev)
+ return -EINVAL;
+
+ ffa_device_get(ffa_dev);
+
+ filp->private_data = ffa_dev;
+
+ return 0;
+}
+
+static int ffa_release(struct inode *inode, struct file *filp)
+{
+ struct ffa_device *ffa_dev = filp->private_data;
+
+ ffa_device_put(ffa_dev);
+
+ filp->private_data = NULL;
+
+ return 0;
+}
+
+static long ffa_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
+{
+ long r = -EINVAL;
+ void __user *argp = (void __user *)arg;
+ struct ffa_device *ffa_dev = filp->private_data;
+
+ switch (ioctl) {
+ case FFA_GET_API_VERSION:
+ if (arg)
+ goto out;
+ r = drv_info->version;
+ break;
+ case FFA_GET_PARTITION_ID:
+ if (arg)
+ goto out;
+ r = ffa_dev->vm_id;
+ break;
+ case FFA_GET_PARTITION_INFO: {
+ struct ffa_partition_info pbuf;
+ struct ffa_part_info __user *pinfo = argp;
+ struct ffa_part_info info;
+ unsigned int count;
+
+ r = -EFAULT;
+ if (copy_from_user(&info, pinfo, sizeof(info)))
+ break;
+ count = ffa_partition_probe(info.uuid_str, &pbuf);
+ if (count > 1) {
+ r = -E2BIG;
+ break;
+ }
+ r = -EFAULT;
+ if (copy_to_user(pinfo, &info, sizeof(info)))
+ break;
+ r = 0;
+ break;
+ }
+ case FFA_SEND_RECEIVE_SYNC: {
+ struct ffa_send_recv_sync __user *udata = argp;
+ struct ffa_send_recv_sync kdata;
+
+ r = -EFAULT;
+ if (copy_from_user(&kdata, udata, sizeof(kdata)))
+ break;
+ r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata.endpoint_id,
+ &kdata.data);
+ if (r)
+ break;
+ if (copy_to_user(udata, &kdata, sizeof(kdata)))
+ break;
+ break;
+ }
+ case FFA_SEND_RECEIVE_ASYNC: {
+ struct ffa_send_recv_async __user *udata = argp;
+ struct ffa_send_recv_async kdata;
+ void *buf;
+
+ r = -EFAULT;
+ if (copy_from_user(&kdata, udata, sizeof(kdata)))
+ break;
+
+ if (kdata.length < 0 || kdata.length > RXTX_BUFFER_SIZE) {
+ r = -EINVAL;
+ break;
+ }
+
+ buf = kzalloc(kdata.length, GFP_KERNEL);
+ if (!buf) {
+ r = -ENOMEM;
+ break;
+ }
+
+ if (copy_from_user(buf, udata->buffer, kdata.length)) {
+ kfree(buf);
+ break;
+ }
+
+ r = ffa_msg_send(ffa_dev->vm_id, kdata.endpoint_id, buf,
+ kdata.length);
+ if (r) {
+ kfree(buf);
+ break;
+ }
+
+ break;
+ }
+ default:
+ r = -EINVAL;
+ }
+out:
+ return r;
+}
+
+static const struct file_operations ffa_fops = {
+ .owner = THIS_MODULE,
+ .open = ffa_open,
+ .release = ffa_release,
+ .unlocked_ioctl = ffa_ioctl,
+ .llseek = noop_llseek,
+};
+
+static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
+{
+ int ret;
+ struct ffa_device *ffa_dev;
+
+ ffa_dev = kzalloc(sizeof(*ffa_dev), GFP_KERNEL);
+ if (!ffa_dev)
+ return -ENOMEM;
+
+ ffa_dev->vm_id = vm_id;
+ if (uuid)
+ memcpy(uuid, &ffa_dev->uuid, sizeof(*uuid));
+
+ dev_set_name(&ffa_dev->dev, name);
+ dev_set_drvdata(&ffa_dev->dev, drv_info);
+ cdev_init(&ffa_dev->cdev, &ffa_fops);
+
+ ret = ffa_device_register(ffa_dev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&ffa_devs_list_mutex);
+ list_add_tail(&ffa_dev->node, &ffa_devs_list);
+ mutex_unlock(&ffa_devs_list_mutex);
+
+ return 0;
+}
+
+static int ffa_setup_partitions(void)
+{
+ int ret;
+ struct ffa_partition_info pbuf;
+ struct device_node *child, *parent;
+ const char *p_uuid, *pfx = "Ignoring FFA partition";
+ const char *compatible = "arm,ffa-1.0-hypervisor";
+ uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
+
+ parent = of_find_compatible_node(NULL, NULL, compatible);
+ if (!parent)
+ return 0;
+
+ for_each_child_of_node(parent, child) {
+ if (!of_device_is_compatible(child, "arm,ffa-1.0-partition")) {
+ of_node_put(child);
+ continue;
+ }
+
+ if (of_property_read_string(child, "uuid", &p_uuid)) {
+ pr_err("%s: failed to parse \"uuid\" property\n", pfx);
+ of_node_put(child);
+ continue;
+ }
+
+ of_node_put(child);
+
+ 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 != 1) {
+ pr_err("%s: %s partition info probe failed\n",
+ pfx, p_uuid);
+ return -EINVAL;
+ }
+
+ ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid);
+ if (ret) {
+ pr_err("%s: failed to register %s\n", pfx, p_uuid);
+ continue;
+ }
+ }
+
+ of_node_put(parent);
+ return 0;
+}
+
static int __init ffa_init(void)
{
int ret;
@@ -262,6 +623,14 @@ static int __init ffa_init(void)
mutex_init(&drv_info->rx_lock);
mutex_init(&drv_info->tx_lock);
+ /* This will be default device both in theguests and host */
+ ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL);
+ if (ret)
+ return ret;
+
+ /* Set up all the partitions that KVM hypervisor maintains */
+ ffa_setup_partitions();
+
return 0;
free_pages:
if (drv_info->tx_buffer)
@@ -275,6 +644,13 @@ module_init(ffa_init);
static void __exit ffa_exit(void)
{
+ struct list_head *p;
+
+ mutex_lock(&ffa_devs_list_mutex);
+ list_for_each(p, &ffa_devs_list)
+ ffa_device_unregister(list_to_ffa_dev(p));
+ mutex_unlock(&ffa_devs_list_mutex);
+
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);
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 2fe16176149f..6824e0633c77 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -17,6 +17,9 @@ struct ffa_device {
uuid_t uuid;
struct device dev;
struct cdev cdev;
+ size_t num_users;
+ struct mutex mutex; /* protects num_users */
+ struct list_head node;
};
#define to_ffa_dev(d) container_of(d, struct ffa_device, dev)
diff --git a/include/uapi/linux/arm_ffa.h b/include/uapi/linux/arm_ffa.h
new file mode 100644
index 000000000000..88ddddb4742f
--- /dev/null
+++ b/include/uapi/linux/arm_ffa.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+/*
+ * Copyright (C) 2020 Arm Ltd.
+ */
+
+#ifndef _UAPI_LINUX_ARM_FFA_H
+#define _UAPI_LINUX_ARM_FFA_H
+
+#include <linux/types.h>
+#include <linux/ioctl.h>
+
+#define FFA_BASE 0xFF
+
+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;
+};
+
+struct ffa_part_info {
+ char uuid_str[36];
+ struct ffa_partition_info info;
+};
+
+struct ffa_send_direct_data {
+ unsigned long data0;
+ unsigned long data1;
+ unsigned long data2;
+ unsigned long data3;
+ unsigned long data4;
+};
+
+struct ffa_send_recv_sync {
+ __u16 endpoint_id;
+ struct ffa_send_direct_data data;
+};
+
+struct ffa_send_recv_async {
+ __u16 endpoint_id;
+ int length;
+ char buffer[];
+};
+
+#define FFA_GET_API_VERSION _IO(FFA_BASE, 0x00)
+#define FFA_GET_PARTITION_ID _IO(FFA_BASE, 0x01)
+#define FFA_GET_PARTITION_INFO _IOWR(FFA_BASE, 0x02, struct ffa_part_info)
+#define FFA_SEND_RECEIVE_SYNC _IOWR(FFA_BASE, 0x03, struct ffa_send_recv_sync)
+#define FFA_SEND_RECEIVE_ASYNC _IOW(FFA_BASE, 0x04, struct ffa_send_recv_async)
+
+#endif /*_UAPI_LINUX_ARM_FFA_H*/
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
` (7 preceding siblings ...)
2020-08-29 17:09 ` [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions Sudeep Holla
@ 2020-08-29 17:09 ` Sudeep Holla
2020-09-01 16:38 ` Jonathan Cameron
2020-09-07 11:32 ` Achin Gupta
8 siblings, 2 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-08-29 17:09 UTC (permalink / raw)
To: linux-arm-kernel, devicetree
Cc: Sudeep Holla, kernel-team, Will Deacon, tsoni, pratikp
In order to also enable in-kernel users of FFA interface along with
the access to user-space applications, let us add simple set of operations
for such devices.
The in-kernel users are registered without the character device interface.
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
drivers/firmware/arm_ffa/driver.c | 119 ++++++++++++++++++++++++++----
include/linux/arm_ffa.h | 12 +++
2 files changed, 117 insertions(+), 14 deletions(-)
diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
index 96113e594db6..811558ef2a1d 100644
--- a/drivers/firmware/arm_ffa/driver.c
+++ b/drivers/firmware/arm_ffa/driver.c
@@ -372,6 +372,97 @@ static void ffa_device_put(struct ffa_device *ffa_dev)
mutex_unlock(&ffa_dev->mutex);
}
+static int ffa_dev_open(struct ffa_device *ffa_dev)
+{
+ ffa_device_get(ffa_dev);
+
+ return 0;
+}
+
+static int ffa_dev_close(struct ffa_device *ffa_dev)
+{
+ ffa_device_put(ffa_dev);
+
+ return 0;
+}
+
+static long
+ffa_dev_ioctl(struct ffa_device *ffa_dev, unsigned int ioctl, void *arg)
+{
+ long r = -EINVAL;
+
+ switch (ioctl) {
+ case FFA_GET_API_VERSION:
+ r = drv_info->version;
+ break;
+ case FFA_GET_PARTITION_ID:
+ r = ffa_dev->vm_id;
+ break;
+ case FFA_GET_PARTITION_INFO: {
+ struct ffa_part_info *pinfo = arg;
+
+ if (ffa_partition_probe(pinfo->uuid_str, &pinfo->info) != 1)
+ r = -E2BIG;
+ break;
+ }
+ case FFA_SEND_RECEIVE_SYNC: {
+ struct ffa_send_recv_sync *kdata = arg;
+
+ r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata->endpoint_id,
+ &kdata->data);
+ break;
+ }
+ case FFA_SEND_RECEIVE_ASYNC: {
+ struct ffa_send_recv_async *kdata = arg;
+
+ if (kdata->length < 0 || kdata->length > RXTX_BUFFER_SIZE) {
+ r = -EINVAL;
+ break;
+ }
+
+ r = ffa_msg_send(ffa_dev->vm_id, kdata->endpoint_id,
+ kdata->buffer, kdata->length);
+ break;
+ }
+ default:
+ r = -EINVAL;
+ }
+
+ return r;
+}
+
+struct ffa_dev_ops ffa_ops = {
+ .open = ffa_dev_open,
+ .close = ffa_dev_close,
+ .ioctl = ffa_dev_ioctl,
+};
+
+struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
+{
+ struct list_head *p;
+ struct ffa_dev_ops *ops = NULL;
+
+ if (uuid_is_null(&dev->uuid))
+ return NULL;
+
+ mutex_lock(&ffa_devs_list_mutex);
+
+ list_for_each(p, &ffa_devs_list) {
+ struct ffa_device *tmp_dev;
+
+ tmp_dev = list_to_ffa_dev(p);
+
+ if (uuid_equal(&tmp_dev->uuid, &dev->uuid)) {
+ ops = &ffa_ops;
+ break;
+ }
+ }
+
+ mutex_unlock(&ffa_devs_list_mutex);
+
+ return ops;
+}
+
static int ffa_open(struct inode *inode, struct file *filp)
{
struct ffa_device *ffa_dev;
@@ -380,7 +471,7 @@ static int ffa_open(struct inode *inode, struct file *filp)
if (!ffa_dev)
return -EINVAL;
- ffa_device_get(ffa_dev);
+ ffa_dev_open(ffa_dev);
filp->private_data = ffa_dev;
@@ -391,7 +482,7 @@ static int ffa_release(struct inode *inode, struct file *filp)
{
struct ffa_device *ffa_dev = filp->private_data;
- ffa_device_put(ffa_dev);
+ ffa_dev_close(ffa_dev);
filp->private_data = NULL;
@@ -406,14 +497,10 @@ static long ffa_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
switch (ioctl) {
case FFA_GET_API_VERSION:
- if (arg)
- goto out;
- r = drv_info->version;
- break;
case FFA_GET_PARTITION_ID:
if (arg)
goto out;
- r = ffa_dev->vm_id;
+ r = ffa_dev_ioctl(ffa_dev, FFA_GET_PARTITION_ID, NULL);
break;
case FFA_GET_PARTITION_INFO: {
struct ffa_partition_info pbuf;
@@ -499,7 +586,8 @@ static const struct file_operations ffa_fops = {
.llseek = noop_llseek,
};
-static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
+static int ffa_device_alloc_register(const char *name, u16 vm_id,
+ uuid_t *uuid, bool cdev_if)
{
int ret;
struct ffa_device *ffa_dev;
@@ -514,7 +602,7 @@ static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
dev_set_name(&ffa_dev->dev, name);
dev_set_drvdata(&ffa_dev->dev, drv_info);
- cdev_init(&ffa_dev->cdev, &ffa_fops);
+ cdev_init(&ffa_dev->cdev, cdev_if ? &ffa_fops : NULL);
ret = ffa_device_register(ffa_dev);
if (ret)
@@ -527,13 +615,13 @@ static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
return 0;
}
-static int ffa_setup_partitions(void)
+static int ffa_setup_partitions(bool hyp)
{
int ret;
struct ffa_partition_info pbuf;
struct device_node *child, *parent;
const char *p_uuid, *pfx = "Ignoring FFA partition";
- const char *compatible = "arm,ffa-1.0-hypervisor";
+ const char *compatible = hyp ? "arm,ffa-1.0-hypervisor" : "arm,ffa-1.0";
uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
parent = of_find_compatible_node(NULL, NULL, compatible);
@@ -567,7 +655,7 @@ static int ffa_setup_partitions(void)
return -EINVAL;
}
- ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid);
+ ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid, hyp);
if (ret) {
pr_err("%s: failed to register %s\n", pfx, p_uuid);
continue;
@@ -624,12 +712,15 @@ static int __init ffa_init(void)
mutex_init(&drv_info->tx_lock);
/* This will be default device both in theguests and host */
- ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL);
+ ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL, true);
if (ret)
return ret;
/* Set up all the partitions that KVM hypervisor maintains */
- ffa_setup_partitions();
+ ffa_setup_partitions(true);
+
+ /* Set up all the partitions which have in-kernel drivers */
+ ffa_setup_partitions(false);
return 0;
free_pages:
diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
index 6824e0633c77..1df7399d207d 100644
--- a/include/linux/arm_ffa.h
+++ b/include/linux/arm_ffa.h
@@ -11,6 +11,7 @@
#include <linux/module.h>
#include <linux/types.h>
#include <linux/uuid.h>
+#include <uapi/linux/arm_ffa.h>
struct ffa_device {
u32 vm_id;
@@ -39,12 +40,19 @@ struct ffa_driver {
#define to_ffa_driver(d) container_of(d, struct ffa_driver, driver)
+struct ffa_dev_ops {
+ int (*open)(struct ffa_device *dev);
+ int (*close)(struct ffa_device *dev);
+ long (*ioctl)(struct ffa_device *dev, unsigned int ioctl, void *arg);
+};
+
#if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
int ffa_device_register(struct ffa_device *ffa_dev);
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);
+struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);
#else
static inline int ffa_device_register(struct ffa_device *ffa_dev)
@@ -63,6 +71,10 @@ ffa_driver_register(struct ffa_driver *driver, struct module *owner,
static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
+struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
+{
+ return NULL;
+}
#endif /* CONFIG_ARM_FFA_TRANSPORT */
#define ffa_register(driver) \
--
2.17.1
^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
@ 2020-09-01 16:03 ` Jonathan Cameron
2020-09-07 11:40 ` Sudeep Holla
2020-09-02 22:14 ` Rob Herring
1 sibling, 1 reply; 24+ messages in thread
From: Jonathan Cameron @ 2020-09-01 16:03 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, Will Deacon, pratikp, tsoni, kernel-team
On Sat, 29 Aug 2020 18:09:15 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:
> From: Will Deacon <will@kernel.org>
>
> Add devicetree bindings for a FF-A-compliant hypervisor, its partitions
> and their memory regions. The naming is ludicrous but also not by fault.
>
> Signed-off-by: Will Deacon <will@kernel.org>
> (sudeep.holla: Dropped PSA from name and elsewhere as it seem to have
> disappeared mysteriously just before the final release)
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
As I was reading this out of curiosity...
I'm far from a yaml expert but a few things in line.
> ---
> .../devicetree/bindings/arm/arm,ffa.yaml | 102 ++++++++++++++++++
> .../reserved-memory/arm,ffa-memory.yaml | 71 ++++++++++++
> 2 files changed, 173 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
> create mode 100644 Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.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..668a5995fcab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%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
> +
> +maintainers:
> + - Will Deacon <will@kernel.org>
> +
> +description: |
> + Firmware frameworks implementing partition setup according to the FF-A
> + specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
> + Framework for Arm v8-A") [0] must provide a "manifest and image" for each
> + partition to the "partition manager" so that the partition execution contexts
> + can be initialised.
> +
> + In the case of a virtual FFA instance, the manifest and image details can be
> + passed to the hypervisor (e.g. Linux KVM) using this binding.
> +
> + [0] https://developer.arm.com/docs/den0077/latest
> +
> +properties:
> + $nodename:
> + const: ffa_hyp
> +
> + compatible:
> + oneOf:
> + - const: arm,ffa-1.0-hypervisor
As below, would enum be more appropriate here?
> +
> + memory-region:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description: |
> + A phandle to the reserved memory region [1] to be used by the hypervisor.
> + The reserved memory region must be compatible with
> + "arm,ffa-1.0-hypervisor-memory-region".
> +
> + [1] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +
> +patternProperties:
> + "^ffa_partition[0-9]+$":
> + type: object
> + description: One or more child nodes, each describing an FFA partition.
> + properties:
> + $nodename:
> + const: ffa_partition
> +
> + compatible:
> + oneOf:
enum?
> + - 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
Feels like we should be able to better type wise than a string for this but
maybe not... Doesn't seem to be much in the way of precedence.
> +
> + nr-exec-ctxs:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + The number of virtual CPUs to instantiate for this partition.
> +
> + exec-state:
> + description: The execution state in which to execute the partition.
> + oneOf:
> + - const: "AArch64"
> + - const: "AArch32"
> +
> + entry-point:
> + $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
> + description: |
> + The entry address of the partition specified as an Intermediate
> + Physical Address (IPA) encoded according to the '#address-cells'
> + property.
> +
> + memory-region:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + A list of phandles to FFA reserved memory regions [3] for this
> + partition.
> +
> + [3] Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + ffa_hyp {
> + compatible = "arm,ffa-1.0-hypervisor";
> + memory-region = <&ffa_hyp_reserved>;
> +
> + ffa_partition0 {
> + compatible = "arm,ffa-1.0-partition";
> + uuid = "12345678-9abc-def0-1234-56789abcdef0";
> + nr-exec-ctxs = <2>;
> + exec-state = "AArch64";
> + entry-point = <0x80000>;
> + memory-region = <&ffa_reserved0 &ffa_reserved1>;
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
> new file mode 100644
> index 000000000000..5335e07abcfc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/arm,ffa-memory.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Memory Region for Arm Firmware Framework for Arm v8-A
> +
> +maintainers:
> + - Will Deacon <will@kernel.org>
> +
> +description: |
> + This binding allows a FF-A implementation to describe the normal memory
> + regions of a partition [1] to a hypervisor according to [2].
> +
> + The physical address range reserved for the partition can be specified as a
> + static allocation using the 'reg' property or as a dynamic allocation using
> + the 'size' property. If both properties are omitted, then the hypervisor can
> + allocate physical memory for the partition however it sees fit.
> +
> + [1] Documentation/devicetree/bindings/arm/arm,ffa.yaml
> + [2] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +
> +properties:
> + $nodename:
> + pattern: "^ffa_mem(@[0-9a-f]+)?$"
> +
> + compatible:
> + oneOf:
> + - const: arm,ffa-1.0-partition-memory-region
If there is just element and it is const why not just?
compatible:
const: ....
I guess it sort of future proofs you for later variants...
However in that case, it would be more common to use
compatible:
enum:
- arm,ffa...
which is more restrictive but doesn't seem like you need the complexity of
oneOf and it's handling of sub schemas here.
> +
> + ipa-range:
> + $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
> + description: |
> + The Intermediate Physical Address (IPA) range (encoded in the same way as
> + a 'reg' property) at which to map the physical memory. If the IPA range is
> + larger than the physical memory region then the region is mapped starting
> + at the base of the IPA range.
> +
> + read-only:
> + type: boolean
> + description: |
> + (static allocation only) The memory region has been pre-populated
> + by the firmware framework and must be mapped without write permission
> + at stage 2.
> +
> + non-executable:
> + type: boolean
> + description: |
> + The memory region must be mapped without execute permission at stage 2.
> +
> +
> +required:
> + - compatible
> +
> +# The "reserved-memory" binding defines additional properties.
> +additionalProperties: true
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + ffa_reserved0: ffa_mem@100000000 {
> + compatible = "arm,ffa-1.0-partition-memory-region";
> + reg = <0x1 0x0 0x0 0x04000000>; // 64M @ 1GB
Isn't that at 4GB?
> + ipa-range = <0x0 0x0 0x0 0x04000000>; // 64M @ 0x0
> + read-only;
> + };
> + };
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver
2020-08-29 17:09 ` [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
@ 2020-09-01 16:23 ` Jonathan Cameron
0 siblings, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2020-09-01 16:23 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, Will Deacon, pratikp, tsoni, kernel-team
On Sat, 29 Aug 2020 18:09:21 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:
> There are requests to keep the transport separate in order to allow
> other possible transports like virtio. So let us keep the SMCCC transport
> specifi routines abstracted.
specific
>
> It is kept simple for now. Once we add another transport, we can develop
> better abstraction.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
...
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions
2020-08-29 17:09 ` [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions Sudeep Holla
@ 2020-09-01 16:34 ` Jonathan Cameron
2020-09-07 11:20 ` Achin Gupta
1 sibling, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2020-09-01 16:34 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, Will Deacon, pratikp, tsoni, kernel-team
On Sat, 29 Aug 2020 18:09:22 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:
> Parse the FFA nodes from the device-tree and register all the partitions
> managed by the KVM hypervisor.
>
> All the partitions including the host(self) are registered as the
> character device with a set of file operations. Most of the interface
> will concentrated in the ioctl.
>
> For now, we have a tiny set of initial ioctls implemented.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
A few trivial comments inline.
> ---
> drivers/firmware/arm_ffa/bus.c | 2 +
> drivers/firmware/arm_ffa/driver.c | 378 +++++++++++++++++++++++++++++-
> include/linux/arm_ffa.h | 3 +
> include/uapi/linux/arm_ffa.h | 56 +++++
> 4 files changed, 438 insertions(+), 1 deletion(-)
> create mode 100644 include/uapi/linux/arm_ffa.h
>
...
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index 3670ba400f89..96113e594db6 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -22,11 +22,17 @@
> #define DRIVER_NAME "ARM FF-A"
> #define pr_fmt(fmt) DRIVER_NAME ": " fmt
>
> +#include <linux/arm_ffa.h>
> #include <linux/bitfield.h>
> +#include <linux/device.h>
> +#include <linux/fs.h>
> #include <linux/io.h>
> #include <linux/module.h>
> +#include <linux/of.h>
> #include <linux/slab.h>
> -#include <linux/arm_ffa.h>
Ah. Fix that in the earlier patch rather than adding then moving it.
> +#include <linux/uaccess.h>
> +#include <linux/uuid.h>
> +#include <uapi/linux/arm_ffa.h>
>
> #include "common.h"
>
...
> +
> +static long ffa_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> +{
> + long r = -EINVAL;
> + void __user *argp = (void __user *)arg;
> + struct ffa_device *ffa_dev = filp->private_data;
> +
> + switch (ioctl) {
> + case FFA_GET_API_VERSION:
> + if (arg)
> + goto out;
Perhaps more readable (and a little shorter) with early returns?
> + r = drv_info->version;
> + break;
> + case FFA_GET_PARTITION_ID:
> + if (arg)
> + goto out;
> + r = ffa_dev->vm_id;
> + break;
> + case FFA_GET_PARTITION_INFO: {
> + struct ffa_partition_info pbuf;
> + struct ffa_part_info __user *pinfo = argp;
> + struct ffa_part_info info;
> + unsigned int count;
> +
> + r = -EFAULT;
> + if (copy_from_user(&info, pinfo, sizeof(info)))
> + break;
> + count = ffa_partition_probe(info.uuid_str, &pbuf);
> + if (count > 1) {
> + r = -E2BIG;
> + break;
> + }
> + r = -EFAULT;
> + if (copy_to_user(pinfo, &info, sizeof(info)))
> + break;
> + r = 0;
> + break;
> + }
> + case FFA_SEND_RECEIVE_SYNC: {
> + struct ffa_send_recv_sync __user *udata = argp;
> + struct ffa_send_recv_sync kdata;
> +
> + r = -EFAULT;
> + if (copy_from_user(&kdata, udata, sizeof(kdata)))
> + break;
> + r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata.endpoint_id,
> + &kdata.data);
> + if (r)
> + break;
> + if (copy_to_user(udata, &kdata, sizeof(kdata)))
> + break;
> + break;
> + }
> + case FFA_SEND_RECEIVE_ASYNC: {
> + struct ffa_send_recv_async __user *udata = argp;
> + struct ffa_send_recv_async kdata;
> + void *buf;
> +
> + r = -EFAULT;
> + if (copy_from_user(&kdata, udata, sizeof(kdata)))
> + break;
> +
> + if (kdata.length < 0 || kdata.length > RXTX_BUFFER_SIZE) {
> + r = -EINVAL;
> + break;
> + }
> +
> + buf = kzalloc(kdata.length, GFP_KERNEL);
> + if (!buf) {
> + r = -ENOMEM;
> + break;
> + }
> +
> + if (copy_from_user(buf, udata->buffer, kdata.length)) {
> + kfree(buf);
> + break;
> + }
> +
> + r = ffa_msg_send(ffa_dev->vm_id, kdata.endpoint_id, buf,
> + kdata.length);
> + if (r) {
> + kfree(buf);
> + break;
> + }
> +
> + break;
> + }
> + default:
> + r = -EINVAL;
> + }
> +out:
> + return r;
> +}
> +
...
> static int __init ffa_init(void)
> {
> int ret;
> @@ -262,6 +623,14 @@ static int __init ffa_init(void)
> mutex_init(&drv_info->rx_lock);
> mutex_init(&drv_info->tx_lock);
>
> + /* This will be default device both in theguests and host */
the guests
> + ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL);
> + if (ret)
> + return ret;
> +
> + /* Set up all the partitions that KVM hypervisor maintains */
> + ffa_setup_partitions();
> +
> return 0;
> free_pages:
> if (drv_info->tx_buffer)
> @@ -275,6 +644,13 @@ module_init(ffa_init);
>
> static void __exit ffa_exit(void)
> {
> + struct list_head *p;
> +
> + mutex_lock(&ffa_devs_list_mutex);
> + list_for_each(p, &ffa_devs_list)
> + ffa_device_unregister(list_to_ffa_dev(p));
> + mutex_unlock(&ffa_devs_list_mutex);
> +
> 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);
...
Jonathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions
2020-08-29 17:09 ` [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
@ 2020-09-01 16:38 ` Jonathan Cameron
2020-09-07 11:32 ` Achin Gupta
1 sibling, 0 replies; 24+ messages in thread
From: Jonathan Cameron @ 2020-09-01 16:38 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, Will Deacon, pratikp, tsoni, kernel-team
On Sat, 29 Aug 2020 18:09:23 +0100
Sudeep Holla <sudeep.holla@arm.com> wrote:
> In order to also enable in-kernel users of FFA interface along with
> the access to user-space applications, let us add simple set of operations
> for such devices.
>
> The in-kernel users are registered without the character device interface.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_ffa/driver.c | 119 ++++++++++++++++++++++++++----
> include/linux/arm_ffa.h | 12 +++
> 2 files changed, 117 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index 96113e594db6..811558ef2a1d 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -372,6 +372,97 @@ static void ffa_device_put(struct ffa_device *ffa_dev)
> mutex_unlock(&ffa_dev->mutex);
> }
...
> +
> +static long
> +ffa_dev_ioctl(struct ffa_device *ffa_dev, unsigned int ioctl, void *arg)
> +{
> + long r = -EINVAL;
> +
> + switch (ioctl) {
> + case FFA_GET_API_VERSION:
> + r = drv_info->version;
Early returns would make this a tiny bit shorter and more readable.
return drv_info->version;
etc
> + break;
> + case FFA_GET_PARTITION_ID:
> + r = ffa_dev->vm_id;
> + break;
> + case FFA_GET_PARTITION_INFO: {
> + struct ffa_part_info *pinfo = arg;
> +
> + if (ffa_partition_probe(pinfo->uuid_str, &pinfo->info) != 1)
> + r = -E2BIG;
> + break;
> + }
> + case FFA_SEND_RECEIVE_SYNC: {
> + struct ffa_send_recv_sync *kdata = arg;
> +
> + r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata->endpoint_id,
> + &kdata->data);
> + break;
> + }
> + case FFA_SEND_RECEIVE_ASYNC: {
> + struct ffa_send_recv_async *kdata = arg;
> +
> + if (kdata->length < 0 || kdata->length > RXTX_BUFFER_SIZE) {
> + r = -EINVAL;
> + break;
> + }
> +
> + r = ffa_msg_send(ffa_dev->vm_id, kdata->endpoint_id,
> + kdata->buffer, kdata->length);
> + break;
> + }
> + default:
> + r = -EINVAL;
> + }
> +
> + return r;
> +}
> +
...
Thanks,
Jonathan
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions
2020-08-29 17:09 ` [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions Sudeep Holla
@ 2020-09-02 21:36 ` Rob Herring
2020-09-07 11:41 ` Sudeep Holla
0 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2020-09-02 21:36 UTC (permalink / raw)
To: Sudeep Holla
Cc: devicetree, kernel-team, linux-arm-kernel, Will Deacon, tsoni, pratikp
On Sat, 29 Aug 2020 18:09:16 +0100, Sudeep Holla 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.
>
> This extends the binding to provide the list of partitions that kernel
> drivers may need to access and are not part of the partitions managed
> by the hypervisor.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> .../devicetree/bindings/arm/arm,ffa.yaml | 34 +++++++++++++++++--
> 1 file changed, 32 insertions(+), 2 deletions(-)
>
> Hi,
>
> I am sure this is incomplete, but I couldn't figure out how to make all
> the child properties optional if it is not managed by hypervisor.
>
> Moreover, if we don't like the idea of adding UUID of all the partitions
> that in-kernel drivers may need to communicate to, one alternative I can
> think of is to allow the creation of FFA device from the FFA driver
> itself.
>
> Regards,
> Sudeep
>
My bot found errors running 'make dt_binding_check' on your patch:
Traceback (most recent call last):
File "/usr/local/bin/dt-extract-example", line 45, in <module>
binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load
return constructor.get_single_data()
File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
node = self.composer.get_single_node()
File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 850, in _ruamel_yaml.CParser._compose_sequence_node
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
File "_ruamel_yaml.pyx", line 731, in _ruamel_yaml.CParser._compose_node
File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: while scanning for the next token
found character that cannot start any token
in "<unicode string>", line 98, column 1
make[1]: *** [Documentation/devicetree/bindings/Makefile:18: Documentation/devicetree/bindings/arm/arm,ffa.example.dts] Error 1
make[1]: *** Deleting file 'Documentation/devicetree/bindings/arm/arm,ffa.example.dts'
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/arm/arm,ffa.yaml: while scanning for the next token
found character that cannot start any token
in "<unicode string>", line 98, column 1
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/arm,ffa.yaml: ignoring, error parsing file
warning: no schema found in file: ./Documentation/devicetree/bindings/arm/arm,ffa.yaml
make: *** [Makefile:1366: dt_binding_check] Error 2
See https://patchwork.ozlabs.org/patch/1353741
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure dt-schema is up to date:
pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
Please check and re-submit.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
2020-09-01 16:03 ` Jonathan Cameron
@ 2020-09-02 22:14 ` Rob Herring
2020-11-03 15:59 ` Sudeep Holla
1 sibling, 1 reply; 24+ messages in thread
From: Rob Herring @ 2020-09-02 22:14 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, kernel-team, Will Deacon, tsoni, pratikp
On Sat, Aug 29, 2020 at 06:09:15PM +0100, Sudeep Holla wrote:
> From: Will Deacon <will@kernel.org>
>
> Add devicetree bindings for a FF-A-compliant hypervisor, its partitions
> and their memory regions. The naming is ludicrous but also not by fault.
>
> Signed-off-by: Will Deacon <will@kernel.org>
> (sudeep.holla: Dropped PSA from name and elsewhere as it seem to have
> disappeared mysteriously just before the final release)
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> .../devicetree/bindings/arm/arm,ffa.yaml | 102 ++++++++++++++++++
> .../reserved-memory/arm,ffa-memory.yaml | 71 ++++++++++++
> 2 files changed, 173 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
> create mode 100644 Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.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..668a5995fcab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0
Dual license new bindings:
(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
> +
> +maintainers:
> + - Will Deacon <will@kernel.org>
> +
> +description: |
> + Firmware frameworks implementing partition setup according to the FF-A
> + specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
> + Framework for Arm v8-A") [0] must provide a "manifest and image" for each
> + partition to the "partition manager" so that the partition execution contexts
> + can be initialised.
> +
> + In the case of a virtual FFA instance, the manifest and image details can be
> + passed to the hypervisor (e.g. Linux KVM) using this binding.
> +
> + [0] https://developer.arm.com/docs/den0077/latest
There's efforts to define 'system DT' describing all the CPUs in a
system (such as both A and R cores) as well as physical partitioning.
I'm not sure that virtual partitioning would need a different binding.
Or at least there's probably some overlap.
> +
> +properties:
> + $nodename:
> + const: ffa_hyp
> +
> + compatible:
> + oneOf:
> + - const: arm,ffa-1.0-hypervisor
> +
> + memory-region:
> + $ref: '/schemas/types.yaml#/definitions/phandle'
> + description: |
> + A phandle to the reserved memory region [1] to be used by the hypervisor.
> + The reserved memory region must be compatible with
> + "arm,ffa-1.0-hypervisor-memory-region".
> +
> + [1] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +
> +patternProperties:
> + "^ffa_partition[0-9]+$":
s/_/-/
Probably should use 'reg' to number partitions. Is the numbering
significant?
> + type: object
> + description: One or more child nodes, each describing an FFA partition.
> + properties:
> + $nodename:
> + const: ffa_partition
> +
> + compatible:
> + oneOf:
> + - const: arm,ffa-1.0-partition
> +
> + uuid:
> + $ref: '/schemas/types.yaml#definitions/string'
json-schema can do better here:
format: uuid
Though 'format' will need to be allowed in our meta-schema.
> + description: |
> + The 128-bit UUID [2] of the service implemented by this partition.
> +
> + [2] https://tools.ietf.org/html/rfc4122
> +
> + nr-exec-ctxs:
> + $ref: '/schemas/types.yaml#/definitions/uint32'
> + description: |
> + The number of virtual CPUs to instantiate for this partition.
Just 'nr-cpus' would be clearer in my opinion.
What happens on big.LITTLE?
> +
> + exec-state:
> + description: The execution state in which to execute the partition.
> + oneOf:
> + - const: "AArch64"
> + - const: "AArch32"
Why is this needed? We don't need anything like this for KVM today.
> +
> + entry-point:
> + $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
> + description: |
> + The entry address of the partition specified as an Intermediate
> + Physical Address (IPA) encoded according to the '#address-cells'
> + property.
Is the address unique or you could have the same image for multiple
partitions? If unique, then you could use 'reg' here.
You didn't document using '#address-cells'. Really, I'd just make this a
fixed uint64 (if not 'reg').
> +
> + memory-region:
> + $ref: '/schemas/types.yaml#/definitions/phandle-array'
> + description: |
> + A list of phandles to FFA reserved memory regions [3] for this
> + partition.
> +
> + [3] Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + ffa_hyp {
> + compatible = "arm,ffa-1.0-hypervisor";
> + memory-region = <&ffa_hyp_reserved>;
> +
> + ffa_partition0 {
> + compatible = "arm,ffa-1.0-partition";
> + uuid = "12345678-9abc-def0-1234-56789abcdef0";
> + nr-exec-ctxs = <2>;
> + exec-state = "AArch64";
> + entry-point = <0x80000>;
> + memory-region = <&ffa_reserved0 &ffa_reserved1>;
> + };
> + };
> diff --git a/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml b/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
> new file mode 100644
> index 000000000000..5335e07abcfc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/arm,ffa-memory.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Memory Region for Arm Firmware Framework for Arm v8-A
> +
> +maintainers:
> + - Will Deacon <will@kernel.org>
> +
> +description: |
> + This binding allows a FF-A implementation to describe the normal memory
> + regions of a partition [1] to a hypervisor according to [2].
> +
> + The physical address range reserved for the partition can be specified as a
> + static allocation using the 'reg' property or as a dynamic allocation using
> + the 'size' property. If both properties are omitted, then the hypervisor can
> + allocate physical memory for the partition however it sees fit.
> +
> + [1] Documentation/devicetree/bindings/arm/arm,ffa.yaml
> + [2] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> +
> +properties:
> + $nodename:
> + pattern: "^ffa_mem(@[0-9a-f]+)?$"
> +
> + compatible:
> + oneOf:
> + - const: arm,ffa-1.0-partition-memory-region
> +
> + ipa-range:
> + $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
> + description: |
> + The Intermediate Physical Address (IPA) range (encoded in the same way as
> + a 'reg' property) at which to map the physical memory. If the IPA range is
> + larger than the physical memory region then the region is mapped starting
> + at the base of the IPA range.
> +
> + read-only:
> + type: boolean
> + description: |
> + (static allocation only) The memory region has been pre-populated
> + by the firmware framework and must be mapped without write permission
> + at stage 2.
> +
> + non-executable:
> + type: boolean
> + description: |
> + The memory region must be mapped without execute permission at stage 2.
> +
> +
> +required:
> + - compatible
> +
> +# The "reserved-memory" binding defines additional properties.
> +additionalProperties: true
> +
> +examples:
> + - |
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + ffa_reserved0: ffa_mem@100000000 {
> + compatible = "arm,ffa-1.0-partition-memory-region";
> + reg = <0x1 0x0 0x0 0x04000000>; // 64M @ 1GB
> + ipa-range = <0x0 0x0 0x0 0x04000000>; // 64M @ 0x0
So we need the PA and IPA? We're using DT to define stage 2 page
tables...
> + read-only;
> + };
> + };
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support
2020-08-29 17:09 ` [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
@ 2020-09-07 7:55 ` Fuad Tabba
2020-09-07 9:29 ` Sudeep Holla
0 siblings, 1 reply; 24+ messages in thread
From: Fuad Tabba @ 2020-09-07 7:55 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, kernel-team, Will Deacon, tsoni, pratikp
Hi Sudeep,
I understand that this is an RFC, but I have a few suggestions about
how the FF-A interface code might be structured. See below.
On Sat, Aug 29, 2020 at 6:09 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> 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.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_ffa/Makefile | 3 +-
> drivers/firmware/arm_ffa/common.h | 23 +++
> drivers/firmware/arm_ffa/driver.c | 288 ++++++++++++++++++++++++++++++
> 3 files changed, 313 insertions(+), 1 deletion(-)
> 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 fadb325ee888..1a9bd2bf8752 100644
> --- a/drivers/firmware/arm_ffa/Makefile
> +++ b/drivers/firmware/arm_ffa/Makefile
> @@ -1,3 +1,4 @@
> # SPDX-License-Identifier: GPL-2.0-only
> -obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-bus.o
> +obj-$(CONFIG_ARM_FFA_TRANSPORT) = ffa-bus.o ffa-driver.o
> ffa-bus-y = bus.o
> +ffa-driver-y = driver.o
> diff --git a/drivers/firmware/arm_ffa/common.h b/drivers/firmware/arm_ffa/common.h
> new file mode 100644
> index 000000000000..720c8425dfd6
> --- /dev/null
> +++ b/drivers/firmware/arm_ffa/common.h
> @@ -0,0 +1,23 @@
> +/* 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);
> +
> +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..3670ba400f89
> --- /dev/null
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -0,0 +1,288 @@
> +// 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/bitfield.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/arm_ffa.h>
> +
> +#include "common.h"
> +
> +#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_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_32(0x71)
> +#define FFA_MEM_LEND FFA_SMC_32(0x72)
> +#define FFA_FN64_MEM_LEND FFA_SMC_32(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 FFA_DRIVER_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 64K for now */
> +#define RXTX_BUFFER_SIZE SZ_64K
The code/definitions above will be reused in other parts that deal
will FF-A (e.g., support for FF-A in KVM itself), so it might be good
to have it in a common header. I was wondering if it might even be a
good idea to reuse the Hafnium headers here (assuming I understand
licensing right):
https://review.trustedfirmware.org/plugins/gitiles/hafnium/hafnium/+/refs/heads/master/inc/vmapi/hf/ffa.h
> +
> +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 >= FFA_RET_ABORTED)
> + return ffa_linux_errmap[-errno];
> + return -EINVAL;
> +}
Hardcoding the range check to be bound by FFA_RET_ABORTED could cause
some issues in the future if more error codes are added. It might be
safer to check against the number of elements in ffa_linux_errmap.
> +
> +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_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, vm_id, 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 = 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);
> +}
> +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.17.1
>
Cheers,
/fuad
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support
2020-09-07 7:55 ` Fuad Tabba
@ 2020-09-07 9:29 ` Sudeep Holla
0 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-09-07 9:29 UTC (permalink / raw)
To: Fuad Tabba
Cc: linux-arm-kernel, devicetree, kernel-team, Will Deacon, tsoni,
pratikp, Sudeep Holla
On Mon, Sep 07, 2020 at 08:55:13AM +0100, Fuad Tabba wrote:
> Hi Sudeep,
>
> I understand that this is an RFC, but I have a few suggestions about
> how the FF-A interface code might be structured. See below.
>
> On Sat, Aug 29, 2020 at 6:09 PM Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > 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.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > drivers/firmware/arm_ffa/Makefile | 3 +-
> > drivers/firmware/arm_ffa/common.h | 23 +++
> > drivers/firmware/arm_ffa/driver.c | 288 ++++++++++++++++++++++++++++++
> > 3 files changed, 313 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/firmware/arm_ffa/common.h
> > create mode 100644 drivers/firmware/arm_ffa/driver.c
> >
[...]
> > +
> > +/**
> > + * 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 64K for now */
> > +#define RXTX_BUFFER_SIZE SZ_64K
>
> The code/definitions above will be reused in other parts that deal
> will FF-A (e.g., support for FF-A in KVM itself), so it might be good
> to have it in a common header. I was wondering if it might even be a
> good idea to reuse the Hafnium headers here (assuming I understand
> licensing right):
> https://review.trustedfirmware.org/plugins/gitiles/hafnium/hafnium/+/refs/heads/master/inc/vmapi/hf/ffa.h
>
I know few DTS files have dual license, but I am not sure about the
headers and other source. But I agree on a common header and forgot to
mention that explicitly but I am aware of, that we not only need common
header, but some of the functions may also be reused. I am keeping them
in the driver for now. We can move once we the KVM part also starts
shaping up(before or after one of then gets merged, doesn't matter much)
> > +
> > +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 >= FFA_RET_ABORTED)
> > + return ffa_linux_errmap[-errno];
> > + return -EINVAL;
> > +}
>
> Hardcoding the range check to be bound by FFA_RET_ABORTED could cause
> some issues in the future if more error codes are added. It might be
> safer to check against the number of elements in ffa_linux_errmap.
>
Makes sense, will see how I can fix that.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions
2020-08-29 17:09 ` [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions Sudeep Holla
2020-09-01 16:34 ` Jonathan Cameron
@ 2020-09-07 11:20 ` Achin Gupta
1 sibling, 0 replies; 24+ messages in thread
From: Achin Gupta @ 2020-09-07 11:20 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, kernel-team, Will Deacon, tsoni,
Pratik Patel, nd
Hi Sudeep,
A couple of questions inline...
> On 29 Aug 2020, at 18:09, Sudeep Holla <Sudeep.Holla@arm.com> wrote:
>
> Parse the FFA nodes from the device-tree and register all the partitions
> managed by the KVM hypervisor.
>
> All the partitions including the host(self) are registered as the
> character device with a set of file operations. Most of the interface
> will concentrated in the ioctl.
>
> For now, we have a tiny set of initial ioctls implemented.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_ffa/bus.c | 2 +
> drivers/firmware/arm_ffa/driver.c | 378 +++++++++++++++++++++++++++++-
> include/linux/arm_ffa.h | 3 +
> include/uapi/linux/arm_ffa.h | 56 +++++
> 4 files changed, 438 insertions(+), 1 deletion(-)
> create mode 100644 include/uapi/linux/arm_ffa.h
>
[snip]
> +
> +static long ffa_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> +{
> + long r = -EINVAL;
> + void __user *argp = (void __user *)arg;
> + struct ffa_device *ffa_dev = filp->private_data;
> +
> + switch (ioctl) {
> + case FFA_GET_API_VERSION:
> + if (arg)
> + goto out;
> + r = drv_info->version;
> + break;
> + case FFA_GET_PARTITION_ID:
> + if (arg)
> + goto out;
> + r = ffa_dev->vm_id;
> + break;
> + case FFA_GET_PARTITION_INFO: {
> + struct ffa_partition_info pbuf;
> + struct ffa_part_info __user *pinfo = argp;
> + struct ffa_part_info info;
> + unsigned int count;
> +
> + r = -EFAULT;
> + if (copy_from_user(&info, pinfo, sizeof(info)))
> + break;
> + count = ffa_partition_probe(info.uuid_str, &pbuf);
> + if (count > 1) {
> + r = -E2BIG;
> + break;
> + }
> + r = -EFAULT;
> + if (copy_to_user(pinfo, &info, sizeof(info)))
> + break;
> + r = 0;
> + break;
> + }
> + case FFA_SEND_RECEIVE_SYNC: {
> + struct ffa_send_recv_sync __user *udata = argp;
> + struct ffa_send_recv_sync kdata;
> +
> + r = -EFAULT;
> + if (copy_from_user(&kdata, udata, sizeof(kdata)))
> + break;
> + r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata.endpoint_id,
> + &kdata.data);
> + if (r)
> + break;
> + if (copy_to_user(udata, &kdata, sizeof(kdata)))
> + break;
> + break;
> + }
> + case FFA_SEND_RECEIVE_ASYNC: {
Should this be FFA_SEND_ASYNC? Since asynchronous messaging is being used, there should be a separate FFA_RECEIVE_ASYNC ioctl?
This could be a request to block until a message arrives or poll.
I am also thinking if instead of sync/async, direct/indirect should be used to align with the function names in the module and the FF-A spec.
Also wanted to confirm if an ioctl for FFA_RUN is also on the cards? Once the message has been posted, a vCPU of the target VM would need to run to pick it up?
I have some more comments on how in-kernel users will use indirect messaging. Will leave those in reply to your next patch.
Cheers,
Achin
> + struct ffa_send_recv_async __user *udata = argp;
> + struct ffa_send_recv_async kdata;
> + void *buf;
> +
> + r = -EFAULT;
> + if (copy_from_user(&kdata, udata, sizeof(kdata)))
> + break;
> +
> + if (kdata.length < 0 || kdata.length > RXTX_BUFFER_SIZE) {
> + r = -EINVAL;
> + break;
> + }
> +
> + buf = kzalloc(kdata.length, GFP_KERNEL);
> + if (!buf) {
> + r = -ENOMEM;
> + break;
> + }
> +
> + if (copy_from_user(buf, udata->buffer, kdata.length)) {
> + kfree(buf);
> + break;
> + }
> +
> + r = ffa_msg_send(ffa_dev->vm_id, kdata.endpoint_id, buf,
> + kdata.length);
> + if (r) {
> + kfree(buf);
> + break;
> + }
> +
> + break;
> + }
> + default:
> + r = -EINVAL;
> + }
> +out:
> + return r;
> +}
> +
> +static const struct file_operations ffa_fops = {
> + .owner = THIS_MODULE,
> + .open = ffa_open,
> + .release = ffa_release,
> + .unlocked_ioctl = ffa_ioctl,
> + .llseek = noop_llseek,
> +};
> +
> +static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
> +{
> + int ret;
> + struct ffa_device *ffa_dev;
> +
> + ffa_dev = kzalloc(sizeof(*ffa_dev), GFP_KERNEL);
> + if (!ffa_dev)
> + return -ENOMEM;
> +
> + ffa_dev->vm_id = vm_id;
> + if (uuid)
> + memcpy(uuid, &ffa_dev->uuid, sizeof(*uuid));
> +
> + dev_set_name(&ffa_dev->dev, name);
> + dev_set_drvdata(&ffa_dev->dev, drv_info);
> + cdev_init(&ffa_dev->cdev, &ffa_fops);
> +
> + ret = ffa_device_register(ffa_dev);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&ffa_devs_list_mutex);
> + list_add_tail(&ffa_dev->node, &ffa_devs_list);
> + mutex_unlock(&ffa_devs_list_mutex);
> +
> + return 0;
> +}
> +
> +static int ffa_setup_partitions(void)
> +{
> + int ret;
> + struct ffa_partition_info pbuf;
> + struct device_node *child, *parent;
> + const char *p_uuid, *pfx = "Ignoring FFA partition";
> + const char *compatible = "arm,ffa-1.0-hypervisor";
> + uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
> +
> + parent = of_find_compatible_node(NULL, NULL, compatible);
> + if (!parent)
> + return 0;
> +
> + for_each_child_of_node(parent, child) {
> + if (!of_device_is_compatible(child, "arm,ffa-1.0-partition")) {
> + of_node_put(child);
> + continue;
> + }
> +
> + if (of_property_read_string(child, "uuid", &p_uuid)) {
> + pr_err("%s: failed to parse \"uuid\" property\n", pfx);
> + of_node_put(child);
> + continue;
> + }
> +
> + of_node_put(child);
> +
> + 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 != 1) {
> + pr_err("%s: %s partition info probe failed\n",
> + pfx, p_uuid);
> + return -EINVAL;
> + }
> +
> + ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid);
> + if (ret) {
> + pr_err("%s: failed to register %s\n", pfx, p_uuid);
> + continue;
> + }
> + }
> +
> + of_node_put(parent);
> + return 0;
> +}
> +
> static int __init ffa_init(void)
> {
> int ret;
> @@ -262,6 +623,14 @@ static int __init ffa_init(void)
> mutex_init(&drv_info->rx_lock);
> mutex_init(&drv_info->tx_lock);
>
> + /* This will be default device both in theguests and host */
> + ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL);
> + if (ret)
> + return ret;
> +
> + /* Set up all the partitions that KVM hypervisor maintains */
> + ffa_setup_partitions();
> +
> return 0;
> free_pages:
> if (drv_info->tx_buffer)
> @@ -275,6 +644,13 @@ module_init(ffa_init);
>
> static void __exit ffa_exit(void)
> {
> + struct list_head *p;
> +
> + mutex_lock(&ffa_devs_list_mutex);
> + list_for_each(p, &ffa_devs_list)
> + ffa_device_unregister(list_to_ffa_dev(p));
> + mutex_unlock(&ffa_devs_list_mutex);
> +
> 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);
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 2fe16176149f..6824e0633c77 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -17,6 +17,9 @@ struct ffa_device {
> uuid_t uuid;
> struct device dev;
> struct cdev cdev;
> + size_t num_users;
> + struct mutex mutex; /* protects num_users */
> + struct list_head node;
> };
>
> #define to_ffa_dev(d) container_of(d, struct ffa_device, dev)
> diff --git a/include/uapi/linux/arm_ffa.h b/include/uapi/linux/arm_ffa.h
> new file mode 100644
> index 000000000000..88ddddb4742f
> --- /dev/null
> +++ b/include/uapi/linux/arm_ffa.h
> @@ -0,0 +1,56 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +/*
> + * Copyright (C) 2020 Arm Ltd.
> + */
> +
> +#ifndef _UAPI_LINUX_ARM_FFA_H
> +#define _UAPI_LINUX_ARM_FFA_H
> +
> +#include <linux/types.h>
> +#include <linux/ioctl.h>
> +
> +#define FFA_BASE 0xFF
> +
> +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;
> +};
> +
> +struct ffa_part_info {
> + char uuid_str[36];
> + struct ffa_partition_info info;
> +};
> +
> +struct ffa_send_direct_data {
> + unsigned long data0;
> + unsigned long data1;
> + unsigned long data2;
> + unsigned long data3;
> + unsigned long data4;
> +};
> +
> +struct ffa_send_recv_sync {
> + __u16 endpoint_id;
> + struct ffa_send_direct_data data;
> +};
> +
> +struct ffa_send_recv_async {
> + __u16 endpoint_id;
> + int length;
> + char buffer[];
> +};
> +
> +#define FFA_GET_API_VERSION _IO(FFA_BASE, 0x00)
> +#define FFA_GET_PARTITION_ID _IO(FFA_BASE, 0x01)
> +#define FFA_GET_PARTITION_INFO _IOWR(FFA_BASE, 0x02, struct ffa_part_info)
> +#define FFA_SEND_RECEIVE_SYNC _IOWR(FFA_BASE, 0x03, struct ffa_send_recv_sync)
> +#define FFA_SEND_RECEIVE_ASYNC _IOW(FFA_BASE, 0x04, struct ffa_send_recv_async)
> +
> +#endif /*_UAPI_LINUX_ARM_FFA_H*/
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions
2020-08-29 17:09 ` [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
2020-09-01 16:38 ` Jonathan Cameron
@ 2020-09-07 11:32 ` Achin Gupta
2020-09-07 12:04 ` Andrew Walbran
1 sibling, 1 reply; 24+ messages in thread
From: Achin Gupta @ 2020-09-07 11:32 UTC (permalink / raw)
To: Sudeep Holla
Cc: linux-arm-kernel, devicetree, kernel-team, Will Deacon, tsoni,
Pratik Patel, nd
Hi Sudeep,
CIL...
> On 29 Aug 2020, at 18:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> In order to also enable in-kernel users of FFA interface along with
> the access to user-space applications, let us add simple set of operations
> for such devices.
>
> The in-kernel users are registered without the character device interface.
>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
> drivers/firmware/arm_ffa/driver.c | 119 ++++++++++++++++++++++++++----
> include/linux/arm_ffa.h | 12 +++
> 2 files changed, 117 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> index 96113e594db6..811558ef2a1d 100644
> --- a/drivers/firmware/arm_ffa/driver.c
> +++ b/drivers/firmware/arm_ffa/driver.c
> @@ -372,6 +372,97 @@ static void ffa_device_put(struct ffa_device *ffa_dev)
> mutex_unlock(&ffa_dev->mutex);
> }
>
> +static int ffa_dev_open(struct ffa_device *ffa_dev)
> +{
> + ffa_device_get(ffa_dev);
> +
> + return 0;
> +}
> +
> +static int ffa_dev_close(struct ffa_device *ffa_dev)
> +{
> + ffa_device_put(ffa_dev);
> +
> + return 0;
> +}
> +
> +static long
> +ffa_dev_ioctl(struct ffa_device *ffa_dev, unsigned int ioctl, void *arg)
> +{
> + long r = -EINVAL;
> +
> + switch (ioctl) {
> + case FFA_GET_API_VERSION:
> + r = drv_info->version;
> + break;
> + case FFA_GET_PARTITION_ID:
> + r = ffa_dev->vm_id;
> + break;
> + case FFA_GET_PARTITION_INFO: {
> + struct ffa_part_info *pinfo = arg;
> +
> + if (ffa_partition_probe(pinfo->uuid_str, &pinfo->info) != 1)
> + r = -E2BIG;
> + break;
> + }
> + case FFA_SEND_RECEIVE_SYNC: {
> + struct ffa_send_recv_sync *kdata = arg;
> +
> + r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata->endpoint_id,
> + &kdata->data);
> + break;
> + }
> + case FFA_SEND_RECEIVE_ASYNC: {
For indirect messaging, I am thinking who is responsible for creating and managing the “threads” for each vCPU of a FF-A VM that has an in-kernel user.
An approach is the one taken by the Hafnium driver [1]. It creates and manages a kthread for each vCPU of each VM.
A client of the Hafnium driver only needs to worry about message exchange with various VMs. Thread management is Hafnium specific and rightly stays in the Hafnium module instead of being replicated in each client.
Is this the direct of travel for the FF-A driver as well?
Cheers,
Achin
[1] https://git.trustedfirmware.org/hafnium/driver/linux.git/tree/main.c#n342
> + struct ffa_send_recv_async *kdata = arg;
> +
> + if (kdata->length < 0 || kdata->length > RXTX_BUFFER_SIZE) {
> + r = -EINVAL;
> + break;
> + }
> +
> + r = ffa_msg_send(ffa_dev->vm_id, kdata->endpoint_id,
> + kdata->buffer, kdata->length);
> + break;
> + }
> + default:
> + r = -EINVAL;
> + }
> +
> + return r;
> +}
> +
> +struct ffa_dev_ops ffa_ops = {
> + .open = ffa_dev_open,
> + .close = ffa_dev_close,
> + .ioctl = ffa_dev_ioctl,
> +};
> +
> +struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
> +{
> + struct list_head *p;
> + struct ffa_dev_ops *ops = NULL;
> +
> + if (uuid_is_null(&dev->uuid))
> + return NULL;
> +
> + mutex_lock(&ffa_devs_list_mutex);
> +
> + list_for_each(p, &ffa_devs_list) {
> + struct ffa_device *tmp_dev;
> +
> + tmp_dev = list_to_ffa_dev(p);
> +
> + if (uuid_equal(&tmp_dev->uuid, &dev->uuid)) {
> + ops = &ffa_ops;
> + break;
> + }
> + }
> +
> + mutex_unlock(&ffa_devs_list_mutex);
> +
> + return ops;
> +}
> +
> static int ffa_open(struct inode *inode, struct file *filp)
> {
> struct ffa_device *ffa_dev;
> @@ -380,7 +471,7 @@ static int ffa_open(struct inode *inode, struct file *filp)
> if (!ffa_dev)
> return -EINVAL;
>
> - ffa_device_get(ffa_dev);
> + ffa_dev_open(ffa_dev);
>
> filp->private_data = ffa_dev;
>
> @@ -391,7 +482,7 @@ static int ffa_release(struct inode *inode, struct file *filp)
> {
> struct ffa_device *ffa_dev = filp->private_data;
>
> - ffa_device_put(ffa_dev);
> + ffa_dev_close(ffa_dev);
>
> filp->private_data = NULL;
>
> @@ -406,14 +497,10 @@ static long ffa_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>
> switch (ioctl) {
> case FFA_GET_API_VERSION:
> - if (arg)
> - goto out;
> - r = drv_info->version;
> - break;
> case FFA_GET_PARTITION_ID:
> if (arg)
> goto out;
> - r = ffa_dev->vm_id;
> + r = ffa_dev_ioctl(ffa_dev, FFA_GET_PARTITION_ID, NULL);
> break;
> case FFA_GET_PARTITION_INFO: {
> struct ffa_partition_info pbuf;
> @@ -499,7 +586,8 @@ static const struct file_operations ffa_fops = {
> .llseek = noop_llseek,
> };
>
> -static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
> +static int ffa_device_alloc_register(const char *name, u16 vm_id,
> + uuid_t *uuid, bool cdev_if)
> {
> int ret;
> struct ffa_device *ffa_dev;
> @@ -514,7 +602,7 @@ static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
>
> dev_set_name(&ffa_dev->dev, name);
> dev_set_drvdata(&ffa_dev->dev, drv_info);
> - cdev_init(&ffa_dev->cdev, &ffa_fops);
> + cdev_init(&ffa_dev->cdev, cdev_if ? &ffa_fops : NULL);
>
> ret = ffa_device_register(ffa_dev);
> if (ret)
> @@ -527,13 +615,13 @@ static int ffa_device_alloc_register(const char *name, u16 vm_id, uuid_t *uuid)
> return 0;
> }
>
> -static int ffa_setup_partitions(void)
> +static int ffa_setup_partitions(bool hyp)
> {
> int ret;
> struct ffa_partition_info pbuf;
> struct device_node *child, *parent;
> const char *p_uuid, *pfx = "Ignoring FFA partition";
> - const char *compatible = "arm,ffa-1.0-hypervisor";
> + const char *compatible = hyp ? "arm,ffa-1.0-hypervisor" : "arm,ffa-1.0";
> uuid_t uuid = UUID_INIT(0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
>
> parent = of_find_compatible_node(NULL, NULL, compatible);
> @@ -567,7 +655,7 @@ static int ffa_setup_partitions(void)
> return -EINVAL;
> }
>
> - ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid);
> + ret = ffa_device_alloc_register(p_uuid, pbuf.id, &uuid, hyp);
> if (ret) {
> pr_err("%s: failed to register %s\n", pfx, p_uuid);
> continue;
> @@ -624,12 +712,15 @@ static int __init ffa_init(void)
> mutex_init(&drv_info->tx_lock);
>
> /* This will be default device both in theguests and host */
> - ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL);
> + ret = ffa_device_alloc_register("self", drv_info->vm_id, NULL, true);
> if (ret)
> return ret;
>
> /* Set up all the partitions that KVM hypervisor maintains */
> - ffa_setup_partitions();
> + ffa_setup_partitions(true);
> +
> + /* Set up all the partitions which have in-kernel drivers */
> + ffa_setup_partitions(false);
>
> return 0;
> free_pages:
> diff --git a/include/linux/arm_ffa.h b/include/linux/arm_ffa.h
> index 6824e0633c77..1df7399d207d 100644
> --- a/include/linux/arm_ffa.h
> +++ b/include/linux/arm_ffa.h
> @@ -11,6 +11,7 @@
> #include <linux/module.h>
> #include <linux/types.h>
> #include <linux/uuid.h>
> +#include <uapi/linux/arm_ffa.h>
>
> struct ffa_device {
> u32 vm_id;
> @@ -39,12 +40,19 @@ struct ffa_driver {
>
> #define to_ffa_driver(d) container_of(d, struct ffa_driver, driver)
>
> +struct ffa_dev_ops {
> + int (*open)(struct ffa_device *dev);
> + int (*close)(struct ffa_device *dev);
> + long (*ioctl)(struct ffa_device *dev, unsigned int ioctl, void *arg);
> +};
> +
> #if IS_REACHABLE(CONFIG_ARM_FFA_TRANSPORT)
> int ffa_device_register(struct ffa_device *ffa_dev);
> 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);
> +struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev);
>
> #else
> static inline int ffa_device_register(struct ffa_device *ffa_dev)
> @@ -63,6 +71,10 @@ ffa_driver_register(struct ffa_driver *driver, struct module *owner,
>
> static inline void ffa_driver_unregister(struct ffa_driver *driver) {}
>
> +struct ffa_dev_ops *ffa_dev_ops_get(struct ffa_device *dev)
> +{
> + return NULL;
> +}
> #endif /* CONFIG_ARM_FFA_TRANSPORT */
>
> #define ffa_register(driver) \
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
2020-09-01 16:03 ` Jonathan Cameron
@ 2020-09-07 11:40 ` Sudeep Holla
0 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-09-07 11:40 UTC (permalink / raw)
To: Jonathan Cameron
Cc: linux-arm-kernel, devicetree, Will Deacon, pratikp, tsoni, kernel-team
On Tue, Sep 01, 2020 at 05:03:54PM +0100, Jonathan Cameron wrote:
> On Sat, 29 Aug 2020 18:09:15 +0100
> Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> > From: Will Deacon <will@kernel.org>
> >
> > Add devicetree bindings for a FF-A-compliant hypervisor, its partitions
> > and their memory regions. The naming is ludicrous but also not by fault.
> >
> > Signed-off-by: Will Deacon <will@kernel.org>
> > (sudeep.holla: Dropped PSA from name and elsewhere as it seem to have
> > disappeared mysteriously just before the final release)
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>
> As I was reading this out of curiosity...
>
Thanks for the review, all comments taken and incorporated in all other
patches. I will look at this yaml in detail soon.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions
2020-09-02 21:36 ` Rob Herring
@ 2020-09-07 11:41 ` Sudeep Holla
0 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-09-07 11:41 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, kernel-team, linux-arm-kernel, Will Deacon, tsoni, pratikp
On Wed, Sep 02, 2020 at 03:36:52PM -0600, Rob Herring wrote:
> On Sat, 29 Aug 2020 18:09:16 +0100, Sudeep Holla 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.
> >
> > This extends the binding to provide the list of partitions that kernel
> > drivers may need to access and are not part of the partitions managed
> > by the hypervisor.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > .../devicetree/bindings/arm/arm,ffa.yaml | 34 +++++++++++++++++--
> > 1 file changed, 32 insertions(+), 2 deletions(-)
> >
> > Hi,
> >
> > I am sure this is incomplete, but I couldn't figure out how to make all
> > the child properties optional if it is not managed by hypervisor.
> >
> > Moreover, if we don't like the idea of adding UUID of all the partitions
> > that in-kernel drivers may need to communicate to, one alternative I can
> > think of is to allow the creation of FFA device from the FFA driver
> > itself.
> >
> > Regards,
> > Sudeep
> >
>
>
> My bot found errors running 'make dt_binding_check' on your patch:
>
> Traceback (most recent call last):
> File "/usr/local/bin/dt-extract-example", line 45, in <module>
> binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
> File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 343, in load
> return constructor.get_single_data()
> File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 111, in get_single_data
> node = self.composer.get_single_node()
> File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
> File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
> File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
> File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 850, in _ruamel_yaml.CParser._compose_sequence_node
> File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
> File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
> File "_ruamel_yaml.pyx", line 731, in _ruamel_yaml.CParser._compose_node
> File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
> ruamel.yaml.scanner.ScannerError: while scanning for the next token
> found character that cannot start any token
> in "<unicode string>", line 98, column 1
> make[1]: *** [Documentation/devicetree/bindings/Makefile:18: Documentation/devicetree/bindings/arm/arm,ffa.example.dts] Error 1
> make[1]: *** Deleting file 'Documentation/devicetree/bindings/arm/arm,ffa.example.dts'
> make[1]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/arm/arm,ffa.yaml: while scanning for the next token
> found character that cannot start any token
> in "<unicode string>", line 98, column 1
> /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/arm,ffa.yaml: ignoring, error parsing file
> warning: no schema found in file: ./Documentation/devicetree/bindings/arm/arm,ffa.yaml
> make: *** [Makefile:1366: dt_binding_check] Error 2
>
>
> See https://patchwork.ozlabs.org/patch/1353741
>
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure dt-schema is up to date:
>
> pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade
>
> Please check and re-submit.
>
Thanks for the report and the steps to setup. I will try this soon.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions
2020-09-07 11:32 ` Achin Gupta
@ 2020-09-07 12:04 ` Andrew Walbran
0 siblings, 0 replies; 24+ messages in thread
From: Andrew Walbran @ 2020-09-07 12:04 UTC (permalink / raw)
To: Achin Gupta
Cc: Sudeep Holla, linux-arm-kernel, devicetree, kernel-team,
Will Deacon, tsoni, Pratik Patel, nd
[-- Attachment #1: Type: text/plain, Size: 3040 bytes --]
On Mon, 7 Sep 2020 at 12:32, Achin Gupta <Achin.Gupta@arm.com> wrote:
>
> Hi Sudeep,
>
> CIL...
>
> > On 29 Aug 2020, at 18:09, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > In order to also enable in-kernel users of FFA interface along with
> > the access to user-space applications, let us add simple set of operations
> > for such devices.
> >
> > The in-kernel users are registered without the character device interface.
> >
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > drivers/firmware/arm_ffa/driver.c | 119 ++++++++++++++++++++++++++----
> > include/linux/arm_ffa.h | 12 +++
> > 2 files changed, 117 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/firmware/arm_ffa/driver.c b/drivers/firmware/arm_ffa/driver.c
> > index 96113e594db6..811558ef2a1d 100644
> > --- a/drivers/firmware/arm_ffa/driver.c
> > +++ b/drivers/firmware/arm_ffa/driver.c
> > @@ -372,6 +372,97 @@ static void ffa_device_put(struct ffa_device *ffa_dev)
> > mutex_unlock(&ffa_dev->mutex);
> > }
> >
> > +static int ffa_dev_open(struct ffa_device *ffa_dev)
> > +{
> > + ffa_device_get(ffa_dev);
> > +
> > + return 0;
> > +}
> > +
> > +static int ffa_dev_close(struct ffa_device *ffa_dev)
> > +{
> > + ffa_device_put(ffa_dev);
> > +
> > + return 0;
> > +}
> > +
> > +static long
> > +ffa_dev_ioctl(struct ffa_device *ffa_dev, unsigned int ioctl, void *arg)
> > +{
> > + long r = -EINVAL;
> > +
> > + switch (ioctl) {
> > + case FFA_GET_API_VERSION:
> > + r = drv_info->version;
> > + break;
> > + case FFA_GET_PARTITION_ID:
> > + r = ffa_dev->vm_id;
> > + break;
> > + case FFA_GET_PARTITION_INFO: {
> > + struct ffa_part_info *pinfo = arg;
> > +
> > + if (ffa_partition_probe(pinfo->uuid_str, &pinfo->info) != 1)
> > + r = -E2BIG;
> > + break;
> > + }
> > + case FFA_SEND_RECEIVE_SYNC: {
> > + struct ffa_send_recv_sync *kdata = arg;
> > +
> > + r = ffa_msg_send_direct_req(ffa_dev->vm_id, kdata->endpoint_id,
> > + &kdata->data);
> > + break;
> > + }
> > + case FFA_SEND_RECEIVE_ASYNC: {
>
> For indirect messaging, I am thinking who is responsible for creating and managing the “threads” for each vCPU of a FF-A VM that has an in-kernel user.
>
> An approach is the one taken by the Hafnium driver [1]. It creates and manages a kthread for each vCPU of each VM.
>
> A client of the Hafnium driver only needs to worry about message exchange with various VMs. Thread management is Hafnium specific and rightly stays in the Hafnium module instead of being replicated in each client.
>
> Is this the direct of travel for the FF-A driver as well?
I think what we want for the Android case at least is for these
threads to be managed in userspace by crosvm, following the
(Protected) KVM model.
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3852 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding
2020-09-02 22:14 ` Rob Herring
@ 2020-11-03 15:59 ` Sudeep Holla
0 siblings, 0 replies; 24+ messages in thread
From: Sudeep Holla @ 2020-11-03 15:59 UTC (permalink / raw)
To: Rob Herring
Cc: linux-arm-kernel, devicetree, kernel-team, Will Deacon, tsoni, pratikp
On Wed, Sep 02, 2020 at 04:14:13PM -0600, Rob Herring wrote:
> On Sat, Aug 29, 2020 at 06:09:15PM +0100, Sudeep Holla wrote:
> > From: Will Deacon <will@kernel.org>
> >
> > Add devicetree bindings for a FF-A-compliant hypervisor, its partitions
> > and their memory regions. The naming is ludicrous but also not by fault.
> >
> > Signed-off-by: Will Deacon <will@kernel.org>
> > (sudeep.holla: Dropped PSA from name and elsewhere as it seem to have
> > disappeared mysteriously just before the final release)
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > ---
> > .../devicetree/bindings/arm/arm,ffa.yaml | 102 ++++++++++++++++++
> > .../reserved-memory/arm,ffa-memory.yaml | 71 ++++++++++++
> > 2 files changed, 173 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/arm/arm,ffa.yaml
> > create mode 100644 Documentation/devicetree/bindings/reserved-memory/arm,ffa-memory.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..668a5995fcab
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/arm,ffa.yaml
> > @@ -0,0 +1,102 @@
> > +# SPDX-License-Identifier: GPL-2.0
>
> Dual license new bindings:
>
> (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
> > +
> > +maintainers:
> > + - Will Deacon <will@kernel.org>
> > +
> > +description: |
> > + Firmware frameworks implementing partition setup according to the FF-A
> > + specification defined by ARM document number ARM DEN 0077A ("Arm Firmware
> > + Framework for Arm v8-A") [0] must provide a "manifest and image" for each
> > + partition to the "partition manager" so that the partition execution contexts
> > + can be initialised.
> > +
> > + In the case of a virtual FFA instance, the manifest and image details can be
> > + passed to the hypervisor (e.g. Linux KVM) using this binding.
> > +
> > + [0] https://developer.arm.com/docs/den0077/latest
>
> There's efforts to define 'system DT' describing all the CPUs in a
> system (such as both A and R cores) as well as physical partitioning.
> I'm not sure that virtual partitioning would need a different binding.
> Or at least there's probably some overlap.
>
> > +
> > +properties:
> > + $nodename:
> > + const: ffa_hyp
> > +
> > + compatible:
> > + oneOf:
> > + - const: arm,ffa-1.0-hypervisor
> > +
> > + memory-region:
> > + $ref: '/schemas/types.yaml#/definitions/phandle'
> > + description: |
> > + A phandle to the reserved memory region [1] to be used by the hypervisor.
> > + The reserved memory region must be compatible with
> > + "arm,ffa-1.0-hypervisor-memory-region".
> > +
> > + [1] Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
> > +
> > +patternProperties:
> > + "^ffa_partition[0-9]+$":
>
> s/_/-/
>
> Probably should use 'reg' to number partitions. Is the numbering
> significant?
>
Not really, the partitions will get IDs assigned by Hypervisor or Secure
Partition Manager.
> > + type: object
> > + description: One or more child nodes, each describing an FFA partition.
> > + properties:
> > + $nodename:
> > + const: ffa_partition
> > +
> > + compatible:
> > + oneOf:
> > + - const: arm,ffa-1.0-partition
> > +
> > + uuid:
> > + $ref: '/schemas/types.yaml#definitions/string'
>
> json-schema can do better here:
>
> format: uuid
>
> Though 'format' will need to be allowed in our meta-schema.
>
OK I need to explore this.
> > + description: |
> > + The 128-bit UUID [2] of the service implemented by this partition.
> > +
> > + [2] https://tools.ietf.org/html/rfc4122
> > +
> > + nr-exec-ctxs:
> > + $ref: '/schemas/types.yaml#/definitions/uint32'
> > + description: |
> > + The number of virtual CPUs to instantiate for this partition.
>
> Just 'nr-cpus' would be clearer in my opinion.
>
The idea must be to use the names as is from the spec.
> What happens on big.LITTLE?
>
AFAIU, we don't have notion of bL in KVM.
> > +
> > + exec-state:
> > + description: The execution state in which to execute the partition.
> > + oneOf:
> > + - const: "AArch64"
> > + - const: "AArch32"
>
> Why is this needed? We don't need anything like this for KVM today.
>
Again this is from the spec listed under manifests for the partitions.
This will let the KVM know to start the partition in Aarch32/64.
> > +
> > + entry-point:
> > + $ref: '/schemas/types.yaml#/definitions/uint32-matrix'
> > + description: |
> > + The entry address of the partition specified as an Intermediate
> > + Physical Address (IPA) encoded according to the '#address-cells'
> > + property.
>
> Is the address unique or you could have the same image for multiple
> partitions? If unique, then you could use 'reg' here.
>
I don't have full knowledge on the details, will let Will comment on this.
> You didn't document using '#address-cells'. Really, I'd just make this a
> fixed uint64 (if not 'reg').
>
I was wondering the same as the format from spec must be fixed, checked and
unfortunately it is not 🙁.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-11-03 16:00 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-29 17:09 [PATCH 0/9] firmware: Add initial support for Arm FF-A Sudeep Holla
2020-08-29 17:09 ` [PATCH 1/9] dt-bindings: Arm: Add Firmware Framework for Armv8-A (FF-A) binding Sudeep Holla
2020-09-01 16:03 ` Jonathan Cameron
2020-09-07 11:40 ` Sudeep Holla
2020-09-02 22:14 ` Rob Herring
2020-11-03 15:59 ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 2/9] dt-bindings: Arm: Extend FF-A binding to support in-kernel usage of partitions Sudeep Holla
2020-09-02 21:36 ` Rob Herring
2020-09-07 11:41 ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 3/9] arm64: smccc: Add support for SMCCCv1.2 input/output registers Sudeep Holla
2020-08-29 17:09 ` [PATCH 4/9] firmware: smccc: export both smccc functions Sudeep Holla
2020-08-29 17:09 ` [PATCH 5/9] firmware: arm_ffa: Add initial FFA bus support for device enumeration Sudeep Holla
2020-08-29 17:09 ` [PATCH 6/9] firmware: arm_ffa: Add initial Arm FFA driver support Sudeep Holla
2020-09-07 7:55 ` Fuad Tabba
2020-09-07 9:29 ` Sudeep Holla
2020-08-29 17:09 ` [PATCH 7/9] firmware: arm_ffa: Add support for SMCCC as transport to FFA driver Sudeep Holla
2020-09-01 16:23 ` Jonathan Cameron
2020-08-29 17:09 ` [PATCH 8/9] firmware: arm_ffa: Setup and register all the KVM managed partitions Sudeep Holla
2020-09-01 16:34 ` Jonathan Cameron
2020-09-07 11:20 ` Achin Gupta
2020-08-29 17:09 ` [PATCH 9/9] firmware: arm_ffa: Setup in-kernel users of FFA partitions Sudeep Holla
2020-09-01 16:38 ` Jonathan Cameron
2020-09-07 11:32 ` Achin Gupta
2020-09-07 12:04 ` Andrew Walbran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).