[MODERATED] Re: [PATCH 1/1 v4] Linux patch #1

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

[-- Attachment #1: Type: text/plain, Size: 8130 bytes --]

..snip..
> What should be done next on top of this:
>         - once Paolo's/Konrad's KVM bits land in the tree, they should 
>           look at the currently active mitigation setting and decide about 
>           doing L1D flushes based on that

I would say the inverse. That is this patch should be on top of the kvm pile
as it simplies it a bit, but <shrugs>

Anyhow got a couple of input that were raised when I posted the patch for KVM
for the warning.

>         - sysfs toggling can also be added later on top
> 
> 
>  Documentation/admin-guide/kernel-parameters.txt | 18 ++++++++
>  arch/x86/Kconfig                                | 18 ++++++++
>  arch/x86/include/asm/processor.h                |  7 ++++
>  arch/x86/kernel/cpu/bugs.c                      | 56 +++++++++++++++++++++++--
>  arch/x86/kvm/vmx.c                              | 19 +++++++++
>  include/linux/cpu.h                             |  2 +
>  kernel/cpu.c                                    |  9 +++-
>  7 files changed, 124 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 8e29c4b6756f..5dc277555ea6 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1971,6 +1971,24 @@
>  			feature (tagged TLBs) on capable Intel chips.
>  			Default is 1 (enabled)
>  
> +	l1tf=           [X86] Control mitigation of L1TF vulnerability on the
> +			      affected CPUs
> +			full	Provide all available mitigations for L1TF
> +				vulnerability (disable HT, perform PTE bit
> +				inversion, allow hypervisors to know that
> +				they should provide all mitigations)
> +			novirt	Provide all available mitigations needed
> +				for running on bare metal (PTE bit inversion),
> +				while not applying mitigations needed for
> +				VM isolation. Hypervisors will be issuing
> +				warning when first VM is being started in
> +				pontentially insecure configuraion
> +			off	Claim "I don't care at all about this issue".
> +				The PTE bit inversion (bare metal mitigation) will
> +				still be performed, but hypervisors will not be
> +				issuing warning when VM is being started in
> +				potentially insecure configuration
> +
>  	l2cr=		[PPC]
>  
>  	l3cr=		[PPC]
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 7a34fdf8daf0..a5231a0812e3 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -2390,6 +2390,24 @@ config MODIFY_LDT_SYSCALL
>  	  surface.  Disabling it removes the modify_ldt(2) system call.
>  
>  	  Saying 'N' here may make sense for embedded or server kernels.
> +choice
> +	prompt "Default L1TF mitigation"
> +	default L1TF_MITIGATION_NOVIRT
> +	help
> +		Define what the default behavior for selecting mitigation on
> +		CPUs affected by L1TF should be. The default can be overrided
> +		on the kernel command-line. Refer to
> +		<file:Documentation/admin-guide/kernel-parameters.txt>
> +
> +config L1TF_MITIGATION_FULL
> +	bool "Full available L1TF mitigation"
> +config L1TF_MITIGATION_NOVIRT
> +	bool "Use L1TF bare metal mitigations only"
> +config L1TF_MITIGATION_OFF
> +	bool "Ignore L1TF issue"
> +
> +endchoice
> +
>  
>  source "kernel/livepatch/Kconfig"
>  
> diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
> index 7e3ac5eedcd6..05471c590964 100644
> --- a/arch/x86/include/asm/processor.h
> +++ b/arch/x86/include/asm/processor.h
> @@ -982,4 +982,11 @@ bool xen_set_default_idle(void);
>  void stop_this_cpu(void *dummy);
>  void df_debug(struct pt_regs *regs, long error_code);
>  void microcode_check(void);
> +
> +enum l1tf_mitigations {
> +	L1TF_MITIGATION_OFF,
> +	L1TF_MITIGATION_NOVIRT,
> +	L1TF_MITIGATION_FULL
> +};
> +enum l1tf_mitigations get_l1tf_mitigation(void);
>  #endif /* _ASM_X86_PROCESSOR_H */
> diff --git a/arch/x86/kernel/cpu/bugs.c b/arch/x86/kernel/cpu/bugs.c
> index 50500cea6eba..9aa8b94334d5 100644
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -657,6 +657,23 @@ void x86_spec_ctrl_setup_ap(void)
>  
>  #undef pr_fmt
>  #define pr_fmt(fmt)	"L1TF: " fmt
> +/* Default mitigation for L1TF-affected CPUs */
> +static int l1tf_mitigation =
> +#ifdef CONFIG_L1TF_MITIGATION_FULL
> +	L1TF_MITIGATION_NOVIRT;
> +#endif
> +#ifdef CONFIG_L1TF_MITIGATION_NOVIRT
> +	L1TF_MITIGATION_NOVIRT;
> +#endif
> +#ifdef CONFIG_L1TF_MITIGATION_OFF
> +	L1TF_MITIGATION_OFF;
> +#endif
> +enum l1tf_mitigations get_l1tf_mitigation(void)
> +{
> +	return l1tf_mitigation;
> +}
> +EXPORT_SYMBOL(get_l1tf_mitigation);
> +
>  static void __init l1tf_select_mitigation(void)
>  {
>  	u64 half_pa;
> @@ -664,6 +681,15 @@ static void __init l1tf_select_mitigation(void)
>  	if (!boot_cpu_has_bug(X86_BUG_L1TF))
>  		return;
>  
> +	switch (get_l1tf_mitigation()) {
> +	case L1TF_MITIGATION_FULL:
> +		cpu_smt_disable(true);
> +		break;
> +	case L1TF_MITIGATION_OFF:
> +	case L1TF_MITIGATION_NOVIRT:
> +		break;
> +	}
> +
>  #if CONFIG_PGTABLE_LEVELS == 2
>  	pr_warn("Kernel not compiled for PAE. No mitigation for L1TF\n");
>  	return;
> @@ -682,10 +708,36 @@ static void __init l1tf_select_mitigation(void)
>  
>  	setup_force_cpu_cap(X86_FEATURE_L1TF_PTEINV);
>  }
> +
> +static int __init l1tf_cmdline(char *str)
> +{
> +	if (!boot_cpu_has_bug(X86_BUG_L1TF))
> +		return 0;
> +
> +	if (!str)
> +		return 0;
> +
> +	if (!strcmp(str, "full"))
> +		l1tf_mitigation = L1TF_MITIGATION_FULL;
> +	else if (!strcmp(str, "novirt"))
> +		l1tf_mitigation = L1TF_MITIGATION_NOVIRT;
> +	else if (!strcmp(str, "off"))
> +		l1tf_mitigation = L1TF_MITIGATION_OFF;
> +
> +	return 0;
> +}
> +early_param("l1tf", l1tf_cmdline);
> +
>  #undef pr_fmt
>  
>  #ifdef CONFIG_SYSFS
>  
> +static const char *l1tf_states[] = {
> +	[L1TF_MITIGATION_FULL]		= "Mitigation: Full",
> +	[L1TF_MITIGATION_NOVIRT]	= "Mitigation: Page Table Inversion",
> +	[L1TF_MITIGATION_OFF]		= "Mitigation: Page Table Inversion"
> +};
> +
>  static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr,
>  			       char *buf, unsigned int bug)
>  {
> @@ -712,9 +764,7 @@ static ssize_t cpu_show_common(struct device *dev, struct device_attribute *attr
>  		return sprintf(buf, "%s\n", ssb_strings[ssb_mode]);
>  
>  	case X86_BUG_L1TF:
> -		if (boot_cpu_has(X86_FEATURE_L1TF_PTEINV))
> -			return sprintf(buf, "Mitigation: Page Table Inversion\n");
> -		break;
> +		return sprintf(buf, "%s\n", l1tf_states[get_l1tf_mitigation()]);
>  
>  	default:
>  		break;
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 559a12b6184d..8a5921ad38e2 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -10370,10 +10370,29 @@ static struct kvm_vcpu *vmx_create_vcpu(struct kvm *kvm, unsigned int id)
>  	return ERR_PTR(err);
>  }
>  
> +#define L1TF_MSG_NOVIRT "kvm: L1TF CPU bug present and virtualization mitigation disabled. Refer to CVE-2018-3620 for details.\n"
> +#define L1TF_MSG_FULL "kvm: L1TF CPU bug present and KVM lacks support for L1D flushes. Refer to CVE-2018-3620 for details.\n"
>  static int vmx_vm_init(struct kvm *kvm)

This should be in a different function - when the guest is created/started.

>  {
>  	if (!ple_gap)
>  		kvm->arch.pause_in_guest = true;
> +	if (boot_cpu_has(X86_BUG_L1TF)) {
> +			switch (get_l1tf_mitigation()) {
> +			case L1TF_MITIGATION_OFF:
> +				break;
> +			case L1TF_MITIGATION_NOVIRT:
> +				printk_once (KERN_ERR L1TF_MSG_NOVIRT);

Linus/Paolo/etc mentioned that it should be WARN not ERR unless you really want to enforce
it in which case it should an error and fail the creation of the guest. Not sure if this patch does that?

> +				break;
> +			case L1TF_MITIGATION_FULL:
> +				/*
> +				 * FIXME: once L1D flushes are implemented for
> +				 * VMX, this will go away and L1TF_MITIGATION_FULL
> +				 * would imply L1D flushing being turned on

Missing stop.
> +				 */
> +				printk_once (KERN_ERR L1TF_MSG_FULL);

But more importantly, I think you are missing the check to see .. why not just
rebase this on top of the kvm/pile. Then you already have the right CPU bits.

Attaching the bundle I had sent to Thomas.

[-- Attachment #2: kvm.l1tf.v5.rc2.bundle --]
[-- Type: application/octet-stream, Size: 12106 bytes --]