Re: [PATCH ARM v8 1/4] mini-os: arm: time

[Ian Campbell <Ian.Campbell@citrix.com>]

On Thu, 2014-11-13 at 16:29 +0000, Thomas Leonard wrote:
> On 27 October 2014 10:34, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Sun, 2014-10-26 at 09:51 +0000, Thomas Leonard wrote:
> >> On 21 October 2014 11:50, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Fri, 2014-10-03 at 10:20 +0100, Thomas Leonard wrote:
> >> >> Based on an initial patch by Karim Raslan.
> >> >>
> >> >> Signed-off-by: Karim Allah Ahmed <karim.allah.ahmed@gmail.com>
> >> >> Signed-off-by: Thomas Leonard <talex5@gmail.com>
> >> >
> >> > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> >> >
> >> >> +/* Wall-clock time is not currently available on ARM, so this is always zero for now:
> >> >> + * http://wiki.xenproject.org/wiki/Xen_ARM_TODO#Expose_Wallclock_time_to_guests
> >> >
> >> > I have some slightly hacky patches for this, I really should dust them
> >> > off and submit them...
> >> >
> >> >> +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);
> >> >> +        __asm__ __volatile__("wfi");
> >> >> +        unset_vtimer_compare();
> >> >> +
> >> >> +        /* Give the IRQ handler a chance to handle whatever woke us up. */
> >> >> +        local_irq_enable();
> >> >> +        local_irq_disable();
> >> >> +    }
> >> >
> >> > Just wondering, is this not roughly equivalent to a wfi loop with
> >> > interrupts enabled?
> >>
> >> I'm not quite sure what you mean.
> >>
> >> If we enable interrupts before the wfi then I think the following could occur:
> >>
> >> 1. Application checks for work, finds none and calls block_domain.
> >> 2. block_domain enables interrupts.
> >> 3. An interrupt occurs.
> >> 4. The interrupt handler sets a flag indicating work to do.
> >> 5. wfi is called, putting the domain to sleep, even though there is work to do.
> >>
> >> Enabling IRQs after block_domain ensures we can't sleep while we have
> >> work to do.
> >
> > Ah, yes.
> 
> So, can this patch be applied as-is now?

We are now post-rc2 in the 4.5.0 release process, so the answer would be
"needs a release exception, but it's a feature so probably not" (and it
would have been a bit dubious towards the end of October too, which was
post rc1, and feature freeze was the end of September in any case).

However this is part of a new mini-os port which isn't even hooked into
the main build system yet (AFAICT), so in that sense it is utterly
harmless to apply. On the other hand there is a bunch more patches to
come which are needed to make the mini-os port actually useful, and I'm
not sure those are all utterly harmless e.g. to common or x86 code (as
in I've not gone looked at the diffstat for the remaining patches), so
in that sense there's no harm waiting for 4.6 development to open.

I defer to the release manager (Konrad, CCd) on this...