[PATCH 12/25] KVM: arm64: nv: Move system instructions to their own sys_reg_desc array

[Joey Gouly]

Salut,

On Mon, Jan 22, 2024 at 08:18:39PM +0000, Marc Zyngier wrote:
> As NV results in a bunch of system instructions being trapped, it makes
> sense to pull the system instructions into their own little array, where
> they will eventually be joined by AT, TLBI and a bunch of other CMOs.
> 
> Based on an initial patch by Jintack Lim.
> 
> Signed-off-by: Marc Zyngier <maz at kernel.org>
> ---
>  arch/arm64/kvm/sys_regs.c | 59 +++++++++++++++++++++++++++++----------
>  1 file changed, 44 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 041b11825578..501de653beb5 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2197,16 +2197,6 @@ static u64 reset_hcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
>   * guest...
>   */
>  static const struct sys_reg_desc sys_reg_descs[] = {
> -	{ SYS_DESC(SYS_DC_ISW), access_dcsw },
> -	{ SYS_DESC(SYS_DC_IGSW), access_dcgsw },
> -	{ SYS_DESC(SYS_DC_IGDSW), access_dcgsw },
> -	{ SYS_DESC(SYS_DC_CSW), access_dcsw },
> -	{ SYS_DESC(SYS_DC_CGSW), access_dcgsw },
> -	{ SYS_DESC(SYS_DC_CGDSW), access_dcgsw },
> -	{ SYS_DESC(SYS_DC_CISW), access_dcsw },
> -	{ SYS_DESC(SYS_DC_CIGSW), access_dcgsw },
> -	{ SYS_DESC(SYS_DC_CIGDSW), access_dcgsw },
> -
>  	DBG_BCR_BVR_WCR_WVR_EL1(0),
>  	DBG_BCR_BVR_WCR_WVR_EL1(1),
>  	{ SYS_DESC(SYS_MDCCINT_EL1), trap_debug_regs, reset_val, MDCCINT_EL1, 0 },
> @@ -2738,6 +2728,18 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	EL2_REG(SP_EL2, NULL, reset_unknown, 0),
>  };
>  
> +static struct sys_reg_desc sys_insn_descs[] = {
> +	{ SYS_DESC(SYS_DC_ISW), access_dcsw },
> +	{ SYS_DESC(SYS_DC_IGSW), access_dcgsw },
> +	{ SYS_DESC(SYS_DC_IGDSW), access_dcgsw },
> +	{ SYS_DESC(SYS_DC_CSW), access_dcsw },
> +	{ SYS_DESC(SYS_DC_CGSW), access_dcgsw },
> +	{ SYS_DESC(SYS_DC_CGDSW), access_dcgsw },
> +	{ SYS_DESC(SYS_DC_CISW), access_dcsw },
> +	{ SYS_DESC(SYS_DC_CIGSW), access_dcgsw },
> +	{ SYS_DESC(SYS_DC_CIGDSW), access_dcgsw },
> +};
> +
>  static const struct sys_reg_desc *first_idreg;
>  
>  static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
> @@ -3431,6 +3433,24 @@ static bool emulate_sys_reg(struct kvm_vcpu *vcpu,
>  	return false;
>  }
>  
> +static int emulate_sys_instr(struct kvm_vcpu *vcpu, struct sys_reg_params *p)
> +{
> +	const struct sys_reg_desc *r;
> +
> +	/* Search from the system instruction table. */
> +	r = find_reg(p, sys_insn_descs, ARRAY_SIZE(sys_insn_descs));
> +
> +	if (likely(r)) {
> +		perform_access(vcpu, p, r);
> +	} else {
> +		kvm_err("Unsupported guest sys instruction at: %lx\n",
> +			*vcpu_pc(vcpu));
> +		print_sys_reg_instr(p);
> +		kvm_inject_undefined(vcpu);
> +	}
> +	return 1;
> +}
> +
>  static void kvm_reset_id_regs(struct kvm_vcpu *vcpu)
>  {
>  	const struct sys_reg_desc *idreg = first_idreg;
> @@ -3478,7 +3498,8 @@ void kvm_reset_sys_regs(struct kvm_vcpu *vcpu)
>  }
>  
>  /**
> - * kvm_handle_sys_reg -- handles a mrs/msr trap on a guest sys_reg access
> + * kvm_handle_sys_reg -- handles a system instruction or mrs/msr instruction
> + *			 trap on a guest execution
>   * @vcpu: The VCPU pointer
>   */
>  int kvm_handle_sys_reg(struct kvm_vcpu *vcpu)
> @@ -3495,12 +3516,19 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu)
>  	params = esr_sys64_to_params(esr);
>  	params.regval = vcpu_get_reg(vcpu, Rt);
>  
> -	if (!emulate_sys_reg(vcpu, &params))
> +	/* System register? */

Can you put a reference here, DDI487 J.a C5.1.2? (2 and 3 look magic otherwise)

> +	if (params.Op0 == 2 || params.Op0 == 3) {
> +		if (!emulate_sys_reg(vcpu, &params))
> +			return 1;
> +
> +		if (!params.is_write)
> +			vcpu_set_reg(vcpu, Rt, params.regval);
> +
>  		return 1;
> +	}
>  
> -	if (!params.is_write)
> -		vcpu_set_reg(vcpu, Rt, params.regval);
> -	return 1;
> +	/* Hints, PSTATE (Op0 == 0) and System instructions (Op0 == 1) */
> +	return emulate_sys_instr(vcpu, &params);
>  }
>  
>  /******************************************************************************
> @@ -3954,6 +3982,7 @@ int __init kvm_sys_reg_table_init(void)
>  	valid &= check_sysreg_table(cp15_regs, ARRAY_SIZE(cp15_regs), true);
>  	valid &= check_sysreg_table(cp15_64_regs, ARRAY_SIZE(cp15_64_regs), true);
>  	valid &= check_sysreg_table(invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs), false);
> +	valid &= check_sysreg_table(sys_insn_descs, ARRAY_SIZE(sys_insn_descs), false);
>  
>  	if (!valid)
>  		return -EINVAL;

Otherwise,

Reviewed-by: Joey Gouly <joey.gouly at arm.com>

Thanks,
Joey