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

[Thomas Leonard <talex5@xxxxxxxxx>]

On 22 September 2014 11:35, Dave Scott <Dave.Scott@xxxxxxxxxx> wrote:
> 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

After discussing this with Dave, it looks like this was a problem in a
previous version of my patches*, which didn't enable the timer
interrupt. This series (v7) does enable the timer and should work
correctly.

* https://github.com/talex5/xen/releases/tag/minios-v0.3


-- 
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