All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marcelo Tosatti <mtosatti@redhat.com>
To: Florian Westphal <fw@strlen.de>
Cc: kvm@vger.kernel.org, avi@redhat.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] kvm: disable stealtime via reboot notifier to avoid mem corruption
Date: Tue, 14 Aug 2012 12:32:21 -0300	[thread overview]
Message-ID: <20120814153221.GD14582@amt.cnet> (raw)
In-Reply-To: <20120814091506.GA13610@breakpoint.cc>

On Tue, Aug 14, 2012 at 11:15:06AM +0200, Florian Westphal wrote:
> Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > On Fri, Aug 10, 2012 at 12:36:22PM +0200, Florian Westphal wrote:
> > > --- a/arch/x86/kernel/kvmclock.c
> > > +++ b/arch/x86/kernel/kvmclock.c
> > > @@ -191,7 +191,6 @@ static void kvm_crash_shutdown(struct pt_regs *regs)
> > >  static void kvm_shutdown(void)
> > >  {
> > >  	native_write_msr(msr_kvm_system_time, 0, 0);
> > > -	kvm_disable_steal_time();
> > >  	native_machine_shutdown();
> > >  }
> > This part below will introduce a bug for shutdown. Can you retest with
> > the addition of kvm_disable_steal_time to kvm_pv_guest_cpu_reboot only,
> > retest and resend please?
> 
> I can, but the problem with kvm_disable_steal_time() in
> kvmclock.c is that with CONFIG_KVM_CLOCK=n the entire
> file won't be compiled.
> 
> And steal time doesn't depend on CONFIG_KVM_CLOCK=y.
> So if removing it there is a bug leaving it in only avoids
> that bug for CONFIG_KVM_CLOCK=y.

I meant to add kvm_disable_steal_time() call to arch/x86/kernel/kvm.c,
the first part of your patch, here:

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index c1d61ee..1596cc8 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -354,6 +354,7 @@ static void kvm_pv_guest_cpu_reboot(void *unused)
      if (kvm_para_has_feature(KVM_FEATURE_PV_EOI))
              wrmsrl(MSR_KVM_PV_EOI_EN, 0);
      kvm_pv_disable_apf();
+     kvm_disable_steal_time();
 }
 
 static int kvm_pv_reboot_notify(struct notifier_block *nb,

Can you test only that part and resend? 

The call to kvm_disable_steal_time() in kvmclock.c should not be
removed.

      reply	other threads:[~2012-08-14 15:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-10 10:36 [PATCH] kvm: disable stealtime via reboot notifier to avoid mem corruption Florian Westphal
2012-08-13 21:52 ` Marcelo Tosatti
2012-08-14  9:15   ` Florian Westphal
2012-08-14 15:32     ` Marcelo Tosatti [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120814153221.GD14582@amt.cnet \
    --to=mtosatti@redhat.com \
    --cc=avi@redhat.com \
    --cc=fw@strlen.de \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.