[v2] target/ppc: Move TCG code from excp_helper.c to tcg-excp_helper.c

[PATCH v2 06/15] target/ppc: Ensure powerpc_checkstop() is only called under TCG

Posted by Philippe Mathieu-Daudé 2 months, 1 week ago

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 target/ppc/excp_helper.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 8956466db1d..b08cd53688c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -19,6 +19,7 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "qemu/log.h"
+#include "system/tcg.h"
 #include "system/system.h"
 #include "system/runstate.h"
 #include "cpu.h"
@@ -30,7 +31,6 @@
 #include "trace.h"
 
 #ifdef CONFIG_TCG
-#include "system/tcg.h"
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
 #endif
@@ -443,13 +443,11 @@ void helper_attn(CPUPPCState *env)
 static void powerpc_mcheck_checkstop(CPUPPCState *env)
 {
     /* KVM guests always have MSR[ME] enabled */
-#ifdef CONFIG_TCG
     if (FIELD_EX64(env->msr, MSR, ME)) {
         return;
     }
-
+    assert(tcg_enabled());
     powerpc_checkstop(env, "machine check with MSR[ME]=0");
-#endif
 }
 
 static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
-- 
2.47.1

Re: [PATCH v2 06/15] target/ppc: Ensure powerpc_checkstop() is only called under TCG

Posted by Harsh Prateek Bora 2 months, 1 week ago


On 1/27/25 15:56, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   target/ppc/excp_helper.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 8956466db1d..b08cd53688c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -19,6 +19,7 @@
>   #include "qemu/osdep.h"
>   #include "qemu/main-loop.h"
>   #include "qemu/log.h"
> +#include "system/tcg.h"
>   #include "system/system.h"
>   #include "system/runstate.h"
>   #include "cpu.h"
> @@ -30,7 +31,6 @@
>   #include "trace.h"
>   
>   #ifdef CONFIG_TCG
> -#include "system/tcg.h"
>   #include "exec/helper-proto.h"
>   #include "exec/cpu_ldst.h"
>   #endif
> @@ -443,13 +443,11 @@ void helper_attn(CPUPPCState *env)
>   static void powerpc_mcheck_checkstop(CPUPPCState *env)
>   {
>       /* KVM guests always have MSR[ME] enabled */
> -#ifdef CONFIG_TCG
>       if (FIELD_EX64(env->msr, MSR, ME)) {
>           return;
>       }
> -
> +    assert(tcg_enabled());

Shouldn't this be a no-op if not TCG ?

Nick, please advise ?

regards,
Harsh
>       powerpc_checkstop(env, "machine check with MSR[ME]=0");
> -#endif
>   }
>   
>   static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)

Re: [PATCH v2 06/15] target/ppc: Ensure powerpc_checkstop() is only called under TCG

Posted by Nicholas Piggin 1 month, 1 week ago

On Tue Jan 28, 2025 at 4:43 PM AEST, Harsh Prateek Bora wrote:
>
>
> On 1/27/25 15:56, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/ppc/excp_helper.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>> 
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 8956466db1d..b08cd53688c 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -19,6 +19,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/main-loop.h"
>>   #include "qemu/log.h"
>> +#include "system/tcg.h"
>>   #include "system/system.h"
>>   #include "system/runstate.h"
>>   #include "cpu.h"
>> @@ -30,7 +31,6 @@
>>   #include "trace.h"
>>   
>>   #ifdef CONFIG_TCG
>> -#include "system/tcg.h"
>>   #include "exec/helper-proto.h"
>>   #include "exec/cpu_ldst.h"
>>   #endif
>> @@ -443,13 +443,11 @@ void helper_attn(CPUPPCState *env)
>>   static void powerpc_mcheck_checkstop(CPUPPCState *env)
>>   {
>>       /* KVM guests always have MSR[ME] enabled */
>> -#ifdef CONFIG_TCG
>>       if (FIELD_EX64(env->msr, MSR, ME)) {
>>           return;
>>       }
>> -
>> +    assert(tcg_enabled());
>
> Shouldn't this be a no-op if not TCG ?
>
> Nick, please advise ?

If KVM, I think we would rather catch that it got called instead of
no-op.

At this point the guest is crashed, so it's not overly rude to
assert. I'm okay with this.

Thanks,
Nick

Re: [PATCH v2 06/15] target/ppc: Ensure powerpc_checkstop() is only called under TCG

Posted by Harsh Prateek Bora 2 months, 1 week ago


On 1/28/25 12:13, Harsh Prateek Bora wrote:
> 
> 
> On 1/27/25 15:56, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   target/ppc/excp_helper.c | 6 ++----
>>   1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 8956466db1d..b08cd53688c 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -19,6 +19,7 @@
>>   #include "qemu/osdep.h"
>>   #include "qemu/main-loop.h"
>>   #include "qemu/log.h"
>> +#include "system/tcg.h"
>>   #include "system/system.h"
>>   #include "system/runstate.h"
>>   #include "cpu.h"
>> @@ -30,7 +31,6 @@
>>   #include "trace.h"
>>   #ifdef CONFIG_TCG
>> -#include "system/tcg.h"
>>   #include "exec/helper-proto.h"
>>   #include "exec/cpu_ldst.h"
>>   #endif
>> @@ -443,13 +443,11 @@ void helper_attn(CPUPPCState *env)
>>   static void powerpc_mcheck_checkstop(CPUPPCState *env)
>>   {
>>       /* KVM guests always have MSR[ME] enabled */
>> -#ifdef CONFIG_TCG
>>       if (FIELD_EX64(env->msr, MSR, ME)) {
>>           return;
>>       }
>> -
>> +    assert(tcg_enabled());
> 
> Shouldn't this be a no-op if not TCG ?
> 
> Nick, please advise ?
> 

Also, patch title needs update - it's powerpc_mcheck_checkstop.

> regards,
> Harsh
>>       powerpc_checkstop(env, "machine check with MSR[ME]=0");
>> -#endif
>>   }
>>   static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>