[PATCH v2 1/9] i386: hvf: Set env->eip in macvm_set_rip()

Roman Bolshakov posted 9 patches 5 years, 4 months ago
Maintainers: Roman Bolshakov <r.bolshakov@yadro.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <rth@twiddle.net>, Eduardo Habkost <ehabkost@redhat.com>
[PATCH v2 1/9] i386: hvf: Set env->eip in macvm_set_rip()
Posted by Roman Bolshakov 5 years, 4 months ago
cpu_synchronize_state() is currently no-op for hvf but BIOS will hang in
vAPIC option ROM when cpu_synchronize_state() is wired to
hvf_cpu_synchronize_state().

cpu_synchronize_state() state is called from vapic_write() during option
ROM initialization. It sets dirty flag on the cpu. macvm_set_rip() is
then invoked to advance IP after the I/O write to vAPIC port.

macvm_set_rip() only modifies VMCS, it doesn't change env->eip.
Therefore on the next iteration of vCPU loop, vcpu_dirty flag is checked
and hvf_put_registers() overwrites correct RIP in VMCS with the value of
env->eip that points to the I/O write instruction. Execution of the CPU
gets stuck on the instruction.

The issue can be avoided if eip doesn't contain stale value when dirty
flag is set on cpu.

Cc: Cameron Esfahani <dirty@apple.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 target/i386/hvf/vmx.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index ce2a1532d5..1e8b29bf7d 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -173,6 +173,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
 
     /* BUG, should take considering overlap.. */
     wreg(cpu->hvf_fd, HV_X86_RIP, rip);
+    env->eip = rip;
 
     /* after moving forward in rip, we need to clean INTERRUPTABILITY */
    val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
-- 
2.26.1


Re: [PATCH v2 1/9] i386: hvf: Set env->eip in macvm_set_rip()
Posted by Paolo Bonzini 5 years, 4 months ago
On 30/06/20 12:28, Roman Bolshakov wrote:
> cpu_synchronize_state() is currently no-op for hvf but BIOS will hang in
> vAPIC option ROM when cpu_synchronize_state() is wired to
> hvf_cpu_synchronize_state().
> 
> cpu_synchronize_state() state is called from vapic_write() during option
> ROM initialization. It sets dirty flag on the cpu. macvm_set_rip() is
> then invoked to advance IP after the I/O write to vAPIC port.
> 
> macvm_set_rip() only modifies VMCS, it doesn't change env->eip.
> Therefore on the next iteration of vCPU loop, vcpu_dirty flag is checked
> and hvf_put_registers() overwrites correct RIP in VMCS with the value of
> env->eip that points to the I/O write instruction. Execution of the CPU
> gets stuck on the instruction.
> 
> The issue can be avoided if eip doesn't contain stale value when dirty
> flag is set on cpu.
> 
> Cc: Cameron Esfahani <dirty@apple.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  target/i386/hvf/vmx.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
> index ce2a1532d5..1e8b29bf7d 100644
> --- a/target/i386/hvf/vmx.h
> +++ b/target/i386/hvf/vmx.h
> @@ -173,6 +173,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
>  
>      /* BUG, should take considering overlap.. */
>      wreg(cpu->hvf_fd, HV_X86_RIP, rip);
> +    env->eip = rip;
>  
>      /* after moving forward in rip, we need to clean INTERRUPTABILITY */
>     val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
> 

Queued except for patch 4.

Paolo