From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A0B93C7618A for ; Mon, 15 Jul 2019 05:00:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 64EF2214C6 for ; Mon, 15 Jul 2019 05:00:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="E0LyV5qF" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726292AbfGOFAA (ORCPT ); Mon, 15 Jul 2019 01:00:00 -0400 Received: from mail-qt1-f193.google.com ([209.85.160.193]:34145 "EHLO mail-qt1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbfGOE77 (ORCPT ); Mon, 15 Jul 2019 00:59:59 -0400 Received: by mail-qt1-f193.google.com with SMTP id k10so14376238qtq.1; Sun, 14 Jul 2019 21:59:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=aahHm3WCNcinBb5h+NoDXnG+b881MXcN71ZGuuhwk1s=; b=E0LyV5qFGikhejAB3iheHHgITssnosZ9cYMNnSMKXvbVLzieP/9UT6A8lz/l5J60jQ WhJ3tOPIWuGYcXXIExxTKbTehEqAfwQSDKyj+Bdk5fV4NiBr0s+ImT65KqH9qYw2bzOC HuMJN0/O3IoK8C6yQPEiJvT8Lv/1us3avJpDbdawnzMICp9HNDSItG4sdx2nq91FJ+Nl FJDtXhQlYJhNZaxlDvmzkbkyZSdzq0hlJL5GpZM090dAZAfrLbrR9FSM52WeTaJ3OnKh uWweQQCy9yJVJclJwUIiiic3ih+81DjbyvU3EQ5V7bhsC6KfIViE6ufmXUI0vuklUutf hrow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=aahHm3WCNcinBb5h+NoDXnG+b881MXcN71ZGuuhwk1s=; b=gZxmCgVOND9YT2tocapRwZI7rdzt/eS7vHoTqQ3UhbmJi/4tPkfs0S1cNfXTpaJvHL VNBBv37pu/LWJNUZ6F93BKdeMSlppmo1yvcG9782AOGmvY9sWnyDGOlZEXXpmqrOmnF9 CjI/7vUV7hQ30OBmBVLIFwZVrVah9W+Lp2HHvxLIifZiUJHhiUPKh0KN1BDvpurz4Pki hS/0CVyTbKCDUxMqovxF+2KWxdIyvoWiRGA+XX22mlNQjGWUab+lMw6QhhY3t4nJVnB4 CPce6A92mMiG/OpchO14iba97yGyDcD8G4EzY2SaGXn0AodjAI3fIYWgglWSRKcZSS3W EVJg== X-Gm-Message-State: APjAAAXwU4JQ8OG2BEx2R78A9T9TMtTDbU5PasD18gMCvAhU5QRn7JYF dJgYFWcj7BLNVtLbpllxIr8= X-Google-Smtp-Source: APXvYqxGJcwegcJeE089CuQzSSi58rS27UPw83/memuQGkWt3zBwI8by/1tKrdgV0Vk0ZZAYJbWf8g== X-Received: by 2002:ac8:2e5d:: with SMTP id s29mr16364847qta.70.1563166798506; Sun, 14 Jul 2019 21:59:58 -0700 (PDT) Received: from [240.0.0.1] ([104.168.52.128]) by smtp.gmail.com with ESMTPSA id f133sm7538442qke.62.2019.07.14.21.59.55 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 14 Jul 2019 21:59:58 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (1.0) Subject: Re: [PATCH] kvm: x86: ioapic and apic debug macros cleanup From: Yi Wang X-Mailer: iPhone Mail (16F203) In-Reply-To: <1562346528-840-1-git-send-email-wang.yi59@zte.com.cn> Date: Mon, 15 Jul 2019 12:59:51 +0800 Cc: rkrcmar@redhat.com, tglx@linutronix.de, mingo@redhat.com, bp@alien8.de, hpa@zytor.com, x86@kernel.org, kvm@vger.kernel.org, linux-kernel@vger.kernel.org, xue.zhihong@zte.com.cn, wang.liang82@zte.com.cn, Yi Wang Content-Transfer-Encoding: quoted-printable Message-Id: <2E6A99FD-CA67-44E7-9ADB-A037CFDFB0CA@gmail.com> References: <1562346528-840-1-git-send-email-wang.yi59@zte.com.cn> To: Yi Wang , pbonzini@redhat.com Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Paolo, Would you help to review this patch, plz? Many thanks. --- Best wishes Yi Wang > =E5=9C=A8 2019=E5=B9=B47=E6=9C=886=E6=97=A5=EF=BC=8C01:08=EF=BC=8CYi Wang <= wang.yi59@zte.com.cn> =E5=86=99=E9=81=93=EF=BC=9A >=20 > 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. >=20 > However, there may be something wrong when pv evi get/put > user, so it's better to retain some log there. >=20 > Signed-off-by: Yi Wang > --- > arch/x86/kvm/ioapic.c | 15 -------- > arch/x86/kvm/lapic.c | 98 +++++------------------------------------------= ---- > 2 files changed, 9 insertions(+), 104 deletions(-) >=20 > 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" >=20 > -#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); >=20 > @@ -294,7 +289,6 @@ static void ioapic_write_indirect(struct kvm_ioapic *i= oapic, u32 val) > default: > index =3D (ioapic->ioregsel - 0x10) >> 1; >=20 > - ioapic_debug("change redir index %x val %x\n", index, val); > if (index >=3D IOAPIC_NUM_PINS) > return; > e =3D &ioapic->redirtbl[index]; > @@ -343,12 +337,6 @@ static int ioapic_service(struct kvm_ioapic *ioapic, i= nt irq, bool line_status) > entry->fields.remote_irr)) > return -1; >=20 > - ioapic_debug("dest=3D%x dest_mode=3D%x delivery_mode=3D%x " > - "vector=3D%x trig_mode=3D%x\n", > - entry->fields.dest_id, entry->fields.dest_mode, > - entry->fields.delivery_mode, entry->fields.vector, > - entry->fields.trig_mode); > - > irqe.dest_id =3D entry->fields.dest_id; > irqe.vector =3D entry->fields.vector; > irqe.dest_mode =3D entry->fields.dest_mode; > @@ -515,7 +503,6 @@ static int ioapic_mmio_read(struct kvm_vcpu *vcpu, str= uct kvm_io_device *this, > if (!ioapic_in_range(ioapic, addr)) > return -EOPNOTSUPP; >=20 > - ioapic_debug("addr %lx\n", (unsigned long)addr); > ASSERT(!(addr & 0xf)); /* check alignment */ >=20 > addr &=3D 0xff; > @@ -558,8 +545,6 @@ static int ioapic_mmio_write(struct kvm_vcpu *vcpu, st= ruct kvm_io_device *this, > if (!ioapic_in_range(ioapic, addr)) > return -EOPNOTSUPP; >=20 > - ioapic_debug("ioapic_mmio_write addr=3D%p len=3D%d val=3D%p\n", > - (void*)addr, len, val); > ASSERT(!(addr & 0xf)); /* check alignment */ >=20 > 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" >=20 > -/* #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, u= 32 *new_ppr) > else > ppr =3D isrv & 0xf0; >=20 > - apic_debug("vlapic %p, ppr 0x%x, isr 0x%x, isrv 0x%x", > - apic, ppr, isr, isrv); > - > *new_ppr =3D ppr; > if (old_ppr !=3D ppr) > kvm_lapic_set_reg(apic, APIC_PROCPRI, ppr); > @@ -762,8 +756,6 @@ static bool kvm_apic_match_logical_addr(struct kvm_lap= ic *apic, u32 mda) > return ((logical_id >> 4) =3D=3D (mda >> 4)) > && (logical_id & mda & 0xf) !=3D 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, struc= t kvm_lapic *source, > struct kvm_lapic *target =3D vcpu->arch.apic; > u32 mda =3D kvm_apic_mda(vcpu, dest, source, target); >=20 > - 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 !=3D 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 *api= c, 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; >=20 > case APIC_DM_STARTUP: > - apic_debug("SIPI to vcpu %d vector 0x%02x\n", > - vcpu->vcpu_id, vector); > result =3D 1; > apic->sipi_vector =3D vector; > /* make sure sipi_vector is visible for the receiver */ > @@ -1223,14 +1204,6 @@ static void apic_send_ipi(struct kvm_lapic *apic) >=20 > trace_kvm_apic_ipi(icr_low, irq.dest_id); >=20 > - 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); > } >=20 > @@ -1284,7 +1257,6 @@ static u32 __apic_read(struct kvm_lapic *apic, unsig= ned int offset) >=20 > switch (offset) { > case APIC_ARBPRI: > - apic_debug("Access APIC ARBPRI register which is for P6\n"); > break; >=20 > 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 =3D 0x43ff01ffffffe70cULL; >=20 > - if ((alignment + len) > 4) { > - apic_debug("KVM_APIC_READ: alignment error %x %d\n", > - offset, len); > + if ((alignment + len) > 4) > return 1; > - } >=20 > - 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; > - } >=20 > result =3D __apic_read(apic, offset & ~0xf); >=20 > @@ -1389,9 +1355,6 @@ static void update_divide_count(struct kvm_lapic *ap= ic) > tmp1 =3D tdcr & 0xf; > tmp2 =3D ((tmp1 & 0x3) | ((tmp1 & 0x8) >> 1)) + 1; > apic->divide_count =3D 0x1 << (tmp2 & 0x7); > - > - apic_debug("timer divide count is 0x%x\n", > - apic->divide_count); > } >=20 > static void limit_periodic_timer_frequency(struct kvm_lapic *apic) > @@ -1616,16 +1579,6 @@ static bool set_target_expiration(struct kvm_lapic *= apic) >=20 > limit_periodic_timer_frequency(apic); >=20 > - 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 =3D kvm_read_l1_tsc(apic->vcpu, tscl) + > nsec_to_cycles(apic->vcpu, apic->lapic_timer.period); > apic->lapic_timer.target_expiration =3D ktime_add_ns(now, apic->lapic_t= imer.period); > @@ -1828,8 +1781,6 @@ static void apic_manage_nmi_watchdog(struct kvm_lapi= c *apic, u32 lvt0_val) > if (apic->lvt0_in_nmi_mode !=3D lvt0_in_nmi_mode) { > apic->lvt0_in_nmi_mode =3D 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 r= eg, u32 val) > case APIC_TDCR: { > uint32_t old_divisor =3D apic->divide_count; >=20 > - 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 !=3D 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 !=3D 0) { > - apic_debug("KVM_WRITE:ESR not zero %x\n", val); > + if (apic_x2apic_mode(apic) && val !=3D 0) > ret =3D 1; > - } > break; >=20 > case APIC_SELF_IPI: > @@ -1972,8 +1919,7 @@ int kvm_lapic_reg_write(struct kvm_lapic *apic, u32 r= eg, u32 val) > ret =3D 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, s= truct kvm_io_device *this, > * 32/64/128 bits registers must be accessed thru 32 bits. > * Refer SDM 8.4.1 > */ > - if (len !=3D 4 || (offset & 0xf)) { > - /* Don't shout loud, $infamous_os would cause only noise. */ > - apic_debug("apic write: bad size=3D%d %lx\n", len, (long)address)= ; > + if (len !=3D 4 || (offset & 0xf)) > return 0; > - } >=20 > val =3D *(u32*)data; >=20 > - /* too common printing */ > - if (offset !=3D 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); >=20 > return 0; > @@ -2146,11 +2084,6 @@ void kvm_lapic_set_base(struct kvm_vcpu *vcpu, u64 v= alue) > if ((value & MSR_IA32_APICBASE_ENABLE) && > apic->base_address !=3D 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); > - > } >=20 > void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool init_event) > @@ -2161,8 +2094,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool ini= t_event) > if (!apic) > return; >=20 > - apic_debug("%s\n", __func__); > - > /* Stop the timer in case it's a reset to an active apic */ > hrtimer_cancel(&apic->lapic_timer.timer); >=20 > @@ -2215,11 +2146,6 @@ void kvm_lapic_reset(struct kvm_vcpu *vcpu, bool in= it_event) >=20 > vcpu->arch.apic_arb_prio =3D 0; > vcpu->arch.apic_attention =3D 0; > - > - apic_debug("%s: vcpu=3D%p, id=3D0x%x, base_msr=3D" > - "0x%016" PRIx64 ", base_address=3D0x%0lx.\n", __func__, > - vcpu, kvm_lapic_get_reg(apic, APIC_ID), > - vcpu->arch.apic_base, apic->base_address); > } >=20 > /* > @@ -2291,7 +2217,6 @@ int kvm_create_lapic(struct kvm_vcpu *vcpu, int time= r_advance_ns) > struct kvm_lapic *apic; >=20 > ASSERT(vcpu !=3D NULL); > - apic_debug("apic_init %d\n", vcpu->vcpu_id); >=20 > apic =3D kzalloc(sizeof(*apic), GFP_KERNEL_ACCOUNT); > if (!apic) > @@ -2645,11 +2570,8 @@ int kvm_x2apic_msr_read(struct kvm_vcpu *vcpu, u32 m= sr, u64 *data) > if (!lapic_in_kernel(vcpu) || !apic_x2apic_mode(apic)) > return 1; >=20 > - if (reg =3D=3D APIC_DFR || reg =3D=3D APIC_ICR2) { > - apic_debug("KVM_APIC_READ: read x2apic reserved register %x\n", > - reg); > + if (reg =3D=3D APIC_DFR || reg =3D=3D APIC_ICR2) > return 1; > - } >=20 > 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 =3D 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 =3D KVM_MP_STATE_RUNNABLE; > } > --=20 > 1.8.3.1 >=20