KVM: arm64: Add EL2 support to FEAT_S1PIE
Marc Zyngier
maintainerv2 cover KVM: arm64: Add EL2 support to FEAT_S1PIE
This series serves a few purposes:
- Complete the S1PIE support to include EL2
- Sneak in the EL2 system register world switch
As mentioned in few of the patches, this implementation relies on a very recent fix to the architecture (D22677 in [0]).
This series has seen a few changes since the initial posting [1]:
- Rebased on top of the AT support branch, which is currently sitting in kvmarm/next
- Add handling for S1 indirect permission in AT, which I'm sure will give Alexandru another king-sized headache
- Picked Mark Brown's series [2] dealing with TCRX and S1PIE visibility, and slapped an extra fix on top for good measure
- Picked up RBs from Joey, with thanks.
[0] https://developer.arm.com/documentation/102105/ka-04/
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
Marc Zyngier (13):
KVM: arm64: nv: Handle CNTHCTL_EL2 specially
KVM: arm64: nv: Save/Restore vEL2 sysregs
KVM: arm64: Add TCR2_EL2 to the sysreg arrays
KVM: arm64: Add save/restore for TCR2_EL2
arm64: Add encoding for PIRE0_EL2
arm64: Remove VNCR definition for PIRE0_EL2
KVM: arm64: Add PIR{,E0}_EL2 to the sysreg arrays
KVM: arm64: Add save/restore for PIR{,E0}_EL2
KVM: arm64: Handle PIR{,E0}_EL2 traps
KVM: arm64: Sanitise ID_AA64MMFR3_EL1
KVM: arm64: Split S1 permission evaluation into direct and
hierarchical parts
KVM: arm64: Implement AT S1PIE support
KVM: arm64: Rely on visibility to let PIR*_ELx/TCR2_ELx UNDEF
Mark Brown (3):
KVM: arm64: Define helper for EL2 registers with custom visibility
KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests
KVM: arm64: Hide S1PIE registers from userspace when disabled for
guests
arch/arm64/include/asm/kvm_host.h | 9 + arch/arm64/include/asm/vncr_mapping.h | 1 - arch/arm64/kvm/at.c | 296 ++++++++++++++++----- arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 5 +- arch/arm64/kvm/hyp/nvhe/sysreg-sr.c | 2 +- arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 155 ++++++++++- arch/arm64/kvm/sys_regs.c | 114 +++++++- arch/arm64/tools/sysreg | 4 + include/kvm/arm_arch_timer.h | 3 + 9 files changed, 505 insertions(+), 84 deletions(-)
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 1/16 KVM: arm64: nv: Handle CNTHCTL_EL2 specially
Accessing CNTHCTL_EL2 is fraught with danger if running with HCR_EL2.E2H=1: half of the bits are held in CNTKCTL_EL1, and thus can be changed behind our back, while the rest lives in the CNTHCTL_EL2 shadow copy that is memory-based.
Yes, this is a lot of fun!
Make sure that we merge the two on read access, while we can write to CNTKCTL_EL1 in a more straightforward manner.
--- arch/arm64/kvm/sys_regs.c | 28 ++++++++++++++++++++++++++++ include/kvm/arm_arch_timer.h | 3 +++ 2 files changed, 31 insertions(+)
arch/arm64/kvm/sys_regs.c +28 -0
@@ -140,6 +140,21 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)if (!is_hyp_ctxt (vcpu)) goto memory_read; +/* +* CNTHCTL_EL2 requires some special treatment to +* account for the bits that can be set via CNTKCTL_EL1. +*/ +switch (reg) { +case CNTHCTL_EL2 : +if (vcpu_el2_e2h_is_set (vcpu)) { + val =read_sysreg_el1 (SYS_CNTKCTL ); + val &=CNTKCTL_VALID_BITS ; + val |=__vcpu_sys_reg (vcpu, reg) & ~CNTKCTL_VALID_BITS ; +return val; + } +break ; + } +/* * If this register does not have an EL1 counterpart, * then read the stored EL2 version.
@@ -190,6 +205,19 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)*/ __vcpu_sys_reg (vcpu, reg) = val; +switch (reg) { +case CNTHCTL_EL2 : +/* +* If E2H=0, CNHTCTL_EL2 is a pure shadow register. +* Otherwise, some of the bits are backed by +* CNTKCTL_EL1, while the rest is kept in memory. +* Yes, this is fun stuff. +*/ +if (vcpu_el2_e2h_is_set (vcpu)) +write_sysreg_el1 (val,SYS_CNTKCTL ); +return ; + } +/* No EL1 counterpart? We're done here.? */ if (reg == el1r)return ;
include/kvm/arm_arch_timer.h +3 -0
@@ -147,6 +147,9 @@ u64 timer_get_cval(struct arch_timer_context *ctxt);void kvm_timer_cpu_up (void );void kvm_timer_cpu_down (void ); +/* CNTKCTL_EL1 valid bits as of DDI0487J.a */ +#define CNTKCTL_VALID_BITS (BIT(17) | GENMASK_ULL(9, 0)) +static inline bool has_cntpoff (void ) {return (has_vhe () &&cpus_have_final_cap (ARM64_HAS_ECV_CNTPOFF ));
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 2/16 KVM: arm64: nv: Save/Restore vEL2 sysregs
In reply to Marc ZyngierWhenever we need to restore the guest's system registers to the CPU, we now need to take care of the EL2 system registers as well. Most of them are accessed via traps only, but some have an immediate effect and also a guest running in VHE mode would expect them to be accessible via their EL1 encoding, which we do not trap.
For vEL2 we write the virtual EL2 registers with an identical format directly into their EL1 counterpart, and translate the few registers that have a different format for the same effect on the execution when running a non-VHE guest guest hypervisor.
Based on an initial patch from Andre Przywara, rewritten many times since.
--- arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 5 +- arch/arm64/kvm/hyp/nvhe/sysreg-sr.c | 2 +- arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 137 ++++++++++++++++++++- 3 files changed, 139 insertions(+), 5 deletions(-)
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +3 -2
@@ -128,9 +128,10 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)write_sysreg (ctxt_sys_reg (ctxt,TPIDRRO_EL0 ), tpidrro_el0); } -static inline void __sysreg_restore_el1_state (struct kvm_cpu_context *ctxt) +static inline void __sysreg_restore_el1_state (struct kvm_cpu_context *ctxt, +u64 mpidr) { -write_sysreg (ctxt_sys_reg (ctxt,MPIDR_EL1 ), vmpidr_el2); +write_sysreg (mpidr, vmpidr_el2);if (has_vhe () || !cpus_have_final_cap (ARM64_WORKAROUND_SPECULATIVE_AT )) {
arch/arm64/kvm/hyp/nvhe/sysreg-sr.c +1 -1
@@ -28,7 +28,7 @@ void __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)void __sysreg_restore_state_nvhe (struct kvm_cpu_context *ctxt) { -__sysreg_restore_el1_state (ctxt); +__sysreg_restore_el1_state (ctxt,ctxt_sys_reg (ctxt,MPIDR_EL1 ));__sysreg_restore_common_state (ctxt);__sysreg_restore_user_state (ctxt);__sysreg_restore_el2_return_state (ctxt);
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +135 -2
166 lines not on this page · raw message
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 3/16 KVM: arm64: Add TCR2_EL2 to the sysreg arrays
In reply to Marc ZyngierAdd the TCR2_EL2 register to the per-vcpu sysreg register array, as well as the sysreg descriptor array.
Access to this register is conditional based on ID_AA64MMFR3_EL1.TCRX being advertised.
--- arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kvm/sys_regs.c | 13 +++++++++++++ 2 files changed, 14 insertions(+)
arch/arm64/include/asm/kvm_host.h +1 -0
@@ -462,6 +462,7 @@ enum vcpu_sysreg {TTBR0_EL2 ,/* Translation Table Base Register 0 (EL2) */ TTBR1_EL2 ,/* Translation Table Base Register 1 (EL2) */ TCR_EL2 ,/* Translation Control Register (EL2) */ +TCR2_EL2 ,/* Extended Translation Control Register (EL2) */ SPSR_EL2 ,/* EL2 saved program status register */ ELR_EL2 ,/* EL2 exception link register */ AFSR0_EL2, /* Auxiliary Fault Status Register 0 (EL2) */
arch/arm64/kvm/sys_regs.c +13 -0
@@ -436,6 +436,18 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,return true; } +static bool access_tcr2_el2 (struct kvm_vcpu *vcpu, +struct sys_reg_params *p, +const struct sys_reg_desc *r) +{ +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,TCRX ,IMP )) { +kvm_inject_undefined (vcpu); +return false; + } + +return access_rw (vcpu, p, r); +} +static bool access_actlr (struct kvm_vcpu *vcpu,struct sys_reg_params *p,const struct sys_reg_desc *r)
@@ -2783,6 +2795,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG (TTBR0_EL2 , access_rw, reset_val,0 ),EL2_REG (TTBR1_EL2 , access_rw, reset_val,0 ),EL2_REG (TCR_EL2 , access_rw, reset_val,TCR_EL2_RES1 ), +EL2_REG (TCR2_EL2 , access_tcr2_el2, reset_val,TCR2_EL2_RES1 ),EL2_REG_VNCR (VTTBR_EL2 , reset_val,0 ), EL2_REG_VNCR(VTCR_EL2, reset_val, 0),
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 4/16 KVM: arm64: Add save/restore for TCR2_EL2
In reply to Marc ZyngierLike its EL1 equivalent, TCR2_EL2 gets context-switched. This is made conditional on FEAT_TCRX being adversised.
--- arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +6 -0
@@ -51,6 +51,9 @@ static void __sysreg_save_vel2_state(struct kvm_cpu_context *ctxt)ctxt_sys_reg (ctxt,TTBR1_EL2 ) =read_sysreg_el1 (SYS_TTBR1 );ctxt_sys_reg (ctxt,TCR_EL2 ) =read_sysreg_el1 (SYS_TCR ); +if (ctxt_has_tcrx (ctxt)) +ctxt_sys_reg (ctxt,TCR2_EL2 ) =read_sysreg_el1 (SYS_TCR2 ); +/* * The EL1 view of CNTKCTL_EL1 has a bunch of RES0 bits where * the interesting CNTHCTL_EL2 bits live. So preserve these
@@ -108,6 +111,9 @@ static void __sysreg_restore_vel2_state(struct kvm_cpu_context *ctxt)write_sysreg_el1 (val,SYS_TCR ); } +if (ctxt_has_tcrx (ctxt)) +write_sysreg_el1 (ctxt_sys_reg (ctxt,TCR2_EL2 ),SYS_TCR2 ); +write_sysreg_el1 (ctxt_sys_reg (ctxt,ESR_EL2 ),SYS_ESR );write_sysreg_el1 (ctxt_sys_reg (ctxt,AFSR0_EL2 ),SYS_AFSR0 );write_sysreg_el1 (ctxt_sys_reg (ctxt,AFSR1_EL2 ),SYS_AFSR1 );
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 5/16 arm64: Add encoding for PIRE0_EL2
In reply to Marc ZyngierPIRE0_EL2 is the equivalent of PIRE0_EL1 for the EL2&0 translation regime, and it is sorely missing from the sysreg file.
Add the sucker.
--- arch/arm64/tools/sysreg | 4 ++++ 1 file changed, 4 insertions(+)
arch/arm64/tools/sysreg +4 -0
@@ -2853,6 +2853,10 @@ Sysreg PIRE0_EL12 3 5 10 2 2 Fields PIRx_ELx EndSysreg +Sysreg PIRE0_EL2 3 4 10 2 2 +Fields PIRx_ELx +EndSysreg + Sysreg PIR_EL1 3 0 10 2 3 Fields PIRx_ELx EndSysreg
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 6/16 arm64: Remove VNCR definition for PIRE0_EL2
In reply to Marc ZyngierAs of the ARM ARM Known Issues document 102105_K.a_04_en, D22677 fixes a problem with the PIRE0_EL2 register, resulting in its removal from the VNCR page (it had no purpose being there the first place).
Follow the architecture update by removing this offset.
--- arch/arm64/include/asm/vncr_mapping.h | 1 - 1 file changed, 1 deletion(-)
arch/arm64/include/asm/vncr_mapping.h +0 -1
@@ -50,7 +50,6 @@#define VNCR_VBAR_EL1 0x250#define VNCR_TCR2_EL1 0x270#define VNCR_PIRE0_EL1 0x290 -#define VNCR_PIRE0_EL2 0x298#define VNCR_PIR_EL1 0x2A0#define VNCR_ICH_LR0_EL2 0x400#define VNCR_ICH_LR1_EL2 0x408
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 7/16 KVM: arm64: Add PIR{,E0}_EL2 to the sysreg arrays
In reply to Marc ZyngierAdd the FEAT_S1PIE EL2 registers to the per-vcpu sysreg register array.
--- arch/arm64/include/asm/kvm_host.h | 2 ++ 1 file changed, 2 insertions(+)
arch/arm64/include/asm/kvm_host.h +2 -0
@@ -463,6 +463,8 @@ enum vcpu_sysreg {TTBR1_EL2 ,/* Translation Table Base Register 1 (EL2) */ TCR_EL2 ,/* Translation Control Register (EL2) */ TCR2_EL2 ,/* Extended Translation Control Register (EL2) */ +PIRE0_EL2 ,/* Permission Indirection Register 0 (EL2) */ +PIR_EL2 ,/* Permission Indirection Register 1 (EL2) */ SPSR_EL2 ,/* EL2 saved program status register */ ELR_EL2 ,/* EL2 exception link register */ AFSR0_EL2, /* Auxiliary Fault Status Register 0 (EL2) */
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 8/16 KVM: arm64: Add save/restore for PIR{,E0}_EL2
In reply to Marc ZyngierLike their EL1 equivalent, the EL2-specific FEAT_S1PIE registers are context-switched. This is made conditional on both FEAT_TCRX and FEAT_S1PIE being adversised.
Note that this change only makes sense if read together with the issue D22677 contained in 102105_K.a_04_en.
--- arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +14 -2
@@ -51,9 +51,15 @@ static void __sysreg_save_vel2_state(struct kvm_cpu_context *ctxt)ctxt_sys_reg (ctxt,TTBR1_EL2 ) =read_sysreg_el1 (SYS_TTBR1 );ctxt_sys_reg (ctxt,TCR_EL2 ) =read_sysreg_el1 (SYS_TCR ); -if (ctxt_has_tcrx (ctxt)) +if (ctxt_has_tcrx (ctxt)) {ctxt_sys_reg (ctxt,TCR2_EL2 ) =read_sysreg_el1 (SYS_TCR2 ); +if (ctxt_has_s1pie (ctxt)) { +ctxt_sys_reg (ctxt,PIRE0_EL2 ) =read_sysreg_el1 (SYS_PIRE0 ); +ctxt_sys_reg (ctxt,PIR_EL2 ) =read_sysreg_el1 (SYS_PIR ); + } + } +/* * The EL1 view of CNTKCTL_EL1 has a bunch of RES0 bits where * the interesting CNTHCTL_EL2 bits live. So preserve these
@@ -111,9 +117,15 @@ static void __sysreg_restore_vel2_state(struct kvm_cpu_context *ctxt)write_sysreg_el1 (val,SYS_TCR ); } -if (ctxt_has_tcrx (ctxt)) +if (ctxt_has_tcrx (ctxt)) {write_sysreg_el1 (ctxt_sys_reg (ctxt,TCR2_EL2 ),SYS_TCR2 ); +if (ctxt_has_s1pie (ctxt)) { +write_sysreg_el1 (ctxt_sys_reg (ctxt,PIR_EL2 ),SYS_PIR ); +write_sysreg_el1 (ctxt_sys_reg (ctxt,PIRE0_EL2 ),SYS_PIRE0 ); + } + } +write_sysreg_el1 (ctxt_sys_reg (ctxt,ESR_EL2 ),SYS_ESR );write_sysreg_el1 (ctxt_sys_reg (ctxt,AFSR0_EL2 ),SYS_AFSR0 );write_sysreg_el1 (ctxt_sys_reg (ctxt,AFSR1_EL2 ),SYS_AFSR1 );
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 9/16 KVM: arm64: Handle PIR{,E0}_EL2 traps
In reply to Marc ZyngierAdd the FEAT_S1PIE EL2 registers the sysreg descriptor array so that they can be handled as a trap.
Access to these registers is conditional based on ID_AA64MMFR3_EL1.S1PIE being advertised.
Similarly to other other changes, PIRE0_EL2 is guaranteed to trap thanks to the D22677 update to the architecture.
--- arch/arm64/kvm/sys_regs.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
arch/arm64/kvm/sys_regs.c +14 -0
@@ -346,6 +346,18 @@ static bool access_rw(struct kvm_vcpu *vcpu,return true; } +static bool check_s1pie_access_rw (struct kvm_vcpu *vcpu, +struct sys_reg_params *p, +const struct sys_reg_desc *r) +{ +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) { +kvm_inject_undefined (vcpu); +return false; + } + +return access_rw (vcpu, p, r); +} +/* * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized). */
@@ -2827,6 +2839,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG (HPFAR_EL2 , access_rw, reset_val,0 ),EL2_REG (MAIR_EL2 , access_rw, reset_val,0 ), +EL2_REG (PIRE0_EL2 , check_s1pie_access_rw, reset_val,0 ), +EL2_REG (PIR_EL2 , check_s1pie_access_rw, reset_val,0 ),EL2_REG (AMAIR_EL2 , access_rw, reset_val,0 ), EL2_REG(VBAR_EL2, access_rw, reset_val, 0),
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 10/16 KVM: arm64: Sanitise ID_AA64MMFR3_EL1
In reply to Marc ZyngierAdd the missing sanitisation of ID_AA64MMFR3_EL1, making sure we solely expose S1PIE and TCRX (we currently don't support anything else).
--- arch/arm64/kvm/sys_regs.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm64/kvm/sys_regs.c +5 -1
@@ -1608,6 +1608,9 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,case SYS_ID_AA64MMFR2_EL1 : val &= ~ID_AA64MMFR2_EL1_CCIDX_MASK ;break ; +case SYS_ID_AA64MMFR3_EL1 : + val &=ID_AA64MMFR3_EL1_TCRX |ID_AA64MMFR3_EL1_S1PIE ; +break ;case SYS_ID_MMFR4_EL1 : val &= ~ARM64_FEATURE_MASK (ID_MMFR4_EL1_CCIDX );break ;
@@ -2470,7 +2473,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {ID_AA64MMFR2_EL1_IDS |ID_AA64MMFR2_EL1_NV | ID_AA64MMFR2_EL1_CCIDX)), -ID_SANITISED (ID_AA64MMFR3_EL1 ), +ID_WRITABLE (ID_AA64MMFR3_EL1 , (ID_AA64MMFR3_EL1_TCRX | +ID_AA64MMFR3_EL1_S1PIE )),ID_SANITISED (ID_AA64MMFR4_EL1 ),ID_UNALLOCATED (7 ,5 ), ID_UNALLOCATED(7,6),
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 11/16 KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts
In reply to Marc ZyngierThe AArch64.S1DirectBasePermissions() pseudocode deals with both direct and hierarchical S1 permission evaluation. While this is probably convenient in the pseudocode, we would like a bit more flexibility to slot things like indirect permissions.
To that effect, split the two permission check parts out of handle_at_slow() and into their own functions. The permissions are passed around in a new s1_perms type that contains the individual permissions across the flow.
--- arch/arm64/kvm/at.c | 164 ++++++++++++++++++++++++++------------------ 1 file changed, 99 insertions(+), 65 deletions(-)
arch/arm64/kvm/at.c +99 -65
@@ -47,6 +47,10 @@ struct s1_walk_result {bool failed; }; +struct s1_perms { +bool ur, uw, ux, pr, pw, px; +}; +static void fail_s1_walk (struct s1_walk_result *wr,u8 fst,bool ptw,bool s2) { wr->fst = fst;
@@ -747,111 +751,141 @@ static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)return sctlr &SCTLR_EL1_EPAN ; } -static u64 handle_at_slow (struct kvm_vcpu *vcpu,u32 op,u64 vaddr) +static void compute_s1_direct_permissions (struct kvm_vcpu *vcpu, +struct s1_walk_info *wi, +struct s1_walk_result *wr, +struct s1_perms *s1p) { -bool perm_fail, ur, uw, ux, pr, pw, px; -struct s1_walk_result wr = {}; -struct s1_walk_info wi = {}; -int ret, idx; - - ret =setup_s1_walk (vcpu, op, &wi, &wr, vaddr); -if (ret) - goto compute_par; - -if (wr.level ==S1_MMU_DISABLED ) - goto compute_par; - - idx =srcu_read_lock (&vcpu->kvm->srcu); - - ret =walk_s1 (vcpu, &wi, &wr, vaddr); - -srcu_read_unlock (&vcpu->kvm->srcu, idx); - -if (ret) - goto compute_par; - -/* FIXME: revisit when adding indirect permission support */ -/* AArch64.S1DirectBasePermissions() */ -if (wi.regime !=TR_EL2 ) { -switch (FIELD_GET (PTE_USER |PTE_RDONLY , wr.desc)) { +/* Non-hierarchical part of AArch64.S1DirectBasePermissions() */ +if (wi->regime !=TR_EL2 ) { +switch (FIELD_GET (PTE_USER |PTE_RDONLY , wr->desc)) {case 0b00 : - pr = pw = true; - ur = uw = false; + s1p->pr = s1p->pw = true; + s1p->ur = s1p->uw = false;break ;case 0b01 : - pr = pw = ur = uw = true; + s1p->pr = s1p->pw = s1p->ur = s1p->uw = true;break ;case 0b10 : - pr = true; - pw = ur = uw = false; + s1p->pr = true; + s1p->pw = s1p->ur = s1p->uw = false;break ;case 0b11 : - pr = ur = true; - pw = uw = false; + s1p->pr = s1p->ur = true; + s1p->pw = s1p->uw = false;break ; } -switch (wr.APTable) { +/* We don't use px for anything yet, but hey... */ + s1p->px = !((wr->desc &PTE_PXN ) || s1p->uw); + s1p->ux = !(wr->desc &PTE_UXN ); + }else { + s1p->ur = s1p->uw = s1p->ux = false; + +if (!(wr->desc &PTE_RDONLY )) { + s1p->pr = s1p->pw = true; + }else { + s1p->pr = true; + s1p->pw = false; + } + +/* XN maps to UXN */ + s1p->px = !(wr->desc &PTE_UXN ); + } +} + +static void compute_s1_hierarchical_permissions (struct kvm_vcpu *vcpu, +struct s1_walk_info *wi, +struct s1_walk_result *wr, +struct s1_perms *s1p) +{ +/* Hierarchical part of AArch64.S1DirectBasePermissions() */ +if (wi->regime !=TR_EL2 ) { +switch (wr->APTable) {case 0b00 :break ;case 0b01 : - ur = uw = false; + s1p->ur = s1p->uw = false;break ;case 0b10 : - pw = uw = false; + s1p->pw = s1p->uw = false;break ;case 0b11 : - pw = ur = uw = false; + s1p->pw = s1p->ur = s1p->uw = false;break ; } -/* We don't use px for anything yet, but hey... */ - px = !((wr.desc &PTE_PXN ) || wr.PXNTable || uw); - ux = !((wr.desc &PTE_UXN ) || wr.UXNTable); - -if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) { -bool pan; - - pan = *vcpu_cpsr (vcpu) &PSR_PAN_BIT ; - pan &= ur || uw || (pan3_enabled (vcpu, wi.regime) && ux); - pw &= !pan; - pr &= !pan; - } + s1p->px &= !wr->PXNTable; + s1p->ux &= !wr->UXNTable; }else { - ur = uw = ux = false; +if (wr->APTable &BIT (1 )) + s1p->pw = false; -if (!(wr.desc &PTE_RDONLY )) { - pr = pw = true; - }else { - pr = true; - pw = false; - } +/* XN maps to UXN */ + s1p->px &= !wr->UXNTable; + } +} -if (wr.APTable &BIT (1 )) - pw = false; +static void compute_s1_permissions (struct kvm_vcpu *vcpu,u32 op, +struct s1_walk_info *wi, +struct s1_walk_result *wr, +struct s1_perms *s1p) +{ +compute_s1_direct_permissions (vcpu, wi, wr, s1p); +compute_s1_hierarchical_permissions (vcpu, wi, wr, s1p); -/* XN maps to UXN */ - px = !((wr.desc &PTE_UXN ) || wr.UXNTable); +if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) { +bool pan; + + pan = *vcpu_cpsr (vcpu) &PSR_PAN_BIT ; + pan &= s1p->ur || s1p->uw || (pan3_enabled (vcpu, wi->regime) && s1p->ux); + s1p->pw &= !pan; + s1p->pr &= !pan; } +} + +static u64 handle_at_slow (struct kvm_vcpu *vcpu,u32 op,u64 vaddr) +{ +struct s1_walk_result wr = {}; +struct s1_walk_info wi = {}; +struct s1_perms s1p = {}; +bool perm_fail = false; +int ret, idx; + + ret =setup_s1_walk (vcpu, op, &wi, &wr, vaddr); +if (ret) + goto compute_par; + +if (wr.level ==S1_MMU_DISABLED ) + goto compute_par; + + idx =srcu_read_lock (&vcpu->kvm->srcu); + + ret =walk_s1 (vcpu, &wi, &wr, vaddr); + +srcu_read_unlock (&vcpu->kvm->srcu, idx); + +if (ret) + goto compute_par; - perm_fail = false; +compute_s1_permissions (vcpu, op, &wi, &wr, &s1p);switch (op) {case OP_AT_S1E1RP :case OP_AT_S1E1R :case OP_AT_S1E2R : - perm_fail = !pr; + perm_fail = !s1p.pr;break ;case OP_AT_S1E1WP :case OP_AT_S1E1W :case OP_AT_S1E2W : - perm_fail = !pw; + perm_fail = !s1p.pw;break ;case OP_AT_S1E0R : - perm_fail = !ur; + perm_fail = !s1p.ur;break ;case OP_AT_S1E0W : - perm_fail = !uw; + perm_fail = !s1p.uw;break ;case OP_AT_S1E1A :case OP_AT_S1E2A :
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 12/16 KVM: arm64: Implement AT S1PIE support
In reply to Marc ZyngierIt doesn't take much effort to imple,emt S1PIE support in AT. This is only a matter of using the AArch64.S1IndirectBasePermissions() encodings for the permission, ignoring GCS which has no impact on AT, and enforce FEAT_PAN3 being enabled as this is a requirement of FEAT_S1PIE.
--- arch/arm64/kvm/at.c | 136 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-)
arch/arm64/kvm/at.c +135 -1
@@ -736,6 +736,23 @@ static u64 compute_par_s1(struct kvm_vcpu *vcpu, struct s1_walk_result *wr,return par; } +static bool s1pie_enabled (struct kvm_vcpu *vcpu,enum trans_regime regime) +{ +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) +return false; + +switch (regime) { +case TR_EL2 : +case TR_EL20 : +return __vcpu_sys_reg (vcpu,TCR2_EL2 ) &TCR2_EL2_PIE ; +case TR_EL10 : +return (__vcpu_sys_reg (vcpu,HCRX_EL2 ) & HCRX_EL2_TCR2En) && + (__vcpu_sys_reg (vcpu,TCR2_EL1 ) & TCR2_EL1x_PIE); +default : +BUG (); + } +} +static bool pan3_enabled (struct kvm_vcpu *vcpu,enum trans_regime regime) {u64 sctlr;
@@ -743,6 +760,9 @@ static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR1_EL1 ,PAN ,PAN3 ))return false; +if (s1pie_enabled (vcpu, regime)) +return true; +if (regime ==TR_EL10 ) sctlr =vcpu_read_sys_reg (vcpu,SCTLR_EL1 );else
@@ -826,12 +846,126 @@ static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, } } +#define pi_idx (v, r, i) ((__vcpu_sys_reg((v), (r)) >> ((i) * 4)) & 0xf) + +#define set_priv_perms (p, r, w, x) \ + do { \ + (p)->pr = (r); \ + (p)->pw = (w); \ + (p)->px = (x); \ + } while (0) + +#define set_unpriv_perms (p, r, w, x) \ + do { \ + (p)->ur = (r); \ + (p)->uw = (w); \ + (p)->ux = (x); \ + } while (0) + +/* Similar to AArch64.S1IndirectBasePermissions(), without GCS */ +#define set_perms (w, p, ip) \ + do { \ + switch ((ip)) { \ + case 0b0000: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b0001: \ + set_ ## w ## _perms((p), true , false, false); \ + break; \ + case 0b0010: \ + set_ ## w ## _perms((p), false, false, true ); \ + break; \ + case 0b0011: \ + set_ ## w ## _perms((p), true , false, true ); \ + break; \ + case 0b0100: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b0101: \ + set_ ## w ## _perms((p), true , true , false); \ + break; \ + case 0b0110: \ + set_ ## w ## _perms((p), true , true , true ); \ + break; \ + case 0b0111: \ + set_ ## w ## _perms((p), true , true , true ); \ + break; \ + case 0b1000: \ + set_ ## w ## _perms((p), true , false, false); \ + break; \ + case 0b1001: \ + set_ ## w ## _perms((p), true , false, false); \ + break; \ + case 0b1010: \ + set_ ## w ## _perms((p), true , false, true ); \ + break; \ + case 0b1011: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b1100: \ + set_ ## w ## _perms((p), true , true , false); \ + break; \ + case 0b1101: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b1110: \ + set_ ## w ## _perms((p), true , true , true ); \ + break; \ + case 0b1111: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + } \ + } while (0) + +static void compute_s1_indirect_permissions (struct kvm_vcpu *vcpu, +struct s1_walk_info *wi, +struct s1_walk_result *wr, +struct s1_perms *s1p) +{ +u8 up, pp, idx; + + idx = (FIELD_GET (GENMASK (54 ,53 ), wr->desc) <<2 | +FIELD_GET (BIT (51 ), wr->desc) <<1 | +FIELD_GET (BIT (6 ), wr->desc)); + +switch (wi->regime) { +case TR_EL10 : + pp =pi_idx (vcpu,PIR_EL1 , idx); + up =pi_idx (vcpu,PIRE0_EL1 , idx); +break ; +case TR_EL20 : + pp =pi_idx (vcpu,PIR_EL2 , idx); + up =pi_idx (vcpu,PIRE0_EL2 , idx); +break ; +case TR_EL2 : + pp =pi_idx (vcpu,PIR_EL2 , idx); + up =0 ; +break ; + } + +set_perms (priv, s1p, pp); + +if (wi->regime !=TR_EL2 ) +set_perms (unpriv, s1p, up); +else +set_unpriv_perms (s1p, false, false, false); + +if (s1p->px && s1p->uw) { +set_priv_perms (s1p, false, false, false); +set_unpriv_perms (s1p, false, false, false); + } +} +static void compute_s1_permissions (struct kvm_vcpu *vcpu,u32 op,struct s1_walk_info *wi,struct s1_walk_result *wr,struct s1_perms *s1p) { -compute_s1_direct_permissions (vcpu, wi, wr, s1p); +if (!s1pie_enabled (vcpu, wi->regime)) +compute_s1_direct_permissions (vcpu, wi, wr, s1p); +else +compute_s1_indirect_permissions (vcpu, wi, wr, s1p); +compute_s1_hierarchical_permissions (vcpu, wi, wr, s1p);if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) {
Signature · 1 line
2.39.2
Joey Gouly
reviewerRe: [PATCH v2 12/16] KVM: arm64: Implement AT S1PIE support
Hello Marc!
Quoted 165 lines · Marc Zyngier
It doesn't take much effort to imple,emt S1PIE support in AT. This is only a matter of using the
AArch64.S1IndirectBasePermissions()encodings for the permission, ignoring GCS which has no impact on AT, and enforceFEAT_PAN3being enabled as this is a requirement ofFEAT_S1PIE.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/at.c | 136 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 1 deletion(-)arch/arm64/kvm/at.c +130
-0@@ -736,6 +736,23 @@ static u64 compute_par_s1(struct kvm_vcpu *vcpu, struct s1_walk_result *wr,return par; } +static bool s1pie_enabled (struct kvm_vcpu *vcpu,enum trans_regime regime) +{ +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) +return false; + +switch (regime) { +case TR_EL2 : +case TR_EL20 : +return __vcpu_sys_reg (vcpu,TCR2_EL2 ) &TCR2_EL2_PIE ; +case TR_EL10 : +return (__vcpu_sys_reg (vcpu,HCRX_EL2 ) & HCRX_EL2_TCR2En) && + (__vcpu_sys_reg (vcpu,TCR2_EL1 ) & TCR2_EL1x_PIE); +default : +BUG (); + } +} +static bool pan3_enabled (struct kvm_vcpu *vcpu,enum trans_regime regime) {u64 sctlr;@@ -743,6 +760,9 @@ static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR1_EL1 ,PAN ,PAN3 ))return false; +if (s1pie_enabled (vcpu, regime)) +return true; +if (regime ==TR_EL10 ) sctlr =vcpu_read_sys_reg (vcpu,SCTLR_EL1 );else @@ -826,12 +846,126 @@ static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, } } +#define pi_idx (v, r, i) ((__vcpu_sys_reg((v), (r)) >> ((i) * 4)) & 0xf) + +#define set_priv_perms (p, r, w, x) \ + do { \ + (p)->pr = (r); \ + (p)->pw = (w); \ + (p)->px = (x); \ + } while (0) + +#define set_unpriv_perms (p, r, w, x) \ + do { \ + (p)->ur = (r); \ + (p)->uw = (w); \ + (p)->ux = (x); \ + } while (0) + +/* Similar to AArch64.S1IndirectBasePermissions(), without GCS */ +#define set_perms (w, p, ip) \ + do { \ + switch ((ip)) { \ + case 0b0000: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b0001: \ + set_ ## w ## _perms((p), true , false, false); \ + break; \ + case 0b0010: \ + set_ ## w ## _perms((p), false, false, true ); \ + break; \ + case 0b0011: \ + set_ ## w ## _perms((p), true , false, true ); \ + break; \ + case 0b0100: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b0101: \ + set_ ## w ## _perms((p), true , true , false); \ + break; \ + case 0b0110: \ + set_ ## w ## _perms((p), true , true , true ); \ + break; \ + case 0b0111: \ + set_ ## w ## _perms((p), true , true , true ); \ + break; \ + case 0b1000: \ + set_ ## w ## _perms((p), true , false, false); \ + break; \ + case 0b1001: \ + set_ ## w ## _perms((p), true , false, false); \ + break; \ + case 0b1010: \ + set_ ## w ## _perms((p), true , false, true ); \ + break; \ + case 0b1011: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b1100: \ + set_ ## w ## _perms((p), true , true , false); \ + break; \ + case 0b1101: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + case 0b1110: \ + set_ ## w ## _perms((p), true , true , true ); \ + break; \ + case 0b1111: \ + set_ ## w ## _perms((p), false, false, false); \ + break; \ + } \ + } while (0) + +static void compute_s1_indirect_permissions (struct kvm_vcpu *vcpu, +struct s1_walk_info *wi, +struct s1_walk_result *wr, +struct s1_perms *s1p) +{ +u8 up, pp, idx; + + idx = (FIELD_GET (GENMASK (54 ,53 ), wr->desc) <<2 | +FIELD_GET (BIT (51 ), wr->desc) <<1 | +FIELD_GET (BIT (6 ), wr->desc)); + +switch (wi->regime) { +case TR_EL10 : + pp =pi_idx (vcpu,PIR_EL1 , idx); + up =pi_idx (vcpu,PIRE0_EL1 , idx); +break ; +case TR_EL20 : + pp =pi_idx (vcpu,PIR_EL2 , idx); + up =pi_idx (vcpu,PIRE0_EL2 , idx); +break ; +case TR_EL2 : + pp =pi_idx (vcpu,PIR_EL2 , idx); + up =0 ; +break ; + } + +set_perms (priv, s1p, pp); + +if (wi->regime !=TR_EL2 ) +set_perms (unpriv, s1p, up); +else +set_unpriv_perms (s1p, false, false, false); + +if (s1p->px && s1p->uw) { +set_priv_perms (s1p, false, false, false); +set_unpriv_perms (s1p, false, false, false); + } +} + static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi,
arch/arm64/kvm/at.c static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, · line 969
Quoted 10 lines
arch/arm64/kvm/at.c +5
-1struct s1_walk_result *wr, struct s1_perms *s1p) { -compute_s1_direct_permissions (vcpu ,wi ,wr ,s1p ); +if (!s1pie_enabled (vcpu, wi->regime)) +compute_s1_direct_permissions (vcpu, wi, wr, s1p); +else +compute_s1_indirect_permissions (vcpu, wi, wr, s1p); +compute_s1_hierarchical_permissions (vcpu, wi, wr, s1p);
Is this (and the previous patch to split this up) right?
Looking at this from the ARM ARM (ARM DDI 0487K.a):
R JHSVW If Indirect permissions are used, then hierarchical permissions are disabled and TCR_ELx.HPDn are RES 1.
arch/arm64/kvm/at.c static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, · line 971
Quoted 3 lines
arch/arm64/kvm/at.c +0
-0if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) {
Thanks,
Joey
Marc Zyngier
maintainerRe: [PATCH v2 12/16] KVM: arm64: Implement AT S1PIE support
Hi Joey,
Thanks for having a look.
Quoted 18 lines · Joey Gouly
Hello Marc!
arch/arm64/kvm/at.c static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, · line 969
Quoted 12 lines
static void compute_s1_permissions(struct
kvm_vcpu*vcpu, u32 op,arch/arm64/kvm/at.c +5
-1struct s1_walk_info *wi, struct s1_walk_result *wr, struct s1_perms *s1p) { -compute_s1_direct_permissions (vcpu ,wi ,wr ,s1p ); + if (!s1pie_enabled(vcpu, wi->regime)) + compute_s1_direct_permissions(vcpu, wi, wr, s1p); + else +compute_s1_indirect_permissions (vcpu ,wi ,wr ,s1p ); +compute_s1_hierarchical_permissions (vcpu, wi, wr, s1p);Is this (and the previous patch to split this up) right?
Quoted 4 lines
Looking at this from the ARM ARM (ARM DDI 0487K.a):
R JHSVW If Indirect permissions are used, then hierarchical permissions are disabled and TCR_ELx.HPDn are RES 1.
Odd. I was convinced that it was when S1POE is enabled that HPs were disabled. But you are absolutely right, and it is once more proven that I can't read. Oh well.
Not to worry, I've since found other issues with this series. I have forgotten the patch dealing with the fast path on another branch, and since decided that TCR2_EL2 needed extra care to cope with individual features being disabled.
The rework is still useful, as I'm looking at POE as well, but I need to hoist the HP stuff up a notch.
I'll repost things once I've sorted these things up.
Thanks again,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Joey Gouly
reviewerRe: [PATCH v2 12/16] KVM: arm64: Implement AT S1PIE support
Quoted 24 lines · Marc Zyngier
Hi Joey,
Thanks for having a look.
Quoted 18 lines · Joey Gouly
Hello Marc!
arch/arm64/kvm/at.c static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, · line 969
Quoted 12 lines
static void compute_s1_permissions(struct
kvm_vcpu*vcpu, u32 op,arch/arm64/kvm/at.c +5
-1struct s1_walk_info *wi, struct s1_walk_result *wr, struct s1_perms *s1p) { -compute_s1_direct_permissions (vcpu ,wi ,wr ,s1p ); + if (!s1pie_enabled(vcpu, wi->regime)) + compute_s1_direct_permissions(vcpu, wi, wr, s1p); + else +compute_s1_indirect_permissions (vcpu ,wi ,wr ,s1p ); +compute_s1_hierarchical_permissions (vcpu, wi, wr, s1p);Is this (and the previous patch to split this up) right?
Quoted 8 lines
Quoted 4 lines
Looking at this from the ARM ARM (ARM DDI 0487K.a):
R JHSVW If Indirect permissions are used, then hierarchical permissions are disabled and TCR_ELx.HPDn are RES 1.
Odd. I was convinced that it was when S1POE is enabled that HPs were disabled. But you are absolutely right, and it is once more proven that I can't read. Oh well.
For POE there is:
RBVXDG Hierarchical Permissions are disabled and the TCR_ELx.{HPD0, HPD1} bits are RES1 for stage 1 of a translation regime using VMSAv8-64 if one or more of POE and E0POE (for EL1&0, EL2&0) is enabled for that translation regime.
Quoted 10 lines
Not to worry, I've since found other issues with this series. I have forgotten the patch dealing with the fast path on another branch, and since decided that
TCR2_EL2needed extra care to cope with individual features being disabled.The rework is still useful, as I'm looking at POE as well, but I need to hoist the HP stuff up a notch.
I'll repost things once I've sorted these things up.
I think the rest of this patch looked fine though.
Quoted 7 lines
Thanks again,
M.Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerRe: [PATCH v2 12/16] KVM: arm64: Implement AT S1PIE support
Quoted 35 lines · Joey Gouly
Quoted 24 lines · Marc Zyngier
Hi Joey,
Thanks for having a look.
Quoted 18 lines · Joey Gouly
Hello Marc!
arch/arm64/kvm/at.c static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, · line 969
Quoted 12 lines
static void compute_s1_permissions(struct
kvm_vcpu*vcpu, u32 op,arch/arm64/kvm/at.c +5
-1struct s1_walk_info *wi, struct s1_walk_result *wr, struct s1_perms *s1p) { -compute_s1_direct_permissions (vcpu ,wi ,wr ,s1p ); + if (!s1pie_enabled(vcpu, wi->regime)) + compute_s1_direct_permissions(vcpu, wi, wr, s1p); + else +compute_s1_indirect_permissions (vcpu ,wi ,wr ,s1p ); +compute_s1_hierarchical_permissions (vcpu, wi, wr, s1p);Is this (and the previous patch to split this up) right?
Quoted 8 lines
Quoted 4 lines
Looking at this from the ARM ARM (ARM DDI 0487K.a):
R JHSVW If Indirect permissions are used, then hierarchical permissions are disabled and TCR_ELx.HPDn are RES 1.
Odd. I was convinced that it was when S1POE is enabled that HPs were disabled. But you are absolutely right, and it is once more proven that I can't read. Oh well.
Quoted 7 lines
For POE there is:
RBVXDG Hierarchical Permissions are disabled and the TCR_ELx.{HPD0, HPD1} bits are RES1 for stage 1 of a translation regime using VMSAv8-64 if one or more of POE and E0POE (for EL1&0, EL2&0) is enabled for that translation regime.
Right, this is the one I had at the back of my head.
At the end of the day, it really looks like hierarchical permissions are as dead as the proverbial dodo, and their sole purpose left is to antagonise implementations. Hopefully someone realises that and eventually allows implementations to build TCR_ELx.HPD* as RES1.
Quoted 11 lines
Quoted 9 lines
Not to worry, I've since found other issues with this series. I have forgotten the patch dealing with the fast path on another branch, and since decided that
TCR2_EL2needed extra care to cope with individual features being disabled.The rework is still useful, as I'm looking at POE as well, but I need to hoist the HP stuff up a notch.
I'll repost things once I've sorted these things up.
I think the rest of this patch looked fine though.
Cool, thanks for having given it a look!
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v2 13/16 KVM: arm64: Define helper for EL2 registers with custom visibility
In reply to Marc ZyngierFrom: Mark Brown <[email protected]>
In preparation for adding more visibility filtering for EL2 registers add a helper macro like EL2_REG() which allows specification of a custom visibility operation.
--- arch/arm64/kvm/sys_regs.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
arch/arm64/kvm/sys_regs.c +11 -2
@@ -2136,6 +2136,15 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu, .val = v, \ } +#define EL2_REG_FILTERED (name, acc, rst, v, filter) { \ + SYS_DESC(SYS_##name), \ + .access = acc, \ + .reset = rst, \ + .reg = name, \ + .visibility = filter, \ + .val = v, \ +} +#define EL2_REG_VNCR (name, rst, v) EL2_REG(name, bad_vncr_trap, rst, v)#define EL2_REG_REDIR (name, rst, v) EL2_REG(name, bad_redir_trap, rst, v)
@@ -2803,8 +2812,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG_VNCR (HFGITR_EL2 , reset_val,0 ),EL2_REG_VNCR (HACR_EL2 , reset_val,0 ), - { SYS_DESC(SYS_ZCR_EL2), .access = access_zcr_el2, .reset = reset_val, - .visibility = sve_el2_visibility, .reg = ZCR_EL2 }, +EL2_REG_FILTERED (ZCR_EL2 , access_zcr_el2, reset_val,0 , + sve_el2_visibility), EL2_REG_VNCR(HCRX_EL2, reset_val, 0),
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 14/16 KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests
In reply to Marc ZyngierFrom: Mark Brown <[email protected]>
When the guest does not support FEAT_TCR2 we should not allow any access to it in order to ensure that we do not create spurious issues with guest migration. Add a visibility operation for it.
--- arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/kvm/sys_regs.c | 29 ++++++++++++++++++++++++++--- 2 files changed, 29 insertions(+), 3 deletions(-)
arch/arm64/include/asm/kvm_host.h +3 -0
@@ -1476,4 +1476,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val); (pa + pi + pa3) ==1 ; \ }) +#define kvm_has_tcr2 (k) \ + (kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP)) +#endif /* __ARM64_KVM_HOST_H__ */
arch/arm64/kvm/sys_regs.c +26 -3
@@ -2319,6 +2319,27 @@ static bool access_zcr_el2(struct kvm_vcpu *vcpu,return true; } +static unsigned int tcr2_visibility (const struct kvm_vcpu *vcpu, +const struct sys_reg_desc *rd) +{ +if (kvm_has_tcr2 (vcpu->kvm)) +return 0 ; + +return REG_HIDDEN ; +} + +static unsigned int tcr2_el2_visibility (const struct kvm_vcpu *vcpu, +const struct sys_reg_desc *rd) +{ +unsigned int r; + + r =el2_visibility (vcpu, rd); +if (r) +return r; + +return tcr2_visibility (vcpu, rd); +} +/* * Architected system registers. * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
@@ -2503,7 +2524,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 }, - { SYS_DESC(SYS_TCR2_EL1), access_vm_reg, reset_val, TCR2_EL1, 0 }, + { SYS_DESC(SYS_TCR2_EL1), access_vm_reg, reset_val, TCR2_EL1, 0, + .visibility = tcr2_visibility },PTRAUTH_KEY (APIA ), PTRAUTH_KEY(APIB),
@@ -2820,7 +2842,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG (TTBR0_EL2 , access_rw, reset_val,0 ),EL2_REG (TTBR1_EL2 , access_rw, reset_val,0 ),EL2_REG (TCR_EL2 , access_rw, reset_val,TCR_EL2_RES1 ), -EL2_REG (TCR2_EL2 , access_tcr2_el2, reset_val,TCR2_EL2_RES1 ), +EL2_REG_FILTERED (TCR2_EL2 , access_tcr2_el2, reset_val,TCR2_EL2_RES1 , + tcr2_el2_visibility),EL2_REG_VNCR (VTTBR_EL2 , reset_val,0 ), EL2_REG_VNCR(VTCR_EL2, reset_val, 0),
@@ -4679,7 +4702,7 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)if (kvm_has_feat (kvm,ID_AA64ISAR2_EL1 ,MOPS ,IMP )) vcpu->arch.hcrx_el2 |= (HCRX_EL2_MSCEn |HCRX_EL2_MCE2 ); -if (kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,TCRX ,IMP )) +if (kvm_has_tcr2 (kvm)) vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En; }
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 15/16 KVM: arm64: Hide S1PIE registers from userspace when disabled for guests
In reply to Marc ZyngierFrom: Mark Brown <[email protected]>
When the guest does not support S1PIE we should not allow any access to the system registers it adds in order to ensure that we do not create spurious issues with guest migration. Add a visibility operation for these registers.
--- arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/kvm/sys_regs.c | 35 ++++++++++++++++++++++++++----- 2 files changed, 33 insertions(+), 5 deletions(-)
arch/arm64/include/asm/kvm_host.h +3 -0
@@ -1479,4 +1479,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);#define kvm_has_tcr2 (k) \ (kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP)) +#define kvm_has_s1pie (k) \ + (kvm_has_feat((k), ID_AA64MMFR3_EL1, S1PIE, IMP)) +#endif /* __ARM64_KVM_HOST_H__ */
arch/arm64/kvm/sys_regs.c +30 -5
@@ -2340,6 +2340,27 @@ static unsigned int tcr2_el2_visibility(const struct kvm_vcpu *vcpu,return tcr2_visibility (vcpu, rd); } +static unsigned int s1pie_visibility (const struct kvm_vcpu *vcpu, +const struct sys_reg_desc *rd) +{ +if (kvm_has_s1pie (vcpu->kvm)) +return 0 ; + +return REG_HIDDEN ; +} + +static unsigned int s1pie_el2_visibility (const struct kvm_vcpu *vcpu, +const struct sys_reg_desc *rd) +{ +unsigned int r; + + r =el2_visibility (vcpu, rd); +if (r) +return r; + +return s1pie_visibility (vcpu, rd); +} +/* * Architected system registers. * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
@@ -2577,8 +2598,10 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_PMMIR_EL1), trap_raz_wi }, { SYS_DESC(SYS_MAIR_EL1), access_vm_reg, reset_unknown, MAIR_EL1 }, - { SYS_DESC(SYS_PIRE0_EL1), NULL, reset_unknown, PIRE0_EL1 }, - { SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1 }, + { SYS_DESC(SYS_PIRE0_EL1), NULL, reset_unknown, PIRE0_EL1, + .visibility = s1pie_visibility }, + { SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1, + .visibility = s1pie_visibility }, { SYS_DESC(SYS_AMAIR_EL1), access_vm_reg, reset_amair_el1, AMAIR_EL1 }, { SYS_DESC(SYS_LORSA_EL1), trap_loregion },
@@ -2875,8 +2898,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG (HPFAR_EL2 , access_rw, reset_val,0 ),EL2_REG (MAIR_EL2 , access_rw, reset_val,0 ), -EL2_REG (PIRE0_EL2 , check_s1pie_access_rw, reset_val,0 ), -EL2_REG (PIR_EL2 , check_s1pie_access_rw, reset_val,0 ), +EL2_REG_FILTERED (PIRE0_EL2 , check_s1pie_access_rw, reset_val,0 , + s1pie_el2_visibility), +EL2_REG_FILTERED (PIR_EL2 , check_s1pie_access_rw, reset_val,0 , + s1pie_el2_visibility),EL2_REG (AMAIR_EL2 , access_rw, reset_val,0 ), EL2_REG(VBAR_EL2, access_rw, reset_val, 0),
@@ -4751,7 +4776,7 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu) kvm->arch.fgu[HFGITR_GROUP ] |= (HFGITR_EL2_ATS1E1RP |HFGITR_EL2_ATS1E1WP ); -if (!kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) +if (!kvm_has_s1pie (kvm)) kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPIRE0_EL1 | HFGxTR_EL2_nPIR_EL1);
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v2 16/16 KVM: arm64: Rely on visibility to let PIR*_ELx/TCR2_ELx UNDEF
In reply to Marc ZyngierWith a visibility defined for these registers, there is no need to check again for S1PIE or TCRX being implemented as perform_access() already handles it.
--- arch/arm64/kvm/sys_regs.c | 36 +++--------------------------------- 1 file changed, 3 insertions(+), 33 deletions(-)
arch/arm64/kvm/sys_regs.c +3 -33
@@ -346,18 +346,6 @@ static bool access_rw(struct kvm_vcpu *vcpu,return true; } -static bool check_s1pie_access_rw (struct kvm_vcpu *vcpu, -struct sys_reg_params *p, -const struct sys_reg_desc *r) -{ -if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) { -kvm_inject_undefined (vcpu); -return false; - } - -return access_rw (vcpu, p, r); -} -/* * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized). */
@@ -424,12 +412,6 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,bool was_enabled =vcpu_has_cache_enabled (vcpu);u64 val, mask, shift; -if (reg_to_encoding (r) ==SYS_TCR2_EL1 && - !kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,TCRX ,IMP )) { -kvm_inject_undefined (vcpu); -return false; - } -BUG_ON (!p->is_write);get_access_mask (r, &mask, &shift);
@@ -448,18 +430,6 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,return true; } -static bool access_tcr2_el2 (struct kvm_vcpu *vcpu, -struct sys_reg_params *p, -const struct sys_reg_desc *r) -{ -if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,TCRX ,IMP )) { -kvm_inject_undefined (vcpu); -return false; - } - -return access_rw (vcpu, p, r); -} -static bool access_actlr (struct kvm_vcpu *vcpu,struct sys_reg_params *p,const struct sys_reg_desc *r)
@@ -2865,7 +2835,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG (TTBR0_EL2 , access_rw, reset_val,0 ),EL2_REG (TTBR1_EL2 , access_rw, reset_val,0 ),EL2_REG (TCR_EL2 , access_rw, reset_val,TCR_EL2_RES1 ), -EL2_REG_FILTERED (TCR2_EL2 , access_tcr2_el2, reset_val,TCR2_EL2_RES1 , +EL2_REG_FILTERED (TCR2_EL2 , access_rw, reset_val,TCR2_EL2_RES1 , tcr2_el2_visibility),EL2_REG_VNCR (VTTBR_EL2 , reset_val,0 ), EL2_REG_VNCR(VTCR_EL2, reset_val, 0),
@@ -2898,9 +2868,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {EL2_REG (HPFAR_EL2 , access_rw, reset_val,0 ),EL2_REG (MAIR_EL2 , access_rw, reset_val,0 ), -EL2_REG_FILTERED (PIRE0_EL2 , check_s1pie_access_rw, reset_val,0 , +EL2_REG_FILTERED (PIRE0_EL2 , access_rw, reset_val,0 , s1pie_el2_visibility), -EL2_REG_FILTERED (PIR_EL2 , check_s1pie_access_rw, reset_val,0 , +EL2_REG_FILTERED (PIR_EL2 , access_rw, reset_val,0 , s1pie_el2_visibility), EL2_REG(AMAIR_EL2, access_rw, reset_val, 0),
Signature · 1 line
2.39.2
Re: [PATCH v2 16/16] KVM: arm64: Rely on visibility to let PIR*_ELx/TCR2_ELx UNDEF
Quoted 3 lines · Marc Zyngier
With a visibility defined for these registers, there is no need to check again for S1PIE or TCRX being implemented as
perform_access()already handles it.
Ah, thanks for updating this: