KVM: arm64: Add EL2 support to FEAT_S1PIE/S1POE
Marc Zyngier
maintainerv4 cover KVM: arm64: Add EL2 support to FEAT_S1PIE/S1POE
This series serves a few purposes:
- Complete the S1PIE/S1POE 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]).
Patches on top of v6.12-rc1.
* From v3 [3]:
- Added a few extra patches to deal with S1POE at EL2, including AT
- Fix CPTR_EL2.E0POE non-sensical trap entry
- Make sure that TVM and TRVM are handling the EL1 PIE/POE traps
- Simplify some of the visibility cruft
- Rebased on v6.12-rc1
* From v2 [2]:
- Correctly reprogram the context for AT in the fast path when S1PIE is in use
- Generalised sysreg RES0/RES1 masking, and added TCR2_EL2 RES0/RES1 bit handling
- Fix TCR2_EL1, PIR_EL1, PIRE0_EL1 access with VHE
- Add a bunch of missing registers to get_el2_to_el1_mapping()
- Correctly map {TCR2,PIR,PIRE0}_EL2 to their EL1 equivalent on NV
- Disable hierarchical permissions when S1PIE is enabled
- Make EL2 world switch directly act on the vcpu rather than an arbitrary context
- Remove SKL{0,1} from the TCR2_EL2 description
* From 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 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]
[3] https://lore.kernel.org/r/[email protected]
Marc Zyngier (33):
arm64: Drop SKL0/SKL1 from TCR2_EL2
arm64: Remove VNCR definition for PIRE0_EL2
arm64: Add encoding for PIRE0_EL2
KVM: arm64: Drop useless struct s2_mmu in __kvm_at_s1e2()
KVM: arm64: nv: Add missing EL2->EL1 mappings in
get_el2_to_el1_mapping()
KVM: arm64: nv: Handle CNTHCTL_EL2 specially
KVM: arm64: nv: Save/Restore vEL2 sysregs
KVM: arm64: Correctly access TCR2_EL1, PIR_EL1, PIRE0_EL1 with VHE
KVM: arm64: Extend masking facility to arbitrary registers
arm64: Define ID_AA64MMFR1_EL1.HAFDBS advertising FEAT_HAFT
KVM: arm64: Add TCR2_EL2 to the sysreg arrays
KVM: arm64: Sanitise TCR2_EL2
KVM: arm64: Add save/restore for TCR2_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: Add AT fast-path support for S1PIE
KVM: arm64: Split S1 permission evaluation into direct and
hierarchical parts
KVM: arm64: Disable hierarchical permissions when S1PIE is enabled
KVM: arm64: Implement AT S1PIE support
KVM: arm64: Rely on visibility to let PIR*_ELx/TCR2_ELx UNDEF
arm64: Add encoding for POR_EL2
KVM: arm64: Add a composite EL2 visibility helper
KVM: arm64: Drop bogus CPTR_EL2.E0POE trap routing
KVM: arm64: Subject S1PIE/S1POE registers to HCR_EL2.{TVM,TRVM}
KVM: arm64: Add basic support for POR_EL2
KVM: arm64: Add save/retore support for POR_EL2
KVM: arm64: Add POE save/restore for AT emulation fast-path
KVM: arm64: Disable hierarchical permissions when POE is enabled
KVM: arm64: Make PAN conditions part of the S1 walk context
KVM: arm64: Handle stage-1 permission overlays
KVM: arm64: Handle WXN attribute
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 | 37 +- arch/arm64/include/asm/vncr_mapping.h | 1 - arch/arm64/kvm/at.c | 470 ++++++++++++++++++--- arch/arm64/kvm/emulate-nested.c | 12 +- 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 | 161 ++++++- arch/arm64/kvm/nested.c | 34 +- arch/arm64/kvm/sys_regs.c | 141 ++++++- arch/arm64/tools/sysreg | 12 +- include/kvm/arm_arch_timer.h | 3 + 11 files changed, 766 insertions(+), 112 deletions(-)
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 1/36 arm64: Drop SKL0/SKL1 from TCR2_EL2
Despite what the documentation says, TCR2_EL2.{SKL0,SKL1} do not exist, and the corresponding information is in the respective TTBRx_EL2. This is a leftover from a development version of the architecture.
This change makes TCR2_EL2 similar to TCR2_EL1 in that respect.
--- arch/arm64/tools/sysreg | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm64/tools/sysreg +1 -2
@@ -2819,8 +2819,7 @@ Field 13 AMEC1 Field 12 AMEC0 Field 11 HAFT Field 10 PTTWI -Field 9:8 SKL1 -Field 7:6 SKL0 +Res0 9:6 Field 5 D128 Field 4 AIE Field 3 POE
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 2/36 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_POR_EL1 0x2A8#define VNCR_ICH_LR0_EL2 0x400
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 3/36 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
@@ -2882,6 +2882,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
Re: [PATCH v4 03/36] arm64: Add encoding for PIRE0_EL2
Quoted 2 lines · Marc Zyngier
PIRE0_EL2is the equivalent ofPIRE0_EL1for the EL2&0 translation regime, and it is sorely missing from the sysreg file.
Marc Zyngier
maintainerPATCH v4 4/36 KVM: arm64: Drop useless struct s2_mmu in __kvm_at_s1e2()
In reply to Marc Zyngier__kvm_at_s1e2() contains the definition of an s2_mmu for the current context, but doesn't make any use of it. Drop it.
--- arch/arm64/kvm/at.c | 3 --- 1 file changed, 3 deletions(-)
arch/arm64/kvm/at.c +0 -3
@@ -992,12 +992,9 @@ void __kvm_at_s1e2(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)* switching context behind everybody's back, disable interrupts... */ scoped_guard(write_lock_irqsave, &vcpu->kvm->mmu_lock) { -struct kvm_s2_mmu *mmu;u64 val, hcr;bool fail; - mmu = &vcpu->kvm->arch.mmu; - val = hcr =read_sysreg (hcr_el2); val &= ~HCR_TGE ; val |=HCR_VM ;
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 5/36 KVM: arm64: nv: Add missing EL2->EL1 mappings in get_el2_to_el1_mapping()
In reply to Marc ZyngierAs KVM has grown a bunch of new system register for NV, it appears that we are missing them in the get_el2_to_el1_mapping() list.
Most of them are not crucial as they don't tend to be accessed via vcpu_read_sys_reg() and vcpu_write_sys_reg().
--- arch/arm64/kvm/sys_regs.c | 9 +++++++++ 1 file changed, 9 insertions(+)
arch/arm64/kvm/sys_regs.c +9 -0
@@ -110,6 +110,14 @@ static bool get_el2_to_el1_mapping(unsigned int reg,PURE_EL2_SYSREG (RVBAR_EL2 );PURE_EL2_SYSREG (TPIDR_EL2 );PURE_EL2_SYSREG (HPFAR_EL2 ); +PURE_EL2_SYSREG (HCRX_EL2 ); +PURE_EL2_SYSREG (HFGRTR_EL2 ); +PURE_EL2_SYSREG (HFGWTR_EL2 ); +PURE_EL2_SYSREG (HFGITR_EL2 ); +PURE_EL2_SYSREG (HDFGRTR_EL2 ); +PURE_EL2_SYSREG (HDFGWTR_EL2 ); +PURE_EL2_SYSREG (HAFGRTR_EL2 ); +PURE_EL2_SYSREG (CNTVOFF_EL2 );PURE_EL2_SYSREG (CNTHCTL_EL2 );MAPPED_EL2_SYSREG (SCTLR_EL2 ,SCTLR_EL1 , translate_sctlr_el2_to_sctlr_el1 );
@@ -130,6 +138,7 @@ static bool get_el2_to_el1_mapping(unsigned int reg,MAPPED_EL2_SYSREG (ELR_EL2 ,ELR_EL1 ,NULL );MAPPED_EL2_SYSREG (SPSR_EL2 ,SPSR_EL1 ,NULL );MAPPED_EL2_SYSREG (ZCR_EL2 ,ZCR_EL1 ,NULL ); +MAPPED_EL2_SYSREG (CONTEXTIDR_EL2 ,CONTEXTIDR_EL1 ,NULL );default :return false; }
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 6/36 KVM: arm64: nv: Handle CNTHCTL_EL2 specially
In reply to Marc ZyngierAccessing 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
@@ -157,6 +157,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.
@@ -207,6 +222,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
Re: [PATCH v4 06/36] KVM: arm64: nv: Handle CNTHCTL_EL2 specially
Hi Marc,
I'm planning to have a look at (some) of the patches, do you have a timeline for merging the series? Just so I know what to prioritise.
Quoted 46 lines · Marc Zyngier
Accessing
CNTHCTL_EL2is fraught with danger if running with HCR_EL2.E2H=1: half of the bits are held inCNTKCTL_EL1, and thus can be changed behind our back, while the rest lives in theCNTHCTL_EL2shadow 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_EL1in a more straightforward manner.Signed-off-by Marc Zyngier <[email protected]>--- 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 +15
-0@@ -157,6 +157,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. @@ -207,6 +222,19 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)*/ __vcpu_sys_reg (vcpu, reg) = val;
arch/arm64/kvm/sys_regs.c void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg) · line 234
Quoted 10 lines
arch/arm64/kvm/sys_regs.c +10
-0+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 );
Sorry, but I just can't seem to get my head around why the RES0 bits aren't cleared. Is KVM relying on the guest to implement Should-Be-Zero-or-Preserved, as per the RES0 definition?
Quoted 11 lines
arch/arm64/kvm/sys_regs.c +3
-0+return ; + } +/* No EL1 counterpart? We're done here.? */ if (reg == el1r)return ;include/kvm/arm_arch_timer.h +0
-0
include/kvm/arm_arch_timer.h u64 timer_get_cval(struct arch_timer_context *ctxt); · line 151
Quoted 5 lines
include/kvm/arm_arch_timer.h +2
-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))
This does match CNTHCTL_EL2_VHE().
Thanks,
Alex
Quoted 7 lines
include/kvm/arm_arch_timer.h +1
-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
maintainerRe: [PATCH v4 06/36] KVM: arm64: nv: Handle CNTHCTL_EL2 specially
Quoted 5 lines · Alexandru Elisei
Hi Marc,
I'm planning to have a look at (some) of the patches, do you have a timeline for merging the series? Just so I know what to prioritise.
I want it merged yesterday. All of it.
Quoted 48 lines · Marc Zyngier
Quoted 46 lines
Accessing
CNTHCTL_EL2is fraught with danger if running with HCR_EL2.E2H=1: half of the bits are held inCNTKCTL_EL1, and thus can be changed behind our back, while the rest lives in theCNTHCTL_EL2shadow 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_EL1in a more straightforward manner.Signed-off-by Marc Zyngier <[email protected]>--- 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 +15
-0@@ -157,6 +157,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. @@ -207,6 +222,19 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)*/ __vcpu_sys_reg (vcpu, reg) = val;
Quoted 14 lines
arch/arm64/kvm/sys_regs.c void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg) · line 234
Quoted 10 lines
arch/arm64/kvm/sys_regs.c +10
-0+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 );Sorry, but I just can't seem to get my head around why the RES0 bits aren't cleared. Is KVM relying on the guest to implement Should-Be-Zero-or-Preserved, as per the RES0 definition?
KVM isn't relying on anything. And it isn't about the RES0 bits not being cleared. It is about the HW not providing storage for some of the CNTHCTL_EL2 bits when the guest is using CNTKCTL_EL1 as a proxy for its own view of CNTHCTL_EL2.
Namely, bits outside of CNTKCTL_VALID_BITS are not guaranteed to be stored until (IIRC) FEAT_NV2p1, which retrospectively fixes the architecture by mandating that the relevant bits have dedicated storage.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Re: [PATCH v4 06/36] KVM: arm64: nv: Handle CNTHCTL_EL2 specially
Hi,
Quoted 78 lines · Marc Zyngier
Quoted 5 lines · Alexandru Elisei
Hi Marc,
I'm planning to have a look at (some) of the patches, do you have a timeline for merging the series? Just so I know what to prioritise.
I want it merged yesterday. All of it.
Quoted 48 lines · Marc Zyngier
Quoted 46 lines
Accessing
CNTHCTL_EL2is fraught with danger if running with HCR_EL2.E2H=1: half of the bits are held inCNTKCTL_EL1, and thus can be changed behind our back, while the rest lives in theCNTHCTL_EL2shadow 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_EL1in a more straightforward manner.Signed-off-by Marc Zyngier <[email protected]>--- 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 +15
-0@@ -157,6 +157,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. @@ -207,6 +222,19 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)*/ __vcpu_sys_reg (vcpu, reg) = val;Quoted 14 lines
arch/arm64/kvm/sys_regs.c void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg) · line 234
Quoted 10 lines
arch/arm64/kvm/sys_regs.c +10
-0+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 );Sorry, but I just can't seem to get my head around why the RES0 bits aren't cleared. Is KVM relying on the guest to implement Should-Be-Zero-or-Preserved, as per the RES0 definition?
KVM isn't relying on anything. And it isn't about the RES0 bits not being cleared. It is about the HW not providing storage for some of the
CNTHCTL_EL2bits when the guest is usingCNTKCTL_EL1as a proxy for its own view ofCNTHCTL_EL2.
Quoted 4 lines
Namely, bits outside of
CNTKCTL_VALID_BITSare not guaranteed to be stored until (IIRC)FEAT_NV2p1, which retrospectively fixes the architecture by mandating that the relevant bits have dedicated storage.
The definition for RES0 says:
'A bit that is RES0 in a context is reserved for possible future use in that context. To preserve forward compatibility, software:
* Must not rely on the bit reading as 0.
* Must use an SBZP policy to write to the bit.'
where Should-Be-Zero-of-Preserved (SBZP):
'When writing this field, software must either write all 0s to this field or, if the register is being restored from a previously read state, write the previously read value to this field. If this is not done, then the result is unpredictable.'
And what about the rest of the RES0 bits from CNTKCTL_EL1, those that are RES0 in both registers?
Thanks,
Alex
Marc Zyngier
maintainerRe: [PATCH v4 06/36] KVM: arm64: nv: Handle CNTHCTL_EL2 specially
Quoted 37 lines · Alexandru Elisei
Quoted 25 lines
Quoted 5 lines
Quoted 5 lines
arch/arm64/kvm/sys_regs.c +0
-0@@ -207,6 +222,19 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)*/ __vcpu_sys_reg (vcpu, reg) = val;Quoted 14 lines
arch/arm64/kvm/sys_regs.c void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg) · line 234
Quoted 10 lines
arch/arm64/kvm/sys_regs.c +10
-0+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 );Sorry, but I just can't seem to get my head around why the RES0 bits aren't cleared. Is KVM relying on the guest to implement Should-Be-Zero-or-Preserved, as per the RES0 definition?
KVM isn't relying on anything. And it isn't about the RES0 bits not being cleared. It is about the HW not providing storage for some of the
CNTHCTL_EL2bits when the guest is usingCNTKCTL_EL1as a proxy for its own view ofCNTHCTL_EL2.Quoted 4 lines
Namely, bits outside of
CNTKCTL_VALID_BITSare not guaranteed to be stored until (IIRC)FEAT_NV2p1, which retrospectively fixes the architecture by mandating that the relevant bits have dedicated storage.The definition for RES0 says:
'A bit that is RES0 in a context is reserved for possible future use in that context. To preserve forward compatibility, software:
* Must not rely on the bit reading as 0.
* Must use an SBZP policy to write to the bit.'
Quoted 6 lines
where Should-Be-Zero-of-Preserved (SBZP):
'When writing this field, software must either write all 0s to this field or, if the register is being restored from a previously read state, write the previously read value to this field. If this is not done, then the result is unpredictable.'
And? I can quote the ARM ARM too, but that's not going to lead us anywhere if you don't explain why what you quote is related to the problem at hand (hint, I don't think it is).
Quoted 2 lines
And what about the rest of the RES0 bits from
CNTKCTL_EL1, those that are RES0 in both registers?
What about them *what*?
It would definitely help if you didn't write in riddles and actually spell out what you mean. If you think this code is wrong, please explain why you think it is wrong, and maybe we'll be able to make some progress.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v4 7/36 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
@@ -152,9 +152,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
Re: [PATCH v4 07/36] KVM: arm64: nv: Save/Restore vEL2 sysregs
arch/arm64/kvm/hyp/vhe/sysreg-sr.c line 78
Quoted 9 lines · Marc Zyngier
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +9
-0+static void __sysreg_restore_vel2_state (struct kvm_vcpu *vcpu) +{ +u64 val; + +/* These registers are common with EL1 */ +write_sysreg (__vcpu_sys_reg (vcpu,PAR_EL1 ), par_el1); +write_sysreg (__vcpu_sys_reg (vcpu,TPIDR_EL1 ), tpidr_el1); + +write_sysreg (read_cpuid_id (), vpidr_el2);
I don't think we need to restore VPIDR_EL2 here, so long as we do it on vcpu_put() when leaving a nested VM context. That seems like the right place to have it, as we could be running a mix of nested and non-nested VMs and don't ever poke VPIDR_EL2 for non-NV VMs.
Quoted 5 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +0
-1@@ -89,7 +192,29 @@ void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu)*/ __sysreg32_restore_state (vcpu);__sysreg_restore_user_state (guest_ctxt); -__sysreg_restore_el1_state (guest_ctxt);
arch/arm64/kvm/hyp/vhe/sysreg-sr.c void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu) · line 204
Quoted 10 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +10
-0+ +if (unlikely (__is_hyp_ctxt (guest_ctxt))) { +__sysreg_restore_vel2_state (vcpu); + }else { +if (vcpu_has_nv (vcpu)) { +/* +* Only set VPIDR_EL2 for nested VMs, as this is the +* only time it changes. We'll restore the MIDR_EL1 +* view on put. +*/
Slightly ambiguous what "VPIDR_EL2" this is referring to (hardware reg v. guest value). Maybe:
/*
* Use the guest hypervisor's VPIDR_EL2 when in a nested
* state. The hardware value of MIDR_EL1 gets restored on
* put.
*/
Quoted 23 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +15
-1+write_sysreg (ctxt_sys_reg (guest_ctxt,VPIDR_EL2 ), vpidr_el2); + +/* +* As we're restoring a nested guest, set the value +* provided by the guest hypervisor. +*/ + mpidr =ctxt_sys_reg (guest_ctxt,VMPIDR_EL2 ); + }else { + mpidr =ctxt_sys_reg (guest_ctxt,MPIDR_EL1 ); + } + +__sysreg_restore_el1_state (guest_ctxt, mpidr); + }vcpu_set_flag (vcpu,SYSREGS_ON_CPU ); }@@ -112,12 +237,20 @@ void __vcpu_put_switch_sysregs(struct kvm_vcpu *vcpu) host_ctxt =host_data_ptr (host_ctxt); -__sysreg_save_el1_state (guest_ctxt); +if (unlikely (__is_hyp_ctxt (guest_ctxt))) +__sysreg_save_vel2_state (vcpu);
arch/arm64/kvm/hyp/vhe/sysreg-sr.c void __vcpu_put_switch_sysregs(struct kvm_vcpu *vcpu) · line 251
Quoted 10 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +4
-0+else +__sysreg_save_el1_state (guest_ctxt); +__sysreg_save_user_state (guest_ctxt);__sysreg32_save_state (vcpu);/* Restore host user state */ __sysreg_restore_user_state (host_ctxt); +/* If leaving a nesting guest, restore MPIDR_EL1 default view */
typo: MIDR_EL1
Quoted 8 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +3
-0+if (vcpu_has_nv (vcpu)) +write_sysreg (read_cpuid_id (), vpidr_el2); +vcpu_clear_flag (vcpu,SYSREGS_ON_CPU ); }Signature · 1 line
2.39.2
Signature · 2 lines
Thanks, Oliver
Re: [PATCH v4 07/36] KVM: arm64: nv: Save/Restore vEL2 sysregs
In reply to Marc ZyngierHi Marc,
Quoted 133 lines · Marc Zyngier
Whenever 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.
Reviewed-by Alexandru Elisei <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- 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@@ -152,9 +152,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 +73
-0@@ -15,6 +15,108 @@#include <asm/kvm_hyp.h> #include <asm/kvm_nested.h> +static void __sysreg_save_vel2_state (struct kvm_vcpu *vcpu) +{ +/* These registers are common with EL1 */ +__vcpu_sys_reg (vcpu,PAR_EL1 ) =read_sysreg (par_el1); +__vcpu_sys_reg (vcpu,TPIDR_EL1 ) =read_sysreg (tpidr_el1); + +__vcpu_sys_reg (vcpu,ESR_EL2 ) =read_sysreg_el1 (SYS_ESR ); +__vcpu_sys_reg (vcpu,AFSR0_EL2 ) =read_sysreg_el1 (SYS_AFSR0 ); +__vcpu_sys_reg (vcpu,AFSR1_EL2 ) =read_sysreg_el1 (SYS_AFSR1 ); +__vcpu_sys_reg (vcpu,FAR_EL2 ) =read_sysreg_el1 (SYS_FAR ); +__vcpu_sys_reg (vcpu,MAIR_EL2 ) =read_sysreg_el1 (SYS_MAIR ); +__vcpu_sys_reg (vcpu,VBAR_EL2 ) =read_sysreg_el1 (SYS_VBAR ); +__vcpu_sys_reg (vcpu,CONTEXTIDR_EL2 ) =read_sysreg_el1 (SYS_CONTEXTIDR ); +__vcpu_sys_reg (vcpu,AMAIR_EL2 ) =read_sysreg_el1 (SYS_AMAIR ); + +/* +* In VHE mode those registers are compatible between EL1 and EL2, +* and the guest uses the _EL1 versions on the CPU naturally. +* So we save them into their _EL2 versions here. +* For nVHE mode we trap accesses to those registers, so our +* _EL2 copy in sys_regs[] is always up-to-date and we don't need +* to save anything here. +*/ +if (vcpu_el2_e2h_is_set (vcpu)) { +u64 val; + +/* +* We don't save CPTR_EL2, as accesses to CPACR_EL1 +* are always trapped, ensuring that the in-memory +* copy is always up-to-date. A small blessing... +*/ +__vcpu_sys_reg (vcpu,SCTLR_EL2 ) =read_sysreg_el1 (SYS_SCTLR ); +__vcpu_sys_reg (vcpu,TTBR0_EL2 ) =read_sysreg_el1 (SYS_TTBR0 ); +__vcpu_sys_reg (vcpu,TTBR1_EL2 ) =read_sysreg_el1 (SYS_TTBR1 ); +__vcpu_sys_reg (vcpu,TCR_EL2 ) =read_sysreg_el1 (SYS_TCR ); + +/* +* The EL1 view of CNTKCTL_EL1 has a bunch of RES0 bits where +* the interesting CNTHCTL_EL2 bits live. So preserve these +* bits when reading back the guest-visible value. +*/ + val =read_sysreg_el1 (SYS_CNTKCTL ); + val &=CNTKCTL_VALID_BITS ; +__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ) &= ~CNTKCTL_VALID_BITS ; +__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ) |= val; + } + +__vcpu_sys_reg (vcpu,SP_EL2 ) =read_sysreg (sp_el1); +__vcpu_sys_reg (vcpu,ELR_EL2 ) =read_sysreg_el1 (SYS_ELR ); +__vcpu_sys_reg (vcpu,SPSR_EL2 ) =read_sysreg_el1 (SYS_SPSR ); +} + +static void __sysreg_restore_vel2_state (struct kvm_vcpu *vcpu) +{ +u64 val; + +/* These registers are common with EL1 */ +write_sysreg (__vcpu_sys_reg (vcpu,PAR_EL1 ), par_el1); +write_sysreg (__vcpu_sys_reg (vcpu,TPIDR_EL1 ), tpidr_el1); + +write_sysreg (read_cpuid_id (), vpidr_el2); +write_sysreg (__vcpu_sys_reg (vcpu,MPIDR_EL1 ), vmpidr_el2); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,MAIR_EL2 ),SYS_MAIR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,VBAR_EL2 ),SYS_VBAR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,CONTEXTIDR_EL2 ),SYS_CONTEXTIDR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,AMAIR_EL2 ),SYS_AMAIR ); + +if (vcpu_el2_e2h_is_set (vcpu)) { +/* +* In VHE mode those registers are compatible between +* EL1 and EL2. +*/ +write_sysreg_el1 (__vcpu_sys_reg (vcpu,SCTLR_EL2 ),SYS_SCTLR );
arch/arm64/kvm/hyp/vhe/sysreg-sr.c line 100
Quoted 10 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +10
-0+write_sysreg_el1 (__vcpu_sys_reg (vcpu,CPTR_EL2 ),SYS_CPACR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TTBR0_EL2 ),SYS_TTBR0 ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TTBR1_EL2 ),SYS_TTBR1 ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TCR_EL2 ),SYS_TCR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ),SYS_CNTKCTL ); + }else { +/* +* CNTHCTL_EL2 only affects EL1 when running nVHE, so +* no need to restore it. +*/
I'm having such a hard time parsing the comment - might be just me coming back to this code after such a long time.
If CNTHCTL_EL2 only affects EL1 when running nVHE, and the else branch deals with the nVHE case, why isn't CNTHCTL_EL2 restored?
As for the 'only' part of the comment: when E2H=1, bits 10 and 11, EL1PCTEN and EL1PTEN (why isn't this named EL1PCEN if it does the same thing as bit 1 when E2H=0?), trap EL1 and EL0 accesses to physical counter and timer registers.
Or 'only' in this context means only EL1, and not EL2 also?
Thanks,
Alex
Quoted 9 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +9
-0+ val =translate_sctlr_el2_to_sctlr_el1 (__vcpu_sys_reg (vcpu,SCTLR_EL2 )); +write_sysreg_el1 (val,SYS_SCTLR ); + val =translate_cptr_el2_to_cpacr_el1 (__vcpu_sys_reg (vcpu,CPTR_EL2 )); +write_sysreg_el1 (val,SYS_CPACR ); + val =translate_ttbr0_el2_to_ttbr0_el1 (__vcpu_sys_reg (vcpu,TTBR0_EL2 )); +write_sysreg_el1 (val,SYS_TTBR0 ); + val =translate_tcr_el2_to_tcr_el1 (__vcpu_sys_reg (vcpu,TCR_EL2 )); +write_sysreg_el1 (val,SYS_TCR ); + }
Marc Zyngier
maintainerRe: [PATCH v4 07/36] KVM: arm64: nv: Save/Restore vEL2 sysregs
Quoted 151 lines · Alexandru Elisei
Hi Marc,
Quoted 133 lines · Marc Zyngier
Whenever 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.
Reviewed-by Alexandru Elisei <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- 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@@ -152,9 +152,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 +73
-0@@ -15,6 +15,108 @@#include <asm/kvm_hyp.h> #include <asm/kvm_nested.h> +static void __sysreg_save_vel2_state (struct kvm_vcpu *vcpu) +{ +/* These registers are common with EL1 */ +__vcpu_sys_reg (vcpu,PAR_EL1 ) =read_sysreg (par_el1); +__vcpu_sys_reg (vcpu,TPIDR_EL1 ) =read_sysreg (tpidr_el1); + +__vcpu_sys_reg (vcpu,ESR_EL2 ) =read_sysreg_el1 (SYS_ESR ); +__vcpu_sys_reg (vcpu,AFSR0_EL2 ) =read_sysreg_el1 (SYS_AFSR0 ); +__vcpu_sys_reg (vcpu,AFSR1_EL2 ) =read_sysreg_el1 (SYS_AFSR1 ); +__vcpu_sys_reg (vcpu,FAR_EL2 ) =read_sysreg_el1 (SYS_FAR ); +__vcpu_sys_reg (vcpu,MAIR_EL2 ) =read_sysreg_el1 (SYS_MAIR ); +__vcpu_sys_reg (vcpu,VBAR_EL2 ) =read_sysreg_el1 (SYS_VBAR ); +__vcpu_sys_reg (vcpu,CONTEXTIDR_EL2 ) =read_sysreg_el1 (SYS_CONTEXTIDR ); +__vcpu_sys_reg (vcpu,AMAIR_EL2 ) =read_sysreg_el1 (SYS_AMAIR ); + +/* +* In VHE mode those registers are compatible between EL1 and EL2, +* and the guest uses the _EL1 versions on the CPU naturally. +* So we save them into their _EL2 versions here. +* For nVHE mode we trap accesses to those registers, so our +* _EL2 copy in sys_regs[] is always up-to-date and we don't need +* to save anything here. +*/ +if (vcpu_el2_e2h_is_set (vcpu)) { +u64 val; + +/* +* We don't save CPTR_EL2, as accesses to CPACR_EL1 +* are always trapped, ensuring that the in-memory +* copy is always up-to-date. A small blessing... +*/ +__vcpu_sys_reg (vcpu,SCTLR_EL2 ) =read_sysreg_el1 (SYS_SCTLR ); +__vcpu_sys_reg (vcpu,TTBR0_EL2 ) =read_sysreg_el1 (SYS_TTBR0 ); +__vcpu_sys_reg (vcpu,TTBR1_EL2 ) =read_sysreg_el1 (SYS_TTBR1 ); +__vcpu_sys_reg (vcpu,TCR_EL2 ) =read_sysreg_el1 (SYS_TCR ); + +/* +* The EL1 view of CNTKCTL_EL1 has a bunch of RES0 bits where +* the interesting CNTHCTL_EL2 bits live. So preserve these +* bits when reading back the guest-visible value. +*/ + val =read_sysreg_el1 (SYS_CNTKCTL ); + val &=CNTKCTL_VALID_BITS ; +__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ) &= ~CNTKCTL_VALID_BITS ; +__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ) |= val; + } + +__vcpu_sys_reg (vcpu,SP_EL2 ) =read_sysreg (sp_el1); +__vcpu_sys_reg (vcpu,ELR_EL2 ) =read_sysreg_el1 (SYS_ELR ); +__vcpu_sys_reg (vcpu,SPSR_EL2 ) =read_sysreg_el1 (SYS_SPSR ); +} + +static void __sysreg_restore_vel2_state (struct kvm_vcpu *vcpu) +{ +u64 val; + +/* These registers are common with EL1 */ +write_sysreg (__vcpu_sys_reg (vcpu,PAR_EL1 ), par_el1); +write_sysreg (__vcpu_sys_reg (vcpu,TPIDR_EL1 ), tpidr_el1); + +write_sysreg (read_cpuid_id (), vpidr_el2); +write_sysreg (__vcpu_sys_reg (vcpu,MPIDR_EL1 ), vmpidr_el2); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,MAIR_EL2 ),SYS_MAIR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,VBAR_EL2 ),SYS_VBAR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,CONTEXTIDR_EL2 ),SYS_CONTEXTIDR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,AMAIR_EL2 ),SYS_AMAIR ); + +if (vcpu_el2_e2h_is_set (vcpu)) { +/* +* In VHE mode those registers are compatible between +* EL1 and EL2. +*/ +write_sysreg_el1 (__vcpu_sys_reg (vcpu,SCTLR_EL2 ),SYS_SCTLR );arch/arm64/kvm/hyp/vhe/sysreg-sr.c line 100
Quoted 10 lines
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +10
-0+write_sysreg_el1 (__vcpu_sys_reg (vcpu,CPTR_EL2 ),SYS_CPACR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TTBR0_EL2 ),SYS_TTBR0 ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TTBR1_EL2 ),SYS_TTBR1 ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TCR_EL2 ),SYS_TCR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,CNTHCTL_EL2 ),SYS_CNTKCTL ); + }else { +/* +* CNTHCTL_EL2 only affects EL1 when running nVHE, so +* no need to restore it. +*/ I'm having such a hard time parsing the comment - might be just me coming back to this code after such a long time.
Quoted 2 lines
If
CNTHCTL_EL2only affects EL1 when running nVHE, and the else branch deals with the nVHE case, why isn'tCNTHCTL_EL2restored?
Because it has no impact at all? As in nothing? Niente? Rien? Zilch? We enter the guest's EL2, so why would we bother with restoring a guest register that has no influence on what we run?
Quoted 6 lines
As for the 'only' part of the comment: when E2H=1, bits 10 and 11, EL1PCTEN and EL1PTEN (why isn't this named EL1PCEN if it does the same thing as bit 1 when E2H=0?), trap EL1 and EL0 accesses to physical counter and timer registers.
Or 'only' in this context means only EL1, and not EL2 also?
None of this makes any sense to me. I don't understand your E2H consideration, nor your digression on the meaning of the word 'only'.
Look at the architecture. Do you see *ANY* bit in CNTHCTL_EL2 having *ANY* influence on EL2 when HCR_EL2.E2H=0? Don't you then come to the conclusion that CNTHCTL_EL2 only affects EL1?
But surely you've spotted something I can't see, and I must be specially thick today... Please enlighten me.
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v4 8/36 KVM: arm64: Correctly access TCR2_EL1, PIR_EL1, PIRE0_EL1 with VHE
In reply to Marc ZyngierFor code that accesses any of the guest registers for emulation purposes, it is crucial to know where the most up-to-date data is.
While this is pretty clear for nVHE (memory is the sole repository), things are a lot muddier for VHE, as depending on the SYSREGS_ON_CPU flag, registers can either be loaded on the HW or be in memory.
Even worse with NV, where the loaded state is by definition partial.
For these reasons, KVM offers the vcpu_read_sys_reg() and vcpu_write_sys_reg() primitives that always do the right thing. However, these primitive must know what register to access, and this is the role of the __vcpu_read_sys_reg_from_cpu() and __vcpu_write_sys_reg_to_cpu() helpers.
As it turns out, TCR2_EL1, PIR_EL1, PIRE0_EL1 and not described in the latter helpers, meaning that the AT code cannot use them to emulate S1PIE.
Add the three registers to the (long) list.
--- arch/arm64/include/asm/kvm_host.h | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/include/asm/kvm_host.h +6 -0
@@ -1030,6 +1030,9 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)case TTBR0_EL1 : *val =read_sysreg_s (SYS_TTBR0_EL12 );break ;case TTBR1_EL1 : *val =read_sysreg_s (SYS_TTBR1_EL12 );break ;case TCR_EL1 : *val =read_sysreg_s (SYS_TCR_EL12 );break ; +case TCR2_EL1 : *val =read_sysreg_s (SYS_TCR2_EL12 );break ; +case PIR_EL1 : *val =read_sysreg_s (SYS_PIR_EL12 );break ; +case PIRE0_EL1 : *val =read_sysreg_s (SYS_PIRE0_EL12 );break ;case ESR_EL1 : *val =read_sysreg_s (SYS_ESR_EL12 );break ;case AFSR0_EL1 : *val =read_sysreg_s (SYS_AFSR0_EL12 );break ;case AFSR1_EL1 : *val =read_sysreg_s (SYS_AFSR1_EL12 );break ;
@@ -1076,6 +1079,9 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)case TTBR0_EL1 :write_sysreg_s (val,SYS_TTBR0_EL12 );break ;case TTBR1_EL1 :write_sysreg_s (val,SYS_TTBR1_EL12 );break ;case TCR_EL1 :write_sysreg_s (val,SYS_TCR_EL12 );break ; +case TCR2_EL1 :write_sysreg_s (val,SYS_TCR2_EL12 );break ; +case PIR_EL1 :write_sysreg_s (val,SYS_PIR_EL12 );break ; +case PIRE0_EL1 :write_sysreg_s (val,SYS_PIRE0_EL12 );break ;case ESR_EL1 :write_sysreg_s (val,SYS_ESR_EL12 );break ;case AFSR0_EL1 :write_sysreg_s (val,SYS_AFSR0_EL12 );break ;case AFSR1_EL1 :write_sysreg_s (val,SYS_AFSR1_EL12 );break ;
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 9/36 KVM: arm64: Extend masking facility to arbitrary registers
In reply to Marc ZyngierWe currently only use the masking (RES0/RES1) facility for VNCR registers, as they are memory-based and thus easy to sanitise.
But we could apply the same thing to other registers if we:
- split the sanitisation from __VNCR_START__
- apply the sanitisation when reading from a HW register
This involves a new "marker" in the vcpu_sysreg enum, which defines the point at which the sanitisation applies (the VNCR registers being of course after this marker).
Whle we are at it, rename kvm_vcpu_sanitise_vncr_reg() to kvm_vcpu_apply_reg_masks(), which is vaguely more explicit, and harden set_sysreg_masks() against setting masks for random registers...
--- arch/arm64/include/asm/kvm_host.h | 19 +++++++++++++------ arch/arm64/kvm/nested.c | 12 ++++++++---- arch/arm64/kvm/sys_regs.c | 3 +++ 3 files changed, 24 insertions(+), 10 deletions(-)
arch/arm64/include/asm/kvm_host.h +13 -6
@@ -367,7 +367,7 @@ struct kvm_arch {u64 ctr_el0; -/* Masks for VNCR-baked sysregs */ +/* Masks for VNCR-backed and general EL2 sysregs */ struct kvm_sysreg_masks *sysreg_masks;/*
@@ -401,6 +401,9 @@ struct kvm_vcpu_fault_info { r = __VNCR_START__ + ((VNCR_ ## r) / 8), \ __after_##r = __MAX__(__before_##r - 1, r) +#define MARKER (m) \ + m, __after_##m = m - 1 +enum vcpu_sysreg { __INVALID_SYSREG__,/* 0 is reserved as an invalid value */ MPIDR_EL1 ,/* MultiProcessor Affinity Register */
@@ -487,7 +490,11 @@ enum vcpu_sysreg {CNTHV_CTL_EL2 ,CNTHV_CVAL_EL2 , - __VNCR_START__,/* Any VNCR-capable reg goes after this point */ +/* Anything from this can be RES0/RES1 sanitised */ +MARKER (__SANITISED_REG_START__), + +/* Any VNCR-capable reg goes after this point */ +MARKER (__VNCR_START__),VNCR (SCTLR_EL1 ),/* System Control Register */ VNCR(ACTLR_EL1),/* Auxiliary Control Register */
@@ -547,7 +554,7 @@ struct kvm_sysreg_masks {struct {u64 res0;u64 res1; - } mask[NR_SYS_REGS - __VNCR_START__]; + } mask[NR_SYS_REGS - __SANITISED_REG_START__]; };struct kvm_cpu_context {
@@ -995,13 +1002,13 @@ static inline u64 *___ctxt_sys_reg(const struct kvm_cpu_context *ctxt, int r)#define ctxt_sys_reg (c,r) (*__ctxt_sys_reg(c,r)) -u64 kvm_vcpu_sanitise_vncr_reg (const struct kvm_vcpu *,enum vcpu_sysreg ); +u64 kvm_vcpu_apply_reg_masks (const struct kvm_vcpu *,enum vcpu_sysreg ,u64 );#define __vcpu_sys_reg (v,r) \ (*({ \ const struct kvm_cpu_context *ctxt = &(v)->arch.ctxt; \ u64 *__r = __ctxt_sys_reg(ctxt, (r)); \ - if (vcpu_has_nv((v)) && (r) >= __VNCR_START__) \ - *__r = kvm_vcpu_sanitise_vncr_reg((v), (r)); \ + if (vcpu_has_nv((v)) && (r) >= __SANITISED_REG_START__) \ + *__r = kvm_vcpu_apply_reg_masks((v), (r), *__r);\ __r; \ }))
arch/arm64/kvm/nested.c +8 -4
@@ -908,15 +908,15 @@ static void limit_nv_id_regs(struct kvm *kvm)kvm_set_vm_id_reg (kvm,SYS_ID_AA64DFR0_EL1 , val); } -u64 kvm_vcpu_sanitise_vncr_reg (const struct kvm_vcpu *vcpu,enum vcpu_sysreg sr) +u64 kvm_vcpu_apply_reg_masks (const struct kvm_vcpu *vcpu, +enum vcpu_sysreg sr,u64 v) { -u64 v =ctxt_sys_reg (&vcpu->arch.ctxt, sr);struct kvm_sysreg_masks *masks; masks = vcpu->kvm->arch.sysreg_masks;if (masks) { - sr -= __VNCR_START__; + sr -= __SANITISED_REG_START__; v &= ~masks->mask[sr].res0; v |= masks->mask[sr].res1;
@@ -927,7 +927,11 @@ u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg sr)static void set_sysreg_masks (struct kvm *kvm,int sr,u64 res0,u64 res1) { -int i = sr - __VNCR_START__; +int i = sr - __SANITISED_REG_START__; + +BUILD_BUG_ON (!__builtin_constant_p (sr)); +BUILD_BUG_ON (sr < __SANITISED_REG_START__); +BUILD_BUG_ON (sr >=NR_SYS_REGS ); kvm->arch.sysreg_masks->mask[i].res0 = res0; kvm->arch.sysreg_masks->mask[i].res1 = res1;
arch/arm64/kvm/sys_regs.c +3 -0
@@ -189,6 +189,9 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)/* Get the current version of the EL1 counterpart. */ WARN_ON (!__vcpu_read_sys_reg_from_cpu (el1r, &val)); +if (reg >= __SANITISED_REG_START__) + val =kvm_vcpu_apply_reg_masks (vcpu, reg, val); +return val; }
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 10/36 arm64: Define ID_AA64MMFR1_EL1.HAFDBS advertising FEAT_HAFT
In reply to Marc ZyngierThis definition is missing, and we are going to need it to sanitise TCR2_ELx.
--- arch/arm64/tools/sysreg | 1 + 1 file changed, 1 insertion(+)
arch/arm64/tools/sysreg +1 -0
@@ -1688,6 +1688,7 @@ UnsignedEnum 3:0 HAFDBS 0b0000 NI 0b0001 AF 0b0010 DBM + 0b0011 HAFT EndEnum EndSysreg
Signature · 1 line
2.39.2
Re: [PATCH v4 10/36] arm64: Define ID_AA64MMFR1_EL1.HAFDBS advertising FEAT_HAFT
arch/arm64/tools/sysreg UnsignedEnum 3:0 HAFDBS · line 1692
Quoted 13 lines · Marc Zyngier
This definition is missing, and we are going to need it to sanitise
TCR2_ELx.arch/arm64/tools/sysreg +1
-0@@ -1688,6 +1688,7 @@ UnsignedEnum 3:0 HAFDBS 0b0000 NI 0b0001 AF 0b0010 DBM + 0b0011 HAFT EndEnum
This is correct in so far as it goes, but we are also mising HDBSS according to the 2024-09 XML, plus a couple of new revisions of ETS. I'll send a patch doing the full update, it should just be a trivial add/add conflict.
It tends to make life a bit easier to note which version of the spec is being referenced with these updates.
Marc Zyngier
maintainerPATCH v4 11/36 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, the sysreg descriptor array, and advertise it as mapped to TCR2_EL1 for NV purposes.
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 | 14 ++++++++++++++ 2 files changed, 15 insertions(+)
arch/arm64/include/asm/kvm_host.h +1 -0
@@ -492,6 +492,7 @@ enum vcpu_sysreg {/* Anything from this can be RES0/RES1 sanitised */ MARKER (__SANITISED_REG_START__), +TCR2_EL2 ,/* Extended Translation Control Register (EL2) */ /* Any VNCR-capable reg goes after this point */ MARKER(__VNCR_START__),
arch/arm64/kvm/sys_regs.c +14 -0
@@ -134,6 +134,7 @@ static bool get_el2_to_el1_mapping(unsigned int reg,MAPPED_EL2_SYSREG (ESR_EL2 ,ESR_EL1 ,NULL );MAPPED_EL2_SYSREG (FAR_EL2 ,FAR_EL1 ,NULL );MAPPED_EL2_SYSREG (MAIR_EL2 ,MAIR_EL1 ,NULL ); +MAPPED_EL2_SYSREG (TCR2_EL2 ,TCR2_EL1 ,NULL );MAPPED_EL2_SYSREG (AMAIR_EL2 ,AMAIR_EL1 ,NULL );MAPPED_EL2_SYSREG (ELR_EL2 ,ELR_EL1 ,NULL );MAPPED_EL2_SYSREG (SPSR_EL2 ,SPSR_EL1 ,NULL );
@@ -452,6 +453,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)
@@ -2830,6 +2843,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 v4 12/36 KVM: arm64: Sanitise TCR2_EL2
In reply to Marc ZyngierTCR2_EL2 is a bag of control bits, all of which are only valid if certain features are present, and RES0 otherwise.
Describe these constraints and register them with the masking infrastructure.
--- arch/arm64/kvm/nested.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
arch/arm64/kvm/nested.c +22 -0
@@ -1179,6 +1179,28 @@ int kvm_init_nv_sysregs(struct kvm *kvm) res0 |= ~(res0 | res1);set_sysreg_masks (kvm,HAFGRTR_EL2 , res0, res1); +/* TCR2_EL2 */ + res0 =TCR2_EL2_RES0 ; + res1 =TCR2_EL2_RES1 ; +if (!kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,D128 ,IMP )) + res0 |= (TCR2_EL2_DisCH0 | TCR2_EL2_DisCH1 |TCR2_EL2_D128 ); +if (!kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,MEC ,IMP )) + res0 |=TCR2_EL2_AMEC1 |TCR2_EL2_AMEC0 ; +if (!kvm_has_feat (kvm,ID_AA64MMFR1_EL1 ,HAFDBS ,HAFT )) + res0 |=TCR2_EL2_HAFT ; +if (!kvm_has_feat (kvm,ID_AA64PFR1_EL1 ,THE ,IMP )) + res0 |=TCR2_EL2_PTTWI | TCR2_EL2_PnCH; +if (!kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,AIE ,IMP )) + res0 |=TCR2_EL2_AIE ; +if (!kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,S1POE ,IMP )) + res0 |=TCR2_EL2_POE |TCR2_EL2_E0POE ; +if (!kvm_has_feat (kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) + res0 |=TCR2_EL2_PIE ; +if (!kvm_has_feat (kvm,ID_AA64MMFR1_EL1 ,VH ,IMP )) + res0 |= (TCR2_EL2_E0POE |TCR2_EL2_D128 | +TCR2_EL2_AMEC1 | TCR2_EL2_DisCH0 | TCR2_EL2_DisCH1); +set_sysreg_masks (kvm,TCR2_EL2 , res0, res1); +/* SCTLR_EL1 */ res0 =SCTLR_EL1_RES0 ; res1 =SCTLR_EL1_RES1 ;
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 13/36 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 | 7 +++++++ 1 file changed, 7 insertions(+)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +7 -0
@@ -51,6 +51,9 @@ static void __sysreg_save_vel2_state(struct kvm_vcpu *vcpu)__vcpu_sys_reg (vcpu,TTBR1_EL2 ) =read_sysreg_el1 (SYS_TTBR1 );__vcpu_sys_reg (vcpu,TCR_EL2 ) =read_sysreg_el1 (SYS_TCR ); +if (ctxt_has_tcrx (&vcpu->arch.ctxt)) +__vcpu_sys_reg (vcpu,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,10 @@ static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)write_sysreg_el1 (val,SYS_TCR ); } +if (ctxt_has_tcrx (&vcpu->arch.ctxt)) +write_sysreg_el1 (__vcpu_sys_reg (vcpu,TCR2_EL2 ),SYS_TCR2 ); + +write_sysreg_el1 (__vcpu_sys_reg (vcpu,ESR_EL2 ),SYS_ESR );write_sysreg_el1 (__vcpu_sys_reg (vcpu,AFSR0_EL2 ),SYS_AFSR0 );write_sysreg_el1 (__vcpu_sys_reg (vcpu,AFSR1_EL2 ),SYS_AFSR1 );
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 14/36 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 ++ arch/arm64/kvm/sys_regs.c | 2 ++ 2 files changed, 4 insertions(+)
arch/arm64/include/asm/kvm_host.h +2 -0
@@ -471,6 +471,8 @@ 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) */ +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) */
arch/arm64/kvm/sys_regs.c +2 -0
@@ -135,6 +135,8 @@ static bool get_el2_to_el1_mapping(unsigned int reg,MAPPED_EL2_SYSREG (FAR_EL2 ,FAR_EL1 ,NULL );MAPPED_EL2_SYSREG (MAIR_EL2 ,MAIR_EL1 ,NULL );MAPPED_EL2_SYSREG (TCR2_EL2 ,TCR2_EL1 ,NULL ); +MAPPED_EL2_SYSREG (PIR_EL2 ,PIR_EL1 ,NULL ); +MAPPED_EL2_SYSREG (PIRE0_EL2 ,PIRE0_EL1 ,NULL );MAPPED_EL2_SYSREG (AMAIR_EL2 ,AMAIR_EL1 ,NULL );MAPPED_EL2_SYSREG (ELR_EL2 ,ELR_EL1 ,NULL );MAPPED_EL2_SYSREG (SPSR_EL2 ,SPSR_EL1 ,NULL );
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 15/36 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 | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +13 -2
@@ -51,9 +51,15 @@ static void __sysreg_save_vel2_state(struct kvm_vcpu *vcpu)__vcpu_sys_reg (vcpu,TTBR1_EL2 ) =read_sysreg_el1 (SYS_TTBR1 );__vcpu_sys_reg (vcpu,TCR_EL2 ) =read_sysreg_el1 (SYS_TCR ); -if (ctxt_has_tcrx (&vcpu->arch.ctxt)) +if (ctxt_has_tcrx (&vcpu->arch.ctxt)) {__vcpu_sys_reg (vcpu,TCR2_EL2 ) =read_sysreg_el1 (SYS_TCR2 ); +if (ctxt_has_s1pie (&vcpu->arch.ctxt)) { +__vcpu_sys_reg (vcpu,PIRE0_EL2 ) =read_sysreg_el1 (SYS_PIRE0 ); +__vcpu_sys_reg (vcpu,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,14 @@ static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)write_sysreg_el1 (val,SYS_TCR ); } -if (ctxt_has_tcrx (&vcpu->arch.ctxt)) +if (ctxt_has_tcrx (&vcpu->arch.ctxt)) {write_sysreg_el1 (__vcpu_sys_reg (vcpu,TCR2_EL2 ),SYS_TCR2 ); +if (ctxt_has_s1pie (&vcpu->arch.ctxt)) { +write_sysreg_el1 (__vcpu_sys_reg (vcpu,PIR_EL2 ),SYS_PIR ); +write_sysreg_el1 (__vcpu_sys_reg (vcpu,PIRE0_EL2 ),SYS_PIRE0 ); + } + }write_sysreg_el1 (__vcpu_sys_reg (vcpu,ESR_EL2 ),SYS_ESR );write_sysreg_el1 (__vcpu_sys_reg (vcpu,AFSR0_EL2 ),SYS_AFSR0 );
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 16/36 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
@@ -369,6 +369,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). */
@@ -2873,6 +2885,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 v4 17/36 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 | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
arch/arm64/kvm/sys_regs.c +5 -2
@@ -1617,7 +1617,9 @@ 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 );
@@ -2500,7 +2502,8 @@ 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_S1POE )), +ID_AA64MMFR3_EL1_S1POE | +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 v4 18/36 KVM: arm64: Add AT fast-path support for S1PIE
In reply to Marc ZyngierEmulating AT using AT instructions requires that the live state matches the translation regime the AT instruction targets.
If targeting the EL1&0 translation regime and that S1PIE is supported, we also need to restore that state (covering TCR2_EL1, PIR_EL1, and PIRE0_EL1).
Add the required system register switcheroo.
--- arch/arm64/kvm/at.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
arch/arm64/kvm/at.c +24 -0
@@ -412,6 +412,9 @@ struct mmu_config {u64 ttbr1;u64 tcr;u64 mair; +u64 tcr2; +u64 pir; +u64 pire0;u64 sctlr;u64 vttbr;u64 vtcr;
@@ -424,6 +427,13 @@ static void __mmu_config_save(struct mmu_config *config) config->ttbr1 =read_sysreg_el1 (SYS_TTBR1 ); config->tcr =read_sysreg_el1 (SYS_TCR ); config->mair =read_sysreg_el1 (SYS_MAIR ); +if (cpus_have_final_cap (ARM64_HAS_TCR2 )) { + config->tcr2 =read_sysreg_el1 (SYS_TCR2 ); +if (cpus_have_final_cap (ARM64_HAS_S1PIE )) { + config->pir =read_sysreg_el1 (SYS_PIR ); + config->pire0 =read_sysreg_el1 (SYS_PIRE0 ); + } + } config->sctlr =read_sysreg_el1 (SYS_SCTLR ); config->vttbr =read_sysreg (vttbr_el2); config->vtcr =read_sysreg (vtcr_el2);
@@ -444,6 +454,13 @@ static void __mmu_config_restore(struct mmu_config *config)write_sysreg_el1 (config->ttbr1,SYS_TTBR1 );write_sysreg_el1 (config->tcr,SYS_TCR );write_sysreg_el1 (config->mair,SYS_MAIR ); +if (cpus_have_final_cap (ARM64_HAS_TCR2 )) { +write_sysreg_el1 (config->tcr2,SYS_TCR2 ); +if (cpus_have_final_cap (ARM64_HAS_S1PIE )) { +write_sysreg_el1 (config->pir,SYS_PIR ); +write_sysreg_el1 (config->pire0,SYS_PIRE0 ); + } + }write_sysreg_el1 (config->sctlr,SYS_SCTLR );write_sysreg (config->vttbr, vttbr_el2);write_sysreg (config->vtcr, vtcr_el2);
@@ -914,6 +931,13 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)write_sysreg_el1 (vcpu_read_sys_reg (vcpu,TTBR1_EL1 ),SYS_TTBR1 );write_sysreg_el1 (vcpu_read_sys_reg (vcpu,TCR_EL1 ),SYS_TCR );write_sysreg_el1 (vcpu_read_sys_reg (vcpu,MAIR_EL1 ),SYS_MAIR ); +if (kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,TCRX ,IMP )) { +write_sysreg_el1 (vcpu_read_sys_reg (vcpu,TCR2_EL1 ),SYS_TCR2 ); +if (kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) { +write_sysreg_el1 (vcpu_read_sys_reg (vcpu,PIR_EL1 ),SYS_PIR ); +write_sysreg_el1 (vcpu_read_sys_reg (vcpu,PIRE0_EL1 ),SYS_PIRE0 ); + } + }write_sysreg_el1 (vcpu_read_sys_reg (vcpu,SCTLR_EL1 ),SYS_SCTLR );__load_stage2 (mmu, mmu->arch);
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 19/36 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 as part of the walk_result structure.
--- arch/arm64/kvm/at.c | 162 +++++++++++++++++++++++++++----------------- 1 file changed, 98 insertions(+), 64 deletions(-)
arch/arm64/kvm/at.c +98 -64
@@ -37,6 +37,12 @@ struct s1_walk_result {u8 APTable;bool UXNTable;bool PXNTable; +bool ur; +bool uw; +bool ux; +bool pr; +bool pw; +bool px; };struct {u8 fst;
@@ -764,111 +770,139 @@ 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) { -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; + wr->pr = wr->pw = true; + wr->ur = wr->uw = false;break ;case 0b01 : - pr = pw = ur = uw = true; + wr->pr = wr->pw = wr->ur = wr->uw = true;break ;case 0b10 : - pr = true; - pw = ur = uw = false; + wr->pr = true; + wr->pw = wr->ur = wr->uw = false;break ;case 0b11 : - pr = ur = true; - pw = uw = false; + wr->pr = wr->ur = true; + wr->pw = wr->uw = false;break ; } -switch (wr.APTable) { +/* We don't use px for anything yet, but hey... */ + wr->px = !((wr->desc &PTE_PXN ) || wr->uw); + wr->ux = !(wr->desc &PTE_UXN ); + }else { + wr->ur = wr->uw = wr->ux = false; + +if (!(wr->desc &PTE_RDONLY )) { + wr->pr = wr->pw = true; + }else { + wr->pr = true; + wr->pw = false; + } + +/* XN maps to UXN */ + wr->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) +{ +/* Hierarchical part of AArch64.S1DirectBasePermissions() */ +if (wi->regime !=TR_EL2 ) { +switch (wr->APTable) {case 0b00 :break ;case 0b01 : - ur = uw = false; + wr->ur = wr->uw = false;break ;case 0b10 : - pw = uw = false; + wr->pw = wr->uw = false;break ;case 0b11 : - pw = ur = uw = false; + wr->pw = wr->ur = wr->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); + wr->px &= !wr->PXNTable; + wr->ux &= !wr->UXNTable; + }else { +if (wr->APTable &BIT (1 )) + wr->pw = false; -if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) { -bool pan; +/* XN maps to UXN */ + wr->px &= !wr->UXNTable; + } +} - pan = *vcpu_cpsr (vcpu) &PSR_PAN_BIT ; - pan &= ur || uw || (pan3_enabled (vcpu, wi.regime) && ux); - pw &= !pan; - pr &= !pan; - } - }else { - ur = uw = ux = false; +static void compute_s1_permissions (struct kvm_vcpu *vcpu,u32 op, +struct s1_walk_info *wi, +struct s1_walk_result *wr) +{ +compute_s1_direct_permissions (vcpu, wi, wr); -if (!(wr.desc &PTE_RDONLY )) { - pr = pw = true; - }else { - pr = true; - pw = false; - } +if (!wi->hpd) +compute_s1_hierarchical_permissions (vcpu, wi, wr); -if (wr.APTable &BIT (1 )) - pw = false; +if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) { +bool pan; -/* XN maps to UXN */ - px = !((wr.desc &PTE_UXN ) || wr.UXNTable); + pan = *vcpu_cpsr (vcpu) &PSR_PAN_BIT ; + pan &= wr->ur || wr->uw || (pan3_enabled (vcpu, wi->regime) && wr->ux); + wr->pw &= !pan; + wr->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 = {}; +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);switch (op) {case OP_AT_S1E1RP :case OP_AT_S1E1R :case OP_AT_S1E2R : - perm_fail = !pr; + perm_fail = !wr.pr;break ;case OP_AT_S1E1WP :case OP_AT_S1E1W :case OP_AT_S1E2W : - perm_fail = !pw; + perm_fail = !wr.pw;break ;case OP_AT_S1E0R : - perm_fail = !ur; + perm_fail = !wr.ur;break ;case OP_AT_S1E0W : - perm_fail = !uw; + perm_fail = !wr.uw;break ;case OP_AT_S1E1A :case OP_AT_S1E2A :
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 20/36 KVM: arm64: Disable hierarchical permissions when S1PIE is enabled
In reply to Marc ZyngierS1PIE implicitly disables hierarchical permissions, as specified in R_JHSVW, by making TCR_ELx.HPDn RES1.
Add a predicate for S1PIE being enabled for a given translation regime, and emulate this behaviour by forcing the hpd field to true if S1PIE is enabled for that translation regime.
--- arch/arm64/kvm/at.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
arch/arm64/kvm/at.c +19 -0
@@ -93,6 +93,23 @@ static enum trans_regime compute_translation_regime(struct kvm_vcpu *vcpu, u32 o } } +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_read_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 int setup_s1_walk (struct kvm_vcpu *vcpu,u32 op,struct s1_walk_info *wi,struct s1_walk_result *wr,u64 va) {
@@ -186,6 +203,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi, (va55 ?FIELD_GET (TCR_HPD1 , tcr) : FIELD_GET(TCR_HPD0, tcr))); +/* R_JHSVW */ + wi->hpd |=s1pie_enabled (vcpu, wi->regime);/* Someone was silly enough to encode TG0/TG1 differently */ if (va55) {
Signature · 1 line
2.39.2
Re: [PATCH v4 20/36] KVM: arm64: Disable hierarchical permissions when S1PIE is enabled
Quoted 24 lines · Marc Zyngier
S1PIE implicitly disables hierarchical permissions, as specified in
R_JHSVW, by making TCR_ELx.HPDn RES1.Add a predicate for S1PIE being enabled for a given translation regime, and emulate this behaviour by forcing the hpd field to true if S1PIE is enabled for that translation regime.
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/at.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)arch/arm64/kvm/at.c +4
-0@@ -93,6 +93,23 @@ static enum trans_regime compute_translation_regime(struct kvm_vcpu *vcpu, u32 o } } +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;
arch/arm64/kvm/at.c static enum trans_regime compute_translation_regime(struct kvm_vcpu *vcpu, u32 o · line 109
Quoted 10 lines
arch/arm64/kvm/at.c +10
-0+ +switch (regime) { +case TR_EL2 : +case TR_EL20 : +return vcpu_read_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 ();
BUILD_BUG()?
Quoted 18 lines
arch/arm64/kvm/at.c +5
-0+ } +} +static int setup_s1_walk (struct kvm_vcpu *vcpu,u32 op,struct s1_walk_info *wi,struct s1_walk_result *wr,u64 va) {@@ -186,6 +203,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi, (va55 ?FIELD_GET (TCR_HPD1 , tcr) : FIELD_GET(TCR_HPD0, tcr))); +/* R_JHSVW */ + wi->hpd |=s1pie_enabled (vcpu, wi->regime);/* Someone was silly enough to encode TG0/TG1 differently */ if (va55) {Signature · 1 line
2.39.2
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerRe: [PATCH v4 20/36] KVM: arm64: Disable hierarchical permissions when S1PIE is enabled
Quoted 22 lines · Oliver Upton
Quoted 20 lines · Marc Zyngier
S1PIE implicitly disables hierarchical permissions, as specified in
R_JHSVW, by making TCR_ELx.HPDn RES1.Add a predicate for S1PIE being enabled for a given translation regime, and emulate this behaviour by forcing the hpd field to true if S1PIE is enabled for that translation regime.
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/at.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)arch/arm64/kvm/at.c +0
-0@@ -93,6 +93,23 @@ static enum trans_regime compute_translation_regime(struct kvm_vcpu *vcpu, u32 o } }
Quoted 16 lines
Quoted 4 lines
arch/arm64/kvm/at.c +4
-0+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;arch/arm64/kvm/at.c static enum trans_regime compute_translation_regime(struct kvm_vcpu *vcpu, u32 o · line 109
Quoted 10 lines
arch/arm64/kvm/at.c +10
-0+ +switch (regime) { +case TR_EL2 : +case TR_EL20 : +return vcpu_read_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 ();
BUILD_BUG()?
Doesn't work. regime is fished out of the walk_info structure, meaning that it is pretty opaque to the compiler, which in turn needs to emit code for all cases.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v4 21/36 KVM: arm64: Implement AT S1PIE support
In reply to Marc ZyngierIt doesn't take much effort to implement S1PIE support in AT.
It 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 | 117 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-)
arch/arm64/kvm/at.c +116 -1
@@ -781,6 +781,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
@@ -862,11 +865,123 @@ static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, } } +#define perm_idx (v, r, i) ((vcpu_read_sys_reg((v), (r)) >> ((i) * 4)) & 0xf) + +#define set_priv_perms (wr, r, w, x) \ + do { \ + (wr)->pr = (r); \ + (wr)->pw = (w); \ + (wr)->px = (x); \ + } while (0) + +#define set_unpriv_perms (wr, r, w, x) \ + do { \ + (wr)->ur = (r); \ + (wr)->uw = (w); \ + (wr)->ux = (x); \ + } while (0) + +/* Similar to AArch64.S1IndirectBasePermissions(), without GCS */ +#define set_perms(w, wr, ip) \ + do { \ + /* R_LLZDZ */ \ + switch ((ip)) { \ + case 0b0000: \ + set_ ## w ## _perms((wr), false, false, false); \ + break; \ + case 0b0001: \ + set_ ## w ## _perms((wr), true , false, false); \ + break; \ + case 0b0010: \ + set_ ## w ## _perms((wr), false, false, true ); \ + break; \ + case 0b0011: \ + set_ ## w ## _perms((wr), true , false, true ); \ + break; \ + case 0b0100: \ + set_ ## w ## _perms((wr), false, false, false); \ + break; \ + case 0b0101: \ + set_ ## w ## _perms((wr), true , true , false); \ + break; \ + case 0b0110: \ + set_ ## w ## _perms((wr), true , true , true ); \ + break; \ + case 0b0111: \ + set_ ## w ## _perms((wr), true , true , true ); \ + break; \ + case 0b1000: \ + set_ ## w ## _perms((wr), true , false, false); \ + break; \ + case 0b1001: \ + set_ ## w ## _perms((wr), true , false, false); \ + break; \ + case 0b1010: \ + set_ ## w ## _perms((wr), true , false, true ); \ + break; \ + case 0b1011: \ + set_ ## w ## _perms((wr), false, false, false); \ + break; \ + case 0b1100: \ + set_ ## w ## _perms((wr), true , true , false); \ + break; \ + case 0b1101: \ + set_ ## w ## _perms((wr), false, false, false); \ + break; \ + case 0b1110: \ + set_ ## w ## _perms((wr), true , true , true ); \ + break; \ + case 0b1111: \ + set_ ## w ## _perms((wr), 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) +{ +u8 up, pp, idx; + + idx =pte_pi_index (wr->desc); + +switch (wi->regime) { +case TR_EL10 : + pp =perm_idx (vcpu,PIR_EL1 , idx); + up =perm_idx (vcpu,PIRE0_EL1 , idx); +break ; +case TR_EL20 : + pp =perm_idx (vcpu,PIR_EL2 , idx); + up =perm_idx (vcpu,PIRE0_EL2 , idx); +break ; +case TR_EL2 : + pp =perm_idx (vcpu,PIR_EL2 , idx); + up =0 ; +break ; + } + +set_perms (priv, wr, pp); + +if (wi->regime !=TR_EL2 ) +set_perms (unpriv, wr, up); +else +set_unpriv_perms (wr, false, false, false); + +/* R_VFPJF */ +if (wr->px && wr->uw) { +set_priv_perms (wr, false, false, false); +set_unpriv_perms (wr, false, false, false); + } +} +static void compute_s1_permissions (struct kvm_vcpu *vcpu,u32 op,struct s1_walk_info *wi,struct s1_walk_result *wr) { -compute_s1_direct_permissions (vcpu, wi, wr); +if (!s1pie_enabled (vcpu, wi->regime)) +compute_s1_direct_permissions (vcpu, wi, wr); +else +compute_s1_indirect_permissions (vcpu, wi, wr);if (!wi->hpd)compute_s1_hierarchical_permissions (vcpu, wi, wr);
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 22/36 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
@@ -2164,6 +2164,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)
@@ -2852,8 +2861,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 v4 23/36 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
@@ -1511,4 +1511,7 @@ 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_tcr2 (k) \ + (kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP)) +#endif /* __ARM64_KVM_HOST_H__ */
arch/arm64/kvm/sys_regs.c +26 -3
@@ -2348,6 +2348,27 @@ static unsigned int s1poe_visibility(const struct kvm_vcpu *vcpu,return REG_HIDDEN ; } +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
@@ -2532,7 +2553,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),
@@ -2869,7 +2891,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),
@@ -4738,7 +4761,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;if (kvm_has_fpmr (kvm))
Signature · 1 line
2.39.2
Re: [PATCH v4 23/36] KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests
Quoted 5 lines · Marc Zyngier
From: Mark Brown <[email protected]>
When the guest does not support
FEAT_TCR2we 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.
This should come at the beginning of ths series (same for the subsequent S1PIE patch) so the EL2 registers use the correct visibility filtering from the start.
Quoted 14 lines
Fixes fbff56068232 ("KVM: arm64: Save/restore TCR2_EL1")Signed-off-by Mark Brown <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- 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 +0
-0
arch/arm64/include/asm/kvm_host.h void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val); · line 1516
Quoted 6 lines
arch/arm64/include/asm/kvm_host.h +3
-0@@ -1511,4 +1511,7 @@ 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_tcr2 (k) \ + (kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP)) +
nit: we should consistently use this predicate if we want to keep it.
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerPATCH v4 24/36 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
@@ -1514,4 +1514,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
@@ -2369,6 +2369,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
@@ -2608,8 +2629,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_POR_EL1), NULL, reset_unknown, POR_EL1, .visibility = s1poe_visibility }, { SYS_DESC(SYS_AMAIR_EL1), access_vm_reg, reset_amair_el1, AMAIR_EL1 },
@@ -2920,8 +2943,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),
@@ -4811,7 +4836,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 v4 25/36 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 | 34 +++------------------------------- 1 file changed, 3 insertions(+), 31 deletions(-)
arch/arm64/kvm/sys_regs.c +3 -31
@@ -369,18 +369,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). */
@@ -445,10 +433,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 )) -return undef_access (vcpu, p, r); -BUG_ON (!p->is_write);get_access_mask (r, &mask, &shift);
@@ -467,18 +451,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)
@@ -2914,7 +2886,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),
@@ -2943,9 +2915,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
Marc Zyngier
maintainerPATCH v4 26/36 arm64: Add encoding for POR_EL2
In reply to Marc ZyngierPOR_EL2 is the equivalent of POR_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
@@ -2907,6 +2907,10 @@ Sysreg POR_EL1 3 0 10 2 4 Fields PIRx_ELx EndSysreg +Sysreg POR_EL2 3 4 10 2 4 +Fields PIRx_ELx +EndSysreg + Sysreg POR_EL12 3 5 10 2 4 Fields PIRx_ELx EndSysreg
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 27/36 KVM: arm64: Add a composite EL2 visibility helper
In reply to Marc ZyngierWe are starting to have a bunch of visibility helpers checking for EL2 + something else, and we are going to add more.
Simplify things somehow by introducing a helper that implement extractly that by taking a visibility helper as a parameter, and convert the existing ones to that.
--- arch/arm64/kvm/sys_regs.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-)
arch/arm64/kvm/sys_regs.c +11 -21
@@ -2277,16 +2277,18 @@ static u64 reset_hcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)return __vcpu_sys_reg (vcpu, r->reg) = val; } +static unsigned int __el2_visibility (const struct kvm_vcpu *vcpu, +const struct sys_reg_desc *rd, +unsigned int (*fn)(const struct kvm_vcpu *, +const struct sys_reg_desc *)) +{ +return el2_visibility (vcpu, rd) ?:fn (vcpu, rd); +} +static unsigned int sve_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 sve_visibility (vcpu, rd); +return __el2_visibility (vcpu, rd, sve_visibility); } static bool access_zcr_el2(struct kvm_vcpu *vcpu,
@@ -2332,13 +2334,7 @@ static unsigned int tcr2_visibility(const struct kvm_vcpu *vcpu,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); +return __el2_visibility (vcpu, rd, tcr2_visibility); } static unsigned int s1pie_visibility(const struct kvm_vcpu *vcpu,
@@ -2353,13 +2349,7 @@ static unsigned int s1pie_visibility(const struct kvm_vcpu *vcpu,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); +return __el2_visibility (vcpu, rd, s1pie_visibility); }/*
Signature · 1 line
2.39.2
Re: [PATCH v4 27/36] KVM: arm64: Add a composite EL2 visibility helper
Quoted 8 lines · Marc Zyngier
We are starting to have a bunch of visibility helpers checking for EL2 + something else, and we are going to add more.
Simplify things somehow by introducing a helper that implement extractly that by taking a visibility helper as a parameter, and convert the existing ones to that.
Signed-off-by Marc Zyngier <[email protected]>
nit: avoid churn and order this before introducing more open-coded instances of EL2 + FEAT_something checks.
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerPATCH v4 28/36 KVM: arm64: Drop bogus CPTR_EL2.E0POE trap routing
In reply to Marc ZyngierIt took me some time to realise it, but CPTR_EL2.E0POE does not apply to a guest, only to EL0 when InHost(). And when InHost(), CPCR_EL2 is mapped to CPACR_EL1, maning that the E0POE bit naturally takes effect without any trap.
To sum it up, this trap bit is better left ignored, we will never have to hanedle it.
--- arch/arm64/kvm/emulate-nested.c | 8 -------- 1 file changed, 8 deletions(-)
arch/arm64/kvm/emulate-nested.c +0 -8
@@ -79,7 +79,6 @@ enum cgt_group_id {CGT_MDCR_E2TB ,CGT_MDCR_TDCC , -CGT_CPACR_E0POE ,CGT_CPTR_TAM , CGT_CPTR_TCPAC,
@@ -362,12 +361,6 @@ static const struct trap_bits coarse_trap_bits[] = { .mask = MDCR_EL2_TDCC, .behaviour = BEHAVE_FORWARD_ANY, }, - [CGT_CPACR_E0POE] = { - .index = CPTR_EL2, - .value = CPACR_ELx_E0POE, - .mask = CPACR_ELx_E0POE, - .behaviour = BEHAVE_FORWARD_ANY, - }, [CGT_CPTR_TAM] = { .index = CPTR_EL2, .value = CPTR_EL2_TAM,
@@ -1141,7 +1134,6 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {SR_TRAP (SYS_AMEVTYPER1_EL0 (13 ),CGT_CPTR_TAM ),SR_TRAP (SYS_AMEVTYPER1_EL0 (14 ),CGT_CPTR_TAM ),SR_TRAP (SYS_AMEVTYPER1_EL0 (15 ),CGT_CPTR_TAM ), -SR_TRAP (SYS_POR_EL0 ,CGT_CPACR_E0POE ),/* op0=2, op1=1, and CRn<0b1000 */ SR_RANGE_TRAP (sys_reg (2 ,1 ,0 ,0 ,0 ), sys_reg(2, 1, 7, 15, 7), CGT_CPTR_TTA),
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 29/36 KVM: arm64: Subject S1PIE/S1POE registers to HCR_EL2.{TVM,TRVM}
In reply to Marc ZyngierAll the El0/EL1 S1PIE/S1POE system register are caught by the HCR_EL2 TCM and TRVM bits. Reflect this in the coarse grained trap table.
--- arch/arm64/kvm/emulate-nested.c | 4 ++++ 1 file changed, 4 insertions(+)
arch/arm64/kvm/emulate-nested.c +4 -0
@@ -704,6 +704,10 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {SR_TRAP (SYS_MAIR_EL1 ,CGT_HCR_TVM_TRVM ),SR_TRAP (SYS_AMAIR_EL1 ,CGT_HCR_TVM_TRVM ),SR_TRAP (SYS_CONTEXTIDR_EL1 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_PIR_EL1 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_PIRE0_EL1 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_POR_EL0 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_POR_EL1 ,CGT_HCR_TVM_TRVM ),SR_TRAP (SYS_TCR2_EL1 , CGT_HCR_TVM_TRVM_HCRX_TCR2En),SR_TRAP (SYS_DC_ZVA ,CGT_HCR_TDZ ), SR_TRAP(SYS_DC_GVA, CGT_HCR_TDZ),
Signature · 1 line
2.39.2
Re: [PATCH v4 29/36] KVM: arm64: Subject S1PIE/S1POE registers to HCR_EL2.{TVM,TRVM}
Quoted 2 lines · Marc Zyngier
All the El0/EL1 S1PIE/S1POE system register are caught by the
HCR_EL2TCM and TRVM bits. Reflect this in the coarse grained trap table.
typo: TVM
Quoted 23 lines
Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/emulate-nested.c | 4 ++++ 1 file changed, 4 insertions(+)arch/arm64/kvm/emulate-nested.c +4
-0@@ -704,6 +704,10 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {SR_TRAP (SYS_MAIR_EL1 ,CGT_HCR_TVM_TRVM ),SR_TRAP (SYS_AMAIR_EL1 ,CGT_HCR_TVM_TRVM ),SR_TRAP (SYS_CONTEXTIDR_EL1 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_PIR_EL1 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_PIRE0_EL1 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_POR_EL0 ,CGT_HCR_TVM_TRVM ), +SR_TRAP (SYS_POR_EL1 ,CGT_HCR_TVM_TRVM ),SR_TRAP (SYS_TCR2_EL1 , CGT_HCR_TVM_TRVM_HCRX_TCR2En),SR_TRAP (SYS_DC_ZVA ,CGT_HCR_TDZ ), SR_TRAP(SYS_DC_GVA, CGT_HCR_TDZ),Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 30/36 KVM: arm64: Add basic support for POR_EL2
In reply to Marc ZyngierS1POE support implies support for POR_EL2, which we provide by
- adding it to the vcpu_sysreg enum
- advertising it as mapped to its EL1 counterpart in get_el2_to_el1_mapping
- wiring it in the sys_reg_desc table with the correct visibility
- handling POR_EL1 in __vcpu_{read,write}_sys_reg_from_cpu()
--- arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/kvm/sys_regs.c | 9 +++++++++ 2 files changed, 12 insertions(+)
arch/arm64/include/asm/kvm_host.h +3 -0
@@ -473,6 +473,7 @@ enum vcpu_sysreg {TCR_EL2 ,/* Translation Control Register (EL2) */ PIRE0_EL2 ,/* Permission Indirection Register 0 (EL2) */ PIR_EL2 ,/* Permission Indirection Register 1 (EL2) */ +POR_EL2 ,/* Permission Overlay Register 2 (EL2) */ SPSR_EL2 ,/* EL2 saved program status register */ ELR_EL2 ,/* EL2 exception link register */ AFSR0_EL2, /* Auxiliary Fault Status Register 0 (EL2) */
@@ -1043,6 +1044,7 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)case TCR2_EL1 : *val =read_sysreg_s (SYS_TCR2_EL12 );break ;case PIR_EL1 : *val =read_sysreg_s (SYS_PIR_EL12 );break ;case PIRE0_EL1 : *val =read_sysreg_s (SYS_PIRE0_EL12 );break ; +case POR_EL1 : *val =read_sysreg_s (SYS_POR_EL12 );break ;case ESR_EL1 : *val =read_sysreg_s (SYS_ESR_EL12 );break ;case AFSR0_EL1 : *val =read_sysreg_s (SYS_AFSR0_EL12 );break ;case AFSR1_EL1 : *val =read_sysreg_s (SYS_AFSR1_EL12 );break ;
@@ -1092,6 +1094,7 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)case TCR2_EL1 :write_sysreg_s (val,SYS_TCR2_EL12 );break ;case PIR_EL1 :write_sysreg_s (val,SYS_PIR_EL12 );break ;case PIRE0_EL1 :write_sysreg_s (val,SYS_PIRE0_EL12 );break ; +case POR_EL1 :write_sysreg_s (val,SYS_POR_EL12 );break ;case ESR_EL1 :write_sysreg_s (val,SYS_ESR_EL12 );break ;case AFSR0_EL1 :write_sysreg_s (val,SYS_AFSR0_EL12 );break ;case AFSR1_EL1 :write_sysreg_s (val,SYS_AFSR1_EL12 );break ;
arch/arm64/kvm/sys_regs.c +9 -0
@@ -137,6 +137,7 @@ static bool get_el2_to_el1_mapping(unsigned int reg,MAPPED_EL2_SYSREG (TCR2_EL2 ,TCR2_EL1 ,NULL );MAPPED_EL2_SYSREG (PIR_EL2 ,PIR_EL1 ,NULL );MAPPED_EL2_SYSREG (PIRE0_EL2 ,PIRE0_EL1 ,NULL ); +MAPPED_EL2_SYSREG (POR_EL2 ,POR_EL1 ,NULL );MAPPED_EL2_SYSREG (AMAIR_EL2 ,AMAIR_EL1 ,NULL );MAPPED_EL2_SYSREG (ELR_EL2 ,ELR_EL1 ,NULL );MAPPED_EL2_SYSREG (SPSR_EL2 ,SPSR_EL1 ,NULL );
@@ -2322,6 +2323,12 @@ static unsigned int s1poe_visibility(const struct kvm_vcpu *vcpu,return REG_HIDDEN ; } +static unsigned int s1poe_el2_visibility (const struct kvm_vcpu *vcpu, +const struct sys_reg_desc *rd) +{ +return __el2_visibility (vcpu, rd, s1poe_visibility); +} +static unsigned int tcr2_visibility (const struct kvm_vcpu *vcpu,const struct sys_reg_desc *rd) {
@@ -2909,6 +2916,8 @@ static const struct sys_reg_desc sys_reg_descs[] = { s1pie_el2_visibility),EL2_REG_FILTERED (PIR_EL2 , access_rw, reset_val,0 , s1pie_el2_visibility), +EL2_REG_FILTERED (POR_EL2 , access_rw, reset_val,0 , + s1poe_el2_visibility),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 v4 31/36 KVM: arm64: Add save/retore support for POR_EL2
In reply to Marc ZyngierPOR_EL2 needs saving when the guest is VHE, and restoring in any case.
--- arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 6 ++++++ 1 file changed, 6 insertions(+)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +6 -0
@@ -58,6 +58,9 @@ static void __sysreg_save_vel2_state(struct kvm_vcpu *vcpu)__vcpu_sys_reg (vcpu,PIRE0_EL2 ) =read_sysreg_el1 (SYS_PIRE0 );__vcpu_sys_reg (vcpu,PIR_EL2 ) =read_sysreg_el1 (SYS_PIR ); } + +if (ctxt_has_s1poe (&vcpu->arch.ctxt)) +__vcpu_sys_reg (vcpu,POR_EL2 ) =read_sysreg_el1 (SYS_POR ); }/*
@@ -124,6 +127,9 @@ static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)write_sysreg_el1 (__vcpu_sys_reg (vcpu,PIR_EL2 ),SYS_PIR );write_sysreg_el1 (__vcpu_sys_reg (vcpu,PIRE0_EL2 ),SYS_PIRE0 ); } + +if (ctxt_has_s1poe (&vcpu->arch.ctxt)) +write_sysreg_el1 (__vcpu_sys_reg (vcpu,POR_EL2 ),SYS_POR ); }write_sysreg_el1 (__vcpu_sys_reg (vcpu,ESR_EL2 ),SYS_ESR );
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 32/36 KVM: arm64: Add POE save/restore for AT emulation fast-path
In reply to Marc ZyngierJust like the other extensions affecting address translation, we must save/restore POE so that an out-of-context translation context can be restored and used with the AT instructions.
--- arch/arm64/kvm/at.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
arch/arm64/kvm/at.c +14 -0
@@ -440,6 +440,8 @@ struct mmu_config {u64 tcr2;u64 pir;u64 pire0; +u64 por_el0; +u64 por_el1;u64 sctlr;u64 vttbr;u64 vtcr;
@@ -458,6 +460,10 @@ static void __mmu_config_save(struct mmu_config *config) config->pir =read_sysreg_el1 (SYS_PIR ); config->pire0 =read_sysreg_el1 (SYS_PIRE0 ); } +if (system_supports_poe ()) { + config->por_el1 =read_sysreg_el1 (SYS_POR ); + config->por_el0 =read_sysreg_s (SYS_POR_EL0 ); + } } config->sctlr =read_sysreg_el1 (SYS_SCTLR ); config->vttbr =read_sysreg (vttbr_el2);
@@ -485,6 +491,10 @@ static void __mmu_config_restore(struct mmu_config *config)write_sysreg_el1 (config->pir,SYS_PIR );write_sysreg_el1 (config->pire0,SYS_PIRE0 ); } +if (system_supports_poe ()) { +write_sysreg_el1 (config->por_el1,SYS_POR ); +write_sysreg_s (config->por_el0,SYS_POR_EL0 ); + } }write_sysreg_el1 (config->sctlr,SYS_SCTLR );write_sysreg (config->vttbr, vttbr_el2);
@@ -1105,6 +1115,10 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)write_sysreg_el1 (vcpu_read_sys_reg (vcpu,PIR_EL1 ),SYS_PIR );write_sysreg_el1 (vcpu_read_sys_reg (vcpu,PIRE0_EL1 ),SYS_PIRE0 ); } +if (kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1POE ,IMP )) { +write_sysreg_el1 (vcpu_read_sys_reg (vcpu,POR_EL1 ),SYS_POR ); +write_sysreg_s (vcpu_read_sys_reg (vcpu,POR_EL0 ),SYS_POR_EL0 ); + } }write_sysreg_el1 (vcpu_read_sys_reg (vcpu,SCTLR_EL1 ),SYS_SCTLR );__load_stage2 (mmu, mmu->arch);
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 33/36 KVM: arm64: Disable hierarchical permissions when POE is enabled
In reply to Marc ZyngierThe hierarchical permissions must be disabled when POE is enabled in the translation regime used for a given table walk.
We store the two enable bits in the s1_walk_info structure so that they can be retrieved down the line, as they will be useful.
--- arch/arm64/kvm/at.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)
arch/arm64/kvm/at.c +36 -0
@@ -24,6 +24,8 @@ struct s1_walk_info {unsigned int txsz;int sl;bool hpd; +bool e0poe; +bool poe;bool be;bool s2; };
@@ -110,6 +112,34 @@ static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) } } +static void compute_s1poe (struct kvm_vcpu *vcpu,struct s1_walk_info *wi) +{ +u64 val; + +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) { + wi->poe = wi->e0poe = false; +return ; + } + +switch (wi->regime) { +case TR_EL2 : +case TR_EL20 : + val =vcpu_read_sys_reg (vcpu,TCR2_EL2 ); + wi->poe = val &TCR2_EL2_POE ; + wi->e0poe = val &TCR2_EL2_E0POE ; +break ; +case TR_EL10 : +if (__vcpu_sys_reg (vcpu,HCRX_EL2 ) & HCRX_EL2_TCR2En) { + wi->poe = wi->e0poe = false; +return ; + } + + val =__vcpu_sys_reg (vcpu,TCR2_EL1 ); + wi->poe = val & TCR2_EL1x_POE; + wi->e0poe = val & TCR2_EL1x_E0POE; + } +} +static int setup_s1_walk (struct kvm_vcpu *vcpu,u32 op,struct s1_walk_info *wi,struct s1_walk_result *wr,u64 va) {
@@ -206,6 +236,12 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi,/* R_JHSVW */ wi->hpd |=s1pie_enabled (vcpu, wi->regime); +/* Do we have POE? */ +compute_s1poe (vcpu, wi); + +/* R_BVXDG */ + wi->hpd |= (wi->poe || wi->e0poe); +/* Someone was silly enough to encode TG0/TG1 differently */ if (va55) { wi->txsz =FIELD_GET (TCR_T1SZ_MASK , tcr);
Signature · 1 line
2.39.2
Re: [PATCH v4 33/36] KVM: arm64: Disable hierarchical permissions when POE is enabled
Quoted 25 lines · Marc Zyngier
The hierarchical permissions must be disabled when POE is enabled in the translation regime used for a given table walk.
We store the two enable bits in the
s1_walk_infostructure so that they can be retrieved down the line, as they will be useful.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/at.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)arch/arm64/kvm/at.c +2
-0@@ -24,6 +24,8 @@ struct s1_walk_info {unsigned int txsz;int sl;bool hpd; +bool e0poe; +bool poe;bool be;bool s2; };
arch/arm64/kvm/at.c static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 119
Quoted 8 lines
arch/arm64/kvm/at.c +5
-0@@ -110,6 +112,34 @@ static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) } } +static void compute_s1poe (struct kvm_vcpu *vcpu,struct s1_walk_info *wi) +{ +u64 val; + +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) {
nit: kvm_has_s1pie()
arch/arm64/kvm/at.c static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 129
Quoted 10 lines
arch/arm64/kvm/at.c +10
-0+ wi->poe = wi->e0poe = false; +return ; + } + +switch (wi->regime) { +case TR_EL2 : +case TR_EL20 : + val =vcpu_read_sys_reg (vcpu,TCR2_EL2 ); + wi->poe = val &TCR2_EL2_POE ; + wi->e0poe = val &TCR2_EL2_E0POE ;
Hmm... E0POE is always false in the EL2 translation regime. The RES0 mask does the heavy lifting here, but that only works if we force userspace to select an nVHE-only or VHE-only vCPU.
It might make sense to have TR_EL2 force this to false to make it a bit more self-documenting, albeit not a functional issue.
Quoted 13 lines
arch/arm64/kvm/at.c +13
-0+break ; +case TR_EL10 : +if (__vcpu_sys_reg (vcpu,HCRX_EL2 ) & HCRX_EL2_TCR2En) { + wi->poe = wi->e0poe = false; +return ; + } + + val =__vcpu_sys_reg (vcpu,TCR2_EL1 ); + wi->poe = val & TCR2_EL1x_POE; + wi->e0poe = val & TCR2_EL1x_E0POE; + } +} +
Signature · 2 lines
Thanks, Oliver
Marc Zyngier
maintainerRe: [PATCH v4 33/36] KVM: arm64: Disable hierarchical permissions when POE is enabled
Quoted 30 lines · Oliver Upton
Quoted 28 lines · Marc Zyngier
The hierarchical permissions must be disabled when POE is enabled in the translation regime used for a given table walk.
We store the two enable bits in the
s1_walk_infostructure so that they can be retrieved down the line, as they will be useful.Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/at.c | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)arch/arm64/kvm/at.c +2
-0@@ -24,6 +24,8 @@ struct s1_walk_info {unsigned int txsz;int sl;bool hpd; +bool e0poe; +bool poe;bool be;bool s2; };@@ -110,6 +112,34 @@ static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) } }
Quoted 7 lines
arch/arm64/kvm/at.c static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 119
Quoted 5 lines
arch/arm64/kvm/at.c +5
-0+static void compute_s1poe (struct kvm_vcpu *vcpu,struct s1_walk_info *wi) +{ +u64 val; + +if (!kvm_has_feat (vcpu->kvm,ID_AA64MMFR3_EL1 ,S1PIE ,IMP )) {nit:
kvm_has_s1pie()
Nah, that's a gold plated bug, and really should check for S1POE. I guess I'll add a helper (kvm_has_s1poe), and use that throughout.
Quoted 15 lines
arch/arm64/kvm/at.c static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 129
Quoted 11 lines
arch/arm64/kvm/at.c +10
-0+ wi->poe = wi->e0poe = false; +return ; + } + +switch (wi->regime) { +case TR_EL2 : +case TR_EL20 : + val =vcpu_read_sys_reg (vcpu,TCR2_EL2 ); + wi->poe = val &TCR2_EL2_POE ; + wi->e0poe = val &TCR2_EL2_E0POE ;Hmm... E0POE is always false in the EL2 translation regime. The RES0 mask does the heavy lifting here, but that only works if we force userspace to select an nVHE-only or VHE-only vCPU.
Which is the plan of record, but yeah, you can't work that out from this snippet.
Quoted 2 lines
It might make sense to have
TR_EL2force this to false to make it a bit more self-documenting, albeit not a functional issue.
Sure, I'll add a mask if that helps making sense of the whole thing.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Marc Zyngier
maintainerPATCH v4 34/36 KVM: arm64: Make PAN conditions part of the S1 walk context
In reply to Marc ZyngierMove the conditions describing PAN as part of the s1_walk_info structure, in an effort to declutter the permission processing.
--- arch/arm64/kvm/at.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-)
arch/arm64/kvm/at.c +11 -10
@@ -26,6 +26,7 @@ struct s1_walk_info {bool hpd;bool e0poe;bool poe; +bool pan;bool be;bool s2; };
@@ -151,6 +152,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi, wi->regime =compute_translation_regime (vcpu, op); as_el0 = (op ==OP_AT_S1E0R || op ==OP_AT_S1E0W ); + wi->pan = (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) && + (*vcpu_cpsr (vcpu) &PSR_PAN_BIT ); va55 = va &BIT (55 );
@@ -1020,10 +1023,12 @@ static void compute_s1_indirect_permissions(struct kvm_vcpu *vcpu, } } -static void compute_s1_permissions (struct kvm_vcpu *vcpu,u32 op, +static void compute_s1_permissions (struct kvm_vcpu *vcpu,struct s1_walk_info *wi,struct s1_walk_result *wr) { +bool pan; +if (!s1pie_enabled (vcpu, wi->regime))compute_s1_direct_permissions (vcpu, wi, wr);else
@@ -1032,14 +1037,10 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,if (!wi->hpd)compute_s1_hierarchical_permissions (vcpu, wi, wr); -if (op ==OP_AT_S1E1RP || op ==OP_AT_S1E1WP ) { -bool pan; - - pan = *vcpu_cpsr (vcpu) &PSR_PAN_BIT ; - pan &= wr->ur || wr->uw || (pan3_enabled (vcpu, wi->regime) && wr->ux); - wr->pw &= !pan; - wr->pr &= !pan; - } + pan = wi->pan && (wr->ur || wr->uw || + (pan3_enabled (vcpu, wi->regime) && wr->ux)); + wr->pw &= !pan; + wr->pr &= !pan; }static u64 handle_at_slow (struct kvm_vcpu *vcpu,u32 op,u64 vaddr)
@@ -1065,7 +1066,7 @@ static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)if (ret) goto compute_par; -compute_s1_permissions (vcpu, op, &wi, &wr); +compute_s1_permissions (vcpu, &wi, &wr);switch (op) {case OP_AT_S1E1RP :
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 35/36 KVM: arm64: Handle stage-1 permission overlays
In reply to Marc ZyngierWe now have the intrastructure in place to emulate S1POE:
- direct permissions are always overlay-capable
- indirect permissions are overlay-capable if the permissions are in the 0b0xxx range
- the overlays are strictly substractive
--- arch/arm64/kvm/at.c | 53 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+)
arch/arm64/kvm/at.c +53 -0
@@ -40,9 +40,11 @@ struct s1_walk_result {u8 APTable;bool UXNTable;bool PXNTable; +bool uov;bool ur;bool uw;bool ux; +bool pov;bool pr;bool pw;bool px;
@@ -881,6 +883,9 @@ static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu,/* XN maps to UXN */ wr->px = !(wr->desc &PTE_UXN ); } + + wr->pov = wi->poe; + wr->uov = wi->e0poe; } static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
@@ -1016,6 +1021,9 @@ static void compute_s1_indirect_permissions(struct kvm_vcpu *vcpu,else set_unpriv_perms (wr, false, false, false); + wr->pov = wi->poe && !(pp &BIT (3 )); + wr->uov = wi->e0poe && !(up &BIT (3 )); +/* R_VFPJF */ if (wr->px && wr->uw) {set_priv_perms (wr, false, false, false);
@@ -1023,6 +1031,48 @@ static void compute_s1_indirect_permissions(struct kvm_vcpu *vcpu, } } +static void compute_s1_overlay_permissions (struct kvm_vcpu *vcpu, +struct s1_walk_info *wi, +struct s1_walk_result *wr) +{ +u8 idx, pov_perms, uov_perms; + + idx =FIELD_GET (PTE_PO_IDX_MASK , wr->desc); + +switch (wi->regime) { +case TR_EL10 : + pov_perms =perm_idx (vcpu,POR_EL1 , idx); + uov_perms =perm_idx (vcpu,POR_EL0 , idx); +break ; +case TR_EL20 : + pov_perms =perm_idx (vcpu,POR_EL2 , idx); + uov_perms =perm_idx (vcpu,POR_EL0 , idx); +break ; +case TR_EL2 : + pov_perms =perm_idx (vcpu,POR_EL2 , idx); + uov_perms =0 ; +break ; + } + +if (pov_perms & ~POE_RXW ) + pov_perms =POE_NONE ; + +if (wi->poe && wr->pov) { + wr->pr &= pov_perms &POE_R ; + wr->px &= pov_perms &POE_X ; + wr->pw &= pov_perms &POE_W ; + } + +if (uov_perms & ~POE_RXW ) + uov_perms =POE_NONE ; + +if (wi->e0poe && wr->uov) { + wr->ur &= uov_perms &POE_R ; + wr->ux &= uov_perms &POE_X ; + wr->uw &= uov_perms &POE_W ; + } +} +static void compute_s1_permissions (struct kvm_vcpu *vcpu,struct s1_walk_info *wi,struct s1_walk_result *wr)
@@ -1037,6 +1087,9 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,if (!wi->hpd)compute_s1_hierarchical_permissions (vcpu, wi, wr); +if (wi->poe || wi->e0poe) +compute_s1_overlay_permissions (vcpu, wi, wr); + pan = wi->pan && (wr->ur || wr->uw || (pan3_enabled (vcpu, wi->regime) && wr->ux)); wr->pw &= !pan;
Signature · 1 line
2.39.2
Marc Zyngier
maintainerPATCH v4 36/36 KVM: arm64: Handle WXN attribute
In reply to Marc ZyngierUntil now, we didn't really care about WXN as it didn't have an effect on the R/W permissions (only the execution could be droppped), and therefore not of interest for AT.
However, with S1POE, WXN can revoke the Write permission if an overlay is active and that execution is allowed. This *is* relevant to AT.
Add full handling of WXN so that we correctly handle this case.
--- arch/arm64/kvm/at.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
arch/arm64/kvm/at.c +45 -0
@@ -40,10 +40,12 @@ struct s1_walk_result {u8 APTable;bool UXNTable;bool PXNTable; +bool uwxn;bool uov;bool ur;bool uw;bool ux; +bool pwxn;bool pov;bool pr;bool pw;
@@ -847,6 +849,8 @@ static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu,struct s1_walk_info *wi, struct s1_walk_result *wr) { + bool wxn; +/* Non-hierarchical part of AArch64.S1DirectBasePermissions() */ if (wi->regime !=TR_EL2 ) {switch (FIELD_GET (PTE_USER |PTE_RDONLY , wr->desc)) {
@@ -884,6 +888,17 @@ static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu, wr->px = !(wr->desc &PTE_UXN ); } +switch (wi->regime) { +case TR_EL2 : +case TR_EL20 : + wxn = (vcpu_read_sys_reg (vcpu,SCTLR_EL2 ) & SCTLR_ELx_WXN); +break ; +case TR_EL10 : + wxn = (__vcpu_sys_reg (vcpu,SCTLR_EL1 ) & SCTLR_ELx_WXN); +break ; + } + + wr->pwxn = wr->uwxn = wxn; wr->pov = wi->poe; wr->uov = wi->e0poe; }
@@ -935,6 +950,16 @@ static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, (wr)->ux = (x); \ } while (0) +#define set_priv_wxn (wr, v) \ + do { \ + (wr)->pwxn = (v); \ + } while (0) + +#define set_unpriv_wxn (wr, v) \ + do { \ + (wr)->uwxn = (v); \ + } while (0) +/* Similar to AArch64.S1IndirectBasePermissions(), without GCS */ #define set_perms (w, wr, ip) \ do { \
@@ -989,6 +1014,10 @@ static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu, set_ ## w ## _perms((wr), false, false, false); \ break; \ } \ + \ + /* R_HJYGR */ \ + set_ ## w ## _wxn((wr), ((ip) == 0b0110)); \ + \ } while (0) static void compute_s1_indirect_permissions(struct kvm_vcpu *vcpu,
@@ -1090,6 +1119,22 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu,if (wi->poe || wi->e0poe)compute_s1_overlay_permissions (vcpu, wi, wr); +/* R_QXXPC */ +if (wr->pwxn) { +if (!wr->pov && wr->pw) + wr->px = false; +if (wr->pov && wr->px) + wr->pw = false; + } + +/* R_NPBXC */ +if (wr->uwxn) { +if (!wr->uov && wr->uw) + wr->ux = false; +if (wr->uov && wr->ux) + wr->uw = false; + } + pan = wi->pan && (wr->ur || wr->uw || (pan3_enabled (vcpu, wi->regime) && wr->ux)); wr->pw &= !pan;
Signature · 1 line
2.39.2