Re: [Qemu-devel] [RFC PATCH v4] powerpc: add PVR mask support

[Alexander Graf]

On 19.08.2013, at 06:06, Alexey Kardashevskiy wrote:

> IBM POWERPC processors encode PVR as a CPU family in higher 16 bits and
> a CPU version in lower 16 bits. Since there is no significant change
> in behavior between versions, there is no point to add every single CPU
> version in QEMU's CPU list. Also, new CPU versions of already supported
> CPU won't break the existing code.
> 
> This does the following:
> 1. add a PVR mask support for a CPU family (in this patch for POWER7 only);
> 2. make CPU family not abstract;
> 3. add a @pvr_default (probably bad name) - the idea when QEMU instantiates
> POWERPC CPU from a CPU family class, this value will be written to PVR
> before the guest starts; KVM uses this to pass the actual PVR value to
> the guest if QEMU was started with -cpu host.
> 
> As CPU family class name for POWER7 is "POWER7-family", there is no need
> to touch aliases.
> 
> Cc: Andreas Färber <address@hidden>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
> 
> ---
> 
> This does not pretend to be the final valid patch which can go to any tree
> (at least because it does need a POWER7+ family difinition and some bits
> for POWER8 family), it is me again asking stupid question in order to
> reduce my ignorance and get some understanding if anyone going to care of
> the PVR masks problem. Thank you for comments.
> 
> ps. :)
> ---
> Changes:
> v4:
> * removed bogus layer from hierarchy
> 
> v3:
> * renamed macros to describe the functionality better
> * added default PVR value for the powerpc cpu family (what alias used to do)
> 
> v2:
> * aliases are replaced with another level in class hierarchy
> ---
> target-ppc/cpu-models.c     | 2 ++
> target-ppc/cpu-models.h     | 7 +++++++
> target-ppc/cpu-qom.h        | 2 ++
> target-ppc/kvm.c            | 2 ++
> target-ppc/translate_init.c | 9 +++++++--
> 5 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/target-ppc/cpu-models.c b/target-ppc/cpu-models.c
> index 8dea560..5ae88a5 100644
> --- a/target-ppc/cpu-models.c
> +++ b/target-ppc/cpu-models.c
> @@ -44,6 +44,8 @@
>         PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);                       \
>                                                                             \
>         pcc->pvr          = _pvr;                                           \
> +        pcc->pvr_default  = 0;                                              \

There is no need for pvr_default if you limit family instantiation to -cpu 
host. Just make sure to filter out from cpu ? (and the qmp equivalent) and we 
should be good.

> +        pcc->pvr_mask     = CPU_POWERPC_DEFAULT_MASK;                       \
>         pcc->svr          = _svr;                                           \
>         dc->desc          = _desc;                                          \
>     }                                                                       \
> diff --git a/target-ppc/cpu-models.h b/target-ppc/cpu-models.h
> index d9145d1..2233053 100644
> --- a/target-ppc/cpu-models.h
> +++ b/target-ppc/cpu-models.h
> @@ -39,6 +39,7 @@ extern PowerPCCPUAlias ppc_cpu_aliases[];
> /*****************************************************************************/
> /* PVR definitions for most known PowerPC                                    
> */
> enum {
> +    CPU_POWERPC_DEFAULT_MASK       = 0xFFFFFFFF,
>     /* PowerPC 401 family */
>     /* Generic PowerPC 401 */
> #define CPU_POWERPC_401              CPU_POWERPC_401G2
> @@ -557,6 +558,12 @@ enum {
>     CPU_POWERPC_POWER7_v23         = 0x003F0203,
>     CPU_POWERPC_POWER7P_v21        = 0x004A0201,
>     CPU_POWERPC_POWER8_v10         = 0x004B0100,
> +    CPU_POWERPC_POWER7             = 0x003F0000,
> +    CPU_POWERPC_POWER7_MASK        = 0xFFFF0000,
> +    CPU_POWERPC_POWER7P            = 0x004A0000,
> +    CPU_POWERPC_POWER7P_MASK       = 0xFFFF0000,
> +    CPU_POWERPC_POWER8             = 0x004B0000,
> +    CPU_POWERPC_POWER8_MASK        = 0xFFFF0000,

Please add support for all the other CPUs supported by PR and HV KVM at least 
on Book3S, but preferably everything Linux supports once this patch is out of 
RFC.

>     CPU_POWERPC_970                = 0x00390202,
>     CPU_POWERPC_970FX_v10          = 0x00391100,
>     CPU_POWERPC_970FX_v20          = 0x003C0200,
> diff --git a/target-ppc/cpu-qom.h b/target-ppc/cpu-qom.h
> index f3c710a..a1a712c 100644
> --- a/target-ppc/cpu-qom.h
> +++ b/target-ppc/cpu-qom.h
> @@ -54,6 +54,8 @@ typedef struct PowerPCCPUClass {
>     void (*parent_reset)(CPUState *cpu);
> 
>     uint32_t pvr;
> +    uint32_t pvr_default;
> +    uint32_t pvr_mask;
>     uint32_t svr;
>     uint64_t insns_flags;
>     uint64_t insns_flags2;
> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
> index 30a870e..315e499 100644
> --- a/target-ppc/kvm.c
> +++ b/target-ppc/kvm.c
> @@ -1731,6 +1731,8 @@ static void kvmppc_host_cpu_class_init(ObjectClass *oc, 
> void *data)
>     uint32_t dcache_size = kvmppc_read_int_cpu_dt("d-cache-size");
>     uint32_t icache_size = kvmppc_read_int_cpu_dt("i-cache-size");
> 
> +    pcc->pvr_default = mfpvr();
> +
>     /* Now fix up the class with information we can query from the host */
> 
>     if (vmx != -1) {
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 13b290c..dfe398d 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -3104,7 +3104,6 @@ static int check_pow_hid0_74xx (CPUPPCState *env)
>     glue(glue(ppc_, _name), _cpu_family_type_info) = {                      \
>         .name = stringify(_name) "-family-" TYPE_POWERPC_CPU,               \
>         .parent = TYPE_POWERPC_CPU,                                         \
> -        .abstract = true,                                                   \
>         .class_init = glue(glue(ppc_, _name), _cpu_family_class_init),      \
>     };                                                                      \
>                                                                             \
> @@ -7213,6 +7212,9 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>     PowerPCCPUClass *pcc = POWERPC_CPU_CLASS(oc);
> 
>     dc->desc = "POWER7";
> +    pcc->pvr = CPU_POWERPC_POWER7;
> +    pcc->pvr_mask = CPU_POWERPC_POWER7_MASK;
> +    pcc->pvr_default = CPU_POWERPC_POWER7_v23;
>     pcc->init_proc = init_proc_POWER7;
>     pcc->check_pow = check_pow_nocheck;
>     pcc->insns_flags = PPC_INSNS_BASE | PPC_ISEL | PPC_STRING | PPC_MFTB |
> @@ -7309,7 +7311,7 @@ static void init_ppc_proc(PowerPCCPU *cpu)
> #endif
>                  SPR_NOACCESS,
>                  &spr_read_generic, SPR_NOACCESS,
> -                 pcc->pvr);
> +                 pcc->pvr_default ? pcc->pvr_default : pcc->pvr);

The automatically generated host class should just take on the host PVR value, 
so there's no need for jumping through hoops here.

This patch is also missing the matching part :).


Alex

>     /* Register SVR if it's defined to anything else than POWERPC_SVR_NONE */
>     if (pcc->svr != POWERPC_SVR_NONE) {
>         if (pcc->svr & POWERPC_SVR_E500) {
> @@ -8553,6 +8555,9 @@ static void ppc_cpu_class_init(ObjectClass *oc, void 
> *data)
>     DeviceClass *dc = DEVICE_CLASS(oc);
> 
>     pcc->parent_realize = dc->realize;
> +    pcc->pvr = CPU_POWERPC_DEFAULT_MASK;
> +    pcc->pvr_mask = 0;
> +    pcc->pvr_default = 0;
>     dc->realize = ppc_cpu_realizefn;
>     dc->unrealize = ppc_cpu_unrealizefn;
> 
> -- 
> 1.8.3.2
>