[PATCH v2 0/2] Fine grain sysregs allowed to trap for nested virtualization

[PATCH v2 0/2] Fine grain sysregs allowed to trap for nested virtualization

[Miguel Luis]

The current HCR_EL2 description includes _EL1 registers that are not affected
by NV. Let's exclude them from those ranges to implement a more fine grained
approach.

Changes v1 -> v2
 patch 1:
	fix indentation
 patch 2:
	improve commit message (Marc)
	fix indentation (Marc)
	follow kernel comment format (Marc)
	describe LRs in ranges (Marc)
	include AMEVCNTVOFF0<n>_EL2 and AMEVCNTVOFF1<n>_EL2
 patch 3:
	drop. Excluded IMPDEF range is trapped by HCR_EL2.TIDCP

v1: https://lore.kernel.org/kvmarm/20230913185209.32282-1-miguel.luis@oracle.com/

Miguel Luis (2):
  arm64: Add missing _EL12 encodings
  arm64/kvm: Fine grain _EL2 system registers list that affect nested
    virtualization

 arch/arm64/include/asm/sysreg.h | 11 +++++++++
 arch/arm64/kvm/emulate-nested.c | 44 ++++++++++++++++++++++++++++-----
 2 files changed, 49 insertions(+), 6 deletions(-)

--
2.39.2


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

[PATCH v2 1/2] arm64: Add missing _EL12 encodings

[Miguel Luis]

Some _EL12 encodings are missing. Add them.

Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 arch/arm64/include/asm/sysreg.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 38296579a4fd..6e167bbf44ff 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -568,18 +568,29 @@
 
 /* VHE encodings for architectural EL0/1 system registers */
 #define SYS_SCTLR_EL12			sys_reg(3, 5, 1, 0, 0)
+#define SYS_CPACR_EL12			sys_reg(3, 5, 1, 0, 2)
+#define SYS_SCTLR2_EL12			sys_reg(3, 5, 1, 0, 3)
+#define SYS_ZCR_EL12			sys_reg(3, 5, 1, 2, 0)
+#define SYS_TRFCR_EL12			sys_reg(3, 5, 1, 2, 1)
+#define SYS_SMCR_EL12			sys_reg(3, 5, 1, 2, 6)
 #define SYS_TTBR0_EL12			sys_reg(3, 5, 2, 0, 0)
 #define SYS_TTBR1_EL12			sys_reg(3, 5, 2, 0, 1)
 #define SYS_TCR_EL12			sys_reg(3, 5, 2, 0, 2)
+#define SYS_TCR2_EL12			sys_reg(3, 5, 2, 0, 3)
 #define SYS_SPSR_EL12			sys_reg(3, 5, 4, 0, 0)
 #define SYS_ELR_EL12			sys_reg(3, 5, 4, 0, 1)
 #define SYS_AFSR0_EL12			sys_reg(3, 5, 5, 1, 0)
 #define SYS_AFSR1_EL12			sys_reg(3, 5, 5, 1, 1)
 #define SYS_ESR_EL12			sys_reg(3, 5, 5, 2, 0)
 #define SYS_TFSR_EL12			sys_reg(3, 5, 5, 6, 0)
+#define SYS_FAR_EL12			sys_reg(3, 5, 6, 0, 0)
+#define SYS_BRBCR_EL12			sys_reg(3, 5, 9, 0, 0)
+#define SYS_PMSCR_EL12			sys_reg(3, 5, 9, 9, 0)
 #define SYS_MAIR_EL12			sys_reg(3, 5, 10, 2, 0)
 #define SYS_AMAIR_EL12			sys_reg(3, 5, 10, 3, 0)
 #define SYS_VBAR_EL12			sys_reg(3, 5, 12, 0, 0)
+#define SYS_CONTEXTIDR_EL12		sys_reg(3, 5, 13, 0, 1)
+#define SYS_SCXTNUM_EL12		sys_reg(3, 5, 13, 0, 7)
 #define SYS_CNTKCTL_EL12		sys_reg(3, 5, 14, 1, 0)
 #define SYS_CNTP_TVAL_EL02		sys_reg(3, 5, 14, 2, 0)
 #define SYS_CNTP_CTL_EL02		sys_reg(3, 5, 14, 2, 1)
-- 
2.39.2


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

[PATCH v2 2/2] arm64/kvm: Fine grain _EL2 system registers list that affect nested virtualization

[Miguel Luis]

Some _EL1 registers got included in the _EL2 ranges, which are not
affected by NV. Remove them, fine grain the ranges to exclusively
include the _EL2 ones and fold SPSR/ELR _EL2 registers into the
existing range.

Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
---
 arch/arm64/kvm/emulate-nested.c | 44 ++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 9ced1bf0c2b7..f6d0c87803f4 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -649,14 +649,46 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
 	SR_TRAP(SYS_APGAKEYHI_EL1,	CGT_HCR_APK),
 	/* All _EL2 registers */
 	SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
-		      sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
+		      sys_reg(3, 4, 4, 0, 1), CGT_HCR_NV),
 	/* Skip the SP_EL1 encoding... */
-	SR_TRAP(SYS_SPSR_EL2,		CGT_HCR_NV),
-	SR_TRAP(SYS_ELR_EL2,		CGT_HCR_NV),
-	SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
-		      sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
+	SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
+		      sys_reg(3, 4, 10, 6, 7), CGT_HCR_NV),
+	/*
+	 * Note that the spec. describes a group of MEC registers
+	 * whose access should not trap, therefore skip the following:
+	 * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
+	 * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
+	 * VMECID_P_EL2.
+	 */
 	SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
-		      sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
+		      sys_reg(3, 4, 12, 1, 1), CGT_HCR_NV),
+	/* ICH_AP0R<m>_EL2 */
+	SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
+		      SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
+	/* ICH_AP1R<m>_EL2 */
+	SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
+		      SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
+	SR_RANGE_TRAP(sys_reg(3, 4, 12, 9, 5),
+		      sys_reg(3, 4, 12, 11, 7), CGT_HCR_NV),
+	/* ICH_LR<m>_EL2 */
+	SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
+		      SYS_ICH_LR7_EL2, CGT_HCR_NV),
+	SR_RANGE_TRAP(SYS_ICH_LR8_EL2,
+		      SYS_ICH_LR15_EL2, CGT_HCR_NV),
+	SR_RANGE_TRAP(sys_reg(3, 4, 13, 0, 1),
+		      sys_reg(3, 4, 13, 0, 7), CGT_HCR_NV),
+	/* AMEVCNTVOFF0<n>_EL2 */
+	SR_RANGE_TRAP(sys_reg(3, 4, 13, 8, 0),
+		      sys_reg(3, 4, 13, 8, 7), CGT_HCR_NV),
+	SR_RANGE_TRAP(sys_reg(3, 4, 13, 9, 0),
+		      sys_reg(3, 4, 13, 9, 7), CGT_HCR_NV),
+	/* AMEVCNTVOFF1<n>_EL2 */
+	SR_RANGE_TRAP(sys_reg(3, 4, 13, 10, 0),
+		      sys_reg(3, 4, 13, 10, 7), CGT_HCR_NV),
+	SR_RANGE_TRAP(sys_reg(3, 4, 13, 11, 0),
+		      sys_reg(3, 4, 13, 11, 7), CGT_HCR_NV),
+	SR_RANGE_TRAP(sys_reg(3, 4, 14, 0, 3),
+		      sys_reg(3, 4, 14, 5, 2), CGT_HCR_NV),
 	/* All _EL02, _EL12 registers */
 	SR_RANGE_TRAP(sys_reg(3, 5, 0, 0, 0),
 		      sys_reg(3, 5, 10, 15, 7), CGT_HCR_NV),
-- 
2.39.2


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

Re: [PATCH v2 1/2] arm64: Add missing _EL12 encodings

[Eric Auger]

Hi Miguel,
On 9/25/23 18:20, Miguel Luis wrote:
> Some _EL12 encodings are missing. Add them.
> 
> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> ---
>  arch/arm64/include/asm/sysreg.h | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 38296579a4fd..6e167bbf44ff 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -568,18 +568,29 @@
>  
>  /* VHE encodings for architectural EL0/1 system registers */
>  #define SYS_SCTLR_EL12			sys_reg(3, 5, 1, 0, 0)
> +#define SYS_CPACR_EL12			sys_reg(3, 5, 1, 0, 2)
> +#define SYS_SCTLR2_EL12			sys_reg(3, 5, 1, 0, 3)
> +#define SYS_ZCR_EL12			sys_reg(3, 5, 1, 2, 0)
> +#define SYS_TRFCR_EL12			sys_reg(3, 5, 1, 2, 1)
> +#define SYS_SMCR_EL12			sys_reg(3, 5, 1, 2, 6)
>  #define SYS_TTBR0_EL12			sys_reg(3, 5, 2, 0, 0)
>  #define SYS_TTBR1_EL12			sys_reg(3, 5, 2, 0, 1)
>  #define SYS_TCR_EL12			sys_reg(3, 5, 2, 0, 2)
> +#define SYS_TCR2_EL12			sys_reg(3, 5, 2, 0, 3)
>  #define SYS_SPSR_EL12			sys_reg(3, 5, 4, 0, 0)
>  #define SYS_ELR_EL12			sys_reg(3, 5, 4, 0, 1)
>  #define SYS_AFSR0_EL12			sys_reg(3, 5, 5, 1, 0)
>  #define SYS_AFSR1_EL12			sys_reg(3, 5, 5, 1, 1)
>  #define SYS_ESR_EL12			sys_reg(3, 5, 5, 2, 0)
>  #define SYS_TFSR_EL12			sys_reg(3, 5, 5, 6, 0)
> +#define SYS_FAR_EL12			sys_reg(3, 5, 6, 0, 0)
> +#define SYS_BRBCR_EL12			sys_reg(3, 5, 9, 0, 0)
isn't it sys_reg(2, 5, 9, 0, 0)?

> +#define SYS_PMSCR_EL12			sys_reg(3, 5, 9, 9, 0)

>  #define SYS_MAIR_EL12			sys_reg(3, 5, 10, 2, 0)
>  #define SYS_AMAIR_EL12			sys_reg(3, 5, 10, 3, 0)
>  #define SYS_VBAR_EL12			sys_reg(3, 5, 12, 0, 0)
> +#define SYS_CONTEXTIDR_EL12		sys_reg(3, 5, 13, 0, 1)
> +#define SYS_SCXTNUM_EL12		sys_reg(3, 5, 13, 0, 7)
>  #define SYS_CNTKCTL_EL12		sys_reg(3, 5, 14, 1, 0)
>  #define SYS_CNTP_TVAL_EL02		sys_reg(3, 5, 14, 2, 0)
>  #define SYS_CNTP_CTL_EL02		sys_reg(3, 5, 14, 2, 1)
Besides
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric


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

Re: [PATCH v2 1/2] arm64: Add missing _EL12 encodings

[Miguel Luis]

Hi Eric,

> On 28 Sep 2023, at 09:39, Eric Auger <eauger@redhat.com> wrote:
> 
> Hi Miguel,
> On 9/25/23 18:20, Miguel Luis wrote:
>> Some _EL12 encodings are missing. Add them.
>> 
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
>> ---
>> arch/arm64/include/asm/sysreg.h | 11 +++++++++++
>> 1 file changed, 11 insertions(+)
>> 
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index 38296579a4fd..6e167bbf44ff 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -568,18 +568,29 @@
>> 
>> /* VHE encodings for architectural EL0/1 system registers */
>> #define SYS_SCTLR_EL12 sys_reg(3, 5, 1, 0, 0)
>> +#define SYS_CPACR_EL12 sys_reg(3, 5, 1, 0, 2)
>> +#define SYS_SCTLR2_EL12 sys_reg(3, 5, 1, 0, 3)
>> +#define SYS_ZCR_EL12 sys_reg(3, 5, 1, 2, 0)
>> +#define SYS_TRFCR_EL12 sys_reg(3, 5, 1, 2, 1)
>> +#define SYS_SMCR_EL12 sys_reg(3, 5, 1, 2, 6)
>> #define SYS_TTBR0_EL12 sys_reg(3, 5, 2, 0, 0)
>> #define SYS_TTBR1_EL12 sys_reg(3, 5, 2, 0, 1)
>> #define SYS_TCR_EL12 sys_reg(3, 5, 2, 0, 2)
>> +#define SYS_TCR2_EL12 sys_reg(3, 5, 2, 0, 3)
>> #define SYS_SPSR_EL12 sys_reg(3, 5, 4, 0, 0)
>> #define SYS_ELR_EL12 sys_reg(3, 5, 4, 0, 1)
>> #define SYS_AFSR0_EL12 sys_reg(3, 5, 5, 1, 0)
>> #define SYS_AFSR1_EL12 sys_reg(3, 5, 5, 1, 1)
>> #define SYS_ESR_EL12 sys_reg(3, 5, 5, 2, 0)
>> #define SYS_TFSR_EL12 sys_reg(3, 5, 5, 6, 0)
>> +#define SYS_FAR_EL12 sys_reg(3, 5, 6, 0, 0)
>> +#define SYS_BRBCR_EL12 sys_reg(3, 5, 9, 0, 0)
> isn't it sys_reg(2, 5, 9, 0, 0)?
> 

Oops. It is indeed.

>> +#define SYS_PMSCR_EL12 sys_reg(3, 5, 9, 9, 0)
> 
>> #define SYS_MAIR_EL12 sys_reg(3, 5, 10, 2, 0)
>> #define SYS_AMAIR_EL12 sys_reg(3, 5, 10, 3, 0)
>> #define SYS_VBAR_EL12 sys_reg(3, 5, 12, 0, 0)
>> +#define SYS_CONTEXTIDR_EL12 sys_reg(3, 5, 13, 0, 1)
>> +#define SYS_SCXTNUM_EL12 sys_reg(3, 5, 13, 0, 7)
>> #define SYS_CNTKCTL_EL12 sys_reg(3, 5, 14, 1, 0)
>> #define SYS_CNTP_TVAL_EL02 sys_reg(3, 5, 14, 2, 0)
>> #define SYS_CNTP_CTL_EL02 sys_reg(3, 5, 14, 2, 1)
> Besides
> Reviewed-by: Eric Auger <eric.auger@redhat.com>
> 

Thanks

Miguel

> Eric
> 
> 


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

Re: [PATCH v2 2/2] arm64/kvm: Fine grain _EL2 system registers list that affect nested virtualization

[Eric Auger]

Hi Miguel,
On 9/25/23 18:20, Miguel Luis wrote:
> Some _EL1 registers got included in the _EL2 ranges, which are not
if they aren't too many, you may list them as it eases the review
> affected by NV. Remove them, fine grain the ranges to exclusively
> include the _EL2 ones and fold SPSR/ELR _EL2 registers into the
> existing range.
> 
> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
Fixes: d0fc0a2519a6 (" KVM: arm64: nv: Add trap forwarding for HCR_EL2") ?
> ---
>  arch/arm64/kvm/emulate-nested.c | 44 ++++++++++++++++++++++++++++-----
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
> index 9ced1bf0c2b7..f6d0c87803f4 100644
> --- a/arch/arm64/kvm/emulate-nested.c
> +++ b/arch/arm64/kvm/emulate-nested.c
> @@ -649,14 +649,46 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
>  	SR_TRAP(SYS_APGAKEYHI_EL1,	CGT_HCR_APK),
>  	/* All _EL2 registers */
>  	SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
> -		      sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
> +		      sys_reg(3, 4, 4, 0, 1), CGT_HCR_NV),
>  	/* Skip the SP_EL1 encoding... */
> -	SR_TRAP(SYS_SPSR_EL2,		CGT_HCR_NV),
> -	SR_TRAP(SYS_ELR_EL2,		CGT_HCR_NV),
> -	SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
> -		      sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
I am not sure I fully understand the sysreg encoding but globally there
are not so many _EL2 regs trapped with .NV. And I can see holes within
somes ranges defined above (I searched all "if EL2Enabled() &&
HCR_EL2.NV == '1' then" in the ARM ARM). Maybe I don't know how to use
the ARM ARM doc but I feel  difficult to understand if the "holes"
within the encoding of some ranges are unused or are allocated to some
other sysregs, which wouldn't be trapped by /NV. I fear range encoding
may be quite risky.
> +	SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
> +		      sys_reg(3, 4, 10, 6, 7), CGT_HCR_NV),
> +	/*
> +	 * Note that the spec. describes a group of MEC registers
> +	 * whose access should not trap, therefore skip the following:
> +	 * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
> +	 * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
> +	 * VMECID_P_EL2.
> +	 */
>  	SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
> -		      sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
> +		      sys_reg(3, 4, 12, 1, 1), CGT_HCR_NV),
> +	/* ICH_AP0R<m>_EL2 */
> +	SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
> +		      SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
> +	/* ICH_AP1R<m>_EL2 */
> +	SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
> +		      SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
> +	SR_RANGE_TRAP(sys_reg(3, 4, 12, 9, 5),
> +		      sys_reg(3, 4, 12, 11, 7), CGT_HCR_NV),
> +	/* ICH_LR<m>_EL2 */
> +	SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
> +		      SYS_ICH_LR7_EL2, CGT_HCR_NV),
> +	SR_RANGE_TRAP(SYS_ICH_LR8_EL2,
> +		      SYS_ICH_LR15_EL2, CGT_HCR_NV),
> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 0, 1),
> +		      sys_reg(3, 4, 13, 0, 7), CGT_HCR_NV),
> +	/* AMEVCNTVOFF0<n>_EL2 */
> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 8, 0),
> +		      sys_reg(3, 4, 13, 8, 7), CGT_HCR_NV),
> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 9, 0),
> +		      sys_reg(3, 4, 13, 9, 7), CGT_HCR_NV),
I think those 2 above ranges can be merged
> +	/* AMEVCNTVOFF1<n>_EL2 */
> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 10, 0),
> +		      sys_reg(3, 4, 13, 10, 7), CGT_HCR_NV),
> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 11, 0),
> +		      sys_reg(3, 4, 13, 11, 7), CGT_HCR_NV),
/* CNT*_EL2 */
> +	SR_RANGE_TRAP(sys_reg(3, 4, 14, 0, 3),
> +		      sys_reg(3, 4, 14, 5, 2), CGT_HCR_NV),
>  	/* All _EL02, _EL12 registers */
>  	SR_RANGE_TRAP(sys_reg(3, 5, 0, 0, 0),
>  		      sys_reg(3, 5, 10, 15, 7), CGT_HCR_NV),
not related to your patch but wrt the EL02 the only ones that I
idenftied beeing trapped by NV using above search are

CNTP_TVAL_EL02	3	5	14	2	0
CNTP_CTL_EL02	3	5	14	2	1
CNTP_CVAL_EL02	3	5	14	2	2
CNTV_TVAL_EL02	3	5	14	3	0
CNTV_CTL_EL02	3	5	14	3	1
CNTV_CVAL_EL02	3	5	14	3	2

Thanks

Eric


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

Re: [PATCH v2 2/2] arm64/kvm: Fine grain _EL2 system registers list that affect nested virtualization

[Miguel Luis]

Hi Eric,

On 29/09/2023 15:08, Eric Auger wrote:
> Hi Miguel,
> On 9/25/23 18:20, Miguel Luis wrote:
>> Some _EL1 registers got included in the _EL2 ranges, which are not
> if they aren't too many, you may list them as it eases the review

Thanks for bringing it up.

Initially I thought those _EL1 registers would be ESR_EL1, TFSR_EL1 and FAR_EL1,
but as I re-run through the process I cannot confirm the statement anymore.
So that statement is a mistake now?

I took as reference Table D18-2 on page D18-6307 where are listed instruction
encodings for non-debug system register accesses. Having to deal with the
document format is surely not an easy task, so I converted it to text using
pdftotext -layout.

After scraping, the end result is a table of encodings which we're allowed to
sort/grep which may be handy to this when you consider the statement that all
accesses (but the exceptions) to system registers ending in _EL2 should trap.

>> affected by NV. Remove them, fine grain the ranges to exclusively
>> include the _EL2 ones and fold SPSR/ELR _EL2 registers into the
>> existing range.
>>
>> Signed-off-by: Miguel Luis <miguel.luis@oracle.com>
> Fixes: d0fc0a2519a6 (" KVM: arm64: nv: Add trap forwarding for HCR_EL2") ?

OK.

>> ---
>>  arch/arm64/kvm/emulate-nested.c | 44 ++++++++++++++++++++++++++++-----
>>  1 file changed, 38 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
>> index 9ced1bf0c2b7..f6d0c87803f4 100644
>> --- a/arch/arm64/kvm/emulate-nested.c
>> +++ b/arch/arm64/kvm/emulate-nested.c
>> @@ -649,14 +649,46 @@ static const struct encoding_to_trap_config encoding_to_cgt[] __initconst = {
>>  	SR_TRAP(SYS_APGAKEYHI_EL1,	CGT_HCR_APK),
>>  	/* All _EL2 registers */
>>  	SR_RANGE_TRAP(sys_reg(3, 4, 0, 0, 0),
>> -		      sys_reg(3, 4, 3, 15, 7), CGT_HCR_NV),
>> +		      sys_reg(3, 4, 4, 0, 1), CGT_HCR_NV),
>>  	/* Skip the SP_EL1 encoding... */
>> -	SR_TRAP(SYS_SPSR_EL2,		CGT_HCR_NV),
>> -	SR_TRAP(SYS_ELR_EL2,		CGT_HCR_NV),
>> -	SR_RANGE_TRAP(sys_reg(3, 4, 4, 1, 1),
>> -		      sys_reg(3, 4, 10, 15, 7), CGT_HCR_NV),
> I am not sure I fully understand the sysreg encoding but globally there
> are not so many _EL2 regs trapped with .NV. And I can see holes within
> somes ranges defined above (I searched all "if EL2Enabled() &&
> HCR_EL2.NV == '1' then" in the ARM ARM). Maybe I don't know how to use
> the ARM ARM doc but I feel  difficult to understand if the "holes"
> within the encoding of some ranges are unused or are allocated to some
> other sysregs, which wouldn't be trapped by /NV. I fear range encoding
> may be quite risky.

That's definitely fair and I share the same concerns too.
Having table D18-2 sorted helped defining those ranges although I did not
find the answer to those questions. Perhaps we could query for assumptions
on the desired approach in which such implementation would rely.

>> +	SR_RANGE_TRAP(sys_reg(3, 4, 4, 3, 0),
>> +		      sys_reg(3, 4, 10, 6, 7), CGT_HCR_NV),
>> +	/*
>> +	 * Note that the spec. describes a group of MEC registers
>> +	 * whose access should not trap, therefore skip the following:
>> +	 * MECID_A0_EL2, MECID_A1_EL2, MECID_P0_EL2,
>> +	 * MECID_P1_EL2, MECIDR_EL2, VMECID_A_EL2,
>> +	 * VMECID_P_EL2.
>> +	 */
>>  	SR_RANGE_TRAP(sys_reg(3, 4, 12, 0, 0),
>> -		      sys_reg(3, 4, 14, 15, 7), CGT_HCR_NV),
>> +		      sys_reg(3, 4, 12, 1, 1), CGT_HCR_NV),
>> +	/* ICH_AP0R<m>_EL2 */
>> +	SR_RANGE_TRAP(SYS_ICH_AP0R0_EL2,
>> +		      SYS_ICH_AP0R3_EL2, CGT_HCR_NV),
>> +	/* ICH_AP1R<m>_EL2 */
>> +	SR_RANGE_TRAP(SYS_ICH_AP1R0_EL2,
>> +		      SYS_ICH_AP1R3_EL2, CGT_HCR_NV),
>> +	SR_RANGE_TRAP(sys_reg(3, 4, 12, 9, 5),
>> +		      sys_reg(3, 4, 12, 11, 7), CGT_HCR_NV),
>> +	/* ICH_LR<m>_EL2 */
>> +	SR_RANGE_TRAP(SYS_ICH_LR0_EL2,
>> +		      SYS_ICH_LR7_EL2, CGT_HCR_NV),
>> +	SR_RANGE_TRAP(SYS_ICH_LR8_EL2,
>> +		      SYS_ICH_LR15_EL2, CGT_HCR_NV),
>> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 0, 1),
>> +		      sys_reg(3, 4, 13, 0, 7), CGT_HCR_NV),
>> +	/* AMEVCNTVOFF0<n>_EL2 */
>> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 8, 0),
>> +		      sys_reg(3, 4, 13, 8, 7), CGT_HCR_NV),
>> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 9, 0),
>> +		      sys_reg(3, 4, 13, 9, 7), CGT_HCR_NV),
> I think those 2 above ranges can be merged

Oh, indeed. For both AMEVCNTVOFF0<n>_EL2 and AMEVCNTVOFF1<n>_EL2.

>> +	/* AMEVCNTVOFF1<n>_EL2 */
>> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 10, 0),
>> +		      sys_reg(3, 4, 13, 10, 7), CGT_HCR_NV),
>> +	SR_RANGE_TRAP(sys_reg(3, 4, 13, 11, 0),
>> +		      sys_reg(3, 4, 13, 11, 7), CGT_HCR_NV),
> /* CNT*_EL2 */

OK.

>> +	SR_RANGE_TRAP(sys_reg(3, 4, 14, 0, 3),
>> +		      sys_reg(3, 4, 14, 5, 2), CGT_HCR_NV),
>>  	/* All _EL02, _EL12 registers */
>>  	SR_RANGE_TRAP(sys_reg(3, 5, 0, 0, 0),
>>  		      sys_reg(3, 5, 10, 15, 7), CGT_HCR_NV),
> not related to your patch but wrt the EL02 the only ones that I
> idenftied beeing trapped by NV using above search are
> 
> CNTP_TVAL_EL02	3	5	14	2	0
> CNTP_CTL_EL02	3	5	14	2	1
> CNTP_CVAL_EL02	3	5	14	2	2
> CNTV_TVAL_EL02	3	5	14	3	0
> CNTV_CTL_EL02	3	5	14	3	1
> CNTV_CVAL_EL02	3	5	14	3	2
> 

That matches my search too. FWIW, below are the _EL12 from my search:

AFSR0_EL12 	3 5 5 1 0
AFSR1_EL12 	3 5 5 1 1
AMAIR_EL12 	3 5 5 3 0
CONTEXTIDR_EL12 3 5 13 0 1
CPACR_EL12 	3 5 1 0 2
ESR_EL12 	3 5 5 2 0
FAR_EL12 	3 5 6 0 0
MAIR_EL12 	3 5 10 2 0
SCTLR2_EL12 	3 5 1 0 3
SCTLR_EL12 	3 5 1 0 0
SMCR_EL12 	3 5 1 2 6
TCR2_EL12 	3 5 2 0 3
TCR_EL12 	3 5 2 0 2
TFSR_EL12 	3 5 5 6 0
TTBR0_EL12 	3 5 2 0 0
TTBR1_EL12 	3 5 2 0 1
VBAR_EL12 	3 5 12 0 0
ZCR_EL12 	3 5 1 2 0
TRFCR_EL12 	3 5 1 2 1
PMSCR_EL12 	3 5 9 9 0
CNTKCTL_EL12 	3 5 14 1 0

Thanks

Miguel

> Thanks
> 
> Eric
> 

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