Re: [PATCH] powerpc: Allow relative pointers in bug table entries

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 30/11/2020 à 02:50, Jordan Niethe a écrit :
> On Mon, Nov 30, 2020 at 12:42 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>>> Le 27/11/2020 à 04:02, Jordan Niethe a écrit :
>>>> This enables GENERIC_BUG_RELATIVE_POINTERS on Power so that 32-bit
>>>> offsets are stored in the bug entries rather than 64-bit pointers.
>>>>
>>>> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
>>>> ---
>>>>    arch/powerpc/Kconfig           |  4 ++++
>>>>    arch/powerpc/include/asm/bug.h | 37 ++++++++++++++++++++++++++++++++--
>>>>    arch/powerpc/xmon/xmon.c       | 17 ++++++++++++++--
>>>>    3 files changed, 54 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>>>> index e9f13fe08492..294108e0e5c6 100644
>>>> --- a/arch/powerpc/Kconfig
>>>> +++ b/arch/powerpc/Kconfig
>>>> @@ -311,6 +311,10 @@ config GENERIC_BUG
>>>>       default y
>>>>       depends on BUG
>>>>
>>>> +config GENERIC_BUG_RELATIVE_POINTERS
>>>> +    def_bool y
>>>> +    depends on GENERIC_BUG
>>>> +
>>>>    config SYS_SUPPORTS_APM_EMULATION
>>>>       default y if PMAC_APM_EMU
>>>>       bool
>>>> diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h
>>>> index 338f36cd9934..d03d834042a1 100644
>>>> --- a/arch/powerpc/include/asm/bug.h
>>>> +++ b/arch/powerpc/include/asm/bug.h
>>>> @@ -12,7 +12,11 @@
>>>>    #ifdef CONFIG_DEBUG_BUGVERBOSE
>>>>    .macro EMIT_BUG_ENTRY addr,file,line,flags
>>>>        .section __bug_table,"aw"
>>>> +#ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
>>>
>>> As far as I understand, as soon as CONFIG_BUG is selected, GENERIC_BUG is automatically selected so
>>> GENERIC_BUG_RELATIVE_POINTERS is selected as well. Therefore this #ifndef is never possible.
>>
>> Yeah.
>>
>> There is one place in the generic code that has an ifndef CONFIG_GENERIC_BUG_RELATIVE_POINTERS
>> but that's because it has to support arches that don't select it.
>>
>> In the arch code we know that it's enabled, so there should be no need
>> for any ifdefs.
> For 32bit, pointers are 4 bytes anyway so it would be pointless to
> store a displacement, so won't we need some ifdefs for that?

I'd say it the other way round:

For 32bit, pointers are 4 bytes anyway so it would be pointless to
make it different from 64bit.

We are definitely not on a performance critical path when dealing with bug entries, so I think it is 
better to keep a common code for PPC32 and PPC64.

Christophe