linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS
@ 2024-10-05 10:37 Mark Brown
  2024-10-05 10:37 ` [PATCH v14 1/5] KVM: arm64: Expose S1PIE to guests Mark Brown
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Mark Brown @ 2024-10-05 10:37 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton,
	Joey Gouly, Suzuki K Poulose, Shuah Khan
  Cc: linux-arm-kernel, linux-doc, kvmarm, linux-kselftest,
	linux-kernel, Mark Brown, Thiago Jung Bauermann

The arm64 Guarded Control Stack (GCS) feature provides support for
hardware protected stacks of return addresses, intended to provide
hardening against return oriented programming (ROP) attacks and to make
it easier to gather call stacks for applications such as profiling.

When GCS is active a secondary stack called the Guarded Control Stack is
maintained, protected with a memory attribute which means that it can
only be written with specific GCS operations.  The current GCS pointer
can not be directly written to by userspace.  When a BL is executed the
value stored in LR is also pushed onto the GCS, and when a RET is
executed the top of the GCS is popped and compared to LR with a fault
being raised if the values do not match.  GCS operations may only be
performed on GCS pages, a data abort is generated if they are not.

The combination of hardware enforcement and lack of extra instructions
in the function entry and exit paths should result in something which
has less overhead and is more difficult to attack than a purely software
implementation like clang's shadow stacks.

This series implements support for managing GCS for KVM guests, it also
includes a fix for S1PIE which has also been sent separately as this
feature is a dependency for GCS.  It is based on:

   https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/gcs

Signed-off-by: Mark Brown <broonie@kernel.org>
---
Changes in v14:
- Rebase onto arm64/for-next/gcs which includes all the non-KVM support.
- Manage the fine grained traps for GCS instructions.
- Manage PSTATE.EXLOCK when delivering exceptions to KVM guests.
- Link to v13: https://lore.kernel.org/r/20241001-arm64-gcs-v13-0-222b78d87eee@kernel.org

Changes in v13:
- Rebase onto v6.12-rc1.
- Allocate VM_HIGH_ARCH_6 since protection keys used all the existing
  bits.
- Implement mm_release() and free transparently allocated GCSs there.
- Use bit 32 of AT_HWCAP for GCS due to AT_HWCAP2 being filled.
- Since we now only set GCSCRE0_EL1 on change ensure that it is
  initialised with GCSPR_EL0 accessible to EL0.
- Fix OOM handling on thread copy.
- Link to v12: https://lore.kernel.org/r/20240829-arm64-gcs-v12-0-42fec947436a@kernel.org

Changes in v12:
- Clarify and simplify the signal handling code so we work with the
  register state.
- When checking for write aborts to shadow stack pages ensure the fault
  is a data abort.
- Depend on !UPROBES.
- Comment cleanups.
- Link to v11: https://lore.kernel.org/r/20240822-arm64-gcs-v11-0-41b81947ecb5@kernel.org

Changes in v11:
- Remove the dependency on the addition of clone3() support for shadow
  stacks, rebasing onto v6.11-rc3.
- Make ID_AA64PFR1_EL1.GCS writeable in KVM.
- Hide GCS registers when GCS is not enabled for KVM guests.
- Require HCRX_EL2.GCSEn if booting at EL1.
- Require that GCSCR_EL1 and GCSCRE0_EL1 be initialised regardless of
  if we boot at EL2 or EL1.
- Remove some stray use of bit 63 in signal cap tokens.
- Warn if we see a GCS with VM_SHARED.
- Remove rdundant check for VM_WRITE in fault handling.
- Cleanups and clarifications in the ABI document.
- Clean up and improve documentation of some sync placement.
- Only set the EL0 GCS mode if it's actually changed.
- Various minor fixes and tweaks.
- Link to v10: https://lore.kernel.org/r/20240801-arm64-gcs-v10-0-699e2bd2190b@kernel.org

Changes in v10:
- Fix issues with THP.
- Tighten up requirements for initialising GCSCR*.
- Only generate GCS signal frames for threads using GCS.
- Only context switch EL1 GCS registers if S1PIE is enabled.
- Move context switch of GCSCRE0_EL1 to EL0 context switch.
- Make GCS registers unconditionally visible to userspace.
- Use FHU infrastructure.
- Don't change writability of ID_AA64PFR1_EL1 for KVM.
- Remove unused arguments from alloc_gcs().
- Typo fixes.
- Link to v9: https://lore.kernel.org/r/20240625-arm64-gcs-v9-0-0f634469b8f0@kernel.org

Changes in v9:
- Rebase onto v6.10-rc3.
- Restructure and clarify memory management fault handling.
- Fix up basic-gcs for the latest clone3() changes.
- Convert to newly merged KVM ID register based feature configuration.
- Fixes for NV traps.
- Link to v8: https://lore.kernel.org/r/20240203-arm64-gcs-v8-0-c9fec77673ef@kernel.org

Changes in v8:
- Invalidate signal cap token on stack when consuming.
- Typo and other trivial fixes.
- Don't try to use process_vm_write() on GCS, it intentionally does not
  work.
- Fix leak of thread GCSs.
- Rebase onto latest clone3() series.
- Link to v7: https://lore.kernel.org/r/20231122-arm64-gcs-v7-0-201c483bd775@kernel.org

Changes in v7:
- Rebase onto v6.7-rc2 via the clone3() patch series.
- Change the token used to cap the stack during signal handling to be
  compatible with GCSPOPM.
- Fix flags for new page types.
- Fold in support for clone3().
- Replace copy_to_user_gcs() with put_user_gcs().
- Link to v6: https://lore.kernel.org/r/20231009-arm64-gcs-v6-0-78e55deaa4dd@kernel.org

Changes in v6:
- Rebase onto v6.6-rc3.
- Add some more gcsb_dsync() barriers following spec clarifications.
- Due to ongoing discussion around clone()/clone3() I've not updated
  anything there, the behaviour is the same as on previous versions.
- Link to v5: https://lore.kernel.org/r/20230822-arm64-gcs-v5-0-9ef181dd6324@kernel.org

Changes in v5:
- Don't map any permissions for user GCSs, we always use EL0 accessors
  or use a separate mapping of the page.
- Reduce the standard size of the GCS to RLIMIT_STACK/2.
- Enforce a PAGE_SIZE alignment requirement on map_shadow_stack().
- Clarifications and fixes to documentation.
- More tests.
- Link to v4: https://lore.kernel.org/r/20230807-arm64-gcs-v4-0-68cfa37f9069@kernel.org

Changes in v4:
- Implement flags for map_shadow_stack() allowing the cap and end of
  stack marker to be enabled independently or not at all.
- Relax size and alignment requirements for map_shadow_stack().
- Add more blurb explaining the advantages of hardware enforcement.
- Link to v3: https://lore.kernel.org/r/20230731-arm64-gcs-v3-0-cddf9f980d98@kernel.org

Changes in v3:
- Rebase onto v6.5-rc4.
- Add a GCS barrier on context switch.
- Add a GCS stress test.
- Link to v2: https://lore.kernel.org/r/20230724-arm64-gcs-v2-0-dc2c1d44c2eb@kernel.org

Changes in v2:
- Rebase onto v6.5-rc3.
- Rework prctl() interface to allow each bit to be locked independently.
- map_shadow_stack() now places the cap token based on the size
  requested by the caller not the actual space allocated.
- Mode changes other than enable via ptrace are now supported.
- Expand test coverage.
- Various smaller fixes and adjustments.
- Link to v1: https://lore.kernel.org/r/20230716-arm64-gcs-v1-0-bf567f93bba6@kernel.org

---
Mark Brown (5):
      KVM: arm64: Expose S1PIE to guests
      arm64/gcs: Ensure FGTs for EL1 GCS instructions are disabled
      KVM: arm64: Manage GCS access and registers for guests
      KVM: arm64: Set PSTATE.EXLOCK when entering an exception
      KVM: selftests: arm64: Add GCS registers to get-reg-list

 arch/arm64/include/asm/el2_setup.h                 |  7 ++++-
 arch/arm64/include/asm/kvm_host.h                  | 12 ++++++++
 arch/arm64/include/asm/vncr_mapping.h              |  2 ++
 arch/arm64/include/uapi/asm/ptrace.h               |  2 ++
 arch/arm64/kvm/hyp/exception.c                     | 10 +++++++
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h         | 31 +++++++++++++++++++
 arch/arm64/kvm/sys_regs.c                          | 35 ++++++++++++++++++++--
 tools/testing/selftests/kvm/aarch64/get-reg-list.c | 28 +++++++++++++++++
 8 files changed, 124 insertions(+), 3 deletions(-)
---
base-commit: ed4983d2da8c3b66ac6d048beb242916bec83522
change-id: 20230303-arm64-gcs-e311ab0d8729

Best regards,
-- 
Mark Brown <broonie@kernel.org>



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

* [PATCH v14 1/5] KVM: arm64: Expose S1PIE to guests
  2024-10-05 10:37 [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS Mark Brown
@ 2024-10-05 10:37 ` Mark Brown
  2024-10-05 10:37 ` [PATCH v14 2/5] arm64/gcs: Ensure FGTs for EL1 GCS instructions are disabled Mark Brown
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-10-05 10:37 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton,
	Joey Gouly, Suzuki K Poulose, Shuah Khan
  Cc: linux-arm-kernel, linux-doc, kvmarm, linux-kselftest,
	linux-kernel, Mark Brown

Prior to commit 70ed7238297f ("KVM: arm64: Sanitise ID_AA64MMFR3_EL1")
we just exposed the santised view of ID_AA64MMFR3_EL1 to guests, meaning
that they saw both TCRX and S1PIE if present on the host machine. That
commit added VMM control over the contents of the register and exposed
S1POE but removed S1PIE, meaning that the extension is no longer visible
to guests. Reenable support for S1PIE with VMM control.

Fixes: 70ed7238297f ("KVM: arm64: Sanitise ID_AA64MMFR3_EL1")
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index dad88e31f9537fe02e28b117d6a740f15572e0ba..d48f89ad6aa7139078e7991ce6c8ebc4a0543551 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1550,7 +1550,8 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,
 		val &= ~ID_AA64MMFR2_EL1_CCIDX_MASK;
 		break;
 	case SYS_ID_AA64MMFR3_EL1:
-		val &= ID_AA64MMFR3_EL1_TCRX | ID_AA64MMFR3_EL1_S1POE;
+		val &= ID_AA64MMFR3_EL1_TCRX | ID_AA64MMFR3_EL1_S1POE |
+			ID_AA64MMFR3_EL1_S1PIE;
 		break;
 	case SYS_ID_MMFR4_EL1:
 		val &= ~ARM64_FEATURE_MASK(ID_MMFR4_EL1_CCIDX);
@@ -2433,6 +2434,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 					ID_AA64MMFR2_EL1_NV |
 					ID_AA64MMFR2_EL1_CCIDX)),
 	ID_WRITABLE(ID_AA64MMFR3_EL1, (ID_AA64MMFR3_EL1_TCRX	|
+				       ID_AA64MMFR3_EL1_S1PIE   |
 				       ID_AA64MMFR3_EL1_S1POE)),
 	ID_SANITISED(ID_AA64MMFR4_EL1),
 	ID_UNALLOCATED(7,5),

-- 
2.39.2



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

* [PATCH v14 2/5] arm64/gcs: Ensure FGTs for EL1 GCS instructions are disabled
  2024-10-05 10:37 [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS Mark Brown
  2024-10-05 10:37 ` [PATCH v14 1/5] KVM: arm64: Expose S1PIE to guests Mark Brown
@ 2024-10-05 10:37 ` Mark Brown
  2024-10-05 10:37 ` [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests Mark Brown
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-10-05 10:37 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton,
	Joey Gouly, Suzuki K Poulose, Shuah Khan
  Cc: linux-arm-kernel, linux-doc, kvmarm, linux-kselftest,
	linux-kernel, Mark Brown

The initial EL2 setup for GCS did not include disabling of EL1 usage of
GCS instructions, also disable these traps.  This is the first disabling
of instruction traps, use x2 to store the value to be written.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/el2_setup.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/el2_setup.h b/arch/arm64/include/asm/el2_setup.h
index 27086a81eae34483a682681ab1be1959a339527a..99e887a5b2190f8810f1ed22d35b7acd26d2fd1e 100644
--- a/arch/arm64/include/asm/el2_setup.h
+++ b/arch/arm64/include/asm/el2_setup.h
@@ -170,6 +170,7 @@
 	cbz	x1, .Lskip_fgt_\@
 
 	mov	x0, xzr
+	mov     x2, xzr
 	mrs	x1, id_aa64dfr0_el1
 	ubfx	x1, x1, #ID_AA64DFR0_EL1_PMSVer_SHIFT, #4
 	cmp	x1, #3
@@ -217,10 +218,14 @@
 	orr	x0, x0, #HFGxTR_EL2_nGCS_EL1_MASK
 	orr	x0, x0, #HFGxTR_EL2_nGCS_EL0_MASK
 
+	orr	x2, x2, #HFGITR_EL2_nGCSEPP_MASK
+	orr	x2, x2, #HFGITR_EL2_nGCSSTR_EL1_MASK
+	orr	x2, x2, #HFGITR_EL2_nGCSPUSHM_EL1_MASK
+
 .Lset_fgt_\@:
 	msr_s	SYS_HFGRTR_EL2, x0
 	msr_s	SYS_HFGWTR_EL2, x0
-	msr_s	SYS_HFGITR_EL2, xzr
+	msr_s	SYS_HFGITR_EL2, x2
 
 	mrs	x1, id_aa64pfr0_el1		// AMU traps UNDEF without AMU
 	ubfx	x1, x1, #ID_AA64PFR0_EL1_AMU_SHIFT, #4

-- 
2.39.2



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

* [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 10:37 [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS Mark Brown
  2024-10-05 10:37 ` [PATCH v14 1/5] KVM: arm64: Expose S1PIE to guests Mark Brown
  2024-10-05 10:37 ` [PATCH v14 2/5] arm64/gcs: Ensure FGTs for EL1 GCS instructions are disabled Mark Brown
@ 2024-10-05 10:37 ` Mark Brown
  2024-10-05 11:34   ` Marc Zyngier
  2024-10-05 10:37 ` [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception Mark Brown
  2024-10-05 10:37 ` [PATCH v14 5/5] KVM: selftests: arm64: Add GCS registers to get-reg-list Mark Brown
  4 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-05 10:37 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton,
	Joey Gouly, Suzuki K Poulose, Shuah Khan
  Cc: linux-arm-kernel, linux-doc, kvmarm, linux-kselftest,
	linux-kernel, Mark Brown

GCS introduces a number of system registers for EL1 and EL0, on systems
with GCS we need to context switch them and expose them to VMMs to allow
guests to use GCS.

In order to allow guests to use GCS we also need to configure
HCRX_EL2.GCSEn, if this is not set GCS instructions will be noops and
CHKFEAT will report GCS as disabled.  Also enable fine grained traps for
access to the GCS registers by guests which do not have the feature
enabled.

In order to allow userspace to control availability of the feature to
guests we enable writability for only ID_AA64PFR1_EL1.GCS, this is a
deliberately conservative choice to avoid errors due to oversights.
Further fields should be made writable in future.

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/asm/kvm_host.h          | 12 ++++++++++++
 arch/arm64/include/asm/vncr_mapping.h      |  2 ++
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 31 ++++++++++++++++++++++++++++++
 arch/arm64/kvm/sys_regs.c                  | 31 +++++++++++++++++++++++++++++-
 4 files changed, 75 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 329619c6fa9611b474dc4823fccf01a3b9dd61a8..31887d3f3de18bf1936ce4329256894f6210c67e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -448,6 +448,10 @@ enum vcpu_sysreg {
 
 	POR_EL0,	/* Permission Overlay Register 0 (EL0) */
 
+	/* Guarded Control Stack registers */
+	GCSCRE0_EL1,	/* Guarded Control Stack Control (EL0) */
+	GCSPR_EL0,	/* Guarded Control Stack Pointer (EL0) */
+
 	/* FP/SIMD/SVE */
 	SVCR,
 	FPMR,
@@ -525,6 +529,10 @@ enum vcpu_sysreg {
 
 	VNCR(POR_EL1),	/* Permission Overlay Register 1 (EL1) */
 
+	/* Guarded Control Stack registers */
+	VNCR(GCSPR_EL1),	/* Guarded Control Stack Pointer (EL1) */
+	VNCR(GCSCR_EL1),	/* Guarded Control Stack Control (EL1) */
+
 	VNCR(HFGRTR_EL2),
 	VNCR(HFGWTR_EL2),
 	VNCR(HFGITR_EL2),
@@ -1495,4 +1503,8 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
 	(system_supports_fpmr() &&			\
 	 kvm_has_feat((k), ID_AA64PFR2_EL1, FPMR, IMP))
 
+#define kvm_has_gcs(k) 					\
+	(system_supports_gcs() &&			\
+	 kvm_has_feat((k), ID_AA64PFR1_EL1, GCS, IMP))
+
 #endif /* __ARM64_KVM_HOST_H__ */
diff --git a/arch/arm64/include/asm/vncr_mapping.h b/arch/arm64/include/asm/vncr_mapping.h
index 06f8ec0906a6e92b6dd57c692e1293ac3d4244fd..e289064148b3268a00830c6fee708029c4c0fbbb 100644
--- a/arch/arm64/include/asm/vncr_mapping.h
+++ b/arch/arm64/include/asm/vncr_mapping.h
@@ -89,6 +89,8 @@
 #define VNCR_PMSIRR_EL1         0x840
 #define VNCR_PMSLATFR_EL1       0x848
 #define VNCR_TRFCR_EL1          0x880
+#define VNCR_GCSPR_EL1		0x8C0
+#define VNCR_GCSCR_EL1		0x8D0
 #define VNCR_MPAM1_EL1          0x900
 #define VNCR_MPAMHCR_EL2        0x930
 #define VNCR_MPAMVPMV_EL2       0x938
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index 1579a3c08a36b989fec7fdc3f38b64cb498326f3..70bd6143083439d75387eccc17f5c3811c689047 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -17,6 +17,7 @@
 #include <asm/kvm_mmu.h>
 
 static inline bool ctxt_has_s1poe(struct kvm_cpu_context *ctxt);
+static inline bool ctxt_has_gcs(struct kvm_cpu_context *ctxt);
 
 static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
 {
@@ -31,6 +32,11 @@ static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt)
 {
 	ctxt_sys_reg(ctxt, TPIDR_EL0)	= read_sysreg(tpidr_el0);
 	ctxt_sys_reg(ctxt, TPIDRRO_EL0)	= read_sysreg(tpidrro_el0);
+
+	if (ctxt_has_gcs(ctxt)) {
+		ctxt_sys_reg(ctxt, GCSPR_EL0) = read_sysreg_s(SYS_GCSPR_EL0);
+		ctxt_sys_reg(ctxt, GCSCRE0_EL1)	= read_sysreg_s(SYS_GCSCRE0_EL1);
+	}
 }
 
 static inline struct kvm_vcpu *ctxt_to_vcpu(struct kvm_cpu_context *ctxt)
@@ -83,6 +89,17 @@ static inline bool ctxt_has_s1poe(struct kvm_cpu_context *ctxt)
 	return kvm_has_feat(kern_hyp_va(vcpu->kvm), ID_AA64MMFR3_EL1, S1POE, IMP);
 }
 
+static inline bool ctxt_has_gcs(struct kvm_cpu_context *ctxt)
+{
+	struct kvm_vcpu *vcpu;
+
+	if (!cpus_have_final_cap(ARM64_HAS_GCS))
+		return false;
+
+	vcpu = ctxt_to_vcpu(ctxt);
+	return kvm_has_feat(kern_hyp_va(vcpu->kvm), ID_AA64PFR1_EL1, GCS, IMP);
+}
+
 static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
 {
 	ctxt_sys_reg(ctxt, SCTLR_EL1)	= read_sysreg_el1(SYS_SCTLR);
@@ -96,6 +113,10 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
 		if (ctxt_has_s1pie(ctxt)) {
 			ctxt_sys_reg(ctxt, PIR_EL1)	= read_sysreg_el1(SYS_PIR);
 			ctxt_sys_reg(ctxt, PIRE0_EL1)	= read_sysreg_el1(SYS_PIRE0);
+			if (ctxt_has_gcs(ctxt)) {
+				ctxt_sys_reg(ctxt, GCSPR_EL1)	= read_sysreg_el1(SYS_GCSPR);
+				ctxt_sys_reg(ctxt, GCSCR_EL1)	= read_sysreg_el1(SYS_GCSCR);
+			}
 		}
 
 		if (ctxt_has_s1poe(ctxt))
@@ -150,6 +171,11 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)
 {
 	write_sysreg(ctxt_sys_reg(ctxt, TPIDR_EL0),	tpidr_el0);
 	write_sysreg(ctxt_sys_reg(ctxt, TPIDRRO_EL0),	tpidrro_el0);
+	if (ctxt_has_gcs(ctxt)) {
+		write_sysreg_s(ctxt_sys_reg(ctxt, GCSPR_EL0), SYS_GCSPR_EL0);
+		write_sysreg_s(ctxt_sys_reg(ctxt, GCSCRE0_EL1),
+			       SYS_GCSCRE0_EL1);
+	}
 }
 
 static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
@@ -181,6 +207,11 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
 		if (ctxt_has_s1pie(ctxt)) {
 			write_sysreg_el1(ctxt_sys_reg(ctxt, PIR_EL1),	SYS_PIR);
 			write_sysreg_el1(ctxt_sys_reg(ctxt, PIRE0_EL1),	SYS_PIRE0);
+
+			if (ctxt_has_gcs(ctxt)) {
+				write_sysreg_el1(ctxt_sys_reg(ctxt, GCSPR_EL1),	SYS_GCSPR);
+				write_sysreg_el1(ctxt_sys_reg(ctxt, GCSCR_EL1),	SYS_GCSCR);
+			}
 		}
 
 		if (ctxt_has_s1poe(ctxt))
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d48f89ad6aa7139078e7991ce6c8ebc4a0543551..0d0c400ce15f0db2ffbe71d310eee6dbc2280429 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1642,6 +1642,15 @@ static unsigned int raz_visibility(const struct kvm_vcpu *vcpu,
 	return REG_RAZ;
 }
 
+static unsigned int gcs_visibility(const struct kvm_vcpu *vcpu,
+				   const struct sys_reg_desc *r)
+{
+	if (kvm_has_gcs(vcpu->kvm))
+		return 0;
+
+	return REG_HIDDEN;
+}
+
 /* cpufeature ID register access trap handlers */
 
 static bool access_id_reg(struct kvm_vcpu *vcpu,
@@ -2377,7 +2386,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 		   ID_AA64PFR0_EL1_RAS |
 		   ID_AA64PFR0_EL1_AdvSIMD |
 		   ID_AA64PFR0_EL1_FP), },
-	ID_SANITISED(ID_AA64PFR1_EL1),
+	ID_WRITABLE(ID_AA64PFR1_EL1, ID_AA64PFR1_EL1_GCS),
 	ID_WRITABLE(ID_AA64PFR2_EL1, ID_AA64PFR2_EL1_FPMR),
 	ID_UNALLOCATED(4,3),
 	ID_WRITABLE(ID_AA64ZFR0_EL1, ~ID_AA64ZFR0_EL1_RES0),
@@ -2463,6 +2472,13 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	PTRAUTH_KEY(APDB),
 	PTRAUTH_KEY(APGA),
 
+	{ SYS_DESC(SYS_GCSCR_EL1), NULL, reset_val, GCSCR_EL1, 0,
+	  .visibility = gcs_visibility },
+	{ SYS_DESC(SYS_GCSPR_EL1), NULL, reset_unknown, GCSPR_EL1,
+	  .visibility = gcs_visibility },
+	{ SYS_DESC(SYS_GCSCRE0_EL1), NULL, reset_val, GCSCRE0_EL1, 0,
+	  .visibility = gcs_visibility },
+
 	{ SYS_DESC(SYS_SPSR_EL1), access_spsr},
 	{ SYS_DESC(SYS_ELR_EL1), access_elr},
 
@@ -2569,6 +2585,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 			     CTR_EL0_IDC_MASK |
 			     CTR_EL0_DminLine_MASK |
 			     CTR_EL0_IminLine_MASK),
+	{ SYS_DESC(SYS_GCSPR_EL0), NULL, reset_unknown, GCSPR_EL0,
+	  .visibility = gcs_visibility },
 	{ SYS_DESC(SYS_SVCR), undef_access, reset_val, SVCR, 0, .visibility = sme_visibility  },
 	{ SYS_DESC(SYS_FPMR), undef_access, reset_val, FPMR, 0, .visibility = fp8_visibility },
 
@@ -4663,6 +4681,9 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
 
 		if (kvm_has_fpmr(kvm))
 			vcpu->arch.hcrx_el2 |= HCRX_EL2_EnFPM;
+
+		if (kvm_has_gcs(kvm))
+			vcpu->arch.hcrx_el2 |= HCRX_EL2_GCSEn;
 	}
 
 	if (test_bit(KVM_ARCH_FLAG_FGU_INITIALIZED, &kvm->arch.flags))
@@ -4716,6 +4737,14 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
 		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPOR_EL1 |
 						HFGxTR_EL2_nPOR_EL0);
 
+	if (!kvm_has_gcs(kvm)) {
+		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nGCS_EL0 |
+						HFGxTR_EL2_nGCS_EL1);
+		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_nGCSEPP |
+						HFGITR_EL2_nGCSSTR_EL1 |
+						HFGITR_EL2_nGCSPUSHM_EL1);
+	}
+
 	if (!kvm_has_feat(kvm, ID_AA64PFR0_EL1, AMU, IMP))
 		kvm->arch.fgu[HAFGRTR_GROUP] |= ~(HAFGRTR_EL2_RES0 |
 						  HAFGRTR_EL2_RES1);

-- 
2.39.2



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

* [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception
  2024-10-05 10:37 [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS Mark Brown
                   ` (2 preceding siblings ...)
  2024-10-05 10:37 ` [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests Mark Brown
@ 2024-10-05 10:37 ` Mark Brown
  2024-10-05 12:36   ` Marc Zyngier
  2024-10-05 10:37 ` [PATCH v14 5/5] KVM: selftests: arm64: Add GCS registers to get-reg-list Mark Brown
  4 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-05 10:37 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton,
	Joey Gouly, Suzuki K Poulose, Shuah Khan
  Cc: linux-arm-kernel, linux-doc, kvmarm, linux-kselftest,
	linux-kernel, Mark Brown

As per DDI 0487 RWTXBY we need to manage PSTATE.EXLOCK when entering an
exception, when the exception is entered from a lower EL the bit is cleared
while if entering from the same EL it is set to GCSCR_ELx.EXLOCKEN.
Implement this behaviour in enter_exception64().

Signed-off-by: Mark Brown <broonie@kernel.org>
---
 arch/arm64/include/uapi/asm/ptrace.h |  2 ++
 arch/arm64/kvm/hyp/exception.c       | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 0f39ba4f3efd4a8760f0fca0fbf1a2563b191c7d..9987957f4f7137bf107653b817885bb976853a83 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -37,6 +37,7 @@
 #define PSR_MODE_EL3t	0x0000000c
 #define PSR_MODE_EL3h	0x0000000d
 #define PSR_MODE_MASK	0x0000000f
+#define PSR_EL_MASK	0x0000000c
 
 /* AArch32 CPSR bits */
 #define PSR_MODE32_BIT		0x00000010
@@ -56,6 +57,7 @@
 #define PSR_C_BIT	0x20000000
 #define PSR_Z_BIT	0x40000000
 #define PSR_N_BIT	0x80000000
+#define PSR_EXLOCK_BIT 0x400000000
 
 #define PSR_BTYPE_SHIFT		10
 
diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
index 424a5107cddb5e1cdd75ef3581adef03aaadabb7..0d41b9b75cf83250b2c0d20cd82c153869efb0e4 100644
--- a/arch/arm64/kvm/hyp/exception.c
+++ b/arch/arm64/kvm/hyp/exception.c
@@ -160,6 +160,16 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
 	// PSTATE.BTYPE is set to zero upon any exception to AArch64
 	// See ARM DDI 0487E.a, pages D1-2293 to D1-2294.
 
+	// PSTATE.EXLOCK is set to 0 upon any exception to a higher
+	// EL, or to GCSCR_ELx.EXLOCKEN for an exception to the same
+	// exception level.  See ARM DDI 0487 RWTXBY, D.1.3.2 in K.a.
+	if (kvm_has_gcs(vcpu->kvm) &&
+	    (target_mode & PSR_EL_MASK) == (mode & PSR_EL_MASK)) {
+		u64 gcscr = __vcpu_read_sys_reg(vcpu, GCSCR_EL1);
+		if (gcscr & GCSCR_ELx_EXLOCKEN)
+			new |= PSR_EXLOCK_BIT;
+	}
+
 	new |= PSR_D_BIT;
 	new |= PSR_A_BIT;
 	new |= PSR_I_BIT;

-- 
2.39.2



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

* [PATCH v14 5/5] KVM: selftests: arm64: Add GCS registers to get-reg-list
  2024-10-05 10:37 [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS Mark Brown
                   ` (3 preceding siblings ...)
  2024-10-05 10:37 ` [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception Mark Brown
@ 2024-10-05 10:37 ` Mark Brown
  4 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2024-10-05 10:37 UTC (permalink / raw)
  To: Catalin Marinas, Will Deacon, Marc Zyngier, Oliver Upton,
	Joey Gouly, Suzuki K Poulose, Shuah Khan
  Cc: linux-arm-kernel, linux-doc, kvmarm, linux-kselftest,
	linux-kernel, Mark Brown, Thiago Jung Bauermann

GCS adds new registers GCSCR_EL1, GCSCRE0_EL1, GCSPR_EL1 and GCSPR_EL0. Add
these to those validated by get-reg-list.

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 tools/testing/selftests/kvm/aarch64/get-reg-list.c | 28 ++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index d43fb3f49050ba3de950d19d56b45beefec9dbeb..c17451069a15d181bb5a7efc8963290828f58c4b 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -29,6 +29,24 @@ static struct feature_id_reg feat_id_regs[] = {
 		0,
 		1
 	},
+	{
+		ARM64_SYS_REG(3, 0, 2, 5, 0),	/* GCSCR_EL1 */
+		ARM64_SYS_REG(3, 0, 0, 4, 1),	/* ID_AA64PFR1_EL1 */
+		44,
+		1
+	},
+	{
+		ARM64_SYS_REG(3, 0, 2, 5, 1),	/* GCSPR_EL1 */
+		ARM64_SYS_REG(3, 0, 0, 4, 1),	/* ID_AA64PFR1_EL1 */
+		44,
+		1
+	},
+	{
+		ARM64_SYS_REG(3, 0, 2, 5, 2),	/* GCSCRE0_EL1 */
+		ARM64_SYS_REG(3, 0, 0, 4, 1),	/* ID_AA64PFR1_EL1 */
+		44,
+		1
+	},
 	{
 		ARM64_SYS_REG(3, 0, 10, 2, 2),	/* PIRE0_EL1 */
 		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
@@ -52,6 +70,12 @@ static struct feature_id_reg feat_id_regs[] = {
 		ARM64_SYS_REG(3, 0, 0, 7, 3),	/* ID_AA64MMFR3_EL1 */
 		16,
 		1
+	},
+	{
+		ARM64_SYS_REG(3, 3, 2, 5, 1),	/* GCSPR_EL0 */
+		ARM64_SYS_REG(3, 0, 0, 4, 1),	/* ID_AA64PFR1_EL1 */
+		44,
+		1
 	}
 };
 
@@ -472,6 +496,9 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 0, 2, 0, 1),	/* TTBR1_EL1 */
 	ARM64_SYS_REG(3, 0, 2, 0, 2),	/* TCR_EL1 */
 	ARM64_SYS_REG(3, 0, 2, 0, 3),	/* TCR2_EL1 */
+	ARM64_SYS_REG(3, 0, 2, 5, 0),	/* GCSCR_EL1 */
+	ARM64_SYS_REG(3, 0, 2, 5, 1),	/* GCSPR_EL1 */
+	ARM64_SYS_REG(3, 0, 2, 5, 2),	/* GCSCRE0_EL1 */
 	ARM64_SYS_REG(3, 0, 5, 1, 0),	/* AFSR0_EL1 */
 	ARM64_SYS_REG(3, 0, 5, 1, 1),	/* AFSR1_EL1 */
 	ARM64_SYS_REG(3, 0, 5, 2, 0),	/* ESR_EL1 */
@@ -488,6 +515,7 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 0, 13, 0, 4),	/* TPIDR_EL1 */
 	ARM64_SYS_REG(3, 0, 14, 1, 0),	/* CNTKCTL_EL1 */
 	ARM64_SYS_REG(3, 2, 0, 0, 0),	/* CSSELR_EL1 */
+	ARM64_SYS_REG(3, 3, 2, 5, 1),	/* GCSPR_EL0 */
 	ARM64_SYS_REG(3, 3, 10, 2, 4),	/* POR_EL0 */
 	ARM64_SYS_REG(3, 3, 13, 0, 2),	/* TPIDR_EL0 */
 	ARM64_SYS_REG(3, 3, 13, 0, 3),	/* TPIDRRO_EL0 */

-- 
2.39.2



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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 10:37 ` [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests Mark Brown
@ 2024-10-05 11:34   ` Marc Zyngier
  2024-10-05 13:08     ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2024-10-05 11:34 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

On Sat, 05 Oct 2024 11:37:30 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> GCS introduces a number of system registers for EL1 and EL0, on systems
> with GCS we need to context switch them and expose them to VMMs to allow
> guests to use GCS.
> 
> In order to allow guests to use GCS we also need to configure
> HCRX_EL2.GCSEn, if this is not set GCS instructions will be noops and
> CHKFEAT will report GCS as disabled.  Also enable fine grained traps for
> access to the GCS registers by guests which do not have the feature
> enabled.
> 
> In order to allow userspace to control availability of the feature to
> guests we enable writability for only ID_AA64PFR1_EL1.GCS, this is a
> deliberately conservative choice to avoid errors due to oversights.
> Further fields should be made writable in future.

It appears I have accidentally dropped the branch fixing
ID_AA64PFR1_EL1.  I'll make sure this goes in as quickly as possible.


> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/asm/kvm_host.h          | 12 ++++++++++++
>  arch/arm64/include/asm/vncr_mapping.h      |  2 ++
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 31 ++++++++++++++++++++++++++++++
>  arch/arm64/kvm/sys_regs.c                  | 31 +++++++++++++++++++++++++++++-
>  4 files changed, 75 insertions(+), 1 deletion(-)
>

[...]

> @@ -4716,6 +4737,14 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
>  		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPOR_EL1 |
>  						HFGxTR_EL2_nPOR_EL0);
>  
> +	if (!kvm_has_gcs(kvm)) {
> +		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nGCS_EL0 |
> +						HFGxTR_EL2_nGCS_EL1);
> +		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_nGCSEPP |
> +						HFGITR_EL2_nGCSSTR_EL1 |
> +						HFGITR_EL2_nGCSPUSHM_EL1);

Where is the handling of traps resulting of HFGITR_EL2.nGCSSTR_EL1?

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception
  2024-10-05 10:37 ` [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception Mark Brown
@ 2024-10-05 12:36   ` Marc Zyngier
  2024-10-05 14:14     ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2024-10-05 12:36 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

On Sat, 05 Oct 2024 11:37:31 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> As per DDI 0487 RWTXBY we need to manage PSTATE.EXLOCK when entering an
> exception, when the exception is entered from a lower EL the bit is cleared
> while if entering from the same EL it is set to GCSCR_ELx.EXLOCKEN.
> Implement this behaviour in enter_exception64().
> 
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  arch/arm64/include/uapi/asm/ptrace.h |  2 ++
>  arch/arm64/kvm/hyp/exception.c       | 10 ++++++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
> index 0f39ba4f3efd4a8760f0fca0fbf1a2563b191c7d..9987957f4f7137bf107653b817885bb976853a83 100644
> --- a/arch/arm64/include/uapi/asm/ptrace.h
> +++ b/arch/arm64/include/uapi/asm/ptrace.h
> @@ -37,6 +37,7 @@
>  #define PSR_MODE_EL3t	0x0000000c
>  #define PSR_MODE_EL3h	0x0000000d
>  #define PSR_MODE_MASK	0x0000000f
> +#define PSR_EL_MASK	0x0000000c
>  
>  /* AArch32 CPSR bits */
>  #define PSR_MODE32_BIT		0x00000010
> @@ -56,6 +57,7 @@
>  #define PSR_C_BIT	0x20000000
>  #define PSR_Z_BIT	0x40000000
>  #define PSR_N_BIT	0x80000000
> +#define PSR_EXLOCK_BIT 0x400000000
>  
>  #define PSR_BTYPE_SHIFT		10
>  
> diff --git a/arch/arm64/kvm/hyp/exception.c b/arch/arm64/kvm/hyp/exception.c
> index 424a5107cddb5e1cdd75ef3581adef03aaadabb7..0d41b9b75cf83250b2c0d20cd82c153869efb0e4 100644
> --- a/arch/arm64/kvm/hyp/exception.c
> +++ b/arch/arm64/kvm/hyp/exception.c
> @@ -160,6 +160,16 @@ static void enter_exception64(struct kvm_vcpu *vcpu, unsigned long target_mode,
>  	// PSTATE.BTYPE is set to zero upon any exception to AArch64
>  	// See ARM DDI 0487E.a, pages D1-2293 to D1-2294.
>  
> +	// PSTATE.EXLOCK is set to 0 upon any exception to a higher
> +	// EL, or to GCSCR_ELx.EXLOCKEN for an exception to the same
> +	// exception level.  See ARM DDI 0487 RWTXBY, D.1.3.2 in K.a.
> +	if (kvm_has_gcs(vcpu->kvm) &&
> +	    (target_mode & PSR_EL_MASK) == (mode & PSR_EL_MASK)) {
> +		u64 gcscr = __vcpu_read_sys_reg(vcpu, GCSCR_EL1);

No, please. This only works by luck when a guest has AArch32 EL0, and
creates more havoc on a NV guest. In general, this PSR_EL_MASK creates
more problem than anything else, and doesn't fit the rest of the code.

So this needs to:
- explicitly only apply to exceptions from AArch64
- handle exception from EL2, since this helper already deals with that

The latter point of course means introducing GCSCR_EL2 (and everything
that depends on it, such as the trap handling).

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 11:34   ` Marc Zyngier
@ 2024-10-05 13:08     ` Mark Brown
  2024-10-05 13:18       ` Marc Zyngier
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-05 13:08 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 836 bytes --]

On Sat, Oct 05, 2024 at 12:34:20PM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > +	if (!kvm_has_gcs(kvm)) {
> > +		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nGCS_EL0 |
> > +						HFGxTR_EL2_nGCS_EL1);
> > +		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_nGCSEPP |
> > +						HFGITR_EL2_nGCSSTR_EL1 |
> > +						HFGITR_EL2_nGCSPUSHM_EL1);

> Where is the handling of traps resulting of HFGITR_EL2.nGCSSTR_EL1?

These will trap with an EC of 0x2d which isn't known so I was expecting
this to get handled in the same way as for example a return of false
from kvm_hyp_handle_fpsimd() for SVE when unsupported, or for the
simiarly unknown SME EC, currently.  I gather from your comment that
you're instead expecting to see an explicit exit handler for this EC
that just injects the UNDEF directly?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 13:08     ` Mark Brown
@ 2024-10-05 13:18       ` Marc Zyngier
  2024-10-05 13:48         ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2024-10-05 13:18 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

On Sat, 05 Oct 2024 14:08:39 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Sat, Oct 05, 2024 at 12:34:20PM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > +	if (!kvm_has_gcs(kvm)) {
> > > +		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nGCS_EL0 |
> > > +						HFGxTR_EL2_nGCS_EL1);
> > > +		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_nGCSEPP |
> > > +						HFGITR_EL2_nGCSSTR_EL1 |
> > > +						HFGITR_EL2_nGCSPUSHM_EL1);
> 
> > Where is the handling of traps resulting of HFGITR_EL2.nGCSSTR_EL1?
> 
> These will trap with an EC of 0x2d which isn't known so I was expecting
> this to get handled in the same way as for example a return of false
> from kvm_hyp_handle_fpsimd() for SVE when unsupported, or for the
> simiarly unknown SME EC, currently.  I gather from your comment that
> you're instead expecting to see an explicit exit handler for this EC
> that just injects the UNDEF directly?

Not just inject an UNDEF directly, but also track whether this needs
to be forwarded when the guest's HFGITR_EL2.nGCSSTR_EL1 is 0 while not
being not RES0. Basically following what the pseudocode describes.

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 13:18       ` Marc Zyngier
@ 2024-10-05 13:48         ` Mark Brown
  2024-10-05 14:02           ` Marc Zyngier
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-05 13:48 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1070 bytes --]

On Sat, Oct 05, 2024 at 02:18:57PM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:
> > On Sat, Oct 05, 2024 at 12:34:20PM +0100, Marc Zyngier wrote:

> > > Where is the handling of traps resulting of HFGITR_EL2.nGCSSTR_EL1?

> > These will trap with an EC of 0x2d which isn't known so I was expecting
> > this to get handled in the same way as for example a return of false
> > from kvm_hyp_handle_fpsimd() for SVE when unsupported, or for the
> > simiarly unknown SME EC, currently.  I gather from your comment that
> > you're instead expecting to see an explicit exit handler for this EC
> > that just injects the UNDEF directly?

> Not just inject an UNDEF directly, but also track whether this needs
> to be forwarded when the guest's HFGITR_EL2.nGCSSTR_EL1 is 0 while not
> being not RES0. Basically following what the pseudocode describes.

Ah, I see.  I'd been under the impression that the generic machinery was
supposed to handle this already using the descriptions in
emulate-nested.c and we only needed handlers for more specific actions.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 13:48         ` Mark Brown
@ 2024-10-05 14:02           ` Marc Zyngier
  2024-10-05 14:26             ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2024-10-05 14:02 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

On Sat, 05 Oct 2024 14:48:09 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Sat, Oct 05, 2024 at 02:18:57PM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> > > On Sat, Oct 05, 2024 at 12:34:20PM +0100, Marc Zyngier wrote:
> 
> > > > Where is the handling of traps resulting of HFGITR_EL2.nGCSSTR_EL1?
> 
> > > These will trap with an EC of 0x2d which isn't known so I was expecting
> > > this to get handled in the same way as for example a return of false
> > > from kvm_hyp_handle_fpsimd() for SVE when unsupported, or for the
> > > simiarly unknown SME EC, currently.  I gather from your comment that
> > > you're instead expecting to see an explicit exit handler for this EC
> > > that just injects the UNDEF directly?
> 
> > Not just inject an UNDEF directly, but also track whether this needs
> > to be forwarded when the guest's HFGITR_EL2.nGCSSTR_EL1 is 0 while not
> > being not RES0. Basically following what the pseudocode describes.
> 
> Ah, I see.  I'd been under the impression that the generic machinery was
> supposed to handle this already using the descriptions in
> emulate-nested.c and we only needed handlers for more specific actions.

From that very file:

/*
 * Map encoding to trap bits for exception reported with EC=0x18.
 [...]
 */

Everything else needs special handling.

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception
  2024-10-05 12:36   ` Marc Zyngier
@ 2024-10-05 14:14     ` Mark Brown
  2024-10-05 16:35       ` Marc Zyngier
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-05 14:14 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]

On Sat, Oct 05, 2024 at 01:36:09PM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > +	// PSTATE.EXLOCK is set to 0 upon any exception to a higher
> > +	// EL, or to GCSCR_ELx.EXLOCKEN for an exception to the same
> > +	// exception level.  See ARM DDI 0487 RWTXBY, D.1.3.2 in K.a.
> > +	if (kvm_has_gcs(vcpu->kvm) &&
> > +	    (target_mode & PSR_EL_MASK) == (mode & PSR_EL_MASK)) {
> > +		u64 gcscr = __vcpu_read_sys_reg(vcpu, GCSCR_EL1);

> No, please. This only works by luck when a guest has AArch32 EL0, and
> creates more havoc on a NV guest. In general, this PSR_EL_MASK creates
> more problem than anything else, and doesn't fit the rest of the code.

You say luck, I say careful architecture definition but sure.

> So this needs to:
> - explicitly only apply to exceptions from AArch64
> - handle exception from EL2, since this helper already deals with that

> The latter point of course means introducing GCSCR_EL2 (and everything
> that depends on it, such as the trap handling).

For clarity, which trap handling specifically?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 14:02           ` Marc Zyngier
@ 2024-10-05 14:26             ` Mark Brown
  2024-10-05 14:33               ` Marc Zyngier
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2024-10-05 14:26 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

On Sat, Oct 05, 2024 at 03:02:09PM +0100, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > Ah, I see.  I'd been under the impression that the generic machinery was
> > supposed to handle this already using the descriptions in
> > emulate-nested.c and we only needed handlers for more specific actions.

> From that very file:

> /*
>  * Map encoding to trap bits for exception reported with EC=0x18.
>  [...]
>  */

> Everything else needs special handling.

I see.  I had noticed that comment on that table but I didn't register
that the comment wound up applying to the whole file rather than being
about a specific part of the handling.  I'm a bit confused about how
things like the SVE traps I mentioned work here...

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

* Re: [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests
  2024-10-05 14:26             ` Mark Brown
@ 2024-10-05 14:33               ` Marc Zyngier
  0 siblings, 0 replies; 16+ messages in thread
From: Marc Zyngier @ 2024-10-05 14:33 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

On Sat, 05 Oct 2024 15:26:38 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Sat, Oct 05, 2024 at 03:02:09PM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > Ah, I see.  I'd been under the impression that the generic machinery was
> > > supposed to handle this already using the descriptions in
> > > emulate-nested.c and we only needed handlers for more specific actions.
> 
> > From that very file:
> 
> > /*
> >  * Map encoding to trap bits for exception reported with EC=0x18.
> >  [...]
> >  */
> 
> > Everything else needs special handling.
> 
> I see.  I had noticed that comment on that table but I didn't register
> that the comment wound up applying to the whole file rather than being
> about a specific part of the handling.  I'm a bit confused about how
> things like the SVE traps I mentioned work here...

Like all ECs, the handling starts in handle_exit.c.

	M.

-- 
Without deviation from the norm, progress is not possible.


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

* Re: [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception
  2024-10-05 14:14     ` Mark Brown
@ 2024-10-05 16:35       ` Marc Zyngier
  0 siblings, 0 replies; 16+ messages in thread
From: Marc Zyngier @ 2024-10-05 16:35 UTC (permalink / raw)
  To: Mark Brown
  Cc: Catalin Marinas, Will Deacon, Oliver Upton, Joey Gouly,
	Suzuki K Poulose, Shuah Khan, linux-arm-kernel, linux-doc, kvmarm,
	linux-kselftest, linux-kernel

On Sat, 05 Oct 2024 15:14:21 +0100,
Mark Brown <broonie@kernel.org> wrote:
> 
> On Sat, Oct 05, 2024 at 01:36:09PM +0100, Marc Zyngier wrote:
> > Mark Brown <broonie@kernel.org> wrote:
> 
> > > +	// PSTATE.EXLOCK is set to 0 upon any exception to a higher
> > > +	// EL, or to GCSCR_ELx.EXLOCKEN for an exception to the same
> > > +	// exception level.  See ARM DDI 0487 RWTXBY, D.1.3.2 in K.a.
> > > +	if (kvm_has_gcs(vcpu->kvm) &&
> > > +	    (target_mode & PSR_EL_MASK) == (mode & PSR_EL_MASK)) {
> > > +		u64 gcscr = __vcpu_read_sys_reg(vcpu, GCSCR_EL1);
> 
> > No, please. This only works by luck when a guest has AArch32 EL0, and
> > creates more havoc on a NV guest. In general, this PSR_EL_MASK creates
> > more problem than anything else, and doesn't fit the rest of the code.
> 
> You say luck, I say careful architecture definition but sure.

I wasn't talking about the architecture, but sure.

> 
> > So this needs to:
> > - explicitly only apply to exceptions from AArch64
> > - handle exception from EL2, since this helper already deals with that
> 
> > The latter point of course means introducing GCSCR_EL2 (and everything
> > that depends on it, such as the trap handling).
> 
> For clarity, which trap handling specifically?

All the traps described in the GCSCR_EL2 documentation -- I see two
control bits described in K.a, all of which needs to be propagated and
their effects handled. Similarly, GCSPR_EL2 needs to be defined.

	M.

-- 
Without deviation from the norm, progress is not possible.


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

end of thread, other threads:[~2024-10-05 16:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-05 10:37 [PATCH v14 0/5] KVM: arm64: Provide guest support for GCS Mark Brown
2024-10-05 10:37 ` [PATCH v14 1/5] KVM: arm64: Expose S1PIE to guests Mark Brown
2024-10-05 10:37 ` [PATCH v14 2/5] arm64/gcs: Ensure FGTs for EL1 GCS instructions are disabled Mark Brown
2024-10-05 10:37 ` [PATCH v14 3/5] KVM: arm64: Manage GCS access and registers for guests Mark Brown
2024-10-05 11:34   ` Marc Zyngier
2024-10-05 13:08     ` Mark Brown
2024-10-05 13:18       ` Marc Zyngier
2024-10-05 13:48         ` Mark Brown
2024-10-05 14:02           ` Marc Zyngier
2024-10-05 14:26             ` Mark Brown
2024-10-05 14:33               ` Marc Zyngier
2024-10-05 10:37 ` [PATCH v14 4/5] KVM: arm64: Set PSTATE.EXLOCK when entering an exception Mark Brown
2024-10-05 12:36   ` Marc Zyngier
2024-10-05 14:14     ` Mark Brown
2024-10-05 16:35       ` Marc Zyngier
2024-10-05 10:37 ` [PATCH v14 5/5] KVM: selftests: arm64: Add GCS registers to get-reg-list Mark Brown

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).