Re: [PATCH v3 1/7] target/ppc: Created !TCG SPR registration macro

[Richard Henderson]

On 4/30/21 12:35 PM, Bruno Larsen (billionai) wrote:
> moved RW callback parameters of _spr_register into an ifdef, to support
> building without TCG in the future, and added definitions for
> spr_register and spr_register_kvm, to keep the same call regardless of
> build options
>
> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> ---
>   target/ppc/translate_init.c.inc | 26 +++++++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 6235eb7536..22b23793fd 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -720,6 +720,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val)
>       helper_mtvscr(env, val);
>   }
>   
> +#ifdef CONFIG_TCG
>   #ifdef CONFIG_USER_ONLY
>   #define spr_register_kvm(env, num, name, uea_read, uea_write,                 
>  \
>                            oea_read, oea_write, one_reg_id, initial_value)      
>  \
> @@ -728,7 +729,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val)
>                               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
> +#else /* CONFIG_USER_ONLY */
>   #if !defined(CONFIG_KVM)
>   #define spr_register_kvm(env, num, name, uea_read, uea_write,                 
>  \
>                            oea_read, oea_write, one_reg_id, initial_value)      
>  \
> @@ -739,7 +740,7 @@ static inline void vscr_init(CPUPPCState *env, uint32_t val)
>                               one_reg_id, initial_value)                        
>  \
>       _spr_register(env, num, name, uea_read, uea_write,                        
>  \
>                     oea_read, oea_write, hea_read, hea_write, initial_value)
> -#else
> +#else /* CONFIG_KVM */
>   #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,                        
>  \
> @@ -751,8 +752,21 @@ static inline void vscr_init(CPUPPCState *env, uint32_t 
> val)
>       _spr_register(env, num, name, uea_read, uea_write,                        
>  \
>                     oea_read, oea_write, hea_read, hea_write,                   
>  \
>                     one_reg_id, initial_value)
> -#endif
> -#endif
> +#endif /* CONFIG_KVM */
> +#endif /* CONFIG_USER_ONLY */
> +#else /* CONFIG_TCG */
> +#ifdef CONFIG_KVM /* sanity check. should always enter this */
> +#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, 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, one_reg_id, initial_value)
> +#else /* CONFIG_KVM */
> +#error "Either TCG or KVM should be configured"
> +#endif /* CONFIG_KVM */
> +#endif /* CONFIG_TCG */

I think this ifdef tree, and the repetition, is unnecessarily confusing.  How 
about something like this?

r~

patch.txt
Description: Text document