[PATCH] Ability to read the MKTME status from userspace

[PATCH] Ability to read the MKTME status from userspace

[Daniel Gutson]

From: Daniel Gutson <daniel@eclypsium.com>

The intent of this patch is to provide visibility of the
MKTME status to userspace. This is an important factor for
firmware security related applilcations.

Signed-off-by: Daniel Gutson <daniel@eclypsium.com>
---
 MAINTAINERS                 |  5 +++
 arch/x86/include/asm/cpu.h  |  8 ++++
 arch/x86/kernel/cpu/intel.c | 12 +++---
 drivers/misc/Kconfig        | 11 +++++
 drivers/misc/Makefile       |  1 +
 drivers/misc/mktme_status.c | 81 +++++++++++++++++++++++++++++++++++++
 6 files changed, 113 insertions(+), 5 deletions(-)
 create mode 100644 drivers/misc/mktme_status.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 50659d76976b..dc3b3c0e4701 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11335,6 +11335,11 @@ W:	https://linuxtv.org
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/radio/radio-miropcm20*
 
+MKTME_STATUS MKTME STATUS READING SUPPORT
+M:	Daniel Gutson <daniel.gutson@eclypsium.com>
+S:	Supported
+F:	drivers/misc/mktme_status.c
+
 MMP SUPPORT
 R:	Lubomir Rintel <lkundrak@v3.sk>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index dd17c2da1af5..8929c1240135 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -26,6 +26,14 @@ struct x86_cpu {
 	struct cpu cpu;
 };
 
+enum mktme_status_type {
+	MKTME_ENABLED,
+	MKTME_DISABLED,
+	MKTME_UNINITIALIZED
+};
+
+extern enum mktme_status_type get_mktme_status(void);
+
 #ifdef CONFIG_HOTPLUG_CPU
 extern int arch_register_cpu(int num);
 extern void arch_unregister_cpu(int);
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index a19a680542ce..1f6054523226 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -489,11 +489,7 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
 #define TME_ACTIVATE_CRYPTO_ALGS(x)	((x >> 48) & 0xffff)	/* Bits 63:48 */
 #define TME_ACTIVATE_CRYPTO_AES_XTS_128	1
 
-/* Values for mktme_status (SW only construct) */
-#define MKTME_ENABLED			0
-#define MKTME_DISABLED			1
-#define MKTME_UNINITIALIZED		2
-static int mktme_status = MKTME_UNINITIALIZED;
+static enum mktme_status_type mktme_status = MKTME_UNINITIALIZED;
 
 static void detect_tme(struct cpuinfo_x86 *c)
 {
@@ -1107,6 +1103,12 @@ bool handle_user_split_lock(struct pt_regs *regs, long error_code)
 	return true;
 }
 
+enum mktme_status_type get_mktme_status(void)
+{
+	return mktme_status;
+}
+EXPORT_SYMBOL_GPL(get_mktme_status);
+
 /*
  * This function is called only when switching between tasks with
  * different split-lock detection modes. It sets the MSR for the
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 99e151475d8f..0dc978efbbd5 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -465,6 +465,17 @@ config PVPANIC
 	  a paravirtualized device provided by QEMU; it lets a virtual machine
 	  (guest) communicate panic events to the host.
 
+config MKTME_STATUS
+	tristate "MKTME status reading support"
+	depends on X86_64 && SECURITYFS
+	help
+	  This driver provides support for reading the MKTME status. The status
+	  can be read from the mktme virtual file in the securityfs filesystem,
+	  under the mktme directory.
+
+	  The MKTME (Multi-Key Total Memory Encryption) status can be
+	  0 (enabled), 1 (disabled), or 3 (uninitialized).
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 9abf2923d831..f2f02efe34fd 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -58,3 +58,4 @@ obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
 obj-$(CONFIG_HABANA_AI)		+= habanalabs/
 obj-$(CONFIG_UACCE)		+= uacce/
 obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
+obj-$(CONFIG_MKTME_STATUS)	+= mktme_status.o
diff --git a/drivers/misc/mktme_status.c b/drivers/misc/mktme_status.c
new file mode 100644
index 000000000000..795993181e77
--- /dev/null
+++ b/drivers/misc/mktme_status.c
@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MKTME Status driver
+ *
+ * Copyright 2020 (c) Daniel Gutson (daniel.gutson@eclypsium.com)
+ *
+ * This file is licensed under  the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/module.h>
+#include <linux/security.h>
+#include <asm/cpu.h>
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+struct dentry *mktme_dir;
+struct dentry *mktme_file;
+
+/* Buffer to return: always 3 because of the following chars:
+ *     value \n \0
+ */
+#define BUFFER_SIZE 3
+
+static ssize_t mktme_status_read(struct file *filp, char __user *buf,
+				 size_t count, loff_t *ppos)
+{
+	char tmp[BUFFER_SIZE];
+
+	if (*ppos == BUFFER_SIZE)
+		return 0; // nothing else to read
+
+	sprintf(tmp, "%d\n", (int)get_mktme_status() & 1);
+	return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp));
+}
+
+static const struct file_operations mktme_status_ops = {
+	.read = mktme_status_read,
+};
+
+static int __init mod_init(void)
+{
+	mktme_dir = securityfs_create_dir("mktme", NULL);
+	if (IS_ERR(mktme_dir)) {
+		pr_err("Couldn't create mktme sysfs dir\n");
+		return -1;
+	}
+
+	pr_info("mktme securityfs dir creation successful\n");
+
+	mktme_file = securityfs_create_file("status", 0600, mktme_dir, NULL,
+					    &mktme_status_ops);
+	if (IS_ERR(mktme_file)) {
+		pr_err("Error creating sysfs file bioswe\n");
+		goto out_file;
+	}
+
+	return 0;
+
+out_file:
+	securityfs_remove(mktme_file);
+	securityfs_remove(mktme_dir);
+	return -1;
+}
+
+static void __exit mod_exit(void)
+{
+	securityfs_remove(mktme_file);
+	securityfs_remove(mktme_dir);
+}
+
+module_init(mod_init);
+module_exit(mod_exit);
+
+MODULE_DESCRIPTION("MKTME Status driver");
+MODULE_AUTHOR("Daniel Gutson <daniel.gutson@eclypsium.com>");
+MODULE_LICENSE("GPL");
-- 
2.25.1

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/18/20 2:02 PM, Daniel Gutson wrote:
> The intent of this patch is to provide visibility of the
> MKTME status to userspace. This is an important factor for
> firmware security related applilcations.

I think we need more specifics before considering adding a new ABI like
this.  Could you elaborate, plesae?

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Thu, Jun 18, 2020 at 06:26:25PM -0300, Daniel Gutson wrote:
> Red Hat and Eclypsium are working on a specification to assess
> firmware platform security. One of the inputs that the specification
> takes into consideration is whether MKTME is enabled or disabled.
> Exposing this value is necessary for tools checking the conformance of
> the specification.

Would it be enough to grep /proc/cpuinfo whether it has "tme" or not,
instead of adding a bunch of code just to read a status value?

@Dave: this is where those flags in /proc/cpuinfo come real handy. :-)

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/18/20 2:26 PM, Daniel Gutson wrote:
> Red Hat and Eclypsium are working on a specification to assess
> firmware platform security. One of the inputs that the specification
> takes into consideration is whether MKTME is enabled or disabled. 
> Exposing this value is necessary for tools checking the conformance
> of the specification.

What does TME's status on the platform tell you, though?

It doesn't tell you if your data is encrypted.  It doesn't even tell you
if your mlock()'d "in RAM" data is encrypted.

So, what good is it?

Are we going to need another one of these when the TME encryption
algorithm changes?  Do we need another driver when running in a SEV
guest to tell us about SEV's status?

Re: [PATCH] Ability to read the MKTME status from userspace

[Greg Kroah-Hartman]

On Thu, Jun 18, 2020 at 06:02:15PM -0300, Daniel Gutson wrote:
> From: Daniel Gutson <daniel@eclypsium.com>
> 
> The intent of this patch is to provide visibility of the
> MKTME status to userspace. This is an important factor for
> firmware security related applilcations.
> 
> Signed-off-by: Daniel Gutson <daniel@eclypsium.com>

Code review that is agnostic as to the need for this at all, I figured
might as well as this needs work even if everyone agrees that the
function/feature is needed:

> ---
>  MAINTAINERS                 |  5 +++
>  arch/x86/include/asm/cpu.h  |  8 ++++
>  arch/x86/kernel/cpu/intel.c | 12 +++---
>  drivers/misc/Kconfig        | 11 +++++
>  drivers/misc/Makefile       |  1 +
>  drivers/misc/mktme_status.c | 81 +++++++++++++++++++++++++++++++++++++
>  6 files changed, 113 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/misc/mktme_status.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 50659d76976b..dc3b3c0e4701 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11335,6 +11335,11 @@ W:	https://linuxtv.org
>  T:	git git://linuxtv.org/media_tree.git
>  F:	drivers/media/radio/radio-miropcm20*
>  
> +MKTME_STATUS MKTME STATUS READING SUPPORT

You list the same thing twice on one line?

> +M:	Daniel Gutson <daniel.gutson@eclypsium.com>
> +S:	Supported
> +F:	drivers/misc/mktme_status.c
> +
>  MMP SUPPORT
>  R:	Lubomir Rintel <lkundrak@v3.sk>
>  L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index dd17c2da1af5..8929c1240135 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -26,6 +26,14 @@ struct x86_cpu {
>  	struct cpu cpu;
>  };
>  
> +enum mktme_status_type {
> +	MKTME_ENABLED,
> +	MKTME_DISABLED,
> +	MKTME_UNINITIALIZED
> +};

You are exporting these values to userspace, so you need to specify
exactly what these enums resolve to, AND put them in a userspace visable
.h file so that they can be checked/read/understood.

> +
> +extern enum mktme_status_type get_mktme_status(void);
> +
>  #ifdef CONFIG_HOTPLUG_CPU
>  extern int arch_register_cpu(int num);
>  extern void arch_unregister_cpu(int);
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index a19a680542ce..1f6054523226 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -489,11 +489,7 @@ static void srat_detect_node(struct cpuinfo_x86 *c)
>  #define TME_ACTIVATE_CRYPTO_ALGS(x)	((x >> 48) & 0xffff)	/* Bits 63:48 */
>  #define TME_ACTIVATE_CRYPTO_AES_XTS_128	1
>  
> -/* Values for mktme_status (SW only construct) */
> -#define MKTME_ENABLED			0
> -#define MKTME_DISABLED			1
> -#define MKTME_UNINITIALIZED		2
> -static int mktme_status = MKTME_UNINITIALIZED;
> +static enum mktme_status_type mktme_status = MKTME_UNINITIALIZED;
>  
>  static void detect_tme(struct cpuinfo_x86 *c)
>  {
> @@ -1107,6 +1103,12 @@ bool handle_user_split_lock(struct pt_regs *regs, long error_code)
>  	return true;
>  }
>  
> +enum mktme_status_type get_mktme_status(void)
> +{
> +	return mktme_status;
> +}
> +EXPORT_SYMBOL_GPL(get_mktme_status);

prefix of the subsystem first please:
	mktme_get_status

Or, better yet, why not just export the variable directly?  Why is this
a function at all?

> +
>  /*
>   * This function is called only when switching between tasks with
>   * different split-lock detection modes. It sets the MSR for the
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 99e151475d8f..0dc978efbbd5 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -465,6 +465,17 @@ config PVPANIC
>  	  a paravirtualized device provided by QEMU; it lets a virtual machine
>  	  (guest) communicate panic events to the host.
>  
> +config MKTME_STATUS
> +	tristate "MKTME status reading support"
> +	depends on X86_64 && SECURITYFS
> +	help
> +	  This driver provides support for reading the MKTME status. The status
> +	  can be read from the mktme virtual file in the securityfs filesystem,
> +	  under the mktme directory.
> +
> +	  The MKTME (Multi-Key Total Memory Encryption) status can be
> +	  0 (enabled), 1 (disabled), or 3 (uninitialized).

name of the module, should you enable this, etc...

> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 9abf2923d831..f2f02efe34fd 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -58,3 +58,4 @@ obj-$(CONFIG_PVPANIC)   	+= pvpanic.o
>  obj-$(CONFIG_HABANA_AI)		+= habanalabs/
>  obj-$(CONFIG_UACCE)		+= uacce/
>  obj-$(CONFIG_XILINX_SDFEC)	+= xilinx_sdfec.o
> +obj-$(CONFIG_MKTME_STATUS)	+= mktme_status.o
> diff --git a/drivers/misc/mktme_status.c b/drivers/misc/mktme_status.c
> new file mode 100644
> index 000000000000..795993181e77
> --- /dev/null
> +++ b/drivers/misc/mktme_status.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MKTME Status driver
> + *
> + * Copyright 2020 (c) Daniel Gutson (daniel.gutson@eclypsium.com)
> + *
> + * This file is licensed under  the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.

these sentances are not needed if you have the SPDX line above, so
please drop them.

> + */
> +
> +#include <linux/module.h>
> +#include <linux/security.h>
> +#include <asm/cpu.h>
> +
> +#ifdef pr_fmt
> +#undef pr_fmt
> +#endif
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt

No need to check, just define the thing, BEFORE the #include lines.

> +
> +struct dentry *mktme_dir;
> +struct dentry *mktme_file;

static?

> +
> +/* Buffer to return: always 3 because of the following chars:
> + *     value \n \0
> + */
> +#define BUFFER_SIZE 3

Why a define?

> +
> +static ssize_t mktme_status_read(struct file *filp, char __user *buf,
> +				 size_t count, loff_t *ppos)
> +{
> +	char tmp[BUFFER_SIZE];
> +
> +	if (*ppos == BUFFER_SIZE)
> +		return 0; // nothing else to read

Why check this if you are using simple_read_from_buffer()?  Shouldn't
that handle this type of check for you correctly?  As it is, I don't
think you are doing this right anyway.

> +
> +	sprintf(tmp, "%d\n", (int)get_mktme_status() & 1);
> +	return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp));
> +}
> +
> +static const struct file_operations mktme_status_ops = {
> +	.read = mktme_status_read,
> +};
> +
> +static int __init mod_init(void)
> +{
> +	mktme_dir = securityfs_create_dir("mktme", NULL);
> +	if (IS_ERR(mktme_dir)) {
> +		pr_err("Couldn't create mktme sysfs dir\n");
> +		return -1;

Don't make up random error numbers, use the EWHATEVER defines please.

> +	}
> +
> +	pr_info("mktme securityfs dir creation successful\n");

If code works properly, it should be quiet, do not do this.  Would you
want to see your kernel log full of this for every individual virtual
file that was created?


> +
> +	mktme_file = securityfs_create_file("status", 0600, mktme_dir, NULL,
> +					    &mktme_status_ops);
> +	if (IS_ERR(mktme_file)) {
> +		pr_err("Error creating sysfs file bioswe\n");

"bioswe", what is that?

And this isn't sysfs.

Did anyone actually review this?


> +		goto out_file;
> +	}
> +
> +	return 0;
> +
> +out_file:
> +	securityfs_remove(mktme_file);
> +	securityfs_remove(mktme_dir);
> +	return -1;

Again, random return values, please do not.

> +}
> +
> +static void __exit mod_exit(void)
> +{
> +	securityfs_remove(mktme_file);
> +	securityfs_remove(mktme_dir);
> +}
> +
> +module_init(mod_init);
> +module_exit(mod_exit);
> +
> +MODULE_DESCRIPTION("MKTME Status driver");

Is this really a driver?

Also no Documentation/ABI/ update for your new userspace api that you
just created?

thanks,

greg k-h

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Thu, Jun 18, 2020 at 07:34:21PM -0300, Daniel Gutson wrote:
> Besides being a CPU capability, it has to be enabled by the BIOS, which is
> what the flag represents.

... yes, and if it is disabled in the BIOS, you clear the CPU cap flag.
Something like this untested diff:

---
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index c25a67a34bd3..59d8342f6a64 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -525,6 +525,7 @@ static void detect_tme(struct cpuinfo_x86 *c)
 	if (!TME_ACTIVATE_LOCKED(tme_activate) || !TME_ACTIVATE_ENABLED(tme_activate)) {
 		pr_info_once("x86/tme: not enabled by BIOS\n");
 		mktme_status = MKTME_DISABLED;
+		clear_cpu_cap(c, X86_FEATURE_TME);
 		return;
 	}
 
