netdev - Re: [net-next v4 10/12] ASoC: SOF: Introduce descriptors for SOF client

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 May 2020 09:05:16 -0500
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Greg KH <gregkh@...uxfoundation.org>, Takashi Iwai <tiwai@...e.de>
Cc:     Jason Gunthorpe <jgg@...pe.ca>,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        davem@...emloft.net, netdev@...r.kernel.org,
        linux-rdma@...r.kernel.org, nhorman@...hat.com,
        sassmann@...hat.com, Fred Oh <fred.oh@...ux.intel.com>
Subject: Re: [net-next v4 10/12] ASoC: SOF: Introduce descriptors for SOF
 client



>>>>> If yes, that's yet another problem... During the PCI probe, we start a
>>>>> workqueue and return success to avoid blocking everything.
>>>>
>>>> That's crazy.
>>>>
>>>>> And only 'later' do we actually create the card. So that's two levels
>>>>> of probe that cannot report a failure. I didn't come up with this
>>>>> design, IIRC this is due to audio-DRM dependencies and it's been used
>>>>> for 10+ years.
>>>>
>>>> Then if the probe function fails, it needs to unwind everything itself
>>>> and unregister the device with the PCI subsystem so that things work
>>>> properly.  If it does not do that today, that's a bug.
>>>>
>>>> What kind of crazy dependencies cause this type of "requirement"?
>>>
>>> I think it is related to the request_module("i915") in
>>> snd_hdac_i915_init(), and possibly other firmware download.
>>>
>>> Adding Takashi for more details.
>>
>> Right, there are a few levels of complexity there.  The HD-audio
>> PCI controller driver, for example, is initialized in an async way
>> with a work.  It loads the firmware files with
>> request_firmware_nowait() and also binds itself as a component master
>> with the DRM graphics driver via component framework.
>>
>> Currently it has no way to unwind the PCI binding itself at the error
>> path, though.  In theory it should be possible to unregister the PCI
>> from the driver itself in the work context, but it failed in the
>> earlier experiments, hence the driver sets itself in a disabled state
>> instead.  Maybe worth to try again.
>>
>> But, to be noted, all belonging sub-devices aren't instantiated but
>> deleted at the error path.  Only the main PCI binding is kept in a
>> disabled state just as a place holder until it's unbound explicitly.
> 
> Ok, that's good to hear.  But platform devices should never be showing
> up as a child of a PCI device.  In the "near future" when we get the
> virtual bus code merged, we can convert any existing users like this to
> the new code.

yes that's the plan. It'll be however more than a 1:1 replacement, i.e. 
we want to use this opportunity to split existing cards into separate 
ones when it makes sense to do so. There's really no rationale for 
having code to deal with HDMI in each machine driver when we could have 
a single driver for HDMI. That's really what drove us to suggest this 
patchset based on the virtual bus: removal of platform devices + 
repartition.