From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andy Lutomirski Subject: Re: [PATCH v2 1/2] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops Date: Mon, 21 Sep 2015 10:43:44 -0700 Message-ID: References: <130a3b7ef4788baae3a6fe71293ab17442bc9a0a.1442793572.git.luto@kernel.org> <20150921084642.GA30984@gmail.com> <56003506.1050100@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <56003506.1050100@linux.intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Arjan van de Ven Cc: Andrew Morton , KVM list , Peter Zijlstra , the arch/x86 maintainers , Linux Kernel Mailing List , xen-devel , Paolo Bonzini , Thomas Gleixner , Linus Torvalds , Ingo Molnar List-Id: xen-devel@lists.xenproject.org On Mon, Sep 21, 2015 at 9:49 AM, Arjan van de Ven wrote: > On 9/21/2015 9:36 AM, Linus Torvalds wrote: >> >> On Mon, Sep 21, 2015 at 1:46 AM, Ingo Molnar wrote: >>> >>> >>> Linus, what's your preference? >> >> >> So quite frankly, is there any reason we don't just implement >> native_read_msr() as just >> >> unsigned long long native_read_msr(unsigned int msr) >> { >> int err; >> unsigned long long val; >> >> val = native_read_msr_safe(msr, &err); >> WARN_ON_ONCE(err); >> return val; >> } >> >> Note: no inline, no nothing. Just put it in arch/x86/lib/msr.c, and be >> done with it. I don't see the downside. >> >> How many msr reads are so critical that the function call >> overhead would matter? > > > if anything qualifies it'd be switch_to() and friends. And maybe the KVM user return notifier. Unfortunately, switch_to might gain another two MSR accesses at some point if we decide to fix the bugs in there. Sigh. > > note that I'm not entirely happy about the notion of "safe" MSRs. > They're safe as in "won't fault". > Reading random MSRs isn't a generic safe operation though, but the name sort > of gives people > the impression that it is. Even with _safe variants, you still need to KNOW > the MSR exists (by means > of CPUID or similar) unfortunately. > I tend to agree. Anyway, the fully out-of-line approach isn't obviously a bad idea, and it simplifies the whole mess (we can drop most of the paravirt patches, too). I'll give it a try and see what happens. --Andy