* [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
@ 2012-09-05 9:05 Henning Schild
2012-09-05 9:36 ` Dunrong Huang
2012-09-05 10:53 ` Kevin Wolf
0 siblings, 2 replies; 6+ messages in thread
From: Henning Schild @ 2012-09-05 9:05 UTC (permalink / raw)
To: qemu-devel
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 .
---
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,
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;
+
snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
1.7.8.6
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
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
1 sibling, 0 replies; 6+ messages in thread
From: Dunrong Huang @ 2012-09-05 9:36 UTC (permalink / raw)
To: Henning Schild; +Cc: qemu-devel
2012/9/5 Henning Schild <henning@hennsch.de>:
> 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 .
>
> ---
> 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,
> 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)
According to the code style, braces {} are needed.
> + *pentry = ehdr.e_entry - ph->p_vaddr + ph->p_paddr;
> +
> snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
> rom_add_blob_fixed(label, data, mem_size, addr);
>
> --
> 1.7.8.6
>
>
--
Best Regards,
Dunrong Huang
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
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
2012-09-05 12:56 ` Henning Schild
1 sibling, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2012-09-05 10:53 UTC (permalink / raw)
To: Henning Schild; +Cc: qemu-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
2012-09-05 10:53 ` Kevin Wolf
@ 2012-09-05 12:56 ` Henning Schild
2012-09-05 13:11 ` Kevin Wolf
0 siblings, 1 reply; 6+ messages in thread
From: Henning Schild @ 2012-09-05 12:56 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel
[-- Attachment #1: Type: text/plain, Size: 3167 bytes --]
Find a hopefully proper patch attached. Take it or leave it.
Signed-off-by: Henning Schild <henning@hennsch.de>
---
hw/elf_ops.h | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..731a983 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name,
int fd,
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;
+ }
+
snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
1.7.8.6
Am Mittwoch, den 05.09.2012, 12:53 +0200 schrieb Kevin Wolf
<kwolf@redhat.com>:
> 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
[-- Attachment #2: 0001-fix-entry-pointer-for-ELF-kernels-loaded-with-kernel.patch --]
[-- Type: text/x-c, Size: 1279 bytes --]
From 0d3c07a78afa9ff77ca8f4ef90bd76b9fd7d948f Mon Sep 17 00:00:00 2001
From: Henning Schild <henning@hennsch.de>
Date: Wed, 5 Sep 2012 14:46:38 +0200
Subject: [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
Signed-off-by: Henning Schild <henning@hennsch.de>
---
hw/elf_ops.h | 11 +++++++++++
1 files changed, 11 insertions(+), 0 deletions(-)
diff --git a/hw/elf_ops.h b/hw/elf_ops.h
index fa65ce2..731a983 100644
--- a/hw/elf_ops.h
+++ b/hw/elf_ops.h
@@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name, int fd,
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;
+ }
+
snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
rom_add_blob_fixed(label, data, mem_size, addr);
--
1.7.8.6
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
2012-09-05 12:56 ` Henning Schild
@ 2012-09-05 13:11 ` Kevin Wolf
2012-09-07 14:10 ` Aurelien Jarno
0 siblings, 1 reply; 6+ messages in thread
From: Kevin Wolf @ 2012-09-05 13:11 UTC (permalink / raw)
To: Henning Schild; +Cc: qemu-devel, Aurelien Jarno
Am 05.09.2012 14:56, schrieb Henning Schild:
> Find a hopefully proper patch attached. Take it or leave it.
>
> Signed-off-by: Henning Schild <henning@hennsch.de>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Aurelien, I think in the past you committed some changes in this area.
Does this look good to you and can you get it committed?
Kevin
> ---
> hw/elf_ops.h | 11 +++++++++++
> 1 files changed, 11 insertions(+), 0 deletions(-)
>
> diff --git a/hw/elf_ops.h b/hw/elf_ops.h
> index fa65ce2..731a983 100644
> --- a/hw/elf_ops.h
> +++ b/hw/elf_ops.h
> @@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name,
> int fd,
> 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;
> + }
> +
> snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
> rom_add_blob_fixed(label, data, mem_size, addr);
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH] fix entry pointer for ELF kernels loaded with -kernel option
2012-09-05 13:11 ` Kevin Wolf
@ 2012-09-07 14:10 ` Aurelien Jarno
0 siblings, 0 replies; 6+ messages in thread
From: Aurelien Jarno @ 2012-09-07 14:10 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-devel, Henning Schild
On Wed, Sep 05, 2012 at 03:11:13PM +0200, Kevin Wolf wrote:
> Am 05.09.2012 14:56, schrieb Henning Schild:
> > Find a hopefully proper patch attached. Take it or leave it.
> >
> > Signed-off-by: Henning Schild <henning@hennsch.de>
>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>
> Aurelien, I think in the past you committed some changes in this area.
> Does this look good to you and can you get it committed?
>
Thanks, committed.
> > ---
> > hw/elf_ops.h | 11 +++++++++++
> > 1 files changed, 11 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/elf_ops.h b/hw/elf_ops.h
> > index fa65ce2..731a983 100644
> > --- a/hw/elf_ops.h
> > +++ b/hw/elf_ops.h
> > @@ -269,6 +269,17 @@ static int glue(load_elf, SZ)(const char *name,
> > int fd,
> > 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;
> > + }
> > +
> > snprintf(label, sizeof(label), "phdr #%d: %s", i, name);
> > rom_add_blob_fixed(label, data, mem_size, addr);
> >
> >
>
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurelien@aurel32.net http://www.aurel32.net
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-09-07 14:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2012-09-05 12:56 ` Henning Schild
2012-09-05 13:11 ` Kevin Wolf
2012-09-07 14:10 ` Aurelien Jarno
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).