Allow distinguishing HV_ILLEGAL_GUEST_STATE in trace events.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/hvf/hvf.c        | 10 +++++++++-
 target/arm/hvf/trace-events |  1 +
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index ef76dcd28de..cc5bbc155d2 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1916,7 +1916,15 @@ int hvf_vcpu_exec(CPUState *cpu)
     bql_unlock();
     r = hv_vcpu_run(cpu->accel->fd);
     bql_lock();
-    assert_hvf_ok(r);
+    switch (r) {
+    case HV_SUCCESS:
+        break;
+    case HV_ILLEGAL_GUEST_STATE:
+        trace_hvf_illegal_guest_state();
+        /* fall through */
+    default:
+        g_assert_not_reached();
+    }
 
     /* handle VMEXIT */
     uint64_t exit_reason = hvf_exit->reason;
diff --git a/target/arm/hvf/trace-events b/target/arm/hvf/trace-events
index 4fbbe4b45ec..a4870e0a5c4 100644
--- a/target/arm/hvf/trace-events
+++ b/target/arm/hvf/trace-events
@@ -11,3 +11,4 @@ hvf_exit(uint64_t syndrome, uint32_t ec, uint64_t pc) "exit: 0x%"PRIx64" [ec=0x%
 hvf_psci_call(uint64_t x0, uint64_t x1, uint64_t x2, uint64_t x3, uint32_t cpuid) "PSCI Call x0=0x%016"PRIx64" x1=0x%016"PRIx64" x2=0x%016"PRIx64" x3=0x%016"PRIx64" cpu=0x%x"
 hvf_vgic_write(const char *name, uint64_t val) "vgic write to %s [val=0x%016"PRIx64"]"
 hvf_vgic_read(const char *name, uint64_t val) "vgic read from %s [val=0x%016"PRIx64"]"
+hvf_illegal_guest_state(void) "HV_ILLEGAL_GUEST_STATE"
-- 
2.49.0

On Mon, 23 Jun 2025 at 13:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> Allow distinguishing HV_ILLEGAL_GUEST_STATE in trace events.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/hvf/hvf.c        | 10 +++++++++-
>  target/arm/hvf/trace-events |  1 +
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index ef76dcd28de..cc5bbc155d2 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -1916,7 +1916,15 @@ int hvf_vcpu_exec(CPUState *cpu)
>      bql_unlock();
>      r = hv_vcpu_run(cpu->accel->fd);
>      bql_lock();
> -    assert_hvf_ok(r);
> +    switch (r) {
> +    case HV_SUCCESS:
> +        break;
> +    case HV_ILLEGAL_GUEST_STATE:
> +        trace_hvf_illegal_guest_state();
> +        /* fall through */
> +    default:
> +        g_assert_not_reached();

This seems kind of odd.

If it can happen, we shouldn't g_assert_not_reached().
If it can't happen, we shouldn't trace it.

But the hvf code already has a lot of "assert success
rather than handling possible-but-fatal errors more
gracefully", so I guess it's OK.

-- PMM

On 1/7/25 11:49, Peter Maydell wrote:
> On Mon, 23 Jun 2025 at 13:19, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>>
>> Allow distinguishing HV_ILLEGAL_GUEST_STATE in trace events.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   target/arm/hvf/hvf.c        | 10 +++++++++-
>>   target/arm/hvf/trace-events |  1 +
>>   2 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
>> index ef76dcd28de..cc5bbc155d2 100644
>> --- a/target/arm/hvf/hvf.c
>> +++ b/target/arm/hvf/hvf.c
>> @@ -1916,7 +1916,15 @@ int hvf_vcpu_exec(CPUState *cpu)
>>       bql_unlock();
>>       r = hv_vcpu_run(cpu->accel->fd);
>>       bql_lock();
>> -    assert_hvf_ok(r);
>> +    switch (r) {
>> +    case HV_SUCCESS:
>> +        break;
>> +    case HV_ILLEGAL_GUEST_STATE:
>> +        trace_hvf_illegal_guest_state();
>> +        /* fall through */
>> +    default:
>> +        g_assert_not_reached();
> 
> This seems kind of odd.
> 
> If it can happen, we shouldn't g_assert_not_reached().
> If it can't happen, we shouldn't trace it.
> 
> But the hvf code already has a lot of "assert success
> rather than handling possible-but-fatal errors more
> gracefully", so I guess it's OK.

OK, you can drop this patch: I will replace with error("unrecoverable:
...") && exit(1); to avoid such oddity.