Re: [PATCH v10 09/20] x86/VPMU: Add public xenpmu.h

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

On 09/11/2014 02:39 AM, Jan Beulich wrote:
>>>> On 10.09.14 at 19:23, <boris.ostrovsky@oracle.com> wrote:
>> On 09/10/2014 10:45 AM, Jan Beulich wrote:
>>>>>> On 04.09.14 at 05:41, <boris.ostrovsky@oracle.com> wrote:
>>>> +struct xen_pmu_arch {
>>>> +    union {
>>>> +        struct cpu_user_regs regs;
>>>> +        uint8_t pad[256];
>>>> +    } r;
>>> Can you remind me again what you need the union and padding for
>>> here?
>> This structure is laid out in a shared page with a (possibly 32-bit)
>> guest who need to access fields that follow this union.
> Hmm, okay. But how would such a guest make reasonable use of
> the regs field then?

When hypervisor is preparing this data for 32-bit consumer in 
vpmu_do_interrupts() it translates registers to 32-bit version:

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

I remember struggling trying to figure a better way of presenting this 
but ended up with the (void *) cast. IIRC I tried putting 
compat_cpu_user_regs into the union but something didn't quite work 
(with compilation).

> And then - why 256 and not 200? struct
> cpu_user_regs can't change size anyway. Plus, finally, why do
> you expose the GPRs but not any of the other register state?

I wanted to leave some padding in case we decide to add non-GPR 
registers and keep major version of the interface unchanged (only minor 
version will bumped). TBH though, I can't think of any non-GPR registers 
to be ever useful.

-boris

> Yes,
> I realize I should have thought of these earlier, but better now
> than after the new ABI got established.
>
> Jan
>