From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.89]) by mx.groups.io with SMTP id smtpd.web10.13648.1602772426682847951 for ; Thu, 15 Oct 2020 07:33:46 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=CDWMf7Lc; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.244.89, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=cJjor6Aao7A5YkwyUDy+v+rmNPhz0XRQipBaPtcf7SyVFlMBeOrWpaZm+1F8EGj/yY5JaShteEE0msfylUIrL4wMq9NVZoE/A2gDV+28O2oFczCnS2hBj5/osgZwFgbwUUFl4LC7L2YuEzcVf1KDwm8HiR2a+zWjWqfCEBmvPYQ57oVPM0ey9R3VwThkhZQJwvQruqN81kWpgmZKKiYPbFFkERJDJ7jMIfTIjCHm+vS6yHXkbo1T+IbjfVPCMhp/65yRjjYxmrkaXpjpM/XSUK1/p0zWXHwrx6EOktpXtwfcNCxsJhoV1LnvhtJPwz4gsW8+axFyedNpjh761qR03w== 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=vwEWasb2WHf7itiqGwdW5xxAvQHD4gx8+RPhGdCKfLI=; b=mwWOiDKrlcK3L05Q7GkPt59t8ISQ2n/OahcgRt/XBTj4UKFIkO1La2vWfke/iO0fah4HoVl+HkIvRfFjClj/N4JUPOjfc3l5fFVqWV53bZmzpeBpGqsSu8F2hLGmVUe0pecqevZAvy9VJGQEfJg26N/9Od9IXEcIQJWIJyuwvGsdT63PsiehFZUXimcLHWRAL6PWqFQDz1f4AdK+BiL5r+sxTiNN25hPeNLudgLRyktE9bqAD5GVPY8EWbR4FUW5yAkIjh43OS6P/xDHf/Wol8h/ianVM4dBgkbXttLCVO6IAYitAf1uSvJIXXgfTMvBqVvswIlf22STzjGMKj8Cyw== 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=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=vwEWasb2WHf7itiqGwdW5xxAvQHD4gx8+RPhGdCKfLI=; b=CDWMf7Lc24Uc4K/2UKZnNULU1CXi6yhs22HHhYpT+NwprqpdkkXF+ahA36WmCeeLXgXPjTzWv0LYr/VsPvTmciasfn36WPJ+KeoVx8t+0039JubppCoAxeqyr7o73ju6nm98nRpAAYkCXf1ILb7VUOSxK8vjYpk6hS2iGsgP9Rk= Authentication-Results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB3081.namprd12.prod.outlook.com (2603:10b6:5:38::27) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.20; Thu, 15 Oct 2020 14:33:45 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4d88:9239:2419:7348]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4d88:9239:2419:7348%2]) with mapi id 15.20.3455.030; Thu, 15 Oct 2020 14:33:45 +0000 Subject: Re: [PATCH 2/9] OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT From: "Lendacky, Thomas" To: Laszlo Ersek , devel@edk2.groups.io CC: Brijesh Singh , Jordan Justen , Ard Biesheuvel References: <34bc7f77-2b5a-2e74-e78e-ddf606b6305b@redhat.com> <71b855ac-8381-4767-798c-2fafd2b9bd56@redhat.com> <78000bae-da10-970c-af49-0e484493f47c@redhat.com> Message-ID: <74720266-1b2a-8be1-2a3f-e0e9ee0e63ed@amd.com> Date: Thu, 15 Oct 2020 09:33:43 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN4PR0501CA0126.namprd05.prod.outlook.com (2603:10b6:803:42::43) 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 SN4PR0501CA0126.namprd05.prod.outlook.com (2603:10b6:803:42::43) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3499.9 via Frontend Transport; Thu, 15 Oct 2020 14:33:44 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: b7d9006b-c512-4ab1-14ee-08d871175204 X-MS-TrafficTypeDiagnostic: DM6PR12MB3081: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:6108; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: ga61JyGxUH++pmTtvw6hoS2AtDIVmRiVvkWYfaHo5inaqnbLO7Asu2VO4h0e/9/MeIWkyva4EZ7fnPu7OoEVnnHbr1buvFQgPT9yBijeCRx0umYDXbMHLOxO35kvuUZ3QCAfK3AwGQf0Jw934KdKTNcJ7GfhnHMhQskTnqATfG6hwI38amGi6ICDJqFP1AGzkryNhUTeKzd73Wp00h11EIgKKqYInEBdHWfa8le7GQ9vBYP2MHydbbqeNtfg7SlxC6l59bl7NXXS+cz4PaxPcUAT4kDZJlF/TeBTZ6kO2dQs0664y7k6K12FPCF+pYume71hu7GUWnbxUjylSHyGt3K+/EDj7mqGKe9z2D6Vwq5/GNpSkmFVziXFWdYtSu7i/lkkvaUD5d35gC2AX76MeLHaffAh8RkAxVvMn5ypB5g= 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)(136003)(376002)(366004)(396003)(39860400002)(26005)(186003)(6512007)(6506007)(16526019)(34490700002)(2906002)(53546011)(6486002)(54906003)(86362001)(36756003)(316002)(4326008)(66476007)(66946007)(8676002)(478600001)(5660300002)(31696002)(66556008)(31686004)(2616005)(956004)(83380400001)(8936002)(52116002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: jzDWXqzWq4mHM6vg3T6RaShcaVF27za8DX3SKK3bgbQDI/Qm21cZEshDo8JqaKBdE4vw+624sDFoU55WYwmZAM2ccmLhWjXh0W0c4Ps0b6aguma8Fki+W+TnomvwglVk+h+Dl4JHSKjYyaIn/do1i1jlUG2lTn7j7aKp720oASA8lIkmo5+TOTaIP0DVMj9jeiWlBl1+2y7uc8K8wC24Z64Y+DoXZaVgwEE4oVrUWyydUlNsqWM6SjTwN34xbzrfo1c8dzLyNgWID5atSQqi2vlmAk0g/b5ASFxevX37ydeQuPcP49mJnmdEdl9vQCt76cwbmOgx1ZR6bE2HFxB9Oo8gki06aVSQHj2HJYscKesqgN0ishroB0yslny9UcTQnQI1ua/br4GzAxGU8HK7AXbXYC5RunbAC43lMh8uoDM9nRpbN/i22I8NtZI2cV2nBDlw3pgAqnaENurGLqqga4Ofqw2irPjrZNDPY7ZXYP5Pcb0ofDZ2+zfIenvobSE69mqGeCfWdSW+73HKufiSuXqdOj1ZNOGt2oVvFOynfGfQIXAs7R2UoJ8lkxF5Tm3/9yo7V3NMP1wn5wzzVo/3WoLZ+HTDBO+69Kttwr7CTTlMvoJY27/IhsDX2epTqYOiLGuJHz0GJXASfkAFpzHWwQ== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: b7d9006b-c512-4ab1-14ee-08d871175204 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Oct 2020 14:33:45.0809 (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: 0hLPa+OdNM4Q5/htdE1TDgji/qjh6T1TkaP1k/3lj3k5AqkbEDo5EufwI7pLIethWTXW0UuVw6cBPDRPey1l9g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB3081 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 10/15/20 9:10 AM, Tom Lendacky wrote: > On 10/15/20 4:19 AM, Laszlo Ersek wrote: >> On 10/15/20 10:50, Laszlo Ersek wrote: >>> On 10/15/20 10:47, Laszlo Ersek wrote: >>>> On 10/10/20 18:07, Tom Lendacky wrote: >>>>> From: Tom Lendacky >>>>> >>>>> All fields that are set in the GHCB should have their associated bit = in >>>>> the GHCB ValidBitmap field set. Add support to set the bits for the >>>>> software exit information fields when performing a VMGEXIT (SwExitCod= e, >>>>> SwExitInfo1, SwExitInfo2). >>>>> >>>>> Fixes: 61bacc0fa16f ("OvmfPkg/VmgExitLib: Implement library support=20 >>>>> for VmgExitLib in OVMF") >>>>> Cc: Jordan Justen >>>>> Cc: Laszlo Ersek >>>>> Cc: Ard Biesheuvel >>>>> Cc: Tom Lendacky >>>>> Cc: Brijesh Singh >>>>> Signed-off-by: Tom Lendacky >>>>> --- >>>>> =C2=A0 OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 30 +++++++++++++++++= +++ >>>>> =C2=A0 1 file changed, 30 insertions(+) >>>>> >>>>> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c=20 >>>>> b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c >>>>> index 53040cc6f649..6cf649c6101b 100644 >>>>> --- a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c >>>>> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitLib.c >>>>> @@ -78,6 +78,32 @@ VmgExitErrorCheck ( >>>>> =C2=A0=C2=A0=C2=A0 return Status; >>>>> =C2=A0 } >>>>> +/** >>>>> +=C2=A0 Marks a field at the specified offset as valid in the GHCB. >>>>> + >>>>> +=C2=A0 The ValidBitmap area represents the areas of the GHCB that ha= ve=20 >>>>> been marked >>>>> +=C2=A0 valid. Set the area of the GHCB at the specified offset as va= lid. >>>>> + >>>>> +=C2=A0 @param[in, out] Ghcb=C2=A0=C2=A0=C2=A0 Pointer to the Guest-H= ypervisor=20 >>>>> Communication Block >>>>> +=C2=A0 @param[in] Offset=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Offset = in the GHCB to mark valid >>>>> + >>>>> +**/ >>>>> +STATIC >>>>> +VOID >>>>> +GhcbSetOffsetValid ( >>>>> +=C2=A0 IN OUT GHCB=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *Ghcb, >>>>> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0 GHCB_QWORD_OFFSET=C2=A0 Offset >>>>> +=C2=A0 ) >>>>> +{ >>>>> +=C2=A0 UINT32=C2=A0 OffsetIndex; >>>>> +=C2=A0 UINT32=C2=A0 OffsetBit; >>>>> + >>>>> +=C2=A0 OffsetIndex =3D Offset / 8; >>>>> +=C2=A0 OffsetBit=C2=A0=C2=A0 =3D Offset & 0x07; >>>> >>>> (1) I suggest improving the consistency of operators -- please either >>>> use division and remainder ("Offset / 8" and "Offset % 8"), or bit shi= ft >>>> and masking ("Offset >> 3" and "Offset & 0x7") >>>> >>>> With that: >>>> >>>> Acked-by: Laszlo Ersek >>> >>> ... I realize I didn't make the same comment for GhcbIsRegValid(), so >>> I'm taking back the above -- consistency with GhcbIsRegValid() is more >>> important. No change needed here. >>> >>> Acked-by: Laszlo Ersek >> >> Wow, I'm needing *a lot* of time for getting back into this. Sorry about >> that. :/ >> >> So, as I'm slowly grasping the idea behind this series (--> wherever we >> make a VmgExit() call, having populated some fields in the GHCB, make >> sure the "valid bitmap" is set correctly too), it's becoming clear that >> we need a new "VmgExitLib.h" API. >> >> Because, (a) VmgExit() is declared in that lib class header anyway, and >> the new helper function effectively helps us set up the (thick) >> parameters for that call, (b) at the end of this v1 series, we have the >> "valid bitmap" setting logic triplicated (not counting the assembly code >> logic in patch #5): >> >> - GhcbSetRegValid() -- existent function in >> "OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c" >> >> - GhcbSetOffsetValid() -- function basically identical to >> GhcbSetRegValid(), added in this patch to >> "OvmfPkg/Library/VmgExitLib/VmgExitLib.c". (This means that the same >> library instance will have two copies of the same logic.) >> >> - QemuFlashPtrWrite() -- from patch #6 >> >> So I think we should first replace the GhcbSetRegValid() function with a >> public (UefiCpuPkg VmgExitLib) API called VmgSetOffsetValid(). This >> would likely take two patches -- first patch, introduce the API and add >> the VmgExitLibNull implementation under UefiCpuPkg; second patch, add >> the real implementation, replacing GhcbSetRegValid(), under OvmfPkg. >> Then, in the rest of the series, call VmgSetOffsetValid() wherever >> needed (in C code, of course; in assembly, the open-coded stuff is OK I >> guess). >=20 > Yup, I was toying with the idea of adding a new function to the library,= =20 > too. I'll do that, using the approach you outlined. Also, I'll add an interface, VmgIsOffsetValid(), so that all the=20 ValidBitmap manipulation and examination is contained in the library. I'll= =20 replace the GhcbIsRegValid() implementation with calls to this interface. Thanks, Tom >=20 > Thanks, > Tom >=20 >> >> Thanks, >> Laszlo >>