Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi Jan, > On Sep 12, 2023, at 08:48, Henry Wang wrote: > > Hi Jan, > >> On Sep 11, 2023, at 23: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. >> >> 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 >> Signed-off-by: Jan Beulich >> --- >> 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. > > Fine with me if this patch can be properly reviewed on time, because of > the security relevance. Based on this, if this patch can be properly reviewed before we release 4.18, please feel free to add: Release-acked-by: Henry Wang Kind regards, Henry
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi Jan, On 14/09/2023 12:01, Jan Beulich wrote: On 14.09.2023 11:18, Julien Grall wrote: On 14/09/2023 10:11, Jan Beulich wrote: On 14.09.2023 11:04, Julien Grall wrote: On 14/09/2023 07:32, Jan Beulich wrote: On 13.09.2023 19:56, Julien Grall wrote: 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. ... we need to somehow let the user know that the HW it is running on may not honor DIT. Tainting might be a bit too harsh, but I think printing a message with warning_add() is necessary. But Intel's claim is that with the bit unavailable hardware behaves as if DIT was in effect. I am confused. Above, you suggested it cannot be guaranteed. So I interpreted we don't know what's happening on any processor. Right. We can trust vendors, or not. So where you referring to... Therefore you're still suggesting to emit a warning on most of Intel's hardware and on all non-Intel one. ... non-Intel HW? That's going too far, imo. We could restrict the warning to non-Intel platform. That still goes too far imo. I'm not convinced we should cover for vendor uncertainty here. We can't make a system any safer than its hardware is, in this regard. We simply have no (or not enough) influence on the internal workings of their pipelines. Fair enough. I would still prefer a message in the log but ... What I have done is add a sentence to the command line option's documentation: "Note that enabling this option cannot guarantee anything beyond what underlying hardware guarantees (with, where available and known to Xen, respective tweaks applied)." Plus I've further qualified the option: ### dit (x86/Intel) ... I am also happy with these two changes. Cheers, -- Julien Grall
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
On Thu, Sep 14, 2023 at 10:04:31AM +0100, Julien Grall wrote:
> Hi Jan,
>
> On 14/09/2023 07:32, Jan Beulich wrote:
> > 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
> > > > Signed-off-by: Jan Beulich
> > > > ---
> > > > 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 understood that. My question was more related to how much it will degrade
> the performance. This will help to know whether the default should be yes or
> no.
This information is not available. You could do benchmarks with and
without this patch if you wanted to obtain it.
> > > > --- 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.
>
> Right. So ...
>
> >
> > > 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.
>
> ... we need to somehow let the user know that the HW it is running on may
> not honor DIT. Tainting might be a bit too harsh, but I think printing a
> message with warning_add() is necessary.
Prior to DOITM, the data-operand-independent timing of certain
instructions was an implicit contract between hardware and software.
DOITM made several changes:
1. The implicit contract has been replaced by an architectural
guarantee. This guarantee has a specific list of instructions to
which it applies, as well as a specific list of operands that timing
will be independent of. The guarantee has also been made conditional
on DOITM being enabled — if it is disabled, no guarantees are made.
2. Intel has retroactively extended this guarantee to all previous CPU
architectures, including ones that do not give software control over
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
On 14.09.2023 11:18, Julien Grall wrote: > On 14/09/2023 10:11, Jan Beulich wrote: >> On 14.09.2023 11:04, Julien Grall wrote: >>> On 14/09/2023 07:32, Jan Beulich wrote: On 13.09.2023 19:56, Julien Grall wrote: > 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. >>> >>> ... we need to somehow let the user know that the HW it is running on >>> may not honor DIT. Tainting might be a bit too harsh, but I think >>> printing a >>> message with warning_add() is necessary. >> >> But Intel's claim is that with the bit unavailable hardware behaves as >> if DIT was in effect. > > I am confused. Above, you suggested it cannot be guaranteed. So I > interpreted we don't know what's happening on any processor. Right. We can trust vendors, or not. > So where > you referring to... > > > Therefore you're still suggesting to emit a >> warning on most of Intel's hardware and on all non-Intel one. > > ... non-Intel HW? > >> That's >> going too far, imo. > > We could restrict the warning to non-Intel platform. That still goes too far imo. I'm not convinced we should cover for vendor uncertainty here. We can't make a system any safer than its hardware is, in this regard. We simply have no (or not enough) influence on the internal workings of their pipelines. What I have done is add a sentence to the command line option's documentation: "Note that enabling this option cannot guarantee anything beyond what underlying hardware guarantees (with, where available and known to Xen, respective tweaks applied)." Plus I've further qualified the option: ### dit (x86/Intel) Jan
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi,
On 14/09/2023 10:11, Jan Beulich wrote:
On 14.09.2023 11:04, Julien Grall wrote:
On 14/09/2023 07:32, Jan Beulich wrote:
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
Signed-off-by: Jan Beulich
---
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 understood that. My question was more related to how much it will
degrade the performance. This will help to know whether the default
should be yes or no.
Well, as said - I have no information beyond that to be found at the
provided URL.
--- 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.
Right. So ...
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.
... we need to somehow let the user know that the HW it is running on
may not honor DIT. Tainting might be a bit too harsh, but I think
printing a
message with warning_add() is necessary.
But Intel's claim is that with the bit unavailable hardware behaves as
if DIT was in effect.
I am confused. Above, you suggested it cannot be guaranteed. So I
interpreted we don't know what's happening on any processor. So where
you referring to...
Therefore you're still suggesting to emit a
warning on most of Intel's hardware and on all non-Intel one.
... non-Intel HW?
That's
going too far, imo.
We could restrict the warning to non-Intel platform.
Jan
--
Julien Grall
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
On 14.09.2023 11:04, Julien Grall wrote:
> On 14/09/2023 07:32, Jan Beulich wrote:
>> 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
Signed-off-by: Jan Beulich
---
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 understood that. My question was more related to how much it will
> degrade the performance. This will help to know whether the default
> should be yes or no.
Well, as said - I have no information beyond that to be found at the
provided URL.
--- 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.
>
> Right. So ...
>
>>
>>> 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.
>
> ... we need to somehow let the user know that the HW it is running on
> may not honor DIT. Tainting might be a bit too harsh, but I think
> printing a
> message with warning_add() is necessary.
But Intel's claim is that with the bit unavailable hardware behaves as
if DIT was in effect. Therefore you're still suggesting to emit a
warning on most of Intel's hardware and on all non-Intel one. That's
going too far, imo.
Jan
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi Jan,
On 14/09/2023 07:32, Jan Beulich wrote:
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
Signed-off-by: Jan Beulich
---
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 understood that. My question was more related to how much it will
degrade the performance. This will help to know whether the default
should be yes or no.
--- 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.
Right. So ...
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.
... we need to somehow let the user know that the HW it is running on
may not honor DIT. Tainting might be a bit too harsh, but I think
printing a
message with warning_add() is necessary.
Cheers,
--
Julien Grall
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
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
>> Signed-off-by: Jan Beulich
>> ---
>> 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
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi,
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?
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
Signed-off-by: Jan Beulich
---
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.
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.
Plus recheck_cpu_features() invoking identify_cpu() takes care of the
BSP during S3 resume.
---
v2: Introduce and use cpu_has_doitm. Add comment "borrowed" from the
XenServer patch queue patch providing similar functionality.
Re-base.
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -788,6 +788,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)
+> `= `
+
+> Default: `CONFIG_DIT_DEFAULT`
+
+Specify whether Xen and guests should operate in Data Independent Timing
+mode.
+
### dma_bits
> `= `
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -15,6 +15,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
@@ -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?
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.
+return;
+
+/*
+ * We don't currently enumerate DOITM to guests. As a conseqeuence, guest
+ * kernels will believe they're safe even when they are not.
+ *
+ * For now, set it unilaterally. This prevents otherwise-correct crypto
+ * code from becoming vulnerable to timing sidechannels.
+ */
+
+rdmsrl(MSR_UARCH_MISC_CTRL, msr);
+msr |= UARCH_CTRL_DOITM;
+if ( !opt_dit )
+msr &= ~UARCH_CTRL_DOITM;
+wrmsrl(MSR_UARCH_MISC_CTRL, msr);
+}
+
bool opt_cpu_info;
boolean_param("cpuinfo", opt_cpu_info);
@@ -589,6 +611,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
mtrr_bp_init();
}
+
+ setup_doitm();
}
/* leaf 0xb SMT level */
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -201,6 +201,7 @@ static inline bool boot_cpu_has(unsigned
#define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
#define cpu_has_tsx_ctrlboot_cpu_has(X86_FEATURE_TSX_CTRL)
#define cpu_has_taa_no boot_cpu_has(X86_FEATURE_TAA_NO)
+#define cpu_has_doitm boot_cpu_has(X86_FEATURE_DOITM)
#define cpu_has_fb_clearboot_cpu_has(X86_FEATURE_FB_CLEAR)
#define cpu_has_rrsba boot_cpu_has(X86_FEATURE_RRSBA)
#define cpu_has_gds_ctrlboot_cpu_has(X86_FEATURE_GDS_CTRL)
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -41,6 +41,9 @@ config HAS_COMPAT
config HAS_DEVICE_TREE
bool
+config HAS_DIT # Data Independent Timing
+ bool
+
config HAS_EX_TABLE
bool
@@ -175,6 +178,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
+
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi, On 12/09/2023 07:21, Jan Beulich wrote: On 12.09.2023 02:48, Henry Wang wrote: On Sep 11, 2023, at 23:01, Jan Beulich wrote: --- a/docs/misc/xen-command-line.pandoc +++ b/docs/misc/xen-command-line.pandoc @@ -788,6 +788,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) +> `= ` + +> Default: `CONFIG_DIT_DEFAULT` + +Specify whether Xen and guests should operate in Data Independent Timing +mode. + Since a new cmdline interface is added, I am wondering would such addtion deserves a CHANGELOG entry? I'm open to opinions, albeit not so much because of the added command line option. I think it would make sense to add an entry to say the user can now decide the operation mode for DIT. Cheers, -- Julien Grall
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
On 12.09.2023 02:48, Henry Wang wrote: >> On Sep 11, 2023, at 23:01, Jan Beulich wrote: >> --- a/docs/misc/xen-command-line.pandoc >> +++ b/docs/misc/xen-command-line.pandoc >> @@ -788,6 +788,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) >> +> `= ` >> + >> +> Default: `CONFIG_DIT_DEFAULT` >> + >> +Specify whether Xen and guests should operate in Data Independent Timing >> +mode. >> + > > Since a new cmdline interface is added, I am wondering would such > addtion deserves a CHANGELOG entry? I'm open to opinions, albeit not so much because of the added command line option. Jan
Re: [PATCH v2 for-4.18?] x86: support data operand independent timing mode
Hi Jan, > On Sep 11, 2023, at 23: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. > > 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 > Signed-off-by: Jan Beulich > --- > 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. Fine with me if this patch can be properly reviewed on time, because of the security relevance. > > 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. > Plus recheck_cpu_features() invoking identify_cpu() takes care of the > BSP during S3 resume. > --- > v2: Introduce and use cpu_has_doitm. Add comment "borrowed" from the >XenServer patch queue patch providing similar functionality. >Re-base. > > --- a/docs/misc/xen-command-line.pandoc > +++ b/docs/misc/xen-command-line.pandoc > @@ -788,6 +788,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) > +> `= ` > + > +> Default: `CONFIG_DIT_DEFAULT` > + > +Specify whether Xen and guests should operate in Data Independent Timing > +mode. > + Since a new cmdline interface is added, I am wondering would such addtion deserves a CHANGELOG entry? Kind regards, Henry
[PATCH v2 for-4.18?] 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/software-security-guidance/best-practices/data-operand-independent-timing-isa-guidance.html
Requested-by: Demi Marie Obenour
Signed-off-by: Jan Beulich
---
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 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.
Plus recheck_cpu_features() invoking identify_cpu() takes care of the
BSP during S3 resume.
---
v2: Introduce and use cpu_has_doitm. Add comment "borrowed" from the
XenServer patch queue patch providing similar functionality.
Re-base.
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -788,6 +788,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)
+> `= `
+
+> Default: `CONFIG_DIT_DEFAULT`
+
+Specify whether Xen and guests should operate in Data Independent Timing
+mode.
+
### dma_bits
> `= `
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -15,6 +15,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
@@ -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 )
+return;
+
+/*
+ * We don't currently enumerate DOITM to guests. As a conseqeuence, guest
+ * kernels will believe they're safe even when they are not.
+ *
+ * For now, set it unilaterally. This prevents otherwise-correct crypto
+ * code from becoming vulnerable to timing sidechannels.
+ */
+
+rdmsrl(MSR_UARCH_MISC_CTRL, msr);
+msr |= UARCH_CTRL_DOITM;
+if ( !opt_dit )
+msr &= ~UARCH_CTRL_DOITM;
+wrmsrl(MSR_UARCH_MISC_CTRL, msr);
+}
+
bool opt_cpu_info;
boolean_param("cpuinfo", opt_cpu_info);
@@ -589,6 +611,8 @@ void identify_cpu(struct cpuinfo_x86 *c)
mtrr_bp_init();
}
+
+ setup_doitm();
}
/* leaf 0xb SMT level */
--- a/xen/arch/x86/include/asm/cpufeature.h
+++ b/xen/arch/x86/include/asm/cpufeature.h
@@ -201,6 +201,7 @@ static inline bool boot_cpu_has(unsigned
#define cpu_has_if_pschange_mc_no boot_cpu_has(X86_FEATURE_IF_PSCHANGE_MC_NO)
#define cpu_has_tsx_ctrlboot_cpu_has(X86_FEATURE_TSX_CTRL)
#define cpu_has_taa_no boot_cpu_has(X86_FEATURE_TAA_NO)
+#define cpu_has_doitm boot_cpu_has(X86_FEATURE_DOITM)
#define cpu_has_fb_clearboot_cpu_has(X86_FEATURE_FB_CLEAR)
#define cpu_has_rrsba boot_cpu_has(X86_FEATURE_RRSBA)
#define cpu_has_gds_ctrlboot_cpu_has(X86_FEATURE_GDS_CTRL)
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -41,6 +41,9 @@ config HAS_COMPAT
config HAS_DEVICE_TREE
bool
+config HAS_DIT # Data Independent Timing
+ bool
+
config HAS_EX_TABLE
bool
@@ -175,6 +178,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.
+
config HYPFS
bool "Hypervisor file system support"
default y
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -28,6 +28,11 @@ CHECK_feature_info;
enum system_state system_state = SYS_STATE_early_boot;
+#ifdef CONFIG_HAS_DIT
+bool __ro_after_init opt_dit = IS_ENABLED(CONFIG_DIT_DEFAULT);
+boolean_param("dit", opt_dit);
