[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v9 05/20] intel/VPMU: Clean up Intel VPMU code

On 08/11/2014 09:45 AM, Jan Beulich wrote:
On 08.08.14 at 18:55, <boris.ostrovsky@xxxxxxxxxx> wrote:
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1208,6 +1208,32 @@ int vmx_add_guest_msr(u32 msr)
      return 0;
+void vmx_rm_guest_msr(u32 msr)
+    struct vcpu *curr = current;
+    unsigned int idx, msr_count = curr->arch.hvm_vmx.msr_count;
+    struct vmx_msr_entry *msr_area = curr->arch.hvm_vmx.msr_area;
+    if ( msr_area == NULL )
+        return;
+    for ( idx = 0; idx < msr_count; idx++ )
+        if ( msr_area[idx].index == msr )
+            break;
+    if ( idx == msr_count )
+        return;
This absence check (producing success) together with the presence
check in the corresponding "add" function (also producing success) is
certainly bogus without reference counting: Two independent callers
of "add" would each validly assume they ought to call "rm" when done
with their job, taking away the control over the MSR from the other.
Yet if you don't think of independent callers, these presence/absence
checks don't make sense at all.

Hmm, yes, that's a problem. Refcounting would require separate data structures which would need to be dynamically grown (we don't know how many MSR we will be tracking and most often the number is very small).

So I wonder whether I should drop support for remove altogether since this is only used in error path and I am not sure whether adding more complexity is worth it.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.