[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/17] ppc: Add macros to register hypervisor mo
From: |
Thomas Huth |
Subject: |
Re: [Qemu-devel] [PATCH 02/17] ppc: Add macros to register hypervisor mode SPRs |
Date: |
Mon, 14 Mar 2016 19:50:15 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 |
On 14.03.2016 17:56, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <address@hidden>
>
> The current set of spr_register_* macros only take the user and
> supervisor function pointers. To make the transition easy, we
> don't change that but we add "_hv" variants that can be used to
> register all 3 sets.
>
> To simplify the transition, users of the "old" macro will set the
> hypervisor callback to be the same as the supervisor one. The new
> registration function only needs to be used for registers that are
> either hypervisor only or behave differently in HV mode.
>
> Signed-off-by: Benjamin Herrenschmidt <address@hidden>
> Reviewed-by: David Gibson <address@hidden>
> ---
> target-ppc/translate.c | 26 ++++++++++++++++----------
> target-ppc/translate_init.c | 35 +++++++++++++++++++++++++++++++----
> 2 files changed, 47 insertions(+), 14 deletions(-)
>
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index e402ff920314..327f3259b4be 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -4282,14 +4282,17 @@ static inline void gen_op_mfspr(DisasContext *ctx)
> void (*read_cb)(DisasContext *ctx, int gprn, int sprn);
> uint32_t sprn = SPR(ctx->opcode);
>
> -#if !defined(CONFIG_USER_ONLY)
> - if (ctx->hv)
> +#if defined(CONFIG_USER_ONLY)
> + read_cb = ctx->spr_cb[sprn].uea_read;
> +#else
> + if (ctx->pr) {
> + read_cb = ctx->spr_cb[sprn].uea_read;
> + } else if (ctx->hv) {
> read_cb = ctx->spr_cb[sprn].hea_read;
> - else if (!ctx->pr)
> + } else if (!ctx->pr) {
That check for !ctx->pr is now superfluous, isn't it? ... because it has
already been checked 4 lines earlier.
> read_cb = ctx->spr_cb[sprn].oea_read;
> - else
> + }
> #endif
> - read_cb = ctx->spr_cb[sprn].uea_read;
> if (likely(read_cb != NULL)) {
> if (likely(read_cb != SPR_NOACCESS)) {
> (*read_cb)(ctx, rD(ctx->opcode), sprn);
> @@ -4437,14 +4440,17 @@ static void gen_mtspr(DisasContext *ctx)
> void (*write_cb)(DisasContext *ctx, int sprn, int gprn);
> uint32_t sprn = SPR(ctx->opcode);
>
> -#if !defined(CONFIG_USER_ONLY)
> - if (ctx->hv)
> +#if defined(CONFIG_USER_ONLY)
> + write_cb = ctx->spr_cb[sprn].uea_write;
> +#else
> + if (ctx->pr) {
> + write_cb = ctx->spr_cb[sprn].uea_write;
> + } else if (ctx->hv) {
> write_cb = ctx->spr_cb[sprn].hea_write;
> - else if (!ctx->pr)
> + } else {
Here it is right already :-)
> write_cb = ctx->spr_cb[sprn].oea_write;
> - else
> + }
> #endif
> - write_cb = ctx->spr_cb[sprn].uea_write;
> if (likely(write_cb != NULL)) {
> if (likely(write_cb != SPR_NOACCESS)) {
> (*write_cb)(ctx, sprn, rS(ctx->opcode));
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index fb206aff29ad..6a11b41206e5 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -579,17 +579,33 @@ static inline void vscr_init (CPUPPCState *env,
> uint32_t val)
> #define spr_register_kvm(env, num, name, uea_read, uea_write,
> \
> oea_read, oea_write, one_reg_id, initial_value)
> \
> _spr_register(env, num, name, uea_read, uea_write, initial_value)
> +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, hea_read, hea_write,
> \
> + one_reg_id, initial_value)
> \
> + _spr_register(env, num, name, uea_read, uea_write, initial_value)
> #else
> #if !defined(CONFIG_KVM)
> #define spr_register_kvm(env, num, name, uea_read, uea_write,
> \
> - oea_read, oea_write, one_reg_id, initial_value) \
> + oea_read, oea_write, one_reg_id, initial_value)
> \
> + _spr_register(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, oea_read, oea_write, initial_value)
> +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, hea_read, hea_write,
> \
> + one_reg_id, initial_value)
> \
> _spr_register(env, num, name, uea_read, uea_write,
> \
> - oea_read, oea_write, initial_value)
> + oea_read, oea_write, hea_read, hea_write, initial_value)
> #else
> #define spr_register_kvm(env, num, name, uea_read, uea_write,
> \
> - oea_read, oea_write, one_reg_id, initial_value) \
> + oea_read, oea_write, one_reg_id, initial_value)
> \
> + _spr_register(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, oea_read, oea_write,
> \
> + one_reg_id, initial_value)
> +#define spr_register_kvm_hv(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, hea_read, hea_write,
> \
> + one_reg_id, initial_value)
> \
> _spr_register(env, num, name, uea_read, uea_write,
> \
> - oea_read, oea_write, one_reg_id, initial_value)
> + oea_read, oea_write, hea_read, hea_write,
> \
> + one_reg_id, initial_value)
> #endif
> #endif
>
> @@ -598,6 +614,13 @@ static inline void vscr_init (CPUPPCState *env, uint32_t
> val)
> spr_register_kvm(env, num, name, uea_read, uea_write,
> \
> oea_read, oea_write, 0, initial_value)
>
> +#define spr_register_hv(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, hea_read, hea_write,
> \
> + initial_value)
> \
> + spr_register_kvm_hv(env, num, name, uea_read, uea_write,
> \
> + oea_read, oea_write, hea_read, hea_write,
> \
> + 0, initial_value)
> +
> static inline void _spr_register(CPUPPCState *env, int num,
> const char *name,
> void (*uea_read)(DisasContext *ctx, int
> gprn, int sprn),
> @@ -606,6 +629,8 @@ static inline void _spr_register(CPUPPCState *env, int
> num,
>
> void (*oea_read)(DisasContext *ctx, int
> gprn, int sprn),
> void (*oea_write)(DisasContext *ctx, int
> sprn, int gprn),
> + void (*hea_read)(DisasContext *opaque, int
> gprn, int sprn),
> + void (*hea_write)(DisasContext *opaque, int
> sprn, int gprn),
> #endif
> #if defined(CONFIG_KVM)
> uint64_t one_reg_id,
> @@ -633,6 +658,8 @@ static inline void _spr_register(CPUPPCState *env, int
> num,
> #if !defined(CONFIG_USER_ONLY)
> spr->oea_read = oea_read;
> spr->oea_write = oea_write;
> + spr->hea_read = hea_read;
> + spr->hea_write = hea_write;
> #endif
> #if defined(CONFIG_KVM)
> spr->one_reg_id = one_reg_id,
Apart from the one superfluous if-statement, the patch looks fine to me.
Thomas
- [Qemu-devel] [PATCH 00/17] ppc: preparing pnv landing, Cédric Le Goater, 2016/03/14
- [Qemu-devel] [PATCH 01/17] ppc: Update SPR definitions, Cédric Le Goater, 2016/03/14
- [Qemu-devel] [PATCH 06/17] ppc: Create cpu_ppc_set_papr() helper, Cédric Le Goater, 2016/03/14
- [Qemu-devel] [PATCH 02/17] ppc: Add macros to register hypervisor mode SPRs, Cédric Le Goater, 2016/03/14
- Re: [Qemu-devel] [PATCH 02/17] ppc: Add macros to register hypervisor mode SPRs,
Thomas Huth <=
- [Qemu-devel] [PATCH 07/17] ppc: Better figure out if processor has HV mode, Cédric Le Goater, 2016/03/14
- [Qemu-devel] [PATCH 03/17] ppc: Add a bunch of hypervisor SPRs to Book3s, Cédric Le Goater, 2016/03/14
[Qemu-devel] [PATCH 05/17] ppc: Fix hreg_store_msr() so that non-HV mode cannot alter MSR:HV, Cédric Le Goater, 2016/03/14