Skip to contentSkip to conversation

KVM: arm64: Stop advertising FEAT_LS64_ACCDATA to guests

PATCH In review Fuad Tabba 4 participants 11 messages Oct 23 – Nov 12

Subsystems (4)
lore
Share & tools
lore

Fuad Tabba

author reviewer
#0 lore raw

cover KVM: arm64: Stop advertising FEAT_LS64_ACCDATA to guests

Hi folks,

On a host with FEAT_LS64_ACCDATA, KVM advertises ST64BV0 and ACCDATA_EL1 to guests but supports neither. The first patch caps ID_AA64ISAR1_EL1.LS64 at LS64_V. A VM saved on an older kernel can carry any level up to the host's, so the patch still accepts those levels from the VMM, capped at LS64_V, and the VM restores. The second patch moves LS64 out of set_id_regs' generic table into a test of its own, since the table's invalid value is now one of those levels. It carries Cc: stable as well, because a stable tree that takes the first patch alone fails the set_id_regs selftest on an LS64_ACCDATA host.

Tested on the FVP AEM with arm_v8_7_accelerator_support_level set to 3 (LS64_ACCDATA) and 4 (LS64WB). Unpatched, on a VHE host, a guest's ST64BV0 hits the WARN in handle_other(). A second VM also reads the ACCDATA_EL1 value the first one wrote, once cpuidle.off=1 stops the FVP's idle states from powering the CPU down. Patched, on VHE, nVHE and protected-mode hosts, ST64BV0 and the ACCDATA_EL1 accesses UNDEF, and set_id_regs passes.

The cap alone would fix the bug, but a VM saved on an older kernel would then fail to restore on a patched kernel. 6685f5d572c2 ("KVM:
arm64: Disable MPAM visibility by default and ignore VMM writes") solved the same problem for MPAM: it hid the field from guests, but kept accepting the value older kernels exposed.

Based on Linux 7.3-rc3 (fd73f4a665989). Patch 2 conflicts with 00e309933879c, now in kvmarm/fixes and kvmarm/next, on the test count. The resolution is:

  -	test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
  +	test_cnt = 4 + MPAM_IDREG_TEST + MTE_IDREG_TEST + LS64_IDREG_TEST;

Cheers,
/fuad

Fuad Tabba (2):
KVM: arm64: Hide FEAT_LS64_ACCDATA from guests
KVM: arm64: selftests: Test ID_AA64ISAR1_EL1.LS64 on its own

 arch/arm64/kvm/sys_regs.c                     | 28 +++++++++--
 .../testing/selftests/kvm/arm64/set_id_regs.c | 49 ++++++++++++++++++-
 2 files changed, 71 insertions(+), 6 deletions(-)

base-commit: fd73f4a6659897191fa0d40695fe370925dd3780

Signature · 1 line
2.47.3
author reviewer
#1 lore raw

PATCH 1/2 KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Since commit 58ce78667a641 ("arm64: Add support for FEAT_{LS64, LS64_V}"), a guest sees the host's ID_AA64ISAR1_EL1.LS64, but KVM only enables the FEAT_LS64 and FEAT_LS64_V instructions in HCRX_EL2. On a host with FEAT_LS64_ACCDATA, a guest's ST64BV0 therefore traps and hits the WARN_ON_ONCE() in handle_other(). ACCDATA_EL1 isn't trapped or context-switched either, so every guest on that CPU shares it.

Rather than support FEAT_LS64_ACCDATA, which requires ACCDATA_EL1 to be context-switched, cap the field at LS64_V. The guest's ACCDATA_EL1 accesses then UNDEF. This hides FEAT_LS64WB too, as it implies FEAT_LS64_ACCDATA.

A VM saved on an older kernel can carry any level up to the host's, so keep accepting those from the VMM, capped the same way. One saved at LS64_ACCDATA or above restores at LS64_V.

Fixes 58ce78667a641 ("arm64: Add support for FEAT_{LS64, LS64_V}")
---
 arch/arm64/kvm/sys_regs.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)
