| Message ID | 1470438282-4226-5-git-send-email-daniel.kiper@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 06.08.16 at 01:04, <daniel.kiper@oracle.com> wrote: > Next patch will leave just required jmp instruction > in xen/x86/boot/reloc.c. I can't make sense of this now, and it'll get even more problematic for archaeologists if the two patches don't get committed one right after the other. Please instead describe what _this_ patch does and why. > --- a/xen/arch/x86/boot/build32.lds > +++ b/xen/arch/x86/boot/build32.lds > @@ -24,6 +24,7 @@ SECTIONS > *(.text) > *(.text.*) > *(.rodata) > + *(.bss) The suggested change to the earlier patch would make this unnecessary, but here you get to see even more clearly why picking just a few sections is bogus. > static void *reloc_mbi_struct(void *old, unsigned int bytes) > { > void *new; > - asm( > - " call 1f \n" > - "1: pop %%edx \n" > - " mov alloc-1b(%%edx),%0 \n" > - " sub %1,%0 \n" > - " and $~15,%0 \n" > - " mov %0,alloc-1b(%%edx) \n" > - " mov %0,%%edi \n" > - " rep movsb \n" > - : "=&r" (new), "+c" (bytes), "+S" (old) > - : : "edx", "edi", "memory"); > - return new; > + > + alloc -= ALIGN_UP(bytes, 16); > + new = (void *)alloc; > + > + while ( bytes-- ) > + *(char *)new++ = *(char *)old++; > + > + return (void *)alloc; > } To further cut down the number of casts, what about making new have type char * and doing while ( bytes-- ) new[bytes] = ((char *)old)[bytes]; return new; One might even argue old could also be of type char * (and actually be const), but that would only move the cast into the caller. Yet perhaps that's still better readable than the expression above. And then, maybe the code could even mostly stay as it is: Is there anything keeping alloc from being of type void *? Jan
diff --git a/xen/arch/x86/boot/build32.lds b/xen/arch/x86/boot/build32.lds index b14c7d5..a658ca8 100644 --- a/xen/arch/x86/boot/build32.lds +++ b/xen/arch/x86/boot/build32.lds @@ -24,6 +24,7 @@ SECTIONS *(.text) *(.text.*) *(.rodata) + *(.bss) } /DISCARD/ : { diff --git a/xen/arch/x86/boot/build32.mk b/xen/arch/x86/boot/build32.mk index eb02b4b..d54d259 100644 --- a/xen/arch/x86/boot/build32.mk +++ b/xen/arch/x86/boot/build32.mk @@ -23,7 +23,7 @@ CFLAGS := $(filter-out -flto,$(CFLAGS)) $(OBJDUMP) -h $< | sed -n '/[0-9]/{s,00*,0,g;p;}' |\ while read idx name sz rest; do \ case "$$name" in \ - .data|.data.*|.rodata.*|.bss|.bss.*) \ + .data|.data.*|.rodata.*|.bss.*) \ test $$sz != 0 || continue; \ echo "Error: non-empty $$name: 0x$$sz" >&2; \ exit $$(expr $$idx + 1);; \ diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c index 63045c0..9ae42e2 100644 --- a/xen/arch/x86/boot/reloc.c +++ b/xen/arch/x86/boot/reloc.c @@ -15,39 +15,33 @@ asm ( " .text \n" " .globl _start \n" "_start: \n" - " call 1f \n" - "1: pop %ebx \n" - " mov %eax,alloc-1b(%ebx) \n" - " jmp reloc \n" - ); - -/* - * This is our data. Because the code must be relocatable, no BSS is - * allowed. All data is accessed PC-relative with inline assembly. - */ -asm ( - "alloc: \n" - " .long 0 \n" + " push %eax \n" + " push 0x8(%esp) \n" + " call reloc \n" + " ret $0x4 \n" ); typedef unsigned int u32; #include "../../../include/xen/multiboot.h" +#define __stdcall __attribute__((__stdcall__)) + +#define ALIGN_UP(arg, align) \ + (((arg) + (align) - 1) & ~((typeof(arg))(align) - 1)) + +static u32 alloc; + static void *reloc_mbi_struct(void *old, unsigned int bytes) { void *new; - asm( - " call 1f \n" - "1: pop %%edx \n" - " mov alloc-1b(%%edx),%0 \n" - " sub %1,%0 \n" - " and $~15,%0 \n" - " mov %0,alloc-1b(%%edx) \n" - " mov %0,%%edi \n" - " rep movsb \n" - : "=&r" (new), "+c" (bytes), "+S" (old) - : : "edx", "edi", "memory"); - return new; + + alloc -= ALIGN_UP(bytes, 16); + new = (void *)alloc; + + while ( bytes-- ) + *(char *)new++ = *(char *)old++; + + return (void *)alloc; } static char *reloc_mbi_string(char *old) @@ -58,11 +52,15 @@ static char *reloc_mbi_string(char *old) return reloc_mbi_struct(old, p - old + 1); } -multiboot_info_t *reloc(multiboot_info_t *mbi_old) +multiboot_info_t __stdcall *reloc(multiboot_info_t *mbi_old, u32 trampoline) { - multiboot_info_t *mbi = reloc_mbi_struct(mbi_old, sizeof(*mbi)); + multiboot_info_t *mbi; int i; + alloc = trampoline; + + mbi = reloc_mbi_struct(mbi_old, sizeof(*mbi)); + if ( mbi->flags & MBI_CMDLINE ) mbi->cmdline = (u32)reloc_mbi_string((char *)mbi->cmdline);
Next patch will leave just required jmp instruction in xen/x86/boot/reloc.c. Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com> --- xen/arch/x86/boot/build32.lds | 1 + xen/arch/x86/boot/build32.mk | 2 +- xen/arch/x86/boot/reloc.c | 52 ++++++++++++++++++++--------------------- 3 files changed, 27 insertions(+), 28 deletions(-)