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,
>> 0x80800000);
>> - spr_register(env, SPR_VRSAVE, "SPR_VRSAVE",
>> - &spr_read_generic, &spr_write_generic,
>> - &spr_read_generic, &spr_write_generic,
>> - 0x00000000);
>> - spr_register(env, SPR_PPR, "PPR",
>> - &spr_read_generic, &spr_write_generic,
>> - &spr_read_generic, &spr_write_generic,
>> - 0x00000000);
>> + spr_register_kvm(env, SPR_VRSAVE, "VRSAVE",
>> + &spr_read_generic, &spr_write_generic,
>> + &spr_read_generic, &spr_write_generic,
>> + KVM_REG_PPC_VRSAVE, 0x00000000);
>> + spr_register_kvm(env, SPR_PPR, "PPR",
>> + &spr_read_generic, &spr_write_generic,
>> + &spr_read_generic, &spr_write_generic,
>> + KVM_REG_PPC_PPR, 0x00000000);
>> + spr_register_kvm(env, SPR_BOOK3S_SIAR, "SIAR",
>> + &spr_read_generic, &spr_write_generic,
>> + &spr_read_generic, &spr_write_generic,
>> + KVM_REG_PPC_SIAR, 0x00000000);
>> + spr_register_kvm(env, SPR_BOOK3S_SDAR, "SDAR",
>> + &spr_read_generic, &spr_write_generic,
>> + &spr_read_generic, &spr_write_generic,
>> + KVM_REG_PPC_SDAR, 0x00000000);
>> /* 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).
