Message ID | 20191213190436.24475-4-andrew.cooper3@citrix.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/suspend: State cleanup | expand |
On Fri, Dec 13, 2019 at 07:04:33PM +0000, Andrew Cooper wrote: > Only the callee-preserved registers need saving/restoring. Spill them to the > stack like regular functions do. %rsp is now the only GPR which gets stashed > in .data > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > --- > CC: Jan Beulich <JBeulich@suse.com> > CC: Wei Liu <wl@xen.org> > CC: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/acpi/wakeup_prot.S | 59 +++++++++-------------------------------- > 1 file changed, 12 insertions(+), 47 deletions(-) > > diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S > index 35fd7a5e9f..2f6c8e18ef 100644 > --- a/xen/arch/x86/acpi/wakeup_prot.S > +++ b/xen/arch/x86/acpi/wakeup_prot.S > @@ -11,24 +11,14 @@ > #define REF(x) x(%rip) > > ENTRY(do_suspend_lowlevel) > + push %rbp > + push %rbx > + push %r12 > + push %r13 > + push %r14 > + push %r15 I was expecting Xen had a macro for this (and the restore counterpart), but I haven't found any (neither any other places where it would be useful). Thanks, Roger.
On 17/12/2019 12:04, Roger Pau Monné wrote: >> diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S >> index 35fd7a5e9f..2f6c8e18ef 100644 >> --- a/xen/arch/x86/acpi/wakeup_prot.S >> +++ b/xen/arch/x86/acpi/wakeup_prot.S >> @@ -11,24 +11,14 @@ >> #define REF(x) x(%rip) >> >> ENTRY(do_suspend_lowlevel) >> + push %rbp >> + push %rbx >> + push %r12 >> + push %r13 >> + push %r14 >> + push %r15 > I was expecting Xen had a macro for this (and the restore > counterpart), but I haven't found any (neither any other places where > it would be useful). We have macros for saving and restoring all GPRs as part of an exception, but this is just regular function prologue/epilogue logic (which happens to be hand written asm because this function isn't quite a regular function). ~Andrew
On Tue, Dec 17, 2019 at 12:10:55PM +0000, Andrew Cooper wrote: > On 17/12/2019 12:04, Roger Pau Monné wrote: > >> diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S > >> index 35fd7a5e9f..2f6c8e18ef 100644 > >> --- a/xen/arch/x86/acpi/wakeup_prot.S > >> +++ b/xen/arch/x86/acpi/wakeup_prot.S > >> @@ -11,24 +11,14 @@ > >> #define REF(x) x(%rip) > >> > >> ENTRY(do_suspend_lowlevel) > >> + push %rbp > >> + push %rbx > >> + push %r12 > >> + push %r13 > >> + push %r14 > >> + push %r15 > > I was expecting Xen had a macro for this (and the restore > > counterpart), but I haven't found any (neither any other places where > > it would be useful). > > We have macros for saving and restoring all GPRs as part of an > exception, but this is just regular function prologue/epilogue logic > (which happens to be hand written asm because this function isn't quite > a regular function). Yes, I've found SAVE_ALL, but as you say that's overkill (and it's partly what you are trying to avoid here). Thanks, Roger.
diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S index 35fd7a5e9f..2f6c8e18ef 100644 --- a/xen/arch/x86/acpi/wakeup_prot.S +++ b/xen/arch/x86/acpi/wakeup_prot.S @@ -11,24 +11,14 @@ #define REF(x) x(%rip) ENTRY(do_suspend_lowlevel) + push %rbp + push %rbx + push %r12 + push %r13 + push %r14 + push %r15 SAVE_GREG(sp) - SAVE_GREG(ax) - SAVE_GREG(bx) - SAVE_GREG(cx) - SAVE_GREG(dx) - SAVE_GREG(bp) - SAVE_GREG(si) - SAVE_GREG(di) - - SAVE_GREG(8) # save r8...r15 - SAVE_GREG(9) - SAVE_GREG(10) - SAVE_GREG(11) - SAVE_GREG(12) - SAVE_GREG(13) - SAVE_GREG(14) - SAVE_GREG(15) mov %cr0, GREG(ax) mov GREG(ax), REF(saved_cr0) @@ -75,22 +65,13 @@ ENTRY(s3_resume) call restore_rest_processor_state - LOAD_GREG(bp) - LOAD_GREG(ax) - LOAD_GREG(bx) - LOAD_GREG(cx) - LOAD_GREG(dx) - LOAD_GREG(si) - LOAD_GREG(di) - LOAD_GREG(8) # save r8...r15 - LOAD_GREG(9) - LOAD_GREG(10) - LOAD_GREG(11) - LOAD_GREG(12) - LOAD_GREG(13) - LOAD_GREG(14) - LOAD_GREG(15) .Lsuspend_err: + pop %r15 + pop %r14 + pop %r13 + pop %r12 + pop %rbx + pop %rbp ret .data @@ -101,21 +82,5 @@ GLOBAL(saved_magic) .align 8 DECLARE_GREG(sp) -DECLARE_GREG(bp) -DECLARE_GREG(ax) -DECLARE_GREG(bx) -DECLARE_GREG(cx) -DECLARE_GREG(dx) -DECLARE_GREG(si) -DECLARE_GREG(di) - -DECLARE_GREG(8) -DECLARE_GREG(9) -DECLARE_GREG(10) -DECLARE_GREG(11) -DECLARE_GREG(12) -DECLARE_GREG(13) -DECLARE_GREG(14) -DECLARE_GREG(15) saved_cr0: .quad 0
Only the callee-preserved registers need saving/restoring. Spill them to the stack like regular functions do. %rsp is now the only GPR which gets stashed in .data Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Wei Liu <wl@xen.org> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/acpi/wakeup_prot.S | 59 +++++++++-------------------------------- 1 file changed, 12 insertions(+), 47 deletions(-)