Re: [patch 06/21] Xen-paravirt: remove ctor for pgd cache

Re: [patch 06/21] Xen-paravirt: remove ctor for pgd cache

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
>  
> @@ -261,10 +261,12 @@ void pgd_ctor(void *pgd, struct kmem_cac
>  	spin_unlock_irqrestore(&pgd_lock, flags);
>  }
>  
> -/* never called when PTRS_PER_PMD > 1 */
> -void pgd_dtor(void *pgd, struct kmem_cache *cache, unsigned long unused)
> +static void pgd_dtor(pgd_t *pgd)
>  {
>  	unsigned long flags; /* can be called from interrupt context */
> +
> +	if (PTRS_PER_PMD == 1)
> +		return;
>  
>  	paravirt_release_pd(__pa(pgd) >> PAGE_SHIFT);
>  	spin_lock_irqsave(&pgd_lock, flags);
>   


Acked, with exceptions.  This bit breaks VMI.  The paravirt_release_pd 
must happen unconditionally.  But I would rather fix it after patches 
get applied just to make sure the entire allocation / deallocation order 
constraints are intact.

Zach

Re: [patch 13/21] Xen-paravirt: Add nosegneg capability to the vsyscall page notes

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> Add 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.
>   

I don't like this because now a kernel compiled with both CONFIG_XEN and 
CONFIG_VMI has "nosegneg" turned on.  We don't actually require this for 
performance or correctness, so it would be nice to be able to 
dynamically turn it off instead of having it forced.

Zach

Re: [patch 13/21] Xen-paravirt: Add nosegneg capability to the vsyscall page notes

[Jeremy Fitzhardinge]

Zachary Amsden wrote:
> I don't like this because now a kernel compiled with both CONFIG_XEN
> and CONFIG_VMI has "nosegneg" turned on.  We don't actually require
> this for performance or correctness, so it would be nice to be able to
> dynamically turn it off instead of having it forced. 

Any suggestions about how to do this?  It seems hard to have a note
dynamically appear and disappear in the vsyscall.so.

I wasn't terribly concerned about this, since there is effectively zero
performance difference between the two library implementations.

    J

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Dan Hecht]

On 02/13/2007 02:17 PM, Jeremy Fitzhardinge wrote:
> The XEN config option enables the Xen paravirt_ops interface, which is
> installed when the kernel finds itself running under Xen. (By some
> as-yet fully defined mechanism, implemented in a future patch.)
> 
> Xen is no longer a sub-architecture, so the X86_XEN subarch config
> option has gone.
> 
> The disabled config options are:
> - PREEMPT: Xen doesn't support it
> - HZ: set to 100Hz for now, to cut down on VCPU context switch rate.
>   This will be adapted to use tickless later.
> - kexec: not yet supported
> 

I assume you plan to eventually get all this stuff working but just want 
to prevent configurations that the Xen paravirt-ops isn't ready for at 
the moment?

Instead can you do it this way:

config XEN
     depends on PARAVIRT && !PREEMPT && HZ_100 && !DOUBLEFAULT && !KEXEC


thanks,
Dan


> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
> Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
> Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> 
> ---
>  arch/i386/Kconfig       |    7 +++++--
>  arch/i386/Kconfig.debug |    1 +
>  arch/i386/xen/Kconfig   |   10 ++++++++++
>  kernel/Kconfig.hz       |    4 ++--
>  kernel/Kconfig.preempt  |    1 +
>  5 files changed, 19 insertions(+), 4 deletions(-)
> 
> ===================================================================
> --- a/arch/i386/Kconfig
> +++ b/arch/i386/Kconfig
> @@ -192,6 +192,8 @@ config PARAVIRT
>  	  under a hypervisor, improving performance significantly.
>  	  However, when run without a hypervisor the kernel is
>  	  theoretically slower.  If in doubt, say N.
> +
> +source "arch/i386/xen/Kconfig"
>  
>  config ACPI_SRAT
>  	bool
> @@ -298,12 +300,12 @@ config X86_UP_IOAPIC
>  
>  config X86_LOCAL_APIC
>  	bool
> -	depends on X86_UP_APIC || ((X86_VISWS || SMP) && !X86_VOYAGER) || X86_GENERICARCH
> +	depends on X86_UP_APIC || (((X86_VISWS || SMP) && !X86_VOYAGER) || X86_GENERICARCH)
>  	default y
>  
>  config X86_IO_APIC
>  	bool
> -	depends on X86_UP_IOAPIC || (SMP && !(X86_VISWS || X86_VOYAGER)) || X86_GENERICARCH
> +	depends on X86_UP_IOAPIC || ((SMP && !(X86_VISWS || X86_VOYAGER)) || X86_GENERICARCH)
>  	default y
>  
>  config X86_VISWS_APIC
> @@ -743,6 +745,7 @@ source kernel/Kconfig.hz
>  
>  config KEXEC
>  	bool "kexec system call"
> +	depends on !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
> ===================================================================
> --- a/arch/i386/Kconfig.debug
> +++ b/arch/i386/Kconfig.debug
> @@ -79,6 +79,7 @@ config DOUBLEFAULT
>  config DOUBLEFAULT
>  	default y
>  	bool "Enable doublefault exception handler" if EMBEDDED
> +	depends on !XEN
>  	help
>            This option allows trapping of rare doublefault exceptions that
>            would otherwise cause a system to silently reboot. Disabling this
> ===================================================================
> --- /dev/null
> +++ b/arch/i386/xen/Kconfig
> @@ -0,0 +1,10 @@
> +#
> +# This Kconfig describes xen options
> +#
> +
> +config XEN
> +	bool "Enable support for Xen hypervisor"
> +	depends PARAVIRT
> +	default y
> +	help
> +	  This is the Linux Xen port.
> ===================================================================
> --- a/kernel/Kconfig.hz
> +++ b/kernel/Kconfig.hz
> @@ -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
> @@ -49,7 +49,7 @@ endchoice
>  
>  config HZ
>  	int
> -	default 100 if HZ_100
> +	default 100 if HZ_100 || XEN
>  	default 250 if HZ_250
>  	default 300 if HZ_300
>  	default 1000 if HZ_1000
> ===================================================================
> --- a/kernel/Kconfig.preempt
> +++ b/kernel/Kconfig.preempt
> @@ -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)
> 

Re: [patch 13/21] Xen-paravirt: Add nosegneg capability to the vsyscall page notes

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>   
>> I don't like this because now a kernel compiled with both CONFIG_XEN
>> and CONFIG_VMI has "nosegneg" turned on.  We don't actually require
>> this for performance or correctness, so it would be nice to be able to
>> dynamically turn it off instead of having it forced. 
>>     
>
> Any suggestions about how to do this?  It seems hard to have a note
> dynamically appear and disappear in the vsyscall.so.
>
> I wasn't terribly concerned about this, since there is effectively zero
> performance difference between the two library implementations.
>   

I'm not super concerned either, but I still don't like it.  There 
already is dynamic replacement for vsyscall.so, so you could have just 
dropped in an an-note-ated version.

Zach

Re: [patch 13/21] Xen-paravirt: Add nosegneg capability to the vsyscall page notes

[Jeremy Fitzhardinge]

Zachary Amsden wrote:
> I'm not super concerned either, but I still don't like it.  There
> already is dynamic replacement for vsyscall.so, so you could have just
> dropped in an an-note-ated version.

Ah, OK.  I'll have a look at that.

    J

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Jeremy Fitzhardinge]

Dan Hecht wrote:
> I assume you plan to eventually get all this stuff working but just
> want to prevent configurations that the Xen paravirt-ops isn't ready
> for at the moment?
>
> Instead can you do it this way:
>
> config XEN
>     depends on PARAVIRT && !PREEMPT && HZ_100 && !DOUBLEFAULT && !KEXEC

That's a bit simpler code-wise, but it does make it pretty complex to
get everything just-so to even see the CONFIG_XEN option.

    J

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Andi Kleen]

> --- a/arch/i386/kernel/entry.S
> +++ b/arch/i386/kernel/entry.S
> @@ -1001,6 +1001,83 @@ ENTRY(kernel_thread_helper)
>  	CFI_ENDPROC
>  ENDPROC(kernel_thread_helper)
>  
> +#ifdef CONFIG_XEN
> +/* Xen only supports sysenter/sysexit in ring0 guests,
> +   and only if it the guest asks for it.  So for now,
> +   this should never be used. */
> +ENTRY(xen_sti_sysexit)
> +	CFI_STARTPROC
> +	ud2
> +	CFI_ENDPROC

Please add ENDPROC()s too, otherwise Jan will be unhappy.

Could be written in C with a real BUG? 

> +ENTRY(xen_failsafe_callback)
> +	CFI_STARTPROC
> +	pushl %eax
> +	CFI_ADJUST_CFA_OFFSET 4
> +	movl $1,%eax
> +1:	mov 4(%esp),%ds
> +2:	mov 8(%esp),%es
> +3:	mov 12(%esp),%fs
> +4:	mov 16(%esp),%gs
> +	testl %eax,%eax
> +	popl %eax
> +	CFI_ADJUST_CFA_OFFSET -4
> +	jz 5f
> +	addl $16,%esp		# EAX != 0 => Category 2 (Bad IRET)
> +	CFI_ADJUST_CFA_OFFSET -16
> +	jmp iret_exc
> +5:	addl $16,%esp		# EAX == 0 => Category 1 (Bad segment)

If you use lea you could move the two adds before the jz

> +#ifdef CONFIG_XEN
> +#include "../xen/xen-head.S"
> +#endif

Can this really not be linked separately? 

> +	
>  /*
>   * Real beginning of normal "text" segment
>   */
> @@ -528,7 +532,7 @@ ENTRY(_stext)
>  /*
>   * BSS section
>   */
> -.section ".bss.page_aligned","w"
> +.section ".bss.page_aligned"

Why? 

> -static fastcall void native_clts(void)
> +fastcall void native_clts(void)

Fastcalls should all go now

> --- a/arch/i386/kernel/vmlinux.lds.S
> +++ b/arch/i386/kernel/vmlinux.lds.S
> @@ -93,6 +93,7 @@ SECTIONS
>  
>    . = ALIGN(4096);
>    .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
> +	*(.data.page_aligned)

Weird that that didn't work before -- we already had page aligned data
and it somehow managed to work. But ok.

>  	*(.data.idt)
>    }
>  
> ===================================================================
> --- a/arch/i386/mm/pgtable.c
> +++ b/arch/i386/mm/pgtable.c
> @@ -267,6 +267,7 @@ static void pgd_ctor(pgd_t *pgd)
>  					swapper_pg_dir + USER_PTRS_PER_PGD,
>  					KERNEL_PGD_PTRS);
>  		} else {
> +			memset(pgd, 0, USER_PTRS_PER_PGD*sizeof(pgd_t));

That looks strange here. Belongs in a different patch? 

> +
> +extern struct Xgt_desc_struct cpu_gdt_descr;
> +extern struct i386_pda boot_pda;
> +extern unsigned long init_pg_tables_end;

No externs in .c files

> +
> +static DEFINE_PER_CPU(unsigned, lazy_mode);
> +
> +/* Code defined in entry.S (not a function) */
> +extern const char xen_sti_sysexit[];

Ok except that

> +{
> +	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
> +	       paravirt_ops.name);

Say something about Xen here?

> +}
> +
> +static fastcall void xen_restore_fl(unsigned long flags)
> +{
> +	struct vcpu_info *vcpu;
> +
> +	preempt_disable();
> +
> +	/* convert from IF type flag */
> +	flags = !(flags & X86_EFLAGS_IF);
> +	vcpu = read_pda(xen.vcpu);
> +	vcpu->evtchn_upcall_mask = flags;
> +	if (flags == 0) {
> +		barrier(); /* unmask then check (avoid races) */

If the barrier is needed shouldn't it be a rmb() ? 

> +	vcpu = read_pda(xen.vcpu);
> +	vcpu->evtchn_upcall_mask = 0;
> +	barrier(); /* unmask then check (avoid races) */

Similar.

> +static fastcall void xen_load_gdt(const struct Xgt_desc_struct *dtr)
> +{
> +        unsigned long va;
> +        int f;
> +	unsigned size = dtr->size + 1;
> +	unsigned long frames[16];
> +
> +	BUG_ON(size > 16*PAGE_SIZE);
> +

Indentation broken

(more occurences in this file) 
> +	type = (high >> 8) & 0x1f;
> +	dpl = (high >> 13) & 3;
> +
> +	if (type != 0xf && type != 0xe)
> +		return 0;
> +
> +	info->vector = vector;
> +	info->address = (high & 0xffff0000) | (low & 0x0000ffff);
> +	info->cs = low >> 16;
> +	info->flags = dpl;
> +	/* interrupt gates clear IF */
> +	if (type == 0xe)
> +		info->flags |= 4;

Nasty magic numbers? 

> +	return 1;
> +}
> +
> +#if 0
> +static void unpack_desc(u32 low, u32 high,
> +			unsigned long *base, unsigned long *limit,
> +			unsigned char *type, unsigned char *flags)
> +{
> +	*base = (high & 0xff000000) | ((high << 16) & 0x00ff0000) | ((low >> 16) & 0xffff);
> +	*limit = (high & 0x000f0000) | (low & 0xffff);
> +	*type = (high >> 8) & 0xff;
> +	*flags = (high >> 20) & 0xf;
> +}
> +#endif

Remove? 

> +
> +/* Locations of each CPU's IDT */
> +static DEFINE_PER_CPU(struct Xgt_desc_struct, idt_desc);

Why is that private here? 

> +	/* Switch to new pagetable.  This is done before
> +	   pagetable_init has done anything so that the new pages
> +	   added to the table can be prepared properly for Xen.  */
> +	printk("about to switch to new pagetable %p...\n", base);
> +	xen_write_cr3(__pa(base));
> +	printk("done\n");

KERN_* ?

> +	if (HYPERVISOR_update_descriptor(virt_to_machine(cpu_gdt_table +
> +							 GDT_ENTRY_PDA).maddr,
> +					 (u64)high << 32 | low))
> +		BUG();

Does a BUG that early actually do anything good?

> +
> +/*
> + * Accessors for packed IRQ information.
> + */

Wouldn't it be a little simpler to just use a bit field? 

> +static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
> +{
> +	int irq = evtchn_to_irq[chn];
> +
> +	BUG_ON(irq == -1);
> +	set_native_irq_info(irq, cpumask_of_cpu(cpu));
> +
> +	__clear_bit(chn, (unsigned long *)cpu_evtchn_mask[cpu_evtchn[chn]]);

Why is the mask not unsigned long in the first place ?

> +  level is a bitset of pending events themselves.
> +*/
> +asmlinkage fastcall void xen_evtchn_do_upcall(struct pt_regs *regs)
> +{
> +	int cpu = smp_processor_id();

Doesn't a preemptive kernel complain about this?

> +	set_64bit((u64 *)ptep, pte_val_ma(pte));
> +}
> +
> +void fastcall xen_pte_clear(struct mm_struct *mm, u32 addr,pte_t *ptep)
> +{
> +#if 1
> +	ptep->pte_low = 0;
> +	smp_wmb();
> +	ptep->pte_high = 0;	
> +#else
> +	set_64bit((u64 *)ptep, 0);

Eliminate #if please

> +fastcall unsigned long long xen_pgd_val(pgd_t pgd)
> +{
> +	unsigned long long ret = pgd.pgd;
> +	if (ret)
> +		ret = machine_to_phys(XMADDR(ret)).paddr | 1;

Why can they be 0 here anyways?

Normally they are all considered undefined when not present

> +	rdtscll(now);
> +	delta = now - shadow->tsc_timestamp;
> +	return scale_delta(delta, shadow->tsc_to_nsec_mul, shadow->tsc_shift);
> +}
> +
> +
> +static void xen_timer_interrupt_hook(void)

Timer code ... hopefully dyntick will not all mess this up. It is scheduled
to go into mainline RSN. You might have to do some more merging.
> +
> +char * __init xen_memory_setup(void);

Prototypes don't need __init

> +void __init xen_arch_setup(void);
> +void __init xen_init_IRQ(void);
> +
> @@ -53,6 +54,7 @@ struct paravirt_ops
>  	void (*arch_setup)(void);
>  	char *(*memory_setup)(void);
>  	void (*init_IRQ)(void);
> +	void (*init_pda)(struct i386_pda *, int cpu);

Hmm weird. For what was that needed again? 

-AndI

Re: [Xen-devel] Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> Dan Hecht wrote:
>   
>> I assume you plan to eventually get all this stuff working but just
>> want to prevent configurations that the Xen paravirt-ops isn't ready
>> for at the moment?
>>
>> Instead can you do it this way:
>>
>> config XEN
>>     depends on PARAVIRT && !PREEMPT && HZ_100 && !DOUBLEFAULT && !KEXEC
>>     
>
> That's a bit simpler code-wise, but it does make it pretty complex to
> get everything just-so to even see the CONFIG_XEN option.
>   

Yes, but that is what you need to do to compile Xen - logically, to 
build a Xen kernel, you need to have a kernel configuration which can 
enable Xen - not limit the configuration of a kernel because Xen has 
been enabled.  This is because  with paravirt-ops, Xen compiled kernels 
may not actually run on Xen, so you can't arbitrarily drop features 
because you assume Xen is there.

One of these has an easy solution - doublefault.  You don't need to 
install the doublefault gate if you don't want it, and the hypervisor 
doesn't need to freak out if you install it, it can just ignore the gate 
entirely and claim #DF is not supported.

Zach

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Dan Hecht]

On 02/13/2007 03:29 PM, Jeremy Fitzhardinge wrote:
> Dan Hecht wrote:
>> I assume you plan to eventually get all this stuff working but just
>> want to prevent configurations that the Xen paravirt-ops isn't ready
>> for at the moment?
>>
>> Instead can you do it this way:
>>
>> config XEN
>>     depends on PARAVIRT && !PREEMPT && HZ_100 && !DOUBLEFAULT && !KEXEC
> 
> That's a bit simpler code-wise, but it does make it pretty complex to
> get everything just-so to even see the CONFIG_XEN option.
> 

Not only is it simpler code-wise, but it is more to the point... it is 
CONFIG_XEN that needs to be fixed to handle PREEMPT, KEXEC, different HZ 
values, etc.  Not the other way around.

Enabling the compile of any paravirt-ops backend shouldn't cripple the 
kernel in any way... instead, the burden should be on the xen 
paravirt-ops backend to be completed.  CONFIG_PREEMPT shouldn't care 
about which paravirt-ops are compiled in.  Instead, CONFIG_XEN is the 
one that needs !PREEMPT.

Dan

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

Andi Kleen wrote:
>> --- a/arch/i386/kernel/entry.S
>> +++ b/arch/i386/kernel/entry.S
>> @@ -1001,6 +1001,83 @@ ENTRY(kernel_thread_helper)
>>  	CFI_ENDPROC
>>  ENDPROC(kernel_thread_helper)
>>  
>> +#ifdef CONFIG_XEN
>> +/* Xen only supports sysenter/sysexit in ring0 guests,
>> +   and only if it the guest asks for it.  So for now,
>> +   this should never be used. */
>> +ENTRY(xen_sti_sysexit)
>> +	CFI_STARTPROC
>> +	ud2
>> +	CFI_ENDPROC
>>     
>
> Please add ENDPROC()s too, otherwise Jan will be unhappy.
>   

Will do.

> Could be written in C with a real BUG? 
>   

I guess, but this seems simpler.  It could be removed altogether, but it
would be nice to make sure it never happens.

>> +ENTRY(xen_failsafe_callback)
>> +	CFI_STARTPROC
>> +	pushl %eax
>> +	CFI_ADJUST_CFA_OFFSET 4
>> +	movl $1,%eax
>> +1:	mov 4(%esp),%ds
>> +2:	mov 8(%esp),%es
>> +3:	mov 12(%esp),%fs
>> +4:	mov 16(%esp),%gs
>> +	testl %eax,%eax
>> +	popl %eax
>> +	CFI_ADJUST_CFA_OFFSET -4
>> +	jz 5f
>> +	addl $16,%esp		# EAX != 0 => Category 2 (Bad IRET)
>> +	CFI_ADJUST_CFA_OFFSET -16
>> +	jmp iret_exc
>> +5:	addl $16,%esp		# EAX == 0 => Category 1 (Bad segment)
>>     
>
> If you use lea you could move the two adds before the jz
>   

Yep.

>> +#ifdef CONFIG_XEN
>> +#include "../xen/xen-head.S"
>> +#endif
>>     
>
> Can this really not be linked separately? 
>   
xen-head.S jumps back to startup_paravirt.  That could be exported, and
then it could be linked separately.  There used to be a requirement that
the code in xen-head.S be at a compile-time known address, but that's no
longer the case.

>> +	
>>  /*
>>   * Real beginning of normal "text" segment
>>   */
>> @@ -528,7 +532,7 @@ ENTRY(_stext)
>>  /*
>>   * BSS section
>>   */
>> -.section ".bss.page_aligned","w"
>> +.section ".bss.page_aligned"
>>     
>
> Why? 
>   

I got complaints about section attribute mismatches without it.

>> -static fastcall void native_clts(void)
>> +fastcall void native_clts(void)
>>     
>
> Fastcalls should all go now
>   

Killing them here as we speak.

>> --- a/arch/i386/kernel/vmlinux.lds.S
>> +++ b/arch/i386/kernel/vmlinux.lds.S
>> @@ -93,6 +93,7 @@ SECTIONS
>>  
>>    . = ALIGN(4096);
>>    .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
>> +	*(.data.page_aligned)
>>     
>
> Weird that that didn't work before -- we already had page aligned data
> and it somehow managed to work. But ok.
>
>   
>>  	*(.data.idt)
>>    }
>>  
>> ===================================================================
>> --- a/arch/i386/mm/pgtable.c
>> +++ b/arch/i386/mm/pgtable.c
>> @@ -267,6 +267,7 @@ static void pgd_ctor(pgd_t *pgd)
>>  					swapper_pg_dir + USER_PTRS_PER_PGD,
>>  					KERNEL_PGD_PTRS);
>>  		} else {
>> +			memset(pgd, 0, USER_PTRS_PER_PGD*sizeof(pgd_t));
>>     
>
> That looks strange here. Belongs in a different patch? 
>   

This is a big rollup patch of all the core Xen stuff; the subject is
wrong.  But I added this for Xen because it needs to make sure the page
is all zero and doesn't have some left-over pieces of old pagetable.

>> +
>> +extern struct Xgt_desc_struct cpu_gdt_descr;
>> +extern struct i386_pda boot_pda;
>> +extern unsigned long init_pg_tables_end;
>>     
>
> No externs in .c files
>   

OK.

>> +
>> +static DEFINE_PER_CPU(unsigned, lazy_mode);
>> +
>> +/* Code defined in entry.S (not a function) */
>> +extern const char xen_sti_sysexit[];
>>     
>
> Ok except that
>
>   
>> +{
>> +	printk(KERN_INFO "Booting paravirtualized kernel on %s\n",
>> +	       paravirt_ops.name);
>>     
>
> Say something about Xen here?
>   

paravirt_ops.name mentions Xen.

>> +}
>> +
>> +static fastcall void xen_restore_fl(unsigned long flags)
>> +{
>> +	struct vcpu_info *vcpu;
>> +
>> +	preempt_disable();
>> +
>> +	/* convert from IF type flag */
>> +	flags = !(flags & X86_EFLAGS_IF);
>> +	vcpu = read_pda(xen.vcpu);
>> +	vcpu->evtchn_upcall_mask = flags;
>> +	if (flags == 0) {
>> +		barrier(); /* unmask then check (avoid races) */
>>     
>
> If the barrier is needed shouldn't it be a rmb() ? 
>
>   
>> +	vcpu = read_pda(xen.vcpu);
>> +	vcpu->evtchn_upcall_mask = 0;
>> +	barrier(); /* unmask then check (avoid races) */
>>     
>
> Similar.
>   

Erm, not sure.  The write needs to be complete before the read happens. 
Which is the right barrier for that?

>> +static fastcall void xen_load_gdt(const struct Xgt_desc_struct *dtr)
>> +{
>> +        unsigned long va;
>> +        int f;
>> +	unsigned size = dtr->size + 1;
>> +	unsigned long frames[16];
>> +
>> +	BUG_ON(size > 16*PAGE_SIZE);
>> +
>>     
>
> Indentation broken
>
> (more occurences in this file) 
>   

OK.

>> +	type = (high >> 8) & 0x1f;
>> +	dpl = (high >> 13) & 3;
>> +
>> +	if (type != 0xf && type != 0xe)
>> +		return 0;
>> +
>> +	info->vector = vector;
>> +	info->address = (high & 0xffff0000) | (low & 0x0000ffff);
>> +	info->cs = low >> 16;
>> +	info->flags = dpl;
>> +	/* interrupt gates clear IF */
>> +	if (type == 0xe)
>> +		info->flags |= 4;
>>     
>
> Nasty magic numbers? 
>   

Yeah.  Are there any existing definitions of the x86 gate types?

>> +	return 1;
>> +}
>> +
>> +#if 0
>> +static void unpack_desc(u32 low, u32 high,
>> +			unsigned long *base, unsigned long *limit,
>> +			unsigned char *type, unsigned char *flags)
>> +{
>> +	*base = (high & 0xff000000) | ((high << 16) & 0x00ff0000) | ((low >> 16) & 0xffff);
>> +	*limit = (high & 0x000f0000) | (low & 0xffff);
>> +	*type = (high >> 8) & 0xff;
>> +	*flags = (high >> 20) & 0xf;
>> +}
>> +#endif
>>     
>
> Remove? 
>   

Yep.

>> +
>> +/* Locations of each CPU's IDT */
>> +static DEFINE_PER_CPU(struct Xgt_desc_struct, idt_desc);
>>     
>
> Why is that private here? 
>   

It's a local per-cpu cache of the idt as set by the kernel.  Xen doesn't
use the idt directly, but it remembers what idt has been set so it can
tell if an idt descriptor update is affecting the current idt or
something else.  Its a bit over-engineered, since the idt isn't really
touched much at all.

>> +	/* Switch to new pagetable.  This is done before
>> +	   pagetable_init has done anything so that the new pages
>> +	   added to the table can be prepared properly for Xen.  */
>> +	printk("about to switch to new pagetable %p...\n", base);
>> +	xen_write_cr3(__pa(base));
>> +	printk("done\n");
>>     
>
> KERN_* ?
>   

Delete.  Don't really need it any more.

>> +	if (HYPERVISOR_update_descriptor(virt_to_machine(cpu_gdt_table +
>> +							 GDT_ENTRY_PDA).maddr,
>> +					 (u64)high << 32 | low))
>> +		BUG();
>>     
>
> Does a BUG that early actually do anything good?
>   

Under Xen, an early BUG gets a nice register dump and backtrack from the
hypervisor, so its pretty useful for debugging.

>> +
>> +/*
>> + * Accessors for packed IRQ information.
>> + */
>>     
>
> Wouldn't it be a little simpler to just use a bit field? 
>   

Yeah, or even just a simple structure, since the elements are
short/byte/byte.

>> +static void bind_evtchn_to_cpu(unsigned int chn, unsigned int cpu)
>> +{
>> +	int irq = evtchn_to_irq[chn];
>> +
>> +	BUG_ON(irq == -1);
>> +	set_native_irq_info(irq, cpumask_of_cpu(cpu));
>> +
>> +	__clear_bit(chn, (unsigned long *)cpu_evtchn_mask[cpu_evtchn[chn]]);
>>     
>
> Why is the mask not unsigned long in the first place ?
>   

Hm, it was.  Seems completely redundant.

>> +  level is a bitset of pending events themselves.
>> +*/
>> +asmlinkage fastcall void xen_evtchn_do_upcall(struct pt_regs *regs)
>> +{
>> +	int cpu = smp_processor_id();
>>     
>
> Doesn't a preemptive kernel complain about this?
>   

Xen doesn't support preempt.

>> +	set_64bit((u64 *)ptep, pte_val_ma(pte));
>> +}
>> +
>> +void fastcall xen_pte_clear(struct mm_struct *mm, u32 addr,pte_t *ptep)
>> +{
>> +#if 1
>> +	ptep->pte_low = 0;
>> +	smp_wmb();
>> +	ptep->pte_high = 0;	
>> +#else
>> +	set_64bit((u64 *)ptep, 0);
>>     
>
> Eliminate #if please
>   

I need to test this a bit.  This is here because set_64bit doesn't work
properly under qemu (its emulation mis-reports the eip if the
instruction faults), but I haven't tested this under native.

>> +fastcall unsigned long long xen_pgd_val(pgd_t pgd)
>> +{
>> +	unsigned long long ret = pgd.pgd;
>> +	if (ret)
>> +		ret = machine_to_phys(XMADDR(ret)).paddr | 1;
>>     
>
> Why can they be 0 here anyways?
>
> Normally they are all considered undefined when not present
>   

Not sure.

>> +	rdtscll(now);
>> +	delta = now - shadow->tsc_timestamp;
>> +	return scale_delta(delta, shadow->tsc_to_nsec_mul, shadow->tsc_shift);
>> +}
>> +
>> +
>> +static void xen_timer_interrupt_hook(void)
>>     
>
> Timer code ... hopefully dyntick will not all mess this up. It is scheduled
> to go into mainline RSN. You might have to do some more merging.
>   

Yep.

>> +
>> +char * __init xen_memory_setup(void);
>>     
>
> Prototypes don't need __init
>   

OK.

>> +void __init xen_arch_setup(void);
>> +void __init xen_init_IRQ(void);
>> +
>> @@ -53,6 +54,7 @@ struct paravirt_ops
>>  	void (*arch_setup)(void);
>>  	char *(*memory_setup)(void);
>>  	void (*init_IRQ)(void);
>> +	void (*init_pda)(struct i386_pda *, int cpu);
>>     
>
> Hmm weird. For what was that needed again? 
>   

The PDA structure contains some Xen-specific (or generally, paravirt
backend specific) parts, which need initialization when the rest of the
PDA is.

    J

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> Wrap the paravirt_ops members we want to export in wrapper functions.
> Since we binary-patch the critical ones, this doesn't make a speed
> impact.
>
> I moved drm_follow_page into the core, to avoid having to wrap the
> various pte ops.  Unlining kernel_fpu_end and using that in the RAID6
> code would remove the need to export clts/read_cr0/write_cr0 too.
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
>
> ===================================================================
> --- a/arch/i386/kernel/paravirt.c
> +++ b/arch/i386/kernel/paravirt.c
> @@ -596,6 +596,123 @@ static int __init print_banner(void)
>  	return 0;
>  }
>  core_initcall(print_banner);
> +
> +unsigned long paravirt_save_flags(void)
> +{
> +	return paravirt_ops.save_fl();
> +}
> +EXPORT_SYMBOL(paravirt_save_flags);
> +
> +void paravirt_restore_flags(unsigned long flags)
> +{
> +	paravirt_ops.restore_fl(flags);
> +}
> +EXPORT_SYMBOL(paravirt_restore_flags);
> +
> +void paravirt_irq_disable(void)
> +{
> +	paravirt_ops.irq_disable();
> +}
> +EXPORT_SYMBOL(paravirt_irq_disable);
> +
> +void paravirt_irq_enable(void)
> +{
> +	paravirt_ops.irq_enable();
> +}
> +EXPORT_SYMBOL(paravirt_irq_enable);
>   

This turned out really hideous looking to me.  Can't we split the struct 
into GPL'd and non-GPL'd functions instead?  We still have the same 
granularity, and none of this function call to an indirect function call 
nonsense.

Zach

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Jeremy Fitzhardinge]

Zachary Amsden wrote:
> This turned out really hideous looking to me.  Can't we split the
> struct into GPL'd and non-GPL'd functions instead?  We still have the
> same granularity, and none of this function call to an indirect
> function call nonsense. 

It's not pretty, but I think having paravirt_ops and paravirt_ops_gpl
would be worse.  I'd be sympathetic to the idea of splitting
paravirt_ops up by function groupings, but splitting it by license seems
like a maintenance headache with no real upside.  Besides, patching will
solve everything (tm).

    J

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>   
>> This turned out really hideous looking to me.  Can't we split the
>> struct into GPL'd and non-GPL'd functions instead?  We still have the
>> same granularity, and none of this function call to an indirect
>> function call nonsense. 
>>     
>
> It's not pretty, but I think having paravirt_ops and paravirt_ops_gpl
> would be worse.  I'd be sympathetic to the idea of splitting
> paravirt_ops up by function groupings, but splitting it by license seems
> like a maintenance headache with no real upside.  Besides, patching will
> solve everything (tm).
>   

Ok.  As long as we plan on patching CR2 and CR0 / clts accessors for FPU 
save during context switch and page fault paths in the future.

Re: [patch 05/21] Xen-paravirt: paravirt_ops: allocate a fixmap slot

[Dan Hecht]

On 02/13/2007 02:17 PM, Jeremy Fitzhardinge wrote:
> Allocate a fixmap slot for use by a paravirt_ops implementation.  Xen
> uses this to map the hypervisor's shared info page, which doesn't have
> a pseudo-physical page number, and therefore can't be mapped
> ordinarily.
> 

Why doesn't Xen allocate the shared_info page from the pseudo-physical 
space?  Doesn't it already have to steal pages from the pseudo-physical 
space for e.g. initial page tables, console, etc?  Why not do the same 
for shared_info, and then you don't need a reserve the fixmap slot.

Dan

Re: [patch 05/21] Xen-paravirt: paravirt_ops: allocate a fixmap slot

[Jeremy Fitzhardinge]

Dan Hecht wrote:
> Why doesn't Xen allocate the shared_info page from the pseudo-physical
> space?  Doesn't it already have to steal pages from the
> pseudo-physical space for e.g. initial page tables, console, etc?  Why
> not do the same for shared_info, and then you don't need a reserve the
> fixmap slot.

Unlike the pagetable pages or the console page, the shared info page
doesn't have a pseudo-physical address, so in order to map it we need to
directly construct a pte containing the mfn for that page.  Inserting
this mapping into the fixmap space seems like the easiest way to do
this.  It's not like a fixmap slot costs anything.

    J

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Jeremy Fitzhardinge]

Zachary Amsden wrote:
> Ok.  As long as we plan on patching CR2 and CR0 / clts accessors for
> FPU save during context switch and page fault paths in the future.

That's up to each backend, right?  Or do these need to be patched for a
correctness reason?

    J

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> Zachary Amsden wrote:
>   
>> Ok.  As long as we plan on patching CR2 and CR0 / clts accessors for
>> FPU save during context switch and page fault paths in the future.
>>     
>
> That's up to each backend, right?  Or do these need to be patched for a
> correctness reason?
>   

No, it needs to be part of the general patch list first, which is still 
hand listed rather than just any op being patchable.  Then it can be up 
to the backend.

Zach

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Jeremy Fitzhardinge]

Zachary Amsden wrote:
> No, it needs to be part of the general patch list first, which is
> still hand listed rather than just any op being patchable.  Then it
> can be up to the backend. 

Ah, right, yes.  We need to add all (most? some?) of the pte operations
to that list too.

    J

Re: [patch 05/21] Xen-paravirt: paravirt_ops: allocate a fixmap slot

[Dan Hecht]

On 02/13/2007 05:36 PM, Jeremy Fitzhardinge wrote:
> Dan Hecht wrote:
>> Why doesn't Xen allocate the shared_info page from the pseudo-physical
>> space?  Doesn't it already have to steal pages from the
>> pseudo-physical space for e.g. initial page tables, console, etc?  Why
>> not do the same for shared_info, and then you don't need a reserve the
>> fixmap slot.
> 
> Unlike the pagetable pages or the console page, the shared info page
> doesn't have a pseudo-physical address, so in order to map it we need to
> directly construct a pte containing the mfn for that page. 

Right.  But that is only because Xen decides to allocate the page from 
the (machine) physical space, rather than from the pseudo-physical 
space.  My question is: why doesn't Xen allocate shared_info from the 
pseudo-physical space?  If it had, then this page wouldn't need to be 
treated specially.  I'm not sure, but I seem to remember on 64-bit 
Xen/XenLinux allocated shared_info from pseudo-physical space already...

  Inserting
> this mapping into the fixmap space seems like the easiest way to do
> this.  It's not like a fixmap slot costs anything.
> 
>

I don't really have an objection to stealing a fixmap slot, just seems 
cleaner if you didn't have to special case the shared_info.

Dan

Re: [Xen-devel] Re: [patch 13/21] Xen-paravirt: Add nosegneg capability to the vsyscall page notes

[Rusty Russell]

On Tue, 2007-02-13 at 14:45 -0800, Zachary Amsden wrote:
> Jeremy Fitzhardinge wrote:
> > Add 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.
> >   
> 
> I don't like this because now a kernel compiled with both CONFIG_XEN and 
> CONFIG_VMI has "nosegneg" turned on.  We don't actually require this for 
> performance or correctness, so it would be nice to be able to 
> dynamically turn it off instead of having it forced.

Ditto for lguest.

Rusty.

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Rusty Russell]

