Re: [Xen-devel] [PATCH v2 for-4.7/4.8] x86: Fix "x86: further CPUID handling adjustments"

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 18.05.18 at 14:21, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 17/05/18 13:23, Jan Beulich wrote:
>>>>> On 16.05.18 at 19:27, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> c/s 62b187969 "x86: further CPUID handling adjustments" make some 
>>> adjustments.
>>> However, it breaks levelling of guests, making it impossible for the 
>>> toolstack
>>> to hide STIBP or IBPB from guests on hardware with up-to-date microcode.
>>>
>>> The dom0 issue referenced in the commit message was fixed by the hunk
>>> adjusting the zeroing alone.  STIBP and IBPB don't need (and indeed, must 
>>> not
>>> be for levelling purposes) OR'd into the leaf.
>>>
>>> One final item which was missed in backport was the need to ignore the
>>> toolstack choice of STIBP, and set it equal to IBRSB.  This needs doing 
>>> after
>>> the mask has been applied.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> What about the patch below instead? This then allows the tool stack to 
>> override
>> STIBP independent of IBRSB.
>>
>> Jan
>>
>> x86: correct "further CPUID handling adjustments"
>>
>> Commit 62b187969 "x86: further CPUID handling adjustments" went too far,
>> breaking feature levelling of DomU-s. Restrict the PV overrides to just
>> Dom0 and undo the HVM overrides.
> 
> Why?  PV overrides (for dom0 or domU) are equally wrong.

Why do you talk about DomU? Those overrides are being removed by the
proposed patch. For Dom0, otoh, there's no other way to do the "IBRSB
implies both STIBP and IBPB" override we otherwise advertise to the tool
stack via the PV and HVM feature sets. And I can't see why we wouldn't
want to treat Dom0 equally in this regard. Even on master I think we only
do the STIBP part of the above, while the IBPB part is only done for the
feature sets.

Furthermore, as indicated before, I'm not convinced overriding STIBP
when the tool stack may have intentionally masked it (as your patch
does, and as I understand is the effect of

    /*
     * Override STIBP to match IBRS.  Guests can safely use STIBP
     * functionality on non-HT hardware, but can't necesserily protect
     * themselves from SP2/Spectre/Branch Target Injection if STIBP is hidden
     * on HT-capable hardware.
     */
    p->feat.stibp = p->feat.ibrsb;

in master) is the right thing to do. Advertising it in the feature set should
be all that's needed.

Jan



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