linux-kernel - Re: [PATCH v8 3/3] x86, mce: Add __mcsafe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Mon, 1 Feb 2016 15:16:54 -0800
From:	Dan Williams <dan.j.williams@...el.com>
To:	Tony Luck <tony.luck@...il.com>
Cc:	Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>,
	Andy Lutomirski <luto@...capital.net>,
	linux-nvdimm <linux-nvdimm@...1.01.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Robert <elliott@....com>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH v8 3/3] x86, mce: Add __mcsafe_copy()

On Mon, Feb 1, 2016 at 3:10 PM, Tony Luck <tony.luck@...il.com> wrote:
>> The most optimal way of alternatively calling two functions would be
>> something like this, IMO:
>>
>> alternative_call(memcpy, __mcsafe_copy, X86_FEATURE_MCRECOVERY,
>>                  ASM_OUTPUT2("=a" (mcsafe_ret.trapnr), "=d" (mcsafe_ret.remain)),
>>                  "D" (dst), "S" (src), "d" (len));
>>
>> I hope I've not messed up the calling convention but you want the inputs
>> in %rdi, %rsi, %rdx and the outputs in %rax, %rdx, respectively. Just
>> check the asm gcc generates and do not trust me :)
>>
>> The other thing you probably would need to do is create our own
>> __memcpy() which returns struct mcsafe_ret so that the signatures of
>> both functions match.
>>
>> Yeah, it is a bit of jumping through hoops but this way we do a CALL
>> <func_ptr> directly in asm, without any JMPs or NOPs padding the other
>> alternatives methods add.
>>
>> But if you don't care about a small JMP and that is not a hot path, you
>> could do the simpler:
>>
>>         if (static_cpu_has(X86_FEATURE_MCRECOVERY))
>>                 return __mcsafe_copy(...);
>>
>>         return memcpy();
>>
>> which adds a JMP or a 5-byte NOP depending on the X86_FEATURE_MCRECOVERY
>> setting.
>
> Dan,
>
> What do you want the API to look like at the point you make a call
> in the libnvdimm code?  Something like:
>
>         r = nvcopy(dst, src, len);
>
> where the innards of nvcopy() does the check for X86_FEATURE_MCE_RECOVERY?
>
> What is useful to you in the return value? The low level __mcsafe_copy() returns
> both a remainder and a trap number. But in your case I don't think you
> need the trap
> number (if the remaining count is not zero, then there must have been a #MC. #PF
> isn't an option for you, right?

RIght, we don't need a trap number just an error.  This is the v1
attempt at integrating mcsafe_copy:

https://lists.01.org/pipermail/linux-nvdimm/2016-January/003869.html

I think the only change needed is to use
static_cpu_has(X86_FEATURE_MCRECOVERY) like so:

+static inline int arch_memcpy_from_pmem(void *dst, const void __pmem *src,
+ size_t n)
+{
+ if (static_cpu_has(X86_FEATURE_MCRECOVERY)) {
+ struct mcsafe_ret ret;
+
+ ret = __mcsafe_copy(dst, (void __force *) src, n);
+ if (ret.remain)
+ return -EIO;
+ return 0;
+ }
+ memcpy(dst, (void __force *) src, n);
+ return 0;
+}