BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198

An SEV-ES guest will generate a #VC exception when it encounters a
non-automatic exit (NAE) event. It is expected that the #VC exception
handler will communicate with the hypervisor using the GHCB to handle
the NAE event.

NAE events can occur during the Sec phase, so initialize exception
handling early in the OVMF Sec support.

Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 OvmfPkg/Sec/SecMain.inf |  1 +
 OvmfPkg/Sec/SecMain.c   | 29 ++++++++++++++++-------------
 2 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
index 63ba4cb555fb..7f53845f5436 100644
--- a/OvmfPkg/Sec/SecMain.inf
+++ b/OvmfPkg/Sec/SecMain.inf
@@ -50,6 +50,7 @@ [LibraryClasses]
   PeCoffExtraActionLib
   ExtractGuidedSectionLib
   LocalApicLib
+  CpuExceptionHandlerLib
 
 [Ppis]
   gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
index bae9764577f0..db319030ee58 100644
--- a/OvmfPkg/Sec/SecMain.c
+++ b/OvmfPkg/Sec/SecMain.c
@@ -24,6 +24,7 @@
 #include <Library/PeCoffExtraActionLib.h>
 #include <Library/ExtractGuidedSectionLib.h>
 #include <Library/LocalApicLib.h>
+#include <Library/CpuExceptionHandlerLib.h>
 
 #include <Ppi/TemporaryRamSupport.h>
 
@@ -737,6 +738,21 @@ SecCoreStartupWithStack (
     Table[Index] = 0;
   }
 
+  //
+  // Initialize IDT
+  //
+  IdtTableInStack.PeiService = NULL;
+  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
+    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
+  }
+
+  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
+  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
+
+  AsmWriteIdtr (&IdtDescriptor);
+
+  InitializeCpuExceptionHandlers (NULL);
+
   ProcessLibraryConstructorList (NULL, NULL);
 
   DEBUG ((EFI_D_INFO,
@@ -751,19 +767,6 @@ SecCoreStartupWithStack (
   //
   InitializeFloatingPointUnits ();
 
-  //
-  // Initialize IDT
-  //
-  IdtTableInStack.PeiService = NULL;
-  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
-    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
-  }
-
-  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
-  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
-
-  AsmWriteIdtr (&IdtDescriptor);
-
 #if defined (MDE_CPU_X64)
   //
   // ASSERT that the Page Tables were set by the reset vector code to
-- 
2.17.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#50970): https://edk2.groups.io/g/devel/message/50970
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/20/19 21:06, Lendacky, Thomas wrote:
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
> 
> An SEV-ES guest will generate a #VC exception when it encounters a
> non-automatic exit (NAE) event. It is expected that the #VC exception
> handler will communicate with the hypervisor using the GHCB to handle
> the NAE event.
> 
> NAE events can occur during the Sec phase, so initialize exception
> handling early in the OVMF Sec support.
> 
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/Sec/SecMain.inf |  1 +
>  OvmfPkg/Sec/SecMain.c   | 29 ++++++++++++++++-------------
>  2 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
> index 63ba4cb555fb..7f53845f5436 100644
> --- a/OvmfPkg/Sec/SecMain.inf
> +++ b/OvmfPkg/Sec/SecMain.inf
> @@ -50,6 +50,7 @@ [LibraryClasses]
>    PeCoffExtraActionLib
>    ExtractGuidedSectionLib
>    LocalApicLib
> +  CpuExceptionHandlerLib
>  
>  [Ppis]
>    gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
> index bae9764577f0..db319030ee58 100644
> --- a/OvmfPkg/Sec/SecMain.c
> +++ b/OvmfPkg/Sec/SecMain.c
> @@ -24,6 +24,7 @@
>  #include <Library/PeCoffExtraActionLib.h>
>  #include <Library/ExtractGuidedSectionLib.h>
>  #include <Library/LocalApicLib.h>
> +#include <Library/CpuExceptionHandlerLib.h>
>  
>  #include <Ppi/TemporaryRamSupport.h>
>  
> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>      Table[Index] = 0;
>    }
>  
> +  //
> +  // Initialize IDT
> +  //
> +  IdtTableInStack.PeiService = NULL;
> +  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
> +    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
> +  }
> +
> +  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
> +  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
> +
> +  AsmWriteIdtr (&IdtDescriptor);
> +
> +  InitializeCpuExceptionHandlers (NULL);
> +
>    ProcessLibraryConstructorList (NULL, NULL);
>  
>    DEBUG ((EFI_D_INFO,

(1) The problem here is that we call multiple library APIs before
calling ProcessLibraryConstructorList() -- namely CopyMem(),
AsmWriteIdtr(), and InitializeCpuExceptionHandlers().

(See also the "SetMem" reference in the leading context, in the source
file -- it is not quoted in this patch.)

Thus, would it be possible to move all the "+" lines, quoted above, just
below the ProcessLibraryConstructorList() call?


(2) If possible I'd like to restrict the
InitializeCpuExceptionHandlers() call to SevEsIsEnabled().

(Unless you're implying that InitializeCpuExceptionHandlers() is useful
even without SEV-ES -- but then the commit message should be reworded
accordingly.)

If you agree to that restriction, then I further suggest reordering this
patch against the next one:

  [edk2-devel] [RFC PATCH v3 31/43]
  OvmfPkg/Sec: Enable cache early to speed up booting

Namely, if you put that patch first, then in the present patch you can
extend the already existing SevEsIsEnabled()-dependent scope, with a
call to InitializeCpuExceptionHandlers().

The end result would be something like:

  ProcessLibraryConstructorList();

  //
  // Initialize IDT
  // ...
  //

  if (SevEsIsEnabled()) {
    //
    // non-automatic exit events (NAE) can occur during SEC ...
    //
    InitializeCpuExceptionHandlers (NULL);

    //
    // Under SEV-ES, the hypervisor can't modify CR0 ...
    //
    AsmEnableCache ();
  }

What's your opinion?

Thanks!
Laszlo

> @@ -751,19 +767,6 @@ SecCoreStartupWithStack (
>    //
>    InitializeFloatingPointUnits ();
>  
> -  //
> -  // Initialize IDT
> -  //
> -  IdtTableInStack.PeiService = NULL;
> -  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
> -    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
> -  }
> -
> -  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
> -  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
> -
> -  AsmWriteIdtr (&IdtDescriptor);
> -
>  #if defined (MDE_CPU_X64)
>    //
>    // ASSERT that the Page Tables were set by the reset vector code to
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51052): https://edk2.groups.io/g/devel/message/51052
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/21/19 6:06 AM, Laszlo Ersek wrote:
> On 11/20/19 21:06, Lendacky, Thomas wrote:
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>>
>> An SEV-ES guest will generate a #VC exception when it encounters a
>> non-automatic exit (NAE) event. It is expected that the #VC exception
>> handler will communicate with the hypervisor using the GHCB to handle
>> the NAE event.
>>
>> NAE events can occur during the Sec phase, so initialize exception
>> handling early in the OVMF Sec support.
>>
>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>  OvmfPkg/Sec/SecMain.inf |  1 +
>>  OvmfPkg/Sec/SecMain.c   | 29 ++++++++++++++++-------------
>>  2 files changed, 17 insertions(+), 13 deletions(-)
>>
>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
>> index 63ba4cb555fb..7f53845f5436 100644
>> --- a/OvmfPkg/Sec/SecMain.inf
>> +++ b/OvmfPkg/Sec/SecMain.inf
>> @@ -50,6 +50,7 @@ [LibraryClasses]
>>    PeCoffExtraActionLib
>>    ExtractGuidedSectionLib
>>    LocalApicLib
>> +  CpuExceptionHandlerLib
>>  
>>  [Ppis]
>>    gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>> index bae9764577f0..db319030ee58 100644
>> --- a/OvmfPkg/Sec/SecMain.c
>> +++ b/OvmfPkg/Sec/SecMain.c
>> @@ -24,6 +24,7 @@
>>  #include <Library/PeCoffExtraActionLib.h>
>>  #include <Library/ExtractGuidedSectionLib.h>
>>  #include <Library/LocalApicLib.h>
>> +#include <Library/CpuExceptionHandlerLib.h>
>>  
>>  #include <Ppi/TemporaryRamSupport.h>
>>  
>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>>      Table[Index] = 0;
>>    }
>>  
>> +  //
>> +  // Initialize IDT
>> +  //
>> +  IdtTableInStack.PeiService = NULL;
>> +  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>> +    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
>> +  }
>> +
>> +  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>> +  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
>> +
>> +  AsmWriteIdtr (&IdtDescriptor);
>> +
>> +  InitializeCpuExceptionHandlers (NULL);
>> +
>>    ProcessLibraryConstructorList (NULL, NULL);
>>  
>>    DEBUG ((EFI_D_INFO,
> 
> (1) The problem here is that we call multiple library APIs before
> calling ProcessLibraryConstructorList() -- namely CopyMem(),
> AsmWriteIdtr(), and InitializeCpuExceptionHandlers().
> 
> (See also the "SetMem" reference in the leading context, in the source
> file -- it is not quoted in this patch.)
> 
> Thus, would it be possible to move all the "+" lines, quoted above, just
> below the ProcessLibraryConstructorList() call?

Unfortunately, I can't. The invocation of ProcessLibraryConstructorList()
results in #VC exceptions and so the exception handler needs to be in
place before invoking ProcessLibraryConstructorList(). It looks like there
are some SerialPort I/O writes to initialize the serial port and some PCI
I/O reads and writes from AcpiTimerLibConstructor() in
OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c.

> 
> 
> (2) If possible I'd like to restrict the
> InitializeCpuExceptionHandlers() call to SevEsIsEnabled().
> 
> (Unless you're implying that InitializeCpuExceptionHandlers() is useful
> even without SEV-ES -- but then the commit message should be reworded
> accordingly.)

It does give you earlier exception handling and displays the exception
information should an exception occur during SEC.

But, it might require some tricks to somehow communicate from the
ResetVector code to the SecMain code that SEV-ES is enabled. This is
because you need to do a CPUID instruction to determine if SEV-ES is
enabled, which will generate a #VC, which requires an exception handler...

Thanks,
Tom

> 
> If you agree to that restriction, then I further suggest reordering this
> patch against the next one:
> 
>   [edk2-devel] [RFC PATCH v3 31/43]
>   OvmfPkg/Sec: Enable cache early to speed up booting
> 
> Namely, if you put that patch first, then in the present patch you can
> extend the already existing SevEsIsEnabled()-dependent scope, with a
> call to InitializeCpuExceptionHandlers().
> 
> The end result would be something like:
> 
>   ProcessLibraryConstructorList();
> 
>   //
>   // Initialize IDT
>   // ...
>   //
> 
>   if (SevEsIsEnabled()) {
>     //
>     // non-automatic exit events (NAE) can occur during SEC ...
>     //
>     InitializeCpuExceptionHandlers (NULL);
> 
>     //
>     // Under SEV-ES, the hypervisor can't modify CR0 ...
>     //
>     AsmEnableCache ();
>   }
> 
> What's your opinion?
> 
> Thanks!
> Laszlo
> 
>> @@ -751,19 +767,6 @@ SecCoreStartupWithStack (
>>    //
>>    InitializeFloatingPointUnits ();
>>  
>> -  //
>> -  // Initialize IDT
>> -  //
>> -  IdtTableInStack.PeiService = NULL;
>> -  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>> -    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
>> -  }
>> -
>> -  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>> -  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
>> -
>> -  AsmWriteIdtr (&IdtDescriptor);
>> -
>>  #if defined (MDE_CPU_X64)
>>    //
>>    // ASSERT that the Page Tables were set by the reset vector code to
>>
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51112): https://edk2.groups.io/g/devel/message/51112
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/21/19 21:46, Tom Lendacky wrote:
> On 11/21/19 6:06 AM, Laszlo Ersek wrote:
>> On 11/20/19 21:06, Lendacky, Thomas wrote:
>>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
>>>
>>> An SEV-ES guest will generate a #VC exception when it encounters a
>>> non-automatic exit (NAE) event. It is expected that the #VC exception
>>> handler will communicate with the hypervisor using the GHCB to handle
>>> the NAE event.
>>>
>>> NAE events can occur during the Sec phase, so initialize exception
>>> handling early in the OVMF Sec support.
>>>
>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>> ---
>>>  OvmfPkg/Sec/SecMain.inf |  1 +
>>>  OvmfPkg/Sec/SecMain.c   | 29 ++++++++++++++++-------------
>>>  2 files changed, 17 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
>>> index 63ba4cb555fb..7f53845f5436 100644
>>> --- a/OvmfPkg/Sec/SecMain.inf
>>> +++ b/OvmfPkg/Sec/SecMain.inf
>>> @@ -50,6 +50,7 @@ [LibraryClasses]
>>>    PeCoffExtraActionLib
>>>    ExtractGuidedSectionLib
>>>    LocalApicLib
>>> +  CpuExceptionHandlerLib
>>>
>>>  [Ppis]
>>>    gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
>>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>>> index bae9764577f0..db319030ee58 100644
>>> --- a/OvmfPkg/Sec/SecMain.c
>>> +++ b/OvmfPkg/Sec/SecMain.c
>>> @@ -24,6 +24,7 @@
>>>  #include <Library/PeCoffExtraActionLib.h>
>>>  #include <Library/ExtractGuidedSectionLib.h>
>>>  #include <Library/LocalApicLib.h>
>>> +#include <Library/CpuExceptionHandlerLib.h>
>>>
>>>  #include <Ppi/TemporaryRamSupport.h>
>>>
>>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>>>      Table[Index] = 0;
>>>    }
>>>
>>> +  //
>>> +  // Initialize IDT
>>> +  //
>>> +  IdtTableInStack.PeiService = NULL;
>>> +  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>> +    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
>>> +  }
>>> +
>>> +  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>>> +  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
>>> +
>>> +  AsmWriteIdtr (&IdtDescriptor);
>>> +
>>> +  InitializeCpuExceptionHandlers (NULL);
>>> +
>>>    ProcessLibraryConstructorList (NULL, NULL);
>>>
>>>    DEBUG ((EFI_D_INFO,
>>
>> (1) The problem here is that we call multiple library APIs before
>> calling ProcessLibraryConstructorList() -- namely CopyMem(),
>> AsmWriteIdtr(), and InitializeCpuExceptionHandlers().
>>
>> (See also the "SetMem" reference in the leading context, in the
>> source file -- it is not quoted in this patch.)
>>
>> Thus, would it be possible to move all the "+" lines, quoted above,
>> just below the ProcessLibraryConstructorList() call?
>
> Unfortunately, I can't. The invocation of
> ProcessLibraryConstructorList() results in #VC exceptions and so the
> exception handler needs to be in place before invoking
> ProcessLibraryConstructorList(). It looks like there are some
> SerialPort I/O writes to initialize the serial port and some PCI I/O
> reads and writes from AcpiTimerLibConstructor() in
> OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c.

I have to accept what you're saying, but this makes the code quite
brittle. It's a tenet that we don't call library APIs before the library
constructor had a chance to initialize whatever memory or hardware the
library APIs rely on.

So, in this case,

(a) please add a comment above this block that we're making an exception
with CopyMem(), AsmWriteIdtr() and InitializeCpuExceptionHandlers(),

(b) I'd still like to see this pre-constructor logic restricted to
SEV-ES. (More on that below.)

So something like:

  if (SevEs) {
    //
    // We have to initialize the IDT and set up exception handlers here,
    // i.e. before calling library constructors, because those library
    // constructors may access hardware such that #VC exceptions are
    // triggered.
    //
    // Due to this code executing before library constructors, *all*
    // library API calls are theoretically interface contract
    // violations. However, because we are in SEC (executing in flash),
    // those constructors cannot write variables with static storage
    // duration anyway. Furthermore, we call a small, restricted set of
    // APIs, such as CopyMem(), AsmWriteIdtr(),
    // InitializeCpuExceptionHandlers(), where we require that the
    // underlying library instance not trigger any #VC exceptions.
    //
    InitIdt ();
    InitExceptionHandlers ();
  }
  ProcessLibraryConstructorList ();
  if (!SevEs) {
    InitIdt ();
  }

This makes me feel safer because:
- we're explicit about the principles we (have to) break,
- even our limited assumptions are restricted to SEV-ES.

>
>>
>>
>> (2) If possible I'd like to restrict the
>> InitializeCpuExceptionHandlers() call to SevEsIsEnabled().
>>
>> (Unless you're implying that InitializeCpuExceptionHandlers() is
>> useful even without SEV-ES -- but then the commit message should be
>> reworded accordingly.)
>
> It does give you earlier exception handling and displays the exception
> information should an exception occur during SEC.
>
> But, it might require some tricks to somehow communicate from the
> ResetVector code to the SecMain code that SEV-ES is enabled. This is
> because you need to do a CPUID instruction to determine if SEV-ES is
> enabled, which will generate a #VC, which requires an exception
> handler...

So even *checking* whether SEV-ES is enabled requires a #VC handler to
be set up. Thanks for the reminder. How about this idea: in

  [edk2-devel] [RFC PATCH v3 37/43]
  OvmfPkg: Reserve a page in memory for the SEV-ES AP reset vector

you are carving out a page at PcdSevEsResetRipBase -- only in
"OvmfPkgX64.fdf". And, a very small (leading) stretch of that page is
used as the SEV_ES_AP_JMP_FAR structure.

Now, could we implement the following?

(1) Append a UINT8 ("BOOLEAN") field to the SEV_ES_AP_JMP_FAR structure
(and possibly rename the structure),

(2) in the OvmfPkgX64 reset vector, where you determine SEV-ES anyway,
explicitly set this field to zero if SEV-ES is disabled, and set it to
one, if SEV-ES is enabled,

(3) In OvmfPkg/Sec, introduce a new (local) header file declaring the
function SevEsIsEnabled(),

(4) Provide two C-language implementations (under the Ia32 and X64
directories): in the 32-bit version, return constant FALSE; in the
64-bit version, return the value of the new field. Something like:

  return ((SEV_ES_AP_JMP_FAR *)FixedPcdGet32 (PcdSevEsResetRipBase))->SevEsEnabled;

FixedPcdGet32() is explicitly safe to use without library constructors
having run.

Does this look viable? (It might require you to reshuffle patch 37 vs.
patch 30.)

Thanks!
Laszlo

>
> Thanks,
> Tom
>
>>
>> If you agree to that restriction, then I further suggest reordering this
>> patch against the next one:
>>
>>   [edk2-devel] [RFC PATCH v3 31/43]
>>   OvmfPkg/Sec: Enable cache early to speed up booting
>>
>> Namely, if you put that patch first, then in the present patch you can
>> extend the already existing SevEsIsEnabled()-dependent scope, with a
>> call to InitializeCpuExceptionHandlers().
>>
>> The end result would be something like:
>>
>>   ProcessLibraryConstructorList();
>>
>>   //
>>   // Initialize IDT
>>   // ...
>>   //
>>
>>   if (SevEsIsEnabled()) {
>>     //
>>     // non-automatic exit events (NAE) can occur during SEC ...
>>     //
>>     InitializeCpuExceptionHandlers (NULL);
>>
>>     //
>>     // Under SEV-ES, the hypervisor can't modify CR0 ...
>>     //
>>     AsmEnableCache ();
>>   }
>>
>> What's your opinion?
>>
>> Thanks!
>> Laszlo
>>
>>> @@ -751,19 +767,6 @@ SecCoreStartupWithStack (
>>>    //
>>>    InitializeFloatingPointUnits ();
>>>
>>> -  //
>>> -  // Initialize IDT
>>> -  //
>>> -  IdtTableInStack.PeiService = NULL;
>>> -  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>> -    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
>>> -  }
>>> -
>>> -  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>>> -  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
>>> -
>>> -  AsmWriteIdtr (&IdtDescriptor);
>>> -
>>>  #if defined (MDE_CPU_X64)
>>>    //
>>>    // ASSERT that the Page Tables were set by the reset vector code to
>>>
>>
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51189): https://edk2.groups.io/g/devel/message/51189
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/22/19 6:52 AM, Laszlo Ersek wrote:
> On 11/21/19 21:46, Tom Lendacky wrote:
>> On 11/21/19 6:06 AM, Laszlo Ersek wrote:
>>> On 11/20/19 21:06, Lendacky, Thomas wrote:
>>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2198&data=02%7C01%7Cthomas.lendacky%40amd.com%7C86943210aff44681d5b908d76f4acf49%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637100239406633853&sdata=8APpMBall%2F2urZh6V3kpuWpsBjOh8GVqhWGNBjL%2B30U%3D&reserved=0
>>>>
>>>> An SEV-ES guest will generate a #VC exception when it encounters a
>>>> non-automatic exit (NAE) event. It is expected that the #VC exception
>>>> handler will communicate with the hypervisor using the GHCB to handle
>>>> the NAE event.
>>>>
>>>> NAE events can occur during the Sec phase, so initialize exception
>>>> handling early in the OVMF Sec support.
>>>>
>>>> Cc: Jordan Justen <jordan.l.justen@intel.com>
>>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>>> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>>>> ---
>>>>   OvmfPkg/Sec/SecMain.inf |  1 +
>>>>   OvmfPkg/Sec/SecMain.c   | 29 ++++++++++++++++-------------
>>>>   2 files changed, 17 insertions(+), 13 deletions(-)
>>>>
>>>> diff --git a/OvmfPkg/Sec/SecMain.inf b/OvmfPkg/Sec/SecMain.inf
>>>> index 63ba4cb555fb..7f53845f5436 100644
>>>> --- a/OvmfPkg/Sec/SecMain.inf
>>>> +++ b/OvmfPkg/Sec/SecMain.inf
>>>> @@ -50,6 +50,7 @@ [LibraryClasses]
>>>>     PeCoffExtraActionLib
>>>>     ExtractGuidedSectionLib
>>>>     LocalApicLib
>>>> +  CpuExceptionHandlerLib
>>>>
>>>>   [Ppis]
>>>>     gEfiTemporaryRamSupportPpiGuid                # PPI ALWAYS_PRODUCED
>>>> diff --git a/OvmfPkg/Sec/SecMain.c b/OvmfPkg/Sec/SecMain.c
>>>> index bae9764577f0..db319030ee58 100644
>>>> --- a/OvmfPkg/Sec/SecMain.c
>>>> +++ b/OvmfPkg/Sec/SecMain.c
>>>> @@ -24,6 +24,7 @@
>>>>   #include <Library/PeCoffExtraActionLib.h>
>>>>   #include <Library/ExtractGuidedSectionLib.h>
>>>>   #include <Library/LocalApicLib.h>
>>>> +#include <Library/CpuExceptionHandlerLib.h>
>>>>
>>>>   #include <Ppi/TemporaryRamSupport.h>
>>>>
>>>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>>>>       Table[Index] = 0;
>>>>     }
>>>>
>>>> +  //
>>>> +  // Initialize IDT
>>>> +  //
>>>> +  IdtTableInStack.PeiService = NULL;
>>>> +  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>>> +    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
>>>> +  }
>>>> +
>>>> +  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>>>> +  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
>>>> +
>>>> +  AsmWriteIdtr (&IdtDescriptor);
>>>> +
>>>> +  InitializeCpuExceptionHandlers (NULL);
>>>> +
>>>>     ProcessLibraryConstructorList (NULL, NULL);
>>>>
>>>>     DEBUG ((EFI_D_INFO,
>>>
>>> (1) The problem here is that we call multiple library APIs before
>>> calling ProcessLibraryConstructorList() -- namely CopyMem(),
>>> AsmWriteIdtr(), and InitializeCpuExceptionHandlers().

We can reduce this exposure a bit and replace the CopyMem() call with
something similar to the loop above it.  I could also use assembler
code directly in here to load the IDTR.

That would leave just InitializeCpuExceptionHandlers(). Is there
something that can added so as to warn when a library has a
CONSTRUCTOR added to/part of the definition?

>>>
>>> (See also the "SetMem" reference in the leading context, in the
>>> source file -- it is not quoted in this patch.)
>>>
>>> Thus, would it be possible to move all the "+" lines, quoted above,
>>> just below the ProcessLibraryConstructorList() call?
>>
>> Unfortunately, I can't. The invocation of
>> ProcessLibraryConstructorList() results in #VC exceptions and so the
>> exception handler needs to be in place before invoking
>> ProcessLibraryConstructorList(). It looks like there are some
>> SerialPort I/O writes to initialize the serial port and some PCI I/O
>> reads and writes from AcpiTimerLibConstructor() in
>> OvmfPkg/Library/AcpiTimerLib/BaseRomAcpiTimerLib.c.
> 
> I have to accept what you're saying, but this makes the code quite
> brittle. It's a tenet that we don't call library APIs before the library
> constructor had a chance to initialize whatever memory or hardware the
> library APIs rely on.
> 
> So, in this case,
> 
> (a) please add a comment above this block that we're making an exception
> with CopyMem(), AsmWriteIdtr() and InitializeCpuExceptionHandlers(),

Can do.

> 
> (b) I'd still like to see this pre-constructor logic restricted to
> SEV-ES. (More on that below.)

Yup, propagating the SEV-ES status from the ResetVector seems to be the
way to go.

> 
> So something like:
> 
>    if (SevEs) {
>      //
>      // We have to initialize the IDT and set up exception handlers here,
>      // i.e. before calling library constructors, because those library
>      // constructors may access hardware such that #VC exceptions are
>      // triggered.
>      //
>      // Due to this code executing before library constructors, *all*
>      // library API calls are theoretically interface contract
>      // violations. However, because we are in SEC (executing in flash),
>      // those constructors cannot write variables with static storage
>      // duration anyway. Furthermore, we call a small, restricted set of
>      // APIs, such as CopyMem(), AsmWriteIdtr(),
>      // InitializeCpuExceptionHandlers(), where we require that the
>      // underlying library instance not trigger any #VC exceptions.
>      //
>      InitIdt ();
>      InitExceptionHandlers ();
>    }
>    ProcessLibraryConstructorList ();
>    if (!SevEs) {
>      InitIdt ();
>    }
> 
> This makes me feel safer because:
> - we're explicit about the principles we (have to) break,
> - even our limited assumptions are restricted to SEV-ES.
> 
>>
>>>
>>>
>>> (2) If possible I'd like to restrict the
>>> InitializeCpuExceptionHandlers() call to SevEsIsEnabled().
>>>
>>> (Unless you're implying that InitializeCpuExceptionHandlers() is
>>> useful even without SEV-ES -- but then the commit message should be
>>> reworded accordingly.)
>>
>> It does give you earlier exception handling and displays the exception
>> information should an exception occur during SEC.
>>
>> But, it might require some tricks to somehow communicate from the
>> ResetVector code to the SecMain code that SEV-ES is enabled. This is
>> because you need to do a CPUID instruction to determine if SEV-ES is
>> enabled, which will generate a #VC, which requires an exception
>> handler...
> 
> So even *checking* whether SEV-ES is enabled requires a #VC handler to
> be set up. Thanks for the reminder. How about this idea: in
> 
>    [edk2-devel] [RFC PATCH v3 37/43]
>    OvmfPkg: Reserve a page in memory for the SEV-ES AP reset vector
> 
> you are carving out a page at PcdSevEsResetRipBase -- only in
> "OvmfPkgX64.fdf". And, a very small (leading) stretch of that page is
> used as the SEV_ES_AP_JMP_FAR structure.
> 
> Now, could we implement the following?
> 
> (1) Append a UINT8 ("BOOLEAN") field to the SEV_ES_AP_JMP_FAR structure
> (and possibly rename the structure),
> 
> (2) in the OvmfPkgX64 reset vector, where you determine SEV-ES anyway,
> explicitly set this field to zero if SEV-ES is disabled, and set it to
> one, if SEV-ES is enabled,
> 
> (3) In OvmfPkg/Sec, introduce a new (local) header file declaring the
> function SevEsIsEnabled(),
> 
> (4) Provide two C-language implementations (under the Ia32 and X64
> directories): in the 32-bit version, return constant FALSE; in the
> 64-bit version, return the value of the new field. Something like:
> 
>    return ((SEV_ES_AP_JMP_FAR *)FixedPcdGet32 (PcdSevEsResetRipBase))->SevEsEnabled;
> 
> FixedPcdGet32() is explicitly safe to use without library constructors
> having run.
> 
> Does this look viable? (It might require you to reshuffle patch 37 vs.
> patch 30.)

I think this does. Since this is SEC and the reset vector page isn't
needed until PEI and later we could even just use the first byte (make a
union with an SEC usage field) and make this even simpler. Then we don't
have to worry about positioning it. Let me work on that and see where I
get. Anything after the #VC is established would use the current method
of determine SEV-ES status.

Thanks!
Tom

> 
> Thanks!
> Laszlo
> 
>>
>> Thanks,
>> Tom
>>
>>>
>>> If you agree to that restriction, then I further suggest reordering this
>>> patch against the next one:
>>>
>>>    [edk2-devel] [RFC PATCH v3 31/43]
>>>    OvmfPkg/Sec: Enable cache early to speed up booting
>>>
>>> Namely, if you put that patch first, then in the present patch you can
>>> extend the already existing SevEsIsEnabled()-dependent scope, with a
>>> call to InitializeCpuExceptionHandlers().
>>>
>>> The end result would be something like:
>>>
>>>    ProcessLibraryConstructorList();
>>>
>>>    //
>>>    // Initialize IDT
>>>    // ...
>>>    //
>>>
>>>    if (SevEsIsEnabled()) {
>>>      //
>>>      // non-automatic exit events (NAE) can occur during SEC ...
>>>      //
>>>      InitializeCpuExceptionHandlers (NULL);
>>>
>>>      //
>>>      // Under SEV-ES, the hypervisor can't modify CR0 ...
>>>      //
>>>      AsmEnableCache ();
>>>    }
>>>
>>> What's your opinion?
>>>
>>> Thanks!
>>> Laszlo
>>>
>>>> @@ -751,19 +767,6 @@ SecCoreStartupWithStack (
>>>>     //
>>>>     InitializeFloatingPointUnits ();
>>>>
>>>> -  //
>>>> -  // Initialize IDT
>>>> -  //
>>>> -  IdtTableInStack.PeiService = NULL;
>>>> -  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>>> -    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate, sizeof (mIdtEntryTemplate));
>>>> -  }
>>>> -
>>>> -  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>>>> -  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable) - 1);
>>>> -
>>>> -  AsmWriteIdtr (&IdtDescriptor);
>>>> -
>>>>   #if defined (MDE_CPU_X64)
>>>>     //
>>>>     // ASSERT that the Page Tables were set by the reset vector code to
>>>>
>>>
>>
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51195): https://edk2.groups.io/g/devel/message/51195
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/22/19 17:30, Tom Lendacky wrote:
> On 11/22/19 6:52 AM, Laszlo Ersek wrote:
>> On 11/21/19 21:46, Tom Lendacky wrote:
>>> On 11/21/19 6:06 AM, Laszlo Ersek wrote:
>>>> On 11/20/19 21:06, Lendacky, Thomas wrote:

>>>>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>>>>>       Table[Index] = 0;
>>>>>     }
>>>>>
>>>>> +  //
>>>>> +  // Initialize IDT
>>>>> +  //
>>>>> +  IdtTableInStack.PeiService = NULL;
>>>>> +  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>>>> +    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate,
>>>>> sizeof (mIdtEntryTemplate));
>>>>> +  }
>>>>> +
>>>>> +  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>>>>> +  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable)
>>>>> - 1);
>>>>> +
>>>>> +  AsmWriteIdtr (&IdtDescriptor);
>>>>> +
>>>>> +  InitializeCpuExceptionHandlers (NULL);
>>>>> +
>>>>>     ProcessLibraryConstructorList (NULL, NULL);
>>>>>
>>>>>     DEBUG ((EFI_D_INFO,
>>>>
>>>> (1) The problem here is that we call multiple library APIs before
>>>> calling ProcessLibraryConstructorList() -- namely CopyMem(),
>>>> AsmWriteIdtr(), and InitializeCpuExceptionHandlers().
> 
> We can reduce this exposure a bit and replace the CopyMem() call with
> something similar to the loop above it.

That would be nice, if you're not too annoyed by the extra busywork.

> I could also use assembler code directly in here to load the IDTR.

I think it would be enough to copy

  MdePkg/Library/BaseLib/Ia32/WriteIdtr.nasm
  MdePkg/Library/BaseLib/X64/WriteIdtr.nasm

under OvmfPkg/Sec, and use a new function name.

> That would leave just InitializeCpuExceptionHandlers(). Is there
> something that can added so as to warn when a library has a
> CONSTRUCTOR added to/part of the definition?

Nothing comes to my mind :(

[...]

>> (1) Append a UINT8 ("BOOLEAN") field to the SEV_ES_AP_JMP_FAR structure
>> (and possibly rename the structure),
>>
>> (2) in the OvmfPkgX64 reset vector, where you determine SEV-ES anyway,
>> explicitly set this field to zero if SEV-ES is disabled, and set it to
>> one, if SEV-ES is enabled,
>>
>> (3) In OvmfPkg/Sec, introduce a new (local) header file declaring the
>> function SevEsIsEnabled(),
>>
>> (4) Provide two C-language implementations (under the Ia32 and X64
>> directories): in the 32-bit version, return constant FALSE; in the
>> 64-bit version, return the value of the new field. Something like:
>>
>>    return ((SEV_ES_AP_JMP_FAR *)FixedPcdGet32
>> (PcdSevEsResetRipBase))->SevEsEnabled;
>>
>> FixedPcdGet32() is explicitly safe to use without library constructors
>> having run.
>>
>> Does this look viable? (It might require you to reshuffle patch 37 vs.
>> patch 30.)
> 
> I think this does. Since this is SEC and the reset vector page isn't
> needed until PEI and later we could even just use the first byte (make a
> union with an SEC usage field) and make this even simpler. Then we don't
> have to worry about positioning it.

Ah, nice.

> Let me work on that and see where I
> get. Anything after the #VC is established would use the current method
> of determine SEV-ES status.

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51199): https://edk2.groups.io/g/devel/message/51199
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

On 11/22/19 3:10 PM, Laszlo Ersek wrote:
> On 11/22/19 17:30, Tom Lendacky wrote:
>> On 11/22/19 6:52 AM, Laszlo Ersek wrote:
>>> On 11/21/19 21:46, Tom Lendacky wrote:
>>>> On 11/21/19 6:06 AM, Laszlo Ersek wrote:
>>>>> On 11/20/19 21:06, Lendacky, Thomas wrote:
> 
>>>>>> @@ -737,6 +738,21 @@ SecCoreStartupWithStack (
>>>>>>        Table[Index] = 0;
>>>>>>      }
>>>>>>
>>>>>> +  //
>>>>>> +  // Initialize IDT
>>>>>> +  //
>>>>>> +  IdtTableInStack.PeiService = NULL;
>>>>>> +  for (Index = 0; Index < SEC_IDT_ENTRY_COUNT; Index ++) {
>>>>>> +    CopyMem (&IdtTableInStack.IdtTable[Index], &mIdtEntryTemplate,
>>>>>> sizeof (mIdtEntryTemplate));
>>>>>> +  }
>>>>>> +
>>>>>> +  IdtDescriptor.Base  = (UINTN)&IdtTableInStack.IdtTable;
>>>>>> +  IdtDescriptor.Limit = (UINT16)(sizeof (IdtTableInStack.IdtTable)
>>>>>> - 1);
>>>>>> +
>>>>>> +  AsmWriteIdtr (&IdtDescriptor);
>>>>>> +
>>>>>> +  InitializeCpuExceptionHandlers (NULL);
>>>>>> +
>>>>>>      ProcessLibraryConstructorList (NULL, NULL);
>>>>>>
>>>>>>      DEBUG ((EFI_D_INFO,
>>>>>
>>>>> (1) The problem here is that we call multiple library APIs before
>>>>> calling ProcessLibraryConstructorList() -- namely CopyMem(),
>>>>> AsmWriteIdtr(), and InitializeCpuExceptionHandlers().
>>
>> We can reduce this exposure a bit and replace the CopyMem() call with
>> something similar to the loop above it.
> 
> That would be nice, if you're not too annoyed by the extra busywork.
> 
>> I could also use assembler code directly in here to load the IDTR.
> 
> I think it would be enough to copy
> 
>    MdePkg/Library/BaseLib/Ia32/WriteIdtr.nasm
>    MdePkg/Library/BaseLib/X64/WriteIdtr.nasm
> 
> under OvmfPkg/Sec, and use a new function name.
> 
>> That would leave just InitializeCpuExceptionHandlers(). Is there
>> something that can added so as to warn when a library has a
>> CONSTRUCTOR added to/part of the definition?
> 
> Nothing comes to my mind :(
> 
> [...]
> 
>>> (1) Append a UINT8 ("BOOLEAN") field to the SEV_ES_AP_JMP_FAR structure
>>> (and possibly rename the structure),
>>>
>>> (2) in the OvmfPkgX64 reset vector, where you determine SEV-ES anyway,
>>> explicitly set this field to zero if SEV-ES is disabled, and set it to
>>> one, if SEV-ES is enabled,
>>>
>>> (3) In OvmfPkg/Sec, introduce a new (local) header file declaring the
>>> function SevEsIsEnabled(),
>>>
>>> (4) Provide two C-language implementations (under the Ia32 and X64
>>> directories): in the 32-bit version, return constant FALSE; in the
>>> 64-bit version, return the value of the new field. Something like:
>>>
>>>     return ((SEV_ES_AP_JMP_FAR *)FixedPcdGet32
>>> (PcdSevEsResetRipBase))->SevEsEnabled;
>>>
>>> FixedPcdGet32() is explicitly safe to use without library constructors
>>> having run.
>>>
>>> Does this look viable? (It might require you to reshuffle patch 37 vs.
>>> patch 30.)
>>
>> I think this does. Since this is SEC and the reset vector page isn't
>> needed until PEI and later we could even just use the first byte (make a
>> union with an SEC usage field) and make this even simpler. Then we don't
>> have to worry about positioning it.
> 
> Ah, nice.

I haven't done the WriteIdtr() stuff, yet, but the passing of the SEV-ES
status from the ResetVector code to the SEC code via the SEV-ES page at
0x0080_B000 is working nicely.

Thanks,
Tom

> 
>> Let me work on that and see where I
>> get. Anything after the #VC is established would use the current method
>> of determine SEV-ES status.
> 
> Thanks!
> Laszlo
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#51200): https://edk2.groups.io/g/devel/message/51200
Mute This Topic: https://groups.io/mt/60973137/1787277
Mute #vc: https://groups.io/mk?hashtag=vc&subid=3901457
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-