-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port libpulp to ppc64le #213
Port libpulp to ppc64le #213
Conversation
b9a5fc7
to
3711545
Compare
3711545
to
c5867b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with the general design of supporting multiple architectures that you chose. Hence the bulk renaming/editing of magic numbers with defines, or separating out specific functions from generic into arch-specific source files is just fine. I'm commented on some individual changes, which you maybe want to tackle. Even without doing anything about that I'm fine with including the patchset as is (after resolving conflicts), i.e. all my comments are optional to tackle.
c5867b2
to
cfaac50
Compare
Libpulp is deeply tied on x86_64 architecture. This commit factors out its code and generalize some of its concepts in order to work in other architectures. Signed-off-by: gbelinassi <[email protected]>
This commit adds a new port to libpulp: Powerpc64le. Signed-off-by: gbelinassi <[email protected]>
cfaac50
to
34d2f8b
Compare
Autoconf 2.69 does not expand the $(target_cpu) variable when creating directories, hence we have to do things manually. Also it doesn't seems to like the @var@ variable replacements. Signed-off-by: Giuliano Belinassi <[email protected]>
34d2f8b
to
b74f6be
Compare
@giulianobelinassi @susematz How about the gcc patch for fpatchable-function entry? Is that upstream? |
No, the patch is not upstream yet for unknown reasons. |
@giulianobelinassi @susematz FYI. The prologue in current implementation works for patching library functions with less than 8 arguments. If the "function to be patched" is having more than 8 arguments, than referencing 8th argument onward in the patched function results in incorrect values. This happens due to auxiliary stack being created by current prologue code, which disturbs the offset referencing of parameters passed on stack by caller(8th argument onwards). |
@giulianobelinassi can you give a push for the gcc PR on bugzilla? |
I just did it: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112980
Do you have a suggestion of how to do it without creating this stack? That can also generate a prologue code with fewer bytes, thus reducing binary size and perhaps performance issues related to it. |
I have tried couple of approaches, that I have experimented. But none of them works perfectly. Every approach is having some limitation. We need to have something like Michael Ellerman's solution to kernel live patching: https://mpe.github.io/posts/2016/05/23/kernel-live-patching-for-ppc64le/ If we can have one register dedicated to store livepatch-stack-pointer(livepatch stack can be provisioned from libpulp code at trigger time), then we can use that register to push LR/TOC value on live patch stack instead of thread stack. This approach will avoid argument issue pointed in last comment. To have some register reserved for such kind of use, I was exploring gcc compiler options like -fcall-saved-reg. Here "reg" would be some register which is non critical during function call. If you have any input around this approach, we can discuss further. |
I just confirmed the problem with passing more than 8 parameters to a function. |
This PR ports libpulp to PowerPC64LE. It is broken down into two commits:
arch/x86_64
.arch/powerpc64le
.This PR requires GCC with the following patch:
https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651025.html