Skip to content
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

Merged
merged 3 commits into from
Oct 14, 2024

Conversation

giulianobelinassi
Copy link
Collaborator

This PR ports libpulp to PowerPC64LE. It is broken down into two commits:

  1. Factor-out x86_64 architecture dependent code into arch/x86_64.
  2. Introduce the ppc64le equivalent code into arch/powerpc64le.

This PR requires GCC with the following patch:
https://gcc.gnu.org/pipermail/gcc-patches/2024-May/651025.html

@giulianobelinassi giulianobelinassi force-pushed the ppc64le_rebase branch 2 times, most recently from b9a5fc7 to 3711545 Compare July 24, 2024 21:47
lib/arch/powerpc64le/patch.c Outdated Show resolved Hide resolved
lib/arch/powerpc64le/patch.c Outdated Show resolved Hide resolved
Copy link
Collaborator

@susematz susematz left a 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.

configure.ac Outdated Show resolved Hide resolved
include/arch/x86_64/arch_common.h Show resolved Hide resolved
lib/arch/powerpc64le/patch.c Outdated Show resolved Hide resolved
lib/arch/powerpc64le/ulp_interface.S Outdated Show resolved Hide resolved
lib/arch/x86_64/patch.c Show resolved Hide resolved
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]>
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]>
@giulianobelinassi giulianobelinassi merged commit 5dc0d46 into SUSE:master Oct 14, 2024
7 checks passed
@dubeyabhishek
Copy link

dubeyabhishek commented Oct 23, 2024

@giulianobelinassi @susematz How about the gcc patch for fpatchable-function entry? Is that upstream?

@giulianobelinassi
Copy link
Collaborator Author

@giulianobelinassi @susematz How about the gcc patch for fpatchable-function entry? Is that upstream?

No, the patch is not upstream yet for unknown reasons.

@dubeyabhishek
Copy link

dubeyabhishek commented Oct 23, 2024

@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).

@dubeyabhishek
Copy link

@giulianobelinassi can you give a push for the gcc PR on bugzilla?

@giulianobelinassi
Copy link
Collaborator Author

@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

@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).

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.

@dubeyabhishek
Copy link

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.

@giulianobelinassi
Copy link
Collaborator Author

I just confirmed the problem with passing more than 8 parameters to a function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants