Patchwork [6/6] target-alpha: Add high-resolution access to wall clock and an alarm.

login
register
mail settings
Submitter Richard Henderson
Date Aug. 25, 2011, 9:45 p.m.
Message ID <1314308722-14495-7-git-send-email-rth@twiddle.net>
Download mbox | patch
Permalink /patch/111676/
State New
Headers show

Comments

Richard Henderson - Aug. 25, 2011, 9:45 p.m.
The alarm is a fully general one-shot time comparator, which will be
usable under Linux as a hrtimer source.  It's much more flexible than
the RTC source available on real hardware.

The wall clock allows the guest access to the host timekeeping.  Much
like the KVM wall clock source for other guests.

Both are accessed via the PALcode Cserve entry point.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 hw/alpha_typhoon.c       |   21 ++++++++++++++++++++-
 target-alpha/cpu.h       |    4 ++++
 target-alpha/helper.h    |    4 ++++
 target-alpha/op_helper.c |   15 +++++++++++++++
 target-alpha/translate.c |   14 ++++++++++++++
 5 files changed, 57 insertions(+), 1 deletions(-)
Peter Maydell - Aug. 26, 2011, 3:51 a.m.
On 25 August 2011 22:45, Richard Henderson <rth@twiddle.net> wrote:
> @@ -1604,6 +1607,12 @@ static void gen_mfpr(int ra, int regno)
>         return;
>     }
>
> +    if (regno == 250) {
> +        /* WALL_TIME */
> +        gen_helper_get_time(cpu_ir[ra]);
> +        return;
> +    }
> +
>     /* The basic registers are data only, and unknown registers
>        are read-zero, write-ignore.  */
>     if (data == 0) {
> @@ -1650,6 +1659,11 @@ static ExitStatus gen_mtpr(DisasContext *ctx, int rb, int regno)
>         gen_helper_halt(tmp);
>         return EXIT_PC_STALE;
>
> +    case 251:
> +        /* ALARM */
> +        gen_helper_set_alarm(tmp);
> +        break;
> +
>     default:
>         /* The basic registers are data only, and unknown registers
>            are read-zero, write-ignore.  */

Don't you need some magic around helper calls that read/write
the time to keep -icount working? I don't understand this but
Paolo does...

-- PMM
Paolo Bonzini - Aug. 26, 2011, 9:07 a.m.
On 08/26/2011 05:51 AM, Peter Maydell wrote:
> Don't you need some magic around helper calls that read/write
> the time to keep -icount working? I don't understand this but
> Paolo does...

Let's say I understand the theory (how icount relies on it) more than 
the practice (how the targets should do it). :)

Paolo
Richard Henderson - Aug. 26, 2011, 4:28 p.m.
On 08/25/2011 11:07 PM, Paolo Bonzini wrote:
> On 08/26/2011 05:51 AM, Peter Maydell wrote:
>> Don't you need some magic around helper calls that read/write
>> the time to keep -icount working? I don't understand this but
>> Paolo does...
> 
> Let's say I understand the theory (how icount relies on it) more than the practice (how the targets should do it). :)

Heh.  Well, let's say that while I've aped the other targets in
how icount is treated in the translator, I have no idea what it
is really supposed to accomplish, and so have never used it.


r~
Peter Maydell - Aug. 26, 2011, 4:36 p.m.
On 26 August 2011 17:28, Richard Henderson <rth@twiddle.net> wrote:
> On 08/25/2011 11:07 PM, Paolo Bonzini wrote:
>> On 08/26/2011 05:51 AM, Peter Maydell wrote:
>>> Don't you need some magic around helper calls that read/write
>>> the time to keep -icount working? I don't understand this but
>>> Paolo does...
>>
>> Let's say I understand the theory (how icount relies on it)
>> more than the practice (how the targets should do it). :)
>
> Heh.  Well, let's say that while I've aped the other targets in
> how icount is treated in the translator, I have no idea what it
> is really supposed to accomplish, and so have never used it.

Well, "what it's supposed to accomplish" is straightforward
enough, we want to keep a count of cycles executed (and I think
also maintain determinism, although that I'm less certain about).
Since we only update the count at the start of each basic block,
we (potentially) have to stop the basic block where we hit an
I/O operation which fiddles with the timer. For I/O via memory
accesses this is all dealt with by the generic code, but where
the CPU has some instruction which does things with timers not
via a memory access there needs to be a bit of special casing.

Look at the way target-i386 and target-mips use gen_io_start()
and gen_io_end() around x86 io insns and MIPS mtc0. I think
that what you need is (a) to bracket with gen_io_start/end
and (b) to end the translation block, but that's really just
guesswork from the existing code...

-- PMM
Richard Henderson - Aug. 26, 2011, 8:12 p.m.
On 08/26/2011 06:36 AM, Peter Maydell wrote:
> Look at the way target-i386 and target-mips use gen_io_start()
> and gen_io_end() around x86 io insns and MIPS mtc0. I think
> that what you need is (a) to bracket with gen_io_start/end
> and (b) to end the translation block, but that's really just
> guesswork from the existing code...

Hmph.  I suspect that the alpha port has never worked with
icount.  Trying to boot a kernel with it results in millions
of "Bad clock read" messages.

I'm 20 minutes from leaving for holidays.  I'll put this on
the to-do list for when I get back.


r~
Edgar Iglesias - Aug. 27, 2011, 4:44 p.m.
On Fri, Aug 26, 2011 at 05:36:19PM +0100, Peter Maydell wrote:
> On 26 August 2011 17:28, Richard Henderson <rth@twiddle.net> wrote:
> > On 08/25/2011 11:07 PM, Paolo Bonzini wrote:
> >> On 08/26/2011 05:51 AM, Peter Maydell wrote:
> >>> Don't you need some magic around helper calls that read/write
> >>> the time to keep -icount working? I don't understand this but
> >>> Paolo does...
> >>
> >> Let's say I understand the theory (how icount relies on it)
> >> more than the practice (how the targets should do it). :)
> >
> > Heh.  Well, let's say that while I've aped the other targets in
> > how icount is treated in the translator, I have no idea what it
> > is really supposed to accomplish, and so have never used it.
> 
> Well, "what it's supposed to accomplish" is straightforward
> enough, we want to keep a count of cycles executed (and I think
> also maintain determinism, although that I'm less certain about).
> Since we only update the count at the start of each basic block,
> we (potentially) have to stop the basic block where we hit an
> I/O operation which fiddles with the timer. For I/O via memory
> accesses this is all dealt with by the generic code, but where
> the CPU has some instruction which does things with timers not
> via a memory access there needs to be a bit of special casing.
> 
> Look at the way target-i386 and target-mips use gen_io_start()
> and gen_io_end() around x86 io insns and MIPS mtc0. I think
> that what you need is (a) to bracket with gen_io_start/end
> and (b) to end the translation block, but that's really just
> guesswork from the existing code...

I think thats right, you need both.

If you forget the gen_io_start/end calls, you'll notice. Everytime
the timer is read you'll hit the "Bad clock read" from cpus.c.

IIUC, If you dont end the TB after timer reads, the read will return
a time in the future (i.e the insns beyond the read will be accounted
as already executed at the point where you read the time).

For alarm setups, you need to end the TB to get precise deliveries, otherwise
I think you risk executing insns beyond the point when the timer should
have fired. Maybe not so likely unless you're setting timers to very
short intervals though.

Cheers

Patch

diff --git a/hw/alpha_typhoon.c b/hw/alpha_typhoon.c
index 3637efc..3d30e0e 100644
--- a/hw/alpha_typhoon.c
+++ b/hw/alpha_typhoon.c
@@ -681,6 +681,16 @@  static void typhoon_set_timer_irq(void *opaque, int irq, int level)
     }
 }
 
+static void typhoon_alarm_timer(void *opaque)
+{
+    TyphoonState *s = (TyphoonState *)((uintptr_t)opaque & ~3);
+    int cpu = (uintptr_t)opaque & 3;
+
+    /* Set the ITI bit for this cpu.  */
+    s->cchip.misc |= 1 << (cpu + 4);
+    cpu_interrupt(s->cchip.cpu[cpu], CPU_INTERRUPT_TIMER);
+}
+
 PCIBus *typhoon_init(ram_addr_t ram_size, qemu_irq *p_rtc_irq,
                      CPUState *cpus[4], pci_map_irq_fn sys_map_irq)
 {
@@ -692,6 +702,7 @@  PCIBus *typhoon_init(ram_addr_t ram_size, qemu_irq *p_rtc_irq,
     PCIHostState *p;
     TyphoonState *s;
     PCIBus *b;
+    int i;
 
     dev = qdev_create(NULL, "typhoon-pcihost");
     qdev_init_nofail(dev);
@@ -700,7 +711,15 @@  PCIBus *typhoon_init(ram_addr_t ram_size, qemu_irq *p_rtc_irq,
     s = container_of(p, TyphoonState, host);
 
     /* Remember the CPUs so that we can deliver interrupts to them.  */
-    memcpy(s->cchip.cpu, cpus, 4 * sizeof(CPUState *));
+    for (i = 0; i < 4; i++) {
+        CPUState *env = cpus[i];
+        s->cchip.cpu[i] = env;
+        if (env) {
+            env->alarm_timer = qemu_new_timer_ns(rtc_clock,
+                                                 typhoon_alarm_timer,
+                                                 (void *)((uintptr_t)s + i));
+        }
+    }
 
     *p_rtc_irq = *qemu_allocate_irqs(typhoon_set_timer_irq, s, 1);
 
diff --git a/target-alpha/cpu.h b/target-alpha/cpu.h
index c2e7bb3..9d61d45 100644
--- a/target-alpha/cpu.h
+++ b/target-alpha/cpu.h
@@ -265,6 +265,10 @@  struct CPUAlphaState {
     uint64_t scratch[24];
 #endif
 
+    /* This alarm doesn't exist in real hardware; we wish it did.  */
+    struct QEMUTimer *alarm_timer;
+    uint64_t alarm_expire;
+
 #if TARGET_LONG_BITS > HOST_LONG_BITS
     /* temporary fixed-point registers
      * used to emulate 64 bits target on 32 bits hosts
diff --git a/target-alpha/helper.h b/target-alpha/helper.h
index c352c24..b693cee 100644
--- a/target-alpha/helper.h
+++ b/target-alpha/helper.h
@@ -113,7 +113,11 @@  DEF_HELPER_2(stq_c_phys, i64, i64, i64)
 
 DEF_HELPER_FLAGS_0(tbia, TCG_CALL_CONST, void)
 DEF_HELPER_FLAGS_1(tbis, TCG_CALL_CONST, void, i64)
+
 DEF_HELPER_1(halt, void, i64);
+
+DEF_HELPER_FLAGS_0(get_time, TCG_CALL_CONST, i64)
+DEF_HELPER_FLAGS_1(set_alarm, TCG_CALL_CONST, void, i64)
 #endif
 
 #include "def-helper.h"
diff --git a/target-alpha/op_helper.c b/target-alpha/op_helper.c
index db5b9e7..6832163 100644
--- a/target-alpha/op_helper.c
+++ b/target-alpha/op_helper.c
@@ -1228,6 +1228,21 @@  void helper_halt(uint64_t restart)
         qemu_system_shutdown_request();
     }
 }
+
+uint64_t helper_get_time(void)
+{
+    return qemu_get_clock_ns(rtc_clock);
+}
+
+void helper_set_alarm(uint64_t expire)
+{
+    if (expire) {
+        env->alarm_expire = expire;
+        qemu_mod_timer(env->alarm_timer, expire);
+    } else {
+        qemu_del_timer(env->alarm_timer);
+    }
+}
 #endif
 
 /*****************************************************************************/
diff --git a/target-alpha/translate.c b/target-alpha/translate.c
index 842f915..37f2f20 100644
--- a/target-alpha/translate.c
+++ b/target-alpha/translate.c
@@ -1590,6 +1590,9 @@  static int cpu_pr_data(int pr)
         return offsetof(CPUAlphaState, shadow[pr - 32]);
     case 40 ... 63:
         return offsetof(CPUAlphaState, scratch[pr - 40]);
+
+    case 251:
+        return offsetof(CPUAlphaState, alarm_expire);
     }
     return 0;
 }
@@ -1604,6 +1607,12 @@  static void gen_mfpr(int ra, int regno)
         return;
     }
 
+    if (regno == 250) {
+        /* WALL_TIME */
+        gen_helper_get_time(cpu_ir[ra]);
+        return;
+    }
+
     /* The basic registers are data only, and unknown registers
        are read-zero, write-ignore.  */
     if (data == 0) {
@@ -1650,6 +1659,11 @@  static ExitStatus gen_mtpr(DisasContext *ctx, int rb, int regno)
         gen_helper_halt(tmp);
         return EXIT_PC_STALE;
 
+    case 251:
+        /* ALARM */
+        gen_helper_set_alarm(tmp);
+        break;
+
     default:
         /* The basic registers are data only, and unknown registers
            are read-zero, write-ignore.  */