[PATCH] KVM: PPC: Apply paravirt to all vcpu

[PATCH] KVM: PPC: Apply paravirt to all vcpu

[Liu Yu]

Previously, only primary vcpu get enabled paravirt.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/kvm/powerpc.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index 73508e7..a727064 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -78,8 +78,13 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
 	switch (nr) {
 	case HC_VENDOR_KVM | KVM_HC_PPC_MAP_MAGIC_PAGE:
 	{
-		vcpu->arch.magic_page_pa = param1;
-		vcpu->arch.magic_page_ea = param2;
+		struct kvm *kvm = vcpu->kvm;
+		struct kvm_vcpu *v;
+
+		kvm_for_each_vcpu(r, v, kvm) {
+			v->arch.magic_page_pa = param1;
+			v->arch.magic_page_ea = param2;
+		}
 
 		r2 = KVM_MAGIC_FEAT_SR | KVM_MAGIC_FEAT_MAS0_TO_SPRG7;
 
-- 
1.6.4

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Alexander Graf]

On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:

> Previously, only primary vcpu get enabled paravirt.

Please fix it the other way around. Thd hypercall is CPU local and should stay that way, so we have to call it on each vcpu inside the guest.

Alex

> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>
> ---
> arch/powerpc/kvm/powerpc.c |    9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index 73508e7..a727064 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -78,8 +78,13 @@ int kvmppc_kvm_pv(struct kvm_vcpu *vcpu)
>    switch (nr) {
>    case HC_VENDOR_KVM | KVM_HC_PPC_MAP_MAGIC_PAGE:
>    {
> -        vcpu->arch.magic_page_pa = param1;
> -        vcpu->arch.magic_page_ea = param2;
> +        struct kvm *kvm = vcpu->kvm;
> +        struct kvm_vcpu *v;
> +
> +        kvm_for_each_vcpu(r, v, kvm) {
> +            v->arch.magic_page_pa = param1;
> +            v->arch.magic_page_ea = param2;
> +        }
> 
>        r2 = KVM_MAGIC_FEAT_SR | KVM_MAGIC_FEAT_MAS0_TO_SPRG7;
> 
> -- 
> 1.6.4
> 
> 

RE: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de] 
> Sent: Tuesday, November 22, 2011 7:14 PM
> To: Liu Yu-B13201
> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> 
> 
> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
> 
> > Previously, only primary vcpu get enabled paravirt.
> 
> Please fix it the other way around. Thd hypercall is CPU 
> local and should stay that way, so we have to call it on each 
> vcpu inside the guest.
> 

The guest kernel already use on_each_cpu()
But seems it doesn't work.
The place primary cpu do hypercall is still in early_init
where secondary cpus don't get kicked.

Thanks,
Yu

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Alexander Graf]

On 22.11.2011, at 12:19, Liu Yu-B13201 <B13201@freescale.com> wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de] 
>> Sent: Tuesday, November 22, 2011 7:14 PM
>> To: Liu Yu-B13201
>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>> 
>> 
>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
>> 
>>> Previously, only primary vcpu get enabled paravirt.
>> 
>> Please fix it the other way around. Thd hypercall is CPU 
>> local and should stay that way, so we have to call it on each 
>> vcpu inside the guest.
>> 
> 
> The guest kernel already use on_each_cpu()
> But seems it doesn't work.
> The place primary cpu do hypercall is still in early_init
> where secondary cpus don't get kicked.

Ouch. Then let's go with this approach and

a) update the hypercall documentation
b) change the guest code to not loop through all cpus
c) flush the tlb cache on all vcpus from the hc handler

Alex

> 

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Scott Wood]

On 11/22/2011 05:27 AM, Alexander Graf wrote:
> 
> 
> 
> 
> On 22.11.2011, at 12:19, Liu Yu-B13201 <B13201@freescale.com> wrote:
> 
>>
>>
>>> -----Original Message-----
>>> From: Alexander Graf [mailto:agraf@suse.de] 
>>> Sent: Tuesday, November 22, 2011 7:14 PM
>>> To: Liu Yu-B13201
>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>>>
>>>
>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
>>>
>>>> Previously, only primary vcpu get enabled paravirt.
>>>
>>> Please fix it the other way around. Thd hypercall is CPU 
>>> local and should stay that way, so we have to call it on each 
>>> vcpu inside the guest.
>>>
>>
>> The guest kernel already use on_each_cpu()
>> But seems it doesn't work.
>> The place primary cpu do hypercall is still in early_init
>> where secondary cpus don't get kicked.
> 
> Ouch. Then let's go with this approach and
> 
> a) update the hypercall documentation
> b) change the guest code to not loop through all cpus
> c) flush the tlb cache on all vcpus from the hc handler

