From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-SN1-obe.outbound.protection.outlook.com (NAM02-SN1-obe.outbound.protection.outlook.com [40.107.77.70]) by mx.groups.io with SMTP id smtpd.web11.12361.1619098511753318949 for ; Thu, 22 Apr 2021 06:35:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=0v9CFDpe; 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.77.70, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZpbCt2MOOIgc3BSZx8dpEdRcgS+jE7Nrd+09XNbLU4qJNC+aNZWXqRIY+8ECzf2jGxKuo9fn1vhAgSVq+YPNL4ko5C+v9VvZHzEw8viEfpO/mK7iZnxe7LE1QOCd57IfJXhzwOtGgJt/t0asg6z/5vyq9wk8UQnaC3+bksYSLxdv0QZFrThOwTqoYQhXNXDwM7V15Y4EYvgcBO27at6p118+upObiIar3W5mm95T39W0C/3ju7HBtiA+zUqQjUFnxMh36gFPb8d67QWkTJLVmthnMFVZ2ohzAakXEgz9MTg9LHiD71tawwPqV5rcki05z1zPTnpkbO5oLmwUSI99Kg== 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=i+hZc2tDxY12oqlhCheLAgsxJOIq3WeuGCrrjQ+4XhU=; b=Nc0YTGQjU+5ZtSk9xRZvSQFSXJ1HKD9fx92d6bjTQAqogG7djTJK8lz8I9CQI0UYjqPcpMM//RgVsAvAsL11Q9q6+lHrZO6yMcoBxx/eMFcGSjT7s747MzfKXutQv4GoEKQS/y3R+Ea1F6lPPZWZyr7lpM7C9c+PRV1Vpnw3bMycEvw1l7jGl/vBlijTMOwVPVQTnV8vEXLR7AUQ7bO6AMy0w4lFGOp7oS62HXjEEQU7eVlZ6GrAmWEsuwrCEE6oIha10XKIFhct2EtvEWpDLC8DCvJq1/LXLoFJS4UqHSAWYpoZR6k+qw7M+TA84BTzv2lOIkmVj7tAeRklInVGqA== 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=i+hZc2tDxY12oqlhCheLAgsxJOIq3WeuGCrrjQ+4XhU=; b=0v9CFDpekhauWUpjB6NPO4StjxdEcIsP+/FbSw3zuNhU0O8aj/97PxLjeYhPYrEng2Xk1osE+rx8BY3PG8okz6Osm/4YuykoQbj/wcu6dOLAlSIvxICXPit66twct/gj6RCceLXv64TseqT9/SYUkvozAsaG8n98EG+DmKOO17E= 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 DM5PR1201MB0025.namprd12.prod.outlook.com (2603:10b6:4:53::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4042.18; Thu, 22 Apr 2021 13:35:09 +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 13:35:09 +0000 Subject: Re: [edk2-devel] [PATCH 1/3] OvfmPkg/VmgExitLib: Properly decode MMIO MOVZX and MOVSX opcodes To: Laszlo Ersek , devel@edk2.groups.io Cc: Joerg Roedel , Borislav Petkov , Ard Biesheuvel , Jordan Justen , Brijesh Singh , James Bottomley , Jiewen Yao , Min Xu References: <71864a75c680c4f7f07ebf9611c9cc2d351ce5d0.1618959281.git.thomas.lendacky@amd.com> <7bf6b4ce-85c7-578e-973a-f976bdfa706b@redhat.com> From: "Lendacky, Thomas" Message-ID: <56f2c943-523e-46d5-dc4d-19e2bb31b07b@amd.com> Date: Thu, 22 Apr 2021 08:35:06 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: <7bf6b4ce-85c7-578e-973a-f976bdfa706b@redhat.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN4PR0501CA0148.namprd05.prod.outlook.com (2603:10b6:803:2c::26) 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 SN4PR0501CA0148.namprd05.prod.outlook.com (2603:10b6:803:2c::26) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4087.15 via Frontend Transport; Thu, 22 Apr 2021 13:35:08 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: be3cf227-978f-4739-cfeb-08d905937280 X-MS-TrafficTypeDiagnostic: DM5PR1201MB0025: 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: 8F9RcGlXyuAiabIu3TYE8ll7K/er2LCkQeJaOUIWntpD9mxsalAlZsQ3dNkgMtwG8uawOgdAi5bm47c77tET74oxN+aDBGQaQXl3rPfTpBXi4qFT/0mZxi0fFM49VEOH2Fw+zwhaKaofKrJBLBSWzt7N4XOiZCVv/wjPdiXUQ7jd/PHKecnX6UFYwRuWD/uX7H9kW2oCE8JVs+RwCQ+Oa4BCRFsfQxU0psariIv5msRLaoW6ezxpkh232+RwLss28ZnnZ+1cukCVV2KQtSaotv4ZsWhOf6/ReWhieIsYxxPeyex2cBeLca2eN/TXP/gFR4iWk79gPes1YyKPwLf5+Lw6Yfz28XpfeASH9G35/Zc5cVGUAyqPS5vyB8w51TgH6EUz9PfyTms/SXkDXvOcTwjfnZL4ftrN0tqW71RHoW5LI9IHOWD/n1QgYsOefmoaABh3FXOKTYjncENOn5VJHBO07zsUYGxG8F73BXzj074Qlo0jZdXDyhJynswgJhypj5m/Cdak1GBMN2kVoT185PORL1Aqaj3Zw8uQf7mryoZeW29xRW/6Ch2e/AilaoWrtb6bfb+xkuVnUe3eo9j4li/tMUj5NiDZVi4s+8hyYvCvJPqn1R3LA3H05voNQoLZHHSxJd0TpwcfX9fbf41EaWNUc6KwIzOCenv4Wwb3zsx70nCW9e1UUe0nEckKaFIGuerIF7uslu7pT5+9SsB+sfzl9ziBFVhr3bTO8l+RbaM/6uPZo68jyip4iNzPr2oQuulUOGw+lc3kZvzXt6eiKw== 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)(346002)(376002)(39850400004)(396003)(136003)(366004)(8676002)(8936002)(2906002)(2616005)(86362001)(66476007)(16526019)(36756003)(966005)(45080400002)(26005)(6512007)(186003)(31696002)(5660300002)(66556008)(956004)(38100700002)(83380400001)(316002)(54906003)(478600001)(6486002)(6506007)(4326008)(53546011)(66946007)(31686004)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?UitHYXJoUGRGVUJqdFIvdnNJSEpBNXl6cW5KQnVTeTlXdjFzVmw3UEhPaWFT?= =?utf-8?B?MDJxUGhvSVRzVGc3U1Z0STFHWm5iemhaU0s0VWErWXlXUVJUTE9WQXdYdmJW?= =?utf-8?B?WURHLzJITm55WWE2WHAyVzV0ek94bklYUlZJb3VUMzlTYmZuSFpGS1UzdklZ?= =?utf-8?B?eUJSVTlHUjN5NGFtNS9xNGdQekJqWDF5cFM0NjBhWnlwVHhOcFkwNDBuT2I4?= =?utf-8?B?UDYvS1ZwSGE3RDA0MURqQ0ZxN1l4REljYndjYi8zUEhaVnR6K2N5b0hUU0RS?= =?utf-8?B?U2VCVHdXQ1BTbzRQc2tQMGtqdE55MVRqYW5Sclp3ME84NWRHNXVDZ1gzZldj?= =?utf-8?B?dVhUa0xOUm9yekZxVGZ0OVBKcm1neit6UGQzYkpUWE1UYUtwWVB0N093SWl4?= =?utf-8?B?eWJRZ25zR2tYd3ovTnlCdm5lRGwzT1BqWVJ3OXNlSWZkU08xU09hRlhBNm1z?= =?utf-8?B?MzJaV2h6aTMydjVrSThiYTBZTDRNSVFjYTNESU04bGt3eTk0NzJrV09FQnVz?= =?utf-8?B?MUpjZUFGb0dVUk1USnQzMTE3UjRxQzJQRXJxNzNUQStCMlJPWmp0OGlsdjlx?= =?utf-8?B?dXViTWZaRCt3bHBaTm04S2NrRU9ROXlsRW0wb0xTb2NBQW9zbnFYSlZBVG1G?= =?utf-8?B?VCtrV2R4UlBScDVHNy80cm01c1Q2WWNsYWpPOEZjK0JZbnFCRkFKdkJLWXBN?= =?utf-8?B?MkNKYVBvTXVvYjdKaklJOTBDaWhWVmtsT2QrWGkwOHZOcDhhLzM2c0lPOXlT?= =?utf-8?B?b2VlaldFQzdidnZtOGhaY0JPN2ZVSlNJcVZ3WFdxSDlkZXZGWko3R3E1aWU4?= =?utf-8?B?VmhwQTlOcWdONXVPVWZ2RCtYRnAzYTYvMVl1M3IzU0VValRTbjVEQjgwOEpQ?= =?utf-8?B?SzZ6WENqMmJzVEMydkF5SzI4QnJiNkpJZlVXSmU5VzZLQjYxbUJWNGxRakRJ?= =?utf-8?B?d0hhMU82aWQ3YWFDcytLdUVPT2hJTzYvbGg0aGZpWG84Sll6SWxaVlhubnhp?= =?utf-8?B?MFNJdWdXUktzeHdiNnF4c0lpM3JVdGdJUllyR25XRnQ0QVlhZ1FqWFllM0p6?= =?utf-8?B?Ym5vbUsydVl1bC95UWZYREI5Y21JUEFPU1lWODc1ZjlSZ044UEo1MFhZUWxH?= =?utf-8?B?WjFVbHplTTB4NldlOVpIWHZZa1ZOaVRhVmlzV0NyK0tiS1lkdEtjdFFLTWp1?= =?utf-8?B?RURiM2M3Q2NyS3lMTE9jb3djaFluMHJpSlkxVmVtbkhmZUVqdWlUenZuSjZK?= =?utf-8?B?eDdPR29jR09OeGdUYVgyU2tHMUdoMlFQc21ROGVobG92UWplaGRKZkg3M1FC?= =?utf-8?B?NzRoc1BSZHZXNVkrOXdJRFdWV1ZWZTBFUmFabHpMdU44YnE2N1VOQU1xZzZK?= =?utf-8?B?clc5eThoaWFPQi9maEw5Q0R5d2pqNStwK0RkamJoVk9JeFJYTkcrVWx5dWl2?= =?utf-8?B?cHo0UDROZ1krTDl4QVV4dG1aZWdCR1JyT3JReU5tRGlLaUYyaUZFOVVMSUky?= =?utf-8?B?eGVuNWNvalR3RlpBa1prWndMRnBIYlJsNll4RkF3QWNqbTF6ZDRxWWhOU2FY?= =?utf-8?B?SkM3Qkl4Y29UcnkvNVFJUVhzbGRVY1BsNzRWQkE2WXEzRFdiT0xoSnRvRCtV?= =?utf-8?B?TklvdFduTEZJclVjK05sNldBZTAwbzl3Z0hvUWlaN0FnZmcwWnNoVXNTdERM?= =?utf-8?B?bmtKUUdFS3N1alpGRncwelNJbU85UWwxcHdHRzVIVTlkSG9WaVBpMEd5ZzBO?= =?utf-8?Q?arSM9p8rhMJvA7UhGILU12bBPyKPBWF7FOYJ7sR?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: be3cf227-978f-4739-cfeb-08d905937280 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Apr 2021 13:35:09.2270 (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: uDEO/yvagI7LocHJ3QWdAIZBp8y1ec9WAYf6JAdk/YKL7btKXq24Xjt8pBNcsYU0TyFTQL+0fBfoKIFO0xvgCg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1201MB0025 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 4/22/21 12:28 AM, Laszlo Ersek 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%7C22bf3a3ae9cb4421e93208d9054f79c8%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637546661229697941%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=1EmUDf%2FfuCuu%2BkXPZijzatfliplMhKEQH8kiZ9Z8ZF0%3D&reserved=0 >> >> The MOVZX and MOVSX instructions use the ModRM byte in the instruction, >> but the instruction decoding support was not decoding it. This resulted >> in invalid decoding and failing of the MMIO operation. Also, when >> performing the zero-extend or sign-extend operation, the memory operation >> should be using the size, and not the size enumeration value. >> >> Add the ModRM byte decoding for the MOVZX and MOVSX opcodes and use the >> true data size to perform the extend operations. Additionally, add a >> DEBUG statement identifying the MMIO address being flagged as encrypted >> during the MMIO address validation. >> >> 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 | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >> index 24259060fd65..273f36499988 100644 >> --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c >> @@ -643,6 +643,7 @@ ValidateMmioMemory ( >> // >> // Any state other than unencrypted is an error, issue a #GP. >> // >> + DEBUG ((DEBUG_INFO, "MMIO using encrypted memory: %lx\n", MemoryAddress)); >> GpEvent.Uint64 = 0; >> GpEvent.Elements.Vector = GP_EXCEPTION; >> GpEvent.Elements.Type = GHCB_EVENT_INJECTION_TYPE_EXCEPTION; > > (1) This can potentially generate a large number of debug messages; > please use the DEBUG_VERBOSE log mask. Actually, you will see this only once since the code will propagate a GP and the guest will terminate in this situation. > > (2) "MemoryAddress" has type UINTN, but %lx takes UINT64. Given that > this is X64-only code, functionally there is no bug, but it's still > cleaner to pass "(UINT64)MemoryAddress" to %lx. Will do. Thanks, Tom > > With that: > > Acked-by: Laszlo Ersek > > Thanks > Laszlo > > >> @@ -817,6 +818,7 @@ MmioExit ( >> // fall through >> // >> case 0xB7: >> + DecodeModRm (Regs, InstructionData); >> Bytes = (Bytes != 0) ? Bytes : 2; >> >> Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); >> @@ -835,7 +837,7 @@ MmioExit ( >> } >> >> Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg); >> - SetMem (Register, InstructionData->DataSize, 0); >> + SetMem (Register, (UINTN) (1 << InstructionData->DataSize), 0); >> CopyMem (Register, Ghcb->SharedBuffer, Bytes); >> break; >> >> @@ -848,6 +850,7 @@ MmioExit ( >> // fall through >> // >> case 0xBF: >> + DecodeModRm (Regs, InstructionData); >> Bytes = (Bytes != 0) ? Bytes : 2; >> >> Status = ValidateMmioMemory (Ghcb, InstructionData->Ext.RmData, Bytes); >> @@ -878,7 +881,7 @@ MmioExit ( >> } >> >> Register = GetRegisterPointer (Regs, InstructionData->Ext.ModRm.Reg); >> - SetMem (Register, InstructionData->DataSize, SignByte); >> + SetMem (Register, (UINTN) (1 << InstructionData->DataSize), SignByte); >> CopyMem (Register, Ghcb->SharedBuffer, Bytes); >> break; >> >> >