Re: [Qemu-devel] [RFC 5/5] arm: Simplify cycle counter

[Peter Crosthwaite]

On Fri, May 1, 2015 at 7:35 AM, Christopher Covington
<address@hidden> wrote:
> On Thu, Apr 30, 2015 at 9:24 PM, Peter Crosthwaite
> <address@hidden> wrote:
>> On Thu, Apr 30, 2015 at 11:14 AM, Christopher Covington
>> <address@hidden> wrote:
>>> Present a system with an instructions per cycle of exactly one.
>>> This makes it less likely a user will mistake the cycle counter
>>> values as meaningful and makes calculations involving cycles
>>> trivial while preserving the necessary property of the cycle
>>> counter register as monotonically increasing.
>>>
>>> Signed-off-by: Christopher Covington <address@hidden>
>>> ---
>>>  target-arm/helper.c | 9 +++------
>>>  1 file changed, 3 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>>> index 3e6fb0b..a027a19 100644
>>> --- a/target-arm/helper.c
>>> +++ b/target-arm/helper.c
>>> @@ -648,8 +648,7 @@ void pmccntr_sync(CPUARMState *env)
>>>  {
>>>      uint64_t temp_ticks;
>>>
>>> -    temp_ticks = muldiv64(qemu_clock_get_us(QEMU_CLOCK_VIRTUAL),
>>> -                          get_ticks_per_sec(), 1000000);
>>> +    temp_ticks = cpu_get_icount_raw();
>>
>> So I have guests (for better or worse) that make assumptions about the
>> rate of the PMCCNTR WRT real time. But isn't the PMCCNTR really a
>> clock cycle counter rather than an instruction counter? That clock
>> rate is well defined even if it is just the trivial
>> get_ticks_per_sec() at the moment. Ideally we should have a
>> configurable clock rate in there instead of get_ticks_per_sec(). This
>> is a major change in definition.
>
> Is this with KVM, user emulation, or system emulation mode? I'm mainly
> looking at things from the TCG (and mostly system mode) perspective.

The same. My guest has only ever run on TCG.

> If not using KVM, would the assumptions of your guests hold if you ran
> them on a hardware device instead of QEMU TCG?

I run them as a matter of course on hardware devices.

What I am most worried about (and I need to run some tests to really
confirm facts) is what happens if a CPU WFIs. Should the PMCCNTR
continue on or hold its value? If we match instruction execution to
PMCCNTR to the PMCCNTR will freeze.

Regards,
Peter

> I'm still trying to
> understand all the code involved, but I don't see get_ticks_per_sec()
> or any other constants as problematic.
>
>> I can see your use case though, where you actually want this to mean
>> something WRT program performance. Should we add a switch between the
>> two behaviours?
>
> This switch already exists, in the form of -icount, and I really
> should have already been using it. What initially scared me away was
> qemu_icount_bias and icount_adjust(). Accounting for halting seems
> like a good thing but the accounting for "speed" by referencing a host
> clock doesn't make sense to me. If I have an ARM development board
> hooked up to an x86 desktop, it's not querying the desktop for time.
> So why does it make sense for QEMU system emulation mode behave like
> that? The -icount help text makes it sound like adjustment for speed
> reasons should only take place when "auto" is specified, but I have
> yet to understand the code well enough to verify that.
>
> Chris
>