Re: [PATCH v2] hypervisor/x86/xen: Unset X86_BUG_SYSRET_SS_ATTRS on Xen PV guests

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 05/05/2015 09:51 AM, Andy Lutomirski wrote:
> On Mon, May 4, 2015 at 8:02 AM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> Commit 61f01dd941ba ("x86_64, asm: Work around AMD SYSRET SS descriptor
>> attribute issue") makes AMD processors set SS to __KERNEL_DS in
>> __switch_to() to deal with cases when SS is NULL.
>>
>> This breaks Xen PV guests who do not want to load SS with__KERNEL_DS.
>>
>> Since the problem that the commit is trying to address would have to be
>> fixed in the hypervisor (if it in fact exists under Xen) there is no
>> reason to set X86_BUG_SYSRET_SS_ATTRS flag for PV VPCUs here.
>>
>> This can be easily achieved by adding x86_hyper_xen_hvm.set_cpu_features
>> op which will clear this flag. (And since this structure is no longer
>> HVM-specific we should do some renaming).
> Looks good to me, but:
>
>>   static void __init xen_hvm_guest_init(void)
>>   {
>> +       if (xen_pv_domain())
>> +               return;
>> +
> How can a platform be "hvm" and "pv"?  Is that the PVHVM thing?

No, this is meant for "true" PV guest not to go through HVM-specific 
initialization.

We have to do it since now x86_hyper_xen_hvm (renamed to x86_hyper_xen) 
will have init_platform() op and so init_hypervisor_platform() will call 
it obviously without knowing the type of the guest.

>
>> +static void xen_set_cpu_features(struct cpuinfo_x86 *c)
>> +{
>> +       if (xen_pv_domain())
>> +               clear_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
>> +}
> I haven't followed all the twists and turns, but, if the guest
> platform is such that the guest kernel is really executing SYSRET and
> we're on AMD, then we have the sysret_ss_attrs bug.  If PVHVM has
> xen_pv_domain() returning true but also uses SYSRET for real, then
> this looks wrong.

HVM guests execute SYSRET and they will continue having 
X86_BUG_SYSRET_SS_ATTRS flag on. PV guests OTOH don't do SYSRET, the 
hypervisor does (which is why if this issue is to be fixed it would have 
to be done in the hypervisor).

-boris