From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754528AbcEPUFr (ORCPT ); Mon, 16 May 2016 16:05:47 -0400 Received: from mail-io0-f169.google.com ([209.85.223.169]:36675 "EHLO mail-io0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754151AbcEPUFq (ORCPT ); Mon, 16 May 2016 16:05:46 -0400 MIME-Version: 1.0 In-Reply-To: <20160516144648.GA22999@gmail.com> References: <20160516144648.GA22999@gmail.com> Date: Mon, 16 May 2016 13:05:45 -0700 X-Google-Sender-Auth: dGdXrY8pn66lT9R3P1F5lTBrLeI Message-ID: Subject: Re: [GIT PULL] EFI fix From: Linus Torvalds To: Ingo Molnar , Alex Thorlton Cc: Linux Kernel Mailing List , Matt Fleming , Thomas Gleixner , "H. Peter Anvin" , Andrew Morton , Peter Zijlstra , Borislav Petkov Content-Type: multipart/mixed; boundary=001a11c16b405ac86c0532fb263d Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --001a11c16b405ac86c0532fb263d Content-Type: text/plain; charset=UTF-8 On Mon, May 16, 2016 at 7:46 AM, Ingo Molnar wrote: > > Please pull the latest efi-urgent-for-linus git tree from: > > git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git efi-urgent-for-linus > > # HEAD: bea23c757f66d91dac8fdadd94da0cba6b0b66bc x86/efi: Fix 7th argument to efi_call() > > A leftover fix from the v4.6 cycle. I'm not pulling this. It seems to be completely broken unless I'm mis-reading things. > diff --git a/arch/x86/platform/efi/efi_stub_64.S b/arch/x86/platform/efi/efi_stub_64.S > index 92723aeae0f9..62938ffbb9f9 100644 > --- a/arch/x86/platform/efi/efi_stub_64.S > +++ b/arch/x86/platform/efi/efi_stub_64.S > @@ -43,7 +43,7 @@ ENTRY(efi_call) > FRAME_BEGIN > SAVE_XMM > mov (%rsp), %rax > - mov 8(%rax), %rax > + mov 16(%rax), %rax > subq $48, %rsp > mov %r9, 32(%rsp) > mov %rax, 40(%rsp) This code is an unmitigated disaster. It makes no sense, but the reason I refuse to pull it is that it also seems to be buggy - with or without that patch. In particular,. the SAME_XMM code saves the old stack pointer, but that's just crazy. It saves the stack pointer *AFTER* we've done that FRAME_BEGIN which will have *changed* the stack pointer, depending on whether stack frames are enabled or not. So when the code then does mov (%rsp), %rax we now move that old stack pointer into %rax, but the offset off that stack pointer will depend on whether that FRAME_BEGIN saved off %rbp or not. So that whole 8-vs-16 offset confusion depends on the frame pointer! If frame pointers were enabled, it will be 16. If they weren't, it will be 8. That patch that changes it from 8 to 16 will just move the bug around. Before, it was correct when frame pointers were disabled and buggy otherwise. Now, it's correct if frame pointers are enabled, and buggy otherwise. I may be missing something, but I think that commit is pure garbage. I think the right fix is to just get rid of that silly conditional frame pointer thing, and always use frame pointers in this stub function. And then we don't need that (odd) load to get the old stack pointer into %rax - we can just use the frame pointer. Something like the attached completely untested patch. But maybe I was missing something. Maybe my patch is crap and the patch above is right for some reason that completely evades me. Since this apparently only affects the SGI EFI stuff, can you please test this, Alex? Linus --001a11c16b405ac86c0532fb263d Content-Type: text/plain; charset=US-ASCII; name="patch.diff" Content-Disposition: attachment; filename="patch.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_ioafx8v40 IGFyY2gveDg2L3BsYXRmb3JtL2VmaS9lZmlfc3R1Yl82NC5TIHwgOSArKysrLS0tLS0KIDEgZmls ZSBjaGFuZ2VkLCA0IGluc2VydGlvbnMoKyksIDUgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEv YXJjaC94ODYvcGxhdGZvcm0vZWZpL2VmaV9zdHViXzY0LlMgYi9hcmNoL3g4Ni9wbGF0Zm9ybS9l ZmkvZWZpX3N0dWJfNjQuUwppbmRleCA5MjcyM2FlYWUwZjkuLjBhOTk1YWFkZGNmYyAxMDA2NDQK LS0tIGEvYXJjaC94ODYvcGxhdGZvcm0vZWZpL2VmaV9zdHViXzY0LlMKKysrIGIvYXJjaC94ODYv cGxhdGZvcm0vZWZpL2VmaV9zdHViXzY0LlMKQEAgLTExLDcgKzExLDYgQEAKICNpbmNsdWRlIDxh c20vbXNyLmg+CiAjaW5jbHVkZSA8YXNtL3Byb2Nlc3Nvci1mbGFncy5oPgogI2luY2x1ZGUgPGFz bS9wYWdlX3R5cGVzLmg+Ci0jaW5jbHVkZSA8YXNtL2ZyYW1lLmg+CiAKICNkZWZpbmUgU0FWRV9Y TU0JCQlcCiAJbW92ICVyc3AsICVyYXg7CQkJXApAQCAtNDAsMTAgKzM5LDEwIEBACiAJbW92ICgl cnNwKSwgJXJzcAogCiBFTlRSWShlZmlfY2FsbCkKLQlGUkFNRV9CRUdJTgorCXB1c2hxICVyYnAK Kwltb3ZxICVyc3AsJXJicAogCVNBVkVfWE1NCi0JbW92ICglcnNwKSwgJXJheAotCW1vdiA4KCVy YXgpLCAlcmF4CisJbW92IDE2KCVyYnApLCAlcmF4CiAJc3VicSAkNDgsICVyc3AKIAltb3YgJXI5 LCAzMiglcnNwKQogCW1vdiAlcmF4LCA0MCglcnNwKQpAQCAtNTMsNiArNTIsNiBAQCBFTlRSWShl ZmlfY2FsbCkKIAljYWxsIColcmRpCiAJYWRkcSAkNDgsICVyc3AKIAlSRVNUT1JFX1hNTQotCUZS QU1FX0VORAorCXBvcHEgJXJicAogCXJldAogRU5EUFJPQyhlZmlfY2FsbCkK --001a11c16b405ac86c0532fb263d--