Re: [PATCH v6 32/43] arm64: rme: Enable PMU support with a realm guest

[Joey Gouly <joey.gouly@arm.com>]

On Thu, Dec 12, 2024 at 03:55:57PM +0000, Steven Price wrote:
> Use the PMU registers from the RmiRecExit structure to identify when an
> overflow interrupt is due and inject it into the guest. Also hook up the
> configuration option for enabling the PMU within the guest.
> 
> When entering a realm guest with a PMU interrupt pending, it is
> necessary to disable the physical interrupt. Otherwise when the RMM
> restores the PMU state the physical interrupt will trigger causing an
> immediate exit back to the host. The guest is expected to acknowledge
> the interrupt causing a host exit (to update the GIC state) which gives
> the opportunity to re-enable the physical interrupt before the next PMU
> event.
> 
> Number of PMU counters is configured by the VMM by writing to PMCR.N.
> 
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v2:
>  * Add a macro kvm_pmu_get_irq_level() to avoid compile issues when PMU
>    support is disabled.
> ---
>  arch/arm64/kvm/arm.c      | 11 +++++++++++
>  arch/arm64/kvm/guest.c    |  7 +++++++
>  arch/arm64/kvm/pmu-emul.c |  3 +++
>  arch/arm64/kvm/rme.c      |  8 ++++++++
>  arch/arm64/kvm/sys_regs.c |  2 +-
>  include/kvm/arm_pmu.h     |  4 ++++
>  6 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 6f7f96ab781d..ae3596a25272 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -15,6 +15,7 @@
>  #include <linux/vmalloc.h>
>  #include <linux/fs.h>
>  #include <linux/mman.h>
> +#include <linux/perf/arm_pmu.h>
>  #include <linux/sched.h>
>  #include <linux/kvm.h>
>  #include <linux/kvm_irqfd.h>
> @@ -1209,6 +1210,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  	run->exit_reason = KVM_EXIT_UNKNOWN;
>  	run->flags = 0;
>  	while (ret > 0) {
> +		bool pmu_stopped = false;
> +
>  		/*
>  		 * Check conditions before entering the guest
>  		 */
> @@ -1240,6 +1243,11 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  
>  		kvm_pmu_flush_hwstate(vcpu);
>  
> +		if (vcpu_is_rec(vcpu) && kvm_pmu_get_irq_level(vcpu)) {
> +			pmu_stopped = true;
> +			arm_pmu_set_phys_irq(false);
> +		}
> +
>  		local_irq_disable();
>  
>  		kvm_vgic_flush_hwstate(vcpu);
> @@ -1342,6 +1350,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
>  
>  		preempt_enable();
>  
> +		if (pmu_stopped)
> +			arm_pmu_set_phys_irq(true);
> +
>  		/*
>  		 * The ARMv8 architecture doesn't give the hypervisor
>  		 * a mechanism to prevent a guest from dropping to AArch32 EL0
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index 920e0fd73698..b4e3839f6076 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -804,6 +804,8 @@ int kvm_arm_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  	return kvm_arm_sys_reg_get_reg(vcpu, reg);
>  }
>  
> +#define KVM_REG_ARM_PMCR_EL0		ARM64_SYS_REG(3, 3, 9, 12, 0)

There's already SYS_PMCR_EL0 defined, so it seems this is not needed?

> +
>  /*
>   * The RMI ABI only enables setting some GPRs and PC. The selection of GPRs
>   * that are available depends on the Realm state and the reason for the last
> @@ -818,6 +820,11 @@ static bool validate_realm_set_reg(struct kvm_vcpu *vcpu,
>  		u64 off = core_reg_offset_from_id(reg->id);
>  
>  		return kvm_realm_validate_core_reg(off);
> +	} else {
> +		switch (reg->id) {
> +		case KVM_REG_ARM_PMCR_EL0:
> +			return true;
> +		}
>  	}
>  
>  	return false;
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 456102bc0b55..5628b573ca41 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -397,6 +397,9 @@ static bool kvm_pmu_overflow_status(struct kvm_vcpu *vcpu)
>  {
>  	u64 reg = __vcpu_sys_reg(vcpu, PMOVSSET_EL0);
>  
> +	if (vcpu_is_rec(vcpu))
> +		return vcpu->arch.rec.run->exit.pmu_ovf_status;
> +
>  	reg &= __vcpu_sys_reg(vcpu, PMINTENSET_EL1);
>  
>  	/*
> diff --git a/arch/arm64/kvm/rme.c b/arch/arm64/kvm/rme.c
> index 27a479feb907..e562e77c1f94 100644
> --- a/arch/arm64/kvm/rme.c
> +++ b/arch/arm64/kvm/rme.c
> @@ -303,6 +303,11 @@ static int realm_create_rd(struct kvm *kvm)
>  	params->rtt_base = kvm->arch.mmu.pgd_phys;
>  	params->vmid = realm->vmid;
>  
> +	if (kvm->arch.arm_pmu) {
> +		params->pmu_num_ctrs = kvm->arch.pmcr_n;
> +		params->flags |= RMI_REALM_PARAM_FLAG_PMU;
> +	}
> +
>  	params_phys = virt_to_phys(params);
>  
>  	if (rmi_realm_create(rd_phys, params_phys)) {
> @@ -1370,6 +1375,9 @@ int kvm_create_rec(struct kvm_vcpu *vcpu)
>  	if (!vcpu_has_feature(vcpu, KVM_ARM_VCPU_PSCI_0_2))
>  		return -EINVAL;
>  
> +	if (vcpu->kvm->arch.arm_pmu && !kvm_vcpu_has_pmu(vcpu))
> +		return -EINVAL;
> +
>  	BUILD_BUG_ON(sizeof(*params) > PAGE_SIZE);
>  	BUILD_BUG_ON(sizeof(*rec->run) > PAGE_SIZE);
>  
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 83c6b4a07ef5..a4713609e230 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1324,7 +1324,7 @@ static int set_pmcr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
>  	 * implements. Ignore this error to maintain compatibility
>  	 * with the existing KVM behavior.
>  	 */
> -	if (!kvm_vm_has_ran_once(kvm) &&
> +	if (!kvm_vm_has_ran_once(kvm) && !kvm_realm_is_created(kvm) &&
>  	    new_n <= kvm_arm_pmu_get_max_counters(kvm))
>  		kvm->arch.pmcr_n = new_n;
>  
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index e61dd7dd2286..30625a6fd143 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -77,6 +77,8 @@ void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_pmu_restore_host(struct kvm_vcpu *vcpu);
>  void kvm_vcpu_pmu_resync_el0(void);
>  
> +#define kvm_pmu_get_irq_level(vcpu) ((vcpu)->arch.pmu.irq_level)
> +
>  #define kvm_vcpu_has_pmu(vcpu)					\
>  	(vcpu_has_feature(vcpu, KVM_ARM_VCPU_PMU_V3))
>  
> @@ -164,6 +166,8 @@ static inline u64 kvm_pmu_get_pmceid(struct kvm_vcpu *vcpu, bool pmceid1)
>  	return 0;
>  }
>  
> +#define kvm_pmu_get_irq_level(vcpu) (false)
> +
>  #define kvm_vcpu_has_pmu(vcpu)		({ false; })
>  static inline void kvm_pmu_update_vcpu_events(struct kvm_vcpu *vcpu) {}
>  static inline void kvm_vcpu_pmu_restore_guest(struct kvm_vcpu *vcpu) {}

Thanks,
Joey