From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: Tom Musta <tommusta@gmail.com>, Alexander Graf <agraf@suse.de>,
qemu-devel@nongnu.org
Cc: qemu-ppc@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers
Date: Fri, 04 Apr 2014 17:58:10 +1100 [thread overview]
Message-ID: <533E5802.6000900@ozlabs.ru> (raw)
In-Reply-To: <533DB2AE.6040003@gmail.com>
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
next prev parent reply other threads:[~2014-04-04 6:58 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-03 13:14 [Qemu-devel] [PATCH 0/4] power7/8 migration patches Alexey Kardashevskiy
2014-04-03 13:14 ` [Qemu-devel] [PATCH 1/4] kvm: Add set_one_reg/get_one_reg helpers Alexey Kardashevskiy
2014-05-08 12:27 ` Alexander Graf
2014-05-09 1:35 ` Alexey Kardashevskiy
2014-05-09 8:06 ` [Qemu-devel] [PATCH] kvm: make one_reg helpers available for everyone Cornelia Huck
2014-05-13 10:01 ` Alexander Graf
2014-04-03 13:14 ` [Qemu-devel] [PATCH 2/4] spapr: Enable DABRX special register Alexey Kardashevskiy
2014-04-03 13:19 ` Alexander Graf
2014-04-04 6:13 ` Alexey Kardashevskiy
2014-04-04 12:21 ` Alexander Graf
2014-04-03 18:42 ` Tom Musta
2014-04-04 0:51 ` Alexey Kardashevskiy
2014-04-04 12:40 ` Tom Musta
2014-04-03 13:14 ` [Qemu-devel] [PATCH 3/4] KVM: PPC: Support POWER8 registers Alexey Kardashevskiy
2014-04-03 13:33 ` Alexander Graf
2014-04-03 19:12 ` Tom Musta
2014-04-04 6:58 ` Alexey Kardashevskiy [this message]
2014-04-04 12:23 ` Alexander Graf
2014-04-03 13:14 ` [Qemu-devel] [PATCH 4/4] spapr: Add support for time base offset migration Alexey Kardashevskiy
2014-04-10 12:34 ` Alexander Graf
2014-04-10 14:31 ` Alexey Kardashevskiy
2014-04-11 9:40 ` Alexander Graf
2014-04-11 21:55 ` Benjamin Herrenschmidt
2014-04-11 22:59 ` Alexander Graf
2014-04-11 23:03 ` [Qemu-devel] [Qemu-ppc] " Alexander Graf
2014-04-12 3:44 ` [Qemu-devel] " Alexey Kardashevskiy
2014-04-12 7:25 ` Alexander Graf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=533E5802.6000900@ozlabs.ru \
--to=aik@ozlabs.ru \
--cc=agraf@suse.de \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=tommusta@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).