Re: [PATCH v2 2/2] KVM: arm64: Don't access PMSELR_EL0/PMUSERENR_EL0 when no PMU is available

2021-02-18 Thread Alexandru Elisei
Hi Marc,

On 2/9/21 11:48 AM, Marc Zyngier wrote:
> When running under a nesting hypervisor, it isn't guaranteed that
> the virtual HW will include a PMU. In which case, let's not try
> to access the PMU registers in the world switch, as that'd be
> deadly.
>
> Reported-by: Andre Przywara 
> Signed-off-by: Marc Zyngier 
> ---
>  arch/arm64/kernel/image-vars.h  | 3 +++
>  arch/arm64/kvm/hyp/include/hyp/switch.h | 9 ++---
>  2 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index f676243abac6..32af3c865700 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -102,6 +102,9 @@ KVM_NVHE_ALIAS(__stop___kvm_ex_table);
>  /* Array containing bases of nVHE per-CPU memory regions. */
>  KVM_NVHE_ALIAS(kvm_arm_hyp_percpu_base);
>  
> +/* PMU available static key */
> +KVM_NVHE_ALIAS(kvm_arm_pmu_available);
> +
>  #endif /* CONFIG_KVM */
>  
>  #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h 
> b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index 84473574c2e7..75c0faa3b791 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -90,15 +90,18 @@ static inline void __activate_traps_common(struct 
> kvm_vcpu *vcpu)
>* counter, which could make a PMXEVCNTR_EL0 access UNDEF at
>* EL1 instead of being trapped to EL2.
>*/
> - write_sysreg(0, pmselr_el0);
> - write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
> + if (kvm_arm_support_pmu_v3()) {
> + write_sysreg(0, pmselr_el0);
> + write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
> + }
>   write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
>  }
>  
>  static inline void __deactivate_traps_common(void)
>  {
>   write_sysreg(0, hstr_el2);
> - write_sysreg(0, pmuserenr_el0);
> + if (kvm_arm_support_pmu_v3())
> + write_sysreg(0, pmuserenr_el0);
>  }
>  
>  static inline void ___activate_traps(struct kvm_vcpu *vcpu)

It looks to me like this indeed fixes the issue with accessing PMU registers 
even
if the PMU is not present. Found another instance in the world switch where the
PMU is accessed, in __kvm_vcpu_run() in the nvhe code, but in that case the
registers were accessed only if there were events that needed to be context
switched. No PMU on the host, no PMU on the guest and no events for either, so
that looks fine to me:

Reviewed-by: Alexandru Elisei 

Thanks,

Alex

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH v2 2/2] KVM: arm64: Don't access PMSELR_EL0/PMUSERENR_EL0 when no PMU is available

2021-02-09 Thread Marc Zyngier
When running under a nesting hypervisor, it isn't guaranteed that
the virtual HW will include a PMU. In which case, let's not try
to access the PMU registers in the world switch, as that'd be
deadly.

Reported-by: Andre Przywara 
Signed-off-by: Marc Zyngier 
---
 arch/arm64/kernel/image-vars.h  | 3 +++
 arch/arm64/kvm/hyp/include/hyp/switch.h | 9 ++---
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index f676243abac6..32af3c865700 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -102,6 +102,9 @@ KVM_NVHE_ALIAS(__stop___kvm_ex_table);
 /* Array containing bases of nVHE per-CPU memory regions. */
 KVM_NVHE_ALIAS(kvm_arm_hyp_percpu_base);
 
+/* PMU available static key */
+KVM_NVHE_ALIAS(kvm_arm_pmu_available);
+
 #endif /* CONFIG_KVM */
 
 #endif /* __ARM64_KERNEL_IMAGE_VARS_H */
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h 
b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 84473574c2e7..75c0faa3b791 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -90,15 +90,18 @@ static inline void __activate_traps_common(struct kvm_vcpu 
*vcpu)
 * counter, which could make a PMXEVCNTR_EL0 access UNDEF at
 * EL1 instead of being trapped to EL2.
 */
-   write_sysreg(0, pmselr_el0);
-   write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+   if (kvm_arm_support_pmu_v3()) {
+   write_sysreg(0, pmselr_el0);
+   write_sysreg(ARMV8_PMU_USERENR_MASK, pmuserenr_el0);
+   }
write_sysreg(vcpu->arch.mdcr_el2, mdcr_el2);
 }
 
 static inline void __deactivate_traps_common(void)
 {
write_sysreg(0, hstr_el2);
-   write_sysreg(0, pmuserenr_el0);
+   if (kvm_arm_support_pmu_v3())
+   write_sysreg(0, pmuserenr_el0);
 }
 
 static inline void ___activate_traps(struct kvm_vcpu *vcpu)
-- 
2.29.2

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm