[PATCH RFC] x86/resume: take care of fully eager FPU around system suspend

[PATCH RFC] x86/resume: take care of fully eager FPU around system suspend

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 1049 bytes --]

Just like in the HVM emulation and EFI runtime call cases we must not
set CR0.TS here in fully eager mode. Note that idle vCPU-s never have
->arch.fully_eager_fpu set (for their initialization not going through
vcpu_init_fpu()), so we won't hit the respective ASSERT() in
vcpu_restore_fpu_eager(). 

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: Not even compile tested, as I'm writing this from home. Also please
         excuse the formatting (hence the attachment) - our mail web frontend
      doesn't allow anything better.

--- a/xen/arch/x86/acpi/suspend.c
+++ b/xen/arch/x86/acpi/suspend.c
@@ -92,8 +92,11 @@ void restore_rest_processor_state(void)
         write_debugreg(7, curr->arch.debugreg[7]);
     }
 
-    /* Reload FPU state on next FPU use. */
-    stts();
+    /* Reload FPU state immediately or on next FPU use. */
+    if ( curr->arch.fully_eager_fpu )
+        vcpu_restore_fpu_eager(curr);
+    else
+        stts();
 
     if (cpu_has_pat)
         wrmsrl(MSR_IA32_CR_PAT, host_pat);


[-- Attachment #2: x86-resume-eager-FPU.patch --]
[-- Type: text/plain, Size: 984 bytes --]

x86/resume: take care of fully eager FPU around system suspend

Just like in the HVM emulation and EFI runtime call cases we must not
set CR0.TS here in fully eager mode. Note that idle vCPU-s never have
->arch.fully_eager_fpu set (for their initialization not going through
vcpu_init_fpu()), so we won't hit the respective ASSERT() in
vcpu_restore_fpu_eager(). 

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
RFC: Not even compile tested, as I'm writing this from home.

--- a/xen/arch/x86/acpi/suspend.c
+++ b/xen/arch/x86/acpi/suspend.c
@@ -92,8 +92,11 @@ void restore_rest_processor_state(void)
         write_debugreg(7, curr->arch.debugreg[7]);
     }
 
-    /* Reload FPU state on next FPU use. */
-    stts();
+    /* Reload FPU state immediately or on next FPU use. */
+    if ( curr->arch.fully_eager_fpu )
+        vcpu_restore_fpu_eager(curr);
+    else
+        stts();
 
     if (cpu_has_pat)
         wrmsrl(MSR_IA32_CR_PAT, host_pat);

[-- Attachment #3: Type: text/plain, Size: 157 bytes --]

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

Re: [PATCH RFC] x86/resume: take care of fully eager FPU around system suspend

[Andrew Cooper]

On 17/06/18 16:56, Jan Beulich wrote:
> Just like in the HVM emulation and EFI runtime call cases we must not
> set CR0.TS here in fully eager mode. Note that idle vCPU-s never have
> ->arch.fully_eager_fpu set (for their initialization not going through
> vcpu_init_fpu()), so we won't hit the respective ASSERT() in
> vcpu_restore_fpu_eager(). 
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> RFC: Not even compile tested, as I'm writing this from home. Also please
>          excuse the formatting (hence the attachment) - our mail web frontend
>       doesn't allow anything better.
>
> --- a/xen/arch/x86/acpi/suspend.c
> +++ b/xen/arch/x86/acpi/suspend.c
> @@ -92,8 +92,11 @@ void restore_rest_processor_state(void)
>          write_debugreg(7, curr->arch.debugreg[7]);
>      }
>  
> -    /* Reload FPU state on next FPU use. */
> -    stts();
> +    /* Reload FPU state immediately or on next FPU use. */
> +    if ( curr->arch.fully_eager_fpu )
> +        vcpu_restore_fpu_eager(curr);
> +    else
> +        stts();

Hmm - thinking about it, this should either be nothing, or a straight
call to vcpu_restore_fpu_eager(curr).

If it is necessary here, then I cant see how the non-lazy xsave states
currently work.

OTOH, we could probably drop a large quantity of this custom
save/restore logic if we force a transition to full idle before taking
the S3 path, at which point the normal ctxt_switch_to() path should DTRT.

~Andrew

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

Re: [PATCH RFC] x86/resume: take care of fully eager FPU around system suspend

[Jan Beulich]

>>> On 18.06.18 at 10:45, <andrew.cooper3@citrix.com> wrote:
> On 17/06/18 16:56, Jan Beulich wrote:
>> --- a/xen/arch/x86/acpi/suspend.c
>> +++ b/xen/arch/x86/acpi/suspend.c
>> @@ -92,8 +92,11 @@ void restore_rest_processor_state(void)
>>          write_debugreg(7, curr->arch.debugreg[7]);
>>      }
>>  
>> -    /* Reload FPU state on next FPU use. */
>> -    stts();
>> +    /* Reload FPU state immediately or on next FPU use. */
>> +    if ( curr->arch.fully_eager_fpu )
>> +        vcpu_restore_fpu_eager(curr);
>> +    else
>> +        stts();
> 
> Hmm - thinking about it, this should either be nothing, or a straight
> call to vcpu_restore_fpu_eager(curr).
> 
> If it is necessary here, then I cant see how the non-lazy xsave states
> currently work.

Hmm, indeed, and same then in the EFI case.

> OTOH, we could probably drop a large quantity of this custom
> save/restore logic if we force a transition to full idle before taking
> the S3 path, at which point the normal ctxt_switch_to() path should DTRT.

Right, but that's not something I'd consider for 4.11. In particular I'd
want such a change to be throughly (over an extended period of time)
tested by someone actively using S3. It also is not really a good option
for the EFI side of things.

Jan



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