From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from NAM01-SN1-obe.outbound.protection.outlook.com (mail-sn1nam01on0058.outbound.protection.outlook.com [104.47.32.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A60F22095BB84 for ; Wed, 6 Sep 2017 08:36:50 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector1-amd-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version; bh=unFP8TAnMFVD2NGFf6HSGPy7Ik3Q2e4AMG5vMJnv7rk=; b=2pEKAdlye8TqpwurE3G3ewRR9bEGSvYY9w/BwutCrkxocRFiSMR801aaJOIsYZzGU+HTCfe9A9dwWousepCc0UrnqEKlRPYW3d89rq1rO4zwmbXxyDvOcYE58nY33+S/2TIrRcoY2/uhZ31HRbA1ayYWyWFOlty4prZ+BsmsLbA= Received: from [10.236.136.62] (165.204.77.1) by SN1PR12MB0160.namprd12.prod.outlook.com (10.162.3.147) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384_P256) id 15.20.35.12; Wed, 6 Sep 2017 15:39:38 +0000 Cc: brijesh.singh@amd.com, "Dong, Eric" To: Laszlo Ersek , "Yao, Jiewen" , "Zeng, Star" , edk2-devel-01 References: <20170903195449.30261-1-lersek@redhat.com> <0C09AFA07DD0434D9E2A0C6AEB0483103B93A125@shsmsx102.ccr.corp.intel.com> <4b24b1eb-362f-3b46-97e2-bdfda53f40c9@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A79BD@shsmsx102.ccr.corp.intel.com> <5f1fdc84-5824-bee2-5a1a-fbd67adf5443@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A7F10@shsmsx102.ccr.corp.intel.com> <12d71f32-9dcf-4f6e-b033-d5f82104caca@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C503A9A852C@shsmsx102.ccr.corp.intel.com> <24d66e81-336e-3924-8045-4749d98e2fbb@redhat.com> From: Brijesh Singh Message-ID: <94154cfe-92bf-ba6f-f25f-3963891f6932@amd.com> Date: Wed, 6 Sep 2017 10:39:35 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <24d66e81-336e-3924-8045-4749d98e2fbb@redhat.com> X-Originating-IP: [165.204.77.1] X-ClientProxiedBy: DM5PR13CA0065.namprd13.prod.outlook.com (10.175.103.155) To SN1PR12MB0160.namprd12.prod.outlook.com (10.162.3.147) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 8e48e18b-651f-4715-c830-08d4f53d7bd1 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(300000500095)(300135000095)(300000501095)(300135300095)(22001)(300000502095)(300135100095)(2017030254152)(48565401081)(300000503095)(300135400095)(2017052603199)(201703131423075)(201703031133081)(201702281549075)(300000504095)(300135200095)(300000505095)(300135600095)(300000506095)(300135500095); SRVR:SN1PR12MB0160; X-Microsoft-Exchange-Diagnostics: 1; SN1PR12MB0160; 3:qLrli0v7M1tjOKBDEVkTSh16hJM5Bc1cRX/i/dbQ/JKswRTPbR3lRYh9g8J14f0jDcz5QSNKnO7dPAJ6+rXyeFCgaOdDF0sLI8kn6V2xqLzWCtyRAekSQaIdpVrJ55ts3hZa5ua8hQj8H4bNDIEvYX3bCJ5sb1vLDJbab4xzKP3JyreOIyYU0WkyvFh5oIgo7WbuRtVIokQVAUX4IPCbWno1OiJ4FHSICfYugODFAs1MDYClX4Rt3J4231TyUyYH; 25:nnU+VgTyIwXK3nIunNefkxaV345+G8LtRRggG3v5EJu+O2EDu8oea4rSWrBMpgbmeGlDIsGtddJSp8vdbaW7c1U0QXhRuvyu+CBhG8dBMePRyfYjgzy0SFigNG6D1u4YS4UZ7R0dTvZtZm8xjkQ7JUdCh4iG0CJrGARjeMV/6JVSFVgDcxBaBWY+58j3JpsmfjQVg/aAKAVHFeiB5DanOe2Rf3MB6W2wcM1hos240WWvTtzlq5tdMg2FVrYghw4iTd7s+IzlK7VjYqM31wTlHC8UIezJ/mD2Nr+dYD9gX9hOoHRF/ksN7BoxVzZAKFmI0JGLe6kSeZiO9vVGmks1oQ==; 31:aAVLxtTn6FzvdDDAIX/x+6d4HgE/ffFC/1x9p3sw2MQOwuoYsnpJtpTHSoufIac2eK3bsW9N6X3C/OcRbLRsioW7f07p6J+U0LSIepElc0xHYcAUTxUeLKA/m9CVHB3nBZOwrY3w2lK02jPu35gfORlzOAzLRdyEtpXXpJ+iuYUGRfIKlJIfFVLJzhkMZVV7yioyodhu924BlcHxTcRe+NQ+jGp77CaxWcxh/rSKkOg= X-MS-TrafficTypeDiagnostic: SN1PR12MB0160: X-Microsoft-Exchange-Diagnostics: 1; SN1PR12MB0160; 20:TpAyfdbyAFUpqd5rSu5m01+K2dFXMZQiUnlM+s0fhbenyaPZ5578f4tcA2pk8kPQF6RbDMxEemYtD5ilB1B32xK7RM8cOmsyo5c/Dcn/F4GWKbB0jlO0dQjkhT0W5enj7AVLtnAe/rszjjfLpmSMzhcRvGhvHuYWWSWC+pOjykBdWLxLqivnzvkmhSGa3qA0ndQtdyKvfm8WTHjYe/bj0otcnv2EFDjpOHMDUs5i3BOBSr4KoxePw1nmUsKwc+pFT9M5piG4OrfJhTO2WtwjBx5dU6FsjYp3NSqq7laxYapWHWDtqjuoyWmYfcU7NxKk6Fvize2m+5DsCIB/HvEQcEahN66lV/y1M56rkzP6SxlU2Ya/0i6cIUSfOXV/jRoIfkRgo62jsNZdMULHMxVOOdP8u7wAziSRXl4QOLBP11VTJOQAatBegIVrL/gThdQrF9RP8jH50Qg/a6le6hbCe7GsmLt6qmtBcoFl3scliv0UVkz2SRH3iMItgKh3+pit X-Exchange-Antispam-Report-Test: UriScan:(192374486261705)(767451399110)(162533806227266)(155532106045638)(228905959029699)(17755550239193); X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(100000700101)(100105000095)(100000701101)(100105300095)(100000702101)(100105100095)(6040450)(2401047)(5005006)(8121501046)(3002001)(100000703101)(100105400095)(10201501046)(93006095)(93001095)(6055026)(6041248)(201703131423075)(201702281528075)(201703061421075)(201703061406153)(20161123562025)(20161123560025)(20161123564025)(20161123558100)(20161123555025)(6072148)(201708071742011)(100000704101)(100105200095)(100000705101)(100105500095); SRVR:SN1PR12MB0160; BCL:0; PCL:0; RULEID:(100000800101)(100110000095)(100000801101)(100110300095)(100000802101)(100110100095)(100000803101)(100110400095)(100000804101)(100110200095)(100000805101)(100110500095); SRVR:SN1PR12MB0160; X-Microsoft-Exchange-Diagnostics: 1; SN1PR12MB0160; 4:p6Gl+fIZimNUuqyGOwG4cEklS2u7+ypq4JReE/vggwWRf3dXTrnuE095OzERlt7Nv0YiyMnHKe14rTCPx2pWTofy6Im23+1YhIQvCHQMD4BZI4+IloiqybFJPDyorytddkQ/9xh7vmp6ahZy+FnhRTwHp4/reqkHs2L3kNmVRKaURhEccgMr6+D1WftmKFNozOBfiGCn6A3Ltb8ebA0OUqlG/4XeWK807hgGPFXCrkOX8H3gV2EfonS4U4JSCEhjNuUShEhQJHrHuHakhY3fvXgBYmTSLNFjF+OlU0VF7qCOJ6bdqoJb7mzcSdRDFYVazAPpdwX1TDw+nmywgRgCuXn5X9QRcOywDqvbaexmQWQ/y2P6MswZZPzlz8uoI/2apxBa9HtUTROgKNgIulqbajm6cy63KQVXflL9TIMqXzYRT6K/9idJd1EjkQZRnIqulxd4yc1fvFgcR1Nf9G4icQ== X-Forefront-PRVS: 0422860ED4 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(6049001)(6009001)(39860400002)(377454003)(24454002)(199003)(51914003)(189002)(76104003)(6246003)(31686004)(8936002)(4001350100001)(305945005)(5660300001)(6306002)(68736007)(189998001)(8676002)(83506001)(97736004)(2906002)(50466002)(81166006)(81156014)(64126003)(25786009)(31696002)(86362001)(478600001)(7736002)(65806001)(65956001)(47776003)(106356001)(66066001)(53546010)(65826007)(36756003)(54356999)(50986999)(105586002)(101416001)(42186005)(76176999)(966005)(4326008)(77096006)(6486002)(6116002)(230700001)(90366009)(3846002)(2950100002)(6666003)(93886005)(53936002)(45080400002)(229853002)(33646002)(23746002); DIR:OUT; SFP:1101; SCL:1; SRVR:SN1PR12MB0160; H:[10.236.136.62]; FPR:; SPF:None; PTR:InfoNoRecords; A:1; MX:1; LANG:en; Received-SPF: None (protection.outlook.com: amd.com does not designate permitted sender hosts) Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=brijesh.singh@amd.com; X-Microsoft-Exchange-Diagnostics: =?Windows-1252?Q?1; SN1PR12MB0160; 23:uO/5VHY7tQ5ZCTB1pPrVeX5vRWnOBxHrqAOF1?= =?Windows-1252?Q?2ho9VtIUGMZ+TsdTh6mMwcJxHTu+SQvLiqFyfQk39I/GFsYIcY3T5riu?= =?Windows-1252?Q?4HJJKeK0d3bqQTQVflcMx0TElJNDsw8g3o5U33kY2LIuXf3yUscZvXVB?= =?Windows-1252?Q?OTHyf9dCtq6d1ohAyAfE8qXDMyVRULP2lAHba5osbU2UpZNumDxOBdxG?= =?Windows-1252?Q?cx/R6yTWJkY7QhYcqyLjkbVpPjHOYTbfK6k8VaY0zLu8HF9dY5/3axrq?= =?Windows-1252?Q?NSSIhO65grIgOSPqMjYc/3brz93uj2kDNY7T9n2rgLMGtt0lnRMQKDk8?= =?Windows-1252?Q?8j7uUQaRoQOKAO0xNj94k4LyrXKT2B5Toq5Nhj85bcNtvIBmvO1YgUlM?= =?Windows-1252?Q?hAvFfNYDKHxTKKUpCW9RwGb4v+BKqoRtzNjTpE+aUiOqAUSBM99EI/Mj?= =?Windows-1252?Q?cd4KpPgxTtXjrmoy87ld7Dpi/KtefP7vPk2LdYGxGZV2mxhtX39zBykQ?= =?Windows-1252?Q?4Y7DVLYYCe+RU78y1j72lth4UW3ZUG9zanK7G0SDjjIeTgGs8dI2bpp3?= =?Windows-1252?Q?OAgEwsmIEGGyrdbN0BCl+dJzUqAk/4F89QN2CTAQ/+mHTKPqJ8hLlTW2?= =?Windows-1252?Q?k/pVm9pFxQ7T5rU+t0IePcunQeWXMuJIMWgIwWNUw2SVoNTmI2SM1leO?= =?Windows-1252?Q?LaxCeqWcfaj7oNHl7/PHvogNQPB7w860ugsfteGEFU6XLEY64OAuV++W?= =?Windows-1252?Q?VwjQ7X0v2BmlTWgUG01dnMG6KmI2xZi3Uk1Bd6RYUSe7MeN56glt928b?= =?Windows-1252?Q?LDu6FwFn2NywnITUksEmfGYLhLdhqOMpO8RD9jM/jElzINvi7TV3zOyd?= =?Windows-1252?Q?xlvHVpA3RebCHtdlGWihRwulvLpT9N5kzwxOamNkCx9kyWvEejp4Xwa1?= =?Windows-1252?Q?SOeTCUXat/+h5qSMTCBpm2lblWtY8yopX4QzLfObrXA0jQyOittLWCrA?= =?Windows-1252?Q?EGKXF20dUCvPWw7le8Z5s79MksqVF/uXjLDD6pBsaLbEnE3RdcpXpfh+?= =?Windows-1252?Q?GsaYOGeF+TD64uBKmNohzWeZSX+HgEUOr1+0Sc8XDQ9Uuq4CTMjQR2X3?= =?Windows-1252?Q?vL3iisMs0bUqHtRxulQhHfMZptljEAsd7whV2Xm3d5it35KvGteYUu21?= =?Windows-1252?Q?RlrEOuQd2YXGDEokxzd4UtbseNuWOj2hbWWCDaPmWD73no5i4G+CuoEl?= =?Windows-1252?Q?IRkBDEqxIVlsH03huzwaNn/3CS5xIyJdY4vCD+Se8H2GpdubxYpvL6z5?= =?Windows-1252?Q?Wq9YYQ/yvYp0ZsOE0mWhFN9knX5rzTYCFiA8WfBFs1penCfYuDN6avWt?= =?Windows-1252?Q?85axAgpTVWEtujuEClqtkHvXVZMD2N1Cz9mAKnqG0woXinz55LX+E5qA?= =?Windows-1252?Q?Ctgb5O9D6xZ4OUhXTlHAkeDGCF5fGia0686X8JEZcL7SkiYNrMWzMZm3?= =?Windows-1252?Q?1rMWjO3EB2Neu88tcbfnZ+T0yvZm20KglSHnxg5ir0aZ+6u6+KKpIIbL?= =?Windows-1252?Q?fDfoigid2J/w28=3D?= X-Microsoft-Exchange-Diagnostics: 1; SN1PR12MB0160; 6:F1iPAwIsWboDsnDRL9goQDymrh/KXk4sIajnDzVUPBK1HlZCIwPlMgPhouhmXOqexcADb8ecrnSMlwwWgyl1as2f3br0p4d2lAhHJsHixFRcERCnKoBNBGHTmZe+/YwOpnppCHoX8bJB58n+MkxzlgCsjomYj/Xci2mKguk5rZzQLUhMhy+FaxnHM8cxO+/YpZ8QPIgI+EuwN7+aZPVF0nbbxe4L0Ht8H4/0lM3cEoOiW4oCoZeYzuuz4PIpFwuKi1Tp0Ep29rmoHFrlf8cm5X9gTKX8eq8QcIQ4eSKezvrcuYErbUXE4MAGKcwwDE7ai9AEJBzegbi+fxWvZ0HwDw==; 5:n13kWhtpQMxDIDaRc6CE5OM3l2w9Bu1DDkmT4xySO5aYVwGEqzZex4TBeufqabGhMfG1wTq3pmehFgfRtAki5p1XDLE/rHt2BHRPpU09+8cvWQ2C0AvG201Kb2k+41AckHDYJTCJquWMcxwp4buRBg==; 24:HgOfE3rb1TDsqB/tCzq1kkUF1SzhHtWZ6cMBtbenhC8DA0uFMsA+IGpaWhqSCzEl337FMvbv7upSw/sE7PImMI6upcnc5ReQg3aU5RZfoSI=; 7:07gJujwaLq2f7KumfsgrYi7xY+/nlznAguI5430FlIG5Ys2C2bj9fkKYSmxka6i2KqK8GHrM0O2jG1PeOyT+Xn1UQwqjG8+qIaBnC7EnXSVUPx8dYV8zkQpKMe0eC7gVODnQ5U9FLKR7nnt+JIrN4hAPYnQOnt/TGZ5u9XZumbFsSir2WKlJ7NEZk+0L2wtK7Bm0/fRH1ABVrteDIg87XZy9LxSJOTtXLC5pKbVkvOQ= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; SN1PR12MB0160; 20:mxxnuhw3nsiMkzSkBjuRqF9oLMM3WBLOx7GpWzTf52wLbEiFNZLB7L4/uH5Zh/gB6nIA5nLt9jbD00rGRSGU6kCrdUKeZbw7DAMbV0mIDxdDlW1WtRqy5HsZoK6nJ9mgXo+tdhp5/gn6ohveFkTOAuOrntzXNYrWOZaWOC6uHzUAnJBWFRCS488PjSE0UFIHUFrYF09KlBAf6akZ4cBKXgaEwTg8m8hJTyJumGnERl8CP8g7NQxlZNSYiuYGOU4R X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Sep 2017 15:39:38.1644 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR12MB0160 Subject: Re: [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 06 Sep 2017 15:36:51 -0000 Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 09/06/2017 07:14 AM, Laszlo Ersek wrote: > On 09/06/17 06:37, Yao, Jiewen wrote: >> Thanks for the clarification. Comment in line. >> >> From: Laszlo Ersek [mailto:lersek@redhat.com] >> Sent: Wednesday, September 6, 2017 1:57 AM >> To: Yao, Jiewen ; Zeng, Star ; edk2-devel-01 >> Cc: Dong, Eric ; Brijesh Singh >> Subject: Re: [edk2] [PATCH 0/4] MdeModulePkg: some PCI HC drivers: unmap common buffers at ExitBootServices() > >>> Then after ExitBootService, the OS will take control of CR3 and set correct >>> encryption bit. >> >> This is true, the guest OS will set up new page tables, and in those >> PTEs, the C-bit ("encrypt") will likely be set by default. >> >> However, the guest OS will not *rewrite* all of the memory, with the >> C-bit set. This means that any memory that the firmware didn't actually >> re-encrypt (in the IOMMU driver) will still expose stale data to the >> hypervisor. >> [Jiewen] That is an interesting question. >> Is there any security concern associated? > > I can't tell for sure. Answering this question is up to the proponents > of SEV, who have designed the threat model for SEV. > > My operating assumption is that we should keep memory as tightly > encrypted as possible at the firmware --> OS control transfer. It could > be an exaggerated expectation from my side; I'd just better be safe than > sorry :) > > Let me give some brief intro on SEV (Secure Encrypted Virtualization) and then we can discuss a security model (if needed). SEV is an extension to the AMD-V architecture which supports running encrypted virtual machines (VMs) under the control of a hypervisor. Encrypted VMs have their pages (code and data) secured such that only the guest itself has access to the unencrypted version. Each encrypted VMs is associated with a unique encryption key; if its data is accessed by a different entity using a different key the encrypted guest data will be incorrectly decrypted, leading to unintelligible data. You can also find some detail for SEV in white paper [1]. SEV encrypted Vs have the concept of private and shared memory. The private memory is encrypted with the guest-specific key, while shared memory may be encrypted with hypervisor key. SEV guest VMs can choose which pages they would like to be private. But the instruction pages and guest page tables are always treated as private by the hardware. The DMA operation inside the guest must be performed on shared pages -- this is mainly because in virtualization world the device DMA needs some assistance from the hypervisor. The security model provided by the SEV ensure that hypervisor will no longer able to inspect or alter any guest code or data. The guest OS controls what it want to share with outside world (in this case with Hypervisor). In software implementation we took approach to encrypt everything possible starting early in boot. In this approach whenever OVMF wants to perform certain DMA operations it allocate a shared page, issues the command, free the shared page after the command is completed (in other word we use sw bounce buffer to complete the DMA operation). We have implemented IOMMU driver to provide the following functions: AllocateBuffer(): -------------------- it allocate a private pages, as per UEFI spec the driver will map the buffer allocated from this routine using BusMasterCommonBuffer hence we allocate extra stash pages in addition to requested pages. Map ---- If requested operation is BusMasterRead or BusMasterWrite then we allocate a shared buffer and DeviceAddress point to shared buffer. If requested operation is BusMasterCommonBuffer then we perform in-place decryption of the contents and update the page-table to clear the C-bit (basically make it shared page) Unmap ------ If operation was BusMasterRead or BusMasterWrite then we complete the unmapping and free the shared buffer allocated in Map(). If operation was BusMasterCommonBuffer then we perform in-place encryption and set the C-bit (basically make the page private) FreeBuffer: ----------- Free the pages Lets run with the below scenario: 1) guest marks a page as "shared" and gives its physical address to HV (e.g DMA read) 2) hypervisor completes the request operation 3) hypervisor caches the shared physical address and monitor it for information leak 4) sometime later if guest write data in its "shared" physical address then hypervisor can easily read it without guest knowledge. SEV hardware can protect us against the attack where someone tries to inject or alter the guest code. As I noted above any instruction fetch is forced as private hence if attacker write some code into a shared buffer and point the RIP to his/her code then instruction fetch will try to decrypt it and get unintelligible opcode. If a page was marked as "shared" then its guest responsibility to make it "private" after its done communicating with hypervisor. [1] http://amd-dev.wpengine.netdna-cdn.com/wordpress/media/2013/12/AMD_Memory_Encryption_Whitepaper_v7-Public.pdf >> If the C-bit is cleared for a read/write buffer, is the content in the >> read/write buffer also exposed to hypervisor? > > Not immediately. Only when the guest also rewrites the memory through > the appropriate virtual address. > > Basically, in the virtual machine, the C-bit in a PTE that covers a > particular guest virtual address (GVA) controls whether a guest write > through that GVA will result in memory encryption, and if a gues read > through that GVA will result in memory decryption. > > The current state of the C-bit doesn't matter for the hypervisor, what > matters is the last guest write through a GVA whose PTE had the C-bit > set, or clear. If the C-bit was clear when the guest last wrote the > area, then the hypervisor can read the data. If the C-bit was set, then > the hypervisor can only read garbage. > > >> I means if there is security concern, the concern should be applied to >> both common buffer and read/write buffer. >> Then we have to figure a way to resolve both buffer. > > Yes, this is correct. The PciIo.Unmap operation, as currently > implemented in OvmfPkg/IoMmuDxe/, handles the encryption/decryption > correctly for all operations, but it can only guarantee *not* freeing > memory for CommonBuffer. So Unmap()ing CommonBuffer at ExitBootServices > is safe, while Unmap()ing Read/Write is not. The encryption would be > re-established just fine for Read/Write as well, but we would change the > UEFI memmap. > > In OVMF, we currently manage this problem by making all asynchronous DMA > mappings CommonBuffer, even if they could othewise be Read or Write. We > use Read and Write only if the DMA operation completes before the > higher-level protocol function returns (so we can immediately Unmap the > operation, and the ExitBootServices() handler doesn't have to care). > > >> To be honest, that is exactly my biggest question on this patch: >> why do we only handle the common buffer specially? > > For the following reason: > > - Given that CommonBuffer mappings take a separate AllocateBuffer() / > FreeBuffer() call-pair, around Map/Unmap, we can *reasonably ask* PciIo > implementors -- beyond what the UEFI spec requries -- to keep *all* > dynamic memory management out of Map/Unmap. If they need dynamic memory > management, we ask them to do it in AllocateBuffer/FreeBuffer instead. > This way Unmap is safe in ExitBootServices handlers. > > - We cannot *reasonably ask* PciIo implementors to provide the same > guarantee for Read and Write mappings, simply because there are no other > APIs that could manage the dynamic memory for Map and Unmap -- > AllocateBuffer and FreeBuffer are not required for Read and Write > mappings. PciIo implementors couldn't agree to our request even if they > wanted to. Therefore Unmapping Read/Write operations is inherently > unsafe in EBS handlers. > > >>> NOTE: The device should still be halt state, because the device is >>> also controlled by OS. >> >> This is the key point -- the device is *not* controlled by the guest OS, >> in the worst case. >> >> The virtual device is ultimately implemented by the hypervisor. We don't >> expect the virtual device (= the hypervisor) to mis-behave on purpose. >> However, if the hypervisor is compromised by an external attacker (for >> example over the network, or via privilege escalation from another >> guest), then all guest RAM that has not been encrypted up to that point >> becomes visible to the attacker. In other words, the hypervisor ("the >> device") becomes retro-actively malicious. >> [Jiewen] If that is the threat model, I have a question on the exposure: >> 1) If the concern is for existing data, it means all DMA data already written. >> However, the DMA data is already consumed or produced by virtual device >> or a hypervisor. If the hypervisor is attacked, it already gets all the data content. > > Maybe, maybe not. The data may have been sent to a different host over > the network, and wiped from memory. > > Or, the data may have been written to a disk image that is separately > encrypted by the host OS, and has been detached (unplugged) from the > guest, and also unmounted from the host, with the disk key securely > wiped from host memory. > > We can also imagine the reverse direction. Assume that the user of the > virtual machine goes into the UEFI shell in OVMF, starts the EDIT > program, and types some secret information into a text file on the ESP. > Further assume that the disk image is encrypted on the host OS. It is > conceivable that fragments of the secret could remain stuck in the AHCI > (disk) and USB (keyboard) DMA buffers. > > Maybe you think that these are "made up" examples, and I agree, I just > made them up. The point is, it is not my place to discount possible > attack vectors (I haven't designed SEV). Attackers will be limited by > their *own* imaginations only, not by mine :) > > >> 2) if the concern is for future data, it means all user data to be written. >> However, the C-bit already prevent that. > > As far as I understand SEV, future data is out of scope. The goal is to > prevent *retroactive* guest data leaks (= whatever is currently in host > OS memory) if an attacker compromises an otherwise non-malicious hypervisor. > > If an attacker compromises a virtualization host, then they can just sit > silent and watch data flow into and out of guests from that point > onward, because they can then monitor all DMA (which always happens in > clear-text) real-time. > > >> Maybe I do not fully understand the threat model defined. >> If you can explain more, that would be great. > > Well I tried to explain *my* understanding of SEV :) I hope Brijesh will > correct me if I'm wrong. > > >> The point of SEV is to keep as much guest data encrypted at all times as >> possible, so that *whenever* the hypervisor is compromised by an >> attacker, the guest memory -- which the attacker can inevitably dump >> from the host side -- remains un-intellegible to the attacker. >> [Jiewen] OK. If this is a security question, I suggest we define a clear >> threat model at first. >> Or what we are doing might be unnecessary or insufficient. > > I agree. > > As I said above, my operating principle has been to re-encrypt all DMA > buffers as soon as possible. For long-standing DMA buffers, re-encrypt > them at ExitBootServices at the latest. > > >> [Jiewen] For "require that Unmap() work for CommonBuffer >> operations without releasing memory", I would hold my opinion until >> it is documented clearly in UEFI spec. > > You are right, of course. > > It's just that we cannot avoid exploring ways, for this feature, that > currently fall outside of the spec. Whatever we do here will be outside > of the spec, one way or another. I suggested what I thought could be a > reasonable extension to the spec, one that could be implemented by PciIo > implementors even before the spec is modified. > > edk2's PciIo.Unmap already behaves like this, without breaking the spec. > > If there's a better way -- for example modifying CoreExitBootServices() > in edk2, to signal IOMMU drivers separately, *after* notifying other > ExitBootServices() handlers --, that might work as well. > > >> My current concern is: >> First, this sentence is NOT defined in UEFI specification. > > Correct. > > >> Second, it might break an existing implementation or existing feature, such as tracing. > > Correct. > >> Till now, I did not see any restriction in UEFI spec say: In this function, you are not allowed >> to call memory services. >> The only restriction is >> 1) TPL restriction, where memory service must be <= TPL_NOTIFY. >> 2) AP restriction, where no UEFI service/protocol is allowed for AP. > > I agree. > > >> I'm just trying to operate with the APIs currently defined by the UEFI >> spec, and these assumptions were the best I could come up with. >> [Jiewen] The PCI device driver must follow and *only* follow UEFI spec. >> Especially the IHV Option ROM should not consume any private API. > > I disagree about "only follow". If there are additional requirements > that can be satisfied without breaking the spec, driver implementors can > choose to conform to both sets of requirements. > > For example (if I understand correctly), Microsoft / Windows present a > bunch of requirements *beyond* the UEFI spec, for both platform and > add-on card firmware. Most vendors conform :) > > >>> [Jiewen] I am not sure who will control "When control is transferred to the OS, all >>> guest memory should be encrypted again, even those areas that were once >>> used as CommonBuffers." >>> For SEV case, I think it should be controlled by OS, because it is just CR3. >> >> If it was indeed just CR3, then I would fully agree with you. >> >> But -- to my understanding --, ensuring that the memory is actually >> encrypted requires that the guest *write* to the memory through a >> virtual address whose PTE has the C-bit set. >> >> And I don't think the guest OS can be expected to rewrite all of its >> memory at launch. :( >> >> [Jiewen] That is fine. >> I suggest we get clear on the threat model as the first step. >> The threat model for SEV usage in OVMF. >> >> I am sorry if that is already discussed before. I might ignore the conversation. > > No problem; it's always good to re-investigate our assumptions and > operating principles. > > >> If you or any SEV owner can share the insight, that will be great. >> See my question above "If that is the threat model, I have a question on the exposure:..." > > I shared *my* impressions of the threat model (which are somewhat > unclear, admittedly, and thus could make me overly cautious). > > I hope Brijesh can extend and/or correct my description. > > >>> So apparently the default behaviors of the VT-d IOMMU and the SEV IOMMU >>> are opposites -- VT-d permits all DMA unless configured otherwise, while >>> SEV forbids all DMA unless configured otherwise. >>> [Jiewen] I do not think so. The default behaviors of VT-d IOMMU is to disable all DMA access. >>> I setup translation table, but mark all entry to be NOT-PRESENT. >>> I grant DMA access only if there is a specific request by a device. >>> >>> I open DMA access in ExitBootServices, just want to make sure it is compatible to >>> the existing OS, which might not have knowledge on IOMMU. >>> I will provide a patch to make it a policy very soon. As such VTd IOMMU may >>> turn off IOMMU, or keep it enabled at ExitBootService. >>> An IOMMU aware OS may take over IOMMU directly and reprogram it. >> >> Thanks for the clarification. >> >> But, again, will this not lead to the possibility that the VT-d IOMMU's >> ExitBootServices() handler disables all DMA *before* the PCI drivers get >> a chance to run their own ExitBootServices() handlers, disabling the >> individual devices? >> [Jiewen] Sorry for the confusing. Let me explain: >> No, VTd never disables *all* DMA buffer at ExitBootService. >> >> "disable VTd" means to "disable IOMMU protection" and "allow all DMA". >> "Keep VTd enabled" means to "keep IOMMU protection enabled" and >> "only allow the DMA from Map() request". > > Okay, but this interpretation was exactly what I thought of first (see > above): "VT-d permits all DMA unless configured otherwise". You answered > that it wasn't the case. > > So basically, if I understand it correctly now, at ExitBootServices the > VT-d IOMMU driver opens up all RAM for DMA access. Is that correct? > > That is equivalent to decrypting all memory under SEV, and is the exact > opposite of what we want. (As I understand it.) > > >> If that happens, then a series of IOMMU faults could be generated, which >> I described above. (I.e., such IOMMU faults could result, at least in >> the case of SEV, in garbage being written to disk, via queued commands.) >> [Jiewen] You are right. That would not happen. >> IOMMU driver should not bring any compatibility problem for the PCI driver, >> iff the PCI driver follows the UEFI specification. > > Agreed. > > >> Now, to finish up, here's an idea I just had. >> >> Are we allowed to call gBS->SignalEvent() in an ExitBootServices() >> notification function? >> >> If we are, then we could do the following: >> >> * PciIo.Unmap() and friends would work as always (no restrictions on >> dynamic memory allocation or freeing, for any kind of DMA operation). >> >> * PCI drivers would not be expected to call PciIo.Unmap() in their >> ExitBootServices() handlers. >> >> * The IOMMU driver would have an ExitBootServices() notification >> function, to be enqueued at the TPL_CALLBACK or TPL_NOTIFY level >> (doesn't matter which). >> >> * In this notification function, the IOMMU driver would signal *another* >> event (a private one). The notification function for this event would >> be enqueued strictly at the TPL_CALLBACK level. >> >> * The notification function for the second event (private to the IOMMU) >> would iterate over all existent mappings, and unmap them, without >> allocating or freeing memory. >> >> The key point is that by signaling the second event, the IOMMU driver >> could order its own cleanup code after all other ExitBootServices() >> callbacks. (Assuming, at least, that no other ExitBootServices() >> callback employs the same trick to defer itself!) Therefore by the time >> the second callback runs, all PCI devices have been halted, and it is >> safe to tear down the translations. >> >> The ordering would be ensured by the following: >> >> - The UEFI spec says under CreateEventEx(), "All events are guaranteed >> to be signaled before the first notification action is taken." This >> means that, by the time the IOMMU's ExitBootServices() handler is >> entered, all other ExitBootServices() handlers have been *queued* at >> least, at TPL_CALLBACK or TPL_NOTIFY. >> >> - Therefore, when we signal our second callback from there, for >> TPL_CALLBACK, the second callback will be queued at the end of the >> TPL_CALLBACK queue. >> >> - CreateEventEx() also says that EFI_EVENT_GROUP_EXIT_BOOT_SERVICES "is >> functionally equivalent to the EVT_SIGNAL_EXIT_BOOT_SERVICES flag >> for the Type argument of CreateEvent." So it wouldn't matter if a >> driver used CreateEvent() or CreateEventEx() for setting up its own >> handler, the handler would be queued just the same. >> >> I think this is ugly and brittle, but perhaps the only way to clean up >> *all* translations safely, with regard to PciIo.Unmap() + >> ExitBootServices(), without updating the UEFI spec. >> >> What do you think? >> >> [Jiewen] If the order is correct, and all PCI device driver is halt at ExitBootServices, that works. >> We do not need update PCI driver and we do not need update UEFI spec. >> We only need update IOMMU driver which is concerned, based upon the threat model. >> That probably is best solution. :-) > > I'm very glad to hear that you like the idea. > > However, it depends on whether we are permitted, by the UEFI spec, to > signal another event in an ExitBootServices() notification function. > > Are we permitted to do that? > > Does the UEFI spec guarantee that the notification function for the > *second* event will be queued just like it would be under "normal" > circumstances? > > (I know we must not allocate or free memory in the notification function > of the *second* event either; I just want to know if the second event's > handler is *queued* like it would normally be.) > > >> I assume you want to handle both common buffer and read/write buffer, right? > > Yes. Under this idea, all kinds of operations would be cleaned up. > > >> And if possible, I still have interest to get clear on the threat model for SEV in OVMF. >> If you or any SEV owner can share ... > > Absolutely. Please see above. > > Thank you! > Laszlo >