Skip to contentSkip to conversation

x86: relocate_kernel - use predefined PAGE_SIZE instead of own alias

PATCH In review Cyrill Gorcunov 3 participants 7 messages Mar 21, 2008 x86

lore
Share & tools
lore

Cyrill Gorcunov

author
#0 lore raw

PATCH 1/1 x86: relocate_kernel - use predefined PAGE_SIZE instead of own alias

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

Signed-off-by Cyrill Gorcunov <[email protected]>

---

 relocate_kernel_32.S |    3 +--
 relocate_kernel_64.S |    3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)
arch/x86/kernel/relocate_kernel_32.S +1 -2
Index: linux-2.6.git/arch/x86/kernel/relocate_kernel_32.S
===================================================================
@@ -15,12 +15,11 @@
  */
 
 #define PTR(x) (x << 2)
-#define PAGE_ALIGNED (1 << PAGE_SHIFT)
 #define PAGE_ATTR 0x63 /* _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY */
 #define PAE_PGD_ATTR 0x01 /* _PAGE_PRESENT */
 
 	.text
-	.align PAGE_ALIGNED
+	.align PAGE_SIZE
 	.globl relocate_kernel
 relocate_kernel:
 	movl	8(%esp), %ebp /* list of pages */
arch/x86/kernel/relocate_kernel_64.S +1 -2
Index: linux-2.6.git/arch/x86/kernel/relocate_kernel_64.S
===================================================================
@@ -15,11 +15,10 @@
  */
 
 #define PTR(x) (x << 3)
-#define PAGE_ALIGNED (1 << PAGE_SHIFT)
 #define PAGE_ATTR 0x63 /* _PAGE_PRESENT|_PAGE_RW|_PAGE_ACCESSED|_PAGE_DIRTY */
 
 	.text
-	.align PAGE_ALIGNED
+	.align PAGE_SIZE
 	.code64
 	.globl relocate_kernel
 relocate_kernel:
reviewer
#1 lore raw
Quoted 4 lines · Cyrill Gorcunov

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

Signed-off-by Cyrill Gorcunov <[email protected]>

Looks good!

-hpa

author
#2 lore raw

[H. Peter Anvin - Fri, Mar 21, 2008 at 01:18:13PM -0700]

Quoted 9 lines
Quoted 3 lines · Cyrill Gorcunov

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

Signed-off-by Cyrill Gorcunov <[email protected]>

Looks good!

-hpa

Thanks Peter for review, btw we have in relocate_kernel_32.S

188: movl %cr4, %eax
189: andl $0, %eax
190: movl %eax, %cr4

but maybe would be better just replace it with

	xorl	%eax, %eax
	movl	%eax, %cr4

or there is some nonobvious reason not to do so?

- Cyrill -

reviewer
#3 lore raw
Quoted 16 lines · Cyrill Gorcunov

[H. Peter Anvin - Fri, Mar 21, 2008 at 01:18:13PM -0700]

Quoted 8 lines
Quoted 3 lines · Cyrill Gorcunov

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

Signed-off-by Cyrill Gorcunov <[email protected]>

Looks good!

-hpa

Thanks Peter for review, btw we have in relocate_kernel_32.S

188: movl %cr4, %eax
189: andl $0, %eax
190: movl %eax, %cr4

Quoted 7 lines

but maybe would be better just replace it with

	xorl	%eax, %eax
	movl	%eax, %cr4

or there is some nonobvious reason not to do so?

The only case I can think of would be if the code expects to patch the zero, but even then it'd be cantankerous, because gas will optimize that zero down to a single byte.

So, no :)

-hpa

author
#4 lore raw

[H. Peter Anvin - Fri, Mar 21, 2008 at 01:50:33PM -0700]

Quoted 19 lines
Quoted 9 lines · Cyrill Gorcunov

[H. Peter Anvin - Fri, Mar 21, 2008 at 01:18:13PM -0700]

Quoted 8 lines
Quoted 3 lines · Cyrill Gorcunov

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

Signed-off-by Cyrill Gorcunov <[email protected]>

Looks good!

-hpa

Quoted 8 lines

Thanks Peter for review, btw we have in relocate_kernel_32.S
188: movl %cr4, %eax
189: andl $0, %eax
190: movl %eax, %cr4
but maybe would be better just replace it with

	xorl	%eax, %eax
	movl	%eax, %cr4

or there is some nonobvious reason not to do so?

Quoted 9 lines

The only case I can think of would be if the code expects to patch the zero, but even then it'd be cantankerous, because gas will optimize that zero down to a single byte.

So, no :)

-hpa

ah! thanks ;)

- Cyrill -

author
#5 lore raw
In reply to H. Peter Anvin

[H. Peter Anvin - Fri, Mar 21, 2008 at 01:50:33PM -0700]

Quoted 19 lines
Quoted 9 lines · Cyrill Gorcunov

[H. Peter Anvin - Fri, Mar 21, 2008 at 01:18:13PM -0700]

Quoted 8 lines
Quoted 3 lines · Cyrill Gorcunov

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

Signed-off-by Cyrill Gorcunov <[email protected]>

Looks good!

-hpa

Quoted 8 lines

Thanks Peter for review, btw we have in relocate_kernel_32.S
188: movl %cr4, %eax
189: andl $0, %eax
190: movl %eax, %cr4
but maybe would be better just replace it with

	xorl	%eax, %eax
	movl	%eax, %cr4

or there is some nonobvious reason not to do so?

Quoted 8 lines

The only case I can think of would be if the code expects to patch the zero, but even then it'd be cantankerous, because gas will optimize that zero down to a single byte.

So, no :)

-hpa

well, Peter, it seems I'm too tired on this week and misinterpred your message, my prev message with text "ah..." is wrong, i'll patch this code later (almost sleeping now ;)

- Cyrill -

#6 lore raw
In reply to Cyrill Gorcunov
Quoted 3 lines · Cyrill Gorcunov

This patch does clean up relocate_kernel_(32|64).S a bit by getting rid of local PAGE_ALIGNED macro. We should use well-known PAGE_SIZE instead

thanks Cyrill, applied.

Ingo