From patchwork Wed Jul 26 05:30:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gabriel Krisman Bertazi X-Patchwork-Id: 9864177 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 CB818602B1 for ; Wed, 26 Jul 2017 05:31:12 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C398928615 for ; Wed, 26 Jul 2017 05:31:12 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B83A428732; Wed, 26 Jul 2017 05:31:12 +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=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id B6E5B28615 for ; Wed, 26 Jul 2017 05:31:11 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 6312B6E168; Wed, 26 Jul 2017 05:30:25 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2E56B6E168 for ; Wed, 26 Jul 2017 05:30:23 +0000 (UTC) Received: from localhost (unknown [IPv6:2a00:5f00:102:0:580d:efff:fe4d:1bd7]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: krisman) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id D70E826B398; Wed, 26 Jul 2017 06:30:21 +0100 (BST) From: Gabriel Krisman Bertazi To: Chris Wilson Organization: Collabora References: <20170725181922.22673-1-krisman@collabora.co.uk> <150100924857.4680.10755970484346903926@mail.alporthouse.com> Date: Wed, 26 Jul 2017 02:30:16 -0300 In-Reply-To: <150100924857.4680.10755970484346903926@mail.alporthouse.com> (Chris Wilson's message of "Tue, 25 Jul 2017 20:00:48 +0100") Message-ID: <87o9s7zrx3.fsf@dilma.collabora.co.uk> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) MIME-Version: 1.0 Cc: intel-gfx@lists.freedesktop.org Subject: Re: [Intel-gfx] [PATCH] drm/i915: Handle msr read failure gracefully X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Chris Wilson writes: > Quoting Gabriel Krisman Bertazi (2017-07-25 19:19:22) >> power = (power & 0x1f00) >> 8; >> units = 1000000 / (1 << power); /* convert to uJ */ >> power = I915_READ(MCH_SECP_NRG_STTS); > > Just after this is a useless cast. Though it will be neater to kill the > (long long unsigned) and s/u64/unsigned long long/ so that we are > consistent with the rdmsrl_safe interface. > > Also we should use 1u << power as we allow power to be 31, or better yet > use: > > units = (power & 0x1f00) >> 8; > power = I915_READ(MCH_SECP_NRG_STTS); > power = (100000 * power) >> units; /* convert to uJ */ Hi Chris, Thanks for your review. I have added your suggestions on a v2 of the patch below. >8 Subject: [PATCH] drm/i915: Handle msr read failure gracefully When reading the i915_energy_uJ debugfs file, it tries to fetch MSR_RAPL_POWER_UNIT, which might not be available, like in a vm environment, causing the exception shown below. We can easily prevent it by doing a rdmsrl_safe read instead, which will handle the exception, allowing us to abort the debugfs file read. This was caught by the new igt@debugfs_test@read_all_entries testcase in the CI. unchecked MSR access error: RDMSR from 0x606 at rIP:0xffffffffa0078f66 (i915_energy_uJ+0x36/0xb0 [i915]) Call Trace: seq_read+0xdc/0x3a0 full_proxy_read+0x4f/0x70 __vfs_read+0x23/0x120 ? putname+0x4f/0x60 ? rcu_read_lock_sched_held+0x75/0x80 ? entry_SYSCALL_64_fastpath+0x5/0xb1 vfs_read+0xa0/0x150 SyS_read+0x44/0xb0 entry_SYSCALL_64_fastpath+0x1c/0xb1 RIP: 0033:0x7f1f5e9f4500 RSP: 002b:00007ffc77e65cf8 EFLAGS: 00000246 ORIG_RAX: 0000000000000000 RAX: ffffffffffffffda RBX: ffffffff8146e003 RCX: 00007f1f5e9f4500 RDX: 0000000000000200 RSI: 00007ffc77e65d10 RDI: 0000000000000006 RBP: ffffc900007abf88 R08: 0000000001eaff20 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000 R13: 0000000000000006 R14: 0000000000000005 R15: 0000000001eb94db ? __this_cpu_preempt_check+0x13/0x20 v2: - Drop unsigned long long cast and improve calculation (Chris) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101901 Signed-off-by: Gabriel Krisman Bertazi Reviewed-by: Chris Wilson --- drivers/gpu/drm/i915/i915_debugfs.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index c25f42c60d61..1dba3a2be849 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -2783,7 +2783,7 @@ static int i915_sink_crc(struct seq_file *m, void *data) static int i915_energy_uJ(struct seq_file *m, void *data) { struct drm_i915_private *dev_priv = node_to_i915(m->private); - u64 power; + unsigned long long power; u32 units; if (INTEL_GEN(dev_priv) < 6) @@ -2791,15 +2791,18 @@ static int i915_energy_uJ(struct seq_file *m, void *data) intel_runtime_pm_get(dev_priv); - rdmsrl(MSR_RAPL_POWER_UNIT, power); - power = (power & 0x1f00) >> 8; - units = 1000000 / (1 << power); /* convert to uJ */ + if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power)) { + intel_runtime_pm_put(dev_priv); + return -ENODEV; + } + + units = (power & 0x1f00) >> 8; power = I915_READ(MCH_SECP_NRG_STTS); - power *= units; + power = (1000000 * power) >> units; /* convert to uJ */ intel_runtime_pm_put(dev_priv); - seq_printf(m, "%llu", (long long unsigned)power); + seq_printf(m, "%llu", power); return 0; }