PM related changes #249

ranj063 · 2018-11-07T03:32:47Z

This pull request includes these changes:

Enable clock gating and power gating for the audio dsp after fw boot for SKL+ platforms
reset HDA controller to enable PGD1 power gating
make pm_runtime_get/put calls symmetric in pci device probe/remove

sound/soc/sof/pcm.c

sound/soc/sof/utils.c

plbossart

Looks mostly good but multiple points are unclear or can be improved.

sound/soc/sof/sof-pci-dev.c

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/intel/hda-ctrl.c

sound/soc/sof/intel/hda-loader.c

sound/soc/sof/intel/hsw.c

sound/soc/sof/loader.c

plbossart · 2018-11-07T20:10:37Z

@ranj063 let me know when you are a new version, I would like to test on top of some of my cleanups for module load/unload (where I still have the PM error message).

ranj063 · 2018-11-08T07:02:14Z

@plbossart @lgirdwood i've updated the PR based on previous comments now.

keyonjie

in general, looks we are lack of taking controller out of reset at resume?

sound/soc/sof/loader.c

sound/soc/sof/intel/hda-ctrl.c

sound/soc/sof/intel/hda-dsp.c

plbossart

We are making progress, thanks for this update!
Couple of fixes requited, e.g. function renaming needed for clarity, some comments desired and need to address Keyon's comment. Let's try and close this week.

sound/soc/sof/sof-pci-dev.c

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/loader.c

sound/soc/sof/intel/hda-ctrl.c

ranj063 · 2018-11-08T17:57:11Z

@plbossart @keyonjie @lgirdwood all comments addressed now.

plbossart · 2018-11-09T00:10:11Z

@ranj063 the code looks good to me but there was a conflict introduced with an earlier merge of Xiuli's work. Can you fix and resubmit? Thanks!

The patch does the following: 1. Move the runtime_put call for the pci dev to its probe routine 2. Use pm_runtime_put_noidle and pm_runtime_get_noresume in pci probe/remove Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

…r gating Add CGCTL/PGCTL bits that will be used for enabling/disabling clock gating and power gating respectively. Also, LP SRAM retention mode is enabled/disabled by BIT(4) of PGCTL register. So, fix the error in its name from PCI_CGCTL_LSRMD_MASK to PCI_PGCTL_LSRMD_MASK. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2018-11-09T02:06:26Z

@plbossart fixed the conflict now!

plbossart

couple of typos and doubts I have on the PM sequences that are worthy of comments as well.

sound/soc/sof/intel/hda-dsp.c

sound/soc/sof/sof-priv.h

sound/soc/sof/intel/skl.c

sound/soc/sof/core.c

sound/soc/sof/pcm.c

Add pre/post ops that will be called to perform actions before and after fw run routine. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This patch defines pre/post fw run ops for SKL+ platforms. Disable clock gating, power gating and L1 support in pre_fw_run. Re-enable these in post_fw_run. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This patch does the following: 1. Reset HDA controller during suspend so that the PGD1 can be power gated. 2.Take controller out of reset during resume 3. This patch modifies the hda_dsp_ctrl_link_reset() method so it can be called to reset the controller during suspend. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Runtime PM for sof device in enabled in pcm_probe() after the topology load has completed. So autosuspend should be called after pm_runtime_enable() here. Remove the call to autosuspend() in sof_probe as this should be done after topology load has been completed. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 · 2018-11-09T17:42:01Z

@plbossart fixed the typo and refs to fw_load now.

plbossart

ok let's merge and start a validation cycle.

Finish RISC-V support

I got a UAF report when doing fuzz test: [ 152.880091][ T8030] ================================================================== [ 152.881240][ T8030] BUG: KASAN: use-after-free in pwq_unbound_release_workfn+0x50/0x190 [ 152.882442][ T8030] Read of size 4 at addr ffff88810d31bd00 by task kworker/3:2/8030 [ 152.883578][ T8030] [ 152.883932][ T8030] CPU: 3 PID: 8030 Comm: kworker/3:2 Not tainted 5.13.0+ #249 [ 152.885014][ T8030] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1 04/01/2014 [ 152.886442][ T8030] Workqueue: events pwq_unbound_release_workfn [ 152.887358][ T8030] Call Trace: [ 152.887837][ T8030] dump_stack_lvl+0x75/0x9b [ 152.888525][ T8030] ? pwq_unbound_release_workfn+0x50/0x190 [ 152.889371][ T8030] print_address_description.constprop.10+0x48/0x70 [ 152.890326][ T8030] ? pwq_unbound_release_workfn+0x50/0x190 [ 152.891163][ T8030] ? pwq_unbound_release_workfn+0x50/0x190 [ 152.891999][ T8030] kasan_report.cold.15+0x82/0xdb [ 152.892740][ T8030] ? pwq_unbound_release_workfn+0x50/0x190 [ 152.893594][ T8030] __asan_load4+0x69/0x90 [ 152.894243][ T8030] pwq_unbound_release_workfn+0x50/0x190 [ 152.895057][ T8030] process_one_work+0x47b/0x890 [ 152.895778][ T8030] worker_thread+0x5c/0x790 [ 152.896439][ T8030] ? process_one_work+0x890/0x890 [ 152.897163][ T8030] kthread+0x223/0x250 [ 152.897747][ T8030] ? set_kthread_struct+0xb0/0xb0 [ 152.898471][ T8030] ret_from_fork+0x1f/0x30 [ 152.899114][ T8030] [ 152.899446][ T8030] Allocated by task 8884: [ 152.900084][ T8030] kasan_save_stack+0x21/0x50 [ 152.900769][ T8030] __kasan_kmalloc+0x88/0xb0 [ 152.901416][ T8030] __kmalloc+0x29c/0x460 [ 152.902014][ T8030] alloc_workqueue+0x111/0x8e0 [ 152.902690][ T8030] __btrfs_alloc_workqueue+0x11e/0x2a0 [ 152.903459][ T8030] btrfs_alloc_workqueue+0x6d/0x1d0 [ 152.904198][ T8030] scrub_workers_get+0x1e8/0x490 [ 152.904929][ T8030] btrfs_scrub_dev+0x1b9/0x9c0 [ 152.905599][ T8030] btrfs_ioctl+0x122c/0x4e50 [ 152.906247][ T8030] __x64_sys_ioctl+0x137/0x190 [ 152.906916][ T8030] do_syscall_64+0x34/0xb0 [ 152.907535][ T8030] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 152.908365][ T8030] [ 152.908688][ T8030] Freed by task 8884: [ 152.909243][ T8030] kasan_save_stack+0x21/0x50 [ 152.909893][ T8030] kasan_set_track+0x20/0x30 [ 152.910541][ T8030] kasan_set_free_info+0x24/0x40 [ 152.911265][ T8030] __kasan_slab_free+0xf7/0x140 [ 152.911964][ T8030] kfree+0x9e/0x3d0 [ 152.912501][ T8030] alloc_workqueue+0x7d7/0x8e0 [ 152.913182][ T8030] __btrfs_alloc_workqueue+0x11e/0x2a0 [ 152.913949][ T8030] btrfs_alloc_workqueue+0x6d/0x1d0 [ 152.914703][ T8030] scrub_workers_get+0x1e8/0x490 [ 152.915402][ T8030] btrfs_scrub_dev+0x1b9/0x9c0 [ 152.916077][ T8030] btrfs_ioctl+0x122c/0x4e50 [ 152.916729][ T8030] __x64_sys_ioctl+0x137/0x190 [ 152.917414][ T8030] do_syscall_64+0x34/0xb0 [ 152.918034][ T8030] entry_SYSCALL_64_after_hwframe+0x44/0xae [ 152.918872][ T8030] [ 152.919203][ T8030] The buggy address belongs to the object at ffff88810d31bc00 [ 152.919203][ T8030] which belongs to the cache kmalloc-512 of size 512 [ 152.921155][ T8030] The buggy address is located 256 bytes inside of [ 152.921155][ T8030] 512-byte region [ffff88810d31bc00, ffff88810d31be00) [ 152.922993][ T8030] The buggy address belongs to the page: [ 152.923800][ T8030] page:ffffea000434c600 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x10d318 [ 152.925249][ T8030] head:ffffea000434c600 order:2 compound_mapcount:0 compound_pincount:0 [ 152.926399][ T8030] flags: 0x57ff00000010200(slab|head|node=1|zone=2|lastcpupid=0x7ff) [ 152.927515][ T8030] raw: 057ff00000010200 dead000000000100 dead000000000122 ffff888009c42c80 [ 152.928716][ T8030] raw: 0000000000000000 0000000080100010 00000001ffffffff 0000000000000000 [ 152.929890][ T8030] page dumped because: kasan: bad access detected [ 152.930759][ T8030] [ 152.931076][ T8030] Memory state around the buggy address: [ 152.931851][ T8030] ffff88810d31bc00: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 152.932967][ T8030] ffff88810d31bc80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 152.934068][ T8030] >ffff88810d31bd00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 152.935189][ T8030] ^ [ 152.935763][ T8030] ffff88810d31bd80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb [ 152.936847][ T8030] ffff88810d31be00: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc [ 152.937940][ T8030] ================================================================== If apply_wqattrs_prepare() fails in alloc_workqueue(), it will call put_pwq() which invoke a work queue to call pwq_unbound_release_workfn() and use the 'wq'. The 'wq' allocated in alloc_workqueue() will be freed in error path when apply_wqattrs_prepare() fails. So it will lead a UAF. CPU0 CPU1 alloc_workqueue() alloc_and_link_pwqs() apply_wqattrs_prepare() fails apply_wqattrs_cleanup() schedule_work(&pwq->unbound_release_work) kfree(wq) worker_thread() pwq_unbound_release_workfn() <- trigger uaf here If apply_wqattrs_prepare() fails, the new pwq are not linked, it doesn't hold any reference to the 'wq', 'wq' is invalid to access in the worker, so add check pwq if linked to fix this. Fixes: 2d5f076 ("workqueue: split apply_workqueue_attrs() into 3 stages") Cc: stable@vger.kernel.org # v4.2+ Reported-by: Hulk Robot <hulkci@huawei.com> Suggested-by: Lai Jiangshan <jiangshanlai@gmail.com> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Reviewed-by: Lai Jiangshan <jiangshanlai@gmail.com> Tested-by: Pavel Skripkin <paskripkin@gmail.com> Signed-off-by: Tejun Heo <tj@kernel.org>

@ret

[BUG] There is syzbot based reproducer that can crash the kernel, with the following call trace: (With some debug output added) DEBUG: rescue=ibadroots parsed BTRFS: device fsid 14d642db-7b15-43e4-81e6-4b8fac6a25f8 devid 1 transid 8 /dev/loop0 (7:0) scanned by repro (1010) BTRFS info (device loop0): first mount of filesystem 14d642db-7b15-43e4-81e6-4b8fac6a25f8 BTRFS info (device loop0): using blake2b (blake2b-256-generic) checksum algorithm BTRFS info (device loop0): using free-space-tree BTRFS warning (device loop0): checksum verify failed on logical 5312512 mirror 1 wanted 0xb043382657aede36608fd3386d6b001692ff406164733d94e2d9a180412c6003 found 0x810ceb2bacb7f0f9eb2bf3b2b15c02af867cb35ad450898169f3b1f0bd818651 level 0 DEBUG: read tree root path failed for tree csum, ret=-5 BTRFS warning (device loop0): checksum verify failed on logical 5328896 mirror 1 wanted 0x51be4e8b303da58e6340226815b70e3a93592dac3f30dd510c7517454de8567a found 0x51be4e8b303da58e634022a315b70e3a93592dac3f30dd510c7517454de8567a level 0 BTRFS warning (device loop0): checksum verify failed on logical 5292032 mirror 1 wanted 0x1924ccd683be9efc2fa98582ef58760e3848e9043db8649ee382681e220cdee4 found 0x0cb6184f6e8799d9f8cb335dccd1d1832da1071d12290dab3b85b587ecacca6e level 0 process 'repro' launched './file2' with NULL argv: empty string added DEBUG: no csum root, idatacsums=0 ibadroots=134217728 Oops: general protection fault, probably for non-canonical address 0xdffffc0000000041: 0000 [#1] SMP KASAN NOPTI KASAN: null-ptr-deref in range [0x0000000000000208-0x000000000000020f] CPU: 5 UID: 0 PID: 1010 Comm: repro Tainted: G OE 6.15.0-custom+ thesofproject#249 PREEMPT(full) Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022 RIP: 0010:btrfs_lookup_csum+0x93/0x3d0 [btrfs] Call Trace: <TASK> btrfs_lookup_bio_sums+0x47a/0xdf0 [btrfs] btrfs_submit_bbio+0x43e/0x1a80 [btrfs] submit_one_bio+0xde/0x160 [btrfs] btrfs_readahead+0x498/0x6a0 [btrfs] read_pages+0x1c3/0xb20 page_cache_ra_order+0x4b5/0xc20 filemap_get_pages+0x2d3/0x19e0 filemap_read+0x314/0xde0 __kernel_read+0x35b/0x900 bprm_execve+0x62e/0x1140 do_execveat_common.isra.0+0x3fc/0x520 __x64_sys_execveat+0xdc/0x130 do_syscall_64+0x54/0x1d0 entry_SYSCALL_64_after_hwframe+0x76/0x7e ---[ end trace 0000000000000000 ]--- [CAUSE] Firstly the fs has a corrupted csum tree root, thus to mount the fs we have to go "ro,rescue=ibadroots" mount option. Normally with that mount option, a bad csum tree root should set BTRFS_FS_STATE_NO_DATA_CSUMS flag, so that any future data read will ignore csum search. But in this particular case, we have the following call trace that caused NULL csum root, but not setting BTRFS_FS_STATE_NO_DATA_CSUMS: load_global_roots_objectid(): ret = btrfs_search_slot(); /* Succeeded */ btrfs_item_key_to_cpu() found = true; /* We found the root item for csum tree. */ root = read_tree_root_path(); if (IS_ERR(root)) { if (!btrfs_test_opt(fs_info, IGNOREBADROOTS)) /* * Since we have rescue=ibadroots mount option, * @ret is still 0. */ break; if (!found || ret) { /* @found is true, @ret is 0, error handling for csum * tree is skipped. */ } This means we completely skipped to set BTRFS_FS_STATE_NO_DATA_CSUMS if the csum tree is corrupted, which results unexpected later csum lookup. [FIX] If read_tree_root_path() failed, always populate @ret to the error number. As at the end of the function, we need @ret to determine if we need to do the extra error handling for csum tree. Fixes: abed4aa ("btrfs: track the csum, extent, and free space trees in a rb tree") Reported-by: Zhiyu Zhang <zhiyuzhang999@gmail.com> Reported-by: Longxing Li <coregee2000@gmail.com> Reviewed-by: David Sterba <dsterba@suse.com> Signed-off-by: Qu Wenruo <wqu@suse.com> Signed-off-by: David Sterba <dsterba@suse.com>

ranj063 requested review from keyonjie, lgirdwood and plbossart November 7, 2018 03:33

lgirdwood requested changes Nov 7, 2018

View reviewed changes

sound/soc/sof/pcm.c Outdated Show resolved Hide resolved

sound/soc/sof/utils.c Outdated Show resolved Hide resolved

plbossart requested changes Nov 7, 2018

View reviewed changes

ranj063 force-pushed the dsp_pg branch from d65160b to 7d44cb5 Compare November 7, 2018 18:09

ranj063 force-pushed the dsp_pg branch from 7d44cb5 to 5923336 Compare November 8, 2018 07:00

ranj063 force-pushed the dsp_pg branch 2 times, most recently from aa2e6c7 to 89c9edb Compare November 8, 2018 07:15

keyonjie reviewed Nov 8, 2018

View reviewed changes

sound/soc/sof/loader.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-ctrl.c Outdated Show resolved Hide resolved

sound/soc/sof/intel/hda-dsp.c Outdated Show resolved Hide resolved

plbossart requested changes Nov 8, 2018

View reviewed changes

ranj063 force-pushed the dsp_pg branch 4 times, most recently from 2b6bb6d to 6833ffe Compare November 8, 2018 17:56

ranj063 mentioned this pull request Nov 8, 2018

ASoC: SOF: topology: add pm-get/put around topology load #241

Closed

ranj063 added 2 commits November 8, 2018 17:07

ranj063 force-pushed the dsp_pg branch from 1ac91fe to 461d142 Compare November 9, 2018 02:06

ranj063 mentioned this pull request Nov 9, 2018

Topology loading needs pm_runtime_get() held. #193

Closed

mengdonglin mentioned this pull request Nov 9, 2018

pause/resume HDMI playback will cause IPC error and kernel call trace #238

Closed

plbossart requested changes Nov 9, 2018

View reviewed changes

ranj063 force-pushed the dsp_pg branch from 461d142 to 7d7a227 Compare November 9, 2018 17:36

ranj063 added 2 commits November 9, 2018 09:38

ASoC: SOF: add pre/post fw run ops

d69b39e

Add pre/post ops that will be called to perform actions before and after fw run routine. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ASoC: SOF: define pre/post fw run ops for SKL+

f41dd30

This patch defines pre/post fw run ops for SKL+ platforms. Disable clock gating, power gating and L1 support in pre_fw_run. Re-enable these in post_fw_run. Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

ranj063 added 2 commits November 9, 2018 09:38

ranj063 force-pushed the dsp_pg branch from 7d7a227 to a7c6900 Compare November 9, 2018 17:39

plbossart approved these changes Nov 9, 2018

View reviewed changes

plbossart merged commit 57a2bf7 into thesofproject:topic/sof-dev Nov 9, 2018

ranj063 mentioned this pull request Nov 11, 2018

Update kernel ABI to latest FW version #260

Merged

ranj063 deleted the dsp_pg branch March 22, 2019 17:08

This was referenced Apr 25, 2019

[APL][GLK] harden: power-gating and clock gating needs to be disabled during firmware download #776

Closed

Audio DSP power gating and clock gating enable #247

Closed

aiChaoSONG pushed a commit to aiChaoSONG/linux that referenced this pull request May 6, 2021

Merge pull request thesofproject#249 from ojeda/riscv

45ffe64

Finish RISC-V support

PM related changes #249

PM related changes #249

Uh oh!

Conversation

ranj063 commented Nov 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart commented Nov 7, 2018

Uh oh!

ranj063 commented Nov 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

keyonjie left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ranj063 commented Nov 8, 2018

Uh oh!

plbossart commented Nov 9, 2018

Uh oh!

ranj063 commented Nov 9, 2018

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ranj063 commented Nov 9, 2018

Uh oh!

plbossart left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ranj063 commented Nov 7, 2018 •

edited

Loading

ranj063 commented Nov 8, 2018 •

edited

Loading