Add UUID-GUID conversion interfaces in ArmFfaLib

[kuqin12]

Description

UUID to EFI GUID conversion is a common operation that is needed for processing FF-A parameters.

This change adds 2 new interfaces to support such conversion and removes an implementation from Standalone MM entrypoint to leverage the new interfaces.

• Breaking change?
  • Breaking change - Does this PR cause a break in build or boot behavior?
  • 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

This change was tested on QEMU SBSA platform and booted to UEFI shell.

Integration Instructions

N/A

[kuqin12]

@samimujawar @leiflindholm @ardbiesheuvel Do you have any suggestions on this change?

[leiflindholm]

Well, there is nothing Arm architecture specific about MS-GUID to UUID conversion, so if we're breaking it out it should move to MdePkg or MdeModulePkg.

[kuqin12]

Well, there is nothing Arm architecture specific about MS-GUID to UUID conversion, so if we're breaking it out it should move to MdePkg or MdeModulePkg.

Okay, I will move it to BaseLib. Probably for the better.

[kuqin12]

These are now moved to BaseLib.

[kuqin12]

@leiflindholm @samimujawar could you please let me know if there are any further comments on this PR? If not, would you mind merging the change? Thanks in advance!

[leiflindholm]

I'm OK with this, but ideally we'd want to hear from MdePkg maintainers.

[kuqin12]

@lgao4 @mdkinney could you please review and provide some feedback? Thanks in advance.

[lgao4]

@lgao4 @mdkinney could you please review and provide some feedback? Thanks in advance.

I agree to add this API in BaseLib.

[kuqin12]

@lgao4 @leiflindholm I know we are in soft freeze, but any opinions on merging this change as part of the 2505 tag? Thanks!

[lgao4]

@lgao4 @mdkinney could you please review and provide some feedback? Thanks in advance.

[lgao4]

@lgao4 @leiflindholm I know we are in soft freeze, but any opinions on merging this change as part of the 2505 tag? Thanks!

Seemly, these changes are not the bug fix. They refine the current implementation. So, I suggest to merge them after the stable tag 202505.

[lgao4]

If no other comment, I will merge it this week.

[mdkinney]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.

This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.

Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

[kuqin12]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.

This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.

Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

Indeed, as you mentioned, the key difference between these two types is endianness. However, explicitly distinguishing between UUID and GUID in the function name enhances clarity more effectively than a swap function. By specifying the types of both input and output data, it improves code readability and makes the flow more intuitive.

[mdkinney]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.
This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.
Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

Indeed, as you mentioned, the key difference between these two types is endianness. However, explicitly distinguishing between UUID and GUID in the function name enhances clarity more effectively than a swap function. By specifying the types of both input and output data, it improves code readability and makes the flow more intuitive.

I think that depends on how often it gets used and if the code that calls the function clearly identifies the need to switch from network byte order to little endian for from little endian to network byte order. We do not have 2 APIs to convert 64-bit values.

It is also confusing because both APIs have an identical implementation.

[kuqin12]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.
This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.
Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

Indeed, as you mentioned, the key difference between these two types is endianness. However, explicitly distinguishing between UUID and GUID in the function name enhances clarity more effectively than a swap function. By specifying the types of both input and output data, it improves code readability and makes the flow more intuitive.

I think that depends on how often it gets used and if the code that calls the function clearly identifies the need to switch from network byte order to little endian for from little endian to network byte order. We do not have 2 APIs to convert 64-bit values.

It is also confusing because both APIs have an identical implementation.

Although the conversion is symmetric, I would prefer that the input and output parameters explicitly include descriptions and types that indicate the endianness/format being passed. This would eliminate the need to manually trace through the entire software stack or keep track of details on paper. Would it be better if the functions are renamed to something like ConvertLittleEndianGuidToBigEndian?

[leiflindholm]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.

No, it's actually worse than that. From https://uefi.org/specs/UEFI/2.11/Apx_A_GUID_and_Time_Formats.html
It should also be noted that TimeLow, TimeMid, TimeHighAndVersion fields in the EFI are encoded as little endian.

[samimujawar]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.

In my experience, every new person that comes across the UUID vs EFI_GUID has at some point got confused and have been led astray.

This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.

I think we can try to clarify the distinction in the comments and/or commit message.

Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

I think we should retain the 2 APIs as it provides clarity and I feel it will help reduce the amount of confusion every new person has when they come across the UUID <-> EFI_GUID conversion. We could add more comments to explain what these APIs do and why.

Also I don't think we should introduce ConvertLittleEndianGuidToBigEndian() as this does not provide a direct mapping to what people expect, i.e. some specifications refer to UUID while edk2 utilises EFI_GUID. I think having a clear mapping would be good.

[mdkinney]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.

In my experience, every new person that comes across the UUID vs EFI_GUID has at some point got confused and have been led astray.

This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.

I think we can try to clarify the distinction in the comments and/or commit message.

Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

I think we should retain the 2 APIs as it provides clarity and I feel it will help reduce the amount of confusion every new person has when they come across the UUID <-> EFI_GUID conversion. We could add more comments to explain what these APIs do and why.

Also I don't think we should introduce ConvertLittleEndianGuidToBigEndian() as this does not provide a direct mapping to what people expect, i.e. some specifications refer to UUID while edk2 utilises EFI_GUID. I think having a clear mapping would be good.

Lets go with 2 APIs with good names for UUID -> GUID and GUID -> UUID. Give parameters good names and complete description of endianess of each field and provide references to RFC4122 and UEFI Spec.

[kuqin12]

I had always though incorrectly that UUID and GUID were the same. From RFC4122, UUID stores values in network byte order (big endian) and the UEFI Specification requires little endian, so EFI_GUID is in little endian byte order. GUID and EFI_GUID are the same type in edk2. So GUID and EFI_GUID are both little endian.

In my experience, every new person that comes across the UUID vs EFI_GUID has at some point got confused and have been led astray.

This distinction is not explicitly called out in the comments for the BaseLib functions being added. This is very similar to the SwapBytesxx() APIs that are used for example in the network stack to convert values larger than 8-bits between network byte order and little endian byte order.

I think we can try to clarify the distinction in the comments and/or commit message.

Do we really need 2 APIs? Could we define a single SwapBytesGuid() or SwapBytesUuid() function that switches byte order of all fields in a GUID/UUID that are larger than 8-bits?

I think we should retain the 2 APIs as it provides clarity and I feel it will help reduce the amount of confusion every new person has when they come across the UUID <-> EFI_GUID conversion. We could add more comments to explain what these APIs do and why.
Also I don't think we should introduce ConvertLittleEndianGuidToBigEndian() as this does not provide a direct mapping to what people expect, i.e. some specifications refer to UUID while edk2 utilises EFI_GUID. I think having a clear mapping would be good.

Lets go with 2 APIs with good names for UUID -> GUID and GUID -> UUID. Give parameters good names and complete description of endianess of each field and provide references to RFC4122 and UEFI Spec.

I added a little essay to the c file describing the difference between the 2, with an example. But please let me know if there is any preference on the function/variable names.

[kuqin12]

@lgao4 We have been through another round of updates. Would you mind helping reviewing and/or merging the PR? Thanks in advance!