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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 20.10.2023 12:53, Roger Pau Monné wrote:
> On Fri, Oct 20, 2023 at 07:53:51AM +0200, 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.
> 
> Couldn't we just expose DOITM in MSR_ARCH_CAPS (by setting the A flag
> in the feature enumeration) and handle accesses to
> MSR_UARCH_MISC_CTRL?
> 
> The complications would be with the leveling of the feature across a
> pool, as AFAICT the bit needs to be OR'ed rather than AND'ed across
> hosts.
> 
> It would also then need some special handling in order to allow
> reporting DOITM on hardware that doesn't have the feature (and writes
> to MSR_UARCH_MISC_CTRL won't be forwarded to hardware in that case).
> 
> From an implementation PoV we might want to treat this as SSBD, and
> allow Xen to run with the guest selection.
> 
> Anyway, likely much more than what you want to do

Right, I understand that's also the reason why a yet more minimalistic
variant exists in XenServe's patch queue.

>> Since Arm64 supposedly also has such a control, put command line option
>> and Kconfig control in common files.

This, implicitly, ...

>> [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>
>> Release-acked-by: Henry Wang <Henry.Wang@xxxxxxx>
> 
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> The only concern I have is with the Kconfig and command line options
> being named DIT instead of DOIT, which would be what I would search
> for given the documentation from Intel.  Maybe that's for unification
> with Arm?

... relates to this: Supposedly DIT is the name used there.

Thanks for the ack.

>> @@ -187,6 +190,18 @@ config SPECULATIVE_HARDEN_GUEST_ACCESS
>>  
>>  endmenu
>>  
>> +config DIT_DEFAULT
>> +    bool "Data Independent Timing default"
>> +    depends on HAS_DIT
>> +    help
>> +      Hardware often surfaces instructions the timing of which is dependent
>> +      on the data they process.  Some of these instructions may be used in
>> +      timing sensitive environments, e.g. cryptography.  When such
>> +      instructions exist, hardware may further surface a control allowing
>> +      to make the behavior of such instructions independent of the data
>> +      they act upon.  Choose the default here for when no "dit" command line
>> +      option is present.
> 
> I would word the last sentence as `Note the build time value can be
> changed at runtime using the "dit" command line option.`

Oh, yes, that's quite a bit better.

Jan