Advanced
Mailing List Archive

Mailing List Archive

  1. Home >
  2. Xen>
  3. Devel
[PATCH v2 06/25] libelf-loader: introduce elf_load_image
stefano.stabellini at eu
Dec 9, 2011, 5:13 AM
Post #1 of 9 (173 views)
Permalink
From: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

Implement a new function, called elf_load_image, to perform the actually
copy of the elf image and clearing the padding.
The function is implemented as memcpy and memset when the library is
built as part of the tools, but it is implemented as copy_to_user and
clear_user when built as part of Xen, so that it can be safely called
with an HVM style dom0.


Changes in v2:

- remove CONFIG_KERNEL_NO_RELOC.


Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>
---
xen/common/libelf/libelf-loader.c | 17 +++++++++++++++--
1 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/xen/common/libelf/libelf-loader.c b/xen/common/libelf/libelf-loader.c
index 1ccf7d3..eec7e7b 100644
--- a/xen/common/libelf/libelf-loader.c
+++ b/xen/common/libelf/libelf-loader.c
@@ -107,11 +107,25 @@ void elf_set_log(struct elf_binary *elf, elf_log_callback *log_callback,
elf->log_caller_data = log_caller_data;
elf->verbose = verbose;
}
+
+static void elf_load_image(void *dst, const void *src, uint64_t filesz, uint64_t memsz)
+{
+ memcpy(dst, src, filesz);
+ memset(dst + filesz, 0, memsz - filesz);
+}
#else
+#include <asm/guest_access.h>
+
void elf_set_verbose(struct elf_binary *elf)
{
elf->verbose = 1;
}
+
+static void elf_load_image(void *dst, const void *src, uint64_t filesz, uint64_t memsz)
+{
+ copy_to_user(dst, src, filesz);
+ clear_user(dst + filesz, memsz - filesz);
+}
#endif

/* Calculate the required additional kernel space for the elf image */
@@ -256,8 +270,7 @@ void elf_load_binary(struct elf_binary *elf)
dest = elf_get_ptr(elf, paddr);
elf_msg(elf, "%s: phdr %" PRIu64 " at 0x%p -> 0x%p\n",
__func__, i, dest, dest + filesz);
- memcpy(dest, elf->image + offset, filesz);
- memset(dest + filesz, 0, memsz - filesz);
+ elf_load_image(dest, elf->image + offset, filesz, memsz);
}

elf_load_bsdsyms(elf);
--
1.7.2.5


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
JBeulich at suse
Dec 9, 2011, 5:33 AM
Post #2 of 9 (166 views)
Permalink
>>> On 09.12.11 at 14:13, <stefano.stabellini@eu.citrix.com> wrote:
> Implement a new function, called elf_load_image, to perform the actually
> copy of the elf image and clearing the padding.
> The function is implemented as memcpy and memset when the library is
> built as part of the tools, but it is implemented as copy_to_user and
> clear_user when built as part of Xen, so that it can be safely called
> with an HVM style dom0.

I meant to ask this on the first round already, but apparently forgot:
What is it that prevents memcpy()/memset() from being used for a
HVM style Dom0? {clear,copy_to}_user() still expect the guest memory
to be visible in the hypervisor's virtual address space - how could a
fault happen here? And if you have to take precautions for a fault,
shouldn't the calling code check the respective return values?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
Ian.Campbell at citrix
Dec 9, 2011, 5:40 AM
Post #3 of 9 (168 views)
Permalink
On Fri, 2011-12-09 at 13:33 +0000, Jan Beulich wrote:
> >>> On 09.12.11 at 14:13, <stefano.stabellini@eu.citrix.com> wrote:
> > Implement a new function, called elf_load_image, to perform the actually
> > copy of the elf image and clearing the padding.
> > The function is implemented as memcpy and memset when the library is
> > built as part of the tools, but it is implemented as copy_to_user and
> > clear_user when built as part of Xen, so that it can be safely called
> > with an HVM style dom0.
>
> I meant to ask this on the first round already, but apparently forgot:
> What is it that prevents memcpy()/memset() from being used for a
> HVM style Dom0? {clear,copy_to}_user() still expect the guest memory
> to be visible in the hypervisor's virtual address space - how could a
> fault happen here? And if you have to take precautions for a fault,
> shouldn't the calling code check the respective return values?

