Re: [Xen-devel] [RFC][PATCH] xen/apic: refactor error_interrupt

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

On 2015/5/4 16:07, Jan Beulich wrote:
> > > > On 04.05.15 at 04:03, <tiejun.chen@xxxxxxxxx> wrote:
> > --- a/xen/arch/x86/apic.c
> > +++ b/xen/arch/x86/apic.c
> > @@ -1319,28 +1319,37 @@ out: ;
> >    * This interrupt should never happen with our APIC/SMP architecture
> >    */
> >
> > +static const char *apic_fault_reasons[] =
> If at all, then this should be const. But...
>
> > +{
> > +    "Send CS error",
> > +    "Receive CS error",
> > +    "Send accept error",
> > +    "Receive accept error",
> > +    "Reserved",
> > +    "Send illegal vector",
> > +    "Received illegal vector",
> > +    "Illegal register address",
> > +};
> > +
> > +static const char *apic_get_fault_reason(u8 fault_reason)
> > +{
> > +    return apic_fault_reasons[fault_reason];
> > +}
> > +
> >   void error_interrupt(struct cpu_user_regs *regs)
> >   {
> >       unsigned long v, v1;
> > +    const char *reason;
> >
> >       /* First tickle the hardware, only then report what went on. -- REW */
> >       v = apic_read(APIC_ESR);
> >       apic_write(APIC_ESR, 0);
> >       v1 = apic_read(APIC_ESR);
> >       ack_APIC_irq();
> > +    reason = apic_get_fault_reason(ffs(v1) - 1);
> ... I'm not convinced - you're treating errors represented by lower
> bits "better" than higher ones. And if there are multiple errors, then
> spelling out one but not the other may end up being confusing.
I just think APIC shouldn't record these multiple errors at the same 
time because these kinds of errors seem be exclusive... But I don't find 
this is described in SDM so I think you're right.
Fortunately, looks Andrew guides me a approach to cover this point, so 
just let me go there.
Thanks
Tiejun

> These errors should be rare enough to warrant manually looking up
> the individual bits' meanings.
>
> Jan
>
> > -    /* Here is what the APIC error bits mean:
> > -       0: Send CS error
> > -       1: Receive CS error
> > -       2: Send accept error
> > -       3: Receive accept error
> > -       4: Reserved
> > -       5: Send illegal vector
> > -       6: Received illegal vector
> > -       7: Illegal register address
> > -    */
> > -    printk (KERN_DEBUG "APIC error on CPU%d: %02lx(%02lx)\n",
> > -            smp_processor_id(), v , v1);
> > +    printk (KERN_DEBUG "APIC error on CPU%d: %02lx(%02lx):%s.\n",
> > +            smp_processor_id(), v , v1, reason);
> >   }
> >
> >   /*
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel