Re: [edk2-devel] [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

2023-10-31 Thread Attar, AbdulLateef (Abdul Lateef) via groups.io 
[Public]

+Laszlo, +Gerd, +Paolo
PR:  https://github.com/tianocore/edk2/pull/4982

-Original Message-
From: Lin, Jacque 
Sent: Tuesday, October 31, 2023 11:07 AM
To: devel@edk2.groups.io
Cc: Lin, Jacque ; Attar, AbdulLateef (Abdul Lateef) 
; Chang, Abner 
Subject: [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in 
AMD MmSaveStateLib

Remove checking SMM Rev ID in AMD Save State lib when reading Save State 
Register EFI_MM_SAVE_STATE_REGISTER_IO.
For AMD, it is not necessary to check SmmRevId when reading Save State Register 
EFI_MM_SAVE_STATE_REGISTER_IO.

Cc: Abdul Lateef Attar 
Cc: Abner Chang 
Signed-off-by: Jacque Lin 
---
 UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
index 3315a6cc44..c4bf6ad4bb 100644
--- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
+++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
@@ -102,7 +102,6 @@ MmSaveStateReadRegister (
   OUT VOID*Buffer   ) {-  UINT32 
SmmRevId;   EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;   AMD_SMRAM_SAVE_STATE_MAP   
*CpuSaveState;   UINT8  DataWidth;@@ -124,18 +123,6 @@ 
MmSaveStateReadRegister (
// Check for special EFI_MM_SAVE_STATE_REGISTER_IO   if (Register == 
EFI_MM_SAVE_STATE_REGISTER_IO) {-//-// Get SMM Revision ID-//-
MmSaveStateReadRegisterByIndex (CpuIndex, 
AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );--
//-// See if the CPU supports the IOMisc register in the save state-//- 
   if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {-  return EFI_NOT_FOUND;-}-  
   // Check if IO Restart Dword [IO Trap] is valid or not using bit 1. if 
(!(CpuSaveState->x64.IO_DWord & 0x02u)) {   return EFI_NOT_FOUND;--
2.36.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110429): https://edk2.groups.io/g/devel/message/110429
Mute This Topic: https://groups.io/mt/102292190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

2023-10-31 Thread Paolo Bonzini 
On Tue, Oct 31, 2023 at 10:16 AM Attar, AbdulLateef (Abdul Lateef)
 wrote:
>
> [Public]
>
> +Laszlo, +Gerd, +Paolo
> PR:  https://github.com/tianocore/edk2/pull/4982

I left a comment in the PR.

The patch is only correct if this code is only ever used on 64-bit
processors. Note that KVM uses the legacy 32-bit format of the SMRAM
state save area, if the virtual machine is configured to clear the LM
bit of CPUID.

Second, the commit message does not explain why you are doing this.
Without such an explanation, it is impossible to provide more
constructive feedback.

Paolo

> -Original Message-
> From: Lin, Jacque 
> Sent: Tuesday, October 31, 2023 11:07 AM
> To: devel@edk2.groups.io
> Cc: Lin, Jacque ; Attar, AbdulLateef (Abdul Lateef) 
> ; Chang, Abner 
> Subject: [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in 
> AMD MmSaveStateLib
>
> Remove checking SMM Rev ID in AMD Save State lib when reading Save State 
> Register EFI_MM_SAVE_STATE_REGISTER_IO.
> For AMD, it is not necessary to check SmmRevId when reading Save State 
> Register EFI_MM_SAVE_STATE_REGISTER_IO.
>
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Signed-off-by: Jacque Lin 
> ---
>  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
>  1 file changed, 13 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
> b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> index 3315a6cc44..c4bf6ad4bb 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> @@ -102,7 +102,6 @@ MmSaveStateReadRegister (
>OUT VOID*Buffer   ) {-  UINT32 
> SmmRevId;   EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;   AMD_SMRAM_SAVE_STATE_MAP   
> *CpuSaveState;   UINT8  DataWidth;@@ -124,18 +123,6 @@ 
> MmSaveStateReadRegister (
> // Check for special EFI_MM_SAVE_STATE_REGISTER_IO   if (Register == 
> EFI_MM_SAVE_STATE_REGISTER_IO) {-//-// Get SMM Revision ID-//-
> MmSaveStateReadRegisterByIndex (CpuIndex, 
> AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );--   
>  //-// See if the CPU supports the IOMisc register in the save state-
> //-if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {-  return EFI_NOT_FOUND;-  
>   }- // Check if IO Restart Dword [IO Trap] is valid or not using bit 1.  
>if (!(CpuSaveState->x64.IO_DWord & 0x02u)) {   return EFI_NOT_FOUND;--
> 2.36.1.windows.1
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110402): https://edk2.groups.io/g/devel/message/110402
Mute This Topic: https://groups.io/mt/102292190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

2023-10-31 Thread Laszlo Ersek 
Hi,

On 10/31/23 10:15, Attar, AbdulLateef (Abdul Lateef) wrote:
> [Public]
> 
> +Laszlo, +Gerd, +Paolo
> PR:  https://github.com/tianocore/edk2/pull/4982

... My opinion, stated elsewhere in this thread in detail, is that this
patch is wrong, and should not be merged.

Laszlo

> 
> -Original Message-
> From: Lin, Jacque 
> Sent: Tuesday, October 31, 2023 11:07 AM
> To: devel@edk2.groups.io
> Cc: Lin, Jacque ; Attar, AbdulLateef (Abdul Lateef) 
> ; Chang, Abner 
> Subject: [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in 
> AMD MmSaveStateLib
> 
> Remove checking SMM Rev ID in AMD Save State lib when reading Save State 
> Register EFI_MM_SAVE_STATE_REGISTER_IO.
> For AMD, it is not necessary to check SmmRevId when reading Save State 
> Register EFI_MM_SAVE_STATE_REGISTER_IO.
> 
> Cc: Abdul Lateef Attar 
> Cc: Abner Chang 
> Signed-off-by: Jacque Lin 
> ---
>  UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
>  1 file changed, 13 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
> b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> index 3315a6cc44..c4bf6ad4bb 100644
> --- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> +++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
> @@ -102,7 +102,6 @@ MmSaveStateReadRegister (
>OUT VOID*Buffer   ) {-  UINT32 
> SmmRevId;   EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;   AMD_SMRAM_SAVE_STATE_MAP   
> *CpuSaveState;   UINT8  DataWidth;@@ -124,18 +123,6 @@ 
> MmSaveStateReadRegister (
> // Check for special EFI_MM_SAVE_STATE_REGISTER_IO   if (Register == 
> EFI_MM_SAVE_STATE_REGISTER_IO) {-//-// Get SMM Revision ID-//-
> MmSaveStateReadRegisterByIndex (CpuIndex, 
> AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );--   
>  //-// See if the CPU supports the IOMisc register in the save state-
> //-if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {-  return EFI_NOT_FOUND;-  
>   }- // Check if IO Restart Dword [IO Trap] is valid or not using bit 1.  
>if (!(CpuSaveState->x64.IO_DWord & 0x02u)) {   return EFI_NOT_FOUND;--
> 2.36.1.windows.1
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110401): https://edk2.groups.io/g/devel/message/110401
Mute This Topic: https://groups.io/mt/102292190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH v2] UefiCpuPkg/MmSaveStateLib: Remove checking Smm Rev ID in AMD MmSaveStateLib

2023-10-30 Thread Jacque Lin via groups.io 
Remove checking SMM Rev ID in AMD Save State lib when reading Save
State Register EFI_MM_SAVE_STATE_REGISTER_IO.
For AMD, it is not necessary to check SmmRevId when reading Save State
Register EFI_MM_SAVE_STATE_REGISTER_IO.

Cc: Abdul Lateef Attar 
Cc: Abner Chang 
Signed-off-by: Jacque Lin 
---
 UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c | 13 -
 1 file changed, 13 deletions(-)

diff --git a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c 
b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
index 3315a6cc44..c4bf6ad4bb 100644
--- a/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
+++ b/UefiCpuPkg/Library/MmSaveStateLib/AmdMmSaveState.c
@@ -102,7 +102,6 @@ MmSaveStateReadRegister (
   OUT VOID*Buffer

   )

 {

-  UINT32 SmmRevId;

   EFI_MM_SAVE_STATE_IO_INFO  *IoInfo;

   AMD_SMRAM_SAVE_STATE_MAP   *CpuSaveState;

   UINT8  DataWidth;

@@ -124,18 +123,6 @@ MmSaveStateReadRegister (
 

   // Check for special EFI_MM_SAVE_STATE_REGISTER_IO

   if (Register == EFI_MM_SAVE_STATE_REGISTER_IO) {

-//

-// Get SMM Revision ID

-//

-MmSaveStateReadRegisterByIndex (CpuIndex, 
AMD_MM_SAVE_STATE_REGISTER_SMMREVID_INDEX, sizeof (SmmRevId), );

-

-//

-// See if the CPU supports the IOMisc register in the save state

-//

-if (SmmRevId < AMD_SMM_MIN_REV_ID_X64) {

-  return EFI_NOT_FOUND;

-}

-

 // Check if IO Restart Dword [IO Trap] is valid or not using bit 1.

 if (!(CpuSaveState->x64.IO_DWord & 0x02u)) {

   return EFI_NOT_FOUND;

-- 
2.36.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110365): https://edk2.groups.io/g/devel/message/110365
Mute This Topic: https://groups.io/mt/102292190/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-