Drop a bunch of #ifdefs CONFIG_PPC64 that are not vital.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/include/asm/ptrace.h | 9 ++++-----
arch/powerpc/include/uapi/asm/ptrace.h | 12 ++++--------
arch/powerpc/kernel/ptrace/ptrace.c | 24 +++---------------------
3 files changed, 11 insertions(+), 34 deletions(-)
diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index faa5a338ac5a..1506a9c61d50 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -36,11 +36,10 @@ struct pt_regs
unsigned long link;
unsigned long xer;
unsigned long ccr;
-#ifdef CONFIG_PPC64
- unsigned long softe;
-#else
- unsigned long mq;
-#endif
+ union {
+ unsigned long softe;
+ unsigned long mq;
+ };
unsigned long trap;
unsigned long dar;
unsigned long dsisr;
diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
index f5f1ccc740fc..37d7befbb8dc 100644
--- a/arch/powerpc/include/uapi/asm/ptrace.h
+++ b/arch/powerpc/include/uapi/asm/ptrace.h
@@ -43,12 +43,11 @@ struct pt_regs
unsigned long link;
unsigned long xer;
unsigned long ccr;
-#ifdef __powerpc64__
- unsigned long softe; /* Soft enabled/disabled */
-#else
- unsigned long mq; /* 601 only (not used at present) */
+ union {
+ unsigned long softe; /* Soft enabled/disabled */
+ unsigned long mq; /* 601 only (not used at present) */
/* Used on APUS to hold IPL value. */
-#endif
+ };
unsigned long trap; /* Reason for being here */
/* N.B. for critical exceptions on 4xx, the dar and dsisr
fields are overloaded to hold srr0 and srr1. */
@@ -105,11 +104,8 @@ struct pt_regs
#define PT_LNK 36
#define PT_XER 37
#define PT_CCR 38
-#ifndef __powerpc64__
#define PT_MQ 39
-#else
#define PT_SOFTE 39
-#endif
#define PT_TRAP 40
#define PT_DAR 41
#define PT_DSISR 42
diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
index 684b0b315c32..0afb223c4d57 100644
--- a/arch/powerpc/kernel/ptrace/ptrace.c
+++ b/arch/powerpc/kernel/ptrace/ptrace.c
@@ -113,11 +113,8 @@ static const struct pt_regs_offset regoffset_table[] = {
REG_OFFSET_NAME(link),
REG_OFFSET_NAME(xer),
REG_OFFSET_NAME(ccr),
-#ifdef CONFIG_PPC64
REG_OFFSET_NAME(softe),
-#else
REG_OFFSET_NAME(mq),
-#endif
REG_OFFSET_NAME(trap),
REG_OFFSET_NAME(dar),
REG_OFFSET_NAME(dsisr),
@@ -289,17 +286,15 @@ int ptrace_get_reg(struct task_struct *task, int regno, unsigned long *data)
if (regno == PT_DSCR)
return get_user_dscr(task, data);
-#ifdef CONFIG_PPC64
/*
* softe copies paca->irq_soft_mask variable state. Since irq_soft_mask is
* no more used as a flag, lets force usr to alway see the softe value as 1
* which means interrupts are not soft disabled.
*/
- if (regno == PT_SOFTE) {
+ if (IS_ENABLED(CONFIG_PPC64) && regno == PT_SOFTE) {
*data = 1;
return 0;
}
-#endif
regs_max = sizeof(struct user_pt_regs) / sizeof(unsigned long);
if (regno < regs_max) {
@@ -2013,7 +2008,6 @@ static const struct user_regset_view user_ppc_native_view = {
.regsets = native_regsets, .n = ARRAY_SIZE(native_regsets)
};
-#ifdef CONFIG_PPC64
#include <linux/compat.h>
static int gpr32_get_common(struct task_struct *target,
@@ -2287,14 +2281,11 @@ static const struct user_regset_view user_ppc_compat_view = {
.name = "ppc", .e_machine = EM_PPC, .ei_osabi = ELF_OSABI,
.regsets = compat_regsets, .n = ARRAY_SIZE(compat_regsets)
};
-#endif /* CONFIG_PPC64 */
const struct user_regset_view *task_user_regset_view(struct task_struct *task)
{
-#ifdef CONFIG_PPC64
- if (test_tsk_thread_flag(task, TIF_32BIT))
+ if (IS_ENABLED(CONFIG_PPC64) && test_tsk_thread_flag(task, TIF_32BIT))
return &user_ppc_compat_view;
-#endif
return &user_ppc_native_view;
}
@@ -3081,11 +3072,7 @@ long arch_ptrace(struct task_struct *child, long request,
else
dbginfo.num_data_bps = 0;
dbginfo.num_condition_regs = 0;
-#ifdef CONFIG_PPC64
- dbginfo.data_bp_alignment = 8;
-#else
- dbginfo.data_bp_alignment = 4;
-#endif
+ dbginfo.data_bp_alignment = sizeof(long);
dbginfo.sizeof_condition = 0;
#ifdef CONFIG_HAVE_HW_BREAKPOINT
dbginfo.features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
@@ -3322,12 +3309,10 @@ long do_syscall_trace_enter(struct pt_regs *regs)
if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
trace_sys_enter(regs, regs->gpr[0]);
-#ifdef CONFIG_PPC64
if (!is_32bit_task())
audit_syscall_entry(regs->gpr[0], regs->gpr[3], regs->gpr[4],
regs->gpr[5], regs->gpr[6]);
else
-#endif
audit_syscall_entry(regs->gpr[0],
regs->gpr[3] & 0xffffffff,
regs->gpr[4] & 0xffffffff,
@@ -3382,13 +3367,10 @@ void __init pt_regs_check(void)
offsetof(struct user_pt_regs, xer));
BUILD_BUG_ON(offsetof(struct pt_regs, ccr) !=
offsetof(struct user_pt_regs, ccr));
-#ifdef __powerpc64__
BUILD_BUG_ON(offsetof(struct pt_regs, softe) !=
offsetof(struct user_pt_regs, softe));
-#else
BUILD_BUG_ON(offsetof(struct pt_regs, mq) !=
offsetof(struct user_pt_regs, mq));
-#endif
BUILD_BUG_ON(offsetof(struct pt_regs, trap) !=
offsetof(struct user_pt_regs, trap));
BUILD_BUG_ON(offsetof(struct pt_regs, dar) !=
--
2.13.3
On Jun 28 2019, Christophe Leroy <[email protected]> wrote:
> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> index f5f1ccc740fc..37d7befbb8dc 100644
> --- a/arch/powerpc/include/uapi/asm/ptrace.h
> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> @@ -43,12 +43,11 @@ struct pt_regs
> unsigned long link;
> unsigned long xer;
> unsigned long ccr;
> -#ifdef __powerpc64__
> - unsigned long softe; /* Soft enabled/disabled */
> -#else
> - unsigned long mq; /* 601 only (not used at present) */
> + union {
> + unsigned long softe; /* Soft enabled/disabled */
> + unsigned long mq; /* 601 only (not used at present) */
> /* Used on APUS to hold IPL value. */
> -#endif
> + };
Anonymous unions are a C11 feature.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Le 28/06/2019 à 18:36, Andreas Schwab a écrit :
> On Jun 28 2019, Christophe Leroy <[email protected]> wrote:
>
>> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
>> index f5f1ccc740fc..37d7befbb8dc 100644
>> --- a/arch/powerpc/include/uapi/asm/ptrace.h
>> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
>> @@ -43,12 +43,11 @@ struct pt_regs
>> unsigned long link;
>> unsigned long xer;
>> unsigned long ccr;
>> -#ifdef __powerpc64__
>> - unsigned long softe; /* Soft enabled/disabled */
>> -#else
>> - unsigned long mq; /* 601 only (not used at present) */
>> + union {
>> + unsigned long softe; /* Soft enabled/disabled */
>> + unsigned long mq; /* 601 only (not used at present) */
>> /* Used on APUS to hold IPL value. */
>> -#endif
>> + };
>
> Anonymous unions are a C11 feature.
>
Is that a problem ?
Kernel has a minimum GCC requirement of version 4.6, doesn't 4.6 support
C11 ?
Christophe
On Jun 28 2019, Christophe Leroy <[email protected]> wrote:
> Le 28/06/2019 à 18:36, Andreas Schwab a écrit :
>> On Jun 28 2019, Christophe Leroy <[email protected]> wrote:
>>
>>> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
>>> index f5f1ccc740fc..37d7befbb8dc 100644
>>> --- a/arch/powerpc/include/uapi/asm/ptrace.h
>>> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
>>> @@ -43,12 +43,11 @@ struct pt_regs
>>> unsigned long link;
>>> unsigned long xer;
>>> unsigned long ccr;
>>> -#ifdef __powerpc64__
>>> - unsigned long softe; /* Soft enabled/disabled */
>>> -#else
>>> - unsigned long mq; /* 601 only (not used at present) */
>>> + union {
>>> + unsigned long softe; /* Soft enabled/disabled */
>>> + unsigned long mq; /* 601 only (not used at present) */
>>> /* Used on APUS to hold IPL value. */
>>> -#endif
>>> + };
>>
>> Anonymous unions are a C11 feature.
>>
>
> Is that a problem ?
Yes, this is a UAPI header.
Andreas.
--
Andreas Schwab, [email protected]
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Christophe Leroy <[email protected]> writes:
> Drop a bunch of #ifdefs CONFIG_PPC64 that are not vital.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> ---
> arch/powerpc/include/asm/ptrace.h | 9 ++++-----
> arch/powerpc/include/uapi/asm/ptrace.h | 12 ++++--------
> arch/powerpc/kernel/ptrace/ptrace.c | 24 +++---------------------
> 3 files changed, 11 insertions(+), 34 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index faa5a338ac5a..1506a9c61d50 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -36,11 +36,10 @@ struct pt_regs
> unsigned long link;
> unsigned long xer;
> unsigned long ccr;
> -#ifdef CONFIG_PPC64
> - unsigned long softe;
> -#else
> - unsigned long mq;
> -#endif
> + union {
> + unsigned long softe;
> + unsigned long mq;
> + };
> unsigned long trap;
> unsigned long dar;
> unsigned long dsisr;
> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
> index f5f1ccc740fc..37d7befbb8dc 100644
> --- a/arch/powerpc/include/uapi/asm/ptrace.h
> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
> @@ -43,12 +43,11 @@ struct pt_regs
> unsigned long link;
> unsigned long xer;
> unsigned long ccr;
> -#ifdef __powerpc64__
> - unsigned long softe; /* Soft enabled/disabled */
> -#else
> - unsigned long mq; /* 601 only (not used at present) */
> + union {
> + unsigned long softe; /* Soft enabled/disabled */
> + unsigned long mq; /* 601 only (not used at present) */
> /* Used on APUS to hold IPL value. */
> -#endif
> + };
As Andreas pointed out this is not safe as this is a uapi header.
> unsigned long trap; /* Reason for being here */
> /* N.B. for critical exceptions on 4xx, the dar and dsisr
> fields are overloaded to hold srr0 and srr1. */
> @@ -105,11 +104,8 @@ struct pt_regs
> #define PT_LNK 36
> #define PT_XER 37
> #define PT_CCR 38
> -#ifndef __powerpc64__
> #define PT_MQ 39
> -#else
> #define PT_SOFTE 39
> -#endif
I'd also rather leave that as it is.
There's a slim chance it could break some code that already has either
of those defined.
If you need them both defined to make other code work in the kernel
that's fine, in the kernel header we can do:
// Ensure these are always defined inside the kernel to avoid #ifdefs
#ifdef CONFIG_PPC64
#define PT_MQ 39
#else
#define PT_SOFTE 39
#endif
> #define PT_TRAP 40
> #define PT_DAR 41
> #define PT_DSISR 42
> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
> index 684b0b315c32..0afb223c4d57 100644
> --- a/arch/powerpc/kernel/ptrace/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
> @@ -113,11 +113,8 @@ static const struct pt_regs_offset regoffset_table[] = {
> REG_OFFSET_NAME(link),
> REG_OFFSET_NAME(xer),
> REG_OFFSET_NAME(ccr),
> -#ifdef CONFIG_PPC64
> REG_OFFSET_NAME(softe),
> -#else
> REG_OFFSET_NAME(mq),
> -#endif
Pretty sure that will cause breakage. The offset is ABI.
cheers
Le 24/02/2020 à 11:48, Michael Ellerman a écrit :
> Christophe Leroy <[email protected]> writes:
>> Drop a bunch of #ifdefs CONFIG_PPC64 that are not vital.
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> ---
>> arch/powerpc/include/asm/ptrace.h | 9 ++++-----
>> arch/powerpc/include/uapi/asm/ptrace.h | 12 ++++--------
>> arch/powerpc/kernel/ptrace/ptrace.c | 24 +++---------------------
>> 3 files changed, 11 insertions(+), 34 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
>> index faa5a338ac5a..1506a9c61d50 100644
>> --- a/arch/powerpc/include/asm/ptrace.h
>> +++ b/arch/powerpc/include/asm/ptrace.h
>> @@ -36,11 +36,10 @@ struct pt_regs
>> unsigned long link;
>> unsigned long xer;
>> unsigned long ccr;
>> -#ifdef CONFIG_PPC64
>> - unsigned long softe;
>> -#else
>> - unsigned long mq;
>> -#endif
>> + union {
>> + unsigned long softe;
>> + unsigned long mq;
>> + };
>> unsigned long trap;
>> unsigned long dar;
>> unsigned long dsisr;
>> diff --git a/arch/powerpc/include/uapi/asm/ptrace.h b/arch/powerpc/include/uapi/asm/ptrace.h
>> index f5f1ccc740fc..37d7befbb8dc 100644
>> --- a/arch/powerpc/include/uapi/asm/ptrace.h
>> +++ b/arch/powerpc/include/uapi/asm/ptrace.h
>> @@ -43,12 +43,11 @@ struct pt_regs
>> unsigned long link;
>> unsigned long xer;
>> unsigned long ccr;
>> -#ifdef __powerpc64__
>> - unsigned long softe; /* Soft enabled/disabled */
>> -#else
>> - unsigned long mq; /* 601 only (not used at present) */
>> + union {
>> + unsigned long softe; /* Soft enabled/disabled */
>> + unsigned long mq; /* 601 only (not used at present) */
>> /* Used on APUS to hold IPL value. */
>> -#endif
>> + };
>
> As Andreas pointed out this is not safe as this is a uapi header.
Ok, dropped
>
>> unsigned long trap; /* Reason for being here */
>> /* N.B. for critical exceptions on 4xx, the dar and dsisr
>> fields are overloaded to hold srr0 and srr1. */
>> @@ -105,11 +104,8 @@ struct pt_regs
>> #define PT_LNK 36
>> #define PT_XER 37
>> #define PT_CCR 38
>> -#ifndef __powerpc64__
>> #define PT_MQ 39
>> -#else
>> #define PT_SOFTE 39
>> -#endif
>
> I'd also rather leave that as it is.
>
> There's a slim chance it could break some code that already has either
> of those defined.
>
> If you need them both defined to make other code work in the kernel
> that's fine, in the kernel header we can do:
>
> // Ensure these are always defined inside the kernel to avoid #ifdefs
> #ifdef CONFIG_PPC64
> #define PT_MQ 39
> #else
> #define PT_SOFTE 39
> #endif
Ok.
Only the PT_SOFTE is missing, I added the following in the relevant case:
#define PT_SOFTE PT_MQ
>
>
>> #define PT_TRAP 40
>> #define PT_DAR 41
>> #define PT_DSISR 42
>> diff --git a/arch/powerpc/kernel/ptrace/ptrace.c b/arch/powerpc/kernel/ptrace/ptrace.c
>> index 684b0b315c32..0afb223c4d57 100644
>> --- a/arch/powerpc/kernel/ptrace/ptrace.c
>> +++ b/arch/powerpc/kernel/ptrace/ptrace.c
>> @@ -113,11 +113,8 @@ static const struct pt_regs_offset regoffset_table[] = {
>> REG_OFFSET_NAME(link),
>> REG_OFFSET_NAME(xer),
>> REG_OFFSET_NAME(ccr),
>> -#ifdef CONFIG_PPC64
>> REG_OFFSET_NAME(softe),
>> -#else
>> REG_OFFSET_NAME(mq),
>> -#endif
>
> Pretty sure that will cause breakage. The offset is ABI.
Ok, dropped
Christophe