[Qemu-devel] Re: PATCH: debugging apic

[Sam King]

 Thanks for the info.  I started looking at the APIC logic and fixing 
the behavior is going to take me a little bit of time to make sure that 
I keep all of the APIC states consistent.  In the meantime, TeLeMan 
suggested a patch a few years ago that would fix this (and potentially 
other) problems.  I have updated this patch and included it in this email:

*** cpu-exec.c  2010-07-22 07:39:04.000000000 -0500
--- ../qemu-0.12.5-fixed/cpu-exec.c     2010-09-29 11:41:25.991566727 -0500
***************
*** 388,396 ****
                                       (env->eflags & IF_MASK &&
                                        !(env->hflags & 
HF_INHIBIT_IRQ_MASK))))) {
                              int intno;
-                             svm_check_intercept(SVM_EXIT_INTR);
                              env->interrupt_request &= 
~(CPU_INTERRUPT_HARD | CPU_INTERRUPT_VIRQ);
                              intno = cpu_get_pic_interrupt(env);
                              qemu_log_mask(CPU_LOG_TB_IN_ASM, 
"Servicing hardware INT=0x%02x\n", intno);
  #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
  #undef env
--- 388,397 ----
                                       (env->eflags & IF_MASK &&
                                        !(env->hflags & 
HF_INHIBIT_IRQ_MASK))))) {
                              int intno;
                              env->interrupt_request &= 
~(CPU_INTERRUPT_HARD | CPU_INTERRUPT_VIRQ);
                              intno = cpu_get_pic_interrupt(env);
+                             if(intno >= 0) {
+                                 svm_check_intercept(SVM_EXIT_INTR);
                                  qemu_log_mask(CPU_LOG_TB_IN_ASM, 
"Servicing hardware INT=0x%02x\n", intno);
  #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
  #undef env
***************
*** 401,406 ****
--- 402,409 ----
                                  /* ensure that no TB jump will be 
modified as
                                     the program flow was changed */
                                  next_tb = 0;
+                             }
+
  #if !defined(CONFIG_USER_ONLY)
                          } else if ((interrupt_request & 
CPU_INTERRUPT_VIRQ) &&
                                     (env->eflags & IF_MASK) &&


On 9/29/10 1:44 AM, Jan Kiszka wrote:
> Am 28.09.2010 21:57, Sam King wrote:
> >   Thanks to Bernhard Kauer for pointing out the problem.  Apparently if
> > software disables LVT_LINT0 when there is a pending CPU_HARD_INTERRUPT
> > you can get into trouble.  I attached a patch that fixes the problem by
> > resetting the interrupt_request.  I am not sure if we need to do the
> > same for LINT1, but this fixed the incorrect GPF I was getting.
> ...
> [ please inline patches ]
>
> > *** hw/apic.c   2010-07-22 07:39:04.000000000 -0500
> > --- ../qemu-0.12.5-fixed/hw/apic.c      2010-09-28 14:45:55.476945540 -0500
> > ***************
> > *** 841,846 ****
> > --- 841,851 ----
> >                s->lvt[n] = val;
> >                if (n == APIC_LVT_TIMER)
> >                    apic_timer_update(s, qemu_get_clock(vm_clock));
> > +
> > +             if(n == APIC_LVT_LINT0) {
> > +                 if((val&  APIC_LVT_MASKED)&&  (env->interrupt_request&  
> > CPU_INTERRUPT_HARD))
> > +                     cpu_reset_interrupt(env, CPU_INTERRUPT_HARD);
> > +             }
> >            }
> >            break;
> >        case 0x38:
> This actually points out open issues, but more work is required:
>
> You need to consider other potentially pending interrupts as well, thus
> you must not blindly reset here. And the same is true for invocations of
> apic_deliver_pic_intr(..., 0). The APIC has to save the PIC line state
> and forward it according to its current LVT mask state, which includes
> raising the interrupt if the mask is removed while the PIC line is high.
>
> Jan