[edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

Abdul Lateef Attar via groups.io posted 9 patches 2 years, 10 months ago
[edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Abdul Lateef Attar via groups.io 2 years, 10 months ago
From: Abdul Lateef Attar <AbdulLateef.Attar@amd.com>

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

Uses new SmmSmramSaveStateLib library,
which fix the CI failure.

Cc: Paul Grimes <paul.grimes@amd.com>
Cc: Garrett Kirkendall <garrett.kirkendall@amd.com>
Cc: Abner Chang <abner.chang@amd.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Signed-off-by: Abdul Lateef Attar <abdattar@amd.com>
---
 OvmfPkg/OvmfPkgIa32X64.dsc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 065b54450647..39465fa23739 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -4,6 +4,7 @@
 #  Copyright (c) 2006 - 2023, Intel Corporation. All rights reserved.<BR>
 #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
 #  Copyright (c) Microsoft Corporation.
+#  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -451,6 +452,7 @@ [LibraryClasses.common.DXE_SMM_DRIVER]
   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
   SmmCpuRendezvousLib|UefiCpuPkg/Library/SmmCpuRendezvousLib/SmmCpuRendezvousLib.inf
+  SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/IntelSmmSmramSaveStateLib.inf
 
 [LibraryClasses.common.SMM_CORE]
   PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-- 
2.25.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102775): https://edk2.groups.io/g/devel/message/102775
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Gerd Hoffmann 2 years, 10 months ago
> Uses new SmmSmramSaveStateLib library,
> which fix the CI failure.

What is the exact failure you are seeing?

> +  SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/IntelSmmSmramSaveStateLib.inf

This should be the AMD version.  KVM uses the AMD smram state.

Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use
actually use the new SmmSmramSaveStateLib library, and once this
has been done it should be possible to remove
OvmfPkg/Include/Register/QemuSmramSaveStateMap.h

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102819): https://edk2.groups.io/g/devel/message/102819
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Attar, AbdulLateef (Abdul Lateef) via groups.io 2 years, 10 months ago
[AMD Official Use Only - General]

Hi Gerd,
        I think CI was failing with the message " SmmSmramSaveStateLib library not found", I didn't preserve the CI log.
Thanks
Abdul

-----Original Message-----
From: Gerd Hoffmann <kraxel@redhat.com>
Sent: 11 April 2023 15:13
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: devel@edk2.groups.io; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Eric Dong <eric.dong@intel.com>; Ray Ni <ray.ni@intel.com>; Rahul Kumar <rahul1.kumar@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Jiewen Yao <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>
Subject: Re: [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


> Uses new SmmSmramSaveStateLib library, which fix the CI failure.

What is the exact failure you are seeing?

> +
> + SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/IntelSm
> + mSmramSaveStateLib.inf

This should be the AMD version.  KVM uses the AMD smram state.

Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use actually use the new SmmSmramSaveStateLib library, and once this has been done it should be possible to remove OvmfPkg/Include/Register/QemuSmramSaveStateMap.h

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102904): https://edk2.groups.io/g/devel/message/102904
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Ni, Ray 2 years, 10 months ago
Gerd,
I am asking Abdul to directly call SaveStateLib from CpuSmm driver.
I don't think SmmCpuFeaturesLib should be in the middle of CpuSmm driver and SaveStateLib
regarding the save state access.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, April 11, 2023 5:43 PM
> To: Abdul Lateef Attar <abdattar@amd.com>
> Cc: devel@edk2.groups.io; Abdul Lateef Attar
> <AbdulLateef.Attar@amd.com>; Paul Grimes <paul.grimes@amd.com>;
> Garrett Kirkendall <garrett.kirkendall@amd.com>; Abner Chang
> <abner.chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses
> SmmSmramSaveStateLib library
> 
> > Uses new SmmSmramSaveStateLib library,
> > which fix the CI failure.
> 
> What is the exact failure you are seeing?
> 
> > +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmSmramSaveStateLib.inf
> 
> This should be the AMD version.  KVM uses the AMD smram state.
> 
> Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use
> actually use the new SmmSmramSaveStateLib library, and once this
> has been done it should be possible to remove
> OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102823): https://edk2.groups.io/g/devel/message/102823
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Attar, AbdulLateef (Abdul Lateef) via groups.io 2 years, 10 months ago
[AMD Official Use Only - General]

Hi Ray,
        You mean directly call the SmramSaveStateReadRegister instead of below code from edk2/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c ?

  Status = SmmCpuFeaturesReadSaveStateRegister (CpuIndex, Register, Width, Buffer);
  if (Status == EFI_UNSUPPORTED) {
    Status = SmramSaveStateReadRegister (CpuIndex, Register, Width, Buffer);
  }

Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: 11 April 2023 15:40
To: devel@edk2.groups.io; kraxel@redhat.com; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Gerd,
I am asking Abdul to directly call SaveStateLib from CpuSmm driver.
I don't think SmmCpuFeaturesLib should be in the middle of CpuSmm driver and SaveStateLib regarding the save state access.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, April 11, 2023 5:43 PM
> To: Abdul Lateef Attar <abdattar@amd.com>
> Cc: devel@edk2.groups.io; Abdul Lateef Attar
> <AbdulLateef.Attar@amd.com>; Paul Grimes <paul.grimes@amd.com>;
> Garrett Kirkendall <garrett.kirkendall@amd.com>; Abner Chang
> <abner.chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses
> SmmSmramSaveStateLib library
>
> > Uses new SmmSmramSaveStateLib library, which fix the CI failure.
>
> What is the exact failure you are seeing?
>
> > +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmSmramSaveStateLib.inf
>
> This should be the AMD version.  KVM uses the AMD smram state.
>
> Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use
> actually use the new SmmSmramSaveStateLib library, and once this has
> been done it should be possible to remove
> OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
>
> take care,
>   Gerd
>
>
>
> 
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102905): https://edk2.groups.io/g/devel/message/102905
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Ni, Ray 2 years, 10 months ago
yes

thanks,
ray
________________________________
From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Sent: Tuesday, April 11, 2023 7:17:03 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>; kraxel@redhat.com <kraxel@redhat.com>
Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

[AMD Official Use Only - General]

Hi Ray,
        You mean directly call the SmramSaveStateReadRegister instead of below code from edk2/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c ?

  Status = SmmCpuFeaturesReadSaveStateRegister (CpuIndex, Register, Width, Buffer);
  if (Status == EFI_UNSUPPORTED) {
    Status = SmramSaveStateReadRegister (CpuIndex, Register, Width, Buffer);
  }

Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: 11 April 2023 15:40
To: devel@edk2.groups.io; kraxel@redhat.com; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Gerd,
I am asking Abdul to directly call SaveStateLib from CpuSmm driver.
I don't think SmmCpuFeaturesLib should be in the middle of CpuSmm driver and SaveStateLib regarding the save state access.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, April 11, 2023 5:43 PM
> To: Abdul Lateef Attar <abdattar@amd.com>
> Cc: devel@edk2.groups.io; Abdul Lateef Attar
> <AbdulLateef.Attar@amd.com>; Paul Grimes <paul.grimes@amd.com>;
> Garrett Kirkendall <garrett.kirkendall@amd.com>; Abner Chang
> <abner.chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen
> <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
> Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses
> SmmSmramSaveStateLib library
>
> > Uses new SmmSmramSaveStateLib library, which fix the CI failure.
>
> What is the exact failure you are seeing?
>
> > +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmSmramSaveStateLib.inf
>
> This should be the AMD version.  KVM uses the AMD smram state.
>
> Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use
> actually use the new SmmSmramSaveStateLib library, and once this has
> been done it should be possible to remove
> OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
>
> take care,
>   Gerd
>
>
>
> 
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102829): https://edk2.groups.io/g/devel/message/102829
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Attar, AbdulLateef (Abdul Lateef) via groups.io 2 years, 9 months ago
[AMD Official Use Only - General]

Hi Ray,
                From open-source code I can see OVMF still implements SmmCpuFeaturesReadSaveStateRegister.
Hence, we can't directly use SmramSaveStateReadRegister in PiSmmCpuDxe driver,
until we implement the MmSaveStateLib for Ovmf.
I think it's better to keep the code as it is, so that we will not break any open-source or closed-source project.

Thanks
AbduL


From: Ni, Ray <ray.ni@intel.com>
Sent: 11 April 2023 18:19
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; devel@edk2.groups.io; kraxel@redhat.com
Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library


[AMD Official Use Only - General]

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

yes

thanks,
ray
________________________________
From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>
Sent: Tuesday, April 11, 2023 7:17:03 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; kraxel@redhat.com<mailto:kraxel@redhat.com> <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Cc: Grimes, Paul <Paul.Grimes@amd.com<mailto:Paul.Grimes@amd.com>>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com<mailto:Garrett.Kirkendall@amd.com>>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

[AMD Official Use Only - General]

Hi Ray,
        You mean directly call the SmramSaveStateReadRegister instead of below code from edk2/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c ?

  Status = SmmCpuFeaturesReadSaveStateRegister (CpuIndex, Register, Width, Buffer);
  if (Status == EFI_UNSUPPORTED) {
    Status = SmramSaveStateReadRegister (CpuIndex, Register, Width, Buffer);
  }

Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: 11 April 2023 15:40
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; kraxel@redhat.com<mailto:kraxel@redhat.com>; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>; Grimes, Paul <Paul.Grimes@amd.com<mailto:Paul.Grimes@amd.com>>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com<mailto:Garrett.Kirkendall@amd.com>>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Gerd,
I am asking Abdul to directly call SaveStateLib from CpuSmm driver.
I don't think SmmCpuFeaturesLib should be in the middle of CpuSmm driver and SaveStateLib regarding the save state access.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, April 11, 2023 5:43 PM
> To: Abdul Lateef Attar <abdattar@amd.com<mailto:abdattar@amd.com>>
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Abdul Lateef Attar
> <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>; Paul Grimes <paul.grimes@amd.com<mailto:paul.grimes@amd.com>>;
> Garrett Kirkendall <garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>>; Abner Chang
> <abner.chang@amd.com<mailto:abner.chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ni, Ray
> <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard
> Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen
> <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
> Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses
> SmmSmramSaveStateLib library
>
> > Uses new SmmSmramSaveStateLib library, which fix the CI failure.
>
> What is the exact failure you are seeing?
>
> > +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmSmramSaveStateLib.inf
>
> This should be the AMD version.  KVM uses the AMD smram state.
>
> Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use
> actually use the new SmmSmramSaveStateLib library, and once this has
> been done it should be possible to remove
> OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
>
> take care,
>   Gerd
>
>
>
> 
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103179): https://edk2.groups.io/g/devel/message/103179
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Ni, Ray 2 years, 9 months ago
Abdul,
What prevents you implementing the OVMF version MmSaveStateLib?
Or I assume OVMF is emulating AMD CPU, why AMD version MmSaveStateLib cannot support OVMF?

Thanks,
Ray

From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>
Sent: Tuesday, April 18, 2023 1:22 PM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; kraxel@redhat.com
Cc: Grimes, Paul <Paul.Grimes@amd.com>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com>; Chang, Abner <Abner.Chang@amd.com>; Dong, Eric <eric.dong@intel.com>; Kumar, Rahul R <rahul.r.kumar@intel.com>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Yao, Jiewen <jiewen.yao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library


[AMD Official Use Only - General]

Hi Ray,
                From open-source code I can see OVMF still implements SmmCpuFeaturesReadSaveStateRegister.
Hence, we can't directly use SmramSaveStateReadRegister in PiSmmCpuDxe driver,
until we implement the MmSaveStateLib for Ovmf.
I think it's better to keep the code as it is, so that we will not break any open-source or closed-source project.

Thanks
AbduL


From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: 11 April 2023 18:19
To: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>; kraxel@redhat.com<mailto:kraxel@redhat.com>
Cc: Grimes, Paul <Paul.Grimes@amd.com<mailto:Paul.Grimes@amd.com>>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com<mailto:Garrett.Kirkendall@amd.com>>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library


[AMD Official Use Only - General]

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.

yes

thanks,
ray
________________________________
From: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>
Sent: Tuesday, April 11, 2023 7:17:03 PM
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; kraxel@redhat.com<mailto:kraxel@redhat.com> <kraxel@redhat.com<mailto:kraxel@redhat.com>>
Cc: Grimes, Paul <Paul.Grimes@amd.com<mailto:Paul.Grimes@amd.com>>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com<mailto:Garrett.Kirkendall@amd.com>>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

[AMD Official Use Only - General]

Hi Ray,
        You mean directly call the SmramSaveStateReadRegister instead of below code from edk2/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c ?

  Status = SmmCpuFeaturesReadSaveStateRegister (CpuIndex, Register, Width, Buffer);
  if (Status == EFI_UNSUPPORTED) {
    Status = SmramSaveStateReadRegister (CpuIndex, Register, Width, Buffer);
  }

Thanks
AbduL

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: 11 April 2023 15:40
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; kraxel@redhat.com<mailto:kraxel@redhat.com>; Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>
Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>; Grimes, Paul <Paul.Grimes@amd.com<mailto:Paul.Grimes@amd.com>>; Kirkendall, Garrett <Garrett.Kirkendall@amd.com<mailto:Garrett.Kirkendall@amd.com>>; Chang, Abner <Abner.Chang@amd.com<mailto:Abner.Chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
Subject: RE: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library

Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.


Gerd,
I am asking Abdul to directly call SaveStateLib from CpuSmm driver.
I don't think SmmCpuFeaturesLib should be in the middle of CpuSmm driver and SaveStateLib regarding the save state access.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Gerd
> Hoffmann
> Sent: Tuesday, April 11, 2023 5:43 PM
> To: Abdul Lateef Attar <abdattar@amd.com<mailto:abdattar@amd.com>>
> Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Abdul Lateef Attar
> <AbdulLateef.Attar@amd.com<mailto:AbdulLateef.Attar@amd.com>>; Paul Grimes <paul.grimes@amd.com<mailto:paul.grimes@amd.com>>;
> Garrett Kirkendall <garrett.kirkendall@amd.com<mailto:garrett.kirkendall@amd.com>>; Abner Chang
> <abner.chang@amd.com<mailto:abner.chang@amd.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Ni, Ray
> <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kumar, Rahul R <rahul.r.kumar@intel.com<mailto:rahul.r.kumar@intel.com>>; Ard
> Biesheuvel <ardb+tianocore@kernel.org<mailto:ardb+tianocore@kernel.org>>; Yao, Jiewen
> <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Justen, Jordan L <jordan.l.justen@intel.com<mailto:jordan.l.justen@intel.com>>
> Subject: Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses
> SmmSmramSaveStateLib library
>
> > Uses new SmmSmramSaveStateLib library, which fix the CI failure.
>
> What is the exact failure you are seeing?
>
> > +
> SmmSmramSaveStateLib|UefiCpuPkg/Library/SmmSmramSaveStateLib/Intel
> SmmSmramSaveStateLib.inf
>
> This should be the AMD version.  KVM uses the AMD smram state.
>
> Also: OvmfPkg/Library/SmmCpuFeaturesLib should be updated to use
> actually use the new SmmSmramSaveStateLib library, and once this has
> been done it should be possible to remove
> OvmfPkg/Include/Register/QemuSmramSaveStateMap.h
>
> take care,
>   Gerd
>
>
>
> 
>


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103131): https://edk2.groups.io/g/devel/message/103131
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/3901457/1787277/102458076/xyzzy [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Gerd Hoffmann 2 years, 9 months ago
On Tue, Apr 18, 2023 at 05:38:09AM +0000, Ni, Ray wrote:
> Abdul,
> What prevents you implementing the OVMF version MmSaveStateLib?

Moving the code to a OVMF-specific MmSaveStateLib should work in
any case.

> Or I assume OVMF is emulating AMD CPU, why AMD version MmSaveStateLib cannot support OVMF?

I'd expect that to work.  kvm might not fully emulate every detail of
amd cpu smm mode, but it uses the amd smram register layout, which is
exactly what MmSaveStateLib implements ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103134): https://edk2.groups.io/g/devel/message/103134
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v8 9/9] OvmfPkg: Uses SmmSmramSaveStateLib library
Posted by Gerd Hoffmann 2 years, 10 months ago
On Tue, Apr 11, 2023 at 10:09:59AM +0000, Ni, Ray wrote:
> Gerd,
> I am asking Abdul to directly call SaveStateLib from CpuSmm driver.
> I don't think SmmCpuFeaturesLib should be in the middle of CpuSmm driver and SaveStateLib
> regarding the save state access.

Yes, sure.  The SmmCpuFeatures{Read,Write}SaveStateRegister() functions
in OvmfPkg/Library/SmmCpuFeaturesLib can be removed in favor of the AMD
version of SmmSmramSaveStateLib.

There are a few more places in OvmfPkg/Library/SmmCpuFeaturesLib which
access the state, for example the functions handling CPU hutplug.  They
use QEMU_SMRAM_SAVE_STATE_MAP today, which should be identical to the
AMD version, so once we have the AMD version merged into the tree ovmf
can just use that instead of its own copy.

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102825): https://edk2.groups.io/g/devel/message/102825
Mute This Topic: https://groups.io/mt/98172964/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-