[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] Avoid additional relocations in trampoline code
On 22.08.2024 17:29, Frediano Ziglio wrote: > The trampoline could have "manual" relocation entries (created > by assembly macros and some code to use them) and (in case of PE) > normal executable relocations. > Remove all normal executable ones. In this way we don't have to > worry about applying both correctly (they need proper order > which is hard to spot looking at the code). > > Signed-off-by: Frediano Ziglio <frediano.ziglio@xxxxxxxxx> I think this wants splitting into one patch replacing sym_offs() and a 2nd one introducing the hand-crafted __XEN_VIRT_START additions (which may want macro-izing). Plus the justification for the change wants extending, to actually explain what the problem is - after all there's no issue anywhere right now. > --- a/xen/arch/x86/boot/trampoline.S > +++ b/xen/arch/x86/boot/trampoline.S > @@ -73,7 +73,7 @@ trampoline_protmode_entry: > mov %ecx,%cr4 > > /* Load pagetable base register. */ > - mov $sym_offs(idle_pg_table),%eax > + mov $idle_pg_table_pa, %eax > add bootsym_rel(trampoline_xen_phys_start,4,%eax) > mov %eax,%cr3 > > @@ -113,7 +113,7 @@ trampoline_protmode_entry: > .code64 > start64: > /* Jump to high mappings. */ > - movabs $__high_start, %rdi > + movabs $__high_start_pa + __XEN_VIRT_START, %rdi > jmpq *%rdi > > #include "video.h" > --- a/xen/arch/x86/boot/wakeup.S > +++ b/xen/arch/x86/boot/wakeup.S > @@ -99,7 +99,7 @@ wakeup_32: > mov $bootsym_rel(wakeup_stack, 4, %esp) > > # check saved magic again > - mov $sym_offs(saved_magic),%eax > + mov $saved_magic_pa, %eax > add bootsym_rel(trampoline_xen_phys_start, 4, %eax) > mov (%eax), %eax > cmp $0x9abcdef0, %eax > @@ -112,7 +112,7 @@ wakeup_32: > mov %ecx, %cr4 > > /* Load pagetable base register */ > - mov $sym_offs(idle_pg_table),%eax > + mov $idle_pg_table_pa ,%eax > add bootsym_rel(trampoline_xen_phys_start,4,%eax) > mov %eax,%cr3 > > @@ -156,7 +156,7 @@ wakeup_32: > .code64 > wakeup_64: > /* Jump to high mappings and the higher-level wakeup code. */ > - movabs $s3_resume, %rbx > + movabs $s3_resume_pa + __XEN_VIRT_START, %rbx > jmp *%rbx > > bogus_saved_magic: With the sym_offs() uses gone, I think it would be best if the macro was #undef-ed ahead of the inclusion of trampoline.S. Since x86_64.S uses the macro, that'll require careful re-arrangement of #include order. > --- a/xen/arch/x86/xen.lds.S > +++ b/xen/arch/x86/xen.lds.S > @@ -71,7 +71,12 @@ SECTIONS > __2M_text_start = .; /* Start of 2M superpages, mapped RX. */ > #endif > > - start_pa = ABSOLUTE(start - __XEN_VIRT_START); > +#define DEFINE_PA_ADDRESS(sym) sym ## _pa = ABSOLUTE(sym - __XEN_VIRT_START) > + DEFINE_PA_ADDRESS(start); > + DEFINE_PA_ADDRESS(saved_magic); > + DEFINE_PA_ADDRESS(idle_pg_table); > + DEFINE_PA_ADDRESS(__high_start); > + DEFINE_PA_ADDRESS(s3_resume); > > . = __XEN_VIRT_START + XEN_IMG_OFFSET; > _start = .; For the cases where in assembly code you add __XEN_VIRT_START this is pretty odd: You subtract the value here just to add it back there. Did you consider a more straightforward approach, like introducing absolute xxx_va symbols? Also, as a general request: Can you please become used to adding meaningful subject prefixes to your patches? Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |