From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH ARM v8 1/4] mini-os: arm: time Date: Fri, 14 Nov 2014 10:29:26 +0000 Message-ID: <1415960966.21321.30.camel@citrix.com> References: <1412328051-20015-1-git-send-email-talex5@gmail.com> <1412328051-20015-2-git-send-email-talex5@gmail.com> <1413888616.23337.22.camel@citrix.com> <1414406079.31057.6.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XpE8D-0005Mu-Es for xen-devel@lists.xenproject.org; Fri, 14 Nov 2014 10:29:33 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Thomas Leonard , Konrad Rzeszutek Wilk Cc: Samuel Thibault , Stefano Stabellini , "xen-devel@lists.xenproject.org" , David Scott , Anil Madhavapeddy List-Id: xen-devel@lists.xenproject.org On Thu, 2014-11-13 at 16:29 +0000, Thomas Leonard wrote: > On 27 October 2014 10:34, Ian Campbell wrote: > > On Sun, 2014-10-26 at 09:51 +0000, Thomas Leonard wrote: > >> On 21 October 2014 11:50, Ian Campbell 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 > >> >> Signed-off-by: Thomas Leonard > >> > > >> > Acked-by: Ian Campbell > >> > > >> >> +/* 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...