[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v25 04/15] x86/VPMU: Interface for setting PMU mode and flags
On 06/22/2015 11:10 AM, Jan Beulich wrote: + switch ( op ) + { + case XENPMU_mode_set: + { + if ( (pmu_params.val & ~(XENPMU_MODE_SELF | XENPMU_MODE_HV)) || + (hweight64(pmu_params.val) > 1) ) + return -EINVAL; + + /* 32-bit dom0 can only sample itself. */ + if ( is_pv_32bit_vcpu(current) && (pmu_params.val & XENPMU_MODE_HV) ) + return -EINVAL; + + spin_lock(&vpmu_lock); + + /* + * We can always safely switch between XENPMU_MODE_SELF and + * XENPMU_MODE_HV while other VPMUs are active. + */ + if ( (vpmu_count == 0) || (vpmu_mode == pmu_params.val) || + ((vpmu_mode ^ pmu_params.val) == + (XENPMU_MODE_SELF | XENPMU_MODE_HV)) ) + vpmu_mode = pmu_params.val; + else + {I think this would better be if ( (vpmu_count == 0) || ((vpmu_mode ^ pmu_params.val) == (XENPMU_MODE_SELF | XENPMU_MODE_HV)) ) vpmu_mode = pmu_params.val; else if ( vpmu_mode != pmu_params.val ) { But there's no need to re-submit just because of this (it could be changed upon commit if you agree). This will generate an error (with a message to the log) even if we keep the mode unchanged, something that I wanted to avoid. (Same thing for XENPMU_feature_set, which is what Kevin mentioned last time). Or wait - I just checked again, and while I thought this was long addressed I still don't seen struct xen_pmu_params "pad" field being checked to be zero as input, and being stored as zero when only an output. Did this get lost? Did I forget about a reason why this isn't being done? Unless the latter is the case, the ack above is dependent upon this getting fixed. Yes, we did talk about this and as result I added major version check (which I should have had there anyway): if we decide to start using this field we'd need to increment the major version. -boris _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |