* [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).