Re: [PATCH v3 05/26] KVM: arm64: nv: Add sanitising to VNCR-backed FGT sysregs

From: Oliver Upton <oliver.upton@linux.dev>
To: Marc Zyngier <maz@kernel.org>
Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Joey Gouly <joey.gouly@arm.com>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH v3 05/26] KVM: arm64: nv: Add sanitising to VNCR-backed FGT sysregs
Date: Wed, 7 Feb 2024 10:57:49 +0000	[thread overview]
Message-ID: <ZcNiLYCbacgnRuaw@linux.dev> (raw)
In-Reply-To: <20240205103431.1104133-6-maz@kernel.org>

On Mon, Feb 05, 2024 at 10:34:10AM +0000, Marc Zyngier wrote:
> Fine Grained Traps are controlled by a whole bunch of features.
> Each one of them must be checked and the corresponding masks
> computed so that we don't let the guest apply traps it shouldn't
> be using.
> 
> This takes care of HFG[IRW]TR_EL2, HDFG[RW]TR_EL2, and HAFGRTR_EL2.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Reviewed-by: Joey Gouly <joey.gouly@arm.com>
> ---
>  arch/arm64/kvm/nested.c | 128 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 128 insertions(+)
> 
> diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
> index a8fe0f970e04..31c4ce0439d6 100644
> --- a/arch/arm64/kvm/nested.c
> +++ b/arch/arm64/kvm/nested.c
> @@ -265,6 +265,134 @@ int kvm_init_nv_sysregs(struct kvm *kvm)
>  		res1 |= HCR_E2H;
>  	set_sysreg_masks(kvm, HCR_EL2, res0, res1);
>  
> +	/* HFG[RW]TR_EL2 */
> +	res0 = res1 = 0;
> +	if (!(__vcpu_has_feature(&kvm->arch, KVM_ARM_VCPU_PTRAUTH_ADDRESS) &&
> +	      __vcpu_has_feature(&kvm->arch, KVM_ARM_VCPU_PTRAUTH_ADDRESS)))

Same thing here w/ the different flags.

> +	/* HDFG[RW]TR_EL2 */
> +	res0 = res1 = 0;
> +	if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, DoubleLock, IMP))
> +		res0 |= HDFGRTR_EL2_OSDLR_EL1;
> +	if (!kvm_has_feat(kvm, ID_AA64DFR0_EL1, PMUVer, IMP))
> +		res0 |= (HDFGRTR_EL2_PMEVCNTRn_EL0 | HDFGRTR_EL2_PMEVTYPERn_EL0 |
> +			 HDFGRTR_EL2_PMCCFILTR_EL0 | HDFGRTR_EL2_PMCCNTR_EL0 |
> +			 HDFGRTR_EL2_PMCNTEN | HDFGRTR_EL2_PMINTEN |
> +			 HDFGRTR_EL2_PMOVS | HDFGRTR_EL2_PMSELR_EL0 |
> +			 HDFGRTR_EL2_PMMIR_EL1 | HDFGRTR_EL2_PMUSERENR_EL0 |
> +			 HDFGRTR_EL2_PMCEIDn_EL0);

I think you're missing HDFGWTR_EL2_PMSWINC_EL0 and HDFGWTR_EL2_PMCR_EL0
here. Silly write-only and read-only registers...

-- 
Thanks,
Oliver