LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: Deprecating of_platform, the path from here...
From: Grant Likely @ 2009-12-10 22:03 UTC (permalink / raw)
  To: David Miller; +Cc: devicetree-discuss, linuxppc-dev, paulus, jk
In-Reply-To: <20091210.135609.124000757.davem@davemloft.net>

On Thu, Dec 10, 2009 at 2:56 PM, David Miller <davem@davemloft.net> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> Date: Thu, 10 Dec 2009 13:47:33 -0700
>
>> Trying to go the other way around (deprecate platform and encouraging
>> of_platform instead) I don't think will gain much traction; whereas I
>> think bringing of_platform features into platform will be an easier
>> sell. =A0I'm trying to be pragmatic here.
>
> When people use words like "traction" and "pragmatic" that means they
> are making decisions for reasons other than technical ones. :-)

100% true.  :-)

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: [PATCH 09/11] of: merge of_attach_node() & of_detach_node()
From: Grant Likely @ 2009-12-10 22:21 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: sfr, monstr, microblaze-uclinux, devicetree-discuss, sparclinux,
	linuxppc-dev, davem
In-Reply-To: <1259208468.16367.234.camel@pasglop>

On Wed, Nov 25, 2009 at 9:07 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Tue, 2009-11-24 at 01:19 -0700, Grant Likely wrote:
>> Merge common code between PowerPC and Microblaze
>
> Some of those guys might wnat to be in of_dynamic (see previous email)
>
> Remember: We want to keep the footprint low for embedded archs that
> don't want to do dynamic stuff. Really low.

I agree.  If it is okay by you I'll update this in a subsequent patch.
 Once the code is merge, then I'm going to do a bunch of refactoring
patches.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* Re: Floating point in the kernel
From: Sean MacLennan @ 2009-12-11  0:17 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linuxppc-dev
In-Reply-To: <200912102235.47985.arnd@arndb.de>

Found it. We are calling sock_sendmsg, which is definitely a call that
can block! The receive side is done in a thread (which does no floating
point ;), but the send was called directly from the "evil FP thread".

It looks like under light load, you tend to get away with it, so our
trivial testing did not catch it. And most of our warp users do RTP via
asterisk, so this RTP path was not really tested.

I really appreciate the input, the comments convinced me I was going
in the wrong direction and forced me to look harder. I am going to back
out the two patches I sent and fix this properly instead.

Cheers,
   Sean

^ permalink raw reply

* Re: dts file for MPC8343EA
From: Junita Ajith @ 2009-12-11  0:34 UTC (permalink / raw)
  To: Scott Wood; +Cc: Linuxppc-dev
In-Reply-To: <20091207180719.GB8914@loki.buserror.net>

[-- Attachment #1: Type: text/plain, Size: 7804 bytes --]

Hi Scott:

I am still stuck at Linux kernel booting in MPC8343EA based board.

I have disabled "Ethernet, PCI, USB, dma engines " in the *.dts file and
also in the kernel config.
I am using MPC8349emitxgp.dts  ; enabled MPC8349ITX support in kernel config
also. In fact, I tried building MPC8349emITX; MPC8349MDS also. Same
behaviour!

With this, the kernel boots up (explicitly passing the DTB file and cuImage
NEVER worked) and hangs after

I am using Linux-V-2.6.27.18 and using powerpc-e300c3-gnu  toolchain  -
gcc-4.1.69-eglibc-2.5.69-1 to build u-boot-1.3.2 & linux.

With this, the kernel boots up (explicitly passing the DTB file and cuImage
NEVER worked) and hangs after printing
"Calibrating delay loop... 133.12 BogoMIPS (lpj=266240)"

With a few debug printks looks like the kernel hangs in
"cpu_idle" in --main/init.c

Any clues..?? HELP!!!
Please find the dts file and screen-dump below:

Thanks,
Junita
DTS file:
=======

/dts-v1/;

/ {
        model = "MPC8349EMITXGP";
        compatible = "MPC8349EMITXGP", "MPC834xMITX", "MPC83xxMITX";
        #address-cells = <1>;
        #size-cells = <1>;

        aliases {
                serial0 = &serial0;
        };

        cpus {
                #address-cells = <1>;
                #size-cells = <0>;

                PowerPC,8349@0 {
                        device_type = "cpu";
                        reg = <0x0>;
                        d-cache-line-size = <32>;
                        i-cache-line-size = <32>;
                        d-cache-size = <32768>;
                        i-cache-size = <32768>;
                        timebase-frequency = <0>;       // from bootloader
                        bus-frequency = <0>;            // from bootloader
                        clock-frequency = <0>;          // from bootloader
                };
        };
memory {
                device_type = "memory";
                reg = <0x00000000 0x10000000>;
        };


        soc8349@e0000000 {
                #address-cells = <1>;
                #size-cells = <1>;
                device_type = "soc";
                compatible = "simple-bus";
                ranges = <0x0 0xe0000000 0x00100000>;
                reg = <0xe0000000 0x00000200>;
                bus-frequency = <0x0fe502a8>;                    // from
bootloader

                wdt@200 {
                        device_type = "watchdog";
                        compatible = "mpc83xx_wdt";
                        reg = <0x200 0x100>;
                };

                i2c@3000 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        cell-index = <0>;
                        compatible = "fsl-i2c";
                        reg = <0x3000 0x100>;
                        interrupts = <14 0x8>;
                        interrupt-parent = <&ipic>;
                        dfsrr;
                };

                mdio@24520 {
                        #address-cells = <1>;
                        #size-cells = <0>;
                        compatible = "fsl,gianfar-mdio";
                        reg = <0x24520 0x20>;

                };
serial0: serial@4500 {
                        cell-index = <0>;
                        device_type = "serial";
                        compatible = "ns16550";
                        reg = <0x4500 0x100>;
                        clock-frequency = <0>;          // from bootloader
                        interrupts = <9 0x8>;
                        interrupt-parent = <&ipic>;
                };

                ipic: pic@700 {
                        interrupt-controller;
                        #address-cells = <0>;
                        #interrupt-cells = <2>;
                        reg = <0x700 0x100>;
                        device_type = "ipic";
                };
        };

};

Screen-shot:
==========
Trying TSEC0
Speed: 100, full duplex
Using TSEC0 device
TFTP from server 192.168.201.14; our IP address is 192.168.201.199
Filename '8349.dtb'.
Load address: 0x900000
Loading: #
done
Bytes transferred = 12288 (3000 hex)
SC3000> bootm 0x00600000 - 0x00900000
## Booting image at 00600000 ...
   Image Name:   Linux-2.6.27.18
   Created:      2009-12-11   0:16:24 UTC
   Image Type:   PowerPC Linux Kernel Image (gzip compressed)
   Data Size:    850785 Bytes = 830.8 kB
   Load Address: 00000000
   Entry Point:  00000000
   Verifying Checksum ... OK
   Uncompressing Kernel Image ... OK
## Current stack ends at 0x0FF9BD48 => set upper limit to 0x00800000
## cmdline at 0x007FFF00 ... 0x007FFF2E
bd address  = 0x0FF9BFC4
memstart    = 0x00000000
memsize     = 0x10000000
flashstart  = 0xFE000000
flashsize   = 0x00080000
flashoffset = 0x00034000
sramstart   = 0x00000000
sramsize    = 0x00000000
bootflags   = 0x00000001
intfreq     = 399.999 MHz
busfreq     = 266.666 MHz
ethaddr     = 00:E0:0C:00:8C:01
IP addr     = 192.168.201.199
baudrate    = 115200 bps
Skipping initrd
   Booting using the fdt at 0x900000
No initrd
## device tree at 0x00900000 ... 0x00902FFF (len=12288=0x3000)
   Loading Device Tree to 007fc000, end 007fefff ... OK
Updating property 'timebase-frequency' =  03 f9 40 aa
Updating property 'bus-frequency' =  0f e5 02 a8
Updating property 'clock-frequency' =  17 d7 83 fc
Updating property 'bus-frequency' =  0f e5 02 a8
Updating property 'clock-frequency' =  0f e5 02 a8
## Transferring control to Linux (at address 00000000) ...
Using MPC834x ITX machine description
Linux version 2.6.27.18 (root@localhost.localdomain) (gcc version 4.1.2) #26
Th9 -> find_legacy_serial_port()
stdout is /soc8349@e0000000/serial@4500
Found legacy serial port 0 for /soc8349@e0000000/serial@4500
  mem=e0004500, taddr=e0004500, irq=0, clk=266666664, speed=0
legacy_serial_console = 0
default console speed = 115740
 <- find_legacy_serial_port()
console [udbg0] enabled
setup_arch: bootmem
mpc834x_itx_setup_arch()
arch: exit
Top of RAM: 0x8000000, Total RAM: 0x8000000
Memory hole size: 0MB
Zone PFN ranges:
  DMA      0x00000000 -> 0x00008000
  Normal   0x00008000 -> 0x00008000
Movable zone start PFN for each node
early_node_map[1] active PFN ranges
    0: 0x00000000 -> 0x00008000
On node 0 totalpages: 32768
free_area_init_node: node 0, pgdat c01b6224, node_mem_map c01d0000
  DMA zone: 32512 pages, LIFO batch:7
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 32512
Kernel command line: root=/dev/ram rw console=ttyS0,115200 mem=128M
IPIC (128 IRQ sources) at fdffd700
PID hash table entries: 512 (order: 9, 2048 bytes)
time_init: decrementer frequency = 66.666666 MHz
time_init: processor frequency   = 399.999996 MHz
clocksource: timebase mult[3c00001] shift[22] registered
clockevent: decrementer mult[1111] shift[16] cpu[0]
 -> check_legacy_serial_console()
 console was specified !
Dentry cache hash table entries: 16384 (order: 4, 65536 bytes)
Inode-cache hash table entries: 8192 (order: 3, 32768 bytes)
SC3000 - After vfs_caches_init_early -1
SC3000 - After vfs_caches_init_early-2
Memory: 127912k/131072k available (1688k kernel code, 3000k reserved, 68k
data,)SC3000 - After vfs_caches_init_early-3
SLUB: Genslabs=12, HWalign=32, Order=0-3, MinObjects=0, CPUs=1, Nodes=1
Calibrating delay loop... 133.12 BogoMIPS (lpj=266240)



On 12/7/09, Scott Wood <scottwood@freescale.com> wrote:
>
> On Sun, Dec 06, 2009 at 10:41:25PM -0800, ajijuni@gmail.com wrote:
> > Hi
> >
> > We have an MPC8343EA based custom board.
> >
> > I am not able to get Linux up and running in this. No serial output to
> debug further.
> >  U-boot shows correct 'bdinfo' & 'clocks' output.
> > inux hangs at machine_probe.
>
> Check that the platform file you're intending to use matches the compatible
> field in the root node of the device tree -- and that said platform file is
> actually being built.
>
>
> -Scott
>

[-- Attachment #2: Type: text/html, Size: 9748 bytes --]

^ permalink raw reply

* Re: [RFC:PATCH 01/03] powerpc: Extended ptrace interface
From: David Gibson @ 2009-12-11  0:44 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091210155715.6697.92627.sendpatchset@norville.austin.ibm.com>

On Thu, Dec 10, 2009 at 01:57:15PM -0200, Dave Kleikamp wrote:
> powerpc: Extended ptrace interface
> 
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> 
> Add a new extended ptrace interface so that user-space has a single
> interface for powerpc, without having to know the specific layout
> of the debug registers.


> Implement:
> PPC_PTRACE_GETHWDEBUGINFO
> PPC_PTRACE_SETHWDEBUG
> PPC_PTRACE_DELHWDEBUG
> 
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>

Apart from the data breakpoint alignment for 32-bit systems, all the
comments below are trivial nits, so:

Acked-by: David Gibson <dwg@au1.ibm.com>

[snip]
> +/*
> + * Trigger Type
> + */
> +#define PPC_BREAKPOINT_TRIGGER_EXECUTE	0x1
> +#define PPC_BREAKPOINT_TRIGGER_READ	0x2
> +#define PPC_BREAKPOINT_TRIGGER_WRITE	0x4
> +#define PPC_BREAKPOINT_TRIGGER_RW	0x6

For a little extra safety, I'd tend towards defining the RW constant
in terms of the READ and WRITE constants.

> +
> +/*
> + * Address Mode
> + */
> +#define PPC_BREAKPOINT_MODE_EXACT		0x0
> +#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE	0x1
> +#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE	0x2
> +#define PPC_BREAKPOINT_MODE_MASK		0x3
> +
> +/*
> + * Condition Mode
> + */
> +#define PPC_BREAKPOINT_CONDITION_NONE	0x0
> +#define PPC_BREAKPOINT_CONDITION_AND	0x1
> +#define PPC_BREAKPOINT_CONDITION_EXACT	0x1

And likewuse define EXACT in terms of AND.

> +#define PPC_BREAKPOINT_CONDITION_OR	0x2
> +#define PPC_BREAKPOINT_CONDITION_AND_OR	0x3
> +#define PPC_BREAKPOINT_CONDITION_BE_ALL	0x00ff0000
> +#define PPC_BREAKPOINT_CONDITION_BE_SHIFT	16
> +#define PPC_BREAKPOINT_CONDITION_BE(n)	\
> +	(1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))

[snip]
> +	case PPC_PTRACE_GETHWDBGINFO: {
> +		struct ppc_debug_info dbginfo;
> +
> +		dbginfo.version = 1;
> +		dbginfo.num_instruction_bps = 0;
> +		dbginfo.num_data_bps = 1;
> +		dbginfo.num_condition_regs = 0;
> +#ifdef CONFIG_PPC64
> +		dbginfo.data_bp_alignment = 8;
> +#else
> +		dbginfo.data_bp_alignment = 0;

Uh.. this looks wrong.  Surely it should be 4.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: David Gibson @ 2009-12-11  0:53 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091210155721.6697.40863.sendpatchset@norville.austin.ibm.com>

On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> powerpc: Add definitions for Debug Registers on BookE Platforms
> 
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> 
> This patch adds additional definitions for BookE Debug Registers
> to the reg_booke.h header file.
> 
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

As with patch 1/3, none of the comments below is anything that
couldn't be fixed up after merging.  So,

Acked-by: David Gibson <dwg@au1.ibm.com>

> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
> 
>  arch/powerpc/include/asm/processor.h |   30 +++++-
>  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
>  2 files changed, 178 insertions(+), 28 deletions(-)

[snip]
> +	/*
> +	 * The following will contain addresses used by debug applications
> +	 * to help trace and trap on particular address locations.
> +	 * The bits in the Debug Control Registers above help define which
> +	 * of the following registers will contain valid data and/or addresses.
> +	 */
> +	unsigned long	iac1;
> +	unsigned long	iac2;
> +	unsigned long	iac3;
> +	unsigned long	iac4;
> +	unsigned long	dac1;
> +	unsigned long	dac2;
> +	unsigned long	dvc1;
> +	unsigned long	dvc2;

I think you'd make the logic in patch 3 substantially easier, if you
defined these as
	unsigned long iac[4];
	unsigned long dac[2];
	unsigned long dvc[2];
instead of as individual structure members.

[snip]
> +#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> +				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> +#define DBCR0_BASE_REG_VALUE	0

These constants are left over from when the interface allowed
more-or-less direct access to the debug regs.  I don't think the
USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
just used in the load_default function, and might as well be inline
there.

[snip]
> +
> +#define dbcr_iac_range(task)	((task)->thread.dbcr0)

Hrm, I think the way these macros work to do the 40x vs. BookE
abstration is kind of ugly.  But an unequivocally better way doesn't
immediately occur to me.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: Dave Kleikamp @ 2009-12-11  1:31 UTC (permalink / raw)
  To: David Gibson
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091211005344.GB8852@yookeroo>

On Fri, 2009-12-11 at 11:53 +1100, David Gibson wrote:
> On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> > powerpc: Add definitions for Debug Registers on BookE Platforms
> > 
> > From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> > 
> > This patch adds additional definitions for BookE Debug Registers
> > to the reg_booke.h header file.
> > 
> > Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> 
> As with patch 1/3, none of the comments below is anything that
> couldn't be fixed up after merging.  So,
> 
> Acked-by: David Gibson <dwg@au1.ibm.com>
> 
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> > Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> > Cc: David Gibson <dwg@au1.ibm.com>
> > Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> > ---
> > 
> >  arch/powerpc/include/asm/processor.h |   30 +++++-
> >  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
> >  2 files changed, 178 insertions(+), 28 deletions(-)
> 
> [snip]
> > +	/*
> > +	 * The following will contain addresses used by debug applications
> > +	 * to help trace and trap on particular address locations.
> > +	 * The bits in the Debug Control Registers above help define which
> > +	 * of the following registers will contain valid data and/or addresses.
> > +	 */
> > +	unsigned long	iac1;
> > +	unsigned long	iac2;
> > +	unsigned long	iac3;
> > +	unsigned long	iac4;
> > +	unsigned long	dac1;
> > +	unsigned long	dac2;
> > +	unsigned long	dvc1;
> > +	unsigned long	dvc2;
> 
> I think you'd make the logic in patch 3 substantially easier, if you
> defined these as
> 	unsigned long iac[4];
> 	unsigned long dac[2];
> 	unsigned long dvc[2];
> instead of as individual structure members.

I'll give that a look.  You're probably right.

> [snip]
> > +#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> > +				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> > +#define DBCR0_BASE_REG_VALUE	0
> 
> These constants are left over from when the interface allowed
> more-or-less direct access to the debug regs.  I don't think the
> USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
> just used in the load_default function, and might as well be inline
> there.

Right.

> [snip]
> > +
> > +#define dbcr_iac_range(task)	((task)->thread.dbcr0)
> 
> Hrm, I think the way these macros work to do the 40x vs. BookE
> abstration is kind of ugly.  But an unequivocally better way doesn't
> immediately occur to me.

Without this, the ifdef's were horrendous.  I'm open to renaming this or
redefining it to be more intuitive if anyone has a better idea.
-- 
David Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 2/3] powerpc/83xx/suspend: Save and restore SICRL, SICRH and SCCR
From: Kumar Gala @ 2009-12-11  1:57 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210180056.GB32482@oksana.dev.rtsoft.ru>


On Dec 10, 2009, at 12:00 PM, Anton Vorontsov wrote:

> We need to save SICRL, SICRH and SCCR registers on suspend, and =
restore
> them on resume. Otherwise, we lose IO and clocks setup on MPC8315E-RDB
> boards when ULPI USB PHY is used (non-POR setup).
>=20
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/platforms/83xx/suspend.c |   48 =
+++++++++++++++++++++++++++++++++
> 1 files changed, 48 insertions(+), 0 deletions(-)

applied to next

- k=

^ permalink raw reply

* Re: [PATCH 3/3] powerpc/83xx: Add power management support for MPC8315E-RDB boards
From: Kumar Gala @ 2009-12-11  1:57 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210180103.GC32482@oksana.dev.rtsoft.ru>


On Dec 10, 2009, at 12:01 PM, Anton Vorontsov wrote:

> - Add nodes for PMC and GTM controllers. GTM4 can be used as a wakeup
>  source;
> 
> - Add fsl,magic-packet properties to eTSEC nodes, i.e. wake-on-lan
>  support. Unlike MPC8313 processors, MPC8315 can resume from deep
>  sleep upon magic packet reception.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
> arch/powerpc/boot/dts/mpc8315erdb.dts |   27 +++++++++++++++++++++++++++
> 1 files changed, 27 insertions(+), 0 deletions(-)

applied to next

- k

^ permalink raw reply

* Re: [PATCH 1/3] powerpc/83xx/suspend: Clear deep_sleeping after devices resume
From: Kumar Gala @ 2009-12-11  1:56 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: Scott Wood, linuxppc-dev
In-Reply-To: <20091210180053.GA32482@oksana.dev.rtsoft.ru>


On Dec 10, 2009, at 12:00 PM, Anton Vorontsov wrote:

> Currently 83xx PMC driver clears deep_sleeping variable very early,
> before devices are resumed. This makes fsl_deep_sleep() unusable in
> drivers' resume() callback.
> 
> Sure, drivers can store fsl_deep_sleep() value on suspend and use
> the stored value on resume. But a better solution is to postpone
> clearing the deep_sleeping variable, i.e. move it into finish()
> callback.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> Acked-by: Scott Wood <scottwood@freescale.com>
> ---
> arch/powerpc/platforms/83xx/suspend.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)

applied to next

- k

^ permalink raw reply

* Re: [PATCH 2/3] asm/gpio.h: support gpio_to_irq()
From: Kumar Gala @ 2009-12-11  2:09 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: Anton Vorontsov, linuxppc-dev
In-Reply-To: <1259700494-17869-2-git-send-email-jacmet@sunsite.dk>


On Dec 1, 2009, at 2:48 PM, Peter Korsgaard wrote:

> gpiolib returns -ENXIO if struct gpio_chip::to_irq isn't set, so it's
> safe to always call.
> 
> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> ---
> arch/powerpc/include/asm/gpio.h |    5 +----
> 1 files changed, 1 insertions(+), 4 deletions(-)

applied to next

- k

^ permalink raw reply

* Re: [PATCH 3/3] mpc8xxx_gpio: add interrupt support
From: Kumar Gala @ 2009-12-11  2:13 UTC (permalink / raw)
  To: Peter Korsgaard; +Cc: Anton Vorontsov, linuxppc-dev
In-Reply-To: <87pr6omwk3.fsf@macbook.be.48ers.dk>


On Dec 9, 2009, at 1:33 AM, Peter Korsgaard wrote:

>>>>>> "Peter" =3D=3D Peter Korsgaard <jacmet@sunsite.dk> writes:
>=20
> Comments?
>=20
> Peter> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
> Peter> ---
> Peter>  arch/powerpc/sysdev/mpc8xxx_gpio.c |  147 =
++++++++++++++++++++++++++++++++++++
> Peter>  1 files changed, 147 insertions(+), 0 deletions(-)

We need a binding document to go with this.

- k=

^ permalink raw reply

* Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface
From: Kumar Gala @ 2009-12-11  2:23 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155709.6697.4635.sendpatchset@norville.austin.ibm.com>


On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> These patches implement an extention to the ptrace interface proposed =
by
> Thiago Bauermann and the the PowerPC gdb team.
>=20
> GDB intends to support the following hardware debug features of BookE
> processors:
>=20
> 4 hardware breakpoints (IAC)
> 2 hardware watchpoints (read, write and read-write) (DAC)
> 2 value conditions for the hardware watchpoints (DVC)
>=20
> For that, we need to extend ptrace so that GDB can query and set these
> resources. Since we're extending, we're trying to create an interface
> that's extendable and that covers both BookE and server processors, so
> that GDB doesn't need to special-case each of them. We propose the
> following 3 new ptrace requests described below.
>=20
> There have been discussions of a generic hardware debug interface for =
the
> kernel which would hopefully contemplate all the functionality below =
and
> supersede it.  But we need something that works now, and which enables =
GDB
> to be simpler and work with both Server and Embedded processors =
without
> special cases.
>=20
> 1. PTRACE_PPC_GETHWDEBUGINFO
>=20
> Query for GDB to discover the hardware debug features. The main info =
to
> be returned here is the minimum alignment for the hardware =
watchpoints.
> BookE processors don't have restrictions here, but server processors =
have
> an 8-byte alignment restriction for hardware watchpoints. We'd like to =
avoid
> adding special cases to GDB based on what it sees in AUXV.
>=20
> Since we're at it, we added other useful info that the kernel can =
return to
> GDB: this query will return the number of hardware breakpoints, =
hardware
> watchpoints and whether it supports a range of addresses and a =
condition.
> The query will fill the following structure provided by the requesting =
process:
>=20
> struct ppc_debug_info {
>       unit32_t version;
>       unit32_t num_instruction_bps;
>       unit32_t num_data_bps;
>       unit32_t num_condition_regs;
>       unit32_t data_bp_alignment;
>       unit32_t sizeof_condition; /* size of the DVC register */
>       uint64_t features; /* bitmask of the individual flags */
> };
>=20
> features will have bits indicating whether there is support for:
>=20
> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8

Is GDB smart enough to deal w/no condition_regs?  On some Book-E devices =
we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the features?

- k=

^ permalink raw reply

* Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface
From: Kumar Gala @ 2009-12-11  2:24 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155709.6697.4635.sendpatchset@norville.austin.ibm.com>


On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> These patches implement an extention to the ptrace interface proposed =
by
> Thiago Bauermann and the the PowerPC gdb team.
>=20
> GDB intends to support the following hardware debug features of BookE
> processors:
>=20
> 4 hardware breakpoints (IAC)
> 2 hardware watchpoints (read, write and read-write) (DAC)
> 2 value conditions for the hardware watchpoints (DVC)
>=20
> For that, we need to extend ptrace so that GDB can query and set these
> resources. Since we're extending, we're trying to create an interface
> that's extendable and that covers both BookE and server processors, so
> that GDB doesn't need to special-case each of them. We propose the
> following 3 new ptrace requests described below.
>=20
> There have been discussions of a generic hardware debug interface for =
the
> kernel which would hopefully contemplate all the functionality below =
and
> supersede it.  But we need something that works now, and which enables =
GDB
> to be simpler and work with both Server and Embedded processors =
without
> special cases.
>=20
> 1. PTRACE_PPC_GETHWDEBUGINFO
>=20
> Query for GDB to discover the hardware debug features. The main info =
to
> be returned here is the minimum alignment for the hardware =
watchpoints.
> BookE processors don't have restrictions here, but server processors =
have
> an 8-byte alignment restriction for hardware watchpoints. We'd like to =
avoid
> adding special cases to GDB based on what it sees in AUXV.
>=20
> Since we're at it, we added other useful info that the kernel can =
return to
> GDB: this query will return the number of hardware breakpoints, =
hardware
> watchpoints and whether it supports a range of addresses and a =
condition.
> The query will fill the following structure provided by the requesting =
process:
>=20
> struct ppc_debug_info {
>       unit32_t version;
>       unit32_t num_instruction_bps;
>       unit32_t num_data_bps;
>       unit32_t num_condition_regs;
>       unit32_t data_bp_alignment;
>       unit32_t sizeof_condition; /* size of the DVC register */
>       uint64_t features; /* bitmask of the individual flags */
> };
>=20
> features will have bits indicating whether there is support for:
>=20
> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
>=20
> 2. PTRACE_SETHWDEBUG
>=20
> Sets a hardware breakpoint or watchpoint, according to the provided =
structure:
>=20
> struct ppc_hw_breakpoint {
>        uint32_t version;
> #define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
> #define PPC_BREAKPOINT_TRIGGER_READ     0x2
> #define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
>        uint32_t trigger_type;       /* only some combinations allowed =
*/
> #define PPC_BREAKPOINT_MODE_EXACT               0x0
> #define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
> #define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
> #define PPC_BREAKPOINT_MODE_MASK                0x3
>        uint32_t addr_mode;          /* address match mode */
>=20
> #define PPC_BREAKPOINT_CONDITION_NONE   0x0
> #define PPC_BREAKPOINT_CONDITION_AND    0x1
> #define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for =
the same thing as above */
> #define PPC_BREAKPOINT_CONDITION_OR     0x2
> #define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
> #define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable =
bits */
> #define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
>        uint32_t condition_mode;     /* break/watchpoint condition =
flags */
>=20
>        uint64_t addr;
>        uint64_t addr2;
>        uint64_t condition_value;
> };
>=20
> A request specifies one event, not necessarily just one register to be =
set.
> For instance, if the request is for a watchpoint with a condition, =
both the
> DAC and DVC registers will be set in the same request.
>=20
> With this GDB can ask for all kinds of hardware breakpoints and =
watchpoints
> that the BookE supports. COMEFROM breakpoints available in server =
processors
> are not contemplated, but that is out of the scope of this work.
>=20
> ptrace will return an integer (handle) uniquely identifying the =
breakpoint or
> watchpoint just created. This integer will be used in the =
PTRACE_DELHWDEBUG
> request to ask for its removal. Return -ENOSPC if the requested =
breakpoint
> can't be allocated on the registers.
>=20
> Some examples of using the structure to:
>=20
> - set a breakpoint in the first breakpoint register
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>  p.addr            =3D (uint64_t) address;
>  p.addr2           =3D 0;
>  p.condition_value =3D 0;
>=20
> - set a watchpoint which triggers on reads in the second watchpoint =
register
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>  p.addr            =3D (uint64_t) address;
>  p.addr2           =3D 0;
>  p.condition_value =3D 0;
>=20
> - set a watchpoint which triggers only with a specific value
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_AND | =
PPC_BREAKPOINT_CONDITION_BE_ALL;
>  p.addr            =3D (uint64_t) address;
>  p.addr2           =3D 0;
>  p.condition_value =3D (uint64_t) condition;
>=20
> - set a ranged hardware breakpoint
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>  p.addr            =3D (uint64_t) begin_range;
>  p.addr2           =3D (uint64_t) end_range;
>  p.condition_value =3D 0;
>=20
> 3. PTRACE_DELHWDEBUG
>=20
> Takes an integer which identifies an existing breakpoint or watchpoint
> (i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
> corresponding breakpoint or watchpoint..

This is a good write up.  We should have it as a commit message for the =
first patch.

- k

^ permalink raw reply

* Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface
From: Dave Kleikamp @ 2009-12-11  2:27 UTC (permalink / raw)
  To: Kumar Gala
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <F0B42818-1EE1-47EE-B2F3-B7529B2E13C8@kernel.crashing.org>

On Thu, 2009-12-10 at 20:23 -0600, Kumar Gala wrote:
> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> 
> > These patches implement an extention to the ptrace interface proposed by
> > Thiago Bauermann and the the PowerPC gdb team.
> > 
> > GDB intends to support the following hardware debug features of BookE
> > processors:
> > 
> > 4 hardware breakpoints (IAC)
> > 2 hardware watchpoints (read, write and read-write) (DAC)
> > 2 value conditions for the hardware watchpoints (DVC)
> > 
> > For that, we need to extend ptrace so that GDB can query and set these
> > resources. Since we're extending, we're trying to create an interface
> > that's extendable and that covers both BookE and server processors, so
> > that GDB doesn't need to special-case each of them. We propose the
> > following 3 new ptrace requests described below.
> > 
> > There have been discussions of a generic hardware debug interface for the
> > kernel which would hopefully contemplate all the functionality below and
> > supersede it.  But we need something that works now, and which enables GDB
> > to be simpler and work with both Server and Embedded processors without
> > special cases.
> > 
> > 1. PTRACE_PPC_GETHWDEBUGINFO
> > 
> > Query for GDB to discover the hardware debug features. The main info to
> > be returned here is the minimum alignment for the hardware watchpoints.
> > BookE processors don't have restrictions here, but server processors have
> > an 8-byte alignment restriction for hardware watchpoints. We'd like to avoid
> > adding special cases to GDB based on what it sees in AUXV.
> > 
> > Since we're at it, we added other useful info that the kernel can return to
> > GDB: this query will return the number of hardware breakpoints, hardware
> > watchpoints and whether it supports a range of addresses and a condition.
> > The query will fill the following structure provided by the requesting process:
> > 
> > struct ppc_debug_info {
> >       unit32_t version;
> >       unit32_t num_instruction_bps;
> >       unit32_t num_data_bps;
> >       unit32_t num_condition_regs;
> >       unit32_t data_bp_alignment;
> >       unit32_t sizeof_condition; /* size of the DVC register */
> >       uint64_t features; /* bitmask of the individual flags */
> > };
> > 
> > features will have bits indicating whether there is support for:
> > 
> > #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> > #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> > #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> > #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
> 
> Is GDB smart enough to deal w/no condition_regs?  On some Book-E
> devices we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the
> features?

I had discussed it with the gdb team.  I could easily add a feature
flag, but it would be equivalent to num_condition_regs > 0.  I don't
have a strong opinion either way.
-- 
David Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface
From: Dave Kleikamp @ 2009-12-11  2:29 UTC (permalink / raw)
  To: Kumar Gala
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <D925FFC0-EF2B-4ED4-8481-38BC743E9770@kernel.crashing.org>

On Thu, 2009-12-10 at 20:24 -0600, Kumar Gala wrote:
> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> 
> > These patches implement an extention to the ptrace interface proposed by
> > Thiago Bauermann and the the PowerPC gdb team.
> > 
> > GDB intends to support the following hardware debug features of BookE
> > processors:
> > 
> > 4 hardware breakpoints (IAC)
> > 2 hardware watchpoints (read, write and read-write) (DAC)
> > 2 value conditions for the hardware watchpoints (DVC)
> > 
> > For that, we need to extend ptrace so that GDB can query and set these
> > resources. Since we're extending, we're trying to create an interface
> > that's extendable and that covers both BookE and server processors, so
> > that GDB doesn't need to special-case each of them. We propose the
> > following 3 new ptrace requests described below.
> > 
> > There have been discussions of a generic hardware debug interface for the
> > kernel which would hopefully contemplate all the functionality below and
> > supersede it.  But we need something that works now, and which enables GDB
> > to be simpler and work with both Server and Embedded processors without
> > special cases.
> > 
> > 1. PTRACE_PPC_GETHWDEBUGINFO
> > 
> > Query for GDB to discover the hardware debug features. The main info to
> > be returned here is the minimum alignment for the hardware watchpoints.
> > BookE processors don't have restrictions here, but server processors have
> > an 8-byte alignment restriction for hardware watchpoints. We'd like to avoid
> > adding special cases to GDB based on what it sees in AUXV.
> > 
> > Since we're at it, we added other useful info that the kernel can return to
> > GDB: this query will return the number of hardware breakpoints, hardware
> > watchpoints and whether it supports a range of addresses and a condition.
> > The query will fill the following structure provided by the requesting process:
> > 
> > struct ppc_debug_info {
> >       unit32_t version;
> >       unit32_t num_instruction_bps;
> >       unit32_t num_data_bps;
> >       unit32_t num_condition_regs;
> >       unit32_t data_bp_alignment;
> >       unit32_t sizeof_condition; /* size of the DVC register */
> >       uint64_t features; /* bitmask of the individual flags */
> > };
> > 
> > features will have bits indicating whether there is support for:
> > 
> > #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> > #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> > #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> > #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
> > 
> > 2. PTRACE_SETHWDEBUG
> > 
> > Sets a hardware breakpoint or watchpoint, according to the provided structure:
> > 
> > struct ppc_hw_breakpoint {
> >        uint32_t version;
> > #define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
> > #define PPC_BREAKPOINT_TRIGGER_READ     0x2
> > #define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
> >        uint32_t trigger_type;       /* only some combinations allowed */
> > #define PPC_BREAKPOINT_MODE_EXACT               0x0
> > #define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
> > #define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
> > #define PPC_BREAKPOINT_MODE_MASK                0x3
> >        uint32_t addr_mode;          /* address match mode */
> > 
> > #define PPC_BREAKPOINT_CONDITION_NONE   0x0
> > #define PPC_BREAKPOINT_CONDITION_AND    0x1
> > #define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for the same thing as above */
> > #define PPC_BREAKPOINT_CONDITION_OR     0x2
> > #define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
> > #define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable bits */
> > #define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
> >        uint32_t condition_mode;     /* break/watchpoint condition flags */
> > 
> >        uint64_t addr;
> >        uint64_t addr2;
> >        uint64_t condition_value;
> > };
> > 
> > A request specifies one event, not necessarily just one register to be set.
> > For instance, if the request is for a watchpoint with a condition, both the
> > DAC and DVC registers will be set in the same request.
> > 
> > With this GDB can ask for all kinds of hardware breakpoints and watchpoints
> > that the BookE supports. COMEFROM breakpoints available in server processors
> > are not contemplated, but that is out of the scope of this work.
> > 
> > ptrace will return an integer (handle) uniquely identifying the breakpoint or
> > watchpoint just created. This integer will be used in the PTRACE_DELHWDEBUG
> > request to ask for its removal. Return -ENOSPC if the requested breakpoint
> > can't be allocated on the registers.
> > 
> > Some examples of using the structure to:
> > 
> > - set a breakpoint in the first breakpoint register
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> >  p.addr            = (uint64_t) address;
> >  p.addr2           = 0;
> >  p.condition_value = 0;
> > 
> > - set a watchpoint which triggers on reads in the second watchpoint register
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_READ;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> >  p.addr            = (uint64_t) address;
> >  p.addr2           = 0;
> >  p.condition_value = 0;
> > 
> > - set a watchpoint which triggers only with a specific value
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_READ;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_AND | PPC_BREAKPOINT_CONDITION_BE_ALL;
> >  p.addr            = (uint64_t) address;
> >  p.addr2           = 0;
> >  p.condition_value = (uint64_t) condition;
> > 
> > - set a ranged hardware breakpoint
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> >  p.addr            = (uint64_t) begin_range;
> >  p.addr2           = (uint64_t) end_range;
> >  p.condition_value = 0;
> > 
> > 3. PTRACE_DELHWDEBUG
> > 
> > Takes an integer which identifies an existing breakpoint or watchpoint
> > (i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
> > corresponding breakpoint or watchpoint..
> 
> This is a good write up.  We should have it as a commit message for the first patch.

I have to give credit to Thiago for this.

Would it be worth adding to Documentation/powerpc/ ?
-- 
David Kleikamp
IBM Linux Technology Center

^ permalink raw reply

* Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface
From: Kumar Gala @ 2009-12-11  2:32 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <1260498575.4329.10.camel@norville.austin.ibm.com>


On Dec 10, 2009, at 8:29 PM, Dave Kleikamp wrote:

> On Thu, 2009-12-10 at 20:24 -0600, Kumar Gala wrote:
>> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
>>=20
>>> These patches implement an extention to the ptrace interface =
proposed by
>>> Thiago Bauermann and the the PowerPC gdb team.
>>>=20
>>> GDB intends to support the following hardware debug features of =
BookE
>>> processors:
>>>=20
>>> 4 hardware breakpoints (IAC)
>>> 2 hardware watchpoints (read, write and read-write) (DAC)
>>> 2 value conditions for the hardware watchpoints (DVC)
>>>=20
>>> For that, we need to extend ptrace so that GDB can query and set =
these
>>> resources. Since we're extending, we're trying to create an =
interface
>>> that's extendable and that covers both BookE and server processors, =
so
>>> that GDB doesn't need to special-case each of them. We propose the
>>> following 3 new ptrace requests described below.
>>>=20
>>> There have been discussions of a generic hardware debug interface =
for the
>>> kernel which would hopefully contemplate all the functionality below =
and
>>> supersede it.  But we need something that works now, and which =
enables GDB
>>> to be simpler and work with both Server and Embedded processors =
without
>>> special cases.
>>>=20
>>> 1. PTRACE_PPC_GETHWDEBUGINFO
>>>=20
>>> Query for GDB to discover the hardware debug features. The main info =
to
>>> be returned here is the minimum alignment for the hardware =
watchpoints.
>>> BookE processors don't have restrictions here, but server processors =
have
>>> an 8-byte alignment restriction for hardware watchpoints. We'd like =
to avoid
>>> adding special cases to GDB based on what it sees in AUXV.
>>>=20
>>> Since we're at it, we added other useful info that the kernel can =
return to
>>> GDB: this query will return the number of hardware breakpoints, =
hardware
>>> watchpoints and whether it supports a range of addresses and a =
condition.
>>> The query will fill the following structure provided by the =
requesting process:
>>>=20
>>> struct ppc_debug_info {
>>>      unit32_t version;
>>>      unit32_t num_instruction_bps;
>>>      unit32_t num_data_bps;
>>>      unit32_t num_condition_regs;
>>>      unit32_t data_bp_alignment;
>>>      unit32_t sizeof_condition; /* size of the DVC register */
>>>      uint64_t features; /* bitmask of the individual flags */
>>> };
>>>=20
>>> features will have bits indicating whether there is support for:
>>>=20
>>> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
>>> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
>>> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
>>> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
>>>=20
>>> 2. PTRACE_SETHWDEBUG
>>>=20
>>> Sets a hardware breakpoint or watchpoint, according to the provided =
structure:
>>>=20
>>> struct ppc_hw_breakpoint {
>>>       uint32_t version;
>>> #define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
>>> #define PPC_BREAKPOINT_TRIGGER_READ     0x2
>>> #define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
>>>       uint32_t trigger_type;       /* only some combinations allowed =
*/
>>> #define PPC_BREAKPOINT_MODE_EXACT               0x0
>>> #define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
>>> #define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
>>> #define PPC_BREAKPOINT_MODE_MASK                0x3
>>>       uint32_t addr_mode;          /* address match mode */
>>>=20
>>> #define PPC_BREAKPOINT_CONDITION_NONE   0x0
>>> #define PPC_BREAKPOINT_CONDITION_AND    0x1
>>> #define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for =
the same thing as above */
>>> #define PPC_BREAKPOINT_CONDITION_OR     0x2
>>> #define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
>>> #define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable =
bits */
>>> #define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
>>>       uint32_t condition_mode;     /* break/watchpoint condition =
flags */
>>>=20
>>>       uint64_t addr;
>>>       uint64_t addr2;
>>>       uint64_t condition_value;
>>> };
>>>=20
>>> A request specifies one event, not necessarily just one register to =
be set.
>>> For instance, if the request is for a watchpoint with a condition, =
both the
>>> DAC and DVC registers will be set in the same request.
>>>=20
>>> With this GDB can ask for all kinds of hardware breakpoints and =
watchpoints
>>> that the BookE supports. COMEFROM breakpoints available in server =
processors
>>> are not contemplated, but that is out of the scope of this work.
>>>=20
>>> ptrace will return an integer (handle) uniquely identifying the =
breakpoint or
>>> watchpoint just created. This integer will be used in the =
PTRACE_DELHWDEBUG
>>> request to ask for its removal. Return -ENOSPC if the requested =
breakpoint
>>> can't be allocated on the registers.
>>>=20
>>> Some examples of using the structure to:
>>>=20
>>> - set a breakpoint in the first breakpoint register
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>>> p.addr            =3D (uint64_t) address;
>>> p.addr2           =3D 0;
>>> p.condition_value =3D 0;
>>>=20
>>> - set a watchpoint which triggers on reads in the second watchpoint =
register
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>>> p.addr            =3D (uint64_t) address;
>>> p.addr2           =3D 0;
>>> p.condition_value =3D 0;
>>>=20
>>> - set a watchpoint which triggers only with a specific value
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_AND | =
PPC_BREAKPOINT_CONDITION_BE_ALL;
>>> p.addr            =3D (uint64_t) address;
>>> p.addr2           =3D 0;
>>> p.condition_value =3D (uint64_t) condition;
>>>=20
>>> - set a ranged hardware breakpoint
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>>> p.addr            =3D (uint64_t) begin_range;
>>> p.addr2           =3D (uint64_t) end_range;
>>> p.condition_value =3D 0;
>>>=20
>>> 3. PTRACE_DELHWDEBUG
>>>=20
>>> Takes an integer which identifies an existing breakpoint or =
watchpoint
>>> (i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
>>> corresponding breakpoint or watchpoint..
>>=20
>> This is a good write up.  We should have it as a commit message for =
the first patch.
>=20
> I have to give credit to Thiago for this.
>=20
> Would it be worth adding to Documentation/powerpc/ ?

That would also work.

- k=

^ permalink raw reply

* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: Kumar Gala @ 2009-12-11  2:41 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155721.6697.40863.sendpatchset@norville.austin.ibm.com>


On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> +#define DBCR1_IAC1US	0xC0000000	/* Instr Addr Cmp 1 Sup/User   =
*/
> +#define DBCR1_IAC1ER	0x30000000	/* Instr Addr Cmp 1 Eff/Real */
> +#define DBCR1_IAC1ER_01	0x10000000	/* reserved */
> +#define DBCR1_IAC1ER_10	0x20000000	/* Instr Addr Cmp 1 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC1ER_11	0x30000000	/* Instr Addr Cmp 1 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC2US	0x0C000000	/* Instr Addr Cmp 2 Sup/User   =
*/
> +#define DBCR1_IAC2ER	0x03000000	/* Instr Addr Cmp 2 Eff/Real */
> +#define DBCR1_IAC2ER_01	0x01000000	/* reserved */
> +#define DBCR1_IAC2ER_10	0x02000000	/* Instr Addr Cmp 2 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC2ER_11	0x03000000	/* Instr Addr Cmp 2 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC12M	0x00C00000	/* Instr Addr 1-2 range enable =
*/
> +#define DBCR1_IAC12M_R	0x00400000	/* Instr Addr 1-2 =
reserved state */
> +#define DBCR1_IAC12M_I	0x00800000	/* Instr Addr 1-2 range =
inclusive */
> +#define DBCR1_IAC12M_X	0x00C00000	/* Instr Addr 1-2 range =
eXclusive */
> +#define DBCR1_IAC12A_T	0x00010000	/* Instr Addr 1-2 range =
Toggle */
> +#define DBCR1_IAC3US	0x0000C000	/* Instr Addr Cmp 3 Sup/User   =
*/
> +#define DBCR1_IAC3ER	0x00003000	/* Instr Addr Cmp 3 Eff/Real */
> +#define DBCR1_IAC3ER_01	0x00001000	/* reserved */
> +#define DBCR1_IAC3ER_10	0x00002000	/* Instr Addr Cmp 3 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC3ER_11	0x00003000	/* Instr Addr Cmp 3 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC4US	0x00000C00	/* Instr Addr Cmp 4 Sup/User   =
*/
> +#define DBCR1_IAC4ER	0x00000300	/* Instr Addr Cmp 4 Eff/Real */
> +#define DBCR1_IAC4ER_01	0x00000100	/* Instr Addr Cmp 4 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC4ER_10	0x00000200	/* Instr Addr Cmp 4 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC4ER_11	0x00000300	/* Instr Addr Cmp 4 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC34M	0x000000C0	/* Instr Addr 3-4 range enable =
*/
> +#define DBCR1_IAC34M_R	0x00000040	/* Instr Addr 3-4 =
reserved state */
> +#define DBCR1_IAC34M_I	0x00000080	/* Instr Addr 3-4 range =
inclusive */
> +#define DBCR1_IAC34M_X	0x000000C0	/* Instr Addr 3-4 range =
eXclusive */
> +#define DBCR1_IAC34A_T	0x00000001	/* Instr Addr 3-4 range =
Toggle */
> +
> +#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
> +#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 | \
> +				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
> +				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
> +				 DBCR1_IAC4US | DBCR1_IAC4ER_10)

We are we using MSR[IS] IS=3D0, why not just any Eff address?  In the =
future we might have user as IS =3D 1, and kernel as IS =3D 0.

- k=

^ permalink raw reply

* Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface
From: Kumar Gala @ 2009-12-11  2:45 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155709.6697.4635.sendpatchset@norville.austin.ibm.com>


On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> These patches implement an extention to the ptrace interface proposed =
by
> Thiago Bauermann and the the PowerPC gdb team.
>=20
> GDB intends to support the following hardware debug features of BookE
> processors:
>=20
> 4 hardware breakpoints (IAC)
> 2 hardware watchpoints (read, write and read-write) (DAC)
> 2 value conditions for the hardware watchpoints (DVC)
>=20
> For that, we need to extend ptrace so that GDB can query and set these
> resources. Since we're extending, we're trying to create an interface
> that's extendable and that covers both BookE and server processors, so
> that GDB doesn't need to special-case each of them. We propose the
> following 3 new ptrace requests described below.
>=20
> There have been discussions of a generic hardware debug interface for =
the
> kernel which would hopefully contemplate all the functionality below =
and
> supersede it.  But we need something that works now, and which enables =
GDB
> to be simpler and work with both Server and Embedded processors =
without
> special cases.

What do we do in EDM mode?  We need a flag somewhere to determine if HW =
supports conveying DBCR0[EDM] and if it does which of the ptrace calls =
fails?

- k=

^ permalink raw reply

* Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
From: Kumar Gala @ 2009-12-11  2:50 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155727.6697.74672.sendpatchset@norville.austin.ibm.com>


On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
>=20
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>=20
> This patch defines context switch and trap related functionality
> for BookE specific Debug Registers. It adds support to ptrace()
> for setting and getting BookE related Debug Registers
>=20
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
>=20
> arch/powerpc/include/asm/system.h |    2=20
> arch/powerpc/kernel/process.c     |  109 ++++++++-
> arch/powerpc/kernel/ptrace.c      |  435 =
++++++++++++++++++++++++++++++++++---
> arch/powerpc/kernel/signal.c      |    6 -
> arch/powerpc/kernel/signal_32.c   |    8 +
> arch/powerpc/kernel/traps.c       |   86 ++++++-
> 6 files changed, 564 insertions(+), 82 deletions(-)
>=20
>=20
> diff --git a/arch/powerpc/include/asm/system.h =
b/arch/powerpc/include/asm/system.h
> index bb8e006..474bf23 100644
> --- a/arch/powerpc/include/asm/system.h
> +++ b/arch/powerpc/include/asm/system.h
> @@ -114,6 +114,8 @@ static inline int debugger_fault_handler(struct =
pt_regs *regs) { return 0; }
> extern int set_dabr(unsigned long dabr);
> extern void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code);
> +extern void do_send_trap(struct pt_regs *regs, unsigned long address,
> +			 unsigned long error_code, int signal_code, int =
errno);
> extern void print_backtrace(unsigned long *);
> extern void show_regs(struct pt_regs * regs);
> extern void flush_instruction_cache(void);
> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
> index c930ac3..a0dbb09 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -245,6 +245,24 @@ void discard_lazy_cpu_state(void)
> }
> #endif /* CONFIG_SMP */
>=20
> +void do_send_trap(struct pt_regs *regs, unsigned long address,
> +		  unsigned long error_code, int signal_code, int errno)
> +{
> +	siginfo_t info;
> +
> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> +		return;
> +
> +	/* Deliver the signal to userspace */
> +	info.si_signo =3D SIGTRAP;
> +	info.si_errno =3D errno;
> +	info.si_code =3D signal_code;
> +	info.si_addr =3D (void __user *)address;
> +	force_sig_info(SIGTRAP, &info, current);
> +}
> +
> +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code)
> {
> @@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long =
address,
> 	if (debugger_dabr_match(regs))
> 		return;
>=20
> -	/* Clear the DAC and struct entries.  One shot trigger */
> -#if defined(CONFIG_BOOKE)
> -	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> -							| DBCR0_IDM));
> -#endif
> -
> 	/* Clear the DABR */
> 	set_dabr(0);
>=20
> @@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long =
address,
> 	info.si_addr =3D (void __user *)address;
> 	force_sig_info(SIGTRAP, &info, current);
> }
> +#endif
>=20
> static DEFINE_PER_CPU(unsigned long, current_dabr);
>=20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +/*
> + * Set the debug registers back to their default "safe" values.
> + */
> +static void set_debug_reg_defaults(struct thread_struct *thread)
> +{
> +	thread->iac1 =3D thread->iac2 =3D thread->iac3 =3D thread->iac4 =
=3D 0;
> +	thread->dac1 =3D thread->dac2 =3D 0;
> +	thread->dvc1 =3D thread->dvc2 =3D 0;
> +	/*
> +	 * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
> +	 * the exception of the reserved bits should be cleared out
> +	 * and set to 0.
> +	 *
> +	 * For the DBCR0 register, the reserved bits are bits 17:30.
> +	 * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
> +	 * And, bits 10:11 for DBCR2.
> +	 */
> +	thread->dbcr0 =3D DBCR0_BASE_REG_VALUE;

This seems to always be 0, why have a special #define for it.

> +	/*
> +	 * First clear all "non reserved" bits from DBCR1 then =
initialize reg
> +	 * to force User/Supervisor bits to b11 (user-only MSR[PR]=3D1) =
and
> +	 * Effective/Real * bits to b10 (trap only if IS=3D=3D0)
> +	 */
> +	thread->dbcr1 =3D DBCR1_BASE_REG_VALUE;
> +	/*
> +	 * Force Data Address Compare User/Supervisor bits to be =
User-only
> +	 * (0b11 MSR[PR]=3D1) and set all other bits in DBCR2 register =
to be 0.
> +	 * This sets the Data Address Compare Effective/Real bits to be =
0b00
> +	 * (Effective, MSR[DS]=3Ddon't care).
> +	 */
> +	thread->dbcr2 =3D DBCR2_BASE_REG_VALUE;
> +}
> +
> +static void prime_debug_regs(struct thread_struct *thread)
> +{
> +	mtspr(SPRN_IAC1, thread->iac1);
> +	mtspr(SPRN_IAC2, thread->iac2);
> +	mtspr(SPRN_IAC3, thread->iac3);
> +	mtspr(SPRN_IAC4, thread->iac4);
> +	mtspr(SPRN_DAC1, thread->dac1);
> +	mtspr(SPRN_DAC2, thread->dac2);
> +	mtspr(SPRN_DVC1, thread->dvc1);
> +	mtspr(SPRN_DVC2, thread->dvc2);
> +	mtspr(SPRN_DBCR0, thread->dbcr0);
> +	mtspr(SPRN_DBCR1, thread->dbcr1);
> +	mtspr(SPRN_DBCR2, thread->dbcr2);

We should probably look at dbginfo.num_condition_regs, =
dbginfo.num_instruction_bps, & dbginfo.num_data_bps and set these =
accordingly.

> +}
> +/*
> + * Unless neither the old or new thread are making use of the
> + * debug registers, set the debug registers from the values
> + * stored in the new thread.
> + */
> +static void switch_booke_debug_regs(struct thread_struct *new_thread)
> +{
> +	if ((current->thread.dbcr0 & DBCR0_IDM)
> +		|| (new_thread->dbcr0 & DBCR0_IDM))
> +			prime_debug_regs(new_thread);
> +}
> +#endif
> +
> int set_dabr(unsigned long dabr)
> {
> 	__get_cpu_var(current_dabr) =3D dabr;
> @@ -284,7 +358,7 @@ int set_dabr(unsigned long dabr)
> 		return ppc_md.set_dabr(dabr);
>=20
> 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
> -#if defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> 	mtspr(SPRN_DAC1, dabr);
> #elif defined(CONFIG_PPC_BOOK3S)
> 	mtspr(SPRN_DABR, dabr);
> @@ -371,10 +445,8 @@ struct task_struct *__switch_to(struct =
task_struct *prev,
>=20
> #endif /* CONFIG_SMP */
>=20
> -#if defined(CONFIG_BOOKE)
> -	/* If new thread DAC (HW breakpoint) is the same then leave it =
*/
> -	if (new->thread.dabr)
> -		set_dabr(new->thread.dabr);
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	switch_booke_debug_regs(&new->thread);
> #else
> 	if (unlikely(__get_cpu_var(current_dabr) !=3D new->thread.dabr))
> 		set_dabr(new->thread.dabr);
> @@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
> 	printk("  CR: %08lx  XER: %08lx\n", regs->ccr, regs->xer);
> 	trap =3D TRAP(regs);
> 	if (trap =3D=3D 0x300 || trap =3D=3D 0x600)
> -#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, =
regs->dsisr);
> #else
> 		printk("DAR: "REG", DSISR: "REG"\n", regs->dar, =
regs->dsisr);
> @@ -568,14 +640,19 @@ void flush_thread(void)
>=20
> 	discard_lazy_cpu_state();
>=20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	/*
> +	 * flush_thread() is called on exec() to reset the
> +	 * thread's status. Set all debug regs back to their
> +	 * default values....Torez
> +	 */
> +	set_debug_reg_defaults(&current->thread);
> +#else
> 	if (current->thread.dabr) {
> 		current->thread.dabr =3D 0;
> 		set_dabr(0);
> -
> -#if defined(CONFIG_BOOKE)
> -		current->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W);
> -#endif
> 	}
> +#endif
> }
>=20
> void
> diff --git a/arch/powerpc/kernel/ptrace.c =
b/arch/powerpc/kernel/ptrace.c
> index 6be2ce0..6710a69 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -737,17 +737,25 @@ void user_disable_single_step(struct task_struct =
*task)
> 	struct pt_regs *regs =3D task->thread.regs;
>=20
> 	if (regs !=3D NULL) {
> -#if defined(CONFIG_BOOKE)
> -		/* If DAC don't clear DBCRO_IDM or MSR_DE */
> -		if (task->thread.dabr)
> -			task->thread.dbcr0 &=3D ~(DBCR0_IC | DBCR0_BT);
> -		else {
> -			task->thread.dbcr0 &=3D ~(DBCR0_IC | DBCR0_BT | =
DBCR0_IDM);
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +		/*
> +		 * The logic to disable single stepping should be as
> +		 * simple as turning off the Instruction Complete flag.
> +		 * And, after doing so, if all debug flags are off, turn
> +		 * off DBCR0(IDM) and MSR(DE) .... Torez
> +		 */
> +		task->thread.dbcr0 &=3D ~DBCR0_IC;
> +		/*
> +		 * Test to see if any of the DBCR_ACTIVE_EVENTS bits are =
set.
> +		 */
> +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> +					task->thread.dbcr1)) {
> +			/*
> +			 * All debug events were off.....
> +			 */
> +			task->thread.dbcr0 &=3D ~DBCR0_IDM;
> 			regs->msr &=3D ~MSR_DE;
> 		}
> -#elif defined(CONFIG_40x)
> -		task->thread.dbcr0 &=3D ~(DBCR0_IC | DBCR0_BT | =
DBCR0_IDM);
> -		regs->msr &=3D ~MSR_DE;
> #else
> 		regs->msr &=3D ~(MSR_SE | MSR_BE);
> #endif
> @@ -769,8 +777,7 @@ int ptrace_set_debugreg(struct task_struct *task, =
unsigned long addr,
> 	if ((data & ~0x7UL) >=3D TASK_SIZE)
> 		return -EIO;
>=20
> -#ifndef CONFIG_BOOKE
> -
> +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> 	/* For processors using DABR (i.e. 970), the bottom 3 bits are =
flags.
> 	 *  It was assumed, on previous implementations, that 3 bits =
were
> 	 *  passed together with the data address, fitting the design of =
the
> @@ -789,21 +796,22 @@ int ptrace_set_debugreg(struct task_struct =
*task, unsigned long addr,
>=20
> 	/* Move contents to the DABR register */
> 	task->thread.dabr =3D data;
> -
> -#endif
> -#if defined(CONFIG_BOOKE)
> -
> +#else
> 	/* As described above, it was assumed 3 bits were passed with =
the data
> 	 *  address, but we will assume only the mode bits will be =
passed
> 	 *  as to not cause alignment restrictions for DAC-based =
processors.
> 	 */
>=20
> 	/* DAC's hold the whole address without any mode flags */
> -	task->thread.dabr =3D data & ~0x3UL;
> -
> -	if (task->thread.dabr =3D=3D 0) {
> -		task->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | =
DBCR0_IDM);
> -		task->thread.regs->msr &=3D ~MSR_DE;
> +	task->thread.dac1 =3D data & ~0x3UL;
> +
> +	if (task->thread.dac1 =3D=3D 0) {
> +		dbcr_dac(task) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> +					task->thread.dbcr1)) {
> +			task->thread.regs->msr &=3D ~MSR_DE;
> +			task->thread.dbcr0 &=3D ~DBCR0_IDM;
> +		}
> 		return 0;
> 	}
>=20
> @@ -814,15 +822,15 @@ int ptrace_set_debugreg(struct task_struct =
*task, unsigned long addr,
>=20
> 	/* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
> 	   register */
> -	task->thread.dbcr0 =3D DBCR0_IDM;
> +	task->thread.dbcr0 |=3D DBCR0_IDM;
>=20
> 	/* Check for write and read flags and set DBCR0
> 	   accordingly */
> +	dbcr_dac(task) &=3D ~(DBCR_DAC1R|DBCR_DAC1W);
> 	if (data & 0x1UL)
> -		task->thread.dbcr0 |=3D DBSR_DAC1R;
> +		dbcr_dac(task) |=3D DBCR_DAC1R;
> 	if (data & 0x2UL)
> -		task->thread.dbcr0 |=3D DBSR_DAC1W;
> -
> +		dbcr_dac(task) |=3D DBCR_DAC1W;
> 	task->thread.regs->msr |=3D MSR_DE;
> #endif
> 	return 0;
> @@ -839,11 +847,324 @@ void ptrace_disable(struct task_struct *child)
> 	user_disable_single_step(child);
> }
>=20
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +static long set_intruction_bp(struct task_struct *child,
> +			      struct ppc_hw_breakpoint *bp_info)
> +{
> +	int slots_needed;
> +	int slot;
> +	int free_slot =3D 0;
> +
> +	/*
> +	 * Find an avalailable slot for the breakpoint.
> +	 * If possible, reserve consecutive slots, 1 & 2, for a range
> +	 * breakpoint.  (Can this be done simpler?)
> +	 */
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		slots_needed =3D 1;
> +	else
> +		slots_needed =3D 2;
> +
> +	if ((child->thread.dbcr0 & DBCR0_IAC1) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			if (child->thread.dbcr0 & DBCR0_IAC2) {
> +				slot =3D 1;
> +				goto found;
> +			}
> +			/* Try to save slots 1 & 2 for range */
> +			free_slot =3D 1;
> +		} else
> +			if ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0) =
{
> +				slot =3D 1;
> +				goto found;
> +			}
> +	} else if ((slots_needed =3D=3D 1) &&
> +		   ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0)) {
> +		slot =3D 2;
> +		goto found;
> +	}
> +	if ((child->thread.dbcr0 & DBCR0_IAC3) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		return -ENOSPC;
> +	} else if (slots_needed =3D=3D 2)
> +		return -ENOSPC;
> +	if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +		slot =3D 4;
> +	} else if (free_slot)
> +		slot =3D free_slot;
> +	else
> +		return -ENOSPC;

Need to factor in if # of IACs is only 2.

> +found:
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT) {
> +		switch (slot) {
> +			case 1:
> +				child->thread.iac1 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC1;
> +				break;
> +			case 2:
> +				child->thread.iac2 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC2;
> +				break;
> +			case 3:
> +				child->thread.iac3 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC3;
> +				break;
> +			case 4:
> +				child->thread.iac4 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC4;
> +				break;
> +		}
> +	} else if (slot =3D=3D 1) {
> +		child->thread.iac1 =3D bp_info->addr;
> +		child->thread.iac2 =3D bp_info->addr2;
> +		child->thread.dbcr0 |=3D (DBCR0_IAC1 | DBCR0_IAC2);
> +		if (bp_info->addr_mode =3D=3D =
PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_I;
> +	} else { /* slot =3D=3D 3 */
> +		child->thread.iac3 =3D bp_info->addr;
> +		child->thread.iac4 =3D bp_info->addr2;
> +		child->thread.dbcr0 |=3D (DBCR0_IAC3 | DBCR0_IAC4);
> +		if (bp_info->addr_mode =3D=3D =
PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_I;
> +	}
> +	child->thread.dbcr0 |=3D DBCR0_IDM;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return slot;
> +}
> +
> +static int del_instruction_bp(struct task_struct *child, int slot)
> +{
> +	switch (slot) {
> +	case 1:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M) {
> +			/* address range - clear slots 1 & 2 */
> +			child->thread.iac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC12M;
> +		}
> +		child->thread.iac1 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC1;
> +		break;
> +	case 2:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M)
> +			/* used in a range */
> +			return -EINVAL;
> +		child->thread.iac2 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +		break;
> +	case 3:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M) {
> +			/* address range - clear slots 3 & 4 */
> +			child->thread.iac4 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC34M;
> +		}
> +		child->thread.iac3 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC3;
> +		break;
> +	case 4:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M)
> +			/* Used in a range */
> +			return -EINVAL;
> +		child->thread.iac4 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int set_dac(struct task_struct *child, struct =
ppc_hw_breakpoint *bp_info)
> +{
> +	int byte_enable =3D
> +		(bp_info->condition_mode >> =
PPC_BREAKPOINT_CONDITION_BE_SHIFT)
> +		& 0xf;
> +	int condition_mode =3D
> +		bp_info->condition_mode & =
PPC_BREAKPOINT_CONDITION_AND_OR;
> +	int slot;
> +
> +	if (byte_enable && (condition_mode =3D=3D 0))
> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) =3D=3D 0) {
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +			dbcr_dac(child) |=3D DBCR_DAC1R;
> +		if (bp_info->trigger_type & =
PPC_BREAKPOINT_TRIGGER_WRITE)
> +			dbcr_dac(child) |=3D DBCR_DAC1W;
> +		child->thread.dac1 =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE
> +		if (byte_enable) {
> +			child->thread.dvc1 =3D
> +				(unsigned long)bp_info->condition_value;
> +			child->thread.dbcr2 |=3D
> +				((byte_enable << DBCR2_DVC1BE_SHIFT) |
> +				 (condition_mode << DBCR2_DVC1M_SHIFT));
> +		}
> +#endif
> +		slot =3D 1;
> +	} else if ((dbcr_dac(child) & (DBCR_DAC2R | DBCR_DAC2W)) =3D=3D =
0) {
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +			dbcr_dac(child) |=3D DBCR_DAC2R;
> +		if (bp_info->trigger_type & =
PPC_BREAKPOINT_TRIGGER_WRITE)
> +			dbcr_dac(child) |=3D DBCR_DAC2W;
> +		child->thread.dac2 =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE
> +		if (byte_enable) {
> +			child->thread.dvc2 =3D
> +				(unsigned long)bp_info->condition_value;
> +			child->thread.dbcr2 |=3D
> +				((byte_enable << DBCR2_DVC2BE_SHIFT) |
> +				 (condition_mode << DBCR2_DVC2M_SHIFT));
> +		}
> +#endif
> +		slot =3D 2;
> +	} else
> +		return -ENOSPC;
> +	child->thread.dbcr0 |=3D DBCR0_IDM;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return slot + 4;
> +}
> +
> +static int del_dac(struct task_struct *child, int slot)
> +{
> +	if (slot =3D=3D 1) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
> +			child->thread.dac1 =3D 0;
> +			child->thread.dac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~(DBCR0_DAC1R | =
DBCR0_DAC1W |
> +						 DBCR0_DAC2R | =
DBCR0_DAC2W);
> +			child->thread.dbcr2 &=3D ~DBCR2_DAC12MODE;
> +			return 0;
> +		}
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC1M | DBCR2_DVC1BE);
> +		child->thread.dvc1 =3D 0;
> +#endif
> +		child->thread.dac1 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +	} else if (slot =3D=3D 2) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE)
> +			/* Part of a range */
> +			return -EINVAL;
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC2M | DBCR2_DVC2BE);
> +		child->thread.dvc2 =3D 0;
> +#endif
> +		child->thread.dac2 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC2R | DBCR_DAC2W);
> +	} else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +#endif /* CONFIG_40x || CONFIG_BOOKE */
> +
> +#ifdef CONFIG_BOOKE
> +static int set_dac_range(struct task_struct *child,
> +			 struct ppc_hw_breakpoint *bp_info)
> +{
> +	int mode =3D bp_info->addr_mode & PPC_BREAKPOINT_MODE_MASK;
> +
> +	/* We don't allow range watchpoints to be used with DVC */
> +	if (bp_info->condition_mode && PPC_BREAKPOINT_CONDITION_BE_ALL)
> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if (child->thread.dbcr0 &
> +	    (DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W))
> +		return -ENOSPC;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1R | DBCR0_DAC2R | =
DBCR0_IDM);
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1W | DBCR0_DAC2W | =
DBCR0_IDM);
> +	child->thread.dac1 =3D bp_info->addr;
> +	child->thread.dac2 =3D bp_info->addr2;
> +	if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12R;
> +	else if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12RX;
> +	else	/* PPC_BREAKPOINT_MODE_MASK */
> +		child->thread.dbcr2  |=3D DBCR2_DAC12MASK;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return 5;
> +}
> +#endif /* CONFIG_BOOKE */
> +
> static long ppc_set_hwdebug(struct task_struct *child,
> 		     struct ppc_hw_breakpoint *bp_info)
> {
> +	if (bp_info->version !=3D 1)
> +		return -ENOTSUPP;
> +
> +#ifdef CONFIG_BOOKE
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode &
> +	     ~(PPC_BREAKPOINT_CONDITION_AND_OR |
> +	       PPC_BREAKPOINT_CONDITION_BE_ALL)))
> +		return -EINVAL;

We should add a sanity check for bp_info->condition_mode !=3D =
PPC_BREAKPOINT_CONDITION_NONE if dbginfo.num_condition_regs =3D 0.

> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D =
PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return set_dac(child, bp_info);
> +
> +	return set_dac_range(child, bp_info);
> +#elif defined(CONFIG_40x)
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode !=3D =
PPC_BREAKPOINT_CONDITION_NONE))
> +		return -EINVAL;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D =
PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +	if (bp_info->addr_mode !=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return -EINVAL;
> +
> +	return set_dac(child, bp_info);
> +#else
> 	/*
> -	 * We currently support one data breakpoint
> +	 * We only support one data breakpoint
> 	 */
> 	if (((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) =3D=3D =
0) ||
> 	    ((bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) !=3D =
0) ||
> @@ -859,28 +1180,35 @@ static long ppc_set_hwdebug(struct task_struct =
*child,
> 		return -EIO;
>=20
> 	child->thread.dabr =3D (unsigned long)bp_info->addr;
> -#ifdef CONFIG_BOOKE
> -	child->thread.dbcr0 =3D DBCR0_IDM;
> -	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> -		child->thread.dbcr0 |=3D DBSR_DAC1R;
> -	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> -		child->thread.dbcr0 |=3D DBSR_DAC1W;
> -	child->thread.regs->msr |=3D MSR_DE;
> -#endif
> 	return 1;
> +#endif
> }
>=20
> static long ppc_del_hwdebug(struct task_struct *child, long addr, long =
data)
> {
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +	int rc;
> +
> +	if (data <=3D 4)
> +		rc =3D del_instruction_bp(child, (int)data);
> +	else
> +		rc =3D del_dac(child, (int)data - 4);
> +
> +	if (!rc) {
> +		if (!DBCR_ACTIVE_EVENTS(child->thread.dbcr0,
> +					child->thread.dbcr1)) {
> +			child->thread.dbcr0 &=3D ~DBCR0_IDM;
> +			child->thread.regs->msr &=3D ~MSR_DE;
> +		}
> +	}
> +	return rc;
> +#else
> 	if ((data !=3D 1) || (child->thread.dabr =3D=3D 0))
> 		return -EINVAL;
>=20
> 	child->thread.dabr =3D 0;
> -#ifdef CONFIG_BOOKE
> -	child->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> -	child->thread.regs->msr &=3D ~MSR_DE;
> -#endif
> 	return 0;
> +#endif
> }
>=20
> /*
> @@ -980,16 +1308,36 @@ long arch_ptrace(struct task_struct *child, =
long request, long addr, long data)
> 		struct ppc_debug_info dbginfo;
>=20
> 		dbginfo.version =3D 1;
> +#ifdef CONFIG_BOOKE
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 2;
> +		dbginfo.data_bp_alignment =3D 0;
> +		dbginfo.sizeof_condition =3D 4;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK |
> +				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
> +#elif defined(CONFIG_40x)
> +		/*
> +		 * I don't know how the DVCs work on 40x, I'm not going
> +		 * to support it now. -- Shaggy
> +		 */
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 0;
> +		dbginfo.data_bp_alignment =3D 0;
> +		dbginfo.sizeof_condition =3D 0;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK;
> +#else
> 		dbginfo.num_instruction_bps =3D 0;
> 		dbginfo.num_data_bps =3D 1;
> 		dbginfo.num_condition_regs =3D 0;
> -#ifdef CONFIG_PPC64
> 		dbginfo.data_bp_alignment =3D 8;
> -#else
> -		dbginfo.data_bp_alignment =3D 0;
> -#endif
> 		dbginfo.sizeof_condition =3D 0;
> 		dbginfo.features =3D 0;
> +#endif

This is a bit ugly and BOOKE 64 parts probably don't have the 8 byte =
alignment.

Should we push some of this into cputable?

>=20
> 		if (!access_ok(VERIFY_WRITE, data,
> 			       sizeof(struct ppc_debug_info)))
> @@ -1025,8 +1373,13 @@ long arch_ptrace(struct task_struct *child, =
long request, long addr, long data)
> 		/* We only support one DABR and no IABRS at the moment =
*/
> 		if (addr > 0)
> 			break;
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +		ret =3D put_user(child->thread.dac1,
> +			       (unsigned long __user *)data);
> +#else
> 		ret =3D put_user(child->thread.dabr,
> 			       (unsigned long __user *)data);
> +#endif
> 		break;
> 	}
>=20
> diff --git a/arch/powerpc/kernel/signal.c =
b/arch/powerpc/kernel/signal.c
> index 00b5078..94df779 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -140,17 +140,15 @@ static int do_signal_pending(sigset_t *oldset, =
struct pt_regs *regs)
> 		return 0;               /* no signals delivered */
> 	}
>=20
> +#if !(defined(CONFIG_BOOKE) || defined(CONFIG_40x))
>         /*
> 	 * Reenable the DABR before delivering the signal to
> 	 * user space. The DABR will have been cleared if it
> 	 * triggered inside the kernel.
> 	 */
> -	if (current->thread.dabr) {
> +	if (current->thread.dabr)
> 		set_dabr(current->thread.dabr);
> -#if defined(CONFIG_BOOKE)
> -		mtspr(SPRN_DBCR0, current->thread.dbcr0);
> #endif
> -	}
>=20
> 	if (is32) {
>         	if (ka.sa.sa_flags & SA_SIGINFO)
> diff --git a/arch/powerpc/kernel/signal_32.c =
b/arch/powerpc/kernel/signal_32.c
> index d670429..6cc6e81 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -1092,8 +1092,12 @@ int sys_debug_setcontext(struct ucontext __user =
*ctx,
> 				new_msr |=3D MSR_DE;
> 				new_dbcr0 |=3D (DBCR0_IDM | DBCR0_IC);
> 			} else {
> -				new_msr &=3D ~MSR_DE;
> -				new_dbcr0 &=3D ~(DBCR0_IDM | DBCR0_IC);
> +				new_dbcr0 &=3D ~DBCR0_IC;
> +				if (!DBCR_ACTIVE_EVENTS(new_dbcr0,
> +						current->thread.dbcr1)) =
{
> +					new_msr &=3D ~MSR_DE;
> +					new_dbcr0 &=3D ~DBCR0_IDM;
> +				}
> 			}
> #else
> 			if (op.dbg_value)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index a81c743..d919571 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1016,9 +1016,63 @@ void SoftwareEmulation(struct pt_regs *regs)
> #endif /* CONFIG_8xx */
>=20
> #if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +static void handle_debug(struct pt_regs *regs, unsigned long =
debug_status)
> +{
> +	int changed =3D 0;
> +	/*
> +	 * Determine the cause of the debug event, clear the
> +	 * event flags and send a trap to the handler. Torez
> +	 */
> +	if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
> +		dbcr_dac(current) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +		do_send_trap(regs, mfspr(SPRN_DAC1), debug_status, =
TRAP_HWBKPT,
> +			     5);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & (DBSR_DAC2R | DBSR_DAC2W)) {
> +		dbcr_dac(current) &=3D ~(DBCR_DAC2R | DBCR_DAC2W);
> +		do_send_trap(regs, mfspr(SPRN_DAC2), debug_status, =
TRAP_HWBKPT,
> +			     6);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC1) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC1;
> +		do_send_trap(regs, mfspr(SPRN_IAC1), debug_status, =
TRAP_HWBKPT,
> +			     1);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC2) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +		do_send_trap(regs, mfspr(SPRN_IAC2), debug_status, =
TRAP_HWBKPT,
> +			     2);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC3) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC3;
> +		do_send_trap(regs, mfspr(SPRN_IAC3), debug_status, =
TRAP_HWBKPT,
> +			     3);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC4) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +		do_send_trap(regs, mfspr(SPRN_IAC4), debug_status, =
TRAP_HWBKPT,
> +			     4);
> +		changed |=3D 0x01;
> +	}
> +	/*
> +	 * At the point this routine was called, the MSR(DE) was turned =
off.
> +	 * Check all other debug flags and see if that bit needs to be =
turned
> +	 * back on or not.
> +	 */
> +	if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0, =
current->thread.dbcr1))
> +		regs->msr |=3D MSR_DE;
> +	else
> +		/* Make sure the IDM flag is off */
> +		current->thread.dbcr0 &=3D ~DBCR0_IDM;
> +
> +	if (changed & 0x01)
> +		mtspr(SPRN_DBCR0, current->thread.dbcr0);
> +}
>=20
> void __kprobes DebugException(struct pt_regs *regs, unsigned long =
debug_status)
> {
> +	current->thread.dbsr =3D debug_status;
> +
> 	/* Hack alert: On BookE, Branch Taken stops on the branch =
itself, while
> 	 * on server, it stops on the target of the branch. In order to =
simulate
> 	 * the server behaviour, we thus restart right away with a =
single step
> @@ -1062,27 +1116,21 @@ void __kprobes DebugException(struct pt_regs =
*regs, unsigned long debug_status)
> 		if (debugger_sstep(regs))
> 			return;
>=20
> -		if (user_mode(regs))
> -			current->thread.dbcr0 &=3D ~(DBCR0_IC);
> -
> -		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
> -	} else if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
> -		regs->msr &=3D ~MSR_DE;
> -
> 		if (user_mode(regs)) {
> -			current->thread.dbcr0 &=3D ~(DBSR_DAC1R | =
DBSR_DAC1W |
> -								=
DBCR0_IDM);
> -		} else {
> -			/* Disable DAC interupts */
> -			mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & =
~(DBSR_DAC1R |
> -						DBSR_DAC1W | =
DBCR0_IDM));
> -
> -			/* Clear the DAC event */
> -			mtspr(SPRN_DBSR, (DBSR_DAC1R | DBSR_DAC1W));
> +			current->thread.dbcr0 &=3D ~DBCR0_IC;
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +			if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0,
> +					       current->thread.dbcr1))
> +				regs->msr |=3D MSR_DE;
> +			else
> +				/* Make sure the IDM bit is off */
> +				current->thread.dbcr0 &=3D ~DBCR0_IDM;
> +#endif
> 		}
> -		/* Setup and send the trap to the handler */
> -		do_dabr(regs, mfspr(SPRN_DAC1), debug_status);
> -	}
> +
> +		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
> +	} else
> +		handle_debug(regs, debug_status);
> }
> #endif /* CONFIG_4xx || CONFIG_BOOKE */
>=20
>=20
> --=20
> Dave Kleikamp
> IBM Linux Technology Center
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [RFC:PATCH 01/03] powerpc: Extended ptrace interface
From: Kumar Gala @ 2009-12-11  2:51 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann, David Gibson
In-Reply-To: <20091210155715.6697.92627.sendpatchset@norville.austin.ibm.com>


On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> powerpc: Extended ptrace interface
>=20
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>=20
> Add a new extended ptrace interface so that user-space has a single
> interface for powerpc, without having to know the specific layout
> of the debug registers.
>=20
> Implement:
> PPC_PTRACE_GETHWDEBUGINFO
> PPC_PTRACE_SETHWDEBUG
> PPC_PTRACE_DELHWDEBUG
>=20
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
>=20
> arch/powerpc/include/asm/ptrace.h |   75 =
++++++++++++++++++++++++++++++++
> arch/powerpc/kernel/ptrace.c      |   88 =
+++++++++++++++++++++++++++++++++++++
> 2 files changed, 163 insertions(+), 0 deletions(-)
>=20
>=20
> diff --git a/arch/powerpc/include/asm/ptrace.h =
b/arch/powerpc/include/asm/ptrace.h
> index 8c34149..7ae887b 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -24,6 +24,12 @@
>  * 2 of the License, or (at your option) any later version.
>  */
>=20
> +#ifdef __KERNEL__
> +#include <linux/types.h>
> +#else
> +#include <stdint.h>
> +#endif
> +
> #ifndef __ASSEMBLY__
>=20
> struct pt_regs {
> @@ -292,4 +298,73 @@ extern void user_disable_single_step(struct =
task_struct *);
>=20
> #define PTRACE_SINGLEBLOCK	0x100	/* resume execution until next =
branch */
>=20
> +#define PPC_PTRACE_GETHWDBGINFO	0x89
> +#define PPC_PTRACE_SETHWDEBUG	0x88
> +#define PPC_PTRACE_DELHWDEBUG	0x87
> +
> +#ifndef __ASSEMBLY__
> +
> +struct ppc_debug_info {
> +	uint32_t version;		/* Only version 1 exists to date =
*/
> +	uint32_t num_instruction_bps;
> +	uint32_t num_data_bps;
> +	uint32_t num_condition_regs;
> +	uint32_t data_bp_alignment;
> +	uint32_t sizeof_condition;	/* size of the DVC register */
> +	uint64_t features;
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +/*
> + * features will have bits indication whether there is support for:
> + */
> +#define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> +#define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> +#define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> +#define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8

Pad these out.

#define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x0000000000000001
#define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x0000000000000002

etc..


> +
> +#ifndef __ASSEMBLY__
> +
> +struct ppc_hw_breakpoint {
> +	uint32_t version;		/* currently, version must be 1 =
*/
> +	uint32_t trigger_type;		/* only some combinations =
allowed */
> +	uint32_t addr_mode;		/* address match mode */
> +	uint32_t condition_mode;	/* break/watchpoint condition =
flags */
> +	uint64_t addr;			/* break/watchpoint address */
> +	uint64_t addr2;			/* range end or mask */
> +	uint64_t condition_value;	/* contents of the DVC register =
*/
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +/*
> + * Trigger Type
> + */
> +#define PPC_BREAKPOINT_TRIGGER_EXECUTE	0x1
> +#define PPC_BREAKPOINT_TRIGGER_READ	0x2
> +#define PPC_BREAKPOINT_TRIGGER_WRITE	0x4
> +#define PPC_BREAKPOINT_TRIGGER_RW	0x6

(ditto on the padding)

> +
> +/*
> + * Address Mode
> + */
> +#define PPC_BREAKPOINT_MODE_EXACT		0x0
> +#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE	0x1
> +#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE	0x2
> +#define PPC_BREAKPOINT_MODE_MASK		0x3
> +

(ditto on the padding)

> +/*
> + * Condition Mode
> + */
> +#define PPC_BREAKPOINT_CONDITION_NONE	0x0
> +#define PPC_BREAKPOINT_CONDITION_AND	0x1
> +#define PPC_BREAKPOINT_CONDITION_EXACT	0x1
> +#define PPC_BREAKPOINT_CONDITION_OR	0x2
> +#define PPC_BREAKPOINT_CONDITION_AND_OR	0x3

(ditto on the padding)

> +#define PPC_BREAKPOINT_CONDITION_BE_ALL	0x00ff0000
> +#define PPC_BREAKPOINT_CONDITION_BE_SHIFT	16
> +#define PPC_BREAKPOINT_CONDITION_BE(n)	\
> +	(1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
> +
> #endif /* _ASM_POWERPC_PTRACE_H */
> diff --git a/arch/powerpc/kernel/ptrace.c =
b/arch/powerpc/kernel/ptrace.c
> index ef14988..6be2ce0 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -839,6 +839,50 @@ void ptrace_disable(struct task_struct *child)
> 	user_disable_single_step(child);
> }
>=20
> +static long ppc_set_hwdebug(struct task_struct *child,
> +		     struct ppc_hw_breakpoint *bp_info)
> +{
> +	/*
> +	 * We currently support one data breakpoint
> +	 */
> +	if (((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) =3D=3D =
0) ||
> +	    ((bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) !=3D =
0) ||
> +	    (bp_info->trigger_type !=3D PPC_BREAKPOINT_TRIGGER_WRITE) ||
> +	    (bp_info->addr_mode !=3D PPC_BREAKPOINT_MODE_EXACT) ||
> +	    (bp_info->condition_mode !=3D =
PPC_BREAKPOINT_CONDITION_NONE))
> +		return -EINVAL;
> +
> +	if (child->thread.dabr)
> +		return -ENOSPC;
> +
> +	if ((unsigned long)bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	child->thread.dabr =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE
> +	child->thread.dbcr0 =3D DBCR0_IDM;
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +		child->thread.dbcr0 |=3D DBSR_DAC1R;
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +		child->thread.dbcr0 |=3D DBSR_DAC1W;
> +	child->thread.regs->msr |=3D MSR_DE;
> +#endif
> +	return 1;
> +}
> +
> +static long ppc_del_hwdebug(struct task_struct *child, long addr, =
long data)
> +{
> +	if ((data !=3D 1) || (child->thread.dabr =3D=3D 0))
> +		return -EINVAL;
> +
> +	child->thread.dabr =3D 0;
> +#ifdef CONFIG_BOOKE
> +	child->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> +	child->thread.regs->msr &=3D ~MSR_DE;
> +#endif
> +	return 0;
> +}
> +
> /*
>  * Here are the old "legacy" powerpc specific getregs/setregs ptrace =
calls,
>  * we mark them as obsolete now, they will be removed in a future =
version
> @@ -932,6 +976,50 @@ long arch_ptrace(struct task_struct *child, long =
request, long addr, long data)
> 		break;
> 	}
>=20
> +	case PPC_PTRACE_GETHWDBGINFO: {
> +		struct ppc_debug_info dbginfo;
> +
> +		dbginfo.version =3D 1;
> +		dbginfo.num_instruction_bps =3D 0;
> +		dbginfo.num_data_bps =3D 1;
> +		dbginfo.num_condition_regs =3D 0;
> +#ifdef CONFIG_PPC64
> +		dbginfo.data_bp_alignment =3D 8;
> +#else
> +		dbginfo.data_bp_alignment =3D 0;
> +#endif
> +		dbginfo.sizeof_condition =3D 0;
> +		dbginfo.features =3D 0;
> +
> +		if (!access_ok(VERIFY_WRITE, data,
> +			       sizeof(struct ppc_debug_info)))
> +			return -EFAULT;
> +		ret =3D __copy_to_user((struct ppc_debug_info __user =
*)data,
> +				     &dbginfo, sizeof(struct =
ppc_debug_info)) ?
> +		      -EFAULT : 0;
> +		break;
> +	}
> +
> +	case PPC_PTRACE_SETHWDEBUG: {
> +		struct ppc_hw_breakpoint bp_info;
> +
> +		if (!access_ok(VERIFY_READ, data,
> +			       sizeof(struct ppc_hw_breakpoint)))
> +			return -EFAULT;
> +		ret =3D __copy_from_user(&bp_info,
> +				       (struct ppc_hw_breakpoint __user =
*)data,
> +				       sizeof(struct ppc_hw_breakpoint)) =
?
> +		      -EFAULT : 0;
> +		if (!ret)
> +			ret =3D ppc_set_hwdebug(child, &bp_info);
> +		break;
> +	}
> +
> +	case PPC_PTRACE_DELHWDEBUG: {
> +		ret =3D ppc_del_hwdebug(child, addr, data);
> +		break;
> +	}
> +
> 	case PTRACE_GET_DEBUGREG: {
> 		ret =3D -EINVAL;
> 		/* We only support one DABR and no IABRS at the moment =
*/
>=20
> --=20
> Dave Kleikamp
> IBM Linux Technology Center
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH] powerpc/85xx: Workaround MPC8572/MPC8536 GPIO 1 errata.
From: Kumar Gala @ 2009-12-11  3:11 UTC (permalink / raw)
  To: Felix Radensky; +Cc: linuxppc-dev
In-Reply-To: <1250056659-420-1-git-send-email-felix@embedded-sol.com>


On Aug 12, 2009, at 12:57 AM, Felix Radensky wrote:

> On MPC8572 and MPC8536 the status of GPIO pins configured
> as output cannot be determined by reading GPDAT register.
> Workaround by reading the status of input pins from GPDAT
> and the status of output pins from a shadow register.
> 
> Signed-off-by: Felix Radensky <felix@embedded-sol.com>
> ---
> arch/powerpc/sysdev/mpc8xxx_gpio.c |   21 ++++++++++++++++++++-
> 1 files changed, 20 insertions(+), 1 deletions(-)

applied to next

- k

^ permalink raw reply

* Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
From: David Gibson @ 2009-12-11  3:26 UTC (permalink / raw)
  To: Dave Kleikamp
  Cc: linuxppc-dev list, Sergio Durigan Junior, Torez Smith,
	Thiago Jung Bauermann
In-Reply-To: <20091210155727.6697.74672.sendpatchset@norville.austin.ibm.com>

On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
> powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
>=20
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>=20
> This patch defines context switch and trap related functionality
> for BookE specific Debug Registers. It adds support to ptrace()
> for setting and getting BookE related Debug Registers
>=20
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
>=20
>  arch/powerpc/include/asm/system.h |    2=20
>  arch/powerpc/kernel/process.c     |  109 ++++++++-
>  arch/powerpc/kernel/ptrace.c      |  435 +++++++++++++++++++++++++++++++=
+++---
>  arch/powerpc/kernel/signal.c      |    6 -
>  arch/powerpc/kernel/signal_32.c   |    8 +
>  arch/powerpc/kernel/traps.c       |   86 ++++++-
>  6 files changed, 564 insertions(+), 82 deletions(-)

[snip]
> +void do_send_trap(struct pt_regs *regs, unsigned long address,
> +		  unsigned long error_code, int signal_code, int errno)
> +{
> +	siginfo_t info;
> +
> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> +		return;
> +
> +	/* Deliver the signal to userspace */
> +	info.si_signo =3D SIGTRAP;
> +	info.si_errno =3D errno;

We're using the errno siginfo field, but not for an errno, so possibly
the parameter should be called something else.

[snip]
> +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
>  void do_dabr(struct pt_regs *regs, unsigned long address,
>  		    unsigned long error_code)
>  {
> @@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long add=
ress,
>  	if (debugger_dabr_match(regs))
>  		return;
> =20
> -	/* Clear the DAC and struct entries.  One shot trigger */
> -#if defined(CONFIG_BOOKE)
> -	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> -							| DBCR0_IDM));
> -#endif
> -
>  	/* Clear the DABR */
>  	set_dabr(0);

Uh.. does this imply we're keeping the one-shot behaviour for
new-style breakpoints?  To me the interface really suggests they're
persistent, although dealing with the semantics of that at signal time
can get curly.

> =20
> @@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long add=
ress,
>  	info.si_addr =3D (void __user *)address;
>  	force_sig_info(SIGTRAP, &info, current);
>  }
> +#endif
> =20
>  static DEFINE_PER_CPU(unsigned long, current_dabr);
> =20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +/*
> + * Set the debug registers back to their default "safe" values.
> + */
> +static void set_debug_reg_defaults(struct thread_struct *thread)
> +{
> +	thread->iac1 =3D thread->iac2 =3D thread->iac3 =3D thread->iac4 =3D 0;
> +	thread->dac1 =3D thread->dac2 =3D 0;
> +	thread->dvc1 =3D thread->dvc2 =3D 0;
> +	/*
> +	 * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
> +	 * the exception of the reserved bits should be cleared out
> +	 * and set to 0.
> +	 *
> +	 * For the DBCR0 register, the reserved bits are bits 17:30.
> +	 * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
> +	 * And, bits 10:11 for DBCR2.
> +	 */
> +	thread->dbcr0 =3D DBCR0_BASE_REG_VALUE;

Since this is now the only place it's used, I'd pull the
BASE_REG_VALUE constant inline here.  Makes the actual definition sit
next to the describing comment which is a bonus.

> +	/*
> +	 * First clear all "non reserved" bits from DBCR1 then initialize reg
> +	 * to force User/Supervisor bits to b11 (user-only MSR[PR]=3D1) and
> +	 * Effective/Real * bits to b10 (trap only if IS=3D=3D0)
> +	 */
> +	thread->dbcr1 =3D DBCR1_BASE_REG_VALUE;
> +	/*
> +	 * Force Data Address Compare User/Supervisor bits to be User-only
> +	 * (0b11 MSR[PR]=3D1) and set all other bits in DBCR2 register to be 0.
> +	 * This sets the Data Address Compare Effective/Real bits to be 0b00
> +	 * (Effective, MSR[DS]=3Ddon't care).
> +	 */
> +	thread->dbcr2 =3D DBCR2_BASE_REG_VALUE;
> +}
> +
> +static void prime_debug_regs(struct thread_struct *thread)
> +{
> +	mtspr(SPRN_IAC1, thread->iac1);
> +	mtspr(SPRN_IAC2, thread->iac2);
> +	mtspr(SPRN_IAC3, thread->iac3);
> +	mtspr(SPRN_IAC4, thread->iac4);
> +	mtspr(SPRN_DAC1, thread->dac1);
> +	mtspr(SPRN_DAC2, thread->dac2);
> +	mtspr(SPRN_DVC1, thread->dvc1);
> +	mtspr(SPRN_DVC2, thread->dvc2);
> +	mtspr(SPRN_DBCR0, thread->dbcr0);
> +	mtspr(SPRN_DBCR1, thread->dbcr1);
> +	mtspr(SPRN_DBCR2, thread->dbcr2);

As Josh pointed out, you'll need to be a little more careful here to
only mtspr() registers which actually exist on the current platform.
We may be better off only implementing the new interface for BookE in
the first cut.

> +}
> +/*
> + * Unless neither the old or new thread are making use of the
> + * debug registers, set the debug registers from the values
> + * stored in the new thread.
> + */
> +static void switch_booke_debug_regs(struct thread_struct *new_thread)
> +{
> +	if ((current->thread.dbcr0 & DBCR0_IDM)
> +		|| (new_thread->dbcr0 & DBCR0_IDM))
> +			prime_debug_regs(new_thread);
> +}
> +#endif
> +
>  int set_dabr(unsigned long dabr)
>  {
>  	__get_cpu_var(current_dabr) =3D dabr;
> @@ -284,7 +358,7 @@ int set_dabr(unsigned long dabr)
>  		return ppc_md.set_dabr(dabr);
> =20
>  	/* XXX should we have a CPU_FTR_HAS_DABR ? */
> -#if defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
>  	mtspr(SPRN_DAC1, dabr);

Uh.. this would seem to be wrong.  set_dabr(0) is called from the
debug exception - but nowadays that could be tiggered by a DAC other
than DAC1.

>  #elif defined(CONFIG_PPC_BOOK3S)
>  	mtspr(SPRN_DABR, dabr);
> @@ -371,10 +445,8 @@ struct task_struct *__switch_to(struct task_struct *=
prev,
> =20
>  #endif /* CONFIG_SMP */
> =20
> -#if defined(CONFIG_BOOKE)
> -	/* If new thread DAC (HW breakpoint) is the same then leave it */
> -	if (new->thread.dabr)
> -		set_dabr(new->thread.dabr);
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	switch_booke_debug_regs(&new->thread);
>  #else
>  	if (unlikely(__get_cpu_var(current_dabr) !=3D new->thread.dabr))
>  		set_dabr(new->thread.dabr);
> @@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
>  	printk("  CR: %08lx  XER: %08lx\n", regs->ccr, regs->xer);
>  	trap =3D TRAP(regs);
>  	if (trap =3D=3D 0x300 || trap =3D=3D 0x600)
> -#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
>  		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
>  #else
>  		printk("DAR: "REG", DSISR: "REG"\n", regs->dar, regs->dsisr);
> @@ -568,14 +640,19 @@ void flush_thread(void)
> =20
>  	discard_lazy_cpu_state();
> =20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	/*
> +	 * flush_thread() is called on exec() to reset the
> +	 * thread's status. Set all debug regs back to their
> +	 * default values....Torez
> +	 */
> +	set_debug_reg_defaults(&current->thread);

Better to define this function as a nop on non-BookE.

[snip]
> +#else
>  	/* As described above, it was assumed 3 bits were passed with the data
>  	 *  address, but we will assume only the mode bits will be passed
>  	 *  as to not cause alignment restrictions for DAC-based processors.
>  	 */
> =20
>  	/* DAC's hold the whole address without any mode flags */
> -	task->thread.dabr =3D data & ~0x3UL;
> -
> -	if (task->thread.dabr =3D=3D 0) {
> -		task->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> -		task->thread.regs->msr &=3D ~MSR_DE;
> +	task->thread.dac1 =3D data & ~0x3UL;
> +
> +	if (task->thread.dac1 =3D=3D 0) {
> +		dbcr_dac(task) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> +					task->thread.dbcr1)) {
> +			task->thread.regs->msr &=3D ~MSR_DE;
> +			task->thread.dbcr0 &=3D ~DBCR0_IDM;
> +		}
>  		return 0;
>  	}

Ok, so effectively the old ptrace method of setting the DABR acts as a
bypass to set DAC1, rather than having the old interface being
implemented via the new interface.  This has some weirdness - you can
clobber a new-style breakpoint in DAC1 using the old interface, for
example.  Still, it might be the simplest approach for a first cut.

What *is* a problem though, is that this means that the SIGTRAP will
always give a slot number, even for a breakpoint established using the
old interface.  Part of the idea of encoding the registered breakpoint
number in the siginfo was to be able to distinguish between old-style
and new-style breakpoints at trap time.

[snip]
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +static long set_intruction_bp(struct task_struct *child,
> +			      struct ppc_hw_breakpoint *bp_info)
> +{
> +	int slots_needed;
> +	int slot;
> +	int free_slot =3D 0;
> +
> +	/*
> +	 * Find an avalailable slot for the breakpoint.
> +	 * If possible, reserve consecutive slots, 1 & 2, for a range
> +	 * breakpoint.  (Can this be done simpler?)
> +	 */
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		slots_needed =3D 1;
> +	else
> +		slots_needed =3D 2;

Ugh.  This logic is pretty hard to follow.  I'm having trouble
convincing myself it gets all the cases right, although I haven't
found something definitely wrong.

> +	if ((child->thread.dbcr0 & DBCR0_IAC1) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			if (child->thread.dbcr0 & DBCR0_IAC2) {
> +				slot =3D 1;
> +				goto found;
> +			}
> +			/* Try to save slots 1 & 2 for range */
> +			free_slot =3D 1;
> +		} else
> +			if ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0) {
> +				slot =3D 1;
> +				goto found;
> +			}
> +	} else if ((slots_needed =3D=3D 1) &&
> +		   ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0)) {
> +		slot =3D 2;
> +		goto found;
> +	}
> +	if ((child->thread.dbcr0 & DBCR0_IAC3) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		return -ENOSPC;
> +	} else if (slots_needed =3D=3D 2)
> +		return -ENOSPC;
> +	if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +		slot =3D 4;
> +	} else if (free_slot)
> +		slot =3D free_slot;
> +	else
> +		return -ENOSPC;
> +found:
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT) {
> +		switch (slot) {
> +			case 1:
> +				child->thread.iac1 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC1;
> +				break;
> +			case 2:
> +				child->thread.iac2 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC2;
> +				break;
> +			case 3:
> +				child->thread.iac3 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC3;
> +				break;
> +			case 4:
> +				child->thread.iac4 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC4;
> +				break;

By using an array instead of individual iac1..4 in the thread struct,
plus a suitable macro to generate the right bit, you could get rid of
this nasty switch.

> +		}
> +	} else if (slot =3D=3D 1) {
> +		child->thread.iac1 =3D bp_info->addr;
> +		child->thread.iac2 =3D bp_info->addr2;

You test that addr is a user address in the caller, but I don't think
you ever check addr2.

> +		child->thread.dbcr0 |=3D (DBCR0_IAC1 | DBCR0_IAC2);

Uh.. I thought for range breakpoints you only needed to enable the bit
for the first of the two IACs (plus the range enable bit, of course).

> +		if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_I;
> +	} else { /* slot =3D=3D 3 */
> +		child->thread.iac3 =3D bp_info->addr;
> +		child->thread.iac4 =3D bp_info->addr2;
> +		child->thread.dbcr0 |=3D (DBCR0_IAC3 | DBCR0_IAC4);
> +		if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_I;
> +	}
> +	child->thread.dbcr0 |=3D DBCR0_IDM;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return slot;

Yeah, ok.  I think the idea of using the register number as the
breakpoint identifier is limiting and we'll need to do something else
eventually.  Nonetheless, since userspace should be treating it as
opaque, it will do for a first cut and we can rework this later.

> +}
> +
> +static int del_instruction_bp(struct task_struct *child, int slot)
> +{
> +	switch (slot) {
> +	case 1:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M) {
> +			/* address range - clear slots 1 & 2 */
> +			child->thread.iac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC12M;
> +		}
> +		child->thread.iac1 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC1;
> +		break;

Uh.. as far as I can tell, you won't return an error if you try to
clear a breakpoint from a slot that isn't used.  That seems like
incorrect behaviour.

> +	case 2:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M)
> +			/* used in a range */
> +			return -EINVAL;
> +		child->thread.iac2 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +		break;
> +	case 3:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M) {
> +			/* address range - clear slots 3 & 4 */
> +			child->thread.iac4 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC34M;
> +		}
> +		child->thread.iac3 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC3;
> +		break;
> +	case 4:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M)
> +			/* Used in a range */
> +			return -EINVAL;
> +		child->thread.iac4 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int set_dac(struct task_struct *child, struct ppc_hw_breakpoint *=
bp_info)
> +{
> +	int byte_enable =3D
> +		(bp_info->condition_mode >> PPC_BREAKPOINT_CONDITION_BE_SHIFT)
> +		& 0xf;
> +	int condition_mode =3D
> +		bp_info->condition_mode & PPC_BREAKPOINT_CONDITION_AND_OR;

Using the fact that AND_OR is also equal to a suitable mask is a bit
overly subtle.  Better to use a separately defined mask constant.

> +	int slot;
> +
> +	if (byte_enable && (condition_mode =3D=3D 0))
> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) =3D=3D 0) {
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +			dbcr_dac(child) |=3D DBCR_DAC1R;
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +			dbcr_dac(child) |=3D DBCR_DAC1W;
> +		child->thread.dac1 =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE

Better to have a runtime feature bit test here, than use an #ifdef to
distinguish the 40x and BookE cases.  Plus, you should return an error
if the user attempts to use a feature not supported on this hardware,
which doesn't seem to happen here.

=18[snip]
> +static int del_dac(struct task_struct *child, int slot)
> +{

Again, you never seem to generate an error if you attempt to delete a
watchpoint that was never set.

> +	if (slot =3D=3D 1) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
> +			child->thread.dac1 =3D 0;
> +			child->thread.dac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~(DBCR0_DAC1R | DBCR0_DAC1W |
> +						 DBCR0_DAC2R | DBCR0_DAC2W);
> +			child->thread.dbcr2 &=3D ~DBCR2_DAC12MODE;
> +			return 0;
> +		}
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC1M | DBCR2_DVC1BE);
> +		child->thread.dvc1 =3D 0;

Since this will just clear fields which are never used on 40x, you
shouldn't need the ifdef.

> +#endif
> +		child->thread.dac1 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +	} else if (slot =3D=3D 2) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE)
> +			/* Part of a range */
> +			return -EINVAL;
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC2M | DBCR2_DVC2BE);
> +		child->thread.dvc2 =3D 0;
> +#endif
> +		child->thread.dac2 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC2R | DBCR_DAC2W);
> +	} else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +#endif /* CONFIG_40x || CONFIG_BOOKE */
> +
> +#ifdef CONFIG_BOOKE
> +static int set_dac_range(struct task_struct *child,
> +			 struct ppc_hw_breakpoint *bp_info)
> +{
> +	int mode =3D bp_info->addr_mode & PPC_BREAKPOINT_MODE_MASK;
> +
> +	/* We don't allow range watchpoints to be used with DVC */
> +	if (bp_info->condition_mode && PPC_BREAKPOINT_CONDITION_BE_ALL)

Uh.. that condition really doesn't look right.  First, surely it
should be a bitwise, not a logical and, second the comment is talking
about range watchpoints, but the condition is about the byte enable
bits.

> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if (child->thread.dbcr0 &
> +	    (DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W))
> +		return -ENOSPC;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1R | DBCR0_DAC2R | DBCR0_IDM);
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1W | DBCR0_DAC2W | DBCR0_IDM);

Again, I thought only the DAC1 R/W bits were used for range watchpoints.

> +	child->thread.dac1 =3D bp_info->addr;
> +	child->thread.dac2 =3D bp_info->addr2;
> +	if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12R;
> +	else if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12RX;
> +	else	/* PPC_BREAKPOINT_MODE_MASK */
> +		child->thread.dbcr2  |=3D DBCR2_DAC12MASK;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return 5;
> +}
> +#endif /* CONFIG_BOOKE */
> +
>  static long ppc_set_hwdebug(struct task_struct *child,
>  		     struct ppc_hw_breakpoint *bp_info)
>  {
> +	if (bp_info->version !=3D 1)
> +		return -ENOTSUPP;
> +
> +#ifdef CONFIG_BOOKE
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode &
> +	     ~(PPC_BREAKPOINT_CONDITION_AND_OR |
> +	       PPC_BREAKPOINT_CONDITION_BE_ALL)))
> +		return -EINVAL;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return set_dac(child, bp_info);
> +
> +	return set_dac_range(child, bp_info);
> +#elif defined(CONFIG_40x)
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode !=3D PPC_BREAKPOINT_CONDITION_NONE))
> +		return -EINVAL;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +	if (bp_info->addr_mode !=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return -EINVAL;
> +
> +	return set_dac(child, bp_info);
> +#else
>  	/*
> -	 * We currently support one data breakpoint
> +	 * We only support one data breakpoint

Uh.. you've updated this comment to something which is still wrong
with the new code...

[snip]
> @@ -980,16 +1308,36 @@ long arch_ptrace(struct task_struct *child, long r=
equest, long addr, long data)
>  		struct ppc_debug_info dbginfo;
> =20
>  		dbginfo.version =3D 1;
> +#ifdef CONFIG_BOOKE
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 2;
> +		dbginfo.data_bp_alignment =3D 0;

Surely that can't be right, I thought there was always a data bp
alignment constraint.

> +		dbginfo.sizeof_condition =3D 4;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK |
> +				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
> +#elif defined(CONFIG_40x)
> +		/*
> +		 * I don't know how the DVCs work on 40x, I'm not going
> +		 * to support it now. -- Shaggy
> +		 */
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 0;
> +		dbginfo.data_bp_alignment =3D 0;
> +		dbginfo.sizeof_condition =3D 0;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK;
> +#else
>  		dbginfo.num_instruction_bps =3D 0;
>  		dbginfo.num_data_bps =3D 1;
>  		dbginfo.num_condition_regs =3D 0;
> -#ifdef CONFIG_PPC64
>  		dbginfo.data_bp_alignment =3D 8;

Uh, this isn't quite right.  32-bit classic PPC can have DABRs, so
!CONFIG_PPC64 does not imply 40x|BOOKE, which means we still need the
32-bit case here.

--=20
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms
From: David Gibson @ 2009-12-11  3:28 UTC (permalink / raw)
  To: Kumar Gala
  Cc: linuxppc-dev list, Sergio Durigan Junior, Dave Kleikamp,
	Thiago Jung Bauermann, Torez Smith
In-Reply-To: <1134F495-6623-437F-AF1F-64D0A87BA112@kernel.crashing.org>

On Thu, Dec 10, 2009 at 08:41:53PM -0600, Kumar Gala wrote:
[snip]
> > +#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
> > +#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 | \
> > +				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
> > +				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
> > +				 DBCR1_IAC4US | DBCR1_IAC4ER_10)
> 
> We are we using MSR[IS] IS=0, why not just any Eff address?  In the
> future we might have user as IS = 1, and kernel as IS = 0.

Since the user can't control that directly, we can update this when
and if we change our use of address spaces.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

^ permalink raw reply

* Re: Q: how to access the MPC5200B's timer gipo's?
From: Grant Likely @ 2009-12-11  6:02 UTC (permalink / raw)
  To: Albrecht Dreß; +Cc: linuxppc-dev
In-Reply-To: <10228901.1260376093135.JavaMail.ngmail@webmail18.arcor-online.net>

On Wed, Dec 9, 2009 at 9:28 AM, Albrecht Dre=DF <albrecht.dress@arcor.de> w=
rote:
> Hi all,
>
> I have a (probably dumb) question regarding the access to the MPC5200B's =
timer gpio's. =A0I added e.g.
>
> timer@640 { =A0 =A0 // General Purpose Timer 4
> =A0 =A0 =A0 =A0compatible =3D "fsl,mpc5200b-gpt","fsl,mpc5200-gpt";
> =A0 =A0 =A0 =A0reg =3D <0x640 0x10>;
> =A0 =A0 =A0 =A0interrupts =3D <1 13 0>;
> =A0 =A0 =A0 =A0interrupt-parent =3D <&mpc5200_pic>;
> =A0 =A0 =A0 =A0gpio-controller;
> =A0 =A0 =A0 =A0#gpio-cells =3D <2>;
> };
>
> in the DTS file to several timers. =A0Several gpiochipxxx folders are now=
 present in /sys/class/gpio/.
>
> In my code, I grab the node by calling of_find_node_by_path("/soc5200@f00=
00000/timer@640"), which returns the proper node. =A0Then I try to get the =
gpio number (for a call to gpio_set_value()) by calling of_get_gpio(node, 0=
) with the node found above. =A0However, this call always dumps
>
> of_get_gpio_flags: can't parse gpios property
> of_get_gpio_flags exited with status -2
>
> What would be the proper way to determine the gpio number for a timer's g=
pio pin from the node name or path?

The existing API (of_get_gpio()) doesn't operate on gpio-controller
nodes.  It operates on a node that uses the gpio (has a 'gpios'
property as documented in
Documentation/powerpc/dts-bindings/gpio/gpio.txt).

Therefore, you need a node to describe the *user* of the GPIO pin, and
call of_get_gpio() on that.  If you want to resolve the GPIO number
from the timer node itself, then you'll need to refactor the API a bit
to expose what you want.  Nobody has asked to do it that way until
now.

g.

--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox