ALSA/ASoC: Intel: avs: HDAudio bus and general fixes

Cezary Rojewski <cezary.rojewski@intel.com> says:

The first half of the patchset concentrates on the driver initialization
procedure - a number of steps do not clean up after themselves when they
fail.  This is for both, HDAudio bus (ext) and the DSP part (avs) and
targets following procedures:

- stream initialization
- link initialization
- bus initialization

The next five are loosely related fixes.  First, split topology-loading
error handling from request-firmware one.  Currently -ENOENT coming from
topology-loading, which is a real error, is ignored.

Second, with deeper test coverage around corrupted firmware/DSP exception
handling, new issues are unearthed.  Switch to async d0ix_work
cancellation to avoid deadlock when D0IX has been scheduled just before
the recovery work.

The 09/10 patch, init_config change fixes possible out-of-bounds bug.
The fix is larger than what one could expect as instead of patching with
if-statements I've decided to refactor the parsing of init_configs.
The scenario that causes the problem no longer exists.
Note: the solution does not impose any changes on the existing topology
files (userspace).

The remaining two, IMHO are self-explanatory.

[1]: https://lore.kernel.org/all/tencent_8E5BBBD19D53B1EFCDB6E89F3B6246A70B06@qq.com/

Link: https://patch.msgid.link/20260902081814.1590883-1-cezary.rojewski@intel.com
This commit is contained in:
Mark Brown 2026-09-03 22:51:43 +01:00
commit 6b2f3ab85f
No known key found for this signature in database
GPG Key ID: 24D68B725D5487D0
10 changed files with 130 additions and 72 deletions

View File

@ -90,8 +90,10 @@ int snd_hdac_ext_bus_get_ml_capabilities(struct hdac_bus *bus)
for (idx = 0; idx < link_count; idx++) {
hlink = kzalloc_obj(*hlink);
if (!hlink)
if (!hlink) {
snd_hdac_ext_link_free_all(bus);
return -ENOMEM;
}
hlink->index = idx;
hlink->bus = bus;
hlink->ml_addr = bus->mlcap + AZX_ML_BASE +

View File

@ -102,8 +102,10 @@ int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
for (i = 0; i < num_stream; i++) {
struct hdac_ext_stream *hext_stream = kzalloc_obj(*hext_stream);
if (!hext_stream)
if (!hext_stream) {
snd_hdac_ext_stream_free_all(bus);
return -ENOMEM;
}
tag = ++stream_tag;
snd_hdac_ext_stream_init(bus, hext_stream, idx, dir, tag);
idx++;
@ -111,7 +113,6 @@ int snd_hdac_ext_stream_init_all(struct hdac_bus *bus, int start_idx,
}
return 0;
}
EXPORT_SYMBOL_GPL(snd_hdac_ext_stream_init_all);

View File

@ -15,6 +15,22 @@
#include "../../../codecs/hda.h"
#include "../utils.h"
static int avs_link_startup(struct snd_pcm_substream *substream)
{
struct snd_soc_pcm_runtime *rtd = snd_soc_substream_to_rtd(substream);
const struct snd_soc_pcm_stream *stream_info;
struct snd_soc_dai *codec_dai;
codec_dai = snd_soc_rtd_to_codec(rtd, 0);
stream_info = snd_soc_dai_get_pcm_stream(codec_dai, substream->stream);
return snd_pcm_hw_constraint_msbits(substream->runtime, 0, 0, stream_info->sig_bits);
}
static const struct snd_soc_ops avs_link_ops = {
.startup = avs_link_startup,
};
static int avs_create_dai_links(struct device *dev, struct hda_codec *codec, int pcm_count,
struct snd_soc_dai_link **links)
{
@ -43,6 +59,7 @@ static int avs_create_dai_links(struct device *dev, struct hda_codec *codec, int
dl[i].platforms = platform;
dl[i].num_platforms = 1;
dl[i].ignore_pmdown_time = 1;
dl[i].ops = &avs_link_ops;
dl[i].codecs = devm_kzalloc(dev, sizeof(*dl->codecs), GFP_KERNEL);
dl[i].cpus = devm_kzalloc(dev, sizeof(*dl->cpus), GFP_KERNEL);

View File

@ -92,16 +92,28 @@ static int avs_hdac_bus_init_streams(struct hdac_bus *bus)
{
unsigned int cp_streams, pb_streams;
unsigned int gcap;
int ret;
gcap = snd_hdac_chip_readw(bus, GCAP);
cp_streams = (gcap >> 8) & 0x0F;
pb_streams = (gcap >> 12) & 0x0F;
bus->num_streams = cp_streams + pb_streams;
snd_hdac_ext_stream_init_all(bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
snd_hdac_ext_stream_init_all(bus, cp_streams, pb_streams, SNDRV_PCM_STREAM_PLAYBACK);
ret = snd_hdac_ext_stream_init_all(bus, 0, cp_streams, SNDRV_PCM_STREAM_CAPTURE);
if (ret)
return ret;
ret = snd_hdac_ext_stream_init_all(bus, cp_streams, pb_streams, SNDRV_PCM_STREAM_PLAYBACK);
if (ret)
goto err;
return snd_hdac_bus_alloc_stream_pages(bus);
ret = snd_hdac_bus_alloc_stream_pages(bus);
if (ret)
goto err;
return 0;
err:
snd_hdac_ext_stream_free_all(bus);
return ret;
}
static bool avs_hdac_bus_init_chip(struct hdac_bus *bus, bool full_reset)
@ -383,6 +395,18 @@ static int avs_bus_init(struct avs_dev *adev, struct pci_dev *pci, const struct
struct device *dev = &pci->dev;
int ret;
ipc = devm_kzalloc(dev, sizeof(*ipc), GFP_KERNEL);
if (!ipc)
return -ENOMEM;
adev->modcfg_buf = devm_kzalloc(dev, AVS_MAILBOX_SIZE, GFP_KERNEL);
if (!adev->modcfg_buf)
return -ENOMEM;
ret = avs_ipc_init(ipc, dev);
if (ret < 0)
return ret;
ret = snd_hdac_ext_bus_init(&bus->core, dev, NULL, &soc_hda_ext_bus_ops);
if (ret < 0)
return ret;
@ -394,17 +418,6 @@ static int avs_bus_init(struct avs_dev *adev, struct pci_dev *pci, const struct
bus->mixer_assigned = -1;
mutex_init(&bus->prepare_mutex);
ipc = devm_kzalloc(dev, sizeof(*ipc), GFP_KERNEL);
if (!ipc)
return -ENOMEM;
ret = avs_ipc_init(ipc, dev);
if (ret < 0)
return ret;
adev->modcfg_buf = devm_kzalloc(dev, AVS_MAILBOX_SIZE, GFP_KERNEL);
if (!adev->modcfg_buf)
return -ENOMEM;
adev->dev = dev;
adev->spec = (const struct avs_spec *)id->driver_data;
adev->ipc = ipc;
@ -456,13 +469,14 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
ret = pcim_request_all_regions(pci, "AVS HDAudio");
if (ret < 0)
return ret;
goto err_request_regions;
bus->addr = pci_resource_start(pci, 0);
bus->remap_addr = pci_ioremap_bar(pci, 0);
if (!bus->remap_addr) {
dev_err(bus->dev, "ioremap error\n");
return -ENXIO;
ret = -ENXIO;
goto err_request_regions;
}
adev->dsp_ba = pci_ioremap_bar(pci, 4);
@ -473,8 +487,13 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
}
snd_hdac_bus_parse_capabilities(bus);
if (bus->mlcap)
snd_hdac_ext_bus_get_ml_capabilities(bus);
if (bus->mlcap) {
ret = snd_hdac_ext_bus_get_ml_capabilities(bus);
if (ret < 0) {
dev_err(dev, "failed to get ml capabilities: %d\n", ret);
goto err_ml_cap;
}
}
if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)))
dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
@ -516,9 +535,13 @@ static int avs_pci_probe(struct pci_dev *pci, const struct pci_device_id *id)
snd_hdac_bus_free_stream_pages(bus);
snd_hdac_ext_stream_free_all(bus);
err_init_streams:
snd_hdac_ext_link_free_all(bus);
err_ml_cap:
iounmap(adev->dsp_ba);
err_remap_bar4:
iounmap(bus->remap_addr);
err_request_regions:
snd_hdac_ext_bus_exit(bus);
return ret;
}

View File

@ -9,6 +9,7 @@
#include <linux/cleanup.h>
#include <linux/debugfs.h>
#include <linux/kfifo.h>
#include <linux/module.h>
#include <linux/wait.h>
#include <linux/sched/signal.h>
#include <linux/string_helpers.h>
@ -236,15 +237,20 @@ static int strace_open(struct inode *inode, struct file *file)
if (!try_module_get(adev->dev->driver->owner))
return -ENODEV;
if (kfifo_initialized(&adev->trace_fifo))
return -EBUSY;
if (kfifo_initialized(&adev->trace_fifo)) {
ret = -EBUSY;
goto err;
}
ret = kfifo_alloc(&adev->trace_fifo, PAGE_SIZE, GFP_KERNEL);
if (ret < 0)
return ret;
goto err;
file->private_data = adev;
return 0;
err:
module_put(adev->dev->driver->owner);
return ret;
}
static int strace_release(struct inode *inode, struct file *file)

View File

@ -172,7 +172,7 @@ static void avs_dsp_exception_caught(struct avs_dev *adev, union avs_notify_msg
/* Avoid deadlock as the exception may be the response to SET_D0IX. */
if (current_work() != &ipc->d0ix_work.work)
cancel_delayed_work_sync(&ipc->d0ix_work);
cancel_delayed_work(&ipc->d0ix_work);
ipc->in_d0ix = false;
/* Re-enabled on recovery completion. */
pm_runtime_disable(adev->dev);
@ -395,11 +395,11 @@ static int avs_dsp_do_send_msg(struct avs_dev *adev, struct avs_ipc_msg *request
struct avs_ipc *ipc = adev->ipc;
int ret;
guard(mutex)(&ipc->msg_mutex);
if (!ipc->ready)
return -EPERM;
guard(mutex)(&ipc->msg_mutex);
spin_lock(&ipc->rx_lock);
avs_ipc_msg_init(ipc, reply);
avs_dsp_send_tx(adev, request, true);

View File

@ -836,15 +836,10 @@ static int avs_path_module_type_create(struct avs_dev *adev, struct avs_path_mod
static int avs_path_module_send_init_configs(struct avs_dev *adev, struct avs_path_module *mod)
{
struct avs_soc_component *acomp;
struct avs_tplg_module *template = mod->template;
acomp = to_avs_soc_component(mod->template->owner->owner->owner->owner->comp);
u32 num_ids = mod->template->num_config_ids;
u32 *ids = mod->template->config_ids;
for (int i = 0; i < num_ids; i++) {
struct avs_tplg_init_config *config = &acomp->tplg->init_configs[ids[i]];
for (int i = 0; i < template->num_init_configs; i++) {
struct avs_tplg_init_config *config = template->init_configs[i];
size_t len = config->length;
void *data = config->data;
u32 param = config->param;

View File

@ -6,6 +6,7 @@
// Amadeusz Slawinski <amadeuszx.slawinski@linux.intel.com>
//
#include <linux/cleanup.h>
#include <linux/debugfs.h>
#include <linux/device.h>
#include <sound/hda_register.h>
@ -987,13 +988,25 @@ static int avs_component_load_libraries(struct avs_soc_component *acomp)
return ret;
}
static int avs_request_topology(struct snd_soc_component *component, const char *name,
const struct firmware **fw)
{
char *fullname __free(kfree) = NULL;
fullname = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix, name);
if (!fullname)
return -ENOMEM;
return request_firmware(fw, fullname, component->dev);
}
static int avs_component_probe(struct snd_soc_component *component)
{
struct snd_soc_card *card = component->card;
struct snd_soc_acpi_mach *mach;
struct avs_soc_component *acomp;
const struct firmware *fw;
struct avs_dev *adev;
char *filename;
int ret;
dev_dbg(card->dev, "probing %s card %s\n", component->name, card->name);
@ -1009,13 +1022,7 @@ static int avs_component_probe(struct snd_soc_component *component)
goto finalize;
/* Load specified topology and create debugfs for it. */
filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix,
mach->tplg_filename);
if (!filename)
return -ENOMEM;
ret = avs_load_topology(component, filename);
kfree(filename);
ret = avs_request_topology(component, mach->tplg_filename, &fw);
if (ret == -ENOENT && !strncmp(mach->tplg_filename, "hda-", 4)) {
unsigned int vendor_id;
@ -1030,18 +1037,17 @@ static int avs_component_probe(struct snd_soc_component *component)
"hda-generic-tplg.bin");
if (!mach->tplg_filename)
return -ENOMEM;
filename = kasprintf(GFP_KERNEL, "%s/%s", component->driver->topology_name_prefix,
mach->tplg_filename);
if (!filename)
return -ENOMEM;
dev_info(card->dev, "trying to load fallback topology %s\n", mach->tplg_filename);
ret = avs_load_topology(component, filename);
kfree(filename);
ret = avs_request_topology(component, mach->tplg_filename, &fw);
}
if (ret < 0)
return ret;
ret = snd_soc_tplg_component_load(component, &avs_tplg_ops, fw);
if (ret)
return ret;
ret = avs_component_load_libraries(acomp);
if (ret < 0) {
dev_err(card->dev, "libraries loading failed: %d\n", ret);

View File

@ -350,6 +350,7 @@ AVS_DEFINE_PTR_PARSER(modcfg_base, struct avs_tplg_modcfg_base, modcfgs_base);
AVS_DEFINE_PTR_PARSER(modcfg_ext, struct avs_tplg_modcfg_ext, modcfgs_ext);
AVS_DEFINE_PTR_PARSER(pplcfg, struct avs_tplg_pplcfg, pplcfgs);
AVS_DEFINE_PTR_PARSER(binding, struct avs_tplg_binding, bindings);
AVS_DEFINE_PTR_PARSER(init_config, struct avs_tplg_init_config, init_configs);
AVS_DEFINE_PTR_PARSER(nhlt_config, struct avs_tplg_nhlt_config, nhlt_configs);
static int
@ -1198,7 +1199,7 @@ static const struct avs_tplg_token_parser module_parsers[] = {
{
.token = AVS_TKN_MOD_INIT_CONFIG_NUM_IDS_U32,
.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
.offset = offsetof(struct avs_tplg_module, num_config_ids),
.offset = offsetof(struct avs_tplg_module, num_init_configs),
.parse = avs_parse_byte_token,
},
{
@ -1214,10 +1215,32 @@ static const struct avs_tplg_token_parser init_config_parsers[] = {
.token = AVS_TKN_MOD_INIT_CONFIG_ID_U32,
.type = SND_SOC_TPLG_TUPLE_TYPE_WORD,
.offset = 0,
.parse = avs_parse_word_token,
.parse = avs_parse_init_config_ptr,
},
};
static int avs_tplg_module_init_configs(struct snd_soc_component *comp,
struct avs_tplg_module *module,
struct snd_soc_tplg_vendor_array *tuples, u32 block_size)
{
struct avs_tplg_init_config **cfgs;
int ret;
if (!module->num_init_configs)
return -EINVAL;
cfgs = devm_kcalloc(comp->card->dev, module->num_init_configs, sizeof(*cfgs), GFP_KERNEL);
if (!cfgs)
return -ENOMEM;
ret = parse_dictionary_entries(comp, tuples, block_size, cfgs, module->num_init_configs,
sizeof(*cfgs), AVS_TKN_MOD_INIT_CONFIG_ID_U32,
init_config_parsers, ARRAY_SIZE(init_config_parsers));
if (!ret)
module->init_configs = cfgs;
return ret;
}
static struct avs_tplg_module *
avs_tplg_module_create(struct snd_soc_component *comp, struct avs_tplg_pipeline *owner,
struct snd_soc_tplg_vendor_array *tuples, u32 block_size)
@ -1244,27 +1267,11 @@ avs_tplg_module_create(struct snd_soc_component *comp, struct avs_tplg_pipeline
block_size -= esize;
/* Parse trailing config ids if any. */
if (block_size) {
u32 num_config_ids = module->num_config_ids;
u32 *config_ids;
if (!num_config_ids)
return ERR_PTR(-EINVAL);
config_ids = devm_kcalloc(comp->card->dev, num_config_ids, sizeof(*config_ids),
GFP_KERNEL);
if (!config_ids)
return ERR_PTR(-ENOMEM);
tuples = avs_tplg_vendor_array_at(tuples, esize);
ret = parse_dictionary_entries(comp, tuples, block_size,
config_ids, num_config_ids, sizeof(*config_ids),
AVS_TKN_MOD_INIT_CONFIG_ID_U32,
init_config_parsers,
ARRAY_SIZE(init_config_parsers));
ret = avs_tplg_module_init_configs(comp, module, tuples, block_size);
if (ret)
return ERR_PTR(ret);
module->config_ids = config_ids;
}
module->owner = owner;
@ -2194,7 +2201,7 @@ avs_control_load(struct snd_soc_component *comp, int index, struct snd_kcontrol_
return 0;
}
static const struct snd_soc_tplg_ops avs_tplg_ops = {
const struct snd_soc_tplg_ops avs_tplg_ops = {
.io_ops = avs_control_ops,
.io_ops_count = ARRAY_SIZE(avs_control_ops),
.control_load = avs_control_load,

View File

@ -221,8 +221,8 @@ struct avs_tplg_module {
u8 domain;
struct avs_tplg_modcfg_ext *cfg_ext;
u32 ctl_id;
u32 num_config_ids;
u32 *config_ids;
u32 num_init_configs;
struct avs_tplg_init_config **init_configs;
struct avs_tplg_nhlt_config *nhlt_config;
struct avs_tplg_pipeline *owner;
@ -230,6 +230,7 @@ struct avs_tplg_module {
struct list_head node;
};
extern const struct snd_soc_tplg_ops avs_tplg_ops;
struct avs_tplg *avs_tplg_new(struct snd_soc_component *comp);
int avs_load_topology(struct snd_soc_component *comp, const char *filename);