> --- 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
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
>>> @@ -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
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
Jeremy Fitzhardinge <[email protected]> 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
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
Jeremy Fitzhardinge <[email protected]> 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
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
Jeremy Fitzhardinge <[email protected]> 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
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
Jeremy Fitzhardinge <[email protected]> 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
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
Jeremy Fitzhardinge <[email protected]> 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