* [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
@ 2010-02-02 10:37 Guenter Roeck
2010-02-02 13:01 ` Ralf Baechle
0 siblings, 1 reply; 8+ messages in thread
From: Guenter Roeck @ 2010-02-02 10:37 UTC (permalink / raw)
To: linux-mips; +Cc: Guenter Roeck
Linux kernel 2.6.32 and later allocates memory from the top of virtual memory
space.
This patch implements virtual memory size detection for 64 bit MIPS CPUs
to avoid resulting crashes.
Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
arch/mips/include/asm/cpu-features.h | 3 +++
arch/mips/include/asm/cpu-info.h | 1 +
arch/mips/include/asm/pgtable-64.h | 4 +++-
arch/mips/kernel/cpu-probe.c | 12 ++++++++++++
4 files changed, 19 insertions(+), 1 deletions(-)
diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
index 1f4df64..284eb55 100644
--- a/arch/mips/include/asm/cpu-features.h
+++ b/arch/mips/include/asm/cpu-features.h
@@ -209,6 +209,9 @@
# ifndef cpu_has_64bit_addresses
# define cpu_has_64bit_addresses 1
# endif
+# ifndef cpu_vmbits
+# define cpu_vmbits cpu_data[0].vmbits
+# endif
#endif
#if defined(CONFIG_CPU_MIPSR2_IRQ_VI) && !defined(cpu_has_vint)
diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
index 1260443..3c694bc 100644
--- a/arch/mips/include/asm/cpu-info.h
+++ b/arch/mips/include/asm/cpu-info.h
@@ -58,6 +58,7 @@ struct cpuinfo_mips {
struct cache_desc tcache; /* Tertiary/split secondary cache */
int srsets; /* Shadow register sets */
int core; /* physical core number */
+ int vmbits; /* Virtual memory size in bits */
#if defined(CONFIG_MIPS_MT_SMP) || defined(CONFIG_MIPS_MT_SMTC)
/*
* In the MIPS MT "SMTC" model, each TC is considered
diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h
index 9cd5089..8eda30b 100644
--- a/arch/mips/include/asm/pgtable-64.h
+++ b/arch/mips/include/asm/pgtable-64.h
@@ -110,7 +110,9 @@
#define VMALLOC_START MAP_BASE
#define VMALLOC_END \
(VMALLOC_START + \
- PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE - (1UL << 32))
+ min(PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE, \
+ (1UL << cpu_vmbits)) - (1UL << 32))
+
#if defined(CONFIG_MODULES) && defined(KBUILD_64BIT_SYM32) && \
VMALLOC_START != CKSSEG
/* Load modules into 32bit-compatible segment. */
diff --git a/arch/mips/kernel/cpu-probe.c b/arch/mips/kernel/cpu-probe.c
index 7a51866..21dc4b8 100644
--- a/arch/mips/kernel/cpu-probe.c
+++ b/arch/mips/kernel/cpu-probe.c
@@ -282,6 +282,16 @@ static inline int __cpu_has_fpu(void)
return ((cpu_get_fpu_id() & 0xff00) != FPIR_IMP_NONE);
}
+static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
+{
+ if (cpu_has_64bits) {
+ write_c0_entryhi(0x3ffffffffffff000ULL);
+ back_to_back_c0_hazard();
+ c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
+ } else
+ c->vmbits = 32;
+}
+
#define R4K_OPTS (MIPS_CPU_TLB | MIPS_CPU_4KEX | MIPS_CPU_4K_CACHE \
| MIPS_CPU_COUNTER)
@@ -967,6 +977,8 @@ __cpuinit void cpu_probe(void)
c->srsets = ((read_c0_srsctl() >> 26) & 0x0f) + 1;
else
c->srsets = 1;
+
+ cpu_set_vmbits(c);
}
__cpuinit void cpu_report(void)
--
1.6.0.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 10:37 [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs Guenter Roeck
@ 2010-02-02 13:01 ` Ralf Baechle
2010-02-02 13:05 ` Dmitri Vorobiev
2010-02-02 16:03 ` Guenter Roeck
0 siblings, 2 replies; 8+ messages in thread
From: Ralf Baechle @ 2010-02-02 13:01 UTC (permalink / raw)
To: Guenter Roeck; +Cc: linux-mips
On Tue, Feb 02, 2010 at 02:37:41AM -0800, Guenter Roeck wrote:
Pretty good, let the nitpicking start :-)
> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> index 1f4df64..284eb55 100644
> --- a/arch/mips/include/asm/cpu-features.h
> +++ b/arch/mips/include/asm/cpu-features.h
> @@ -209,6 +209,9 @@
> # ifndef cpu_has_64bit_addresses
> # define cpu_has_64bit_addresses 1
> # endif
> +# ifndef cpu_vmbits
> +# define cpu_vmbits cpu_data[0].vmbits
> +# endif
> #endif
For 32-bit kernels we probably should fix cpu_vmbits to 31.
#ifdef CONFIG_64BIT
# ifndef cpu_vmbits
# define cpu_vmbits cpu_data[0].vmbits
# define __NEED_VMBITS
# endif
#else
# #define cpu_vmbits 31
#endif
> #if defined(CONFIG_CPU_MIPSR2_IRQ_VI) && !defined(cpu_has_vint)
> diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
> index 1260443..3c694bc 100644
> --- a/arch/mips/include/asm/cpu-info.h
> +++ b/arch/mips/include/asm/cpu-info.h
> @@ -58,6 +58,7 @@ struct cpuinfo_mips {
> struct cache_desc tcache; /* Tertiary/split secondary cache */
> int srsets; /* Shadow register sets */
> int core; /* physical core number */
> + int vmbits; /* Virtual memory size in bits */
#ifdef __NEED_VMBITS
int vmbits; /* Virtual memory size in bits */
#endif
To do for a later separate patch - minimize the sizes of all cpuinfo_mips
members; int is overkill for many members.
> #if defined(CONFIG_MIPS_MT_SMP) || defined(CONFIG_MIPS_MT_SMTC)
> /*
> * In the MIPS MT "SMTC" model, each TC is considered
> diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h
> index 9cd5089..8eda30b 100644
> --- a/arch/mips/include/asm/pgtable-64.h
> +++ b/arch/mips/include/asm/pgtable-64.h
> @@ -110,7 +110,9 @@
> #define VMALLOC_START MAP_BASE
> #define VMALLOC_END \
> (VMALLOC_START + \
> - PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE - (1UL << 32))
> + min(PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE, \
> + (1UL << cpu_vmbits)) - (1UL << 32))
> +
This overlaps with other allocations near the top - yes, we've been living
dangerous but with bottom up allocations a conflict was very, very unlikely.
Dealing with them in a sane way in the VMALLOC_END macro definition is hard,
so I suggest changing the definition to something like:
extern unsigned long vmalloc_end;
#define VMALLOC_START MAP_BASE
#define VMALLOC_END vmalloc_end
And initialize the value of vmalloc_end in something like the mm section of
arch/mips/kernel/traps.c:per_cpu_trap_init().
There you can also ensure the value of vmalloc_end is <= FIXADDR_START.
Finally probing can be used to fix yet another sin. For 64-bit kernels
arch/mips/include/asm/processor.h defines TASK_SIZE:
#define TASK_SIZE 0x10000000000UL
This should turn into something like.
#define TASK_SIZE (1UL << vmbits)
This is also why I sugget to keep vmbits as 31, not 32. Hello S390 :-)
> +static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
> +{
> + if (cpu_has_64bits) {
> + write_c0_entryhi(0x3ffffffffffff000ULL);
> + back_to_back_c0_hazard();
> + c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
> + } else
> + c->vmbits = 32;
> +}
With my changes suggested above this would need changing to something like:
static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
{
if (__NEED_VMBITS) {
write_c0_entryhi(0x3ffffffffffff000ULL);
back_to_back_c0_hazard();
c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
return;
}
BUG();
}
> #define R4K_OPTS (MIPS_CPU_TLB | MIPS_CPU_4KEX | MIPS_CPU_4K_CACHE \
> | MIPS_CPU_COUNTER)
>
> @@ -967,6 +977,8 @@ __cpuinit void cpu_probe(void)
> c->srsets = ((read_c0_srsctl() >> 26) & 0x0f) + 1;
> else
> c->srsets = 1;
> +
> + cpu_set_vmbits(c);
I think a name like cpu_probe_vmbits() would be clearer.
Thanks,
Ralf
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 13:01 ` Ralf Baechle
@ 2010-02-02 13:05 ` Dmitri Vorobiev
2010-02-02 13:10 ` Ralf Baechle
2010-02-02 16:03 ` Guenter Roeck
1 sibling, 1 reply; 8+ messages in thread
From: Dmitri Vorobiev @ 2010-02-02 13:05 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Guenter Roeck, linux-mips
On Tue, Feb 2, 2010 at 3:01 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Tue, Feb 02, 2010 at 02:37:41AM -0800, Guenter Roeck wrote:
>
> Pretty good, let the nitpicking start :-)
I haven't been following this closely enough, so maybe my question is
stupid. However: will this work on SGI R5000? I'm using an Indy, so
I'm quite concerned about this CPU.
Thanks,
Dmitri
>
>> diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
>> index 1f4df64..284eb55 100644
>> --- a/arch/mips/include/asm/cpu-features.h
>> +++ b/arch/mips/include/asm/cpu-features.h
>> @@ -209,6 +209,9 @@
>> # ifndef cpu_has_64bit_addresses
>> # define cpu_has_64bit_addresses 1
>> # endif
>> +# ifndef cpu_vmbits
>> +# define cpu_vmbits cpu_data[0].vmbits
>> +# endif
>> #endif
>
> For 32-bit kernels we probably should fix cpu_vmbits to 31.
>
> #ifdef CONFIG_64BIT
> # ifndef cpu_vmbits
> # define cpu_vmbits cpu_data[0].vmbits
> # define __NEED_VMBITS
> # endif
> #else
> # #define cpu_vmbits 31
> #endif
>
>> #if defined(CONFIG_CPU_MIPSR2_IRQ_VI) && !defined(cpu_has_vint)
>> diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
>> index 1260443..3c694bc 100644
>> --- a/arch/mips/include/asm/cpu-info.h
>> +++ b/arch/mips/include/asm/cpu-info.h
>> @@ -58,6 +58,7 @@ struct cpuinfo_mips {
>> struct cache_desc tcache; /* Tertiary/split secondary cache */
>> int srsets; /* Shadow register sets */
>> int core; /* physical core number */
>> + int vmbits; /* Virtual memory size in bits */
>
> #ifdef __NEED_VMBITS
> int vmbits; /* Virtual memory size in bits */
> #endif
>
> To do for a later separate patch - minimize the sizes of all cpuinfo_mips
> members; int is overkill for many members.
>
>> #if defined(CONFIG_MIPS_MT_SMP) || defined(CONFIG_MIPS_MT_SMTC)
>> /*
>> * In the MIPS MT "SMTC" model, each TC is considered
>> diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h
>> index 9cd5089..8eda30b 100644
>> --- a/arch/mips/include/asm/pgtable-64.h
>> +++ b/arch/mips/include/asm/pgtable-64.h
>> @@ -110,7 +110,9 @@
>> #define VMALLOC_START MAP_BASE
>> #define VMALLOC_END \
>> (VMALLOC_START + \
>> - PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE - (1UL << 32))
>> + min(PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE, \
>> + (1UL << cpu_vmbits)) - (1UL << 32))
>> +
>
> This overlaps with other allocations near the top - yes, we've been living
> dangerous but with bottom up allocations a conflict was very, very unlikely.
>
> Dealing with them in a sane way in the VMALLOC_END macro definition is hard,
> so I suggest changing the definition to something like:
>
> extern unsigned long vmalloc_end;
>
> #define VMALLOC_START MAP_BASE
> #define VMALLOC_END vmalloc_end
>
> And initialize the value of vmalloc_end in something like the mm section of
> arch/mips/kernel/traps.c:per_cpu_trap_init().
>
> There you can also ensure the value of vmalloc_end is <= FIXADDR_START.
>
> Finally probing can be used to fix yet another sin. For 64-bit kernels
> arch/mips/include/asm/processor.h defines TASK_SIZE:
>
> #define TASK_SIZE 0x10000000000UL
>
> This should turn into something like.
>
> #define TASK_SIZE (1UL << vmbits)
>
> This is also why I sugget to keep vmbits as 31, not 32. Hello S390 :-)
>
>> +static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
>> +{
>> + if (cpu_has_64bits) {
>> + write_c0_entryhi(0x3ffffffffffff000ULL);
>> + back_to_back_c0_hazard();
>> + c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
>> + } else
>> + c->vmbits = 32;
>> +}
>
> With my changes suggested above this would need changing to something like:
>
> static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
> {
> if (__NEED_VMBITS) {
> write_c0_entryhi(0x3ffffffffffff000ULL);
> back_to_back_c0_hazard();
> c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
>
> return;
> }
>
> BUG();
> }
>
>> #define R4K_OPTS (MIPS_CPU_TLB | MIPS_CPU_4KEX | MIPS_CPU_4K_CACHE \
>> | MIPS_CPU_COUNTER)
>>
>> @@ -967,6 +977,8 @@ __cpuinit void cpu_probe(void)
>> c->srsets = ((read_c0_srsctl() >> 26) & 0x0f) + 1;
>> else
>> c->srsets = 1;
>> +
>> + cpu_set_vmbits(c);
>
> I think a name like cpu_probe_vmbits() would be clearer.
>
> Thanks,
>
> Ralf
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 13:05 ` Dmitri Vorobiev
@ 2010-02-02 13:10 ` Ralf Baechle
2010-02-02 13:34 ` Dmitri Vorobiev
0 siblings, 1 reply; 8+ messages in thread
From: Ralf Baechle @ 2010-02-02 13:10 UTC (permalink / raw)
To: Dmitri Vorobiev; +Cc: Guenter Roeck, linux-mips
On Tue, Feb 02, 2010 at 03:05:49PM +0200, Dmitri Vorobiev wrote:
> On Tue, Feb 2, 2010 at 3:01 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
> > On Tue, Feb 02, 2010 at 02:37:41AM -0800, Guenter Roeck wrote:
> >
> > Pretty good, let the nitpicking start :-)
>
> I haven't been following this closely enough, so maybe my question is
> stupid. However: will this work on SGI R5000? I'm using an Indy, so
> I'm quite concerned about this CPU.
The point is that your Indy right now is not working with a 64-bit kernel :)
Ralf
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 13:10 ` Ralf Baechle
@ 2010-02-02 13:34 ` Dmitri Vorobiev
2010-02-02 13:46 ` Ralf Baechle
0 siblings, 1 reply; 8+ messages in thread
From: Dmitri Vorobiev @ 2010-02-02 13:34 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Guenter Roeck, linux-mips
On Tue, Feb 2, 2010 at 3:10 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Tue, Feb 02, 2010 at 03:05:49PM +0200, Dmitri Vorobiev wrote:
>
>> On Tue, Feb 2, 2010 at 3:01 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
>> > On Tue, Feb 02, 2010 at 02:37:41AM -0800, Guenter Roeck wrote:
>> >
>> > Pretty good, let the nitpicking start :-)
>>
>> I haven't been following this closely enough, so maybe my question is
>> stupid. However: will this work on SGI R5000? I'm using an Indy, so
>> I'm quite concerned about this CPU.
>
> The point is that your Indy right now is not working with a 64-bit kernel :)
I know. :)
What I was asking about was whether my Indy is going to work with the
new scheme of calculating VMALLOC_END.
Dmitri
>
> Ralf
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 13:34 ` Dmitri Vorobiev
@ 2010-02-02 13:46 ` Ralf Baechle
2010-02-02 13:49 ` Dmitri Vorobiev
0 siblings, 1 reply; 8+ messages in thread
From: Ralf Baechle @ 2010-02-02 13:46 UTC (permalink / raw)
To: Dmitri Vorobiev; +Cc: Guenter Roeck, linux-mips
On Tue, Feb 02, 2010 at 03:34:31PM +0200, Dmitri Vorobiev wrote:
> >> I haven't been following this closely enough, so maybe my question is
> >> stupid. However: will this work on SGI R5000? I'm using an Indy, so
> >> I'm quite concerned about this CPU.
> >
> > The point is that your Indy right now is not working with a 64-bit kernel :)
>
> I know. :)
>
> What I was asking about was whether my Indy is going to work with the
> new scheme of calculating VMALLOC_END.
Of course it will.
Ralf
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 13:46 ` Ralf Baechle
@ 2010-02-02 13:49 ` Dmitri Vorobiev
0 siblings, 0 replies; 8+ messages in thread
From: Dmitri Vorobiev @ 2010-02-02 13:49 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Guenter Roeck, linux-mips
On Tue, Feb 2, 2010 at 3:46 PM, Ralf Baechle <ralf@linux-mips.org> wrote:
> On Tue, Feb 02, 2010 at 03:34:31PM +0200, Dmitri Vorobiev wrote:
>
>> >> I haven't been following this closely enough, so maybe my question is
>> >> stupid. However: will this work on SGI R5000? I'm using an Indy, so
>> >> I'm quite concerned about this CPU.
>> >
>> > The point is that your Indy right now is not working with a 64-bit kernel :)
>>
>> I know. :)
>>
>> What I was asking about was whether my Indy is going to work with the
>> new scheme of calculating VMALLOC_END.
>
> Of course it will.
OK, thanks!
Dmitri
>
> Ralf
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs
2010-02-02 13:01 ` Ralf Baechle
2010-02-02 13:05 ` Dmitri Vorobiev
@ 2010-02-02 16:03 ` Guenter Roeck
1 sibling, 0 replies; 8+ messages in thread
From: Guenter Roeck @ 2010-02-02 16:03 UTC (permalink / raw)
To: Ralf Baechle; +Cc: linux-mips@linux-mips.org
On Tue, Feb 02, 2010 at 08:01:53AM -0500, Ralf Baechle wrote:
> On Tue, Feb 02, 2010 at 02:37:41AM -0800, Guenter Roeck wrote:
>
> Pretty good, let the nitpicking start :-)
>
> > diff --git a/arch/mips/include/asm/cpu-features.h b/arch/mips/include/asm/cpu-features.h
> > index 1f4df64..284eb55 100644
> > --- a/arch/mips/include/asm/cpu-features.h
> > +++ b/arch/mips/include/asm/cpu-features.h
> > @@ -209,6 +209,9 @@
> > # ifndef cpu_has_64bit_addresses
> > # define cpu_has_64bit_addresses 1
> > # endif
> > +# ifndef cpu_vmbits
> > +# define cpu_vmbits cpu_data[0].vmbits
> > +# endif
> > #endif
>
> For 32-bit kernels we probably should fix cpu_vmbits to 31.
>
> #ifdef CONFIG_64BIT
> # ifndef cpu_vmbits
> # define cpu_vmbits cpu_data[0].vmbits
> # define __NEED_VMBITS
> # endif
> #else
> # #define cpu_vmbits 31
> #endif
>
Makes sense in theory. However, cpu-features.h includes cpu-info.h
which would need to have __NEED_VMBITS defined. So we would have a circular dependency.
To avoid that, I could use CONFIG_64BIT directly whenever vmbits is used or set.
vmbits would still be calculated but not used if cpu_vmbits is defined separately.
Another option would be to define the vmbits variable with #ifdef CONFIG_64BIT,
but populate and use it only if __NEED_VMBITS is defined. That would avoid the
unnecessary calculation.
Let me know what you prefer, and if you have a better idea. Right now I use
the second approach.
> > #if defined(CONFIG_CPU_MIPSR2_IRQ_VI) && !defined(cpu_has_vint)
> > diff --git a/arch/mips/include/asm/cpu-info.h b/arch/mips/include/asm/cpu-info.h
> > index 1260443..3c694bc 100644
> > --- a/arch/mips/include/asm/cpu-info.h
> > +++ b/arch/mips/include/asm/cpu-info.h
> > @@ -58,6 +58,7 @@ struct cpuinfo_mips {
> > struct cache_desc tcache; /* Tertiary/split secondary cache */
> > int srsets; /* Shadow register sets */
> > int core; /* physical core number */
> > + int vmbits; /* Virtual memory size in bits */
>
> #ifdef __NEED_VMBITS
> int vmbits; /* Virtual memory size in bits */
> #endif
>
> To do for a later separate patch - minimize the sizes of all cpuinfo_mips
> members; int is overkill for many members.
>
> > #if defined(CONFIG_MIPS_MT_SMP) || defined(CONFIG_MIPS_MT_SMTC)
> > /*
> > * In the MIPS MT "SMTC" model, each TC is considered
> > diff --git a/arch/mips/include/asm/pgtable-64.h b/arch/mips/include/asm/pgtable-64.h
> > index 9cd5089..8eda30b 100644
> > --- a/arch/mips/include/asm/pgtable-64.h
> > +++ b/arch/mips/include/asm/pgtable-64.h
> > @@ -110,7 +110,9 @@
> > #define VMALLOC_START MAP_BASE
> > #define VMALLOC_END \
> > (VMALLOC_START + \
> > - PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE - (1UL << 32))
> > + min(PTRS_PER_PGD * PTRS_PER_PMD * PTRS_PER_PTE * PAGE_SIZE, \
> > + (1UL << cpu_vmbits)) - (1UL << 32))
> > +
>
> This overlaps with other allocations near the top - yes, we've been living
> dangerous but with bottom up allocations a conflict was very, very unlikely.
>
> Dealing with them in a sane way in the VMALLOC_END macro definition is hard,
> so I suggest changing the definition to something like:
>
> extern unsigned long vmalloc_end;
>
> #define VMALLOC_START MAP_BASE
> #define VMALLOC_END vmalloc_end
>
> And initialize the value of vmalloc_end in something like the mm section of
> arch/mips/kernel/traps.c:per_cpu_trap_init().
>
> There you can also ensure the value of vmalloc_end is <= FIXADDR_START.
>
FIXADDR_START is defined as 0xff000000, 0xfffe0000, or (0xff000000 - 0x20000),
for all kernels. If vmalloc_end must be below FIXADDR_START, I might as well
forget about the probing and just use the 32 bit define for VMALLOC_END.
Am I missing something ?
> Finally probing can be used to fix yet another sin. For 64-bit kernels
> arch/mips/include/asm/processor.h defines TASK_SIZE:
>
> #define TASK_SIZE 0x10000000000UL
>
> This should turn into something like.
>
> #define TASK_SIZE (1UL << vmbits)
>
> This is also why I sugget to keep vmbits as 31, not 32. Hello S390 :-)
>
A comment at the TASK_SIZE definition says "This is hardcoded into a few places,
so don't change it unless you know what you are doing". Changing it to a variable
causes compilation to fail, so it looks as if there are some deeper complexities
involved.
I think it would be better to change TASK_SIZE in a separate commit, after making
sure that the change does not break anything. So far the patch is a bug fix;
this would make it into an enhancement with unknown side effects.
> > +static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
> > +{
> > + if (cpu_has_64bits) {
> > + write_c0_entryhi(0x3ffffffffffff000ULL);
> > + back_to_back_c0_hazard();
> > + c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
> > + } else
> > + c->vmbits = 32;
> > +}
>
> With my changes suggested above this would need changing to something like:
>
> static inline void cpu_set_vmbits(struct cpuinfo_mips *c)
> {
> if (__NEED_VMBITS) {
> write_c0_entryhi(0x3ffffffffffff000ULL);
> back_to_back_c0_hazard();
> c->vmbits = fls64(read_c0_entryhi() & 0x3ffffffffffff000ULL);
>
> return;
> }
>
> BUG();
> }
>
BUG() doesn't work because it would fire if __NEED_VMBITS is false.
Also, I would have to define __NEED_VMBITS as TRUE/FALSE if I want to use it
in an if statement. So I decided to use #ifdef __NEED_VMBITS instead.
> > #define R4K_OPTS (MIPS_CPU_TLB | MIPS_CPU_4KEX | MIPS_CPU_4K_CACHE \
> > | MIPS_CPU_COUNTER)
> >
> > @@ -967,6 +977,8 @@ __cpuinit void cpu_probe(void)
> > c->srsets = ((read_c0_srsctl() >> 26) & 0x0f) + 1;
> > else
> > c->srsets = 1;
> > +
> > + cpu_set_vmbits(c);
>
> I think a name like cpu_probe_vmbits() would be clearer.
>
Agreed.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2010-02-02 16:01 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-02 10:37 [PATCH v5] Virtual memory size detection for 64 bit MIPS CPUs Guenter Roeck
2010-02-02 13:01 ` Ralf Baechle
2010-02-02 13:05 ` Dmitri Vorobiev
2010-02-02 13:10 ` Ralf Baechle
2010-02-02 13:34 ` Dmitri Vorobiev
2010-02-02 13:46 ` Ralf Baechle
2010-02-02 13:49 ` Dmitri Vorobiev
2010-02-02 16:03 ` Guenter Roeck
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.