Strange behavior between GCC 11 and GCC 12

Strange behavior between GCC 11 and GCC 12

[Lendacky, Thomas]

I've been trying to debug a problem I'm seeing when I moved to the GCC 12
compiler. Under SEV it results in the guest crashing.

I narrowed the issue down to the call to TemporaryRamMigration() in
PeiCheckAndSwitchStack() of MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c.

I get this output on GCC11:
   Old Stack size 32768, New stack size 131072
   Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
   Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
   *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
   TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
   *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=3BF95D20

and everything is good.

However, I get this output on GCC12:
   Old Stack size 32768, New stack size 131072
   Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
   Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
   *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
   TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
   *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=7770BD20
   MMIO using encrypted memory: 7770BD48
   !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!

and terminate because SecCoreData has been corrupted and points to an
address in an MMIO range (this is an SEV-ES/SEV-SNP example).

As near as I can tell from looking at the object code, on GCC12 it looks
like the SecCoreData value is stored in the RBP register, which appears to
be getting corrupted when calling TemporaryRamMigration().

Does anyone have any thoughts on this?

Thanks,
Tom

Re: Strange behavior between GCC 11 and GCC 12

[Lendacky, Thomas]

On 4/14/23 15:23, Tom Lendacky wrote:
> I've been trying to debug a problem I'm seeing when I moved to the GCC 12
> compiler. Under SEV it results in the guest crashing.

False alarm, I was on the wrong branch that does not have ff36b2550f94 
("OvmfPkg/Sec: fix stack switch")... disregard.

Thanks,
Tom

> 
> I narrowed the issue down to the call to TemporaryRamMigration() in
> PeiCheckAndSwitchStack() of MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c.
> 
> I get this output on GCC11:
>    Old Stack size 32768, New stack size 131072
>    Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>    Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>    *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>    TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>    *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=3BF95D20
> 
> and everything is good.
> 
> However, I get this output on GCC12:
>    Old Stack size 32768, New stack size 131072
>    Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>    Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>    *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>    TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>    *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=7770BD20
>    MMIO using encrypted memory: 7770BD48
>    !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 
> 00000000 !!!!
> 
> and terminate because SecCoreData has been corrupted and points to an
> address in an MMIO range (this is an SEV-ES/SEV-SNP example).
> 
> As near as I can tell from looking at the object code, on GCC12 it looks
> like the SecCoreData value is stored in the RBP register, which appears to
> be getting corrupted when calling TemporaryRamMigration().
> 
> Does anyone have any thoughts on this?
> 
> Thanks,
> Tom
> 
> 

Re: Strange behavior between GCC 11 and GCC 12

[Ard Biesheuvel]

On Fri, 14 Apr 2023 at 22:23, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> I've been trying to debug a problem I'm seeing when I moved to the GCC 12
> compiler. Under SEV it results in the guest crashing.
>
> I narrowed the issue down to the call to TemporaryRamMigration() in
> PeiCheckAndSwitchStack() of MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c.
>
> I get this output on GCC11:
>    Old Stack size 32768, New stack size 131072
>    Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>    Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>    *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>    TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>    *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=3BF95D20
>
> and everything is good.
>
> However, I get this output on GCC12:
>    Old Stack size 32768, New stack size 131072
>    Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>    Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>    *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>    TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>    *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=7770BD20
>    MMIO using encrypted memory: 7770BD48
>    !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!
>
> and terminate because SecCoreData has been corrupted and points to an
> address in an MMIO range (this is an SEV-ES/SEV-SNP example).
>
> As near as I can tell from looking at the object code, on GCC12 it looks
> like the SecCoreData value is stored in the RBP register, which appears to
> be getting corrupted when calling TemporaryRamMigration().
>
> Does anyone have any thoughts on this?
>

The stack switching logic in OvmfPkg/Sec/SecMain.c looks highly dubious to me.

LongJump() can be used to do a long return, i.e., it allows to return
from several levels deep in the call stack to back up to where
SetJump() was called. However, using LongJump() to return to the
caller with a different stack is, quite frankly, insane, and I'm
surprised it didn't break a lot sooner.

In this particular case, RBX gets updated along with RSP, presumably
because the code assumes it is being used as a frame pointer? Are you
building with -fomit-frame-pointer perhaps?

Re: Strange behavior between GCC 11 and GCC 12

[Lendacky, Thomas]

On 4/14/23 16:39, Ard Biesheuvel wrote:
> On Fri, 14 Apr 2023 at 22:23, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> I've been trying to debug a problem I'm seeing when I moved to the GCC 12
>> compiler. Under SEV it results in the guest crashing.
>>
>> I narrowed the issue down to the call to TemporaryRamMigration() in
>> PeiCheckAndSwitchStack() of MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c.
>>
>> I get this output on GCC11:
>>     Old Stack size 32768, New stack size 131072
>>     Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>>     Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>>     *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>>     TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>>     *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=3BF95D20
>>
>> and everything is good.
>>
>> However, I get this output on GCC12:
>>     Old Stack size 32768, New stack size 131072
>>     Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>>     Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>>     *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>>     TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>>     *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=7770BD20
>>     MMIO using encrypted memory: 7770BD48
>>     !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!
>>
>> and terminate because SecCoreData has been corrupted and points to an
>> address in an MMIO range (this is an SEV-ES/SEV-SNP example).
>>
>> As near as I can tell from looking at the object code, on GCC12 it looks
>> like the SecCoreData value is stored in the RBP register, which appears to
>> be getting corrupted when calling TemporaryRamMigration().
>>
>> Does anyone have any thoughts on this?
>>
> 
> The stack switching logic in OvmfPkg/Sec/SecMain.c looks highly dubious to me.
> 
> LongJump() can be used to do a long return, i.e., it allows to return
> from several levels deep in the call stack to back up to where
> SetJump() was called. However, using LongJump() to return to the
> caller with a different stack is, quite frankly, insane, and I'm
> surprised it didn't break a lot sooner.
> 
> In this particular case, RBX gets updated along with RSP, presumably
> because the code assumes it is being used as a frame pointer? Are you
> building with -fomit-frame-pointer perhaps?

Looks like our emails crossed paths...  turns out I was on the wrong 
branch for my testing and didn't have ff36b2550f94 ("OvmfPkg/Sec: fix 
stack switch").

So you can disregard, but thanks for taking a look.

Thanks,
Tom

Re: [edk2-devel] Strange behavior between GCC 11 and GCC 12

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 3184 bytes --]

Why does OVMF choose to migrate the content from NEM to MEM itself?
PEI core can do the migration well.

thanks,
ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io>
Sent: Saturday, April 15, 2023 5:50:25 AM
To: Ard Biesheuvel <ardb@kernel.org>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [edk2-devel] Strange behavior between GCC 11 and GCC 12

On 4/14/23 16:39, Ard Biesheuvel wrote:
> On Fri, 14 Apr 2023 at 22:23, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> I've been trying to debug a problem I'm seeing when I moved to the GCC 12
>> compiler. Under SEV it results in the guest crashing.
>>
>> I narrowed the issue down to the call to TemporaryRamMigration() in
>> PeiCheckAndSwitchStack() of MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c.
>>
>> I get this output on GCC11:
>>     Old Stack size 32768, New stack size 131072
>>     Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>>     Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>>     *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>>     TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>>     *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=3BF95D20
>>
>> and everything is good.
>>
>> However, I get this output on GCC12:
>>     Old Stack size 32768, New stack size 131072
>>     Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>>     Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>>     *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>>     TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>>     *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=7770BD20
>>     MMIO using encrypted memory: 7770BD48
>>     !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!
>>
>> and terminate because SecCoreData has been corrupted and points to an
>> address in an MMIO range (this is an SEV-ES/SEV-SNP example).
>>
>> As near as I can tell from looking at the object code, on GCC12 it looks
>> like the SecCoreData value is stored in the RBP register, which appears to
>> be getting corrupted when calling TemporaryRamMigration().
>>
>> Does anyone have any thoughts on this?
>>
>
> The stack switching logic in OvmfPkg/Sec/SecMain.c looks highly dubious to me.
>
> LongJump() can be used to do a long return, i.e., it allows to return
> from several levels deep in the call stack to back up to where
> SetJump() was called. However, using LongJump() to return to the
> caller with a different stack is, quite frankly, insane, and I'm
> surprised it didn't break a lot sooner.
>
> In this particular case, RBX gets updated along with RSP, presumably
> because the code assumes it is being used as a frame pointer? Are you
> building with -fomit-frame-pointer perhaps?

Looks like our emails crossed paths...  turns out I was on the wrong
branch for my testing and didn't have ff36b2550f94 ("OvmfPkg/Sec: fix
stack switch").

So you can disregard, but thanks for taking a look.

Thanks,
Tom






[-- Attachment #2: Type: text/html, Size: 4731 bytes --]

Re: [edk2-devel] Strange behavior between GCC 11 and GCC 12

[Ni, Ray]

[-- Attachment #1: Type: text/plain, Size: 4030 bytes --]

I did a quick check on what TemporaryRamMigration does for OVMF.
No magic there. I think you could try to remove the code in OvmfPkg/Sec/SecMain.c
that does the TemporaryRamSupportPpi installation.
If no such PPI, the migration will be done by PEI core.

Thanks,
Ray

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
Sent: Saturday, April 15, 2023 8:50 AM
To: devel@edk2.groups.io; thomas.lendacky@amd.com; Ard Biesheuvel <ardb@kernel.org>
Cc: devel@edk2.groups.io; Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [edk2-devel] Strange behavior between GCC 11 and GCC 12

Why does OVMF choose to migrate the content from NEM to MEM itself?
PEI core can do the migration well.

thanks,
ray
________________________________
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Lendacky, Thomas via groups.io <thomas.lendacky=amd.com@groups.io<mailto:thomas.lendacky=amd.com@groups.io>>
Sent: Saturday, April 15, 2023 5:50:25 AM
To: Ard Biesheuvel <ardb@kernel.org<mailto:ardb@kernel.org>>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Gerd Hoffmann <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Subject: Re: [edk2-devel] Strange behavior between GCC 11 and GCC 12

On 4/14/23 16:39, Ard Biesheuvel wrote:
> On Fri, 14 Apr 2023 at 22:23, Tom Lendacky <thomas.lendacky@amd.com<mailto:thomas.lendacky@amd.com>> wrote:
>>
>> I've been trying to debug a problem I'm seeing when I moved to the GCC 12
>> compiler. Under SEV it results in the guest crashing.
>>
>> I narrowed the issue down to the call to TemporaryRamMigration() in
>> PeiCheckAndSwitchStack() of MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c.
>>
>> I get this output on GCC11:
>>     Old Stack size 32768, New stack size 131072
>>     Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>>     Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>>     *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>>     TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>>     *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=3BF95D20
>>
>> and everything is good.
>>
>> However, I get this output on GCC12:
>>     Old Stack size 32768, New stack size 131072
>>     Stack Hob: BaseAddress=0x3BF76000 Length=0x20000
>>     Heap Offset = 0x3B786000 Stack Offset = 0x3B776000
>>     *** DEBUG: PeiCheckAndSwitchStack:851 - SecCoreData=3BF95D20
>>     TemporaryRamMigration(0x810000, 0x3BF8E000, 0x10000)
>>     *** DEBUG: PeiCheckAndSwitchStack:871 - SecCoreData=7770BD20
>>     MMIO using encrypted memory: 7770BD48
>>     !!!! X64 Exception Type - 0D(#GP - General Protection)  CPU Apic ID - 00000000 !!!!
>>
>> and terminate because SecCoreData has been corrupted and points to an
>> address in an MMIO range (this is an SEV-ES/SEV-SNP example).
>>
>> As near as I can tell from looking at the object code, on GCC12 it looks
>> like the SecCoreData value is stored in the RBP register, which appears to
>> be getting corrupted when calling TemporaryRamMigration().
>>
>> Does anyone have any thoughts on this?
>>
>
> The stack switching logic in OvmfPkg/Sec/SecMain.c looks highly dubious to me.
>
> LongJump() can be used to do a long return, i.e., it allows to return
> from several levels deep in the call stack to back up to where
> SetJump() was called. However, using LongJump() to return to the
> caller with a different stack is, quite frankly, insane, and I'm
> surprised it didn't break a lot sooner.
>
> In this particular case, RBX gets updated along with RSP, presumably
> because the code assumes it is being used as a frame pointer? Are you
> building with -fomit-frame-pointer perhaps?

Looks like our emails crossed paths...  turns out I was on the wrong
branch for my testing and didn't have ff36b2550f94 ("OvmfPkg/Sec: fix
stack switch").

So you can disregard, but thanks for taking a look.

Thanks,
Tom






[-- Attachment #2: Type: text/html, Size: 8159 bytes --]

Re: Strange behavior between GCC 11 and GCC 12

[Gerd Hoffmann]

On Fri, Apr 14, 2023 at 04:39:23PM -0500, Tom Lendacky wrote:
> On 4/14/23 15:23, Tom Lendacky wrote:
> > I've been trying to debug a problem I'm seeing when I moved to the GCC 12
> > compiler. Under SEV it results in the guest crashing.
> 
> False alarm, I was on the wrong branch that does not have ff36b2550f94
> ("OvmfPkg/Sec: fix stack switch")... disregard.

FYI: You need 129404f6e439 too.

HTH,
  Gerd