[Qemu-devel] Re: PATCH: debugging apic

[Sam King <kingst@gmail.com>]

  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
>