Re: nVMX: Shadowing of CPU_BASED_VM_EXEC_CONTROL broken

[Jan Kiszka <jan.kiszka@siemens.com>]

On 2014-10-08 17:44, Paolo Bonzini wrote:
> Il 08/10/2014 17:07, Jan Kiszka ha scritto:
>> As usual, this was a nasty race that involved some concurrent VCPUs and
>> proper host load, so hard to write unit tests...
>>
>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>> index 04fa1b8..d6bcaca 100644
>> --- a/arch/x86/kvm/vmx.c
>> +++ b/arch/x86/kvm/vmx.c
>> @@ -6417,6 +6417,8 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
>>  	const unsigned long *fields = shadow_read_write_fields;
>>  	const int num_fields = max_shadow_read_write_fields;
>>  
>> +	preempt_disable();
>> +
>>  	vmcs_load(shadow_vmcs);
>>  
>>  	for (i = 0; i < num_fields; i++) {
>> @@ -6440,6 +6442,8 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx)
>>  
>>  	vmcs_clear(shadow_vmcs);
>>  	vmcs_load(vmx->loaded_vmcs->vmcs);
>> +
>> +	preempt_enable();
>>  }
>>  
>>  static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>> @@ -6457,6 +6461,8 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>>  	u64 field_value = 0;
>>  	struct vmcs *shadow_vmcs = vmx->nested.current_shadow_vmcs;
>>  
>> +	preempt_disable();
>> +
>>  	vmcs_load(shadow_vmcs);
>>  
>>  	for (q = 0; q < ARRAY_SIZE(fields); q++) {
>> @@ -6483,6 +6489,8 @@ static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx)
>>  
>>  	vmcs_clear(shadow_vmcs);
>>  	vmcs_load(vmx->loaded_vmcs->vmcs);
>> +
>> +	preempt_enable();
>>  }
>>  
>> No proper patch yet because there might be a smarter approach without
>> using the preempt_disable() hammer.
> 
> copy_vmcs12_to_shadow already runs with preemption disabled; for stable@
> it's not that bad to do the same in copy_shadow_to_vmcs12.
> 
> For 3.18 it could be nice of course to use loaded_vmcs properly, but it
> would also incur some overhead.

If the other direction is already under preempt_disable, I'm not sure if
there is much to gain for this direction.

Anyway, fix sent.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SES-DE
Corporate Competence Center Embedded Linux