[PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Andy Lutomirski]

virtio-rng is both too complicated and insufficient for initial rng
seeding.  It's far too complicated to use for KASLR or any other
early boot random number needs.  It also provides /dev/random-style
bits, which means that making guest boot wait for virtio-rng is
unacceptably slow, and doing it asynchronously means that
/dev/urandom might be predictable when userspace starts.

This introduces a very simple synchronous mechanism to get
/dev/urandom-style bits.

This is a KVM change: am I supposed to write a unit test somewhere?

Andy Lutomirski (4):
  x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit
  random,x86: Add arch_get_slow_rng_u64
  random: Seed pools from arch_get_slow_rng_u64 at startup
  x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available

 Documentation/virtual/kvm/cpuid.txt  |  3 +++
 arch/x86/Kconfig                     |  4 ++++
 arch/x86/boot/compressed/aslr.c      | 27 +++++++++++++++++++++++++++
 arch/x86/include/asm/archslowrng.h   | 30 ++++++++++++++++++++++++++++++
 arch/x86/include/uapi/asm/kvm_para.h |  2 ++
 arch/x86/kernel/kvm.c                | 22 ++++++++++++++++++++++
 arch/x86/kvm/cpuid.c                 |  3 ++-
 arch/x86/kvm/x86.c                   |  4 ++++
 drivers/char/random.c                | 14 +++++++++++++-
 include/linux/random.h               |  9 +++++++++
 10 files changed, 116 insertions(+), 2 deletions(-)
 create mode 100644 arch/x86/include/asm/archslowrng.h

-- 
1.9.3

[PATCH 1/4] x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit

[Andy Lutomirski]

This adds a simple interface to allow a guest to request 64 bits of
host nonblocking entropy.  This is independent of virtio-rng for a
couple of reasons:

 - It's intended to be usable during early boot, when a trivial
   synchronous interface is needed.

 - virtio-rng gives blocking entropy, and making guest boot wait for
   the host's /dev/random will cause problems.

MSR_KVM_GET_RNG_SEED is intended to provide 64 bits of best-effort
cryptographically secure data for use as a seed.  It provides no
guarantee that the result contains any actual entropy.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 Documentation/virtual/kvm/cpuid.txt  | 3 +++
 arch/x86/include/uapi/asm/kvm_para.h | 2 ++
 arch/x86/kvm/cpuid.c                 | 3 ++-
 arch/x86/kvm/x86.c                   | 4 ++++
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/virtual/kvm/cpuid.txt b/Documentation/virtual/kvm/cpuid.txt
index 3c65feb..0ab043b 100644
--- a/Documentation/virtual/kvm/cpuid.txt
+++ b/Documentation/virtual/kvm/cpuid.txt
@@ -54,6 +54,9 @@ KVM_FEATURE_PV_UNHALT              ||     7 || guest checks this feature bit
                                    ||       || before enabling paravirtualized
                                    ||       || spinlock support.
 ------------------------------------------------------------------------------
+KVM_FEATURE_GET_RNG_SEED           ||     8 || host provides rng seed data via
+                                   ||       || MSR_KVM_GET_RNG_SEED.
+------------------------------------------------------------------------------
 KVM_FEATURE_CLOCKSOURCE_STABLE_BIT ||    24 || host will warn if no guest-side
                                    ||       || per-cpu warps are expected in
                                    ||       || kvmclock.
diff --git a/arch/x86/include/uapi/asm/kvm_para.h b/arch/x86/include/uapi/asm/kvm_para.h
index 94dc8ca..e2eaf93 100644
--- a/arch/x86/include/uapi/asm/kvm_para.h
+++ b/arch/x86/include/uapi/asm/kvm_para.h
@@ -24,6 +24,7 @@
 #define KVM_FEATURE_STEAL_TIME		5
 #define KVM_FEATURE_PV_EOI		6
 #define KVM_FEATURE_PV_UNHALT		7
+#define KVM_FEATURE_GET_RNG_SEED	8
 
 /* The last 8 bits are used to indicate how to interpret the flags field
  * in pvclock structure. If no bits are set, all flags are ignored.
@@ -40,6 +41,7 @@
 #define MSR_KVM_ASYNC_PF_EN 0x4b564d02
 #define MSR_KVM_STEAL_TIME  0x4b564d03
 #define MSR_KVM_PV_EOI_EN      0x4b564d04
+#define MSR_KVM_GET_RNG_SEED 0x4b564d05
 
 struct kvm_steal_time {
 	__u64 steal;
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 38a0afe..40d6763 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -479,7 +479,8 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
 			     (1 << KVM_FEATURE_ASYNC_PF) |
 			     (1 << KVM_FEATURE_PV_EOI) |
 			     (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT) |
-			     (1 << KVM_FEATURE_PV_UNHALT);
+			     (1 << KVM_FEATURE_PV_UNHALT) |
+			     (1 << KVM_FEATURE_GET_RNG_SEED);
 
 		if (sched_info_on())
 			entry->eax |= (1 << KVM_FEATURE_STEAL_TIME);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index f644933..4e81853 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -48,6 +48,7 @@
 #include <linux/pci.h>
 #include <linux/timekeeper_internal.h>
 #include <linux/pvclock_gtod.h>
+#include <linux/random.h>
 #include <trace/events/kvm.h>
 
 #define CREATE_TRACE_POINTS
@@ -2480,6 +2481,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, u32 msr, u64 *pdata)
 	case MSR_KVM_PV_EOI_EN:
 		data = vcpu->arch.pv_eoi.msr_val;
 		break;
+	case MSR_KVM_GET_RNG_SEED:
+		get_random_bytes(&data, sizeof(data));
+		break;
 	case MSR_IA32_P5_MC_ADDR:
 	case MSR_IA32_P5_MC_TYPE:
 	case MSR_IA32_MCG_CAP:
-- 
1.9.3

[PATCH 2/4] random,x86: Add arch_get_slow_rng_u64

[Andy Lutomirski]

arch_get_slow_rng_u64 tries to get 64 bits of RNG seed data.  Unlike
arch_get_random_{bytes,seed}, etc., it makes no claims about entropy
content.  It's also likely to be much slower and should not be used
frequently.  That being said, it should be fast enough to call
several times during boot without any noticeable slowdown.

This initial implementation backs it with MSR_KVM_GET_RNG_SEED if
available.  The intent is for other hypervisor guest implementations
to implement this interface.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/Kconfig                   |  4 ++++
 arch/x86/include/asm/archslowrng.h | 30 ++++++++++++++++++++++++++++++
 arch/x86/kernel/kvm.c              | 22 ++++++++++++++++++++++
 include/linux/random.h             |  9 +++++++++
 4 files changed, 65 insertions(+)
 create mode 100644 arch/x86/include/asm/archslowrng.h

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a8f749e..4dfb539 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -593,6 +593,7 @@ config KVM_GUEST
 	bool "KVM Guest support (including kvmclock)"
 	depends on PARAVIRT
 	select PARAVIRT_CLOCK
+	select ARCH_SLOW_RNG
 	default y
 	---help---
 	  This option enables various optimizations for running under the KVM
@@ -627,6 +628,9 @@ config PARAVIRT_TIME_ACCOUNTING
 config PARAVIRT_CLOCK
 	bool
 
+config ARCH_SLOW_RNG
+       bool
+
 endif #HYPERVISOR_GUEST
 
 config NO_BOOTMEM
diff --git a/arch/x86/include/asm/archslowrng.h b/arch/x86/include/asm/archslowrng.h
new file mode 100644
index 0000000..c8e8d0d
--- /dev/null
+++ b/arch/x86/include/asm/archslowrng.h
@@ -0,0 +1,30 @@
+/*
+ * This file is part of the Linux kernel.
+ *
+ * Copyright (c) 2014 Andy Lutomirski
+ * Authors: Andy Lutomirski <luto@amacapital.net>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef ASM_X86_ARCHSLOWRANDOM_H
+#define ASM_X86_ARCHSLOWRANDOM_H
+
+#ifndef CONFIG_ARCH_SLOW_RNG
+# error archslowrng.h should not be included if !CONFIG_ARCH_SLOW_RNG
+#endif
+
+/*
+ * Performance is irrelevant here, so there's no point in using the
+ * paravirt ops mechanism.  Instead just use a function pointer.
+ */
+extern int (*arch_get_slow_rng_u64)(u64 *v);
+
+#endif /* ASM_X86_ARCHSLOWRANDOM_H */
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 3dd8e2c..8d64d28 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -416,6 +416,25 @@ void kvm_disable_steal_time(void)
 	wrmsr(MSR_KVM_STEAL_TIME, 0, 0);
 }
 
+static int nop_get_slow_rng_u64(u64 *v)
+{
+	return 0;
+}
+
+static int kvm_get_slow_rng_u64(u64 *v)
+{
+	/*
+	 * Allow migration from a hypervisor with the GET_RNG_SEED
+	 * feature to a hypervisor without it.
+	 */
+	if (rdmsrl_safe(MSR_KVM_GET_RNG_SEED, v) == 0)
+		return 1;
+	else
+		return 0;
+}
+
+int (*arch_get_slow_rng_u64)(u64 *v) = nop_get_slow_rng_u64;
+
 #ifdef CONFIG_SMP
 static void __init kvm_smp_prepare_boot_cpu(void)
 {
@@ -493,6 +512,9 @@ void __init kvm_guest_init(void)
 	if (kvmclock_vsyscall)
 		kvm_setup_vsyscall_timeinfo();
 
+	if (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED))
+		arch_get_slow_rng_u64 = kvm_get_slow_rng_u64;
+
 #ifdef CONFIG_SMP
 	smp_ops.smp_prepare_boot_cpu = kvm_smp_prepare_boot_cpu;
 	register_cpu_notifier(&kvm_cpu_notifier);
diff --git a/include/linux/random.h b/include/linux/random.h
index 57fbbff..ceafbcf 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -106,6 +106,15 @@ static inline int arch_has_random_seed(void)
 }
 #endif
 
+#ifdef CONFIG_ARCH_SLOW_RNG
+# include <asm/archslowrng.h>
+#else
+static inline int arch_get_slow_rng_u64(u64 *v)
+{
+	return 0;
+}
+#endif
+
 /* Pseudo random number generator from numerical recipes. */
 static inline u32 next_pseudo_random32(u32 seed)
 {
-- 
1.9.3

[PATCH 3/4] random: Seed pools from arch_get_slow_rng_u64 at startup

[Andy Lutomirski]

This should help solve the problem of guests starting out with
predictable RNG state.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 drivers/char/random.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0a7ac0a..bd88a24 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1248,7 +1248,7 @@ EXPORT_SYMBOL(get_random_bytes_arch);
  */
 static void init_std_data(struct entropy_store *r)
 {
-	int i;
+	int i, slow_rng_bits = 0;
 	ktime_t now = ktime_get_real();
 	unsigned long rv;
 
@@ -1261,6 +1261,18 @@ static void init_std_data(struct entropy_store *r)
 		mix_pool_bytes(r, &rv, sizeof(rv), NULL);
 	}
 	mix_pool_bytes(r, utsname(), sizeof(*(utsname())), NULL);
+
+	for (i = 0; i < 4; i++) {
+		u64 rv64;
+
+		if (arch_get_slow_rng_u64(&rv64)) {
+			mix_pool_bytes(r, &rv64, sizeof(rv64), NULL);
+			slow_rng_bits += 8 * sizeof(rv64);
+		}
+	}
+	if (slow_rng_bits)
+		pr_info("random: seeded %s pool with %d bits of arch slow rng data\n",
+		       r->name, slow_rng_bits);
 }
 
 /*
-- 
1.9.3

[PATCH 4/4] x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available

[Andy Lutomirski]

It's considerably better than any of the alternatives on KVM.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/boot/compressed/aslr.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/arch/x86/boot/compressed/aslr.c b/arch/x86/boot/compressed/aslr.c
index fc6091a..8583f0e 100644
--- a/arch/x86/boot/compressed/aslr.c
+++ b/arch/x86/boot/compressed/aslr.c
@@ -5,6 +5,8 @@
 #include <asm/archrandom.h>
 #include <asm/e820.h>
 
+#include <uapi/asm/kvm_para.h>
+
 #include <generated/compile.h>
 #include <linux/module.h>
 #include <linux/uts.h>
@@ -15,6 +17,22 @@
 static const char build_str[] = UTS_RELEASE " (" LINUX_COMPILE_BY "@"
 		LINUX_COMPILE_HOST ") (" LINUX_COMPILER ") " UTS_VERSION;
 
+static bool kvm_para_has_feature(unsigned int feature)
+{
+	u32 kvm_base;
+	u32 features;
+
+	if (!has_cpuflag(X86_FEATURE_HYPERVISOR))
+		return false;
+
+	kvm_base = hypervisor_cpuid_base("KVMKVMKVM\0\0\0", KVM_CPUID_FEATURES);
+	if (!kvm_base)
+		return false;
+
+	features = cpuid_eax(kvm_base | KVM_CPUID_FEATURES);
+	return features & (1UL << feature);
+}
+
 #define I8254_PORT_CONTROL	0x43
 #define I8254_PORT_COUNTER0	0x40
 #define I8254_CMD_READBACK	0xC0
@@ -81,6 +99,15 @@ static unsigned long get_random_long(void)
 		}
 	}
 
+	if (kvm_para_has_feature(KVM_FEATURE_GET_RNG_SEED)) {
+		u64 seed;
+
+		debug_putstr(" MSR_KVM_GET_RNG_SEED");
+		rdmsrl(MSR_KVM_GET_RNG_SEED, seed);
+		random ^= (unsigned long)seed;
+		use_i8254 = false;
+	}
+
 	if (has_cpuflag(X86_FEATURE_TSC)) {
 		debug_putstr(" RDTSC");
 		rdtscll(raw);
-- 
1.9.3

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Gleb Natapov]

On Tue, Jul 15, 2014 at 07:48:06PM -0700, Andy Lutomirski wrote:
> virtio-rng is both too complicated and insufficient for initial rng
> seeding.  It's far too complicated to use for KASLR or any other
> early boot random number needs.  It also provides /dev/random-style
> bits, which means that making guest boot wait for virtio-rng is
> unacceptably slow, and doing it asynchronously means that
> /dev/urandom might be predictable when userspace starts.
> 
> This introduces a very simple synchronous mechanism to get
> /dev/urandom-style bits.
Why can't you use RDRAND instruction for that?

> 
> This is a KVM change: am I supposed to write a unit test somewhere?
> 
> Andy Lutomirski (4):
>   x86,kvm: Add MSR_KVM_GET_RNG_SEED and a matching feature bit
>   random,x86: Add arch_get_slow_rng_u64
>   random: Seed pools from arch_get_slow_rng_u64 at startup
>   x86,kaslr: Use MSR_KVM_GET_RNG_SEED for KASLR if available
> 
>  Documentation/virtual/kvm/cpuid.txt  |  3 +++
>  arch/x86/Kconfig                     |  4 ++++
>  arch/x86/boot/compressed/aslr.c      | 27 +++++++++++++++++++++++++++
>  arch/x86/include/asm/archslowrng.h   | 30 ++++++++++++++++++++++++++++++
>  arch/x86/include/uapi/asm/kvm_para.h |  2 ++
>  arch/x86/kernel/kvm.c                | 22 ++++++++++++++++++++++
>  arch/x86/kvm/cpuid.c                 |  3 ++-
>  arch/x86/kvm/x86.c                   |  4 ++++
>  drivers/char/random.c                | 14 +++++++++++++-
>  include/linux/random.h               |  9 +++++++++
>  10 files changed, 116 insertions(+), 2 deletions(-)
>  create mode 100644 arch/x86/include/asm/archslowrng.h
> 
> -- 
> 1.9.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
			Gleb.

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Daniel Borkmann]

On 07/16/2014 08:41 AM, Gleb Natapov wrote:
> On Tue, Jul 15, 2014 at 07:48:06PM -0700, Andy Lutomirski wrote:
>> virtio-rng is both too complicated and insufficient for initial rng
>> seeding.  It's far too complicated to use for KASLR or any other
>> early boot random number needs.  It also provides /dev/random-style
>> bits, which means that making guest boot wait for virtio-rng is
>> unacceptably slow, and doing it asynchronously means that
>> /dev/urandom might be predictable when userspace starts.
>>
>> This introduces a very simple synchronous mechanism to get
>> /dev/urandom-style bits.
 >
> Why can't you use RDRAND instruction for that?

You mean using it directly? I think simply for the very same reasons
as in c2557a303a ...

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Gleb Natapov]

On Wed, Jul 16, 2014 at 09:10:27AM +0200, Daniel Borkmann wrote:
> On 07/16/2014 08:41 AM, Gleb Natapov wrote:
> >On Tue, Jul 15, 2014 at 07:48:06PM -0700, Andy Lutomirski wrote:
> >>virtio-rng is both too complicated and insufficient for initial rng
> >>seeding.  It's far too complicated to use for KASLR or any other
> >>early boot random number needs.  It also provides /dev/random-style
> >>bits, which means that making guest boot wait for virtio-rng is
> >>unacceptably slow, and doing it asynchronously means that
> >>/dev/urandom might be predictable when userspace starts.
> >>
> >>This introduces a very simple synchronous mechanism to get
> >>/dev/urandom-style bits.
> >
> >Why can't you use RDRAND instruction for that?
> 
> You mean using it directly? I think simply for the very same reasons
> as in c2557a303a ...
So you trust your hypervisor vendor more than you trust your CPU vendor? :)

--
			Gleb.

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Paolo Bonzini]

Il 16/07/2014 09:10, Daniel Borkmann ha scritto:
> On 07/16/2014 08:41 AM, Gleb Natapov wrote:
>> On Tue, Jul 15, 2014 at 07:48:06PM -0700, Andy Lutomirski wrote:
>>> virtio-rng is both too complicated and insufficient for initial rng
>>> seeding.  It's far too complicated to use for KASLR or any other
>>> early boot random number needs.  It also provides /dev/random-style
>>> bits, which means that making guest boot wait for virtio-rng is
>>> unacceptably slow, and doing it asynchronously means that
>>> /dev/urandom might be predictable when userspace starts.
>>>
>>> This introduces a very simple synchronous mechanism to get
>>> /dev/urandom-style bits.
>>
>> Why can't you use RDRAND instruction for that?
>
> You mean using it directly? I think simply for the very same reasons
> as in c2557a303a ...

No, this is very different.  This mechanism "provides no guarantee that 
the result contains any actual entropy".  In fact, patch 3 adds a call 
to the new arch_get_slow_rng_u64 just below a call to 
arch_get_random_lang aka RDRAND.  I agree with Gleb that it's simpler to 
just expect a relatively recent processor and use RDRAND.

BTW, the logic for crediting entropy to RDSEED but not RDRAND escapes 
me.  If you trust the processor, you could use Intel's algorithm to 
force reseeding of RDRAND.  If you don't trust the processor, the same 
paranoia applies to RDRAND and RDSEED.

In a guest you must trust the hypervisor anyway to use RDRAND or RDSEED, 
since the hypervisor can trap it.  A malicious hypervisor is no 
different from a malicious processor.

In any case, is there a matching QEMU patch somewhere?

Paolo

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Andy Lutomirski]

On Wed, Jul 16, 2014 at 12:36 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 16/07/2014 09:10, Daniel Borkmann ha scritto:
>
>> On 07/16/2014 08:41 AM, Gleb Natapov wrote:
>>>
>>> On Tue, Jul 15, 2014 at 07:48:06PM -0700, Andy Lutomirski wrote:
>>>>
>>>> virtio-rng is both too complicated and insufficient for initial rng
>>>> seeding.  It's far too complicated to use for KASLR or any other
>>>> early boot random number needs.  It also provides /dev/random-style
>>>> bits, which means that making guest boot wait for virtio-rng is
>>>> unacceptably slow, and doing it asynchronously means that
>>>> /dev/urandom might be predictable when userspace starts.
>>>>
>>>> This introduces a very simple synchronous mechanism to get
>>>> /dev/urandom-style bits.
>>>
>>>
>>> Why can't you use RDRAND instruction for that?
>>
>>
>> You mean using it directly? I think simply for the very same reasons
>> as in c2557a303a ...
>
>
> No, this is very different.  This mechanism "provides no guarantee that the
> result contains any actual entropy".  In fact, patch 3 adds a call to the
> new arch_get_slow_rng_u64 just below a call to arch_get_random_lang aka
> RDRAND.  I agree with Gleb that it's simpler to just expect a relatively
> recent processor and use RDRAND.
>
> BTW, the logic for crediting entropy to RDSEED but not RDRAND escapes me.
> If you trust the processor, you could use Intel's algorithm to force
> reseeding of RDRAND.  If you don't trust the processor, the same paranoia
> applies to RDRAND and RDSEED.
>
> In a guest you must trust the hypervisor anyway to use RDRAND or RDSEED,
> since the hypervisor can trap it.  A malicious hypervisor is no different
> from a malicious processor.
>

This patch has nothing whatsoever to do with how much I trust the CPU
vs the hypervisor.  It's for the enormous installed base of machines
without RDRAND.

hpa suggested emulating RDRAND awhile ago, but I think that'll
unusably slow -- the kernel uses RDRAND in various places where it's
expected to be fast, and not using it at all will be preferable to
causing a VM exit for every few bytes.  I've been careful to only use
this in the guest in places where a few hundred to a few thousand
cycles per 64 bits of RNG seed is acceptable.

> In any case, is there a matching QEMU patch somewhere?

What QEMU change is needed?  I admit I'm a bit vague on how QEMU and
KVM cooperate here, but there's no state to save and restore.  I guess
that QEMU wants the ability to turn this on and off for migration.
How does that work?  I couldn't spot the KVM code that allows this
type of control.

--Andy

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Paolo Bonzini]

Il 16/07/2014 16:07, Andy Lutomirski ha scritto:
> This patch has nothing whatsoever to do with how much I trust the CPU
> vs the hypervisor.  It's for the enormous installed base of machines
> without RDRAND.

Ok.  I think an MSR is fine, though I don't think it's useful for the 
guest to use it if it already has RDRAND and/or RDSEED.

> > In any case, is there a matching QEMU patch somewhere?
>
> What QEMU change is needed?  I admit I'm a bit vague on how QEMU and
> KVM cooperate here, but there's no state to save and restore.  I guess
> that QEMU wants the ability to turn this on and off for migration.
> How does that work?  I couldn't spot the KVM code that allows this
> type of control.

It is QEMU who decides the CPUID bits that are visible to the guest.  By 
default it blocks bits that it doesn't know about.  You would need to 
add the bit in the kvm_default_features and kvm_feature_name arrays.

For migration, we have "versioned" machine types, for example pc-2.1.
Once the versioned machine type exists, blocking the feature is a 
one-liner like

     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_NAME);

Unfortunately, QEMU is in hard freeze, so you'd likely be the one 
creating pc-2.2.  This is a boilerplate but relatively complicated 
patch.  But let's cross that bridge when we'll reach it.  For now, you 
can simply add the bit to the two arrays above.

Paolo

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Gleb Natapov]

On Wed, Jul 16, 2014 at 04:32:19PM +0200, Paolo Bonzini wrote:
> Il 16/07/2014 16:07, Andy Lutomirski ha scritto:
> >This patch has nothing whatsoever to do with how much I trust the CPU
> >vs the hypervisor.  It's for the enormous installed base of machines
> >without RDRAND.
> 
> Ok.  I think an MSR is fine, though I don't think it's useful for the guest
> to use it if it already has RDRAND and/or RDSEED.
> 
Agree. It is unfortunate that we add PV interfaces for a HW that will be extinct
in a couple of years though :(

--
			Gleb.

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Andy Lutomirski]

On Wed, Jul 16, 2014 at 7:32 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> Il 16/07/2014 16:07, Andy Lutomirski ha scritto:
>
>> This patch has nothing whatsoever to do with how much I trust the CPU
>> vs the hypervisor.  It's for the enormous installed base of machines
>> without RDRAND.
>
>
> Ok.  I think an MSR is fine, though I don't think it's useful for the guest
> to use it if it already has RDRAND and/or RDSEED.
>
>
>> > In any case, is there a matching QEMU patch somewhere?
>>
>> What QEMU change is needed?  I admit I'm a bit vague on how QEMU and
>> KVM cooperate here, but there's no state to save and restore.  I guess
>> that QEMU wants the ability to turn this on and off for migration.
>> How does that work?  I couldn't spot the KVM code that allows this
>> type of control.
>
>
> It is QEMU who decides the CPUID bits that are visible to the guest.  By
> default it blocks bits that it doesn't know about.  You would need to add
> the bit in the kvm_default_features and kvm_feature_name arrays.
>
> For migration, we have "versioned" machine types, for example pc-2.1.
> Once the versioned machine type exists, blocking the feature is a one-liner
> like
>
>     x86_cpu_compat_disable_kvm_features(FEAT_KVM, KVM_FEATURE_NAME);
>
> Unfortunately, QEMU is in hard freeze, so you'd likely be the one creating
> pc-2.2.  This is a boilerplate but relatively complicated patch.  But let's
> cross that bridge when we'll reach it.  For now, you can simply add the bit
> to the two arrays above.
>

Done.

NB: Patch 4 of this series is bad due to an asm constraint issue that
I haven't figured out yet.  I'll send a replacement once I get it
working.  *sigh* the x86 kernel loading code is a bit of a compilation
mess.

> Paolo



-- 
Andy Lutomirski
AMA Capital Management, LLC

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[H. Peter Anvin]

On 07/16/2014 07:07 AM, Andy Lutomirski wrote:
> 
> This patch has nothing whatsoever to do with how much I trust the CPU
> vs the hypervisor.  It's for the enormous installed base of machines
> without RDRAND.
> 
> hpa suggested emulating RDRAND awhile ago, but I think that'll
> unusably slow -- the kernel uses RDRAND in various places where it's
> expected to be fast, and not using it at all will be preferable to
> causing a VM exit for every few bytes.  I've been careful to only use
> this in the guest in places where a few hundred to a few thousand
> cycles per 64 bits of RNG seed is acceptable.
> 

I suggested emulating RDRAND *but not set the CPUID bit*.  We already
developed a protocol in KVM/Qemu to enumerate emulated features (created
for MOVBE as I recall), specifically to service the semantic "feature X
will work but will be substantially slower than normal."

	-hpa

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Paolo Bonzini]

Il 16/07/2014 18:03, H. Peter Anvin ha scritto:
> I suggested emulating RDRAND *but not set the CPUID bit*.  We already
> developed a protocol in KVM/Qemu to enumerate emulated features (created
> for MOVBE as I recall), specifically to service the semantic "feature X
> will work but will be substantially slower than normal."

But those will set the CPUID bit.  There is currently no way for KVM 
guests to know if a CPUID bit is real or emulated.

Paolo

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[H. Peter Anvin]

On 07/16/2014 09:08 AM, Paolo Bonzini wrote:
> Il 16/07/2014 18:03, H. Peter Anvin ha scritto:
>> I suggested emulating RDRAND *but not set the CPUID bit*.  We already
>> developed a protocol in KVM/Qemu to enumerate emulated features (created
>> for MOVBE as I recall), specifically to service the semantic "feature X
>> will work but will be substantially slower than normal."
> 
> But those will set the CPUID bit.  There is currently no way for KVM
> guests to know if a CPUID bit is real or emulated.
> 

OK, so there wasn't any protocol implemented in the end.  I sit corrected.

	-hpa

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Gleb Natapov]

On Wed, Jul 16, 2014 at 09:13:23AM -0700, H. Peter Anvin wrote:
> On 07/16/2014 09:08 AM, Paolo Bonzini wrote:
> > Il 16/07/2014 18:03, H. Peter Anvin ha scritto:
> >> I suggested emulating RDRAND *but not set the CPUID bit*.  We already
> >> developed a protocol in KVM/Qemu to enumerate emulated features (created
> >> for MOVBE as I recall), specifically to service the semantic "feature X
> >> will work but will be substantially slower than normal."
> > 
> > But those will set the CPUID bit.  There is currently no way for KVM
> > guests to know if a CPUID bit is real or emulated.
> > 
> 
> OK, so there wasn't any protocol implemented in the end.  I sit corrected.
> 
That protocol that was implemented is between qemu and kvm, not kvm and a guest.

--
			Gleb.

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[H. Peter Anvin]

On 07/16/2014 09:21 AM, Gleb Natapov wrote:
> On Wed, Jul 16, 2014 at 09:13:23AM -0700, H. Peter Anvin wrote:
>> On 07/16/2014 09:08 AM, Paolo Bonzini wrote:
>>> Il 16/07/2014 18:03, H. Peter Anvin ha scritto:
>>>> I suggested emulating RDRAND *but not set the CPUID bit*.  We already
>>>> developed a protocol in KVM/Qemu to enumerate emulated features (created
>>>> for MOVBE as I recall), specifically to service the semantic "feature X
>>>> will work but will be substantially slower than normal."
>>>
>>> But those will set the CPUID bit.  There is currently no way for KVM
>>> guests to know if a CPUID bit is real or emulated.
>>>
>>
>> OK, so there wasn't any protocol implemented in the end.  I sit corrected.
>>
> That protocol that was implemented is between qemu and kvm, not kvm and a guest.
> 

Either which way, the notion was to have a PV CPUID bit like the
proposed kvm_get_rng_seed bit, but to have it exercised by executing RDRAND.

The biggest reason to *not* do this would be that with an MSR it is not
available to guest user space, which may be better under the circumstances.

	-hpa

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[Andy Lutomirski]

On Wed, Jul 16, 2014 at 1:20 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 07/16/2014 09:21 AM, Gleb Natapov wrote:
>> On Wed, Jul 16, 2014 at 09:13:23AM -0700, H. Peter Anvin wrote:
>>> On 07/16/2014 09:08 AM, Paolo Bonzini wrote:
>>>> Il 16/07/2014 18:03, H. Peter Anvin ha scritto:
>>>>> I suggested emulating RDRAND *but not set the CPUID bit*.  We already
>>>>> developed a protocol in KVM/Qemu to enumerate emulated features (created
>>>>> for MOVBE as I recall), specifically to service the semantic "feature X
>>>>> will work but will be substantially slower than normal."
>>>>
>>>> But those will set the CPUID bit.  There is currently no way for KVM
>>>> guests to know if a CPUID bit is real or emulated.
>>>>
>>>
>>> OK, so there wasn't any protocol implemented in the end.  I sit corrected.
>>>
>> That protocol that was implemented is between qemu and kvm, not kvm and a guest.
>>
>
> Either which way, the notion was to have a PV CPUID bit like the
> proposed kvm_get_rng_seed bit, but to have it exercised by executing RDRAND.
>
> The biggest reason to *not* do this would be that with an MSR it is not
> available to guest user space, which may be better under the circumstances.

On the theory that I see no legitimate reason to expose this to guest
user space, I think we shouldn't expose it.  If we wanted to add a
get_random_bytes syscall, that would be an entirely different story,
though.

Should I send v3 as one series or should I split it into host and guest parts?

--Andy

Re: [PATCH 0/4] random,x86,kvm: Add and use MSR_KVM_GET_RNG_SEED

[H. Peter Anvin]

On 07/16/2014 02:32 PM, Andy Lutomirski wrote:
> 
> On the theory that I see no legitimate reason to expose this to guest
> user space, I think we shouldn't expose it.  If we wanted to add a
> get_random_bytes syscall, that would be an entirely different story,
> though.
> 
> Should I send v3 as one series or should I split it into host and guest parts?
> 

It doesn't matter... as long as they are separate *patches*.

	-hpa