lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 26 Jun 2013 14:11:05 +0530
From:	"Srivatsa S. Bhat" <srivatsa.bhat@...ux.vnet.ibm.com>
To:	Paolo Bonzini <pbonzini@...hat.com>
CC:	Gleb Natapov <gleb@...hat.com>, peterz@...radead.org,
	fweisbec@...il.com, linux-kernel@...r.kernel.org,
	"H. Peter Anvin" <hpa@...or.com>, walken@...gle.com,
	mingo@...nel.org, linux-arch@...r.kernel.org,
	vincent.guittot@...aro.org, kvm@...r.kernel.org, x86@...nel.org,
	xiaoguangrong@...ux.vnet.ibm.com, Ingo Molnar <mingo@...hat.com>,
	wangyun@...ux.vnet.ibm.com, paulmck@...ux.vnet.ibm.com,
	nikunj@...ux.vnet.ibm.com, linux-pm@...r.kernel.org,
	rusty@...tcorp.com.au, rostedt@...dmis.org, namhyung@...nel.org,
	tglx@...utronix.de, laijs@...fujitsu.com, zhong@...ux.vnet.ibm.com,
	netdev@...r.kernel.org, oleg@...hat.com, sbw@....edu,
	tj@...nel.org, akpm@...ux-foundation.org,
	linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v2 29/45] kvm/vmx: Use get/put_online_cpus_atomic() to
 prevent CPU offline

On 06/26/2013 01:53 PM, Paolo Bonzini wrote:
> Il 26/06/2013 10:06, Srivatsa S. Bhat ha scritto:
>> On 06/26/2013 01:16 PM, Paolo Bonzini wrote:
>>> Il 25/06/2013 22:30, Srivatsa S. Bhat ha scritto:
>>>> -	cpu = get_cpu();
>>>> +	cpu = get_online_cpus_atomic();
>>>>  	vmx_vcpu_load(&vmx->vcpu, cpu);
>>>>  	vmx->vcpu.cpu = cpu;
>>>>  	err = vmx_vcpu_setup(vmx);
>>>>  	vmx_vcpu_put(&vmx->vcpu);
>>>> -	put_cpu();
>>>> +	put_online_cpus_atomic();
>>>
>>> The new API has a weird name.  Why are you adding new functions instead
>>> of just modifying get/put_cpu?
>>>
>>
>> Because the purpose of those two functions are distinctly different
>> from each other.
>>
>> get/put_cpu() is used to disable preemption on the local CPU. (Which
>> also disables offlining the local CPU during that critical section).
> 
> Ok, then I understood correctly... and I acked the other KVM patch.
>

Thank you!
 
> However, keeping the code on the local CPU is exactly the point of this
> particular use of get_cpu()/put_cpu().  Why does it need to synchronize
> with offlining of other CPUs?
> 

Now that I looked at it again, I think you are right, get/put_cpu() is
good enough here.

But let me explain why I initially thought we needed full synchronization
with CPU offline. In short, I wanted to synchronize the calls to
__loaded_vmcs_clear(). We have the scenario shown below:

CPU offline:
	CPU_DYING:
		hardware_disable();
		->vmclear_local_loaded_vmcss();
		  ->__loaded_vmcs_clear(v);



And vmx_vcpu_load() (among others) can do:
       vmx_vcpu_load();
       -> loaded_vmcs_clear();
          -> __loaded_vmcs_clear();


So I wanted to avoid this race-condition and hence wrapped the code with
get/put_online_cpus_atomic().

But the point I missed earlier is that loaded_vmcs_clear() calls
__loaded_vmcs_clear() using smp_call_function_single(), which itself
synchronizes properly with CPU hotplug. So there is no need to add full
hotplug synchronization in the vmx code, as you noted above.

So, please ignore this patch, and sorry for the noise!

Regards,
Srivatsa S. Bhat

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists