From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751542AbdBAW7x (ORCPT ); Wed, 1 Feb 2017 17:59:53 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:51244 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750939AbdBAW7v (ORCPT ); Wed, 1 Feb 2017 17:59:51 -0500 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org 7A2CF607BE Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=jhugo@codeaurora.org Subject: Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() To: Ard Biesheuvel , leif.lindholm@linaro.org, mark.rutland@arm.com, Ingo Molnar , Thomas Gleixner , "H . Peter Anvin" References: <1485971102-23330-1-git-send-email-ard.biesheuvel@linaro.org> <1485971102-23330-2-git-send-email-ard.biesheuvel@linaro.org> Cc: linux-arm-kernel@lists.infradead.org, riku.voipio@linaro.org, matt@codeblueprint.co.uk, linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org From: Jeffrey Hugo Message-ID: <08ff3f33-d97d-1efb-a05d-5ed1fccd2859@codeaurora.org> Date: Wed, 1 Feb 2017 15:59:48 -0700 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/1/2017 2:08 PM, Jeffrey Hugo wrote: > On 2/1/2017 10:45 AM, Ard Biesheuvel wrote: >> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(), >> after which unaligned accesses to RAM are no longer supported. >> >> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the >> kernel") fixed an issue in the memory map handling of the stub FDT code, >> but inadvertently created an issue with such firmwares, by moving some >> of the FDT manipulation to after the invocation of ExitBootServices(). >> Given that the stub's libfdt implementation uses the ordinary, >> accelerated >> string functions, which rely on hardware handling of unaligned accesses, >> manipulating the FDT with the MMU off may result in alignment faults. >> >> So fix the situation by moving the update_fdt_memmap() call into the >> callback function invoked by efi_exit_boot_services() right before it >> calls the ExitBootServices() UEFI service (which is arguably a better >> place for it anyway) >> >> Note that disabling the MMU in ExitBootServices() is not compliant with >> the UEFI spec, and carries great risk due to the fact that switching from >> cached to uncached memory accesses halfway through compiler generated >> code >> (i.e., involving a stack) can never be done in a way that is >> architecturally >> safe. >> >> Cc: >> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the >> kernel") >> Tested-by: Riku Voipio >> Signed-off-by: Ard Biesheuvel > > NACK, please. This causes a regression on my platform, in the form of > an assert in UEFI once ExitBootServices() is called, per initial > testing. I'll do more testing to determine why. > Sorry, false alarm. The assert appears to have been the result of a bad tree and a bad target configuration, and was not reproduced on another setup, nor did the assert make sense in context with this change. I withdraw my NACK. -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeffrey Hugo Subject: Re: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Date: Wed, 1 Feb 2017 15:59:48 -0700 Message-ID: <08ff3f33-d97d-1efb-a05d-5ed1fccd2859@codeaurora.org> References: <1485971102-23330-1-git-send-email-ard.biesheuvel@linaro.org> <1485971102-23330-2-git-send-email-ard.biesheuvel@linaro.org> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Sender: linux-efi-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Ard Biesheuvel , leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, mark.rutland-5wv7dgnIgG8@public.gmane.org, Ingo Molnar , Thomas Gleixner , "H . Peter Anvin" Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, riku.voipio-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org, matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-efi@vger.kernel.org On 2/1/2017 2:08 PM, Jeffrey Hugo wrote: > On 2/1/2017 10:45 AM, Ard Biesheuvel wrote: >> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(), >> after which unaligned accesses to RAM are no longer supported. >> >> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the >> kernel") fixed an issue in the memory map handling of the stub FDT code, >> but inadvertently created an issue with such firmwares, by moving some >> of the FDT manipulation to after the invocation of ExitBootServices(). >> Given that the stub's libfdt implementation uses the ordinary, >> accelerated >> string functions, which rely on hardware handling of unaligned accesses, >> manipulating the FDT with the MMU off may result in alignment faults. >> >> So fix the situation by moving the update_fdt_memmap() call into the >> callback function invoked by efi_exit_boot_services() right before it >> calls the ExitBootServices() UEFI service (which is arguably a better >> place for it anyway) >> >> Note that disabling the MMU in ExitBootServices() is not compliant with >> the UEFI spec, and carries great risk due to the fact that switching from >> cached to uncached memory accesses halfway through compiler generated >> code >> (i.e., involving a stack) can never be done in a way that is >> architecturally >> safe. >> >> Cc: >> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the >> kernel") >> Tested-by: Riku Voipio >> Signed-off-by: Ard Biesheuvel > > NACK, please. This causes a regression on my platform, in the form of > an assert in UEFI once ExitBootServices() is called, per initial > testing. I'll do more testing to determine why. > Sorry, false alarm. The assert appears to have been the result of a bad tree and a bad target configuration, and was not reproduced on another setup, nor did the assert make sense in context with this change. I withdraw my NACK. -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project. From mboxrd@z Thu Jan 1 00:00:00 1970 From: jhugo@codeaurora.org (Jeffrey Hugo) Date: Wed, 1 Feb 2017 15:59:48 -0700 Subject: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() In-Reply-To: References: <1485971102-23330-1-git-send-email-ard.biesheuvel@linaro.org> <1485971102-23330-2-git-send-email-ard.biesheuvel@linaro.org> Message-ID: <08ff3f33-d97d-1efb-a05d-5ed1fccd2859@codeaurora.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2/1/2017 2:08 PM, Jeffrey Hugo wrote: > On 2/1/2017 10:45 AM, Ard Biesheuvel wrote: >> Some AArch64 UEFI implementations disable the MMU in ExitBootServices(), >> after which unaligned accesses to RAM are no longer supported. >> >> Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the >> kernel") fixed an issue in the memory map handling of the stub FDT code, >> but inadvertently created an issue with such firmwares, by moving some >> of the FDT manipulation to after the invocation of ExitBootServices(). >> Given that the stub's libfdt implementation uses the ordinary, >> accelerated >> string functions, which rely on hardware handling of unaligned accesses, >> manipulating the FDT with the MMU off may result in alignment faults. >> >> So fix the situation by moving the update_fdt_memmap() call into the >> callback function invoked by efi_exit_boot_services() right before it >> calls the ExitBootServices() UEFI service (which is arguably a better >> place for it anyway) >> >> Note that disabling the MMU in ExitBootServices() is not compliant with >> the UEFI spec, and carries great risk due to the fact that switching from >> cached to uncached memory accesses halfway through compiler generated >> code >> (i.e., involving a stack) can never be done in a way that is >> architecturally >> safe. >> >> Cc: >> Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the >> kernel") >> Tested-by: Riku Voipio >> Signed-off-by: Ard Biesheuvel > > NACK, please. This causes a regression on my platform, in the form of > an assert in UEFI once ExitBootServices() is called, per initial > testing. I'll do more testing to determine why. > Sorry, false alarm. The assert appears to have been the result of a bad tree and a bad target configuration, and was not reproduced on another setup, nor did the assert make sense in context with this change. I withdraw my NACK. -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.