[alsa-devel] [PATCH] ASoC: Intel: skl_hda_dsp_common: Fix global-out-of-bounds bug

[Wasko, Michal]

On 1/23/2020 7:22 PM, Pierre-Louis Bossart wrote:
>
>
>>>> For the last few days we have been playing with "vanilla" 5.5 
>>>> kernel - one without ton of /skylake patches - to find out how 
>>>> could hda-dsp be enabled on skl/ kbl+ with the least amount of 
>>>> changes pulled from our branch possible.
>>>>
>>>> Turned out the addition of this single patch AND topology binary 
>>>> update got the job done.
>>>>
>>>> Now, how can we proceed with such solution. Can share the topology 
>>>> binary/ .conf if needed, so anyone interested can check it out.
>>>
>>> I am personally interested for tests but I doubt this option is 
>>> usable by anyone outside of Intel - additional issues with probe 
>>> race conditions with i915, e.g. on Linus' Dell XPS 9350, no DMIC 
>>> support and not selected anyways by Jaroslav's new logic, no UCM, 
>>> and no plans for the use of the HDMI common codec.
>>
>> The Linux Skylake driver officially support audio over DSP on Intel 
>> cAVS 1.5+ boards, that include Skylake HW target with hda-dsp 
>> configuration. The configuration is regularly tested by Intel Audio 
>> CI team.
>>
>> As it was agreed with you Pierre the Skylake driver will be kept 
>> under maintenance and the proposed changes are about to keep hda-dsp 
>> configuration functional for anyone who would like to use it. Linus 
>> laptop issue is actually one of the good reasons why we would like to 
>> keep hda-dsp configuration functional
>
> We have to agree on what 'maintained' means then.
>
> I don't mind leaving the Skylake driver in the kernel and letting 
> people who have access to Intel support use it. Cezary is listed as 
> the maintainer as I suggested it, and this patch provides an necessary 
> fix.
>
> But does this mean this Hdaudio option is usable by distributions and 
> Linux users who don't have access to Intel support?
>
> I will assert that it's not, based on my own experience only 2 weeks 
> ago. I tried to make audio work on a KBL NUC and had to comment stuff 
> out due to an obsolete topology. see 
> https://github.com/thesofproject/linux/pull/1667#issuecomment-572312157
That is exactly the reason why we would like to update the Skylake 
driver upstream code and it configuration files so that it will be 
usable by the community and not only keep it functional internally. As 
it was clarified by Cezary, we would like to make a minimum number of 
changes that are required.

Is there Pierre any non-technical reason why we should not fix the 
Skylake driver code on the upstream?
>
> You should also look at the help text for the option:
>
> config SND_SOC_INTEL_SKYLAKE_HDAUDIO_CODEC
>     bool "HDAudio codec support"
>     help
>       This option broke audio on Linus' Skylake laptop in December 2018
>       and the race conditions during the probe were not fixed since.
>       This option is DEPRECATED, all HDaudio codec support needs
>       to be handled by the SOF driver.
>       Distributions should not enable this option and there are no known
>       users of this capability.
>
> No one objected to this wording back in October, but we still see this 
> option selected in multiple distros, so the last suggestion is to move 
> to an opt-in selection to guide distributions.
>
>> Your other statements Pierre are quite outdated:
>>
>>      - Probe race conditions with i915 - resolved in HDA
>
> I checked last month and things still break on the Dell XPS. There are 
> challenging race conditions that are not seen on Intel RVPs and NUCs, 
> but broke Linus' laptop and a slew of others:
>
> https://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143549.html 
>
>
> https://mailman.alsa-project.org/pipermail/alsa-devel/2018-December/143596.html 
>
>
> Unless you've verified SST support on those platforms, your claim of 
> 'resolved' is invalid.
>
>>      - DMIC is supported
>
> There is no topology provided with DMIC+HDaudio support. I asked for 
> this more than 18 months ago and it was never made available, even to 
> me, and SOF become the default solution for HDAudio+DMIC cases.
>
>>      - UCM is not directly driver related and can be easily updated
>
> "easily", but hasn't been done in 18 months, and it actually takes a 
> lot of work to get things right. Especially with the SST driver and 
> the mixers required on the platform side since nothing is connected by 
> default.
>
>>      - Intel Audio CI was focused on common HD-A codec but the HDMI 
>> common codec is supported as well
>>
>>> In case you didn't see it, the Skylake driver 'HDaudio codec' option 
>>> is suggested as one of the 'unsupported' features here:
>>> https://github.com/thesofproject/linux/pull/1742
>>>
>>> -Pierre
>>
>> The suggestion to mark the Skylake driver 'HDaudio codec' option as 
>> 'unsupported' is coming from you Pierre (patch from two daysago?) and 
>> I believe that you should consult such opinion with Intel Skylake 
>> driver maintainers.
>
> You were in copy and did not comment, same for Cezary.
>
> Maybe 'unsupported' is too strong a word, but it was Takashi's 
> suggestion :-)
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel at alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel