Rename EDKII_PEI_MP_SERVICES2_PPI to EFI_PEI_MP_SERVICES2_PPI

[sachinami]

Description

EDKII_PEI_MP_SERVICES2_PPI was introduced in https://mantis.uefi.org/mantis/view.php?id=2160

As per the naming conventions of the PI specification, it should be renamed to EFI_PEI_MP_SERVICES2_PPI.

The PI specification correction has been submitted in this mantis ticket: https://mantis.uefi.org/mantis/view.php?id=2500

• Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behaviour?
  • Examples: Does it add a new library class or move a module to a different repo.
• Impacts security?
  • Security - Does this PR have a direct security impact?
  • Examples: Crypto algorithm change or buffer overflow fix.
• Includes tests?
  • Tests - Does this PR include any explicit test code?
  • Examples: Unit tests or integration tests.

How This Was Tested

Integration Instructions

Multi Processor Services 2 PPI consumers have to use the PPI structure EFI_PEI_MP_SERVICES2_PPI present in MdePkg/Include/Ppi/MpServices2.h and PPI GUID gEfiPeiMpServices2PpiGuid from MdePkg.

[mergify]

PR can not be merged due to conflict. Please rebase and resubmit

[mdkinney]

Thank you for starting this cleanup. I am wondering if we should keep an alias of the EDK II PPI structure and EDK II GUID to allow downstream consumers time to migrate from the EDK II names to the EFI names.

[sachinami]

Thank you for starting this cleanup. I am wondering if we should keep an alias of the EDK II PPI structure and EDK II GUID to allow downstream consumers time to migrate from the EDK II names to the EFI names.

@mdkinney Sure, the header and the PPI GUID have now been retained for backwards compatibility.

[lzeng14]

If it is removed, how could it maintain backwards compatibility for the code still consuming this GUID ?

[sachinami]

The problem is, since the PPI GUID has been moved to MdePkg.dec as gEfiPeiMpServices2PpiGuid.
Having the same GUID here with a diffrent name will lead to a build error.

I don't believe it is possible to have an alias or Conditional Processing in DEC.

Is there a way around this?

[niruiyu]

If it is removed, how could it maintain backwards compatibility for the code still consuming this GUID ?

The PR changed the PPI name which already breaks the backward compatibility:)
I thought we would maintain it through typedef and two Guid Name pointing to same GUID value for a while.

• @mdkinney for comments.

[mdkinney]

The EDK II GUID and the EFI GUID should never have used the same GUID value. I think that is a bug in the PI Spec.

[sachinami]

The GUID has existed in PI Spec since the 1.8 version (Released on Mar 03, 2023) and in edk2 since the edk2-stable201908 tag.

We would have to sacrifice backwards compatibility in either PI Spec or edk2. Or find a way to have two GUID Names pointing to the same GUID value in DEC files.

CC: @felixpgithub

[ardbiesheuvel]

Is there a way to generate a build warning for builds that are using the EDK II prefixed names so all consumers know to update and we can remove the EDK II names as quickly as possible?

Given that build warnings are fatal these days anyway, this will break the build for those consumers.

So what we might do instead is declare the EDKII aliases conditionally, based on a pre-processor #define. That gives out-of-tree platforms an easy way to fix the broken build (adding the -D option to the compiler command line) when they upgrade, and this also serves as a natural opt-in to the deprecation and subsequent removal of those definitions. (i.e., setting the -D option implies an acknowledgement of the deprecation).

[For the GUID in particular, we could force a build error if the -D option is not set by conditionally emitting a declaration of gEdkiiPeiMpServices2PpiGuid into the PPI header with a conflicting type, considering that #ifdefs don't work in .DEC files]

[mdkinney]

That matches what we have done in the past for the unsafe string functions using the define DISABLE_NEW_DEPRECATED_INTERFACES. Though I notice that has been removed from code but is still in many DSC files.

So suggestion is to remove MpServices.h from UefiCpuPkg. Add MpServices.h to MdePkg following PI spec and add something like #ifdef ENABLE_DEPRECATED_EDKII_MP_SERVICES around EDK II typedef and GUID.

[sachinami]

@ardbiesheuvel @mdkinney Thank you for your comments.

MpServices2.h has been removed from UefiCpuPkg and the instance in MdePkg has been modified for conditional backwards compatibility.

Also I believe it will be worth it to leave DISABLE_NEW_DEPRECATED_INTERFACES in the DSC files for now.
In UEFI 2.10A specification there are a bunch of items which were removed. These are definitions and interfaces which were deprecated in earlier versions of the spec and have been removed now.

Some of them are :

• gEfiDeviceIoProtocolGuid
• gEfiUgaIoProtocolGuid
• gEfiUgaDrawProtocolGuid
• gEfiScsiPassThruProtocolGuid
• EFI_VARIABLE_AUTHENTICATION
• EFI_IP4_CONFIG_PROTOCOL
• EFI_DRIVER_CONFIGURATION_PROTOCOL
• EFI_DRIVER_DIAGNOSTICS_PROTOCOL
• EFI_COMPONENT_NAME_PROTOCOL

I was thinking of creating a PR which will remove some of these and wrap up the others in DISABLE_NEW_DEPRECATED_INTERFACES.

[mdkinney]

I would recommend a new define name be used for this and the define name should include the name/version of the spec the items were deprecated. That way, a platform can choose which deprecated items to keep or not based in spec name/version. This also makes it easier to review platform DSC file for these types of defines.

For example, for UEFI 2.10, we could use a define name such as ENABLE_UEFI_2_10_DEPRECATED_INTERFACES

[sachinami]

Got it @mdkinney. Thank you!

[ardbiesheuvel]

Please ensure that no platform gets broken even temporarily when only some of the patches are applied in the order you are presenting them here.

Generally, that means the following when renaming and/or moving something

• create the new version
• move all users to the new version
• remove the old version

[sachinami]

@ardbiesheuvel The commits have been re-ordered.

[ardbiesheuvel]

In order for this series to be bisectable, these declarations should be combined with the first patch, and the same patch should remove the original header file from UefiCpuPkg. Only the #ifdef/#endif should be added at the very end of the series.

Otherwise, there will be ambiguity around which set of declarations are visible when compiling code that depends on both MdePkg and UefiCpuPkg.

Doing so will break code that relies on UefiCpuPkg but not on MdePkg, but the assumption is that such code does not exist.

[sachinami]

I agree that these declarations should be combined with the first patch. But if the same patch should remove the original header file from UefiCpuPkg, that would mean that MdePkg changes and UefiCpuPkg would be going into the same commit.

And I believe cross-package commits are not permitted in edk2.

We could avoid this, if we bank on the assumption there there would be no code that relies on UefiCpuPkg but not on MdePkg.

[ardbiesheuvel]

I agree that these declarations should be combined with the first patch. But if the same patch should remove the original header file from UefiCpuPkg, that would mean that MdePkg changes and UefiCpuPkg would be going into the same commit.

And I believe cross-package commits are not permitted in edk2.

It is permitted in exceptional cases.

We could avoid this, if we bank on the assumption there there would be no code that relies on UefiCpuPkg but not on MdePkg.

That still means that the new header file that you introduce, which has the same base name, must provide both the old and the new PPI, without relying on any #defines.

So

• patch #1 adds the new header in MdePkg, with backward-compatible aliases defined unconditionally
• patch #2 removes the old header
• patches #3 - #(n-1) update the in-tree users
• patch #n adds the #ifdef conditionals around the backward-compatible aliases.

How does that sound?

[sachinami]

Sounds Good @ardbiesheuvel. The commits have been changed accordingly.