Re: [Xen-devel] [PATCH v2 06/25] libelf-loader: introduce elf_load_image

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2011-12-09 at 15:34 +0000, Jan Beulich wrote:
> >>> On 09.12.11 at 16:23, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > On Fri, 2011-12-09 at 15:12 +0000, Jan Beulich wrote:
> >> >>> On 09.12.11 at 14:40, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> >> > On Fri, 2011-12-09 at 13:33 +0000, Jan Beulich wrote:
> >> >> >>> On 09.12.11 at 14:13, <stefano.stabellini@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel