[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v16 02/23] x86/VPMU: Don't globally disable VPMU if initialization fails.
>>> On 18.12.14 at 17:08, <boris.ostrovsky@xxxxxxxxxx> wrote: > On 12/18/2014 09:10 AM, Jan Beulich wrote: >>>>> On 17.12.14 at 16:38, <boris.ostrovsky@xxxxxxxxxx> wrote: >>> The failure to initialize VPMU may be temporary so we shouldn'd disable VMPU >>> forever. >> Reported-by: Jan Beulich <jbeulich@xxxxxxxx> >> (or Suggested-by if you like that better) >> >>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx> >>> --- >>> xen/arch/x86/hvm/vpmu.c | 15 ++++++++------- >>> 1 file changed, 8 insertions(+), 7 deletions(-) >>> >>> diff --git a/xen/arch/x86/hvm/vpmu.c b/xen/arch/x86/hvm/vpmu.c >>> index 1df74c2..b43ea80 100644 >>> --- a/xen/arch/x86/hvm/vpmu.c >>> +++ b/xen/arch/x86/hvm/vpmu.c >>> @@ -218,6 +218,7 @@ void vpmu_initialise(struct vcpu *v) >>> { >>> struct vpmu_struct *vpmu = vcpu_vpmu(v); >>> uint8_t vendor = current_cpu_data.x86_vendor; >>> + int ret; >>> >>> if ( is_pvh_vcpu(v) ) >>> return; >>> @@ -230,21 +231,21 @@ void vpmu_initialise(struct vcpu *v) >>> switch ( vendor ) >>> { >>> case X86_VENDOR_AMD: >>> - if ( svm_vpmu_initialise(v, opt_vpmu_enabled) != 0 ) >>> - opt_vpmu_enabled = 0; >>> + ret = svm_vpmu_initialise(v, opt_vpmu_enabled); >>> break; >>> >>> case X86_VENDOR_INTEL: >>> - if ( vmx_vpmu_initialise(v, opt_vpmu_enabled) != 0 ) >>> - opt_vpmu_enabled = 0; >>> + ret = vmx_vpmu_initialise(v, opt_vpmu_enabled); >>> break; >>> >>> default: >>> - printk("VPMU: Initialization failed. " >>> - "Unknown CPU vendor %d\n", vendor); >>> - opt_vpmu_enabled = 0; >>> + ret = -EINVAL; >>> + printk(XENLOG_G_WARNING "VPMU: Unknown CPU vendor %d\n", vendor); >>> break; >>> } >>> + >>> + if ( ret ) >>> + printk(XENLOG_G_WARNING "VPMU: Initialization failed for %pv\n", >>> v); >> Please avoid issuing two messages for the same problem. And the >> one in the default case probably doesn't make sense to be issued >> more than once (and then perhaps at boot time, if that doesn't >> happen already). > > I see not doing this for default case but arch-specific inits may (at > least in principle) fail for some VCPUs and not for others so I think I > should keep warnings. Right - that's what I asked for. The two aspects I disliked are that you print two messages for a single vCPU in the default case, and repeatedly the default case message despite the condition being invariable across vCPU-s. > As for printing only once --- I often wondered whether we should have > something similar to Linux' WARN_ONCE(). It would have been printk_once() you want here, but with the change you're planning that's probably moot now anyway. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |