[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v12 for-xen-4.5 09/20] x86/VPMU: Add public xenpmu.h



>>> On 29.09.14 at 16:57, <boris.ostrovsky@xxxxxxxxxx> wrote:
> On 09/29/2014 10:17 AM, Jan Beulich wrote:
>>>>> On 25.09.14 at 21:28, <boris.ostrovsky@xxxxxxxxxx> wrote:
>>> @@ -228,6 +229,11 @@ void vpmu_initialise(struct vcpu *v)
>>>       struct vpmu_struct *vpmu = vcpu_vpmu(v);
>>>       uint8_t vendor = current_cpu_data.x86_vendor;
>>>   
>>> +    BUILD_BUG_ON(sizeof(struct xen_pmu_intel_ctxt) > XENPMU_CTXT_PAD_SZ);
>>> +    BUILD_BUG_ON(sizeof(struct xen_pmu_amd_ctxt) > XENPMU_CTXT_PAD_SZ);
>>> +    BUILD_BUG_ON(sizeof(struct xen_pmu_regs) > XENPMU_REGS_PAD_SZ);
>>> +    BUILD_BUG_ON(sizeof(struct compat_pmu_regs) > XENPMU_REGS_PAD_SZ);
>> I'm having trouble finding where struct compat_pmu_regs gets defined
>> (largely since you're not adding anything to xen/include/xlat.h).
> 
> 
> It is generated into include/compat/arch-x86/xen-x86_32.h which is 
> included via xen.h. And so is xlat.h -- but I didn't think I needed to 
> add anything there (via xlat.lst) since I didn't see any reason for 
> checking or translating it.

Right. In which case the check above is a bit pointless, and
possibly confusing (just like it happened to me now). If you
need the struct in a subsequent patch, move the check there.
If you don't ever need it till the end of your series, please
annotate the above stating that this is just for completeness
despite the hypervisor itself never using the structure.

>>>   #define vpmu_vcpu(vpmu)   (container_of((vpmu), struct vcpu, \
>>>                                             arch.hvm_vcpu.vpmu))
>>> -#define vpmu_domain(vpmu) (vpmu_vcpu(vpmu)->domain)
>> Is this really useful to delete, i.e. are absolutely sure that no future
>> use will ever arise?
> 
> We never use it (and never had, apparently) so I didn't see any reason 
> to carry it forward.

Oh, if it was never used, then the change is unrelated here - I was
taking it being here as a sign that the patch replaced the last
existing user.

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.