On Mon, Mar 14, 2016 at 11:04 AM, Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > On Mon, Mar 14, 2016 at 10:17 AM, Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote: >> >> 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.) > > Bad kernel page faults are something completely different. They would > be actual bugs regardless. > > The MSR thing has *often* been just silly "this CPU doesn't do that > MSR". Generic bootup setup code etc that just didn't know or care > about the particular badly documented rule for one particular random > CPU version and stepping. > > In fact, when I say "often", I suspect I should really just say > "always". I don't think we've ever found a case where oopsing would > have been the right thing. But it has definitely caused lots of > problems, especially in the early paths where your code doesn't even > work right now. I can fix that part by literally deleting a few lines of code. I just need to audit things to make sure that won't break anything. > > Now, when it comes to the warning, I guess I could live with it, but I > think it's stupid to make this a low-level exception handler thing. > > So what I think should be done: > > - make sure that wr/rdmsr_safe() actually works during very early > init. At some point it didn't. > > - get rid of the current wrmsr/rdmsr *entirely*. It's shit. > > - Add this wrapper: > > #define wrmsr(msr, low, high) \ > WARN_ON_ONCE(wrmsr_safe(msr, low, high)) > > and be done with it. We could even decide to make that WARN_ON_ONCE() > be something we could configure out, because it's really a debugging > thing and isn't like it should be all that fatal. > > None of this insane complicated crap that buys us exactly *nothing*, > and depends on fancy new exception handling support etc etc. > > So what's the downside to just doing this simple thing? More code size increase and extra branches on fast paths. Using the fancy new exception handling means we get to have the check and the warning completely out of line. In fact, I'm tempted to change the _safe variants to use the fancy new handling as well (once it works in early boot) to shorten the code size and remove some asm code. A couple of the wrmsr users actually care about performance. These are the ones involved in context switching and, to a lesser extent, in switching in and out of guest mode. --Andy -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
