On 05/18/2015 05:43 AM, Dietmar Hahn wrote:
Am Freitag 08 Mai 2015, 17:06:11 schrieb Boris Ostrovsky:
+ if ( !is_hvm_vcpu(sampling) )
+ {
+ /* PV(H) guest */
+ const struct cpu_user_regs *cur_regs;
+ uint64_t *flags = &vpmu->xenpmu_data->pmu.pmu_flags;
+ domid_t domid = DOMID_SELF;
+
+ if ( !vpmu->xenpmu_data )
+ return;
+
+ if ( is_pvh_vcpu(sampling) &&
+ !vpmu->arch_vpmu_ops->do_interrupt(regs) )
Here you expect vpmu->arch_vpmu_ops != NULL but ...
+ return;
+
+ if ( *flags & PMU_CACHED )
+ return;
+
...
+
+ return;
+ }
if ( vpmu->arch_vpmu_ops )
... here is a check.
Maybe this check here is unnecessary because you would never get this interrupt
without having arch_vpmu_ops != NULL to switch on the machinery?
There are some other locations too with checks before calling
vpmu->arch_vpmu_ops->... and some without. Maybe it would make sense to force
always a complete set of arch_vpmu_ops - functions to avoid this?
I was actually thinking about (eventually) dropping ops tests and
checking that all of them exist during VPMU initialization.
As for this particular test, it may be worth moving it to the beginning
of the routine, mostly to guard against spurious interrupts (but also to
avoid performing it more than once)
}
-void vpmu_load(struct vcpu *v)
+int vpmu_load(struct vcpu *v, bool_t verify)
vpmu_load uses "verify" but within the arch_vpmu_load functions
(core2_vpmu_load() and amd_vpmu_load()) you use "from_guest" for the same
meaning. This is a little bit confusing.
Always using "verify" would be clearer I think.
Then this will not be consistent with the save part (which doesn't use
the flag to verify the context but rather to only state that the routine
should copy it). So I think renaming 'verify' to 'from_guest' and
keeping arch ops as they are now would be more consistent.
Thanks.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
