On Tue, 01 Nov 2011, Anthony Liguori wrote:
> On 10/31/2011 03:07 PM, Eric B Munson wrote:
> >When a host stops or suspends a VM it will set a flag to show this. The
> >watchdog will use these functions to determine if a softlockup is real, or the
> >result of a suspended VM.
> >
> >Signed-off-by: Eric B Munson<emunson@xxxxxxxxx>
> >---
> > arch/x86/include/asm/pvclock.h | 2 ++
> > arch/x86/kernel/kvmclock.c | 19 +++++++++++++++++++
> > 2 files changed, 21 insertions(+), 0 deletions(-)
> >
> >diff --git a/arch/x86/include/asm/pvclock.h b/arch/x86/include/asm/pvclock.h
> >index c59cc97..7d3ba41 100644
> >--- a/arch/x86/include/asm/pvclock.h
> >+++ b/arch/x86/include/asm/pvclock.h
> >@@ -13,6 +13,8 @@ void pvclock_read_wallclock(struct pvclock_wall_clock *wall,
> > struct timespec *ts);
> > void pvclock_resume(void);
> >
> >+bool kvm_check_and_clear_host_stopped(int cpu);
>
> A kvm-specific function shouldn't be declared in pvclock.h as this
> is shared code between KVM and Xen.
I'll move it for V3.
>
> >+
> > /*
> > * Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction,
> > * yielding a 64-bit result.
> >diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> >index c1a0188..8ddcfaf 100644
> >--- a/arch/x86/kernel/kvmclock.c
> >+++ b/arch/x86/kernel/kvmclock.c
> >@@ -113,6 +113,25 @@ static void kvm_get_preset_lpj(void)
> > preset_lpj = lpj;
> > }
> >
> >+bool kvm_check_and_clear_host_stopped(int cpu)
> >+{
> >+ bool ret = false;
> >+ struct pvclock_vcpu_time_info *src;
> >+
> >+ /*
> >+ * per_cpu() is safe here because this function is only called from
> >+ * timer functions where preemption is already disabled.
> >+ */
> >+ WARN_ON(!in_atomic());
> >+ src =&per_cpu(hv_clock, cpu);
> >+ if ((src->flags& PVCLOCK_GUEST_STOPPED) != 0) {
> >+ src->flags = src->flags& (~PVCLOCK_GUEST_STOPPED);
> >+ ret = true;
> >+ }
>
> It would be helpful to separate out the patches for guest enablement
> from host enablement. This is a guest function, correct?
>
Yes, this is guest only.
> Don't you need an atomic getandset here? Can the host clear this
> flag or only set it?
I didn't think they needed to be atomic because an over-write of this means that
a soft lockup message is delayed by the detector's timeout in the worst case.
If you disagree with that assessment, I can move to atomic get/set.
>
> It's ashame we don't have a proper pvclock spec in Documentation/.
> I would suggest at least adding a description of how this bit works
> in the first patch.
>
> Regards,
>
> Anthony Liguori
>
> >+
> >+ return ret;
> >+}
> >+
> > static struct clocksource kvm_clock = {
> > .name = "kvm-clock",
> > .read = kvm_clock_get_cycles,
>
Attachment:
signature.asc
Description: Digital signature
