On 05/11/21 17:43, Tom Lendacky wrote: > On 5/11/21 4:59 AM, Laszlo Ersek wrote: >> On 05/07/21 22:38, Brijesh Singh wrote: >>> From: Tom Lendacky <thomas.lenda...@amd.com> >>> >>> 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.
As long as AMD keeps older revisions of the spec available for download, I think it's fine to include precise references (covering the spec revision number too). >>> +#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. If it's not a big burden, can you please do both? I.e., (a) include the spec reference(s) in the commit message, and (b) introduce either bit-fields, or symbolic names (macros), for the relevant bits? Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#75096): https://edk2.groups.io/g/devel/message/75096 Mute This Topic: https://groups.io/mt/82665185/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
