diff mbox

[v2,for-4.7,5/6] xen/xsplice: add ELFOSABI_FREEBSD as a supported OSABI for payloads

Message ID 1462272910-41200-6-git-send-email-roger.pau@citrix.com (mailing list archive)
State New, archived
Headers show

Commit Message

Roger Pau Monné May 3, 2016, 10:55 a.m. UTC
The calling convention used by the FreeBSD ELF OSABI is exactly the same as
the the one defined by System V, so payloads with a FreeBSD OSABI should be
accepted by the xsplice machinery.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 xen/common/xsplice_elf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Andrew Cooper May 3, 2016, 12:21 p.m. UTC | #1
On 03/05/16 11:55, Roger Pau Monne wrote:
> The calling convention used by the FreeBSD ELF OSABI is exactly the same as
> the the one defined by System V, so payloads with a FreeBSD OSABI should be
> accepted by the xsplice machinery.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>  xen/common/xsplice_elf.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
> index 1e1f167..918a1bf 100644
> --- a/xen/common/xsplice_elf.c
> +++ b/xen/common/xsplice_elf.c
> @@ -397,7 +397,8 @@ static int xsplice_header_check(const struct xsplice_elf *elf)
>      if ( hdr->e_version != EV_CURRENT ||
>           hdr->e_ident[EI_VERSION] != EV_CURRENT ||
>           hdr->e_ident[EI_ABIVERSION] != 0 ||
> -         hdr->e_ident[EI_OSABI] != ELFOSABI_NONE ||
> +         (hdr->e_ident[EI_OSABI] != ELFOSABI_NONE &&
> +         hdr->e_ident[EI_OSABI] != ELFOSABI_FREEBSD) ||

You should have an extra space of indentation here.

Other than that, Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>           hdr->e_type != ET_REL ||
>           hdr->e_phnum != 0 )
>      {
Jan Beulich May 3, 2016, 2:17 p.m. UTC | #2
>>> On 03.05.16 at 12:55, <roger.pau@citrix.com> wrote:
> The calling convention used by the FreeBSD ELF OSABI is exactly the same as
> the the one defined by System V, so payloads with a FreeBSD OSABI should be
> accepted by the xsplice machinery.

Well, you realize that the ABI is more than just the calling convention?
I.e. your patch basically says ELFOSABI_NONE == ELFOSABI_FREEBSD,
in which case I wonder why the latter exists in the first place. Is there
a proper document somewhere describing everything the latter implies,
so that one can check whether for xSplice purposes such similar
treatment is indeed okay? Until then I'm afraid I'm opposed to this going
in.

Jan
Roger Pau Monné May 4, 2016, 9:48 a.m. UTC | #3
On Tue, May 03, 2016 at 08:17:15AM -0600, Jan Beulich wrote:
> >>> On 03.05.16 at 12:55, <roger.pau@citrix.com> wrote:
> > The calling convention used by the FreeBSD ELF OSABI is exactly the same as
> > the the one defined by System V, so payloads with a FreeBSD OSABI should be
> > accepted by the xsplice machinery.
> 
> Well, you realize that the ABI is more than just the calling convention?
> I.e. your patch basically says ELFOSABI_NONE == ELFOSABI_FREEBSD,
> in which case I wonder why the latter exists in the first place. Is there
> a proper document somewhere describing everything the latter implies,
> so that one can check whether for xSplice purposes such similar
> treatment is indeed okay? Until then I'm afraid I'm opposed to this going
> in.

The FreeBSD elf OSABI only has a meaning for userspace applications, it's 
used by FreeBSD in order to detect if an application is native or if it 
needs to be run in the linuxator (the Linux emulator, or any other emulator 
that is available and matches the ELF OSABI specified in the binary FWIW).

THe only difference from SYSV to FreeBSD OSABI is the sysentvec that's 
selected inside of the FreeBSD kernel (the ABI between the kernel and the 
user-space application), but of course this doesn't apply to kernel code, 
which is what Xen and the xsplice payloads are. Sadly this is not written 
anywhere.

Roger.
Jan Beulich May 4, 2016, 10:34 a.m. UTC | #4
>>> On 04.05.16 at 11:48, <roger.pau@citrix.com> wrote:
> On Tue, May 03, 2016 at 08:17:15AM -0600, Jan Beulich wrote:
>> >>> On 03.05.16 at 12:55, <roger.pau@citrix.com> wrote:
>> > The calling convention used by the FreeBSD ELF OSABI is exactly the same as
>> > the the one defined by System V, so payloads with a FreeBSD OSABI should be
>> > accepted by the xsplice machinery.
>> 
>> Well, you realize that the ABI is more than just the calling convention?
>> I.e. your patch basically says ELFOSABI_NONE == ELFOSABI_FREEBSD,
>> in which case I wonder why the latter exists in the first place. Is there
>> a proper document somewhere describing everything the latter implies,
>> so that one can check whether for xSplice purposes such similar
>> treatment is indeed okay? Until then I'm afraid I'm opposed to this going
>> in.
> 
> The FreeBSD elf OSABI only has a meaning for userspace applications, it's 
> used by FreeBSD in order to detect if an application is native or if it 
> needs to be run in the linuxator (the Linux emulator, or any other emulator 
> that is available and matches the ELF OSABI specified in the binary FWIW).
> 
> THe only difference from SYSV to FreeBSD OSABI is the sysentvec that's 
> selected inside of the FreeBSD kernel (the ABI between the kernel and the 
> user-space application), but of course this doesn't apply to kernel code, 
> which is what Xen and the xsplice payloads are. Sadly this is not written 
> anywhere.

Well, okay, in that case I agree the patch should be fine.

Jan
Roger Pau Monné May 5, 2016, 4:35 p.m. UTC | #5
On Wed, May 04, 2016 at 04:34:26AM -0600, Jan Beulich wrote:
> >>> On 04.05.16 at 11:48, <roger.pau@citrix.com> wrote:
> > On Tue, May 03, 2016 at 08:17:15AM -0600, Jan Beulich wrote:
> >> >>> On 03.05.16 at 12:55, <roger.pau@citrix.com> wrote:
> >> > The calling convention used by the FreeBSD ELF OSABI is exactly the same as
> >> > the the one defined by System V, so payloads with a FreeBSD OSABI should be
> >> > accepted by the xsplice machinery.
> >> 
> >> Well, you realize that the ABI is more than just the calling convention?
> >> I.e. your patch basically says ELFOSABI_NONE == ELFOSABI_FREEBSD,
> >> in which case I wonder why the latter exists in the first place. Is there
> >> a proper document somewhere describing everything the latter implies,
> >> so that one can check whether for xSplice purposes such similar
> >> treatment is indeed okay? Until then I'm afraid I'm opposed to this going
> >> in.
> > 
> > The FreeBSD elf OSABI only has a meaning for userspace applications, it's 
> > used by FreeBSD in order to detect if an application is native or if it 
> > needs to be run in the linuxator (the Linux emulator, or any other emulator 
> > that is available and matches the ELF OSABI specified in the binary FWIW).
> > 
> > THe only difference from SYSV to FreeBSD OSABI is the sysentvec that's 
> > selected inside of the FreeBSD kernel (the ABI between the kernel and the 
> > user-space application), but of course this doesn't apply to kernel code, 
> > which is what Xen and the xsplice payloads are. Sadly this is not written 
> > anywhere.
> 
> Well, okay, in that case I agree the patch should be fine.

Would you like me to resend this with a more expanded commit message, or are 
you going to squash my explanation in the commit message?

Thanks, Roger.
Jan Beulich May 6, 2016, 6:51 a.m. UTC | #6
>>> On 05.05.16 at 18:35, <roger.pau@citrix.com> wrote:
> On Wed, May 04, 2016 at 04:34:26AM -0600, Jan Beulich wrote:
>> >>> On 04.05.16 at 11:48, <roger.pau@citrix.com> wrote:
>> > On Tue, May 03, 2016 at 08:17:15AM -0600, Jan Beulich wrote:
>> >> >>> On 03.05.16 at 12:55, <roger.pau@citrix.com> wrote:
>> >> > The calling convention used by the FreeBSD ELF OSABI is exactly the same 
> as
>> >> > the the one defined by System V, so payloads with a FreeBSD OSABI should 
> be
>> >> > accepted by the xsplice machinery.
>> >> 
>> >> Well, you realize that the ABI is more than just the calling convention?
>> >> I.e. your patch basically says ELFOSABI_NONE == ELFOSABI_FREEBSD,
>> >> in which case I wonder why the latter exists in the first place. Is there
>> >> a proper document somewhere describing everything the latter implies,
>> >> so that one can check whether for xSplice purposes such similar
>> >> treatment is indeed okay? Until then I'm afraid I'm opposed to this going
>> >> in.
>> > 
>> > The FreeBSD elf OSABI only has a meaning for userspace applications, it's 
>> > used by FreeBSD in order to detect if an application is native or if it 
>> > needs to be run in the linuxator (the Linux emulator, or any other emulator 
> 
>> > that is available and matches the ELF OSABI specified in the binary FWIW).
>> > 
>> > THe only difference from SYSV to FreeBSD OSABI is the sysentvec that's 
>> > selected inside of the FreeBSD kernel (the ABI between the kernel and the 
>> > user-space application), but of course this doesn't apply to kernel code, 
>> > which is what Xen and the xsplice payloads are. Sadly this is not written 
>> > anywhere.
>> 
>> Well, okay, in that case I agree the patch should be fine.
> 
> Would you like me to resend this with a more expanded commit message, or are 
> you going to squash my explanation in the commit message?

I'd be fine folding it in, but I may not be the one ending up
committing it - it needs an xSplice maintainer's ack first, and
it may well be that Konrad then would apply it.

Jan
Ross Lagerwall May 6, 2016, 12:48 p.m. UTC | #7
On 05/03/2016 11:55 AM, Roger Pau Monne wrote:
> The calling convention used by the FreeBSD ELF OSABI is exactly the same as
> the the one defined by System V, so payloads with a FreeBSD OSABI should be
> accepted by the xsplice machinery.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>   xen/common/xsplice_elf.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
> index 1e1f167..918a1bf 100644
> --- a/xen/common/xsplice_elf.c
> +++ b/xen/common/xsplice_elf.c
> @@ -397,7 +397,8 @@ static int xsplice_header_check(const struct xsplice_elf *elf)
>       if ( hdr->e_version != EV_CURRENT ||
>            hdr->e_ident[EI_VERSION] != EV_CURRENT ||
>            hdr->e_ident[EI_ABIVERSION] != 0 ||
> -         hdr->e_ident[EI_OSABI] != ELFOSABI_NONE ||
> +         (hdr->e_ident[EI_OSABI] != ELFOSABI_NONE &&
> +         hdr->e_ident[EI_OSABI] != ELFOSABI_FREEBSD) ||
>            hdr->e_type != ET_REL ||
>            hdr->e_phnum != 0 )
>       {
>

Acked-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Konrad Rzeszutek Wilk May 6, 2016, 4:34 p.m. UTC | #8
On Fri, May 06, 2016 at 01:48:08PM +0100, Ross Lagerwall wrote:
> On 05/03/2016 11:55 AM, Roger Pau Monne wrote:
> >The calling convention used by the FreeBSD ELF OSABI is exactly the same as
> >the the one defined by System V, so payloads with a FreeBSD OSABI should be
> >accepted by the xsplice machinery.
> >
> >Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> >---
> >Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> >---
> >  xen/common/xsplice_elf.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> >diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
> >index 1e1f167..918a1bf 100644
> >--- a/xen/common/xsplice_elf.c
> >+++ b/xen/common/xsplice_elf.c
> >@@ -397,7 +397,8 @@ static int xsplice_header_check(const struct xsplice_elf *elf)
> >      if ( hdr->e_version != EV_CURRENT ||
> >           hdr->e_ident[EI_VERSION] != EV_CURRENT ||
> >           hdr->e_ident[EI_ABIVERSION] != 0 ||
> >-         hdr->e_ident[EI_OSABI] != ELFOSABI_NONE ||
> >+         (hdr->e_ident[EI_OSABI] != ELFOSABI_NONE &&
> >+         hdr->e_ident[EI_OSABI] != ELFOSABI_FREEBSD) ||
> >           hdr->e_type != ET_REL ||
> >           hdr->e_phnum != 0 )
> >      {
> >
> 
> Acked-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Applied with the change log change and an extra space.
diff mbox

Patch

diff --git a/xen/common/xsplice_elf.c b/xen/common/xsplice_elf.c
index 1e1f167..918a1bf 100644
--- a/xen/common/xsplice_elf.c
+++ b/xen/common/xsplice_elf.c
@@ -397,7 +397,8 @@  static int xsplice_header_check(const struct xsplice_elf *elf)
     if ( hdr->e_version != EV_CURRENT ||
          hdr->e_ident[EI_VERSION] != EV_CURRENT ||
          hdr->e_ident[EI_ABIVERSION] != 0 ||
-         hdr->e_ident[EI_OSABI] != ELFOSABI_NONE ||
+         (hdr->e_ident[EI_OSABI] != ELFOSABI_NONE &&
+         hdr->e_ident[EI_OSABI] != ELFOSABI_FREEBSD) ||
          hdr->e_type != ET_REL ||
          hdr->e_phnum != 0 )
     {