From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
we know that the value of a function pointer in a language like C is
the address of the function descriptor and the first doubleword
of the function descriptor contains the address of the entry point
of the function.
So, when we want to jump to an address (e.g. addr) to execute for
PPC-elf64abi, we should assign the address of addr *NOT* addr itself
to the function pointer or system will jump to the wrong address.
Link: https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
Signed-off-by: He Ying <[email protected]>
---
arch/powerpc/boot/main.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
index cae31a6e8f02..50fd7f11b642 100644
--- a/arch/powerpc/boot/main.c
+++ b/arch/powerpc/boot/main.c
@@ -268,7 +268,16 @@ void start(void)
if (console_ops.close)
console_ops.close();
+#ifdef CONFIG_PPC64_BOOT_WRAPPER
+ /*
+ * For PPC-elf64abi, the value of a function pointer is the address
+ * of the function descriptor. And the first doubleword of a function
+ * descriptor contains the address of the entry point of the function.
+ */
+ kentry = (kernel_entry_t) &vmlinux.addr;
+#else
kentry = (kernel_entry_t) vmlinux.addr;
+#endif
if (ft_addr) {
if(platform_ops.kentry)
platform_ops.kentry(ft_addr, vmlinux.addr);
--
2.17.1
Le 04/06/2021 à 11:22, He Ying a écrit :
> From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
> we know that the value of a function pointer in a language like C is
> the address of the function descriptor and the first doubleword
> of the function descriptor contains the address of the entry point
> of the function.
>
> So, when we want to jump to an address (e.g. addr) to execute for
> PPC-elf64abi, we should assign the address of addr *NOT* addr itself
> to the function pointer or system will jump to the wrong address.
>
> Link: https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
> Signed-off-by: He Ying <[email protected]>
> ---
> arch/powerpc/boot/main.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
> index cae31a6e8f02..50fd7f11b642 100644
> --- a/arch/powerpc/boot/main.c
> +++ b/arch/powerpc/boot/main.c
> @@ -268,7 +268,16 @@ void start(void)
> if (console_ops.close)
> console_ops.close();
>
> +#ifdef CONFIG_PPC64_BOOT_WRAPPER
This kind of need doesn't desserve a #ifdef, see
https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation
You can do:
kentry = (kernel_entry_t)(IS_ENABLED(CONFIG_PPC64_BOOT_WRAPPER) ? &vmlinux.addr : vmlinux.addr);
Or, if you prefer something less compact:
if (IS_ENABLED(CONFIG_PPC64_BOOT_WRAPPER))
kentry = (kernel_entry_t) &vmlinux.addr;
else
kentry = (kernel_entry_t) vmlinux.addr;
> + /*
> + * For PPC-elf64abi, the value of a function pointer is the address
> + * of the function descriptor. And the first doubleword of a function
> + * descriptor contains the address of the entry point of the function.
> + */
> + kentry = (kernel_entry_t) &vmlinux.addr;
> +#else
> kentry = (kernel_entry_t) vmlinux.addr;
> +#endif
> if (ft_addr) {
> if(platform_ops.kentry)
> platform_ops.kentry(ft_addr, vmlinux.addr);
>
On Fri, Jun 4, 2021 at 7:39 PM He Ying <[email protected]> wrote:
>
> From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
> we know that the value of a function pointer in a language like C is
> the address of the function descriptor and the first doubleword
> of the function descriptor contains the address of the entry point
> of the function.
>
> So, when we want to jump to an address (e.g. addr) to execute for
> PPC-elf64abi, we should assign the address of addr *NOT* addr itself
> to the function pointer or system will jump to the wrong address.
How have you tested this?
IIRC the 64bit wrapper is only used for ppc64le builds. For that case
the current code is work because the LE ABI (ABIv2) doesn't use
function descriptors. I think even for a BE kernel we need the current
behaviour because the vmlinux's entry point is screwed up (i.e.
doesn't point a descriptor) and tools in the wild (probably kexec)
expect it to be screwed up.
ABIv2 (LE) reference:
https://openpowerfoundation.org/?resource_lib=64-bit-elf-v2-abi-specification-power-architecture
Hello,
在 2021/6/8 13:26, Oliver O'Halloran 写道:
> On Fri, Jun 4, 2021 at 7:39 PM He Ying <[email protected]> wrote:
>> From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
>> we know that the value of a function pointer in a language like C is
>> the address of the function descriptor and the first doubleword
>> of the function descriptor contains the address of the entry point
>> of the function.
>>
>> So, when we want to jump to an address (e.g. addr) to execute for
>> PPC-elf64abi, we should assign the address of addr *NOT* addr itself
>> to the function pointer or system will jump to the wrong address.
> How have you tested this?
I tested ppc64-elf big-endian. I changed the Kconfig so that ppc64
big-endian
selects PPC64_WRAPPER_BOOT. I used qemu to run the cuImage and found
the problem. It made me confused. By applying this patch, I found it works.
I thought it works for ppc64le too. So I upstream this patch.
>
> IIRC the 64bit wrapper is only used for ppc64le builds. For that case
> the current code is work because the LE ABI (ABIv2) doesn't use
> function descriptors. I think even for a BE kernel we need the current
> behaviour because the vmlinux's entry point is screwed up (i.e.
> doesn't point a descriptor) and tools in the wild (probably kexec)
> expect it to be screwed up.
Yes, you're right. PPC64_WRAPPER_BOOT is only used for ppc64le builds
currently.
LE ABI (ABI v2) doesn't use function descriptors. Is that right? I don't
test that. If so,
this patch should be dropped. But why does ppc64 have different ABIs? So
strange.
If the wrapper is built to ppc64be, my patch is tested right. The entry
point in the ELF
header is always right so you can assign the header->e_entry to the
function pointer
and then jump to the entry by calling the function. But in the ppc
wrapper, the address
is intialized to 0 or malloced to be an address later. In this
situation, I think my patch
should be right for ppc64be.
>
> ABIv2 (LE) reference:
> https://openpowerfoundation.org/?resource_lib=64-bit-elf-v2-abi-specification-power-architecture
> .
On Tue, Jun 8, 2021 at 4:33 PM He Ying <[email protected]> wrote:
>
> Hello,
>
> 在 2021/6/8 13:26, Oliver O'Halloran 写道:
> > On Fri, Jun 4, 2021 at 7:39 PM He Ying <[email protected]> wrote:
> >> From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
> >> we know that the value of a function pointer in a language like C is
> >> the address of the function descriptor and the first doubleword
> >> of the function descriptor contains the address of the entry point
> >> of the function.
> >>
> >> So, when we want to jump to an address (e.g. addr) to execute for
> >> PPC-elf64abi, we should assign the address of addr *NOT* addr itself
> >> to the function pointer or system will jump to the wrong address.
> > How have you tested this?
>
> I tested ppc64-elf big-endian. I changed the Kconfig so that ppc64
> big-endian
>
> selects PPC64_WRAPPER_BOOT. I used qemu to run the cuImage and found
>
> the problem. It made me confused. By applying this patch, I found it works.
>
> I thought it works for ppc64le too. So I upstream this patch.
>
> >
> > IIRC the 64bit wrapper is only used for ppc64le builds. For that case
> > the current code is work because the LE ABI (ABIv2) doesn't use
> > function descriptors. I think even for a BE kernel we need the current
> > behaviour because the vmlinux's entry point is screwed up (i.e.
> > doesn't point a descriptor) and tools in the wild (probably kexec)
> > expect it to be screwed up.
>
> Yes, you're right. PPC64_WRAPPER_BOOT is only used for ppc64le builds
> currently.
>
> LE ABI (ABI v2) doesn't use function descriptors. Is that right? I don't
> test that. If so,
>
> this patch should be dropped. But why does ppc64 have different ABIs? So
> strange.
Yeah, it is strange. When LE support was added the toolchain team took
the opportunity to revamp the ABI since BE and LE binaries were never
going to be compatible. IIRC there is a slight performance advantage
to using v2 since function descriptors added an extra load when
performing a non-local function call. I think.
> If the wrapper is built to ppc64be, my patch is tested right. The entry
> point in the ELF
>
> header is always right so you can assign the header->e_entry to the
> function pointer
>
> and then jump to the entry by calling the function. But in the ppc
> wrapper, the address
>
> is intialized to 0 or malloced to be an address later. In this
> situation, I think my patch
>
> should be right for ppc64be.
Yeah maybe it's fine. I just have some memories of running into some
bizzare edge case at some point. It might have been the entrypoint of
the zImage rather than the vmlinux which had (has?) that problem.
Hello,
在 2021/6/8 12:55, Christophe Leroy 写道:
>
>
> Le 04/06/2021 à 11:22, He Ying a écrit :
>> From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
>> we know that the value of a function pointer in a language like C is
>> the address of the function descriptor and the first doubleword
>> of the function descriptor contains the address of the entry point
>> of the function.
>>
>> So, when we want to jump to an address (e.g. addr) to execute for
>> PPC-elf64abi, we should assign the address of addr *NOT* addr itself
>> to the function pointer or system will jump to the wrong address.
>>
>> Link:
>> https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
>> Signed-off-by: He Ying <[email protected]>
>> ---
>> arch/powerpc/boot/main.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
>> index cae31a6e8f02..50fd7f11b642 100644
>> --- a/arch/powerpc/boot/main.c
>> +++ b/arch/powerpc/boot/main.c
>> @@ -268,7 +268,16 @@ void start(void)
>> if (console_ops.close)
>> console_ops.close();
>> +#ifdef CONFIG_PPC64_BOOT_WRAPPER
>
> This kind of need doesn't desserve a #ifdef, see
> https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation
>
> You can do:
>
>
> kentry = (kernel_entry_t)(IS_ENABLED(CONFIG_PPC64_BOOT_WRAPPER) ?
> &vmlinux.addr : vmlinux.addr);
>
>
> Or, if you prefer something less compact:
>
>
> if (IS_ENABLED(CONFIG_PPC64_BOOT_WRAPPER))
> kentry = (kernel_entry_t) &vmlinux.addr;
> else
> kentry = (kernel_entry_t) vmlinux.addr;
Thanks for reviewing. But from Oliver's reply, this patch should be dropped.
Because all ppc platforms will not build wrapper to ppc64be ELF currently.
And ppc64le uses LE ABI (ABIv2) which doesn't use function descriptors.
So this may not be a problem for now.
Thanks again.
>
>
>> + /*
>> + * For PPC-elf64abi, the value of a function pointer is the address
>> + * of the function descriptor. And the first doubleword of a
>> function
>> + * descriptor contains the address of the entry point of the
>> function.
>> + */
>> + kentry = (kernel_entry_t) &vmlinux.addr;
>> +#else
>> kentry = (kernel_entry_t) vmlinux.addr;
>> +#endif
>> if (ft_addr) {
>> if(platform_ops.kentry)
>> platform_ops.kentry(ft_addr, vmlinux.addr);
>>
> .