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

[Boris Ostrovsky <boris.ostrovsky@oracle.com>]

On 08/11/2014 09:45 AM, Jan Beulich wrote:
>>>> On 08.08.14 at 18:55, <boris.ostrovsky@oracle.com> 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.

-boris