Closed source updates, clang/gcc porting.#53
Open
jaykrell wants to merge 25 commits intomicrosoft:mainfrom
Open
Closed source updates, clang/gcc porting.#53jaykrell wants to merge 25 commits intomicrosoft:mainfrom
jaykrell wants to merge 25 commits intomicrosoft:mainfrom
Conversation
jaykrell
force-pushed
the
mu_327_jay2-clang-only
branch
from
ce15b93 to
6c8428f
Compare
Otherwise fails to link to memcpy.
We have identical BdTripleFault and TripleFault. They are the same. We do not need both. Delete BdTripleFault and keep TripleFault. Use mnemonic instead of db for ud2. Change the declaration to EFIAPI. EFIAPI is important for assembly functions when using gcc, where the default calling convention is not EFIAPI, but we shall only have EFIAPI for assembly functions. EFIAPI is also needed for protocols, if we interoperate with binaries from other builds, such as RuntimeServices called by OS. Otherwise, arguably, EFIAPI can vary, i.e. in a consistent build. Tested via disassembly both before and one after. Related work items: #57987809
I will add unwind data later. It has always been missing. Related work items: #57987809
…cpy. (AZIHSM_DDI_API_REV) Also remove toplevel const in parameters. It has a meaning, it can be useful, people do it on purpose. But usually it is an accident. Related work items: #57987809
Gcc tends to warn, if an inline function "maybe" not inlinable. We can fix this by making ALWAYS_INLINE always empty for gcc. Or pragma off the warning. But, in this PR, just..well, I went back and forth. The best approach, really is to move the code out of .h to .c. Let the compiler decide. We do use LTCG. inline is no longer useful in non-template code, assuming single build variant. But that is an unreadable diff. AI does not assist. Maybe do it anyway. It has the advantage of being C89. Another approach is to #if the inline code and include it only once, without inline. In this PR, use "statiic inline" in .h, and nothing in .c. Related work items: #57987809
Use ZeroMem instead.
```
#define INIT {...}
typedef struct A { ... } A;
A a = (A)INIT;
```
is nice, but kinda not allowed in EFI.
Instead:
```
#define INIT {...}
typedef struct A { ... } A;
static A s = INIT;
A a;
CopyMem(&a, &s, sizeof(a));
```
or `ASSIGN_STRUCT(&a, &s)`
In this case, if we instantiate more variations, we could avoid the copy.
It requires a bit of a trick though, with designated initializers, in order to not break the `INIT` abstraction.
Like this:
```
typedef struct WrapA { A a; } WrapA;
static WrapA s {.a = INIT, .a.foo = 1};
```
The copying in this case is pretty small. I think it is OK.
Related work items: #57987809
…emset. Compilers are prone to output calls to memset and memcpy. Gcc is a little more prone, but Clang is a "problem" too. (Not clearly a problem. Compiler writers are good candidates for writing these functions.) Therefore they have been provided. Use them. It is easy to eliminate most of the calls, but difficult to eliminate them all. This presents the dilema then to eliminate some or not. In some cases, the copy or set is only made explicit, not removed. And other times I noticed excessive copying, easiiy removed. Given that presently upstream AMD64 is broken, and upstream AMD64 community does not like using this library by default, the present approach is a hybrid. Eliminate what we can. Use the library when we can. Hope that covers everything. Which it does seem to. Also submit upstream patches. Related work items: #57987809
Related work items: #57987809
Related work items: #57987809
Light handed alternative to !15020076
We forked around 2016 or earlier.
In 2019 and 2020 mErrorCodeFlag got two more bits.
Gcc/clang fail to link due to the duplicate symbols.
Remove ours in C and link to upstream/fork.
Update ours in assembly.
Here is the history of the two bits:
Ours:
```
commit ee188cdbb20c13f2361192fa1f07192eea8835ef
Author: Larry Cleeton <lcleeton@ntdev.microsoft.com>
Date: Wed Jan 13 12:50:27 2016 -0800
checkpoint - tries to load a boot loader!
```
Upstream:
```
commit 5277540e37e88a1a69f9517c4ad895051b4b3ed3
Author: Tom Lendacky <thomas.lendacky@amd.com>
Date: Wed Aug 12 15:21:36 2020 -0500
UefiCpuPkg/CpuExceptionHandler: Add base support for the #VC exception
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2198
Add base support to handle #VC exceptions. Update the common exception
-CONST UINT32 mErrorCodeFlag = 0x00227d00;
+CONST UINT32 mErrorCodeFlag = 0x20227d00;
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "Reserved",
+ "#VC - VMM Communication"
```
```
commit 0d25074cbcc272532d6ca5a47974ac5b31f4b6ec
Author: Jiewen Yao <jiewen.yao@intel.com>
Date: Fri Feb 22 21:30:35 2019 +0800
UefiCpuPkg/ExceptionLib: Add CET support.
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1521
Add information dump for Control Protection exception.
-CONST UINT32 mErrorCodeFlag = 0x00027d00;
+CONST UINT32 mErrorCodeFlag = 0x00227d00;
- "#VE - Virtualization"
+ "#VE - Virtualization",
+ "#CP - Control Protection"
};
```
Related work items: #57987809
…chitecture tag. Related work items: #57987809
- Gcc LTO checks function signatures. Fix them. - Add/remove stubs. Related work items: #57987809
Make the lld-link flags not the gcc flags. Set gcc flags. Note that gcc flags are sometimes filealign:64K, rendering it, not great. We should use windows targeting/cygwin/mingwin tools instead, so that there is no ELF=>PE conversion, and sectioAlign and filAalign can be unequal. Related work items: #57987809
Upcoming module update breaks this code. Make it more resililent. Remove 33 lines, add 11, no loss of meaning or clarity, except we no longer list all the zeroed member data. Related work items: #57987809
Also for Msvc in MsKdDebugPkg. Related work items: #57987809
- Standard. - Shorter. - I saw other PRs doing it. - A few cases deliberately deferred, since they will not compile (still using __FUNCTION__ as concatenable string literal, instead of non-concatenable char array; stuff not in clang build?) ---- This pull request is a code cleanup that standardizes debug logging by replacing all instances of `__FUNCTION__` with `__func__`. The pull request updates the logging macros across the codebase to use `__func__`, ensuring more consistent and portable debug messages without changing the underlying functionality. - `MsvmPkg/AcpiPlatformDxe/Dsdt.c`, `Tcg2PreInitLibPei.c`, and similar files now use `__func__` in their DEBUG statements. - Modules under `AziHsmDxe` have been modified to uniformly report errors and status using `__func__`. - `VmbusDxe` and `VmbusRoot.c` files have updated logging for memory operations and protocol communications. - Various libraries (e.g., `MsvmRngLib`, `Tpm2DeviceLib`) and unit test files also reflect the change for improved log consistency. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #57987809
See microsoft/mu_basecore#1712 Related work items: #57987809
It is an old optimization, that we never use. Remove it. Related work items: #57987809
It used by a lot of the Masm, and derivation might be used by Nasm. You really cannot write decent ABI-following code with Masm without it. Though the ABI in UEFI is a little unclear wrt unwind, and Nasm does not really "work". Related work items: #57987809
Convert macamd64.inc to macamd64.nasm to reduce porting work. ---- #### AI description (iteration 4) #### PR Classification This pull request adds a new feature by introducing a NASM portability layer to support Clang builds. #### PR Summary The changes introduce a comprehensive assembly macro file that provides AMD64 macros to ease the transition from MASM to NASM, aligning with the Clang build requirements for hyperv.uefi AARCH64. - `MsvmPkg/Include/macamd64.nasm`: Newly added file defining macros for register pushes, stack allocation, register and xmm saving, and function prologue/epilogue management. <!-- GitOpsUserAgent=GitOps.Apps.Server.pullrequestcopilot --> Related work items: #57987809
…KS3.c). `char big[99]= "small"` is `memcpy` and/or `memset`. In this case, the copy is not needed. (In other cases we often add CopyMem/ZeroMem calls as part of the fix). Related work items: #57987809
jaykrell
force-pushed
the
mu_327_jay2-clang-only
branch
from
6c8428f to
584e37a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.