Re: [PATCHV5 3/3] x86, ras: Add __mcsafe_copy() function to recover from machine checks

Andy Lutomirski
2016-01-03T03:40:51Z

On Fri, Jan 1, 2016 at 2:19 PM, Tony Luck <tony.luck@gmail.com> wrote:
> Somehow this didn't get sent ... found it in the "Drafts" folder. But
> it's rubbish, skip to the
> bottom.
>
> On Thu, Dec 31, 2015 at 12:30 PM, Tony Luck <tony.luck@gmail.com> wrote:
>> I switched to BIAS 0xC0000000 ... and now I should get class 1 entries
>> (bit31=0, bit30=1).
>>
>> New patch series coming soon.
>
> Or not :-(
>
> arch/x86/lib/lib.a(memcpy_64.o):(__ex_table+0x4): relocation truncated
> to fit: R_X86_64_PC32 against `.fixup'
> arch/x86/lib/lib.a(memcpy_64.o):(__ex_table+0xc): relocation truncated
> to fit: R_X86_64_PC32 against `.fixup'
> ...
>
> I guess it was something like this that made you do the 0x20000000 and
> subtract the BIAS?
>
> I have a bad feeling that we may not really have four classes, just three:
>
> 00: no funny arithmetic
> 10: BIAS = 0x80000000 ... doesn't trigger truncation warning because
> sign bit is set
> 11: BIAS = 0x40000000 ... ditto
> 01: BIAS = ? ... Is there some magic value for BIAS that gets this?
>
> --- end of Draft ... now to the real bit
>
> Not sure why I was hung up on *subtracting* values to get the desired
> class bits. Just
> blindly copying the initial case from your patch?
>
> If you can't get from A to B one way, try going around the other
> direction. Subtracting
> 0xC0000000 is the same as adding 0x40000000 (when playing with u32 values).
> That doesn't upset the linker.
>
> I rebased:
> git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git mcsafev6
>
> still needs a little cleanup, but it all works, and seems to be a much
> cleaner approach. So clean that I wonder whether I really need
> the CONFIG_MCE_KERNEL_RECOVERY any more?? The only
> place it is used now is around the __mcsafe_copy()
>

Looks nice!

It might a bit clearer if you rename fix_class2 to fix_class_ex, etc,
and then use the C99 syntax for the table:

allclasses ... = {
[EXTABLE_CLASS_WHATEVER >> 30] = fix_class_whatever,
...
};

you could try "[extable_class(EXTABLE_CLASS_WHATEVER)] = fix_class_whatever"

Maybe rename EXTABLE_CLASS_FAULT to EXTABLE_CLASS_FAULT_OR_MC?

I might still attack this code later to add the indirect fixup idea
rather than just returning the fault number in EAX.

--Andy