On Tue, 2007-02-13 at 17:06 -0800, Zachary Amsden wrote:
> Jeremy Fitzhardinge wrote:
> > Wrap the paravirt_ops members we want to export in wrapper functions.
> > Since we binary-patch the critical ones, this doesn't make a speed
> > impact.
> 
> This turned out really hideous looking to me.  Can't we split the struct 
> into GPL'd and non-GPL'd functions instead?  We still have the same 
> granularity, and none of this function call to an indirect function call 
> nonsense.

This patch, indeed, should not have been pushed in this series.  But not
for that reason: I actually prefer explicit exports.

KVM and lguest need more symbols, so the real patch will make them use
native_XXX versions explicitly...

Cheers,
Rusty.

[Xen-devel] Re: [patch 05/21] Xen-paravirt: paravirt_ops: allocate a fixmap slot

[Jan Beulich]

>>> Jeremy Fitzhardinge <jeremy@goop.org> 14.02.07 02:36 >>>
>Dan Hecht wrote:
>> Why doesn't Xen allocate the shared_info page from the pseudo-physical
>> space?  Doesn't it already have to steal pages from the
>> pseudo-physical space for e.g. initial page tables, console, etc?  Why
>> not do the same for shared_info, and then you don't need a reserve the
>> fixmap slot.
>
>Unlike the pagetable pages or the console page, the shared info page
>doesn't have a pseudo-physical address, so in order to map it we need to
>directly construct a pte containing the mfn for that page.  Inserting
>this mapping into the fixmap space seems like the easiest way to do
>this.  It's not like a fixmap slot costs anything.

Otoh there are many fixmap slots not used under Xen, perhaps it would
be possible to use one of those... One slot certainly doesn't cost a lot,
but many (like the IO-APIC group) may already matter, especially on
PAE systems with lots of memory). Consequently, if these can't be
squeezed out as needed, re-using would seem more appropriate than
adding.
(I would certainly favor [conditionally] removing them, but can't easily
see how to do this under CONFIG_PARAVIRT. Background being that
we've already been hit by those [namely the LAPIC one] remaining
present, hence the build not being able to detect that accesses to the
LAPIC page can't work. While no such access was ever left in the base
kernel, modules are more susceptible to this, and in our case it was
the [native, i.e. pre-xenoprof] oprofile driver that had been forgotten
to be disabled.)

Jan

Re: [patch 05/21] Xen-paravirt: paravirt_ops: allocate a fixmap slot

[Gerd Hoffmann]

Dan Hecht wrote:
> Right.  But that is only because Xen decides to allocate the page from 
> the (machine) physical space, rather than from the pseudo-physical 
> space.  My question is: why doesn't Xen allocate shared_info from the 
> pseudo-physical space?

Historical reasons ...

> If it had, then this page wouldn't need to be 
> treated specially.  I'm not sure, but I seem to remember on 64-bit 
> Xen/XenLinux allocated shared_info from pseudo-physical space already...

Yep, the ia64 port which came later handles some things differently,
specifically some "magic" pages are allocated more clever ;)

Changing that for x86 would break existing guests though.

cheers,
  Gerd

-- 
Gerd Hoffmann <kraxel@suse.de>

[Xen-devel] Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jan Beulich]

>>> @@ -528,7 +532,7 @@ ENTRY(_stext)
>>>  /*
>>>   * BSS section
>>>   */
>>> -.section ".bss.page_aligned","w"
>>> +.section ".bss.page_aligned"
>>>     
>>
>> Why? 
>>   
>
>I got complaints about section attribute mismatches without it.

Then perhaps ... "aw" is meant?

>>> +fastcall unsigned long long xen_pgd_val(pgd_t pgd)
>>> +{
>>> +	unsigned long long ret = pgd.pgd;
>>> +	if (ret)
>>> +		ret = machine_to_phys(XMADDR(ret)).paddr | 1;
>>>     
>>
>> Why can they be 0 here anyways?
>>
>> Normally they are all considered undefined when not present
>>   
>
>Not sure.

This should probably get sync-ed up with the page table handling changes
submitted to xen-devel yesterday. Using zero/non-zero tests in contexts
like this was always broken; should check for _PAGE_PRESENT.

Jan

Re: [Xen-devel] Re: [patch 05/21] Xen-paravirt: paravirt_ops: allocate a fixmap slot

[Andi Kleen]

On Wed, Feb 14, 2007 at 08:37:26AM +0000, Jan Beulich wrote:
> >>> Jeremy Fitzhardinge <jeremy@goop.org> 14.02.07 02:36 >>>
> >Dan Hecht wrote:
> >> Why doesn't Xen allocate the shared_info page from the pseudo-physical
> >> space?  Doesn't it already have to steal pages from the
> >> pseudo-physical space for e.g. initial page tables, console, etc?  Why
> >> not do the same for shared_info, and then you don't need a reserve the
> >> fixmap slot.
> >
> >Unlike the pagetable pages or the console page, the shared info page
> >doesn't have a pseudo-physical address, so in order to map it we need to
> >directly construct a pte containing the mfn for that page.  Inserting
> >this mapping into the fixmap space seems like the easiest way to do
> >this.  It's not like a fixmap slot costs anything.
> 
> Otoh there are many fixmap slots not used under Xen, perhaps it would
> be possible to use one of those... One slot certainly doesn't cost a lot,

I don't have a problem with reserving one page for this.

-Andi

Re: [patch 02/21] Xen-paravirt: Handle a zero-sized VT console

[Gerd Hoffmann]

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

Jeremy Fitzhardinge wrote:
> If we're running under Xen, then there's no VT console.  This results
> in vc->vc_screenbuf_size == 0, which causes alloc_bootmem to panic.
> Don't bother allocating a vc_screenbuf if its going to be 0 sized.

NAK.

The *real* problem is that the real-mode boot code never ever runs, thus
SCREEN_INFO is not initialized (all zeros), and vgacon doesn't catch
that case.  Instead it thinks it runs on a EGA card with 0 lines and 0
columns.

So better fix vgacon to catch this and switch to the 80x25 dummy console
instead, patch attached.

cheers,

  Gerd


[-- Attachment #2: vgacon --]
[-- Type: text/plain, Size: 779 bytes --]

---
 drivers/video/console/vgacon.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: paravirt-2.6.20-hg749/drivers/video/console/vgacon.c
===================================================================
--- paravirt-2.6.20-hg749.orig/drivers/video/console/vgacon.c
+++ paravirt-2.6.20-hg749/drivers/video/console/vgacon.c
@@ -372,7 +372,8 @@ static const char *vgacon_startup(void)
 	}
 
 	/* VGA16 modes are not handled by VGACON */
-	if ((ORIG_VIDEO_MODE == 0x0D) ||	/* 320x200/4 */
+	if ((ORIG_VIDEO_MODE == 0x00) ||	/* SCREEN_INFO not initialized */
+	    (ORIG_VIDEO_MODE == 0x0D) ||	/* 320x200/4 */
 	    (ORIG_VIDEO_MODE == 0x0E) ||	/* 640x200/4 */
 	    (ORIG_VIDEO_MODE == 0x10) ||	/* 640x350/4 */
 	    (ORIG_VIDEO_MODE == 0x12) ||	/* 640x480/4 */

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

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

Andi Kleen wrote:
>> +#ifdef CONFIG_XEN
>> +#include "../xen/xen-head.S"
>> +#endif
>>     
>
> Can this really not be linked separately? 

I did a patch to do this (attached).  In principle it should be pretty
simple, but I think I'm running into toolchain issues.  If I link
xen-head.S separately (with appropriate headers added), it compiles OK
and contains the right notes, but they don't appear in the final vmlinux
properly.  The note segment and section are there, but no tools will
parse them as notes:

: ezr:pts/1; readelf -n vmlinux 
: ezr:pts/1; ../../unstable/tools/xcutils/readnotes vmlinux 
: ezr:pts/1; readelf -l vmlinux 

Elf file type is EXEC (Executable file)
Entry point 0x2e1f70
There are 3 program headers, starting at offset 52

Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0xc0100000 0x00100000 0x2a8920 0x2a8920 R E 0x1000
  LOAD           0x2aa000 0xc03a9000 0x003a9000 0x5f085 0xaf000 RWE 0x1000
  NOTE           0x26d2910 0x00239010 0x00239010 0x000e8 0x00000 R   0x4

 Section to Segment mapping:
  Segment Sections...
   00     .text .text.head __ex_table .rodata .pci_fixup __ksymtab __ksymtab_gpl __kcrctab __kcrctab_gpl __ksymtab_strings __param __bug_table 
   01     .data .paravirtprobe .data_nosave .data.page_aligned .data.cacheline_aligned .data.read_mostly .data.init_task .init.text .init.data .init.setup .initcall.init .con_initcall.init .altinstructions .altinstr_replacement .parainstructions .exit.text .init.ramfs .bss 
   02     .notes 
: ezr:pts/1; objdump -j .notes -s vmlinux 

vmlinux:     file format elf32-i386

Contents of section .notes:
 239010 04000000 06000000 06000000 58656e00  ............Xen.
 239020 6c696e75 78000000 04000000 04000000  linux...........
 239030 07000000 58656e00 322e3600 04000000  ....Xen.2.6.....
 239040 08000000 05000000 58656e00 78656e2d  ........Xen.xen-
 239050 332e3000 04000000 04000000 03000000  3.0.............
 239060 58656e00 000000c0 04000000 04000000  Xen.............
 239070 01000000 58656e00 c80710c0 04000000  ....Xen.........
 239080 04000000 02000000 58656e00 00b040c0  ........Xen...@.
 239090 04000000 2a000000 0a000000 58656e00  ....*.......Xen.
 2390a0 21777269 7461626c 655f7061 67655f74  !writable_page_t
 2390b0 61626c65 737c7061 655f7067 6469725f  ables|pae_pgdir_
 2390c0 61626f76 655f3467 62000000 04000000  above_4gb.......
 2390d0 04000000 09000000 58656e00 79657300  ........Xen.yes.
 2390e0 04000000 08000000 08000000 58656e00  ............Xen.
 2390f0 67656e65 72696300                    generic.        


I can't see what the problem is here, but it looks like a toolchain
problem to me (I'm using binutils-2.17.50.0.6-2.fc6).

Any clues?

Thanks,
    J

[-- Attachment #2: xen-head.patch --]
[-- Type: text/x-patch, Size: 1322 bytes --]

diff -r 4721c1690d24 arch/i386/kernel/head.S
--- a/arch/i386/kernel/head.S	Wed Feb 14 03:16:30 2007 -0800
+++ b/arch/i386/kernel/head.S	Wed Feb 14 04:01:58 2007 -0800
@@ -504,7 +504,7 @@ ignore_int:
 
 .section .text
 #ifdef CONFIG_PARAVIRT
-startup_paravirt:
+ENTRY(startup_paravirt)
 	cld
  	movl $(init_thread_union+THREAD_SIZE),%esp
 
@@ -535,10 +535,6 @@ unhandled_paravirt:
 	ud2
 #endif
 
-#ifdef CONFIG_XEN
-#include "../xen/xen-head.S"
-#endif
-	
 /*
  * Real beginning of normal "text" segment
  */
diff -r 4721c1690d24 arch/i386/xen/Makefile
--- a/arch/i386/xen/Makefile	Wed Feb 14 03:16:30 2007 -0800
+++ b/arch/i386/xen/Makefile	Wed Feb 14 04:01:58 2007 -0800
@@ -1,2 +1,2 @@ obj-y		:= enlighten.o setup.o events.o t
-obj-y		:= enlighten.o setup.o events.o time.o \
+obj-y		:= xen-head.o enlighten.o setup.o events.o time.o \
 			features.o mmu.o multicalls.o
diff -r 4721c1690d24 arch/i386/xen/xen-head.S
--- a/arch/i386/xen/xen-head.S	Wed Feb 14 03:16:30 2007 -0800
+++ b/arch/i386/xen/xen-head.S	Wed Feb 14 04:01:58 2007 -0800
@@ -1,8 +1,11 @@
 /* Xen-specific pieces of head.S, intended to be included in the right
 	place in head.S */
 
+.text
 #include <linux/elfnote.h>
+#include <linux/linkage.h>
 #include <asm/boot.h>
+#include <asm/page.h>
 #include <xen/interface/elfnote.h>
 
 ENTRY(startup_xen)

Re: [patch 10/21] Xen-paravirt: Name: dont export paravirt_ops structure, do individual functions

[Christoph Hellwig]

On Tue, Feb 13, 2007 at 05:06:42PM -0800, Zachary Amsden wrote:
> >I moved drm_follow_page into the core, to avoid having to wrap the
> >various pte ops.  Unlining kernel_fpu_end and using that in the RAID6
> >code would remove the need to export clts/read_cr0/write_cr0 too.

Please don't push the drm_follow_page part.  Dave and I fixed drm
to not need this junk anymore, and it should be part of the next drm merge.

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Andi Kleen wrote:
>>> +#ifdef CONFIG_XEN
>>> +#include "../xen/xen-head.S"
>>> +#endif
>>>     
>>
>> Can this really not be linked separately? 
>
> I did a patch to do this (attached).  In principle it should be pretty
> simple, but I think I'm running into toolchain issues.  If I link
> xen-head.S separately (with appropriate headers added), it compiles OK
> and contains the right notes, but they don't appear in the final vmlinux
> properly.  The note segment and section are there, but no tools will
> parse them as notes:
>
> : ezr:pts/1; readelf -n vmlinux 
> : ezr:pts/1; ../../unstable/tools/xcutils/readnotes vmlinux 
> : ezr:pts/1; readelf -l vmlinux 
>
> Elf file type is EXEC (Executable file)
> Entry point 0x2e1f70
> There are 3 program headers, starting at offset 52
>
> Program Headers:
>   Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
>   LOAD           0x001000 0xc0100000 0x00100000 0x2a8920 0x2a8920 R E 0x1000
>   LOAD           0x2aa000 0xc03a9000 0x003a9000 0x5f085 0xaf000 RWE 0x1000
>   NOTE           0x26d2910 0x00239010 0x00239010 0x000e8 0x00000 R   0x4
>
>  Section to Segment mapping:
>   Segment Sections...
>    00 .text .text.head __ex_table .rodata .pci_fixup __ksymtab __ksymtab_gpl
> __kcrctab __kcrctab_gpl __ksymtab_strings __param __bug_table
>    01 .data .paravirtprobe .data_nosave .data.page_aligned
> .data.cacheline_aligned .data.read_mostly .data.init_task .init.text .init.data
> .init.setup .initcall.init .con_initcall.init .altinstructions
> .altinstr_replacement .parainstructions .exit.text .init.ramfs .bss
>    02     .notes 
> : ezr:pts/1; objdump -j .notes -s vmlinux 
>
> vmlinux:     file format elf32-i386
>
> Contents of section .notes:
>  239010 04000000 06000000 06000000 58656e00  ............Xen.
>  239020 6c696e75 78000000 04000000 04000000  linux...........
>  239030 07000000 58656e00 322e3600 04000000  ....Xen.2.6.....
>  239040 08000000 05000000 58656e00 78656e2d  ........Xen.xen-
>  239050 332e3000 04000000 04000000 03000000  3.0.............
>  239060 58656e00 000000c0 04000000 04000000  Xen.............
>  239070 01000000 58656e00 c80710c0 04000000  ....Xen.........
>  239080 04000000 02000000 58656e00 00b040c0  ........Xen...@.
>  239090 04000000 2a000000 0a000000 58656e00  ....*.......Xen.
>  2390a0 21777269 7461626c 655f7061 67655f74  !writable_page_t
>  2390b0 61626c65 737c7061 655f7067 6469725f  ables|pae_pgdir_
>  2390c0 61626f76 655f3467 62000000 04000000  above_4gb.......
>  2390d0 04000000 09000000 58656e00 79657300  ........Xen.yes.
>  2390e0 04000000 08000000 08000000 58656e00  ............Xen.
>  2390f0 67656e65 72696300                    generic.        
>
>
> I can't see what the problem is here, but it looks like a toolchain
> problem to me (I'm using binutils-2.17.50.0.6-2.fc6).

Well I did a little by hand parsing and the not I parsed looked ok.

How does the output differ from a what you get when xen-head.S is
included?

Eric

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

There need to be alignment directives for the page aligned chunks.

Placing the page aligned chunks in a special section is nice in that
it ensures the linker packs everything tightly but should be
completely unnecessary if the alignment is correct.

>  
> ===================================================================
> --- a/arch/i386/kernel/head.S
> +++ b/arch/i386/kernel/head.S
> @@ -519,6 +519,10 @@ 1:
>  	jmp	1b
>  #endif
>  
> +#ifdef CONFIG_XEN
> +#include "../xen/xen-head.S"
> +#endif
> +	
>  /*
>   * Real beginning of normal "text" segment
>   */
> @@ -528,7 +532,7 @@ ENTRY(_stext)
>  /*
>   * BSS section
>   */
> -.section ".bss.page_aligned","w"
> +.section ".bss.page_aligned"
>  ENTRY(swapper_pg_dir)
>  	.fill 1024,4,0
>  ENTRY(empty_zero_page)
> @@ -598,7 +602,8 @@ ENTRY(boot_gdt_table)
>  /*
>   * The Global Descriptor Table contains 28 quadwords, per-CPU.
>   */
> -	.align L1_CACHE_BYTES
> +	.section ".data.page_aligned"
> +	.align PAGE_SIZE_asm
>  ENTRY(cpu_gdt_table)
>  	.quad 0x0000000000000000	/* NULL descriptor */
>  	.quad 0x0000000000000000	/* 0x0b reserved */
> @@ -647,3 +652,6 @@ ENTRY(cpu_gdt_table)
>  	.quad 0x0000000000000000	/* 0xf0 - unused */
>  	.quad 0x0000000000000000 /* 0xf8 - GDT entry 31: double-fault TSS */
>  
> +	/* Be sure this is zeroed to avoid false validations in Xen */
> +	.fill PAGE_SIZE_asm / 8 - GDT_ENTRIES,8,0
> +	.previous

> ===================================================================
> --- a/arch/i386/kernel/vmlinux.lds.S
> +++ b/arch/i386/kernel/vmlinux.lds.S
> @@ -93,6 +93,7 @@ SECTIONS
>  
>    . = ALIGN(4096);
>    .data.page_aligned : AT(ADDR(.data.page_aligned) - LOAD_OFFSET) {
> +	*(.data.page_aligned)
>  	*(.data.idt)
>    }
>  

> --- /dev/null
> +++ b/arch/i386/xen/xen-head.S
> @@ -0,0 +1,29 @@
> +/* Xen-specific pieces of head.S, intended to be included in the right
> +	place in head.S */
> +
> +#include <linux/elfnote.h>
> +#include <asm/boot.h>
> +#include <xen/interface/elfnote.h>
> +
> +ENTRY(startup_xen)
> +	movl %esi,xen_start_info
> +	jmp startup_paravirt
> +	
> +.pushsection ".bss.page_aligned"
> +ENTRY(hypercall_page)
> +	.skip 0x1000
> +.popsection
> +
> +	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,       .asciz, "linux")
> +	ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,  .asciz, "2.6")
> +	ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,    .asciz, "xen-3.0")
> +	ELFNOTE(Xen, XEN_ELFNOTE_VIRT_BASE,      .long,  __PAGE_OFFSET)
> +	ELFNOTE(Xen, XEN_ELFNOTE_ENTRY,          .long,  startup_xen)
> +	ELFNOTE(Xen, XEN_ELFNOTE_HYPERCALL_PAGE, .long,  hypercall_page)
> + ELFNOTE(Xen, XEN_ELFNOTE_FEATURES, .asciz,
> "!writable_page_tables|pae_pgdir_above_4gb")
> +#ifdef CONFIG_X86_PAE
> +	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz, "yes")
> +#else
> +	ELFNOTE(Xen, XEN_ELFNOTE_PAE_MODE,       .asciz, "no")
> +#endif
> +	ELFNOTE(Xen, XEN_ELFNOTE_LOADER,         .asciz, "generic")

Re: [patch 21/21] Xen-paravirt: Add the Xen virtual network device driver.

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> +++ b/drivers/xen/Kconfig.net
> @@ -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.

Am I reading this correctly I can directly use the network interface
of another guest OS (no protection)?

I think this description is misleading, and probably say something
about virtual hardware.

Eric

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> Well I did a little by hand parsing and the not I parsed looked ok.
>
> How does the output differ from a what you get when xen-head.S is
> included?
>   
Ah!

The .notes section gets SHT_NOTE in vmlinux when xen-head.S is included;
PROGBITS when linked.  I tried putting @note on the .section in
linux/elfnote.h, but that didn't seem to help (xen-note.o got a SHT_NOTE
entry, but it didn't make it into vmlinux).  I couldn't see any way of
forcing the section type in vmlinux.lds; my experiments all ended with
ld dumping core.

    J

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> Jeremy Fitzhardinge <jeremy@goop.org> writes:
>
> There need to be alignment directives for the page aligned chunks.
>   

OK.

> Placing the page aligned chunks in a special section is nice in that
> it ensures the linker packs everything tightly but should be
> completely unnecessary if the alignment is correct.
>   
Yes, I made the extra section to get better packing.

    J

Re: [patch 15/21] Xen-paravirt: Add Xen interface header files

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Add 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.
>
> Define macros and inline functions for doing hypercalls into the
> hypervisor.
>
> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
> Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
> Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
>
>
> --
>  include/asm-i386/hypercall.h          |  416 +++++++++++++++++++++++++++++
>  include/asm-i386/hypervisor.h         |   72 +++++

Are hypercall.h and hypervisor.h generic or are they Xen specific.
If they are Xen specific (as it appears) then are inappropriately
named.

>  include/xen/interface/arch-x86_32.h   |  187 +++++++++++++
Why isn't this file include-asm-i386/xen/arch-x86_32.h ?

>  include/xen/interface/elfnote.h       |  133 +++++++++
>  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      |   96 ++++++
>  include/xen/interface/io/console.h    |   23 +
>  include/xen/interface/io/netif.h      |  152 ++++++++++
>  include/xen/interface/io/ring.h       |  260 ++++++++++++++++++
>  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           |  445 ++++++++++++++++++++++++++++++++
>  19 files changed, 2904 insertions(+)

Eric

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Eric W. Biederman wrote:
>> Well I did a little by hand parsing and the not I parsed looked ok.
>>
>> How does the output differ from a what you get when xen-head.S is
>> included?
>>   
> Ah!
>
> The .notes section gets SHT_NOTE in vmlinux when xen-head.S is included;
> PROGBITS when linked.  I tried putting @note on the .section in
> linux/elfnote.h, but that didn't seem to help (xen-note.o got a SHT_NOTE
> entry, but it didn't make it into vmlinux).  I couldn't see any way of
> forcing the section type in vmlinux.lds; my experiments all ended with
> ld dumping core.

Ok.  If that is all this may be a difference that makes no difference.
binutils has a bad habit of looking at sections (which are fully
optional) instead of segments on ET_EXEC and ET_DYN objects.  Only
ET_REL objects (.o files) are required to have sections.

It is worth checking but as long as something looking strictly at the
program headers can find the notes and read them we are in good shape.

That readelf -n has a problem there concerns me a little but as
usually that program is better than the rest of binutils.

So I recommend for testing write a 100 line program that includes
elf.h and reads out the note segment.  If all is well we can split
this code out.

Eric

Re: [patch 15/21] Xen-paravirt: Add Xen interface header files

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> Jeremy Fitzhardinge <jeremy@goop.org> writes:
>
>   
>> Add 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.
>>
>> Define macros and inline functions for doing hypercalls into the
>> hypervisor.
>>
>> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
>> Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
>> Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
>> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
>>
>>
>> --
>>  include/asm-i386/hypercall.h          |  416 +++++++++++++++++++++++++++++
>>  include/asm-i386/hypervisor.h         |   72 +++++
>>     
>
> Are hypercall.h and hypervisor.h generic or are they Xen specific.
> If they are Xen specific (as it appears) then are inappropriately
> named.
>   

Thanks for the reminder; I've been meaning to move/rename these.

>>  include/xen/interface/arch-x86_32.h   |  187 +++++++++++++
>>     
> Why isn't this file include-asm-i386/xen/arch-x86_32.h ?
>   

Those files are more or less directly copied from the Xen tree, and its
easier if they don't drift too far in name and directory structure.

    J

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> Ok.  If that is all this may be a difference that makes no difference.
> binutils has a bad habit of looking at sections (which are fully
> optional) instead of segments on ET_EXEC and ET_DYN objects.  Only
> ET_REL objects (.o files) are required to have sections.
>   

The Xen domain loader will have to be changed to deal with that, which
isn't too much of a problem.

My main concern is the randomness of it, and whether it will fail in
some more harmful way on other versions of binutils.

> So I recommend for testing write a 100 line program that includes
> elf.h and reads out the note segment.  If all is well we can split
> this code out.
>   

The Xen readnotes utility is essentially that.  I'll hack it.

    J

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Eric W. Biederman wrote:
>> Ok.  If that is all this may be a difference that makes no difference.
>> binutils has a bad habit of looking at sections (which are fully
>> optional) instead of segments on ET_EXEC and ET_DYN objects.  Only
>> ET_REL objects (.o files) are required to have sections.
>>   
>
> The Xen domain loader will have to be changed to deal with that, which
> isn't too much of a problem.

Ok.  Please fix the Xen domain loader to not look at sections.  It
is a bug for any kind of executable loader to look at anything other
then segments.

> My main concern is the randomness of it, and whether it will fail in
> some more harmful way on other versions of binutils.

Reasonable and it's probably worth letting the binutils developer know.
I do agree that it is weird.   It might be that something in binutils
doesn't like us dropping some of the notes.

>> So I recommend for testing write a 100 line program that includes
>> elf.h and reads out the note segment.  If all is well we can split
>> this code out.
>>   
>
> The Xen readnotes utility is essentially that.  I'll hack it.

Sounds good.

Eric

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> Reasonable and it's probably worth letting the binutils developer know.
> I do agree that it is weird.   It might be that something in binutils
> doesn't like us dropping some of the notes.
>   

What do you mean by "dropping some of the notes"?  I think the only
notes (at least in this case) are the Xen ones, and they're all included.

    J

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Eric W. Biederman wrote:
>> Reasonable and it's probably worth letting the binutils developer know.
>> I do agree that it is weird.   It might be that something in binutils
>> doesn't like us dropping some of the notes.
>>   
>
> What do you mean by "dropping some of the notes"?  I think the only
> notes (at least in this case) are the Xen ones, and they're all included.

I'm pretty certain we explicitly drop the weird GNU note that
is automatically generated by gcc and specifies something informational.

Basically into .note we include *(.note.*) but not *(.note).

I don't think anything we are doing is wrong but ld gets confused easily
in the corner cases.  I'm modestly surprised we didn't have to mark our
.note.xxx scions as ".section .note.xxx @note"  or whatever the proper
gas syntax is.

Eric

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Jeremy Fitzhardinge]

Eric W. Biederman wrote:
> I'm pretty certain we explicitly drop the weird GNU note that
> is automatically generated by gcc and specifies something informational.
>   
But that's something else again, since it appears as a PT_GNU_STACK phdr.

> I don't think anything we are doing is wrong but ld gets confused easily
> in the corner cases.  I'm modestly surprised we didn't have to mark our
> .note.xxx scions as ".section .note.xxx @note"  or whatever the proper
> gas syntax is.

I did try that, and it didn't make a difference.  The manual says that
the output section type follows the input section type, so I agree its a
bit surprising we ever get a SHT_NOTE out of it without the @note stuff.

    J

Re: [patch 16/21] Xen-paravirt: Add code into head.S to handle being booted by Xen

[Eric W. Biederman]

Jeremy Fitzhardinge <jeremy@goop.org> writes:

> Eric W. Biederman wrote:
>> I'm pretty certain we explicitly drop the weird GNU note that
>> is automatically generated by gcc and specifies something informational.
>>   
> But that's something else again, since it appears as a PT_GNU_STACK phdr.

Not that.  It's more like abi version or gcc version or something
like.  At least there used to be one of those notes in every .o file
and compiled program.

>> I don't think anything we are doing is wrong but ld gets confused easily
>> in the corner cases.  I'm modestly surprised we didn't have to mark our
>> .note.xxx scions as ".section .note.xxx @note"  or whatever the proper
>> gas syntax is.
>
> I did try that, and it didn't make a difference.  The manual says that
> the output section type follows the input section type, so I agree its a
> bit surprising we ever get a SHT_NOTE out of it without the @note stuff.

Right.  So the surprise is that SHT_NOTE got set.  There are some
defaults based on the section name somewhere that appear to have done
the right thing.

My best hunch really is that ld treated the .note sections normally
and just mist the handling of the magic SHT_NOTE type.  Which is why
I'm not to worried.

Eric

Re: [patch 15/21] Xen-paravirt: Add Xen interface header files

[Christoph Hellwig]

On Wed, Feb 14, 2007 at 04:10:50PM -0800, Jeremy Fitzhardinge wrote:
> Eric W. Biederman wrote:
> > Jeremy Fitzhardinge <jeremy@goop.org> writes:
> >
> >   
> >> Add 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.
> >>
> >> Define macros and inline functions for doing hypercalls into the
> >> hypervisor.
> >>
> >> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
> >> Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
> >> Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
> >> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
> >>
> >>
> >> --
> >>  include/asm-i386/hypercall.h          |  416 +++++++++++++++++++++++++++++
> >>  include/asm-i386/hypervisor.h         |   72 +++++
> >>     
> >
> > Are hypercall.h and hypervisor.h generic or are they Xen specific.
> > If they are Xen specific (as it appears) then are inappropriately
> > named.
> >   
> 
> Thanks for the reminder; I've been meaning to move/rename these.
> 
> >>  include/xen/interface/arch-x86_32.h   |  187 +++++++++++++
> >>     
> > Why isn't this file include-asm-i386/xen/arch-x86_32.h ?
> >   
> 
> Those files are more or less directly copied from the Xen tree, and its
> easier if they don't drift too far in name and directory structure.

Nack, we don't put per-arch crap there.  Either you'll have it in separate
places, or you clean up the utterly braindead scheme in the Xen tree
aswell.

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Avi Kivity]

