[PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[FionaLi-oc]

When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to
save or restore a set of MSRs.

Signed-off-by: FionaLi-oc <fionali-oc@zhaoxin.com>
---
 xen/arch/x86/acpi/suspend.c | 6 ++++--
 xen/arch/x86/x86_64/traps.c | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/acpi/suspend.c b/xen/arch/x86/acpi/suspend.c
index 9e69bf2..b0ef1c2 100644
--- a/xen/arch/x86/acpi/suspend.c
+++ b/xen/arch/x86/acpi/suspend.c
@@ -27,7 +27,8 @@ void save_rest_processor_state(void)
     rdmsrl(MSR_SHADOW_GS_BASE, saved_kernel_gs_base);
     rdmsrl(MSR_CSTAR, saved_cstar);
     rdmsrl(MSR_LSTAR, saved_lstar);
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
+    if ( boot_cpu_data.x86_vendor &
+        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
     {
         rdmsrl(MSR_IA32_SYSENTER_ESP, saved_sysenter_esp);
         rdmsrl(MSR_IA32_SYSENTER_EIP, saved_sysenter_eip);
@@ -51,7 +52,8 @@ void restore_rest_processor_state(void)
     wrgsbase(saved_gs_base);
     wrmsrl(MSR_SHADOW_GS_BASE, saved_kernel_gs_base);
 
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
+    if ( boot_cpu_data.x86_vendor &
+        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
     {
         /* Recover sysenter MSRs */
         wrmsrl(MSR_IA32_SYSENTER_ESP, saved_sysenter_esp);
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 44af765..6506d6a 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
                                    (unsigned long)lstar_enter);
     stub_va += offset;
 
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
+    if ( boot_cpu_data.x86_vendor &
+        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
     {
         /* SYSENTER entry. */
         wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
-- 
1.8.3.1


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

[Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[FionaLi-oc]

When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to
save or restore a set of MSRs.

Signed-off-by: FionaLi-oc <fionali-oc@zhaoxin.com>
---
 xen/arch/x86/acpi/suspend.c | 6 ++++--
 xen/arch/x86/x86_64/traps.c | 3 ++-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/arch/x86/acpi/suspend.c b/xen/arch/x86/acpi/suspend.c
index 9e69bf2..b0ef1c2 100644
--- a/xen/arch/x86/acpi/suspend.c
+++ b/xen/arch/x86/acpi/suspend.c
@@ -27,7 +27,8 @@ void save_rest_processor_state(void)
     rdmsrl(MSR_SHADOW_GS_BASE, saved_kernel_gs_base);
     rdmsrl(MSR_CSTAR, saved_cstar);
     rdmsrl(MSR_LSTAR, saved_lstar);
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
+    if ( boot_cpu_data.x86_vendor &
+        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
     {
         rdmsrl(MSR_IA32_SYSENTER_ESP, saved_sysenter_esp);
         rdmsrl(MSR_IA32_SYSENTER_EIP, saved_sysenter_eip);
@@ -51,7 +52,8 @@ void restore_rest_processor_state(void)
     wrgsbase(saved_gs_base);
     wrmsrl(MSR_SHADOW_GS_BASE, saved_kernel_gs_base);
 
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
+    if ( boot_cpu_data.x86_vendor &
+        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
     {
         /* Recover sysenter MSRs */
         wrmsrl(MSR_IA32_SYSENTER_ESP, saved_sysenter_esp);
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 44af765..6506d6a 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
                                    (unsigned long)lstar_enter);
     stub_va += offset;
 
-    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
+    if ( boot_cpu_data.x86_vendor &
+        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
     {
         /* SYSENTER entry. */
         wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
-- 
1.8.3.1


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

Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Andrew Cooper]

On 19/04/2019 09:50, FionaLi-oc wrote:
> When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to
> save or restore a set of MSRs.
>
> Signed-off-by: FionaLi-oc <fionali-oc@zhaoxin.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I've pulled this into x86-next, and it will get committed when CI has
been repaired and is working again.

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

Re: [Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Andrew Cooper]

On 19/04/2019 09:50, FionaLi-oc wrote:
> When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to
> save or restore a set of MSRs.
>
> Signed-off-by: FionaLi-oc <fionali-oc@zhaoxin.com>

Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>

I've pulled this into x86-next, and it will get committed when CI has
been repaired and is working again.

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

Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Jan Beulich]

>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
> When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to

SYSENTER

> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>                                     (unsigned long)lstar_enter);
>      stub_va += offset;
>  
> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
> +    if ( boot_cpu_data.x86_vendor &
> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>      {
>          /* SYSENTER entry. */
>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);

How is this hunk related to the title of the change? I think the
title wants to be adjusted.

Furthermore for all of the changes done, wouldn't we better
switch to use cpu_has_sep? init_amd() as well as default_init()
already clear this flag. Andrew, thoughts?

Jan



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

Re: [Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Jan Beulich]

>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
> When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to

SYSENTER

> --- a/xen/arch/x86/x86_64/traps.c
> +++ b/xen/arch/x86/x86_64/traps.c
> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>                                     (unsigned long)lstar_enter);
>      stub_va += offset;
>  
> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
> +    if ( boot_cpu_data.x86_vendor &
> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>      {
>          /* SYSENTER entry. */
>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);

How is this hunk related to the title of the change? I think the
title wants to be adjusted.

Furthermore for all of the changes done, wouldn't we better
switch to use cpu_has_sep? init_amd() as well as default_init()
already clear this flag. Andrew, thoughts?

Jan



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

Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Andrew Cooper]

On 25/04/2019 14:21, Jan Beulich wrote:
>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
>> When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to
> SYSENTER
>
>> --- a/xen/arch/x86/x86_64/traps.c
>> +++ b/xen/arch/x86/x86_64/traps.c
>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>>                                     (unsigned long)lstar_enter);
>>      stub_va += offset;
>>  
>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
>> +    if ( boot_cpu_data.x86_vendor &
>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>>      {
>>          /* SYSENTER entry. */
>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
> How is this hunk related to the title of the change? I think the
> title wants to be adjusted.
>
> Furthermore for all of the changes done, wouldn't we better
> switch to use cpu_has_sep? init_amd() as well as default_init()
> already clear this flag. Andrew, thoughts?

I wondered exactly that after queuing this patch, but didn't get around
to experimenting.

We have to be a little careful with the ordering of operations. 
cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon before
cpu_has_sep is safe to use (although for the MSRs, it should be safe).

~Andrew

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

Re: [Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Andrew Cooper]

On 25/04/2019 14:21, Jan Beulich wrote:
>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
>> When executing SYSEXIT or SYSENTRY in Zhaoxin CPU, CPU needs to
> SYSENTER
>
>> --- a/xen/arch/x86/x86_64/traps.c
>> +++ b/xen/arch/x86/x86_64/traps.c
>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>>                                     (unsigned long)lstar_enter);
>>      stub_va += offset;
>>  
>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) )
>> +    if ( boot_cpu_data.x86_vendor &
>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>>      {
>>          /* SYSENTER entry. */
>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
> How is this hunk related to the title of the change? I think the
> title wants to be adjusted.
>
> Furthermore for all of the changes done, wouldn't we better
> switch to use cpu_has_sep? init_amd() as well as default_init()
> already clear this flag. Andrew, thoughts?

I wondered exactly that after queuing this patch, but didn't get around
to experimenting.

We have to be a little careful with the ordering of operations. 
cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon before
cpu_has_sep is safe to use (although for the MSRs, it should be safe).

~Andrew

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

Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Jan Beulich]

>>> On 25.04.19 at 15:25, <andrew.cooper3@citrix.com> wrote:
> On 25/04/2019 14:21, Jan Beulich wrote:
>>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
>>> --- a/xen/arch/x86/x86_64/traps.c
>>> +++ b/xen/arch/x86/x86_64/traps.c
>>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>>>                                     (unsigned long)lstar_enter);
>>>      stub_va += offset;
>>>  
>>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) 
> )
>>> +    if ( boot_cpu_data.x86_vendor &
>>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>>>      {
>>>          /* SYSENTER entry. */
>>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
>> How is this hunk related to the title of the change? I think the
>> title wants to be adjusted.
>>
>> Furthermore for all of the changes done, wouldn't we better
>> switch to use cpu_has_sep? init_amd() as well as default_init()
>> already clear this flag. Andrew, thoughts?
> 
> I wondered exactly that after queuing this patch, but didn't get around
> to experimenting.
> 
> We have to be a little careful with the ordering of operations. 
> cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon before
> cpu_has_sep is safe to use (although for the MSRs, it should be safe).

trap_init() (and hence percpu_traps_init()) runs after, in
particular, init_speculation_mitigations(), which means all
feature collection and massaging must have happened. So
unless you explicitly say otherwise, I'd like to ask FionaLi
to submit a v2 with that change (and with the other cosmetic
adjustments carried out).

Jan



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

Re: [Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Jan Beulich]

>>> On 25.04.19 at 15:25, <andrew.cooper3@citrix.com> wrote:
> On 25/04/2019 14:21, Jan Beulich wrote:
>>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
>>> --- a/xen/arch/x86/x86_64/traps.c
>>> +++ b/xen/arch/x86/x86_64/traps.c
>>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>>>                                     (unsigned long)lstar_enter);
>>>      stub_va += offset;
>>>  
>>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) 
> )
>>> +    if ( boot_cpu_data.x86_vendor &
>>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>>>      {
>>>          /* SYSENTER entry. */
>>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
>> How is this hunk related to the title of the change? I think the
>> title wants to be adjusted.
>>
>> Furthermore for all of the changes done, wouldn't we better
>> switch to use cpu_has_sep? init_amd() as well as default_init()
>> already clear this flag. Andrew, thoughts?
> 
> I wondered exactly that after queuing this patch, but didn't get around
> to experimenting.
> 
> We have to be a little careful with the ordering of operations. 
> cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon before
> cpu_has_sep is safe to use (although for the MSRs, it should be safe).

trap_init() (and hence percpu_traps_init()) runs after, in
particular, init_speculation_mitigations(), which means all
feature collection and massaging must have happened. So
unless you explicitly say otherwise, I'd like to ask FionaLi
to submit a v2 with that change (and with the other cosmetic
adjustments carried out).

Jan



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

Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Andrew Cooper]

On 25/04/2019 14:37, Jan Beulich wrote:
>>>> On 25.04.19 at 15:25, <andrew.cooper3@citrix.com> wrote:
>> On 25/04/2019 14:21, Jan Beulich wrote:
>>>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
>>>> --- a/xen/arch/x86/x86_64/traps.c
>>>> +++ b/xen/arch/x86/x86_64/traps.c
>>>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>>>>                                     (unsigned long)lstar_enter);
>>>>      stub_va += offset;
>>>>  
>>>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) 
>> )
>>>> +    if ( boot_cpu_data.x86_vendor &
>>>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>>>>      {
>>>>          /* SYSENTER entry. */
>>>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
>>> How is this hunk related to the title of the change? I think the
>>> title wants to be adjusted.
>>>
>>> Furthermore for all of the changes done, wouldn't we better
>>> switch to use cpu_has_sep? init_amd() as well as default_init()
>>> already clear this flag. Andrew, thoughts?
>> I wondered exactly that after queuing this patch, but didn't get around
>> to experimenting.
>>
>> We have to be a little careful with the ordering of operations. 
>> cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon before
>> cpu_has_sep is safe to use (although for the MSRs, it should be safe).
> trap_init() (and hence percpu_traps_init()) runs after, in
> particular, init_speculation_mitigations(), which means all
> feature collection and massaging must have happened. So
> unless you explicitly say otherwise, I'd like to ask FionaLi
> to submit a v2 with that change (and with the other cosmetic
> adjustments carried out).

I'll drop this patch from x86-next.  Overall, I think a cpu_has_sep
based solution would be preferable.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[Andrew Cooper]

On 25/04/2019 14:37, Jan Beulich wrote:
>>>> On 25.04.19 at 15:25, <andrew.cooper3@citrix.com> wrote:
>> On 25/04/2019 14:21, Jan Beulich wrote:
>>>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
>>>> --- a/xen/arch/x86/x86_64/traps.c
>>>> +++ b/xen/arch/x86/x86_64/traps.c
>>>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
>>>>                                     (unsigned long)lstar_enter);
>>>>      stub_va += offset;
>>>>  
>>>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR) 
>> )
>>>> +    if ( boot_cpu_data.x86_vendor &
>>>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR | X86_VENDOR_SHANGHAI) )
>>>>      {
>>>>          /* SYSENTER entry. */
>>>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
>>> How is this hunk related to the title of the change? I think the
>>> title wants to be adjusted.
>>>
>>> Furthermore for all of the changes done, wouldn't we better
>>> switch to use cpu_has_sep? init_amd() as well as default_init()
>>> already clear this flag. Andrew, thoughts?
>> I wondered exactly that after queuing this patch, but didn't get around
>> to experimenting.
>>
>> We have to be a little careful with the ordering of operations. 
>> cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon before
>> cpu_has_sep is safe to use (although for the MSRs, it should be safe).
> trap_init() (and hence percpu_traps_init()) runs after, in
> particular, init_speculation_mitigations(), which means all
> feature collection and massaging must have happened. So
> unless you explicitly say otherwise, I'd like to ask FionaLi
> to submit a v2 with that change (and with the other cosmetic
> adjustments carried out).

I'll drop this patch from x86-next.  Overall, I think a cpu_has_sep
based solution would be preferable.

~Andrew

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

Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[FionaLi-oc]

Andrew，
Do I need to submit a v3 with cpu_has_sep based solution? Or do you deal with it?

> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: Thursday, April 25, 2019 9:42 PM
> To: Jan Beulich <JBeulich@suse.com>; FionaLi-oc <FionaLi-oc@zhaoxin.com>
> Cc: Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wei.liu2@citrix.com>;
> xen-devel <xen-devel@lists.xenproject.org>; Cobe Chen(BJ-RD)
> <CobeChen@zhaoxin.com>
> Subject: Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for
> Zhaoxin CPU
> 
> On 25/04/2019 14:37, Jan Beulich wrote:
> >>>> On 25.04.19 at 15:25, <andrew.cooper3@citrix.com> wrote:
> >> On 25/04/2019 14:21, Jan Beulich wrote:
> >>>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
> >>>> --- a/xen/arch/x86/x86_64/traps.c
> >>>> +++ b/xen/arch/x86/x86_64/traps.c
> >>>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
> >>>>                                     (unsigned long)lstar_enter);
> >>>>      stub_va += offset;
> >>>>
> >>>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL |
> X86_VENDOR_CENTAUR)
> >> )
> >>>> +    if ( boot_cpu_data.x86_vendor &
> >>>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR |
> >>>> + X86_VENDOR_SHANGHAI) )
> >>>>      {
> >>>>          /* SYSENTER entry. */
> >>>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
> >>> How is this hunk related to the title of the change? I think the
> >>> title wants to be adjusted.
> >>>
> >>> Furthermore for all of the changes done, wouldn't we better switch
> >>> to use cpu_has_sep? init_amd() as well as default_init() already
> >>> clear this flag. Andrew, thoughts?
> >> I wondered exactly that after queuing this patch, but didn't get
> >> around to experimenting.
> >>
> >> We have to be a little careful with the ordering of operations.
> >> cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon
> >> before cpu_has_sep is safe to use (although for the MSRs, it should be safe).
> > trap_init() (and hence percpu_traps_init()) runs after, in particular,
> > init_speculation_mitigations(), which means all feature collection and
> > massaging must have happened. So unless you explicitly say otherwise,
> > I'd like to ask FionaLi to submit a v2 with that change (and with the
> > other cosmetic adjustments carried out).
> 
> I'll drop this patch from x86-next.  Overall, I think a cpu_has_sep based
> solution would be preferable.
> 
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for Zhaoxin CPU

[FionaLi-oc]

Andrew，
Do I need to submit a v3 with cpu_has_sep based solution? Or do you deal with it?

> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: Thursday, April 25, 2019 9:42 PM
> To: Jan Beulich <JBeulich@suse.com>; FionaLi-oc <FionaLi-oc@zhaoxin.com>
> Cc: Roger Pau Monne <roger.pau@citrix.com>; Wei Liu <wei.liu2@citrix.com>;
> xen-devel <xen-devel@lists.xenproject.org>; Cobe Chen(BJ-RD)
> <CobeChen@zhaoxin.com>
> Subject: Re: [PATCH 2/2 v2] x86/acpi: Improve suspend and resume process for
> Zhaoxin CPU
> 
> On 25/04/2019 14:37, Jan Beulich wrote:
> >>>> On 25.04.19 at 15:25, <andrew.cooper3@citrix.com> wrote:
> >> On 25/04/2019 14:21, Jan Beulich wrote:
> >>>>>> On 19.04.19 at 10:50, <fionali-oc@zhaoxin.com> wrote:
> >>>> --- a/xen/arch/x86/x86_64/traps.c
> >>>> +++ b/xen/arch/x86/x86_64/traps.c
> >>>> @@ -334,7 +334,8 @@ void subarch_percpu_traps_init(void)
> >>>>                                     (unsigned long)lstar_enter);
> >>>>      stub_va += offset;
> >>>>
> >>>> -    if ( boot_cpu_data.x86_vendor & (X86_VENDOR_INTEL |
> X86_VENDOR_CENTAUR)
> >> )
> >>>> +    if ( boot_cpu_data.x86_vendor &
> >>>> +        (X86_VENDOR_INTEL | X86_VENDOR_CENTAUR |
> >>>> + X86_VENDOR_SHANGHAI) )
> >>>>      {
> >>>>          /* SYSENTER entry. */
> >>>>          wrmsrl(MSR_IA32_SYSENTER_ESP, stack_bottom);
> >>> How is this hunk related to the title of the change? I think the
> >>> title wants to be adjusted.
> >>>
> >>> Furthermore for all of the changes done, wouldn't we better switch
> >>> to use cpu_has_sep? init_amd() as well as default_init() already
> >>> clear this flag. Andrew, thoughts?
> >> I wondered exactly that after queuing this patch, but didn't get
> >> around to experimenting.
> >>
> >> We have to be a little careful with the ordering of operations.
> >> cpu_has_sep is visible in CPUID but needs clobbering in AMD/Hygon
> >> before cpu_has_sep is safe to use (although for the MSRs, it should be safe).
> > trap_init() (and hence percpu_traps_init()) runs after, in particular,
> > init_speculation_mitigations(), which means all feature collection and
> > massaging must have happened. So unless you explicitly say otherwise,
> > I'd like to ask FionaLi to submit a v2 with that change (and with the
> > other cosmetic adjustments carried out).
> 
> I'll drop this patch from x86-next.  Overall, I think a cpu_has_sep based
> solution would be preferable.
> 
> ~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel