KVM Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] kvm: x86: ioapic and apic debug macros cleanup
@ 2019-07-05 17:08 Yi Wang
  2019-07-15  4:59 ` Yi Wang
  2019-07-15 18:39 ` Paolo Bonzini
  0 siblings, 2 replies; 3+ messages in thread
From: Yi Wang @ 2019-07-05 17:08 UTC (permalink / raw)
  To: pbonzini
  Cc: rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
	xue.zhihong, wang.yi59, up2wing, wang.liang82

The ioapic_debug and apic_debug have been not used
for years, and kvm tracepoints are enough for debugging,
so remove them as Paolo suggested.

However, there may be something wrong when pv evi get/put
user, so it's better to retain some log there.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 arch/x86/kvm/ioapic.c | 15 --------
 arch/x86/kvm/lapic.c  | 98 +++++----------------------------------------------
 2 files changed, 9 insertions(+), 104 deletions(-)

diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
index 1add1bc..d859ae8 100644
--- a/arch/x86/kvm/ioapic.c
+++ b/arch/x86/kvm/ioapic.c
@@ -45,11 +45,6 @@
 #include "lapic.h"
 #include "irq.h"
 
-#if 0
-#define ioapic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg)
-#else
-#define ioapic_debug(fmt, arg...)
-#endif
 static int ioapic_service(struct kvm_ioapic *vioapic, int irq,
 		bool line_status);
 
@@ -294,7 +289,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
 	default:
 		index = (ioapic->ioregsel - 0x10) >> 1;
 
-		ioapic_debug("change redir index %x val %x\n", index, val);
 		if (index >= IOAPIC_NUM_PINS)
 			return;
 		e = &ioapic->redirtbl[index];
@@ -343,12 +337,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
 	    entry->fields.remote_irr))
 		return -1;
 
-	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
-		     "vector=%x trig_mode=%x\n",
-		     entry->fields.dest_id, entry->fields.dest_mode,
-		     entry->fields.delivery_mode, entry->fields.vector,
-		     entry->fields.trig_mode);
-
 	irqe.dest_id = entry->fields.dest_id;
 	irqe.vector = entry->fields.vector;
 	irqe.dest_mode = entry->fields.dest_mode;
@@ -515,7 +503,6 @@ static int ioapic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 	if (!ioapic_in_range(ioapic, addr))
 		return -EOPNOTSUPP;
 
-	ioapic_debug("addr %lx\n", (unsigned long)addr);
 	ASSERT(!(addr & 0xf));	/* check alignment */
 
 	addr &= 0xff;
@@ -558,8 +545,6 @@ static int ioapic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 	if (!ioapic_in_range(ioapic, addr))
 		return -EOPNOTSUPP;
 
-	ioapic_debug("ioapic_mmio_write addr=%p len=%d val=%p\n",
-		     (void*)addr, len, val);
 	ASSERT(!(addr & 0xf));	/* check alignment */
 
 	switch (len) {
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 4dabc31..0f3b57e 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -52,9 +52,6 @@
 #define PRIu64 "u"
 #define PRIo64 "o"
 
-/* #define apic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg) */
-#define apic_debug(fmt, arg...) do {} while (0)
-
 /* 14 is the version for Xeon and Pentium 8.4.8*/
 #define APIC_VERSION			(0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
 #define LAPIC_MMIO_LENGTH		(1 << 12)
@@ -631,7 +628,7 @@ static bool pv_eoi_get_pending(struct kvm_vcpu *vcpu)
 {
 	u8 val;
 	if (pv_eoi_get_user(vcpu, &val) < 0)
-		apic_debug("Can't read EOI MSR value: 0x%llx\n",
+		printk(KERN_WARNING "Can't read EOI MSR value: 0x%llx\n",
 			   (unsigned long long)vcpu->arch.pv_eoi.msr_val);
 	return val & 0x1;
 }
@@ -639,7 +636,7 @@ static bool pv_eoi_get_pending(struct kvm_vcpu *vcpu)
 static void pv_eoi_set_pending(struct kvm_vcpu *vcpu)
 {
 	if (pv_eoi_put_user(vcpu, KVM_PV_EOI_ENABLED) < 0) {
-		apic_debug("Can't set EOI MSR value: 0x%llx\n",
+		printk(KERN_WARNING "Can't set EOI MSR value: 0x%llx\n",
 			   (unsigned long long)vcpu->arch.pv_eoi.msr_val);
 		return;
 	}
@@ -649,7 +646,7 @@ static void pv_eoi_set_pending(struct kvm_vcpu *vcpu)
 static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
 {
 	if (pv_eoi_put_user(vcpu, KVM_PV_EOI_DISABLED) < 0) {
-		apic_debug("Can't clear EOI MSR value: 0x%llx\n",
+		printk(KERN_WARNING "Can't clear EOI MSR value: 0x%llx\n",
 			   (unsigned long long)vcpu->arch.pv_eoi.msr_val);
 		return;
 	}
@@ -683,9 +680,6 @@ static bool __apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr)
 	else
 		ppr = isrv & 0xf0;
 
-	apic_debug("vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x",
-		   apic, ppr, isr, isrv);
-
 	*new_ppr = ppr;
 	if (old_ppr != ppr)
 		kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr);
@@ -762,8 +756,6 @@ static bool kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
 		return ((logical_id >> 4) == (mda >> 4))
 		       && (logical_id & mda & 0xf) != 0;
 	default:
-		apic_debug("Bad DFR vcpu %d: %08x\n",
-			   apic->vcpu->vcpu_id, kvm_lapic_get_reg(apic, APIC_DFR));
 		return false;
 	}
 }
@@ -802,10 +794,6 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 	struct kvm_lapic *target = vcpu->arch.apic;
 	u32 mda = kvm_apic_mda(vcpu, dest, source, target);
 
-	apic_debug("target %p, source %p, dest 0x%x, "
-		   "dest_mode 0x%x, short_hand 0x%x\n",
-		   target, source, dest, dest_mode, short_hand);
-
 	ASSERT(target);
 	switch (short_hand) {
 	case APIC_DEST_NOSHORT:
@@ -820,8 +808,6 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
 	case APIC_DEST_ALLBUT:
 		return target != source;
 	default:
-		apic_debug("kvm: apic: Bad dest shorthand value %x\n",
-			   short_hand);
 		return false;
 	}
 }
@@ -1097,15 +1083,10 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
 			smp_wmb();
 			kvm_make_request(KVM_REQ_EVENT, vcpu);
 			kvm_vcpu_kick(vcpu);
-		} else {
-			apic_debug("Ignoring de-assert INIT to vcpu %d\n",
-				   vcpu->vcpu_id);
 		}
 		break;
 
 	case APIC_DM_STARTUP:
-		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
-			   vcpu->vcpu_id, vector);
 		result = 1;
 		apic->sipi_vector = vector;
 		/* make sure sipi_vector is visible for the receiver */
@@ -1223,14 +1204,6 @@ static void apic_send_ipi(struct kvm_lapic *apic)
 
 	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, "
-		   "msi_redir_hint 0x%x\n",
-		   icr_high, icr_low, irq.shorthand, irq.dest_id,
-		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
-		   irq.vector, irq.msi_redir_hint);
-
 	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
 }
 
@@ -1284,7 +1257,6 @@ static u32 __apic_read(struct kvm_lapic *apic, unsigned int offset)
 
 	switch (offset) {
 	case APIC_ARBPRI:
-		apic_debug("Access APIC ARBPRI register which is for P6\n");
 		break;
 
 	case APIC_TMCCT:	/* Timer CCR */
@@ -1321,17 +1293,11 @@ int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
 	/* this bitmask has a bit cleared for each reserved register */
 	static const u64 rmask = 0x43ff01ffffffe70cULL;
 
-	if ((alignment + len) > 4) {
-		apic_debug("KVM_APIC_READ: alignment error %x %d\n",
-			   offset, len);
+	if ((alignment + len) > 4)
 		return 1;
-	}
 
-	if (offset > 0x3f0 || !(rmask & (1ULL << (offset >> 4)))) {
-		apic_debug("KVM_APIC_READ: read reserved register %x\n",
-			   offset);
+	if (offset > 0x3f0 || !(rmask & (1ULL << (offset >> 4))))
 		return 1;
-	}
 
 	result = __apic_read(apic, offset & ~0xf);
 
@@ -1389,9 +1355,6 @@ static void update_divide_count(struct kvm_lapic *apic)
 	tmp1 = tdcr & 0xf;
 	tmp2 = ((tmp1 & 0x3) | ((tmp1 & 0x8) >> 1)) + 1;
 	apic->divide_count = 0x1 << (tmp2 & 0x7);
-
-	apic_debug("timer divide count is 0x%x\n",
-				   apic->divide_count);
 }
 
 static void limit_periodic_timer_frequency(struct kvm_lapic *apic)
@@ -1616,16 +1579,6 @@ static bool set_target_expiration(struct kvm_lapic *apic)
 
 	limit_periodic_timer_frequency(apic);
 
-	apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
-		   PRIx64 ", "
-		   "timer initial count 0x%x, period %lldns, "
-		   "expire @ 0x%016" PRIx64 ".\n", __func__,
-		   APIC_BUS_CYCLE_NS, ktime_to_ns(now),
-		   kvm_lapic_get_reg(apic, APIC_TMICT),
-		   apic->lapic_timer.period,
-		   ktime_to_ns(ktime_add_ns(now,
-				apic->lapic_timer.period)));
-
 	apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
 		nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
 	apic->lapic_timer.target_expiration = ktime_add_ns(now, apic->lapic_timer.period);
@@ -1828,8 +1781,6 @@ static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val)
 	if (apic->lvt0_in_nmi_mode != lvt0_in_nmi_mode) {
 		apic->lvt0_in_nmi_mode = lvt0_in_nmi_mode;
 		if (lvt0_in_nmi_mode) {
-			apic_debug("Receive NMI setting on APIC_LVT0 "
-				   "for cpu %d\n", apic->vcpu->vcpu_id);
 			atomic_inc(&apic->vcpu->kvm->arch.vapics_in_nmi_mode);
 		} else
 			atomic_dec(&apic->vcpu->kvm->arch.vapics_in_nmi_mode);
@@ -1943,8 +1894,6 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 	case APIC_TDCR: {
 		uint32_t old_divisor = apic->divide_count;
 
-		if (val & 4)
-			apic_debug("KVM_WRITE:TDCR %x\n", val);
 		kvm_lapic_set_reg(apic, APIC_TDCR, val);
 		update_divide_count(apic);
 		if (apic->divide_count != old_divisor &&
@@ -1956,10 +1905,8 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 		break;
 	}
 	case APIC_ESR:
-		if (apic_x2apic_mode(apic) && val != 0) {
-			apic_debug("KVM_WRITE:ESR not zero %x\n", val);
+		if (apic_x2apic_mode(apic) && val != 0)
 			ret = 1;
-		}
 		break;
 
 	case APIC_SELF_IPI:
@@ -1972,8 +1919,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
 		ret = 1;
 		break;
 	}
-	if (ret)
-		apic_debug("Local APIC Write to read-only register %x\n", reg);
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(kvm_lapic_reg_write);
@@ -2001,19 +1947,11 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
 	 * 32/64/128 bits registers must be accessed thru 32 bits.
 	 * Refer SDM 8.4.1
 	 */
-	if (len != 4 || (offset & 0xf)) {
-		/* Don't shout loud, $infamous_os would cause only noise. */
-		apic_debug("apic write: bad size=%d %lx\n", len, (long)address);
+	if (len != 4 || (offset & 0xf))
 		return 0;
-	}
 
 	val = *(u32*)data;
 
-	/* too common printing */
-	if (offset != APIC_EOI)
-		apic_debug("%s: offset 0x%x with length 0x%x, and value is "
-			   "0x%x\n", __func__, offset, len, val);
-
 	kvm_lapic_reg_write(apic, offset & 0xff0, val);
 
 	return 0;
@@ -2146,11 +2084,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
 	if ((value & MSR_IA32_APICBASE_ENABLE) &&
 	     apic->base_address != APIC_DEFAULT_PHYS_BASE)
 		pr_warn_once("APIC base relocation is unsupported by KVM");
-
-	/* with FSB delivery interrupt, we can restart APIC functionality */
-	apic_debug("apic base msr is 0x%016" PRIx64 ", and base address is "
-		   "0x%lx.\n", apic->vcpu->arch.apic_base, apic->base_address);
-
 }
 
 void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
@@ -2161,8 +2094,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
 	if (!apic)
 		return;
 
-	apic_debug("%s\n", __func__);
-
 	/* Stop the timer in case it's a reset to an active apic */
 	hrtimer_cancel(&apic->lapic_timer.timer);
 
@@ -2215,11 +2146,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
 
 	vcpu->arch.apic_arb_prio = 0;
 	vcpu->arch.apic_attention = 0;
-
-	apic_debug("%s: vcpu=%p, id=0x%x, base_msr="
-		   "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
-		   vcpu, kvm_lapic_get_reg(apic, APIC_ID),
-		   vcpu->arch.apic_base, apic->base_address);
 }
 
 /*
@@ -2291,7 +2217,6 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
 	struct kvm_lapic *apic;
 
 	ASSERT(vcpu != NULL);
-	apic_debug("apic_init %d\n", vcpu->vcpu_id);
 
 	apic = kzalloc(sizeof(*apic), GFP_KERNEL_ACCOUNT);
 	if (!apic)
@@ -2645,11 +2570,8 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
 	if (!lapic_in_kernel(vcpu) || !apic_x2apic_mode(apic))
 		return 1;
 
-	if (reg == APIC_DFR || reg == APIC_ICR2) {
-		apic_debug("KVM_APIC_READ: read x2apic reserved register %x\n",
-			   reg);
+	if (reg == APIC_DFR || reg == APIC_ICR2)
 		return 1;
-	}
 
 	if (kvm_lapic_reg_read(apic, reg, 4, &low))
 		return 1;
@@ -2747,8 +2669,6 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
 		/* evaluate pending_events before reading the vector */
 		smp_rmb();
 		sipi_vector = apic->sipi_vector;
-		apic_debug("vcpu %d received sipi with vector # %x\n",
-			 vcpu->vcpu_id, sipi_vector);
 		kvm_vcpu_deliver_sipi_vector(vcpu, sipi_vector);
 		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
 	}
-- 
1.8.3.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kvm: x86: ioapic and apic debug macros cleanup
  2019-07-05 17:08 [PATCH] kvm: x86: ioapic and apic debug macros cleanup Yi Wang
@ 2019-07-15  4:59 ` Yi Wang
  2019-07-15 18:39 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Yi Wang @ 2019-07-15  4:59 UTC (permalink / raw)
  To: Yi Wang, pbonzini
  Cc: rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
	xue.zhihong, wang.liang82, Yi Wang

Hi Paolo,
Would you help to review this patch, plz?
Many thanks.

---
Best wishes
Yi Wang

> 在 2019年7月6日,01:08,Yi Wang <wang.yi59@zte.com.cn> 写道:
> 
> The ioapic_debug and apic_debug have been not used
> for years, and kvm tracepoints are enough for debugging,
> so remove them as Paolo suggested.
> 
> However, there may be something wrong when pv evi get/put
> user, so it's better to retain some log there.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> ---
> arch/x86/kvm/ioapic.c | 15 --------
> arch/x86/kvm/lapic.c  | 98 +++++----------------------------------------------
> 2 files changed, 9 insertions(+), 104 deletions(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 1add1bc..d859ae8 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -45,11 +45,6 @@
> #include "lapic.h"
> #include "irq.h"
> 
> -#if 0
> -#define ioapic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg)
> -#else
> -#define ioapic_debug(fmt, arg...)
> -#endif
> static int ioapic_service(struct kvm_ioapic *vioapic, int irq,
>        bool line_status);
> 
> @@ -294,7 +289,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
>    default:
>        index = (ioapic->ioregsel - 0x10) >> 1;
> 
> -        ioapic_debug("change redir index %x val %x\n", index, val);
>        if (index >= IOAPIC_NUM_PINS)
>            return;
>        e = &ioapic->redirtbl[index];
> @@ -343,12 +337,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
>        entry->fields.remote_irr))
>        return -1;
> 
> -    ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
> -             "vector=%x trig_mode=%x\n",
> -             entry->fields.dest_id, entry->fields.dest_mode,
> -             entry->fields.delivery_mode, entry->fields.vector,
> -             entry->fields.trig_mode);
> -
>    irqe.dest_id = entry->fields.dest_id;
>    irqe.vector = entry->fields.vector;
>    irqe.dest_mode = entry->fields.dest_mode;
> @@ -515,7 +503,6 @@ static int ioapic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>    if (!ioapic_in_range(ioapic, addr))
>        return -EOPNOTSUPP;
> 
> -    ioapic_debug("addr %lx\n", (unsigned long)addr);
>    ASSERT(!(addr & 0xf));    /* check alignment */
> 
>    addr &= 0xff;
> @@ -558,8 +545,6 @@ static int ioapic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>    if (!ioapic_in_range(ioapic, addr))
>        return -EOPNOTSUPP;
> 
> -    ioapic_debug("ioapic_mmio_write addr=%p len=%d val=%p\n",
> -             (void*)addr, len, val);
>    ASSERT(!(addr & 0xf));    /* check alignment */
> 
>    switch (len) {
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 4dabc31..0f3b57e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -52,9 +52,6 @@
> #define PRIu64 "u"
> #define PRIo64 "o"
> 
> -/* #define apic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg) */
> -#define apic_debug(fmt, arg...) do {} while (0)
> -
> /* 14 is the version for Xeon and Pentium 8.4.8*/
> #define APIC_VERSION            (0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
> #define LAPIC_MMIO_LENGTH        (1 << 12)
> @@ -631,7 +628,7 @@ static bool pv_eoi_get_pending(struct kvm_vcpu *vcpu)
> {
>    u8 val;
>    if (pv_eoi_get_user(vcpu, &val) < 0)
> -        apic_debug("Can't read EOI MSR value: 0x%llx\n",
> +        printk(KERN_WARNING "Can't read EOI MSR value: 0x%llx\n",
>               (unsigned long long)vcpu->arch.pv_eoi.msr_val);
>    return val & 0x1;
> }
> @@ -639,7 +636,7 @@ static bool pv_eoi_get_pending(struct kvm_vcpu *vcpu)
> static void pv_eoi_set_pending(struct kvm_vcpu *vcpu)
> {
>    if (pv_eoi_put_user(vcpu, KVM_PV_EOI_ENABLED) < 0) {
> -        apic_debug("Can't set EOI MSR value: 0x%llx\n",
> +        printk(KERN_WARNING "Can't set EOI MSR value: 0x%llx\n",
>               (unsigned long long)vcpu->arch.pv_eoi.msr_val);
>        return;
>    }
> @@ -649,7 +646,7 @@ static void pv_eoi_set_pending(struct kvm_vcpu *vcpu)
> static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
> {
>    if (pv_eoi_put_user(vcpu, KVM_PV_EOI_DISABLED) < 0) {
> -        apic_debug("Can't clear EOI MSR value: 0x%llx\n",
> +        printk(KERN_WARNING "Can't clear EOI MSR value: 0x%llx\n",
>               (unsigned long long)vcpu->arch.pv_eoi.msr_val);
>        return;
>    }
> @@ -683,9 +680,6 @@ static bool __apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr)
>    else
>        ppr = isrv & 0xf0;
> 
> -    apic_debug("vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x",
> -           apic, ppr, isr, isrv);
> -
>    *new_ppr = ppr;
>    if (old_ppr != ppr)
>        kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr);
> @@ -762,8 +756,6 @@ static bool kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
>        return ((logical_id >> 4) == (mda >> 4))
>               && (logical_id & mda & 0xf) != 0;
>    default:
> -        apic_debug("Bad DFR vcpu %d: %08x\n",
> -               apic->vcpu->vcpu_id, kvm_lapic_get_reg(apic, APIC_DFR));
>        return false;
>    }
> }
> @@ -802,10 +794,6 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>    struct kvm_lapic *target = vcpu->arch.apic;
>    u32 mda = kvm_apic_mda(vcpu, dest, source, target);
> 
> -    apic_debug("target %p, source %p, dest 0x%x, "
> -           "dest_mode 0x%x, short_hand 0x%x\n",
> -           target, source, dest, dest_mode, short_hand);
> -
>    ASSERT(target);
>    switch (short_hand) {
>    case APIC_DEST_NOSHORT:
> @@ -820,8 +808,6 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>    case APIC_DEST_ALLBUT:
>        return target != source;
>    default:
> -        apic_debug("kvm: apic: Bad dest shorthand value %x\n",
> -               short_hand);
>        return false;
>    }
> }
> @@ -1097,15 +1083,10 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>            smp_wmb();
>            kvm_make_request(KVM_REQ_EVENT, vcpu);
>            kvm_vcpu_kick(vcpu);
> -        } else {
> -            apic_debug("Ignoring de-assert INIT to vcpu %d\n",
> -                   vcpu->vcpu_id);
>        }
>        break;
> 
>    case APIC_DM_STARTUP:
> -        apic_debug("SIPI to vcpu %d vector 0x%02x\n",
> -               vcpu->vcpu_id, vector);
>        result = 1;
>        apic->sipi_vector = vector;
>        /* make sure sipi_vector is visible for the receiver */
> @@ -1223,14 +1204,6 @@ static void apic_send_ipi(struct kvm_lapic *apic)
> 
>    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, "
> -           "msi_redir_hint 0x%x\n",
> -           icr_high, icr_low, irq.shorthand, irq.dest_id,
> -           irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
> -           irq.vector, irq.msi_redir_hint);
> -
>    kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
> }
> 
> @@ -1284,7 +1257,6 @@ static u32 __apic_read(struct kvm_lapic *apic, unsigned int offset)
> 
>    switch (offset) {
>    case APIC_ARBPRI:
> -        apic_debug("Access APIC ARBPRI register which is for P6\n");
>        break;
> 
>    case APIC_TMCCT:    /* Timer CCR */
> @@ -1321,17 +1293,11 @@ int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
>    /* this bitmask has a bit cleared for each reserved register */
>    static const u64 rmask = 0x43ff01ffffffe70cULL;
> 
> -    if ((alignment + len) > 4) {
> -        apic_debug("KVM_APIC_READ: alignment error %x %d\n",
> -               offset, len);
> +    if ((alignment + len) > 4)
>        return 1;
> -    }
> 
> -    if (offset > 0x3f0 || !(rmask & (1ULL << (offset >> 4)))) {
> -        apic_debug("KVM_APIC_READ: read reserved register %x\n",
> -               offset);
> +    if (offset > 0x3f0 || !(rmask & (1ULL << (offset >> 4))))
>        return 1;
> -    }
> 
>    result = __apic_read(apic, offset & ~0xf);
> 
> @@ -1389,9 +1355,6 @@ static void update_divide_count(struct kvm_lapic *apic)
>    tmp1 = tdcr & 0xf;
>    tmp2 = ((tmp1 & 0x3) | ((tmp1 & 0x8) >> 1)) + 1;
>    apic->divide_count = 0x1 << (tmp2 & 0x7);
> -
> -    apic_debug("timer divide count is 0x%x\n",
> -                   apic->divide_count);
> }
> 
> static void limit_periodic_timer_frequency(struct kvm_lapic *apic)
> @@ -1616,16 +1579,6 @@ static bool set_target_expiration(struct kvm_lapic *apic)
> 
>    limit_periodic_timer_frequency(apic);
> 
> -    apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
> -           PRIx64 ", "
> -           "timer initial count 0x%x, period %lldns, "
> -           "expire @ 0x%016" PRIx64 ".\n", __func__,
> -           APIC_BUS_CYCLE_NS, ktime_to_ns(now),
> -           kvm_lapic_get_reg(apic, APIC_TMICT),
> -           apic->lapic_timer.period,
> -           ktime_to_ns(ktime_add_ns(now,
> -                apic->lapic_timer.period)));
> -
>    apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
>        nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
>    apic->lapic_timer.target_expiration = ktime_add_ns(now, apic->lapic_timer.period);
> @@ -1828,8 +1781,6 @@ static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val)
>    if (apic->lvt0_in_nmi_mode != lvt0_in_nmi_mode) {
>        apic->lvt0_in_nmi_mode = lvt0_in_nmi_mode;
>        if (lvt0_in_nmi_mode) {
> -            apic_debug("Receive NMI setting on APIC_LVT0 "
> -                   "for cpu %d\n", apic->vcpu->vcpu_id);
>            atomic_inc(&apic->vcpu->kvm->arch.vapics_in_nmi_mode);
>        } else
>            atomic_dec(&apic->vcpu->kvm->arch.vapics_in_nmi_mode);
> @@ -1943,8 +1894,6 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>    case APIC_TDCR: {
>        uint32_t old_divisor = apic->divide_count;
> 
> -        if (val & 4)
> -            apic_debug("KVM_WRITE:TDCR %x\n", val);
>        kvm_lapic_set_reg(apic, APIC_TDCR, val);
>        update_divide_count(apic);
>        if (apic->divide_count != old_divisor &&
> @@ -1956,10 +1905,8 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>        break;
>    }
>    case APIC_ESR:
> -        if (apic_x2apic_mode(apic) && val != 0) {
> -            apic_debug("KVM_WRITE:ESR not zero %x\n", val);
> +        if (apic_x2apic_mode(apic) && val != 0)
>            ret = 1;
> -        }
>        break;
> 
>    case APIC_SELF_IPI:
> @@ -1972,8 +1919,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>        ret = 1;
>        break;
>    }
> -    if (ret)
> -        apic_debug("Local APIC Write to read-only register %x\n", reg);
> +
>    return ret;
> }
> EXPORT_SYMBOL_GPL(kvm_lapic_reg_write);
> @@ -2001,19 +1947,11 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>     * 32/64/128 bits registers must be accessed thru 32 bits.
>     * Refer SDM 8.4.1
>     */
> -    if (len != 4 || (offset & 0xf)) {
> -        /* Don't shout loud, $infamous_os would cause only noise. */
> -        apic_debug("apic write: bad size=%d %lx\n", len, (long)address);
> +    if (len != 4 || (offset & 0xf))
>        return 0;
> -    }
> 
>    val = *(u32*)data;
> 
> -    /* too common printing */
> -    if (offset != APIC_EOI)
> -        apic_debug("%s: offset 0x%x with length 0x%x, and value is "
> -               "0x%x\n", __func__, offset, len, val);
> -
>    kvm_lapic_reg_write(apic, offset & 0xff0, val);
> 
>    return 0;
> @@ -2146,11 +2084,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
>    if ((value & MSR_IA32_APICBASE_ENABLE) &&
>         apic->base_address != APIC_DEFAULT_PHYS_BASE)
>        pr_warn_once("APIC base relocation is unsupported by KVM");
> -
> -    /* with FSB delivery interrupt, we can restart APIC functionality */
> -    apic_debug("apic base msr is 0x%016" PRIx64 ", and base address is "
> -           "0x%lx.\n", apic->vcpu->arch.apic_base, apic->base_address);
> -
> }
> 
> void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
> @@ -2161,8 +2094,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
>    if (!apic)
>        return;
> 
> -    apic_debug("%s\n", __func__);
> -
>    /* Stop the timer in case it's a reset to an active apic */
>    hrtimer_cancel(&apic->lapic_timer.timer);
> 
> @@ -2215,11 +2146,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
> 
>    vcpu->arch.apic_arb_prio = 0;
>    vcpu->arch.apic_attention = 0;
> -
> -    apic_debug("%s: vcpu=%p, id=0x%x, base_msr="
> -           "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
> -           vcpu, kvm_lapic_get_reg(apic, APIC_ID),
> -           vcpu->arch.apic_base, apic->base_address);
> }
> 
> /*
> @@ -2291,7 +2217,6 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
>    struct kvm_lapic *apic;
> 
>    ASSERT(vcpu != NULL);
> -    apic_debug("apic_init %d\n", vcpu->vcpu_id);
> 
>    apic = kzalloc(sizeof(*apic), GFP_KERNEL_ACCOUNT);
>    if (!apic)
> @@ -2645,11 +2570,8 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
>    if (!lapic_in_kernel(vcpu) || !apic_x2apic_mode(apic))
>        return 1;
> 
> -    if (reg == APIC_DFR || reg == APIC_ICR2) {
> -        apic_debug("KVM_APIC_READ: read x2apic reserved register %x\n",
> -               reg);
> +    if (reg == APIC_DFR || reg == APIC_ICR2)
>        return 1;
> -    }
> 
>    if (kvm_lapic_reg_read(apic, reg, 4, &low))
>        return 1;
> @@ -2747,8 +2669,6 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
>        /* evaluate pending_events before reading the vector */
>        smp_rmb();
>        sipi_vector = apic->sipi_vector;
> -        apic_debug("vcpu %d received sipi with vector # %x\n",
> -             vcpu->vcpu_id, sipi_vector);
>        kvm_vcpu_deliver_sipi_vector(vcpu, sipi_vector);
>        vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
>    }
> -- 
> 1.8.3.1
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] kvm: x86: ioapic and apic debug macros cleanup
  2019-07-05 17:08 [PATCH] kvm: x86: ioapic and apic debug macros cleanup Yi Wang
  2019-07-15  4:59 ` Yi Wang
@ 2019-07-15 18:39 ` Paolo Bonzini
  1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2019-07-15 18:39 UTC (permalink / raw)
  To: Yi Wang
  Cc: rkrcmar, tglx, mingo, bp, hpa, x86, kvm, linux-kernel,
	xue.zhihong, up2wing, wang.liang82

On 05/07/19 19:08, Yi Wang wrote:
> The ioapic_debug and apic_debug have been not used
> for years, and kvm tracepoints are enough for debugging,
> so remove them as Paolo suggested.
> 
> However, there may be something wrong when pv evi get/put
> user, so it's better to retain some log there.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> ---
>  arch/x86/kvm/ioapic.c | 15 --------
>  arch/x86/kvm/lapic.c  | 98 +++++----------------------------------------------
>  2 files changed, 9 insertions(+), 104 deletions(-)
> 
> diff --git a/arch/x86/kvm/ioapic.c b/arch/x86/kvm/ioapic.c
> index 1add1bc..d859ae8 100644
> --- a/arch/x86/kvm/ioapic.c
> +++ b/arch/x86/kvm/ioapic.c
> @@ -45,11 +45,6 @@
>  #include "lapic.h"
>  #include "irq.h"
>  
> -#if 0
> -#define ioapic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg)
> -#else
> -#define ioapic_debug(fmt, arg...)
> -#endif
>  static int ioapic_service(struct kvm_ioapic *vioapic, int irq,
>  		bool line_status);
>  
> @@ -294,7 +289,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *ioapic, u32 val)
>  	default:
>  		index = (ioapic->ioregsel - 0x10) >> 1;
>  
> -		ioapic_debug("change redir index %x val %x\n", index, val);
>  		if (index >= IOAPIC_NUM_PINS)
>  			return;
>  		e = &ioapic->redirtbl[index];
> @@ -343,12 +337,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, int irq, bool line_status)
>  	    entry->fields.remote_irr))
>  		return -1;
>  
> -	ioapic_debug("dest=%x dest_mode=%x delivery_mode=%x "
> -		     "vector=%x trig_mode=%x\n",
> -		     entry->fields.dest_id, entry->fields.dest_mode,
> -		     entry->fields.delivery_mode, entry->fields.vector,
> -		     entry->fields.trig_mode);
> -
>  	irqe.dest_id = entry->fields.dest_id;
>  	irqe.vector = entry->fields.vector;
>  	irqe.dest_mode = entry->fields.dest_mode;
> @@ -515,7 +503,6 @@ static int ioapic_mmio_read(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  	if (!ioapic_in_range(ioapic, addr))
>  		return -EOPNOTSUPP;
>  
> -	ioapic_debug("addr %lx\n", (unsigned long)addr);
>  	ASSERT(!(addr & 0xf));	/* check alignment */
>  
>  	addr &= 0xff;
> @@ -558,8 +545,6 @@ static int ioapic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  	if (!ioapic_in_range(ioapic, addr))
>  		return -EOPNOTSUPP;
>  
> -	ioapic_debug("ioapic_mmio_write addr=%p len=%d val=%p\n",
> -		     (void*)addr, len, val);
>  	ASSERT(!(addr & 0xf));	/* check alignment */
>  
>  	switch (len) {
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index 4dabc31..0f3b57e 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -52,9 +52,6 @@
>  #define PRIu64 "u"
>  #define PRIo64 "o"
>  
> -/* #define apic_debug(fmt,arg...) printk(KERN_WARNING fmt,##arg) */
> -#define apic_debug(fmt, arg...) do {} while (0)
> -
>  /* 14 is the version for Xeon and Pentium 8.4.8*/
>  #define APIC_VERSION			(0x14UL | ((KVM_APIC_LVT_NUM - 1) << 16))
>  #define LAPIC_MMIO_LENGTH		(1 << 12)
> @@ -631,7 +628,7 @@ static bool pv_eoi_get_pending(struct kvm_vcpu *vcpu)
>  {
>  	u8 val;
>  	if (pv_eoi_get_user(vcpu, &val) < 0)
> -		apic_debug("Can't read EOI MSR value: 0x%llx\n",
> +		printk(KERN_WARNING "Can't read EOI MSR value: 0x%llx\n",
>  			   (unsigned long long)vcpu->arch.pv_eoi.msr_val);
>  	return val & 0x1;
>  }
> @@ -639,7 +636,7 @@ static bool pv_eoi_get_pending(struct kvm_vcpu *vcpu)
>  static void pv_eoi_set_pending(struct kvm_vcpu *vcpu)
>  {
>  	if (pv_eoi_put_user(vcpu, KVM_PV_EOI_ENABLED) < 0) {
> -		apic_debug("Can't set EOI MSR value: 0x%llx\n",
> +		printk(KERN_WARNING "Can't set EOI MSR value: 0x%llx\n",
>  			   (unsigned long long)vcpu->arch.pv_eoi.msr_val);
>  		return;
>  	}
> @@ -649,7 +646,7 @@ static void pv_eoi_set_pending(struct kvm_vcpu *vcpu)
>  static void pv_eoi_clr_pending(struct kvm_vcpu *vcpu)
>  {
>  	if (pv_eoi_put_user(vcpu, KVM_PV_EOI_DISABLED) < 0) {
> -		apic_debug("Can't clear EOI MSR value: 0x%llx\n",
> +		printk(KERN_WARNING "Can't clear EOI MSR value: 0x%llx\n",
>  			   (unsigned long long)vcpu->arch.pv_eoi.msr_val);
>  		return;
>  	}
> @@ -683,9 +680,6 @@ static bool __apic_update_ppr(struct kvm_lapic *apic, u32 *new_ppr)
>  	else
>  		ppr = isrv & 0xf0;
>  
> -	apic_debug("vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x",
> -		   apic, ppr, isr, isrv);
> -
>  	*new_ppr = ppr;
>  	if (old_ppr != ppr)
>  		kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr);
> @@ -762,8 +756,6 @@ static bool kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
>  		return ((logical_id >> 4) == (mda >> 4))
>  		       && (logical_id & mda & 0xf) != 0;
>  	default:
> -		apic_debug("Bad DFR vcpu %d: %08x\n",
> -			   apic->vcpu->vcpu_id, kvm_lapic_get_reg(apic, APIC_DFR));
>  		return false;
>  	}
>  }
> @@ -802,10 +794,6 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  	struct kvm_lapic *target = vcpu->arch.apic;
>  	u32 mda = kvm_apic_mda(vcpu, dest, source, target);
>  
> -	apic_debug("target %p, source %p, dest 0x%x, "
> -		   "dest_mode 0x%x, short_hand 0x%x\n",
> -		   target, source, dest, dest_mode, short_hand);
> -
>  	ASSERT(target);
>  	switch (short_hand) {
>  	case APIC_DEST_NOSHORT:
> @@ -820,8 +808,6 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
>  	case APIC_DEST_ALLBUT:
>  		return target != source;
>  	default:
> -		apic_debug("kvm: apic: Bad dest shorthand value %x\n",
> -			   short_hand);
>  		return false;
>  	}
>  }
> @@ -1097,15 +1083,10 @@ static int __apic_accept_irq(struct kvm_lapic *apic, int delivery_mode,
>  			smp_wmb();
>  			kvm_make_request(KVM_REQ_EVENT, vcpu);
>  			kvm_vcpu_kick(vcpu);
> -		} else {
> -			apic_debug("Ignoring de-assert INIT to vcpu %d\n",
> -				   vcpu->vcpu_id);
>  		}
>  		break;
>  
>  	case APIC_DM_STARTUP:
> -		apic_debug("SIPI to vcpu %d vector 0x%02x\n",
> -			   vcpu->vcpu_id, vector);
>  		result = 1;
>  		apic->sipi_vector = vector;
>  		/* make sure sipi_vector is visible for the receiver */
> @@ -1223,14 +1204,6 @@ static void apic_send_ipi(struct kvm_lapic *apic)
>  
>  	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, "
> -		   "msi_redir_hint 0x%x\n",
> -		   icr_high, icr_low, irq.shorthand, irq.dest_id,
> -		   irq.trig_mode, irq.level, irq.dest_mode, irq.delivery_mode,
> -		   irq.vector, irq.msi_redir_hint);
> -
>  	kvm_irq_delivery_to_apic(apic->vcpu->kvm, apic, &irq, NULL);
>  }
>  
> @@ -1284,7 +1257,6 @@ static u32 __apic_read(struct kvm_lapic *apic, unsigned int offset)
>  
>  	switch (offset) {
>  	case APIC_ARBPRI:
> -		apic_debug("Access APIC ARBPRI register which is for P6\n");
>  		break;
>  
>  	case APIC_TMCCT:	/* Timer CCR */
> @@ -1321,17 +1293,11 @@ int kvm_lapic_reg_read(struct kvm_lapic *apic, u32 offset, int len,
>  	/* this bitmask has a bit cleared for each reserved register */
>  	static const u64 rmask = 0x43ff01ffffffe70cULL;
>  
> -	if ((alignment + len) > 4) {
> -		apic_debug("KVM_APIC_READ: alignment error %x %d\n",
> -			   offset, len);
> +	if ((alignment + len) > 4)
>  		return 1;
> -	}
>  
> -	if (offset > 0x3f0 || !(rmask & (1ULL << (offset >> 4)))) {
> -		apic_debug("KVM_APIC_READ: read reserved register %x\n",
> -			   offset);
> +	if (offset > 0x3f0 || !(rmask & (1ULL << (offset >> 4))))
>  		return 1;
> -	}
>  
>  	result = __apic_read(apic, offset & ~0xf);
>  
> @@ -1389,9 +1355,6 @@ static void update_divide_count(struct kvm_lapic *apic)
>  	tmp1 = tdcr & 0xf;
>  	tmp2 = ((tmp1 & 0x3) | ((tmp1 & 0x8) >> 1)) + 1;
>  	apic->divide_count = 0x1 << (tmp2 & 0x7);
> -
> -	apic_debug("timer divide count is 0x%x\n",
> -				   apic->divide_count);
>  }
>  
>  static void limit_periodic_timer_frequency(struct kvm_lapic *apic)
> @@ -1616,16 +1579,6 @@ static bool set_target_expiration(struct kvm_lapic *apic)
>  
>  	limit_periodic_timer_frequency(apic);
>  
> -	apic_debug("%s: bus cycle is %" PRId64 "ns, now 0x%016"
> -		   PRIx64 ", "
> -		   "timer initial count 0x%x, period %lldns, "
> -		   "expire @ 0x%016" PRIx64 ".\n", __func__,
> -		   APIC_BUS_CYCLE_NS, ktime_to_ns(now),
> -		   kvm_lapic_get_reg(apic, APIC_TMICT),
> -		   apic->lapic_timer.period,
> -		   ktime_to_ns(ktime_add_ns(now,
> -				apic->lapic_timer.period)));
> -
>  	apic->lapic_timer.tscdeadline = kvm_read_l1_tsc(apic->vcpu, tscl) +
>  		nsec_to_cycles(apic->vcpu, apic->lapic_timer.period);
>  	apic->lapic_timer.target_expiration = ktime_add_ns(now, apic->lapic_timer.period);
> @@ -1828,8 +1781,6 @@ static void apic_manage_nmi_watchdog(struct kvm_lapic *apic, u32 lvt0_val)
>  	if (apic->lvt0_in_nmi_mode != lvt0_in_nmi_mode) {
>  		apic->lvt0_in_nmi_mode = lvt0_in_nmi_mode;
>  		if (lvt0_in_nmi_mode) {
> -			apic_debug("Receive NMI setting on APIC_LVT0 "
> -				   "for cpu %d\n", apic->vcpu->vcpu_id);
>  			atomic_inc(&apic->vcpu->kvm->arch.vapics_in_nmi_mode);
>  		} else
>  			atomic_dec(&apic->vcpu->kvm->arch.vapics_in_nmi_mode);
> @@ -1943,8 +1894,6 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  	case APIC_TDCR: {
>  		uint32_t old_divisor = apic->divide_count;
>  
> -		if (val & 4)
> -			apic_debug("KVM_WRITE:TDCR %x\n", val);
>  		kvm_lapic_set_reg(apic, APIC_TDCR, val);
>  		update_divide_count(apic);
>  		if (apic->divide_count != old_divisor &&
> @@ -1956,10 +1905,8 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  		break;
>  	}
>  	case APIC_ESR:
> -		if (apic_x2apic_mode(apic) && val != 0) {
> -			apic_debug("KVM_WRITE:ESR not zero %x\n", val);
> +		if (apic_x2apic_mode(apic) && val != 0)
>  			ret = 1;
> -		}
>  		break;
>  
>  	case APIC_SELF_IPI:
> @@ -1972,8 +1919,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 reg, u32 val)
>  		ret = 1;
>  		break;
>  	}
> -	if (ret)
> -		apic_debug("Local APIC Write to read-only register %x\n", reg);
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(kvm_lapic_reg_write);
> @@ -2001,19 +1947,11 @@ static int apic_mmio_write(struct kvm_vcpu *vcpu, struct kvm_io_device *this,
>  	 * 32/64/128 bits registers must be accessed thru 32 bits.
>  	 * Refer SDM 8.4.1
>  	 */
> -	if (len != 4 || (offset & 0xf)) {
> -		/* Don't shout loud, $infamous_os would cause only noise. */
> -		apic_debug("apic write: bad size=%d %lx\n", len, (long)address);
> +	if (len != 4 || (offset & 0xf))
>  		return 0;
> -	}
>  
>  	val = *(u32*)data;
>  
> -	/* too common printing */
> -	if (offset != APIC_EOI)
> -		apic_debug("%s: offset 0x%x with length 0x%x, and value is "
> -			   "0x%x\n", __func__, offset, len, val);
> -
>  	kvm_lapic_reg_write(apic, offset & 0xff0, val);
>  
>  	return 0;
> @@ -2146,11 +2084,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 value)
>  	if ((value & MSR_IA32_APICBASE_ENABLE) &&
>  	     apic->base_address != APIC_DEFAULT_PHYS_BASE)
>  		pr_warn_once("APIC base relocation is unsupported by KVM");
> -
> -	/* with FSB delivery interrupt, we can restart APIC functionality */
> -	apic_debug("apic base msr is 0x%016" PRIx64 ", and base address is "
> -		   "0x%lx.\n", apic->vcpu->arch.apic_base, apic->base_address);
> -
>  }
>  
>  void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
> @@ -2161,8 +2094,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
>  	if (!apic)
>  		return;
>  
> -	apic_debug("%s\n", __func__);
> -
>  	/* Stop the timer in case it's a reset to an active apic */
>  	hrtimer_cancel(&apic->lapic_timer.timer);
>  
> @@ -2215,11 +2146,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event)
>  
>  	vcpu->arch.apic_arb_prio = 0;
>  	vcpu->arch.apic_attention = 0;
> -
> -	apic_debug("%s: vcpu=%p, id=0x%x, base_msr="
> -		   "0x%016" PRIx64 ", base_address=0x%0lx.\n", __func__,
> -		   vcpu, kvm_lapic_get_reg(apic, APIC_ID),
> -		   vcpu->arch.apic_base, apic->base_address);
>  }
>  
>  /*
> @@ -2291,7 +2217,6 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int timer_advance_ns)
>  	struct kvm_lapic *apic;
>  
>  	ASSERT(vcpu != NULL);
> -	apic_debug("apic_init %d\n", vcpu->vcpu_id);
>  
>  	apic = kzalloc(sizeof(*apic), GFP_KERNEL_ACCOUNT);
>  	if (!apic)
> @@ -2645,11 +2570,8 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 msr, u64 *data)
>  	if (!lapic_in_kernel(vcpu) || !apic_x2apic_mode(apic))
>  		return 1;
>  
> -	if (reg == APIC_DFR || reg == APIC_ICR2) {
> -		apic_debug("KVM_APIC_READ: read x2apic reserved register %x\n",
> -			   reg);
> +	if (reg == APIC_DFR || reg == APIC_ICR2)
>  		return 1;
> -	}
>  
>  	if (kvm_lapic_reg_read(apic, reg, 4, &low))
>  		return 1;
> @@ -2747,8 +2669,6 @@ void kvm_apic_accept_events(struct kvm_vcpu *vcpu)
>  		/* evaluate pending_events before reading the vector */
>  		smp_rmb();
>  		sipi_vector = apic->sipi_vector;
> -		apic_debug("vcpu %d received sipi with vector # %x\n",
> -			 vcpu->vcpu_id, sipi_vector);
>  		kvm_vcpu_deliver_sipi_vector(vcpu, sipi_vector);
>  		vcpu->arch.mp_state = KVM_MP_STATE_RUNNABLE;
>  	}
> 

Queued, thanks.

Paolo

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-05 17:08 [PATCH] kvm: x86: ioapic and apic debug macros cleanup Yi Wang
2019-07-15  4:59 ` Yi Wang
2019-07-15 18:39 ` Paolo Bonzini

KVM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvm/0 kvm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvm kvm/ https://lore.kernel.org/kvm \
		kvm@vger.kernel.org kvm@archiver.kernel.org
	public-inbox-index kvm


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.kvm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox