* Re: Next May 11 : BUG during scsi initialization
From: Stephen Rothwell @ 2009-05-12 5:56 UTC (permalink / raw)
To: Nick Piggin
Cc: linux-scsi, Matthew Wilcox, linuxppc-dev, Pekka Enberg,
linux-next
In-Reply-To: <20090512045716.GC32535@wotan.suse.de>
[-- Attachment #1: Type: text/plain, Size: 720 bytes --]
Hi Nick,
On Tue, 12 May 2009 06:57:16 +0200 Nick Piggin <npiggin@suse.de> wrote:
>
> Hmm, I think (hope) your problems were fixed with the recent memory
> coruption bug fix for SLQB. (if not, let me know)
>
> This one possibly looks like a problem with remote memory allocation
> or memory hotplug or something like that. I'll do a bit of code
> review....
These are -next kernels which include the two fixes you posted recently
(I am pretty sure). I am also getting the network failures that Sachin
is seeing on several of my machines here. The previously reported
problems have gone away.
--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/
[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* Re: device trees.
From: Grant Likely @ 2009-05-12 5:30 UTC (permalink / raw)
To: Stephen Neuendorffer; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <20090512042733.8D4A21400054@mail184-dub.bigfish.com>
On Mon, May 11, 2009 at 10:27 PM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
>> > OK, so the key question seems to be *when* the bitstream is
> associated
>> > with the
>> > device tree. =A0If at bitstream generation time, you can prepend the
> .dtb
>> > to the bitstream. =A0As long as the dtb doesn't contain the magic
>> > bitstream start code, you can go back and access it later.
>> >
>> You really mean prepend ? I was presuming that things would work
> better
>> if it was appended ?
>
> Yes, actually prepend is simpler because you don't have to know the size
> of the bitstream.
> Everything before the SYNC code in the bitstream is ignored.
...In this case, might need to preprocess the .dtb the escape out the
possibility of a sync code appearing.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: device trees.
From: Grant Likely @ 2009-05-12 5:22 UTC (permalink / raw)
To: Grant Likely, David H. Lynch Jr., linuxppc-dev
In-Reply-To: <20090512011219.GB18223@yookeroo.seuss>
On Mon, May 11, 2009 at 7:12 PM, David Gibson
<david@gibson.dropbear.id.au> wrote:
> On Mon, May 11, 2009 at 05:09:27PM -0600, Grant Likely wrote:
>> In other words; having your bootloader support FDT is preferred, but
>> not required.
>
> I wouldn't even go so far as to say it's preferred. =A0IMO, people have
> gone a bit prematurely keen on moving devtree handling into the
> firmware. =A0Putting it in the firmware has a number of advantages, but
> it also has a number of non-trivial disadvantages.
I disagree. The more I work with it, the more I appreciate the
advantage of decoupling the kernel image file from the hardware
description. It is valuable being able to build a single image file
that boots on a wide range of boards because the device tree passed in
by firmware.
I'm not downplaying the disadvantages and problems, but I still hold
the view that the striving for generic multiplatform kernel images is
worth the effort.
... but I do agree that hard linking the .dtb into firmware, or making
the .dtb hard to upgrade is the way of madness.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: Next May 11 : BUG during scsi initialization
From: Nick Piggin @ 2009-05-12 4:57 UTC (permalink / raw)
To: Matthew Wilcox
Cc: Stephen Rothwell, linux-scsi, linuxppc-dev, Pekka Enberg,
linux-next
In-Reply-To: <20090511122135.GC8112@parisc-linux.org>
On Mon, May 11, 2009 at 06:21:35AM -0600, Matthew Wilcox wrote:
> On Mon, May 11, 2009 at 05:34:07PM +0530, Sachin Sant wrote:
> > Matthew Wilcox wrote:
> >> On Mon, May 11, 2009 at 05:16:10PM +0530, Sachin Sant wrote:
> >>
> >>> Today's Next tree failed to boot on a Power6 box with following BUG :
> >>
> >> This doesn't actually appear to be a SCSI bug ... it looks like SCSI tried
> >> to allocate memory and things went wrong in the memory allocator:
> >>
> >> [c0000000c7d038b0] [c0000000005d67d8] ._spin_lock+0x10/0x24
> >> [c0000000c7d03920] [c00000000013fbdc] .__slab_alloc_page+0x344/0x3cc
> >> [c0000000c7d039e0] [c000000000141168] .kmem_cache_alloc+0x13c/0x21c
> >> [c0000000c7d03aa0] [c000000000141b04] .kmem_cache_create+0x294/0x2a8
> >> [c0000000c7d03b90] [d000000000ea14cc] .scsi_init_queue+0x38/0x170 [scsi_mod]
> >>
> >> Which memory allocator did you have selected (SLAB, SLUB, SLOB, SLQB)?
> >>
> > Default one. SLQB
> >
> > CONFIG_SLQB_ALLOCATOR=y
> > CONFIG_SLQB=y
> >
> > Page size is 64K with Config DEBUG_PAGEALLOC set.
> >
> > CONFIG_PPC_HAS_HASH_64K=y
> > CONFIG_PPC_64K_PAGES=y
> > CONFIG_DEBUG_PAGEALLOC=y
>
> Hm. We've seen some similar problems at Intel while doing database
> performance tests with SLQB. Any ideas, Nick?
Hmm, I think (hope) your problems were fixed with the recent memory
coruption bug fix for SLQB. (if not, let me know)
This one possibly looks like a problem with remote memory allocation
or memory hotplug or something like that. I'll do a bit of code
review....
^ permalink raw reply
* RE: device trees.
From: Stephen Neuendorffer @ 2009-05-12 4:27 UTC (permalink / raw)
To: David H. Lynch Jr., linuxppc-dev
In-Reply-To: <4A08E050.9000302@dlasys.net>
> -----Original Message-----
> From: David H. Lynch Jr. [mailto:dhlii@dlasys.net]
> Sent: Monday, May 11, 2009 7:35 PM
> To: Stephen Neuendorffer; linuxppc-dev@ozlabs.org
> Subject: Re: device trees.
> =
> Stephen Neuendorffer wrote:
> >
> >> Many of our systems are LX systems but currently we are not running
> >> Linux on them.
> >>
> >
> > Master SelectMap, I presume? What FPGA family?
> > Does the FPGA have access to the CPLD after boot, other than through
the
> > configuration pins?
> >
> >
> One of the skills I have not had time to develop - because Pico has
> plenty of qualified firmware/hardware people and not enough OS people,
> is fluency with Xilinx tools.
> =
> FPGA Families - currently Spartan, virtex 4 and virtex 5.
> LX & FX, in all kinds of sizes. So that as an example our E14 (1st
> generation cardbus) comes in FX40's and FX60's and LX## .......
> =
> I am presuming that is what you mean.
> =
> Access to the hardware on the cards can be weird. As our cards are
often
> hosted, that means the firmware is often setup to allow host and
target
> access to hardware. In others only the host or only the target does.
> =
> What I know is that if I send a few magic values to the CPLD and then
> start reading the bit file out of flash, I will trigger the CPLD to
> reload the FPGA from the bitfile I selected.
> =
> =
> =
> =
> =
> > OK, so that means the boot monitor can open sector 2 of the flash
and
> > read info, right? (Or wherever else the bitstream is coming from.
> On powerup the CPLD boots from sector 2. IF it is rebooted by the host
> or target it can reboot from any flash sector.
> In the "normal" setup the target has indirect access to the entire NOR
> flash.
> =
> > Can
> > the CPLD store one 32 bit int that the new bitstream can come back
and
> > read later?
> >
> Off the top of my head I do nto know, but if you are saying I need to
> find someway of preserving a 32bit value through rebooting the FPGA I
> can find a way to do it.
> =
> >
> >
> > OK, so the key question seems to be *when* the bitstream is
associated
> > with the
> > device tree. If at bitstream generation time, you can prepend the
.dtb
> > to the bitstream. As long as the dtb doesn't contain the magic
> > bitstream start code, you can go back and access it later.
> >
> You really mean prepend ? I was presuming that things would work
better
> if it was appended ?
Yes, actually prepend is simpler because you don't have to know the size
of the bitstream.
Everything before the SYNC code in the bitstream is ignored.
For instance, this is the start of a spartan 3ADSP bitstream:
00000000 00 09 0f f0 0f f0 0f f0 0f f0 00 00 01 61 00 0b
|.............a..|
00000010 73 79 73 74 65 6d 2e 6e 63 64 00 62 00 0e 33 73
|system.ncd.b..3s|
00000020 64 33 34 30 30 61 66 67 36 37 36 00 63 00 0b 32
|d3400afg676.c..2|
00000030 30 30 39 2f 20 35 2f 20 31 00 64 00 09 31 34 3a |009/ 5/
1.d..14:|
00000040 34 38 3a 32 31 00 65 00 16 59 d4 ff ff ff ff ff
|48:21.e..Y......|
00000050 ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
|................|
00000060 ff ff ff ff ff ff ff ff ff ff ff aa 99 30 a1 00
|.............0..|
'FF FF AA 99' is the Spartan 3 DUMMY code/SYNC code combination,
everything before that
is discarded by the FPGA.
> Regardless, I have the means to know exactly what bit file is
currently
> loaded, and I can then look it up in the NOR Flash.
Yes, so as long as when the bitstream is loaded, it can figure out where
in the flash it was loaded *from* and then go back there and get the
dtb, then that should work. This would make it difficult to do certain
things (like loading a useful bitstream from anywhere OTHER than flash),
but if you can make that restriction, then it should work. I can think
of several ways of doing this, the simple of which is to have your
bootloader go to the CPLD after it has loaded to figure out where the
bitstream came from.
> I can glue the dtb and the bit file together in anyway that will make
> xilinx happy.
Fortunately, you don't have to keep Xilinx happy, only the FPGA.. :)
> If it is prepended the only case I care about is the power up
sequence,
> because that must start loading the bit file at sector 2.
> We do cope with the scenario where the sector 2 bit file is completely
> screwed up. The CPLD STARTS trying to load at sector 2, but it will
> continue to the end of flash until something actually loads. The dead
> card scenario is sector 2 is the start of a valid but non-functional
bit
> file.
> At that point you must load a bit image using JTAG and then write a
good
> bit file to flash.
> =
> Alright lets say I prepend. I am loosely familar with the magic start
> code. Does that need to be aligned in anyway ?
My understanding is that alignment does matter somewhat if you are using
SelectMap32, so if this is what you are doing, it's probably best to
preserve the existing alignment, padding the dtb to a number of words
divisible by 4. If you're using serial configuration modes, or
selectmap8, then it doesn't matter.
> And just for the sake of argument lets say I append the dtb. Do I need
> some padding between the bitfile and the dtb to keep the
> FPGA from loading the dtb as firmware ? Is there a magic stop sequence
?
> Is the load terminated by the length of the bits.
The bitstream ends with a DESYNC code, followed by a pad of frame data.
After that, everything is ignored until the next SYNC code. BTW, all of
this is documented in the various configuration user guides.
I'll add that if you are doing V4 and V5 only and don't care about S3,
then there is another mechanism which avoids 'knowing where you loaded
the bitstream from'. This relies on the USR_ACCESS_VIRTEX4 and
USER_ACCESS_VIRTEX5 primitives to pass further configuration information
to the FPGA design from the configuration source. You could use this to
append the dtb to the bitstream and to copy it into external memory
using a little bit of FPGA logic, and presumably, the processor.
However, this mechanism is not available on S3.
Steve
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
From: Michael Neuling @ 2009-05-12 2:48 UTC (permalink / raw)
To: prasad, linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <4234.1242089764@neuling.org>
> > +struct arch_hw_breakpoint {
> > + char *name; /* Contains name of the symbol to set bkpt */
> > + unsigned long address;
> > + u8 type;
> > +};
>
> Can you reorder this to pack the struct better (ie. put the unsigned
> long first).
Oops, this is a char * not just a char. Order is fine.
Mikey
^ permalink raw reply
* Re: device trees.
From: David H. Lynch Jr. @ 2009-05-12 2:34 UTC (permalink / raw)
To: Stephen Neuendorffer, linuxppc-dev
In-Reply-To: <20090512005554.EEE1019D009B@mail129-dub.bigfish.com>
Stephen Neuendorffer wrote:
>
>> Many of our systems are LX systems but currently we are not running
>> Linux on them.
>>
>
> Master SelectMap, I presume? What FPGA family?
> Does the FPGA have access to the CPLD after boot, other than through the
> configuration pins?
>
>
One of the skills I have not had time to develop - because Pico has
plenty of qualified firmware/hardware people and not enough OS people,
is fluency with Xilinx tools.
FPGA Families - currently Spartan, virtex 4 and virtex 5.
LX & FX, in all kinds of sizes. So that as an example our E14 (1st
generation cardbus) comes in FX40's and FX60's and LX## .......
I am presuming that is what you mean.
Access to the hardware on the cards can be weird. As our cards are often
hosted, that means the firmware is often setup to allow host and target
access to hardware. In others only the host or only the target does.
What I know is that if I send a few magic values to the CPLD and then
start reading the bit file out of flash, I will trigger the CPLD to
reload the FPGA from the bitfile I selected.
> OK, so that means the boot monitor can open sector 2 of the flash and
> read info, right? (Or wherever else the bitstream is coming from.
On powerup the CPLD boots from sector 2. IF it is rebooted by the host
or target it can reboot from any flash sector.
In the "normal" setup the target has indirect access to the entire NOR
flash.
> Can
> the CPLD store one 32 bit int that the new bitstream can come back and
> read later?
>
Off the top of my head I do nto know, but if you are saying I need to
find someway of preserving a 32bit value through rebooting the FPGA I
can find a way to do it.
>
>
> OK, so the key question seems to be *when* the bitstream is associated
> with the
> device tree. If at bitstream generation time, you can prepend the .dtb
> to the bitstream. As long as the dtb doesn't contain the magic
> bitstream start code, you can go back and access it later.
>
You really mean prepend ? I was presuming that things would work better
if it was appended ?
Regardless, I have the means to know exactly what bit file is currently
loaded, and I can then look it up in the NOR Flash.
I can glue the dtb and the bit file together in anyway that will make
xilinx happy.
If it is prepended the only case I care about is the power up sequence,
because that must start loading the bit file at sector 2.
We do cope with the scenario where the sector 2 bit file is completely
screwed up. The CPLD STARTS trying to load at sector 2, but it will
continue to the end of flash until something actually loads. The dead
card scenario is sector 2 is the start of a valid but non-functional bit
file.
At that point you must load a bit image using JTAG and then write a good
bit file to flash.
Alright lets say I prepend. I am loosely familar with the magic start
code. Does that need to be aligned in anyway ?
And just for the sake of argument lets say I append the dtb. Do I need
some padding between the bitfile and the dtb to keep the
FPGA from loading the dtb as firmware ? Is there a magic stop sequence ?
Is the load terminated by the length of the bits.
> Is this sounding reasonable?
>
>
I am sure there is something I can work with here.
Thanks.
> Steve
>
> This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
>
>
>
--
Dave Lynch DLA Systems
Software Development: Embedded Linux
717.627.3770 dhlii@dlasys.net http://www.dlasys.net
fax: 1.253.369.9244 Cell: 1.717.587.7774
Over 25 years' experience in platforms, languages, and technologies too numerous to list.
"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein
^ permalink raw reply
* Re: device trees.
From: Michael Ellerman @ 2009-05-12 2:37 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <20090511225631.GA15371@yookeroo.seuss>
[-- Attachment #1: Type: text/plain, Size: 2365 bytes --]
On Tue, 2009-05-12 at 08:56 +1000, David Gibson wrote:
> On Mon, May 11, 2009 at 05:38:16PM -0400, David H. Lynch Jr. wrote:
> > Grant Likely wrote:
> > >> Anyway, all I was looking for was a leg up on figuring out how to do
> > >> what I want with them. Rather than starting from scratch.
> > >> I am not looking to be convinced that I am approaching this all wrong.
> > >> If you are happy with what you have - great. I am not.
> > >> While I was not looking to restart a great debate over device trees
> > >> - I do not actually think they are a bad idea.
> > >>
> > >
> > > I still don't understand what you're worried about starting an arguing
> > > about. Pretty much any of the PowerPC maintainers can point at warts
> > > and problems in the current handling of device trees. I'm not
> > > particularly happy with simpleImage (and I wrote it), but it takes
> > > time and effort to write something more capable.
> > >
> > I was not trying to start an argument, my initial question was
> >
> > "Is there an example somewhere that shows building a device tree on the fly ?"
> >
> > The responses have questioned why I want to do that rather than how can I do that.
> > I was not actually seeking a debate over the merit of device trees, or u-boot, or libdft, or ....
>
> Probably the closest things to suitable examples here are
> arch/powerpc/kernel/prom_init.c, which builds the dtb from the "live"
> OF device tree. At the moment that's a bit of a special case, but
> there's no inherent reason that logic couldn't be moved to the
> bootwrapper.
>
> The other, not in the main tree, is Paul's restodt, which builds a
> device tree from old-style PReP residual information. I recall it was
> sent to the list as a userspace program (we were gathering information
> on the sorts of PReP out there). I think Paul made at least a first
> cut at fusing it into the bootwrapper, but I don't know if that ever
> got sent out to the list.
There's also the iseries code, which is not pretty, but shows a very
minimal example of constructing a device tree on the fly, some of it
based on static config and some that's not.
The routines for constructing DT nodes and properties is about 110
lines, and most of that is brackets.
arch/powerpc/platforms/iseries/dt.c
cheers
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 197 bytes --]
^ permalink raw reply
* get data failed with DMA
From: Sauce.Cheng @ 2009-05-12 1:59 UTC (permalink / raw)
To: linuxppc-dev
i tried to fetch data from periphial memories with DMA
the flow as following
flags = claim_dma_lock();
disable_dma(DMA_CH);
clear_dma_ff();
set_dma_count(DMA_CH, 4);
set_dma_addr(DMA_CH, virt_to_bus(kv_buf));
release_dma_lock(flags);
enable_dma(DMA_CH);
the code hault at disable_dma(DMA_CH)
in addition, i defined DMA_CH as 0 for channel 0
'kv_buf' is an kernel space virtual address. i can red the correct value of
periphal from this address
i supposed the fault along with the configured of registers about DMA in
processer. but i am not sure.
give some suggestion about this, thanks!
--
View this message in context: http://www.nabble.com/get-data-failed-with-DMA-tp23494879p23494879.html
Sent from the linuxppc-dev mailing list archive at Nabble.com.
^ permalink raw reply
* Re: device trees.
From: David Gibson @ 2009-05-12 1:12 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <fa686aa40905111609l3feaf555taadf048714030a1@mail.gmail.com>
On Mon, May 11, 2009 at 05:09:27PM -0600, Grant Likely wrote:
> On Mon, May 11, 2009 at 3:38 PM, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
> > Grant Likely wrote:
> >>
> >> What do you mean by "one size fits all solution?"
> >>
> >> The kernel doesn't care where the device tree comes from. All it
> >> cares about is that by the time the kernel is started the device tree
> >> must be fully formed and populated. It can be completely pre-canned
> >> (like simpleImage), it can be modified by firmware (like u-boot), or
> >> it can be generated from scratch (like with real OpenFirmware). There
> >> is lots of flexibility on how to handle it.
> >>
> >>
> > First device trees are now the ONLY means of passing information to the
> > kernel.
> > By definition that means it is a one size fits all solution.
> > While there is nothing inherently wrong with that, solutions intended to
> > meet all circumstances need to be
> > simple, powerful, and flexible. They need to work well 100% of the time
> > not 98%.
> >
> > Not only is the device tree expected to pass static hardware
> > configuration information, but it is the sole means of passing anything.
> > As an example Command lines are to be in the device tree.
> > Everything is supposed to be in the device tree, whether that
> > information is static or dynamic, whether it is hardware information,
> > or user choices.
>
> It is the sole means of passing anything *to the kernel*. You can
> pass whatever you like to the bootwrapper. :-)
>
> > That means that whether you are in a Sun or Apple Desktop or a
> > system with the no flash and barely enough resources to run Linux,
> > you still may have to manipulate the device tree.
>
> ...or if you really are constrained, then define a format for the data
> you want to pass, pass it to the bootwrapper along with the device
> tree, and use the bootwrapper (which already has libfdt) to update the
> device tree. cuImage targets do this to support older u-boots which
> don't understand device trees. You are not forced to put device tree
> support in your firmware.
>
> In other words; having your bootloader support FDT is preferred, but
> not required.
I wouldn't even go so far as to say it's preferred. IMO, people have
gone a bit prematurely keen on moving devtree handling into the
firmware. Putting it in the firmware has a number of advantages, but
it also has a number of non-trivial disadvantages.
--
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] Hardware Breakpoint interfaces implementation for PPC64
From: Michael Neuling @ 2009-05-12 0:56 UTC (permalink / raw)
To: prasad; +Cc: linuxppc-dev, Benjamin Herrenschmidt, paulus
In-Reply-To: <20090511200355.GA17988@in.ibm.com>
> Hi PPC Dev folks,
> Please find a patch below that implements the proposed Hardware
> Breakpoint interfaces for PPC64 architecture.
>
> As a brief introduction, the proposed Hardware Breakpoint
> infrastructure provides generic in-kernel interfaces to which users
> from kernel- and user-space can request for breakpoint registers. An
> ftrace plugin that can trace accesses to data variables is also part
> of the generic HW Breakpoint interface patchset. The latest submission
> for this patchset along with an x86 implementation can be found here:
> http://lkml.org/lkml/2009/5/11/159.
>
> The following are the salient features of the PPC64 patch.
>
> - Arch-specific definitions for kernel and user-space requests are
> defined in arch/powerpc/kernel/hw_breakpoint.c
> - Ptrace is converted to use the HW Breakpoint interfaces
Will we fall back to use the old method if more than one address is
being watched?
> - The ftrace plugin called ksym_tracer is tested to work fine. For
> instance when tracing pid_max kernel variable, the following was
> obtained as output
Could you split the patch into a few pieces which implement these
different parts. The smaller logical chucks will make it easier to
review?
>
> # cat trace
> # tracer: ksym_tracer
> #
> # TASK-PID CPU# Symbol Type Function
> # | | | | |
> bash 4502 3 pid_max RW .do_proc_dointvec_minmax_
conv+0x78/0x10c
> bash 4502 3 pid_max RW .do_proc_dointvec_minmax_
conv+0xa0/0x10c
> bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
>
> There are however a few limitations/caveats of the patch as identified
> below:
>
> - The patch is currently implemented only for PPC64 architecture. Other
> architectures (especially Book-E implementations are expected to
> happen in due course).
>
> - HW Breakpoints over data addresses through Xmon (using "bd" command)
> and the proposed HW Breakpoint interfaces can now operate in a
> mutually exclusive manner. Xmon's integration is pending and is
> dependant on successful triggering of breakpoints through "bd<ops>".
> (Note: On a Power5 machine running 2.6.29, Xmon could not trigger HW
> Breakpoints when tested).
>
> Kindly let me know your comments.
>
> Thanks,
> K.Prasad
>
>
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
> arch/powerpc/Kconfig | 1
> arch/powerpc/include/asm/hw_breakpoint.h | 52 +++++
> arch/powerpc/include/asm/processor.h | 1
> arch/powerpc/include/asm/reg.h | 2
> arch/powerpc/include/asm/thread_info.h | 2
> arch/powerpc/kernel/Makefile | 2
> arch/powerpc/kernel/hw_breakpoint.c | 271 ++++++++++++++++++++++++++++
+++
> arch/powerpc/kernel/process.c | 18 ++
> arch/powerpc/kernel/ptrace.c | 48 +++++
> arch/powerpc/mm/fault.c | 14 -
> samples/hw_breakpoint/data_breakpoint.c | 4
> 12 files changed, 423 insertions(+), 9 deletions(-)
You've not touched prace32.c. Can we use this for 32 bit apps on a 64
bit kernel?
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/Kconfig
> +++ linux-2.6-tip.hbkpt/arch/powerpc/Kconfig
> @@ -125,6 +125,7 @@ config PPC
> select USE_GENERIC_SMP_HELPERS if SMP
> select HAVE_OPROFILE
> select HAVE_SYSCALL_WRAPPERS if PPC64
> + select HAVE_HW_BREAKPOINT if PPC64
>
> config EARLY_PRINTK
> bool
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/Makefile
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/Makefile
> @@ -33,7 +33,7 @@ obj-$(CONFIG_PPC64) += setup_64.o sys_p
> signal_64.o ptrace32.o \
> paca.o cpu_setup_ppc970.o \
> cpu_setup_pa6t.o \
> - firmware.o nvram_64.o
> + firmware.o nvram_64.o hw_breakpoint.o
> obj64-$(CONFIG_RELOCATABLE) += reloc_64.o
> obj-$(CONFIG_PPC64) += vdso64/
> obj-$(CONFIG_ALTIVEC) += vecemu.o vector.o
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> ===================================================================
> --- /dev/null
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/hw_breakpoint.c
> @@ -0,0 +1,271 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
.
> + *
> + * Copyright (C) 2009 IBM Corporation
> + */
> +
> +/*
> + * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
> + * using the CPU's debug registers.
> + */
> +
> +#include <linux/notifier.h>
> +#include <linux/kallsyms.h>
> +#include <linux/kprobes.h>
> +#include <linux/percpu.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/sched.h>
> +#include <linux/init.h>
> +#include <linux/smp.h>
> +
> +#include <asm/hw_breakpoint.h>
> +#include <asm/processor.h>
> +#include <asm/sstep.h>
> +
> +/* Store the kernel-space breakpoint address value */
> +static unsigned long kdabr;
> +
> +/*
> + * Temporarily stores address for DABR before it is written by the
> + * single-step handler routine
> + */
> +static DEFINE_PER_CPU(unsigned long, dabr_data);
> +
> +void arch_update_kernel_hw_breakpoint(void *unused)
> +{
> + struct hw_breakpoint *bp;
> +
> + /* Check if there is nothing to update */
> + if (hbp_kernel_pos == HB_NUM)
> + return;
> + bp = hbp_kernel[hbp_kernel_pos];
> + if (bp == NULL)
> + kdabr = 0;
> + else
> + kdabr = bp->info.address | bp->info.type | DABR_TRANSLATION;
> + set_dabr(kdabr);
> +}
> +
> +/*
> + * Install the thread breakpoints in their debug registers.
> + */
> +void arch_install_thread_hw_breakpoint(struct task_struct *tsk)
> +{
> + set_dabr(tsk->thread.dabr);
> +}
> +
> +/*
> + * Install the debug register values for just the kernel, no thread.
> + */
> +void arch_uninstall_thread_hw_breakpoint()
> +{
> + set_dabr(0);
> +}
> +
> +/*
> + * Check for virtual address in user space.
> + */
> +int arch_check_va_in_userspace(unsigned long va, u8 hbp_len)
> +{
> + return (va <= TASK_SIZE - HW_BREAKPOINT_LEN);
> +}
You pass ing hbp_len, but then use HW_BREAKPOINT_LEN? Is that right?
> +
> +/*
> + * Check for virtual address in kernel space.
> + */
> +int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len)
> +{
> + return (va >= TASK_SIZE) && ((va + HW_BREAKPOINT_LEN - 1) >= TASK_SIZE)
;
Can you use is_kernel_addr() here?
> +}
> +
> +/*
> + * Store a breakpoint's encoded address, length, and type.
> + */
> +int arch_store_info(struct hw_breakpoint *bp)
> +{
> + /*
> + * User-space requests will always have the address field populated
> + * For kernel-addresses, either the address or symbol name can be
> + * specified.
> + */
> + if (bp->info.name)
> + bp->info.address = (unsigned long)
> + kallsyms_lookup_name(bp->info.name);
> + if (bp->info.address)
> + return 0;
> + return -EINVAL;
> +}
> +
> +/*
> + * Validate the arch-specific HW Breakpoint register settings
> + */
> +int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
> + struct task_struct *tsk)
> +{
> + int ret = -EINVAL;
> +
> + if (!bp)
> + return ret;
> +
> + switch (bp->info.type) {
> + case DABR_DATA_READ:
> + break;
> + case DABR_DATA_WRITE:
> + break;
> + case DABR_DATA_RW:
> + break;
> + default:
> + return ret;
> + }
> +
> + if (bp->triggered)
> + ret = arch_store_info(bp);
> +
> + /* Check for double word alignment - 8 bytes */
> + if (bp->info.address & HW_BREAKPOINT_ALIGN)
> + return -EINVAL;
> + return ret;
> +}
> +
> +void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk)
> +{
> + struct thread_struct *thread = &(tsk->thread);
> + struct hw_breakpoint *bp = thread->hbp[0];
> +
> + if (bp)
> + thread->dabr = bp->info.address | bp->info.type |
> + DABR_TRANSLATION;
> + else
> + thread->dabr = 0;
> +}
> +
> +void arch_flush_thread_hw_breakpoint(struct task_struct *tsk)
> +{
> + struct thread_struct *thread = &(tsk->thread);
> +
> + thread->dabr = 0;
> +}
> +
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_handler(struct die_args *args)
> +{
> + int rc = NOTIFY_STOP;
> + struct hw_breakpoint *bp;
> + struct pt_regs *regs = args->regs;
> + unsigned long dar;
> + int cpu, stepped, is_kernel;
> +
> + /* Disable breakpoints during exception handling */
> + set_dabr(0);
> +
> + dar = regs->dar & (~HW_BREAKPOINT_ALIGN);
> + is_kernel = (dar >= TASK_SIZE) ? 1 : 0;
> +
> + if (is_kernel)
> + bp = hbp_kernel[0];
> + else {
> + bp = current->thread.hbp[0];
> + /* Lazy debug register switching */
> + if (!bp)
> + return rc;
> + rc = NOTIFY_DONE;
> + }
> +
> + (bp->triggered)(bp, regs);
> +
> + cpu = get_cpu();
> + if (is_kernel)
> + per_cpu(dabr_data, cpu) = kdabr;
> + else
> + per_cpu(dabr_data, cpu) = current->thread.dabr;
> +
> + stepped = emulate_step(regs, regs->nip);
> + /*
> + * Single-step the causative instruction manually if
> + * emulate_step() could not execute it
> + */
> + if (stepped == 0) {
> + regs->msr |= MSR_SE;
> + goto out;
> + }
This is where we backout to single step mode?
> +
> + set_dabr(per_cpu(dabr_data, cpu));
> +out:
> + /* Enable pre-emption only if single-stepping is finished */
> + if (stepped)
> + put_cpu_no_resched();
> + return rc;
> +}
> +
+/*
> + * Handle single-step exceptions following a DABR hit.
> + */
> +int __kprobes single_step_dabr_instruction(struct die_args *args)
> +{
> + struct pt_regs *regs = args->regs;
> + int cpu = get_cpu();
> + int ret = NOTIFY_DONE;
> + siginfo_t info;
> + unsigned long this_dabr_data = per_cpu(dabr_data, cpu);
> +
> + /*
> + * Check if we are single-stepping as a result of a
> + * previous HW Breakpoint exception
> + */
> + if (this_dabr_data == 0)
> + goto out;
> +
> + regs->msr &= ~MSR_SE;
> + /* Deliver signal to user-space */
> + if (this_dabr_data < TASK_SIZE) {
> + info.si_signo = SIGTRAP;
> + info.si_errno = 0;
> + info.si_code = TRAP_HWBKPT;
> + info.si_addr = (void __user *)(per_cpu(dabr_data, cpu));
> + force_sig_info(SIGTRAP, &info, current);
> + }
> +
> + set_dabr(this_dabr_data);
> + per_cpu(dabr_data, cpu) = 0;
> + ret = NOTIFY_STOP;
> + put_cpu_no_resched();
> +
> +out:
> + put_cpu_no_resched();
This looks wrong. put_cpu_no_resched() twice?
> + return ret;
> +}
> +
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_exceptions_notify(
> + struct notifier_block *unused, unsigned long val, void *data)
> +{
> + int ret = NOTIFY_DONE;
> +
> + switch (val) {
> + case DIE_DABR_MATCH:
> + ret = hw_breakpoint_handler(data);
> + break;
> + case DIE_SSTEP:
> + ret = single_step_dabr_instruction(data);
> + break;
> + }
> +
> + return ret;
> +}
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -0,0 +1,52 @@
> +#ifndef _PPC64_HW_BREAKPOINT_H
> +#define _PPC64_HW_BREAKPOINT_H
> +
> +#ifdef __KERNEL__
> +#define __ARCH_HW_BREAKPOINT_H
> +
> +struct arch_hw_breakpoint {
> + char *name; /* Contains name of the symbol to set bkpt */
> + unsigned long address;
> + u8 type;
> +};
Can you reorder this to pack the struct better (ie. put the unsigned
long first).
> +
> +#include <linux/kdebug.h>
> +#include <asm/reg.h>
> +#include <asm-generic/hw_breakpoint.h>
> +
> +#define HW_BREAKPOINT_READ DABR_DATA_READ
> +#define HW_BREAKPOINT_WRITE DABR_DATA_WRITE
> +#define HW_BREAKPOINT_RW DABR_DATA_RW
> +
> +#define HW_BREAKPOINT_ALIGN 0x7
> +#define HW_BREAKPOINT_LEN 4
What is HW_BREAKPOINT_LEN?
Obviouslt all instructions on ppc64 are 4 bytes. This seems to be
defined in a few places, like here and MCOUNT_INSN_SIZE (ftrace.h). Can
you create a #define for this generically and get all refers to use this
instead of #define-ing 4 everywhere.
> +
> +extern struct hw_breakpoint *hbp_kernel[HB_NUM];
> +extern unsigned int hbp_user_refcount[HB_NUM];
> +
> +/*
> + * Ptrace support: breakpoint trigger routine.
> + */
> +extern int __modify_user_hw_breakpoint(int pos, struct task_struct *tsk,
> + struct hw_breakpoint *bp);
> +
> +extern void arch_install_thread_hw_breakpoint(struct task_struct *tsk);
> +extern void arch_uninstall_thread_hw_breakpoint(void);
> +extern int arch_check_va_in_userspace(unsigned long va, u8 hbp_len);
> +extern int arch_check_va_in_kernelspace(unsigned long va, u8 hbp_len);
> +extern int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
> + struct task_struct *tsk);
> +extern void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk)
;
> +extern void arch_flush_thread_hw_breakpoint(struct task_struct *tsk);
> +extern void arch_update_kernel_hw_breakpoint(void *);
> +extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
> + unsigned long val, void *data);
> +
> +extern void flush_thread_hw_breakpoint(struct task_struct *tsk);
> +extern int copy_thread_hw_breakpoint(struct task_struct *tsk,
> + struct task_struct *child, unsigned long clone_flags);
> +extern void switch_to_thread_hw_breakpoint(struct task_struct *tsk);
> +
> +#endif /* __KERNEL__ */
> +#endif /* _PPC64_HW_BREAKPOINT_H */
> +
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/processor.h
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
> @@ -177,6 +177,7 @@ struct thread_struct {
> #ifdef CONFIG_PPC64
> unsigned long start_tb; /* Start purr when proc switched in */
> unsigned long accum_tb; /* Total accumilated purr for process *
/
> + struct hw_breakpoint *hbp[HB_NUM];
> #endif
> unsigned long dabr; /* Data address breakpoint register */
> #ifdef CONFIG_ALTIVEC
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/ptrace.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/ptrace.c
> @@ -37,6 +37,9 @@
> #include <asm/page.h>
> #include <asm/pgtable.h>
> #include <asm/system.h>
> +#ifdef CONFIG_PPC64
> +#include <asm/hw_breakpoint.h>
> +#endif
>
> /*
> * does not yet catch signals sent when the child dies.
> @@ -735,9 +738,22 @@ void user_disable_single_step(struct tas
> clear_tsk_thread_flag(task, TIF_SINGLESTEP);
> }
>
> +static void ptrace_triggered(struct hw_breakpoint *bp, struct pt_regs *regs)
> +{
> + /*
> + * The SIGTRAP signal is generated automatically for us in do_dabr().
> + * We don't have to do anything here
> + */
> +}
> +
> int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
> unsigned long data)
> {
> +#ifdef CONFIG_PPC64
> + struct thread_struct *thread = &(task->thread);
> + struct hw_breakpoint *bp;
> + int ret;
> +#endif
> /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
> * For embedded processors we support one DAC and no IAC's at the
> * moment.
> @@ -767,6 +783,38 @@ int ptrace_set_debugreg(struct task_stru
> if (data && !(data & DABR_TRANSLATION))
> return -EIO;
>
> +#ifdef CONFIG_PPC64
> + bp = thread->hbp[0];
> + if ((data & ~0x7UL) == 0) {
Should use HW_BREAKPOINT_ALIGN here instead of 0x7? Is 0 some special
command? Seems to be lots of special numbers here related to using data
(later you mask is 0x3). What is happening here? What is contained in
the data parameter? It overloads the type and the address?
> + if (bp) {
> + unregister_user_hw_breakpoint(task, bp);
> + kfree(bp);
> + thread->hbp[0] = NULL;
> + }
> + return 0;
> + }
> +
> + if (bp) {
> + bp->info.type = data & 0x3UL;
> + task->thread.dabr = bp->info.address =
> + (data & ~HW_BREAKPOINT_ALIGN);
> + return __modify_user_hw_breakpoint(0, task, bp);
> + }
> + bp = kzalloc(sizeof(struct hw_breakpoint), GFP_KERNEL);
When a processes ends, will this be correctly freed? Should it be freed
in arch_flush_thread_hw_breakpoint?
> + if (!bp)
> + return -ENOMEM;
> +
> + /* Store the type of breakpoint */
> + bp->info.type = data & 0x3UL;
> + bp->triggered = ptrace_triggered;
> + task->thread.dabr = bp->info.address = (data & ~HW_BREAKPOINT_ALIGN);
> +
> + ret = register_user_hw_breakpoint(task, bp);
> + if (ret)
> + return ret;
> + set_tsk_thread_flag(task, TIF_DEBUG);
> +#endif /* CONFIG_PPC64 */
> +
> /* Move contents to the DABR register */
> task->thread.dabr = data;
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/kernel/process.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/kernel/process.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/kernel/process.c
> @@ -50,6 +50,7 @@
> #include <asm/syscalls.h>
> #ifdef CONFIG_PPC64
> #include <asm/firmware.h>
> +#include <asm/hw_breakpoint.h>
> #endif
> #include <linux/kprobes.h>
> #include <linux/kdebug.h>
> @@ -254,8 +255,10 @@ void do_dabr(struct pt_regs *regs, unsig
> 11, SIGSEGV) == NOTIFY_STOP)
> return;
>
> +#ifndef CONFIG_PPC64
> if (debugger_dabr_match(regs))
> return;
> +#endif
>
> /* Clear the DAC and struct entries. One shot trigger */
> #if defined(CONFIG_BOOKE)
> @@ -372,8 +375,13 @@ struct task_struct *__switch_to(struct t
>
> #endif /* CONFIG_SMP */
>
> +#ifdef CONFIG_PPC64
> + if (unlikely(test_tsk_thread_flag(new, TIF_DEBUG)))
> + switch_to_thread_hw_breakpoint(new);
> +#else
> if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
> set_dabr(new->thread.dabr);
> +#endif /* CONFIG_PPC64 */
>
> #if defined(CONFIG_BOOKE)
> /* If new thread DAC (HW breakpoint) is the same then leave it */
> @@ -550,6 +558,10 @@ void show_regs(struct pt_regs * regs)
> void exit_thread(void)
> {
> discard_lazy_cpu_state();
> +#ifdef CONFIG_PPC64
> + if (unlikely(test_tsk_thread_flag(current, TIF_DEBUG)))
> + flush_thread_hw_breakpoint(current);
> +#endif /* CONFIG_PPC64 */
> }
>
> void flush_thread(void)
> @@ -605,6 +617,9 @@ int copy_thread(unsigned long clone_flag
> struct pt_regs *childregs, *kregs;
> extern void ret_from_fork(void);
> unsigned long sp = (unsigned long)task_stack_page(p) + THREAD_SIZE;
> +#ifdef CONFIG_PPC64
> + struct task_struct *tsk = current;
> +#endif
>
> CHECK_FULL_REGS(regs);
> /* Copy registers */
> @@ -672,6 +687,9 @@ int copy_thread(unsigned long clone_flag
> * function.
> */
> kregs->nip = *((unsigned long *)ret_from_fork);
> +
> + if (unlikely(test_tsk_thread_flag(tsk, TIF_DEBUG)))
> + copy_thread_hw_breakpoint(tsk, p, clone_flags);
> #else
> kregs->nip = (unsigned long)ret_from_fork;
> #endif
> Index: linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/samples/hw_breakpoint/data_breakpoint.c
> +++ linux-2.6-tip.hbkpt/samples/hw_breakpoint/data_breakpoint.c
> @@ -54,6 +54,10 @@ static int __init hw_break_module_init(v
> sample_hbp.info.type = HW_BREAKPOINT_WRITE;
> sample_hbp.info.len = HW_BREAKPOINT_LEN_4;
> #endif /* CONFIG_X86 */
> +#ifdef CONFIG_PPC64
> + sample_hbp.info.name = ksym_name;
> + sample_hbp.info.type = DABR_DATA_WRITE;
> +#endif /* CONFIG_PPC64 */
>
> sample_hbp.triggered = (void *)sample_hbp_handler;
>
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/thread_info.h
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/thread_info.h
> @@ -114,6 +114,7 @@ static inline struct thread_info *curren
> #define TIF_FREEZE 14 /* Freezing for suspend */
> #define TIF_RUNLATCH 15 /* Is the runlatch enabled? */
> #define TIF_ABI_PENDING 16 /* 32/64 bit switch needed */
> +#define TIF_DEBUG 17 /* uses debug registers */
>
> /* as above, but as bit values */
> #define _TIF_SYSCALL_TRACE (1<<TIF_SYSCALL_TRACE)
> @@ -132,6 +133,7 @@ static inline struct thread_info *curren
> #define _TIF_FREEZE (1<<TIF_FREEZE)
> #define _TIF_RUNLATCH (1<<TIF_RUNLATCH)
> #define _TIF_ABI_PENDING (1<<TIF_ABI_PENDING)
> +#define _TIF_DEBUG (1<<TIF_DEBUG)
> #define _TIF_SYSCALL_T_OR_A (_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SEC
COMP)
>
> #define _TIF_USER_WORK_MASK (_TIF_SIGPENDING | _TIF_NEED_RESCHED | \
> Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/reg.h
> +++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
> @@ -184,9 +184,11 @@
> #define CTRL_TE 0x00c00000 /* thread enable */
> #define CTRL_RUNLATCH 0x1
> #define SPRN_DABR 0x3F5 /* Data Address Breakpoint Register */
> +#define HB_NUM 1 /* Number of physical HW breakpoint registers *
/
You've shortedned "hardware breakpoint" to "hbp" elsewhere. Can you be
consistent here so change this to HBP_NUM?
> #define DABR_TRANSLATION (1UL << 2)
> #define DABR_DATA_WRITE (1UL << 1)
> #define DABR_DATA_READ (1UL << 0)
> +#define DABR_DATA_RW (3UL << 0)
> #define SPRN_DABR2 0x13D /* e300 */
> #define SPRN_DABRX 0x3F7 /* Data Address Breakpoint Register Extension *
/
> #define DABRX_USER (1UL << 0)
> Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> ===================================================================
> --- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
> +++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> @@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
> error_code &= 0x48200000;
> else
> is_write = error_code & DSISR_ISSTORE;
> +
> + if (error_code & DSISR_DABRMATCH) {
> + /* DABR match */
> + do_dabr(regs, address, error_code);
> + return 0;
> + }
> #else
> is_write = error_code & ESR_DST;
> #endif /* CONFIG_4xx || CONFIG_BOOKE */
> @@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
> if (!user_mode(regs) && (address >= TASK_SIZE))
> return SIGSEGV;
>
> -#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> - if (error_code & DSISR_DABRMATCH) {
> - /* DABR match */
> - do_dabr(regs, address, error_code);
> - return 0;
> - }
> -#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
> -
> if (in_atomic() || mm == NULL) {
> if (!user_mode(regs))
> return SIGSEGV;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
^ permalink raw reply
* Re: device trees.
From: David Gibson @ 2009-05-11 23:27 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <4A085862.5070303@dlasys.net>
On Mon, May 11, 2009 at 12:54:58PM -0400, David H. Lynch Jr. wrote:
> Timur Tabi wrote:
> > On Mon, May 11, 2009 at 1:32 AM, David H. Lynch Jr. <dhlii@dlasys.net> wrote:
> >
> > So all you need to do is have your boot loader create a device tree
> > from scratch. If you're using U-Boot, then you can already do this by
> > making the appropriate libfdt calls. Otherwise, you should probably
> > add libfdt to your boot loader.
> >
> As I mentioned before, we do nto use u-boot. I am not looking to
> start a debate on it either, but it does not meet a number of our needs,
> and would require significant architectural changes to do so. The
> difference between it and devicetrees is that u-boot is avaiable to us
> if we want, I did port u-boot to our hardware at one point and it did
> everything it promised, but u-boot is optional, device trees are not.
> I do not have to re-architect u-boot to fit into 16k of bram, or
> load bit files or .....
> If I want to move past 2.6.26 I have to not only use device trees
> but actually make them work in a way that will function as we need with
> our systems.
> It is likely I will use libdft as a starting point, but I can not
> see it as more than a short term solution. libdft is orders of magnitude
> large than our entire monitor, and it is a toolkit rather than the whole
> solution.
I'm very willing to make changes to libfdt, if it helps make it more
useful to you without breaking it for other people. I'm a little
horrified at the size of libfdt, and a bit baffled as to why it's
ending up that large. From some initial poking around, it looks like
-fPIC inflates the code size a *lot* (~3x on x86, which I know is not
what we're dealing with here, but was the easiest for a quick test).
So if you don't need that in your context, that could save a heap.
The separation into separate .o files is also deliberate, so that
portions you don't use can be omitted. I have considered breaking it
up into many more .c files, so that you can omit individual not-needed
functions.
--
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: device trees.
From: David Gibson @ 2009-05-11 22:56 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
On Mon, May 11, 2009 at 05:38:16PM -0400, David H. Lynch Jr. wrote:
> Grant Likely wrote:
> >
> > What do you mean by "one size fits all solution?"
> >
> > The kernel doesn't care where the device tree comes from. All it
> > cares about is that by the time the kernel is started the device tree
> > must be fully formed and populated. It can be completely pre-canned
> > (like simpleImage), it can be modified by firmware (like u-boot), or
> > it can be generated from scratch (like with real OpenFirmware). There
> > is lots of flexibility on how to handle it.
> First device trees are now the ONLY means of passing information to
> the kernel.
That's not really true in practice. Yes, the device tree is the only
way to pass information to the kernel proper, but having a
bootwrapper, built along with the kernel, which translates information
in some other form into a device tree is a perfectly reasonable
solution for the right circumstances.
> By definition that means it is a one size fits all
> solution. While there is nothing inherently wrong with that,
> solutions intended to meet all circumstances need to be simple,
> powerful, and flexible. They need to work well 100% of the time not
> 98%.
>
> Not only is the device tree expected to pass static hardware
> configuration information, but it is the sole means of passing anything.
> As an example Command lines are to be in the device tree.
> Everything is supposed to be in the device tree, whether that
> information is static or dynamic, whether it is hardware information,
> or user choices.
>
> That means that whether you are in a Sun or Apple Desktop or a
> system with the no flash and barely enough resources to run Linux,
> you still may have to manipulate the device tree.
Compared to running Linux, manipulating the device tree is a complete
triviality, so I don't see what the problem is here.
[snip]
> Welding the bit file to the dtb might solve 75% of my issues,
> And it probably would get me to the point where I could move
> forward and live with
> the remaining issues untile I was inspired to solve them.
> but it does not solve everything. It is increasingly clear to me
> that I am going to have to
> manipulate device trees.
That's probably true. But you don't necessarily have to do it within
your BRAM firmware - you can do it inside the Linux bootwrapper.
> >> Anyway, all I was looking for was a leg up on figuring out how to do
> >> what I want with them. Rather than starting from scratch.
> >> I am not looking to be convinced that I am approaching this all wrong.
> >> If you are happy with what you have - great. I am not.
> >> While I was not looking to restart a great debate over device trees
> >> - I do not actually think they are a bad idea.
> >>
> >
> > I still don't understand what you're worried about starting an arguing
> > about. Pretty much any of the PowerPC maintainers can point at warts
> > and problems in the current handling of device trees. I'm not
> > particularly happy with simpleImage (and I wrote it), but it takes
> > time and effort to write something more capable.
> >
> I was not trying to start an argument, my initial question was
>
> "Is there an example somewhere that shows building a device tree on the fly ?"
>
> The responses have questioned why I want to do that rather than how can I do that.
> I was not actually seeking a debate over the merit of device trees, or u-boot, or libdft, or ....
Probably the closest things to suitable examples here are
arch/powerpc/kernel/prom_init.c, which builds the dtb from the "live"
OF device tree. At the moment that's a bit of a special case, but
there's no inherent reason that logic couldn't be moved to the
bootwrapper.
The other, not in the main tree, is Paul's restodt, which builds a
device tree from old-style PReP residual information. I recall it was
sent to the list as a userspace program (we were gathering information
on the sorts of PReP out there). I think Paul made at least a first
cut at fusing it into the bootwrapper, but I don't know if that ever
got sent out to the list.
--
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: question about softirqs
From: Chris Friesen @ 2009-05-12 0:43 UTC (permalink / raw)
To: David Miller; +Cc: linuxppc-dev, Ingo Molnar, paulus, netdev
In-Reply-To: <20090511.162436.193717082.davem@davemloft.net>
This started out as a thread on the ppc list, but on the suggestion of
DaveM and Paul Mackerras I'm expanding the receiver list a bit.
Currently, if a softirq is raised in process context the
TIF_RESCHED_PENDING flag gets set and on return to userspace we run the
scheduler, expecting it to switch to ksoftirqd to handle the softirqd
processing.
I think I see a possible problem with this. Suppose I have a SCHED_FIFO
task spinning on recvmsg() with MSG_DONTWAIT set. Under the scenario
above, schedule() would re-run the spinning task rather than ksoftirqd,
thus preventing any incoming packets from being sent up the stack until
we get a real hardware interrupt--which could be a whole jiffy if
interrupt mitigation is enabled in the net device.
DaveM pointed out that if we're doing transmits we're likely to hit
local_bh_enable(), which would process the softirq work. However, I
think we may still have a problem in the above rx-only scenario--or is
it too contrived to matter?
Thanks,
Chris
^ permalink raw reply
* Re: device trees.
From: Grant Likely @ 2009-05-12 0:04 UTC (permalink / raw)
To: Stephen Neuendorffer; +Cc: linuxppc-dev, David H. Lynch Jr.
In-Reply-To: <20090511231915.007AC1430052@mail201-wa4.bigfish.com>
On Mon, May 11, 2009 at 5:19 PM, Stephen Neuendorffer
<stephen.neuendorffer@xilinx.com> wrote:
>
>> >> =A0 =A0The best alternative to creating the device tree dynamically
> would
>> >> be to
>> >> =A0 =A0append the devicetree to the bitimage in a way the boot loader
> could
>> >> always find it.
>> >>
>> >
>> > That sounds like a good solution to me.
>> >
>> =A0 =A0 I am glad you like it. If Xilinx would like to offer any advice =
as
>> to how to prepend a device tree to the end of a bit file without
>> =A0 =A0 foreclosing any of their future plans or .... I would be happy t=
o
>> look at implementing it.
>
> There are options here...
>
> For the benefit of the mailing list: they are tied into how the FPGA
> configuration is loaded, and the
> particular FPGA family. =A0By far the most portable/generic option is to
> simply use an initialized BRAM for this.
>
> David: If you would like to have a discussion regarding particular
> design tradeoffs, I'd be happy to, but since I doubt there is anyone on
> this list who is interested in the vagaries of FPGA configuration
> methods, I suggest we have the discussion privately.
I disagree. There are lots of lurkers on this list who care about
virtex stuff, not to mention non-lurkers like me. :-) I would at
least like to be a fly on the wall of any such discussion.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* RE: device trees.
From: Stephen Neuendorffer @ 2009-05-11 23:19 UTC (permalink / raw)
To: David H. Lynch Jr., grant.likely; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
> >> The best alternative to creating the device tree dynamically
would
> >> be to
> >> append the devicetree to the bitimage in a way the boot loader
could
> >> always find it.
> >>
> >
> > That sounds like a good solution to me.
> >
> I am glad you like it. If Xilinx would like to offer any advice as
> to how to prepend a device tree to the end of a bit file without
> foreclosing any of their future plans or .... I would be happy to
> look at implementing it.
There are options here...
For the benefit of the mailing list: they are tied into how the FPGA
configuration is loaded, and the
particular FPGA family. By far the most portable/generic option is to
simply use an initialized BRAM for this.
David: If you would like to have a discussion regarding particular
design tradeoffs, I'd be happy to, but since I doubt there is anyone on
this list who is interested in the vagaries of FPGA configuration
methods, I suggest we have the discussion privately.
Steve
This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: question about softirqs
From: Paul Mackerras @ 2009-05-11 23:34 UTC (permalink / raw)
To: Chris Friesen; +Cc: linuxppc-dev, David Miller
In-Reply-To: <4A086DB2.8040703@nortel.com>
Chris Friesen writes:
> Suppose I have a SCHED_FIFO task spinning on recvmsg() with MSG_DONTWAIT
> set (and maybe doing other stuff if there are no messages). In this
> case, schedule() would re-run the spinning task rather than running
> ksoftirqd. This could prevent any incoming packets from actually being
> sent up the stack until we get a real hardware interrupt--which could be
> a whole jiffy if interrupt mitigation is enabled in the net device.
I suggest you ask Ingo Molnar about that.
> (And maybe longer if NOHZ is enabled.)
We still have a timer interrupt every jiffy when stuff is running; we
only turn off the timer interrupts when idle.
Paul.
^ permalink raw reply
* Re: question about softirqs
From: David Miller @ 2009-05-11 23:24 UTC (permalink / raw)
To: cfriesen; +Cc: linuxppc-dev, paulus
In-Reply-To: <4A086DB2.8040703@nortel.com>
From: "Chris Friesen" <cfriesen@nortel.com>
Date: Mon, 11 May 2009 12:25:54 -0600
> David Miller wrote:
>
>> You know, for networking over loopback (one of the only real cases
>> that even matters, if we get a hard interrupt then the return from
>> that would process any softints), we probably make out just fine
>> anyways. As long as we hit a local_bh_enable() (and in the return
>> path from device transmit that's exceedingly likely as all of the
>> networking locking is BH safe) we'll run the softints from that and
>> thus long before we get to syscall return.
>
> What about the issue I raised earlier? (I don't think you were copied
> at that point.)
I'm sure all of the networking experts on linuxppc-dev will have
an answer.
And yes that was sarcasm :-) You need to ask this on netdev or similar
list.
^ permalink raw reply
* Re: [PATCH] i2c-cpm: Pass dev ptr to dma_*_coherent rather than NULL
From: Ben Dooks @ 2009-05-11 23:18 UTC (permalink / raw)
To: Mark Ware; +Cc: Linuxppc-dev Development, Ben Dooks, linux-i2c
In-Reply-To: <4A07A982.40504@elphinstone.net>
On Mon, May 11, 2009 at 02:28:50PM +1000, Mark Ware wrote:
> Ben Dooks wrote:
>> On Wed, Apr 29, 2009 at 08:43:14AM -0500, Kumar Gala wrote:
>>> On Apr 22, 2009, at 4:56 PM, Ben Dooks wrote:
>>>
>>>> On Tue, Apr 21, 2009 at 10:11:51AM -0500, Kumar Gala wrote:
>>>>> On Apr 21, 2009, at 7:49 AM, Mark Ware wrote:
>>>>>
>>>>>> Recent DMA changes result in a BUG() when NULL is passed to
>>>>>> dma_alloc_coherent in place of a device.
>>>>>>
>>>>>> Signed-off-by: Mark Ware <mware@elphinstone.net>
>>>>>> ---
>>>>>>
>>>>>> This patch fixes the BUG() during boot that has appeared during the
>>>>>> 2.6.30 window. It has been tested and appears correct on my
>>>>>> 8280 based
>>>>>> board.
>>>>>> Sent to both linuxppc-dev and linux-i2c, since I'm not sure where it
>>>>>> belongs.
>>>>>>
>>>>>>
>>>>>> drivers/i2c/busses/i2c-cpm.c | 14 ++++++++------
>>>>>> 1 files changed, 8 insertions(+), 6 deletions(-)
>>>>> Acked-by: Kumar Gala <galak@kernel.crashing.org>
>>>>>
>>>>> Ben, I'm expecting you to pick this up unless you tell me otherwise.
>>>> Yes.
>>> This go in yet?
>>
>> I've had to do a manual apply due to some changes in the
>> driver, so can someone please do a build of my git tree
>> at:
>>
>> git://aeryn.fluff.org.uk/bjdooks/linux.git i2c-for-2630-rc5
>>
>> or tell me which arch and defconfig to build.
>>
>
> Ping. Is there anything still blocking this?
sorry, have been ill recently, will get this sorted and
pushed.
--
Ben (ben@fluff.org, http://www.fluff.org/)
'a smiley only costs 4 bytes'
^ permalink raw reply
* Re: device trees.
From: Grant Likely @ 2009-05-11 23:09 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
On Mon, May 11, 2009 at 3:38 PM, David H. Lynch Jr. <dhlii@dlasys.net> wrot=
e:
> Grant Likely wrote:
>>
>> What do you mean by "one size fits all solution?"
>>
>> The kernel doesn't care where the device tree comes from. =A0All it
>> cares about is that by the time the kernel is started the device tree
>> must be fully formed and populated. =A0It can be completely pre-canned
>> (like simpleImage), it can be modified by firmware (like u-boot), or
>> it can be generated from scratch (like with real OpenFirmware). =A0There
>> is lots of flexibility on how to handle it.
>>
>>
> First device trees are now the ONLY means of =A0passing information to th=
e
> kernel.
> By definition that means it is a one size fits all solution.
> While there is nothing inherently wrong with that, solutions intended to
> meet all circumstances need to be
> simple, powerful, and flexible. They need to work well 100% of the time
> not 98%.
>
> Not only is the device tree expected to pass static hardware
> configuration information, but it is the sole means of passing anything.
> As an example Command lines are to be in the device tree.
> Everything is supposed to be in the device tree, whether that
> information is static or dynamic, whether it is hardware information,
> or user choices.
It is the sole means of passing anything *to the kernel*. You can
pass whatever you like to the bootwrapper. :-)
> =A0 =A0That means that whether you are in a Sun or Apple Desktop or a
> system with the no flash and barely enough resources to run Linux,
> you still may have to manipulate the device tree.
...or if you really are constrained, then define a format for the data
you want to pass, pass it to the bootwrapper along with the device
tree, and use the bootwrapper (which already has libfdt) to update the
device tree. cuImage targets do this to support older u-boots which
don't understand device trees. You are not forced to put device tree
support in your firmware.
In other words; having your bootloader support FDT is preferred, but
not required.
Word of warning; if you do define your own format, be very very very
careful. Otherwise you end up with something subtly broken and not
future proof. This is one of the reasons adding FDT support to your
firmware is preferred; these issues have already been thought about.
> =A0 =A0Though I still have an issue. One of the problem with 98% solution=
s,
> is that they result in a chain of work arrounds for
> =A0 =A0the other 2%. Instead of one case or two cases, you end up with a
> dozen cases each handling increasingly tiny slivers.
> =A0 =A0And it becomes increasingly easy to claim that the problem is with
> the slivers not the broad solution.
No, it's not. Everything you need to do can be done within the
bootwrapper if you so desire, passing data in whatever format you
like.
> =A0 =A0I am not sure what you are saying here ?
> =A0 =A0By firmware do you mean bootloader ? Or do you mean bitfiles ?
Yes, I'm using the terms "firmware" and "bootloader" interchangeably.
>> I still don't understand what you're worried about starting an arguing
>> about. =A0Pretty much any of the PowerPC maintainers can point at warts
>> and problems in the current handling of device trees. =A0I'm not
>> particularly happy with simpleImage (and I wrote it), but it takes
>> time and effort to write something more capable.
>>
> =A0 =A0I was not trying to start an argument, my initial question was
>
> =A0 "Is there an example somewhere that shows building a device tree on t=
he fly ?"
>
> =A0 The responses have questioned why I want to do that rather than how c=
an I do that.
> =A0 I was not actually seeking a debate over the merit of device trees, o=
r u-boot, or libdft, or ....
... but the questions were necessary to understand your problem set.
I cannot give good advice without understanding.
g.
--=20
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
^ permalink raw reply
* Re: [PATCH] fix sata_sil compilation on non-DMI platforms
From: Benjamin Herrenschmidt @ 2009-05-11 22:31 UTC (permalink / raw)
To: Alan Cox
Cc: Jeff Garzik, linuxppc-dev, Konstantinos Margaritis, linux-ide,
rmk, Lennart Sorensen, Lennert Buytenhek
In-Reply-To: <20090511194410.46641214@lxorguk.ukuu.org.uk>
On Mon, 2009-05-11 at 19:44 +0100, Alan Cox wrote:
> > Ideally the DMI subsystem should be provided wrappers for platforms
> > without DMI, rendering patches like this unnecessary.
>
> Interesting question - is the PPC OpenFirmware machine information
> mappable onto the DMI space ? or the various static bits of name
> information in the various ARM and the like machine descriptions.
>
> If not which bits are similar enough we could replace dmi at the high
> level with an abstract interface for system/vendor/... that was ?
The one thing we could try to map would be the device-tree "compatible"
property which would contain vendor,system tuples from more specific to
more generic with which the machine is compatible with.
Of course I would argue the other way around and create a device-tree
from the DMI data :-)
Cheers,
Ben.
^ permalink raw reply
* Re: device trees.
From: Benjamin Herrenschmidt @ 2009-05-11 22:29 UTC (permalink / raw)
To: David H. Lynch Jr.; +Cc: linuxppc-dev
In-Reply-To: <4A089AC8.9080704@dlasys.net>
On Mon, 2009-05-11 at 17:38 -0400, David H. Lynch Jr. wrote:
> Not only is the device tree expected to pass static hardware
> configuration information, but it is the sole means of passing
> anything.
> As an example Command lines are to be in the device tree.
> Everything is supposed to be in the device tree, whether that
> information is static or dynamic, whether it is hardware information,
> or user choices.
>
> That means that whether you are in a Sun or Apple Desktop or a
> system with the no flash and barely enough resources to run Linux,
> you still may have to manipulate the device tree.
To some extend, but the user choice information is well isolated, it's
all in /chosen, so it's really only that node that needs to be modified
or created from scratch by the bootloader.
Thus, your bootloader can pick the right static dtb for a given bitfile,
and then just use libfdt to generate the right /chosen node with other
informations.
Ben.
^ permalink raw reply
* Re: device trees.
From: Benjamin Herrenschmidt @ 2009-05-11 22:25 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev, dhlii
In-Reply-To: <fa686aa40905102108l5c964893y2dfb84763e13511@mail.gmail.com>
> > Not pretty, but it does more or less what you're talking about. Would
> > need some work to get it going on !pseries obviously.
>
> Heh, I didn't even know this existed. :-)
>
> Thinking about this more, it seems to me that the tricky bit would be
> figuring out how to drop all references to a node before it is pruned
> from the tree. of_platform_devices would probably be the easiest
> because the bus could walked before pruning the node, but there are
> also references on the i2c, spi and mdio busses that must be dealt
> with appropriately.
Also, the way userspace manipulates the tree via /proc/ppc64 is quite
gross... Some time ago we discussed about moving that to the kernel
since the firmware call that gives us the new bits of tree could be done
there in a relatively generic way but that's only one option.
Maybe it's time to have a proper devicetreefs so one can use standard fs
APIs to create nodes & properties though we would need a trick to turn
them from "temporary" to "online" and similar for offlining. Maybe with
mv from/to a "staging" directory that isn't "live" vs. the kernel.
Cheers,
Ben.
^ permalink raw reply
* Re: device trees.
From: David H. Lynch Jr. @ 2009-05-11 21:38 UTC (permalink / raw)
To: Grant Likely; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40905111047p5f350b0dh796564cc1383a336@mail.gmail.com>
Grant Likely wrote:
>
> What do you mean by "one size fits all solution?"
>
> The kernel doesn't care where the device tree comes from. All it
> cares about is that by the time the kernel is started the device tree
> must be fully formed and populated. It can be completely pre-canned
> (like simpleImage), it can be modified by firmware (like u-boot), or
> it can be generated from scratch (like with real OpenFirmware). There
> is lots of flexibility on how to handle it.
>
>
First device trees are now the ONLY means of passing information to the
kernel.
By definition that means it is a one size fits all solution.
While there is nothing inherently wrong with that, solutions intended to
meet all circumstances need to be
simple, powerful, and flexible. They need to work well 100% of the time
not 98%.
Not only is the device tree expected to pass static hardware
configuration information, but it is the sole means of passing anything.
As an example Command lines are to be in the device tree.
Everything is supposed to be in the device tree, whether that
information is static or dynamic, whether it is hardware information,
or user choices.
That means that whether you are in a Sun or Apple Desktop or a
system with the no flash and barely enough resources to run Linux,
you still may have to manipulate the device tree.
>> The best alternative to creating the device tree dynamically would
>> be to
>> append the devicetree to the bitimage in a way the boot loader could
>> always find it.
>>
>
> That sounds like a good solution to me.
>
I am glad you like it. If Xilinx would like to offer any advice as
to how to prepend a device tree to the end of a bit file without
foreclosing any of their future plans or .... I would be happy to
look at implementing it.
> As for using up BRAM, a gzipped dtb image is smaller than 2k and it
> can be reclaimed for other uses after the kernel has booted. That may
> not help your situation, but for my use cases the tradeoff works.
>
If I recall the minimum increment for BRAM is 16K.
I am not trying to claim it is not an answer for anyone, or even
most people.
Though I still have an issue. One of the problem with 98% solutions,
is that they result in a chain of work arrounds for
the other 2%. Instead of one case or two cases, you end up with a
dozen cases each handling increasingly tiny slivers.
And it becomes increasingly easy to claim that the problem is with
the slivers not the broad solution.
>> Regardless it still makes my point. The problem with devicetrees as
>> they are is that they handle probably 98% of all cases well.
>> The remaining 2% are a mess.
>>
>
> No it isn't. It is expected that firmware will fixup the device tree
> data with board specific values. This is intentional. The device
> tree is simply the bearer. It makes no determination about where the
> data comes from.
>
I am not sure what you are saying here ?
By firmware do you mean bootloader ? Or do you mean bitfiles ?
In large systems like Sun or Apple desktop the OF Device tree need
not be static.
There is software that may well be larger than the linux kernel.
Our "firmware" bootloader is actually stored in the bitfile - 16K of
BRAM basically becomes the boot ROM - except that it is bitfile
initialized RAM.
I guess this is much like you dtb in BRAM.
We are not increasing the size of the BRAM, while most of our
systems have NOR flash, or ... or ...., the all don't.
Even the amount of DRAM varies, and our code is written not to use
DRAM until we have verified that it works.
>
>> lots of .dtb files lying arround is only a better solution than
>> simpleimage.
>> I will guarantee that unless they are welded together the wrong
>> device tree will get used with the wrong bit file.
>>
>
> I agree.
>
>
>> Inevitably I will make that mistake myself occasionally and waste
>> hours or possibly days trying to debug it.
>> And if I will do it rarely clients will do it frequently.
>>
>> In my expereince if you create a situation where confusion can exist
>> it will.
>>
>> It is also my expereince that time spend coding a solution to a
>> common client problem is well spent.
>> If it takes me a week to work out dynamically creating a device
>> tree, that ill likely save many weeks of
>> support headaches.
>>
>
> Again, it doesn't sound like you want dynamic *creation* of device
> trees. It sounds like you want a reliable way to make sure the
> bitstream is welded together with the correct dtb, preferably within
> the Xilinx toolchain.
>
Welding the bit file to the dtb might solve 75% of my issues,
And it probably would get me to the point where I could move
forward and live with
the remaining issues untile I was inspired to solve them.
but it does not solve everything. It is increasingly clear to me
that I am going to have to
manipulate device trees.
>
>> Even if I do not end up creating the device tree dynamically, I am
>> likely to end up at a minimum doing some validation on it.
>> i.e. once the bitfile is loaded scanning the device tree and probing
>> to ascertain that the hardware that I am supposed to expect
>> it really present.
>>
>
> If you like.
>
>
>> ultimately devicetrees are supposed to be a database not a black box.
>>
>
> I don't understand what you mean by this statement.
>
Right now for embedded systems device trees are a black box not a
database.
You get them from EDK and pass them on to linux.
Only Linux and the EDK understand and manipulate them.
I gather u-boot is twiddling at the fringes.
>
>> Anyway, all I was looking for was a leg up on figuring out how to do
>> what I want with them. Rather than starting from scratch.
>> I am not looking to be convinced that I am approaching this all wrong.
>> If you are happy with what you have - great. I am not.
>> While I was not looking to restart a great debate over device trees
>> - I do not actually think they are a bad idea.
>>
>
> I still don't understand what you're worried about starting an arguing
> about. Pretty much any of the PowerPC maintainers can point at warts
> and problems in the current handling of device trees. I'm not
> particularly happy with simpleImage (and I wrote it), but it takes
> time and effort to write something more capable.
>
I was not trying to start an argument, my initial question was
"Is there an example somewhere that shows building a device tree on the fly ?"
The responses have questioned why I want to do that rather than how can I do that.
I was not actually seeking a debate over the merit of device trees, or u-boot, or libdft, or ....
--
Dave Lynch DLA Systems
Software Development: Embedded Linux
717.627.3770 dhlii@dlasys.net http://www.dlasys.net
fax: 1.253.369.9244 Cell: 1.717.587.7774
Over 25 years' experience in platforms, languages, and technologies too numerous to list.
"Any intelligent fool can make things bigger and more complex... It takes a touch of genius - and a lot of courage to move in the opposite direction."
Albert Einstein
^ permalink raw reply
* Re: [PATCH] ps3: remove driver_data direct access of struct device
From: Geoff Levand @ 2009-05-11 21:07 UTC (permalink / raw)
To: Roel Kluin; +Cc: Geert.Uytterhoeven, linuxppc-dev, cbe-oss-dev, lkml
In-Reply-To: <4A087DB9.50305@gmail.com>
Hi Roel,
On 05/11/2009 12:34 PM, Roel Kluin wrote:
> To avoid direct access to the driver_data pointer in struct device, the
> functions dev_get_drvdata() and dev_set_drvdata() should be used.
>
> Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
> ---
> Please review. especially note that I removed a
> kfree(dev->sbd.core.driver_data);
> Is that correct?
Comment below.
> arch/powerpc/include/asm/ps3.h | 4 ++--
> drivers/char/ps3flash.c | 11 +++++------
> drivers/scsi/ps3rom.c | 10 +++++-----
> drivers/video/ps3fb.c | 6 +++---
> 4 files changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/ps3.h b/arch/powerpc/include/asm/ps3.h
> index cdb6fd8..a55717e 100644
> --- a/arch/powerpc/include/asm/ps3.h
> +++ b/arch/powerpc/include/asm/ps3.h
> @@ -421,12 +421,12 @@ static inline struct ps3_system_bus_driver *
> static inline void ps3_system_bus_set_driver_data(
> struct ps3_system_bus_device *dev, void *data)
> {
> - dev->core.driver_data = data;
> + dev_set_drvdata(&dev->core, data);
> }
> static inline void *ps3_system_bus_get_driver_data(
> struct ps3_system_bus_device *dev)
> {
> - return dev->core.driver_data;
> + return dev_get_drvdata(&dev->core);
> }
>
> /* These two need global scope for get_dma_ops(). */
> diff --git a/drivers/char/ps3flash.c b/drivers/char/ps3flash.c
> index afbe456..6083032 100644
> --- a/drivers/char/ps3flash.c
> +++ b/drivers/char/ps3flash.c
> @@ -108,7 +108,7 @@ static ssize_t ps3flash_read(struct file *file, char __user *buf, size_t count,
> loff_t *pos)
> {
> struct ps3_storage_device *dev = ps3flash_dev;
> - struct ps3flash_private *priv = dev->sbd.core.driver_data;
> + struct ps3flash_private *priv = dev_get_drvdata(&dev->sbd.core);
These should all be using ps3_system_bus_get_driver_data() and
ps3_system_bus_set_driver_data():
struct ps3flash_private *priv = ps3_system_bus_get_driver_data(&dev->sbd)
> @@ -404,8 +404,7 @@ static int ps3flash_remove(struct ps3_system_bus_device *_dev)
>
> misc_deregister(&ps3flash_misc);
> ps3stor_teardown(dev);
> - kfree(dev->sbd.core.driver_data);
> - dev->sbd.core.driver_data = NULL;
> + dev_set_drvdata(&dev->sbd.core, NULL);
It seems that will result in a memory leak. Why did you
think the kfree() should be removed?
Won't this work?
kfree(ps3_system_bus_get_driver_data(&dev->sbd));
ps3_system_bus_set_driver_data(&dev->sbd, NULL);
> ps3flash_dev = NULL;
> return 0;
> }
-Geoff
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox