Re: [Qemu-devel] [PATCH v6 11/14] target/arm: PMU: Add instruction and cycle events

From: Aaron Lindsay <Aaron@os.amperecomputing.com>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: Aaron Lindsay <aclindsa@gmail.com>,
	"qemu-arm@nongnu.org" <qemu-arm@nongnu.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	Alistair Francis <alistair.francis@xilinx.com>,
	Wei Huang <wei@redhat.com>,
	Peter Crosthwaite <crosthwaite.peter@gmail.com>,
	Michael Spradling <mspradli@codeaurora.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Digant Desai <digantd@codeaurora.org>
Subject: Re: [Qemu-devel] [PATCH v6 11/14] target/arm: PMU: Add instruction and cycle events
Date: Wed, 17 Oct 2018 19:47:56 +0000	[thread overview]
Message-ID: <20181017194748.GB25905@quinoa.localdomain> (raw)
In-Reply-To: <be878ed3-509f-8ed8-8c28-71e3d0214837@linaro.org>

On Oct 16 17:04, Richard Henderson wrote:
> On 10/10/18 1:37 PM, Aaron Lindsay wrote:
> > + * Return the underlying cycle count for the PMU cycle counters. If we're in
> > + * usermode, simply return 0.
> > + */
> > +static uint64_t cycles_get_count(CPUARMState *env)
> > +{
> > +#ifndef CONFIG_USER_ONLY
> > +    return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
> > +                   ARM_CPU_FREQ, NANOSECONDS_PER_SECOND);
> > +#else
> > +    return 0;
> > +#endif
> > +}
> 
> Usually we pass through the host cycle counter.
> See cpu_get_host_ticks().

Why do you prefer cpu_get_host_ticks()? And are you suggesting this for
just user-mode, or both system and user?

PMCCNTR used this same qemu_clock_get_ns() call previous to my patch
(see where this patch replaces that call with one to cycles_get_count()
in pmccntr_op_start()). Of course, we could keep the preexisting PMCCNTR
behavior while making the new cycle counter use cpu_get_host_ticks(),
but having two ways through the same interface which count cycles
differently feels wrong.

-Aaron