From mboxrd@z Thu Jan  1 00:00:00 1970
Subject: Re: [edk2-devel] [RFC] Remove PI spec TemporaryRamSupport PPI from PEI Core
To: Nate DeSimone <nathaniel.l.desimone@intel.com>,devel@edk2.groups.io
From: "Ni, Ray" <ray.ni@intel.com>
X-Originating-Location: Bayan Lepas, Penang, MY (192.198.146.182)
X-Originating-Platform: Windows Chrome 108
User-Agent: GROUPS.IO Web Poster
MIME-Version: 1.0
Date: Thu, 12 Jan 2023 02:16:56 -0800
References: <02A34F284D1DA44BB705E61F7180EF0AAEA6B259@ORSMSX114.amr.corp.intel.com>
In-Reply-To: <02A34F284D1DA44BB705E61F7180EF0AAEA6B259@ORSMSX114.amr.corp.intel.com>
Message-ID: <16581.1673518616985803102@groups.io>
Content-Type: multipart/alternative; boundary="MMR7efguvunAW6R7Y7rH"

--MMR7efguvunAW6R7Y7rH
Content-Type: text/plain; charset="utf-8"; markup=markdown
Content-Transfer-Encoding: quoted-printable

Nate,
GCC generates assembly code that uses `RBP` to store `Private` local variab=
le for below C code in PeiCore/Dispatcher.c

```c
      if (StackOffsetPositive) {
        SecCoreData =3D (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID *)SecCo=
reData + StackOffset);
        Private     =3D (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private + Stac=
kOffset);
      } else {
        SecCoreData =3D (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID *)SecCo=
reData - StackOffset);
        Private     =3D (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private - Stac=
kOffset);
      }

      TemporaryRamSupportPpi->TemporaryRamMigration (
                                PeiServices,
                                TemporaryRamBase,
                                (EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack=
 - TemporaryStackSize),
                                TemporaryRamSize
                                );

      PeiCore (SecCoreData, NULL, Private);
```

If `TemporaryRamMigration()` updates the `RBP` to point to physical memory =
by adding/subtracting
the `StackOffset`, that results the Private is added/subtracted by `StackOf=
fset` twice: One in the C code before calling
TemporayRamSupport PPI, the other in `TemporaryRamMigration ()`.

Since `FspSecMain.SecSwitchStack()` does update the `RBP`, have you met the=
 similar issue?

The issue doesn't always happen. It depends on whether `RBP` is used to sto=
re either `SecCoreData` or `Private`.

--MMR7efguvunAW6R7Y7rH
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

<p>Nate,
GCC generates assembly code that uses <code>RBP</code> to store <code>Priva=
te</code> local variable for below C code in PeiCore/Dispatcher.c</p>
<pre class=3D"prettyprint"><code class=3D"language-c">      if (StackOffset=
Positive) {
        SecCoreData =3D (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID *)SecCo=
reData + StackOffset);
        Private     =3D (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private + Stac=
kOffset);
      } else {
        SecCoreData =3D (CONST EFI_SEC_PEI_HAND_OFF *)((UINTN)(VOID *)SecCo=
reData - StackOffset);
        Private     =3D (PEI_CORE_INSTANCE *)((UINTN)(VOID *)Private - Stac=
kOffset);
      }

      TemporaryRamSupportPpi-&gt;TemporaryRamMigration (
                                PeiServices,
                                TemporaryRamBase,
                                (EFI_PHYSICAL_ADDRESS)(UINTN)(TopOfNewStack=
 - TemporaryStackSize),
                                TemporaryRamSize
                                );

      PeiCore (SecCoreData, NULL, Private);
</code></pre>
<p>If <code>TemporaryRamMigration()</code> updates the <code>RBP</code> to =
point to physical memory by adding/subtracting
the <code>StackOffset</code>, that results the Private is added/subtracted =
by <code>StackOffset</code> twice: One in the C code before calling
TemporayRamSupport PPI, the other in <code>TemporaryRamMigration ()</code>.=
</p>

<p>Since <code>FspSecMain.SecSwitchStack()</code> does update the <code>RBP=
</code>, have you met the similar issue?</p>

<p>The issue doesn't always happen. It depends on whether <code>RBP</code> =
is used to store either <code>SecCoreData</code> or <code>Private</code>.</=
p>

--MMR7efguvunAW6R7Y7rH--