From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754750AbcK2Szm (ORCPT ); Tue, 29 Nov 2016 13:55:42 -0500 Received: from mail-bl2nam02on0088.outbound.protection.outlook.com ([104.47.38.88]:45946 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752094AbcK2Szb (ORCPT ); Tue, 29 Nov 2016 13:55:31 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Thomas.Lendacky@amd.com; Subject: Re: [RFC PATCH v3 19/20] x86: Add support to make use of Secure Memory Encryption To: Borislav Petkov References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003826.3280.5546.stgit@tlendack-t1.amdoffice.net> <20161124125038.j4wmtzgv2ju3v2ym@pd.tnic> CC: , , , , , , , , , Rik van Riel , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Arnd Bergmann , Jonathan Corbet , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov From: Tom Lendacky Message-ID: <78d7ed9a-e709-86e3-dcde-1f67cfa7684c@amd.com> Date: Tue, 29 Nov 2016 12:40:06 -0600 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161124125038.j4wmtzgv2ju3v2ym@pd.tnic> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: CY1PR03CA0004.namprd03.prod.outlook.com (10.174.128.14) To DM5PR12MB1148.namprd12.prod.outlook.com (10.168.236.143) X-MS-Office365-Filtering-Correlation-Id: e97fb377-3d74-429c-8236-08d418872d8c X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:DM5PR12MB1148; X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1148;3:ZYcwvZsZ70EfXZClV3N58MeF2BDZJU5o/iXVgZABlz/KA8EV5Yab3kFKEA65v9ihE4aIHX4DdgBWb4USbXyadngiHImXoMor1W87fQC0jLBP5Y01ySBdyYBqNG6R4LzcWSqypmmAPBqw9+Z2at0ua50kJy20FKouhlxD6o0p8ob5Q1lpmfncx3WGqslFegeDSOndbxb+OmVn/s3UHMYfPSFZgxQThZyo4pmwb6GJS6T9o5rXQi4tTsSAl5m5cYAOT4yNP/KH86jspJHqwA1+JQ==;25:u06j/1zETouoegnLpnCPwD/1zHQYExfcPPgRLjP+RddneH7XlcD8imIxbfTewMTTalIcAJ49H6JDxk+Kct0+CJMfq2dvK1dGowG7DWDtjUY3T8ZSvR09D0rI50hoosNXu20VOcUKP3eVVIfkknnfICnUr+4n+WbNuvoHj3f4os2C/ABSvvVJvzM80z+g/vLKl2GkoKMfZNITHcNvowndlV41n7Eocn+8np8O8P83bca6ewxnBVb0xmRH/IR0PXejdCq/vpJVmt1e6JYzGgvDcvcGfCeN7PLbqoRijDX0B8UDvYOG2A8A0QkHy4r4YT0dH1Y6VfZgsFHn4Fo7rNrL3yddPjbUMnBewYd+Lh1GvE+dXQesB9oPb4U6xETUxGBYP8V4org9yHqCl8iOzj3jizy2dFUcVCUOvygq5MJr5FsiXWZLbjC59KXg57W8Ceqwd/NhvOXiQX5mHiESg4gNhQ== X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1148;31:aS/FJi9GfPP7z32lyRUHXMWrT4ZNFBvC7B0rrGHzb/q+YiuJAomC7w7TWCmvOksNbBC5dsbFLqhruXXfhV1uxts0l0W1fd5M1Ai5zKaJto5xVKJJoxYcTQsVL4TwFcxumtA4mrWfrjdTWAIESDCwtGA9XFCDJTqaA0SdW/+/b37xw4vb/f7lLKlh7RouNr2vwxEn86KCn80ibO6mgKuXNgheO+jfPWG9fA5NJXvLMd9W9139BeyARN3BUxnvDgfK+mOq/SETZc7KQAtKjaR1vQ==;20:6MsCWPyYDEjLYi15Rv0ila2wT7oCR0IhYFGIEQzrKxIUALAlzA59nnxYhZcvhVaDBvJwSTSNusp7Zop25H4I4tBGRLECRD1z0XEWkuNNpRdPspkD7OdW28jrDWqZDmwY8KjCZeTpheBJ1sLA2hSTaQ6uKtCifSwP4OZHHDFeqEgLtu/iF3msHo5pKgCxkoiEeoxvfsUcsfKw8ZtmiUXEYxq4FehFL3D/VCvjFoS1V82/PldzMCdJBc8U+PZpn1w7hw8G9H0sud4Vb8aeFyt8UER6A1SJmCKk2Mz7hKGjQVCCTAwauFpSORT6mVgG+qL725v7QedxhPGHi351zpp5hL/RskO7gRUq+Zyrp2gxDMsFYGDpGh/M9ClDdMpD8+x87g+7878p8P8zrBA4UEPjXQpUAKr/uPCOLcOvGoI+tYtDm4PJwVUUbvMmEdBIkeTCkc4mUmaIweH7JcsZUQ6LsoDuHZJUVpyb/1rRL8itQ5dF4SAcx+g+KvFTyXJlJJ9Z X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(131327999870524)(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6060326)(6040361)(6045199)(601004)(2401047)(5005006)(8121501046)(10201501046)(3002001)(6055026)(6061324)(6041248)(20161123560025)(20161123558021)(20161123555025)(20161123562025)(20161123564025)(6072148);SRVR:DM5PR12MB1148;BCL:0;PCL:0;RULEID:;SRVR:DM5PR12MB1148; X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1148;4:j0jZeHlLQwFqFxSY6ThSg/qHgRTN6KFmjlnAcywh5i2u4tQbs05f2vPoe7EN8d7Kjy9bBfuznppP9M/bNmFs5aSNo+7PMQibk5+J6k+lddNPCn4qQ2WYcYYx+2boEWicdmB8YuBmVonmga/Aafb7SZZvKcJoKYRNEZcs/3EdPpBxi9LXQSR0mgIGSXCAS5IrmsJ+3tfEhQ+zqsRV5Hz52IDPlzRfUmY4eXC8Y9Aq7y7xppb+vPfCvIs1BMC3bMgvsOWq3xU1NxIwEpOG/31TdJLZNmxH3xSp6jpjn9EDQmjzQuhlfNKR8swVvqMkKr+gTEnaVaCBPxDMf9G6VjMtMlubPPHfFgvPV0shnFEzl1ELD29VywT9BZnedZ9sTLOubGhlaEbA9OIC6esP1Bqz4SO5R8gShAyBAIR6E/7d9Anru4Kx2i09z0ahrZAEPmAIaXHa+RWv8Us340A2mCKeJNdMnpS+0w8z2AHiChDS5ncdjHTivokExBI7knQslVRniUjnhTTo5+/7e5TzII+BogdhHjJtBTr3uBzBgNv/BcVTMjAefDVyYpb6Ik/54v4AiQfYYuHeVMNpohvzMVECbhViLJQhw6q0JVVb0/6/EVXUacESk8S1Kf4U3t/2RDMfbORYwn9OD8+yxtRsazVrWavkdsJcKHs6fhINloa/9g5m0TUT0KXBtDoZj/CVTQUmv2ynZgYJ/5ujq4fIY+UaiH+CP6YRfMkNuTTw2czTtPm9hHv4XpsxIUgXRqCdKj6R X-Forefront-PRVS: 01415BB535 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6049001)(6009001)(7916002)(377454003)(24454002)(189002)(199003)(6916009)(50466002)(39400400001)(105586002)(47776003)(6486002)(39380400001)(39450400002)(7736002)(733004)(86362001)(230700001)(66066001)(65956001)(77096006)(305945005)(83506001)(38730400001)(33646002)(23676002)(31696002)(39410400001)(229853002)(3846002)(6116002)(5660300001)(81166006)(2906002)(101416001)(81156014)(31686004)(7846002)(92566002)(64126003)(2950100002)(65826007)(7416002)(68736007)(4326007)(110136003)(189998001)(106356001)(36756003)(42186005)(76176999)(50986999)(6666003)(65806001)(8676002)(54356999)(4001350100001)(97736004)(2004002)(217873001);DIR:OUT;SFP:1101;SCL:1;SRVR:DM5PR12MB1148;H:[10.236.64.222];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtETTVQUjEyTUIxMTQ4OzIzOjQ1M1h6YWk5TngySHhUVkFUUTkxK1pkUWZU?= =?utf-8?B?ZTVSd2wvTjVUUXRTNXpaMldpUXZRK1VqRXViWXVwU1Z6OVlNVXV5aUQyd01K?= =?utf-8?B?b0tIZkFGMkt6WVNuT1h3WHlFTlhvQWdFelI1bEwvS3VKT0hWaWZJdWpzSjBm?= =?utf-8?B?dTE3ZlhZN0NHZkszWHZEdVpiemRJOFhyMk9HN1lQTDJGT0ljeWNLNk8vWnlp?= =?utf-8?B?eHRhS0RVTStYb0dBc0xSQ3drM0NzNFE0T0NKenpFdEdyZ2MwanZmdEQyeURC?= =?utf-8?B?dTBFakZaSnhUYkdCVUNRSDY2ZHZleFBEcDVWbjB3cWoxRmQxdmVwd2FTVlR1?= =?utf-8?B?eUFoN1hLSng1REdySldGaWYwSytUQytTa1FqUmRpZ1Y5ZldCWmE0VVNTOFhp?= =?utf-8?B?R3dKOEJJMkoxUkJrMVIvOGI3REwveWRaaDhJakZ5Q2JXNERWOFJIdXdBbHc4?= =?utf-8?B?L3BYbUZYcWh4V1dUWmgra3gva0hmRkpMQlBuRks4RnpzZnk1RW1CdFg1NVVJ?= =?utf-8?B?SmtIS0J5VCtZRGFaU1NGSGJJTEpNTTduL3VhVXlWT2tRTGFjZHJCaVEvakNu?= =?utf-8?B?MTRBZEFUNnRiM3BFeUxVdTh0QVh6Z3hiU051VFhUMDN2cnF4R3VmVnRhd1NV?= =?utf-8?B?VFV5VnBOUjBPYUEvaFVaWHp3R2doSCtmdFJxMi83bnpWeVNaaHgyRnp0a3pZ?= =?utf-8?B?T2VadDRGOEdTSXNCR0lOMXZJVWlBNWVicUNTaGZXQWRhYlFuMmlKbjhpMFhB?= =?utf-8?B?VmhtQmpLOWVDcmhoNG5kbGt6VDY2UmJLMW1PTHBoaDBtSldwcCswbHVOT1Ru?= =?utf-8?B?UmhhNWJUMEZMVWFVWnJFemFBd1pPNnp3dnFRZDd4UDhQR09XeEVSakU4WVRp?= =?utf-8?B?ODdmQWtYdGZXTnl6VUk5N29UWmxXVjhSSk5IbEpzVlhRd3M1YUpTR0VGcnEz?= =?utf-8?B?ZDRoV1RuUktETk5mL0VWcklTSUw0OEJoQVJzVTVPTnBtekdZSGl6Wm5xQzFS?= =?utf-8?B?SWRyOXdLNS9wM3NmRHFFWGVUNmN4S1h4M2JxTUFSNWFzeVZZaUR1MTFLOTlB?= =?utf-8?B?ZCtjK0dNMzF4YTY3VzBxb2J6Rys5dXUvZitiVnB6L0lFMXJVLzhUT0xJR2VI?= =?utf-8?B?aXpTRUZmSmYyeUQ5TVlwQ2RxSlowVXplRmgwNWREc1hhcVp3eVZCOGl6Q1lk?= =?utf-8?B?SXNPdHVoWTFFeUMvTnNNN25rdTlhTW1VK09rQ0d6VnlWMWwvK2wxL2lXa3J1?= =?utf-8?B?NnQzSEUwMlJyUWZDNUhSRFJENEVFWXZHK3ptZ1hpcjFvUDNOSFdHZ21yR0E5?= =?utf-8?B?VU1yem1CelFDUXN1QjBHTStDYVJhS3k2V3lMUXZMd3M3MytFeWt2dDZxZWZ2?= =?utf-8?B?Z3FVMEZ2UHZaNmo4WWFtbU9jTXFoM0t0SjBpWEkweXRoYzVMRis5TjNsN3Bp?= =?utf-8?B?azlFb09UVWRONkhnU3prYWhwWkY3Vkllbm1VamZyRWcvNUZ0eEhxSWRPWFBQ?= =?utf-8?B?ajFJWU15OGdYdXcyenozTVlUV0dkYmRnbWIvWGVnTGhxZkFNNkNGY0tTeW9p?= =?utf-8?B?NUFnUlRRVmluNWFmcUo4eUFEMk8vVFFacENaVVBlK2paQnlmZHFvaFhUSFM1?= =?utf-8?B?emNhK2tQRmorWE84TVBiTVBDZWtnVnNXWlJZaGRYYWs1S3RxT1ZGZ0wxTlFQ?= =?utf-8?B?UDZuWmdYamp3WkM5S2VzSHMxdThrWTVPK09XSGJVdUlUbWZrcFhGcUdzd1hs?= =?utf-8?B?QTlzREg3Z1Y5THFRQnh4R0x0cW5WUk0yNGVESDhCYjVudkV3S1g4VmtnV2lw?= =?utf-8?B?SGdzVWU4MzdTZWVtVnV2NEc0ek5MWnIxcitkdHFrVEpMMDY1M25QM1NDTmNT?= =?utf-8?B?cVNQRzFneHVIWUl4bnZqTzlkQVRzeTlGSTlsakRVM0dUc1pmZ1JNMTlVaWYz?= =?utf-8?B?ZENUZThUZXhkcU1pSURXQ3Uxc0xqTDZ6REkxTEs0TjBqdVEvL2NxRWZUVnMx?= =?utf-8?Q?rM8v2B?= X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1148;6:NO113i7EkpBYBK3m+BfOVQ9gF2D4DOxHDTRcXNLVDPhteDserhylE3pLTRDFwyf32TEBJSE9v12v8zwIq5W+sPLuvIHd58SA0LebQtdUziw8XGGfJl9OoK6f9zHZPFPVz6gxVkJNor7mvkTKeF7D0D5QZ2Ojz5cl8D1dnnGTKxkOSF8XyHqyjNqc0LZHKEm9jonAhGPfd8r6erKaeSlz7DHxXUGO2HbZxnEJWgZiverdECg55RMrK3BgOnEkGdRTndQX84ohOXBsDyGmqER6A9TYjRvCVeeC5+XIeWo59EM+cU1lwar0tprA3TjM3aJeF9/LmK0It1xK8hqu8rsMcLczKBhE/3UdGWolQppAd2xcqtodZN2zF8IXZXRXgMg4HXJvM4iudlPWlEzM5RLdQJSbWnc0c8vNEVsHDFTcU79XFwABwRXYGV1h87RDssd5hTIT26uSqyVizvSE+xFJLbGLTqX0CSZjdnLKOyAQR/8=;5:qs6Wbt3XaQ4fzzHRhUSPM+Fws8E4wCKFbvucrTewIG9K0LS6T9tQ1M/H6LeD8Bfq573cDKVYU7y2IkJeha7M4qyQRtDh1LqbtLHOIP+T91jemhPtkCswhmA1xI0ZowzLb1Ho9wATkRxMZB/TiGSVoFtrhOfMnLrFSIYz8mVXfk0=;24:0z3ppK2aSPMelzwRSG3diJluX2Q0sBbcAIsIvzz28yqG3UbXKb9LPgjQVHugk/qLZyjCIUdZ8ET/eE7WHQMqUjc7zzfgOsXAzsnuCLUAvRk= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;DM5PR12MB1148;7:k4Wyn5f71fDk/Y7mjN9epa5z3IiV+kgdh+v2uu8YPWt3yY/3HonCBMancEqjFre+c8XbYrM4sgNdkxBsKFfm47PMX1xXTUErEczA+fd9EUdiR9ANwW+o6GcuP14OzBKIug6l8GUV3x7D4VpjGTDV5aoY1Ju++x2xpXjAjuK0PjC1chFMQ57e9V01Vkb8bRW+K2krWzbYKlaejEFobYCjzU05cSiAX1yQv/S1qQU2uC6o0BaN0v1GBih6Mv5dF/8m6uli8Aga/L5qymGt2U4cyY66Htv9lw1Aue9kVZWPQo+hEIAfGJJWW7DRjO4KyR0hfHDMuFYEQnfejUgyquRHfzEoicB65egH+55UZ2vvXjgCeHJ5w3GRgWOAZWN2V0I9EEgKmrgqGaDMRH+1roKbq8i3lRhV8kVwUVEZKnB6KlCwZfvWBeATipIr/ZYZU3y46aro8n9jWLV77i6wi1FgTw==;20:FocRnKd1n40hzhGSwPWsNiGHoFkzF7jfUHvFpWO/dY3NvViP9WrVfmaNNaa5XaxqK3GXj05WJ6tpnftqnet70e8pRKpqCe1mOX3xzPUflcyS/Kll9HdxdQ4CPDAtHgrkMiDPlVbJtVvP0jrfOiL2abZOtIqr+knJqbQqaTZjxBFzcOwZFuhV85fEqKrtUGHV6dsJ5+ivmOIo+J8upMQ1AOcl+N5aJvs6WfNNM2UOO4SwK2YmpU+ePZqrZG7Rpx4/ X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Nov 2016 18:40:21.6335 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1148 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/24/2016 6:50 AM, Borislav Petkov wrote: > On Wed, Nov 09, 2016 at 06:38:26PM -0600, Tom Lendacky wrote: >> This patch adds the support to check if SME has been enabled and if the >> mem_encrypt=on command line option is set. If both of these conditions >> are true, then the encryption mask is set and the kernel is encrypted >> "in place." >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/kernel/Makefile | 1 >> arch/x86/kernel/mem_encrypt_boot.S | 156 +++++++++++++++++++++++++++++ >> arch/x86/kernel/mem_encrypt_init.c | 196 ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 353 insertions(+) >> create mode 100644 arch/x86/kernel/mem_encrypt_boot.S >> >> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile >> index 27e22f4..020759f 100644 >> --- a/arch/x86/kernel/Makefile >> +++ b/arch/x86/kernel/Makefile >> @@ -143,4 +143,5 @@ ifeq ($(CONFIG_X86_64),y) >> obj-y += vsmp_64.o >> >> obj-y += mem_encrypt_init.o >> + obj-y += mem_encrypt_boot.o > > So there's a lot of ifdeffery below which is not really needed and those > objects above get built-in by default. > > So let's clean that up: > > obj-$(CONFIG_AMD_MEM_ENCRYPT) += ... > > for all .c files. > > Then, put prototypes of all externally visible elements - functions, > vars, etc - in include/linux/mem_encrypt.h in the > > #else /* !CONFIG_AMD_MEM_ENCRYPT */ > > branch so that the build works too for people who don't enable > CONFIG_AMD_MEM_ENCRYPT. Much cleaner this way. This was mainly done to avoid putting #ifdefs around the calls in head_64.S. I'll look closer as to what I can do to make this cleaner but it may be a trade off on where the #ifdefs go when dealing with assembler files. > >> endif >> diff --git a/arch/x86/kernel/mem_encrypt_boot.S b/arch/x86/kernel/mem_encrypt_boot.S >> new file mode 100644 >> index 0000000..d4917ba >> --- /dev/null >> +++ b/arch/x86/kernel/mem_encrypt_boot.S >> @@ -0,0 +1,156 @@ >> +/* >> + * AMD Memory Encryption Support >> + * >> + * Copyright (C) 2016 Advanced Micro Devices, Inc. >> + * >> + * Author: Tom Lendacky >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> + .text >> + .code64 >> +ENTRY(sme_encrypt_execute) > > sme_encrypt() looks perfectly fine to me. > > Btw, is this the reason why this is still in asm: > > "(not everything could be converted, e.g. the routine that does the > actual encryption needs to be copied into a safe location and it is > difficult to determine the actual length of the function in order to > copy it)" > > ? > > If so, ELF symbols have sizes and you can query function sizes, perhaps > lookup_symbol_attrs() in kernel/kallsyms.c which returns size in one of > its args, etc. > > Alternatively, we can have markers around the function if the kallsyms > game doesn't work. I'll look into that a bit more, but with the small assembler routines it might be safer to stay with assembler rather than worrying about how the compiler may do things in the future and what effects that might have. > > Below are just some small nits, I'll review this fully once we solve the > in-asm question. > >> diff --git a/arch/x86/kernel/mem_encrypt_init.c b/arch/x86/kernel/mem_encrypt_init.c >> index 388d6fb..7bdd159 100644 >> --- a/arch/x86/kernel/mem_encrypt_init.c >> +++ b/arch/x86/kernel/mem_encrypt_init.c >> @@ -13,9 +13,205 @@ >> #include >> #include >> #include >> +#include >> + >> +#include >> + >> +#ifdef CONFIG_AMD_MEM_ENCRYPT >> + >> +extern void sme_encrypt_execute(unsigned long, unsigned long, unsigned long, >> + void *, pgd_t *); >> + >> +#define PGD_FLAGS _KERNPG_TABLE_NO_ENC >> +#define PUD_FLAGS _KERNPG_TABLE_NO_ENC >> +#define PMD_FLAGS __PAGE_KERNEL_LARGE_EXEC >> + >> +static void __init *sme_pgtable_entry(pgd_t *pgd, void *next_page, >> + void *vaddr, pmdval_t pmd_val) >> +{ >> + pud_t *pud; >> + pmd_t *pmd; >> + >> + pgd += pgd_index((unsigned long)vaddr); >> + if (pgd_none(*pgd)) { >> + pud = next_page; >> + memset(pud, 0, sizeof(*pud) * PTRS_PER_PUD); >> + native_set_pgd(pgd, >> + native_make_pgd((unsigned long)pud + PGD_FLAGS)); >> + next_page += sizeof(*pud) * PTRS_PER_PUD; >> + } else { >> + pud = (pud_t *)(native_pgd_val(*pgd) & ~PTE_FLAGS_MASK); >> + } >> + >> + pud += pud_index((unsigned long)vaddr); >> + if (pud_none(*pud)) { >> + pmd = next_page; >> + memset(pmd, 0, sizeof(*pmd) * PTRS_PER_PMD); >> + native_set_pud(pud, >> + native_make_pud((unsigned long)pmd + PUD_FLAGS)); >> + next_page += sizeof(*pmd) * PTRS_PER_PMD; >> + } else { >> + pmd = (pmd_t *)(native_pud_val(*pud) & ~PTE_FLAGS_MASK); >> + } >> + >> + pmd += pmd_index((unsigned long)vaddr); >> + if (pmd_none(*pmd) || !pmd_large(*pmd)) >> + native_set_pmd(pmd, native_make_pmd(pmd_val)); >> + >> + return next_page; >> +} >> + >> +static unsigned long __init sme_pgtable_calc(unsigned long start, >> + unsigned long end) >> +{ >> + unsigned long addr, total; >> + >> + total = 0; >> + addr = start; >> + while (addr < end) { >> + unsigned long pgd_end; >> + >> + pgd_end = (addr & PGDIR_MASK) + PGDIR_SIZE; >> + if (pgd_end > end) >> + pgd_end = end; >> + >> + total += sizeof(pud_t) * PTRS_PER_PUD * 2; >> + >> + while (addr < pgd_end) { >> + unsigned long pud_end; >> + >> + pud_end = (addr & PUD_MASK) + PUD_SIZE; >> + if (pud_end > end) >> + pud_end = end; >> + >> + total += sizeof(pmd_t) * PTRS_PER_PMD * 2; >> + >> + addr = pud_end; >> + } >> + >> + addr = pgd_end; >> + } >> + total += sizeof(pgd_t) * PTRS_PER_PGD; >> + >> + return total; >> +} >> +#endif /* CONFIG_AMD_MEM_ENCRYPT */ >> >> void __init sme_encrypt_kernel(void) >> { >> +#ifdef CONFIG_AMD_MEM_ENCRYPT >> + pgd_t *pgd; >> + void *workarea, *next_page, *vaddr; >> + unsigned long kern_start, kern_end, kern_len; >> + unsigned long index, paddr, pmd_flags; >> + unsigned long exec_size, full_size; > > Args in reversed christmas tree order please: > > unsigned long kern_start, kern_end, kern_len; > unsigned long index, paddr, pmd_flags; > void *workarea, *next_page, *vaddr; > unsigned long exec_size, full_size; > pgd_t *pgd; Ok > >> + >> + /* If SME is not active then no need to prepare */ >> + if (!sme_me_mask) >> + return; >> + >> + /* Set the workarea to be after the kernel */ >> + workarea = (void *)ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE); >> + >> + /* >> + * Prepare for encrypting the kernel by building new pagetables with >> + * the necessary attributes needed to encrypt the kernel in place. >> + * >> + * One range of virtual addresses will map the memory occupied >> + * by the kernel as encrypted. >> + * >> + * Another range of virtual addresses will map the memory occupied >> + * by the kernel as un-encrypted and write-protected. >> + * >> + * The use of write-protect attribute will prevent any of the >> + * memory from being cached. >> + */ >> + >> + /* Physical address gives us the identity mapped virtual address */ >> + kern_start = __pa_symbol(_text); >> + kern_end = ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE) - 1; >> + kern_len = kern_end - kern_start + 1; >> + >> + /* >> + * Calculate required number of workarea bytes needed: >> + * executable encryption area size: >> + * stack page (PAGE_SIZE) >> + * encryption routine page (PAGE_SIZE) >> + * intermediate copy buffer (PMD_PAGE_SIZE) >> + * pagetable structures for workarea (in case not currently mapped) >> + * pagetable structures for the encryption of the kernel >> + */ >> + exec_size = (PAGE_SIZE * 2) + PMD_PAGE_SIZE; >> + >> + full_size = exec_size; >> + full_size += ALIGN(exec_size, PMD_PAGE_SIZE) / PMD_PAGE_SIZE * > > Is that a fancy way of saying "2"? > > IOW, something like: > > /* + 2 pmd_t pagetable pages for the executable encryption area size */ > full_size += 2 * PAGE_SIZE; > > looks much more readable to me... Given the current way it's implemented, yes, it's a fancy way of saying 2 * PAGE_SIZE. I did it that way so that if exec_size changes then nothing else would have to be changed. I also used the "sizeof(pmd_t) * PTRS_PER_PMD" to be consistent with where that is used elsewhere. Thanks, Tom > >> + sizeof(pmd_t) * PTRS_PER_PMD; >> + full_size += sme_pgtable_calc(kern_start, kern_end + exec_size); >> + >> + next_page = workarea + exec_size; >> + >> + /* Make sure the current pagetables have entries for the workarea */ >> + pgd = (pgd_t *)native_read_cr3(); >> + paddr = (unsigned long)workarea; >> + while (paddr < (unsigned long)workarea + full_size) { >> + vaddr = (void *)paddr; >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + PMD_FLAGS); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + native_write_cr3(native_read_cr3()); >> + >> + /* Calculate a PGD index to be used for the un-encrypted mapping */ >> + index = (pgd_index(kern_end + full_size) + 1) & (PTRS_PER_PGD - 1); >> + index <<= PGDIR_SHIFT; >> + >> + /* Set and clear the PGD */ >> + pgd = next_page; >> + memset(pgd, 0, sizeof(*pgd) * PTRS_PER_PGD); >> + next_page += sizeof(*pgd) * PTRS_PER_PGD; >> + >> + /* Add encrypted (identity) mappings for the kernel */ >> + pmd_flags = PMD_FLAGS | _PAGE_ENC; >> + paddr = kern_start; >> + while (paddr < kern_end) { >> + vaddr = (void *)paddr; >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + pmd_flags); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + >> + /* Add un-encrypted (non-identity) mappings for the kernel */ >> + pmd_flags = (PMD_FLAGS & ~_PAGE_CACHE_MASK) | (_PAGE_PAT | _PAGE_PWT); >> + paddr = kern_start; >> + while (paddr < kern_end) { >> + vaddr = (void *)(paddr + index); >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + pmd_flags); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + >> + /* Add the workarea to both mappings */ >> + paddr = kern_end + 1; >> + while (paddr < (kern_end + exec_size)) { >> + vaddr = (void *)paddr; >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + PMD_FLAGS); >> + >> + vaddr = (void *)(paddr + index); >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + PMD_FLAGS); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + >> + /* Perform the encryption */ >> + sme_encrypt_execute(kern_start, kern_start + index, kern_len, >> + workarea, pgd); >> + >> +#endif /* CONFIG_AMD_MEM_ENCRYPT */ >> } >> >> unsigned long __init sme_get_me_mask(void) >> >> > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Lendacky Subject: Re: [RFC PATCH v3 19/20] x86: Add support to make use of Secure Memory Encryption Date: Tue, 29 Nov 2016 12:40:06 -0600 Message-ID: <78d7ed9a-e709-86e3-dcde-1f67cfa7684c@amd.com> References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003826.3280.5546.stgit@tlendack-t1.amdoffice.net> <20161124125038.j4wmtzgv2ju3v2ym@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20161124125038.j4wmtzgv2ju3v2ym@pd.tnic> Sender: owner-linux-mm@kvack.org To: Borislav Petkov Cc: linux-arch@vger.kernel.org, linux-efi@vger.kernel.org, kvm@vger.kernel.org, linux-doc@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, kasan-dev@googlegroups.com, linux-mm@kvack.org, iommu@lists.linux-foundation.org, Rik van Riel , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Arnd Bergmann , Jonathan Corbet , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov List-Id: linux-efi@vger.kernel.org On 11/24/2016 6:50 AM, Borislav Petkov wrote: > On Wed, Nov 09, 2016 at 06:38:26PM -0600, Tom Lendacky wrote: >> This patch adds the support to check if SME has been enabled and if the >> mem_encrypt=on command line option is set. If both of these conditions >> are true, then the encryption mask is set and the kernel is encrypted >> "in place." >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/kernel/Makefile | 1 >> arch/x86/kernel/mem_encrypt_boot.S | 156 +++++++++++++++++++++++++++++ >> arch/x86/kernel/mem_encrypt_init.c | 196 ++++++++++++++++++++++++++++++++++++ >> 3 files changed, 353 insertions(+) >> create mode 100644 arch/x86/kernel/mem_encrypt_boot.S >> >> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile >> index 27e22f4..020759f 100644 >> --- a/arch/x86/kernel/Makefile >> +++ b/arch/x86/kernel/Makefile >> @@ -143,4 +143,5 @@ ifeq ($(CONFIG_X86_64),y) >> obj-y += vsmp_64.o >> >> obj-y += mem_encrypt_init.o >> + obj-y += mem_encrypt_boot.o > > So there's a lot of ifdeffery below which is not really needed and those > objects above get built-in by default. > > So let's clean that up: > > obj-$(CONFIG_AMD_MEM_ENCRYPT) += ... > > for all .c files. > > Then, put prototypes of all externally visible elements - functions, > vars, etc - in include/linux/mem_encrypt.h in the > > #else /* !CONFIG_AMD_MEM_ENCRYPT */ > > branch so that the build works too for people who don't enable > CONFIG_AMD_MEM_ENCRYPT. Much cleaner this way. This was mainly done to avoid putting #ifdefs around the calls in head_64.S. I'll look closer as to what I can do to make this cleaner but it may be a trade off on where the #ifdefs go when dealing with assembler files. > >> endif >> diff --git a/arch/x86/kernel/mem_encrypt_boot.S b/arch/x86/kernel/mem_encrypt_boot.S >> new file mode 100644 >> index 0000000..d4917ba >> --- /dev/null >> +++ b/arch/x86/kernel/mem_encrypt_boot.S >> @@ -0,0 +1,156 @@ >> +/* >> + * AMD Memory Encryption Support >> + * >> + * Copyright (C) 2016 Advanced Micro Devices, Inc. >> + * >> + * Author: Tom Lendacky >> + * >> + * This program is free software; you can redistribute it and/or modify >> + * it under the terms of the GNU General Public License version 2 as >> + * published by the Free Software Foundation. >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> + .text >> + .code64 >> +ENTRY(sme_encrypt_execute) > > sme_encrypt() looks perfectly fine to me. > > Btw, is this the reason why this is still in asm: > > "(not everything could be converted, e.g. the routine that does the > actual encryption needs to be copied into a safe location and it is > difficult to determine the actual length of the function in order to > copy it)" > > ? > > If so, ELF symbols have sizes and you can query function sizes, perhaps > lookup_symbol_attrs() in kernel/kallsyms.c which returns size in one of > its args, etc. > > Alternatively, we can have markers around the function if the kallsyms > game doesn't work. I'll look into that a bit more, but with the small assembler routines it might be safer to stay with assembler rather than worrying about how the compiler may do things in the future and what effects that might have. > > Below are just some small nits, I'll review this fully once we solve the > in-asm question. > >> diff --git a/arch/x86/kernel/mem_encrypt_init.c b/arch/x86/kernel/mem_encrypt_init.c >> index 388d6fb..7bdd159 100644 >> --- a/arch/x86/kernel/mem_encrypt_init.c >> +++ b/arch/x86/kernel/mem_encrypt_init.c >> @@ -13,9 +13,205 @@ >> #include >> #include >> #include >> +#include >> + >> +#include >> + >> +#ifdef CONFIG_AMD_MEM_ENCRYPT >> + >> +extern void sme_encrypt_execute(unsigned long, unsigned long, unsigned long, >> + void *, pgd_t *); >> + >> +#define PGD_FLAGS _KERNPG_TABLE_NO_ENC >> +#define PUD_FLAGS _KERNPG_TABLE_NO_ENC >> +#define PMD_FLAGS __PAGE_KERNEL_LARGE_EXEC >> + >> +static void __init *sme_pgtable_entry(pgd_t *pgd, void *next_page, >> + void *vaddr, pmdval_t pmd_val) >> +{ >> + pud_t *pud; >> + pmd_t *pmd; >> + >> + pgd += pgd_index((unsigned long)vaddr); >> + if (pgd_none(*pgd)) { >> + pud = next_page; >> + memset(pud, 0, sizeof(*pud) * PTRS_PER_PUD); >> + native_set_pgd(pgd, >> + native_make_pgd((unsigned long)pud + PGD_FLAGS)); >> + next_page += sizeof(*pud) * PTRS_PER_PUD; >> + } else { >> + pud = (pud_t *)(native_pgd_val(*pgd) & ~PTE_FLAGS_MASK); >> + } >> + >> + pud += pud_index((unsigned long)vaddr); >> + if (pud_none(*pud)) { >> + pmd = next_page; >> + memset(pmd, 0, sizeof(*pmd) * PTRS_PER_PMD); >> + native_set_pud(pud, >> + native_make_pud((unsigned long)pmd + PUD_FLAGS)); >> + next_page += sizeof(*pmd) * PTRS_PER_PMD; >> + } else { >> + pmd = (pmd_t *)(native_pud_val(*pud) & ~PTE_FLAGS_MASK); >> + } >> + >> + pmd += pmd_index((unsigned long)vaddr); >> + if (pmd_none(*pmd) || !pmd_large(*pmd)) >> + native_set_pmd(pmd, native_make_pmd(pmd_val)); >> + >> + return next_page; >> +} >> + >> +static unsigned long __init sme_pgtable_calc(unsigned long start, >> + unsigned long end) >> +{ >> + unsigned long addr, total; >> + >> + total = 0; >> + addr = start; >> + while (addr < end) { >> + unsigned long pgd_end; >> + >> + pgd_end = (addr & PGDIR_MASK) + PGDIR_SIZE; >> + if (pgd_end > end) >> + pgd_end = end; >> + >> + total += sizeof(pud_t) * PTRS_PER_PUD * 2; >> + >> + while (addr < pgd_end) { >> + unsigned long pud_end; >> + >> + pud_end = (addr & PUD_MASK) + PUD_SIZE; >> + if (pud_end > end) >> + pud_end = end; >> + >> + total += sizeof(pmd_t) * PTRS_PER_PMD * 2; >> + >> + addr = pud_end; >> + } >> + >> + addr = pgd_end; >> + } >> + total += sizeof(pgd_t) * PTRS_PER_PGD; >> + >> + return total; >> +} >> +#endif /* CONFIG_AMD_MEM_ENCRYPT */ >> >> void __init sme_encrypt_kernel(void) >> { >> +#ifdef CONFIG_AMD_MEM_ENCRYPT >> + pgd_t *pgd; >> + void *workarea, *next_page, *vaddr; >> + unsigned long kern_start, kern_end, kern_len; >> + unsigned long index, paddr, pmd_flags; >> + unsigned long exec_size, full_size; > > Args in reversed christmas tree order please: > > unsigned long kern_start, kern_end, kern_len; > unsigned long index, paddr, pmd_flags; > void *workarea, *next_page, *vaddr; > unsigned long exec_size, full_size; > pgd_t *pgd; Ok > >> + >> + /* If SME is not active then no need to prepare */ >> + if (!sme_me_mask) >> + return; >> + >> + /* Set the workarea to be after the kernel */ >> + workarea = (void *)ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE); >> + >> + /* >> + * Prepare for encrypting the kernel by building new pagetables with >> + * the necessary attributes needed to encrypt the kernel in place. >> + * >> + * One range of virtual addresses will map the memory occupied >> + * by the kernel as encrypted. >> + * >> + * Another range of virtual addresses will map the memory occupied >> + * by the kernel as un-encrypted and write-protected. >> + * >> + * The use of write-protect attribute will prevent any of the >> + * memory from being cached. >> + */ >> + >> + /* Physical address gives us the identity mapped virtual address */ >> + kern_start = __pa_symbol(_text); >> + kern_end = ALIGN(__pa_symbol(_end), PMD_PAGE_SIZE) - 1; >> + kern_len = kern_end - kern_start + 1; >> + >> + /* >> + * Calculate required number of workarea bytes needed: >> + * executable encryption area size: >> + * stack page (PAGE_SIZE) >> + * encryption routine page (PAGE_SIZE) >> + * intermediate copy buffer (PMD_PAGE_SIZE) >> + * pagetable structures for workarea (in case not currently mapped) >> + * pagetable structures for the encryption of the kernel >> + */ >> + exec_size = (PAGE_SIZE * 2) + PMD_PAGE_SIZE; >> + >> + full_size = exec_size; >> + full_size += ALIGN(exec_size, PMD_PAGE_SIZE) / PMD_PAGE_SIZE * > > Is that a fancy way of saying "2"? > > IOW, something like: > > /* + 2 pmd_t pagetable pages for the executable encryption area size */ > full_size += 2 * PAGE_SIZE; > > looks much more readable to me... Given the current way it's implemented, yes, it's a fancy way of saying 2 * PAGE_SIZE. I did it that way so that if exec_size changes then nothing else would have to be changed. I also used the "sizeof(pmd_t) * PTRS_PER_PMD" to be consistent with where that is used elsewhere. Thanks, Tom > >> + sizeof(pmd_t) * PTRS_PER_PMD; >> + full_size += sme_pgtable_calc(kern_start, kern_end + exec_size); >> + >> + next_page = workarea + exec_size; >> + >> + /* Make sure the current pagetables have entries for the workarea */ >> + pgd = (pgd_t *)native_read_cr3(); >> + paddr = (unsigned long)workarea; >> + while (paddr < (unsigned long)workarea + full_size) { >> + vaddr = (void *)paddr; >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + PMD_FLAGS); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + native_write_cr3(native_read_cr3()); >> + >> + /* Calculate a PGD index to be used for the un-encrypted mapping */ >> + index = (pgd_index(kern_end + full_size) + 1) & (PTRS_PER_PGD - 1); >> + index <<= PGDIR_SHIFT; >> + >> + /* Set and clear the PGD */ >> + pgd = next_page; >> + memset(pgd, 0, sizeof(*pgd) * PTRS_PER_PGD); >> + next_page += sizeof(*pgd) * PTRS_PER_PGD; >> + >> + /* Add encrypted (identity) mappings for the kernel */ >> + pmd_flags = PMD_FLAGS | _PAGE_ENC; >> + paddr = kern_start; >> + while (paddr < kern_end) { >> + vaddr = (void *)paddr; >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + pmd_flags); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + >> + /* Add un-encrypted (non-identity) mappings for the kernel */ >> + pmd_flags = (PMD_FLAGS & ~_PAGE_CACHE_MASK) | (_PAGE_PAT | _PAGE_PWT); >> + paddr = kern_start; >> + while (paddr < kern_end) { >> + vaddr = (void *)(paddr + index); >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + pmd_flags); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + >> + /* Add the workarea to both mappings */ >> + paddr = kern_end + 1; >> + while (paddr < (kern_end + exec_size)) { >> + vaddr = (void *)paddr; >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + PMD_FLAGS); >> + >> + vaddr = (void *)(paddr + index); >> + next_page = sme_pgtable_entry(pgd, next_page, vaddr, >> + paddr + PMD_FLAGS); >> + >> + paddr += PMD_PAGE_SIZE; >> + } >> + >> + /* Perform the encryption */ >> + sme_encrypt_execute(kern_start, kern_start + index, kern_len, >> + workarea, pgd); >> + >> +#endif /* CONFIG_AMD_MEM_ENCRYPT */ >> } >> >> unsigned long __init sme_get_me_mask(void) >> >> > -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org