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, 20 May 2020 09:54:37 -0300
From:   Jason Gunthorpe <jgg@...pe.ca>
To:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>
Cc:     davem@...emloft.net, gregkh@...uxfoundation.org,
        Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>,
        netdev@...r.kernel.org, linux-rdma@...r.kernel.org,
        nhorman@...hat.com, sassmann@...hat.com,
        pierre-louis.bossart@...ux.intel.com,
        Fred Oh <fred.oh@...ux.intel.com>
Subject: Re: [net-next v4 10/12] ASoC: SOF: Introduce descriptors for SOF
 client

On Wed, May 20, 2020 at 12:02:25AM -0700, Jeff Kirsher wrote:
> From: Ranjani Sridharan <ranjani.sridharan@...ux.intel.com>
> 
> A client in the SOF (Sound Open Firmware) context is a
> device that needs to communicate with the DSP via IPC
> messages. The SOF core is responsible for serializing the
> IPC messages to the DSP from the different clients. One
> example of an SOF client would be an IPC test client that
> floods the DSP with test IPC messages to validate if the
> serialization works as expected. Multi-client support will
> also add the ability to split the existing audio cards
> into multiple ones, so as to e.g. to deal with HDMI with a
> dedicated client instead of adding HDMI to all cards.
> 
> This patch introduces descriptors for SOF client driver
> and SOF client device along with APIs for registering
> and unregistering a SOF client driver, sending IPCs from
> a client device and accessing the SOF core debugfs root entry.
> 
> Along with this, add a couple of new members to struct
> snd_sof_dev that will be used for maintaining the list of
> clients.

If you want to use sound as the rational for virtual bus then drop the
networking stuff and present a complete device/driver pairing based on
this sound stuff instead.

> +int sof_client_dev_register(struct snd_sof_dev *sdev,
> +			    const char *name)
> +{
> +	struct sof_client_dev *cdev;
> +	struct virtbus_device *vdev;
> +	unsigned long time, timeout;
> +	int ret;
> +
> +	cdev = kzalloc(sizeof(*cdev), GFP_KERNEL);
> +	if (!cdev)
> +		return -ENOMEM;
> +
> +	cdev->sdev = sdev;
> +	init_completion(&cdev->probe_complete);
> +	vdev = &cdev->vdev;
> +	vdev->match_name = name;
> +	vdev->dev.parent = sdev->dev;
> +	vdev->release = sof_client_virtdev_release;
> +
> +	/*
> +	 * Register virtbus device for the client.
> +	 * The error path in virtbus_register_device() calls put_device(),
> +	 * which will free cdev in the release callback.
> +	 */
> +	ret = virtbus_register_device(vdev);
> +	if (ret < 0)
> +		return ret;
> +
> +	/* make sure the probe is complete before updating client list */
> +	timeout = msecs_to_jiffies(SOF_CLIENT_PROBE_TIMEOUT_MS);
> +	time = wait_for_completion_timeout(&cdev->probe_complete, timeout);

This seems bonkers - the whole point of something like virtual bus is
to avoid madness like this.

> +	if (!time) {
> +		dev_err(sdev->dev, "error: probe of virtbus dev %s timed out\n",
> +			name);
> +		virtbus_unregister_device(vdev);

Unregister does kfree? In general I've found that to be a bad idea,
many drivers need to free up resources after unregistering from their
subsystem.

> +#define virtbus_dev_to_sof_client_dev(virtbus_dev) \
> +	container_of(virtbus_dev, struct sof_client_dev, vdev)

Use static inline

Jason