LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH 1/5] generic __remove_pages() support
From: Badari Pulavarty @ 2008-03-21 16:55 UTC (permalink / raw)
  To: Yasunori Goto; +Cc: linuxppc-dev, Andrew Morton, paulus, lkml
In-Reply-To: <20080321235322.2B17.E1E9C6FF@jp.fujitsu.com>

On Sat, 2008-03-22 at 00:25 +0900, Yasunori Goto wrote:
> Badari-san.
> 
> > Index: linux-2.6.25-rc2/mm/memory_hotplug.c
> > ===================================================================
> > --- linux-2.6.25-rc2.orig/mm/memory_hotplug.c	2008-02-27 12:58:17.000000000 -0800
> > +++ linux-2.6.25-rc2/mm/memory_hotplug.c	2008-02-27 16:06:50.000000000 -0800
> > @@ -102,6 +102,21 @@ static int __add_section(struct zone *zo
> >  	return register_new_memory(__pfn_to_section(phys_start_pfn));
> >  }
> >  
> > +static int __remove_section(struct zone *zone, struct mem_section *ms)
> > +{
> > +	int ret = -EINVAL;
> > +
> > +	if (!valid_section(ms))
> > +		return ret;
> > +
> > +	ret = unregister_memory_section(ms);
> > +	if (ret)
> > +		return ret;
> > +
> > +	sparse_remove_one_section(zone, ms);
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Reasonably generic function for adding memory.  It is
> >   * expected that archs that support memory hotplug will
> > @@ -135,6 +150,35 @@ int __add_pages(struct zone *zone, unsig
> >  }
> >  EXPORT_SYMBOL_GPL(__add_pages);
> >  
> > +int __remove_pages(struct zone *zone, unsigned long phys_start_pfn,
> > +		 unsigned long nr_pages)
> > +{
> > +	unsigned long i, ret = 0;
> > +	int sections_to_remove;
> > +	unsigned long flags;
> > +	struct pglist_data *pgdat = zone->zone_pgdat;
> > +
> > +	/*
> > +	 * We can only remove entire sections
> > +	 */
> > +	BUG_ON(phys_start_pfn & ~PAGE_SECTION_MASK);
> > +	BUG_ON(nr_pages % PAGES_PER_SECTION);
> > +
> > +	release_mem_region(phys_start_pfn << PAGE_SHIFT, nr_pages * PAGE_SIZE);
> > +
> > +	sections_to_remove = nr_pages / PAGES_PER_SECTION;
> > +	for (i = 0; i < sections_to_remove; i++) {
> > +		unsigned long pfn = phys_start_pfn + i*PAGES_PER_SECTION;
> > +		pgdat_resize_lock(pgdat, &flags);
> > +		ret = __remove_section(zone, __pfn_to_section(pfn));
> > +		pgdat_resize_unlock(pgdat, &flags);
> > +		if (ret)
> > +			break;
> > +	}
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL_GPL(__remove_pages);
> 
> Here may be a bug.
> __remove_section() is called with pgdat_resize_lock() which is
> spin_lock_irqsave().
> 
>   __remove_section()
>       |
>       +--> unregister_memory_section()
>               |
>               +--> remove_memory_block()
>                       |
>                       +--> unregister_memory()
>                              |
>                              +--> sysdev_unregister()
> 
> sysdev_unregister() calls mutex_lock().
> It might sleep with irq disable, right?
> I found BUG()'s messages by this.

Hmm. I think you are right. Let me take a look
and see if we can move sysdev_unregister() outside
the lock.

Thanks for finding it.

Thanks,
Badari

^ permalink raw reply

* RE: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.
From: Stephen Neuendorffer @ 2008-03-21 16:08 UTC (permalink / raw)
  To: Segher Boessenkool, Paul Mackerras; +Cc: John Linn, linuxppc-dev
In-Reply-To: <82ce3dbb9ebdb1fa4fc7407ed3396411@kernel.crashing.org>


I thought about this, and reconsidered it again after I finally figured
out how to get the Power.org website to relenquish the ePAPR spec, which
(FWIW) has reg-shift as an optional property of the ns16550 binding.

However, I'm not enamoured of it, since I doubt it's really good to be
doing ioremaps and memory management on unaligned blocks.  It seems more
logical to me to have the reg property define a range of aligned
addresses, and in this case, it happens that the driver never touches
some of those bytes.

In any event, the real point of the patch was to spark some discussion,
which we failed to get otherwise.. :)  I think any of these proposals
are workable...

Steve

> -----Original Message-----
> From: =
linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org] On Behalf Of
Segher Boessenkool
> Sent: Friday, March 21, 2008 4:40 AM
> To: Paul Mackerras
> Cc: linuxppc-dev@ozlabs.org; John Linn
> Subject: Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for
Xilinx uart16550.
>=20
> >> Personally, I'm not fond of this approach.  There is already some
> >> traction to using the reg-shift property to specify spacing, and I
> >> think it would be appropriate to also define a reg-offset property
to
> >> handle the +3 offset and then let the xilinx 16550 nodes use those.
> >
> > Why do we need a reg-offset property when we can just add the offset
> > to the appropriate word(s) in the reg property?
>=20
> Because if you do that, the "reg" property cannot describe the full
> register block (it misses the first few bytes).  Not a huge problem
> in practice, sure.
>=20
>=20
> Segher
>=20
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

^ permalink raw reply

* Re: [PATCH 6/8] ptrace: arch_ptrace -ENOSYS return
From: Ingo Molnar @ 2008-03-21 15:07 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: linux-arch, tony.luck, linux-ia64, linux-kernel, David Miller,
	linuxppc-dev, Paul Mackerras, sparclinux, Andrew Morton,
	Thomas Gleixner, Roland McGrath, Richard Henderson
In-Reply-To: <alpine.LFD.1.00.0803210744470.3020@woody.linux-foundation.org>


* Linus Torvalds <torvalds@linux-foundation.org> wrote:

> > The reason I took the approach I did instead is incrementalism. I 
> > can't change that signature without breaking about 22 arch builds.
> 
> Don't worry about the arch builds. If that's your main worry and 
> reason for this, it's not worth it. Yes, ptrace changes, and yes, we 
> may have arch issues, but no, it's not that big of a deal. Just break 
> them.
> 
> Make sure x86[-64] works, and make sure that other architectures *can* 
> work (and explain it on linux-arch) when you have to fix something up, 
> but ptrace is a blip on the radar for people, it's not going to be a 
> huge issue.

for a long time all the nice but intrusive utrace improvements from 
Roland had this big adoption barrier. So Roland went around that and 
with a lot of effort he made it optional, incremental and per arch, so 
that we could try it on x86 and merge it upstream.

Now that we see how cleaner it is and that it actually was an almost 
regression-free endevour on x86 (we had 2 regressions so far, both fixed 
- which is an amazing feat for such wide changes IMO), i very much agree 
that we should just do the "rest" of this in one big step.

it's a bit of a chicken & egg problem for such changes. If it breaks 
architectures it gets dropped out of -mm - even though 90% of our 
developers, 95% of our testers and 99% of our active users are using x86 
only.

but in general, prototyping something on a single architecture in the 
first step is OK - and maybe even the first merge is OK. But once it has 
been proven on the most tested Linux platform that we have (and there's 
no blatant x86-ism in it), there's no reason not to mandate it.

	Ingo

^ permalink raw reply

* Re: [PATCH 1/2 v2] Driver for Freescale 8610 and 5121 DIU
From: Timur Tabi @ 2008-03-21 16:12 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-fbdev-devel, Peter Zijlstra, linux-kernel, linuxppc-dev,
	York Sun
In-Reply-To: <20080320152708.23c6c734.akpm@linux-foundation.org>

Andrew Morton wrote:

>> +static struct diu_hw dr = {
>> +	.mode = MFB_MODE1,
>> +	.reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
>> +};
> 
> I'm not clear on what's supposed to happen with __SPIN_LOCK_UNLOCKED().  I
> do know that its documentation is crap.

Yes, "__SPIN_LOCK_UNLOCKED(old_style_spin_init)" is wrong.  We'll fix it.

>  static struct diu_hw dr = {
>  	.mode = MFB_MODE1,
> -	.reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> +	.reg_lock = __SPIN_LOCK_UNLOCKED(diu_hw.reg_lock),
>  };

Yes, this is better.  Did you already make this change when you applied it to
your -mm repo?

> GFP_DMA implies GFP_ATOMIC, but it's appropriate for documentation purposes.

So does that mean that "GFP_DMA | GFP_KERNEL" is always wrong?  If so, this
combination is used a lot in the kernel today.

>> +	if (virt) {
>> +		*phys = virt_to_phys(virt);
>> +		pr_debug("virt %p, phys=%llx\n", virt, (uint64_t) *phys);
>> +		memset(virt, 0, size);
> 
> Could have used __GFP_ZERO, I guess.

I had completely forgotten about __GFP_ZERO.  Thanks.

>> +	virt = (void *) rh_alloc(&diu_ops.diu_rh_info, size, "DIU");
> 
> hm, I'd have expected checkpatch to whine about the space after the cast
> there.  Whatever.

I thought a space after a cast is the right thing to do?

> please take a look, and please use checkpatch on all future patches.

Sorry, we forgot to run it again after our second version of the patch.

>> +static void free_irq_local(int irq)
>> +{
>> +	struct diu *hw = dr.diu_reg;
>> +
>> +	/* Disable all LCDC interrupt */
>> +	out_be32(&(hw->int_mask), 0x1f);
>> +
>> +	free_irq(irq, 0);
>> +}
> 
> and the free_irq() will go splat?

Sorry, but I don't understand what's wrong with this code.

We'll make the other changes you've suggested and repost.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply

* RE: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.
From: Stephen Neuendorffer @ 2008-03-21 16:14 UTC (permalink / raw)
  To: Grant Likely, John Linn; +Cc: linuxppc-dev
In-Reply-To: <fa686aa40803201719i7bf02ba2qd259f1a36a24d943@mail.gmail.com>



> -----Original Message-----
> From: =
linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org] On Behalf Of =
Grant
Likely
> Sent: Thursday, March 20, 2008 5:19 PM
> To: John Linn
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for
Xilinx uart16550.
>=20
> On Thu, Mar 20, 2008 at 8:43 AM, John Linn <john.linn@xilinx.com>
wrote:
> > The Xilinx 16550 uart core is not a standard 16550, because it uses
> >  word-based addressing rather than byte-based addressing.  As a
result,
> >  it is not compatible with the open firmware 'ns16550' compatible
> >  binding.  This code introduces new bindings, which pass the correct
> >  register base and regshift properties to the uart driver to enable
> >  this core to be used.  Doing this cleanly required some refactoring
of
> >  the existing code.
>=20
> Personally, I'm not fond of this approach.  There is already some
> traction to using the reg-shift property to specify spacing, and I
> think it would be appropriate to also define a reg-offset property to
> handle the +3 offset and then let the xilinx 16550 nodes use those.

