Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Takao Indoh]

On Thu, 7 Sep 2006 08:57:13 +0800, "Zou, Nanhai" wrote:

>> On Wed, 06 Sep 2006 18:06:53 +0900, Takao Indoh wrote:
>> 
>> >>>+		/* change region of gp to region5 */
>> >>>+		asm volatile ("movl gp=__gp"::: "memory");
>> >>
>> >>Why?  gp is already pointing at the kernel code before we enter
>> >>ia64_init_handler().
>> >
>> >The purpose of this code is just changing region of gp from region7 to
>> >region5. For example, if gp is e000000004ace170, I want to change gp to
>>                                 ~~~~~~~~~~~~~~~~e000000000ace170
>> Sorry, I mistook address...
>> 
>> 
>> Regards,
>> Takao Indoh
>> 
>  You don't need to set gp here. Control page does not use GP, and 
>purgatory code will set GP to a physical address which was pre-calculated 
>at the time of crash kernel loading.

If gp points region7, panic occurs at kdump_disable_iosapic.

kdump_disable_iosapic(void)
{
	u32 low32;
	struct iosapic_intr_info *info;
	struct iosapic_rte_info *rte;
	for (info = iosapic_intr_info; info <
			iosapic_intr_info + IA64_NUM_VECTORS; ++info) {
		low32 = info->low32 |= IOSAPIC_MASK;
		list_for_each_entry(rte, &info->rtes,
				rte_list) {
(snip)

The panic occurs at list_for_each_entry. When I found this problem,
&info->rtes and rte->rte_list pointed same data but they had different
address.

&info->rtes   => 0xe0000000049231e0
rte->rte_list => 0xa0000001009231e0

info->rtes points region7 because its address is calculated from gp.
This difference of address caused panic.

Regards,
Takao Indoh

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Zou, Nanhai]

> -----Original Message-----
> From: linux-ia64-owner@vger.kernel.org
> [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Takao Indoh
> Sent: 2006Äê9ÔÂ7ÈÕ 9:19
> To: Zou, Nanhai
> Cc: Takao Indoh; Keith Owens; linux-ia64@vger.kernel.org;
> fastboot@lists.osdl.org
> Subject: Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT
> 
> On Thu, 7 Sep 2006 08:57:13 +0800, "Zou, Nanhai" wrote:
> 
> >> On Wed, 06 Sep 2006 18:06:53 +0900, Takao Indoh wrote:
> >>
> >> >>>+		/* change region of gp to region5 */
> >> >>>+		asm volatile ("movl gp=__gp"::: "memory");
> >> >>
> >> >>Why?  gp is already pointing at the kernel code before we enter
> >> >>ia64_init_handler().
> >> >
> >> >The purpose of this code is just changing region of gp from region7 to
> >> >region5. For example, if gp is e000000004ace170, I want to change gp to
> >>                                 ~~~~~~~~~~~~~~~~e000000000ace170
> >> Sorry, I mistook address...
> >>
> >>
> >> Regards,
> >> Takao Indoh
> >>
> >  You don't need to set gp here. Control page does not use GP, and
> >purgatory code will set GP to a physical address which was pre-calculated
> >at the time of crash kernel loading.
> 
> If gp points region7, panic occurs at kdump_disable_iosapic.
> 
> kdump_disable_iosapic(void)
> {
> 	u32 low32;
> 	struct iosapic_intr_info *info;
> 	struct iosapic_rte_info *rte;
> 	for (info = iosapic_intr_info; info <
> 			iosapic_intr_info + IA64_NUM_VECTORS; ++info) {
> 		low32 = info->low32 |= IOSAPIC_MASK;
> 		list_for_each_entry(rte, &info->rtes,
> 				rte_list) {
> (snip)
> 
> The panic occurs at list_for_each_entry. When I found this problem,
> &info->rtes and rte->rte_list pointed same data but they had different
> address.
> 
> &info->rtes   => 0xe0000000049231e0
> rte->rte_list => 0xa0000001009231e0
> 
> info->rtes points region7 because its address is calculated from gp.
> This difference of address caused panic.
> 
> Regards,
> Takao Indoh
>

  Ok, ia64_setreg should be better.
We should avoid put inline asm code in C file.

  Thanks
  Zou Nan hai
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Takao Indoh]

On Thu, 7 Sep 2006 09:32:38 +0800, "Zou, Nanhai" wrote:

>> On Thu, 7 Sep 2006 08:57:13 +0800, "Zou, Nanhai" wrote:
>> 
>> >> On Wed, 06 Sep 2006 18:06:53 +0900, Takao Indoh wrote:
>> >>
>> >> >>>+		/* change region of gp to region5 */
>> >> >>>+		asm volatile ("movl gp=__gp"::: "memory");
>> >> >>
>> >> >>Why?  gp is already pointing at the kernel code before we enter
>> >> >>ia64_init_handler().
>> >> >
>> >> >The purpose of this code is just changing region of gp from region7 to
>> >> >region5. For example, if gp is e000000004ace170, I want to change gp to
>> >>                                 ~~~~~~~~~~~~~~~~e000000000ace170
>> >> Sorry, I mistook address...
>> >>
>> >>
>> >> Regards,
>> >> Takao Indoh
>> >>
>> >  You don't need to set gp here. Control page does not use GP, and
>> >purgatory code will set GP to a physical address which was pre-calculated
>> >at the time of crash kernel loading.
>> 
>> If gp points region7, panic occurs at kdump_disable_iosapic.
>> 
>> kdump_disable_iosapic(void)
>> {
>> 	u32 low32;
>> 	struct iosapic_intr_info *info;
>> 	struct iosapic_rte_info *rte;
>> 	for (info = iosapic_intr_info; info <
>> 			iosapic_intr_info + IA64_NUM_VECTORS; ++info) {
>> 		low32 = info->low32 |= IOSAPIC_MASK;
>> 		list_for_each_entry(rte, &info->rtes,
>> 				rte_list) {
>> (snip)
>> 
>> The panic occurs at list_for_each_entry. When I found this problem,
>> &info->rtes and rte->rte_list pointed same data but they had different
>> address.
>> 
>> &info->rtes   => 0xe0000000049231e0
>> rte->rte_list => 0xa0000001009231e0
>> 
>> info->rtes points region7 because its address is calculated from gp.
>> This difference of address caused panic.
>> 
>> Regards,
>> Takao Indoh
>>
>
>  Ok, ia64_setreg should be better.
>We should avoid put inline asm code in C file.

Yes, at first I used ia64_setreg, but this problem was not solved.

	if (sos->rv_rc != 1 && (kdump_send_ipi || kdump_on_init)) {
		extern char __gp[];
		local_irq_disable();
		set_curr_task(cpu, previous_current);

		/* change region of gp to region5 */
		ia64_setreg(_IA64_REG_GP, __gp);
		crash_kexec(regs);


The disassemble code:
a000000100047080:       0b 38 01 02 00 24       [MMI]       addl r39=0,r1;;
a000000100047086:       50 02 9c 00 42 00                   mov r37=r39
a00000010004708c:       00 00 04 00                         nop.i 0x0;;
a000000100047090:       0a 08 00 4a 00 21       [MMI]       mov r1=r37;;
a000000100047096:       c0 02 80 00 42 00                   mov r44=r32
a00000010004709c:       00 00 04 00                         nop.i 0x0
a0000001000470a0:       17 00 02 51 02 14       [BBB]       br.call.sptk.many b0 000001000db4a0 <crash_kexec>

This disassembled code does not change content of r1.
Am I missing something?

Regards,
Takao Indoh

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Zou, Nanhai]

> -----Original Message-----
> From: Takao Indoh [mailto:indou.takao@jp.fujitsu.com]
> Sent: 2006Äê9ÔÂ7ÈÕ 11:21
> To: Zou, Nanhai
> Cc: Keith Owens; linux-ia64@vger.kernel.org; fastboot@lists.osdl.org
> Subject: RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT
> 
> On Thu, 7 Sep 2006 09:32:38 +0800, "Zou, Nanhai" wrote:
> 
> >> On Thu, 7 Sep 2006 08:57:13 +0800, "Zou, Nanhai" wrote:
> >>
> >> >> On Wed, 06 Sep 2006 18:06:53 +0900, Takao Indoh wrote:
> >> >>
> >> >> >>>+		/* change region of gp to region5 */
> >> >> >>>+		asm volatile ("movl gp=__gp"::: "memory");
> >> >> >>
> >> >> >>Why?  gp is already pointing at the kernel code before we enter
> >> >> >>ia64_init_handler().
> >> >> >
> >> >> >The purpose of this code is just changing region of gp from region7 to
> >> >> >region5. For example, if gp is e000000004ace170, I want to change gp
> to
> >> >>                                 ~~~~~~~~~~~~~~~~e000000000ace170
> >> >> Sorry, I mistook address...
> >> >>
> >> >>
> >> >> Regards,
> >> >> Takao Indoh
> >> >>
> >> >  You don't need to set gp here. Control page does not use GP, and
> >> >purgatory code will set GP to a physical address which was pre-calculated
> >> >at the time of crash kernel loading.
> >>
> >> If gp points region7, panic occurs at kdump_disable_iosapic.
> >>
> >> kdump_disable_iosapic(void)
> >> {
> >> 	u32 low32;
> >> 	struct iosapic_intr_info *info;
> >> 	struct iosapic_rte_info *rte;
> >> 	for (info = iosapic_intr_info; info <
> >> 			iosapic_intr_info + IA64_NUM_VECTORS; ++info) {
> >> 		low32 = info->low32 |= IOSAPIC_MASK;
> >> 		list_for_each_entry(rte, &info->rtes,
> >> 				rte_list) {
> >> (snip)
> >>
> >> The panic occurs at list_for_each_entry. When I found this problem,
> >> &info->rtes and rte->rte_list pointed same data but they had different
> >> address.
> >>
> >> &info->rtes   => 0xe0000000049231e0
> >> rte->rte_list => 0xa0000001009231e0
> >>
> >> info->rtes points region7 because its address is calculated from gp.
> >> This difference of address caused panic.
> >>
> >> Regards,
> >> Takao Indoh
> >>
> >
> >  Ok, ia64_setreg should be better.
> >We should avoid put inline asm code in C file.
> 
> Yes, at first I used ia64_setreg, but this problem was not solved.
> 
> 	if (sos->rv_rc != 1 && (kdump_send_ipi || kdump_on_init)) {
> 		extern char __gp[];
> 		local_irq_disable();
> 		set_curr_task(cpu, previous_current);
> 
> 		/* change region of gp to region5 */
> 		ia64_setreg(_IA64_REG_GP, __gp);
> 		crash_kexec(regs);
> 
> 
> The disassemble code:
> a000000100047080:       0b 38 01 02 00 24       [MMI]       addl r39=0,r1;;
> a000000100047086:       50 02 9c 00 42 00                   mov r37=r39
> a00000010004708c:       00 00 04 00                         nop.i 0x0;;
> a000000100047090:       0a 08 00 4a 00 21       [MMI]       mov r1=r37;;
> a000000100047096:       c0 02 80 00 42 00                   mov r44=r32
> a00000010004709c:       00 00 04 00                         nop.i 0x0
> a0000001000470a0:       17 00 02 51 02 14       [BBB]
> br.call.sptk.many b0 000001000db4a0 <crash_kexec>
> 
> This disassembled code does not change content of r1.
> Am I missing something?
> 
  That is strange...

  For the crash_kexec code, only 1 cpu need execute crash_kexec which will boot the second kernel, While other cpus could wait in spin loop. 
  So we can choose the monarch CPU in from notify_die chain after the monarch/slave walkaround.
  Thanks
  Zou Nan hai

> Regards,
> Takao Indoh

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Yu Luming]

> I agree. I'm also happy if kdb and kdump share the code.
> I'll check kdb code.
>
> >>+		/* change region of gp to region5 */
> >>+		asm volatile ("movl gp=__gp"::: "memory");
> >
> >Why?  gp is already pointing at the kernel code before we enter
> >ia64_init_handler().
>
> The purpose of this code is just changing region of gp from region7 to
> region5. For example, if gp is e000000004ace170, I want to change gp to
> a000000104ace170, but I couldn't find good method. Is there good method
> to change region?
>
Did  saved Os gp in SAL OS state  somehow get overridden before 
ia64_set_kernel_registers prior to invocation of ia64_init_handler?

I assume the following code (in ia64_set_kernel_registers ) should restore 
correct GP value.

ia64_set_kernel_registers:
	...
        ;;
        ld8 r1=[temp4]          // OS GP from SAL OS state

-- 
Thanks,
Luming

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Andreas Schwab]

Takao Indoh <indou.takao@jp.fujitsu.com> writes:

> The disassemble code:
> a000000100047080:       0b 38 01 02 00 24       [MMI]       addl r39=0,r1;;
> a000000100047086:       50 02 9c 00 42 00                   mov r37=r39
> a00000010004708c:       00 00 04 00                         nop.i 0x0;;
> a000000100047090:       0a 08 00 4a 00 21       [MMI]       mov r1=r37;;
> a000000100047096:       c0 02 80 00 42 00                   mov r44=r32
> a00000010004709c:       00 00 04 00                         nop.i 0x0
> a0000001000470a0:       17 00 02 51 02 14       [BBB]       br.call.sptk.many b0 000001000db4a0 <crash_kexec>
>
> This disassembled code does not change content of r1.

It does:

a000000100047090:       0a 08 00 4a 00 21       [MMI]       mov r1=r37;;

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Takao Indoh]

On Thu, 7 Sep 2006 16:48:19 +0800, Yu Luming wrote:

>> I agree. I'm also happy if kdb and kdump share the code.
>> I'll check kdb code.
>>
>> >>+		/* change region of gp to region5 */
>> >>+		asm volatile ("movl gp=__gp"::: "memory");
>> >
>> >Why?  gp is already pointing at the kernel code before we enter
>> >ia64_init_handler().
>>
>> The purpose of this code is just changing region of gp from region7 to
>> region5. For example, if gp is e000000004ace170, I want to change gp to
>> a000000104ace170, but I couldn't find good method. Is there good method
>> to change region?
>>
>Did  saved Os gp in SAL OS state  somehow get overridden before 
>ia64_set_kernel_registers prior to invocation of ia64_init_handler?
>
>I assume the following code (in ia64_set_kernel_registers ) should restore 
>correct GP value.
>
>ia64_set_kernel_registers:
>	...
>        ;;
>        ld8 r1=[temp4]          // OS GP from SAL OS state

Yes, but this gp value is physical address.
In the next line, gp is changed to virtual address.

DATA_PA_TO_VA(r1,temp1)

This macro just sets region7 bit, so gp value becomes region7 address.

Regards,
Takao Indoh

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Yu, Luming]

>>Did  saved Os gp in SAL OS state  somehow get overridden before 
>>ia64_set_kernel_registers prior to invocation of ia64_init_handler?
>>
>>I assume the following code (in ia64_set_kernel_registers ) 
>should restore 
>>correct GP value.
>>
>>ia64_set_kernel_registers:
>>	...
>>        ;;
>>        ld8 r1=[temp4]          // OS GP from SAL OS state
>
>Yes, but this gp value is physical address.
>In the next line, gp is changed to virtual address.
>
>DATA_PA_TO_VA(r1,temp1)
>
>This macro just sets region7 bit, so gp value becomes region7 address.

Ok, could you please confirm what is the value of r1 before
DATA_PA_TO_VA(r1, temp1)?
I guess we need to set r1 = __gp in ia64_set_kernel_registers.
If not,  this is a bug.  Am I right?

Thanks,
Luming

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Takao Indoh]

On Thu, 7 Sep 2006 21:01:17 +0800, "Yu, Luming" wrote:

>>>Did  saved Os gp in SAL OS state  somehow get overridden before 
>>>ia64_set_kernel_registers prior to invocation of ia64_init_handler?
>>>
>>>I assume the following code (in ia64_set_kernel_registers ) 
>>should restore 
>>>correct GP value.
>>>
>>>ia64_set_kernel_registers:
>>>	...
>>>        ;;
>>>        ld8 r1=[temp4]          // OS GP from SAL OS state
>>
>>Yes, but this gp value is physical address.
>>In the next line, gp is changed to virtual address.
>>
>>DATA_PA_TO_VA(r1,temp1)
>>
>>This macro just sets region7 bit, so gp value becomes region7 address.
>
>Ok, could you please confirm what is the value of r1 before
>DATA_PA_TO_VA(r1, temp1)?
>I guess we need to set r1 = __gp in ia64_set_kernel_registers.
>If not,  this is a bug.  Am I right?

In the ia64_mca_init, the value of ia64_tpa(ia64_getreg(_IA64_REG_GP)
is registered as gp value. When cpu comes into the above code, r1 is set
to this value, I think. Therefore, r1 has physical address of __gp
before DATA_PA_TO_VA(r1, temp1). How do you think?

I think you are right, I also think r1 has to be set to __gp before 
calling ia64_init_handler.

Regards,
Takao Indoh

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Luck, Tony]

On Thu, Sep 07, 2006 at 11:08:34AM +0200, Andreas Schwab wrote:
> Takao Indoh <indou.takao@jp.fujitsu.com> writes:
> 
> > The disassemble code:
> > a000000100047080:       0b 38 01 02 00 24       [MMI]       addl r39=0,r1;;
> > a000000100047086:       50 02 9c 00 42 00                   mov r37=r39
> > a00000010004708c:       00 00 04 00                         nop.i 0x0;;
> > a000000100047090:       0a 08 00 4a 00 21       [MMI]       mov r1=r37;;
> > a000000100047096:       c0 02 80 00 42 00                   mov r44=r32
> > a00000010004709c:       00 00 04 00                         nop.i 0x0
> > a0000001000470a0:       17 00 02 51 02 14       [BBB]       br.call.sptk.many b0 000001000db4a0 <crash_kexec>
> >
> > This disassembled code does not change content of r1.
> 
> It does:
> 
> a000000100047090:       0a 08 00 4a 00 21       [MMI]       mov r1=r37;;

But looking before that instruction r37 is a copy of r39 (mov r37=r39).
r39 was calculated by "addl r39=0,r1" i.e. r1+0, also known as "r1". So
this sequence just takes the original value from r1 and puts it into r1
(passing through two other registers along the way).  Not only unhelpful,
but inefficient too :-)

Perhaps just make a tiny function:

GLOBAL_ENTRY(ia64_set_gp)
	movl r1=__gp
	br.ret.sptk.many b0
END(ia64_set_gp)

and put it in head.S to bypass this "cleverness" by gcc and
also avoid the need for inline asm.

-Tony

---
Yarn store opening in Sunnyvale, CA on Oct 14th. http://purlescenceyarns.com

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Andreas Schwab]

"Luck, Tony" <tony.luck@intel.com> writes:

> But looking before that instruction r37 is a copy of r39 (mov r37=r39).
> r39 was calculated by "addl r39=0,r1" i.e. r1+0, also known as "r1".

0 is modified by the result of the GOT relocation for __gp.

> and put it in head.S to bypass this "cleverness" by gcc and

__gp has no special meaning for gcc.  It's just another global variable.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Luck, Tony]

On Thu, Sep 07, 2006 at 06:44:30PM +0200, Andreas Schwab wrote:
> 0 is modified by the result of the GOT relocation for __gp
The 0 is the already modified value.

> __gp has no special meaning for gcc.  It's just another global variable.

Precisely the problem.  On ia64 gcc likes to compute the address
of global variables as offsets from __gp ... which it knows is
always available in r1.  So __gp gets no special treatment at all,
gcc knows that __gp = GOT + offset (and for __gp "offset" is 0).

-Tony

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Zou, Nanhai]

> -----Original Message-----
> From: linux-ia64-owner@vger.kernel.org
> [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Takao Indoh
> Sent: 2006Äê9ÔÂ7ÈÕ 22:49
> To: Yu, Luming
> Cc: fastboot@lists.osdl.org; Keith Owens; linux-ia64@vger.kernel.org
> Subject: RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT
> 
> On Thu, 7 Sep 2006 21:01:17 +0800, "Yu, Luming" wrote:
> 
> >>>Did  saved Os gp in SAL OS state  somehow get overridden before
> >>>ia64_set_kernel_registers prior to invocation of ia64_init_handler?
> >>>
> >>>I assume the following code (in ia64_set_kernel_registers )
> >>should restore
> >>>correct GP value.
> >>>
> >>>ia64_set_kernel_registers:
> >>>	...
> >>>        ;;
> >>>        ld8 r1=[temp4]          // OS GP from SAL OS state
> >>
> >>Yes, but this gp value is physical address.
> >>In the next line, gp is changed to virtual address.
> >>
> >>DATA_PA_TO_VA(r1,temp1)
> >>
> >>This macro just sets region7 bit, so gp value becomes region7 address.
> >
> >Ok, could you please confirm what is the value of r1 before
> >DATA_PA_TO_VA(r1, temp1)?
> >I guess we need to set r1 = __gp in ia64_set_kernel_registers.
> >If not,  this is a bug.  Am I right?
> 
> In the ia64_mca_init, the value of ia64_tpa(ia64_getreg(_IA64_REG_GP)
> is registered as gp value. When cpu comes into the above code, r1 is set
> to this value, I think. Therefore, r1 has physical address of __gp
> before DATA_PA_TO_VA(r1, temp1). How do you think?
> 
> I think you are right, I also think r1 has to be set to __gp before
> calling ia64_init_handler.
> 
  Yes, I agree GP should be set to __gp before calling into ia64_init_handler, not only the region issue.
  E.g. If an OS_INIT happen in the context of a module, the GP value could be point to the modules GP.

  Thanks
  Zou Nan hai

> Regards,
> Takao Indoh
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Zou Nan hai]

On Mon, 2006-09-11 at 14:56, Keith Owens wrote:
> Takao Indoh (on Thu, 07 Sep 2006 23:48:51 +0900) wrote:
> >On Thu, 7 Sep 2006 21:01:17 +0800, "Yu, Luming" wrote:
> >
> >>>>Did  saved Os gp in SAL OS state  somehow get overridden before 
> >>>>ia64_set_kernel_registers prior to invocation of
> ia64_init_handler?
> >>>>
> >>>>I assume the following code (in ia64_set_kernel_registers ) 
> >>>should restore 
> >>>>correct GP value.
> >>>>
> >>>>ia64_set_kernel_registers:
> >>>>    ...
> >>>>        ;;
> >>>>        ld8 r1=[temp4]          // OS GP from SAL OS state
> >>>
> >>>Yes, but this gp value is physical address.
> >>>In the next line, gp is changed to virtual address.
> >>>
> >>>DATA_PA_TO_VA(r1,temp1)
> >>>
> >>>This macro just sets region7 bit, so gp value becomes region7
> address.
> >>
> >>Ok, could you please confirm what is the value of r1 before
> >>DATA_PA_TO_VA(r1, temp1)?
> >>I guess we need to set r1 = __gp in ia64_set_kernel_registers.
> >>If not,  this is a bug.  Am I right?
> >
> >In the ia64_mca_init, the value of ia64_tpa(ia64_getreg(_IA64_REG_GP)
> >is registered as gp value. When cpu comes into the above code, r1 is
> set
> >to this value, I think. Therefore, r1 has physical address of __gp
> >before DATA_PA_TO_VA(r1, temp1). How do you think?
> >
> >I think you are right, I also think r1 has to be set to __gp before 
> >calling ia64_init_handler.
> 
> It already is set in mca_asm.S.  Remember that ia64_init_handler is by
> definition part of the kernel, it cannot be a module.  Therefore
> before
> entering ia64_init_handler, r1 must be set to what the kernel expects
> to be in r1.  The standard kernel's r1 is a region 7 address, not a
> region 5 address.  The kernel (including __gp) is compiled as region 5
> but relocated to region 7 during kernel load.
> 
> Is the kexec kernel running in region 5?  That may be where the
> confusion is coming from.
> 
Hi Keith,
	For 2.6 kernel, I think GP is a region 5 address when inside kernel.
The entire kernel image is resided in region 5 without relocate to
region 7. 
	Another issue I can see from calculate GP from physical address passed
from OS_INIT is,
	I am not quite sure though, if OS_INIT is happened during a module
execution, or happened when system is in user space, what GP value will
be passed from SAL/PAL? If the gp value is the physical address of the
modules 's gp, we can't set GP value according to that, because
ia64_init_handler is inside kernel.
	
	Kexec kernel used in kdump is usually exactly the same kernel with the
host kernel, except they are located in different physical address
ranges. The first kernel will never touch the RAM belongs to the crash
kernel at run time. The crash kernel will not touch the RAM belongs to
the first kernel unless you read it via /proc/vmcore.

Thanks 	
Zou Nan hai 

> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64"
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Keith Owens]

Takao Indoh (on Thu, 07 Sep 2006 23:48:51 +0900) wrote:
>On Thu, 7 Sep 2006 21:01:17 +0800, "Yu, Luming" wrote:
>
>>>>Did  saved Os gp in SAL OS state  somehow get overridden before 
>>>>ia64_set_kernel_registers prior to invocation of ia64_init_handler?
>>>>
>>>>I assume the following code (in ia64_set_kernel_registers ) 
>>>should restore 
>>>>correct GP value.
>>>>
>>>>ia64_set_kernel_registers:
>>>>	...
>>>>        ;;
>>>>        ld8 r1=[temp4]          // OS GP from SAL OS state
>>>
>>>Yes, but this gp value is physical address.
>>>In the next line, gp is changed to virtual address.
>>>
>>>DATA_PA_TO_VA(r1,temp1)
>>>
>>>This macro just sets region7 bit, so gp value becomes region7 address.
>>
>>Ok, could you please confirm what is the value of r1 before
>>DATA_PA_TO_VA(r1, temp1)?
>>I guess we need to set r1 = __gp in ia64_set_kernel_registers.
>>If not,  this is a bug.  Am I right?
>
>In the ia64_mca_init, the value of ia64_tpa(ia64_getreg(_IA64_REG_GP)
>is registered as gp value. When cpu comes into the above code, r1 is set
>to this value, I think. Therefore, r1 has physical address of __gp
>before DATA_PA_TO_VA(r1, temp1). How do you think?
>
>I think you are right, I also think r1 has to be set to __gp before 
>calling ia64_init_handler.

It already is set in mca_asm.S.  Remember that ia64_init_handler is by
definition part of the kernel, it cannot be a module.  Therefore before
entering ia64_init_handler, r1 must be set to what the kernel expects
to be in r1.  The standard kernel's r1 is a region 7 address, not a
region 5 address.  The kernel (including __gp) is compiled as region 5
but relocated to region 7 during kernel load.

Is the kexec kernel running in region 5?  That may be where the
confusion is coming from.

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Luck, Tony]

On Mon, Sep 11, 2006 at 02:12:13PM +0800, Zou Nan hai wrote:
> On Mon, 2006-09-11 at 14:56, Keith Owens wrote:
> > entering ia64_init_handler, r1 must be set to what the kernel expects
> > to be in r1.  The standard kernel's r1 is a region 7 address, not a
> > region 5 address.  The kernel (including __gp) is compiled as region 5
> > but relocated to region 7 during kernel load.
> > 
> > Is the kexec kernel running in region 5?  That may be where the
> > confusion is coming from.
> > 
> Hi Keith,
> 	For 2.6 kernel, I think GP is a region 5 address when inside kernel.
> The entire kernel image is resided in region 5 without relocate to
> region 7. 

Nanhai is right here.  The kernel is linked at region 5
addresses and mapped into physical memory by ITR[0]/DTR[0]

Code in head.S that initializes r1 before we call any C code
is a simple "movl gp=__gp".  Disassembling vmlinux, I see that
became: "movl r1=0xa000000100b6ea80".  Peeking with a debugger
at the running kernel I see a region 5 address in r1.

Perhaps you are remembering the tricks that we play with the
address of "init_task" ... that has truly schizophrenic
existance between region5 and region7 (and our preferred access
address has changed from region7 to region5 as the years have
gone by to make list_structs inside init_task work correctly).

-Tony

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Takao Indoh]

On 11 Sep 2006 14:12:13 +0800, Zou Nan hai wrote:

>	I am not quite sure though, if OS_INIT is happened during a module
>execution, or happened when system is in user space, what GP value will
>be passed from SAL/PAL? If the gp value is the physical address of the
>modules 's gp, we can't set GP value according to that, because
>ia64_init_handler is inside kernel.

When OS_INIT handler is called from SAL, gp register is always
set to the value which is specified by ia64_sal_set_vectors,
I think.

Regards,
Takao Indoh

Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Keith Owens]

Zou Nan hai (on 11 Sep 2006 14:12:13 +0800) wrote:
>	For 2.6 kernel, I think GP is a region 5 address when inside kernel.
>The entire kernel image is resided in region 5 without relocate to
>region 7. 

You are right, I have only three excuses.

(1) Having too much fun skiing to think about computers.
(2) The kernel _used_ to be in region 7.
(3) The kernel addresses in region 5 can always be accessed via the
    identity map in region 7 and sometimes we do just that, see below.

>	Another issue I can see from calculate GP from physical address passed
>from OS_INIT is,
>	I am not quite sure though, if OS_INIT is happened during a module
>execution, or happened when system is in user space, what GP value will
>be passed from SAL/PAL? If the gp value is the physical address of the
>modules 's gp, we can't set GP value according to that, because
>ia64_init_handler is inside kernel.

It does not matter where the kernel was when the INIT was delivered,
even if the INIT ws delivered while a module was in control.  r1 must
be loaded with the correct gp for the kernel, because ia64_init_handler
is a kernel routine, not a module routine.  The sequence goes :-

* Kernel boots

* ia64_mca_init() -> ia64_sal_set_vectors(), passing the _physical_
  value of the kernel's gp. This physical address is derived from the
  region 5 virtual gp by tpa().

* Kernel runs.
  INIT is delivered.  It does not matter what state the machine is in,
  in kernel, a module or PAL/SAL is irrelevant.

* ia64_os_init_dispatch() -> ia64_set_kernel_registers() which converts
  the physical gp to a region 7 identity mapped virtual gp.

* ia64_os_init_dispatch() -> ia64_init_handler().

* ia64_init_handler() runs using a region 7 virtual gp.  This is a
  different virtual address than the original region 5 gp that the
  kernel booted with, but both the region 5 and region 7 gp values map
  the same physical pages.


>	Kexec kernel used in kdump is usually exactly the same kernel with the
>host kernel, except they are located in different physical address
>ranges. The first kernel will never touch the RAM belongs to the crash
>kernel at run time. The crash kernel will not touch the RAM belongs to
>the first kernel unless you read it via /proc/vmcore.

Which means that the region 7 gp used by MCA/INIT is no good for the
kexec kernel.  You either need the original region 5 gp or you need to
adjust r1 by the difference between the physical start of the real
kernel and the physical start of the kexec kernel.

Note: When I did the code in ia64_set_kernel_registers(), it was too
messy to work out what the original region 5 gp value was, so I just
did 'DATA_PA_TO_VA(r1,temp1)' and relied on the identity mapping in
region 7.  Perhaps the correct fix is for ia64_set_kernel_registers()
to set the correct region 5 gp and not rely on the identity mapping.
That should easy enough to do, define a global variable that contains
the region 5 value of gp and load that value in
ia64_set_kernel_registers instead of using DATA_PA_TO_VA.

RE: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT

[Zou, Nanhai]

> -----Original Message-----
> From: linux-ia64-owner@vger.kernel.org
> [mailto:linux-ia64-owner@vger.kernel.org] On Behalf Of Keith Owens
> Sent: 2006Äê9ÔÂ13ÈÕ 15:10
> To: Zou, Nanhai
> Cc: Takao Indoh; Yu, Luming; fastboot; Linux-IA64
> Subject: Re: [Fastboot] [PATCH]IA64 kexec/kdump patch for INIT
> 
> Zou Nan hai (on 11 Sep 2006 14:12:13 +0800) wrote:
> >	For 2.6 kernel, I think GP is a region 5 address when inside kernel.
> >The entire kernel image is resided in region 5 without relocate to
> >region 7.
> 
> You are right, I have only three excuses.
> 
> (1) Having too much fun skiing to think about computers.
> (2) The kernel _used_ to be in region 7.
> (3) The kernel addresses in region 5 can always be accessed via the
>     identity map in region 7 and sometimes we do just that, see below.
> 
> >	Another issue I can see from calculate GP from physical address passed
> >from OS_INIT is,
> >	I am not quite sure though, if OS_INIT is happened during a module
> >execution, or happened when system is in user space, what GP value will
> >be passed from SAL/PAL? If the gp value is the physical address of the
> >modules 's gp, we can't set GP value according to that, because
> >ia64_init_handler is inside kernel.
> 
> It does not matter where the kernel was when the INIT was delivered,
> even if the INIT ws delivered while a module was in control.  r1 must
> be loaded with the correct gp for the kernel, because ia64_init_handler
> is a kernel routine, not a module routine.  The sequence goes :-
> 
> * Kernel boots
> 
> * ia64_mca_init() -> ia64_sal_set_vectors(), passing the _physical_
>   value of the kernel's gp. This physical address is derived from the
>   region 5 virtual gp by tpa().
> 
> * Kernel runs.
>   INIT is delivered.  It does not matter what state the machine is in,
>   in kernel, a module or PAL/SAL is irrelevant.
> 
> * ia64_os_init_dispatch() -> ia64_set_kernel_registers() which converts
>   the physical gp to a region 7 identity mapped virtual gp.
> 
> * ia64_os_init_dispatch() -> ia64_init_handler().
> 
> * ia64_init_handler() runs using a region 7 virtual gp.  This is a
>   different virtual address than the original region 5 gp that the
>   kernel booted with, but both the region 5 and region 7 gp values map
>   the same physical pages.
> 
> 
  While the content of region 5 and region 7 gp pointed to are the same, the data pointers their self are different.

For the particular sample,
there is a loop, list_for_each_entry(rte, &info->rte, rte_list)
if r1 is set to the identity mapping address in region 7. 
Where head is pointer to global data structure, so gcc decide to get the &info->rte value with the help of gp, then the address become a region 7 value. However the value in list_entry(head->next) is a region 5 value stored before.
Then list_for_each_entry() become an endless loop, because pos->member = head is comparing a region 5 pointer to a region 7 pointer, although they refers to the same physical address, the compare still will not return true....

Note that if we move the code to notify call back function we should not see this problem, because gp in callback function is fetched from function entry, kernel fetch the region 5 gp value in function pointer entry with the help of region 7 gp,  then in notify callback function we got region 5 gp again....

However I think this is still an issue, 
GP is inconsistent to kernel GP at the MCA C codes not referenced by function pointer.

> >	Kexec kernel used in kdump is usually exactly the same kernel with the
> >host kernel, except they are located in different physical address
> >ranges. The first kernel will never touch the RAM belongs to the crash
> >kernel at run time. The crash kernel will not touch the RAM belongs to
> >the first kernel unless you read it via /proc/vmcore.
> 
> Which means that the region 7 gp used by MCA/INIT is no good for the
> kexec kernel.  You either need the original region 5 gp or you need to
> adjust r1 by the difference between the physical start of the real
> kernel and the physical start of the kexec kernel.
> 
  The second kernel is only separated with first kernel in _PHYSICAL_ memory space. They both own the entire virtual address space ... :). 

  Thanks
Zou Nan hai
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html