On Tue, Aug 24, 2021 at 01:30:18PM -0300, Daniel Henrique Barboza wrote: > From: Gustavo Romero <grom...@linux.ibm.com> > > This patch adds handling of UMMCR0 and UMMCR2 user read which, > according to PowerISA 3.1, has some bits ommited to the
Nit: One 'm' in "omited".
> userspace.
>
> CC: Gustavo Romero <gustavo.rom...@linaro.org>
> Signed-off-by: Gustavo Romero <grom...@linux.ibm.com>
> Signed-off-by: Daniel Henrique Barboza <danielhb...@gmail.com>
> ---
> target/ppc/cpu.h | 5 +++++
> target/ppc/cpu_init.c | 4 ++--
> target/ppc/spr_tcg.h | 2 ++
> target/ppc/translate.c | 37 +++++++++++++++++++++++++++++++++++++
> 4 files changed, 46 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 627fc8d732..739005ba29 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -343,6 +343,11 @@ typedef struct ppc_v3_pate_t {
> #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 */
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index c72c7fabea..5510c3799f 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6868,7 +6868,7 @@ 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_write_PMU_groupA_ureg,
> + &spr_read_MMCR0_ureg, &spr_write_PMU_groupA_ureg,
Hrm.. so combined with the previous patch this means that userspace
can write any bit in MMCR0, but only read some of them. Is that
really correct?
> &spr_read_ureg, &spr_write_ureg,
> 0x00000000);
> spr_register(env, SPR_POWER_UMMCR1, "UMMCR1",
> @@ -6976,7 +6976,7 @@ 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_write_PMU_groupA_ureg,
> + &spr_read_MMCR2_ureg, &spr_write_PMU_groupA_ureg,
> &spr_read_ureg, &spr_write_ureg,
> 0x00000000);
> spr_register(env, SPR_POWER_USIER, "USIER",
> diff --git a/target/ppc/spr_tcg.h b/target/ppc/spr_tcg.h
> index 027ec4c3f7..64ef2cd089 100644
> --- a/target/ppc/spr_tcg.h
> +++ b/target/ppc/spr_tcg.h
> @@ -32,6 +32,8 @@ void spr_write_lr(DisasContext *ctx, int sprn, int gprn);
> void spr_read_ctr(DisasContext *ctx, int gprn, int sprn);
> void spr_write_ctr(DisasContext *ctx, int sprn, int gprn);
> void spr_read_ureg(DisasContext *ctx, int gprn, int sprn);
> +void spr_read_MMCR0_ureg(DisasContext *ctx, int gprn, int sprn);
> +void spr_read_MMCR2_ureg(DisasContext *ctx, int gprn, int sprn);
> void spr_read_tbl(DisasContext *ctx, int gprn, int sprn);
> void spr_read_tbu(DisasContext *ctx, int gprn, int sprn);
> void spr_read_atbl(DisasContext *ctx, int gprn, int sprn);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 3a1eafbba8..ec4160378d 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -520,6 +520,43 @@ void spr_read_ureg(DisasContext *ctx, int gprn, int sprn)
> gen_load_spr(cpu_gpr[gprn], sprn + 0x10);
> }
>
> +void spr_read_MMCR0_ureg(DisasContext *ctx, int gprn, int sprn)
> +{
> + TCGv t0 = tcg_temp_new();
> +
> + /*
> + * 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, SPR_POWER_MMCR0);
> + tcg_gen_andi_tl(t0, t0, MMCR0_FC | MMCR0_PMAO | MMCR0_PMAE);
I think #defining this mask somewhere would be worthwhile.
> + tcg_gen_mov_tl(cpu_gpr[gprn], t0);
> +
> + tcg_temp_free(t0);
> +}
> +
> +void spr_read_MMCR2_ureg(DisasContext *ctx, int gprn, int sprn)
> +{
> + TCGv t0 = tcg_temp_new();
> +
> + /*
> + * 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, SPR_POWER_MMCR2);
> + tcg_gen_andi_tl(t0, t0, 0x4020100804020000UL);
Even more so here, where it's a big bare literal.
> + tcg_gen_mov_tl(cpu_gpr[gprn], t0);
> +
> + 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
signature.asc
Description: PGP signature
