From patchwork Wed Jan 25 12:38:40 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 9536997 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 93EE5601D7 for ; Wed, 25 Jan 2017 12:40:22 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 84BB127DF9 for ; Wed, 25 Jan 2017 12:40:22 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 7932027F81; Wed, 25 Jan 2017 12:40:22 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_SORBS_SPAM,T_DKIM_INVALID autolearn=no version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E4C8B27DF9 for ; Wed, 25 Jan 2017 12:40:20 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cWMs2-0004AD-HY; Wed, 25 Jan 2017 12:40:14 +0000 Received: from mail-lf0-f44.google.com ([209.85.215.44]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cWMrx-0002xj-EP for linux-arm-kernel@lists.infradead.org; Wed, 25 Jan 2017 12:40:12 +0000 Received: by mail-lf0-f44.google.com with SMTP id v186so127974122lfa.1 for ; Wed, 25 Jan 2017 04:39:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=Iizfs8+aJzwIIVPaK+CWWFZR2xxJOpG/v5rGnkhhNDw=; b=HnC8fqhgan+WgFD2BIuVba01cutE7iBhHmyqtxnyGQbAqHHA0DLyKJmWUtMPpqbRzg 1nT57SiYZaIJSRpD9CjUgq1toZk9WgH/SUzJebGBZMzlbiUCsJ8MyyQI1gNnoNFw/A06 cX41Jr3qerRDwcnsQNQceIia/NqScfVPaDjqs= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=Iizfs8+aJzwIIVPaK+CWWFZR2xxJOpG/v5rGnkhhNDw=; b=l/TdqgbWwZ1mnDn9tPJ83Ywhv6nEjilOhkz84/y0akuDpPekAKu9b7G05IyC7g74jJ cZAqPQSDnv2IMGHHUGdAOLnWd7SjtS02TjmYE3KlY+qb5yatBdxZnOJzb9IGb4qTsBAk OBbnGMeBy9HDCIbtJrSc421n8EYoGWS7ccqfRkHkkiH2/5dbZVg7GO/jVHqYNSFMkYPV fbpBhgAuFVj+ikDp0cLZezlWd1CNGIl6WxeGOGdAVewhPsF8ZBCQqxmbADrRWqqwF9QO yqYe+DSBjQWLlE6mpzmn4cYFv4X3ghRwiTlc5UYRjUwJbIbg+UcrTiDLykJTE9pG/rWE RARQ== X-Gm-Message-State: AIkVDXJBLbsAHPUXpr2gsYhju6o558dIJFQCJL84ZPc2t8LOtPcbP4blGFqJyvCTnd8nF/vI X-Received: by 10.25.190.7 with SMTP id o7mr4606634lff.87.1485347926678; Wed, 25 Jan 2017 04:38:46 -0800 (PST) Received: from localhost (x1-6-50-6a-03-de-ec-c2.cpe.webspeed.dk. [2.108.209.202]) by smtp.gmail.com with ESMTPSA id m18sm49422lfe.45.2017.01.25.04.38.45 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 25 Jan 2017 04:38:45 -0800 (PST) Date: Wed, 25 Jan 2017 13:38:40 +0100 From: Christoffer Dall To: Auger Eric Subject: Re: [PATCH v2] KVM: arm/arm64: vgic: Add debugfs vgic-state file Message-ID: <20170125123840.GT15850@cbox> References: <20170124132555.19094-1-christoffer.dall@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170125_044009_878979_DF87F79E X-CRM114-Status: GOOD ( 39.34 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, Marc Zyngier , Andre Przywara , Alex Bennee , kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Jan 25, 2017 at 10:07:15AM +0100, Auger Eric wrote: > Hi Christoffer, > > On 24/01/2017 14:25, Christoffer Dall wrote: > > Add a file to debugfs to read the in-kernel state of the vgic. We don't > > do any locking of the entire VGIC state while traversing all the IRQs, > > so if the VM is running the user/developer may not see a quiesced state, > > but should take care to pause the VM using facilities in user space for > > that purpose. > > > > We also don't support LPIs yet, but they can be added easily if needed. > > > > Signed-off-by: Christoffer Dall > > --- > > Changes since v1: > > - Check error code in vgic_debug_stop so that users which did not > > succeed in opening the seq file also doesn't do the cleanup, which > > could lead to a kernel crash with multiple readers of this file > > before. > > - Renamed some functions to be a bit less cute > > - Rebased on the patch that gets rid of the pendin field in struct > > vgic_irq. > > > > arch/arm/kvm/Makefile | 1 + > > arch/arm64/kvm/Makefile | 1 + > > include/kvm/arm_vgic.h | 5 + > > virt/kvm/arm/vgic/vgic-debug.c | 284 +++++++++++++++++++++++++++++++++++++++++ > > virt/kvm/arm/vgic/vgic-init.c | 4 + > > virt/kvm/arm/vgic/vgic.h | 3 + > > 6 files changed, 298 insertions(+) > > create mode 100644 virt/kvm/arm/vgic/vgic-debug.c > > > > diff --git a/arch/arm/kvm/Makefile b/arch/arm/kvm/Makefile > > index d571243..12b6281 100644 > > --- a/arch/arm/kvm/Makefile > > +++ b/arch/arm/kvm/Makefile > > @@ -33,5 +33,6 @@ obj-y += $(KVM)/arm/vgic/vgic-mmio-v2.o > > obj-y += $(KVM)/arm/vgic/vgic-mmio-v3.o > > obj-y += $(KVM)/arm/vgic/vgic-kvm-device.o > > obj-y += $(KVM)/arm/vgic/vgic-its.o > > +obj-y += $(KVM)/arm/vgic/vgic-debug.o > > obj-y += $(KVM)/irqchip.o > > obj-y += $(KVM)/arm/arch_timer.o > > diff --git a/arch/arm64/kvm/Makefile b/arch/arm64/kvm/Makefile > > index d50a82a..e025bec 100644 > > --- a/arch/arm64/kvm/Makefile > > +++ b/arch/arm64/kvm/Makefile > > @@ -31,6 +31,7 @@ kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v2.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-mmio-v3.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-kvm-device.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-its.o > > +kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/vgic/vgic-debug.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/irqchip.o > > kvm-$(CONFIG_KVM_ARM_HOST) += $(KVM)/arm/arch_timer.o > > kvm-$(CONFIG_KVM_ARM_PMU) += $(KVM)/arm/pmu.o > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > > index da2ce08..0af1477 100644 > > --- a/include/kvm/arm_vgic.h > > +++ b/include/kvm/arm_vgic.h > > @@ -166,6 +166,8 @@ struct vgic_its { > > struct list_head collection_list; > > }; > > > > +struct vgic_state_iter; > > + > > struct vgic_dist { > > bool in_kernel; > > bool ready; > > @@ -213,6 +215,9 @@ struct vgic_dist { > > spinlock_t lpi_list_lock; > > struct list_head lpi_list_head; > > int lpi_list_count; > > + > > + /* used by vgic-debug */ > > + struct vgic_state_iter *iter; > > }; > > > > struct vgic_v2_cpu_if { > > diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c > > new file mode 100644 > > index 0000000..ec466a6 > > --- /dev/null > > +++ b/virt/kvm/arm/vgic/vgic-debug.c > > @@ -0,0 +1,284 @@ > > +/* > > + * Copyright (C) 2016 Linaro > > + * Author: Christoffer Dall > > + * > > + * This program is free software; you can redistribute it and/or modify > > + * it under the terms of the GNU General Public License version 2 as > > + * published by the Free Software Foundation. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program. If not, see . > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > +#include > not requested > > +#include > > +#include > > +#include "vgic.h" > > + > > +/* > > + * Structure to control looping through the entire vgic state. We start at > > + * zero for each field and move upwards. So, if dist_id is 0 we print the > > + * distributor info. When dist_id is 1, we have already printed it and move > > + * on. > > + * > > + * When vcpu_id < nr_cpus we print the vcpu info until vcpu_id == nr_cpus and > > + * so on. > > + */ > > +struct vgic_state_iter { > > + int nr_cpus; > > + int nr_spis; > > + int dist_id; > > + int vcpu_id; > > + int intid; > > +}; > > + > > +static void iter_next(struct vgic_state_iter *iter) > > +{ > > + if (iter->dist_id == 0) { > > + iter->dist_id++; > > + return; > > + } > > + > > + iter->intid++; > > + if (iter->intid == VGIC_NR_PRIVATE_IRQS && > > + ++iter->vcpu_id < iter->nr_cpus) > > + iter->intid = 0; > > +} > > + > > +static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter, > > + loff_t pos) > > +{ > > + int nr_cpus = atomic_read(&kvm->online_vcpus); > > + > > + memset(iter, 0, sizeof(*iter)); > > + > > + iter->nr_cpus = nr_cpus; > > + iter->nr_spis = kvm->arch.vgic.nr_spis; > > + > > + /* Fast forward to the right position if needed */ > > + while (pos--) > > + iter_next(iter); > > +} > > + > > +static bool end_of_vgic(struct vgic_state_iter *iter) > > +{ > > + return iter->dist_id > 0 && > > + iter->vcpu_id == iter->nr_cpus && > > + (iter->intid - VGIC_NR_PRIVATE_IRQS) == iter->nr_spis; > > +} > > + > > +static void *vgic_debug_start(struct seq_file *s, loff_t *pos) > > +{ > > + struct kvm *kvm = (struct kvm *)s->private; > > + struct vgic_state_iter *iter; > > + > > + mutex_lock(&kvm->lock); > > + iter = kvm->arch.vgic.iter; > > + if (iter) { > > + iter = ERR_PTR(-EBUSY); > > + goto out; > > + } > > + > > + iter = kmalloc(sizeof(*iter), GFP_KERNEL); > > + if (!iter) { > > + iter = ERR_PTR(-ENOMEM); > > + goto out; > > + } > > + > > + iter_init(kvm, iter, *pos); > > + kvm->arch.vgic.iter = iter; > > + > > + if (end_of_vgic(iter)) > > + iter = NULL; > > +out: > > + mutex_unlock(&kvm->lock); > > + return iter; > > +} > > + > > +static void *vgic_debug_next(struct seq_file *s, void *v, loff_t *pos) > > +{ > > + struct kvm *kvm = (struct kvm *)s->private; > > + struct vgic_state_iter *iter = kvm->arch.vgic.iter; > > + > > + ++*pos; > > + iter_next(iter); > > + if (end_of_vgic(iter)) > > + iter = NULL; > > + return iter; > > +} > > + > > +static void vgic_debug_stop(struct seq_file *s, void *v) > > +{ > > + struct kvm *kvm = (struct kvm *)s->private; > > + struct vgic_state_iter *iter; > > + > > + /* > > + * If the seq file wasn't properly opened, there's nothing to clearn > > + * up. > > + */ > > + if (IS_ERR(v)) > > + return; > > + > > + mutex_lock(&kvm->lock); > > + iter = kvm->arch.vgic.iter; > > + kfree(iter); > > + kvm->arch.vgic.iter = NULL; > > + mutex_unlock(&kvm->lock); > > +} > > + > > +static void print_dist_state(struct seq_file *s, struct vgic_dist *dist) > > +{ > > + seq_printf(s, "Distributor\n"); > > + seq_printf(s, "===========\n"); > > + seq_printf(s, "vgic_model:\t%s\n", > > + (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V3) ? > > + "GICv3" : "GICv2"); > > + seq_printf(s, "nr_spis:\t%d\n", dist->nr_spis); > > + seq_printf(s, "enabled:\t%d\n", dist->enabled); > > + seq_printf(s, "\n"); > > + > > + seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n"); > > + seq_printf(s, "E=enabled, H=hw, L=config_level\n"); > s/Pending/Pending Latched > s/S=soft_pending, // > > +} > > + > > +static void print_header(struct seq_file *s, struct vgic_irq *irq, > > + struct kvm_vcpu *vcpu) > > +{ > > + int id = 0; > > + char *hdr = "SPI "; > > + > > + if (vcpu) { > > + hdr = "VCPU"; > > + id = vcpu->vcpu_id; > > + } > > + > > + seq_printf(s, "\n"); > > + seq_printf(s, "%s%2d TYP ID TGT_ID PLAEHL HWID TARGET SRC PRI VCPU_ID\n", hdr, id); > > + seq_printf(s, "---------------------------------------------------------------\n"); > > +} > > + > > +static void print_irq_state(struct seq_file *s, struct vgic_irq *irq, > > + struct kvm_vcpu *vcpu) > > +{ > > + char *type; > > + if (irq->intid < VGIC_NR_SGIS) > > + type = "SGI"; > > + else if (irq->intid < VGIC_NR_PRIVATE_IRQS) > > + type = "PPI"; > > + else > > + type = "SPI"; > > + > > + if (irq->intid ==0 || irq->intid == VGIC_NR_PRIVATE_IRQS) > > + print_header(s, irq, vcpu); > > + > > + seq_printf(s, " %s %4d " > > + " %2d " > > + "%d%d%d%d%d%d " > > + "%8x " > > + "%8x " > > + " %2x " > > + " %2x " > > + " %2d " > > + "\n", > > + type, irq->intid, > > + (irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1, > > + irq->pending_latch, > > + irq->line_level, > > + irq->active, > > + irq->enabled, > > + irq->hw, > > + irq->config == VGIC_CONFIG_LEVEL, > > + irq->hwintid, > nit: why not choosing a %4d format like intid? > > + irq->mpidr, > is it set for GICv2? > > + irq->source, > > + irq->priority, > nit: decimal format for prio? > > + (irq->vcpu) ? irq->vcpu->vcpu_id : -1); > > + > > +} > > + > > +static int vgic_debug_show(struct seq_file *s, void *v) > > +{ > > + struct kvm *kvm = (struct kvm *)s->private; > > + struct vgic_state_iter *iter = (struct vgic_state_iter *)v; > > + struct vgic_irq *irq; > > + struct kvm_vcpu *vcpu = NULL; > > + > > + if (iter->dist_id == 0) { > > + print_dist_state(s, &kvm->arch.vgic); > > + return 0; > > + } > > + > > + if (!kvm->arch.vgic.initialized) > > + return 0; > > + > > + if (iter->vcpu_id < iter->nr_cpus) { > > + vcpu = kvm_get_vcpu(kvm, iter->vcpu_id); > > + irq = &vcpu->arch.vgic_cpu.private_irqs[iter->intid]; > > + } else { > > + irq = &kvm->arch.vgic.spis[iter->intid - VGIC_NR_PRIVATE_IRQS]; > > + } > > + > > + spin_lock(&irq->irq_lock); > > + print_irq_state(s, irq, vcpu); > > + spin_unlock(&irq->irq_lock); > > + > > + return 0; > > +} > > + > > +static struct seq_operations vgic_debug_seq_ops = { > > + .start = vgic_debug_start, > > + .next = vgic_debug_next, > > + .stop = vgic_debug_stop, > > + .show = vgic_debug_show > > +}; > > + > > +static int debug_open(struct inode *inode, struct file *file) > > +{ > > + int ret; > > + ret = seq_open(file, &vgic_debug_seq_ops); > > + if (!ret) { > > + struct seq_file *seq; > > + /* seq_open will have modified file->private_data */ > > + seq = file->private_data; > > + seq->private = inode->i_private; > > + } > > + > > + return ret; > > +}; > > + > > +static struct file_operations vgic_debug_fops = { > > + .owner = THIS_MODULE, > > + .open = debug_open, > > + .read = seq_read, > > + .llseek = seq_lseek, > > + .release = seq_release > > +}; > > + > > +int vgic_debug_init(struct kvm *kvm) > > +{ > > + if (!kvm->debugfs_dentry) > > + return -ENOENT; > > + > > + if (!debugfs_create_file("vgic-state", 0444, > > + kvm->debugfs_dentry, > > + kvm, > > + &vgic_debug_fops)) > > + return -ENOMEM; > > + > > + return 0; > > +} > > + > > +int vgic_debug_destroy(struct kvm *kvm) > > +{ > > + return 0; > > +} > > diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c > > index c737ea0..276139a 100644 > > --- a/virt/kvm/arm/vgic/vgic-init.c > > +++ b/virt/kvm/arm/vgic/vgic-init.c > > @@ -259,6 +259,8 @@ int vgic_init(struct kvm *kvm) > > if (ret) > > goto out; > > > > + vgic_debug_init(kvm); > > + > > dist->initialized = true; > > out: > > return ret; > > @@ -288,6 +290,8 @@ static void __kvm_vgic_destroy(struct kvm *kvm) > > struct kvm_vcpu *vcpu; > > int i; > > > > + vgic_debug_destroy(kvm); > > + > > kvm_vgic_dist_destroy(kvm); > > > > kvm_for_each_vcpu(i, vcpu, kvm) > > diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h > > index b2cf34b..48da1f6 100644 > > --- a/virt/kvm/arm/vgic/vgic.h > > +++ b/virt/kvm/arm/vgic/vgic.h > > @@ -102,4 +102,7 @@ int kvm_register_vgic_device(unsigned long type); > > int vgic_lazy_init(struct kvm *kvm); > > int vgic_init(struct kvm *kvm); > > > > +int vgic_debug_init(struct kvm *kvm); > > +int vgic_debug_destroy(struct kvm *kvm); > > + > > #endif > > > > Besides > > Reviewed-by: Eric Auger > Tested-by: Eric Auger > > Tested on Cavium ThunderX I've addressed your comments with the following changes: Thanks, -Christoffer diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c index ec466a6..19c4566 100644 --- a/virt/kvm/arm/vgic/vgic-debug.c +++ b/virt/kvm/arm/vgic/vgic-debug.c @@ -20,7 +20,6 @@ #include #include #include -#include #include #include #include "vgic.h" @@ -147,8 +146,8 @@ static void print_dist_state(struct seq_file *s, struct vgic_dist *dist) seq_printf(s, "enabled:\t%d\n", dist->enabled); seq_printf(s, "\n"); - seq_printf(s, "P=pending, L=line_level, S=soft_pending, A=active\n"); - seq_printf(s, "E=enabled, H=hw, L=config_level\n"); + seq_printf(s, "P=pending_latch, L=line_level, A=active\n"); + seq_printf(s, "E=enabled, H=hw, C=config (level=1, edge=0)\n"); } static void print_header(struct seq_file *s, struct vgic_irq *irq, @@ -184,10 +183,10 @@ static void print_irq_state(struct seq_file *s, struct vgic_irq *irq, seq_printf(s, " %s %4d " " %2d " "%d%d%d%d%d%d " + "%8d " "%8x " - "%8x " - " %2x " " %2x " + "%3d " " %2d " "\n", type, irq->intid,