arch/arm64/kvm/sys_regs.c +24 -4
index 44aae52c473d7..7801166991b12 100644
@@ -1958,6 +1958,9 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,
 				 ID_AA64ISAR1_EL1_API |
 				 ID_AA64ISAR1_EL1_GPA |
 				 ID_AA64ISAR1_EL1_GPI);
+
+		/* ACCDATA_EL1 isn't context-switched, so no FEAT_LS64_ACCDATA */
+		val = ID_REG_LIMIT_FIELD_ENUM(val, ID_AA64ISAR1_EL1, LS64, LS64_V);
 		break;
 	case SYS_ID_AA64ISAR2_EL1:
 		if (!vcpu_has_ptrauth(vcpu))
@@ -2403,6 +2406,22 @@ static int set_id_aa64pfr2_el1(struct kvm_vcpu *vcpu,
 	return set_id_reg(vcpu, rd, user_val);
 }
 
+static int set_id_aa64isar1_el1(struct kvm_vcpu *vcpu,
+				const struct sys_reg_desc *rd, u64 user_val)
+{
+	u64 hw_val = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
+	u64 ls64_mask = ID_AA64ISAR1_EL1_LS64_MASK;
+
+	/*
+	 * Older kernels let userspace set LS64 up to the host's level, so
+	 * keep accepting those values, capped at LS64_V.
+	 */
+	if ((user_val & ls64_mask) <= (hw_val & ls64_mask))
+		user_val = ID_REG_LIMIT_FIELD_ENUM(user_val, ID_AA64ISAR1_EL1, LS64, LS64_V);
+
+	return set_id_reg(vcpu, rd, user_val);
+}
+
 /*
  * Allow userspace to de-feature a stage-2 translation granule but prevent it
  * from claiming the impossible.
@@ -3428,10 +3447,11 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 
 	/* CRm=6 */
 	ID_WRITABLE(ID_AA64ISAR0_EL1, ~ID_AA64ISAR0_EL1_RES0),
-	ID_WRITABLE(ID_AA64ISAR1_EL1, ~(ID_AA64ISAR1_EL1_GPI |
-					ID_AA64ISAR1_EL1_GPA |
-					ID_AA64ISAR1_EL1_API |
-					ID_AA64ISAR1_EL1_APA)),
+	ID_FILTERED(ID_AA64ISAR1_EL1, id_aa64isar1_el1,
+		    ~(ID_AA64ISAR1_EL1_GPI |
+		      ID_AA64ISAR1_EL1_GPA |
+		      ID_AA64ISAR1_EL1_API |
+		      ID_AA64ISAR1_EL1_APA)),
 	ID_WRITABLE(ID_AA64ISAR2_EL1, ~(ID_AA64ISAR2_EL1_RES0 |
 					ID_AA64ISAR2_EL1_APA3 |
 					ID_AA64ISAR2_EL1_GPA3)),
Signature · 1 line
2.47.3
maintainer
#2 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Quoted 8 lines · Fuad Tabba

Since commit 58ce78667a641 ("arm64: Add support for FEAT_{LS64, LS64_V}"), a guest sees the host's ID_AA64ISAR1_EL1.LS64, but KVM only enables the FEAT_LS64 and FEAT_LS64_V instructions in HCRX_EL2. On a host with FEAT_LS64_ACCDATA, a guest's ST64BV0 therefore traps and hits the WARN_ON_ONCE() in handle_other(). ACCDATA_EL1 isn't trapped or context-switched either, so every guest on that CPU shares it.

Quoted 4 lines

Rather than support FEAT_LS64_ACCDATA, which requires ACCDATA_EL1 to be context-switched, cap the field at LS64_V. The guest's ACCDATA_EL1 accesses then UNDEF. This hides FEAT_LS64WB too, as it implies FEAT_LS64_ACCDATA.

Why settle for the least capable option? Context switching ACCDATA_EL1 is easy enough, can be gated on guest features, and avoid the userspace horror story.

I'd rather see that.

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
author reviewer
#3 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Hi Marc,

On Fri, 23 Oct 2026 09:31:32 +0100, Marc Zyngier <[email protected]> wrote:
[...]

Quoted 5 lines

Why settle for the least capable option? Context switching ACCDATA_EL1 is easy enough, can be gated on guest features, and avoid the userspace horror story.

I'd rather see that.

Context switching it means accessing ACCDATA_EL1 at EL2, which traps to EL3 unless SCR_EL3.ADEn is set. booting.rst doesn't require ADEn today, and TF-A only sets it when built with ENABLE_FEAT_LS64_ACCDATA, which defaults to off. On such a host, KVM traps to EL3 the first time it switches ACCDATA_EL1 for a VM, and every VM has LS64_ACCDATA by default. The cap leaves the register alone, and the fine-grained trap takes priority over ADEn, so it works whatever ADEn is set to.

Should we keep the cap as the fix for stable, and post ACCDATA support on top? If you agree, I'd rebase your patches 3, 4 and 8 from [1], add a booting.rst requirement for ADEn and EnAS0, and lift the cap and the compat.

Cheers,
/fuad

[1] https://lore.kernel.org/all/[email protected]/

maintainer
#4 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Quoted 9 lines · Fuad Tabba

Hi Marc,

On Fri, 23 Oct 2026 09:31:32 +0100, Marc Zyngier <[email protected]> wrote:
[...]

Quoted 4 lines

Why settle for the least capable option? Context switching ACCDATA_EL1 is easy enough, can be gated on guest features, and avoid the userspace horror story.

Quoted 6 lines
Quoted 1 line

I'd rather see that.

Context switching it means accessing ACCDATA_EL1 at EL2, which traps to EL3 unless SCR_EL3.ADEn is set. booting.rst doesn't require ADEn today, and TF-A only sets it when built with ENABLE_FEAT_LS64_ACCDATA, which defaults to off.

I couldn't care less.

booting.rst is not authoritative and has absolutely no value when it comes to trap bits. Firmware could perfectly enable a feature on trap instead of catching fire. Additionally, shipping firmware that doesn't support features that the CPU advertises is a bug, and I want to see these systems burn.

Quoted 9 lines

On such a host, KVM traps to EL3 the first time
it switches ACCDATA_EL1 for a VM, and every VM has LS64_ACCDATA by default. The cap leaves the register alone, and the fine-grained trap takes priority over ADEn, so it works whatever ADEn is set to.

Should we keep the cap as the fix for stable, and post ACCDATA support on top? If you agree, I'd rebase your patches 3, 4 and 8 from [1], add a booting.rst requirement for ADEn and EnAS0, and lift the cap and the compat.

No, I want this fixed by enabling ACCDATA support by default, which is what we have *already* promised to userspace, and see the fireworks. You can add an arm64.nols64 kernel option if you want, which is consistent with what we do for CNTPOFF, NV3, and other extensions that depend on FW enabling them.

Thanks,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
author reviewer
#5 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Hi Marc,

On Fri, 23 Oct 2026 11:28:17 +0100, Marc Zyngier <[email protected]> wrote:
[...]

Quoted 5 lines

No, I want this fixed by enabling ACCDATA support by default, which is what we have *already* promised to userspace, and see the fireworks. You can add an arm64.nols64 kernel option if you want, which is consistent with what we do for CNTPOFF, NV3, and other extensions that depend on FW enabling them.

Will do. v2 will have your patches 3 and 4 from [1] rebased, EnAS0 set when the guest has LS64_ACCDATA, and an arm64.nols64 option.

Cheers,
/fuad

[1] https://lore.kernel.org/all/[email protected]/

#6 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Quoted 10 lines · Fuad Tabba

On Fri, 23 Oct 2026 11:28:17 +0100, Marc Zyngier <[email protected]> wrote:
[...]

Quoted 5 lines

No, I want this fixed by enabling ACCDATA support by default, which is what we have *already* promised to userspace, and see the fireworks. You can add an arm64.nols64 kernel option if you want, which is consistent with what we do for CNTPOFF, NV3, and other extensions that depend on FW enabling them.

Will do. v2 will have your patches 3 and 4 from [1] rebased, EnAS0 set when the guest has LS64_ACCDATA, and an arm64.nols64 option.

That sounds good. For reference, I have prototyped support for LS64_ACCDATA in Linux drivers based on the corresponding x86 code. The code is not yet tested on hardware, but it may help understand how this may be used in the future.

Arnd

----
From: Arnd Bergmann <[email protected]>
Subject: [PATCH] arm64: dynamically enable st64bv0 for SVA

I/O devices that understand the st64bv0 instruction for doing shared virtual access from userspace need to set the pasid value in the accdata_el1 register, but in general we want this to be disabled unless a driver actually hands an MMIO area to userspace that accepts these commands and installs a pasid using iommu_sva_bind_device().

Rather than trying to find all threads that use an mm_struct by the time the pasid created, iommu_sva_bind_device() only stores the pasid in 'struct iommu_mm_data' and sets up the iommu domain in the device. The first attempt to use SVA from userspace traps into do_el0_other(), which checks that this is indeed valid and then sets the configured pasid before enabling SCTLR_EL1_EnAS0.

This is pretty much a lifted from the x86 try_fixup_enqcmd_gp() function.

arch/arm64/include/asm/exception.h +1 -0
index a2da3cb21c24..7c61b50718d6 100644
@@ -81,6 +81,7 @@ void do_el0_sys(unsigned long esr, struct pt_regs *regs);
 void do_sp_pc_abort(unsigned long addr, unsigned long esr, struct pt_regs *regs);
 void bad_el0_sync(struct pt_regs *regs, int reason, unsigned long esr);
 void do_el0_cp15(unsigned long esr, struct pt_regs *regs);
+void do_el0_other(struct pt_regs *regs, unsigned long esr);
 int do_compat_alignment_fixup(unsigned long addr, struct pt_regs *regs);
 void do_el0_svc(struct pt_regs *regs);
 void do_el0_svc_compat(struct pt_regs *regs);
arch/arm64/include/asm/mmu_context.h +17 -0
index 803b68758152..55965d752ad5 100644
@@ -16,6 +16,7 @@
 #include <linux/mm_types.h>
 #include <linux/pgtable.h>
 #include <linux/pkeys.h>
+#include <linux/iommu.h>
 
 #include <asm/cacheflush.h>
 #include <asm/cpufeature.h>
@@ -247,6 +248,20 @@ static inline void __switch_mm(struct mm_struct *next)
 	check_and_switch_context(next);
 }
 
+/* switch to PASID if new task has enabled st64bv0 */
+static inline void switch_iommu_mm(struct mm_struct *mm, struct task_struct *tsk)
+{
+	u32 pasid;
+
+	if (!IS_ENABLED(CONFIG_IOMMU_SVA) ||
+	    !(tsk->thread.sctlr_user & SCTLR_EL1_EnAS0))
+		return;
+
+	pasid = mm_get_enqcmd_pasid(mm);
+	if (pasid != IOMMU_PASID_INVALID)
+		write_sysreg(pasid, accdata_el1);
+}
+
 static inline void
 switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	  struct task_struct *tsk)
@@ -254,6 +269,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	if (prev != next)
 		__switch_mm(next);
 
+	switch_iommu_mm(next, tsk); /* FIXME: is there a better place to do this? */
+
 	/*
 	 * Update the saved TTBR0_EL1 of the scheduled-in task as the previous
 	 * value may have not been initialised yet (activate_mm caller) or the
arch/arm64/kernel/entry-common.c +11 -0
index 11d3f888fcd8..abeffb7f1fb2 100644
@@ -806,6 +806,14 @@ static void noinstr el0_fpac(struct pt_regs *regs, unsigned long esr)
 	arm64_exit_to_user_mode(regs);
 }
 
+static void noinstr el0_other(struct pt_regs *regs, unsigned long esr)
+{
+	arm64_enter_from_user_mode(regs);
+	local_daif_restore(DAIF_PROCCTX);
+	do_el0_other(regs, esr);
+	arm64_exit_to_user_mode(regs);
+}
+
 asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
 {
 	unsigned long esr = read_sysreg(esr_el1);
@@ -869,6 +877,9 @@ asmlinkage void noinstr el0t_64_sync_handler(struct pt_regs *regs)
 	case ESR_ELx_EC_FPAC:
 		el0_fpac(regs, esr);
 		break;
+	case ESR_ELx_EC_OTHER:
+		el0_other(regs, esr);
+		break;
 	default:
 		el0_inv(regs, esr);
 	}
arch/arm64/kernel/traps.c +51 -0

69 lines not on this page · raw message

arch/arm64/tools/sysreg +5 -0
index b15a59b9e95d..e358d9ad229b 100644
@@ -3656,6 +3656,11 @@ Sysreg	TPIDR_EL1	3	0	13	0	4
 Field	63:0	ThreadID
 EndSysreg
 
+Sysreg	ACCDATA_EL1	3	0	13	0	5
+Res0	63:32
+Field	31:0	ACCDATA
+EndSysreg
+
 Sysreg	RCWMASK_EL1	3	0	13	0	6
 Field	63:0	RCWMASK
 EndSysreg
author reviewer
#7 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Hi Arnd,

On Fri, 23 Oct 2026 15:10:23 +0100, "Arnd Bergmann" <[email protected]> wrote:
[...]

Quoted 4 lines

That sounds good. For reference, I have prototyped support for LS64_ACCDATA in Linux drivers based on the corresponding x86 code. The code is not yet tested on hardware, but it may help understand how this may be used in the future.

Thanks, that's useful to see.

[...]

Quoted 1 line
arch/arm64/include/asm/mmu_context.h +0 -0

[...]

arch/arm64/include/asm/mmu_context.h switch_mm(struct mm_struct *prev, struct mm_struct *next, · line 272

Quoted 5 lines
arch/arm64/include/asm/mmu_context.h +1 -0
@@ -254,6 +269,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
         if (prev != next)
                 __switch_mm(next);
 
+        switch_iommu_mm(next, tsk); /* FIXME: is there a better place to do this? */

switch_mm() isn't enough when the same thread also runs a vCPU. In v2, which I'll post soon, KVM will save and restore ACCDATA_EL1 only for the guest, since nothing in the host uses it today. So after KVM_RUN returns from a guest that has LS64_ACCDATA, the VMM thread's next ST64BV0 would use the guest's value, unless KVM also saves and restores the host's.

[...]

Quoted 1 line
arch/arm64/kernel/traps.c +0 -0

[...]

Quoted 1 line

+static bool try_enable_st64bv0(struct pt_regs *regs, unsigned long esr)

[...]

arch/arm64/kernel/traps.c void do_el1_mops(struct pt_regs *regs, unsigned long esr) · line 567

Quoted 3 lines
arch/arm64/kernel/traps.c +3 -0
+        isar1 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
+        if (!(isar1 & ID_AA64ISAR1_EL1_LS64_LS64_ACCDATA))
+                return false;

v2 will add an ARM64_HAS_LS64_ACCDATA cpucap, so this could test that instead. As written it tests bits [1:0] of the register, not the LS64 field, since ID_AA64ISAR1_EL1_LS64_LS64_ACCDATA is the unshifted field value.

arch/arm64/kernel/traps.c void do_el1_mops(struct pt_regs *regs, unsigned long esr) · line 573

Quoted 6 lines
arch/arm64/kernel/traps.c +6 -0
+
+        sysreg_clear_set(accdata_el1, ACCDATA_EL1_ACCDATA, pasid);
+        sctlr = current->thread.sctlr_user;
+        sctlr |= SCTLR_EL1_EnAS0;
+        current->thread.sctlr_user = sctlr;
+        update_sctlr_el1(sctlr);

On a host that runs at EL1 (nVHE or hVHE), this also requires __init_el2_fgt to set nACCDATA_EL1 in HFGRTR_EL2 and HFGWTR_EL2, and __init_el2_hcrx to set HCRX_EL2.EnAS0. Without them, the ACCDATA_EL1 access traps to EL2 and the hypervisor panics, and so does ST64BV0 once SCTLR_EL1.EnAS0 is set. Happy to write those and the KVM change once you're ready to post this.

Cheers,
/fuad

#8 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Quoted 7 lines · Fuad Tabba
Quoted 1 line · Arnd Bergmann
arch/arm64/include/asm/mmu_context.h +0 -0

[...]

Quoted 4 lines
arch/arm64/include/asm/mmu_context.h +0 -0
@@ -254,6 +269,8 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
         if (prev != next)
                 __switch_mm(next);
 
Quoted 8 lines

arch/arm64/include/asm/mmu_context.h switch_mm(struct mm_struct *prev, struct mm_struct *next, · line 272

Quoted 1 line
arch/arm64/include/asm/mmu_context.h +1 -0
+        switch_iommu_mm(next, tsk); /* FIXME: is there a better place to do this? */

switch_mm() isn't enough when the same thread also runs a vCPU. In v2, which I'll post soon, KVM will save and restore ACCDATA_EL1 only for the guest, since nothing in the host uses it today. So after KVM_RUN returns from a guest that has LS64_ACCDATA, the VMM thread's next ST64BV0 would use the guest's value, unless KVM also saves and restores the host's.

Right, makes sense.

I was a little unsure at first how ACCDATA_EL1 gets handled on PCIe TLPs when there are both host and guest users that set it. As I understand it now, this only works if the PCIe requesterID is assigned to the guest VMID for stage2 translation, so a guest st64bv0 uses the ACCDATA_EL1 as PASID in combination with that VMID. The host would just use ACCDATA_EL1/PASID for its own stage1 translation without stage2.

This means that with your v2 patch and the switch_mm() code I have, this should be safe. There is a possible optimization to skip the host ACCDATA_EL1 save/restore if current->thread.sctlr_user does not enable SCTLR_EL1_EnAS0 (i.e. the same process that runs the KVM guest has mapped the MMIO area of an SVA device). This would make a difference if ACCDATA_EL1 access traps to EL3, but otherwise likely does not much much of a difference (I'd have to do some measurements on hardware).

Quoted 4 lines
Quoted 1 line
arch/arm64/kernel/traps.c +0 -0

[...]

Quoted 1 line

+static bool try_enable_st64bv0(struct pt_regs *regs, unsigned long esr)

[...]

Quoted 8 lines

arch/arm64/kernel/traps.c void do_el1_mops(struct pt_regs *regs, unsigned long esr) · line 567

Quoted 3 lines
arch/arm64/kernel/traps.c +3 -0
+        isar1 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
+        if (!(isar1 & ID_AA64ISAR1_EL1_LS64_LS64_ACCDATA))
+                return false;

v2 will add an ARM64_HAS_LS64_ACCDATA cpucap, so this could test that instead. As written it tests bits [1:0] of the register, not the LS64 field, since ID_AA64ISAR1_EL1_LS64_LS64_ACCDATA is the unshifted field value.

Thanks for pointing it out. I have rebased on top of your patches now, so this line is gone.

Quoted 7 lines

arch/arm64/kernel/traps.c void do_el1_mops(struct pt_regs *regs, unsigned long esr) · line 573

Quoted 6 lines
arch/arm64/kernel/traps.c +6 -0
+
+        sysreg_clear_set(accdata_el1, ACCDATA_EL1_ACCDATA, pasid);
+        sctlr = current->thread.sctlr_user;
+        sctlr |= SCTLR_EL1_EnAS0;
+        current->thread.sctlr_user = sctlr;
+        update_sctlr_el1(sctlr);
Quoted 6 lines

On a host that runs at EL1 (nVHE or hVHE), this also requires __init_el2_fgt to set nACCDATA_EL1 in HFGRTR_EL2 and HFGWTR_EL2, and __init_el2_hcrx to set HCRX_EL2.EnAS0. Without them, the ACCDATA_EL1 access traps to EL2 and the hypervisor panics, and so does ST64BV0 once SCTLR_EL1.EnAS0 is set. Happy to write those and the KVM change once you're ready to post this.

Thanks for the offer, that part is definitely not my area.

Arnd

#9 lore raw

Re: [PATCH 1/2] KVM: arm64: Hide FEAT_LS64_ACCDATA from guests

Quoted 9 lines · Arnd Bergmann
Quoted 6 lines · Fuad Tabba

On a host that runs at EL1 (nVHE or hVHE), this also requires __init_el2_fgt to set nACCDATA_EL1 in HFGRTR_EL2 and HFGWTR_EL2, and __init_el2_hcrx to set HCRX_EL2.EnAS0. Without them, the ACCDATA_EL1 access traps to EL2 and the hypervisor panics, and so does ST64BV0 once SCTLR_EL1.EnAS0 is set. Happy to write those and the KVM change once you're ready to post this.

Thanks for the offer, that part is definitely not my area.

Hi Arnd, Fuad,

We are preparing to post the Microsoft Smart Data Accelerator (SDA) VF dmaengine driver for Microsoft Cobalt SoCs, which uses IOMMU SVA with direct EL0 ST64BV0 portal submissions carrying the process PASID in ACCDATA_EL1.

Now that Fuad's KVM guest FEAT_LS64_ACCDATA series is queued, are you planning to post the host ST64BV0 SVA + EL2/KVM patches as a standalone series soon?

Either way, we have Cobalt hardware with FEAT_LS64_ACCDATA available and would be happy to test your patches on silicon, or help put together and carry any remaining pieces alongside our driver series if you prefer.

Thanks,
Siddharth

author reviewer
#10 lore raw

PATCH 2/2 KVM: arm64: selftests: Test ID_AA64ISAR1_EL1.LS64 on its own

In reply to Fuad Tabba

KVM caps ID_AA64ISAR1_EL1.LS64 at LS64_V, but accepts a write of any level up to the host's and caps it. On a host at LS64_ACCDATA or above, the generic test writes the level above KVM's value and expects it to be rejected, but KVM accepts it.

Move LS64 out of the generic table into its own test. It checks that the field reads as the host's level capped at LS64_V, that a write of 0 is kept and one of the host's level is capped, and that a level above the host's is rejected.

---
 .../testing/selftests/kvm/arm64/set_id_regs.c | 49 ++++++++++++++++++-
 1 file changed, 47 insertions(+), 2 deletions(-)
tools/testing/selftests/kvm/arm64/set_id_regs.c +47 -2
index 7429a1055df56..60b96f7608dc3 100644
@@ -108,7 +108,6 @@ static const struct reg_ftr_bits ftr_id_aa64isar0_el1[] = {
 };
 
 static const struct reg_ftr_bits ftr_id_aa64isar1_el1[] = {
-	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR1_EL1, LS64, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR1_EL1, XS, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR1_EL1, I8MM, 0),
 	REG_FTR_BITS(FTR_LOWER_SAFE, ID_AA64ISAR1_EL1, DGH, 0),
@@ -644,6 +643,51 @@ static void test_user_set_mte_reg(struct kvm_vcpu *vcpu)
 		ksft_test_result_pass("ID_AA64PFR1_EL1.MTE_frac no longer 0xF\n");
 }
 
+#define LS64_IDREG_TEST 4
+static void test_user_set_ls64_reg(struct kvm_vcpu *vcpu)
+{
+	u64 reg = KVM_ARM64_SYS_REG(SYS_ID_AA64ISAR1_EL1);
+	u64 val, ls64, host, limit;
+	int err;
+
+	/* LS64 is visible to EL0, so this reads the host's sanitised level */
+	host = FIELD_GET(ID_AA64ISAR1_EL1_LS64, read_sysreg(id_aa64isar1_el1));
+	limit = min(host, ID_AA64ISAR1_EL1_LS64_LS64_V);
+
+	val = vcpu_get_reg(vcpu, reg);
+	ls64 = FIELD_GET(ID_AA64ISAR1_EL1_LS64, val);
+	if (ls64 == limit)
+		ksft_test_result_pass("ID_AA64ISAR1_EL1.LS64=%lu, host at %lu\n", ls64, host);
+	else
+		ksft_test_result_fail("ID_AA64ISAR1_EL1.LS64=%lu, host at %lu\n", ls64, host);
+
+	val &= ~ID_AA64ISAR1_EL1_LS64_MASK;
+	err = __vcpu_set_reg(vcpu, reg, val);
+	ls64 = FIELD_GET(ID_AA64ISAR1_EL1_LS64, vcpu_get_reg(vcpu, reg));
+	if (!err && !ls64)
+		ksft_test_result_pass("ID_AA64ISAR1_EL1.LS64=0 accepted\n");
+	else
+		ksft_test_result_fail("ID_AA64ISAR1_EL1.LS64=0: err %d, reads %lu\n", err, ls64);
+
+	/*
+	 * A VM saved on an older kernel carries the host's level, which KVM
+	 * must accept from the VMM but keep at the limit.
+	 */
+	err = __vcpu_set_reg(vcpu, reg, val | FIELD_PREP(ID_AA64ISAR1_EL1_LS64_MASK, host));
+	ls64 = FIELD_GET(ID_AA64ISAR1_EL1_LS64, vcpu_get_reg(vcpu, reg));
+	if (!err && ls64 == limit)
+		ksft_test_result_pass("ID_AA64ISAR1_EL1.LS64=%lu accepted as %lu\n", host, ls64);
+	else
+		ksft_test_result_fail("ID_AA64ISAR1_EL1.LS64=%lu: err %d, reads %lu\n",
+				      host, err, ls64);
+
+	err = __vcpu_set_reg(vcpu, reg, val | FIELD_PREP(ID_AA64ISAR1_EL1_LS64_MASK, host + 1));
+	if (err && errno == EINVAL)
+		ksft_test_result_pass("ID_AA64ISAR1_EL1.LS64=%lu rejected\n", host + 1);
+	else
+		ksft_test_result_fail("ID_AA64ISAR1_EL1.LS64=%lu not rejected\n", host + 1);
+}
+
 static u64 reset_mutable_bits(u32 id, u64 val)
 {
 	struct test_feature_reg *reg = NULL;
@@ -828,7 +872,7 @@ int main(void)
 
 	ksft_print_header();
 
-	test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST;
+	test_cnt = 3 + MPAM_IDREG_TEST + MTE_IDREG_TEST + LS64_IDREG_TEST;
 	for (i = 0; i < ARRAY_SIZE(test_regs); i++)
 		for (j = 0; test_regs[i].ftr_bits[j].type != FTR_END; j++)
 			test_cnt++;
@@ -840,6 +884,7 @@ int main(void)
 	test_vcpu_non_ftr_id_regs(vcpu);
 	test_user_set_mpam_reg(vcpu);
 	test_user_set_mte_reg(vcpu);
+	test_user_set_ls64_reg(vcpu);
 
 	test_guest_reg_read(vcpu);
 
Signature · 1 line
2.47.3