Merge branch 'wwan-debugfs-tweaks'

Sergey Ryazanov says:

====================
WWAN debugfs tweaks

This is a follow-up series to just applied IOSM (and WWAN) debugfs
interface support [1]. The series has two main goals:
1. move the driver-specific debugfs knobs to a subdirectory;
2. make the debugfs interface optional for both IOSM and for the WWAN
   core.

As for the first part, I must say that it was my mistake. I suggested to
place debugfs entries under a common per WWAN device directory. But I
missed the driver subdirectory in the example, so it become:

/sys/kernel/debugfs/wwan/wwan0/trace

Since the traces collection is a driver-specific feature, it is better
to keep it under the driver-specific subdirectory:

/sys/kernel/debugfs/wwan/wwan0/iosm/trace

It is desirable to be able to entirely disable the debugfs interface. It
can be disabled for several reasons, including security and consumed
storage space. See detailed rationale with usage example in the 4th
patch.

The changes themselves are relatively simple, but require a code
rearrangement. So to make changes clear, I chose to split them into
preparatory and main changes and properly describe each of them.

IOSM part is compile-tested only since I do not have IOSM supported
device, so it needs Ack from the driver developers.

I would like to thank Johannes Berg and Leon Romanovsky. Their
suggestions and comments helped a lot to rework the initial
over-engineered solution to something less confusing and much more
simple. Thanks!

1. https://lore.kernel.org/netdev/20211120162155.1216081-1-m.chetan.kumar@linux.intel.com
2. https://patchwork.kernel.org/project/netdevbpf/patch/20211204174033.950528-1-arnd@kernel.org/
====================

Link: https://lore.kernel.org/r/20211207092140.19142-1-ryazanov.s.a@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Jakub Kicinski 2021-12-08 17:59:03 -08:00
commit 16daf3d9ec
12 changed files with 134 additions and 41 deletions

View File

@ -16,6 +16,17 @@ config WWAN
if WWAN
config WWAN_DEBUGFS
bool "WWAN devices debugfs interface" if EXPERT
depends on DEBUG_FS
default y
help
Enables debugfs infrastructure for the WWAN core and device drivers.
If this option is selected, then you can find the debug interface
elements for each WWAN device in a directory that is corresponding to
the device name: debugfs/wwan/wwanX.
config WWAN_HWSIM
tristate "Simulated WWAN device"
help
@ -85,7 +96,7 @@ config IOSM
tristate "IOSM Driver for Intel M.2 WWAN Device"
depends on INTEL_IOMMU
select NET_DEVLINK
select RELAY
select RELAY if WWAN_DEBUGFS
help
This driver enables Intel M.2 WWAN Device communication.

View File

@ -21,7 +21,10 @@ iosm-y = \
iosm_ipc_mux_codec.o \
iosm_ipc_devlink.o \
iosm_ipc_flash.o \
iosm_ipc_coredump.o \
iosm_ipc_coredump.o
iosm-$(CONFIG_WWAN_DEBUGFS) += \
iosm_ipc_debugfs.o \
iosm_ipc_trace.o
obj-$(CONFIG_IOSM) := iosm.o

View File

@ -0,0 +1,29 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (C) 2020-2021 Intel Corporation.
*/
#include <linux/debugfs.h>
#include <linux/wwan.h>
#include "iosm_ipc_imem.h"
#include "iosm_ipc_trace.h"
#include "iosm_ipc_debugfs.h"
void ipc_debugfs_init(struct iosm_imem *ipc_imem)
{
struct dentry *debugfs_pdev = wwan_get_debugfs_dir(ipc_imem->dev);
ipc_imem->debugfs_dir = debugfs_create_dir(KBUILD_MODNAME,
debugfs_pdev);
ipc_imem->trace = ipc_trace_init(ipc_imem);
if (!ipc_imem->trace)
dev_warn(ipc_imem->dev, "trace channel init failed");
}
void ipc_debugfs_deinit(struct iosm_imem *ipc_imem)
{
ipc_trace_deinit(ipc_imem->trace);
debugfs_remove_recursive(ipc_imem->debugfs_dir);
}

View File

@ -0,0 +1,17 @@
/* SPDX-License-Identifier: GPL-2.0-only
*
* Copyright (C) 2020-2021 Intel Corporation.
*/
#ifndef IOSM_IPC_DEBUGFS_H
#define IOSM_IPC_DEBUGFS_H
#ifdef CONFIG_WWAN_DEBUGFS
void ipc_debugfs_init(struct iosm_imem *ipc_imem);
void ipc_debugfs_deinit(struct iosm_imem *ipc_imem);
#else
static inline void ipc_debugfs_init(struct iosm_imem *ipc_imem) {}
static inline void ipc_debugfs_deinit(struct iosm_imem *ipc_imem) {}
#endif
#endif

View File

@ -11,6 +11,7 @@
#include "iosm_ipc_imem.h"
#include "iosm_ipc_port.h"
#include "iosm_ipc_trace.h"
#include "iosm_ipc_debugfs.h"
/* Check the wwan ips if it is valid with Channel as input. */
static int ipc_imem_check_wwan_ips(struct ipc_mem_channel *chnl)
@ -272,8 +273,8 @@ static void ipc_imem_dl_skb_process(struct iosm_imem *ipc_imem,
if (port_id == IPC_MEM_CTRL_CHL_ID_7)
ipc_imem_sys_devlink_notify_rx(ipc_imem->ipc_devlink,
skb);
else if (port_id == ipc_imem->trace->chl_id)
ipc_trace_port_rx(ipc_imem->trace, skb);
else if (ipc_is_trace_channel(ipc_imem, port_id))
ipc_trace_port_rx(ipc_imem, skb);
else
wwan_port_rx(ipc_imem->ipc_port[port_id]->iosm_port,
skb);
@ -554,11 +555,7 @@ static void ipc_imem_run_state_worker(struct work_struct *instance)
ctrl_chl_idx++;
}
ipc_imem->trace = ipc_imem_trace_channel_init(ipc_imem);
if (!ipc_imem->trace) {
dev_err(ipc_imem->dev, "trace channel init failed");
return;
}
ipc_debugfs_init(ipc_imem);
ipc_task_queue_send_task(ipc_imem, ipc_imem_send_mdm_rdy_cb, 0, NULL, 0,
false);
@ -1175,7 +1172,7 @@ void ipc_imem_cleanup(struct iosm_imem *ipc_imem)
if (test_and_clear_bit(FULLY_FUNCTIONAL, &ipc_imem->flag)) {
ipc_mux_deinit(ipc_imem->mux);
ipc_trace_deinit(ipc_imem->trace);
ipc_debugfs_deinit(ipc_imem);
ipc_wwan_deinit(ipc_imem->wwan);
ipc_port_deinit(ipc_imem->ipc_port);
}

View File

@ -341,6 +341,7 @@ enum ipc_phase {
* @ev_mux_net_transmit_pending:0 means inform the IPC tasklet to pass
* @reset_det_n: Reset detect flag
* @pcie_wake_n: Pcie wake flag
* @debugfs_dir: Debug FS directory for driver-specific entries
*/
struct iosm_imem {
struct iosm_mmio *mmio;
@ -350,7 +351,9 @@ struct iosm_imem {
struct iosm_mux *mux;
struct iosm_cdev *ipc_port[IPC_MEM_MAX_CHANNELS];
struct iosm_pcie *pcie;
#ifdef CONFIG_WWAN_DEBUGFS
struct iosm_trace *trace;
#endif
struct device *dev;
enum ipc_mem_device_ipc_state ipc_requested_state;
struct ipc_mem_channel channels[IPC_MEM_MAX_CHANNELS];
@ -380,6 +383,9 @@ struct iosm_imem {
ev_mux_net_transmit_pending:1,
reset_det_n:1,
pcie_wake_n:1;
#ifdef CONFIG_WWAN_DEBUGFS
struct dentry *debugfs_dir;
#endif
};
/**

View File

@ -11,7 +11,6 @@
#include "iosm_ipc_imem_ops.h"
#include "iosm_ipc_port.h"
#include "iosm_ipc_task_queue.h"
#include "iosm_ipc_trace.h"
/* Open a packet data online channel between the network layer and CP. */
int ipc_imem_sys_wwan_open(struct iosm_imem *ipc_imem, int if_id)
@ -108,23 +107,6 @@ void ipc_imem_wwan_channel_init(struct iosm_imem *ipc_imem,
"failed to register the ipc_wwan interfaces");
}
/**
* ipc_imem_trace_channel_init - Initializes trace channel.
* @ipc_imem: Pointer to iosm_imem struct.
*
* Returns: Pointer to trace instance on success else NULL
*/
struct iosm_trace *ipc_imem_trace_channel_init(struct iosm_imem *ipc_imem)
{
struct ipc_chnl_cfg chnl_cfg = { 0 };
ipc_chnl_cfg_get(&chnl_cfg, IPC_MEM_CTRL_CHL_ID_3);
ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL, chnl_cfg,
IRQ_MOD_OFF);
return ipc_trace_init(ipc_imem);
}
/* Map SKB to DMA for transfer */
static int ipc_imem_map_skb_to_dma(struct iosm_imem *ipc_imem,
struct sk_buff *skb)

View File

@ -141,5 +141,5 @@ int ipc_imem_sys_devlink_read(struct iosm_devlink *ipc_devlink, u8 *data,
*/
int ipc_imem_sys_devlink_write(struct iosm_devlink *ipc_devlink,
unsigned char *buf, int count);
struct iosm_trace *ipc_imem_trace_channel_init(struct iosm_imem *ipc_imem);
#endif

View File

@ -17,11 +17,13 @@
/**
* ipc_trace_port_rx - Receive trace packet from cp and write to relay buffer
* @ipc_trace: Pointer to the ipc trace data-struct
* @ipc_imem: Pointer to iosm_imem structure
* @skb: Pointer to struct sk_buff
*/
void ipc_trace_port_rx(struct iosm_trace *ipc_trace, struct sk_buff *skb)
void ipc_trace_port_rx(struct iosm_imem *ipc_imem, struct sk_buff *skb)
{
struct iosm_trace *ipc_trace = ipc_imem->trace;
if (ipc_trace->ipc_rchan)
relay_write(ipc_trace->ipc_rchan, skb->data, skb->len);
@ -132,9 +134,14 @@ static const struct file_operations ipc_trace_fops = {
*/
struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
{
struct iosm_trace *ipc_trace = kzalloc(sizeof(*ipc_trace), GFP_KERNEL);
struct dentry *debugfs_pdev;
struct ipc_chnl_cfg chnl_cfg = { 0 };
struct iosm_trace *ipc_trace;
ipc_chnl_cfg_get(&chnl_cfg, IPC_MEM_CTRL_CHL_ID_3);
ipc_imem_channel_init(ipc_imem, IPC_CTYPE_CTRL, chnl_cfg,
IRQ_MOD_OFF);
ipc_trace = kzalloc(sizeof(*ipc_trace), GFP_KERNEL);
if (!ipc_trace)
return NULL;
@ -144,15 +151,14 @@ struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
ipc_trace->chl_id = IPC_MEM_CTRL_CHL_ID_3;
mutex_init(&ipc_trace->trc_mutex);
debugfs_pdev = wwan_get_debugfs_dir(ipc_imem->dev);
ipc_trace->ctrl_file = debugfs_create_file(IOSM_TRC_DEBUGFS_TRACE_CTRL,
IOSM_TRC_FILE_PERM,
debugfs_pdev,
ipc_imem->debugfs_dir,
ipc_trace, &ipc_trace_fops);
ipc_trace->ipc_rchan = relay_open(IOSM_TRC_DEBUGFS_TRACE,
debugfs_pdev,
ipc_imem->debugfs_dir,
IOSM_TRC_SUB_BUFF_SIZE,
IOSM_TRC_N_SUB_BUFF,
&relay_callbacks, NULL);
@ -166,6 +172,9 @@ struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem)
*/
void ipc_trace_deinit(struct iosm_trace *ipc_trace)
{
if (!ipc_trace)
return;
debugfs_remove(ipc_trace->ctrl_file);
relay_close(ipc_trace->ipc_rchan);
mutex_destroy(&ipc_trace->trc_mutex);

View File

@ -45,7 +45,30 @@ struct iosm_trace {
enum trace_ctrl_mode mode;
};
#ifdef CONFIG_WWAN_DEBUGFS
static inline bool ipc_is_trace_channel(struct iosm_imem *ipc_mem, u16 chl_id)
{
return ipc_mem->trace && ipc_mem->trace->chl_id == chl_id;
}
struct iosm_trace *ipc_trace_init(struct iosm_imem *ipc_imem);
void ipc_trace_deinit(struct iosm_trace *ipc_trace);
void ipc_trace_port_rx(struct iosm_trace *ipc_trace, struct sk_buff *skb);
void ipc_trace_port_rx(struct iosm_imem *ipc_imem, struct sk_buff *skb);
#else
static inline bool ipc_is_trace_channel(struct iosm_imem *ipc_mem, u16 chl_id)
{
return false;
}
static inline void ipc_trace_port_rx(struct iosm_imem *ipc_imem,
struct sk_buff *skb)
{
dev_kfree_skb(skb);
}
#endif
#endif

View File

@ -50,7 +50,9 @@ struct wwan_device {
atomic_t port_id;
const struct wwan_ops *ops;
void *ops_ctxt;
#ifdef CONFIG_WWAN_DEBUGFS
struct dentry *debugfs_dir;
#endif
};
/**
@ -146,6 +148,7 @@ static struct wwan_device *wwan_dev_get_by_name(const char *name)
return to_wwan_dev(dev);
}
#ifdef CONFIG_WWAN_DEBUGFS
struct dentry *wwan_get_debugfs_dir(struct device *parent)
{
struct wwan_device *wwandev;
@ -157,6 +160,7 @@ struct dentry *wwan_get_debugfs_dir(struct device *parent)
return wwandev->debugfs_dir;
}
EXPORT_SYMBOL_GPL(wwan_get_debugfs_dir);
#endif
/* This function allocates and registers a new WWAN device OR if a WWAN device
* already exist for the given parent, it gets a reference and return it.
@ -166,7 +170,6 @@ EXPORT_SYMBOL_GPL(wwan_get_debugfs_dir);
static struct wwan_device *wwan_create_dev(struct device *parent)
{
struct wwan_device *wwandev;
const char *wwandev_name;
int err, id;
/* The 'find-alloc-register' operation must be protected against
@ -206,9 +209,11 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
goto done_unlock;
}
wwandev_name = kobject_name(&wwandev->dev.kobj);
wwandev->debugfs_dir = debugfs_create_dir(wwandev_name,
wwan_debugfs_dir);
#ifdef CONFIG_WWAN_DEBUGFS
wwandev->debugfs_dir =
debugfs_create_dir(kobject_name(&wwandev->dev.kobj),
wwan_debugfs_dir);
#endif
done_unlock:
mutex_unlock(&wwan_register_lock);
@ -240,7 +245,9 @@ static void wwan_remove_dev(struct wwan_device *wwandev)
ret = device_for_each_child(&wwandev->dev, NULL, is_wwan_child);
if (!ret) {
#ifdef CONFIG_WWAN_DEBUGFS
debugfs_remove_recursive(wwandev->debugfs_dir);
#endif
device_unregister(&wwandev->dev);
} else {
put_device(&wwandev->dev);
@ -1140,7 +1147,9 @@ static int __init wwan_init(void)
goto destroy;
}
#ifdef CONFIG_WWAN_DEBUGFS
wwan_debugfs_dir = debugfs_create_dir("wwan", NULL);
#endif
return 0;

View File

@ -171,6 +171,13 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
void wwan_unregister_ops(struct device *parent);
#ifdef CONFIG_WWAN_DEBUGFS
struct dentry *wwan_get_debugfs_dir(struct device *parent);
#else
static inline struct dentry *wwan_get_debugfs_dir(struct device *parent)
{
return ERR_PTR(-ENODEV);
}
#endif
#endif /* __WWAN_H */