* [PATCH] add Altivec/VMX state to coredumps
From: Mark Nelson @ 2007-09-25 4:03 UTC (permalink / raw)
To: linuxppc-dev
Update dump_task_altivec() (that has so far never been put to use)
so that it dumps the Altivec/VMX registers (VR[0] - VR[31], VSCR
and VRSAVE) in the same format as the ptrace get_vrregs() and add
the appropriate glue typedefs and #defines to
include/asm-powerpc/elf.h for it to work.
Signed-off-by: Mark Nelson <markn@au1.ibm.com>
---
arch/powerpc/kernel/process.c | 28 +++++++++++++++++++++++++---
include/asm-powerpc/elf.h | 7 +++++++
2 files changed, 32 insertions(+), 3 deletions(-)
Index: linux/arch/powerpc/kernel/process.c
===================================================================
--- linux.orig/arch/powerpc/kernel/process.c
+++ linux/arch/powerpc/kernel/process.c
@@ -149,10 +149,32 @@ void flush_altivec_to_thread(struct task
}
}
-int dump_task_altivec(struct pt_regs *regs, elf_vrregset_t *vrregs)
+int dump_task_altivec(struct task_struct *tsk, elf_vrregset_t *vrregs)
{
- flush_altivec_to_thread(current);
- memcpy(vrregs, ¤t->thread.vr[0], sizeof(*vrregs));
+ /* ELF_NVRREG includes the VSCR and VRSAVE which we need to save
+ * separately, see below */
+ const int nregs = ELF_NVRREG - 2;
+ elf_vrreg_t *reg;
+ u32 *dest;
+
+ if (tsk == current)
+ flush_altivec_to_thread(tsk);
+
+ reg = (elf_vrreg_t *)vrregs;
+
+ /* copy the 32 vr registers */
+ memcpy(reg, &tsk->thread.vr[0], nregs * sizeof(*reg));
+ reg += nregs;
+
+ /* copy the vscr */
+ memcpy(reg, &tsk->thread.vscr, sizeof(*reg));
+ reg++;
+
+ /* vrsave is stored in the high 32bit slot of the final 128bits */
+ memset(reg, 0, sizeof(*reg));
+ dest = (u32 *)reg;
+ *dest = tsk->thread.vrsave;
+
return 1;
}
#endif /* CONFIG_ALTIVEC */
Index: linux/include/asm-powerpc/elf.h
===================================================================
--- linux.orig/include/asm-powerpc/elf.h
+++ linux/include/asm-powerpc/elf.h
@@ -212,6 +212,13 @@ static inline int dump_task_regs(struct
extern int dump_task_fpu(struct task_struct *, elf_fpregset_t *);
#define ELF_CORE_COPY_FPREGS(tsk, elf_fpregs) dump_task_fpu(tsk, elf_fpregs)
+typedef elf_vrregset_t elf_fpxregset_t;
+
+#ifdef CONFIG_ALTIVEC
+extern int dump_task_altivec(struct task_struct *, elf_vrregset_t *vrregs);
+#define ELF_CORE_COPY_XFPREGS(tsk, regs) dump_task_altivec(tsk, regs)
+#endif
+
#endif /* __KERNEL__ */
/* ELF_HWCAP yields a mask that user programs can use to figure out what
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
From: Benjamin Herrenschmidt @ 2007-09-25 3:58 UTC (permalink / raw)
To: Roland McGrath
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
In-Reply-To: <20070925005945.02E924D04B7@magilla.localdomain>
On Mon, 2007-09-24 at 17:59 -0700, Roland McGrath wrote:
> > Yup, I think I ditched most of them.. for some reason I decided it
> > couldn't happen, but maybe I'm wrong ?
>
> Well, it's a BUG_ON. It's supposed to be for something that "can't happen".
> That's why it's a sanity check, not a wild assertion. ;-)
>
> The 2/2 patch is an example of a bug that CHECK_FULL_REGS catches.
> In the status quo, using PTRACE_PEEKUSR in a bug case crashes while using
> PTRACE_GETREGS in the same place might get bogus data. (In the actual bug
> thus found, the data is not bogus and only the bit that FULL_REGS checks is.)
Fair enough.
Ben.
^ permalink raw reply
* Re: [PATCH 8/15] bootwrapper: convert flatdevtree to version 16
From: David Gibson @ 2007-09-25 3:46 UTC (permalink / raw)
To: Milton Miller; +Cc: ppcdev, Paul Mackerras
In-Reply-To: <5e71423247aef068d82b84166102fd7d@bga.com>
On Mon, Sep 24, 2007 at 01:54:32AM -0500, Milton Miller wrote:
>
> On Sep 23, 2007, at 10:36 PM, David Gibson wrote:
>
> > On Fri, Sep 21, 2007 at 06:05:06PM -0500, Milton Miller wrote:
[snip]
> >> +#define MIN_VERSION 2
> >> +#define OUT_VERSION 16
> >
> > Should output version 17. In any case, don't try to be so general -
> > just convert v123 (all basically the same) to latest (i.e. v17)
> > without all the #if nonsense.
>
> Outputing v17 instead of 16 requires more words to be added to the
> header, and the library does fine with v16.
For now. libfdt will want v17. Although it will (eventually) have
it's own v16->v17 conversion code.
> Actually the v1 trees has
> some other differences such as initrd addresses were kernel linear not
> real, cpus were assigned logical numbers ... so while the structure
> didn't change except for the header field, the contents did.
!? what's your source for this. v2 and v3 were absolutely supposed to
be backwards compatible with v1 which would not be the case with
silent semantic changes such as this.
> Actually,
> when converting v3 to v16 some of the code issn't needed, the ifs allow
> the code size to be reduced.
Yes, but it never will be, because the only reason we'd include this
file is for converting old kexec-tools device trees which are v2 not
v3.
> >> +#define OUT_COMPAT 16
> >> +
> >> +#ifdef NO_CHECK
> >> +static int check_v123_tree(u32 *start, u32 *limit)
> >> +{
> >> + return 0;
> >> +}
> >> +#else
> >> +/**
> >> + * check_v123_tree - check integrety of a version 1, 2, or 3 tree
> >> + * @start: the start of the device tree struct
> >> + * @limit: the end of the region for the struct
> >> + * structural checks on device_tree
> >> + */
> >> +static int check_v123_tree(u32 *start, u32 *limit)
> >
> > What is the point of this check? If the device tree is corrupt, we're
> > stuffed anyway, so why bother?
>
> Hence the ifdef NO_CHECK. When developing, sometimes its nice to know
> if its your input or your program. These functions are destructive to
> an improperlly formed tree, and in non-obvious ways. When debugging,
> it's not hard to hardcode console write or read the printf string
> buffer with a hardware debugger to see error messages. That said, it
> could be removed.
Right. Debugging code shouldn't pollute final patches.
[snip]
> >> +#if MIN_VERSION < 3 || OUT_VERSION > 16
> >> +/**
> >> + * move_nops_fwd - move nops in a v16 dt_struct to the beginning
> >> + * @start - device tree starting address
> >> + * @count - number of %OF_DT_NOP cells to move
> >> + */
> >> +static void move_nops_fwd(u32 *start, int count)
> >
> > What on earth is the point of this. The NOPs are perfectly valid
> > scattered within the tree, why go to all this trouble to shuffle them
> > about.
>
> And if you notice, there is a "how many to move" argument. The point
> of moving them to the front of the tree is the v17 device tree header
> takes more space than the v3 one, and the v2 header is smaller than
> both v17 and v16 header. Since I am converting the tree in place, the
> space has to come from somewhere. Since we are pretty much guaranteed
> to get several nops, this function moves them forward so they can be
> overwritten. In practice we move 1-3 NOPS from the early properties >
> 8 bytes and early grandchild nodes (eg /cpus/PowerPC,xxx).
This is a hell of a lot of bother to go to for a few bytes.
Allocating a big-enough buffer for any reasonable devtree in BSS and
memmove()ing into there would be far simpler.
Remember this is a hack for horrid old device trees produced by
kexec-tools. It simply doesn't justify large amounts of code to work
around.
[snip]
> >> + if (tree->last_comp_version > 3)
> >> + return; /* don't know what to do */
> >
> > Rather, don't need to do anything.
>
> If the tree is >= 16 then we don't presently need to do anything. If
> there is a theoritical v4 tree we don't know what to do. And if output
> version is 17 but input is 16 we don't know what to do either, because
> there likely aren't nops in the tree to consume. I suppose we could
> preceed it with a check for version == OUTPUT_VERSION, but then I'm
> sure I'd get pointless differentation :-).
There will never be a v4. Or anything between v3 and v16.
Again this is far too general. It's a hack to deal with kexec-tools
old trees. Therefore it doesn't need to deal with any general old
tree, just the minimum to deal with trees as produced by old
kexec-tools.
[snip]
> >> + /*
> >> + * move mem_rsvmap and dt_strings if they are before dt_struct
> >> + * onto our nops . Adjust start addresses for the 3 sections.
> >> + */
> >
> > Hrm. Do we really need to worry about this case. You may be
> > producing v2 trees in kexec-tools, but do they actually have the
> > blocks out of order? dtc certainly never produced them that way.
>
> Out of order? There has never been a spec as to the order of the
> blocks, only the implicit assumption that they follow the device tree
> header in a reasonably packed sequence. booting-without-of says it
> must be in ram,; the offsets are unsigned 32 bit quantities.
> As to the order, used, the first implemntation was the kernel which
> writes memreserve, strings, then struct (both the openfirmware client
> in prom_init and the iSeries procedural library in dt.c). The second
> implentation written is a procedural based library (similar to iseries,
> never published, but still used internally) that starts with a
> pre-built header and string table, builds the dt_struct as the
> functions are called, and when finished copies the memreserve table and
> fills in the dt_size, total size, and memreserve offset.
Hrm. Yes, well, the iSeries tree is weird in a bunch of ways.
> fs2dt writes memreserve, struct, then strings. Aparently the same as
> dtc. But yes, the strings can be before the struct, and the mem
> reserve may or may not be when the strings are before the struct.
Again. We don't need to deal with the general case here - just the
real case of trees produced by old kexec-tools.
[snip]
--
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: [PATCH 1/15] boot: find initrd location from device-tree
From: David Gibson @ 2007-09-25 3:27 UTC (permalink / raw)
To: Milton Miller; +Cc: ppcdev, Paul Mackerras
In-Reply-To: <c76a222305d08e74b878c946d4181078@bga.com>
On Mon, Sep 24, 2007 at 03:02:36AM -0500, Milton Miller wrote:
>
> On Sep 23, 2007, at 9:58 PM, David Gibson wrote:
>
> > On Fri, Sep 21, 2007 at 06:03:24PM -0500, Milton Miller wrote:
> >> Some platforms have a boot agent that can create or modify properties
> >> in
> >> the device-tree and load images into memory. Provide a helper to set
> >> loader_info used by prep_initrd().
> >>
> >> Signed-off-by: Milton Miller <miltonm@bga.com>
> >> Acked-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > Hrm, despite my earlier ack, I'm going to whinge about a few nits
> > here.
> >
> >> ---
> >> re 12168
> >> rediffed types.h, offset in ops.h
> >>
> >> Index: kernel/arch/powerpc/boot/ops.h
> >> ===================================================================
> >> --- kernel.orig/arch/powerpc/boot/ops.h 2007-09-17 22:12:47.000000000
> >> -0500
> >> +++ kernel/arch/powerpc/boot/ops.h 2007-09-17 22:12:51.000000000 -0500
> >> @@ -163,6 +163,7 @@ void dt_fixup_clock(const char *path, u3
> >> void __dt_fixup_mac_addresses(u32 startindex, ...);
> >> #define dt_fixup_mac_addresses(...) \
> >> __dt_fixup_mac_addresses(0, __VA_ARGS__, NULL)
> >> +void dt_find_initrd(void);
> >>
> >>
> >> static inline void *find_node_by_linuxphandle(const u32 linuxphandle)
> >> Index: kernel/arch/powerpc/boot/types.h
> >> ===================================================================
> >> --- kernel.orig/arch/powerpc/boot/types.h 2007-09-17
> >> 22:12:47.000000000 -0500
> >> +++ kernel/arch/powerpc/boot/types.h 2007-09-17 22:12:51.000000000
> >> -0500
> >> @@ -12,6 +12,8 @@ typedef short s16;
> >> typedef int s32;
> >> typedef long long s64;
> >>
> >> +#define UINT_MAX 0xFFFFFFFF
> >
> > I actually don't like this constant - at the point you compare you
> > care, explicitly, about the value not being over 32-bits, rather than
> > whether it fits a uint, so the named constant is more misleading than
> > helpful.
>
> Arguable, I don't like counting F's or zeros in C code.
So check if (addr >> 32) is non-zero.
> It is used as the max address that the wrapper deals with, both here
> and memranges, which happens to be 32 bits.
Right and the reasons for that being the value it is are not because
it also hapeens to be the max uint *or* ulong.
> Actually it cares about overflowing the unsigned long in loader_info,
> not that the address fits in 32 bits.
>
> So it should be ULONG_MAX now (malloc and all the address code was
> changed to use unsigned long instead of unsigned int since the patch
> was written).
>
> And dt_xlate needs the same information. Its is using a hardcoded 64
> bit constant to provide the a simiar check.
>
>
> >> +
> >> #define min(x,y) ({ \
> >> typeof(x) _x = (x); \
> >> typeof(y) _y = (y); \
> >> Index: kernel/arch/powerpc/boot/devtree.c
> >> ===================================================================
> >> --- kernel.orig/arch/powerpc/boot/devtree.c 2007-09-17
> >> 22:12:47.000000000 -0500
> >> +++ kernel/arch/powerpc/boot/devtree.c 2007-09-17 22:12:51.000000000
> >> -0500
> >> @@ -1,6 +1,7 @@
> >> /*
> >> * devtree.c - convenience functions for device tree manipulation
> >> * Copyright 2007 David Gibson, IBM Corporation.
> >> + * Copyright 2007 Milton Miller, IBM Corporation.
> >> * Copyright (c) 2007 Freescale Semiconductor, Inc.
> >> *
> >> * Authors: David Gibson <david@gibson.dropbear.id.au>
> >> @@ -333,3 +334,68 @@ int dt_is_compatible(void *node, const c
> >>
> >> return 0;
> >> }
> >> +
> >> +/**
> >> + * dt_find_initrd - set loader initrd location based on existing
> >> properties
> >> + *
> >> + * finds the linux,initrd-start and linux,initrd-end properties in
> >> + * the /chosen node and sets the loader initrd fields accordingly.
> >> + *
> >> + * Use this if your loader sets the properties to allow other code to
> >> + * relocate the tree and/or cause r3 and r4 to be set on true OF
> >> + * platforms.
> >
> > I am unable to make sense of the paragraph above.
>
> The phrase "relocate the tree" should be "relocate the initrd", which
> the wrapper will do if it located below vmlinux.size. Also, r3 and r4
> need to be set when booting the kernel from a client interface with an
> initrd so it can take it into consideration when choosing the location
> to build the flat tree.
>
> How about:
>
> Filling in the loader info allows main.c to be aware of the initrd,
> meaning prep_initrd will move the initrd if it will be overwritten when
> the kernel is copied to its runtime location. In addition, if you are
> booting the kernel from a client interface instead of a flat device
> tree, this also causes r3 and r4 to be set so the kernel can avoid
> overwriting the initrd when creating the flat tree.
Clearer, but I still don't see that it says anything useful - "finds
the initrd from the devtree and does all the things that are supposed
to be done with an initrd" - which is implied in the previous
paragraph.
>
> >
> >> + */
> >> +void dt_find_initrd(void)
> >> +{
> >> + int rc;
> >> + unsigned long long initrd_start, initrd_end;
> >> + void *devp;
> >> + static const char start_prop[] = "linux,initrd-start";
> >> + static const char end_prop[] = "linux,initrd-end";
> >
> > I think these constants are more obscuring than useful.
>
> They are useful to the extent they reduce the number of source
> characters causing about 8 less line wraps. Since they are used
> multiple places, the compiler only needs to emit one copy of this byte
> sequence. Admitedly you made this point in a prior review.
The compiler is perfectly capable of folding identical string
constants.
> >> +
> >> + devp = finddevice("/chosen");
> >> + if (! devp) {
> >> + return;
> >> + }
> >
> > CodingStyle would not put { } here.
>
> Yea, nit. not sure why I have the braces there, I usually follow that
> one. And what's that space doing after !?
>
> >
> >> +
> >> + rc = getprop(devp, start_prop, &initrd_start, sizeof(initrd_start));
> >> + if (rc < 0)
> >> + return; /* not found */
> >> + /* The properties had to be 8 bytes until 2.6.22 */
> >> + if (rc == sizeof(unsigned long)) {
> >> + unsigned long tmp;
> >> + memcpy(&tmp, &initrd_start, rc);
> >> + initrd_start = tmp;
> >> + } else if (rc != sizeof(initrd_start)) { /* now they
> >> can be 4 */
> >
> > Right. 8 bytes and 4 bytes, so you should be using explicit length
> > types instead of long and long long.
>
> Ok, I guess we ahve u32 and u64 in types.h now. But there is other
> code in the wrapper that assumes its compiled 32 bit ILP.
Yes, but that's no reason to propagate the sin.
> >> + printf("unexpected length of %s in /chosen!\n\r", start_prop);
> >> + return;
> >
> > All these printf() / return stanzas add a lot of verbosity to this
> > function. Any way they can be consolidated a bit, maybe a single
> > error path that just prints the property values, so the user can
> > figure out what was wrong with them.
>
> On a prior review I got asked to split the reasons for ingoring the
> values below (the > 32 bit address from end < start cases). Admitedly
> without all the detailed errors the justification for the string
> varables is reduced.
Hrm. Well, I can't say I'm entirely convinced either way on this
one. I guess I'll think about it again once the rest is cleaned up.
--
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] PowerPC: add more than 4MB kernel image size support to bootwarapper
From: David Gibson @ 2007-09-25 2:29 UTC (permalink / raw)
To: Valentine Barshak; +Cc: linuxppc-dev
In-Reply-To: <20070924113627.GA30504@ru.mvista.com>
On Mon, Sep 24, 2007 at 03:36:27PM +0400, Valentine Barshak wrote:
> Currently zImage is linked at the 4MB base address.
> Some platforms (using cuboot, treeboot) need the zImage's
> entry point and base address. They place zImage exactly
> at the base address it's been linked to. Sometimes 4MB left
> at the start of the memory is simply not enough to unpack zImage.
> This could happen with initramfs enabled, since the kernel image
> packed along with initramfs.cpio could be over 5MB in size.
> This patch checks vmlinux image size and links zImage at
> the base address that is equal to the unpacked image size
> aligned to 4MB boundary. This way zImage base address is 4MB
> only if unpacked kernel image size is less then 4MB.
Good plan. However..
[snip]
> diff -ruN linux-2.6.orig/arch/powerpc/boot/zImage.lds.S linux-2.6/arch/powerpc/boot/zImage.lds.S
> --- linux-2.6.orig/arch/powerpc/boot/zImage.lds.S 2007-09-22 00:48:08.000000000 +0400
> +++ linux-2.6/arch/powerpc/boot/zImage.lds.S 2007-09-22 04:03:58.000000000 +0400
> @@ -3,7 +3,7 @@
> EXTERN(_zimage_start)
> SECTIONS
> {
> - . = (4*1024*1024);
> + . = ALIGN((_kend - _kstart), (4*1024*1024));
..I don't see any reason to keep the 4MB granularity. I would just
align the address to say a 64k boundary (64k because that's the
granularity used in the normal ABI).
> _start = .;
> .text :
> {
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>
--
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: Problems compiling Linux for MPC8272ADS
From: David Gibson @ 2007-09-25 2:16 UTC (permalink / raw)
To: Alan Bennett; +Cc: linuxppc-dev, Manil Gaouar
In-Reply-To: <bfa0697f0709240946i341739b3n8d5541b30bca8cb@mail.gmail.com>
On Mon, Sep 24, 2007 at 10:46:41AM -0600, Alan Bennett wrote:
> I added this to my boot/Makefile near the other mcpu=440 lines. I
> believe there may be a fix coming?
I believe there is.
> $(obj)/treeboot-walnut.o: BOOTCFLAGS += -mcpu=440
It should be -mcpu=405 for Walnut, though.
> $(obj)/4xx.o: BOOTCFLAGS += -mcpu=440
> $(obj)/ebony.o: BOOTCFLAGS += -mcpu=440
--
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: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC
From: David Gibson @ 2007-09-25 2:13 UTC (permalink / raw)
To: Peter Korsgaard; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <878x6wpq3t.fsf@macbook.be.48ers.dk>
On Mon, Sep 24, 2007 at 07:52:22AM +0200, Peter Korsgaard wrote:
> >>>>> "David" == David Gibson <david@gibson.dropbear.id.au> writes:
>
> Hi
>
> >> compatible = "fsl-i2c";
> >> reg = <3100 100>;
> >> interrupts = <f 8>;
> >> interrupt-parent = < &ipic >;
> >> dfsrr;
> >> +
> >> + rtc@68 {
> >> + device_type = "rtc";
> >> + compatible = "dallas,ds1339";
> >> + reg = <68>;
> >> + };
>
> David> I think we want to think a bit more carefully about how to do bindings
> David> for RTC devices. No "rtc" device_type is defined, but again we might
> David> want to.
>
> Could be. I've simply done it like kuroboxHD.dts already does and
> fsl_soc.c expects.
>
> David> I did find one real OF binding for a different Dallas RTC (and NVRAM),
> David> see:
>
> David> http://playground.sun.com/1275/proposals/Closed/Remanded/Accepted/346-it.txt
>
> David> It's a little different from the example above.
>
> David> The fact that NVRAM+RTC chips are so common is a bit of an issue from
> David> the point of view of defining a device class binding - a device can't
> David> have type "rtc" and "nvram".
>
> True. I think we should primarily focus on the RTC part rather than
> NVRAM as that's the "main" functionality and leave a NVRAM class for
> I2C EEPROMs.
>
> The Linux driver for the chip (rtc-1307.c) doesn't expose the NVRAM
> bytes either.
Incidentally how are you planning on instantiating the driver? AFAIK
all the rtc-* drivers are platform drivers rather than of_platform
drivers. I had been thinking of an rtc helper function that would go
through the tree instantiating platform devices for any RTCs based on
a compatible -> platform device name table.
--
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: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC
From: David Gibson @ 2007-09-25 2:11 UTC (permalink / raw)
To: Segher Boessenkool; +Cc: linuxppc-dev, Timur Tabi
In-Reply-To: <b312e03ef67dcd6cdc5f57e90da746ef@kernel.crashing.org>
On Mon, Sep 24, 2007 at 11:11:28PM +0200, Segher Boessenkool wrote:
> >> Scott> #size-cells is zero on i2c, so it should just be reg = <68>.
> >>
> >> Scott> You'll probably need to add #address-cells and #size-cells to
> >> the
> >> Scott> controller node, as well.
> >
> > Uh.. yes.. i2c interfaces should really always have #a and #s.
>
> More generally, every node that defines a bus needs it (unless the
> defaults of 2 resp. 1 are correct for this bus, but even then you
> might want it because it makes things more explicit).
Yes. Actually I think we should make explicit #a and #s at least
strongly recommended in the flat tree documentation.
> >> i2c@3100 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> device_type = "i2c";
> >
> > Hrm... we probably want an "i2c" device_type class, but I don't think
> > we've actually defined one, which is a problem
>
> By defining new device_type's, or new semantics for device_type,
> you open the door to (accidentally) becoming incompatible with
> "real" OF.
Hrm... perhaps. But is it a realistic danger - I'll have to think
more about that.
> And you don't need to: "real" OF has a mechanism for specifying
> the "generic device class" already, if you use the "generic names"
> recommended practice (and you do, for both this node and the rtc
> node): it's the generic name itself!
Hmm, I guess.
> >> + rtc@68 {
> >> + device_type = "rtc";
> >> + compatible = "dallas,ds1339";
> >> + reg = <68>;
> >> + };
> >
> > I think we want to think a bit more carefully about how to do bindings
> > for RTC devices. No "rtc" device_type is defined, but again we might
> > want to.
>
> Actually, "device_type" = "rtc" _is_ defined (in the "device support
> extensions" recommended practice), and there is no useful way a flat
> device tree can implement it (it merely defines get-time and set-time
> methods).
Ah.. right. That changes things a bit, in that we might want to
include device_type purely for similarity with real OF tree.
Real OF has a device_type == "nvram" too, doesn't it? AFAICT the real
OF systems I have (which I think all have ISA-like CMOS RTC/NVRAM
chips) the RTC is labelled as "nvram" rather than "rtc".
> > The fact that NVRAM+RTC chips are so common is a bit of an issue from
> > the point of view of defining a device class binding - a device can't
> > have type "rtc" and "nvram".
>
> You should match OS drivers on "compatible" only anyway.
Absolutely. I was only thinking of defining "device classes" where
for some reason it is useful to examine them without needing to pick a
particular driver.
> Those cases where OS drivers don't nicely 1-1 match device nodes are a
> bit of a headache; for RTC/NVRAM devices, these problems are nicely
> side-stepped by handling this from platform code.
Not necessarily. The new RTC class drivers are just drivers like
anything other and are not especially instantiated from the platform
code.
And drat. I was only really mentioning stuff about device_type in
passing, but it's the only thing anyone's responded to. I was also
mostly suggesting changing the format of compatible, for greater
similarity with the existing ds1385 binding.
--
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: AMCC yosemite 440ep PCI slot doesn't work.
From: Andrew Liu @ 2007-09-25 2:05 UTC (permalink / raw)
To: Valentine Barshak; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <46F79D6C.5070500@ru.mvista.com>
Currently, ppc44x use ppc branch, not powerpc, especially for AMCC
yosemite 440ep.
Through adding printk in function ppc4xx_pic_init which is in
arch/ppc/syslib/ppc4xx_pic.c, whether setting IRQ_LEVEL has no effect
on it.
BRs,
Andrew
Valentine Barshak wrote:
> Stefan Roese wrote:
>> On Monday 24 September 2007, Andrew Liu wrote:
>>> By default, it is IRQ_SENSE_LEVEL | IRQ_POLARITY_NEGATIVE
>>
>> And that should be correct for PCI interrupts.
>>
>>> I change it to IRQ_SENSE_EDGE | IRQ_POLARITY_NEGATIVE or
>>> IRQ_SENSE_EDGE | IRQ_POLARITY_POSITIVE.
>>> It displays:
>>> eth2: link up, 100Mbps, full-duplex, lpa 0x45E1
>>> eth2: no IPv6 routers present
>>> NETDEV WATCHDOG: eth2: transmit timed out
>>> eth2: Transmit timeout, status 0c 0005 c07f media 10.
>>> eth2: Tx queue start entry 4 dirty entry 0.
>>> eth2: Tx descriptor 0 is 0008a05a. (queue head)
>>> eth2: Tx descriptor 1 is 0008a04e.
>>> eth2: Tx descriptor 2 is 0008a046.
>>> eth2: Tx descriptor 3 is 0008a05a.
>>> eth2: link up, 100Mbps, full-duplex, lpa 0x45E1
>>>
>>> and
>>> root@localhost:/root> ifconfig eth2
>>> eth2 Link encap:Ethernet HWaddr 00:0E:2E:7E:F5:E6
>>> inet addr:128.224.149.13 Bcast:128.224.255.255
>>> Mask:255.255.0.0
>>> inet6 addr: fe80::20e:2eff:fe7e:f5e6/64 Scope:Link
>>> UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
>>> RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>>> TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>>> collisions:0 txqueuelen:1000
>>> RX bytes:0 (0.0 b) TX bytes:0 (0.0 b)
>>> Interrupt:25 Base address:0x6f00
>>>
>>>
>>> and
>>> root@localhost:/root> cat /proc/interrupts
>>> CPU0
>>> 2: 0 UIC0 Edge IBM IIC
>>> 7: 0 UIC0 Edge IBM IIC
>>> 10: 11961 UIC0 Edge MAL TX EOB
>>> 11: 18471 UIC0 Edge MAL RX EOB
>>> 25: 0 UIC0 Edge eth2
>>> 32: 0 UIC1 Edge MAL SERR
>>> 33: 0 UIC1 Edge MAL TX DE
>>> 34: 0 UIC1 Edge MAL RX DE
>>> 40: 32 UIC1 Edge ohci_hcd:usb1
>>> 60: 0 UIC1 Edge EMAC
>>> BAD: 0
>>>
>>>
>>> Can PCI slot use level trigger?
>>>
>>> give me some advice.
>
> This could be caused by misseting the IRQ_lEVEL flag in desc->status.
> There have been a commit 4dc7b4b0405fd7320940849b6e31ea8ea68fd0df that
> fixes the problem.
> Thanks,
> Valentine.
>
>>
>> Hmmm. Did you try an earlier Linux version too? For example 2.6.22 or
>> even earlier? Please give it a try and let us know if the same
>> problem occurs.
>>
>> Best regards,
>> Stefan
>>
>> =====================================================================
>> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
>> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
>> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
>> =====================================================================
>
>
^ permalink raw reply
* Re: AMCC yosemite 440ep PCI slot doesn't work.
From: Andrew Liu @ 2007-09-25 2:02 UTC (permalink / raw)
To: Stefan Roese; +Cc: linuxppc-dev
In-Reply-To: <200709241314.18123.sr@denx.de>
I have tested it on linux-2.6.19 which is from AMCC website , linux
2.6.21.7 which is from linux mainline and linux-2.6.23.rc7 which is
from git://www.denx.de/git/linux-2.6-denx.git
Have the same problem.
I don't understand, why after enabling this PCI interrupts. in so
short time, produce 100,000 interrupt request (25: 100000 UIC0
Level eth2).
BRs,
Andrew.
Stefan Roese wrote:
> On Monday 24 September 2007, Andrew Liu wrote:
>
>> By default, it is IRQ_SENSE_LEVEL | IRQ_POLARITY_NEGATIVE
>>
>
> And that should be correct for PCI interrupts.
>
>
>> I change it to IRQ_SENSE_EDGE | IRQ_POLARITY_NEGATIVE or
>> IRQ_SENSE_EDGE | IRQ_POLARITY_POSITIVE.
>> It displays:
>> eth2: link up, 100Mbps, full-duplex, lpa 0x45E1
>> eth2: no IPv6 routers present
>> NETDEV WATCHDOG: eth2: transmit timed out
>> eth2: Transmit timeout, status 0c 0005 c07f media 10.
>> eth2: Tx queue start entry 4 dirty entry 0.
>> eth2: Tx descriptor 0 is 0008a05a. (queue head)
>> eth2: Tx descriptor 1 is 0008a04e.
>> eth2: Tx descriptor 2 is 0008a046.
>> eth2: Tx descriptor 3 is 0008a05a.
>> eth2: link up, 100Mbps, full-duplex, lpa 0x45E1
>>
>> and
>> root@localhost:/root> ifconfig eth2
>> eth2 Link encap:Ethernet HWaddr 00:0E:2E:7E:F5:E6
>> inet addr:128.224.149.13 Bcast:128.224.255.255 Mask:255.255.0.0
>> inet6 addr: fe80::20e:2eff:fe7e:f5e6/64 Scope:Link
>> UP BROADCAST RUNNING MULTICAST MTU:1500 Metric:1
>> RX packets:0 errors:0 dropped:0 overruns:0 frame:0
>> TX packets:0 errors:0 dropped:0 overruns:0 carrier:0
>> collisions:0 txqueuelen:1000
>> RX bytes:0 (0.0 b) TX bytes:0 (0.0 b)
>> Interrupt:25 Base address:0x6f00
>>
>>
>> and
>> root@localhost:/root> cat /proc/interrupts
>> CPU0
>> 2: 0 UIC0 Edge IBM IIC
>> 7: 0 UIC0 Edge IBM IIC
>> 10: 11961 UIC0 Edge MAL TX EOB
>> 11: 18471 UIC0 Edge MAL RX EOB
>> 25: 0 UIC0 Edge eth2
>> 32: 0 UIC1 Edge MAL SERR
>> 33: 0 UIC1 Edge MAL TX DE
>> 34: 0 UIC1 Edge MAL RX DE
>> 40: 32 UIC1 Edge ohci_hcd:usb1
>> 60: 0 UIC1 Edge EMAC
>> BAD: 0
>>
>>
>> Can PCI slot use level trigger?
>>
>> give me some advice.
>>
>
> Hmmm. Did you try an earlier Linux version too? For example 2.6.22 or even
> earlier? Please give it a try and let us know if the same problem occurs.
>
> Best regards,
> Stefan
>
> =====================================================================
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: office@denx.de
> =====================================================================
>
>
^ permalink raw reply
* Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC
From: David Gibson @ 2007-09-25 2:04 UTC (permalink / raw)
To: Scott Wood; +Cc: Timur Tabi, linuxppc-dev
In-Reply-To: <46F7CF2F.9070805@freescale.com>
On Mon, Sep 24, 2007 at 09:52:31AM -0500, Scott Wood wrote:
> David Gibson wrote:
> >> i2c@3100 {
> >> + #address-cells = <1>;
> >> + #size-cells = <0>;
> >> device_type = "i2c";
> >
> > Hrm... we probably want an "i2c" device_type class, but I don't think
> > we've actually defined one, which is a problem
>
> Right... but we need to get the kernel to stop expecting the device type
> to be there before we yell at people for including it. :-)
Obviously. We should make sure all the corresponding compatibles are
specific enough, change the drivers, then think about getting rid of
it.
> > The fact that NVRAM+RTC chips are so common is a bit of an issue from
> > the point of view of defining a device class binding - a device can't
> > have type "rtc" and "nvram".
>
> This is one of the reasons that I'd prefer to use compatible for such
> things.
Yeah, fair enough.
--
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: [PATCH v2] bootwrapper: adds cuboot for MPC7448HPC2 platform
From: David Gibson @ 2007-09-25 1:57 UTC (permalink / raw)
To: Zang Roy-r61911; +Cc: linuxppc-dev list, Paul Mackerras
In-Reply-To: <1190629915.20267.8.camel@localhost.localdomain>
On Mon, Sep 24, 2007 at 06:31:55PM +0800, Zang Roy-r61911 wrote:
> From: Roy Zang <tie-fei.zang@freescale.com>
>
> This patch adds cuboot support for MPC7448HPC2 platform.
> The cuImage can be used with legacy u-boot without FDT support.
>
> Signed-off-by: Roy Zang <tie-fei.zang@freescale.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
--
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: [PATCH 2/2] bootwrapper: Add PlanetCore firmware support.
From: David Gibson @ 2007-09-25 1:52 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070924200949.GC21347@loki.buserror.net>
On Mon, Sep 24, 2007 at 03:09:49PM -0500, Scott Wood wrote:
> This is a library that board code can use to extract information from the
> PlanetCore configuration keys. PlanetCore is used on various boards from
> Embedded Planet.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
--
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: [PATCH] Make instruction dumping work in real mode.
From: Olof Johansson @ 2007-09-25 1:41 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev, paulus
In-Reply-To: <20070924190100.GB4761@loki.buserror.net>
On Mon, Sep 24, 2007 at 02:01:00PM -0500, Scott Wood wrote:
> On non-book-E, if a faulting PC is in the first few pages, and it's not an
> ITLB miss, it's likely executing in real mode, probably at an exception vector.
>
> Rather than print a useless XXXXXXXX, it is assumed that this is the case, and
> the address is treated as physical. This helps when debugging corruption at
> the beginning of memory.
Please use regs->msr & MSR_IR instead.
-Olof
^ permalink raw reply
* Re: [PATCH 15/40] bootwrapper: Factor out dt_set_mac_address().
From: David Gibson @ 2007-09-25 1:20 UTC (permalink / raw)
To: Scott Wood; +Cc: linuxppc-dev
In-Reply-To: <20070924200911.GB21347@loki.buserror.net>
On Mon, Sep 24, 2007 at 03:09:11PM -0500, Scott Wood wrote:
> This allows callers to set addresses one at a time when that would be more
> convenient.
>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
Sorry I've been such a pain about this previously.
Acked-by: David Gibson <david@gibson.dropbear.id.au>
--
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: [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
From: Roland McGrath @ 2007-09-25 0:59 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
In-Reply-To: <1190680406.12382.9.camel@localhost.localdomain>
> Yup, I think I ditched most of them.. for some reason I decided it
> couldn't happen, but maybe I'm wrong ?
Well, it's a BUG_ON. It's supposed to be for something that "can't happen".
That's why it's a sanity check, not a wild assertion. ;-)
The 2/2 patch is an example of a bug that CHECK_FULL_REGS catches.
In the status quo, using PTRACE_PEEKUSR in a bug case crashes while using
PTRACE_GETREGS in the same place might get bogus data. (In the actual bug
thus found, the data is not bogus and only the bit that FULL_REGS checks is.)
Thanks,
Roland
^ permalink raw reply
* Re: [PATCH 2/2] powerpc: FULL_REGS on exec
From: Benjamin Herrenschmidt @ 2007-09-25 0:34 UTC (permalink / raw)
To: Roland McGrath
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
In-Reply-To: <20070924235244.2F3BB4D04B7@magilla.localdomain>
On Mon, 2007-09-24 at 16:52 -0700, Roland McGrath wrote:
> When PTRACE_O_TRACEEXEC is used, a ptrace call to fetch the registers at
> the PTRACE_EVENT_EXEC stop (PTRACE_PEEKUSR) will oops in CHECK_FULL_REGS.
> With recent versions, "gdb --args /bin/sh -c 'exec /bin/true'" and "run" at
> the (gdb) prompt is sufficient to produce this. I also have written an
> isolated test case, see https://bugzilla.redhat.com/show_bug.cgi?id=301791#c15.
>
> This change fixes the problem by clearing the low bit of pt_regs.trap in
> start_thread so that FULL_REGS is true again. This is correct since all of
> the GPRs that "full" refers to are cleared in start_thread.
Looks good, nice catch.
> Signed-off-by: Roland McGrath <roland@redhat.com>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> arch/powerpc/kernel/process.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index e477c9d..fd799d2 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -605,6 +605,13 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
> regs->ccr = 0;
> regs->gpr[1] = sp;
>
> + /*
> + * We have just cleared all the nonvolatile GPRs, so make
> + * FULL_REGS(regs) return true. This is necessary to allow
> + * ptrace to examine the thread immediately after exec.
> + */
> + regs->trap &= ~1UL;
> +
> #ifdef CONFIG_PPC32
> regs->mq = 0;
> regs->nip = start;
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
From: Benjamin Herrenschmidt @ 2007-09-25 0:33 UTC (permalink / raw)
To: Roland McGrath
Cc: David Woodhouse, linux-kernel, linuxppc-dev, Paul Mackerras,
Andrew Morton, Linus Torvalds
In-Reply-To: <20070924235052.5AFC24D04B7@magilla.localdomain>
On Mon, 2007-09-24 at 16:50 -0700, Roland McGrath wrote:
> This restores the CHECK_FULL_REGS sanity check to every place that can
> access the nonvolatile GPRs for ptrace. This is already done for
> native-bitwidth PTRACE_PEEKUSR, but was omitted for many other cases
> (32-bit ptrace, PTRACE_GETREGS, etc.); I think there may have been more
> uniform checks before that were lost in the recent cleanup of GETREGS et al.
Yup, I think I ditched most of them.. for some reason I decided it
couldn't happen, but maybe I'm wrong ?
Cheers,
Ben.
> Signed-off-by: Roland McGrath <roland@redhat.com>
> ---
> arch/powerpc/kernel/ptrace.c | 4 ++++
> arch/powerpc/kernel/ptrace32.c | 8 ++++++++
> 2 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 8a177bd..40d34b3 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -331,6 +331,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned long __user *tmp = (unsigned long __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = put_user(*reg, tmp);
> if (ret)
> @@ -346,6 +347,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned long __user *tmp = (unsigned long __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = get_user(*reg, tmp);
> if (ret)
> @@ -517,6 +519,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret |= __put_user(ptrace_get_reg(child, ui),
> @@ -537,6 +540,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret = __get_user(tmp, (unsigned long __user *) data);
> diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
> index 9e6baea..6b86960 100644
> --- a/arch/powerpc/kernel/ptrace32.c
> +++ b/arch/powerpc/kernel/ptrace32.c
> @@ -53,6 +53,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned int __user *tmp = (unsigned int __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = put_user(*reg, tmp);
> if (ret)
> @@ -68,6 +69,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
> unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
> unsigned int __user *tmp = (unsigned int __user *)addr;
>
> + CHECK_FULL_REGS(child->thread.regs);
> for (i = 0; i < 32; i++) {
> ret = get_user(*reg, tmp);
> if (ret)
> @@ -164,6 +166,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> if ((addr & 3) || (index > PT_FPSCR32))
> break;
>
> + CHECK_FULL_REGS(child->thread.regs);
> if (index < PT_FPR0) {
> tmp = ptrace_get_reg(child, index);
> } else {
> @@ -210,6 +213,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> if ((addr & 3) || numReg > PT_FPSCR)
> break;
>
> + CHECK_FULL_REGS(child->thread.regs);
> if (numReg >= PT_FPR0) {
> flush_fp_to_thread(child);
> tmp = ((unsigned long int *)child->thread.fpr)[numReg - PT_FPR0];
> @@ -270,6 +274,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> if ((addr & 3) || (index > PT_FPSCR32))
> break;
>
> + CHECK_FULL_REGS(child->thread.regs);
> if (index < PT_FPR0) {
> ret = ptrace_put_reg(child, index, data);
> } else {
> @@ -307,6 +312,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> */
> if ((addr & 3) || (numReg > PT_FPSCR))
> break;
> + CHECK_FULL_REGS(child->thread.regs);
> if (numReg < PT_FPR0) {
> unsigned long freg = ptrace_get_reg(child, numReg);
> if (index % 2)
> @@ -342,6 +348,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret |= __put_user(ptrace_get_reg(child, ui),
> @@ -359,6 +366,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
> ret = -EIO;
> break;
> }
> + CHECK_FULL_REGS(child->thread.regs);
> ret = 0;
> for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
> ret = __get_user(tmp, (unsigned int __user *) data);
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
^ permalink raw reply
* [PATCH 2/2] powerpc: FULL_REGS on exec
From: Roland McGrath @ 2007-09-24 23:52 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev
Cc: David Woodhouse, Andrew Morton, Linus Torvalds, linux-kernel
When PTRACE_O_TRACEEXEC is used, a ptrace call to fetch the registers at
the PTRACE_EVENT_EXEC stop (PTRACE_PEEKUSR) will oops in CHECK_FULL_REGS.
With recent versions, "gdb --args /bin/sh -c 'exec /bin/true'" and "run" at
the (gdb) prompt is sufficient to produce this. I also have written an
isolated test case, see https://bugzilla.redhat.com/show_bug.cgi?id=301791#c15.
This change fixes the problem by clearing the low bit of pt_regs.trap in
start_thread so that FULL_REGS is true again. This is correct since all of
the GPRs that "full" refers to are cleared in start_thread.
Signed-off-by: Roland McGrath <roland@redhat.com>
---
arch/powerpc/kernel/process.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index e477c9d..fd799d2 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -605,6 +605,13 @@ void start_thread(struct pt_regs *regs, unsigned long start, unsigned long sp)
regs->ccr = 0;
regs->gpr[1] = sp;
+ /*
+ * We have just cleared all the nonvolatile GPRs, so make
+ * FULL_REGS(regs) return true. This is necessary to allow
+ * ptrace to examine the thread immediately after exec.
+ */
+ regs->trap &= ~1UL;
+
#ifdef CONFIG_PPC32
regs->mq = 0;
regs->nip = start;
^ permalink raw reply related
* [PATCH 1/2] powerpc: ptrace CHECK_FULL_REGS
From: Roland McGrath @ 2007-09-24 23:50 UTC (permalink / raw)
To: Paul Mackerras, linuxppc-dev
Cc: David Woodhouse, Andrew Morton, Linus Torvalds, linux-kernel
This restores the CHECK_FULL_REGS sanity check to every place that can
access the nonvolatile GPRs for ptrace. This is already done for
native-bitwidth PTRACE_PEEKUSR, but was omitted for many other cases
(32-bit ptrace, PTRACE_GETREGS, etc.); I think there may have been more
uniform checks before that were lost in the recent cleanup of GETREGS et al.
Signed-off-by: Roland McGrath <roland@redhat.com>
---
arch/powerpc/kernel/ptrace.c | 4 ++++
arch/powerpc/kernel/ptrace32.c | 8 ++++++++
2 files changed, 12 insertions(+), 0 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 8a177bd..40d34b3 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -331,6 +331,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned long __user *tmp = (unsigned long __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = put_user(*reg, tmp);
if (ret)
@@ -346,6 +347,7 @@ static long arch_ptrace_old(struct task_struct *child, long request, long addr,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned long __user *tmp = (unsigned long __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = get_user(*reg, tmp);
if (ret)
@@ -517,6 +519,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret |= __put_user(ptrace_get_reg(child, ui),
@@ -537,6 +540,7 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret = __get_user(tmp, (unsigned long __user *) data);
diff --git a/arch/powerpc/kernel/ptrace32.c b/arch/powerpc/kernel/ptrace32.c
index 9e6baea..6b86960 100644
--- a/arch/powerpc/kernel/ptrace32.c
+++ b/arch/powerpc/kernel/ptrace32.c
@@ -53,6 +53,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned int __user *tmp = (unsigned int __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = put_user(*reg, tmp);
if (ret)
@@ -68,6 +69,7 @@ static long compat_ptrace_old(struct task_struct *child, long request,
unsigned long *reg = &((unsigned long *)child->thread.regs)[0];
unsigned int __user *tmp = (unsigned int __user *)addr;
+ CHECK_FULL_REGS(child->thread.regs);
for (i = 0; i < 32; i++) {
ret = get_user(*reg, tmp);
if (ret)
@@ -164,6 +166,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
if ((addr & 3) || (index > PT_FPSCR32))
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (index < PT_FPR0) {
tmp = ptrace_get_reg(child, index);
} else {
@@ -210,6 +213,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
if ((addr & 3) || numReg > PT_FPSCR)
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (numReg >= PT_FPR0) {
flush_fp_to_thread(child);
tmp = ((unsigned long int *)child->thread.fpr)[numReg - PT_FPR0];
@@ -270,6 +274,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
if ((addr & 3) || (index > PT_FPSCR32))
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (index < PT_FPR0) {
ret = ptrace_put_reg(child, index, data);
} else {
@@ -307,6 +312,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
*/
if ((addr & 3) || (numReg > PT_FPSCR))
break;
+ CHECK_FULL_REGS(child->thread.regs);
if (numReg < PT_FPR0) {
unsigned long freg = ptrace_get_reg(child, numReg);
if (index % 2)
@@ -342,6 +348,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret |= __put_user(ptrace_get_reg(child, ui),
@@ -359,6 +366,7 @@ long compat_sys_ptrace(int request, int pid, unsigned long addr,
ret = -EIO;
break;
}
+ CHECK_FULL_REGS(child->thread.regs);
ret = 0;
for (ui = 0; ui < PT_REGS_COUNT; ui ++) {
ret = __get_user(tmp, (unsigned int __user *) data);
^ permalink raw reply related
* Re: EBC peripheral, access causing Machine Check
From: Josh Boyer @ 2007-09-24 23:42 UTC (permalink / raw)
To: Dave Cogley; +Cc: linuxppc-embedded
In-Reply-To: <005301c7fedf$c46f7770$9601a8c0@DCOGLEYNEW>
On Mon, 24 Sep 2007 12:19:01 -0700
"Dave Cogley" <dcogley@uslinc.com> wrote:
> Good Afternoon,
>
>
>
> I am using an AMCC 440EPx processor with an FPGA peripheral attached to the
> EBC. I am doing the entire bank configuration in the u-boot boot loader
> before Linux is loaded. I can directly read and write registers using
> memory read and write (md, nm) within area 0xC0000000 to 0xC000FFFF direct
> memory I/O within the context of u-boot without any faults. When I attempt
> to access any register in area 0xC000000 within the context of my Linux
> device driver I get a Machine Check fault (attached text) which appears to
> be an access permission violation. I am currently running the DENX ELDK
> 2.6.19.2 platform that was specific to the "Sequoia" platform.
>
>
>
> I have configured the EBC access permissions and control resisters as
> follows for bank 1:
>
>
>
> PB1CR: 0xC003C000
>
> PB1AP: 0x80040380
You need proper TLB entries set up before you can access things. Linux
discards the MMU settings that U-Boot does. So your driver needs to call
ioremap on the physical address.
Other than that, we can't help you much without your driver code.
josh
^ permalink raw reply
* Re: [PATCH 2/3] usb: ehci-ppc-of dts bindings.
From: Segher Boessenkool @ 2007-09-24 21:40 UTC (permalink / raw)
To: David Gibson; +Cc: linuxppc-dev, linux-usb-devel
In-Reply-To: <20070920005201.GE14404@localhost.localdomain>
>> Nothing in a flat device tree should ever define a device_type, except
>> perhaps for compatibility with legacy kernel code.
>
> This is not necessarily true. As Segher says, device_type originally
> indicated the OF programming model for a device. However, we've
> extended the notion for the flat device tree to allow device_type to
> cover "device classes" which could have certain common properties and
> semantics.
Such device classes are already handled by the "generic names"
recommended practice.
Reusing device_type for a different purpose is dangerous: before you
know it, you'll end up with a conflict, as was nicely demonstrated
today with device_type "rtc".
> However, in this case, a meaningful class binding must
> already be defined: it might make sense for usb to have a defined
> device_type, but it's not been defined so far, so for now you must
> omit device_type (if a device_type is defined in future, it's easier
> to add legacy hooks that will include devices which are missing the
> relevant device_type marker than to work around devices which *do*
> have the marker, but pre-date and don't follow the defined class
> binding).
Nothing in the kernel should assume it can find all devices of a
certain "class" any other way than by exhaustively matching on
possible "compatible" values. I'm not sure why you would want to,
anyway: you need "compatible" to select which driver to use, already.
Segher
^ permalink raw reply
* EBC peripheral, access causing Machine Check
From: Dave Cogley @ 2007-09-24 19:19 UTC (permalink / raw)
To: linuxppc-embedded
[-- Attachment #1.1: Type: text/plain, Size: 916 bytes --]
Good Afternoon,
I am using an AMCC 440EPx processor with an FPGA peripheral attached to the
EBC. I am doing the entire bank configuration in the u-boot boot loader
before Linux is loaded. I can directly read and write registers using
memory read and write (md, nm) within area 0xC0000000 to 0xC000FFFF direct
memory I/O within the context of u-boot without any faults. When I attempt
to access any register in area 0xC000000 within the context of my Linux
device driver I get a Machine Check fault (attached text) which appears to
be an access permission violation. I am currently running the DENX ELDK
2.6.19.2 platform that was specific to the "Sequoia" platform.
I have configured the EBC access permissions and control resisters as
follows for bank 1:
PB1CR: 0xC003C000
PB1AP: 0x80040380
Dave Cogley
Software Engineer
Ultra Stereo Labs, Inc.
(805) 549-0161
mailto:dcogley@uslinc.com
[-- Attachment #1.2: Type: text/html, Size: 3940 bytes --]
[-- Attachment #2: mcdump.txt --]
[-- Type: text/plain, Size: 1767 bytes --]
bash-3.00#
Machine check in kernel mode.
Data Read PLB Error
OPB to PLB3: BSTAT= 0x00000000
PLB3 to PLB4: BEAR=0xffffffffffffffff BESR0=0x00000000 BESR1=0x00000000
PLB4 to PLB3: BEAR=0xfffffffffffffffd BESR0=0x00000000 BESR1=0x00000000
PLB3 to OPB: BEAR=0xffffffff BESR0=0x00000000 BESR1=0x00000000
PLB3 arbiter: BEAR=0xfffffffe ACR=0x00000000 BESR=0x00000000
PLB4 to OPB1: BEAR=0x0000000dbfffffbf BESR0=0x00000000 BESR1=0x00000000
PLB40 Arbiter: BEAR=0x00000000c0000000 ACR=0xda000000 BESR0=0x0f000000
PLB41 Arbiter: BEAR=0xfffffffffffffffc ACR=0xdb000000 BESR0=0x00000000
POB0: BEAR=0xc27e3194ffffffff BESR0=0x00000000 BESR1=0x00000000
OPB0: BEAR=0x0000000000000000 BSTAT=0x00000000
Oops: machine check, sig: 7 [#2]
NIP: D107E2D4 LR: C0069F24 CTR: 0FF40B20
REGS: c02a9f50 TRAP: 0202 Not tainted (2.6.19.2)
MSR: 00029000 <EE,ME> CR: 40000022 XER: 00000000
TASK = cf6c1030[268] 'banjocore' THREAD: cf1b8000
GPR00: 40044B01 CF1B9EB0 CF6C1030 00000000 CF0EF5C0 40044B01 7F95B928 0FF39D08
GPR08: 00000000 00000000 FFFFFFE7 D1080000 80000028 10018B10 00000000 100D8ED8
GPR16: 100C0000 00000000 100C0000 10090000 00000000 100DF660 100C7C48 00000000
GPR24: 100D9038 00000000 100D8F58 40044B01 FFFFFFF7 CF0EF5C0 0FFBB73C 7F95B928
NIP [D107E2D4] banjo_ioctl+0xc0/0x14c [banjodecoder]
LR [C0069F24] do_ioctl+0x80/0x84
Call Trace:
[CF1B9EB0] [00000001] 0x1 (unreliable)
[CF1B9ED0] [C0069F24] do_ioctl+0x80/0x84
[CF1B9EE0] [C0069FB4] vfs_ioctl+0x8c/0x418
[CF1B9F10] [C006A380] sys_ioctl+0x40/0x74
[CF1B9F40] [C0001C84] ret_from_syscall+0x0/0x3c
Instruction dump:
60004b00 7f850000 419e0044 3c004004 60004b01 7f850000 38600000 409eff74
3d60d108 812bf2a4 7c0004ac 7d204c2c <0c090000> 4c00012c 38600000 91260000
Bus error
bash-3.00#
^ permalink raw reply
* Re: [patch 3/3] mpc8349emitx.dts: Add ds1339 RTC
From: Segher Boessenkool @ 2007-09-24 21:11 UTC (permalink / raw)
To: David Gibson; +Cc: Timur Tabi, linuxppc-dev
In-Reply-To: <20070924050709.GM8058@localhost.localdomain>
>> Scott> #size-cells is zero on i2c, so it should just be reg = <68>.
>>
>> Scott> You'll probably need to add #address-cells and #size-cells to
>> the
>> Scott> controller node, as well.
>
> Uh.. yes.. i2c interfaces should really always have #a and #s.
More generally, every node that defines a bus needs it (unless the
defaults of 2 resp. 1 are correct for this bus, but even then you
might want it because it makes things more explicit).
>> i2c@3100 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> device_type = "i2c";
>
> Hrm... we probably want an "i2c" device_type class, but I don't think
> we've actually defined one, which is a problem
By defining new device_type's, or new semantics for device_type,
you open the door to (accidentally) becoming incompatible with
"real" OF.
And you don't need to: "real" OF has a mechanism for specifying
the "generic device class" already, if you use the "generic names"
recommended practice (and you do, for both this node and the rtc
node): it's the generic name itself!
>> + rtc@68 {
>> + device_type = "rtc";
>> + compatible = "dallas,ds1339";
>> + reg = <68>;
>> + };
>
> I think we want to think a bit more carefully about how to do bindings
> for RTC devices. No "rtc" device_type is defined, but again we might
> want to.
Actually, "device_type" = "rtc" _is_ defined (in the "device support
extensions" recommended practice), and there is no useful way a flat
device tree can implement it (it merely defines get-time and set-time
methods).
> The fact that NVRAM+RTC chips are so common is a bit of an issue from
> the point of view of defining a device class binding - a device can't
> have type "rtc" and "nvram".
You should match OS drivers on "compatible" only anyway.
Those cases where OS drivers don't nicely 1-1 match device nodes are a
bit of a headache; for RTC/NVRAM devices, these problems are nicely
side-stepped by handling this from platform code.
Segher
^ permalink raw reply
* Re: Sequoia kernel crash workaround.
From: Josh Boyer @ 2007-09-24 21:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: linuxppc-dev, Olof Johansson, Josh Boyer, Stefan Roese,
David Gibson
In-Reply-To: <1190667350.6886.59.camel@localhost.localdomain>
On Tue, 25 Sep 2007 06:55:50 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Mon, 2007-09-24 at 14:35 +0400, Valentine Barshak wrote:
> >
> > From what I've seen, setup_cpu is never called for BOOKE.
> > Currently It's called from cputable.c for PPC64 and from head_32.S for
> > 6xx.
> > Thanks,
>
> Right. We need to fix/converge some of these but it's not totally
> trivial.
Valentine has patches for adding this for BookE on the list.
josh
^ 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