[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <cea5b9e9-99c1-47cb-bb2f-cdb922f4d349@redhat.com>
Date: Thu, 30 May 2024 18:54:23 +0200
From: Eric Auger <eauger@...hat.com>
To: Sebastian Ott <sebott@...hat.com>, linux-arm-kernel@...ts.infradead.org,
kvmarm@...ts.linux.dev, linux-kernel@...r.kernel.org
Cc: Marc Zyngier <maz@...nel.org>, Oliver Upton <oliver.upton@...ux.dev>,
James Morse <james.morse@....com>, Suzuki K Poulose
<suzuki.poulose@....com>, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>
Subject: Re: [PATCH v3 1/6] KVM: arm64: unify code to prepare traps
Hi Sebastian,
On 5/14/24 09:22, Sebastian Ott wrote:
> There are 2 functions to calculate traps via HCR_EL2:
> * kvm_init_sysreg() called via KVM_RUN (before the 1st run or when
> the pid changes)
> * vcpu_reset_hcr() called via KVM_ARM_VCPU_INIT
>
> To unify these 2 and to support traps that are dependent on the
> ID register configuration, move the code from vcpu_reset_hcr()
> to sys_regs.c and call it via kvm_init_sysreg().
>
> We still have to keep the non-FWB handling stuff in vcpu_reset_hcr().
> Also the initialization with HCR_GUEST_FLAGS is kept there but guarded
> by !vcpu_has_run_once() to ensure that previous calculated values
> don't get overwritten.
>
> While at it rename kvm_init_sysreg() to kvm_calculate_traps() to
> better reflect what it's doing.
>
> Signed-off-by: Sebastian Ott <sebott@...hat.com>
Looks good to me
Reviewed-by: Eric Auger <eric.auger@...hat.com>
Eric
> ---
> arch/arm64/include/asm/kvm_emulate.h | 40 +++++++---------------------
> arch/arm64/include/asm/kvm_host.h | 2 +-
> arch/arm64/kvm/arm.c | 2 +-
> arch/arm64/kvm/sys_regs.c | 34 +++++++++++++++++++++--
> 4 files changed, 43 insertions(+), 35 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
> index 501e3e019c93..84dc3fac9711 100644
> --- a/arch/arm64/include/asm/kvm_emulate.h
> +++ b/arch/arm64/include/asm/kvm_emulate.h
> @@ -69,39 +69,17 @@ static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
>
> static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
> {
> - vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
> - if (has_vhe() || has_hvhe())
> - vcpu->arch.hcr_el2 |= HCR_E2H;
> - if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN)) {
> - /* route synchronous external abort exceptions to EL2 */
> - vcpu->arch.hcr_el2 |= HCR_TEA;
> - /* trap error record accesses */
> - vcpu->arch.hcr_el2 |= HCR_TERR;
> - }
> + if (!vcpu_has_run_once(vcpu))
> + vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
>
> - if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB)) {
> - vcpu->arch.hcr_el2 |= HCR_FWB;
> - } else {
> - /*
> - * For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
> - * get set in SCTLR_EL1 such that we can detect when the guest
> - * MMU gets turned on and do the necessary cache maintenance
> - * then.
> - */
> + /*
> + * For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
> + * get set in SCTLR_EL1 such that we can detect when the guest
> + * MMU gets turned on and do the necessary cache maintenance
> + * then.
> + */
> + if (!cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
> vcpu->arch.hcr_el2 |= HCR_TVM;
> - }
> -
> - if (cpus_have_final_cap(ARM64_HAS_EVT) &&
> - !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE))
> - vcpu->arch.hcr_el2 |= HCR_TID4;
> - else
> - vcpu->arch.hcr_el2 |= HCR_TID2;
> -
> - if (vcpu_el1_is_32bit(vcpu))
> - vcpu->arch.hcr_el2 &= ~HCR_RW;
> -
> - if (kvm_has_mte(vcpu->kvm))
> - vcpu->arch.hcr_el2 |= HCR_ATA;
> }
>
> static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 8170c04fde91..212ae77eefaf 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1122,7 +1122,7 @@ int __init populate_nv_trap_config(void);
> bool lock_all_vcpus(struct kvm *kvm);
> void unlock_all_vcpus(struct kvm *kvm);
>
> -void kvm_init_sysreg(struct kvm_vcpu *);
> +void kvm_calculate_traps(struct kvm_vcpu *);
>
> /* MMIO helpers */
> void kvm_mmio_write_buf(void *buf, unsigned int len, unsigned long data);
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 9996a989b52e..6b217afb4e8e 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -797,7 +797,7 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
> * This needs to happen after NV has imposed its own restrictions on
> * the feature set
> */
> - kvm_init_sysreg(vcpu);
> + kvm_calculate_traps(vcpu);
>
> ret = kvm_timer_enable(vcpu);
> if (ret)
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 22b45a15d068..41741bf4d2b2 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -4041,11 +4041,33 @@ int kvm_vm_ioctl_get_reg_writable_masks(struct kvm *kvm, struct reg_mask_range *
> return 0;
> }
>
> -void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> +static void vcpu_set_hcr(struct kvm_vcpu *vcpu)
> {
> struct kvm *kvm = vcpu->kvm;
>
> - mutex_lock(&kvm->arch.config_lock);
> + if (has_vhe() || has_hvhe())
> + vcpu->arch.hcr_el2 |= HCR_E2H;
> + if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN)) {
> + /* route synchronous external abort exceptions to EL2 */
> + vcpu->arch.hcr_el2 |= HCR_TEA;
> + /* trap error record accesses */
> + vcpu->arch.hcr_el2 |= HCR_TERR;
> + }
> +
> + if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
> + vcpu->arch.hcr_el2 |= HCR_FWB;
> +
> + if (cpus_have_final_cap(ARM64_HAS_EVT) &&
> + !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE))
> + vcpu->arch.hcr_el2 |= HCR_TID4;
> + else
> + vcpu->arch.hcr_el2 |= HCR_TID2;
> +
> + if (vcpu_el1_is_32bit(vcpu))
> + vcpu->arch.hcr_el2 &= ~HCR_RW;
> +
> + if (kvm_has_mte(vcpu->kvm))
> + vcpu->arch.hcr_el2 |= HCR_ATA;
>
> /*
> * In the absence of FGT, we cannot independently trap TLBI
> @@ -4054,6 +4076,14 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
> */
> if (!kvm_has_feat(kvm, ID_AA64ISAR0_EL1, TLB, OS))
> vcpu->arch.hcr_el2 |= HCR_TTLBOS;
> +}
> +
> +void kvm_calculate_traps(struct kvm_vcpu *vcpu)
> +{
> + struct kvm *kvm = vcpu->kvm;
> +
> + mutex_lock(&kvm->arch.config_lock);
> + vcpu_set_hcr(vcpu);
>
> if (cpus_have_final_cap(ARM64_HAS_HCX)) {
> vcpu->arch.hcrx_el2 = HCRX_GUEST_FLAGS;
Powered by blists - more mailing lists