Re: [Xen-devel] [PATCH 2/3] xenoprof: Handle 32-bit guest stacks properly in a 64-bit hypervisor

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 24.01.12 at 20:27, Marcus Granado <marcus.granado@xxxxxxxxxx> wrote:
> I'm trying to understand the compat handle. It is not clear to me how to 
> map one from head (a 64-bit pointer), since COMPAT_HANDLE seems to store 

That cannot generally be done, as a 64-bit pointer can never be
represented as a compat handle.

Proper conversion has to start at where 'head' is first generated (i.e.
the line

    head = (struct frame_head *)regs->ebp;

in xenoprof_backtrace() (the more that here you really *want* to
drop the upper 32 bits in the compat case. Working with a union is
a possible approach, but it may also be acceptable to actually do
the truncation in dump_guest_backtrace(), properly explaining why
the dropping of the upper half is valid and intended there.

(I've already put fixing up of this already committed patch on my
todo list, so feel free to drop further attempts; once I'm done I'd
appreciate review/testing of the code though.)

Jan

> a 32-bit compat_ptr_t value in its structure. Ideally, what I would like 
> to do is
> 
> COMPAT_HANDLE(char) guest_head = map_guest_handle_to_compat_handle 
> (guest_handle_from_ptr(head, char));
> or
> COMPAT_HANDLE(char) guest_head = compat_handle_from_ptr(head, char));
> but I can't find any equivalent functions in any header.
> 
> The following line compiles,
> COMPAT_HANDLE(char) guest_head = { (full_ptr_t)head };
> but it looks like, in this case, the compat handle structure in compat.h 
> will truncate the most significant bits from the head pointer, so 
> compat_handle_okay(guest_head,...) and 
> __copy_from_compat(...,guest_head,...) below will be using a truncated 
> pointer:
> 
>       56 static struct frame_head *
>       57 dump_guest_backtrace(struct domain *d, struct vcpu *vcpu,
>       58                      struct frame_head * head, int mode)
>       59 {
>       60     struct frame_head bufhead[2];
>       61
>       62 #ifdef CONFIG_X86_64
>       63     if ( is_32bit_vcpu(vcpu) )
>       64     {
>       65         COMPAT_HANDLE(char) guest_head = { (full_ptr_t)head };
>       66         struct frame_head_32bit bufhead32[2];
>       67         /* Also check accessibility of one struct frame_head 
> beyond */
>       68         if (!compat_handle_okay(guest_head, sizeof(bufhead32)))
>       69             return 0;
>       70         if (__copy_from_compat((char *)bufhead32, guest_head,
>       71                                      sizeof(bufhead32)))
>       72             return 0;
>       73         bufhead[0].ebp=(struct frame_head 
> *)(full_ptr_t)bufhead32[0].ebp;
>       74         bufhead[0].ret=bufhead32[0].ret;
>       75     }
>       76     else
>       77 #endif
> 
> Any advice? Maybe the best option in this case is to avoid the compat* 
> functions and to use the original guest* functions instead.
> 
> Thanks,
> Marcus




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