On Tue Apr 25, 2023 at 12:47 AM AEST, Harsh Prateek Bora wrote:
> The bits in cr reg are grouped into eight 4-bit fields represented
> by env->crf[8] and the related calculations should be abstracted to
> keep the calling routines simpler to read. This is a step towards
> cleaning up the [h_enter|spapr_exit]_nested calls for better readability.
>
> Signed-off-by: Harsh Prateek Bora <hars...@linux.ibm.com>
> Reviewed-by: Fabiano Rosas <faro...@suse.de>
> ---
> hw/ppc/spapr_hcall.c | 18 ++----------------
Could you either convert all callers, or do implementation and
conversion as separate patches. Preference for former if you can
be bothered.
save_user_regs(), restore_user_regs(), gdb read/write register * 2,
kvm_arch_get/put_registers, monitor_get_ccr, at a quick glance.
> target/ppc/cpu.c | 17 +++++++++++++++++
> target/ppc/cpu.h | 2 ++
> 3 files changed, 21 insertions(+), 16 deletions(-)
>
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index ec4def62f8..124cee5e53 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
[snip]
> diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c
> index 1a97b41c6b..3b444e58b5 100644
> --- a/target/ppc/cpu.c
> +++ b/target/ppc/cpu.c
> @@ -67,6 +67,23 @@ uint32_t ppc_get_vscr(CPUPPCState *env)
> return env->vscr | (sat << VSCR_SAT);
> }
>
> +void ppc_store_cr(CPUPPCState *env, uint64_t cr)
Set is normal counterpart to get. Or load and store, but
I think set and get is probably better.
Good refactoring though, it shouldn't be open-coded everywhere.
Thanks,
Nick
> +{
> + for (int i = 7; i >= 0; i--) {
> + env->crf[i] = cr & 15;
> + cr >>= 4;
> + }
> +}
> +
> +uint64_t ppc_get_cr(CPUPPCState *env)
> +{
> + uint64_t cr = 0;
> + for (int i = 0; i < 8; i++) {
> + cr |= (env->crf[i] & 15) << (4 * (7 - i));
> + }
> + return cr;
> +}
> +
> /* GDBstub can read and write MSR... */
> void ppc_store_msr(CPUPPCState *env, target_ulong value)
> {
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 557d736dab..b4c21459f1 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2773,6 +2773,8 @@ void dump_mmu(CPUPPCState *env);
> void ppc_maybe_bswap_register(CPUPPCState *env, uint8_t *mem_buf, int len);
> void ppc_store_vscr(CPUPPCState *env, uint32_t vscr);
> uint32_t ppc_get_vscr(CPUPPCState *env);
> +void ppc_store_cr(CPUPPCState *env, uint64_t cr);
> +uint64_t ppc_get_cr(CPUPPCState *env);
>
>
> /*****************************************************************************/
> /* Power management enable checks
> */
> --
> 2.31.1
