diff mbox

add trace points in irqchip code

Message ID 20090706131723.GA28046@redhat.com (mailing list archive)
State New, archived
Headers show

Commit Message

Gleb Natapov July 6, 2009, 1:17 p.m. UTC 
Add tracepoint in msi/ioapic/pic set_irq() functions,
in IPI sending and in the point where IRQ is placed into
apic's IRR.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Avi Kivity July 7, 2009, 10:06 a.m. UTC | #1 
On 07/06/2009 04:17 PM, Gleb Natapov wrote:
> Add tracepoint in msi/ioapic/pic set_irq() functions,
> in IPI sending and in the point where IRQ is placed into
> apic's IRR.
>
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
> index 1d1bb75..e4bcbdd 100644
> --- a/arch/x86/kvm/i8259.c
> +++ b/arch/x86/kvm/i8259.c
> @@ -30,6 +30,7 @@
>   #include "irq.h"
>
>   #include<linux/kvm_host.h>
> +#include "trace.h"
>
>   static void pic_lock(struct kvm_pic *s)
>   	__acquires(&s->lock)
> @@ -190,6 +191,8 @@ int kvm_pic_set_irq(void *opaque, int irq, int level)
>   	if (irq>= 0&&  irq<  PIC_NUM_PINS) {
>   		ret = pic_set_irq1(&s->pics[irq>>  3], irq&  7, level);
>   		pic_update_irq(s);
> +		trace_kvm_pic_set_irq(irq>>  3, irq&  7, s->pics[irq>>  3].elcr,
> +				      s->pics[irq>>  3].imr, ret == 0);
>    

It's better to send undecoded arguments (irq, s, ret) and do the 
shifting/masking/derefing in the TP_fast_assign.  This is because 
argument preparation is always compiled inline and executed (so I 
understand; not sure) but TP_fast_assign is out-of-line and only 
executed if the tracepoint is enabled.
> +TRACE_EVENT(kvm_pic_set_irq,
> +	    TP_PROTO(__u8 chip, __u8 pin, __u8 elcr, __u8 imr, int coalesced),
> +	    TP_ARGS(chip, pin, elcr, imr, coalesced),
> +
> +	TP_STRUCT__entry(
> +		__field(	__u8,		chip		)
> +		__field(	__u8,		pin		)
> +		__field(	__u8,		elcr		)
> +		__field(	__u8,		imr		)
> +		__field(	int,		coalesced	)
> +	),
>    

bool is slightly more descriptive.

> +
> +	TP_fast_assign(
> +		__entry->chip		= chip;
> +		__entry->pin		= pin;
> +		__entry->elcr		= elcr;
> +		__entry->imr		= imr;
> +		__entry->coalesced	= coalesced;
> +	),
> +
> +	TP_printk("chip=%u pin=%u (%s%s)%s",
> +		  __entry->chip, __entry->pin,
> +		  (__entry->elcr&  (1<<  __entry->pin)) ? "level":"edge",
> +		  (__entry->imr&  (1<<  __entry->pin)) ? "|masked":"",
> +		  __entry->coalesced ? " (coalesced)" : "")
> +);
>    

kvm style is not to use equals signs.

> +
> +	TP_printk("dst=%x vec=%u (%s|%s|%s|%s|%s)",
> +		  __entry->dest_id, (u8)__entry->icr_low,
> +		  __print_symbolic((__entry->icr_low>>  8&  0x7),
> +				   kvm_deliver_mode),
> +		  (__entry->icr_low&  (1<<11)) ? "logical":"physical",
> +		  (__entry->icr_low&  (1<<14)) ? "assert":"de-assert",
> +		  (__entry->icr_low&  (1<<15)) ? "level":"edge",
>    

Spaces around :.

> +	TP_STRUCT__entry(
> +		__field(	__u32,		apicid		)
> +		__field(	__u8,		dm		)
> +		__field(	__u8,		tm		)
> +		__field(	__u8,		vec		)
> +		__field(	__u8,		coalesced	)
> +	),
>    

bool.

> +
> +TRACE_EVENT(kvm_msi_set_irq,
> +	    TP_PROTO(__u32 address, __u64 data),
> +	    TP_ARGS(address, data),
> +
> +	TP_STRUCT__entry(
> +		__field(	__u32,		address		)
>    

Make this a u64 just in case.
Gleb Natapov July 7, 2009, 12:44 p.m. UTC | #2 
On Tue, Jul 07, 2009 at 01:06:00PM +0300, Avi Kivity wrote:
> On 07/06/2009 04:17 PM, Gleb Natapov wrote:
>> Add tracepoint in msi/ioapic/pic set_irq() functions,
>> in IPI sending and in the point where IRQ is placed into
>> apic's IRR.
>>
>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
>> index 1d1bb75..e4bcbdd 100644
>> --- a/arch/x86/kvm/i8259.c
>> +++ b/arch/x86/kvm/i8259.c
>> @@ -30,6 +30,7 @@
>>   #include "irq.h"
>>
>>   #include<linux/kvm_host.h>
>> +#include "trace.h"
>>
>>   static void pic_lock(struct kvm_pic *s)
>>   	__acquires(&s->lock)
>> @@ -190,6 +191,8 @@ int kvm_pic_set_irq(void *opaque, int irq, int level)
>>   	if (irq>= 0&&  irq<  PIC_NUM_PINS) {
>>   		ret = pic_set_irq1(&s->pics[irq>>  3], irq&  7, level);
>>   		pic_update_irq(s);
>> +		trace_kvm_pic_set_irq(irq>>  3, irq&  7, s->pics[irq>>  3].elcr,
>> +				      s->pics[irq>>  3].imr, ret == 0);
>>    
>
> It's better to send undecoded arguments (irq, s, ret) and do the  
> shifting/masking/derefing in the TP_fast_assign.  This is because  
> argument preparation is always compiled inline and executed (so I  
> understand; not sure) but TP_fast_assign is out-of-line and only  
> executed if the tracepoint is enabled.
This will move knowledge how gsi is mapped to pic chips into tracing
code. This will break if we will add emulation to some fantastic HW with
number of pics different from 2.

--
			Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Avi Kivity July 7, 2009, 12:52 p.m. UTC | #3 
On 07/07/2009 03:44 PM, Gleb Natapov wrote:
> On Tue, Jul 07, 2009 at 01:06:00PM +0300, Avi Kivity wrote:
>    
>> On 07/06/2009 04:17 PM, Gleb Natapov wrote:
>>      
>>> Add tracepoint in msi/ioapic/pic set_irq() functions,
>>> in IPI sending and in the point where IRQ is placed into
>>> apic's IRR.
>>>
>>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>> diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
>>> index 1d1bb75..e4bcbdd 100644
>>> --- a/arch/x86/kvm/i8259.c
>>> +++ b/arch/x86/kvm/i8259.c
>>> @@ -30,6 +30,7 @@
>>>    #include "irq.h"
>>>
>>>    #include<linux/kvm_host.h>
>>> +#include "trace.h"
>>>
>>>    static void pic_lock(struct kvm_pic *s)
>>>    	__acquires(&s->lock)
>>> @@ -190,6 +191,8 @@ int kvm_pic_set_irq(void *opaque, int irq, int level)
>>>    	if (irq>= 0&&   irq<   PIC_NUM_PINS) {
>>>    		ret = pic_set_irq1(&s->pics[irq>>   3], irq&   7, level);
>>>    		pic_update_irq(s);
>>> +		trace_kvm_pic_set_irq(irq>>   3, irq&   7, s->pics[irq>>   3].elcr,
>>> +				      s->pics[irq>>   3].imr, ret == 0);
>>>
>>>        
>> It's better to send undecoded arguments (irq, s, ret) and do the
>> shifting/masking/derefing in the TP_fast_assign.  This is because
>> argument preparation is always compiled inline and executed (so I
>> understand; not sure) but TP_fast_assign is out-of-line and only
>> executed if the tracepoint is enabled.
>>      
> This will move knowledge how gsi is mapped to pic chips into tracing
> code. This will break if we will add emulation to some fantastic HW with
> number of pics different from 2.
>    

Right, that doesn't work out well in this case.  Something to keep in 
mind in general.

(Could still be handled by defining inline functions or macros to do the 
decoding and using them in TP_fast_assign, but probably not worth it in 
this case)
diff mbox

Patch

diff --git a/arch/x86/kvm/i8259.c b/arch/x86/kvm/i8259.c
index 1d1bb75..e4bcbdd 100644
--- a/arch/x86/kvm/i8259.c
+++ b/arch/x86/kvm/i8259.c
@@ -30,6 +30,7 @@ 
 #include "irq.h"
 
 #include <linux/kvm_host.h>
+#include "trace.h"
 
 static void pic_lock(struct kvm_pic *s)
 	__acquires(&s->lock)
@@ -190,6 +191,8 @@  int kvm_pic_set_irq(void *opaque, int irq, int level)
 	if (irq >= 0 && irq < PIC_NUM_PINS) {
 		ret = pic_set_irq1(&s->pics[irq >> 3], irq & 7, level);
 		pic_update_irq(s);
+		trace_kvm_pic_set_irq(irq >> 3, irq & 7, s->pics[irq >> 3].elcr,
+				      s->pics[irq >> 3].imr, ret == 0);
 	}
 	pic_unlock(s);
 
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 6f559d6..bd5cba9 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -375,6 +375,8 @@  static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			break;
 
 		result = !apic_test_and_set_irr(vector, apic);
+		trace_kvm_apic_accept_irq(vcpu->vcpu_id, delivery_mode,
+					  trig_mode, vector, result);
 		if (!result) {
 			if (trig_mode)
 				apic_debug("level trig mode repeatedly for "
@@ -493,6 +495,8 @@  static void apic_send_ipi(struct kvm_lapic *apic)
 	else
 		irq.dest_id = GET_APIC_DEST_FIELD(icr_high);
 
+	trace_kvm_apic_ipi(icr_low, irq.dest_id);
+
 	apic_debug("icr_high 0x%x, icr_low 0x%x, "
 		   "short_hand 0x%x, dest 0x%x, trig_mode 0x%x, level 0x%x, "
 		   "dest_mode 0x%x, delivery_mode 0x%x, vector 0x%x\n",
diff --git a/arch/x86/kvm/trace.h b/arch/x86/kvm/trace.h
index 6c2c87f..056a942 100644
--- a/arch/x86/kvm/trace.h
+++ b/arch/x86/kvm/trace.h
@@ -264,6 +264,91 @@  TRACE_EVENT(kvm_cr,
 #define trace_kvm_cr_read(cr, val)		trace_kvm_cr(0, cr, val)
 #define trace_kvm_cr_write(cr, val)		trace_kvm_cr(1, cr, val)
 
+TRACE_EVENT(kvm_pic_set_irq,
+	    TP_PROTO(__u8 chip, __u8 pin, __u8 elcr, __u8 imr, int coalesced),
+	    TP_ARGS(chip, pin, elcr, imr, coalesced),
+
+	TP_STRUCT__entry(
+		__field(	__u8,		chip		)
+		__field(	__u8,		pin		)
+		__field(	__u8,		elcr		)
+		__field(	__u8,		imr		)
+		__field(	int,		coalesced	)
+	),
+
+	TP_fast_assign(
+		__entry->chip		= chip;
+		__entry->pin		= pin;
+		__entry->elcr		= elcr;
+		__entry->imr		= imr;
+		__entry->coalesced	= coalesced;
+	),
+
+	TP_printk("chip=%u pin=%u (%s%s)%s",
+		  __entry->chip, __entry->pin,
+		  (__entry->elcr & (1 << __entry->pin)) ? "level":"edge",
+		  (__entry->imr & (1 << __entry->pin)) ? "|masked":"",
+		  __entry->coalesced ? " (coalesced)" : "")
+);
+
+#define kvm_apic_dst_shorthand		\
+	{0x0, "dst"},			\
+	{0x1, "self"},			\
+	{0x2, "all"},			\
+	{0x3, "all-but-self"}
+
+TRACE_EVENT(kvm_apic_ipi,
+	    TP_PROTO(__u32 icr_low, __u32 dest_id),
+	    TP_ARGS(icr_low, dest_id),
+
+	TP_STRUCT__entry(
+		__field(	__u32,		icr_low		)
+		__field(	__u32,		dest_id		)
+	),
+
+	TP_fast_assign(
+		__entry->icr_low	= icr_low;
+		__entry->dest_id	= dest_id;
+	),
+
+	TP_printk("dst=%x vec=%u (%s|%s|%s|%s|%s)",
+		  __entry->dest_id, (u8)__entry->icr_low,
+		  __print_symbolic((__entry->icr_low >> 8 & 0x7),
+				   kvm_deliver_mode),
+		  (__entry->icr_low & (1<<11)) ? "logical":"physical",
+		  (__entry->icr_low & (1<<14)) ? "assert":"de-assert",
+		  (__entry->icr_low & (1<<15)) ? "level":"edge",
+		  __print_symbolic((__entry->icr_low >> 18 & 0x3),
+				   kvm_apic_dst_shorthand))
+);
+
+TRACE_EVENT(kvm_apic_accept_irq,
+	    TP_PROTO(__u32 apicid, __u8 dm, __u8 tm, __u8 vec, int coalesced),
+	    TP_ARGS(apicid, dm, tm, vec, coalesced),
+
+	TP_STRUCT__entry(
+		__field(	__u32,		apicid		)
+		__field(	__u8,		dm		)
+		__field(	__u8,		tm		)
+		__field(	__u8,		vec		)
+		__field(	__u8,		coalesced	)
+	),
+
+	TP_fast_assign(
+		__entry->apicid		= apicid;
+		__entry->dm		= dm;
+		__entry->tm		= tm;
+		__entry->vec		= vec;
+		__entry->coalesced	= coalesced;
+	),
+
+	TP_printk("apicid=%x vec=%u (%s|%s)%s",
+		  __entry->apicid, __entry->vec,
+		  (__entry->dm == APIC_DM_LOWEST) ? "lowest":"fixed",
+		  __entry->tm ? "level":"edge",
+		  __entry->coalesced ? " (coalesced)" : "")
+);
+
 #endif /* _TRACE_KVM_H */
 
 /* This part must be outside protection */
diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
index 2c29235..04335db 100644
--- a/include/trace/events/kvm.h
+++ b/include/trace/events/kvm.h
@@ -28,6 +28,62 @@  TRACE_EVENT(kvm_set_irq,
 		  __entry->gsi, __entry->level, __entry->irq_source_id)
 );
 
+#define kvm_deliver_mode		\
+	{0x0, "Fixed"},			\
+	{0x1, "LowPrio"},		\
+	{0x2, "SMI"},			\
+	{0x3, "Res3"},			\
+	{0x4, "NMI"},			\
+	{0x5, "INIT"},			\
+	{0x6, "SIPI"},			\
+	{0x7, "ExtINT"}
+
+TRACE_EVENT(kvm_ioapic_set_irq,
+	    TP_PROTO(__u64 e, int pin, int coalesced),
+	    TP_ARGS(e, pin, coalesced),
+
+	TP_STRUCT__entry(
+		__field(	__u64,		e		)
+		__field(	int,		pin		)
+		__field(	int,		coalesced	)
+	),
+
+	TP_fast_assign(
+		__entry->e		= e;
+		__entry->pin		= pin;
+		__entry->coalesced	= coalesced;
+	),
+
+	TP_printk("pin=%u dst=%x vec=%u (%s|%s|%s%s)%s",
+		  __entry->pin, (u8)(__entry->e >> 56), (u8)__entry->e,
+		  __print_symbolic((__entry->e >> 8 & 0x7), kvm_deliver_mode),
+		  (__entry->e & (1<<11)) ? "logical":"physical",
+		  (__entry->e & (1<<15)) ? "level":"edge",
+		  (__entry->e & (1<<16)) ? "|masked":"",
+		  __entry->coalesced ? " (coalesced)" : "")
+);
+
+TRACE_EVENT(kvm_msi_set_irq,
+	    TP_PROTO(__u32 address, __u64 data),
+	    TP_ARGS(address, data),
+
+	TP_STRUCT__entry(
+		__field(	__u32,		address		)
+		__field(	__u64,		data		)
+	),
+
+	TP_fast_assign(
+		__entry->address	= address;
+		__entry->data		= data;
+	),
+
+	TP_printk("dst=%u vec=%x (%s|%s|%s%s)",
+		  (u8)(__entry->address >> 12), (u8)__entry->data,
+		  __print_symbolic((__entry->data >> 8 & 0x7), kvm_deliver_mode),
+		  (__entry->address & (1<<2)) ? "logical":"physical",
+		  (__entry->data & (1<<15)) ? "level":"edge",
+		  (__entry->address & (1<<3)) ? "|rh":"")
+);
 
 #define kvm_irqchips						\
 	{KVM_IRQCHIP_PIC_MASTER,	"PIC master"},		\
diff --git a/virt/kvm/ioapic.c b/virt/kvm/ioapic.c
index 92496ff..b91fbb2 100644
--- a/virt/kvm/ioapic.c
+++ b/virt/kvm/ioapic.c
@@ -36,6 +36,7 @@ 
 #include <asm/processor.h>
 #include <asm/page.h>
 #include <asm/current.h>
+#include <trace/events/kvm.h>
 
 #include "ioapic.h"
 #include "lapic.h"
@@ -193,6 +194,7 @@  int kvm_ioapic_set_irq(struct kvm_ioapic *ioapic, int irq, int level)
 			    (!edge && !entry.fields.remote_irr))
 				ret = ioapic_service(ioapic, irq);
 		}
+		trace_kvm_ioapic_set_irq(entry.bits, irq, ret == 0);
 	}
 	return ret;
 }
diff --git a/virt/kvm/irq_comm.c b/virt/kvm/irq_comm.c
index c380bf0..43b692e 100644
--- a/virt/kvm/irq_comm.c
+++ b/virt/kvm/irq_comm.c
@@ -100,6 +100,8 @@  static int kvm_set_msi(struct kvm_kernel_irq_routing_entry *e,
 {
 	struct kvm_lapic_irq irq;
 
+	trace_kvm_msi_set_irq(e->msi.address_lo, e->msi.data);
+
 	irq.dest_id = (e->msi.address_lo &
 			MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT;
 	irq.vector = (e->msi.data &