HVM guest memory is not (necessarily) mapped in the hypervisor page
tables, it needs to be mapped on demand. Also the source/target (delete
as appropriate) is a guest virtual address so even if the RAM happened
to be mapped it would (likely) not be in the same place so at a minimum
we need to translate things.

This is what copy_{to,from}_user does for an HVM guest even on X86,
where copy_to_user becomes copy_to_user_hvm as appropriate.

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
JBeulich at suse
Dec 9, 2011, 7:12 AM
Post #4 of 9 (168 views)
Permalink
>>> On 09.12.11 at 14:40, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2011-12-09 at 13:33 +0000, Jan Beulich wrote:
>> >>> On 09.12.11 at 14:13, <stefano.stabellini@eu.citrix.com> wrote:
>> > Implement a new function, called elf_load_image, to perform the actually
>> > copy of the elf image and clearing the padding.
>> > The function is implemented as memcpy and memset when the library is
>> > built as part of the tools, but it is implemented as copy_to_user and
>> > clear_user when built as part of Xen, so that it can be safely called
>> > with an HVM style dom0.
>>
>> I meant to ask this on the first round already, but apparently forgot:
>> What is it that prevents memcpy()/memset() from being used for a
>> HVM style Dom0? {clear,copy_to}_user() still expect the guest memory
>> to be visible in the hypervisor's virtual address space - how could a
>> fault happen here? And if you have to take precautions for a fault,
>> shouldn't the calling code check the respective return values?
>
> HVM guest memory is not (necessarily) mapped in the hypervisor page
> tables, it needs to be mapped on demand. Also the source/target (delete
> as appropriate) is a guest virtual address so even if the RAM happened
> to be mapped it would (likely) not be in the same place so at a minimum
> we need to translate things.
>
> This is what copy_{to,from}_user does for an HVM guest even on X86,
> where copy_to_user becomes copy_to_user_hvm as appropriate.

But that's not true - the distinction of hvm vs pv is at the
copy_to_guest() layer (raw_copy_to_guest() in the x86 case). So
maybe the patch meant to use those interfaces (and then we'd need
a clear_guest() too, which should also have been obvious by the fact
that the prior patch only introduced clear_user(), but no hvm variant
of it)?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
Ian.Campbell at citrix
Dec 9, 2011, 7:23 AM
Post #5 of 9 (167 views)
Permalink
On Fri, 2011-12-09 at 15:12 +0000, Jan Beulich wrote:
> >>> On 09.12.11 at 14:40, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2011-12-09 at 13:33 +0000, Jan Beulich wrote:
> >> >>> On 09.12.11 at 14:13, <stefano.stabellini@eu.citrix.com> wrote:
> >> > Implement a new function, called elf_load_image, to perform the actually
> >> > copy of the elf image and clearing the padding.
> >> > The function is implemented as memcpy and memset when the library is
> >> > built as part of the tools, but it is implemented as copy_to_user and
> >> > clear_user when built as part of Xen, so that it can be safely called
> >> > with an HVM style dom0.
> >>
> >> I meant to ask this on the first round already, but apparently forgot:
> >> What is it that prevents memcpy()/memset() from being used for a
> >> HVM style Dom0? {clear,copy_to}_user() still expect the guest memory
> >> to be visible in the hypervisor's virtual address space - how could a
> >> fault happen here? And if you have to take precautions for a fault,
> >> shouldn't the calling code check the respective return values?
> >
> > HVM guest memory is not (necessarily) mapped in the hypervisor page
> > tables, it needs to be mapped on demand. Also the source/target (delete
> > as appropriate) is a guest virtual address so even if the RAM happened
> > to be mapped it would (likely) not be in the same place so at a minimum
> > we need to translate things.
> >
> > This is what copy_{to,from}_user does for an HVM guest even on X86,
> > where copy_to_user becomes copy_to_user_hvm as appropriate.
>
> But that's not true - the distinction of hvm vs pv is at the
> copy_to_guest() layer (raw_copy_to_guest() in the x86 case). So
> maybe the patch meant to use those interfaces (and then we'd need
> a clear_guest() too, which should also have been obvious by the fact
> that the prior patch only introduced clear_user(), but no hvm variant
> of it)?

This code is also compiled in userspace which doesn't have copy_to_user
and in that case we need to use memcpy.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
JBeulich at suse
Dec 9, 2011, 7:34 AM
Post #6 of 9 (168 views)
Permalink
>>> On 09.12.11 at 16:23, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> On Fri, 2011-12-09 at 15:12 +0000, Jan Beulich wrote:
>> >>> On 09.12.11 at 14:40, Ian Campbell <Ian.Campbell@citrix.com> wrote:
>> > On Fri, 2011-12-09 at 13:33 +0000, Jan Beulich wrote:
>> >> >>> On 09.12.11 at 14:13, <stefano.stabellini@eu.citrix.com> wrote:
>> >> > Implement a new function, called elf_load_image, to perform the actually
>> >> > copy of the elf image and clearing the padding.
>> >> > The function is implemented as memcpy and memset when the library is
>> >> > built as part of the tools, but it is implemented as copy_to_user and
>> >> > clear_user when built as part of Xen, so that it can be safely called
>> >> > with an HVM style dom0.
>> >>
>> >> I meant to ask this on the first round already, but apparently forgot:
>> >> What is it that prevents memcpy()/memset() from being used for a
>> >> HVM style Dom0? {clear,copy_to}_user() still expect the guest memory
>> >> to be visible in the hypervisor's virtual address space - how could a
>> >> fault happen here? And if you have to take precautions for a fault,
>> >> shouldn't the calling code check the respective return values?
>> >
>> > HVM guest memory is not (necessarily) mapped in the hypervisor page
>> > tables, it needs to be mapped on demand. Also the source/target (delete
>> > as appropriate) is a guest virtual address so even if the RAM happened
>> > to be mapped it would (likely) not be in the same place so at a minimum
>> > we need to translate things.
>> >
>> > This is what copy_{to,from}_user does for an HVM guest even on X86,
>> > where copy_to_user becomes copy_to_user_hvm as appropriate.
>>
>> But that's not true - the distinction of hvm vs pv is at the
>> copy_to_guest() layer (raw_copy_to_guest() in the x86 case). So
>> maybe the patch meant to use those interfaces (and then we'd need
>> a clear_guest() too, which should also have been obvious by the fact
>> that the prior patch only introduced clear_user(), but no hvm variant
>> of it)?
>
> This code is also compiled in userspace which doesn't have copy_to_user
> and in that case we need to use memcpy.

Oh, the need for a distinct case for the user space version of it I
understand (assuming memcpy()/memset() indeed can't be used in the
hypervisor). What I was trying to tell you is that the ..._user()
interfaces don't have the property you're apparently thinking they
have, but instead you'd have to use the ..._guest() ones (I didn't look
at the ARM bits, but if there things are done differently that's likely a
mistake).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
Ian.Campbell at citrix
Dec 9, 2011, 7:41 AM
Post #7 of 9 (162 views)
Permalink
On Fri, 2011-12-09 at 15:34 +0000, Jan Beulich wrote:
> >>> On 09.12.11 at 16:23, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Fri, 2011-12-09 at 15:12 +0000, Jan Beulich wrote:
> >> >>> On 09.12.11 at 14:40, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> >> > On Fri, 2011-12-09 at 13:33 +0000, Jan Beulich wrote:
> >> >> >>> On 09.12.11 at 14:13, <stefano.stabellini@eu.citrix.com> wrote:
> >> >> > Implement a new function, called elf_load_image, to perform the actually
> >> >> > copy of the elf image and clearing the padding.
> >> >> > The function is implemented as memcpy and memset when the library is
> >> >> > built as part of the tools, but it is implemented as copy_to_user and
> >> >> > clear_user when built as part of Xen, so that it can be safely called
> >> >> > with an HVM style dom0.
> >> >>
> >> >> I meant to ask this on the first round already, but apparently forgot:
> >> >> What is it that prevents memcpy()/memset() from being used for a
> >> >> HVM style Dom0? {clear,copy_to}_user() still expect the guest memory
> >> >> to be visible in the hypervisor's virtual address space - how could a
> >> >> fault happen here? And if you have to take precautions for a fault,
> >> >> shouldn't the calling code check the respective return values?
> >> >
> >> > HVM guest memory is not (necessarily) mapped in the hypervisor page
> >> > tables, it needs to be mapped on demand. Also the source/target (delete
> >> > as appropriate) is a guest virtual address so even if the RAM happened
> >> > to be mapped it would (likely) not be in the same place so at a minimum
> >> > we need to translate things.
> >> >
> >> > This is what copy_{to,from}_user does for an HVM guest even on X86,
> >> > where copy_to_user becomes copy_to_user_hvm as appropriate.
> >>
> >> But that's not true - the distinction of hvm vs pv is at the
> >> copy_to_guest() layer (raw_copy_to_guest() in the x86 case). So
> >> maybe the patch meant to use those interfaces (and then we'd need
> >> a clear_guest() too, which should also have been obvious by the fact
> >> that the prior patch only introduced clear_user(), but no hvm variant
> >> of it)?
> >
> > This code is also compiled in userspace which doesn't have copy_to_user
> > and in that case we need to use memcpy.
>
> Oh, the need for a distinct case for the user space version of it I
> understand (assuming memcpy()/memset() indeed can't be used in the
> hypervisor). What I was trying to tell you is that the ..._user()
> interfaces don't have the property you're apparently thinking they
> have, but instead you'd have to use the ..._guest() ones

Oh, I see, yes you are right.

> (I didn't look
> at the ARM bits, but if there things are done differently that's likely a
> mistake).

This ARM port has no PV guests so it has no "user" in the sense that
x86's copy_to_user means, so copy_to_user and copy_to_guest are both the
same and do things the HVM way.

Maybe the right thing to do is to not supply copy_to_user on ARM at all.
I had feared this would be a big bit of string to pull on but it seems
that copy_to_user isn't actually widely used in common code:
$ rgrep --binary-files=without-match copy_to_user xen | grep -vE x86\|ia64\|asm
xen/common/gdbstub.c: r = gdb_arch_copy_to_user((void*)addr, (void*)&val, 1);
xen/include/linux/gdbstub.h:unsigned int gdb_arch_copy_to_user(
xen/include/xen/gdbstub.h:unsigned int gdb_arch_copy_to_user(

Stefano, what do you think?

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
keir at xen
Dec 9, 2011, 7:46 AM
Post #8 of 9 (164 views)
Permalink
On 09/12/2011 15:41, "Ian Campbell" <Ian.Campbell@citrix.com> wrote:

>> (I didn't look
>> at the ARM bits, but if there things are done differently that's likely a
>> mistake).
>
> This ARM port has no PV guests so it has no "user" in the sense that
> x86's copy_to_user means, so copy_to_user and copy_to_guest are both the
> same and do things the HVM way.
>
> Maybe the right thing to do is to not supply copy_to_user on ARM at all.
> I had feared this would be a big bit of string to pull on but it seems
> that copy_to_user isn't actually widely used in common code:
> $ rgrep --binary-files=without-match copy_to_user xen | grep -vE
> x86\|ia64\|asm
> xen/common/gdbstub.c: r = gdb_arch_copy_to_user((void*)addr,
> (void*)&val, 1);
> xen/include/linux/gdbstub.h:unsigned int gdb_arch_copy_to_user(
> xen/include/xen/gdbstub.h:unsigned int gdb_arch_copy_to_user(

copy_to/from_user isn't used by and doesn't really have any meaning to
common code. You can leave them out.

-- Keir

> Stefano, what do you think?



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Re: [PATCH v2 06/25] libelf-loader: introduce elf_load_image [ In reply to ]
stefano.stabellini at eu
Dec 13, 2011, 7:57 AM
Post #9 of 9 (164 views)
Permalink
On Fri, 9 Dec 2011, Ian Campbell wrote:
> This ARM port has no PV guests so it has no "user" in the sense that
> x86's copy_to_user means, so copy_to_user and copy_to_guest are both the
> same and do things the HVM way.
>
> Maybe the right thing to do is to not supply copy_to_user on ARM at all.
> I had feared this would be a big bit of string to pull on but it seems
> that copy_to_user isn't actually widely used in common code:
> $ rgrep --binary-files=without-match copy_to_user xen | grep -vE x86\|ia64\|asm
> xen/common/gdbstub.c: r = gdb_arch_copy_to_user((void*)addr, (void*)&val, 1);
> xen/include/linux/gdbstub.h:unsigned int gdb_arch_copy_to_user(
> xen/include/xen/gdbstub.h:unsigned int gdb_arch_copy_to_user(

We are not compiling gdbstub at the moment.

> Stefano, what do you think?

I think we should only provide the *_guest functions in our port.

The only downside is that the clear_user patch will grow also to
implement clear_guest for ia64 and x86 as well. Nothing difficult, but
more code.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

©2024 GT.net. A Gossamer Threads company.
5th Floor, 455 Granville St., Vancouver, BC V6C 1T1, Canada | Legal