Re: [PATCH 17/38] KVM: PPC: BookE: Add support for vcpu->mode

[Scott Wood <scottwood@freescale.com>]

On 08/14/2012 07:26 PM, Alexander Graf wrote:
> 
> On 15.08.2012, at 02:17, Scott Wood wrote:
> 
>> On 08/14/2012 06:04 PM, Alexander Graf wrote:
>>> Generic KVM code might want to know whether we are inside guest context
>>> or outside. It also wants to be able to push us out of guest context.
>>>
>>> Add support to the BookE code for the generic vcpu->mode field that describes
>>> the above states.
>>>
>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>> ---
>>> arch/powerpc/kvm/booke.c |   11 +++++++++++
>>> 1 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c
>>> index bcf87fe..70a86c0 100644
>>> --- a/arch/powerpc/kvm/booke.c
>>> +++ b/arch/powerpc/kvm/booke.c
>>> @@ -501,6 +501,15 @@ static int kvmppc_prepare_to_enter(struct kvm_vcpu *vcpu)
>>> 			continue;
>>> 		}
>>>
>>> +		if (vcpu->mode == EXITING_GUEST_MODE) {
>>> +			r = 1;
>>> +			break;
>>> +		}
>>> +
>>> +		/* Going into guest context! Yay! */
>>> +		vcpu->mode = IN_GUEST_MODE;
>>> +		smp_wmb();
>>> +
>>> 		break;
>>> 	}
>>
>> Normally on entry to this function mode should be OUTSIDE_GUEST_MODE,
>> right?  How could it possibly be EXITING_GUEST_MODE then, since that
>> only replaces IN_GUEST_MODE?
>>
>> This doesn't match what x86 does with mode on entry.  Mode is supposed
>> to be set to IN_GUEST_MODE before requests are checked.
>>
>> I'm not sure what the point of EXITING_GUEST_MODE is at all, compared to
>> just waiting until after interrupts are disabled before setting
>> IN_GUEST_MODE (which we do on ppc, but not on x86 even though it seems
>> like a trivial change), plus the existing ordering between mode and
>> requests.
> 
> Well, the only real use case I could find for the mode was the remote
> vcpu kick. If we're not outside of guest mode, we get an IPI to
> notify us that requests are outstanding.

I'm curious why this is done so differently for broadcast requests than
for single-cpu requests.

> So I only get us into OUTSIDE_GUEST_MODE when we really exit
> __vcpu_run, thus are in user space. That doesn't reflect what x86
> does, right, but so doesn't our whole loop concept.

OK.  We still need to do ordering like x86 does, because otherwise
there's a race where we could check requests before the request bit is
set, and still have make_all_cpus_request see OUTSIDE_GUEST_MODE and not
send an IPI.

> However, since we might do the vcpu_block in this loop, we will never
> really get into OUTSIDE_GUEST_MODE, right?

Not when the guest is just idling, only when it's exited to QEMU.

> Hrm. So what would you
> suggest? Do all the handle_exit in OUTSIDE_GUEST_MODE scope and then
> reenter IN_GUEST_MODE in prepare_to_enter? 

I suggest leaving this optimization until a need is felt.

> We still wouldn't need an EXITING_GUEST_MODE though.

Right.

-Scott