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

[FionaLi-oc <FionaLi-oc@xxxxxxxxxxx>]

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@xxxxxxxxxx>
> Sent: Thursday, April 25, 2019 9:42 PM
> To: Jan Beulich <JBeulich@xxxxxxxx>; FionaLi-oc <FionaLi-oc@xxxxxxxxxxx>
> Cc: Roger Pau Monne <roger.pau@xxxxxxxxxx>; Wei Liu <wei.liu2@xxxxxxxxxx>;
> xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>; Cobe Chen(BJ-RD)
> <CobeChen@xxxxxxxxxxx>
> 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@xxxxxxxxxx> wrote:
> >> On 25/04/2019 14:21, Jan Beulich wrote:
> >>>>>> On 19.04.19 at 10:50, <fionali-oc@xxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel