[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v23 11/15] VPMU/AMD: Check MSR values before writing to hardware
>>> On 29.05.15 at 20:42, <boris.ostrovsky@xxxxxxxxxx> wrote: > @@ -289,19 +302,24 @@ static int amd_vpmu_do_wrmsr(unsigned int msr, uint64_t > msr_content, > { > struct vcpu *v = current; > struct vpmu_struct *vpmu = vcpu_vpmu(v); > + unsigned int idx = 0; > + int type = get_pmu_reg_type(msr, &idx); > > ASSERT(!supported); > > /* For all counters, enable guest only mode for HVM guest */ > - if ( has_hvm_container_vcpu(v) && > - (get_pmu_reg_type(msr) == MSR_TYPE_CTRL) && > + if ( has_hvm_container_vcpu(v) && (type == MSR_TYPE_CTRL) && > !is_guest_mode(msr_content) ) > { > set_guest_mode(msr_content); > } > > + if ( (type == MSR_TYPE_CTRL ) && > + ((msr_content & CTRL_RSVD_MASK) != ctrl_rsvd[idx]) ) > + return 1; I think the check should go before the use of the value for anything, i.e. including the set_guest_mode() above. Also I'm pretty sure I asked about the meaning of 1 as a return value of a function returning int: If this is a boolean, the function should return bool_t (and probably use 1 as success indicator, while the case at hand appears to be a failure one). If this is an error indicator, -E... values should be used. Of course, if for some reason this is meant to represent success, considering the function being that way even before your change, I'd not insist on you re-working the return value aspects of it. Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |