qemu-ppc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-ppc] [PATCH] spapr: add "compat" machine option


From: Alexander Graf
Subject: Re: [Qemu-ppc] [PATCH] spapr: add "compat" machine option
Date: Mon, 30 Sep 2013 13:25:32 +0200

On 27.09.2013, at 10:06, Alexey Kardashevskiy wrote:

> To be able to boot on newer hardware that the software support,
> PowerISA defines a logical PVR, one per every PowerISA specification
> version from 2.04.
> 
> This adds the "compat" option which takes values 205 or 206 and forces
> QEMU to boot the guest with a logical PVR (CPU_POWERPC_LOGICAL_2_05 or
> CPU_POWERPC_LOGICAL_2_06).
> 
> The guest reads the logical PVR value from "cpu-version" property of
> a CPU device node.
> 
> Cc: Nikunj A Dadhania <address@hidden>
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> ---
> hw/ppc/spapr.c          | 40 ++++++++++++++++++++++++++++++++++++++++
> include/hw/ppc/spapr.h  |  2 ++
> target-ppc/cpu-models.h | 10 ++++++++++
> target-ppc/cpu.h        |  3 +++
> target-ppc/kvm.c        |  2 ++
> vl.c                    |  4 ++++
> 6 files changed, 61 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index a09a1d9..737452d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -33,6 +33,7 @@
> #include "sysemu/kvm.h"
> #include "kvm_ppc.h"
> #include "mmu-hash64.h"
> +#include "cpu-models.h"
> 
> #include "hw/boards.h"
> #include "hw/ppc/ppc.h"
> @@ -196,6 +197,26 @@ static XICSState *xics_system_init(int nr_servers, int 
> nr_irqs)
>     return icp;
> }
> 
> +static void spapr_compat_mode_init(sPAPREnvironment *spapr)
> +{
> +    QemuOpts *machine_opts = qemu_get_machine_opts();
> +    uint64_t compat = qemu_opt_get_number(machine_opts, "compat", 0);
> +
> +    switch (compat) {
> +    case 0:
> +        break;
> +    case 205:
> +        spapr->arch_compat = CPU_POWERPC_LOGICAL_2_05;
> +        break;
> +    case 206:
> +        spapr->arch_compat = CPU_POWERPC_LOGICAL_2_06;

Does it make sense to declare compat mode a number or would a string be easier 
for users? I can imagine that "-machine compat=power6" is easier to understand 
for a user than "-machine compat=205".

> +        break;
> +    default:
> +        perror("Unsupported mode, only are 205, 206 supported\n");
> +        break;
> +    }
> +}
> +
> static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment *spapr)
> {
>     int ret = 0, offset;
> @@ -206,6 +227,7 @@ static int spapr_fixup_cpu_dt(void *fdt, sPAPREnvironment 
> *spapr)
> 
>     CPU_FOREACH(cpu) {
>         DeviceClass *dc = DEVICE_GET_CLASS(cpu);
> +        CPUPPCState *env = &(POWERPC_CPU(cpu)->env);
>         uint32_t associativity[] = {cpu_to_be32(0x5),
>                                     cpu_to_be32(0x0),
>                                     cpu_to_be32(0x0),
> @@ -238,6 +260,14 @@ static int spapr_fixup_cpu_dt(void *fdt, 
> sPAPREnvironment *spapr)
>         if (ret < 0) {
>             return ret;
>         }
> +
> +        if (env->arch_compat) {
> +            ret = fdt_setprop(fdt, offset, "cpu-version",
> +                              &env->arch_compat, sizeof(env->arch_compat));
> +            if (ret < 0) {
> +                return ret;
> +            }
> +        }
>     }
>     return ret;
> }
> @@ -1145,6 +1175,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>     spapr = g_malloc0(sizeof(*spapr));
>     QLIST_INIT(&spapr->phbs);
> 
> +    spapr_compat_mode_init(spapr);
> +
>     cpu_ppc_hypercall = emulate_spapr_hypercall;
> 
>     /* Allocate RMA if necessary */
> @@ -1226,6 +1258,14 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
> 
>         xics_cpu_setup(spapr->icp, cpu);
> 
> +        /*
> +         * If compat mode is set in the command line, pass it to CPU so KVM
> +         * will be able to set it in the host kernel.
> +         */
> +        if (spapr->arch_compat) {
> +            env->arch_compat = spapr->arch_compat;

You should set the compat mode in KVM here, rather than doing it in the 
put_registers call which gets invoked on every register sync. Or can the guest 
change the mode?

Also, we need to handle failure. If the kernel can not set the CPU to 2.05 mode 
for example (IIRC POWER8 doesn't allow you to) we should bail out here.

And then there's the TCG question. We either have to disable CPU features 
similar to how we handle it in KVM (by setting and honoring the respective bits 
in PCR) or we need to bail out too and declare compat mode unsupported for TCG.

And then there's the fact that the kernel interface isn't upstream in a way that

> +        }
> +
>         qemu_register_reset(spapr_cpu_reset, cpu);
>     }
> 
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index ca175b0..201c578 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -34,6 +34,8 @@ typedef struct sPAPREnvironment {
>     uint32_t epow_irq;
>     Notifier epow_notifier;
> 
> +    uint32_t arch_compat;        /* Compatible PVR from the command line */
> +
>     /* Migration state */
>     int htab_save_index;
>     bool htab_first_pass;
> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> index 49ba4a4..d7c033c 100644
> --- a/target-ppc/cpu-models.h
> +++ b/target-ppc/cpu-models.h
> @@ -583,6 +583,16 @@ enum {
>     CPU_POWERPC_RS64II             = 0x00340000,
>     CPU_POWERPC_RS64III            = 0x00360000,
>     CPU_POWERPC_RS64IV             = 0x00370000,
> +
> +    /* Logical CPUs */
> +    CPU_POWERPC_LOGICAL_MASK       = 0xFFFFFFFF,
> +    CPU_POWERPC_LOGICAL_2_04       = 0x0F000001,
> +    CPU_POWERPC_LOGICAL_2_05       = 0x0F000002,
> +    CPU_POWERPC_LOGICAL_2_06       = 0x0F000003,
> +    CPU_POWERPC_LOGICAL_2_06_PLUS  = 0x0F100003,
> +    CPU_POWERPC_LOGICAL_2_07       = 0x0F000004,
> +    CPU_POWERPC_LOGICAL_2_08       = 0x0F000005,

These don't really belong here.

> +
> #endif /* defined(TARGET_PPC64) */
>     /* Original POWER */
>     /* XXX: should be POWER (RIOS), RSC3308, RSC4608,
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index 422a6bb..fc837c1 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -999,6 +999,9 @@ struct CPUPPCState {
>     /* Device control registers */
>     ppc_dcr_t *dcr_env;
> 
> +    /* Architecture compatibility mode */
> +    uint32_t arch_compat;

Do we really need to carry this in the vcpu struct? Or can we just 
fire-and-forget about it? If we want to preserve anything, it should be the PCR 
register.


Alex

> +
>     int dcache_line_size;
>     int icache_line_size;
> 
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 4bc4496..7b853a3 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -872,6 +872,7 @@ int kvm_arch_put_registers(CPUState *cs, int level)
>                 DPRINTF("Warning: Unable to set VPA information to KVM\n");
>             }
>         }
> +        kvm_set_one_reg(cs, KVM_REG_PPC_ARCH_COMPAT, &env->arch_compat);
> #endif /* TARGET_PPC64 */
>     }
> 
> @@ -1083,6 +1084,7 @@ int kvm_arch_get_registers(CPUState *cs)
>                 DPRINTF("Warning: Unable to get VPA information from KVM\n");
>             }
>         }
> +        kvm_get_one_reg(cs, KVM_REG_PPC_ARCH_COMPAT, &env->arch_compat);
> #endif
>     }
> 
> diff --git a/vl.c b/vl.c
> index 4e709d5..90dad7b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -427,6 +427,10 @@ static QemuOptsList qemu_machine_opts = {
>             .name = "usb",
>             .type = QEMU_OPT_BOOL,
>             .help = "Set on/off to enable/disable usb",
> +        }, {
> +            .name = "compat",
> +            .type = QEMU_OPT_NUMBER,
> +            .help = "Selects compatibility mode on CPU",
>         },
>         { /* End of list */ }
>     },
> -- 
> 1.8.4.rc4
> 




reply via email to

[Prev in Thread] Current Thread [Next in Thread]