Re: [edk2-devel] [PATCH v10 06/46] MdePkg/BaseLib: Add support for the XGETBV instruction

["Lendacky, Thomas" <thomas.lendacky@amd.com>]

On 7/15/20 11:17 AM, Tom Lendacky wrote:
> On 7/15/20 10:55 AM, Laszlo Ersek wrote:
>> Hi Tom,
>>
>> On 07/14/20 16:27, Lendacky, Thomas wrote:
>>> From: Tom Lendacky <thomas.lendacky@amd.com>
>>>
>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C00c7f97271bf4efd050608d828d79a13%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637304253757199823&amp;sdata=hQ%2BP%2F5DintrtGi66sfqRI%2B9h4V1rTB1YlYdY1VX6sLM%3D&amp;reserved=0
>>>
>>> Under SEV-ES, a CPUID instruction requires the current value of the XCR0
>>> register. In order to retrieve that value, the XGETBV instruction needs
>>> to be executed.
>>>
>>> Provide the necessary support to execute the XGETBV instruction.
>>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>> ---
>>>   MdePkg/Library/BaseLib/BaseLib.inf      |  2 ++
>>>   MdePkg/Include/Library/BaseLib.h        | 17 +++++++++++++
>>>   MdePkg/Library/BaseLib/Ia32/GccInline.c | 28 ++++++++++++++++++++
>>>   MdePkg/Library/BaseLib/X64/GccInline.c  | 30 ++++++++++++++++++++++
>>>   MdePkg/Library/BaseLib/Ia32/XGetBv.nasm | 31 ++++++++++++++++++++++
>>>   MdePkg/Library/BaseLib/X64/XGetBv.nasm  | 34 +++++++++++++++++++++++++
>>>   6 files changed, 142 insertions(+)
>>>   create mode 100644 MdePkg/Library/BaseLib/Ia32/XGetBv.nasm
>>>   create mode 100644 MdePkg/Library/BaseLib/X64/XGetBv.nasm
>>
>> This patch no longer applies, it conflicts with commit d3c9e40abcc8
>> ("MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions",
>> 2020-07-15).
> 
> Ok, that looks like it landed just after I sent my patches.
> 
>>
>> The conflict persists if I successfully apply the series on c7195b9ec3c5
>> -- just preceding Mike's series "[Patch v3 00/16] UnitTestFrameworkPkg:
>> Enhancements and bug fixes" --, and then try to rebase to current master
>> (e77966b341b9) from there.
>>
>> Can you please investigate?
> 
> Yup, I'll take a look.

Ok, after rebasing, the GccInline.c split caused the conflict and has been 
resolved.

There was also an added PCD that conflicted with one of mine that needed 
updating.

I'll send out a new version today or Monday.

Thanks,
Tom

