On 25/04/2019 14:37, Jan Beulich wrote:
>>>> On 25.04.19 at 15:25, <andrew.coop...@citrix.com> wrote:
>> On 25/04/2019 14:21, Jan Beulich wrote:
>>>>>> On 19.04.19 at 10:50, <fionali...@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
