Re: [PATCH] i386: hvf: Reset IRQ inhibition after moving RIP

2020-03-30 Thread Cameron Esfahani via 
Reviewed-by: Cameron Esfahani 

LGTM.

Cameron Esfahani
di...@apple.com

"There are times in the life of a nation when the only place a decent man can 
find himself is in prison."



> On Mar 28, 2020, at 10:44 AM, Roman Bolshakov  wrote:
> 
> The sequence of instructions exposes an issue:
>  sti
>  hlt
> 
> Interrupts cannot be delivered to hvf after hlt instruction cpu because
> HF_INHIBIT_IRQ_MASK is set just before hlt is handled and never reset
> after moving instruction pointer beyond hlt.
> 
> So, after hvf_vcpu_exec() returns, CPU thread gets locked up forever in
> qemu_wait_io_event() (cpu_thread_is_idle() evaluates inhibition
> flag and considers the CPU idle if the flag is set).
> 
> Cc: Cameron Esfahani 
> Signed-off-by: Roman Bolshakov 
> ---
> target/i386/hvf/vmx.h | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
> index 03d2c79b9c..ce2a1532d5 100644
> --- a/target/i386/hvf/vmx.h
> +++ b/target/i386/hvf/vmx.h
> @@ -167,6 +167,8 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, 
> uint64_t cr4)
> 
> static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
> {
> +X86CPU *x86_cpu = X86_CPU(cpu);
> +CPUX86State *env = _cpu->env;
> uint64_t val;
> 
> /* BUG, should take considering overlap.. */
> @@ -176,6 +178,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t 
> rip)
>val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
>if (val & (VMCS_INTERRUPTIBILITY_STI_BLOCKING |
>VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING)) {
> +env->hflags &= ~HF_INHIBIT_IRQ_MASK;
> wvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY,
>val & ~(VMCS_INTERRUPTIBILITY_STI_BLOCKING |
>VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING));
> -- 
> 2.24.1
> 
>

Re: [PATCH] i386: hvf: Reset IRQ inhibition after moving RIP

2020-03-30 Thread Paolo Bonzini 
On 28/03/20 18:44, Roman Bolshakov wrote:
> The sequence of instructions exposes an issue:
>   sti
>   hlt
> 
> Interrupts cannot be delivered to hvf after hlt instruction cpu because
> HF_INHIBIT_IRQ_MASK is set just before hlt is handled and never reset
> after moving instruction pointer beyond hlt.
> 
> So, after hvf_vcpu_exec() returns, CPU thread gets locked up forever in
> qemu_wait_io_event() (cpu_thread_is_idle() evaluates inhibition
> flag and considers the CPU idle if the flag is set).
> 
> Cc: Cameron Esfahani 
> Signed-off-by: Roman Bolshakov 
> ---
>  target/i386/hvf/vmx.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
> index 03d2c79b9c..ce2a1532d5 100644
> --- a/target/i386/hvf/vmx.h
> +++ b/target/i386/hvf/vmx.h
> @@ -167,6 +167,8 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, 
> uint64_t cr4)
>  
>  static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
>  {
> +X86CPU *x86_cpu = X86_CPU(cpu);
> +CPUX86State *env = _cpu->env;
>  uint64_t val;
>  
>  /* BUG, should take considering overlap.. */
> @@ -176,6 +178,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t 
> rip)
> val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
> if (val & (VMCS_INTERRUPTIBILITY_STI_BLOCKING |
> VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING)) {
> +env->hflags &= ~HF_INHIBIT_IRQ_MASK;
>  wvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY,
> val & ~(VMCS_INTERRUPTIBILITY_STI_BLOCKING |
> VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING));
> 

Queued, thanks!

Paolo

[PATCH] i386: hvf: Reset IRQ inhibition after moving RIP

2020-03-28 Thread Roman Bolshakov 
The sequence of instructions exposes an issue:
  sti
  hlt

Interrupts cannot be delivered to hvf after hlt instruction cpu because
HF_INHIBIT_IRQ_MASK is set just before hlt is handled and never reset
after moving instruction pointer beyond hlt.

So, after hvf_vcpu_exec() returns, CPU thread gets locked up forever in
qemu_wait_io_event() (cpu_thread_is_idle() evaluates inhibition
flag and considers the CPU idle if the flag is set).

Cc: Cameron Esfahani 
Signed-off-by: Roman Bolshakov 
---
 target/i386/hvf/vmx.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/target/i386/hvf/vmx.h b/target/i386/hvf/vmx.h
index 03d2c79b9c..ce2a1532d5 100644
--- a/target/i386/hvf/vmx.h
+++ b/target/i386/hvf/vmx.h
@@ -167,6 +167,8 @@ static inline void macvm_set_cr4(hv_vcpuid_t vcpu, uint64_t 
cr4)
 
 static inline void macvm_set_rip(CPUState *cpu, uint64_t rip)
 {
+X86CPU *x86_cpu = X86_CPU(cpu);
+CPUX86State *env = _cpu->env;
 uint64_t val;
 
 /* BUG, should take considering overlap.. */
@@ -176,6 +178,7 @@ static inline void macvm_set_rip(CPUState *cpu, uint64_t 
rip)
val = rvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY);
if (val & (VMCS_INTERRUPTIBILITY_STI_BLOCKING |
VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING)) {
+env->hflags &= ~HF_INHIBIT_IRQ_MASK;
 wvmcs(cpu->hvf_fd, VMCS_GUEST_INTERRUPTIBILITY,
val & ~(VMCS_INTERRUPTIBILITY_STI_BLOCKING |
VMCS_INTERRUPTIBILITY_MOVSS_BLOCKING));
-- 
2.24.1