Re: [Xen-devel] [PATCH v8 16/19] x86/VPMU: Add privileged PMU mode

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 07/29/2014 03:55 AM, Jan Beulich wrote:
> > > > On 01.07.14 at 16:37, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > @@ -187,34 +194,68 @@ int vpmu_do_interrupt(struct cpu_user_regs *regs)
> >           vpmu->arch_vpmu_ops->arch_vpmu_save(v);
> >           vpmu_reset(vpmu, VPMU_CONTEXT_SAVE | VPMU_CONTEXT_LOADED);
> >   
> > -        /* Store appropriate registers in xenpmu_data */
> > -        if ( is_pv_32bit_domain(curr_vcpu->domain) )
> > +        if ( !is_hvm_domain(current->domain) )
> "current"? Not "curr_vcpu"?
>
> >           {
> > -            /*
> > -             * 32-bit dom0 cannot process Xen's addresses (which are 64 bit)
> > -             * and therefore we treat it the same way as a non-priviledged
> > -             * PV 32-bit domain.
> > -             */
> > -            struct compat_cpu_user_regs *cmp;
> > -
> > -            gregs = guest_cpu_user_regs();
> > -
> > -            cmp = (void *)&vpmu->xenpmu_data->pmu.r.regs;
> > -            XLAT_cpu_user_regs(cmp, gregs);
> > +            /* Store appropriate registers in xenpmu_data */
> > +            if ( is_pv_32bit_domain(curr_vcpu->domain) )
> I.e. I wonder how the qualifiers above and here can depend on
> different domains.

OK, this is indeed ugly.

There are two VCPUs in play when we are in system-wide profiling mode 
--- one that is being profiled and the other which is collecting samples 
(i.e. dom0).
The first check that you commented on is trying to see whether the 
profiled CPU (i.e. current) is an HVM one (ignore the diff'ed line 
above, it was moved somewhere else, you are seeing 'quirkiness' of the 
diff). The second check with your comment is trying to figure how to 
store profiled data to be sent to the collecting VCPU (which is poorly 
named curr_vcpu).
The logic is rather convoluted here. I will (1) pick better names, (2) 
add comments and (3) possibly swap the 'if ( 
!is_hvm_domain(current->domain) )' and its 'else' clauses since I think 
it might make code a little more understandable.

-boris

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