From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-MW2-obe.outbound.protection.outlook.com (NAM10-MW2-obe.outbound.protection.outlook.com [40.107.94.81]) by mx.groups.io with SMTP id smtpd.web11.200.1627487956812946322 for ; Wed, 28 Jul 2021 08:59:17 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amd.com header.s=selector1 header.b=P7b5OsUm; 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.94.81, mailfrom: brijesh.singh@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WfqSEgw1ivk9/Iv6o+ZDuew5O6riohTzvHfn0rFFUGXG/UzSh3cQSTMQ17TWO6dS9f+4aTZhBkE4lYGSHthFo/6lSX8EruyWn3Tx+hh400xntHUJGYdCYg1MLCnYsjVUcTeZr3X0TTBLCYpgbjKmtXl9PBR5bdqr7ea2EscTEeiPJaHhgG+mV5WKpUFh9Tkf8DdiKBgdunZsq2AZmOAdstKnkwngje+cEO0VFYeus18MmJunRR/APuPskpYogBRv9bRijwX7FLzcU51BslaWrX7dGtftbW7MGagQalY7OPYWq85XT39J7Plls8F12D2WHv1DWGj8kzap7UmC983Tjw== 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=te3koTjjQPVdQaoGS+RfB/0WvVtoR/chUDoFaB1RsN8=; b=do+IVftyrr4BpzVFPi0t6tjdkEJKrCPNoQnWC/OrPwOtknTIFzJz0T9omfuFDeT3DvmJLdYmUS6GzgnNNYVxFim3vHNDdeg1pIZ2jCcobhWXzGMPs7l3/2DdAhnYdgwzdgTL8koOfK6uB/HHus1KsZ7ikB0afHJwwxWWELZ4Thku2PL3bbeCjEQkciRS8VRb7zL8gFBqLZe6AfO01+2h7akLJ20n7ySdqVj6xFQUsp7uhcdnM8YXQOe7S+M0LmGxKTwcA1wKmdh/5oIn11vAyXoBHrSbLhblK+cdohS1mKDb+QuLv6qfOpJKxFhoCdLOxL9/KfVjIa8WnQKbqi1mKg== 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=te3koTjjQPVdQaoGS+RfB/0WvVtoR/chUDoFaB1RsN8=; b=P7b5OsUmXi5OKzV6iDudqNhJTaIqC63EY755wgFZEmSJDDcBKztOGQGYLBrOgYhTY043q+eMO2uAN1vm1vukE4d75NqEZHKWiLqIoBWQ4uPqfl8/3FZwHwXBmS/OqkzaGMCF+8dgNeyx+7rwHIB5nLrsuXsTkuomlkj6mJkbJCs= 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 SA0PR12MB4592.namprd12.prod.outlook.com (2603:10b6:806:9b::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4352.26; Wed, 28 Jul 2021 15:59:15 +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.4373.019; Wed, 28 Jul 2021 15:59:15 +0000 CC: brijesh.singh@amd.com, Ard Biesheuvel , "Justen, Jordan L" , Erdem Aktas , James Bottomley , Tom Lendacky Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector To: "Yao, Jiewen" , "devel@edk2.groups.io" , "Xu, Min M" References: <4E4F0C83-ED04-4CFC-BDE2-33825C106DB9@intel.com> From: "Brijesh Singh" Message-ID: <97ad36da-13cd-ccb1-48f3-17ee03934aa6@amd.com> Date: Wed, 28 Jul 2021 10:59:13 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 In-Reply-To: X-ClientProxiedBy: SA9PR13CA0035.namprd13.prod.outlook.com (2603:10b6:806:22::10) 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 SA9PR13CA0035.namprd13.prod.outlook.com (2603:10b6:806:22::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4394.8 via Frontend Transport; Wed, 28 Jul 2021 15:59:14 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 4b3a2df5-836c-4e75-302f-08d951e0a621 X-MS-TrafficTypeDiagnostic: SA0PR12MB4592: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:8882; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: fWXo8EL1bczRaJ1ryiwAMJRHrwRIrZhhLi91SqcnCdP6kK2FwORaYXDxOgdPkSYoI0YVzMMbzFG5ifiLvnYp42YlpMa04zhH+1vSfx1EoE2GMKs/izkazBY/xd629gtAvusIMypEX4veJhWOu+eZUXJbvM+X6M9b+17jK6aDUxUQ7CxyhLiuXYu4nD3JeAI6B1IuA1yaogJScdYkccGr8q3QVe02JUGwZStUdmGQWz5ZN4XdojNB0N47AnXOFykdveOGBIyDoi6znqgppc9n1WJbE1uB1uWfbKq5wE9444+HCnRRQgPQMUVlGcvIi9c5tRt8awUh19Hg8jDHdWDVwgIUg2Y+p4L68Kqjm6k81lwWXajCKByeVmwzYieAE1goOgDVgfZ/CMNwEqsuQymWpDY/8NLjlbjzLaLjwSEPZshUrYG+A7rL+6r45HXZAV6sGsBj171ByQx4K9tFCkgN3s3xzUpTxZqcBEl4N4HFQbMkKgI7s6oQVujdQ9/M+7mD1HkgzPV451E2DAU6RbVWEzNhwWQqgjA8ONI5PyaPYWFWWrsdUoHEDFG7zvoY4xtRjEQ/pDzag0dOpoAm9TzfdWoJ6j25+rhZGclzX9dk26eKDeqTv2/P0jmkAwZmIeHNOR1SXNi6rk7JBXeF81rVmLwc24fFNqVqnekQva7wMdFWFhcxeFZPJDr1YN2BzDD9rmuKqmUN48FCvJmcN6Zzx0J/Sb4vtMbw4xfkSrv2pr5sIHzze2nfUHhdnvOdheLi283XhGOyMB2ZPP6EmQzDrvWraWTX0WyPHCGveT1fEUnUppTXGdDnEL5vWEtGlLVQL+x3j3erskglDf1vnzMcS6H84tnsKxdJd1e+5fB2kqk= 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)(39860400002)(396003)(346002)(136003)(366004)(376002)(83380400001)(86362001)(44832011)(38100700002)(38350700002)(26005)(53546011)(5660300002)(52116002)(66946007)(2906002)(110136005)(54906003)(16576012)(36756003)(186003)(4326008)(316002)(966005)(6486002)(66476007)(66556008)(8676002)(478600001)(31686004)(31696002)(45080400002)(8936002)(956004)(2616005)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?NUgIrq7FfwEIZWqZ8zfLLMKEeVI/HU1HnQWUlZvglCrrwPLeK0xx//kzwoh0?= =?us-ascii?Q?NwJLBspjJsuatdwZ0xsJEKzc27Cl8xaOSmIOHxg9H9pPI/lhNFH/bzjERGra?= =?us-ascii?Q?Att9VzVO4KB/mQiJFCiK55/f6wc/UDlQlY//2ILKfuGlwBtjenVe5X5326KX?= =?us-ascii?Q?iVHCUK1gZxYIPvKdznJ8JZopMNcs4X5iM2PqP7CBmLE8FHwPyDmHw0A7NZxO?= =?us-ascii?Q?X7T3Goj58PgRdTXwdLjjzA2ZpQze+sCbd4G+JK74HducwY8/ADioJ9iikeUB?= =?us-ascii?Q?wTrVHSui034QLPuXNZJT+vhtG9dZ830YWJmwEg6r98cDbh17l7YRmifRxKGl?= =?us-ascii?Q?vhrd9cqmwwRm+W4+7GulUvTkNWDONG9/5iLZA2AjpDQnb2398ETpxKu0Mvwn?= =?us-ascii?Q?fix9neDjgqbHJSfgsDm2laDJjEn9d8Jyom4UVH0t8Xd8L8ghTE/WE5ymjrkA?= =?us-ascii?Q?Vre6QLIp2Mq/BbrqG52fATYtdOgePke+hMIopHgP+TRb/3q3aMJCAo0tzLZH?= =?us-ascii?Q?X0mgHhFqHLjFEAfcvrcSXr9jqvL6jBXvE/9Z79koXVtNCox3VmshNJxZXfpe?= =?us-ascii?Q?meAyBHt8uNC/2p6Ril6bV9eIftstJ1PSNEzj7q8axHFvz0yAtALrf+5WaNcq?= =?us-ascii?Q?ojBbSFC3b+wInGJ8X6GZkAtndn5gffNC7AUe1TZgNiQGcVsmhlaTF9YbxJeM?= =?us-ascii?Q?8kZwEflE3DEI6knaN1sf/j8QtXsQ5jsfnguHBoxLSCVam3mqq8DKmUbZtA9M?= =?us-ascii?Q?lWGX7ceiVGRqgfbM9O06nhk6eC6nDChAykfYNs7VYInfCfPJmi2gNqURSG6B?= =?us-ascii?Q?Oztnx1OLTMvuDYhZ/nIoAqaUSJWlTLF4WQ1opeQqawiZMS5qtmeXq2xhCRyK?= =?us-ascii?Q?86H36V6LyEYXLRkVm60N1q3i83nh6h0wOteYHXWb4620SYMvg3bsqVO7ADXg?= =?us-ascii?Q?ifFZnvz9RkytbcHkl3UHeATY8Mc11U7H73eQLPQmKT4chBufwbYYK/cAFHCC?= =?us-ascii?Q?APfHMEEK3B66hrIeZHa5TLHh/5OIVCk+ptrqw16BLCPP7mHmGGMdSBRmaBWH?= =?us-ascii?Q?RPvRLWMZfG9q8QyrWH4vrrnVU3JYVuSssLPDeoMEwRNHo/I7AJTAK0Neypfg?= =?us-ascii?Q?x8wiWWwuzOVXPawTH1zk2zPn9D7Fc+8KcauZSz/oVC28KfDggfqsUcM+QHNd?= =?us-ascii?Q?4yBW8TbpSUjnYCHeyx6DphUxCiMwwzNXg5lp5nS5V2uFULSkaK+cq1MONl/M?= =?us-ascii?Q?Aq6QO26IUsTizNU1AopxTz3lGDIcohqP2eJ+UbUvSsg5tYq2Xht3RXkQD9J6?= =?us-ascii?Q?rjRA34MU1NnGuYL6zhF1xfLc?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4b3a2df5-836c-4e75-302f-08d951e0a621 X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jul 2021 15:59:15.2442 (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: qiwG3bUOnU8za3pn1BxfrxfL1iEpnZSzVZjOcKsP/Sf77SWheGMWLrFppXU+Ae3y1WlUAx6FwVN+5XsfI93ycg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR12MB4592 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable Hi Yao Jiewen, I guess I am still trying to figure out why we need the header in the=20 work area. Can't we do something like this: typedef struct { UINT8 SevEsEnabled; // If Es is enabled then this field must be zeroed UINT8 MustBeZero; UINT8 Reserved1[6]; UINT64 RandomData; UINT64 EncryptionMask; } SEC_SEV_ES_WORK_AREA; typedef struct { // If Tdx is enabled then it must be zeroed UINT8 MustBeZero UINT8 TdxEnabled; UINT8 Reserved2[6]; .... } TX_WORK_AREA; typedef union { SEC_SEV_ES_WORK_AREA SevEsWorkArea; TDX_WORK_AREA TdxWorkArea; } CC_WORK_AREA; I am trying to minimize the changes to the existing code. The SEV and=20 TDX probe logic should ensure that if the feature is detected, then it=20 must clear the MustBeZero'ed field. Basically, we already have a 64-bit value reserved in the SevEsWork area= =20 and currently only one byte is used and second byte can be detected for=20 the TDX. Basically the which encryption technology is active the=20 definition of the work area will change. Am I missing something ? Thanks On 7/28/21 10:22 AM, Yao, Jiewen wrote: > Hi Brijesh > Thanks! >=20 > I think if we want to reuse this, we need rename the data structure. >=20 > First, we should use a generic name. >=20 > Second, I don=E2=80=99t think it is good idea to define two *enable* fie= lds. Since only one of them should be enabled, we should use 1 field as enu= meration. >=20 > Third, we should hide the SEV specific and TDX specific definition in CC= common work area. >=20 > If we agree to use a common work area, I recommend below: >=20 > typedef struct { > UINT8 HeaderVersion; // 0 > UINT8 HeadLength; // 4 > UINT8 Type; // 0 - legacy, 1 - SEV, 2 - TDX > UINT8 SubType; // Type specific sub type, if needed. > } CC_COMMON_WORK_AREA_HEADER; >=20 > typedef struct { > CC_COMMON_WORK_AREA_HEADER Header; > // reset is valid if Type =3D=3D 1 > UINT8 Reserved1[4]; > UINT64 RandomData; > UINT64 EncryptionMask; > } SEC_SEV_ES_WORK_AREA; >=20 > typedef struct { > CC_COMMON_WORK_AREA_HEADER Header; > // reset is valid if Type =3D=3D 2 > UINT8 TdxSpecific[]; // TBD > } TDX_WORK_AREA; >=20 > Thank you > Yao Jiewen >=20 >> -----Original Message----- >> From: devel@edk2.groups.io On Behalf Of Brijesh >> Singh via groups.io >> Sent: Wednesday, July 28, 2021 10:34 PM >> To: Yao, Jiewen ; Xu, Min M >> Cc: brijesh.singh@amd.com; devel@edk2.groups.io; Ard Biesheuvel >> ; Justen, Jordan L ; >> Erdem Aktas ; James Bottomley >> ; Tom Lendacky >> Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in >> ResetVector >> >> Hi Jiewen and Min, >> >> See my comments below. >> >> >> On 7/28/21 2:54 AM, Yao, Jiewen wrote: >>> Yes. I am thinking the same thing. >>> >>> [CC Flag memory location] >>> 1) A general purpose register, such as EBP. >>> >>> 2) A global variable, such as >>> .data >>> TeeFlags: DD 0 >>> >>> 3) A fixed region in stack, such as >>> dword[STACK_TOP - 4] >>> >>> 4) A new CC common fixed region, such as >>> dword[CC_COMMON_FLAGS] >>> >>> 5) A fixed region piggyback on existing CC working area, such as >>> dword[CC_WORKING_AREA] >>> >>> Hi Brijesh/Min >>> Any preference? >>> >>> [CC Indicator Flags] >>> Proposal: UINT8[4] >>> >>> Byte [0] Version: 0 >>> byte [1] Length: 4 >>> byte [2] Type: >>> =090: legacy >>> =091: SEV >>> =092: TDX >>> byte [3] Sub Type: >>> =09If Type is 0 (legacy), then >>> =09 0: legacy >>> =09If Type is 1 (SEV), then >>> =09 0: SEV >>> =09 1: SEV-ES >>> =09 2: SEV-SNP >>> =09If Type is 2 (TDX), then >>> =09 0: TDX 1.0 >>> >>> Thank you >>> Yao Jiewen >>> >>> >>>> -----Original Message----- >>>> From: Xu, Min M >>>> Sent: Wednesday, July 28, 2021 2:58 PM >>>> To: Yao, Jiewen >>>> Cc: Brijesh Singh ; devel@edk2.groups.io; Ard >>>> Biesheuvel ; Justen, Jordan L >>>> ; Erdem Aktas ; >> James >>>> Bottomley ; Tom Lendacky >> >>>> Subject: RE: [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector >>>> >>>> On July 28, 2021 2:05 PM, Yao, Jiewen wrote: >>>>> It does not necessary to be a working area. >>>>> >>>>> We just need a common TEE flag to indicate if the system run in lega= cy, SEV, >>>> or >>>>> TDX, right? >>>> Right. We need somewhere to store this flag, either in a Register or = in >> Memory. >>>> If it is memory, then in Tdx the memory region should be initialized = by host >> VMM. >>>>> >>>>> thank you! >>>>> Yao, Jiewen >>>>> >>>>> >>>>>> =E5=9C=A8 2021=E5=B9=B47=E6=9C=8828=E6=97=A5=EF=BC=8C=E4=B8=8B=E5= =8D=881:07=EF=BC=8CXu, Min M =E5=86=99 >> =E9=81=93=EF=BC=9A >>>>>> >>>>>> =EF=BB=BFOn July 27, 2021 8:46 PM, Yao, Jiewen wrote: >>>>>>> HI Min >>>>>>> I agree with Brijesh. >>>>>>> >>>>>>> The basic rule is: SEV file shall never refer to TDX data structur= e. >>>>>>> TDX file shall never refer to SEV data structure. >>>>>>> These code should be isolated clearly. >>>>>>> >>>>>>> Do we still need that logic if we follow the new pattern? >>>>>> I have replied to Brijesh's mail about the concern of the new patte= rn. >>>>>> >>>>>> I have some concern in the current pattern: >>>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>>> OneTimeCall PreMainFunctionHookSev >>>>>> OneTimeCall PreMainFunctionHookTdx >>>>>> MainFunction: >>>>>> XXXXXX >>>>>> OneTimeCall PostMainFunctionHookSev >>>>>> OneTimeCall PostMainFunctionHookTdx >>>>>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D >>>>>> The TEE function need implement a TEE check function (such as IsSev= , or >>>> IsTdx). >>>>>> >>>>>> Tdx call CPUID(0x21) to determine if it is tdx guest in the very >>>>>> beginning of ResetVector. Then 'TDXG' is set in TDX_WORK_AREA. SEV >> does >>>>> the similar work which call CheckSevFeatures to set SEV_ES_WORK_AREA= to >> 1. >>>>>> >>>>>> After that both TDX and SEV read the above WORK_AREA to check if it= is >> TDX >>>>> or SEV or legacy guest. >>>>>> >>>>>> In Tdx the access to SEV_ES_WORK_AREA will trigger error because >>>>> SEV_ES_WORK_AREA is *NOT* initialized by host VMM. >>>>>> In SEV-SNP I am afraid the access to TDX_WORK_AREA will trigger err= or >> too. >>>>>> >>>>>> I am wondering if TDX and SEV can use the same memory region (for >>>> example, >>>>> TEE_WORK_AREA) as the work area? >>>>>> So that this work area is guaranteed to be initialized in both TDX = and >>>>>> SEV. Structure of the TEE_WORK_AREA may look like this: >>>>>> typedef struct { >>>>>> UINT8 Flag[4]; 'TDXG' or 'SEVG' or all-0 >>>>>> UINT8 Others[]; >>>>>> } TEE_WORK_AREA; >>>>>>> >> >> Are we reserving a new page for the TDX_WORK_AREA ? I am wondering why >> can't we use the SEV_ES_WORK_AREA instead of wasting space in the MEMFD= . >> >> The SEV_ES_WORK_AREA layout looks like this: >> >> typedef struct _SEC_SEV_ES_WORK_AREA { >> UINT8 SevEsEnabled; >> UINT8 Reserved1[7]; >> >> UINT64 RandomData; >> >> UINT64 EncryptionMask; >> } SEC_SEV_ES_WORK_AREA; >> >> There is reserved bit after the SevEsEnabled and one byte can be used >> for the TdxEnabled; >> >> typedef struct _SEC_SEV_ES_WORK_AREA { >> UINT8 SevEsEnabled; >> UINT8 TdxEnabled; >> UINT8 Reserved2[6]; >> >> UINT64 RandomData; >> >> UINT64 EncryptionMask; >> } SEC_SEV_ES_WORK_AREA; >> >> The SEV_ES_WORK_AREA can be treated as a TEE_WORK_AREA and we can be >> pull out from MemEncrypSevLib.h to CcWorkAreaLib.h and rename the >> structure (if needed). >> >> Both the SEV-SNP and TEE host-VMM accepts the TEE_WORK_AREA before >> booting the guest to ensure that its safe to access the memory without >> going through the accept/validation process. >> >> In case of the TDX, the reset vector code sets the TdxEnabled on the >> entry. In case of the SEV, the workarea is valid from SEC to PEI phase >> only and it gets reused for other purposes. The PEI phase set the Pcd's >> (such as SevEsEnabled or SevEnabled etc) so that Dxe or other EDK2 core >> does not need to know anything about the workarea and they simply can >> read the PCDs. >> >> -Brijesh >> >> >>=20 >> >=20