| Message ID | 1452688338-70075-2-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 13.01.16 at 13:32, <roger.pau@citrix.com> wrote: Re the title: I think the fix here really is to the checking against the right value, not the initialization? > tools/libxc/xc_dom_elfloader.c | 2 +- > xen/common/libelf/libelf-dominfo.c | 1 + > xen/include/xen/libelf.h | 1 + > 3 files changed, 3 insertions(+), 1 deletion(-) With this diffstat, any reason only tools maintainers got Cc-ed? Contents: Acked-by: Jan Beulich <jbeulich@suse.com> > --- a/tools/libxc/xc_dom_elfloader.c > +++ b/tools/libxc/xc_dom_elfloader.c > @@ -57,7 +57,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom, > uint64_t machine = elf_uval(elf, elf->ehdr, e_machine); > > if ( dom->container_type == XC_DOM_HVM_CONTAINER && > - dom->parms.phys_entry != UNSET_ADDR ) > + dom->parms.phys_entry != UNSET_ADDR32 ) > return "hvm-3.0-x86_32"; > > switch ( machine ) > diff --git a/xen/common/libelf/libelf-dominfo.c > b/xen/common/libelf/libelf-dominfo.c > index 02d6cfb..ec69449 100644 > --- a/xen/common/libelf/libelf-dominfo.c > +++ b/xen/common/libelf/libelf-dominfo.c > @@ -503,6 +503,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, > parms->virt_hv_start_low = UNSET_ADDR; > parms->p2m_base = UNSET_ADDR; > parms->elf_paddr_offset = UNSET_ADDR; > + parms->phys_entry = UNSET_ADDR32; > > /* Find and parse elf notes. */ > count = elf_phdr_count(elf); > diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h > index 6da4cc0..95b5370 100644 > --- a/xen/include/xen/libelf.h > +++ b/xen/include/xen/libelf.h > @@ -386,6 +386,7 @@ elf_errorstatus elf_reloc(struct elf_binary *elf); > /* xc_libelf_dominfo.c > */ > > #define UNSET_ADDR ((uint64_t)-1) > +#define UNSET_ADDR32 ((uint32_t)-1) > > enum xen_elfnote_type { > XEN_ENT_NONE = 0, > -- > 1.9.5 (Apple Git-50.3) > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
El 13/01/16 a les 13.46, Jan Beulich ha escrit: >>>> On 13.01.16 at 13:32, <roger.pau@citrix.com> wrote: > > Re the title: I think the fix here really is to the checking against the > right value, not the initialization? Yes, the initialization doesn't matter much, the proper fix is the checking. I would change it to: xen/elfnotes: check phys_entry against UNSET_ADDR32 Which I think can be done by the committer (or I can resend if needed). >> tools/libxc/xc_dom_elfloader.c | 2 +- >> xen/common/libelf/libelf-dominfo.c | 1 + >> xen/include/xen/libelf.h | 1 + >> 3 files changed, 3 insertions(+), 1 deletion(-) > > With this diffstat, any reason only tools maintainers got Cc-ed? Hm, weird, that's what scripts/get_maintainer.pl gave me as output. > Contents: Acked-by: Jan Beulich <jbeulich@suse.com> > >> --- a/tools/libxc/xc_dom_elfloader.c >> +++ b/tools/libxc/xc_dom_elfloader.c >> @@ -57,7 +57,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom, >> uint64_t machine = elf_uval(elf, elf->ehdr, e_machine); >> >> if ( dom->container_type == XC_DOM_HVM_CONTAINER && >> - dom->parms.phys_entry != UNSET_ADDR ) >> + dom->parms.phys_entry != UNSET_ADDR32 ) >> return "hvm-3.0-x86_32"; >> >> switch ( machine ) >> diff --git a/xen/common/libelf/libelf-dominfo.c >> b/xen/common/libelf/libelf-dominfo.c >> index 02d6cfb..ec69449 100644 >> --- a/xen/common/libelf/libelf-dominfo.c >> +++ b/xen/common/libelf/libelf-dominfo.c >> @@ -503,6 +503,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, >> parms->virt_hv_start_low = UNSET_ADDR; >> parms->p2m_base = UNSET_ADDR; >> parms->elf_paddr_offset = UNSET_ADDR; >> + parms->phys_entry = UNSET_ADDR32; >> >> /* Find and parse elf notes. */ >> count = elf_phdr_count(elf); >> diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h >> index 6da4cc0..95b5370 100644 >> --- a/xen/include/xen/libelf.h >> +++ b/xen/include/xen/libelf.h >> @@ -386,6 +386,7 @@ elf_errorstatus elf_reloc(struct elf_binary *elf); >> /* xc_libelf_dominfo.c >> */ >> >> #define UNSET_ADDR ((uint64_t)-1) >> +#define UNSET_ADDR32 ((uint32_t)-1) >> >> enum xen_elfnote_type { >> XEN_ENT_NONE = 0, >> -- >> 1.9.5 (Apple Git-50.3) >> >> >> _______________________________________________ >> Xen-devel mailing list >> Xen-devel@lists.xen.org >> http://lists.xen.org/xen-devel > > > >
>>> On 13.01.16 at 13:52, <roger.pau@citrix.com> wrote: > El 13/01/16 a les 13.46, Jan Beulich ha escrit: >>>>> On 13.01.16 at 13:32, <roger.pau@citrix.com> wrote: >> >> Re the title: I think the fix here really is to the checking against the >> right value, not the initialization? > > Yes, the initialization doesn't matter much, the proper fix is the > checking. I would change it to: > > xen/elfnotes: check phys_entry against UNSET_ADDR32 > > Which I think can be done by the committer (or I can resend if needed). Sure. >>> tools/libxc/xc_dom_elfloader.c | 2 +- >>> xen/common/libelf/libelf-dominfo.c | 1 + >>> xen/include/xen/libelf.h | 1 + >>> 3 files changed, 3 insertions(+), 1 deletion(-) >> >> With this diffstat, any reason only tools maintainers got Cc-ed? > > Hm, weird, that's what scripts/get_maintainer.pl gave me as output. Something must be broken there. Jan
On 13/01/16 13:02, Jan Beulich wrote: >>>> On 13.01.16 at 13:52, <roger.pau@citrix.com> wrote: >> El 13/01/16 a les 13.46, Jan Beulich ha escrit: >>>>>> On 13.01.16 at 13:32, <roger.pau@citrix.com> wrote: >>> Re the title: I think the fix here really is to the checking against the >>> right value, not the initialization? >> Yes, the initialization doesn't matter much, the proper fix is the >> checking. I would change it to: >> >> xen/elfnotes: check phys_entry against UNSET_ADDR32 >> >> Which I think can be done by the committer (or I can resend if needed). > Sure. > >>>> tools/libxc/xc_dom_elfloader.c | 2 +- >>>> xen/common/libelf/libelf-dominfo.c | 1 + >>>> xen/include/xen/libelf.h | 1 + >>>> 3 files changed, 3 insertions(+), 1 deletion(-) >>> With this diffstat, any reason only tools maintainers got Cc-ed? >> Hm, weird, that's what scripts/get_maintainer.pl gave me as output. > Something must be broken there. There is not an entry covering xen/common. ~Andrew
>>> On 13.01.16 at 14:05, <andrew.cooper3@citrix.com> wrote: > On 13/01/16 13:02, Jan Beulich wrote: >>>>> On 13.01.16 at 13:52, <roger.pau@citrix.com> wrote: >>> El 13/01/16 a les 13.46, Jan Beulich ha escrit: >>>>>>> On 13.01.16 at 13:32, <roger.pau@citrix.com> wrote: >>>>> tools/libxc/xc_dom_elfloader.c | 2 +- >>>>> xen/common/libelf/libelf-dominfo.c | 1 + >>>>> xen/include/xen/libelf.h | 1 + >>>>> 3 files changed, 3 insertions(+), 1 deletion(-) >>>> With this diffstat, any reason only tools maintainers got Cc-ed? >>> Hm, weird, that's what scripts/get_maintainer.pl gave me as output. >> Something must be broken there. > > There is not an entry covering xen/common. And hence for those file it should fall back to THE REST. Jan
On 13/01/16 13:08, Jan Beulich wrote: >>>> On 13.01.16 at 14:05, <andrew.cooper3@citrix.com> wrote: >> On 13/01/16 13:02, Jan Beulich wrote: >>>>>> On 13.01.16 at 13:52, <roger.pau@citrix.com> wrote: >>>> El 13/01/16 a les 13.46, Jan Beulich ha escrit: >>>>>>>> On 13.01.16 at 13:32, <roger.pau@citrix.com> wrote: >>>>>> tools/libxc/xc_dom_elfloader.c | 2 +- >>>>>> xen/common/libelf/libelf-dominfo.c | 1 + >>>>>> xen/include/xen/libelf.h | 1 + >>>>>> 3 files changed, 3 insertions(+), 1 deletion(-) >>>>> With this diffstat, any reason only tools maintainers got Cc-ed? >>>> Hm, weird, that's what scripts/get_maintainer.pl gave me as output. >>> Something must be broken there. >> There is not an entry covering xen/common. > And hence for those file it should fall back to THE REST. Per original review, THE REST is only invoked on a patch which doesn't match any paths. As one file matched toolstack, THE REST wasn't invoked. There really should be a xen/ entry. ~Andrew
diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c index 2ae575e..5039f3f 100644 --- a/tools/libxc/xc_dom_elfloader.c +++ b/tools/libxc/xc_dom_elfloader.c @@ -57,7 +57,7 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom, uint64_t machine = elf_uval(elf, elf->ehdr, e_machine); if ( dom->container_type == XC_DOM_HVM_CONTAINER && - dom->parms.phys_entry != UNSET_ADDR ) + dom->parms.phys_entry != UNSET_ADDR32 ) return "hvm-3.0-x86_32"; switch ( machine ) diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c index 02d6cfb..ec69449 100644 --- a/xen/common/libelf/libelf-dominfo.c +++ b/xen/common/libelf/libelf-dominfo.c @@ -503,6 +503,7 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, parms->virt_hv_start_low = UNSET_ADDR; parms->p2m_base = UNSET_ADDR; parms->elf_paddr_offset = UNSET_ADDR; + parms->phys_entry = UNSET_ADDR32; /* Find and parse elf notes. */ count = elf_phdr_count(elf); diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h index 6da4cc0..95b5370 100644 --- a/xen/include/xen/libelf.h +++ b/xen/include/xen/libelf.h @@ -386,6 +386,7 @@ elf_errorstatus elf_reloc(struct elf_binary *elf); /* xc_libelf_dominfo.c */ #define UNSET_ADDR ((uint64_t)-1) +#define UNSET_ADDR32 ((uint32_t)-1) enum xen_elfnote_type { XEN_ENT_NONE = 0,
And introduce UNSET_ADDR32. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> --- tools/libxc/xc_dom_elfloader.c | 2 +- xen/common/libelf/libelf-dominfo.c | 1 + xen/include/xen/libelf.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-)