On 02/14/2017 05:41 PM, Tom St Denis wrote: > On 14/02/17 10:08 AM, Samuel Pitoiset wrote: >> Totally untested but as long as read_sensor() has been recently >> implemented for dpm based boards, amdgpu_sensors can now be >> exposed. >> >> v2: - make sure read_sensor is not NULL on dpm chips >> - keep sanity check for powerplay chips >> >> Cc: Tom St Denis <tom.stdenis at amd.com> >> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> index 6f021e70f15f..80821b436aeb 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c >> @@ -3204,6 +3204,9 @@ static ssize_t amdgpu_debugfs_sensor_read(struct >> file *f, char __user *buf, >> valuesize = sizeof(values); >> if (adev->powerplay.pp_funcs && >> adev->powerplay.pp_funcs->read_sensor) >> r = >> adev->powerplay.pp_funcs->read_sensor(adev->powerplay.pp_handle, idx, >> &values[0], &valuesize); >> + else if (adev->pm.funcs && adev->pm.funcs->read_sensor) >> + r = adev->pm.funcs->read_sensor(adev, idx, &values[0], >> + &valuesize); >> else >> return -EINVAL; >> >> > > Sorry NAK again, even with dpm=0 those function pointers are set and you > end up inside the dpm code trying to parse data structures that aren't > initialized. No worries. I thought that check was enough. Anyway, writing code without the hardware should be avoided. :) Can you try the thing suggested by Alex? Because I will need to fix up the DRM ioctl codepath as well. > > For instance, this oops on my Kaveri when trying to read SCLK. > > [ 729.616382] BUG: unable to handle kernel NULL pointer dereference at > 00000000000000c4 > [ 729.616467] IP: [<ffffffffc0e62d40>] kv_dpm_read_sensor+0xa0/0xd0 > [amdgpu] > [ 729.616619] PGD 232c58067 > [ 729.616644] PUD 232c4f067 > [ 729.616670] PMD 0 > > [ 729.616697] Oops: 0000 [#1] SMP > [ 729.616726] Modules linked in: fuse amdkfd amd_iommu_v2 amdgpu > i2c_algo_bit ttm drm_kms_helper drm xt_CHECKSUM iptable_mangle > ipt_MASQUERADE nf_nat_masquerade_ipv4 iptable_nat nf_nat_ipv4 nf_nat > nf_conntrack_ipv4 nf_defrag_ipv4 xt_conntrack nf_conntrack tun bridge > ebtable_filter ebtables ip6table_filter ip6_tables rpcsec_gss_krb5 nfsv4 > dns_resolver nfs fscache bnep vfat fat edac_mce_amd arc4 edac_core > kvm_amd iwlmvm snd_hda_codec_realtek mac80211 kvm snd_hda_codec_generic > snd_hda_codec_hdmi snd_hda_intel snd_hda_codec snd_hda_core btusb btrtl > btbcm btintel iwlwifi bluetooth snd_hwdep cfg80211 snd_seq > snd_seq_device snd_pcm irqbypass joydev snd_timer snd crct10dif_pclmul > crc32_pclmul pcspkr ghash_clmulni_intel rfkill fam15h_power sp5100_tco > k10temp soundcore acpi_cpufreq tpm_tis i2c_piix4 tpm_tis_core > [ 729.617499] shpchp video tpm nfsd auth_rpcgss nfs_acl lockd grace > sunrpc ata_generic pata_acpi 8021q crc32c_intel garp stp llc serio_raw > mrp pata_atiixp r8169 mii fjes > [ 729.617672] CPU: 0 PID: 1417 Comm: umr Not tainted 4.9.0+ #4 > [ 729.617720] Hardware name: Gigabyte Technology Co., Ltd. To be filled > by O.E.M./F2A88XN-WIFI, BIOS F5 04/22/2015 > [ 729.617802] task: ffff8e43ef37d880 task.stack: ffff9d7501594000 > [ 729.617852] RIP: 0010:[<ffffffffc0e62d40>] [<ffffffffc0e62d40>] > kv_dpm_read_sensor+0xa0/0xd0 [amdgpu] > [ 729.618000] RSP: 0018:ffff9d7501597d60 EFLAGS: 00010246 > [ 729.618046] RAX: 0000000000000000 RBX: ffff9d7501597d98 RCX: > 0000000000000000 > [ 729.618104] RDX: 0000000000000000 RSI: 0000000000000286 RDI: > 0000000000000286 > [ 729.618162] RBP: ffff9d7501597d80 R08: 0000000000000000 R09: > 0000000000000000 > [ 729.618221] R10: 0000000000000000 R11: ffff8e43ef37d880 R12: > ffff9d7501597d98 > [ 729.618285] R13: 0000000000000000 R14: ffff9d7501597d94 R15: > 0000000000000000 > [ 729.618344] FS: 00007f6525bfd700(0000) GS:ffff8e43fec00000(0000) > knlGS:0000000000000000 > [ 729.618411] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 729.618458] CR2: 00000000000000c4 CR3: 00000002261f6000 CR4: > 00000000000406f0 > [ 729.618515] Stack: > [ 729.618535] 0000000000000004 00007ffe1e85ca00 ffff8e43f3162100 > 0000000000000004 > [ 729.618607] ffff9d7501597df8 ffffffffc0e2c637 000000401e85ca00 > ffff9d7501597f18 > [ 729.618676] ffff9d7501597df8 ffffffffc0e2dc7d 0000000000005403 > 0000000000000000 > [ 729.618747] Call Trace: > [ 729.618826] [<ffffffffc0e2c637>] > amdgpu_debugfs_sensor_read+0xf7/0x120 [amdgpu] > [ 729.618938] [<ffffffffc0e2dc7d>] ? > amdgpu_debugfs_regs_read+0x17d/0x220 [amdgpu] > [ 729.619002] [<ffffffff8f3433e4>] full_proxy_read+0x54/0x90 > [ 729.619049] [<ffffffff8f24be67>] __vfs_read+0x37/0x150 > [ 729.619095] [<ffffffff8f35e67b>] ? security_file_permission+0x9b/0xc0 > [ 729.621382] [<ffffffff8f24cfe6>] vfs_read+0x96/0x130 > [ 729.622997] [<ffffffff8f24e4d5>] SyS_read+0x55/0xc0 > [ 729.624540] [<ffffffff8f8010b7>] entry_SYSCALL_64_fastpath+0x1a/0xa9 > [ 729.626139] Code: 24 31 c0 41 c7 06 04 00 00 00 41 5c 41 5d 41 5e 41 > 5f 5d c3 c1 e8 10 83 e0 1f 83 f8 07 77 ad 48 8d 14 c5 00 00 00 00 5b 48 > 29 c2 <41> 8b 84 95 c4 00 00 00 0f c8 41 89 04 24 41 c7 06 04 00 00 00 >
