Re: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

["Ni, Ray" <ray.ni@intel.com>]

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

I checked the AMD64 Architecture Programmer's Manual and it says below:
--- AMD64 manual ---
SMM-revision Level—Bits 15:0. Specifies the version of SMM supported by the processor. The SMM-revision level is of the form 0_xx64h, where xx starts with 00 and is incremented for later revisions to the SMM mechanism.
---

That means the SMMRevId is 0_xx64h for AMD64 processor. But I am not sure what the value is for AMD32 processor. Maybe 0 according to the OVMF logic.
--- OVMF's logic ---
  AMD_SMRAM_SAVE_STATE_MAP  *CpuSaveState;

  CpuSaveState = (AMD_SMRAM_SAVE_STATE_MAP *)(UINTN)(SMM_DEFAULT_SMBASE + SMRAM_SAVE_STATE_MAP_OFFSET);

  if ((CpuSaveState->x86.SMMRevId & 0xFFFF) == 0) {
    CpuSaveState->x86.SMBASE = (UINT32)SmBase;
  } else {
    CpuSaveState->x64.SMBASE = (UINT32)SmBase;
  }
------


But, I am very suspicious about the logic in AMD's version as below:
--- AMD's version
  SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_32BIT;

  LMAValue = (UINT32)AsmReadMsr64 (EFER_ADDRESS) & LMA;
  if (LMAValue) {
    SmmSaveStateRegisterLma = (UINT8)EFI_MM_SAVE_STATE_REGISTER_LMA_64BIT;
  }
---
The above logic detects the current CPU mode and 64bit save state area layout is used if it's running in 64bit.

But if a AMD64 CPU runs in 32bit mode, the above logic causes the 32bit save state area layout is used. It's not right!
The save state area layout does not depend on the CPU running mode, but whether it's a legacy CPU or a 64-capable CPU.

Jiaxin, I agree that the confusion should be cleaned up by AMD experts. Let's not change any existing behavior.

Thanks,
Ray
________________________________
From: Wu, Jiaxin <jiaxin.wu@intel.com>
Sent: Thursday, April 25, 2024 8:54
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Dong, Guo <guo.dong@intel.com>; Rhodes, Sean <sean@starlabs.systems>; Lu, James <james.lu@intel.com>; Guo, Gua <gua.guo@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Abdul Lateef Attar <AbdulLateef.Attar@amd.com>; Abner Chang <abner.chang@amd.com>; Tom Lendacky <thomas.lendacky@amd.com>
Subject: RE: [edk2-devel] [PATCH v3 00/13] Add SmmRelocationLib

> >
> > AMD version is not work for IA32X64 ovmf.
> >
> > I checked the detailed: CpuSaveState->x64 is always used for OVMF no
> matter IA32 or X64, while AMD is not, which is decided by the MSR
> EFER_ADDRESS LMA bit check.
>
> Hmm, probably because only PEI runs in 32-bit mode whereas DXE and SMM
> run in 64-bit mode, so 32-bit PEI has to prepare things for the 64-bit
> SMM.
>
> > There is a potential issue/open in OVMF why need use the X64
> > CpuSaveState for IA32. Before this open resolved, I still prefer to
> > keep use the ovmf specific lib instance.
>
> Yes, lets stick to the ovmf version for now, and maybe remove it later
> when fixing the ia32 ovmf builds.
>

Ok, no problem.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#118266): https://edk2.groups.io/g/devel/message/118266
Mute This Topic: https://groups.io/mt/105593568/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



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