From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [RFC PATCH 5/6] KVM: PPC: Book3E: Add ONE_REG AltiVec support Date: Wed, 3 Jul 2013 13:31:16 -0500 Message-ID: <1372876276.8183.136@snotra> References: <300B73AA675FCE4A93EB4FC1D42459FF0A2A3592@039-SN2MPN1-013.039d.mgd.msft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Wood Scott-B07421 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" To: Caraman Mihai Claudiu-B02008 Return-path: In-Reply-To: <300B73AA675FCE4A93EB4FC1D42459FF0A2A3592@039-SN2MPN1-013.039d.mgd.msft.net> (from B02008@freescale.com on Wed Jul 3 07:11:52 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 07/03/2013 07:11:52 AM, Caraman Mihai Claudiu-B02008 wrote: > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Wednesday, June 05, 2013 1:40 AM > > To: Caraman Mihai Claudiu-B02008 > > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc- > > dev@lists.ozlabs.org; Caraman Mihai Claudiu-B02008 > > Subject: Re: [RFC PATCH 5/6] KVM: PPC: Book3E: Add ONE_REG AltiVec > > support > > > > On 06/03/2013 03:54:27 PM, Mihai Caraman wrote: > > > Add ONE_REG support for AltiVec on Book3E. > > > > > > Signed-off-by: Mihai Caraman > > > --- > > > arch/powerpc/kvm/booke.c | 32 ++++++++++++++++++++++++++++++++ > > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > > > index 01eb635..019496d 100644 > > > --- a/arch/powerpc/kvm/booke.c > > > +++ b/arch/powerpc/kvm/booke.c > > > @@ -1570,6 +1570,22 @@ int kvm_vcpu_ioctl_get_one_reg(struct > kvm_vcpu > > > *vcpu, struct kvm_one_reg *reg) > > > case KVM_REG_PPC_DEBUG_INST: > > > val = get_reg_val(reg->id, KVMPPC_INST_EHPRIV); > > > break; > > > +#ifdef CONFIG_ALTIVEC > > > + case KVM_REG_PPC_VR0 ... KVM_REG_PPC_VR31: > > > + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > > > + r = -ENXIO; > > > + break; > > > + } > > > + val.vval = vcpu->arch.vr[reg->id - KVM_REG_PPC_VR0]; > > > + break; > > > + case KVM_REG_PPC_VSCR: > > > + if (!cpu_has_feature(CPU_FTR_ALTIVEC)) { > > > + r = -ENXIO; > > > + break; > > > + } > > > + val = get_reg_val(reg->id, vcpu->arch.vscr.u[3]); > > > + break; > > > > Why u[3]? > > AltiVec PEM manual says: "The VSCR has two defined bits, the AltiVec > non-Java > mode (NJ) bit (VSCR[15]) and the AltiVec saturation (SAT) bit > (VSCR[31]); > the remaining bits are reserved." > > I think this is the reason Paul M. exposed KVM_REG_PPC_VSCR width as > 32-bit. Ugh. It's documented as a 32-bit register in the ISA, but it can only be accessed via a vector register (seems like an odd design choice, but whatever). And the kernel chose to represent it as a 128-bit vector, while KVM chose to represent it as the register (not the access thereto) is architected. It would have been nice to be consistent... At least put in a comment explaining this. -Scott