Re: [PATCH v7 11/19] x86/VPMU: Interface for setting PMU mode and flags

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

On 06/11/2014 06:14 AM, Jan Beulich wrote:
>>>> On 06.06.14 at 19:40, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/x86_64/compat/entry.S
>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>> @@ -417,6 +417,8 @@ ENTRY(compat_hypercall_table)
>>           .quad do_domctl
>>           .quad compat_kexec_op
>>           .quad do_tmem_op
>> +        .quad do_ni_hypercall           /* reserved for XenClient */
>> +        .quad do_xenpmu_op              /* 40 */
>>           .rept __HYPERVISOR_arch_0-((.-compat_hypercall_table)/8)
>>           .quad compat_ni_hypercall
>>           .endr
>> @@ -465,6 +467,8 @@ ENTRY(compat_hypercall_args_table)
>>           .byte 1 /* do_domctl                */
>>           .byte 2 /* compat_kexec_op          */
>>           .byte 1 /* do_tmem_op               */
>> +        .byte 0 /* reserved for XenClient   */
>> +        .byte 2 /* do_xenpmu_op             */  /* 40 */
>>           .rept __HYPERVISOR_arch_0-(.-compat_hypercall_args_table)
>>           .byte 0 /* compat_ni_hypercall      */
>>           .endr
> I'm pretty certain I said on an earlier version of the patch series
> already: If you use the same handler for native and compat mode
> hypercalls, you should add structure layout verification (see the ?
> entries in xen/include/xlat.lst and how what they produce is being
> used in various places). And the fact that the interface structure
> contains a handle already makes clear that this check will fail - I
> don't think there's  a way around having at least a thin compat
> mode wrapper here.


Right. And I decided to drop the handle since noone is using it now 
anyway and I don't want to add a compat layer for something that may 
never get used.

I have a question though. I want to add a compat check for

struct xen_pmu_arch {
     union {
         struct cpu_user_regs regs;
         uint8_t pad[256];
     } r;
     union {
         uint32_t lapic_lvtpc;
         uint64_t pad;
     } l;
     union {
         struct xen_pmu_amd_ctxt amd;
         struct xen_pmu_intel_ctxt intel;
         uint8_t pad[128];
     } c;
};

which is a shared structure (i.e. it's not used across a hypercall). The 
problem I am having is that cpu_user_regs needs to be translated. Does 
the xlat framework allow checking a structure that needs one of its 
fields translated? I went through the scripts and it appears that it 
doesn't.

Thanks.
-boris