Re: [Qemu-devel] [PATCH qemu 2/2] ppc/spapr: Add "ibm, pa-features" property to the device-tree

[Alexander Graf <agraf@suse.de>]

On 22.10.15 09:30, Alexey Kardashevskiy wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> LoPAPR defines a "ibm,pa-features" per-CPU device tree property which
> describes extended features of the Processor Architecture.
> 
> This adds the property to the device tree. At the moment this is the
> copy of what pHyp advertises except "I=1 (cache inhibited) Large Pages"
> which is enabled for TCG and disabled when running under HV KVM host
> with 4K system page size.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [aik: rebased, changed commit log, moved ci_large_pages initialization,
> renamed pa_features arrays]
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr.c              | 31 +++++++++++++++++++++++++++++++
>  target-ppc/cpu.h            |  1 +
>  target-ppc/kvm.c            |  7 +++++++
>  target-ppc/translate_init.c |  1 +
>  4 files changed, 40 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3852ad1..21c1312 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -597,6 +597,24 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>      uint32_t vcpus_per_socket = smp_threads * smp_cores;
>      uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
>  
> +    /* Note: we keep CI large pages off for now because a 64K capable guest
> +     * provisioned with large pages might otherwise try to map a qemu
> +     * framebuffer (or other kind of memory mapped PCI BAR) using 64K pages
> +     * even if that qemu runs on a 4k host.
> +     *
> +     * We can later add this bit back when we are confident this is not
> +     * an issue (!HV KVM or 64K host)
> +     */
> +    uint8_t pa_features_206[] = { 6, 0,
> +        0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
> +    uint8_t pa_features_207[] = { 24, 0,
> +        0xf6, 0x1f, 0xc7, 0xc0, 0x80, 0xf0,
> +        0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
> +        0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
> +        0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };

That's a lot of magic numbers. Do you think you could convert them into
something slightly more readable?

> +    uint8_t *pa_features;
> +    size_t pa_size;
> +
>      _FDT((fdt_setprop_cell(fdt, offset, "reg", index)));
>      _FDT((fdt_setprop_string(fdt, offset, "device_type", "cpu")));
>  
> @@ -662,6 +680,19 @@ static void spapr_populate_cpu_dt(CPUState *cs, void *fdt, int offset,
>                            page_sizes_prop, page_sizes_prop_size)));
>      }
>  
> +    /* Do the ibm,pa-features property, adjust it for ci-large-pages */
> +    if (env->mmu_model == POWERPC_MMU_2_06) {
> +        pa_features = pa_features_206;
> +        pa_size = sizeof(pa_features_206);
> +    } else /* env->mmu_model == POWERPC_MMU_2_07 */ {
> +        pa_features = pa_features_207;
> +        pa_size = sizeof(pa_features_207);
> +    }
> +    if (env->ci_large_pages) {
> +        pa_features[3] |= 0x20;
> +    }
> +    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features, pa_size)));
> +
>      _FDT((fdt_setprop_cell(fdt, offset, "ibm,chip-id",
>                             cs->cpu_index / vcpus_per_socket)));
>  
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 69d8cf6..b34aed6 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1073,6 +1073,7 @@ struct CPUPPCState {
>      uint64_t insns_flags2;
>  #if defined(TARGET_PPC64)
>      struct ppc_segment_page_sizes sps;
> +    bool ci_large_pages;
>  #endif
>  
>  #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 7671ae7..0c59f7f 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -414,6 +414,13 @@ static void kvm_fixup_page_sizes(PowerPCCPU *cpu)
>      /* Convert to QEMU form */
>      memset(&env->sps, 0, sizeof(env->sps));
>  
> +    /* If we have HV KVM, we need to forbid CI large pages if our
> +     * host page size is smaller than 64K.
> +     */
> +    if (smmu_info.flags & KVM_PPC_PAGE_SIZES_REAL) {
> +        env->ci_large_pages = getpagesize() >= 0x10000;

There's a global variable for the page size, no?


Alex