From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (NAM10-BN7-obe.outbound.protection.outlook.com [40.107.92.63]) by mx.groups.io with SMTP id smtpd.web11.7503.1627553287584434679 for ; Thu, 29 Jul 2021 03:08:08 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amd.com header.s=selector1 header.b=c3pv6DaK; 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.92.63, mailfrom: brijesh.singh@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=RWGFLFuiHnEopSOIuHoyM6+foIB1mydEFOYs/kRkRMbwBm/3dC7RELWUoblrxs73Vg8CfNagwbXTIvGnn/aExMisI4Fqz60IMcyNRyNiIiikNi8/lkTIEtTrzWgw9wkfOGR3FU1IDChVaTBXRKZ/Qx66XRoAXGWB3eV/XPh1Zh7yeSBgpEKXABQ7Tg2Fy9Hq2xecZ2HAdbDYteVdn3QYkWyleHosk+6bFNp5Wlq2L6u+kk5EQFFFiKoaDLw7wNZzkA/qg72RtS/IribnRdCICrE80pQTkZd5zTDWgdNRSONMJw825/CVE0Urja9ubxQP49MbzhvOMciPljErsKBiJg== 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=IoCY4ofD6VzOG/VHHnDx5by75QcuAG4GFSHQbPE/u7Q=; b=WQmZTlmbLZEN7mML4nq/DXPBCsQNrKvnjZ5LcIirUe2yYpYfAhXB8IoRlKLOTVnCu1eWhgCpaIHsuiE1NKDuyQ+msXcMaSLf2YwoLPXXqRuyGO4ZLnP3zu3unUqBvViJMh2tXEgQ3fFQEcDn/Go2UpaH8MHPPKxethpZ0yx/qj9inSXP8NiN03aa4h6zvHy4IN/5rhv/ozXm0eP3ephKTlEtpL9quoeIjuphI4j72iOUFXRtWrPClquq3gvPnX1CbHaf4NpaIfPL7apwxeDMoFeTspiVVzpUssFyNryvuuUvXaAfsAQ96zeWOCKsuH4QYTeVvAMZnLbwEh5Mo4YeIg== 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=IoCY4ofD6VzOG/VHHnDx5by75QcuAG4GFSHQbPE/u7Q=; b=c3pv6DaKfIMM9QglyuQkkX7Egk2IG6gD4dKgva4mNJ5GXVtCQB85kRm7pI/THBsx3aeZcSe4ijVgkkPBGbmzsnDshb0hTU8ZDVRVz5DC6sGX98D47wexI9QsKDwDPrxC8ltVw8ZF+gBOOZ2fK2nRCO0WOXKI5cEuiAg71M/giTY= 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 SN6PR12MB2688.namprd12.prod.outlook.com (2603:10b6:805:6f::29) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4352.28; Thu, 29 Jul 2021 10:08:02 +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.021; Thu, 29 Jul 2021 10:08:02 +0000 Subject: Re: [edk2-devel] [PATCH V3 06/10] OvmfPkg: Add AmdSev.asm in ResetVector To: "Xu, Min M" , "Yao, Jiewen" , "devel@edk2.groups.io" CC: Ard Biesheuvel , "Justen, Jordan L" , Erdem Aktas , James Bottomley , Tom Lendacky References: <4E4F0C83-ED04-4CFC-BDE2-33825C106DB9@intel.com> <97ad36da-13cd-ccb1-48f3-17ee03934aa6@amd.com> <1e234d04-6348-5ac8-9c99-0557d6b44ef6@amd.com> From: "Brijesh Singh" Message-ID: <61da69ab-31fa-7179-53bf-0142badc5f9d@amd.com> Date: Thu, 29 Jul 2021 05:07:57 -0500 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 In-Reply-To: X-ClientProxiedBy: SA0PR12CA0016.namprd12.prod.outlook.com (2603:10b6:806:6f::21) 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 Brijeshs-MacBook-Pro.local (165.204.77.11) by SA0PR12CA0016.namprd12.prod.outlook.com (2603:10b6:806:6f::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.20 via Frontend Transport; Thu, 29 Jul 2021 10:07:59 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: a805a78d-29e6-4595-45ab-08d95278bffa X-MS-TrafficTypeDiagnostic: SN6PR12MB2688: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: /R3QkA/wgKpVq4JGWrR9mG8XuvIKtGa/B8OArt6azwMAbONS/QwoXoHL6Nic+yB3Vzt2OFAl1/IUdnmrbkhHw1Bzova5YbNa0R0E4W2EwGPs0qm81loiCwYScgBb7ncDuvcID/auPxMSAknsxjhEOuLMDAf4hny8MoXneDbdOvtJ09H3IryoQgO6XX7TwQa5lfbcn5UEyzrCac/213rQrMHi1decOZCp+I9YI83GVhkdETW3O/8VMbEr34HlhUdQKL6P7F6MeBYsOpmzHH9rP3QBxIyM9rKe6JAsBcPTnM3y3O60sFzYc/Q7yE0jWjCoVBz8wpGHycD9WcjbRuXLXYn6YwbqyoDn6uZnmgu6umtcPDvk1YBs8gtxJ8Mdw0ErrEZ+ZPsfMI946ScP/3utJlrB0tb8S4Bnz0Jo636H6Cvaj5q6gvHnaV8EtoY216S9iSDO8Gvyk2xz+F4rZY1dt6lR82cM45NCKXZrQiCYV302NnSLAWqTb3NGsS5WsAOQrIuIrPp7gEOZ2KhpEr4GEiekdoIQPG+OHdYS20XD7i/JdIg8QVG521OEgCfNVbxftdF587KKzuz0tFDWwP8+f4tkIcdJnwf7ZUE+Leobqbb+GH8FUkG4hfw40rMjFY367vMOZ4wtac1lHUqBGaDlVDyXTA04vq/4mAgnTGcIn6kiW3Vb/Pr2wWK3n6H74v0xBlKE2txWdlNCqmQTnbpCIQUURAiWmjmy1LjWSENmKrbCLnUH6NTs/bjqUYRVgBezDBQPNgb7PWQ9aoTMxlIQfQ== 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)(136003)(346002)(396003)(376002)(366004)(39860400002)(4326008)(956004)(66946007)(2616005)(66476007)(36756003)(66556008)(316002)(86362001)(44832011)(8936002)(8676002)(6512007)(31696002)(478600001)(54906003)(83380400001)(110136005)(5660300002)(38100700002)(38350700002)(6486002)(31686004)(52116002)(53546011)(6506007)(26005)(186003)(2906002)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?2gCHuwLjwyAtykogHARVWw2MKlhllXjUQDNDaMj10tDy8MrsrwZLK6TMlE1/?= =?us-ascii?Q?Mu7ePzfZSiawl+R5PIoilT3Orhq5gsBciCN/sWW+tlWkza/uN0206vIzQD50?= =?us-ascii?Q?jlsSaiQoM06ERQlbyyoYJU2aGdXmA/NPj4zhAdh+A5/GXXf8rjGs97QajJvC?= =?us-ascii?Q?pdZ+3wwNGPmSPLYnchSk+X6CzsgDU5bkzb2HE/fUt0Ev2AEpN7XmWgMiZ3sP?= =?us-ascii?Q?rPmwkNzY2U8gRPZzhLQxEy2+VCmY3UEC6J/8ZpkWHIdOhvr7UJ0p1yhMfGzG?= =?us-ascii?Q?wI6kQoAzLURtulyysgP8W1FFXkVXs8mOF2uHL0FHkoyfFFZoYPsQ9uxINSKX?= =?us-ascii?Q?+q2+mlYXU26cpJVXEULZO25OC5C/+6WigwbuUyAMBLzGT6E+rhlWFsGjqeqp?= =?us-ascii?Q?P53vIdORlkP+chBw5jqjq9DqNy18kvA+BYlb1Cgl6z6SvT4fNCV44h+lP7cm?= =?us-ascii?Q?OgHD0EYZxrljeFRjnYmPrzd3CdiBl3ac38RrilceqSL5/JGNLrs1T7e6q6XQ?= =?us-ascii?Q?MHQQqsB+L4eVnV59I5DS+StPR5iN3usWLQKfxoZzOQ+oA/GJ7pP/2Qrjv9/Y?= =?us-ascii?Q?JPr3Mj+RfWeMwJg2EoAW/2obzT+QKs2sMLMG0MgQBfLiLQBCyaZMaVFjsD5x?= =?us-ascii?Q?vf3P+qqxp0mMdro7TG/vsp05GxT1dJ7oMqlYYLF+XyzLTpHRgYlvKyLtregO?= =?us-ascii?Q?s8bzVDczIXS81aqCdZmYjjbbvD6gVLaHx9qSky5OMkfKNqqUEMRpx9GFI7Ae?= =?us-ascii?Q?/cLJkyE1Fclr9qBxH48Ihj/ImgwBfpFYVpk86yFdPe+g6WBWD0zfQptA0zB1?= =?us-ascii?Q?8PIiJxS4kjkfTU5wToAFKjdt/27M12K9sXTwbRtlan0Hjq7G76UvtzSCcybG?= =?us-ascii?Q?U9Q4kjlYRjPlvR0pqSQQp1oEqncKoQo+iMmRwjGQ8Cdn6+IBnX5JYLgk9OWe?= =?us-ascii?Q?JYKjU0UBvvWhM4PXg4//hfF/5wgVzy/+HJKoWYCpvLhA1OqJMfBgZyFQ6yWd?= =?us-ascii?Q?RHypIjD/sfJc3ICgpzrMUDgr9gfICKL6/zG81WHeAjVMGmC9Gb6/WLKl0ewc?= =?us-ascii?Q?rMrAdX26em41qQfDB+QBGEYj7uArM3jeM5WThU8vzsRNYpasd21TOK0TB59y?= =?us-ascii?Q?FWzl2tVOdzuvA34q1KgBKqnUXFGFVcBuCWq46putKYvWKuOkcGIYJTHmY9Oa?= =?us-ascii?Q?i1ifM5U2MqqaKhu6252GrMdOdhmkHL5EtEMXkd96SeH2kKD4jkL7KEDmF206?= =?us-ascii?Q?5x9tX3mFuTF4t3bpXbIsjK7YguhFBUM9Sqt4o2YYR37M1rUHwlWVkD9RaPOJ?= =?us-ascii?Q?EuY7lVdd3zP0B+xDURx5cCKB?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: a805a78d-29e6-4595-45ab-08d95278bffa X-MS-Exchange-CrossTenant-AuthSource: SN6PR12MB2718.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Jul 2021 10:08:02.2677 (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: rt+/KIikMq873xqRGuQEgz+BD/N1cahzOfVzq3ql52VAqp2+WhESZ5+RnvHROHh7d7WIOlEZ88HDi9exVUssxg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2688 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Content-Language: en-US On 7/29/21 1:07 AM, Xu, Min M wrote: > On July 29, 2021 12:29 PM, Brijesh Singh wrote: >> On 7/28/21 9:44 PM, Xu, Min M wrote: >>> Jiewen & Singh >>> >>> From the discussion I am thinking we have below rules to follow to the >>> design the structure of TEE_WORK_AREA: >>> 1. Design should be flexible but not too complicated 2. Reuse the >>> current SEV_ES_WORK_AREA (PcdSevEsWorkAreaBase) as TEE_WORK_AREA 3. >>> TEE_WORK_AREA should be initialized to all-0 at the beginning of >>> ResetVecotr 4. Reduce the changes to exiting code if possible >>> >>> So I try to make below conclusions below: (Please review) 1. >>> SEV_ES_WORK_AREA is used as the TEE_WORK_AREA by both TDX and SEV, >>> maybe in the future it can be used by other CC technologies. >>> >>> 2. In MEMFD, add below initial value. So that TEE_WORK_AREA is >>> guaranteed to be cleared in legacy guest. In TDX this memory region is >>> initialized to be all-0 by host VMM. In SEV the memory region is cleare= d as well. >>> 0x00B000|0x001000 >>> >> gUefiCpuPkgTokenSpaceGuid.PcdSevEsWorkAreaBase|gUefiCpuPkgTokenSpace >> Guid.PcdSevEsWorkAreaSize >>> DATA =3D { >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, >>> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 >>> } >> Hmm, I thought the contents of the data pages are controlled by the host= VMM. >> If the backing pages are not zero filled then there is no guarantee that= memory >> will be zero.=C2=A0 To verify it: >> >> 1. I applied your above change in OvmfPkgX86.fdt. I modified the DATA va= lues >> from 0x00 -> 0xCC >> >> 2. Modified the SecMain.c to dump the SevEsWorkArea on entry >> >> And dump does not contain the 0xcc. >> >> And to confirm further,=C2=A0 I attached to the qemu with the GDB before= the booting >> the OVMF, and modified the SevEsWorkArea with some garbage number=C2=A0 = and >> this time the dump printed garbage value I put through the debugger. >> >> In summary, the OVMF to zero the workarea memory on the entry and we >> cannot rely on the DATA=3D{0x00, 0x00...} to zero the CCWorkArea. > So in legacy guest, CCWorkArea is cleared to all-0 without the DATA=3D{0x= 00,0x00...}, right? Okay, maybe I was not able to communicate it correctly. The run I did is for the legacy guest. For the legacy guest, the contents of the CCWorkArea may *not* be always zero even when you use the DATA=3D{0x00, 0x00...}. Currently, Qemu uses zero filled backing pages, so we will get a zero filled CCWorkArea; but nothing says that a backing page *must* be zero. Another VMM may choose to do things differently. In summary, the OVMF reset vector code must zero=C2=A0 the CCWorkArea=C2=A0 before calling SEV o= r TDX probes. thanks > >> Did I miss something ? >> >> >>> 3. The structure of TEE_WORK_AREA >>> The current SEV_ES_WORK_AREA is defined as below: >>> typedef struct { >>> UINT8 SevEsEnabled; >>> UINT8 Reserved1[7]; >>> [Others...] >>> } SEC_SEV_ES_WORK_AREA; >>> >>> So I think the TEE_WORK_AREA can be: >>> Byte[0] Type: >>> 0: legacy 1: SEV 2: TDX >>> Byte[1] Subtype: >>> If Type is 0, then it is 0 >>> If Type is 1, then it is up to SEV's definition >>> If Type is 2, then it is up to TDX's definition Byte[] other bytes >>> are defined based on the Type/Subtype >> I personally like Yao Jiewen's struct definition, but I can also live wi= th this one as >> well :). The only question I had was with his proposal was what if we re= move the >> Length and Version fields. If the header length was fixed then life woul= d be much >> easier in the ASM code. > Yao Jiewen's structure is like below. If the HeaderVersion/HeaderLength a= re removed > you will find it is just what I am saying. The first 2 bytes are used to = distinguish the > legacy/SEV/TDX. The left bytes are up to the first 2 bytes. > 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; > > 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; > > typedef struct { > CC_COMMON_WORK_AREA_HEADER Header; > // reset is valid if Type =3D=3D 2 > UINT8 TdxSpecific[]; // TBD > } TDX_WORK_AREA; >> >>> I check the code in SecMain.c. >>> SevEsIsEnabled() need updated to check SevEsWorkarea->SevEsEnabled =3D= =3D 1, >> not non-0. >>> @Brijesh Singh Is there any other code need update? >> As noted before, the SevEsWorkAreas is used to pass the information from= the >> Sec to PEI phase. The workarea gets reused for totally different purpose= after >> the PEI phase. > So only the above line in SecMain.c/SevEsIsEnabled() need updated, right? > Thanks! > Xu, Min