Xen devel - Jul 2006 - xenoprof passive profiling and "mode" setting

Xiaowei,

I''m puzzled by the following bit of code in p4_check_ctrs():

       if (guest_kernel_mode(current, regs))
                mode = 1;
        else if (ring_0(regs))
                mode = 2;

If I look at the definition for guest_kernel_mode() it is:

include/asm-x86/x86_64/regs.h:

#define guest_kernel_mode(v, r)   \
    (ring_3(r) && ((v)->arch.flags & TF_kernel_mode))

(Note well:  I am running a 64 bit guest and a 64 bit host.)

So, how is that possibly true for the kernel in an HVM guest?
(Wouldn''t guest_kernel_mode(regs) be false in that case since it is not
running ring_3()?)

Also, wouldn''t ring_0(regs) be true if we are running in kernel mode in
an HVM
guest?    This code appears to assign such a sample to being part of xen, 
right?

So I''m unsure as to whether the above snippet of code sets the mode
correctly.

-- 
Ray Bryant
AMD Performance Labs                   Austin, Tx
512-602-0038 (o)                 512-507-7807 (c)



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

On 5 Jul 2006, at 19:27, Ray Bryant wrote:
> So, how is that possibly true for the kernel in an HVM guest?
> (Wouldn''t guest_kernel_mode(regs) be false in that case since it
is not
> running ring_3()?)
guest_kernel_mode() does not work for HVM guests. It may need to be 
fixed -- it had previously only been used in paravirtual-only contexts.

It might make sense to invert[*] the predicate and rename to 
user_mode(). Then definition is simply ring_3(regs) for x86/32 and 
(ring_3(regs) && !((v)->arch.flags & TF_kernel_mode)) for x86/64.
> Also, wouldn''t ring_0(regs) be true if we are running in kernel
mode
> in an HVM
> guest?    This code appears to assign such a sample to being part of 
> xen,
> right?
ring_0(regs) should be replaced by !guest_mode(regs).

So maybe:
   int mode = 2;
   if (guest_mode(regs))
       mode = user_mode(current, regs) ? 0 : 1;

  -- Keir

[*] Not really inversion, as neither guest_kernel_mode() nor 
user_mode() are true for hypervisor mode.


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

>guest_kernel_mode() does not work for HVM guests. It may need to be
>fixed -- it had previously only been used in paravirtual-only contexts.
>
>It might make sense to invert[*] the predicate and rename to
>user_mode(). Then definition is simply ring_3(regs) for x86/32 and
>(ring_3(regs) && !((v)->arch.flags & TF_kernel_mode)) for
x86/64.
>
>So maybe:
>   int mode = 2;
>   if (guest_mode(regs))
>       mode = user_mode(current, regs) ? 0 : 1;
>
Yes, this is a better solution for sure, to take both para-domain and
hvm into account. 
But it''s not a problem for now:) _mode_ logic only applies to active
domiain. Oprofile doesn''t use it for samples between PASSIVE_START_CODE
and PASSIVE_STOP_CODE. Rather it relies on PC range to distinguish
xen/kernel/app samples. 

Thanks,
Xiaowei

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

On Wednesday 05 July 2006 21:53, Yang, Xiaowei wrote:
> >guest_kernel_mode() does not work for HVM guests. It may need to be
> >fixed -- it had previously only been used in paravirtual-only contexts.
> >
> >It might make sense to invert[*] the predicate and rename to
> >user_mode(). Then definition is simply ring_3(regs) for x86/32 and
> >(ring_3(regs) && !((v)->arch.flags & TF_kernel_mode))
for x86/64.
> >
> >So maybe:
> >   int mode = 2;
> >   if (guest_mode(regs))
> >       mode = user_mode(current, regs) ? 0 : 1;
>
> Yes, this is a better solution for sure, to take both para-domain and
> hvm into account.
> But it''s not a problem for now:) _mode_ logic only applies to
active
> domiain. Oprofile doesn''t use it for samples between
PASSIVE_START_CODE
> and PASSIVE_STOP_CODE. Rather it relies on PC range to distinguish
> xen/kernel/app samples.
>
> Thanks,
> Xiaowei
Xiaowei,

Hmmm.... It seems to me that the pc range thing is at the very least obscure 
(I certainly didn''t spot it, but then I''ve avoided looking at
the oprofile
code).   I''d rather have the mode set correctly for hvm guests (passive
profiling) as well, and fix the logic downstream in oprofile to deal with it 
correctly without the pc range info.

Besides, isn''t this problem in the guest_kernel_mode(regs) macro just
going to
bite someone else later on?   It would make most sense to fix this so it 
works for all guests, not just paravirtualized ones.

-- 
Ray Bryant
AMD Performance Labs                   Austin, Tx
512-602-0038 (o)                 512-507-7807 (c)



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

Ray is right.
We should set the mode correctly in Xen (for both paravirtualized and
HVM guests) and fix Oprofile to use mode instead of relying on PC range.
This would be a good opportunity to fix the way Oprofile determine the
kernel PC range for passive domains. Right now the PC range is defined
by hardcoded constants which obviously is not the right thing to do.

Renato
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xensource.com 
>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of 
>> Ray Bryant
>> Sent: Thursday, July 06, 2006 9:47 AM
>> To: Yang, Xiaowei
>> Cc: xen-devel@lists.xensource.com
>> Subject: Re: [Xen-devel] xenoprof passive profiling and 
>> "mode" setting
>> 
>> On Wednesday 05 July 2006 21:53, Yang, Xiaowei wrote:
>> > >guest_kernel_mode() does not work for HVM guests. It may 
>> need to be 
>> > >fixed -- it had previously only been used in 
>> paravirtual-only contexts.
>> > >
>> > >It might make sense to invert[*] the predicate and rename to 
>> > >user_mode(). Then definition is simply ring_3(regs) for x86/32
and
>> > >(ring_3(regs) && !((v)->arch.flags &
TF_kernel_mode)) for x86/64.
>> > >
>> > >So maybe:
>> > >   int mode = 2;
>> > >   if (guest_mode(regs))
>> > >       mode = user_mode(current, regs) ? 0 : 1;
>> >
>> > Yes, this is a better solution for sure, to take both 
>> para-domain and 
>> > hvm into account.
>> > But it''s not a problem for now:) _mode_ logic only
applies
>> to active 
>> > domiain. Oprofile doesn''t use it for samples between 
>> > PASSIVE_START_CODE and PASSIVE_STOP_CODE. Rather it relies 
>> on PC range 
>> > to distinguish xen/kernel/app samples.
>> >
>> > Thanks,
>> > Xiaowei
>> 
>> Xiaowei,
>> 
>> Hmmm.... It seems to me that the pc range thing is at the 
>> very least obscure (I certainly didn''t spot it, but then 
>> I''ve avoided looking at the oprofile 
>> code).   I''d rather have the mode set correctly for hvm 
>> guests (passive 
>> profiling) as well, and fix the logic downstream in oprofile 
>> to deal with it correctly without the pc range info.
>> 
>> Besides, isn''t this problem in the guest_kernel_mode(regs) 
>> macro just going to 
>> bite someone else later on?   It would make most sense to 
>> fix this so it 
>> works for all guests, not just paravirtualized ones.
>> 
>> --
>> Ray Bryant
>> AMD Performance Labs                   Austin, Tx
>> 512-602-0038 (o)                 512-507-7807 (c)
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel