2006-03-22 06:48:18

by Chris Wright

[permalink] [raw]
Subject: [RFC PATCH 14/35] subarch modify CPU capabilities

Allow subarchitectures to modify CPU capabilities during bootstrap CPU
identification. Add a subarch implementation for Xen which hides
features unsupported by the hypervisor.

Signed-off-by: Ian Pratt <[email protected]>
Signed-off-by: Christian Limpach <[email protected]>
Signed-off-by: Chris Wright <[email protected]>
---
arch/i386/kernel/cpu/common.c | 3 +++
include/asm-i386/mach-default/mach_cpu.h | 5 +++++
include/asm-i386/mach-xen/mach_cpu.h | 2 ++
include/asm-i386/mach-xen/setup_arch_post.h | 15 +++++++++++++++
include/asm-i386/mach-xen/setup_arch_pre.h | 1 +
5 files changed, 26 insertions(+)

--- xen-subarch-2.6.orig/arch/i386/kernel/cpu/common.c
+++ xen-subarch-2.6/arch/i386/kernel/cpu/common.c
@@ -16,6 +16,7 @@
#include <asm/apic.h>
#include <mach_apic.h>
#endif
+#include <mach_cpu.h>

#include "cpu.h"

@@ -420,6 +421,8 @@ void __devinit identify_cpu(struct cpuin
c->x86_vendor, c->x86_model);
}

+ machine_specific_modify_cpu_capabilities(c);
+
/* Now the feature flags better reflect actual CPU features! */

printk(KERN_DEBUG "CPU: After all inits, caps:");
--- xen-subarch-2.6.orig/include/asm-i386/mach-xen/setup_arch_post.h
+++ xen-subarch-2.6/include/asm-i386/mach-xen/setup_arch_post.h
@@ -18,6 +18,19 @@ static char * __init machine_specific_me
return "Xen";
}

+void __devinit machine_specific_modify_cpu_capabilities(struct cpuinfo_x86 *c)
+{
+ clear_bit(X86_FEATURE_VME, c->x86_capability);
+ clear_bit(X86_FEATURE_DE, c->x86_capability);
+ clear_bit(X86_FEATURE_PSE, c->x86_capability);
+ clear_bit(X86_FEATURE_PGE, c->x86_capability);
+ clear_bit(X86_FEATURE_SEP, c->x86_capability);
+ clear_bit(X86_FEATURE_MWAIT, c->x86_capability);
+ if (!(xen_start_info->flags & SIF_PRIVILEGED))
+ clear_bit(X86_FEATURE_MTRR, c->x86_capability);
+ c->hlt_works_ok = 0;
+}
+
static void __init machine_specific_arch_setup(void)
{
struct physdev_op op;
@@ -30,6 +43,8 @@ static void __init machine_specific_arch
__KERNEL_CS, (unsigned long)hypervisor_callback,
__KERNEL_CS, (unsigned long)failsafe_callback);

+ machine_specific_modify_cpu_capabilities(&boot_cpu_data);
+
init_pg_tables_end = __pa(xen_start_info->pt_base) +
PFN_PHYS(xen_start_info->nr_pt_frames);

--- xen-subarch-2.6.orig/include/asm-i386/mach-xen/setup_arch_pre.h
+++ xen-subarch-2.6/include/asm-i386/mach-xen/setup_arch_pre.h
@@ -1,6 +1,7 @@

#include <xen/interface/xen.h>
#include <asm/hypervisor.h>
+#include <mach_cpu.h>

struct start_info *xen_start_info;
EXPORT_SYMBOL(xen_start_info);
--- /dev/null
+++ xen-subarch-2.6/include/asm-i386/mach-default/mach_cpu.h
@@ -0,0 +1,5 @@
+
+static inline void
+machine_specific_modify_cpu_capabilities(struct cpuinfo_x86 *c)
+{
+}
--- /dev/null
+++ xen-subarch-2.6/include/asm-i386/mach-xen/mach_cpu.h
@@ -0,0 +1,2 @@
+
+extern void machine_specific_modify_cpu_capabilities(struct cpuinfo_x86 *);

--


2006-03-22 08:37:13

by Zachary Amsden

[permalink] [raw]
Subject: Re: [Xen-devel] [RFC PATCH 14/35] subarch modify CPU capabilities

Chris Wright wrote:
>
> +void __devinit machine_specific_modify_cpu_capabilities(struct cpuinfo_x86 *c)
> +{
> + clear_bit(X86_FEATURE_VME, c->x86_capability);
> + clear_bit(X86_FEATURE_DE, c->x86_capability);
> + clear_bit(X86_FEATURE_PSE, c->x86_capability);
> + clear_bit(X86_FEATURE_PGE, c->x86_capability);
> + clear_bit(X86_FEATURE_SEP, c->x86_capability);
> + clear_bit(X86_FEATURE_MWAIT, c->x86_capability);
> + if (!(xen_start_info->flags & SIF_PRIVILEGED))
> + clear_bit(X86_FEATURE_MTRR, c->x86_capability);
> + c->hlt_works_ok = 0;
>

That's pretty heinous. Suppose Xen decides to start supporting any of
these features. Now, you need to change all of the Xen modified kernels
to remove pieces of this.

You've effectively introduced completely arbitrary, but most
importantly, static constraints into Linux based on what features Xen
currently supports. This is not productive on either end.

This is the entire point of the VMI interface. You can arbitrarily
enable and disable these features in a hidden layer, call it VMI, call
it Xen-bridge, call it hypercall page, whatever. Using this layer, you
can automatically abstract away exactly this type of nasty, deliberately
deprecating extraneous code. We use this feature exactly to strip away
these bits, completely hidden from the guest, by "trapping" the CPUID
instruction in the VMI layer. The VMI interface as it stands today is
unimportant - it has and will continue to change to accommodate Xen.
But the principles of it are important for maintainability, flexibility,
and future compatibility. One of these principles is to avoid
unbalanced changes like this to the guest kernel.

And we certainly support VME, DE, PSE, PGE, and SEP features, and really
would not like them disabled unconditionally in a virtual environment.

Zach

2006-03-22 09:32:56

by Keir Fraser

[permalink] [raw]
Subject: Re: [Xen-devel] [RFC PATCH 14/35] subarch modify CPU capabilities


On 22 Mar 2006, at 08:35, Zachary Amsden wrote:

> That's pretty heinous. Suppose Xen decides to start supporting any of
> these features. Now, you need to change all of the Xen modified
> kernels to remove pieces of this.

Well, old kernels would still work and most of those features aren't
all that useful in a virtualised environment anyway. But, point taken,
adding a hypervisor trap for CPUID and making use of it would clean up
that code.

-- Keir

2006-03-22 19:29:09

by Chris Wright

[permalink] [raw]
Subject: Re: [Xen-devel] [RFC PATCH 14/35] subarch modify CPU capabilities

* Keir Fraser ([email protected]) wrote:
>
> On 22 Mar 2006, at 08:35, Zachary Amsden wrote:
>
> >That's pretty heinous. Suppose Xen decides to start supporting any of
> >these features. Now, you need to change all of the Xen modified
> >kernels to remove pieces of this.
>
> Well, old kernels would still work and most of those features aren't
> all that useful in a virtualised environment anyway. But, point taken,
> adding a hypervisor trap for CPUID and making use of it would clean up
> that code.

Agreed, added to todo.

thanks,
-chris