Re: [PATCH 2/2] KVM: arm64: Treat PMEVTYPER<n>_EL0.NSH as RES0

[Suzuki K Poulose <suzuki.poulose@xxxxxxx>]

On 11/10/2023 17:17, Oliver Upton wrote:
> On Wed, Oct 11, 2023 at 01:33:16PM +0100, Suzuki K Poulose wrote:
>
> [...]
>
> > However, I think we are missing the support for a guest using the
> > combination of PMEVTYPER.NS{K/U} instead of the PMEVTYPER.{P/U} for
> > filtering the events. As per Arm ARM, it is permitted to use the
> > PMEVTYPER.NSK/U (leaving PMEVTYPER.{P,U} == 0) for filtering in Non-Secure
> > EL1.
>
> Ah, good eye. The pseudocode is easy enough to rip off, something like
> the below diff would get things going. There's an extra step of making
> these bits RES0 if EL3 isn't present in the guest's ID register values,
> but not a huge deal.

True, the change below looks good to me. Thanks for addressing this.

Suzuki

> > Anyways, for this patch:
> >
> > Reviewed-by: Suzuki K Poulose <suzuki.poulose@xxxxxxx?
>
> Thanks!
>
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 087764435390..b6df9ba39940 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -585,6 +585,7 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
>   	struct perf_event *event;
>   	struct perf_event_attr attr;
>   	u64 eventsel, reg, data;
> +	bool p, u, nsk, nsu;
>   
>   	reg = counter_index_to_evtreg(pmc->idx);
>   	data = __vcpu_sys_reg(vcpu, reg);
> @@ -611,13 +612,18 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc *pmc)
>   	    !test_bit(eventsel, vcpu->kvm->arch.pmu_filter))
>   		return;
>   
> +	p = data & ARMV8_PMU_EXCLUDE_EL1;
> +	u = data & ARMV8_PMU_EXCLUDE_EL0;
> +	nsk = data & ARMV8_PMU_EXCLUDE_NS_EL1;
> +	nsu = data & ARMV8_PMU_EXCLUDE_NS_EL0;
> +
>   	memset(&attr, 0, sizeof(struct perf_event_attr));
>   	attr.type = arm_pmu->pmu.type;
>   	attr.size = sizeof(attr);
>   	attr.pinned = 1;
>   	attr.disabled = !kvm_pmu_counter_is_enabled(pmc);
> -	attr.exclude_user = data & ARMV8_PMU_EXCLUDE_EL0 ? 1 : 0;
> -	attr.exclude_kernel = data & ARMV8_PMU_EXCLUDE_EL1 ? 1 : 0;
> +	attr.exclude_user = (u != nsu);
> +	attr.exclude_kernel = (p != nsk);
>   	attr.exclude_hv = 1; /* Don't count EL2 events */
>   	attr.exclude_host = 1; /* Don't count host events */
>   	attr.config = eventsel;
> @@ -663,7 +669,8 @@ void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>   	if (!kvm_vcpu_has_pmu(vcpu))
>   		return;
>   
> -	mask = ARMV8_PMU_EXCLUDE_EL1 | ARMV8_PMU_EXCLUDE_EL0;
> +	mask = ARMV8_PMU_EXCLUDE_EL1 | ARMV8_PMU_EXCLUDE_EL0 |
> +	       ARMV8_PMU_EXCLUDE_NS_EL1 | ARMV8_PMU_EXCLUDE_NS_EL0;
>   	mask |= kvm_pmu_event_mask(vcpu->kvm);
>   
>   	reg = counter_index_to_evtreg(pmc->idx);
> diff --git a/include/linux/perf/arm_pmuv3.h b/include/linux/perf/arm_pmuv3.h
> index 753f8dbd9d10..872119cc2bac 100644
> --- a/include/linux/perf/arm_pmuv3.h
> +++ b/include/linux/perf/arm_pmuv3.h
> @@ -235,9 +235,11 @@
>   /*
>    * Event filters for PMUv3
>    */
> -#define ARMV8_PMU_EXCLUDE_EL1	(1U << 31)
> -#define ARMV8_PMU_EXCLUDE_EL0	(1U << 30)
> -#define ARMV8_PMU_INCLUDE_EL2	(1U << 27)
> +#define ARMV8_PMU_EXCLUDE_EL1		(1U << 31)
> +#define ARMV8_PMU_EXCLUDE_EL0		(1U << 30)
> +#define ARMV8_PMU_EXCLUDE_NS_EL1	(1U << 29)
> +#define ARMV8_PMU_EXCLUDE_NS_EL0	(1U << 28)
> +#define ARMV8_PMU_INCLUDE_EL2		(1U << 27)
>   
>   /*
>    * PMUSERENR: user enable reg