Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752987AbcCNRRa (ORCPT ); Mon, 14 Mar 2016 13:17:30 -0400 Received: from mail-ob0-f178.google.com ([209.85.214.178]:36858 "EHLO mail-ob0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751889AbcCNRR2 (ORCPT ); Mon, 14 Mar 2016 13:17:28 -0400 MIME-Version: 1.0 In-Reply-To: References: <20160314120202.GD15800@pd.tnic> From: Andy Lutomirski Date: Mon, 14 Mar 2016 10:17:07 -0700 Message-ID: Subject: Re: [PATCH v4 2/5] x86/msr: Carry on after a non-"safe" MSR access fails without !panic_on_oops To: Linus Torvalds Cc: Paolo Bonzini , xen-devel , Arjan van de Ven , Borislav Petkov , X86 ML , Andrew Morton , KVM list , Andy Lutomirski , "linux-kernel@vger.kernel.org" , Peter Zijlstra Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1911 Lines: 42 On Mon, Mar 14, 2016 at 10:11 AM, Linus Torvalds wrote: > > On Mar 14, 2016 10:05 AM, "Andy Lutomirski" wrote: >> >> We could probably remove that check and let custom fixups run early. >> I don't see any compelling reason to keep them disabled. That should >> probably be a separate change, though. > > Or we could just use the existing wrmsr_safe() code and not add this new > special code at all. > > Look, why are you doing this? We should get rid of the difference between > wrmsr and wrmsr_safe(), not make it bigger. > > Just make everything safe. There has never in the history of anything been > an advantage to making things oops and to making things more complicated. > > Why is rd/wrmsr() so magically important? Because none of this is actually safe unless the code that calls whatever_safe actually does something intelligent with the return value. I think that most code that does rdmsr or wrmsr actually thinks "I know that this MSR exists and I want to access it" and the code may actually malfunction if the access fails. So I think we really do want the warning so we can fix the bugs if they happen. And I wouldn't be at all surprised if there's a bug or two in perf where some perf event registers itself incorrectly in some cases because it messed up the probe sequence. We don't want to totally ignore the resulting failure of the perf event -- we want to get a warning so that we know about the bug. Or suppose we're writing some important but easy-to-miss MSR, like PAT or one of the excessive number of system call setup MSRs. If the write fails, the system could easily still appear to work until something unfortunate happens. So yes, let's please warn. I'm okay with removing the panic_on_oops thing though. (But if anyone suggests that we should stop OOPSing on bad kernel page faults, I *will* fight back.) --Andy