From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-DM6-obe.outbound.protection.outlook.com (NAM11-DM6-obe.outbound.protection.outlook.com [40.107.223.50]) by mx.groups.io with SMTP id smtpd.web10.13102.1602771040587449641 for ; Thu, 15 Oct 2020 07:10:40 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=FBmYrokd; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.223.50, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=X+a0j/Ddtxths4XOWTkspin6WKUVsAGfOzFl+PLJU/Xw9wSJpPO1tE8zUlJVo0umN1vTfcc6Ci7oKxbSr46RUL/rSfESeJ/C0COH6Zve7s+FvtKMGZc2krAiOH5p2j0XzQiX+1JFTZ1iQCuZi/9IILP08IHVuzAQiqUkADroy/P9je8dEmtbcKPi+0XKwT347M6BXib47+T9JVCmnB8NpolkKHlV0vzwoXZ9nv5+P+JF7dJ6I37+rl4qIIdOuLbuGP4mxOYta7bWftxXrej3ATXdJognTiYlCF1cBNTUIM9SLOAqc0YRzCFFdK0V5tchHej2N05+zfMtz4eaLPfJhA== 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=hRgli86sgyolzux491KjUnABC98qxg48IPLTXgdQPro=; b=BqGZe3GVURsubO70HaRZe8Eun2xhe/kzvxCLtwPIZdyBc4EGVd6bWZo9r7pQHoIM4Oz8MljkvVIsNBGoiaM0KP11xfbAMSFknIIzRV9JTNMLZZ8qdeiSIpO/kthvIGjFXEf8TPF4Vm0f3GdHm7zvs0bYuTPmoaf548SWShhCc1NbzWusz7k7pTpD9qk6TxlgKtGVHgcJvj7d2T40/P5DwfnqUvs1C0U48T80TDlgMRu99A+uP4h7iTDt1AvOvgdXoIEipN8lORDU9efvxM4LPqdaxyEtEC8dtuGQDhlRTjORiPrQXNxPkfk/A3jw4aQ9glhDpKO9AdPx2fvP5QzJzQ== 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=hRgli86sgyolzux491KjUnABC98qxg48IPLTXgdQPro=; b=FBmYrokdcfV0ZbtpbGBjivzrNRuxLKUhDj+2H8fj8THxIAh6AWp7agsYNfdttUOebUEK2geMB2Mk6jClF80F/xJfZzFizUeCwgcBl/PL4Y2Y/tG7IHmF2zhrf2+ZQzxgixPEyZYgd5KBIeOPbBfch2piKPEdc5Zg5zwVylv7g2o= 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 DM5PR1201MB0122.namprd12.prod.outlook.com (2603:10b6:4:57::10) 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:10:38 +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:10:38 +0000 Subject: Re: [PATCH 2/9] OvmfPkg/VmgExitLib: Set the SW exit fields when performing VMGEXIT 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> From: "Lendacky, Thomas" Message-ID: Date: Thu, 15 Oct 2020 09:10:36 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: <78000bae-da10-970c-af49-0e484493f47c@redhat.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN4PR0401CA0008.namprd04.prod.outlook.com (2603:10b6:803:21::18) 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 SN4PR0401CA0008.namprd04.prod.outlook.com (2603:10b6:803:21::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.21 via Frontend Transport; Thu, 15 Oct 2020 14:10:37 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: b17c5638-1ec3-4cb5-0b1b-08d87114174d X-MS-TrafficTypeDiagnostic: DM5PR1201MB0122: 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: Uo+U69TnhNEA+m20WF0JIVJ9YxpyuZshOF5MOTy7WJNY391YSN7oDoklGUQ630Tg/2G047SgIA8C8KYAOnYVR9OdMQKQ8/SYA5N9PkNqH98tSaSBgCkMtG8L1P5l25g4eaSLHqbJ0GPUi3tL+Bi0l/I5QKCQL+sFnOQOf+q3bZdDc9QFpxbmhyuX+M3X3lAOTzlGNBiuKLx9MZnUe8NSnn/NL45Ti/DqVZeUeys+MsuWAnmO3SwOUvJLPm0xA6f+mLDB3VxOMNiAljomd99rUlx04RDeLGqpxTH8z89t9lJfqghpyTyjFlHvmx27giSWgGFCaTyCd9pl2TbyTGhU2qkY6u4QVnENRz4wze3LTGhRI39EWKicnmV7ruUFvcztNp3VPa1Z6bSTpzOnbp16KVfkBVIKwS9aM7WbesLkae0= 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)(136003)(346002)(366004)(396003)(39860400002)(376002)(26005)(186003)(6512007)(16526019)(34490700002)(86362001)(2906002)(54906003)(36756003)(316002)(53546011)(4326008)(6486002)(6506007)(478600001)(66946007)(66476007)(8936002)(8676002)(31696002)(52116002)(31686004)(66556008)(2616005)(956004)(83380400001)(5660300002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: wHQGE/txklgz+7vbBGt2FWghaZDWL9B1WKyoLZ1YteFkgyV7EyLJ+1E1s0rjzdFc+MfBTXbLBwFLEMpI+tqpsoeWKu/VQFZvA4wtxYMjoUGbuYlp5hX37Qi3sPd4IND1OfAj88hjUNdCot2IKx30RY3fbJ0pPLII2vlRB30mL343kK/VgIGP8umZZjoUBaFBGbu4Q+zXU2ZOMe4BLUCvK25vOwTVaJcNEjEWYhPjuwSdD1lpXLxIJhQOmDrjs8zFbSiVn1lU7eoFeMGParKzjFoZdwmmJGPlUFoQT0VQW16Zuag3h62hx6OHKqqPLa396ZTwdi6/0E3XOW+f4qo6ikpY39LkJh7VnmGTROlCS3antNMJGwcojLJ0H1tmhsQ12lqTqO4lW2jVN2qx3vkmcqUKNx7TvYMoVJB0OcOVflgf+M4Vvorzw6ZrIG+w9NOMbIaWDQo5uujBPaexiWB8sy2q5eCoAdvlFWhU+eyFcQGWKQkEdA2r4/Co6gINvbU2VVpfwfQ5JjM4ZRu0OvC2WrYMSDlkjGMUCX0P807oTP/zrRI+eVFQEKARuEeMbnaHdqyEttI6AMKuHE3xOFmzfA/9bvIDwuVfTOkJWDSNFWwuMI/+lpK9wrpQJBSJNIsaG93S07oR9nvlKs5tnFJPXQ== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: b17c5638-1ec3-4cb5-0b1b-08d87114174d 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:10:38.0223 (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: nrSOM7mclP/43lPRwA1B1B4hZRfZzx5CLMk7P/TF3DZ+fNHsi5yJPlXHLYa3xy6D9wqZvbgzTU1Vz7myW/d88g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1201MB0122 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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 (SwExitCode, >>>> SwExitInfo1, SwExitInfo2). >>>> >>>> Fixes: 61bacc0fa16f ("OvmfPkg/VmgExitLib: Implement library support for VmgExitLib in OVMF") >>>> Cc: Jordan Justen >>>> Cc: Laszlo Ersek >>>> Cc: Ard Biesheuvel >>>> Cc: Tom Lendacky >>>> Cc: Brijesh Singh >>>> Signed-off-by: Tom Lendacky >>>> --- >>>> OvmfPkg/Library/VmgExitLib/VmgExitLib.c | 30 ++++++++++++++++++++ >>>> 1 file changed, 30 insertions(+) >>>> >>>> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitLib.c 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 ( >>>> return Status; >>>> } >>>> >>>> +/** >>>> + Marks a field at the specified offset as valid in the GHCB. >>>> + >>>> + The ValidBitmap area represents the areas of the GHCB that have been marked >>>> + valid. Set the area of the GHCB at the specified offset as valid. >>>> + >>>> + @param[in, out] Ghcb Pointer to the Guest-Hypervisor Communication Block >>>> + @param[in] Offset Offset in the GHCB to mark valid >>>> + >>>> +**/ >>>> +STATIC >>>> +VOID >>>> +GhcbSetOffsetValid ( >>>> + IN OUT GHCB *Ghcb, >>>> + IN GHCB_QWORD_OFFSET Offset >>>> + ) >>>> +{ >>>> + UINT32 OffsetIndex; >>>> + UINT32 OffsetBit; >>>> + >>>> + OffsetIndex = Offset / 8; >>>> + OffsetBit = Offset & 0x07; >>> >>> (1) I suggest improving the consistency of operators -- please either >>> use division and remainder ("Offset / 8" and "Offset % 8"), or bit shift >>> 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). Yup, I was toying with the idea of adding a new function to the library, too. I'll do that, using the approach you outlined. Thanks, Tom > > Thanks, > Laszlo >