The current HCR_EL2 description defines ranges of system register encodings in
which accesses should trap for NV. These ranges include encodings which aren't
defined in the reference manual. In order avoid this, let's rather implement a
more fine grained approach excluding those undefined.
v3 -> v4
patch 2:
Add BRBCR_EL2.
patch 3:
Handle BRBCR_EL2.
Add Eric's R-b. Thanks!
v3: https://lore.kernel.org/kvmarm/[email protected]/
v2: https://lore.kernel.org/kvmarm/[email protected]/
v1: https://lore.kernel.org/kvmarm/[email protected]/
Miguel Luis (3):
arm64: Add missing _EL12 encodings
arm64: Add missing _EL2 encodings
arm64/kvm: Fine grain _EL2 system registers list that affect nested
virtualization
arch/arm64/include/asm/sysreg.h | 50 ++++++++++++++++++
arch/arm64/kvm/emulate-nested.c | 89 ++++++++++++++++++++++++++++++---
2 files changed, 133 insertions(+), 6 deletions(-)
--
2.39.2
Some _EL12 encodings are missing. Add them.
Reviewed-by: Eric Auger <[email protected]>
Signed-off-by: Miguel Luis <[email protected]>
---
arch/arm64/include/asm/sysreg.h | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 38296579a4fd..ba5db50effec 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -567,19 +567,30 @@
#define SYS_CNTHCTL_EL2 sys_reg(3, 4, 14, 1, 0)
/* VHE encodings for architectural EL0/1 system registers */
+#define SYS_BRBCR_EL12 sys_reg(2, 5, 9, 0, 0)
#define SYS_SCTLR_EL12 sys_reg(3, 5, 1, 0, 0)
+#define SYS_CPACR_EL12 sys_reg(3, 5, 1, 0, 2)
+#define SYS_SCTLR2_EL12 sys_reg(3, 5, 1, 0, 3)
+#define SYS_ZCR_EL12 sys_reg(3, 5, 1, 2, 0)
+#define SYS_TRFCR_EL12 sys_reg(3, 5, 1, 2, 1)
+#define SYS_SMCR_EL12 sys_reg(3, 5, 1, 2, 6)
#define SYS_TTBR0_EL12 sys_reg(3, 5, 2, 0, 0)
#define SYS_TTBR1_EL12 sys_reg(3, 5, 2, 0, 1)
#define SYS_TCR_EL12 sys_reg(3, 5, 2, 0, 2)
+#define SYS_TCR2_EL12 sys_reg(3, 5, 2, 0, 3)
#define SYS_SPSR_EL12 sys_reg(3, 5, 4, 0, 0)
#define SYS_ELR_EL12 sys_reg(3, 5, 4, 0, 1)
#define SYS_AFSR0_EL12 sys_reg(3, 5, 5, 1, 0)
#define SYS_AFSR1_EL12 sys_reg(3, 5, 5, 1, 1)
#define SYS_ESR_EL12 sys_reg(3, 5, 5, 2, 0)
#define SYS_TFSR_EL12 sys_reg(3, 5, 5, 6, 0)
+#define SYS_FAR_EL12 sys_reg(3, 5, 6, 0, 0)
+#define SYS_PMSCR_EL12 sys_reg(3, 5, 9, 9, 0)
#define SYS_MAIR_EL12 sys_reg(3, 5, 10, 2, 0)
#define SYS_AMAIR_EL12 sys_reg(3, 5, 10, 3, 0)
#define SYS_VBAR_EL12 sys_reg(3, 5, 12, 0, 0)
+#define SYS_CONTEXTIDR_EL12 sys_reg(3, 5, 13, 0, 1)
+#define SYS_SCXTNUM_EL12 sys_reg(3, 5, 13, 0, 7)
#define SYS_CNTKCTL_EL12 sys_reg(3, 5, 14, 1, 0)
#define SYS_CNTP_TVAL_EL02 sys_reg(3, 5, 14, 2, 0)
#define SYS_CNTP_CTL_EL02 sys_reg(3, 5, 14, 2, 1)
--
2.39.2
Implement a fine grained approach in the _EL2 sysreg ranges.
Fixes: d0fc0a2519a6 ("KVM: arm64: nv: Add trap forwarding for HCR_EL2")
Reviewed-by: Eric Auger <[email protected]>
Signed-off-by: Miguel Luis <[email protected]>
---
arch/arm64/kvm/emulate-nested.c | 89 ++++++++++++++++++++++++++++++---
1 file changed, 83 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 9ced1bf0c2b7..3a7d4003fc2b 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -648,15 +648,92 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
SR_TRAP(SYS_APGAKEYLO_EL1, CGT_HCR_APK),
SR_TRAP(SYS_APGAKEYHI_EL1, CGT_HCR_APK),
/* All _EL2 registers */
- SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
- sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
+ SR_TRAP(SYS_BRBCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VPIDR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VMPIDR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_SCTLR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_ACTLR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_SCTLR2_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_HCR_EL2,
+ SYS_HCRX_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_SMPRIMAP_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_SMCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_SDER32_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_TTBR0_EL2,
+ SYS_TCR2_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VTTBR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VTCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VNCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VSTTBR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VSTCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_DACR32_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_HDFGRTR_EL2,
+ SYS_HAFGRTR_EL2, CGT_HCR_NV),
/* Skip the SP_EL1 encoding... */
SR_TRAP(SYS_SPSR_EL2, CGT_HCR_NV),
SR_TRAP(SYS_ELR_EL2, CGT_HCR_NV),
- SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
- sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
- SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
- sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
+ /* SPSR_irq, SPSR_abt, SPSR_und, SPSR_fiq */
+ SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
+ sys_reg(3, 4, 4, 3, 3), CGT_HCR_NV),
+ SR_TRAP(SYS_IFSR32_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_AFSR0_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_AFSR1_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_ESR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VSESR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_FPEXC32_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_TFSR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_FAR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_HPFAR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_PMSCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_MAIR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_AMAIR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_MPAMHCR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_MPAMVPMV_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_MPAM2_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_MPAMVPM0_EL2,
+ SYS_MPAMVPM7_EL2, CGT_HCR_NV),
+ /*
+ * Note that the spec. describes a group of MEC registers
+ * whose access should not trap, therefore skip the following:
+ * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
+ * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
+ * VMECID_P_EL2.
+ */
+ SR_RANGE_TRAP(SYS_VBAR_EL2,
+ SYS_RMR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_VDISR_EL2, CGT_HCR_NV),
+ /* ICH_AP0R<m>_EL2 */
+ SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
+ SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
+ /* ICH_AP1R<m>_EL2 */
+ SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
+ SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_ICC_SRE_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_ICH_HCR_EL2,
+ SYS_ICH_EISR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_ICH_ELRSR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_ICH_VMCR_EL2, CGT_HCR_NV),
+ /* ICH_LR<m>_EL2 */
+ SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
+ SYS_ICH_LR15_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_CONTEXTIDR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_TPIDR_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_SCXTNUM_EL2, CGT_HCR_NV),
+ /* AMEVCNTVOFF0<n>_EL2, AMEVCNTVOFF1<n>_EL2 */
+ SR_RANGE_TRAP(SYS_AMEVCNTVOFF0n_EL2(0),
+ SYS_AMEVCNTVOFF1n_EL2(15), CGT_HCR_NV),
+ /* CNT*_EL2 */
+ SR_TRAP(SYS_CNTVOFF_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_CNTPOFF_EL2, CGT_HCR_NV),
+ SR_TRAP(SYS_CNTHCTL_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_CNTHP_TVAL_EL2,
+ SYS_CNTHP_CVAL_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_CNTHV_TVAL_EL2,
+ SYS_CNTHV_CVAL_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_CNTHVS_TVAL_EL2,
+ SYS_CNTHVS_CVAL_EL2, CGT_HCR_NV),
+ SR_RANGE_TRAP(SYS_CNTHPS_TVAL_EL2,
+ SYS_CNTHPS_CVAL_EL2, CGT_HCR_NV),
/* All _EL02, _EL12 registers */
SR_RANGE_TRAP(sys_reg(3, 5, 0, 0, 0),
sys_reg(3, 5, 10, 15, 7), CGT_HCR_NV),
--
2.39.2
Some _EL2 encodings are missing. Add them.
Signed-off-by: Miguel Luis <[email protected]>
---
arch/arm64/include/asm/sysreg.h | 39 +++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index ba5db50effec..8653fb67a339 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -270,6 +270,8 @@
/* ETM */
#define SYS_TRCOSLAR sys_reg(2, 1, 1, 0, 4)
+#define SYS_BRBCR_EL2 sys_reg(2, 4, 9, 0, 0)
+
#define SYS_MIDR_EL1 sys_reg(3, 0, 0, 0, 0)
#define SYS_MPIDR_EL1 sys_reg(3, 0, 0, 0, 5)
#define SYS_REVIDR_EL1 sys_reg(3, 0, 0, 0, 6)
@@ -484,6 +486,7 @@
#define SYS_SCTLR_EL2 sys_reg(3, 4, 1, 0, 0)
#define SYS_ACTLR_EL2 sys_reg(3, 4, 1, 0, 1)
+#define SYS_SCTLR2_EL2 sys_reg(3, 4, 1, 0, 3)
#define SYS_HCR_EL2 sys_reg(3, 4, 1, 1, 0)
#define SYS_MDCR_EL2 sys_reg(3, 4, 1, 1, 1)
#define SYS_CPTR_EL2 sys_reg(3, 4, 1, 1, 2)
@@ -497,6 +500,10 @@
#define SYS_VTCR_EL2 sys_reg(3, 4, 2, 1, 2)
#define SYS_TRFCR_EL2 sys_reg(3, 4, 1, 2, 1)
+#define SYS_SDER32_EL2 sys_reg(3, 4, 1, 3, 1)
+#define SYS_VNCR_EL2 sys_reg(3, 4, 2, 2, 0)
+#define SYS_VSTTBR_EL2 sys_reg(3, 4, 2, 6, 0)
+#define SYS_VSTCR_EL2 sys_reg(3, 4, 2, 6, 2)
#define SYS_HAFGRTR_EL2 sys_reg(3, 4, 3, 1, 6)
#define SYS_SPSR_EL2 sys_reg(3, 4, 4, 0, 0)
#define SYS_ELR_EL2 sys_reg(3, 4, 4, 0, 1)
@@ -514,6 +521,18 @@
#define SYS_MAIR_EL2 sys_reg(3, 4, 10, 2, 0)
#define SYS_AMAIR_EL2 sys_reg(3, 4, 10, 3, 0)
+#define SYS_MPAMHCR_EL2 sys_reg(3, 4, 10, 4, 0)
+#define SYS_MPAMVPMV_EL2 sys_reg(3, 4, 10, 4, 1)
+#define SYS_MPAM2_EL2 sys_reg(3, 4, 10, 5, 0)
+#define __SYS__MPAMVPMx_EL2(x) sys_reg(3, 4, 10, 6, x)
+#define SYS_MPAMVPM0_EL2 __SYS__MPAMVPMx_EL2(0)
+#define SYS_MPAMVPM1_EL2 __SYS__MPAMVPMx_EL2(1)
+#define SYS_MPAMVPM2_EL2 __SYS__MPAMVPMx_EL2(2)
+#define SYS_MPAMVPM3_EL2 __SYS__MPAMVPMx_EL2(3)
+#define SYS_MPAMVPM4_EL2 __SYS__MPAMVPMx_EL2(4)
+#define SYS_MPAMVPM5_EL2 __SYS__MPAMVPMx_EL2(5)
+#define SYS_MPAMVPM6_EL2 __SYS__MPAMVPMx_EL2(6)
+#define SYS_MPAMVPM7_EL2 __SYS__MPAMVPMx_EL2(7)
#define SYS_VBAR_EL2 sys_reg(3, 4, 12, 0, 0)
#define SYS_RVBAR_EL2 sys_reg(3, 4, 12, 0, 1)
@@ -562,9 +581,29 @@
#define SYS_CONTEXTIDR_EL2 sys_reg(3, 4, 13, 0, 1)
#define SYS_TPIDR_EL2 sys_reg(3, 4, 13, 0, 2)
+#define SYS_SCXTNUM_EL2 sys_reg(3, 4, 13, 0, 7)
+
+#define __AMEV_op2(m) (m & 0x7)
+#define __AMEV_CRm(n, m) (n | ((m & 0x8) >> 3))
+#define __SYS__AMEVCNTVOFF0n_EL2(m) sys_reg(3, 4, 13, __AMEV_CRm(0x8, m), __AMEV_op2(m))
+#define SYS_AMEVCNTVOFF0n_EL2(m) __SYS__AMEVCNTVOFF0n_EL2(m)
+#define __SYS__AMEVCNTVOFF1n_EL2(m) sys_reg(3, 4, 13, __AMEV_CRm(0xA, m), __AMEV_op2(m))
+#define SYS_AMEVCNTVOFF1n_EL2(m) __SYS__AMEVCNTVOFF1n_EL2(m)
#define SYS_CNTVOFF_EL2 sys_reg(3, 4, 14, 0, 3)
#define SYS_CNTHCTL_EL2 sys_reg(3, 4, 14, 1, 0)
+#define SYS_CNTHP_TVAL_EL2 sys_reg(3, 4, 14, 2, 0)
+#define SYS_CNTHP_CTL_EL2 sys_reg(3, 4, 14, 2, 1)
+#define SYS_CNTHP_CVAL_EL2 sys_reg(3, 4, 14, 2, 2)
+#define SYS_CNTHV_TVAL_EL2 sys_reg(3, 4, 14, 3, 0)
+#define SYS_CNTHV_CTL_EL2 sys_reg(3, 4, 14, 3, 1)
+#define SYS_CNTHV_CVAL_EL2 sys_reg(3, 4, 14, 3, 2)
+#define SYS_CNTHVS_TVAL_EL2 sys_reg(3, 4, 14, 4, 0)
+#define SYS_CNTHVS_CTL_EL2 sys_reg(3, 4, 14, 4, 1)
+#define SYS_CNTHVS_CVAL_EL2 sys_reg(3, 4, 14, 4, 2)
+#define SYS_CNTHPS_TVAL_EL2 sys_reg(3, 4, 14, 5, 0)
+#define SYS_CNTHPS_CTL_EL2 sys_reg(3, 4, 14, 5, 1)
+#define SYS_CNTHPS_CVAL_EL2 sys_reg(3, 4, 14, 5, 2)
/* VHE encodings for architectural EL0/1 system registers */
#define SYS_BRBCR_EL12 sys_reg(2, 5, 9, 0, 0)
--
2.39.2
On 10/16/23 13:17, Miguel Luis wrote:
> Some _EL2 encodings are missing. Add them.
>
> Signed-off-by: Miguel Luis <[email protected]>
Reviewed-by: Eric Auger <[email protected]>
Eric
> ---
> arch/arm64/include/asm/sysreg.h | 39 +++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index ba5db50effec..8653fb67a339 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -270,6 +270,8 @@
> /* ETM */
> #define SYS_TRCOSLAR sys_reg(2, 1, 1, 0, 4)
>
> +#define SYS_BRBCR_EL2 sys_reg(2, 4, 9, 0, 0)
> +
> #define SYS_MIDR_EL1 sys_reg(3, 0, 0, 0, 0)
> #define SYS_MPIDR_EL1 sys_reg(3, 0, 0, 0, 5)
> #define SYS_REVIDR_EL1 sys_reg(3, 0, 0, 0, 6)
> @@ -484,6 +486,7 @@
>
> #define SYS_SCTLR_EL2 sys_reg(3, 4, 1, 0, 0)
> #define SYS_ACTLR_EL2 sys_reg(3, 4, 1, 0, 1)
> +#define SYS_SCTLR2_EL2 sys_reg(3, 4, 1, 0, 3)
> #define SYS_HCR_EL2 sys_reg(3, 4, 1, 1, 0)
> #define SYS_MDCR_EL2 sys_reg(3, 4, 1, 1, 1)
> #define SYS_CPTR_EL2 sys_reg(3, 4, 1, 1, 2)
> @@ -497,6 +500,10 @@
> #define SYS_VTCR_EL2 sys_reg(3, 4, 2, 1, 2)
>
> #define SYS_TRFCR_EL2 sys_reg(3, 4, 1, 2, 1)
> +#define SYS_SDER32_EL2 sys_reg(3, 4, 1, 3, 1)
> +#define SYS_VNCR_EL2 sys_reg(3, 4, 2, 2, 0)
> +#define SYS_VSTTBR_EL2 sys_reg(3, 4, 2, 6, 0)
> +#define SYS_VSTCR_EL2 sys_reg(3, 4, 2, 6, 2)
> #define SYS_HAFGRTR_EL2 sys_reg(3, 4, 3, 1, 6)
> #define SYS_SPSR_EL2 sys_reg(3, 4, 4, 0, 0)
> #define SYS_ELR_EL2 sys_reg(3, 4, 4, 0, 1)
> @@ -514,6 +521,18 @@
>
> #define SYS_MAIR_EL2 sys_reg(3, 4, 10, 2, 0)
> #define SYS_AMAIR_EL2 sys_reg(3, 4, 10, 3, 0)
> +#define SYS_MPAMHCR_EL2 sys_reg(3, 4, 10, 4, 0)
> +#define SYS_MPAMVPMV_EL2 sys_reg(3, 4, 10, 4, 1)
> +#define SYS_MPAM2_EL2 sys_reg(3, 4, 10, 5, 0)
> +#define __SYS__MPAMVPMx_EL2(x) sys_reg(3, 4, 10, 6, x)
> +#define SYS_MPAMVPM0_EL2 __SYS__MPAMVPMx_EL2(0)
> +#define SYS_MPAMVPM1_EL2 __SYS__MPAMVPMx_EL2(1)
> +#define SYS_MPAMVPM2_EL2 __SYS__MPAMVPMx_EL2(2)
> +#define SYS_MPAMVPM3_EL2 __SYS__MPAMVPMx_EL2(3)
> +#define SYS_MPAMVPM4_EL2 __SYS__MPAMVPMx_EL2(4)
> +#define SYS_MPAMVPM5_EL2 __SYS__MPAMVPMx_EL2(5)
> +#define SYS_MPAMVPM6_EL2 __SYS__MPAMVPMx_EL2(6)
> +#define SYS_MPAMVPM7_EL2 __SYS__MPAMVPMx_EL2(7)
>
> #define SYS_VBAR_EL2 sys_reg(3, 4, 12, 0, 0)
> #define SYS_RVBAR_EL2 sys_reg(3, 4, 12, 0, 1)
> @@ -562,9 +581,29 @@
>
> #define SYS_CONTEXTIDR_EL2 sys_reg(3, 4, 13, 0, 1)
> #define SYS_TPIDR_EL2 sys_reg(3, 4, 13, 0, 2)
> +#define SYS_SCXTNUM_EL2 sys_reg(3, 4, 13, 0, 7)
> +
> +#define __AMEV_op2(m) (m & 0x7)
> +#define __AMEV_CRm(n, m) (n | ((m & 0x8) >> 3))
> +#define __SYS__AMEVCNTVOFF0n_EL2(m) sys_reg(3, 4, 13, __AMEV_CRm(0x8, m), __AMEV_op2(m))
> +#define SYS_AMEVCNTVOFF0n_EL2(m) __SYS__AMEVCNTVOFF0n_EL2(m)
> +#define __SYS__AMEVCNTVOFF1n_EL2(m) sys_reg(3, 4, 13, __AMEV_CRm(0xA, m), __AMEV_op2(m))
> +#define SYS_AMEVCNTVOFF1n_EL2(m) __SYS__AMEVCNTVOFF1n_EL2(m)
>
> #define SYS_CNTVOFF_EL2 sys_reg(3, 4, 14, 0, 3)
> #define SYS_CNTHCTL_EL2 sys_reg(3, 4, 14, 1, 0)
> +#define SYS_CNTHP_TVAL_EL2 sys_reg(3, 4, 14, 2, 0)
> +#define SYS_CNTHP_CTL_EL2 sys_reg(3, 4, 14, 2, 1)
> +#define SYS_CNTHP_CVAL_EL2 sys_reg(3, 4, 14, 2, 2)
> +#define SYS_CNTHV_TVAL_EL2 sys_reg(3, 4, 14, 3, 0)
> +#define SYS_CNTHV_CTL_EL2 sys_reg(3, 4, 14, 3, 1)
> +#define SYS_CNTHV_CVAL_EL2 sys_reg(3, 4, 14, 3, 2)
> +#define SYS_CNTHVS_TVAL_EL2 sys_reg(3, 4, 14, 4, 0)
> +#define SYS_CNTHVS_CTL_EL2 sys_reg(3, 4, 14, 4, 1)
> +#define SYS_CNTHVS_CVAL_EL2 sys_reg(3, 4, 14, 4, 2)
> +#define SYS_CNTHPS_TVAL_EL2 sys_reg(3, 4, 14, 5, 0)
> +#define SYS_CNTHPS_CTL_EL2 sys_reg(3, 4, 14, 5, 1)
> +#define SYS_CNTHPS_CVAL_EL2 sys_reg(3, 4, 14, 5, 2)
>
> /* VHE encodings for architectural EL0/1 system registers */
> #define SYS_BRBCR_EL12 sys_reg(2, 5, 9, 0, 0)
Hi Eric,
> On 16 Oct 2023, at 11:31, Eric Auger <[email protected]> wrote:
>
>
>
> On 10/16/23 13:17, Miguel Luis wrote:
>> Some _EL2 encodings are missing. Add them.
>>
>> Signed-off-by: Miguel Luis <[email protected]>
> Reviewed-by: Eric Auger <[email protected]>
>
Thank you!
Miguel
> Eric
>> ---
>> arch/arm64/include/asm/sysreg.h | 39 +++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index ba5db50effec..8653fb67a339 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -270,6 +270,8 @@
>> /* ETM */
>> #define SYS_TRCOSLAR sys_reg(2, 1, 1, 0, 4)
>>
>> +#define SYS_BRBCR_EL2 sys_reg(2, 4, 9, 0, 0)
>> +
>> #define SYS_MIDR_EL1 sys_reg(3, 0, 0, 0, 0)
>> #define SYS_MPIDR_EL1 sys_reg(3, 0, 0, 0, 5)
>> #define SYS_REVIDR_EL1 sys_reg(3, 0, 0, 0, 6)
>> @@ -484,6 +486,7 @@
>>
>> #define SYS_SCTLR_EL2 sys_reg(3, 4, 1, 0, 0)
>> #define SYS_ACTLR_EL2 sys_reg(3, 4, 1, 0, 1)
>> +#define SYS_SCTLR2_EL2 sys_reg(3, 4, 1, 0, 3)
>> #define SYS_HCR_EL2 sys_reg(3, 4, 1, 1, 0)
>> #define SYS_MDCR_EL2 sys_reg(3, 4, 1, 1, 1)
>> #define SYS_CPTR_EL2 sys_reg(3, 4, 1, 1, 2)
>> @@ -497,6 +500,10 @@
>> #define SYS_VTCR_EL2 sys_reg(3, 4, 2, 1, 2)
>>
>> #define SYS_TRFCR_EL2 sys_reg(3, 4, 1, 2, 1)
>> +#define SYS_SDER32_EL2 sys_reg(3, 4, 1, 3, 1)
>> +#define SYS_VNCR_EL2 sys_reg(3, 4, 2, 2, 0)
>> +#define SYS_VSTTBR_EL2 sys_reg(3, 4, 2, 6, 0)
>> +#define SYS_VSTCR_EL2 sys_reg(3, 4, 2, 6, 2)
>> #define SYS_HAFGRTR_EL2 sys_reg(3, 4, 3, 1, 6)
>> #define SYS_SPSR_EL2 sys_reg(3, 4, 4, 0, 0)
>> #define SYS_ELR_EL2 sys_reg(3, 4, 4, 0, 1)
>> @@ -514,6 +521,18 @@
>>
>> #define SYS_MAIR_EL2 sys_reg(3, 4, 10, 2, 0)
>> #define SYS_AMAIR_EL2 sys_reg(3, 4, 10, 3, 0)
>> +#define SYS_MPAMHCR_EL2 sys_reg(3, 4, 10, 4, 0)
>> +#define SYS_MPAMVPMV_EL2 sys_reg(3, 4, 10, 4, 1)
>> +#define SYS_MPAM2_EL2 sys_reg(3, 4, 10, 5, 0)
>> +#define __SYS__MPAMVPMx_EL2(x) sys_reg(3, 4, 10, 6, x)
>> +#define SYS_MPAMVPM0_EL2 __SYS__MPAMVPMx_EL2(0)
>> +#define SYS_MPAMVPM1_EL2 __SYS__MPAMVPMx_EL2(1)
>> +#define SYS_MPAMVPM2_EL2 __SYS__MPAMVPMx_EL2(2)
>> +#define SYS_MPAMVPM3_EL2 __SYS__MPAMVPMx_EL2(3)
>> +#define SYS_MPAMVPM4_EL2 __SYS__MPAMVPMx_EL2(4)
>> +#define SYS_MPAMVPM5_EL2 __SYS__MPAMVPMx_EL2(5)
>> +#define SYS_MPAMVPM6_EL2 __SYS__MPAMVPMx_EL2(6)
>> +#define SYS_MPAMVPM7_EL2 __SYS__MPAMVPMx_EL2(7)
>>
>> #define SYS_VBAR_EL2 sys_reg(3, 4, 12, 0, 0)
>> #define SYS_RVBAR_EL2 sys_reg(3, 4, 12, 0, 1)
>> @@ -562,9 +581,29 @@
>>
>> #define SYS_CONTEXTIDR_EL2 sys_reg(3, 4, 13, 0, 1)
>> #define SYS_TPIDR_EL2 sys_reg(3, 4, 13, 0, 2)
>> +#define SYS_SCXTNUM_EL2 sys_reg(3, 4, 13, 0, 7)
>> +
>> +#define __AMEV_op2(m) (m & 0x7)
>> +#define __AMEV_CRm(n, m) (n | ((m & 0x8) >> 3))
>> +#define __SYS__AMEVCNTVOFF0n_EL2(m) sys_reg(3, 4, 13, __AMEV_CRm(0x8, m), __AMEV_op2(m))
>> +#define SYS_AMEVCNTVOFF0n_EL2(m) __SYS__AMEVCNTVOFF0n_EL2(m)
>> +#define __SYS__AMEVCNTVOFF1n_EL2(m) sys_reg(3, 4, 13, __AMEV_CRm(0xA, m), __AMEV_op2(m))
>> +#define SYS_AMEVCNTVOFF1n_EL2(m) __SYS__AMEVCNTVOFF1n_EL2(m)
>>
>> #define SYS_CNTVOFF_EL2 sys_reg(3, 4, 14, 0, 3)
>> #define SYS_CNTHCTL_EL2 sys_reg(3, 4, 14, 1, 0)
>> +#define SYS_CNTHP_TVAL_EL2 sys_reg(3, 4, 14, 2, 0)
>> +#define SYS_CNTHP_CTL_EL2 sys_reg(3, 4, 14, 2, 1)
>> +#define SYS_CNTHP_CVAL_EL2 sys_reg(3, 4, 14, 2, 2)
>> +#define SYS_CNTHV_TVAL_EL2 sys_reg(3, 4, 14, 3, 0)
>> +#define SYS_CNTHV_CTL_EL2 sys_reg(3, 4, 14, 3, 1)
>> +#define SYS_CNTHV_CVAL_EL2 sys_reg(3, 4, 14, 3, 2)
>> +#define SYS_CNTHVS_TVAL_EL2 sys_reg(3, 4, 14, 4, 0)
>> +#define SYS_CNTHVS_CTL_EL2 sys_reg(3, 4, 14, 4, 1)
>> +#define SYS_CNTHVS_CVAL_EL2 sys_reg(3, 4, 14, 4, 2)
>> +#define SYS_CNTHPS_TVAL_EL2 sys_reg(3, 4, 14, 5, 0)
>> +#define SYS_CNTHPS_CTL_EL2 sys_reg(3, 4, 14, 5, 1)
>> +#define SYS_CNTHPS_CVAL_EL2 sys_reg(3, 4, 14, 5, 2)
>>
>> /* VHE encodings for architectural EL0/1 system registers */
>> #define SYS_BRBCR_EL12 sys_reg(2, 5, 9, 0, 0)
>
On Mon, 16 Oct 2023 12:17:41 +0100,
Miguel Luis <[email protected]> wrote:
>
> Some _EL2 encodings are missing. Add them.
>
> Signed-off-by: Miguel Luis <[email protected]>
> ---
> arch/arm64/include/asm/sysreg.h | 39 +++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index ba5db50effec..8653fb67a339 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
[...]
> +#define SYS_SDER32_EL2 sys_reg(3, 4, 1, 3, 1)
[...]
> +#define SYS_VSTTBR_EL2 sys_reg(3, 4, 2, 6, 0)
> +#define SYS_VSTCR_EL2 sys_reg(3, 4, 2, 6, 2)
[...]
> +#define SYS_CNTHVS_TVAL_EL2 sys_reg(3, 4, 14, 4, 0)
> +#define SYS_CNTHVS_CTL_EL2 sys_reg(3, 4, 14, 4, 1)
> +#define SYS_CNTHVS_CVAL_EL2 sys_reg(3, 4, 14, 4, 2)
> +#define SYS_CNTHPS_TVAL_EL2 sys_reg(3, 4, 14, 5, 0)
> +#define SYS_CNTHPS_CTL_EL2 sys_reg(3, 4, 14, 5, 1)
> +#define SYS_CNTHPS_CVAL_EL2 sys_reg(3, 4, 14, 5, 2)
While the secure definitions seem correct, what is the rationale
behind their presence here? They cannot be trapped from non-secure,
and the pseudocode is pretty explicit:
if !IsCurrentSecurityState(SS_Secure) then
UNDEFINED;
Given that, they cannot be trapped, handled or accessed from a KVM
guest, as Linux on arm64 *always* runs non-secure.
M.
--
Without deviation from the norm, progress is not possible.
On Mon, 16 Oct 2023 12:17:42 +0100,
Miguel Luis <[email protected]> wrote:
>
> Implement a fine grained approach in the _EL2 sysreg ranges.
>
> Fixes: d0fc0a2519a6 ("KVM: arm64: nv: Add trap forwarding for HCR_EL2")
> Reviewed-by: Eric Auger <[email protected]>
> Signed-off-by: Miguel Luis <[email protected]>
> ---
> arch/arm64/kvm/emulate-nested.c | 89 ++++++++++++++++++++++++++++++---
> 1 file changed, 83 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 9ced1bf0c2b7..3a7d4003fc2b 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -648,15 +648,92 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
> SR_TRAP(SYS_APGAKEYLO_EL1, CGT_HCR_APK),
> SR_TRAP(SYS_APGAKEYHI_EL1, CGT_HCR_APK),
> /* All _EL2 registers */
> - SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
> - sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
> + SR_TRAP(SYS_BRBCR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VPIDR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VMPIDR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_SCTLR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_ACTLR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_SCTLR2_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_HCR_EL2,
> + SYS_HCRX_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_SMPRIMAP_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_SMCR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_SDER32_EL2, CGT_HCR_NV),
No. This is a *secure* register. How could it be trapped?
> + SR_RANGE_TRAP(SYS_TTBR0_EL2,
> + SYS_TCR2_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VTTBR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VTCR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VNCR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VSTTBR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VSTCR_EL2, CGT_HCR_NV),
Secure registers.
> + SR_TRAP(SYS_DACR32_EL2, CGT_HCR_NV),
This only exists if EL1 is AArch32 capable. Which contradicts the
basic principle that we don't support AArch32 with NV. Why would you
want to forward such a trap?
> + SR_RANGE_TRAP(SYS_HDFGRTR_EL2,
> + SYS_HAFGRTR_EL2, CGT_HCR_NV),
> /* Skip the SP_EL1 encoding... */
> SR_TRAP(SYS_SPSR_EL2, CGT_HCR_NV),
> SR_TRAP(SYS_ELR_EL2, CGT_HCR_NV),
> - SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
> - sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
> - SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
> - sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
> + /* SPSR_irq, SPSR_abt, SPSR_und, SPSR_fiq */
> + SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
> + sys_reg(3, 4, 4, 3, 3), CGT_HCR_NV),
> + SR_TRAP(SYS_IFSR32_EL2, CGT_HCR_NV),
Again: AArch32 related register. The spec is very clear that it UNDEFs
when AArch32 doesn't exist. Even the SPSR_* registers should be
removed and handled as RES0 without reinjection of the trap.
> + SR_TRAP(SYS_AFSR0_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_AFSR1_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_ESR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VSESR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_FPEXC32_EL2, CGT_HCR_NV),
AArch32.
> + SR_TRAP(SYS_TFSR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_FAR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_HPFAR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_PMSCR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_MAIR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_AMAIR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_MPAMHCR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_MPAMVPMV_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_MPAM2_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_MPAMVPM0_EL2,
> + SYS_MPAMVPM7_EL2, CGT_HCR_NV),
> + /*
> + * Note that the spec. describes a group of MEC registers
> + * whose access should not trap, therefore skip the following:
> + * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
> + * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
> + * VMECID_P_EL2.
> + */
> + SR_RANGE_TRAP(SYS_VBAR_EL2,
> + SYS_RMR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_VDISR_EL2, CGT_HCR_NV),
> + /* ICH_AP0R<m>_EL2 */
> + SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
> + SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
> + /* ICH_AP1R<m>_EL2 */
> + SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
> + SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_ICC_SRE_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_ICH_HCR_EL2,
> + SYS_ICH_EISR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_ICH_ELRSR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_ICH_VMCR_EL2, CGT_HCR_NV),
> + /* ICH_LR<m>_EL2 */
> + SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
> + SYS_ICH_LR15_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_CONTEXTIDR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_TPIDR_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_SCXTNUM_EL2, CGT_HCR_NV),
> + /* AMEVCNTVOFF0<n>_EL2, AMEVCNTVOFF1<n>_EL2 */
> + SR_RANGE_TRAP(SYS_AMEVCNTVOFF0n_EL2(0),
> + SYS_AMEVCNTVOFF1n_EL2(15), CGT_HCR_NV),
> + /* CNT*_EL2 */
> + SR_TRAP(SYS_CNTVOFF_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_CNTPOFF_EL2, CGT_HCR_NV),
> + SR_TRAP(SYS_CNTHCTL_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_CNTHP_TVAL_EL2,
> + SYS_CNTHP_CVAL_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_CNTHV_TVAL_EL2,
> + SYS_CNTHV_CVAL_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_CNTHVS_TVAL_EL2,
> + SYS_CNTHVS_CVAL_EL2, CGT_HCR_NV),
> + SR_RANGE_TRAP(SYS_CNTHPS_TVAL_EL2,
> + SYS_CNTHPS_CVAL_EL2, CGT_HCR_NV),
None of these secure registers can be accessed, and they will UNDEF at
EL1.
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
> On 19 Oct 2023, at 11:39, Marc Zyngier <[email protected]> wrote:
>
> On Mon, 16 Oct 2023 12:17:41 +0100,
> Miguel Luis <[email protected]> wrote:
>>
>> Some _EL2 encodings are missing. Add them.
>>
>> Signed-off-by: Miguel Luis <[email protected]>
>> ---
>> arch/arm64/include/asm/sysreg.h | 39 +++++++++++++++++++++++++++++++++
>> 1 file changed, 39 insertions(+)
>>
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index ba5db50effec..8653fb67a339 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>
> [...]
>
>> +#define SYS_SDER32_EL2 sys_reg(3, 4, 1, 3, 1)
>
> [...]
>
>> +#define SYS_VSTTBR_EL2 sys_reg(3, 4, 2, 6, 0)
>> +#define SYS_VSTCR_EL2 sys_reg(3, 4, 2, 6, 2)
>
> [...]
>
>> +#define SYS_CNTHVS_TVAL_EL2 sys_reg(3, 4, 14, 4, 0)
>> +#define SYS_CNTHVS_CTL_EL2 sys_reg(3, 4, 14, 4, 1)
>> +#define SYS_CNTHVS_CVAL_EL2 sys_reg(3, 4, 14, 4, 2)
>> +#define SYS_CNTHPS_TVAL_EL2 sys_reg(3, 4, 14, 5, 0)
>> +#define SYS_CNTHPS_CTL_EL2 sys_reg(3, 4, 14, 5, 1)
>> +#define SYS_CNTHPS_CVAL_EL2 sys_reg(3, 4, 14, 5, 2)
>
> While the secure definitions seem correct, what is the rationale
> behind their presence here? They cannot be trapped from non-secure,
> and the pseudocode is pretty explicit:
>
> if !IsCurrentSecurityState(SS_Secure) then
> UNDEFINED;
>
> Given that, they cannot be trapped, handled or accessed from a KVM
> guest, as Linux on arm64 *always* runs non-secure.
>
Thank you for clarifying.
Those definitions were needed for the refinement on patch 3 which clearly
didn’t considered that statement beforehand.
Yet, should we keep them here so they could be used?
Thank you
Miguel
> M.
>
> --
> Without deviation from the norm, progress is not possible.
On Thu, 19 Oct 2023 14:23:09 +0100,
Miguel Luis <[email protected]> wrote:
>
> Hi Marc,
>
> > On 19 Oct 2023, at 11:39, Marc Zyngier <[email protected]> wrote:
> >
> > On Mon, 16 Oct 2023 12:17:41 +0100,
> > Miguel Luis <[email protected]> wrote:
> >>
> >> Some _EL2 encodings are missing. Add them.
> >>
> >> Signed-off-by: Miguel Luis <[email protected]>
> >> ---
> >> arch/arm64/include/asm/sysreg.h | 39 +++++++++++++++++++++++++++++++++
> >> 1 file changed, 39 insertions(+)
> >>
> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> >> index ba5db50effec..8653fb67a339 100644
> >> --- a/arch/arm64/include/asm/sysreg.h
> >> +++ b/arch/arm64/include/asm/sysreg.h
> >
> > [...]
> >
> >> +#define SYS_SDER32_EL2 sys_reg(3, 4, 1, 3, 1)
> >
> > [...]
> >
> >> +#define SYS_VSTTBR_EL2 sys_reg(3, 4, 2, 6, 0)
> >> +#define SYS_VSTCR_EL2 sys_reg(3, 4, 2, 6, 2)
> >
> > [...]
> >
> >> +#define SYS_CNTHVS_TVAL_EL2 sys_reg(3, 4, 14, 4, 0)
> >> +#define SYS_CNTHVS_CTL_EL2 sys_reg(3, 4, 14, 4, 1)
> >> +#define SYS_CNTHVS_CVAL_EL2 sys_reg(3, 4, 14, 4, 2)
> >> +#define SYS_CNTHPS_TVAL_EL2 sys_reg(3, 4, 14, 5, 0)
> >> +#define SYS_CNTHPS_CTL_EL2 sys_reg(3, 4, 14, 5, 1)
> >> +#define SYS_CNTHPS_CVAL_EL2 sys_reg(3, 4, 14, 5, 2)
> >
> > While the secure definitions seem correct, what is the rationale
> > behind their presence here? They cannot be trapped from non-secure,
> > and the pseudocode is pretty explicit:
> >
> > if !IsCurrentSecurityState(SS_Secure) then
> > UNDEFINED;
> >
> > Given that, they cannot be trapped, handled or accessed from a KVM
> > guest, as Linux on arm64 *always* runs non-secure.
> >
>
> Thank you for clarifying.
>
> Those definitions were needed for the refinement on patch 3 which clearly
> didn’t considered that statement beforehand.
>
> Yet, should we keep them here so they could be used?
But that's the whole point: they *cannot* be used. You can't use a
secure system register in non-secure, and we *always* run in
non-secure, no ifs no buts.
If a guest ever uses one of those, it will get an UNDEF exception
directly, without any SW intervention, because that's just illegal.
KVM will never see it.
As far as Linux is concerned, this is purely dead code.
M.
--
Without deviation from the norm, progress is not possible.
Hi Marc,
> On 19 Oct 2023, at 12:41, Marc Zyngier <[email protected]> wrote:
>
> On Mon, 16 Oct 2023 12:17:42 +0100,
> Miguel Luis <[email protected]> wrote:
>>
>> Implement a fine grained approach in the _EL2 sysreg ranges.
>>
>> Fixes: d0fc0a2519a6 ("KVM: arm64: nv: Add trap forwarding for HCR_EL2")
>> Reviewed-by: Eric Auger <[email protected]>
>> Signed-off-by: Miguel Luis <[email protected]>
>> ---
>> arch/arm64/kvm/emulate-nested.c | 89 ++++++++++++++++++++++++++++++---
>> 1 file changed, 83 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
>> index 9ced1bf0c2b7..3a7d4003fc2b 100644
>> --- a/arch/arm64/kvm/emulate-nested.c
>> +++ b/arch/arm64/kvm/emulate-nested.c
>> @@ -648,15 +648,92 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
>> SR_TRAP(SYS_APGAKEYLO_EL1, CGT_HCR_APK),
>> SR_TRAP(SYS_APGAKEYHI_EL1, CGT_HCR_APK),
>> /* All _EL2 registers */
>> - SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
>> - sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
>> + SR_TRAP(SYS_BRBCR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VPIDR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VMPIDR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_SCTLR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_ACTLR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_SCTLR2_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_HCR_EL2,
>> + SYS_HCRX_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_SMPRIMAP_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_SMCR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_SDER32_EL2, CGT_HCR_NV),
>
> No. This is a *secure* register. How could it be trapped?
Ack. Please see below.
>
>> + SR_RANGE_TRAP(SYS_TTBR0_EL2,
>> + SYS_TCR2_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VTTBR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VTCR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VNCR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VSTTBR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VSTCR_EL2, CGT_HCR_NV),
>
> Secure registers.
Ack.
>
>> + SR_TRAP(SYS_DACR32_EL2, CGT_HCR_NV),
>
> This only exists if EL1 is AArch32 capable. Which contradicts the
> basic principle that we don't support AArch32 with NV. Why would you
> want to forward such a trap?
Ack. Please see below.
>
>> + SR_RANGE_TRAP(SYS_HDFGRTR_EL2,
>> + SYS_HAFGRTR_EL2, CGT_HCR_NV),
>> /* Skip the SP_EL1 encoding... */
>> SR_TRAP(SYS_SPSR_EL2, CGT_HCR_NV),
>> SR_TRAP(SYS_ELR_EL2, CGT_HCR_NV),
>> - SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
>> - sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
>> - SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
>> - sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
>> + /* SPSR_irq, SPSR_abt, SPSR_und, SPSR_fiq */
>> + SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
>> + sys_reg(3, 4, 4, 3, 3), CGT_HCR_NV),
>> + SR_TRAP(SYS_IFSR32_EL2, CGT_HCR_NV),
>
> Again: AArch32 related register. The spec is very clear that it UNDEFs
> when AArch32 doesn't exist. Even the SPSR_* registers should be
> removed and handled as RES0 without reinjection of the trap.
>
OK.
>> + SR_TRAP(SYS_AFSR0_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_AFSR1_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_ESR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VSESR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_FPEXC32_EL2, CGT_HCR_NV),
>
> AArch32.
Got it.
>
>> + SR_TRAP(SYS_TFSR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_FAR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_HPFAR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_PMSCR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_MAIR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_AMAIR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_MPAMHCR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_MPAMVPMV_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_MPAM2_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_MPAMVPM0_EL2,
>> + SYS_MPAMVPM7_EL2, CGT_HCR_NV),
>> + /*
>> + * Note that the spec. describes a group of MEC registers
>> + * whose access should not trap, therefore skip the following:
>> + * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
>> + * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
>> + * VMECID_P_EL2.
>> + */
>> + SR_RANGE_TRAP(SYS_VBAR_EL2,
>> + SYS_RMR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_VDISR_EL2, CGT_HCR_NV),
>> + /* ICH_AP0R<m>_EL2 */
>> + SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
>> + SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
>> + /* ICH_AP1R<m>_EL2 */
>> + SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
>> + SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_ICC_SRE_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_ICH_HCR_EL2,
>> + SYS_ICH_EISR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_ICH_ELRSR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_ICH_VMCR_EL2, CGT_HCR_NV),
>> + /* ICH_LR<m>_EL2 */
>> + SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
>> + SYS_ICH_LR15_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_CONTEXTIDR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_TPIDR_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_SCXTNUM_EL2, CGT_HCR_NV),
>> + /* AMEVCNTVOFF0<n>_EL2, AMEVCNTVOFF1<n>_EL2 */
>> + SR_RANGE_TRAP(SYS_AMEVCNTVOFF0n_EL2(0),
>> + SYS_AMEVCNTVOFF1n_EL2(15), CGT_HCR_NV),
>> + /* CNT*_EL2 */
>> + SR_TRAP(SYS_CNTVOFF_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_CNTPOFF_EL2, CGT_HCR_NV),
>> + SR_TRAP(SYS_CNTHCTL_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_CNTHP_TVAL_EL2,
>> + SYS_CNTHP_CVAL_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_CNTHV_TVAL_EL2,
>> + SYS_CNTHV_CVAL_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_CNTHVS_TVAL_EL2,
>> + SYS_CNTHVS_CVAL_EL2, CGT_HCR_NV),
>> + SR_RANGE_TRAP(SYS_CNTHPS_TVAL_EL2,
>> + SYS_CNTHPS_CVAL_EL2, CGT_HCR_NV),
>
> None of these secure registers can be accessed, and they will UNDEF at
> EL1.
>
In summary, the refinement on this patch started by considering the
spec statement that register accesses using MRS or MSR with a name
ending in _EL2 but the exceptions stated, should trap. Solely by that
statement this patch would, indeed, include registers ending in _EL2 which
should not be contemplated.
NV won’t support Aarch32, so those Aarch32 registers must be removed and I wasn’t
aware that KVM runs always in Non-secure state so secure registers must also
be removed.
Should I spin v5 ?
Thank you
Miguel
> M.
>
> --
> Without deviation from the norm, progress is not possible.
On Thu, 19 Oct 2023 15:46:41 +0100,
Miguel Luis <[email protected]> wrote:
>
> Should I spin v5 ?
Nah, I fixed it locally, and added an extra patch to handle SPSR_*
directly, without any reinjection.
I'm currently running a couple of tests, and post the result shortly.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
> On 16 Oct 2023, at 11:17, Miguel Luis <[email protected]> wrote:
>
> Some _EL12 encodings are missing. Add them.
>
> Reviewed-by: Eric Auger <[email protected]>
> Signed-off-by: Miguel Luis <[email protected]>
> ---
> arch/arm64/include/asm/sysreg.h | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 38296579a4fd..ba5db50effec 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -567,19 +567,30 @@
> #define SYS_CNTHCTL_EL2 sys_reg(3, 4, 14, 1, 0)
>
> /* VHE encodings for architectural EL0/1 system registers */
> +#define SYS_BRBCR_EL12 sys_reg(2, 5, 9, 0, 0)
I think there’s a nit here as BRBCR_EL12 could be placed
before MIDR_EL1 so the file keeps its definitions in order.
Miguel
> #define SYS_SCTLR_EL12 sys_reg(3, 5, 1, 0, 0)
> +#define SYS_CPACR_EL12 sys_reg(3, 5, 1, 0, 2)
> +#define SYS_SCTLR2_EL12 sys_reg(3, 5, 1, 0, 3)
> +#define SYS_ZCR_EL12 sys_reg(3, 5, 1, 2, 0)
> +#define SYS_TRFCR_EL12 sys_reg(3, 5, 1, 2, 1)
> +#define SYS_SMCR_EL12 sys_reg(3, 5, 1, 2, 6)
> #define SYS_TTBR0_EL12 sys_reg(3, 5, 2, 0, 0)
> #define SYS_TTBR1_EL12 sys_reg(3, 5, 2, 0, 1)
> #define SYS_TCR_EL12 sys_reg(3, 5, 2, 0, 2)
> +#define SYS_TCR2_EL12 sys_reg(3, 5, 2, 0, 3)
> #define SYS_SPSR_EL12 sys_reg(3, 5, 4, 0, 0)
> #define SYS_ELR_EL12 sys_reg(3, 5, 4, 0, 1)
> #define SYS_AFSR0_EL12 sys_reg(3, 5, 5, 1, 0)
> #define SYS_AFSR1_EL12 sys_reg(3, 5, 5, 1, 1)
> #define SYS_ESR_EL12 sys_reg(3, 5, 5, 2, 0)
> #define SYS_TFSR_EL12 sys_reg(3, 5, 5, 6, 0)
> +#define SYS_FAR_EL12 sys_reg(3, 5, 6, 0, 0)
> +#define SYS_PMSCR_EL12 sys_reg(3, 5, 9, 9, 0)
> #define SYS_MAIR_EL12 sys_reg(3, 5, 10, 2, 0)
> #define SYS_AMAIR_EL12 sys_reg(3, 5, 10, 3, 0)
> #define SYS_VBAR_EL12 sys_reg(3, 5, 12, 0, 0)
> +#define SYS_CONTEXTIDR_EL12 sys_reg(3, 5, 13, 0, 1)
> +#define SYS_SCXTNUM_EL12 sys_reg(3, 5, 13, 0, 7)
> #define SYS_CNTKCTL_EL12 sys_reg(3, 5, 14, 1, 0)
> #define SYS_CNTP_TVAL_EL02 sys_reg(3, 5, 14, 2, 0)
> #define SYS_CNTP_CTL_EL02 sys_reg(3, 5, 14, 2, 1)
> --
> 2.39.2
>