Re: [Xen-devel] [PATCH v10 07/11] x86/entry: Avoid using alternatives in NMI/#MC paths

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.01.18 at 16:19, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 25/01/18 15:14, Jan Beulich wrote:
>>>>> On 25.01.18 at 16:04, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> On 25/01/18 13:43, Jan Beulich wrote:
>>>>>>> On 24.01.18 at 14:12, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>> @@ -256,6 +261,69 @@
>>>>>          DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_SET,           \
>>>>>          DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_CLEAR
>>>>>  
>>>>> +/* TODO: Drop these when the alternatives infrastructure is NMI/#MC 
>>>>> safe. 
> */
>>>>> +.macro SPEC_CTRL_ENTRY_FROM_INTR_IST
>>>>> +/*
>>>>> + * Requires %rsp=regs, %r14=stack_end
>>>>> + * Clobbers %rax, %rcx, %rdx
>>>>> + *
>>>>> + * This is logical merge of DO_OVERWRITE_RSB and DO_SPEC_CTRL_ENTRY
>>>>> + * maybexen=1, but with conditionals rather than alternatives.
>>>>> + */
>>>>> +    movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %eax
>>>>> +
>>>>> +    testb $BTI_IST_RSB, %al
>>>>> +    jz .L\@_skip_rsb
>>>>> +
>>>>> +    DO_OVERWRITE_RSB
>>>>> +
>>>>> +.L\@_skip_rsb:
>>>>> +
>>>>> +    testb $BTI_IST_WRMSR, %al
>>>>> +    jz .L\@_skip_wrmsr
>>>>> +
>>>>> +    testb $3, UREGS_cs(%rsp)
>>>>> +    jz .L\@_entry_from_xen
>>>>> +
>>>>> +    movb $0, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
>>>>> +
>>>>> +.L\@_entry_from_xen:
>>>>> +    /*
>>>>> +     * Load Xen's intended value.  SPEC_CTRL_IBRS vs 0 is encoded in the
>>>>> +     * bottom bit of bti_ist_info, via a deliberate alias with 
> BTI_IST_IBRS.
>>>>> +     */
>>>>> +    mov $MSR_SPEC_CTRL, %ecx
>>>>> +    and $BTI_IST_IBRS, %eax
>>>>> +    xor %edx, %edx
>>>>> +    wrmsr
>>>>> +
>>>>> +    /* Opencoded UNLIKELY_START() with no condition. */
>>>>> +.Ldispatch.\@_serialise:
>>>>> +    .subsection 1
>>>> How about adding .ifnb to UNLIKELY_START(), instead of open coding?
>>>> Or wait - why can't you use it as-is above (instead of the
>>>> "jz .L\@_skip_wrmsr")?
>>> Because then the end label is wrong.  One way of another, we either need
>>> to opencode something, or implement UNLIKELY_ELSE() which is actually
>>> the likely case, and this is a substantially larger can of worms.
>> I'm willing to trust you for now (until I get around to play with this
>> myself), but I don't follow: In an UNLIKELY_START() /
>> UNLIKELY_END() pair, how could the labels be wrong if both specify
>> the same tag?
> 
> The problem is that execution needs to jmp to after the wrmsr, rather
> than before the testb $3, UREGS_cs(%rsp), which is where UNLIKELY_END()
> would currently leave it.
> 
> We do not have suitable UNLIKELY() infrastructure to express a non-empty
> likely basic block.

At least we have UNLIKELY_DISPATCH_LABEL() - may I ask you
don't open-code that one then. UNLIKELY_DONE(), as it looks,
won't be of any help here, otoh.

Jan


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