From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: lvivier@redhat.com, thuth@redhat.com, qemu-devel@nongnu.org,
agraf@suse.de, qemu-ppc@nongnu.org, gkurz@linux.vnet.ibm.com
Subject: Re: [Qemu-devel] [PATCHv2 1/3] target-ppc: Split out SREGS get/put functions
Date: Tue, 8 Mar 2016 16:13:08 +1100 [thread overview]
Message-ID: <56DE5F64.5070208@ozlabs.ru> (raw)
In-Reply-To: <20160308035333.GT22546@voom.fritz.box>
On 03/08/2016 02:53 PM, David Gibson wrote:
> On Tue, Mar 08, 2016 at 02:01:12PM +1100, Alexey Kardashevskiy wrote:
>> On 03/08/2016 11:37 AM, David Gibson wrote:
>>> On Mon, Mar 07, 2016 at 01:26:24PM +1100, David Gibson wrote:
>>>> Currently the getting and setting of Power MMU registers (sregs) take up
>>>> large inline chunks of the kvm_arch_get_registers() and
>>>> kvm_arch_put_registers() functions. Especially since there are two
>>>> variants (for Book-E and Book-S CPUs), only one of which will be used in
>>>> practice, this is pretty hard to read.
>>>>
>>>> This patch splits these out into helper functions for clarity. No
>>>> functional change is expected.
>>>>
>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>
>>> Greg, or Alexey, can I can an R-b for this one as well?
>>
>>
>> Well, the patch itself is quite alright but you could do BookE and BookS
>> changes in separate patches.
>
> Well.. if you insist.
I do not really, this is rather a wish for the future :)
>
>> And some more comments below.
>
>
>>
>>>
>>>> ---
>>>> target-ppc/kvm.c | 421 ++++++++++++++++++++++++++++++-------------------------
>>>> 1 file changed, 228 insertions(+), 193 deletions(-)
>>>>
>>>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>>>> index d67c169..8a762e8 100644
>>>> --- a/target-ppc/kvm.c
>>>> +++ b/target-ppc/kvm.c
>>>> @@ -867,6 +867,44 @@ static int kvm_put_vpa(CPUState *cs)
>>>> }
>>>> #endif /* TARGET_PPC64 */
>>>>
>>>> +static int kvmppc_put_books_sregs(PowerPCCPU *cpu)
>>>> +{
>>>> + CPUPPCState *env = &cpu->env;
>>>> + struct kvm_sregs sregs;
>>>> + int i;
>>>> +
>>>> + sregs.pvr = env->spr[SPR_PVR];
>>>> +
>>>> + sregs.u.s.sdr1 = env->spr[SPR_SDR1];
>>>> +
>>>> + /* Sync SLB */
>>>> +#ifdef TARGET_PPC64
>>>> + for (i = 0; i < ARRAY_SIZE(env->slb); i++) {
>>>> + sregs.u.s.ppc64.slb[i].slbe = env->slb[i].esid;
>>>> + if (env->slb[i].esid & SLB_ESID_V) {
>>>> + sregs.u.s.ppc64.slb[i].slbe |= i;
>>>> + }
>>>> + sregs.u.s.ppc64.slb[i].slbv = env->slb[i].vsid;
>>>> + }
>>>> +#endif
>>>> +
>>>> + /* Sync SRs */
>>>> + for (i = 0; i < 16; i++) {
>>>> + sregs.u.s.ppc32.sr[i] = env->sr[i];
>>>> + }
>>>> +
>>>> + /* Sync BATs */
>>>> + for (i = 0; i < 8; i++) {
>>>> + /* Beware. We have to swap upper and lower bits here */
>>>> + sregs.u.s.ppc32.dbat[i] = ((uint64_t)env->DBAT[0][i] << 32)
>>>> + | env->DBAT[1][i];
>>>> + sregs.u.s.ppc32.ibat[i] = ((uint64_t)env->IBAT[0][i] << 32)
>>>> + | env->IBAT[1][i];
>>>> + }
>>>> +
>>>> + return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, &sregs);
>>>> +}
>>>> +
>>>> int kvm_arch_put_registers(CPUState *cs, int level)
>>>> {
>>>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>> @@ -920,39 +958,8 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>>>> }
>>>>
>>>> if (cap_segstate && (level >= KVM_PUT_RESET_STATE)) {
>>>> - struct kvm_sregs sregs;
>>>> -
>>>> - sregs.pvr = env->spr[SPR_PVR];
>>>> -
>>>> - sregs.u.s.sdr1 = env->spr[SPR_SDR1];
>>>> -
>>>> - /* Sync SLB */
>>>> -#ifdef TARGET_PPC64
>>>> - for (i = 0; i < ARRAY_SIZE(env->slb); i++) {
>>>> - sregs.u.s.ppc64.slb[i].slbe = env->slb[i].esid;
>>>> - if (env->slb[i].esid & SLB_ESID_V) {
>>>> - sregs.u.s.ppc64.slb[i].slbe |= i;
>>>> - }
>>>> - sregs.u.s.ppc64.slb[i].slbv = env->slb[i].vsid;
>>>> - }
>>>> -#endif
>>>> -
>>>> - /* Sync SRs */
>>>> - for (i = 0; i < 16; i++) {
>>>> - sregs.u.s.ppc32.sr[i] = env->sr[i];
>>>> - }
>>>> -
>>>> - /* Sync BATs */
>>>> - for (i = 0; i < 8; i++) {
>>>> - /* Beware. We have to swap upper and lower bits here */
>>>> - sregs.u.s.ppc32.dbat[i] = ((uint64_t)env->DBAT[0][i] << 32)
>>>> - | env->DBAT[1][i];
>>>> - sregs.u.s.ppc32.ibat[i] = ((uint64_t)env->IBAT[0][i] << 32)
>>>> - | env->IBAT[1][i];
>>>> - }
>>>> -
>>>> - ret = kvm_vcpu_ioctl(cs, KVM_SET_SREGS, &sregs);
>>>> - if (ret) {
>>>> + ret = kvmppc_put_books_sregs(cpu);
>>>> + if (ret < 0) {
>>>> return ret;
>>>> }
>>>> }
>>>> @@ -1014,12 +1021,197 @@ static void kvm_sync_excp(CPUPPCState *env, int vector, int ivor)
>>>> env->excp_vectors[vector] = env->spr[ivor] + env->spr[SPR_BOOKE_IVPR];
>>>> }
>>>>
>>>> +static int kvmppc_get_booke_sregs(PowerPCCPU *cpu)
>>
>>
>> I found it confusing that the patch is not adding kvmppc_put_booke_sregs...
>> Not a problem of this patch though but if the commit log for booke-only
>> patch has mentioned it, I would not have to have a look in QEMU tree :)
>
> Hm, ok.
>
>>>> +{
>>>> + CPUPPCState *env = &cpu->env;
>>>> + struct kvm_sregs sregs;
>>>> + int ret;
>>>> +
>>>> + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS, &sregs);
>>>> + if (ret < 0) {
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_BASE) {
>>>> + env->spr[SPR_BOOKE_CSRR0] = sregs.u.e.csrr0;
>>>> + env->spr[SPR_BOOKE_CSRR1] = sregs.u.e.csrr1;
>>>> + env->spr[SPR_BOOKE_ESR] = sregs.u.e.esr;
>>>> + env->spr[SPR_BOOKE_DEAR] = sregs.u.e.dear;
>>>> + env->spr[SPR_BOOKE_MCSR] = sregs.u.e.mcsr;
>>>> + env->spr[SPR_BOOKE_TSR] = sregs.u.e.tsr;
>>>> + env->spr[SPR_BOOKE_TCR] = sregs.u.e.tcr;
>>>> + env->spr[SPR_DECR] = sregs.u.e.dec;
>>>> + env->spr[SPR_TBL] = sregs.u.e.tb & 0xffffffff;
>>>> + env->spr[SPR_TBU] = sregs.u.e.tb >> 32;
>>>> + env->spr[SPR_VRSAVE] = sregs.u.e.vrsave;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_ARCH206) {
>>>> + env->spr[SPR_BOOKE_PIR] = sregs.u.e.pir;
>>>> + env->spr[SPR_BOOKE_MCSRR0] = sregs.u.e.mcsrr0;
>>>> + env->spr[SPR_BOOKE_MCSRR1] = sregs.u.e.mcsrr1;
>>>> + env->spr[SPR_BOOKE_DECAR] = sregs.u.e.decar;
>>>> + env->spr[SPR_BOOKE_IVPR] = sregs.u.e.ivpr;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_64) {
>>>> + env->spr[SPR_BOOKE_EPCR] = sregs.u.e.epcr;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_SPRG8) {
>>>> + env->spr[SPR_BOOKE_SPRG8] = sregs.u.e.sprg8;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_IVOR) {
>>>> + env->spr[SPR_BOOKE_IVOR0] = sregs.u.e.ivor_low[0];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_CRITICAL, SPR_BOOKE_IVOR0);
>>>> + env->spr[SPR_BOOKE_IVOR1] = sregs.u.e.ivor_low[1];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_MCHECK, SPR_BOOKE_IVOR1);
>>>> + env->spr[SPR_BOOKE_IVOR2] = sregs.u.e.ivor_low[2];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_DSI, SPR_BOOKE_IVOR2);
>>>> + env->spr[SPR_BOOKE_IVOR3] = sregs.u.e.ivor_low[3];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_ISI, SPR_BOOKE_IVOR3);
>>>> + env->spr[SPR_BOOKE_IVOR4] = sregs.u.e.ivor_low[4];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_EXTERNAL, SPR_BOOKE_IVOR4);
>>>> + env->spr[SPR_BOOKE_IVOR5] = sregs.u.e.ivor_low[5];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_ALIGN, SPR_BOOKE_IVOR5);
>>>> + env->spr[SPR_BOOKE_IVOR6] = sregs.u.e.ivor_low[6];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_PROGRAM, SPR_BOOKE_IVOR6);
>>>> + env->spr[SPR_BOOKE_IVOR7] = sregs.u.e.ivor_low[7];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_FPU, SPR_BOOKE_IVOR7);
>>>> + env->spr[SPR_BOOKE_IVOR8] = sregs.u.e.ivor_low[8];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_SYSCALL, SPR_BOOKE_IVOR8);
>>>> + env->spr[SPR_BOOKE_IVOR9] = sregs.u.e.ivor_low[9];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_APU, SPR_BOOKE_IVOR9);
>>>> + env->spr[SPR_BOOKE_IVOR10] = sregs.u.e.ivor_low[10];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_DECR, SPR_BOOKE_IVOR10);
>>>> + env->spr[SPR_BOOKE_IVOR11] = sregs.u.e.ivor_low[11];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_FIT, SPR_BOOKE_IVOR11);
>>>> + env->spr[SPR_BOOKE_IVOR12] = sregs.u.e.ivor_low[12];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_WDT, SPR_BOOKE_IVOR12);
>>>> + env->spr[SPR_BOOKE_IVOR13] = sregs.u.e.ivor_low[13];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_DTLB, SPR_BOOKE_IVOR13);
>>>> + env->spr[SPR_BOOKE_IVOR14] = sregs.u.e.ivor_low[14];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_ITLB, SPR_BOOKE_IVOR14);
>>>> + env->spr[SPR_BOOKE_IVOR15] = sregs.u.e.ivor_low[15];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_DEBUG, SPR_BOOKE_IVOR15);
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_SPE) {
>>>> + env->spr[SPR_BOOKE_IVOR32] = sregs.u.e.ivor_high[0];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_SPEU, SPR_BOOKE_IVOR32);
>>>> + env->spr[SPR_BOOKE_IVOR33] = sregs.u.e.ivor_high[1];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_EFPDI, SPR_BOOKE_IVOR33);
>>>> + env->spr[SPR_BOOKE_IVOR34] = sregs.u.e.ivor_high[2];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_EFPRI, SPR_BOOKE_IVOR34);
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_PM) {
>>>> + env->spr[SPR_BOOKE_IVOR35] = sregs.u.e.ivor_high[3];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_EPERFM, SPR_BOOKE_IVOR35);
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_PC) {
>>>> + env->spr[SPR_BOOKE_IVOR36] = sregs.u.e.ivor_high[4];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_DOORI, SPR_BOOKE_IVOR36);
>>>> + env->spr[SPR_BOOKE_IVOR37] = sregs.u.e.ivor_high[5];
>>>> + kvm_sync_excp(env, POWERPC_EXCP_DOORCI, SPR_BOOKE_IVOR37);
>>>> + }
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_ARCH206_MMU) {
>>>> + env->spr[SPR_BOOKE_MAS0] = sregs.u.e.mas0;
>>>> + env->spr[SPR_BOOKE_MAS1] = sregs.u.e.mas1;
>>>> + env->spr[SPR_BOOKE_MAS2] = sregs.u.e.mas2;
>>>> + env->spr[SPR_BOOKE_MAS3] = sregs.u.e.mas7_3 & 0xffffffff;
>>>> + env->spr[SPR_BOOKE_MAS4] = sregs.u.e.mas4;
>>>> + env->spr[SPR_BOOKE_MAS6] = sregs.u.e.mas6;
>>>> + env->spr[SPR_BOOKE_MAS7] = sregs.u.e.mas7_3 >> 32;
>>>> + env->spr[SPR_MMUCFG] = sregs.u.e.mmucfg;
>>>> + env->spr[SPR_BOOKE_TLB0CFG] = sregs.u.e.tlbcfg[0];
>>>> + env->spr[SPR_BOOKE_TLB1CFG] = sregs.u.e.tlbcfg[1];
>>>> + }
>>
>> Wrong indend.
>
> Yeah, Thomas noted that so I've already fixed it.
>
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_EXP) {
>>>> + env->spr[SPR_BOOKE_EPR] = sregs.u.e.epr;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.features & KVM_SREGS_E_PD) {
>>>> + env->spr[SPR_BOOKE_EPLC] = sregs.u.e.eplc;
>>>> + env->spr[SPR_BOOKE_EPSC] = sregs.u.e.epsc;
>>>> + }
>>>> +
>>>> + if (sregs.u.e.impl_id == KVM_SREGS_E_IMPL_FSL) {
>>>> + env->spr[SPR_E500_SVR] = sregs.u.e.impl.fsl.svr;
>>>> + env->spr[SPR_Exxx_MCAR] = sregs.u.e.impl.fsl.mcar;
>>>> + env->spr[SPR_HID0] = sregs.u.e.impl.fsl.hid0;
>>>> +
>>>> + if (sregs.u.e.impl.fsl.features & KVM_SREGS_E_FSL_PIDn) {
>>>> + env->spr[SPR_BOOKE_PID1] = sregs.u.e.impl.fsl.pid1;
>>>> + env->spr[SPR_BOOKE_PID2] = sregs.u.e.impl.fsl.pid2;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int kvmppc_get_books_sregs(PowerPCCPU *cpu)
>>>> +{
>>>> + CPUPPCState *env = &cpu->env;
>>>> + struct kvm_sregs sregs;
>>>> + int ret;
>>>> + int i;
>>>> +
>>>> + ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_SREGS, &sregs);
>>>> + if (ret < 0) {
>>>> + return ret;
>>>> + }
>>>> +
>>>> + if (!env->external_htab) {
>>>> + ppc_store_sdr1(env, sregs.u.s.sdr1);
>>>> + }
>>>> +
>>>> + /* Sync SLB */
>>>> +#ifdef TARGET_PPC64
>>>> + /*
>>>> + * The packed SLB array we get from KVM_GET_SREGS only contains
>>>> + * information about valid entries. So we flush our internal copy
>>>> + * to get rid of stale ones, then put all valid SLB entries back
>>>> + * in.
>>>> + */
>>>> + memset(env->slb, 0, sizeof(env->slb));
>>>> + for (i = 0; i < ARRAY_SIZE(env->slb); i++) {
>>>> + target_ulong rb = sregs.u.s.ppc64.slb[i].slbe;
>>>> + target_ulong rs = sregs.u.s.ppc64.slb[i].slbv;
>>>> + /*
>>>> + * Only restore valid entries
>>>> + */
>>>> + if (rb & SLB_ESID_V) {
>>>> + ppc_store_slb(cpu, rb & 0xfff, rb & ~0xfffULL, rs);
>>>> + }
>>>> + }
>>>> +#endif
>>>> +
>>>> + /* Sync SRs */
>>>> + for (i = 0; i < 16; i++) {
>>>> + env->sr[i] = sregs.u.s.ppc32.sr[i];
>>>> + }
>>>> +
>>>> + /* Sync BATs */
>>>> + for (i = 0; i < 8; i++) {
>>>> + env->DBAT[0][i] = sregs.u.s.ppc32.dbat[i] & 0xffffffff;
>>>> + env->DBAT[1][i] = sregs.u.s.ppc32.dbat[i] >> 32;
>>>> + env->IBAT[0][i] = sregs.u.s.ppc32.ibat[i] & 0xffffffff;
>>>> + env->IBAT[1][i] = sregs.u.s.ppc32.ibat[i] >> 32;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> int kvm_arch_get_registers(CPUState *cs)
>>>> {
>>>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>>>> CPUPPCState *env = &cpu->env;
>>>> struct kvm_regs regs;
>>>> - struct kvm_sregs sregs;
>>>> uint32_t cr;
>>>> int i, ret;
>>>>
>>>> @@ -1059,174 +1251,17 @@ int kvm_arch_get_registers(CPUState *cs)
>>>> kvm_get_fp(cs);
>>>>
>>>> if (cap_booke_sregs) {
>>>> - ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
>>>> + ret = kvmppc_get_booke_sregs(cpu);
>>>> if (ret < 0) {
>>>> return ret;
>>>> }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_BASE) {
>>>> - env->spr[SPR_BOOKE_CSRR0] = sregs.u.e.csrr0;
>>>> - env->spr[SPR_BOOKE_CSRR1] = sregs.u.e.csrr1;
>>>> - env->spr[SPR_BOOKE_ESR] = sregs.u.e.esr;
>>>> - env->spr[SPR_BOOKE_DEAR] = sregs.u.e.dear;
>>>> - env->spr[SPR_BOOKE_MCSR] = sregs.u.e.mcsr;
>>>> - env->spr[SPR_BOOKE_TSR] = sregs.u.e.tsr;
>>>> - env->spr[SPR_BOOKE_TCR] = sregs.u.e.tcr;
>>>> - env->spr[SPR_DECR] = sregs.u.e.dec;
>>>> - env->spr[SPR_TBL] = sregs.u.e.tb & 0xffffffff;
>>>> - env->spr[SPR_TBU] = sregs.u.e.tb >> 32;
>>>> - env->spr[SPR_VRSAVE] = sregs.u.e.vrsave;
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_ARCH206) {
>>>> - env->spr[SPR_BOOKE_PIR] = sregs.u.e.pir;
>>>> - env->spr[SPR_BOOKE_MCSRR0] = sregs.u.e.mcsrr0;
>>>> - env->spr[SPR_BOOKE_MCSRR1] = sregs.u.e.mcsrr1;
>>>> - env->spr[SPR_BOOKE_DECAR] = sregs.u.e.decar;
>>>> - env->spr[SPR_BOOKE_IVPR] = sregs.u.e.ivpr;
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_64) {
>>>> - env->spr[SPR_BOOKE_EPCR] = sregs.u.e.epcr;
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_SPRG8) {
>>>> - env->spr[SPR_BOOKE_SPRG8] = sregs.u.e.sprg8;
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_IVOR) {
>>>> - env->spr[SPR_BOOKE_IVOR0] = sregs.u.e.ivor_low[0];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_CRITICAL, SPR_BOOKE_IVOR0);
>>>> - env->spr[SPR_BOOKE_IVOR1] = sregs.u.e.ivor_low[1];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_MCHECK, SPR_BOOKE_IVOR1);
>>>> - env->spr[SPR_BOOKE_IVOR2] = sregs.u.e.ivor_low[2];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_DSI, SPR_BOOKE_IVOR2);
>>>> - env->spr[SPR_BOOKE_IVOR3] = sregs.u.e.ivor_low[3];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_ISI, SPR_BOOKE_IVOR3);
>>>> - env->spr[SPR_BOOKE_IVOR4] = sregs.u.e.ivor_low[4];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_EXTERNAL, SPR_BOOKE_IVOR4);
>>>> - env->spr[SPR_BOOKE_IVOR5] = sregs.u.e.ivor_low[5];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_ALIGN, SPR_BOOKE_IVOR5);
>>>> - env->spr[SPR_BOOKE_IVOR6] = sregs.u.e.ivor_low[6];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_PROGRAM, SPR_BOOKE_IVOR6);
>>>> - env->spr[SPR_BOOKE_IVOR7] = sregs.u.e.ivor_low[7];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_FPU, SPR_BOOKE_IVOR7);
>>>> - env->spr[SPR_BOOKE_IVOR8] = sregs.u.e.ivor_low[8];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_SYSCALL, SPR_BOOKE_IVOR8);
>>>> - env->spr[SPR_BOOKE_IVOR9] = sregs.u.e.ivor_low[9];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_APU, SPR_BOOKE_IVOR9);
>>>> - env->spr[SPR_BOOKE_IVOR10] = sregs.u.e.ivor_low[10];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_DECR, SPR_BOOKE_IVOR10);
>>>> - env->spr[SPR_BOOKE_IVOR11] = sregs.u.e.ivor_low[11];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_FIT, SPR_BOOKE_IVOR11);
>>>> - env->spr[SPR_BOOKE_IVOR12] = sregs.u.e.ivor_low[12];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_WDT, SPR_BOOKE_IVOR12);
>>>> - env->spr[SPR_BOOKE_IVOR13] = sregs.u.e.ivor_low[13];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_DTLB, SPR_BOOKE_IVOR13);
>>>> - env->spr[SPR_BOOKE_IVOR14] = sregs.u.e.ivor_low[14];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_ITLB, SPR_BOOKE_IVOR14);
>>>> - env->spr[SPR_BOOKE_IVOR15] = sregs.u.e.ivor_low[15];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_DEBUG, SPR_BOOKE_IVOR15);
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_SPE) {
>>>> - env->spr[SPR_BOOKE_IVOR32] = sregs.u.e.ivor_high[0];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_SPEU, SPR_BOOKE_IVOR32);
>>>> - env->spr[SPR_BOOKE_IVOR33] = sregs.u.e.ivor_high[1];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_EFPDI, SPR_BOOKE_IVOR33);
>>>> - env->spr[SPR_BOOKE_IVOR34] = sregs.u.e.ivor_high[2];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_EFPRI, SPR_BOOKE_IVOR34);
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_PM) {
>>>> - env->spr[SPR_BOOKE_IVOR35] = sregs.u.e.ivor_high[3];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_EPERFM, SPR_BOOKE_IVOR35);
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_PC) {
>>>> - env->spr[SPR_BOOKE_IVOR36] = sregs.u.e.ivor_high[4];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_DOORI, SPR_BOOKE_IVOR36);
>>>> - env->spr[SPR_BOOKE_IVOR37] = sregs.u.e.ivor_high[5];
>>>> - kvm_sync_excp(env, POWERPC_EXCP_DOORCI, SPR_BOOKE_IVOR37);
>>>> - }
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_ARCH206_MMU) {
>>>> - env->spr[SPR_BOOKE_MAS0] = sregs.u.e.mas0;
>>>> - env->spr[SPR_BOOKE_MAS1] = sregs.u.e.mas1;
>>>> - env->spr[SPR_BOOKE_MAS2] = sregs.u.e.mas2;
>>>> - env->spr[SPR_BOOKE_MAS3] = sregs.u.e.mas7_3 & 0xffffffff;
>>>> - env->spr[SPR_BOOKE_MAS4] = sregs.u.e.mas4;
>>>> - env->spr[SPR_BOOKE_MAS6] = sregs.u.e.mas6;
>>>> - env->spr[SPR_BOOKE_MAS7] = sregs.u.e.mas7_3 >> 32;
>>>> - env->spr[SPR_MMUCFG] = sregs.u.e.mmucfg;
>>>> - env->spr[SPR_BOOKE_TLB0CFG] = sregs.u.e.tlbcfg[0];
>>>> - env->spr[SPR_BOOKE_TLB1CFG] = sregs.u.e.tlbcfg[1];
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_EXP) {
>>>> - env->spr[SPR_BOOKE_EPR] = sregs.u.e.epr;
>>>> - }
>>>> -
>>>> - if (sregs.u.e.features & KVM_SREGS_E_PD) {
>>>> - env->spr[SPR_BOOKE_EPLC] = sregs.u.e.eplc;
>>>> - env->spr[SPR_BOOKE_EPSC] = sregs.u.e.epsc;
>>>> - }
>>>> -
>>>> - if (sregs.u.e.impl_id == KVM_SREGS_E_IMPL_FSL) {
>>>> - env->spr[SPR_E500_SVR] = sregs.u.e.impl.fsl.svr;
>>>> - env->spr[SPR_Exxx_MCAR] = sregs.u.e.impl.fsl.mcar;
>>>> - env->spr[SPR_HID0] = sregs.u.e.impl.fsl.hid0;
>>>> -
>>>> - if (sregs.u.e.impl.fsl.features & KVM_SREGS_E_FSL_PIDn) {
>>>> - env->spr[SPR_BOOKE_PID1] = sregs.u.e.impl.fsl.pid1;
>>>> - env->spr[SPR_BOOKE_PID2] = sregs.u.e.impl.fsl.pid2;
>>>> - }
>>>> - }
>>>> }
>>>>
>>>> if (cap_segstate) {
>>>> - ret = kvm_vcpu_ioctl(cs, KVM_GET_SREGS, &sregs);
>>>> + ret = kvmppc_get_books_sregs(cpu);
>>
>>
>> s/kvmppc_get_books_sregs/kvmppc_get_books_segstate_sregs/
>
> Um.. why? It's the {GET,SET}_SREGS call that these are wrapping.
> cap_segstate is just a kinda-old name for it.
So cap_segstate == book3s_sregs effectively? Ok.
>> or fold "if (cap_hior)" (at the end of this chunk) into
>> kvmppc_get_books_sregs().
>
> Again, why? HIOR is a separate one-reg call, not in the sregs block.
I misinterpret what HIOR is, this is not pseries but rather powermac,
right? Then never mind.
With that indent fixed,
Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>>> if (ret < 0) {
>>>> return ret;
>>>> }
>>>> -
>>>> - if (!env->external_htab) {
>>>> - ppc_store_sdr1(env, sregs.u.s.sdr1);
>>>> - }
>>>> -
>>>> - /* Sync SLB */
>>>> -#ifdef TARGET_PPC64
>>>> - /*
>>>> - * The packed SLB array we get from KVM_GET_SREGS only contains
>>>> - * information about valid entries. So we flush our internal
>>>> - * copy to get rid of stale ones, then put all valid SLB entries
>>>> - * back in.
>>>> - */
>>>> - memset(env->slb, 0, sizeof(env->slb));
>>>> - for (i = 0; i < ARRAY_SIZE(env->slb); i++) {
>>>> - target_ulong rb = sregs.u.s.ppc64.slb[i].slbe;
>>>> - target_ulong rs = sregs.u.s.ppc64.slb[i].slbv;
>>>> - /*
>>>> - * Only restore valid entries
>>>> - */
>>>> - if (rb & SLB_ESID_V) {
>>>> - ppc_store_slb(cpu, rb & 0xfff, rb & ~0xfffULL, rs);
>>>> - }
>>>> - }
>>>> -#endif
>>>> -
>>>> - /* Sync SRs */
>>>> - for (i = 0; i < 16; i++) {
>>>> - env->sr[i] = sregs.u.s.ppc32.sr[i];
>>>> - }
>>>> -
>>>> - /* Sync BATs */
>>>> - for (i = 0; i < 8; i++) {
>>>> - env->DBAT[0][i] = sregs.u.s.ppc32.dbat[i] & 0xffffffff;
>>>> - env->DBAT[1][i] = sregs.u.s.ppc32.dbat[i] >> 32;
>>>> - env->IBAT[0][i] = sregs.u.s.ppc32.ibat[i] & 0xffffffff;
>>>> - env->IBAT[1][i] = sregs.u.s.ppc32.ibat[i] >> 32;
>>>> - }
>>>> }
>>>>
>>>> if (cap_hior) {
>>>
>>
>>
>
--
Alexey
next prev parent reply other threads:[~2016-03-08 5:13 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-03-07 2:26 [Qemu-devel] [PATCHv2 0/3] target-ppc: Clean up handling of SDR1 and external HPTs David Gibson
2016-03-07 2:26 ` [Qemu-devel] [PATCHv2 1/3] target-ppc: Split out SREGS get/put functions David Gibson
2016-03-07 11:22 ` Thomas Huth
2016-03-08 0:32 ` David Gibson
2016-03-08 0:37 ` David Gibson
2016-03-08 3:01 ` Alexey Kardashevskiy
2016-03-08 3:53 ` David Gibson
2016-03-08 5:13 ` Alexey Kardashevskiy [this message]
2016-03-08 5:50 ` David Gibson
2016-03-08 8:50 ` Greg Kurz
2016-03-07 2:26 ` [Qemu-devel] [PATCHv2 2/3] target-ppc: Add helpers for updating a CPU's SDR1 and external HPT David Gibson
2016-03-07 13:37 ` Greg Kurz
2016-03-07 15:56 ` Thomas Huth
2016-03-22 16:33 ` Laurent Vivier
2016-03-24 5:35 ` David Gibson
2016-03-24 8:41 ` Laurent Vivier
2016-03-25 9:13 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2016-03-29 6:39 ` David Gibson
2016-03-07 2:26 ` [Qemu-devel] [PATCHv2 3/3] target-ppc: Eliminate kvmppc_kern_htab global David Gibson
2016-03-07 13:41 ` Greg Kurz
2016-03-08 0:36 ` David Gibson
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=56DE5F64.5070208@ozlabs.ru \
--to=aik@ozlabs.ru \
--cc=agraf@suse.de \
--cc=david@gibson.dropbear.id.au \
--cc=gkurz@linux.vnet.ibm.com \
--cc=lvivier@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=thuth@redhat.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).