Re: [PATCH 3/4] arm64: arm_pmu: Exclude EL1,2 with :G :H perf attributes

[Suzuki K Poulose <suzuki.poulose@arm.com>]

Hi Andrew,

On 15/11/2018 12:55, Andrew Murray wrote:
> When using VHE, EL1 is unused by the host and EL2 is unused by the
> guest - therefore we can filter out these events with the PMU as per
> the 'exclude_host' and 'exclude_guest' attributes.
> 
> With both VHE and non-VHE we switch the counters between host/guest
> at EL2. With non-VHE when using 'exclude_host' we filter out EL2.
> 
> These changes eliminate counters counting host events on the
> boundaries of guest entry/exit when using :G. However when using :H
> unless exclude_hv is set on non-VHE then there is a small blackout
> window at the guest entry/exit where host events are not captured.
> 
> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
> ---
>   arch/arm64/kernel/perf_event.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 89d444f..c079c1f 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -971,12 +971,14 @@ static int armv8pmu_set_event_filter(struct hw_perf_event *event,
>   	 * with other architectures (x86 and Power).
>   	 */
>   	if (is_kernel_in_hyp_mode()) {
> -		if (!attr->exclude_kernel)
> +		if (!attr->exclude_kernel && !attr->exclude_host)
>   			config_base |= ARMV8_PMU_INCLUDE_EL2;
> +		if (attr->exclude_guest)
> +			config_base |= ARMV8_PMU_EXCLUDE_EL1;

Do we really need this ? exclude_guest also implies you need to
exclude guest EL0. We anyway disable the events when we enter the
guest. So the above is not necessary and could possibly create
confusion.

>   	} else {
>   		if (attr->exclude_kernel)
>   			config_base |= ARMV8_PMU_EXCLUDE_EL1;
> -		if (!attr->exclude_hv)
> +		if (!attr->exclude_hv && !attr->exclude_host)
>   			config_base |= ARMV8_PMU_INCLUDE_EL2;
>   	}
>   	if (attr->exclude_user)
> 


I think this can be folded into the previous patch, which adds the
support for exclude_host/guest support. :G, :H are nothing but
the those exclude_ flags.

Otherwise looks good to me.

Cheers
Suzuki