Using the full 64-bit register values is wrong in this case; especially
soon after a mode switch from long mode to 32-bit one upper halves of
registers may continue to be non-zero.

Fixes: 09fce8016596 ("Nested VMX: Emulation of guest VMXON/OFF instruction")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Note that the affected VMX insns are invalid to use from compatibility
mode, and hence the more expensive vmx_guest_x86_mode() doesn't need
using here. (VMCALL and VMFUNC, which are permitted in compatibility
mode, aren't taking this path. In fact both aren't dealt with at all
[explicitly] in vvmx.c.)

--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -360,7 +360,12 @@ enum vmx_insn_errno set_vvmcs_real_safe(
 static unsigned long reg_read(struct cpu_user_regs *regs,
                               unsigned int index)
 {
-    return *decode_gpr(regs, index);
+    unsigned long val = *decode_gpr(regs, index);
+
+    if ( !hvm_long_mode_active(current) )
+        val = (uint32_t)val;
+
+    return val;
 }
 
 static void reg_write(struct cpu_user_regs *regs,

On 11/06/2025 11:42 am, Jan Beulich wrote:
> Using the full 64-bit register values is wrong in this case; especially
> soon after a mode switch from long mode to 32-bit one upper halves of
> registers may continue to be non-zero.
>
> Fixes: 09fce8016596 ("Nested VMX: Emulation of guest VMXON/OFF instruction")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Note that the affected VMX insns are invalid to use from compatibility
> mode, and hence the more expensive vmx_guest_x86_mode() doesn't need
> using here.

Fine, but you must have a comment to this effect in the code, and what
prevents compatibility mode getting here?

> (VMCALL and VMFUNC, which are permitted in compatibility
> mode, aren't taking this path. In fact both aren't dealt with at all
> [explicitly] in vvmx.c.)

VMCALL has no operands, implicit or otherwise.

VMFUNC does have implicit operands, but post-dates the last time anyone
did any serious nested virt work, hence why it's unhandled.

Personally, I think vVMFUNC emulation can safely be left to whomever
first wants it.  The only potential usecase I'm aware of has ceased to
be for unrelated reasons.

~Andrew

On 11.06.2025 13:07, Andrew Cooper wrote:
> On 11/06/2025 11:42 am, Jan Beulich wrote:
>> Using the full 64-bit register values is wrong in this case; especially
>> soon after a mode switch from long mode to 32-bit one upper halves of
>> registers may continue to be non-zero.
>>
>> Fixes: 09fce8016596 ("Nested VMX: Emulation of guest VMXON/OFF instruction")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Note that the affected VMX insns are invalid to use from compatibility
>> mode, and hence the more expensive vmx_guest_x86_mode() doesn't need
>> using here.
> 
> Fine, but you must have a comment to this effect in the code, and what
> prevents compatibility mode getting here?

Sure, I can add a comment there. As to compatibility mode - the insns will
#UD, and hence no (instruction based) VMEXIT will occur.

Jan