From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753490AbdEQSy4 (ORCPT ); Wed, 17 May 2017 14:54:56 -0400 Received: from mail-bl2nam02on0084.outbound.protection.outlook.com ([104.47.38.84]:11840 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751701AbdEQSyu (ORCPT ); Wed, 17 May 2017 14:54:50 -0400 Authentication-Results: google.com; dkim=none (message not signed) header.d=none;google.com; dmarc=none action=none header.from=amd.com; Subject: Re: [PATCH v5 17/32] x86/mm: Add support to access boot related data in the clear To: Borislav Petkov References: <20170418211612.10190.82788.stgit@tlendack-t1.amdoffice.net> <20170418211921.10190.1537.stgit@tlendack-t1.amdoffice.net> <20170515183517.mb4k2gp2qobbuvtm@pd.tnic> CC: , , , , , , , , , , Rik van Riel , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Toshimitsu Kani , Arnd Bergmann , Jonathan Corbet , Matt Fleming , "Michael S. Tsirkin" , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Brijesh Singh , Ingo Molnar , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Dave Young , Thomas Gleixner , Dmitry Vyukov From: Tom Lendacky Message-ID: <4845df29-bae7-9b78-0428-ff96dbef2128@amd.com> Date: Wed, 17 May 2017 13:54:39 -0500 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170515183517.mb4k2gp2qobbuvtm@pd.tnic> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [165.204.78.1] X-ClientProxiedBy: CY4PR12CA0027.namprd12.prod.outlook.com (10.175.82.141) To MWHPR12MB1151.namprd12.prod.outlook.com (10.169.204.15) X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: MWHPR12MB1151: X-MS-Office365-Filtering-Correlation-Id: 70991aeb-b97a-4d79-e5c9-08d49d562f52 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(48565401081)(201703131423075)(201703031133081);SRVR:MWHPR12MB1151; X-Microsoft-Exchange-Diagnostics: 1;MWHPR12MB1151;3:1mWAOEqctAwyRuMcVOAZpnMvJFMbtjVucdKrqSHzmo0Ei0LGXiIbnHXld+inxOephCH2AnvWM4GAPbdik5c0qaNz0o4o9auyk5FBumH9E9FwNBq1hqviT03sLrwqMlBKYRKMamOUE18znwd9QklK4y/9V4zRVUKt0j4+Fx1GpblmP1XVGeqVwOhAcssgULnWM1kySloOOORfPEv7fzXozYJqCRnhQZWQ8jSN1y7dZuzgAtwQRoLkR9X7hTP0kPsCsMonr0V63xS/+ZDfsusTPg5wbZUU6UR16OkRZSA6gRM1AY5zYVTYcVXdbS6+SeRhj546aCSHdC5VAs+dkUucojW0ascOg454LsBYC6EZrns=;25:nM0YRW2kEJw9KV1s1zK/gfRkndmfwxnGQXDzSd6zkFt98LbIfRuWopYy1mzfNcQqFKoOcW/0RhTgRsOZ/HOHPrDZ43SeEkL/DB1wXQLMJ98vUn3XVx4iwrfiB1kEpT4Fm2dfsTXssqcUH2J5v1WzePUSxlpCLrazx59ykaOi6PlKiQbLz2sabUH+VtG11e89WHYI0aPPWsEn1ALUDj3Sngk/67kQAphYlzwVVblhHDRaeTEu9TvxADeAlpmbqx3RL/rk0lipfL6c5iOhfmYlWUYFn5WcB/CdVsLvq0YTQ2b9r8NhlowAy5DQhwarXFJPxAIYuA0dfrN67tEhZ6iNIiT+e7dRH1aXNk7NxivLIDThw5YUm6v2LUeG9vHTEeor6a55c2XMc0ktvJUuUD/m44F8aPemXD6dLFmtDU2/fuj3jHKx4HGSHVz7ULXWw9nwH1i2byW1oLHYzAGDnWbpdeHIFSdTLBU8QMELsiGVHaM= X-Microsoft-Exchange-Diagnostics: 1;MWHPR12MB1151;31:u054pDgi4sXzef6Y3qIhXM8P7HZbzRZOT4YL1+TDfWrd6/OWtmbB1xx3iXyn5Bl9YRqn0vAMJ4ggKj9lzA8end5GLEW6y/yPN/6TY21FVBl7dK6xRuniCtVNnVv5hXWXz6ZLHlfq5IX3LLTXdzc7HkRCPOd65IkYwpu7/iHi3xEwzTNRX9wXUBH6UEum+53YmXCHBmsMU4N2/z5WBGQwh6hwvt5VaHP+MUGIpgx3Zcs=;20:zqFH7i22oGY2+cUKHO64yIzfEigGkQ80wkDt3RlC0n7Ybph+6toeZCjaWHSBf1COq0WEXqBxn9GthklYXiuB2xZS9pAmoOjI0C8875NeM8AdSdxg0MyFraXLuFVH1w7aczxg2iSPz/xR0Xl7oXtSto240les3no7oGPUBVSO+kx7/+xJqYBc3wmQbZAQwFWkEostPmiUCm4eMIQfp6vijMN4z9p8ZX/FIbBZA7wvGhCGgWsq2zFIZKe/zhlrUDDnYlOZkrHyhHzMfHKcevK5Apcz7BJCcNV3YtpfEVvMi32U2E5RyUbXUzq48AY3c4oz6ppPqSGKuhxjORuvvBGchHPxud8xSh32QpXd6EY66xfPI7PTo2Ib+ztz0I2rQ1pYg+n3tds4jBNgQAD2T3tf8EtURHcKeWb1JI0HPEdExLVeZo3JKDCfXYQzo+Pv3kHq7i5+n25ymkfm9OE3xbHvCT2KChX6h7SdZvLKMSFvIFnZcAY9u+8kmlKXUFGYQcxJ X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040450)(601004)(2401047)(5005006)(8121501046)(3002001)(93006095)(93001095)(10201501046)(6055026)(6041248)(20161123555025)(20161123560025)(20161123562025)(20161123558100)(20161123564025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(6072148);SRVR:MWHPR12MB1151;BCL:0;PCL:0;RULEID:;SRVR:MWHPR12MB1151; X-Microsoft-Exchange-Diagnostics: 1;MWHPR12MB1151;4:zrTxx2apCvjq69n9pzvgtlJgpPDdSAYrOcpPNEY2uTZzbYsFKjGS+JLF71Avfd1E/0LWUwBan+6iRXFwg1WwtLVFajTz/0T+vOMmUvbICm7DXVU94bK1OfF7S6FQPYBAORJ6VE4Si8TOp4RHCgDbE9mIJd7BSZZzKIIsXUtDYljXVH9sXnw5tdVH1/1DeaBWAm8M8KSFYkX+Mta39D+VvJpZNdb8I39GzHhaKavkx/jTp8M+fcmSl+W6ivuhKuuWmhWjGehfrjS5ZZ9b++Uuy8HRcaRAdLrSAfYSS0anUtU1PkYM3Y4EMcg6DZM7sUAwBahG+evkwB+6w2Cd/yydeS4WwSjgBotDdtX030KqISz/npsndBC/R5xWyw8g3UZg3SXdHljV787Ka0ThxMVmAcqO0hlcKIDz4iTQ65KWTAyHVgJlAR0cgsnBPDCrZ0KRm13jnT/ENUj18TuuIqaESr6+j9o/S8Yzwc11HjYimfCrWrsbTaQKZwolWNlbehW40XNs4PXDe9zNwzp29zgnanZyiWlNmiUUa+SjBJCysiFwr0eoENsIfT16i30kWJgVGjmBREcP7mzleksU+lra0TMnh3TjZ7MHU96w7e4M/UYZTFKPucuc+EJwZYnlyHizmcElHMbQwRoaDV9INyjseiA8bHc2yt1lqySe6Kl38C9CvI9H8JrBfHP5KET+QDPv4X7mvJ8iBoqnC1IrI6EgS0sJ7LVxYWsU7Dcdg7yr0bgn/rMzv73gvtTaXSf2VkJXS1z1UalVUuaXynI2NpeNDjOcrU38t+zBFPASHW2rQoEnP7cLS1yvMU576hsnfB1XFwwTeUp8y64OG4gE1pQu1w== X-Forefront-PRVS: 0310C78181 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6009001)(6049001)(39450400003)(39410400002)(39400400002)(39850400002)(39840400002)(39860400002)(377454003)(43544003)(24454002)(305945005)(53546009)(83506001)(7736002)(25786009)(189998001)(33646002)(76176999)(42186005)(50466002)(575784001)(31686004)(86362001)(50986999)(54356999)(54906002)(23676002)(7406005)(77096006)(3846002)(6116002)(65806001)(65956001)(6486002)(66066001)(90366009)(2950100002)(72206003)(6666003)(38730400002)(230700001)(110136004)(2906002)(229853002)(81166006)(8676002)(5660300001)(36756003)(47776003)(478600001)(6916009)(53936002)(4326008)(4001350100001)(7416002);DIR:OUT;SFP:1101;SCL:1;SRVR:MWHPR12MB1151;H:[10.236.64.250];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtNV0hQUjEyTUIxMTUxOzIzOlFmOFFmaW40ZUFVcVFUdkFuaElnaHBwN2FZ?= =?utf-8?B?UUhqaWJJYmpqZ2FZc3ZHQXJnVTFxVDliNTdSR1JyeHI2L1hMS0R3MDJhaTNa?= =?utf-8?B?TG9jaXZNK2NvYVFGWXZQUUcvdGlmVWtFd2twazArMDdMUjZieFVXZTRreExR?= =?utf-8?B?eTh3S2JQdEVibGRyM3RGclpIOUl0QWNUczdWM2krWFhzNU92Mkp2aEpOVUtN?= =?utf-8?B?NS9zbWtMR1RrK0ZNekFnMGVuRGVqV1VKWmZJQXhCRVZ1Q3Jmc1pCdGZDOUd3?= =?utf-8?B?T05Wc3dRbW1iUmVsVjZOZG44ZzRSOTM5YnJjdkhqVlZoUld1NkVDUmxKOFBH?= =?utf-8?B?cEczUlN5dVRzY2EyMTF4MWs1RDhwbWxVc3BpZUQrSWhSNnNXQXY2NkZOTktX?= =?utf-8?B?ZG8rR1FqbG1ablJaT0NtZkdoRXlIUEd0VExQUTJhSFAweE9PNkoyQ0s0b2Iz?= =?utf-8?B?cHd2MS9icTNyRlBIbkNWbnN6czIrdzZ3NFJCNGNGOUZETldWWUFQMGltK3Y1?= =?utf-8?B?RUhUZCtNU1NtVUc1VGJJeitoRXFoSUtXMEZLS1FadGg5NW4rdlMxWmlTL0t6?= =?utf-8?B?cWFjcEliYXZCOWFkTEdvVnJQa3hpR0JnNU43MUtkRlh3TmNvWmxQaXFzdHpj?= =?utf-8?B?bzNGMmVscDJ4YnJXcjVrNHM1V1QyUXJwZnNUd1BPcVZTaDU1VTdaakFjNzU3?= =?utf-8?B?R3ZGejlETWZuaWhxSHYwaWFTbGUvcWNyN0F6MWRwWlYyaHNYckdRNENHMUYy?= =?utf-8?B?RXhVUjAwUFhZZzVqL29namduYXo0dElJYlBZTXlBbFhYakErVEE1dWp4SFQ4?= =?utf-8?B?TjRFRUoxbGNMNEtmbk15RTAxL2hubHl0VjBZYW9TaUJROFFUL3dyRGExcTFa?= =?utf-8?B?QVZnNEVweG9ORXFFYldNNTV4T1dpSlVNV2NpNlRzM01nc3FyTVFPczZOZmsz?= =?utf-8?B?ckI3SGtiK2x3RTh0Z0dJcXMwVUVqU2lpcmN1ODdKRDhYU2R5QVlSSUQ2amt4?= =?utf-8?B?ZHRZYWU2bkVBYmFwZHprNFpJd2ltN2RCeFZGRUVaWG41RFByL0tTNXR2dHg1?= =?utf-8?B?RWlRVld3dU13UGlnUTZHUVlmTkJhbE16U2doamxKUGRQRWdydVFmS1czYmhH?= =?utf-8?B?QlZXdUhQRmxRTjlBUnA2eXVaRnZFWFZzMUJNZlM5ckxxWG5RRHlXcWxhVElE?= =?utf-8?B?UWZSWFBZd05qbWRlK3NaMytqdTM2NWYvUCsrbXNMU1ByUnRRMkJReEV4aUk2?= =?utf-8?B?VlRPa0ZyUVNiQUlkRlEyUkN6VElpQnQ5N3pwczZxam94UUlvbEJ5aDJVRXBX?= =?utf-8?B?UERYYTR6Z3hFSDFsY0dpN3ZuUnpOSUlzNEsyQTZEaUtLcW13Q2JsNHhUR1Nk?= =?utf-8?B?amt6QTNHMjlVVEl3RktRcGlXcnFXaGgwYVhOZlI3cTF3RHFtZXJoU0Z0T2t3?= =?utf-8?B?L0R4b1lqTHBuZGRXWXB1T1FpSmVGRHhqK3g5ejJDcGU1MUFTSmNwM0R4d25G?= =?utf-8?B?aCtBV3BTYkZhL2duZ1ZFUjhUb0dCN2owaW04ZmtUdFY2LzJndjhDMGc1MlB2?= =?utf-8?B?L0J0L3czdzJrcXZ4eHBvNm5ORTNBK1dGQnVNdGVvdTBFdndaQnNEYjFCeW1E?= =?utf-8?B?NUlhWXg0LzZ2RWVmZVBCVmlRMWpLVklOTUlXOEg4aFVzSlFUQlVpMUdhQmxa?= =?utf-8?B?VnVqS1F1eGNFcXpCckJ5SkgzbUpjM0srSHFWbVpNV1lHUGFoZ1NzLzJzUGFD?= =?utf-8?B?bnYyVlMrQm1hUGJISERMQ0RqK1ZsMncydTVMMjhjdHBvUklLT1pTMHd2bVcr?= =?utf-8?B?eWFScEVJYnBCbVAzNndwUHNvbXBpcWdxcnhjTkRJNHVqNzZDMHdlbGlNYUxG?= =?utf-8?Q?sb86RKjWoIYRcOXRHwJahVGf9PlP8Hy8?= X-Microsoft-Exchange-Diagnostics: 1;MWHPR12MB1151;6:oSHOp6zSFQyaaPHLjmBgLYwAgxAzymIKlVo4C0ZbP7g9io9GrexYTFUL5hR6KhawJ8rzno0O91rdveTCYEk01mtSZF6wrrczRRmU0PVB5dJrsgZzavD8Od2phbpRYxZKbdtGNdo3wqdeTPWWtImCAyWEGM0zSl4s98jcSwHbuAsx2l6vBxGVcWvRM66WTwHy54nh25lBycdiuEcfyjoUizedZFdwA3NfB34uYU/zEPZBqo6LJk5kuTSd1GR9RckYWQfBPTkAafXQAH6prb51MvohMHQCIlHzstbFNSIpmx92U0gxNMv9vMAJk/hruRGZX54OweXG8J5t4D5hClyQ708tNI4NyddLMC5Ywk+QvQbBrdnTFam7rkvm05aLG81E9wI3YOU9REw+9AfB8MRwqn9JsrVQLCclNE1IIf/cAkmP1jvL8zzN09Q/NZYm+LrwcT3BfjrgLn5cECumuQWTYfTL56X6czlri1voCOFQvzneiobdC4L5+JOIfhESlqAgx+92dpO1XizHwlVqraHBqmC2nFR7essCyMbUPnU5UlQ=;5:rtc1beTuo9364qyqE+KVukImuiOnzqfH4KPvLYoMjY00/OYUqx/fo7z1ZzWWNt5dzvkv0/9c6WHfPV6XMFokFxuLEM0eK6hApPUQAR86Aews3wPMzI5/WELOl7TbMxxa1/BjVLJapGRnb8f+5CcSgg==;24:nxLAUI3zpgjvFtRGyZ2YLJIleRP0jB/A0YMCNtsDPaTvEanYYBzVg3sHkapOyUW+hmu+MTX2jq3ItVF9FyKdT+EgQ7/B6ppjoFuTtCnFRzw= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;MWHPR12MB1151;7:k3KT09UJUEHMXI8rIjsRfvihyjh5R+mbitlXKBxw6+idfgpx9t33GVh1UMroVhfoYFse+2VvyPUhqD8jKt7fwH/N22e89YXYoL+vdCodRcMl17sNizfb534k+/lsAvDASjQPVicAsK+JgM/GzRyvSWaJZabYLPW+tswC3gz+kRcr/S6ZOtD/n+12WwgC4JI6obBaXdXGBFr/HWJ9WyvEnYzg8BbfGUnTWG2WwJWIB9yFKVg030EqMJbN9AvhmKQcFnWbXAZPsrIQNx151Cac0udCM6eE1fDa5UVLXD+KNS1rAacxEOjsxxJdrMgo6o6AeKsV6BbNceh7KV+UQ9qTQA==;20:XPjPEPQOrexIlLib+mCbYAJZmaqZBEspqnRiTJkSM59RS6WvP7sATPdd7AoeigUlqINRmAShsJTecC+4WKFXrme3/VVeu5bhT2oo7LJUj1y5LIsqFCNZzzknsKReqbg79HqHEwxmXEyD/wakS/k8u0R3+63DjjbZ5PjGim2bzLNaWSGCbO0CNsFVgfDwCV4InTzp8kMJNTZxJjGAXb50GK7ZjsBa8tCVZq3hxt5iREhFmr+9I3KyBh4Sj7bSEK7y X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 May 2017 18:54:43.0784 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR12MB1151 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/15/2017 1:35 PM, Borislav Petkov wrote: > On Tue, Apr 18, 2017 at 04:19:21PM -0500, Tom Lendacky wrote: >> Boot data (such as EFI related data) is not encrypted when the system is >> booted because UEFI/BIOS does not run with SME active. In order to access >> this data properly it needs to be mapped decrypted. >> >> The early_memremap() support is updated to provide an arch specific > > "Update early_memremap() to provide... " Will do. > >> routine to modify the pagetable protection attributes before they are >> applied to the new mapping. This is used to remove the encryption mask >> for boot related data. >> >> The memremap() support is updated to provide an arch specific routine > > Ditto. Passive tone always reads harder than an active tone, > "doer"-sentence. Ditto. > >> to determine if RAM remapping is allowed. RAM remapping will cause an >> encrypted mapping to be generated. By preventing RAM remapping, >> ioremap_cache() will be used instead, which will provide a decrypted >> mapping of the boot related data. >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/include/asm/io.h | 4 + >> arch/x86/mm/ioremap.c | 182 +++++++++++++++++++++++++++++++++++++++++++++ >> include/linux/io.h | 2 >> kernel/memremap.c | 20 ++++- >> mm/early_ioremap.c | 18 ++++ >> 5 files changed, 219 insertions(+), 7 deletions(-) >> >> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h >> index 7afb0e2..75f2858 100644 >> --- a/arch/x86/include/asm/io.h >> +++ b/arch/x86/include/asm/io.h >> @@ -381,4 +381,8 @@ extern int __must_check arch_phys_wc_add(unsigned long base, >> #define arch_io_reserve_memtype_wc arch_io_reserve_memtype_wc >> #endif >> >> +extern bool arch_memremap_do_ram_remap(resource_size_t offset, size_t size, >> + unsigned long flags); >> +#define arch_memremap_do_ram_remap arch_memremap_do_ram_remap >> + >> #endif /* _ASM_X86_IO_H */ >> diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c >> index 9bfcb1f..bce0604 100644 >> --- a/arch/x86/mm/ioremap.c >> +++ b/arch/x86/mm/ioremap.c >> @@ -13,6 +13,7 @@ >> #include >> #include >> #include >> +#include >> >> #include >> #include >> @@ -21,6 +22,7 @@ >> #include >> #include >> #include >> +#include >> >> #include "physaddr.h" >> >> @@ -419,6 +421,186 @@ void unxlate_dev_mem_ptr(phys_addr_t phys, void *addr) >> iounmap((void __iomem *)((unsigned long)addr & PAGE_MASK)); >> } >> >> +/* >> + * Examine the physical address to determine if it is an area of memory >> + * that should be mapped decrypted. If the memory is not part of the >> + * kernel usable area it was accessed and created decrypted, so these >> + * areas should be mapped decrypted. >> + */ >> +static bool memremap_should_map_decrypted(resource_size_t phys_addr, >> + unsigned long size) >> +{ >> + /* Check if the address is outside kernel usable area */ >> + switch (e820__get_entry_type(phys_addr, phys_addr + size - 1)) { >> + case E820_TYPE_RESERVED: >> + case E820_TYPE_ACPI: >> + case E820_TYPE_NVS: >> + case E820_TYPE_UNUSABLE: >> + return true; >> + default: >> + break; >> + } >> + >> + return false; >> +} >> + >> +/* >> + * Examine the physical address to determine if it is EFI data. Check >> + * it against the boot params structure and EFI tables and memory types. >> + */ >> +static bool memremap_is_efi_data(resource_size_t phys_addr, >> + unsigned long size) >> +{ >> + u64 paddr; >> + >> + /* Check if the address is part of EFI boot/runtime data */ >> + if (efi_enabled(EFI_BOOT)) { > > Save indentation level: > > if (!efi_enabled(EFI_BOOT)) > return false; > I was worried what the compiler might do when CONFIG_EFI is not set, but it appears to take care of it. I'll double check though. > >> + paddr = boot_params.efi_info.efi_memmap_hi; >> + paddr <<= 32; >> + paddr |= boot_params.efi_info.efi_memmap; >> + if (phys_addr == paddr) >> + return true; >> + >> + paddr = boot_params.efi_info.efi_systab_hi; >> + paddr <<= 32; >> + paddr |= boot_params.efi_info.efi_systab; > > So those two above look like could be two global vars which are > initialized somewhere in the EFI init path: > > efi_memmap_phys and efi_systab_phys or so. > > Matt ? > > And then you won't need to create that paddr each time on the fly. I > mean, it's not a lot of instructions but still... > >> + if (phys_addr == paddr) >> + return true; >> + >> + if (efi_table_address_match(phys_addr)) >> + return true; >> + >> + switch (efi_mem_type(phys_addr)) { >> + case EFI_BOOT_SERVICES_DATA: >> + case EFI_RUNTIME_SERVICES_DATA: >> + return true; >> + default: >> + break; >> + } >> + } >> + >> + return false; >> +} >> + >> +/* >> + * Examine the physical address to determine if it is boot data by checking >> + * it against the boot params setup_data chain. >> + */ >> +static bool memremap_is_setup_data(resource_size_t phys_addr, >> + unsigned long size) >> +{ >> + struct setup_data *data; >> + u64 paddr, paddr_next; >> + >> + paddr = boot_params.hdr.setup_data; >> + while (paddr) { >> + bool is_setup_data = false; > > You don't need that bool: > > static bool memremap_is_setup_data(resource_size_t phys_addr, > unsigned long size) > { > struct setup_data *data; > u64 paddr, paddr_next; > > paddr = boot_params.hdr.setup_data; > while (paddr) { > if (phys_addr == paddr) > return true; > > data = memremap(paddr, sizeof(*data), MEMREMAP_WB | MEMREMAP_DEC); > > paddr_next = data->next; > > if ((phys_addr > paddr) && (phys_addr < (paddr + data->len))) { > memunmap(data); > return true; > } > > memunmap(data); > > paddr = paddr_next; > } > return false; > } > > Flow is a bit clearer. I may introduce a length variable to capture data->len right after paddr_next is set and then have just a single memunmap() call before the if check. > >> +/* >> + * Examine the physical address to determine if it is boot data by checking >> + * it against the boot params setup_data chain (early boot version). >> + */ >> +static bool __init early_memremap_is_setup_data(resource_size_t phys_addr, >> + unsigned long size) >> +{ >> + struct setup_data *data; >> + u64 paddr, paddr_next; >> + >> + paddr = boot_params.hdr.setup_data; >> + while (paddr) { >> + bool is_setup_data = false; >> + >> + if (phys_addr == paddr) >> + return true; >> + >> + data = early_memremap_decrypted(paddr, sizeof(*data)); >> + >> + paddr_next = data->next; >> + >> + if ((phys_addr > paddr) && (phys_addr < (paddr + data->len))) >> + is_setup_data = true; >> + >> + early_memunmap(data, sizeof(*data)); >> + >> + if (is_setup_data) >> + return true; >> + >> + paddr = paddr_next; >> + } >> + >> + return false; >> +} > > This one is begging to be unified with memremap_is_setup_data() to both > call a __ worker function. I tried that, but calling an "__init" function (early_memremap()) from a non "__init" function generated warnings. I suppose I can pass in a function for the map and unmap but that looks worse to me (also the unmap functions take different arguments). > >> + >> +/* >> + * Architecture function to determine if RAM remap is allowed. By default, a >> + * RAM remap will map the data as encrypted. Determine if a RAM remap should >> + * not be done so that the data will be mapped decrypted. >> + */ >> +bool arch_memremap_do_ram_remap(resource_size_t phys_addr, unsigned long size, >> + unsigned long flags) > > So this function doesn't do anything - it replies to a yes/no question. > So the name should not say "do" but sound like a question. Maybe: > > if (arch_memremap_can_remap( ... )) > > or so... Ok, I'll change that. > >> +{ >> + if (!sme_active()) >> + return true; >> + >> + if (flags & MEMREMAP_ENC) >> + return true; >> + >> + if (flags & MEMREMAP_DEC) >> + return false; > > So this looks strange to me: both flags MEMREMAP_ENC and _DEC override > setup and efi data checking. But we want to remap setup and EFI data > *always* decrypted because that data was not encrypted as, as you say, > firmware doesn't run with SME active. > > So my simple logic says that EFI stuff should *always* be mapped DEC, > regardless of flags. Ditto for setup data. So that check below should > actually *override* the flags checks and go before them, no? This is like the chicken and the egg scenario. In order to determine if an address is setup data I have to explicitly map the setup data chain as decrypted. In order to do that I have to supply a flag to explicitly map the data decrypted otherwise I wind up back in the memremap_is_setup_data() function again and again and again... > >> + >> + if (memremap_is_setup_data(phys_addr, size) || >> + memremap_is_efi_data(phys_addr, size) || >> + memremap_should_map_decrypted(phys_addr, size)) >> + return false; >> + >> + return true; >> +} >> + >> +/* >> + * Architecture override of __weak function to adjust the protection attributes >> + * used when remapping memory. By default, early_memremp() will map the data > > early_memremAp() - a is missing. Got it. Thanks, Tom >