Since the ePAPR ns16550 defines reg-shift, I don't think it makes sense
to have a separate binding for sparse16550, or for the xilinx16550
types.  Personally, I like having reg-offset better than
adding three to the reg space, but I leave that up to someone who is
much more involved in ePAPR.

> >  +static struct of_serial_info __devinitdata ns8250_info =3D { .type =
=3D
PORT_8250 };
> >  +static struct of_serial_info __devinitdata ns16450_info =3D { =
.type
=3D PORT_16450 };
> >  +static struct of_serial_info __devinitdata ns16550_info =3D { =
.type
=3D PORT_16550 };
> >  +static struct of_serial_info __devinitdata ns16750_info =3D { =
.type
=3D PORT_16750 };
> >  +static struct of_serial_info __devinitdata xilinx_16550_info =3D {
> >  +       .type =3D PORT_16550,
> >  +       .regshift =3D 2,
> >  +       .regoffset =3D 3,
> >  +};
> >  +static struct of_serial_info __devinitdata unknown_info =3D { =
.type
=3D PORT_UNKNOWN };
>=20
> In support of my argument; the fact that you need a table of data says
> to me that this data should really be encoded in the device tree.  :-)

The table of data was always there to bind the UART types, I just added
some more info to it.

Steve

^ permalink raw reply

* Re: muram in device tree for mpc8250 in arch/powerpc
From: Scott Wood @ 2008-03-21 16:18 UTC (permalink / raw)
  To: James Black; +Cc: linuxppc-embedded
In-Reply-To: <b77025b40803201602q68fce3e1j703502a270351179@mail.gmail.com>

On Thu, Mar 20, 2008 at 05:02:35PM -0600, James Black wrote:
> On Thu, Mar 20, 2008 at 12:20 PM, Scott Wood <scottwood@freescale.com> wrote:
> > James Black wrote:
> >  > Zone PFN ranges:
> >  >   DMA             0 ->    16384
> >  >   Normal      16384 ->    16384
> >  > Movable zone start PFN for each node
> >  > early_node_map[1] active PFN ranges
> >  >     0:        0 ->    16384
> >  > Unable to handle kernel paging request for data at address 0xe001a000
> >  > Faulting instruction address: 0xc00e1a6c
> >
> >  What function is 0xc00e1a6c in?
> Drilled down into the source and find that mm/bootmem.c:line 232
> 
>            : find_next_zero_bit()
> may be the culprit. When I put printks in, the line moves around a bit.

Is it node_bootmem_map, eidx, or i that is bad?  What is end_pfn?  What is
calling alloc_bootmem?

> >  You can't use all of 0x8000-0x8fff; there is device parameter RAM in
> >  there.  If you can figure out the portions that aren't in use, you can
> >  use those, but I wouldn't bother unless you really need the extra muram.
> 
> So, are you are saying that the muram node is giving the ram to the
> kernel and is no longer available for devices to use as buffer
> descriptors?

No.

> I imagined the opposite. I thought I was telling the
> kernel where the parameter ram and buffer descriptors live.

It's telling the kernel where available muram is, to be allocated by CPM
drivers for buffer descriptors.

> I am going to turn on the MCC2 device eventually and will need all of the
> dpram for buffer descriptors.
> 
> The memory map in my mind right now is
> 0x0000 - 0x3FFF --> buffer descriptors for SCCx and MCC2.
> 0x8000 - 0x8FFF --> paramter ram for SCCx and MCC2, et al. (reserved)
> 0xB000 - 0xB8FF --> FCCx specific (reserved)
> 
> In my case, I shouldn't define any of the dual port ram for the kernel?

You shouldn't define any of the reserved areas as general purpose muram.

-Scott

^ permalink raw reply

* Re: [PATCH v2] Make 83xx perfmon support selectable
From: Scott Wood @ 2008-03-21 16:24 UTC (permalink / raw)
  To: Jerry Van Baren; +Cc: linuxppc-dev list, Phillips Kim, Paul Mackerras
In-Reply-To: <47E2FA6D.9020200@gmail.com>

On Thu, Mar 20, 2008 at 07:59:41PM -0400, Jerry Van Baren wrote:
> Per Andy (and my limited reading of the UMs), some 83xx have the PMR  
> registers and some don't.  The compiler either supports the PMR register  
> or it doesn't.  If you make it runtime configurable, people running CPUs  
> that don't support the specific PMR will have to change compiler  
> configurations in order to compile with the PMR register in there (could  
> have unintended consequences).

I'm not saying make it runtime-only -- you can still have a config option to
determine whether to build a kernel that supports it.  I'm saying there
should be an additional runtime check so that if you run a multiplatform
kernel with perfmon enabled on a chip that doesn't support it, you won't
take a program check.

> Also, if you look at the code (arch/powerpc/kernel/pmc.c), there are  
> several different types of PMR registers, based on the core and flavor  
> of the core.  Finding or making a compiler setup that supports all of  
> the possible PMR registers could be a problem.

It only needs to support all possible registers within a class of hardware
over which we support multiplatform kernels.

> You would still have to make the PMR read/write runtime selectable  
> because the CPUs that don't support that register will raise an  
> exception IIRC (an Really Bad Thing[tm]).

Yes, that was my point.  The changelog on the patch seemed to indicate that
the compile-time option was intended to address this, not just the toolchain
problem.

-Scott

^ permalink raw reply

* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Segher Boessenkool @ 2008-03-21 16:45 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linuxppc-dev, John Linn
In-Reply-To: <47E3DA40.6000007@ru.mvista.com>

>    The proposed use clearly would treat them as generic, since in the 
> context of the Xilinx UART they're just not needed -- it's known 
> beforehand and most probably fixed how/where the registers are mapped. 
> There's just no need for such info in the device tree -- unless you're 
> going to teach the *generic* driver to handle this specific (and 
> possibly others alike) kind of a device.

I was under the impression that the "xilinx uart" was just a 16550 (or 
so)
with its registers wired up in a slightly unusual way.  If it's a 
completely
different device, of course you need a separate binding, and you might 
not
want reg-shift properties etc. there.

>> "reg-*" has nothing to do with Linux device driver implementation
>> issues: it describes how a device is physically wired up!
>
>    Hm... wasn't that you who were telling that use of "range" 
> properties guarantees 1:1 correspondence of the upstream/downstream 
> bus addresses (in their LSB part of course -- meaning that the device 
> registers 0..x are seen by the CPU at addresses base+0..base+X?

I have no idea what "ranges" has to do with this.  This device is not
a memory-mapped bus, it's a UART.

>>>> In support of my argument; the fact that you need a table of data 
>>>> says
>>>> to me that this data should really be encoded in the device tree.  
>>>> :-)
>
>>>    Not at all.
>
>> Not _necessarily_.  I agree with Grant here: for many of these devices
>> with byte-size registers, it is very common to find them with their
>> register banks wired up differently, and that is often the *only*
>> difference to the "normal" device.  In this situation, it makes a lot
>> of sense to describe that difference with "reg-*" properties.
>
>    Note that "compicated" mapping is not (necessarily) a property of 
> the device itself but generally a property of the chip select circuit, 
> i.e. external entity.

There is no difference insofar as the device tree is concerned.


Segher

^ permalink raw reply

* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.
From: Segher Boessenkool @ 2008-03-21 16:48 UTC (permalink / raw)
  To: Stephen Neuendorffer; +Cc: John Linn, linuxppc-dev, Paul Mackerras
In-Reply-To: <20080321160802.7CED7196805C@mail103-sin.bigfish.com>

> I thought about this, and reconsidered it again after I finally figured
> out how to get the Power.org website to relenquish the ePAPR spec, 
> which
> (FWIW) has reg-shift as an optional property of the ns16550 binding.
>
> However, I'm not enamoured of it, since I doubt it's really good to be
> doing ioremaps and memory management on unaligned blocks.  It seems 
> more
> logical to me to have the reg property define a range of aligned
> addresses, and in this case, it happens that the driver never touches
> some of those bytes.

Yes, and that is (part of) why "reg-offset" might be needed /
might be a good idea.

> In any event, the real point of the patch was to spark some discussion,
> which we failed to get otherwise.. :)

You succeeded :-)


Segher

^ permalink raw reply

* RE: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart16550.
From: Stephen Neuendorffer @ 2008-03-21 16:50 UTC (permalink / raw)
  To: Segher Boessenkool, Sergei Shtylyov; +Cc: John Linn, linuxppc-dev
In-Reply-To: <0a442bfb97492bd8d687678480c6217a@kernel.crashing.org>



> -----Original Message-----
> From: =
linuxppc-dev-bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org
[mailto:linuxppc-dev-
> bounces+stephen.neuendorffer=3Dxilinx.com@ozlabs.org] On Behalf Of
Segher Boessenkool
> Sent: Friday, March 21, 2008 9:46 AM
> To: Sergei Shtylyov
> Cc: linuxppc-dev@ozlabs.org; John Linn
> Subject: Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for
Xilinx uart16550.
>=20
> >    The proposed use clearly would treat them as generic, since in
the
> > context of the Xilinx UART they're just not needed -- it's known
> > beforehand and most probably fixed how/where the registers are
mapped.
> > There's just no need for such info in the device tree -- unless
you're
> > going to teach the *generic* driver to handle this specific (and
> > possibly others alike) kind of a device.
>=20
> I was under the impression that the "xilinx uart" was just a 16550 (or
> so)
> with its registers wired up in a slightly unusual way.  If it's a
> completely
> different device, of course you need a separate binding, and you might
> not
> want reg-shift properties etc. there.

It's a standard 16550, with registers in the standard order, but at
offsets 3,7,11, etc.

Steve

^ permalink raw reply

* Re: [PATCH 2/3] [POWERPC] Xilinx: of_serial support for Xilinx uart 16550.
From: Sergei Shtylyov @ 2008-03-21 17:01 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, John Linn
In-Reply-To: <0a442bfb97492bd8d687678480c6217a@kernel.crashing.org>

Segher Boessenkool wrote:

>>    The proposed use clearly would treat them as generic, since in the 
>> context of the Xilinx UART they're just not needed -- it's known 
>> beforehand and most probably fixed how/where the registers are mapped. 
>> There's just no need for such info in the device tree -- unless you're 
>> going to teach the *generic* driver to handle this specific (and 
>> possibly others alike) kind of a device.

> I was under the impression that the "xilinx uart" was just a 16550 (or so)
> with its registers wired up in a slightly unusual way.  If it's a 
> completely
> different device, of course you need a separate binding, and you might not
> want reg-shift properties etc. there.

    It seems to be 16550 in the core, although coming in many flavors. :-)

>>> "reg-*" has nothing to do with Linux device driver implementation
>>> issues: it describes how a device is physically wired up!

>>    Hm... wasn't that you who were telling that use of "range" 
>> properties guarantees 1:1 correspondence of the upstream/downstream 
>> bus addresses (in their LSB part of course -- meaning that the device 
>> registers 0..x are seen by the CPU at addresses base+0..base+X?

> I have no idea what "ranges" has to do with this.

    Me neither. :-)

> This device is not a memory-mapped bus, it's a UART.

    IIRC, we were discussing MTD then, with an imaginary (for PPC) case of it 
being reverse-endian (actually happened on other archs)... IMHO, how the child 
bus addresses correspond to parent's one 1:1, doesn't imply that you can't 
have a device wired to the child bus a strange way, e.g. with byte-enables 
"reversed"...

>>>>> In support of my argument; the fact that you need a table of data says
>>>>> to me that this data should really be encoded in the device tree.  :-)

>>>>    Not at all.

>>> Not _necessarily_.  I agree with Grant here: for many of these devices
>>> with byte-size registers, it is very common to find them with their
>>> register banks wired up differently, and that is often the *only*
>>> difference to the "normal" device.  In this situation, it makes a lot
>>> of sense to describe that difference with "reg-*" properties.

>>    Note that "compicated" mapping is not (necessarily) a property of 
>> the device itself but generally a property of the chip select circuit, 
>> i.e. external entity.

> There is no difference insofar as the device tree is concerned.

    OK...

> Segher

WBR, Sergei

^ permalink raw reply

* Re: [PATCH] [POWERPC] Add AMCC Glacier 460GT eval board dts
From: Segher Boessenkool @ 2008-03-21 17:28 UTC (permalink / raw)
  To: Josh Boyer; +Cc: linuxppc-dev, Stefan Roese
In-Reply-To: <20080321072419.2372774b@hansolo.jdub.homelinux.org>

>>> As for the DTS, maybe a "compatible" property in the CPU might make 
>>> some
>>> sense with a content along the lines of "ppc440x6" or whatever rev of
>>> the 440 core it is.
>>>
>>> What do you think ?
>>
>> Good idea. I'll try to come up with a list for all existing 4xx SoC's 
>> and it's
>> core versions.
>
> I don't really care either way, but what does that buy us?  Merely
> documentation?

Obviously, as long as the kernel doesn't use the device tree for probing
the CPUs available, additions to the device tree here don't affect the
kernel at all ;-P


Segher

^ permalink raw reply

* Re: [PATCH 1/2 v2] Driver for Freescale 8610 and 5121 DIU
From: Andrew Morton @ 2008-03-21 18:12 UTC (permalink / raw)
  To: Timur Tabi
  Cc: linux-fbdev-devel, Peter Zijlstra, linux-kernel, linuxppc-dev,
	York Sun
In-Reply-To: <47E3DE6E.2050801@freescale.com>

On Fri, 21 Mar 2008 11:12:30 -0500 Timur Tabi <timur@freescale.com> wrote:

> Andrew Morton wrote:
> 
> >> +static struct diu_hw dr = {
> >> +	.mode = MFB_MODE1,
> >> +	.reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> >> +};
> > 
> > I'm not clear on what's supposed to happen with __SPIN_LOCK_UNLOCKED().  I
> > do know that its documentation is crap.
> 
> Yes, "__SPIN_LOCK_UNLOCKED(old_style_spin_init)" is wrong.  We'll fix it.
> 
> >  static struct diu_hw dr = {
> >  	.mode = MFB_MODE1,
> > -	.reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
> > +	.reg_lock = __SPIN_LOCK_UNLOCKED(diu_hw.reg_lock),
> >  };
> 
> Yes, this is better.  Did you already make this change when you applied it to
> your -mm repo?

I did:

--- a/drivers/video/fsl-diu-fb.c~fbdev-driver-for-freescale-8610-and-5121-diu-fix
+++ a/drivers/video/fsl-diu-fb.c
@@ -274,7 +274,7 @@ static struct mfb_info mfb_template[] = 
 
 static struct diu_hw dr = {
 	.mode = MFB_MODE1,
-	.reg_lock = __SPIN_LOCK_UNLOCKED(old_style_spin_init),
+	.reg_lock = __SPIN_LOCK_UNLOCKED(dr.reg_lock),
 };
 
 static struct diu_pool pool;

> > GFP_DMA implies GFP_ATOMIC, but it's appropriate for documentation purposes.
> 
> So does that mean that "GFP_DMA | GFP_KERNEL" is always wrong?

No, that's OK too.  It's just that GFP_DMA|GFP_ATOMIC is a bit redundant
and misleading.  GFP_DMA is already atomic; the only effect of adding
GFP_ATOMIC to GFP_DMA is to add __GFP_HIGH.

Don't wory about it ;)

>  If so, this
> combination is used a lot in the kernel today.
> 
> >> +	if (virt) {
> >> +		*phys = virt_to_phys(virt);
> >> +		pr_debug("virt %p, phys=%llx\n", virt, (uint64_t) *phys);
> >> +		memset(virt, 0, size);
> > 
> > Could have used __GFP_ZERO, I guess.
> 
> I had completely forgotten about __GFP_ZERO.  Thanks.
> 
> >> +	virt = (void *) rh_alloc(&diu_ops.diu_rh_info, size, "DIU");
> > 
> > hm, I'd have expected checkpatch to whine about the space after the cast
> > there.  Whatever.
> 
> I thought a space after a cast is the right thing to do?

Last time I grepped, no-space is a lot more common.

> > please take a look, and please use checkpatch on all future patches.
> 
> Sorry, we forgot to run it again after our second version of the patch.
> 
> >> +static void free_irq_local(int irq)
> >> +{
> >> +	struct diu *hw = dr.diu_reg;
> >> +
> >> +	/* Disable all LCDC interrupt */
> >> +	out_be32(&(hw->int_mask), 0x1f);
> >> +
> >> +	free_irq(irq, 0);
> >> +}
> > 
> > and the free_irq() will go splat?
> 
> Sorry, but I don't understand what's wrong with this code.

You snipped a bit.  Earlier, request_irq() failures were ignored.  So I
think there's a code path where free_irq_local() can free an IRQ which this
driver never owned.

> We'll make the other changes you've suggested and repost.

^ permalink raw reply

* Re: crash in init_ipic_sysfs on efika
From: Grant Likely @ 2008-03-21 18:51 UTC (permalink / raw)
  To: Matt Sealey; +Cc: linuxppc-dev, Olaf Hering, Paul Mackerras
In-Reply-To: <47E3B4AF.3030505@genesi-usa.com>

On Fri, Mar 21, 2008 at 7:14 AM, Matt Sealey <matt@genesi-usa.com> wrote:
> Is the MPC5200B PSC-AC97 driver in there?

Audio drivers need to go in via one of the ALSA developers and I
haven't been paying close enough attention to know if it has not in
yet.

g.


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

^ permalink raw reply

* Re: Oops with TQM5200 on TQM5200
From: Grant Likely @ 2008-03-21 19:02 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: linuxppc-dev, Paul Mackerras, Anatolij Gustschin
In-Reply-To: <47E3561B.6040505@grandegger.com>

On Fri, Mar 21, 2008 at 12:30 AM, Wolfgang Grandegger <wg@grandegger.com> wrote:
> Bartlomiej Sieka wrote:
>  > Grant Likely wrote:
>  >> Paul, my git server is down at the moment.  Can you please pick this
>  >> one up for .25?  It is needed to boot the tqm5200 board.
>  >
>  > Could we also have similar fixes for cm5200.dts and motionpro.dts picked
>  > up? I could provide the patches within a day -- will this be OK?
>
>  At that occasion, could you please also add two CAN nodes to tqm5200.dts
>  as shown:

Acked-by: Grant Likely <grant.likely@secretlab.ca>

>
>  s$ diff -u tqm5200.dts tqm5200.dts.CAN
>  --- tqm5200.dts 2008-03-20 18:44:40.000000000 +0100
>  +++ tqm5200.dts.CAN     2008-03-21 07:27:49.000000000 +0100
>  @@ -68,6 +68,22 @@
>                         fsl,has-wdt;
>                 };
>
>  +               can@900 {
>  +                       compatible = "fsl,mpc5200-mscan";
>  +                       cell-index = <0>;
>  +                       interrupts = <2 11 0>;
>  +                       interrupt-parent = <&mpc5200_pic>;
>  +                       reg = <900 80>;
>  +               };
>  +
>  +               can@980 {
>  +                       compatible = "fsl,mpc5200-mscan";
>  +                       cell-index = <1>;
>  +                       interrupts = <2 12 0>;
>  +                       interrupt-parent = <&mpc5200_pic>;
>  +                       reg = <980 80>;
>  +               };
>  +
>                 gpio@b00 {
>                         compatible = "fsl,mpc5200-gpio";
>                         reg = <b00 40>;
>
-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

^ permalink raw reply

* dcr stuff.
From: Stephen Neuendorffer @ 2008-03-21 19:05 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: git-dev


Is anybody working on the dcr infrastructure?

In particular, there seems to be inconsistency between what the kernel
does and what ePAPR thinks it should.  The code in the kernel selects
mmio or native access based on a Kconfig parameter and uses some device
tree parameters to correctly configure the mmio access, whereas ePAPR
doesn't seem to document all the device tree attributes (in particular,
dcr-mmio-stride and dcr-mmio-range) and provides a dynamic binding for
dcr-access-method...

Is there a plan here?  (In particular, in FPGA designs, its possible to
have crazy things like multiple independent dcr busses, some using
native access, some using mmio.  Or even if there is one bus, many of
the existing non-linux drivers we have assume native or mmio access.
I'd like to clean this up, obviously.. :)

Steve

^ permalink raw reply

* Re: muram in device tree for mpc8250 in arch/powerpc
From: James Black @ 2008-03-21 20:14 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-embedded
In-Reply-To: <20080321161833.GA4170@loki.buserror.net>

PiAgSXMgaXQgbm9kZV9ib290bWVtX21hcCwgZWlkeCwgb3IgaSB0aGF0IGlzIGJhZD8gIFdoYXQg
aXMgZW5kX3Bmbj8gIFdoYXQgaXMKPiAgY2FsbGluZyBhbGxvY19ib290bWVtPwoKVGhlIGNhbGwg
c3RhcnRzIGluIGluaXQvbWFpbi5jIHdpdGggcGFnaW5nX2luaXQoKSBhbmQgZW5kcyB1cCBpbgpt
bS9ib290bWVtLmM6YWxsb2NfYm9vdG1lbV9ub2RlKCkuCkkgaGF2ZSBiZWVuIG1vdmluZyB0aGUg
cHJpbnRrIGFyb3VuZCB0cnlpbmcgdG8gZmluZCBleGFjdGx5IHdoZXJlIHRoaXMKdGhpbmcgZmFp
bHMuIEl0IG5vdyBzZWVtcyB0aGF0IHRoZSBwcm9ibGVtIGlzIGFsc28gbW92aW5nLiBUaGUgbGF0
ZXN0Cm91dHB1dCBsb29rcyBsaWtlIGl0IGdvdCB0aHJvdWdoIHRoZSBwYWdlIGFsbG9jYXRpb24u
IEkgZ3JlcGVkIHRoZSBlbGYKZmlsZSBhbmQgdGhlIGZhdWx0aW5nIGFkZHJlc3MgaW5zdHJ1Y3Rp
b24gIDB4YzAwMTEyMTggaXMgaW4gc3RyY21wLgpbamJsYWNrQGpibGFjayBsaW51eC0yLjYuMjRd
JCBubSB2bWxpbnV4IHxncmVwIGMwMDExMjEKYzAwMTEyMTAgVCBzdHJjbXAKCkkndmUgYm9vdGVk
IHNldmVyYWwgdGltZXMgbm93IGFuZCB0aGUgcHJvYmxlbSBtb3ZlcyB3aXRoIHRoZSBwcmludGsu
CkJ1dCwgd2hlbiBJIGRpc2FibGUgZWFybHkgZGVidWdnaW5nIHRoZSBwcm9ibGVtIGNvbWVzIGJh
Y2sgYW5kIHRoZQpib290IGhhbmdzIG9yIGNyYXNoZXMgYXMgdXN1YWwuIEhlcmUgYXJlIGEgY291
cGxlIG9mIG91dHB1dHMgdGhhdCBzaG93Cml0IGlzIGdldHRpbmcgdGhyb3VnaCB0aGUgcGZuIGFs
bG9jYXRpb25zLiBTb21ldGhpbmcgaXNuJ3QgZGVjbGFyZWQKcmlnaHQgb3IgY29uZmlndXJlZCBy
aWdodC4gQnV0IHdoZXJlIHRvIGxvb2s/CgpJJ3ZlIGNvbXBpbGVkIHRoZSBrZXJuZWwgd2l0aCBn
Y2MgNC4wLjAgYW5kICA0LjIuMiBib3RoIHZlcnNpb25zIGhhdmUKdGhlIHNhbWUgcHJvYmxlbS4g
U2hvdWxkIEkgdHJ5IHRoZSAyLjYuMjUga2VybmVsIHNpbmNlIHRoZSAyLjYuMTkKd29ya3MganVz
dCBmaW5lLiBJJ3ZlIHN0cmlwcGVkIHRoZSBrZXJuZWwgb3B0aW9ucyBkb3duIGFsbW9zdCBhcyBm
YXIKYXMgSSBjYW4gZ28uCgpzZXR1cF9hcmNoOiBleGl0ClpvbmUgUEZOIHJhbmdlczoKICBETUEg
ICAgICAxMzQyMTc3MjggLT4gICAgMTYzODQKICBOb3JtYWwgICAgICAxNjM4NCAtPiAgICAxNjM4
NApNb3ZhYmxlIHpvbmUgc3RhcnQgUEZOIGZvciBlYWNoIG5vZGUKZWFybHlfbm9kZV9tYXBbMV0g
YWN0aXZlIFBGTiByYW5nZXMKICAyMDk3MTUyOiAxMzQyMTc3MjggLT4gICAgMTYzODQKQ291bGQg
bm90IGZpbmQgc3RhcnRfcGZuIGZvciBub2RlIDAKZnJlZV9hcmVhX2luaXRfbm9kZXMoKTogcGdk
YXQ6IGMwMWNhZGMwCm5vZGVfYm9vdG1lbV9tYXAoYWRkcik6IGMzZmZlMDAwCmVpZHgoc2l6ZSk6
IDE2Mzg0Cmkob2Zmc2V0KTogMAplbmRfcGZuOiAxNjM4NApnb2FsOiAxMDczNzQxODIzCnByZWZl
cnJlZDogMApub2RlX2Jvb3RtZW1fbWFwKGFkZHIpOiBjM2ZmZTAwMAplaWR4KHNpemUpOiAxNjM4
NAppKG9mZnNldCk6IDAKZW5kX3BmbjogMTYzODQKZ29hbDogMTA3Mzc0MTgyMwpwcmVmZXJyZWQ6
IDAKQnVpbHQgMSB6b25lbGlzdHMgaW4gWm9uZSBvcmRlciwgbW9iaWxpdHkgZ3JvdXBpbmcgb2Zm
LiAgVG90YWwgcGFnZXM6CjAKS2VybmVsIGNvbW1hbmQgbGluZTogL2Rldi9yb290PS9kZXYvcmFt
IHI3IHJhbWRpc2tfc2l6ZT00MDk2ClVuYWJsZSB0byBoYW5kbGUga2VybmVsIHBhZ2luZyByZXF1
ZXN0IGZvciBkYXRhIGF0IGFkZHJlc3MgMHgwMDAwMDAwMApGYXVsdGluZyBpbnN0cnVjdGlvbiBh
ZGRyZXNzOiAweGMwMDExMjE4Ck9vcHM6IEtlcm5lbCBhY2Nlc3Mgb2YgYmFkIGFyZWEsIHNpZzog
MTEgWyMxXQpBenRlayBOZXR3b3JrcyBjdGE1MDAwcwpOSVA6IGMwMDExMjE4IExSOiBjMDBmMGRh
MCBDVFI6IGMwMWFkZWM0ClJFR1M6IGMwMWQzZTcwIFRSQVA6IDAzMDAgICBOb3QgdGFpbnRlZCAg
KDIuNi4yNC4yKQpNU1I6IDAwMDAxMDMyIDxNRSxJUixEUj4gIENSOiA0NDA0MjA0OCAgWEVSOiAy
MDAwMDAwMApEQVI6IDAwMDAwMDAwLCBEU0lTUjogMjAwMDAwMDAKVEFTSyA9IGMwMWMyNTcwWzBd
ICdzd2FwcGVyJyBUSFJFQUQ6IGMwMWQyMDAwCkdQUjAwOiAwMDAwMDA2MyBjMDFkM2YyMCBjMDFj
MjU3MCAwMDAwMDAwMCBjMDE5MzYwNyBmZmZmZmZmZiBjMDFlMjM5OAowMDAwMDAwMApHUFIwODog
MDAwMDAwMDAgMDAwMDAwNzAgYzAxY2NiYjAgYzA3ZmQwN2EgMjQwNDIwNDggMjA4MDIwMTAgMDNm
ZmYwMDAKMDA3ZmZmMDAKR1BSMTY6IDAzYmJjZDNlIDAwMWE2ZDNlIDAyMjAwMDQwIDAzYTE2MDAw
IDAwN2ZmZWIwIDAzZmY5OGU0IDAwMDAwMDAwCjAwMDAwMDA0CkdQUjI0OiAwMDAwMDAwMCAwMjQw
MDAwMCA0MDAwMDAwMCBjMDFkOWQxNCAwMDAwMDAwMCBjMDFkM2Y1OCBjMDE5MzYwOApjM2ZmZTJm
MApDYWxsIFRyYWNlOgpbYzAxZDNmNDBdIFtjMDBmMGRlMF0KW2MwMWQzZjUwXSBbYzAwZjBlMjhd
CgpzZXR1cF9hcmNoOiBleGl0ClpvbmUgUEZOIHJhbmdlczoKICBETUEgICAgICAgICAgICAgMCAt
PiAgICAxNjM4NAogIE5vcm1hbCAgICAgIDE2Mzg0IC0+ICAgIDE2Mzg0Ck1vdmFibGUgem9uZSBz
dGFydCBQRk4gZm9yIGVhY2ggbm9kZQplYXJseV9ub2RlX21hcFsxXSBhY3RpdmUgUEZOIHJhbmdl
cwogICAgMDogICAgICAgIDAgLT4gICAgMTYzODQKbm9kZV9ib290bWVtX21hcChhZGRyKTogYzNm
ZmUwMDAKZWlkeChzaXplKTogMTYzODQKaShvZmZzZXQpOiAwCmVuZF9wZm46IDE2Mzg0CmdvYWw6
IDEwNzM3NDE4MjMKcHJlZmVycmVkOiAwCl9fYWxsb2NfYm9vdG1lbV9ub2RlKCk6IHB0cjogYzAx
ZTkwMDAKbm9kZV9ib290bWVtX21hcChhZGRyKTogYzNmZmUwMDAKZWlkeChzaXplKTogMTYzODQK
aShvZmZzZXQpOiAwCmVuZF9wZm46IDE2Mzg0CmdvYWw6IDEwNzM3NDE4MjMKcHJlZmVycmVkOiAw
Cl9fYWxsb2NfYm9vdG1lbV9ub2RlKCk6IHB0cjogYzAxMzEwMDAKbm9kZV9ib290bWVtX21hcChh
ZGRyKTogYzNmZmUwMDAKZWlkeChzaXplKTogMTYzODQKaShvZmZzZXQpOiAwCmVuZF9wZm46IDE2
Mzg0CmdvYWw6IDEwNzM3NDE4MjMKcHJlZmVycmVkOiAwCl9fYWxsb2NfYm9vdG1lbV9ub2RlKCk6
IHB0cjogYzAxNzEwMDAKbm9kZV9ib290bWVtX21hcChhZGRyKTogYzNmZmUwMDAKZWlkeChzaXpl
KTogMTYzODQKaShvZmZzZXQpOiAwCmVuZF9wZm46IDE2Mzg0CmdvYWw6IDEwNzM3NDE4MjMKcHJl
ZmVycmVkOiAwCm5vZGVfYm9vdG1lbV9tYXAoYWRkcik6IGMzZmZlMDAwCmVpZHgoc2l6ZSk6IDE2
Mzg0Cmkob2Zmc2V0KTogMAplbmRfcGZuOiAxNjM4NApnb2FsOiAxMDczNzQxODIzCnByZWZlcnJl
ZDogMApCdWlsdCAxIHpvbmVsaXN0cyBpbiBab25lIG9yZGVyLCBtb2JpbGl0eSBncm91cGluZyBv
bi4gIFRvdGFsIHBhZ2VzOgoxNjI1NgpLZXJuZWwgY29tbWFuZCBsaW5lOiAvZGVyb290PS9kZXYv
cu+/vW0gcncgcmFtZGlza19zaXplPTQwOTYKVW5hYmxlIHRvIGhhbmRsZSBrZXJuZWwgcGFnaW5n
IHJlcXVlc3QgZm9yIGRhdGEgYXQgYWRkcmVzcyAweDAwMDAwMDAwCkZhdWx0aW5nIGluc3RydWN0
aW9uIGFkZHJlc3M6IDB4YzAwMTEyMTgKT29wczogS2VybmVsIGFjY2VzcyBvZiBiYWQgYXJlYSwg
c2lnOiAxMSBbIzgxOTNdCkF6dGVrIE5ldHdvcmtzIGN0YTUwMDBzCk5JUDogYzAwMTEyMTggTFI6
IGMwMGYwZGEwIENUUjogYzAxYWRlYzQKUkVHUzogYzAxZDNlNzAgVFJBUDogMDMwMCAgIE5vdCB0
YWludGVkICAoMi42LjI0LjIpCk1TUjogMDAwMDEwMzIgPE1FLElSLERSPiAgQ1I6IDQ0MDQyMDI4
ICBYRVI6IDIwMDAwMDAwCkRBUjogMDAwMDAwMDAsIERTSVNSOiAyMDAwMDAwMApUQVNLID0gYzAx
YzI1NzBbMF0gJ3N3YXBwZXInIFRIUkVBRDogYzAxZDIwMDAKR1BSMDA6IDAwMDAwMDYzIGMwMWQz
ZjIwIGMwMWMyNTcwIDAwMDAwMDAwIGMwMTkzNjA3IGZmZmZmZmZmIGMwMWUyMzk4CjAwMDAwMDAw
CkdQUjA4OiBjMDFhMTJlMCBjMDFlMDAwMCBjMDE5MTcwMCBjMDFlMjM4NCAyNDA0MjAyOCAyNDg4
NjAxMiAwM2ZmZjAwMAowMDdmZmYwMApHUFIxNjogMDNiYmNkM2UgMDAxYTZkM2UgMDIyMDAwNDAg
MDNhMTYwMDAgMDA3ZmZlYjAgMDNmZjk4ZTQgMDAwMDAwMDAKMDAwMDAwMDQKR1BSMjQ6IDAwMDAw
MDAwIDAyNDAwMDAwIDQwMDAwMDAwIGMwMWQ5ZDE0IDAwMDAwMDAwIGMwMWQzZjU4IGMwMTkzNjA4
CmMzYmZmMTc0CkNhbGwgVHJhY2U6CltjMDFkM2Y0MF0gW2MwMGYwZGUwXQpbYzAxZDNmNTBdIFtj
MDBmMGUyOF0KW2MwMWQzZjgwXSBbYzAwZjBmMWNdCltjMDFkM2ZhMF0gW2MwMWFkZWU4XQpbYzAx
ZDNmYjBdIFtjMDFhOTg1NF0KW2MwMWQzZmMwXSBbYzAxYTM5MGNdCltjMDFkM2ZmMF0gWzAwMDAz
NDM4XQpJbnN0cnVjdGlvbiBkdW1wOgozODg0ZmZmZiA4YzA1MDAwMSAyYzAwMDAwMCA0MDgyZmZm
OCAzOGE1ZmZmZiA4YzA0MDAwMSAyYzAwMDAwMAo5YzA1MDAwMQo0MDgyZmZmNCA0ZTgwMDAyMCAz
OGEzZmZmZiAzODg0ZmZmZiA8OGM2NTAwMDE+IDJjODMwMDAwIDhjMDQwMDAxCjdjNjAxODUxCi0t
LVsgZW5kIHRyYWNlIDg2NDBhYmU2OWEzMTZkZWUgXS0tLQpLZXJuZWwgcGFuaWMgLSBub3Qgc3lu
Y2luZzogQXR0ZW1wdGVkIHRvIGtpbGwgdGhlIGlkbGUgdGFzayEKUmVib290aW5nIGluIDE4MCBz
ZWNvbmRzLi4K

^ permalink raw reply

* Re: muram in device tree for mpc8250 in arch/powerpc
From: Scott Wood @ 2008-03-21 20:34 UTC (permalink / raw)
  To: James Black; +Cc: linuxppc-embedded
In-Reply-To: <b77025b40803211314g74a41aa9of01f7cf3560a2505@mail.gmail.com>

James Black wrote:
> I've compiled the kernel with gcc 4.0.0 and  4.2.2 both versions have
> the same problem. Should I try the 2.6.25 kernel since the 2.6.19
> works just fine.

It's worth a try.  Also, since it seems to be a random corruption issue, 
make sure that no DMA is left running by the firmware (try resetting the 
CPM (though you'll have to turn off the early udbg output)), that you're 
not trying to use more memory than you have, etc.  Try dumping various 
registers to see if anything significant is different compared to 2.6.19.

If none of that helps, then I'm out of ideas as far as debug-by-email 
goes...

> I've stripped the kernel options down almost as far
> as I can go.

You turned off too much. :-)
Turn kallsyms back on, so you get symbols in crash dumps.

-Scott

^ permalink raw reply

* Re: dcr stuff.
From: Benjamin Herrenschmidt @ 2008-03-21 21:24 UTC (permalink / raw)
  To: Stephen Neuendorffer; +Cc: linuxppc-dev, git-dev
In-Reply-To: <20080321190524.4F3491CE0087@mail160-dub.bigfish.com>


On Fri, 2008-03-21 at 12:05 -0700, Stephen Neuendorffer wrote:
> Is anybody working on the dcr infrastructure?
> 
> In particular, there seems to be inconsistency between what the kernel
> does and what ePAPR thinks it should.  The code in the kernel selects
> mmio or native access based on a Kconfig parameter and uses some device
> tree parameters to correctly configure the mmio access, whereas ePAPR
> doesn't seem to document all the device tree attributes (in particular,
> dcr-mmio-stride and dcr-mmio-range) and provides a dynamic binding for
> dcr-access-method...
> 
> Is there a plan here?  (In particular, in FPGA designs, its possible to
> have crazy things like multiple independent dcr busses, some using
> native access, some using mmio.  Or even if there is one bus, many of
> the existing non-linux drivers we have assume native or mmio access.
> I'd like to clean this up, obviously.. :)

So far, the crazy things never happened, at least as far as I can tell,
which is why the kernel does what it does. That allows to inline the DCR
instructions on 4xx processors, and thus avoids code bloat. It also
avoids trying to use DCR instructions on processors that don't support
them like Cell with the Axon bridge.

If it becomes necessary, we can make it dynamic tho.

Ben.

^ permalink raw reply

* Re: muram in device tree for mpc8250 in arch/powerpc
From: James Black @ 2008-03-21 21:35 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-embedded
In-Reply-To: <47E41BE7.9020109@freescale.com>

Thanks much for the help Scott. Do you feel that the dts is correct
now and probably not the culprit? Do you have 2.6.24.2 running on
other boards? I've been checking u-boot to make sure that nothing over
there is messing me up. I noticed that there are new CFG_.. I'm using

#define CONFIG_OF_LIBFDT
#define CONFIG_OF_BOARD_SETUP
#define CONFIG_OF_HAS_UBOOT_ENV
#define CONFIG_OF_HAS_BD_T
#define CONFIG_OF_BOARD_SETUP
#define CONFIG_OF_BOOT_CPU      0

#define OF_CPU  "PowerPC,8250@0"
#define OF_SOC  "soc0@f0000000"
#define OF_TBCLK        (bd->bi_busfreq / 4)
#define OF_STDOUT_PATH  "/soc/cpm/serial@11a00"


/*
 * Device Tree for the CTA5000S board with an MPC8250 chip.
 * Device Tree for the PQ2FADS-ZU board with an MPC8280 chip.
 *
 * Copyright 2007 Freescale Semiconductor Inc.
 *
 * 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.
 */

/ {
	model = "cta5000s";
	compatible = "fsl,cta5000s";
	#address-cells = <1>;
	#size-cells = <1>;

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

		PowerPC,8250@0 {
			device_type = "cpu";
			reg = <0>;
			d-cache-line-size = <20>;
			i-cache-line-size = <20>;
			d-cache-size = <4000>;
			i-cache-size = <4000>;
			timebase-frequency = <0>;
			clock-frequency = <0>;
		};
	};

	memory {
		device_type = "memory";
		reg = <0 0>;
	};

	localbus@f0010100 {
		compatible = "fsl,mpc8250-localbus",
                   "fsl,pq2-localbus";

		#address-cells = <2>;
		#size-cells = <1>;
		reg = <f0010100 60>;
		ranges = <0 0 fe000000 00200000>;

		flash@fe000000,0 {
			compatible = "cfi-flash";
			reg = <0 fe000000  00200000>;
			bank-width = <2>;
			device-width = <1>;
		};
	};

	soc@f0000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		device_type = "soc";
		compatible = "fsl,mpc8250", "fsl,pq2-soc";
		ranges = <00000000 f0000000 00014000>;

		// Temporary -- will go away once kernel uses ranges for get_immrbase().
		reg = <f0000000 00014000>;

		cpm@119c0 {
			#address-cells = <1>;
			#size-cells = <1>;
			#interrupt-cells = <2>;
			compatible = "fsl,mpc8250-cpm", "fsl,cpm2";
			reg = <119c0 30>;
			ranges;

			muram@0 {
				#address-cells = <1>;
				#size-cells = <1>;
				ranges = <0 0 10000>;

				data{
					compatible = "fsl,cpm-muram-data";
					reg = <0 4000>;
				};
			};

			brg@119f0 {
				compatible = "fsl,mpc8250-brg",
				             "fsl,cpm2-brg",
				             "fsl,cpm-brg";
				reg = <119f0 10>;
			};

			serial@11a00 {
				device_type = "serial";
				compatible = "fsl,mpc8250-scc-uart",
				             "fsl,cpm2-scc-uart";
				reg = <11a00 20 8000 100>;
				interrupts = <28 8>;
				interrupt-parent = <&PIC>;
				fsl,cpm-brg = <1>;
				fsl,cpm-command = <00800000>;
			};

			serial@11a40 {
				device_type = "serial";
				compatible = "fsl,mpc8250-scc-uart",
				             "fsl,cpm2-scc-uart";
				reg = <11a40 20 8200 100>;
				interrupts = <2a 8>;
				interrupt-parent = <&PIC>;
				fsl,cpm-brg = <5>;
				fsl,cpm-command = <08c00000>;
			};

			ethernet@11300 {
				device_type = "network";
				compatible = "fsl,mpc8250-fcc-enet",
				             "fsl,cpm2-fcc-enet";
				reg = <11300 20 8400 100>;
				interrupts = <20 8>;
				interrupt-parent = <&PIC>;
				linux,network-index = <0>;
				fsl,cpm-command = <12000300>;
			};
		};

		PIC: interrupt-controller@10c00 {
			#interrupt-cells = <2>;
			interrupt-controller;
			reg = <10c00 80>;
			compatible = "fsl,mpc8250-pic", "fsl,cpm2-pic";
		};

	};

	chosen {
		linux,stdout-path = "/soc/cpm/serial@11a00";
	};
};

I think I'll try fresh source in 2.6.24, if I get the same result,
I'll give 2.6.25 a shot.

Again Thanks much...

Jim Black


On Fri, Mar 21, 2008 at 2:34 PM, Scott Wood <scottwood@freescale.com> wrote:
> James Black wrote:
>  > I've compiled the kernel with gcc 4.0.0 and  4.2.2 both versions have
>  > the same problem. Should I try the 2.6.25 kernel since the 2.6.19
>  > works just fine.
>
>  It's worth a try.  Also, since it seems to be a random corruption issue,
>  make sure that no DMA is left running by the firmware (try resetting the
>  CPM (though you'll have to turn off the early udbg output)), that you're
>  not trying to use more memory than you have, etc.  Try dumping various
>  registers to see if anything significant is different compared to 2.6.19.
>
>  If none of that helps, then I'm out of ideas as far as debug-by-email
>  goes...
>
>
>  > I've stripped the kernel options down almost as far
>  > as I can go.
>
>  You turned off too much. :-)
>  Turn kallsyms back on, so you get symbols in crash dumps.
>
>  -Scott
>

^ permalink raw reply

* [PATCH 0/8] pseries: phyp dump: hypervisor-assisted dump
From: Manish Ahuja @ 2008-03-21 22:42 UTC (permalink / raw)
  To: ppc, paulus, michael, Linas Vepstas

The following series of patches implement a basic framework
for hypervisor-assisted dump. The very first patch provides 
documentation explaining what this is. 

A list of open issues / todo list is included in the documentation.
It also appears that the not-yet-released firmware versions this was tested 
on are still,incomplete; this work is also pending.

The following is a list of changes from previous version:
- Deleted ifdef CONFIG_PHYP_DUMP from early_init_dt_scan_phyp_dump function.
- Changed reserve_crashed_mem() to phyp_dump_reserve_mem() as suggested.
- Added #ifdef CONFIG_PHYP_DUMP around of_scan_flat_dt call, removed empty function
  from header file.
- Changed phyp_dump_global to phyp_dump_vars.
- Changed style issues at several places.

Manish & Linas.

^ permalink raw reply

* [PATCH 1/8] pseries: phyp dump: Documentation
From: Manish Ahuja @ 2008-03-21 23:33 UTC (permalink / raw)
  To: linuxppc-dev, paulus, michael; +Cc: mahuja, linasvepstas
In-Reply-To: <47E439C3.6050904@austin.ibm.com>


Basic documentation for hypervisor-assisted dump.

Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>
Signed-off-by: Manish Ahuja <mahuja@us.ibm.com>

---
 Documentation/powerpc/phyp-assisted-dump.txt |  127 +++++++++++++++++++++++++++
 1 file changed, 127 insertions(+)

Index: 2.6.25-rc1/Documentation/powerpc/phyp-assisted-dump.txt
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ 2.6.25-rc1/Documentation/powerpc/phyp-assisted-dump.txt	2008-02-18 03:22:33.000000000 -0600
@@ -0,0 +1,127 @@
+
+                   Hypervisor-Assisted Dump
+                   ------------------------
+                       November 2007
+
+The goal of hypervisor-assisted dump is to enable the dump of
+a crashed system, and to do so from a fully-reset system, and
+to minimize the total elapsed time until the system is back
+in production use.
+
+As compared to kdump or other strategies, hypervisor-assisted
+dump offers several strong, practical advantages:
+
+-- Unlike kdump, the system has been reset, and loaded
+   with a fresh copy of the kernel.  In particular,
+   PCI and I/O devices have been reinitialized and are
+   in a clean, consistent state.
+-- As the dump is performed, the dumped memory becomes
+   immediately available to the system for normal use.
+-- After the dump is completed, no further reboots are
+   required; the system will be fully usable, and running
+   in it's normal, production mode on it normal kernel.
+
+The above can only be accomplished by coordination with,
+and assistance from the hypervisor. The procedure is
+as follows:
+
+-- When a system crashes, the hypervisor will save
+   the low 256MB of RAM to a previously registered
+   save region. It will also save system state, system
+   registers, and hardware PTE's.
+
+-- After the low 256MB area has been saved, the
+   hypervisor will reset PCI and other hardware state.
+   It will *not* clear RAM. It will then launch the
+   bootloader, as normal.
+
+-- The freshly booted kernel will notice that there
+   is a new node (ibm,dump-kernel) in the device tree,
+   indicating that there is crash data available from
+   a previous boot. It will boot into only 256MB of RAM,
+   reserving the rest of system memory.
+
+-- Userspace tools will parse /sys/kernel/release_region
+   and read /proc/vmcore to obtain the contents of memory,
+   which holds the previous crashed kernel. The userspace
+   tools may copy this info to disk, or network, nas, san,
+   iscsi, etc. as desired.
+
+   For Example: the values in /sys/kernel/release-region
+   would look something like this (address-range pairs).
+   CPU:0x177fee000-0x10000: HPTE:0x177ffe020-0x1000: /
+   DUMP:0x177fff020-0x10000000, 0x10000000-0x16F1D370A
+
+-- As the userspace tools complete saving a portion of
+   dump, they echo an offset and size to
+   /sys/kernel/release_region to release the reserved
+   memory back to general use.
+
+   An example of this is:
+     "echo 0x40000000 0x10000000 > /sys/kernel/release_region"
+   which will release 256MB at the 1GB boundary.
+
+Please note that the hypervisor-assisted dump feature
+is only available on Power6-based systems with recent
+firmware versions.
+
+Implementation details:
+----------------------
+
+During boot, a check is made to see if firmware supports
+this feature on this particular machine. If it does, then
+we check to see if a active dump is waiting for us. If yes
+then everything but 256 MB of RAM is reserved during early
+boot. This area is released once we collect a dump from user
+land scripts that are run. If there is dump data, then
+the /sys/kernel/release_region file is created, and
+the reserved memory is held.
+
+If there is no waiting dump data, then only the highest
+256MB of the ram is reserved as a scratch area. This area
+is *not* be released: this region will be kept permanently
+reserved, so that it can act as a receptacle for a copy
+of the low 256MB in the case a crash does occur. See,
+however, "open issues" below, as to whether
+such a reserved region is really needed.
+
+Currently the dump will be copied from /proc/vmcore to a
+a new file upon user intervention. The starting address
+to be read and the range for each data point in provided
+in /sys/kernel/release_region.
+
+The tools to examine the dump will be same as the ones
+used for kdump.
+
+General notes:
+--------------
+Security: please note that there are potential security issues
+with any sort of dump mechanism. In particular, plaintext
+(unencrypted) data, and possibly passwords, may be present in
+the dump data. Userspace tools must take adequate precautions to
+preserve security.
+
+Open issues/ToDo:
+------------
+ o The various code paths that tell the hypervisor that a crash
+   occurred, vs. it simply being a normal reboot, should be
+   reviewed, and possibly clarified/fixed.
+
+ o Instead of using /sys/kernel, should there be a /sys/dump
+   instead? There is a dump_subsys being created by the s390 code,
+   perhaps the pseries code should use a similar layout as well.
+
+ o Is reserving a 256MB region really required? The goal of
+   reserving a 256MB scratch area is to make sure that no
+   important crash data is clobbered when the hypervisor
+   save low mem to the scratch area. But, if one could assure
+   that nothing important is located in some 256MB area, then
+   it would not need to be reserved. Something that can be
+   improved in subsequent versions.
+
+ o Still working the kdump team to integrate this with kdump,
+   some work remains but this would not affect the current
+   patches.
+
+ o Still need to write a shell script, to copy the dump away.
+   Currently I am parsing it manually.

^ permalink raw reply

* [PATCH 2/8] pseries: phyp dump: reserve-release
From: Manish Ahuja @ 2008-03-21 23:37 UTC (permalink / raw)
  To: linuxppc-dev, paulus, michael; +Cc: mahuja, linasvepstas
In-Reply-To: <47E439C3.6050904@austin.ibm.com>


Initial patch for reserving memory in early boot, and freeing it later.
If the previous boot had ended with a crash, the reserved memory would contain
a copy of the crashed kernel data.

Signed-off-by: Manish Ahuja <mahuja@us.ibm.com>
Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>

---
 arch/powerpc/kernel/prom.c                 |   52 ++++++++++++++
 arch/powerpc/platforms/pseries/Makefile    |    1 
 arch/powerpc/platforms/pseries/phyp_dump.c |  103 +++++++++++++++++++++++++++++
 include/asm-powerpc/phyp_dump.h            |   41 +++++++++++
 4 files changed, 197 insertions(+)

Index: 2.6.25-rc1/include/asm-powerpc/phyp_dump.h
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ 2.6.25-rc1/include/asm-powerpc/phyp_dump.h	2008-03-21 23:37:11.000000000 -0500
@@ -0,0 +1,41 @@
+/*
+ * Hypervisor-assisted dump
+ *
+ * Linas Vepstas, Manish Ahuja 2008
+ * Copyright 2008 IBM Corp.
+ *
+ *      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.
+ */
+
+#ifndef _PPC64_PHYP_DUMP_H
+#define _PPC64_PHYP_DUMP_H
+
+#ifdef CONFIG_PHYP_DUMP
+
+/* The RMR region will be saved for later dumping
+ * whenever the kernel crashes. Set this to 256MB. */
+#define PHYP_DUMP_RMR_START 0x0
+#define PHYP_DUMP_RMR_END   (1UL<<28)
+
+struct phyp_dump {
+	/* Memory that is reserved during very early boot. */
+	unsigned long init_reserve_start;
+	unsigned long init_reserve_size;
+	/* Check status during boot if dump supported, active & present*/
+	unsigned long phyp_dump_configured;
+	unsigned long phyp_dump_is_active;
+	/* store cpu & hpte size */
+	unsigned long cpu_state_size;
+	unsigned long hpte_region_size;
+};
+
+extern struct phyp_dump *phyp_dump_info;
+
+int early_init_dt_scan_phyp_dump(unsigned long node,
+		const char *uname, int depth, void *data);
+
+#endif /* CONFIG_PHYP_DUMP */
+#endif /* _PPC64_PHYP_DUMP_H */
Index: 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c	2008-03-21 23:37:12.000000000 -0500
@@ -0,0 +1,103 @@
+/*
+ * Hypervisor-assisted dump
+ *
+ * Linas Vepstas, Manish Ahuja 2008
+ * Copyright 2008 IBM Corp.
+ *
+ *      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.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/mm.h>
+#include <linux/pfn.h>
+#include <linux/swap.h>
+
+#include <asm/page.h>
+#include <asm/phyp_dump.h>
+#include <asm/machdep.h>
+#include <asm/prom.h>
+
+/* Variables, used to communicate data between early boot and late boot */
+static struct phyp_dump phyp_dump_vars;
+struct phyp_dump *phyp_dump_info = &phyp_dump_vars;
+
+/**
+ * release_memory_range -- release memory previously lmb_reserved
+ * @start_pfn: starting physical frame number
+ * @nr_pages: number of pages to free.
+ *
+ * This routine will release memory that had been previously
+ * lmb_reserved in early boot. The released memory becomes
+ * available for genreal use.
+ */
+static void
+release_memory_range(unsigned long start_pfn, unsigned long nr_pages)
+{
+	struct page *rpage;
+	unsigned long end_pfn;
+	long i;
+
+	end_pfn = start_pfn + nr_pages;
+
+	for (i = start_pfn; i <= end_pfn; i++) {
+		rpage = pfn_to_page(i);
+		if (PageReserved(rpage)) {
+			ClearPageReserved(rpage);
+			init_page_count(rpage);
+			__free_page(rpage);
+			totalram_pages++;
+		}
+	}
+}
+
+static int __init phyp_dump_setup(void)
+{
+	unsigned long start_pfn, nr_pages;
+
+	/* If no memory was reserved in early boot, there is nothing to do */
+	if (phyp_dump_info->init_reserve_size == 0)
+		return 0;
+
+	/* Release memory that was reserved in early boot */
+	start_pfn = PFN_DOWN(phyp_dump_info->init_reserve_start);
+	nr_pages = PFN_DOWN(phyp_dump_info->init_reserve_size);
+	release_memory_range(start_pfn, nr_pages);
+
+	return 0;
+}
+machine_subsys_initcall(pseries, phyp_dump_setup);
+
+int __init early_init_dt_scan_phyp_dump(unsigned long node,
+		const char *uname, int depth, void *data)
+{
+	const unsigned int *sizes;
+
+	phyp_dump_info->phyp_dump_configured = 0;
+	phyp_dump_info->phyp_dump_is_active = 0;
+
+	if (depth != 1 || strcmp(uname, "rtas") != 0)
+		return 0;
+
+	if (of_get_flat_dt_prop(node, "ibm,configure-kernel-dump", NULL))
+		phyp_dump_info->phyp_dump_configured++;
+
+	if (of_get_flat_dt_prop(node, "ibm,dump-kernel", NULL))
+		phyp_dump_info->phyp_dump_is_active++;
+
+	sizes = of_get_flat_dt_prop(node, "ibm,configure-kernel-dump-sizes",
+				    NULL);
+	if (!sizes)
+		return 0;
+
+	if (sizes[0] == 1)
+		phyp_dump_info->cpu_state_size = *((unsigned long *)&sizes[1]);
+
+	if (sizes[3] == 2)
+		phyp_dump_info->hpte_region_size =
+						*((unsigned long *)&sizes[4]);
+	return 1;
+}
Index: 2.6.25-rc1/arch/powerpc/platforms/pseries/Makefile
===================================================================
--- 2.6.25-rc1.orig/arch/powerpc/platforms/pseries/Makefile	2008-03-21 00:01:26.000000000 -0500
+++ 2.6.25-rc1/arch/powerpc/platforms/pseries/Makefile	2008-03-21 00:02:15.000000000 -0500
@@ -18,3 +18,4 @@ obj-$(CONFIG_HOTPLUG_CPU)	+= hotplug-cpu
 obj-$(CONFIG_HVC_CONSOLE)	+= hvconsole.o
 obj-$(CONFIG_HVCS)		+= hvcserver.o
 obj-$(CONFIG_HCALL_STATS)	+= hvCall_inst.o
+obj-$(CONFIG_PHYP_DUMP)	+= phyp_dump.o
Index: 2.6.25-rc1/arch/powerpc/kernel/prom.c
===================================================================
--- 2.6.25-rc1.orig/arch/powerpc/kernel/prom.c	2008-03-21 00:01:26.000000000 -0500
+++ 2.6.25-rc1/arch/powerpc/kernel/prom.c	2008-03-21 22:59:37.000000000 -0500
@@ -51,6 +51,7 @@
 #include <asm/machdep.h>
 #include <asm/pSeries_reconfig.h>
 #include <asm/pci-bridge.h>
+#include <asm/phyp_dump.h>
 #include <asm/kexec.h>
 
 #ifdef DEBUG
@@ -1039,6 +1040,51 @@ static void __init early_reserve_mem(voi
 #endif
 }
 
+#ifdef CONFIG_PHYP_DUMP
+/**
+ * phyp_dump_reserve_mem() - reserve all not-yet-dumped mmemory
+ *
+ * This routine may reserve memory regions in the kernel only
+ * if the system is supported and a dump was taken in last
+ * boot instance or if the hardware is supported and the
+ * scratch area needs to be setup. In other instances it returns
+ * without reserving anything. The memory in case of dump being
+ * active is freed when the dump is collected (by userland tools).
+ */
+static void __init phyp_dump_reserve_mem(void)
+{
+	unsigned long base, size;
+	if (!phyp_dump_info->phyp_dump_configured) {
+		printk(KERN_ERR "Phyp-dump not supported on this hardware\n");
+		return;
+	}
+
+	if (phyp_dump_info->phyp_dump_is_active) {
+		/* Reserve *everything* above RMR.Area freed by userland tools*/
+		base = PHYP_DUMP_RMR_END;
+		size = lmb_end_of_DRAM() - base;
+
+		/* XXX crashed_ram_end is wrong, since it may be beyond
+		 * the memory_limit, it will need to be adjusted. */
+		lmb_reserve(base, size);
+
+		phyp_dump_info->init_reserve_start = base;
+		phyp_dump_info->init_reserve_size = size;
+	} else {
+		size = phyp_dump_info->cpu_state_size +
+			phyp_dump_info->hpte_region_size +
+			PHYP_DUMP_RMR_END;
+		base = lmb_end_of_DRAM() - size;
+		lmb_reserve(base, size);
+		phyp_dump_info->init_reserve_start = base;
+		phyp_dump_info->init_reserve_size = size;
+	}
+}
+#else
+static inline void __init phyp_dump_reserve_mem(void) {}
+#endif /* CONFIG_PHYP_DUMP  && CONFIG_PPC_RTAS */
+
+
 void __init early_init_devtree(void *params)
 {
 	DBG(" -> early_init_devtree(%p)\n", params);
@@ -1051,6 +1097,11 @@ void __init early_init_devtree(void *par
 	of_scan_flat_dt(early_init_dt_scan_rtas, NULL);
 #endif
 
+#ifdef CONFIG_PHYP_DUMP
+	/* scan tree to see if dump occured during last boot */
+	of_scan_flat_dt(early_init_dt_scan_phyp_dump, NULL);
+#endif
+
 	/* Retrieve various informations from the /chosen node of the
 	 * device-tree, including the platform type, initrd location and
 	 * size, TCE reserve, and more ...
@@ -1071,6 +1122,7 @@ void __init early_init_devtree(void *par
 	reserve_kdump_trampoline();
 	reserve_crashkernel();
 	early_reserve_mem();
+	phyp_dump_reserve_mem();
 
 	lmb_enforce_memory_limit(memory_limit);
 	lmb_analyze();

^ permalink raw reply

* [PATCH 3/8] pseries: phyp dump: use sysfs to release reserved mem
From: Manish Ahuja @ 2008-03-21 23:39 UTC (permalink / raw)
  To: linuxppc-dev, paulus, michael; +Cc: mahuja, linasvepstas
In-Reply-To: <47E439C3.6050904@austin.ibm.com>


Check to see if there actually is data from a previously
crashed kernel waiting. If so, Allow user-sapce tools to
grab the data (by reading /proc/kcore). When user-space 
finishes dumping a section, it must release that memory
by writing to sysfs. For example,

  echo "0x40000000 0x10000000" > /sys/kernel/release_region

will release 256MB starting at the 1GB.  The released memory
becomes free for general use.

Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>
Signed-off-by: Manish Ahuja <mahuja@us.ibm.com>

---
 arch/powerpc/platforms/pseries/phyp_dump.c |   81 +++++++++++++++++++++++++++--
 1 file changed, 76 insertions(+), 5 deletions(-)

Index: 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c
===================================================================
--- 2.6.25-rc1.orig/arch/powerpc/platforms/pseries/phyp_dump.c	2008-03-21 00:10:15.000000000 -0500
+++ 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c	2008-03-21 22:39:21.000000000 -0500
@@ -12,19 +12,24 @@
  */
 
 #include <linux/init.h>
+#include <linux/kobject.h>
 #include <linux/mm.h>
+#include <linux/of.h>
 #include <linux/pfn.h>
 #include <linux/swap.h>
+#include <linux/sysfs.h>
 
 #include <asm/page.h>
 #include <asm/phyp_dump.h>
 #include <asm/machdep.h>
 #include <asm/prom.h>
+#include <asm/rtas.h>
 
 /* Variables, used to communicate data between early boot and late boot */
 static struct phyp_dump phyp_dump_vars;
 struct phyp_dump *phyp_dump_info = &phyp_dump_vars;
 
+/* ------------------------------------------------- */
 /**
  * release_memory_range -- release memory previously lmb_reserved
  * @start_pfn: starting physical frame number
@@ -54,18 +59,84 @@ release_memory_range(unsigned long start
 	}
 }
 
-static int __init phyp_dump_setup(void)
+/* ------------------------------------------------- */
+/**
+ * sysfs_release_region -- sysfs interface to release memory range.
+ *
+ * Usage:
+ *   "echo <start addr> <length> > /sys/kernel/release_region"
+ *
+ * Example:
+ *   "echo 0x40000000 0x10000000 > /sys/kernel/release_region"
+ *
+ * will release 256MB starting at 1GB.
+ */
+static ssize_t store_release_region(struct kobject *kobj,
+				struct kobj_attribute *attr,
+				const char *buf, size_t count)
 {
+	unsigned long start_addr, length, end_addr;
 	unsigned long start_pfn, nr_pages;
+	ssize_t ret;
+
+	ret = sscanf(buf, "%lx %lx", &start_addr, &length);
+	if (ret != 2)
+		return -EINVAL;
+
+	/* Range-check - don't free any reserved memory that
+	 * wasn't reserved for phyp-dump */
+	if (start_addr < phyp_dump_info->init_reserve_start)
+		start_addr = phyp_dump_info->init_reserve_start;
+
+	end_addr = phyp_dump_info->init_reserve_start +
+			phyp_dump_info->init_reserve_size;
+	if (start_addr+length > end_addr)
+		length = end_addr - start_addr;
+
+	/* Release the region of memory assed in by user */
+	start_pfn = PFN_DOWN(start_addr);
+	nr_pages = PFN_DOWN(length);
+	release_memory_range(start_pfn, nr_pages);
+
+	return count;
+}
+
+static struct kobj_attribute rr = __ATTR(release_region, 0600,
+					 NULL, store_release_region);
+
+static int __init phyp_dump_setup(void)
+{
+	struct device_node *rtas;
+	const int *dump_header = NULL;
+	int header_len = 0;
+	int rc;
 
 	/* If no memory was reserved in early boot, there is nothing to do */
 	if (phyp_dump_info->init_reserve_size == 0)
 		return 0;
 
-	/* Release memory that was reserved in early boot */
-	start_pfn = PFN_DOWN(phyp_dump_info->init_reserve_start);
-	nr_pages = PFN_DOWN(phyp_dump_info->init_reserve_size);
-	release_memory_range(start_pfn, nr_pages);
+	/* Return if phyp dump not supported */
+	if (!phyp_dump_info->phyp_dump_configured)
+		return -ENOSYS;
+
+	/* Is there dump data waiting for us? */
+	rtas = of_find_node_by_path("/rtas");
+	if (rtas) {
+		dump_header = of_get_property(rtas, "ibm,kernel-dump",
+						&header_len);
+		of_node_put(rtas);
+	}
+
+	if (dump_header == NULL)
+		return 0;
+
+	/* Should we create a dump_subsys, analogous to s390/ipl.c ? */
+	rc = sysfs_create_file(kernel_kobj, &rr.attr);
+	if (rc) {
+		printk(KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n",
+									rc);
+		return 0;
+	}
 
 	return 0;
 }

^ permalink raw reply

* [PATCH 4/8] pseries: phyp dump: register dump area.
From: Manish Ahuja @ 2008-03-21 23:43 UTC (permalink / raw)
  To: linuxppc-dev, paulus, michael; +Cc: mahuja, linasvepstas
In-Reply-To: <47E439C3.6050904@austin.ibm.com>



Set up the actual dump header, register it with the hypervisor.

Signed-off-by: Manish Ahuja <mahuja@us.ibm.com>
Signed-off-by: Linas Vepstas <linasvepstas@gmail.com>

---
 arch/powerpc/platforms/pseries/phyp_dump.c |  137 +++++++++++++++++++++++++++--
 1 file changed, 131 insertions(+), 6 deletions(-)

Index: 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c
===================================================================
--- 2.6.25-rc1.orig/arch/powerpc/platforms/pseries/phyp_dump.c	2008-03-21 22:39:21.000000000 -0500
+++ 2.6.25-rc1/arch/powerpc/platforms/pseries/phyp_dump.c	2008-03-21 22:52:53.000000000 -0500
@@ -29,6 +29,117 @@
 static struct phyp_dump phyp_dump_vars;
 struct phyp_dump *phyp_dump_info = &phyp_dump_vars;
 
+static int ibm_configure_kernel_dump;
+/* ------------------------------------------------- */
+/* RTAS interfaces to declare the dump regions */
+
+struct dump_section {
+	u32 dump_flags;
+	u16 source_type;
+	u16 error_flags;
+	u64 source_address;
+	u64 source_length;
+	u64 length_copied;
+	u64 destination_address;
+};
+
+struct phyp_dump_header {
+	u32 version;
+	u16 num_of_sections;
+	u16 status;
+
+	u32 first_offset_section;
+	u32 dump_disk_section;
+	u64 block_num_dd;
+	u64 num_of_blocks_dd;
+	u32 offset_dd;
+	u32 maxtime_to_auto;
+	/* No dump disk path string used */
+
+	struct dump_section cpu_data;
+	struct dump_section hpte_data;
+	struct dump_section kernel_data;
+};
+
+/* The dump header *must be* in low memory, so .bss it */
+static struct phyp_dump_header phdr;
+
+#define NUM_DUMP_SECTIONS	3
+#define DUMP_HEADER_VERSION	0x1
+#define DUMP_REQUEST_FLAG	0x1
+#define DUMP_SOURCE_CPU		0x0001
+#define DUMP_SOURCE_HPTE	0x0002
+#define DUMP_SOURCE_RMO		0x0011
+
+/**
+ * init_dump_header() - initialize the header declaring a dump
+ * Returns: length of dump save area.
+ *
+ * When the hypervisor saves crashed state, it needs to put
+ * it somewhere. The dump header tells the hypervisor where
+ * the data can be saved.
+ */
+static unsigned long init_dump_header(struct phyp_dump_header *ph)
+{
+	unsigned long addr_offset = 0;
+
+	/* Set up the dump header */
+	ph->version = DUMP_HEADER_VERSION;
+	ph->num_of_sections = NUM_DUMP_SECTIONS;
+	ph->status = 0;
+
+	ph->first_offset_section =
+		(u32)offsetof(struct phyp_dump_header, cpu_data);
+	ph->dump_disk_section = 0;
+	ph->block_num_dd = 0;
+	ph->num_of_blocks_dd = 0;
+	ph->offset_dd = 0;
+
+	ph->maxtime_to_auto = 0; /* disabled */
+
+	/* The first two sections are mandatory */
+	ph->cpu_data.dump_flags = DUMP_REQUEST_FLAG;
+	ph->cpu_data.source_type = DUMP_SOURCE_CPU;
+	ph->cpu_data.source_address = 0;
+	ph->cpu_data.source_length = phyp_dump_info->cpu_state_size;
+	ph->cpu_data.destination_address = addr_offset;
+	addr_offset += phyp_dump_info->cpu_state_size;
+
+	ph->hpte_data.dump_flags = DUMP_REQUEST_FLAG;
+	ph->hpte_data.source_type = DUMP_SOURCE_HPTE;
+	ph->hpte_data.source_address = 0;
+	ph->hpte_data.source_length = phyp_dump_info->hpte_region_size;
+	ph->hpte_data.destination_address = addr_offset;
+	addr_offset += phyp_dump_info->hpte_region_size;
+
+	/* This section describes the low kernel region */
+	ph->kernel_data.dump_flags = DUMP_REQUEST_FLAG;
+	ph->kernel_data.source_type = DUMP_SOURCE_RMO;
+	ph->kernel_data.source_address = PHYP_DUMP_RMR_START;
+	ph->kernel_data.source_length = PHYP_DUMP_RMR_END;
+	ph->kernel_data.destination_address = addr_offset;
+	addr_offset += ph->kernel_data.source_length;
+
+	return addr_offset;
+}
+
+static void register_dump_area(struct phyp_dump_header *ph, unsigned long addr)
+{
+	int rc;
+	ph->cpu_data.destination_address += addr;
+	ph->hpte_data.destination_address += addr;
+	ph->kernel_data.destination_address += addr;
+
+	do {
+		rc = rtas_call(ibm_configure_kernel_dump, 3, 1, NULL,
+				1, ph, sizeof(struct phyp_dump_header));
+	} while (rtas_busy_delay(rc));
+
+	if (rc)
+		printk(KERN_ERR "phyp-dump: unexpected error (%d) on "
+						"register\n", rc);
+}
+
 /* ------------------------------------------------- */
 /**
  * release_memory_range -- release memory previously lmb_reserved
@@ -107,7 +218,9 @@ static struct kobj_attribute rr = __ATTR
 static int __init phyp_dump_setup(void)
 {
 	struct device_node *rtas;
-	const int *dump_header = NULL;
+ 	const struct phyp_dump_header *dump_header = NULL;
+ 	unsigned long dump_area_start;
+ 	unsigned long dump_area_length;
 	int header_len = 0;
 	int rc;
 
@@ -119,7 +232,13 @@ static int __init phyp_dump_setup(void)
 	if (!phyp_dump_info->phyp_dump_configured)
 		return -ENOSYS;
 
-	/* Is there dump data waiting for us? */
+	/* Is there dump data waiting for us? If there isn't,
+	 * then register a new dump area, and release all of
+	 * the rest of the reserved ram.
+	 *
+	 * The /rtas/ibm,kernel-dump rtas node is present only
+	 * if there is dump data waiting for us.
+	 */
 	rtas = of_find_node_by_path("/rtas");
 	if (rtas) {
 		dump_header = of_get_property(rtas, "ibm,kernel-dump",
@@ -127,17 +246,23 @@ static int __init phyp_dump_setup(void)
 		of_node_put(rtas);
 	}
 
-	if (dump_header == NULL)
+	dump_area_length = init_dump_header(&phdr);
+
+	/* align down */
+	dump_area_start = phyp_dump_info->init_reserve_start & PAGE_MASK;
+
+	if (dump_header == NULL) {
+		register_dump_area(&phdr, dump_area_start);
 		return 0;
+	}
 
 	/* Should we create a dump_subsys, analogous to s390/ipl.c ? */
 	rc = sysfs_create_file(kernel_kobj, &rr.attr);
-	if (rc) {
+	if (rc)
 		printk(KERN_ERR "phyp-dump: unable to create sysfs file (%d)\n",
 									rc);
-		return 0;
-	}
 
+	/* ToDo: re-register the dump area, for next time. */
 	return 0;
 }
 machine_subsys_initcall(pseries, phyp_dump_setup);

^ permalink raw reply


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