linux-kernel - Re: [PATCH] powerpc/8xx: fix single

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d371d987-aa4b-20bb-e751-6ced387d5575@c-s.fr>
Date:	Thu, 18 Aug 2016 12:13:21 +0200
From:	Christophe Leroy <christophe.leroy@....fr>
To:	Gabriel Paubert <paubert@...m.es>
Cc:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Paul Mackerras <paulus@...ba.org>,
	Michael Ellerman <mpe@...erman.id.au>,
	Scott Wood <oss@...error.net>, linuxppc-dev@...ts.ozlabs.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc/8xx: fix single_step debug



Le 18/08/2016 à 11:58, Gabriel Paubert a écrit :
> On Thu, Aug 18, 2016 at 11:44:20AM +0200, Christophe Leroy wrote:
>> SPRN_ICR must be read for clearing the internal freeze signal which
>> is asserted by the single step exception, otherwise the timebase and
>> decrementer remain freezed
>
> Minor nit: s/freezed/frozen/
>
> If the timebase and decrementer are frozen even for a few cycles, this
> probably upsets timekeeping. I consider this a completely stupid design
> decision, and maybe I'm not alone.
>
>     Gabriel

We could also unset TBF bit (TimeBase Freeze enable) in TBSCR register 
(today it is set in arch/powerpc/platforms/8xx/m8xx_setup.c) but then it 
would impact debug done with an external BDM system which expects the 
decrementer and TB frozen when it freezes the execution.

Christophe


>
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@....fr>
>> ---
>>  arch/powerpc/include/asm/reg_8xx.h | 1 +
>>  arch/powerpc/kernel/traps.c        | 8 ++++++++
>>  2 files changed, 9 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/reg_8xx.h b/arch/powerpc/include/asm/reg_8xx.h
>> index feaf641..6dae71f 100644
>> --- a/arch/powerpc/include/asm/reg_8xx.h
>> +++ b/arch/powerpc/include/asm/reg_8xx.h
>> @@ -17,6 +17,7 @@
>>  #define SPRN_DC_DAT	570	/* Read-only data register */
>>
>>  /* Misc Debug */
>> +#define SPRN_ICR	148
>>  #define SPRN_DPDR	630
>>  #define SPRN_MI_CAM	816
>>  #define SPRN_MI_RAM0	817
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 2cb5892..0f1f0ce 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -400,8 +400,16 @@ static inline int check_io_access(struct pt_regs *regs)
>>  #define REASON_TRAP		0x20000
>>
>>  #define single_stepping(regs)	((regs)->msr & MSR_SE)
>> +#ifdef CONFIG_PPC_8xx
>> +static inline void clear_single_step(struct pt_regs *regs)
>> +{
>> +	regs->msr &= ~MSR_SE;
>> +	mfspr(SPRN_ICR);
>> +}
>> +#else
>>  #define clear_single_step(regs)	((regs)->msr &= ~MSR_SE)
>>  #endif
>> +#endif
>>
>>  #if defined(CONFIG_4xx)
>>  int machine_check_4xx(struct pt_regs *regs)
>> --
>> 2.1.0