Linux MIPS Architecture development
 help / color / mirror / Atom feed
* [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
@ 2006-10-06 13:36 Franck Bui-Huu
  2006-10-06 17:21 ` Thiemo Seufer
  0 siblings, 1 reply; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-06 13:36 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: linux-mips

From: Franck Bui-Huu <fbuihuu@gmail.com>

This patch introduces __pa_symbol() macro which should be used to
calculate the physical address of kernel symbols. We should fix any
linker issues in this macro, if any, but more importantly
__pa_symbol() uses __pa() to do the real conversion.

One resulting thing is that we can see that most of CPHYSADDR() uses
weren't needed.

It also rely on RELOC_HIDE() to avoid any compiler's oddities when
doing arithmetics on symbols.

Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
---
 arch/mips/kernel/setup.c |   17 ++++++++++-------
 include/asm-mips/page.h  |    1 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
index fdbb508..cccccd5 100644
--- a/arch/mips/kernel/setup.c
+++ b/arch/mips/kernel/setup.c
@@ -204,12 +204,12 @@ static void __init finalize_initrd(void)
 		printk(KERN_INFO "Initrd not found or empty");
 		goto disable;
 	}
-	if (CPHYSADDR(initrd_end) > PFN_PHYS(max_low_pfn)) {
+	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
 		printk("Initrd extends beyond end of memory");
 		goto disable;
 	}
 
-	reserve_bootmem(CPHYSADDR(initrd_start), size);
+	reserve_bootmem(__pa(initrd_start), size);
 	initrd_below_start_ok = 1;
 
 	printk(KERN_INFO "Initial ramdisk at: 0x%lx (%lu bytes)\n",
@@ -256,7 +256,10 @@ static void __init bootmem_init(void)
 	 * of usable memory.
 	 */
 	reserved_end = init_initrd();
-	reserved_end = PFN_UP(CPHYSADDR(max(reserved_end, (unsigned long)&_end)));
+	if (reserved_end > (unsigned long)&_end)
+		reserved_end = PFN_UP(__pa(reserved_end));
+	else
+		reserved_end = PFN_UP(__pa_symbol(&_end));
 
 	/*
 	 * Find the highest page frame number we have available.
@@ -428,10 +431,10 @@ static void __init resource_init(void)
 	if (UNCAC_BASE != IO_BASE)
 		return;
 
-	code_resource.start = virt_to_phys(&_text);
-	code_resource.end = virt_to_phys(&_etext) - 1;
-	data_resource.start = virt_to_phys(&_etext);
-	data_resource.end = virt_to_phys(&_edata) - 1;
+	code_resource.start = __pa_symbol(&_text);
+	code_resource.end = __pa_symbol(&_etext) - 1;
+	data_resource.start = __pa_symbol(&_etext);
+	data_resource.end = __pa_symbol(&_edata) - 1;
 
 	/*
 	 * Request address space for all standard RAM.
diff --git a/include/asm-mips/page.h b/include/asm-mips/page.h
index 32e5625..1d5f4a3 100644
--- a/include/asm-mips/page.h
+++ b/include/asm-mips/page.h
@@ -132,6 +132,7 @@ #endif /* !__ASSEMBLY__ */
 #define PAGE_ALIGN(addr)	(((addr) + PAGE_SIZE - 1) & PAGE_MASK)
 
 #define __pa(x)			((unsigned long) (x) - PAGE_OFFSET)
+#define __pa_symbol(x)		__pa(RELOC_HIDE((unsigned long)(x),0))
 #define __va(x)			((void *)((unsigned long) (x) + PAGE_OFFSET))
 
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
-- 
1.4.2.3

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-06 13:36 [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR() Franck Bui-Huu
@ 2006-10-06 17:21 ` Thiemo Seufer
  2006-10-09 11:58   ` Franck Bui-Huu
  0 siblings, 1 reply; 19+ messages in thread
From: Thiemo Seufer @ 2006-10-06 17:21 UTC (permalink / raw)
  To: Franck Bui-Huu; +Cc: Ralf Baechle, linux-mips

Franck Bui-Huu wrote:
> From: Franck Bui-Huu <fbuihuu@gmail.com>
> 
> This patch introduces __pa_symbol() macro which should be used to
> calculate the physical address of kernel symbols. We should fix any
> linker issues in this macro, if any, but more importantly
> __pa_symbol() uses __pa() to do the real conversion.
> 
> One resulting thing is that we can see that most of CPHYSADDR() uses
> weren't needed.
> 
> It also rely on RELOC_HIDE() to avoid any compiler's oddities when
> doing arithmetics on symbols.
> 
> Signed-off-by: Franck Bui-Huu <fbuihuu@gmail.com>
> ---
>  arch/mips/kernel/setup.c |   17 ++++++++++-------
>  include/asm-mips/page.h  |    1 +
>  2 files changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> index fdbb508..cccccd5 100644
> --- a/arch/mips/kernel/setup.c
> +++ b/arch/mips/kernel/setup.c
> @@ -204,12 +204,12 @@ static void __init finalize_initrd(void)
>  		printk(KERN_INFO "Initrd not found or empty");
>  		goto disable;
>  	}
> -	if (CPHYSADDR(initrd_end) > PFN_PHYS(max_low_pfn)) {
> +	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {

ISTR this failed on O2, where kernel+initrd are loaded into KSEG0 but the
PAGE_OFFSET is for XKPHYS.


Thiemo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-06 17:21 ` Thiemo Seufer
@ 2006-10-09 11:58   ` Franck Bui-Huu
  2006-10-09 13:21     ` Thiemo Seufer
  0 siblings, 1 reply; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-09 11:58 UTC (permalink / raw)
  To: Thiemo Seufer; +Cc: Franck Bui-Huu, Ralf Baechle, linux-mips

Thiemo Seufer wrote:
> Franck Bui-Huu wrote:
>>
>> -	if (CPHYSADDR(initrd_end) > PFN_PHYS(max_low_pfn)) {
>> +	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
> 
> ISTR this failed on O2, where kernel+initrd are loaded into KSEG0 but the
> PAGE_OFFSET is for XKPHYS.
> 

I guess that you were meaning somthing like:

LOADADDR    = 0xffffffff80004000
PAGE_OFFSET = 0xa800000000000000

is that correct ? If so could you explain the choice of these values
because I fail to understand it.

Thanks
		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 11:58   ` Franck Bui-Huu
@ 2006-10-09 13:21     ` Thiemo Seufer
  2006-10-09 14:25       ` Franck Bui-Huu
  0 siblings, 1 reply; 19+ messages in thread
From: Thiemo Seufer @ 2006-10-09 13:21 UTC (permalink / raw)
  To: Franck Bui-Huu; +Cc: Ralf Baechle, linux-mips

Franck Bui-Huu wrote:
> Thiemo Seufer wrote:
> > Franck Bui-Huu wrote:
> >>
> >> -	if (CPHYSADDR(initrd_end) > PFN_PHYS(max_low_pfn)) {
> >> +	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
> > 
> > ISTR this failed on O2, where kernel+initrd are loaded into KSEG0 but the
> > PAGE_OFFSET is for XKPHYS.
> > 
> 
> I guess that you were meaning somthing like:
> 
> LOADADDR    = 0xffffffff80004000
> PAGE_OFFSET = 0xa800000000000000
> 
> is that correct ?

Yes.

> If so could you explain the choice of these values
> because I fail to understand it.

It allows to load a 64-bit kernel in KSEG0, and use short 2-instruction
symbol references there. At the same time, it allows access to more
address space for memory and I/O than would fit in KSEG0.


Thiemo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 13:21     ` Thiemo Seufer
@ 2006-10-09 14:25       ` Franck Bui-Huu
  2006-10-09 14:58         ` Thiemo Seufer
  0 siblings, 1 reply; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-09 14:25 UTC (permalink / raw)
  To: Thiemo Seufer; +Cc: Franck Bui-Huu, Ralf Baechle, linux-mips

Thiemo Seufer wrote:
> Franck Bui-Huu wrote:
>> Thiemo Seufer wrote:
>>> Franck Bui-Huu wrote:
>>>> -	if (CPHYSADDR(initrd_end) > PFN_PHYS(max_low_pfn)) {
>>>> +	if (__pa(initrd_end) > PFN_PHYS(max_low_pfn)) {
>>> ISTR this failed on O2, where kernel+initrd are loaded into KSEG0 but the
>>> PAGE_OFFSET is for XKPHYS.
>>>
>> I guess that you were meaning somthing like:
>>
>> LOADADDR    = 0xffffffff80004000
>> PAGE_OFFSET = 0xa800000000000000
>>
>> is that correct ?
> 
> Yes.
> 
>> If so could you explain the choice of these values
>> because I fail to understand it.
> 
> It allows to load a 64-bit kernel in KSEG0,

sorry to be ignorant of 64 bit kernels, but what's the point
to load them in KSEG0.

> and use short 2-instruction symbol references there.

do you mean "it allows to use only 2 'lui' instructions to load
a symbol address into a register" ?

Futhermore I don't see how some part of the kernel convert virtual
address into a physical one with such values. For example in setup.c,
the function resource_init() does:

	code_resource.start = virt_to_phys(&_text);
	code_resource.end = virt_to_phys(&_etext) - 1;
	data_resource.start = virt_to_phys(&_etext);
	data_resource.end = virt_to_phys(&_edata) - 1;

How does it work in this case ?

Thanks
		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 14:25       ` Franck Bui-Huu
@ 2006-10-09 14:58         ` Thiemo Seufer
  2006-10-09 15:30           ` Franck Bui-Huu
  2006-10-09 15:51           ` Atsushi Nemoto
  0 siblings, 2 replies; 19+ messages in thread
From: Thiemo Seufer @ 2006-10-09 14:58 UTC (permalink / raw)
  To: Franck Bui-Huu; +Cc: Ralf Baechle, linux-mips

Franck Bui-Huu wrote:
[snip]
> >> If so could you explain the choice of these values
> >> because I fail to understand it.
> > 
> > It allows to load a 64-bit kernel in KSEG0,
> 
> sorry to be ignorant of 64 bit kernels, but what's the point
> to load them in KSEG0.

Smaller code with better performance.

> > and use short 2-instruction symbol references there.
> 
> do you mean "it allows to use only 2 'lui' instructions to load
> a symbol address into a register" ?

It allows a 2-instruction "lui ; addiu" sequence instead of a
6-instruction "lui ; lui ; addiu ; addiu ; dsll32 ; addu" sequence.

> Futhermore I don't see how some part of the kernel convert virtual
> address into a physical one with such values. For example in setup.c,
> the function resource_init() does:
> 
> 	code_resource.start = virt_to_phys(&_text);
> 	code_resource.end = virt_to_phys(&_etext) - 1;
> 	data_resource.start = virt_to_phys(&_etext);
> 	data_resource.end = virt_to_phys(&_edata) - 1;
> 
> How does it work in this case ?

Those are addresses in 64-bit space, no special handling is needed
there.

The same doesn't hold for the initrd addresses supplied by the (32-bit)
firmware. The firmware doesn't convert the kernel parameters to 64-bit
values because the O2 kernel used to allow a pure 32-bit build, and the
firmware can't find out what's actually inside the object file.


Thiemo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 14:58         ` Thiemo Seufer
@ 2006-10-09 15:30           ` Franck Bui-Huu
  2006-10-09 15:51           ` Atsushi Nemoto
  1 sibling, 0 replies; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-09 15:30 UTC (permalink / raw)
  To: Thiemo Seufer; +Cc: Franck Bui-Huu, Ralf Baechle, linux-mips

Thiemo Seufer wrote:
> Franck Bui-Huu wrote:
>> sorry to be ignorant of 64 bit kernels, but what's the point
>> to load them in KSEG0.
> 
> Smaller code with better performance.
> 

you mean we get smaller code _only_ by using the short 2 instructions
you described below ?

>>> and use short 2-instruction symbol references there.
>> do you mean "it allows to use only 2 'lui' instructions to load
>> a symbol address into a register" ?
> 
> It allows a 2-instruction "lui ; addiu" sequence instead of a
> 6-instruction "lui ; lui ; addiu ; addiu ; dsll32 ; addu" sequence.
> 
[snip]
>>
>> 	code_resource.start = virt_to_phys(&_text);
>> 	code_resource.end = virt_to_phys(&_etext) - 1;
>> 	data_resource.start = virt_to_phys(&_etext);
>> 	data_resource.end = virt_to_phys(&_edata) - 1;
>>
>> How does it work in this case ?
> 
> Those are addresses in 64-bit space, no special handling is needed
> there.

hm I'missing something there. Let's say that '&_text' is in KSEG0 and
is equal to 0xffffffff80000000. In this case virt_to_phys() returns
0x57ffffff80000000 (with PAGE_OFFSET = 0xa800000000000000). Is this 
physical address correct ??

> 
> The same doesn't hold for the initrd addresses supplied by the (32-bit)
> firmware. The firmware doesn't convert the kernel parameters to 64-bit
> values because the O2 kernel used to allow a pure 32-bit build, and the
> firmware can't find out what's actually inside the object file.
> 

This should be already handled by this code taken from setup.c:

static int __init rd_start_early(char *p)
{
        unsigned long start = memparse(p, &p);

#ifdef CONFIG_64BIT
        /* HACK: Guess if the sign extension was forgotten */
        if (start > 0x0000000080000000 && start < 0x00000000ffffffff)
                start |= 0xffffffff00000000UL;
#endif
        initrd_start = start;
        initrd_end += start;

        return 0;
}

Thanks
		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 14:58         ` Thiemo Seufer
  2006-10-09 15:30           ` Franck Bui-Huu
@ 2006-10-09 15:51           ` Atsushi Nemoto
  2006-10-09 16:59             ` Thiemo Seufer
  1 sibling, 1 reply; 19+ messages in thread
From: Atsushi Nemoto @ 2006-10-09 15:51 UTC (permalink / raw)
  To: ths; +Cc: vagabon.xyz, ralf, linux-mips

On Mon, 9 Oct 2006 15:58:17 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > do you mean "it allows to use only 2 'lui' instructions to load
> > a symbol address into a register" ?
> 
> It allows a 2-instruction "lui ; addiu" sequence instead of a
> 6-instruction "lui ; lui ; addiu ; addiu ; dsll32 ; addu" sequence.

Just for clarification: IIRC this optimization needs somewhat
up-to-date binutils/gcc and is not enabled on current lmo kernel,
right?

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 15:51           ` Atsushi Nemoto
@ 2006-10-09 16:59             ` Thiemo Seufer
  2006-10-10  8:49               ` Atsushi Nemoto
  0 siblings, 1 reply; 19+ messages in thread
From: Thiemo Seufer @ 2006-10-09 16:59 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: vagabon.xyz, ralf, linux-mips

Atsushi Nemoto wrote:
> On Mon, 9 Oct 2006 15:58:17 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > > do you mean "it allows to use only 2 'lui' instructions to load
> > > a symbol address into a register" ?
> > 
> > It allows a 2-instruction "lui ; addiu" sequence instead of a
> > 6-instruction "lui ; lui ; addiu ; addiu ; dsll32 ; addu" sequence.
> 
> Just for clarification: IIRC this optimization needs somewhat
> up-to-date binutils/gcc and is not enabled on current lmo kernel,
> right?

For old toolchains there used to be a gruesome hack (which AFAIR broke
at some point), for modern toolchains there's -msym32.


Thiemo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-09 16:59             ` Thiemo Seufer
@ 2006-10-10  8:49               ` Atsushi Nemoto
  2006-10-10 13:49                 ` Franck Bui-Huu
  0 siblings, 1 reply; 19+ messages in thread
From: Atsushi Nemoto @ 2006-10-10  8:49 UTC (permalink / raw)
  To: ths; +Cc: vagabon.xyz, ralf, linux-mips

On Mon, 9 Oct 2006 17:59:20 +0100, Thiemo Seufer <ths@networkno.de> wrote:
> > Just for clarification: IIRC this optimization needs somewhat
> > up-to-date binutils/gcc and is not enabled on current lmo kernel,
> > right?
> 
> For old toolchains there used to be a gruesome hack (which AFAIR broke
> at some point), for modern toolchains there's -msym32.

Hmm, I found that the -msym32 is enabled if BUILD_ELF64 was not
selected, since 2.6.17.  But does CONFIG_BUILD_ELF64=n really work for
modules?  While MAP_BASE is 0xc000000000000000 for most 64-bit
platforms, I suppose modules should not be compiled with -msym32.

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10  8:49               ` Atsushi Nemoto
@ 2006-10-10 13:49                 ` Franck Bui-Huu
  2006-10-10 14:19                   ` Atsushi Nemoto
  0 siblings, 1 reply; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-10 13:49 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: ths, vagabon.xyz, ralf, linux-mips

Atsushi Nemoto wrote:
> On Mon, 9 Oct 2006 17:59:20 +0100, Thiemo Seufer <ths@networkno.de> wrote:
>>> Just for clarification: IIRC this optimization needs somewhat
>>> up-to-date binutils/gcc and is not enabled on current lmo kernel,
>>> right?
>> For old toolchains there used to be a gruesome hack (which AFAIR broke
>> at some point), for modern toolchains there's -msym32.
> 
> Hmm, I found that the -msym32 is enabled if BUILD_ELF64 was not
> selected, since 2.6.17.  But does CONFIG_BUILD_ELF64=n really work for
> modules?  While MAP_BASE is 0xc000000000000000 for most 64-bit
> platforms, I suppose modules should not be compiled with -msym32.
> 

heh ? I'm wondering if anybody is using 'CONFIG_BUILD_ELF64=n' config at
all...

Atsushi, do you have any idea on how address are translated with
'CONFIG_BUILD_ELF64=n' config ? How such code is supposed to work ?

	code_resource.start = virt_to_phys(&_text);
 	code_resource.end = virt_to_phys(&_etext) - 1;
	data_resource.start = virt_to_phys(&_etext);
 	data_resource.end = virt_to_phys(&_edata) - 1;

Let's say that '&_text' is in KSEG0 and is equal to 0xffffffff80000000.
In this case virt_to_phys() returns 0x57ffffff80000000
(with PAGE_OFFSET = 0xa800000000000000). Is this physical address
correct ?

Thanks
		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 13:49                 ` Franck Bui-Huu
@ 2006-10-10 14:19                   ` Atsushi Nemoto
  2006-10-10 15:01                     ` Franck Bui-Huu
  0 siblings, 1 reply; 19+ messages in thread
From: Atsushi Nemoto @ 2006-10-10 14:19 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ths, ralf, linux-mips

On Tue, 10 Oct 2006 15:49:27 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> heh ? I'm wondering if anybody is using 'CONFIG_BUILD_ELF64=n' config at
> all...

arch/mips/configs/bigsur_defconfig:# CONFIG_BUILD_ELF64 is not set
arch/mips/configs/ip27_defconfig:# CONFIG_BUILD_ELF64 is not set
arch/mips/configs/ip32_defconfig:# CONFIG_BUILD_ELF64 is not set
arch/mips/configs/ocelot_c_defconfig:# CONFIG_BUILD_ELF64 is not set
arch/mips/configs/ocelot_g_defconfig:# CONFIG_BUILD_ELF64 is not set
arch/mips/configs/sb1250-swarm_defconfig:# CONFIG_BUILD_ELF64 is not set

According to arch/mips/configs, nobody is using CONFIG_BUILD_ELF64=y :-)

Also one might use gcc 3.x which ignore -msym32 option ...

> Atsushi, do you have any idea on how address are translated with
> 'CONFIG_BUILD_ELF64=n' config ? How such code is supposed to work ?
> 
> 	code_resource.start = virt_to_phys(&_text);
>  	code_resource.end = virt_to_phys(&_etext) - 1;
> 	data_resource.start = virt_to_phys(&_etext);
>  	data_resource.end = virt_to_phys(&_edata) - 1;
> 
> Let's say that '&_text' is in KSEG0 and is equal to 0xffffffff80000000.
> In this case virt_to_phys() returns 0x57ffffff80000000
> (with PAGE_OFFSET = 0xa800000000000000). Is this physical address
> correct ?

I think this peice of code is just broken, as you said.  This is bogus
but harmless since we have not checked these resources are
successfully registered or not.

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 14:19                   ` Atsushi Nemoto
@ 2006-10-10 15:01                     ` Franck Bui-Huu
  2006-10-10 15:29                       ` Atsushi Nemoto
  0 siblings, 1 reply; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-10 15:01 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: vagabon.xyz, ths, ralf, linux-mips

Atsushi Nemoto wrote:
> On Tue, 10 Oct 2006 15:49:27 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>> heh ? I'm wondering if anybody is using 'CONFIG_BUILD_ELF64=n' config at
>> all...
> 
> arch/mips/configs/bigsur_defconfig:# CONFIG_BUILD_ELF64 is not set
> arch/mips/configs/ip27_defconfig:# CONFIG_BUILD_ELF64 is not set
> arch/mips/configs/ip32_defconfig:# CONFIG_BUILD_ELF64 is not set
> arch/mips/configs/ocelot_c_defconfig:# CONFIG_BUILD_ELF64 is not set
> arch/mips/configs/ocelot_g_defconfig:# CONFIG_BUILD_ELF64 is not set
> arch/mips/configs/sb1250-swarm_defconfig:# CONFIG_BUILD_ELF64 is not set
> 
> According to arch/mips/configs, nobody is using CONFIG_BUILD_ELF64=y :-)
> 
> Also one might use gcc 3.x which ignore -msym32 option ...
> 

yeah, that's probably the reason...

>> Atsushi, do you have any idea on how address are translated with
>> 'CONFIG_BUILD_ELF64=n' config ? How such code is supposed to work ?
>>
>> 	code_resource.start = virt_to_phys(&_text);
>>  	code_resource.end = virt_to_phys(&_etext) - 1;
>> 	data_resource.start = virt_to_phys(&_etext);
>>  	data_resource.end = virt_to_phys(&_edata) - 1;
>>
>> Let's say that '&_text' is in KSEG0 and is equal to 0xffffffff80000000.
>> In this case virt_to_phys() returns 0x57ffffff80000000
>> (with PAGE_OFFSET = 0xa800000000000000). Is this physical address
>> correct ?
> 
> I think this peice of code is just broken, as you said.  This is bogus
> but harmless since we have not checked these resources are
> successfully registered or not.
> 

what about all other uses of virt_to_phys(x) ? And what the point to set
PAGE_OFFSET to 0xa800000000000000 ? I'm really confused...

		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 15:01                     ` Franck Bui-Huu
@ 2006-10-10 15:29                       ` Atsushi Nemoto
  2006-10-10 16:04                         ` Franck Bui-Huu
  0 siblings, 1 reply; 19+ messages in thread
From: Atsushi Nemoto @ 2006-10-10 15:29 UTC (permalink / raw)
  To: vagabon.xyz; +Cc: ths, ralf, linux-mips

On Tue, 10 Oct 2006 17:01:53 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
> > I think this peice of code is just broken, as you said.  This is bogus
> > but harmless since we have not checked these resources are
> > successfully registered or not.
> 
> what about all other uses of virt_to_phys(x) ? And what the point to set
> PAGE_OFFSET to 0xa800000000000000 ? I'm really confused...

For now I have not seen any problem on other usages.

We can use large flat mapping space in XKPHYS.  No TLB conversion, no
highmem trick.

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 15:29                       ` Atsushi Nemoto
@ 2006-10-10 16:04                         ` Franck Bui-Huu
  2006-10-10 16:16                           ` Franck Bui-Huu
  2006-10-10 21:51                           ` Ralf Baechle
  0 siblings, 2 replies; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-10 16:04 UTC (permalink / raw)
  To: Atsushi Nemoto; +Cc: vagabon.xyz, ths, ralf, linux-mips

Atsushi Nemoto wrote:
> On Tue, 10 Oct 2006 17:01:53 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>>> I think this peice of code is just broken, as you said.  This is bogus
>>> but harmless since we have not checked these resources are
>>> successfully registered or not.
>> what about all other uses of virt_to_phys(x) ? And what the point to set
>> PAGE_OFFSET to 0xa800000000000000 ? I'm really confused...
> 
> For now I have not seen any problem on other usages.
> 

do you know at which point the kernel starts to use XKPHYS addresses ?

> We can use large flat mapping space in XKPHYS.  No TLB conversion, no
> highmem trick.
> 

ok, and does the trick on KSEG0/XKPHYS really worth ? I mean what is
the size code gain ?

Thanks
		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 16:04                         ` Franck Bui-Huu
@ 2006-10-10 16:16                           ` Franck Bui-Huu
  2006-10-10 21:51                           ` Ralf Baechle
  1 sibling, 0 replies; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-10 16:16 UTC (permalink / raw)
  To: Franck; +Cc: Atsushi Nemoto, ths, ralf, linux-mips

Franck Bui-Huu wrote:
> Atsushi Nemoto wrote:
>> On Tue, 10 Oct 2006 17:01:53 +0200, Franck Bui-Huu <vagabon.xyz@gmail.com> wrote:
>>>> I think this peice of code is just broken, as you said.  This is bogus
>>>> but harmless since we have not checked these resources are
>>>> successfully registered or not.
>>> what about all other uses of virt_to_phys(x) ? And what the point to set
>>> PAGE_OFFSET to 0xa800000000000000 ? I'm really confused...
>> For now I have not seen any problem on other usages.
>>
> 
> do you know at which point the kernel starts to use XKPHYS addresses ?
> 

Ok, I found the answer to this question...

		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 16:04                         ` Franck Bui-Huu
  2006-10-10 16:16                           ` Franck Bui-Huu
@ 2006-10-10 21:51                           ` Ralf Baechle
  2006-10-11  3:11                             ` Atsushi Nemoto
  2006-10-11  9:37                             ` Franck Bui-Huu
  1 sibling, 2 replies; 19+ messages in thread
From: Ralf Baechle @ 2006-10-10 21:51 UTC (permalink / raw)
  To: Franck Bui-Huu; +Cc: Atsushi Nemoto, ths, linux-mips

On Tue, Oct 10, 2006 at 06:04:53PM +0200, Franck Bui-Huu wrote:

> ok, and does the trick on KSEG0/XKPHYS really worth ? I mean what is
> the size code gain ?

Gcc / gas generate a 6 instruction sequence to load something from a
64-bit address, basically lui, add, dsll16, add, dsll16, add.  It's
just 2 instructions for 32-bit addresses.  This boils down to space
savings in the hundred of kilobytes for a kernel.

Of course there are more intelligent way to load an address via global
pointer optimization but that's the two choices we got today.

  Ralf

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 21:51                           ` Ralf Baechle
@ 2006-10-11  3:11                             ` Atsushi Nemoto
  2006-10-11  9:37                             ` Franck Bui-Huu
  1 sibling, 0 replies; 19+ messages in thread
From: Atsushi Nemoto @ 2006-10-11  3:11 UTC (permalink / raw)
  To: ralf; +Cc: vagabon.xyz, ths, linux-mips

On Tue, 10 Oct 2006 22:51:24 +0100, Ralf Baechle <ralf@linux-mips.org> wrote:
> > ok, and does the trick on KSEG0/XKPHYS really worth ? I mean what is
> > the size code gain ?
> 
> Gcc / gas generate a 6 instruction sequence to load something from a
> 64-bit address, basically lui, add, dsll16, add, dsll16, add.  It's
> just 2 instructions for 32-bit addresses.  This boils down to space
> savings in the hundred of kilobytes for a kernel.

Yes, I got ~10% smaller kernel (4.8MB to 4.4MB) with -msym32.

If modules are loaded into CKSEG2, we can use -msym32 for modules as
well.  Until then this patch can be used for workaroung:

http://www.linux-mips.org/archives/linux-mips/2006-10/msg00111.html

---
Atsushi Nemoto

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR()
  2006-10-10 21:51                           ` Ralf Baechle
  2006-10-11  3:11                             ` Atsushi Nemoto
@ 2006-10-11  9:37                             ` Franck Bui-Huu
  1 sibling, 0 replies; 19+ messages in thread
From: Franck Bui-Huu @ 2006-10-11  9:37 UTC (permalink / raw)
  To: Ralf Baechle; +Cc: Franck Bui-Huu, Atsushi Nemoto, ths, linux-mips

Ralf Baechle wrote:
> On Tue, Oct 10, 2006 at 06:04:53PM +0200, Franck Bui-Huu wrote:
> 
>> ok, and does the trick on KSEG0/XKPHYS really worth ? I mean what is
>> the size code gain ?
> 
> Gcc / gas generate a 6 instruction sequence to load something from a
> 64-bit address, basically lui, add, dsll16, add, dsll16, add.  It's
> just 2 instructions for 32-bit addresses.  This boils down to space
> savings in the hundred of kilobytes for a kernel.
> 

ok, the hack seems to worth it...

		Franck

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2006-10-11  9:37 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-10-06 13:36 [PATCH] setup.c: introduce __pa_symbol() and get ride of CPHYSADDR() Franck Bui-Huu
2006-10-06 17:21 ` Thiemo Seufer
2006-10-09 11:58   ` Franck Bui-Huu
2006-10-09 13:21     ` Thiemo Seufer
2006-10-09 14:25       ` Franck Bui-Huu
2006-10-09 14:58         ` Thiemo Seufer
2006-10-09 15:30           ` Franck Bui-Huu
2006-10-09 15:51           ` Atsushi Nemoto
2006-10-09 16:59             ` Thiemo Seufer
2006-10-10  8:49               ` Atsushi Nemoto
2006-10-10 13:49                 ` Franck Bui-Huu
2006-10-10 14:19                   ` Atsushi Nemoto
2006-10-10 15:01                     ` Franck Bui-Huu
2006-10-10 15:29                       ` Atsushi Nemoto
2006-10-10 16:04                         ` Franck Bui-Huu
2006-10-10 16:16                           ` Franck Bui-Huu
2006-10-10 21:51                           ` Ralf Baechle
2006-10-11  3:11                             ` Atsushi Nemoto
2006-10-11  9:37                             ` Franck Bui-Huu

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