Zachary Amsden wrote:
> Jeremy Fitzhardinge wrote:
>> The XEN config option enables the Xen paravirt_ops interface, which is
>> installed when the kernel finds itself running under Xen. (By some
>> as-yet fully defined mechanism, implemented in a future patch.)
>>
>> Xen is no longer a sub-architecture, so the X86_XEN subarch config
>> option has gone.
>>
>> The disabled config options are:
>> - PREEMPT: Xen doesn't support it
>> - HZ: set to 100Hz for now, to cut down on VCPU context switch rate.
>>   This will be adapted to use tickless later.
>> - kexec: not yet supported
>>
>> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
>> Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
>> Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
>> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
>>   
>
> We do support different HZ values, although 100HZ is actually 
> preferable for us, so I don't object to that.  

Ditto for kvm.

> PREEMPT is supported by us, but not as tested as I would like, so I 
> also don't object to dropping it for generic paravirt guests - Rusty - 
> Avi any objections to dropping preempt in terms of lguest/KVM 
> paravirtualization?

I don't have any objections myself, but Ingo (who has done the bulk of 
the kvm paravirt work; cc'ed) uses PREEMPT_RT, so he will certainly object.

>
> Paravirt-ops definitely needs a hook for kexec, although we should not 
> disable kexec for the natively booted paravirt-ops.  Eric - is there a 
> way to disable it at runtime?

kvm paravirt should work correctly with kexec.

>
> We do support the doublefault task gate, and it would be good to keep 
> it, but I can't complain so much if it is gone from generic paravirt 
> kernels for now, because it is non-essential, and generally fatal 
> anyway.  We do need it for native boots of paravirt-ops kernels, 
> however, so turning off the config option still needs to be revisited.

kvm doesn't support task gates (a task switch will immediately kill the 
guest).


-- 
error compiling committee.c: too many arguments to function

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Keir Fraser]

On 16/2/07 08:37, "Dan Hecht" <dhecht@vmware.com> wrote:

> Hmm?  I thought the periodic timer and one-shot timer both generate the
> same VIRQ.  So, how can you mask one without masking the other?
> 
> The tickless idle works since the block hypercall disables the periodic
> timer.  But for dynticks (aka NO_HZ), you'll need to mask the periodic
> timer (even for a running vcpu), while keeping the one-shot timer
> unmasked.  I don't think Xen provides an interface to do that.

There's no interface for this right now, but we can easily add one in a
backward compatible way. This also will not prevent a dyntick kernel from
running on an older Xen -- the guest will then receive to many timer
interrupts, but that should only be a performance concern fixable by
upgrading the hypervisor component of the system.

 -- Keir

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Keir Fraser]

On 16/2/07 07:25, "Jeremy Fitzhardinge" <jeremy@goop.org> wrote:

>> Oh, so that's why it doesn't break when CONFIG_PREEMPT=y.  In which case
>> that preempt_disable() I spotted is wrong-and-unneeded.
>> 
>> Why doesn't Xen work with preemption??
> 
> I've forgotten the details.  Ian?  Keir?  Steven?  Maybe it can be done.

It breaks guest save/restore for us currently because threads can be
sleeping with machine addresses in local storage (registers, stack). There
are a few ways to achieve an acceptable solution:

 1. Put processes in the freezer when we suspend. This should avoid any
thread being in a critical section with machine addresses in its hand. We
haven't yet investigated the performance impact of freezing processes,
particularly on the downtime of live relocation.

 2. Allow CONFIG_PREEMPT to be compiled in, but disable it at runtime. We
could do this by, for example, reserving a bit in preempt_count() so that
most preemption checks do not touch any more cache lines. I guess it would
need a bit of fixing up (e.g., so that in_atomic() would not be always
asserted). Even better for us would be to allow switching between
involuntary and voluntary preemption at runtime. It looks as though the hook
points for these two techniques are not usually compiled in at the same
time, however.

 -- Keir

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Dan Hecht]

On 02/16/2007 12:05 AM, Jeremy Fitzhardinge wrote:
> Dan Hecht wrote:
>> On 02/15/2007 11:04 PM, Jeremy Fitzhardinge wrote:
>>> HZ - I'm assuming dynticks will appear in the short term, and this will
>>> become moot
>> Doesn't Xen send any non-blocked domain a 100hz alarm implicitly,
>> without anyway for the guest to disable it?  I guess you'll have to
>> break kernel/hypervisor compatibility if you want dynticks? 
> 
> The timer VIRQ will generate events at a fixed 100Hz (which can be
> masked, like any other event), but you can also ask for a timer event at
> a particular time.  We do this already for tickless idle.
> 
>     J
> 

Hmm?  I thought the periodic timer and one-shot timer both generate the 
same VIRQ.  So, how can you mask one without masking the other?

The tickless idle works since the block hypercall disables the periodic 
timer.  But for dynticks (aka NO_HZ), you'll need to mask the periodic 
timer (even for a running vcpu), while keeping the one-shot timer 
unmasked.  I don't think Xen provides an interface to do that.

Dan

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Zachary Amsden]

Eric W. Biederman wrote:
> Ian Campbell <Ian.Campbell@XenSource.com> writes:
>
>   
>> On Fri, 2007-02-16 at 00:33 -0700, Eric W. Biederman wrote:
>>     
>>> I know there actually has been some work to get kexec actually working under
>>> Xen but I don't know where that has gone.
>>>       
>> kexec/kdump works with Xen 3.0.4 but it's a dom0 only thing so it
>> doesn't appear in this patchset.
>>     
>
> Ok but we still need to get the failure to user space for domU instead
> of trying what works on real hardware and failing.
>
> Eric

Acked-by: Zachary Amsden <zach@vmware.com>

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Eric W. Biederman]

Ian Campbell <Ian.Campbell@XenSource.com> writes:

> On Fri, 2007-02-16 at 00:33 -0700, Eric W. Biederman wrote:
>> I know there actually has been some work to get kexec actually working under
>> Xen but I don't know where that has gone.
>
> kexec/kdump works with Xen 3.0.4 but it's a dom0 only thing so it
> doesn't appear in this patchset.

Ok but we still need to get the failure to user space for domU instead
of trying what works on real hardware and failing.

Eric

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Ian Campbell]

On Fri, 2007-02-16 at 00:33 -0700, Eric W. Biederman wrote:
> I know there actually has been some work to get kexec actually working under
> Xen but I don't know where that has gone.

kexec/kdump works with Xen 3.0.4 but it's a dom0 only thing so it
doesn't appear in this patchset.

Ian.

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Jeremy Fitzhardinge]

Dan Hecht wrote:
> On 02/15/2007 11:04 PM, Jeremy Fitzhardinge wrote:
>> HZ - I'm assuming dynticks will appear in the short term, and this will
>> become moot
>
> Doesn't Xen send any non-blocked domain a 100hz alarm implicitly,
> without anyway for the guest to disable it?  I guess you'll have to
> break kernel/hypervisor compatibility if you want dynticks? 

The timer VIRQ will generate events at a fixed 100Hz (which can be
masked, like any other event), but you can also ask for a timer event at
a particular time.  We do this already for tickless idle.

    J

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Dan Hecht]

On 02/15/2007 11:04 PM, Jeremy Fitzhardinge wrote:
> HZ - I'm assuming dynticks will appear in the short term, and this will
> become moot

Doesn't Xen send any non-blocked domain a 100hz alarm implicitly, 
without anyway for the guest to disable it?  I guess you'll have to 
break kernel/hypervisor compatibility if you want dynticks?

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Eric W. Biederman]

Zachary Amsden <zach@vmware.com> writes:

>
> We do support different HZ values, although 100HZ is actually preferable for us,
> so I don't object to that.  PREEMPT is supported by us, but not as tested as I
> would like, so I also don't object to dropping it for generic paravirt guests -
> Rusty - Avi any objections to dropping preempt in terms of lguest/KVM
> paravirtualization?
>
> Paravirt-ops definitely needs a hook for kexec, although we should not disable
> kexec for the natively booted paravirt-ops.  Eric - is there a way to disable it
> at runtime?
>
> We do support the doublefault task gate, and it would be good to keep it, but I
> can't complain so much if it is gone from generic paravirt kernels for now,
> because it is non-essential, and generally fatal anyway.  We do need it for
> native boots of paravirt-ops kernels, however, so turning off the config option
> still needs to be revisited

Have machine_kexec_prepare fail.

I think your machine description or paravirt_ops or whatever it is needs
to hook both machine_kexec_prepare and machine_kexec.

I know there actually has been some work to get kexec actually working under
Xen but I don't know where that has gone.

Eric

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Jeremy Fitzhardinge]

Andrew Morton wrote:
> On Thu, 15 Feb 2007 22:14:45 -0800 Dan Hecht <dhecht@vmware.com> wrote:
>
>   
>>>  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)
>>>
>>>       
>
> Oh, so that's why it doesn't break when CONFIG_PREEMPT=y.  In which case
> that preempt_disable() I spotted is wrong-and-unneeded.
>
> Why doesn't Xen work with preemption??
>   

I've forgotten the details.  Ian?  Keir?  Steven?  Maybe it can be done.

    J

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Andrew Morton]

On Thu, 15 Feb 2007 22:14:45 -0800 Dan Hecht <dhecht@vmware.com> wrote:

> >  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)
> > 

Oh, so that's why it doesn't break when CONFIG_PREEMPT=y.  In which case
that preempt_disable() I spotted is wrong-and-unneeded.

Why doesn't Xen work with preemption??

> I hate to sound like a broken record, but this really isn't the right 
> way to do this.  If you are going to inhibit config settings when Xen 
> support is compiled, then it should be:
> 
> config XEN
>      depends on !KEXEC && !DOUBLEFAULT && HZ_100 && !PREEMPT
> 

Agree.

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Jeremy Fitzhardinge]

Dan Hecht wrote:
> 1) Complete the xen paravirt-ops backend so it can handle these
> "incompatible" options.  I realize this is just a matter of time (at
> least for most of them, what is your plan for PREEMPT?).

Hope it will go away?  There's some relatively deep reason to do with
migration which makes preempt very difficult to support, but I've
forgotten the details.  I've been wondering if there's some way to make
it runtime selectable without massive performance cost.

> 2) Disable the option at runtime only if the kernel is booted on Xen.
> When the kernel is booted on native, lhype, paravirt kvm, vmware, etc
> it should be not be inhibited.  This may not be feasible for all of
> these options, but as Zach pointed out, is easy enough for
> DOUBLEFAULT.  Maybe it can be done for KEXEC?  And HZ is easy to allow
> too, even though Xen will still give interrupts at 100hz -- we do this
> when you boot on VMI.  PREEMPT is probably the only real compile time
> incompatible options with Xen.  You just simply have to change the
> loop decrementors in your timer interrupt.
>
> You basically chose #2 for SMP: while your backend doesn't support it
> yet, it's not harmful to have the config option enabled; you just
> don't allow a second vcpu to startup when running on Xen.

Yes, this has been my preferred approach.  So for each of the restrictions:
PREEMPT - hard, will try to do something at runtime
HZ - I'm assuming dynticks will appear in the short term, and this will
become moot
KEXEC - there's been some work on making KEXEC and Xen play nicely
together; I was waiting to see if that matures, and/or make it runtime
switchable in the meantime
DOUBLEFAULT - not really an issue; guests won't get them under Xen (I
think) and we can ignore setting the gate pretty easily

    J

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Zachary Amsden]

Jeremy Fitzhardinge wrote:
> The XEN config option enables the Xen paravirt_ops interface, which is
> installed when the kernel finds itself running under Xen. (By some
> as-yet fully defined mechanism, implemented in a future patch.)
>
> Xen is no longer a sub-architecture, so the X86_XEN subarch config
> option has gone.
>
> The disabled config options are:
> - PREEMPT: Xen doesn't support it
> - HZ: set to 100Hz for now, to cut down on VCPU context switch rate.
>   This will be adapted to use tickless later.
> - kexec: not yet supported
>
> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
> Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
> Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
> Signed-off-by: Chris Wright <chrisw@sous-sol.org>
>   

We do support different HZ values, although 100HZ is actually preferable 
for us, so I don't object to that.  PREEMPT is supported by us, but not 
as tested as I would like, so I also don't object to dropping it for 
generic paravirt guests - Rusty - Avi any objections to dropping preempt 
in terms of lguest/KVM paravirtualization?

Paravirt-ops definitely needs a hook for kexec, although we should not 
disable kexec for the natively booted paravirt-ops.  Eric - is there a 
way to disable it at runtime?

We do support the doublefault task gate, and it would be good to keep 
it, but I can't complain so much if it is gone from generic paravirt 
kernels for now, because it is non-essential, and generally fatal 
anyway.  We do need it for native boots of paravirt-ops kernels, 
however, so turning off the config option still needs to be revisited.

Zach

Re: [patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Dan Hecht]

On 02/15/2007 06:25 PM, Jeremy Fitzhardinge wrote:
> The XEN config option enables the Xen paravirt_ops interface, which is
> installed when the kernel finds itself running under Xen. (By some
> as-yet fully defined mechanism, implemented in a future patch.)
> 
> Xen is no longer a sub-architecture, so the X86_XEN subarch config
> option has gone.
> 
> The disabled config options are:
> - PREEMPT: Xen doesn't support it
> - HZ: set to 100Hz for now, to cut down on VCPU context switch rate.
>   This will be adapted to use tickless later.
> - kexec: not yet supported
> 

>  
>  config KEXEC
>  	bool "kexec system call"
> +	depends on !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

>  config DOUBLEFAULT
>  	default y
>  	bool "Enable doublefault exception handler" if EMBEDDED
> +	depends on !XEN
>  	help
>            This option allows trapping of rare doublefault exceptions that
>            would otherwise cause a system to silently reboot. Disabling this


> +config XEN
> +	bool "Enable support for Xen hypervisor"
> +	depends PARAVIRT
> +	default y
> +	help
> +	  This is the Linux Xen port.

>  choice
> -	prompt "Timer frequency"
> +	prompt "Timer frequency" if !XEN
>  	default HZ_250
>  	help
>  	 Allows the configuration of the timer frequency. It is customary
> @@ -49,7 +49,7 @@ endchoice
>  
>  config HZ
>  	int
> -	default 100 if HZ_100
> +	default 100 if HZ_100 || XEN
>  	default 250 if HZ_250
>  	default 300 if HZ_300
>  	default 1000 if HZ_1000


>  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)
> 

I hate to sound like a broken record, but this really isn't the right 
way to do this.  If you are going to inhibit config settings when Xen 
support is compiled, then it should be:

config XEN
     depends on !KEXEC && !DOUBLEFAULT && HZ_100 && !PREEMPT

I'm really not trying to make things more difficult for you or for Xen 
users.

One of the primary goals of paravirt-ops is to allow the same kernel 
binary to bind to multiple hypervisor interfaces (and native), at 
runtime.  So, we should assume that kernels which are built with 
CONFIG_PARAVIRT will also enable all the paravirt-ops backends; and this 
is a good thing.

But, the problem is, then, when someone enables paravirt (and all the 
backends, including Xen), you are silently forcing all these options 
off.  Compiling in paravirt support should not change force you into 
using some set of config option; when a paravirt-ops kernel is run on 
native, it should be as fully featured as a non-paravirtops kernel.

So, this should really be solved (in order of preference) by:

1) Complete the xen paravirt-ops backend so it can handle these 
"incompatible" options.  I realize this is just a matter of time (at 
least for most of them, what is your plan for PREEMPT?).

2) Disable the option at runtime only if the kernel is booted on Xen. 
When the kernel is booted on native, lhype, paravirt kvm, vmware, etc it 
should be not be inhibited.  This may not be feasible for all of these 
options, but as Zach pointed out, is easy enough for DOUBLEFAULT.  Maybe 
it can be done for KEXEC?  And HZ is easy to allow too, even though Xen 
will still give interrupts at 100hz -- we do this when you boot on VMI. 
  PREEMPT is probably the only real compile time incompatible options 
with Xen.  You just simply have to change the loop decrementors in your 
timer interrupt.

You basically chose #2 for SMP: while your backend doesn't support it 
yet, it's not harmful to have the config option enabled; you just don't 
allow a second vcpu to startup when running on Xen.

3) Use the config XEN line I suggest above.  The net effect is the same 
as your proposed change (it prevents users from compiling the 
incompatible options together), but at least the user understands what 
is going on, instead of options just silently changing on them.

Dan

[patch 14/21] Xen-paravirt: Add XEN config options and disable unsupported config options.

[Jeremy Fitzhardinge]

[-- Attachment #1: xen-config.patch --]
[-- Type: text/plain, Size: 3697 bytes --]

The XEN config option enables the Xen paravirt_ops interface, which is
installed when the kernel finds itself running under Xen. (By some
as-yet fully defined mechanism, implemented in a future patch.)

Xen is no longer a sub-architecture, so the X86_XEN subarch config
option has gone.

The disabled config options are:
- PREEMPT: Xen doesn't support it
- HZ: set to 100Hz for now, to cut down on VCPU context switch rate.
  This will be adapted to use tickless later.
- kexec: not yet supported

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Signed-off-by: Ian Pratt <ian.pratt@xensource.com>
Signed-off-by: Christian Limpach <Christian.Limpach@cl.cam.ac.uk>
Signed-off-by: Chris Wright <chrisw@sous-sol.org>

---
 arch/i386/Kconfig       |    7 +++++--
 arch/i386/Kconfig.debug |    1 +
 arch/i386/xen/Kconfig   |   10 ++++++++++
 kernel/Kconfig.hz       |    4 ++--
 kernel/Kconfig.preempt  |    1 +
 5 files changed, 19 insertions(+), 4 deletions(-)

===================================================================
--- a/arch/i386/Kconfig
+++ b/arch/i386/Kconfig
@@ -192,6 +192,8 @@ config PARAVIRT
 	  under a hypervisor, improving performance significantly.
 	  However, when run without a hypervisor the kernel is
 	  theoretically slower.  If in doubt, say N.
+
+source "arch/i386/xen/Kconfig"
 
 config ACPI_SRAT
 	bool
@@ -298,12 +300,12 @@ config X86_UP_IOAPIC
 
 config X86_LOCAL_APIC
 	bool
-	depends on X86_UP_APIC || ((X86_VISWS || SMP) && !X86_VOYAGER) || X86_GENERICARCH
+	depends on X86_UP_APIC || (((X86_VISWS || SMP) && !X86_VOYAGER) || X86_GENERICARCH)
 	default y
 
 config X86_IO_APIC
 	bool
-	depends on X86_UP_IOAPIC || (SMP && !(X86_VISWS || X86_VOYAGER)) || X86_GENERICARCH
+	depends on X86_UP_IOAPIC || ((SMP && !(X86_VISWS || X86_VOYAGER)) || X86_GENERICARCH)
 	default y
 
 config X86_VISWS_APIC
@@ -743,6 +745,7 @@ source kernel/Kconfig.hz
 
 config KEXEC
 	bool "kexec system call"
+	depends on !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
===================================================================
--- a/arch/i386/Kconfig.debug
+++ b/arch/i386/Kconfig.debug
@@ -79,6 +79,7 @@ config DOUBLEFAULT
 config DOUBLEFAULT
 	default y
 	bool "Enable doublefault exception handler" if EMBEDDED
+	depends on !XEN
 	help
           This option allows trapping of rare doublefault exceptions that
           would otherwise cause a system to silently reboot. Disabling this
===================================================================
--- /dev/null
+++ b/arch/i386/xen/Kconfig
@@ -0,0 +1,10 @@
+#
+# This Kconfig describes xen options
+#
+
+config XEN
+	bool "Enable support for Xen hypervisor"
+	depends PARAVIRT
+	default y
+	help
+	  This is the Linux Xen port.
===================================================================
--- a/kernel/Kconfig.hz
+++ b/kernel/Kconfig.hz
@@ -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
@@ -49,7 +49,7 @@ endchoice
 
 config HZ
 	int
-	default 100 if HZ_100
+	default 100 if HZ_100 || XEN
 	default 250 if HZ_250
 	default 300 if HZ_300
 	default 1000 if HZ_1000
===================================================================
--- a/kernel/Kconfig.preempt
+++ b/kernel/Kconfig.preempt
@@ -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)

--