[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <865y47enz2.wl-maz@kernel.org>
Date: Mon, 18 Sep 2023 12:10:25 +0100
From: Marc Zyngier <maz@...nel.org>
To: Kristina Martsenko <kristina.martsenko@....com>
Cc: kvmarm@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
Oliver Upton <oliver.upton@...ux.dev>,
James Morse <james.morse@....com>,
Suzuki K Poulose <suzuki.poulose@....com>,
Zenghui Yu <yuzenghui@...wei.com>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>,
Vladimir Murzin <vladimir.murzin@....com>,
Colton Lewis <coltonlewis@...gle.com>,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3] KVM: arm64: Configure HCRX_EL2 dynamically
Hi Kristina,
On Fri, 15 Sep 2023 13:48:38 +0100,
Kristina Martsenko <kristina.martsenko@....com> wrote:
>
> At the moment the HCRX_EL2 system register is always initialized to
> HCRX_GUEST_FLAGS when running a guest. Instead, choose the configuration
> at vcpu reset time and save it in the vcpu struct, similarly to how
> HCR_EL2 is set up. This will be needed in a subsequent change to
> configure the register based on CPU features detected at runtime.
>
> Signed-off-by: Kristina Martsenko <kristina.martsenko@....com>
> ---
> arch/arm64/include/asm/kvm_emulate.h | 5 +++++
> arch/arm64/include/asm/kvm_host.h | 1 +
> arch/arm64/kvm/arm.c | 1 +
> arch/arm64/kvm/hyp/include/hyp/switch.h | 2 +-
> arch/arm64/kvm/hyp/nvhe/hyp-main.c | 1 +
> arch/arm64/kvm/hyp/nvhe/pkvm.c | 1 +
> 6 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 3d6725ff0bf6..64ea27e6deb1 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -134,6 +134,11 @@ static inline void vcpu_ptrauth_disable(struct kvm_vcpu *vcpu)
> vcpu->arch.hcr_el2 &= ~(HCR_API | HCR_APK);
> }
>
> +static inline void vcpu_reset_hcrx(struct kvm_vcpu *vcpu)
> +{
> + vcpu->arch.hcrx_el2 = HCRX_GUEST_FLAGS;
> +}
> +
> static inline unsigned long vcpu_get_vsesr(struct kvm_vcpu *vcpu)
> {
> return vcpu->arch.vsesr_el2;
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index af06ccb7ee34..2764748756a7 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -487,6 +487,7 @@ struct kvm_vcpu_arch {
>
> /* Values of trap registers for the guest. */
> u64 hcr_el2;
> + u64 hcrx_el2;
Do we really need this extra field? Yes, this is only an extra 64bit,
but they tend to accumulate...
Looking at patch #3, the change is related to this:
vcpu->arch.hcrx_el2 = HCRX_GUEST_FLAGS;
+
+ if (cpus_have_final_cap(ARM64_HAS_MOPS)) {
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_MSCEn;
+ vcpu->arch.hcrx_el2 |= HCRX_EL2_MCE2;
+ }
meaning that this is a constant value for a given boot of the host.
At this stage, I'd rather you define HCRX_GUEST_FLAGS as:
#define HCRX_GUEST_FLAGS \
(HCRX_EL2_SMPME | HCRX_EL2_TCR2En | \
cpus_have_final_cap(ARM64_HAS_MOPS) ? \
(HCRX_EL2_MSCEn | HCRX_EL2_MCE2) : 0)
and drop the new field altogether, until we have something that
requires dynamic flipping of an HCRX_EL2 field.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Powered by blists - more mailing lists