linux-kernel - Re: [RFC PATCH v2 7/7] KVM: KVM: don't break vcpu 'halt' state due to apfs

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 linux-hardening linux-cve-announce 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]

Message-ID: <4CCFBE4E.3090804@cn.fujitsu.com>
Date:	Tue, 02 Nov 2010 15:31:26 +0800
From:	Xiao Guangrong <xiaoguangrong@...fujitsu.com>
To:	Gleb Natapov <gleb@...hat.com>
CC:	Avi Kivity <avi@...hat.com>, Marcelo Tosatti <mtosatti@...hat.com>,
	LKML <linux-kernel@...r.kernel.org>, KVM <kvm@...r.kernel.org>
Subject: Re: [RFC PATCH v2 7/7] KVM: KVM: don't break vcpu 'halt' state due
 to apfs

On 11/02/2010 02:56 PM, Gleb Natapov wrote:
> On Tue, Nov 02, 2010 at 10:30:10AM +0800, Xiao Guangrong wrote:
>> On 11/01/2010 08:55 PM, Gleb Natapov wrote:
>>
>>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>>> index 2cfdf2d..f7aed95 100644
>>> --- a/arch/x86/kvm/x86.c
>>> +++ b/arch/x86/kvm/x86.c
>>> @@ -5295,8 +5295,9 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
>>>  			{
>>>  				switch(vcpu->arch.mp_state) {
>>>  				case KVM_MP_STATE_HALTED:
>>> -					vcpu->arch.mp_state =
>>> -						KVM_MP_STATE_RUNNABLE;
>>> +					if (list_empty_careful(&vcpu->async_pf.done))
>>> +						vcpu->arch.mp_state =
>>> +							KVM_MP_STATE_RUNNABLE;
>>
>> if nmi/interrupt and apfs completed event occur at the same time, we will miss to
>> exit halt sate. Maybe we can check the pending event here, see below patch please.
>>
> No, we will not. If nmi/interrupt and apfs completed event occur at the same
> time kvm_vcpu_block() will exit with KVM_REQ_UNHALT set, but cpu will
> not be unhalted because of list_empty_careful(&vcpu->async_pf.done)
> check. Vcpu then will process pending apf completion and enter
> kvm_vcpu_block() again which will immediately exit because
> kvm_arch_vcpu_runnable() will return true since there is pending
> nmi/interrupt. This time vcpu will be unhalted.

Thanks for your explanation, but if it has nmi/interrupt pending,
kvm_arch_can_inject_async_page_present() always return false in PV guest case,
it can not process pending apf completion, so, the vcpu is remain halt state
forever?

Also, the pv guest can only handle an apf completion at one time, it can not ensure
vcpu->async_pf.done is empty after kvm_check_async_pf_completion()

> 
>>>  				case KVM_MP_STATE_RUNNABLE:
>>>  					vcpu->arch.apf.halted = false;
>>>  					break;
>>> @@ -6279,6 +6280,7 @@ void kvm_arch_async_page_present(struct kvm_vcpu *vcpu,
>>>  		vcpu->arch.fault.error_code = 0;
>>>  		vcpu->arch.fault.address = work->arch.token;
>>>  		kvm_inject_page_fault(vcpu);
>>> +		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
>>>  	}
>>>  }
>>
>> Have a stupid question, why we make the vcpu runnable here? Sorry i don't know
>> kvm pv guest to much. :-(
> Because kvm_arch_vcpu_runnable() does not check for pending exceptions.
> Since now we do not unhalt vcpu when apf completion happens we need to
> unhalt it here. But, as I said, the patch is untested.
> 

As i know, exception can not let guest exit halt state, only NMI/interruption can do it, yes? :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/