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

[Dave Scott <Dave.Scott@xxxxxxxxxx>]

Hi,

On 8 Sep 2014, at 17:08, Thomas Leonard <talex5@xxxxxxxxx> wrote:

> On 8 September 2014 11:59, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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.

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


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