Re: [edk2] [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.

2018-07-24 Thread Laszlo Ersek 
On 07/24/18 08:38, Gao, Liming wrote:
> To keep compatibility, I suggest to update MdeModulePkg PPI definition to 
> include MdePkg one, then typedef structure name and define macro name for SMM 
> one. MdePkg SMM protocol uses this way to refer to MM protocol. 

I agree this is a better approach than using
DISABLE_NEW_DEPRECATED_INTERFACES.

That macro is a heavy hammer. I recall using it twice in the past:
- once for the sake of the PCD APIs that end with "S" (i.e., for
deprecating those PCD APIs that didn't return a status),
- and another time for the sake of the string APIs that similary end
with "S" (i.e., for deprecating "unsafe" string functions).

In both of those cases, it had been perfectly possible to write
safe/secure code using the "old" (to-be-deprecated) interfaces, such as:

- a platform might basically never use NVRAM-backed (= HII) dynamic
PCDs, just in-memory dynamic PCDs, hence the set operations were always
expected to succeed -- indeed the conversion to the new interfaces
mostly invovled receiving the status codes and heaping
ASSERT_EFI_ERROR() macro invocations on top,

- a programmer should always track the size of the character arrays
anyway -- sometimes this implies memory functions should be used rather
than string functions altogether, and sometimes this implies that the
carefully called string function can never overflow.

However, the safety / security benefit of deprecating those two sets of
APIs was undeniable on a larger scale; there *were* a good number of
dubious string manipulations that had been caught and fixed through the
deprecation, in practice. (Sometimes the fix would be a quite elaborate
reworking of the code -- I seem to recall some examples in ARM-related
packages that Ard patched and others and I reviewed.)

I see no such safety / security benefit in deprecating the names "SMM"
and "SMI" in existent x86 code.


Another thing I didn't spell out under the OvmfPkg patch: it would not
be just about type names and comments, but variable names too. For
example, consider:

 STATIC
 EFI_STATUS
 EFIAPI
-SmmAccessPeiGetCapabilities (
+MmAccessPeiGetCapabilities (
   IN EFI_PEI_SERVICES**PeiServices,
-  IN PEI_SMM_ACCESS_PPI  *This,
+  IN EFI_PEI_MM_ACCESS_PPI   *This,
   IN OUT UINTN   *SmramMapSize,
-  IN OUT EFI_SMRAM_DESCRIPTOR*SmramMap
+  IN OUT EFI_MMRAM_DESCRIPTOR*SmramMap
   )

For complete coverage / consistency, we'd have to rename "SmramMapSize"
to "MmramMapSize", and "SmramMap" to "MmramMap". That would introduce a
*lot* more code changes. As I stated above, I don't see the benefit at all.

Thanks
Laszlo

> 
> Thanks
> Liming
>> -Original Message-
>> From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
>> Sent: Tuesday, July 24, 2018 9:41 AM
>> To: edk2-devel@lists.01.org
>> Cc: Kinney, Michael D ; Gao, Liming
>> ; Zeng, Star ; Dong, Eric
>> ; Ni, Ruiyu ; ler...@redhat.com;
>> Steele, Kelly ; Justen, Jordan L
>> ; ard.biesheu...@linaro.org
>> Subject: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
>>
>> MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI
>> PPIs. Do not allow their usage when
>> DISABLE_NEW_DEPRECATED_INTERFACES
>> is defined.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marvin Haeuser 
>> ---
>> MdeModulePkg/Include/Ppi/SmmAccess.h| 4 
>> MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 
>> MdeModulePkg/Include/Ppi/SmmControl.h   | 4 
>> 3 files changed, 12 insertions(+)
>>
>> diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h
>> b/MdeModulePkg/Include/Ppi/SmmAccess.h
>> index 795c8815a9c1..0ca6d0e3af22 100644
>> --- a/MdeModulePkg/Include/Ppi/SmmAccess.h
>> +++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
>> @@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>> KIND, EITHER EXPRESS OR IMPLIED.
>>
>> **/
>>
>> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>> +
>> #ifndef _SMM_ACCESS_PPI_H_
>> #define _SMM_ACCESS_PPI_H_
>>
>> @@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {
>> extern EFI_GUID gPeiSmmAccessPpiGuid;
>>
>> #endif
>> +
>> +#endif
>> diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> index 8ac86a443a15..7f24d851ae09 100644
>> --- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> +++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>> @@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>> KIND, EITHER EXPRESS OR IMPLIED.
>>
>> **/
>>
>> +#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>> +
>>
>> #ifndef _SMM_COMMUNICATION_PPI_H_
>> #define _SMM_COMMUNICATION_PPI_H_
>> @@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {
>> extern EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
>>
>> #endif
>> +
>> +#endif
>> diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h
>> b/MdeModulePkg/Include/Ppi/SmmControl.h
>> index 0c62196fb44c..597a6db07f2c 100644
>> --- a/MdeModulePkg/Include/Ppi/SmmControl.h
>> +++

Re: [edk2] [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.

2018-07-24 Thread Zeng, Star 
I do not think we prefer to deprecate the definitions in MdeModulePkg. The 
definitions are also referred by platforms.
And I do not think we prefer to update the consumer code at this moment. The 
SMM driver refers the SMM_ prefix definitions, that definitely makes sense.

I agree Liming's suggestion.


Thanks,
Star
-Original Message-
From: Gao, Liming 
Sent: Tuesday, July 24, 2018 2:39 PM
To: marvin.haeu...@outlook.com; edk2-devel@lists.01.org
Cc: Kinney, Michael D ; Zeng, Star 
; Dong, Eric ; Ni, Ruiyu 
; ler...@redhat.com; Steele, Kelly 
; Justen, Jordan L ; 
ard.biesheu...@linaro.org
Subject: RE: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.

To keep compatibility, I suggest to update MdeModulePkg PPI definition to 
include MdePkg one, then typedef structure name and define macro name for SMM 
one. MdePkg SMM protocol uses this way to refer to MM protocol. 

Thanks
Liming
>-Original Message-
>From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
>Sent: Tuesday, July 24, 2018 9:41 AM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Gao, Liming 
>; Zeng, Star ; Dong, Eric 
>; Ni, Ruiyu ; 
>ler...@redhat.com; Steele, Kelly ; Justen, 
>Jordan L ; ard.biesheu...@linaro.org
>Subject: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
>
>MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI PPIs. 
>Do not allow their usage when DISABLE_NEW_DEPRECATED_INTERFACES is 
>defined.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Marvin Haeuser 
>---
> MdeModulePkg/Include/Ppi/SmmAccess.h| 4 
> MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 
> MdeModulePkg/Include/Ppi/SmmControl.h   | 4 
> 3 files changed, 12 insertions(+)
>
>diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h
>b/MdeModulePkg/Include/Ppi/SmmAccess.h
>index 795c8815a9c1..0ca6d0e3af22 100644
>--- a/MdeModulePkg/Include/Ppi/SmmAccess.h
>+++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
>@@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
>EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
> #ifndef _SMM_ACCESS_PPI_H_
> #define _SMM_ACCESS_PPI_H_
>
>@@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {  extern EFI_GUID 
>gPeiSmmAccessPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>index 8ac86a443a15..7f24d851ae09 100644
>--- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>+++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, 
>EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_COMMUNICATION_PPI_H_
> #define _SMM_COMMUNICATION_PPI_H_
>@@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {  extern 
>EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h
>b/MdeModulePkg/Include/Ppi/SmmControl.h
>index 0c62196fb44c..597a6db07f2c 100644
>--- a/MdeModulePkg/Include/Ppi/SmmControl.h
>+++ b/MdeModulePkg/Include/Ppi/SmmControl.h
>@@ -22,6 +22,8 @@
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_CONTROL_PPI_H_
> #define _SMM_CONTROL_PPI_H_
>@@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {  extern EFI_GUID 
>gPeiSmmControlPpiGuid;
>
> #endif
>+
>+#endif
>--
>2.18.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.

2018-07-24 Thread Gao, Liming 
To keep compatibility, I suggest to update MdeModulePkg PPI definition to 
include MdePkg one, then typedef structure name and define macro name for SMM 
one. MdePkg SMM protocol uses this way to refer to MM protocol. 

Thanks
Liming
>-Original Message-
>From: Marvin Häuser [mailto:marvin.haeu...@outlook.com]
>Sent: Tuesday, July 24, 2018 9:41 AM
>To: edk2-devel@lists.01.org
>Cc: Kinney, Michael D ; Gao, Liming
>; Zeng, Star ; Dong, Eric
>; Ni, Ruiyu ; ler...@redhat.com;
>Steele, Kelly ; Justen, Jordan L
>; ard.biesheu...@linaro.org
>Subject: [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.
>
>MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI
>PPIs. Do not allow their usage when
>DISABLE_NEW_DEPRECATED_INTERFACES
>is defined.
>
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Marvin Haeuser 
>---
> MdeModulePkg/Include/Ppi/SmmAccess.h| 4 
> MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 
> MdeModulePkg/Include/Ppi/SmmControl.h   | 4 
> 3 files changed, 12 insertions(+)
>
>diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h
>b/MdeModulePkg/Include/Ppi/SmmAccess.h
>index 795c8815a9c1..0ca6d0e3af22 100644
>--- a/MdeModulePkg/Include/Ppi/SmmAccess.h
>+++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
>@@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
> #ifndef _SMM_ACCESS_PPI_H_
> #define _SMM_ACCESS_PPI_H_
>
>@@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {
> extern EFI_GUID gPeiSmmAccessPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>index 8ac86a443a15..7f24d851ae09 100644
>--- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
>+++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
>@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
>KIND, EITHER EXPRESS OR IMPLIED.
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_COMMUNICATION_PPI_H_
> #define _SMM_COMMUNICATION_PPI_H_
>@@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {
> extern EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
>
> #endif
>+
>+#endif
>diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h
>b/MdeModulePkg/Include/Ppi/SmmControl.h
>index 0c62196fb44c..597a6db07f2c 100644
>--- a/MdeModulePkg/Include/Ppi/SmmControl.h
>+++ b/MdeModulePkg/Include/Ppi/SmmControl.h
>@@ -22,6 +22,8 @@
>
> **/
>
>+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
>+
>
> #ifndef _SMM_CONTROL_PPI_H_
> #define _SMM_CONTROL_PPI_H_
>@@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {
> extern EFI_GUID gPeiSmmControlPpiGuid;
>
> #endif
>+
>+#endif
>--
>2.18.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 8/8] MdeModulePkg: Deprecate Smm* PPIs.

2018-07-23 Thread Marvin Häuser 
MdeModulePkg's Smm* PPIs have been deprecated entirely by UEFI PI
PPIs. Do not allow their usage when DISABLE_NEW_DEPRECATED_INTERFACES
is defined.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marvin Haeuser 
---
 MdeModulePkg/Include/Ppi/SmmAccess.h| 4 
 MdeModulePkg/Include/Ppi/SmmCommunication.h | 4 
 MdeModulePkg/Include/Ppi/SmmControl.h   | 4 
 3 files changed, 12 insertions(+)

diff --git a/MdeModulePkg/Include/Ppi/SmmAccess.h 
b/MdeModulePkg/Include/Ppi/SmmAccess.h
index 795c8815a9c1..0ca6d0e3af22 100644
--- a/MdeModulePkg/Include/Ppi/SmmAccess.h
+++ b/MdeModulePkg/Include/Ppi/SmmAccess.h
@@ -24,6 +24,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 **/
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 #ifndef _SMM_ACCESS_PPI_H_
 #define _SMM_ACCESS_PPI_H_
 
@@ -144,3 +146,5 @@ struct _PEI_SMM_ACCESS_PPI {
 extern EFI_GUID gPeiSmmAccessPpiGuid;
 
 #endif
+
+#endif
diff --git a/MdeModulePkg/Include/Ppi/SmmCommunication.h 
b/MdeModulePkg/Include/Ppi/SmmCommunication.h
index 8ac86a443a15..7f24d851ae09 100644
--- a/MdeModulePkg/Include/Ppi/SmmCommunication.h
+++ b/MdeModulePkg/Include/Ppi/SmmCommunication.h
@@ -19,6 +19,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 **/
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 
 #ifndef _SMM_COMMUNICATION_PPI_H_
 #define _SMM_COMMUNICATION_PPI_H_
@@ -62,3 +64,5 @@ struct _EFI_PEI_SMM_COMMUNICATION_PPI {
 extern EFI_GUID gEfiPeiSmmCommunicationPpiGuid;
 
 #endif
+
+#endif
diff --git a/MdeModulePkg/Include/Ppi/SmmControl.h 
b/MdeModulePkg/Include/Ppi/SmmControl.h
index 0c62196fb44c..597a6db07f2c 100644
--- a/MdeModulePkg/Include/Ppi/SmmControl.h
+++ b/MdeModulePkg/Include/Ppi/SmmControl.h
@@ -22,6 +22,8 @@
 
 **/
 
+#ifndef DISABLE_NEW_DEPRECATED_INTERFACES
+
 
 #ifndef _SMM_CONTROL_PPI_H_
 #define _SMM_CONTROL_PPI_H_
@@ -94,3 +96,5 @@ struct _PEI_SMM_CONTROL_PPI {
 extern EFI_GUID gPeiSmmControlPpiGuid;
 
 #endif
+
+#endif
-- 
2.18.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel