Re: [edk2-devel] 回复: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field offsets and save area
Lendacky, Thomas
thomas.lendacky at amd.com
Mon Oct 19 12:50:56 UTC 2020
On 10/18/20 8:41 PM, gaoliming wrote:
> Tom:
>
>> -----邮件原件-----
>> 发件人: Tom Lendacky <thomas.lendacky at amd.com>
>> 发送时间: 2020年10月17日 0:09
>> 收件人: devel at edk2.groups.io
>> 抄送: Brijesh Singh <brijesh.singh at amd.com>; Michael D Kinney
>> <michael.d.kinney at intel.com>; Liming Gao <gaoliming at byosoft.com.cn>;
>> Zhiguang Liu <zhiguang.liu at intel.com>; Jordan Justen
>> <jordan.l.justen at intel.com>; Laszlo Ersek <lersek at redhat.com>; Ard
>> Biesheuvel <ard.biesheuvel at arm.com>
>> 主题: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field offsets and
>> save area
>>
>> From: Tom Lendacky <thomas.lendacky at amd.com>
>>
>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3008&data=04%7C01%7Cthomas.lendacky%40amd.com%7C4fd17a68a915477d9d1508d873d027b1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637386685165902990%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=NBPdQ98RyMUlma0qkEvhBjUdykQ24p7pTK9I61ttX7Q%3D&reserved=0
>>
>> Use OFFSET_OF () and sizeof () to calculate the GHCB register field
>> offsets instead of hardcoding the values in the GHCB_REGISTER enum.
>> Rename GHCB_REGISTER to GHCB_QWORD_OFFSET to more appropriately
>> describe
>> the enum. While redefining the values, only include (and add) fields that
>> are used per the GHCB specification.
>>
>> Also, remove the DR7 field from the GHCB_SAVE_AREA structure since it is
>> not used/defined in the GHCB specification and then rename the reserved
>> fields as appropriate.
>>
>> Cc: Michael D Kinney <michael.d.kinney at intel.com>
>> Cc: Liming Gao <gaoliming at byosoft.com.cn>
>> Cc: Zhiguang Liu <zhiguang.liu at intel.com>
>> Cc: Jordan Justen <jordan.l.justen at intel.com>
>> Cc: Laszlo Ersek <lersek at redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel at arm.com>
>> Cc: Tom Lendacky <thomas.lendacky at amd.com>
>> Cc: Brijesh Singh <brijesh.singh at amd.com>
>> Signed-off-by: Tom Lendacky <thomas.lendacky at amd.com>
>> ---
>> MdePkg/Include/Register/Amd/Ghcb.h | 40
>> +++++++-------------
>> OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 4 +-
>> 2 files changed, 16 insertions(+), 28 deletions(-)
>>
>
> Please separate the patch for the change in OvmfPkg.
> One commit can't cross the different packages.
> I understand this is the incompatible change. But, we still need to follow
> this rule.
Ok, then I'll resubmit without the name change. To me, it's not worth
doing 3 commits just to accomplish the rename from GHCB_REGISTER to
GHCB_QWORD_OFFSET.
Thanks,
Tom
>
>> diff --git a/MdePkg/Include/Register/Amd/Ghcb.h
>> b/MdePkg/Include/Register/Amd/Ghcb.h
>> index 54a80da0f6d7..93fb6e3cb0fc 100644
>> --- a/MdePkg/Include/Register/Amd/Ghcb.h
>> +++ b/MdePkg/Include/Register/Amd/Ghcb.h
>> @@ -82,35 +82,10 @@
>> #define IOIO_SEG_DS (BIT11 | BIT10)
>>
>>
>> -typedef enum {
>> - GhcbCpl = 25,
>> - GhcbRflags = 46,
>> - GhcbRip,
>> - GhcbRsp = 59,
>> - GhcbRax = 63,
>> - GhcbRcx = 97,
>> - GhcbRdx,
>> - GhcbRbx,
>> - GhcbRbp = 101,
>> - GhcbRsi,
>> - GhcbRdi,
>> - GhcbR8,
>> - GhcbR9,
>> - GhcbR10,
>> - GhcbR11,
>> - GhcbR12,
>> - GhcbR13,
>> - GhcbR14,
>> - GhcbR15,
>> - GhcbXCr0 = 125,
>> -} GHCB_REGISTER;
>> -
>> typedef PACKED struct {
>> UINT8 Reserved1[203];
>> UINT8 Cpl;
>> - UINT8 Reserved2[148];
>> - UINT64 Dr7;
>> - UINT8 Reserved3[144];
>> + UINT8 Reserved8[300];
>> UINT64 Rax;
>> UINT8 Reserved4[264];
>> UINT64 Rcx;
>> @@ -136,6 +111,19 @@ typedef PACKED struct {
>> UINT32 GhcbUsage;
>> } GHCB;
>>
>> +typedef enum {
>> + GhcbCpl = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),
>
> OFFSET_OF (GHCB, SaveArea.Cpl) is 204. But, it can't fully divide 8
> (sizeof(UINT64)). Is it correct?
>
> Thanks
> Liming
>> + GhcbRax = OFFSET_OF (GHCB, SaveArea.Rax) / sizeof
>> (UINT64),
>> + GhcbRbx = OFFSET_OF (GHCB, SaveArea.Rbx) / sizeof
>> (UINT64),
>> + GhcbRcx = OFFSET_OF (GHCB, SaveArea.Rcx) / sizeof
>> (UINT64),
>> + GhcbRdx = OFFSET_OF (GHCB, SaveArea.Rdx) / sizeof
>> (UINT64),
>> + GhcbXCr0 = OFFSET_OF (GHCB, SaveArea.XCr0) / sizeof
>> (UINT64),
>> + GhcbSwExitCode = OFFSET_OF (GHCB, SaveArea.SwExitCode) / sizeof
>> (UINT64),
>> + GhcbSwExitInfo1 = OFFSET_OF (GHCB, SaveArea.SwExitInfo1) / sizeof
>> (UINT64),
>> + GhcbSwExitInfo2 = OFFSET_OF (GHCB, SaveArea.SwExitInfo2) / sizeof
>> (UINT64),
>> + GhcbSwScratch = OFFSET_OF (GHCB, SaveArea.SwScratch) / sizeof
>> (UINT64),
>> +} GHCB_QWORD_OFFSET;
>> +
>> typedef union {
>> struct {
>> UINT32 Lower32Bits;
>> diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
>> b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
>> index 8e42b305e83c..c5484a3f478c 100644
>> --- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
>> +++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
>> @@ -153,7 +153,7 @@ STATIC
>> BOOLEAN
>> GhcbIsRegValid (
>> IN GHCB *Ghcb,
>> - IN GHCB_REGISTER Reg
>> + IN GHCB_QWORD_OFFSET Reg
>> )
>> {
>> UINT32 RegIndex;
>> @@ -179,7 +179,7 @@ STATIC
>> VOID
>> GhcbSetRegValid (
>> IN OUT GHCB *Ghcb,
>> - IN GHCB_REGISTER Reg
>> + IN GHCB_QWORD_OFFSET Reg
>> )
>> {
>> UINT32 RegIndex;
>> --
>> 2.28.0
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66387): https://edk2.groups.io/g/devel/message/66387
Mute This Topic: https://groups.io/mt/77650045/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list