From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754469AbbL0CQO (ORCPT ); Sat, 26 Dec 2015 21:16:14 -0500 Received: from mail-ob0-f170.google.com ([209.85.214.170]:36190 "EHLO mail-ob0-f170.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754145AbbL0CQN (ORCPT ); Sat, 26 Dec 2015 21:16:13 -0500 MIME-Version: 1.0 In-Reply-To: References: <20151224214632.GF4128@pd.tnic> <20151225114937.GA862@pd.tnic> <5FBC1CF1-095B-466D-85D6-832FBFA98364@intel.com> <20151226103252.GA21988@pd.tnic> From: Andy Lutomirski Date: Sat, 26 Dec 2015 18:15:52 -0800 Message-ID: Subject: Re: [PATCHV5 3/3] x86, ras: Add __mcsafe_copy() function to recover from machine checks To: Tony Luck Cc: Borislav Petkov , linux-nvdimm , X86 ML , "elliott@hpe.com" , "linux-mm@kvack.org" , Andrew Morton , "Williams, Dan J" , Ingo Molnar , "linux-kernel@vger.kernel.org" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sat, Dec 26, 2015 at 6:08 PM, Tony Luck wrote: > On Sat, Dec 26, 2015 at 6:54 AM, Andy Lutomirski wrote: >> On Dec 26, 2015 6:33 PM, "Borislav Petkov" wrote: >>> Andy, why is that? It makes the exception handling much simpler this way... >>> >> >> I like the idea of moving more logic into C, but I don't like >> splitting the logic across files and adding nasty special cases like >> this. >> >> But what if we generalized it? An extable entry gives a fault IP and >> a landing pad IP. Surely we can squeeze a flag bit in there. > > The clever squeezers have already been here. Instead of a pair > of 64-bit values for fault_ip and fixup_ip they managed with a pair > of 32-bit values that are each the relative offset of the desired address > from the table location itself. > > We could make one of them 31-bits (since even an "allyesconfig" kernel > is still much smaller than a gigabyte) to free a bit for a flag. But there > are those external tools to pre-sort exception tables that would all > need to be fixed too. > > Or we could direct the new fixups into a .fixup2 ELF section and put > begin/end labels around that ... so we could check the address of the > fixup to see whether it is a legacy or new format entry. > Either of those sounds good to me. >> set the bit, you get an extended extable entry. Instead of storing a >> landing pad, it stores a pointer to a handler descriptor: >> >> struct extable_handler { >> bool (*handler)(struct pt_regs *, struct extable_handler *, ...): >> }; >> >> handler returns true if it handled the error and false if it didn't. > > It may be had to call that from the machine check handler ... the > beauty of just patching the IP and returning from the handler was > that it got us out of machine check context. Your handler will need to know that it's in machine check context :) In most cases (e.g. yours), the handler should just modify regs and return. > >> The "..." encodes the fault number, error code, cr2, etc. Maybe it >> would be "unsigned long exception, const struct extable_info *info" >> where extable_info contains a union? I really wish C would grow up >> and learn about union types. > > All this is made more difficult because the h/w doesn't give us > all the things we might want to know (e.g. the virtual address). > We just have a physical address (which may be missing some > low order bits). True. I'm afraid that nothing I suggest can possibly help you there. Anyhow, this could be a decent signature: bool (*handler)(struct pt_regs *, struct extable_handler *, unsigned int exception, unsigned long error_code, unsigned long extra): If exception is X86_TRAP_PF, then extra is CR2. If exception is X86_TRAP_MC, then extra is however much of the PA you know. --Andy