Re: [Qemu-devel] [PATCH 8/9] spapr: Implement processor compatibility in ibm, client-architecture-support

[Alexander Graf <agraf@suse.de>]

On 15.05.14 13:28, Alexey Kardashevskiy wrote:
> Modern Linux kernels support last POWERPC CPUs so when a kernel boots,
> in most cases it can find a matching cpu_spec in the kernel's cpu_specs
> list. However if the kernel is quite old, it may be missing a definition
> of the actual CPU. To provide an ability for old kernels to work on modern
> hardware, a Processor Compatibility Mode has been introduced
> by the PowerISA specification.
>
>  From the hardware prospective, it is supported by the Processor
> Compatibility Register (PCR) which is defined in PowerISA. The register
> enables one of the compatibility modes (2.05/2.06/2.07).
> Since PCR is a hypervisor privileged register and cannot be
> accessed from the guest, the mode selection is done via
> ibm,client-architecture-support (CAS) RTAS call using which the guest
> specifies what "raw" and "architected" CPU versions it supports.
> QEMU works out the best match, changes a "cpu-version" property of
> every CPU and notifies the guest about the change by setting these
> properties in the buffer passed as a response on a custom H_CAS hypercall.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   hw/ppc/spapr.c       | 40 +++++++++++++++++++++++++
>   hw/ppc/spapr_hcall.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   trace-events         |  5 ++++
>   3 files changed, 128 insertions(+)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a2c9106..a0882a1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -35,6 +35,7 @@
>   #include "kvm_ppc.h"
>   #include "mmu-hash64.h"
>   #include "cpu-models.h"
> +#include "qom/cpu.h"
>   
>   #include "hw/boards.h"
>   #include "hw/ppc/ppc.h"
> @@ -592,11 +593,50 @@ int spapr_h_cas_compose_response(target_ulong addr, target_ulong size)
>   {
>       void *fdt;
>       sPAPRDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> +    CPUState *cs;
> +    int smt = kvmppc_smt_threads();
>   
>       size -= sizeof(hdr);
>   
>       fdt = g_malloc0(size);
>       _FDT((fdt_create(fdt, size)));
> +    _FDT((fdt_begin_node(fdt, "cpus")));
> +
> +    CPU_FOREACH(cs) {
> +        PowerPCCPU *cpu = POWERPC_CPU(cs);
> +        DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> +        int smpt = spapr_get_compat_smp_threads(cpu);
> +        int i, index = ppc_get_vcpu_dt_id(cpu);
> +        uint32_t servers_prop[smpt];
> +        uint32_t gservers_prop[smpt * 2];
> +        char tmp[32];
> +
> +        if ((index % smt) != 0) {
> +            continue;
> +        }
> +
> +        snprintf(tmp, 32, "%s@%x", dc->fw_name, index);
> +        trace_spapr_cas_add_subnode(tmp);
> +
> +        _FDT((fdt_begin_node(fdt, tmp)));
> +        _FDT((fdt_property_cell(fdt, "cpu-version", cpu->cpu_version)));
> +
> +        /* Build interrupt servers and gservers properties */
> +        for (i = 0; i < smpt; i++) {
> +            servers_prop[i] = cpu_to_be32(index + i);
> +            /* Hack, direct the group queues back to cpu 0 */
> +            gservers_prop[i*2] = cpu_to_be32(index + i);
> +            gservers_prop[i*2 + 1] = 0;
> +        }
> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-server#s",
> +                           servers_prop, sizeof(servers_prop))));
> +        _FDT((fdt_property(fdt, "ibm,ppc-interrupt-gserver#s",
> +                           gservers_prop, sizeof(gservers_prop))));
> +
> +        _FDT((fdt_end_node(fdt)));
> +    }
> +
> +    _FDT((fdt_end_node(fdt)));

Why is this so much code? Can we only replace full nodes? Then please 
extract the original CPU node creation into a function and just call it 
from the original generation and from here.

If we can also replace single properties I'd say we only need to 
override cpu-version.

>   
>       _FDT((fdt_finish(fdt)));
>   
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 2f6aa5c..cb815c3 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -3,6 +3,9 @@
>   #include "helper_regs.h"
>   #include "hw/ppc/spapr.h"
>   #include "mmu-hash64.h"
> +#include "cpu-models.h"
> +#include "trace.h"
> +#include "kvm_ppc.h"
>   
>   struct SPRSyncState {
>       CPUState *cs;
> @@ -752,12 +755,92 @@ out:
>       return ret;
>   }
>   
> +#define get_compat_level(cpuver) ( \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_05) ? 2050 : \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_06) ? 2060 : \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_06_PLUS) ? 2061 : \
> +    ((cpuver) == CPU_POWERPC_LOGICAL_2_07) ? 2070 : 0)
> +
>   static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>                                                     sPAPREnvironment *spapr,
>                                                     target_ulong opcode,
>                                                     target_ulong *args)
>   {
>       target_ulong list = args[0];
> +    PowerPCCPUClass *pcc_ = POWERPC_CPU_GET_CLASS(cpu_);
> +    CPUState *cs;
> +    CPUState *cs_ = CPU(cpu_);
> +    bool cpu_match = false;
> +    unsigned old_cpu_version = cpu_->cpu_version;
> +    unsigned compat_lvl = 0, cpu_version = 0;
> +    unsigned max_lvl = get_compat_level(cpu_->max_compat);
> +
> +    /* Parse PVR list */
> +    for ( ; ; ) {
> +        uint32_t pvr, pvr_mask;
> +
> +        pvr_mask = ldl_phys(cs_->as, list);

We access memory that the guest thinks gets accessed by real mode, so we 
need to mask it again. Just use the rtas helpers.

> +        list += 4;
> +        pvr = ldl_phys(cs_->as, list);
> +        list += 4;
> +
> +        trace_spapr_cas_pvr_try(pvr);
> +        if (!max_lvl &&
> +            ((cpu_->env.spr[SPR_PVR] & pvr_mask) == (pvr & pvr_mask))) {
> +            cpu_match = true;
> +            cpu_version = 0;
> +        } else if (pvr == cpu_->cpu_version) {
> +            cpu_match = true;
> +            cpu_version = cpu_->cpu_version;
> +        } else if (!cpu_match) {
> +            /* If it is a logical PVR, try to determine the highest level */
> +            unsigned lvl = get_compat_level(pvr);
> +            if (lvl) {
> +                bool is205 = (pcc_->pcr_mask & POWERPC_ISA_COMPAT_2_05) &&
> +                     (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_05));
> +                bool is206 = (pcc_->pcr_mask & POWERPC_ISA_COMPAT_2_06) &&
> +                    ((lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06)) ||
> +                    (lvl == get_compat_level(CPU_POWERPC_LOGICAL_2_06_PLUS)));
> +
> +                if (is205 || is206) {
> +                    if (!max_lvl) {
> +                        /* User did not set the level, choose the highest */
> +                        if (compat_lvl <= lvl) {
> +                            compat_lvl = lvl;
> +                            cpu_version = pvr;
> +                        }
> +                    } else if (max_lvl >= lvl) {
> +                        /* User chose the level, don't set higher than this */
> +                        compat_lvl = lvl;
> +                        cpu_version = pvr;
> +                    }
> +                }
> +            }
> +        }
> +        /* Terminator record */
> +        if (~pvr_mask & pvr) {
> +            break;
> +        }
> +    }
> +
> +    /* For the future use: here @list points to the first capability */
> +
> +    /* Parsing finished */
> +    trace_spapr_cas_pvr(cpu_->cpu_version, cpu_match,
> +                        cpu_version, pcc_->pcr_mask);
> +
> +    /* Update CPUs */
> +    if (old_cpu_version != cpu_version) {
> +        CPU_FOREACH(cs) {
> +            PowerPCCPU *cpu = POWERPC_CPU(cs);
> +
> +            ppc_set_compat(cpu, cpu_version);

Better make this a run_on call to ensure the vcpus get their new compat 
properties set.


Alex

> +        }
> +    }
> +
> +    if (!cpu_version) {
> +        return H_SUCCESS;
> +    }
>   
>       if (!list) {
>           return H_SUCCESS;
> diff --git a/trace-events b/trace-events
> index 03565dc..c2db909 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -1182,6 +1182,11 @@ xics_ics_eoi(int nr) "ics_eoi: irq %#x"
>   # hw/ppc/spapr.c
>   spapr_cas_failed(unsigned long n) "DT diff buffer is too small: %ld bytes"
>   spapr_cas_continue(unsigned long n) "Copy changes to the guest: %ld bytes"
> +spapr_cas_add_subnode(const char *s) "%s"
> +
> +# hw/ppc/spapr_hcall.c
> +spapr_cas_pvr_try(uint32_t pvr) "%x"
> +spapr_cas_pvr(uint32_t cur_pvr, bool cpu_match, uint32_t new_pvr, uint64_t pcr) "current=%x, cpu_match=%u, new=%x, compat flags=%"PRIx64
>   
>   # hw/ppc/spapr_iommu.c
>   spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64