Xen i386 paravirtualization support
Subsystems (7)
cover Xen i386 paravirtualization support
Unlike full virtualization in which the virtual machine provides the same platform interface as running natively on the hardware, paravirtualization requires modification to the guest operating system to work with the platform interface provided by the hypervisor.
Xen was designed with performance in mind. Calls to the hypervisor are minimized, batched if necessary, and non-critical codepaths are left unmodified in the case where the privileged instruction can be trapped and emulated by the hypervisor. The Xen API is designed to be OS agnostic and has had Linux, NetBSD, FreeBSD, Solaris, Plan9 and Netware ported to it. Xen also provides support for running directly on native hardware.
The following patch series provides the minimal support required to launch Xen paravirtual guests on standard x86 hardware running the Xen hypervisor. These patches effectively port the Linux kernel to run on the platform interface provided by Xen. This port is done as an i386 subarch.
With these patches you will be able to launch an unprivileged guest running the modified Linux kernel and unmodified userspace. This guest is x86, UP only, runs in shadow translated mode, and has no direct access to hardware. This simplifies the patchset to the minimum functionality needed to support a paravirtualized guest. It's worth noting that a fair amount of this patchset deals with paravirtualizing I/O, not just CPU-only. The additional missing functionality is primarily about full SMP support, optimizations such as direct writable page tables, and the management interface.
At a high-level, the patches provide the following:
- Kconfig and Makefile changes required to support Xen
- subarch changes to allow more platform functionality to be implemented by an i386 subarch
- Xen subarch implementation
- start of day code for running in the hypervisor provided environment (paging enabled)
- basic Xen drivers to provide a fully functional guest
The Xen platform API encapsulates the following types of requirements:
- idt, gdt, ldt (descriptor table handling)
- cr2, fpu_taskswitch, debug registers (privileged register handling)
- mmu (page table, tlb, and cache handling)
- memory reservations
- time and timer
- vcpu (init, up/down vcpu)
- schedule (processor yield, shutdown, etc)
- event channel (generalized virtual interrupt handling)
- grant table (shared memory interface for high speed interdomain communication)
- block device I/O
- network device I/O
- console device I/O
- Xen feature map
- Xen version info
Thanks to all have reviewed earlier versions of these patches. -chris
--
RFC PATCH 1/33 Add apply_to_page_range() function
Add a new mm function apply_to_page_range() which applies a given function to every pte in a given virtual address range in a given mm structure. This is a generic alternative to cut-and-pasting the Linux idiomatic pagetable walking code in every place that a sequence of PTEs must be accessed.
Although this interface is intended to be useful in a wide range of situations, it is currently used specifically by several Xen subsystems, for example: to ensure that pagetables have been allocated for a virtual address range, and to construct batched special pagetable update requests to map I/O memory (in ioremap()).
--- include/linux/mm.h | 5 ++ mm/memory.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+)
include/linux/mm.h +5 -0
@@ -1013,6 +1013,11 @@ struct page *follow_page(struct vm_area_#define FOLL_GET 0x04/* do get_page on page */ #define FOLL_ANON 0x08/* give ZERO_PAGE if no pgtable */ +typedef int (*pte_fn_t )(pte_t *pte,struct page *pmd_page,unsigned long addr, +void *data); +extern int apply_to_page_range (struct mm_struct *mm,unsigned long address, +unsigned long size,pte_fn_t fn,void *data); +#ifdef CONFIG_PROC_FS void vm_stat_account (struct mm_struct *,unsigned long ,struct file *,long );#else
mm/memory.c +94 -0
100 lines not on this page · raw message
--
Re: [RFC PATCH 01/33] Add apply_to_page_range() function
Quoted 9 lines · Chris Wright
...
mm/memory.c +2
-0@@ -1358,6 +1358,100 @@ int remap_pfn_range(struct vm_area_struc }EXPORT_SYMBOL (remap_pfn_range); +static inline int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd, + unsigned long addr, unsigned long end,
mm/memory.c int remap_pfn_range(struct vm_area_struc · line 1413
Quoted 10 lines
mm/memory.c +7
-0+ pte_fn_t fn, void *data) +static inline int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud, + unsigned long addr, unsigned long end, + pte_fn_t fn, void *data) +static inline int apply_to_pud_range(struct mm_struct *mm, pgd_t *pgd, + unsigned long addr, unsigned long end, + pte_fn_t fn, void *data)
Please avoid "inline" in C files.
(gcc already automatically inlines static functions with only one caller.)
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedRe: [RFC PATCH 01/33] Add apply_to_page_range() function
Quoted 23 lines · * Adrian Bunk ([email protected]) wrote:
Quoted 9 lines · Chris Wright
...
mm/memory.c +2
-0@@ -1358,6 +1358,100 @@ int remap_pfn_range(struct vm_area_struc }EXPORT_SYMBOL (remap_pfn_range); +static inline int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd, + unsigned long addr, unsigned long end,mm/memory.c int remap_pfn_range(struct vm_area_struc · line 1413
Quoted 10 lines
mm/memory.c +7
-0+ pte_fn_t fn, void *data) +static inline int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud, + unsigned long addr, unsigned long end, + pte_fn_t fn, void *data) +static inline int apply_to_pud_range(struct mm_struct *mm, pgd_t *pgd, + unsigned long addr, unsigned long end, + pte_fn_t fn, void *data)Please avoid "inline" in C files.
Quoted 1 line
(gcc already automatically inlines static functions with only one caller.)
Sure, that's fair. The surrounding similar code follows the same format as above, perhaps you plan to patch?
Re: [RFC PATCH 01/33] Add apply_to_page_range() function
Quoted 24 lines · Chris Wright
Quoted 23 lines · * Adrian Bunk ([email protected]) wrote:
Quoted 9 lines · Chris Wright
...
mm/memory.c +2
-0@@ -1358,6 +1358,100 @@ int remap_pfn_range(struct vm_area_struc }EXPORT_SYMBOL (remap_pfn_range); +static inline int apply_to_pte_range(struct mm_struct *mm, pmd_t *pmd, + unsigned long addr, unsigned long end,mm/memory.c int remap_pfn_range(struct vm_area_struc · line 1413
Quoted 10 lines
mm/memory.c +7
-0+ pte_fn_t fn, void *data) +static inline int apply_to_pmd_range(struct mm_struct *mm, pud_t *pud, + unsigned long addr, unsigned long end, + pte_fn_t fn, void *data) +static inline int apply_to_pud_range(struct mm_struct *mm, pgd_t *pgd, + unsigned long addr, unsigned long end, + pte_fn_t fn, void *data)Please avoid "inline" in C files.
Quoted 4 lines
Quoted 1 line
(gcc already automatically inlines static functions with only one caller.)
Sure, that's fair. The surrounding similar code follows the same format as above, perhaps you plan to patch?
Already part of a bigger item on my TODO list for some months.
Priority: low...
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedRFC PATCH 2/33 Add sync bitops
In reply to Chris WrightAdd "always lock'd" implementations of set_bit, clear_bit and change_bit and the corresponding test_and_ functions. Also add "always lock'd" implementation of cmpxchg. These give guaranteed strong synchronisation and are required for non-SMP kernels running on an SMP hypervisor.
--- include/asm-i386/synch_bitops.h | 166 ++++++++++++++++++++++++++++++++++++++++ include/asm-i386/system.h | 33 +++++++ 2 files changed, 199 insertions(+)
include/asm-i386/system.h +33 -0
@@ -263,6 +263,9 @@ static inline unsigned long __xchg(unsig#define cmpxchg (ptr,o,n)\ ((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)(o),\ (unsigned long)(n),sizeof(*(ptr)))) +#define synch_cmpxchg (ptr,o,n)\ + ((__typeof__(*(ptr)))__synch_cmpxchg((ptr),(unsigned long)(o),\ + (unsigned long)(n),sizeof(*(ptr))))#endif static inline unsigned long __cmpxchg(volatile void *ptr, unsigned long old,
@@ -291,6 +294,36 @@ static inline unsigned long __cmpxchg(vo }return old; } + +#define __LOCK_PREFIX "lock ; " +static inline unsigned long __synch_cmpxchg (volatile void *ptr, +unsigned long old, +unsigned long new,int size) +{ +unsigned long prev; +switch (size) { +case 1 : + __asm__ __volatile__(__LOCK_PREFIX"cmpxchgb %b1,%2" + :"=a" (prev) + :"q" (new),"m" (*__xg (ptr)),"0" (old) + :"memory" ); +return prev; +case 2 : + __asm__ __volatile__(__LOCK_PREFIX"cmpxchgw %w1,%2" + :"=a" (prev) + :"r" (new),"m" (*__xg (ptr)),"0" (old) + :"memory" ); +return prev; +case 4 : + __asm__ __volatile__(__LOCK_PREFIX"cmpxchgl %1,%2" + :"=a" (prev) + :"r" (new),"m" (*__xg (ptr)),"0" (old) + :"memory" ); +return prev; + } +return old; +} +#undef __LOCK_PREFIX#ifndef CONFIG_X86_CMPXCHG /*
include/asm-i386/synch_bitops.h new file +166 -0
166 lines not on this page · raw message
--
Re: [RFC PATCH 02/33] Add sync bitops
Quoted 6 lines · Chris Wright
plain text document attachment (synch-ops)
Add "always lock'd" implementations ofset_bit,clear_bitandchange_bitand the correspondingtest_and_functions. Also add "always lock'd" implementation of cmpxchg. These give guaranteed strong synchronisation and are required for non-SMP kernels running on an SMP hypervisor.
Hi,
this sounds really like the wrong approach; you know you're compiling for xen, so why not just make set_bit() and the others use the lock'd instructions at compile time?
Greetings,
Arjan van de Ven
Re: [RFC PATCH 02/33] Add sync bitops
Quoted 7 lines · Arjan van de Ven
Quoted 6 lines
plain text document attachment (synch-ops)
Add "always lock'd" implementations ofset_bit,clear_bitandchange_bitand the correspondingtest_and_functions. Also add "always lock'd" implementation of cmpxchg. These give guaranteed strong synchronisation and are required for non-SMP kernels running on an SMP hypervisor.
Quoted 5 lines
Hi,
this sounds really like the wrong approach; you know you're compiling for xen, so why not just make
set_bit()and the others use the lock'd instructions at compile time?
Then all users of bitops would unnecessarily pay the price when running in a single-CPU guest. Only a few of our bitops callers absolutely require the lock prefix in all cases.
-- Keir
Re: [RFC PATCH 02/33] Add sync bitops
In reply to Arjan van de VenQuoted 8 lines · Arjan van de Ven
Quoted 6 lines · Chris Wright
plain text document attachment (synch-ops)
Add "always lock'd" implementations ofset_bit,clear_bitandchange_bitand the correspondingtest_and_functions. Also add "always lock'd" implementation of cmpxchg. These give guaranteed strong synchronisation and are required for non-SMP kernels running on an SMP hypervisor.
Quoted 5 lines
Hi,
this sounds really like the wrong approach; you know you're compiling for xen, so why not just make
set_bit()and the others use the lock'd instructions at compile time?
I guess because they only need it for a small subset of set_bits.
-Andi
Re: [RFC PATCH 02/33] Add sync bitops
In reply to Chris WrightQuoted 19 lines · Chris Wright
Add "always lock'd" implementations of
set_bit,clear_bitandchange_bitand the correspondingtest_and_functions. Also add "always lock'd" implementation of cmpxchg. These give guaranteed strong synchronisation and are required for non-SMP kernels running on an SMP hypervisor.Signed-off-by Ian Pratt <[email protected]>Signed-off-by Christian Limpach <[email protected]>Signed-off-by Chris Wright <[email protected]>Cc Christoph Lameter <[email protected]>--- include/asm-i386/synch_bitops.h | 166 ++++++++++++++++++++++++++++++++++++++++ include/asm-i386/system.h | 33 +++++++ 2 files changed, 199 insertions(+)diff -r
935903fb1136include/asm-i386/system.h
include/asm-i386/system.h static inline unsigned long __xchg(unsig · line 269
Quoted 8 lines
include/asm-i386/system.h +3
-0@@ -263,6 +263,9 @@ static inline unsigned long __xchg(unsig#define cmpxchg (ptr,o,n)\ ((__typeof__(*(ptr)))__cmpxchg((ptr),(unsigned long)(o),\ (unsigned long)(n),sizeof(*(ptr)))) +#define synch_cmpxchg (ptr,o,n)\ + ((__typeof__(*(ptr)))__synch_cmpxchg((ptr),(unsigned long)(o),\ + (unsigned long)(n),sizeof(*(ptr))))#endif
Do I miss anything, or is CONFIG_X86_XEN=y, CONFIG_M386=y an allowed configuration that will result in a compile error?
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedRFC PATCH 3/33 Add nosegneg capability to the vsyscall page notes
In reply to Chris WrightAdd the "nosegneg" fake capabilty to the vsyscall page notes. This is used by the runtime linker to select a glibc version which then disables negative-offset accesses to the thread-local segment via %gs. These accesses require emulation in Xen (because segments are truncated to protect the hypervisor address space) and avoiding them provides a measurable performance boost.
--- arch/i386/kernel/vsyscall-note.S | 29 +++++++++++++++++++++++++++++ 1 files changed, 29 insertions(+)
arch/i386/kernel/vsyscall-note.S +29 -0
@@ -3,6 +3,7 @@ * Here we can supply some information useful to userland. */ +#include <linux/config.h> #include <linux/uts.h> #include <linux/version.h>
@@ -23,3 +24,31 @@ 3: .balign 4; /* pad out section */ ASM_ELF_NOTE_BEGIN(".note.kernel-version", "a", UTS_SYSNAME, 0) .long LINUX_VERSION_CODE ASM_ELF_NOTE_END + +#ifdef CONFIG_XEN +/* + * Add a special note telling glibc's dynamic linker a fake hardware + * flavor that it will use to choose the search path for libraries in the + * same way it uses real hardware capabilities like "mmx". + * We supply "nosegneg" as the fake capability, to indicate that we + * do not like negative offsets in instructions using segment overrides, + * since we implement those inefficiently. This makes it possible to + * install libraries optimized to avoid those access patterns in someplace + * like /lib/i686/tls/nosegneg. Note that an /etc/ld.so.conf.d/file + * corresponding to the bits here is needed to make ldconfig work right. + * It should contain: + * hwcap 0 nosegneg + * to match the mapping of bit to name that we give here. + */ +#define NOTE_KERNELCAP_BEGIN(ncaps, mask) \ + ASM_ELF_NOTE_BEGIN(".note.kernelcap", "a", "GNU", 2) \ + .long ncaps, mask +#define NOTE_KERNELCAP(bit, name) \ + .byte bit; .asciz name +#define NOTE_KERNELCAP_END ASM_ELF_NOTE_END + +NOTE_KERNELCAP_BEGIN(1, 1) +NOTE_KERNELCAP(1, "nosegneg") +NOTE_KERNELCAP_END +#endif +
--
RFC PATCH 4/33 Add XEN config options and disable unsupported config options.
In reply to Chris WrightThe XEN config option is selected from the i386 subarch menu by choosing the X86_XEN "Xen-compatible" subarch.
The XEN_SHADOW_MODE option defines the memory virtualization mode for the kernel -- with it enabled, the kernel expects the hypervisor to perform translation between pseudo-physical and machine addresses on its behalf.
The disabled config options are:
- DOUBLEFAULT: are trapped by Xen and not virtualized
- HZ: defaults to 100 in Xen VMs
- Power management: not supported in unprivileged VMs
- SMP: not supported in this set of patches
- X86_{UP,LOCAL,IO}_APIC: not supported in unprivileged VMs
--- arch/i386/Kconfig | 18 ++++++++++++++---- arch/i386/Kconfig.debug | 1 + drivers/xen/Kconfig | 21 +++++++++++++++++++++ kernel/Kconfig.hz | 4 ++-- kernel/Kconfig.preempt | 1 + 5 files changed, 39 insertions(+), 6 deletions(-)
arch/i386/Kconfig +15 -5
@@ -67,6 +67,7 @@ menu "Processor type and features" config SMP bool "Symmetric multi-processing support" + depends on !X86_XEN ---help--- This enables support for systems with more than one CPU. If you have a system with only one CPU, like most personal computers, say N. If
@@ -102,6 +103,12 @@ config X86_PC bool "PC-compatible" help Choose this option if your computer is a standard PC or compatible. + +config X86_XEN + bool "Xen-compatible" + help + Choose this option if you plan to run this kernel on top of the + Xen Hypervisor. config X86_ELAN bool "AMD Elan"
@@ -211,6 +218,7 @@ source "arch/i386/Kconfig.cpu" config HPET_TIMER bool "HPET Timer Support" + depends on !X86_XEN help This enables the use of the HPET for the kernel's internal timer. HPET is the next generation timer replacing legacy 8254s.
@@ -261,7 +269,7 @@ source "kernel/Kconfig.preempt" config X86_UP_APIC bool "Local APIC support on uniprocessors" - depends on !SMP && !(X86_VISWS || X86_VOYAGER) + depends on !SMP && !(X86_VISWS || X86_VOYAGER || X86_XEN) help A local APIC (Advanced Programmable Interrupt Controller) is an integrated interrupt controller in the CPU. If you have a single-CPU
@@ -286,12 +294,12 @@ config X86_UP_IOAPIC config X86_LOCAL_APIC bool - depends on X86_UP_APIC || ((X86_VISWS || SMP) && !X86_VOYAGER) + depends on X86_UP_APIC || ((X86_VISWS || SMP) && !(X86_VOYAGER || X86_XEN)) default y config X86_IO_APIC bool - depends on X86_UP_IOAPIC || (SMP && !(X86_VISWS || X86_VOYAGER)) + depends on X86_UP_IOAPIC || (SMP && !(X86_VISWS || X86_VOYAGER || X86_XEN)) default y config X86_VISWS_APIC
@@ -739,7 +747,7 @@ source kernel/Kconfig.hz config KEXEC bool "kexec system call (EXPERIMENTAL)" - depends on EXPERIMENTAL + depends on EXPERIMENTAL && !X86_XEN help kexec is a system call that implements the ability to shutdown your current kernel, and to start another kernel. It is like a reboot
@@ -808,7 +816,7 @@ config ARCH_ENABLE_MEMORY_HOTPLUG depends on HIGHMEM menu "Power management options (ACPI, APM)" - depends on !X86_VOYAGER + depends on !(X86_VOYAGER || X86_XEN) source kernel/power/Kconfig
@@ -1144,6 +1152,8 @@ source "security/Kconfig" source "crypto/Kconfig" +source "drivers/xen/Kconfig" + source "lib/Kconfig" #
arch/i386/Kconfig.cpu +1 -1
@@ -251,7 +251,7 @@ config X86_PPRO_FENCE config X86_F00F_BUG bool - depends on M586MMX || M586TSC || M586 || M486 || M386 + depends on (M586MMX || M586TSC || M586 || M486 || M386) && !XEN default y config X86_WP_WORKS_OK
arch/i386/Kconfig.debug +1 -0
@@ -79,6 +79,7 @@ config DOUBLEFAULT config DOUBLEFAULT default y bool "Enable doublefault exception handler" if EMBEDDED + depends on !X86_XEN help This option allows trapping of rare doublefault exceptions that would otherwise cause a system to silently reboot. Disabling this
kernel/Kconfig.hz +2 -2
@@ -3,7 +3,7 @@ # choice - prompt "Timer frequency" + prompt "Timer frequency" if !XEN default HZ_250 help Allows the configuration of the timer frequency. It is customary
@@ -40,7 +40,7 @@ endchoice config HZ int - default 100 if HZ_100 + default 100 if HZ_100 || XEN default 250 if HZ_250 default 1000 if HZ_1000
kernel/Kconfig.preempt +1 -0
@@ -35,6 +35,7 @@ config PREEMPT_VOLUNTARY config PREEMPT bool "Preemptible Kernel (Low-Latency Desktop)" + depends on !XEN help This option reduces the latency of the kernel by making all kernel code (that is not executing in a critical section)
drivers/xen/Kconfig new file +21 -0
@@ -0,0 +1,21 @@ +# +# This Kconfig describe xen options +# + +mainmenu "Xen Configuration" + +config XEN + bool + default y if X86_XEN + help + This is the Linux Xen port. + +if XEN + +config XEN_SHADOW_MODE + bool + default y + help + Fakes out a shadow mode kernel + +endif
--
Re: [RFC PATCH 04/33] Add XEN config options and disable unsupported config options.
Quoted 4 lines
The disabled config options are:
- DOUBLEFAULT: are trapped by Xen and not virtualized
- HZ: defaults to 100 in Xen VMs
Hi,
this makes no real sense with dynamic ticks coming up... HZ=1000 should be perfectly fine then...
arch/i386/Kconfig source kernel/Kconfig.hz · line 753
Quoted 7 lines
config KEXEC
arch/i386/Kconfig +1
-1bool "kexec system call (EXPERIMENTAL)" - depends on EXPERIMENTAL + depends on EXPERIMENTAL && !X86_XEN help kexec is a system call that implements the ability to shutdown your current kernel, and to start another kernel. It is like a reboot
hmmm why is kexec incompatible with xen? Don't you want to support crash dumps from guests?
drivers/xen/Kconfig line 20
Quoted 6 lines
drivers/xen/Kconfig new file +6
-0+config XEN_SHADOW_MODE + bool + default y + help + Fakes out a shadow mode kernel +
this probably wants a better description...
Re: [RFC PATCH 04/33] Add XEN config options and disable unsupported config options.
Quoted 11 lines · Arjan van de Ven
Quoted 8 lines
config KEXEC
arch/i386/Kconfig +1
-1bool "kexec system call (EXPERIMENTAL)" - depends on EXPERIMENTAL + depends on EXPERIMENTAL && !X86_XEN help kexec is a system call that implements the ability to shutdownyour
current kernel, and to start another kernel. It is like a reboothmmm why is kexec incompatible with xen? Don't you want to support crash dumps from guests?
There are kexec patches for Xen which haven't yet been merged into our tree. Even without that we can take coredumps of guests from the control VM, without the guest's active involvement.
-- Keir
RFC PATCH 5/33 Makefile support to build Xen subarch
In reply to Chris WrightUse arch/i386/mach-xen when building Xen subarch. The separate subarchitecture allows us to hide details of interfacing with the hypervisor from i386 common code.
--- arch/i386/Makefile | 5 +++++ arch/i386/mach-xen/Makefile | 7 +++++++ 2 files changed, 12 insertions(+)
arch/i386/Makefile +5 -0
@@ -71,6 +71,10 @@ mflags-$(CONFIG_X86_SUMMIT) := -Iinclude mflags-$(CONFIG_X86_SUMMIT) := -Iinclude/asm-i386/mach-summit mcore-$(CONFIG_X86_SUMMIT) := mach-default +# Xen subarch support +mflags-$(CONFIG_X86_XEN) := -Iinclude/asm-i386/mach-xen +mcore-$(CONFIG_X86_XEN) := mach-xen + # generic subarchitecture mflags-$(CONFIG_X86_GENERICARCH) := -Iinclude/asm-i386/mach-generic mcore-$(CONFIG_X86_GENERICARCH) := mach-default
@@ -99,6 +103,7 @@ drivers-$(CONFIG_PM) += arch/i386/powe CFLAGS += $(mflags-y) AFLAGS += $(mflags-y) +CPPFLAGS += $(mflags-y) boot := arch/i386/boot
arch/i386/mach-xen/Makefile new file +7 -0
@@ -0,0 +1,7 @@ +# +# Makefile for the linux kernel. +# + +obj-y := setup.o + +setup-y := ../mach-default/setup.o
--
Re: [RFC PATCH 05/33] Makefile support to build Xen subarch
Quoted 4 lines · Chris Wright
plain text document attachment (i386-mach-xen)
Usearch/i386/mach-xenwhen building Xen subarch. The separate subarchitecture allows us to hide details of interfacing with the hypervisor from i386 common code.
Hi,
please change the order of your patches next time, by sending the makefile and config options first you'll break git bisect...
Greetings,
Arjan van de Ven
Re: [RFC PATCH 05/33] Makefile support to build Xen subarch
Quoted 7 lines · On Tue, 18 Jul 2006 12:00:13 +0200 Arjan van de Ven <[email protected]> wrote:
Quoted 4 lines · Chris Wright
plain text document attachment (i386-mach-xen)
Usearch/i386/mach-xenwhen building Xen subarch. The separate subarchitecture allows us to hide details of interfacing with the hypervisor from i386 common code.
Quoted 5 lines
Hi,
please change the order of your patches next time, by sending the makefile and config options first you'll break git bisect...
No, this patch is OK (in that respect). As long as the git-bisect user sets CONFIG_XEN=n (and surely he will), things will compile.
That being said,
+mflags-$(CONFIG_X86_XEN) := -Iinclude/asm-i386/mach-xen
+mcore-$(CONFIG_X86_XEN) := mach-xen
+
# generic subarchitecture mflags-$(CONFIG_X86_GENERICARCH) := -Iinclude/asm-i386/mach-generic mcore-$(CONFIG_X86_GENERICARCH) := mach-default @@ -99,6 +103,7 @@ drivers-$(CONFIG_PM) += arch/i386/powe
CFLAGS += $(mflags-y)
AFLAGS += $(mflags-y)
+CPPFLAGS += $(mflags-y)
This change affects _all_ subarchitectures (by potentially altering their CPPFLAGS) and it's rather a mystery why one subarch needs the -Ifoo in its CPPFLAGS whereas all the others do not.
Re: [RFC PATCH 05/33] Makefile support to build Xen subarch
Quoted 10 lines · Andrew Morton
arch/i386/Makefile +1
-0@@ -99,6 +103,7 @@ drivers-$(CONFIG_PM) += arch/i386/powe CFLAGS += $(mflags-y) AFLAGS += $(mflags-y) +CPPFLAGS += $(mflags-y)This change affects _all_ subarchitectures (by potentially altering their CPPFLAGS) and it's rather a mystery why one subarch needs the -Ifoo in its CPPFLAGS whereas all the others do not.
The reason is that arch-i386/kernel/vmlinux.lds.S is run through CPP, and it includes asm/thread_info.h and asm/page.h, which end up including "mach_page.h" (which this patch series introduces). There is a version in both mach-default/mach_page.h and mach-xen/mach_page.h, so the -I is necessary for non-Xen sub-arches as well.
I guess the conservative approach would be to only add this -I for the vmlinux.lds target, assuming there are no later compile problems. On the flip-side, would you want C and Asm code getting a different set of includes from "manually" preprocessed-files? I would think you'd want either defines/includes at all, or to have them identical.
The CPPFLAGS assignment also appears to make the previous two lines redundant, since a_flags and c_flags (which is what actually gets used for compilation) end up having CPPFLAGS incorporated into them.
J
Re: [RFC PATCH 05/33] Makefile support to build Xen subarch
In reply to Arjan van de VenQuoted 3 lines · Arjan van de Ven
please change the order of your patches next time, by sending the makefile and config options first you'll break git bisect...
I think Chris was planning on doing this, but I suspect it got put to one side in order to get the patch set out in time for KS.
J
RFC PATCH 6/33 Add Xen interface header files
In reply to Chris WrightAdd Xen interface header files. These are taken fairly directly from the Xen tree and hence the style is not entirely in accordance with Linux guidelines. There is a tension between fitting with Linux coding rules and ease of maintenance.
--- include/xen/interface/arch-x86_32.h | 187 ++++++++++++++ include/xen/interface/event_channel.h | 195 +++++++++++++++ include/xen/interface/features.h | 43 +++ include/xen/interface/grant_table.h | 301 +++++++++++++++++++++++ include/xen/interface/io/blkif.h | 75 +++++ include/xen/interface/io/console.h | 23 + include/xen/interface/io/netif.h | 74 +++++ include/xen/interface/io/ring.h | 252 +++++++++++++++++++ include/xen/interface/io/xenbus.h | 42 +++ include/xen/interface/io/xs_wire.h | 87 ++++++ include/xen/interface/memory.h | 145 +++++++++++ include/xen/interface/physdev.h | 61 ++++ include/xen/interface/sched.h | 77 ++++++ include/xen/interface/vcpu.h | 109 ++++++++ include/xen/interface/version.h | 60 ++++ include/xen/interface/xen.h | 431 ++++++++++++++++++++++++++++++++++ 16 files changed, 2162 insertions(+)
include/xen/interface/arch-x86_32.h new file +187 -0
@@ -0,0 +1,187 @@ +/****************************************************************************** +* arch-x86_32.h +* +* Guest OS interface to x86 32-bit Xen. +* +* Copyright (c) 2004, K A Fraser +*/ + +#ifndef __XEN_PUBLIC_ARCH_X86_32_H__ +#define __XEN_PUBLIC_ARCH_X86_32_H__ + +#ifdef __XEN__ +#define __DEFINE_GUEST_HANDLE (name, type) \ + typedef struct { type *p; } __guest_handle_ ## name +#else +#define __DEFINE_GUEST_HANDLE (name, type) \ + typedef type * __guest_handle_ ## name +#endif + +#define DEFINE_GUEST_HANDLE_STRUCT (name) \ + __DEFINE_GUEST_HANDLE(name, struct name) +#define DEFINE_GUEST_HANDLE (name) __DEFINE_GUEST_HANDLE(name, name) +#define GUEST_HANDLE (name) __guest_handle_ ## name + +#ifndef __ASSEMBLY__ +/* Guest handles for primitive C types. */ +__DEFINE_GUEST_HANDLE(uchar, unsigned char); +__DEFINE_GUEST_HANDLE(uint, unsigned int); +__DEFINE_GUEST_HANDLE(ulong, unsigned long); +DEFINE_GUEST_HANDLE (char); +DEFINE_GUEST_HANDLE (int); +DEFINE_GUEST_HANDLE (long); +DEFINE_GUEST_HANDLE (void); +#endif + +/* +* SEGMENT DESCRIPTOR TABLES +*/ +/* +* A number of GDT entries are reserved by Xen. These are not situated at the +* start of the GDT because some stupid OSes export hard-coded selector values +* in their ABI. These hard-coded values are always near the start of the GDT, +* so Xen places itself out of the way, at the far end of the GDT. +*/ +#define FIRST_RESERVED_GDT_PAGE 14 +#define FIRST_RESERVED_GDT_BYTE (FIRST_RESERVED_GDT_PAGE * 4096) +#define FIRST_RESERVED_GDT_ENTRY (FIRST_RESERVED_GDT_BYTE / 8) + +/* +* These flat segments are in the Xen-private section of every GDT. Since these +* are also present in the initial GDT, many OSes will be able to avoid +* installing their own GDT. +*/ +#define FLAT_RING1_CS 0xe019/* GDT index 259 */ +#define FLAT_RING1_DS 0xe021/* GDT index 260 */ +#define FLAT_RING1_SS 0xe021/* GDT index 260 */ +#define FLAT_RING3_CS 0xe02b/* GDT index 261 */ +#define FLAT_RING3_DS 0xe033/* GDT index 262 */ +#define FLAT_RING3_SS 0xe033/* GDT index 262 */ + +#define FLAT_KERNEL_CS FLAT_RING1_CS +#define FLAT_KERNEL_DS FLAT_RING1_DS +#define FLAT_KERNEL_SS FLAT_RING1_SS +#define FLAT_USER_CS FLAT_RING3_CS +#define FLAT_USER_DS FLAT_RING3_DS +#define FLAT_USER_SS FLAT_RING3_SS + +/* And the trap vector is... */ +#define TRAP_INSTR "int $0x82" + +/* +* Virtual addresses beyond this are not modifiable by guest OSes. The +* machine->physical mapping table starts at this address, read-only. +*/ +#ifdef CONFIG_X86_PAE +#define __HYPERVISOR_VIRT_START 0xF5800000 +#else +#define __HYPERVISOR_VIRT_START 0xFC000000 +#endif + +#ifndef HYPERVISOR_VIRT_START +#define HYPERVISOR_VIRT_START mk_unsigned_long(__HYPERVISOR_VIRT_START) +#endif + +#ifndef machine_to_phys_mapping +#define machine_to_phys_mapping ((unsigned long *)HYPERVISOR_VIRT_START) +#endif + +/* Maximum number of virtual CPUs in multi-processor guests. */ +#define MAX_VIRT_CPUS 32 + +#ifndef __ASSEMBLY__ + +/* +* Send an array of these to HYPERVISOR_set_trap_table() +*/ +#define TI_GET_DPL (_ti) ((_ti)->flags & 3) +#define TI_GET_IF (_ti) ((_ti)->flags & 4) +#define TI_SET_DPL (_ti,_dpl) ((_ti)->flags |= (_dpl)) +#define TI_SET_IF (_ti,_if) ((_ti)->flags |= ((!!(_if))<<2)) +struct trap_info { +uint8_t vector;/* exception vector */ +uint8_t flags;/* 0-3: privilege level; 4: clear event enable? */ +uint16_t cs;/* code selector */ +unsigned long address;/* code offset */ +}; +DEFINE_GUEST_HANDLE_STRUCT (trap_info); + +struct cpu_user_regs { +uint32_t ebx; +uint32_t ecx; +uint32_t edx; +uint32_t esi; +uint32_t edi; +uint32_t ebp; +uint32_t eax; +uint16_t error_code;/* private */ +uint16_t entry_vector;/* private */ +uint32_t eip; +uint16_t cs; +uint8_t saved_upcall_mask; +uint8_t _pad0; +uint32_t eflags;/* eflags.IF == !saved_upcall_mask */ +uint32_t esp; +uint16_t ss, _pad1; +uint16_t es, _pad2; +uint16_t ds, _pad3; +uint16_t fs, _pad4; +uint16_t gs, _pad5; +}; +DEFINE_GUEST_HANDLE_STRUCT (cpu_user_regs); + +typedef uint64_t tsc_timestamp_t ;/* RDTSC timestamp */ + +/* +* The following is all CPU context. Note that the fpu_ctxt block is filled +* in by FXSAVE if the CPU has feature FXSR; otherwise FSAVE is used. +*/ +struct vcpu_guest_context { +/* FPU registers come first so they can be aligned for FXSAVE/FXRSTOR. */ +struct {char x[512 ]; } fpu_ctxt;/* User-level FPU registers */ +#define VGCF_I387_VALID (1<<0) +#define VGCF_HVM_GUEST (1<<1) +#define VGCF_IN_KERNEL (1<<2) +unsigned long flags;/* VGCF_* flags */ +struct cpu_user_regs user_regs;/* User-level CPU registers */ +struct trap_info trap_ctxt[256 ];/* Virtual IDT */ +unsigned long ldt_base, ldt_ents;/* LDT (linear address, # ents) */ +unsigned long gdt_frames[16 ], gdt_ents;/* GDT (machine frames, # ents) */ +unsigned long kernel_ss, kernel_sp;/* Virtual TSS (only SS1/SP1) */ +unsigned long ctrlreg[8 ];/* CR0-CR7 (control registers) */ +unsigned long debugreg[8 ];/* DB0-DB7 (debug registers) */ +unsigned long event_callback_cs;/* CS:EIP of event callback */ +unsigned long event_callback_eip; +unsigned long failsafe_callback_cs;/* CS:EIP of failsafe callback */ +unsigned long failsafe_callback_eip; +unsigned long vm_assist;/* VMASST_TYPE_* bitmap */ +}; +DEFINE_GUEST_HANDLE_STRUCT (vcpu_guest_context); + +struct arch_shared_info { +unsigned long max_pfn;/* max pfn that appears in table */ +/* Frame containing list of mfns containing list of mfns containing p2m. */ +unsigned long pfn_to_mfn_frame_list_list; +unsigned long nmi_reason; +}; + +struct arch_vcpu_info { +unsigned long cr2; +unsigned long pad[5 ];/* sizeof(struct vcpu_info) == 64 */ +}; + +#endif /* !__ASSEMBLY__ */ + +/* +* Prefix forces emulation of some non-trapping instructions. +* Currently only CPUID. +*/ +#ifdef __ASSEMBLY__ +#define XEN_EMULATE_PREFIX .byte 0x0f,0x0b,0x78,0x65,0x6e ; +#define XEN_CPUID XEN_EMULATE_PREFIX cpuid +#else +#define XEN_EMULATE_PREFIX ".byte 0x0f,0x0b,0x78,0x65,0x6e ; " +#define XEN_CPUID XEN_EMULATE_PREFIX "cpuid" +#endif + +#endif
include/xen/interface/event_channel.h new file +195 -0
195 lines not on this page · raw message
include/xen/interface/features.h new file +43 -0
43 lines not on this page · raw message
include/xen/interface/grant_table.h new file +301 -0
301 lines not on this page · raw message
include/xen/interface/io/blkif.h new file +75 -0
75 lines not on this page · raw message
include/xen/interface/io/console.h new file +23 -0
@@ -0,0 +1,23 @@ +/****************************************************************************** +* console.h +* +* Console I/O interface for Xen guest OSes. +* +* Copyright (c) 2005, Keir Fraser +*/ + +#ifndef __XEN_PUBLIC_IO_CONSOLE_H__ +#define __XEN_PUBLIC_IO_CONSOLE_H__ + +typedef uint32_t XENCONS_RING_IDX ; + +#define MASK_XENCONS_IDX (idx, ring) ((idx) & (sizeof(ring)-1)) + +struct xencons_interface { +char in[1024 ]; +char out[2048 ]; +XENCONS_RING_IDX in_cons, in_prod; +XENCONS_RING_IDX out_cons, out_prod; +}; + +#endif /* __XEN_PUBLIC_IO_CONSOLE_H__ */
include/xen/interface/io/netif.h new file +78 -0
78 lines not on this page · raw message
include/xen/interface/io/ring.h new file +260 -0
260 lines not on this page · raw message
include/xen/interface/io/xenbus.h new file +42 -0
42 lines not on this page · raw message
include/xen/interface/io/xs_wire.h new file +87 -0
87 lines not on this page · raw message
include/xen/interface/memory.h new file +145 -0
145 lines not on this page · raw message
include/xen/interface/physdev.h new file +61 -0
61 lines not on this page · raw message
include/xen/interface/sched.h new file +77 -0
77 lines not on this page · raw message
include/xen/interface/vcpu.h new file +109 -0
109 lines not on this page · raw message
include/xen/interface/version.h new file +60 -0
60 lines not on this page · raw message
include/xen/interface/xen.h new file +431 -0
431 lines not on this page · raw message
--
RFC PATCH 7/33 Hypervisor interface header files.
In reply to Chris WrightDefine macros and inline functions for doing hypercalls into the hypervisor.
--- include/asm-i386/hypercall.h | 309 ++++++++++++++++++++++++++++++++++++++++++ include/asm-i386/hypervisor.h | 70 +++++++++ 2 files changed, 379 insertions(+)
include/asm-i386/hypercall.h new file +309 -0
@@ -0,0 +1,309 @@ +/****************************************************************************** +* hypercall.h +* +* Linux-specific hypervisor handling. +* +* Copyright (c) 2002-2004, K A Fraser +* +* This program is free software; you can redistribute it and/or +* modify it under the terms of the GNU General Public License version 2 +* as published by the Free Software Foundation; or, when distributed +* separately from the Linux kernel or incorporated into other +* software packages, subject to the following license: +* +* Permission is hereby granted, free of charge, to any person obtaining a copy +* of this source file (the "Software"), to deal in the Software without +* restriction, including without limitation the rights to use, copy, modify, +* merge, publish, distribute, sublicense, and/or sell copies of the Software, +* and to permit persons to whom the Software is furnished to do so, subject to +* the following conditions: +* +* The above copyright notice and this permission notice shall be included in +* all copies or substantial portions of the Software. +* +* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS +* IN THE SOFTWARE. +*/ + +#ifndef __HYPERCALL_H__ +#define __HYPERCALL_H__ + +#include <linux/config.h> + +#include <xen/interface/xen.h> +#include <xen/interface/sched.h> + +#define __STR (x) #x +#define STR (x) __STR(x) + +#define _hypercall0 (type, name) \ +({ \ + long __res; \ + asm volatile ( \ + "call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\ + : "=a" (__res) \ + : \ + : "memory" ); \ + (type)__res; \ +}) + +#define _hypercall1 (type, name, a1) \ +({ \ + long __res, __ign1; \ + asm volatile ( \ + "call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\ + : "=a" (__res), "=b" (__ign1) \ + : "1" ((long)(a1)) \ + : "memory" ); \ + (type)__res; \ +}) + +#define _hypercall2 (type, name, a1, a2) \ +({ \ + long __res, __ign1, __ign2; \ + asm volatile ( \ + "call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\ + : "=a" (__res), "=b" (__ign1), "=c" (__ign2) \ + : "1" ((long)(a1)), "2" ((long)(a2)) \ + : "memory" ); \ + (type)__res; \ +}) + +#define _hypercall3 (type, name, a1, a2, a3) \ +({ \ + long __res, __ign1, __ign2, __ign3; \ + asm volatile ( \ + "call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\ + : "=a" (__res), "=b" (__ign1), "=c" (__ign2), \ + "=d" (__ign3) \ + : "1" ((long)(a1)), "2" ((long)(a2)), \ + "3" ((long)(a3)) \ + : "memory" ); \ + (type)__res; \ +}) + +#define _hypercall4 (type, name, a1, a2, a3, a4) \ +({ \ + long __res, __ign1, __ign2, __ign3, __ign4; \ + asm volatile ( \ + "call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\ + : "=a" (__res), "=b" (__ign1), "=c" (__ign2), \ + "=d" (__ign3), "=S" (__ign4) \ + : "1" ((long)(a1)), "2" ((long)(a2)), \ + "3" ((long)(a3)), "4" ((long)(a4)) \ + : "memory" ); \ + (type)__res; \ +}) + +#define _hypercall5 (type, name, a1, a2, a3, a4, a5) \ +({ \ + long __res, __ign1, __ign2, __ign3, __ign4, __ign5; \ + asm volatile ( \ + "call hypercall_page + ("STR(__HYPERVISOR_##name)" * 32)"\ + : "=a" (__res), "=b" (__ign1), "=c" (__ign2), \ + "=d" (__ign3), "=S" (__ign4), "=D" (__ign5) \ + : "1" ((long)(a1)), "2" ((long)(a2)), \ + "3" ((long)(a3)), "4" ((long)(a4)), \ + "5" ((long)(a5)) \ + : "memory" ); \ + (type)__res; \ +}) + +static inline int +HYPERVISOR_set_trap_table ( +struct trap_info *table) +{ +return _hypercall1 (int, set_trap_table, table); +} + +static inline int +HYPERVISOR_mmu_update ( +struct mmu_update *req,int count,int *success_count,domid_t domid) +{ +return _hypercall4 (int, mmu_update, req, count, success_count, domid); +} + +static inline int +HYPERVISOR_mmuext_op ( +struct mmuext_op *op,int count,int *success_count,domid_t domid) +{ +return _hypercall4 (int, mmuext_op, op, count, success_count, domid); +} + +static inline int +HYPERVISOR_set_gdt ( +unsigned long *frame_list,int entries) +{ +return _hypercall2 (int, set_gdt, frame_list, entries); +} + +static inline int +HYPERVISOR_stack_switch ( +unsigned long ss,unsigned long esp) +{ +return _hypercall2 (int, stack_switch, ss, esp); +} + +static inline int +HYPERVISOR_set_callbacks ( +unsigned long event_selector,unsigned long event_address, +unsigned long failsafe_selector,unsigned long failsafe_address) +{ +return _hypercall4 (int, set_callbacks, + event_selector, event_address, + failsafe_selector, failsafe_address); +} + +static inline int +HYPERVISOR_fpu_taskswitch ( +int set) +{ +return _hypercall1 (int, fpu_taskswitch, set); +} + +static inline int +HYPERVISOR_sched_op ( +int cmd,unsigned long arg) +{ +return _hypercall2 (int, sched_op, cmd, arg); +} + +static inline long +HYPERVISOR_set_timer_op ( +u64 timeout) +{ +unsigned long timeout_hi = (unsigned long )(timeout>>32 ); +unsigned long timeout_lo = (unsigned long )timeout; +return _hypercall2 (long, set_timer_op, timeout_lo, timeout_hi); +} + +static inline int +HYPERVISOR_set_debugreg ( +int reg,unsigned long value) +{ +return _hypercall2 (int, set_debugreg, reg, value); +} + +static inline unsigned long +HYPERVISOR_get_debugreg ( +int reg) +{ + return _hypercall1(unsigned long, get_debugreg, reg); +} + +static inline int +HYPERVISOR_update_descriptor ( +u64 ma,u64 desc) +{ +return _hypercall4 (int, update_descriptor, ma, ma>>32 , desc, desc>>32 ); +} + +static inline int +HYPERVISOR_memory_op ( +unsigned int cmd,void *arg) +{ +return _hypercall2 (int, memory_op, cmd, arg); +} + +static inline int +HYPERVISOR_multicall ( +void *call_list,int nr_calls) +{ +return _hypercall2 (int, multicall, call_list, nr_calls); +} + +static inline int +HYPERVISOR_update_va_mapping ( +unsigned long va,pte_t new_val,unsigned long flags) +{ +unsigned long pte_hi =0 ; +#ifdef CONFIG_X86_PAE + pte_hi = new_val.pte_high; +#endif +return _hypercall4 (int, update_va_mapping, va, + new_val.pte_low, pte_hi, flags); +} + +static inline int +HYPERVISOR_event_channel_op ( +void *op) +{ +return _hypercall1 (int, event_channel_op, op); +} + +static inline int +HYPERVISOR_xen_version ( +int cmd,void *arg) +{ +return _hypercall2 (int, xen_version, cmd, arg); +} + +static inline int +HYPERVISOR_console_io ( +int cmd,int count,char *str) +{ +return _hypercall3 (int, console_io, cmd, count, str); +} + +static inline int +HYPERVISOR_physdev_op ( +void *physdev_op) +{ +return _hypercall1 (int, physdev_op, physdev_op); +} + +static inline int +HYPERVISOR_grant_table_op ( +unsigned int cmd,void *uop,unsigned int count) +{ +return _hypercall3 (int, grant_table_op, cmd, uop, count); +} + +static inline int +HYPERVISOR_update_va_mapping_otherdomain ( +unsigned long va,pte_t new_val,unsigned long flags,domid_t domid) +{ +unsigned long pte_hi =0 ; +#ifdef CONFIG_X86_PAE + pte_hi = new_val.pte_high; +#endif +return _hypercall5 (int, update_va_mapping_otherdomain, va, + new_val.pte_low, pte_hi, flags, domid); +} + +static inline int +HYPERVISOR_vm_assist ( +unsigned int cmd,unsigned int type) +{ +return _hypercall2 (int, vm_assist, cmd, type); +} + +static inline int +HYPERVISOR_vcpu_op ( +int cmd,int vcpuid,void *extra_args) +{ +return _hypercall3 (int, vcpu_op, cmd, vcpuid, extra_args); +} + +static inline int +HYPERVISOR_suspend ( +unsigned long srec) +{ +return _hypercall3 (int, sched_op, SCHEDOP_shutdown, + SHUTDOWN_suspend, srec); +} + +static inline int +HYPERVISOR_nmi_op ( +unsigned long op, +unsigned long arg) +{ +return _hypercall2 (int, nmi_op, op, arg); +} + +#endif /* __HYPERCALL_H__ */
include/asm-i386/hypervisor.h new file +69 -0
69 lines not on this page · raw message
--
RFC PATCH 8/33 Add vmlinuz build target.
In reply to Chris WrightThe vmlinuz image is a stripped and compressed kernel image, it is smaller than the vmlinux image and the Xen domain builder supports loading compressed images directly.
--- arch/i386/Makefile | 5 +++-- arch/i386/boot/Makefile | 10 +++++++++- 2 files changed, 12 insertions(+), 3 deletions(-)
arch/i386/Makefile +3 -2
@@ -108,15 +108,16 @@ boot := arch/i386/boot boot := arch/i386/boot PHONY += zImage bzImage compressed zlilo bzlilo \ - zdisk bzdisk fdimage fdimage144 fdimage288 isoimage install + zdisk bzdisk fdimage fdimage144 fdimage288 isoimage install vmlinuz all: bzImage # KBUILD_IMAGE specify target image being built KBUILD_IMAGE := $(boot)/bzImage zImage zlilo zdisk: KBUILD_IMAGE := arch/i386/boot/zImage +vmlinuz: KBUILD_IMAGE := $(boot)/vmlinuz -zImage bzImage: vmlinux +zImage bzImage vmlinuz: vmlinux $(Q)$(MAKE) $(build)=$(boot) $(KBUILD_IMAGE) compressed: zImage
arch/i386/boot/Makefile +9 -1
@@ -26,7 +26,7 @@ SVGA_MODE := -DSVGA_MODE=NORMAL_VGA #RAMDISK := -DRAMDISK=512 targets := vmlinux.bin bootsect bootsect.o \ - setup setup.o zImage bzImage + setup setup.o zImage bzImage vmlinuz subdir- := compressed hostprogs-y := tools/build
@@ -128,5 +128,13 @@ zlilo: $(BOOTIMAGE) cp System.map $(INSTALL_PATH)/ if [ -x /sbin/lilo ]; then /sbin/lilo; else /etc/lilo/install; fi +$(obj)/vmlinuz: $(obj)/vmlinux-stripped FORCE + $(call if_changed,gzip) + @echo 'Kernel: $@ is ready' ' (#'`cat .version`')' + +$(obj)/vmlinux-stripped: OBJCOPYFLAGS := -g --strip-unneeded +$(obj)/vmlinux-stripped: vmlinux FORCE + $(call if_changed,objcopy) + install: sh $(srctree)/$(src)/install.sh $(KERNELRELEASE) $(BOOTIMAGE) System.map "$(INSTALL_PATH)"
--
RFC PATCH 9/33 Add start-of-day setup hooks to subarch
In reply to Chris WrightImplement the start-of-day subarchitecture setup hooks for booting on Xen. Add subarch macros for determining loader type and initrd location.
--- arch/i386/kernel/setup.c | 2 + arch/i386/mach-default/Makefile | 2 - arch/i386/mach-default/setup-memory.c | 43 ++++++++++++++++++++++ arch/i386/mach-default/setup.c | 43 ---------------------- arch/i386/mach-xen/Makefile | 2 - arch/i386/mach-xen/setup-xen.c | 54 ++++++++++++++++++++++++++++ include/asm-i386/hypervisor.h | 4 ++ include/asm-i386/mach-default/mach_setup.h | 12 ++++++ include/asm-i386/mach-xen/mach_setup.h | 15 +++++++ include/asm-i386/mach-xen/setup_arch.h | 9 ++++ include/asm-i386/setup.h | 20 ++++++---- 11 files changed, 153 insertions(+), 53 deletions(-)
arch/i386/kernel/setup.c +2 -0
@@ -458,6 +458,7 @@ static void __init print_memory_map(char } } +#ifndef HAVE_ARCH_E820_SANITIZE /* * Sanitize the BIOS e820 map. *
@@ -677,6 +678,7 @@ int __init copy_e820_map(struct e820entr } while (biosmap++,--nr_map);return 0 ; } +#endif #if defined(CONFIG_EDD ) || defined(CONFIG_EDD_MODULE )struct edd edd;
arch/i386/mach-default/Makefile +1 -1
@@ -2,4 +2,4 @@ # Makefile for the linux kernel. # -obj-y := setup.o +obj-y := setup.o setup-memory.o
arch/i386/mach-default/setup.c +0 -43
52 lines not on this page · raw message
arch/i386/mach-xen/Makefile +1 -1
@@ -2,6 +2,6 @@ # Makefile for the linux kernel. # -obj-y := setup.o +obj-y := setup.o setup-xen.o setup-y := ../mach-default/setup.o
include/asm-i386/hypervisor.h +4 -0
@@ -55,6 +55,10 @@ extern struct shared_info *HYPERVISOR_shextern struct shared_info *HYPERVISOR_shared_info;extern struct start_info *xen_start_info; +/* arch/i386/mach-xen/entry.S */ +extern void hypervisor_callback (void ); +extern void failsafe_callback (void ); +/* arch/i386/mach-xen/evtchn.c */ /* Force a proper event-channel callback from Xen. */ extern void force_evtchn_callback (void );
include/asm-i386/setup.h +12 -8
@@ -49,10 +49,10 @@ extern unsigned char boot_params[PARAM_S#define VIDEO_MODE (*(unsigned short *) (PARAM+0x1FA))#define ORIG_ROOT_DEV (*(unsigned short *) (PARAM+0x1FC))#define AUX_DEVICE_INFO (*(unsigned char *) (PARAM+0x1FF)) -#define LOADER_TYPE (*(unsigned char *) (PARAM+0x210)) +#define LOADER_TYPE MACH_LOADER_TYPE#define KERNEL_START (*(unsigned long *) (PARAM+0x214)) -#define INITRD_START (*(unsigned long *) (PARAM+0x218)) -#define INITRD_SIZE (*(unsigned long *) (PARAM+0x21c)) +#define INITRD_START MACH_INITRD_START +#define INITRD_SIZE MACH_INITRD_SIZE#define EDID_INFO (*(struct edid_info *) (PARAM+0x140))#define EDD_NR (*(unsigned char *) (PARAM+EDDNR))#define EDD_MBR_SIG_NR (*(unsigned char *) (PARAM+EDD_MBR_SIG_NR_BUF))
@@ -65,15 +65,19 @@ extern unsigned char boot_params[PARAM_S*/ #define LOWMEMSIZE () (0x9f000) +extern unsigned long init_pg_tables_end; +struct e820entry ; -char *__init machine_specific_memory_setup (void ); +char *machine_specific_memory_setup (void ); -int __init copy_e820_map (struct e820entry * biosmap,int nr_map); -int __init sanitize_e820_map (struct e820entry * biosmap,char * pnr_map); -void __init add_memory_region (unsigned long long start, -unsigned long long size,int type); +int copy_e820_map (struct e820entry * biosmap,int nr_map); +int sanitize_e820_map (struct e820entry * biosmap,char * pnr_map); +void add_memory_region (unsigned long long start, +unsigned long long size,int type);#endif /* __ASSEMBLY__ */ +#include <mach_setup.h> +#endif /* _i386_SETUP_H */
arch/i386/mach-default/setup-memory.c new file +44 -0
44 lines not on this page · raw message
arch/i386/mach-xen/setup-xen.c new file +54 -0
54 lines not on this page · raw message
include/asm-i386/mach-default/mach_setup.h new file +12 -0
@@ -0,0 +1,12 @@ +#ifndef __ASM_MACH_SETUP_H +#define __ASM_MACH_SETUP_H + +#ifndef __ASSEMBLY__ + +#define MACH_LOADER_TYPE (*(unsigned char *) (PARAM+0x210)) +#define MACH_INITRD_START (*(unsigned long *) (PARAM+0x218)) +#define MACH_INITRD_SIZE (*(unsigned long *) (PARAM+0x21c)) + +#endif /* __ASSEMBLY__ */ + +#endif /* __ASM_MACH_SETUP_H */
include/asm-i386/mach-xen/mach_setup.h new file +15 -0
@@ -0,0 +1,15 @@ +#ifndef __ASM_MACH_SETUP_H +#define __ASM_MACH_SETUP_H + +#ifndef __ASSEMBLY__ + +#include <xen/interface/xen.h> + +#define MACH_LOADER_TYPE 0x6e6558/* "Xen" */ +#define MACH_INITRD_START \ + (xen_start_info->mod_start ? __pa(xen_start_info->mod_start) : 0) +#define MACH_INITRD_SIZE (xen_start_info->mod_len) + +#endif /* __ASSEMBLY__ */ + +#endif /* __ASM_MACH_SETUP_H */
include/asm-i386/mach-xen/setup_arch.h new file +9 -0
@@ -0,0 +1,9 @@ + +#define ARCH_SETUP machine_specific_arch_setup(); + +extern struct start_info *xen_start_info; +extern struct shared_info *HYPERVISOR_shared_info; + +void __init machine_specific_arch_setup (void ); + +#define HAVE_ARCH_E820_SANITIZE
--
Re: [RFC PATCH 09/33] Add start-of-day setup hooks to subarch
Quoted 17 lines · Chris Wright
plain text document attachment (i386-setup)
Implement the start-of-day subarchitecture setup hooks for booting on Xen. Add subarch macros for determining loader type and initrd location.diff -r
a5848bce3730arch/i386/kernel/setup.carch/i386/kernel/setup.c +1
-0@@ -458,6 +458,7 @@ static void __init print_memory_map(char } } +#ifndef HAVE_ARCH_E820_SANITIZE /* * Sanitize the BIOS e820 map. *
arch/i386/kernel/setup.c int __init copy_e820_map(struct e820entr · line 682
Quoted 5 lines
arch/i386/kernel/setup.c +1
-0@@ -677,6 +678,7 @@ int __init copy_e820_map(struct e820entr } while (biosmap++,--nr_map);return 0 ; } +#endif
Hi,
what is this for? Isn't this 1) undocumented and 2) unclear and 3) ugly ? (I'm pretty sure the HAVE_ARCH_* stuff is highly deprecated for new things nowadays)
Greetings,
Arjan van de Ven
Re: [RFC PATCH 09/33] Add start-of-day setup hooks to subarch
Quoted 4 lines · Arjan van de Ven
what is this for? Isn't this 1) undocumented and 2) unclear and 3) ugly ? (I'm pretty sure the HAVE_ARCH_* stuff is highly deprecated for new things nowadays)
It appears to be completely unnecessary. Xen doesn't use sanitize_e820_map(), but there's no obvious harm in leaving it there (and it's init code, so there's no long-term cost).
J
Re: [RFC PATCH 09/33] Add start-of-day setup hooks to subarch
In reply to Chris Wrightarch/i386/mach-xen/setup-xen.c line 18
Quoted 6 lines · Chris Wright
...
arch/i386/mach-xen/setup-xen.c new file +2
-0+struct start_info *xen_start_info; +EXPORT_SYMBOL (xen_start_info);
EXPORT_SYMBOL_GPL?
arch/i386/mach-xen/setup-xen.c line 25
Quoted 7 lines
arch/i386/mach-xen/setup-xen.c new file +6
-0+/* +* Point at the empty zero page to start with. We map the real shared_info +* page as soon as fixmap is up and running. +*/ +struct shared_info *HYPERVISOR_shared_info = (struct shared_info *)empty_zero_page; +EXPORT_SYMBOL (HYPERVISOR_shared_info);
EXPORT_SYMBOL_GPL?
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedRe: [RFC PATCH 09/33] Add start-of-day setup hooks to subarch
Quoted 4 lines · Adrian Bunk
arch/i386/mach-xen/setup-xen.c line 18
Quoted 2 lines
arch/i386/mach-xen/setup-xen.c new file +2
-0+struct start_info *xen_start_info; +EXPORT_SYMBOL (xen_start_info);
EXPORT_SYMBOL_GPL?
Possibly.
Quoted 11 lines
Quoted 9 lines
+/*
+ * Point at the empty zero page to start with. We map the realshared_info
+ * page as soon as fixmap is up and running.
+ */
+structshared_info*HYPERVISOR_shared_info= (structshared_info*)empty_zero_page; +EXPORT_SYMBOL(HYPERVISOR_shared_info);
...
EXPORT_SYMBOL_GPL?
Interrupt-control macros (local_irq_enable/disable and friends) used this symbol, so GPLing it made non-GPL modules fail. We made a bunch of the macros proper functions so this may no longer be the case.
-- Keir
Re: [RFC PATCH 09/33] Add start-of-day setup hooks to subarch
Quoted 8 lines · Keir Fraser
Quoted 6 lines
Quoted 4 lines
+struct
shared_info*HYPERVISOR_shared_info= (structshared_info*)empty_zero_page; +EXPORT_SYMBOL(HYPERVISOR_shared_info);
...
EXPORT_SYMBOL_GPL?Interrupt-control macros (local_irq_enable/disable and friends) used this symbol, so GPLing it made non-GPL modules fail. We made a bunch of the macros proper functions so this may no longer be the case.
Given that this is just a page exported from the hypervisor, it doesn't make much sense to impose a GPL requirement on this symbol (to put it another way, this is more part of the Xen ABI than the Linux API, so Linux can't make much claim to it).
J
RFC PATCH 10/33 add support for Xen feature queries
In reply to Chris WrightAdd support for parsing and interpreting hypervisor feature flags. These allow the kernel to determine what features are provided by the underlying hypervisor. For example, whether page tables need to be write protected explicitly by the kernel, and whether the kernel (appears to) run in ring 0 rather than ring 1. This information allows the kernel to improve performance by avoiding unnecessary actions.
--- arch/i386/mach-xen/setup-xen.c | 2 ++ drivers/Makefile | 1 + drivers/xen/Makefile | 3 +++ drivers/xen/core/Makefile | 3 +++ drivers/xen/core/features.c | 31 +++++++++++++++++++++++++++++++ include/asm-i386/hypervisor.h | 1 + include/xen/features.h | 20 ++++++++++++++++++++ 7 files changed, 61 insertions(+)
arch/i386/mach-xen/setup-xen.c +2 -0
@@ -40,6 +40,8 @@ char * __init machine_specific_memory_sevoid __init machine_specific_arch_setup (void ) { +setup_xen_features (); +#ifdef CONFIG_ACPI if (!(xen_start_info->flags &SIF_INITDOMAIN )) {printk (KERN_INFO "ACPI in unprivileged domain disabled\n" );
drivers/Makefile +1 -0
@@ -31,6 +31,7 @@ obj-$(CONFIG_NUBUS) += nubus/ obj-$(CONFIG_NUBUS) += nubus/ obj-$(CONFIG_ATM) += atm/ obj-$(CONFIG_PPC_PMAC) += macintosh/ +obj-$(CONFIG_XEN) += xen/ obj-$(CONFIG_IDE) += ide/ obj-$(CONFIG_FC4) += fc4/ obj-$(CONFIG_SCSI) += scsi/
include/asm-i386/hypervisor.h +1 -0
@@ -40,6 +40,7 @@#include <xen/interface/xen.h> #include <xen/interface/version.h> +#include <xen/features.h> #include <asm/ptrace.h> #include <asm/page.h>
drivers/xen/Makefile new file +3 -0
@@ -0,0 +1,3 @@ + +obj-y += core/ +
drivers/xen/core/Makefile new file +3 -0
@@ -0,0 +1,3 @@ + +obj-y := features.o +
drivers/xen/core/features.c new file +31 -0
@@ -0,0 +1,31 @@ +/****************************************************************************** +* features.c +* +* Xen feature flags. +* +* Copyright (c) 2006, Ian Campbell, XenSource Inc. +*/ +#include <linux/types.h> +#include <linux/cache.h> +#include <linux/module.h> +#include <asm/hypervisor.h> +#include <xen/features.h> + +u8 xen_features[XENFEAT_NR_SUBMAPS *32 ]__read_mostly ; +EXPORT_SYMBOL_GPL (xen_features); + +void setup_xen_features (void ) +{ +#if 0 /* hypercalls come later in the series */ +struct xen_feature_info fi; +int i, j; + +for (i =0 ; i <XENFEAT_NR_SUBMAPS ; i++) { + fi.submap_idx = i; +if (HYPERVISOR_xen_version (XENVER_get_features, &fi) <0 ) +break ; +for (j=0 ; j<32 ; j++) + xen_features[i*32 +j] = !!(fi.submap &1 <<j); + } +#endif +}
include/xen/features.h new file +20 -0
@@ -0,0 +1,20 @@ +/****************************************************************************** +* features.h +* +* Query the features reported by Xen. +* +* Copyright (c) 2006, Ian Campbell +*/ + +#ifndef __ASM_XEN_FEATURES_H__ +#define __ASM_XEN_FEATURES_H__ + +#include <xen/interface/version.h> + +extern void setup_xen_features (void ); + +extern u8 xen_features[XENFEAT_NR_SUBMAPS *32 ]; + +#define xen_feature (flag) (xen_features[flag]) + +#endif /* __ASM_XEN_FEATURES_H__ */
--
RFC PATCH 11/33 Add Xen-specific memory management definitions
In reply to Chris WrightAdd extra memory management definitions used by Xen-specific code. These allow conversion between 'pseudophysical' memory addresses, which provide the illusion of a physically contiguous memory map, and underlying real machine addresses. This conversion is neceesary when interpreting and updating PTEs. Also support write protection of page mappings, which is needed to allow successful validation of page tables.
The current definitions are incomplete and only a stub implementation, allowing us to re-use existing code (drivers) which references these memory management code changes.
--- arch/i386/mach-xen/Makefile | 2 arch/i386/mach-xen/memory.c | 31 +++++++++++ include/asm-i386/mach-default/mach_page.h | 4 + include/asm-i386/mach-xen/mach_page.h | 75 +++++++++++++++++++++++++++++ include/asm-i386/page.h | 2 5 files changed, 113 insertions(+), 1 deletion(-)
arch/i386/mach-xen/Makefile +1 -1
@@ -2,6 +2,6 @@ # Makefile for the linux kernel. # -obj-y := setup.o setup-xen.o +obj-y := setup.o setup-xen.o memory.o setup-y := ../mach-default/setup.o
include/asm-i386/page.h +2 -0
@@ -82,6 +82,8 @@ typedef struct { unsigned long pgprot; }/* to align the pointer to the (next) page boundary */ #define PAGE_ALIGN (addr) (((addr)+PAGE_SIZE-1)&PAGE_MASK) +#include <mach_page.h> +/* * This handles the memory map.. We could make this a config * option, but too many people screw it up, and too few need
arch/i386/mach-xen/memory.c new file +31 -0
@@ -0,0 +1,31 @@ + +#include <linux/stddef.h> +#include <asm/desc.h> +#include <asm/hypervisor.h> +#include <asm/page.h> +#include <asm/pgtable.h> + +maddr_t arbitrary_virt_to_machine (unsigned long address) +{ +pte_t *pte =lookup_address (address); +maddr_t m; + +BUG_ON (pte ==NULL ); + + m = (maddr_t )pte_mfn (*pte) <<PAGE_SHIFT ; +return m | (address & (PAGE_SIZE -1 )); +} + +void make_lowmem_page_readonly (unsigned long address,unsigned int feature) +{ +pte_t *pte; +int rc; + +if (xen_feature (feature)) +return ; + + pte =lookup_address (address); +BUG_ON (pte ==NULL ); + rc =HYPERVISOR_update_va_mapping (address,pte_wrprotect (*pte),0 ); +BUG_ON (rc); +}
include/asm-i386/mach-default/mach_page.h new file +4 -0
@@ -0,0 +1,4 @@ +#ifndef __ASM_MACH_PAGE_H +#define __ASM_MACH_PAGE_H + +#endif /* __ASM_MACH_PAGE_H */
include/asm-i386/mach-xen/mach_page.h new file +75 -0
75 lines not on this page · raw message
--
RFC PATCH 12/33 Change __FIXADDR_TOP to leave room for the hypervisor.
In reply to Chris WrightMove the definition of __FIXADDR_TOP into a subarch include file so that it can be overridden for subarch xen -- the hypervisor needs about 64MB at the top of the address space.
--- include/asm-i386/fixmap.h | 8 +------- include/asm-i386/mach-default/mach_fixmap.h | 11 +++++++++++ include/asm-i386/mach-xen/mach_fixmap.h | 11 +++++++++++ 3 files changed, 23 insertions(+), 7 deletions(-)
include/asm-i386/fixmap.h +1 -7
@@ -13,13 +13,7 @@#ifndef _ASM_FIXMAP_H#define _ASM_FIXMAP_H - -/* used by vmalloc.c, vsyscall.lds.S. -* -* Leave one empty page between vmalloc'ed areas and -* the start of the fixmap. -*/ -#define __FIXADDR_TOP 0xfffff000 +#include <mach_fixmap.h> #ifndef __ASSEMBLY__#include <linux/kernel.h>
include/asm-i386/mach-default/mach_fixmap.h new file +11 -0
@@ -0,0 +1,11 @@ +#ifndef __ASM_MACH_FIXMAP_H +#define __ASM_MACH_FIXMAP_H + +/* used by vmalloc.c, vsyscall.lds.S. +* +* Leave one empty page between vmalloc'ed areas and +* the start of the fixmap. +*/ +#define __FIXADDR_TOP 0xfffff000 + +#endif /* __ASM_MACH_FIXMAP_H */
include/asm-i386/mach-xen/mach_fixmap.h new file +13 -0
@@ -0,0 +1,13 @@ +#ifndef __ASM_MACH_FIXMAP_H +#define __ASM_MACH_FIXMAP_H + +#include <xen/interface/xen.h> + +/* used by vmalloc.c, vsyscall.lds.S. +* +* Leave one empty page between vmalloc'ed areas and +* the start of the fixmap. +*/ +#define __FIXADDR_TOP (HYPERVISOR_VIRT_START - 2 * PAGE_SIZE) + +#endif /* __ASM_MACH_FIXMAP_H */
--
RFC PATCH 13/33 Add a new head.S start-of-day file for booting on Xen.
In reply to Chris WrightWhen running on Xen, the kernel is started with paging enabled. Also don't check for cpu features which are present on all cpus supported by Xen.
Don't define segments which are not supported when running on Xen.
Define the __xen_guest section which exports information about the kernel to the domain builder.
--- arch/i386/Makefile | 7 +- arch/i386/kernel/head-cpu.S | 86 +++++++++++++++++++++++++++++ arch/i386/kernel/head.S | 71 ++---------------------- arch/i386/mach-xen/Makefile | 2 arch/i386/mach-xen/head.S | 127 +++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 226 insertions(+), 67 deletions(-)
arch/i386/Makefile +5 -2
@@ -48,6 +48,9 @@ CFLAGS += $(shell if [ $(call cc-vers CFLAGS += $(cflags-y) +# Default subarch head files +head-y := arch/i386/kernel/head.o arch/i386/kernel/init_task.o + # Default subarch .c files mcore-y := mach-default
@@ -74,6 +77,8 @@ mcore-$(CONFIG_X86_SUMMIT) := mach-defa # Xen subarch support mflags-$(CONFIG_X86_XEN) := -Iinclude/asm-i386/mach-xen mcore-$(CONFIG_X86_XEN) := mach-xen +head-$(CONFIG_X86_XEN) := arch/i386/mach-xen/head.o \ + arch/i386/kernel/init_task.o # generic subarchitecture mflags-$(CONFIG_X86_GENERICARCH) := -Iinclude/asm-i386/mach-generic
@@ -87,8 +92,6 @@ core-$(CONFIG_X86_ES7000) := arch/i386/m # default subarch .h files mflags-y += -Iinclude/asm-i386/mach-default - -head-y := arch/i386/kernel/head.o arch/i386/kernel/init_task.o libs-y += arch/i386/lib/ core-y += arch/i386/kernel/ \
arch/i386/kernel/head.S +6 -65
90 lines not on this page · raw message
arch/i386/mach-xen/Makefile +2 -0
@@ -2,6 +2,8 @@ # Makefile for the linux kernel. # +extra-y := head.o + obj-y := setup.o setup-xen.o memory.o setup-y := ../mach-default/setup.o
arch/i386/kernel/head-cpu.S new file +86 -0
86 lines not on this page · raw message
arch/i386/mach-xen/head.S new file +127 -0
127 lines not on this page · raw message
--
Re: [RFC PATCH 13/33] Add a new head.S start-of-day file for booting on Xen.
Quoted 4 lines · Chris Wright
plain text document attachment (
i386-head.S)
When running on Xen, the kernel is started with paging enabled. Also don't check for cpu features which are present on all cpus supported by Xen.
Hi,
I didn't see much of this last sentence in the actual patch, which is good, because I just don't see any reason to do that at all; if they features are there anyway, why not preserve their detection, it's not hurting and it means you need to be less different "just because"...
Greetings,
Arjan van de Ven
Re: [RFC PATCH 13/33] Add a new head.S start-of-day file for booting on Xen.
Quoted 5 lines · Arjan van de Ven
I didn't see much of this last sentence in the actual patch, which is good, because I just don't see any reason to do that at all; if they features are there anyway, why not preserve their detection, it's not hurting and it means you need to be less different "just because"...
mach-xen/head.S is simpler than kernel/head.S, partly because it doesn't contain the CPU identification and feature tests (cpuid is assumed, though ironically, we don't actually use it).
J
RFC PATCH 14/33 subarch support for controlling interrupt delivery
In reply to Chris WrightAbstract the code that controls interrupt delivery, and add a separate subarch implementation for Xen that manipulates a shared-memory event delivery mask.
--- arch/i386/mach-xen/Makefile | 2 arch/i386/mach-xen/interrupt.c | 67 +++++++++++++++++++++++++ include/asm-i386/irqflags.h | 61 +--------------------- include/asm-i386/mach-default/mach_irqflags.h | 64 +++++++++++++++++++++++ include/asm-i386/mach-default/mach_system.h | 4 + include/asm-i386/mach-xen/mach_irqflags.h | 29 ++++++++++ include/asm-i386/mach-xen/mach_system.h | 5 + 7 files changed, 174 insertions(+), 58 deletions(-)
arch/i386/mach-xen/Makefile +1 -1
@@ -4,6 +4,6 @@ extra-y := head.o -obj-y := setup.o setup-xen.o memory.o +obj-y := setup.o setup-xen.o memory.o interrupt.o setup-y := ../mach-default/setup.o
include/asm-i386/irqflags.h +4 -57
81 lines not on this page · raw message
arch/i386/mach-xen/interrupt.c new file +67 -0
67 lines not on this page · raw message
include/asm-i386/mach-default/mach_irqflags.h new file +64 -0
64 lines not on this page · raw message
include/asm-i386/mach-default/mach_system.h new file +4 -0
@@ -0,0 +1,4 @@ +#ifndef __ASM_MACH_SYSTEM_H +#define __ASM_MACH_SYSTEM_H + +#endif /* __ASM_MACH_SYSTEM_H */
include/asm-i386/mach-xen/mach_irqflags.h new file +29 -0
@@ -0,0 +1,29 @@ +#ifndef ASM_MACH_IRQFLAGS_H +#define ASM_MACH_IRQFLAGS_H + +#ifndef __ASSEMBLY__ + +#ifdef CONFIG_SMP +#define __vcpu_id smp_processor_id() +#else +#define __vcpu_id 0 +#endif + +/* interrupt control.. */ + +unsigned long __raw_local_save_flags (void ); +void raw_local_irq_restore (unsigned long flags); +void raw_local_irq_disable (void ); +void raw_local_irq_enable (void ); + +#define raw_safe_halt () do { } while(0) +#define halt () do { } while(0) + +static inline int raw_irqs_disabled_flags (unsigned long flags) +{ +return flags !=0 ; +} + +#endif /* __ASSEMBLY__ */ + +#endif /* ASM_MACH_IRQFLAGS_H */
include/asm-i386/mach-xen/mach_system.h new file +5 -0
@@ -0,0 +1,5 @@ +#ifndef __ASM_MACH_SYSTEM_H +#define __ASM_MACH_SYSTEM_H + + +#endif /* __ASM_MACH_SYSTEM_H */
--
RFC PATCH 15/33 move segment checks to subarch
In reply to Chris WrightWe allow for the fact that the guest kernel may not run in ring 0. This requires some abstraction in a few places when setting %cs or checking privilege level (user vs kernel).
--- arch/i386/kernel/process.c | 2 +- arch/i386/mm/fault.c | 8 +++++--- include/asm-i386/mach-default/mach_segment.h | 8 ++++++++ include/asm-i386/mach-xen/mach_segment.h | 9 +++++++++ include/asm-i386/mach-xen/mach_system.h | 1 + include/asm-i386/ptrace.h | 6 ++++-- include/asm-i386/segment.h | 2 ++ include/asm-i386/system.h | 1 + 8 files changed, 31 insertions(+), 6 deletions(-)
arch/i386/kernel/process.c +1 -1
@@ -346,7 +346,7 @@ int kernel_thread(int (*fn)(void *), voi regs.xes = __USER_DS; regs.orig_eax =-1 ; regs.eip = (unsigned long ) kernel_thread_helper; - regs.xcs = __KERNEL_CS; + regs.xcs =get_kernel_cs (); regs.eflags =X86_EFLAGS_IF |X86_EFLAGS_SF |X86_EFLAGS_PF |0x2 ;/* Ok, create the new process.. */
arch/i386/mm/fault.c +5 -3
@@ -28,6 +28,8 @@#include <asm/desc.h> #include <asm/kdebug.h> +#include <mach_segment.h> +extern void die (const char *,struct pt_regs *,long );#ifdef CONFIG_KPROBES
@@ -119,10 +121,10 @@ static inline unsigned long get_segment_ }/* The standard kernel/user address space limit. */ - *eip_limit = (seg &3 ) ?USER_DS .seg :KERNEL_DS .seg; + *eip_limit = (seg &USER_MODE_MASK ) ?USER_DS .seg :KERNEL_DS .seg;/* By far the most common cases. */ -if (likely (seg == __USER_CS || seg == __KERNEL_CS)) +if (likely (seg == __USER_CS || seg ==get_kernel_cs ()))return eip;/* Check the segment exists, is within the current LDT/GDT size,
@@ -437,7 +439,7 @@ good_area:switch (error_code &3 ) {default :/* 3: write, present */ #ifdef TEST_VERIFY_AREA -if (regs->cs ==KERNEL_CS ) +if (regs->cs ==get_kernel_cs ())printk ("WP fault at %08lx\n" , regs->eip);#endif /* fall through */
include/asm-i386/ptrace.h +4 -2
@@ -1,5 +1,7 @@#ifndef _I386_PTRACE_H#define _I386_PTRACE_H + +#include <mach_segment.h> #define EBX 0#define ECX 1
@@ -73,11 +75,11 @@ extern void send_sigtrap(struct task_str*/ static inline int user_mode (struct pt_regs *regs) { -return (regs->xcs &3 ) !=0 ; +return (regs->xcs &USER_MODE_MASK ) !=0 ; }static inline int user_mode_vm (struct pt_regs *regs) { -return ((regs->xcs &3 ) | (regs->eflags &VM_MASK )) !=0 ; +return ((regs->xcs &USER_MODE_MASK ) | (regs->eflags &VM_MASK )) !=0 ; }#define instruction_pointer (regs) ((regs)->eip)#if defined(CONFIG_SMP ) && defined(CONFIG_FRAME_POINTER )
include/asm-i386/segment.h +2 -0
@@ -1,5 +1,7 @@#ifndef _ASM_SEGMENT_H#define _ASM_SEGMENT_H + +#include <mach_segment.h> /* * The layout of the per-CPU GDT under Linux:
include/asm-i386/system.h +1 -0
@@ -487,6 +487,7 @@ static inline unsigned long long __cmpxc#endif #include <linux/irqflags.h> +#include <mach_system.h> /* * disable hlt during certain critical i/o operations
include/asm-i386/mach-xen/mach_system.h +1 -0
@@ -1,5 +1,6 @@#ifndef __ASM_MACH_SYSTEM_H#define __ASM_MACH_SYSTEM_H +#include <asm/hypervisor.h> #endif /* __ASM_MACH_SYSTEM_H */
include/asm-i386/mach-default/mach_segment.h new file +8 -0
@@ -0,0 +1,8 @@ +#ifndef __ASM_MACH_SEGMENT_H +#define __ASM_MACH_SEGMENT_H + +#define USER_MODE_MASK 3 + +#define get_kernel_cs () __KERNEL_CS + +#endif /* __ASM_MACH_SEGMENT_H */
include/asm-i386/mach-xen/mach_segment.h new file +9 -0
@@ -0,0 +1,9 @@ +#ifndef __ASM_MACH_SEGMENT_H +#define __ASM_MACH_SEGMENT_H + +#define USER_MODE_MASK 2 + +#define get_kernel_cs () \ + (__KERNEL_CS + (xen_feature(XENFEAT_supervisor_mode_kernel) ? 0 : 1)) + +#endif /* __ASM_MACH_SEGMENT_H */
--
Re: [RFC PATCH 15/33] move segment checks to subarch
arch/i386/kernel/process.c int kernel_thread(int (*fn)(void *), voi · line 349
Quoted 7 lines · Chris Wright
plain text document attachment (i386-segments)
We allow for the fact that the guest kernel may not run in ring 0. This requires some abstraction in a few places when setting %cs or checking privilege level (user vs kernel).arch/i386/kernel/process.c +1
-1- regs.xcs = __KERNEL_CS; + regs.xcs =get_kernel_cs ();
Hi,
wouldn't this patch be simpler if __KERNEL_CS just became the macro that currently is get_kernel_cs() for the XEN case? then code like this doesn't need changing at all...
Greetings,
Arjan van de Ven
Re: [RFC PATCH 15/33] move segment checks to subarch
Quoted 15 lines · Arjan van de Ven
arch/i386/kernel/process.c int kernel_thread(int (*fn)(void *), voi · line 349
Quoted 10 lines · Chris Wright
plain text document attachment (i386-segments)
We allow for the fact that the guest kernel may not run in ring 0. This requires some abstraction in a few places when setting %cs or checking privilege level (user vs kernel).arch/i386/kernel/process.c +1
-1- regs.xcs = __KERNEL_CS; + regs.xcs =get_kernel_cs ();Hi,
Quoted 4 lines
wouldn't this patch be simpler if
__KERNEL_CSjust became the macro that currently isget_kernel_cs()for the XEN case? then code like this doesn't need changing at all...
The tradeoff is that then you can't use __KERNEL_CS is assembler code, and it is used in entry.S to detect NMI / debug trap workarounds - which don't actually need to be paravirtualized, as it is easier to hide the nasty cases which cause those side effects.
Zach
Re: [RFC PATCH 15/33] move segment checks to subarch
In reply to Chris WrightQuoted 4 lines · Chris Wright
plain text document attachment (i386-segments)
We allow for the fact that the guest kernel may not run in ring 0. This requires some abstraction in a few places when setting %cs or checking privilege level (user vs kernel).
Zach had an alternate patch for this, which didn't assume the kernel ran in a compile-time known ring, but is otherwise very similar. I've put it below for discussion (but Zach now tells me the asm parts are not
required: Zach, can you mod this patch and comment?).
Your patch #16 finishes the job you started here, by doing the mods to entry.S. I think it's cleaner to have all this in one patch (and it can go in almost immediately AFAICT).
Comments?
Rusty.
Name: Kernel Ring Cleanups
Status: Booted on 2.6.18-rc1
This is Zach's patch to clean up assumptions about the kernel running in ring 0 (which it doesn't when running paravirtualized).
1) Remove the hardcoded 3 and introduce #define SEGMENT_RPL_MASK 3
2) Add a get_kernel_rpl() function
3) Create COMPARE_SEGMENT_STACK and COMPARE_SEGMENT_REG macros which can mask out the bottom two bits (RPL) when comparing for paravirtualization.
arch/i386/kernel/entry.S +14 -22
@@ -228,9 +228,11 @@ ret_from_intr: GET_THREAD_INFO(%ebp) check_userspace: movl EFLAGS(%esp), %eax # mix EFLAGS and CS + andl $VM_MASK, %eax movb CS(%esp), %al - testl $(VM_MASK | 3), %eax - jz resume_kernel + andb $SEGMENT_RPL_MASK, %al + cmpl $SEGMENT_RPL_MASK, %eax + jb resume_kernel # returning to kernel or vm86-space ENTRY(resume_userspace) cli # make sure we don't miss an interrupt # setting need_resched or sigpending
@@ -517,22 +519,16 @@ syscall_badsys: /* put ESP to the proper location */ \ movl %eax, %esp; #define UNWIND_ESPFIX_STACK \ - pushl %eax; \ - CFI_ADJUST_CFA_OFFSET 4; \ - movl %ss, %eax; \ - /* see if on 16bit stack */ \ - cmpw $__ESPFIX_SS, %ax; \ - je 28f; \ -27: popl %eax; \ - CFI_ADJUST_CFA_OFFSET -4; \ + COMPARE_SEGMENT_REG(__ESPFIX_SS, %ss); \ + jne 28f; \ .section .fixup,"ax"; \ -28: movl $__KERNEL_DS, %eax; \ + movl $__KERNEL_DS, %eax; \ movl %eax, %ds; \ movl %eax, %es; \ /* switch to 32bit stack */ \ FIXUP_ESPFIX_STACK; \ - jmp 27b; \ -.previous +.previous \ +28: ; /* * Build the entry stubs and pointer table with
@@ -629,6 +625,7 @@ error_code: popl %ecx CFI_ADJUST_CFA_OFFSET -4 /*CFI_REGISTER es, ecx*/ + movl EAX(%esp), %eax movl ES(%esp), %edi # get the function address movl ORIG_EAX(%esp), %edx # get the error code movl %eax, ORIG_EAX(%esp)
@@ -694,12 +691,12 @@ device_not_available_emulate: * the instruction that would have done it for sysenter. */ #define FIX_STACK(offset, ok, label) \ - cmpw $__KERNEL_CS,4(%esp); \ + COMPARE_SEGMENT_STACK(__KERNEL_CS, 4); \ jne ok; \ label: \ movl TSS_sysenter_esp0+offset(%esp),%esp; \ pushfl; \ - pushl $__KERNEL_CS; \ + push %cs; \ pushl $sysenter_past_esp KPROBE_ENTRY(debug)
@@ -727,12 +724,7 @@ debug_stack_correct: */ ENTRY(nmi) RING0_INT_FRAME - pushl %eax - CFI_ADJUST_CFA_OFFSET 4 - movl %ss, %eax - cmpw $__ESPFIX_SS, %ax - popl %eax - CFI_ADJUST_CFA_OFFSET -4 + COMPARE_SEGMENT_REG(__ESPFIX_SS, %ss) je nmi_16bit_stack cmpl $sysenter_entry,(%esp) je nmi_stack_fixup
@@ -763,7 +755,7 @@ nmi_stack_fixup: FIX_STACK(12,nmi_stack_correct, 1) jmp nmi_stack_correct nmi_debug_stack_check: - cmpw $__KERNEL_CS,16(%esp) + COMPARE_SEGMENT_STACK(__KERNEL_CS, 16) jne nmi_stack_correct cmpl $debug,(%esp) jb nmi_stack_correct
arch/i386/kernel/process.c +1 -1
@@ -346,7 +346,7 @@ int kernel_thread(int (*fn)(void *), voi regs.xes = __USER_DS; regs.orig_eax =-1 ; regs.eip = (unsigned long ) kernel_thread_helper; - regs.xcs = __KERNEL_CS; + regs.xcs = __KERNEL_CS |get_kernel_rpl (); regs.eflags =X86_EFLAGS_IF |X86_EFLAGS_SF |X86_EFLAGS_PF |0x2 ;/* Ok, create the new process.. */
arch/i386/kernel/traps.c +2 -2
@@ -1034,10 +1034,10 @@ fastcall void setup_x86_bogus_stack(unsimemcpy ((void *)(stack_bot + iret_frame16_off), ®s->eip,20 );/* fill in the switch pointers */ switch16_ptr[0 ] = (regs->esp &0xffff0000 ) | iret_frame16_off; - switch16_ptr[1 ] = __ESPFIX_SS; + switch16_ptr[1 ] = __ESPFIX_SS |get_kernel_rpl (); switch32_ptr[0 ] = (unsigned long )stk +sizeof (struct pt_regs ) +8 -CPU_16BIT_STACK_SIZE ; - switch32_ptr[1 ] = __KERNEL_DS; + switch32_ptr[1 ] = __KERNEL_DS |get_kernel_rpl (); } fastcall unsigned char * fixup_x86_bogus_stack(unsigned short sp)
include/asm-i386/ptrace.h +3 -2
@@ -60,6 +60,7 @@ struct pt_regs {#ifdef __KERNEL__#include <asm/vm86.h> +#include <asm/segment.h> struct task_struct ;extern void send_sigtrap (struct task_struct *tsk,struct pt_regs *regs,int error_code);
@@ -73,11 +74,11 @@ extern void send_sigtrap(struct task_str*/ static inline int user_mode (struct pt_regs *regs) { -return (regs->xcs &3 ) !=0 ; +return (regs->xcs &SEGMENT_RPL_MASK ) ==3 ; }static inline int user_mode_vm (struct pt_regs *regs) { -return ((regs->xcs &3 ) | (regs->eflags &VM_MASK )) !=0 ; +return (((regs->xcs &SEGMENT_RPL_MASK ) | (regs->eflags &VM_MASK )) >=3 ); }#define instruction_pointer (regs) ((regs)->eip)#if defined(CONFIG_SMP ) && defined(CONFIG_FRAME_POINTER )
include/asm-i386/segment.h +16 -0
@@ -112,4 +112,20 @@*/ #define IDT_ENTRIES 256 +/* Bottom three bits of xcs give the ring privilege level */ +#define SEGMENT_RPL_MASK 0x3 + +#define get_kernel_rpl () 0 + +#define COMPARE_SEGMENT_STACK (segment, offset) \ + cmpw $segment, offset(%esp); + +#define COMPARE_SEGMENT_REG (segment, reg) \ + pushl %eax; \ + CFI_ADJUST_CFA_OFFSET 4; \ + mov reg, %eax; \ + cmpw $segment,%ax; \ + popl %eax; \ + CFI_ADJUST_CFA_OFFSET -4 +#endif
Signature · 1 line
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law
Re: [RFC PATCH 15/33] move segment checks to subarch
Quoted 10 lines · * Rusty Russell ([email protected]) wrote:
Quoted 4 lines · Chris Wright
plain text document attachment (i386-segments)
We allow for the fact that the guest kernel may not run in ring 0. This requires some abstraction in a few places when setting %cs or checking privilege level (user vs kernel).Zach had an alternate patch for this, which didn't assume the kernel ran in a compile-time known ring, but is otherwise very similar. I've put it below for discussion (but Zach now tells me the asm parts are not
required: Zach, can you mod this patch and comment?).
This patch also doesn't have a compile time known ring, it's using get_kernel_cs() because the Xen method for booting native is dynamic and would resolve to ring 0 in that case (XENFEAT_supervisor_mode_kernel).
Quoted 3 lines
Your patch #16 finishes the job you started here, by doing the mods to
entry.S. I think it's cleaner to have all this in one patch (and it can go in almost immediately AFAICT).
I totally agree. I actually started doing rearranging the patch order last night and that was one of the bits that got consolidated. It was getting light this morning and I gave up since I was wasting lots of time fixing patch rejects from all the rearranging, and needed a bit of sleep before KS;-)
Quoted 6 lines
Comments?
Rusty.Name: Kernel Ring Cleanups
Status: Booted on 2.6.18-rc1Signed-off-by Rusty Russell <[email protected]>
Looks fine to me, granted I haven't tried to boot it yet. I'll double check and report back.
thanks,
-chris
Re: [Xen-devel] Re: [RFC PATCH 15/33] move segment checks to subarch
Quoted 7 lines · Chris Wright
Quoted 6 lines · * Rusty Russell ([email protected]) wrote:
Quoted 4 lines · Chris Wright
plain text document attachment (i386-segments)
We allow for the fact that the guest kernel may not run in ring 0. This requires some abstraction in a few places when setting %cs or checking privilege level (user vs kernel).
Quoted 8 lines
Quoted 4 lines
Zach had an alternate patch for this, which didn't assume the kernel ran in a compile-time known ring, but is otherwise very similar. I've put it below for discussion (but Zach now tells me the asm parts are not
required: Zach, can you mod this patch and comment?).This patch also doesn't have a compile time known ring, it's using
get_kernel_cs()because the Xen method for booting native is dynamic and would resolve to ring 0 in that case (XENFEAT_supervisor_mode_kernel).
I was referring to the different ways the two patches figure out whether we're in user mode:
Yours:
static inline int user_mode(struct pt_regs *regs)
{
return (regs->xcs & USER_MODE_MASK) != 0;}
Where you have for native:
#define USER_MODE_MASK 3 vs Xen: #define USER_MODE_MASK 2
Zach's patch does this:
static inline int user_mode(struct pt_regs *regs)
{
return (regs->xcs & SEGMENT_RPL_MASK) == 3;}
I'm no x86pert, but the latter seems more generic to me (user mode is ring 3, vs. usermode is anything >= 2). Perhaps they are in fact equivalent?
Thanks!
Rusty.
Signature · 1 line
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law
RFC PATCH 16/33 Add support for Xen to entry.S.
In reply to Chris Wright- change cli/sti
- change test for user mode return to work for kernel mode in ring1
- check hypervisor saved event mask on return from exception
- add entry points for the hypervisor upcall handlers
- avoid math emulation check when running on Xen
- add nmi handler for running on Xen
--- arch/i386/kernel/asm-offsets.c | 26 +++++++ arch/i386/kernel/entry.S | 141 ++++++++++++++++++++++++++++++++++++----- arch/i386/mach-xen/setup-xen.c | 19 +++++ drivers/xen/core/features.c | 2 4 files changed, 169 insertions(+), 19 deletions(-)
arch/i386/kernel/entry.S +124 -17
@@ -76,8 +76,39 @@ NT_MASK = 0x00004000 NT_MASK = 0x00004000 VM_MASK = 0x00020000 +#ifndef CONFIG_XEN +#define DISABLE_INTERRUPTS cli +#define ENABLE_INTERRUPTS sti +#else +#include <xen/interface/xen.h> + +EVENT_MASK = 0x2E + +/* Offsets into shared_info_t. */ +#define evtchn_upcall_pending /* 0 */ +#define evtchn_upcall_mask 1 + +#ifdef CONFIG_SMP +/* Set %esi to point to the appropriate vcpu structure */ +#define GET_VCPU_INFO movl TI_cpu(%ebp),%esi ; \ + shl $SIZEOF_VCPU_INFO_SHIFT,%esi ; \ + addl HYPERVISOR_shared_info,%esi +#else +#define GET_VCPU_INFO movl HYPERVISOR_shared_info,%esi +#endif + +/* The following end up using/clobbering %esi, because of GET_VCPU_INFO */ +#define __DISABLE_INTERRUPTS movb $1,evtchn_upcall_mask(%esi) +#define DISABLE_INTERRUPTS GET_VCPU_INFO ; \ + __DISABLE_INTERRUPTS +#define ENABLE_INTERRUPTS GET_VCPU_INFO ; \ + movb $0,evtchn_upcall_mask(%esi) +#define __TEST_PENDING testb $0xFF,evtchn_upcall_pending(%esi) +#endif + + #ifdef CONFIG_PREEMPT -#define preempt_stop cli; TRACE_IRQS_OFF +#define preempt_stop DISABLE_INTERRUPTS; TRACE_IRQS_OFF #else #define preempt_stop #define resume_kernel restore_nocheck
@@ -229,10 +260,10 @@ check_userspace: check_userspace: movl EFLAGS(%esp), %eax # mix EFLAGS and CS movb CS(%esp), %al - testl $(VM_MASK | 3), %eax + testl $(VM_MASK | USER_MODE_MASK), %eax jz resume_kernel ENTRY(resume_userspace) - cli # make sure we don't miss an interrupt + DISABLE_INTERRUPTS # make sure we don't miss an interrupt # setting need_resched or sigpending # between sampling and the iret movl TI_flags(%ebp), %ecx
@@ -243,7 +274,7 @@ ENTRY(resume_userspace) #ifdef CONFIG_PREEMPT ENTRY(resume_kernel) - cli + DISABLE_INTERRUPTS cmpl $0,TI_preempt_count(%ebp) # non-zero preempt_count ? jnz restore_nocheck need_resched:
@@ -271,7 +302,7 @@ sysenter_past_esp: * No need to follow this irqs on/off section: the syscall * disabled irqs and here we enable it straight after entry: */ - sti + ENABLE_INTERRUPTS pushl $(__USER_DS) CFI_ADJUST_CFA_OFFSET 4 /*CFI_REL_OFFSET ss, 0*/
@@ -316,7 +347,7 @@ 1: movl (%ebp),%ebp jae syscall_badsys call *sys_call_table(,%eax,4) movl %eax,EAX(%esp) - cli + DISABLE_INTERRUPTS TRACE_IRQS_OFF movl TI_flags(%ebp), %ecx testw $_TIF_ALLWORK_MASK, %cx
@@ -326,7 +357,7 @@ 1: movl (%ebp),%ebp movl OLDESP(%esp), %ecx xorl %ebp,%ebp TRACE_IRQS_ON - sti + ENABLE_INTERRUPTS sysexit CFI_ENDPROC
@@ -352,7 +383,7 @@ syscall_call: call *sys_call_table(,%eax,4) movl %eax,EAX(%esp) # store the return value syscall_exit: - cli # make sure we don't miss an interrupt + DISABLE_INTERRUPTS # make sure we don't miss an interrupt # setting need_resched or sigpending # between sampling and the iret TRACE_IRQS_OFF
@@ -361,6 +392,7 @@ syscall_exit: jne syscall_exit_work restore_all: +#ifndef CONFIG_XEN movl EFLAGS(%esp), %eax # mix EFLAGS, SS and CS # Warning: OLDSS(%esp) contains the wrong/random values if we # are returning to the kernel.
@@ -372,6 +404,27 @@ restore_all: CFI_REMEMBER_STATE je ldt_ss # returning to user-space with LDT SS restore_nocheck: +#else +restore_nocheck: + /* Return back to the hypervisor if we're not returning to user-mode */ + movl EFLAGS(%esp), %eax # mix EFLAGS and CS + movb CS(%esp), %al + andl $(VM_MASK | 3), %eax + cmpl $3, %eax + jne hypervisor_iret + ENABLE_INTERRUPTS + __TEST_PENDING + jz restore_regs_and_iret + __DISABLE_INTERRUPTS + jmp do_hypervisor_callback +hypervisor_iret: + TRACE_IRQS_IRET + RESTORE_REGS + addl $4, %esp + CFI_ADJUST_CFA_OFFSET -4 + jmp hypercall_page + (__HYPERVISOR_iret * 32) +#endif /* CONFIG_XEN */ +restore_regs_and_iret: TRACE_IRQS_IRET restore_nocheck_notrace: RESTORE_REGS
@@ -380,8 +433,10 @@ 1: iret 1: iret .section .fixup,"ax" iret_exc: +#ifndef CONFIG_XEN TRACE_IRQS_ON - sti + ENABLE_INTERRUPTS +#endif pushl $0 # no error code pushl $do_iret_error jmp error_code
@@ -392,6 +447,7 @@ iret_exc: .previous CFI_RESTORE_STATE +#ifndef CONFIG_XEN ldt_ss: larl OLDSS(%esp), %eax jnz restore_nocheck
@@ -405,7 +461,7 @@ ldt_ss: * dosemu and wine happy. */ subl $8, %esp # reserve space for switch16 pointer CFI_ADJUST_CFA_OFFSET 8 - cli + DISABLE_INTERRUPTS TRACE_IRQS_OFF movl %esp, %eax /* Set up the 16bit stack frame with switch32 pointer on top,
@@ -420,6 +476,7 @@ 1: iret .align 4 .long 1b,iret_exc .previous +#endif CFI_ENDPROC # perform work that needs to be done immediately before resumption
@@ -430,7 +487,7 @@ work_pending: jz work_notifysig work_resched: call schedule - cli # make sure we don't miss an interrupt + DISABLE_INTERRUPTS # make sure we don't miss an interrupt # setting need_resched or sigpending # between sampling and the iret TRACE_IRQS_OFF
@@ -486,7 +543,7 @@ syscall_exit_work: testb $(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SINGLESTEP), %cl jz work_pending TRACE_IRQS_ON - sti # could let do_syscall_trace() call + ENABLE_INTERRUPTS # could let do_syscall_trace() call # schedule() instead movl %esp, %eax movl $1, %edx
@@ -508,6 +565,7 @@ syscall_badsys: jmp resume_userspace CFI_ENDPROC +#ifndef CONFIG_XEN #define FIXUP_ESPFIX_STACK \ movl %esp, %eax; \ /* switch to 32bit stack using the pointer on top of 16bit stack */ \
@@ -586,6 +644,9 @@ ENTRY(name) \ /* The include is where all of the SMP etc. interrupts come from */ #include "entry_arch.h" +#else +#define UNWIND_ESPFIX_STACK +#endif ENTRY(divide_error) RING0_INT_FRAME
@@ -642,6 +703,44 @@ error_code: jmp ret_from_exception CFI_ENDPROC +#ifdef CONFIG_XEN +ENTRY(hypervisor_callback) + pushl %eax + SAVE_ALL +do_hypervisor_callback: + push %esp + call evtchn_do_upcall + add $4,%esp + jmp ret_from_intr + +# Hypervisor uses this for application faults while it executes. +ENTRY(failsafe_callback) +1: popl %ds +2: popl %es +3: popl %fs +4: popl %gs + subl $4,%esp + SAVE_ALL + jmp ret_from_exception +.section .fixup,"ax"; \ +6: movl $0,(%esp); \ + jmp 1b; \ +7: movl $0,(%esp); \ + jmp 2b; \ +8: movl $0,(%esp); \ + jmp 3b; \ +9: movl $0,(%esp); \ + jmp 4b; \ +.previous; \ +.section __ex_table,"a";\ + .align 4; \ + .long 1b,6b; \ + .long 2b,7b; \ + .long 3b,8b; \ + .long 4b,9b; \ +.previous +#endif + ENTRY(coprocessor_error) RING0_INT_FRAME pushl $0
@@ -665,18 +764,20 @@ ENTRY(device_not_available) pushl $-1 # mark this as an int CFI_ADJUST_CFA_OFFSET 4 SAVE_ALL +#ifndef CONFIG_XEN movl %cr0, %eax testl $0x4, %eax # EM (math emulation bit) - jne device_not_available_emulate - preempt_stop - call math_state_restore - jmp ret_from_exception -device_not_available_emulate: + je device_available_emulate pushl $0 # temporary storage for ORIG_EIP CFI_ADJUST_CFA_OFFSET 4 call math_emulate addl $4, %esp CFI_ADJUST_CFA_OFFSET -4 + jmp ret_from_exception +device_available_emulate: +#endif + preempt_stop + call math_state_restore jmp ret_from_exception CFI_ENDPROC
@@ -717,6 +818,8 @@ debug_stack_correct: jmp ret_from_exception CFI_ENDPROC .previous .text + +#ifndef CONFIG_XEN /* * NMI is doubly nasty. It can happen _while_ we're handling * a debug fault, and the debug fault hasn't yet been able to
@@ -801,6 +904,10 @@ 1: iret .align 4 .long 1b,iret_exc .previous +#else +ENTRY(nmi) + jmp restore_all +#endif KPROBE_ENTRY(int3) RING0_INT_FRAME
arch/i386/mach-xen/setup-xen.c +19 -0
@@ -40,7 +40,26 @@ char * __init machine_specific_memory_sevoid __init machine_specific_arch_setup (void ) { +struct physdev_op op; +setup_xen_features (); + + HYPERVISOR_shared_info = + (struct shared_info *)__va (xen_start_info->shared_info); +memset (empty_zero_page,0 ,sizeof (empty_zero_page)); + +HYPERVISOR_vm_assist (VMASST_CMD_enable, VMASST_TYPE_4gb_segments); + +HYPERVISOR_set_callbacks ( + __KERNEL_CS, (unsigned long )hypervisor_callback, + __KERNEL_CS, (unsigned long )failsafe_callback); + + init_pg_tables_end =__pa (xen_start_info->pt_base) + +PFN_PHYS (xen_start_info->nr_pt_frames); + + op.cmd =PHYSDEVOP_SET_IOPL ; + op.u.set_iopl.iopl =1 ; +HYPERVISOR_physdev_op (&op);#ifdef CONFIG_ACPI if (!(xen_start_info->flags &SIF_INITDOMAIN )) {
drivers/xen/core/features.c +0 -2
@@ -16,7 +16,6 @@ EXPORT_SYMBOL_GPL(xen_features);void setup_xen_features (void ) { -#if 0 /* hypercalls come later in the series */ struct xen_feature_info fi;int i, j;
@@ -27,5 +26,4 @@ void setup_xen_features(void)for (j=0 ; j<32 ; j++) xen_features[i*32 +j] = !!(fi.submap &1 <<j); } -#endif }
arch/i386/kernel/asm-offsets.c +26 -0
@@ -15,6 +15,7 @@#include <asm/processor.h> #include <asm/thread_info.h> #include <asm/elf.h> +#include <xen/interface/xen.h> #define DEFINE (sym, val) \ asm volatile("\n->" #sym " %0 " #val : : "i" (val))
@@ -23,6 +24,27 @@#define OFFSET (sym, str, mem) \ DEFINE(sym, offsetof(struct str, mem)); + +/* return log2(const) computed at compile time */ +extern inline int get_shift (const int x) +{ +extern int __bad_size_in_get_shift (void ); +if (__builtin_constant_p (x)) { +switch (x) { +case 1 :return 0 ; +case 2 :return 1 ; +case 4 :return 2 ; +case 8 :return 3 ; +case 16 :return 4 ; +case 32 :return 5 ; +case 64 :return 6 ; +case 128 :return 7 ; +case 256 :return 8 ; + } + } + +return __bad_size_in_get_shift (); +}void foo (void ) {
@@ -74,4 +96,8 @@ void foo(void)DEFINE (VDSO_PRELINK ,VDSO_PRELINK );OFFSET (crypto_tfm_ctx_offset, crypto_tfm, __crt_ctx); + +BLANK (); +DEFINE (SIZEOF_VCPU_INFO ,sizeof (struct vcpu_info )); +DEFINE (SIZEOF_VCPU_INFO_SHIFT ,get_shift (sizeof (struct vcpu_info ))); }
--
Re: [RFC PATCH 16/33] Add support for Xen to entry.S.
Quoted 7 lines · Chris Wright
arch/i386/kernel/entry.S +1
-4+#ifndef CONFIG_XEN movl %cr0, %eax testl $0x4, %eax # EM (math emulation bit) - jne device_not_available_emulate - preempt_stop - call math_state_restore - jmp ret_from_exception
arch/i386/kernel/entry.S ENTRY(device_not_available) · line 778
Quoted 10 lines
arch/i386/kernel/entry.S +4
-1-device_not_available_emulate: + je device_available_emulate pushl $0 # temporary storage for ORIG_EIP CFI_ADJUST_CFA_OFFSET 4 call math_emulate addl $4, %esp CFI_ADJUST_CFA_OFFSET -4 + jmp ret_from_exception +device_available_emulate: +#endif
Hi,
can you explain what this chunk is for? It appears to be for the non-xen case due to the ifndef, yet it seems to visibly change the code for that... that deserves an explanation for sure ...
Greetings,
Arjan van de Ven
Re: [RFC PATCH 16/33] Add support for Xen to entry.S.
Quoted 24 lines · Arjan van de Ven
Quoted 8 lines · Chris Wright
arch/i386/kernel/entry.S +1
-4+#ifndef CONFIG_XEN movl %cr0, %eax testl $0x4, %eax # EM (math emulation bit) - jne device_not_available_emulate - preempt_stop - call math_state_restore - jmp ret_from_exceptionarch/i386/kernel/entry.S ENTRY(device_not_available) · line 778
Quoted 10 lines
arch/i386/kernel/entry.S +4
-1-device_not_available_emulate: + je device_available_emulate pushl $0 # temporary storage for ORIG_EIP CFI_ADJUST_CFA_OFFSET 4 call math_emulate addl $4, %esp CFI_ADJUST_CFA_OFFSET -4 + jmp ret_from_exception +device_available_emulate: +#endifHi,
Quoted 4 lines
can you explain what this chunk is for? It appears to be for the non-xen case due to the ifndef, yet it seems to visibly change the code for that... that deserves an explanation for sure ...
This inverts the logic of the test so that the lines:
preempt_stop
call math_state_restore
jmp ret_from_exception
can be hoisted out of the ifdef and shared in the Xen and non-Xen cases.
J
Re: [RFC PATCH 16/33] Add support for Xen to entry.S.
In reply to Chris WrightQuoted 28 lines · Chris Wright
plain text document attachment (
i386-entry.S)
- change cli/sti
- change test for user mode return to work for kernel mode in ring1
- check hypervisor saved event mask on return from exception
- add entry points for the hypervisor upcall handlers
- avoid math emulation check when running on Xen
- add nmi handler for running on XenSigned-off-by Ian Pratt <[email protected]>Signed-off-by Christian Limpach <[email protected]>Signed-off-by Jeremy Fitzhardinge <[email protected]>Signed-off-by Chris Wright <[email protected]>--- arch/i386/kernel/asm-offsets.c | 26 +++++++ arch/i386/kernel/entry.S | 141 ++++++++++++++++++++++++++++++++++++----- arch/i386/mach-xen/setup-xen.c | 19 +++++ drivers/xen/core/features.c | 2 4 files changed, 169 insertions(+), 19 deletions(-)diff -r
5cca1805b8a7arch/i386/kernel/entry.Sarch/i386/kernel/entry.S +1
-0@@ -76,8 +76,39 @@ NT_MASK = 0x00004000 NT_MASK = 0x00004000 VM_MASK = 0x00020000 +#ifndef CONFIG_XEN
arch/i386/kernel/entry.S NT_MASK = 0x00004000 · line 89
Quoted 10 lines
arch/i386/kernel/entry.S +10
-0+#define DISABLE_INTERRUPTS cli +#define ENABLE_INTERRUPTS sti +#else +#include <xen/interface/xen.h> + +EVENT_MASK = 0x2E + +/* Offsets into shared_info_t. */ +#define evtchn_upcall_pending /* 0 */ +#define evtchn_upcall_mask 1
Erk... Can we get these into asm-offsets?
Quoted 10 lines
arch/i386/kernel/entry.S +10
-0+ +#ifdef CONFIG_SMP +/* Set %esi to point to the appropriate vcpu structure */ +#define GET_VCPU_INFO movl TI_cpu(%ebp),%esi ; \ + shl $SIZEOF_VCPU_INFO_SHIFT,%esi ; \ + addl HYPERVISOR_shared_info,%esi +#else +#define GET_VCPU_INFO movl HYPERVISOR_shared_info,%esi +#endif +
arch/i386/kernel/entry.S NT_MASK = 0x00004000 · line 109
Quoted 10 lines
arch/i386/kernel/entry.S +10
-0+/* The following end up using/clobbering %esi, because of GET_VCPU_INFO */ +#define __DISABLE_INTERRUPTS movb $1,evtchn_upcall_mask(%esi) +#define DISABLE_INTERRUPTS GET_VCPU_INFO ; \ + __DISABLE_INTERRUPTS +#define ENABLE_INTERRUPTS GET_VCPU_INFO ; \ + movb $0,evtchn_upcall_mask(%esi) +#define __TEST_PENDING testb $0xFF,evtchn_upcall_pending(%esi) +#endif + +
Actually, is it possible to move these to a header somewhere? In the paravirt_ops patches I used the names CLI and STI (copied from the VMI patches), but didn't allow them to clobber any regs. They're defined in asm-i386/paravirt.h.
If we want to allow them to clobber %esi, then your names are probably better, but we should clobber %esi in the !CONFIG_XEN case too if CONFIG_DEBUG_KERNEL.
Rusty.
Signature · 1 line
Help! Save Australia from the worst of the DMCA: http://linux.org.au/law
Re: [RFC PATCH 16/33] Add support for Xen to entry.S.
Quoted 28 lines · * Rusty Russell ([email protected]) wrote:
Quoted 27 lines · Chris Wright
plain text document attachment (
i386-entry.S)
- change cli/sti
- change test for user mode return to work for kernel mode in ring1
- check hypervisor saved event mask on return from exception
- add entry points for the hypervisor upcall handlers
- avoid math emulation check when running on Xen
- add nmi handler for running on XenSigned-off-by Ian Pratt <[email protected]>Signed-off-by Christian Limpach <[email protected]>Signed-off-by Jeremy Fitzhardinge <[email protected]>Signed-off-by Chris Wright <[email protected]>--- arch/i386/kernel/asm-offsets.c | 26 +++++++ arch/i386/kernel/entry.S | 141 ++++++++++++++++++++++++++++++++++++----- arch/i386/mach-xen/setup-xen.c | 19 +++++ drivers/xen/core/features.c | 2 4 files changed, 169 insertions(+), 19 deletions(-)diff -r
5cca1805b8a7arch/i386/kernel/entry.Sarch/i386/kernel/entry.S +0
-0@@ -76,8 +76,39 @@ NT_MASK = 0x00004000 NT_MASK = 0x00004000 VM_MASK = 0x00020000
Quoted 13 lines
arch/i386/kernel/entry.S NT_MASK = 0x00004000 · line 89
Quoted 11 lines
arch/i386/kernel/entry.S +11
-0+#ifndef CONFIG_XEN +#define DISABLE_INTERRUPTS cli +#define ENABLE_INTERRUPTS sti +#else +#include <xen/interface/xen.h> + +EVENT_MASK = 0x2E + +/* Offsets into shared_info_t. */ +#define evtchn_upcall_pending /* 0 */ +#define evtchn_upcall_mask 1Erk... Can we get these into asm-offsets?
Hmm, we put the vcpu shift in, guess we missed that. Thanks for noticing.
Quoted 15 lines
Quoted 15 lines
arch/i386/kernel/entry.S +15
-0+ +#ifdef CONFIG_SMP +/* Set %esi to point to the appropriate vcpu structure */ +#define GET_VCPU_INFO movl TI_cpu(%ebp),%esi ; \ + shl $SIZEOF_VCPU_INFO_SHIFT,%esi ; \ + addl HYPERVISOR_shared_info,%esi +#else +#define GET_VCPU_INFO movl HYPERVISOR_shared_info,%esi +#endif + +/* The following end up using/clobbering %esi, because of GET_VCPU_INFO */ +#define __DISABLE_INTERRUPTS movb $1,evtchn_upcall_mask(%esi) +#define DISABLE_INTERRUPTS GET_VCPU_INFO ; \ + __DISABLE_INTERRUPTS +#define ENABLE_INTERRUPTS GET_VCPU_INFO ; \
Quoted 8 lines
arch/i386/kernel/entry.S NT_MASK = 0x00004000 · line 107
Quoted 3 lines
arch/i386/kernel/entry.S +3
-0+ movb $0,evtchn_upcall_mask(%esi) +#define __TEST_PENDING testb $0xFF,evtchn_upcall_pending(%esi) +#endifActually, is it possible to move these to a header somewhere? In the
paravirt_opspatches I used the names CLI and STI (copied from the VMI patches), but didn't allow them to clobber any regs. They're defined inasm-i386/paravirt.h.
Sure, although it's only used here. Got a preference?
Quoted 3 lines
If we want to allow them to clobber %esi, then your names are probably better, but we should clobber %esi in the !CONFIG_XEN case too if
CONFIG_DEBUG_KERNEL.
That makes sense to me, just to catch those bugs.
thanks,
-chris
Re: [RFC PATCH 16/33] Add support for Xen to entry.S.
Quoted 74 lines · Chris Wright
Quoted 29 lines · * Rusty Russell ([email protected]) wrote:
Quoted 27 lines · Chris Wright
plain text document attachment (
i386-entry.S)
- change cli/sti
- change test for user mode return to work for kernel mode in ring1
- check hypervisor saved event mask on return from exception
- add entry points for the hypervisor upcall handlers
- avoid math emulation check when running on Xen
- add nmi handler for running on XenSigned-off-by Ian Pratt <[email protected]>Signed-off-by Christian Limpach <[email protected]>Signed-off-by Jeremy Fitzhardinge <[email protected]>Signed-off-by Chris Wright <[email protected]>--- arch/i386/kernel/asm-offsets.c | 26 +++++++ arch/i386/kernel/entry.S | 141 ++++++++++++++++++++++++++++++++++++----- arch/i386/mach-xen/setup-xen.c | 19 +++++ drivers/xen/core/features.c | 2 4 files changed, 169 insertions(+), 19 deletions(-)diff -r
5cca1805b8a7arch/i386/kernel/entry.Sarch/i386/kernel/entry.S +0
-0@@ -76,8 +76,39 @@ NT_MASK = 0x00004000 NT_MASK = 0x00004000 VM_MASK = 0x00020000Quoted 14 lines
arch/i386/kernel/entry.S NT_MASK = 0x00004000
Quoted 12 lines
arch/i386/kernel/entry.S +11
-0+#ifndef CONFIG_XEN +#define DISABLE_INTERRUPTS cli +#define ENABLE_INTERRUPTS sti +#else +#include <xen/interface/xen.h> + +EVENT_MASK = 0x2E + +/* Offsets into shared_info_t. */ +#define evtchn_upcall_pending /* 0 */ +#define evtchn_upcall_mask 1Erk... Can we get these into asm-offsets?
Hmm, we put the vcpu shift in, guess we missed that. Thanks for noticing.
Quoted 15 lines
Quoted 15 lines
arch/i386/kernel/entry.S +15
-0+ +#ifdef CONFIG_SMP +/* Set %esi to point to the appropriate vcpu structure */ +#define GET_VCPU_INFO movl TI_cpu(%ebp),%esi ; \ + shl $SIZEOF_VCPU_INFO_SHIFT,%esi ; \ + addl HYPERVISOR_shared_info,%esi +#else +#define GET_VCPU_INFO movl HYPERVISOR_shared_info,%esi +#endif + +/* The following end up using/clobbering %esi, because of GET_VCPU_INFO */ +#define __DISABLE_INTERRUPTS movb $1,evtchn_upcall_mask(%esi) +#define DISABLE_INTERRUPTS GET_VCPU_INFO ; \ + __DISABLE_INTERRUPTS +#define ENABLE_INTERRUPTS GET_VCPU_INFO ; \Quoted 9 lines
arch/i386/kernel/entry.S NT_MASK = 0x00004000
Quoted 4 lines
arch/i386/kernel/entry.S +3
-0+ movb $0,evtchn_upcall_mask(%esi) +#define __TEST_PENDING testb $0xFF,evtchn_upcall_pending(%esi) +#endifActually, is it possible to move these to a header somewhere? In the
paravirt_opspatches I used the names CLI and STI (copied from the VMI patches), but didn't allow them to clobber any regs. They're defined inasm-i386/paravirt.h.
Quoted 2 lines
Sure, although it's only used here. Got a preference?
It might be an idea to use something other than STI, CLI and CPUID, since the assembler is case-insensitive. We've already had one bug where CPUID didn't get replaced and the assembler quietly accepted it as-is.
J
Re: [RFC PATCH 16/33] Add support for Xen to entry.S.
Quoted 3 lines · * Jeremy Fitzhardinge ([email protected]) wrote:
It might be an idea to use something other than STI, CLI and CPUID, since the assembler is case-insensitive. We've already had one bug where CPUID didn't get replaced and the assembler quietly accepted it as-is.
Yes, having been bitten by that, I'd have to second that.
thanks,
-chris
RFC PATCH 17/33 Support loading an initrd when running on Xen
In reply to Chris WrightDue to the initial physical memory layout when booting on Xen, the initrd image ends up below min_low_pfn (as registered with the bootstrap memory allocator). Add Xen subarch support to enable initrd_below_start_ok flag, and disable initrd_reserve_bootmem.
--- arch/i386/kernel/setup.c | 4 +++- include/asm-i386/mach-xen/setup_arch_post.h | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-)
arch/i386/kernel/setup.c +3 -1
@@ -1205,6 +1205,7 @@ extern unsigned long __init setup_memoryextern unsigned long __init setup_memory (void );extern void zone_sizes_init (void );#endif /* !CONFIG_NEED_MULTIPLE_NODES */ +int initrd_reserve_bootmem =1 ;void __init setup_bootmem_allocator (void ) {
@@ -1265,7 +1266,8 @@ void __init setup_bootmem_allocator(void#ifdef CONFIG_BLK_DEV_INITRD if (LOADER_TYPE &&INITRD_START ) {if (INITRD_START +INITRD_SIZE <= (max_low_pfn <<PAGE_SHIFT )) { -reserve_bootmem (INITRD_START ,INITRD_SIZE ); +if (initrd_reserve_bootmem) +reserve_bootmem (INITRD_START ,INITRD_SIZE ); initrd_start =INITRD_START ?INITRD_START +PAGE_OFFSET :0 ; initrd_end = initrd_start+INITRD_SIZE ;
arch/i386/mach-xen/setup-xen.c +8 -0
@@ -5,6 +5,8 @@#include <linux/config.h> #include <linux/module.h> #include <linux/mm.h> +#include <linux/bootmem.h> +#include <linux/initrd.h> #include <asm/e820.h> #include <asm/setup.h>
@@ -68,4 +70,10 @@ void __init machine_specific_arch_setup(memcpy (saved_command_line, xen_start_info->cmd_line,MAX_GUEST_CMDLINE >COMMAND_LINE_SIZE ?COMMAND_LINE_SIZE :MAX_GUEST_CMDLINE ); + +if (INITRD_START && +INITRD_START +PAGE_OFFSET < min_low_pfn <<PAGE_SHIFT ) { + initrd_reserve_bootmem =0 ; + initrd_below_start_ok =1 ; + } }
include/asm-i386/setup.h +1 -0
@@ -66,6 +66,7 @@ extern unsigned char boot_params[PARAM_S#define LOWMEMSIZE () (0x9f000)extern unsigned long init_pg_tables_end; +extern int initrd_reserve_bootmem;struct e820entry ;
--
RFC PATCH 18/33 Subarch support for CPUID instruction
In reply to Chris WrightAllow subarchitectures to modify the CPUID instruction. This allows the subarch to provide a limited set of CPUID feature flags during CPU identification. Add a subarch implementation for Xen that traps to the hypervisor where unsupported feature flags can be hidden from guests.
--- arch/i386/kernel/head-cpu.S | 4 ++-- include/asm-i386/mach-default/mach_processor.h | 7 +++++++ include/asm-i386/mach-xen/mach_processor.h | 9 +++++++++ include/asm-i386/processor.h | 13 +++++++------ 4 files changed, 25 insertions(+), 8 deletions(-)
arch/i386/kernel/head-cpu.S +4 -2
@@ -1,4 +1,6 @@ /* Some macros for head.S */ + +#include <mach_processor.h> .macro CPU_GDT_TABLE .quad 0x0000000000000000 /* NULL descriptor */
@@ -57,7 +59,7 @@ .macro CPUID_GET_VENDOR_INFO cpuid_level, x86_vendor_id /* get vendor info */ xorl %eax,%eax # call CPUID with 0 -> return vendor ID - cpuid + CPUID movl %eax,\cpuid_level # save CPUID level movl %ebx,\x86_vendor_id # lo 4 chars movl %edx,\x86_vendor_id+4 # next 4 chars
@@ -73,7 +75,7 @@ */ .macro CPUID_GET_CPU_TYPE family, model, mask, capability movl $1,%eax # Use the CPUID instruction to get CPU type - cpuid + CPUID movb %al,%cl # save reg for future use andb $0x0f,%ah # mask processor family movb %ah,\family
include/asm-i386/processor.h +7 -6
@@ -20,6 +20,7 @@#include <linux/threads.h> #include <asm/percpu.h> #include <linux/cpumask.h> +#include <mach_processor.h> /* flag for disabling the tsc */ extern int tsc_disable;
@@ -147,7 +148,7 @@ static inline void detect_ht(struct cpui*/ static inline void cpuid (unsigned int op,unsigned int *eax,unsigned int *ebx,unsigned int *ecx,unsigned int *edx) { - __asm__("cpuid" + __asm__(CPUID_STR : "=a" (*eax),"=b" (*ebx), "=c" (*ecx),
@@ -159,7 +160,7 @@ static inline void cpuid_count(int op, istatic inline void cpuid_count (int op,int count,int *eax,int *ebx,int *ecx,int *edx) { - __asm__("cpuid" + __asm__(CPUID_STR : "=a" (*eax),"=b" (*ebx), "=c" (*ecx),
@@ -174,7 +175,7 @@ static inline unsigned int cpuid_eax(uns {unsigned int eax; - __asm__("cpuid" + __asm__(CPUID_STR : "=a" (eax) : "0" (op) : "bx", "cx", "dx");
@@ -184,7 +185,7 @@ static inline unsigned int cpuid_ebx(uns {unsigned int eax, ebx; - __asm__("cpuid" + __asm__(CPUID_STR : "=a" (eax), "=b" (ebx) : "0" (op) : "cx", "dx" );
@@ -194,7 +195,7 @@ static inline unsigned int cpuid_ecx(uns {unsigned int eax, ecx; - __asm__("cpuid" + __asm__(CPUID_STR : "=a" (eax), "=c" (ecx) : "0" (op) : "bx", "dx" );
@@ -204,7 +205,7 @@ static inline unsigned int cpuid_edx(uns {unsigned int eax, edx; - __asm__("cpuid" + __asm__(CPUID_STR : "=a" (eax), "=d" (edx) : "0" (op) : "bx", "cx");
include/asm-i386/mach-default/mach_processor.h new file +7 -0
@@ -0,0 +1,7 @@ +#ifndef __ASM_MACH_PROCESSOR_H +#define __ASM_MACH_PROCESSOR_H + +#define CPUID cpuid +#define CPUID_STR "cpuid" + +#endif /* __ASM_MACH_PROCESSOR_H */
include/asm-i386/mach-xen/mach_processor.h new file +9 -0
@@ -0,0 +1,9 @@ +#ifndef __ASM_MACH_PROCESSOR_H +#define __ASM_MACH_PROCESSOR_H + +#include <xen/interface/arch-x86_32.h> + +#define CPUID XEN_CPUID +#define CPUID_STR XEN_CPUID + +#endif /* __ASM_MACH_PROCESSOR_H */
--
Re: [RFC PATCH 18/33] Subarch support for CPUID instruction
Quoted 5 lines · Chris Wright
plain text document attachment (i386-cpuid)
Allow subarchitectures to modify the CPUID instruction. This allows the subarch to provide a limited set of CPUID feature flags during CPU identification. Add a subarch implementation for Xen that traps to the hypervisor where unsupported feature flags can be hidden from guests.
Hi,
I'm wondering if this is entirely the wrong level of abstraction; to me it feels the subarch shouldn't override the actual cpuid, but the cpu feature flags that linux uses. That's a lot less messy: cpuid has many many pieces of information which are near impossible to filter in practice, however filtering the USAGE of it is trivial; linux basically flattens the cpuid namespace into a simple bitmap of "what the kernel can use". That is really what the subarch should filter/fixup, just like we do for cpu quirks etc etc.
Greetings,
Arjan van de Ven
Re: [RFC PATCH 18/33] Subarch support for CPUID instruction
Quoted 5 lines · Arjan van de Ven
Quoted 4 lines
Allow subarchitectures to modify the CPUID instruction. This allows the subarch to provide a limited set of CPUID feature flags during CPU identification. Add a subarch implementation for Xen that traps to the hypervisor where unsupported feature flags can be hidden from guests.
Quoted 8 lines
I'm wondering if this is entirely the wrong level of abstraction; to me it feels the subarch shouldn't override the actual cpuid, but the cpu feature flags that linux uses. That's a lot less messy: cpuid has many many pieces of information which are near impossible to filter in practice, however filtering the USAGE of it is trivial; linux basically flattens the cpuid namespace into a simple bitmap of "what the kernel can use". That is really what the subarch should filter/fixup, just like we do for cpu quirks etc etc.
Maybe we should have that *as well*, but it makes sense to allow the hypervisor to apply a filter too. For example, whether it supports PSE, FXSAVE/FXRSTOR, etc. These are things the 'platform' is telling the OS -- not something the OS can filter for itself. To trap on CPUID invocations requires the guest to use a special code sequence for CPUID, since the instruction will never normally fault. Hence moving to mach-* to hide this detail.
-- Keir
Re: [RFC PATCH 18/33] Subarch support for CPUID instruction
Quoted 18 lines · Keir Fraser
Quoted 6 lines · Arjan van de Ven
Quoted 5 lines
Allow subarchitectures to modify the CPUID instruction. This allows the subarch to provide a limited set of CPUID feature flags during CPU identification. Add a subarch implementation for Xen that traps to the
hypervisor where unsupported feature flags can be hidden from guests.Quoted 9 lines
I'm wondering if this is entirely the wrong level of abstraction; to me it feels the subarch shouldn't override the actual cpuid, but the cpu feature flags that linux uses. That's a lot less messy: cpuid has many many pieces of information which are near impossible to filter in practice, however filtering the USAGE of it is trivial; linux basically flattens the cpuid namespace into a simple bitmap of "what the kernel can use". That is really what the subarch should filter/fixup, just like
we do for cpu quirks etc etc.
Quoted 4 lines
Maybe we should have that *as well*, but it makes sense to allow the hypervisor to apply a filter too. For example, whether it supports PSE, FXSAVE/FXRSTOR, etc. These are things the 'platform' is telling the OS -- not something the OS can filter for itself.
To some degree "Xen" is just a magic type of cpu that can/should have a quirk to filter these out... it just feels wrong and fragile to me to do it via a cpuid filter... once you have the "other" filter as I suggested I bet the need for the cpuid filter just goes away...
Re: [RFC PATCH 18/33] Subarch support for CPUID instruction
In reply to Arjan van de VenQuoted 11 lines · Arjan van de Ven
Quoted 6 lines · Chris Wright
plain text document attachment (i386-cpuid)
Allow subarchitectures to modify the CPUID instruction. This allows the subarch to provide a limited set of CPUID feature flags during CPU identification. Add a subarch implementation for Xen that traps to the hypervisor where unsupported feature flags can be hidden from guests.Hi,
Quoted 9 lines
I'm wondering if this is entirely the wrong level of abstraction; to me it feels the subarch shouldn't override the actual cpuid, but the cpu feature flags that linux uses. That's a lot less messy: cpuid has many many pieces of information which are near impossible to filter in practice, however filtering the USAGE of it is trivial; linux basically flattens the cpuid namespace into a simple bitmap of "what the kernel can use". That is really what the subarch should filter/fixup, just like we do for cpu quirks etc etc.
You really need a CPUID hook. The instruction is non-virtualizable, and anything claiming to be a hypervisor really has to support masking and flattening the cpuid namespace for the instruction itself. It is used in assembler code and very early in boot. The alternative is injecting a bunch of Xen-specific code to filter feature bits into the i386 layer, which is both bad for Linux and bad for Xen - and was quite ugly in the last set of Xen patches.
Zach
David Miller
maintainerRe: [RFC PATCH 18/33] Subarch support for CPUID instruction
From: Zachary Amsden <[email protected]>
Date: Tue, 18 Jul 2006 04:33:52 -0700
Quoted 7 lines
You really need a CPUID hook. The instruction is non-virtualizable, and anything claiming to be a hypervisor really has to support masking and flattening the cpuid namespace for the instruction itself. It is used in assembler code and very early in boot. The alternative is injecting a bunch of Xen-specific code to filter feature bits into the i386 layer, which is both bad for Linux and bad for Xen - and was quite ugly in the last set of Xen patches.
Userspace will still see the full set of cpuid bits, since it can still execute cpuid unimpeded, is this ok?
Re: [RFC PATCH 18/33] Subarch support for CPUID instruction
Quoted 11 lines · * David Miller ([email protected]) wrote:
From: Zachary Amsden <[email protected]>
Date: Tue, 18 Jul 2006 04:33:52 -0700Quoted 7 lines
You really need a CPUID hook. The instruction is non-virtualizable, and anything claiming to be a hypervisor really has to support masking and flattening the cpuid namespace for the instruction itself. It is used in assembler code and very early in boot. The alternative is injecting a bunch of Xen-specific code to filter feature bits into the i386 layer, which is both bad for Linux and bad for Xen - and was quite ugly in the last set of Xen patches.
Quoted 2 lines
Userspace will still see the full set of cpuid bits, since it can still execute cpuid unimpeded, is this ok?
Yup, it's for the kernel (things like systenter vs int 80 in vdso).
thanks,
-chris
RFC PATCH 19/33 Support gdt/idt/ldt handling on Xen.
In reply to Chris WrightMove the macros which handle gdt/idt/ldt's into a subarch include file and add implementations for running on Xen.
--- arch/i386/mach-xen/memory.c | 17 +++++++ include/asm-i386/desc.h | 65 ++--------------------------- include/asm-i386/mach-default/mach_desc.h | 67 ++++++++++++++++++++++++++++++ include/asm-i386/mach-xen/mach_desc.h | 62 +++++++++++++++++++++++++++ 4 files changed, 151 insertions(+), 60 deletions(-)
arch/i386/mach-xen/memory.c +17 -0
@@ -29,3 +29,20 @@ void make_lowmem_page_readonly(unsigned rc =HYPERVISOR_update_va_mapping (address,pte_wrprotect (*pte),0 );BUG_ON (rc); } + +void load_gdt (struct Xgt_desc_struct *gdt_descr) +{ +unsigned long frames[16 ]; +unsigned long va; +int f; + +for (va = gdt_descr->address, f =0 ; + va < gdt_descr->address + gdt_descr->size; + va +=PAGE_SIZE , f++) { + frames[f] =virt_to_mfn (va); +make_lowmem_page_readonly (va, + XENFEAT_writable_descriptor_tables); + } +if (HYPERVISOR_set_gdt (frames, gdt_descr->size /8 )) +BUG (); +}
include/asm-i386/desc.h +8 -52
98 lines not on this page · raw message
include/asm-i386/mach-default/mach_desc.h new file +62 -0
62 lines not on this page · raw message
include/asm-i386/mach-xen/mach_desc.h new file +51 -0
51 lines not on this page · raw message
--
RFC PATCH 20/33 subarch support for interrupt and exception gates
In reply to Chris WrightAbstract the code that sets up interrupt and exception gates, and add a separate subarch implementation for Xen.
--- arch/i386/kernel/traps.c | 35 ++++++----------------------- include/asm-i386/mach-default/mach_idt.h | 24 +++++++++++++++++++ include/asm-i386/mach-xen/mach_idt.h | 25 ++++++++++++++++++++ include/asm-i386/mach-xen/setup_arch_pre.h | 2 + 4 files changed, 59 insertions(+), 27 deletions(-)
arch/i386/kernel/traps.c +8 -27
@@ -1084,29 +1084,11 @@ void __init trap_init_f00f_bug(void) }#endif -#define _set_gate (gate_addr,type,dpl,addr,seg) \ -do { \ - int __d0, __d1; \ - __asm__ __volatile__ ("movw %%dx,%%ax\n\t" \ - "movw %4,%%dx\n\t" \ - "movl %%eax,%0\n\t" \ - "movl %%edx,%1" \ - :"=m" (*((long *) (gate_addr))), \ - "=m" (*(1+(long *) (gate_addr))), "=&a" (__d0), "=&d" (__d1) \ - :"i" ((short) (0x8000+(dpl<<13)+(type<<8))), \ - "3" ((char *) (addr)),"2" ((seg) << 16)); \ -} while (0) - - -/* -* This needs to use 'idt_table' rather than 'idt', and -* thus use the _nonmapped_ version of the IDT, as the -* Pentium F0 0F bugfix can have resulted in the mapped -* IDT being write-protected. -*/ +#include <mach_idt.h> +void set_intr_gate (unsigned int n,void *addr) { -_set_gate (idt_table+n,14 ,0 ,addr,__KERNEL_CS); +_set_gate (n,14 ,0 ,addr,__KERNEL_CS); }/*
@@ -1114,24 +1096,23 @@ void set_intr_gate(unsigned int n, void*/ static inline void set_system_intr_gate (unsigned int n,void *addr) { -_set_gate (idt_table+n,14 ,3 , addr, __KERNEL_CS); +_set_gate (n,14 ,3 , addr, __KERNEL_CS); }static void __init set_trap_gate (unsigned int n,void *addr) { -_set_gate (idt_table+n,15 ,0 ,addr,__KERNEL_CS); +_set_gate (n,15 ,0 ,addr,__KERNEL_CS); }static void __init set_system_gate (unsigned int n,void *addr) { -_set_gate (idt_table+n,15 ,3 ,addr,__KERNEL_CS); +_set_gate (n,15 ,3 ,addr,__KERNEL_CS); }static void __init set_task_gate (unsigned int n,unsigned int gdt_entry) { -_set_gate (idt_table+n,5 ,0 ,0 ,(gdt_entry<<3 )); -} - +_set_gate (n,5 ,0 ,0 ,(gdt_entry<<3 )); +}void __init trap_init (void ) {
arch/i386/mach-xen/setup-xen.c +2 -0
@@ -23,6 +23,8 @@ EXPORT_SYMBOL(xen_start_info);*/ struct shared_info *HYPERVISOR_shared_info = (struct shared_info *)empty_zero_page;EXPORT_SYMBOL (HYPERVISOR_shared_info); + +struct trap_info xen_trap_table[257 ];/** * machine_specific_memory_setup - Hook for machine specific memory setup.
include/asm-i386/mach-xen/setup_arch.h +1 -0
@@ -3,6 +3,7 @@extern struct start_info *xen_start_info;extern struct shared_info *HYPERVISOR_shared_info; +extern struct trap_info xen_trap_table[257 ];void __init machine_specific_arch_setup (void );
include/asm-i386/mach-default/mach_idt.h new file +24 -0
@@ -0,0 +1,24 @@ +#ifndef __ASM_MACH_IDT_H +#define __ASM_MACH_IDT_H + +/* +* This needs to use 'idt_table' rather than 'idt', and +* thus use the _nonmapped_ version of the IDT, as the +* Pentium F0 0F bugfix can have resulted in the mapped +* IDT being write-protected. +*/ +#define _set_gate (vector,type,dpl,addr,seg) \ +do { \ + int __d0, __d1; \ + struct gate_struct *__gate_addr = idt_table + (vector); \ + __asm__ __volatile__ ("movw %%dx,%%ax\n\t" \ + "movw %4,%%dx\n\t" \ + "movl %%eax,%0\n\t" \ + "movl %%edx,%1" \ + :"=m" (*((long *) __gate_addr)), \ + "=m" (*(1+(long *) __gate_addr)), "=&a" (__d0), "=&d" (__d1) \ + :"i" ((short) (0x8000+(dpl<<13)+(type<<8))), \ + "3" ((char *) (addr)),"2" ((seg) << 16)); \ +} while (0) + +#endif /* __ASM_MACH_IDT_H */
include/asm-i386/mach-xen/mach_idt.h new file +25 -0
@@ -0,0 +1,25 @@ +#ifndef __ASM_MACH_IDT_H +#define __ASM_MACH_IDT_H + +static inline void _set_gate (unsigned int vector,uint8_t type,uint8_t dpl, +void *addr,uint16_t seg) +{ +struct trap_info *t = xen_trap_table; + +BUG_ON (vector >256 ); + +if (type ==5 ) +return ; + +while (t->address && t->vector != vector) + t++; + + t->vector = vector; + t->cs = seg; +TI_SET_DPL (t, dpl); +if (type ==14 || vector ==7 ) +TI_SET_IF (t,1 ); + t->address = (unsigned long )addr; +} + +#endif /* __ASM_MACH_IDT_H */
--
RFC PATCH 21/33 subarch support for control register accesses
In reply to Chris WrightAbstract the code that accesses control register, and add a separate subarch implementation for Xen.
--- include/asm-i386/mach-default/mach_system.h | 57 +++++++++++++++++++++++++++ include/asm-i386/mach-xen/mach_system.h | 54 +++++++++++++++++++++++++ include/asm-i386/system.h | 57 --------------------------- 3 files changed, 111 insertions(+), 57 deletions(-)
include/asm-i386/mach-default/mach_system.h +57 -0
@@ -3,4 +3,61 @@#define clearsegment (seg) +#define read_cr0 () ({ \ + unsigned int __dummy; \ + __asm__ __volatile__( \ + "movl %%cr0,%0\n\t" \ + :"=r" (__dummy)); \ + __dummy; \ +}) +#define write_cr0 (x) \ + __asm__ __volatile__("movl %0,%%cr0": :"r" (x)) + +#define read_cr2 () ({ \ + unsigned int __dummy; \ + __asm__ __volatile__( \ + "movl %%cr2,%0\n\t" \ + :"=r" (__dummy)); \ + __dummy; \ +}) +#define write_cr2 (x) \ + __asm__ __volatile__("movl %0,%%cr2": :"r" (x)) + +#define read_cr3 () ({ \ + unsigned int __dummy; \ + __asm__ ( \ + "movl %%cr3,%0\n\t" \ + :"=r" (__dummy)); \ + __dummy; \ +}) +#define write_cr3 (x) \ + __asm__ __volatile__("movl %0,%%cr3": :"r" (x)) + +#define read_cr4 () ({ \ + unsigned int __dummy; \ + __asm__( \ + "movl %%cr4,%0\n\t" \ + :"=r" (__dummy)); \ + __dummy; \ +}) +#define read_cr4_safe () ({ \ + unsigned int __dummy; \ +/* This could fault if %cr4 does not exist */ \ + __asm__("1: movl %%cr4, %0 \n" \ +"2: \n" \ +".section __ex_table,\"a\" \n" \ +".long 1b,2b \n" \ +".previous \n" \ + :"=r" (__dummy):"0" (0 )); \ + __dummy; \ +}) +#define write_cr4 (x) \ + __asm__ __volatile__("movl %0,%%cr4": :"r" (x)) + +/* +* Clear and set 'TS' bit respectively +*/ +#define clts () __asm__ __volatile__ ("clts") +#define stts () write_cr0(8 | read_cr0()) +#endif /* __ASM_MACH_SYSTEM_H */
include/asm-i386/mach-xen/mach_system.h +54 -0
58 lines not on this page · raw message
include/asm-i386/system.h +0 -57
63 lines not on this page · raw message
--
RFC PATCH 22/33 subarch stack pointer update
In reply to Chris WrightRegister the new kernel ('ring 0') stack pointer with the hypervisor during context switch.
--- include/asm-i386/mach-default/mach_processor.h | 7 +++++++ include/asm-i386/mach-xen/mach_processor.h | 8 ++++++++ include/asm-i386/processor.h | 1 + 3 files changed, 16 insertions(+)
include/asm-i386/mach-default/mach_processor.h +7 -0
@@ -4,4 +4,11 @@#define CPUID cpuid#define CPUID_STR "cpuid" +#ifndef __ASSEMBLY__ +static inline void mach_update_kernel_stack (unsigned long esp0, +unsigned short ss0) +{ +} +#endif +#endif /* __ASM_MACH_PROCESSOR_H */
include/asm-i386/mach-xen/mach_processor.h +8 -0
@@ -6,4 +6,12 @@#define CPUID XEN_CPUID#define CPUID_STR XEN_CPUID +#ifndef __ASSEMBLY__ +static inline void mach_update_kernel_stack (unsigned long esp0, +unsigned short ss0) +{ +HYPERVISOR_stack_switch (ss0, esp0); +} +#endif +#endif /* __ASM_MACH_PROCESSOR_H */
include/asm-i386/processor.h +1 -0
@@ -500,6 +500,7 @@ static inline void load_esp0(struct tss_ tss->ss1 = thread->sysenter_cs;wrmsr (MSR_IA32_SYSENTER_CS , thread->sysenter_cs,0 ); } +mach_update_kernel_stack (tss->esp0, tss->ss0); }#define start_thread (regs, new_eip, new_esp) do { \
--
RFC PATCH 23/33 subarch TLB support
In reply to Chris WrightParavirtualize TLB flushes by using the flush interfaces provided by the hypervisor. These hide the details of cross-CPU shootdowns and allow significant optimisations (for example, by avoiding shooting down on virtual CPUs that are descheduled). This is considerably faster in most cases than performing virtual IPIs in the guest kernel.
--- include/asm-i386/mach-default/mach_tlbflush.h | 59 ++++++++++++++++++++++++++ include/asm-i386/mach-xen/mach_tlbflush.h | 25 +++++++++++ include/asm-i386/tlbflush.h | 55 ------------------------ 3 files changed, 85 insertions(+), 54 deletions(-)
include/asm-i386/tlbflush.h +1 -54
@@ -5,64 +5,11 @@#include <linux/mm.h> #include <asm/processor.h> -#define __flush_tlb () \ - do { \ - unsigned int tmpreg; \ - \ - __asm__ __volatile__( \ - "movl %%cr3, %0; \n" \ - "movl %0, %%cr3; # flush TLB \n" \ - : "=r" (tmpreg) \ - :: "memory"); \ - } while (0) - -/* -* Global pages have to be flushed a bit differently. Not a real -* performance problem because this does not happen often. -*/ -#define __flush_tlb_global () \ - do { \ - unsigned int tmpreg, cr4, cr4_orig; \ - \ - __asm__ __volatile__( \ - "movl %%cr4, %2; # turn off PGE \n" \ - "movl %2, %1; \n" \ - "andl %3, %1; \n" \ - "movl %1, %%cr4; \n" \ - "movl %%cr3, %0; \n" \ - "movl %0, %%cr3; # flush TLB \n" \ - "movl %2, %%cr4; # turn PGE back on \n" \ - : "=&r" (tmpreg), "=&r" (cr4), "=&r" (cr4_orig) \ - : "i" (~X86_CR4_PGE) \ - : "memory"); \ - } while (0) -extern unsigned long pgkern_mask; - -# define __flush_tlb_all () \ - do { \ - if (cpu_has_pge) \ - __flush_tlb_global(); \ - else \ - __flush_tlb(); \ - } while (0)#define cpu_has_invlpg (boot_cpu_data.x86 > 3) -#define __flush_tlb_single (addr) \ - __asm__ __volatile__("invlpg %0": :"m" (*(char *) addr)) - -#ifdef CONFIG_X86_INVLPG -# define __flush_tlb_one (addr) __flush_tlb_single(addr) -#else -# define __flush_tlb_one (addr) \ - do { \ - if (cpu_has_invlpg) \ - __flush_tlb_single(addr); \ - else \ - __flush_tlb(); \ - } while (0) -#endif +#include <mach_tlbflush.h> /* * TLB flushing:
include/asm-i386/mach-default/mach_tlbflush.h new file +59 -0
59 lines not on this page · raw message
include/asm-i386/mach-xen/mach_tlbflush.h new file +25 -0
@@ -0,0 +1,25 @@ +#ifndef __ASM_MACH_TLBFLUSH_H +#define __ASM_MACH_TLBFLUSH_H + +static inline void xen_tlb_flush (void ) +{ +struct mmuext_op op; + op.cmd =MMUEXT_TLB_FLUSH_LOCAL ; +BUG_ON (HYPERVISOR_mmuext_op (&op,1 ,NULL ,DOMID_SELF ) <0 ); +} + +static inline void xen_invlpg (unsigned long ptr) +{ +struct mmuext_op op; + op.cmd =MMUEXT_INVLPG_LOCAL ; + op.arg1.linear_addr = ptr &PAGE_MASK ; +BUG_ON (HYPERVISOR_mmuext_op (&op,1 ,NULL ,DOMID_SELF ) <0 ); +} + +#define __flush_tlb () xen_tlb_flush() +#define __flush_tlb_global () xen_tlb_flush() +#define __flush_tlb_all () xen_tlb_flush() +#define __flush_tlb_single (addr) xen_invlpg(addr) +#define __flush_tlb_one (addr) __flush_tlb_single(addr) + +#endif /* __ASM_MACH_TLBFLUSH_H */
--
David Miller
maintainerRe: [RFC PATCH 23/33] subarch TLB support
From: Chris Wright <[email protected]>
Date: Tue, 18 Jul 2006 00:00:23 -0700
Quoted 1 line
+
BUG_ON(HYPERVISOR_mmuext_op(&op, 1, NULL, DOMID_SELF) < 0);
Although it happens to work currently, I think we should get out of the habit of putting operations with wanted side effects into BUG_ON() calls. The following is therefore more preferable:
ret = HYPERVISOR_mmuext_op(&op, 1, NULL, DOMID_SELF); BUG_ON(ret < 0);
If this were ASSERT() in userspace, turning off debugging at build time would make the evaluations inside of the macro never occur. It is my opinion that BUG_ON() should behave similarly.
Re: [RFC PATCH 23/33] subarch TLB support
Quoted 9 lines · * David Miller ([email protected]) wrote:
From: Chris Wright <[email protected]>
Date: Tue, 18 Jul 2006 00:00:23 -0700Quoted 1 line
+
BUG_ON(HYPERVISOR_mmuext_op(&op, 1, NULL, DOMID_SELF) < 0);Although it happens to work currently, I think we should get out of the habit of putting operations with wanted side effects into
BUG_ON()calls. The following is therefore more preferable:
Quoted 6 lines
ret = HYPERVISOR_mmuext_op(&op, 1, NULL, DOMID_SELF); BUG_ON(ret < 0);If this were
ASSERT()in userspace, turning off debugging at build time would make the evaluations inside of the macro never occur. It is my opinion thatBUG_ON()should behave similarly.
Good point, I'll clean those up.
thanks,
-chris
RFC PATCH 24/33 Add support for Xen event channels.
In reply to Chris WrightSupport Xen event channels instead of the i8259 PIC.
Event channels are used to inject events into the kernel, either from the hypervisor or from another VM. The injected events are mapped to interrupts.
If an event needs to be injected, the hypervisor causes an upcall into the kernel. The upcall handler then scans the event pending bitmap and calls do_IRQ for each pending event.
--- arch/i386/kernel/Makefile | 6 drivers/xen/core/evtchn.c | 876 ++++++++++++++++++++++++++++++++ include/asm-i386/hw_irq.h | 4 include/asm-i386/mach-xen/irq_vectors.h | 109 +++ include/xen/evtchn.h | 116 ++++ 5 files changed, 1109 insertions(+), 2 deletions(-)
arch/i386/kernel/Makefile +5 -1
@@ -5,7 +5,7 @@ extra-y := head.o init_task.o vmlinux.ld extra-y := head.o init_task.o vmlinux.lds obj-y := process.o semaphore.o signal.o entry.o traps.o irq.o \ - ptrace.o time.o ioport.o ldt.o setup.o i8259.o sys_i386.o \ + ptrace.o time.o ioport.o ldt.o setup.o hw_irq.o sys_i386.o \ pci-dma.o i386_ksyms.o i387.o bootflag.o \ quirks.o i8237.o topology.o alternative.o i8253.o tsc.o
@@ -42,6 +42,10 @@ EXTRA_AFLAGS := -traditional EXTRA_AFLAGS := -traditional obj-$(CONFIG_SCx200) += scx200.o + +hw_irq-y := i8259.o + +hw_irq-$(CONFIG_XEN) := ../../../drivers/xen/core/evtchn.o # vsyscall.o contains the vsyscall DSO images as __initdata. # We must build both images before we can assemble it.
drivers/xen/core/evtchn.c new file +880 -0
880 lines not on this page · raw message
include/asm-i386/mach-xen/irq_vectors.h new file +109 -0
109 lines not on this page · raw message
include/xen/evtchn.h new file +116 -0
116 lines not on this page · raw message
--
RFC PATCH 25/33 Implement timekeeping for Xen
In reply to Chris WrightUse the hypervisor as the basis of the guest's time. This means that the hypervisor wallclock time is used to emulate the cmos clock, and set the inital system clock at boot.
It also registers a Xen clocksource, so the system clock is kept in sync with the hypervisor's clock.
This does not implement setting the hypervisor clock; nor does it implement non-independent time, so hypervisor wallclock changes will not affect the guest.
--- arch/i386/mach-default/Makefile | 2 arch/i386/mach-default/setup-time.c | 16 + arch/i386/mach-default/setup.c | 14 - drivers/xen/core/Makefile | 2 drivers/xen/core/time.c | 285 +++++++++++++++++++++++++++++++++ include/asm-i386/mach-xen/do_timer.h | 3 include/asm-i386/mach-xen/mach_time.h | 16 + 7 files changed, 322 insertions(+), 16 deletions(-)
arch/i386/mach-default/Makefile +1 -1
@@ -2,4 +2,4 @@ # Makefile for the linux kernel. # -obj-y := setup.o setup-memory.o +obj-y := setup.o setup-memory.o setup-time.o
arch/i386/mach-default/setup.c +0 -14
@@ -77,20 +77,6 @@ void __init trap_init_hook(void) { } -static struct irqaction irq0 = { timer_interrupt,IRQF_DISABLED ,CPU_MASK_NONE ,"timer" ,NULL ,NULL }; - -/** -* time_init_hook - do any specific initialisations for the system timer. -* -* Description: -* Must plug the system timer interrupt source at HZ into the IRQ listed -* in irq_vectors.h:TIMER_IRQ -**/ -void __init time_init_hook (void ) -{ -setup_irq (0 , &irq0); -} -#ifdef CONFIG_MCA /** * mca_nmi_hook - hook into MCA specific NMI chain
drivers/xen/core/Makefile +1 -1
@@ -1,3 +1,3 @@ -obj-y := features.o +obj-y := features.o time.o
arch/i386/mach-default/setup-time.c new file +16 -0
@@ -0,0 +1,16 @@ +#include <linux/interrupt.h> +#include <asm/arch_hooks.h> + +static struct irqaction irq0 = { timer_interrupt,IRQF_DISABLED ,CPU_MASK_NONE ,"timer" ,NULL ,NULL }; + +/** +* time_init_hook - do any specific initialisations for the system timer. +* +* Description: +* Must plug the system timer interrupt source at HZ into the IRQ listed +* in irq_vectors.h:TIMER_IRQ +**/ +void __init time_init_hook (void ) +{ +setup_irq (0 , &irq0); +}
drivers/xen/core/time.c new file +362 -0
362 lines not on this page · raw message
include/asm-i386/mach-xen/do_timer.h new file +3 -0
@@ -0,0 +1,3 @@ + +void do_timer_interrupt_hook (struct pt_regs *regs); +
include/asm-i386/mach-xen/mach_time.h new file +16 -0
@@ -0,0 +1,16 @@ +#ifndef _MACH_TIME_H +#define _MACH_TIME_H + +/* Still need these definitions for other uses of cmos/nvram */ +#include <linux/mc146818rtc.h> + +/* for check timing call set_rtc_mmss() 500ms */ +/* used in arch/i386/time.c::do_timer_interrupt() */ +#define USEC_AFTER 500000 +#define USEC_BEFORE 500000 + +/* In Xen, "CMOS" time is fetched from the hypervisor */ +int mach_set_rtc_mmss (unsigned long nowtime); +unsigned long mach_get_cmos_time (void ); + +#endif /* !_MACH_TIME_H */
--
Re: [RFC PATCH 25/33] Implement timekeeping for Xen
Quoted 7 lines · Chris Wright
plain text document attachment (xen-time)
Use the hypervisor as the basis of the guest's time. This means that the hypervisor wallclock time is used to emulate the cmos clock, and set the inital system clock at boot.It also registers a Xen clocksource, so the system clock is kept in sync with the hypervisor's clock.
Interesting. The Andi has been bugging me for a similarly designed per-cpu TSC clocksource, but just for generic use. I'm a little skeptical that it will be 100% without error, since anything dealing w/ the TSCs have been nothing but trouble in my mind, but this looks like a good proving ground for the concept.
It was mentioned to me that the clocksource approach helped cleanup some of the xen time changes (is that really true? :), but there were still some outstanding issues (time inconsistencies, perhaps?). I'm just curious if there are any details about the issues there, or if I misunderstood?
Quoted 3 lines
This does not implement setting the hypervisor clock; nor does it implement non-independent time, so hypervisor wallclock changes will not affect the guest.
Hmmm. I'm not sure if I understood that last line or not. I guess I need to think a bit about CLOCK_REALTIME vs CLOCK_MONOTONIC wrt hypervisiors.
I guess the question is "who owns time?" the guest OS (does it have its own CLOCK_REALTIME, independent of other guests?) or does the hypervisor? What does NTPd running on a guest actually adjust?
Anyway, some comments and boneheaded questions below.
Quoted 23 lines
diff -r
2657dae4b5cddrivers/xen/core/time.cdrivers/xen/core/time.c new file +19
-0@@ -0,0 +1,362 @@ +/* +* Xen-specific time functions +*/ + +#include <linux/kernel.h> +#include <linux/time.h> +#include <linux/interrupt.h> +#include <linux/clocksource.h> +#include <linux/kernel_stat.h> + +#include <asm/arch_hooks.h> +#include <asm/hypervisor.h> + +#include <xen/evtchn.h> +#include <xen/interface/xen.h> +#include <xen/interface/vcpu.h> + +#include "mach_time.h" +#include "do_timer.h"
drivers/xen/core/time.c line 29
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+ + +/* Permitted clock jitter, in nsecs, beyond which a warning will be printed. */ +static unsigned long permitted_clock_jitter =10000000UL ;/* 10ms */ +static int __init __permitted_clock_jitter (char *str) +{ + permitted_clock_jitter =simple_strtoul (str,NULL ,0 ); +return 1 ; +} +__setup ("permitted_clock_jitter=" , __permitted_clock_jitter);
permitted_clock_jitter is a little vague and might get confused w/ the NTP notion of jitter. Is there a better name, or could we get a xen_ prefix there?
drivers/xen/core/time.c line 37
Quoted 6 lines
drivers/xen/core/time.c new file +6
-0+/* These are perodically updated in shared_info, and then copied here. */ +struct shadow_time_info { +u64 tsc_timestamp;/* TSC at last update of time vals. */ +u64 system_timestamp;/* Time, in nanosecs, since boot. */ +u32 tsc_to_nsec_mul; +u32 tsc_to_usec_mul;
Hmmm. Keeping separate cycle->usec and cycle->nsec multipliers is an interesting optimization. I'd even consider it for the generic clocksource code, but I suspect recalculating the independent adjustment factors for both kills the performance benefit. Have you actually compaired against the cost of the /1000 going from nsec to usec?
drivers/xen/core/time.c line 39
Quoted 2 lines
drivers/xen/core/time.c new file +2
-0+int tsc_shift; +u32 version;
Errr.. Why is a version value necessary?
drivers/xen/core/time.c line 46
Quoted 6 lines
drivers/xen/core/time.c new file +6
-0+ +static DEFINE_PER_CPU(struct shadow_time_info, shadow_time); + +/* Keep track of last time we did processing/updating of jiffies and xtime. */ +static u64 processed_system_time;/* System time (ns) at last processing. */ +static DEFINE_PER_CPU(u64, processed_system_time);
Errr. That would confuse me right off. Global and per-cpu values having the same name?
drivers/xen/core/time.c line 50
Quoted 3 lines
drivers/xen/core/time.c new file +3
-0+/* How much CPU time was spent blocked and how much was 'stolen'? */ +static DEFINE_PER_CPU(u64, processed_stolen_time); +static DEFINE_PER_CPU(u64, processed_blocked_time);
These seem like more generic accounting structures. Surely other virtualized arches have something similar? Something that should be looked into.
Quoted 49 lines
drivers/xen/core/time.c new file +49
-0+/* Current runstate of each CPU (updated automatically by the hypervisor). */ +static DEFINE_PER_CPU(struct vcpu_runstate_info, runstate); + +/* Must be signed, as it's compared with s64 quantities which can be -ve. */ +#define NS_PER_TICK (1000000000LL/HZ) + +/* +* Reads a consistent set of time-base values from Xen, into a shadow data +* area. +*/ +static void get_time_values_from_xen (void ) +{ +struct shared_info *s = HYPERVISOR_shared_info; +struct vcpu_time_info *src; +struct shadow_time_info *dst; + + src = &s->vcpu_info[smp_processor_id ()].time; + dst = &per_cpu (shadow_time,smp_processor_id ()); + +do { + dst->version = src->version; +rmb (); + dst->tsc_timestamp = src->tsc_timestamp; + dst->system_timestamp = src->system_time; + dst->tsc_to_nsec_mul = src->tsc_to_system_mul; + dst->tsc_shift = src->tsc_shift; +rmb (); + }while ((src->version &1 ) | (dst->version ^ src->version)); + + dst->tsc_to_usec_mul = dst->tsc_to_nsec_mul /1000 ; +} + +static inline int time_values_up_to_date (int cpu) +{ +struct vcpu_time_info *src; +struct shadow_time_info *dst; + + src = &HYPERVISOR_shared_info->vcpu_info[cpu].time; + dst = &per_cpu (shadow_time, cpu); + +rmb (); +return (dst->version == src->version); +} + +/* +* Scale a 64-bit delta by scaling and multiplying by a 32-bit fraction, +* yielding a 64-bit result. +*/ +static inline u64 scale_delta (u64 delta,u32 mul_frac,int shift)
drivers/xen/core/time.c line 110
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+{ +u64 product; +#ifdef __i386__ +u32 tmp1, tmp2; +#endif + +if (shift <0 ) + delta >>= -shift; +else + delta <<= shift;
I think there is a shift_right() macro that can avoid this.
Also I'm not sure I follow why you shift before multiply instead of multiply before shift? Does that not hurt your precision?
Quoted 11 lines
drivers/xen/core/time.c new file +11
-0+#ifdef __i386__ + __asm__ ( +"mul %5 ; " +"mov %4,%%eax ; " +"mov %%edx,%4 ; " +"mul %5 ; " +"xor %5,%5 ; " +"add %4,%%eax ; " +"adc %5,%%edx ; " + :"=A" (product),"=r" (tmp1),"=r" (tmp2) + :"a" ((u32 )delta),"1" ((u32)(delta >>32 )),"2" (mul_frac) );
drivers/xen/core/time.c line 132
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+#elif __x86_64__ + __asm__ ( +"mul %%rdx ; shrd $32,%%rdx,%%rax" + :"=a" (product) :"0" (delta),"d" ((u64 )mul_frac) ); +#else +#error implement me! +#endif + +return product; +}
I think we need some generic mul_llxl_ll() wrappers here.
drivers/xen/core/time.c line 140
Quoted 8 lines
drivers/xen/core/time.c new file +8
-0+ +static u64 get_nsec_offset (struct shadow_time_info *shadow) +{ +u64 now, delta; +rdtscll (now); + delta = now - shadow->tsc_timestamp; +return scale_delta (delta, shadow->tsc_to_nsec_mul, shadow->tsc_shift); +}
get_nsec_offset is a little generic for a name. I know xen_ prefixes everywhere are irritating, but maybe something a little more specific would be a good idea.
Quoted 86 lines
drivers/xen/core/time.c new file +86
-0+ +void do_timer_interrupt_hook (struct pt_regs *regs) +{ +s64 delta, delta_cpu, stolen, blocked; +u64 sched_time; +int i, cpu =smp_processor_id (); +struct shadow_time_info *shadow = &per_cpu (shadow_time, cpu); +struct vcpu_runstate_info *runstate = &per_cpu (runstate, cpu); + +do { +get_time_values_from_xen (); + +/* Obtain a consistent snapshot of elapsed wallclock cycles. */ + delta = delta_cpu = + shadow->system_timestamp +get_nsec_offset (shadow); + delta -= processed_system_time; + delta_cpu -=per_cpu (processed_system_time, cpu); + +/* +* Obtain a consistent snapshot of stolen/blocked cycles. We +* can use state_entry_time to detect if we get preempted here. +*/ +do { + sched_time = runstate->state_entry_time; +barrier (); + stolen = runstate->time[RUNSTATE_runnable] + + runstate->time[RUNSTATE_offline] - +per_cpu (processed_stolen_time, cpu); + blocked = runstate->time[RUNSTATE_blocked] - +per_cpu (processed_blocked_time, cpu); +barrier (); + }while (sched_time != runstate->state_entry_time); + }while (!time_values_up_to_date (cpu)); + +if ((unlikely (delta < -(s64 )permitted_clock_jitter) || +unlikely (delta_cpu < -(s64 )permitted_clock_jitter)) + &&printk_ratelimit ()) { +printk ("Timer ISR/%d: Time went backwards: " +"delta=%lld delta_cpu=%lld shadow=%lld " +"off=%lld processed=%lld cpu_processed=%lld\n" , + cpu, delta, delta_cpu, shadow->system_timestamp, + (s64 )get_nsec_offset (shadow), + processed_system_time, +per_cpu (processed_system_time, cpu)); +for (i =0 ; i <num_online_cpus (); i++) +printk (" %d: %lld\n" , i, +per_cpu (processed_system_time, i)); + } + +/* System-wide jiffy work. */ +while (delta >=NS_PER_TICK ) { + delta -=NS_PER_TICK ; + processed_system_time +=NS_PER_TICK ; +do_timer (regs); + } +/* +* Account stolen ticks. +* HACK: Passing NULL to account_steal_time() +* ensures that the ticks are accounted as stolen. +*/ +if ((stolen >0 ) && (delta_cpu >0 )) { + delta_cpu -= stolen; +if (unlikely (delta_cpu <0 )) + stolen += delta_cpu;/* clamp local-time progress */ +do_div (stolen,NS_PER_TICK ); +per_cpu (processed_stolen_time, cpu) += stolen *NS_PER_TICK ; +per_cpu (processed_system_time, cpu) += stolen *NS_PER_TICK ; +account_steal_time (NULL , (cputime_t )stolen); + } + +/* +* Account blocked ticks. +* HACK: Passing idle_task to account_steal_time() +* ensures that the ticks are accounted as idle/wait. +*/ +if ((blocked >0 ) && (delta_cpu >0 )) { + delta_cpu -= blocked; +if (unlikely (delta_cpu <0 )) + blocked += delta_cpu;/* clamp local-time progress */ +do_div (blocked,NS_PER_TICK ); +per_cpu (processed_blocked_time, cpu) += blocked *NS_PER_TICK ; +per_cpu (processed_system_time, cpu) += blocked *NS_PER_TICK ; +account_steal_time (idle_task (cpu), (cputime_t )blocked); + } + +update_process_times (user_mode_vm (regs));
drivers/xen/core/time.c line 237
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+} + +static cycle_t xen_clocksource_read (void ) +{ +struct shadow_time_info *shadow = &per_cpu (shadow_time,smp_processor_id ()); + +get_time_values_from_xen (); + +return shadow->system_timestamp +get_nsec_offset (shadow); +}
Does get_time_values_from_xen() really need to be called on every clocksource_read call?
Quoted 41 lines
drivers/xen/core/time.c new file +41
-0+static void xen_get_wallclock (struct timespec *ts) +{ +const struct shared_info *s = HYPERVISOR_shared_info; +u32 version; +u64 delta; +struct timespec now; + +/* get wallclock at system boot */ +do { + version = s->wc_version; +rmb (); + now.tv_sec = s->wc_sec; + now.tv_nsec = s->wc_nsec; +rmb (); + }while ((s->wc_version &1 ) | (version ^ s->wc_version)); + + delta =xen_clocksource_read ();/* time since system boot */ + delta += now.tv_sec * (u64 )NSEC_PER_SEC + now.tv_nsec; + + now.tv_nsec =do_div (delta,NSEC_PER_SEC ); + now.tv_sec = delta; + +set_normalized_timespec (ts, now.tv_sec, now.tv_nsec); +} + +unsigned long mach_get_cmos_time (void ) +{ +struct timespec ts; + +xen_get_wallclock (&ts); + +return ts.tv_sec; +} + +int mach_set_rtc_mmss (unsigned long now) +{ +/* do nothing for domU */ +return -1 ; +} + +static void init_cpu_khz (void )
drivers/xen/core/time.c line 289
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+{ +u64 __cpu_khz =1000000ULL <<32 ; +struct vcpu_time_info *info; + info = &HYPERVISOR_shared_info->vcpu_info[0 ].time; +do_div (__cpu_khz, info->tsc_to_system_mul); +if (info->tsc_shift <0 ) + cpu_khz = __cpu_khz << -info->tsc_shift; +else + cpu_khz = __cpu_khz >> info->tsc_shift; +}
Err.. That could use some comments.
drivers/xen/core/time.c line 299
Quoted 9 lines
drivers/xen/core/time.c new file +9
-0+static struct clocksource xen_clocksource = { + .name ="xen" , + .rating =400 , + .read = xen_clocksource_read, + .mask = ~0 , + .mult =1 ,/* time directly in nanoseconds */ + .shift =0 , + .is_continuous =1 +};
Hmmm. The 1/0 mul/shift pair is interesting. Is it expected that NTP does not ever adjust this clocksource? If not the clocksource_adjust() function won't do well with this at all, so you might consider something like:
#define XEN_SHIFT 22
.mult = 1<<XEN_SHIFT
.shift = XEN_SHIFT
Quoted 6 lines
drivers/xen/core/time.c new file +6
-0+static void init_missing_ticks_accounting (int cpu) +{ +struct vcpu_register_runstate_memory_area area; +struct vcpu_runstate_info *runstate = &per_cpu (runstate, cpu); + +memset (runstate,0 ,sizeof (*runstate));
drivers/xen/core/time.c line 316
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+ + area.addr.v = runstate; +HYPERVISOR_vcpu_op (VCPUOP_register_runstate_memory_area, cpu, &area); + +per_cpu (processed_blocked_time, cpu) = + runstate->time[RUNSTATE_blocked]; +per_cpu (processed_stolen_time, cpu) = + runstate->time[RUNSTATE_runnable] + + runstate->time[RUNSTATE_offline]; +}
Again, this accounting seems like it could be generically useful.
Quoted 13 lines
drivers/xen/core/time.c new file +13
-0+__init void time_init_hook (void ) +{ +get_time_values_from_xen (); + + processed_system_time =per_cpu (shadow_time,0 ).system_timestamp; +per_cpu (processed_system_time,0 ) = processed_system_time; + +init_cpu_khz (); +printk (KERN_INFO "Xen reported: %u.%03u MHz processor.\n" , + cpu_khz /1000 , cpu_khz %1000 ); + +/* Cannot request_irq() until kmem is initialised. */ + late_time_init = setup_cpu0_timer_irq;
drivers/xen/core/time.c line 362
Quoted 10 lines
drivers/xen/core/time.c new file +10
-0+ +init_missing_ticks_accounting (0 ); + +clocksource_register (&xen_clocksource); + +/* Set initial system time with full resolution */ +xen_get_wallclock (&xtime); +set_normalized_timespec (&wall_to_monotonic, + -xtime.tv_sec, -xtime.tv_nsec); +}
Some mention of which functions require to hold what on xtime_lock would be useful as well (applies to this function as well as the previous ones already commented on).
My only thoughts after looking at it: Using nanoseconds as a primary unit is often easier to work with, but less efficient. So rather then keeping a tsc_timestamp + system_timestamp in two different units, why not keep a calculated TSC base that includes the "cycles since boot" which is adjusted in the same manner internally to Xen as the system_timestamp is. Then let the timekeeping code do the conversion for you.
I haven't fully thought about what else it would affect in the above (I realize stolen_time, etc is in nsecs), but it might be something to consider.
Am I making any sense or just babbling?
thanks
-john
Re: [RFC PATCH 25/33] Implement timekeeping for Xen
Hi John,
I was hoping to catch up with you at OLS to talk about this, but, well, you know...
Quoted 6 lines · john stultz
Interesting. The Andi has been bugging me for a similarly designed per-cpu TSC clocksource, but just for generic use. I'm a little skeptical that it will be 100% without error, since anything dealing w/ the TSCs have been nothing but trouble in my mind, but this looks like a good proving ground for the concept.
I haven't had much direct experience with it, but Ian Pratt says it has been pretty successful in practice. The big problem has been CPUs (typically AMD) which drop into a thermal throttling state without telling anyone (no interrupt, etc). Xen doesn't really support CPU speed switching yet, so that hasn't been a problem.
Quoted 6 lines
It was mentioned to me that the clocksource approach helped cleanup some of the xen time changes (is that really true? :), but there were still some outstanding issues (time inconsistencies, perhaps?). I'm just curious if there are any details about the issues there, or if I misunderstood?
No, the time rework did make it much easier to plug Xen in. The original Xen time patch was basically a complete copy of arch/i386/kernel/time.c with additional hacks. That patch did do more than this once, since I was primarily aiming for basically functional & clean rather than fully functional, on the assumption that the patch would generate this discussion so we could sort out the rest of the issues.
This Xen time patch deals with:
* setting the system clock at boot
* ticks
* lost & stolen ticks
* tickless idle
In addition to this, we would also like to be able to absolutely slave the guest time to the hypervisor time, so that even if the hypervisor time changes there still a very small (or ideally 0) skew between the guest time.
Quoted 5 lines
Quoted 4 lines
This does not implement setting the hypervisor clock; nor does it implement non-independent time, so hypervisor wallclock changes will not affect the guest.
Quoted 8 lines
Hmmm. I'm not sure if I understood that last line or not. I guess I need to think a bit about
CLOCK_REALTIMEvsCLOCK_MONOTONICwrt hypervisiors.I guess the question is "who owns time?" the guest OS (does it have its own
CLOCK_REALTIME, independent of other guests?) or does the hypervisor? What does NTPd running on a guest actually adjust?
In Xen there are two types of guest domain: dom0 and domU. Dom0 (of which there can be only one) can perform privileged hypervisor operations, has direct access to hardware, etc; only dom0 can change hypervisor time. DomU domains are typically not privileged, only have virtual devices, etc. With respect to time, domains can either operate with "dependent time" or "independent time".
With independent time, a guest maintains its own notion of time. It may be initially set from the hypervisor, or it might use something else, like ntp. It may also choose to use the hypervisor as a clock source, or it can use any other clock. This is the easy case, since its the same as a stand-alone kernel.
With dependent time, the guest time is slaved to the hypervisor time. At any point in time, all dependent-time guests should have a very small (ideally 0) skew between each other and the hypervisor itself, regardless of how the hypervisor time changes. It would never make sense to use ntp or a non-hypervisor clocksource in a dependent-time guest. settimeofday and adjtimex would be null operations (I'm not sure if it makes sense to have a dependent-time dom0).
There are two problems in implementing this at present. The first is that the clock abstractions have nicely moved all the maintenance of the system time of day into the core, with no need for the arch-specific code to know about it. This is nice, but it isn't clear to me how we'd implement a dependent-time guest.
The second problem is that at present the only way to set the hypervisor time is with a simple stepwise settimeofday interface, rather than a time-warping adjtimex interface. This means that there's at least time potential for a other guests with slaved clocks could see non-monotonic time. We probably need to have a full time implementation of a clock within Xen itself, so that the dom0 adjtimex calls just call down into it rather than really maintaining a local clock.
The added complication is that we don't want to make a real hypercall into the hypervisor every time to fetch the time. To avoid this there is a shared memory region mapped from the hypervisor into the guests with contains periodically updated time information. This is updated by the hypervisor timer interrupt (100Hz?), and so is fairly low precision. To get more precise time, the guests extrapolate from this using the tsc. The obvious trouble with this extrapolation is that if the hypervisor time slows down, the guest could see non-monotonic time because their extrapolation over-shoots the real hypervisor tick-to-tick time step.
Also, we had quite a few discussions at OLS about introducing a general hypervisor interface layer, and how to handle time is obviously a part of that. I would imagine that most of this is a common problem to any hypervisor-based system rather than something specific to Xen (though obviously the details might vary).
Quoted 5 lines
Quoted 5 lines
drivers/xen/core/time.c new file +5
-0+/* Permitted clock jitter, in nsecs, beyond which a warning will be printed. */ +static unsigned long permitted_clock_jitter =10000000UL ;/* 10ms */ +static int __init __permitted_clock_jitter (char *str) +{ + permitted_clock_jitter =simple_strtoul (str,NULL ,0 );
Quoted 9 lines
drivers/xen/core/time.c line 29
Quoted 4 lines
drivers/xen/core/time.c new file +3
-0+return 1 ; +} +__setup ("permitted_clock_jitter=" , __permitted_clock_jitter);
permitted_clock_jitteris a little vague and might get confused w/ the NTP notion of jitter. Is there a better name, or could we get axen_prefix there?
Sure. I copied this over without really looking into it deeply, so I need to work out what this really means. Would "permitted_backstep" might be a better name? Adding "xen_" is a no-brainer though, though it might be something which will be common to all systems with virtualized time.
Quoted 5 lines
Quoted 5 lines
drivers/xen/core/time.c new file +5
-0+/* These are perodically updated in shared_info, and then copied here. */ +struct shadow_time_info { +u64 tsc_timestamp;/* TSC at last update of time vals. */ +u64 system_timestamp;/* Time, in nanosecs, since boot. */ +u32 tsc_to_nsec_mul;
Quoted 9 lines
drivers/xen/core/time.c line 37
Quoted 2 lines
drivers/xen/core/time.c new file +1
-0+u32 tsc_to_usec_mul;Hmmm. Keeping separate
cycle->usecandcycle->nsecmultipliers is an interesting optimization. I'd even consider it for the generic clocksource code, but I suspect recalculating the independent adjustment factors for both kills the performance benefit. Have you actually compaired against the cost of the /1000 going from nsec to usec?
This is also something I copied over. It wasn't obvious to me that it would be a big win either. In fact, this patch doesn't use tsc_to_usec at all, so its completely redundant.
Quoted 6 lines
drivers/xen/core/time.c line 39
Quoted 3 lines
drivers/xen/core/time.c new file +2
-0+int tsc_shift; +u32 version;Errr.. Why is a version value necessary?
This structure is derived from the time structure Xen maintains in shared memory. If the shared memory version matches the local shadow version, we don't need to sync the two (I'm not sure if this is actually used in this patch).
Quoted 11 lines
drivers/xen/core/time.c line 46
Quoted 7 lines
drivers/xen/core/time.c new file +6
-0+ +static DEFINE_PER_CPU(struct shadow_time_info, shadow_time); + +/* Keep track of last time we did processing/updating of jiffies and xtime. */ +static u64 processed_system_time;/* System time (ns) at last processing. */ +static DEFINE_PER_CPU(u64, processed_system_time);Errr. That would confuse me right off. Global and per-cpu values having the same name?
Yes, I was wondering about that myself.
Quoted 9 lines
drivers/xen/core/time.c line 50
Quoted 4 lines
drivers/xen/core/time.c new file +3
-0+/* How much CPU time was spent blocked and how much was 'stolen'? */ +static DEFINE_PER_CPU(u64, processed_stolen_time); +static DEFINE_PER_CPU(u64, processed_blocked_time);These seem like more generic accounting structures. Surely other virtualized arches have something similar? Something that should be looked into.
Yes.
Quoted 8 lines
drivers/xen/core/time.c line 110
Quoted 5 lines
drivers/xen/core/time.c new file +4
-0+if (shift <0 ) + delta >>= -shift; +else + delta <<= shift;I think there is a
shift_right()macro that can avoid this.
OK.
Quoted 3 lines
Also I'm not sure I follow why you shift before multiply instead of multiply before shift? Does that not hurt your precision?
It would seem so. I'm not sure what the original thought was here.
Quoted 16 lines
Quoted 16 lines
drivers/xen/core/time.c new file +16
-0+#ifdef __i386__ + __asm__ ( +"mul %5 ; " +"mov %4,%%eax ; " +"mov %%edx,%4 ; " +"mul %5 ; " +"xor %5,%5 ; " +"add %4,%%eax ; " +"adc %5,%%edx ; " + :"=A" (product),"=r" (tmp1),"=r" (tmp2) + :"a" ((u32 )delta),"1" ((u32)(delta >>32 )),"2" (mul_frac) ); +#elif __x86_64__ + __asm__ ( +"mul %%rdx ; shrd $32,%%rdx,%%rax" + :"=a" (product) :"0" (delta),"d" ((u64 )mul_frac) ); +#else
Quoted 9 lines
drivers/xen/core/time.c line 132
Quoted 6 lines
drivers/xen/core/time.c new file +5
-0+#error implement me! +#endif + +return product; +}I think we need some generic
mul_llxl_ll()wrappers here.
OK.
Quoted 7 lines
Review
Quoted 3 lines
(quoted diff) +2
-0+ +static u64 get_nsec_offset(struct shadow_time_info *shadow)
get_nsec_offsetis a little generic for a name. I knowxen_prefixes everywhere are irritating, but maybe something a little more specific would be a good idea.
It's static, so it isn't affecting the global namespace. Do you just mean in terms of making it easy to find with tags or backtraces?
Quoted 4 lines
Quoted 4 lines
drivers/xen/core/time.c new file +4
-0+static cycle_t xen_clocksource_read (void ) +{ +struct shadow_time_info *shadow = &per_cpu (shadow_time,smp_processor_id ()); +
Quoted 9 lines
drivers/xen/core/time.c line 237
Quoted 5 lines
drivers/xen/core/time.c new file +4
-0+get_time_values_from_xen (); + +return shadow->system_timestamp +get_nsec_offset (shadow); +}Does
get_time_values_from_xen()really need to be called on everyclocksource_readcall?
In principle it shouldn't cost much, if anything. Hm, it doesn't look like it uses the version-comparison optimisation, but even without that it just copies some values with a low likelihood of needing to iterate. The version-comparison test would eliminate the tsc_to_usec divide as well (though that's redundant anyway).
We could call it less and rely on longer extrapolations of time, but I'm not sure it's worth it when traded against the possibility of non-monotonicity.
Quoted 6 lines
Quoted 6 lines
drivers/xen/core/time.c new file +6
-0+static void init_cpu_khz (void ) +{ +u64 __cpu_khz =1000000ULL <<32 ; +struct vcpu_time_info *info; + info = &HYPERVISOR_shared_info->vcpu_info[0 ].time; +do_div (__cpu_khz, info->tsc_to_system_mul);
Quoted 9 lines
drivers/xen/core/time.c line 289
Quoted 6 lines
drivers/xen/core/time.c new file +5
-0+if (info->tsc_shift <0 ) + cpu_khz = __cpu_khz << -info->tsc_shift; +else + cpu_khz = __cpu_khz >> info->tsc_shift; +}Err.. That could use some comments.
Yep.
Quoted 11 lines
drivers/xen/core/time.c line 299
Quoted 10 lines
drivers/xen/core/time.c new file +9
-0+static struct clocksource xen_clocksource = { + .name ="xen" , + .rating =400 , + .read = xen_clocksource_read, + .mask = ~0 , + .mult =1 ,/* time directly in nanoseconds */ + .shift =0 , + .is_continuous =1 +};
Quoted 8 lines
Hmmm. The 1/0 mul/shift pair is interesting. Is it expected that NTP does not ever adjust this clocksource? If not the
clocksource_adjust()function won't do well with this at all, so you might consider something like:
#defineXEN_SHIFT22
.mult = 1<<XEN_SHIFT
.shift =XEN_SHIFT
OK. I added the 1/0 pair without really thinking about the implications for ntp. It does make sense to use ntp, so I'll fix that.
Quoted 11 lines
Quoted 11 lines
drivers/xen/core/time.c new file +11
-0+static void init_missing_ticks_accounting (int cpu) +{ +struct vcpu_register_runstate_memory_area area; +struct vcpu_runstate_info *runstate = &per_cpu (runstate, cpu); + +memset (runstate,0 ,sizeof (*runstate)); + + area.addr.v = runstate; +HYPERVISOR_vcpu_op (VCPUOP_register_runstate_memory_area, cpu, &area); + + per_cpu(processed_blocked_time, cpu) =
Quoted 9 lines
drivers/xen/core/time.c line 316
Quoted 6 lines
drivers/xen/core/time.c new file +5
-0+ runstate->time[RUNSTATE_blocked]; +per_cpu (processed_stolen_time, cpu) = + runstate->time[RUNSTATE_runnable] + + runstate->time[RUNSTATE_offline]; +}Again, this accounting seems like it could be generically useful.
OK.
Quoted 20 lines
Quoted 20 lines
drivers/xen/core/time.c new file +20
-0+__init void time_init_hook (void ) +{ +get_time_values_from_xen (); + + processed_system_time =per_cpu (shadow_time,0 ).system_timestamp; +per_cpu (processed_system_time,0 ) = processed_system_time; + +init_cpu_khz (); +printk (KERN_INFO "Xen reported: %u.%03u MHz processor.\n" , + cpu_khz /1000 , cpu_khz %1000 ); + +/* Cannot request_irq() until kmem is initialised. */ + late_time_init = setup_cpu0_timer_irq; + +init_missing_ticks_accounting (0 ); + +clocksource_register (&xen_clocksource); + +/* Set initial system time with full resolution */ +xen_get_wallclock (&xtime);
Quoted 9 lines
drivers/xen/core/time.c line 362
Quoted 4 lines
drivers/xen/core/time.c new file +3
-0+set_normalized_timespec (&wall_to_monotonic, + -xtime.tv_sec, -xtime.tv_nsec); +}Some mention of which functions require to hold what on
xtime_lockwould be useful as well (applies to this function as well as the previous ones already commented on).
This is called from time_init(), which sets xtime without holding the lock. I originally took the lock here, but removed it when I noticed that time_init() didn't bother.
Quoted 8 lines
My only thoughts after looking at it: Using nanoseconds as a primary unit is often easier to work with, but less efficient. So rather then keeping a
tsc_timestamp+system_timestampin two different units, why not keep a calculated TSC base that includes the "cycles since boot" which is adjusted in the same manner internally to Xen as thesystem_timestampis. Then let the timekeeping code do the conversion for you.
It's worth considering; we'll need to consider how that changes Xen's interface, but I think we'll need to look at that anyway.
Quoted 5 lines
I haven't fully thought about what else it would affect in the above (I realize
stolen_time, etc is in nsecs), but it might be something to consider.Am I making any sense or just babbling?
Definitely makes sense. And I'd like to know your thoughts about how we can take more direct control of the system wallclock in a clean and sensible manner (or perhaps we shouldn't; maybe the right answer is that all the guests run an ntp server pointing at dom0, though that has its own downsides).
J
RFC PATCH 26/33 subarch suport for idle loop (NO_IDLE_HZ for Xen)
In reply to Chris WrightParavirtualize the idle loop to explicitly trap to the hypervisor when blocking, and to use the NO_IDLE_HZ functionality introduced by s390 to inform the rcu subsystem that the CPU is quiescent.
--- arch/i386/mach-xen/setup-xen.c | 59 ++++++++++++++++++++++++++++++++++ drivers/xen/Kconfig | 8 ++++ drivers/xen/core/time.c | 21 ++++++++++++ include/asm-i386/mach-xen/mach_time.h | 3 + 4 files changed, 91 insertions(+)
arch/i386/mach-xen/setup-xen.c +59 -0
@@ -7,6 +7,7 @@#include <linux/mm.h> #include <linux/bootmem.h> #include <linux/initrd.h> +#include <linux/pm.h> #include <asm/e820.h> #include <asm/setup.h>
@@ -15,6 +16,9 @@#include <xen/interface/physdev.h> +#include "mach_time.h" + +static void xen_idle (void );struct start_info *xen_start_info;EXPORT_SYMBOL (xen_start_info);
@@ -82,4 +86,59 @@ void __init machine_specific_arch_setup( initrd_reserve_bootmem =0 ; initrd_below_start_ok =1 ; } + + + pm_idle = xen_idle; } + +/* +* stop_hz_timer / start_hz_timer - enter/exit 'tickless mode' on an idle cpu +* These functions are based on implementations from arch/s390/kernel/time.c +*/ +static void stop_hz_timer (void ) +{ +unsigned int cpu =smp_processor_id (); +unsigned long j; + +cpu_set (cpu, nohz_cpu_mask); + +/* See matching smp_mb in rcu_start_batch in rcupdate.c. These mbs */ +/* ensure that if __rcu_pending (nested in rcu_needs_cpu) fetches a */ +/* value of rcp->cur that matches rdp->quiescbatch and allows us to */ +/* stop the hz timer then the cpumasks created for subsequent values */ +/* of cur in rcu_start_batch are guaranteed to pick up the updated */ +/* nohz_cpu_mask and so will not depend on this cpu. */ + +smp_mb (); + +/* Leave ourselves in 'tick mode' if rcu or softirq or timer pending. */ +if (rcu_needs_cpu (cpu) ||local_softirq_pending () || + (j =next_timer_interrupt (),time_before_eq (j, jiffies))) { +cpu_clear (cpu, nohz_cpu_mask); + j = jiffies +1 ; + } + +BUG_ON (HYPERVISOR_set_timer_op (jiffies_to_st (j)) !=0 ); +} + +static void start_hz_timer (void ) +{ +cpu_clear (smp_processor_id (), nohz_cpu_mask); +} + +static void xen_idle (void ) +{ +local_irq_disable (); + +if (need_resched ()) +local_irq_enable (); +else { +current_thread_info ()->status &= ~TS_POLLING ; +smp_mb__after_clear_bit (); +stop_hz_timer (); +/* Blocking includes an implicit local_irq_enable(). */ +HYPERVISOR_sched_op (SCHEDOP_block,0 ); +start_hz_timer (); +current_thread_info ()->status |=TS_POLLING ; + } +}
drivers/xen/Kconfig +8 -0
@@ -12,6 +12,14 @@ config XEN if XEN +config NO_IDLE_HZ + bool + default y + help + Switches the regular HZ timer off when the system is going idle. + This helps Xen to detect that the Linux system is idle, reducing + the overhead of idle systems. + config XEN_SHADOW_MODE bool default y
drivers/xen/core/time.c +21 -0
@@ -227,6 +227,27 @@ void do_timer_interrupt_hook(struct pt_rupdate_process_times (user_mode_vm (regs)); } + +/* Convert jiffies to Xen system time. */ +u64 jiffies_to_st (unsigned long j) +{ +unsigned long seq; +long delta; +u64 st; + +do { + seq =read_seqbegin (&xtime_lock); + delta = j - jiffies; +/* NB. The next check can trigger in some wrap-around cases, +* but that's ok: we'll just end up with a shorter timeout. */ +if (delta <1 ) + delta =1 ; + st = processed_system_time + (delta * (u64 )NS_PER_TICK ); + }while (read_seqretry (&xtime_lock, seq)); + +return st; +} +static cycle_t xen_clocksource_read (void ) {struct shadow_time_info *shadow = &per_cpu (shadow_time,smp_processor_id ());
include/asm-i386/mach-xen/mach_time.h +3 -0
@@ -13,4 +13,7 @@ int mach_set_rtc_mmss(unsigned long nowtint mach_set_rtc_mmss (unsigned long nowtime);unsigned long mach_get_cmos_time (void ); +/* Convert jiffies to Xen system time. */ +u64 jiffies_to_st (unsigned long j); +#endif /* !_MACH_TIME_H */
--
RFC PATCH 27/33 Add the Xen virtual console driver.
In reply to Chris WrightThis provides a bootstrap and ongoing emergency console which is intended to be available from very early during boot and at all times thereafter, in contrast with alternatives such as UDP-based syslogd, or logging in via ssh. The protocol is based on a simple shared-memory ring buffer.
--- drivers/char/Kconfig | 1 drivers/xen/Makefile | 1 drivers/xen/console/Makefile | 2 drivers/xen/console/console.c | 622 ++++++++++++++++++++++++++++++++++++ drivers/xen/console/xencons_ring.c | 141 ++++++++ include/xen/xencons.h | 14 6 files changed, 781 insertions(+)
drivers/char/Kconfig +1 -0
@@ -6,6 +6,7 @@ menu "Character devices" config VT bool "Virtual terminal" if EMBEDDED + depends on !XEN select INPUT default y if !VIOCONS ---help---
drivers/xen/Makefile +1 -0
@@ -1,3 +1,4 @@ obj-y += core/ +obj-y += console/
arch/i386/kernel/early_printk.c +2 -0
@@ -1,2 +1,4 @@ +#ifndef CONFIG_XEN #include "../../x86_64/kernel/early_printk.c" +#endif
drivers/xen/console/Makefile new file +2 -0
@@ -0,0 +1,2 @@ + +obj-y := console.o xencons_ring.o
drivers/xen/console/console.c new file +663 -0
663 lines not on this page · raw message
drivers/xen/console/xencons_ring.c new file +141 -0
141 lines not on this page · raw message
include/xen/xencons.h new file +14 -0
@@ -0,0 +1,14 @@ +#ifndef __ASM_XENCONS_H__ +#define __ASM_XENCONS_H__ + +void xencons_force_flush (void ); +void xencons_resume (void ); + +/* Interrupt work hooks. Receive data, or kick data out. */ +void xencons_rx (char *buf,unsigned len,struct pt_regs *regs); +void xencons_tx (void ); + +int xencons_ring_init (void ); +int xencons_ring_send (const char *data,unsigned len); + +#endif /* __ASM_XENCONS_H__ */
--
Re: [RFC PATCH 27/33] Add the Xen virtual console driver.
drivers/xen/console/console.c line 330
Quoted 13 lines · Chris Wright
drivers/xen/console/console.c new file +13
-0+ }else if (sysrq_requested) { +unsigned long sysrq_timeout = + sysrq_requested +HZ *2 ; + sysrq_requested =0 ; +if (time_before (jiffies, sysrq_timeout)) { +spin_unlock_irqrestore ( + &xencons_lock, flags); +handle_sysrq ( + buf[i], regs, xencons_tty); +spin_lock_irqsave ( + &xencons_lock, flags); +continue ; + }
Lindent can be harmful...
Quoted 8 lines
drivers/xen/console/console.c new file +8
-0+static void xencons_wait_until_sent (struct tty_struct *tty,int timeout) +{ +unsigned long orig_jiffies = jiffies; + +if (TTY_INDEX (tty) !=0 ) +return ; + +while (DRV (tty->driver)->chars_in_buffer (tty)) {
drivers/xen/console/console.c line 537
Quoted 10 lines
drivers/xen/console/console.c new file +10
-0+set_current_state (TASK_INTERRUPTIBLE ); +schedule_timeout (1 ); +if (signal_pending (current)) +break ; +if (timeout &&time_after (jiffies, orig_jiffies + timeout)) +break ; + } + +set_current_state (TASK_RUNNING ); +}
hmm somehow I find this code scary; we had similar code recently elsewhere where this turned out to be a real issue; you now sleep for "1" time, so you sleep for a fixed time if you aren't getting wakeups, but if you are getting wakeups your code is upside down, I would expect it to look like
+ set_current_state(TASK_INTERRUPTIBLE);
+ while (DRV(tty->driver)->chars_in_buffer(tty))
+ schedule_timeout(1);
+ if (signal_pending(current))
+ break;
+ if (timeout && time_after(jiffies, orig_jiffies + timeout))
+ break;
+ set_current_state(TASK_INTERRUPTIBLE);
+ }
instead, so that you don't have the wakeup race..
Re: [RFC PATCH 27/33] Add the Xen virtual console driver.
Quoted 16 lines · Arjan van de Ven
hmm somehow I find this code scary; we had similar code recently elsewhere where this turned out to be a real issue; you now sleep for "1" time, so you sleep for a fixed time if you aren't getting wakeups, but if you are getting wakeups your code is upside down, I would expect it to look like
drivers/xen/console/console.c new file +9
-0+set_current_state (TASK_INTERRUPTIBLE ); +while (DRV (tty->driver)->chars_in_buffer (tty)) +schedule_timeout (1 ); +if (signal_pending (current)) +break ; +if (timeout &&time_after (jiffies, orig_jiffies + timeout)) +break ; +set_current_state (TASK_INTERRUPTIBLE ); + }
Quoted 1 line
instead, so that you don't have the wakeup race..
There's no wakeup signal, so no possibility of a wakeup race. That's why we schedule_timeout() instead of wait_event() or similar. This code is only used to flush the console when the kernel crashes, so we can get the full oops, so waiting a little bit too long is acceptable.
Your suggested change is perhaps more idiomatic though, and less jarring for reviewers. :-)
Thanks for your comments by the way. Reviewing lots of patches isn't much fun.
-- Keir
Re: [RFC PATCH 27/33] Add the Xen virtual console driver.
In reply to Chris WrightQuoted 9 lines · Chris Wright
plain text document attachment (xen-console)
This provides a bootstrap and ongoing emergency console which is intended to be available from very early during boot and at all times thereafter, in contrast with alternatives such as UDP-based syslogd, or logging in via ssh. The protocol is based on a simple shared-memory ring buffer.+/*
+ * Modes:
Quoted 8 lines
+ * 'xencons=off' [XC_OFF]: Console is disabled. + * 'xencons=tty' [XC_TTY]: Console attached to '/dev/tty[0-9]+'. + * 'xencons=ttyS' [XC_SERIAL]: Console attached to '/dev/ttyS[0-9]+'.+ * [
XC_DEFAULT]: DOM0 ->XC_SERIAL; all others ->XC_TTY.
+ *
+ * NB. In modeXC_TTY, we create dummy consoles for tty2-63. This suppresses
+ * warnings from standard distro startup scripts.
+ */
The console driver should not be hijacking /dev/tty or /dev/ttyS devices. Instead, it should use its own char device, like the pSeries hypervisor console does (/dev/hvc).
Yes, I understand that installers, inittab, and securetty files will need to be updated. Luckily, there are a couple of distros interested in supporting Xen who I suspect will be able to help.
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center
RFC PATCH 28/33 Add Xen grant table support
In reply to Chris WrightAdd Xen 'grant table' driver which allows granting of access to selected local memory pages by other virtual machines and, symmetrically, the mapping of remote memory pages which other virtual machines have granted access to.
This driver is a prerequisite for many of the Xen virtual device drivers, which grant the 'device driver domain' restricted and temporary access to only those memory pages that are currently involved in I/O operations.
--- drivers/xen/core/Makefile | 2 drivers/xen/core/gnttab.c | 422 ++++++++++++++++++++++++++++++++++++++++++++++ include/xen/gnttab.h | 105 +++++++++++ 3 files changed, 528 insertions(+), 1 deletion(-)
drivers/xen/core/Makefile +1 -1
@@ -1,3 +1,3 @@ -obj-y := features.o time.o +obj-y := features.o time.o gnttab.o
drivers/xen/core/gnttab.c new file +426 -0
426 lines not on this page · raw message
include/xen/gnttab.h new file +107 -0
107 lines not on this page · raw message
--
Re: [Xen-devel] [RFC PATCH 28/33] Add Xen grant table support
I don't think that the current grant-table API/implementation quite meets the requirements for revocation.
Specifically I think the requirements are that:
1) When a domain starts to reclaim a set of resources it allocated for inter-domain communication this process is guaranteed to complete without cooperation from any other domain.
2) When a domain reclaims resources it allocated for inter-domain communication it can't cause a page fault or other exception in any other domain.
The current API/implementation meets the second requirement but not the first.
It's possible to make the current implementation meet the first requirement if you relax it a bit and allow the superuser to kill an offending domain via domain 0 but this expands the scope of the failure from a single inter-domain communication channel to a whole domain which is undesirable.
Another potential issue with grant-tables is support for efficient N-way communication but I haven't investigated that personally.
A final point is that quite a lot of fairly tricky code is required to combine grant-tables, event-channels and xenbus before you can create an inter-domain communication channel which can be correctly disconnected and reconnected across module load and unload.
It's appropriate to have a low-level API at the hypervisor because it keeps the hypervisor as small as possible and therefore easier to audit.
But, whatever the low-level API, whether grant-tables or something which has better support for revocation and n-way communication, I think there needs to be a small library to implement a higher level API that is more convenient for driver authors to use directly.
Re: [RFC PATCH 28/33] Add Xen grant table support
In reply to Chris WrightQuoted 16 lines · Chris Wright
drivers/xen/core/gnttab.c new file +15
-0+int gnttab_end_foreign_access_ref (grant_ref_t ref,int readonly) +{ +u16 flags, nflags; + + nflags = shared[ref].flags; +do { +if ((flags = nflags) & (GTF_reading|GTF_writing)) { +printk (KERN_ALERT "WARNING: g.e. still in use!\n" ); +return 0 ; + } + }while ((nflags =synch_cmpxchg (&shared[ref].flags, flags,0 )) != + flags); + +return 1 ; +}
Quoted 1 line
From patch 06/33:
+/*
+ * A grant table comprises a packed array of grant entries in one or more
+ * page frames shared between Xen and a guest.
+ * [XEN]: This field is written by Xen and read by the sharing guest.
+ * [GST]: This field is written by the guest and read by Xen.
+ */
+struct grant_entry {
+ /* GTF_xxx: various type and flag information. [XEN,GST] */ + uint16_t flags; + /* The domain being granted foreign privileges. [GST] */ + domid_t domid; + /*
+ * GTF_permit_access: Frame that @domid is allowed to map and
access. [GST]
+ * GTF_accept_transfer: Frame whose ownership transferred by
@domid. [XEN]
+ */
+ uint32_t frame;
+};
I object to these uses of (synch_)cmpxchg on a uint16_t in common code. Many architectures, including PowerPC, do not support 2-byte atomic operations, but this code is common to all Xen architectures.
Signature · 2 lines
Hollis Blanchard IBM Linux Technology Center
Re: [RFC PATCH 28/33] Add Xen grant table support
Quoted 3 lines · Hollis Blanchard
I object to these uses of (synch_)cmpxchg on a
uint16_tin common code. Many architectures, including PowerPC, do not support 2-byte atomic operations, but this code is common to all Xen architectures.
We'll use synch_cmpxchg_subword() in the next iteration of these patches. It's already been applied to our main Xen repository but hadn't been applied to our merge repo when these patches were created.
-- Keir
Re: [RFC PATCH 28/33] Add Xen grant table support
In reply to Hollis BlanchardQuoted 3 lines
I object to these uses of (synch_)cmpxchg on a
uint16_tin common code. Many architectures, including PowerPC, do not support 2-byte atomic operations, but this code is common to all Xen architectures.
RMW operations you mean, I suppose. On PowerPC, all (naturally aligned) halfword operations are atomic; there do not exist any RMW operations; but word and doubleword atomic RMW operations can be emulated.
Boils down to the same thing of course, but it isn't the same ;-)
Segher
RFC PATCH 29/33 Add Xen driver utility functions.
In reply to Chris WrightAllocate/destroy a 'vmalloc' VM area: alloc_vm_area and free_vm_area The alloc function ensures that page tables are constructed for the region of kernel virtual address space and mapped into init_mm.
Lock an area so that PTEs are accessible in the current address space:lock_vm_area and unlock_vm_area
The lock function prevents context switches to a lazy mm that doesn't have the area mapped into its page tables. It also ensures that the page tables are mapped into the current mm by causing the page fault handler to copy the page directory pointers from init_mm into the current mm.
---
drivers/xen/Makefile | 2 + drivers/xen/util.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++ include/xen/driver_util.h | 16 ++++++++++ 3 files changed, 88 insertions(+)
drivers/xen/Makefile +2 -0
@@ -1,3 +1,5 @@ + +obj-y += util.o obj-y += core/ obj-y += console/
drivers/xen/util.c new file +70 -0
70 lines not on this page · raw message
include/xen/driver_util.h new file +16 -0
@@ -0,0 +1,16 @@ + +#ifndef __ASM_XEN_DRIVER_UTIL_H__ +#define __ASM_XEN_DRIVER_UTIL_H__ + +#include <linux/config.h> +#include <linux/vmalloc.h> + +/* Allocate/destroy a 'vmalloc' VM area. */ +extern struct vm_struct *alloc_vm_area (unsigned long size); +extern void free_vm_area (struct vm_struct *area); + +/* Lock an area so that PTEs are accessible in the current address space. */ +extern void lock_vm_area (struct vm_struct *area); +extern void unlock_vm_area (struct vm_struct *area); + +#endif /* __ASM_XEN_DRIVER_UTIL_H__ */
--
RFC PATCH 30/33 Add the Xenbus sysfs and virtual device hotplug driver.
In reply to Chris WrightThis communicates with the machine control software via a registry residing in a controlling virtual machine. This allows dynamic creation, destruction and modification of virtual device configurations (network devices, block devices and CPUS, to name some examples).
--- drivers/xen/Makefile | 1 drivers/xen/xenbus/Makefile | 7 drivers/xen/xenbus/xenbus_client.c | 528 ++++++++++++++++++++ drivers/xen/xenbus/xenbus_comms.c | 222 ++++++++ drivers/xen/xenbus/xenbus_comms.h | 43 + drivers/xen/xenbus/xenbus_probe.c | 954 +++++++++++++++++++++++++++++++++++++ drivers/xen/xenbus/xenbus_xs.c | 828 ++++++++++++++++++++++++++++++++ include/xen/xenbus.h | 199 +++++++ 8 files changed, 2782 insertions(+)
drivers/xen/Makefile +1 -0
@@ -3,4 +3,5 @@ obj-y += util.o obj-y += core/ obj-y += console/ +obj-y += xenbus/
drivers/xen/xenbus/Makefile new file +7 -0
@@ -0,0 +1,7 @@ +obj-y += xenbus.o + +xenbus-objs = +xenbus-objs += xenbus_client.o +xenbus-objs += xenbus_comms.o +xenbus-objs += xenbus_xs.o +xenbus-objs += xenbus_probe.o
drivers/xen/xenbus/xenbus_client.c new file +528 -0
528 lines not on this page · raw message
drivers/xen/xenbus/xenbus_comms.c new file +222 -0
222 lines not on this page · raw message
drivers/xen/xenbus/xenbus_comms.h new file +43 -0
43 lines not on this page · raw message
drivers/xen/xenbus/xenbus_probe.c new file +954 -0
954 lines not on this page · raw message
drivers/xen/xenbus/xenbus_xs.c new file +828 -0
828 lines not on this page · raw message
include/xen/xenbus.h new file +199 -0
199 lines not on this page · raw message
--
RFC PATCH 31/33 Add Xen subarch reboot support
In reply to Chris WrightAdd remote reboot capability, so that a virtual machine can be rebooted, halted or 'powered off' by external management tools.
--- arch/i386/kernel/Makefile | 1 drivers/xen/core/reboot.c | 232 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 233 insertions(+)
arch/i386/kernel/Makefile +1 -0
@@ -48,6 +48,7 @@ hw_irq-y := i8259.o hw_irq-y := i8259.o hw_irq-$(CONFIG_XEN) := ../../../drivers/xen/core/evtchn.o +reboot-$(CONFIG_XEN) := ../../../drivers/xen/core/reboot.o # vsyscall.o contains the vsyscall DSO images as __initdata. # We must build both images before we can assemble it.
drivers/xen/core/reboot.c new file +232 -0
232 lines not on this page · raw message
--
Re: [RFC PATCH 31/33] Add Xen subarch reboot support
drivers/xen/core/reboot.c line 69
Quoted 8 lines · Chris Wright
...
drivers/xen/core/reboot.c new file +3
-0+EXPORT_SYMBOL (machine_restart); +EXPORT_SYMBOL (machine_halt); +EXPORT_SYMBOL (machine_power_off);
Why?
cu
Adrian
Signature · 4 lines
"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon SeedRFC PATCH 32/33 Add the Xen virtual network device driver.
In reply to Chris WrightThe network device frontend driver allows the kernel to access network devices exported exported by a virtual machine containing a physical network device driver.
---
drivers/net/Kconfig | 2 drivers/xen/Kconfig.net | 14 drivers/xen/Makefile | 3 drivers/xen/net_driver_util.c | 58 + drivers/xen/netfront/Makefile | 4 drivers/xen/netfront/netfront.c | 1326 ++++++++++++++++++++++++++++++++++++++++ include/xen/net_driver_util.h | 48 + 7 files changed, 1455 insertions(+)
drivers/net/Kconfig +2 -0
@@ -2325,6 +2325,8 @@ source "drivers/atm/Kconfig" source "drivers/s390/net/Kconfig" +source "drivers/xen/Kconfig.net" + config ISERIES_VETH tristate "iSeries Virtual Ethernet driver support" depends on PPC_ISERIES
drivers/xen/Makefile +2 -0
@@ -5,3 +5,5 @@ obj-y += console/ obj-y += console/ obj-y += xenbus/ +obj-$(CONFIG_XEN_NETDEV_FRONTEND) += netfront/ +
drivers/xen/Kconfig.net new file +14 -0
@@ -0,0 +1,14 @@ +menu "Xen network device drivers" + depends on NETDEVICES && XEN + +config XEN_NETDEV_FRONTEND + tristate "Network-device frontend driver" + depends on XEN + default y + help + The network-device frontend driver allows the kernel to access + network interfaces within another guest OS. Unless you are building a + dedicated device-driver domain, or your master control domain + (domain 0), then you almost certainly want to say Y here. + +endmenu
drivers/xen/netfront/Makefile new file +4 -0
@@ -0,0 +1,4 @@ + +obj-$(CONFIG_XEN_NETDEV_FRONTEND) := xennet.o + +xennet-objs := netfront.o
drivers/xen/netfront/netfront.c new file +1584 -0
1,584 lines not on this page · raw message
--
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 18 lines · Chris Wright
drivers/xen/netfront/netfront.c new file +18
-0+ +/** Send a packet on a net device to encourage switches to learn the +* MAC. We send a fake ARP request. +* +* @param dev device +* @return 0 on success, error code otherwise +*/ +static int send_fake_arp (struct net_device *dev) +{ +struct sk_buff *skb; +u32 src_ip, dst_ip; + + dst_ip =INADDR_BROADCAST ; + src_ip =inet_select_addr (dev, dst_ip,RT_SCOPE_LINK ); + +/* No IP? Then nothing to do. */ +if (src_ip ==0 ) +return 0 ;
drivers/xen/netfront/netfront.c line 452
Quoted 10 lines
drivers/xen/netfront/netfront.c new file +10
-0+ + skb =arp_create (ARPOP_REPLY ,ETH_P_ARP , + dst_ip, dev, src_ip, +/*dst_hw*/ NULL ,/*src_hw*/ NULL , +/*target_hw*/ dev->dev_addr); +if (skb ==NULL ) +return -ENOMEM ; + +return dev_queue_xmit (skb); +}
Hi,
Hmmm maybe it's me, but something bugs me if a NIC driver is going to send IP level ARP packets... that just feels very very wrong and is a blatant layering violation.... shouldn't the ifup/ifconfig scripts just be fixed instead if this is critical behavior?
Greetings,
Arjan van de Ven
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 4 lines · Arjan van de Ven
Hmmm maybe it's me, but something bugs me if a NIC driver is going to send IP level ARP packets... that just feels very very wrong and is a blatant layering violation.... shouldn't the ifup/ifconfig scripts just be fixed instead if this is critical behavior?
Maybe we should be faking this out from our hotplug scripts in the control VM, although triggering this from user space is probably a bit of a pain. Regardless, the function can be removed from the driver if it's too distasteful: it's only a performance 'hack' to get network traffic more quickly redirected when migrating a VM between physical hosts. Things won't break horribly if it's removed.
-- Keir
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 7 lines · Keir Fraser
Quoted 4 lines · Arjan van de Ven
Hmmm maybe it's me, but something bugs me if a NIC driver is going to send IP level ARP packets... that just feels very very wrong and is a blatant layering violation.... shouldn't the ifup/ifconfig scripts just be fixed instead if this is critical behavior?
Quoted 6 lines
Maybe we should be faking this out from our hotplug scripts in the control VM, although triggering this from user space is probably a bit of a pain. Regardless, the function can be removed from the driver if it's too distasteful: it's only a performance 'hack' to get network traffic more quickly redirected when migrating a VM between physical hosts. Things won't break horribly if it's removed.
that sounds like a perfect job for a udev-kind of notification to userspace, eg "hey I migrated", and then udev/hotplug can just do this sending of arps.. maybe they also want to do other things just after a migration (which sounds entirely reasonable to me) such as renewing the dhcp lease, resetting the periodic userspace hardware polling timers in "hal", network manager etc etc...
so it looks entirely useful for me to have a userspace upcall for "post migration", and once you have that that udev side of things can just do this...
(question is if you also want a pre-migration upcall; my gut feeling says "no" because upcalls are async and you may end up running that upcall on the new physical machine)
Greetings,
Arjan van de Ven
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 17 lines · Arjan van de Ven
Quoted 7 lines · Keir Fraser
Quoted 4 lines · Arjan van de Ven
Hmmm maybe it's me, but something bugs me if a NIC driver is going to send IP level ARP packets... that just feels very very wrong and is a blatant layering violation.... shouldn't the ifup/ifconfig scripts just be fixed instead if this is critical behavior?
Quoted 6 lines
Maybe we should be faking this out from our hotplug scripts in the control VM, although triggering this from user space is probably a bit of a pain. Regardless, the function can be removed from the driver if it's too distasteful: it's only a performance 'hack' to get network traffic more quickly redirected when migrating a VM between physical hosts. Things won't break horribly if it's removed.
Quoted 6 lines
that sounds like a perfect job for a udev-kind of notification to userspace, eg "hey I migrated", and then udev/hotplug can just do this sending of arps.. maybe they also want to do other things just after a migration (which sounds entirely reasonable to me) such as renewing the dhcp lease, resetting the periodic userspace hardware polling timers in "hal", network manager etc etc...
I think we had this discussion the last time these patches came around. Other virtual environments (VMWare, POWER) have the same issue, and there are probably other scenarios (bonding?) that could use the same function. This doesn't belong in the driver.
Dave B
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
In reply to Arjan van de VenQuoted 4 lines · Arjan van de Ven
Hmmm maybe it's me, but something bugs me if a NIC driver is going to send IP level ARP packets... that just feels very very wrong and is a blatant layering violation....
It is but the bonding driver has been setting precedence for years now on sending ARPs from a driver;->
It does make a lot of sense to put it in user space. More interesting policies may include sending more than just ARPs and once you hard-code in the kernel you loose that flexibility.
Quoted 3 lines
shouldn't the ifup/ifconfig scripts just
be fixed instead if this is critical behavior?
I dont think the ifup/ifconfig provide operational status (i.e link up/down) - or do they? If they can be made to invoke scripts in such a case then we are set.
Note: you will get netlink events when devices are created or devices change their admin (via ifconfig) or operational (link down/up) status. [Try running "ip monitor" to see]
One could write a little daemon that reacts to these specific events. The problem has been some people claiming that daemons are a bad idea from a usability perspective. Patrick has mentioned he may be working on a daemon in user space that does exactly that. The other alternative is to do the udev thing and have the kernel invoke a script whenever an event of interest happens.
cheers,
jamal
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 4 lines · jamal
I dont think the ifup/ifconfig provide operational status (i.e link up/down) - or do they? If they can be made to invoke scripts in such a case then we are set.
In fact, that's a very good reason why this shouldn't be in netfront. Indeed, it shouldn't be in the guest at all. The reason is that the guest has no idea whether the physical carrier is present.
It's much better for the host to send the ARP packet on behalf of the guest since the host knows the carrier status and the guest's MAC address.
Cheers,
Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txtRe: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 5 lines · Herbert Xu
Quoted 3 lines · jamal
I dont think the ifup/ifconfig provide operational status (i.e link up/down) - or do they? If they can be made to invoke scripts in such a case then we are set.
Quoted 7 lines
In fact, that's a very good reason why this shouldn't be in netfront. Indeed, it shouldn't be in the guest at all. The reason is that the guest has no idea whether the physical carrier is present.
It's much better for the host to send the ARP packet on behalf of the guest since the host knows the carrier status and the guest's MAC address.
But sending ARPs is not the right thing if the guest is expecting to use IPv6 networking, in which case unsolicited neighbor advertisements are the right thing to do. The driver just doesn't seem to be the right place to do this, as it doesn't/ shouldn't need to know the difference between IPv4/IPv6.
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 6 lines · John Haller
But sending ARPs is not the right thing if the guest is expecting to use IPv6 networking, in which case unsolicited neighbor advertisements are the right thing to do. The driver just doesn't seem to be the right place to do this, as it doesn't/ shouldn't need to know the difference between IPv4/IPv6.
In this case it doesn't really matter because AFAIK they're trying to get switches to notice that the MAC has moved. So all you need is some packet that the switches can grok.
Cheers,
Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txtRe: [RFC PATCH 32/33] Add the Xen virtual network device driver.
In reply to Chris Wrightdrivers/xen/netfront/netfront.c line 77
Quoted 12 lines
diff -r
eadc12b20f35drivers/xen/netfront/netfront.cdrivers/xen/netfront/netfront.c new file +6
-0@@ -0,0 +1,1584 @@ +static inline void init_skb_shinfo (struct sk_buff *skb) +{ +atomic_set (&(skb_shinfo (skb)->dataref),1 ); +skb_shinfo (skb)->nr_frags =0 ; +skb_shinfo (skb)->frag_list =NULL ; +}
Shouldn't this move to skbuff.h?
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 6 lines · Stephen Hemminger
Quoted 5 lines
diff -r
eadc12b20f35drivers/xen/netfront/netfront.cdrivers/xen/netfront/netfront.c Fri Jun 09 15:03:12 2006 -0400 renamed from dev/null Thu Jan 01 00:00:00 1970 +0000 +0
-0@@ -0,0 +1,1584 @@
Quoted 8 lines
drivers/xen/netfront/netfront.c Fri Jun 09 15:03:12 2006 -0400 line 6
Quoted 6 lines
drivers/xen/netfront/netfront.c Fri Jun 09 15:03:12 2006 -0400 renamed from dev/null Thu Jan 01 00:00:00 1970 +0000 +6
-0+static inline void init_skb_shinfo(struct sk_buff *skb) +{ + atomic_set(&(skb_shinfo(skb)->dataref), 1); + skb_shinfo(skb)->nr_frags = 0; + skb_shinfo(skb)->frag_list = NULL; +}Shouldn't this move to
skbuff.h?
If and when my dom0=>domU GSO patches are applied, this will simply disappear.
Cheers,
Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txtDavid Miller
maintainerRe: [RFC PATCH 32/33] Add the Xen virtual network device driver.
In reply to Chris WrightFrom: Chris Wright <[email protected]>
Date: Tue, 18 Jul 2006 00:00:32 -0700
drivers/xen/netfront/netfront.c line 61
Quoted 3 lines
drivers/xen/netfront/netfront.c new file +3
-0+#ifdef CONFIG_XEN_BALLOON +#include <xen/balloon.h> +#endif
Let's put the ifdefs in xen/balloon.h not in the files including it.
drivers/xen/netfront/netfront.c line 629
Quoted 4 lines
drivers/xen/netfront/netfront.c new file +4
-0+#ifdef CONFIG_XEN_BALLOON +/* Tell the ballon driver what is going on. */ +balloon_update_driver_allowance (i); +#endif
Similarly let's define empty do-nothing functions in xen/balloon.h when the config option isn't set so we don't need to crap up the C sources with these ifdefs.
Re: [RFC PATCH 32/33] Add the Xen virtual network device driver.
Quoted 10 lines · * David Miller ([email protected]) wrote:
From: Chris Wright <[email protected]>
Date: Tue, 18 Jul 2006 00:00:32 -0700drivers/xen/netfront/netfront.c line 61
Quoted 3 lines
drivers/xen/netfront/netfront.c new file +3
-0+#ifdef CONFIG_XEN_BALLOON +#include <xen/balloon.h> +#endif Let's put the ifdefs in
xen/balloon.hnot in the files including it.
Quoted 8 lines
drivers/xen/netfront/netfront.c line 629
Quoted 4 lines
drivers/xen/netfront/netfront.c new file +4
-0+#ifdef CONFIG_XEN_BALLOON +/* Tell the ballon driver what is going on. */ +balloon_update_driver_allowance (i); +#endif Similarly let's define empty do-nothing functions in
xen/balloon.hwhen the config option isn't set so we don't need to crap up the C sources with these ifdefs.
Yeah, sorry it's kept more as a reminder to me. Upstream Xen doesn't do this, but this patchset doesn't support ballooning yet, so there just is not xen/balloon.h in the set. When merging with upstream Xen there's patch rejects either way, so I agree, better to just drop this since it's just future placeholder.
thanks,
-chris
RFC PATCH 33/33 Add Xen virtual block device driver.
In reply to Chris WrightThe block device frontend driver allows the kernel to access block devices exported exported by a virtual machine containing a physical block device driver.
--- drivers/block/Kconfig | 2 drivers/xen/Kconfig.blk | 14 drivers/xen/Makefile | 1 drivers/xen/blkfront/Makefile | 5 drivers/xen/blkfront/blkfront.c | 813 ++++++++++++++++++++++++++++++++++++++++ drivers/xen/blkfront/block.h | 156 +++++++ drivers/xen/blkfront/vbd.c | 216 ++++++++++ 7 files changed, 1207 insertions(+)
drivers/block/Kconfig +2 -0
@@ -450,6 +450,8 @@ config CDROM_PKTCDVD_WCACHE source "drivers/s390/block/Kconfig" +source "drivers/xen/Kconfig.blk" + config ATA_OVER_ETH tristate "ATA over Ethernet support" depends on NET
drivers/xen/Makefile +1 -0
@@ -5,5 +5,6 @@ obj-y += console/ obj-y += console/ obj-y += xenbus/ +obj-$(CONFIG_XEN_BLKDEV_FRONTEND) += blkfront/ obj-$(CONFIG_XEN_NETDEV_FRONTEND) += netfront/
drivers/xen/Kconfig.blk new file +14 -0
@@ -0,0 +1,14 @@ +menu "Xen block device drivers" + depends on XEN + +config XEN_BLKDEV_FRONTEND + tristate "Block device frontend driver" + depends on XEN + default y + help + The block device frontend driver allows the kernel to access block + devices exported from a device driver virtual machine. Unless you + are building a dedicated device driver virtual machine, then you + almost certainly want to say Y here. + +endmenu
drivers/xen/blkfront/Makefile new file +5 -0
@@ -0,0 +1,5 @@ + +obj-$(CONFIG_XEN_BLKDEV_FRONTEND) := xenblk.o + +xenblk-objs := blkfront.o vbd.o +
drivers/xen/blkfront/blkfront.c new file +811 -0
811 lines not on this page · raw message
drivers/xen/blkfront/block.h new file +155 -0
155 lines not on this page · raw message
drivers/xen/blkfront/vbd.c new file +214 -0
214 lines not on this page · raw message
--
Re: [RFC PATCH 33/33] Add Xen virtual block device driver.
Quoted 4 lines · Chris Wright
plain text document attachment (blkfront)
The block device frontend driver allows the kernel to access block devices exported exported by a virtual machine containing a physical block device driver.
Hi,
as first general comment, I think that some of the memory allocation GFP_ flags are possibly incorrect; I would expect several places to use GFP_NOIO rather than GFP_KERNEL, to avoid recursion/deadlocks
drivers/xen/blkfront/blkfront.c line 714
Quoted 9 lines
drivers/xen/blkfront/blkfront.c new file +9
-0+static void blkif_recover (struct blkfront_info *info) +{ +int i; +struct blkif_request *req; +struct blk_shadow *copy; +int j; + +/* Stage 1: Make a safe copy of the shadow state. */ + copy =kmalloc (sizeof (info->shadow),GFP_KERNEL | __GFP_NOFAIL);
like here..
drivers/xen/blkfront/blkfront.c line 715
Quoted 1 line
drivers/xen/blkfront/blkfront.c new file +1
-0+memcpy (copy, info->shadow,sizeof (info->shadow));
and __GFP_NOFAIL is usually horrid; is this because error recovery was an afterthought, or because it's physically impossible? In addition __GFP_NOFAIL in a block device driver is... an interesting way to add OOM deadlocks... have the VM guys looked into this yet?
drivers/xen/blkfront/block.h line 65
Quoted 6 lines
drivers/xen/blkfront/block.h new file +6
-0+#if 1 +#define IPRINTK(fmt, args...) \ + printk(KERN_INFO "xen_blk: " fmt, ##args) +#else +#define IPRINTK(fmt, args...) ((void)0) +#endif
hmm isn't this a duplication of the pr_debug() and dev_dbg() infrastructure? Please don't reinvent new ones..
Re: [RFC PATCH 33/33] Add Xen virtual block device driver.
Quoted 14 lines · Arjan van de Ven
as first general comment, I think that some of the memory allocation
GFP_flags are possibly incorrect; I would expect several places to useGFP_NOIOrather thanGFP_KERNEL, to avoid recursion/deadlocksdrivers/xen/blkfront/blkfront.c line 714
Quoted 5 lines
drivers/xen/blkfront/blkfront.c new file +3
-0+static void blkif_recover (struct blkfront_info *info) +/* Stage 1: Make a safe copy of the shadow state. */ + copy =kmalloc (sizeof (info->shadow),GFP_KERNEL | __GFP_NOFAIL);like here..
Quoted 5 lines
and
__GFP_NOFAILis usually horrid; is this because error recovery was an afterthought, or because it's physically impossible? In addition__GFP_NOFAILin a block device driver is... an interesting way to add OOM deadlocks... have the VM guys looked into this yet?
In this particular case, it's only used on the resume path, which I'm guessing would not lead to IO recursion. There doesn't seem to be any particular reason for this to be NOFAIL though (but I haven't really analyzed it).
There don't appear to be any memory allocations on the IO path; they're all in setup code.
J
RFC PATCH 33/33 Add Xen virtual block device driver.
In reply to Chris WrightQuoted 4 lines · Chris Wright
The block device frontend driver allows the kernel to access block devices exported exported by a virtual machine containing a physical block device driver.
First, I think this belongs in drivers/block (and the network driver belongs in drivers/net). If we're going to bring xen to the party, lets not leave it hiding out in a corner.
drivers/xen/blkfront/blkfront.c line 70
Quoted 13 lines
drivers/xen/blkfront/blkfront.c new file +13
-0+static void connect (struct blkfront_info *); +static void blkfront_closing (struct xenbus_device *); +static int blkfront_remove (struct xenbus_device *); +static int talk_to_backend (struct xenbus_device *,struct blkfront_info *); +static int setup_blkring (struct xenbus_device *,struct blkfront_info *); + +static void kick_pending_request_queues (struct blkfront_info *); + +static irqreturn_t blkif_int (int irq,void *dev_id,struct pt_regs *ptregs); +static void blkif_restart_queue (void *arg); +static void blkif_recover (struct blkfront_info *); +static void blkif_completion (struct blk_shadow *); +static void blkif_free (struct blkfront_info *,int );
I'm pretty sure you can rearrange the code to get rid of the forward references.
Quoted 11 lines
drivers/xen/blkfront/blkfront.c new file +11
-0+/** +* We are reconnecting to the backend, due to a suspend/resume, or a backend +* driver restart. We tear down our blkif structure and recreate it, but +* leave the device-layer structures intact so that this is transparent to the +* rest of the kernel. +*/ +static int blkfront_resume (struct xenbus_device *dev) +{ +struct blkfront_info *info = dev->dev.driver_data; +int err; +
drivers/xen/blkfront/blkfront.c line 143
Quoted 10 lines
drivers/xen/blkfront/blkfront.c new file +10
-0+DPRINTK ("blkfront_resume: %s\n" , dev->nodename); + +blkif_free (info,1 ); + + err =talk_to_backend (dev, info); +if (!err) +blkif_recover (info); + +return err; +}
Should blkfront_resume grab blkif_io_lock?
drivers/xen/blkfront/blkfront.c line 262
Quoted 6 lines
drivers/xen/blkfront/blkfront.c new file +6
-0+switch (backend_state) { +case XenbusStateUnknown: +case XenbusStateInitialising: +case XenbusStateInitWait: +case XenbusStateInitialised: +case XenbusStateClosed:
This actually should get fixed elsewhere, but SillyCaps???
Quoted 7 lines
drivers/xen/blkfront/blkfront.c new file +7
-0+static inline int GET_ID_FROM_FREELIST ( +struct blkfront_info *info) +{ +unsigned long free = info->shadow_free; +BUG_ON (free >BLK_RING_SIZE ); + info->shadow_free = info->shadow[free].req.id; + info->shadow[free].req.id =0x0fffffee ;/* debug */
drivers/xen/blkfront/blkfront.c line 384
Quoted 10 lines
drivers/xen/blkfront/blkfront.c new file +10
-0+return free; +} + +static inline void ADD_ID_TO_FREELIST ( +struct blkfront_info *info,unsigned long id) +{ + info->shadow[id].req.id = info->shadow_free; + info->shadow[id].request =0 ; + info->shadow_free = id; +}
A real nit..but why are these routines SHOUTING?
drivers/xen/blkfront/blkfront.c line 432
Quoted 5 lines
drivers/xen/blkfront/blkfront.c new file +5
-0+int blkif_release (struct inode *inode,struct file *filep) +{ +struct blkfront_info *info = inode->i_bdev->bd_disk->private_data; + info->users--; +if (info->users ==0 ) {
Hrm...this strikes me as racey. Don't you need at least a memory barrier here to handle SMP?
drivers/xen/blkfront/vbd.c line 52
Quoted 4 lines
drivers/xen/blkfront/vbd.c new file +4
-0+static struct xlbd_major_info xvd_major_info = { + .major =201 , + .type = &xvd_type_info +};
I've forgotten what the current policy is around new major numbers.
Dave B
Re: [RFC PATCH 33/33] Add Xen virtual block device driver.
Quoted 8 lines · Dave Boutcher
Quoted 3 lines · Chris Wright
The block device frontend driver allows the kernel to access block devices exported exported by a virtual machine containing a physical block device driver.
First, I think this belongs in
drivers/block(and the network driver belongs indrivers/net). If we're going to bring xen to the party, lets not leave it hiding out in a corner.
Strongly agreed.
Quoted 8 lines
Quoted 8 lines
drivers/xen/blkfront/blkfront.c new file +8
-0+static void connect (struct blkfront_info *); +static void blkfront_closing (struct xenbus_device *); +static int blkfront_remove (struct xenbus_device *); +static int talk_to_backend (struct xenbus_device *,struct blkfront_info *); +static int setup_blkring (struct xenbus_device *,struct blkfront_info *); + +static void kick_pending_request_queues (struct blkfront_info *); +
Quoted 8 lines
drivers/xen/blkfront/blkfront.c line 70
Quoted 5 lines
drivers/xen/blkfront/blkfront.c new file +5
-0+static irqreturn_t blkif_int (int irq,void *dev_id,struct pt_regs *ptregs); +static void blkif_restart_queue (void *arg); +static void blkif_recover (struct blkfront_info *); +static void blkif_completion (struct blk_shadow *); +static void blkif_free (struct blkfront_info *,int );I'm pretty sure you can rearrange the code to get rid of the forward references.
I've never thought this was useful... If the function ordering helps the human...
Quoted 8 lines
drivers/xen/blkfront/blkfront.c line 262
Quoted 6 lines
drivers/xen/blkfront/blkfront.c new file +6
-0+switch (backend_state) { +case XenbusStateUnknown: +case XenbusStateInitialising: +case XenbusStateInitWait: +case XenbusStateInitialised: +case XenbusStateClosed:This actually should get fixed elsewhere, but SillyCaps???
Agreed.
Re: [RFC PATCH 33/33] Add Xen virtual block device driver.
In reply to Dave BoutcherQuoted 9 lines · * Dave Boutcher ([email protected]) wrote:
Quoted 4 lines · Chris Wright
The block device frontend driver allows the kernel to access block devices exported exported by a virtual machine containing a physical block device driver.
First, I think this belongs in
drivers/block(and the network driver belongs indrivers/net). If we're going to bring xen to the party, lets not leave it hiding out in a corner.
Yeah, I think so too.
Quoted 8 lines
drivers/xen/blkfront/blkfront.c line 262
Quoted 6 lines
drivers/xen/blkfront/blkfront.c new file +6
-0+switch (backend_state) { +case XenbusStateUnknown: +case XenbusStateInitialising: +case XenbusStateInitWait: +case XenbusStateInitialised: +case XenbusStateClosed:This actually should get fixed elsewhere, but SillyCaps???
I really don't care either way. There's no shortage of this style specifically for enums, in fact there's a wide variety of interesting styles for enums.
Quoted 11 lines
Quoted 11 lines
drivers/xen/blkfront/blkfront.c new file +11
-0+static inline int GET_ID_FROM_FREELIST ( +struct blkfront_info *info) +{ +unsigned long free = info->shadow_free; +BUG_ON (free >BLK_RING_SIZE ); + info->shadow_free = info->shadow[free].req.id; + info->shadow[free].req.id =0x0fffffee ;/* debug */ +return free; +} + +static inline void ADD_ID_TO_FREELIST(
Quoted 8 lines
drivers/xen/blkfront/blkfront.c line 384
Quoted 6 lines
drivers/xen/blkfront/blkfront.c new file +6
-0+ struct blkfront_info *info, unsigned long id) +{ + info->shadow[id].req.id = info->shadow_free; + info->shadow[id].request =0 ; + info->shadow_free = id; +}A real nit..but why are these routines SHOUTING?
GOOD QUESTION! I had missed that, thanks. Seems likely it was just half converted from macro. I see no reason not to clean that up the rest of the way.
Quoted 9 lines
drivers/xen/blkfront/blkfront.c line 432
Quoted 5 lines
drivers/xen/blkfront/blkfront.c new file +5
-0+int blkif_release (struct inode *inode,struct file *filep) +{ +struct blkfront_info *info = inode->i_bdev->bd_disk->private_data; + info->users--; +if (info->users ==0 ) {Hrm...this strikes me as racey. Don't you need at least a memory barrier here to handle SMP?
Quoted 6 lines
drivers/xen/blkfront/vbd.c line 52
Quoted 4 lines
drivers/xen/blkfront/vbd.c new file +4
-0+static struct xlbd_major_info xvd_major_info = { + .major =201 , + .type = &xvd_type_info +};I've forgotten what the current policy is around new major numbers.
Damn, this is rather funny. There was a number reserved, but somehow this is the wrong one (should be 202 according to lanana[1]). Thanks, I'll fix that up.
thanks,
-chris
Re: [RFC PATCH 33/33] Add Xen virtual block device driver.
In reply to Dave BoutcherQuoted 46 lines · Dave Boutcher
Quoted 4 lines · Chris Wright
The block device frontend driver allows the kernel to access block devices exported exported by a virtual machine containing a physical block device driver.
First, I think this belongs in
drivers/block(and the network driver belongs indrivers/net). If we're going to bring xen to the party, lets not leave it hiding out in a corner.Quoted 4 lines
drivers/xen/blkfront/blkfront.c new file +3
-0+static void connect (struct blkfront_info *); +static void blkfront_closing (struct xenbus_device *); +static int blkfront_remove (struct xenbus_device *);drivers/xen/blkfront/blkfront.c line 70
Quoted 10 lines
drivers/xen/blkfront/blkfront.c new file +10
-0+static int talk_to_backend (struct xenbus_device *,struct blkfront_info *); +static int setup_blkring (struct xenbus_device *,struct blkfront_info *); + +static void kick_pending_request_queues (struct blkfront_info *); + +static irqreturn_t blkif_int (int irq,void *dev_id,struct pt_regs *ptregs); +static void blkif_restart_queue (void *arg); +static void blkif_recover (struct blkfront_info *); +static void blkif_completion (struct blk_shadow *); +static void blkif_free (struct blkfront_info *,int );I'm pretty sure you can rearrange the code to get rid of the forward references.
Quoted 15 lines
drivers/xen/blkfront/blkfront.c new file +15
-0+/** +* We are reconnecting to the backend, due to a suspend/resume, or a backend +* driver restart. We tear down our blkif structure and recreate it, but +* leave the device-layer structures intact so that this is transparent to the +* rest of the kernel. +*/ +static int blkfront_resume (struct xenbus_device *dev) +{ +struct blkfront_info *info = dev->dev.driver_data; +int err; + +DPRINTK ("blkfront_resume: %s\n" , dev->nodename); + +blkif_free (info,1 ); +
Quoted 9 lines
drivers/xen/blkfront/blkfront.c line 143
Quoted 7 lines
drivers/xen/blkfront/blkfront.c new file +6
-0+ err =talk_to_backend (dev, info); +if (!err) +blkif_recover (info); + +return err; +}Should
blkfront_resumegrabblkif_io_lock?
There should be no concurrent activity until info->connected has been set to BLKIF_STATE_CONNECTED, which doesn't happen until blkif_recover has completed successfully. blkif_queue_request and blkif_int both test the connection state before doing anything. (Not sure if a concurrent XenBus event can happen though.)
Quoted 23 lines
Quoted 9 lines
drivers/xen/blkfront/blkfront.c new file +7
-0+static inline int GET_ID_FROM_FREELIST ( +struct blkfront_info *info) +{ +unsigned long free = info->shadow_free; +BUG_ON (free >BLK_RING_SIZE ); + info->shadow_free = info->shadow[free].req.id; + info->shadow[free].req.id =0x0fffffee ;/* debug */ drivers/xen/blkfront/blkfront.c line 384
Quoted 10 lines
drivers/xen/blkfront/blkfront.c new file +10
-0+return free; +} + +static inline void ADD_ID_TO_FREELIST ( +struct blkfront_info *info,unsigned long id) +{ + info->shadow[id].req.id = info->shadow_free; + info->shadow[id].request =0 ; + info->shadow_free = id; +}A real nit..but why are these routines SHOUTING?
Quoted 10 lines
drivers/xen/blkfront/blkfront.c line 432
Quoted 6 lines
drivers/xen/blkfront/blkfront.c new file +5
-0+int blkif_release (struct inode *inode,struct file *filep) +{ +struct blkfront_info *info = inode->i_bdev->bd_disk->private_data; + info->users--; +if (info->users ==0 ) {Hrm...this strikes me as racey. Don't you need at least a memory barrier here to handle SMP?
Hm. Doesn't look good to me.
Quoted 8 lines
drivers/xen/blkfront/vbd.c line 52
Quoted 5 lines
drivers/xen/blkfront/vbd.c new file +4
-0+static struct xlbd_major_info xvd_major_info = { + .major =201 , + .type = &xvd_type_info +};I've forgotten what the current policy is around new major numbers.
This is wrong. 201 is allocated to Veritas, but 202 has been allocated for the Xen VBD.
J