| Message ID | 20110901230851.GA26220@schlenkerla.am.freescale.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Scott Wood wrote:
> Signed-off-by: Scott Wood<scottwood@freescale.com>
How about providing a better description?
Am 02.09.2011 um 01:08 schrieb Scott Wood <scottwood@freescale.com>: > Signed-off-by: Scott Wood <scottwood@freescale.com> Patch description missing. Also, since pir == vcpu_id now, can't we just remove pir? Alex > --- > arch/powerpc/kvm/booke.c | 4 ++-- > arch/powerpc/kvm/e500.c | 3 --- > 2 files changed, 2 insertions(+), 5 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index ee45fa0..d967faf 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -753,7 +753,7 @@ static void get_sregs_arch206(struct kvm_vcpu *vcpu, > { > sregs->u.e.features |= KVM_SREGS_E_ARCH206; > > - sregs->u.e.pir = 0; > + sregs->u.e.pir = vcpu->vcpu_id; > sregs->u.e.mcsrr0 = vcpu->arch.mcsrr0; > sregs->u.e.mcsrr1 = vcpu->arch.mcsrr1; > sregs->u.e.decar = vcpu->arch.decar; > @@ -766,7 +766,7 @@ static int set_sregs_arch206(struct kvm_vcpu *vcpu, > if (!(sregs->u.e.features & KVM_SREGS_E_ARCH206)) > return 0; > > - if (sregs->u.e.pir != 0) > + if (sregs->u.e.pir != vcpu->vcpu_id) > return -EINVAL; > > vcpu->arch.mcsrr0 = sregs->u.e.mcsrr0; > diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c > index b8f065c..e8f5ec2 100644 > --- a/arch/powerpc/kvm/e500.c > +++ b/arch/powerpc/kvm/e500.c > @@ -70,9 +70,6 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) > vcpu->arch.pvr = mfspr(SPRN_PVR); > vcpu_e500->svr = mfspr(SPRN_SVR); > > - /* Since booke kvm only support one core, update all vcpus' PIR to 0 */ > - vcpu->vcpu_id = 0; > - > return 0; > } > > -- > 1.7.6 > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/02/2011 10:12 AM, Alexander Graf wrote: > > Am 02.09.2011 um 01:08 schrieb Scott Wood <scottwood@freescale.com>: > >> Signed-off-by: Scott Wood <scottwood@freescale.com> > > Patch description missing. It's not missing, it's just brief. :-) I suppose you could add "The hardcoded behavior prevents SMP support. QEMU shall specify the vcpu's PIR as the vcpu id". > Also, since pir == vcpu_id now, can't we just remove pir? From sregs? Is that worth the compatibility breakage? We could define a new bit for ARCH206 without PIR, but older QEMU would then not see the other ARCH206 stuff. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02.09.2011, at 20:14, Scott Wood wrote: > On 09/02/2011 10:12 AM, Alexander Graf wrote: >> >> Am 02.09.2011 um 01:08 schrieb Scott Wood <scottwood@freescale.com>: >> >>> Signed-off-by: Scott Wood <scottwood@freescale.com> >> >> Patch description missing. > > It's not missing, it's just brief. :-) > > I suppose you could add "The hardcoded behavior prevents SMP support. > QEMU shall specify the vcpu's PIR as the vcpu id". Ok, let me get my head around this. Before, PIR was forced to 0 by the setup code and set_sregs with PIR != 0 failed. Now it's simply vcpu_id which is already the correct value. Why didn't I run into this failure? Why did SMP work for me at all then? Shouldn't the guest be completely confused and find two CPU 0s? > >> Also, since pir == vcpu_id now, can't we just remove pir? > > From sregs? Is that worth the compatibility breakage? We could define > a new bit for ARCH206 without PIR, but older QEMU would then not see the > other ARCH206 stuff. Ugh. This is only in sregs. In KVM we already do use vcpu_id. Yes, all is fine there. I misread things as if we were having vcpu->pir :). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/02/2011 02:23 PM, Alexander Graf wrote: > > On 02.09.2011, at 20:14, Scott Wood wrote: > >> On 09/02/2011 10:12 AM, Alexander Graf wrote: >>> >>> Am 02.09.2011 um 01:08 schrieb Scott Wood <scottwood@freescale.com>: >>> >>>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>> >>> Patch description missing. >> >> It's not missing, it's just brief. :-) >> >> I suppose you could add "The hardcoded behavior prevents SMP support. >> QEMU shall specify the vcpu's PIR as the vcpu id". > > Ok, let me get my head around this. Before, PIR was forced to 0 by > the setup code and set_sregs with PIR != 0 failed. Now it's simply > vcpu_id which is already the correct value. Why didn't I run into > this failure? Why did SMP work for me at all then? Shouldn't the > guest be completely confused and find two CPU 0s? I was wondering about that myself. It looks like PIR isn't used much in Linux on e500v2. There's no msgsnd. It's used to for __secondary_hold_acknowledge, but that has a silent timeout. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02.09.2011, at 21:35, Scott Wood wrote: > On 09/02/2011 02:23 PM, Alexander Graf wrote: >> >> On 02.09.2011, at 20:14, Scott Wood wrote: >> >>> On 09/02/2011 10:12 AM, Alexander Graf wrote: >>>> >>>> Am 02.09.2011 um 01:08 schrieb Scott Wood <scottwood@freescale.com>: >>>> >>>>> Signed-off-by: Scott Wood <scottwood@freescale.com> >>>> >>>> Patch description missing. >>> >>> It's not missing, it's just brief. :-) >>> >>> I suppose you could add "The hardcoded behavior prevents SMP support. >>> QEMU shall specify the vcpu's PIR as the vcpu id". >> >> Ok, let me get my head around this. Before, PIR was forced to 0 by >> the setup code and set_sregs with PIR != 0 failed. Now it's simply >> vcpu_id which is already the correct value. Why didn't I run into >> this failure? Why did SMP work for me at all then? Shouldn't the >> guest be completely confused and find two CPU 0s? > > I was wondering about that myself. It looks like PIR isn't used much in > Linux on e500v2. There's no msgsnd. It's used to for > __secondary_hold_acknowledge, but that has a silent timeout. Interesting. Well - either way. Just resend with a proper patch description and I'll apply it :) Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index ee45fa0..d967faf 100644 --- a/arch/powerpc/kvm/booke.c +++ b/arch/powerpc/kvm/booke.c @@ -753,7 +753,7 @@ static void get_sregs_arch206(struct kvm_vcpu *vcpu, { sregs->u.e.features |= KVM_SREGS_E_ARCH206; - sregs->u.e.pir = 0; + sregs->u.e.pir = vcpu->vcpu_id; sregs->u.e.mcsrr0 = vcpu->arch.mcsrr0; sregs->u.e.mcsrr1 = vcpu->arch.mcsrr1; sregs->u.e.decar = vcpu->arch.decar; @@ -766,7 +766,7 @@ static int set_sregs_arch206(struct kvm_vcpu *vcpu, if (!(sregs->u.e.features & KVM_SREGS_E_ARCH206)) return 0; - if (sregs->u.e.pir != 0) + if (sregs->u.e.pir != vcpu->vcpu_id) return -EINVAL; vcpu->arch.mcsrr0 = sregs->u.e.mcsrr0; diff --git a/arch/powerpc/kvm/e500.c b/arch/powerpc/kvm/e500.c index b8f065c..e8f5ec2 100644 --- a/arch/powerpc/kvm/e500.c +++ b/arch/powerpc/kvm/e500.c @@ -70,9 +70,6 @@ int kvmppc_core_vcpu_setup(struct kvm_vcpu *vcpu) vcpu->arch.pvr = mfspr(SPRN_PVR); vcpu_e500->svr = mfspr(SPRN_SVR); - /* Since booke kvm only support one core, update all vcpus' PIR to 0 */ - vcpu->vcpu_id = 0; - return 0; }
Signed-off-by: Scott Wood <scottwood@freescale.com> --- arch/powerpc/kvm/booke.c | 4 ++-- arch/powerpc/kvm/e500.c | 3 --- 2 files changed, 2 insertions(+), 5 deletions(-)