@@ -553,10 +554,11 @@ static void detect_tme(struct cpuinfo_x86 *c)
 		pr_info_once("x86/mktme: disabled by BIOS\n");
 	}
 
-	if (mktme_status == MKTME_UNINITIALIZED) {
-		/* MKTME is usable */
+	/* MKTME is usable */
+	if (mktme_status == MKTME_UNINITIALIZED)
 		mktme_status = MKTME_ENABLED;
-	}
+	else if (mktme_status == MKTME_DISABLED)
+		clear_cpu_cap(c, X86_FEATURE_TME);
 
 	/*
 	 * KeyID bits effectively lower the number of physical address

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Thu, Jun 18, 2020 at 04:52:18PM -0700, Dave Hansen wrote:
> Do we need another driver when running in a SEV guest to tell us about
> SEV's status?

We use /proc/cpuinfo for that.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Fri, Jun 19, 2020 at 10:01:36AM -0300, Daniel Gutson wrote:
> Then the user will not know that he/she could improve the security of the
> system by enabling the feature in the BIOS.

And how is the user going to know from your "module"? AFAICT, your
module loads on any system - not only on ones which have MKTME in CPUID.

> The fact that the CPU has the cap and the BIOS disables it, can
> trigger a prevention action.

I can only venture guesses what "prevention action" is - you'll have to
be more verbose here.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 00:52, Dave Hansen <dave.hansen@intel.com> wrote:
> It doesn't tell you if your data is encrypted.

Sorry for the perhaps naive question, but I thought MKTME was
essentially full physical memory encryption?

Richard

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 14:22, Borislav Petkov <bp@alien8.de> wrote:
> And how is the user going to know from your "module"? AFAICT, your
> module loads on any system - not only on ones which have MKTME in CPUID.

I maintain fwupd, which would be one consumer of this information. At
the moment we already look at the CPUID for the TME flag, which
successfully recognises CPU systems which support the feature. What we
don't know is if the firmware platform has disabled the MKTME feature.
Ideally we would export two things:

1. that the CPU supports TME (->cpuid, already done)
2. that the platform has not disabled TME in some way

The only way we have at the moment to see if TME is supported on the
platform (rather than just the CPU) is by grepping the entire systemd
journal at boot time, grepping for the "x86/tme: enabled by BIOS"
string. With a securityfs/sysfs/procfs file we don't have to do this
expensive operation for reading one tiny bit of data.

Richard

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/19/20 6:25 AM, Richard Hughes wrote:
> On Fri, 19 Jun 2020 at 00:52, Dave Hansen <dave.hansen@intel.com> wrote:
>> It doesn't tell you if your data is encrypted.
> Sorry for the perhaps naive question, but I thought MKTME was
> essentially full physical memory encryption?

Nope.

It means there is some encryption available.  But, it doesn't tell you
that your data is encrypted.  There is persistent memory which isn't
protected by TME, or necessarily protected by MKTME.  There can be
memory on accelerators attached by Compute Express Link which isn't
encrypted.  There's even an entire bit in the UEFI memory map to tell
the OS which memory can be encrypted or not.

On top of that, the kernel can just swap data out to unencrypted storage.

So, I really wonder what folks want from this flag in the first place.
It really tells you _nothing_.

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 14:33, Dave Hansen <dave.hansen@intel.com> wrote:
> On top of that, the kernel can just swap data out to unencrypted storage.

Right, but for the most part you'd agree that a machine with
functioning TME and encrypted swap partition is more secure than a
machine without TME?

> So, I really wonder what folks want from this flag in the first place.
> It really tells you _nothing_.

Can I use TME if the CPU supports it, but the platform has disabled
it? How do I know that my system is actually *using* the benefits the
TME feature provides?

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Fri, Jun 19, 2020 at 02:31:11PM +0100, Richard Hughes wrote:
> 1. that the CPU supports TME (->cpuid, already done)
> 2. that the platform has not disabled TME in some way

Yes, this is what I'm proposing with clearing the flag in /proc/cpuinfo.
The needed information is there:

1. TME in CPUID
2. TME *not* in /proc/cpuinfo

which means the platform doesn't support it.

If we are going to export a list of features which the OS
kernel/platform has enabled - and this means a contract between kernel
and userspace - then this should not be a misc driver which gets loaded
as a module but builtin, maybe a proper sysfs layout similar to

/sys/devices/system/cpu/vulnerabilities

which userspace can use. Along with proper ABI definition, design,
documentation and all that belongs to a proper interface with userspace.
Because once userspace uses it, it is practically cast in stone.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 14:44, Borislav Petkov <bp@alien8.de> wrote:
> Yes, this is what I'm proposing with clearing the flag in /proc/cpuinfo.
> The needed information is there:
> 1. TME in CPUID
> 2. TME *not* in /proc/cpuinfo

No, it's not a boolean at all. If the platform disable is a BIOS
configuration we don't know if TME isn't available because the CPU
doesn't support it or because the firmware has disabled it. In the
latter case, a firmware update or firmware configuration change might
actually enable it. If the user installs a CPU with TME support and
then we tell the user "your system doesn't support TME" then we're
going to have some very confused users unless we can differentiate the
two cases.

> Along with proper ABI definition, design,
> documentation and all that belongs to a proper interface with userspace.

I don't think Daniels patch was a "final version" and I'm sure
follow-ups can add this kind of thing. At the moment it's just people
telling him "you don't need this" when as a potential consumer I'm
saying we really do.

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/19/20 6:37 AM, Richard Hughes wrote:
> On Fri, 19 Jun 2020 at 14:33, Dave Hansen <dave.hansen@intel.com> wrote:
>> On top of that, the kernel can just swap data out to unencrypted storage.
> 
> Right, but for the most part you'd agree that a machine with
> functioning TME and encrypted swap partition is more secure than a
> machine without TME?

Nope.  There might be zero memory connected to the memory controller
that supports TME.

>> So, I really wonder what folks want from this flag in the first place.
>> It really tells you _nothing_.
> 
> Can I use TME if the CPU supports it, but the platform has disabled
> it? How do I know that my system is actually *using* the benefits the
> TME feature provides?

You must have a system with UEFI 2.8, ensure TME is enabled, then make
sure the OS parses EFI_MEMORY_CPU_CRYPTO, then ensure you request that
you data be placed in a region marked with EFI_MEMORY_CPU_CRYPTO, and
that it be *kept* there (hint: NUMA APIs don't do this).

Re: [PATCH] Ability to read the MKTME status from userspace

[Greg Kroah-Hartman]

On Fri, Jun 19, 2020 at 10:51:32AM -0300, Daniel Gutson wrote:
> > > +enum mktme_status_type get_mktme_status(void)
> > > +{
> > > +     return mktme_status;
> > > +}
> > > +EXPORT_SYMBOL_GPL(get_mktme_status);
> >
> > prefix of the subsystem first please:
> >         mktme_get_status
> >
> 
> OK.
> 
> > Or, better yet, why not just export the variable directly?  Why is this
> > a function at all?
> 
> Because I want this to be read only.

read-only to who?

> > > +
> > > +/* Buffer to return: always 3 because of the following chars:
> > > + *     value \n \0
> > > + */
> > > +#define BUFFER_SIZE 3
> >
> > Why a define?
> 
> Do you suggest `static const int` instead?

Why do you need it at all?

> > > +
> > > +     sprintf(tmp, "%d\n", (int)get_mktme_status() & 1);
> > > +     return simple_read_from_buffer(buf, count, ppos, tmp, sizeof(tmp));
> > > +}
> > > +
> > > +static const struct file_operations mktme_status_ops = {
> > > +     .read = mktme_status_read,
> > > +};
> > > +
> > > +static int __init mod_init(void)
> > > +{
> > > +     mktme_dir = securityfs_create_dir("mktme", NULL);
> > > +     if (IS_ERR(mktme_dir)) {
> > > +             pr_err("Couldn't create mktme sysfs dir\n");
> > > +             return -1;
> >
> > Don't make up random error numbers, use the EWHATEVER defines please.
> >
> 
> Could you please suggest one?

Why not return the error given to you?  Why throw that information away?

> > Also no Documentation/ABI/ update for your new userspace api that you
> > just created?
> >
> 
> should that be a comment in the .h?

No, you need a description in Documentation/ABI/ about any
sysfs/configfs/securityfs/whatever for new things you are creating.

thanks,

greg k-h

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 14:58, Dave Hansen <dave.hansen@intel.com> wrote:
> > Right, but for the most part you'd agree that a machine with
> > functioning TME and encrypted swap partition is more secure than a
> > machine without TME?
>
> Nope.  There might be zero memory connected to the memory controller
> that supports TME.

So you're saying that a machine with TME available and enabled is not
considered more secure than a machine without TME?

What I want to do is have a sliding scale of TME not available < TME
available but disabled < TME available and enabled < TME available,
enabled and being used. The extra nugget of data gets me from state 2
to state 3.

> > Can I use TME if the CPU supports it, but the platform has disabled
> > it? How do I know that my system is actually *using* the benefits the
> > TME feature provides?
>
> You must have a system with UEFI 2.8, ensure TME is enabled, then make
> sure the OS parses EFI_MEMORY_CPU_CRYPTO, then ensure you request that
> you data be placed in a region marked with EFI_MEMORY_CPU_CRYPTO, and
> that it be *kept* there (hint: NUMA APIs don't do this).

So my take-away from that is that it's currently impossible to
actually say if your system is *actually* using TME.

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/19/20 7:09 AM, Richard Hughes wrote:
> On Fri, 19 Jun 2020 at 14:58, Dave Hansen <dave.hansen@intel.com> wrote:
>>> Right, but for the most part you'd agree that a machine with
>>> functioning TME and encrypted swap partition is more secure than a
>>> machine without TME?
>>
>> Nope.  There might be zero memory connected to the memory controller
>> that supports TME.
> 
> So you're saying that a machine with TME available and enabled is not
> considered more secure than a machine without TME?

Yes, it is not necessarily more secure.

> What I want to do is have a sliding scale of TME not available < TME
> available but disabled < TME available and enabled < TME available,
> enabled and being used. The extra nugget of data gets me from state 2
> to state 3.

I'd assert that availability tells you nothing if you don't pair it with
use.

Last night, I asked my kids if they brushed their teeth.  They said:
"Dad, my toothbrush was available."  They argued that mere availability
was a better situation than not *having* a toothbrush.  They were
logically right, of course, but they still got cavities.

>>> Can I use TME if the CPU supports it, but the platform has disabled
>>> it? How do I know that my system is actually *using* the benefits the
>>> TME feature provides?
>>
>> You must have a system with UEFI 2.8, ensure TME is enabled, then make
>> sure the OS parses EFI_MEMORY_CPU_CRYPTO, then ensure you request that
>> you data be placed in a region marked with EFI_MEMORY_CPU_CRYPTO, and
>> that it be *kept* there (hint: NUMA APIs don't do this).
> 
> So my take-away from that is that it's currently impossible to
> actually say if your system is *actually* using TME.

Not in a generic way, and it can't be derived from cpuid or MSRs alone.

Let's say I'm buying a fleet of servers.  I know I'm buying some fancy
Xeon with TME, I know I'm only using DRAM for storing user data, and I
don't have any accelerators around.  I control and enforce my BIOS
settings.  I'm pretty sure I'm using TME, but I didn't become sure from
poking at sysfs.

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 15:23, Dave Hansen <dave.hansen@intel.com> wrote:
> Last night, I asked my kids if they brushed their teeth.  They said:
> "Dad, my toothbrush was available."  They argued that mere availability
> was a better situation than not *having* a toothbrush.  They were
> logically right, of course, but they still got cavities.

I don't see how that's comparable, sorry. Surely Intel wants to sell
hardware advertising TME as a security feature?

> > So my take-away from that is that it's currently impossible to
> > actually say if your system is *actually* using TME.
> Not in a generic way, and it can't be derived from cpuid or MSRs alone.

Well, it seems not in any way at the moment.

> I'm pretty sure I'm using TME, but I didn't become sure from
> poking at sysfs.

How do you know that Lenovo didn't disable TME without looking at
dmesg? I don't think "pretty sure" is good enough when TME is
considered a security feature.

Richard

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/19/20 7:36 AM, Richard Hughes wrote:
> On Fri, 19 Jun 2020 at 15:23, Dave Hansen <dave.hansen@intel.com> wrote:
>> Last night, I asked my kids if they brushed their teeth.  They said:
>> "Dad, my toothbrush was available."  They argued that mere availability
>> was a better situation than not *having* a toothbrush.  They were
>> logically right, of course, but they still got cavities.
> 
> I don't see how that's comparable, sorry. Surely Intel wants to sell
> hardware advertising TME as a security feature?

Of course!  Just like AVX-512 or VNNI or whatever, Intel will totally
tell you about the stuff baked into its silicon!  But, just like
AVX-512, there's a lot of work to do on top of mere presence in the
silicon to ensure it is providing benefit.

>>> So my take-away from that is that it's currently impossible to
>>> actually say if your system is *actually* using TME.
>> Not in a generic way, and it can't be derived from cpuid or MSRs alone.
> 
> Well, it seems not in any way at the moment.
> 
>> I'm pretty sure I'm using TME, but I didn't become sure from
>> poking at sysfs.
> 
> How do you know that Lenovo didn't disable TME without looking at
> dmesg? I don't think "pretty sure" is good enough when TME is
> considered a security feature.

You cut out the important part.  The "pretty sure" involves a bunch of
preconditions and knowing what your hardware configuration is in the
first place.

Let's take a step back.  We add read-only ABIs so that decisions can be
made.  What decision will somebody make from the ABI being proposed here?

Someone does 'cat /proc/mktme' (or whatever) and it says "1" or
whatever, which means yay, encryption is on.  What do they do?

What do they do differently when it says "0"?

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 15:48, Dave Hansen <dave.hansen@intel.com> wrote:
> You cut out the important part.  The "pretty sure" involves a bunch of
> preconditions and knowing what your hardware configuration is in the
> first place.

Totally agree.

> Let's take a step back.  We add read-only ABIs so that decisions can be
> made.  What decision will somebody make from the ABI being proposed here?

The question of "is my memory encrypted" is what I'm trying to decide.
To the end user (or the person marking a compliance ticksheet for a
government contract) all they want to know is the end result. At the
moment for AMD SME this seems much simpler as there are less
"preconditions".

> Someone does 'cat /proc/mktme' (or whatever) and it says "1" or
> whatever, which means yay, encryption is on.  What do they do?

I think "is my memory encrypted" for Intel has to be a superset of:

1. TME in CPU info
2. not disabled by the platform
3. not using unencrypted swap
4. not using a memory accelerator
5. entire DRAM area is marked with EFI_MEMORY_CPU_CRYPTO

It seems the only way to answer the questions and make it easy for the
consumer to know the answer is to ask the kernel for each of the 5
different questions. At the moment we can only get 1, 3, maybe 4, soon
to be 5, but not 2.

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/19/20 8:02 AM, Richard Hughes wrote:
>> Someone does 'cat /proc/mktme' (or whatever) and it says "1" or
>> whatever, which means yay, encryption is on.  What do they do?
> I think "is my memory encrypted" for Intel has to be a superset of:
> 
> 1. TME in CPU info
> 2. not disabled by the platform
> 3. not using unencrypted swap
> 4. not using a memory accelerator
> 5. entire DRAM area is marked with EFI_MEMORY_CPU_CRYPTO

Also realize that this can all be true at one point in time, but can
change if memory is added.

> It seems the only way to answer the questions and make it easy for the
> consumer to know the answer is to ask the kernel for each of the 5
> different questions. At the moment we can only get 1, 3, maybe 4, soon
> to be 5, but not 2.

Actually, the accelerators I had in mind would show up in the memory map
and would have EFI_MEMORY_CPU_CRYPTO properly set by the firmware.

In any case, if we do something like this, I think it fundamentally
needs to be more fine-grained than the whole system.  It probably needs
to be on a per-NUMA-node basis.  That's really the only way for us to
provide meaningful promises about encryption to end users.

Re: [PATCH] Ability to read the MKTME status from userspace

[Andy Lutomirski]

> On Jun 19, 2020, at 6:50 AM, Richard Hughes <hughsient@gmail.com> wrote:
> 
> On Fri, 19 Jun 2020 at 14:44, Borislav Petkov <bp@alien8.de> wrote:
>> Yes, this is what I'm proposing with clearing the flag in /proc/cpuinfo.
>> The needed information is there:
>> 1. TME in CPUID
>> 2. TME *not* in /proc/cpuinfo
> 
> No, it's not a boolean at all. If the platform disable is a BIOS
> configuration we don't know if TME isn't available because the CPU
> doesn't support it or because the firmware has disabled it. In the
> latter case, a firmware update or firmware configuration change might
> actually enable it. If the user installs a CPU with TME support and
> then we tell the user "your system doesn't support TME" then we're
> going to have some very confused users unless we can differentiate the
> two cases.
> 
>> Along with proper ABI definition, design,
>> documentation and all that belongs to a proper interface with userspace.
> 
> I don't think Daniels patch was a "final version" and I'm sure
> follow-ups can add this kind of thing. At the moment it's just people
> telling him "you don't need this" when as a potential consumer I'm
> saying we really do.

I think it’s reasonable for the kernel to ask why.

Is the idea that some GUI would show a big warning like “your silly BIOS has TME disabled”?

Boris, it wouldn’t be totally crazy for cpuinfo to learn to distinguish between “your platform has this feature but Linux isn’t using it” and “your platform doesn’t have this feature in the first place”. And I suppose there’s this extra silly state “your platform has this feature, but your firmware didn’t enable it”.  This would be a big job.

Regardless, knowing what the actual point of this patch is would be nice.

> 
> Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Fri, Jun 19, 2020 at 02:50:39PM +0100, Richard Hughes wrote:
> No, it's not a boolean at all.

So before you do any kernel modules, you have to sit down and precisely
define what you want do to:

- do you want to initialize different stuff in your application based on
platform support?

- do you want to tell the user something so that the user can act
accordingly?

- do you just want to display feature support?

- ...

All this needs to be properly written down and discussed first, and then
we can think of the best way for the kernel to provide it, if there is
even any kind of info the kernel can provide reliably.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Borislav Petkov]

On Fri, Jun 19, 2020 at 08:48:47AM -0700, Andy Lutomirski wrote:
> Boris, it wouldn’t be totally crazy for cpuinfo to learn to
> distinguish between “your platform has this feature but Linux
> isn’t using it” and “your platform doesn’t have this feature
> in the first place”. And I suppose there’s this extra silly state
> “your platform has this feature, but your firmware didn’t enable
> it”. This would be a big job.

Well, I believe all the kernel can do is supply bits of information -
just like MSRs - and depending on the settings of those bits, userspace
can decide what the situation is. For example:

bit 0 - CPUID support
bit 1 - BIOS enabled
bit 2 - quirk applied
bit 3 - microcode fixes present
...

and so on.

It needs a proper definition though and userspace to say, yes, we want
that and that is useful for us.

Where it ends up is then beside the point - /proc/cpuinfo,
/sys/devices/system/cpu, whatever...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Andy Lutomirski]

> On Jun 19, 2020, at 9:17 AM, Borislav Petkov <bp@alien8.de> wrote:
> 
> On Fri, Jun 19, 2020 at 08:48:47AM -0700, Andy Lutomirski wrote:
>> Boris, it wouldn’t be totally crazy for cpuinfo to learn to
>> distinguish between “your platform has this feature but Linux
>> isn’t using it” and “your platform doesn’t have this feature
>> in the first place”. And I suppose there’s this extra silly state
>> “your platform has this feature, but your firmware didn’t enable
>> it”. This would be a big job.
> 
> Well, I believe all the kernel can do is supply bits of information -
> just like MSRs - and depending on the settings of those bits, userspace
> can decide what the situation is. For example:
> 
> bit 0 - CPUID support
> bit 1 - BIOS enabled
> bit 2 - quirk applied
> bit 3 - microcode fixes present
> ...
> 
> and so on.
> 

Indeed.

> It needs a proper definition though and userspace to say, yes, we want
> that and that is useful for us.

Maybe the right approach is to just keep this kind of use in mind for when we inevitably redo cpu features when Intel ships their hybrid Atom/Core machines.

> 
> Where it ends up is then beside the point - /proc/cpuinfo,
> /sys/devices/system/cpu, whatever...
> 
> -- 
> Regards/Gruss,
>    Boris.
> 
> https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 17:17, Borislav Petkov <bp@alien8.de> wrote:
> Well, I believe all the kernel can do is supply bits of information -
> just like MSRs - and depending on the settings of those bits, userspace
> can decide what the situation is. For example:
> bit 0 - CPUID support
> bit 1 - BIOS enabled

Yes, something like this would be entirely fine for my purpose -- I
don't mind where or how I get the data out of the kernel, but parsing
the entire systemd journal for a magic string is something I really
want to avoid.

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 17:10, Borislav Petkov <bp@alien8.de> wrote:
> - do you just want to display feature support?

Yes. I want to show the user *why* TME is not available.

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Greg Kroah-Hartman]

On Fri, Jun 19, 2020 at 05:33:39PM +0100, Richard Hughes wrote:
> On Fri, 19 Jun 2020 at 17:10, Borislav Petkov <bp@alien8.de> wrote:
> > - do you just want to display feature support?
> 
> Yes. I want to show the user *why* TME is not available.

So even if it is "available" that's fine, even if it is not being used?

And how can you ever tell if a BIOS disables a CPU feature, yet the chip
still has it?  That should be not seen from the OS as it really doesn't
care, it just knows if the feature is able to be used or not.

thanks,

greg k-h

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 17:41, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> > Yes. I want to show the user *why* TME is not available.
> So even if it is "available" that's fine, even if it is not being used?

No, it's just one more thing we can check and report. For instance,
"Full memory encryption: NO [firmware-disabled, unencrypted-swap, EFI
memory map incomplete]

> And how can you ever tell if a BIOS disables a CPU feature, yet the chip
> still has it?

Isn't that what the "x86/tme: enabled by BIOS" kernel log entry is for?

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Andy Lutomirski]

On Fri, Jun 19, 2020 at 9:47 AM Richard Hughes <hughsient@gmail.com> wrote:
>
> On Fri, 19 Jun 2020 at 17:41, Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > > Yes. I want to show the user *why* TME is not available.
> > So even if it is "available" that's fine, even if it is not being used?
>
> No, it's just one more thing we can check and report. For instance,
> "Full memory encryption: NO [firmware-disabled, unencrypted-swap, EFI
> memory map incomplete]

The list is bigger than this, and it will probably get even bigger in
the future.  For example, if the specific thing you care about is "is
my memory encrypted on the DIMM", choices include:

 - Yes, mostly, TME
 - Yes, mostly, SME
 - Yes, mostly, SEV (which comes in several variants)
 - Yes, because this is actually a Graphene SGX enclave or similar.
 - No, your memory controller can't do this.
 - No.  Although your memory controller can do this, it isn't right
now, because [reason].
 - (in the future, maybe) Partially, because *MK* TME is enabled and
encryption depends on the specific policy.
 - (in the future, maybe) No, but you think yes, because MKTME is
enabled and you used a fixed key that is stored somewhere.
 - (in the future, maybe) Maybe, because some memory is encrypted and
some isn't.

But if what you *actually* care about is whether someone who resets
the system and takes over gets to learn the contents of the DIMMs
(i.e. a "cold-boot attack"), then there are more answers:

 - Protected because of TXT: the memory controller will not allow
reads until the DIMMs are cleared.
 - Protected because of whatever AMD's equivalent is.
 - Protected to some extent because the installed firmware will wipe
memory on reset.

If you care about whether the contents of anonymous memory will be
encrypted at rest on swap, then you care about what the swap backing
store is *and* where the key came from.

If you care about whether and to what degree the contents of anonymous
memory are protected from a malicious hypervisor, the answer is
complicated.

I don't object in principle to Linux giving userspace more visibility
into what's going on, but I'm not convinced that adding a new
must-support-for-a-long-time interface that only solves 5% of your
problem is worth it.  Especially if, some day, the shiny new interface
says "TME is on", but this really means "MKTME is on and the
administrator configured it to only encrypt specific things for
performance reasons".

Re: [PATCH] Ability to read the MKTME status from userspace

[Richard Hughes]

On Fri, 19 Jun 2020 at 20:41, Andy Lutomirski <luto@kernel.org> wrote:
> I don't object in principle to Linux giving userspace more visibility
> into what's going on, but I'm not convinced that adding a new
> must-support-for-a-long-time interface that only solves 5% of your
> problem is worth it.

At the moment the only visibility we have is "the CPU supports TME"
and "the kernel printed a message in the journal". The sysfs/procfs
file read allows us to notify the admin if the firmware is
deliberately disabling TME for some reason, without resorting to
`grep` on dmesg. I don't think perfect should be the enemy of the
good.

Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Andy Lutomirski]

On Fri, Jun 19, 2020 at 12:58 PM Richard Hughes <hughsient@gmail.com> wrote:
>
> On Fri, 19 Jun 2020 at 20:41, Andy Lutomirski <luto@kernel.org> wrote:
> > I don't object in principle to Linux giving userspace more visibility
> > into what's going on, but I'm not convinced that adding a new
> > must-support-for-a-long-time interface that only solves 5% of your
> > problem is worth it.
>
> At the moment the only visibility we have is "the CPU supports TME"
> and "the kernel printed a message in the journal". The sysfs/procfs
> file read allows us to notify the admin if the firmware is
> deliberately disabling TME for some reason, without resorting to
> `grep` on dmesg. I don't think perfect should be the enemy of the
> good.

I am unconvinced that this has hit the "good" bar, especially since
SME is completely missing here.

Boris, etc: would it be reasonable to add a list of CPU features that
are present but turned off by firmware?  SME is far from the only
thing that's frequently in this category.  x2apic, fast strings, and
virtualization come to mind.

>
> Richard.

Re: [PATCH] Ability to read the MKTME status from userspace

[Dave Hansen]

On 6/19/20 1:20 PM, Andy Lutomirski wrote:
> Boris, etc: would it be reasonable to add a list of CPU features that
> are present but turned off by firmware?  SME is far from the only
> thing that's frequently in this category.  x2apic, fast strings, and
> virtualization come to mind.

Sounds sane to me.  I like the idea of proving ammo to end users to
either go flip a BIOS switch, or yell at their firmware vendor.

Re: [PATCH] Ability to read the MKTME status from userspace

[Boris Petkov]

On June 19, 2020 10:24:23 PM GMT+02:00, Dave Hansen <dave.hansen@intel.com> wrote:
>On 6/19/20 1:20 PM, Andy Lutomirski wrote:
>> Boris, etc: would it be reasonable to add a list of CPU features that
>> are present but turned off by firmware?  SME is far from the only
>> thing that's frequently in this category.  x2apic, fast strings, and
>> virtualization come to mind.
>
>Sounds sane to me.  I like the idea of proving ammo to end users to
>either go flip a BIOS switch, or yell at their firmware vendor.

Sure if the reenabling the feature in BIOS would enable the support. Which is not the case with TME, as ypu pointed out, so I'm not sure a list CPU features which are present but turned off in firmware, is enough.

I'm thinking more along the lines of adding freetext doc for such "complex" to enable features which explains to users what and where to check, what to switch on and off and what other prerequisites can be...

And yes, it is ugly. ;-/

-- 
Sent from a small device: formatting sux and brevity is inevitable.