From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752324AbdFNUku (ORCPT ); Wed, 14 Jun 2017 16:40:50 -0400 Received: from mail-cys01nam02on0049.outbound.protection.outlook.com ([104.47.37.49]:55072 "EHLO NAM02-CY1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752394AbdFNUkm (ORCPT ); Wed, 14 Jun 2017 16:40:42 -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 v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption 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, kexec@lists.infradead.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?= , 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 References: <20170607191309.28645.15241.stgit@tlendack-t1.amdoffice.net> <20170607191745.28645.81756.stgit@tlendack-t1.amdoffice.net> <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> From: Tom Lendacky Message-ID: <0611d01a-19f8-d6ae-2682-932789855518@amd.com> Date: Wed, 14 Jun 2017 15:40:28 -0500 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [165.204.78.1] X-ClientProxiedBy: BN6PR17CA0001.namprd17.prod.outlook.com (10.173.147.11) To BN6PR12MB1137.namprd12.prod.outlook.com (10.168.226.139) X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BN6PR12MB1137: X-MS-Office365-Filtering-Correlation-Id: 92994803-663a-417f-ba1d-08d4b3659c7d X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001)(48565401081)(201703131423075)(201703031133081);SRVR:BN6PR12MB1137; X-Microsoft-Exchange-Diagnostics: 1;BN6PR12MB1137;3:YY8d2bIHcxbynwR6J77HBz8Z70jszZNwFiFdXKox7pdX8wdRnpQrhHoOZ0OuTlC2lktZ92lhrcU+g8IOgKzGOIUovYArcSUeC6BFTJxVZ1AfCiTzvKVQu8+IWjF0hT7qDJOg0bg5wH4PmFFV7Ns5x1UBkugl7A3RrebcYU+/GXGNlXfhEEhAtXXjSoYekQIUVY8oA3tpNo6DL6lPayzyyivTehvIxj/V3BPfIWX6EdWru4youXc5toNwfPnFDsf0Xd2pT8nlKIZ6tsIj5mO0uj7djazk20RIASREbrp3Oku3W0daxmdQqRW5jwWwSzHOr05hzLEEXUm7jAUHFGad7IYo/KSFIKvLFNFxuQJMZsM=;25:0uMXejhNk9WNCSHFJVcAS95TW4BDCsZR/4Ew8hI9jpq2V4p4NbMDQIAcqES0O1cQ4BlIb0Rj1oAVt+An2IwKYTflHy8ShqlsfXh7sRDpE6UZF60p0Iqxew1mtXbCZlMirTD2VDULdcFk84l48CR3/PTee4GQTwGNSs6Ml9mJxnO7kDsBsVN20YXbcYiHh5cQidUaJm03Vohqaxc/S08mhnl9QrNayy+T1Z8bHQfpWlGYw37b61QRl3+grexRQG35uKCDOeb+54VtJN9AEloxp16HEHWxYlqid9VhsWwDM/Bzn2UQU3+hS42s+VFdnrVh0muSRoC8+WnvJ3zgXKK7qA+CB/w8BxzypysYBjPiSbN77ULYg6yjuN5UTSE7XY6zNKme2hDDCQzxjvx5n0q5ncQrew1N4C+6K5Kq9P/NNwgFA3FlwdF4+en5oO+iblSTmc4uS9LxuPWbo1hkVL8AeJ7Fc5mnHi42+5ya/fpKfLQ= X-Microsoft-Exchange-Diagnostics: 1;BN6PR12MB1137;31:w5Fandk/Hj7oJSWXmLmXrqFbCpRcWBM/oMbB0M9pB52W61bK89WempaIp88MgiRnua9hYcxesEc/h3gXb9IgQ/v65wXDuLoXcFBQd2Y14AhXfWldlW9aqOfHVOZn+/zuV/T1Rmj1JiuT/zlF8gCs030lBgER93u/N852tujk9OhGhG8Mazed+iypHNrNikBleODwlc0pstgncqGfS6crJt292LbvICcMMp+KG96KYaM=;20:n4D84/gX79ZxahkgsXalj7zomEx4Cdi1Ar2n80RZbSpGVrrRJixgOtoPXnYAhjhj6yeKkeCVYk6wOb1nm7WqHaA8GR8oEl8Px0hbSgzj1i55jSqeianyn4u2fBhp5Afr240MsgtGpKraznVfLF4WpYRhG0XzCbh5I53ih8ANGeWhN7chZaR1w8FPflaPkWeczjVP902i2GOUDOhwKIJNK6VzsyssKA9KnaeHiRkkMJg2CSdIMDk+m+GBiqkkkQ6KffFsWNONognZaUPHUKZqaVuEydq30uKkn8DmQt5ZNA1fi7LEqH8CEZfQHJEykduOLAdZpez1lZ2Oh08yJgb3Fw/iJ8bq4dUHFpXdD+xi9u87LE/lLPJCl7dJ/eDsQuefC4PM49npMa5nZ+T51OXuC7/pHi+aXB15xFSfhIXCxZbxe+Itk8GgM+nM+m/4sWUPXNTfRPS0lD7NNT6lHHguXJ6OksBZkd33OUAsLCXP++yTlObz2su4SlgToHjXKMzr X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(601004)(2401047)(5005006)(8121501046)(100000703101)(100105400095)(3002001)(10201501046)(93006095)(93001095)(6055026)(6041248)(20161123560025)(20161123555025)(20161123562025)(20161123564025)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123558100)(6072148)(100000704101)(100105200095)(100000705101)(100105500095);SRVR:BN6PR12MB1137;BCL:0;PCL:0;RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095);SRVR:BN6PR12MB1137; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTjZQUjEyTUIxMTM3OzQ6aDFJekd1ZCthY3ZkUmZMV0MrcUVONnptVmY5?= =?utf-8?B?TEhvV3lHNldyN3pOUHR6MmJQRUtkT3AvZ3lJVnFxeTBBNmhlOVJVbWlzelFD?= =?utf-8?B?RUlsRWQrL2l6TGFZN2RXM1pFQ0d6azl2ZjlNcnFTeHFTRjdNV1ROcUZydys4?= =?utf-8?B?ZG8vQVFCei9wa3NYdkZGZHJHZjh1Snh0V3VpSnVBSmxOY3JZTEIrOEkwZXFS?= =?utf-8?B?VmwrWi9kc3NDQ1U4a3VjS2N4RE1TVnFRVXl0alBBVFp0b01XVnZuSk01UzZt?= =?utf-8?B?SE84NHNHS3UxMGRtaVBINlBtUjNoNmZHaHlEcUtOVFVmQlVCNExWVEVlYXB0?= =?utf-8?B?bDdWSG0xbFVNTyswaFdMNGJDZDZzajVJZ2NTbGJOVVRKdnM5WkhWdHlER3Rw?= =?utf-8?B?dm5oYmlJbUNNdnBqdWZHa2Q4NmlCV2pQM1RpeU56TDdRc1MxSzBneGRHbytw?= =?utf-8?B?RTYxQkNYbTVoYTNnWXliUDNORU04QzRwRGxjYkhCT1pOelJCVE9CUU1pUTR3?= =?utf-8?B?UFZTeFlWUWdBWFNuaUYwQjNrZWx1aEZ6cGZHK1ZTUWUxcWRFN2lnMHRMcWxv?= =?utf-8?B?NkdYK1ZDMHhnL3lpR3B1LzN6Sy8xMlRLdUdHeXFETzFxTzJEY3dxZGN2U3Jy?= =?utf-8?B?dzlXNE1LVVhZd2IxT0FiY2ZDY1huakxjZnl4M2hlRnhXQTR2amdqT0RWU3Ux?= =?utf-8?B?aHovMmk4NENYVHFqSnJlei9ZUVovaVZBdWp0NUgwcW9mckUzVEl4SjFoNEI0?= =?utf-8?B?Y2NrRXVuNHEwTnBickF5MEpKYTFvc0FMS2tDQ2tkU2FFTU9EeTkrUTZQQ3k5?= =?utf-8?B?bThUR3RXeE5BdENScFRlT0phaGh5NkpiUUJsL3hrRGd3NUxtVDZYeEdwZzNy?= =?utf-8?B?M29SUVd1UVluM1hRZk1Fdk1qV3dkZjc4UUV4RDRwK0FkdFJQWWsxc2RLZzNr?= =?utf-8?B?YmErR2ttazZIblBya3Z6STRQMG5qYzV2V1Rnb1NCRTM4ckV0QmtNcmF0aDdu?= =?utf-8?B?UzZ5WmdFQm85bmtOWTA0N043RU9pUStYc2RybnFhV3BFcldqVWczNDljdUZW?= =?utf-8?B?NS9tN2VpVjdCZFp6aXZ6N1U0UlB4WkhtYzZBaXNzbTJ3ZmdHZDJIT1Z3R05y?= =?utf-8?B?dVgrdW5teUcyR1N6STFIOHRQZUdlMVNUaWV2aFRRNWZiTXdBMFJybXJGVy8w?= =?utf-8?B?MGVtcFpoSHJ4K2sxZGxYV1ZBT1BzL2hZMlFlMThWWDRUY3pUdEpldG5ENWtX?= =?utf-8?B?N0wyZnpiZjgxR3NZd29nVGQ2QXpOSkRHc1VKMVdLN1VuWGErTnFLVjdaeS9O?= =?utf-8?B?V1lNWUFGVXRkQWtCNTRWdXJzUjJ5M25uVnFwR2FmVXAzTFFUdmwzWDh2RFMw?= =?utf-8?B?YXVjaitLa0M4STI3QjhiaWF2N2FHczR6a1grYjhPVUxuRnI1eGVTZnRtckF6?= =?utf-8?B?K1c5SkN1cnhJKzl5V2lWaHdsb2lFWlREZTV6bW1tZzgwMG9aUytIdC9kK29l?= =?utf-8?B?Ym9xZEp6b0RZbndmaHBWZnRSYXpTNkU3SHlJakQ5b2ZkdDN0Z0ZlSXloQjVm?= =?utf-8?B?ZDB4TWp3UWhacjhzNGtFVTd4YXZ1SGJqVkdKQVJ5TXJva1E1NUR6Snc0VWRS?= =?utf-8?Q?Za5tWTE0r9ObF9+UI1d?= X-Forefront-PRVS: 033857D0BD X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6049001)(6009001)(39450400003)(39860400002)(39410400002)(39840400002)(39400400002)(24454002)(377454003)(57704003)(50466002)(53546009)(478600001)(81166006)(64126003)(8676002)(189998001)(2906002)(72206003)(4001350100001)(6246003)(110136004)(38730400002)(83506001)(4326008)(33646002)(229853002)(36756003)(6916009)(6666003)(6116002)(3846002)(2950100002)(31696002)(575784001)(86362001)(5660300001)(76176999)(54356999)(65826007)(50986999)(6486002)(230700001)(53936002)(42186005)(90366009)(31686004)(23676002)(305945005)(25786009)(66066001)(7736002)(3260700006)(47776003)(7406005)(7416002)(65956001)(77096006)(217873001);DIR:OUT;SFP:1101;SCL:1;SRVR:BN6PR12MB1137;H:[10.236.64.250];FPR:;SPF:None;MLV:sfv;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCTjZQUjEyTUIxMTM3OzIzOjV2aXUralRCUkdObWNBQ2piS052M01vVVlG?= =?utf-8?B?UC83WnZUT0VkRkdoUG9oM3lOVUNBL0tKbUM5TjFsQU9SQTNDOFkxTWt4Smsz?= =?utf-8?B?LzJDSEFaMWhtcVBoaU00U01nK3Vva0FVVnl3VWxHWnpDSE1neUlHYjYrR2U3?= =?utf-8?B?bDBoZVZUdm0zUjdFQ0pmY3ljM1RkeFNsWFR4K1FxaVhZVXAxeHFLYmFTdCs4?= =?utf-8?B?Z21IYkd1MUpYenliT2srME11dEp0WTYrb2hMS3daYTlRTUtSUXNsYnpoREJK?= =?utf-8?B?cTNLajBZZzZXQjZLNVZFeUFKZVRON1BTVXl2Q1NVRnk4WmZzbk55QlFtb0FP?= =?utf-8?B?cXZ2ZldDTE03ekJMRkNmc0JsSmwyY1hLRDZSdU9vVTZOT1AyV0lvd1RLdzMz?= =?utf-8?B?YXpJLzB1T2RBRWVsb0pDTlBsQklzM3BNZEZneVFpNWRWelJYOXJmVnM3MEZs?= =?utf-8?B?aXQvYlNGSnV5QlFWWG1lRGlNalQ1MnZFWUVuN2s2dGlDSVlhK3VBZHNQZVJv?= =?utf-8?B?MjQ1eU1GWkJMYm43bVZNeTI5ZUZJTjdaRXhPcDNvV2FpVFM5QjB5UTNDdnI3?= =?utf-8?B?TExEQVIvM2tFUkxOdkplZWZSTU03YWhvM3pIMjVzdFpOMVFReUxHR2FmRGd4?= =?utf-8?B?QXgyeW9nYXBMT3ZTK3M2NUs0YXRhdFJud2xmeW5HN3JuOW84ZE5lQlRCcWdQ?= =?utf-8?B?dnpZQWt1bTlSeHBycEpybjFTTndRejRXWnlhMWo0ZWFiR3F0ejNFK2w2b1hF?= =?utf-8?B?RFlUeVdUYzZESUM3b2hIekd1NFF2dUNtNFUvOWt2MksyZ0ltamxselE4TnRH?= =?utf-8?B?R29SUHhuamNORXA4NFlaWXMrMXFTZXdNUmlLWjRZeVM0dWM1cDBEb2tqWWY0?= =?utf-8?B?Ui9mS1c1M284R0FWQVFkdnh6ODhiNTFaWitycytTRUh0aEpONWE5aUlUWGVP?= =?utf-8?B?b1c5U1o0VlpVN0N5QTkzTjBUdWR1UzZGY2VrM1hqT1ZQS2ZyUnY1Vm1ad1hW?= =?utf-8?B?Y3VWb2xsUGZrN3lLM0Q0VkZtQ3pqZkNFbEJFQVBWY05GUTllMXJLb3JRMkkr?= =?utf-8?B?UjRVcjlQZm1lN1FFZG9sMHBxdm8wVjhDMEdDNFhpZ1ArVjJpb0FSUUFVUGZ0?= =?utf-8?B?dDFXS3pxaFY2czNRNTc1emtPVDVIQ3kyZzRWT1VTYnpINVJTWERpODRmelBv?= =?utf-8?B?MHBibGx3blEwK1ZWbmNxdjUwcWhNVGZhNExPeGJqZ0V3NHVkL25rSVVDK2Ix?= =?utf-8?B?WDdwZTVCaUhGSlNFQWtuWFNNdVlaa2Vxck1uYXVVazNROTFXZnQ5OWdwVUlP?= =?utf-8?B?WnE5SXBEWXhIY2lCRkJYVGFtMFpjT2xxU21DQURGL0hBeElCUjArWElQZ25s?= =?utf-8?B?SkZuMUxrUkg1RHQ2TkZOcWlCNzBVbWEvczdud1EwYnlqNHNvMkhRN3krYTFE?= =?utf-8?B?V2tJWHZIZXd6MVNVOE5nQ2NxbDNsZFY4c1c2b0pHdG9rbmlGbUZLN1lFT1Y3?= =?utf-8?B?SzZLQUNPYmQ3SUVuRHk0ZHMwa01iUHNXZW1uOXBDQ3FpY1pHbWJPQWpMeDkw?= =?utf-8?B?emUyd1BaUEtMRnVKdEdrZlBTSkw1R1ErYjl0S2loSWI4cis1bmE1T1JCMWVu?= =?utf-8?B?bGNhQTVSWFJnc0JOOGJTSkh2U20xRVV3WjE3VkN5M3JGb2NHUk41WmZyanlY?= =?utf-8?B?QmY5bXFmMGRwUEl5V1dNdCs1R0FNSnRyWnFkMUZKTEZLbTVXblk1eVY4empa?= =?utf-8?B?MGxocjUvM0Zxb0ZDT042aXJNVmM2WmJodjJMbkozLzE0MVloRDNrVE1tekFU?= =?utf-8?B?d3lNUkxvcEJJVTB4dFJiVjV6SkZVZG50ZWljMFpiNm1sallTUlRvcElpa2hP?= =?utf-8?B?QzdRc2t1WHJhL2JLaUJ6L0J1U2NFUjliMTkrV1hqaTNaK3FTSjN3aXpIZm1R?= =?utf-8?B?Vk9jRlNOcUI1Vm1xakxiRzRmbjlxUmxzUEVNdGtnVzRkeVFON09lVWhHQXNj?= =?utf-8?Q?IwvebR?= X-Microsoft-Exchange-Diagnostics: 1;BN6PR12MB1137;6:s7Rmh/8IJPr1DUfQnQnlJ5MP5fCI1zj2ucBCGud9rBB/NqoJzcHA+kqkaus3hoEVivqp84dT+8ihfjFhhjSB32zrpztpNJKSyFjrFMdH1OXs3rSO/PIYTAy4btaNrNi1ojHhLKYIUiyZxwUvUonJ1d8ypvb7bq2LlAULeIV2wwLuxPr5cJua6YapZBgwYKAjR4M53z5Lp7xXrQYh+XmggVX4YUDLIEuu+SYwgO2QHrWYgcN4Qke6vWWnl/MOZuwrswCyCsEb4xLW753bg6gDNkeZV4ns9fu06X9/fFEfKlyr/taZdZ+OL/1G1yE2YUiLoXs/kMFJr06iY1aQrwJ2QGX4T4eSbFjm+xVFfNjaqOGSDFk1cyZcHD6SZCZv0RFsx29rbwp5tldS3Kg986ArNZCbEEmflwlUDYQpQhc9LeN9DWGifyCgWHCB8HmNeDLZ7eT3RPhWxWqigMCU0cOlrSYYUkZh+86k/h73a+VRWzaNT6XHA1Mw1ecbyC8E69ytgwWmpWbJq3+o8tT0X3+xWajBGp5rRnfkoStRaIqOsEY= X-Microsoft-Exchange-Diagnostics: 1;BN6PR12MB1137;5:zwVCIBrQd1oDKMcIftwT4lV8j+6tSEj6BbLPErZT3nTeNpiqfqk0bjQYajeKvEykgmlBt1LIRFzpcKSeXATBiYT9wl9EQQzqq9VWiyNtNiTJc+j3Lmi9Arp6mKPvakHXMfM/KgQQgCu8o2gWZJfYe43L7srXQAqnioLdPoBLx9963WEstu56wce2X+Ue6CCmRQTorEX9e373qOWsIT+Y8/tdAFOgZOsC9vJ4Frs46GtD6DnYzJ5TNERqe3i4FiQTTnu+sWhrDmCC9YvC1FAmTbCKTMvGuhwjYLm7fNegsgMEqkO4k44YImgWwKrKsz5lMieF/EZm8N9aG4WYKXdHPIEkk8yz6yVDqYY0isewVF9VaEav0uI4a9tiAHGnbCtf6iQP8QEAqC22PckvPUgerTyvpXWuruw5vMDSgKs2pLrgDk6N7nUmOo+9RWSDvCtQMqXyVlulx+bXoe/bpADPNKqjxiaEbExGNH65Wgzs5brDIkLHIs0zD/WVWKxXHG9X;24:Mn8e1XRBAPRdmsoHHhOYu+iPv4xyIF3l5MO0Ovi3WHVtiDe3syXXjRiQdSK74nZycFJ1y2UWzP6+UWB1CcoiVtla4jTnIKqMzl4k2qvy7xQ= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BN6PR12MB1137;7:g4d4JnBvVGGoo1PQZPug5h7qjuP/Hun27A/IYZ7D80Xj0qGB2rx6+nVxCeh4mbpUNfeO0aPdeVq84FJJHRixH2nKBj0MCSYAoAWvnYXCo5/nTufv3HpELNdcqcid6ZTthHLrliljuZjRFhFhkR+soc2ArPot20WsrREfX4tlQfPcs3JWyPcsLOKb9rQgKREvHe9Zm9PpH/rEDjQJ8K3utGDxTu62UXaEIAVkqb48aUIzzF9RvPDD2a1BS5U6KE5wo92yY8SnszLh/FVTBtSMcrsTY3gk7/UuDQy8zQ8vC4JdF2CDVBAa0Kj8tyKzU+DpHcI5FVwLTam6xYKoCei0yw==;20:PHSyY3ZW7bmahC7uHsJvw44NB2q8jlLOivgN9Kz0c7bBF6cdw4oe/x5BFTYEiyyNaOYqY4Koa7nYx8YKjDOyTLVz3Fdz95F6t75CmmnSZy7OxMj3okqGh3gDq3ZCToU9D5W+sepFC6F6QdlWF3TOAtXU65kh5kjeG2pSDUWYKkFJ+cBcNfiuyr2x9a09s2a2pFFhHBApumVL5WCGlbtjuAYuUJTgrKCsqHldcCjfnuKe9UdXS/Rm8PKjKF9LKI5i X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2017 20:40:31.8838 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR12MB1137 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 6/14/2017 12:42 PM, Borislav Petkov wrote: > On Wed, Jun 07, 2017 at 02:17:45PM -0500, Tom Lendacky wrote: >> The IOMMU is programmed with physical addresses for the various tables >> and buffers that are used to communicate between the device and the >> driver. When the driver allocates this memory it is encrypted. In order >> for the IOMMU to access the memory as encrypted the encryption mask needs >> to be included in these physical addresses during configuration. >> >> The PTE entries created by the IOMMU should also include the encryption >> mask so that when the device behind the IOMMU performs a DMA, the DMA >> will be performed to encrypted memory. >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/include/asm/mem_encrypt.h | 7 +++++++ >> arch/x86/mm/mem_encrypt.c | 30 ++++++++++++++++++++++++++++++ >> drivers/iommu/amd_iommu.c | 36 +++++++++++++++++++----------------- >> drivers/iommu/amd_iommu_init.c | 18 ++++++++++++------ >> drivers/iommu/amd_iommu_proto.h | 10 ++++++++++ >> drivers/iommu/amd_iommu_types.h | 2 +- >> include/asm-generic/mem_encrypt.h | 5 +++++ >> 7 files changed, 84 insertions(+), 24 deletions(-) >> >> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h >> index c7a2525..d86e544 100644 >> --- a/arch/x86/include/asm/mem_encrypt.h >> +++ b/arch/x86/include/asm/mem_encrypt.h >> @@ -31,6 +31,8 @@ void __init sme_early_decrypt(resource_size_t paddr, >> >> void __init sme_early_init(void); >> >> +bool sme_iommu_supported(void); >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void); >> >> @@ -62,6 +64,11 @@ static inline void __init sme_early_init(void) >> { >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Some more file real-estate saving: > > static inline bool sme_iommu_supported(void) { return true; } > >> + >> #endif /* CONFIG_AMD_MEM_ENCRYPT */ >> >> static inline bool sme_active(void) >> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c >> index 5d7c51d..018b58a 100644 >> --- a/arch/x86/mm/mem_encrypt.c >> +++ b/arch/x86/mm/mem_encrypt.c >> @@ -197,6 +197,36 @@ void __init sme_early_init(void) >> protection_map[i] = pgprot_encrypted(protection_map[i]); >> } >> >> +bool sme_iommu_supported(void) > > Why is this one exported with all the header file declarations if it is > going to be used in the iommu code only? IOW, you can make it a static > function there and save yourself all the exporting. I was trying to keep all the logic for it here in the SME related files rather than put it in the iommu code itself. But it is easy enough to move if you think it's worth it. > >> +{ >> + struct cpuinfo_x86 *c = &boot_cpu_data; >> + >> + if (!sme_me_mask || (c->x86 != 0x17)) > > me_mask or sme_active()? I like using sme_active() outside of the SME-specific files and using sme_me_mask in the SME-specific files to save any changes that will have to be made once SEV comes around. > > Or is the IOMMU "disabled" in a way the moment the BIOS decides that SME > can be enabled? There's a fix in the AGESA layer of the BIOS that permits the IOMMU to function properly when SME is enabled. Unfortunately, the only easy way to determine if that fix is available is through the patch level check. > > Also, family checks are always a bad idea for enablement. Why do we need > the family check? Because future families will work with the IOMMU? :-) Yes, any future family that supports SME will (should) work with the IOMMU without having to check patch levels. > >> + return true; >> + >> + /* For Fam17h, a specific level of support is required */ >> + switch (c->microcode & 0xf000) { > > Also, you said in another mail on this subthread that c->microcode > is not yet set. Are you saying, that the iommu init gunk runs before > init_amd(), where we do set c->microcode? > > If so, we can move the setting to early_init_amd() or so. I'll look into that. > >> + case 0x0000: >> + return false; >> + case 0x1000: >> + switch (c->microcode & 0x0f00) { >> + case 0x0000: >> + return false; >> + case 0x0100: >> + if ((c->microcode & 0xff) < 0x26) >> + return false; >> + break; >> + case 0x0200: >> + if ((c->microcode & 0xff) < 0x05) >> + return false; >> + break; >> + } > > So this is the microcode revision, why those complex compares? Can't you > simply check a range of values? I'll look into simplifying the checks. > >> + break; >> + } >> + >> + return true; >> +} >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void) >> { >> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >> index 63cacf5..94eb130 100644 >> --- a/drivers/iommu/amd_iommu.c >> +++ b/drivers/iommu/amd_iommu.c >> @@ -544,7 +544,7 @@ static void dump_dte_entry(u16 devid) >> >> static void dump_command(unsigned long phys_addr) >> { >> - struct iommu_cmd *cmd = phys_to_virt(phys_addr); >> + struct iommu_cmd *cmd = iommu_phys_to_virt(phys_addr); >> int i; >> >> for (i = 0; i < 4; ++i) >> @@ -863,13 +863,15 @@ static void copy_cmd_to_buffer(struct amd_iommu *iommu, >> writel(tail, iommu->mmio_base + MMIO_CMD_TAIL_OFFSET); >> } >> >> -static void build_completion_wait(struct iommu_cmd *cmd, u64 address) >> +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > > WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst > #134: FILE: drivers/iommu/amd_iommu.c:866: > +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > The semaphore area is written to by the device so the use of volatile is appropriate in this case. Thanks, Tom >> { >> + u64 address = iommu_virt_to_phys((void *)sem); >> + >> WARN_ON(address & 0x7ULL); >> >> memset(cmd, 0, sizeof(*cmd)); >> - cmd->data[0] = lower_32_bits(__pa(address)) | CMD_COMPL_WAIT_STORE_MASK; >> - cmd->data[1] = upper_32_bits(__pa(address)); >> + cmd->data[0] = lower_32_bits(address) | CMD_COMPL_WAIT_STORE_MASK; >> + cmd->data[1] = upper_32_bits(address); >> cmd->data[2] = 1; >> CMD_SET_TYPE(cmd, CMD_COMPL_WAIT); > > <... snip stuff which Joerg needs to review... > > >> diff --git a/include/asm-generic/mem_encrypt.h b/include/asm-generic/mem_encrypt.h >> index fb02ff0..bbc49e1 100644 >> --- a/include/asm-generic/mem_encrypt.h >> +++ b/include/asm-generic/mem_encrypt.h >> @@ -27,6 +27,11 @@ static inline u64 sme_dma_mask(void) >> return 0ULL; >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Save some more file real-estate... you get the idea by now, I'm sure. > > :-) > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Lendacky Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption Date: Wed, 14 Jun 2017 15:40:28 -0500 Message-ID: <0611d01a-19f8-d6ae-2682-932789855518@amd.com> References: <20170607191309.28645.15241.stgit@tlendack-t1.amdoffice.net> <20170607191745.28645.81756.stgit@tlendack-t1.amdoffice.net> <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> Content-Language: en-US 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, kexec@lists.infradead.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?= , 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 List-Id: linux-efi@vger.kernel.org On 6/14/2017 12:42 PM, Borislav Petkov wrote: > On Wed, Jun 07, 2017 at 02:17:45PM -0500, Tom Lendacky wrote: >> The IOMMU is programmed with physical addresses for the various tables >> and buffers that are used to communicate between the device and the >> driver. When the driver allocates this memory it is encrypted. In order >> for the IOMMU to access the memory as encrypted the encryption mask needs >> to be included in these physical addresses during configuration. >> >> The PTE entries created by the IOMMU should also include the encryption >> mask so that when the device behind the IOMMU performs a DMA, the DMA >> will be performed to encrypted memory. >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/include/asm/mem_encrypt.h | 7 +++++++ >> arch/x86/mm/mem_encrypt.c | 30 ++++++++++++++++++++++++++++++ >> drivers/iommu/amd_iommu.c | 36 +++++++++++++++++++----------------- >> drivers/iommu/amd_iommu_init.c | 18 ++++++++++++------ >> drivers/iommu/amd_iommu_proto.h | 10 ++++++++++ >> drivers/iommu/amd_iommu_types.h | 2 +- >> include/asm-generic/mem_encrypt.h | 5 +++++ >> 7 files changed, 84 insertions(+), 24 deletions(-) >> >> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h >> index c7a2525..d86e544 100644 >> --- a/arch/x86/include/asm/mem_encrypt.h >> +++ b/arch/x86/include/asm/mem_encrypt.h >> @@ -31,6 +31,8 @@ void __init sme_early_decrypt(resource_size_t paddr, >> >> void __init sme_early_init(void); >> >> +bool sme_iommu_supported(void); >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void); >> >> @@ -62,6 +64,11 @@ static inline void __init sme_early_init(void) >> { >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Some more file real-estate saving: > > static inline bool sme_iommu_supported(void) { return true; } > >> + >> #endif /* CONFIG_AMD_MEM_ENCRYPT */ >> >> static inline bool sme_active(void) >> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c >> index 5d7c51d..018b58a 100644 >> --- a/arch/x86/mm/mem_encrypt.c >> +++ b/arch/x86/mm/mem_encrypt.c >> @@ -197,6 +197,36 @@ void __init sme_early_init(void) >> protection_map[i] = pgprot_encrypted(protection_map[i]); >> } >> >> +bool sme_iommu_supported(void) > > Why is this one exported with all the header file declarations if it is > going to be used in the iommu code only? IOW, you can make it a static > function there and save yourself all the exporting. I was trying to keep all the logic for it here in the SME related files rather than put it in the iommu code itself. But it is easy enough to move if you think it's worth it. > >> +{ >> + struct cpuinfo_x86 *c = &boot_cpu_data; >> + >> + if (!sme_me_mask || (c->x86 != 0x17)) > > me_mask or sme_active()? I like using sme_active() outside of the SME-specific files and using sme_me_mask in the SME-specific files to save any changes that will have to be made once SEV comes around. > > Or is the IOMMU "disabled" in a way the moment the BIOS decides that SME > can be enabled? There's a fix in the AGESA layer of the BIOS that permits the IOMMU to function properly when SME is enabled. Unfortunately, the only easy way to determine if that fix is available is through the patch level check. > > Also, family checks are always a bad idea for enablement. Why do we need > the family check? Because future families will work with the IOMMU? :-) Yes, any future family that supports SME will (should) work with the IOMMU without having to check patch levels. > >> + return true; >> + >> + /* For Fam17h, a specific level of support is required */ >> + switch (c->microcode & 0xf000) { > > Also, you said in another mail on this subthread that c->microcode > is not yet set. Are you saying, that the iommu init gunk runs before > init_amd(), where we do set c->microcode? > > If so, we can move the setting to early_init_amd() or so. I'll look into that. > >> + case 0x0000: >> + return false; >> + case 0x1000: >> + switch (c->microcode & 0x0f00) { >> + case 0x0000: >> + return false; >> + case 0x0100: >> + if ((c->microcode & 0xff) < 0x26) >> + return false; >> + break; >> + case 0x0200: >> + if ((c->microcode & 0xff) < 0x05) >> + return false; >> + break; >> + } > > So this is the microcode revision, why those complex compares? Can't you > simply check a range of values? I'll look into simplifying the checks. > >> + break; >> + } >> + >> + return true; >> +} >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void) >> { >> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >> index 63cacf5..94eb130 100644 >> --- a/drivers/iommu/amd_iommu.c >> +++ b/drivers/iommu/amd_iommu.c >> @@ -544,7 +544,7 @@ static void dump_dte_entry(u16 devid) >> >> static void dump_command(unsigned long phys_addr) >> { >> - struct iommu_cmd *cmd = phys_to_virt(phys_addr); >> + struct iommu_cmd *cmd = iommu_phys_to_virt(phys_addr); >> int i; >> >> for (i = 0; i < 4; ++i) >> @@ -863,13 +863,15 @@ static void copy_cmd_to_buffer(struct amd_iommu *iommu, >> writel(tail, iommu->mmio_base + MMIO_CMD_TAIL_OFFSET); >> } >> >> -static void build_completion_wait(struct iommu_cmd *cmd, u64 address) >> +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > > WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst > #134: FILE: drivers/iommu/amd_iommu.c:866: > +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > The semaphore area is written to by the device so the use of volatile is appropriate in this case. Thanks, Tom >> { >> + u64 address = iommu_virt_to_phys((void *)sem); >> + >> WARN_ON(address & 0x7ULL); >> >> memset(cmd, 0, sizeof(*cmd)); >> - cmd->data[0] = lower_32_bits(__pa(address)) | CMD_COMPL_WAIT_STORE_MASK; >> - cmd->data[1] = upper_32_bits(__pa(address)); >> + cmd->data[0] = lower_32_bits(address) | CMD_COMPL_WAIT_STORE_MASK; >> + cmd->data[1] = upper_32_bits(address); >> cmd->data[2] = 1; >> CMD_SET_TYPE(cmd, CMD_COMPL_WAIT); > > <... snip stuff which Joerg needs to review... > > >> diff --git a/include/asm-generic/mem_encrypt.h b/include/asm-generic/mem_encrypt.h >> index fb02ff0..bbc49e1 100644 >> --- a/include/asm-generic/mem_encrypt.h >> +++ b/include/asm-generic/mem_encrypt.h >> @@ -27,6 +27,11 @@ static inline u64 sme_dma_mask(void) >> return 0ULL; >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Save some more file real-estate... you get the idea by now, I'm sure. > > :-) > -- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg0-f70.google.com (mail-pg0-f70.google.com [74.125.83.70]) by kanga.kvack.org (Postfix) with ESMTP id 5B3326B0279 for ; Wed, 14 Jun 2017 16:40:42 -0400 (EDT) Received: by mail-pg0-f70.google.com with SMTP id d191so10046893pga.15 for ; Wed, 14 Jun 2017 13:40:42 -0700 (PDT) Received: from NAM03-CO1-obe.outbound.protection.outlook.com (mail-co1nam03on0077.outbound.protection.outlook.com. [104.47.40.77]) by mx.google.com with ESMTPS id 195si698642pgg.302.2017.06.14.13.40.41 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 14 Jun 2017 13:40:41 -0700 (PDT) Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption References: <20170607191309.28645.15241.stgit@tlendack-t1.amdoffice.net> <20170607191745.28645.81756.stgit@tlendack-t1.amdoffice.net> <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> From: Tom Lendacky Message-ID: <0611d01a-19f8-d6ae-2682-932789855518@amd.com> Date: Wed, 14 Jun 2017 15:40:28 -0500 MIME-Version: 1.0 In-Reply-To: <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: owner-linux-mm@kvack.org List-ID: 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, kexec@lists.infradead.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?= , 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 On 6/14/2017 12:42 PM, Borislav Petkov wrote: > On Wed, Jun 07, 2017 at 02:17:45PM -0500, Tom Lendacky wrote: >> The IOMMU is programmed with physical addresses for the various tables >> and buffers that are used to communicate between the device and the >> driver. When the driver allocates this memory it is encrypted. In order >> for the IOMMU to access the memory as encrypted the encryption mask needs >> to be included in these physical addresses during configuration. >> >> The PTE entries created by the IOMMU should also include the encryption >> mask so that when the device behind the IOMMU performs a DMA, the DMA >> will be performed to encrypted memory. >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/include/asm/mem_encrypt.h | 7 +++++++ >> arch/x86/mm/mem_encrypt.c | 30 ++++++++++++++++++++++++++++++ >> drivers/iommu/amd_iommu.c | 36 +++++++++++++++++++----------------- >> drivers/iommu/amd_iommu_init.c | 18 ++++++++++++------ >> drivers/iommu/amd_iommu_proto.h | 10 ++++++++++ >> drivers/iommu/amd_iommu_types.h | 2 +- >> include/asm-generic/mem_encrypt.h | 5 +++++ >> 7 files changed, 84 insertions(+), 24 deletions(-) >> >> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h >> index c7a2525..d86e544 100644 >> --- a/arch/x86/include/asm/mem_encrypt.h >> +++ b/arch/x86/include/asm/mem_encrypt.h >> @@ -31,6 +31,8 @@ void __init sme_early_decrypt(resource_size_t paddr, >> >> void __init sme_early_init(void); >> >> +bool sme_iommu_supported(void); >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void); >> >> @@ -62,6 +64,11 @@ static inline void __init sme_early_init(void) >> { >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Some more file real-estate saving: > > static inline bool sme_iommu_supported(void) { return true; } > >> + >> #endif /* CONFIG_AMD_MEM_ENCRYPT */ >> >> static inline bool sme_active(void) >> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c >> index 5d7c51d..018b58a 100644 >> --- a/arch/x86/mm/mem_encrypt.c >> +++ b/arch/x86/mm/mem_encrypt.c >> @@ -197,6 +197,36 @@ void __init sme_early_init(void) >> protection_map[i] = pgprot_encrypted(protection_map[i]); >> } >> >> +bool sme_iommu_supported(void) > > Why is this one exported with all the header file declarations if it is > going to be used in the iommu code only? IOW, you can make it a static > function there and save yourself all the exporting. I was trying to keep all the logic for it here in the SME related files rather than put it in the iommu code itself. But it is easy enough to move if you think it's worth it. > >> +{ >> + struct cpuinfo_x86 *c = &boot_cpu_data; >> + >> + if (!sme_me_mask || (c->x86 != 0x17)) > > me_mask or sme_active()? I like using sme_active() outside of the SME-specific files and using sme_me_mask in the SME-specific files to save any changes that will have to be made once SEV comes around. > > Or is the IOMMU "disabled" in a way the moment the BIOS decides that SME > can be enabled? There's a fix in the AGESA layer of the BIOS that permits the IOMMU to function properly when SME is enabled. Unfortunately, the only easy way to determine if that fix is available is through the patch level check. > > Also, family checks are always a bad idea for enablement. Why do we need > the family check? Because future families will work with the IOMMU? :-) Yes, any future family that supports SME will (should) work with the IOMMU without having to check patch levels. > >> + return true; >> + >> + /* For Fam17h, a specific level of support is required */ >> + switch (c->microcode & 0xf000) { > > Also, you said in another mail on this subthread that c->microcode > is not yet set. Are you saying, that the iommu init gunk runs before > init_amd(), where we do set c->microcode? > > If so, we can move the setting to early_init_amd() or so. I'll look into that. > >> + case 0x0000: >> + return false; >> + case 0x1000: >> + switch (c->microcode & 0x0f00) { >> + case 0x0000: >> + return false; >> + case 0x0100: >> + if ((c->microcode & 0xff) < 0x26) >> + return false; >> + break; >> + case 0x0200: >> + if ((c->microcode & 0xff) < 0x05) >> + return false; >> + break; >> + } > > So this is the microcode revision, why those complex compares? Can't you > simply check a range of values? I'll look into simplifying the checks. > >> + break; >> + } >> + >> + return true; >> +} >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void) >> { >> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >> index 63cacf5..94eb130 100644 >> --- a/drivers/iommu/amd_iommu.c >> +++ b/drivers/iommu/amd_iommu.c >> @@ -544,7 +544,7 @@ static void dump_dte_entry(u16 devid) >> >> static void dump_command(unsigned long phys_addr) >> { >> - struct iommu_cmd *cmd = phys_to_virt(phys_addr); >> + struct iommu_cmd *cmd = iommu_phys_to_virt(phys_addr); >> int i; >> >> for (i = 0; i < 4; ++i) >> @@ -863,13 +863,15 @@ static void copy_cmd_to_buffer(struct amd_iommu *iommu, >> writel(tail, iommu->mmio_base + MMIO_CMD_TAIL_OFFSET); >> } >> >> -static void build_completion_wait(struct iommu_cmd *cmd, u64 address) >> +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > > WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst > #134: FILE: drivers/iommu/amd_iommu.c:866: > +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > The semaphore area is written to by the device so the use of volatile is appropriate in this case. Thanks, Tom >> { >> + u64 address = iommu_virt_to_phys((void *)sem); >> + >> WARN_ON(address & 0x7ULL); >> >> memset(cmd, 0, sizeof(*cmd)); >> - cmd->data[0] = lower_32_bits(__pa(address)) | CMD_COMPL_WAIT_STORE_MASK; >> - cmd->data[1] = upper_32_bits(__pa(address)); >> + cmd->data[0] = lower_32_bits(address) | CMD_COMPL_WAIT_STORE_MASK; >> + cmd->data[1] = upper_32_bits(address); >> cmd->data[2] = 1; >> CMD_SET_TYPE(cmd, CMD_COMPL_WAIT); > > <... snip stuff which Joerg needs to review... > > >> diff --git a/include/asm-generic/mem_encrypt.h b/include/asm-generic/mem_encrypt.h >> index fb02ff0..bbc49e1 100644 >> --- a/include/asm-generic/mem_encrypt.h >> +++ b/include/asm-generic/mem_encrypt.h >> @@ -27,6 +27,11 @@ static inline u64 sme_dma_mask(void) >> return 0ULL; >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Save some more file real-estate... you get the idea by now, I'm sure. > > :-) > -- 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-cys01nam02on0086.outbound.protection.outlook.com ([104.47.37.86] helo=NAM02-CY1-obe.outbound.protection.outlook.com) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dLF65-00077n-Gh for kexec@lists.infradead.org; Wed, 14 Jun 2017 20:41:03 +0000 Subject: Re: [PATCH v6 26/34] iommu/amd: Allow the AMD IOMMU to work with memory encryption References: <20170607191309.28645.15241.stgit@tlendack-t1.amdoffice.net> <20170607191745.28645.81756.stgit@tlendack-t1.amdoffice.net> <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> From: Tom Lendacky Message-ID: <0611d01a-19f8-d6ae-2682-932789855518@amd.com> Date: Wed, 14 Jun 2017 15:40:28 -0500 MIME-Version: 1.0 In-Reply-To: <20170614174208.p2yr5exs4b6pjxhf@pd.tnic> Content-Language: en-US List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Borislav Petkov Cc: linux-efi@vger.kernel.org, Brijesh Singh , Toshimitsu Kani , =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= , Matt Fleming , x86@kernel.org, linux-mm@kvack.org, Alexander Potapenko , "H. Peter Anvin" , Larry Woodman , linux-arch@vger.kernel.org, kvm@vger.kernel.org, Jonathan Corbet , Joerg Roedel , linux-doc@vger.kernel.org, kasan-dev@googlegroups.com, Ingo Molnar , Andrey Ryabinin , Dave Young , Rik van Riel , Arnd Bergmann , Konrad Rzeszutek Wilk , Andy Lutomirski , Thomas Gleixner , Dmitry Vyukov , kexec@lists.infradead.org, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, "Michael S. Tsirkin" , Paolo Bonzini On 6/14/2017 12:42 PM, Borislav Petkov wrote: > On Wed, Jun 07, 2017 at 02:17:45PM -0500, Tom Lendacky wrote: >> The IOMMU is programmed with physical addresses for the various tables >> and buffers that are used to communicate between the device and the >> driver. When the driver allocates this memory it is encrypted. In order >> for the IOMMU to access the memory as encrypted the encryption mask needs >> to be included in these physical addresses during configuration. >> >> The PTE entries created by the IOMMU should also include the encryption >> mask so that when the device behind the IOMMU performs a DMA, the DMA >> will be performed to encrypted memory. >> >> Signed-off-by: Tom Lendacky >> --- >> arch/x86/include/asm/mem_encrypt.h | 7 +++++++ >> arch/x86/mm/mem_encrypt.c | 30 ++++++++++++++++++++++++++++++ >> drivers/iommu/amd_iommu.c | 36 +++++++++++++++++++----------------- >> drivers/iommu/amd_iommu_init.c | 18 ++++++++++++------ >> drivers/iommu/amd_iommu_proto.h | 10 ++++++++++ >> drivers/iommu/amd_iommu_types.h | 2 +- >> include/asm-generic/mem_encrypt.h | 5 +++++ >> 7 files changed, 84 insertions(+), 24 deletions(-) >> >> diff --git a/arch/x86/include/asm/mem_encrypt.h b/arch/x86/include/asm/mem_encrypt.h >> index c7a2525..d86e544 100644 >> --- a/arch/x86/include/asm/mem_encrypt.h >> +++ b/arch/x86/include/asm/mem_encrypt.h >> @@ -31,6 +31,8 @@ void __init sme_early_decrypt(resource_size_t paddr, >> >> void __init sme_early_init(void); >> >> +bool sme_iommu_supported(void); >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void); >> >> @@ -62,6 +64,11 @@ static inline void __init sme_early_init(void) >> { >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Some more file real-estate saving: > > static inline bool sme_iommu_supported(void) { return true; } > >> + >> #endif /* CONFIG_AMD_MEM_ENCRYPT */ >> >> static inline bool sme_active(void) >> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c >> index 5d7c51d..018b58a 100644 >> --- a/arch/x86/mm/mem_encrypt.c >> +++ b/arch/x86/mm/mem_encrypt.c >> @@ -197,6 +197,36 @@ void __init sme_early_init(void) >> protection_map[i] = pgprot_encrypted(protection_map[i]); >> } >> >> +bool sme_iommu_supported(void) > > Why is this one exported with all the header file declarations if it is > going to be used in the iommu code only? IOW, you can make it a static > function there and save yourself all the exporting. I was trying to keep all the logic for it here in the SME related files rather than put it in the iommu code itself. But it is easy enough to move if you think it's worth it. > >> +{ >> + struct cpuinfo_x86 *c = &boot_cpu_data; >> + >> + if (!sme_me_mask || (c->x86 != 0x17)) > > me_mask or sme_active()? I like using sme_active() outside of the SME-specific files and using sme_me_mask in the SME-specific files to save any changes that will have to be made once SEV comes around. > > Or is the IOMMU "disabled" in a way the moment the BIOS decides that SME > can be enabled? There's a fix in the AGESA layer of the BIOS that permits the IOMMU to function properly when SME is enabled. Unfortunately, the only easy way to determine if that fix is available is through the patch level check. > > Also, family checks are always a bad idea for enablement. Why do we need > the family check? Because future families will work with the IOMMU? :-) Yes, any future family that supports SME will (should) work with the IOMMU without having to check patch levels. > >> + return true; >> + >> + /* For Fam17h, a specific level of support is required */ >> + switch (c->microcode & 0xf000) { > > Also, you said in another mail on this subthread that c->microcode > is not yet set. Are you saying, that the iommu init gunk runs before > init_amd(), where we do set c->microcode? > > If so, we can move the setting to early_init_amd() or so. I'll look into that. > >> + case 0x0000: >> + return false; >> + case 0x1000: >> + switch (c->microcode & 0x0f00) { >> + case 0x0000: >> + return false; >> + case 0x0100: >> + if ((c->microcode & 0xff) < 0x26) >> + return false; >> + break; >> + case 0x0200: >> + if ((c->microcode & 0xff) < 0x05) >> + return false; >> + break; >> + } > > So this is the microcode revision, why those complex compares? Can't you > simply check a range of values? I'll look into simplifying the checks. > >> + break; >> + } >> + >> + return true; >> +} >> + >> /* Architecture __weak replacement functions */ >> void __init mem_encrypt_init(void) >> { >> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c >> index 63cacf5..94eb130 100644 >> --- a/drivers/iommu/amd_iommu.c >> +++ b/drivers/iommu/amd_iommu.c >> @@ -544,7 +544,7 @@ static void dump_dte_entry(u16 devid) >> >> static void dump_command(unsigned long phys_addr) >> { >> - struct iommu_cmd *cmd = phys_to_virt(phys_addr); >> + struct iommu_cmd *cmd = iommu_phys_to_virt(phys_addr); >> int i; >> >> for (i = 0; i < 4; ++i) >> @@ -863,13 +863,15 @@ static void copy_cmd_to_buffer(struct amd_iommu *iommu, >> writel(tail, iommu->mmio_base + MMIO_CMD_TAIL_OFFSET); >> } >> >> -static void build_completion_wait(struct iommu_cmd *cmd, u64 address) >> +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > > WARNING: Use of volatile is usually wrong: see Documentation/process/volatile-considered-harmful.rst > #134: FILE: drivers/iommu/amd_iommu.c:866: > +static void build_completion_wait(struct iommu_cmd *cmd, volatile u64 *sem) > The semaphore area is written to by the device so the use of volatile is appropriate in this case. Thanks, Tom >> { >> + u64 address = iommu_virt_to_phys((void *)sem); >> + >> WARN_ON(address & 0x7ULL); >> >> memset(cmd, 0, sizeof(*cmd)); >> - cmd->data[0] = lower_32_bits(__pa(address)) | CMD_COMPL_WAIT_STORE_MASK; >> - cmd->data[1] = upper_32_bits(__pa(address)); >> + cmd->data[0] = lower_32_bits(address) | CMD_COMPL_WAIT_STORE_MASK; >> + cmd->data[1] = upper_32_bits(address); >> cmd->data[2] = 1; >> CMD_SET_TYPE(cmd, CMD_COMPL_WAIT); > > <... snip stuff which Joerg needs to review... > > >> diff --git a/include/asm-generic/mem_encrypt.h b/include/asm-generic/mem_encrypt.h >> index fb02ff0..bbc49e1 100644 >> --- a/include/asm-generic/mem_encrypt.h >> +++ b/include/asm-generic/mem_encrypt.h >> @@ -27,6 +27,11 @@ static inline u64 sme_dma_mask(void) >> return 0ULL; >> } >> >> +static inline bool sme_iommu_supported(void) >> +{ >> + return true; >> +} > > Save some more file real-estate... you get the idea by now, I'm sure. > > :-) > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec