| 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
| ||
|
Message-ID: <1a80c749-1cbc-4ad9-ac14-dec660bd7f8b@linux.intel.com>
Date: Tue, 6 Aug 2024 16:49:51 +0200
From: Amadeusz Sławiński
<amadeuszx.slawinski@...ux.intel.com>
To: Wesley Cheng <quic_wcheng@...cinc.com>, srinivas.kandagatla@...aro.org,
mathias.nyman@...el.com, perex@...ex.cz, conor+dt@...nel.org,
corbet@....net, broonie@...nel.org, lgirdwood@...il.com, krzk+dt@...nel.org,
Thinh.Nguyen@...opsys.com, bgoswami@...cinc.com, tiwai@...e.com,
gregkh@...uxfoundation.org, robh@...nel.org
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
linux-sound@...r.kernel.org, linux-usb@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-doc@...r.kernel.org,
alsa-devel@...a-project.org, Mathias Nyman <mathias.nyman@...ux.intel.com>
Subject: Re: [PATCH v24 03/34] xhci: sideband: add initial api to register a
sideband entity
On 8/1/2024 3:16 AM, Wesley Cheng wrote:
> From: Mathias Nyman <mathias.nyman@...ux.intel.com>
>
> Introduce XHCI sideband, which manages the USB endpoints being requested by
> a client driver. This is used for when client drivers are attempting to
> offload USB endpoints to another entity for handling USB transfers. XHCI
> sideband will allow for drivers to fetch the required information about the
> transfer ring, so the user can submit transfers independently. Expose the
> required APIs for drivers to register and request for a USB endpoint and to
> manage XHCI secondary interrupters.
>
> Multiple ring segment page linking, proper endpoint clean up, and allowing
> module compliation added by Wesley Cheng to complete original concept code
> by Mathias Nyman.
>
> Signed-off-by: Mathias Nyman <mathias.nyman@...ux.intel.com>
> Co-developed-by: Wesley Cheng <quic_wcheng@...cinc.com>
> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
> ---
> drivers/usb/host/Kconfig | 9 +
> drivers/usb/host/Makefile | 2 +
> drivers/usb/host/xhci-sideband.c | 419 ++++++++++++++++++++++++++++++
> drivers/usb/host/xhci.h | 4 +
> include/linux/usb/xhci-sideband.h | 68 +++++
> 5 files changed, 502 insertions(+)
> create mode 100644 drivers/usb/host/xhci-sideband.c
> create mode 100644 include/linux/usb/xhci-sideband.h
>
> diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
> index 4448d0ab06f0..6135603c5dc4 100644
> --- a/drivers/usb/host/Kconfig
> +++ b/drivers/usb/host/Kconfig
> @@ -104,6 +104,15 @@ config USB_XHCI_RZV2M
> Say 'Y' to enable the support for the xHCI host controller
> found in Renesas RZ/V2M SoC.
>
> +config USB_XHCI_SIDEBAND
> + tristate "xHCI support for sideband"
> + help
> + Say 'Y' to enable the support for the xHCI sideband capability.
> + provide a mechanism for a sideband datapath for payload associated
Sentence should start from capital letter, so provide -> Provide
> + with audio class endpoints. This allows for an audio DSP to use
> + xHCI USB endpoints directly, allowing CPU to sleep while playing
> + audio
Missing '.' at the end of sentence.
(...)
> +/**
> + * xhci_sideband_remove_endpoint - remove endpoint from sideband access list
> + * @sb: sideband instance for this usb device
> + * @host_ep: usb host endpoint
> + *
> + * Removes an endpoint from the list of sideband accessed endpoints for this usb
> + * device.
> + * sideband client should no longer touch the endpoint transfer buffer after
> + * calling this.
> + *
> + * Return: 0 on success, negative error otherwise.
> + */
> +int
> +xhci_sideband_remove_endpoint(struct xhci_sideband *sb,
> + struct usb_host_endpoint *host_ep)
> +{
> + struct xhci_virt_ep *ep;
> + unsigned int ep_index;
> +
> + mutex_lock(&sb->mutex);
> + ep_index = xhci_get_endpoint_index(&host_ep->desc);
> + ep = sb->eps[ep_index];
> +
> + if (!ep || !ep->sideband) {
> + mutex_unlock(&sb->mutex);
> + return -ENODEV;
> + }
> +
> + __xhci_sideband_remove_endpoint(sb, ep);
> + xhci_initialize_ring_info(ep->ring, 1);
> + mutex_unlock(&sb->mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(xhci_sideband_remove_endpoint);
> +
> +int
> +xhci_sideband_stop_endpoint(struct xhci_sideband *sb,
> + struct usb_host_endpoint *host_ep)
> +{
> + struct xhci_virt_ep *ep;
> + unsigned int ep_index;
> +
> + ep_index = xhci_get_endpoint_index(&host_ep->desc);
> + ep = sb->eps[ep_index];
> +
> + if (!ep || ep->sideband != sb)
Any reason why we check if ep->sideband != sb only on stop but not on
remove above?
> + return -EINVAL;
> +
> + return xhci_stop_endpoint_sync(sb->xhci, ep, 0, GFP_KERNEL);
> +}
> +EXPORT_SYMBOL_GPL(xhci_sideband_stop_endpoint);
> +
> +/**
> + * xhci_sideband_get_endpoint_buffer - gets the endpoint transfer buffer address
> + * @sb: sideband instance for this usb device
> + * @host_ep: usb host endpoint
> + *
> + * Returns the address of the endpoint buffer where xHC controller reads queued
> + * transfer TRBs from. This is the starting address of the ringbuffer where the
> + * sideband client should write TRBs to.
> + *
> + * Caller needs to free the returned sg_table
> + *
> + * Return: struct sg_table * if successful. NULL otherwise.
> + */
> +struct sg_table *
> +xhci_sideband_get_endpoint_buffer(struct xhci_sideband *sb,
> + struct usb_host_endpoint *host_ep)
> +{
> + struct xhci_virt_ep *ep;
> + unsigned int ep_index;
> +
> + ep_index = xhci_get_endpoint_index(&host_ep->desc);
> + ep = sb->eps[ep_index];
> +
> + if (!ep)
And here there is none of checks done in above 2 functions? Seems bit weird.
> + return NULL;
> +
> + return xhci_ring_to_sgtable(sb, ep->ring);
> +}
> +EXPORT_SYMBOL_GPL(xhci_sideband_get_endpoint_buffer);
> +
(...)
> +MODULE_DESCRIPTION("XHCI sideband driver for secondary interrupter management");
XHCI -> xHCI
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
> index efbd1f651da4..9232c53d204a 100644
> --- a/drivers/usb/host/xhci.h
> +++ b/drivers/usb/host/xhci.h
> @@ -693,6 +693,8 @@ struct xhci_virt_ep {
> int next_frame_id;
> /* Use new Isoch TRB layout needed for extended TBC support */
> bool use_extended_tbc;
> + /* set if this endpoint is controlled via sideband access*/
> + struct xhci_sideband *sideband;
> };
>
> enum xhci_overhead_type {
> @@ -755,6 +757,8 @@ struct xhci_virt_device {
> u16 current_mel;
> /* Used for the debugfs interfaces. */
> void *debugfs_private;
> + /* set if this device is registered for sideband access */
> + struct xhci_sideband *sideband;
> };
>
> /*
> diff --git a/include/linux/usb/xhci-sideband.h b/include/linux/usb/xhci-sideband.h
> new file mode 100644
> index 000000000000..1035dae43cee
> --- /dev/null
> +++ b/include/linux/usb/xhci-sideband.h
> @@ -0,0 +1,68 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * xHCI host controller sideband support
> + *
> + * Copyright (c) 2023, Intel Corporation.
> + *
> + * Author: Mathias Nyman <mathias.nyman@...ux.intel.com>
> + */
> +
> +#ifndef __LINUX_XHCI_SIDEBAND_H
> +#define __LINUX_XHCI_SIDEBAND_H
> +
> +#include <linux/scatterlist.h>
> +#include <linux/usb.h>
> +
> +#define EP_CTX_PER_DEV 31 /* FIMXME defined twice, from xhci.h */
If it is left for later, FIMXME -> FIXME
(...)
Powered by blists - more mailing lists