[PATCH v2 00/12] KVM: arm64: Fixes to fine grain traps and pKVM traps

[PATCH v2 00/12] KVM: arm64: Fixes to fine grain traps and pKVM traps

[Fuad Tabba]

Changes from v1:
- Handle HAFGRTR_EL2 in nested virt
- Update the nested virt FGT tables with the added trap bits (Marc)
- Use the generated value for RES0 (Marc)
- Calculate the value of NMASK based on RES0 and MASK

This patch series has fixes, updates, and code for validating
fine grain trap register masks, as well as some fixes to feature
trapping in pKVM.

New fine grain trap (FGT) bits have been defined in the latest
Arm Architecture System Registers xml specification (2023-09)
[*], so the code is updated to reflect them. Moreover, some of
the already-defined masks overlap with RES0, which this series
fixes.

It also adds FGT register masks that weren't defined earlier,
handling of HAFGRTR_EL2 in nested virt, as well as build time
validation that the bits of the various masks are all accounted
for and without overlap.

Based on 6.7-rc4.

Cheers,
/fuad

[*] https://developer.arm.com/downloads/-/exploration-tools

Fuad Tabba (12):
  KVM: Add missing HCRX_EL2 field definitions
  KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt
  KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt
  KVM: arm64: Add bit masks for HAFGRTR_EL2
  KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt
  KVM: arm64: Update and fix FGT register masks
  KVM: arm64: Add build validation for FGT trap mask values
  KVM: arm64: Use generated FGT RES0 bits instead of specifying them
  KVM: arm64: Generate the HFGWTR-only RES0 bits
  KVM: arm64: Define FGT NMASK bits relative to other fields
  KVM: arm64: Trap external trace for protected VMs
  KVM: arm64: Mark CMOW as allowed for protected VMs

 arch/arm64/include/asm/kvm_arm.h              | 58 +++++++------
 arch/arm64/include/asm/kvm_host.h             |  1 +
 arch/arm64/include/asm/sysreg.h               | 20 +++++
 arch/arm64/kvm/emulate-nested.c               | 63 ++++++++++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h       | 29 +++++++
 .../arm64/kvm/hyp/include/nvhe/fixed_config.h |  4 +-
 arch/arm64/kvm/hyp/nvhe/pkvm.c                |  4 +
 arch/arm64/kvm/sys_regs.c                     |  1 +
 arch/arm64/tools/sysreg                       | 82 ++++++++++++++++++-
 9 files changed, 231 insertions(+), 31 deletions(-)


base-commit: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 00/12] KVM: arm64: Fixes to fine grain traps and pKVM traps

[Fuad Tabba]

Changes from v1:
- Handle HAFGRTR_EL2 in nested virt
- Update the nested virt FGT tables with the added trap bits (Marc)
- Use the generated value for RES0 (Marc)
- Calculate the value of NMASK based on RES0 and MASK

This patch series has fixes, updates, and code for validating
fine grain trap register masks, as well as some fixes to feature
trapping in pKVM.

New fine grain trap (FGT) bits have been defined in the latest
Arm Architecture System Registers xml specification (2023-09)
[*], so the code is updated to reflect them. Moreover, some of
the already-defined masks overlap with RES0, which this series
fixes.

It also adds FGT register masks that weren't defined earlier,
handling of HAFGRTR_EL2 in nested virt, as well as build time
validation that the bits of the various masks are all accounted
for and without overlap.

Based on 6.7-rc4.

Cheers,
/fuad

[*] https://developer.arm.com/downloads/-/exploration-tools

Fuad Tabba (12):
  KVM: Add missing HCRX_EL2 field definitions
  KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt
  KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt
  KVM: arm64: Add bit masks for HAFGRTR_EL2
  KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt
  KVM: arm64: Update and fix FGT register masks
  KVM: arm64: Add build validation for FGT trap mask values
  KVM: arm64: Use generated FGT RES0 bits instead of specifying them
  KVM: arm64: Generate the HFGWTR-only RES0 bits
  KVM: arm64: Define FGT NMASK bits relative to other fields
  KVM: arm64: Trap external trace for protected VMs
  KVM: arm64: Mark CMOW as allowed for protected VMs

 arch/arm64/include/asm/kvm_arm.h              | 58 +++++++------
 arch/arm64/include/asm/kvm_host.h             |  1 +
 arch/arm64/include/asm/sysreg.h               | 20 +++++
 arch/arm64/kvm/emulate-nested.c               | 63 ++++++++++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h       | 29 +++++++
 .../arm64/kvm/hyp/include/nvhe/fixed_config.h |  4 +-
 arch/arm64/kvm/hyp/nvhe/pkvm.c                |  4 +
 arch/arm64/kvm/sys_regs.c                     |  1 +
 arch/arm64/tools/sysreg                       | 82 ++++++++++++++++++-
 9 files changed, 231 insertions(+), 31 deletions(-)


base-commit: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Fuad Tabba]

Add the latest field definitions to HCRX_EL2 from the 2023-09 Arm
Architecture System Registers xml specification [*].

These fields aren't used yet. This is done mainly to get the
correct generated value for the register's RES0 mask.

[*] https://developer.arm.com/documentation/ddi0601/2023-09/AArch64-Registers/HCRX-EL2--Extended-Hypervisor-Configuration-Register

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/tools/sysreg | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 96cbeeab4eec..c6cc8f2396e6 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2300,7 +2300,9 @@ Fields	ZCR_ELx
 EndSysreg
 
 Sysreg	HCRX_EL2	3	4	1	2	2
-Res0	63:23
+Res0	63:25
+Field	24	PACMEn
+Field	23	EnFPM
 Field	22	GCSEn
 Field	21	EnIDCP128
 Field	20	EnSDERR
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Fuad Tabba]

Add the latest field definitions to HCRX_EL2 from the 2023-09 Arm
Architecture System Registers xml specification [*].

These fields aren't used yet. This is done mainly to get the
correct generated value for the register's RES0 mask.

[*] https://developer.arm.com/documentation/ddi0601/2023-09/AArch64-Registers/HCRX-EL2--Extended-Hypervisor-Configuration-Register

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/tools/sysreg | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 96cbeeab4eec..c6cc8f2396e6 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2300,7 +2300,9 @@ Fields	ZCR_ELx
 EndSysreg
 
 Sysreg	HCRX_EL2	3	4	1	2	2
-Res0	63:23
+Res0	63:25
+Field	24	PACMEn
+Field	23	EnFPM
 Field	22	GCSEn
 Field	21	EnIDCP128
 Field	20	EnSDERR
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 02/12] KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt

[Fuad Tabba]

Add the missing nested virt FGT table entries HFGxTR_EL2. Based
on the 2023-09 Arm Architecture System Registers xml
specification [*].

Also adds definitions of some of the missing system registers,
which can be trapped by the FGT bits.

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/sysreg.h | 13 +++++++++++++
 arch/arm64/kvm/emulate-nested.c | 10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 5e65f51c10d2..7b469b3ac1f9 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -297,6 +297,10 @@
 #define SYS_APGAKEYLO_EL1		sys_reg(3, 0, 2, 3, 0)
 #define SYS_APGAKEYHI_EL1		sys_reg(3, 0, 2, 3, 1)
 
+#define SYS_GCSCR_EL1			sys_reg(3, 0, 2, 5, 0)
+#define SYS_GCSPR_EL1			sys_reg(3, 0, 2, 5, 1)
+#define SYS_GCSCRE0_EL1			sys_reg(3, 0, 2, 5, 2)
+
 #define SYS_SPSR_EL1			sys_reg(3, 0, 4, 0, 0)
 #define SYS_ELR_EL1			sys_reg(3, 0, 4, 0, 1)
 
@@ -356,7 +360,11 @@
 #define SYS_PMMIR_EL1			sys_reg(3, 0, 9, 14, 6)
 
 #define SYS_MAIR_EL1			sys_reg(3, 0, 10, 2, 0)
+#define SYS_MAIR2_EL1			sys_reg(3, 0, 10, 2, 1)
+#define SYS_POR_EL1			sys_reg(3, 0, 10, 2, 4)
+#define SYS_S2POR_EL1			sys_reg(3, 0, 10, 2, 5)
 #define SYS_AMAIR_EL1			sys_reg(3, 0, 10, 3, 0)
+#define SYS_AMAIR2_EL1			sys_reg(3, 0, 10, 3, 1)
 
 #define SYS_VBAR_EL1			sys_reg(3, 0, 12, 0, 0)
 #define SYS_DISR_EL1			sys_reg(3, 0, 12, 1, 1)
@@ -390,6 +398,7 @@
 #define SYS_ICC_IGRPEN1_EL1		sys_reg(3, 0, 12, 12, 7)
 
 #define SYS_ACCDATA_EL1			sys_reg(3, 0, 13, 0, 5)
+#define SYS_RCWMASK_EL1			sys_reg(3, 0, 13, 0, 6)
 
 #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1, 0)
 
@@ -398,6 +407,8 @@
 #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
 #define SYS_RNDRRS_EL0			sys_reg(3, 3, 2, 4, 1)
 
+#define SYS_GCSPR_EL0			sys_reg(3, 3, 2, 5, 1)
+
 #define SYS_PMCR_EL0			sys_reg(3, 3, 9, 12, 0)
 #define SYS_PMCNTENSET_EL0		sys_reg(3, 3, 9, 12, 1)
 #define SYS_PMCNTENCLR_EL0		sys_reg(3, 3, 9, 12, 2)
@@ -412,6 +423,8 @@
 #define SYS_PMUSERENR_EL0		sys_reg(3, 3, 9, 14, 0)
 #define SYS_PMOVSSET_EL0		sys_reg(3, 3, 9, 14, 3)
 
+#define SYS_POR_EL0			sys_reg(3, 3, 10, 2, 4)
+
 #define SYS_TPIDR_EL0			sys_reg(3, 3, 13, 0, 2)
 #define SYS_TPIDRRO_EL0			sys_reg(3, 3, 13, 0, 3)
 #define SYS_TPIDR2_EL0			sys_reg(3, 3, 13, 0, 5)
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 06185216a297..8b473a1bbc11 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -1042,10 +1042,20 @@ enum fg_filter_id {
 
 static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = {
 	/* HFGRTR_EL2, HFGWTR_EL2 */
+	SR_FGT(SYS_AMAIR2_EL1,		HFGxTR, nAMAIR2_EL1, 0),
+	SR_FGT(SYS_MAIR2_EL1,		HFGxTR, nMAIR2_EL1, 0),
+	SR_FGT(SYS_S2POR_EL1,		HFGxTR, nS2POR_EL1, 0),
+	SR_FGT(SYS_POR_EL1,		HFGxTR, nPOR_EL1, 0),
+	SR_FGT(SYS_POR_EL0,		HFGxTR, nPOR_EL0, 0),
 	SR_FGT(SYS_PIR_EL1,		HFGxTR, nPIR_EL1, 0),
 	SR_FGT(SYS_PIRE0_EL1,		HFGxTR, nPIRE0_EL1, 0),
+	SR_FGT(SYS_RCWMASK_EL1,		HFGxTR, nRCWMASK_EL1, 0),
 	SR_FGT(SYS_TPIDR2_EL0,		HFGxTR, nTPIDR2_EL0, 0),
 	SR_FGT(SYS_SMPRI_EL1,		HFGxTR, nSMPRI_EL1, 0),
+	SR_FGT(SYS_GCSCR_EL1,		HFGxTR, nGCS_EL1, 0),
+	SR_FGT(SYS_GCSPR_EL1,		HFGxTR, nGCS_EL1, 0),
+	SR_FGT(SYS_GCSCRE0_EL1,		HFGxTR, nGCS_EL0, 0),
+	SR_FGT(SYS_GCSPR_EL0,		HFGxTR, nGCS_EL0, 0),
 	SR_FGT(SYS_ACCDATA_EL1,		HFGxTR, nACCDATA_EL1, 0),
 	SR_FGT(SYS_ERXADDR_EL1,		HFGxTR, ERXADDR_EL1, 1),
 	SR_FGT(SYS_ERXPFGCDN_EL1,	HFGxTR, ERXPFGCDN_EL1, 1),
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 02/12] KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt

[Fuad Tabba]

Add the missing nested virt FGT table entries HFGxTR_EL2. Based
on the 2023-09 Arm Architecture System Registers xml
specification [*].

Also adds definitions of some of the missing system registers,
which can be trapped by the FGT bits.

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/sysreg.h | 13 +++++++++++++
 arch/arm64/kvm/emulate-nested.c | 10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 5e65f51c10d2..7b469b3ac1f9 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -297,6 +297,10 @@
 #define SYS_APGAKEYLO_EL1		sys_reg(3, 0, 2, 3, 0)
 #define SYS_APGAKEYHI_EL1		sys_reg(3, 0, 2, 3, 1)
 
+#define SYS_GCSCR_EL1			sys_reg(3, 0, 2, 5, 0)
+#define SYS_GCSPR_EL1			sys_reg(3, 0, 2, 5, 1)
+#define SYS_GCSCRE0_EL1			sys_reg(3, 0, 2, 5, 2)
+
 #define SYS_SPSR_EL1			sys_reg(3, 0, 4, 0, 0)
 #define SYS_ELR_EL1			sys_reg(3, 0, 4, 0, 1)
 
@@ -356,7 +360,11 @@
 #define SYS_PMMIR_EL1			sys_reg(3, 0, 9, 14, 6)
 
 #define SYS_MAIR_EL1			sys_reg(3, 0, 10, 2, 0)
+#define SYS_MAIR2_EL1			sys_reg(3, 0, 10, 2, 1)
+#define SYS_POR_EL1			sys_reg(3, 0, 10, 2, 4)
+#define SYS_S2POR_EL1			sys_reg(3, 0, 10, 2, 5)
 #define SYS_AMAIR_EL1			sys_reg(3, 0, 10, 3, 0)
+#define SYS_AMAIR2_EL1			sys_reg(3, 0, 10, 3, 1)
 
 #define SYS_VBAR_EL1			sys_reg(3, 0, 12, 0, 0)
 #define SYS_DISR_EL1			sys_reg(3, 0, 12, 1, 1)
@@ -390,6 +398,7 @@
 #define SYS_ICC_IGRPEN1_EL1		sys_reg(3, 0, 12, 12, 7)
 
 #define SYS_ACCDATA_EL1			sys_reg(3, 0, 13, 0, 5)
+#define SYS_RCWMASK_EL1			sys_reg(3, 0, 13, 0, 6)
 
 #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1, 0)
 
@@ -398,6 +407,8 @@
 #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
 #define SYS_RNDRRS_EL0			sys_reg(3, 3, 2, 4, 1)
 
+#define SYS_GCSPR_EL0			sys_reg(3, 3, 2, 5, 1)
+
 #define SYS_PMCR_EL0			sys_reg(3, 3, 9, 12, 0)
 #define SYS_PMCNTENSET_EL0		sys_reg(3, 3, 9, 12, 1)
 #define SYS_PMCNTENCLR_EL0		sys_reg(3, 3, 9, 12, 2)
@@ -412,6 +423,8 @@
 #define SYS_PMUSERENR_EL0		sys_reg(3, 3, 9, 14, 0)
 #define SYS_PMOVSSET_EL0		sys_reg(3, 3, 9, 14, 3)
 
+#define SYS_POR_EL0			sys_reg(3, 3, 10, 2, 4)
+
 #define SYS_TPIDR_EL0			sys_reg(3, 3, 13, 0, 2)
 #define SYS_TPIDRRO_EL0			sys_reg(3, 3, 13, 0, 3)
 #define SYS_TPIDR2_EL0			sys_reg(3, 3, 13, 0, 5)
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 06185216a297..8b473a1bbc11 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -1042,10 +1042,20 @@ enum fg_filter_id {
 
 static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = {
 	/* HFGRTR_EL2, HFGWTR_EL2 */
+	SR_FGT(SYS_AMAIR2_EL1,		HFGxTR, nAMAIR2_EL1, 0),
+	SR_FGT(SYS_MAIR2_EL1,		HFGxTR, nMAIR2_EL1, 0),
+	SR_FGT(SYS_S2POR_EL1,		HFGxTR, nS2POR_EL1, 0),
+	SR_FGT(SYS_POR_EL1,		HFGxTR, nPOR_EL1, 0),
+	SR_FGT(SYS_POR_EL0,		HFGxTR, nPOR_EL0, 0),
 	SR_FGT(SYS_PIR_EL1,		HFGxTR, nPIR_EL1, 0),
 	SR_FGT(SYS_PIRE0_EL1,		HFGxTR, nPIRE0_EL1, 0),
+	SR_FGT(SYS_RCWMASK_EL1,		HFGxTR, nRCWMASK_EL1, 0),
 	SR_FGT(SYS_TPIDR2_EL0,		HFGxTR, nTPIDR2_EL0, 0),
 	SR_FGT(SYS_SMPRI_EL1,		HFGxTR, nSMPRI_EL1, 0),
+	SR_FGT(SYS_GCSCR_EL1,		HFGxTR, nGCS_EL1, 0),
+	SR_FGT(SYS_GCSPR_EL1,		HFGxTR, nGCS_EL1, 0),
+	SR_FGT(SYS_GCSCRE0_EL1,		HFGxTR, nGCS_EL0, 0),
+	SR_FGT(SYS_GCSPR_EL0,		HFGxTR, nGCS_EL0, 0),
 	SR_FGT(SYS_ACCDATA_EL1,		HFGxTR, nACCDATA_EL1, 0),
 	SR_FGT(SYS_ERXADDR_EL1,		HFGxTR, ERXADDR_EL1, 1),
 	SR_FGT(SYS_ERXPFGCDN_EL1,	HFGxTR, ERXPFGCDN_EL1, 1),
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 03/12] KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt

[Fuad Tabba]

Add the missing nested virt FGT table entries HFGITR_EL2. Based
on the 2023-09 Arm Architecture System Registers xml
specification [*]. Add the missing field definitions as well,
both to generate the correct RES0 mask and to be able to toggle
their FGT bits.

Also adds definitions of some of the missing system registers and
instructions, which can be trapped by the FGT bits.

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/sysreg.h | 7 +++++++
 arch/arm64/kvm/emulate-nested.c | 5 +++++
 arch/arm64/tools/sysreg         | 4 +++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 7b469b3ac1f9..5892f9f1b541 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -658,6 +658,7 @@
 #define OP_AT_S1E0W	sys_insn(AT_Op0, 0, AT_CRn, 8, 3)
 #define OP_AT_S1E1RP	sys_insn(AT_Op0, 0, AT_CRn, 9, 0)
 #define OP_AT_S1E1WP	sys_insn(AT_Op0, 0, AT_CRn, 9, 1)
+#define OP_AT_S1E1A	sys_insn(AT_Op0, 0, AT_CRn, 9, 2)
 #define OP_AT_S1E2R	sys_insn(AT_Op0, 4, AT_CRn, 8, 0)
 #define OP_AT_S1E2W	sys_insn(AT_Op0, 4, AT_CRn, 8, 1)
 #define OP_AT_S12E1R	sys_insn(AT_Op0, 4, AT_CRn, 8, 4)
@@ -794,10 +795,16 @@
 #define OP_TLBI_VMALLS12E1NXS		sys_insn(1, 4, 9, 7, 6)
 
 /* Misc instructions */
+#define OP_GCSPUSHX			sys_insn(1, 0, 7, 7, 4)
+#define OP_GCSPOPCX			sys_insn(1, 0, 7, 7, 5)
+#define OP_GCSPOPX			sys_insn(1, 0, 7, 7, 6)
+#define OP_GCSPUSHM			sys_insn(1, 3, 7, 7, 0)
+
 #define OP_BRB_IALL			sys_insn(1, 1, 7, 2, 4)
 #define OP_BRB_INJ			sys_insn(1, 1, 7, 2, 5)
 #define OP_CFP_RCTX			sys_insn(1, 3, 7, 3, 4)
 #define OP_DVP_RCTX			sys_insn(1, 3, 7, 3, 5)
+#define OP_COSP_RCTX			sys_insn(1, 3, 7, 3, 6)
 #define OP_CPP_RCTX			sys_insn(1, 3, 7, 3, 7)
 
 /* Common SCTLR_ELx flags. */
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 8b473a1bbc11..89901550db34 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -1117,6 +1117,11 @@ static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = {
 	SR_FGT(SYS_AFSR1_EL1, 		HFGxTR, AFSR1_EL1, 1),
 	SR_FGT(SYS_AFSR0_EL1, 		HFGxTR, AFSR0_EL1, 1),
 	/* HFGITR_EL2 */
+	SR_FGT(OP_AT_S1E1A, 		HFGITR, ATS1E1A, 1),
+	SR_FGT(OP_COSP_RCTX, 		HFGITR, COSPRCTX, 1),
+	SR_FGT(OP_GCSPUSHX, 		HFGITR, nGCSEPP, 0),
+	SR_FGT(OP_GCSPOPX, 		HFGITR, nGCSEPP, 0),
+	SR_FGT(OP_GCSPUSHM, 		HFGITR, nGCSPUSHM_EL1, 0),
 	SR_FGT(OP_BRB_IALL, 		HFGITR, nBRBIALL, 0),
 	SR_FGT(OP_BRB_INJ, 		HFGITR, nBRBINJ, 0),
 	SR_FGT(SYS_DC_CVAC, 		HFGITR, DCCVAC, 1),
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index c6cc8f2396e6..61cc3bcfc3fa 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2102,7 +2102,9 @@ Fields	HFGxTR_EL2
 EndSysreg
 
 Sysreg HFGITR_EL2	3	4	1	1	6
-Res0	63:61
+Res0	63
+Field	62	ATS1E1A
+Res0	61
 Field	60	COSPRCTX
 Field	59	nGCSEPP
 Field	58	nGCSSTR_EL1
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 03/12] KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt

[Fuad Tabba]

Add the missing nested virt FGT table entries HFGITR_EL2. Based
on the 2023-09 Arm Architecture System Registers xml
specification [*]. Add the missing field definitions as well,
both to generate the correct RES0 mask and to be able to toggle
their FGT bits.

Also adds definitions of some of the missing system registers and
instructions, which can be trapped by the FGT bits.

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/sysreg.h | 7 +++++++
 arch/arm64/kvm/emulate-nested.c | 5 +++++
 arch/arm64/tools/sysreg         | 4 +++-
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 7b469b3ac1f9..5892f9f1b541 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -658,6 +658,7 @@
 #define OP_AT_S1E0W	sys_insn(AT_Op0, 0, AT_CRn, 8, 3)
 #define OP_AT_S1E1RP	sys_insn(AT_Op0, 0, AT_CRn, 9, 0)
 #define OP_AT_S1E1WP	sys_insn(AT_Op0, 0, AT_CRn, 9, 1)
+#define OP_AT_S1E1A	sys_insn(AT_Op0, 0, AT_CRn, 9, 2)
 #define OP_AT_S1E2R	sys_insn(AT_Op0, 4, AT_CRn, 8, 0)
 #define OP_AT_S1E2W	sys_insn(AT_Op0, 4, AT_CRn, 8, 1)
 #define OP_AT_S12E1R	sys_insn(AT_Op0, 4, AT_CRn, 8, 4)
@@ -794,10 +795,16 @@
 #define OP_TLBI_VMALLS12E1NXS		sys_insn(1, 4, 9, 7, 6)
 
 /* Misc instructions */
+#define OP_GCSPUSHX			sys_insn(1, 0, 7, 7, 4)
+#define OP_GCSPOPCX			sys_insn(1, 0, 7, 7, 5)
+#define OP_GCSPOPX			sys_insn(1, 0, 7, 7, 6)
+#define OP_GCSPUSHM			sys_insn(1, 3, 7, 7, 0)
+
 #define OP_BRB_IALL			sys_insn(1, 1, 7, 2, 4)
 #define OP_BRB_INJ			sys_insn(1, 1, 7, 2, 5)
 #define OP_CFP_RCTX			sys_insn(1, 3, 7, 3, 4)
 #define OP_DVP_RCTX			sys_insn(1, 3, 7, 3, 5)
+#define OP_COSP_RCTX			sys_insn(1, 3, 7, 3, 6)
 #define OP_CPP_RCTX			sys_insn(1, 3, 7, 3, 7)
 
 /* Common SCTLR_ELx flags. */
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 8b473a1bbc11..89901550db34 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -1117,6 +1117,11 @@ static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = {
 	SR_FGT(SYS_AFSR1_EL1, 		HFGxTR, AFSR1_EL1, 1),
 	SR_FGT(SYS_AFSR0_EL1, 		HFGxTR, AFSR0_EL1, 1),
 	/* HFGITR_EL2 */
+	SR_FGT(OP_AT_S1E1A, 		HFGITR, ATS1E1A, 1),
+	SR_FGT(OP_COSP_RCTX, 		HFGITR, COSPRCTX, 1),
+	SR_FGT(OP_GCSPUSHX, 		HFGITR, nGCSEPP, 0),
+	SR_FGT(OP_GCSPOPX, 		HFGITR, nGCSEPP, 0),
+	SR_FGT(OP_GCSPUSHM, 		HFGITR, nGCSPUSHM_EL1, 0),
 	SR_FGT(OP_BRB_IALL, 		HFGITR, nBRBIALL, 0),
 	SR_FGT(OP_BRB_INJ, 		HFGITR, nBRBINJ, 0),
 	SR_FGT(SYS_DC_CVAC, 		HFGITR, DCCVAC, 1),
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index c6cc8f2396e6..61cc3bcfc3fa 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2102,7 +2102,9 @@ Fields	HFGxTR_EL2
 EndSysreg
 
 Sysreg HFGITR_EL2	3	4	1	1	6
-Res0	63:61
+Res0	63
+Field	62	ATS1E1A
+Res0	61
 Field	60	COSPRCTX
 Field	59	nGCSEPP
 Field	58	nGCSSTR_EL1
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 04/12] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Fuad Tabba]

To support HAFGRTR_EL2 supported in nested virt in the following
patch, first add its bitmask definitions based on the 2023-09
Arm Architecture System Registers xml specification [*].

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b85f46a73e21..7de0a7062625 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -370,6 +370,10 @@
 #define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
+#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
+#define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
+#define __HAFGRTR_EL2_nMASK	0UL
+
 /* Similar definitions for HCRX_EL2 */
 #define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
 #define __HCRX_EL2_MASK		(0)
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 04/12] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Fuad Tabba]

To support HAFGRTR_EL2 supported in nested virt in the following
patch, first add its bitmask definitions based on the 2023-09
Arm Architecture System Registers xml specification [*].

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b85f46a73e21..7de0a7062625 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -370,6 +370,10 @@
 #define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
+#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
+#define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
+#define __HAFGRTR_EL2_nMASK	0UL
+
 /* Similar definitions for HCRX_EL2 */
 #define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
 #define __HCRX_EL2_MASK		(0)
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Fuad Tabba]

Add the encodings to fine grain trapping fields for HAFGRTR_EL2
and add the associated handling code in nested virt. Based on
the 2023-09 Arm Architecture System Registers xml
specification [*]. Add the missing field definitions as well,
both to generate the correct RES0 mask and to be able to toggle
their FGT bits.

Also add the code for handling FGT trapping, reading of the
register, to nested virt.

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_host.h       |  1 +
 arch/arm64/kvm/emulate-nested.c         | 48 +++++++++++++++++++++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 11 ++++++
 arch/arm64/kvm/sys_regs.c               |  1 +
 arch/arm64/tools/sysreg                 | 43 ++++++++++++++++++++++
 5 files changed, 104 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 824f29f04916..ba14648e2de2 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -443,6 +443,7 @@ enum vcpu_sysreg {
 	HFGITR_EL2,
 	HDFGRTR_EL2,
 	HDFGWTR_EL2,
+	HAFGRTR_EL2,
 	CNTHP_CTL_EL2,
 	CNTHP_CVAL_EL2,
 	CNTHV_CTL_EL2,
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 89901550db34..431fd429932d 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -1012,6 +1012,7 @@ enum fgt_group_id {
 	HDFGRTR_GROUP,
 	HDFGWTR_GROUP,
 	HFGITR_GROUP,
+	HAFGRTR_GROUP,
 
 	/* Must be last */
 	__NR_FGT_GROUP_IDS__
@@ -1689,6 +1690,49 @@ static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = {
 	SR_FGT(SYS_PMCR_EL0,		HDFGWTR, PMCR_EL0, 1),
 	SR_FGT(SYS_PMSWINC_EL0,		HDFGWTR, PMSWINC_EL0, 1),
 	SR_FGT(SYS_OSLAR_EL1,		HDFGWTR, OSLAR_EL1, 1),
+	/*
+	 * HAFGRTR_EL2
+	 */
+	SR_FGT(SYS_AMEVTYPER1_EL0(15),	HAFGRTR, AMEVTYPER115_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(14),	HAFGRTR, AMEVTYPER114_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(13),	HAFGRTR, AMEVTYPER113_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(12),	HAFGRTR, AMEVTYPER112_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(11),	HAFGRTR, AMEVTYPER111_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(10),	HAFGRTR, AMEVTYPER110_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(9),	HAFGRTR, AMEVTYPER19_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(8),	HAFGRTR, AMEVTYPER18_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(7),	HAFGRTR, AMEVTYPER17_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(6),	HAFGRTR, AMEVTYPER16_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(5),	HAFGRTR, AMEVTYPER15_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(4),	HAFGRTR, AMEVTYPER14_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(3),	HAFGRTR, AMEVTYPER13_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(2),	HAFGRTR, AMEVTYPER12_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(1),	HAFGRTR, AMEVTYPER11_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(0),	HAFGRTR, AMEVTYPER10_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(15),	HAFGRTR, AMEVCNTR115_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(14),	HAFGRTR, AMEVCNTR114_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(13),	HAFGRTR, AMEVCNTR113_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(12),	HAFGRTR, AMEVCNTR112_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(11),	HAFGRTR, AMEVCNTR111_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(10),	HAFGRTR, AMEVCNTR110_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(9),	HAFGRTR, AMEVCNTR19_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(8),	HAFGRTR, AMEVCNTR18_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(7),	HAFGRTR, AMEVCNTR17_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(6),	HAFGRTR, AMEVCNTR16_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(5),	HAFGRTR, AMEVCNTR15_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(4),	HAFGRTR, AMEVCNTR14_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(3),	HAFGRTR, AMEVCNTR13_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(2),	HAFGRTR, AMEVCNTR12_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(1),	HAFGRTR, AMEVCNTR11_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(0),	HAFGRTR, AMEVCNTR10_EL0, 1),
+	SR_FGT(SYS_AMCNTENCLR1_EL0,	HAFGRTR, AMCNTEN1, 1),
+	SR_FGT(SYS_AMCNTENSET1_EL0,	HAFGRTR, AMCNTEN1, 1),
+	SR_FGT(SYS_AMCNTENCLR0_EL0,	HAFGRTR, AMCNTEN0, 1),
+	SR_FGT(SYS_AMCNTENSET0_EL0,	HAFGRTR, AMCNTEN0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(3),	HAFGRTR, AMEVCNTR03_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(2),	HAFGRTR, AMEVCNTR02_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(1),	HAFGRTR, AMEVCNTR01_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(0),	HAFGRTR, AMEVCNTR00_EL0, 1),
 };
 
 static union trap_config get_trap_config(u32 sysreg)
@@ -1909,6 +1953,10 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
 			val = sanitised_sys_reg(vcpu, HDFGWTR_EL2);
 		break;
 
+	case HAFGRTR_GROUP:
+		val = sanitised_sys_reg(vcpu, HAFGRTR_EL2);
+		break;
+
 	case HFGITR_GROUP:
 		val = sanitised_sys_reg(vcpu, HFGITR_EL2);
 		switch (tc.fgf) {
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index f99d8af0b9af..76a3d78b13d9 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -153,6 +153,17 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 
 	write_sysreg_s(r_val, SYS_HDFGRTR_EL2);
 	write_sysreg_s(w_val, SYS_HDFGWTR_EL2);
+
+	ctxt_sys_reg(hctxt, HAFGRTR_EL2) = read_sysreg_s(SYS_HAFGRTR_EL2);
+
+	r_clr = r_set = 0;
+	compute_clr_set(vcpu, HAFGRTR_EL2, r_clr, r_set);
+
+	r_val = __HAFGRTR_EL2_nMASK;
+	r_val |= r_set;
+	r_val &= ~r_clr;
+
+	write_sysreg_s(r_val, SYS_HAFGRTR_EL2);
 }
 
 static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 4735e1b37fb3..8bb297a2df38 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2532,6 +2532,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_DACR32_EL2), trap_undef, reset_unknown, DACR32_EL2 },
 	EL2_REG(HDFGRTR_EL2, access_rw, reset_val, 0),
 	EL2_REG(HDFGWTR_EL2, access_rw, reset_val, 0),
+	EL2_REG(HAFGRTR_EL2, access_rw, reset_val, 0),
 	EL2_REG(SPSR_EL2, access_rw, reset_val, 0),
 	EL2_REG(ELR_EL2, access_rw, reset_val, 0),
 	{ SYS_DESC(SYS_SP_EL1), access_sp_el1},
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 61cc3bcfc3fa..e5631f4e62f4 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2297,6 +2297,49 @@ Field	1	DBGBVRn_EL1
 Field	0	DBGBCRn_EL1
 EndSysreg
 
+Sysreg HAFGRTR_EL2	3	4	3	1	6
+Res0	63:50
+Field	49	AMEVTYPER115_EL0
+Field	48	AMEVCNTR115_EL0
+Field	47	AMEVTYPER114_EL0
+Field	46	AMEVCNTR114_EL0
+Field	45	AMEVTYPER113_EL0
+Field	44	AMEVCNTR113_EL0
+Field	43	AMEVTYPER112_EL0
+Field	42	AMEVCNTR112_EL0
+Field	41	AMEVTYPER111_EL0
+Field	40	AMEVCNTR111_EL0
+Field	39	AMEVTYPER110_EL0
+Field	38	AMEVCNTR110_EL0
+Field	37	AMEVTYPER19_EL0
+Field	36	AMEVCNTR19_EL0
+Field	35	AMEVTYPER18_EL0
+Field	34	AMEVCNTR18_EL0
+Field	33	AMEVTYPER17_EL0
+Field	32	AMEVCNTR17_EL0
+Field	31	AMEVTYPER16_EL0
+Field	30	AMEVCNTR16_EL0
+Field	29	AMEVTYPER15_EL0
+Field	28	AMEVCNTR15_EL0
+Field	27	AMEVTYPER14_EL0
+Field	26	AMEVCNTR14_EL0
+Field	25	AMEVTYPER13_EL0
+Field	24	AMEVCNTR13_EL0
+Field	23	AMEVTYPER12_EL0
+Field	22	AMEVCNTR12_EL0
+Field	21	AMEVTYPER11_EL0
+Field	20	AMEVCNTR11_EL0
+Field	19	AMEVTYPER10_EL0
+Field	18	AMEVCNTR10_EL0
+Field	17	AMCNTEN1
+Res0	16:5
+Field	4	AMEVCNTR03_EL0
+Field	3	AMEVCNTR02_EL0
+Field	2	AMEVCNTR01_EL0
+Field	1	AMEVCNTR00_EL0
+Field	0	AMCNTEN0
+EndSysreg
+
 Sysreg	ZCR_EL2	3	4	1	2	0
 Fields	ZCR_ELx
 EndSysreg
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Fuad Tabba]

Add the encodings to fine grain trapping fields for HAFGRTR_EL2
and add the associated handling code in nested virt. Based on
the 2023-09 Arm Architecture System Registers xml
specification [*]. Add the missing field definitions as well,
both to generate the correct RES0 mask and to be able to toggle
their FGT bits.

Also add the code for handling FGT trapping, reading of the
register, to nested virt.

