[Intel-gfx] [PATCH] drm/i915: Handle msr read failure gracefully

[Chris Wilson]

Quoting Gabriel Krisman Bertazi (2017-07-25 19:19:22)
> 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 at 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
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101901
> Signed-off-by: Gabriel Krisman Bertazi <krisman at collabora.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c25f42c60d61..770b2846fed9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2791,7 +2791,11 @@ static int i915_energy_uJ(struct seq_file *m, void *data)
>  
>         intel_runtime_pm_get(dev_priv);
>  
> -       rdmsrl(MSR_RAPL_POWER_UNIT, power);
> +       if (rdmsrl_safe(MSR_RAPL_POWER_UNIT, &power)) {
> +               intel_runtime_pm_put(dev_priv);
> +               return -ENODEV;
> +       }
> +
>         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 */
-Chris