Re: [PATCH v3 2/6] target/ppc: Reorder #ifdef'ry in kvm_ppc.h

[Greg Kurz]

On Tue, 27 Jun 2023 13:51:20 +0200
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:

> Keep a single if/else/endif block checking CONFIG_KVM.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  target/ppc/kvm_ppc.h | 62 ++++++++++++++++++++------------------------
>  1 file changed, 28 insertions(+), 34 deletions(-)
> 
> diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
> index 2e395416f0..49954a300b 100644
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@ -93,7 +93,34 @@ void kvmppc_set_reg_tb_offset(PowerPCCPU *cpu, int64_t 
> tb_offset);
>  
>  int kvm_handle_nmi(PowerPCCPU *cpu, struct kvm_run *run);
>  
> -#else
> +#define kvmppc_eieio() \
> +    do {                                          \
> +        if (kvm_enabled()) {                          \
> +            asm volatile("eieio" : : : "memory"); \
> +        } \
> +    } while (0)
> +
> +/* Store data cache blocks back to memory */
> +static inline void kvmppc_dcbst_range(PowerPCCPU *cpu, uint8_t *addr, int 
> len)
> +{
> +    uint8_t *p;
> +
> +    for (p = addr; p < addr + len; p += cpu->env.dcache_line_size) {
> +        asm volatile("dcbst 0,%0" : : "r"(p) : "memory");
> +    }
> +}
> +
> +/* Invalidate instruction cache blocks */
> +static inline void kvmppc_icbi_range(PowerPCCPU *cpu, uint8_t *addr, int len)
> +{
> +    uint8_t *p;
> +
> +    for (p = addr; p < addr + len; p += cpu->env.icache_line_size) {
> +        asm volatile("icbi 0,%0" : : "r"(p));
> +    }
> +}
> +
> +#else /* !CONFIG_KVM */
>  
>  static inline uint32_t kvmppc_get_tbfreq(void)
>  {
> @@ -440,10 +467,6 @@ static inline bool 
> kvmppc_pvr_workaround_required(PowerPCCPU *cpu)
>      return false;
>  }
>  
> -#endif
> -
> -#ifndef CONFIG_KVM
> -
>  #define kvmppc_eieio() do { } while (0)
>  
>  static inline void kvmppc_dcbst_range(PowerPCCPU *cpu, uint8_t *addr, int 
> len)
> @@ -454,35 +477,6 @@ static inline void kvmppc_icbi_range(PowerPCCPU *cpu, 
> uint8_t *addr, int len)
>  {
>  }
>  
> -#else   /* CONFIG_KVM */
> -
> -#define kvmppc_eieio() \

Arguably the kvm and non-kvm implementations will now come from
different commits in git blame. I personally favor keeping the
git blame consistency over bare code movement that doesn't fix
any actual bug.

Also this patch doesn't seem to be strictly needed to reach the
goal of kicking "kvm_ppc.h" out of user emulation.

> -    do {                                          \
> -        if (kvm_enabled()) {                          \
> -            asm volatile("eieio" : : : "memory"); \
> -        } \
> -    } while (0)
> -
> -/* Store data cache blocks back to memory */
> -static inline void kvmppc_dcbst_range(PowerPCCPU *cpu, uint8_t *addr, int 
> len)
> -{
> -    uint8_t *p;
> -
> -    for (p = addr; p < addr + len; p += cpu->env.dcache_line_size) {
> -        asm volatile("dcbst 0,%0" : : "r"(p) : "memory");
> -    }
> -}
> -
> -/* Invalidate instruction cache blocks */
> -static inline void kvmppc_icbi_range(PowerPCCPU *cpu, uint8_t *addr, int len)
> -{
> -    uint8_t *p;
> -
> -    for (p = addr; p < addr + len; p += cpu->env.icache_line_size) {
> -        asm volatile("icbi 0,%0" : : "r"(p));
> -    }
> -}
> -
>  #endif  /* CONFIG_KVM */
>  
>  #endif /* KVM_PPC_H */



-- 
Greg