[*] https://developer.arm.com/downloads/-/exploration-tools

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_host.h       |  1 +
 arch/arm64/kvm/emulate-nested.c         | 48 +++++++++++++++++++++++++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 11 ++++++
 arch/arm64/kvm/sys_regs.c               |  1 +
 arch/arm64/tools/sysreg                 | 43 ++++++++++++++++++++++
 5 files changed, 104 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 824f29f04916..ba14648e2de2 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -443,6 +443,7 @@ enum vcpu_sysreg {
 	HFGITR_EL2,
 	HDFGRTR_EL2,
 	HDFGWTR_EL2,
+	HAFGRTR_EL2,
 	CNTHP_CTL_EL2,
 	CNTHP_CVAL_EL2,
 	CNTHV_CTL_EL2,
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 89901550db34..431fd429932d 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -1012,6 +1012,7 @@ enum fgt_group_id {
 	HDFGRTR_GROUP,
 	HDFGWTR_GROUP,
 	HFGITR_GROUP,
+	HAFGRTR_GROUP,
 
 	/* Must be last */
 	__NR_FGT_GROUP_IDS__
@@ -1689,6 +1690,49 @@ static const struct encoding_to_trap_config encoding_to_fgt[] __initconst = {
 	SR_FGT(SYS_PMCR_EL0,		HDFGWTR, PMCR_EL0, 1),
 	SR_FGT(SYS_PMSWINC_EL0,		HDFGWTR, PMSWINC_EL0, 1),
 	SR_FGT(SYS_OSLAR_EL1,		HDFGWTR, OSLAR_EL1, 1),
+	/*
+	 * HAFGRTR_EL2
+	 */
+	SR_FGT(SYS_AMEVTYPER1_EL0(15),	HAFGRTR, AMEVTYPER115_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(14),	HAFGRTR, AMEVTYPER114_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(13),	HAFGRTR, AMEVTYPER113_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(12),	HAFGRTR, AMEVTYPER112_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(11),	HAFGRTR, AMEVTYPER111_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(10),	HAFGRTR, AMEVTYPER110_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(9),	HAFGRTR, AMEVTYPER19_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(8),	HAFGRTR, AMEVTYPER18_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(7),	HAFGRTR, AMEVTYPER17_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(6),	HAFGRTR, AMEVTYPER16_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(5),	HAFGRTR, AMEVTYPER15_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(4),	HAFGRTR, AMEVTYPER14_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(3),	HAFGRTR, AMEVTYPER13_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(2),	HAFGRTR, AMEVTYPER12_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(1),	HAFGRTR, AMEVTYPER11_EL0, 1),
+	SR_FGT(SYS_AMEVTYPER1_EL0(0),	HAFGRTR, AMEVTYPER10_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(15),	HAFGRTR, AMEVCNTR115_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(14),	HAFGRTR, AMEVCNTR114_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(13),	HAFGRTR, AMEVCNTR113_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(12),	HAFGRTR, AMEVCNTR112_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(11),	HAFGRTR, AMEVCNTR111_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(10),	HAFGRTR, AMEVCNTR110_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(9),	HAFGRTR, AMEVCNTR19_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(8),	HAFGRTR, AMEVCNTR18_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(7),	HAFGRTR, AMEVCNTR17_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(6),	HAFGRTR, AMEVCNTR16_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(5),	HAFGRTR, AMEVCNTR15_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(4),	HAFGRTR, AMEVCNTR14_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(3),	HAFGRTR, AMEVCNTR13_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(2),	HAFGRTR, AMEVCNTR12_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(1),	HAFGRTR, AMEVCNTR11_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR1_EL0(0),	HAFGRTR, AMEVCNTR10_EL0, 1),
+	SR_FGT(SYS_AMCNTENCLR1_EL0,	HAFGRTR, AMCNTEN1, 1),
+	SR_FGT(SYS_AMCNTENSET1_EL0,	HAFGRTR, AMCNTEN1, 1),
+	SR_FGT(SYS_AMCNTENCLR0_EL0,	HAFGRTR, AMCNTEN0, 1),
+	SR_FGT(SYS_AMCNTENSET0_EL0,	HAFGRTR, AMCNTEN0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(3),	HAFGRTR, AMEVCNTR03_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(2),	HAFGRTR, AMEVCNTR02_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(1),	HAFGRTR, AMEVCNTR01_EL0, 1),
+	SR_FGT(SYS_AMEVCNTR0_EL0(0),	HAFGRTR, AMEVCNTR00_EL0, 1),
 };
 
 static union trap_config get_trap_config(u32 sysreg)
@@ -1909,6 +1953,10 @@ bool __check_nv_sr_forward(struct kvm_vcpu *vcpu)
 			val = sanitised_sys_reg(vcpu, HDFGWTR_EL2);
 		break;
 
+	case HAFGRTR_GROUP:
+		val = sanitised_sys_reg(vcpu, HAFGRTR_EL2);
+		break;
+
 	case HFGITR_GROUP:
 		val = sanitised_sys_reg(vcpu, HFGITR_EL2);
 		switch (tc.fgf) {
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index f99d8af0b9af..76a3d78b13d9 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -153,6 +153,17 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 
 	write_sysreg_s(r_val, SYS_HDFGRTR_EL2);
 	write_sysreg_s(w_val, SYS_HDFGWTR_EL2);
+
+	ctxt_sys_reg(hctxt, HAFGRTR_EL2) = read_sysreg_s(SYS_HAFGRTR_EL2);
+
+	r_clr = r_set = 0;
+	compute_clr_set(vcpu, HAFGRTR_EL2, r_clr, r_set);
+
+	r_val = __HAFGRTR_EL2_nMASK;
+	r_val |= r_set;
+	r_val &= ~r_clr;
+
+	write_sysreg_s(r_val, SYS_HAFGRTR_EL2);
 }
 
 static inline void __deactivate_traps_hfgxtr(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 4735e1b37fb3..8bb297a2df38 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2532,6 +2532,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_DACR32_EL2), trap_undef, reset_unknown, DACR32_EL2 },
 	EL2_REG(HDFGRTR_EL2, access_rw, reset_val, 0),
 	EL2_REG(HDFGWTR_EL2, access_rw, reset_val, 0),
+	EL2_REG(HAFGRTR_EL2, access_rw, reset_val, 0),
 	EL2_REG(SPSR_EL2, access_rw, reset_val, 0),
 	EL2_REG(ELR_EL2, access_rw, reset_val, 0),
 	{ SYS_DESC(SYS_SP_EL1), access_sp_el1},
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 61cc3bcfc3fa..e5631f4e62f4 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2297,6 +2297,49 @@ Field	1	DBGBVRn_EL1
 Field	0	DBGBCRn_EL1
 EndSysreg
 
+Sysreg HAFGRTR_EL2	3	4	3	1	6
+Res0	63:50
+Field	49	AMEVTYPER115_EL0
+Field	48	AMEVCNTR115_EL0
+Field	47	AMEVTYPER114_EL0
+Field	46	AMEVCNTR114_EL0
+Field	45	AMEVTYPER113_EL0
+Field	44	AMEVCNTR113_EL0
+Field	43	AMEVTYPER112_EL0
+Field	42	AMEVCNTR112_EL0
+Field	41	AMEVTYPER111_EL0
+Field	40	AMEVCNTR111_EL0
+Field	39	AMEVTYPER110_EL0
+Field	38	AMEVCNTR110_EL0
+Field	37	AMEVTYPER19_EL0
+Field	36	AMEVCNTR19_EL0
+Field	35	AMEVTYPER18_EL0
+Field	34	AMEVCNTR18_EL0
+Field	33	AMEVTYPER17_EL0
+Field	32	AMEVCNTR17_EL0
+Field	31	AMEVTYPER16_EL0
+Field	30	AMEVCNTR16_EL0
+Field	29	AMEVTYPER15_EL0
+Field	28	AMEVCNTR15_EL0
+Field	27	AMEVTYPER14_EL0
+Field	26	AMEVCNTR14_EL0
+Field	25	AMEVTYPER13_EL0
+Field	24	AMEVCNTR13_EL0
+Field	23	AMEVTYPER12_EL0
+Field	22	AMEVCNTR12_EL0
+Field	21	AMEVTYPER11_EL0
+Field	20	AMEVCNTR11_EL0
+Field	19	AMEVTYPER10_EL0
+Field	18	AMEVCNTR10_EL0
+Field	17	AMCNTEN1
+Res0	16:5
+Field	4	AMEVCNTR03_EL0
+Field	3	AMEVCNTR02_EL0
+Field	2	AMEVCNTR01_EL0
+Field	1	AMEVCNTR00_EL0
+Field	0	AMCNTEN0
+EndSysreg
+
 Sysreg	ZCR_EL2	3	4	1	2	0
 Fields	ZCR_ELx
 EndSysreg
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

New trap bits have been defined in the 2023-09 Arm Architecture
System Registers xml specification [*]. Moreover, the existing
definitions of some of the mask and the RES0 bits overlap, which
could be wrong, confusing, or both.

Update the bits to represent the latest spec (as of this patch,
2023-09), and ensure that the existing bits are consistent.

Subsequent patches will use the generated RES0 fields instead of
specifying them manually. This patch keeps the manual encoding of
the bits to make it easier to review the series.

[*] https://developer.arm.com/downloads/-/exploration-tools

Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 7de0a7062625..b0dc3249d5cd 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -344,30 +344,39 @@
  * Once we get to a point where the two describe the same thing, we'll
  * merge the definitions. One day.
  */
-#define __HFGRTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51))
+#define __HFGRTR_EL2_RES0	BIT(51)
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGRTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
+#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGWTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51) |	\
-				 BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
-				 GENMASK(26, 25) | BIT(21) | BIT(18) |	\
+#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
+				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
 				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGWTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
+#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
+				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
+				 GENMASK(24, 22) | GENMASK(20, 19) | \
+				 GENMASK(17, 16) | GENMASK(13, 11) | \
+				 GENMASK(8, 3) | GENMASK(1, 0))
+#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGITR_EL2_RES0	GENMASK(63, 57)
-#define __HFGITR_EL2_MASK	GENMASK(54, 0)
-#define __HFGITR_EL2_nMASK	GENMASK(56, 55)
+#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
+#define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
+#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
 
 #define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
 				 GENMASK(21, 20) | BIT(8))
-#define __HDFGRTR_EL2_MASK	~__HDFGRTR_EL2_nMASK
+#define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
+				 GENMASK(41, 40) | GENMASK(37, 22) | \
+				 GENMASK(19, 9) | GENMASK(7, 0))
 #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
 
 #define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
 				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
 				 BIT(22) | BIT(9) | BIT(6))
-#define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
+#define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
+				 GENMASK(46, 44) | GENMASK(42, 41) | \
+				 GENMASK(37, 35) | GENMASK(33, 31) | \
+				 GENMASK(29, 23) | GENMASK(21, 10) | \
+				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
 #define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
@@ -375,9 +384,9 @@
 #define __HAFGRTR_EL2_nMASK	0UL
 
 /* Similar definitions for HCRX_EL2 */
-#define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
-#define __HCRX_EL2_MASK		(0)
-#define __HCRX_EL2_nMASK	(GENMASK(15, 14) | GENMASK(4, 0))
+#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
+#define __HCRX_EL2_MASK		(BIT(6))
+#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

New trap bits have been defined in the 2023-09 Arm Architecture
System Registers xml specification [*]. Moreover, the existing
definitions of some of the mask and the RES0 bits overlap, which
could be wrong, confusing, or both.

Update the bits to represent the latest spec (as of this patch,
2023-09), and ensure that the existing bits are consistent.

Subsequent patches will use the generated RES0 fields instead of
specifying them manually. This patch keeps the manual encoding of
the bits to make it easier to review the series.

[*] https://developer.arm.com/downloads/-/exploration-tools

Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 7de0a7062625..b0dc3249d5cd 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -344,30 +344,39 @@
  * Once we get to a point where the two describe the same thing, we'll
  * merge the definitions. One day.
  */
-#define __HFGRTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51))
+#define __HFGRTR_EL2_RES0	BIT(51)
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGRTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
+#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGWTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51) |	\
-				 BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
-				 GENMASK(26, 25) | BIT(21) | BIT(18) |	\
+#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
+				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
 				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGWTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
+#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
+				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
+				 GENMASK(24, 22) | GENMASK(20, 19) | \
+				 GENMASK(17, 16) | GENMASK(13, 11) | \
+				 GENMASK(8, 3) | GENMASK(1, 0))
+#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGITR_EL2_RES0	GENMASK(63, 57)
-#define __HFGITR_EL2_MASK	GENMASK(54, 0)
-#define __HFGITR_EL2_nMASK	GENMASK(56, 55)
+#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
+#define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
+#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
 
 #define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
 				 GENMASK(21, 20) | BIT(8))
-#define __HDFGRTR_EL2_MASK	~__HDFGRTR_EL2_nMASK
+#define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
+				 GENMASK(41, 40) | GENMASK(37, 22) | \
+				 GENMASK(19, 9) | GENMASK(7, 0))
 #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
 
 #define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
 				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
 				 BIT(22) | BIT(9) | BIT(6))
-#define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
+#define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
+				 GENMASK(46, 44) | GENMASK(42, 41) | \
+				 GENMASK(37, 35) | GENMASK(33, 31) | \
+				 GENMASK(29, 23) | GENMASK(21, 10) | \
+				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
 #define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
@@ -375,9 +384,9 @@
 #define __HAFGRTR_EL2_nMASK	0UL
 
 /* Similar definitions for HCRX_EL2 */
-#define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
-#define __HCRX_EL2_MASK		(0)
-#define __HCRX_EL2_nMASK	(GENMASK(15, 14) | GENMASK(4, 0))
+#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
+#define __HCRX_EL2_MASK		(BIT(6))
+#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 07/12] KVM: arm64: Add build validation for FGT trap mask values

[Fuad Tabba]

These checks help ensure that all the bits are accounted for,
that there hasn't been a transcribing error from the spec nor
from the generated mask values, which will be used in subsequent
patches.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 76a3d78b13d9..91d0f73161bd 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -79,6 +79,16 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
 		clr |= ~hfg & __ ## reg ## _nMASK; 			\
 	} while(0)
 
+/*
+ * Validate the fine grain trap masks.
+ * Check that the masks do not overlap and that all bits are accounted for.
+ */
+#define CHECK_FGT_MASKS(reg)							\
+	do {									\
+		BUILD_BUG_ON((__ ## reg ## _MASK) & (__ ## reg ## _nMASK));	\
+		BUILD_BUG_ON(~((__ ## reg ## _RES0) ^ (__ ## reg ## _MASK) ^	\
+			       (__ ## reg ## _nMASK)));				\
+	} while(0)
 
 static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
@@ -86,6 +96,14 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
 	u64 r_val, w_val;
 
+	CHECK_FGT_MASKS(HFGRTR_EL2);
+	CHECK_FGT_MASKS(HFGWTR_EL2);
+	CHECK_FGT_MASKS(HFGITR_EL2);
+	CHECK_FGT_MASKS(HDFGRTR_EL2);
+	CHECK_FGT_MASKS(HDFGWTR_EL2);
+	CHECK_FGT_MASKS(HAFGRTR_EL2);
+	CHECK_FGT_MASKS(HCRX_EL2);
+
 	if (!cpus_have_final_cap(ARM64_HAS_FGT))
 		return;
 
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 07/12] KVM: arm64: Add build validation for FGT trap mask values

[Fuad Tabba]

These checks help ensure that all the bits are accounted for,
that there hasn't been a transcribing error from the spec nor
from the generated mask values, which will be used in subsequent
patches.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 76a3d78b13d9..91d0f73161bd 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -79,6 +79,16 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
 		clr |= ~hfg & __ ## reg ## _nMASK; 			\
 	} while(0)
 
+/*
+ * Validate the fine grain trap masks.
+ * Check that the masks do not overlap and that all bits are accounted for.
+ */
+#define CHECK_FGT_MASKS(reg)							\
+	do {									\
+		BUILD_BUG_ON((__ ## reg ## _MASK) & (__ ## reg ## _nMASK));	\
+		BUILD_BUG_ON(~((__ ## reg ## _RES0) ^ (__ ## reg ## _MASK) ^	\
+			       (__ ## reg ## _nMASK)));				\
+	} while(0)
 
 static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
@@ -86,6 +96,14 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
 	u64 r_val, w_val;
 
+	CHECK_FGT_MASKS(HFGRTR_EL2);
+	CHECK_FGT_MASKS(HFGWTR_EL2);
+	CHECK_FGT_MASKS(HFGITR_EL2);
+	CHECK_FGT_MASKS(HDFGRTR_EL2);
+	CHECK_FGT_MASKS(HDFGWTR_EL2);
+	CHECK_FGT_MASKS(HAFGRTR_EL2);
+	CHECK_FGT_MASKS(HCRX_EL2);
+
 	if (!cpus_have_final_cap(ARM64_HAS_FGT))
 		return;
 
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 08/12] KVM: arm64: Use generated FGT RES0 bits instead of specifying them

[Fuad Tabba]

Now that all FGT fields are accounted for and represented, use
the generated value instead of manually specifying them.

For __HFGWTR_EL2_RES0, however, there is no generated value. Its
fields are subset of HFGRTR_EL2, with the remaining being RES0.
Therefore, add a mask that represents the HFGRTR_EL2 only bits
and define __HFGWTR_EL2_* using those and the __HFGRTR_EL2_*
fields.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 34 +++++++++++++++-----------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b0dc3249d5cd..5b634e909d1c 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -344,34 +344,32 @@
  * Once we get to a point where the two describe the same thing, we'll
  * merge the definitions. One day.
  */
-#define __HFGRTR_EL2_RES0	BIT(51)
+#define __HFGRTR_EL2_RES0	HFGxTR_EL2_RES0
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
 #define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
-				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
+/*
+ * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any
+ * future additions, define __HFGWTR* macros relative to __HFGRTR* ones.
+ */
+#define __HFGxTR_READ_ONLY_MASK	(BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
+				 GENMASK(26, 25) | BIT(21) | BIT(18) | \
 				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
-				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
-				 GENMASK(24, 22) | GENMASK(20, 19) | \
-				 GENMASK(17, 16) | GENMASK(13, 11) | \
-				 GENMASK(8, 3) | GENMASK(1, 0))
-#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
-
-#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
+#define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | __HFGxTR_READ_ONLY_MASK)
+#define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~__HFGxTR_READ_ONLY_MASK)
+#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~__HFGxTR_READ_ONLY_MASK)
+
+#define __HFGITR_EL2_RES0	HFGITR_EL2_RES0
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
 #define __HFGITR_EL2_nMASK	GENMASK(59, 55)
 
-#define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
-				 GENMASK(21, 20) | BIT(8))
+#define __HDFGRTR_EL2_RES0	HDFGRTR_EL2_RES0
 #define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
 				 GENMASK(41, 40) | GENMASK(37, 22) | \
 				 GENMASK(19, 9) | GENMASK(7, 0))
 #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
 
-#define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
-				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
-				 BIT(22) | BIT(9) | BIT(6))
+#define __HDFGWTR_EL2_RES0	HDFGWTR_EL2_RES0
 #define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
 				 GENMASK(46, 44) | GENMASK(42, 41) | \
 				 GENMASK(37, 35) | GENMASK(33, 31) | \
@@ -379,12 +377,12 @@
 				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
-#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
+#define __HAFGRTR_EL2_RES0	HAFGRTR_EL2_RES0
 #define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
 #define __HAFGRTR_EL2_nMASK	0UL
 
 /* Similar definitions for HCRX_EL2 */
-#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
+#define __HCRX_EL2_RES0         HCRX_EL2_RES0
 #define __HCRX_EL2_MASK		(BIT(6))
 #define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
 
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 08/12] KVM: arm64: Use generated FGT RES0 bits instead of specifying them

[Fuad Tabba]

Now that all FGT fields are accounted for and represented, use
the generated value instead of manually specifying them.

For __HFGWTR_EL2_RES0, however, there is no generated value. Its
fields are subset of HFGRTR_EL2, with the remaining being RES0.
Therefore, add a mask that represents the HFGRTR_EL2 only bits
and define __HFGWTR_EL2_* using those and the __HFGRTR_EL2_*
fields.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 34 +++++++++++++++-----------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b0dc3249d5cd..5b634e909d1c 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -344,34 +344,32 @@
  * Once we get to a point where the two describe the same thing, we'll
  * merge the definitions. One day.
  */
-#define __HFGRTR_EL2_RES0	BIT(51)
+#define __HFGRTR_EL2_RES0	HFGxTR_EL2_RES0
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
 #define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
-				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
+/*
+ * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any
+ * future additions, define __HFGWTR* macros relative to __HFGRTR* ones.
+ */
+#define __HFGxTR_READ_ONLY_MASK	(BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
+				 GENMASK(26, 25) | BIT(21) | BIT(18) | \
 				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
-				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
-				 GENMASK(24, 22) | GENMASK(20, 19) | \
-				 GENMASK(17, 16) | GENMASK(13, 11) | \
-				 GENMASK(8, 3) | GENMASK(1, 0))
-#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
-
-#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
+#define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | __HFGxTR_READ_ONLY_MASK)
+#define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~__HFGxTR_READ_ONLY_MASK)
+#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~__HFGxTR_READ_ONLY_MASK)
+
+#define __HFGITR_EL2_RES0	HFGITR_EL2_RES0
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
 #define __HFGITR_EL2_nMASK	GENMASK(59, 55)
 
-#define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
-				 GENMASK(21, 20) | BIT(8))
+#define __HDFGRTR_EL2_RES0	HDFGRTR_EL2_RES0
 #define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
 				 GENMASK(41, 40) | GENMASK(37, 22) | \
 				 GENMASK(19, 9) | GENMASK(7, 0))
 #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
 
-#define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
-				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
-				 BIT(22) | BIT(9) | BIT(6))
+#define __HDFGWTR_EL2_RES0	HDFGWTR_EL2_RES0
 #define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
 				 GENMASK(46, 44) | GENMASK(42, 41) | \
 				 GENMASK(37, 35) | GENMASK(33, 31) | \
