Re: [Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers

2014-04-04 Thread Alexey Kardashevskiy 
On 04/04/2014 06:12 AM, Tom Musta wrote:
 On 4/3/2014 8:33 AM, Alexander Graf wrote:

 On 03.04.14 15:14, Alexey Kardashevskiy wrote:
 This enabled KVM and migration support for a number of POWER8 registers:
 
 snip
 

 Tom, please have a look through this as well :).

 ---
 
 snip
 
 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -426,6 +426,8 @@ struct ppc_slb_t {
   #define MSR_TAG  62 /* Tag-active mode (POWERx ?) 
*/
   #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on 630   
*/
   #define MSR_SHV  60 /* hypervisor state   
 hflags */
 +#define MSR_TS   33 /* Transactional state, 2 bits (Book3s)
   */

 2 bits means you want to add another define or at least a comment at bit 34. 
 I find it rather counterintuitive to declare bit 33 MSR_TS as 2-bit wide too 
 - you'd expect (3  MSR_TS) gives you the right mask, but then it'd have to 
 be 34, no?
 
 Is this better?
 
 #define MSR_TS0 34
 #define MSR_TS1 33
 
 You should also add a decoder:
 
 #define msr_ts ((env-msr  MSR_TS1)  3)

Yes, this is better. Thanks!



 +target_ulong tm_gpr[32];
 +ppc_avr_t tm_vsr[64];
 +uint64_t tm_cr;
 +uint64_t tm_lr;
 +uint64_t tm_ctr;
 +uint64_t tm_fpscr;
 +uint64_t tm_amr;
 +uint64_t tm_ppr;
 +uint64_t tm_vrsave;
 +uint32_t tm_vscr;
 +uint64_t tm_dscr;
 +uint64_t tm_tar;
   };
 
 If vscr is declared as 32 bits, should CR and VRSAVE also be 32-bits?

Nope.

linux-headers/asm-powerpc/kvm.h:
#define KVM_REG_PPC_TM_CR   (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x60)
#define KVM_REG_PPC_TM_LR   (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x61)
#define KVM_REG_PPC_TM_CTR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x62)
#define KVM_REG_PPC_TM_FPSCR(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x63)
#define KVM_REG_PPC_TM_AMR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x64)
#define KVM_REG_PPC_TM_PPR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x65)
#define KVM_REG_PPC_TM_VRSAVE   (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x66)
#define KVM_REG_PPC_TM_VSCR (KVM_REG_PPC_TM | KVM_REG_SIZE_U32 | 0x67)
#define KVM_REG_PPC_TM_DSCR (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x68)
#define KVM_REG_PPC_TM_TAR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x69)

For the reason unknown (Paul is not in the office to ask) all TM-related
registers are defined as 64bit and only VSCR is 32bit. And this is in the
host kernel already.


 diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
 index 9974b10..ead69fa 100644
 --- a/target-ppc/kvm.c
 +++ b/target-ppc/kvm.c
 @@ -866,6 +866,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
   }
 #ifdef TARGET_PPC64
 +if ((cpu-env.msr  MSR_TS)  3) {

 Ah, it works because you're shifting the other direction. That works. How 
 about we just introduce an msr_ts() helper similar to the other lower case 
 helpers to make this obvious?

 
 Agreed.
 
 
 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 1627bb0..4b20c29 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -7025,14 +7025,22 @@ static void init_proc_POWER7 (CPUPPCState *env)
SPR_NOACCESS, SPR_NOACCESS,
spr_read_generic, SPR_NOACCESS,
0x8080);
 -spr_register(env, SPR_VRSAVE, SPR_VRSAVE,
 - spr_read_generic, spr_write_generic,
 - spr_read_generic, spr_write_generic,
 - 0x);
 -spr_register(env, SPR_PPR, PPR,
 - spr_read_generic, spr_write_generic,
 - spr_read_generic, spr_write_generic,
 - 0x);
 +spr_register_kvm(env, SPR_VRSAVE, VRSAVE,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_VRSAVE, 0x);
 +spr_register_kvm(env, SPR_PPR, PPR,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_PPR, 0x);
 +spr_register_kvm(env, SPR_BOOK3S_SIAR, SIAR,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_SIAR, 0x);
 +spr_register_kvm(env, SPR_BOOK3S_SDAR, SDAR,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_SDAR, 0x);
   /* Logical partitionning */
   spr_register_kvm(env, SPR_LPCR, LPCR,
SPR_NOACCESS, SPR_NOACCESS,

 
 These need to go into P8 as well? (see my comment for patch 2).

Yes. VRSAVE, SIAR, SDAR are even defined for 970 (Alex, should I add them
to 970 definitions?), PPR is not defined in any 970 spec but is in 2.04..2.07.


-- 
Alexey

Re: [Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers

2014-04-04 Thread Alexander Graf 

On 04/04/2014 08:58 AM, Alexey Kardashevskiy wrote:

On 04/04/2014 06:12 AM, Tom Musta wrote:

On 4/3/2014 8:33 AM, Alexander Graf wrote:

On 03.04.14 15:14, Alexey Kardashevskiy wrote:

This enabled KVM and migration support for a number of POWER8 registers:

snip


Tom, please have a look through this as well :).


---

snip


--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -426,6 +426,8 @@ struct ppc_slb_t {
   #define MSR_TAG  62 /* Tag-active mode (POWERx ?)
*/
   #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on 630  
*/
   #define MSR_SHV  60 /* hypervisor state   hflags 
*/
+#define MSR_TS   33 /* Transactional state, 2 bits (Book3s)  */

2 bits means you want to add another define or at least a comment at bit 34. I find 
it rather counterintuitive to declare bit 33 MSR_TS as 2-bit wide too - you'd expect 
(3  MSR_TS) gives you the right mask, but then it'd have to be 34, no?

Is this better?

 #define MSR_TS0 34
 #define MSR_TS1 33

You should also add a decoder:

 #define msr_ts ((env-msr  MSR_TS1)  3)

Yes, this is better. Thanks!



+target_ulong tm_gpr[32];
+ppc_avr_t tm_vsr[64];
+uint64_t tm_cr;
+uint64_t tm_lr;
+uint64_t tm_ctr;
+uint64_t tm_fpscr;
+uint64_t tm_amr;
+uint64_t tm_ppr;
+uint64_t tm_vrsave;
+uint32_t tm_vscr;
+uint64_t tm_dscr;
+uint64_t tm_tar;
   };

If vscr is declared as 32 bits, should CR and VRSAVE also be 32-bits?

Nope.

linux-headers/asm-powerpc/kvm.h:
#define KVM_REG_PPC_TM_CR   (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x60)
#define KVM_REG_PPC_TM_LR   (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x61)
#define KVM_REG_PPC_TM_CTR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x62)
#define KVM_REG_PPC_TM_FPSCR(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x63)
#define KVM_REG_PPC_TM_AMR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x64)
#define KVM_REG_PPC_TM_PPR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x65)
#define KVM_REG_PPC_TM_VRSAVE   (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x66)
#define KVM_REG_PPC_TM_VSCR (KVM_REG_PPC_TM | KVM_REG_SIZE_U32 | 0x67)
#define KVM_REG_PPC_TM_DSCR (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x68)
#define KVM_REG_PPC_TM_TAR  (KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x69)

For the reason unknown (Paul is not in the office to ask) all TM-related
registers are defined as 64bit and only VSCR is 32bit. And this is in the
host kernel already.



diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 9974b10..ead69fa 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -866,6 +866,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
   }
 #ifdef TARGET_PPC64
+if ((cpu-env.msr  MSR_TS)  3) {

Ah, it works because you're shifting the other direction. That works. How about 
we just introduce an msr_ts() helper similar to the other lower case helpers to 
make this obvious?


Agreed.



diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
index 1627bb0..4b20c29 100644
--- a/target-ppc/translate_init.c
+++ b/target-ppc/translate_init.c
@@ -7025,14 +7025,22 @@ static void init_proc_POWER7 (CPUPPCState *env)
SPR_NOACCESS, SPR_NOACCESS,
spr_read_generic, SPR_NOACCESS,
0x8080);
-spr_register(env, SPR_VRSAVE, SPR_VRSAVE,
- spr_read_generic, spr_write_generic,
- spr_read_generic, spr_write_generic,
- 0x);
-spr_register(env, SPR_PPR, PPR,
- spr_read_generic, spr_write_generic,
- spr_read_generic, spr_write_generic,
- 0x);
+spr_register_kvm(env, SPR_VRSAVE, VRSAVE,
+ spr_read_generic, spr_write_generic,
+ spr_read_generic, spr_write_generic,
+ KVM_REG_PPC_VRSAVE, 0x);
+spr_register_kvm(env, SPR_PPR, PPR,
+ spr_read_generic, spr_write_generic,
+ spr_read_generic, spr_write_generic,
+ KVM_REG_PPC_PPR, 0x);
+spr_register_kvm(env, SPR_BOOK3S_SIAR, SIAR,
+ spr_read_generic, spr_write_generic,
+ spr_read_generic, spr_write_generic,
+ KVM_REG_PPC_SIAR, 0x);
+spr_register_kvm(env, SPR_BOOK3S_SDAR, SDAR,
+ spr_read_generic, spr_write_generic,
+ spr_read_generic, spr_write_generic,
+ KVM_REG_PPC_SDAR, 0x);
   /* Logical partitionning */
   spr_register_kvm(env, SPR_LPCR, LPCR,
SPR_NOACCESS, SPR_NOACCESS,

These need to go into P8 as well? (see my comment for patch 2).

Yes. VRSAVE, SIAR, SDAR are even defined for 970 (Alex, should I add them
to 970 definitions?), PPR is not defined in any 970 spec but is in 2.04..2.07.


Anything that 970 can work with should be handled in the 970 case as 
well, yes. Keep in mind that we

Re: [Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers

2014-04-03 Thread Alexander Graf 


On 03.04.14 15:14, Alexey Kardashevskiy wrote:

This enabled KVM and migration support for a number of POWER8 registers:
* Program Prioirty Register (PPR)


Typo


* Sampled Instruction Address Register (SIAR)
* Sampled Data Address Register (SDAR)
* Vector Registers Save Register (VRSAVE)

This enables save/restore of transactional state if MSR_TS is set
in the MSR.

This adds new POWER8 registers support required for migration.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru


Tom, please have a look through this as well :).


---
  target-ppc/cpu.h| 18 ++
  target-ppc/kvm.c| 38 ++
  target-ppc/machine.c| 34 ++
  target-ppc/translate_init.c | 24 
  4 files changed, 106 insertions(+), 8 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2719c08..2b476b4 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -426,6 +426,8 @@ struct ppc_slb_t {
  #define MSR_TAG  62 /* Tag-active mode (POWERx ?)
*/
  #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on 630  
*/
  #define MSR_SHV  60 /* hypervisor state   hflags 
*/
+#define MSR_TS   33 /* Transactional state, 2 bits (Book3s)  */


2 bits means you want to add another define or at least a comment at bit 
34. I find it rather counterintuitive to declare bit 33 MSR_TS as 2-bit 
wide too - you'd expect (3  MSR_TS) gives you the right mask, but then 
it'd have to be 34, no?



+#define MSR_TM   32 /* Transactional Memory Available (Book3s)   */
  #define MSR_CM   31 /* Computation mode for BookE hflags 
*/
  #define MSR_ICM  30 /* Interrupt computation mode for BookE  
*/
  #define MSR_THV  29 /* hypervisor state for 32 bits PowerPC   hflags 
*/
@@ -1081,6 +1083,20 @@ struct CPUPPCState {
   */
  uint8_t fit_period[4];
  uint8_t wdt_period[4];
+
+/* Transactional memory state migration */


No migration, we're laying the foundation for TM emulation.


+target_ulong tm_gpr[32];
+ppc_avr_t tm_vsr[64];
+uint64_t tm_cr;
+uint64_t tm_lr;
+uint64_t tm_ctr;
+uint64_t tm_fpscr;
+uint64_t tm_amr;
+uint64_t tm_ppr;
+uint64_t tm_vrsave;
+uint32_t tm_vscr;
+uint64_t tm_dscr;
+uint64_t tm_tar;
  };
  
  #define SET_FIT_PERIOD(a_, b_, c_, d_)  \

@@ -1479,6 +1495,8 @@ static inline int cpu_mmu_index (CPUPPCState *env)
  #define SPR_MPC_MD_EPN(0x30B)
  #define SPR_PERFC (0x30C)
  #define SPR_MPC_MD_TWB(0x30C)
+#define SPR_BOOK3S_SIAR   (0x30C)
+#define SPR_BOOK3S_SDAR   (0x30D)
  #define SPR_PERFD (0x30D)
  #define SPR_MPC_MD_TWC(0x30D)
  #define SPR_PERFE (0x30E)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 9974b10..ead69fa 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -866,6 +866,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
  }
  
  #ifdef TARGET_PPC64

+if ((cpu-env.msr  MSR_TS)  3) {


Ah, it works because you're shifting the other direction. That works. 
How about we just introduce an msr_ts() helper similar to the other 
lower case helpers to make this obvious?



+for (i = 0; i  sizeof(env-tm_gpr)/sizeof(env-tm_gpr[0]); i++) {


ARRAY_SIZE?


Alex


+kvm_set_one_reg(cs, KVM_REG_PPC_TM_GPR(i), env-tm_gpr[i]);
+}
+for (i = 0; i  sizeof(env-tm_vsr)/sizeof(env-tm_vsr[0]); i++) {
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_VSR(i), env-tm_vsr[i]);
+}
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_CR, env-tm_cr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_LR, env-tm_lr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_CTR, env-tm_ctr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_FPSCR, env-tm_fpscr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_AMR, env-tm_amr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_PPR, env-tm_ppr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_VRSAVE, env-tm_vrsave);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_VSCR, env-tm_vscr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_DSCR, env-tm_dscr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_TAR, env-tm_tar);
+}
+
  if (cap_papr) {
  if (kvm_put_vpa(cs)  0) {
  DPRINTF(Warning: Unable to set VPA information to KVM\n);
@@ -1090,6 +1109,25 @@ int kvm_arch_get_registers(CPUState *cs)
  }
  
  #ifdef TARGET_PPC64

+if ((cpu-env.msr  MSR_TS)  3) {
+for (i = 0; i  sizeof(env-tm_gpr)/sizeof(env-tm_gpr[0]); i++) {
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_GPR(i), env-tm_gpr[i]);
+}
+for (i = 0; i  sizeof(env-tm_vsr)/sizeof(env-tm_vsr[0]); i++) {
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_VSR(i),

[Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers

2014-04-03 Thread Alexey Kardashevskiy 
This enabled KVM and migration support for a number of POWER8 registers:
* Program Prioirty Register (PPR)
* Sampled Instruction Address Register (SIAR)
* Sampled Data Address Register (SDAR)
* Vector Registers Save Register (VRSAVE)

This enables save/restore of transactional state if MSR_TS is set
in the MSR.

This adds new POWER8 registers support required for migration.

Signed-off-by: Alexey Kardashevskiy a...@ozlabs.ru
---
 target-ppc/cpu.h| 18 ++
 target-ppc/kvm.c| 38 ++
 target-ppc/machine.c| 34 ++
 target-ppc/translate_init.c | 24 
 4 files changed, 106 insertions(+), 8 deletions(-)

diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
index 2719c08..2b476b4 100644
--- a/target-ppc/cpu.h
+++ b/target-ppc/cpu.h
@@ -426,6 +426,8 @@ struct ppc_slb_t {
 #define MSR_TAG  62 /* Tag-active mode (POWERx ?)*/
 #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on 630  */
 #define MSR_SHV  60 /* hypervisor state   hflags */
+#define MSR_TS   33 /* Transactional state, 2 bits (Book3s)  */
+#define MSR_TM   32 /* Transactional Memory Available (Book3s)   */
 #define MSR_CM   31 /* Computation mode for BookE hflags */
 #define MSR_ICM  30 /* Interrupt computation mode for BookE  */
 #define MSR_THV  29 /* hypervisor state for 32 bits PowerPC   hflags */
@@ -1081,6 +1083,20 @@ struct CPUPPCState {
  */
 uint8_t fit_period[4];
 uint8_t wdt_period[4];
+
+/* Transactional memory state migration */
+target_ulong tm_gpr[32];
+ppc_avr_t tm_vsr[64];
+uint64_t tm_cr;
+uint64_t tm_lr;
+uint64_t tm_ctr;
+uint64_t tm_fpscr;
+uint64_t tm_amr;
+uint64_t tm_ppr;
+uint64_t tm_vrsave;
+uint32_t tm_vscr;
+uint64_t tm_dscr;
+uint64_t tm_tar;
 };
 
 #define SET_FIT_PERIOD(a_, b_, c_, d_)  \
@@ -1479,6 +1495,8 @@ static inline int cpu_mmu_index (CPUPPCState *env)
 #define SPR_MPC_MD_EPN(0x30B)
 #define SPR_PERFC (0x30C)
 #define SPR_MPC_MD_TWB(0x30C)
+#define SPR_BOOK3S_SIAR   (0x30C)
+#define SPR_BOOK3S_SDAR   (0x30D)
 #define SPR_PERFD (0x30D)
 #define SPR_MPC_MD_TWC(0x30D)
 #define SPR_PERFE (0x30E)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 9974b10..ead69fa 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -866,6 +866,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
 }
 
 #ifdef TARGET_PPC64
+if ((cpu-env.msr  MSR_TS)  3) {
+for (i = 0; i  sizeof(env-tm_gpr)/sizeof(env-tm_gpr[0]); i++) {
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_GPR(i), env-tm_gpr[i]);
+}
+for (i = 0; i  sizeof(env-tm_vsr)/sizeof(env-tm_vsr[0]); i++) {
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_VSR(i), env-tm_vsr[i]);
+}
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_CR, env-tm_cr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_LR, env-tm_lr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_CTR, env-tm_ctr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_FPSCR, env-tm_fpscr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_AMR, env-tm_amr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_PPR, env-tm_ppr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_VRSAVE, env-tm_vrsave);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_VSCR, env-tm_vscr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_DSCR, env-tm_dscr);
+kvm_set_one_reg(cs, KVM_REG_PPC_TM_TAR, env-tm_tar);
+}
+
 if (cap_papr) {
 if (kvm_put_vpa(cs)  0) {
 DPRINTF(Warning: Unable to set VPA information to KVM\n);
@@ -1090,6 +1109,25 @@ int kvm_arch_get_registers(CPUState *cs)
 }
 
 #ifdef TARGET_PPC64
+if ((cpu-env.msr  MSR_TS)  3) {
+for (i = 0; i  sizeof(env-tm_gpr)/sizeof(env-tm_gpr[0]); i++) {
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_GPR(i), env-tm_gpr[i]);
+}
+for (i = 0; i  sizeof(env-tm_vsr)/sizeof(env-tm_vsr[0]); i++) {
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_VSR(i), env-tm_vsr[i]);
+}
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_CR, env-tm_cr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_LR, env-tm_lr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_CTR, env-tm_ctr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_FPSCR, env-tm_fpscr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_AMR, env-tm_amr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_PPR, env-tm_ppr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_VRSAVE, env-tm_vrsave);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_VSCR, env-tm_vscr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_DSCR, env-tm_dscr);
+kvm_get_one_reg(cs, KVM_REG_PPC_TM_TAR,

Re: [Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers

2014-04-03 Thread Tom Musta 
On 4/3/2014 8:33 AM, Alexander Graf wrote:
 
 On 03.04.14 15:14, Alexey Kardashevskiy wrote:
 This enabled KVM and migration support for a number of POWER8 registers:

snip

 
 Tom, please have a look through this as well :).
 
 ---

snip

 --- a/target-ppc/cpu.h
 +++ b/target-ppc/cpu.h
 @@ -426,6 +426,8 @@ struct ppc_slb_t {
   #define MSR_TAG  62 /* Tag-active mode (POWERx ?)  
   */
   #define MSR_ISF  61 /* Sixty-four-bit interrupt mode on 630
   */
   #define MSR_SHV  60 /* hypervisor state   
 hflags */
 +#define MSR_TS   33 /* Transactional state, 2 bits (Book3s) 
  */
 
 2 bits means you want to add another define or at least a comment at bit 34. 
 I find it rather counterintuitive to declare bit 33 MSR_TS as 2-bit wide too 
 - you'd expect (3  MSR_TS) gives you the right mask, but then it'd have to 
 be 34, no?

Is this better?

#define MSR_TS0 34
#define MSR_TS1 33

You should also add a decoder:

#define msr_ts ((env-msr  MSR_TS1)  3)

 
 +target_ulong tm_gpr[32];
 +ppc_avr_t tm_vsr[64];
 +uint64_t tm_cr;
 +uint64_t tm_lr;
 +uint64_t tm_ctr;
 +uint64_t tm_fpscr;
 +uint64_t tm_amr;
 +uint64_t tm_ppr;
 +uint64_t tm_vrsave;
 +uint32_t tm_vscr;
 +uint64_t tm_dscr;
 +uint64_t tm_tar;
   };

If vscr is declared as 32 bits, should CR and VRSAVE also be 32-bits?

 diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
 index 9974b10..ead69fa 100644
 --- a/target-ppc/kvm.c
 +++ b/target-ppc/kvm.c
 @@ -866,6 +866,25 @@ int kvm_arch_put_registers(CPUState *cs, int level)
   }
 #ifdef TARGET_PPC64
 +if ((cpu-env.msr  MSR_TS)  3) {
 
 Ah, it works because you're shifting the other direction. That works. How 
 about we just introduce an msr_ts() helper similar to the other lower case 
 helpers to make this obvious?
 

Agreed.


 diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
 index 1627bb0..4b20c29 100644
 --- a/target-ppc/translate_init.c
 +++ b/target-ppc/translate_init.c
 @@ -7025,14 +7025,22 @@ static void init_proc_POWER7 (CPUPPCState *env)
SPR_NOACCESS, SPR_NOACCESS,
spr_read_generic, SPR_NOACCESS,
0x8080);
 -spr_register(env, SPR_VRSAVE, SPR_VRSAVE,
 - spr_read_generic, spr_write_generic,
 - spr_read_generic, spr_write_generic,
 - 0x);
 -spr_register(env, SPR_PPR, PPR,
 - spr_read_generic, spr_write_generic,
 - spr_read_generic, spr_write_generic,
 - 0x);
 +spr_register_kvm(env, SPR_VRSAVE, VRSAVE,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_VRSAVE, 0x);
 +spr_register_kvm(env, SPR_PPR, PPR,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_PPR, 0x);
 +spr_register_kvm(env, SPR_BOOK3S_SIAR, SIAR,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_SIAR, 0x);
 +spr_register_kvm(env, SPR_BOOK3S_SDAR, SDAR,
 + spr_read_generic, spr_write_generic,
 + spr_read_generic, spr_write_generic,
 + KVM_REG_PPC_SDAR, 0x);
   /* Logical partitionning */
   spr_register_kvm(env, SPR_LPCR, LPCR,
SPR_NOACCESS, SPR_NOACCESS,
 

These need to go into P8 as well? (see my comment for patch 2).