From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753292AbcKOUdY (ORCPT ); Tue, 15 Nov 2016 15:33:24 -0500 Received: from mail-bl2nam02on0089.outbound.protection.outlook.com ([104.47.38.89]:50519 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752923AbcKOUdR (ORCPT ); Tue, 15 Nov 2016 15:33:17 -0500 Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Thomas.Lendacky@amd.com; Subject: Re: [RFC PATCH v3 13/20] x86: DMA support for memory encryption To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003723.3280.62636.stgit@tlendack-t1.amdoffice.net> <20161115143943.GC2185@potion> <20161115181736.GA14060@potion> CC: , , , , , , , , , Rik van Riel , Arnd Bergmann , Jonathan Corbet , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Borislav Petkov , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov From: Tom Lendacky Message-ID: <55bba6f6-b134-6c7b-fc20-10d2dbf781f1@amd.com> Date: Tue, 15 Nov 2016 14:33: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: <20161115181736.GA14060@potion> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: BLUPR01CA040.prod.exchangelabs.com (10.160.23.30) To CY4PR12MB1143.namprd12.prod.outlook.com (10.168.164.135) X-Microsoft-Exchange-Diagnostics: 1;CY4PR12MB1143;2:giIPE/tcaPD9KgBw1Ml5SxZkw4eu6Yg4E/8a97r0+tpNU1EENUqKb+goDjIPZTyb7VKtbVHAt9DcxE8vc03N8W7PYZBhC5pR80azT1JpqqJw89J7erTMAQ/DZbhRxc+Dn9s/Jd3fn8Ck9oRVgE5n9/sZlEmglcae24piGM0rVGI=;3:Tv2rMgfu/P8mnI1pv3BelYgONV/3zPlF5sW5ArnOybED7NdNvm6/6Rf5FmEgXr4hwr0Bi5fPXLItvc15Lmp9QyYdHMaZFnyFH+SC2SE6htftKmJjZMl7Blok2JYmcujqN/0B4OI0M+Y4TH4iasWnMIgCx2CA8wHouVFf1Sh5xNk=;25:3u2VdIwuOmbWmYFnfDN2JDw9MCWTxB1JnxNo1bZ06rpykN1AJtA8l7p2s9VUzWU4eiv+UQqxMcj0bTGmGiT9hBIGyZRjeF2IOMxoGXNNk872QP6Gvdhy8oY5Y44f06cK4RUxWth9CD1mn9vy/sJVjj1efNhepOUc73Z5HTlyoX9Ht8wmJb1ublZaSnjeBJh9yHvKVOioDqnHkBMOuf1wWK+y7o2ksUdjezKEhqv8XmhXP5Y/jFF/T3zu/xynvuSi2mGv8qKE+eohIo48RTVvRzr2KUKS7CzktF5Q87g14VLeKQ3A4CmTIU+GjYFkyZKqoLPm1SxL1AYa1uzmE7Djvwf2zYBVLpAk+0lg47xPnDifxtIG4qr6gzEUdsNeupeqJ1wH9TfXiBXbBxQXJVifd6DM4J3g6YUpBDw/pgOh1qHXedHyDTbcqhRTS4gc//XjS1FLMjsGd6hkPdS3++g4Pw== X-MS-Office365-Filtering-Correlation-Id: 74fe1995-2399-4879-7f70-08d40d969f37 X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:CY4PR12MB1143; X-Microsoft-Exchange-Diagnostics: 1;CY4PR12MB1143;31:K47ZfEfPtTgJHuF5D6rvYjfbJ8vqdcx8TDLRWi1HvRB0cMb6lp7QOLGIa2f8FL+VGlSfAJakmqgAj6K8QW9VaEl+y0JwVKp1mVi095K0mfAD3oTniYIDMqM5SF/v0IbrYLBmaHHklzW6sUvAp6yfPC/uzfLthGRBZdjVdqtcIpH2X66xbSTeaLIGiA8fq3Beb9o0PEqSTcN9nrxwXc5tbypVun2BnER5v+W/giOB5QbEPVbFMumn25SdFysXgS39F7iNQSh5JDPBXaCHqSLsgA==;20:0fcuWSNiilp19Uq19FwnnS01djnPCm1CLAaRKLP8rRkhhll+2vVMwrWvzZmjQ/9R/atLz6/nOxj+U/KPlBpnphzIkQmWu8iD1OaF4xg6KYXL93BtUQi+wXqXVK9xx70LuA9tjSUZWZ5fM+2I4zHh8EzaTsN0p9XSCs5eNzd/VCwT8hYQtHDHgueiSn9LOIoS5a41gVQ6XSrYQgoIPBTc9hlnqBYd+ApQfG1dVnvvl6Z2dAm9usimCgH/ALVKfcL7+sXFaVTcRrRDz5Dq2PuBBX7yeM0/NFgnJulihx4ALv4zu8uLwK+67L/tBZyqpqo6n0GW7joleTVZEMjoddJQ3XHW7UENHN20leKMU7sojSgVAxgvJPx2ekBzpO7I4poR88Lzjt7LgGPmYp1W3ZN6fJ48akloVOHvx9sjyPANmrBvKfE6taW4wKMRQQm9r/BJMphJiMc+lvw85uK207Nl6mjHVdHmCSLZVutFNkwD7Ot6U8RmJxEmHkOUK2t94czf X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(767451399110); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6060326)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6055026)(6061324);SRVR:CY4PR12MB1143;BCL:0;PCL:0;RULEID:;SRVR:CY4PR12MB1143; X-Microsoft-Exchange-Diagnostics: 1;CY4PR12MB1143;4:ndT1rTgdsYKZLGar3kmeTFrdpOHBpyWGgEgm6M1RQYhm2y2D2PepeO2L8tTXpmWiQyu2EqoO08qJLtjxJP0YxkNnAVmHSfXXixWxGYU11XPxVYZN0IKZfpMdwUPCo8GKDXQdHjdFLGp7e0Hfl+68PmrEUUTik9m/XHt19GW3c9fOHGoWpmQHQ9eJntXKZhGz2EgX1HkZAFHxfgn4oYtadDKAQ4IJADQCxmLLoqB1PshSKcEVGFDeFS6ZDjyEt0Pq3Gzogr/StU2Zv/nRqsblRR1jylHvPiNOpBFN2BIJWdzQfnxfQjbFaIcKcN4gQ8PaKm4Bpo7BXaBSld/IFwXDQ5ktFn7Aec4t+ho5shPGpAvqeOB+dIh5OxvgOaDwjL8KiWrDlRFKsn0e6WQ71OH0DZRFTRqutRY/BJdSLdqEJH/QXgG+AlxjL+yZt013DgVJpqQN8e5yjQDUqtBBM30SN7fKgyIz5Y4b1xQw91nRVZc= X-Forefront-PRVS: 012792EC17 X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10009020)(4630300001)(6049001)(6009001)(7916002)(377454003)(189002)(377424004)(199003)(24454002)(68736007)(92566002)(77096005)(2870700001)(4326007)(2906002)(65806001)(86362001)(8676002)(6116002)(50466002)(23676002)(105586002)(81166006)(3846002)(81156014)(65956001)(66066001)(42186005)(106356001)(31686004)(6916009)(5660300001)(83506001)(31696002)(47776003)(36756003)(97736004)(2950100002)(7736002)(229853002)(4001350100001)(50986999)(33646002)(189998001)(76176999)(54356999)(4001150100001)(7416002)(7846002)(101416001)(93886004)(64126003)(65826007)(110136003)(6666003)(305945005)(217873001);DIR:OUT;SFP:1101;SCL:1;SRVR:CY4PR12MB1143;H:[10.236.64.222];FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtDWTRQUjEyTUIxMTQzOzIzOmMrZGdwb2ZncmxCVkc4akpaWTB4NFp4M1Zr?= =?utf-8?B?V3YyTnA1MngxbHRXMlFuUDBTYmtBMmhBN3JsdzQ0RGFKLzlsNFEzendCNEs5?= =?utf-8?B?ZXF5K2xYaVA5d01UNWVsZTl4R2svK0ZnN0x2YTlRQ2lrMlF6S1ErNGNHUTNo?= =?utf-8?B?eStWdUZnWklPbkdwdlcxYUtHNS9nSS96cjdyb2RaSy9mVEtJMm5zd1hycm1l?= =?utf-8?B?dDViNzZvSVBOazJDY3htT1BXWVNjMVk3V2FaMmhMZFRTalJJQjB5WWRkc0V3?= =?utf-8?B?Zm1PU29QcEIvQVdtUTBUNkpseENDTFB3WmxMcFFhbFNzN2FNOE5TUmJ6UExt?= =?utf-8?B?dmZDUWwzakt5MHJmMm1wZ3FXM05vdUR2MGpuYXRyQlRXay8rakNjQTRpWlo5?= =?utf-8?B?V1A2SEh1TkduSkJvdG9DaVd5UkhRYjJ4WWFoNmlBemFDdUhxdVQ0TG9hczI0?= =?utf-8?B?MUJVZjFqQUNqaHJwNkdFMk5RMjN1VGl3RWR3V25kMzNrSW5BZE1sNlFnSS9I?= =?utf-8?B?UjBkaFhlSVNab2pjdEx1M0tOZFFjbWVxSWpXak9ZOVBnajE0WTZRVmdlWm0v?= =?utf-8?B?eDY3dnNFNjFsb0tTMk1zam1kZ0VVeHkwNTVNQ2hPVDZHdEFSV0tnZllwR3Ra?= =?utf-8?B?a3NFYlZGU3IwMEdLSDh6OWNlUzJNWmxqVTR5QW95WE10UUYvOEVTMkRGOWNJ?= =?utf-8?B?YXNpZmlUYk5qeHArekJkQ1o4bVdGSEx2NTVZbSt1TmFheG4vdHlxakxXbHFu?= =?utf-8?B?dDFJSzdoNEFFRm5DNGJqWUhhcGkrVncwcUNMcmRSQUlzaFBWemZzSkdSMVUv?= =?utf-8?B?a2FYUjhRa3JxblRJaFJvUHJ4YlJqeXlhejhHQ0hiTEhwYzl4S1JyK2dWZnVx?= =?utf-8?B?alBzNERkY3BBc1F5dlVwZ1Yra3ZoNSsrZEVseEdCY2VoYjZGKzNFZTdBYVNU?= =?utf-8?B?QUk4djNlZ25STWNhMUUrL2Q4c3VuUjI4dHphV1pYNFE0d2NsNm12ais3cVhE?= =?utf-8?B?c2poRytvd21kZDJPbEtEa2hmMUtBNHNkeUFrTnowaXk1WVdxa2lWU29FQm9a?= =?utf-8?B?eXhJTXQydmhDbk4yNHdaNXlPcUZKelBPY2U4Q3pHN0Fhako5KzNjQ3NLZjFI?= =?utf-8?B?K0tzUEZSQTNuRWxaOXRaRWZ6czdEeVFaSWxrY1dheGJpMUxQSDh4WWNoZkN1?= =?utf-8?B?SFpvWFZ1KzNaQ1A0Rzh4S2tINDNqT21NUm9nNjhlWTNiZG1mbVd4RzhPLzFT?= =?utf-8?B?YU9rUk9rT3lPV3c5Y3h6WkMrVW5YUHlYOEpTSklMMnNrOVVRc2gvYjhTcm1z?= =?utf-8?B?UzJOb1h1U04vSjRwUkdzTllaM1NOSHAyNzcySW9HcWE5bW1CUTZyRDFKUXA5?= =?utf-8?B?dlg5a1g1aGxacnhmdU9ES1pUTnJEYU95OHd5dkdJU1l6cFhadWppR0I4NHhx?= =?utf-8?B?M1hDY0pwQ3I4UDhrbXNVaTE1MjR4Qm1jcDByRGprQWpxajhqeU5mbnpDUHJK?= =?utf-8?B?eExFQWdWYVBOL2l0aFBOY1F3aVpSWnFQVkhIUnVxeXRUS0VYQjNZVkwrWmZq?= =?utf-8?B?ZVcvQ1B2ZHpoUFZNQndpeFFRK25EcUJTZWo1WGNLYkdMZko2c3BHOFpLd09a?= =?utf-8?B?b0tNWit3VkZ0ZkYzNGRQWmJhakhIRkE5M1JWVTNVMUJ2Ly9keUlSOVp3R0JG?= =?utf-8?B?MG8xMGt6TGZySkNPTmt6b1I2U2dEZ3R3RFV3aHBoK0pRNW5aQnhJSld2UExW?= =?utf-8?B?bStPWUdOdkdSK1JtRncyQ0xWNHB6WTZEUi9DY001ZVdsQlZkZFVaYklLNkFq?= =?utf-8?B?YXN6REtuM0EwNm9QQ01DRjNRWFZxUVZJd2prQjVUd2xBWCtMQTdDVW9JS3JJ?= =?utf-8?Q?YdLL/i5y6t3SzU4QnAIAAx+wZQUuakP5?= X-Microsoft-Exchange-Diagnostics: 1;CY4PR12MB1143;6:Ns/DSXI98nxWKODO0Oq8jbaLmVYX8ObqFWHrIqy5+Ba+gkgcl0vSQKkjyPWvEE4cjn7LhSAmmHl5X24dahEJ1t9IRjWsFWxYXStEuYXuw+pQFMvXBfG7HOzoEtIg7l7e8fOBvuAl2gA0CJu7y9OI076qWf76Vf6S1PxFxrCj+V5PDlDGw+YCy+CuXVT5neLQttGLKvqS5LYIWQQ4HrnVxFmG9JkYoBiL3wZ0JdMyQ5C2yJpakdTiosYzwHcePLpRMkrTJvoXiPxbM14FnIq1sOfkVqFg6HjwCUoFL+YYtn8p5DTrxy0tG6MkWebcYon81sSiOZwiTGz/RNtgJGvHumnIG36Bcw9mRenjultOR7POAUwWlWTl70WhbwIHxU5t;5:x+Q0CW8spGAQtxA5Mli2QX6hNJc7Xz76D5MeMZaFWShLYe/R5VpVecbzCtNO0osqvU0Ov2/WGPrS59V6gf+1rRxMeLfyZbw6Iecoj7zPnQDFLCDTHvvD7k3hJ8kh3F3mRT/zSgXRq6fitLKu/kPBOw==;24:YNhAoeELXAm9c5+ESVk7VuFcrsVPE1NwcrEsL60x+d+h8N9WO0Pa8WneOxbexkfCsWvKiqRYl5be/WfbnFr0cssnVQTYTGqYW9/z8dwI+BA= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;CY4PR12MB1143;7:Os48aI+1lPuXb463ksbPAozciFdZR29RxddHdEIy5iVdxjT/lAFp4fdGTvHkScQZxfhD5Sa8/RXdN3KKflNbGaFO3/bNd1DLtVIhlOtxrU/ADtZFRJjmaP4kTvNNeRnK9wpvoZc5yFa+xrRDIe6cpYicP3Zt5R2d54LKRe8DBeDN0AW//2Ba8Omfsnzf7hHVdXwl79+F5MNSmOazntWthNK7pzLQ5fo6RqUb27ipBfsb8ZLpvrkdrOw5d3J1dA4ESzIBhghyHNhEyMN20ynECaNA6TMuk9lF0GpVMuHvLllyGFtfMitaLHXOLhoihPqdheKlpGmf+OewwzfK2BYBrUEiF2991YFLGs/V7XIshEc=;20:OnEmVCBNxtIzJevLXj3N85QctvZUo2nOvjbOYRmWBv1XXDDrY5C30pBR06nQJU78nEwtO8YJoTy03Sw/HIW3f/LLdVWqhlqBliJ/danFK216YgRwbCMGPTGOqcg4VxDJ2RB51WeX/8BV48VF3epJF+C9aFszqPAPSwhz4kgB8muyGOmdQvqWGTQ/IyiS7i3pQhEph/pNt+6FWFZ0HgQECgFxyNdaFZd+lsd38JsBaYpXVZgsI2la7Q0IERorMsXx X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Nov 2016 20:33:10.7100 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR12MB1143 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 11/15/2016 12:17 PM, Radim Krčmář wrote: > 2016-11-15 11:02-0600, Tom Lendacky: >> On 11/15/2016 8:39 AM, Radim Krčmář wrote: >>> 2016-11-09 18:37-0600, Tom Lendacky: >>>> Since DMA addresses will effectively look like 48-bit addresses when the >>>> memory encryption mask is set, SWIOTLB is needed if the DMA mask of the >>>> device performing the DMA does not support 48-bits. SWIOTLB will be >>>> initialized to create un-encrypted bounce buffers for use by these devices. >>>> >>>> Signed-off-by: Tom Lendacky >>>> --- >>>> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c >>>> @@ -64,13 +66,15 @@ static struct dma_map_ops swiotlb_dma_ops = { >>>> * pci_swiotlb_detect_override - set swiotlb to 1 if necessary >>>> * >>>> * This returns non-zero if we are forced to use swiotlb (by the boot >>>> - * option). >>>> + * option). If memory encryption is enabled then swiotlb will be set >>>> + * to 1 so that bounce buffers are allocated and used for devices that >>>> + * do not support the addressing range required for the encryption mask. >>>> */ >>>> int __init pci_swiotlb_detect_override(void) >>>> { >>>> int use_swiotlb = swiotlb | swiotlb_force; >>>> >>>> - if (swiotlb_force) >>>> + if (swiotlb_force || sme_me_mask) >>>> swiotlb = 1; >>>> >>>> return use_swiotlb; >>> >>> We want to return 1 even if only sme_me_mask is 1, because the return >>> value is used for detection. The following would be less obscure, IMO: >>> >>> if (swiotlb_force || sme_me_mask) >>> swiotlb = 1; >>> >>> return swiotlb; >> >> If we do that then all DMA would go through the swiotlb bounce buffers. > > No, that is decided for example in swiotlb_map_page() and we need to > call pci_swiotlb_init() to register that function. > >> By setting swiotlb to 1 we indicate that the bounce buffers will be >> needed for those devices that can't support the addressing range when >> the encryption bit is set (48 bit DMA). But if the device can support >> the addressing range we won't use the bounce buffers. > > If we return 0 here, then pci_swiotlb_init() will not be called => > dma_ops won't be set to swiotlb_dma_ops => we won't use bounce buffers. > Ok, I see why this was working for me... By setting swiotlb = 1 and returning 0 it was continuing to the pci_swiotlb_detect_4gb table which would return the current value of swiotlb, which is 1, and so the swiotlb ops were setup. So the change that you mentioned will work, thanks for pointing that out and getting me to dig deeper on it. I'll update the patch. >>> We setup encrypted swiotlb and then decrypt it, but sometimes set it up >>> decrypted (late_alloc) ... why isn't the swiotlb set up decrypted >>> directly? >> >> When swiotlb is allocated in swiotlb_init(), it is too early to make >> use of the api to the change the page attributes. Because of this, >> the callback to make those changes is needed. > > Thanks. (I don't know page table setup enough to see a lesser evil. :]) > >>>> @@ -541,7 +583,7 @@ static phys_addr_t >>>> map_single(struct device *hwdev, phys_addr_t phys, size_t size, >>>> enum dma_data_direction dir) >>>> { >>>> - dma_addr_t start_dma_addr = phys_to_dma(hwdev, io_tlb_start); >>>> + dma_addr_t start_dma_addr = swiotlb_phys_to_dma(hwdev, io_tlb_start); >>> >>> We have decrypted io_tlb_start before, so shouldn't its physical address >>> be saved without the sme bit? (Which changes a lot ...) >> >> I'm not sure what you mean here, can you elaborate a bit more? > > The C-bit (sme bit) is a part of the physical address. The C-bit (sme_me_mask) isn't part of the physical address for io_tlb_start, but since the original call was to phys_to_dma(), which now will automatically "or" in the C-bit, I needed to adjust that by using swiotlb_phys_to_dma() to remove the C-bit. > If we know that a certain physical page should be accessed as > unencrypted (the bounce buffer) then the C-bit is 0. > I'm wondering why we save the physical address with the C-bit set when > we know that it can't be accessed that way (because we remove it every > time). It's not saved with the C-bit, but the phys_to_dma call will "or" in the C-bit automatically. And since this is common code I need to leave that call to phys_to_dma in. > > The naming is a bit confusing, because physical addresses are actually > virtualized by SME -- maybe we should be calling them SME addresses? Interesting idea... I'll have to look at how that plays out in the patches and documentation. Thanks, Tom > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Lendacky Subject: Re: [RFC PATCH v3 13/20] x86: DMA support for memory encryption Date: Tue, 15 Nov 2016 14:33:06 -0600 Message-ID: <55bba6f6-b134-6c7b-fc20-10d2dbf781f1@amd.com> References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003723.3280.62636.stgit@tlendack-t1.amdoffice.net> <20161115143943.GC2185@potion> <20161115181736.GA14060@potion> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20161115181736.GA14060@potion> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-doc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Matt Fleming , linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, Alexander Potapenko , "H. Peter Anvin" , Larry Woodman , linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Jonathan Corbet , x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, kasan-dev-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org, Ingo Molnar , Andrey Ryabinin , Rik van Riel , Arnd Bergmann , Borislav Petkov , Andy Lutomirski , Thomas Gleixner , Dmitry Vyukov , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Paolo Bonzini List-Id: linux-efi@vger.kernel.org T24gMTEvMTUvMjAxNiAxMjoxNyBQTSwgUmFkaW0gS3LEjW3DocWZIHdyb3RlOgo+IDIwMTYtMTEt MTUgMTE6MDItMDYwMCwgVG9tIExlbmRhY2t5Ogo+PiBPbiAxMS8xNS8yMDE2IDg6MzkgQU0sIFJh ZGltIEtyxI1tw6HFmSB3cm90ZToKPj4+IDIwMTYtMTEtMDkgMTg6MzctMDYwMCwgVG9tIExlbmRh Y2t5Ogo+Pj4+IFNpbmNlIERNQSBhZGRyZXNzZXMgd2lsbCBlZmZlY3RpdmVseSBsb29rIGxpa2Ug NDgtYml0IGFkZHJlc3NlcyB3aGVuIHRoZQo+Pj4+IG1lbW9yeSBlbmNyeXB0aW9uIG1hc2sgaXMg c2V0LCBTV0lPVExCIGlzIG5lZWRlZCBpZiB0aGUgRE1BIG1hc2sgb2YgdGhlCj4+Pj4gZGV2aWNl IHBlcmZvcm1pbmcgdGhlIERNQSBkb2VzIG5vdCBzdXBwb3J0IDQ4LWJpdHMuIFNXSU9UTEIgd2ls bCBiZQo+Pj4+IGluaXRpYWxpemVkIHRvIGNyZWF0ZSB1bi1lbmNyeXB0ZWQgYm91bmNlIGJ1ZmZl cnMgZm9yIHVzZSBieSB0aGVzZSBkZXZpY2VzLgo+Pj4+Cj4+Pj4gU2lnbmVkLW9mZi1ieTogVG9t IExlbmRhY2t5IDx0aG9tYXMubGVuZGFja3lAYW1kLmNvbT4KPj4+PiAtLS0KPj4+PiBkaWZmIC0t Z2l0IGEvYXJjaC94ODYva2VybmVsL3BjaS1zd2lvdGxiLmMgYi9hcmNoL3g4Ni9rZXJuZWwvcGNp LXN3aW90bGIuYwo+Pj4+IEBAIC02NCwxMyArNjYsMTUgQEAgc3RhdGljIHN0cnVjdCBkbWFfbWFw X29wcyBzd2lvdGxiX2RtYV9vcHMgPSB7Cj4+Pj4gICAqIHBjaV9zd2lvdGxiX2RldGVjdF9vdmVy cmlkZSAtIHNldCBzd2lvdGxiIHRvIDEgaWYgbmVjZXNzYXJ5Cj4+Pj4gICAqCj4+Pj4gICAqIFRo aXMgcmV0dXJucyBub24temVybyBpZiB3ZSBhcmUgZm9yY2VkIHRvIHVzZSBzd2lvdGxiIChieSB0 aGUgYm9vdAo+Pj4+IC0gKiBvcHRpb24pLgo+Pj4+ICsgKiBvcHRpb24pLiBJZiBtZW1vcnkgZW5j cnlwdGlvbiBpcyBlbmFibGVkIHRoZW4gc3dpb3RsYiB3aWxsIGJlIHNldAo+Pj4+ICsgKiB0byAx IHNvIHRoYXQgYm91bmNlIGJ1ZmZlcnMgYXJlIGFsbG9jYXRlZCBhbmQgdXNlZCBmb3IgZGV2aWNl cyB0aGF0Cj4+Pj4gKyAqIGRvIG5vdCBzdXBwb3J0IHRoZSBhZGRyZXNzaW5nIHJhbmdlIHJlcXVp cmVkIGZvciB0aGUgZW5jcnlwdGlvbiBtYXNrLgo+Pj4+ICAgKi8KPj4+PiAgaW50IF9faW5pdCBw Y2lfc3dpb3RsYl9kZXRlY3Rfb3ZlcnJpZGUodm9pZCkKPj4+PiAgewo+Pj4+ICAJaW50IHVzZV9z d2lvdGxiID0gc3dpb3RsYiB8IHN3aW90bGJfZm9yY2U7Cj4+Pj4gIAo+Pj4+IC0JaWYgKHN3aW90 bGJfZm9yY2UpCj4+Pj4gKwlpZiAoc3dpb3RsYl9mb3JjZSB8fCBzbWVfbWVfbWFzaykKPj4+PiAg CQlzd2lvdGxiID0gMTsKPj4+PiAgCj4+Pj4gIAlyZXR1cm4gdXNlX3N3aW90bGI7Cj4+Pgo+Pj4g V2Ugd2FudCB0byByZXR1cm4gMSBldmVuIGlmIG9ubHkgc21lX21lX21hc2sgaXMgMSwgYmVjYXVz ZSB0aGUgcmV0dXJuCj4+PiB2YWx1ZSBpcyB1c2VkIGZvciBkZXRlY3Rpb24uICBUaGUgZm9sbG93 aW5nIHdvdWxkIGJlIGxlc3Mgb2JzY3VyZSwgSU1POgo+Pj4KPj4+IAlpZiAoc3dpb3RsYl9mb3Jj ZSB8fCBzbWVfbWVfbWFzaykKPj4+IAkJc3dpb3RsYiA9IDE7Cj4+Pgo+Pj4gCXJldHVybiBzd2lv dGxiOwo+Pgo+PiBJZiB3ZSBkbyB0aGF0IHRoZW4gYWxsIERNQSB3b3VsZCBnbyB0aHJvdWdoIHRo ZSBzd2lvdGxiIGJvdW5jZSBidWZmZXJzLgo+IAo+IE5vLCB0aGF0IGlzIGRlY2lkZWQgZm9yIGV4 YW1wbGUgaW4gc3dpb3RsYl9tYXBfcGFnZSgpIGFuZCB3ZSBuZWVkIHRvCj4gY2FsbCBwY2lfc3dp b3RsYl9pbml0KCkgdG8gcmVnaXN0ZXIgdGhhdCBmdW5jdGlvbi4KPiAKPj4gQnkgc2V0dGluZyBz d2lvdGxiIHRvIDEgd2UgaW5kaWNhdGUgdGhhdCB0aGUgYm91bmNlIGJ1ZmZlcnMgd2lsbCBiZQo+ PiBuZWVkZWQgZm9yIHRob3NlIGRldmljZXMgdGhhdCBjYW4ndCBzdXBwb3J0IHRoZSBhZGRyZXNz aW5nIHJhbmdlIHdoZW4KPj4gdGhlIGVuY3J5cHRpb24gYml0IGlzIHNldCAoNDggYml0IERNQSku IEJ1dCBpZiB0aGUgZGV2aWNlIGNhbiBzdXBwb3J0Cj4+IHRoZSBhZGRyZXNzaW5nIHJhbmdlIHdl IHdvbid0IHVzZSB0aGUgYm91bmNlIGJ1ZmZlcnMuCj4gCj4gSWYgd2UgcmV0dXJuIDAgaGVyZSwg dGhlbiBwY2lfc3dpb3RsYl9pbml0KCkgd2lsbCBub3QgYmUgY2FsbGVkID0+Cj4gZG1hX29wcyB3 b24ndCBiZSBzZXQgdG8gc3dpb3RsYl9kbWFfb3BzID0+IHdlIHdvbid0IHVzZSBib3VuY2UgYnVm ZmVycy4KPiAKCk9rLCBJIHNlZSB3aHkgdGhpcyB3YXMgd29ya2luZyBmb3IgbWUuLi4gIEJ5IHNl dHRpbmcgc3dpb3RsYiA9IDEgYW5kCnJldHVybmluZyAwIGl0IHdhcyBjb250aW51aW5nIHRvIHRo ZSBwY2lfc3dpb3RsYl9kZXRlY3RfNGdiIHRhYmxlIHdoaWNoCndvdWxkIHJldHVybiB0aGUgY3Vy cmVudCB2YWx1ZSBvZiBzd2lvdGxiLCB3aGljaCBpcyAxLCBhbmQgc28gdGhlCnN3aW90bGIgb3Bz IHdlcmUgc2V0dXAuCgpTbyB0aGUgY2hhbmdlIHRoYXQgeW91IG1lbnRpb25lZCB3aWxsIHdvcmss IHRoYW5rcyBmb3IgcG9pbnRpbmcgdGhhdCBvdXQKYW5kIGdldHRpbmcgbWUgdG8gZGlnIGRlZXBl ciBvbiBpdC4gIEknbGwgdXBkYXRlIHRoZSBwYXRjaC4KCj4+PiBXZSBzZXR1cCBlbmNyeXB0ZWQg c3dpb3RsYiBhbmQgdGhlbiBkZWNyeXB0IGl0LCBidXQgc29tZXRpbWVzIHNldCBpdCB1cAo+Pj4g ZGVjcnlwdGVkIChsYXRlX2FsbG9jKSAuLi4gd2h5IGlzbid0IHRoZSBzd2lvdGxiIHNldCB1cCBk ZWNyeXB0ZWQKPj4+IGRpcmVjdGx5Pwo+Pgo+PiBXaGVuIHN3aW90bGIgaXMgYWxsb2NhdGVkIGlu IHN3aW90bGJfaW5pdCgpLCBpdCBpcyB0b28gZWFybHkgdG8gbWFrZQo+PiB1c2Ugb2YgdGhlIGFw aSB0byB0aGUgY2hhbmdlIHRoZSBwYWdlIGF0dHJpYnV0ZXMuIEJlY2F1c2Ugb2YgdGhpcywKPj4g dGhlIGNhbGxiYWNrIHRvIG1ha2UgdGhvc2UgY2hhbmdlcyBpcyBuZWVkZWQuCj4gCj4gVGhhbmtz LiAoSSBkb24ndCBrbm93IHBhZ2UgdGFibGUgc2V0dXAgZW5vdWdoIHRvIHNlZSBhIGxlc3NlciBl dmlsLiA6XSkKPiAKPj4+PiBAQCAtNTQxLDcgKzU4Myw3IEBAIHN0YXRpYyBwaHlzX2FkZHJfdAo+ Pj4+ICBtYXBfc2luZ2xlKHN0cnVjdCBkZXZpY2UgKmh3ZGV2LCBwaHlzX2FkZHJfdCBwaHlzLCBz aXplX3Qgc2l6ZSwKPj4+PiAgCSAgIGVudW0gZG1hX2RhdGFfZGlyZWN0aW9uIGRpcikKPj4+PiAg ewo+Pj4+IC0JZG1hX2FkZHJfdCBzdGFydF9kbWFfYWRkciA9IHBoeXNfdG9fZG1hKGh3ZGV2LCBp b190bGJfc3RhcnQpOwo+Pj4+ICsJZG1hX2FkZHJfdCBzdGFydF9kbWFfYWRkciA9IHN3aW90bGJf cGh5c190b19kbWEoaHdkZXYsIGlvX3RsYl9zdGFydCk7Cj4+Pgo+Pj4gV2UgaGF2ZSBkZWNyeXB0 ZWQgaW9fdGxiX3N0YXJ0IGJlZm9yZSwgc28gc2hvdWxkbid0IGl0cyBwaHlzaWNhbCBhZGRyZXNz Cj4+PiBiZSBzYXZlZCB3aXRob3V0IHRoZSBzbWUgYml0PyAgKFdoaWNoIGNoYW5nZXMgYSBsb3Qg Li4uKQo+Pgo+PiBJJ20gbm90IHN1cmUgd2hhdCB5b3UgbWVhbiBoZXJlLCBjYW4geW91IGVsYWJv cmF0ZSBhIGJpdCBtb3JlPwo+IAo+IFRoZSBDLWJpdCAoc21lIGJpdCkgaXMgYSBwYXJ0IG9mIHRo ZSBwaHlzaWNhbCBhZGRyZXNzLgoKVGhlIEMtYml0IChzbWVfbWVfbWFzaykgaXNuJ3QgcGFydCBv ZiB0aGUgcGh5c2ljYWwgYWRkcmVzcyBmb3IKaW9fdGxiX3N0YXJ0LCBidXQgc2luY2UgdGhlIG9y aWdpbmFsIGNhbGwgd2FzIHRvIHBoeXNfdG9fZG1hKCksIHdoaWNoCm5vdyB3aWxsIGF1dG9tYXRp Y2FsbHkgIm9yIiBpbiB0aGUgQy1iaXQsIEkgbmVlZGVkIHRvIGFkanVzdCB0aGF0IGJ5CnVzaW5n IHN3aW90bGJfcGh5c190b19kbWEoKSB0byByZW1vdmUgdGhlIEMtYml0LgoKPiBJZiB3ZSBrbm93 IHRoYXQgYSBjZXJ0YWluIHBoeXNpY2FsIHBhZ2Ugc2hvdWxkIGJlIGFjY2Vzc2VkIGFzCj4gdW5l bmNyeXB0ZWQgKHRoZSBib3VuY2UgYnVmZmVyKSB0aGVuIHRoZSBDLWJpdCBpcyAwLgo+IEknbSB3 b25kZXJpbmcgd2h5IHdlIHNhdmUgdGhlIHBoeXNpY2FsIGFkZHJlc3Mgd2l0aCB0aGUgQy1iaXQg c2V0IHdoZW4KPiB3ZSBrbm93IHRoYXQgaXQgY2FuJ3QgYmUgYWNjZXNzZWQgdGhhdCB3YXkgKGJl Y2F1c2Ugd2UgcmVtb3ZlIGl0IGV2ZXJ5Cj4gdGltZSkuCgpJdCdzIG5vdCBzYXZlZCB3aXRoIHRo ZSBDLWJpdCwgYnV0IHRoZSBwaHlzX3RvX2RtYSBjYWxsIHdpbGwgIm9yIiBpbiB0aGUKQy1iaXQg YXV0b21hdGljYWxseS4gIEFuZCBzaW5jZSB0aGlzIGlzIGNvbW1vbiBjb2RlIEkgbmVlZCB0byBs ZWF2ZSB0aGF0CmNhbGwgdG8gcGh5c190b19kbWEgaW4uCgo+IAo+IFRoZSBuYW1pbmcgaXMgYSBi aXQgY29uZnVzaW5nLCBiZWNhdXNlIHBoeXNpY2FsIGFkZHJlc3NlcyBhcmUgYWN0dWFsbHkKPiB2 aXJ0dWFsaXplZCBieSBTTUUgLS0gbWF5YmUgd2Ugc2hvdWxkIGJlIGNhbGxpbmcgdGhlbSBTTUUg YWRkcmVzc2VzPwoKSW50ZXJlc3RpbmcgaWRlYS4uLiAgSSdsbCBoYXZlIHRvIGxvb2sgYXQgaG93 IHRoYXQgcGxheXMgb3V0IGluIHRoZQpwYXRjaGVzIGFuZCBkb2N1bWVudGF0aW9uLgoKVGhhbmtz LApUb20KCj4gCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f CmlvbW11IG1haWxpbmcgbGlzdAppb21tdUBsaXN0cy5saW51eC1mb3VuZGF0aW9uLm9yZwpodHRw czovL2xpc3RzLmxpbnV4Zm91bmRhdGlvbi5vcmcvbWFpbG1hbi9saXN0aW5mby9pb21tdQ== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bl2nam02on0089.outbound.protection.outlook.com ([104.47.38.89]:50519 "EHLO NAM02-BL2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752923AbcKOUdR (ORCPT ); Tue, 15 Nov 2016 15:33:17 -0500 Subject: Re: [RFC PATCH v3 13/20] x86: DMA support for memory encryption References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003723.3280.62636.stgit@tlendack-t1.amdoffice.net> <20161115143943.GC2185@potion> <20161115181736.GA14060@potion> From: Tom Lendacky Message-ID: <55bba6f6-b134-6c7b-fc20-10d2dbf781f1@amd.com> Date: Tue, 15 Nov 2016 14:33:06 -0600 MIME-Version: 1.0 In-Reply-To: <20161115181736.GA14060@potion> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= 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 , Arnd Bergmann , Jonathan Corbet , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Borislav Petkov , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov Message-ID: <20161115203306.t6-QZGtotwcnBmQ4I04PNUcka8doo740ZtgqGWRbUbk@z> On 11/15/2016 12:17 PM, Radim Krčmář wrote: > 2016-11-15 11:02-0600, Tom Lendacky: >> On 11/15/2016 8:39 AM, Radim Krčmář wrote: >>> 2016-11-09 18:37-0600, Tom Lendacky: >>>> Since DMA addresses will effectively look like 48-bit addresses when the >>>> memory encryption mask is set, SWIOTLB is needed if the DMA mask of the >>>> device performing the DMA does not support 48-bits. SWIOTLB will be >>>> initialized to create un-encrypted bounce buffers for use by these devices. >>>> >>>> Signed-off-by: Tom Lendacky >>>> --- >>>> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c >>>> @@ -64,13 +66,15 @@ static struct dma_map_ops swiotlb_dma_ops = { >>>> * pci_swiotlb_detect_override - set swiotlb to 1 if necessary >>>> * >>>> * This returns non-zero if we are forced to use swiotlb (by the boot >>>> - * option). >>>> + * option). If memory encryption is enabled then swiotlb will be set >>>> + * to 1 so that bounce buffers are allocated and used for devices that >>>> + * do not support the addressing range required for the encryption mask. >>>> */ >>>> int __init pci_swiotlb_detect_override(void) >>>> { >>>> int use_swiotlb = swiotlb | swiotlb_force; >>>> >>>> - if (swiotlb_force) >>>> + if (swiotlb_force || sme_me_mask) >>>> swiotlb = 1; >>>> >>>> return use_swiotlb; >>> >>> We want to return 1 even if only sme_me_mask is 1, because the return >>> value is used for detection. The following would be less obscure, IMO: >>> >>> if (swiotlb_force || sme_me_mask) >>> swiotlb = 1; >>> >>> return swiotlb; >> >> If we do that then all DMA would go through the swiotlb bounce buffers. > > No, that is decided for example in swiotlb_map_page() and we need to > call pci_swiotlb_init() to register that function. > >> By setting swiotlb to 1 we indicate that the bounce buffers will be >> needed for those devices that can't support the addressing range when >> the encryption bit is set (48 bit DMA). But if the device can support >> the addressing range we won't use the bounce buffers. > > If we return 0 here, then pci_swiotlb_init() will not be called => > dma_ops won't be set to swiotlb_dma_ops => we won't use bounce buffers. > Ok, I see why this was working for me... By setting swiotlb = 1 and returning 0 it was continuing to the pci_swiotlb_detect_4gb table which would return the current value of swiotlb, which is 1, and so the swiotlb ops were setup. So the change that you mentioned will work, thanks for pointing that out and getting me to dig deeper on it. I'll update the patch. >>> We setup encrypted swiotlb and then decrypt it, but sometimes set it up >>> decrypted (late_alloc) ... why isn't the swiotlb set up decrypted >>> directly? >> >> When swiotlb is allocated in swiotlb_init(), it is too early to make >> use of the api to the change the page attributes. Because of this, >> the callback to make those changes is needed. > > Thanks. (I don't know page table setup enough to see a lesser evil. :]) > >>>> @@ -541,7 +583,7 @@ static phys_addr_t >>>> map_single(struct device *hwdev, phys_addr_t phys, size_t size, >>>> enum dma_data_direction dir) >>>> { >>>> - dma_addr_t start_dma_addr = phys_to_dma(hwdev, io_tlb_start); >>>> + dma_addr_t start_dma_addr = swiotlb_phys_to_dma(hwdev, io_tlb_start); >>> >>> We have decrypted io_tlb_start before, so shouldn't its physical address >>> be saved without the sme bit? (Which changes a lot ...) >> >> I'm not sure what you mean here, can you elaborate a bit more? > > The C-bit (sme bit) is a part of the physical address. The C-bit (sme_me_mask) isn't part of the physical address for io_tlb_start, but since the original call was to phys_to_dma(), which now will automatically "or" in the C-bit, I needed to adjust that by using swiotlb_phys_to_dma() to remove the C-bit. > If we know that a certain physical page should be accessed as > unencrypted (the bounce buffer) then the C-bit is 0. > I'm wondering why we save the physical address with the C-bit set when > we know that it can't be accessed that way (because we remove it every > time). It's not saved with the C-bit, but the phys_to_dma call will "or" in the C-bit automatically. And since this is common code I need to leave that call to phys_to_dma in. > > The naming is a bit confusing, because physical addresses are actually > virtualized by SME -- maybe we should be calling them SME addresses? Interesting idea... I'll have to look at how that plays out in the patches and documentation. Thanks, Tom > 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 CB4AD6B02B3 for ; Tue, 15 Nov 2016 15:33:17 -0500 (EST) Received: by mail-pg0-f70.google.com with SMTP id 3so116269902pgd.3 for ; Tue, 15 Nov 2016 12:33:17 -0800 (PST) Received: from NAM02-BL2-obe.outbound.protection.outlook.com (mail-bl2nam02on0068.outbound.protection.outlook.com. [104.47.38.68]) by mx.google.com with ESMTPS id r11si27921179pfk.40.2016.11.15.12.33.16 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 15 Nov 2016 12:33:16 -0800 (PST) Subject: Re: [RFC PATCH v3 13/20] x86: DMA support for memory encryption References: <20161110003426.3280.2999.stgit@tlendack-t1.amdoffice.net> <20161110003723.3280.62636.stgit@tlendack-t1.amdoffice.net> <20161115143943.GC2185@potion> <20161115181736.GA14060@potion> From: Tom Lendacky Message-ID: <55bba6f6-b134-6c7b-fc20-10d2dbf781f1@amd.com> Date: Tue, 15 Nov 2016 14:33:06 -0600 MIME-Version: 1.0 In-Reply-To: <20161115181736.GA14060@potion> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit Sender: owner-linux-mm@kvack.org List-ID: To: =?UTF-8?B?UmFkaW0gS3LEjW3DocWZ?= 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 , Arnd Bergmann , Jonathan Corbet , Matt Fleming , Joerg Roedel , Konrad Rzeszutek Wilk , Paolo Bonzini , Larry Woodman , Ingo Molnar , Borislav Petkov , Andy Lutomirski , "H. Peter Anvin" , Andrey Ryabinin , Alexander Potapenko , Thomas Gleixner , Dmitry Vyukov On 11/15/2016 12:17 PM, Radim KrA?mA!A? wrote: > 2016-11-15 11:02-0600, Tom Lendacky: >> On 11/15/2016 8:39 AM, Radim KrA?mA!A? wrote: >>> 2016-11-09 18:37-0600, Tom Lendacky: >>>> Since DMA addresses will effectively look like 48-bit addresses when the >>>> memory encryption mask is set, SWIOTLB is needed if the DMA mask of the >>>> device performing the DMA does not support 48-bits. SWIOTLB will be >>>> initialized to create un-encrypted bounce buffers for use by these devices. >>>> >>>> Signed-off-by: Tom Lendacky >>>> --- >>>> diff --git a/arch/x86/kernel/pci-swiotlb.c b/arch/x86/kernel/pci-swiotlb.c >>>> @@ -64,13 +66,15 @@ static struct dma_map_ops swiotlb_dma_ops = { >>>> * pci_swiotlb_detect_override - set swiotlb to 1 if necessary >>>> * >>>> * This returns non-zero if we are forced to use swiotlb (by the boot >>>> - * option). >>>> + * option). If memory encryption is enabled then swiotlb will be set >>>> + * to 1 so that bounce buffers are allocated and used for devices that >>>> + * do not support the addressing range required for the encryption mask. >>>> */ >>>> int __init pci_swiotlb_detect_override(void) >>>> { >>>> int use_swiotlb = swiotlb | swiotlb_force; >>>> >>>> - if (swiotlb_force) >>>> + if (swiotlb_force || sme_me_mask) >>>> swiotlb = 1; >>>> >>>> return use_swiotlb; >>> >>> We want to return 1 even if only sme_me_mask is 1, because the return >>> value is used for detection. The following would be less obscure, IMO: >>> >>> if (swiotlb_force || sme_me_mask) >>> swiotlb = 1; >>> >>> return swiotlb; >> >> If we do that then all DMA would go through the swiotlb bounce buffers. > > No, that is decided for example in swiotlb_map_page() and we need to > call pci_swiotlb_init() to register that function. > >> By setting swiotlb to 1 we indicate that the bounce buffers will be >> needed for those devices that can't support the addressing range when >> the encryption bit is set (48 bit DMA). But if the device can support >> the addressing range we won't use the bounce buffers. > > If we return 0 here, then pci_swiotlb_init() will not be called => > dma_ops won't be set to swiotlb_dma_ops => we won't use bounce buffers. > Ok, I see why this was working for me... By setting swiotlb = 1 and returning 0 it was continuing to the pci_swiotlb_detect_4gb table which would return the current value of swiotlb, which is 1, and so the swiotlb ops were setup. So the change that you mentioned will work, thanks for pointing that out and getting me to dig deeper on it. I'll update the patch. >>> We setup encrypted swiotlb and then decrypt it, but sometimes set it up >>> decrypted (late_alloc) ... why isn't the swiotlb set up decrypted >>> directly? >> >> When swiotlb is allocated in swiotlb_init(), it is too early to make >> use of the api to the change the page attributes. Because of this, >> the callback to make those changes is needed. > > Thanks. (I don't know page table setup enough to see a lesser evil. :]) > >>>> @@ -541,7 +583,7 @@ static phys_addr_t >>>> map_single(struct device *hwdev, phys_addr_t phys, size_t size, >>>> enum dma_data_direction dir) >>>> { >>>> - dma_addr_t start_dma_addr = phys_to_dma(hwdev, io_tlb_start); >>>> + dma_addr_t start_dma_addr = swiotlb_phys_to_dma(hwdev, io_tlb_start); >>> >>> We have decrypted io_tlb_start before, so shouldn't its physical address >>> be saved without the sme bit? (Which changes a lot ...) >> >> I'm not sure what you mean here, can you elaborate a bit more? > > The C-bit (sme bit) is a part of the physical address. The C-bit (sme_me_mask) isn't part of the physical address for io_tlb_start, but since the original call was to phys_to_dma(), which now will automatically "or" in the C-bit, I needed to adjust that by using swiotlb_phys_to_dma() to remove the C-bit. > If we know that a certain physical page should be accessed as > unencrypted (the bounce buffer) then the C-bit is 0. > I'm wondering why we save the physical address with the C-bit set when > we know that it can't be accessed that way (because we remove it every > time). It's not saved with the C-bit, but the phys_to_dma call will "or" in the C-bit automatically. And since this is common code I need to leave that call to phys_to_dma in. > > The naming is a bit confusing, because physical addresses are actually > virtualized by SME -- maybe we should be calling them SME addresses? Interesting idea... I'll have to look at how that plays out in the patches and documentation. Thanks, Tom > -- 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