@@ -379,12 +377,12 @@
 				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
-#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
+#define __HAFGRTR_EL2_RES0	HAFGRTR_EL2_RES0
 #define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
 #define __HAFGRTR_EL2_nMASK	0UL
 
 /* Similar definitions for HCRX_EL2 */
-#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
+#define __HCRX_EL2_RES0         HCRX_EL2_RES0
 #define __HCRX_EL2_MASK		(BIT(6))
 #define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
 
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 09/12] KVM: arm64: Generate the HFGWTR-only RES0 bits

[Fuad Tabba]

Generate the HFGWTR-only RES0 bits in sysreg. This is done to
consolidate all the bit definitions in one place.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 13 +++----------
 arch/arm64/tools/sysreg          | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 5b634e909d1c..02442bc90717 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -348,16 +348,9 @@
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
 #define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-/*
- * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any
- * future additions, define __HFGWTR* macros relative to __HFGRTR* ones.
- */
-#define __HFGxTR_READ_ONLY_MASK	(BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
-				 GENMASK(26, 25) | BIT(21) | BIT(18) | \
-				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | __HFGxTR_READ_ONLY_MASK)
-#define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~__HFGxTR_READ_ONLY_MASK)
-#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~__HFGxTR_READ_ONLY_MASK)
+#define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | HFGWTR_ONLY_EL2_RES0)
+#define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~HFGWTR_ONLY_EL2_RES0)
+#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~HFGWTR_ONLY_EL2_RES0)
 
 #define __HFGITR_EL2_RES0	HFGITR_EL2_RES0
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index e5631f4e62f4..85f8b385fed2 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2093,6 +2093,32 @@ Field	1	AFSR1_EL1
 Field	0	AFSR0_EL1
 EndSysregFields
 
+# HFGWTR_EL2 bits are a subset of those for HFGRTR_EL2.
+# Define RES0 only present in HFGWTR_EL2.
+SysregFields	HFGWTR_ONLY_EL2
+Unkn	63:47
+Res0	46
+Unkn	45:43
+Res0	42
+Unkn	41
+Res0	40
+Unkn	39:29
+Res0	28
+Unkn	27
+Res0	26:25
+Unkn	24:22
+Res0	21
+Unkn	20:19
+Res0	18
+Unkn	17:16
+Res0	15:14
+Unkn	13:11
+Res0	10:9
+Unkn	8:3
+Res0	2
+Unkn	1:0
+EndSysregFields
+
 Sysreg HFGRTR_EL2	3	4	1	1	4
 Fields	HFGxTR_EL2
 EndSysreg
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 09/12] KVM: arm64: Generate the HFGWTR-only RES0 bits

[Fuad Tabba]

Generate the HFGWTR-only RES0 bits in sysreg. This is done to
consolidate all the bit definitions in one place.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 13 +++----------
 arch/arm64/tools/sysreg          | 26 ++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 5b634e909d1c..02442bc90717 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -348,16 +348,9 @@
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
 #define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-/*
- * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any
- * future additions, define __HFGWTR* macros relative to __HFGRTR* ones.
- */
-#define __HFGxTR_READ_ONLY_MASK	(BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
-				 GENMASK(26, 25) | BIT(21) | BIT(18) | \
-				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | __HFGxTR_READ_ONLY_MASK)
-#define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~__HFGxTR_READ_ONLY_MASK)
-#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~__HFGxTR_READ_ONLY_MASK)
+#define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | HFGWTR_ONLY_EL2_RES0)
+#define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~HFGWTR_ONLY_EL2_RES0)
+#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~HFGWTR_ONLY_EL2_RES0)
 
 #define __HFGITR_EL2_RES0	HFGITR_EL2_RES0
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index e5631f4e62f4..85f8b385fed2 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -2093,6 +2093,32 @@ Field	1	AFSR1_EL1
 Field	0	AFSR0_EL1
 EndSysregFields
 
+# HFGWTR_EL2 bits are a subset of those for HFGRTR_EL2.
+# Define RES0 only present in HFGWTR_EL2.
+SysregFields	HFGWTR_ONLY_EL2
+Unkn	63:47
+Res0	46
+Unkn	45:43
+Res0	42
+Unkn	41
+Res0	40
+Unkn	39:29
+Res0	28
+Unkn	27
+Res0	26:25
+Unkn	24:22
+Res0	21
+Unkn	20:19
+Res0	18
+Unkn	17:16
+Res0	15:14
+Unkn	13:11
+Res0	10:9
+Unkn	8:3
+Res0	2
+Unkn	1:0
+EndSysregFields
+
 Sysreg HFGRTR_EL2	3	4	1	1	4
 Fields	HFGxTR_EL2
 EndSysreg
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 10/12] KVM: arm64: Define FGT NMASK bits relative to other fields

[Fuad Tabba]

Now that RES0 and MASK have full coverage, no speed to manually
encode NMASK. Calculate it relative to the other fields.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 02442bc90717..cb01dd18f763 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -346,21 +346,21 @@
  */
 #define __HFGRTR_EL2_RES0	HFGxTR_EL2_RES0
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
+#define __HFGRTR_EL2_nMASK	~(__HFGRTR_EL2_RES0 | __HFGRTR_EL2_MASK)
 
 #define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | HFGWTR_ONLY_EL2_RES0)
 #define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~HFGWTR_ONLY_EL2_RES0)
-#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~HFGWTR_ONLY_EL2_RES0)
+#define __HFGWTR_EL2_nMASK	~(__HFGWTR_EL2_RES0 | __HFGWTR_EL2_MASK)
 
 #define __HFGITR_EL2_RES0	HFGITR_EL2_RES0
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
-#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
+#define __HFGITR_EL2_nMASK	~(__HFGITR_EL2_RES0 | __HFGITR_EL2_MASK)
 
 #define __HDFGRTR_EL2_RES0	HDFGRTR_EL2_RES0
 #define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
 				 GENMASK(41, 40) | GENMASK(37, 22) | \
 				 GENMASK(19, 9) | GENMASK(7, 0))
-#define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
+#define __HDFGRTR_EL2_nMASK	~(__HDFGRTR_EL2_RES0 | __HDFGRTR_EL2_MASK)
 
 #define __HDFGWTR_EL2_RES0	HDFGWTR_EL2_RES0
 #define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
@@ -368,16 +368,16 @@
 				 GENMASK(37, 35) | GENMASK(33, 31) | \
 				 GENMASK(29, 23) | GENMASK(21, 10) | \
 				 GENMASK(8, 7) | GENMASK(5, 0))
-#define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
+#define __HDFGWTR_EL2_nMASK	~(__HDFGWTR_EL2_RES0 | __HDFGWTR_EL2_MASK)
 
 #define __HAFGRTR_EL2_RES0	HAFGRTR_EL2_RES0
 #define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
-#define __HAFGRTR_EL2_nMASK	0UL
+#define __HAFGRTR_EL2_nMASK	~(__HAFGRTR_EL2_RES0 | __HAFGRTR_EL2_MASK)
 
 /* Similar definitions for HCRX_EL2 */
 #define __HCRX_EL2_RES0         HCRX_EL2_RES0
 #define __HCRX_EL2_MASK		(BIT(6))
-#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
+#define __HCRX_EL2_nMASK	~(__HCRX_EL2_RES0 | __HCRX_EL2_MASK)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 10/12] KVM: arm64: Define FGT NMASK bits relative to other fields

[Fuad Tabba]

Now that RES0 and MASK have full coverage, no speed to manually
encode NMASK. Calculate it relative to the other fields.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 02442bc90717..cb01dd18f763 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -346,21 +346,21 @@
  */
 #define __HFGRTR_EL2_RES0	HFGxTR_EL2_RES0
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
+#define __HFGRTR_EL2_nMASK	~(__HFGRTR_EL2_RES0 | __HFGRTR_EL2_MASK)
 
 #define __HFGWTR_EL2_RES0	(__HFGRTR_EL2_RES0 | HFGWTR_ONLY_EL2_RES0)
 #define __HFGWTR_EL2_MASK	(__HFGRTR_EL2_MASK & ~HFGWTR_ONLY_EL2_RES0)
-#define __HFGWTR_EL2_nMASK	(__HFGRTR_EL2_nMASK & ~HFGWTR_ONLY_EL2_RES0)
+#define __HFGWTR_EL2_nMASK	~(__HFGWTR_EL2_RES0 | __HFGWTR_EL2_MASK)
 
 #define __HFGITR_EL2_RES0	HFGITR_EL2_RES0
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
-#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
+#define __HFGITR_EL2_nMASK	~(__HFGITR_EL2_RES0 | __HFGITR_EL2_MASK)
 
 #define __HDFGRTR_EL2_RES0	HDFGRTR_EL2_RES0
 #define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
 				 GENMASK(41, 40) | GENMASK(37, 22) | \
 				 GENMASK(19, 9) | GENMASK(7, 0))
-#define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
+#define __HDFGRTR_EL2_nMASK	~(__HDFGRTR_EL2_RES0 | __HDFGRTR_EL2_MASK)
 
 #define __HDFGWTR_EL2_RES0	HDFGWTR_EL2_RES0
 #define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
@@ -368,16 +368,16 @@
 				 GENMASK(37, 35) | GENMASK(33, 31) | \
 				 GENMASK(29, 23) | GENMASK(21, 10) | \
 				 GENMASK(8, 7) | GENMASK(5, 0))
-#define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
+#define __HDFGWTR_EL2_nMASK	~(__HDFGWTR_EL2_RES0 | __HDFGWTR_EL2_MASK)
 
 #define __HAFGRTR_EL2_RES0	HAFGRTR_EL2_RES0
 #define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
-#define __HAFGRTR_EL2_nMASK	0UL
+#define __HAFGRTR_EL2_nMASK	~(__HAFGRTR_EL2_RES0 | __HAFGRTR_EL2_MASK)
 
 /* Similar definitions for HCRX_EL2 */
 #define __HCRX_EL2_RES0         HCRX_EL2_RES0
 #define __HCRX_EL2_MASK		(BIT(6))
-#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
+#define __HCRX_EL2_nMASK	~(__HCRX_EL2_RES0 | __HCRX_EL2_MASK)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 11/12] KVM: arm64: Trap external trace for protected VMs

[Fuad Tabba]

pKVM does not support external trace for protected VMs. Trap
external trace, and add the ExtTrcBuff to make it possible to
check for the feature.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/pkvm.c | 4 ++++
 arch/arm64/tools/sysreg        | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 9d23a51d7f75..84b5c3f387d8 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -136,6 +136,10 @@ static void pvm_init_traps_aa64dfr0(struct kvm_vcpu *vcpu)
 			cptr_set |= CPTR_EL2_TTA;
 	}
 
+	/* Trap External Trace */
+	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_ExtTrcBuff), feature_ids))
+		mdcr_clear |= MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT;
+
 	vcpu->arch.mdcr_el2 |= mdcr_set;
 	vcpu->arch.mdcr_el2 &= ~mdcr_clear;
 	vcpu->arch.cptr_el2 |= cptr_set;
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 85f8b385fed2..be857797cc00 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -1115,7 +1115,10 @@ Enum	63:60	HPMN0
 	0b0000	UNPREDICTABLE
 	0b0001	DEF
 EndEnum
-Res0	59:56
+UnsignedEnum	59:56	ExtTrcBuff
+	0b0000	NI
+	0b0001	IMP
+EndEnum
 UnsignedEnum	55:52	BRBE
 	0b0000	NI
 	0b0001	IMP
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 11/12] KVM: arm64: Trap external trace for protected VMs

[Fuad Tabba]

pKVM does not support external trace for protected VMs. Trap
external trace, and add the ExtTrcBuff to make it possible to
check for the feature.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/pkvm.c | 4 ++++
 arch/arm64/tools/sysreg        | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 9d23a51d7f75..84b5c3f387d8 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -136,6 +136,10 @@ static void pvm_init_traps_aa64dfr0(struct kvm_vcpu *vcpu)
 			cptr_set |= CPTR_EL2_TTA;
 	}
 
+	/* Trap External Trace */
+	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_ExtTrcBuff), feature_ids))
+		mdcr_clear |= MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT;
+
 	vcpu->arch.mdcr_el2 |= mdcr_set;
 	vcpu->arch.mdcr_el2 &= ~mdcr_clear;
 	vcpu->arch.cptr_el2 |= cptr_set;
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 85f8b385fed2..be857797cc00 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -1115,7 +1115,10 @@ Enum	63:60	HPMN0
 	0b0000	UNPREDICTABLE
 	0b0001	DEF
 EndEnum
-Res0	59:56
+UnsignedEnum	59:56	ExtTrcBuff
+	0b0000	NI
+	0b0001	IMP
+EndEnum
 UnsignedEnum	55:52	BRBE
 	0b0000	NI
 	0b0001	IMP
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 12/12] KVM: arm64: Mark CMOW as allowed for protected VMs

[Fuad Tabba]

Cache maintenance operations are not trapped for protected VMs,
and shouldn't be. Mark the feature as allowed.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/nvhe/fixed_config.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
index e91922daa8ca..e628541585fc 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
@@ -101,6 +101,7 @@
  * - Privileged Access Never
  * - SError interrupt exceptions from speculative reads
  * - Enhanced Translation Synchronization
+ * - Control for cache maintenance permission
  */
 #define PVM_ID_AA64MMFR1_ALLOW (\
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_HAFDBS) | \
@@ -108,7 +109,8 @@
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_HPDS) | \
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_PAN) | \
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_SpecSEI) | \
-	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_ETS) \
+	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_ETS) | \
+	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_CMOW) \
 	)
 
 /*
-- 
2.43.0.rc2.451.g8631bc7472-goog

[PATCH v2 12/12] KVM: arm64: Mark CMOW as allowed for protected VMs

[Fuad Tabba]

Cache maintenance operations are not trapped for protected VMs,
and shouldn't be. Mark the feature as allowed.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/nvhe/fixed_config.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
index e91922daa8ca..e628541585fc 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
@@ -101,6 +101,7 @@
  * - Privileged Access Never
  * - SError interrupt exceptions from speculative reads
  * - Enhanced Translation Synchronization
+ * - Control for cache maintenance permission
  */
 #define PVM_ID_AA64MMFR1_ALLOW (\
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_HAFDBS) | \
@@ -108,7 +109,8 @@
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_HPDS) | \
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_PAN) | \
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_SpecSEI) | \
-	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_ETS) \
+	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_ETS) | \
+	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_CMOW) \
 	)
 
 /*
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 09/12] KVM: arm64: Generate the HFGWTR-only RES0 bits

[Fuad Tabba]

Hi,

On Wed, Dec 6, 2023 at 10:05 AM Fuad Tabba <tabba@google.com> wrote:
>
> Generate the HFGWTR-only RES0 bits in sysreg. This is done to
> consolidate all the bit definitions in one place.

I should have marked this as an RFC. I'm not sure if this approach is
better than the previous patch, or if I should just bite the bullet
and fully define HFGWTR's fields.

Cheers,
/fuad

>
> No functional change intended.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_arm.h | 13 +++----------
>  arch/arm64/tools/sysreg          | 26 ++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 5b634e909d1c..02442bc90717 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -348,16 +348,9 @@
>  #define __HFGRTR_EL2_MASK      GENMASK(49, 0)
>  #define __HFGRTR_EL2_nMASK     (GENMASK(63, 52) | BIT(50))
>
> -/*
> - * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any
> - * future additions, define __HFGWTR* macros relative to __HFGRTR* ones.
> - */
> -#define __HFGxTR_READ_ONLY_MASK        (BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> -                                GENMASK(26, 25) | BIT(21) | BIT(18) | \
> -                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> -#define __HFGWTR_EL2_RES0      (__HFGRTR_EL2_RES0 | __HFGxTR_READ_ONLY_MASK)
> -#define __HFGWTR_EL2_MASK      (__HFGRTR_EL2_MASK & ~__HFGxTR_READ_ONLY_MASK)
> -#define __HFGWTR_EL2_nMASK     (__HFGRTR_EL2_nMASK & ~__HFGxTR_READ_ONLY_MASK)
> +#define __HFGWTR_EL2_RES0      (__HFGRTR_EL2_RES0 | HFGWTR_ONLY_EL2_RES0)
> +#define __HFGWTR_EL2_MASK      (__HFGRTR_EL2_MASK & ~HFGWTR_ONLY_EL2_RES0)
> +#define __HFGWTR_EL2_nMASK     (__HFGRTR_EL2_nMASK & ~HFGWTR_ONLY_EL2_RES0)
>
>  #define __HFGITR_EL2_RES0      HFGITR_EL2_RES0
>  #define __HFGITR_EL2_MASK      (BIT(62) | BIT(60) | GENMASK(54, 0))
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index e5631f4e62f4..85f8b385fed2 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -2093,6 +2093,32 @@ Field    1       AFSR1_EL1
>  Field  0       AFSR0_EL1
>  EndSysregFields
>
> +# HFGWTR_EL2 bits are a subset of those for HFGRTR_EL2.
> +# Define RES0 only present in HFGWTR_EL2.
> +SysregFields   HFGWTR_ONLY_EL2
> +Unkn   63:47
> +Res0   46
> +Unkn   45:43
> +Res0   42
> +Unkn   41
> +Res0   40
> +Unkn   39:29
> +Res0   28
> +Unkn   27
> +Res0   26:25
> +Unkn   24:22
> +Res0   21
> +Unkn   20:19
> +Res0   18
> +Unkn   17:16
> +Res0   15:14
> +Unkn   13:11
> +Res0   10:9
> +Unkn   8:3
> +Res0   2
> +Unkn   1:0
> +EndSysregFields
> +
>  Sysreg HFGRTR_EL2      3       4       1       1       4
>  Fields HFGxTR_EL2
>  EndSysreg
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>

Re: [PATCH v2 09/12] KVM: arm64: Generate the HFGWTR-only RES0 bits

[Fuad Tabba]

Hi,

On Wed, Dec 6, 2023 at 10:05 AM Fuad Tabba <tabba@google.com> wrote:
>
> Generate the HFGWTR-only RES0 bits in sysreg. This is done to
> consolidate all the bit definitions in one place.

I should have marked this as an RFC. I'm not sure if this approach is
better than the previous patch, or if I should just bite the bullet
and fully define HFGWTR's fields.

Cheers,
/fuad

>
> No functional change intended.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_arm.h | 13 +++----------
>  arch/arm64/tools/sysreg          | 26 ++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 5b634e909d1c..02442bc90717 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -348,16 +348,9 @@
>  #define __HFGRTR_EL2_MASK      GENMASK(49, 0)
>  #define __HFGRTR_EL2_nMASK     (GENMASK(63, 52) | BIT(50))
>
> -/*
> - * The HFGWTR bits are a subset of HFGRTR bits. To ensure we don't miss any
> - * future additions, define __HFGWTR* macros relative to __HFGRTR* ones.
> - */
> -#define __HFGxTR_READ_ONLY_MASK        (BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> -                                GENMASK(26, 25) | BIT(21) | BIT(18) | \
> -                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> -#define __HFGWTR_EL2_RES0      (__HFGRTR_EL2_RES0 | __HFGxTR_READ_ONLY_MASK)
> -#define __HFGWTR_EL2_MASK      (__HFGRTR_EL2_MASK & ~__HFGxTR_READ_ONLY_MASK)
> -#define __HFGWTR_EL2_nMASK     (__HFGRTR_EL2_nMASK & ~__HFGxTR_READ_ONLY_MASK)
> +#define __HFGWTR_EL2_RES0      (__HFGRTR_EL2_RES0 | HFGWTR_ONLY_EL2_RES0)
> +#define __HFGWTR_EL2_MASK      (__HFGRTR_EL2_MASK & ~HFGWTR_ONLY_EL2_RES0)
> +#define __HFGWTR_EL2_nMASK     (__HFGRTR_EL2_nMASK & ~HFGWTR_ONLY_EL2_RES0)
>
>  #define __HFGITR_EL2_RES0      HFGITR_EL2_RES0
>  #define __HFGITR_EL2_MASK      (BIT(62) | BIT(60) | GENMASK(54, 0))
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index e5631f4e62f4..85f8b385fed2 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -2093,6 +2093,32 @@ Field    1       AFSR1_EL1
>  Field  0       AFSR0_EL1
>  EndSysregFields
>
> +# HFGWTR_EL2 bits are a subset of those for HFGRTR_EL2.
> +# Define RES0 only present in HFGWTR_EL2.
> +SysregFields   HFGWTR_ONLY_EL2
> +Unkn   63:47
> +Res0   46
> +Unkn   45:43
> +Res0   42
> +Unkn   41
> +Res0   40
> +Unkn   39:29
> +Res0   28
> +Unkn   27
> +Res0   26:25
> +Unkn   24:22
> +Res0   21
> +Unkn   20:19
> +Res0   18
> +Unkn   17:16
> +Res0   15:14
> +Unkn   13:11
> +Res0   10:9
> +Unkn   8:3
> +Res0   2
> +Unkn   1:0
> +EndSysregFields
> +
>  Sysreg HFGRTR_EL2      3       4       1       1       4
>  Fields HFGxTR_EL2
>  EndSysreg
> --
> 2.43.0.rc2.451.g8631bc7472-goog
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Joey Gouly]

Hello Fuad,

On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> New trap bits have been defined in the 2023-09 Arm Architecture
> System Registers xml specification [*]. Moreover, the existing
> definitions of some of the mask and the RES0 bits overlap, which
> could be wrong, confusing, or both.
> 
> Update the bits to represent the latest spec (as of this patch,
> 2023-09), and ensure that the existing bits are consistent.
> 
> Subsequent patches will use the generated RES0 fields instead of
> specifying them manually. This patch keeps the manual encoding of
> the bits to make it easier to review the series.
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools
> 
> Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 7de0a7062625..b0dc3249d5cd 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -344,30 +344,39 @@
>   * Once we get to a point where the two describe the same thing, we'll
>   * merge the definitions. One day.
>   */
> -#define __HFGRTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51))
> +#define __HFGRTR_EL2_RES0	BIT(51)
>  #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
> -#define __HFGRTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> +#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
>  
> -#define __HFGWTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51) |	\
> -				 BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> -				 GENMASK(26, 25) | BIT(21) | BIT(18) |	\
> +#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> +				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
>  				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> -#define __HFGWTR_EL2_MASK	GENMASK(49, 0)
> -#define __HFGWTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> +#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
> +				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
> +				 GENMASK(24, 22) | GENMASK(20, 19) | \
> +				 GENMASK(17, 16) | GENMASK(13, 11) | \
> +				 GENMASK(8, 3) | GENMASK(1, 0))
> +#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))

By adding all these bits to *_nMASK, we're allowing a guest to access registers
which KVM doesn't (currently) deal with.  For example if I apply this patch
series, a guest can access S2POR_EL1, previously it would print something like:

	kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]                                                                      
	 { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read }, 

After applying this patch series, the guest can read S2POR_EL1.

We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
guest shouldn't access it, but there's nothing stopping it.

My question is, is this intended? Or do we need to update the following code
(and comment!) to trap all the stuff we don't currently handle (along with
ACCDATA_EL1):

	static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
	{

		..

	        /* The default is not to trap anything but ACCDATA_EL1 */
        	r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
	        r_val |= r_set;
        	r_val &= ~r_clr;


Thanks,
Joey

>  
> -#define __HFGITR_EL2_RES0	GENMASK(63, 57)
> -#define __HFGITR_EL2_MASK	GENMASK(54, 0)
> -#define __HFGITR_EL2_nMASK	GENMASK(56, 55)
> +#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
> +#define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
> +#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
>  
>  #define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
>  				 GENMASK(21, 20) | BIT(8))
> -#define __HDFGRTR_EL2_MASK	~__HDFGRTR_EL2_nMASK
> +#define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> +				 GENMASK(41, 40) | GENMASK(37, 22) | \
> +				 GENMASK(19, 9) | GENMASK(7, 0))
>  #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
>  
>  #define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
>  				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
>  				 BIT(22) | BIT(9) | BIT(6))
> -#define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
> +#define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
> +				 GENMASK(46, 44) | GENMASK(42, 41) | \
> +				 GENMASK(37, 35) | GENMASK(33, 31) | \
> +				 GENMASK(29, 23) | GENMASK(21, 10) | \
> +				 GENMASK(8, 7) | GENMASK(5, 0))
>  #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
>  
>  #define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
> @@ -375,9 +384,9 @@
>  #define __HAFGRTR_EL2_nMASK	0UL
>  
>  /* Similar definitions for HCRX_EL2 */
> -#define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
> -#define __HCRX_EL2_MASK		(0)
> -#define __HCRX_EL2_nMASK	(GENMASK(15, 14) | GENMASK(4, 0))
> +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> +#define __HCRX_EL2_MASK		(BIT(6))
> +#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~UL(0xf))
> -- 
> 2.43.0.rc2.451.g8631bc7472-goog
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Joey Gouly]

Hello Fuad,

On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> New trap bits have been defined in the 2023-09 Arm Architecture
> System Registers xml specification [*]. Moreover, the existing
> definitions of some of the mask and the RES0 bits overlap, which
> could be wrong, confusing, or both.
> 
> Update the bits to represent the latest spec (as of this patch,
> 2023-09), and ensure that the existing bits are consistent.
> 
> Subsequent patches will use the generated RES0 fields instead of
> specifying them manually. This patch keeps the manual encoding of
> the bits to make it easier to review the series.
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools
> 
> Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 7de0a7062625..b0dc3249d5cd 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -344,30 +344,39 @@
>   * Once we get to a point where the two describe the same thing, we'll
>   * merge the definitions. One day.
>   */
> -#define __HFGRTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51))
> +#define __HFGRTR_EL2_RES0	BIT(51)
>  #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
> -#define __HFGRTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> +#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
>  
> -#define __HFGWTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51) |	\
> -				 BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> -				 GENMASK(26, 25) | BIT(21) | BIT(18) |	\
> +#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> +				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
>  				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> -#define __HFGWTR_EL2_MASK	GENMASK(49, 0)
> -#define __HFGWTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> +#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
> +				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
> +				 GENMASK(24, 22) | GENMASK(20, 19) | \
> +				 GENMASK(17, 16) | GENMASK(13, 11) | \
> +				 GENMASK(8, 3) | GENMASK(1, 0))
> +#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))

By adding all these bits to *_nMASK, we're allowing a guest to access registers
which KVM doesn't (currently) deal with.  For example if I apply this patch
series, a guest can access S2POR_EL1, previously it would print something like:

	kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]                                                                      
	 { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read }, 

After applying this patch series, the guest can read S2POR_EL1.

We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
guest shouldn't access it, but there's nothing stopping it.

My question is, is this intended? Or do we need to update the following code
(and comment!) to trap all the stuff we don't currently handle (along with
ACCDATA_EL1):

	static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
	{

		..

	        /* The default is not to trap anything but ACCDATA_EL1 */
        	r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
	        r_val |= r_set;
        	r_val &= ~r_clr;


Thanks,
Joey

>  
> -#define __HFGITR_EL2_RES0	GENMASK(63, 57)
> -#define __HFGITR_EL2_MASK	GENMASK(54, 0)
> -#define __HFGITR_EL2_nMASK	GENMASK(56, 55)
> +#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
> +#define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
> +#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
>  
>  #define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
>  				 GENMASK(21, 20) | BIT(8))
> -#define __HDFGRTR_EL2_MASK	~__HDFGRTR_EL2_nMASK
> +#define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> +				 GENMASK(41, 40) | GENMASK(37, 22) | \
> +				 GENMASK(19, 9) | GENMASK(7, 0))
>  #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
>  
>  #define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
>  				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
>  				 BIT(22) | BIT(9) | BIT(6))
> -#define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
> +#define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
> +				 GENMASK(46, 44) | GENMASK(42, 41) | \
> +				 GENMASK(37, 35) | GENMASK(33, 31) | \
> +				 GENMASK(29, 23) | GENMASK(21, 10) | \
> +				 GENMASK(8, 7) | GENMASK(5, 0))
>  #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
>  
>  #define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
> @@ -375,9 +384,9 @@
>  #define __HAFGRTR_EL2_nMASK	0UL
>  
>  /* Similar definitions for HCRX_EL2 */
> -#define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
> -#define __HCRX_EL2_MASK		(0)
> -#define __HCRX_EL2_nMASK	(GENMASK(15, 14) | GENMASK(4, 0))
> +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> +#define __HCRX_EL2_MASK		(BIT(6))
> +#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~UL(0xf))
> -- 
> 2.43.0.rc2.451.g8631bc7472-goog
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

Hi Joey,

On Thu, Dec 7, 2023 at 3:00 PM Joey Gouly <joey.gouly@arm.com> wrote:
>
> Hello Fuad,
>
> On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> > New trap bits have been defined in the 2023-09 Arm Architecture
> > System Registers xml specification [*]. Moreover, the existing
> > definitions of some of the mask and the RES0 bits overlap, which
> > could be wrong, confusing, or both.
> >
> > Update the bits to represent the latest spec (as of this patch,
> > 2023-09), and ensure that the existing bits are consistent.
> >
> > Subsequent patches will use the generated RES0 fields instead of
> > specifying them manually. This patch keeps the manual encoding of
> > the bits to make it easier to review the series.
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
> >
> > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> >  1 file changed, 24 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > index 7de0a7062625..b0dc3249d5cd 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -344,30 +344,39 @@
> >   * Once we get to a point where the two describe the same thing, we'll
> >   * merge the definitions. One day.
> >   */
> > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > +#define __HFGRTR_EL2_RES0    BIT(51)
> >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> >
> > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
>
> By adding all these bits to *_nMASK, we're allowing a guest to access registers
> which KVM doesn't (currently) deal with.  For example if I apply this patch
> series, a guest can access S2POR_EL1, previously it would print something like:
>
>         kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]
>          { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read },
>
> After applying this patch series, the guest can read S2POR_EL1.
>
> We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
> guest shouldn't access it, but there's nothing stopping it.
>
> My question is, is this intended? Or do we need to update the following code
> (and comment!) to trap all the stuff we don't currently handle (along with
> ACCDATA_EL1):

Thanks for pointing this out, and no, it's not intended. For
consistency, I think it might be good to update the code, as you
suggest, to prevent these accesses. If you and the other reviewers are
happy with that solution I can respin with your suggested fix. I have
a couple of other minor fixes as well for the series.

What do you think?

Cheers,
/fuad


>
>         static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
>         {
>
>                 ..
>
>                 /* The default is not to trap anything but ACCDATA_EL1 */
>                 r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
>                 r_val |= r_set;
>                 r_val &= ~r_clr;
>
>
> Thanks,
> Joey
>
> >
> > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> >
> >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> >                                GENMASK(21, 20) | BIT(8))
> > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > +                              GENMASK(19, 9) | GENMASK(7, 0))
> >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> >
> >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> >                                BIT(22) | BIT(9) | BIT(6))
> > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > +                              GENMASK(8, 7) | GENMASK(5, 0))
> >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> >
> >  #define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > @@ -375,9 +384,9 @@
> >  #define __HAFGRTR_EL2_nMASK  0UL
> >
> >  /* Similar definitions for HCRX_EL2 */
> > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > -#define __HCRX_EL2_MASK              (0)
> > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > +#define __HCRX_EL2_MASK              (BIT(6))
> > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> >
> >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> >  #define HPFAR_MASK   (~UL(0xf))
> > --
> > 2.43.0.rc2.451.g8631bc7472-goog
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

Hi Joey,

On Thu, Dec 7, 2023 at 3:00 PM Joey Gouly <joey.gouly@arm.com> wrote:
>
> Hello Fuad,
>
> On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> > New trap bits have been defined in the 2023-09 Arm Architecture
> > System Registers xml specification [*]. Moreover, the existing
> > definitions of some of the mask and the RES0 bits overlap, which
> > could be wrong, confusing, or both.
> >
> > Update the bits to represent the latest spec (as of this patch,
> > 2023-09), and ensure that the existing bits are consistent.
> >
> > Subsequent patches will use the generated RES0 fields instead of
> > specifying them manually. This patch keeps the manual encoding of
> > the bits to make it easier to review the series.
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
> >
> > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> >  1 file changed, 24 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > index 7de0a7062625..b0dc3249d5cd 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -344,30 +344,39 @@
> >   * Once we get to a point where the two describe the same thing, we'll
> >   * merge the definitions. One day.
> >   */
> > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > +#define __HFGRTR_EL2_RES0    BIT(51)
> >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> >
> > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
>
> By adding all these bits to *_nMASK, we're allowing a guest to access registers
> which KVM doesn't (currently) deal with.  For example if I apply this patch
> series, a guest can access S2POR_EL1, previously it would print something like:
>
>         kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]
>          { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read },
>
> After applying this patch series, the guest can read S2POR_EL1.
>
> We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
> guest shouldn't access it, but there's nothing stopping it.
>
> My question is, is this intended? Or do we need to update the following code
> (and comment!) to trap all the stuff we don't currently handle (along with
> ACCDATA_EL1):

Thanks for pointing this out, and no, it's not intended. For
consistency, I think it might be good to update the code, as you
suggest, to prevent these accesses. If you and the other reviewers are
happy with that solution I can respin with your suggested fix. I have
a couple of other minor fixes as well for the series.

What do you think?

Cheers,
/fuad


>
>         static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
>         {
>
>                 ..
>
>                 /* The default is not to trap anything but ACCDATA_EL1 */
>                 r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
>                 r_val |= r_set;
>                 r_val &= ~r_clr;
>
>
> Thanks,
> Joey
>
> >
> > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> >
> >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> >                                GENMASK(21, 20) | BIT(8))
> > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > +                              GENMASK(19, 9) | GENMASK(7, 0))
> >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> >
> >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> >                                BIT(22) | BIT(9) | BIT(6))
> > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > +                              GENMASK(8, 7) | GENMASK(5, 0))
> >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> >
> >  #define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > @@ -375,9 +384,9 @@
> >  #define __HAFGRTR_EL2_nMASK  0UL
> >
> >  /* Similar definitions for HCRX_EL2 */
> > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > -#define __HCRX_EL2_MASK              (0)
> > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > +#define __HCRX_EL2_MASK              (BIT(6))
> > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> >
> >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> >  #define HPFAR_MASK   (~UL(0xf))
> > --
> > 2.43.0.rc2.451.g8631bc7472-goog
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Mark Brown]

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

On Wed, Dec 06, 2023 at 10:04:51AM +0000, Fuad Tabba wrote:
> Add the latest field definitions to HCRX_EL2 from the 2023-09 Arm
> Architecture System Registers xml specification [*].
> 
> These fields aren't used yet. This is done mainly to get the
> correct generated value for the register's RES0 mask.

> ---
>  arch/arm64/tools/sysreg | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

This isn't a KVM specific patch, this is an arch patch - I'd have
expected a subject line "arm64/sysreg:" or something...

>  Sysreg	HCRX_EL2	3	4	1	2	2
> -Res0	63:23
> +Res0	63:25
> +Field	24	PACMEn
> +Field	23	EnFPM
>  Field	22	GCSEn
>  Field	21	EnIDCP128
>  Field	20	EnSDERR

This is the same as my equivalent patch in the 2023 dpISA series:

   https://lore.kernel.org/linux-arm-kernel/20231205-arm64-2023-dpisa-v3-7-dbcbcd867a7f@kernel.org/

Reviewed-by: Mark Brown <broonie@kernel.org>

(that series does make use of EnFPM).  Given the likelyhood of overlaps
like this I really think we should be merging these sysreg updates even
if the serieses that depend on them are still in flight, it doesn't make
sense for multiple people to end up doing the typing.

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

Re: [PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1195 bytes --]

On Wed, Dec 06, 2023 at 10:04:51AM +0000, Fuad Tabba wrote:
> Add the latest field definitions to HCRX_EL2 from the 2023-09 Arm
> Architecture System Registers xml specification [*].
> 
> These fields aren't used yet. This is done mainly to get the
> correct generated value for the register's RES0 mask.

> ---
>  arch/arm64/tools/sysreg | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

This isn't a KVM specific patch, this is an arch patch - I'd have
expected a subject line "arm64/sysreg:" or something...

>  Sysreg	HCRX_EL2	3	4	1	2	2
> -Res0	63:23
> +Res0	63:25
> +Field	24	PACMEn
> +Field	23	EnFPM
>  Field	22	GCSEn
>  Field	21	EnIDCP128
>  Field	20	EnSDERR

This is the same as my equivalent patch in the 2023 dpISA series:

   https://lore.kernel.org/linux-arm-kernel/20231205-arm64-2023-dpisa-v3-7-dbcbcd867a7f@kernel.org/

Reviewed-by: Mark Brown <broonie@kernel.org>

(that series does make use of EnFPM).  Given the likelyhood of overlaps
like this I really think we should be merging these sysreg updates even
if the serieses that depend on them are still in flight, it doesn't make
sense for multiple people to end up doing the typing.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/12] KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt

[Mark Brown]

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

On Wed, Dec 06, 2023 at 10:04:52AM +0000, Fuad Tabba wrote:

> +#define SYS_GCSCR_EL1			sys_reg(3, 0, 2, 5, 0)
> +#define SYS_GCSPR_EL1			sys_reg(3, 0, 2, 5, 1)
> +#define SYS_GCSCRE0_EL1			sys_reg(3, 0, 2, 5, 2)
> +

> +#define SYS_GCSPR_EL0			sys_reg(3, 3, 2, 5, 1)
> +

Unless there's some complication with representing them (mainly register
layouts that aren't representable with the language or *very* repetitive
blocks of registers) we should be adding any new sysregs to the sysreg
file rather than manually encoding them.  We're trying to move things
out of sysreg.h as much as possible.

For the above you can pick up the patch from my GCS series:

  https://lore.kernel.org/linux-arm-kernel/20231122-arm64-gcs-v7-6-201c483bd775@kernel.org/

> @@ -412,6 +423,8 @@
>  #define SYS_PMUSERENR_EL0		sys_reg(3, 3, 9, 14, 0)
>  #define SYS_PMOVSSET_EL0		sys_reg(3, 3, 9, 14, 3)
>  
> +#define SYS_POR_EL0			sys_reg(3, 3, 10, 2, 4)
> +

This is in Joey's POR series:

  https://lore.kernel.org/linux-arm-kernel/20231124163510.1835740-2-joey.gouly@arm.com/

Unfortunately I'm not aware of anyone having already done the rest.

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

Re: [PATCH v2 02/12] KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1159 bytes --]

On Wed, Dec 06, 2023 at 10:04:52AM +0000, Fuad Tabba wrote:

> +#define SYS_GCSCR_EL1			sys_reg(3, 0, 2, 5, 0)
> +#define SYS_GCSPR_EL1			sys_reg(3, 0, 2, 5, 1)
> +#define SYS_GCSCRE0_EL1			sys_reg(3, 0, 2, 5, 2)
> +

> +#define SYS_GCSPR_EL0			sys_reg(3, 3, 2, 5, 1)
> +

Unless there's some complication with representing them (mainly register
layouts that aren't representable with the language or *very* repetitive
blocks of registers) we should be adding any new sysregs to the sysreg
file rather than manually encoding them.  We're trying to move things
out of sysreg.h as much as possible.

For the above you can pick up the patch from my GCS series:

  https://lore.kernel.org/linux-arm-kernel/20231122-arm64-gcs-v7-6-201c483bd775@kernel.org/

> @@ -412,6 +423,8 @@
>  #define SYS_PMUSERENR_EL0		sys_reg(3, 3, 9, 14, 0)
>  #define SYS_PMOVSSET_EL0		sys_reg(3, 3, 9, 14, 3)
>  
> +#define SYS_POR_EL0			sys_reg(3, 3, 10, 2, 4)
> +

This is in Joey's POR series:

  https://lore.kernel.org/linux-arm-kernel/20231124163510.1835740-2-joey.gouly@arm.com/

Unfortunately I'm not aware of anyone having already done the rest.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Joey Gouly]

On Thu, Dec 07, 2023 at 03:06:44PM +0000, Fuad Tabba wrote:
> Hi Joey,
> 
> On Thu, Dec 7, 2023 at 3:00 PM Joey Gouly <joey.gouly@arm.com> wrote:
> >
> > Hello Fuad,
> >
> > On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> > > New trap bits have been defined in the 2023-09 Arm Architecture
> > > System Registers xml specification [*]. Moreover, the existing
> > > definitions of some of the mask and the RES0 bits overlap, which
> > > could be wrong, confusing, or both.
> > >
> > > Update the bits to represent the latest spec (as of this patch,
> > > 2023-09), and ensure that the existing bits are consistent.
> > >
> > > Subsequent patches will use the generated RES0 fields instead of
> > > specifying them manually. This patch keeps the manual encoding of
> > > the bits to make it easier to review the series.
> > >
> > > [*] https://developer.arm.com/downloads/-/exploration-tools
> > >
> > > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> > >  1 file changed, 24 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > > index 7de0a7062625..b0dc3249d5cd 100644
> > > --- a/arch/arm64/include/asm/kvm_arm.h
> > > +++ b/arch/arm64/include/asm/kvm_arm.h
> > > @@ -344,30 +344,39 @@
> > >   * Once we get to a point where the two describe the same thing, we'll
> > >   * merge the definitions. One day.
> > >   */
> > > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > > +#define __HFGRTR_EL2_RES0    BIT(51)
> > >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> > >
> > > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> > >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> >
> > By adding all these bits to *_nMASK, we're allowing a guest to access registers
> > which KVM doesn't (currently) deal with.  For example if I apply this patch
> > series, a guest can access S2POR_EL1, previously it would print something like:
> >
> >         kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]
> >          { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read },
> >
> > After applying this patch series, the guest can read S2POR_EL1.
> >
> > We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
> > guest shouldn't access it, but there's nothing stopping it.
> >
> > My question is, is this intended? Or do we need to update the following code
> > (and comment!) to trap all the stuff we don't currently handle (along with
> > ACCDATA_EL1):
> 
> Thanks for pointing this out, and no, it's not intended. For
> consistency, I think it might be good to update the code, as you
> suggest, to prevent these accesses. If you and the other reviewers are
> happy with that solution I can respin with your suggested fix. I have
> a couple of other minor fixes as well for the series.
> 

Thanks for clarifying that.

To me it seems weird to use the generated bits directly, and I think that's why
the code seems confusing?

I think we should stop using __HFGRTR_EL2_nMASK directly in that function, and
build it up from '0'. This also matches, I think, with Marc Z wanting to move
towards setting these things based on features [1], so maybe something like:

	u64 r_val = 0;

	if (cpus_have_final_cap(ARM64_HAS_S1PIE))
		r_set |= HFGxTR_EL2_nPIR_EL1 | HFGxTR_EL2_nPIREO_EL1;

	r_val |= r_set;
	r_val &= ~r_clr;

	write_sysreg_s(r_val, SYS_HFGRTR_EL2);

That way we just explicitly set which bits we don't want to trap on. We have
some weird behaviour right now were we set nSMPRI_EL1 to 1, unless you have
FEAT_SME where we set it to 0.. but if you don't have FEAT_SME it's RES0
anyway? The above approach would fix that.

Thanks,
Joey

[1] https://lore.kernel.org/linux-arm-kernel/87bkb285ud.wl-maz@kernel.org/ (last paragraph)

> 
> Cheers,
> /fuad
> 
> 
> >
> >         static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
> >         {
> >
> >                 ..
> >
> >                 /* The default is not to trap anything but ACCDATA_EL1 */
> >                 r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
> >                 r_val |= r_set;
> >                 r_val &= ~r_clr;
> >
> >
> > Thanks,
> > Joey
> >
> > >
> > > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> > >
> > >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> > >                                GENMASK(21, 20) | BIT(8))
> > > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > > +                              GENMASK(19, 9) | GENMASK(7, 0))
> > >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> > >
> > >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> > >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> > >                                BIT(22) | BIT(9) | BIT(6))
> > > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > > +                              GENMASK(8, 7) | GENMASK(5, 0))
> > >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> > >
> > >  #define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > > @@ -375,9 +384,9 @@
> > >  #define __HAFGRTR_EL2_nMASK  0UL
> > >
> > >  /* Similar definitions for HCRX_EL2 */
> > > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > > -#define __HCRX_EL2_MASK              (0)
> > > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > > +#define __HCRX_EL2_MASK              (BIT(6))
> > > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> > >
> > >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> > >  #define HPFAR_MASK   (~UL(0xf))
> > > --
> > > 2.43.0.rc2.451.g8631bc7472-goog
> > >
> > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Joey Gouly]

On Thu, Dec 07, 2023 at 03:06:44PM +0000, Fuad Tabba wrote:
> Hi Joey,
> 
> On Thu, Dec 7, 2023 at 3:00 PM Joey Gouly <joey.gouly@arm.com> wrote:
> >
> > Hello Fuad,
> >
> > On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> > > New trap bits have been defined in the 2023-09 Arm Architecture
> > > System Registers xml specification [*]. Moreover, the existing
> > > definitions of some of the mask and the RES0 bits overlap, which
> > > could be wrong, confusing, or both.
> > >
> > > Update the bits to represent the latest spec (as of this patch,
> > > 2023-09), and ensure that the existing bits are consistent.
> > >
> > > Subsequent patches will use the generated RES0 fields instead of
> > > specifying them manually. This patch keeps the manual encoding of
> > > the bits to make it easier to review the series.
> > >
> > > [*] https://developer.arm.com/downloads/-/exploration-tools
> > >
> > > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > ---
> > >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> > >  1 file changed, 24 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > > index 7de0a7062625..b0dc3249d5cd 100644
> > > --- a/arch/arm64/include/asm/kvm_arm.h
> > > +++ b/arch/arm64/include/asm/kvm_arm.h
> > > @@ -344,30 +344,39 @@
> > >   * Once we get to a point where the two describe the same thing, we'll
> > >   * merge the definitions. One day.
> > >   */
> > > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > > +#define __HFGRTR_EL2_RES0    BIT(51)
> > >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> > >
> > > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> > >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> >
> > By adding all these bits to *_nMASK, we're allowing a guest to access registers
> > which KVM doesn't (currently) deal with.  For example if I apply this patch
> > series, a guest can access S2POR_EL1, previously it would print something like:
> >
> >         kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]
> >          { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read },
> >
> > After applying this patch series, the guest can read S2POR_EL1.
> >
> > We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
> > guest shouldn't access it, but there's nothing stopping it.
> >
> > My question is, is this intended? Or do we need to update the following code
> > (and comment!) to trap all the stuff we don't currently handle (along with
> > ACCDATA_EL1):
> 
> Thanks for pointing this out, and no, it's not intended. For
> consistency, I think it might be good to update the code, as you
> suggest, to prevent these accesses. If you and the other reviewers are
> happy with that solution I can respin with your suggested fix. I have
> a couple of other minor fixes as well for the series.
> 

Thanks for clarifying that.

To me it seems weird to use the generated bits directly, and I think that's why
the code seems confusing?

I think we should stop using __HFGRTR_EL2_nMASK directly in that function, and
build it up from '0'. This also matches, I think, with Marc Z wanting to move
towards setting these things based on features [1], so maybe something like:

	u64 r_val = 0;

	if (cpus_have_final_cap(ARM64_HAS_S1PIE))
		r_set |= HFGxTR_EL2_nPIR_EL1 | HFGxTR_EL2_nPIREO_EL1;

	r_val |= r_set;
	r_val &= ~r_clr;

	write_sysreg_s(r_val, SYS_HFGRTR_EL2);

That way we just explicitly set which bits we don't want to trap on. We have
some weird behaviour right now were we set nSMPRI_EL1 to 1, unless you have
FEAT_SME where we set it to 0.. but if you don't have FEAT_SME it's RES0
anyway? The above approach would fix that.

Thanks,
Joey

[1] https://lore.kernel.org/linux-arm-kernel/87bkb285ud.wl-maz@kernel.org/ (last paragraph)

> 
> Cheers,
> /fuad
> 
> 
> >
> >         static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
> >         {
> >
> >                 ..
> >
> >                 /* The default is not to trap anything but ACCDATA_EL1 */
> >                 r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
> >                 r_val |= r_set;
> >                 r_val &= ~r_clr;
> >
> >
> > Thanks,
> > Joey
> >
> > >
> > > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> > >
> > >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> > >                                GENMASK(21, 20) | BIT(8))
> > > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > > +                              GENMASK(19, 9) | GENMASK(7, 0))
> > >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> > >
> > >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> > >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> > >                                BIT(22) | BIT(9) | BIT(6))
> > > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > > +                              GENMASK(8, 7) | GENMASK(5, 0))
> > >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> > >
> > >  #define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > > @@ -375,9 +384,9 @@
> > >  #define __HAFGRTR_EL2_nMASK  0UL
> > >
> > >  /* Similar definitions for HCRX_EL2 */
> > > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > > -#define __HCRX_EL2_MASK              (0)
> > > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > > +#define __HCRX_EL2_MASK              (BIT(6))
> > > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> > >
> > >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> > >  #define HPFAR_MASK   (~UL(0xf))
> > > --
> > > 2.43.0.rc2.451.g8631bc7472-goog
> > >
> > >

Re: [PATCH v2 03/12] KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt

[Mark Brown]

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

On Wed, Dec 06, 2023 at 10:04:53AM +0000, Fuad Tabba wrote:

> Add the missing nested virt FGT table entries HFGITR_EL2. Based
> on the 2023-09 Arm Architecture System Registers xml
> specification [*]. Add the missing field definitions as well,
> both to generate the correct RES0 mask and to be able to toggle
> their FGT bits.
> 
> Also adds definitions of some of the missing system registers and
> instructions, which can be trapped by the FGT bits.
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools

That should be a reference to DDI0602 (for the instruction XML) and
DD0601 (for the system register XML) - it's the same content, just a
more stable name.

>  Sysreg HFGITR_EL2	3	4	1	1	6
> -Res0	63:61
> +Res0	63
> +Field	62	ATS1E1A
> +Res0	61
>  Field	60	COSPRCTX
>  Field	59	nGCSEPP
>  Field	58	nGCSSTR_EL1

The sysreg looks good:

Reviewed-by: Mark Brown <broonie@kernel.org>

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

Re: [PATCH v2 03/12] KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 928 bytes --]

On Wed, Dec 06, 2023 at 10:04:53AM +0000, Fuad Tabba wrote:

> Add the missing nested virt FGT table entries HFGITR_EL2. Based
> on the 2023-09 Arm Architecture System Registers xml
> specification [*]. Add the missing field definitions as well,
> both to generate the correct RES0 mask and to be able to toggle
> their FGT bits.
> 
> Also adds definitions of some of the missing system registers and
> instructions, which can be trapped by the FGT bits.
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools

That should be a reference to DDI0602 (for the instruction XML) and
DD0601 (for the system register XML) - it's the same content, just a
more stable name.

>  Sysreg HFGITR_EL2	3	4	1	1	6
> -Res0	63:61
> +Res0	63
> +Field	62	ATS1E1A
> +Res0	61
>  Field	60	COSPRCTX
>  Field	59	nGCSEPP
>  Field	58	nGCSSTR_EL1

The sysreg looks good:

Reviewed-by: Mark Brown <broonie@kernel.org>

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 04/12] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Mark Brown]

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

On Wed, Dec 06, 2023 at 10:04:54AM +0000, Fuad Tabba wrote:
> To support HAFGRTR_EL2 supported in nested virt in the following
> patch, first add its bitmask definitions based on the 2023-09
> Arm Architecture System Registers xml specification [*].
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools

DDI0601 is a more stable name, and I'd have expected an "arm64/sysreg"
subject.

> +#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
> +#define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
> +#define __HAFGRTR_EL2_nMASK	0UL
> +

This one is an example of something where some manual encoding does make
sense given all the AMCNTEN<x> and so on fields so:

Reviewed-by: Mark Brown <broonie@kernel.org>

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

Re: [PATCH v2 04/12] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 756 bytes --]

On Wed, Dec 06, 2023 at 10:04:54AM +0000, Fuad Tabba wrote:
> To support HAFGRTR_EL2 supported in nested virt in the following
> patch, first add its bitmask definitions based on the 2023-09
> Arm Architecture System Registers xml specification [*].
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools

DDI0601 is a more stable name, and I'd have expected an "arm64/sysreg"
subject.

> +#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
> +#define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
> +#define __HAFGRTR_EL2_nMASK	0UL
> +

This one is an example of something where some manual encoding does make
sense given all the AMCNTEN<x> and so on fields so:

Reviewed-by: Mark Brown <broonie@kernel.org>

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Mark Brown]

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

On Wed, Dec 06, 2023 at 10:04:55AM +0000, Fuad Tabba wrote:

> Add the encodings to fine grain trapping fields for HAFGRTR_EL2
> and add the associated handling code in nested virt. Based on
> the 2023-09 Arm Architecture System Registers xml
> specification [*]. Add the missing field definitions as well,
> both to generate the correct RES0 mask and to be able to toggle
> their FGT bits.

> Also add the code for handling FGT trapping, reading of the
> register, to nested virt.

> [*] https://developer.arm.com/downloads/-/exploration-tools

DDI0601.

