Re: [PATCH 3/8] x86/svm: VMEntry/Exit logic for MSR_SPEC_CTRL

[Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>]

On 27/01/2022 07:25, Jan Beulich wrote:
> On 26.01.2022 21:16, Andrew Cooper wrote:
>> On 26/01/2022 16:50, Jan Beulich wrote:
>>> On 26.01.2022 09:44, Andrew Cooper wrote:
>>>> 1) It would be slightly more efficient to pass curr and cpu_info into
>>>>    vm{entry,exit}_spec_ctrl(), but setup of such state can't be in the
>>>>    ALTERNATIVE block because then the call displacement won't get fixed up.
>>>>    All the additional accesses are hot off the stack, so almost certainly
>>>>    negligible compared to the WRMSR.
>>> What's wrong with using two instances of ALTERNATIVE, one to setup the
>>> call arguments and the 2nd for just the CALL?
>> Hmm
>>
>> diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
>> index c718328ac4cf..1d4be7e97ae2 100644
>> --- a/xen/arch/x86/hvm/svm/entry.S
>> +++ b/xen/arch/x86/hvm/svm/entry.S
>> @@ -59,6 +59,7 @@ __UNLIKELY_END(nsvm_hap)
>>  
>>          /* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
>>          /* SPEC_CTRL_EXIT_TO_SVM       Req:                          
>> Clob: C   */
>> +        ALTERNATIVE "", __stringify(mov %rbx, %rdi; mov %rsp, %rsi),
>> X86_FEATURE_SC_MSR_HVM
>>          ALTERNATIVE "", __stringify(call vmentry_spec_ctrl),
>> X86_FEATURE_SC_MSR_HVM
>>  
>>          pop  %r15
>>
>> is somewhat of a long line, but isn't too terrible.
>>
>> I'm tempted to switch back to using STR() seeing as we have both and it
>> is much more concise.
> No objections. In fact I think when I introduced stringify.h over 10 years
> back, I didn't realize we already had STR(). Quite certainly first and
> foremost because of its bogus placement in xen/config.h (same would go for
> at least IS_ALIGNED() as well as KB() and friends).
>
> I wouldn't even mind phasing out stringify.h again. Or maybe we want to
> move STR() there ...

Now that we've given up using freestanding headers anywhere, there's a
xen/stddef.h shaped hole.

Things like NULL should move across, but it would also be the
appropriate place for ARRAY_SIZE() and pretty much everything else we
expect to be ubiquitous.  Linux includes things like offsetof().

~Andrew