Re: [PATCH] target/i386: Added VGIF feature

[Paolo Bonzini <pbonzini@redhat.com>]

On 23/07/21 13:27, Lara Lazier wrote:
> @@ -353,9 +358,12 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
>           tlb_flush(cs);
>           break;
>       }
> -
> -    env->hflags2 |= HF2_GIF_MASK;
> -
> +    if (virtual_gif_enabled(env, int_ctl)) {
> +        x86_stl_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.int_ctl),
> +                        int_ctl |= V_GIF_MASK);
> +    } else {
> +        env->hflags2 |= HF2_GIF_MASK;
> +    }

This should not be changed, because it is setting the "real" GIF; vGIF 
hasn't taken effect yet.

The CLGI/VMRUN/STGI sequence is there to avoid delivering an interrupt 
while the processor state has been partly changed to whatever the guest 
wants.  VMRUN sets GIF so that it is possible for interrupts (or 
NMI/SMI) to cause a vmexit, but the vmexit immediately clears the GIF 
again so that it is only handled after the host executes STGI.

>       if (int_ctl & V_IRQ_MASK) {
>           CPUState *cs = env_cpu(env);
>   
> @@ -513,13 +521,31 @@ void helper_vmsave(CPUX86State *env, int aflag)
>   void helper_stgi(CPUX86State *env)
>   {
>       cpu_svm_check_intercept_param(env, SVM_EXIT_STGI, 0, GETPC());
> -    env->hflags2 |= HF2_GIF_MASK;
> +
> +    CPUState *cs = env_cpu(env);
> +    uint32_t int_ctl = x86_ldl_phys(cs,
> +                       env->vm_vmcb + offsetof(struct vmcb, control.int_ctl));
> +    if (virtual_gif_enabled(env, int_ctl) && likely(env->hflags & HF_GUEST_MASK)) {
> +        x86_stl_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.int_ctl),
> +                        int_ctl |= V_GIF_MASK);

No need to use "|=", likewise for "&=" below.

Thanks,

Paolo

> +    } else {
> +        env->hflags2 |= HF2_GIF_MASK;
> +    }
>   }
>   
>   void helper_clgi(CPUX86State *env)
>   {
>       cpu_svm_check_intercept_param(env, SVM_EXIT_CLGI, 0, GETPC());
> -    env->hflags2 &= ~HF2_GIF_MASK;
> +
> +    CPUState *cs = env_cpu(env);
> +    uint32_t int_ctl = x86_ldl_phys(cs,
> +                       env->vm_vmcb + offsetof(struct vmcb, control.int_ctl));
> +    if (virtual_gif_enabled(env, int_ctl) && likely(env->hflags & HF_GUEST_MASK)) {
> +        x86_stl_phys(cs, env->vm_vmcb + offsetof(struct vmcb, control.int_ctl),
> +                        int_ctl &= ~V_GIF_MASK);
> +    } else {
> +        env->hflags2 &= ~HF2_GIF_MASK;
> +    }
>   }
>   
>   bool cpu_svm_has_intercept(CPUX86State *env, uint32_t type)
>