> +Sysreg HAFGRTR_EL2	3	4	3	1	6
> +Res0	63:50
> +Field	49	AMEVTYPER115_EL0
> +Field	48	AMEVCNTR115_EL0
> +Field	47	AMEVTYPER114_EL0
> +Field	46	AMEVCNTR114_EL0
> +Field	45	AMEVTYPER113_EL0
> +Field	44	AMEVCNTR113_EL0
> +Field	43	AMEVTYPER112_EL0
> +Field	42	AMEVCNTR112_EL0
> +Field	41	AMEVTYPER111_EL0
> +Field	40	AMEVCNTR111_EL0
> +Field	39	AMEVTYPER110_EL0
> +Field	38	AMEVCNTR110_EL0
> +Field	37	AMEVTYPER19_EL0
> +Field	36	AMEVCNTR19_EL0
> +Field	35	AMEVTYPER18_EL0
> +Field	34	AMEVCNTR18_EL0
> +Field	33	AMEVTYPER17_EL0
> +Field	32	AMEVCNTR17_EL0
> +Field	31	AMEVTYPER16_EL0
> +Field	30	AMEVCNTR16_EL0
> +Field	29	AMEVTYPER15_EL0
> +Field	28	AMEVCNTR15_EL0
> +Field	27	AMEVTYPER14_EL0
> +Field	26	AMEVCNTR14_EL0
> +Field	25	AMEVTYPER13_EL0
> +Field	24	AMEVCNTR13_EL0
> +Field	23	AMEVTYPER12_EL0
> +Field	22	AMEVCNTR12_EL0
> +Field	21	AMEVTYPER11_EL0
> +Field	20	AMEVCNTR11_EL0
> +Field	19	AMEVTYPER10_EL0
> +Field	18	AMEVCNTR10_EL0
> +Field	17	AMCNTEN1
> +Res0	16:5
> +Field	4	AMEVCNTR03_EL0
> +Field	3	AMEVCNTR02_EL0
> +Field	2	AMEVCNTR01_EL0
> +Field	1	AMEVCNTR00_EL0
> +Field	0	AMCNTEN0
> +EndSysreg
> +

Oh, we actually have the register fully encoded since we needed the
fields.  Given this why did we need the manual encoding in the previous
patch?  In any case this looks good according to DDI0601 2023-09:

Reviewed-by: Mark Brown <broonie@kernel.org>

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

Re: [PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1922 bytes --]

On Wed, Dec 06, 2023 at 10:04:55AM +0000, Fuad Tabba wrote:

> Add the encodings to fine grain trapping fields for HAFGRTR_EL2
> and add the associated handling code in nested virt. Based on
> the 2023-09 Arm Architecture System Registers xml
> specification [*]. Add the missing field definitions as well,
> both to generate the correct RES0 mask and to be able to toggle
> their FGT bits.

> Also add the code for handling FGT trapping, reading of the
> register, to nested virt.

> [*] https://developer.arm.com/downloads/-/exploration-tools

DDI0601.

> +Sysreg HAFGRTR_EL2	3	4	3	1	6
> +Res0	63:50
> +Field	49	AMEVTYPER115_EL0
> +Field	48	AMEVCNTR115_EL0
> +Field	47	AMEVTYPER114_EL0
> +Field	46	AMEVCNTR114_EL0
> +Field	45	AMEVTYPER113_EL0
> +Field	44	AMEVCNTR113_EL0
> +Field	43	AMEVTYPER112_EL0
> +Field	42	AMEVCNTR112_EL0
> +Field	41	AMEVTYPER111_EL0
> +Field	40	AMEVCNTR111_EL0
> +Field	39	AMEVTYPER110_EL0
> +Field	38	AMEVCNTR110_EL0
> +Field	37	AMEVTYPER19_EL0
> +Field	36	AMEVCNTR19_EL0
> +Field	35	AMEVTYPER18_EL0
> +Field	34	AMEVCNTR18_EL0
> +Field	33	AMEVTYPER17_EL0
> +Field	32	AMEVCNTR17_EL0
> +Field	31	AMEVTYPER16_EL0
> +Field	30	AMEVCNTR16_EL0
> +Field	29	AMEVTYPER15_EL0
> +Field	28	AMEVCNTR15_EL0
> +Field	27	AMEVTYPER14_EL0
> +Field	26	AMEVCNTR14_EL0
> +Field	25	AMEVTYPER13_EL0
> +Field	24	AMEVCNTR13_EL0
> +Field	23	AMEVTYPER12_EL0
> +Field	22	AMEVCNTR12_EL0
> +Field	21	AMEVTYPER11_EL0
> +Field	20	AMEVCNTR11_EL0
> +Field	19	AMEVTYPER10_EL0
> +Field	18	AMEVCNTR10_EL0
> +Field	17	AMCNTEN1
> +Res0	16:5
> +Field	4	AMEVCNTR03_EL0
> +Field	3	AMEVCNTR02_EL0
> +Field	2	AMEVCNTR01_EL0
> +Field	1	AMEVCNTR00_EL0
> +Field	0	AMCNTEN0
> +EndSysreg
> +

Oh, we actually have the register fully encoded since we needed the
fields.  Given this why did we need the manual encoding in the previous
patch?  In any case this looks good according to DDI0601 2023-09:

Reviewed-by: Mark Brown <broonie@kernel.org>

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Fuad Tabba]

Hi Mark,

Thanks for your reviews!

On Thu, Dec 7, 2023 at 4:57 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:51AM +0000, Fuad Tabba wrote:
> > Add the latest field definitions to HCRX_EL2 from the 2023-09 Arm
> > Architecture System Registers xml specification [*].
> >
> > These fields aren't used yet. This is done mainly to get the
> > correct generated value for the register's RES0 mask.
>
> > ---
> >  arch/arm64/tools/sysreg | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
>
> This isn't a KVM specific patch, this is an arch patch - I'd have
> expected a subject line "arm64/sysreg:" or something...

Sorry. That was the intention, instead of removing the KVM for my
"KVM:arm64"s I removed the arm. Will fix it on the respin.

>
> >  Sysreg       HCRX_EL2        3       4       1       2       2
> > -Res0 63:23
> > +Res0 63:25
> > +Field        24      PACMEn
> > +Field        23      EnFPM
> >  Field        22      GCSEn
> >  Field        21      EnIDCP128
> >  Field        20      EnSDERR
>
> This is the same as my equivalent patch in the 2023 dpISA series:
>
>    https://lore.kernel.org/linux-arm-kernel/20231205-arm64-2023-dpisa-v3-7-dbcbcd867a7f@kernel.org/
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
>
> (that series does make use of EnFPM).  Given the likelyhood of overlaps
> like this I really think we should be merging these sysreg updates even
> if the serieses that depend on them are still in flight, it doesn't make
> sense for multiple people to end up doing the typing.

Makes sense. We'll ask one of the maintainers to pick up your updates.

Cheers,
/fuad

Re: [PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Fuad Tabba]

Hi Mark,

Thanks for your reviews!

On Thu, Dec 7, 2023 at 4:57 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:51AM +0000, Fuad Tabba wrote:
> > Add the latest field definitions to HCRX_EL2 from the 2023-09 Arm
> > Architecture System Registers xml specification [*].
> >
> > These fields aren't used yet. This is done mainly to get the
> > correct generated value for the register's RES0 mask.
>
> > ---
> >  arch/arm64/tools/sysreg | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
>
> This isn't a KVM specific patch, this is an arch patch - I'd have
> expected a subject line "arm64/sysreg:" or something...

Sorry. That was the intention, instead of removing the KVM for my
"KVM:arm64"s I removed the arm. Will fix it on the respin.

>
> >  Sysreg       HCRX_EL2        3       4       1       2       2
> > -Res0 63:23
> > +Res0 63:25
> > +Field        24      PACMEn
> > +Field        23      EnFPM
> >  Field        22      GCSEn
> >  Field        21      EnIDCP128
> >  Field        20      EnSDERR
>
> This is the same as my equivalent patch in the 2023 dpISA series:
>
>    https://lore.kernel.org/linux-arm-kernel/20231205-arm64-2023-dpisa-v3-7-dbcbcd867a7f@kernel.org/
>
> Reviewed-by: Mark Brown <broonie@kernel.org>
>
> (that series does make use of EnFPM).  Given the likelyhood of overlaps
> like this I really think we should be merging these sysreg updates even
> if the serieses that depend on them are still in flight, it doesn't make
> sense for multiple people to end up doing the typing.

Makes sense. We'll ask one of the maintainers to pick up your updates.

Cheers,
/fuad

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 02/12] KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt

[Fuad Tabba]

Hi Mark,

On Thu, Dec 7, 2023 at 5:06 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:52AM +0000, Fuad Tabba wrote:
>
> > +#define SYS_GCSCR_EL1                        sys_reg(3, 0, 2, 5, 0)
> > +#define SYS_GCSPR_EL1                        sys_reg(3, 0, 2, 5, 1)
> > +#define SYS_GCSCRE0_EL1                      sys_reg(3, 0, 2, 5, 2)
> > +
>
> > +#define SYS_GCSPR_EL0                        sys_reg(3, 3, 2, 5, 1)
> > +
>
> Unless there's some complication with representing them (mainly register
> layouts that aren't representable with the language or *very* repetitive
> blocks of registers) we should be adding any new sysregs to the sysreg
> file rather than manually encoding them.  We're trying to move things
> out of sysreg.h as much as possible.

I thought that we're still using the encoded ones if we're not using
the fields of the registers. That said, I'll move to the generated one
in the respin.

>
> For the above you can pick up the patch from my GCS series:
>
>   https://lore.kernel.org/linux-arm-kernel/20231122-arm64-gcs-v7-6-201c483bd775@kernel.org/
>
> > @@ -412,6 +423,8 @@
> >  #define SYS_PMUSERENR_EL0            sys_reg(3, 3, 9, 14, 0)
> >  #define SYS_PMOVSSET_EL0             sys_reg(3, 3, 9, 14, 3)
> >
> > +#define SYS_POR_EL0                  sys_reg(3, 3, 10, 2, 4)
> > +
>
> This is in Joey's POR series:
>
>   https://lore.kernel.org/linux-arm-kernel/20231124163510.1835740-2-joey.gouly@arm.com/
>
> Unfortunately I'm not aware of anyone having already done the rest.

I'll pick up what you and Joey have done and add the rest.

Thanks,
/fuad

Re: [PATCH v2 02/12] KVM: arm64: Add latest HFGxTR_EL2 FGT entries to nested virt

[Fuad Tabba]

Hi Mark,

On Thu, Dec 7, 2023 at 5:06 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:52AM +0000, Fuad Tabba wrote:
>
> > +#define SYS_GCSCR_EL1                        sys_reg(3, 0, 2, 5, 0)
> > +#define SYS_GCSPR_EL1                        sys_reg(3, 0, 2, 5, 1)
> > +#define SYS_GCSCRE0_EL1                      sys_reg(3, 0, 2, 5, 2)
> > +
>
> > +#define SYS_GCSPR_EL0                        sys_reg(3, 3, 2, 5, 1)
> > +
>
> Unless there's some complication with representing them (mainly register
> layouts that aren't representable with the language or *very* repetitive
> blocks of registers) we should be adding any new sysregs to the sysreg
> file rather than manually encoding them.  We're trying to move things
> out of sysreg.h as much as possible.

I thought that we're still using the encoded ones if we're not using
the fields of the registers. That said, I'll move to the generated one
in the respin.

>
> For the above you can pick up the patch from my GCS series:
>
>   https://lore.kernel.org/linux-arm-kernel/20231122-arm64-gcs-v7-6-201c483bd775@kernel.org/
>
> > @@ -412,6 +423,8 @@
> >  #define SYS_PMUSERENR_EL0            sys_reg(3, 3, 9, 14, 0)
> >  #define SYS_PMOVSSET_EL0             sys_reg(3, 3, 9, 14, 3)
> >
> > +#define SYS_POR_EL0                  sys_reg(3, 3, 10, 2, 4)
> > +
>
> This is in Joey's POR series:
>
>   https://lore.kernel.org/linux-arm-kernel/20231124163510.1835740-2-joey.gouly@arm.com/
>
> Unfortunately I'm not aware of anyone having already done the rest.

I'll pick up what you and Joey have done and add the rest.

Thanks,
/fuad

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 03/12] KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt

[Fuad Tabba]

Hi Mark,

On Thu, Dec 7, 2023 at 5:14 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:53AM +0000, Fuad Tabba wrote:
>
> > Add the missing nested virt FGT table entries HFGITR_EL2. Based
> > on the 2023-09 Arm Architecture System Registers xml
> > specification [*]. Add the missing field definitions as well,
> > both to generate the correct RES0 mask and to be able to toggle
> > their FGT bits.
> >
> > Also adds definitions of some of the missing system registers and
> > instructions, which can be trapped by the FGT bits.
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> That should be a reference to DDI0602 (for the instruction XML) and
> DD0601 (for the system register XML) - it's the same content, just a
> more stable name.

Got it. Will fix it on the respin.

Cheers,
/fuad

>
> >  Sysreg HFGITR_EL2    3       4       1       1       6
> > -Res0 63:61
> > +Res0 63
> > +Field        62      ATS1E1A
> > +Res0 61
> >  Field        60      COSPRCTX
> >  Field        59      nGCSEPP
> >  Field        58      nGCSSTR_EL1
>
> The sysreg looks good:
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Re: [PATCH v2 03/12] KVM: arm64: Add latest HFGITR_EL2 FGT entries to nested virt

[Fuad Tabba]

Hi Mark,

On Thu, Dec 7, 2023 at 5:14 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:53AM +0000, Fuad Tabba wrote:
>
> > Add the missing nested virt FGT table entries HFGITR_EL2. Based
> > on the 2023-09 Arm Architecture System Registers xml
> > specification [*]. Add the missing field definitions as well,
> > both to generate the correct RES0 mask and to be able to toggle
> > their FGT bits.
> >
> > Also adds definitions of some of the missing system registers and
> > instructions, which can be trapped by the FGT bits.
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> That should be a reference to DDI0602 (for the instruction XML) and
> DD0601 (for the system register XML) - it's the same content, just a
> more stable name.

Got it. Will fix it on the respin.

Cheers,
/fuad

>
> >  Sysreg HFGITR_EL2    3       4       1       1       6
> > -Res0 63:61
> > +Res0 63
> > +Field        62      ATS1E1A
> > +Res0 61
> >  Field        60      COSPRCTX
> >  Field        59      nGCSEPP
> >  Field        58      nGCSSTR_EL1
>
> The sysreg looks good:
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 04/12] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Fuad Tabba]

Hi,

On Thu, Dec 7, 2023 at 5:19 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:54AM +0000, Fuad Tabba wrote:
> > To support HAFGRTR_EL2 supported in nested virt in the following
> > patch, first add its bitmask definitions based on the 2023-09
> > Arm Architecture System Registers xml specification [*].
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> DDI0601 is a more stable name, and I'd have expected an "arm64/sysreg"
> subject.

Got it.

Cheers,
/fuad

>
> > +#define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > +#define __HAFGRTR_EL2_MASK   (GENMASK(49, 17) | GENMASK(4, 0))
> > +#define __HAFGRTR_EL2_nMASK  0UL
> > +
>
> This one is an example of something where some manual encoding does make
> sense given all the AMCNTEN<x> and so on fields so:
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

Re: [PATCH v2 04/12] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Fuad Tabba]

Hi,

On Thu, Dec 7, 2023 at 5:19 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:54AM +0000, Fuad Tabba wrote:
> > To support HAFGRTR_EL2 supported in nested virt in the following
> > patch, first add its bitmask definitions based on the 2023-09
> > Arm Architecture System Registers xml specification [*].
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> DDI0601 is a more stable name, and I'd have expected an "arm64/sysreg"
> subject.

Got it.

Cheers,
/fuad

>
> > +#define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > +#define __HAFGRTR_EL2_MASK   (GENMASK(49, 17) | GENMASK(4, 0))
> > +#define __HAFGRTR_EL2_nMASK  0UL
> > +
>
> This one is an example of something where some manual encoding does make
> sense given all the AMCNTEN<x> and so on fields so:
>
> Reviewed-by: Mark Brown <broonie@kernel.org>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Fuad Tabba]

Hi,

On Thu, Dec 7, 2023 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:55AM +0000, Fuad Tabba wrote:
>
> > Add the encodings to fine grain trapping fields for HAFGRTR_EL2
> > and add the associated handling code in nested virt. Based on
> > the 2023-09 Arm Architecture System Registers xml
> > specification [*]. Add the missing field definitions as well,
> > both to generate the correct RES0 mask and to be able to toggle
> > their FGT bits.
>
> > Also add the code for handling FGT trapping, reading of the
> > register, to nested virt.
>
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> DDI0601.

Yup! :)

> > +Sysreg HAFGRTR_EL2   3       4       3       1       6
> > +Res0 63:50
> > +Field        49      AMEVTYPER115_EL0
> > +Field        48      AMEVCNTR115_EL0
> > +Field        47      AMEVTYPER114_EL0
> > +Field        46      AMEVCNTR114_EL0
> > +Field        45      AMEVTYPER113_EL0
> > +Field        44      AMEVCNTR113_EL0
> > +Field        43      AMEVTYPER112_EL0
> > +Field        42      AMEVCNTR112_EL0
> > +Field        41      AMEVTYPER111_EL0
> > +Field        40      AMEVCNTR111_EL0
> > +Field        39      AMEVTYPER110_EL0
> > +Field        38      AMEVCNTR110_EL0
> > +Field        37      AMEVTYPER19_EL0
> > +Field        36      AMEVCNTR19_EL0
> > +Field        35      AMEVTYPER18_EL0
> > +Field        34      AMEVCNTR18_EL0
> > +Field        33      AMEVTYPER17_EL0
> > +Field        32      AMEVCNTR17_EL0
> > +Field        31      AMEVTYPER16_EL0
> > +Field        30      AMEVCNTR16_EL0
> > +Field        29      AMEVTYPER15_EL0
> > +Field        28      AMEVCNTR15_EL0
> > +Field        27      AMEVTYPER14_EL0
> > +Field        26      AMEVCNTR14_EL0
> > +Field        25      AMEVTYPER13_EL0
> > +Field        24      AMEVCNTR13_EL0
> > +Field        23      AMEVTYPER12_EL0
> > +Field        22      AMEVCNTR12_EL0
> > +Field        21      AMEVTYPER11_EL0
> > +Field        20      AMEVCNTR11_EL0
> > +Field        19      AMEVTYPER10_EL0
> > +Field        18      AMEVCNTR10_EL0
> > +Field        17      AMCNTEN1
> > +Res0 16:5
> > +Field        4       AMEVCNTR03_EL0
> > +Field        3       AMEVCNTR02_EL0
> > +Field        2       AMEVCNTR01_EL0
> > +Field        1       AMEVCNTR00_EL0
> > +Field        0       AMCNTEN0
> > +EndSysreg
> > +
>
> Oh, we actually have the register fully encoded since we needed the
> fields.  Given this why did we need the manual encoding in the previous
> patch?  In any case this looks good according to DDI0601 2023-09:

We need both because we need to know the polarity. RES0 and the valid
bits are generated. In later patches in this series I move completely
to using the generated RES0, and to calculating nMASK based on the
other fields. But we still need the definitions in kvm_arm.h for the
polarity.

> Reviewed-by: Mark Brown <broonie@kernel.org>

Thanks!
/fuad

Re: [PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Fuad Tabba]

Hi,

On Thu, Dec 7, 2023 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:
>
> On Wed, Dec 06, 2023 at 10:04:55AM +0000, Fuad Tabba wrote:
>
> > Add the encodings to fine grain trapping fields for HAFGRTR_EL2
> > and add the associated handling code in nested virt. Based on
> > the 2023-09 Arm Architecture System Registers xml
> > specification [*]. Add the missing field definitions as well,
> > both to generate the correct RES0 mask and to be able to toggle
> > their FGT bits.
>
> > Also add the code for handling FGT trapping, reading of the
> > register, to nested virt.
>
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> DDI0601.

Yup! :)

> > +Sysreg HAFGRTR_EL2   3       4       3       1       6
> > +Res0 63:50
> > +Field        49      AMEVTYPER115_EL0
> > +Field        48      AMEVCNTR115_EL0
> > +Field        47      AMEVTYPER114_EL0
> > +Field        46      AMEVCNTR114_EL0
> > +Field        45      AMEVTYPER113_EL0
> > +Field        44      AMEVCNTR113_EL0
> > +Field        43      AMEVTYPER112_EL0
> > +Field        42      AMEVCNTR112_EL0
> > +Field        41      AMEVTYPER111_EL0
> > +Field        40      AMEVCNTR111_EL0
> > +Field        39      AMEVTYPER110_EL0
> > +Field        38      AMEVCNTR110_EL0
> > +Field        37      AMEVTYPER19_EL0
> > +Field        36      AMEVCNTR19_EL0
> > +Field        35      AMEVTYPER18_EL0
> > +Field        34      AMEVCNTR18_EL0
> > +Field        33      AMEVTYPER17_EL0
> > +Field        32      AMEVCNTR17_EL0
> > +Field        31      AMEVTYPER16_EL0
> > +Field        30      AMEVCNTR16_EL0
> > +Field        29      AMEVTYPER15_EL0
> > +Field        28      AMEVCNTR15_EL0
> > +Field        27      AMEVTYPER14_EL0
> > +Field        26      AMEVCNTR14_EL0
> > +Field        25      AMEVTYPER13_EL0
> > +Field        24      AMEVCNTR13_EL0
> > +Field        23      AMEVTYPER12_EL0
> > +Field        22      AMEVCNTR12_EL0
> > +Field        21      AMEVTYPER11_EL0
> > +Field        20      AMEVCNTR11_EL0
> > +Field        19      AMEVTYPER10_EL0
> > +Field        18      AMEVCNTR10_EL0
> > +Field        17      AMCNTEN1
> > +Res0 16:5
> > +Field        4       AMEVCNTR03_EL0
> > +Field        3       AMEVCNTR02_EL0
> > +Field        2       AMEVCNTR01_EL0
> > +Field        1       AMEVCNTR00_EL0
> > +Field        0       AMCNTEN0
> > +EndSysreg
> > +
>
> Oh, we actually have the register fully encoded since we needed the
> fields.  Given this why did we need the manual encoding in the previous
> patch?  In any case this looks good according to DDI0601 2023-09:

We need both because we need to know the polarity. RES0 and the valid
bits are generated. In later patches in this series I move completely
to using the generated RES0, and to calculating nMASK based on the
other fields. But we still need the definitions in kvm_arm.h for the
polarity.

> Reviewed-by: Mark Brown <broonie@kernel.org>

Thanks!
/fuad

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

Hi Joey,

On Thu, Dec 7, 2023 at 5:12 PM Joey Gouly <joey.gouly@arm.com> wrote:
>
> On Thu, Dec 07, 2023 at 03:06:44PM +0000, Fuad Tabba wrote:
> > Hi Joey,
> >
> > On Thu, Dec 7, 2023 at 3:00 PM Joey Gouly <joey.gouly@arm.com> wrote:
> > >
> > > Hello Fuad,
> > >
> > > On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> > > > New trap bits have been defined in the 2023-09 Arm Architecture
> > > > System Registers xml specification [*]. Moreover, the existing
> > > > definitions of some of the mask and the RES0 bits overlap, which
> > > > could be wrong, confusing, or both.
> > > >
> > > > Update the bits to represent the latest spec (as of this patch,
> > > > 2023-09), and ensure that the existing bits are consistent.
> > > >
> > > > Subsequent patches will use the generated RES0 fields instead of
> > > > specifying them manually. This patch keeps the manual encoding of
> > > > the bits to make it easier to review the series.
> > > >
> > > > [*] https://developer.arm.com/downloads/-/exploration-tools
> > > >
> > > > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > > ---
> > > >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> > > >  1 file changed, 24 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > > > index 7de0a7062625..b0dc3249d5cd 100644
> > > > --- a/arch/arm64/include/asm/kvm_arm.h
> > > > +++ b/arch/arm64/include/asm/kvm_arm.h
> > > > @@ -344,30 +344,39 @@
> > > >   * Once we get to a point where the two describe the same thing, we'll
> > > >   * merge the definitions. One day.
> > > >   */
> > > > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > > > +#define __HFGRTR_EL2_RES0    BIT(51)
> > > >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > > > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> > > >
> > > > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > > > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > > > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > > > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > > > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> > > >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > > > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > > > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > > > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > > > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > > > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > > > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > > > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> > >
> > > By adding all these bits to *_nMASK, we're allowing a guest to access registers
> > > which KVM doesn't (currently) deal with.  For example if I apply this patch
> > > series, a guest can access S2POR_EL1, previously it would print something like:
> > >
> > >         kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]
> > >          { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read },
> > >
> > > After applying this patch series, the guest can read S2POR_EL1.
> > >
> > > We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
> > > guest shouldn't access it, but there's nothing stopping it.
> > >
> > > My question is, is this intended? Or do we need to update the following code
> > > (and comment!) to trap all the stuff we don't currently handle (along with
> > > ACCDATA_EL1):
> >
> > Thanks for pointing this out, and no, it's not intended. For
> > consistency, I think it might be good to update the code, as you
> > suggest, to prevent these accesses. If you and the other reviewers are
> > happy with that solution I can respin with your suggested fix. I have
> > a couple of other minor fixes as well for the series.
> >
>
> Thanks for clarifying that.
>
> To me it seems weird to use the generated bits directly, and I think that's why
> the code seems confusing?
>
> I think we should stop using __HFGRTR_EL2_nMASK directly in that function, and
> build it up from '0'. This also matches, I think, with Marc Z wanting to move
> towards setting these things based on features [1], so maybe something like:
>
>         u64 r_val = 0;
>
>         if (cpus_have_final_cap(ARM64_HAS_S1PIE))
>                 r_set |= HFGxTR_EL2_nPIR_EL1 | HFGxTR_EL2_nPIREO_EL1;
>
>         r_val |= r_set;
>         r_val &= ~r_clr;
>
>         write_sysreg_s(r_val, SYS_HFGRTR_EL2);
>
> That way we just explicitly set which bits we don't want to trap on. We have
> some weird behaviour right now were we set nSMPRI_EL1 to 1, unless you have
> FEAT_SME where we set it to 0.. but if you don't have FEAT_SME it's RES0
> anyway? The above approach would fix that.

I don't have a strong opinion about that, but I am inclined to agree
with you. Let's also hear what Marc Zygier (since he's the author of
that part) opinion is, and I'm happy either way.

Cheers,
/fuad

> Thanks,
> Joey
>
> [1] https://lore.kernel.org/linux-arm-kernel/87bkb285ud.wl-maz@kernel.org/ (last paragraph)
>
> >
> > Cheers,
> > /fuad
> >
> >
> > >
> > >         static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
> > >         {
> > >
> > >                 ..
> > >
> > >                 /* The default is not to trap anything but ACCDATA_EL1 */
> > >                 r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
> > >                 r_val |= r_set;
> > >                 r_val &= ~r_clr;
> > >
> > >
> > > Thanks,
> > > Joey
> > >
> > > >
> > > > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > > > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > > > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > > > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > > > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > > > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> > > >
> > > >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> > > >                                GENMASK(21, 20) | BIT(8))
> > > > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > > > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > > > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > > > +                              GENMASK(19, 9) | GENMASK(7, 0))
> > > >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> > > >
> > > >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> > > >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> > > >                                BIT(22) | BIT(9) | BIT(6))
> > > > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > > > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > > > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > > > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > > > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > > > +                              GENMASK(8, 7) | GENMASK(5, 0))
> > > >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> > > >
> > > >  #define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > > > @@ -375,9 +384,9 @@
> > > >  #define __HAFGRTR_EL2_nMASK  0UL
> > > >
> > > >  /* Similar definitions for HCRX_EL2 */
> > > > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > > > -#define __HCRX_EL2_MASK              (0)
> > > > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > > > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > > > +#define __HCRX_EL2_MASK              (BIT(6))
> > > > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> > > >
> > > >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> > > >  #define HPFAR_MASK   (~UL(0xf))
> > > > --
> > > > 2.43.0.rc2.451.g8631bc7472-goog
> > > >
> > > >

Re: [PATCH v2 06/12] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

Hi Joey,

On Thu, Dec 7, 2023 at 5:12 PM Joey Gouly <joey.gouly@arm.com> wrote:
>
> On Thu, Dec 07, 2023 at 03:06:44PM +0000, Fuad Tabba wrote:
> > Hi Joey,
> >
> > On Thu, Dec 7, 2023 at 3:00 PM Joey Gouly <joey.gouly@arm.com> wrote:
> > >
> > > Hello Fuad,
> > >
> > > On Wed, Dec 06, 2023 at 10:04:56AM +0000, Fuad Tabba wrote:
> > > > New trap bits have been defined in the 2023-09 Arm Architecture
> > > > System Registers xml specification [*]. Moreover, the existing
> > > > definitions of some of the mask and the RES0 bits overlap, which
> > > > could be wrong, confusing, or both.
> > > >
> > > > Update the bits to represent the latest spec (as of this patch,
> > > > 2023-09), and ensure that the existing bits are consistent.
> > > >
> > > > Subsequent patches will use the generated RES0 fields instead of
> > > > specifying them manually. This patch keeps the manual encoding of
> > > > the bits to make it easier to review the series.
> > > >
> > > > [*] https://developer.arm.com/downloads/-/exploration-tools
> > > >
> > > > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > > > Signed-off-by: Fuad Tabba <tabba@google.com>
> > > > ---
> > > >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> > > >  1 file changed, 24 insertions(+), 15 deletions(-)
> > > >
> > > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > > > index 7de0a7062625..b0dc3249d5cd 100644
> > > > --- a/arch/arm64/include/asm/kvm_arm.h
> > > > +++ b/arch/arm64/include/asm/kvm_arm.h
> > > > @@ -344,30 +344,39 @@
> > > >   * Once we get to a point where the two describe the same thing, we'll
> > > >   * merge the definitions. One day.
> > > >   */
> > > > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > > > +#define __HFGRTR_EL2_RES0    BIT(51)
> > > >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > > > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> > > >
> > > > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > > > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > > > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > > > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > > > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> > > >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > > > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > > > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > > > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > > > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > > > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > > > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > > > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > > > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> > >
> > > By adding all these bits to *_nMASK, we're allowing a guest to access registers
> > > which KVM doesn't (currently) deal with.  For example if I apply this patch
> > > series, a guest can access S2POR_EL1, previously it would print something like:
> > >
> > >         kvm [80]: Unsupported guest sys_reg access at: ffffc42969c1f270 [600000c5]
> > >          { Op0( 3), Op1( 0), CRn(10), CRm( 2), Op2( 5), func_read },
> > >
> > > After applying this patch series, the guest can read S2POR_EL1.
> > >
> > > We don't expose S2POE to the guest through ID_AA64MMFR3_EL1, so a well behaved
> > > guest shouldn't access it, but there's nothing stopping it.
> > >
> > > My question is, is this intended? Or do we need to update the following code
> > > (and comment!) to trap all the stuff we don't currently handle (along with
> > > ACCDATA_EL1):
> >
> > Thanks for pointing this out, and no, it's not intended. For
> > consistency, I think it might be good to update the code, as you
> > suggest, to prevent these accesses. If you and the other reviewers are
> > happy with that solution I can respin with your suggested fix. I have
> > a couple of other minor fixes as well for the series.
> >
>
> Thanks for clarifying that.
>
> To me it seems weird to use the generated bits directly, and I think that's why
> the code seems confusing?
>
> I think we should stop using __HFGRTR_EL2_nMASK directly in that function, and
> build it up from '0'. This also matches, I think, with Marc Z wanting to move
> towards setting these things based on features [1], so maybe something like:
>
>         u64 r_val = 0;
>
>         if (cpus_have_final_cap(ARM64_HAS_S1PIE))
>                 r_set |= HFGxTR_EL2_nPIR_EL1 | HFGxTR_EL2_nPIREO_EL1;
>
>         r_val |= r_set;
>         r_val &= ~r_clr;
>
>         write_sysreg_s(r_val, SYS_HFGRTR_EL2);
>
> That way we just explicitly set which bits we don't want to trap on. We have
> some weird behaviour right now were we set nSMPRI_EL1 to 1, unless you have
> FEAT_SME where we set it to 0.. but if you don't have FEAT_SME it's RES0
> anyway? The above approach would fix that.

I don't have a strong opinion about that, but I am inclined to agree
with you. Let's also hear what Marc Zygier (since he's the author of
that part) opinion is, and I'm happy either way.

Cheers,
/fuad

> Thanks,
> Joey
>
> [1] https://lore.kernel.org/linux-arm-kernel/87bkb285ud.wl-maz@kernel.org/ (last paragraph)
>
> >
> > Cheers,
> > /fuad
> >
> >
> > >
> > >         static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
> > >         {
> > >
> > >                 ..
> > >
> > >                 /* The default is not to trap anything but ACCDATA_EL1 */
> > >                 r_val = __HFGRTR_EL2_nMASK & ~HFGxTR_EL2_nACCDATA_EL1;
> > >                 r_val |= r_set;
> > >                 r_val &= ~r_clr;
> > >
> > >
> > > Thanks,
> > > Joey
> > >
> > > >
> > > > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > > > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > > > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > > > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > > > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > > > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> > > >
> > > >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> > > >                                GENMASK(21, 20) | BIT(8))
> > > > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > > > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > > > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > > > +                              GENMASK(19, 9) | GENMASK(7, 0))
> > > >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> > > >
> > > >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> > > >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> > > >                                BIT(22) | BIT(9) | BIT(6))
> > > > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > > > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > > > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > > > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > > > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > > > +                              GENMASK(8, 7) | GENMASK(5, 0))
> > > >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> > > >
> > > >  #define __HAFGRTR_EL2_RES0   (GENMASK(63, 50) | GENMASK(16, 5))
> > > > @@ -375,9 +384,9 @@
> > > >  #define __HAFGRTR_EL2_nMASK  0UL
> > > >
> > > >  /* Similar definitions for HCRX_EL2 */
> > > > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > > > -#define __HCRX_EL2_MASK              (0)
> > > > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > > > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > > > +#define __HCRX_EL2_MASK              (BIT(6))
> > > > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> > > >
> > > >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> > > >  #define HPFAR_MASK   (~UL(0xf))
> > > > --
> > > > 2.43.0.rc2.451.g8631bc7472-goog
> > > >
> > > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Mark Brown]

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

On Fri, Dec 08, 2023 at 08:14:58AM +0000, Fuad Tabba wrote:
> On Thu, Dec 7, 2023 at 4:57 PM Mark Brown <broonie@kernel.org> wrote:

> > (that series does make use of EnFPM).  Given the likelyhood of overlaps
> > like this I really think we should be merging these sysreg updates even
> > if the serieses that depend on them are still in flight, it doesn't make
> > sense for multiple people to end up doing the typing.

> Makes sense. We'll ask one of the maintainers to pick up your updates.

Thanks.  I'm in the middle of pulling a series together which gathers
all the sysreg additions I'm aware of together which are ready
(including yours from this series) - should be out later today unless
something unexpected happens in my CI, hopefully one way or another
everything will get pulled in.  I'll Cc you when I post.

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

Re: [PATCH v2 01/12] KVM: Add missing HCRX_EL2 field definitions

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 840 bytes --]

On Fri, Dec 08, 2023 at 08:14:58AM +0000, Fuad Tabba wrote:
> On Thu, Dec 7, 2023 at 4:57 PM Mark Brown <broonie@kernel.org> wrote:

> > (that series does make use of EnFPM).  Given the likelyhood of overlaps
> > like this I really think we should be merging these sysreg updates even
> > if the serieses that depend on them are still in flight, it doesn't make
> > sense for multiple people to end up doing the typing.

> Makes sense. We'll ask one of the maintainers to pick up your updates.

Thanks.  I'm in the middle of pulling a series together which gathers
all the sysreg additions I'm aware of together which are ready
(including yours from this series) - should be out later today unless
something unexpected happens in my CI, hopefully one way or another
everything will get pulled in.  I'll Cc you when I post.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Mark Brown]

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

On Fri, Dec 08, 2023 at 08:19:43AM +0000, Fuad Tabba wrote:
> On Thu, Dec 7, 2023 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:

> > > +Sysreg HAFGRTR_EL2   3       4       3       1       6
> > > +Res0 63:50

> > Oh, we actually have the register fully encoded since we needed the
> > fields.  Given this why did we need the manual encoding in the previous
> > patch?  In any case this looks good according to DDI0601 2023-09:

> We need both because we need to know the polarity. RES0 and the valid
> bits are generated. In later patches in this series I move completely
> to using the generated RES0, and to calculating nMASK based on the
> other fields. But we still need the definitions in kvm_arm.h for the
> polarity.

Ah, I see - that makes sense.  We could consider extending the language
to capture this information in the sysreg definition rather than typing
it in separately, though it's not going to get that much usage.

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

Re: [PATCH v2 05/12] KVM: arm64: Handle HAFGRTR_EL2 trapping in nested virt

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 959 bytes --]

On Fri, Dec 08, 2023 at 08:19:43AM +0000, Fuad Tabba wrote:
> On Thu, Dec 7, 2023 at 5:28 PM Mark Brown <broonie@kernel.org> wrote:

> > > +Sysreg HAFGRTR_EL2   3       4       3       1       6
> > > +Res0 63:50

> > Oh, we actually have the register fully encoded since we needed the
> > fields.  Given this why did we need the manual encoding in the previous
> > patch?  In any case this looks good according to DDI0601 2023-09:

> We need both because we need to know the polarity. RES0 and the valid
> bits are generated. In later patches in this series I move completely
> to using the generated RES0, and to calculating nMASK based on the
> other fields. But we still need the definitions in kvm_arm.h for the
> polarity.

Ah, I see - that makes sense.  We could consider extending the language
to capture this information in the sysreg definition rather than typing
it in separately, though it's not going to get that much usage.

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

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel