[net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[Jan Karcher]

Previously, there was no clean separation between SMC-D code and the ISM
device driver.This patch series addresses the situation to make ISM available
for uses outside of SMC-D.
In detail: SMC-D offers an interface via struct smcd_ops, which only the
ISM module implements so far. However, there is no real separation between
the smcd and ism modules, which starts right with the ISM device
initialization, which calls directly into the SMC-D code.
This patch series introduces a new API in the ISM module, which allows
registration of arbitrary clients via include/linux/ism.h: struct ism_client.
Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
dependencies on SMC-D in the device structure), and adds a number of API
calls for data transfers via ISM (see ism_register_dmb() & friends).
Still, the ISM module implements the SMC-D API, and therefore has a number
of internal helper functions for that matter.
Note that the ISM API is consciously kept thin for now (as compared to the
SMC-D API calls), as a number of API calls are only used with SMC-D and
hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.

v1 -> v2:
  Removed s390x dependency which broke config for other archs.

Stefan Raspl (8):
  net/smc: Terminate connections prior to device removal
  net/ism: Add missing calls to disable bus-mastering
  s390/ism: Introduce struct ism_dmb
  net/ism: Add new API for client registration
  net/smc: Register SMC-D as ISM client
  net/smc: Separate SMC-D and ISM APIs
  s390/ism: Consolidate SMC-D-related code
  net/smc: De-tangle ism and smc device initialization

 drivers/s390/net/ism.h     |  19 +-
 drivers/s390/net/ism_drv.c | 376 ++++++++++++++++++++++++++++++-------
 include/linux/ism.h        |  98 ++++++++++
 include/net/smc.h          |  24 +--
 net/smc/af_smc.c           |   9 +-
 net/smc/smc_clc.c          |  11 +-
 net/smc/smc_core.c         |  13 +-
 net/smc/smc_diag.c         |   3 +-
 net/smc/smc_ism.c          | 180 ++++++++++--------
 net/smc/smc_ism.h          |   3 +-
 net/smc/smc_pnet.c         |  40 ++--
 11 files changed, 560 insertions(+), 216 deletions(-)
 create mode 100644 include/linux/ism.h

-- 
2.25.1

[net-next v2 1/8] net/smc: Terminate connections prior to device removal

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

Removing an ISM device prior to terminating its associated connections
doesn't end well.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 net/smc/smc_ism.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index 911fe08bc54b..28e1641f990c 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -462,11 +462,11 @@ void smcd_unregister_dev(struct smcd_dev *smcd)
 {
 	pr_warn_ratelimited("smc: removing smcd device %s\n",
 			    dev_name(&smcd->dev));
+	smcd->going_away = 1;
+	smc_smcd_terminate_all(smcd);
 	mutex_lock(&smcd_dev_list.mutex);
 	list_del_init(&smcd->list);
 	mutex_unlock(&smcd_dev_list.mutex);
-	smcd->going_away = 1;
-	smc_smcd_terminate_all(smcd);
 	destroy_workqueue(smcd->event_wq);
 
 	device_del(&smcd->dev);
-- 
2.25.1

[net-next v2 2/8] net/ism: Add missing calls to disable bus-mastering

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism_drv.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index dfd401d9e362..e253949aa975 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -582,6 +582,7 @@ static int ism_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 err_free:
 	smcd_free_dev(ism->smcd);
 err_resource:
+	pci_clear_master(pdev);
 	pci_release_mem_regions(pdev);
 err_disable:
 	pci_disable_device(pdev);
@@ -612,6 +613,7 @@ static void ism_remove(struct pci_dev *pdev)
 	ism_dev_exit(ism);
 
 	smcd_free_dev(ism->smcd);
+	pci_clear_master(pdev);
 	pci_release_mem_regions(pdev);
 	pci_disable_device(pdev);
 	dev_set_drvdata(&pdev->dev, NULL);
-- 
2.25.1

[net-next v2 3/8] s390/ism: Introduce struct ism_dmb

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

Conceptually, a DMB is a structure that belongs to ISM devices. However,
SMC currently 'owns' this structure. So future exploiters of ISM devices
would be forced to include SMC headers to work - which is just weird.
Therefore, we switch ISM to struct ism_dmb, introduce a new public header
with the definition (will be populated with further API calls later on),
and, add a thin wrapper to please SMC. Since structs smcd_dmb and ism_dmb
are identical, we can simply convert between the two for now.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism.h     |  1 +
 drivers/s390/net/ism_drv.c | 22 ++++++++++++++++------
 include/linux/ism.h        | 23 +++++++++++++++++++++++
 3 files changed, 40 insertions(+), 6 deletions(-)
 create mode 100644 include/linux/ism.h

diff --git a/drivers/s390/net/ism.h b/drivers/s390/net/ism.h
index 38fe90c2597d..90af51370183 100644
--- a/drivers/s390/net/ism.h
+++ b/drivers/s390/net/ism.h
@@ -5,6 +5,7 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/pci.h>
+#include <linux/ism.h>
 #include <net/smc.h>
 #include <asm/pci_insn.h>
 
diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index e253949aa975..b9f33f411d78 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -215,14 +215,14 @@ static int ism_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid,
 	return ism_cmd(ism, &cmd);
 }
 
-static void ism_free_dmb(struct ism_dev *ism, struct smcd_dmb *dmb)
+static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	clear_bit(dmb->sba_idx, ism->sba_bitmap);
 	dma_free_coherent(&ism->pdev->dev, dmb->dmb_len,
 			  dmb->cpu_addr, dmb->dma_addr);
 }
 
-static int ism_alloc_dmb(struct ism_dev *ism, struct smcd_dmb *dmb)
+static int ism_alloc_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	unsigned long bit;
 
@@ -251,7 +251,7 @@ static int ism_alloc_dmb(struct ism_dev *ism, struct smcd_dmb *dmb)
 	return dmb->cpu_addr ? 0 : -ENOMEM;
 }
 
-static int ism_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
+static int ism_register_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
 {
 	struct ism_dev *ism = smcd->priv;
 	union ism_reg_dmb cmd;
@@ -282,7 +282,12 @@ static int ism_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
 	return ret;
 }
 
-static int ism_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
+static int smcd_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
+{
+	return ism_register_dmb(smcd, (struct ism_dmb *)dmb);
+}
+
+static int ism_unregister_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
 {
 	struct ism_dev *ism = smcd->priv;
 	union ism_unreg_dmb cmd;
@@ -303,6 +308,11 @@ static int ism_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
 	return ret;
 }
 
+static int smcd_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
+{
+	return ism_unregister_dmb(smcd, (struct ism_dmb *)dmb);
+}
+
 static int ism_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
 {
 	struct ism_dev *ism = smcd->priv;
@@ -475,8 +485,8 @@ static irqreturn_t ism_handle_irq(int irq, void *data)
 
 static const struct smcd_ops ism_ops = {
 	.query_remote_gid = ism_query_rgid,
-	.register_dmb = ism_register_dmb,
-	.unregister_dmb = ism_unregister_dmb,
+	.register_dmb = smcd_register_dmb,
+	.unregister_dmb = smcd_unregister_dmb,
 	.add_vlan_id = ism_add_vlan_id,
 	.del_vlan_id = ism_del_vlan_id,
 	.set_vlan_required = ism_set_vlan_required,
diff --git a/include/linux/ism.h b/include/linux/ism.h
new file mode 100644
index 000000000000..69bfbf0faaa1
--- /dev/null
+++ b/include/linux/ism.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *  Internal Shared Memory
+ *
+ *  Definitions for the ISM module
+ *
+ *  Copyright IBM Corp. 2022
+ */
+#ifndef _ISM_H
+#define _ISM_H
+
+struct ism_dmb {
+	u64 dmb_tok;
+	u64 rgid;
+	u32 dmb_len;
+	u32 sba_idx;
+	u32 vlan_valid;
+	u32 vlan_id;
+	void *cpu_addr;
+	dma_addr_t dma_addr;
+};
+
+#endif	/* _ISM_H */
-- 
2.25.1

[net-next v2 4/8] net/ism: Add new API for client registration

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

Add a new API that allows other drivers to concurrently access ISM devices.
To do so, we introduce a new API that allows other modules to register for
ISM device usage. Furthermore, we move the GID to struct ism, where it
belongs conceptually, and rename and relocate struct smcd_event to struct
ism_event.
This is the first part of a bigger overhaul of the interfaces between SMC
and ISM.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism.h     |  18 +---
 drivers/s390/net/ism_drv.c | 172 +++++++++++++++++++++++++++++++++++--
 include/linux/ism.h        |  67 +++++++++++++++
 include/net/smc.h          |  11 +--
 net/smc/smc_ism.c          |   4 +-
 5 files changed, 236 insertions(+), 36 deletions(-)

diff --git a/drivers/s390/net/ism.h b/drivers/s390/net/ism.h
index 90af51370183..70c5bbda0fea 100644
--- a/drivers/s390/net/ism.h
+++ b/drivers/s390/net/ism.h
@@ -16,7 +16,6 @@
  */
 #define ISM_DMB_WORD_OFFSET	1
 #define ISM_DMB_BIT_OFFSET	(ISM_DMB_WORD_OFFSET * 32)
-#define ISM_NR_DMBS		1920
 #define ISM_IDENT_MASK		0x00FFFF
 
 #define ISM_REG_SBA	0x1
@@ -178,7 +177,7 @@ struct ism_eq_header {
 
 struct ism_eq {
 	struct ism_eq_header header;
-	struct smcd_event entry[15];
+	struct ism_event entry[15];
 };
 
 struct ism_sba {
@@ -190,21 +189,6 @@ struct ism_sba {
 	u16 dmbe_mask[ISM_NR_DMBS];
 };
 
-struct ism_dev {
-	spinlock_t lock;
-	struct pci_dev *pdev;
-	struct smcd_dev *smcd;
-
-	struct ism_sba *sba;
-	dma_addr_t sba_dma_addr;
-	DECLARE_BITMAP(sba_bitmap, ISM_NR_DMBS);
-
-	struct ism_eq *ieq;
-	dma_addr_t ieq_dma_addr;
-
-	int ieq_idx;
-};
-
 #define ISM_CREATE_REQ(dmb, idx, sf, offset)		\
 	((dmb) | (idx) << 24 | (sf) << 23 | (offset))
 
diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index b9f33f411d78..24983224f47e 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -15,9 +15,6 @@
 #include <linux/err.h>
 #include <linux/ctype.h>
 #include <linux/processor.h>
-#include <net/smc.h>
-
-#include <asm/debug.h>
 
 #include "ism.h"
 
@@ -34,6 +31,84 @@ static const struct pci_device_id ism_device_table[] = {
 MODULE_DEVICE_TABLE(pci, ism_device_table);
 
 static debug_info_t *ism_debug_info;
+static const struct smcd_ops ism_ops;
+
+#define NO_CLIENT		0xff		/* must be >= MAX_CLIENTS */
+static struct ism_client *clients[MAX_CLIENTS];	/* use an array rather than */
+						/* a list for fast mapping  */
+static u8 max_client;
+static DEFINE_SPINLOCK(clients_lock);
+struct ism_dev_list {
+	struct list_head list;
+	struct mutex mutex; /* protects ism device list */
+};
+
+static struct ism_dev_list ism_dev_list = {
+	.list = LIST_HEAD_INIT(ism_dev_list.list),
+	.mutex = __MUTEX_INITIALIZER(ism_dev_list.mutex),
+};
+
+int ism_register_client(struct ism_client *client)
+{
+	struct ism_dev *ism;
+	unsigned long flags;
+	int i, rc = -ENOSPC;
+
+	mutex_lock(&ism_dev_list.mutex);
+	spin_lock_irqsave(&clients_lock, flags);
+	for (i = 0; i < MAX_CLIENTS; ++i) {
+		if (!clients[i]) {
+			clients[i] = client;
+			client->id = i;
+			if (i == max_client)
+				max_client++;
+			rc = 0;
+			break;
+		}
+	}
+	spin_unlock_irqrestore(&clients_lock, flags);
+	if (i < MAX_CLIENTS) {
+		/* initialize with all devices that we got so far */
+		list_for_each_entry(ism, &ism_dev_list.list, list) {
+			ism->priv[i] = NULL;
+			client->add(ism);
+		}
+	}
+	mutex_unlock(&ism_dev_list.mutex);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ism_register_client);
+
+int ism_unregister_client(struct ism_client *client)
+{
+	struct ism_dev *ism;
+	unsigned long flags;
+	int rc = 0;
+
+	mutex_lock(&ism_dev_list.mutex);
+	spin_lock_irqsave(&clients_lock, flags);
+	clients[client->id] = NULL;
+	if (client->id + 1 == max_client)
+		max_client--;
+	spin_unlock_irqrestore(&clients_lock, flags);
+	list_for_each_entry(ism, &ism_dev_list.list, list) {
+		for (int i = 0; i < ISM_NR_DMBS; ++i) {
+			if (ism->sba_client_arr[i] == client->id) {
+				pr_err("%s: attempt to unregister client '%s'"
+				       "with registered dmb(s)\n", __func__,
+				       client->name);
+				rc = -EBUSY;
+				goto out;
+			}
+		}
+	}
+out:
+	mutex_unlock(&ism_dev_list.mutex);
+
+	return rc;
+}
+EXPORT_SYMBOL_GPL(ism_unregister_client);
 
 static int ism_cmd(struct ism_dev *ism, void *cmd)
 {
@@ -193,7 +268,7 @@ static int ism_read_local_gid(struct ism_dev *ism)
 	if (ret)
 		goto out;
 
-	ism->smcd->local_gid = cmd.response.gid;
+	ism->local_gid = cmd.response.gid;
 out:
 	return ret;
 }
@@ -437,7 +512,8 @@ static u16 ism_get_chid(struct smcd_dev *smcd)
 
 static void ism_handle_event(struct ism_dev *ism)
 {
-	struct smcd_event *entry;
+	struct ism_event *entry;
+	int i;
 
 	while ((ism->ieq_idx + 1) != READ_ONCE(ism->ieq->header.idx)) {
 		if (++(ism->ieq_idx) == ARRAY_SIZE(ism->ieq->entry))
@@ -445,13 +521,18 @@ static void ism_handle_event(struct ism_dev *ism)
 
 		entry = &ism->ieq->entry[ism->ieq_idx];
 		debug_event(ism_debug_info, 2, entry, sizeof(*entry));
-		smcd_handle_event(ism->smcd, entry);
+		spin_lock(&clients_lock);
+		for (i = 0; i < max_client; ++i)
+			if (clients[i])
+				clients[i]->handle_event(ism, entry);
+		spin_unlock(&clients_lock);
 	}
 }
 
 static irqreturn_t ism_handle_irq(int irq, void *data)
 {
 	struct ism_dev *ism = data;
+	struct ism_client *clt;
 	unsigned long bit, end;
 	unsigned long *bv;
 	u16 dmbemask;
@@ -471,7 +552,8 @@ static irqreturn_t ism_handle_irq(int irq, void *data)
 		dmbemask = ism->sba->dmbe_mask[bit + ISM_DMB_BIT_OFFSET];
 		ism->sba->dmbe_mask[bit + ISM_DMB_BIT_OFFSET] = 0;
 		barrier();
-		smcd_handle_irq(ism->smcd, bit + ISM_DMB_BIT_OFFSET, dmbemask);
+		clt = clients[ism->sba_client_arr[bit]];
+		clt->handle_irq(ism, bit + ISM_DMB_BIT_OFFSET, dmbemask);
 	}
 
 	if (ism->sba->e) {
@@ -497,10 +579,21 @@ static const struct smcd_ops ism_ops = {
 	.get_chid = ism_get_chid,
 };
 
+static void ism_dev_add_work_func(struct work_struct *work)
+{
+	struct ism_client *client = container_of(work, struct ism_client,
+						 add_work);
+
+	client->add(client->tgt_ism);
+	atomic_dec(&client->tgt_ism->add_dev_cnt);
+	wake_up(&client->tgt_ism->waitq);
+}
+
 static int ism_dev_init(struct ism_dev *ism)
 {
 	struct pci_dev *pdev = ism->pdev;
-	int ret;
+	unsigned long flags;
+	int i, ret;
 
 	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
 	if (ret <= 0)
@@ -527,6 +620,28 @@ static int ism_dev_init(struct ism_dev *ism)
 		/* hardware is V2 capable */
 		ism_create_system_eid();
 
+	init_waitqueue_head(&ism->waitq);
+	atomic_set(&ism->free_clients_cnt, 0);
+	atomic_set(&ism->add_dev_cnt, 0);
+
+	wait_event(ism->waitq, !atomic_read(&ism->add_dev_cnt));
+	spin_lock_irqsave(&clients_lock, flags);
+	for (i = 0; i < max_client; ++i)
+		if (clients[i]) {
+			INIT_WORK(&clients[i]->add_work,
+				  ism_dev_add_work_func);
+			clients[i]->tgt_ism = ism;
+			atomic_inc(&ism->add_dev_cnt);
+			schedule_work(&clients[i]->add_work);
+		}
+	spin_unlock_irqrestore(&clients_lock, flags);
+
+	wait_event(ism->waitq, !atomic_read(&ism->add_dev_cnt));
+
+	mutex_lock(&ism_dev_list.mutex);
+	list_add(&ism->list, &ism_dev_list.list);
+	mutex_unlock(&ism_dev_list.mutex);
+
 	ret = smcd_register_dev(ism->smcd);
 	if (ret)
 		goto unreg_ieq;
@@ -602,9 +717,36 @@ static int ism_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return ret;
 }
 
+static void ism_dev_remove_work_func(struct work_struct *work)
+{
+	struct ism_client *client = container_of(work, struct ism_client,
+						 remove_work);
+
+	client->remove(client->tgt_ism);
+	atomic_dec(&client->tgt_ism->free_clients_cnt);
+	wake_up(&client->tgt_ism->waitq);
+}
+
+/* Callers must hold ism_dev_list.mutex */
 static void ism_dev_exit(struct ism_dev *ism)
 {
 	struct pci_dev *pdev = ism->pdev;
+	unsigned long flags;
+	int i;
+
+	wait_event(ism->waitq, !atomic_read(&ism->free_clients_cnt));
+	spin_lock_irqsave(&clients_lock, flags);
+	for (i = 0; i < max_client; ++i)
+		if (clients[i]) {
+			INIT_WORK(&clients[i]->remove_work,
+				  ism_dev_remove_work_func);
+			clients[i]->tgt_ism = ism;
+			atomic_inc(&ism->free_clients_cnt);
+			schedule_work(&clients[i]->remove_work);
+		}
+	spin_unlock_irqrestore(&clients_lock, flags);
+
+	wait_event(ism->waitq, !atomic_read(&ism->free_clients_cnt));
 
 	smcd_unregister_dev(ism->smcd);
 	if (SYSTEM_EID.serial_number[0] != '0' ||
@@ -614,18 +756,22 @@ static void ism_dev_exit(struct ism_dev *ism)
 	unregister_sba(ism);
 	free_irq(pci_irq_vector(pdev, 0), ism);
 	pci_free_irq_vectors(pdev);
+	list_del_init(&ism->list);
 }
 
 static void ism_remove(struct pci_dev *pdev)
 {
 	struct ism_dev *ism = dev_get_drvdata(&pdev->dev);
 
+	mutex_lock(&ism_dev_list.mutex);
 	ism_dev_exit(ism);
+	mutex_unlock(&ism_dev_list.mutex);
 
 	smcd_free_dev(ism->smcd);
 	pci_clear_master(pdev);
 	pci_release_mem_regions(pdev);
 	pci_disable_device(pdev);
+	device_del(&ism->dev);
 	dev_set_drvdata(&pdev->dev, NULL);
 	kfree(ism);
 }
@@ -645,6 +791,8 @@ static int __init ism_init(void)
 	if (!ism_debug_info)
 		return -ENODEV;
 
+	memset(clients, 0, sizeof(clients));
+	max_client = 0;
 	debug_register_view(ism_debug_info, &debug_hex_ascii_view);
 	ret = pci_register_driver(&ism_driver);
 	if (ret)
@@ -655,6 +803,14 @@ static int __init ism_init(void)
 
 static void __exit ism_exit(void)
 {
+	struct ism_dev *ism;
+
+	mutex_lock(&ism_dev_list.mutex);
+	list_for_each_entry(ism, &ism_dev_list.list, list) {
+		ism_dev_exit(ism);
+	}
+	mutex_unlock(&ism_dev_list.mutex);
+
 	pci_unregister_driver(&ism_driver);
 	debug_unregister(ism_debug_info);
 }
diff --git a/include/linux/ism.h b/include/linux/ism.h
index 69bfbf0faaa1..55c8ad306928 100644
--- a/include/linux/ism.h
+++ b/include/linux/ism.h
@@ -9,6 +9,8 @@
 #ifndef _ISM_H
 #define _ISM_H
 
+#include <linux/workqueue.h>
+
 struct ism_dmb {
 	u64 dmb_tok;
 	u64 rgid;
@@ -20,4 +22,69 @@ struct ism_dmb {
 	dma_addr_t dma_addr;
 };
 
+/* Unless we gain unexpected popularity, this limit should hold for a while */
+#define MAX_CLIENTS		8
+#define ISM_NR_DMBS		1920
+
+struct ism_dev {
+	spinlock_t lock; /* protects the ism device */
+	struct list_head list;
+	struct pci_dev *pdev;
+	struct smcd_dev *smcd;
+
+	struct ism_sba *sba;
+	dma_addr_t sba_dma_addr;
+	DECLARE_BITMAP(sba_bitmap, ISM_NR_DMBS);
+	u8 *sba_client_arr;	/* entries are indices into 'clients' array */
+	void *priv[MAX_CLIENTS];
+
+	struct ism_eq *ieq;
+	dma_addr_t ieq_dma_addr;
+
+	struct device dev;
+	u64 local_gid;
+	int ieq_idx;
+
+	atomic_t free_clients_cnt;
+	atomic_t add_dev_cnt;
+	wait_queue_head_t waitq;
+};
+
+struct ism_event {
+	u32 type;
+	u32 code;
+	u64 tok;
+	u64 time;
+	u64 info;
+};
+
+struct ism_client {
+	const char *name;
+	void (*add)(struct ism_dev *dev);
+	void (*remove)(struct ism_dev *dev);
+	void (*handle_event)(struct ism_dev *dev, struct ism_event *event);
+	/* Parameter dmbemask contains a bit vector with updated DMBEs, if sent
+	 * via ism_move_data(). Callback function must handle all active bits
+	 * indicated by dmbemask.
+	 */
+	void (*handle_irq)(struct ism_dev *dev, unsigned int bit, u16 dmbemask);
+	/* Private area - don't touch! */
+	struct work_struct remove_work;
+	struct work_struct add_work;
+	struct ism_dev *tgt_ism;
+	u8 id;
+};
+
+int ism_register_client(struct ism_client *client);
+int  ism_unregister_client(struct ism_client *client);
+static inline void *ism_get_priv(struct ism_dev *dev,
+				 struct ism_client *client) {
+	return dev->priv[client->id];
+}
+
+static inline void ism_set_priv(struct ism_dev *dev, struct ism_client *client,
+				void *priv) {
+	dev->priv[client->id] = priv;
+}
+
 #endif	/* _ISM_H */
diff --git a/include/net/smc.h b/include/net/smc.h
index c926d3313e05..98689b16b841 100644
--- a/include/net/smc.h
+++ b/include/net/smc.h
@@ -15,6 +15,7 @@
 #include <linux/spinlock.h>
 #include <linux/types.h>
 #include <linux/wait.h>
+#include "linux/ism.h"
 
 struct sock;
 
@@ -48,14 +49,6 @@ struct smcd_dmb {
 
 #define ISM_ERROR	0xFFFF
 
-struct smcd_event {
-	u32 type;
-	u32 code;
-	u64 tok;
-	u64 time;
-	u64 info;
-};
-
 struct smcd_dev;
 
 struct smcd_ops {
@@ -100,6 +93,6 @@ struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
 int smcd_register_dev(struct smcd_dev *smcd);
 void smcd_unregister_dev(struct smcd_dev *smcd);
 void smcd_free_dev(struct smcd_dev *smcd);
-void smcd_handle_event(struct smcd_dev *dev, struct smcd_event *event);
+void smcd_handle_event(struct smcd_dev *dev, struct ism_event *event);
 void smcd_handle_irq(struct smcd_dev *dev, unsigned int bit, u16 dmbemask);
 #endif	/* _SMC_H */
diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index 28e1641f990c..215409889872 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -296,7 +296,7 @@ int smcd_nl_get_device(struct sk_buff *skb, struct netlink_callback *cb)
 struct smc_ism_event_work {
 	struct work_struct work;
 	struct smcd_dev *smcd;
-	struct smcd_event event;
+	struct ism_event event;
 };
 
 #define ISM_EVENT_REQUEST		0x0001
@@ -490,7 +490,7 @@ EXPORT_SYMBOL_GPL(smcd_free_dev);
  * Context:
  * - Function called in IRQ context from ISM device driver event handler.
  */
-void smcd_handle_event(struct smcd_dev *smcd, struct smcd_event *event)
+void smcd_handle_event(struct smcd_dev *smcd, struct ism_event *event)
 {
 	struct smc_ism_event_work *wrk;
 
-- 
2.25.1

[net-next v2 5/8] net/smc: Register SMC-D as ISM client

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

Register the smc module with the new ism device driver API.
This is the second part of a bigger overhaul of the interfaces between SMC
and ISM.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism_drv.c |  5 ---
 include/net/smc.h          |  5 +--
 net/smc/af_smc.c           |  8 +++-
 net/smc/smc_core.c         |  1 +
 net/smc/smc_ism.c          | 82 +++++++++++++++++++++++++++-----------
 net/smc/smc_ism.h          |  3 +-
 6 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index 24983224f47e..f35c6077db04 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -642,10 +642,6 @@ static int ism_dev_init(struct ism_dev *ism)
 	list_add(&ism->list, &ism_dev_list.list);
 	mutex_unlock(&ism_dev_list.mutex);
 
-	ret = smcd_register_dev(ism->smcd);
-	if (ret)
-		goto unreg_ieq;
-
 	query_info(ism);
 	return 0;
 
@@ -748,7 +744,6 @@ static void ism_dev_exit(struct ism_dev *ism)
 
 	wait_event(ism->waitq, !atomic_read(&ism->free_clients_cnt));
 
-	smcd_unregister_dev(ism->smcd);
 	if (SYSTEM_EID.serial_number[0] != '0' ||
 	    SYSTEM_EID.type[0] != '0')
 		ism_del_vlan_id(ism->smcd, ISM_RESERVED_VLANID);
diff --git a/include/net/smc.h b/include/net/smc.h
index 98689b16b841..151aa54d9ad2 100644
--- a/include/net/smc.h
+++ b/include/net/smc.h
@@ -90,9 +90,6 @@ struct smcd_dev {
 
 struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
 				const struct smcd_ops *ops, int max_dmbs);
-int smcd_register_dev(struct smcd_dev *smcd);
-void smcd_unregister_dev(struct smcd_dev *smcd);
 void smcd_free_dev(struct smcd_dev *smcd);
-void smcd_handle_event(struct smcd_dev *dev, struct ism_event *event);
-void smcd_handle_irq(struct smcd_dev *dev, unsigned int bit, u16 dmbemask);
+
 #endif	/* _SMC_H */
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index e12d4fa5aece..5d037714ab78 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -3382,12 +3382,14 @@ static int __init smc_init(void)
 	if (rc)
 		goto out_pernet_subsys;
 
-	smc_ism_init();
+	rc = smc_ism_init();
+	if (rc)
+		goto out_pernet_subsys_stat;
 	smc_clc_init();
 
 	rc = smc_nl_init();
 	if (rc)
-		goto out_pernet_subsys_stat;
+		goto out_ism;
 
 	rc = smc_pnet_init();
 	if (rc)
@@ -3480,6 +3482,8 @@ static int __init smc_init(void)
 	smc_pnet_exit();
 out_nl:
 	smc_nl_exit();
+out_ism:
+	smc_ism_exit();
 out_pernet_subsys_stat:
 	unregister_pernet_subsys(&smc_net_stat_ops);
 out_pernet_subsys:
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index c305d8dd23f8..e15ee084cc5a 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -2595,6 +2595,7 @@ static int smc_core_reboot_event(struct notifier_block *this,
 {
 	smc_lgrs_shutdown();
 	smc_ib_unregister_client();
+	smc_ism_exit();
 	return 0;
 }
 
diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index 215409889872..6d31e9bbc5f9 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -17,6 +17,7 @@
 #include "smc_ism.h"
 #include "smc_pnet.h"
 #include "smc_netlink.h"
+#include "linux/ism.h"
 
 struct smcd_dev_list smcd_dev_list = {
 	.list = LIST_HEAD_INIT(smcd_dev_list.list),
@@ -26,6 +27,20 @@ struct smcd_dev_list smcd_dev_list = {
 static bool smc_ism_v2_capable;
 static u8 smc_ism_v2_system_eid[SMC_MAX_EID_LEN];
 
+static void smcd_register_dev(struct ism_dev *ism);
+static void smcd_unregister_dev(struct ism_dev *ism);
+static void smcd_handle_event(struct ism_dev *ism, struct ism_event *event);
+static void smcd_handle_irq(struct ism_dev *ism, unsigned int dmbno,
+			    u16 dmbemask);
+
+static struct ism_client smc_ism_client = {
+	.name = "SMC-D",
+	.add = smcd_register_dev,
+	.remove = smcd_unregister_dev,
+	.handle_event = smcd_handle_event,
+	.handle_irq = smcd_handle_irq,
+};
+
 /* Test if an ISM communication is possible - same CPC */
 int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *smcd)
 {
@@ -409,8 +424,6 @@ struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
 	device_initialize(&smcd->dev);
 	dev_set_name(&smcd->dev, name);
 	smcd->ops = ops;
-	if (smc_pnetid_by_dev_port(parent, 0, smcd->pnetid))
-		smc_pnetid_by_table_smcd(smcd);
 
 	spin_lock_init(&smcd->lock);
 	spin_lock_init(&smcd->lgr_lock);
@@ -421,9 +434,25 @@ struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
 }
 EXPORT_SYMBOL_GPL(smcd_alloc_dev);
 
-int smcd_register_dev(struct smcd_dev *smcd)
+void smcd_free_dev(struct smcd_dev *smcd)
 {
-	int rc;
+	put_device(&smcd->dev);
+}
+EXPORT_SYMBOL_GPL(smcd_free_dev);
+
+static void smcd_register_dev(struct ism_dev *ism)
+{
+	const struct smcd_ops *ops = NULL;
+	struct smcd_dev *smcd;
+
+	smcd = smcd_alloc_dev(&ism->pdev->dev, dev_name(&ism->pdev->dev), ops,
+			      ISM_NR_DMBS);
+	if (!smcd)
+		return;
+	smcd->priv = ism;
+	ism_set_priv(ism, &smc_ism_client, smcd);
+	if (smc_pnetid_by_dev_port(&ism->pdev->dev, 0, smcd->pnetid))
+		smc_pnetid_by_table_smcd(smcd);
 
 	mutex_lock(&smcd_dev_list.mutex);
 	if (list_empty(&smcd_dev_list.list)) {
@@ -447,19 +476,20 @@ int smcd_register_dev(struct smcd_dev *smcd)
 			    dev_name(&smcd->dev), smcd->pnetid,
 			    smcd->pnetid_by_user ? " (user defined)" : "");
 
-	rc = device_add(&smcd->dev);
-	if (rc) {
+	if (device_add(&smcd->dev)) {
 		mutex_lock(&smcd_dev_list.mutex);
 		list_del(&smcd->list);
 		mutex_unlock(&smcd_dev_list.mutex);
+		smcd_free_dev(smcd);
 	}
 
-	return rc;
+	return;
 }
-EXPORT_SYMBOL_GPL(smcd_register_dev);
 
-void smcd_unregister_dev(struct smcd_dev *smcd)
+static void smcd_unregister_dev(struct ism_dev *ism)
 {
+	struct smcd_dev *smcd = ism_get_priv(ism, &smc_ism_client);
+
 	pr_warn_ratelimited("smc: removing smcd device %s\n",
 			    dev_name(&smcd->dev));
 	smcd->going_away = 1;
@@ -471,16 +501,9 @@ void smcd_unregister_dev(struct smcd_dev *smcd)
 
 	device_del(&smcd->dev);
 }
-EXPORT_SYMBOL_GPL(smcd_unregister_dev);
-
-void smcd_free_dev(struct smcd_dev *smcd)
-{
-	put_device(&smcd->dev);
-}
-EXPORT_SYMBOL_GPL(smcd_free_dev);
 
 /* SMCD Device event handler. Called from ISM device interrupt handler.
- * Parameters are smcd device pointer,
+ * Parameters are ism device pointer,
  * - event->type (0 --> DMB, 1 --> GID),
  * - event->code (event code),
  * - event->tok (either DMB token when event type 0, or GID when event type 1)
@@ -490,8 +513,9 @@ EXPORT_SYMBOL_GPL(smcd_free_dev);
  * Context:
  * - Function called in IRQ context from ISM device driver event handler.
  */
-void smcd_handle_event(struct smcd_dev *smcd, struct ism_event *event)
+static void smcd_handle_event(struct ism_dev *ism, struct ism_event *event)
 {
+	struct smcd_dev *smcd = ism_get_priv(ism, &smc_ism_client);
 	struct smc_ism_event_work *wrk;
 
 	if (smcd->going_away)
@@ -505,17 +529,18 @@ void smcd_handle_event(struct smcd_dev *smcd, struct ism_event *event)
 	wrk->event = *event;
 	queue_work(smcd->event_wq, &wrk->work);
 }
-EXPORT_SYMBOL_GPL(smcd_handle_event);
 
 /* SMCD Device interrupt handler. Called from ISM device interrupt handler.
- * Parameters are smcd device pointer, DMB number, and the DMBE bitmask.
+ * Parameters are the ism device pointer, DMB number, and the DMBE bitmask.
  * Find the connection and schedule the tasklet for this connection.
  *
  * Context:
  * - Function called in IRQ context from ISM device driver IRQ handler.
  */
-void smcd_handle_irq(struct smcd_dev *smcd, unsigned int dmbno, u16 dmbemask)
+static void smcd_handle_irq(struct ism_dev *ism, unsigned int dmbno,
+			    u16 dmbemask)
 {
+	struct smcd_dev *smcd = ism_get_priv(ism, &smc_ism_client);
 	struct smc_connection *conn = NULL;
 	unsigned long flags;
 
@@ -525,10 +550,21 @@ void smcd_handle_irq(struct smcd_dev *smcd, unsigned int dmbno, u16 dmbemask)
 		tasklet_schedule(&conn->rx_tsklet);
 	spin_unlock_irqrestore(&smcd->lock, flags);
 }
-EXPORT_SYMBOL_GPL(smcd_handle_irq);
 
-void __init smc_ism_init(void)
+int smc_ism_init(void)
 {
 	smc_ism_v2_capable = false;
 	memset(smc_ism_v2_system_eid, 0, SMC_MAX_EID_LEN);
+#if IS_ENABLED(CONFIG_ISM)
+	return ism_register_client(&smc_ism_client);
+#else
+	return 0;
+#endif
+}
+
+void smc_ism_exit(void)
+{
+#if IS_ENABLED(CONFIG_ISM)
+	ism_unregister_client(&smc_ism_client);
+#endif
 }
diff --git a/net/smc/smc_ism.h b/net/smc/smc_ism.h
index d6b2db604fe8..832b2f42d79f 100644
--- a/net/smc/smc_ism.h
+++ b/net/smc/smc_ism.h
@@ -42,7 +42,8 @@ int smc_ism_signal_shutdown(struct smc_link_group *lgr);
 void smc_ism_get_system_eid(u8 **eid);
 u16 smc_ism_get_chid(struct smcd_dev *dev);
 bool smc_ism_is_v2_capable(void);
-void smc_ism_init(void);
+int smc_ism_init(void);
+void smc_ism_exit(void);
 int smcd_nl_get_device(struct sk_buff *skb, struct netlink_callback *cb);
 
 static inline int smc_ism_write(struct smcd_dev *smcd, u64 dmb_tok,
-- 
2.25.1

[net-next v2 6/8] net/smc: Separate SMC-D and ISM APIs

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

We separate the code implementing the struct smcd_ops API in the ISM
device driver from the functions that may be used by other exploiters of
ISM devices.
Note: We start out small, and don't offer the whole breadth of the ISM
device for public use, as many functions are specific to or likely only
ever used in the context of SMC-D.
This is the third part of a bigger overhaul of the interfaces between SMC
and ISM.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism_drv.c | 92 ++++++++++++++++++++++++++------------
 include/linux/ism.h        |  7 +++
 include/net/smc.h          |  3 +-
 net/smc/smc_clc.c          | 11 +++--
 net/smc/smc_core.c         |  6 ++-
 net/smc/smc_diag.c         |  3 +-
 6 files changed, 86 insertions(+), 36 deletions(-)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index f35c6077db04..e6c810a96b24 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -273,10 +273,9 @@ static int ism_read_local_gid(struct ism_dev *ism)
 	return ret;
 }
 
-static int ism_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid,
+static int ism_query_rgid(struct ism_dev *ism, u64 rgid, u32 vid_valid,
 			  u32 vid)
 {
-	struct ism_dev *ism = smcd->priv;
 	union ism_query_rgid cmd;
 
 	memset(&cmd, 0, sizeof(cmd));
@@ -290,6 +289,11 @@ static int ism_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid,
 	return ism_cmd(ism, &cmd);
 }
 
+static int smcd_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid, u32 vid)
+{
+	return ism_query_rgid(smcd->priv, rgid, vid_valid, vid);
+}
+
 static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	clear_bit(dmb->sba_idx, ism->sba_bitmap);
@@ -326,9 +330,9 @@ static int ism_alloc_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 	return dmb->cpu_addr ? 0 : -ENOMEM;
 }
 
-static int ism_register_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
+int ism_register_dmb(struct ism_dev *ism, struct ism_dmb *dmb,
+		     struct ism_client *client)
 {
-	struct ism_dev *ism = smcd->priv;
 	union ism_reg_dmb cmd;
 	int ret;
 
@@ -353,18 +357,19 @@ static int ism_register_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
 		goto out;
 	}
 	dmb->dmb_tok = cmd.response.dmb_tok;
+	ism->sba_client_arr[dmb->sba_idx - ISM_DMB_BIT_OFFSET] = client->id;
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ism_register_dmb);
 
 static int smcd_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
 {
-	return ism_register_dmb(smcd, (struct ism_dmb *)dmb);
+	return ism_register_dmb(smcd->priv, (struct ism_dmb *)dmb, NULL);
 }
 
-static int ism_unregister_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
+int ism_unregister_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
-	struct ism_dev *ism = smcd->priv;
 	union ism_unreg_dmb cmd;
 	int ret;
 
@@ -374,6 +379,8 @@ static int ism_unregister_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
 
 	cmd.request.dmb_tok = dmb->dmb_tok;
 
+	ism->sba_client_arr[dmb->sba_idx - ISM_DMB_BIT_OFFSET] = NO_CLIENT;
+
 	ret = ism_cmd(ism, &cmd);
 	if (ret && ret != ISM_ERROR)
 		goto out;
@@ -382,15 +389,15 @@ static int ism_unregister_dmb(struct smcd_dev *smcd, struct ism_dmb *dmb)
 out:
 	return ret;
 }
+EXPORT_SYMBOL_GPL(ism_unregister_dmb);
 
 static int smcd_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
 {
-	return ism_unregister_dmb(smcd, (struct ism_dmb *)dmb);
+	return ism_unregister_dmb(smcd->priv, (struct ism_dmb *)dmb);
 }
 
-static int ism_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+static int ism_add_vlan_id(struct ism_dev *ism, u64 vlan_id)
 {
-	struct ism_dev *ism = smcd->priv;
 	union ism_set_vlan_id cmd;
 
 	memset(&cmd, 0, sizeof(cmd));
@@ -402,9 +409,13 @@ static int ism_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
 	return ism_cmd(ism, &cmd);
 }
 
-static int ism_del_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+static int smcd_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+{
+	return ism_add_vlan_id(smcd->priv, vlan_id);
+}
+
+static int ism_del_vlan_id(struct ism_dev *ism, u64 vlan_id)
 {
-	struct ism_dev *ism = smcd->priv;
 	union ism_set_vlan_id cmd;
 
 	memset(&cmd, 0, sizeof(cmd));
@@ -416,6 +427,11 @@ static int ism_del_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
 	return ism_cmd(ism, &cmd);
 }
 
+static int smcd_del_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+{
+	return ism_del_vlan_id(smcd->priv, vlan_id);
+}
+
 static int ism_set_vlan_required(struct smcd_dev *smcd)
 {
 	return ism_cmd_simple(smcd->priv, ISM_SET_VLAN);
@@ -426,8 +442,8 @@ static int ism_reset_vlan_required(struct smcd_dev *smcd)
 	return ism_cmd_simple(smcd->priv, ISM_RESET_VLAN);
 }
 
-static int ism_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq,
-			  u32 event_code, u64 info)
+static int smcd_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq,
+			   u32 event_code, u64 info)
 {
 	struct ism_dev *ism = smcd->priv;
 	union ism_sig_ieq cmd;
@@ -450,8 +466,9 @@ static unsigned int max_bytes(unsigned int start, unsigned int len,
 	return min(boundary - (start & (boundary - 1)), len);
 }
 
-static int ism_move(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx,
-		    bool sf, unsigned int offset, void *data, unsigned int size)
+static int smcd_move(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx,
+		     bool sf, unsigned int offset, void *data,
+		     unsigned int size)
 {
 	struct ism_dev *ism = smcd->priv;
 	unsigned int bytes;
@@ -495,14 +512,15 @@ static void ism_create_system_eid(void)
 	memcpy(&SYSTEM_EID.type, tmp, 4);
 }
 
-static u8 *ism_get_system_eid(void)
+u8 *ism_get_seid(void)
 {
 	return SYSTEM_EID.seid_string;
 }
+EXPORT_SYMBOL_GPL(ism_get_seid);
 
-static u16 ism_get_chid(struct smcd_dev *smcd)
+static u16 smcd_get_chid(struct smcd_dev *smcd)
 {
-	struct ism_dev *ism = (struct ism_dev *)smcd->priv;
+	struct ism_dev *ism = smcd->priv;
 
 	if (!ism || !ism->pdev)
 		return 0;
@@ -565,18 +583,26 @@ static irqreturn_t ism_handle_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+static u64 smcd_get_local_gid(struct smcd_dev *smcd)
+{
+	struct ism_dev *ism = smcd->priv;
+
+	return ism->local_gid;
+}
+
 static const struct smcd_ops ism_ops = {
-	.query_remote_gid = ism_query_rgid,
+	.query_remote_gid = smcd_query_rgid,
 	.register_dmb = smcd_register_dmb,
 	.unregister_dmb = smcd_unregister_dmb,
-	.add_vlan_id = ism_add_vlan_id,
-	.del_vlan_id = ism_del_vlan_id,
+	.add_vlan_id = smcd_add_vlan_id,
+	.del_vlan_id = smcd_del_vlan_id,
 	.set_vlan_required = ism_set_vlan_required,
 	.reset_vlan_required = ism_reset_vlan_required,
-	.signal_event = ism_signal_ieq,
-	.move_data = ism_move,
-	.get_system_eid = ism_get_system_eid,
-	.get_chid = ism_get_chid,
+	.signal_event = smcd_signal_ieq,
+	.move_data = smcd_move,
+	.get_system_eid = ism_get_seid,
+	.get_local_gid = smcd_get_local_gid,
+	.get_chid = smcd_get_chid,
 };
 
 static void ism_dev_add_work_func(struct work_struct *work)
@@ -599,10 +625,15 @@ static int ism_dev_init(struct ism_dev *ism)
 	if (ret <= 0)
 		goto out;
 
+	ism->sba_client_arr = kzalloc(ISM_NR_DMBS, GFP_KERNEL);
+	if (!ism->sba_client_arr)
+		goto free_vectors;
+	memset(ism->sba_client_arr, NO_CLIENT, ISM_NR_DMBS);
+
 	ret = request_irq(pci_irq_vector(pdev, 0), ism_handle_irq, 0,
 			  pci_name(pdev), ism);
 	if (ret)
-		goto free_vectors;
+		goto free_client_arr;
 
 	ret = register_sba(ism);
 	if (ret)
@@ -616,7 +647,7 @@ static int ism_dev_init(struct ism_dev *ism)
 	if (ret)
 		goto unreg_ieq;
 
-	if (!ism_add_vlan_id(ism->smcd, ISM_RESERVED_VLANID))
+	if (!ism_add_vlan_id(ism, ISM_RESERVED_VLANID))
 		/* hardware is V2 capable */
 		ism_create_system_eid();
 
@@ -651,6 +682,8 @@ static int ism_dev_init(struct ism_dev *ism)
 	unregister_sba(ism);
 free_irq:
 	free_irq(pci_irq_vector(pdev, 0), ism);
+free_client_arr:
+	kfree(ism->sba_client_arr);
 free_vectors:
 	pci_free_irq_vectors(pdev);
 out:
@@ -746,10 +779,11 @@ static void ism_dev_exit(struct ism_dev *ism)
 
 	if (SYSTEM_EID.serial_number[0] != '0' ||
 	    SYSTEM_EID.type[0] != '0')
-		ism_del_vlan_id(ism->smcd, ISM_RESERVED_VLANID);
+		ism_del_vlan_id(ism, ISM_RESERVED_VLANID);
 	unregister_ieq(ism);
 	unregister_sba(ism);
 	free_irq(pci_irq_vector(pdev, 0), ism);
+	kfree(ism->sba_client_arr);
 	pci_free_irq_vectors(pdev);
 	list_del_init(&ism->list);
 }
diff --git a/include/linux/ism.h b/include/linux/ism.h
index 55c8ad306928..bdd29e08d4fe 100644
--- a/include/linux/ism.h
+++ b/include/linux/ism.h
@@ -87,4 +87,11 @@ static inline void ism_set_priv(struct ism_dev *dev, struct ism_client *client,
 	dev->priv[client->id] = priv;
 }
 
+int  ism_register_dmb(struct ism_dev *dev, struct ism_dmb *dmb,
+		      struct ism_client *client);
+int  ism_unregister_dmb(struct ism_dev *dev, struct ism_dmb *dmb);
+int  ism_move(struct ism_dev *dev, u64 dmb_tok, unsigned int idx, bool sf,
+	      unsigned int offset, void *data, unsigned int size);
+u8  *ism_get_seid(void);
+
 #endif	/* _ISM_H */
diff --git a/include/net/smc.h b/include/net/smc.h
index 151aa54d9ad2..d5f8f18169d7 100644
--- a/include/net/smc.h
+++ b/include/net/smc.h
@@ -66,14 +66,15 @@ struct smcd_ops {
 			 bool sf, unsigned int offset, void *data,
 			 unsigned int size);
 	u8* (*get_system_eid)(void);
+	u64 (*get_local_gid)(struct smcd_dev *dev);
 	u16 (*get_chid)(struct smcd_dev *dev);
 };
 
 struct smcd_dev {
 	const struct smcd_ops *ops;
 	struct device dev;
+	struct ism_dev *ism;
 	void *priv;
-	u64 local_gid;
 	struct list_head list;
 	spinlock_t lock;
 	struct smc_connection **conn;
diff --git a/net/smc/smc_clc.c b/net/smc/smc_clc.c
index dfb9797f7bc6..b9b8b07aa702 100644
--- a/net/smc/smc_clc.c
+++ b/net/smc/smc_clc.c
@@ -813,6 +813,7 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
 	struct smc_clc_v2_extension *v2_ext;
 	struct smc_clc_msg_smcd *pclc_smcd;
 	struct smc_clc_msg_trail *trl;
+	struct smcd_dev *smcd;
 	int len, i, plen, rc;
 	int reason_code = 0;
 	struct kvec vec[8];
@@ -868,7 +869,9 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
 	if (smcd_indicated(ini->smc_type_v1)) {
 		/* add SMC-D specifics */
 		if (ini->ism_dev[0]) {
-			pclc_smcd->ism.gid = htonll(ini->ism_dev[0]->local_gid);
+			smcd = ini->ism_dev[0];
+			pclc_smcd->ism.gid =
+				htonll(smcd->ops->get_local_gid(smcd));
 			pclc_smcd->ism.chid =
 				htons(smc_ism_get_chid(ini->ism_dev[0]));
 		}
@@ -914,8 +917,9 @@ int smc_clc_send_proposal(struct smc_sock *smc, struct smc_init_info *ini)
 		plen += sizeof(*smcd_v2_ext);
 		if (ini->ism_offered_cnt) {
 			for (i = 1; i <= ini->ism_offered_cnt; i++) {
+				smcd = ini->ism_dev[i];
 				gidchids[i - 1].gid =
-					htonll(ini->ism_dev[i]->local_gid);
+					htonll(smcd->ops->get_local_gid(smcd));
 				gidchids[i - 1].chid =
 					htons(smc_ism_get_chid(ini->ism_dev[i]));
 			}
@@ -1000,7 +1004,8 @@ static int smc_clc_send_confirm_accept(struct smc_sock *smc,
 		memcpy(clc->hdr.eyecatcher, SMCD_EYECATCHER,
 		       sizeof(SMCD_EYECATCHER));
 		clc->hdr.typev1 = SMC_TYPE_D;
-		clc->d0.gid = conn->lgr->smcd->local_gid;
+		clc->d0.gid =
+			conn->lgr->smcd->ops->get_local_gid(conn->lgr->smcd);
 		clc->d0.token = conn->rmb_desc->token;
 		clc->d0.dmbe_size = conn->rmbe_size_short;
 		clc->d0.dmbe_idx = 0;
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index e15ee084cc5a..ec04966e9bf9 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -500,6 +500,7 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr,
 				struct netlink_callback *cb)
 {
 	char smc_pnet[SMC_MAX_PNETID_LEN + 1];
+	struct smcd_dev *smcd = lgr->smcd;
 	struct nlattr *attrs;
 	void *nlh;
 
@@ -515,8 +516,9 @@ static int smc_nl_fill_smcd_lgr(struct smc_link_group *lgr,
 
 	if (nla_put_u32(skb, SMC_NLA_LGR_D_ID, *((u32 *)&lgr->id)))
 		goto errattr;
-	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_GID, lgr->smcd->local_gid,
-			      SMC_NLA_LGR_D_PAD))
+	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_GID,
+			      smcd->ops->get_local_gid(smcd),
+				  SMC_NLA_LGR_D_PAD))
 		goto errattr;
 	if (nla_put_u64_64bit(skb, SMC_NLA_LGR_D_PEER_GID, lgr->peer_gid,
 			      SMC_NLA_LGR_D_PAD))
diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index 80ea7d954ece..7ff2152971a5 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -167,12 +167,13 @@ static int __smc_diag_dump(struct sock *sk, struct sk_buff *skb,
 	    !list_empty(&smc->conn.lgr->list)) {
 		struct smc_connection *conn = &smc->conn;
 		struct smcd_diag_dmbinfo dinfo;
+		struct smcd_dev *smcd = conn->lgr->smcd;
 
 		memset(&dinfo, 0, sizeof(dinfo));
 
 		dinfo.linkid = *((u32 *)conn->lgr->id);
 		dinfo.peer_gid = conn->lgr->peer_gid;
-		dinfo.my_gid = conn->lgr->smcd->local_gid;
+		dinfo.my_gid = smcd->ops->get_local_gid(smcd);
 		dinfo.token = conn->rmb_desc->token;
 		dinfo.peer_token = conn->peer_token;
 
-- 
2.25.1

[net-next v2 7/8] s390/ism: Consolidate SMC-D-related code

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

The ism module had SMC-D-specific code sprinkled across the entire module.
We are now consolidating the SMC-D-specific parts into the latter parts
of the module, so it becomes more clear what code is intended for use with
ISM, and which parts are glue code for usage in the context of SMC-D.
This is the fourth part of a bigger overhaul of the interfaces between SMC
and ISM.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism_drv.c | 162 ++++++++++++++++++++++---------------
 include/linux/ism.h        |   2 +
 include/net/smc.h          |   5 +-
 net/smc/smc_ism.c          |  63 +++++++++------
 4 files changed, 143 insertions(+), 89 deletions(-)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index e6c810a96b24..73c8f42a22a7 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -289,11 +289,6 @@ static int ism_query_rgid(struct ism_dev *ism, u64 rgid, u32 vid_valid,
 	return ism_cmd(ism, &cmd);
 }
 
-static int smcd_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid, u32 vid)
-{
-	return ism_query_rgid(smcd->priv, rgid, vid_valid, vid);
-}
-
 static void ism_free_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	clear_bit(dmb->sba_idx, ism->sba_bitmap);
@@ -363,11 +358,6 @@ int ism_register_dmb(struct ism_dev *ism, struct ism_dmb *dmb,
 }
 EXPORT_SYMBOL_GPL(ism_register_dmb);
 
-static int smcd_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
-{
-	return ism_register_dmb(smcd->priv, (struct ism_dmb *)dmb, NULL);
-}
-
 int ism_unregister_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 {
 	union ism_unreg_dmb cmd;
@@ -391,11 +381,6 @@ int ism_unregister_dmb(struct ism_dev *ism, struct ism_dmb *dmb)
 }
 EXPORT_SYMBOL_GPL(ism_unregister_dmb);
 
-static int smcd_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
-{
-	return ism_unregister_dmb(smcd->priv, (struct ism_dmb *)dmb);
-}
-
 static int ism_add_vlan_id(struct ism_dev *ism, u64 vlan_id)
 {
 	union ism_set_vlan_id cmd;
@@ -409,11 +394,6 @@ static int ism_add_vlan_id(struct ism_dev *ism, u64 vlan_id)
 	return ism_cmd(ism, &cmd);
 }
 
-static int smcd_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
-{
-	return ism_add_vlan_id(smcd->priv, vlan_id);
-}
-
 static int ism_del_vlan_id(struct ism_dev *ism, u64 vlan_id)
 {
 	union ism_set_vlan_id cmd;
@@ -427,25 +407,9 @@ static int ism_del_vlan_id(struct ism_dev *ism, u64 vlan_id)
 	return ism_cmd(ism, &cmd);
 }
 
-static int smcd_del_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
-{
-	return ism_del_vlan_id(smcd->priv, vlan_id);
-}
-
-static int ism_set_vlan_required(struct smcd_dev *smcd)
+static int ism_signal_ieq(struct ism_dev *ism, u64 rgid, u32 trigger_irq,
+			  u32 event_code, u64 info)
 {
-	return ism_cmd_simple(smcd->priv, ISM_SET_VLAN);
-}
-
-static int ism_reset_vlan_required(struct smcd_dev *smcd)
-{
-	return ism_cmd_simple(smcd->priv, ISM_RESET_VLAN);
-}
-
-static int smcd_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq,
-			   u32 event_code, u64 info)
-{
-	struct ism_dev *ism = smcd->priv;
 	union ism_sig_ieq cmd;
 
 	memset(&cmd, 0, sizeof(cmd));
@@ -466,11 +430,9 @@ static unsigned int max_bytes(unsigned int start, unsigned int len,
 	return min(boundary - (start & (boundary - 1)), len);
 }
 
-static int smcd_move(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx,
-		     bool sf, unsigned int offset, void *data,
-		     unsigned int size)
+int ism_move(struct ism_dev *ism, u64 dmb_tok, unsigned int idx, bool sf,
+	     unsigned int offset, void *data, unsigned int size)
 {
-	struct ism_dev *ism = smcd->priv;
 	unsigned int bytes;
 	u64 dmb_req;
 	int ret;
@@ -491,6 +453,7 @@ static int smcd_move(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(ism_move);
 
 static struct ism_systemeid SYSTEM_EID = {
 	.seid_string = "IBM-SYSZ-ISMSEID00000000",
@@ -518,10 +481,8 @@ u8 *ism_get_seid(void)
 }
 EXPORT_SYMBOL_GPL(ism_get_seid);
 
-static u16 smcd_get_chid(struct smcd_dev *smcd)
+static u16 ism_get_chid(struct ism_dev *ism)
 {
-	struct ism_dev *ism = smcd->priv;
-
 	if (!ism || !ism->pdev)
 		return 0;
 
@@ -583,28 +544,11 @@ static irqreturn_t ism_handle_irq(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
-static u64 smcd_get_local_gid(struct smcd_dev *smcd)
+static u64 ism_get_local_gid(struct ism_dev *ism)
 {
-	struct ism_dev *ism = smcd->priv;
-
 	return ism->local_gid;
 }
 
-static const struct smcd_ops ism_ops = {
-	.query_remote_gid = smcd_query_rgid,
-	.register_dmb = smcd_register_dmb,
-	.unregister_dmb = smcd_unregister_dmb,
-	.add_vlan_id = smcd_add_vlan_id,
-	.del_vlan_id = smcd_del_vlan_id,
-	.set_vlan_required = ism_set_vlan_required,
-	.reset_vlan_required = ism_reset_vlan_required,
-	.signal_event = smcd_signal_ieq,
-	.move_data = smcd_move,
-	.get_system_eid = ism_get_seid,
-	.get_local_gid = smcd_get_local_gid,
-	.get_chid = smcd_get_chid,
-};
-
 static void ism_dev_add_work_func(struct work_struct *work)
 {
 	struct ism_client *client = container_of(work, struct ism_client,
@@ -846,3 +790,95 @@ static void __exit ism_exit(void)
 
 module_init(ism_init);
 module_exit(ism_exit);
+
+/*************************** SMC-D Implementation *****************************/
+
+#if IS_ENABLED(CONFIG_SMC)
+static int smcd_query_rgid(struct smcd_dev *smcd, u64 rgid, u32 vid_valid,
+			   u32 vid)
+{
+	return ism_query_rgid(smcd->priv, rgid, vid_valid, vid);
+}
+
+static int smcd_register_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb,
+			     struct ism_client *client)
+{
+	return ism_register_dmb(smcd->priv, (struct ism_dmb *)dmb, client);
+}
+
+static int smcd_unregister_dmb(struct smcd_dev *smcd, struct smcd_dmb *dmb)
+{
+	return ism_unregister_dmb(smcd->priv, (struct ism_dmb *)dmb);
+}
+
+static int smcd_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+{
+	return ism_add_vlan_id(smcd->priv, vlan_id);
+}
+
+static int smcd_del_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+{
+	return ism_del_vlan_id(smcd->priv, vlan_id);
+}
+
+static int smcd_set_vlan_required(struct smcd_dev *smcd)
+{
+	return ism_cmd_simple(smcd->priv, ISM_SET_VLAN);
+}
+
+static int smcd_reset_vlan_required(struct smcd_dev *smcd)
+{
+	return ism_cmd_simple(smcd->priv, ISM_RESET_VLAN);
+}
+
+static int smcd_signal_ieq(struct smcd_dev *smcd, u64 rgid, u32 trigger_irq,
+			   u32 event_code, u64 info)
+{
+	return ism_signal_ieq(smcd->priv, rgid, trigger_irq, event_code, info);
+}
+
+static int smcd_move(struct smcd_dev *smcd, u64 dmb_tok, unsigned int idx,
+		     bool sf, unsigned int offset, void *data,
+		     unsigned int size)
+{
+	return ism_move(smcd->priv, dmb_tok, idx, sf, offset, data, size);
+}
+
+static u64 smcd_get_local_gid(struct smcd_dev *smcd)
+{
+	return ism_get_local_gid(smcd->priv);
+}
+
+static u16 smcd_get_chid(struct smcd_dev *smcd)
+{
+	return ism_get_chid(smcd->priv);
+}
+
+static inline struct device *smcd_get_dev(struct smcd_dev *dev)
+{
+	struct ism_dev *ism = dev->priv;
+
+	return &ism->dev;
+}
+
+static const struct smcd_ops ism_ops = {
+	.query_remote_gid = smcd_query_rgid,
+	.register_dmb = smcd_register_dmb,
+	.unregister_dmb = smcd_unregister_dmb,
+	.add_vlan_id = smcd_add_vlan_id,
+	.del_vlan_id = smcd_del_vlan_id,
+	.set_vlan_required = smcd_set_vlan_required,
+	.reset_vlan_required = smcd_reset_vlan_required,
+	.signal_event = smcd_signal_ieq,
+	.move_data = smcd_move,
+	.get_system_eid = ism_get_seid,
+	.get_local_gid = smcd_get_local_gid,
+	.get_chid = smcd_get_chid,
+};
+
+const struct smcd_ops *ism_get_smcd_ops(void)
+{
+	return &ism_ops;
+}
+EXPORT_SYMBOL_GPL(ism_get_smcd_ops);
+#endif
diff --git a/include/linux/ism.h b/include/linux/ism.h
index bdd29e08d4fe..104ce2fd503a 100644
--- a/include/linux/ism.h
+++ b/include/linux/ism.h
@@ -94,4 +94,6 @@ int  ism_move(struct ism_dev *dev, u64 dmb_tok, unsigned int idx, bool sf,
 	      unsigned int offset, void *data, unsigned int size);
 u8  *ism_get_seid(void);
 
+const struct smcd_ops *ism_get_smcd_ops(void);
+
 #endif	/* _ISM_H */
diff --git a/include/net/smc.h b/include/net/smc.h
index d5f8f18169d7..556b96c12279 100644
--- a/include/net/smc.h
+++ b/include/net/smc.h
@@ -50,11 +50,13 @@ struct smcd_dmb {
 #define ISM_ERROR	0xFFFF
 
 struct smcd_dev;
+struct ism_client;
 
 struct smcd_ops {
 	int (*query_remote_gid)(struct smcd_dev *dev, u64 rgid, u32 vid_valid,
 				u32 vid);
-	int (*register_dmb)(struct smcd_dev *dev, struct smcd_dmb *dmb);
+	int (*register_dmb)(struct smcd_dev *dev, struct smcd_dmb *dmb,
+			    struct ism_client *client);
 	int (*unregister_dmb)(struct smcd_dev *dev, struct smcd_dmb *dmb);
 	int (*add_vlan_id)(struct smcd_dev *dev, u64 vlan_id);
 	int (*del_vlan_id)(struct smcd_dev *dev, u64 vlan_id);
@@ -73,7 +75,6 @@ struct smcd_ops {
 struct smcd_dev {
 	const struct smcd_ops *ops;
 	struct device dev;
-	struct ism_dev *ism;
 	void *priv;
 	struct list_head list;
 	spinlock_t lock;
diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index 6d31e9bbc5f9..6196b305df44 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -27,6 +27,7 @@ struct smcd_dev_list smcd_dev_list = {
 static bool smc_ism_v2_capable;
 static u8 smc_ism_v2_system_eid[SMC_MAX_EID_LEN];
 
+#if IS_ENABLED(CONFIG_ISM)
 static void smcd_register_dev(struct ism_dev *ism);
 static void smcd_unregister_dev(struct ism_dev *ism);
 static void smcd_handle_event(struct ism_dev *ism, struct ism_event *event);
@@ -40,6 +41,7 @@ static struct ism_client smc_ism_client = {
 	.handle_event = smcd_handle_event,
 	.handle_irq = smcd_handle_irq,
 };
+#endif
 
 /* Test if an ISM communication is possible - same CPC */
 int smc_ism_cantalk(u64 peer_gid, unsigned short vlan_id, struct smcd_dev *smcd)
@@ -198,6 +200,7 @@ int smc_ism_unregister_dmb(struct smcd_dev *smcd, struct smc_buf_desc *dmb_desc)
 int smc_ism_register_dmb(struct smc_link_group *lgr, int dmb_len,
 			 struct smc_buf_desc *dmb_desc)
 {
+#if IS_ENABLED(CONFIG_ISM)
 	struct smcd_dmb dmb;
 	int rc;
 
@@ -206,7 +209,7 @@ int smc_ism_register_dmb(struct smc_link_group *lgr, int dmb_len,
 	dmb.sba_idx = dmb_desc->sba_idx;
 	dmb.vlan_id = lgr->vlan_id;
 	dmb.rgid = lgr->peer_gid;
-	rc = lgr->smcd->ops->register_dmb(lgr->smcd, &dmb);
+	rc = lgr->smcd->ops->register_dmb(lgr->smcd, &dmb, &smc_ism_client);
 	if (!rc) {
 		dmb_desc->sba_idx = dmb.sba_idx;
 		dmb_desc->token = dmb.dmb_tok;
@@ -215,6 +218,9 @@ int smc_ism_register_dmb(struct smc_link_group *lgr, int dmb_len,
 		dmb_desc->len = dmb.dmb_len;
 	}
 	return rc;
+#else
+	return 0;
+#endif
 }
 
 static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
@@ -308,6 +314,7 @@ int smcd_nl_get_device(struct sk_buff *skb, struct netlink_callback *cb)
 	return skb->len;
 }
 
+#if IS_ENABLED(CONFIG_ISM)
 struct smc_ism_event_work {
 	struct work_struct work;
 	struct smcd_dev *smcd;
@@ -351,24 +358,6 @@ static void smcd_handle_sw_event(struct smc_ism_event_work *wrk)
 	}
 }
 
-int smc_ism_signal_shutdown(struct smc_link_group *lgr)
-{
-	int rc;
-	union smcd_sw_event_info ev_info;
-
-	if (lgr->peer_shutdown)
-		return 0;
-
-	memcpy(ev_info.uid, lgr->id, SMC_LGR_ID_SIZE);
-	ev_info.vlan_id = lgr->vlan_id;
-	ev_info.code = ISM_EVENT_REQUEST;
-	rc = lgr->smcd->ops->signal_event(lgr->smcd, lgr->peer_gid,
-					  ISM_EVENT_REQUEST_IR,
-					  ISM_EVENT_CODE_SHUTDOWN,
-					  ev_info.info);
-	return rc;
-}
-
 /* worker for SMC-D events */
 static void smc_ism_event_work(struct work_struct *work)
 {
@@ -442,9 +431,12 @@ EXPORT_SYMBOL_GPL(smcd_free_dev);
 
 static void smcd_register_dev(struct ism_dev *ism)
 {
-	const struct smcd_ops *ops = NULL;
+	const struct smcd_ops *ops = ism_get_smcd_ops();
 	struct smcd_dev *smcd;
 
+	if (!ops)
+		return;
+
 	smcd = smcd_alloc_dev(&ism->pdev->dev, dev_name(&ism->pdev->dev), ops,
 			      ISM_NR_DMBS);
 	if (!smcd)
@@ -550,16 +542,39 @@ static void smcd_handle_irq(struct ism_dev *ism, unsigned int dmbno,
 		tasklet_schedule(&conn->rx_tsklet);
 	spin_unlock_irqrestore(&smcd->lock, flags);
 }
+#endif
+
+int smc_ism_signal_shutdown(struct smc_link_group *lgr)
+{
+	int rc = 0;
+#if IS_ENABLED(CONFIG_ISM)
+	union smcd_sw_event_info ev_info;
+
+	if (lgr->peer_shutdown)
+		return 0;
+
+	memcpy(ev_info.uid, lgr->id, SMC_LGR_ID_SIZE);
+	ev_info.vlan_id = lgr->vlan_id;
+	ev_info.code = ISM_EVENT_REQUEST;
+	rc = lgr->smcd->ops->signal_event(lgr->smcd, lgr->peer_gid,
+					  ISM_EVENT_REQUEST_IR,
+					  ISM_EVENT_CODE_SHUTDOWN,
+					  ev_info.info);
+#endif
+	return rc;
+}
 
 int smc_ism_init(void)
 {
+	int rc = 0;
+
+#if IS_ENABLED(CONFIG_ISM)
 	smc_ism_v2_capable = false;
 	memset(smc_ism_v2_system_eid, 0, SMC_MAX_EID_LEN);
-#if IS_ENABLED(CONFIG_ISM)
-	return ism_register_client(&smc_ism_client);
-#else
-	return 0;
+
+	rc = ism_register_client(&smc_ism_client);
 #endif
+	return rc;
 }
 
 void smc_ism_exit(void)
-- 
2.25.1

[net-next v2 8/8] net/smc: De-tangle ism and smc device initialization

[Jan Karcher]

From: Stefan Raspl <raspl@linux.ibm.com>

The struct device for ISM devices was part of struct smcd_dev. Move to
struct ism_dev, provide a new API call in struct smcd_ops, and convert
existing SMCD code accordingly.
Furthermore, remove struct smcd_dev from struct ism_dev.
This is the final part of a bigger overhaul of the interfaces between SMC
and ISM.

Signed-off-by: Stefan Raspl <raspl@linux.ibm.com>
Signed-off-by: Jan Karcher <jaka@linux.ibm.com>
Signed-off-by: Wenjia Zhang <wenjia@linux.ibm.com>
---
 drivers/s390/net/ism_drv.c | 25 +++++++++--------
 include/linux/ism.h        |  1 -
 include/net/smc.h          |  6 +----
 net/smc/af_smc.c           |  1 +
 net/smc/smc_core.c         |  6 +++--
 net/smc/smc_ism.c          | 55 +++++++++-----------------------------
 net/smc/smc_pnet.c         | 40 ++++++++++++++-------------
 7 files changed, 52 insertions(+), 82 deletions(-)

diff --git a/drivers/s390/net/ism_drv.c b/drivers/s390/net/ism_drv.c
index 73c8f42a22a7..eb7e13486087 100644
--- a/drivers/s390/net/ism_drv.c
+++ b/drivers/s390/net/ism_drv.c
@@ -646,6 +646,12 @@ static int ism_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	spin_lock_init(&ism->lock);
 	dev_set_drvdata(&pdev->dev, ism);
 	ism->pdev = pdev;
+	ism->dev.parent = &pdev->dev;
+	device_initialize(&ism->dev);
+	dev_set_name(&ism->dev, dev_name(&pdev->dev));
+	ret = device_add(&ism->dev);
+	if (ret)
+		goto err_dev;
 
 	ret = pci_enable_device_mem(pdev);
 	if (ret)
@@ -663,30 +669,23 @@ static int ism_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	dma_set_max_seg_size(&pdev->dev, SZ_1M);
 	pci_set_master(pdev);
 
-	ism->smcd = smcd_alloc_dev(&pdev->dev, dev_name(&pdev->dev), &ism_ops,
-				   ISM_NR_DMBS);
-	if (!ism->smcd) {
-		ret = -ENOMEM;
-		goto err_resource;
-	}
-
-	ism->smcd->priv = ism;
 	ret = ism_dev_init(ism);
 	if (ret)
-		goto err_free;
+		goto err_resource;
 
 	return 0;
 
-err_free:
-	smcd_free_dev(ism->smcd);
 err_resource:
 	pci_clear_master(pdev);
 	pci_release_mem_regions(pdev);
 err_disable:
 	pci_disable_device(pdev);
 err:
-	kfree(ism);
+	device_del(&ism->dev);
+err_dev:
 	dev_set_drvdata(&pdev->dev, NULL);
+	kfree(ism);
+
 	return ret;
 }
 
@@ -740,7 +739,6 @@ static void ism_remove(struct pci_dev *pdev)
 	ism_dev_exit(ism);
 	mutex_unlock(&ism_dev_list.mutex);
 
-	smcd_free_dev(ism->smcd);
 	pci_clear_master(pdev);
 	pci_release_mem_regions(pdev);
 	pci_disable_device(pdev);
@@ -874,6 +872,7 @@ static const struct smcd_ops ism_ops = {
 	.get_system_eid = ism_get_seid,
 	.get_local_gid = smcd_get_local_gid,
 	.get_chid = smcd_get_chid,
+	.get_dev = smcd_get_dev,
 };
 
 const struct smcd_ops *ism_get_smcd_ops(void)
diff --git a/include/linux/ism.h b/include/linux/ism.h
index 104ce2fd503a..ea2bcdae7401 100644
--- a/include/linux/ism.h
+++ b/include/linux/ism.h
@@ -30,7 +30,6 @@ struct ism_dev {
 	spinlock_t lock; /* protects the ism device */
 	struct list_head list;
 	struct pci_dev *pdev;
-	struct smcd_dev *smcd;
 
 	struct ism_sba *sba;
 	dma_addr_t sba_dma_addr;
diff --git a/include/net/smc.h b/include/net/smc.h
index 556b96c12279..597cb9381182 100644
--- a/include/net/smc.h
+++ b/include/net/smc.h
@@ -70,11 +70,11 @@ struct smcd_ops {
 	u8* (*get_system_eid)(void);
 	u64 (*get_local_gid)(struct smcd_dev *dev);
 	u16 (*get_chid)(struct smcd_dev *dev);
+	struct device* (*get_dev)(struct smcd_dev *dev);
 };
 
 struct smcd_dev {
 	const struct smcd_ops *ops;
-	struct device dev;
 	void *priv;
 	struct list_head list;
 	spinlock_t lock;
@@ -90,8 +90,4 @@ struct smcd_dev {
 	u8 going_away : 1;
 };
 
-struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
-				const struct smcd_ops *ops, int max_dmbs);
-void smcd_free_dev(struct smcd_dev *smcd);
-
 #endif	/* _SMC_H */
diff --git a/net/smc/af_smc.c b/net/smc/af_smc.c
index 5d037714ab78..036532cf39aa 100644
--- a/net/smc/af_smc.c
+++ b/net/smc/af_smc.c
@@ -3499,6 +3499,7 @@ static void __exit smc_exit(void)
 	sock_unregister(PF_SMC);
 	smc_core_exit();
 	smc_ib_unregister_client();
+	smc_ism_exit();
 	destroy_workqueue(smc_close_wq);
 	destroy_workqueue(smc_tcp_ls_wq);
 	destroy_workqueue(smc_hs_wq);
diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c
index ec04966e9bf9..7642b16c41d1 100644
--- a/net/smc/smc_core.c
+++ b/net/smc/smc_core.c
@@ -822,6 +822,7 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
 {
 	struct smc_link_group *lgr;
 	struct list_head *lgr_list;
+	struct smcd_dev *smcd;
 	struct smc_link *lnk;
 	spinlock_t *lgr_lock;
 	u8 link_idx;
@@ -868,7 +869,8 @@ static int smc_lgr_create(struct smc_sock *smc, struct smc_init_info *ini)
 	lgr->conns_all = RB_ROOT;
 	if (ini->is_smcd) {
 		/* SMC-D specific settings */
-		get_device(&ini->ism_dev[ini->ism_selected]->dev);
+		smcd = ini->ism_dev[ini->ism_selected];
+		get_device(smcd->ops->get_dev(smcd));
 		lgr->peer_gid = ini->ism_peer_gid[ini->ism_selected];
 		lgr->smcd = ini->ism_dev[ini->ism_selected];
 		lgr_list = &ini->ism_dev[ini->ism_selected]->lgr_list;
@@ -1387,7 +1389,7 @@ static void smc_lgr_free(struct smc_link_group *lgr)
 	destroy_workqueue(lgr->tx_wq);
 	if (lgr->is_smcd) {
 		smc_ism_put_vlan(lgr->smcd, lgr->vlan_id);
-		put_device(&lgr->smcd->dev);
+		put_device(lgr->smcd->ops->get_dev(lgr->smcd));
 	}
 	smc_lgr_put(lgr); /* theoretically last lgr_put */
 }
diff --git a/net/smc/smc_ism.c b/net/smc/smc_ism.c
index 6196b305df44..3b0b7710c6b0 100644
--- a/net/smc/smc_ism.c
+++ b/net/smc/smc_ism.c
@@ -231,9 +231,11 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
 	struct smc_pci_dev smc_pci_dev;
 	struct nlattr *port_attrs;
 	struct nlattr *attrs;
+	struct ism_dev *ism;
 	int use_cnt = 0;
 	void *nlh;
 
+	ism = smcd->priv;
 	nlh = genlmsg_put(skb, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq,
 			  &smc_gen_nl_family, NLM_F_MULTI,
 			  SMC_NETLINK_GET_DEV_SMCD);
@@ -248,7 +250,7 @@ static int smc_nl_handle_smcd_dev(struct smcd_dev *smcd,
 	if (nla_put_u8(skb, SMC_NLA_DEV_IS_CRIT, use_cnt > 0))
 		goto errattr;
 	memset(&smc_pci_dev, 0, sizeof(smc_pci_dev));
-	smc_set_pci_values(to_pci_dev(smcd->dev.parent), &smc_pci_dev);
+	smc_set_pci_values(to_pci_dev(ism->dev.parent), &smc_pci_dev);
 	if (nla_put_u32(skb, SMC_NLA_DEV_PCI_FID, smc_pci_dev.pci_fid))
 		goto errattr;
 	if (nla_put_u16(skb, SMC_NLA_DEV_PCI_CHID, smc_pci_dev.pci_pchid))
@@ -377,41 +379,24 @@ static void smc_ism_event_work(struct work_struct *work)
 	kfree(wrk);
 }
 
-static void smcd_release(struct device *dev)
-{
-	struct smcd_dev *smcd = container_of(dev, struct smcd_dev, dev);
-
-	kfree(smcd->conn);
-	kfree(smcd);
-}
-
-struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
-				const struct smcd_ops *ops, int max_dmbs)
+static struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
+				       const struct smcd_ops *ops, int max_dmbs)
 {
 	struct smcd_dev *smcd;
 
-	smcd = kzalloc(sizeof(*smcd), GFP_KERNEL);
+	smcd = devm_kzalloc(parent, sizeof(*smcd), GFP_KERNEL);
 	if (!smcd)
 		return NULL;
-	smcd->conn = kcalloc(max_dmbs, sizeof(struct smc_connection *),
-			     GFP_KERNEL);
-	if (!smcd->conn) {
-		kfree(smcd);
+	smcd->conn = devm_kcalloc(parent, max_dmbs,
+				  sizeof(struct smc_connection *), GFP_KERNEL);
+	if (!smcd->conn)
 		return NULL;
-	}
 
 	smcd->event_wq = alloc_ordered_workqueue("ism_evt_wq-%s)",
 						 WQ_MEM_RECLAIM, name);
-	if (!smcd->event_wq) {
-		kfree(smcd->conn);
-		kfree(smcd);
+	if (!smcd->event_wq)
 		return NULL;
-	}
 
-	smcd->dev.parent = parent;
-	smcd->dev.release = smcd_release;
-	device_initialize(&smcd->dev);
-	dev_set_name(&smcd->dev, name);
 	smcd->ops = ops;
 
 	spin_lock_init(&smcd->lock);
@@ -421,13 +406,6 @@ struct smcd_dev *smcd_alloc_dev(struct device *parent, const char *name,
 	init_waitqueue_head(&smcd->lgrs_deleted);
 	return smcd;
 }
-EXPORT_SYMBOL_GPL(smcd_alloc_dev);
-
-void smcd_free_dev(struct smcd_dev *smcd)
-{
-	put_device(&smcd->dev);
-}
-EXPORT_SYMBOL_GPL(smcd_free_dev);
 
 static void smcd_register_dev(struct ism_dev *ism)
 {
@@ -465,16 +443,9 @@ static void smcd_register_dev(struct ism_dev *ism)
 	mutex_unlock(&smcd_dev_list.mutex);
 
 	pr_warn_ratelimited("smc: adding smcd device %s with pnetid %.16s%s\n",
-			    dev_name(&smcd->dev), smcd->pnetid,
+			    dev_name(&ism->dev), smcd->pnetid,
 			    smcd->pnetid_by_user ? " (user defined)" : "");
 
-	if (device_add(&smcd->dev)) {
-		mutex_lock(&smcd_dev_list.mutex);
-		list_del(&smcd->list);
-		mutex_unlock(&smcd_dev_list.mutex);
-		smcd_free_dev(smcd);
-	}
-
 	return;
 }
 
@@ -483,15 +454,13 @@ static void smcd_unregister_dev(struct ism_dev *ism)
 	struct smcd_dev *smcd = ism_get_priv(ism, &smc_ism_client);
 
 	pr_warn_ratelimited("smc: removing smcd device %s\n",
-			    dev_name(&smcd->dev));
+			    dev_name(&ism->dev));
 	smcd->going_away = 1;
 	smc_smcd_terminate_all(smcd);
 	mutex_lock(&smcd_dev_list.mutex);
 	list_del_init(&smcd->list);
 	mutex_unlock(&smcd_dev_list.mutex);
 	destroy_workqueue(smcd->event_wq);
-
-	device_del(&smcd->dev);
 }
 
 /* SMCD Device event handler. Called from ISM device interrupt handler.
diff --git a/net/smc/smc_pnet.c b/net/smc/smc_pnet.c
index 25fb2fd186e2..11775401df68 100644
--- a/net/smc/smc_pnet.c
+++ b/net/smc/smc_pnet.c
@@ -103,7 +103,7 @@ static int smc_pnet_remove_by_pnetid(struct net *net, char *pnet_name)
 	struct smc_pnetentry *pnetelem, *tmp_pe;
 	struct smc_pnettable *pnettable;
 	struct smc_ib_device *ibdev;
-	struct smcd_dev *smcd_dev;
+	struct smcd_dev *smcd;
 	struct smc_net *sn;
 	int rc = -ENOENT;
 	int ibport;
@@ -162,16 +162,17 @@ static int smc_pnet_remove_by_pnetid(struct net *net, char *pnet_name)
 	mutex_unlock(&smc_ib_devices.mutex);
 	/* remove smcd devices */
 	mutex_lock(&smcd_dev_list.mutex);
-	list_for_each_entry(smcd_dev, &smcd_dev_list.list, list) {
-		if (smcd_dev->pnetid_by_user &&
+	list_for_each_entry(smcd, &smcd_dev_list.list, list) {
+		if (smcd->pnetid_by_user &&
 		    (!pnet_name ||
-		     smc_pnet_match(pnet_name, smcd_dev->pnetid))) {
+		     smc_pnet_match(pnet_name, smcd->pnetid))) {
 			pr_warn_ratelimited("smc: smcd device %s "
 					    "erased user defined pnetid "
-					    "%.16s\n", dev_name(&smcd_dev->dev),
-					    smcd_dev->pnetid);
-			memset(smcd_dev->pnetid, 0, SMC_MAX_PNETID_LEN);
-			smcd_dev->pnetid_by_user = false;
+					    "%.16s\n",
+					    dev_name(smcd->ops->get_dev(smcd)),
+					    smcd->pnetid);
+			memset(smcd->pnetid, 0, SMC_MAX_PNETID_LEN);
+			smcd->pnetid_by_user = false;
 			rc = 0;
 		}
 	}
@@ -331,8 +332,8 @@ static struct smcd_dev *smc_pnet_find_smcd(char *smcd_name)
 
 	mutex_lock(&smcd_dev_list.mutex);
 	list_for_each_entry(smcd_dev, &smcd_dev_list.list, list) {
-		if (!strncmp(dev_name(&smcd_dev->dev), smcd_name,
-			     IB_DEVICE_NAME_MAX - 1))
+		if (!strncmp(dev_name(smcd_dev->ops->get_dev(smcd_dev)),
+			     smcd_name, IB_DEVICE_NAME_MAX - 1))
 			goto out;
 	}
 	smcd_dev = NULL;
@@ -411,7 +412,8 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
 	struct smc_ib_device *ib_dev;
 	bool smcddev_applied = true;
 	bool ibdev_applied = true;
-	struct smcd_dev *smcd_dev;
+	struct smcd_dev *smcd;
+	struct device *dev;
 	bool new_ibdev;
 
 	/* try to apply the pnetid to active devices */
@@ -425,14 +427,16 @@ static int smc_pnet_add_ib(struct smc_pnettable *pnettable, char *ib_name,
 					    ib_port,
 					    ib_dev->pnetid[ib_port - 1]);
 	}
-	smcd_dev = smc_pnet_find_smcd(ib_name);
-	if (smcd_dev) {
-		smcddev_applied = smc_pnet_apply_smcd(smcd_dev, pnet_name);
-		if (smcddev_applied)
+	smcd = smc_pnet_find_smcd(ib_name);
+	if (smcd) {
+		smcddev_applied = smc_pnet_apply_smcd(smcd, pnet_name);
+		if (smcddev_applied) {
+			dev = smcd->ops->get_dev(smcd);
 			pr_warn_ratelimited("smc: smcd device %s "
 					    "applied user defined pnetid "
-					    "%.16s\n", dev_name(&smcd_dev->dev),
-					    smcd_dev->pnetid);
+					    "%.16s\n", dev_name(dev),
+					    smcd->pnetid);
+		}
 	}
 	/* Apply fails when a device has a hardware-defined pnetid set, do not
 	 * add a pnet table entry in that case.
@@ -1181,7 +1185,7 @@ int smc_pnetid_by_table_ib(struct smc_ib_device *smcibdev, u8 ib_port)
  */
 int smc_pnetid_by_table_smcd(struct smcd_dev *smcddev)
 {
-	const char *ib_name = dev_name(&smcddev->dev);
+	const char *ib_name = dev_name(smcddev->ops->get_dev(smcddev));
 	struct smc_pnettable *pnettable;
 	struct smc_pnetentry *tmp_pe;
 	struct smc_net *sn;
-- 
2.25.1

Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Mon, 23 Jan 2023 19:17:44 +0100 you wrote:
> Previously, there was no clean separation between SMC-D code and the ISM
> device driver.This patch series addresses the situation to make ISM available
> for uses outside of SMC-D.
> In detail: SMC-D offers an interface via struct smcd_ops, which only the
> ISM module implements so far. However, there is no real separation between
> the smcd and ism modules, which starts right with the ISM device
> initialization, which calls directly into the SMC-D code.
> This patch series introduces a new API in the ISM module, which allows
> registration of arbitrary clients via include/linux/ism.h: struct ism_client.
> Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
> dependencies on SMC-D in the device structure), and adds a number of API
> calls for data transfers via ISM (see ism_register_dmb() & friends).
> Still, the ISM module implements the SMC-D API, and therefore has a number
> of internal helper functions for that matter.
> Note that the ISM API is consciously kept thin for now (as compared to the
> SMC-D API calls), as a number of API calls are only used with SMC-D and
> hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/8] net/smc: Terminate connections prior to device removal
    https://git.kernel.org/netdev/net-next/c/c40bff4132e5
  - [net-next,v2,2/8] net/ism: Add missing calls to disable bus-mastering
    https://git.kernel.org/netdev/net-next/c/462502ff9acb
  - [net-next,v2,3/8] s390/ism: Introduce struct ism_dmb
    https://git.kernel.org/netdev/net-next/c/1baedb13f1d5
  - [net-next,v2,4/8] net/ism: Add new API for client registration
    https://git.kernel.org/netdev/net-next/c/89e7d2ba61b7
  - [net-next,v2,5/8] net/smc: Register SMC-D as ISM client
    https://git.kernel.org/netdev/net-next/c/8747716f3942
  - [net-next,v2,6/8] net/smc: Separate SMC-D and ISM APIs
    https://git.kernel.org/netdev/net-next/c/9de4df7b6be1
  - [net-next,v2,7/8] s390/ism: Consolidate SMC-D-related code
    https://git.kernel.org/netdev/net-next/c/820f21009f1b
  - [net-next,v2,8/8] net/smc: De-tangle ism and smc device initialization
    https://git.kernel.org/netdev/net-next/c/8c81ba20349d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[Dust Li]

On Mon, Jan 23, 2023 at 07:17:44PM +0100, Jan Karcher wrote:
>Previously, there was no clean separation between SMC-D code and the ISM
>device driver.This patch series addresses the situation to make ISM available
>for uses outside of SMC-D.
>In detail: SMC-D offers an interface via struct smcd_ops, which only the
>ISM module implements so far. However, there is no real separation between
>the smcd and ism modules, which starts right with the ISM device
>initialization, which calls directly into the SMC-D code.
>This patch series introduces a new API in the ISM module, which allows
>registration of arbitrary clients via include/linux/ism.h: struct ism_client.
>Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
>dependencies on SMC-D in the device structure), and adds a number of API
>calls for data transfers via ISM (see ism_register_dmb() & friends).
>Still, the ISM module implements the SMC-D API, and therefore has a number
>of internal helper functions for that matter.
>Note that the ISM API is consciously kept thin for now (as compared to the
>SMC-D API calls), as a number of API calls are only used with SMC-D and
>hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.

Hi,

Great work ! This makes the SMC & ISM code much more clear !

I like this patchset, just some questions on this refactor.
I still see there are some SMC related code in
'drivers/s390/net/ism_drv.c', mainly to implement smcd_ops.

As ISM is the lower layer of SMC, I think remove the dependency
on SMC would be better ? Do you have any plan to do that ?

One more thing:
I didn't find any call for smcd_ops->set_vlan_required/reset_vlan_required,
looks it's not needed, so why not remove it, am I missed something ?

Thanks!

>
>v1 -> v2:
>  Removed s390x dependency which broke config for other archs.
>
>Stefan Raspl (8):
>  net/smc: Terminate connections prior to device removal
>  net/ism: Add missing calls to disable bus-mastering
>  s390/ism: Introduce struct ism_dmb
>  net/ism: Add new API for client registration
>  net/smc: Register SMC-D as ISM client
>  net/smc: Separate SMC-D and ISM APIs
>  s390/ism: Consolidate SMC-D-related code
>  net/smc: De-tangle ism and smc device initialization
>
> drivers/s390/net/ism.h     |  19 +-
> drivers/s390/net/ism_drv.c | 376 ++++++++++++++++++++++++++++++-------
> include/linux/ism.h        |  98 ++++++++++
> include/net/smc.h          |  24 +--
> net/smc/af_smc.c           |   9 +-
> net/smc/smc_clc.c          |  11 +-
> net/smc/smc_core.c         |  13 +-
> net/smc/smc_diag.c         |   3 +-
> net/smc/smc_ism.c          | 180 ++++++++++--------
> net/smc/smc_ism.h          |   3 +-
> net/smc/smc_pnet.c         |  40 ++--
> 11 files changed, 560 insertions(+), 216 deletions(-)
> create mode 100644 include/linux/ism.h
>
>-- 
>2.25.1

Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[Wenjia Zhang]

On 29.01.23 12:48, Dust Li wrote:
> On Mon, Jan 23, 2023 at 07:17:44PM +0100, Jan Karcher wrote:
>> Previously, there was no clean separation between SMC-D code and the ISM
>> device driver.This patch series addresses the situation to make ISM available
>> for uses outside of SMC-D.
>> In detail: SMC-D offers an interface via struct smcd_ops, which only the
>> ISM module implements so far. However, there is no real separation between
>> the smcd and ism modules, which starts right with the ISM device
>> initialization, which calls directly into the SMC-D code.
>> This patch series introduces a new API in the ISM module, which allows
>> registration of arbitrary clients via include/linux/ism.h: struct ism_client.
>> Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
>> dependencies on SMC-D in the device structure), and adds a number of API
>> calls for data transfers via ISM (see ism_register_dmb() & friends).
>> Still, the ISM module implements the SMC-D API, and therefore has a number
>> of internal helper functions for that matter.
>> Note that the ISM API is consciously kept thin for now (as compared to the
>> SMC-D API calls), as a number of API calls are only used with SMC-D and
>> hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.
> 
> Hi,
> 
> Great work ! This makes the SMC & ISM code much more clear !
> 
> I like this patchset, just some questions on this refactor.
> I still see there are some SMC related code in
> 'drivers/s390/net/ism_drv.c', mainly to implement smcd_ops.
> 
> As ISM is the lower layer of SMC, I think remove the dependency
> on SMC would be better ? Do you have any plan to do that ?
> 
Since SMC is the main user of the ISM currently, we still want to keep 
the dependency for now, Sure, I agree with you, in the future we should 
remove the dependency.

> One more thing:
> I didn't find any call for smcd_ops->set_vlan_required/reset_vlan_required,
> looks it's not needed, so why not remove it, am I missed something ?
> 
You didn't miss anything, that’s just for the usage in case

> Thanks!
> 
>>
>> v1 -> v2:
>>   Removed s390x dependency which broke config for other archs.
>>
>> Stefan Raspl (8):
>>   net/smc: Terminate connections prior to device removal
>>   net/ism: Add missing calls to disable bus-mastering
>>   s390/ism: Introduce struct ism_dmb
>>   net/ism: Add new API for client registration
>>   net/smc: Register SMC-D as ISM client
>>   net/smc: Separate SMC-D and ISM APIs
>>   s390/ism: Consolidate SMC-D-related code
>>   net/smc: De-tangle ism and smc device initialization
>>
>> drivers/s390/net/ism.h     |  19 +-
>> drivers/s390/net/ism_drv.c | 376 ++++++++++++++++++++++++++++++-------
>> include/linux/ism.h        |  98 ++++++++++
>> include/net/smc.h          |  24 +--
>> net/smc/af_smc.c           |   9 +-
>> net/smc/smc_clc.c          |  11 +-
>> net/smc/smc_core.c         |  13 +-
>> net/smc/smc_diag.c         |   3 +-
>> net/smc/smc_ism.c          | 180 ++++++++++--------
>> net/smc/smc_ism.h          |   3 +-
>> net/smc/smc_pnet.c         |  40 ++--
>> 11 files changed, 560 insertions(+), 216 deletions(-)
>> create mode 100644 include/linux/ism.h
>>
>> -- 
>> 2.25.1

Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[Wen Gu]

On 2023/1/24 02:17, Jan Karcher wrote:

> Previously, there was no clean separation between SMC-D code and the ISM
> device driver.This patch series addresses the situation to make ISM available
> for uses outside of SMC-D.
> In detail: SMC-D offers an interface via struct smcd_ops, which only the
> ISM module implements so far. However, there is no real separation between
> the smcd and ism modules, which starts right with the ISM device
> initialization, which calls directly into the SMC-D code.
> This patch series introduces a new API in the ISM module, which allows
> registration of arbitrary clients via include/linux/ism.h: struct ism_client.
> Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
> dependencies on SMC-D in the device structure), and adds a number of API
> calls for data transfers via ISM (see ism_register_dmb() & friends).
> Still, the ISM module implements the SMC-D API, and therefore has a number
> of internal helper functions for that matter.
> Note that the ISM API is consciously kept thin for now (as compared to the
> SMC-D API calls), as a number of API calls are only used with SMC-D and
> hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.
> 

Hi,

Thanks for the great work!

We are tring to adapt loopback and virtio-ism device into SMC-D based on the new
interface and want to confirm something. (cc: Alexandra Winter, Jan Karcher, Wenjia Zhang)

 From my understanding, this patch set is from the perspective of ISM device driver
and aims to make ISM device not only used by SMC-D, which is great!

But from the perspective of SMC, SMC-D protocol now binds with the helper in
smc_ism.c (smc_ism_* helper) and some part of smc_ism.c and smcd_ops seems to be
dedicated to only serve ISM device.

For example,

- The input param of smcd_register_dev() and smcd_unregister_dev() is ism_dev,
   instead of abstract smcd_dev like before.

- the smcd->ops->register_dmb has param of ism_client, exposing specific underlay.

So I want to confirm that, which of the following is our future direction of the
SMC-D device expansion?

(1) All extended devices (eg. virtio-ism and loopback) are ISM devices and SMC-D
     only supports ISM type device.

     SMC-D protocol -> smc_ism_* helper in smc_ism.c -> only ISM device.

     Future extended device must under the definition of ism_dev, in order to share
     the ism-specific helper in smc_ism.c (such as smcd_register_dev(), smcd_ops->register_dmbs..).

     With this design intention, futher extended SMC-D used device may be like:

                     +---------------------+
                     |    SMC-D protocol   |
                     +---------------------+
                       | current helper in|
                       |    smc_ism.c     |
          +--------------------------------------------+
          |              Broad ISM device              |
          |             defined as ism_dev             |
          |  +----------+ +------------+ +----------+  |
          |  | s390 ISM | | virtio-ism | | loopback |  |
          |  +----------+ +------------+ +----------+  |
          +--------------------------------------------+

(2) All extended devices (eg. virtio-ism and loopback) are abstracted as smcd_dev and
     SMC-D protocol use the abstracted capabilities.

     SMC-D does not care about the type of the underlying device, and only focus on the
     capabilities provided by smcd_dev.

     SMC-D protocol use a kind of general helpers, which only invoking smcd_dev->ops,
     without underlay device exposed. Just like most of helpers now in smc_ism.c, such as
     smc_ism_cantalk()/smc_ism_get_chid()/smc_ism_set_conn()..

     With this design intention, futher extended SMC-D used device should be like:

                      +----------------------+
                      |     SMC-D protocol   |
                      +----------------------+
                       |   general helper   |
                       |invoke smcd_dev->ops|
                       | hiding underlay dev|
            +-----------+  +------------+  +----------+
            | smc_ism.c |  | smc_vism.c |  | smc_lo.c |
            |           |  |            |  |          |
            | s390 ISM  |  | virtio-ism |  | loopback |
            |  device   |  |   device   |  |  device  |
            +-----------+  +------------+  +----------+

IMHO, (2) is more clean and beneficial to the flexible expansion of SMC-D devices, with no
underlay devices exposed.

So (2) should be our target. Do you agree? :)

If so, maybe we should make some part of helpers or ops of SMC-D device (such as smcd_register/unregister_dev
and smcd->ops->register_dmb) more generic？

Thanks,
Wen Gu

Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[Wenjia Zhang]

On 02.02.23 14:53, Wen Gu wrote:
> 
> 
> On 2023/1/24 02:17, Jan Karcher wrote:
> 
>> Previously, there was no clean separation between SMC-D code and the ISM
>> device driver.This patch series addresses the situation to make ISM 
>> available
>> for uses outside of SMC-D.
>> In detail: SMC-D offers an interface via struct smcd_ops, which only the
>> ISM module implements so far. However, there is no real separation 
>> between
>> the smcd and ism modules, which starts right with the ISM device
>> initialization, which calls directly into the SMC-D code.
>> This patch series introduces a new API in the ISM module, which allows
>> registration of arbitrary clients via include/linux/ism.h: struct 
>> ism_client.
>> Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
>> dependencies on SMC-D in the device structure), and adds a number of API
>> calls for data transfers via ISM (see ism_register_dmb() & friends).
>> Still, the ISM module implements the SMC-D API, and therefore has a 
>> number
>> of internal helper functions for that matter.
>> Note that the ISM API is consciously kept thin for now (as compared to 
>> the
>> SMC-D API calls), as a number of API calls are only used with SMC-D and
>> hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related 
>> calls.
>>
> 
> Hi,
> 
> Thanks for the great work!
> 
> We are tring to adapt loopback and virtio-ism device into SMC-D based on 
> the new
> interface and want to confirm something. (cc: Alexandra Winter, Jan 
> Karcher, Wenjia Zhang)
> 
>  From my understanding, this patch set is from the perspective of ISM 
> device driver
> and aims to make ISM device not only used by SMC-D, which is great!
> 
> But from the perspective of SMC, SMC-D protocol now binds with the 
> helper in
> smc_ism.c (smc_ism_* helper) and some part of smc_ism.c and smcd_ops 
> seems to be
> dedicated to only serve ISM device.
> 
> For example,
> 
> - The input param of smcd_register_dev() and smcd_unregister_dev() is 
> ism_dev,
>    instead of abstract smcd_dev like before.
> 
> - the smcd->ops->register_dmb has param of ism_client, exposing specific 
> underlay.
> 
> So I want to confirm that, which of the following is our future 
> direction of the
> SMC-D device expansion?
> 
> (1) All extended devices (eg. virtio-ism and loopback) are ISM devices 
> and SMC-D
>      only supports ISM type device.
> 
>      SMC-D protocol -> smc_ism_* helper in smc_ism.c -> only ISM device.
> 
>      Future extended device must under the definition of ism_dev, in 
> order to share
>      the ism-specific helper in smc_ism.c (such as smcd_register_dev(), 
> smcd_ops->register_dmbs..).
> 
>      With this design intention, futher extended SMC-D used device may 
> be like:
> 
>                      +---------------------+
>                      |    SMC-D protocol   |
>                      +---------------------+
>                        | current helper in|
>                        |    smc_ism.c     |
>           +--------------------------------------------+
>           |              Broad ISM device              |
>           |             defined as ism_dev             |
>           |  +----------+ +------------+ +----------+  |
>           |  | s390 ISM | | virtio-ism | | loopback |  |
>           |  +----------+ +------------+ +----------+  |
>           +--------------------------------------------+
> 
> (2) All extended devices (eg. virtio-ism and loopback) are abstracted as 
> smcd_dev and
>      SMC-D protocol use the abstracted capabilities.
> 
>      SMC-D does not care about the type of the underlying device, and 
> only focus on the
>      capabilities provided by smcd_dev.
> 
>      SMC-D protocol use a kind of general helpers, which only invoking 
> smcd_dev->ops,
>      without underlay device exposed. Just like most of helpers now in 
> smc_ism.c, such as
>      smc_ism_cantalk()/smc_ism_get_chid()/smc_ism_set_conn()..
> 
>      With this design intention, futher extended SMC-D used device 
> should be like:
> 
>                       +----------------------+
>                       |     SMC-D protocol   |
>                       +----------------------+
>                        |   general helper   |
>                        |invoke smcd_dev->ops|
>                        | hiding underlay dev|
>             +-----------+  +------------+  +----------+
>             | smc_ism.c |  | smc_vism.c |  | smc_lo.c |
>             |           |  |            |  |          |
>             | s390 ISM  |  | virtio-ism |  | loopback |
>             |  device   |  |   device   |  |  device  |
>             +-----------+  +------------+  +----------+
> 
> IMHO, (2) is more clean and beneficial to the flexible expansion of 
> SMC-D devices, with no
> underlay devices exposed.
> 
> So (2) should be our target. Do you agree? :)
> 
> If so, maybe we should make some part of helpers or ops of SMC-D device 
> (such as smcd_register/unregister_dev
> and smcd->ops->register_dmb) more generic？
> 
> Thanks,
> Wen Gu

Currently we tend a bit more towards the first solution. The reasoning 
behind it is the following:
If we create a full blown interface, we would have an own file for every 
new device which on the one hand is clean, but on the other hand raises 
the risk of duplicated code.
So if we go down that path (2) we have to take care that we avoid 
duplicated code.

In the context of the currently discussed changes this could mean:
- ISM is the only device right now using indirect copy,
- lo & vism should (AFAIU) copy directly.

As you may see this leaves us with the big question: How much 
abstraction is enough vs. when do we go overboard?

Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

[Wen Gu]

On 2023/2/6 18:47, Wenjia Zhang wrote:
> 
> 
> On 02.02.23 14:53, Wen Gu wrote:
>>
>>
>> On 2023/1/24 02:17, Jan Karcher wrote:
>>
>>> Previously, there was no clean separation between SMC-D code and the ISM
>>> device driver.This patch series addresses the situation to make ISM available
>>> for uses outside of SMC-D.
>>> In detail: SMC-D offers an interface via struct smcd_ops, which only the
>>> ISM module implements so far. However, there is no real separation between
>>> the smcd and ism modules, which starts right with the ISM device
>>> initialization, which calls directly into the SMC-D code.
>>> This patch series introduces a new API in the ISM module, which allows
>>> registration of arbitrary clients via include/linux/ism.h: struct ism_client.
>>> Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
>>> dependencies on SMC-D in the device structure), and adds a number of API
>>> calls for data transfers via ISM (see ism_register_dmb() & friends).
>>> Still, the ISM module implements the SMC-D API, and therefore has a number
>>> of internal helper functions for that matter.
>>> Note that the ISM API is consciously kept thin for now (as compared to the
>>> SMC-D API calls), as a number of API calls are only used with SMC-D and
>>> hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.
>>>
>>
>> Hi,
>>
>> Thanks for the great work!
>>
>> We are tring to adapt loopback and virtio-ism device into SMC-D based on the new
>> interface and want to confirm something. (cc: Alexandra Winter, Jan Karcher, Wenjia Zhang)
>>
>>  From my understanding, this patch set is from the perspective of ISM device driver
>> and aims to make ISM device not only used by SMC-D, which is great!
>>
>> But from the perspective of SMC, SMC-D protocol now binds with the helper in
>> smc_ism.c (smc_ism_* helper) and some part of smc_ism.c and smcd_ops seems to be
>> dedicated to only serve ISM device.
>>
>> For example,
>>
>> - The input param of smcd_register_dev() and smcd_unregister_dev() is ism_dev,
>>    instead of abstract smcd_dev like before.
>>
>> - the smcd->ops->register_dmb has param of ism_client, exposing specific underlay.
>>
>> So I want to confirm that, which of the following is our future direction of the
>> SMC-D device expansion?
>>
>> (1) All extended devices (eg. virtio-ism and loopback) are ISM devices and SMC-D
>>      only supports ISM type device.
>>
>>      SMC-D protocol -> smc_ism_* helper in smc_ism.c -> only ISM device.
>>
>>      Future extended device must under the definition of ism_dev, in order to share
>>      the ism-specific helper in smc_ism.c (such as smcd_register_dev(), smcd_ops->register_dmbs..).
>>
>>      With this design intention, futher extended SMC-D used device may be like:
>>
>>                      +---------------------+
>>                      |    SMC-D protocol   |
>>                      +---------------------+
>>                        | current helper in|
>>                        |    smc_ism.c     |
>>           +--------------------------------------------+
>>           |              Broad ISM device              |
>>           |             defined as ism_dev             |
>>           |  +----------+ +------------+ +----------+  |
>>           |  | s390 ISM | | virtio-ism | | loopback |  |
>>           |  +----------+ +------------+ +----------+  |
>>           +--------------------------------------------+
>>
>> (2) All extended devices (eg. virtio-ism and loopback) are abstracted as smcd_dev and
>>      SMC-D protocol use the abstracted capabilities.
>>
>>      SMC-D does not care about the type of the underlying device, and only focus on the
>>      capabilities provided by smcd_dev.
>>
>>      SMC-D protocol use a kind of general helpers, which only invoking smcd_dev->ops,
>>      without underlay device exposed. Just like most of helpers now in smc_ism.c, such as
>>      smc_ism_cantalk()/smc_ism_get_chid()/smc_ism_set_conn()..
>>
>>      With this design intention, futher extended SMC-D used device should be like:
>>
>>                       +----------------------+
>>                       |     SMC-D protocol   |
>>                       +----------------------+
>>                        |   general helper   |
>>                        |invoke smcd_dev->ops|
>>                        | hiding underlay dev|
>>             +-----------+  +------------+  +----------+
>>             | smc_ism.c |  | smc_vism.c |  | smc_lo.c |
>>             |           |  |            |  |          |
>>             | s390 ISM  |  | virtio-ism |  | loopback |
>>             |  device   |  |   device   |  |  device  |
>>             +-----------+  +------------+  +----------+
>>
>> IMHO, (2) is more clean and beneficial to the flexible expansion of SMC-D devices, with no
>> underlay devices exposed.
>>
>> So (2) should be our target. Do you agree? :)
>>
>> If so, maybe we should make some part of helpers or ops of SMC-D device (such as smcd_register/unregister_dev
>> and smcd->ops->register_dmb) more generic？
>>
>> Thanks,
>> Wen Gu
> 
> Currently we tend a bit more towards the first solution. The reasoning behind it is the following:
> If we create a full blown interface, we would have an own file for every new device which on the one hand is clean, but 
> on the other hand raises the risk of duplicated code.
> So if we go down that path (2) we have to take care that we avoid duplicated code.
> 
> In the context of the currently discussed changes this could mean:
> - ISM is the only device right now using indirect copy,
> - lo & vism should (AFAIU) copy directly.
> 
> As you may see this leaves us with the big question: How much abstraction is enough vs. when do we go overboard?

I see.

I can understand the difficulty in designing proper abstract and generic helpers, especially when the user's
(lo and vism) implementation code has not been finalized.

I think we can keep optimizing this based on the update of smcd-lo and smcd-vism patches (which will be sent
out as soon as possible).

Thanks,
Wen