IO-APIC: tweak debug key info formatting

IO-APIC: tweak debug key info formatting

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 409 bytes --]

The formatting of the IO-APIC debug key info has niggled me for a while,
and with the latest interrupt bug I am chasing, has finally motivated me
to fix it.

The attached patch causes all columns to line up, and removes the comma
which served no purpose in combination with the spaces already present.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: dump_ioapic_irq_info_format.patch --]
[-- Type: text/x-patch, Size: 1598 bytes --]

# HG changeset patch
# Parent e2722b24dc0962de37215320b05d1bb7c4c42864
Change io_apic debug information format to align columns

Having the columns aligned makes for much easier reading.  Also remove
the commas which only add to visual clutter in combination with
spaces.

Furthermore, printing fewer characters makes it less likely that the
serial buffer will overflow resulting in loss of critical debugging
information.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r e2722b24dc09 xen/arch/x86/io_apic.c
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2406,13 +2406,13 @@ void dump_ioapic_irq_info(void)
             *(((int *)&rte) + 1) = io_apic_read(entry->apic, 0x11 + 2 * pin);
             spin_unlock_irqrestore(&ioapic_lock, flags);
 
-            printk("vector=%u, delivery_mode=%u, dest_mode=%s, "
-                   "delivery_status=%d, polarity=%d, irr=%d, "
-                   "trigger=%s, mask=%d, dest_id:%d\n",
+            printk("vector=%3u delivery_mode=%u dest_mode=%s "
+                   "delivery_status=%d polarity=%d irr=%d "
+                   "trigger=%s mask=%d dest_id:%d\n",
                    rte.vector, rte.delivery_mode,
-                   rte.dest_mode ? "logical" : "physical",
+                   rte.dest_mode ? "logical " : "physical",
                    rte.delivery_status, rte.polarity, rte.irr,
-                   rte.trigger ? "level" : "edge", rte.mask,
+                   rte.trigger ? "level" : "edge ", rte.mask,
                    rte.dest.logical.logical_dest);
 
             if ( entry->next == 0 )

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: IO-APIC: tweak debug key info formatting

[Jan Beulich]

>>> On 03.02.12 at 14:30, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>Furthermore, printing fewer characters makes it less likely that the
>serial buffer will overflow resulting in loss of critical debugging
>information.

For that part, shortening some of the strings would certainly be
desirable too (delivery_mode being the worst).

>--- a/xen/arch/x86/io_apic.c
>+++ b/xen/arch/x86/io_apic.c
>@@ -2406,13 +2406,13 @@ void dump_ioapic_irq_info(void)
>             *(((int *)&rte) + 1) = io_apic_read(entry->apic, 0x11 + 2 * pin);
>             spin_unlock_irqrestore(&ioapic_lock, flags);
> 
>-            printk("vector=%u, delivery_mode=%u, dest_mode=%s, "
>-                   "delivery_status=%d, polarity=%d, irr=%d, "
>-                   "trigger=%s, mask=%d, dest_id:%d\n",
>+            printk("vector=%3u delivery_mode=%u dest_mode=%s "

Could you please print the vector as %02x instead? We should really
do this consistently everywhere, and vectors in decimal are pretty
meaningless anyway (as one will always need to convert them for
purposes of priority determination or comparison with #define-s in
the sources).

Jan

>+                   "delivery_status=%d polarity=%d irr=%d "
>+                   "trigger=%s mask=%d dest_id:%d\n",
>                    rte.vector, rte.delivery_mode,
>-                   rte.dest_mode ? "logical" : "physical",
>+                   rte.dest_mode ? "logical " : "physical",
>                    rte.delivery_status, rte.polarity, rte.irr,
>-                   rte.trigger ? "level" : "edge", rte.mask,
>+                   rte.trigger ? "level" : "edge ", rte.mask,
>                    rte.dest.logical.logical_dest);
> 
>             if ( entry->next == 0 )

Re: IO-APIC: tweak debug key info formatting

[Andrew Cooper]

On 03/02/12 13:44, Jan Beulich wrote:
>>>> On 03.02.12 at 14:30, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> Furthermore, printing fewer characters makes it less likely that the
>> serial buffer will overflow resulting in loss of critical debugging
>> information.
> For that part, shortening some of the strings would certainly be
> desirable too (delivery_mode being the worst).

I was considering that as well, but just wanted to get the alignment
issue sorted first.  I will respin the patch with much shorter lines.

>> --- a/xen/arch/x86/io_apic.c
>> +++ b/xen/arch/x86/io_apic.c
>> @@ -2406,13 +2406,13 @@ void dump_ioapic_irq_info(void)
>>             *(((int *)&rte) + 1) = io_apic_read(entry->apic, 0x11 + 2 * pin);
>>             spin_unlock_irqrestore(&ioapic_lock, flags);
>>
>> -            printk("vector=%u, delivery_mode=%u, dest_mode=%s, "
>> -                   "delivery_status=%d, polarity=%d, irr=%d, "
>> -                   "trigger=%s, mask=%d, dest_id:%d\n",
>> +            printk("vector=%3u delivery_mode=%u dest_mode=%s "
> Could you please print the vector as %02x instead? We should really
> do this consistently everywhere, and vectors in decimal are pretty
> meaningless anyway (as one will always need to convert them for
> purposes of priority determination or comparison with #define-s in
> the sources).

Very true - I agree as well.

> Jan
>
>> +                   "delivery_status=%d polarity=%d irr=%d "
>> +                   "trigger=%s mask=%d dest_id:%d\n",
>>                    rte.vector, rte.delivery_mode,
>> -                   rte.dest_mode ? "logical" : "physical",
>> +                   rte.dest_mode ? "logical " : "physical",
>>                    rte.delivery_status, rte.polarity, rte.irr,
>> -                   rte.trigger ? "level" : "edge", rte.mask,
>> +                   rte.trigger ? "level" : "edge ", rte.mask,
>>                    rte.dest.logical.logical_dest);
>>
>>             if ( entry->next == 0 )
>

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: IO-APIC: tweak debug key info formatting v2

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 2400 bytes --]

Respun to reduce the verbosity of names written out for each line. 
Contractions are:

vector -> vec
delivery_mode -> delivery
dest_mode -> dest
delivery_status -> status
trigger -> trig

In addition:
    vector is printed in hex
    dest and trig mode binary choices are represented with single
characters rather than full names
    delivery mode is converted from a number to a short string.

Output on one of my text boxes is as follows:

idol login: (XEN) *** Serial input -> Xen (type 'CTRL-a' three times to
switch input to DOM0)
(XEN) Guest interrupt information:
<... snip ...>
(XEN)    IRQ:  22 affinity:1 vec:b0 type=IO-APIC-level   status=00000010
in-flight=0 domain-list=0: 22(----),
(XEN)    IRQ:  23 affinity:1 vec:c0 type=IO-APIC-level   status=00000010
in-flight=0 domain-list=0: 23(----),
(XEN)    IRQ:  24 affinity:1 vec:41 type=PCI-MSI         status=00000010
in-flight=0 domain-list=0:279(----),
(XEN)    IRQ:  25 affinity:1 vec:31 type=PCI-MSI         status=00000010
in-flight=0 domain-list=0:278(----),
(XEN) IO-APIC interrupt information:
(XEN)     IRQ  0 Vec240:
(XEN)       Apic 0x00, Pin  2: vec=f0 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  1 Vec 40:
(XEN)       Apic 0x00, Pin  1: vec=28 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  3 Vec 48:
(XEN)       Apic 0x00, Pin  3: vec=30 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  4 Vec241:
(XEN)       Apic 0x00, Pin  4: vec=f1 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  5 Vec 56:
(XEN)       Apic 0x00, Pin  5: vec=38 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  6 Vec 64:
(XEN)       Apic 0x00, Pin  6: vec=40 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  7 Vec 72:
(XEN)       Apic 0x00, Pin  7: vec=48 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  8 Vec 80:
(XEN)       Apic 0x00, Pin  8: vec=50 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=E mask=0 dest_id:1
(XEN)     IRQ  9 Vec 88:
(XEN)       Apic 0x00, Pin  9: vec=58 delivery=LoPri dest=L status=0
polarity=0 irr=0 trig=L mask=0 dest_id:1
<... snip ...>

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: dump_ioapic_irq_info_format.patch --]
[-- Type: text/x-patch, Size: 2461 bytes --]

# HG changeset patch
# Parent e2722b24dc0962de37215320b05d1bb7c4c42864
IO-APIC: Reformat IO-APIC RTE debug info (v2)

Having the columns aligned makes for much easier reading.  Also remove
the commas which only add to visual clutter in combination with
spaces.

Furthermore, printing fewer characters makes it less likely that the
serial buffer will overflow resulting in loss of critical debugging
information.

Changes since v1:
 *  Format vector as hex rather than dec
 *  Contract some names
 *  destination mode uses 'L' or 'P' instead of full words
 *  trigger mode uses 'L' or 'E' instead of full words
 *  delivery mode uses short string instead of a number

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

diff -r e2722b24dc09 xen/arch/x86/io_apic.c
--- a/xen/arch/x86/io_apic.c
+++ b/xen/arch/x86/io_apic.c
@@ -2374,6 +2374,23 @@ int ioapic_guest_write(unsigned long phy
     return 0;
 }
 
+static const char * delivery_mode_2_str(
+    const enum ioapic_irq_destination_types mode)
+{
+    switch ( mode )
+    {
+    case dest_Fixed: return "Fixed";
+    case dest_LowestPrio: return "LoPri";
+    case dest_SMI: return "SMI";
+    case dest_NMI: return "NMI";
+    case dest_INIT: return "INIT";
+    case dest_ExtINT: return "ExINT";
+    case dest__reserved_1:
+    case dest__reserved_2: return "Resvd";
+    default: return "INVAL";
+    }
+}
+
 void dump_ioapic_irq_info(void)
 {
     struct irq_pin_list *entry;
@@ -2406,13 +2423,12 @@ void dump_ioapic_irq_info(void)
             *(((int *)&rte) + 1) = io_apic_read(entry->apic, 0x11 + 2 * pin);
             spin_unlock_irqrestore(&ioapic_lock, flags);
 
-            printk("vector=%u, delivery_mode=%u, dest_mode=%s, "
-                   "delivery_status=%d, polarity=%d, irr=%d, "
-                   "trigger=%s, mask=%d, dest_id:%d\n",
-                   rte.vector, rte.delivery_mode,
-                   rte.dest_mode ? "logical" : "physical",
+            printk("vec=%02x delivery=%-5s dest=%c status=%d "
+                   "polarity=%d irr=%d trig=%c mask=%d dest_id:%d\n",
+                   rte.vector, delivery_mode_2_str(rte.delivery_mode),
+                   rte.dest_mode ? 'L' : 'P',
                    rte.delivery_status, rte.polarity, rte.irr,
-                   rte.trigger ? "level" : "edge", rte.mask,
+                   rte.trigger ? 'L' : 'E', rte.mask,
                    rte.dest.logical.logical_dest);
 
             if ( entry->next == 0 )

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel