From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH] Added one_reg interface for timer registers Date: Fri, 8 Feb 2013 14:34:58 -0600 Message-ID: <1360355698.22064.5@snotra> References: <1360317974-22775-1-git-send-email-bharat.bhushan@freescale.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: , , , Bharat Bhushan To: Bharat Bhushan Return-path: In-Reply-To: <1360317974-22775-1-git-send-email-bharat.bhushan@freescale.com> (from r65777@freescale.com on Fri Feb 8 04:06:14 2013) Content-Disposition: inline Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 02/08/2013 04:06:14 AM, Bharat Bhushan wrote: > If userspace wants to change some specific bits of TSR > (timer status register) then it uses GET/SET_SREGS ioctl interface. > So the steps will be: > i) user-space will make get ioctl, > ii) change TSR in userspace > iii) then make set ioctl. > It can happen that TSR gets changed by kernel after step i) and > before step iii). > > To avoid this we have added below one_reg ioctls for oring and > clearing > specific bits in TSR. This patch adds one registerface for: > 1) setting specific bit in TSR (timer status register) > 2) clearing specific bit in TSR (timer status register) > 3) setting the TCR register. There are cases where we want to > only > change TCR and not TSR. Although we can uses SREGS without > KVM_SREGS_E_UPDATE_TSR flag but I think one reg is better. I > am open > if someone feels we should use SREGS only here. > > Signed-off-by: Bharat Bhushan > --- > arch/powerpc/include/uapi/asm/kvm.h | 4 ++++ > arch/powerpc/kvm/booke.c | 18 ++++++++++++++++++ > 2 files changed, 22 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/include/uapi/asm/kvm.h > b/arch/powerpc/include/uapi/asm/kvm.h > index 16064d0..b4ad5d4 100644 > --- a/arch/powerpc/include/uapi/asm/kvm.h > +++ b/arch/powerpc/include/uapi/asm/kvm.h > @@ -417,4 +417,8 @@ struct kvm_get_htab_header { > #define KVM_REG_PPC_EPCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x85) > #define KVM_REG_PPC_EPR (KVM_REG_PPC | KVM_REG_SIZE_U32 > | 0x86) > > +/* Timer Status Register OR/CLEAR interface */ > +#define KVM_REG_PPC_OR_TSR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x87) > +#define KVM_REG_PPC_CLEAR_TSR (KVM_REG_PPC | KVM_REG_SIZE_U32 > | 0x88) > +#define KVM_REG_PPC_SET_TCR (KVM_REG_PPC | KVM_REG_SIZE_U32 | 0x89) > #endif /* __LINUX_KVM_POWERPC_H */ > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > index 020923e..983c06f 100644 > --- a/arch/powerpc/kvm/booke.c > +++ b/arch/powerpc/kvm/booke.c > @@ -1481,6 +1481,24 @@ int kvm_vcpu_ioctl_set_one_reg(struct kvm_vcpu > *vcpu, struct kvm_one_reg *reg) > break; > } > #endif > + case KVM_REG_PPC_OR_TSR: { > + u32 tsr_bits; > + r = get_user(tsr_bits, (u32 __user *)(long)reg->addr); > + kvmppc_set_tsr_bits(vcpu, tsr_bits); > + break; > + } > + case KVM_REG_PPC_CLEAR_TSR: { > + u32 tsr_bits; > + r = get_user(tsr_bits, (u32 __user *)(long)reg->addr); > + kvmppc_clr_tsr_bits(vcpu, tsr_bits); > + break; > + } > + case KVM_REG_PPC_SET_TCR: { > + u32 tcr; > + r = get_user(tcr, (u32 __user *)(long)reg->addr); > + kvmppc_set_tcr(vcpu, tcr); > + break; > + } KVM_REG_PPC_SET_TCR should just be KVM_REG_PPC_TCR -- we should be able to "GET" it too... Might as well add a KVM_REG_PPC_TSR as well that has normal read/write semantics (in addition to the CLEAR and OR versions), if we're going to do it for TCR. -Scott