[PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM

Steven Price posted 43 patches 5 months, 3 weeks ago
There is a newer version of this series
[PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM
Posted by Steven Price 5 months, 3 weeks ago
The RMM (Realm Management Monitor) provides functionality that can be
accessed by SMC calls from the host.

The SMC definitions are based on DEN0137[1] version 1.0-rel0

[1] https://developer.arm.com/documentation/den0137/1-0rel0/

Reviewed-by: Gavin Shan <gshan@redhat.com>
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
Signed-off-by: Steven Price <steven.price@arm.com>
---
Changes since v9:
 * Corrected size of 'ripas_value' in struct rec_exit. The spec states
   this is an 8-bit type with padding afterwards (rather than a u64).
Changes since v8:
 * Added RMI_PERMITTED_GICV3_HCR_BITS to define which bits the RMM
   permits to be modified.
Changes since v6:
 * Renamed REC_ENTER_xxx defines to include 'FLAG' to make it obvious
   these are flag values.
Changes since v5:
 * Sorted the SMC #defines by value.
 * Renamed SMI_RxI_CALL to SMI_RMI_CALL since the macro is only used for
   RMI calls.
 * Renamed REC_GIC_NUM_LRS to REC_MAX_GIC_NUM_LRS since the actual
   number of available list registers could be lower.
 * Provided a define for the reserved fields of FeatureRegister0.
 * Fix inconsistent names for padding fields.
Changes since v4:
 * Update to point to final released RMM spec.
 * Minor rearrangements.
Changes since v3:
 * Update to match RMM spec v1.0-rel0-rc1.
Changes since v2:
 * Fix specification link.
 * Rename rec_entry->rec_enter to match spec.
 * Fix size of pmu_ovf_status to match spec.
---
 arch/arm64/include/asm/rmi_smc.h | 269 +++++++++++++++++++++++++++++++
 1 file changed, 269 insertions(+)
 create mode 100644 arch/arm64/include/asm/rmi_smc.h

diff --git a/arch/arm64/include/asm/rmi_smc.h b/arch/arm64/include/asm/rmi_smc.h
new file mode 100644
index 000000000000..1000368f1bca
--- /dev/null
+++ b/arch/arm64/include/asm/rmi_smc.h
@@ -0,0 +1,269 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2023-2024 ARM Ltd.
+ *
+ * The values and structures in this file are from the Realm Management Monitor
+ * specification (DEN0137) version 1.0-rel0:
+ * https://developer.arm.com/documentation/den0137/1-0rel0/
+ */
+
+#ifndef __ASM_RMI_SMC_H
+#define __ASM_RMI_SMC_H
+
+#include <linux/arm-smccc.h>
+
+#define SMC_RMI_CALL(func)				\
+	ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL,		\
+			   ARM_SMCCC_SMC_64,		\
+			   ARM_SMCCC_OWNER_STANDARD,	\
+			   (func))
+
+#define SMC_RMI_VERSION			SMC_RMI_CALL(0x0150)
+#define SMC_RMI_GRANULE_DELEGATE	SMC_RMI_CALL(0x0151)
+#define SMC_RMI_GRANULE_UNDELEGATE	SMC_RMI_CALL(0x0152)
+#define SMC_RMI_DATA_CREATE		SMC_RMI_CALL(0x0153)
+#define SMC_RMI_DATA_CREATE_UNKNOWN	SMC_RMI_CALL(0x0154)
+#define SMC_RMI_DATA_DESTROY		SMC_RMI_CALL(0x0155)
+
+#define SMC_RMI_REALM_ACTIVATE		SMC_RMI_CALL(0x0157)
+#define SMC_RMI_REALM_CREATE		SMC_RMI_CALL(0x0158)
+#define SMC_RMI_REALM_DESTROY		SMC_RMI_CALL(0x0159)
+#define SMC_RMI_REC_CREATE		SMC_RMI_CALL(0x015a)
+#define SMC_RMI_REC_DESTROY		SMC_RMI_CALL(0x015b)
+#define SMC_RMI_REC_ENTER		SMC_RMI_CALL(0x015c)
+#define SMC_RMI_RTT_CREATE		SMC_RMI_CALL(0x015d)
+#define SMC_RMI_RTT_DESTROY		SMC_RMI_CALL(0x015e)
+#define SMC_RMI_RTT_MAP_UNPROTECTED	SMC_RMI_CALL(0x015f)
+
+#define SMC_RMI_RTT_READ_ENTRY		SMC_RMI_CALL(0x0161)
+#define SMC_RMI_RTT_UNMAP_UNPROTECTED	SMC_RMI_CALL(0x0162)
+
+#define SMC_RMI_PSCI_COMPLETE		SMC_RMI_CALL(0x0164)
+#define SMC_RMI_FEATURES		SMC_RMI_CALL(0x0165)
+#define SMC_RMI_RTT_FOLD		SMC_RMI_CALL(0x0166)
+#define SMC_RMI_REC_AUX_COUNT		SMC_RMI_CALL(0x0167)
+#define SMC_RMI_RTT_INIT_RIPAS		SMC_RMI_CALL(0x0168)
+#define SMC_RMI_RTT_SET_RIPAS		SMC_RMI_CALL(0x0169)
+
+#define RMI_ABI_MAJOR_VERSION	1
+#define RMI_ABI_MINOR_VERSION	0
+
+#define RMI_ABI_VERSION_GET_MAJOR(version) ((version) >> 16)
+#define RMI_ABI_VERSION_GET_MINOR(version) ((version) & 0xFFFF)
+#define RMI_ABI_VERSION(major, minor)      (((major) << 16) | (minor))
+
+#define RMI_UNASSIGNED			0
+#define RMI_ASSIGNED			1
+#define RMI_TABLE			2
+
+#define RMI_RETURN_STATUS(ret)		((ret) & 0xFF)
+#define RMI_RETURN_INDEX(ret)		(((ret) >> 8) & 0xFF)
+
+#define RMI_SUCCESS		0
+#define RMI_ERROR_INPUT		1
+#define RMI_ERROR_REALM		2
+#define RMI_ERROR_REC		3
+#define RMI_ERROR_RTT		4
+
+enum rmi_ripas {
+	RMI_EMPTY = 0,
+	RMI_RAM = 1,
+	RMI_DESTROYED = 2,
+};
+
+#define RMI_NO_MEASURE_CONTENT	0
+#define RMI_MEASURE_CONTENT	1
+
+#define RMI_FEATURE_REGISTER_0_S2SZ		GENMASK(7, 0)
+#define RMI_FEATURE_REGISTER_0_LPA2		BIT(8)
+#define RMI_FEATURE_REGISTER_0_SVE_EN		BIT(9)
+#define RMI_FEATURE_REGISTER_0_SVE_VL		GENMASK(13, 10)
+#define RMI_FEATURE_REGISTER_0_NUM_BPS		GENMASK(19, 14)
+#define RMI_FEATURE_REGISTER_0_NUM_WPS		GENMASK(25, 20)
+#define RMI_FEATURE_REGISTER_0_PMU_EN		BIT(26)
+#define RMI_FEATURE_REGISTER_0_PMU_NUM_CTRS	GENMASK(31, 27)
+#define RMI_FEATURE_REGISTER_0_HASH_SHA_256	BIT(32)
+#define RMI_FEATURE_REGISTER_0_HASH_SHA_512	BIT(33)
+#define RMI_FEATURE_REGISTER_0_GICV3_NUM_LRS	GENMASK(37, 34)
+#define RMI_FEATURE_REGISTER_0_MAX_RECS_ORDER	GENMASK(41, 38)
+#define RMI_FEATURE_REGISTER_0_Reserved		GENMASK(63, 42)
+
+#define RMI_REALM_PARAM_FLAG_LPA2		BIT(0)
+#define RMI_REALM_PARAM_FLAG_SVE		BIT(1)
+#define RMI_REALM_PARAM_FLAG_PMU		BIT(2)
+
+/*
+ * Note many of these fields are smaller than u64 but all fields have u64
+ * alignment, so use u64 to ensure correct alignment.
+ */
+struct realm_params {
+	union { /* 0x0 */
+		struct {
+			u64 flags;
+			u64 s2sz;
+			u64 sve_vl;
+			u64 num_bps;
+			u64 num_wps;
+			u64 pmu_num_ctrs;
+			u64 hash_algo;
+		};
+		u8 padding0[0x400];
+	};
+	union { /* 0x400 */
+		u8 rpv[64];
+		u8 padding1[0x400];
+	};
+	union { /* 0x800 */
+		struct {
+			u64 vmid;
+			u64 rtt_base;
+			s64 rtt_level_start;
+			u64 rtt_num_start;
+		};
+		u8 padding2[0x800];
+	};
+};
+
+/*
+ * The number of GPRs (starting from X0) that are
+ * configured by the host when a REC is created.
+ */
+#define REC_CREATE_NR_GPRS		8
+
+#define REC_PARAMS_FLAG_RUNNABLE	BIT_ULL(0)
+
+#define REC_PARAMS_AUX_GRANULES		16
+
+struct rec_params {
+	union { /* 0x0 */
+		u64 flags;
+		u8 padding0[0x100];
+	};
+	union { /* 0x100 */
+		u64 mpidr;
+		u8 padding1[0x100];
+	};
+	union { /* 0x200 */
+		u64 pc;
+		u8 padding2[0x100];
+	};
+	union { /* 0x300 */
+		u64 gprs[REC_CREATE_NR_GPRS];
+		u8 padding3[0x500];
+	};
+	union { /* 0x800 */
+		struct {
+			u64 num_rec_aux;
+			u64 aux[REC_PARAMS_AUX_GRANULES];
+		};
+		u8 padding4[0x800];
+	};
+};
+
+#define REC_ENTER_FLAG_EMULATED_MMIO	BIT(0)
+#define REC_ENTER_FLAG_INJECT_SEA	BIT(1)
+#define REC_ENTER_FLAG_TRAP_WFI		BIT(2)
+#define REC_ENTER_FLAG_TRAP_WFE		BIT(3)
+#define REC_ENTER_FLAG_RIPAS_RESPONSE	BIT(4)
+
+#define REC_RUN_GPRS			31
+#define REC_MAX_GIC_NUM_LRS		16
+
+#define RMI_PERMITTED_GICV3_HCR_BITS	(ICH_HCR_EL2_UIE |		\
+					 ICH_HCR_EL2_LRENPIE |		\
+					 ICH_HCR_EL2_NPIE |		\
+					 ICH_HCR_EL2_VGrp0EIE |		\
+					 ICH_HCR_EL2_VGrp0DIE |		\
+					 ICH_HCR_EL2_VGrp1EIE |		\
+					 ICH_HCR_EL2_VGrp1DIE |		\
+					 ICH_HCR_EL2_TDIR)
+
+struct rec_enter {
+	union { /* 0x000 */
+		u64 flags;
+		u8 padding0[0x200];
+	};
+	union { /* 0x200 */
+		u64 gprs[REC_RUN_GPRS];
+		u8 padding1[0x100];
+	};
+	union { /* 0x300 */
+		struct {
+			u64 gicv3_hcr;
+			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
+		};
+		u8 padding2[0x100];
+	};
+	u8 padding3[0x400];
+};
+
+#define RMI_EXIT_SYNC			0x00
+#define RMI_EXIT_IRQ			0x01
+#define RMI_EXIT_FIQ			0x02
+#define RMI_EXIT_PSCI			0x03
+#define RMI_EXIT_RIPAS_CHANGE		0x04
+#define RMI_EXIT_HOST_CALL		0x05
+#define RMI_EXIT_SERROR			0x06
+
+struct rec_exit {
+	union { /* 0x000 */
+		u8 exit_reason;
+		u8 padding0[0x100];
+	};
+	union { /* 0x100 */
+		struct {
+			u64 esr;
+			u64 far;
+			u64 hpfar;
+		};
+		u8 padding1[0x100];
+	};
+	union { /* 0x200 */
+		u64 gprs[REC_RUN_GPRS];
+		u8 padding2[0x100];
+	};
+	union { /* 0x300 */
+		struct {
+			u64 gicv3_hcr;
+			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
+			u64 gicv3_misr;
+			u64 gicv3_vmcr;
+		};
+		u8 padding3[0x100];
+	};
+	union { /* 0x400 */
+		struct {
+			u64 cntp_ctl;
+			u64 cntp_cval;
+			u64 cntv_ctl;
+			u64 cntv_cval;
+		};
+		u8 padding4[0x100];
+	};
+	union { /* 0x500 */
+		struct {
+			u64 ripas_base;
+			u64 ripas_top;
+			u8 ripas_value;
+			u8 padding8[7];
+		};
+		u8 padding5[0x100];
+	};
+	union { /* 0x600 */
+		u16 imm;
+		u8 padding6[0x100];
+	};
+	union { /* 0x700 */
+		struct {
+			u8 pmu_ovf_status;
+		};
+		u8 padding7[0x100];
+	};
+};
+
+struct rec_run {
+	struct rec_enter enter;
+	struct rec_exit exit;
+};
+
+#endif /* __ASM_RMI_SMC_H */
-- 
2.43.0
Re: [PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM
Posted by Marc Zyngier 4 months, 1 week ago
On Wed, 20 Aug 2025 15:55:23 +0100,
Steven Price <steven.price@arm.com> wrote:
> 
> The RMM (Realm Management Monitor) provides functionality that can be
> accessed by SMC calls from the host.
> 
> The SMC definitions are based on DEN0137[1] version 1.0-rel0
> 
> [1] https://developer.arm.com/documentation/den0137/1-0rel0/
> 
> Reviewed-by: Gavin Shan <gshan@redhat.com>
> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v9:
>  * Corrected size of 'ripas_value' in struct rec_exit. The spec states
>    this is an 8-bit type with padding afterwards (rather than a u64).
> Changes since v8:
>  * Added RMI_PERMITTED_GICV3_HCR_BITS to define which bits the RMM
>    permits to be modified.
> Changes since v6:
>  * Renamed REC_ENTER_xxx defines to include 'FLAG' to make it obvious
>    these are flag values.
> Changes since v5:
>  * Sorted the SMC #defines by value.
>  * Renamed SMI_RxI_CALL to SMI_RMI_CALL since the macro is only used for
>    RMI calls.
>  * Renamed REC_GIC_NUM_LRS to REC_MAX_GIC_NUM_LRS since the actual
>    number of available list registers could be lower.
>  * Provided a define for the reserved fields of FeatureRegister0.
>  * Fix inconsistent names for padding fields.
> Changes since v4:
>  * Update to point to final released RMM spec.
>  * Minor rearrangements.
> Changes since v3:
>  * Update to match RMM spec v1.0-rel0-rc1.
> Changes since v2:
>  * Fix specification link.
>  * Rename rec_entry->rec_enter to match spec.
>  * Fix size of pmu_ovf_status to match spec.
> ---
>  arch/arm64/include/asm/rmi_smc.h | 269 +++++++++++++++++++++++++++++++
>  1 file changed, 269 insertions(+)
>  create mode 100644 arch/arm64/include/asm/rmi_smc.h
> 
> diff --git a/arch/arm64/include/asm/rmi_smc.h b/arch/arm64/include/asm/rmi_smc.h
> new file mode 100644
> index 000000000000..1000368f1bca
> --- /dev/null
> +++ b/arch/arm64/include/asm/rmi_smc.h

[...]

> +#define RMI_PERMITTED_GICV3_HCR_BITS	(ICH_HCR_EL2_UIE |		\
> +					 ICH_HCR_EL2_LRENPIE |		\
> +					 ICH_HCR_EL2_NPIE |		\
> +					 ICH_HCR_EL2_VGrp0EIE |		\
> +					 ICH_HCR_EL2_VGrp0DIE |		\
> +					 ICH_HCR_EL2_VGrp1EIE |		\
> +					 ICH_HCR_EL2_VGrp1DIE |		\
> +					 ICH_HCR_EL2_TDIR)

Why should KVM care about what bits the RMM wants to use? Also, why
should KVM be forbidden to use the TALL0, TALL1 and TC bits? If
interrupt delivery is the host's business, then the RMM has no
business interfering with the GIC programming.

> +
> +struct rec_enter {
> +	union { /* 0x000 */
> +		u64 flags;
> +		u8 padding0[0x200];
> +	};
> +	union { /* 0x200 */
> +		u64 gprs[REC_RUN_GPRS];
> +		u8 padding1[0x100];
> +	};
> +	union { /* 0x300 */
> +		struct {
> +			u64 gicv3_hcr;
> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
> +		};
> +		u8 padding2[0x100];
> +	};
> +	u8 padding3[0x400];
> +};
> +
> +#define RMI_EXIT_SYNC			0x00
> +#define RMI_EXIT_IRQ			0x01
> +#define RMI_EXIT_FIQ			0x02
> +#define RMI_EXIT_PSCI			0x03
> +#define RMI_EXIT_RIPAS_CHANGE		0x04
> +#define RMI_EXIT_HOST_CALL		0x05
> +#define RMI_EXIT_SERROR			0x06
> +
> +struct rec_exit {
> +	union { /* 0x000 */
> +		u8 exit_reason;
> +		u8 padding0[0x100];
> +	};
> +	union { /* 0x100 */
> +		struct {
> +			u64 esr;
> +			u64 far;
> +			u64 hpfar;
> +		};
> +		u8 padding1[0x100];
> +	};
> +	union { /* 0x200 */
> +		u64 gprs[REC_RUN_GPRS];
> +		u8 padding2[0x100];
> +	};
> +	union { /* 0x300 */
> +		struct {
> +			u64 gicv3_hcr;
> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
> +			u64 gicv3_misr;

Why do we care about ICH_MISR_EL2? Surely we get everything in the
registers themselves, right? I think this goes back to my question
above: why is the RMM getting in the way of ICH_*_EL2 accesses?

> +			u64 gicv3_vmcr;
> +		};

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM
Posted by Steven Price 4 months, 1 week ago
Hi Marc,

On 01/10/2025 11:05, Marc Zyngier wrote:
> On Wed, 20 Aug 2025 15:55:23 +0100,
> Steven Price <steven.price@arm.com> wrote:
>>
>> The RMM (Realm Management Monitor) provides functionality that can be
>> accessed by SMC calls from the host.
>>
>> The SMC definitions are based on DEN0137[1] version 1.0-rel0
>>
>> [1] https://developer.arm.com/documentation/den0137/1-0rel0/
>>
>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>> Signed-off-by: Steven Price <steven.price@arm.com>
>> ---
>> Changes since v9:
>>  * Corrected size of 'ripas_value' in struct rec_exit. The spec states
>>    this is an 8-bit type with padding afterwards (rather than a u64).
>> Changes since v8:
>>  * Added RMI_PERMITTED_GICV3_HCR_BITS to define which bits the RMM
>>    permits to be modified.
>> Changes since v6:
>>  * Renamed REC_ENTER_xxx defines to include 'FLAG' to make it obvious
>>    these are flag values.
>> Changes since v5:
>>  * Sorted the SMC #defines by value.
>>  * Renamed SMI_RxI_CALL to SMI_RMI_CALL since the macro is only used for
>>    RMI calls.
>>  * Renamed REC_GIC_NUM_LRS to REC_MAX_GIC_NUM_LRS since the actual
>>    number of available list registers could be lower.
>>  * Provided a define for the reserved fields of FeatureRegister0.
>>  * Fix inconsistent names for padding fields.
>> Changes since v4:
>>  * Update to point to final released RMM spec.
>>  * Minor rearrangements.
>> Changes since v3:
>>  * Update to match RMM spec v1.0-rel0-rc1.
>> Changes since v2:
>>  * Fix specification link.
>>  * Rename rec_entry->rec_enter to match spec.
>>  * Fix size of pmu_ovf_status to match spec.
>> ---
>>  arch/arm64/include/asm/rmi_smc.h | 269 +++++++++++++++++++++++++++++++
>>  1 file changed, 269 insertions(+)
>>  create mode 100644 arch/arm64/include/asm/rmi_smc.h
>>
>> diff --git a/arch/arm64/include/asm/rmi_smc.h b/arch/arm64/include/asm/rmi_smc.h
>> new file mode 100644
>> index 000000000000..1000368f1bca
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/rmi_smc.h
> 
> [...]
> 
>> +#define RMI_PERMITTED_GICV3_HCR_BITS	(ICH_HCR_EL2_UIE |		\
>> +					 ICH_HCR_EL2_LRENPIE |		\
>> +					 ICH_HCR_EL2_NPIE |		\
>> +					 ICH_HCR_EL2_VGrp0EIE |		\
>> +					 ICH_HCR_EL2_VGrp0DIE |		\
>> +					 ICH_HCR_EL2_VGrp1EIE |		\
>> +					 ICH_HCR_EL2_VGrp1DIE |		\
>> +					 ICH_HCR_EL2_TDIR)
> 
> Why should KVM care about what bits the RMM wants to use? Also, why
> should KVM be forbidden to use the TALL0, TALL1 and TC bits? If
> interrupt delivery is the host's business, then the RMM has no
> business interfering with the GIC programming.

The RMM receives the guest's GIC state in a field within the REC entry
structure (enter.gicv3_hcr). The RMM spec states that the above is the
list of fields that will be considered and that everything else must be
0[1]. So this is used to filter the configuration to make sure it's
valid for the RMM.

In terms of TALL0/TALL1/TC bits: these control trapping to EL2, and when
in a realm guest the RMM is EL2 - so it's up to the RMM to configure
these bits appropriately as it is the RMM which will have to deal with
the trap.

[1] RWVGFJ in the 1.0 spec from
https://developer.arm.com/documentation/den0137/latest

>> +
>> +struct rec_enter {
>> +	union { /* 0x000 */
>> +		u64 flags;
>> +		u8 padding0[0x200];
>> +	};
>> +	union { /* 0x200 */
>> +		u64 gprs[REC_RUN_GPRS];
>> +		u8 padding1[0x100];
>> +	};
>> +	union { /* 0x300 */
>> +		struct {
>> +			u64 gicv3_hcr;
>> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
>> +		};
>> +		u8 padding2[0x100];
>> +	};
>> +	u8 padding3[0x400];
>> +};
>> +
>> +#define RMI_EXIT_SYNC			0x00
>> +#define RMI_EXIT_IRQ			0x01
>> +#define RMI_EXIT_FIQ			0x02
>> +#define RMI_EXIT_PSCI			0x03
>> +#define RMI_EXIT_RIPAS_CHANGE		0x04
>> +#define RMI_EXIT_HOST_CALL		0x05
>> +#define RMI_EXIT_SERROR			0x06
>> +
>> +struct rec_exit {
>> +	union { /* 0x000 */
>> +		u8 exit_reason;
>> +		u8 padding0[0x100];
>> +	};
>> +	union { /* 0x100 */
>> +		struct {
>> +			u64 esr;
>> +			u64 far;
>> +			u64 hpfar;
>> +		};
>> +		u8 padding1[0x100];
>> +	};
>> +	union { /* 0x200 */
>> +		u64 gprs[REC_RUN_GPRS];
>> +		u8 padding2[0x100];
>> +	};
>> +	union { /* 0x300 */
>> +		struct {
>> +			u64 gicv3_hcr;
>> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
>> +			u64 gicv3_misr;
> 
> Why do we care about ICH_MISR_EL2? Surely we get everything in the
> registers themselves, right? I think this goes back to my question
> above: why is the RMM getting in the way of ICH_*_EL2 accesses?

As mentioned above, the state of the guest's GIC isn't passed through
the CPU's registers, but instead using the rec_enter/rec_exit
structures. So unlike a normal guest entry we don't set all the CPU's
register state before entering, but instead hand over a shared data
structure and the RMM is responsible for actually programming the
registers on the CPU. Since many of the registers are (deliberately)
unavailable to the host (e.g. all the GPRs) it makes some sense the RMM
also handles the GIC registers save/restore.

Thanks,
Steve

>> +			u64 gicv3_vmcr;
>> +		};
> 
> Thanks,
> 
> 	M.
>
Re: [PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM
Posted by Marc Zyngier 4 months, 1 week ago
On Wed, 01 Oct 2025 12:00:14 +0100,
Steven Price <steven.price@arm.com> wrote:
> 
> Hi Marc,
> 
> On 01/10/2025 11:05, Marc Zyngier wrote:
> > On Wed, 20 Aug 2025 15:55:23 +0100,
> > Steven Price <steven.price@arm.com> wrote:
> >>
> >> The RMM (Realm Management Monitor) provides functionality that can be
> >> accessed by SMC calls from the host.
> >>
> >> The SMC definitions are based on DEN0137[1] version 1.0-rel0
> >>
> >> [1] https://developer.arm.com/documentation/den0137/1-0rel0/
> >>
> >> Reviewed-by: Gavin Shan <gshan@redhat.com>
> >> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
> >> Signed-off-by: Steven Price <steven.price@arm.com>
> >> ---
> >> Changes since v9:
> >>  * Corrected size of 'ripas_value' in struct rec_exit. The spec states
> >>    this is an 8-bit type with padding afterwards (rather than a u64).
> >> Changes since v8:
> >>  * Added RMI_PERMITTED_GICV3_HCR_BITS to define which bits the RMM
> >>    permits to be modified.
> >> Changes since v6:
> >>  * Renamed REC_ENTER_xxx defines to include 'FLAG' to make it obvious
> >>    these are flag values.
> >> Changes since v5:
> >>  * Sorted the SMC #defines by value.
> >>  * Renamed SMI_RxI_CALL to SMI_RMI_CALL since the macro is only used for
> >>    RMI calls.
> >>  * Renamed REC_GIC_NUM_LRS to REC_MAX_GIC_NUM_LRS since the actual
> >>    number of available list registers could be lower.
> >>  * Provided a define for the reserved fields of FeatureRegister0.
> >>  * Fix inconsistent names for padding fields.
> >> Changes since v4:
> >>  * Update to point to final released RMM spec.
> >>  * Minor rearrangements.
> >> Changes since v3:
> >>  * Update to match RMM spec v1.0-rel0-rc1.
> >> Changes since v2:
> >>  * Fix specification link.
> >>  * Rename rec_entry->rec_enter to match spec.
> >>  * Fix size of pmu_ovf_status to match spec.
> >> ---
> >>  arch/arm64/include/asm/rmi_smc.h | 269 +++++++++++++++++++++++++++++++
> >>  1 file changed, 269 insertions(+)
> >>  create mode 100644 arch/arm64/include/asm/rmi_smc.h
> >>
> >> diff --git a/arch/arm64/include/asm/rmi_smc.h b/arch/arm64/include/asm/rmi_smc.h
> >> new file mode 100644
> >> index 000000000000..1000368f1bca
> >> --- /dev/null
> >> +++ b/arch/arm64/include/asm/rmi_smc.h
> > 
> > [...]
> > 
> >> +#define RMI_PERMITTED_GICV3_HCR_BITS	(ICH_HCR_EL2_UIE |		\
> >> +					 ICH_HCR_EL2_LRENPIE |		\
> >> +					 ICH_HCR_EL2_NPIE |		\
> >> +					 ICH_HCR_EL2_VGrp0EIE |		\
> >> +					 ICH_HCR_EL2_VGrp0DIE |		\
> >> +					 ICH_HCR_EL2_VGrp1EIE |		\
> >> +					 ICH_HCR_EL2_VGrp1DIE |		\
> >> +					 ICH_HCR_EL2_TDIR)
> > 
> > Why should KVM care about what bits the RMM wants to use? Also, why
> > should KVM be forbidden to use the TALL0, TALL1 and TC bits? If
> > interrupt delivery is the host's business, then the RMM has no
> > business interfering with the GIC programming.
> 
> The RMM receives the guest's GIC state in a field within the REC entry
> structure (enter.gicv3_hcr). The RMM spec states that the above is the
> list of fields that will be considered and that everything else must be
> 0[1]. So this is used to filter the configuration to make sure it's
> valid for the RMM.
> 
> In terms of TALL0/TALL1/TC bits: these control trapping to EL2, and when
> in a realm guest the RMM is EL2 - so it's up to the RMM to configure
> these bits appropriately as it is the RMM which will have to deal with
> the trap.

And I claim this is *wrong*. Again, if the host is in charge of
interrupt injection, then the RMM has absolutely no business is
deciding what can or cannot be trapped. There is zero information
exposed by these traps that the host is not already aware of.

> [1] RWVGFJ in the 1.0 spec from
> https://developer.arm.com/documentation/den0137/latest

Well, until someone explains what this is protecting against, I
consider this as broken.

> >> +	union { /* 0x300 */
> >> +		struct {
> >> +			u64 gicv3_hcr;
> >> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
> >> +			u64 gicv3_misr;
> > 
> > Why do we care about ICH_MISR_EL2? Surely we get everything in the
> > registers themselves, right? I think this goes back to my question
> > above: why is the RMM getting in the way of ICH_*_EL2 accesses?
> 
> As mentioned above, the state of the guest's GIC isn't passed through
> the CPU's registers, but instead using the rec_enter/rec_exit
> structures. So unlike a normal guest entry we don't set all the CPU's
> register state before entering, but instead hand over a shared data
> structure and the RMM is responsible for actually programming the
> registers on the CPU. Since many of the registers are (deliberately)
> unavailable to the host (e.g. all the GPRs) it makes some sense the RMM
> also handles the GIC registers save/restore.

And I claim this is nonsense. There is nothing in these registers that
the host doesn't need to know about, which is why they are basically
copied over.

It all feels just wrong.

	M.

-- 
Without deviation from the norm, progress is not possible.
Re: [PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM
Posted by Steven Price 4 months, 1 week ago
On 01/10/2025 12:58, Marc Zyngier wrote:
> On Wed, 01 Oct 2025 12:00:14 +0100,
> Steven Price <steven.price@arm.com> wrote:
>>
>> Hi Marc,
>>
>> On 01/10/2025 11:05, Marc Zyngier wrote:
>>> On Wed, 20 Aug 2025 15:55:23 +0100,
>>> Steven Price <steven.price@arm.com> wrote:
>>>>
>>>> The RMM (Realm Management Monitor) provides functionality that can be
>>>> accessed by SMC calls from the host.
>>>>
>>>> The SMC definitions are based on DEN0137[1] version 1.0-rel0
>>>>
>>>> [1] https://developer.arm.com/documentation/den0137/1-0rel0/
>>>>
>>>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>>>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>> ---
>>>> Changes since v9:
>>>>  * Corrected size of 'ripas_value' in struct rec_exit. The spec states
>>>>    this is an 8-bit type with padding afterwards (rather than a u64).
>>>> Changes since v8:
>>>>  * Added RMI_PERMITTED_GICV3_HCR_BITS to define which bits the RMM
>>>>    permits to be modified.
>>>> Changes since v6:
>>>>  * Renamed REC_ENTER_xxx defines to include 'FLAG' to make it obvious
>>>>    these are flag values.
>>>> Changes since v5:
>>>>  * Sorted the SMC #defines by value.
>>>>  * Renamed SMI_RxI_CALL to SMI_RMI_CALL since the macro is only used for
>>>>    RMI calls.
>>>>  * Renamed REC_GIC_NUM_LRS to REC_MAX_GIC_NUM_LRS since the actual
>>>>    number of available list registers could be lower.
>>>>  * Provided a define for the reserved fields of FeatureRegister0.
>>>>  * Fix inconsistent names for padding fields.
>>>> Changes since v4:
>>>>  * Update to point to final released RMM spec.
>>>>  * Minor rearrangements.
>>>> Changes since v3:
>>>>  * Update to match RMM spec v1.0-rel0-rc1.
>>>> Changes since v2:
>>>>  * Fix specification link.
>>>>  * Rename rec_entry->rec_enter to match spec.
>>>>  * Fix size of pmu_ovf_status to match spec.
>>>> ---
>>>>  arch/arm64/include/asm/rmi_smc.h | 269 +++++++++++++++++++++++++++++++
>>>>  1 file changed, 269 insertions(+)
>>>>  create mode 100644 arch/arm64/include/asm/rmi_smc.h
>>>>
>>>> diff --git a/arch/arm64/include/asm/rmi_smc.h b/arch/arm64/include/asm/rmi_smc.h
>>>> new file mode 100644
>>>> index 000000000000..1000368f1bca
>>>> --- /dev/null
>>>> +++ b/arch/arm64/include/asm/rmi_smc.h
>>>
>>> [...]
>>>
>>>> +#define RMI_PERMITTED_GICV3_HCR_BITS	(ICH_HCR_EL2_UIE |		\
>>>> +					 ICH_HCR_EL2_LRENPIE |		\
>>>> +					 ICH_HCR_EL2_NPIE |		\
>>>> +					 ICH_HCR_EL2_VGrp0EIE |		\
>>>> +					 ICH_HCR_EL2_VGrp0DIE |		\
>>>> +					 ICH_HCR_EL2_VGrp1EIE |		\
>>>> +					 ICH_HCR_EL2_VGrp1DIE |		\
>>>> +					 ICH_HCR_EL2_TDIR)
>>>
>>> Why should KVM care about what bits the RMM wants to use? Also, why
>>> should KVM be forbidden to use the TALL0, TALL1 and TC bits? If
>>> interrupt delivery is the host's business, then the RMM has no
>>> business interfering with the GIC programming.
>>
>> The RMM receives the guest's GIC state in a field within the REC entry
>> structure (enter.gicv3_hcr). The RMM spec states that the above is the
>> list of fields that will be considered and that everything else must be
>> 0[1]. So this is used to filter the configuration to make sure it's
>> valid for the RMM.
>>
>> In terms of TALL0/TALL1/TC bits: these control trapping to EL2, and when
>> in a realm guest the RMM is EL2 - so it's up to the RMM to configure
>> these bits appropriately as it is the RMM which will have to deal with
>> the trap.
> 
> And I claim this is *wrong*. Again, if the host is in charge of
> interrupt injection, then the RMM has absolutely no business is
> deciding what can or cannot be trapped. There is zero information
> exposed by these traps that the host is not already aware of.
> 
>> [1] RWVGFJ in the 1.0 spec from
>> https://developer.arm.com/documentation/den0137/latest
> 
> Well, until someone explains what this is protecting against, I
> consider this as broken.

I'm not sure I understand how you want this to work. Ultimately the
realm guest entry is a bounce from NS-EL2 to EL3 to R-EL2 to R-EL1/0. So
the RMM has to have some control over the trapping behaviour for its own
protection. The current spec means that the RMM does not have to
implement the trap handlers for TALL0/TALL1/TC and can simply force
these bits to 0. Allowing the host to enable traps that the RMM isn't
expecting will obviously end in problems.

If your argument is that because the NS host is emulating the GIC it
needs to be able to do these traps, then that's something that can be
fed back to the spec and hopefully improved. In that case the trap
information would be provided in the rec_entry structure and on trap the
RMM would return prepare information in the rec_exit structure. This
could in theory be handled similar to an emulatable data abort with a
new exit reason.

The other approach would be to push more GIC handling into the RMM such
that these trap bits are not needed (i.e. there's no requirement to exit
to the NS host to handle the trap, and the RMM can program them
independently). I'm afraid I don't understand the GIC well enough to
know how these traps are used and how feasible it is for the RMM to just
"do the right thing" here.

>>>> +	union { /* 0x300 */
>>>> +		struct {
>>>> +			u64 gicv3_hcr;
>>>> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
>>>> +			u64 gicv3_misr;
>>>
>>> Why do we care about ICH_MISR_EL2? Surely we get everything in the
>>> registers themselves, right? I think this goes back to my question
>>> above: why is the RMM getting in the way of ICH_*_EL2 accesses?
>>
>> As mentioned above, the state of the guest's GIC isn't passed through
>> the CPU's registers, but instead using the rec_enter/rec_exit
>> structures. So unlike a normal guest entry we don't set all the CPU's
>> register state before entering, but instead hand over a shared data
>> structure and the RMM is responsible for actually programming the
>> registers on the CPU. Since many of the registers are (deliberately)
>> unavailable to the host (e.g. all the GPRs) it makes some sense the RMM
>> also handles the GIC registers save/restore.
> 
> And I claim this is nonsense. There is nothing in these registers that
> the host doesn't need to know about, which is why they are basically
> copied over.

Well it's fairly obvious that the host (generally) doesn't need to know
the general purpose registers. And it's fairly clear that confidential
compute would be pretty pointless if the hypervisor leaked those
registers. So I hope we agree that some architectural registers are
going to have to be handled differently from a normal guest.

The GIC is unusual because it's (partly) emulated by the host. The
configuration is also complex because during guest entry rather than
just dropping down to EL1/0 we're actually performing an SMC to EL3 and
world-switching. So I'm not sure to what extent programming the
architectural registers in the normal world would work.

> It all feels just wrong.

I think fundamentally the confusing thing is there are two hypervisors
pretending to be one. Both KVM and the RMM are providing part of the
role of the hypervisor. It would "feel" neater for the RMM to take on
more responsibility of the hypervisor role but that leads to more
complexity in the RMM (whose simplicity is part of the value of CCA) and
potentially less flexibility because you haven't got the functionality
of KVM.

Thanks,
Steve
Re: [PATCH v10 03/43] arm64: RME: Add SMC definitions for calling the RMM
Posted by Suzuki K Poulose 4 months ago
Hi,

On 01/10/2025 15:05, Steven Price wrote:
> On 01/10/2025 12:58, Marc Zyngier wrote:
>> On Wed, 01 Oct 2025 12:00:14 +0100,
>> Steven Price <steven.price@arm.com> wrote:
>>>
>>> Hi Marc,
>>>
>>> On 01/10/2025 11:05, Marc Zyngier wrote:
>>>> On Wed, 20 Aug 2025 15:55:23 +0100,
>>>> Steven Price <steven.price@arm.com> wrote:
>>>>>
>>>>> The RMM (Realm Management Monitor) provides functionality that can be
>>>>> accessed by SMC calls from the host.
>>>>>
>>>>> The SMC definitions are based on DEN0137[1] version 1.0-rel0
>>>>>
>>>>> [1] https://developer.arm.com/documentation/den0137/1-0rel0/
>>>>>
>>>>> Reviewed-by: Gavin Shan <gshan@redhat.com>
>>>>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
>>>>> Signed-off-by: Steven Price <steven.price@arm.com>
>>>>> ---
>>>>> Changes since v9:
>>>>>   * Corrected size of 'ripas_value' in struct rec_exit. The spec states
>>>>>     this is an 8-bit type with padding afterwards (rather than a u64).
>>>>> Changes since v8:
>>>>>   * Added RMI_PERMITTED_GICV3_HCR_BITS to define which bits the RMM
>>>>>     permits to be modified.
>>>>> Changes since v6:
>>>>>   * Renamed REC_ENTER_xxx defines to include 'FLAG' to make it obvious
>>>>>     these are flag values.
>>>>> Changes since v5:
>>>>>   * Sorted the SMC #defines by value.
>>>>>   * Renamed SMI_RxI_CALL to SMI_RMI_CALL since the macro is only used for
>>>>>     RMI calls.
>>>>>   * Renamed REC_GIC_NUM_LRS to REC_MAX_GIC_NUM_LRS since the actual
>>>>>     number of available list registers could be lower.
>>>>>   * Provided a define for the reserved fields of FeatureRegister0.
>>>>>   * Fix inconsistent names for padding fields.
>>>>> Changes since v4:
>>>>>   * Update to point to final released RMM spec.
>>>>>   * Minor rearrangements.
>>>>> Changes since v3:
>>>>>   * Update to match RMM spec v1.0-rel0-rc1.
>>>>> Changes since v2:
>>>>>   * Fix specification link.
>>>>>   * Rename rec_entry->rec_enter to match spec.
>>>>>   * Fix size of pmu_ovf_status to match spec.
>>>>> ---
>>>>>   arch/arm64/include/asm/rmi_smc.h | 269 +++++++++++++++++++++++++++++++
>>>>>   1 file changed, 269 insertions(+)
>>>>>   create mode 100644 arch/arm64/include/asm/rmi_smc.h
>>>>>
>>>>> diff --git a/arch/arm64/include/asm/rmi_smc.h b/arch/arm64/include/asm/rmi_smc.h
>>>>> new file mode 100644
>>>>> index 000000000000..1000368f1bca
>>>>> --- /dev/null
>>>>> +++ b/arch/arm64/include/asm/rmi_smc.h
>>>>
>>>> [...]
>>>>
>>>>> +#define RMI_PERMITTED_GICV3_HCR_BITS	(ICH_HCR_EL2_UIE |		\
>>>>> +					 ICH_HCR_EL2_LRENPIE |		\
>>>>> +					 ICH_HCR_EL2_NPIE |		\
>>>>> +					 ICH_HCR_EL2_VGrp0EIE |		\
>>>>> +					 ICH_HCR_EL2_VGrp0DIE |		\
>>>>> +					 ICH_HCR_EL2_VGrp1EIE |		\
>>>>> +					 ICH_HCR_EL2_VGrp1DIE |		\
>>>>> +					 ICH_HCR_EL2_TDIR)
>>>>
>>>> Why should KVM care about what bits the RMM wants to use? Also, why
>>>> should KVM be forbidden to use the TALL0, TALL1 and TC bits? If
>>>> interrupt delivery is the host's business, then the RMM has no
>>>> business interfering with the GIC programming.
>>>
>>> The RMM receives the guest's GIC state in a field within the REC entry
>>> structure (enter.gicv3_hcr). The RMM spec states that the above is the
>>> list of fields that will be considered and that everything else must be
>>> 0[1]. So this is used to filter the configuration to make sure it's
>>> valid for the RMM.
>>>
>>> In terms of TALL0/TALL1/TC bits: these control trapping to EL2, and when
>>> in a realm guest the RMM is EL2 - so it's up to the RMM to configure
>>> these bits appropriately as it is the RMM which will have to deal with
>>> the trap.
>>
>> And I claim this is *wrong*. Again, if the host is in charge of
>> interrupt injection, then the RMM has absolutely no business is
>> deciding what can or cannot be trapped. There is zero information
>> exposed by these traps that the host is not already aware of.
>>
>>> [1] RWVGFJ in the 1.0 spec from
>>> https://developer.arm.com/documentation/den0137/latest
>>
>> Well, until someone explains what this is protecting against, I
>> consider this as broken.
> 
> I'm not sure I understand how you want this to work. Ultimately the
> realm guest entry is a bounce from NS-EL2 to EL3 to R-EL2 to R-EL1/0. So
> the RMM has to have some control over the trapping behaviour for its own
> protection. The current spec means that the RMM does not have to
> implement the trap handlers for TALL0/TALL1/TC and can simply force
> these bits to 0. Allowing the host to enable traps that the RMM isn't
> expecting will obviously end in problems.

The RMM design took a conservative approach of exposing bare minimum
controls to the host to manage the VGIC, without increasing the
complexity in the RMM. But if you think that the current set of
controls are not sufficient for the Host to manage the Realm VGIC,
like Steven mentions below, we could feed this back to the RMM spec
and extend it in the future versions. I expect the new traps
would be reported back as "sysreg" accesses (similar to the already
exposed ICC_DIR, ICC_SGIxR).

Thanks
Suzuki


> 
> If your argument is that because the NS host is emulating the GIC it
> needs to be able to do these traps, then that's something that can be
> fed back to the spec and hopefully improved. In that case the trap
> information would be provided in the rec_entry structure and on trap the
> RMM would return prepare information in the rec_exit structure. This
> could in theory be handled similar to an emulatable data abort with a
> new exit reason.
> 
> The other approach would be to push more GIC handling into the RMM such
> that these trap bits are not needed (i.e. there's no requirement to exit
> to the NS host to handle the trap, and the RMM can program them
> independently). I'm afraid I don't understand the GIC well enough to
> know how these traps are used and how feasible it is for the RMM to just
> "do the right thing" here.
> 
>>>>> +	union { /* 0x300 */
>>>>> +		struct {
>>>>> +			u64 gicv3_hcr;
>>>>> +			u64 gicv3_lrs[REC_MAX_GIC_NUM_LRS];
>>>>> +			u64 gicv3_misr;
>>>>
>>>> Why do we care about ICH_MISR_EL2? Surely we get everything in the
>>>> registers themselves, right? I think this goes back to my question
>>>> above: why is the RMM getting in the way of ICH_*_EL2 accesses?
>>>
>>> As mentioned above, the state of the guest's GIC isn't passed through
>>> the CPU's registers, but instead using the rec_enter/rec_exit
>>> structures. So unlike a normal guest entry we don't set all the CPU's
>>> register state before entering, but instead hand over a shared data
>>> structure and the RMM is responsible for actually programming the
>>> registers on the CPU. Since many of the registers are (deliberately)
>>> unavailable to the host (e.g. all the GPRs) it makes some sense the RMM
>>> also handles the GIC registers save/restore.
>>
>> And I claim this is nonsense. There is nothing in these registers that
>> the host doesn't need to know about, which is why they are basically
>> copied over.
> 
> Well it's fairly obvious that the host (generally) doesn't need to know
> the general purpose registers. And it's fairly clear that confidential
> compute would be pretty pointless if the hypervisor leaked those
> registers. So I hope we agree that some architectural registers are
> going to have to be handled differently from a normal guest.
> 
> The GIC is unusual because it's (partly) emulated by the host. The
> configuration is also complex because during guest entry rather than
> just dropping down to EL1/0 we're actually performing an SMC to EL3 and
> world-switching. So I'm not sure to what extent programming the
> architectural registers in the normal world would work.
> 
>> It all feels just wrong.
> 
> I think fundamentally the confusing thing is there are two hypervisors
> pretending to be one. Both KVM and the RMM are providing part of the
> role of the hypervisor. It would "feel" neater for the RMM to take on
> more responsibility of the hypervisor role but that leads to more
> complexity in the RMM (whose simplicity is part of the value of CCA) and
> potentially less flexibility because you haven't got the functionality
> of KVM.
> 
> Thanks,
> Steve