Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.09.2023 19:56, Julien Grall wrote:
> On 11/09/2023 16:01, Jan Beulich wrote:
>> [1] specifies a long list of instructions which are intended to exhibit
>> timing behavior independent of the data they operate on. On certain
>> hardware this independence is optional, controlled by a bit in a new
>> MSR. Provide a command line option to control the mode Xen and its
>> guests are to operate in, with a build time control over the default.
>> Longer term we may want to allow guests to control this.
> 
> If I read correctly the discussion on the previous version. There was 
> some concern with this version of patch. I can't find anything public 
> suggesting that the concerned were dismissed. Do you have any pointer?

Well, I can point to the XenServer patch queue, which since then has
gained a patch of similar (less flexible) functionality (and seeing
the similarity I was puzzled by this patch, which was posted late
for 4.17, not having gone in quite some time ago). This to me
demonstrates that the original concerns were "downgraded". It would
of course still be desirable to have guests control the behavior for
themselves, but that's a more intrusive change left for the future.

Beyond that I guess I need to have Andrew speak for himself.

>> Since Arm64 supposedly also has such a control, put command line option
>> and Kconfig control in common files.
>>
>> [1] 
>> https://www.intel.com/content/www/us/en/developer/articles/technical/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html
>>
>> Requested-by: Demi Marie Obenour <demi@xxxxxxxxxxxxxxxxxxxxxx>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> This may be viewed as a new feature, and hence be too late for 4.18. It
>> may, however, also be viewed as security relevant, which is why I'd like
>> to propose to at least consider it.
>>
>> Slightly RFC, in particular for whether the Kconfig option should
>> default to Y or N.
> 
> I am not entirely sure. I understand that DIT will help in the 
> cryptographic case but it is not clear to me what is the performance impact.

The entire purpose of the bit is to have a performance impact, slowing
down execution when fastpaths could be taken, but shouldn't to achieve
the named goal. I have no numbers though, and like you I can only go
from what the referenced documentation says.

>> --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -204,6 +204,28 @@ void ctxt_switch_levelling(const struct
>>              alternative_vcall(ctxt_switch_masking, next);
>>   }
>>   
>> +static void setup_doitm(void)
>> +{
>> +    uint64_t msr;
>> +
>> +    if ( !cpu_has_doitm )
> 
> I am not very familiar with the feature. If it is not present, then can 
> we guarantee that the instructions will be executed in constant time?

_We_ cannot guarantee anything. It is only hardware vendors who can. As a
result I can only again refer you to the referenced documentation. It
claims that without the bit being supported in hardware, an extensive
list of insns is going to expose data operand independent performance.

> If not, I think we should taint Xen and/or print a message if the admin 
> requested to use DIT. This would make clear that the admin is trying to 
> do something that doesn't work.

Tainting Xen on all hardware not exposing the bit would seem entirely
unreasonable to me. If we learned of specific cases where the vendor
promises are broken, we could taint there, sure. I guess that would be
individual XSAs / CVEs then for each instance.

Jan