All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Andy Lutomirski <luto@kernel.org>
Cc: x86@kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	KVM list <kvm@vger.kernel.org>,
	Arjan van de Ven <arjan@linux.intel.com>,
	xen-devel <Xen-devel@lists.xen.org>,
	linux-kernel@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andrew Morton <akpm@linux-foundation.org>,
	"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops
Date: Thu, 17 Sep 2015 09:19:20 +0200	[thread overview]
Message-ID: <20150917071920.GA14296@gmail.com> (raw)
In-Reply-To: <cover.1442445946.git.luto@kernel.org>


* Andy Lutomirski <luto@kernel.org> wrote:

> Setting CONFIG_PARAVIRT=y has an unintended side effect: it silently
> turns all rdmsr and wrmsr operations into the safe variants without
> any checks that the operations actually succeed.
> 
> This is IMO awful: it papers over bugs.  In particular, KVM gueests
> might be unwittingly depending on this behavior because
> CONFIG_KVM_GUEST currently depends on CONFIG_PARAVIRT.  I'm not
> aware of any such problems, but applying this series would be a good
> way to shake them out.
> 
> Fix it so that the MSR operations work the same on CONFIG_PARAVIRT=n
> and CONFIG_PARAVIRT=y as long as Xen isn't being used.  The Xen
> maintainers are welcome to make a similar change on top of this.
> 
> Since there's plenty of time before the next merge window, I think
> we should apply and fix anything that breaks.

No, I think we should at most generate a warning instead, and not crash the kernel 
via rdmsr()!

Most big distro kernels on bare metal have CONFIG_PARAVIRT=y (I checked Ubuntu and 
Fedora), so we are potentially exposing a lot of users to problems.

Crashing the bootup on an unknown MSR is bad. Many MSR reads and writes are 
non-critical and returning the 'safe' result is much better than crashing or 
hanging the bootup.

( We should double check that rdmsr()/wrmsr() results are never left 
  uninitialized, but are set to zero or so, for cases where the return code is not 
  checked. )

Thanks,

	Ingo

  parent reply	other threads:[~2015-09-17  7:19 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-16 23:33 [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops Andy Lutomirski
2015-09-16 23:33 ` [PATCH 1/3] x86/paravirt: Add _safe to the read_msr and write_msr PV hooks Andy Lutomirski
2015-09-16 23:33 ` Andy Lutomirski
2015-09-16 23:33 ` [PATCH 2/3] x86/paravirt: Add paravirt_{read, write}_msr Andy Lutomirski
2015-09-16 23:33 ` [PATCH 2/3] x86/paravirt: Add paravirt_{read,write}_msr Andy Lutomirski
2015-09-16 23:33 ` [PATCH 3/3] x86/paravirt: Make "unsafe" MSR accesses unsafe even if PARAVIRT=y Andy Lutomirski
2015-09-16 23:33 ` Andy Lutomirski
2015-09-17  7:19 ` Ingo Molnar [this message]
2015-09-17  9:31   ` [PATCH 0/3] x86/paravirt: Fix baremetal paravirt MSR ops Borislav Petkov
2015-09-17 11:22     ` H. Peter Anvin
2015-09-17 11:22     ` H. Peter Anvin
2015-09-17 11:39     ` Paolo Bonzini
2015-09-17 15:27       ` Borislav Petkov
2015-09-17 15:27       ` Borislav Petkov
2015-09-17 15:32         ` Andrew Cooper
2015-09-17 15:32         ` [Xen-devel] " Andrew Cooper
2015-09-17 15:37           ` Borislav Petkov
2015-09-17 15:37           ` [Xen-devel] " Borislav Petkov
2015-09-17 11:39     ` Paolo Bonzini
2015-09-17  9:31   ` Borislav Petkov
2015-09-17 15:23   ` Andy Lutomirski
2015-09-17 15:27     ` Arjan van de Ven
2015-09-17 15:29       ` Paolo Bonzini
2015-09-17 15:31         ` Arjan van de Ven
2015-09-17 15:31         ` Arjan van de Ven
2015-09-17 15:33           ` Paolo Bonzini
2015-09-17 15:33           ` Paolo Bonzini
2015-09-17 15:29       ` Paolo Bonzini
2015-09-17 15:27     ` Arjan van de Ven
2015-09-17 17:30     ` Ingo Molnar
2015-09-17 18:51       ` Andy Lutomirski
2015-09-17 18:51       ` Andy Lutomirski
2015-09-17 17:30     ` Ingo Molnar
2015-09-17 15:23   ` Andy Lutomirski
2015-09-17  7:19 ` Ingo Molnar
2015-09-17  8:58 ` Peter Zijlstra
2015-09-17  8:58 ` Peter Zijlstra
2015-09-17 11:40   ` Paolo Bonzini
2015-09-17 12:27     ` Peter Zijlstra
2015-09-17 15:17       ` Andy Lutomirski
2015-09-17 15:17         ` Peter Zijlstra
2015-09-17 15:17         ` Peter Zijlstra
2015-09-17 15:26           ` Andy Lutomirski
2015-09-17 15:26           ` Andy Lutomirski
2015-09-17 15:29             ` Paolo Bonzini
2015-09-17 15:29             ` Paolo Bonzini
2015-09-17 15:17       ` Andy Lutomirski
2015-09-17 12:27     ` Peter Zijlstra
2015-09-17 11:40   ` Paolo Bonzini
2015-09-17  9:10 ` Andrew Cooper
2015-09-17  9:10 ` [Xen-devel] " Andrew Cooper
2015-09-17 15:11   ` Boris Ostrovsky
2015-09-17 15:11   ` Boris Ostrovsky
  -- strict thread matches above, loose matches on Subject: below --
2015-09-16 23:33 Andy Lutomirski

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=20150917071920.GA14296@gmail.com \
    --to=mingo@kernel.org \
    --cc=Xen-devel@lists.xen.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@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.