[v2] target/ppc: mmu cleanup

[PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop

Posted by Lucas Mateus Castro (alqotel) 4 years, 8 months ago

The function ppc_tlb_invalid_all is now a no op when compiling without TCG.

Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
---
 target/ppc/mmu_helper.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 1ecb36e85a..e7ba39c9e1 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -825,6 +825,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
     return ret;
 }
 
+#ifdef CONFIG_TCG
 static void booke206_flush_tlb(CPUPPCState *env, int flags,
                                const int check_iprot)
 {
@@ -846,6 +847,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
 
     tlb_flush(env_cpu(env));
 }
+#endif
 
 static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
                                         ppcmas_tlb_t *tlb)
@@ -1956,6 +1958,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, target_ulong value)
 /* TLB management */
 void ppc_tlb_invalidate_all(CPUPPCState *env)
 {
+#ifdef CONFIG_TCG
 #if defined(TARGET_PPC64)
     if (mmu_is_64bit(env->mmu_model)) {
         env->tlb_need_flush = 0;
@@ -1994,6 +1997,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
         cpu_abort(env_cpu(env), "Unknown MMU model %x\n", env->mmu_model);
         break;
     }
+#endif
 }
 
 #ifdef CONFIG_TCG
-- 
2.17.1

Re: [PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop

Posted by Bruno Piazera Larsen 4 years, 8 months ago

On 10/06/2021 13:46, Lucas Mateus Castro (alqotel) wrote:
> The function ppc_tlb_invalid_all is now a no op when compiling without TCG.
>
> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
> ---
>   target/ppc/mmu_helper.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 1ecb36e85a..e7ba39c9e1 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -825,6 +825,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>       return ret;
>   }
>   
> +#ifdef CONFIG_TCG
>   static void booke206_flush_tlb(CPUPPCState *env, int flags,
>                                  const int check_iprot)
>   {
> @@ -846,6 +847,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
>   
>       tlb_flush(env_cpu(env));
>   }
> +#endif
I think you could use ATTRIBUTE_UNUSED instead of ifdefs here. Not sure 
which would be preferable in this case, but IMHO unused is a bit more 
informative as to what is happening...
>   
>   static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
>                                           ppcmas_tlb_t *tlb)
> @@ -1956,6 +1958,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, target_ulong value)
>   /* TLB management */
>   void ppc_tlb_invalidate_all(CPUPPCState *env)
>   {
> +#ifdef CONFIG_TCG
>   #if defined(TARGET_PPC64)
>       if (mmu_is_64bit(env->mmu_model)) {
>           env->tlb_need_flush = 0;
> @@ -1994,6 +1997,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
>           cpu_abort(env_cpu(env), "Unknown MMU model %x\n", env->mmu_model);
>           break;
>       }
> +#endif
>   }
>   
>   #ifdef CONFIG_TCG
-- 
Bruno Piazera Larsen
Instituto de Pesquisas ELDORADO 
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Analista de Software Trainee
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop

Posted by David Gibson 4 years, 8 months ago

On Thu, Jun 10, 2021 at 01:46:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
> The function ppc_tlb_invalid_all is now a no op when compiling without TCG.
> 
> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>


Hm, I think I suggested making ppc_tlb_invalidate_all() a stub, rather
than removing the call from !TCG code.  But looking at this again, I
think that was a mistake.  I think it makes more sense to suppress
this on the caller side, and make this a TCG only function.

> ---
>  target/ppc/mmu_helper.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 1ecb36e85a..e7ba39c9e1 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -825,6 +825,7 @@ static int mmubooke_get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>      return ret;
>  }
>  
> +#ifdef CONFIG_TCG
>  static void booke206_flush_tlb(CPUPPCState *env, int flags,
>                                 const int check_iprot)
>  {
> @@ -846,6 +847,7 @@ static void booke206_flush_tlb(CPUPPCState *env, int flags,
>  
>      tlb_flush(env_cpu(env));
>  }
> +#endif
>  
>  static hwaddr booke206_tlb_to_page_size(CPUPPCState *env,
>                                          ppcmas_tlb_t *tlb)
> @@ -1956,6 +1958,7 @@ void helper_store_601_batl(CPUPPCState *env, uint32_t nr, target_ulong value)
>  /* TLB management */
>  void ppc_tlb_invalidate_all(CPUPPCState *env)
>  {
> +#ifdef CONFIG_TCG
>  #if defined(TARGET_PPC64)
>      if (mmu_is_64bit(env->mmu_model)) {
>          env->tlb_need_flush = 0;
> @@ -1994,6 +1997,7 @@ void ppc_tlb_invalidate_all(CPUPPCState *env)
>          cpu_abort(env_cpu(env), "Unknown MMU model %x\n", env->mmu_model);
>          break;
>      }
> +#endif
>  }
>  
>  #ifdef CONFIG_TCG

-- 
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

Re: [PATCH v2 1/3] target/ppc: Turn ppc_tlb_invalid_all in a noop

Posted by Lucas Mateus Martins Araujo e Castro 4 years, 7 months ago

On 15/06/2021 02:38, David Gibson wrote:
> On Thu, Jun 10, 2021 at 01:46:46PM -0300, Lucas Mateus Castro (alqotel) wrote:
>> The function ppc_tlb_invalid_all is now a no op when compiling without TCG.
>>
>> Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.araujo@eldorado.org.br>
>
> Hm, I think I suggested making ppc_tlb_invalidate_all() a stub, rather
> than removing the call from !TCG code.  But looking at this again, I
> think that was a mistake.  I think it makes more sense to suppress
> this on the caller side, and make this a TCG only function.
Ok, I'll add the if in ppc_cpu_reset that calls ppc_tlb_invalidate_all 
to an #ifdef CONFIG_TCG block, as Fabiano suggested last patch.
-- 
Lucas Mateus M. Araujo e Castro
Instituto de Pesquisas ELDORADO 
<https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station>
Departamento Computação Embarcada
Estagiario
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>