[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v24 04/15] x86/VPMU: Interface for setting PMU mode and flags
>>> On 11.06.15 at 16:54, <boris.ostrovsky@xxxxxxxxxx> wrote: > On 06/11/2015 04:17 AM, Tian, Kevin wrote: >>> From: Boris Ostrovsky [mailto:boris.ostrovsky@xxxxxxxxxx] > >>> switch ( vendor ) >>> { >>> case X86_VENDOR_AMD: >>> - ret = svm_vpmu_initialise(v, opt_vpmu_enabled); >>> + ret = svm_vpmu_initialise(v); >>> break; >>> >>> case X86_VENDOR_INTEL: >>> - ret = vmx_vpmu_initialise(v, opt_vpmu_enabled); >>> + ret = vmx_vpmu_initialise(v); >>> break; >>> >>> default: >>> - if ( opt_vpmu_enabled ) >>> + if ( vpmu_mode != XENPMU_MODE_OFF ) >>> { >>> printk(XENLOG_G_WARNING "VPMU: Unknown CPU vendor %d. " >>> "Disabling VPMU\n", vendor); >>> opt_vpmu_enabled = 0; >>> + vpmu_mode = XENPMU_MODE_OFF; >>> } >>> - return; >>> + return; /* Don't bother restoring vpmu_count, VPMU is off forever > */ >> >> why not restoring vpmu_count here? There could be a race condition >> regarding to the mode control path: >> >> + 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; >> >> It's possible "vpmu_mode=pmu_params.val" happens later than >> "vpmu_mode = XENPMU_MODE_OFF"... >> >> It might not be a big problem since opt_vpmu_enabled is 0 then, but >> then there's pointless to reset vpmu_mode further if the behavior >> is not guaranteed. > > > It is somewhat pointless to reset it, but mostly because if we ever get > into the default case above we have much bigger problems than VPMU: the > only way that I can see when this can happen (vendor not being AMD or > Intel) is that current_cpu_data.x86_vendor got overwritten by something > else, which means memory corruption. Or you're running on a Cyrix CPU (unless there's code earlier on preventing that switch() from being reached). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |