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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 01.10.14 at 16:08, <boris.ostrovsky@xxxxxxxxxx> wrote:
> 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?

You're right - I don't know what I was thinking (or whether I
misplaced the question).

>>>> 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) )

Actually I think instead the determination of sampling needs to
depend on the register context rather than solely on the current
domain's ID.

Jan


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