[PATCH v1 1/6] KVM: arm64: Update and fix FGT register masks

[Marc Zyngier]

On Tue, 05 Dec 2023 10:22:43 +0000,
Fuad Tabba <tabba at google.com> wrote:
> 
> New trap bits have been defined in the latest Arm Architecture
> System Registers xml specification [*]. Moreover, the existing
> definitions of some of the mask and the res0 bits overlap, which

nit: s/res0/RES0/

> could be wrong, confusing, or potentially both.
> 
> Update the bits to represent the latest spec (as of this patch),
> and ensure that the existing bits are consistent.
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools

Please indicate which version of the XML you are referring to (it
changes on average every 3 months).

> 
> Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> Signed-off-by: Fuad Tabba <tabba at 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 b85f46a73e21..b1061647e837 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -344,36 +344,45 @@
>   * 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)

If we are now using the current architectural definition for RES0, can
we please move over to the generated values, as indicated in the above
comment?

>  #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)
>  
>  /* 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))

The main driver for not following the latest and greatest version
published in the architecture (including the FGT2 stuff) was to limit
the amount of work we need to do in NV.

Can you please update the FGT tables in emulated-nested.c so that we
have a matching description of the added bits? This also overlaps with
the work Joey is doing on POE, so there is scope for collaboration
between you two here.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.