Re: [PATCH v4 02/14] KVM: arm64: Group setting traps for protected VMs by control register

[Quentin Perret <qperret@google.com>]

On Monday 02 Dec 2024 at 15:47:29 (+0000), Fuad Tabba wrote:
> +static void pvm_init_traps_cptr(struct kvm_vcpu *vcpu)
>  {
> -	const u64 feature_ids = pvm_read_id_reg(vcpu, SYS_ID_AA64DFR0_EL1);
> -	u64 mdcr_set = 0;
> -	u64 mdcr_clear = 0;
> -	u64 cptr_set = 0;
> +	const u64 id_aa64pfr0 = pvm_read_id_reg(vcpu, SYS_ID_AA64PFR0_EL1);
> +	const u64 id_aa64pfr1 = pvm_read_id_reg(vcpu, SYS_ID_AA64PFR1_EL1);
> +	const u64 id_aa64dfr0 = pvm_read_id_reg(vcpu, SYS_ID_AA64DFR0_EL1);
> +	u64 val = vcpu->arch.cptr_el2;
>  
> -	/* Trap/constrain PMU */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), feature_ids)) {
> -		mdcr_set |= MDCR_EL2_TPM | MDCR_EL2_TPMCR;
> -		mdcr_clear |= MDCR_EL2_HPME | MDCR_EL2_MTPME |
> -			      MDCR_EL2_HPMN_MASK;
> +	if (!has_hvhe()) {
> +		val |= CPTR_NVHE_EL2_RES1;
> +		val &= ~(CPTR_NVHE_EL2_RES0);

It's a bit odd to do that at the beginning of the function no? Perhaps
do it last to be sure to overwrite all the changes below?

Also, not sure we need the RES1 part here, kvm_get_reset_cptr_el2()
should have set those for us.

>  	}
>  
> -	/* Trap Debug */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), feature_ids))
> -		mdcr_set |= MDCR_EL2_TDRA | MDCR_EL2_TDA | MDCR_EL2_TDE;
> -
> -	/* Trap OS Double Lock */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DoubleLock), feature_ids))
> -		mdcr_set |= MDCR_EL2_TDOSA;
> +	/* Trap AMU */
> +	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU), id_aa64pfr0))
> +		val |= CPTR_EL2_TAM;
>  
> -	/* Trap SPE */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer), feature_ids)) {
> -		mdcr_set |= MDCR_EL2_TPMS;
> -		mdcr_clear |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;
> +	/* Trap SVE */
> +	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE), id_aa64pfr0)) {
> +		if (has_hvhe())
> +			val &= ~(CPACR_ELx_ZEN);
> +		else
> +			val |= CPTR_EL2_TZ;
>  	}
>  
> -	/* Trap Trace Filter */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_TraceFilt), feature_ids))
> -		mdcr_set |= MDCR_EL2_TTRF;
> +	/* No SME support in KVM. */
> +	BUG_ON(FIELD_GET(ARM64_FEATURE_MASK(ID_AA64PFR1_EL1_SME), id_aa64pfr1));
> +	if (has_hvhe())
> +		val &= ~(CPACR_ELx_SMEN);
> +	else
> +		val |= CPTR_EL2_TSM;
>  
>  	/* Trap Trace */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_TraceVer), feature_ids)) {
> +	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_TraceVer), id_aa64dfr0)) {
>  		if (has_hvhe())
> -			cptr_set |= CPACR_EL1_TTA;
> +			val |= CPACR_EL1_TTA;
>  		else
> -			cptr_set |= CPTR_EL2_TTA;
> +			val |= 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;
> -}
> -
> -/*
> - * Set trap register values based on features in ID_AA64MMFR0.
> - */
> -static void pvm_init_traps_aa64mmfr0(struct kvm_vcpu *vcpu)
> -{
> -	const u64 feature_ids = pvm_read_id_reg(vcpu, SYS_ID_AA64MMFR0_EL1);
> -	u64 mdcr_set = 0;
> -
> -	/* Trap Debug Communications Channel registers */
> -	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64MMFR0_EL1_FGT), feature_ids))
> -		mdcr_set |= MDCR_EL2_TDCC;
> -
> -	vcpu->arch.mdcr_el2 |= mdcr_set;
> +	vcpu->arch.cptr_el2 = val;
>  }