[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 09/29/2014 11:40 AM, Jan Beulich wrote:
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.

Hypervisor does use but in a later patch (#16, interrupt handling). I'll move the check there.

   #define vpmu_vcpu(vpmu)   (container_of((vpmu), struct vcpu, \
-#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.

I'll add a comment about removal of this macro into commit message.


Xen-devel mailing list



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