Re: [PATCH ARM v7 09/13] mini-os: arm: time

[Stefano Stabellini <stefano.stabellini@eu.citrix.com>]

[-- Attachment #1: Type: text/plain, Size: 5415 bytes --]

On Mon, 22 Sep 2014, Dave Scott wrote:
> Hi,
> 
> On 8 Sep 2014, at 17:08, Thomas Leonard <talex5@gmail.com> wrote:
> 
> > On 8 September 2014 11:59, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> On Fri, 2014-08-08 at 16:47 +0100, Thomas Leonard wrote:
> >> 
> >> Sorry for how long I took to get to this, travel and the resulting
> >> backlog conspired against me.
> > 
> > Hi Ian,
> > 
> > I'm on holiday for a few weeks now. Please feel free to apply any
> > minor changes if you don't want to wait. In any case, I'll take
> > another look at the end of Sep.
> > 
> > Regards,
> > 
> >>> +    __asm__ __volatile__("isb;mrrc p15, 1, %0, %1, c14":"=r"(c_lo), "=r"(c_hi));
> >> 
> >> Is the isb really necessary here?
> > 
> > I don't think so.
> > 
> >>> +    return (((uint64_t) c_hi) << 32) + c_lo;
> >>> +}
> >>> +
> >>> +/* monotonic_clock(): returns # of nanoseconds passed since time_init()
> >>> + *        Note: This function is required to return accurate
> >>> + *        time even in the absence of multiple timer ticks.
> >>> + */
> >>> +uint64_t monotonic_clock(void)
> >>> +{
> >>> +    s_time_t time = ticks_to_ns(read_virtual_count() - cntvct_at_init);
> >>> +    //printk("monotonic_clock: %llu (%llu)\n", time, NSEC_TO_SEC(time));
> >> 
> >> There's quite a few of these //printk(debug) statements in this patch...
> >> 
> >>> +    return time;
> >>> +}
> >>> +
> >>> +int gettimeofday(struct timeval *tv, void *tz)
> >>> +{
> >>> +    uint64_t nsec = monotonic_clock();
> >>> +    nsec += shadow_ts.tv_nsec;
> >>> +
> >>> +    tv->tv_sec = shadow_ts.tv_sec;
> >>> +    tv->tv_sec += NSEC_TO_SEC(nsec);
> >>> +    tv->tv_usec = NSEC_TO_USEC(nsec % 1000000000UL);
> >>> +
> >>> +    return 0;
> >>> +}
> >>> +
> >>> +void set_vtimer_compare(uint64_t value) {
> >>> +    uint32_t x, y;
> >>> +
> >>> +    DEBUG("New CompareValue : %llx\n", value);
> >>> +    x = 0xFFFFFFFFULL & value;
> >>> +    y = (value >> 32) & 0xFFFFFFFF;
> >>> +
> >>> +    __asm__ __volatile__("mcrr p15, 3, %0, %1, c14"
> >>> +            ::"r"(x), "r"(y));
> >> 
> >> I think you can use
> >>        "mcrr p15, 3, %0, %H0, c14" :: "r" (value)
> >> here.
> >> 
> >> 
> >>> +
> >>> +    __asm__ __volatile__("mov %0, #0x1\n"
> >>> +            "mcr p15, 0, %0, c14, c3, 1\n" /* Enable timer and unmask the output signal */
> >>> +            "isb":"=r"(x));
> >> 
> >> x = 1 before this would avoid the mov inside the inline stuff as well as
> >> the strange looking use of an output register for a write.
> >> 
> >>> +}
> >>> +
> >>> +void unset_vtimer_compare(void) {
> >>> +    uint32_t x;
> >>> +
> >>> +    __asm__ __volatile__("mov %0, #0x2\n"
> >>> +            "mcr p15, 0, %0, c14, c3, 1\n" /* Disable timer and mask the output signal */
> >>> +            "isb":"=r"(x));
> >> 
> >> and again. You probably just want a write_timer_ctl(uiint.. val) type
> >> function.
> >> 
> >>> +}
> >>> +
> >>> +void block_domain(s_time_t until)
> >>> +{
> >>> +    uint64_t until_count = ns_to_ticks(until) + cntvct_at_init;
> >>> +    ASSERT(irqs_disabled());
> >>> +    if (read_virtual_count() < until_count)
> >>> +    {
> >>> +        set_vtimer_compare(until_count);
> >>> +        //char buf[] = "sleep\n"; (void)HYPERVISOR_console_io(CONSOLEIO_write, strlen(buf), buf);
> >>> +        __asm__ __volatile__("wfi");
> >>> +        //char wake[] = "wake\n"; (void)HYPERVISOR_console_io(CONSOLEIO_write, strlen(wake), wake);
> >> 
> >> More left over debug.
> 
> I had a play with this yesterday on my cubieboard with Mirage. Using network and vchan connections worked fine, so event channels are working ok. However when I had no external event channel input and my domain blocked on a timer, it seemed to block forever in the ‘wfi’ (as I could see by enabling these debug lines and then pressing ‘enter’ to trigger an interrupt on the console). As far as I can tell the monotonic clock is giving me sensible values, and the ‘until’ value looked sensible. For now I can work around the problem by attaching a vif to a busy network :-)
> 
> Sorry for the vagueness of the bug report. I’ll try to make a more minimal repro example when I get the time.

This reminds me of many fun memories about late debugging sessions.

What Xen version are you using?

WFI means wait-for-interrupt and it is trapped into Xen, see:

xen/arch/arm/traps.c:do_trap_hypervisor, label HSR_EC_WFI_WFE.

When the guest issues a WFI instruction, Xen blocks the vcpu on its
behalf. Any interrupt for the vcpu in question should wake it up, see
the vcpu_unblock call at the end of
xen/arch/arm/vgic.c:vgic_vcpu_inject_irq. vgic_vcpu_inject_irq is called
by the timer code, both vtimer.c (emulated time) and time.c (virtual
timer).

If something goes wrong with the wake-up, the vcpu could remain blocked.

Have fun! ;-)


> Dave
> 
> 
> >> 
> >>> +        unset_vtimer_compare();
> >>> +
> >>> +        /* Give the IRQ handler a chance to handle whatever woke us up. */
> >>> +        local_irq_enable();
> >>> +        local_irq_disable();
> >>> +    }
> >>> +}
> >> 
> >> Ian.
> >> 
> > 
> > 
> > 
> > -- 
> > Dr Thomas Leonard        http://0install.net/
> > GPG: 9242 9807 C985 3C07 44A6  8B9A AE07 8280 59A5 3CC1
> > GPG: DA98 25AE CAD0 8975 7CDA  BD8E 0713 3F96 CA74 D8BA
> 

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel