arch/x86/sched.c | 32 +++++++++----------------------- include/x86/os.h | 19 +++++++++++-------- 2 files changed, 20 insertions(+), 31 deletions(-)
Instead of having #ifdefs sprinkled around in x86 code, add some
macros defining constants for asm statements to address differences
between 32- and 64-bit mode.
Modify existing code to use those macros.
Signed-off-by: Juergen Gross <jgross@suse.com>
---
arch/x86/sched.c | 32 +++++++++-----------------------
include/x86/os.h | 19 +++++++++++--------
2 files changed, 20 insertions(+), 31 deletions(-)
diff --git a/arch/x86/sched.c b/arch/x86/sched.c
index dabe6fd6..460dea2e 100644
--- a/arch/x86/sched.c
+++ b/arch/x86/sched.c
@@ -60,16 +60,10 @@ void dump_stack(struct thread *thread)
unsigned long *bottom = (unsigned long *)(thread->stack + STACK_SIZE);
unsigned long *pointer = (unsigned long *)thread->sp;
int count;
- if(thread == current)
- {
-#ifdef __i386__
- asm("movl %%esp,%0"
- : "=r"(pointer));
-#else
- asm("movq %%rsp,%0"
- : "=r"(pointer));
-#endif
- }
+
+ if ( thread == current )
+ asm(ASM_MOV" "ASM_SP",%0" : "=r"(pointer));
+
printk("The stack for \"%s\"\n", thread->name);
for(count = 0; count < 25 && pointer < bottom; count ++)
{
@@ -119,20 +113,12 @@ struct thread* arch_create_thread(char *name, void (*function)(void *),
void run_idle_thread(void)
{
- /* Switch stacks and run the thread */
-#if defined(__i386__)
- __asm__ __volatile__("mov %0,%%esp\n\t"
- "push %1\n\t"
- "ret"
+ /* Switch stacks and run the thread */
+ __asm__ __volatile__("mov %0,"ASM_SP"\n\t"
+ "push %1\n\t"
+ "ret"
:"=m" (idle_thread->sp)
- :"m" (idle_thread->ip));
-#elif defined(__x86_64__)
- __asm__ __volatile__("mov %0,%%rsp\n\t"
- "push %1\n\t"
- "ret"
- :"=m" (idle_thread->sp)
- :"m" (idle_thread->ip));
-#endif
+ :"m" (idle_thread->ip));
}
unsigned long __local_irq_save(void)
diff --git a/include/x86/os.h b/include/x86/os.h
index ee34d784..485d90b8 100644
--- a/include/x86/os.h
+++ b/include/x86/os.h
@@ -77,6 +77,17 @@ int arch_suspend(void);
void arch_post_suspend(int canceled);
void arch_fini(void);
+#if defined(__i386__)
+#define __SZ "l"
+#define __REG "e"
+#else
+#define __SZ "q"
+#define __REG "r"
+#endif
+
+#define ASM_SP "%%"__REG"sp"
+#define ASM_MOV "mov"__SZ
+
#ifdef CONFIG_PARAVIRT
/*
@@ -141,14 +152,6 @@ do { \
#else
-#if defined(__i386__)
-#define __SZ "l"
-#define __REG "e"
-#else
-#define __SZ "q"
-#define __REG "r"
-#endif
-
#define __cli() asm volatile ( "cli" : : : "memory" )
#define __sti() asm volatile ( "sti" : : : "memory" )
--
2.35.3
On 16/04/2024 8:11 am, Juergen Gross wrote: > diff --git a/arch/x86/sched.c b/arch/x86/sched.c > index dabe6fd6..460dea2e 100644 > --- a/arch/x86/sched.c > +++ b/arch/x86/sched.c > @@ -119,20 +113,12 @@ struct thread* arch_create_thread(char *name, void (*function)(void *), > > void run_idle_thread(void) > { > - /* Switch stacks and run the thread */ > -#if defined(__i386__) > - __asm__ __volatile__("mov %0,%%esp\n\t" > - "push %1\n\t" > - "ret" > + /* Switch stacks and run the thread */ > + __asm__ __volatile__("mov %0,"ASM_SP"\n\t" > + "push %1\n\t" > + "ret" > :"=m" (idle_thread->sp) > - :"m" (idle_thread->ip)); > -#elif defined(__x86_64__) > - __asm__ __volatile__("mov %0,%%rsp\n\t" > - "push %1\n\t" > - "ret" > - :"=m" (idle_thread->sp) > - :"m" (idle_thread->ip)); > -#endif > + :"m" (idle_thread->ip)); I know you're only transforming the existing logic, but this is dodgy in several ways. First, PUSH/RET is more commonly spelt JMP. Second, sp is an input parameter not an output, so I'm pretty sure this only works by luck. 00000000000001ee <run_idle_thread>: 1ee: 55 push %rbp 1ef: 48 89 e5 mov %rsp,%rbp 1f2: 48 8b 05 00 00 00 00 mov 0x0(%rip),%rax # 1f9 <run_idle_thread+0xb> 1f9: 48 8b 15 00 00 00 00 mov 0x0(%rip),%rdx # 200 <run_idle_thread+0x12> 200: 48 8b 60 10 mov 0x10(%rax),%rsp 204: ff 72 18 pushq 0x18(%rdx) 207: c3 retq 208: 90 nop 209: 5d pop %rbp 20a: c3 retq This only works because the address constructed for sp's "output" is happens to be on a single-parameter instruction where's its good as an input too. Anyway, this is a much better way of writing it: asm volatile ("mov %[sp], %%"ASM_SP"\n\t" "jmp *%[ip]\n\t" : : [sp] "m" (idle_thread->sp), [ip] "m" (idle_thread->ip)); and also highlights that run_idle_thread() should be marked noreturn. > } > > unsigned long __local_irq_save(void) > diff --git a/include/x86/os.h b/include/x86/os.h > index ee34d784..485d90b8 100644 > --- a/include/x86/os.h > +++ b/include/x86/os.h > @@ -77,6 +77,17 @@ int arch_suspend(void); > void arch_post_suspend(int canceled); > void arch_fini(void); > > +#if defined(__i386__) > +#define __SZ "l" > +#define __REG "e" > +#else > +#define __SZ "q" > +#define __REG "r" > +#endif > + > +#define ASM_SP "%%"__REG"sp" The %% should be at the usage sites, not here. That way, you can use ASM_SP in e.g. file-scope asm where it's spelt with a single %. > +#define ASM_MOV "mov"__SZ There's no need for ASM_MOV. Regular plain mov (no suffix) will work in all the cases you're trying to use it, and makes the asm easier to read. Notice how run_idle_thread() is already like this. ~Andrew
© 2016 - 2024 Red Hat, Inc.