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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 11.08.14 at 18:01, <boris.ostrovsky@xxxxxxxxxx> wrote:
> 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.

Probably not - see e.g. the debugctl and lbr handling, which also only
ever adds MSRs to that list. That's not optimal, but sufficient for now -
if we ever learn this presents a (performance) problem, we could then
start thinking about adding remove support.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel