diff mbox

[v2,16/17] kvm: x86: Rework identity map and TSS setup for larger BIOS sizes

Message ID e7542e718fee0a901d1e626d07ede2227290c552.1294043582.git.jan.kiszka@web.de
State New
Headers show

Commit Message

Jan Kiszka Jan. 3, 2011, 8:33 a.m. UTC
From: Jan Kiszka <jan.kiszka@siemens.com>

First of all, we only need this EPT identity and TSS reservation on
Intel CPUs. Then, in order to support loading BIOSes > 256K, reorder the
code, adjusting the base if the kernel supports moving the identity map.
We can drop the check for KVM_CAP_SET_TSS_ADDR as we already depend on
much newer features.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |   91 ++++++++++++++++++++++++----------------------------
 1 files changed, 42 insertions(+), 49 deletions(-)

Comments

Avi Kivity Jan. 3, 2011, 4:06 p.m. UTC | #1
On 01/03/2011 10:33 AM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> First of all, we only need this EPT identity and TSS reservation on
> Intel CPUs.

kvm-amd will ignore it just fine.  I'd like to keep arch differences 
away from userspace.

> Then, in order to support loading BIOSes>  256K, reorder the
> code, adjusting the base if the kernel supports moving the identity map.
> We can drop the check for KVM_CAP_SET_TSS_ADDR as we already depend on
> much newer features.

There is no ordering on kvm features.  Each can come and go as it pleases.
Jan Kiszka Jan. 3, 2011, 4:52 p.m. UTC | #2
Am 03.01.2011 17:06, Avi Kivity wrote:
> On 01/03/2011 10:33 AM, Jan Kiszka wrote:
>> From: Jan Kiszka<jan.kiszka@siemens.com>
>>
>> First of all, we only need this EPT identity and TSS reservation on
>> Intel CPUs.
> 
> kvm-amd will ignore it just fine.  I'd like to keep arch differences
> away from userspace.

And I would prefer to avoid needlessly cluttering the physical guest
address space where not needed. Long term, we could even give user space
a hint (unless it can test it directly) that this workaround is no
longer needed as the host Intel CPU supports true real mode.

> 
>> Then, in order to support loading BIOSes>  256K, reorder the
>> code, adjusting the base if the kernel supports moving the identity map.
>> We can drop the check for KVM_CAP_SET_TSS_ADDR as we already depend on
>> much newer features.
> 
> There is no ordering on kvm features.  Each can come and go as it pleases.
> 

Well, at least this is not how kvm upstream works so far.

Jan
Avi Kivity Jan. 3, 2011, 4:59 p.m. UTC | #3
On 01/03/2011 06:52 PM, Jan Kiszka wrote:
> Am 03.01.2011 17:06, Avi Kivity wrote:
> >  On 01/03/2011 10:33 AM, Jan Kiszka wrote:
> >>  From: Jan Kiszka<jan.kiszka@siemens.com>
> >>
> >>  First of all, we only need this EPT identity and TSS reservation on
> >>  Intel CPUs.
> >
> >  kvm-amd will ignore it just fine.  I'd like to keep arch differences
> >  away from userspace.
>
> And I would prefer to avoid needlessly cluttering the physical guest
> address space where not needed. Long term, we could even give user space
> a hint (unless it can test it directly) that this workaround is no
> longer needed as the host Intel CPU supports true real mode.

Having different physical address spaces based on the host cpu is bad, 
even disregarding live migration.  If there's a real need, we can do it 
as an option.  I don't see such a need though.

We can definitely add a new KVM_CAP for "tss/ept identity supported but 
not needed".  If emulate_invalid_guest_state is eventually fully 
implemented and becomes the default, it will even be true across the board.

> >
> >>  Then, in order to support loading BIOSes>   256K, reorder the
> >>  code, adjusting the base if the kernel supports moving the identity map.
> >>  We can drop the check for KVM_CAP_SET_TSS_ADDR as we already depend on
> >>  much newer features.
> >
> >  There is no ordering on kvm features.  Each can come and go as it pleases.
> >
>
> Well, at least this is not how kvm upstream works so far.

Let's change it then.
Jan Kiszka Jan. 3, 2011, 5:19 p.m. UTC | #4
Am 03.01.2011 17:59, Avi Kivity wrote:
> On 01/03/2011 06:52 PM, Jan Kiszka wrote:
>> Am 03.01.2011 17:06, Avi Kivity wrote:
>> >  On 01/03/2011 10:33 AM, Jan Kiszka wrote:
>> >>  From: Jan Kiszka<jan.kiszka@siemens.com>
>> >>
>> >>  First of all, we only need this EPT identity and TSS reservation on
>> >>  Intel CPUs.
>> >
>> >  kvm-amd will ignore it just fine.  I'd like to keep arch differences
>> >  away from userspace.
>>
>> And I would prefer to avoid needlessly cluttering the physical guest
>> address space where not needed. Long term, we could even give user space
>> a hint (unless it can test it directly) that this workaround is no
>> longer needed as the host Intel CPU supports true real mode.
> 
> Having different physical address spaces based on the host cpu is bad,
> even disregarding live migration.  If there's a real need, we can do it
> as an option.  I don't see such a need though.

I see, migration is a killer argument.

Jan
diff mbox

Patch

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6d31dde..caca407 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -575,23 +575,12 @@  static int kvm_get_supported_msrs(void)
     return ret;
 }
 
-static int kvm_init_identity_map_page(void)
+static bool host_supports_vmx(void)
 {
-#ifdef KVM_CAP_SET_IDENTITY_MAP_ADDR
-    int ret;
-    uint64_t addr = 0xfffbc000;
-
-    if (!kvm_check_extension(KVM_CAP_SET_IDENTITY_MAP_ADDR)) {
-        return 0;
-    }
+    uint32_t ecx, unused;
 
-    ret = kvm_vm_ioctl(KVM_SET_IDENTITY_MAP_ADDR, &addr);
-    if (ret < 0) {
-        fprintf(stderr, "kvm_set_identity_map_addr: %s\n", strerror(ret));
-        return ret;
-    }
-#endif
-    return 0;
+    host_cpuid(1, 0, &unused, &unused, &ecx, &unused);
+    return ecx & CPUID_EXT_VMX;
 }
 
 int kvm_arch_init(void)
@@ -611,35 +600,47 @@  int kvm_arch_init(void)
     uname(&utsname);
     lm_capable_kernel = strcmp(utsname.machine, "x86_64") == 0;
 
-    /* create vm86 tss.  KVM uses vm86 mode to emulate 16-bit code
-     * directly.  In order to use vm86 mode, a TSS is needed.  Since this
-     * must be part of guest physical memory, we need to allocate it.  Older
-     * versions of KVM just assumed that it would be at the end of physical
-     * memory but that doesn't work with more than 4GB of memory.  We simply
-     * refuse to work with those older versions of KVM. */
-    ret = kvm_check_extension(KVM_CAP_SET_TSS_ADDR);
-    if (ret <= 0) {
-        fprintf(stderr, "kvm does not support KVM_CAP_SET_TSS_ADDR\n");
-        return ret;
-    }
+    if (host_supports_vmx()) {
+        /*
+         * On older Intel CPUs, KVM uses vm86 mode to emulate 16-bit code
+         * directly. In order to use vm86 mode, an EPT identity map and a TSS
+         * are needed. Since these must be part of guest physical memory, we
+         * need to allocate them, both by setting their start addresses in the
+         * kernel and by creating a corresponding e820 entry. We need 4 pages
+         * before the BIOS.
+         *
+         * Older KVM versions may not support setting the identity map base.
+         * In that case we need to stick with the default, i.e. a 256K maximum
+         * BIOS size.
+         */
+        uint64_t identity_base = 0xfffbc000;
 
-    /* this address is 3 pages before the bios, and the bios should present
-     * as unavaible memory.  FIXME, need to ensure the e820 map deals with
-     * this?
-     */
-    /*
-     * Tell fw_cfg to notify the BIOS to reserve the range.
-     */
-    if (e820_add_entry(0xfffbc000, 0x4000, E820_RESERVED) < 0) {
-        perror("e820_add_entry() table is full");
-        exit(1);
-    }
-    ret = kvm_vm_ioctl(KVM_SET_TSS_ADDR, 0xfffbd000);
-    if (ret < 0) {
-        return ret;
+#ifdef KVM_CAP_SET_IDENTITY_MAP_ADDR
+        if (kvm_check_extension(KVM_CAP_SET_IDENTITY_MAP_ADDR)) {
+            /* Allows up to 16M BIOSes. */
+            identity_base = 0xfeffc000;
+
+            ret = kvm_vm_ioctl(KVM_SET_IDENTITY_MAP_ADDR, &identity_base);
+            if (ret < 0) {
+                return ret;
+            }
+        }
+#endif
+        /* Set TSS base one page after EPT identity map. */
+        ret = kvm_vm_ioctl(KVM_SET_TSS_ADDR, identity_base + 0x1000);
+        if (ret < 0) {
+            return ret;
+        }
+
+        /* Tell fw_cfg to notify the BIOS to reserve the range. */
+        ret = e820_add_entry(identity_base, 0x4000, E820_RESERVED);
+        if (ret < 0) {
+            fprintf(stderr, "e820_add_entry() table is full\n");
+            return ret;
+        }
     }
 
-    return kvm_init_identity_map_page();
+    return 0;
 }
 
 static void set_v8086_seg(struct kvm_segment *lhs, const SegmentCache *rhs)
@@ -1592,14 +1593,6 @@  static int kvm_handle_halt(CPUState *env)
     return 1;
 }
 
-static bool host_supports_vmx(void)
-{
-    uint32_t ecx, unused;
-
-    host_cpuid(1, 0, &unused, &unused, &ecx, &unused);
-    return ecx & CPUID_EXT_VMX;
-}
-
 #define VMX_INVALID_GUEST_STATE 0x80000021
 
 int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run)