From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (NAM04-DM6-obe.outbound.protection.outlook.com [40.107.102.85]) by mx.groups.io with SMTP id smtpd.web11.26168.1628529683870268970 for ; Mon, 09 Aug 2021 10:21:24 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=OksOJGoh; 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.102.85, mailfrom: brijesh.singh@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mjOouASvrO4EsNf/DfG16GRRsEgVGNjh8NLR6/ZJcYevS/TCjmRgbSojwOdCwWQrjLl/+1pEDKhj1U7PRRGTwOwIzUiDcDpZPWmAD5fzVKlBfIl56uxpnPYZgNkHhSLnnDoDEtPZS5wt5YYGklpFON7PgsuIAKJrCiJMoOxM90SCUeEvp6gMr4QyrS/5bGNty+4NDOId7x1sk3w3hVBkTLU1yQGNR4DAAserYxvzcEuk0ktjJCWO3zjuYVTZJHX2tyffRP1BlXYQQ8uFJahE40Rx3ohIy8OPKtDFVTgdZPikkMRlY4eqb2SzTbumtyfGKrx19AJIoT8AuM2qYL5nbg== 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=V8jaUG9wIZ8m4FXYSdoKRvY/P29RZDe2Wvdwz5qEfYM=; b=fYZx//2RsnNX0GwQj/ysocDldijES4sMnNbCc6Erg4Us5EF5DQraU6e7NT8sd3LanrmhqXyqIZvEEalJXY06g56P6s7mrjyCA7n+k39w9dRIK1Cm4EfDidLh3mOtJ+0Ry/GsKmrruCJZSNA6IcFL5e8oJNlLbNMu8rux8EgLG9dURdj6eWPoffPVg0I12pOetpJMA153wHpkCZsBGqjL93FIcI9YKsSmzyIS5hdTC1BzuSTFyi0LX0IQwSBQVESHTOKujo+XECQtHt8AzclC/AXZCrYmAuZLREz5k1g4RovffCFrSnTbRXfE1abOLW37ciGf7XRu0nAKUsJuw94psQ== 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=V8jaUG9wIZ8m4FXYSdoKRvY/P29RZDe2Wvdwz5qEfYM=; b=OksOJGohQ7nu0ROjeW9DFFsc7iBbCPhbzgJ3E/+ZhP5bzaM2NW5S3QNhJUhbu9++2fyWMQkQ1ha1+JnH+Oa1rDCkH0IYLqf8ij8G+aOpqtRBJ6eAh3HSRMG5F3dlvJ2hO7FbIAUU0exXPi7WMHkJ6gdlX+rvA5psE+nnfjRvscY= Authentication-Results: amd.com; dkim=none (message not signed) header.d=none;amd.com; dmarc=none action=none header.from=amd.com; Received: from SN6PR12MB2718.namprd12.prod.outlook.com (2603:10b6:805:6f::22) by SN6PR12MB2719.namprd12.prod.outlook.com (2603:10b6:805:6c::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.15; Mon, 9 Aug 2021 17:21:22 +0000 Received: from SN6PR12MB2718.namprd12.prod.outlook.com ([fe80::a8a9:2aac:4fd1:88fa]) by SN6PR12MB2718.namprd12.prod.outlook.com ([fe80::a8a9:2aac:4fd1:88fa%3]) with mapi id 15.20.4394.023; Mon, 9 Aug 2021 17:21:22 +0000 Cc: brijesh.singh@amd.com, James Bottomley , Min Xu , Jiewen Yao , Jordan Justen , Ard Biesheuvel , Erdem Aktas , Michael Roth Subject: Re: [PATCH 2/3] OvmfPkg/ResetVector: update SEV support to use new work area format To: Tom Lendacky , devel@edk2.groups.io References: <20210804202003.17543-1-brijesh.singh@amd.com> <20210804202003.17543-3-brijesh.singh@amd.com> <0da0dec2-0e2f-8beb-22c2-c949b9a56b8c@amd.com> From: "Brijesh Singh" Message-ID: <0cfb2197-59ce-7285-95ce-75122b0347a1@amd.com> Date: Mon, 9 Aug 2021 12:21:20 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 In-Reply-To: <0da0dec2-0e2f-8beb-22c2-c949b9a56b8c@amd.com> X-ClientProxiedBy: SN7P222CA0027.NAMP222.PROD.OUTLOOK.COM (2603:10b6:806:124::30) To SN6PR12MB2718.namprd12.prod.outlook.com (2603:10b6:805:6f::22) Return-Path: brijesh.singh@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [10.236.31.95] (165.204.77.1) by SN7P222CA0027.NAMP222.PROD.OUTLOOK.COM (2603:10b6:806:124::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.17 via Frontend Transport; Mon, 9 Aug 2021 17:21:21 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 1871001a-b322-4598-12b3-08d95b5a1bb7 X-MS-TrafficTypeDiagnostic: SN6PR12MB2719: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:6790; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: sK8NzshsRpd1weHuxMprg5zvkpattdqRzOzV1MlcIgorekCSiqkvcnz6kh89CP6LY+nF8GQzJZu9Us9qJe1hQ5SeslA08cmjxf0t0c5gzLL3WNxSkHGfCLH1HkTzgTPQASK0UZfeYcLzxyf7RLMCTDO+Qg6YYCiTH6CKvC4I+DMr9Bb653tnnHRsWDoKeSK5gOTZqYV2PVo7xHEVfw/tB4sOvqvP355D/j6j7LGwktZXEvQq1E9SUcCSLb4+4OjKIeXqL80qkaw/R8hTiSsU3bBrUudADjzmMk8fOpRjxCeC+xJBmzgk20eaPhKy0fcSXVPmjgNWPp1dES5ux/jDSPTrjnBSDeKG2dCI9h9RTKcm7q15Jtifg4nMa0TJdVz9ka0qfp68CGlLTL+X4OcuixektcJ1WrBqSHrAQo23jUnirln84ZBvy6oesUrWpYU45Q/S7xHiJ4MNQij1xT+5y8qkP9JmPtKch0NHduVGueqHQYSDqmCariIfN0I0LLaVmWVfOmrHbsFXxnEvr5pjeg+D6slxqzWjcRViRNX4l27jt1pyzas2rkLzALTwf9he7pMFmWLicTEfuR+uJbHoIdX/19SU8RqsUliA67KmirGmraGtVWB6+/C7ypkxO4GoNE67K/kwi2W2S6Q39G7oYQF9XH4/4xwGtW+0z+CYJCVaROza4tt3Bbffxl50eT6d2yo8pcrb8wJkU89A818gckfHGyTPKF5h8GBOPC/txufWPxkJziPFnaho3VSdT1nRynagxUyvFfvPNqRon0BO06FexoQ2DEPupB77t96orCsG6o5PuHNetfr+JlGTuP1olZec3ZT4a6b3oV3hnJ3wRvX4FNcjeshXJyyeRnZbEZ4= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:SN6PR12MB2718.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(376002)(346002)(136003)(39860400002)(366004)(396003)(83380400001)(53546011)(38100700002)(38350700002)(52116002)(16576012)(316002)(31686004)(31696002)(2906002)(86362001)(186003)(36756003)(54906003)(6486002)(26005)(15650500001)(966005)(478600001)(5660300002)(4326008)(8936002)(2616005)(19627235002)(956004)(44832011)(66476007)(66556008)(66946007)(8676002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?U2NNZHRhdU5ML09XNC9RYjkwbm5oeXFmOXhQU3FDK3ZTOGV0Ym9tS3NKbVZr?= =?utf-8?B?V2NPRVBKSUEzaXFYS0k4OURnTlJpRWw2L3ZTb09SQ2gzbU1YWkNRUTJ3dm1z?= =?utf-8?B?YTBuVTVJeVkzeWwvSG83TXROUU1ZZjhWVm5TSThSOXBTZGNVdEYrc1Q5enor?= =?utf-8?B?WTg1Q281WDFkNCs5dmlnMlVBK3NBNy9PWWp1bXFDT3VqWk9wSW55b3NIRFN6?= =?utf-8?B?VC9XbUk3Nm9JTlNFdmNOQ0NoSis4MTdYVkVMY0ZsdFErRWlkeURWbVZoN04v?= =?utf-8?B?RjFRT2E0S0pZbEc5M3ZaZWlBeFYwV0d5WnNmNkhDcnVmRlhTd2I3WFlsY01X?= =?utf-8?B?WVhIU1pad3E2K1U3bUhrMXBlRGI4azQrMjRIN2MwMUVjcWtYdStkYTlVY0pW?= =?utf-8?B?ak5OTFU5N3I2dE50Q3RkN0psbHV3dU10MzgzV3ZsVDg5Uy9sZHRQQlNDZXRT?= =?utf-8?B?c04xZnEvdm04dzRNdEw0RWlnckMxV1BlVzY3RWVRZHF2ckt6YTNZelNOU0NG?= =?utf-8?B?SzZFajlJK3lFbEZrUmZYWWxoaUROMEZNSDJnTVFIYUpTWGhHK1k3WlNJVlZi?= =?utf-8?B?SDhiOWVLZ1NHQVBpMjBSWWV5RkRmZTl3S0p3aUtBLzdDdklDUXZSN2pERFlS?= =?utf-8?B?bGhCbW1LakhwZjF3bUh0eFc1Y1FBMlBFNG80S3puU1JNemZ1d0hQQm5YQ0Qv?= =?utf-8?B?YlJjUjVXN2RGbHF3MzVkMThmZ24xZFBhV0FYa1poQmV3Q0I4bEJYOHZUNGxI?= =?utf-8?B?Y1h2TDVSQ2tMS3RyOXAyVGJZRTRnb3dEc05DZEs0aGNteENpa1BPNlNHR2Uz?= =?utf-8?B?OEVsaktvVjVaM0hhS3AxZXRNeXQ2SS8zWElxMU5Kczd6Q2ExcDBrQ3hNQ2N2?= =?utf-8?B?T0w3R3U0aVFBanlaL0xJR3lrcitaRzlJTytJKzFLTU9Fait0Q01GbHVoQUpi?= =?utf-8?B?UmFPOXgzZHVQcE01SVJROVVHUFZxQ2FCd1REeVRlSmRYRXpXZmRueVFRdXRG?= =?utf-8?B?Wm0yc0Z6MGQ1SU9pUkxieFRFZ1UwM1RqRVluS1Z2UTljd3lVaXN1TzFzanln?= =?utf-8?B?d1NGMWRFVHo0RkpRdmZTcjEwS1ArbTk1dEJuV3RsNVh0WnNoeWdiSXlxemdO?= =?utf-8?B?TTV0ZEdaL0VYcDRKcWhkeXJxVk9DMEd6bXluNCs4TnVtdXlXWUlkV0dhNXZ3?= =?utf-8?B?MnBRdVZQYmsrTGt1TlN5aDJ2VTRZOTZwVFpZVC9DSmlKVjNEc0R5Qjh1dU83?= =?utf-8?B?bTFuRCt1WnBvQ2VUZjZLa0hEL1ZESXNqekRTU1FYMDVQc2ZObXh6WTJ4Rnow?= =?utf-8?B?b0gwNXhYbjdTVUM0ZlFiK0NmZGlpQkROZmFJd3hRcTV4MDdyQ1NMcDR5QmFy?= =?utf-8?B?aXp4cEhiSWYzRGg4aWg2Y1o5NmtzZ0pwQ3ZJSGZvak93eXo3eUtaNy8rOWFT?= =?utf-8?B?VXdsNTNpTUgzRlpVbUdjWWFScmplQnlDRUhwTXdqS2pXME1ndC9JL3Q0dmpX?= =?utf-8?B?ZzQ2RDFEWldUQVlteFdrNElpc3dlemZtc1IzOUhJTGNZamoxRjRTUjVmeW9N?= =?utf-8?B?YklTc1U1eDBqMmFiOXY4Q1dqNTY3SVFlSUo2N3RmUUNleVd0cnAyU3VHRXVE?= =?utf-8?B?RTRUSDhERm93ZVVuTnNsbjdzZTFJMTRUSERnVzRlS3NienNWWG1qbzZlT3p3?= =?utf-8?B?UDdSUXA1NkNmL1lHMnFZNmw5UHl4eFRDZHFiVnNlYnBSbTV3ekpDOEwranBP?= =?utf-8?Q?6/k6cbZ93DnhoxVhphIc+gaStnlbndkow4FyPdc?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 1871001a-b322-4598-12b3-08d95b5a1bb7 X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Aug 2021 17:21:22.0912 (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: 1nn+tXRwLV5HLqwXfnfxit1hrble2ho1Of8UMBhda5uXDH3UUAreSXZ3zo6NoHNyZArDB4+4FD9ANtlUnd5KCw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2719 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 8/9/21 11:54 AM, Tom Lendacky wrote: > On 8/4/21 3:20 PM, Brijesh Singh wrote: >> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3429 >> >> Update the SEV support to switch to using the newer work area format. >> >> Cc: James Bottomley >> Cc: Min Xu >> Cc: Jiewen Yao >> Cc: Tom Lendacky >> Cc: Jordan Justen >> Cc: Ard Biesheuvel >> Cc: Erdem Aktas >> Signed-off-by: Brijesh Singh >> --- >> OvmfPkg/ResetVector/ResetVector.inf | 1 + >> OvmfPkg/Sec/SecMain.inf | 1 + >> OvmfPkg/Sec/SecMain.c | 25 ++++++++++++++++++++++- >> OvmfPkg/ResetVector/Ia32/AmdSev.asm | 8 ++++++++ >> OvmfPkg/ResetVector/Ia32/PageTables64.asm | 4 ++++ >> OvmfPkg/ResetVector/ResetVector.nasmb | 1 + >> 6 files changed, 39 insertions(+), 1 deletion(-) >> >> diff --git a/OvmfPkg/ResetVector/ResetVector.inf b/OvmfPkg/ResetVector/ResetVector.inf >> index d028c92d8cfa..6ec9cca40c3a 100644 >> --- a/OvmfPkg/ResetVector/ResetVector.inf >> +++ b/OvmfPkg/ResetVector/ResetVector.inf >> @@ -34,6 +34,7 @@ [BuildOptions] >> *_*_X64_NASMB_FLAGS = -I$(WORKSPACE)/UefiCpuPkg/ResetVector/Vtf0/ >> >> [Pcd] >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase > > Laszlo was trying to keep things sorted, so you should move this down to > the end of the list. Yes, I will try to keep it sorted. > >> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize >> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf >> index 7f78dcee2772..82910dcbd5c2 100644 >> --- a/OvmfPkg/Sec/SecMain.inf >> +++ b/OvmfPkg/Sec/SecMain.inf >> @@ -56,6 +56,7 @@ [Ppis] >> gEfiTemporaryRamSupportPpiGuid # PPI ALWAYS_PRODUCED >> >> [Pcd] >> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfWorkAreaBase > > Ditto here, even though the list isn't truly sorted to begin with. Noted. > >> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase >> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize >> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c >> index 9db67e17b2aa..dda572c7ad7d 100644 >> --- a/OvmfPkg/Sec/SecMain.c >> +++ b/OvmfPkg/Sec/SecMain.c >> @@ -807,6 +807,29 @@ SevEsProtocolCheck ( >> Ghcb->GhcbUsage = GHCB_STANDARD_USAGE; >> } >> >> +/** >> + Determine if the SEV is active. >> + >> + During the early booting, GuestType is set in the work area. Verify that it >> + is an SEV guest. >> + >> + @retval TRUE SEV is enabled >> + @retval FALSE SEV is not enabled >> + >> +**/ >> +STATIC >> +BOOLEAN >> +IsSevGuest ( >> + VOID >> + ) >> +{ >> + OVMF_WORK_AREA *WorkArea; >> + >> + WorkArea = (OVMF_WORK_AREA *) FixedPcdGet32 (PcdOvmfWorkAreaBase); >> + >> + return ((WorkArea != NULL) && (WorkArea->GuestType == GUEST_TYPE_AMD_SEV)); >> +} >> + >> /** >> Determine if SEV-ES is active. >> >> @@ -828,7 +851,7 @@ SevEsIsEnabled ( >> >> SevEsWorkArea = (SEC_SEV_ES_WORK_AREA *) FixedPcdGet32 (PcdSevEsWorkAreaBase); >> >> - return ((SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0)); >> + return (((IsSevGuest()) && SevEsWorkArea != NULL) && (SevEsWorkArea->SevEsEnabled != 0)); > > The IsSevGuest() function checks for a NULL work area, so there's no need > to check for SevEsWorkArea being non-NULL now. I think it would read > better, though, to do: > > if (!IsSevGuest ()) { > return FALSE; > } > > SevEsWorkArea = ... > > return (SevEsWorkArea->SevEsEnabled != 0); > >> } >> Sure, it makes it a bit more readiable and avoids unessary checks. >> VOID >> diff --git a/OvmfPkg/ResetVector/Ia32/AmdSev.asm b/OvmfPkg/ResetVector/Ia32/AmdSev.asm >> index aa95d06eaddb..87d81b01e263 100644 >> --- a/OvmfPkg/ResetVector/Ia32/AmdSev.asm >> +++ b/OvmfPkg/ResetVector/Ia32/AmdSev.asm >> @@ -171,6 +171,9 @@ CheckSevFeatures: >> bt eax, 0 >> jnc NoSev >> >> + ; Set the work area header to indicate that the SEV is enabled > > s/the SEV/SEV/ Noted. > >> + mov byte[WORK_AREA_GUEST_TYPE], 1 > > The "1" should probably be defined in ResetVector.nasmb as a %define. Sure, I will define the constant > >> + >> ; Check for SEV-ES memory encryption feature: >> ; CPUID Fn8000_001F[EAX] - Bit 3 >> ; CPUID raises a #VC exception if running as an SEV-ES guest >> @@ -257,6 +260,11 @@ SevExit: >> IsSevEsEnabled: >> xor eax, eax >> >> + ; During CheckSevFeatures, the WORK_AREA_GUEST_TYPE is set >> + ; to 1 if SEV is enabled. >> + cmp byte[WORK_AREA_GUEST_TYPE], 1 >> + jne SevEsDisabled >> + >> ; During CheckSevFeatures, the SEV_ES_WORK_AREA was set to 1 if >> ; SEV-ES is enabled. >> cmp byte[SEV_ES_WORK_AREA], 1 >> diff --git a/OvmfPkg/ResetVector/Ia32/PageTables64.asm b/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> index eacdb69ddb9f..f688909f1c7d 100644 >> --- a/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> +++ b/OvmfPkg/ResetVector/Ia32/PageTables64.asm >> @@ -42,6 +42,10 @@ BITS 32 >> ; >> SetCr3ForPageTables64: >> >> + ; Clear the WorkArea header. The SEV probe routines will populate the > > How about: > ; Initialize the WorkArea header to indicate a legacy guest. The ... > >> + ; work area when detected. >> + mov byte[WORK_AREA_GUEST_TYPE], 0 > > And then use a %define here for the '0' > I will make the required changes. >> + >> OneTimeCall CheckSevFeatures >> xor edx, edx >> test eax, eax >> diff --git a/OvmfPkg/ResetVector/ResetVector.nasmb b/OvmfPkg/ResetVector/ResetVector.nasmb >> index acec46a32450..d1d800c56745 100644 >> --- a/OvmfPkg/ResetVector/ResetVector.nasmb >> +++ b/OvmfPkg/ResetVector/ResetVector.nasmb >> @@ -72,6 +72,7 @@ >> %define GHCB_PT_ADDR (FixedPcdGet32 (PcdOvmfSecGhcbPageTableBase)) >> %define GHCB_BASE (FixedPcdGet32 (PcdOvmfSecGhcbBase)) >> %define GHCB_SIZE (FixedPcdGet32 (PcdOvmfSecGhcbSize)) >> + %define WORK_AREA_GUEST_TYPE (FixedPcdGet32 (PcdOvmfWorkAreaBase)) > > Create %defines for each of the defined enum types from the first patch. > Got it. thanks