It's currently only our internal tree that does it from early_init (as
part of the idle paravirt patch, to avoid races -- though I can't recall
now what the problematic race is there).  It should have been changed
for the SPRG4-7 paravirt as well.  We don't want a secondary CPU to take
an exception and save something into a paravirt SPRG, but read from the
hardware SPRG, due to the patching being incomplete.

An alternative would be to still do it after secondaries are up, but
instead of just doing the hcall in kvm_map_magic_page, all but one cpu
would be held in a loop with interrupts off until the patching is complete.

Or just always use the supervisor version of SPRG4-7 for kernel access,
whether reading or writing -- this should always trap in PR-mode.

-Scott

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Alexander Graf]

On 22.11.2011, at 19:36, Scott Wood <scottwood@freescale.com> wrote:

> On 11/22/2011 05:27 AM, Alexander Graf wrote:
>> 
>> 
>> 
>> 
>> On 22.11.2011, at 12:19, Liu Yu-B13201 <B13201@freescale.com> wrote:
>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: Alexander Graf [mailto:agraf@suse.de] 
>>>> Sent: Tuesday, November 22, 2011 7:14 PM
>>>> To: Liu Yu-B13201
>>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
>>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>>>> 
>>>> 
>>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
>>>> 
>>>>> Previously, only primary vcpu get enabled paravirt.
>>>> 
>>>> Please fix it the other way around. Thd hypercall is CPU 
>>>> local and should stay that way, so we have to call it on each 
>>>> vcpu inside the guest.
>>>> 
>>> 
>>> The guest kernel already use on_each_cpu()
>>> But seems it doesn't work.
>>> The place primary cpu do hypercall is still in early_init
>>> where secondary cpus don't get kicked.
>> 
>> Ouch. Then let's go with this approach and
>> 
>> a) update the hypercall documentation
>> b) change the guest code to not loop through all cpus
>> c) flush the tlb cache on all vcpus from the hc handler
> 
> It's currently only our internal tree that does it from early_init (as
> part of the idle paravirt patch, to avoid races -- though I can't recall
> now what the problematic race is there).  It should have been changed
> for the SPRG4-7 paravirt as well.  We don't want a secondary CPU to take
> an exception and save something into a paravirt SPRG, but read from the
> hardware SPRG, due to the patching being incomplete.
> 
> An alternative would be to still do it after secondaries are up, but
> instead of just doing the hcall in kvm_map_magic_page, all but one cpu
> would be held in a loop with interrupts off until the patching is complete.

That sounds good. Then they can all do the hcall themselves and contine running.

The only downside here would be that it wastes a few cycles because the spinning needs to happen somewhere. Too bad ppc doesn't have mwait ;)

> 
> Or just always use the supervisor version of SPRG4-7 for kernel access,
> whether reading or writing -- this should always trap in PR-mode.

No, halting all other cpus sounds better.

Alex

> 

RE: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Yoder Stuart-B08248]

