From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (NAM04-BN3-obe.outbound.protection.outlook.com [40.107.68.49]) by mx.groups.io with SMTP id smtpd.web12.399.1619106153885372851 for ; Thu, 22 Apr 2021 08:42:34 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=O4zuApcm; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: amd.com, ip: 40.107.68.49, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=FoEvi5I3lbd0XR2B0zI/wIsFZYx/qG70XGJdwS+c9pKn+zMOXFBKXsStJptdqVhkY9wZnvP7c/WLfykuHyn86XKy4bxbDQ7c2SHv4hZYYNWkzA9bFi8H9j0V5N8CuKAu/yvfPoM7bIGOxZwb5YS07+vcFknH1N0i9pFzBL0gp8plRKbPSrftrk1B8i+fr75lmNBTFRZIqT0EnceubzKOcB1HwdAhqV8WL3ZFGHQKfP8N+ruDoMVyF+lnguHvHl3r2gC/+ALdsgi/0+PfN6422TS0jpyzNTbBfc96bUSqJdpC5gVuLRSsgZKbp/JnfG14QUhar91LJFhhoNCgbYbktw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+KTtl8y448EFUThjYPmTFNN1e79Xxxwun5qs/tnAWss=; b=aksgTGX2MQPnycx24y97EhUy0mj6YRbDyBT7UxIBxipxc+DfwOXNNnY7zZ2U/tH8ZDDYWozpmMdaKuRqItYYwNTEPgM/wbJbz75jbNneMPk8trIl94E4umRSPfMsALyLhufPGFYFXvMGg/Hjepb3fGMK7c6drEOzCBeXJiTD43tbBb9dZrH8q1dPmzEKI04KSmeVYUw8PyOC9lzr6ldgavWEPPAZYklztMYENxySFLwwa8O7YXDweieczFZZdFhqThhu9C3LbgBIc0Ruj/xbGN6+adymc3Dk5U5zbjxGWOMGht0CDNYjowdm7HdqY76uBK3GD8GM/MXm2jUWBTbxbg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=+KTtl8y448EFUThjYPmTFNN1e79Xxxwun5qs/tnAWss=; b=O4zuApcm0pd50BhyvhiVODxZuhYjNZkYn730a3N1cO38Dbd/WIgAfSMmk8HweTrRODIIn2saxeSRtIKq+kG+ShadyPqXlmInZgzfx0uD1mubzJD96WtfNDdUNkWrszgIGakKEQk6zdIi7rYwAKouv8QpSd+PYx3SLmt/dOqlWYM= Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB3964.namprd12.prod.outlook.com (2603:10b6:5:1c5::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.22; Thu, 22 Apr 2021 15:42:30 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9%12]) with mapi id 15.20.4065.023; Thu, 22 Apr 2021 15:42:30 +0000 Subject: Re: [edk2-devel] [PATCH 2/3] OvmfPkg/VmgExitLib: Add support for new MMIO MOV opcodes From: "Lendacky, Thomas" To: devel@edk2.groups.io, lersek@redhat.com Cc: Joerg Roedel , Borislav Petkov , Ard Biesheuvel , Jordan Justen , Brijesh Singh , James Bottomley , Jiewen Yao , Min Xu References: <79ed645c089ffab10716cdb8813f191f6e0afcfb.1618959281.git.thomas.lendacky@amd.com> <5d4e5bdd-65ea-7594-3b51-e33284fe990f@amd.com> Message-ID: Date: Thu, 22 Apr 2021 10:42:27 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: <5d4e5bdd-65ea-7594-3b51-e33284fe990f@amd.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN6PR08CA0014.namprd08.prod.outlook.com (2603:10b6:805:66::27) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN6PR08CA0014.namprd08.prod.outlook.com (2603:10b6:805:66::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.21 via Frontend Transport; Thu, 22 Apr 2021 15:42:29 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 54b1faf1-8748-42d6-39e5-08d905a53d0f X-MS-TrafficTypeDiagnostic: DM6PR12MB3964: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: rzEOEUcAn1NmFoy/+tq5GnzpY1pwWU56sQonuCvo2hJ3ASHZxCUJeRVCLCy7p/UBQsR7z2Gf07yzhagOaCxqFmYBhEHTCMxOhCnckfDiD4xI/KNLR2cQnjcuS7rCUNOvGyCgTXveqycUjpwm7KloKhGFtqH4fn3lkPcNrzHUKb9e+498DwiEuJ4NfCi76n+ewkVcWD9X0ler/z6zQTQ2jIZpOukNTwX2Os4NMtiu+pnsLeUHirJXFI1ZTWuPplJ++FBKmJLj+6a60B1tb1V4zbyboAaHzWFwmi33FhQUbwazVqBime754WmKb1R8t/Ujzb9tc8WpE/68ylfe0Ywxp47lMj1RmPWKI3FD/C3hGPUPq/vi1BR297C3W80b8mp9pF7959NX3W6J0W2X2Ks6cagSQFSieWR+xxIKKzblBabOyRCGqGG6nQa601w+sd9Z/BvY7RPAnFuNuENzjasA94kj4T6oIypzVuLsjlCviBcELPb9lngiXdM3vdmEFK87sNZNU0DGsubTZaR5Uq7v0hj3hlf7Sv14sL/U/6QpPaiczbONNH0cJbzqedS3Bn8aM7hqifSyrU5p55amewPcmhGad8t1H2sFCRTBSOQv/ohy0tJkEdrvtpZ+vjecoim97fxlCbr7vhm2AQ9oGhqqHbM+0SeeZV/RzXz9r95DbuINm8mnWB/stv4LEnh+z8rDhC8eZkBwnaLhEEpqjsOjMLw/tNzZCWc6Ho7Hx5ctCSsG4ywj5imM05+VFPohXZHaV/HrxvtxO3HL1dLfcFkIvw== X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(39860400002)(346002)(376002)(136003)(366004)(396003)(38100700002)(966005)(45080400002)(66946007)(8676002)(186003)(36756003)(4326008)(83380400001)(6486002)(66556008)(66476007)(6512007)(5660300002)(26005)(16526019)(478600001)(54906003)(31696002)(2906002)(316002)(6506007)(2616005)(956004)(8936002)(31686004)(86362001)(53546011)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?dHJDVkwxMFlnbGVUUFVGZTdLTnZGNXlYY1YwdjFta1lUSldMQmRTWmNHK2xa?= =?utf-8?B?MDhSRTV3VGM1NkM1alorSklUYm9aS3c5YUZneUg3a1MrUE93L1M3b2NJV2oz?= =?utf-8?B?bjdyZmovdWlGVjRpdzIyajdGaDQ1enU5SWhENkNEZG1sYm1kUmEwaFFqWVcz?= =?utf-8?B?NEg5T25KeXBWZ3FHT0Fod3ZhdS9IYWlHRmFtZERTL3dVbkszUnZWMlROK2hs?= =?utf-8?B?d0daQ2JKRUZFdURTckVtRmlMdjJDREtUc3BqNUs2b2xIMWRSUUdnRmh2aUJ1?= =?utf-8?B?cUNRZ1A5b3NCZk9KMFhlVy9zajN5ai9Sd2ErenB5MC9teUF0QjZyZGJIOEUw?= =?utf-8?B?Q2J1WkVzU2pOMUdRK01MUTl2OXZNQ3VwOVRlQnBid0FMMjBPbWc5WFZmdW1i?= =?utf-8?B?TG0vM3Uyd1pWa0hUSmYyM2xKNUlTR3Yxd2loanVkTWlSc0ZHQTlweXcydEFI?= =?utf-8?B?M1JMdGlhVVlLYlh6bDg4ZG5BYldqSG9nRGVVd1VHVDd3MnlRakZHVDJCTnVj?= =?utf-8?B?VUJBZGpuRmZiUjlRTzRnQ1Q1akdsTEp4TWs5Vk5YQkdiOVVkb3NSbit0bWJx?= =?utf-8?B?eFJyVXExdkVDdUFSdmpSN3FkNVhiSXhJSURrc2hYenpzaWk0V3ZQOTU4UXZJ?= =?utf-8?B?U2Zpd1JBL2I4UVVTcDJaYjM4Q2xvdGwrem0vVFUzYS94ejQ1b203YWFUbklY?= =?utf-8?B?OTBncHhadWlqNWRaQ0xtS3hzWXNsbmNKMWhnQjV2R2RzUWREcG96T1NQVE5o?= =?utf-8?B?Q09NMEh2NU1tQjlubGN1ZWJaTW0rekQ4SGlGd0Z6eVBTRTZpc1RqencvOThB?= =?utf-8?B?MEx1Z3dyRVFLUEdsVjI1dGhIMFdRaG15d3hsUXRRaTVFYWtpUFNaTWlleUlC?= =?utf-8?B?cC90VGluWHJ2NHUvL0hROEpjaXVZcFZ1Y2l0eUQwZUNod3B2bmVrVm1yNnYv?= =?utf-8?B?TDA5UDdZOWE5cE84Vmg2UVdobkZnV2poUGxsam93dnJwRHpmaUdwb0QzSmt0?= =?utf-8?B?dmRXbS80WFVNaWJDSXh3VFFTMGZTUTJSUWdCeUh2TDh6WjlnRExRUlFscGZi?= =?utf-8?B?NGsyRGVVUmZQNGREUVBWTEVHMzEySHpnZSs0dEdBcmt2Ym93UWZybkUwZEho?= =?utf-8?B?L1IrcmhKbzVhb3Q5M1J2akRRcHphSmdBdVpzVkJDTjZoL0haaTlueWVFcmM3?= =?utf-8?B?bnU2V3VOeWh3NDNPdE1qT0dTT3RidGkvMzlKUXZCa0NUQUhqakNLMVFoL1Mv?= =?utf-8?B?V1pPaVRPYVNxN3lVOTM4bWdhR3p3WlY3ckpPaDhhTUcrVkRMTHhHQnU5RkNq?= =?utf-8?B?WGVCdmRrY0NCZnZWTldacGNicUVQYnZqSkppdzF2c28xNXVxcGFsSk10WFlm?= =?utf-8?B?MmM4QkNkVDhjZ1djU2RwZTlRMTIzUWlFbjJTbG92Y0xqejdudVpLcmpyRjVn?= =?utf-8?B?OUVObW9aMzU3Qk9SRWFVVHRFVGN1SGJRNUo0MjEyc2UzVUlLbkppVktIWjNl?= =?utf-8?B?a1ZOYVlJZ3FqbnQrVGNqQ0RZN3dYVVI4ZVpJUDZLZElUYU9wTUtSczZuam9a?= =?utf-8?B?VlVUb2xCdGFjUHhzRXB6ZGNIa2JXeStRWFNuOXFGclJoOTdIRjRwRkgvcjN0?= =?utf-8?B?V09kYStORXh2MEZOTGJIQ3VMa0pPYkJxTVFpS1hiejM3a0l3MkRxbzlTZTNK?= =?utf-8?B?cy8yNG1kUGJsLzFoRUkxSldvc1FXeW1zZ1hwL2VLd1pZQ05qNWhVYjhyV2U3?= =?utf-8?Q?UFcrPQ/25Ty7nyJxon7tq2x1hvW9UD6a6qzk6Cv?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 54b1faf1-8748-42d6-39e5-08d905a53d0f X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2021 15:42:30.3315 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: dYilreOLmC3gnRgW7wzztvTlVUXs97GmwdQW0/Sh15FMkqwdDOq/hUx6pMon7LxafRETKfOfyiPvtHoDcca8Dg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3964 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 4/22/21 9:15 AM, Tom Lendacky wrote: > On 4/22/21 12:50 AM, Laszlo Ersek via groups.io wrote: >> On 04/21/21 00:54, Lendacky, Thomas wrote: >>> From: Tom Lendacky >>> >>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&data=04%7C01%7Cthomas.lendacky%40amd.com%7C19a7d97e2a7b461830ed08d905528472%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637546674232278910%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=znSezOvpnItW7mHAJkr%2FtJtkQNFc2H0dG9STpmOpVqU%3D&reserved=0 >>> >>> Enabling TPM support results in guest termination of an SEV-ES guest >>> because it uses MMIO opcodes that are not currently supported. >>> >>> Add support for the new MMIO opcodes (0xA0 - 0xA3), MOV instructions which >>> use a memory offset directly encoded in the instruction. Also, add a DEBUG >>> statement to identify an unsupported MMIO opcode being used. >>> >>> Fixes: c45f678a1ea2080344e125dc55b14e4b9f98483d >>> Cc: Laszlo Ersek >>> Cc: Ard Biesheuvel >>> Cc: Jordan Justen >>> Cc: Brijesh Singh >>> Cc: James Bottomley >>> Cc: Jiewen Yao >>> Cc: Min Xu >>> Signed-off-by: Tom Lendacky >>> --- >>> OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 99 +++++++++++++++++++ >>> 1 file changed, 99 insertions(+) >>> >>> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >>> index 273f36499988..f9660b757d8e 100644 >>> --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >>> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >>> @@ -678,6 +678,7 @@ MmioExit ( >>> UINTN Bytes; >>> UINT64 *Register; >>> UINT8 OpCode, SignByte; >>> + UINTN Address; >>> >>> Bytes = 0; >>> >>> @@ -727,6 +728,51 @@ MmioExit ( >>> } >>> break; >>> >>> + // >>> + // MMIO write (MOV moffsetX, aX) >>> + // >>> + case 0xA2: >>> + Bytes = 1; >>> + // >>> + // fall through >>> + // >>> + case 0xA3: >>> + Bytes = ((Bytes != 0) ? Bytes : >>> + (InstructionData->DataSize == Size16Bits) ? 2 : >>> + (InstructionData->DataSize == Size32Bits) ? 4 : >>> + (InstructionData->DataSize == Size64Bits) ? 8 : >>> + 0); >>> + >>> + InstructionData->ImmediateSize = (UINTN) (1 << InstructionData->AddrSize); >>> + InstructionData->End += (UINTN) (1 << InstructionData->AddrSize); >>> + >>> + if (InstructionData->AddrSize == Size8Bits) { >>> + Address = *(UINT8 *) InstructionData->Immediate; >>> + } else if (InstructionData->AddrSize == Size16Bits) { >>> + Address = *(UINT16 *) InstructionData->Immediate; >>> + } else if (InstructionData->AddrSize == Size32Bits) { >>> + Address = *(UINT32 *) InstructionData->Immediate; >>> + } else { >>> + Address = *(UINTN *) InstructionData->Immediate; >>> + } >> >> (1) Can we simplify this as follows? >> >> InstructionData->ImmediateSize = 1 << InstructionData->AddrSize; >> InstructionData->End += InstructionData->ImmediateSize; >> Address = 0; >> CopyMem (&Address, InstructionData->Immediate, >> InstructionData->ImmediateSize); > > Yup, that can be done. "Address" is a type UINTN, but since this is X64 only code, an 8-byte copy isn't an issue. Should I add a comment about that above the setting of "Address"? Or should I convert "Address" to a UINT64 - although ValidateMmioMemory expects a UINTN... Thoughts? Thanks, Tom > >> >>> + >>> + Status = ValidateMmioMemory (Ghcb, Address, Bytes); >>> + if (Status != 0) { >>> + return Status; >>> + } >>> + >>> + ExitInfo1 = Address; >>> + ExitInfo2 = Bytes; >>> + CopyMem (Ghcb->SharedBuffer, &Regs->Rax, Bytes); >>> + >>> + Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer; >>> + VmgSetOffsetValid (Ghcb, GhcbSwScratch); >>> + Status = VmgExit (Ghcb, SVM_EXIT_MMIO_WRITE, ExitInfo1, ExitInfo2); >>> + if (Status != 0) { >>> + return Status; >>> + } >>> + break; >>> + >>> // >>> // MMIO write (MOV reg/memX, immX) >>> // >>> @@ -809,6 +855,58 @@ MmioExit ( >>> CopyMem (Register, Ghcb->SharedBuffer, Bytes); >>> break; >>> >>> + // >>> + // MMIO read (MOV aX, moffsetX) >>> + // >>> + case 0xA0: >>> + Bytes = 1; >>> + // >>> + // fall through >>> + // >>> + case 0xA1: >>> + Bytes = ((Bytes != 0) ? Bytes : >>> + (InstructionData->DataSize == Size16Bits) ? 2 : >>> + (InstructionData->DataSize == Size32Bits) ? 4 : >>> + (InstructionData->DataSize == Size64Bits) ? 8 : >>> + 0); >>> + >>> + InstructionData->ImmediateSize = (UINTN) (1 << InstructionData->AddrSize); >>> + InstructionData->End += (UINTN) (1 << InstructionData->AddrSize); >>> + >>> + if (InstructionData->AddrSize == Size8Bits) { >>> + Address = *(UINT8 *) InstructionData->Immediate; >>> + } else if (InstructionData->AddrSize == Size16Bits) { >>> + Address = *(UINT16 *) InstructionData->Immediate; >>> + } else if (InstructionData->AddrSize == Size32Bits) { >>> + Address = *(UINT32 *) InstructionData->Immediate; >>> + } else { >>> + Address = *(UINTN *) InstructionData->Immediate; >>> + } >> >> (2) Similar question as (1). > > Will do. > >> >>> + >>> + Status = ValidateMmioMemory (Ghcb, Address, Bytes); >>> + if (Status != 0) { >>> + return Status; >>> + } >>> + >>> + ExitInfo1 = Address; >>> + ExitInfo2 = Bytes; >>> + >>> + Ghcb->SaveArea.SwScratch = (UINT64) Ghcb->SharedBuffer; >>> + VmgSetOffsetValid (Ghcb, GhcbSwScratch); >>> + Status = VmgExit (Ghcb, SVM_EXIT_MMIO_READ, ExitInfo1, ExitInfo2); >>> + if (Status != 0) { >>> + return Status; >>> + } >>> + >>> + if (Bytes == 4) { >>> + // >>> + // Zero-extend for 32-bit operation >>> + // >>> + Regs->Rax = 0; >>> + } >> >> (3) This is also seen with opcode 0x8B, but can you remind me please why >> we ignore (Bytes == 1) and (Bytes == 2) for zero extension? > > That comes from the APM Vol 3, Table B-1, that says, in 64-bit mode, for a > 32-bit operand size the 32-bit register results are zero-extended to 64-bits. > >> >>> + CopyMem (&Regs->Rax, Ghcb->SharedBuffer, Bytes); >>> + break; >>> + >>> // >>> // MMIO read w/ zero-extension ((MOVZX regX, reg/memX) >>> // >>> @@ -886,6 +984,7 @@ MmioExit ( >>> break; >>> >>> default: >>> + DEBUG ((DEBUG_INFO, "Invalid MMIO opcode (%x)\n", OpCode)); >>> Status = GP_EXCEPTION; >>> ASSERT (FALSE); >>> } >>> >> >> (4) We should use the DEBUG_ERROR log mask here. > > Will change. > > Thanks, > Tom > >> >> Thanks >> Laszlo >> >> >> >> >> >>