Re: [PATCH v6 15/19] x86/VPMU: Add privileged PMU mode

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 05/27/2014 05:10 AM, Jan Beulich wrote:
>>>> On 27.05.14 at 04:08, <boris.ostrovsky@oracle.com> wrote:
>> On 05/26/2014 07:48 AM, Jan Beulich wrote:
>>>> +                gregs = guest_cpu_user_regs();
>>>> +                memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
>>>> +                       gregs, sizeof(struct cpu_user_regs));
>>>> +            }
>>>> +            else
>>>> +                memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
>>>> +                       regs, sizeof(struct cpu_user_regs));
>>>>    
>>>> -            cmp = (void *)&v->arch.vpmu.xenpmu_data->pmu.r.regs;
>>>> -            XLAT_cpu_user_regs(cmp, gregs);
>>>> -            memcpy(&v->arch.vpmu.xenpmu_data->pmu.r.regs,
>>>> -                   &cmp, sizeof(struct compat_cpu_user_regs));
>>>> +            gregs = &v->arch.vpmu.xenpmu_data->pmu.r.regs;
>>>> +            gregs->cs = (current->arch.flags & TF_kernel_mode) ? 0 : 0x3;
>>> Ah, no - you want to modify the structure here. But you could do this
>>> directly on the ->pmu.r.regs field rather than first latching the pointer.
>>>
>>> And as said before, it doesn't really look correct to simply set ->cs to
>>> just the RPL, especially without any comment explaining why this is
>>> (a) being done and (b) correct.
>> The reason for only passing up the RPL is that's the only field that the
>> guest is interested in (whether the interrupt happened in kernel or user
>> space). I added a comment in the code to this effect.
>>
>> Do you think that all fields need to be passed?
> How do you know what a guest is interested in? Namely 32-bit OSes
> may have uses for more than a single flat code segment, and hence
> may have an interest in knowing the full selector.

The registers are used only by the PMU handler in the guest and that 
code only wants to know where the (v)cpu was sampled.

OTOH there is really no reason *not* to pass full selector value. So 
I'll fix this.

-boris