Quoting Gabriel Krisman Bertazi (2017-07-26 06:30:16) > Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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 <krisman@xxxxxxxxxxxxxxx> Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