> 
> Thanks,
> Tom
> 
>>
>> Thanks,
>> Laszlo
>>
>>>
>>> diff --git a/MdePkg/Library/BaseLib/BaseLib.inf b/MdePkg/Library/BaseLib/BaseLib.inf
>>> index a57ae2da31f3..da6bc22a3e2b 100644
>>> --- a/MdePkg/Library/BaseLib/BaseLib.inf
>>> +++ b/MdePkg/Library/BaseLib/BaseLib.inf
>>> @@ -153,6 +153,7 @@ [Sources.Ia32]
>>>     Ia32/ARShiftU64.c | MSFT
>>>     Ia32/EnableCache.c | MSFT
>>>     Ia32/DisableCache.c | MSFT
>>> +  Ia32/XGetBv.nasm | MSFT
>>>   
>>>   
>>>     Ia32/GccInline.c | GCC
>>> @@ -287,6 +288,7 @@ [Sources.X64]
>>>     X64/ReadCr2.nasm| MSFT
>>>     X64/ReadCr0.nasm| MSFT
>>>     X64/ReadEflags.nasm| MSFT
>>> +  X64/XGetBv.nasm | MSFT
>>>   
>>>   
>>>     X64/Non-existing.c
>>> diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
>>> index 8e7b87cbda4e..7edf0051a0a0 100644
>>> --- a/MdePkg/Include/Library/BaseLib.h
>>> +++ b/MdePkg/Include/Library/BaseLib.h
>>> @@ -7831,6 +7831,23 @@ AsmLfence (
>>>     VOID
>>>     );
>>>   
>>> +/**
>>> +  Executes a XGETBV instruction
>>> +
>>> +  Executes a XGETBV instruction. This function is only available on IA-32 and
>>> +  x64.
>>> +
>>> +  @param[in] Index        Extended control register index
>>> +
>>> +  @return                 The current value of the extended control register
>>> +**/
>>> +UINT64
>>> +EFIAPI
>>> +AsmXGetBv (
>>> +  IN UINT32  Index
>>> +  );
>>> +
>>> +
>>>   /**
>>>     Patch the immediate operand of an IA32 or X64 instruction such that the byte,
>>>     word, dword or qword operand is encoded at the end of the instruction's
>>> diff --git a/MdePkg/Library/BaseLib/Ia32/GccInline.c b/MdePkg/Library/BaseLib/Ia32/GccInline.c
>>> index 5287200f8754..c962bcfa4617 100644
>>> --- a/MdePkg/Library/BaseLib/Ia32/GccInline.c
>>> +++ b/MdePkg/Library/BaseLib/Ia32/GccInline.c
>>> @@ -1763,3 +1763,31 @@ AsmFlushCacheLine (
>>>   }
>>>   
>>>   
>>> +/**
>>> +  Executes a XGETBV instruction
>>> +
>>> +  Executes a XGETBV instruction. This function is only available on IA-32 and
>>> +  x64.
>>> +
>>> +  @param[in] Index        Extended control register index
>>> +
>>> +  @return                 The current value of the extended control register
>>> +**/
>>> +UINT64
>>> +EFIAPI
>>> +AsmXGetBv (
>>> +  IN UINT32 Index
>>> +  )
>>> +{
>>> +  UINT64 Data;
>>> +
>>> +  __asm__ __volatile__ (
>>> +    "xgetbv"
>>> +    : "=A" (Data)
>>> +    : "c"  (Index)
>>> +    );
>>> +
>>> +  return Data;
>>> +}
>>> +
>>> +
>>> diff --git a/MdePkg/Library/BaseLib/X64/GccInline.c b/MdePkg/Library/BaseLib/X64/GccInline.c
>>> index 154ce1f57e92..915555198f9f 100644
>>> --- a/MdePkg/Library/BaseLib/X64/GccInline.c
>>> +++ b/MdePkg/Library/BaseLib/X64/GccInline.c
>>> @@ -1798,3 +1798,33 @@ AsmFlushCacheLine (
>>>   }
>>>   
>>>   
>>> +/**
>>> +  Executes a XGETBV instruction
>>> +
>>> +  Executes a XGETBV instruction. This function is only available on IA-32 and
>>> +  x64.
>>> +
>>> +  @param[in] Index        Extended control register index
>>> +
>>> +  @return                 The current value of the extended control register
>>> +**/
>>> +UINT64
>>> +EFIAPI
>>> +AsmXGetBv (
>>> +  IN UINT32 Index
>>> +  )
>>> +{
>>> +  UINT32 LowData;
>>> +  UINT32 HighData;
>>> +
>>> +  __asm__ __volatile__ (
>>> +    "xgetbv"
>>> +    : "=a" (LowData),
>>> +      "=d" (HighData)
>>> +    : "c"  (Index)
>>> +    );
>>> +
>>> +  return (((UINT64)HighData) << 32) | LowData;
>>> +}
>>> +
>>> +
>>> diff --git a/MdePkg/Library/BaseLib/Ia32/XGetBv.nasm b/MdePkg/Library/BaseLib/Ia32/XGetBv.nasm
>>> new file mode 100644
>>> index 000000000000..9f7b03bbff35
>>> --- /dev/null
>>> +++ b/MdePkg/Library/BaseLib/Ia32/XGetBv.nasm
>>> @@ -0,0 +1,31 @@
>>> +;------------------------------------------------------------------------------
>>> +;
>>> +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
>>> +; SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +;
>>> +; Module Name:
>>> +;
>>> +;   XGetBv.Asm
>>> +;
>>> +; Abstract:
>>> +;
>>> +;   AsmXgetBv function
>>> +;
>>> +; Notes:
>>> +;
>>> +;------------------------------------------------------------------------------
>>> +
>>> +    SECTION .text
>>> +
>>> +;------------------------------------------------------------------------------
>>> +; UINT64
>>> +; EFIAPI
>>> +; AsmXGetBv (
>>> +;   IN UINT32  Index
>>> +;   );
>>> +;------------------------------------------------------------------------------
>>> +global ASM_PFX(AsmXGetBv)
>>> +ASM_PFX(AsmXGetBv):
>>> +    mov     ecx, [esp + 4]
>>> +    xgetbv
>>> +    ret
>>> diff --git a/MdePkg/Library/BaseLib/X64/XGetBv.nasm b/MdePkg/Library/BaseLib/X64/XGetBv.nasm
>>> new file mode 100644
>>> index 000000000000..09f3be8ae0a8
>>> --- /dev/null
>>> +++ b/MdePkg/Library/BaseLib/X64/XGetBv.nasm
>>> @@ -0,0 +1,34 @@
>>> +;------------------------------------------------------------------------------
>>> +;
>>> +; Copyright (C) 2020, Advanced Micro Devices, Inc. All rights reserved.<BR>
>>> +; SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +;
>>> +; Module Name:
>>> +;
>>> +;   XGetBv.Asm
>>> +;
>>> +; Abstract:
>>> +;
>>> +;   AsmXgetBv function
>>> +;
>>> +; Notes:
>>> +;
>>> +;------------------------------------------------------------------------------
>>> +
>>> +    DEFAULT REL
>>> +    SECTION .text
>>> +
>>> +;------------------------------------------------------------------------------
>>> +; UINT64
>>> +; EFIAPI
>>> +; AsmXGetBv (
>>> +;   IN UINT32  Index
>>> +;   );
>>> +;------------------------------------------------------------------------------
>>> +global ASM_PFX(AsmXGetBv)
>>> +ASM_PFX(AsmXGetBv):
>>> +    xgetbv
>>> +    shl     rdx, 32
>>> +    or      rax, rdx
>>> +    ret
>>> +
>>>
>>