[PATCH] target/i386: svm: fix sign extension of exit code

[PATCH] target/i386: svm: fix sign extension of exit code

[Paolo Bonzini]

The exit_code parameter of cpu_vmexit is declared as uint32_t, but exit
codes are 64 bits wide according to the AMD SVM specification.  And because
uint32_t is unsigned, this causes exit codes to be zero-extended, for example
writing SVM_EXIT_ERR as 0xffff_ffff instead of the expected 0xffff_ffff_ffff_ffff.

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2977
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target/i386/tcg/helper-tcg.h        | 2 +-
 target/i386/tcg/system/svm_helper.c | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/i386/tcg/helper-tcg.h b/target/i386/tcg/helper-tcg.h
index be011b06b7c..e41cbda407a 100644
--- a/target/i386/tcg/helper-tcg.h
+++ b/target/i386/tcg/helper-tcg.h
@@ -99,7 +99,7 @@ void cpu_load_eflags(CPUX86State *env, int eflags, int update_mask);
 
 /* system/svm_helper.c */
 #ifndef CONFIG_USER_ONLY
-G_NORETURN void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code,
+G_NORETURN void cpu_vmexit(CPUX86State *nenv, uint64_t exit_code,
                            uint64_t exit_info_1, uintptr_t retaddr);
 void do_vmexit(CPUX86State *env);
 #endif
diff --git a/target/i386/tcg/system/svm_helper.c b/target/i386/tcg/system/svm_helper.c
index 505788b0e26..4b86796518f 100644
--- a/target/i386/tcg/system/svm_helper.c
+++ b/target/i386/tcg/system/svm_helper.c
@@ -128,7 +128,7 @@ static inline bool virtual_gif_enabled(CPUX86State *env)
     return false;
 }
 
-static inline bool virtual_vm_load_save_enabled(CPUX86State *env, uint32_t exit_code, uintptr_t retaddr)
+static inline bool virtual_vm_load_save_enabled(CPUX86State *env, uint64_t exit_code, uintptr_t retaddr)
 {
     uint64_t lbr_ctl;
 
@@ -723,7 +723,7 @@ void helper_svm_check_io(CPUX86State *env, uint32_t port, uint32_t param,
     }
 }
 
-void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
+void cpu_vmexit(CPUX86State *env, uint64_t exit_code, uint64_t exit_info_1,
                 uintptr_t retaddr)
 {
     CPUState *cs = env_cpu(env);
@@ -732,7 +732,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
 
     qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016"
                   PRIx64 ", " TARGET_FMT_lx ")!\n",
-                  exit_code, exit_info_1,
+                  (uint32_t)exit_code, exit_info_1,
                   x86_ldq_phys(cs, env->vm_vmcb + offsetof(struct vmcb,
                                                    control.exit_info_2)),
                   env->eip);
-- 
2.51.1

Re: [PATCH] target/i386: svm: fix sign extension of exit code

[Richard Henderson]

On 11/15/25 01:26, Paolo Bonzini wrote:
> -void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
> +void cpu_vmexit(CPUX86State *env, uint64_t exit_code, uint64_t exit_info_1,
>                   uintptr_t retaddr)
>   {
>       CPUState *cs = env_cpu(env);
> @@ -732,7 +732,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
>   
>       qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016"
>                     PRIx64 ", " TARGET_FMT_lx ")!\n",
> -                  exit_code, exit_info_1,
> +                  (uint32_t)exit_code, exit_info_1,

Why cast instead of printing all 64 bits?

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

Re: [PATCH] target/i386: svm: fix sign extension of exit code

[Paolo Bonzini]

On 11/17/25 10:42, Richard Henderson wrote:
> On 11/15/25 01:26, Paolo Bonzini wrote:
>> -void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t 
>> exit_info_1,
>> +void cpu_vmexit(CPUX86State *env, uint64_t exit_code, uint64_t 
>> exit_info_1,
>>                   uintptr_t retaddr)
>>   {
>>       CPUState *cs = env_cpu(env);
>> @@ -732,7 +732,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t 
>> exit_code, uint64_t exit_info_1,
>>       qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", 
>> %016"
>>                     PRIx64 ", " TARGET_FMT_lx ")!\n",
>> -                  exit_code, exit_info_1,
>> +                  (uint32_t)exit_code, exit_info_1,
> 
> Why cast instead of printing all 64 bits?

Because in practice exit_code is either a very small negative value 
(-1...-4) or a positive value.  For QEMU in addition the positive value 
will also be small (less than 16 bits); values between 0x8000_0000 and 
0xffff_ffff could happen in principle but are for use by software and by 
the processor[1].  So the high 32 bits are basically unused, and the 
cast removes eight zeroes or f's from the log.

Paolo

[1] see for example arch/x86/include/uapi/asm/svm.h in Linux:

#define SVM_VMGEXIT_MMIO_READ                   0x80000001
#define SVM_VMGEXIT_MMIO_WRITE                  0x80000002
#define SVM_VMGEXIT_NMI_COMPLETE                0x80000003
#define SVM_VMGEXIT_AP_HLT_LOOP                 0x80000004
#define SVM_VMGEXIT_AP_JUMP_TABLE               0x80000005
#define SVM_VMGEXIT_PSC                         0x80000010
#define SVM_VMGEXIT_GUEST_REQUEST               0x80000011
#define SVM_VMGEXIT_EXT_GUEST_REQUEST           0x80000012
#define SVM_VMGEXIT_AP_CREATION                 0x80000013
#define SVM_VMGEXIT_SNP_RUN_VMPL                0x80000018
#define SVM_VMGEXIT_SAVIC                       0x8000001a
#define SVM_VMGEXIT_HV_FEATURES                 0x8000fffd
#define SVM_VMGEXIT_TERM_REQUEST                0x8000fffe
#define SVM_VMGEXIT_UNSUPPORTED_EVENT           0x8000ffff
#define SVM_EXIT_SW                             0xf0000000

SVM_VMGEXIT_* values are actually stored by the guest in its own memory, 
not by the processor; whereas SVM_EXIT_SW is only used in nested 
virtualization scenarios and will not be passed to cpu_vmexit.

Re: [PATCH] target/i386: svm: fix sign extension of exit code

[Richard Henderson]

On 11/17/25 12:32, Paolo Bonzini wrote:
> On 11/17/25 10:42, Richard Henderson wrote:
>> On 11/15/25 01:26, Paolo Bonzini wrote:
>>> -void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
>>> +void cpu_vmexit(CPUX86State *env, uint64_t exit_code, uint64_t exit_info_1,
>>>                   uintptr_t retaddr)
>>>   {
>>>       CPUState *cs = env_cpu(env);
>>> @@ -732,7 +732,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t 
>>> exit_info_1,
>>>       qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016"
>>>                     PRIx64 ", " TARGET_FMT_lx ")!\n",
>>> -                  exit_code, exit_info_1,
>>> +                  (uint32_t)exit_code, exit_info_1,
>>
>> Why cast instead of printing all 64 bits?
> 
> Because in practice exit_code is either a very small negative value (-1...-4) or a 
> positive value.  For QEMU in addition the positive value will also be small (less than 16 
> bits); values between 0x8000_0000 and 0xffff_ffff could happen in principle but are for 
> use by software and by the processor[1].  So the high 32 bits are basically unused, and 
> the cast removes eight zeroes or f's from the log.

Then maybe you really want the signed int64_t?


r~

Re: [PATCH] target/i386: svm: fix sign extension of exit code

[Paolo Bonzini]

On Tue, Nov 18, 2025 at 9:31 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 11/17/25 12:32, Paolo Bonzini wrote:
> > On 11/17/25 10:42, Richard Henderson wrote:
> >> On 11/15/25 01:26, Paolo Bonzini wrote:
> >>> -void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
> >>> +void cpu_vmexit(CPUX86State *env, uint64_t exit_code, uint64_t exit_info_1,
> >>>                   uintptr_t retaddr)
> >>>   {
> >>>       CPUState *cs = env_cpu(env);
> >>> @@ -732,7 +732,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t
> >>> exit_info_1,
> >>>       qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016"
> >>>                     PRIx64 ", " TARGET_FMT_lx ")!\n",
> >>> -                  exit_code, exit_info_1,
> >>> +                  (uint32_t)exit_code, exit_info_1,
> >>
> >> Why cast instead of printing all 64 bits?
> >
> > Because in practice exit_code is either a very small negative value (-1...-4) or a
> > positive value.  For QEMU in addition the positive value will also be small (less than 16
> > bits); values between 0x8000_0000 and 0xffff_ffff could happen in principle but are for
> > use by software and by the processor[1].  So the high 32 bits are basically unused, and
> > the cast removes eight zeroes or f's from the log.
>
> Then maybe you really want the signed int64_t?

The problem is not in the type (int64_t or uint64_t work equally well,
they're all just constants), it's in the format string. Positive codes
are written in hexadecimal in the manual, so:
- %ld makes it hard to match the positive codes in the manual
- %lx still prints a -1 as ffffffffffffffff.

So all the cast is doing is making the log more readable.

Paolo

Re: [PATCH] target/i386: svm: fix sign extension of exit code

[Richard Henderson]

On 11/18/25 14:07, Paolo Bonzini wrote:
> On Tue, Nov 18, 2025 at 9:31 AM Richard Henderson
> <richard.henderson@linaro.org> wrote:
>>
>> On 11/17/25 12:32, Paolo Bonzini wrote:
>>> On 11/17/25 10:42, Richard Henderson wrote:
>>>> On 11/15/25 01:26, Paolo Bonzini wrote:
>>>>> -void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1,
>>>>> +void cpu_vmexit(CPUX86State *env, uint64_t exit_code, uint64_t exit_info_1,
>>>>>                    uintptr_t retaddr)
>>>>>    {
>>>>>        CPUState *cs = env_cpu(env);
>>>>> @@ -732,7 +732,7 @@ void cpu_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t
>>>>> exit_info_1,
>>>>>        qemu_log_mask(CPU_LOG_TB_IN_ASM, "vmexit(%08x, %016" PRIx64 ", %016"
>>>>>                      PRIx64 ", " TARGET_FMT_lx ")!\n",
>>>>> -                  exit_code, exit_info_1,
>>>>> +                  (uint32_t)exit_code, exit_info_1,
>>>>
>>>> Why cast instead of printing all 64 bits?
>>>
>>> Because in practice exit_code is either a very small negative value (-1...-4) or a
>>> positive value.  For QEMU in addition the positive value will also be small (less than 16
>>> bits); values between 0x8000_0000 and 0xffff_ffff could happen in principle but are for
>>> use by software and by the processor[1].  So the high 32 bits are basically unused, and
>>> the cast removes eight zeroes or f's from the log.
>>
>> Then maybe you really want the signed int64_t?
> 
> The problem is not in the type (int64_t or uint64_t work equally well,
> they're all just constants), it's in the format string. Positive codes
> are written in hexadecimal in the manual, so:
> - %ld makes it hard to match the positive codes in the manual
> - %lx still prints a -1 as ffffffffffffffff.
> 
> So all the cast is doing is making the log more readable.

Ok then.


r~