From patchwork Tue Jan 24 13:22:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 9535167 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 A69AA6042D for ; Tue, 24 Jan 2017 13:24:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9D2A327F54 for ; Tue, 24 Jan 2017 13:24:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 90D8027F8C; Tue, 24 Jan 2017 13:24:53 +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 61DFB27F54 for ; Tue, 24 Jan 2017 13:24:52 +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 1cW15W-0007Np-Hw; Tue, 24 Jan 2017 13:24:42 +0000 Received: from mail-lf0-f43.google.com ([209.85.215.43]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cW157-0007B5-I5 for linux-arm-kernel@lists.infradead.org; Tue, 24 Jan 2017 13:24:19 +0000 Received: by mail-lf0-f43.google.com with SMTP id n124so110360729lfd.2 for ; Tue, 24 Jan 2017 05:23:57 -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=JzYBsTQK7bpboh8iHzZDNAbxxY4Fs1CJ9srHUDjlPSk=; b=AIX4Aw8RPUexErnlxVxtEYvKHCbLuhU2j6wuDAs37Jnw4iAkgKbxmtej2vdPavmkzD rpf9J79zlAXwiX0fm1X/083wCA9W1iYfZKObjFkYRfLdhuz6h1LA9JWBNZbvxwEI8zVX YJ2rjjE6gwEHeZPgsiPelDiW0Pnq/zgcL1hTU= 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=JzYBsTQK7bpboh8iHzZDNAbxxY4Fs1CJ9srHUDjlPSk=; b=Wo/ewwXio+F9JqzjE+TrWD84TZj3RxLURMvE9AtcXF/RVECyiDd5yCYcrORpXNasIL JwqgeeCcWPxxj0n8YRe8Ft5CGjAEnwuBRc7QUCfR34TWfr3DPpTpIsdXwknqNaQYwIS/ +J0HGkmVJltya0PjqcY20KQr8VzZsc4VOynxzQOKkN7i6LfmqB1qD42o07+UiFb0ocza /+f3/uHrIuzHUo1U9uwJplQiffmmz4HDMseFRJhh2PwFOVaqfDmco8ZN3rVqeyxoJ7Fa e5Vse10yQKn59Scao0z/F4z+a9oYpxzo/jdEn4Am+xXU/iYDGVs02ycvhAAXzlHJWaL6 vcwg== X-Gm-Message-State: AIkVDXJOI50j8S3jZ+YtikocKbqrH1fc6DH4lk9AACwYMTME1WhyqBv90kexB7b9YWEyWQPY X-Received: by 10.25.92.145 with SMTP id u17mr249083lfi.160.1485264175549; Tue, 24 Jan 2017 05:22:55 -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 85sm7336177lfr.37.2017.01.24.05.22.53 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Tue, 24 Jan 2017 05:22:54 -0800 (PST) Date: Tue, 24 Jan 2017 14:22:48 +0100 From: Christoffer Dall To: Andre Przywara Subject: Re: [PATCH] KVM: arm/arm64: vgic: Add debugfs vgic-state file Message-ID: <20170124132144.GQ15850@cbox> References: <20170120103330.13858-1-christoffer.dall@linaro.org> <524751ae-8e17-f9a0-88c6-95d4614478af@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <524751ae-8e17-f9a0-88c6-95d4614478af@arm.com> 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-20170124_052417_801318_1735071E X-CRM114-Status: GOOD ( 25.06 ) 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 , Eric Auger , 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 Tue, Jan 24, 2017 at 10:23:57AM +0000, Andre Przywara wrote: > Hi, > > so I gave this patch (adapted to live without the soft_pending state) > some testing on my machines (Midway, Juno, GICv3 ITS h/w) and it seemed > to work well - at least if one is nice and starts only one process > accessing vgic-state (see below). I caught some IRQs red-handed and the > output looked plausible. > The only comment I have is that "MPIDR" is a misleading header name for > that column. It's actually a union with the GICv2 targets field, which > is a bitmask of the targetting VCPUs. So the output looks more like a > bitmask and not an MPIDR on many machines. But that's just cosmetic. > > Just discovered one thing: As soon as a second task is reading the file > (with "while [ 1 ]; do cat vgic-state > /dev/null; done &" in the > background, for instance), I get this splat on the host: > > [60796.007461] Unable to handle kernel NULL pointer dereference at > virtual address 00000008 > [60796.015538] pgd = ffff800974e30000 > [60796.018952] [00000008] *pgd=00000009f4d0a003[60796.023067] Internal > error: Oops: 96000006 [#1] PREEMPT SMP > [60796.028588] Modules linked in: > [60796.031626] CPU: 3 PID: 5690 Comm: cat Tainted: G W > 4.9.0-00026-ge24503e-dirty #444 > [60796.040326] Hardware name: ARM Juno development board (r0) (DT) > [60796.046190] task: ffff80097231ab00 task.stack: ffff800973894000 > [60796.052059] PC is at iter_next+0x18/0x80 > [60796.055946] LR is at debug_next+0x38/0x90 > [60796.059917] pc : [] lr : [] Turns out it wasn't a difficult fix (patch includes rebase on pending change and making the naming slightly less cute). The bugfix is in vgic_debug_stop which now checks if the supplied pointer is an error pointer. I'll send out a v2. Thanks, -Christoffer diff --git a/virt/kvm/arm/vgic/vgic-debug.c b/virt/kvm/arm/vgic/vgic-debug.c index 76e8b11..ec466a6 100644 --- a/virt/kvm/arm/vgic/vgic-debug.c +++ b/virt/kvm/arm/vgic/vgic-debug.c @@ -70,14 +70,14 @@ static void iter_init(struct kvm *kvm, struct vgic_state_iter *iter, iter_next(iter); } -static bool the_end(struct vgic_state_iter *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 *debug_start(struct seq_file *s, loff_t *pos) +static void *vgic_debug_start(struct seq_file *s, loff_t *pos) { struct kvm *kvm = (struct kvm *)s->private; struct vgic_state_iter *iter; @@ -98,30 +98,37 @@ static void *debug_start(struct seq_file *s, loff_t *pos) iter_init(kvm, iter, *pos); kvm->arch.vgic.iter = iter; - if (the_end(iter)) + if (end_of_vgic(iter)) iter = NULL; out: mutex_unlock(&kvm->lock); return iter; } -static void *debug_next(struct seq_file *s, void *v, loff_t *pos) +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 (the_end(iter)) + if (end_of_vgic(iter)) iter = NULL; return iter; } -static void debug_stop(struct seq_file *s, void *v) +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); @@ -156,8 +163,8 @@ static void print_header(struct seq_file *s, struct vgic_irq *irq, } seq_printf(s, "\n"); - seq_printf(s, "%s%2d TYP ID TGT_ID PLSAEHL HWID MPIDR SRC PRI VCPU_ID\n", hdr, 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, @@ -176,7 +183,7 @@ 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%d " + "%d%d%d%d%d%d " "%8x " "%8x " " %2x " @@ -185,9 +192,8 @@ static void print_irq_state(struct seq_file *s, struct vgic_irq *irq, "\n", type, irq->intid, (irq->target_vcpu) ? irq->target_vcpu->vcpu_id : -1, - irq->pending, + irq->pending_latch, irq->line_level, - irq->soft_pending, irq->active, irq->enabled, irq->hw, @@ -200,7 +206,7 @@ static void print_irq_state(struct seq_file *s, struct vgic_irq *irq, } -static int debug_show(struct seq_file *s, void *v) +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; @@ -229,17 +235,17 @@ static int debug_show(struct seq_file *s, void *v) return 0; } -static struct seq_operations debug_seq_ops = { - .start = debug_start, - .next = debug_next, - .stop = debug_stop, - .show = debug_show +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, &debug_seq_ops); + ret = seq_open(file, &vgic_debug_seq_ops); if (!ret) { struct seq_file *seq; /* seq_open will have modified file->private_data */