Re: [PATCH v9 06/11] x86/entry: Organise the use of MSR_SPEC_CTRL at each entry/exit point

["Jan Beulich" <JBeulich@suse.com>]

>>> On 22.01.18 at 12:42, <andrew.cooper3@citrix.com> wrote:
> On 19/01/18 13:51, Jan Beulich wrote:
>>>>> On 19.01.18 at 14:36, <andrew.cooper3@citrix.com> wrote:
>>> On 19/01/18 11:43, Jan Beulich wrote:
>>>>>>> On 18.01.18 at 16:46, <andrew.cooper3@citrix.com> wrote:
>>>>> @@ -729,6 +760,9 @@ ENTRY(nmi)
>>>>>  handle_ist_exception:
>>>>>          SAVE_ALL CLAC
>>>>>  
>>>>> +        SPEC_CTRL_ENTRY_FROM_INTR /* Req: %rsp=regs, Clob: acd */
>>>>> +        /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
>>>> Following my considerations towards alternative patching to
>>>> eliminate as much overhead as possible from the Meltdown
>>>> band-aid in case it is being disabled, I'm rather hesitant to see any
>>>> patchable code being introduced into the NMI/#MC entry paths
>>>> without the patching logic first being made safe in this regard.
>>>> Exceptions coming here aren't very frequent (except perhaps on
>>>> hardware about to die), so the path isn't performance critical.
>>>> Therefore I think we should try to avoid any patching here, and
>>>> just conditionals instead. This in fact is one of the reasons why I
>>>> didn't want to macro-ize the assembly additions done in the
>>>> Meltdown band-aid.
>>>>
>>>> I do realize that this then also affects the exit-to-Xen path,
>>>> which I agree is less desirable to use conditionals on.
>>> While I agree that our lack of IST-safe patching is a problem, these
>>> alternative points are already present on the NMI and MCE paths, and
>>> need to be.  As a result, the DF handler is in no worse of a position. 
>>> As a perfect example, observe the CLAC in context.
>> Oh, indeed. We should change that.
>>
>>> I could perhaps be talked into making a SPEC_CTRL_ENTRY_FROM_IST variant
>>> which doesn't use alternatives (but IMO this is pointless in the
>>> presence of CLAC), but still don't think it is reasonable to treat DF
>>> differently to NMI/MCE.
>> #DF is debatable: On one hand I can see that if things go wrong,
>> it can equally be raised at any time. Otoh #MC and even more so
>> NMI can be raised _without_ things going (fatally) wrong, i.e. the
>> patching may break a boot which would otherwise have succeeded
>> (whereas the #DF would make the boot fail anyway).
> 
> I don't see a conclusion here, or a reason for treating #DF differently
> to NMI or #MC.

Odd - I thought my reply was pretty clear in this regard. I have
no good idea how to word it differently. Furthermore the goal
of the reply was not to settle on how to treat #DF, but to try
to convince you to avoid adding more patch points to the NMI /
#MC path (if you want #DF treated similarly, I wouldn't
object patching to be avoided there too).

> There is currently a very very slim race on boot where an NMI or #MC
> hitting the main application of alternatives may cause Xen to explode. 
> This has been the case since alternatives were introduced, and this
> patch doesn't make the problem meaningfully worse.

SMAP patching affects 3 bytes (and I'm intending to put together a
patch removing that patching from the NMI / #MC path), while you
add patching of quite a few more bytes, increasing the risk
accordingly.

If you really don't want to switch away from the patching approach,
I won't refuse to ack the patch. But it'll mean subsequent changes
will be more intrusive, to get this converted to conditionals instead
(unless someone has _immediate_ plans to deal with the issues in
the patching logic itself).

Jan


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