[PATCH for-4.17?] x86: support data operand independent timing mode

Jan Beulich posted 1 patch 1 year, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
[PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Jan Beulich 1 year, 7 months ago
[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.

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@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
This may be viewed as a new feature, and hence be too late for 4.17. 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 would have wanted to invoke setup_doitm() from cpu_init(), but that
works only on the BSP. On APs cpu_init() runs before ucode loading.

--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -746,6 +746,14 @@ Specify the size of the console debug tr
 additionally a trace buffer of the specified size is allocated per cpu.
 The debug trace feature is only enabled in debugging builds of Xen.
 
+### dit (x86)
+> `= <boolean>`
+
+> Default: `CONFIG_DIT_DEFAULT`
+
+Specify whether Xen and guests should operate in Data Independent Timing
+mode.
+
 ### dma_bits
 > `= <integer>`
 
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -14,6 +14,7 @@ config X86
 	select HAS_ALTERNATIVE
 	select HAS_COMPAT
 	select HAS_CPUFREQ
+	select HAS_DIT
 	select HAS_EHCI
 	select HAS_EX_TABLE
 	select HAS_FAST_MULTIPLY
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -209,6 +209,24 @@ void ctxt_switch_levelling(const struct
 		alternative_vcall(ctxt_switch_masking, next);
 }
 
+static void setup_doitm(void)
+{
+    uint64_t msr;
+
+    if ( !cpu_has_arch_caps )
+        return;
+
+    rdmsrl(MSR_ARCH_CAPABILITIES, msr);
+    if ( !(msr & ARCH_CAPS_DOITM) )
+        return;
+
+    rdmsrl(MSR_UARCH_MISC_CTRL, msr);
+    msr |= UARCH_CTRL_DOITM;
+    if ( !opt_dit )
+        msr &= ~UARCH_CTRL_DOITM;
+    wrmsrl(MSR_UARCH_MISC_CTRL, msr);
+}
+
 bool_t opt_cpu_info;
 boolean_param("cpuinfo", opt_cpu_info);
 
@@ -581,6 +599,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
 
 		mtrr_bp_init();
 	}
+
+	setup_doitm();
 }
 
 /* leaf 0xb SMT level */
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -37,6 +37,9 @@ config HAS_COMPAT
 config HAS_DEVICE_TREE
 	bool
 
+config HAS_DIT # Data Independent Timing
+	bool
+
 config HAS_EX_TABLE
 	bool
 
@@ -171,6 +174,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 when no "dit" command line
+	  option is present.
+
 config HYPFS
 	bool "Hypervisor file system support"
 	default y
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -463,6 +463,11 @@ static int __init cf_check param_init(vo
 __initcall(param_init);
 #endif
 
+#ifdef CONFIG_HAS_DIT
+bool __ro_after_init opt_dit = IS_ENABLED(CONFIG_DIT_DEFAULT);
+boolean_param("dit", opt_dit);
+#endif
+
 # define DO(fn) long do_##fn
 
 #endif
--- a/xen/include/xen/param.h
+++ b/xen/include/xen/param.h
@@ -184,6 +184,8 @@ extern struct param_hypfs __paramhypfs_s
     string_param(_name, _var); \
     string_runtime_only_param(_name, _var)
 
+extern bool opt_dit;
+
 static inline void no_config_param(const char *cfg, const char *param,
                                    const char *s, const char *e)
 {
Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Andrew Cooper 1 year, 7 months ago
On 15/09/2022 11:04, 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.
>
> 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@invisiblethingslab.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

This patch should not be taken; at least not in this form.  The whole
DOITM infrastructure is currently under argument, for being impossible
to use appropriately.

I understand why Qubes want this blanket set, but it is a steep penalty
to pay;  It's only code which is already written trying to be constant
time/cache which gains any security from this.  On current parts, using
SSBD has the same behaviour, but this isn't expected to remain true in
the future.

Forcing it on behind the back of a VM is mutually exclusive with
enumerating it for VMs to use at some point in the future when we have
the capability to.  i.e. specifically, you are not able to maintain the
ABI/API in this patch in the future.

If we do move forward with something like this (under the strict
understanding that the behaviour is going to change in the future), then
"DIT" is too short of an acronym to use.  Amongst other things, it's not
"data independent timing"; it's "controls for forcing ..." which is
important because these are going to be vendor specific, if even needed
in the first place.

~Andrew
Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Marek Marczykowski-Górecki 1 year, 7 months ago
On Fri, Sep 30, 2022 at 11:25:12AM +0000, Andrew Cooper wrote:
> On 15/09/2022 11:04, 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.
> >
> > 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@invisiblethingslab.com>
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> This patch should not be taken; at least not in this form.  The whole
> DOITM infrastructure is currently under argument, for being impossible
> to use appropriately.
> 
> I understand why Qubes want this blanket set, but it is a steep penalty
> to pay;  It's only code which is already written trying to be constant
> time/cache which gains any security from this. 

Based on the bit description, I'd say rather "prevent _breaking_
security of the code already written". It is not setting this bit that
change behaviour on new parts, but it's not setting it that breaks
previous guarantees. It's really bizarre design choice from Intel...

>  On current parts, using
> SSBD has the same behaviour, but this isn't expected to remain true in
> the future.
> 
> Forcing it on behind the back of a VM is mutually exclusive with
> enumerating it for VMs to use at some point in the future when we have
> the capability to.  i.e. specifically, you are not able to maintain the
> ABI/API in this patch in the future.

Regarding the current behavior of the hypervisor (without this patch):
will guest see DOITM present but not set? Or will they not see it at
all?

Documentation clearly state:
    For Intel® Core™ family processors based on microarchitectures
    before Ice Lake and Intel Atom® family processors based on
    microarchitectures before Gracemont that do not enumerate
    IA32_UARCH_MISC_CTL, developers may assume that the instructions listed
    here operate as if DOITM is enabled.

So, if guests will not see the feature at all, it Xen should set it
unconditionally, to remain compliant with expected hardware behaviour.

If guests will see the thing (and see it not enabled), then indeed it's
less clear what should be done right now (but I'd still like to have an
option to enable it).

> If we do move forward with something like this (under the strict
> understanding that the behaviour is going to change in the future), then
> "DIT" is too short of an acronym to use.  Amongst other things, it's not
> "data independent timing"; it's "controls for forcing ..." which is
> important because these are going to be vendor specific, if even needed
> in the first place.
> 
> ~Andrew

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Andrew Cooper 1 year, 7 months ago
On 30/09/2022 16:41, Marek Marczykowski-Górecki wrote:

On Fri, Sep 30, 2022 at 11:25:12AM +0000, Andrew Cooper wrote:


On 15/09/2022 11:04, 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.

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@invisiblethingslab.com><mailto:demi@invisiblethingslab.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com><mailto:jbeulich@suse.com>



This patch should not be taken; at least not in this form.  The whole
DOITM infrastructure is currently under argument, for being impossible
to use appropriately.

I understand why Qubes want this blanket set, but it is a steep penalty
to pay;  It's only code which is already written trying to be constant
time/cache which gains any security from this.



Based on the bit description, I'd say rather "prevent _breaking_
security of the code already written". It is not setting this bit that
change behaviour on new parts, but it's not setting it that breaks
previous guarantees. It's really bizarre design choice from Intel...



 On current parts, using
SSBD has the same behaviour, but this isn't expected to remain true in
the future.

Forcing it on behind the back of a VM is mutually exclusive with
enumerating it for VMs to use at some point in the future when we have
the capability to.  i.e. specifically, you are not able to maintain the
ABI/API in this patch in the future.



Regarding the current behavior of the hypervisor (without this patch):
will guest see DOITM present but not set? Or will they not see it at
all?

Documentation clearly state:
    For Intel® Core™ family processors based on microarchitectures
    before Ice Lake and Intel Atom® family processors based on
    microarchitectures before Gracemont that do not enumerate
    IA32_UARCH_MISC_CTL, developers may assume that the instructions listed
    here operate as if DOITM is enabled.

So, if guests will not see the feature at all, it Xen should set it
unconditionally, to remain compliant with expected hardware behaviour.

If guests will see the thing (and see it not enabled), then indeed it's
less clear what should be done right now (but I'd still like to have an
option to enable it).

Hmm.  So yes, lets approach the problem from the other side, as "this bit needs setting to unbreak crypto code".

On hardware supporting DOITM, where we do not advertise the feature to guests (all guests right now), the guest kernel would conclude that it is safe, when in fact it is not.

So Xen should set the bit behind the back of a guest which doesn't have the DOITM enumeration presented (which is all guests right now).

But I don't think we want any Kconfig about this, or a dedicated cmdline option.  So how about this for a plan which avoids painting ourselves into a corner.

1) Extend cpuid= with a no-doitm option.  I know it's not actually a CPUID enumeration, but MSR_ARCH_CAPS should have been CPUID data, and this is the mechanism we have meaning "pretend this feature isn't enumerated".

2) On boot, and S3 resume, if DOITM and availble, set invariant mode.

That should do as a stopgap for now that keeps software safe.


Then, when we've got MSR_ARCH_CAPS working for guests, the internals of MSR_UARCH_MISC_CTL change to being a context switched thing which, like MSR_SPEC_CTRL, we have options for bits set behind the guest's back.  Then we set DOITM behind the guests back if levelling causes the feature to be hidden.  We do this for some bits already, and need to do so for more controls too.

~Andrew
Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Marek Marczykowski-Górecki 1 year, 7 months ago
On Fri, Sep 30, 2022 at 05:24:21PM +0000, Andrew Cooper wrote:
> Hmm.  So yes, lets approach the problem from the other side, as "this bit needs setting to unbreak crypto code".
> 
> On hardware supporting DOITM, where we do not advertise the feature to guests (all guests right now), the guest kernel would conclude that it is safe, when in fact it is not.
> 
> So Xen should set the bit behind the back of a guest which doesn't have the DOITM enumeration presented (which is all guests right now).

Yes, makes sense.

> But I don't think we want any Kconfig about this, or a dedicated cmdline option.  So how about this for a plan which avoids painting ourselves into a corner.
> 
> 1) Extend cpuid= with a no-doitm option.  I know it's not actually a CPUID enumeration, but MSR_ARCH_CAPS should have been CPUID data, and this is the mechanism we have meaning "pretend this feature isn't enumerated".

Sounds fine. But I wonder if there is any plan for [virtualizing] other
MSR_ARCH_CAPS - will they be treated as cpuid too?

> 2) On boot, and S3 resume, if DOITM and availble, set invariant mode.

+1

> That should do as a stopgap for now that keeps software safe.
> 
> 
> Then, when we've got MSR_ARCH_CAPS working for guests, the internals of MSR_UARCH_MISC_CTL change to being a context switched thing which, like MSR_SPEC_CTRL, we have options for bits set behind the guest's back.  Then we set DOITM behind the guests back if levelling causes the feature to be hidden.  We do this for some bits already, and need to do so for more controls too.
> 
> ~Andrew

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
RE: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Henry Wang 1 year, 7 months ago
Hi Jan,

> -----Original Message-----
> Subject: [PATCH for-4.17?] x86: support data operand independent timing
> mode
> 
> [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.
> 
> 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/so
> ftware-security-guidance/best-practices/data-operand-independent-timing-
> isa-guidance.html
> 
> Requested-by: Demi Marie Obenour <demi@invisiblethingslab.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> This may be viewed as a new feature, and hence be too late for 4.17. It
> may, however, also be viewed as security relevant, which is why I'd like
> to propose to at least consider it.

Based on the discussion in this thread so far, I think people would view
this patch as a security relevant patch, so I guess without strong
objection to merge this in 4.17, it is fine to add this in the release (with
proper review, of course).

Kind regards,
Henry

Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Demi Marie Obenour 1 year, 7 months ago
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On Thu, Sep 15, 2022 at 12:04:55PM +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.
> 
> 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@invisiblethingslab.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Thanks for the patch, Jan!

> This may be viewed as a new feature, and hence be too late for 4.17. It
> may, however, also be viewed as security relevant, which is why I'd like
> to propose to at least consider it.

I consider it security relevant indeed, which is why I was so insistent
on it.  Whether it is worth a full XSA is up to the Xen Security Team.
If it could be backported to stable releases, that would be great.

Marek, Simon, would you consider backporting this to R4.1?

> Slightly RFC, in particular for whether the Kconfig option should
> default to Y or N.

I think it should default to Y as long as guests do not have the ability
to control this.  Otherwise any cryptographic code in the guests thinks
it is constant time when it may not be.  Once guests have the ability to
control this I would be open to reconsidering this.
- -- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmMjC4MACgkQsoi1X/+c
IsELUg//fTRCauj/woVL8a3NpcB/2T2/gM06Lhg/eT7DsW4aJEIinB+jZ1mQ4oUb
MWEe3Ljwo0bxhbWbbQt2Xqp0pRM1MsDT7D0Boe0qEbpFYCgs8NrRvNE+MrtXG24x
B+2E/KZBIesjLV26S3uWTItHfUiFbqo5xzJURCDNHZqZiDnvCs4adiCMNDfroXyL
4UnP1slglrL/x/WqU9VKsWOOJAHTId2cBFd5FDlCQ7UX/GQISUIk7NZqCvutbtny
nJpSlbYoUcuQ3IfB4S7zDE4sN2YatCDqojZsAYuwRCRCRgM4nmZJUvK5KwzR1k6Z
0DfvZ0R4h5gdSrylqABzteEwLbob2icXxY89QHhssh/737R0HE5sRK2HKOPRZgUz
bmdlismQMqAuzUceAFreIGoPsIQUongF2xZJIY6AtGLudvaB8GZVyeJCgvH/eYyA
N05zybw3brLDgTjLN+HXTtsH4X7t4/ktCbGCLZWUytu5h4tr/wg/IXhd84uCu88n
3oLHLuqtpJUNItDYSmLSNQ7KO3Py4pbGjV7ienUl4fGpLS9MKG6raCTj12xO5nq8
5C/vMuzCRiJF3lEvHOrkVjH7vANk/8pfnqoHoMHs4lM2QnlskdOsjCPa17ZZWHs0
knT9OrN4hL7GgA2aU33rfhvgtDq6p7n5Xg2+YbNAbj7lSydjSho=
=dfry
-----END PGP SIGNATURE-----
Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Julien Grall 1 year, 7 months ago
Hi Demi,

On 15/09/2022 12:24, Demi Marie Obenour wrote:
> On Thu, Sep 15, 2022 at 12:04:55PM +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.
> 
>> 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@invisiblethingslab.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Thanks for the patch, Jan!
> 
>> This may be viewed as a new feature, and hence be too late for 4.17. It
>> may, however, also be viewed as security relevant, which is why I'd like
>> to propose to at least consider it.
> 
> I consider it security relevant indeed, which is why I was so insistent
> on it.  Whether it is worth a full XSA is up to the Xen Security Team.
> If it could be backported to stable releases, that would be great.
> 
> Marek, Simon, would you consider backporting this to R4.1?
> 
>> Slightly RFC, in particular for whether the Kconfig option should
>> default to Y or N.
> 
> I think it should default to Y as long as guests do not have the ability
> to control this.

This raises two questions:
  1) What is the performance impact to turn this on by default? I am 
looking for actual numbers.
  2) What happen on HW that doesn't support DIT? Are we going to mark 
them as unsupported?

>  Otherwise any cryptographic code in the guests thinks
> it is constant time when it may not be.

Why would a guest think that? Are we telling the guest DIT is supported 
but doesn't honour it?

If yes, then I would argue that we should clear that bit. Otherwise...

>  Once guests have the ability to
> control this I would be open to reconsidering this.

... this will introduce a problem once we expose it to the guest because 
we cannot change the global default as some user my start to rely on it 
on the default.

Cheers,

-- 
Julien Grall
Re: [PATCH for-4.17?] x86: support data operand independent timing mode
Posted by Demi Marie Obenour 1 year, 7 months ago
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On Thu, Sep 15, 2022 at 01:56:06PM +0100, Julien Grall wrote:
> Hi Demi,
> 
> On 15/09/2022 12:24, Demi Marie Obenour wrote:
> > On Thu, Sep 15, 2022 at 12:04:55PM +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.
> > 
> > > 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@invisiblethingslab.com>
> > > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> > 
> > Thanks for the patch, Jan!
> > 
> > > This may be viewed as a new feature, and hence be too late for 4.17. It
> > > may, however, also be viewed as security relevant, which is why I'd like
> > > to propose to at least consider it.
> > 
> > I consider it security relevant indeed, which is why I was so insistent
> > on it.  Whether it is worth a full XSA is up to the Xen Security Team.
> > If it could be backported to stable releases, that would be great.
> > 
> > Marek, Simon, would you consider backporting this to R4.1?
> > 
> > > Slightly RFC, in particular for whether the Kconfig option should
> > > default to Y or N.
> > 
> > I think it should default to Y as long as guests do not have the ability
> > to control this.
> 
> This raises two questions:
>  1) What is the performance impact to turn this on by default? I am looking
> for actual numbers.

I do not have access to such hardware and so cannot provide such
numbers.  I was hoping that someone else would be able to do the needed
benchmarking.

>  2) What happen on HW that doesn't support DIT? Are we going to mark them as
> unsupported?

The relevant text in Intel’s documentation is:

> For Intel® Core™ family processors based on microarchitectures before
> Ice Lake and Intel Atom® family processors based on microarchitectures
> before Gracemont that do not enumerate IA32_UARCH_MISC_CTL, developers
> may assume that the instructions listed here operate as if DOITM is
> enabled.
> 
> Intel Core family processors based on Ice Lake and later, such as
> Tiger Lake, Lakefield, and Rocket Lake will enumerate DOITM. Intel
> Atom processors based on Gracemont and later will also enumerate
> DOITM. Refer to the Enumeration and Architectural MSRs section for
> more information.

In other words, no action is needed (or possible) on CPUs that do not
enumerate DOITM.  CPUs that do enumerate DOITM require it to be
explicitly enabled by for cryptographic code to be secure.  This was a
poor design decision on Intel’s part, which might be why it appears that
Linux will treat DOITM as a CPU bug.

> >  Otherwise any cryptographic code in the guests thinks
> > it is constant time when it may not be.
> 
> Why would a guest think that? Are we telling the guest DIT is supported but
> doesn't honour it?

Xen is telling guests that DOITM is not required for constant-time
operation of cryptographic code, even though the hardware actually
requires it.  Furthermore, Xen does not allow guests to set DOITM.

> If yes, then I would argue that we should clear that bit. Otherwise...

Xen actually needs to *set* that bit, not clear it.

> >  Once guests have the ability to
> > control this I would be open to reconsidering this.
> 
> ... this will introduce a problem once we expose it to the guest because we
> cannot change the global default as some user my start to rely on it on the
> default.

I would be fine with requiring the toolstack to opt-out from the safe
default.
- -- 
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmMjNDYACgkQsoi1X/+c
IsEi+RAAskGqEC1ParSsfyWS7fw9mhKzH+pS+IsKE5tjCu1l6Ctub17qqysC3Oej
yzqMPuK+YwwEW5X3SNlnd6Qs5C/2XIdkr8cwB8PitjMabS1dvJu01qIRbIyMDM2i
ii9qtuTLCDUAz5rrLW+qr0TE9vBcfteOKzkcWkmQQpWjermZHy+lHooXNmwjS9oc
J+nGbUWhNy1uNvFA+sUjEcEucAlSyM7UWkL/c9FzfWV2UwPWppdGpyz7jrRdZT+H
brb3FwRd1hoS4t94Xv9ynzZ9K7KeEZ5pF8rKOoKh0cfZ9Ydcuh0wr9dtD6aWTS3r
I2rNQig6AqurCF6AVUIlwf6TYcI71BRNDB3SQNAh7h5yxjy1S0srE0RLIZXFiboz
0RAyq76NFWS9qN2BG11PvfaG665EL14+7I+xaJxeSwPOZlUmbhTznYN7thFlx5MC
AxSwthokvIThiy1tpu3XdiU1zFWb5Spd4vwkSv/bo2fCPySECJ59tGsDtNvreJbf
z7A1CB968bTdPJAsYgY2OkkO+zFm3vR+fmrSia5b5cHoY80Mb0FmxKrPKxcOsyHJ
PJGoWMDxRxi0RTYi5nrI3qGWJSl9GpryQPX5jaB01mLcXHo/mmQB1PiFda+PioPa
T/bwpiYCc06EyeuJBMfu0Gv9NYnAuDC7qBtJfjKzm+OofrfWzQI=
=mpE9
-----END PGP SIGNATURE-----