* [pm] Strange cleanups in -test8 kernel/acpi/wakeup.S
@ 2003-10-18 20:12 Pavel Machek
2003-10-20 18:26 ` Patrick Mochel
0 siblings, 1 reply; 5+ messages in thread
From: Pavel Machek @ 2003-10-18 20:12 UTC (permalink / raw)
To: Patrick Mochel, kernel list
Hi!
Some more changes landed in -test8. I have not seen them
before. Patrick, please, if you change something, can you post patch
somewhere for review before merging with Linus?
bkcvs info is:
BitKeeper to RCS/CVS export
----------------------------
revision 1.5
date: 2003/10/08 22:55:45; author: mochel; state: Exp; lines: +37
-89
[power] Clean up ACPI STR assembly.
diff -Nru a/arch/i386/kernel/acpi/wakeup.S
b/arch/i386/kernel/acpi/wakeup.S
--- a/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
+++ b/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
@@ -172,14 +172,13 @@
.org 0x1000
wakeup_pmode_return:
- movl $__KERNEL_DS, %eax
- movl %eax, %ds
- movw $0x0e00 + 'u', %ds:(0xb8016)
-
- # restore other segment registers
- xorl %eax, %eax
+ movw $__KERNEL_DS, %ax
+ movw %ax, %ss
+ movw %ax, %ds
+ movw %ax, %es
movw %ax, %fs
movw %ax, %gs
+ movw $0x0e00 + 'u', 0xb8016
# reload the gdt, as we need the full 32 bit address
lgdt saved_gdt
~~~~~~~~~~~~~~~~~
Notice lgdt here. You have moved setup of segment registers before
loading gdt. This is actually okay, if you can be sure that all such
registers are in gdt (and not in ldt, for example).
Now, if you are sure this is okay (there is another load of gdt in
real mode), you can kill this lgdt, too, and kill the comment below.
@@ -192,46 +191,30 @@
wbinvd
# and restore the stack ... but you need gdt for this to work
- movl $__KERNEL_DS, %eax
- movw %ax, %ss
- movw %ax, %ds
- movw %ax, %es
- movw %ax, %fs
- movw %ax, %gs
- movl saved_esp, %esp
+ movl saved_context_esp, %esp
...
- movw $0x0e00 + 'W', %ds:(0xb8018)
- movl $(1024*1024*3), %ecx
- movl $0, %esi
- rep lodsb
- movw $0x0e00 + 'O', %ds:(0xb8018)
+ movw $0x0e00 + 'W', 0xb8018
+ outl %eax, $0x80
+ outl %eax, $0x80
+ movw $0x0e00 + 'O', 0xb8018
What are these outl-s? I do not see %ax being initialized... Some kind
of debugging hack? If you are trying to emulate delays, those are not
needed, what you killed were debugging hacks.
You no longer save %eax, %ecx, %edx, %esp, %ebp, %edi, %esi. This
means you probably need to add asmlinkage somewhere...
Pavel
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [pm] Strange cleanups in -test8 kernel/acpi/wakeup.S
2003-10-18 20:12 [pm] Strange cleanups in -test8 kernel/acpi/wakeup.S Pavel Machek
@ 2003-10-20 18:26 ` Patrick Mochel
2003-10-20 19:31 ` Pavel Machek
2003-10-20 20:34 ` Mika Penttilä
0 siblings, 2 replies; 5+ messages in thread
From: Patrick Mochel @ 2003-10-20 18:26 UTC (permalink / raw)
To: Pavel Machek; +Cc: kernel list
> Some more changes landed in -test8. I have not seen them
> before. Patrick, please, if you change something, can you post patch
> somewhere for review before merging with Linus?
Pavel, I wrote the code in the first place, before you littered your
'debug hacks' throughout it. I have merely been trying to simplify it for
debugging on other processors that are known not to work. While I
understand your generic plea for review, I fail to see how it would help
with this assembly..
> bkcvs info is:
> BitKeeper to RCS/CVS export
> ----------------------------
> revision 1.5
> date: 2003/10/08 22:55:45; author: mochel; state: Exp; lines: +37
> -89
> [power] Clean up ACPI STR assembly.
It might help if you read the full changeset comments.
> diff -Nru a/arch/i386/kernel/acpi/wakeup.S
> b/arch/i386/kernel/acpi/wakeup.S
> --- a/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
> +++ b/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
> @@ -172,14 +172,13 @@
> .org 0x1000
>
> wakeup_pmode_return:
> - movl $__KERNEL_DS, %eax
> - movl %eax, %ds
> - movw $0x0e00 + 'u', %ds:(0xb8016)
> -
> - # restore other segment registers
> - xorl %eax, %eax
> + movw $__KERNEL_DS, %ax
> + movw %ax, %ss
> + movw %ax, %ds
> + movw %ax, %es
> movw %ax, %fs
> movw %ax, %gs
> + movw $0x0e00 + 'u', 0xb8016
>
> # reload the gdt, as we need the full 32 bit address
> lgdt saved_gdt
> ~~~~~~~~~~~~~~~~~
>
> Notice lgdt here. You have moved setup of segment registers before
> loading gdt. This is actually okay, if you can be sure that all such
> registers are in gdt (and not in ldt, for example).
All segments are in the GDT, as we use the same GDT in real mode as we do
in protected mode. However, you must reload the GDT in protected mode
because the GDTR is only 24 bits in real mode, but 32 in protected mode.
> # and restore the stack ... but you need gdt for this to work
> - movl $__KERNEL_DS, %eax
> - movw %ax, %ss
> - movw %ax, %ds
> - movw %ax, %es
> - movw %ax, %fs
> - movw %ax, %gs
> - movl saved_esp, %esp
> + movl saved_context_esp, %esp
>
> ...
>
> - movw $0x0e00 + 'W', %ds:(0xb8018)
> - movl $(1024*1024*3), %ecx
> - movl $0, %esi
> - rep lodsb
> - movw $0x0e00 + 'O', %ds:(0xb8018)
> + movw $0x0e00 + 'W', 0xb8018
> + outl %eax, $0x80
> + outl %eax, $0x80
> + movw $0x0e00 + 'O', 0xb8018
>
> What are these outl-s? I do not see %ax being initialized... Some kind
> of debugging hack? If you are trying to emulate delays, those are not
> needed, what you killed were debugging hacks.
Debugging hacks that did what? AFAICT, it only implemented a small delay
by loading %eax with 0 a few million times.
That's better done by writing to port 0x80. It's a debug port that the
BIOS uses to report progress. It can also be used for delay. There are
systems and PCI cards that have LEDs that report what is written to port
0x80.
> You no longer save %eax, %ecx, %edx, %esp, %ebp, %edi, %esi. This
> means you probably need to add asmlinkage somewhere...
Uh, first read save_registers() near the end of the file. Next, please
understand, as I've told you before, that %eax, %ecx, and %edx are
caller-savable registers. They're not expected to be saved.
Pat
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pm] Strange cleanups in -test8 kernel/acpi/wakeup.S
2003-10-20 18:26 ` Patrick Mochel
@ 2003-10-20 19:31 ` Pavel Machek
2003-10-20 19:41 ` Patrick Mochel
2003-10-20 20:34 ` Mika Penttilä
1 sibling, 1 reply; 5+ messages in thread
From: Pavel Machek @ 2003-10-20 19:31 UTC (permalink / raw)
To: Patrick Mochel; +Cc: kernel list
Hi!
> > bkcvs info is:
> > BitKeeper to RCS/CVS export
> > ----------------------------
> > revision 1.5
> > date: 2003/10/08 22:55:45; author: mochel; state: Exp; lines: +37
> > -89
> > [power] Clean up ACPI STR assembly.
>
> It might help if you read the full changeset comments.
Sorry, I did not see them anywhere :-(.
> > diff -Nru a/arch/i386/kernel/acpi/wakeup.S
> > b/arch/i386/kernel/acpi/wakeup.S
> > --- a/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
> > +++ b/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
> > @@ -172,14 +172,13 @@
> > .org 0x1000
> >
> > wakeup_pmode_return:
> > - movl $__KERNEL_DS, %eax
> > - movl %eax, %ds
> > - movw $0x0e00 + 'u', %ds:(0xb8016)
> > -
> > - # restore other segment registers
> > - xorl %eax, %eax
> > + movw $__KERNEL_DS, %ax
> > + movw %ax, %ss
> > + movw %ax, %ds
> > + movw %ax, %es
> > movw %ax, %fs
> > movw %ax, %gs
> > + movw $0x0e00 + 'u', 0xb8016
> >
> > # reload the gdt, as we need the full 32 bit address
> > lgdt saved_gdt
> > ~~~~~~~~~~~~~~~~~
> >
> > Notice lgdt here. You have moved setup of segment registers before
> > loading gdt. This is actually okay, if you can be sure that all such
> > registers are in gdt (and not in ldt, for example).
>
> All segments are in the GDT, as we use the same GDT in real mode as we do
> in protected mode. However, you must reload the GDT in protected mode
> because the GDTR is only 24 bits in real mode, but 32 in protected
> mode.
Ok, sorry.
> > # and restore the stack ... but you need gdt for this to work
> > - movl $__KERNEL_DS, %eax
> > - movw %ax, %ss
> > - movw %ax, %ds
> > - movw %ax, %es
> > - movw %ax, %fs
> > - movw %ax, %gs
> > - movl saved_esp, %esp
> > + movl saved_context_esp, %esp
> >
> > ...
> >
> > - movw $0x0e00 + 'W', %ds:(0xb8018)
> > - movl $(1024*1024*3), %ecx
> > - movl $0, %esi
> > - rep lodsb
> > - movw $0x0e00 + 'O', %ds:(0xb8018)
> > + movw $0x0e00 + 'W', 0xb8018
> > + outl %eax, $0x80
> > + outl %eax, $0x80
> > + movw $0x0e00 + 'O', 0xb8018
> >
> > What are these outl-s? I do not see %ax being initialized... Some kind
> > of debugging hack? If you are trying to emulate delays, those are not
> > needed, what you killed were debugging hacks.
>
> Debugging hacks that did what? AFAICT, it only implemented a small delay
> by loading %eax with 0 a few million times.
One was test "is stack working or do we get fault", and second was "do
our page tables look reasonable". I was not trying to pause.
Anyway it was undocumented and it was hack. Best thing is probably to
kill it. Here's a patch.
Pavel
--- tmp/linux/arch/i386/kernel/acpi/wakeup.S 2003-10-18 20:26:27.000000000 +0200
+++ linux/arch/i386/kernel/acpi/wakeup.S 2003-10-20 21:16:06.000000000 +0200
@@ -193,11 +193,6 @@
# and restore the stack ... but you need gdt for this to work
movl saved_context_esp, %esp
- movw $0x0e00 + 'W', 0xb8018
- outl %eax, $0x80
- outl %eax, $0x80
- movw $0x0e00 + 'O', 0xb8018
-
movl %cs:saved_magic, %eax
cmpl $0x12345678, %eax
jne bogus_magic
@@ -205,9 +200,6 @@
# jump to place where we left off
movl saved_eip,%eax
movw $0x0e00 + 'x', 0xb8018
- outl %eax, $0x80
- outl %eax, $0x80
- movw $0x0e00 + '!', 0xb801a
jmp *%eax
bogus_magic:
--
When do you have a heart between your knees?
[Johanka's followup: and *two* hearts?]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pm] Strange cleanups in -test8 kernel/acpi/wakeup.S
2003-10-20 18:26 ` Patrick Mochel
2003-10-20 19:31 ` Pavel Machek
@ 2003-10-20 20:34 ` Mika Penttilä
1 sibling, 0 replies; 5+ messages in thread
From: Mika Penttilä @ 2003-10-20 20:34 UTC (permalink / raw)
To: Patrick Mochel; +Cc: Pavel Machek, kernel list
Patrick Mochel wrote:
>>Some more changes landed in -test8. I have not seen them
>>before. Patrick, please, if you change something, can you post patch
>>somewhere for review before merging with Linus?
>>
>>
>
>Pavel, I wrote the code in the first place, before you littered your
>'debug hacks' throughout it. I have merely been trying to simplify it for
>debugging on other processors that are known not to work. While I
>understand your generic plea for review, I fail to see how it would help
>with this assembly..
>
>
>
>>bkcvs info is:
>>BitKeeper to RCS/CVS export
>>----------------------------
>>revision 1.5
>>date: 2003/10/08 22:55:45; author: mochel; state: Exp; lines: +37
>>-89
>>[power] Clean up ACPI STR assembly.
>>
>>
>
>It might help if you read the full changeset comments.
>
>
>
>>diff -Nru a/arch/i386/kernel/acpi/wakeup.S
>>b/arch/i386/kernel/acpi/wakeup.S
>>--- a/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
>>+++ b/arch/i386/kernel/acpi/wakeup.S Fri Oct 17 14:43:50 2003
>>@@ -172,14 +172,13 @@
>> .org 0x1000
>>
>> wakeup_pmode_return:
>>- movl $__KERNEL_DS, %eax
>>- movl %eax, %ds
>>- movw $0x0e00 + 'u', %ds:(0xb8016)
>>-
>>- # restore other segment registers
>>- xorl %eax, %eax
>>+ movw $__KERNEL_DS, %ax
>>+ movw %ax, %ss
>>+ movw %ax, %ds
>>+ movw %ax, %es
>> movw %ax, %fs
>> movw %ax, %gs
>>+ movw $0x0e00 + 'u', 0xb8016
>>
>> # reload the gdt, as we need the full 32 bit address
>> lgdt saved_gdt
>> ~~~~~~~~~~~~~~~~~
>>
>>Notice lgdt here. You have moved setup of segment registers before
>>loading gdt. This is actually okay, if you can be sure that all such
>>registers are in gdt (and not in ldt, for example).
>>
>>
>
>All segments are in the GDT, as we use the same GDT in real mode as we do
>in protected mode. However, you must reload the GDT in protected mode
>because the GDTR is only 24 bits in real mode, but 32 in protected mode.
>
>
>
To be exact, GDTR is always 48 bits in x86. Reloading it twice seems
pretty pointless.
--Mika
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2003-10-20 20:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-10-18 20:12 [pm] Strange cleanups in -test8 kernel/acpi/wakeup.S Pavel Machek
2003-10-20 18:26 ` Patrick Mochel
2003-10-20 19:31 ` Pavel Machek
2003-10-20 19:41 ` Patrick Mochel
2003-10-20 20:34 ` Mika Penttilä
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.