* Re: [PATCH v3 00/14] arm64: Support for running as a guest in Arm CCA
2024-06-05 9:29 [PATCH v3 00/14] arm64: Support for running as a guest in Arm CCA Steven Price
@ 2024-06-05 8:37 ` Itaru Kitayama
2024-06-06 9:03 ` Steven Price
2024-06-05 9:29 ` [PATCH v3 01/14] arm64: rsi: Add RSI definitions Steven Price
` (14 subsequent siblings)
15 siblings, 1 reply; 54+ messages in thread
From: Itaru Kitayama @ 2024-06-05 8:37 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni
Hi Steven,
On Wed, Jun 05, 2024 at 10:29:52AM +0100, Steven Price wrote:
> This series adds support for running Linux in a protected VM under the
> Arm Confidential Compute Architecture (CCA). This has been updated
> following the feedback from the v2 posting[1]. Thanks for the feedback!
> Individual patches have a change log for v3.
>
> The biggest change from v2 is fixing set_memory_{en,de}crypted() to
> perform a break-before-make sequence. Note that only the virtual address
> supplied is flipped between shared and protected, so if e.g. a vmalloc()
> address is passed the linear map will still point to the (now invalid)
> previous IPA. Attempts to access the wrong address may trigger a
> Synchronous External Abort. However any code which attempts to access
> the 'encrypted' alias after set_memory_decrypted() is already likely to
> be broken on platforms that implement memory encryption, so I don't
> expect problems.
>
> The ABI to the RMM from a realm (the RSI) is based on the final RMM v1.0
> (EAC 5) specification[2]. Future RMM specifications will be backwards
> compatible so a guest using the v1.0 specification (i.e. this series)
> will be able to run on future versions of the RMM without modification.
>
> Arm plans to set up a CI system to perform at a minimum boot testing of
> Linux as a guest within a realm.
>
> This series is based on v6.10-rc1. It is also available as a git
> repository:
>
> https://gitlab.arm.com/linux-arm/linux-cca cca-guest/v3
>
> This series (the guest side) should be in a good state so please review
> with the intention that this could be merged soon. The host side (KVM
> changes) is likely to require some more iteration and I'll post that as
> a separate series shortly - note that there is no tie between the series
> (i.e. you can mix and match v2 and v3 postings of the host and guest).
>
> Introduction (unchanged from v2 posting)
> ============
> A more general introduction to Arm CCA is available on the Arm
> website[3], and links to the other components involved are available in
> the overall cover letter.
>
> Arm Confidential Compute Architecture adds two new 'worlds' to the
> architecture: Root and Realm. A new software component known as the RMM
> (Realm Management Monitor) runs in Realm EL2 and is trusted by both the
> Normal World and VMs running within Realms. This enables mutual
> distrust between the Realm VMs and the Normal World.
>
> Virtual machines running within a Realm can decide on a (4k)
> page-by-page granularity whether to share a page with the (Normal World)
> host or to keep it private (protected). This protection is provided by
> the hardware and attempts to access a page which isn't shared by the
> Normal World will trigger a Granule Protection Fault.
>
> Realm VMs can communicate with the RMM via another SMC interface known
> as RSI (Realm Services Interface). This series adds wrappers for the
> full set of RSI commands and uses them to manage the Realm IPA State
> (RIPAS) and to discover the configuration of the realm.
>
> The VM running within the Realm needs to ensure that memory that is
> going to use is marked as 'RIPAS_RAM' (i.e. protected memory accessible
> only to the guest). This could be provided by the VMM (and subject to
> measurement to ensure it is setup correctly) or the VM can set it
> itself. This series includes a patch which will iterate over all
> described RAM and set the RIPAS. This is a relatively cheap operation,
> and doesn't require memory donation from the host. Instead, memory can
> be dynamically provided by the host on fault. An alternative would be to
> update booting.rst and state this as a requirement, but this would
> reduce the flexibility of the VMM to manage the available memory to the
> guest (as the initial RIPAS state is part of the guest's measurement).
>
> Within the Realm the most-significant active bit of the IPA is used to
> select whether the access is to protected memory or to memory shared
> with the host. This series treats this bit as if it is attribute bit in
> the page tables and will modify it when sharing/unsharing memory with
> the host.
>
> This top bit usage also necessitates that the IPA width is made more
> dynamic in the guest. The VMM will choose a width (and therefore which
> bit controls the shared flag) and the guest must be able to identify
> this bit to mask it out when necessary. PHYS_MASK_SHIFT/PHYS_MASK are
> therefore made dynamic.
>
> To allow virtio to communicate with the host the shared buffers must be
> placed in memory which has this top IPA bit set. This is achieved by
> implementing the set_memory_{encrypted,decrypted} APIs for arm64 and
> forcing the use of bounce buffers. For now all device access is
> considered to required the memory to be shared, at this stage there is
> no support for real devices to be assigned to a realm guest - obviously
> if device assignment is added this will have to change.
>
> Finally the GIC is (largely) emulated by the (untrusted) host. The RMM
> provides some management (including register save/restore) but the
> ITS buffers must be placed into shared memory for the host to emulate.
> There is likely to be future work to harden the GIC driver against a
> malicious host (along with any other drivers used within a Realm guest).
>
> [1] https://lore.kernel.org/r/20240412084213.1733764-1-steven.price%40arm.com
> [2] https://developer.arm.com/documentation/den0137/1-0eac5/
> [3] https://www.arm.com/architecture/security-features/arm-confidential-compute-architecture
>
The v3 guest built with clang booted fine on FVP backed by v2 host kernel.
Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com>
Thanks,
Itaru.
> Sami Mujawar (2):
> arm64: rsi: Interfaces to query attestation token
> virt: arm-cca-guest: TSM_REPORT support for realms
>
> Steven Price (5):
> arm64: realm: Query IPA size from the RMM
> arm64: Mark all I/O as non-secure shared
> arm64: Make the PHYS_MASK_SHIFT dynamic
> arm64: Enforce bounce buffers for realm DMA
> arm64: realm: Support nonsecure ITS emulation shared
>
> Suzuki K Poulose (7):
> arm64: rsi: Add RSI definitions
> arm64: Detect if in a realm and set RIPAS RAM
> fixmap: Allow architecture overriding set_fixmap_io
> arm64: Override set_fixmap_io
> arm64: Enable memory encrypt for Realms
> arm64: Force device mappings to be non-secure shared
> efi: arm64: Map Device with Prot Shared
>
> arch/arm64/Kconfig | 3 +
> arch/arm64/include/asm/fixmap.h | 4 +-
> arch/arm64/include/asm/io.h | 6 +-
> arch/arm64/include/asm/mem_encrypt.h | 17 ++
> arch/arm64/include/asm/pgtable-hwdef.h | 6 -
> arch/arm64/include/asm/pgtable-prot.h | 3 +
> arch/arm64/include/asm/pgtable.h | 7 +-
> arch/arm64/include/asm/rsi.h | 48 ++++
> arch/arm64/include/asm/rsi_cmds.h | 143 ++++++++++++
> arch/arm64/include/asm/rsi_smc.h | 142 ++++++++++++
> arch/arm64/include/asm/set_memory.h | 3 +
> arch/arm64/kernel/Makefile | 3 +-
> arch/arm64/kernel/efi.c | 2 +-
> arch/arm64/kernel/rsi.c | 96 ++++++++
> arch/arm64/kernel/setup.c | 8 +
> arch/arm64/mm/init.c | 10 +-
> arch/arm64/mm/mmu.c | 13 ++
> arch/arm64/mm/pageattr.c | 65 +++++-
> drivers/irqchip/irq-gic-v3-its.c | 90 ++++++--
> drivers/virt/coco/Kconfig | 2 +
> drivers/virt/coco/Makefile | 1 +
> drivers/virt/coco/arm-cca-guest/Kconfig | 11 +
> drivers/virt/coco/arm-cca-guest/Makefile | 2 +
> .../virt/coco/arm-cca-guest/arm-cca-guest.c | 211 ++++++++++++++++++
> include/asm-generic/fixmap.h | 2 +
> 25 files changed, 858 insertions(+), 40 deletions(-)
> create mode 100644 arch/arm64/include/asm/mem_encrypt.h
> create mode 100644 arch/arm64/include/asm/rsi.h
> create mode 100644 arch/arm64/include/asm/rsi_cmds.h
> create mode 100644 arch/arm64/include/asm/rsi_smc.h
> create mode 100644 arch/arm64/kernel/rsi.c
> create mode 100644 drivers/virt/coco/arm-cca-guest/Kconfig
> create mode 100644 drivers/virt/coco/arm-cca-guest/Makefile
> create mode 100644 drivers/virt/coco/arm-cca-guest/arm-cca-guest.c
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH v3 00/14] arm64: Support for running as a guest in Arm CCA
2024-06-05 8:37 ` Itaru Kitayama
@ 2024-06-06 9:03 ` Steven Price
0 siblings, 0 replies; 54+ messages in thread
From: Steven Price @ 2024-06-06 9:03 UTC (permalink / raw)
To: Itaru Kitayama
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Suzuki K Poulose, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni
On 05/06/2024 09:37, Itaru Kitayama wrote:
> Hi Steven,
> On Wed, Jun 05, 2024 at 10:29:52AM +0100, Steven Price wrote:
>> This series adds support for running Linux in a protected VM under the
>> Arm Confidential Compute Architecture (CCA). This has been updated
>> following the feedback from the v2 posting[1]. Thanks for the feedback!
>> Individual patches have a change log for v3.
>>
>> The biggest change from v2 is fixing set_memory_{en,de}crypted() to
>> perform a break-before-make sequence. Note that only the virtual address
>> supplied is flipped between shared and protected, so if e.g. a vmalloc()
>> address is passed the linear map will still point to the (now invalid)
>> previous IPA. Attempts to access the wrong address may trigger a
>> Synchronous External Abort. However any code which attempts to access
>> the 'encrypted' alias after set_memory_decrypted() is already likely to
>> be broken on platforms that implement memory encryption, so I don't
>> expect problems.
>>
>> The ABI to the RMM from a realm (the RSI) is based on the final RMM v1.0
>> (EAC 5) specification[2]. Future RMM specifications will be backwards
>> compatible so a guest using the v1.0 specification (i.e. this series)
>> will be able to run on future versions of the RMM without modification.
>>
>> Arm plans to set up a CI system to perform at a minimum boot testing of
>> Linux as a guest within a realm.
>>
>> This series is based on v6.10-rc1. It is also available as a git
>> repository:
>>
>> https://gitlab.arm.com/linux-arm/linux-cca cca-guest/v3
>>
>> This series (the guest side) should be in a good state so please review
>> with the intention that this could be merged soon. The host side (KVM
>> changes) is likely to require some more iteration and I'll post that as
>> a separate series shortly - note that there is no tie between the series
>> (i.e. you can mix and match v2 and v3 postings of the host and guest).
>>
>> Introduction (unchanged from v2 posting)
>> ============
>> A more general introduction to Arm CCA is available on the Arm
>> website[3], and links to the other components involved are available in
>> the overall cover letter.
>>
>> Arm Confidential Compute Architecture adds two new 'worlds' to the
>> architecture: Root and Realm. A new software component known as the RMM
>> (Realm Management Monitor) runs in Realm EL2 and is trusted by both the
>> Normal World and VMs running within Realms. This enables mutual
>> distrust between the Realm VMs and the Normal World.
>>
>> Virtual machines running within a Realm can decide on a (4k)
>> page-by-page granularity whether to share a page with the (Normal World)
>> host or to keep it private (protected). This protection is provided by
>> the hardware and attempts to access a page which isn't shared by the
>> Normal World will trigger a Granule Protection Fault.
>>
>> Realm VMs can communicate with the RMM via another SMC interface known
>> as RSI (Realm Services Interface). This series adds wrappers for the
>> full set of RSI commands and uses them to manage the Realm IPA State
>> (RIPAS) and to discover the configuration of the realm.
>>
>> The VM running within the Realm needs to ensure that memory that is
>> going to use is marked as 'RIPAS_RAM' (i.e. protected memory accessible
>> only to the guest). This could be provided by the VMM (and subject to
>> measurement to ensure it is setup correctly) or the VM can set it
>> itself. This series includes a patch which will iterate over all
>> described RAM and set the RIPAS. This is a relatively cheap operation,
>> and doesn't require memory donation from the host. Instead, memory can
>> be dynamically provided by the host on fault. An alternative would be to
>> update booting.rst and state this as a requirement, but this would
>> reduce the flexibility of the VMM to manage the available memory to the
>> guest (as the initial RIPAS state is part of the guest's measurement).
>>
>> Within the Realm the most-significant active bit of the IPA is used to
>> select whether the access is to protected memory or to memory shared
>> with the host. This series treats this bit as if it is attribute bit in
>> the page tables and will modify it when sharing/unsharing memory with
>> the host.
>>
>> This top bit usage also necessitates that the IPA width is made more
>> dynamic in the guest. The VMM will choose a width (and therefore which
>> bit controls the shared flag) and the guest must be able to identify
>> this bit to mask it out when necessary. PHYS_MASK_SHIFT/PHYS_MASK are
>> therefore made dynamic.
>>
>> To allow virtio to communicate with the host the shared buffers must be
>> placed in memory which has this top IPA bit set. This is achieved by
>> implementing the set_memory_{encrypted,decrypted} APIs for arm64 and
>> forcing the use of bounce buffers. For now all device access is
>> considered to required the memory to be shared, at this stage there is
>> no support for real devices to be assigned to a realm guest - obviously
>> if device assignment is added this will have to change.
>>
>> Finally the GIC is (largely) emulated by the (untrusted) host. The RMM
>> provides some management (including register save/restore) but the
>> ITS buffers must be placed into shared memory for the host to emulate.
>> There is likely to be future work to harden the GIC driver against a
>> malicious host (along with any other drivers used within a Realm guest).
>>
>> [1] https://lore.kernel.org/r/20240412084213.1733764-1-steven.price%40arm.com
>> [2] https://developer.arm.com/documentation/den0137/1-0eac5/
>> [3] https://www.arm.com/architecture/security-features/arm-confidential-compute-architecture
>>
>
> The v3 guest built with clang booted fine on FVP backed by v2 host kernel.
>
> Tested-by: Itaru Kitayama <itaru.kitayama@fujitsu.com>
Thanks for testing!
Steve
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 01/14] arm64: rsi: Add RSI definitions
2024-06-05 9:29 [PATCH v3 00/14] arm64: Support for running as a guest in Arm CCA Steven Price
2024-06-05 8:37 ` Itaru Kitayama
@ 2024-06-05 9:29 ` Steven Price
2024-06-10 14:14 ` Catalin Marinas
2024-06-05 9:29 ` [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM Steven Price
` (13 subsequent siblings)
15 siblings, 1 reply; 54+ messages in thread
From: Steven Price @ 2024-06-05 9:29 UTC (permalink / raw)
To: kvm, kvmarm
Cc: Suzuki K Poulose, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Steven Price
From: Suzuki K Poulose <suzuki.poulose@arm.com>
The RMM (Realm Management Monitor) provides functionality that can be
accessed by a realm guest through SMC (Realm Services Interface) calls.
The SMC definitions are based on DEN0137[1] version A-eac5.
[1] https://developer.arm.com/documentation/den0137/latest
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
Changes since v2:
* Rename rsi_get_version() to rsi_request_version()
* Fix size/alignment of struct realm_config
---
arch/arm64/include/asm/rsi_cmds.h | 47 ++++++++++
arch/arm64/include/asm/rsi_smc.h | 142 ++++++++++++++++++++++++++++++
2 files changed, 189 insertions(+)
create mode 100644 arch/arm64/include/asm/rsi_cmds.h
create mode 100644 arch/arm64/include/asm/rsi_smc.h
diff --git a/arch/arm64/include/asm/rsi_cmds.h b/arch/arm64/include/asm/rsi_cmds.h
new file mode 100644
index 000000000000..ad425c5d6f1b
--- /dev/null
+++ b/arch/arm64/include/asm/rsi_cmds.h
@@ -0,0 +1,47 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 ARM Ltd.
+ */
+
+#ifndef __ASM_RSI_CMDS_H
+#define __ASM_RSI_CMDS_H
+
+#include <linux/arm-smccc.h>
+
+#include <asm/rsi_smc.h>
+
+static inline void invoke_rsi_fn_smc_with_res(unsigned long function_id,
+ unsigned long arg0,
+ unsigned long arg1,
+ unsigned long arg2,
+ unsigned long arg3,
+ struct arm_smccc_res *res)
+{
+ arm_smccc_smc(function_id, arg0, arg1, arg2, arg3, 0, 0, 0, res);
+}
+
+static inline unsigned long rsi_request_version(unsigned long req,
+ unsigned long *out_lower,
+ unsigned long *out_higher)
+{
+ struct arm_smccc_res res;
+
+ invoke_rsi_fn_smc_with_res(SMC_RSI_ABI_VERSION, req, 0, 0, 0, &res);
+
+ if (out_lower)
+ *out_lower = res.a1;
+ if (out_higher)
+ *out_higher = res.a2;
+
+ return res.a0;
+}
+
+static inline unsigned long rsi_get_realm_config(struct realm_config *cfg)
+{
+ struct arm_smccc_res res;
+
+ invoke_rsi_fn_smc_with_res(SMC_RSI_REALM_CONFIG, virt_to_phys(cfg), 0, 0, 0, &res);
+ return res.a0;
+}
+
+#endif
diff --git a/arch/arm64/include/asm/rsi_smc.h b/arch/arm64/include/asm/rsi_smc.h
new file mode 100644
index 000000000000..c0a65caa3ab3
--- /dev/null
+++ b/arch/arm64/include/asm/rsi_smc.h
@@ -0,0 +1,142 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2023 ARM Ltd.
+ */
+
+#ifndef __SMC_RSI_H_
+#define __SMC_RSI_H_
+
+/*
+ * This file describes the Realm Services Interface (RSI) Application Binary
+ * Interface (ABI) for SMC calls made from within the Realm to the RMM and
+ * serviced by the RMM.
+ */
+
+#define SMC_RSI_CALL_BASE 0xC4000000
+
+/*
+ * The major version number of the RSI implementation. Increase this whenever
+ * the binary format or semantics of the SMC calls change.
+ */
+#define RSI_ABI_VERSION_MAJOR 1
+
+/*
+ * The minor version number of the RSI implementation. Increase this when
+ * a bug is fixed, or a feature is added without breaking binary compatibility.
+ */
+#define RSI_ABI_VERSION_MINOR 0
+
+#define RSI_ABI_VERSION ((RSI_ABI_VERSION_MAJOR << 16) | \
+ RSI_ABI_VERSION_MINOR)
+
+#define RSI_ABI_VERSION_GET_MAJOR(_version) ((_version) >> 16)
+#define RSI_ABI_VERSION_GET_MINOR(_version) ((_version) & 0xFFFF)
+
+#define RSI_SUCCESS 0
+#define RSI_ERROR_INPUT 1
+#define RSI_ERROR_STATE 2
+#define RSI_INCOMPLETE 3
+
+#define SMC_RSI_FID(_x) (SMC_RSI_CALL_BASE + (_x))
+
+#define SMC_RSI_ABI_VERSION SMC_RSI_FID(0x190)
+
+/*
+ * arg1 == Challenge value, bytes: 0 - 7
+ * arg2 == Challenge value, bytes: 7 - 15
+ * arg3 == Challenge value, bytes: 16 - 23
+ * arg4 == Challenge value, bytes: 24 - 31
+ * arg5 == Challenge value, bytes: 32 - 39
+ * arg6 == Challenge value, bytes: 40 - 47
+ * arg7 == Challenge value, bytes: 48 - 55
+ * arg8 == Challenge value, bytes: 56 - 63
+ * ret0 == Status / error
+ * ret1 == Upper bound of token size in bytes
+ */
+#define SMC_RSI_ATTESTATION_TOKEN_INIT SMC_RSI_FID(0x194)
+
+/*
+ * arg1 == The IPA of token buffer
+ * arg2 == Offset within the granule of the token buffer
+ * arg3 == Size of the granule buffer
+ * ret0 == Status / error
+ * ret1 == Length of token bytes copied to the granule buffer
+ */
+#define SMC_RSI_ATTESTATION_TOKEN_CONTINUE SMC_RSI_FID(0x195)
+
+/*
+ * arg1 == Index, which measurements slot to extend
+ * arg2 == Size of realm measurement in bytes, max 64 bytes
+ * arg3 == Measurement value, bytes: 0 - 7
+ * arg4 == Measurement value, bytes: 7 - 15
+ * arg5 == Measurement value, bytes: 16 - 23
+ * arg6 == Measurement value, bytes: 24 - 31
+ * arg7 == Measurement value, bytes: 32 - 39
+ * arg8 == Measurement value, bytes: 40 - 47
+ * arg9 == Measurement value, bytes: 48 - 55
+ * arg10 == Measurement value, bytes: 56 - 63
+ * ret0 == Status / error
+ */
+#define SMC_RSI_MEASUREMENT_EXTEND SMC_RSI_FID(0x193)
+
+/*
+ * arg1 == Index, which measurements slot to read
+ * ret0 == Status / error
+ * ret1 == Measurement value, bytes: 0 - 7
+ * ret2 == Measurement value, bytes: 7 - 15
+ * ret3 == Measurement value, bytes: 16 - 23
+ * ret4 == Measurement value, bytes: 24 - 31
+ * ret5 == Measurement value, bytes: 32 - 39
+ * ret6 == Measurement value, bytes: 40 - 47
+ * ret7 == Measurement value, bytes: 48 - 55
+ * ret8 == Measurement value, bytes: 56 - 63
+ */
+#define SMC_RSI_MEASUREMENT_READ SMC_RSI_FID(0x192)
+
+#ifndef __ASSEMBLY__
+
+struct realm_config {
+ union {
+ struct {
+ unsigned long ipa_bits; /* Width of IPA in bits */
+ unsigned long hash_algo; /* Hash algorithm */
+ };
+ u8 pad[0x1000];
+ };
+} __aligned(0x1000);
+
+#endif /* __ASSEMBLY__ */
+
+/*
+ * arg1 == struct realm_config addr
+ * ret0 == Status / error
+ */
+#define SMC_RSI_REALM_CONFIG SMC_RSI_FID(0x196)
+
+/*
+ * arg1 == Base IPA address of target region
+ * arg2 == Top of the region
+ * arg3 == RIPAS value
+ * arg4 == flags
+ * ret0 == Status / error
+ * ret1 == Top of modified IPA range
+ */
+#define SMC_RSI_IPA_STATE_SET SMC_RSI_FID(0x197)
+
+#define RSI_NO_CHANGE_DESTROYED 0
+#define RSI_CHANGE_DESTROYED 1
+
+/*
+ * arg1 == IPA of target page
+ * ret0 == Status / error
+ * ret1 == RIPAS value
+ */
+#define SMC_RSI_IPA_STATE_GET SMC_RSI_FID(0x198)
+
+/*
+ * arg1 == IPA of host call structure
+ * ret0 == Status / error
+ */
+#define SMC_RSI_HOST_CALL SMC_RSI_FID(0x199)
+
+#endif /* __SMC_RSI_H_ */
--
2.34.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v3 01/14] arm64: rsi: Add RSI definitions
2024-06-05 9:29 ` [PATCH v3 01/14] arm64: rsi: Add RSI definitions Steven Price
@ 2024-06-10 14:14 ` Catalin Marinas
0 siblings, 0 replies; 54+ messages in thread
From: Catalin Marinas @ 2024-06-10 14:14 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Suzuki K Poulose, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni
On Wed, Jun 05, 2024 at 10:29:53AM +0100, Steven Price wrote:
> --- /dev/null
> +++ b/arch/arm64/include/asm/rsi_cmds.h
> @@ -0,0 +1,47 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2023 ARM Ltd.
> + */
> +
> +#ifndef __ASM_RSI_CMDS_H
> +#define __ASM_RSI_CMDS_H
[...]
> --- /dev/null
> +++ b/arch/arm64/include/asm/rsi_smc.h
> @@ -0,0 +1,142 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * Copyright (C) 2023 ARM Ltd.
> + */
> +
> +#ifndef __SMC_RSI_H_
> +#define __SMC_RSI_H_
A small nitpick if you respin some patches - please make the header
guards consistent. We tend to to use the top variant above, so the
rsi_smc.h would be __ASM_RSI_SMC_H. The same throughout this series.
--
Catalin
^ permalink raw reply [flat|nested] 54+ messages in thread
* [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM
2024-06-05 9:29 [PATCH v3 00/14] arm64: Support for running as a guest in Arm CCA Steven Price
2024-06-05 8:37 ` Itaru Kitayama
2024-06-05 9:29 ` [PATCH v3 01/14] arm64: rsi: Add RSI definitions Steven Price
@ 2024-06-05 9:29 ` Steven Price
2024-06-10 14:11 ` Catalin Marinas
` (2 more replies)
2024-06-05 9:29 ` [PATCH v3 03/14] arm64: realm: Query IPA size from the RMM Steven Price
` (12 subsequent siblings)
15 siblings, 3 replies; 54+ messages in thread
From: Steven Price @ 2024-06-05 9:29 UTC (permalink / raw)
To: kvm, kvmarm
Cc: Suzuki K Poulose, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Steven Price
From: Suzuki K Poulose <suzuki.poulose@arm.com>
Detect that the VM is a realm guest by the presence of the RSI
interface.
If in a realm then all memory needs to be marked as RIPAS RAM initially,
the loader may or may not have done this for us. To be sure iterate over
all RAM and mark it as such. Any failure is fatal as that implies the
RAM regions passed to Linux are incorrect - which would mean failing
later when attempting to access non-existent RAM.
Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Co-developed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
Changes since v2:
* Use DECLARE_STATIC_KEY_FALSE rather than "extern struct
static_key_false".
* Rename set_memory_range() to rsi_set_memory_range().
* Downgrade some BUG()s to WARN()s and handle the condition by
propagating up the stack. Comment the remaining case that ends in a
BUG() to explain why.
* Rely on the return from rsi_request_version() rather than checking
the version the RMM claims to support.
* Rename the generic sounding arm64_setup_memory() to
arm64_rsi_setup_memory() and move the call site to setup_arch().
---
arch/arm64/include/asm/rsi.h | 48 +++++++++++++++++++++
arch/arm64/include/asm/rsi_cmds.h | 22 ++++++++++
arch/arm64/kernel/Makefile | 3 +-
arch/arm64/kernel/rsi.c | 69 +++++++++++++++++++++++++++++++
arch/arm64/kernel/setup.c | 8 ++++
arch/arm64/mm/init.c | 1 +
6 files changed, 150 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/rsi.h
create mode 100644 arch/arm64/kernel/rsi.c
diff --git a/arch/arm64/include/asm/rsi.h b/arch/arm64/include/asm/rsi.h
new file mode 100644
index 000000000000..ce2cdb501d84
--- /dev/null
+++ b/arch/arm64/include/asm/rsi.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 ARM Ltd.
+ */
+
+#ifndef __ASM_RSI_H_
+#define __ASM_RSI_H_
+
+#include <linux/jump_label.h>
+#include <asm/rsi_cmds.h>
+
+DECLARE_STATIC_KEY_FALSE(rsi_present);
+
+void __init arm64_rsi_init(void);
+void __init arm64_rsi_setup_memory(void);
+static inline bool is_realm_world(void)
+{
+ return static_branch_unlikely(&rsi_present);
+}
+
+static inline int rsi_set_memory_range(phys_addr_t start, phys_addr_t end,
+ enum ripas state)
+{
+ unsigned long ret;
+ phys_addr_t top;
+
+ while (start != end) {
+ ret = rsi_set_addr_range_state(start, end, state, &top);
+ if (WARN_ON(ret || top < start || top > end))
+ return -EINVAL;
+ start = top;
+ }
+
+ return 0;
+}
+
+static inline int rsi_set_memory_range_protected(phys_addr_t start,
+ phys_addr_t end)
+{
+ return rsi_set_memory_range(start, end, RSI_RIPAS_RAM);
+}
+
+static inline int rsi_set_memory_range_shared(phys_addr_t start,
+ phys_addr_t end)
+{
+ return rsi_set_memory_range(start, end, RSI_RIPAS_EMPTY);
+}
+#endif
diff --git a/arch/arm64/include/asm/rsi_cmds.h b/arch/arm64/include/asm/rsi_cmds.h
index ad425c5d6f1b..ab8ad435f10e 100644
--- a/arch/arm64/include/asm/rsi_cmds.h
+++ b/arch/arm64/include/asm/rsi_cmds.h
@@ -10,6 +10,11 @@
#include <asm/rsi_smc.h>
+enum ripas {
+ RSI_RIPAS_EMPTY,
+ RSI_RIPAS_RAM,
+};
+
static inline void invoke_rsi_fn_smc_with_res(unsigned long function_id,
unsigned long arg0,
unsigned long arg1,
@@ -44,4 +49,21 @@ static inline unsigned long rsi_get_realm_config(struct realm_config *cfg)
return res.a0;
}
+static inline unsigned long rsi_set_addr_range_state(phys_addr_t start,
+ phys_addr_t end,
+ enum ripas state,
+ phys_addr_t *top)
+{
+ struct arm_smccc_res res;
+
+ invoke_rsi_fn_smc_with_res(SMC_RSI_IPA_STATE_SET,
+ start, end, state, RSI_NO_CHANGE_DESTROYED,
+ &res);
+
+ if (top)
+ *top = res.a1;
+
+ return res.a0;
+}
+
#endif
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 763824963ed1..a483b916ed11 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -33,7 +33,8 @@ obj-y := debug-monitors.o entry.o irq.o fpsimd.o \
return_address.o cpuinfo.o cpu_errata.o \
cpufeature.o alternative.o cacheinfo.o \
smp.o smp_spin_table.o topology.o smccc-call.o \
- syscall.o proton-pack.o idle.o patching.o pi/
+ syscall.o proton-pack.o idle.o patching.o pi/ \
+ rsi.o
obj-$(CONFIG_COMPAT) += sys32.o signal32.o \
sys_compat.o
diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
new file mode 100644
index 000000000000..3a992bdfd6bb
--- /dev/null
+++ b/arch/arm64/kernel/rsi.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2023 ARM Ltd.
+ */
+
+#include <linux/jump_label.h>
+#include <linux/memblock.h>
+#include <asm/rsi.h>
+
+DEFINE_STATIC_KEY_FALSE_RO(rsi_present);
+EXPORT_SYMBOL(rsi_present);
+
+static bool rsi_version_matches(void)
+{
+ unsigned long ver_lower, ver_higher;
+ unsigned long ret = rsi_request_version(RSI_ABI_VERSION,
+ &ver_lower,
+ &ver_higher);
+
+ if (ret == SMCCC_RET_NOT_SUPPORTED)
+ return false;
+
+ if (ret != RSI_SUCCESS) {
+ pr_err("RME: RMM doesn't support RSI version %u.%u. Supported range: %lu.%lu-%lu.%lu\n",
+ RSI_ABI_VERSION_MAJOR, RSI_ABI_VERSION_MINOR,
+ RSI_ABI_VERSION_GET_MAJOR(ver_lower),
+ RSI_ABI_VERSION_GET_MINOR(ver_lower),
+ RSI_ABI_VERSION_GET_MAJOR(ver_higher),
+ RSI_ABI_VERSION_GET_MINOR(ver_higher));
+ return false;
+ }
+
+ pr_info("RME: Using RSI version %lu.%lu\n",
+ RSI_ABI_VERSION_GET_MAJOR(ver_lower),
+ RSI_ABI_VERSION_GET_MINOR(ver_lower));
+
+ return true;
+}
+
+void __init arm64_rsi_setup_memory(void)
+{
+ u64 i;
+ phys_addr_t start, end;
+
+ if (!is_realm_world())
+ return;
+
+ /*
+ * Iterate over the available memory ranges
+ * and convert the state to protected memory.
+ *
+ * BUG_ON is used because if the attempt to switch the memory to
+ * protected has failed here, then future accesses to the memory are
+ * simply going to be reflected as a fault which we can't handle.
+ * Bailing out early prevents the guest limping on and dieing later.
+ */
+ for_each_mem_range(i, &start, &end) {
+ BUG_ON(rsi_set_memory_range_protected(start, end));
+ }
+}
+
+void __init arm64_rsi_init(void)
+{
+ if (!rsi_version_matches())
+ return;
+
+ static_branch_enable(&rsi_present);
+}
+
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index a096e2451044..143f87615af0 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -43,6 +43,7 @@
#include <asm/cpu_ops.h>
#include <asm/kasan.h>
#include <asm/numa.h>
+#include <asm/rsi.h>
#include <asm/scs.h>
#include <asm/sections.h>
#include <asm/setup.h>
@@ -293,6 +294,11 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
* cpufeature code and early parameters.
*/
jump_label_init();
+ /*
+ * Init RSI before early param so that "earlycon" console uses the
+ * shared alias when in a realm
+ */
+ arm64_rsi_init();
parse_early_param();
dynamic_scs_init();
@@ -328,6 +334,8 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
arm64_memblock_init();
+ arm64_rsi_setup_memory();
+
paging_init();
acpi_table_upgrade();
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index 9b5ab6818f7f..9d8d38e3bee2 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -41,6 +41,7 @@
#include <asm/kvm_host.h>
#include <asm/memory.h>
#include <asm/numa.h>
+#include <asm/rsi.h>
#include <asm/sections.h>
#include <asm/setup.h>
#include <linux/sizes.h>
--
2.34.1
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM
2024-06-05 9:29 ` [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM Steven Price
@ 2024-06-10 14:11 ` Catalin Marinas
2024-06-10 14:16 ` Steven Price
2024-06-12 10:40 ` Jean-Philippe Brucker
2024-06-14 18:57 ` Suzuki K Poulose
2 siblings, 1 reply; 54+ messages in thread
From: Catalin Marinas @ 2024-06-10 14:11 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Suzuki K Poulose, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni
On Wed, Jun 05, 2024 at 10:29:54AM +0100, Steven Price wrote:
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 9b5ab6818f7f..9d8d38e3bee2 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -41,6 +41,7 @@
> #include <asm/kvm_host.h>
> #include <asm/memory.h>
> #include <asm/numa.h>
> +#include <asm/rsi.h>
> #include <asm/sections.h>
> #include <asm/setup.h>
> #include <linux/sizes.h>
What's this random include here? It looks like a leftover from the
previous version.
--
Catalin
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM
2024-06-10 14:11 ` Catalin Marinas
@ 2024-06-10 14:16 ` Steven Price
0 siblings, 0 replies; 54+ messages in thread
From: Steven Price @ 2024-06-10 14:16 UTC (permalink / raw)
To: Catalin Marinas
Cc: kvm, kvmarm, Suzuki K Poulose, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni
On 10/06/2024 15:11, Catalin Marinas wrote:
> On Wed, Jun 05, 2024 at 10:29:54AM +0100, Steven Price wrote:
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 9b5ab6818f7f..9d8d38e3bee2 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -41,6 +41,7 @@
>> #include <asm/kvm_host.h>
>> #include <asm/memory.h>
>> #include <asm/numa.h>
>> +#include <asm/rsi.h>
>> #include <asm/sections.h>
>> #include <asm/setup.h>
>> #include <linux/sizes.h>
>
> What's this random include here? It looks like a leftover from the
> previous version.
>
Whoops - indeed that shouldn't be there.
Thanks,
Steve
^ permalink raw reply [flat|nested] 54+ messages in thread
* Re: [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM
2024-06-05 9:29 ` [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM Steven Price
2024-06-10 14:11 ` Catalin Marinas
@ 2024-06-12 10:40 ` Jean-Philippe Brucker
2024-06-12 10:59 ` Suzuki K Poulose
2024-06-26 0:12 ` Jeremy Linton
2024-06-14 18:57 ` Suzuki K Poulose
2 siblings, 2 replies; 54+ messages in thread
From: Jean-Philippe Brucker @ 2024-06-12 10:40 UTC (permalink / raw)
To: Steven Price
Cc: kvm, kvmarm, Suzuki K Poulose, Catalin Marinas, Marc Zyngier,
Will Deacon, James Morse, Oliver Upton, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni
[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]
On Wed, Jun 05, 2024 at 10:29:54AM +0100, Steven Price wrote:
> From: Suzuki K Poulose <suzuki.poulose@arm.com>
>
> Detect that the VM is a realm guest by the presence of the RSI
> interface.
>
> If in a realm then all memory needs to be marked as RIPAS RAM initially,
> the loader may or may not have done this for us. To be sure iterate over
> all RAM and mark it as such. Any failure is fatal as that implies the
> RAM regions passed to Linux are incorrect - which would mean failing
> later when attempting to access non-existent RAM.
>
> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Co-developed-by: Steven Price <steven.price@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> +static bool rsi_version_matches(void)
> +{
> + unsigned long ver_lower, ver_higher;
> + unsigned long ret = rsi_request_version(RSI_ABI_VERSION,
> + &ver_lower,
> + &ver_higher);
There is a regression on QEMU TCG (in emulation mode, not running under KVM):
qemu-system-aarch64 -M virt -cpu max -kernel Image -nographic
This doesn't implement EL3 or EL2, so SMC is UNDEFINED (DDI0487J.a R_HMXQS),
and we end up with an undef instruction exception. So this patch would
also break hardware that only implements EL1 (I don't know if it exists).
The easiest fix is to detect the SMC conduit through the PSCI node in DT.
SMCCC helpers already do this, but we can't use them this early in the
boot. I tested adding an early probe to the PSCI driver to check this, see
attached patches.
Note that we do need to test the conduit after finding a PSCI node,
because even though it doesn't implement EL2 in this configuration, QEMU
still accepts PSCI HVCs in order to support SMP.
Thanks,
Jean
[-- Attachment #2: 0001-firmware-psci-Add-psci_early_test_conduit.patch --]
[-- Type: text/plain, Size: 2589 bytes --]
From 788bfd45e7ce521666a19dba99277106e4d33c80 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
Date: Tue, 11 Jun 2024 19:15:30 +0100
Subject: [PATCH 1/2] firmware/psci: Add psci_early_test_conduit()
Add a function to test early if PSCI is present and what conduit it
uses. Because the PSCI conduit corresponds to the SMCCC one, this will
let the kernel know whether it can use SMC instructions to discuss with
the Realm Management Monitor (RMM), early enough to enable RAM and
serial access when running in a Realm.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
include/linux/psci.h | 5 +++++
drivers/firmware/psci/psci.c | 25 +++++++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/include/linux/psci.h b/include/linux/psci.h
index 4ca0060a3fc4..a1fc1703ba20 100644
--- a/include/linux/psci.h
+++ b/include/linux/psci.h
@@ -45,8 +45,13 @@ struct psci_0_1_function_ids get_psci_0_1_function_ids(void);
#if defined(CONFIG_ARM_PSCI_FW)
int __init psci_dt_init(void);
+bool __init psci_early_test_conduit(enum arm_smccc_conduit conduit);
#else
static inline int psci_dt_init(void) { return 0; }
+static inline bool psci_early_test_conduit(enum arm_smccc_conduit conduit)
+{
+ return false;
+}
#endif
#if defined(CONFIG_ARM_PSCI_FW) && defined(CONFIG_ACPI)
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index d9629ff87861..a40dcaf17822 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -13,6 +13,7 @@
#include <linux/errno.h>
#include <linux/linkage.h>
#include <linux/of.h>
+#include <linux/of_fdt.h>
#include <linux/pm.h>
#include <linux/printk.h>
#include <linux/psci.h>
@@ -767,6 +768,30 @@ int __init psci_dt_init(void)
return ret;
}
+/*
+ * Test early if PSCI is supported, and if its conduit matches @conduit
+ */
+bool __init psci_early_test_conduit(enum arm_smccc_conduit conduit)
+{
+ int len;
+ int psci_node;
+ const char *method;
+ unsigned long dt_root;
+
+ /* DT hasn't been unflattened yet, we have to work with the flat blob */
+ dt_root = of_get_flat_dt_root();
+ psci_node = of_get_flat_dt_subnode_by_name(dt_root, "psci");
+ if (psci_node <= 0)
+ return false;
+
+ method = of_get_flat_dt_prop(psci_node, "method", &len);
+ if (!method)
+ return false;
+
+ return (conduit == SMCCC_CONDUIT_SMC && strncmp(method, "smc", len) == 0) ||
+ (conduit == SMCCC_CONDUIT_HVC && strncmp(method, "hvc", len) == 0);
+}
+
#ifdef CONFIG_ACPI
/*
* We use PSCI 0.2+ when ACPI is deployed on ARM64 and it's
--
2.45.2
[-- Attachment #3: 0002-squash-arm64-Detect-if-in-a-realm-and-set-RIPAS-RAM.patch --]
[-- Type: text/plain, Size: 1700 bytes --]
From fcb16e1eb494d2ce21792495955d6d3f26c319c9 Mon Sep 17 00:00:00 2001
From: Jean-Philippe Brucker <jean-philippe@linaro.org>
Date: Wed, 12 Jun 2024 09:08:17 +0100
Subject: [PATCH 2/2] squash! arm64: Detect if in a realm and set RIPAS RAM
Before issuing an SMC, detect whether SMCCC is available through this
conduit. On platforms that do not implement EL3 nor EL2, the SMC
instruction is UNDEFINED. SMCCC advises probing the SMCCC availability
by first looking for a PSCI node in DT (or in ACPI, but we expect a
realm to boot with DT at the moment). Since RMM requires using the SMC
conduit for both PSCI and RSI, we rely on the PSCI method property to
ensure SMC is available. We could also check that the SMCCC version is
at least 1.2 as required by RMM, but no platform requires this extra
step at the moment, and we can't use the SMCCC helpers this early in
boot.
Signed-off-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
---
arch/arm64/kernel/rsi.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/kernel/rsi.c b/arch/arm64/kernel/rsi.c
index 898952d135b0..21fc261a1d26 100644
--- a/arch/arm64/kernel/rsi.c
+++ b/arch/arm64/kernel/rsi.c
@@ -7,6 +7,7 @@
#include <linux/memblock.h>
#include <linux/swiotlb.h>
#include <linux/cc_platform.h>
+#include <linux/psci.h>
#include <asm/rsi.h>
@@ -82,6 +83,12 @@ void __init arm64_rsi_setup_memory(void)
void __init arm64_rsi_init(void)
{
+ /*
+ * If PSCI isn't using SMC, RMM isn't present. Don't try to execute an
+ * SMC as it could be UNDEFINED.
+ */
+ if (!psci_early_test_conduit(SMCCC_CONDUIT_SMC))
+ return;
if (!rsi_version_matches())
return;
if (rsi_get_realm_config(&config))
--
2.45.2
^ permalink raw reply related [flat|nested] 54+ messages in thread* Re: [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM
2024-06-12 10:40 ` Jean-Philippe Brucker
@ 2024-06-12 10:59 ` Suzuki K Poulose
2024-06-13 10:51 ` Jean-Philippe Brucker
2024-06-26 0:12 ` Jeremy Linton
1 sibling, 1 reply; 54+ messages in thread
From: Suzuki K Poulose @ 2024-06-12 10:59 UTC (permalink / raw)
To: Jean-Philippe Brucker, Steven Price
Cc: kvm, kvmarm, Catalin Marinas, Marc Zyngier, Will Deacon,
James Morse, Oliver Upton, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni
On 12/06/2024 11:40, Jean-Philippe Brucker wrote:
> On Wed, Jun 05, 2024 at 10:29:54AM +0100, Steven Price wrote:
>> From: Suzuki K Poulose <suzuki.poulose@arm.com>
>>
>> Detect that the VM is a realm guest by the presence of the RSI
>> interface.
>>
>> If in a realm then all memory needs to be marked as RIPAS RAM initially,
>> the loader may or may not have done this for us. To be sure iterate over
>> all RAM and mark it as such. Any failure is fatal as that implies the
>> RAM regions passed to Linux are incorrect - which would mean failing
>> later when attempting to access non-existent RAM.
>>
>> Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Co-developed-by: Steven Price <steven.price@arm.com>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>
>> +static bool rsi_version_matches(void)
>> +{
>> + unsigned long ver_lower, ver_higher;
>> + unsigned long ret = rsi_request_version(RSI_ABI_VERSION,
>> + &ver_lower,
>> + &ver_higher);
>
> There is a regression on QEMU TCG (in emulation mode, not running under KVM):
>
> qemu-system-aarch64 -M virt -cpu max -kernel Image -nographic
>
> This doesn't implement EL3 or EL2, so SMC is UNDEFINED (DDI0487J.a R_HMXQS),
> and we end up with an undef instruction exception. So this patch would
> also break hardware that only implements EL1 (I don't know if it exists).
Thanks for the report, Could we not check ID_AA64PFR0_EL1.EL3 >= 0 ? I
think we do this for kvm-unit-tests, we need the same here.
Suzuki
>
> The easiest fix is to detect the SMC conduit through the PSCI node in DT.
> SMCCC helpers already do this, but we can't use them this early in the
> boot. I tested adding an early probe to the PSCI driver to check this, see
> attached patches.
>
> Note that we do need to test the conduit after finding a PSCI node,
> because even though it doesn't implement EL2 in this configuration, QEMU
> still accepts PSCI HVCs in order to support SMP.
>
> Thanks,
> Jean
>
^ permalink raw reply [flat|nested] 54+ messages in thread* Re: [PATCH v3 02/14] arm64: Detect if in a realm and set RIPAS RAM
2024-06-12 10:59 ` Suzuki K Poulose
@ 2024-06-13 10:51 ` Jean-Philippe Brucker
2024-06-17 10:27 ` Peter Maydell
0 siblings, 1 reply; 54+ messages in thread
From: Jean-Philippe Brucker @ 2024-06-13 10:51 UTC (permalink / raw)
To: Suzuki K Poulose
Cc: Steven Price, kvm, kvmarm, Catalin Marinas, Marc Zyngier,
Will Deacon, James Morse, Oliver Upton, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
peter.maydell
On Wed, Jun 12, 2024 at 11:59:22AM +0100, Suzuki K Poulose wrote:
> On 12/06/2024 11:40, Jean-Philippe Brucker wrote:
> > On Wed, Jun 05, 2024 at 10:29:54AM +0100, Steven Price wrote:
> > > From: Suzuki K Poulose <suzuki.poulose@arm.com>
> > >
> > > Detect that the VM is a realm guest by the presence of the RSI
> > > interface.
> > >
> > > If in a realm then all memory needs to be marked as RIPAS RAM initially,
> > > the loader may or may not have done this for us. To be sure iterate over
> > > all RAM and mark it as such. Any failure is fatal as that implies the
> > > RAM regions passed to Linux are incorrect - which would mean failing
> > > later when attempting to access non-existent RAM.
> > >
> > > Signed-off-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> > > Co-developed-by: Steven Price <steven.price@arm.com>
> > > Signed-off-by: Steven Price <steven.price@arm.com>
> >
> > > +static bool rsi_version_matches(void)
> > > +{
> > > + unsigned long ver_lower, ver_higher;
> > > + unsigned long ret = rsi_request_version(RSI_ABI_VERSION,
> > > + &ver_lower,
> > > + &ver_higher);
> >
> > There is a regression on QEMU TCG (in emulation mode, not running under KVM):
> >
> > qemu-system-aarch64 -M virt -cpu max -kernel Image -nographic
> >
> > This doesn't implement EL3 or EL2, so SMC is UNDEFINED (DDI0487J.a R_HMXQS),
> > and we end up with an undef instruction exception. So this patch would
> > also break hardware that only implements EL1 (I don't know if it exists).
>
> Thanks for the report, Could we not check ID_AA64PFR0_EL1.EL3 >= 0 ? I
> think we do this for kvm-unit-tests, we need the same here.
Good point, it also fixes this case and is simpler. It assumes RMM doesn't
hide this field, but I can't think of a reason it would.
This command won't work anymore:
qemu-system-aarch64 -M virt,secure=on -cpu max -kernel Image -nographic
implements EL3 and SMC still treated as undef. QEMU has a special case for
starting at EL2 in this case, but I couldn't find what this is for.
Treating SMC as undef is correct because SCR_EL3.SMD resets to an
architectutally UNKNOWN value. But the architecture requires that the CPU
resets to the highest implemented exception level (DDI0487J.a R_JYLQV). So
in my opinion we can use the ID_AA64PFR0_EL1.EL3 field here, and breaking
this particular configuration is not a problem: users shouldn't expect
Linux to boot when EL3 is implemented and doesn't run a firmware.
Thanks,
Jean
>
>
> Suzuki
>
> >
> > The easiest fix is to detect the SMC conduit through the PSCI node in DT.
> > SMCCC helpers already do this, but we can't use them this early in the
> > boot. I tested adding an early probe to the PSCI driver to check this, see
> > attached patches.
> >
> > Note that we do need to test the conduit after finding a PSCI node,
> > because even though it doesn't implement EL2 in this configuration, QEMU
> > still accepts PSCI HVCs in order to support SMP.
> >
> > Thanks,
> > Jean
> >
>
^ permalink raw reply [flat|nested] 54+ messages in thread