On Thu, Feb 09, 2017 at 01:14:08PM +1100, David Gibson wrote:
> On Tue, Feb 07, 2017 at 01:56:46PM +1100, Sam Bobroff wrote:
> > Use the new ioctl, KVM_PPC_GET_RMMU_INFO, to fetch radix MMU
> > information from KVM and present the page encodings in the device tree
> > under ibm,processor-radix-AP-encodings. This provides page size
> > information to the guest which is necessary for it to use radix mode.
> >
> > Signed-off-by: Sam Bobroff <sam.bobr...@au1.ibm.com>
> > ---
> > hw/ppc/spapr.c | 7 +++++++
> > target/ppc/cpu.h | 5 +++++
> > target/ppc/kvm.c | 32 +++++++++++++++++++++++++++++++-
> > 3 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index a642e663d4..d629e2630c 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -496,6 +496,13 @@ static void spapr_populate_cpu_dt(CPUState *cs, void
> > *fdt, int offset,
> >
> > _FDT(spapr_fixup_cpu_smt_dt(fdt, offset, cpu,
> > ppc_get_compat_smt_threads(cpu)));
> > +
> > + if (env->radix_page_info.count) {
> > + _FDT((fdt_setprop(fdt, offset, "ibm,processor-radix-AP-encodings",
> > + env->radix_page_info.entries,
> > + env->radix_page_info.count *
> > + sizeof(env->radix_page_info.entries[0]))));
> > + }
> > }
> >
> > static void spapr_populate_cpus_dt_node(void *fdt, sPAPRMachineState
> > *spapr)
> > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> > index afb7ddbdd0..5a96d98b6f 100644
> > --- a/target/ppc/cpu.h
> > +++ b/target/ppc/cpu.h
> > @@ -914,6 +914,10 @@ struct ppc_segment_page_sizes {
> > struct ppc_one_seg_page_size sps[PPC_PAGE_SIZES_MAX_SZ];
> > };
> >
> > +struct ppc_radix_page_info {
> > + uint32_t count;
> > + uint32_t entries[PPC_PAGE_SIZES_MAX_SZ];
>
> IIUC this info is ready for direct inclusion in the device tree:
> i.e. it's big-endian. That absolutely needs a comment in the
> structure. I'm also not sure it's a good idea, since I assume the TCG
> POWER9 code will eventually need to access this information directly
> to implement the radix MMU.
>
> Might be clearer to make this data structure native and do the BE
> conversion when generating the device tree.
Sounds good, I'll try it.
> > +};
> >
> >
> > /*****************************************************************************/
> > /* The whole PowerPC CPU context */
> > @@ -1053,6 +1057,7 @@ struct CPUPPCState {
> > ppc_slb_t vrma_slb;
> > target_ulong rmls;
> > bool ci_large_pages;
> > + struct ppc_radix_page_info radix_page_info;
>
> I'm not sure this belongs in CPUPPCState. New fields should generally
> be added to PowerPCCPU ("cpu") rather than to CPUPPCState ("env")
> unless they need to be directly accessed from TCG generated code.
>
> But even more, AFAICT this should vary only with the CPU type/model,
> not with the individual CPU instance. So this information could
> probably go into the CPU class structure instead of the instance.
>
> Yes, there are a lot of things that don't obey those rules already -
> that's because a lot of stuff hasn't been converted since the new QOM
> stuff was introduced. But we shouldn't make it any worse with new
> patches.
Agreed, I'll move it.
> > #endif
> >
> > #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
> > diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
> > index ec92c64159..390d6342cc 100644
> > --- a/target/ppc/kvm.c
> > +++ b/target/ppc/kvm.c
> > @@ -328,6 +328,18 @@ static void kvm_get_smmu_info(PowerPCCPU *cpu, struct
> > kvm_ppc_smmu_info *info)
> > kvm_get_fallback_smmu_info(cpu, info);
> > }
> >
> > +static bool kvm_get_rmmu_info(PowerPCCPU *cpu, struct kvm_ppc_rmmu_info
> > *info)
> > +{
> > + CPUState *cs = CPU(cpu);
> > + int ret;
> > +
> > + if (kvm_check_extension(cs->kvm_state, KVM_CAP_PPC_MMU_RADIX)) {
> > + ret = kvm_vm_ioctl(cs->kvm_state, KVM_PPC_GET_RMMU_INFO, info);
> > + return (ret == 0);
> > + }
> > + return false;
> > +}
> > +
> > static long gethugepagesize(const char *mem_path)
> > {
> > struct statfs fs;
> > @@ -441,9 +453,11 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> > {
> > static struct kvm_ppc_smmu_info smmu_info;
> > static bool has_smmu_info;
> > + static struct kvm_ppc_rmmu_info rmmu_info;
> > + static bool has_rmmu_info;
> > CPUPPCState *env = &cpu->env;
> > long rampagesize;
> > - int iq, ik, jq, jk;
> > + int iq, ik, jq, jk, i;
> > bool has_64k_pages = false;
> >
> > /* We only handle page sizes for 64-bit server guests for now */
> > @@ -508,6 +522,22 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
> > if (!has_64k_pages) {
> > env->mmu_model &= ~POWERPC_MMU_64K;
> > }
> > +
> > + /* Collect radix page info from kernel if not already */
> > + if (!has_rmmu_info) {
>
> Putting the data in the class would avoid this ugly messing with a
> static local, for one thing.
Yeah, that will be nice :-)
> > + env->radix_page_info.count = 0;
> > + if (kvm_get_rmmu_info(cpu, &rmmu_info)) {
> > + for (i = 0; i < 8; i++) {
>
> s/8/PPC_PAGE_SIZES_MAX_SZ/ ?
Yes!
> > + if (rmmu_info.ap_encodings[i]) {
> > + env->radix_page_info.entries[i] =
> > + cpu_to_be32(rmmu_info.ap_encodings[i]);
> > + env->radix_page_info.count++;
> > + }
> > + }
> > + }
> > + has_rmmu_info = true;
> > + }
> > +
> > }
> > #else /* defined (TARGET_PPC64) */
> >
>
> --
> 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
