get_pteptr prototype

get_pteptr prototype

[Hollis Blanchard]

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

Hi, I think the prototype for get_pteptr should be moved to
asm/pgtable.h . It currently is declared extern in mm/fault.c, and I
will need it for platforms/ibm405lp_pm.c .

Please apply to _2_4_devel and -2.5, and probably _2_4 for that matter.

-Hollis
--
PowerPC Linux
IBM Linux Technology Center

[-- Attachment #2: get_pteptr.diff --]
[-- Type: text/plain, Size: 1637 bytes --]

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux 2.4 for PowerPC development tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1179  -> 1.1180 
#	include/asm-ppc/pgtable.h	1.52    -> 1.53   
#	 arch/ppc/mm/fault.c	1.28    -> 1.29   
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/01/06	hollis@granite.austin.ibm.com	1.1180
# move get_pteptr prototype to pgtable.h
# --------------------------------------------
#
diff -Nru a/arch/ppc/mm/fault.c b/arch/ppc/mm/fault.c
--- a/arch/ppc/mm/fault.c	Mon Jan  6 16:46:23 2003
+++ b/arch/ppc/mm/fault.c	Mon Jan  6 16:46:23 2003
@@ -55,7 +55,6 @@
 extern void die_if_kernel(char *, struct pt_regs *, long);
 void bad_page_fault(struct pt_regs *, unsigned long, int sig);
 void do_page_fault(struct pt_regs *, unsigned long, unsigned long);
-extern int get_pteptr(struct mm_struct *mm, unsigned long addr, pte_t **ptep);
 
 /*
  * For 600- and 800-family processors, the error_code parameter is DSISR
diff -Nru a/include/asm-ppc/pgtable.h b/include/asm-ppc/pgtable.h
--- a/include/asm-ppc/pgtable.h	Mon Jan  6 16:46:23 2003
+++ b/include/asm-ppc/pgtable.h	Mon Jan  6 16:46:23 2003
@@ -112,6 +112,7 @@
 extern unsigned long va_to_phys(unsigned long address);
 extern pte_t *va_to_pte(unsigned long address);
 extern unsigned long ioremap_bot, ioremap_base;
+extern int get_pteptr(struct mm_struct *mm, unsigned long addr, pte_t **ptep);
 #endif /* __ASSEMBLY__ */
 
 /*

Re: get_pteptr prototype

[David Gibson]

On Mon, Jan 06, 2003 at 04:59:26PM -0600, Hollis Blanchard wrote:
> Hi, I think the prototype for get_pteptr should be moved to
> asm/pgtable.h . It currently is declared extern in mm/fault.c, and I
> will need it for platforms/ibm405lp_pm.c .
>
> Please apply to _2_4_devel and -2.5, and probably _2_4 for that matter.

Hrm... what are you actually intending to use get_pteptr() for.  There
are currently only two users, one of which needs to die (in 2.5, at
least).  My experience has been that the concept of get_pteptr() is a
great deal less useful that one would, at first, think.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: get_pteptr prototype

[Hollis Blanchard]

On Mon, 2003-01-06 at 18:57, David Gibson wrote:
>
> On Mon, Jan 06, 2003 at 04:59:26PM -0600, Hollis Blanchard wrote:
> > Hi, I think the prototype for get_pteptr should be moved to
> > asm/pgtable.h . It currently is declared extern in mm/fault.c, and I
> > will need it for platforms/ibm405lp_pm.c .
> >
> > Please apply to _2_4_devel and -2.5, and probably _2_4 for that matter.
>
> Hrm... what are you actually intending to use get_pteptr() for.

I use it to acquire the PTE pointer for a page I need to mark writable
(see the init function in
http://penguinppc.org/~hollis/405LP-sleep.diff). It's sort of like
CONFIG_XMON mapping the whole kernel writable, except I only need one
page. If you know of a better way, please let me know!

> There
> are currently only two users, one of which needs to die (in 2.5, at
> least).  My experience has been that the concept of get_pteptr() is a
> great deal less useful that one would, at first, think.

It's simply a wrapper around successive pte/pmd/pgd_offset calls, with
some error checking. Are you saying those calls are not the correct way
to get the PTE pointer for an address?

-Hollis
--
PowerPC Linux
IBM Linux Technology Center


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: get_pteptr prototype

[David Gibson]

On Wed, Jan 08, 2003 at 09:48:48AM -0600, Hollis Blanchard wrote:
>
> On Mon, 2003-01-06 at 18:57, David Gibson wrote:
> >
> > On Mon, Jan 06, 2003 at 04:59:26PM -0600, Hollis Blanchard wrote:
> > > Hi, I think the prototype for get_pteptr should be moved to
> > > asm/pgtable.h . It currently is declared extern in mm/fault.c, and I
> > > will need it for platforms/ibm405lp_pm.c .
> > >
> > > Please apply to _2_4_devel and -2.5, and probably _2_4 for that matter.
> >
> > Hrm... what are you actually intending to use get_pteptr() for.
>
> I use it to acquire the PTE pointer for a page I need to mark writable
> (see the init function in
> http://penguinppc.org/~hollis/405LP-sleep.diff). It's sort of like
> CONFIG_XMON mapping the whole kernel writable, except I only need one
> page. If you know of a better way, please let me know!
>
> > There
> > are currently only two users, one of which needs to die (in 2.5, at
> > least).  My experience has been that the concept of get_pteptr() is a
> > great deal less useful that one would, at first, think.
>
> It's simply a wrapper around successive pte/pmd/pgd_offset calls, with
> some error checking. Are you saying those calls are not the correct way
> to get the PTE pointer for an address?

Trouble is that this will break badly in 2.5 where we use large-page
entries for the lowmem mapping.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: get_pteptr prototype

[Hollis Blanchard]

On Wed, 2003-01-08 at 17:49, David Gibson wrote:
> On Wed, Jan 08, 2003 at 09:48:48AM -0600, Hollis Blanchard wrote:
> > It's simply a wrapper around successive pte/pmd/pgd_offset calls, with
> > some error checking. Are you saying those calls are not the correct way
> > to get the PTE pointer for an address?
>
> Trouble is that this will break badly in 2.5 where we use large-page
> entries for the lowmem mapping.

I'm happy to change it to something 2.5-friendly if someone can suggest
how...

Alternatively I can add 405LP to the XMON/KGDB/BDI2000 list of ifdefs
that map the whole kernel writable, which would be overkill but avoid
adding PTE manipulations.

-Hollis
--
PowerPC Linux
IBM Linux Technology Center


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: get_pteptr prototype

[David Gibson]

On Wed, Jan 08, 2003 at 06:11:25PM -0600, Hollis Blanchard wrote:
>
> On Wed, 2003-01-08 at 17:49, David Gibson wrote:
> > On Wed, Jan 08, 2003 at 09:48:48AM -0600, Hollis Blanchard wrote:
> > > It's simply a wrapper around successive pte/pmd/pgd_offset calls, with
> > > some error checking. Are you saying those calls are not the correct way
> > > to get the PTE pointer for an address?
> >
> > Trouble is that this will break badly in 2.5 where we use large-page
> > entries for the lowmem mapping.
>
> I'm happy to change it to something 2.5-friendly if someone can suggest
> how...
>
> Alternatively I can add 405LP to the XMON/KGDB/BDI2000 list of ifdefs
> that map the whole kernel writable, which would be overkill but avoid
> adding PTE manipulations.

Hmm... what's the reason that wakeup_info needs to be reserved in
head_4xx.S, rather than just being a normal variable in the data area
(which should be writable anyway)?  Its not obvious to me from the
patch.

Actually, skimming through the patch I noticed a minor nit: you only
have one .long in head_4xx.S reserving space for the wakeup_info
struct which is 3 words long.  In practice the . = in the exception
handlers will give you plenty of space, but I think it would be good
form to explicitly reserve the right amount of space.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: get_pteptr prototype

[Hollis Blanchard]

On Wed, 2003-01-08 at 20:33, David Gibson wrote:
>
> Hmm... what's the reason that wakeup_info needs to be reserved in
> head_4xx.S, rather than just being a normal variable in the data area
> (which should be writable anyway)?  Its not obvious to me from the
> patch.

Sorry, I was hoping the Documentation file explained it well enough. On
wake the firmware needs to transfer control back to Linux, so the
firmware needs to know where to jump to. The only way I could think of
communicating that information was with a fixed memory location known to
both the firmware and to Linux. In future processors there will be a
scratch register (whose contents are saved during sleep) to solve this
problem.

> Actually, skimming through the patch I noticed a minor nit: you only
> have one .long in head_4xx.S reserving space for the wakeup_info
> struct which is 3 words long.  In practice the . = in the exception
> handlers will give you plenty of space, but I think it would be good
> form to explicitly reserve the right amount of space.

It's just a (physical) pointer to the structure which is elsewhere in
memory (actually on the stack).

-Hollis
--
PowerPC Linux
IBM Linux Technology Center


** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: get_pteptr prototype

[David Gibson]

On Thu, Jan 09, 2003 at 09:55:40AM -0600, Hollis Blanchard wrote:
>
> On Wed, 2003-01-08 at 20:33, David Gibson wrote:
> >
> > Hmm... what's the reason that wakeup_info needs to be reserved in
> > head_4xx.S, rather than just being a normal variable in the data area
> > (which should be writable anyway)?  Its not obvious to me from the
> > patch.
>
> Sorry, I was hoping the Documentation file explained it well enough. On
> wake the firmware needs to transfer control back to Linux, so the
> firmware needs to know where to jump to. The only way I could think of
> communicating that information was with a fixed memory location known to
> both the firmware and to Linux. In future processors there will be a
> scratch register (whose contents are saved during sleep) to solve this
> problem.

Ah, ok - I only skimmed the patch I'm afraid, so I didn't notice that.

> > Actually, skimming through the patch I noticed a minor nit: you only
> > have one .long in head_4xx.S reserving space for the wakeup_info
> > struct which is 3 words long.  In practice the . = in the exception
> > handlers will give you plenty of space, but I think it would be good
> > form to explicitly reserve the right amount of space.
>
> It's just a (physical) pointer to the structure which is elsewhere in
> memory (actually on the stack).

Hmm... in that case, why does the pointer need to be updated at
runtime: could you just statically allocate the real structure (in the
data segment), and have the value at 0xc00000fc filled in constant at
link time.

--
David Gibson			| For every complex problem there is a
david@gibson.dropbear.id.au	| solution which is simple, neat and
				| wrong.
http://www.ozlabs.org/people/dgibson

** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/

Re: 405LP sleep, no PTEs (was: get_pteptr prototype)

[Hollis Blanchard]

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

On Thu, 2003-01-09 at 04:22, Benjamin Herrenschmidt wrote:
> On Thu, 2003-01-09 at 01:11, Hollis Blanchard wrote:
> >
> > Alternatively I can add 405LP to the XMON/KGDB/BDI2000 list of ifdefs
> > that map the whole kernel writable, which would be overkill but avoid
> > adding PTE manipulations.
>
> A simpler solution would be to just disable MSR:DR while filling the
> wakeup infos.

You're right. The update is attached and at
http://penguinppc.org/~hollis/405LP-sleep.diff . The PTE manipulations
have been removed.

-Hollis
--
PowerPC Linux
IBM Linux Technology Center

[-- Attachment #2: 405LP-sleep.diff.gz --]
[-- Type: application/x-gzip, Size: 13438 bytes --]