Message ID | 20240219185809.286724-4-oliver.upton@linux.dev |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-71849-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2685:b0:108:e6aa:91d0 with SMTP id mn5csp1480436dyc; Mon, 19 Feb 2024 11:07:17 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCV4Zsv7WXYZXDn2y5tqf+16iMSJsQmuW3/drN16rMYYTS98FlTbXjVViD/WpeDIEatwaO/jv3Rt37/gjgVA+51lSbIR8w== X-Google-Smtp-Source: AGHT+IG4xTMQwNqJTjRZsPy4EkxqiYw1Lu7e/Mk/Nj8YBm/UrZmmWI+sSeIBKbyjqNRUPXkicQI2 X-Received: by 2002:a17:902:820c:b0:1db:7052:2f62 with SMTP id x12-20020a170902820c00b001db70522f62mr11190348pln.50.1708369637204; Mon, 19 Feb 2024 11:07:17 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708369637; cv=pass; d=google.com; s=arc-20160816; b=ejAYWZ+hMbPC3QAvdYRpIHAZvuIDT6OIE8/0Z3e4/wQMoDRbjNdkMrhlBiumWB7P4a FyI4JQqC36O4bWf1f3ms+Ow10JQdXw8k5yzo6xrFjWn4egdSTeiVNm8pHTXKoiA+H+x4 0+kRwtbSL8nQCa1oZEWFVttnrnE7W9MxGgo54m4txFCCzc2f6LETXYmYCuRj8wS0Vi1d GlzMTf1zlXheAxP1YdRaHhKj/WzUhTT75GvJTDdA135Z9L5a/aAi85JzCVwkzBmF6aF+ +g/bcbBBZ54bl/5Fn1WgXTjUZk1BzczU1mpGE9iu64fekG5a2vLdXlmxzy4fNSlP4K/8 dZ9g== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:references:in-reply-to:message-id :date:subject:cc:to:from:dkim-signature; bh=O6mYbl34iGDajSSuwNwYK5N248BsKIXOEJAXAf9uLUE=; fh=fgV4Kp0ZgGkINw9PbGnSyYl5n9Uaijr/GJFF1V5tx/4=; b=oQ97gvKJvLddnRhZw6l1E9sNtZWiBnwEqM1dghK5TbS36szZb8DUwigcJPmsZ4wNOW NtZUje9hof2TtH8XHouoxxpZxiRNODmAJb/gDGD0zwCydM2zWPlbcZiH9PWVAMU6nva7 pgzqbnVWuqbQcyLO6dILeTjMJrkhbNeq8QIpAiYrfDpuXW2ZomBOiALTY2SixF3CHprq UvvtBRGl5xne5ejnG+SwNWIJ4I6aiptEGGKXJnmL2mdmHDxvKrZZVQ4yYQF5tUWQwYyr xKmAjn89IQwe+q6gTOTPEx+L4QEIwdWGeOAOJF60ee967l4q1XXaoSu17kdzFD5KEtoQ JL2A==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=erdwcROx; arc=pass (i=1 spf=pass spfdomain=linux.dev dkim=pass dkdomain=linux.dev dmarc=pass fromdomain=linux.dev); spf=pass (google.com: domain of linux-kernel+bounces-71849-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-71849-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: from sy.mirrors.kernel.org (sy.mirrors.kernel.org. [2604:1380:40f1:3f00::1]) by mx.google.com with ESMTPS id lv11-20020a1709032a8b00b001da1d2ad541si4953923plb.572.2024.02.19.11.07.16 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Feb 2024 11:07:17 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-71849-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1; Authentication-Results: mx.google.com; dkim=pass header.i=@linux.dev header.s=key1 header.b=erdwcROx; arc=pass (i=1 spf=pass spfdomain=linux.dev dkim=pass dkdomain=linux.dev dmarc=pass fromdomain=linux.dev); spf=pass (google.com: domain of linux-kernel+bounces-71849-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-71849-ouuuleilei=gmail.com@vger.kernel.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linux.dev Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sy.mirrors.kernel.org (Postfix) with ESMTPS id 636A7B21FF1 for <ouuuleilei@gmail.com>; Mon, 19 Feb 2024 18:59:42 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 7B6E64F60E; Mon, 19 Feb 2024 18:58:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="erdwcROx" Received: from out-176.mta0.migadu.com (out-176.mta0.migadu.com [91.218.175.176]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4C878487A3 for <linux-kernel@vger.kernel.org>; Mon, 19 Feb 2024 18:58:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.176 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708369111; cv=none; b=HKqRKFi4IOuQVELcYHEoNJpne7FRovAFyhU6SmgYicZ04pFNPRAl4iGBADCweW48c5ZYfcmtGDZMGgU+fUJJBpYh/35TdHHO+BenfFq66v6RLCkpR89Dl7N6vKckt/N6NzI28qELWsBKvMcQLwHB2/g5UU31apiJecUYa8pBbRA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708369111; c=relaxed/simple; bh=AVsdCCM8FvfXsv0NexSqvyqeP7RkJJs+vf3bnHoUDbA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=AEQqmj4F2Xja86x2jUzuRUZuLloyU+bBvIKgZUjQUP5MmkqFAtBI6Tnx7gVDP5i1RdnF7Lv8ZqEXV9wL+KUVqve3pDHLAZ4V9IchL+05SGssmolAJVchs0IYuAWTBHDkLZWTCqQf4gyJbns2rvh5Gl6VgLj5PhWKmuAg55cMnPc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=erdwcROx; arc=none smtp.client-ip=91.218.175.176 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1708369107; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=O6mYbl34iGDajSSuwNwYK5N248BsKIXOEJAXAf9uLUE=; b=erdwcROxjTVj9GYsF9NxwZtHNpUShYCx45EDDOMeS+jAJihxWdrPa0U1lhk/4/FnuC7+2g 6JUYy8lf8giYc6dFvutLvzVCiE2tKOwcceN27OylehFuPrm9rlUze2Uoy930paO+8vWiFX OVADPtybepCZRTctZIlkmbbnW0WSTKY= From: Oliver Upton <oliver.upton@linux.dev> To: Thomas Gleixner <tglx@linutronix.de> Cc: Marc Zyngier <maz@kernel.org>, Zenghui Yu <yuzenghui@huawei.com>, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, kvmarm@lists.linux.dev, Jing Zhang <jingzhangos@google.com>, Oliver Upton <oliver.upton@linux.dev> Subject: [PATCH 3/3] irqchip/gic-v3-its: Print the vPE table installed in redistributor Date: Mon, 19 Feb 2024 18:58:08 +0000 Message-ID: <20240219185809.286724-4-oliver.upton@linux.dev> In-Reply-To: <20240219185809.286724-1-oliver.upton@linux.dev> References: <20240219185809.286724-1-oliver.upton@linux.dev> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Migadu-Flow: FLOW_OUT X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791355400856288166 X-GMAIL-MSGID: 1791355400856288166 |
Series |
irqchip/gic-v3-its: Fix GICv4.1 initialization after kexec
|
|
Commit Message
Oliver Upton
Feb. 19, 2024, 6:58 p.m. UTC
Hindsight is 20/20 of course, but the recent vPE table programming bug
could've been root caused a bit more quickly if we print the table
getting installed at every redistributor.
Promote to pr_info() and add some additional context, such as the
provenance of the installed vPE table.
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
drivers/irqchip/irq-gic-v3-its.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
Comments
On Mon, 19 Feb 2024 18:58:08 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > Hindsight is 20/20 of course, but the recent vPE table programming bug > could've been root caused a bit more quickly if we print the table > getting installed at every redistributor. > > Promote to pr_info() and add some additional context, such as the > provenance of the installed vPE table. > > Signed-off-by: Oliver Upton <oliver.upton@linux.dev> > --- > drivers/irqchip/irq-gic-v3-its.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c > index 63d1743f08cc..c3ef9665a2ad 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -2835,7 +2835,8 @@ static int allocate_vpe_l1_table(void) > u64 val, gpsz, npg, pa; > unsigned int psz = SZ_64K; > unsigned int np, epp, esz; > - struct page *page; > + struct page *page = NULL; > + bool from_its = false; > > if (!gic_rdists->has_rvpeid) > return 0; > @@ -2865,8 +2866,10 @@ static int allocate_vpe_l1_table(void) > return -ENOMEM; > > val = inherit_vpe_l1_table_from_its(); > - if (val & GICR_VPROPBASER_4_1_VALID) > + if (val & GICR_VPROPBASER_4_1_VALID) { > + from_its = true; > goto out; > + } nit: from_its = val & GICR_VPROPBASER_4_1_VALID; if (from_its) ... > > /* First probe the page size */ > val = FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE, GIC_PAGE_SIZE_64K); > @@ -2945,9 +2948,12 @@ static int allocate_vpe_l1_table(void) > gicr_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER); > cpumask_set_cpu(smp_processor_id(), gic_data_rdist()->vpe_table_mask); > > - pr_debug("CPU%d: VPROPBASER = %llx %*pbl\n", > - smp_processor_id(), val, > - cpumask_pr_args(gic_data_rdist()->vpe_table_mask)); > + pr_info("CPU%d: Using %s vPE table @%llx (%s)\n", > + smp_processor_id(), > + (val & GICR_VPROPBASER_4_1_INDIRECT) ? "indirect" : "direct", > + val & GICR_VPROPBASER_4_1_ADDR, > + (page) ? "allocated" : > + ((from_its) ? "inherited from ITS" : "inherited from RD")); From past experience, having the vpe_table_mask value displayed did help tracking VPE table affinity bugs. This said, my problem with this patch is that we already have tons of these statement printed once per CPU/RD. This is really huge and accounts for a significant part of the boot time on large machines (64+ CPUs). Before we add more of those, I'd really want to have a way to tone them down and only print them at runtime *if* required by the user. Kind of a dymanic debug, but driven from the command-line and present early enough. What do you think? M.
On Sat, Feb 24, 2024 at 10:41:36AM +0000, Marc Zyngier wrote: > On Mon, 19 Feb 2024 18:58:08 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > - pr_debug("CPU%d: VPROPBASER = %llx %*pbl\n", > > - smp_processor_id(), val, > > - cpumask_pr_args(gic_data_rdist()->vpe_table_mask)); > > + pr_info("CPU%d: Using %s vPE table @%llx (%s)\n", > > + smp_processor_id(), > > + (val & GICR_VPROPBASER_4_1_INDIRECT) ? "indirect" : "direct", > > + val & GICR_VPROPBASER_4_1_ADDR, > > + (page) ? "allocated" : > > + ((from_its) ? "inherited from ITS" : "inherited from RD")); > > From past experience, having the vpe_table_mask value displayed did > help tracking VPE table affinity bugs. My reasoning behind it was that the change expanded the table mask by way of printing what's going on at every RD. But easy enough to throw back in! > This said, my problem with this patch is that we already have tons of > these statement printed once per CPU/RD. This is really huge and > accounts for a significant part of the boot time on large machines > (64+ CPUs). > > Before we add more of those, I'd really want to have a way to tone > them down and only print them at runtime *if* required by the user. > Kind of a dymanic debug, but driven from the command-line and present > early enough. Yeah, what'd be really nice is a way to enable pr_debug() on a per file / driver / whatever basis, since turning on all of it becomes a bit of a firehose... But I guess that's what grep is for. WDYT about leaving it at pr_debug() for now, with the additional context of what exactly VPROPBASE is getting programmend with?
On Sat, 24 Feb 2024 11:11:41 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > On Sat, Feb 24, 2024 at 10:41:36AM +0000, Marc Zyngier wrote: > > On Mon, 19 Feb 2024 18:58:08 +0000, Oliver Upton <oliver.upton@linux.dev> wrote: > > > - pr_debug("CPU%d: VPROPBASER = %llx %*pbl\n", > > > - smp_processor_id(), val, > > > - cpumask_pr_args(gic_data_rdist()->vpe_table_mask)); > > > + pr_info("CPU%d: Using %s vPE table @%llx (%s)\n", > > > + smp_processor_id(), > > > + (val & GICR_VPROPBASER_4_1_INDIRECT) ? "indirect" : "direct", > > > + val & GICR_VPROPBASER_4_1_ADDR, > > > + (page) ? "allocated" : > > > + ((from_its) ? "inherited from ITS" : "inherited from RD")); > > > > From past experience, having the vpe_table_mask value displayed did > > help tracking VPE table affinity bugs. > > My reasoning behind it was that the change expanded the table mask by > way of printing what's going on at every RD. But easy enough to throw > back in! Yeah, true enough. But the mask presents a nice, concise way to express the affinities that comparing a bunch of lines doesn't exactly convey. > > > This said, my problem with this patch is that we already have tons of > > these statement printed once per CPU/RD. This is really huge and > > accounts for a significant part of the boot time on large machines > > (64+ CPUs). > > > > Before we add more of those, I'd really want to have a way to tone > > them down and only print them at runtime *if* required by the user. > > Kind of a dymanic debug, but driven from the command-line and present > > early enough. > > Yeah, what'd be really nice is a way to enable pr_debug() on a per > file / driver / whatever basis, since turning on all of it becomes a bit > of a firehose... But I guess that's what grep is for. > > WDYT about leaving it at pr_debug() for now, with the additional context > of what exactly VPROPBASE is getting programmend with? Yup, I'm all for that. Hopefully we can find a good way to way to control the debug output in the near future. Thanks, M.
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c index 63d1743f08cc..c3ef9665a2ad 100644 --- a/drivers/irqchip/irq-gic-v3-its.c +++ b/drivers/irqchip/irq-gic-v3-its.c @@ -2835,7 +2835,8 @@ static int allocate_vpe_l1_table(void) u64 val, gpsz, npg, pa; unsigned int psz = SZ_64K; unsigned int np, epp, esz; - struct page *page; + struct page *page = NULL; + bool from_its = false; if (!gic_rdists->has_rvpeid) return 0; @@ -2865,8 +2866,10 @@ static int allocate_vpe_l1_table(void) return -ENOMEM; val = inherit_vpe_l1_table_from_its(); - if (val & GICR_VPROPBASER_4_1_VALID) + if (val & GICR_VPROPBASER_4_1_VALID) { + from_its = true; goto out; + } /* First probe the page size */ val = FIELD_PREP(GICR_VPROPBASER_4_1_PAGE_SIZE, GIC_PAGE_SIZE_64K); @@ -2945,9 +2948,12 @@ static int allocate_vpe_l1_table(void) gicr_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER); cpumask_set_cpu(smp_processor_id(), gic_data_rdist()->vpe_table_mask); - pr_debug("CPU%d: VPROPBASER = %llx %*pbl\n", - smp_processor_id(), val, - cpumask_pr_args(gic_data_rdist()->vpe_table_mask)); + pr_info("CPU%d: Using %s vPE table @%llx (%s)\n", + smp_processor_id(), + (val & GICR_VPROPBASER_4_1_INDIRECT) ? "indirect" : "direct", + val & GICR_VPROPBASER_4_1_ADDR, + (page) ? "allocated" : + ((from_its) ? "inherited from ITS" : "inherited from RD")); return 0; }