qemu-ppc
[Top][All Lists]
Advanced

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

Re: [PATCH 02/19] target/ppc: add exclusive user read function for PMU r


From: David Gibson
Subject: Re: [PATCH 02/19] target/ppc: add exclusive user read function for PMU regs
Date: Tue, 10 Aug 2021 13:21:48 +1000

On Mon, Aug 09, 2021 at 10:10:40AM -0300, Daniel Henrique Barboza wrote:
> From: Gustavo Romero <gromero@linux.ibm.com>
> 
> User read on PowerPC PMU regs requires extra handling in some
> instances. Instead of changing the existing read ureg function
> (spr_read_ureg) this patch adds a specific read function for
> user PMU SPRs, spr_read_pmu_ureg().
> 
> This function does extra handling of UMMCR0 and UMMCR2 and falls
> back to the default behavior for the not yet handled regs. Aside
> for UMMCR0 and UMMCR2 reads, no functional change is made.
> 
> CC: Gustavo Romero <gustavo.romero@linaro.org>
> Signed-off-by: Gustavo Romero <gromero@linux.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  target/ppc/cpu.h       |  8 ++++++++
>  target/ppc/cpu_init.c  | 26 +++++++++++++-------------
>  target/ppc/spr_tcg.h   |  1 +
>  target/ppc/translate.c | 41 +++++++++++++++++++++++++++++++++++++++--
>  4 files changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 500205229c..4d96015f81 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -342,6 +342,14 @@ typedef struct ppc_v3_pate_t {
>  #define MSR_RI   1  /* Recoverable interrupt                        1        
> */
>  #define MSR_LE   0  /* Little-endian mode                           1 hflags 
> */
>  
> +/* PMU bits */
> +#define MMCR0_FC    PPC_BIT(32)         /* Freeze Counters  */
> +#define MMCR0_PMAO  PPC_BIT(56)         /* Perf Monitor Alert Ocurred */
> +#define MMCR0_PMAE  PPC_BIT(37)         /* Perf Monitor Alert Enable */
> +#define MMCR0_EBE   PPC_BIT(43)         /* Perf Monitor EBB Enable */
> +#define MMCR0_FCECE PPC_BIT(38)         /* FC on Enabled Cond or Event */
> +#define MMCR0_PMCC  PPC_BITMASK(44, 45) /* PMC Control */
> +
>  /* LPCR bits */
>  #define LPCR_VPM0         PPC_BIT(0)
>  #define LPCR_VPM1         PPC_BIT(1)
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 021c1bc750..d30aa0fe1e 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6868,47 +6868,47 @@ static void register_book3s_pmu_sup_sprs(CPUPPCState 
> *env)
>  static void register_book3s_pmu_user_sprs(CPUPPCState *env)
>  {
>      spr_register(env, SPR_POWER_UMMCR0, "UMMCR0",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UMMCRA, "UMMCRA",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UPMC1, "UPMC1",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UPMC2, "UPMC2",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UPMC3, "UPMC3",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UPMC4, "UPMC4",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UPMC5, "UPMC5",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_UPMC6, "UPMC6",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_USIAR, "USIAR",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_USDAR, "USDAR",
> -                 &spr_read_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
>                   &spr_read_ureg, &spr_write_ureg,
>                   0x00000000);
>  }
> @@ -6976,8 +6976,8 @@ static void register_power8_pmu_sup_sprs(CPUPPCState 
> *env)
>  static void register_power8_pmu_user_sprs(CPUPPCState *env)
>  {
>      spr_register(env, SPR_POWER_UMMCR2, "UMMCR2",
> -                 &spr_read_ureg, SPR_NOACCESS,
> -                 &spr_read_ureg, &spr_write_ureg,
> +                 &spr_read_pmu_ureg, SPR_NOACCESS,
> +                 &spr_read_pmu_ureg, &spr_write_ureg,
>                   0x00000000);
>      spr_register(env, SPR_POWER_USIER, "USIER",
>                   &spr_read_generic, SPR_NOACCESS,
> diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
> index 2aab5878a0..84ecba220f 100644
> --- a/target/ppc/spr_tcg.h
> +++ b/target/ppc/spr_tcg.h
> @@ -27,6 +27,7 @@ void spr_read_generic(DisasContext *ctx, int gprn, int 
> sprn);
>  void spr_write_generic(DisasContext *ctx, int sprn, int gprn);
>  void spr_read_pmu_generic(DisasContext *ctx, int gprn, int sprn);
>  void spr_write_pmu_generic(DisasContext *ctx, int sprn, int gprn);
> +void spr_read_pmu_ureg(DisasContext *ctx, int gprn, int sprn);
>  void spr_read_xer(DisasContext *ctx, int gprn, int sprn);
>  void spr_write_xer(DisasContext *ctx, int sprn, int gprn);
>  void spr_read_lr(DisasContext *ctx, int gprn, int sprn);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index c8f3878002..d3a4d42ff8 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -522,8 +522,6 @@ void spr_write_ctr(DisasContext *ctx, int sprn, int gprn)
>  
>  /* User read access to SPR */
>  /* USPRx */
> -/* UMMCRx */
> -/* UPMCx */
>  /* USIA */
>  /* UDECR */
>  void spr_read_ureg(DisasContext *ctx, int gprn, int sprn)
> @@ -531,6 +529,45 @@ void spr_read_ureg(DisasContext *ctx, int gprn, int sprn)
>      gen_load_spr(cpu_gpr[gprn], sprn + 0x10);
>  }
>  
> +/* User special read access to PMU SPRs */
> +void spr_read_pmu_ureg(DisasContext *ctx, int gprn, int sprn)
> +{
> +    TCGv t0 = tcg_temp_new();
> +    int effective_sprn = sprn + 0x10;
> +
> +    switch (effective_sprn) {
> +    case SPR_POWER_MMCR0:

As with patch 1, it seems silly to multiplex here when we already have
something to dispatch to specific logic for each register.  In general
stacked multiplexers are a code smell, if there's not some significant
shared logic at each level.

> +        /*
> +         * Filter out all bits but FC, PMAO, and PMAE, according
> +         * to ISA v3.1, in 10.4.4 Monitor Mode Control Register 0,
> +         * fourth paragraph.
> +         */
> +        gen_load_spr(t0, effective_sprn);
> +        tcg_gen_andi_tl(t0, t0, MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE);
> +        tcg_gen_mov_tl(cpu_gpr[gprn], t0);
> +        break;
> +    case SPR_POWER_MMCR2:
> +        /*
> +         * On read, filter out all bits that are not FCnP0 bits.
> +         * When MMCR0[PMCC] is set to 0b10 or 0b11, providing
> +         * problem state programs read/write access to MMCR2,
> +         * only the FCnP0 bits can be accessed. All other bits are
> +         * not changed when mtspr is executed in problem state, and
> +         * all other bits return 0s when mfspr is executed in problem
> +         * state, according to ISA v3.1, section 10.4.6 Monitor Mode
> +         * Control Register 2, p. 1316, third paragraph.
> +         */
> +        gen_load_spr(t0, effective_sprn);
> +        tcg_gen_andi_tl(t0, t0, 0x4020100804020000UL);
> +        tcg_gen_mov_tl(cpu_gpr[gprn], t0);
> +        break;
> +    default:
> +        gen_load_spr(cpu_gpr[gprn], effective_sprn);
> +    }
> +
> +    tcg_temp_free(t0);
> +}
> +
>  #if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
>  void spr_write_ureg(DisasContext *ctx, int sprn, int gprn)
>  {

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


reply via email to

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