Re: [Qemu-devel] [RFC v3 30/56] i386: convert to cpu_interrupt_request

[Richard Henderson]

On 10/19/18 2:05 AM, Emilio G. Cota wrote:
> @@ -713,9 +713,9 @@ int hvf_vcpu_exec(CPUState *cpu)
>          switch (exit_reason) {
>          case EXIT_REASON_HLT: {
>              macvm_set_rip(cpu, rip + ins_len);
> -            if (!((cpu->interrupt_request & CPU_INTERRUPT_HARD) &&
> +            if (!((cpu_interrupt_request(cpu) & CPU_INTERRUPT_HARD) &&
>                  (EFLAGS(env) & IF_MASK))
> -                && !(cpu->interrupt_request & CPU_INTERRUPT_NMI) &&
> +                && !(cpu_interrupt_request(cpu) & CPU_INTERRUPT_NMI) &&
>                  !(idtvec_info & VMCS_IDT_VEC_VALID)) {
>                  cpu_halted_set(cpu, 1);
>                  ret = EXCP_HLT;

Likewise wrt multiple calls.

> @@ -400,7 +401,8 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
>          };
>      }
>  
> -    if (cpu_state->interrupt_request & CPU_INTERRUPT_NMI) {
> +    cpu_mutex_lock(cpu_state);
> +    if (cpu_interrupt_request(cpu_state) & CPU_INTERRUPT_NMI) {
>          if (!(env->hflags2 & HF2_NMI_MASK) && !(info & VMCS_INTR_VALID)) {
>              cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_NMI);
>              info = VMCS_INTR_VALID | VMCS_INTR_T_NMI | NMI_VEC;
> @@ -411,7 +413,7 @@ bool hvf_inject_interrupts(CPUState *cpu_state)
>      }
>  
>      if (!(env->hflags & HF_INHIBIT_IRQ_MASK) &&
> -        (cpu_state->interrupt_request & CPU_INTERRUPT_HARD) &&
> +        (cpu_interrupt_request(cpu_state) & CPU_INTERRUPT_HARD) &&
>          (EFLAGS(env) & IF_MASK) && !(info & VMCS_INTR_VALID)) {
>          int line = cpu_get_pic_interrupt(&x86cpu->env);
>          cpu_reset_interrupt(cpu_state, CPU_INTERRUPT_HARD);

Likewise.

I think you need to be more careful about this in the conversions.  Previously,
the compiler would CSE these two loads; now you're taking a lock twice.

Or in the second instance, once, since you explicitly take the lock around a
big block.  But I think that's papering over the fact that you make 4 calls
when you should have made one, *and* not hold the lock across all that code.


r~