Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
On 06/27/2012 06:41 AM, Caraman Mihai Claudiu-B02008 wrote: >> -Original Message- >> From: Wood Scott-B07421 >> Sent: Wednesday, June 27, 2012 1:35 AM >> To: Caraman Mihai Claudiu-B02008 >> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- >> d...@lists.ozlabs.org; qemu-...@nongnu.org >> Subject: Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in >> sregs >> >> On 06/25/2012 07:26 AM, Mihai Caraman wrote: >>> Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for >>> 64-bit hosts. >>> >>> Signed-off-by: Mihai Caraman >>> --- >>> arch/powerpc/kvm/booke.c | 14 ++ >>> 1 files changed, 14 insertions(+), 0 deletions(-) >>> >>> diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index >>> f9fa260..d15c4b5 100644 >>> --- a/arch/powerpc/kvm/booke.c >>> +++ b/arch/powerpc/kvm/booke.c >>> @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, >>> u64 tb = get_tb(); >>> >>> sregs->u.e.features |= KVM_SREGS_E_BASE; >>> +#ifdef CONFIG_64BIT >>> + sregs->u.e.features |= KVM_SREGS_E_64;0 #endif >>> >>> sregs->u.e.csrr0 = vcpu->arch.csrr0; >>> sregs->u.e.csrr1 = vcpu->arch.csrr1; @@ -1063,6 +1066,9 @@ static >>> void get_sregs_base(struct kvm_vcpu *vcpu, >>> sregs->u.e.dec = kvmppc_get_dec(vcpu, tb); >>> sregs->u.e.tb = tb; >>> sregs->u.e.vrsave = vcpu->arch.vrsave; >>> +#ifdef CONFIG_64BIT >>> + sregs->u.e.epcr = vcpu->arch.epcr; >>> +#endif >>> } >>> >>> static int set_sregs_base(struct kvm_vcpu *vcpu, @@ -1071,6 +1077,11 >>> @@ static int set_sregs_base(struct kvm_vcpu *vcpu, >>> if (!(sregs->u.e.features & KVM_SREGS_E_BASE)) >>> return 0; >>> >>> +#ifdef CONFIG_64BIT >>> + if (!(sregs->u.e.features & KVM_SREGS_E_64)) >>> + return 0; >>> +#endif >> >> This means that a QEMU targeting a 32-bit guest won't be able to set any >> special registers, if it sets feature bits manually rather than getting >> them from GET_SREGS. > > I had some concerns about his. I only check qemu ppc code which uses get/set > approach and I followed the BASE model. Now I see that qemu x86 set them > manually :( > Why do we care if the caller set or not BASE? BASE contains things which should be present on all booke chips. If that's not set something's wrong. None of the other feature bits are handled that way. -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
> -Original Message-
> From: Wood Scott-B07421
> Sent: Wednesday, June 27, 2012 1:35 AM
> To: Caraman Mihai Claudiu-B02008
> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc-
> d...@lists.ozlabs.org; qemu-...@nongnu.org
> Subject: Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in
> sregs
>
> On 06/25/2012 07:26 AM, Mihai Caraman wrote:
> > Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for
> > 64-bit hosts.
> >
> > Signed-off-by: Mihai Caraman
> > ---
> > arch/powerpc/kvm/booke.c | 14 ++
> > 1 files changed, 14 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index
> > f9fa260..d15c4b5 100644
> > --- a/arch/powerpc/kvm/booke.c
> > +++ b/arch/powerpc/kvm/booke.c
> > @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu,
> > u64 tb = get_tb();
> >
> > sregs->u.e.features |= KVM_SREGS_E_BASE;
> > +#ifdef CONFIG_64BIT
> > + sregs->u.e.features |= KVM_SREGS_E_64;0 #endif
> >
> > sregs->u.e.csrr0 = vcpu->arch.csrr0;
> > sregs->u.e.csrr1 = vcpu->arch.csrr1; @@ -1063,6 +1066,9 @@ static
> > void get_sregs_base(struct kvm_vcpu *vcpu,
> > sregs->u.e.dec = kvmppc_get_dec(vcpu, tb);
> > sregs->u.e.tb = tb;
> > sregs->u.e.vrsave = vcpu->arch.vrsave;
> > +#ifdef CONFIG_64BIT
> > + sregs->u.e.epcr = vcpu->arch.epcr;
> > +#endif
> > }
> >
> > static int set_sregs_base(struct kvm_vcpu *vcpu, @@ -1071,6 +1077,11
> > @@ static int set_sregs_base(struct kvm_vcpu *vcpu,
> > if (!(sregs->u.e.features & KVM_SREGS_E_BASE))
> > return 0;
> >
> > +#ifdef CONFIG_64BIT
> > + if (!(sregs->u.e.features & KVM_SREGS_E_64))
> > + return 0;
> > +#endif
>
> This means that a QEMU targeting a 32-bit guest won't be able to set any
> special registers, if it sets feature bits manually rather than getting
> them from GET_SREGS.
I had some concerns about his. I only check qemu ppc code which uses get/set
approach and I followed the BASE model. Now I see that qemu x86 set them
manually :(
Why do we care if the caller set or not BASE?
-Mike
N�r��yb�X��ǧv�^�){.n�+jir)w* jg��� �ݢj/���z�ޖ��2�ޙ&�)ߡ�a���� �G���h��j:+v���w��٥
Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
On 06/25/2012 07:26 AM, Mihai Caraman wrote: > Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs > for 64-bit hosts. > > Signed-off-by: Mihai Caraman > --- > arch/powerpc/kvm/booke.c | 14 ++ > 1 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index f9fa260..d15c4b5 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, > u64 tb = get_tb(); > > sregs->u.e.features |= KVM_SREGS_E_BASE; > +#ifdef CONFIG_64BIT > + sregs->u.e.features |= KVM_SREGS_E_64;0 > +#endif > > sregs->u.e.csrr0 = vcpu->arch.csrr0; > sregs->u.e.csrr1 = vcpu->arch.csrr1; > @@ -1063,6 +1066,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, > sregs->u.e.dec = kvmppc_get_dec(vcpu, tb); > sregs->u.e.tb = tb; > sregs->u.e.vrsave = vcpu->arch.vrsave; > +#ifdef CONFIG_64BIT > + sregs->u.e.epcr = vcpu->arch.epcr; > +#endif > } > > static int set_sregs_base(struct kvm_vcpu *vcpu, > @@ -1071,6 +1077,11 @@ static int set_sregs_base(struct kvm_vcpu *vcpu, > if (!(sregs->u.e.features & KVM_SREGS_E_BASE)) > return 0; > > +#ifdef CONFIG_64BIT > + if (!(sregs->u.e.features & KVM_SREGS_E_64)) > + return 0; > +#endif This means that a QEMU targeting a 32-bit guest won't be able to set any special registers, if it sets feature bits manually rather than getting them from GET_SREGS. This check should only qualify whether we look at sregs.u.e.epcr, not whether this function works at all. BTW, shouldn't the BASE check return an error rather than silently no-op? -Scott -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
On 06/25/2012 04:24 PM, Caraman Mihai Claudiu-B02008 wrote: >> -Original Message- >> From: Avi Kivity [mailto:a...@redhat.com] >> Sent: Monday, June 25, 2012 4:00 PM >> To: Caraman Mihai Claudiu-B02008 >> Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- >> d...@lists.ozlabs.org; qemu-...@nongnu.org >> Subject: Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in >> sregs >> >> On 06/25/2012 03:26 PM, Mihai Caraman wrote: >> > Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for >> > 64-bit hosts. >> > >> > Signed-off-by: Mihai Caraman >> > --- >> > arch/powerpc/kvm/booke.c | 14 ++ >> > 1 files changed, 14 insertions(+), 0 deletions(-) >> > >> > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index >> > f9fa260..d15c4b5 100644 >> > --- a/arch/powerpc/kvm/booke.c >> > +++ b/arch/powerpc/kvm/booke.c >> > @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, >> >u64 tb = get_tb(); >> > >> >sregs->u.e.features |= KVM_SREGS_E_BASE; >> > +#ifdef CONFIG_64BIT >> > + sregs->u.e.features |= KVM_SREGS_E_64; #endif >> > >> >> This is an ABI, but I see no trace of it in Documentation. > > The ppc sregs documentation in api.txt redirects to > arch/powerpc/include/asm/kvm.h. > Isn't this enough? I guess it's okay. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
> -Original Message- > From: Avi Kivity [mailto:a...@redhat.com] > Sent: Monday, June 25, 2012 4:00 PM > To: Caraman Mihai Claudiu-B02008 > Cc: kvm-ppc@vger.kernel.org; k...@vger.kernel.org; linuxppc- > d...@lists.ozlabs.org; qemu-...@nongnu.org > Subject: Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in > sregs > > On 06/25/2012 03:26 PM, Mihai Caraman wrote: > > Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs for > > 64-bit hosts. > > > > Signed-off-by: Mihai Caraman > > --- > > arch/powerpc/kvm/booke.c | 14 ++ > > 1 files changed, 14 insertions(+), 0 deletions(-) > > > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c index > > f9fa260..d15c4b5 100644 > > --- a/arch/powerpc/kvm/booke.c > > +++ b/arch/powerpc/kvm/booke.c > > @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, > > u64 tb = get_tb(); > > > > sregs->u.e.features |= KVM_SREGS_E_BASE; > > +#ifdef CONFIG_64BIT > > + sregs->u.e.features |= KVM_SREGS_E_64; #endif > > > > This is an ABI, but I see no trace of it in Documentation. The ppc sregs documentation in api.txt redirects to arch/powerpc/include/asm/kvm.h. Isn't this enough? Mike -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 03/17] KVM: PPC64: booke: Add EPCR support in sregs
On 06/25/2012 03:26 PM, Mihai Caraman wrote: > Add KVM_SREGS_E_64 feature and EPCR spr support in get/set sregs > for 64-bit hosts. > > Signed-off-by: Mihai Caraman > --- > arch/powerpc/kvm/booke.c | 14 ++ > 1 files changed, 14 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index f9fa260..d15c4b5 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -1052,6 +1052,9 @@ static void get_sregs_base(struct kvm_vcpu *vcpu, > u64 tb = get_tb(); > > sregs->u.e.features |= KVM_SREGS_E_BASE; > +#ifdef CONFIG_64BIT > + sregs->u.e.features |= KVM_SREGS_E_64; > +#endif > This is an ABI, but I see no trace of it in Documentation. -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
