From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-CO1-obe.outbound.protection.outlook.com (NAM11-CO1-obe.outbound.protection.outlook.com [40.107.220.59]) by mx.groups.io with SMTP id smtpd.web09.763.1620747827607115924 for ; Tue, 11 May 2021 08:43:47 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=m13VXpDx; 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.220.59, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KJW01oRdzvw2Tk/QSYG/3a4Tjr17nw04+m5F9Ww8Te7cbZ90t3T7rL67bBzExqXlzEM0CwTRi7w6JEwoPgbQ1gcHwOCCIHa5v8wLWfQH914vV2AKqfLX2WAdBIG66YROel1J8074u0/IGqNpcENXvu+1PPivOXBVolpG5W3iKqAT36aAROdPOwayQLsJ7g7WxwVQf1g/TSsF/7/D014i2/CsH4D3MEfkbm84t65PYBLp/rHXbc8a8YCs1U+fcHLIp+dpR3yJj25m5cD2mUXlotTa35SAXb62A6V/Rdc/ulX8SsHxI3i3a9yJigMdRDufXdZXKGoIbeYXEWChzZTlTg== 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=GGn3Rc9VmmDIFYgiGV1z0nNyHLJ0LWVUjeFnGaug3ek=; b=JnzdQXtbBtGpiGR39zvlU7Y6llS0RItEEc8EdNPa+koTYka5W3g5gsHrFiri2PStF7ZeesnGbOeI7l1Y7eZHlvjUxWB3OwwgmB09ZrPgFzkgAFA6XVfOH0uk6zeuH164Sv9cLlpCOqMlm5fzJbLe8N9V725W4gHldpIf3vR8uNfi6AuZr5vyVjEWmFaB16EiEEnDWVS7oEcJY/8VFj6LYVAbqCPkCr9npYOsxO1yaZbjo3ZdIcsoocfRTELak+DlrH6gnKf3UV1RShyH2Od4WFx3dM5V5Gi1Wh1YqDmB+32RIwuJZEcszStgGOhY2DJ84BngG3eriD+UvvHr19NQ9Q== 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=GGn3Rc9VmmDIFYgiGV1z0nNyHLJ0LWVUjeFnGaug3ek=; b=m13VXpDxcDwhUhV3z8WhVIt/ulfqNNnenaxClKJpapccfSRiE7YAmABI6BCWXPOW5XLJdXBuR1GfOAZkTcc5p8mus7ajRtGFtUOe/zMZ0IpJyfYGdo5e+aeHUf3MGxgmQzgqbvKA4Q/dxWdU9je0v6rh5o8UHRHMIRFNwuq4krQ= Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB4433.namprd12.prod.outlook.com (2603:10b6:5:2a1::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4108.24; Tue, 11 May 2021 15:43:46 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9%12]) with mapi id 15.20.4129.025; Tue, 11 May 2021 15:43:46 +0000 Subject: Re: [edk2-devel] [PATCH 06/13] MdePkg/Register/Amd: define GHCB macros for SNP AP creation To: Laszlo Ersek , devel@edk2.groups.io, brijesh.singh@amd.com Cc: James Bottomley , Min Xu , Jiewen Yao , Jordan Justen , Ard Biesheuvel , Erdem Aktas , Michael D Kinney , Liming Gao , Zhiguang Liu References: <20210507203838.23706-1-brijesh.singh@amd.com> <20210507203838.23706-7-brijesh.singh@amd.com> <541e97fd-7f49-1cd0-fa69-14dd58b2432b@redhat.com> From: "Lendacky, Thomas" Message-ID: <9d9d6def-5e15-6795-71cf-02c86279ef67@amd.com> Date: Tue, 11 May 2021 10:43:43 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: <541e97fd-7f49-1cd0-fa69-14dd58b2432b@redhat.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN4PR0601CA0003.namprd06.prod.outlook.com (2603:10b6:803:2f::13) 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 SN4PR0601CA0003.namprd06.prod.outlook.com (2603:10b6:803:2f::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4108.25 via Frontend Transport; Tue, 11 May 2021 15:43:45 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 9fbf5d20-f0e7-4e11-e94a-08d914939000 X-MS-TrafficTypeDiagnostic: DM6PR12MB4433: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: jp31qqgL8hzUGvO5G8s0VepukuFz/khl4U3mRkl9M1se8S++z8U+8I8lOHXyMCp0WIrlxVmJnmTeU7vfYl3iZGlGjWdEBZCHa4AiO8sHqxa9tfJoMto+bS1bcGLBfl53M8GojckNYxAK3ceHr91C5vg80Lv6NnLnC/sFioC6eic868IvzkWZ6FoQSX9OWHzK1ANFl7T6bMxT5atZEh1jWo5q+UK73DkSavTOfa9uEHEI+zM/k6du5/QXxtn4cyOHKB0pmjBoCOIzkz2donokjHsttklOMgHHzdFowf3TvQr0Vbozf3Wn07GE21BY8kZB4/Jn4xHJRkBz6ulzybHM+0sPKnfGv3SSlSdrBd4iCsXbSCjTvUru9s1H3vr8GGkud0ofa2KFEsJzY8lMLOxwcAkZmU4HgJV+sH7t4D4YQpdOQpUgs3nSZ+Jq1/TDWHzf9rZ6IXJvfGW83O0tP8msERhub5K08Rn3O4zk5SUYbt9LoZx+Z6hRyoxnYZWZdfRz1SBi4FJy91v9fCNqexuC80gHhr//oUJ0slgRlTe1IJ4BqxMVZxzOv7OfTbuaPg97Z46NK0k2UPBiR9pH2SspDINzkIApOCOXFGmNpSP+o1pJBRsSRV9bC2ySwhVrHjO8ZbPFRcAebRJ9+l+inBPA9CxZum66zi4X8WBBwvPgzpPmwyRbLXtBPmdzyXYQUWXiY0gY1lwZAviNU/Tq6+4J60xVgo/us4hs4Jo4LalHIU109VuA7hzOrnP47aYYvihk4vM+Pb/Ld6Xv1hA/8GhZAA== 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)(396003)(376002)(366004)(136003)(39860400002)(66946007)(16526019)(53546011)(478600001)(38100700002)(45080400002)(186003)(6506007)(36756003)(83380400001)(2616005)(956004)(6636002)(31686004)(8676002)(5660300002)(7416002)(6486002)(66476007)(6512007)(54906003)(2906002)(4326008)(31696002)(66556008)(26005)(316002)(8936002)(86362001)(966005)(45980500001)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?T3k5d3FHaDN5QVgzN3A4bGVuaFRYZ0MvN1RieHErZXp1RU04dFVBTTY4NVA1?= =?utf-8?B?S3NrNk1XSm5XdE4rRkhtOFNkZENUNEVYOVU1YzJnRitMSkRIWXdmN3JtTlR3?= =?utf-8?B?MkNvVjF4TnlyYmlJMStFRk44LzZmYmM5YXJEVVljTTB2elhuY2p3TmIvR3VC?= =?utf-8?B?Q3JyelpFV1hiUlM4M3NtVzFKL09ZdEtyWGIxSXo1ajFENDJmTlJ0NlBwYWNB?= =?utf-8?B?OHZlTGtHN3dCUXBRanpkSUQ5anpKUUhsV3lwOWRNalZTdUhEYng4YWFvR3lr?= =?utf-8?B?RmZYT2h6M1c2aVNZNHBEb1lxZWdSZTZnK1QyMjlSMDUrcG5PUEZMZnIzb3p2?= =?utf-8?B?TWVSL09MdW54RWIzZThUbHM0OGtOUDJTRWdjVTZaWFJtMnU2aUlxbTdVWVdY?= =?utf-8?B?UGxKVWVMdytiL2IrdEV3RDhjSjhlSFFSdDJCWmY1R2JkeGN3ZWErWFltcVZI?= =?utf-8?B?cm5YZi9Ob2Myc1B0Rm9GN0JjRnZmUDNrK2ZmdTZ0Y3gwNFpjL3FiTVB4VHky?= =?utf-8?B?bU0vNmJqQUNUcnpvVjdGTWJ4WFRmbnVXYUpZL043MnpmWDQrZXVZLy84b3Rj?= =?utf-8?B?cUR2ZDRXcEtpRm85eHFSNkJXcXJPaVRKeTA0dCsvdmxxWDZ3VXd1OTBXOHM3?= =?utf-8?B?OWFoSklvRXBIVk14Q3FSRUlXdVhNY0ZYY0dvUzFJN3pubkhkZHpYbzB6dmRz?= =?utf-8?B?RFRyaVU0K3dDQjdyRDlzRGZnTXJzM1hHeHZneGo1RThVS2lUdTBFd0JiR3hu?= =?utf-8?B?bmhBTHQxMTl2NHZKNVlpWmQydXMwSG5UdDZKSFdhZThFOXgzZGYwbEtWdGx0?= =?utf-8?B?UXF5SDJBd1FXTVpkOEhGLzBHZ1ZSUzBheWhPNGNLNDBHaHZJMWxyYmd2TUVE?= =?utf-8?B?ekkxNkVuc2piMFUreDBjR0hoNFNVQldlYUhiT2IxdjROaVp5TXR0R0lDNy9j?= =?utf-8?B?QmJqNTFycHp1RkhEcXRMWWord2tHYWhWZTlJWDV6cnNVYlB3RlVYc25JSWpF?= =?utf-8?B?YWlzSzlXM3MyL0pKSzBadHNLL243bDlXMGNTS3dGWUQ1bFFEcmo1cStqK20x?= =?utf-8?B?SjdQSFp6ZjhpWWJ5UzlhQ2FaRkxhR0pEaDkxdVZINElvc3ZFTUZEMHZKWUlG?= =?utf-8?B?QmlCZXkyS0JpaC93VVNrRjFFWDd3OThscEFZekVSd2I5R2RGTTBIcWZxajRZ?= =?utf-8?B?c0tNR0EvTmhQUVpJeDJLRWdGRkxpbGF3L0tIem80d3RDaWJyZHo1WE1mSnFt?= =?utf-8?B?RmlmSHBmUTRwMWJ0Q3hnT2xVR3ZxYVJab3BBOFBpU2hwVlBTYU9VYy9pU0I2?= =?utf-8?B?eXMrRUdtMjg3aFRWNmRSQ1JkcWVEbnhrL0Q3ZzNOQ1k0Ymg5RmtsZG5GbG1x?= =?utf-8?B?TGp2Y3I4NXk2WTNzblZ0SVNpYWphU3F0WGswRFpUVVBwcXRnQkhWVTBwWUZC?= =?utf-8?B?c20zTUQvbDM3MHlhc2tHWEd2cjRvcXdHSDlVcGxCR2psb0c0d1dMM01HSDQ4?= =?utf-8?B?UVd5aU96cUlGREZvSnMrWkFsTzBHNk1WMUFqNnVQWmdaLzZtNHljM2ZDMjhZ?= =?utf-8?B?d2F5WGFyUkx2QWZMMW45eStRNWhjVGh5TE8rQkxmZ0RITjMyR0k1QTBOanBj?= =?utf-8?B?bXEveDZLNThIVFZrUTBvMTJZTGtJYzcyWWNOZ3dOY1pPa2pUSzZFdkpzZzlZ?= =?utf-8?B?NzVzdFoxczNTOFRnTjE4RE5RNlVjY0JYSDlXQzNUck9sL09iVTZscm95cS9l?= =?utf-8?Q?vLlUzSu+MU0h9K/hOXz765P6KAHL2/LbNzB0Ode?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9fbf5d20-f0e7-4e11-e94a-08d914939000 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 May 2021 15:43:46.1291 (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: 4F/FCoqj9GmF+tmrFGVCB2NK3XH6TxpryUo+zzxjGpEV3vyUqEwJhCdSz49mQsncEc98YTpkglhrJ6RJhttVjw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4433 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 5/11/21 4:59 AM, Laszlo Ersek wrote: > On 05/07/21 22:38, Brijesh Singh wrote: >> From: Tom Lendacky >> >> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3275&data=04%7C01%7Cthomas.lendacky%40amd.com%7C92c1323bd1e84a2a38e208d914636ddf%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637563239563579592%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=DMDhcseilROSsL6EISUoT9p0pI%2BmXtEC3rLHIQS4NmI%3D&reserved=0 >> >> Version 2 of GHCB introduces NAE for creating AP when SEV-SNP is >> enabled in the guest VM. See the GHCB spec section for additional >> details. > > (1) The actual section reference is missing. I'll fix it up: from where > the spec introduces exit code 0x8000_0013, the sections appear to be > 4.1.9 and 4.3.2. Also, Table 5. "List of Supported Non-Automatic Events" > is relevant for the SVM_VMGEXIT_SNP_AP_* macros. There are some needed changes to this patch, so I can fix that up. I just avoided putting actual section numbers in there because it is possible that they can change in future versions. > >> >> While at it, define the VMSA state save area that are required for > > (2) I think "area" (singular) is correct here, so we should say "is". > I'll update it. I can fix that up. > >> creating the AP. The save area format is defined in AMD APM volume >> 2 (Table B-4). >> >> Cc: James Bottomley >> Cc: Min Xu >> Cc: Jiewen Yao >> Cc: Tom Lendacky >> Cc: Jordan Justen >> Cc: Ard Biesheuvel >> Cc: Laszlo Ersek >> Cc: Erdem Aktas >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> Signed-off-by: Tom Lendacky >> Signed-off-by: Brijesh Singh >> --- >> MdePkg/Include/Register/Amd/Ghcb.h | 70 ++++++++++++++++++++++++++++++ >> 1 file changed, 70 insertions(+) >> >> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h b/MdePkg/Include/Register/Amd/Ghcb.h >> index a15b4b7e2760..956cefbc003c 100644 >> --- a/MdePkg/Include/Register/Amd/Ghcb.h >> +++ b/MdePkg/Include/Register/Amd/Ghcb.h >> @@ -55,6 +55,7 @@ >> #define SVM_EXIT_AP_RESET_HOLD 0x80000004ULL >> #define SVM_EXIT_AP_JUMP_TABLE 0x80000005ULL >> #define SVM_EXIT_SNP_PAGE_STATE_CHANGE 0x80000010ULL >> +#define SVM_EXIT_SNP_AP_CREATION 0x80000013ULL >> #define SVM_EXIT_HYPERVISOR_FEATURES 0x8000FFFDULL >> #define SVM_EXIT_UNSUPPORTED 0x8000FFFFULL >> >> @@ -83,6 +84,12 @@ >> #define IOIO_SEG_ES 0 >> #define IOIO_SEG_DS (BIT11 | BIT10) >> >> +// >> +// AP Creation Information >> +// >> +#define SVM_VMGEXIT_SNP_AP_CREATE_ON_INIT 0 >> +#define SVM_VMGEXIT_SNP_AP_CREATE 1 >> +#define SVM_VMGEXIT_SNP_AP_DESTROY 2 >> >> typedef PACKED struct { >> UINT8 Reserved1[203]; >> @@ -195,4 +202,67 @@ typedef struct { >> SNP_PAGE_STATE_ENTRY Entry[SNP_PAGE_STATE_MAX_ENTRY]; >> } SNP_PAGE_STATE_CHANGE_INFO; >> >> +// >> +// SEV-ES save area mapping structures used for SEV-SNP AP Creation. >> +// Only the fields required to be set to a non-zero value are defined. >> +// >> +#pragma pack(1) >> +typedef struct { >> + UINT16 Selector; >> + UINT16 Attributes; >> + UINT32 Limit; >> + UINT64 Base; >> +} SEV_ES_SEGMENT_REGISTER; >> +#pragma pack() > > (3) there should be a space character between "pack" and "(" -- two > instances. Will do. > >> + >> +#define SEV_ES_RESET_CS_ATTRIBUTES (BIT7 | BIT4 | BIT3 | BIT1) >> +#define SEV_ES_RESET_DS_ATTRIBUTES (BIT7 | BIT4 | BIT1) >> +#define SEV_ES_RESET_ES_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES >> +#define SEV_ES_RESET_FS_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES >> +#define SEV_ES_RESET_GS_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES >> +#define SEV_ES_RESET_SS_ATTRIBUTES SEV_ES_RESET_DS_ATTRIBUTES >> + >> +#define SEV_ES_RESET_GDTR_ATTRIBUTES 0 >> +#define SEV_ES_RESET_LDTR_ATTRIBUTES (BIT7 | 2) >> +#define SEV_ES_RESET_IDTR_ATTRIBUTES 0 >> +#define SEV_ES_RESET_TR_ATTRIBUTES (BIT7 | 3) > > (4) ... I guess I can't go ahead merging this myself, after all (Liming > may of course still merge the MdePkg patches, if he wants to). > > My problem here is that the bit positions are cryptic. > > I've found the *normal* (not SEV-ES) segment descriptor attributes in > the AMD APM (publication #24593, revision 3.37, date March 2021, volume > 2, sections sections 4.7 and 4.8). > > However, the bit positions SEV-ES descriptors are surely different. For > the "normal" segment descriptors, we already have the > IA32_SEGMENT_DESCRIPTOR type in edk2, with the nicely broken-out > attribute bits. The bit meanings within > "SEV_ES_SEGMENT_REGISTER.Attributes" remain unclear to me. > > Please at least provide a *specific* documentation reference in the > commit message where I can verify (or at least "decode") the attribute bits. Yeah, it is a strange format. The format is documented in sections 15.5 (VMRUN Instruction) and 10 (System-Management Mode). I can try to further document the bit assignments, e.g. #define SEV_ES_SEGMENT_ATTRIBUTE_PRESENT BIT7 #define SEV_ES_SEGMENT_ATTRIBUTE_USER BIT4 etc. > >> + >> +#pragma pack(1) >> +typedef struct { >> + SEV_ES_SEGMENT_REGISTER Es; >> + SEV_ES_SEGMENT_REGISTER Cs; >> + SEV_ES_SEGMENT_REGISTER Ss; >> + SEV_ES_SEGMENT_REGISTER Ds; >> + SEV_ES_SEGMENT_REGISTER Fs; >> + SEV_ES_SEGMENT_REGISTER Gs; >> + SEV_ES_SEGMENT_REGISTER Gdtr; >> + SEV_ES_SEGMENT_REGISTER Ldtr; >> + SEV_ES_SEGMENT_REGISTER Idtr; >> + SEV_ES_SEGMENT_REGISTER Tr; >> + UINT8 Reserved1[42]; > > (5) This doesn't seem right. The comment higher up says that "Only the > fields required to be set to a non-zero value are defined", which is > fine. But in Table B-4, between fields "Tr" and "Vmpl", we have 5 qword > fields (PL0_SSP through PL3_SSP, plus U_CET), and a reserved dword > field. That makes for 5*8+4 = 44 bytes, not 42. > > Hmmm. If I look at the table differently, I get a different result. > Namely, the first offset right after Tr is 0A0h, and the start offset of > Vmpl is 0CAh. The difference is indeed 42 (decimal). > > Ah, I've found it. The bug is in the spec. The Reserved field at offset > 0C8h is listed with size "dword", but the field right after it, namely > VMPL, starts at offset 0CAh -- that is, only two bytes later. Which > means that the "dword" size for Reserved is wrong; it should be "word" only. Right, the offsets are correct, the use of "dword" is incorrect. > > I couldn't find a more recent release of the APM than "revision 3.37". > Please add a remark to the commit message on this patch that highlights > this typo in the spec. Will do. > >> + UINT8 Vmpl; >> + UINT8 Reserved2[5]; >> + UINT64 Efer; >> + UINT8 Reserved3[112]; > > OK > >> + UINT64 Cr4; >> + UINT8 Reserved4[8]; > > OK (although I'm not sure much is saved over spelling out "Cr3") > >> + UINT64 Cr0; >> + UINT64 Dr7; >> + UINT64 Dr6; >> + UINT64 Rflags; >> + UINT64 Rip; >> + UINT8 Reserved5[232]; > > OK > >> + UINT64 GPat; >> + UINT8 Reserved6[320]; > > OK > >> + UINT64 SevFeatures; >> + UINT8 Reserved7[48]; > > OK > >> + UINT64 XCr0; >> + UINT8 Reserved8[24]; > > OK > >> + UINT32 Mxcsr; >> + UINT64 X87Ftw; > > (6) If you are packing all four words (X87_FTW, X87_FSW, X87_FCW, > X87_FOP) into a qword, then please give the latter a different name. The > spec associates X87_FTW with just the word at offset 40Ch. I propose the > name "FpState" for the UINT64 field in the edk2 structure. Yes, that is incorrect. They should be UINT16 entries as you state below. > >> + UINT64 Reserved9[8]; >> + UINT64 X87Fcw; > > (7) Ugh, wait, something's wrong -- you *are* apparently adding a field > for X87_FCW separately! But then the type of X87Ftw is wrong -- it > should be UINT16. Same for X87Fcw. > > The distance between them is also wrong, it should be only 2 bytes > (basically the X87_FSW field). I have no idea at all where the 8*8=64 > bytes padding comes from! > >> +} SEV_ES_SAVE_AREA; >> +#pragma pack() > > (8) same as (3); please insert a space character between Will do. Thanks, Tom > >> + >> #endif >> > > Thanks > Laszlo >