From: "Jan Beulich" <JBeulich@suse.com>
To: Daniel Kiper <daniel.kiper@oracle.com>
Cc: Juergen Gross <JGross@suse.com>,
stefano.stabellini@eu.citrix.com,
Andrew Cooper <andrew.cooper3@citrix.com>,
cardoe@cardoe.com, pgnet.dev@gmail.com, ning.sun@intel.com,
david.vrabel@citrix.com, xen-devel@lists.xenproject.org,
qiaowei.ren@intel.com, richard.l.maliszewski@intel.com,
gang.wei@intel.com, fu.wei@linaro.org
Subject: Re: [PATCH v3 03/16] x86/boot: call reloc() using cdecl calling convention
Date: Fri, 17 Jun 2016 03:30:45 -0600 [thread overview]
Message-ID: <5763DF6502000078000F600B@prv-mh.provo.novell.com> (raw)
In-Reply-To: <20160617084156.GL5490@olila.local.net-space.pl>
>>> On 17.06.16 at 10:41, <daniel.kiper@oracle.com> wrote:
> On Fri, Apr 15, 2016 at 04:56:26PM +0100, Andrew Cooper wrote:
>> On 15/04/16 13:33, Daniel Kiper wrote:
>> > reloc() is not called according to cdecl calling convention.
>> > This makes confusion and does not scale well for more arguments.
>> > And patch adding multiboot2 protocol support have to pass 3
>> > arguments instead of 2. Hence, move reloc() call to cdecl
>> > calling convention.
>> >
>> > I add push %ebp/mov %esp,%ebp/leave instructions here. Though they
>> > are not strictly needed in this patch. However, then assembly code
>> > in patch adding multiboot2 protocol support is easier to read.
>> >
>> > Suggested-by: Jan Beulich <jbeulich@suse.com>
>> > Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>
>> > ---
>> > v3 - suggestions/fixes:
>> > - simplify assembly in xen/arch/x86/boot/reloc.c file
>> > (suggested by Jan Beulich),
>> > - reorder arguments for reloc() call from xen/arch/x86/boot/head.S
>> > (suggested by Jan Beulich),
>> > - improve commit message
>> > (suggested by Jan Beulich).
>> > ---
>> > xen/arch/x86/boot/head.S | 4 +++-
>> > xen/arch/x86/boot/reloc.c | 18 ++++++++++++++----
>> > 2 files changed, 17 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
>> > index 32a54a0..28ac721 100644
>> > --- a/xen/arch/x86/boot/head.S
>> > +++ b/xen/arch/x86/boot/head.S
>> > @@ -119,8 +119,10 @@ __start:
>> >
>> > /* Save the Multiboot info struct (after relocation) for later
> use. */
>> > mov $sym_phys(cpu0_stack)+1024,%esp
>> > - push %ebx
>> > + push %eax /* Boot trampoline address. */
>> > + push %ebx /* Multiboot information address. */
>> > call reloc
>> > + add $8,%esp /* Remove reloc() args from stack. */
>> > mov %eax,sym_phys(multiboot_ptr)
>> >
>> > /* Initialize BSS (no nasty surprises!). */
>> > diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
>> > index 63045c0..006f41d 100644
>> > --- a/xen/arch/x86/boot/reloc.c
>> > +++ b/xen/arch/x86/boot/reloc.c
>> > @@ -10,15 +10,25 @@
>> > * Keir Fraser <keir@xen.org>
>> > */
>> >
>> > -/* entered with %eax = BOOT_TRAMPOLINE */
>> > +/*
>> > + * This entry point is entered from xen/arch/x86/boot/head.S with:
>> > + * - 0x4(%esp) = MULTIBOOT_INFORMATION_ADDRESS,
>> > + * - 0x8(%esp) = BOOT_TRAMPOLINE_ADDRESS.
>> > + */
>> > asm (
>> > " .text \n"
>> > " .globl _start \n"
>> > "_start: \n"
>> > + " push %ebp \n"
>> > + " mov %esp,%ebp \n"
>> > " call 1f \n"
>> > - "1: pop %ebx \n"
>> > - " mov %eax,alloc-1b(%ebx) \n"
>> > - " jmp reloc \n"
>> > + "1: pop %ecx \n"
>> > + " mov 0xc(%ebp),%eax \n"
>> > + " mov %eax,alloc-1b(%ecx) \n"
>> > + " push 0x8(%ebp) \n"
>> > + " call reloc \n"
>> > + " leave \n"
>> > + " ret \n"
>> > );
>> >
>> > /*
>>
>> Come to think of this, why are we playing asm games like this at all?
>>
>> This object file gets linked with head.o anyway, and the reloc()
>> function is safe to live anywhere in .init.text. It might be worth
>
> It does not. reloc.c is converted to asm and then included in head.S.
> However,
> as we discussed during hackhaton I tried to link reloc.o directly with other
> objects. As we expected it is easy to convert 32-bit ELF to 64-bit ELF file.
> Though ld fails. As I saw the main problem is that virtual addresses start at
> 0xffff82d080200000. This value simply overflows 32-bit relocations, e.g.:
>
> prelink.o: In function `reloc':
> (.text+0x359): relocation truncated to fit: R_X86_64_32 against `.text'
>
> There is a chance that we can fix it by changing virtual addresses to
> physical addresses. At first sight it should work because final 32-bit
> ELF image contains only physical addresses in ELF headers. However, I am
> not sure that this way we will not break something. Hmmm... I have just
> realized that at least debugging and crash analysis of hypervisor memory
> could be more difficult. Am I missing anything else?
I don't think you can switch the linking process to use physical
addresses: The final image has to have proper virtual addresses
used for any involved relocations. This would only be benign if
_all_ relocations were relative ones.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2016-06-17 9:30 UTC|newest]
Thread overview: 94+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-15 12:33 [PATCH v3 00/16] x86: multiboot2 protocol support Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 01/16] x86/boot: do not create unwind tables Daniel Kiper
2016-04-15 15:45 ` Andrew Cooper
2016-04-15 12:33 ` [PATCH v3 02/16] x86: zero BSS using stosl instead of stosb Daniel Kiper
2016-04-15 13:57 ` Konrad Rzeszutek Wilk
2016-04-15 15:48 ` Andrew Cooper
2016-04-15 12:33 ` [PATCH v3 03/16] x86/boot: call reloc() using cdecl calling convention Daniel Kiper
2016-04-15 15:56 ` Andrew Cooper
2016-06-17 8:41 ` Daniel Kiper
2016-06-17 9:30 ` Jan Beulich [this message]
2016-05-24 8:42 ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 04/16] x86/boot/reloc: create generic alloc and copy functions Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 05/16] x86/boot: use %ecx instead of %eax Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 06/16] x86/boot/reloc: Rename some variables and rearrange code a bit Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 07/16] x86/boot: create *.lnk files with linker script Daniel Kiper
2016-04-15 14:04 ` Konrad Rzeszutek Wilk
2016-05-24 9:05 ` Jan Beulich
2016-05-24 12:28 ` Daniel Kiper
2016-05-24 12:52 ` Jan Beulich
2016-06-17 9:06 ` Daniel Kiper
2016-06-17 10:04 ` Jan Beulich
2016-06-17 10:34 ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 08/16] x86: add multiboot2 protocol support Daniel Kiper
2016-05-24 15:46 ` Jan Beulich
2016-05-25 16:34 ` Daniel Kiper
2016-05-26 10:28 ` Andrew Cooper
2016-05-27 8:08 ` Jan Beulich
2016-05-27 8:13 ` Andrew Cooper
2016-05-27 8:24 ` Jan Beulich
2016-05-27 8:11 ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 09/16] efi: explicitly define efi struct in xen/arch/x86/efi/stub.c Daniel Kiper
2016-05-25 7:03 ` Jan Beulich
2016-05-25 16:45 ` Daniel Kiper
2016-05-27 8:16 ` Jan Beulich
2016-06-01 15:07 ` Daniel Kiper
2016-07-05 18:33 ` Daniel Kiper
2016-07-06 6:55 ` Jan Beulich
2016-07-06 10:27 ` Daniel Kiper
2016-07-06 12:00 ` Jan Beulich
2016-07-06 12:55 ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 10/16] efi: create efi_enabled() Daniel Kiper
2016-05-25 7:20 ` Jan Beulich
2016-05-25 17:15 ` Daniel Kiper
2016-05-26 10:31 ` Andrew Cooper
2016-05-27 8:22 ` Jan Beulich
2016-06-01 15:23 ` Daniel Kiper
2016-06-01 15:41 ` Jan Beulich
2016-06-01 19:28 ` Daniel Kiper
2016-06-02 8:06 ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 11/16] efi: build xen.gz with EFI code Daniel Kiper
2016-05-25 7:53 ` Jan Beulich
2016-05-25 19:07 ` Daniel Kiper
2016-05-27 8:31 ` Jan Beulich
2016-06-01 15:48 ` Daniel Kiper
2016-06-01 15:58 ` Jan Beulich
2016-06-01 19:39 ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 12/16 - RFC] x86/efi: create new early memory allocator Daniel Kiper
2016-05-25 8:39 ` Jan Beulich
2016-05-25 19:48 ` Daniel Kiper
2016-05-27 8:37 ` Jan Beulich
2016-06-01 15:58 ` Daniel Kiper
2016-06-01 16:02 ` Jan Beulich
2016-06-01 19:53 ` Daniel Kiper
2016-06-02 8:11 ` Jan Beulich
2016-06-02 10:43 ` Daniel Kiper
2016-06-02 11:10 ` Jan Beulich
2016-06-01 16:01 ` Daniel Kiper
2016-07-05 18:26 ` Daniel Kiper
2016-07-06 7:22 ` Jan Beulich
2016-07-06 11:15 ` Daniel Kiper
2016-04-15 12:33 ` [PATCH v3 13/16 - RFC] x86: add multiboot2 protocol support for EFI platforms Daniel Kiper
2016-05-25 9:32 ` Jan Beulich
2016-05-25 10:29 ` Jan Beulich
2016-05-25 21:02 ` Daniel Kiper
2016-05-27 9:02 ` Jan Beulich
2016-06-01 19:03 ` Daniel Kiper
2016-06-02 8:34 ` Jan Beulich
2016-06-02 16:12 ` Daniel Kiper
2016-06-03 9:26 ` Jan Beulich
2016-06-03 17:06 ` Konrad Rzeszutek Wilk
2016-04-15 12:33 ` [PATCH v3 14/16] x86/boot: implement early command line parser in C Daniel Kiper
2016-05-25 10:33 ` Jan Beulich
2016-05-25 21:36 ` Daniel Kiper
2016-05-27 9:33 ` Jan Beulich
2016-06-02 8:15 ` Daniel Kiper
2016-06-02 8:39 ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 15/16 - RFC] x86: make Xen early boot code relocatable Daniel Kiper
2016-05-25 10:48 ` Jan Beulich
2016-04-15 12:33 ` [PATCH v3 16/16] x86: add multiboot2 protocol support for relocatable images Daniel Kiper
2016-05-25 11:03 ` Jan Beulich
2016-06-01 13:35 ` Daniel Kiper
2016-06-01 14:44 ` Jan Beulich
2016-06-01 19:16 ` Daniel Kiper
2016-06-02 8:41 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5763DF6502000078000F600B@prv-mh.provo.novell.com \
--to=jbeulich@suse.com \
--cc=JGross@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=cardoe@cardoe.com \
--cc=daniel.kiper@oracle.com \
--cc=david.vrabel@citrix.com \
--cc=fu.wei@linaro.org \
--cc=gang.wei@intel.com \
--cc=ning.sun@intel.com \
--cc=pgnet.dev@gmail.com \
--cc=qiaowei.ren@intel.com \
--cc=richard.l.maliszewski@intel.com \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).