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,
>>> 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).
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
