Re: [PATCH 2/2] KVM: arm64: PMU: Don't save PMCR_EL0.{C,P} for the vCPU

From: Marc Zyngier <maz@kernel.org>
To: Reiji Watanabe <reijiw@google.com>
Cc: Oliver Upton <oliver.upton@linux.dev>,
	kvmarm@lists.linux.dev, kvm@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	James Morse <james.morse@arm.com>,
	Alexandru Elisei <alexandru.elisei@arm.com>,
	Zenghui Yu <yuzenghui@huawei.com>,
	Suzuki K Poulose <suzuki.poulose@arm.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Ricardo Koller <ricarkol@google.com>,
	Jing Zhang <jingzhangos@google.com>,
	Raghavendra Rao Anata <rananta@google.com>,
	Will Deacon <will@kernel.org>
Subject: Re: [PATCH 2/2] KVM: arm64: PMU: Don't save PMCR_EL0.{C,P} for the vCPU
Date: Sun, 12 Mar 2023 15:01:06 +0000	[thread overview]
Message-ID: <87y1o23tfx.wl-maz@kernel.org> (raw)
In-Reply-To: <20230302055033.3081456-3-reijiw@google.com>

On Thu, 02 Mar 2023 05:50:33 +0000,
Reiji Watanabe <reijiw@google.com> wrote:
> 
> Presently, when a guest writes 1 to PMCR_EL0.{C,P}, which is WO/RAZ,
> KVM saves the register value, including these bits.
> When userspace reads the register using KVM_GET_ONE_REG, KVM returns
> the saved register value as it is (the saved value might have these
> bits set).  This could result in userspace setting these bits on the
> destination during migration.  Consequently, KVM may end up resetting
> the vPMU counter registers (PMCCNTR_EL0 and/or PMEVCNTR<n>_EL0) to
> zero on the first KVM_RUN after migration.
> 
> Fix this by not saving those bits when a guest writes 1 to those bits.
> 
> Signed-off-by: Reiji Watanabe <reijiw@google.com>
> ---
>  arch/arm64/kvm/pmu-emul.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 24908400e190..a5a0a9811ddb 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -538,7 +538,9 @@ void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val)
>  	if (!kvm_pmu_is_3p5(vcpu))
>  		val &= ~ARMV8_PMU_PMCR_LP;
>  
> -	__vcpu_sys_reg(vcpu, PMCR_EL0) = val;
> +	/* The reset bits don't indicate any state, and shouldn't be saved. */
> +	__vcpu_sys_reg(vcpu, PMCR_EL0) =
> +				val & ~(ARMV8_PMU_PMCR_C | ARMV8_PMU_PMCR_P);

nit: assignment on a single line, please.

With that,

Reviewed-by: Marc Zyngier <maz@kernel.org>

Thanks,

	M.

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

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