[Xen-devel] [PATCH] x86/suspend: Don't save/restore %cr8

Andrew Cooper posted 1 patch 4 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/xen tags/patchew/20190715171034.29709-1-andrew.cooper3@citrix.com
xen/arch/x86/acpi/wakeup_prot.S | 15 ---------------
1 file changed, 15 deletions(-)
[Xen-devel] [PATCH] x86/suspend: Don't save/restore %cr8
Posted by Andrew Cooper 4 years, 9 months ago
%cr8 is an alias of APIC_TASKPRI, which is handled by
lapic_{suspend,resume}() with the rest of the Local APIC state.  Saving
and restoring the TPR state in isolation is not a clever idea.

Drop it all.

While editing wakeup_prot.S, trim its include list to just the headers
which are used, which is precicely none of them.

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>
CC: Juergen Gross <jgross@suse.com>

This is a Xen mirror to my Linux patch of the same effect:
https://lore.kernel.org/lkml/20190715151641.29210-1-andrew.cooper3@citrix.com/T/#u

With a bit of care, I'm pretty sure the whole of wakeup_prot.S can
disappear, but -ETIME right now.

I've confirmed that after resume TPR retains its value of 0x10.  However, all
attempts to debug the internals of lapic_suspend/resume have eluded me,
including manually poking the UART.  Again, -ETIME to investigate further.
---
 xen/arch/x86/acpi/wakeup_prot.S | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/xen/arch/x86/acpi/wakeup_prot.S b/xen/arch/x86/acpi/wakeup_prot.S
index 361751d290..4a92627436 100644
--- a/xen/arch/x86/acpi/wakeup_prot.S
+++ b/xen/arch/x86/acpi/wakeup_prot.S
@@ -1,13 +1,5 @@
         .file __FILE__
         .text
-
-#include <xen/multiboot.h>
-#include <public/xen.h>
-#include <asm/asm_defns.h>
-#include <asm/desc.h>
-#include <asm/page.h>
-#include <asm/msr.h>
-
         .code64
 
 #define GREG(x)         %r##x
@@ -40,9 +32,6 @@ ENTRY(do_suspend_lowlevel)
         pushfq;
         popq    SAVED_GREG(flags)
 
-        mov     %cr8, GREG(ax)
-        mov     GREG(ax), REF(saved_cr8)
-
         mov     %ss, REF(saved_ss)
 
         sgdt    REF(saved_gdt)
@@ -90,9 +79,6 @@ ENTRY(__ret_point)
         pushq   %rax
         lretq
 1:
-        mov     REF(saved_cr8), %rax
-        mov     %rax, %cr8
-
         pushq   SAVED_GREG(flags)
         popfq
 
@@ -149,4 +135,3 @@ saved_ldt:      .quad   0,0
 
 saved_cr0:      .quad   0
 saved_cr3:      .quad   0
-saved_cr8:      .quad   0
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH] x86/suspend: Don't save/restore %cr8
Posted by Jan Beulich 4 years, 9 months ago
On 15.07.2019 19:10, Andrew Cooper wrote:
> %cr8 is an alias of APIC_TASKPRI, which is handled by
> lapic_{suspend,resume}() with the rest of the Local APIC state.  Saving
> and restoring the TPR state in isolation is not a clever idea.
> 
> Drop it all.
> 
> While editing wakeup_prot.S, trim its include list to just the headers
> which are used, which is precicely none of them.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel