qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Henning Schild <henning@hennsch.de>
Cc: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
Date: Wed, 05 Sep 2012 12:53:02 +0200	[thread overview]
Message-ID: <50472F0E.1030803@redhat.com> (raw)
In-Reply-To: <354bfe08c6245723a07efecb8da1347f-EhVcX11BQgteRwEVBgsBVggwfgFLV15YQUBGAEFeXC9ZXF0WXV9xH1BLXzBeQkEEV1lcSVlW-webmailer2@server04.webmailer.hosteurope.de>

Am 05.09.2012 11:05, schrieb Henning Schild:
>  This patch fixes a bug in qemu which prevents Multiboot ELF kernels 
>  from being loaded with the -kernel option. Find a full description of 
>  the problem here https://bugs.launchpad.net/qemu/+bug/1044727 .

The logic looks good to me, but there are a few points about the patch
itself (see http://wiki.qemu.org/Contribute/SubmitAPatch).

First thing is that the patch needs a proper Signed-off-by line. This is
absolutely crucial. The other points could be fixed manually by a
potential patient enough maintainer, but you are the only one who can
provide the Signed-off-by. Without it, the patch won't be applied.

>  ---
>   hw/elf_ops.h |   10 ++++++++++
>   1 files changed, 10 insertions(+), 0 deletions(-)
> 
>  diff --git a/hw/elf_ops.h b/hw/elf_ops.h
>  index fa65ce2..aeddd11 100644
>  --- a/hw/elf_ops.h
>  +++ b/hw/elf_ops.h
>  @@ -269,6 +269,16 @@ static int glue(load_elf, SZ)(const char *name, 
>  int fd,

The patch is corrupted by line wraps. Using git format-patch/send-email
avoids this kind of problems. Alternatively, attach the patch in
addition so that an uncorrupted version can be used for applying it.

>                   addr = ph->p_paddr;
>               }
>  
>  +            /* the entry pointer in the ELF header is a virtual
>  +             * address, if the text segments paddr and vaddr differ
>  +             * we need to adjust the entry */
>  +            if (pentry && !translate_fn &&
>  +                ph->p_vaddr != ph->p_paddr &&
>  +                ehdr.e_entry >= ph->p_vaddr &&
>  +                ehdr.e_entry < ph->p_vaddr + ph->p_filesz &&
>  +                ph->p_flags & PF_X)
>  +                    *pentry = ehdr.e_entry - ph->p_vaddr + 
>  ph->p_paddr;

The coding style problem that was already mentioned. qemu puts braces
even for single statements.

Kevin

  parent reply	other threads:[~2012-09-05 10:53 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-05  9:05 [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option Henning Schild
2012-09-05  9:36 ` Dunrong Huang
2012-09-05 10:53 ` Kevin Wolf [this message]
2012-09-05 12:56   ` Henning Schild
2012-09-05 13:11     ` Kevin Wolf
2012-09-07 14:10       ` Aurelien Jarno

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=50472F0E.1030803@redhat.com \
    --to=kwolf@redhat.com \
    --cc=henning@hennsch.de \
    --cc=qemu-devel@nongnu.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).