qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Alexey Kardashevskiy <aik@ozlabs.ru>
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 14:53:33 +1100	[thread overview]
Message-ID: <20160308035333.GT22546@voom.fritz.box> (raw)
In-Reply-To: <56DE4078.3050809@ozlabs.ru>

[-- Attachment #1: Type: text/plain, Size: 22623 bytes --]

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.

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

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

> >>          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) {
> >
> 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

  reply	other threads:[~2016-03-08  3:54 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 [this message]
2016-03-08  5:13         ` Alexey Kardashevskiy
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=20160308035333.GT22546@voom.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=agraf@suse.de \
    --cc=aik@ozlabs.ru \
    --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).