> -----Original Message-----
> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of
> Alexander Graf
> Sent: Tuesday, November 22, 2011 2:45 PM
> To: Wood Scott-B07421
> Cc: Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> 
> 
> On 22.11.2011, at 19:36, Scott Wood <scottwood@freescale.com> wrote:
> 
> > On 11/22/2011 05:27 AM, Alexander Graf wrote:
> >>
> >>
> >>
> >>
> >> On 22.11.2011, at 12:19, Liu Yu-B13201 <B13201@freescale.com> wrote:
> >>
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Alexander Graf [mailto:agraf@suse.de]
> >>>> Sent: Tuesday, November 22, 2011 7:14 PM
> >>>> To: Liu Yu-B13201
> >>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
> >>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> >>>>
> >>>>
> >>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
> >>>>
> >>>>> Previously, only primary vcpu get enabled paravirt.
> >>>>
> >>>> Please fix it the other way around. Thd hypercall is CPU local and
> >>>> should stay that way, so we have to call it on each vcpu inside the
> >>>> guest.
> >>>>
> >>>
> >>> The guest kernel already use on_each_cpu() But seems it doesn't
> >>> work.
> >>> The place primary cpu do hypercall is still in early_init where
> >>> secondary cpus don't get kicked.
> >>
> >> Ouch. Then let's go with this approach and
> >>
> >> a) update the hypercall documentation
> >> b) change the guest code to not loop through all cpus
> >> c) flush the tlb cache on all vcpus from the hc handler
> >
> > It's currently only our internal tree that does it from early_init (as
> > part of the idle paravirt patch, to avoid races -- though I can't
> > recall now what the problematic race is there).  It should have been
> > changed for the SPRG4-7 paravirt as well.  We don't want a secondary
> > CPU to take an exception and save something into a paravirt SPRG, but
> > read from the hardware SPRG, due to the patching being incomplete.
> >
> > An alternative would be to still do it after secondaries are up, but
> > instead of just doing the hcall in kvm_map_magic_page, all but one cpu
> > would be held in a loop with interrupts off until the patching is complete.
> 
> That sounds good. Then they can all do the hcall themselves and contine running.

Why do the secondaries need to spin...can they just make the call
as the very first thing when coming out of the spin table?

Just let the boot CPU do the patching before releasing
the secondaries.

Stuart

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Alexander Graf]

On 22.11.2011, at 22:11, Yoder Stuart-B08248 wrote:

> 
> 
>> -----Original Message-----
>> From: kvm-ppc-owner@vger.kernel.org [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of
>> Alexander Graf
>> Sent: Tuesday, November 22, 2011 2:45 PM
>> To: Wood Scott-B07421
>> Cc: Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>> 
>> 
>> On 22.11.2011, at 19:36, Scott Wood <scottwood@freescale.com> wrote:
>> 
>>> On 11/22/2011 05:27 AM, Alexander Graf wrote:
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On 22.11.2011, at 12:19, Liu Yu-B13201 <B13201@freescale.com> wrote:
>>>> 
>>>>> 
>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>>>> Sent: Tuesday, November 22, 2011 7:14 PM
>>>>>> To: Liu Yu-B13201
>>>>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
>>>>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>>>>>> 
>>>>>> 
>>>>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
>>>>>> 
>>>>>>> Previously, only primary vcpu get enabled paravirt.
>>>>>> 
>>>>>> Please fix it the other way around. Thd hypercall is CPU local and
>>>>>> should stay that way, so we have to call it on each vcpu inside the
>>>>>> guest.
>>>>>> 
>>>>> 
>>>>> The guest kernel already use on_each_cpu() But seems it doesn't
>>>>> work.
>>>>> The place primary cpu do hypercall is still in early_init where
>>>>> secondary cpus don't get kicked.
>>>> 
>>>> Ouch. Then let's go with this approach and
>>>> 
>>>> a) update the hypercall documentation
>>>> b) change the guest code to not loop through all cpus
>>>> c) flush the tlb cache on all vcpus from the hc handler
>>> 
>>> It's currently only our internal tree that does it from early_init (as
>>> part of the idle paravirt patch, to avoid races -- though I can't
>>> recall now what the problematic race is there).  It should have been
>>> changed for the SPRG4-7 paravirt as well.  We don't want a secondary
>>> CPU to take an exception and save something into a paravirt SPRG, but
>>> read from the hardware SPRG, due to the patching being incomplete.
>>> 
>>> An alternative would be to still do it after secondaries are up, but
>>> instead of just doing the hcall in kvm_map_magic_page, all but one cpu
>>> would be held in a loop with interrupts off until the patching is complete.
>> 
>> That sounds good. Then they can all do the hcall themselves and contine running.
> 
> Why do the secondaries need to spin...can they just make the call
> as the very first thing when coming out of the spin table?
> 
> Just let the boot CPU do the patching before releasing
> the secondaries.

That is very subarch-specific, so we'd have to treat e500 different from 440 different from book3s_32 different from book3s_64 I suppose.
If you want to go through that exercise, it might be worth it. The overall thing would be easier then at the end of the day - except for the startup code for secondaries.


Alex

RE: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Yoder Stuart-B08248]

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de]
> Sent: Tuesday, November 22, 2011 3:28 PM
> To: Yoder Stuart-B08248
> Cc: Wood Scott-B07421; Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> 
> 
> On 22.11.2011, at 22:11, Yoder Stuart-B08248 wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: kvm-ppc-owner@vger.kernel.org
> >> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Alexander Graf
> >> Sent: Tuesday, November 22, 2011 2:45 PM
> >> To: Wood Scott-B07421
> >> Cc: Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
> >> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> >>
> >>
> >> On 22.11.2011, at 19:36, Scott Wood <scottwood@freescale.com> wrote:
> >>
> >>> On 11/22/2011 05:27 AM, Alexander Graf wrote:
> >>>>
> >>>>
> >>>>
> >>>>
> >>>> On 22.11.2011, at 12:19, Liu Yu-B13201 <B13201@freescale.com> wrote:
> >>>>
> >>>>>
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Alexander Graf [mailto:agraf@suse.de]
> >>>>>> Sent: Tuesday, November 22, 2011 7:14 PM
> >>>>>> To: Liu Yu-B13201
> >>>>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
> >>>>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> >>>>>>
> >>>>>>
> >>>>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
> >>>>>>
> >>>>>>> Previously, only primary vcpu get enabled paravirt.
> >>>>>>
> >>>>>> Please fix it the other way around. Thd hypercall is CPU local
> >>>>>> and should stay that way, so we have to call it on each vcpu
> >>>>>> inside the guest.
> >>>>>>
> >>>>>
> >>>>> The guest kernel already use on_each_cpu() But seems it doesn't
> >>>>> work.
> >>>>> The place primary cpu do hypercall is still in early_init where
> >>>>> secondary cpus don't get kicked.
> >>>>
> >>>> Ouch. Then let's go with this approach and
> >>>>
> >>>> a) update the hypercall documentation
> >>>> b) change the guest code to not loop through all cpus
> >>>> c) flush the tlb cache on all vcpus from the hc handler
> >>>
> >>> It's currently only our internal tree that does it from early_init
> >>> (as part of the idle paravirt patch, to avoid races -- though I
> >>> can't recall now what the problematic race is there).  It should
> >>> have been changed for the SPRG4-7 paravirt as well.  We don't want a
> >>> secondary CPU to take an exception and save something into a
> >>> paravirt SPRG, but read from the hardware SPRG, due to the patching being incomplete.
> >>>
> >>> An alternative would be to still do it after secondaries are up, but
> >>> instead of just doing the hcall in kvm_map_magic_page, all but one
> >>> cpu would be held in a loop with interrupts off until the patching is complete.
> >>
> >> That sounds good. Then they can all do the hcall themselves and contine running.
> >
> > Why do the secondaries need to spin...can they just make the call as
> > the very first thing when coming out of the spin table?
> >
> > Just let the boot CPU do the patching before releasing the
> > secondaries.
> 
> That is very subarch-specific, so we'd have to treat e500 different from 440 different from
> book3s_32 different from book3s_64 I suppose.
> If you want to go through that exercise, it might be worth it. The overall thing would be
> easier then at the end of the day - except for the startup code for secondaries.

Hmm...not sure which approach makes sense.

So to make sure I understand Scott's proposal the sequence would
be something like:

   -after secondaries CPUs are released from spin and can
    determine they are running under KVM they:
      -disable interrupts
      -set a flag indicating they are waiting for paravirt
       patching to complete
      -spin in a loop until a flag is set that indicates
       paravirt patch is complete
      -make the paravirt hcall

  -boot CPU
      -waits for all secondaries to set the 'waiting for 
       paravirt' flag
      -makes the paravirt hcall
      -patches the kernel
      -sets 'paravirt complete' flag

Is that basically correct?

Stuart

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Scott Wood]

On 11/22/2011 03:45 PM, Yoder Stuart-B08248 wrote:
> Hmm...not sure which approach makes sense.
> 
> So to make sure I understand Scott's proposal the sequence would
> be something like:
> 
>    -after secondaries CPUs are released from spin and can
>     determine they are running under KVM they:
>       -disable interrupts
>       -set a flag indicating they are waiting for paravirt
>        patching to complete
>       -spin in a loop until a flag is set that indicates
>        paravirt patch is complete
>       -make the paravirt hcall
> 
>   -boot CPU
>       -waits for all secondaries to set the 'waiting for 
>        paravirt' flag
>       -makes the paravirt hcall
>       -patches the kernel
>       -sets 'paravirt complete' flag
> 
> Is that basically correct?

That's not my proposal.

My proposal is just to add something like this at the end of
kvm_map_magic_page():

if (not the cpu doing the patching) {
	while (!kvm_patching_done)
		cpu_relax();
}

And even that is only really worth it if there are races besides SPRG3-7
to worry about.  Using the privileged version of SPRG3-7 inside the
kernel would avoid needing anything special for KVM, and would make SPRG
access slightly simpler than they are now (no separate W/R versions to
keep track of) -- I'm not sure why the kernel bothers with the user-read
version of the SPRs in the first place.

-Scott

RE: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Liu Yu-B13201]

 

> -----Original Message-----
> From: Alexander Graf [mailto:agraf@suse.de] 
> Sent: Wednesday, November 23, 2011 5:28 AM
> To: Yoder Stuart-B08248
> Cc: Wood Scott-B07421; Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> 
> 
> On 22.11.2011, at 22:11, Yoder Stuart-B08248 wrote:
> 
> > 
> > 
> >> -----Original Message-----
> >> From: kvm-ppc-owner@vger.kernel.org 
> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of
> >> Alexander Graf
> >> Sent: Tuesday, November 22, 2011 2:45 PM
> >> To: Wood Scott-B07421
> >> Cc: Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
> >> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> >> 
> >> 
> >> On 22.11.2011, at 19:36, Scott Wood 
> <scottwood@freescale.com> wrote:
> >> 
> >>> On 11/22/2011 05:27 AM, Alexander Graf wrote:
> >>>> 
> >>>> 
> >>>> 
> >>>> 
> >>>> On 22.11.2011, at 12:19, Liu Yu-B13201 
> <B13201@freescale.com> wrote:
> >>>> 
> >>>>> 
> >>>>> 
> >>>>>> -----Original Message-----
> >>>>>> From: Alexander Graf [mailto:agraf@suse.de]
> >>>>>> Sent: Tuesday, November 22, 2011 7:14 PM
> >>>>>> To: Liu Yu-B13201
> >>>>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
> >>>>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
> >>>>>> 
> >>>>>> 
> >>>>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
> >>>>>> 
> >>>>>>> Previously, only primary vcpu get enabled paravirt.
> >>>>>> 
> >>>>>> Please fix it the other way around. Thd hypercall is 
> CPU local and
> >>>>>> should stay that way, so we have to call it on each 
> vcpu inside the
> >>>>>> guest.
> >>>>>> 
> >>>>> 
> >>>>> The guest kernel already use on_each_cpu() But seems it doesn't
> >>>>> work.
> >>>>> The place primary cpu do hypercall is still in early_init where
> >>>>> secondary cpus don't get kicked.
> >>>> 
> >>>> Ouch. Then let's go with this approach and
> >>>> 
> >>>> a) update the hypercall documentation
> >>>> b) change the guest code to not loop through all cpus
> >>>> c) flush the tlb cache on all vcpus from the hc handler
> >>> 
> >>> It's currently only our internal tree that does it from 
> early_init (as
> >>> part of the idle paravirt patch, to avoid races -- though I can't
> >>> recall now what the problematic race is there).  It 
> should have been
> >>> changed for the SPRG4-7 paravirt as well.  We don't want 
> a secondary
> >>> CPU to take an exception and save something into a 
> paravirt SPRG, but
> >>> read from the hardware SPRG, due to the patching being incomplete.
> >>> 
> >>> An alternative would be to still do it after secondaries 
> are up, but
> >>> instead of just doing the hcall in kvm_map_magic_page, 
> all but one cpu
> >>> would be held in a loop with interrupts off until the 
> patching is complete.
> >> 
> >> That sounds good. Then they can all do the hcall 
> themselves and contine running.
> > 
> > Why do the secondaries need to spin...can they just make the call
> > as the very first thing when coming out of the spin table?
> > 
> > Just let the boot CPU do the patching before releasing
> > the secondaries.
> 
> That is very subarch-specific, so we'd have to treat e500 
> different from 440 different from book3s_32 different from 
> book3s_64 I suppose.
> If you want to go through that exercise, it might be worth 
> it. The overall thing would be easier then at the end of the 
> day - except for the startup code for secondaries.
> 

I'm still worried that the spin may affect host and other guests and give users a bad experience.
Why the method like this patch would be very subarch-specific?

Thanks,
Yu

Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu

[Alexander Graf]

On 25.11.2011, at 07:52, Liu Yu-B13201 <B13201@freescale.com> wrote:

> 
> 
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf@suse.de] 
>> Sent: Wednesday, November 23, 2011 5:28 AM
>> To: Yoder Stuart-B08248
>> Cc: Wood Scott-B07421; Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>> 
>> 
>> On 22.11.2011, at 22:11, Yoder Stuart-B08248 wrote:
>> 
>>> 
>>> 
>>>> -----Original Message-----
>>>> From: kvm-ppc-owner@vger.kernel.org 
>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of
>>>> Alexander Graf
>>>> Sent: Tuesday, November 22, 2011 2:45 PM
>>>> To: Wood Scott-B07421
>>>> Cc: Liu Yu-B13201; <kvm-ppc@vger.kernel.org>
>>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>>>> 
>>>> 
>>>> On 22.11.2011, at 19:36, Scott Wood 
>> <scottwood@freescale.com> wrote:
>>>> 
>>>>> On 11/22/2011 05:27 AM, Alexander Graf wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On 22.11.2011, at 12:19, Liu Yu-B13201 
>> <B13201@freescale.com> wrote:
>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>>> -----Original Message-----
>>>>>>>> From: Alexander Graf [mailto:agraf@suse.de]
>>>>>>>> Sent: Tuesday, November 22, 2011 7:14 PM
>>>>>>>> To: Liu Yu-B13201
>>>>>>>> Cc: <kvm-ppc@vger.kernel.org>; Liu Yu-B13201
>>>>>>>> Subject: Re: [PATCH] KVM: PPC: Apply paravirt to all vcpu
>>>>>>>> 
>>>>>>>> 
>>>>>>>> On 22.11.2011, at 10:55, Liu Yu <yu.liu@freescale.com> wrote:
>>>>>>>> 
>>>>>>>>> Previously, only primary vcpu get enabled paravirt.
>>>>>>>> 
>>>>>>>> Please fix it the other way around. Thd hypercall is 
>> CPU local and
>>>>>>>> should stay that way, so we have to call it on each 
>> vcpu inside the
>>>>>>>> guest.
>>>>>>>> 
>>>>>>> 
>>>>>>> The guest kernel already use on_each_cpu() But seems it doesn't
>>>>>>> work.
>>>>>>> The place primary cpu do hypercall is still in early_init where
>>>>>>> secondary cpus don't get kicked.
>>>>>> 
>>>>>> Ouch. Then let's go with this approach and
>>>>>> 
>>>>>> a) update the hypercall documentation
>>>>>> b) change the guest code to not loop through all cpus
>>>>>> c) flush the tlb cache on all vcpus from the hc handler
>>>>> 
>>>>> It's currently only our internal tree that does it from 
>> early_init (as
>>>>> part of the idle paravirt patch, to avoid races -- though I can't
>>>>> recall now what the problematic race is there).  It 
>> should have been
>>>>> changed for the SPRG4-7 paravirt as well.  We don't want 
>> a secondary
>>>>> CPU to take an exception and save something into a 
>> paravirt SPRG, but
>>>>> read from the hardware SPRG, due to the patching being incomplete.
>>>>> 
>>>>> An alternative would be to still do it after secondaries 
>> are up, but
>>>>> instead of just doing the hcall in kvm_map_magic_page, 
>> all but one cpu
>>>>> would be held in a loop with interrupts off until the 
>> patching is complete.
>>>> 
>>>> That sounds good. Then they can all do the hcall 
>> themselves and contine running.
>>> 
>>> Why do the secondaries need to spin...can they just make the call
>>> as the very first thing when coming out of the spin table?
>>> 
>>> Just let the boot CPU do the patching before releasing
>>> the secondaries.
>> 
>> That is very subarch-specific, so we'd have to treat e500 
>> different from 440 different from book3s_32 different from 
>> book3s_64 I suppose.
>> If you want to go through that exercise, it might be worth 
>> it. The overall thing would be easier then at the end of the 
>> day - except for the startup code for secondaries.
>> 
> 
> I'm still worried that the spin may affect host and other guests and give users a bad experience.
> Why the method like this patch would be very subarch-specific?

Because we would have to make sure that the secondaries use no magic-page accesses before doing their hypercall. That means we have to do the hypercall really early in secondary booutup code - which is subarch specific :)


Alex