From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43674) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cgmlK-0000mg-Ua for qemu-devel@nongnu.org; Thu, 23 Feb 2017 01:20:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cgmlI-0006G1-81 for qemu-devel@nongnu.org; Thu, 23 Feb 2017 01:20:22 -0500 Message-ID: <1487830804.23895.24.camel@gmail.com> From: Suraj Jitindar Singh Date: Thu, 23 Feb 2017 17:20:04 +1100 In-Reply-To: <20170223020936.29220-7-david@gibson.dropbear.id.au> References: <20170223020936.29220-1-david@gibson.dropbear.id.au> <20170223020936.29220-7-david@gibson.dropbear.id.au> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 6/6] target/ppc: Manage external HPT via virtual hypervisor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , qemu-ppc@nongnu.org, aik@ozlabs.ru Cc: qemu-devel@nongnu.org, agraf@suse.de, thuth@redhat.com, lvivier@redhat.com, mdroth@linux.vnet.ibm.com, paulus@samba.org On Thu, 2017-02-23 at 13:09 +1100, David Gibson wrote: > The pseries machine type implements the behaviour of a PAPR compliant > hypervisor, without actually executing such a hypervisor on the > virtual > CPU.  To do this we need some hooks in the CPU code to make > hypervisor > facilities get redirected to the machine instead of emulated > internally. > > For hypercalls this is managed through the cpu->vhyp field, which > points > to a QOM interface with a method implementing the hypercall. > > For the hashed page table (HPT) - also a hypervisor resource - we use > an > older hack.  CPUPPCState has an 'external_htab' field which when non- > NULL > indicates that the HPT is stored in qemu memory, rather than within > the > guest's address space. > > For consistency - and to make some future extensions easier - this > merges > the external HPT mechanism into the vhyp mechanism.  Methods are > added > to vhyp for the basic operations the core hash MMU code needs: > map_hptes() > and unmap_hptes() for reading the HPT, store_hpte() for updating it > and > hpt_mask() to retrieve its size. > > To match this, the pseries machine now sets these vhyp fields in its > existing vhyp class, rather than reaching into the cpu object to set > the > external_htab field. > > Signed-off-by: David Gibson Reviewed-by: Suraj Jitindar Singh > --- >  hw/ppc/spapr.c          | 58 +++++++++++++++++++++++++++++ >  hw/ppc/spapr_cpu_core.c | 17 ++++++++- >  hw/ppc/spapr_hcall.c    |  3 +- >  target/ppc/cpu.h        | 10 ++++- >  target/ppc/kvm.c        |  2 +- >  target/ppc/machine.c    |  4 +- >  target/ppc/mmu-hash32.h |  8 ---- >  target/ppc/mmu-hash64.c | 98 ++++++++++++++++----------------------- > ---------- >  target/ppc/mmu-hash64.h |  7 +++- >  target/ppc/mmu_helper.c |  3 +- >  10 files changed, 123 insertions(+), 87 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 5904e64..c159961 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1050,6 +1050,60 @@ static void close_htab_fd(sPAPRMachineState > *spapr) >      spapr->htab_fd = -1; >  } >   > +static hwaddr spapr_hpt_mask(PPCVirtualHypervisor *vhyp) > +{ > +    sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp); > + > +    return HTAB_SIZE(spapr) / HASH_PTEG_SIZE_64 - 1; > +} > + > +static const ppc_hash_pte64_t *spapr_map_hptes(PPCVirtualHypervisor > *vhyp, > +                                                hwaddr ptex, int n) > +{ > +    sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp); > +    hwaddr pte_offset = ptex * HASH_PTE_SIZE_64; > + > +    if (!spapr->htab) { > +        /* > +         * HTAB is controlled by KVM. Fetch the PTEG into a new > buffer. > +         */ > +        return kvmppc_map_hptes(ptex, n); > +    } > + > +    /* > +     * HTAB is controlled by QEMU. Just point to the internally > +     * accessible PTEG. > +     */ > +    return (const ppc_hash_pte64_t *)(spapr->htab + pte_offset); > +} > + > +static void spapr_unmap_hptes(PPCVirtualHypervisor *vhyp, > +                              const ppc_hash_pte64_t *hptes, > +                              hwaddr ptex, int n) > +{ > +    sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp); > + > +    if (!spapr->htab) { > +        kvmppc_unmap_hptes(hptes, ptex, n); > +    } > + > +    /* Nothing to do for qemu managed HPT */ > +} > + > +static void spapr_store_hpte(PPCVirtualHypervisor *vhyp, hwaddr > ptex, > +                             uint64_t pte0, uint64_t pte1) > +{ > +    sPAPRMachineState *spapr = SPAPR_MACHINE(vhyp); > +    hwaddr offset = ptex * HASH_PTE_SIZE_64; > + > +    if (!spapr->htab) { > +        kvmppc_hash64_write_pte(ptex, pte0, pte1); > +    } else { > +        stq_p(spapr->htab + offset, pte0); > +        stq_p(spapr->htab + offset + HASH_PTE_SIZE_64 / 2, pte1); > +    } > +} > + >  static int spapr_hpt_shift_for_ramsize(uint64_t ramsize) >  { >      int shift; > @@ -2910,6 +2964,10 @@ static void > spapr_machine_class_init(ObjectClass *oc, void *data) >      nc->nmi_monitor_handler = spapr_nmi; >      smc->phb_placement = spapr_phb_placement; >      vhc->hypercall = emulate_spapr_hypercall; > +    vhc->hpt_mask = spapr_hpt_mask; > +    vhc->map_hptes = spapr_map_hptes; > +    vhc->unmap_hptes = spapr_unmap_hptes; > +    vhc->store_hpte = spapr_store_hpte; >  } >   >  static const TypeInfo spapr_machine_info = { > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c > index 76563c4..ddb130f 100644 > --- a/hw/ppc/spapr_cpu_core.c > +++ b/hw/ppc/spapr_cpu_core.c > @@ -13,10 +13,12 @@ >  #include "hw/boards.h" >  #include "qapi/error.h" >  #include "sysemu/cpus.h" > +#include "sysemu/kvm.h" >  #include "target/ppc/kvm_ppc.h" >  #include "hw/ppc/ppc.h" >  #include "target/ppc/mmu-hash64.h" >  #include "sysemu/numa.h" > +#include "qemu/error-report.h" >   >  static void spapr_cpu_reset(void *opaque) >  { > @@ -34,8 +36,19 @@ static void spapr_cpu_reset(void *opaque) >   >      env->spr[SPR_HIOR] = 0; >   > -    ppc_hash64_set_external_hpt(cpu, spapr->htab, spapr->htab_shift, > -                                &error_fatal); > +    /* > +     * This is a hack for the benefit of KVM PR - it abuses the SDR1 > +     * slot in kvm_sregs to communicate the userspace address of the > +     * HPT > +     */ > +    if (kvm_enabled()) { > +        env->spr[SPR_SDR1] = (target_ulong)(uintptr_t)spapr->htab > +            | (spapr->htab_shift - 18); > +        if (kvmppc_put_books_sregs(cpu) < 0) { > +            error_report("Unable to update SDR1 in KVM"); > +            exit(1); > +        } > +    } >  } >   >  static void spapr_cpu_destroy(PowerPCCPU *cpu) > diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c > index 85d96f6..f05a90e 100644 > --- a/hw/ppc/spapr_hcall.c > +++ b/hw/ppc/spapr_hcall.c > @@ -326,7 +326,6 @@ static target_ulong h_protect(PowerPCCPU *cpu, > sPAPRMachineState *spapr, >  static target_ulong h_read(PowerPCCPU *cpu, sPAPRMachineState > *spapr, >                             target_ulong opcode, target_ulong *args) >  { > -    CPUPPCState *env = &cpu->env; >      target_ulong flags = args[0]; >      target_ulong ptex = args[1]; >      uint8_t *hpte; > @@ -342,7 +341,7 @@ static target_ulong h_read(PowerPCCPU *cpu, > sPAPRMachineState *spapr, >          n_entries = 4; >      } >   > -    hpte = env->external_htab + (ptex * HASH_PTE_SIZE_64); > +    hpte = spapr->htab + (ptex * HASH_PTE_SIZE_64); >   >      for (i = 0, ridx = 0; i < n_entries; i++) { >          args[ridx++] = ldq_p(hpte); > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index c6cd9ab..27b78ca 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -1002,8 +1002,6 @@ struct CPUPPCState { >  #endif >      /* segment registers */ >      target_ulong sr[32]; > -    /* externally stored hash table */ > -    uint8_t *external_htab; >      /* BATs */ >      uint32_t nb_BATs; >      target_ulong DBAT[2][8]; > @@ -1211,6 +1209,14 @@ struct PPCVirtualHypervisor { >  struct PPCVirtualHypervisorClass { >      InterfaceClass parent; >      void (*hypercall)(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu); > +    hwaddr (*hpt_mask)(PPCVirtualHypervisor *vhyp); > +    const ppc_hash_pte64_t *(*map_hptes)(PPCVirtualHypervisor *vhyp, > +                                         hwaddr ptex, int n); > +    void (*unmap_hptes)(PPCVirtualHypervisor *vhyp, > +                        const ppc_hash_pte64_t *hptes, > +                        hwaddr ptex, int n); > +    void (*store_hpte)(PPCVirtualHypervisor *vhyp, hwaddr ptex, > +                       uint64_t pte0, uint64_t pte1); >  }; >   >  #define TYPE_PPC_VIRTUAL_HYPERVISOR "ppc-virtual-hypervisor" > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c > index 9d3e57e..c40f6aa 100644 > --- a/target/ppc/kvm.c > +++ b/target/ppc/kvm.c > @@ -1251,7 +1251,7 @@ static int kvmppc_get_books_sregs(PowerPCCPU > *cpu) >          return ret; >      } >   > -    if (!env->external_htab) { > +    if (!cpu->vhyp) { >          ppc_store_sdr1(env, sregs.u.s.sdr1); >      } >   > diff --git a/target/ppc/machine.c b/target/ppc/machine.c > index 1ccbc8a..6cb3a48 100644 > --- a/target/ppc/machine.c > +++ b/target/ppc/machine.c > @@ -76,7 +76,7 @@ static int cpu_load_old(QEMUFile *f, void *opaque, > int version_id) >          qemu_get_betls(f, &env->pb[i]); >      for (i = 0; i < 1024; i++) >          qemu_get_betls(f, &env->spr[i]); > -    if (!env->external_htab) { > +    if (!cpu->vhyp) { >          ppc_store_sdr1(env, sdr1); >      } >      qemu_get_be32s(f, &env->vscr); > @@ -228,7 +228,7 @@ static int cpu_post_load(void *opaque, int > version_id) >          env->IBAT[1][i+4] = env->spr[SPR_IBAT4U + 2*i + 1]; >      } >   > -    if (!env->external_htab) { > +    if (!cpu->vhyp) { >          ppc_store_sdr1(env, env->spr[SPR_SDR1]); Not really relevant to this patch series, but do we really need to do this? It's kinda a no-op. >      } >   > diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h > index 054be65..644cfe6 100644 > --- a/target/ppc/mmu-hash32.h > +++ b/target/ppc/mmu-hash32.h > @@ -78,10 +78,8 @@ static inline hwaddr > ppc_hash32_hpt_mask(PowerPCCPU *cpu) >  static inline target_ulong ppc_hash32_load_hpte0(PowerPCCPU *cpu, >                                                   hwaddr pte_offset) >  { > -    CPUPPCState *env = &cpu->env; >      target_ulong base = ppc_hash32_hpt_base(cpu); >   > -    assert(!env->external_htab); /* Not supported on 32-bit for now > */ >      return ldl_phys(CPU(cpu)->as, base + pte_offset); >  } >   > @@ -89,29 +87,23 @@ static inline target_ulong > ppc_hash32_load_hpte1(PowerPCCPU *cpu, >                                                   hwaddr pte_offset) >  { >      target_ulong base = ppc_hash32_hpt_base(cpu); > -    CPUPPCState *env = &cpu->env; >   > -    assert(!env->external_htab); /* Not supported on 32-bit for now > */ >      return ldl_phys(CPU(cpu)->as, base + pte_offset + > HASH_PTE_SIZE_32 / 2); >  } >   >  static inline void ppc_hash32_store_hpte0(PowerPCCPU *cpu, >                                            hwaddr pte_offset, > target_ulong pte0) >  { > -    CPUPPCState *env = &cpu->env; >      target_ulong base = ppc_hash32_hpt_base(cpu); >   > -    assert(!env->external_htab); /* Not supported on 32-bit for now > */ >      stl_phys(CPU(cpu)->as, base + pte_offset, pte0); >  } >   >  static inline void ppc_hash32_store_hpte1(PowerPCCPU *cpu, >                                            hwaddr pte_offset, > target_ulong pte1) >  { > -    CPUPPCState *env = &cpu->env; >      target_ulong base = ppc_hash32_hpt_base(cpu); >   > -    assert(!env->external_htab); /* Not supported on 32-bit for now > */ >      stl_phys(CPU(cpu)->as, base + pte_offset + HASH_PTE_SIZE_32 / 2, > pte1); >  } >   > diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c > index bb87777..ce96823 100644 > --- a/target/ppc/mmu-hash64.c > +++ b/target/ppc/mmu-hash64.c > @@ -38,12 +38,6 @@ >  #endif >   >  /* > - * Used to indicate that a CPU has its hash page table (HPT) managed > - * within the host kernel > - */ > -#define MMU_HASH64_KVM_MANAGED_HPT      ((void *)-1) > - > -/* >   * SLB handling >   */ >   > @@ -313,31 +307,6 @@ void ppc_hash64_set_sdr1(PowerPCCPU *cpu, > target_ulong value, >      env->spr[SPR_SDR1] = value; >  } >   > -void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int > shift, > -                                 Error **errp) > -{ > -    CPUPPCState *env = &cpu->env; > -    Error *local_err = NULL; > - > -    if (hpt) { > -        env->external_htab = hpt; > -    } else { > -        env->external_htab = MMU_HASH64_KVM_MANAGED_HPT; > -    } > -    ppc_hash64_set_sdr1(cpu, (target_ulong)(uintptr_t)hpt | (shift - > 18), > -                        &local_err); > -    if (local_err) { > -        error_propagate(errp, local_err); > -        return; > -    } > - > -    if (kvm_enabled()) { > -        if (kvmppc_put_books_sregs(cpu) < 0) { > -            error_setg(errp, "Unable to update SDR1 in KVM"); > -        } > -    } > -} > - >  static int ppc_hash64_pte_prot(PowerPCCPU *cpu, >                                 ppc_slb_t *slb, ppc_hash_pte64_t pte) >  { > @@ -429,28 +398,24 @@ static int ppc_hash64_amr_prot(PowerPCCPU *cpu, > ppc_hash_pte64_t pte) >  const ppc_hash_pte64_t *ppc_hash64_map_hptes(PowerPCCPU *cpu, >                                               hwaddr ptex, int n) >  { > -    const ppc_hash_pte64_t *hptes = NULL; >      hwaddr pte_offset = ptex * HASH_PTE_SIZE_64; > +    hwaddr base = ppc_hash64_hpt_base(cpu); > +    hwaddr plen = n * HASH_PTE_SIZE_64; > +    const ppc_hash_pte64_t *hptes; > + > +    if (cpu->vhyp) { > +        PPCVirtualHypervisorClass *vhc = > +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); > +        return vhc->map_hptes(cpu->vhyp, ptex, n); > +    } >   > -    if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) { > -        /* > -         * HTAB is controlled by KVM. Fetch the PTEG into a new > buffer. > -         */ > -        hptes = kvmppc_map_hptes(ptex, n); > -    } else if (cpu->env.external_htab) { > -        /* > -         * HTAB is controlled by QEMU. Just point to the internally > -         * accessible PTEG. > -         */ > -        hptes = (const ppc_hash_pte64_t *)(cpu->env.external_htab + > pte_offset); > -    } else if (ppc_hash64_hpt_base(cpu)) { > -        hwaddr base = ppc_hash64_hpt_base(cpu); > -        hwaddr plen = n * HASH_PTE_SIZE_64; > -        hptes = address_space_map(CPU(cpu)->as, base + pte_offset, > -                                  &plen, false); > -        if (plen < (n * HASH_PTE_SIZE_64)) { > -            hw_error("%s: Unable to map all requested HPTEs\n", > __FUNCTION__); > -        } > +    if (!base) { > +        return NULL; > +    } > + > +    hptes = address_space_map(CPU(cpu)->as, base + pte_offset, > &plen, false); > +    if (plen < (n * HASH_PTE_SIZE_64)) { > +        hw_error("%s: Unable to map all requested HPTEs\n", > __FUNCTION__); >      } >      return hptes; >  } > @@ -458,12 +423,15 @@ const ppc_hash_pte64_t > *ppc_hash64_map_hptes(PowerPCCPU *cpu, >  void ppc_hash64_unmap_hptes(PowerPCCPU *cpu, const ppc_hash_pte64_t > *hptes, >                              hwaddr ptex, int n) >  { > -    if (cpu->env.external_htab == MMU_HASH64_KVM_MANAGED_HPT) { > -        kvmppc_unmap_hptes(hptes, ptex, n); > -    } else if (!cpu->env.external_htab) { > -        address_space_unmap(CPU(cpu)->as, (void *)hptes, n * > HASH_PTE_SIZE_64, > -                            false, n * HASH_PTE_SIZE_64); > +    if (cpu->vhyp) { > +        PPCVirtualHypervisorClass *vhc = > +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); > +        vhc->unmap_hptes(cpu->vhyp, hptes, ptex, n); > +        return; >      } > + > +    address_space_unmap(CPU(cpu)->as, (void *)hptes, n * > HASH_PTE_SIZE_64, > +                        false, n * HASH_PTE_SIZE_64); >  } >   >  static unsigned hpte_page_shift(const struct ppc_one_seg_page_size > *sps, > @@ -915,22 +883,18 @@ hwaddr > ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr) >  void ppc_hash64_store_hpte(PowerPCCPU *cpu, hwaddr ptex, >                             uint64_t pte0, uint64_t pte1) >  { > -    CPUPPCState *env = &cpu->env; > +    hwaddr base = ppc_hash64_hpt_base(cpu); >      hwaddr offset = ptex * HASH_PTE_SIZE_64; >   > -    if (env->external_htab == MMU_HASH64_KVM_MANAGED_HPT) { > -        kvmppc_hash64_write_pte(ptex, pte0, pte1); > +    if (cpu->vhyp) { > +        PPCVirtualHypervisorClass *vhc = > +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); > +        vhc->store_hpte(cpu->vhyp, ptex, pte0, pte1); >          return; >      } >   > -    if (env->external_htab) { > -        stq_p(env->external_htab + offset, pte0); > -        stq_p(env->external_htab + offset + HASH_PTE_SIZE_64 / 2, > pte1); > -    } else { > -        hwaddr base = ppc_hash64_hpt_base(cpu); > -        stq_phys(CPU(cpu)->as, base + offset, pte0); > -        stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2, > pte1); > -    } > +    stq_phys(CPU(cpu)->as, base + offset, pte0); > +    stq_phys(CPU(cpu)->as, base + offset + HASH_PTE_SIZE_64 / 2, > pte1); >  } >   >  void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex, > diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h > index dc0bc99..9c33e9d 100644 > --- a/target/ppc/mmu-hash64.h > +++ b/target/ppc/mmu-hash64.h > @@ -101,13 +101,16 @@ static inline hwaddr > ppc_hash64_hpt_base(PowerPCCPU *cpu) >   >  static inline hwaddr ppc_hash64_hpt_mask(PowerPCCPU *cpu) >  { > +    if (cpu->vhyp) { > +        PPCVirtualHypervisorClass *vhc = > +            PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp); > +        return vhc->hpt_mask(cpu->vhyp); > +    } >      return (1ULL << ((cpu->env.spr[SPR_SDR1] & SDR_64_HTABSIZE) + 18 > - 7)) - 1; >  } >   >  void ppc_hash64_set_sdr1(PowerPCCPU *cpu, target_ulong value, >                           Error **errp); > -void ppc_hash64_set_external_hpt(PowerPCCPU *cpu, void *hpt, int > shift, > -                                 Error **errp); >   >  struct ppc_hash_pte64 { >      uint64_t pte0, pte1; > diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c > index 1381635..0176ab6 100644 > --- a/target/ppc/mmu_helper.c > +++ b/target/ppc/mmu_helper.c > @@ -2001,8 +2001,9 @@ void ppc_tlb_invalidate_one(CPUPPCState *env, > target_ulong addr) >  /* Special registers manipulation */ >  void ppc_store_sdr1(CPUPPCState *env, target_ulong value) >  { > +    PowerPCCPU *cpu = ppc_env_get_cpu(env); >      qemu_log_mask(CPU_LOG_MMU, "%s: " TARGET_FMT_lx "\n", __func__, > value); > -    assert(!env->external_htab); > +    assert(!cpu->vhyp); >  #if defined(TARGET_PPC64) >      if (env->mmu_model & POWERPC_MMU_64) { >          PowerPCCPU *cpu = ppc_env_get_cpu(env);