qemu-ppc
[Top][All Lists]
Advanced

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

Re: [Qemu-ppc] [PATCH RFC] target-ppc: support ibm, pa-features device p


From: Alexander Graf
Subject: Re: [Qemu-ppc] [PATCH RFC] target-ppc: support ibm, pa-features device property
Date: Thu, 6 Feb 2014 15:51:52 +0100

On 28.01.2014, at 09:03, Aneesh Kumar K.V <address@hidden> wrote:

> We will use this later to disable Transactional memory in case of PR KVM
> 
> Signed-off-by: Aneesh Kumar K.V <address@hidden>
> ---
> NOTE:
> PPC2_TM value may need update before merging this.
> 
> hw/ppc/spapr.c              | 42 ++++++++++++++++++++++++++++++++++++++++++
> target-ppc/cpu.h            |  2 ++
> target-ppc/kvm_ppc.h        | 10 ++++++++++
> target-ppc/translate_init.c |  2 +-
> 4 files changed, 55 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 9241cdd2a86e..e6e3fc0a1b55 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -287,6 +287,46 @@ static size_t create_page_sizes_prop(CPUPPCState *env, 
> uint32_t *prop,
>     } while (0)
> 
> 
> +#define MAX_PA_FEATURES 23

What does this mean?

> +static void spapr_pa_features_skel(CPUState *cs, void *fdt)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    char pa_features[2 + MAX_PA_FEATURES];
> +
> +    /*
> +     * We are currently using this only to disable P8 TM on PR.
> +     * So don't add the property for any other case
> +     */
> +    if (!(env->insns_flags2 & PPC2_TM) || !kvm_is_pr(cs)) {

Better fetch a CAP from KVM to check whether it supports TM. Then we only need 
a patch in the kernel later to enable it.

> +        return;

How about the other cases? Why don't we always provide this node? How hard is 
it to simply implement TM in PR KVM? Can we play dumb and always fail 
transactions?

> +    }
> +    memset(pa_features, 0, sizeof(pa_features));
> +    pa_features[0] = MAX_PA_FEATURES;
> +    pa_features[1] = 0; /* We only support a attribute specifier type of 0 */
> +
> +    /*
> +     * Linux kernel only looks at the below feature set
> +     * Big Endian/Little Endian
> +     */
> +    pa_features[2] |= 1 << 7; /* PPC_FEATURE_HAS_MMU */
> +    pa_features[2] |= 1 << 6; /* PPC_FEATURE_HAS_FPU */
> +    pa_features[2] |= 1 << 5; /* MMU_FTR_SLB */
> +    pa_features[2] |= 1 << 4; /* CPU_FTR_CTRL */
> +    pa_features[2] |= 1 << 1; /* CPU_FTR_NOEXECUTE */
> +
> +    pa_features[3] |= 1 << 6; /* CPU_FTR_NODSISRALIGN */
> +    pa_features[3] |= 1 << 5; /* MMU_FTR_CI_LARGE_PAGE */
> +
> +    pa_features[7] |= 1 << 7; /* CPU_FTR_REAL_LE */

Any particular reason you give all those nice readable names, but don't just 
use them as defines? I'm sure you can also derive most of them from feature 
flags we already know about the CPU.

> +
> +    /* pa_features[24] CPU_FTR_TM is disabled */
> +
> +    _FDT((fdt_property(fdt, "ibm,pa-features",
> +                       pa_features, sizeof(pa_features))));
> +    return;
> +}
> +
> static void *spapr_create_fdt_skel(hwaddr initrd_base,
>                                    hwaddr initrd_size,
>                                    hwaddr kernel_size,
> @@ -463,6 +503,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>                                page_sizes_prop, page_sizes_prop_size)));
>         }
> 
> +        spapr_pa_features_skel(cs, fdt);
> +
>         _FDT((fdt_end_node(fdt)));
>     }
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index b0f66e5104dd..cf83bc6c0e0b 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1876,6 +1876,8 @@ enum {
>     PPC2_DBRX          = 0x0000000000000010ULL,
>     /* Book I 2.05 PowerPC specification                                     
> */
>     PPC2_ISA205        = 0x0000000000000020ULL,
> +    /* Transaction Memory                                                    
> */
> +    PPC2_TM            = 0x0000000000000040ULL,
> 
> #define PPC_TCG_INSNS2 (PPC2_BOOKE206 | PPC2_VSX | PPC2_PRCNTL | PPC2_DBRX | \
>   PPC2_ISA205)
> diff --git a/target-ppc/kvm_ppc.h b/target-ppc/kvm_ppc.h
> index a65d34571914..a7c9384efc95 100644
> --- a/target-ppc/kvm_ppc.h
> +++ b/target-ppc/kvm_ppc.h
> @@ -14,6 +14,7 @@
> void kvmppc_init(void);
> 
> #ifdef CONFIG_KVM
> +#include "sysemu/kvm.h"
> 
> uint32_t kvmppc_get_tbfreq(void);
> uint64_t kvmppc_get_clockfreq(void);
> @@ -35,6 +36,11 @@ void *kvmppc_create_spapr_tce(uint32_t liobn, uint32_t 
> window_size, int *pfd);
> int kvmppc_remove_spapr_tce(void *table, int pfd, uint32_t window_size);
> int kvmppc_reset_htab(int shift_hint);
> uint64_t kvmppc_rma_size(uint64_t current_size, unsigned int hash_shift);
> +static inline int kvm_is_pr(CPUState *cs)
> +{
> +    return (kvm_check_extension(cs->kvm_state, KVM_CAP_PPC_GET_PVINFO) == 1);

There's no reason HV KVM couldn't implement PVINFO.

> +}
> +
> #endif /* !CONFIG_USER_ONLY */
> int kvmppc_fixup_cpu(PowerPCCPU *cpu);
> bool kvmppc_has_cap_epr(void);
> @@ -159,6 +165,10 @@ static inline int kvmppc_update_sdr1(CPUPPCState *env)
>     return 0;
> }
> 
> +static inline int kvm_is_pr(CPUState *cs)
> +{
> +    return false;
> +}
> #endif /* !CONFIG_USER_ONLY */
> 
> static inline int kvmppc_fixup_cpu(PowerPCCPU *cpu)
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index c030a2032a0f..f41e6b1bdd32 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -7312,7 +7312,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>                        PPC_64B | PPC_ALTIVEC |
>                        PPC_SEGMENT_64B | PPC_SLBI |
>                        PPC_POPCNTB | PPC_POPCNTWD;
> -    pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX;
> +    pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_TM;

This means that with TCG we break with -cpu POWER8, no?


Alex

>     pcc->msr_mask = 0x800000000284FF36ULL;
>     pcc->mmu_model = POWERPC_MMU_2_06;
> #if defined(CONFIG_SOFTMMU)
> -- 
> 1.8.5.3
> 




reply via email to

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