linux-kernel - Re: [PATCHv2 3/4] arm-cci: Add routines to enable/disable all counters

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 4 Nov 2015 18:28:54 +0000
From:	Mark Rutland <mark.rutland@....com>
To:	"Suzuki K. Poulose" <suzuki.poulose@....com>
Cc:	linux-arm-kernel@...ts.infradead.org, punit.agrawal@....com,
	arm@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv2 3/4] arm-cci: Add routines to enable/disable all
 counters

On Tue, Oct 20, 2015 at 02:05:25PM +0100, Suzuki K. Poulose wrote:
> Adds helper routines to manipulate the counter controls for
> all the counters on the CCI PMU.
> 
> pmu_disable_counters_ctrl: Iterates over the counters,
> checking the status of each counter and disabling any enabled
> counters. For each such changed counter, the mask is updated so that
> one can restore the state later using pmu_enable_counters_ctrl.
> 
> Cc: Punit Agrawal <punit.agrawal@....com>
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: arm@...nel.org
> Signed-off-by: Suzuki K. Poulose <suzuki.poulose@....com>
> ---
>  drivers/bus/arm-cci.c |   30 ++++++++++++++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/bus/arm-cci.c b/drivers/bus/arm-cci.c
> index 1a75010..7f4a266 100644
> --- a/drivers/bus/arm-cci.c
> +++ b/drivers/bus/arm-cci.c
> @@ -663,6 +663,36 @@ static u32 pmu_get_event(struct cci_pmu *cci_pmu, int idx)
>  }
>  
>  /*
> + * Restore the status of the counters.
> + * For each counter set in the mask, enable the counter back.
> + */
> +static void pmu_restore_counters_ctrl(struct cci_pmu *cci_pmu, unsigned long *mask)
> +{
> +	int i;
> +
> +	for_each_set_bit(i, mask, cci_pmu->num_cntrs)
> +		pmu_enable_counter(cci_pmu, i);
> +}
> +
> +/*
> + * For all counters on the CCI-PMU, disable any 'enabled' counters,
> + * saving the changed counters in the mask, so that we can restore
> + * it later using pmu_restore_counters_ctrl.
> + */
> +static void pmu_disable_counters_ctrl(struct cci_pmu *cci_pmu, unsigned long *mask)
> +{
> +	int i;
> +
> +	for (i = 0; i < cci_pmu->num_cntrs; i++) {
> +		clear_bit(i, mask);
> +		if (pmu_get_counter_ctrl(cci_pmu, i)) {
> +			set_bit(i, mask);
> +			pmu_disable_counter(cci_pmu, i);
> +		}
> +	}
> +}

I don't understand what's going on with the mask here. Why do we clear
ieach bit when the only user (introduced in the next patch) explicitly
clears the mask anyway?

Can we not get rid of the mask entirely? The combination of used_mask
and each event's hwc->state tells us which counters are actually in use.

Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/