Re: [Xen-devel] [PATCH v12 for-xen-4.5 16/20] x86/VPMU: Handle PMU interrupts for PV guests

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 10/01/2014 09:18 AM, Jan Beulich wrote:
> > > > On 01.10.14 at 14:53, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > On 10/01/2014 02:49 AM, Jan Beulich wrote:
> > > > > > On 30.09.14 at 18:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > > On 09/30/2014 11:44 AM, Jan Beulich wrote:
> > > > > > > > +            {
> > > > > > > > +                r->cs = cur_regs->cs;
> > > > > > > > +                if ( sampled->arch.flags & TF_kernel_mode )
> > > > > > > > +                    r->cs &= ~3;
> > > > > > > And once again I wonder how the consumer of this data is to tell
> > > > > > > apart guest kernel and hypervisor addresses.
> > > > > > Based on the RIP --- perf, for example, searches through various symbol
> > > > > > tables.
> > > > > That doesn't help when profiling HVM/PVH guests - addresses are
> > > > > ambiguous in that case.
> > > > Hypervisor traces are only sent to dom0, which is currently PV only. The
> > > > key here, of course, is the word 'currently'.
> > > So you completely ignore PVH Dom0? Experimental or not, I don't
> > > think that's the way to go.
> > As I mentioned in an earlier reply, I will set domain_id in the reported
> > structure to DOMID_XEN when we are reporting hypervisor sample.
> >
> > > Furthermore the check around this is
> > > once again using sampled, not sampling.
> > Which check are you referring to?
> The if() right outside (above) the still visible patch context.
Why should it be 'sampling'? I am collecting registers from sampled vcpu 
so I need to look at that domain's flags to determine the mode, don't I?

> > > Looking at the separation of hypervisor vs guest context to report
> > > again
> > >
> > >               /* Non-privileged domains are always in XENPMU_MODE_SELF mode */
> > >               if ( (vpmu_mode & XENPMU_MODE_SELF) ||
> > >                    (!is_hardware_domain(sampled->domain) &&
> > >                     !is_idle_vcpu(sampled)) )
> > >                   cur_regs = guest_cpu_user_regs();
> > >               else
> > >                   cur_regs = regs;
> > >
> > > I now additionally wonder why the condition here isn't just the SELF
> > > check: If the interrupt happened while in the hypervisor, why would
> > > you override this unconditionally to report a guest sample instead?
> > > Shouldn't the profiling domain tell you what it wants in that case
> > > (global vs guest local view)?
> > The second part of the check (!is_hardware_domain(sampled->domain) &&
> > !is_idle_vcpu(sampled)) is to prevent sending hypervisor sample to a
> > non-privileged guest. vpmu_mode may be, for example, XENPMU_MODE_HV but
> > that only means that dom0 can get hypervisor samples.
> Right, but that's not what the code above does: Instead of sending
> the hypervisor sample to Dom0 it converts it to a guest mode one.
Oh, I see --- when we get interrupted while in a non-privileged guest's 
context (but in hypervisor) I send guest's registers, not Xen's.
I think just SELF check is not sufficient though, we need to make sure 
that we are not sending hypervisor sample to non-dom0. So
    if ( (vpmu_mode & XENPMU_MODE_SELF) || 
!is_hardware_domain(sampling->domain) )
-boris


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel