[PATCH] Added one_reg interface for timer registers

[PATCH] Added one_reg interface for timer registers

[Bharat Bhushan]

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 <bharat.bhushan@freescale.com>
---
 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;
+	}
 	default:
 		break;
 	}
-- 
1.7.0.4

Re: [PATCH] Added one_reg interface for timer registers

[Scott Wood]

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 <bharat.bhushan@freescale.com>
> ---
>  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