LinuxLists.cc - [PATCH v2 00/17] net: introduce Qualcomm IPA driver

[permalink] [raw]

Subject: [PATCH v2 04/17] soc: qcom: ipa: configuration data

This patch defines configuration data that is used to specify some
of the details of IPA hardware supported by the driver. It is built
as Device Tree match data, discovered at boot time. Initially the
driver only supports the Qualcomm SDM845 SoC.

Signed-off-by: Alex Elder <[email protected]>
---
drivers/net/ipa/ipa_data-sdm845.c | 245 +++++++++++++++++++++++++++
drivers/net/ipa/ipa_data.h | 267 ++++++++++++++++++++++++++++++
2 files changed, 512 insertions(+)
create mode 100644 drivers/net/ipa/ipa_data-sdm845.c
create mode 100644 drivers/net/ipa/ipa_data.h

diff --git a/drivers/net/ipa/ipa_data-sdm845.c b/drivers/net/ipa/ipa_data-sdm845.c
new file mode 100644
index 000000000000..62c0f25f5161
--- /dev/null
+++ b/drivers/net/ipa/ipa_data-sdm845.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2019 Linaro Ltd.
+ */
+
+#include <linux/log2.h>
+
+#include "gsi.h"
+#include "ipa_data.h"
+#include "ipa_endpoint.h"
+
+/* Differentiate Boolean from numerical options */
+#define NO 0
+#define YES 1
+
+/* Endpoint configuration for the SDM845 SoC. */
+static const struct gsi_ipa_endpoint_data gsi_ipa_endpoint_data[] = {
+ {
+ .ee_id = GSI_EE_AP,
+ .channel_id = 4,
+ .endpoint_id = IPA_ENDPOINT_AP_COMMAND_TX,
+ .toward_ipa = YES,
+ .channel = {
+ .tlv_count = 20,
+ .wrr_priority = YES,
+ .tre_count = 256,
+ .event_count = 512,
+ },
+ .endpoint = {
+ .seq_type = IPA_SEQ_DMA_ONLY,
+ .config = {
+ .dma_mode = YES,
+ .dma_endpoint = IPA_ENDPOINT_AP_LAN_RX,
+ },
+ },
+ },
+ {
+ .ee_id = GSI_EE_AP,
+ .channel_id = 5,
+ .endpoint_id = IPA_ENDPOINT_AP_LAN_RX,
+ .toward_ipa = NO,
+ .channel = {
+ .tlv_count = 8,
+ .tre_count = 256,
+ .event_count = 256,
+ },
+ .endpoint = {
+ .seq_type = IPA_SEQ_INVALID,
+ .config = {
+ .checksum = YES,
+ .aggregation = YES,
+ .status_enable = YES,
+ .rx = {
+ .pad_align = ilog2(sizeof(u32)),
+ },
+ },
+ },
+ },
+ {
+ .ee_id = GSI_EE_AP,
+ .channel_id = 3,
+ .endpoint_id = IPA_ENDPOINT_AP_MODEM_TX,
+ .toward_ipa = YES,
+ .channel = {
+ .tlv_count = 16,
+ .tre_count = 512,
+ .event_count = 512,
+ },
+ .endpoint = {
+ .support_flt = YES,
+ .seq_type =
+ IPA_SEQ_2ND_PKT_PROCESS_PASS_NO_DEC_UCP,
+ .config = {
+ .checksum = YES,
+ .qmap = YES,
+ .status_enable = YES,
+ .tx = {
+ .delay = YES,
+ .status_endpoint =
+ IPA_ENDPOINT_MODEM_AP_RX,
+ },
+ },
+ },
+ },
+ {
+ .ee_id = GSI_EE_AP,
+ .channel_id = 6,
+ .endpoint_id = IPA_ENDPOINT_AP_MODEM_RX,
+ .toward_ipa = NO,
+ .channel = {
+ .tlv_count = 8,
+ .tre_count = 256,
+ .event_count = 256,
+ },
+ .endpoint = {
+ .seq_type = IPA_SEQ_INVALID,
+ .config = {
+ .checksum = YES,
+ .qmap = YES,
+ .aggregation = YES,
+ .rx = {
+ .aggr_close_eof = YES,
+ },
+ },
+ },
+ },
+ {
+ .ee_id = GSI_EE_MODEM,
+ .channel_id = 1,
+ .endpoint_id = IPA_ENDPOINT_MODEM_COMMAND_TX,
+ .toward_ipa = YES,
+ .endpoint = {
+ .seq_type = IPA_SEQ_PKT_PROCESS_NO_DEC_UCP,
+ },
+ },
+ {
+ .ee_id = GSI_EE_MODEM,
+ .channel_id = 0,
+ .endpoint_id = IPA_ENDPOINT_MODEM_LAN_TX,
+ .toward_ipa = YES,
+ .endpoint = {
+ .support_flt = YES,
+ },
+ },
+ {
+ .ee_id = GSI_EE_MODEM,
+ .channel_id = 3,
+ .endpoint_id = IPA_ENDPOINT_MODEM_LAN_RX,
+ .toward_ipa = NO,
+ },
+ {
+ .ee_id = GSI_EE_MODEM,
+ .channel_id = 4,
+ .endpoint_id = IPA_ENDPOINT_MODEM_AP_TX,
+ .toward_ipa = YES,
+ .endpoint = {
+ .support_flt = YES,
+ },
+ },
+ {
+ .ee_id = GSI_EE_MODEM,
+ .channel_id = 2,
+ .endpoint_id = IPA_ENDPOINT_MODEM_AP_RX,
+ .toward_ipa = NO,
+ },
+};
+
+static const struct ipa_resource_src ipa_resource_src[] = {
+ {
+ .type = IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 1,
+ .max = 63,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 1,
+ .max = 63,
+ },
+ },
+ {
+ .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 10,
+ .max = 10,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 10,
+ .max = 10,
+ },
+ },
+ {
+ .type = IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 12,
+ .max = 12,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 14,
+ .max = 14,
+ },
+ },
+ {
+ .type = IPA_RESOURCE_TYPE_SRC_HPS_DMARS,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 0,
+ .max = 63,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 0,
+ .max = 63,
+ },
+ },
+ {
+ .type = IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 14,
+ .max = 14,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 20,
+ .max = 20,
+ },
+ },
+};
+
+static const struct ipa_resource_dst ipa_resource_dst[] = {
+ {
+ .type = IPA_RESOURCE_TYPE_DST_DATA_SECTORS,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 4,
+ .max = 4,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 4,
+ .max = 4,
+ },
+ },
+ {
+ .type = IPA_RESOURCE_TYPE_DST_DPS_DMARS,
+ .limits[IPA_RESOURCE_GROUP_LWA_DL] = {
+ .min = 2,
+ .max = 63,
+ },
+ .limits[IPA_RESOURCE_GROUP_UL_DL] = {
+ .min = 1,
+ .max = 63,
+ },
+ },
+};
+
+/* Resource configuration for the SDM845 SoC. */
+static const struct ipa_resource_data ipa_resource_data = {
+ .resource_src = ipa_resource_src,
+ .resource_src_count = ARRAY_SIZE(ipa_resource_src),
+ .resource_dst = ipa_resource_dst,
+ .resource_dst_count = ARRAY_SIZE(ipa_resource_dst),
+};
+
+/* Configuration data for the SDM845 SoC. */
+const struct ipa_data ipa_data_sdm845 = {
+ .endpoint_data = gsi_ipa_endpoint_data,
+ .endpoint_data_count = ARRAY_SIZE(gsi_ipa_endpoint_data),
+ .resource_data = &ipa_resource_data,
+};
diff --git a/drivers/net/ipa/ipa_data.h b/drivers/net/ipa/ipa_data.h
new file mode 100644
index 000000000000..f7669f73efc3
--- /dev/null
+++ b/drivers/net/ipa/ipa_data.h
@@ -0,0 +1,267 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/* Copyright (c) 2012-2018, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2019 Linaro Ltd.
+ */
+#ifndef _IPA_DATA_H_
+#define _IPA_DATA_H_
+
+#include <linux/types.h>
+
+#include "ipa_endpoint.h"
+
+/**
+ * DOC: IPA/GSI Configuration Data
+ *
+ * Boot-time configuration data is used to define the configuration of the
+ * IPA and GSI resources to use for a given platform. This data is supplied
+ * via the Device Tree match table, associated with a particular compatible
+ * string. The data defines information about resources, endpoints, and
+ * channels. For endpoints and channels, the configuration data defines how
+ * these hardware entities are initially configured, but in almost all cases,
+ * this configuration never changes.
+ *
+ * Resources are data structures used internally by the IPA hardware. The
+ * configuration data defines the number (or limits of the number) of various
+ * types of these resources.
+ *
+ * Endpoint configuration data defines properties of both IPA endpoints and
+ * GSI channels. A channel is a GSI construct, and represents a single
+ * communication path between the IPA and a particular execution environment
+ * (EE), such as the AP or Modem. Each EE has a set of channels associated
+ * with it, and each channel has an ID unique for that EE. Only GSI channels
+ * associated with the AP are of concern to this driver.
+ *
+ * An endpoint is an IPA construct representing a single channel anywhere
+ * within the system. As such, an IPA endpoint ID maps directly to an
+ * (EE, channel_id) pair. Generally, this driver is concerned with only
+ * endpoints associated with the AP, however this will change when support
+ * for routing (etc.) is added. IPA endpoint and GSI channel configuration
+ * data are defined together, establishing the endpoint_id->(EE, channel_id)
+ * mapping.
+ *
+ * Endpoint configuration data consists of three parts: properties that
+ * are common to IPA and GSI (EE ID, channel ID, endpoint ID, and direction);
+ * properties associated with the GSI channel; and properties associated with
+ * the IPA endpoint.
+ */
+
+/**
+ * struct gsi_channel_data - GSI channel configuration data
+ * @tlv_count: number of entries in channel's TLV FIFO
+ * @wrr_priority: whether channel gets priority (AP command TX only)
+ * @tre_count: number of TREs in the channel ring
+ * @event_count: number of slots in the associated event ring
+ *
+ * A GSI channel is a unidirectional means of transferring data to or from
+ * (and through) the IPA. A GSI channel has a fixed number of "transfer
+ * elements" (TREs) that specify individual commands. A set of commands
+ * are provided to a GSI channel, and when they complete the GSI generates
+ * an event (and an interrupt) to signal their completion. These event
+ * structures are managed in a fixed-size event ring.
+ *
+ * Each GSI channel is fed by a FIFO if type/length/value (TLV) structures,
+ * and the number of entries in this FIFO limits the number of TREs that can
+ * be included in a single transaction.
+ *
+ * The GSI does weighted round-robin servicing of its channels, and it's
+ * possible to adjust a channel's priority of service. Only the AP command
+ * TX channel specifies that it should get priority.
+ */
+struct gsi_channel_data {
+ u32 tlv_count;
+
+ u32 wrr_priority;
+ u32 tre_count;
+ u32 event_count;
+};
+
+/**
+ * struct ipa_endpoint_tx_data - configuration data for TX endpoints
+ * @delay: whether endpoint starts in delay mode
+ * @status_endpoint: endpoint to which status elements are sent
+ *
+ * Delay mode prevents an endpoint from transmitting anything, even if
+ * commands have been presented to the hardware. Once the endpoint exits
+ * delay mode, queued transfer commands are sent.
+ *
+ * The @status_endpoint is only valid if the endpoint's @status_enable
+ * flag is set.
+ */
+struct ipa_endpoint_tx_data {
+ u32 delay;
+ enum ipa_endpoint_id status_endpoint;
+};
+
+/**
+ * struct ipa_endpoint_rx_data - configuration data for RX endpoints
+ * @pad_align: power-of-2 boundary to which packet payload is aligned
+ * @aggr_close_eof: whether aggregation closes on end-of-frame
+ *
+ * With each packet it transfers, the IPA hardware can perform certain
+ * transformations of its packet data. One of these is adding pad bytes
+ * to the end of the packet data so the result ends on a power-of-2 boundary.
+ *
+ * It is also able to aggregate multiple packets into a single receive buffer.
+ * Aggregation is "open" while a buffer is being filled, and "closes" when
+ * certain criteria are met. One of those criteria is the sender indicating
+ * a "frame" consisting of several transfers has ended.
+ */
+struct ipa_endpoint_rx_data {
+ u32 pad_align;
+ u32 aggr_close_eof;
+};
+
+/**
+ * struct ipa_endpoint_config_data - IPA endpoint hardware configuration
+ * @checksum: whether checksum offload is enabled
+ * @qmap: whether endpoint uses QMAP protocol
+ * @aggregation: whether endpoint supports aggregation
+ * @dma_mode: whether endpoint operates in DMA mode
+ * @dma_endpoint: peer endpoint, if operating in DMA mode
+ * @status_enable: whether status elements are generated for endpoint
+ * @tx: TX-specific endpoint information (see above)
+ * @rx: RX-specific endpoint information (see above)
+ */
+struct ipa_endpoint_config_data {
+ u32 checksum;
+ u32 qmap;
+ u32 aggregation;
+ u32 dma_mode;
+ enum ipa_endpoint_id dma_endpoint;
+ u32 status_enable;
+ union {
+ struct ipa_endpoint_tx_data tx;
+ struct ipa_endpoint_rx_data rx;
+ };
+};
+
+/**
+ * struct ipa_endpoint_data - IPA endpoint configuration data
+ * @support_flt: whether endpoint supports filtering
+ * @seq_type: hardware sequencer type used for endpoint
+ * @config: hardware configuration (see above)
+ *
+ * Not all endpoints support the IPA filtering capability. A filter table
+ * defines the filters to apply for those endpoints that support it. The
+ * AP is responsible for initializing this table, and it must include entries
+ * for non-AP endpoints. For this reason we define *all* endpoints used
+ * in the system, and indicate whether they support filtering.
+ *
+ * The remaining endpoint configuration data applies only to AP endpoints.
+ * The IPA hardware is implemented by sequencers, and the AP must program
+ * the type(s) of these sequencers at initialization time. The remaining
+ * endpoint configuration data is defined above.
+ */
+struct ipa_endpoint_data {
+ u32 support_flt;
+ /* The rest are specified only for AP endpoints */
+ enum ipa_seq_type seq_type;
+ struct ipa_endpoint_config_data config;
+};
+
+/**
+ * struct gsi_ipa_endpoint_data - GSI channel/IPA endpoint data
+ * ee: GSI execution environment ID
+ * channel_id: GSI channel ID
+ * endpoint_id: IPA endpoint ID
+ * toward_ipa: direction of data transfer
+ * gsi: GSI channel configuration data (see above)
+ * ipa: IPA endpoint configuration data (see above)
+ */
+struct gsi_ipa_endpoint_data {
+ u32 ee_id;
+ u32 channel_id;
+ enum ipa_endpoint_id endpoint_id;
+ u32 toward_ipa;
+
+ struct gsi_channel_data channel;
+ struct ipa_endpoint_data endpoint;
+};
+
+/** enum ipa_resource_group - IPA resource group */
+enum ipa_resource_group {
+ IPA_RESOURCE_GROUP_LWA_DL, /* currently not used */
+ IPA_RESOURCE_GROUP_UL_DL,
+ IPA_RESOURCE_GROUP_MAX,
+};
+
+/** enum ipa_resource_type_src - source resource types */
+enum ipa_resource_type_src {
+ IPA_RESOURCE_TYPE_SRC_PKT_CONTEXTS,
+ IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_LISTS,
+ IPA_RESOURCE_TYPE_SRC_DESCRIPTOR_BUFF,
+ IPA_RESOURCE_TYPE_SRC_HPS_DMARS,
+ IPA_RESOURCE_TYPE_SRC_ACK_ENTRIES,
+};
+
+/** enum ipa_resource_type_dst - destination resource types */
+enum ipa_resource_type_dst {
+ IPA_RESOURCE_TYPE_DST_DATA_SECTORS,
+ IPA_RESOURCE_TYPE_DST_DPS_DMARS,
+};
+
+/**
+ * struct ipa_resource_limits - minimum and maximum resource counts
+ * @min: minimum number of resources of a given type
+ * @max: maximum number of resources of a given type
+ */
+struct ipa_resource_limits {
+ u32 min;
+ u32 max;
+};
+
+/**
+ * struct ipa_resource_src - source endpoint group resource usage
+ * @type: source group resource type
+ * @limits: array of limits to use for each resource group
+ */
+struct ipa_resource_src {
+ enum ipa_resource_type_src type;
+ struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_MAX];
+};
+
+/**
+ * struct ipa_resource_dst - destination endpoint group resource usage
+ * @type: destination group resource type
+ * @limits: array of limits to use for each resource group
+ */
+struct ipa_resource_dst {
+ enum ipa_resource_type_dst type;
+ struct ipa_resource_limits limits[IPA_RESOURCE_GROUP_MAX];
+};
+
+/**
+ * struct ipa_resource_data - IPA resource configuration data
+ * @resource_src: source endpoint group resources
+ * @resource_src_count: number of entries in the resource_src array
+ * @resource_dst: destination endpoint group resources
+ * @resource_dst_count: number of entries in the resource_dst array
+ *
+ * In order to manage quality of service between endpoints, certain resources
+ * required for operation are allocated to groups of endpoints. Generally
+ * this information is invisible to the AP, but the AP is responsible for
+ * programming it at initialization time, so we specify it here.
+ */
+struct ipa_resource_data {
+ const struct ipa_resource_src *resource_src;
+ u32 resource_src_count;
+ const struct ipa_resource_dst *resource_dst;
+ u32 resource_dst_count;
+};
+
+/**
+ * struct ipa_data - combined IPA/GSI configuration data
+ * @resource_data: IPA resource configuration data
+ * @endpoint_data: IPA endpoint/GSI channel data
+ * @endpoint_data_count: number of entries in endpoint_data array
+ */
+struct ipa_data {
+ const struct ipa_resource_data *resource_data;
+ const struct gsi_ipa_endpoint_data *endpoint_data;
+ u32 endpoint_data_count; /* # entries in endpoint_data[] */
+};
+
+extern const struct ipa_data ipa_data_sdm845;
+
+#endif /* _IPA_DATA_H_ */
--
2.20.1

2019-05-31 03:59:29

[permalink] [raw]

Subject: [PATCH v2 02/17] dt-bindings: soc: qcom: add IPA bindings

Add the binding definitions for the "qcom,ipa" device tree node.

Signed-off-by: Alex Elder <[email protected]>
---
.../devicetree/bindings/net/qcom,ipa.yaml | 180 ++++++++++++++++++
1 file changed, 180 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/qcom,ipa.yaml

diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
new file mode 100644
index 000000000000..0037fc278a61
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
@@ -0,0 +1,180 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/qcom,ipa.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm IP Accelerator (IPA)
+
+maintainers:
+ - Alex Elder <[email protected]>
+
+description:
+ This binding describes the Qualcomm IPA. The IPA is capable of offloading
+ certain network processing tasks (e.g. filtering, routing, and NAT) from
+ the main processor.
+
+ The IPA sits between multiple independent "execution environments,"
+ including the Application Processor (AP) and the modem. The IPA presents
+ a Generic Software Interface (GSI) to each execution environment.
+ The GSI is an integral part of the IPA, but it is logically isolated
+ and has a distinct interrupt and a separately-defined address space.
+
+ See also soc/qcom/qcom,smp2p.txt and interconnect/interconnect.txt.
+
+ - |
+ -------- ---------
+ | | | |
+ | AP +<---. .----+ Modem |
+ | +--. | | .->+ |
+ | | | | | | | |
+ -------- | | | | ---------
+ v | v |
+ --+-+---+-+--
+ | GSI |
+ |-----------|
+ | |
+ | IPA |
+ | |
+ -------------
+
+properties:
+ compatible:
+ const: "qcom,sdm845-ipa"
+
+ reg:
+ items:
+ - description: IPA registers
+ - description: IPA shared memory
+ - description: GSI registers
+
+ reg-names:
+ items:
+ - const: ipa-reg
+ - const: ipa-shared
+ - const: gsi
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ const: core
+
+ interrupts:
+ items:
+ - description: IPA interrupt (hardware IRQ)
+ - description: GSI interrupt (hardware IRQ)
+ - description: Modem clock query interrupt (smp2p interrupt)
+ - description: Modem setup ready interrupt (smp2p interrupt)
+
+ interrupt-names:
+ items:
+ - const: ipa
+ - const: gsi
+ - const: ipa-clock-query
+ - const: ipa-setup-ready
+
+ interconnects:
+ items:
+ - description: Interconnect path between IPA and main memory
+ - description: Interconnect path between IPA and internal memory
+ - description: Interconnect path between IPA and the AP subsystem
+
+ interconnect-names:
+ items:
+ - const: memory
+ - const: imem
+ - const: config
+
+ qcom,smem-states:
+ description: State bits used in by the AP to signal the modem.
+ items:
+ - description: Whether the "ipa-clock-enabled" state bit is valid
+ - description: Whether the IPA clock is enabled (if valid)
+
+ qcom,smem-state-names:
+ description: The names of the state bits used for SMP2P output
+ items:
+ - const: ipa-clock-enabled-valid
+ - const: ipa-clock-enabled
+
+ modem-init:
+ type: boolean
+ description:
+ If present, it indicates that the modem is responsible for
+ performing early IPA initialization, including loading and
+ validating firwmare used by the GSI.
+
+ memory-region:
+ maxItems: 1
+ description:
+ If present, a phandle for a reserved memory area that holds
+ the firmware passed to Trust Zone for authentication. Required
+ when Trust Zone (not the modem) performs early initialization.
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - interrupts
+ - interconnects
+ - qcom,smem-states
+
+oneOf:
+ - required:
+ - modem-init
+ - required:
+ - memory-region
+
+examples:
+ - |
+ smp2p-mpss {
+ compatible = "qcom,smp2p";
+ ipa_smp2p_out: ipa-ap-to-modem {
+ qcom,entry-name = "ipa";
+ #qcom,smem-state-cells = <1>;
+ };
+
+ ipa_smp2p_in: ipa-modem-to-ap {
+ qcom,entry-name = "ipa";
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+ };
+ ipa@1e40000 {
+ compatible = "qcom,sdm845-ipa";
+
+ modem-init;
+
+ reg = <0 0x1e40000 0 0x7000>,
+ <0 0x1e47000 0 0x2000>,
+ <0 0x1e04000 0 0x2c000>;
+ reg-names = "ipa-reg",
+ "ipa-shared";
+ "gsi";
+
+ interrupts-extended = <&intc 0 311 IRQ_TYPE_EDGE_RISING>,
+ <&intc 0 432 IRQ_TYPE_LEVEL_HIGH>,
+ <&ipa_smp2p_in 0 IRQ_TYPE_EDGE_RISING>,
+ <&ipa_smp2p_in 1 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "ipa",
+ "gsi",
+ "ipa-clock-query",
+ "ipa-setup-ready";
+
+ clocks = <&rpmhcc RPMH_IPA_CLK>;
+ clock-names = "core";
+
+ interconnects =
+ <&rsc_hlos MASTER_IPA &rsc_hlos SLAVE_EBI1>,
+ <&rsc_hlos MASTER_IPA &rsc_hlos SLAVE_IMEM>,
+ <&rsc_hlos MASTER_APPSS_PROC &rsc_hlos SLAVE_IPA_CFG>;
+ interconnect-names = "memory",
+ "imem",
+ "config";
+
+ qcom,smem-states = <&ipa_smp2p_out 0>,
+ <&ipa_smp2p_out 1>;
+ qcom,smem-state-names = "ipa-clock-enabled-valid",
+ "ipa-clock-enabled";
+ };
--
2.20.1

2019-05-31 15:02:23

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
> This series presents the driver for the Qualcomm IP Accelerator
> (IPA).
>
> This is version 2 of the series. This version has addressed almost
> all of the feedback received in the first version:
>
> https://lore.kernel.org/lkml/[email protected]/
> More detail is included in the individual patches, but here is a
> high-level summary of what's changed since then:
> - Two spinlocks have been removed.
> - The code for enabling and disabling endpoint interrupts has
> been simplified considerably, and the spinlock is no longer
> required
> - A spinlock used when updating ring buffer pointers is no
> longer needed. Integers indexing the ring are used instead
> (and they don't even have to be atomic).
> - One spinlock remains to protect list updates, but it is always
> acquired using spin_lock_bh() (no more irqsave).
> - Information about the queueing and completion of messages is now
> supplied to the network stack in batches rather than one at a
> time.
> - I/O completion handling has been simplified, with the IRQ
> handler now consisting mainly of disabling the interrupt and
> calling napi_schedule().
> - Some comments have been updated and improved througout.
>
> What follows is the introduction supplied with v1 of the series.
>
> -----
>
> The IPA is a component present in some Qualcomm SoCs that allows
> network functions such as aggregation, filtering, routing, and NAT
> to be performed without active involvement of the main application
> processor (AP).
>
> Initially, these advanced features are disabled; the IPA driver
> simply provides a network interface that makes the modem's LTE
> network available to the AP. In addition, only support for the
> IPA found in the Qualcomm SDM845 SoC is provided.

My question from the Nov 2018 IPA rmnet driver still stands; how does
this relate to net/ethernet/qualcomm/rmnet/ if at all? And if this is
really just a netdev talking to the IPA itself and unrelated to
net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop cargo-
culting rmnet around just because it happens to be a net driver for a
QC SoC.

Is the firmware that the driver loads already in linux-firmware or
going to be there soon?

How does the driver support multiple PDNs (eg PDP or EPS contexts) that
are enabled through the control plane via QMI messages? I couldn't
quite find that out.

Thanks,
Dan

> This code is derived from a driver developed internally by Qualcomm.
> A version of the original source can be seen here:
> https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree
> in the "drivers/platform/msm/ipa" directory. Many were involved in
> developing this, but the following individuals deserve explicit
> acknowledgement for their substantial contributions:
>
> Abhishek Choubey
> Ady Abraham
> Chaitanya Pratapa
> David Arinzon
> Ghanim Fodi
> Gidon Studinski
> Ravi Gummadidala
> Shihuan Liu
> Skylar Chang
>
> A version of this code was posted in November 2018 as an RFC.
>
> https://lore.kernel.org/lkml/[email protected]/
> All feedback received was addressed. The code has undergone
> considerable further rework since that time, and most of the
> "future work" described then has now been completed.
>
> This code is available in buildable form here, based on kernel
> v5.2-rc1:
> remote: ssh://[email protected]/people/alex.elder/linux.git
> branch: ipa-v2_kernel-v5.2-rc2
> 75adf2ac1266 arm64: defconfig: enable build of IPA code
>
> The branch depends on a commit now found in in net-next. It has
> been cherry-picked, and (in this branch) has this commit ID:
> 13c627b5a078 net: qualcomm: rmnet: Move common struct definitions
> to include
> by
>
> -Alex
>
> Alex Elder (17):
> bitfield.h: add FIELD_MAX() and field_max()
> dt-bindings: soc: qcom: add IPA bindings
> soc: qcom: ipa: main code
> soc: qcom: ipa: configuration data
> soc: qcom: ipa: clocking, interrupts, and memory
> soc: qcom: ipa: GSI headers
> soc: qcom: ipa: the generic software interface
> soc: qcom: ipa: GSI transactions
> soc: qcom: ipa: IPA interface to GSI
> soc: qcom: ipa: IPA endpoints
> soc: qcom: ipa: immediate commands
> soc: qcom: ipa: IPA network device and microcontroller
> soc: qcom: ipa: AP/modem communications
> soc: qcom: ipa: support build of IPA code
> MAINTAINERS: add entry for the Qualcomm IPA driver
> arm64: dts: sdm845: add IPA information
> arm64: defconfig: enable build of IPA code
>
> .../devicetree/bindings/net/qcom,ipa.yaml | 180 ++
> MAINTAINERS | 6 +
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 51 +
> arch/arm64/configs/defconfig | 1 +
> drivers/net/Kconfig | 2 +
> drivers/net/Makefile | 1 +
> drivers/net/ipa/Kconfig | 16 +
> drivers/net/ipa/Makefile | 7 +
> drivers/net/ipa/gsi.c | 1635
> +++++++++++++++++
> drivers/net/ipa/gsi.h | 246 +++
> drivers/net/ipa/gsi_private.h | 148 ++
> drivers/net/ipa/gsi_reg.h | 376 ++++
> drivers/net/ipa/gsi_trans.c | 624 +++++++
> drivers/net/ipa/gsi_trans.h | 116 ++
> drivers/net/ipa/ipa.h | 131 ++
> drivers/net/ipa/ipa_clock.c | 297 +++
> drivers/net/ipa/ipa_clock.h | 52 +
> drivers/net/ipa/ipa_cmd.c | 377 ++++
> drivers/net/ipa/ipa_cmd.h | 116 ++
> drivers/net/ipa/ipa_data-sdm845.c | 245 +++
> drivers/net/ipa/ipa_data.h | 267 +++
> drivers/net/ipa/ipa_endpoint.c | 1283 +++++++++++++
> drivers/net/ipa/ipa_endpoint.h | 97 +
> drivers/net/ipa/ipa_gsi.c | 48 +
> drivers/net/ipa/ipa_gsi.h | 49 +
> drivers/net/ipa/ipa_interrupt.c | 279 +++
> drivers/net/ipa/ipa_interrupt.h | 53 +
> drivers/net/ipa/ipa_main.c | 921 ++++++++++
> drivers/net/ipa/ipa_mem.c | 234 +++
> drivers/net/ipa/ipa_mem.h | 83 +
> drivers/net/ipa/ipa_netdev.c | 251 +++
> drivers/net/ipa/ipa_netdev.h | 24 +
> drivers/net/ipa/ipa_qmi.c | 402 ++++
> drivers/net/ipa/ipa_qmi.h | 35 +
> drivers/net/ipa/ipa_qmi_msg.c | 583 ++++++
> drivers/net/ipa/ipa_qmi_msg.h | 238 +++
> drivers/net/ipa/ipa_reg.h | 279 +++
> drivers/net/ipa/ipa_smp2p.c | 304 +++
> drivers/net/ipa/ipa_smp2p.h | 47 +
> drivers/net/ipa/ipa_uc.c | 208 +++
> drivers/net/ipa/ipa_uc.h | 32 +
> include/linux/bitfield.h | 14 +
> 42 files changed, 10358 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/net/qcom,ipa.yaml
> create mode 100644 drivers/net/ipa/Kconfig
> create mode 100644 drivers/net/ipa/Makefile
> create mode 100644 drivers/net/ipa/gsi.c
> create mode 100644 drivers/net/ipa/gsi.h
> create mode 100644 drivers/net/ipa/gsi_private.h
> create mode 100644 drivers/net/ipa/gsi_reg.h
> create mode 100644 drivers/net/ipa/gsi_trans.c
> create mode 100644 drivers/net/ipa/gsi_trans.h
> create mode 100644 drivers/net/ipa/ipa.h
> create mode 100644 drivers/net/ipa/ipa_clock.c
> create mode 100644 drivers/net/ipa/ipa_clock.h
> create mode 100644 drivers/net/ipa/ipa_cmd.c
> create mode 100644 drivers/net/ipa/ipa_cmd.h
> create mode 100644 drivers/net/ipa/ipa_data-sdm845.c
> create mode 100644 drivers/net/ipa/ipa_data.h
> create mode 100644 drivers/net/ipa/ipa_endpoint.c
> create mode 100644 drivers/net/ipa/ipa_endpoint.h
> create mode 100644 drivers/net/ipa/ipa_gsi.c
> create mode 100644 drivers/net/ipa/ipa_gsi.h
> create mode 100644 drivers/net/ipa/ipa_interrupt.c
> create mode 100644 drivers/net/ipa/ipa_interrupt.h
> create mode 100644 drivers/net/ipa/ipa_main.c
> create mode 100644 drivers/net/ipa/ipa_mem.c
> create mode 100644 drivers/net/ipa/ipa_mem.h
> create mode 100644 drivers/net/ipa/ipa_netdev.c
> create mode 100644 drivers/net/ipa/ipa_netdev.h
> create mode 100644 drivers/net/ipa/ipa_qmi.c
> create mode 100644 drivers/net/ipa/ipa_qmi.h
> create mode 100644 drivers/net/ipa/ipa_qmi_msg.c
> create mode 100644 drivers/net/ipa/ipa_qmi_msg.h
> create mode 100644 drivers/net/ipa/ipa_reg.h
> create mode 100644 drivers/net/ipa/ipa_smp2p.c
> create mode 100644 drivers/net/ipa/ipa_smp2p.h
> create mode 100644 drivers/net/ipa/ipa_uc.c
> create mode 100644 drivers/net/ipa/ipa_uc.h
>

2019-05-31 16:39:40

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 5/31/19 9:58 AM, Dan Williams wrote:
> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
>> This series presents the driver for the Qualcomm IP Accelerator
>> (IPA).
>>
>> This is version 2 of the series. This version has addressed almost
>> all of the feedback received in the first version:
>>
>> https://lore.kernel.org/lkml/[email protected]/
>> More detail is included in the individual patches, but here is a
>> high-level summary of what's changed since then:
>> - Two spinlocks have been removed.
>> - The code for enabling and disabling endpoint interrupts has
>> been simplified considerably, and the spinlock is no longer
>> required
>> - A spinlock used when updating ring buffer pointers is no
>> longer needed. Integers indexing the ring are used instead
>> (and they don't even have to be atomic).
>> - One spinlock remains to protect list updates, but it is always
>> acquired using spin_lock_bh() (no more irqsave).
>> - Information about the queueing and completion of messages is now
>> supplied to the network stack in batches rather than one at a
>> time.
>> - I/O completion handling has been simplified, with the IRQ
>> handler now consisting mainly of disabling the interrupt and
>> calling napi_schedule().
>> - Some comments have been updated and improved througout.
>>
>> What follows is the introduction supplied with v1 of the series.
>>
>> -----
>>
>> The IPA is a component present in some Qualcomm SoCs that allows
>> network functions such as aggregation, filtering, routing, and NAT
>> to be performed without active involvement of the main application
>> processor (AP).
>>
>> Initially, these advanced features are disabled; the IPA driver
>> simply provides a network interface that makes the modem's LTE
>> network available to the AP. In addition, only support for the
>> IPA found in the Qualcomm SDM845 SoC is provided.
>
> My question from the Nov 2018 IPA rmnet driver still stands; how does
> this relate to net/ethernet/qualcomm/rmnet/ if at all? And if this is
> really just a netdev talking to the IPA itself and unrelated to
> net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop cargo-
> culting rmnet around just because it happens to be a net driver for a
> QC SoC.

First, the relationship between the IPA driver and the rmnet driver
is that the IPA driver is assumed to sit between the rmnet driver
and the hardware.

Currently the modem is assumed to use QMAP protocol. This means
each packet is prefixed by a (struct rmnet_map_header) structure
that allows the IPA connection to be multiplexed for several logical
connections. The rmnet driver parses such messages and implements
the multiplexed network interfaces.

QMAP protocol can also be used for aggregating many small packets
into a larger message. The rmnet driver implements de-aggregation
of such messages (and could probably aggregate them for TX as well).

Finally, the IPA can support checksum offload, and the rmnet
driver handles providing a prepended header (for TX) and
interpreting the appended trailer (for RX) if these features
are enabled.

So basically, the purpose of the rmnet driver is to handle QMAP
protocol connections, and right now that's what the modem
provides.

> Is the firmware that the driver loads already in linux-firmware or
> going to be there soon?

It is not right now, and I have no information on when it can be
available. The AP *can* load the firmware but right now we rely
on the modem doing it (until we can make the firmware available).

> How does the driver support multiple PDNs (eg PDP or EPS contexts) that
> are enabled through the control plane via QMI messages? I couldn't
> quite find that out.

To be honest, I don't know the answer to this. All of my work
has been on this transport driver and I believe these things
are handled by user space. But I really don't know details.

-Alex

> Thanks,
> Dan
>
>> This code is derived from a driver developed internally by Qualcomm.
>> A version of the original source can be seen here:
>> https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree
>> in the "drivers/platform/msm/ipa" directory. Many were involved in
>> developing this, but the following individuals deserve explicit
>> acknowledgement for their substantial contributions:
>>
>> Abhishek Choubey
>> Ady Abraham
>> Chaitanya Pratapa
>> David Arinzon
>> Ghanim Fodi
>> Gidon Studinski
>> Ravi Gummadidala
>> Shihuan Liu
>> Skylar Chang
>>
>> A version of this code was posted in November 2018 as an RFC.
>>
>> https://lore.kernel.org/lkml/[email protected]/
>> All feedback received was addressed. The code has undergone
>> considerable further rework since that time, and most of the
>> "future work" described then has now been completed.
>>
>> This code is available in buildable form here, based on kernel
>> v5.2-rc1:
>> remote: ssh://[email protected]/people/alex.elder/linux.git
>> branch: ipa-v2_kernel-v5.2-rc2
>> 75adf2ac1266 arm64: defconfig: enable build of IPA code
>>
>> The branch depends on a commit now found in in net-next. It has
>> been cherry-picked, and (in this branch) has this commit ID:
>> 13c627b5a078 net: qualcomm: rmnet: Move common struct definitions
>> to include
>> by
>>
>> -Alex
>>
>> Alex Elder (17):
>> bitfield.h: add FIELD_MAX() and field_max()
>> dt-bindings: soc: qcom: add IPA bindings
>> soc: qcom: ipa: main code
>> soc: qcom: ipa: configuration data
>> soc: qcom: ipa: clocking, interrupts, and memory
>> soc: qcom: ipa: GSI headers
>> soc: qcom: ipa: the generic software interface
>> soc: qcom: ipa: GSI transactions
>> soc: qcom: ipa: IPA interface to GSI
>> soc: qcom: ipa: IPA endpoints
>> soc: qcom: ipa: immediate commands
>> soc: qcom: ipa: IPA network device and microcontroller
>> soc: qcom: ipa: AP/modem communications
>> soc: qcom: ipa: support build of IPA code
>> MAINTAINERS: add entry for the Qualcomm IPA driver
>> arm64: dts: sdm845: add IPA information
>> arm64: defconfig: enable build of IPA code
>>
>> .../devicetree/bindings/net/qcom,ipa.yaml | 180 ++
>> MAINTAINERS | 6 +
>> arch/arm64/boot/dts/qcom/sdm845.dtsi | 51 +
>> arch/arm64/configs/defconfig | 1 +
>> drivers/net/Kconfig | 2 +
>> drivers/net/Makefile | 1 +
>> drivers/net/ipa/Kconfig | 16 +
>> drivers/net/ipa/Makefile | 7 +
>> drivers/net/ipa/gsi.c | 1635
>> +++++++++++++++++
>> drivers/net/ipa/gsi.h | 246 +++
>> drivers/net/ipa/gsi_private.h | 148 ++
>> drivers/net/ipa/gsi_reg.h | 376 ++++
>> drivers/net/ipa/gsi_trans.c | 624 +++++++
>> drivers/net/ipa/gsi_trans.h | 116 ++
>> drivers/net/ipa/ipa.h | 131 ++
>> drivers/net/ipa/ipa_clock.c | 297 +++
>> drivers/net/ipa/ipa_clock.h | 52 +
>> drivers/net/ipa/ipa_cmd.c | 377 ++++
>> drivers/net/ipa/ipa_cmd.h | 116 ++
>> drivers/net/ipa/ipa_data-sdm845.c | 245 +++
>> drivers/net/ipa/ipa_data.h | 267 +++
>> drivers/net/ipa/ipa_endpoint.c | 1283 +++++++++++++
>> drivers/net/ipa/ipa_endpoint.h | 97 +
>> drivers/net/ipa/ipa_gsi.c | 48 +
>> drivers/net/ipa/ipa_gsi.h | 49 +
>> drivers/net/ipa/ipa_interrupt.c | 279 +++
>> drivers/net/ipa/ipa_interrupt.h | 53 +
>> drivers/net/ipa/ipa_main.c | 921 ++++++++++
>> drivers/net/ipa/ipa_mem.c | 234 +++
>> drivers/net/ipa/ipa_mem.h | 83 +
>> drivers/net/ipa/ipa_netdev.c | 251 +++
>> drivers/net/ipa/ipa_netdev.h | 24 +
>> drivers/net/ipa/ipa_qmi.c | 402 ++++
>> drivers/net/ipa/ipa_qmi.h | 35 +
>> drivers/net/ipa/ipa_qmi_msg.c | 583 ++++++
>> drivers/net/ipa/ipa_qmi_msg.h | 238 +++
>> drivers/net/ipa/ipa_reg.h | 279 +++
>> drivers/net/ipa/ipa_smp2p.c | 304 +++
>> drivers/net/ipa/ipa_smp2p.h | 47 +
>> drivers/net/ipa/ipa_uc.c | 208 +++
>> drivers/net/ipa/ipa_uc.h | 32 +
>> include/linux/bitfield.h | 14 +
>> 42 files changed, 10358 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/net/qcom,ipa.yaml
>> create mode 100644 drivers/net/ipa/Kconfig
>> create mode 100644 drivers/net/ipa/Makefile
>> create mode 100644 drivers/net/ipa/gsi.c
>> create mode 100644 drivers/net/ipa/gsi.h
>> create mode 100644 drivers/net/ipa/gsi_private.h
>> create mode 100644 drivers/net/ipa/gsi_reg.h
>> create mode 100644 drivers/net/ipa/gsi_trans.c
>> create mode 100644 drivers/net/ipa/gsi_trans.h
>> create mode 100644 drivers/net/ipa/ipa.h
>> create mode 100644 drivers/net/ipa/ipa_clock.c
>> create mode 100644 drivers/net/ipa/ipa_clock.h
>> create mode 100644 drivers/net/ipa/ipa_cmd.c
>> create mode 100644 drivers/net/ipa/ipa_cmd.h
>> create mode 100644 drivers/net/ipa/ipa_data-sdm845.c
>> create mode 100644 drivers/net/ipa/ipa_data.h
>> create mode 100644 drivers/net/ipa/ipa_endpoint.c
>> create mode 100644 drivers/net/ipa/ipa_endpoint.h
>> create mode 100644 drivers/net/ipa/ipa_gsi.c
>> create mode 100644 drivers/net/ipa/ipa_gsi.h
>> create mode 100644 drivers/net/ipa/ipa_interrupt.c
>> create mode 100644 drivers/net/ipa/ipa_interrupt.h
>> create mode 100644 drivers/net/ipa/ipa_main.c
>> create mode 100644 drivers/net/ipa/ipa_mem.c
>> create mode 100644 drivers/net/ipa/ipa_mem.h
>> create mode 100644 drivers/net/ipa/ipa_netdev.c
>> create mode 100644 drivers/net/ipa/ipa_netdev.h
>> create mode 100644 drivers/net/ipa/ipa_qmi.c
>> create mode 100644 drivers/net/ipa/ipa_qmi.h
>> create mode 100644 drivers/net/ipa/ipa_qmi_msg.c
>> create mode 100644 drivers/net/ipa/ipa_qmi_msg.h
>> create mode 100644 drivers/net/ipa/ipa_reg.h
>> create mode 100644 drivers/net/ipa/ipa_smp2p.c
>> create mode 100644 drivers/net/ipa/ipa_smp2p.h
>> create mode 100644 drivers/net/ipa/ipa_uc.c
>> create mode 100644 drivers/net/ipa/ipa_uc.h
>>
>

2019-05-31 19:21:06

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
> On 5/31/19 9:58 AM, Dan Williams wrote:
> > On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
> >
> > My question from the Nov 2018 IPA rmnet driver still stands; how does
> > this relate to net/ethernet/qualcomm/rmnet/ if at all? And if this is
> > really just a netdev talking to the IPA itself and unrelated to
> > net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop cargo-
> > culting rmnet around just because it happens to be a net driver for a
> > QC SoC.
>
> First, the relationship between the IPA driver and the rmnet driver
> is that the IPA driver is assumed to sit between the rmnet driver
> and the hardware.

Does this mean that IPA can only be used to back rmnet, and rmnet
can only be used on top of IPA, or can or both of them be combined
with another driver to talk to instead?

> Currently the modem is assumed to use QMAP protocol. This means
> each packet is prefixed by a (struct rmnet_map_header) structure
> that allows the IPA connection to be multiplexed for several logical
> connections. The rmnet driver parses such messages and implements
> the multiplexed network interfaces.
>
> QMAP protocol can also be used for aggregating many small packets
> into a larger message. The rmnet driver implements de-aggregation
> of such messages (and could probably aggregate them for TX as well).
>
> Finally, the IPA can support checksum offload, and the rmnet
> driver handles providing a prepended header (for TX) and
> interpreting the appended trailer (for RX) if these features
> are enabled.
>
> So basically, the purpose of the rmnet driver is to handle QMAP
> protocol connections, and right now that's what the modem provides.

Do you have any idea why this particular design was picked?

My best guess is that it evolved organically with multiple
generations of hardware and software, rather than being thought
out as a nice abstraction layer. If the two are tightly connected,
this might mean that what we actually want here is to reintegrate
the two components into a single driver with a much simpler
RX and TX path that handles the checksumming and aggregation
of data packets directly as it passes them from the network
stack into the hardware.

Always passing data from one netdev to another both ways
sounds like it introduces both direct CPU overhead, and
problems with flow control when data gets buffered inbetween.
The intermediate buffer here acts like a router that must
pass data along or randomly drop packets when the consumer
can't keep up with the producer.

Arnd

2019-05-31 20:49:06

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 5/31/19 2:19 PM, Arnd Bergmann wrote:
> On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
>> On 5/31/19 9:58 AM, Dan Williams wrote:
>>> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
>>>
>>> My question from the Nov 2018 IPA rmnet driver still stands; how does
>>> this relate to net/ethernet/qualcomm/rmnet/ if at all? And if this is
>>> really just a netdev talking to the IPA itself and unrelated to
>>> net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop cargo-
>>> culting rmnet around just because it happens to be a net driver for a
>>> QC SoC.
>>
>> First, the relationship between the IPA driver and the rmnet driver
>> is that the IPA driver is assumed to sit between the rmnet driver
>> and the hardware.
>
> Does this mean that IPA can only be used to back rmnet, and rmnet
> can only be used on top of IPA, or can or both of them be combined
> with another driver to talk to instead?

No it does not mean that.

As I understand it, one reason for the rmnet layer was to abstract
the back end, which would allow using a modem, or using something
else (a LAN?), without exposing certain details of the hardware.
(Perhaps to support multiplexing, etc. without duplicating that
logic in two "back-end" drivers?)

To be perfectly honest, at first I thought having IPA use rmnet
was a cargo cult thing like Dan suggested, because I didn't see
the benefit. I now see why one would use that pass-through layer
to handle the QMAP features.

But back to your question. The other thing is that I see no
reason the IPA couldn't present a "normal" (non QMAP) interface
for a modem. It's something I'd really like to be able to do,
but I can't do it without having the modem firmware change its
configuration for these endpoints. My access to the people who
implement the modem firmware has been very limited (something
I hope to improve), and unless and until I can get corresponding
changes on the modem side to implement connections that don't
use QMAP, I can't implement such a thing.

>> Currently the modem is assumed to use QMAP protocol. This means
>> each packet is prefixed by a (struct rmnet_map_header) structure
>> that allows the IPA connection to be multiplexed for several logical
>> connections. The rmnet driver parses such messages and implements
>> the multiplexed network interfaces.
>>
>> QMAP protocol can also be used for aggregating many small packets
>> into a larger message. The rmnet driver implements de-aggregation
>> of such messages (and could probably aggregate them for TX as well).
>>
>> Finally, the IPA can support checksum offload, and the rmnet
>> driver handles providing a prepended header (for TX) and
>> interpreting the appended trailer (for RX) if these features
>> are enabled.
>>
>> So basically, the purpose of the rmnet driver is to handle QMAP
>> protocol connections, and right now that's what the modem provides.
>
> Do you have any idea why this particular design was picked?

I don't really. I inherited it. Early on, when I asked about
the need for QMAP I was told it was important because it offered
certain features, but at that time I was somewhat new to the code
and didn't have the insight to judge the merits of the design.
Since then I've mostly just accepted it and concentrated on
improving the IPA driver.

> My best guess is that it evolved organically with multiple
> generations of hardware and software, rather than being thought
> out as a nice abstraction layer. If the two are tightly connected,
> this might mean that what we actually want here is to reintegrate
> the two components into a single driver with a much simpler
> RX and TX path that handles the checksumming and aggregation
> of data packets directly as it passes them from the network
> stack into the hardware.

In general, I agree. And Dan suggested combining the rmnet
and IPA drivers into a single driver when I posted the RFC
code last year. There's still the notion of switching back
ends that I mentioned earlier; if that's indeed an important
feature it might argue for keeping rmnet as a shim layer.
But I'm really not the person to comment on this. Someone
(Subash?) from Qualcomm might be able to provide better answers.

> Always passing data from one netdev to another both ways
> sounds like it introduces both direct CPU overhead, and
> problems with flow control when data gets buffered inbetween.

My impression is the rmnet driver is a pretty thin layer,
so the CPU overhead is probably not that great (though
deaggregating a message is expensive). I agree with you
on the flow control.

> The intermediate buffer here acts like a router that must
> pass data along or randomly drop packets when the consumer
> can't keep up with the producer.

I haven't reviewed the rmnet code in any detail, but you
may be right.

-Alex

>
> Arnd
>

2019-05-31 21:14:01

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Fri, May 31, 2019 at 10:47 PM Alex Elder <[email protected]> wrote:
> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
> > On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
> >> On 5/31/19 9:58 AM, Dan Williams wrote:
> >>> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
> >
> > Does this mean that IPA can only be used to back rmnet, and rmnet
> > can only be used on top of IPA, or can or both of them be combined
> > with another driver to talk to instead?
>
> No it does not mean that.
>
> As I understand it, one reason for the rmnet layer was to abstract
> the back end, which would allow using a modem, or using something
> else (a LAN?), without exposing certain details of the hardware.
> (Perhaps to support multiplexing, etc. without duplicating that
> logic in two "back-end" drivers?)
>
> To be perfectly honest, at first I thought having IPA use rmnet
> was a cargo cult thing like Dan suggested, because I didn't see
> the benefit. I now see why one would use that pass-through layer
> to handle the QMAP features.
>
> But back to your question. The other thing is that I see no
> reason the IPA couldn't present a "normal" (non QMAP) interface
> for a modem. It's something I'd really like to be able to do,
> but I can't do it without having the modem firmware change its
> configuration for these endpoints. My access to the people who
> implement the modem firmware has been very limited (something
> I hope to improve), and unless and until I can get corresponding
> changes on the modem side to implement connections that don't
> use QMAP, I can't implement such a thing.

Why would that require firmware changes? What I was thinking
here is to turn the bits of the rmnet driver that actually do anything
interesting on the headers into a library module (or a header file
with inline functions) that can be called directly by the ipa driver,
keeping the protocol unchanged.

> > Always passing data from one netdev to another both ways
> > sounds like it introduces both direct CPU overhead, and
> > problems with flow control when data gets buffered inbetween.
>
> My impression is the rmnet driver is a pretty thin layer,
> so the CPU overhead is probably not that great (though
> deaggregating a message is expensive). I agree with you
> on the flow control.

The CPU overhead I mean is not from executing code in the
rmnet driver, but from passing packets through the network
stack between the two drivers, i.e. adding each frame to
a queue and taking it back out. I'm not sure how this ends
up working in reality but from a first look it seems like
we might bounce in an out of the softirq handler inbetween.

Arnd

2019-05-31 22:11:04

by Subash Abhinov Kasiviswanathan

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 5/31/19 4:12 PM, Arnd Bergmann wrote:
> On Fri, May 31, 2019 at 10:47 PM Alex Elder <[email protected]> wrote:
>> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
>>> On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
>>>> On 5/31/19 9:58 AM, Dan Williams wrote:
>>>>> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
>>>
>>> Does this mean that IPA can only be used to back rmnet, and rmnet
>>> can only be used on top of IPA, or can or both of them be combined
>>> with another driver to talk to instead?
>>
>> No it does not mean that.
>>
>> As I understand it, one reason for the rmnet layer was to abstract
>> the back end, which would allow using a modem, or using something
>> else (a LAN?), without exposing certain details of the hardware.
>> (Perhaps to support multiplexing, etc. without duplicating that
>> logic in two "back-end" drivers?)
>>
>> To be perfectly honest, at first I thought having IPA use rmnet
>> was a cargo cult thing like Dan suggested, because I didn't see
>> the benefit. I now see why one would use that pass-through layer
>> to handle the QMAP features.
>>
>> But back to your question. The other thing is that I see no
>> reason the IPA couldn't present a "normal" (non QMAP) interface
>> for a modem. It's something I'd really like to be able to do,
>> but I can't do it without having the modem firmware change its
>> configuration for these endpoints. My access to the people who
>> implement the modem firmware has been very limited (something
>> I hope to improve), and unless and until I can get corresponding
>> changes on the modem side to implement connections that don't
>> use QMAP, I can't implement such a thing.
>
> Why would that require firmware changes? What I was thinking
> here is to turn the bits of the rmnet driver that actually do anything
> interesting on the headers into a library module (or a header file
> with inline functions) that can be called directly by the ipa driver,
> keeping the protocol unchanged.

You know, it's possible you're right about not needing
firmware changes. But it has always been my impression
they would be needed. Here's why.

It looks like this:

GSI Channel GSI Channel
| |
---------- v ------- v -------------
| AP (ep)|=======| IPA |=======|(ep) Modem |
---------- ------- -------------

The AP and Modem each have IPA endpoints (ep), which use GSI channels,
to communicate with the IPA. Each endpoint has configuration options
(such as checksum offload). I *thought* that the configurations of
the two endpoints need to be compatible (e.g., they need to agree on
whether they're aggregating). But with your questioning I now think
you may be right, that only the local endpoint's configuration matters.

I will inquire further on this. I *know* that the AP and modem
exchange some information about IPA configuration, but looking more
closely that looks like it's all about the configuration of shared
IPA resources, not endpoints.

That said, the broader design (including the user space code)
surely assumes rmnet, and I don't have any sense of what impact
changing that would make. I am sure that changing it would not
be well received.

-Alex

>>> Always passing data from one netdev to another both ways
>>> sounds like it introduces both direct CPU overhead, and
>>> problems with flow control when data gets buffered inbetween.
>>
>> My impression is the rmnet driver is a pretty thin layer,
>> so the CPU overhead is probably not that great (though
>> deaggregating a message is expensive). I agree with you
>> on the flow control.
>
> The CPU overhead I mean is not from executing code in the
> rmnet driver, but from passing packets through the network
> stack between the two drivers, i.e. adding each frame to
> a queue and taking it back out. I'm not sure how this ends
> up working in reality but from a first look it seems like
> we might bounce in an out of the softirq handler inbetween.
>
> Arnd
>

2019-05-31 23:29:57

by Bjorn Andersson

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Fri 31 May 12:19 PDT 2019, Arnd Bergmann wrote:

> On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
> > On 5/31/19 9:58 AM, Dan Williams wrote:
> > > On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
[..]
> > So basically, the purpose of the rmnet driver is to handle QMAP
> > protocol connections, and right now that's what the modem provides.
>
> Do you have any idea why this particular design was picked?
>

From what I've seen of QMAP it seems like a reasonable design choice to
have a software component (rmnet) dealing with this, separate from the
transport. And I think IPA is the 4th or 5th mechanism for transporting
QMAP packets back and forth to the modem.

Downstream rmnet is copyright 2007-, and I know of interest in bringing
at least one of the other transports upstream.

Regards,
Bjorn

2019-05-31 23:34:47

by Bjorn Andersson

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Fri 31 May 13:47 PDT 2019, Alex Elder wrote:

> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
> > On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
> >> On 5/31/19 9:58 AM, Dan Williams wrote:
> >>> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
> >>>
> >>> My question from the Nov 2018 IPA rmnet driver still stands; how does
> >>> this relate to net/ethernet/qualcomm/rmnet/ if at all? And if this is
> >>> really just a netdev talking to the IPA itself and unrelated to
> >>> net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop cargo-
> >>> culting rmnet around just because it happens to be a net driver for a
> >>> QC SoC.
> >>
> >> First, the relationship between the IPA driver and the rmnet driver
> >> is that the IPA driver is assumed to sit between the rmnet driver
> >> and the hardware.
> >
> > Does this mean that IPA can only be used to back rmnet, and rmnet
> > can only be used on top of IPA, or can or both of them be combined
> > with another driver to talk to instead?
>
> No it does not mean that.
>
> As I understand it, one reason for the rmnet layer was to abstract
> the back end, which would allow using a modem, or using something
> else (a LAN?), without exposing certain details of the hardware.
> (Perhaps to support multiplexing, etc. without duplicating that
> logic in two "back-end" drivers?)
>
> To be perfectly honest, at first I thought having IPA use rmnet
> was a cargo cult thing like Dan suggested, because I didn't see
> the benefit. I now see why one would use that pass-through layer
> to handle the QMAP features.
>
> But back to your question. The other thing is that I see no
> reason the IPA couldn't present a "normal" (non QMAP) interface
> for a modem. It's something I'd really like to be able to do,
> but I can't do it without having the modem firmware change its
> configuration for these endpoints. My access to the people who
> implement the modem firmware has been very limited (something
> I hope to improve), and unless and until I can get corresponding
> changes on the modem side to implement connections that don't
> use QMAP, I can't implement such a thing.
>

But any such changes would either be years into the future or for
specific devices and as such not applicable to any/most of devices on
the market now or in the coming years.

But as Arnd points out, if the software split between IPA and rmnet is
suboptimal your are encouraged to fix that.

Regards,
Bjorn

2019-06-01 00:01:10

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 2019-05-31 17:33, Bjorn Andersson wrote:
> On Fri 31 May 13:47 PDT 2019, Alex Elder wrote:
>
>> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
>> > On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
>> >> On 5/31/19 9:58 AM, Dan Williams wrote:
>> >>> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
>> >>>
>> >>> My question from the Nov 2018 IPA rmnet driver still stands; how does
>> >>> this relate to net/ethernet/qualcomm/rmnet/ if at all? And if this is
>> >>> really just a netdev talking to the IPA itself and unrelated to
>> >>> net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop cargo-
>> >>> culting rmnet around just because it happens to be a net driver for a
>> >>> QC SoC.
>> >>
>> >> First, the relationship between the IPA driver and the rmnet driver
>> >> is that the IPA driver is assumed to sit between the rmnet driver
>> >> and the hardware.
>> >
>> > Does this mean that IPA can only be used to back rmnet, and rmnet
>> > can only be used on top of IPA, or can or both of them be combined
>> > with another driver to talk to instead?
>>
>> No it does not mean that.
>>
>> As I understand it, one reason for the rmnet layer was to abstract
>> the back end, which would allow using a modem, or using something
>> else (a LAN?), without exposing certain details of the hardware.
>> (Perhaps to support multiplexing, etc. without duplicating that
>> logic in two "back-end" drivers?)
>>
>> To be perfectly honest, at first I thought having IPA use rmnet
>> was a cargo cult thing like Dan suggested, because I didn't see
>> the benefit. I now see why one would use that pass-through layer
>> to handle the QMAP features.
>>
>> But back to your question. The other thing is that I see no
>> reason the IPA couldn't present a "normal" (non QMAP) interface
>> for a modem. It's something I'd really like to be able to do,
>> but I can't do it without having the modem firmware change its
>> configuration for these endpoints. My access to the people who
>> implement the modem firmware has been very limited (something
>> I hope to improve), and unless and until I can get corresponding
>> changes on the modem side to implement connections that don't
>> use QMAP, I can't implement such a thing.
>>
>
> But any such changes would either be years into the future or for
> specific devices and as such not applicable to any/most of devices on
> the market now or in the coming years.
>
>
> But as Arnd points out, if the software split between IPA and rmnet is
> suboptimal your are encouraged to fix that.
>
> Regards,
> Bjorn

The split rmnet design was chosen because we could place rmnet
over any transport - IPA, PCIe (https://lkml.org/lkml/2018/4/26/1159)
or USB.

rmnet registers a rx handler, so the rmnet packet processing itself
happens in the same softirq when packets are queued to network stack
by IPA.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-06-03 11:54:26

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Sat, Jun 1, 2019 at 1:59 AM Subash Abhinov Kasiviswanathan
<[email protected]> wrote:
> On 2019-05-31 17:33, Bjorn Andersson wrote:
> > On Fri 31 May 13:47 PDT 2019, Alex Elder wrote:
> >> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
> > But any such changes would either be years into the future or for
> > specific devices and as such not applicable to any/most of devices on
> > the market now or in the coming years.
> >
> >
> > But as Arnd points out, if the software split between IPA and rmnet is
> > suboptimal your are encouraged to fix that.
>
> The split rmnet design was chosen because we could place rmnet
> over any transport - IPA, PCIe (https://lkml.org/lkml/2018/4/26/1159)
> or USB.
>
> rmnet registers a rx handler, so the rmnet packet processing itself
> happens in the same softirq when packets are queued to network stack
> by IPA.

I've read up on the implementation some more, and concluded that
it's mostly a regular protocol wrapper, doing IP over QMAP. There
is nothing wrong with the basic concept I think, and as you describe
this is an abstraction to keep the common bits in one place, and
have them configured consistently.

A few observations on more details here:

- What I'm worried about most here is the flow control handling on the
transmit side. The IPA driver now uses the modern BQL method to
control how much data gets submitted to the hardware at any time.
The rmnet driver also uses flow control using the
rmnet_map_command() function, that blocks tx on the higher
level device when the remote side asks us to.
I fear that doing flow control for a single physical device on two
separate netdev instances is counterproductive and confuses
both sides.

- I was a little confused by the location of the rmnet driver in
drivers/net/ethernet/... More conventionally, I think as a protocol
handler it should go into net/qmap/, with the ipa driver going
into drivers/net/qmap/ipa/, similar to what we have fo ethernet,
wireless, ppp, appletalk, etc.

- The rx_handler uses gro_cells, which as I understand is meant
for generic tunnelling setups and takes another loop through
NAPI to aggregate data from multiple queues, but in case of
IPA's single-queue receive calling gro directly would be simpler
and more efficient.

- I'm still not sure I understand the purpose of the layering with
using an rx_handler as opposed to just using
EXPORT_SYMBOL(rmnet_rx_handler) and calling that from
the hardware driver directly.
From the overall design and the rmnet Kconfig description, it
appears as though the intention as that rmnet could be a
generic wrapper on top of any device, but from the
implementation it seems that IPA is not actually usable that
way and would always go through IPA.

Arnd

2019-06-03 13:34:55

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 6/3/19 5:04 AM, Arnd Bergmann wrote:
> On Sat, Jun 1, 2019 at 1:59 AM Subash Abhinov Kasiviswanathan
> <[email protected]> wrote:
>> On 2019-05-31 17:33, Bjorn Andersson wrote:
>>> On Fri 31 May 13:47 PDT 2019, Alex Elder wrote:
>>>> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
>>> But any such changes would either be years into the future or for
>>> specific devices and as such not applicable to any/most of devices on
>>> the market now or in the coming years.
>>>
>>>
>>> But as Arnd points out, if the software split between IPA and rmnet is
>>> suboptimal your are encouraged to fix that.
>>
>> The split rmnet design was chosen because we could place rmnet
>> over any transport - IPA, PCIe (https://lkml.org/lkml/2018/4/26/1159)
>> or USB.
>>
>> rmnet registers a rx handler, so the rmnet packet processing itself
>> happens in the same softirq when packets are queued to network stack
>> by IPA.
>
> I've read up on the implementation some more, and concluded that
> it's mostly a regular protocol wrapper, doing IP over QMAP. There
> is nothing wrong with the basic concept I think, and as you describe
> this is an abstraction to keep the common bits in one place, and
> have them configured consistently.
>
> A few observations on more details here:
>
> - What I'm worried about most here is the flow control handling on the
> transmit side. The IPA driver now uses the modern BQL method to
> control how much data gets submitted to the hardware at any time.
> The rmnet driver also uses flow control using the
> rmnet_map_command() function, that blocks tx on the higher
> level device when the remote side asks us to.
> I fear that doing flow control for a single physical device on two
> separate netdev instances is counterproductive and confuses
> both sides.

I understand what you're saying here, and instinctively I think
you're right.

But BQL manages the *local* interface's ability to get rid of
packets, whereas the QMAP flow control is initiated by the other
end of the connection (the modem in this case).

With multiplexing, it's possible that one of several logical
devices on the modem side has exhausted a resource and must
ask the source of the data on the host side to suspend the
flow. Meanwhile the other logical devices sharing the physical
link might be fine, and should not be delayed by the first one.

It is the multiplexing itself that confuses the BQL algorithm.
The abstraction obscures the *real* rates at which individual
logical connections are able to transmit data.

Even if the multiple logical interfaces implemented BQL, they
would not get the feedback they need directly from the IPA
driver, because transmitting over the physical interface might
succeed even if the logical interface on the modem side can't
handle more data. So I think the flow control commands may be
necessary, given multiplexing.

The rmnet driver could use BQL, and could return NETDEV_TX_BUSY
for a logical interface when its TX flow has been stopped by a
QMAP command. That way the feedback for BQL on the logical
interfaces would be provided in the right place.

I have no good intuition about the interaction between
two layered BQL managed queues though.

> - I was a little confused by the location of the rmnet driver in
> drivers/net/ethernet/... More conventionally, I think as a protocol
> handler it should go into net/qmap/, with the ipa driver going
> into drivers/net/qmap/ipa/, similar to what we have fo ethernet,
> wireless, ppp, appletalk, etc.
>
> - The rx_handler uses gro_cells, which as I understand is meant
> for generic tunnelling setups and takes another loop through
> NAPI to aggregate data from multiple queues, but in case of
> IPA's single-queue receive calling gro directly would be simpler
> and more efficient.

I have been planning to investigate some of the generic GRO
stuff for IPA but was going to wait on that until the basic
code was upstream.

> - I'm still not sure I understand the purpose of the layering with
> using an rx_handler as opposed to just using
> EXPORT_SYMBOL(rmnet_rx_handler) and calling that from
> the hardware driver directly.

I think that's a good idea.

> From the overall design and the rmnet Kconfig description, it
> appears as though the intention as that rmnet could be a
> generic wrapper on top of any device, but from the
> implementation it seems that IPA is not actually usable that
> way and would always go through IPA.

As far as I know *nothing* upstream currently uses rmnet; the
IPA driver will be the first, but as Bjorn said others seem to
be on the way. I'm not sure what you mean by "IPA is not
usable that way." Currently the IPA driver assumes a fixed
configuration, and that configuration assumes the use of QMAP,
and therefore assumes the rmnet driver is layered above it.
That doesn't preclude rmnet from using a different back end.

And I'll also mention that although QMAP *can* do multiplexed
connections over a single physical link, the IPA code I posted
currently supports only one logical interface.

-Alex

>
> Arnd
>

2019-06-03 14:57:40

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Fri, 2019-05-31 at 15:47 -0500, Alex Elder wrote:
> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
> > On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]>
> > wrote:
> > > On 5/31/19 9:58 AM, Dan Williams wrote:
> > > > On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
> > > >
> > > > My question from the Nov 2018 IPA rmnet driver still stands;
> > > > how does
> > > > this relate to net/ethernet/qualcomm/rmnet/ if at all? And if
> > > > this is
> > > > really just a netdev talking to the IPA itself and unrelated to
> > > > net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop
> > > > cargo-
> > > > culting rmnet around just because it happens to be a net driver
> > > > for a
> > > > QC SoC.
> > >
> > > First, the relationship between the IPA driver and the rmnet
> > > driver
> > > is that the IPA driver is assumed to sit between the rmnet driver
> > > and the hardware.
> >
> > Does this mean that IPA can only be used to back rmnet, and rmnet
> > can only be used on top of IPA, or can or both of them be combined
> > with another driver to talk to instead?
>
> No it does not mean that.
>
> As I understand it, one reason for the rmnet layer was to abstract
> the back end, which would allow using a modem, or using something
> else (a LAN?), without exposing certain details of the hardware.
> (Perhaps to support multiplexing, etc. without duplicating that
> logic in two "back-end" drivers?)
>
> To be perfectly honest, at first I thought having IPA use rmnet
> was a cargo cult thing like Dan suggested, because I didn't see

To be clear I only meant cargo-culting the naming, not any
functionality. Clearly IPA/rmnet/QMAP are pretty intimately connected
at this point. But this goes back to whether IPA needs a netdev itself
or whether you need an rmnet device created on top. If the former then
I'd say no cargo-culting, if the later then it's a moot point because
the device name will be rmnet%d anyway.

Dan

> the benefit. I now see why one would use that pass-through layer
> to handle the QMAP features.
>
> But back to your question. The other thing is that I see no
> reason the IPA couldn't present a "normal" (non QMAP) interface
> for a modem. It's something I'd really like to be able to do,
> but I can't do it without having the modem firmware change its
> configuration for these endpoints. My access to the people who
> implement the modem firmware has been very limited (something
> I hope to improve), and unless and until I can get corresponding
> changes on the modem side to implement connections that don't
> use QMAP, I can't implement such a thing.
>
> > > Currently the modem is assumed to use QMAP protocol. This means
> > > each packet is prefixed by a (struct rmnet_map_header) structure
> > > that allows the IPA connection to be multiplexed for several
> > > logical
> > > connections. The rmnet driver parses such messages and
> > > implements
> > > the multiplexed network interfaces.
> > >
> > > QMAP protocol can also be used for aggregating many small packets
> > > into a larger message. The rmnet driver implements de-
> > > aggregation
> > > of such messages (and could probably aggregate them for TX as
> > > well).
> > >
> > > Finally, the IPA can support checksum offload, and the rmnet
> > > driver handles providing a prepended header (for TX) and
> > > interpreting the appended trailer (for RX) if these features
> > > are enabled.
> > >
> > > So basically, the purpose of the rmnet driver is to handle QMAP
> > > protocol connections, and right now that's what the modem
> > > provides.
> >
> > Do you have any idea why this particular design was picked?
>
> I don't really. I inherited it. Early on, when I asked about
> the need for QMAP I was told it was important because it offered
> certain features, but at that time I was somewhat new to the code
> and didn't have the insight to judge the merits of the design.
> Since then I've mostly just accepted it and concentrated on
> improving the IPA driver.
>
> > My best guess is that it evolved organically with multiple
> > generations of hardware and software, rather than being thought
> > out as a nice abstraction layer. If the two are tightly connected,
> > this might mean that what we actually want here is to reintegrate
> > the two components into a single driver with a much simpler
> > RX and TX path that handles the checksumming and aggregation
> > of data packets directly as it passes them from the network
> > stack into the hardware.
>
> In general, I agree. And Dan suggested combining the rmnet
> and IPA drivers into a single driver when I posted the RFC
> code last year. There's still the notion of switching back
> ends that I mentioned earlier; if that's indeed an important
> feature it might argue for keeping rmnet as a shim layer.
> But I'm really not the person to comment on this. Someone
> (Subash?) from Qualcomm might be able to provide better answers.
>
> > Always passing data from one netdev to another both ways
> > sounds like it introduces both direct CPU overhead, and
> > problems with flow control when data gets buffered inbetween.
>
> My impression is the rmnet driver is a pretty thin layer,
> so the CPU overhead is probably not that great (though
> deaggregating a message is expensive). I agree with you
> on the flow control.
>
> > The intermediate buffer here acts like a router that must
> > pass data along or randomly drop packets when the consumer
> > can't keep up with the producer.
>
> I haven't reviewed the rmnet code in any detail, but you
> may be right.
>
> -Alex
>
> > Arnd
> >

2019-06-03 15:54:51

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 6/3/19 9:54 AM, Dan Williams wrote:
>> To be perfectly honest, at first I thought having IPA use rmnet
>> was a cargo cult thing like Dan suggested, because I didn't see
> To be clear I only meant cargo-culting the naming, not any
> functionality. Clearly IPA/rmnet/QMAP are pretty intimately connected
> at this point. But this goes back to whether IPA needs a netdev itself
> or whether you need an rmnet device created on top. If the former then
> I'd say no cargo-culting, if the later then it's a moot point because
> the device name will be rmnet%d anyway.

OK I thought you weren't sure why rmnet was a layer at all. As I
said, I didn't have a very good understanding of why it was even
needed when I first started working on this.

I can't (or won't) comment right now on whether IPA needs its own
netdev for rmnet to use. The IPA endpoints used for the modem
network interfaces are enabled when the netdev is opened and
disabled when closed. Outside of that, TX and RX are pretty
much immediately passed through to the layer below or above.
IPA currently has no other net device operations.

-Alex

2019-06-03 16:21:22

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Mon, 2019-06-03 at 10:52 -0500, Alex Elder wrote:
> On 6/3/19 9:54 AM, Dan Williams wrote:
> > > To be perfectly honest, at first I thought having IPA use rmnet
> > > was a cargo cult thing like Dan suggested, because I didn't see
> > To be clear I only meant cargo-culting the naming, not any
> > functionality. Clearly IPA/rmnet/QMAP are pretty intimately
> > connected
> > at this point. But this goes back to whether IPA needs a netdev
> > itself
> > or whether you need an rmnet device created on top. If the former
> > then
> > I'd say no cargo-culting, if the later then it's a moot point
> > because
> > the device name will be rmnet%d anyway.
>
> OK I thought you weren't sure why rmnet was a layer at all. As I
> said, I didn't have a very good understanding of why it was even
> needed when I first started working on this.

No problem.

> I can't (or won't) comment right now on whether IPA needs its own
> netdev for rmnet to use. The IPA endpoints used for the modem
> network interfaces are enabled when the netdev is opened and
> disabled when closed. Outside of that, TX and RX are pretty
> much immediately passed through to the layer below or above.
> IPA currently has no other net device operations.

I don't really have issues with the patchset underneath the netdev
layer. I'm interested in how the various bits present themselves to
userspace, which is why I am trying to tie this in with Johannes'
conversation about WWAN devices, netdevs, channels, and how the various
drivers present API for creating data channels that map to different
modem contexts.

So let me rephrase. If the control plane has set up the default context
and sent a QMI Start Network message (or the network attached the
default one) and the resulting IP details are applied to the IPA netdev
can things just start sending data? Or do we need to create an rmnet on
top to get that working?

Dan

2019-06-03 18:02:10

by Subash Abhinov Kasiviswanathan

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Fri, 2019-05-31 at 17:59 -0600, Subash Abhinov Kasiviswanathan
wrote:
> On 2019-05-31 17:33, Bjorn Andersson wrote:
> > On Fri 31 May 13:47 PDT 2019, Alex Elder wrote:
> >
> > > On 5/31/19 2:19 PM, Arnd Bergmann wrote:
> > > > On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]>
> > > > wrote:
> > > > > On 5/31/19 9:58 AM, Dan Williams wrote:
> > > > > > On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
> > > > > >
> > > > > > My question from the Nov 2018 IPA rmnet driver still
> > > > > > stands; how does
> > > > > > this relate to net/ethernet/qualcomm/rmnet/ if at all? And
> > > > > > if this is
> > > > > > really just a netdev talking to the IPA itself and
> > > > > > unrelated to
> > > > > > net/ethernet/qualcomm/rmnet, let's call it "ipa%d" and stop
> > > > > > cargo-
> > > > > > culting rmnet around just because it happens to be a net
> > > > > > driver for a
> > > > > > QC SoC.
> > > > >
> > > > > First, the relationship between the IPA driver and the rmnet
> > > > > driver
> > > > > is that the IPA driver is assumed to sit between the rmnet
> > > > > driver
> > > > > and the hardware.
> > > >
> > > > Does this mean that IPA can only be used to back rmnet, and
> > > > rmnet
> > > > can only be used on top of IPA, or can or both of them be
> > > > combined
> > > > with another driver to talk to instead?
> > >
> > > No it does not mean that.
> > >
> > > As I understand it, one reason for the rmnet layer was to
> > > abstract
> > > the back end, which would allow using a modem, or using something
> > > else (a LAN?), without exposing certain details of the hardware.
> > > (Perhaps to support multiplexing, etc. without duplicating that
> > > logic in two "back-end" drivers?)
> > >
> > > To be perfectly honest, at first I thought having IPA use rmnet
> > > was a cargo cult thing like Dan suggested, because I didn't see
> > > the benefit. I now see why one would use that pass-through layer
> > > to handle the QMAP features.
> > >
> > > But back to your question. The other thing is that I see no
> > > reason the IPA couldn't present a "normal" (non QMAP) interface
> > > for a modem. It's something I'd really like to be able to do,
> > > but I can't do it without having the modem firmware change its
> > > configuration for these endpoints. My access to the people who
> > > implement the modem firmware has been very limited (something
> > > I hope to improve), and unless and until I can get corresponding
> > > changes on the modem side to implement connections that don't
> > > use QMAP, I can't implement such a thing.
> > >
> >
> > But any such changes would either be years into the future or for
> > specific devices and as such not applicable to any/most of devices
> > on
> > the market now or in the coming years.
> >
> >
> > But as Arnd points out, if the software split between IPA and rmnet
> > is
> > suboptimal your are encouraged to fix that.
> >
> > Regards,
> > Bjorn
>
> The split rmnet design was chosen because we could place rmnet
> over any transport - IPA, PCIe (https://lkml.org/lkml/2018/4/26/1159)
> or USB.

Yeah, that's what I was looking for clarification on :) Clearly since
rmnet can have many transports it should be able to be used by
different HW drivers, be that qmi_wwan, IPA, and maybe even
rmnet_smd.c?

> rmnet registers a rx handler, so the rmnet packet processing itself
> happens in the same softirq when packets are queued to network stack
> by IPA.

This directly relates to the discussion about a WWAN subsystem that
Johannes Berg started a couple weeks ago. IPA appears to create a
netdev of its own. Is that netdev usable immediately, or does one need
to create an rmnet device on top to access the default PDN?

Thanks,
Dan

2019-06-03 19:06:25

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

>> I can't (or won't) comment right now on whether IPA needs its own
>> netdev for rmnet to use. The IPA endpoints used for the modem
>> network interfaces are enabled when the netdev is opened and
>> disabled when closed. Outside of that, TX and RX are pretty
>> much immediately passed through to the layer below or above.
>> IPA currently has no other net device operations.
>
> I don't really have issues with the patchset underneath the netdev
> layer. I'm interested in how the various bits present themselves to
> userspace, which is why I am trying to tie this in with Johannes'
> conversation about WWAN devices, netdevs, channels, and how the various
> drivers present API for creating data channels that map to different
> modem contexts.
>
> So let me rephrase. If the control plane has set up the default context
> and sent a QMI Start Network message (or the network attached the
> default one) and the resulting IP details are applied to the IPA netdev
> can things just start sending data? Or do we need to create an rmnet on
> top to get that working?
>
> Dan

Hi Dan

All data from the hardware will have the MAP headers.
We still need to create rmnet devs over the IPA netdev and use it for
the
data path.
The IPA netdev will pass on the packets which it receives from the
hardware
and queue it to network stack where it will be intercepted by the
rmnet rx handler.

--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

2019-06-04 08:16:49

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Mon, Jun 3, 2019 at 3:32 PM Alex Elder <[email protected]> wrote:
> On 6/3/19 5:04 AM, Arnd Bergmann wrote:
> > On Sat, Jun 1, 2019 at 1:59 AM Subash Abhinov Kasiviswanathan
> >
> > - What I'm worried about most here is the flow control handling on the
> > transmit side. The IPA driver now uses the modern BQL method to
> > control how much data gets submitted to the hardware at any time.
> > The rmnet driver also uses flow control using the
> > rmnet_map_command() function, that blocks tx on the higher
> > level device when the remote side asks us to.
> > I fear that doing flow control for a single physical device on two
> > separate netdev instances is counterproductive and confuses
> > both sides.
>
> I understand what you're saying here, and instinctively I think
> you're right.
>
> But BQL manages the *local* interface's ability to get rid of
> packets, whereas the QMAP flow control is initiated by the other
> end of the connection (the modem in this case).
>
> With multiplexing, it's possible that one of several logical
> devices on the modem side has exhausted a resource and must
> ask the source of the data on the host side to suspend the
> flow. Meanwhile the other logical devices sharing the physical
> link might be fine, and should not be delayed by the first one.
>
> It is the multiplexing itself that confuses the BQL algorithm.
> The abstraction obscures the *real* rates at which individual
> logical connections are able to transmit data.

I would assume that the real rate constantly changes, at least
for wireless interfaces that are also shared with other users
on the same network. BQL is meant to deal with that, at least
when using a modern queuing algorithm.

> Even if the multiple logical interfaces implemented BQL, they
> would not get the feedback they need directly from the IPA
> driver, because transmitting over the physical interface might
> succeed even if the logical interface on the modem side can't
> handle more data. So I think the flow control commands may be
> necessary, given multiplexing.

Can you describe what kind of multiplexing is actually going on?
I'm still unclear about what we actually use multiple logical
interfaces for here, and how they relate to one another.

> The rmnet driver could use BQL, and could return NETDEV_TX_BUSY
> for a logical interface when its TX flow has been stopped by a
> QMAP command. That way the feedback for BQL on the logical
> interfaces would be provided in the right place.
>
> I have no good intuition about the interaction between
> two layered BQL managed queues though.

Returning NETDEV_TX_BUSY is usually a bad idea as that
leads to unnecessary frame drop.

I do think that using BQL and the QMAP flow command on
the /same/ device would be best, as that throttles the connection
when either of the two algorithms wants us to slow down.

The question is mainly which of the two devices that should be.
Doing it in the ipa driver is probably easier to implement here,
but ideally I think we'd only have a single queue visible to the
network stack, if we can come up with a way to do that.

> > - I was a little confused by the location of the rmnet driver in
> > drivers/net/ethernet/... More conventionally, I think as a protocol
> > handler it should go into net/qmap/, with the ipa driver going
> > into drivers/net/qmap/ipa/, similar to what we have fo ethernet,
> > wireless, ppp, appletalk, etc.
> >
> > - The rx_handler uses gro_cells, which as I understand is meant
> > for generic tunnelling setups and takes another loop through
> > NAPI to aggregate data from multiple queues, but in case of
> > IPA's single-queue receive calling gro directly would be simpler
> > and more efficient.
>
> I have been planning to investigate some of the generic GRO
> stuff for IPA but was going to wait on that until the basic
> code was upstream.

That's ok, that part can easily be changed after the fact, as it
does not impact the user interface or the general design.

> > From the overall design and the rmnet Kconfig description, it
> > appears as though the intention as that rmnet could be a
> > generic wrapper on top of any device, but from the
> > implementation it seems that IPA is not actually usable that
> > way and would always go through IPA.
>
> As far as I know *nothing* upstream currently uses rmnet; the
> IPA driver will be the first, but as Bjorn said others seem to
> be on the way. I'm not sure what you mean by "IPA is not
> usable that way." Currently the IPA driver assumes a fixed
> configuration, and that configuration assumes the use of QMAP,
> and therefore assumes the rmnet driver is layered above it.
> That doesn't preclude rmnet from using a different back end.

Yes, that's what I meant above: IPA can only be used through
rmnet (I wrote "through IPA", sorry for the typo), but cannot be
used by itself.

Arnd

2019-06-04 15:20:35

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Tue, 2019-06-04 at 10:13 +0200, Arnd Bergmann wrote:
> On Mon, Jun 3, 2019 at 3:32 PM Alex Elder <[email protected]> wrote:
> > On 6/3/19 5:04 AM, Arnd Bergmann wrote:
> > > On Sat, Jun 1, 2019 at 1:59 AM Subash Abhinov Kasiviswanathan
> > >
> > > - What I'm worried about most here is the flow control handling
> > > on the
> > > transmit side. The IPA driver now uses the modern BQL method to
> > > control how much data gets submitted to the hardware at any
> > > time.
> > > The rmnet driver also uses flow control using the
> > > rmnet_map_command() function, that blocks tx on the higher
> > > level device when the remote side asks us to.
> > > I fear that doing flow control for a single physical device on
> > > two
> > > separate netdev instances is counterproductive and confuses
> > > both sides.
> >
> > I understand what you're saying here, and instinctively I think
> > you're right.
> >
> > But BQL manages the *local* interface's ability to get rid of
> > packets, whereas the QMAP flow control is initiated by the other
> > end of the connection (the modem in this case).
> >
> > With multiplexing, it's possible that one of several logical
> > devices on the modem side has exhausted a resource and must
> > ask the source of the data on the host side to suspend the
> > flow. Meanwhile the other logical devices sharing the physical
> > link might be fine, and should not be delayed by the first one.
> >
> > It is the multiplexing itself that confuses the BQL algorithm.
> > The abstraction obscures the *real* rates at which individual
> > logical connections are able to transmit data.
>
> I would assume that the real rate constantly changes, at least
> for wireless interfaces that are also shared with other users
> on the same network. BQL is meant to deal with that, at least
> when using a modern queuing algorithm.
>
> > Even if the multiple logical interfaces implemented BQL, they
> > would not get the feedback they need directly from the IPA
> > driver, because transmitting over the physical interface might
> > succeed even if the logical interface on the modem side can't
> > handle more data. So I think the flow control commands may be
> > necessary, given multiplexing.
>
> Can you describe what kind of multiplexing is actually going on?
> I'm still unclear about what we actually use multiple logical
> interfaces for here, and how they relate to one another.

Each logical interface represents a different "connection" (PDP/EPS
context) to the provider network with a distinct IP address and QoS.
VLANs may be a suitable analogy but here they are L3+QoS.

In realistic example the main interface (say rmnet0) would be used for
web browsing and have best-effort QoS. A second interface (say rmnet1)
would be used for VOIP and have certain QoS guarantees from both the
modem and the network itself.

QMAP can also aggregate frames for a given channel (connection/EPS/PDP
context/rmnet interface/etc) to better support LTE speeds.

Dan

> > The rmnet driver could use BQL, and could return NETDEV_TX_BUSY
> > for a logical interface when its TX flow has been stopped by a
> > QMAP command. That way the feedback for BQL on the logical
> > interfaces would be provided in the right place.
> >
> > I have no good intuition about the interaction between
> > two layered BQL managed queues though.
>
> Returning NETDEV_TX_BUSY is usually a bad idea as that
> leads to unnecessary frame drop.
>
> I do think that using BQL and the QMAP flow command on
> the /same/ device would be best, as that throttles the connection
> when either of the two algorithms wants us to slow down.
>
> The question is mainly which of the two devices that should be.
> Doing it in the ipa driver is probably easier to implement here,
> but ideally I think we'd only have a single queue visible to the
> network stack, if we can come up with a way to do that.
>
> > > - I was a little confused by the location of the rmnet driver in
> > > drivers/net/ethernet/... More conventionally, I think as a
> > > protocol
> > > handler it should go into net/qmap/, with the ipa driver going
> > > into drivers/net/qmap/ipa/, similar to what we have fo
> > > ethernet,
> > > wireless, ppp, appletalk, etc.
> > >
> > > - The rx_handler uses gro_cells, which as I understand is meant
> > > for generic tunnelling setups and takes another loop through
> > > NAPI to aggregate data from multiple queues, but in case of
> > > IPA's single-queue receive calling gro directly would be
> > > simpler
> > > and more efficient.
> >
> > I have been planning to investigate some of the generic GRO
> > stuff for IPA but was going to wait on that until the basic
> > code was upstream.
>
> That's ok, that part can easily be changed after the fact, as it
> does not impact the user interface or the general design.
>
> > > From the overall design and the rmnet Kconfig description, it
> > > appears as though the intention as that rmnet could be a
> > > generic wrapper on top of any device, but from the
> > > implementation it seems that IPA is not actually usable that
> > > way and would always go through IPA.
> >
> > As far as I know *nothing* upstream currently uses rmnet; the
> > IPA driver will be the first, but as Bjorn said others seem to
> > be on the way. I'm not sure what you mean by "IPA is not
> > usable that way." Currently the IPA driver assumes a fixed
> > configuration, and that configuration assumes the use of QMAP,
> > and therefore assumes the rmnet driver is layered above it.
> > That doesn't preclude rmnet from using a different back end.
>
> Yes, that's what I meant above: IPA can only be used through
> rmnet (I wrote "through IPA", sorry for the typo), but cannot be
> used by itself.
>
> Arnd

2019-06-04 15:23:46

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Mon, 2019-06-03 at 13:04 -0600, Subash Abhinov Kasiviswanathan
wrote:
> > > I can't (or won't) comment right now on whether IPA needs its own
> > > netdev for rmnet to use. The IPA endpoints used for the modem
> > > network interfaces are enabled when the netdev is opened and
> > > disabled when closed. Outside of that, TX and RX are pretty
> > > much immediately passed through to the layer below or above.
> > > IPA currently has no other net device operations.
> >
> > I don't really have issues with the patchset underneath the netdev
> > layer. I'm interested in how the various bits present themselves to
> > userspace, which is why I am trying to tie this in with Johannes'
> > conversation about WWAN devices, netdevs, channels, and how the
> > various
> > drivers present API for creating data channels that map to
> > different
> > modem contexts.
> >
> > So let me rephrase. If the control plane has set up the default
> > context
> > and sent a QMI Start Network message (or the network attached the
> > default one) and the resulting IP details are applied to the IPA
> > netdev
> > can things just start sending data? Or do we need to create an
> > rmnet on
> > top to get that working?
> >
> > Dan
>
> Hi Dan
>
> All data from the hardware will have the MAP headers.
> We still need to create rmnet devs over the IPA netdev and use it
> for
> the
> data path.
> The IPA netdev will pass on the packets which it receives from the
> hardware
> and queue it to network stack where it will be intercepted by the
> rmnet rx handler.

Ok, so IPA only needs a netdev so that rmnet has something to
send/receive packets to/from? This gets even closer to the discussion
in "cellular modem driver APIs - take 2" from last week.

Dan

2019-06-04 20:27:18

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Tue, Jun 4, 2019 at 5:18 PM Dan Williams <[email protected]> wrote:
> On Tue, 2019-06-04 at 10:13 +0200, Arnd Bergmann wrote:
> >
> > Can you describe what kind of multiplexing is actually going on?
> > I'm still unclear about what we actually use multiple logical
> > interfaces for here, and how they relate to one another.
>
> Each logical interface represents a different "connection" (PDP/EPS
> context) to the provider network with a distinct IP address and QoS.
> VLANs may be a suitable analogy but here they are L3+QoS.
>
> In realistic example the main interface (say rmnet0) would be used for
> web browsing and have best-effort QoS. A second interface (say rmnet1)
> would be used for VOIP and have certain QoS guarantees from both the
> modem and the network itself.
>
> QMAP can also aggregate frames for a given channel (connection/EPS/PDP
> context/rmnet interface/etc) to better support LTE speeds.

Thanks, that's a very helpful explanation!

Is it correct to say then that the concept of having those separate
connections would be required for any proper LTE modem implementation,
but the QMAP protocol (and based on that, the rmnet implementation)
is Qualcomm specific and shared only among several generations of
modems from that one vendor?

You mentioned the need to have a common user space interface
for configuration, and if the above is true, I agree that we should try
to achieve that, either by ensuring rmnet is generic enough to
cover other vendors (and non-QMAP clients), or by creating a
new user level interface that IPA/rmnet can be adapted to.

Arnd

2019-06-04 21:30:53

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On Tue, 2019-06-04 at 22:04 +0200, Arnd Bergmann wrote:
> On Tue, Jun 4, 2019 at 5:18 PM Dan Williams <[email protected]> wrote:
> > On Tue, 2019-06-04 at 10:13 +0200, Arnd Bergmann wrote:
> > > Can you describe what kind of multiplexing is actually going on?
> > > I'm still unclear about what we actually use multiple logical
> > > interfaces for here, and how they relate to one another.
> >
> > Each logical interface represents a different "connection" (PDP/EPS
> > context) to the provider network with a distinct IP address and
> > QoS.
> > VLANs may be a suitable analogy but here they are L3+QoS.
> >
> > In realistic example the main interface (say rmnet0) would be used
> > for
> > web browsing and have best-effort QoS. A second interface (say
> > rmnet1)
> > would be used for VOIP and have certain QoS guarantees from both
> > the
> > modem and the network itself.
> >
> > QMAP can also aggregate frames for a given channel
> > (connection/EPS/PDP
> > context/rmnet interface/etc) to better support LTE speeds.
>
> Thanks, that's a very helpful explanation!
>
> Is it correct to say then that the concept of having those separate
> connections would be required for any proper LTE modem
> implementation,
> but the QMAP protocol (and based on that, the rmnet implementation)
> is Qualcomm specific and shared only among several generations of
> modems from that one vendor?

Exactly correct. This is what Johannes is discussing in his "cellular
modem APIs - take 2" thread about how this should all be organized at
the driver level and I think we should figure that out before we commit
to IPA-with-a-useless-netdev that requires rmnets to be created on top.
That may end up being the solution but let's have that discussion.

> > You mentioned the need to have a common user space interface
> for configuration, and if the above is true, I agree that we should
> try
> to achieve that, either by ensuring rmnet is generic enough to
> cover other vendors (and non-QMAP clients), or by creating a
> new user level interface that IPA/rmnet can be adapted to.

I would not suggest making rmnet generic; it's pretty QMAP specific
(but QMAP is spoken by many many modems both SoC, USB stick, and PCIe
minicard).

Instead, I think what Johannes is discussing is a better approach. A
kernel WWAN framework with consistent user API that
rmnet/IPA/qmi_wwan/MBIM/QMI/serial/Sierra can all implement.

That wouldn't affect the core packet processing of IPA/rmnet but
instead:

1) when/how an rmnet device actually gets created on top of the IPA (or
qmi_wwan) device

AND (one of these two)

a) whether IPA creates a netdev on probe

OR

b) whether there is some "WWAN device" kernel object which userspace
interacts with create rmnet channels on top of IPA

Dan

2019-06-06 21:12:23

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 6/4/19 4:29 PM, Dan Williams wrote:
> On Tue, 2019-06-04 at 22:04 +0200, Arnd Bergmann wrote:
>> On Tue, Jun 4, 2019 at 5:18 PM Dan Williams <[email protected]> wrote:
>>> On Tue, 2019-06-04 at 10:13 +0200, Arnd Bergmann wrote:
>>>> Can you describe what kind of multiplexing is actually going on?
>>>> I'm still unclear about what we actually use multiple logical
>>>> interfaces for here, and how they relate to one another.
>>>
>>> Each logical interface represents a different "connection" (PDP/EPS
>>> context) to the provider network with a distinct IP address and
>>> QoS.
>>> VLANs may be a suitable analogy but here they are L3+QoS.
>>>
>>> In realistic example the main interface (say rmnet0) would be used
>>> for
>>> web browsing and have best-effort QoS. A second interface (say
>>> rmnet1)
>>> would be used for VOIP and have certain QoS guarantees from both
>>> the
>>> modem and the network itself.
>>>
>>> QMAP can also aggregate frames for a given channel
>>> (connection/EPS/PDP
>>> context/rmnet interface/etc) to better support LTE speeds.
>>
>> Thanks, that's a very helpful explanation!
>>
>> Is it correct to say then that the concept of having those separate
>> connections would be required for any proper LTE modem
>> implementation,
>> but the QMAP protocol (and based on that, the rmnet implementation)
>> is Qualcomm specific and shared only among several generations of
>> modems from that one vendor?
>
> Exactly correct. This is what Johannes is discussing in his "cellular
> modem APIs - take 2" thread about how this should all be organized at
> the driver level and I think we should figure that out before we commit
> to IPA-with-a-useless-netdev that requires rmnets to be created on top.
> That may end up being the solution but let's have that discussion.

I looked at Johannes' message and the follow-on discussion. As I've
made clear before, my work on this has been focused on the IPA transport,
and some of this higher-level LTE architecture is new to me. But it
seems pretty clear that an abstracted WWAN subsystem is a good plan,
because these devices represent a superset of what a "normal" netdev
implements.

HOWEVER I disagree with your suggestion that the IPA code should
not be committed until after that is all sorted out. In part it's
for selfish reasons, but I think there are legitimate reasons to
commit IPA now *knowing* that it will need to be adapted to fit
into the generic model that gets defined and developed. Here
are some reasons why.

First, the layer of the IPA code that actually interacts with rmnet
is very small--less than 3% if you simply do a word count of the
source files. Arnd actually suggested eliminating the "ipa_netdev"
files and merging their content elsewhere. This suggests two things:
- The interface to rmnet is isolated, so the effect of whatever
updates are made to support a WWAN (rather than netdev) model will
be focused
- The vast majority of the driver has nothing to do with that upper
layer, and deals almost exclusively with managing the IPA hardware.
The idea of a generic framework isn't minor, but it isn't the
main focus of the IPA driver either, so I don't think it should
hold it up.

Second, the IPA code has been out for review recently, and has been
the subject of some detailed discussion in the past few weeks. Arnd
especially has invested considerable time in review and discussion.
Delaying things until after a better generic model is settled on
(which I'm guessing might be on the order of months) means the IPA
driver will have to be submitted for review again after that, at
which point it will no longer be "fresh"; it'll be a bit like
starting over.

Third, having the code upstream actually means the actual requirements
for rmnet-over-IPA are clear and explicit. This might not be a huge
deal, but I think it's better to devise a generic WWAN scheme that
can refer to actual code than to do so with assumptions about what
will work with rmnet (and others). As far as I know, the upstream
rmnet has no other upstream back end; IPA will make it "real."

I support the idea of developing a generic WWAN framework, and I
can assure you I'll be involved enough to perhaps be one of the
first to implement a new generic scheme.

Optimistically, the IPA code itself hasn't seen much feedback
for v2; maybe that means it's in good shape?

Anyway, I'd obviously like to get the IPA code accepted sooner
rather than later, and I think there are good reasons to do that.

-Alex

>>> You mentioned the need to have a common user space interface
>> for configuration, and if the above is true, I agree that we should
>> try
>> to achieve that, either by ensuring rmnet is generic enough to
>> cover other vendors (and non-QMAP clients), or by creating a
>> new user level interface that IPA/rmnet can be adapted to.
>
> I would not suggest making rmnet generic; it's pretty QMAP specific
> (but QMAP is spoken by many many modems both SoC, USB stick, and PCIe
> minicard).
>
> Instead, I think what Johannes is discussing is a better approach. A
> kernel WWAN framework with consistent user API that
> rmnet/IPA/qmi_wwan/MBIM/QMI/serial/Sierra can all implement.
>
> That wouldn't affect the core packet processing of IPA/rmnet but
> instead:
>
> 1) when/how an rmnet device actually gets created on top of the IPA (or
> qmi_wwan) device
>
> AND (one of these two)
>
> a) whether IPA creates a netdev on probe
>
> OR
>
> b) whether there is some "WWAN device" kernel object which userspace
> interacts with create rmnet channels on top of IPA
>
> Dan
>

2019-06-07 18:59:57

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 5/31/19 5:08 PM, Alex Elder wrote:
> On 5/31/19 4:12 PM, Arnd Bergmann wrote:
>> On Fri, May 31, 2019 at 10:47 PM Alex Elder <[email protected]> wrote:
>>> On 5/31/19 2:19 PM, Arnd Bergmann wrote:
>>>> On Fri, May 31, 2019 at 6:36 PM Alex Elder <[email protected]> wrote:
>>>>> On 5/31/19 9:58 AM, Dan Williams wrote:
>>>>>> On Thu, 2019-05-30 at 22:53 -0500, Alex Elder wrote:
>>>>
>>>> Does this mean that IPA can only be used to back rmnet, and rmnet
>>>> can only be used on top of IPA, or can or both of them be combined
>>>> with another driver to talk to instead?
>>>
>>> No it does not mean that.
>>>
>>> As I understand it, one reason for the rmnet layer was to abstract
>>> the back end, which would allow using a modem, or using something
>>> else (a LAN?), without exposing certain details of the hardware.
>>> (Perhaps to support multiplexing, etc. without duplicating that
>>> logic in two "back-end" drivers?)
>>>
>>> To be perfectly honest, at first I thought having IPA use rmnet
>>> was a cargo cult thing like Dan suggested, because I didn't see
>>> the benefit. I now see why one would use that pass-through layer
>>> to handle the QMAP features.
>>>
>>> But back to your question. The other thing is that I see no
>>> reason the IPA couldn't present a "normal" (non QMAP) interface
>>> for a modem. It's something I'd really like to be able to do,
>>> but I can't do it without having the modem firmware change its
>>> configuration for these endpoints. My access to the people who
>>> implement the modem firmware has been very limited (something
>>> I hope to improve), and unless and until I can get corresponding
>>> changes on the modem side to implement connections that don't
>>> use QMAP, I can't implement such a thing.
>>
>> Why would that require firmware changes? What I was thinking
>> here is to turn the bits of the rmnet driver that actually do anything
>> interesting on the headers into a library module (or a header file
>> with inline functions) that can be called directly by the ipa driver,
>> keeping the protocol unchanged.

OK, I follow up below, but now that I re-read this I think I
misunderstood what you were saying. I now think you were just
talking about having the QMAP parsing functionality provided
by rmnet be put into a library that the IPA driver (and others)
could use, rather than having them be two separate but stacked
drivers.

That seems like a very reasonable idea, and I don't think rmnet
does anything particularly special that would preclude it.
But:
- I have not reviewed the rmnet driver in any detail
- It's possible this wouldn't work with other back ends
- This would also need to be considered as part of any
effort to create a generic WWAN framework (though it
wouldn't pose any problem as far as I can tell)

> You know, it's possible you're right about not needing
> firmware changes. But it has always been my impression
> they would be needed. Here's why.

Now I'm just following up on the above statement.

First, what I was saying initially is that I would like to
be able to configure a simple network device that does not
implement any of the QMAP (rmnet) protocol. That would
allow IPA to be used standalone, without any need for what
the rmnet driver provides. This would suffice for the
current use, because the driver currently supports *only*
a single fixed-configuration data connection to the modem,
and has no need for the logical channels (or aggregation)
that QMAP provides.

I have not done that, because it would require both the AP
side and modem side endpoints to change their configuration
to *not* use QMAP. I can configure the AP endpoint, but I
have no control over how the modem configures its endpoint.

Something about your question made me think I might have
been misunderstanding that requirement though. I.e., I
thought it might be possible for the local (AP side)
endpoint to be configured to not use QMAP, regardless of
what the modem side does.

I didn't inquire, but I reviewed some documents, and I now
conclude that my previous understanding was correct. I
can't configure an AP endpoint to not use QMAP without also
having the modem configure its corresponding endpoint to
not use QMAP. The AP and modem need to agree on how their
respective "connected" endpoints are configured.

-Alex

> It looks like this:
>
> GSI Channel GSI Channel
> | |
> ---------- v ------- v -------------
> | AP (ep)|=======| IPA |=======|(ep) Modem |
> ---------- ------- -------------
>
> The AP and Modem each have IPA endpoints (ep), which use GSI channels,
> to communicate with the IPA. Each endpoint has configuration options
> (such as checksum offload). I *thought* that the configurations of
> the two endpoints need to be compatible (e.g., they need to agree on
> whether they're aggregating). But with your questioning I now think
> you may be right, that only the local endpoint's configuration matters.
>
> I will inquire further on this. I *know* that the AP and modem
> exchange some information about IPA configuration, but looking more
> closely that looks like it's all about the configuration of shared
> IPA resources, not endpoints.
>
> That said, the broader design (including the user space code)
> surely assumes rmnet, and I don't have any sense of what impact
> changing that would make. I am sure that changing it would not
> be well received.
>
> -Alex
>
>>>> Always passing data from one netdev to another both ways
>>>> sounds like it introduces both direct CPU overhead, and
>>>> problems with flow control when data gets buffered inbetween.
>>>
>>> My impression is the rmnet driver is a pretty thin layer,
>>> so the CPU overhead is probably not that great (though
>>> deaggregating a message is expensive). I agree with you
>>> on the flow control.
>>
>> The CPU overhead I mean is not from executing code in the
>> rmnet driver, but from passing packets through the network
>> stack between the two drivers, i.e. adding each frame to
>> a queue and taking it back out. I'm not sure how this ends
>> up working in reality but from a first look it seems like
>> we might bounce in an out of the softirq handler inbetween.
>>
>> Arnd
>>
>

2019-06-10 02:46:25

[permalink] [raw]

Subject: Re: [PATCH v2 00/17] net: introduce Qualcomm IPA driver

On 5/30/19 10:53 PM, Alex Elder wrote:
> This series presents the driver for the Qualcomm IP Accelerator (IPA).
>
> This is version 2 of the series. This version has addressed almost
> all of the feedback received in the first version:
> https://lore.kernel.org/lkml/[email protected]/
> More detail is included in the individual patches, but here is a
> high-level summary of what's changed since then:
> - Two spinlocks have been removed.
> - The code for enabling and disabling endpoint interrupts has
> been simplified considerably, and the spinlock is no longer
> required
> - A spinlock used when updating ring buffer pointers is no
> longer needed. Integers indexing the ring are used instead
> (and they don't even have to be atomic).
> - One spinlock remains to protect list updates, but it is always
> acquired using spin_lock_bh() (no more irqsave).
> - Information about the queueing and completion of messages is now
> supplied to the network stack in batches rather than one at a
> time.
> - I/O completion handling has been simplified, with the IRQ
> handler now consisting mainly of disabling the interrupt and
> calling napi_schedule().
> - Some comments have been updated and improved througout.
>
> What follows is the introduction supplied with v1 of the series.

Any more feedback? The only comment that I acted on is a trivial
suggestion from Dave Miller: change the types for the route_virt
and filter_virt fields of the ipa structure from void pointer to
u64 pointer. That required no other changes to the code.

At this point I plan to post a version 3 of this series in the
coming week and it will include just that one change. I might
do some comment updates before then as well.

But if anyone expects to provide any additional input on the
code in the near term, I can delay posting v3 until that has
been addressed. If this applies to you, please let me know.
(No pressure; things can always wait for v4...)

Thanks.

-Alex

> -----
>
> The IPA is a component present in some Qualcomm SoCs that allows
> network functions such as aggregation, filtering, routing, and NAT
> to be performed without active involvement of the main application
> processor (AP).
>
> Initially, these advanced features are disabled; the IPA driver
> simply provides a network interface that makes the modem's LTE
> network available to the AP. In addition, only support for the
> IPA found in the Qualcomm SDM845 SoC is provided.
>
> This code is derived from a driver developed internally by Qualcomm.
> A version of the original source can be seen here:
> https://source.codeaurora.org/quic/la/kernel/msm-4.9/tree
> in the "drivers/platform/msm/ipa" directory. Many were involved in
> developing this, but the following individuals deserve explicit
> acknowledgement for their substantial contributions:
>
> Abhishek Choubey
> Ady Abraham
> Chaitanya Pratapa
> David Arinzon
> Ghanim Fodi
> Gidon Studinski
> Ravi Gummadidala
> Shihuan Liu
> Skylar Chang
>
> A version of this code was posted in November 2018 as an RFC.
> https://lore.kernel.org/lkml/[email protected]/
> All feedback received was addressed. The code has undergone
> considerable further rework since that time, and most of the
> "future work" described then has now been completed.
>
> This code is available in buildable form here, based on kernel
> v5.2-rc1:
> remote: ssh://[email protected]/people/alex.elder/linux.git
> branch: ipa-v2_kernel-v5.2-rc2
> 75adf2ac1266 arm64: defconfig: enable build of IPA code
>
> The branch depends on a commit now found in in net-next. It has
> been cherry-picked, and (in this branch) has this commit ID:
> 13c627b5a078 net: qualcomm: rmnet: Move common struct definitions to include
> by
>
> -Alex
>
> Alex Elder (17):
> bitfield.h: add FIELD_MAX() and field_max()
> dt-bindings: soc: qcom: add IPA bindings
> soc: qcom: ipa: main code
> soc: qcom: ipa: configuration data
> soc: qcom: ipa: clocking, interrupts, and memory
> soc: qcom: ipa: GSI headers
> soc: qcom: ipa: the generic software interface
> soc: qcom: ipa: GSI transactions
> soc: qcom: ipa: IPA interface to GSI
> soc: qcom: ipa: IPA endpoints
> soc: qcom: ipa: immediate commands
> soc: qcom: ipa: IPA network device and microcontroller
> soc: qcom: ipa: AP/modem communications
> soc: qcom: ipa: support build of IPA code
> MAINTAINERS: add entry for the Qualcomm IPA driver
> arm64: dts: sdm845: add IPA information
> arm64: defconfig: enable build of IPA code
>
> .../devicetree/bindings/net/qcom,ipa.yaml | 180 ++
> MAINTAINERS | 6 +
> arch/arm64/boot/dts/qcom/sdm845.dtsi | 51 +
> arch/arm64/configs/defconfig | 1 +
> drivers/net/Kconfig | 2 +
> drivers/net/Makefile | 1 +
> drivers/net/ipa/Kconfig | 16 +
> drivers/net/ipa/Makefile | 7 +
> drivers/net/ipa/gsi.c | 1635 +++++++++++++++++
> drivers/net/ipa/gsi.h | 246 +++
> drivers/net/ipa/gsi_private.h | 148 ++
> drivers/net/ipa/gsi_reg.h | 376 ++++
> drivers/net/ipa/gsi_trans.c | 624 +++++++
> drivers/net/ipa/gsi_trans.h | 116 ++
> drivers/net/ipa/ipa.h | 131 ++
> drivers/net/ipa/ipa_clock.c | 297 +++
> drivers/net/ipa/ipa_clock.h | 52 +
> drivers/net/ipa/ipa_cmd.c | 377 ++++
> drivers/net/ipa/ipa_cmd.h | 116 ++
> drivers/net/ipa/ipa_data-sdm845.c | 245 +++
> drivers/net/ipa/ipa_data.h | 267 +++
> drivers/net/ipa/ipa_endpoint.c | 1283 +++++++++++++
> drivers/net/ipa/ipa_endpoint.h | 97 +
> drivers/net/ipa/ipa_gsi.c | 48 +
> drivers/net/ipa/ipa_gsi.h | 49 +
> drivers/net/ipa/ipa_interrupt.c | 279 +++
> drivers/net/ipa/ipa_interrupt.h | 53 +
> drivers/net/ipa/ipa_main.c | 921 ++++++++++
> drivers/net/ipa/ipa_mem.c | 234 +++
> drivers/net/ipa/ipa_mem.h | 83 +
> drivers/net/ipa/ipa_netdev.c | 251 +++
> drivers/net/ipa/ipa_netdev.h | 24 +
> drivers/net/ipa/ipa_qmi.c | 402 ++++
> drivers/net/ipa/ipa_qmi.h | 35 +
> drivers/net/ipa/ipa_qmi_msg.c | 583 ++++++
> drivers/net/ipa/ipa_qmi_msg.h | 238 +++
> drivers/net/ipa/ipa_reg.h | 279 +++
> drivers/net/ipa/ipa_smp2p.c | 304 +++
> drivers/net/ipa/ipa_smp2p.h | 47 +
> drivers/net/ipa/ipa_uc.c | 208 +++
> drivers/net/ipa/ipa_uc.h | 32 +
> include/linux/bitfield.h | 14 +
> 42 files changed, 10358 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/qcom,ipa.yaml
> create mode 100644 drivers/net/ipa/Kconfig
> create mode 100644 drivers/net/ipa/Makefile
> create mode 100644 drivers/net/ipa/gsi.c
> create mode 100644 drivers/net/ipa/gsi.h
> create mode 100644 drivers/net/ipa/gsi_private.h
> create mode 100644 drivers/net/ipa/gsi_reg.h
> create mode 100644 drivers/net/ipa/gsi_trans.c
> create mode 100644 drivers/net/ipa/gsi_trans.h
> create mode 100644 drivers/net/ipa/ipa.h
> create mode 100644 drivers/net/ipa/ipa_clock.c
> create mode 100644 drivers/net/ipa/ipa_clock.h
> create mode 100644 drivers/net/ipa/ipa_cmd.c
> create mode 100644 drivers/net/ipa/ipa_cmd.h
> create mode 100644 drivers/net/ipa/ipa_data-sdm845.c
> create mode 100644 drivers/net/ipa/ipa_data.h
> create mode 100644 drivers/net/ipa/ipa_endpoint.c
> create mode 100644 drivers/net/ipa/ipa_endpoint.h
> create mode 100644 drivers/net/ipa/ipa_gsi.c
> create mode 100644 drivers/net/ipa/ipa_gsi.h
> create mode 100644 drivers/net/ipa/ipa_interrupt.c
> create mode 100644 drivers/net/ipa/ipa_interrupt.h
> create mode 100644 drivers/net/ipa/ipa_main.c
> create mode 100644 drivers/net/ipa/ipa_mem.c
> create mode 100644 drivers/net/ipa/ipa_mem.h
> create mode 100644 drivers/net/ipa/ipa_netdev.c
> create mode 100644 drivers/net/ipa/ipa_netdev.h
> create mode 100644 drivers/net/ipa/ipa_qmi.c
> create mode 100644 drivers/net/ipa/ipa_qmi.h
> create mode 100644 drivers/net/ipa/ipa_qmi_msg.c
> create mode 100644 drivers/net/ipa/ipa_qmi_msg.h
> create mode 100644 drivers/net/ipa/ipa_reg.h
> create mode 100644 drivers/net/ipa/ipa_smp2p.c
> create mode 100644 drivers/net/ipa/ipa_smp2p.h
> create mode 100644 drivers/net/ipa/ipa_uc.c
> create mode 100644 drivers/net/ipa/ipa_uc.h
>

2019-06-10 22:09:43

by Rob Herring

[permalink] [raw]

Subject: Re: [PATCH v2 02/17] dt-bindings: soc: qcom: add IPA bindings

On Thu, May 30, 2019 at 9:53 PM Alex Elder <[email protected]> wrote:
>
> Add the binding definitions for the "qcom,ipa" device tree node.
>
> Signed-off-by: Alex Elder <[email protected]>
> ---
> .../devicetree/bindings/net/qcom,ipa.yaml | 180 ++++++++++++++++++
> 1 file changed, 180 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/qcom,ipa.yaml
>
> diff --git a/Documentation/devicetree/bindings/net/qcom,ipa.yaml b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
> new file mode 100644
> index 000000000000..0037fc278a61
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/qcom,ipa.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: GPL-2.0

New bindings are preferred to be dual GPL-2.0 and BSD-2-Clause. But
that's really a decision for the submitter.

Reviewed-by: Rob Herring <[email protected]>

2019-06-11 02:13:01