All of lore.kernel.org
 help / color / mirror / Atom feed
* (no subject)
@ 2021-01-08 10:32 ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

Subject: [RFC PATCH 00/10] Add Fujitsu A64FX soc entry/hardware barrier driver

Hello,

This series adds Fujitsu A64FX SoC entry in drivers/soc and hardware
barrier driver for it.

[Driver Description]
 A64FX CPU has several functions for HPC workload and hardware barrier
 is one of them. It is a mechanism to realize fast synchronization by
 PEs belonging to the same L3 cache domain by using implementation
 defined hardware registers.
 For more details, see A64FX HPC extension specification in
 https://github.com/fujitsu/A64FX
 
 The driver mainly offers a set of ioctls to manipulate related registers.
 Patch 1-9 implements driver code and patch 10 finally adds kconfig,
 Makefile and MAINTAINER entry for the driver.  

 Also, C library and test program for this driver is available on: 
 https://github.com/fujitsu/hardware_barrier

 The driver is based on v5.11-rc2 and tested on FX700 environment.

[RFC]
 This is the first time we upstream drivers for our chip and I want to
 confirm driver location and patch submission process.

 Based on my observation it seems drivers/soc folder is right place to put
 this driver, so I added Kconfig entry for arm64 platform config, created
 soc/fujitsu folder and updated MAINTAINER entry accordingly (last patch).
 Is it right?

 Also for final submission I think I need to 1) create some public git
 tree to push driver code (github or something), 2) make pull request to
 SOC team (soc@kernel.org). Is it a correct procedure?

 I will appreciate any help/comments.

sidenote: We plan to post other drivers for A64FX HPC extension
(prefetch control and cache control) too anytime soon.

Misono Tomohiro (10):
  soc: fujitsu: hwb: Add hardware barrier driver init/exit code
  soc: fujtisu: hwb: Add open operation
  soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl
  soc: fujitsu: hwb: Add IOC_BW_ASSIGN ioctl
  soc: fujitsu: hwb: Add IOC_BW_UNASSIGN ioctl
  soc: fujitsu: hwb: Add IOC_BB_FREE ioctl
  soc: fujitsu: hwb: Add IOC_GET_PE_INFO ioctl
  soc: fujitsu: hwb: Add release operation
  soc: fujitsu: hwb: Add sysfs entry
  soc: fujitsu: hwb: Add Kconfig/Makefile to build fujitsu_hwb driver

 MAINTAINERS                            |    7 +
 arch/arm64/Kconfig.platforms           |    5 +
 drivers/soc/Kconfig                    |    1 +
 drivers/soc/Makefile                   |    1 +
 drivers/soc/fujitsu/Kconfig            |   24 +
 drivers/soc/fujitsu/Makefile           |    2 +
 drivers/soc/fujitsu/fujitsu_hwb.c      | 1253 ++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |   41 +
 8 files changed, 1334 insertions(+)
 create mode 100644 drivers/soc/fujitsu/Kconfig
 create mode 100644 drivers/soc/fujitsu/Makefile
 create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c
 create mode 100644 include/uapi/linux/fujitsu_hpc_ioctl.h

-- 
2.26.2


^ permalink raw reply	[flat|nested] 32+ messages in thread

* (no subject)
@ 2021-01-08 10:32 ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

Subject: [RFC PATCH 00/10] Add Fujitsu A64FX soc entry/hardware barrier driver

Hello,

This series adds Fujitsu A64FX SoC entry in drivers/soc and hardware
barrier driver for it.

[Driver Description]
 A64FX CPU has several functions for HPC workload and hardware barrier
 is one of them. It is a mechanism to realize fast synchronization by
 PEs belonging to the same L3 cache domain by using implementation
 defined hardware registers.
 For more details, see A64FX HPC extension specification in
 https://github.com/fujitsu/A64FX
 
 The driver mainly offers a set of ioctls to manipulate related registers.
 Patch 1-9 implements driver code and patch 10 finally adds kconfig,
 Makefile and MAINTAINER entry for the driver.  

 Also, C library and test program for this driver is available on: 
 https://github.com/fujitsu/hardware_barrier

 The driver is based on v5.11-rc2 and tested on FX700 environment.

[RFC]
 This is the first time we upstream drivers for our chip and I want to
 confirm driver location and patch submission process.

 Based on my observation it seems drivers/soc folder is right place to put
 this driver, so I added Kconfig entry for arm64 platform config, created
 soc/fujitsu folder and updated MAINTAINER entry accordingly (last patch).
 Is it right?

 Also for final submission I think I need to 1) create some public git
 tree to push driver code (github or something), 2) make pull request to
 SOC team (soc@kernel.org). Is it a correct procedure?

 I will appreciate any help/comments.

sidenote: We plan to post other drivers for A64FX HPC extension
(prefetch control and cache control) too anytime soon.

Misono Tomohiro (10):
  soc: fujitsu: hwb: Add hardware barrier driver init/exit code
  soc: fujtisu: hwb: Add open operation
  soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl
  soc: fujitsu: hwb: Add IOC_BW_ASSIGN ioctl
  soc: fujitsu: hwb: Add IOC_BW_UNASSIGN ioctl
  soc: fujitsu: hwb: Add IOC_BB_FREE ioctl
  soc: fujitsu: hwb: Add IOC_GET_PE_INFO ioctl
  soc: fujitsu: hwb: Add release operation
  soc: fujitsu: hwb: Add sysfs entry
  soc: fujitsu: hwb: Add Kconfig/Makefile to build fujitsu_hwb driver

 MAINTAINERS                            |    7 +
 arch/arm64/Kconfig.platforms           |    5 +
 drivers/soc/Kconfig                    |    1 +
 drivers/soc/Makefile                   |    1 +
 drivers/soc/fujitsu/Kconfig            |   24 +
 drivers/soc/fujitsu/Makefile           |    2 +
 drivers/soc/fujitsu/fujitsu_hwb.c      | 1253 ++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |   41 +
 8 files changed, 1334 insertions(+)
 create mode 100644 drivers/soc/fujitsu/Kconfig
 create mode 100644 drivers/soc/fujitsu/Makefile
 create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c
 create mode 100644 include/uapi/linux/fujitsu_hpc_ioctl.h

-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

This adds hardware barrier driver's struct definitions and
module init/exit code. We use miscdeice for barrier driver ioctl
and /dev/fujitsu_hwb will be created upon module load.
Following commits will add each ioctl definition.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 313 ++++++++++++++++++++++++++++++
 1 file changed, 313 insertions(+)
 create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
new file mode 100644
index 000000000000..44c32c1683df
--- /dev/null
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020 FUJITSU LIMITED
+ *
+ * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
+ * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
+ * On A64FX, CMG is the same as L3 cache domain.
+ *
+ * The main purpose of the driver is setting up registers which cannot be accessed
+ * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
+ * in synchronization main logic can be accessed from EL0 (therefore it is fast).
+ *
+ * Simplified barrier operation flow of user application is as follows:
+ *  (one PE)
+ *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
+ *       This specifies which PEs join synchronization
+ *  (on each PE joining synchronization)
+ *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
+ *    3. Barrier main logic (all logic runs in EL0)
+ *      a) Write 1 to BST_SYNC register
+ *      b) Read LBSY_SYNC register
+ *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
+ *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
+ *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
+ *  (one PE)
+ *    5. Call IOC_BB_FREE to reset INIT_SYNC register
+ */
+
+#include <asm/cputype.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+#define pr_fmt(fmt) "[%s:%s:%d] " fmt, KBUILD_MODNAME, __func__, __LINE__
+
+/* Since miscdevice is used, /dev/fujitsu_hwb will be created when module is loaded */
+#define FHWB_DEV_NAME "fujitsu_hwb"
+
+/* Implementation defined registers for barrier shared in CMG */
+#define FHWB_INIT_SYNC_BB0_EL1  sys_reg(3, 0, 15, 13, 0)
+#define FHWB_INIT_SYNC_BB1_EL1  sys_reg(3, 0, 15, 13, 1)
+#define FHWB_INIT_SYNC_BB2_EL1  sys_reg(3, 0, 15, 13, 2)
+#define FHWB_INIT_SYNC_BB3_EL1  sys_reg(3, 0, 15, 13, 3)
+#define FHWB_INIT_SYNC_BB4_EL1  sys_reg(3, 0, 15, 13, 4)
+#define FHWB_INIT_SYNC_BB5_EL1  sys_reg(3, 0, 15, 13, 5)
+
+/* Implementation defined registers for barrier per PE */
+#define FHWB_CTRL_EL1           sys_reg(3, 0, 11, 12, 0)
+#define FHWB_BST_BIT_EL1        sys_reg(3, 0, 11, 12, 4)
+#define FHWB_ASSIGN_SYNC_W0_EL1 sys_reg(3, 0, 15, 15, 0)
+#define FHWB_ASSIGN_SYNC_W1_EL1 sys_reg(3, 0, 15, 15, 1)
+#define FHWB_ASSIGN_SYNC_W2_EL1 sys_reg(3, 0, 15, 15, 2)
+#define FHWB_ASSIGN_SYNC_W3_EL1 sys_reg(3, 0, 15, 15, 3)
+
+/* Field definitions for above registers */
+#define FHWB_INIT_SYNC_BB_EL1_MASK_FIELD  GENMASK_ULL(44, 32)
+#define FHWB_INIT_SYNC_BB_EL1_BST_FIELD   GENMASK_ULL(12, 0)
+#define FHWB_CTRL_EL1_EL1AE               BIT_ULL(63)
+#define FHWB_CTRL_EL1_EL0AE               BIT_ULL(62)
+#define FHWB_BST_BIT_EL1_CMG_FILED        GENMASK_ULL(5, 4)
+#define FHWB_BST_BIT_EL1_PE_FILED         GENMASK_ULL(3, 0)
+#define FHWB_ASSIGN_SYNC_W_EL1_VALID      BIT_ULL(63)
+
+static enum cpuhp_state _hp_state;
+
+/*
+ * Each PE has its own CMG and Physical PE number (determined by BST_BIT_EL1 register).
+ * Barrier operation can be performed by PEs which belong to the same CMG.
+ */
+struct pe_info {
+	/* CMG number of this PE */
+	u8 cmg;
+	/* Physical PE number of this PE */
+	u8 ppe;
+};
+
+/* Hardware information of running system */
+struct hwb_hwinfo {
+	/* CPU type (part number) */
+	unsigned int type;
+	/* Number of CMG */
+	u8 num_cmg;
+	/* Number of barrier blade(BB) per CMG */
+	u8 num_bb;
+	/* Number of barrier window(BW) per PE */
+	u8 num_bw;
+	/*
+	 * Maximum number of PE per CMG.
+	 * Depending on BIOS configuration, each CMG has up to max_pe_per_cmg PEs
+	 * and each PE has unique physical PE number between 0 ~ (max_pe_per_cmg-1)
+	 */
+	u8 max_pe_per_cmg;
+
+	/* Bitmap for currently allocated BB per CMG */
+	unsigned long *used_bb_bmap;
+	/* Bitmap for currently allocated BW per PE */
+	unsigned long *used_bw_bmap;
+	/* Mapping table of cpuid -> CMG/PE number */
+	struct pe_info *core_map;
+};
+static struct hwb_hwinfo _hwinfo;
+
+/* List for barrier blade currently used per FD */
+struct hwb_private_data {
+	struct list_head bb_list;
+	spinlock_t list_lock;
+};
+
+/* Each barrier blade info */
+#define BB_FREEING 1
+struct bb_info {
+	/* cpumask for PEs which participate synchronization */
+	cpumask_var_t pemask;
+	/* cpumask for PEs which currently assigned BW for this BB */
+	cpumask_var_t assigned_pemask;
+	/* Added to hwb_private_data::bb_list */
+	struct list_head node;
+	/* For indicating if this bb is currently being freed or not */
+	unsigned long flag;
+	/* For waiting ongoing assign/unassign operation to finish before freeing BB */
+	wait_queue_head_t wq;
+	/* Track ongoing assign/unassign operation count */
+	atomic_t ongoing_assign_count;
+	/* CMG  number of this blade */
+	u8 cmg;
+	/* BB number of this blade */
+	u8 bb;
+	/* Hold assigned window number of each PE corresponding to @assigned_pemask */
+	u8 *bw;
+	/* Track usage count as IOC_BB_FREE and IOC_BW_[UN]ASSIGN might be run in parallel */
+	struct kref kref;
+};
+static struct kmem_cache *bb_info_cachep;
+
+static const struct file_operations fujitsu_hwb_dev_fops = {
+	.owner          = THIS_MODULE,
+};
+
+static struct miscdevice bar_miscdev = {
+	.fops  = &fujitsu_hwb_dev_fops,
+	.minor = MISC_DYNAMIC_MINOR,
+	.mode  = 0666,
+	.name  = FHWB_DEV_NAME,
+};
+
+static void destroy_bb_info_cachep(void)
+{
+	kmem_cache_destroy(bb_info_cachep);
+}
+
+static int __init init_bb_info_cachep(void)
+{
+	/*
+	 * Since cpumask value will be copied from userspace to the beginning of
+	 * struct bb_info, use kmem_cache_create_usercopy to mark that region.
+	 * Otherwise CONFIG_HARDENED_USERCOPY gives user_copy_warn.
+	 */
+	bb_info_cachep = kmem_cache_create_usercopy("bb_info_cache", sizeof(struct bb_info),
+			0, SLAB_HWCACHE_ALIGN, 0, sizeof(cpumask_var_t), NULL);
+	if (bb_info_cachep == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void free_map(void)
+{
+	kfree(_hwinfo.used_bw_bmap);
+	kfree(_hwinfo.used_bb_bmap);
+	kfree(_hwinfo.core_map);
+}
+
+static int __init alloc_map(void)
+{
+	_hwinfo.core_map = kcalloc(num_possible_cpus(), sizeof(struct pe_info), GFP_KERNEL);
+	_hwinfo.used_bb_bmap = kcalloc(_hwinfo.num_cmg, sizeof(unsigned long), GFP_KERNEL);
+	_hwinfo.used_bw_bmap = kcalloc(num_possible_cpus(), sizeof(unsigned long), GFP_KERNEL);
+	if (!_hwinfo.core_map || !_hwinfo.used_bb_bmap || !_hwinfo.used_bw_bmap)
+		goto fail;
+
+	/* 0 is valid number for both CMG/PE. Set all bits to 1 to represents uninitialized state */
+	memset(_hwinfo.core_map, 0xFF, sizeof(struct pe_info) * num_possible_cpus());
+
+	return 0;
+
+fail:
+	free_map();
+	return -ENOMEM;
+}
+
+/* Get this system's CPU type (part number). If it is not fujitsu CPU, return -1 */
+static int __init get_cpu_type(void)
+{
+	if (read_cpuid_implementor() != ARM_CPU_IMP_FUJITSU)
+		return -1;
+
+	return read_cpuid_part_number();
+}
+
+static int __init setup_hwinfo(void)
+{
+	int type;
+
+	type = get_cpu_type();
+	if (type < 0)
+		return -ENODEV;
+
+	_hwinfo.type = type;
+	switch (type) {
+	case FUJITSU_CPU_PART_A64FX:
+		_hwinfo.num_cmg = 4;
+		_hwinfo.num_bb = 6;
+		_hwinfo.num_bw = 4;
+		_hwinfo.max_pe_per_cmg = 13;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int hwb_cpu_online(unsigned int cpu)
+{
+	u64 val;
+	int i;
+
+	/* Setup core_map by reading BST_BIT_EL1 register of each PE */
+	val = read_sysreg_s(FHWB_BST_BIT_EL1);
+	_hwinfo.core_map[cpu].cmg = FIELD_GET(FHWB_BST_BIT_EL1_CMG_FILED, val);
+	_hwinfo.core_map[cpu].ppe = FIELD_GET(FHWB_BST_BIT_EL1_PE_FILED, val);
+
+	/* Since these registers' values are UNKNOWN on reset, explicitly clear all */
+	for (i = 0; i < _hwinfo.num_bw; i++)
+		write_bw_reg(i, 0);
+
+	write_sysreg_s(0, FHWB_CTRL_EL1);
+
+	return 0;
+}
+
+static int __init hwb_init(void)
+{
+	int ret;
+
+	ret = setup_hwinfo();
+	if (ret < 0) {
+		pr_err("Unsupported CPU type\n");
+		return ret;
+	}
+
+	ret = alloc_map();
+	if (ret < 0)
+		return ret;
+
+	ret = init_bb_info_cachep();
+	if (ret < 0)
+		goto out1;
+
+	/*
+	 * Setup cpuhp callback to ensure each PE's resource will be initialized
+	 * even if some PEs are offline at this point
+	 */
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/fujitsu_hwb:online",
+		hwb_cpu_online, NULL);
+	if (ret < 0) {
+		pr_err("cpuhp setup failed: %d\n", ret);
+		goto out2;
+	}
+	_hp_state = ret;
+
+	ret = misc_register(&bar_miscdev);
+	if (ret < 0) {
+		pr_err("misc_register failed: %d\n", ret);
+		goto out3;
+	}
+
+	return 0;
+
+out3:
+	cpuhp_remove_state(_hp_state);
+out2:
+	destroy_bb_info_cachep();
+out1:
+	free_map();
+
+	return ret;
+}
+
+static void __exit hwb_exit(void)
+{
+	misc_deregister(&bar_miscdev);
+	cpuhp_remove_state(_hp_state);
+	destroy_bb_info_cachep();
+	free_map();
+}
+
+module_init(hwb_init);
+module_exit(hwb_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("FUJITSU LIMITED");
+MODULE_DESCRIPTION("FUJITSU HPC Hardware Barrier Driver");
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

This adds hardware barrier driver's struct definitions and
module init/exit code. We use miscdeice for barrier driver ioctl
and /dev/fujitsu_hwb will be created upon module load.
Following commits will add each ioctl definition.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 313 ++++++++++++++++++++++++++++++
 1 file changed, 313 insertions(+)
 create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
new file mode 100644
index 000000000000..44c32c1683df
--- /dev/null
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -0,0 +1,313 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2020 FUJITSU LIMITED
+ *
+ * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
+ * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
+ * On A64FX, CMG is the same as L3 cache domain.
+ *
+ * The main purpose of the driver is setting up registers which cannot be accessed
+ * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
+ * in synchronization main logic can be accessed from EL0 (therefore it is fast).
+ *
+ * Simplified barrier operation flow of user application is as follows:
+ *  (one PE)
+ *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
+ *       This specifies which PEs join synchronization
+ *  (on each PE joining synchronization)
+ *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
+ *    3. Barrier main logic (all logic runs in EL0)
+ *      a) Write 1 to BST_SYNC register
+ *      b) Read LBSY_SYNC register
+ *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
+ *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
+ *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
+ *  (one PE)
+ *    5. Call IOC_BB_FREE to reset INIT_SYNC register
+ */
+
+#include <asm/cputype.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cpu.h>
+#include <linux/cpumask.h>
+#include <linux/kernel.h>
+#include <linux/miscdevice.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/wait.h>
+
+#ifdef pr_fmt
+#undef pr_fmt
+#endif
+#define pr_fmt(fmt) "[%s:%s:%d] " fmt, KBUILD_MODNAME, __func__, __LINE__
+
+/* Since miscdevice is used, /dev/fujitsu_hwb will be created when module is loaded */
+#define FHWB_DEV_NAME "fujitsu_hwb"
+
+/* Implementation defined registers for barrier shared in CMG */
+#define FHWB_INIT_SYNC_BB0_EL1  sys_reg(3, 0, 15, 13, 0)
+#define FHWB_INIT_SYNC_BB1_EL1  sys_reg(3, 0, 15, 13, 1)
+#define FHWB_INIT_SYNC_BB2_EL1  sys_reg(3, 0, 15, 13, 2)
+#define FHWB_INIT_SYNC_BB3_EL1  sys_reg(3, 0, 15, 13, 3)
+#define FHWB_INIT_SYNC_BB4_EL1  sys_reg(3, 0, 15, 13, 4)
+#define FHWB_INIT_SYNC_BB5_EL1  sys_reg(3, 0, 15, 13, 5)
+
+/* Implementation defined registers for barrier per PE */
+#define FHWB_CTRL_EL1           sys_reg(3, 0, 11, 12, 0)
+#define FHWB_BST_BIT_EL1        sys_reg(3, 0, 11, 12, 4)
+#define FHWB_ASSIGN_SYNC_W0_EL1 sys_reg(3, 0, 15, 15, 0)
+#define FHWB_ASSIGN_SYNC_W1_EL1 sys_reg(3, 0, 15, 15, 1)
+#define FHWB_ASSIGN_SYNC_W2_EL1 sys_reg(3, 0, 15, 15, 2)
+#define FHWB_ASSIGN_SYNC_W3_EL1 sys_reg(3, 0, 15, 15, 3)
+
+/* Field definitions for above registers */
+#define FHWB_INIT_SYNC_BB_EL1_MASK_FIELD  GENMASK_ULL(44, 32)
+#define FHWB_INIT_SYNC_BB_EL1_BST_FIELD   GENMASK_ULL(12, 0)
+#define FHWB_CTRL_EL1_EL1AE               BIT_ULL(63)
+#define FHWB_CTRL_EL1_EL0AE               BIT_ULL(62)
+#define FHWB_BST_BIT_EL1_CMG_FILED        GENMASK_ULL(5, 4)
+#define FHWB_BST_BIT_EL1_PE_FILED         GENMASK_ULL(3, 0)
+#define FHWB_ASSIGN_SYNC_W_EL1_VALID      BIT_ULL(63)
+
+static enum cpuhp_state _hp_state;
+
+/*
+ * Each PE has its own CMG and Physical PE number (determined by BST_BIT_EL1 register).
+ * Barrier operation can be performed by PEs which belong to the same CMG.
+ */
+struct pe_info {
+	/* CMG number of this PE */
+	u8 cmg;
+	/* Physical PE number of this PE */
+	u8 ppe;
+};
+
+/* Hardware information of running system */
+struct hwb_hwinfo {
+	/* CPU type (part number) */
+	unsigned int type;
+	/* Number of CMG */
+	u8 num_cmg;
+	/* Number of barrier blade(BB) per CMG */
+	u8 num_bb;
+	/* Number of barrier window(BW) per PE */
+	u8 num_bw;
+	/*
+	 * Maximum number of PE per CMG.
+	 * Depending on BIOS configuration, each CMG has up to max_pe_per_cmg PEs
+	 * and each PE has unique physical PE number between 0 ~ (max_pe_per_cmg-1)
+	 */
+	u8 max_pe_per_cmg;
+
+	/* Bitmap for currently allocated BB per CMG */
+	unsigned long *used_bb_bmap;
+	/* Bitmap for currently allocated BW per PE */
+	unsigned long *used_bw_bmap;
+	/* Mapping table of cpuid -> CMG/PE number */
+	struct pe_info *core_map;
+};
+static struct hwb_hwinfo _hwinfo;
+
+/* List for barrier blade currently used per FD */
+struct hwb_private_data {
+	struct list_head bb_list;
+	spinlock_t list_lock;
+};
+
+/* Each barrier blade info */
+#define BB_FREEING 1
+struct bb_info {
+	/* cpumask for PEs which participate synchronization */
+	cpumask_var_t pemask;
+	/* cpumask for PEs which currently assigned BW for this BB */
+	cpumask_var_t assigned_pemask;
+	/* Added to hwb_private_data::bb_list */
+	struct list_head node;
+	/* For indicating if this bb is currently being freed or not */
+	unsigned long flag;
+	/* For waiting ongoing assign/unassign operation to finish before freeing BB */
+	wait_queue_head_t wq;
+	/* Track ongoing assign/unassign operation count */
+	atomic_t ongoing_assign_count;
+	/* CMG  number of this blade */
+	u8 cmg;
+	/* BB number of this blade */
+	u8 bb;
+	/* Hold assigned window number of each PE corresponding to @assigned_pemask */
+	u8 *bw;
+	/* Track usage count as IOC_BB_FREE and IOC_BW_[UN]ASSIGN might be run in parallel */
+	struct kref kref;
+};
+static struct kmem_cache *bb_info_cachep;
+
+static const struct file_operations fujitsu_hwb_dev_fops = {
+	.owner          = THIS_MODULE,
+};
+
+static struct miscdevice bar_miscdev = {
+	.fops  = &fujitsu_hwb_dev_fops,
+	.minor = MISC_DYNAMIC_MINOR,
+	.mode  = 0666,
+	.name  = FHWB_DEV_NAME,
+};
+
+static void destroy_bb_info_cachep(void)
+{
+	kmem_cache_destroy(bb_info_cachep);
+}
+
+static int __init init_bb_info_cachep(void)
+{
+	/*
+	 * Since cpumask value will be copied from userspace to the beginning of
+	 * struct bb_info, use kmem_cache_create_usercopy to mark that region.
+	 * Otherwise CONFIG_HARDENED_USERCOPY gives user_copy_warn.
+	 */
+	bb_info_cachep = kmem_cache_create_usercopy("bb_info_cache", sizeof(struct bb_info),
+			0, SLAB_HWCACHE_ALIGN, 0, sizeof(cpumask_var_t), NULL);
+	if (bb_info_cachep == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static void free_map(void)
+{
+	kfree(_hwinfo.used_bw_bmap);
+	kfree(_hwinfo.used_bb_bmap);
+	kfree(_hwinfo.core_map);
+}
+
+static int __init alloc_map(void)
+{
+	_hwinfo.core_map = kcalloc(num_possible_cpus(), sizeof(struct pe_info), GFP_KERNEL);
+	_hwinfo.used_bb_bmap = kcalloc(_hwinfo.num_cmg, sizeof(unsigned long), GFP_KERNEL);
+	_hwinfo.used_bw_bmap = kcalloc(num_possible_cpus(), sizeof(unsigned long), GFP_KERNEL);
+	if (!_hwinfo.core_map || !_hwinfo.used_bb_bmap || !_hwinfo.used_bw_bmap)
+		goto fail;
+
+	/* 0 is valid number for both CMG/PE. Set all bits to 1 to represents uninitialized state */
+	memset(_hwinfo.core_map, 0xFF, sizeof(struct pe_info) * num_possible_cpus());
+
+	return 0;
+
+fail:
+	free_map();
+	return -ENOMEM;
+}
+
+/* Get this system's CPU type (part number). If it is not fujitsu CPU, return -1 */
+static int __init get_cpu_type(void)
+{
+	if (read_cpuid_implementor() != ARM_CPU_IMP_FUJITSU)
+		return -1;
+
+	return read_cpuid_part_number();
+}
+
+static int __init setup_hwinfo(void)
+{
+	int type;
+
+	type = get_cpu_type();
+	if (type < 0)
+		return -ENODEV;
+
+	_hwinfo.type = type;
+	switch (type) {
+	case FUJITSU_CPU_PART_A64FX:
+		_hwinfo.num_cmg = 4;
+		_hwinfo.num_bb = 6;
+		_hwinfo.num_bw = 4;
+		_hwinfo.max_pe_per_cmg = 13;
+		break;
+	default:
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int hwb_cpu_online(unsigned int cpu)
+{
+	u64 val;
+	int i;
+
+	/* Setup core_map by reading BST_BIT_EL1 register of each PE */
+	val = read_sysreg_s(FHWB_BST_BIT_EL1);
+	_hwinfo.core_map[cpu].cmg = FIELD_GET(FHWB_BST_BIT_EL1_CMG_FILED, val);
+	_hwinfo.core_map[cpu].ppe = FIELD_GET(FHWB_BST_BIT_EL1_PE_FILED, val);
+
+	/* Since these registers' values are UNKNOWN on reset, explicitly clear all */
+	for (i = 0; i < _hwinfo.num_bw; i++)
+		write_bw_reg(i, 0);
+
+	write_sysreg_s(0, FHWB_CTRL_EL1);
+
+	return 0;
+}
+
+static int __init hwb_init(void)
+{
+	int ret;
+
+	ret = setup_hwinfo();
+	if (ret < 0) {
+		pr_err("Unsupported CPU type\n");
+		return ret;
+	}
+
+	ret = alloc_map();
+	if (ret < 0)
+		return ret;
+
+	ret = init_bb_info_cachep();
+	if (ret < 0)
+		goto out1;
+
+	/*
+	 * Setup cpuhp callback to ensure each PE's resource will be initialized
+	 * even if some PEs are offline at this point
+	 */
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/fujitsu_hwb:online",
+		hwb_cpu_online, NULL);
+	if (ret < 0) {
+		pr_err("cpuhp setup failed: %d\n", ret);
+		goto out2;
+	}
+	_hp_state = ret;
+
+	ret = misc_register(&bar_miscdev);
+	if (ret < 0) {
+		pr_err("misc_register failed: %d\n", ret);
+		goto out3;
+	}
+
+	return 0;
+
+out3:
+	cpuhp_remove_state(_hp_state);
+out2:
+	destroy_bb_info_cachep();
+out1:
+	free_map();
+
+	return ret;
+}
+
+static void __exit hwb_exit(void)
+{
+	misc_deregister(&bar_miscdev);
+	cpuhp_remove_state(_hp_state);
+	destroy_bb_info_cachep();
+	free_map();
+}
+
+module_init(hwb_init);
+module_exit(hwb_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("FUJITSU LIMITED");
+MODULE_DESCRIPTION("FUJITSU HPC Hardware Barrier Driver");
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 02/10] soc: fujtisu: hwb: Add open operation
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

Nothing special. Just preparing private_data for this FD.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 44c32c1683df..1dec3d3c652f 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -142,8 +142,28 @@ struct bb_info {
 };
 static struct kmem_cache *bb_info_cachep;
 
+static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
+{
+	struct hwb_private_data *pdata;
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&pdata->bb_list);
+	spin_lock_init(&pdata->list_lock);
+
+	/*
+	 * misc_open() sets pointer of the miscdevice to filp->private_data.
+	 * Just override it since barrier fops does not use it
+	 */
+	filp->private_data = pdata;
+
+	return 0;
+}
+
 static const struct file_operations fujitsu_hwb_dev_fops = {
 	.owner          = THIS_MODULE,
+	.open           = fujitsu_hwb_dev_open,
 };
 
 static struct miscdevice bar_miscdev = {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 02/10] soc: fujtisu: hwb: Add open operation
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

Nothing special. Just preparing private_data for this FD.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 44c32c1683df..1dec3d3c652f 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -142,8 +142,28 @@ struct bb_info {
 };
 static struct kmem_cache *bb_info_cachep;
 
+static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
+{
+	struct hwb_private_data *pdata;
+
+	pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+	INIT_LIST_HEAD(&pdata->bb_list);
+	spin_lock_init(&pdata->list_lock);
+
+	/*
+	 * misc_open() sets pointer of the miscdevice to filp->private_data.
+	 * Just override it since barrier fops does not use it
+	 */
+	filp->private_data = pdata;
+
+	return 0;
+}
+
 static const struct file_operations fujitsu_hwb_dev_fops = {
 	.owner          = THIS_MODULE,
+	.open           = fujitsu_hwb_dev_open,
 };
 
 static struct miscdevice bar_miscdev = {
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 03/10] soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

IOC_BB_ALLOC ioctl initialize INIT_SYNC register which represents
PEs in a CMG joining synchronization. Although we get cpumask of
PEs from userspace, INIT_SYNC register requires mask value based
on physical PE number which is written in each PE's BST register.
So we perform conversion of cpumask value in validate_and_conver_pemask().

Since INIT_SYNC register is a shared resource per CMG, we pick
up one PE and send IPI to it to write the register.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 223 +++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |  23 +++
 2 files changed, 246 insertions(+)
 create mode 100644 include/uapi/linux/fujitsu_hpc_ioctl.h

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 1dec3d3c652f..24d1bb00f55c 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -38,6 +38,8 @@
 #include <linux/slab.h>
 #include <linux/wait.h>
 
+#include <linux/fujitsu_hpc_ioctl.h>
+
 #ifdef pr_fmt
 #undef pr_fmt
 #endif
@@ -142,6 +144,226 @@ struct bb_info {
 };
 static struct kmem_cache *bb_info_cachep;
 
+static void free_bb_info(struct kref *kref)
+{
+	struct bb_info *bb_info = container_of(kref, struct bb_info, kref);
+
+	free_cpumask_var(bb_info->assigned_pemask);
+	free_cpumask_var(bb_info->pemask);
+	kfree(bb_info->bw);
+	kmem_cache_free(bb_info_cachep, bb_info);
+}
+
+static struct bb_info *alloc_bb_info(void)
+{
+	struct bb_info *bb_info;
+
+	bb_info = kmem_cache_zalloc(bb_info_cachep, GFP_KERNEL);
+	if (!bb_info)
+		return NULL;
+
+	bb_info->bw = kcalloc(_hwinfo.max_pe_per_cmg, sizeof(u8), GFP_KERNEL);
+	if (!bb_info->bw) {
+		free_bb_info(&bb_info->kref);
+		return NULL;
+	}
+	if (!zalloc_cpumask_var(&bb_info->pemask, GFP_KERNEL) ||
+		!zalloc_cpumask_var(&bb_info->assigned_pemask, GFP_KERNEL)) {
+		free_bb_info(&bb_info->kref);
+		return NULL;
+	}
+
+	init_waitqueue_head(&bb_info->wq);
+	kref_init(&bb_info->kref);
+
+	return bb_info;
+}
+
+static inline void put_bb_info(struct bb_info *bb_info)
+{
+	kref_put(&bb_info->kref, free_bb_info);
+}
+
+/* Validate pemask's range and convert it to a mask based on physical PE number */
+static int validate_and_convert_pemask(struct bb_info *bb_info, unsigned long *phys_pemask)
+{
+	int cpu;
+	u8 cmg;
+
+	if (cpumask_weight(bb_info->pemask) < 2) {
+		pr_err("pemask needs at least two bit set: %*pbl\n",
+						cpumask_pr_args(bb_info->pemask));
+		return -EINVAL;
+	}
+
+	if (!cpumask_subset(bb_info->pemask, cpu_online_mask)) {
+		pr_err("pemask needs to be subset of online cpu: %*pbl, %*pbl\n",
+			cpumask_pr_args(bb_info->pemask), cpumask_pr_args(cpu_online_mask));
+		return -EINVAL;
+	}
+
+	/*
+	 * INIT_SYNC register requires a mask value based on physical PE number.
+	 * So convert pemask to it while checking if all PEs belongs to the same CMG
+	 */
+	cpu = cpumask_first(bb_info->pemask);
+	cmg = _hwinfo.core_map[cpu].cmg;
+	*phys_pemask = 0;
+	for_each_cpu(cpu, bb_info->pemask) {
+		if (_hwinfo.core_map[cpu].cmg != cmg) {
+			pr_err("All PEs must belong to the same CMG: %*pbl\n",
+							cpumask_pr_args(bb_info->pemask));
+			return -EINVAL;
+		}
+		set_bit(_hwinfo.core_map[cpu].ppe, phys_pemask);
+	}
+	bb_info->cmg = cmg;
+
+	pr_debug("pemask: %*pbl, physical_pemask: %lx\n",
+					cpumask_pr_args(bb_info->pemask), *phys_pemask);
+
+	return 0;
+}
+
+/* Search free BB in_hwinfo->used_bb_bitmap[cmg] */
+static int search_free_bb(u8 cmg)
+{
+	int i;
+
+	for (i = 0; i < _hwinfo.num_bb; i++) {
+		if (!test_and_set_bit(i, &_hwinfo.used_bb_bmap[cmg])) {
+			pr_debug("Use BB %u in CMG %u, bitmap: %lx\n",
+							i, cmg, _hwinfo.used_bb_bmap[cmg]);
+			return i;
+		}
+	}
+
+	pr_err("All barrier blade is currently used in CMG %u\n", cmg);
+	return -EBUSY;
+}
+
+struct init_sync_args {
+	u64 val;
+	u8 bb;
+};
+
+static void write_init_sync_reg(void *args)
+{
+	struct init_sync_args *sync_args = (struct init_sync_args *)args;
+
+	switch (sync_args->bb) {
+	case 0:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB0_EL1);
+		break;
+	case 1:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB1_EL1);
+		break;
+	case 2:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB2_EL1);
+		break;
+	case 3:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB3_EL1);
+		break;
+	case 4:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB4_EL1);
+		break;
+	case 5:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB5_EL1);
+		break;
+	}
+}
+
+/* Send IPI to initialize INIT_SYNC register */
+static void setup_bb(struct bb_info *bb_info, unsigned long phys_pemask)
+{
+	struct init_sync_args args = {0};
+	int cpu;
+
+	/* INIT_SYNC register is shared resource in CMG. Pick one PE to set it up */
+	cpu = cpumask_any(bb_info->pemask);
+
+	args.bb = bb_info->bb;
+	args.val = FIELD_PREP(FHWB_INIT_SYNC_BB_EL1_MASK_FIELD, phys_pemask);
+	on_each_cpu_mask(cpumask_of(cpu), write_init_sync_reg, &args, 1);
+
+	pr_debug("Setup bb. cpu: %d, CMG: %u, BB: %u, bimtap: %lx\n",
+			cpu, bb_info->cmg, bb_info->bb, _hwinfo.used_bb_bmap[bb_info->cmg]);
+}
+
+static int ioc_bb_alloc(struct file *filp, void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bb_ctl bb_ctl;
+	struct bb_info *bb_info;
+	unsigned long physical_pemask;
+	unsigned int size;
+	int ret;
+
+	if (copy_from_user(&bb_ctl, (struct fujitsu_hwb_ioc_bb_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bb_ctl)))
+		return -EFAULT;
+
+	bb_info = alloc_bb_info();
+	if (!bb_info)
+		return -ENOMEM;
+
+	/* cpumask size may vary in user and kernel space. Use the smaller one */
+	size = min(cpumask_size(), bb_ctl.size);
+	if (copy_from_user(bb_info->pemask, bb_ctl.pemask, size)) {
+		ret = -EFAULT;
+		goto put_bb_info;
+	}
+
+	ret = validate_and_convert_pemask(bb_info, &physical_pemask);
+	if (ret < 0)
+		goto put_bb_info;
+
+	ret = search_free_bb(bb_info->cmg);
+	if (ret < 0)
+		goto put_bb_info;
+	bb_info->bb = ret;
+
+	/* Copy back CMG/BB number to be used to user */
+	bb_ctl.cmg = bb_info->cmg;
+	bb_ctl.bb  = bb_info->bb;
+	if (copy_to_user((struct fujitsu_hwb_ioc_bb_ctl __user *)argp, &bb_ctl,
+						sizeof(struct fujitsu_hwb_ioc_bb_ctl))) {
+		ret = -EFAULT;
+		clear_bit(bb_ctl.bb, &_hwinfo.used_bb_bmap[bb_ctl.cmg]);
+		goto put_bb_info;
+	}
+
+	setup_bb(bb_info, physical_pemask);
+
+	spin_lock(&pdata->list_lock);
+	list_add_tail(&bb_info->node, &pdata->bb_list);
+	spin_unlock(&pdata->list_lock);
+
+	return 0;
+
+put_bb_info:
+	put_bb_info(bb_info);
+
+	return ret;
+}
+
+static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	int ret;
+
+	switch (cmd) {
+	case FUJITSU_HWB_IOC_BB_ALLOC:
+		ret = ioc_bb_alloc(filp, argp);
+		break;
+	default:
+		ret = -ENOTTY;
+		break;
+	}
+
+	return ret;
+}
+
 static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
 {
 	struct hwb_private_data *pdata;
@@ -164,6 +386,7 @@ static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
 static const struct file_operations fujitsu_hwb_dev_fops = {
 	.owner          = THIS_MODULE,
 	.open           = fujitsu_hwb_dev_open,
+	.unlocked_ioctl = fujitsu_hwb_dev_ioctl,
 };
 
 static struct miscdevice bar_miscdev = {
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
new file mode 100644
index 000000000000..c87a5bad3f59
--- /dev/null
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/* Copyright 2020 FUJITSU LIMITED */
+#ifndef _UAPI_LINUX_FUJITSU_HPC_IOC_H
+#define _UAPI_LINUX_FUJITSU_HPC_IOC_H
+
+#include <linux/ioctl.h>
+#include <asm/types.h>
+
+#define __FUJITSU_IOCTL_MAGIC 'F'
+
+/* ioctl definitions for hardware barrier driver */
+struct fujitsu_hwb_ioc_bb_ctl {
+	__u8 cmg;
+	__u8 bb;
+	__u8 unused[2];
+	__u32 size;
+	unsigned long __user *pemask;
+};
+
+#define FUJITSU_HWB_IOC_BB_ALLOC _IOWR(__FUJITSU_IOCTL_MAGIC, \
+	0x00, struct fujitsu_hwb_ioc_bb_ctl)
+
+#endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 03/10] soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

IOC_BB_ALLOC ioctl initialize INIT_SYNC register which represents
PEs in a CMG joining synchronization. Although we get cpumask of
PEs from userspace, INIT_SYNC register requires mask value based
on physical PE number which is written in each PE's BST register.
So we perform conversion of cpumask value in validate_and_conver_pemask().

Since INIT_SYNC register is a shared resource per CMG, we pick
up one PE and send IPI to it to write the register.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 223 +++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |  23 +++
 2 files changed, 246 insertions(+)
 create mode 100644 include/uapi/linux/fujitsu_hpc_ioctl.h

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 1dec3d3c652f..24d1bb00f55c 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -38,6 +38,8 @@
 #include <linux/slab.h>
 #include <linux/wait.h>
 
+#include <linux/fujitsu_hpc_ioctl.h>
+
 #ifdef pr_fmt
 #undef pr_fmt
 #endif
@@ -142,6 +144,226 @@ struct bb_info {
 };
 static struct kmem_cache *bb_info_cachep;
 
+static void free_bb_info(struct kref *kref)
+{
+	struct bb_info *bb_info = container_of(kref, struct bb_info, kref);
+
+	free_cpumask_var(bb_info->assigned_pemask);
+	free_cpumask_var(bb_info->pemask);
+	kfree(bb_info->bw);
+	kmem_cache_free(bb_info_cachep, bb_info);
+}
+
+static struct bb_info *alloc_bb_info(void)
+{
+	struct bb_info *bb_info;
+
+	bb_info = kmem_cache_zalloc(bb_info_cachep, GFP_KERNEL);
+	if (!bb_info)
+		return NULL;
+
+	bb_info->bw = kcalloc(_hwinfo.max_pe_per_cmg, sizeof(u8), GFP_KERNEL);
+	if (!bb_info->bw) {
+		free_bb_info(&bb_info->kref);
+		return NULL;
+	}
+	if (!zalloc_cpumask_var(&bb_info->pemask, GFP_KERNEL) ||
+		!zalloc_cpumask_var(&bb_info->assigned_pemask, GFP_KERNEL)) {
+		free_bb_info(&bb_info->kref);
+		return NULL;
+	}
+
+	init_waitqueue_head(&bb_info->wq);
+	kref_init(&bb_info->kref);
+
+	return bb_info;
+}
+
+static inline void put_bb_info(struct bb_info *bb_info)
+{
+	kref_put(&bb_info->kref, free_bb_info);
+}
+
+/* Validate pemask's range and convert it to a mask based on physical PE number */
+static int validate_and_convert_pemask(struct bb_info *bb_info, unsigned long *phys_pemask)
+{
+	int cpu;
+	u8 cmg;
+
+	if (cpumask_weight(bb_info->pemask) < 2) {
+		pr_err("pemask needs at least two bit set: %*pbl\n",
+						cpumask_pr_args(bb_info->pemask));
+		return -EINVAL;
+	}
+
+	if (!cpumask_subset(bb_info->pemask, cpu_online_mask)) {
+		pr_err("pemask needs to be subset of online cpu: %*pbl, %*pbl\n",
+			cpumask_pr_args(bb_info->pemask), cpumask_pr_args(cpu_online_mask));
+		return -EINVAL;
+	}
+
+	/*
+	 * INIT_SYNC register requires a mask value based on physical PE number.
+	 * So convert pemask to it while checking if all PEs belongs to the same CMG
+	 */
+	cpu = cpumask_first(bb_info->pemask);
+	cmg = _hwinfo.core_map[cpu].cmg;
+	*phys_pemask = 0;
+	for_each_cpu(cpu, bb_info->pemask) {
+		if (_hwinfo.core_map[cpu].cmg != cmg) {
+			pr_err("All PEs must belong to the same CMG: %*pbl\n",
+							cpumask_pr_args(bb_info->pemask));
+			return -EINVAL;
+		}
+		set_bit(_hwinfo.core_map[cpu].ppe, phys_pemask);
+	}
+	bb_info->cmg = cmg;
+
+	pr_debug("pemask: %*pbl, physical_pemask: %lx\n",
+					cpumask_pr_args(bb_info->pemask), *phys_pemask);
+
+	return 0;
+}
+
+/* Search free BB in_hwinfo->used_bb_bitmap[cmg] */
+static int search_free_bb(u8 cmg)
+{
+	int i;
+
+	for (i = 0; i < _hwinfo.num_bb; i++) {
+		if (!test_and_set_bit(i, &_hwinfo.used_bb_bmap[cmg])) {
+			pr_debug("Use BB %u in CMG %u, bitmap: %lx\n",
+							i, cmg, _hwinfo.used_bb_bmap[cmg]);
+			return i;
+		}
+	}
+
+	pr_err("All barrier blade is currently used in CMG %u\n", cmg);
+	return -EBUSY;
+}
+
+struct init_sync_args {
+	u64 val;
+	u8 bb;
+};
+
+static void write_init_sync_reg(void *args)
+{
+	struct init_sync_args *sync_args = (struct init_sync_args *)args;
+
+	switch (sync_args->bb) {
+	case 0:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB0_EL1);
+		break;
+	case 1:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB1_EL1);
+		break;
+	case 2:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB2_EL1);
+		break;
+	case 3:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB3_EL1);
+		break;
+	case 4:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB4_EL1);
+		break;
+	case 5:
+		write_sysreg_s(sync_args->val, FHWB_INIT_SYNC_BB5_EL1);
+		break;
+	}
+}
+
+/* Send IPI to initialize INIT_SYNC register */
+static void setup_bb(struct bb_info *bb_info, unsigned long phys_pemask)
+{
+	struct init_sync_args args = {0};
+	int cpu;
+
+	/* INIT_SYNC register is shared resource in CMG. Pick one PE to set it up */
+	cpu = cpumask_any(bb_info->pemask);
+
+	args.bb = bb_info->bb;
+	args.val = FIELD_PREP(FHWB_INIT_SYNC_BB_EL1_MASK_FIELD, phys_pemask);
+	on_each_cpu_mask(cpumask_of(cpu), write_init_sync_reg, &args, 1);
+
+	pr_debug("Setup bb. cpu: %d, CMG: %u, BB: %u, bimtap: %lx\n",
+			cpu, bb_info->cmg, bb_info->bb, _hwinfo.used_bb_bmap[bb_info->cmg]);
+}
+
+static int ioc_bb_alloc(struct file *filp, void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bb_ctl bb_ctl;
+	struct bb_info *bb_info;
+	unsigned long physical_pemask;
+	unsigned int size;
+	int ret;
+
+	if (copy_from_user(&bb_ctl, (struct fujitsu_hwb_ioc_bb_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bb_ctl)))
+		return -EFAULT;
+
+	bb_info = alloc_bb_info();
+	if (!bb_info)
+		return -ENOMEM;
+
+	/* cpumask size may vary in user and kernel space. Use the smaller one */
+	size = min(cpumask_size(), bb_ctl.size);
+	if (copy_from_user(bb_info->pemask, bb_ctl.pemask, size)) {
+		ret = -EFAULT;
+		goto put_bb_info;
+	}
+
+	ret = validate_and_convert_pemask(bb_info, &physical_pemask);
+	if (ret < 0)
+		goto put_bb_info;
+
+	ret = search_free_bb(bb_info->cmg);
+	if (ret < 0)
+		goto put_bb_info;
+	bb_info->bb = ret;
+
+	/* Copy back CMG/BB number to be used to user */
+	bb_ctl.cmg = bb_info->cmg;
+	bb_ctl.bb  = bb_info->bb;
+	if (copy_to_user((struct fujitsu_hwb_ioc_bb_ctl __user *)argp, &bb_ctl,
+						sizeof(struct fujitsu_hwb_ioc_bb_ctl))) {
+		ret = -EFAULT;
+		clear_bit(bb_ctl.bb, &_hwinfo.used_bb_bmap[bb_ctl.cmg]);
+		goto put_bb_info;
+	}
+
+	setup_bb(bb_info, physical_pemask);
+
+	spin_lock(&pdata->list_lock);
+	list_add_tail(&bb_info->node, &pdata->bb_list);
+	spin_unlock(&pdata->list_lock);
+
+	return 0;
+
+put_bb_info:
+	put_bb_info(bb_info);
+
+	return ret;
+}
+
+static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
+{
+	void __user *argp = (void __user *)arg;
+	int ret;
+
+	switch (cmd) {
+	case FUJITSU_HWB_IOC_BB_ALLOC:
+		ret = ioc_bb_alloc(filp, argp);
+		break;
+	default:
+		ret = -ENOTTY;
+		break;
+	}
+
+	return ret;
+}
+
 static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
 {
 	struct hwb_private_data *pdata;
@@ -164,6 +386,7 @@ static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
 static const struct file_operations fujitsu_hwb_dev_fops = {
 	.owner          = THIS_MODULE,
 	.open           = fujitsu_hwb_dev_open,
+	.unlocked_ioctl = fujitsu_hwb_dev_ioctl,
 };
 
 static struct miscdevice bar_miscdev = {
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
new file mode 100644
index 000000000000..c87a5bad3f59
--- /dev/null
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/* Copyright 2020 FUJITSU LIMITED */
+#ifndef _UAPI_LINUX_FUJITSU_HPC_IOC_H
+#define _UAPI_LINUX_FUJITSU_HPC_IOC_H
+
+#include <linux/ioctl.h>
+#include <asm/types.h>
+
+#define __FUJITSU_IOCTL_MAGIC 'F'
+
+/* ioctl definitions for hardware barrier driver */
+struct fujitsu_hwb_ioc_bb_ctl {
+	__u8 cmg;
+	__u8 bb;
+	__u8 unused[2];
+	__u32 size;
+	unsigned long __user *pemask;
+};
+
+#define FUJITSU_HWB_IOC_BB_ALLOC _IOWR(__FUJITSU_IOCTL_MAGIC, \
+	0x00, struct fujitsu_hwb_ioc_bb_ctl)
+
+#endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 04/10] soc: fujitsu: hwb: Add IOC_BW_ASSIGN ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

IOC_BW_ASSIGN ioctl sets up control register and window register on each
PE. Therefore, this ioctl will be called as many times as the number of
PEs joining synchronization. Also, the caller thread is expected to be
bound to one PE at this point.

Since barrier window and control register is per-PE resource and
context switch is not supported at this point, we forbid concurrent
running of ioc_bw_assign() on the same PE by disabling preemption.

After this ioctl returns successfully, user program (EL0) can access
BST_SYNC/LBSY_SYNC registers directly to realize synchronization.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 187 +++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |   7 +
 2 files changed, 194 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 24d1bb00f55c..85ffc1642dd9 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -179,6 +179,34 @@ static struct bb_info *alloc_bb_info(void)
 	return bb_info;
 }
 
+static struct bb_info *get_bb_info(struct hwb_private_data *pdata, u8 cmg, u8 bb)
+{
+	struct bb_info *bb_info;
+
+	if (cmg >= _hwinfo.num_cmg || bb >= _hwinfo.num_bb) {
+		pr_err("CMG/BB number is invalid: %u/%u\n", cmg, bb);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!test_bit(bb, &_hwinfo.used_bb_bmap[cmg])) {
+		pr_err("BB is not allocated: %u/%u\n", cmg, bb);
+		return ERR_PTR(-ENOENT);
+	}
+
+	spin_lock(&pdata->list_lock);
+	list_for_each_entry(bb_info, &pdata->bb_list, node) {
+		if (bb_info->cmg == cmg && bb_info->bb == bb) {
+			kref_get(&bb_info->kref);
+			spin_unlock(&pdata->list_lock);
+			return bb_info;
+		}
+	}
+	spin_unlock(&pdata->list_lock);
+
+	pr_err("BB is not allocated by this process: %u/%u\n", cmg, bb);
+	return ERR_PTR(-EPERM);
+}
+
 static inline void put_bb_info(struct bb_info *bb_info)
 {
 	kref_put(&bb_info->kref, free_bb_info);
@@ -347,6 +375,162 @@ static int ioc_bb_alloc(struct file *filp, void __user *argp)
 	return ret;
 }
 
+static bool is_bound_only_one_pe(void)
+{
+	if (current->nr_cpus_allowed == 1)
+		return true;
+
+	pr_err("Thread must be bound to one PE between assign and unassign\n");
+	return false;
+}
+
+/* Check if this PE can be assignable and set window number to be used to @bw_ctl->window */
+static int is_bw_assignable(struct bb_info *bb_info, struct fujitsu_hwb_ioc_bw_ctl *bw_ctl, int cpu)
+{
+	int i;
+
+	if (!cpumask_test_cpu(cpu, bb_info->pemask)) {
+		pr_err("This pe is not supposed to join sync, %u/%u/%d\n",
+						bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	if (cpumask_test_cpu(cpu, bb_info->assigned_pemask)) {
+		pr_err("This pe is already assigned to window: %u/%u/%d\n",
+						bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	if (bw_ctl->window >= 0) {
+		/* User specifies window number to use. Check if available */
+		if (bw_ctl->window >= _hwinfo.num_bw) {
+			pr_err("Window number is invalid: %u/%u/%d/%u\n",
+						bb_info->cmg, bb_info->bb, cpu, bw_ctl->window);
+			return -EINVAL;
+		}
+
+		if (test_bit(bw_ctl->window, &_hwinfo.used_bw_bmap[cpu])) {
+			pr_err("Window is already used: %u/%u/%d/%u\n",
+						bb_info->cmg, bb_info->bb, cpu, bw_ctl->window);
+			return -EBUSY;
+		}
+	} else {
+		/* User does not specify window number. Use free window */
+		i = ffz(_hwinfo.used_bw_bmap[cpu]);
+		if (i == _hwinfo.num_bw) {
+			pr_err("There is no free window: %u/%u/%d\n",
+					bb_info->cmg, bb_info->bb, cpu);
+			return -EBUSY;
+		}
+
+		bw_ctl->window = i;
+	}
+
+	return 0;
+}
+
+static void setup_ctl_reg(struct bb_info *bb_info, int cpu)
+{
+	u64 val;
+
+	if (_hwinfo.used_bw_bmap[cpu] != 0)
+		/* Already setup. Nothing todo */
+		return;
+
+	/*
+	 * This is the first assign on this PE.
+	 * Setup ctrl reg to allow access to BST_SYNC/LBSY_SYNC from EL0
+	 */
+	val = (FHWB_CTRL_EL1_EL1AE | FHWB_CTRL_EL1_EL0AE);
+	write_sysreg_s(val, FHWB_CTRL_EL1);
+
+	pr_debug("Setup ctl reg. cpu: %d\n", cpu);
+}
+
+static void write_bw_reg(u8 window, u64 val)
+{
+	switch (window) {
+	case 0:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W0_EL1);
+		break;
+	case 1:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W1_EL1);
+		break;
+	case 2:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W2_EL1);
+		break;
+	case 3:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W3_EL1);
+		break;
+	}
+}
+
+static void setup_bw(struct bb_info *bb_info, struct fujitsu_hwb_ioc_bw_ctl *bw_ctl, int cpu)
+{
+	u64 val;
+	u8 ppe;
+
+	/* Set valid bit and bb number */
+	val = (FHWB_ASSIGN_SYNC_W_EL1_VALID | bw_ctl->bb);
+	write_bw_reg(bw_ctl->window, val);
+
+	/* Update bitmap info */
+	ppe = _hwinfo.core_map[cpu].ppe;
+	set_bit(bw_ctl->window, &_hwinfo.used_bw_bmap[cpu]);
+	cpumask_set_cpu(cpu, bb_info->assigned_pemask);
+	bb_info->bw[ppe] = bw_ctl->window;
+
+	pr_debug("Setup bw. cpu: %d, window: %u, BB: %u, bw_bmap: %lx, assigned_pemask: %*pbl\n",
+			cpu, bw_ctl->window, bw_ctl->bb,
+			_hwinfo.used_bw_bmap[cpu], cpumask_pr_args(bb_info->assigned_pemask));
+}
+
+static int ioc_bw_assign(struct file *filp, void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
+	struct bb_info *bb_info;
+	int ret;
+	int cpu;
+	u8 cmg;
+
+	if (!is_bound_only_one_pe())
+		return -EPERM;
+
+	if (copy_from_user(&bw_ctl, (struct fujitsu_hwb_ioc_bw_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bw_ctl)))
+		return -EFAULT;
+
+	cpu = smp_processor_id();
+	cmg = _hwinfo.core_map[cpu].cmg;
+	bb_info = get_bb_info(pdata, cmg, bw_ctl.bb);
+	if (IS_ERR(bb_info))
+		return PTR_ERR(bb_info);
+
+	/*
+	 * Barrier window register and control register is each PE's resource.
+	 * context switch is not supported and mutual exclusion is needed for
+	 * assign and unassign on this PE
+	 */
+	preempt_disable();
+	ret = is_bw_assignable(bb_info, &bw_ctl, cpu);
+	if (!ret) {
+		setup_ctl_reg(bb_info, cpu);
+		setup_bw(bb_info, &bw_ctl, cpu);
+	}
+	preempt_enable();
+
+	put_bb_info(bb_info);
+
+	/* Copy back window number to be used to user */
+	if (!ret && copy_to_user((struct fujitsu_hwb_ioc_bw_ctl __user *)argp, &bw_ctl,
+						sizeof(struct fujitsu_hwb_ioc_bw_ctl)))
+		/* Leave cleanup to f_op->release() */
+		return -EFAULT;
+
+	return ret;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -356,6 +540,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BB_ALLOC:
 		ret = ioc_bb_alloc(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_BW_ASSIGN:
+		ret = ioc_bw_assign(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index c87a5bad3f59..ad90f8f3ae9a 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -17,7 +17,14 @@ struct fujitsu_hwb_ioc_bb_ctl {
 	unsigned long __user *pemask;
 };
 
+struct fujitsu_hwb_ioc_bw_ctl {
+	__u8 bb;
+	__s8 window;
+};
+
 #define FUJITSU_HWB_IOC_BB_ALLOC _IOWR(__FUJITSU_IOCTL_MAGIC, \
 	0x00, struct fujitsu_hwb_ioc_bb_ctl)
+#define FUJITSU_HWB_IOC_BW_ASSIGN _IOWR(__FUJITSU_IOCTL_MAGIC, \
+	0x01, struct fujitsu_hwb_ioc_bw_ctl)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 04/10] soc: fujitsu: hwb: Add IOC_BW_ASSIGN ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

IOC_BW_ASSIGN ioctl sets up control register and window register on each
PE. Therefore, this ioctl will be called as many times as the number of
PEs joining synchronization. Also, the caller thread is expected to be
bound to one PE at this point.

Since barrier window and control register is per-PE resource and
context switch is not supported at this point, we forbid concurrent
running of ioc_bw_assign() on the same PE by disabling preemption.

After this ioctl returns successfully, user program (EL0) can access
BST_SYNC/LBSY_SYNC registers directly to realize synchronization.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 187 +++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |   7 +
 2 files changed, 194 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 24d1bb00f55c..85ffc1642dd9 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -179,6 +179,34 @@ static struct bb_info *alloc_bb_info(void)
 	return bb_info;
 }
 
+static struct bb_info *get_bb_info(struct hwb_private_data *pdata, u8 cmg, u8 bb)
+{
+	struct bb_info *bb_info;
+
+	if (cmg >= _hwinfo.num_cmg || bb >= _hwinfo.num_bb) {
+		pr_err("CMG/BB number is invalid: %u/%u\n", cmg, bb);
+		return ERR_PTR(-EINVAL);
+	}
+
+	if (!test_bit(bb, &_hwinfo.used_bb_bmap[cmg])) {
+		pr_err("BB is not allocated: %u/%u\n", cmg, bb);
+		return ERR_PTR(-ENOENT);
+	}
+
+	spin_lock(&pdata->list_lock);
+	list_for_each_entry(bb_info, &pdata->bb_list, node) {
+		if (bb_info->cmg == cmg && bb_info->bb == bb) {
+			kref_get(&bb_info->kref);
+			spin_unlock(&pdata->list_lock);
+			return bb_info;
+		}
+	}
+	spin_unlock(&pdata->list_lock);
+
+	pr_err("BB is not allocated by this process: %u/%u\n", cmg, bb);
+	return ERR_PTR(-EPERM);
+}
+
 static inline void put_bb_info(struct bb_info *bb_info)
 {
 	kref_put(&bb_info->kref, free_bb_info);
@@ -347,6 +375,162 @@ static int ioc_bb_alloc(struct file *filp, void __user *argp)
 	return ret;
 }
 
+static bool is_bound_only_one_pe(void)
+{
+	if (current->nr_cpus_allowed == 1)
+		return true;
+
+	pr_err("Thread must be bound to one PE between assign and unassign\n");
+	return false;
+}
+
+/* Check if this PE can be assignable and set window number to be used to @bw_ctl->window */
+static int is_bw_assignable(struct bb_info *bb_info, struct fujitsu_hwb_ioc_bw_ctl *bw_ctl, int cpu)
+{
+	int i;
+
+	if (!cpumask_test_cpu(cpu, bb_info->pemask)) {
+		pr_err("This pe is not supposed to join sync, %u/%u/%d\n",
+						bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	if (cpumask_test_cpu(cpu, bb_info->assigned_pemask)) {
+		pr_err("This pe is already assigned to window: %u/%u/%d\n",
+						bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	if (bw_ctl->window >= 0) {
+		/* User specifies window number to use. Check if available */
+		if (bw_ctl->window >= _hwinfo.num_bw) {
+			pr_err("Window number is invalid: %u/%u/%d/%u\n",
+						bb_info->cmg, bb_info->bb, cpu, bw_ctl->window);
+			return -EINVAL;
+		}
+
+		if (test_bit(bw_ctl->window, &_hwinfo.used_bw_bmap[cpu])) {
+			pr_err("Window is already used: %u/%u/%d/%u\n",
+						bb_info->cmg, bb_info->bb, cpu, bw_ctl->window);
+			return -EBUSY;
+		}
+	} else {
+		/* User does not specify window number. Use free window */
+		i = ffz(_hwinfo.used_bw_bmap[cpu]);
+		if (i == _hwinfo.num_bw) {
+			pr_err("There is no free window: %u/%u/%d\n",
+					bb_info->cmg, bb_info->bb, cpu);
+			return -EBUSY;
+		}
+
+		bw_ctl->window = i;
+	}
+
+	return 0;
+}
+
+static void setup_ctl_reg(struct bb_info *bb_info, int cpu)
+{
+	u64 val;
+
+	if (_hwinfo.used_bw_bmap[cpu] != 0)
+		/* Already setup. Nothing todo */
+		return;
+
+	/*
+	 * This is the first assign on this PE.
+	 * Setup ctrl reg to allow access to BST_SYNC/LBSY_SYNC from EL0
+	 */
+	val = (FHWB_CTRL_EL1_EL1AE | FHWB_CTRL_EL1_EL0AE);
+	write_sysreg_s(val, FHWB_CTRL_EL1);
+
+	pr_debug("Setup ctl reg. cpu: %d\n", cpu);
+}
+
+static void write_bw_reg(u8 window, u64 val)
+{
+	switch (window) {
+	case 0:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W0_EL1);
+		break;
+	case 1:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W1_EL1);
+		break;
+	case 2:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W2_EL1);
+		break;
+	case 3:
+		write_sysreg_s(val, FHWB_ASSIGN_SYNC_W3_EL1);
+		break;
+	}
+}
+
+static void setup_bw(struct bb_info *bb_info, struct fujitsu_hwb_ioc_bw_ctl *bw_ctl, int cpu)
+{
+	u64 val;
+	u8 ppe;
+
+	/* Set valid bit and bb number */
+	val = (FHWB_ASSIGN_SYNC_W_EL1_VALID | bw_ctl->bb);
+	write_bw_reg(bw_ctl->window, val);
+
+	/* Update bitmap info */
+	ppe = _hwinfo.core_map[cpu].ppe;
+	set_bit(bw_ctl->window, &_hwinfo.used_bw_bmap[cpu]);
+	cpumask_set_cpu(cpu, bb_info->assigned_pemask);
+	bb_info->bw[ppe] = bw_ctl->window;
+
+	pr_debug("Setup bw. cpu: %d, window: %u, BB: %u, bw_bmap: %lx, assigned_pemask: %*pbl\n",
+			cpu, bw_ctl->window, bw_ctl->bb,
+			_hwinfo.used_bw_bmap[cpu], cpumask_pr_args(bb_info->assigned_pemask));
+}
+
+static int ioc_bw_assign(struct file *filp, void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
+	struct bb_info *bb_info;
+	int ret;
+	int cpu;
+	u8 cmg;
+
+	if (!is_bound_only_one_pe())
+		return -EPERM;
+
+	if (copy_from_user(&bw_ctl, (struct fujitsu_hwb_ioc_bw_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bw_ctl)))
+		return -EFAULT;
+
+	cpu = smp_processor_id();
+	cmg = _hwinfo.core_map[cpu].cmg;
+	bb_info = get_bb_info(pdata, cmg, bw_ctl.bb);
+	if (IS_ERR(bb_info))
+		return PTR_ERR(bb_info);
+
+	/*
+	 * Barrier window register and control register is each PE's resource.
+	 * context switch is not supported and mutual exclusion is needed for
+	 * assign and unassign on this PE
+	 */
+	preempt_disable();
+	ret = is_bw_assignable(bb_info, &bw_ctl, cpu);
+	if (!ret) {
+		setup_ctl_reg(bb_info, cpu);
+		setup_bw(bb_info, &bw_ctl, cpu);
+	}
+	preempt_enable();
+
+	put_bb_info(bb_info);
+
+	/* Copy back window number to be used to user */
+	if (!ret && copy_to_user((struct fujitsu_hwb_ioc_bw_ctl __user *)argp, &bw_ctl,
+						sizeof(struct fujitsu_hwb_ioc_bw_ctl)))
+		/* Leave cleanup to f_op->release() */
+		return -EFAULT;
+
+	return ret;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -356,6 +540,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BB_ALLOC:
 		ret = ioc_bb_alloc(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_BW_ASSIGN:
+		ret = ioc_bw_assign(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index c87a5bad3f59..ad90f8f3ae9a 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -17,7 +17,14 @@ struct fujitsu_hwb_ioc_bb_ctl {
 	unsigned long __user *pemask;
 };
 
+struct fujitsu_hwb_ioc_bw_ctl {
+	__u8 bb;
+	__s8 window;
+};
+
 #define FUJITSU_HWB_IOC_BB_ALLOC _IOWR(__FUJITSU_IOCTL_MAGIC, \
 	0x00, struct fujitsu_hwb_ioc_bb_ctl)
+#define FUJITSU_HWB_IOC_BW_ASSIGN _IOWR(__FUJITSU_IOCTL_MAGIC, \
+	0x01, struct fujitsu_hwb_ioc_bw_ctl)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 05/10] soc: fujitsu: hwb: Add IOC_BW_UNASSIGN ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

IOC_BW_UNASSIGN resets what IOC_BW_ASSIGN did on each PE.
This ioctl will also be called as many times as the number of PEs joining
synchronization.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 93 ++++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |  2 +
 2 files changed, 95 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 85ffc1642dd9..8c4cabd60872 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -531,6 +531,96 @@ static int ioc_bw_assign(struct file *filp, void __user *argp)
 	return ret;
 }
 
+static int is_bw_unassignable(struct bb_info *bb_info, int cpu)
+{
+	u8 ppe;
+
+	if (!cpumask_test_and_clear_cpu(cpu, bb_info->assigned_pemask)) {
+		pr_err("This pe is not assigned: %u/%u/%d\n", bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	ppe = _hwinfo.core_map[cpu].ppe;
+	if (!test_bit(bb_info->bw[ppe], &_hwinfo.used_bw_bmap[cpu])) {
+		/* should not happen */
+		pr_crit("Logic error. This window is not assigned: %u/%u/%d\n",
+							bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void teardown_ctl_reg(struct bb_info *bb_info, int cpu)
+{
+	if (_hwinfo.used_bw_bmap[cpu] != 0)
+		/* Other window on this PE is still in use. Nothing todo */
+		return;
+
+	/*
+	 * This is the last unassign on this PE.
+	 * Clear all bits to disallow access to BST_SYNC/LBSY_SYNC from EL0
+	 */
+	write_sysreg_s(0, FHWB_CTRL_EL1);
+
+	pr_debug("Teardown ctl reg. cpu: %d\n", cpu);
+}
+
+static void teardown_bw(struct bb_info *bb_info, int cpu)
+{
+	u8 window;
+	u8 ppe;
+
+	/* Just clear all bits */
+	ppe = _hwinfo.core_map[cpu].ppe;
+	window = bb_info->bw[ppe];
+	write_bw_reg(window, 0);
+
+	/* Update bitmap info */
+	clear_bit(window, &_hwinfo.used_bw_bmap[cpu]);
+	bb_info->bw[ppe] = -1;
+
+	pr_debug("Teardown bw. cpu: %d, window: %u, BB: %u, bw_bmap: %lx, assigned_pemask: %*pbl\n",
+			cpu, window, bb_info->bb,
+			_hwinfo.used_bw_bmap[cpu], cpumask_pr_args(bb_info->assigned_pemask));
+}
+
+static int ioc_bw_unassign(struct file *filp, void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
+	struct bb_info *bb_info;
+	int cpu;
+	int ret;
+	u8 cmg;
+
+	if (!is_bound_only_one_pe())
+		return -EPERM;
+
+	if (copy_from_user(&bw_ctl, (struct fujitsu_hwb_ioc_bw_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bw_ctl)))
+		return -EFAULT;
+
+	cpu = smp_processor_id();
+	cmg = _hwinfo.core_map[cpu].cmg;
+	bb_info = get_bb_info(pdata, cmg, bw_ctl.bb);
+	if (IS_ERR(bb_info))
+		return PTR_ERR(bb_info);
+
+	/* See comments in ioc_bw_assign() */
+	preempt_disable();
+	ret = is_bw_unassignable(bb_info, cpu);
+	if (!ret) {
+		teardown_bw(bb_info, cpu);
+		teardown_ctl_reg(bb_info, cpu);
+	}
+	preempt_enable();
+
+	put_bb_info(bb_info);
+
+	return ret;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -543,6 +633,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BW_ASSIGN:
 		ret = ioc_bw_assign(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_BW_UNASSIGN:
+		ret = ioc_bw_unassign(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index ad90f8f3ae9a..396029f2bc0d 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -26,5 +26,7 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	0x00, struct fujitsu_hwb_ioc_bb_ctl)
 #define FUJITSU_HWB_IOC_BW_ASSIGN _IOWR(__FUJITSU_IOCTL_MAGIC, \
 	0x01, struct fujitsu_hwb_ioc_bw_ctl)
+#define FUJITSU_HWB_IOC_BW_UNASSIGN _IOW(__FUJITSU_IOCTL_MAGIC, \
+	0x02, struct fujitsu_hwb_ioc_bw_ctl)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 05/10] soc: fujitsu: hwb: Add IOC_BW_UNASSIGN ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

IOC_BW_UNASSIGN resets what IOC_BW_ASSIGN did on each PE.
This ioctl will also be called as many times as the number of PEs joining
synchronization.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 93 ++++++++++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |  2 +
 2 files changed, 95 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 85ffc1642dd9..8c4cabd60872 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -531,6 +531,96 @@ static int ioc_bw_assign(struct file *filp, void __user *argp)
 	return ret;
 }
 
+static int is_bw_unassignable(struct bb_info *bb_info, int cpu)
+{
+	u8 ppe;
+
+	if (!cpumask_test_and_clear_cpu(cpu, bb_info->assigned_pemask)) {
+		pr_err("This pe is not assigned: %u/%u/%d\n", bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	ppe = _hwinfo.core_map[cpu].ppe;
+	if (!test_bit(bb_info->bw[ppe], &_hwinfo.used_bw_bmap[cpu])) {
+		/* should not happen */
+		pr_crit("Logic error. This window is not assigned: %u/%u/%d\n",
+							bb_info->cmg, bb_info->bb, cpu);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void teardown_ctl_reg(struct bb_info *bb_info, int cpu)
+{
+	if (_hwinfo.used_bw_bmap[cpu] != 0)
+		/* Other window on this PE is still in use. Nothing todo */
+		return;
+
+	/*
+	 * This is the last unassign on this PE.
+	 * Clear all bits to disallow access to BST_SYNC/LBSY_SYNC from EL0
+	 */
+	write_sysreg_s(0, FHWB_CTRL_EL1);
+
+	pr_debug("Teardown ctl reg. cpu: %d\n", cpu);
+}
+
+static void teardown_bw(struct bb_info *bb_info, int cpu)
+{
+	u8 window;
+	u8 ppe;
+
+	/* Just clear all bits */
+	ppe = _hwinfo.core_map[cpu].ppe;
+	window = bb_info->bw[ppe];
+	write_bw_reg(window, 0);
+
+	/* Update bitmap info */
+	clear_bit(window, &_hwinfo.used_bw_bmap[cpu]);
+	bb_info->bw[ppe] = -1;
+
+	pr_debug("Teardown bw. cpu: %d, window: %u, BB: %u, bw_bmap: %lx, assigned_pemask: %*pbl\n",
+			cpu, window, bb_info->bb,
+			_hwinfo.used_bw_bmap[cpu], cpumask_pr_args(bb_info->assigned_pemask));
+}
+
+static int ioc_bw_unassign(struct file *filp, void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
+	struct bb_info *bb_info;
+	int cpu;
+	int ret;
+	u8 cmg;
+
+	if (!is_bound_only_one_pe())
+		return -EPERM;
+
+	if (copy_from_user(&bw_ctl, (struct fujitsu_hwb_ioc_bw_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bw_ctl)))
+		return -EFAULT;
+
+	cpu = smp_processor_id();
+	cmg = _hwinfo.core_map[cpu].cmg;
+	bb_info = get_bb_info(pdata, cmg, bw_ctl.bb);
+	if (IS_ERR(bb_info))
+		return PTR_ERR(bb_info);
+
+	/* See comments in ioc_bw_assign() */
+	preempt_disable();
+	ret = is_bw_unassignable(bb_info, cpu);
+	if (!ret) {
+		teardown_bw(bb_info, cpu);
+		teardown_ctl_reg(bb_info, cpu);
+	}
+	preempt_enable();
+
+	put_bb_info(bb_info);
+
+	return ret;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -543,6 +633,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BW_ASSIGN:
 		ret = ioc_bw_assign(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_BW_UNASSIGN:
+		ret = ioc_bw_unassign(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index ad90f8f3ae9a..396029f2bc0d 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -26,5 +26,7 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	0x00, struct fujitsu_hwb_ioc_bb_ctl)
 #define FUJITSU_HWB_IOC_BW_ASSIGN _IOWR(__FUJITSU_IOCTL_MAGIC, \
 	0x01, struct fujitsu_hwb_ioc_bw_ctl)
+#define FUJITSU_HWB_IOC_BW_UNASSIGN _IOW(__FUJITSU_IOCTL_MAGIC, \
+	0x02, struct fujitsu_hwb_ioc_bw_ctl)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 06/10] soc: fujitsu: hwb: Add IOC_BB_FREE ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

IOC_BB_FREE ioctl resets what IOC_BB_ALLOC ioctl did.

We need to forbid assign/unassign operation happens during free
operation, so we set the flag to indicate it and also wait
ongoing assign/unassign to finish first.

If there exist PEs on which IOC_BW_UNASSIGN is not called,
we send IPI to do effectively the same operation as IOC_BW_UNASSIGN.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 125 ++++++++++++++++++++++++-
 include/uapi/linux/fujitsu_hpc_ioctl.h |   2 +
 2 files changed, 122 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 8c4cabd60872..2535942cc0d7 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -196,6 +196,12 @@ static struct bb_info *get_bb_info(struct hwb_private_data *pdata, u8 cmg, u8 bb
 	spin_lock(&pdata->list_lock);
 	list_for_each_entry(bb_info, &pdata->bb_list, node) {
 		if (bb_info->cmg == cmg && bb_info->bb == bb) {
+			if (test_bit(BB_FREEING,  &bb_info->flag)) {
+				pr_err("BB is currently being freed: %u/%u\n", cmg, bb);
+				spin_unlock(&pdata->list_lock);
+				return ERR_PTR(-EPERM);
+			}
+
 			kref_get(&bb_info->kref);
 			spin_unlock(&pdata->list_lock);
 			return bb_info;
@@ -389,6 +395,11 @@ static int is_bw_assignable(struct bb_info *bb_info, struct fujitsu_hwb_ioc_bw_c
 {
 	int i;
 
+	if (test_bit(BB_FREEING, &bb_info->flag)) {
+		pr_err("BB is currently being freed: %u/%u/%d\n", bb_info->cmg, bb_info->bb, cpu);
+		return -EPERM;
+	}
+
 	if (!cpumask_test_cpu(cpu, bb_info->pemask)) {
 		pr_err("This pe is not supposed to join sync, %u/%u/%d\n",
 						bb_info->cmg, bb_info->bb, cpu);
@@ -490,6 +501,7 @@ static int ioc_bw_assign(struct file *filp, void __user *argp)
 	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
 	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
 	struct bb_info *bb_info;
+	unsigned long flags;
 	int ret;
 	int cpu;
 	u8 cmg;
@@ -507,18 +519,27 @@ static int ioc_bw_assign(struct file *filp, void __user *argp)
 	if (IS_ERR(bb_info))
 		return PTR_ERR(bb_info);
 
+	/* Increment counter to avoid this BB being freed during assign operation */
+	atomic_inc(&bb_info->ongoing_assign_count);
+
 	/*
 	 * Barrier window register and control register is each PE's resource.
 	 * context switch is not supported and mutual exclusion is needed for
-	 * assign and unassign on this PE
+	 * assign and unassign on this PE. As cleanup_bw() might be executed
+	 * in interrupt context via on_each_cpu_mask, disabling irq is needed
 	 */
-	preempt_disable();
+	local_irq_save(flags);
 	ret = is_bw_assignable(bb_info, &bw_ctl, cpu);
 	if (!ret) {
 		setup_ctl_reg(bb_info, cpu);
 		setup_bw(bb_info, &bw_ctl, cpu);
 	}
-	preempt_enable();
+	local_irq_restore(flags);
+
+	/* Wakeup if there is a process waiting in ioc_bb_free() */
+	if (atomic_dec_and_test(&bb_info->ongoing_assign_count) &&
+					test_bit(BB_FREEING, &bb_info->flag))
+		wake_up(&bb_info->wq);
 
 	put_bb_info(bb_info);
 
@@ -535,6 +556,12 @@ static int is_bw_unassignable(struct bb_info *bb_info, int cpu)
 {
 	u8 ppe;
 
+	if (test_bit(BB_FREEING, &bb_info->flag)) {
+		pr_err("This bb is currently being freed: %u/%u/%d\n",
+							bb_info->cmg, bb_info->bb, cpu);
+		return -EPERM;
+	}
+
 	if (!cpumask_test_and_clear_cpu(cpu, bb_info->assigned_pemask)) {
 		pr_err("This pe is not assigned: %u/%u/%d\n", bb_info->cmg, bb_info->bb, cpu);
 		return -EINVAL;
@@ -590,6 +617,7 @@ static int ioc_bw_unassign(struct file *filp, void __user *argp)
 	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
 	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
 	struct bb_info *bb_info;
+	unsigned long flags;
 	int cpu;
 	int ret;
 	u8 cmg;
@@ -608,19 +636,103 @@ static int ioc_bw_unassign(struct file *filp, void __user *argp)
 		return PTR_ERR(bb_info);
 
 	/* See comments in ioc_bw_assign() */
-	preempt_disable();
+	atomic_inc(&bb_info->ongoing_assign_count);
+
+	local_irq_save(flags);
 	ret = is_bw_unassignable(bb_info, cpu);
 	if (!ret) {
 		teardown_bw(bb_info, cpu);
 		teardown_ctl_reg(bb_info, cpu);
 	}
-	preempt_enable();
+	local_irq_restore(flags);
+
+	if (atomic_dec_and_test(&bb_info->ongoing_assign_count) &&
+					test_bit(BB_FREEING, &bb_info->flag))
+		wake_up(&bb_info->wq);
 
 	put_bb_info(bb_info);
 
 	return ret;
 }
 
+static void cleanup_bw_func(void *args)
+{
+	struct bb_info *bb_info = (struct bb_info *)args;
+	int cpu = smp_processor_id();
+
+	teardown_bw(bb_info, cpu);
+	teardown_ctl_reg(bb_info, cpu);
+}
+
+/* Send IPI to reset INIT_SYNC register */
+static void teardown_bb(struct bb_info *bb_info)
+{
+	struct init_sync_args args = {0};
+	int cpu;
+
+	/* Reset BW on each PE if IOC_BW_UNASSIGN is not called properly  */
+	if (cpumask_weight(bb_info->assigned_pemask) != 0) {
+		pr_warn("unassign is not called properly. CMG: %d, BB: %d, unassigned PE: %*pbl\n",
+			bb_info->cmg, bb_info->bb, cpumask_pr_args(bb_info->assigned_pemask));
+		on_each_cpu_mask(bb_info->assigned_pemask, cleanup_bw_func, bb_info, 1);
+	}
+
+	/* INIT_SYNC register is shared resource in CMG. Pick one PE */
+	cpu = cpumask_any(bb_info->pemask);
+
+	args.bb = bb_info->bb;
+	/* Just clear all bits */
+	args.val = 0;
+	on_each_cpu_mask(cpumask_of(cpu), write_init_sync_reg, &args, 1);
+
+	clear_bit(bb_info->bb, &_hwinfo.used_bb_bmap[bb_info->cmg]);
+
+	pr_debug("Teardown bb: cpu: %d, CMG: %u, BB: %u, bitmap: %lx\n",
+			cpu, bb_info->cmg, bb_info->bb, _hwinfo.used_bb_bmap[bb_info->cmg]);
+}
+
+static int ioc_bb_free(struct file *filp,  void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bb_ctl bb_ctl;
+	struct bb_info *bb_info;
+
+	if (copy_from_user(&bb_ctl, (struct fujitsu_hwb_ioc_bb_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bb_ctl)))
+		return -EFAULT;
+
+	bb_info = get_bb_info(pdata, bb_ctl.cmg, bb_ctl.bb);
+	if (IS_ERR(bb_info))
+		return PTR_ERR(bb_info);
+
+	/* Forbid free/assign/unassign operation from now on */
+	if (test_and_set_bit(BB_FREEING, &bb_info->flag)) {
+		pr_err("IOC_BB_FREE is already called. CMG: %u, BB: %u\n", bb_ctl.cmg, bb_ctl.bb);
+		put_bb_info(bb_info);
+		return -EPERM;
+	}
+
+	/* Wait current ongoing assign/unassign operation to finish */
+	if (wait_event_interruptible(bb_info->wq,
+					(atomic_read(&bb_info->ongoing_assign_count) == 0))) {
+		clear_bit(BB_FREEING, &bb_info->flag);
+		put_bb_info(bb_info);
+		pr_debug("IOC_BB_FREE is interrupted. CMG: %u, BB: %u\n", bb_ctl.cmg, bb_ctl.bb);
+		return -EINTR;
+	}
+
+	teardown_bb(bb_info);
+	spin_lock(&pdata->list_lock);
+	list_del_init(&bb_info->node);
+	spin_unlock(&pdata->list_lock);
+
+	/* 1 put for get_bb_info, 1 for alloc_bb_info */
+	put_bb_info(bb_info);
+	put_bb_info(bb_info);
+
+	return 0;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -636,6 +748,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BW_UNASSIGN:
 		ret = ioc_bw_unassign(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_BB_FREE:
+		ret = ioc_bb_free(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index 396029f2bc0d..7a285d8db0a9 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -28,5 +28,7 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	0x01, struct fujitsu_hwb_ioc_bw_ctl)
 #define FUJITSU_HWB_IOC_BW_UNASSIGN _IOW(__FUJITSU_IOCTL_MAGIC, \
 	0x02, struct fujitsu_hwb_ioc_bw_ctl)
+#define FUJITSU_HWB_IOC_BB_FREE _IOW(__FUJITSU_IOCTL_MAGIC, \
+	0x03, struct fujitsu_hwb_ioc_bb_ctl)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 06/10] soc: fujitsu: hwb: Add IOC_BB_FREE ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

IOC_BB_FREE ioctl resets what IOC_BB_ALLOC ioctl did.

We need to forbid assign/unassign operation happens during free
operation, so we set the flag to indicate it and also wait
ongoing assign/unassign to finish first.

If there exist PEs on which IOC_BW_UNASSIGN is not called,
we send IPI to do effectively the same operation as IOC_BW_UNASSIGN.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 125 ++++++++++++++++++++++++-
 include/uapi/linux/fujitsu_hpc_ioctl.h |   2 +
 2 files changed, 122 insertions(+), 5 deletions(-)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 8c4cabd60872..2535942cc0d7 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -196,6 +196,12 @@ static struct bb_info *get_bb_info(struct hwb_private_data *pdata, u8 cmg, u8 bb
 	spin_lock(&pdata->list_lock);
 	list_for_each_entry(bb_info, &pdata->bb_list, node) {
 		if (bb_info->cmg == cmg && bb_info->bb == bb) {
+			if (test_bit(BB_FREEING,  &bb_info->flag)) {
+				pr_err("BB is currently being freed: %u/%u\n", cmg, bb);
+				spin_unlock(&pdata->list_lock);
+				return ERR_PTR(-EPERM);
+			}
+
 			kref_get(&bb_info->kref);
 			spin_unlock(&pdata->list_lock);
 			return bb_info;
@@ -389,6 +395,11 @@ static int is_bw_assignable(struct bb_info *bb_info, struct fujitsu_hwb_ioc_bw_c
 {
 	int i;
 
+	if (test_bit(BB_FREEING, &bb_info->flag)) {
+		pr_err("BB is currently being freed: %u/%u/%d\n", bb_info->cmg, bb_info->bb, cpu);
+		return -EPERM;
+	}
+
 	if (!cpumask_test_cpu(cpu, bb_info->pemask)) {
 		pr_err("This pe is not supposed to join sync, %u/%u/%d\n",
 						bb_info->cmg, bb_info->bb, cpu);
@@ -490,6 +501,7 @@ static int ioc_bw_assign(struct file *filp, void __user *argp)
 	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
 	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
 	struct bb_info *bb_info;
+	unsigned long flags;
 	int ret;
 	int cpu;
 	u8 cmg;
@@ -507,18 +519,27 @@ static int ioc_bw_assign(struct file *filp, void __user *argp)
 	if (IS_ERR(bb_info))
 		return PTR_ERR(bb_info);
 
+	/* Increment counter to avoid this BB being freed during assign operation */
+	atomic_inc(&bb_info->ongoing_assign_count);
+
 	/*
 	 * Barrier window register and control register is each PE's resource.
 	 * context switch is not supported and mutual exclusion is needed for
-	 * assign and unassign on this PE
+	 * assign and unassign on this PE. As cleanup_bw() might be executed
+	 * in interrupt context via on_each_cpu_mask, disabling irq is needed
 	 */
-	preempt_disable();
+	local_irq_save(flags);
 	ret = is_bw_assignable(bb_info, &bw_ctl, cpu);
 	if (!ret) {
 		setup_ctl_reg(bb_info, cpu);
 		setup_bw(bb_info, &bw_ctl, cpu);
 	}
-	preempt_enable();
+	local_irq_restore(flags);
+
+	/* Wakeup if there is a process waiting in ioc_bb_free() */
+	if (atomic_dec_and_test(&bb_info->ongoing_assign_count) &&
+					test_bit(BB_FREEING, &bb_info->flag))
+		wake_up(&bb_info->wq);
 
 	put_bb_info(bb_info);
 
@@ -535,6 +556,12 @@ static int is_bw_unassignable(struct bb_info *bb_info, int cpu)
 {
 	u8 ppe;
 
+	if (test_bit(BB_FREEING, &bb_info->flag)) {
+		pr_err("This bb is currently being freed: %u/%u/%d\n",
+							bb_info->cmg, bb_info->bb, cpu);
+		return -EPERM;
+	}
+
 	if (!cpumask_test_and_clear_cpu(cpu, bb_info->assigned_pemask)) {
 		pr_err("This pe is not assigned: %u/%u/%d\n", bb_info->cmg, bb_info->bb, cpu);
 		return -EINVAL;
@@ -590,6 +617,7 @@ static int ioc_bw_unassign(struct file *filp, void __user *argp)
 	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
 	struct fujitsu_hwb_ioc_bw_ctl bw_ctl;
 	struct bb_info *bb_info;
+	unsigned long flags;
 	int cpu;
 	int ret;
 	u8 cmg;
@@ -608,19 +636,103 @@ static int ioc_bw_unassign(struct file *filp, void __user *argp)
 		return PTR_ERR(bb_info);
 
 	/* See comments in ioc_bw_assign() */
-	preempt_disable();
+	atomic_inc(&bb_info->ongoing_assign_count);
+
+	local_irq_save(flags);
 	ret = is_bw_unassignable(bb_info, cpu);
 	if (!ret) {
 		teardown_bw(bb_info, cpu);
 		teardown_ctl_reg(bb_info, cpu);
 	}
-	preempt_enable();
+	local_irq_restore(flags);
+
+	if (atomic_dec_and_test(&bb_info->ongoing_assign_count) &&
+					test_bit(BB_FREEING, &bb_info->flag))
+		wake_up(&bb_info->wq);
 
 	put_bb_info(bb_info);
 
 	return ret;
 }
 
+static void cleanup_bw_func(void *args)
+{
+	struct bb_info *bb_info = (struct bb_info *)args;
+	int cpu = smp_processor_id();
+
+	teardown_bw(bb_info, cpu);
+	teardown_ctl_reg(bb_info, cpu);
+}
+
+/* Send IPI to reset INIT_SYNC register */
+static void teardown_bb(struct bb_info *bb_info)
+{
+	struct init_sync_args args = {0};
+	int cpu;
+
+	/* Reset BW on each PE if IOC_BW_UNASSIGN is not called properly  */
+	if (cpumask_weight(bb_info->assigned_pemask) != 0) {
+		pr_warn("unassign is not called properly. CMG: %d, BB: %d, unassigned PE: %*pbl\n",
+			bb_info->cmg, bb_info->bb, cpumask_pr_args(bb_info->assigned_pemask));
+		on_each_cpu_mask(bb_info->assigned_pemask, cleanup_bw_func, bb_info, 1);
+	}
+
+	/* INIT_SYNC register is shared resource in CMG. Pick one PE */
+	cpu = cpumask_any(bb_info->pemask);
+
+	args.bb = bb_info->bb;
+	/* Just clear all bits */
+	args.val = 0;
+	on_each_cpu_mask(cpumask_of(cpu), write_init_sync_reg, &args, 1);
+
+	clear_bit(bb_info->bb, &_hwinfo.used_bb_bmap[bb_info->cmg]);
+
+	pr_debug("Teardown bb: cpu: %d, CMG: %u, BB: %u, bitmap: %lx\n",
+			cpu, bb_info->cmg, bb_info->bb, _hwinfo.used_bb_bmap[bb_info->cmg]);
+}
+
+static int ioc_bb_free(struct file *filp,  void __user *argp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct fujitsu_hwb_ioc_bb_ctl bb_ctl;
+	struct bb_info *bb_info;
+
+	if (copy_from_user(&bb_ctl, (struct fujitsu_hwb_ioc_bb_ctl __user *)argp,
+						sizeof(struct fujitsu_hwb_ioc_bb_ctl)))
+		return -EFAULT;
+
+	bb_info = get_bb_info(pdata, bb_ctl.cmg, bb_ctl.bb);
+	if (IS_ERR(bb_info))
+		return PTR_ERR(bb_info);
+
+	/* Forbid free/assign/unassign operation from now on */
+	if (test_and_set_bit(BB_FREEING, &bb_info->flag)) {
+		pr_err("IOC_BB_FREE is already called. CMG: %u, BB: %u\n", bb_ctl.cmg, bb_ctl.bb);
+		put_bb_info(bb_info);
+		return -EPERM;
+	}
+
+	/* Wait current ongoing assign/unassign operation to finish */
+	if (wait_event_interruptible(bb_info->wq,
+					(atomic_read(&bb_info->ongoing_assign_count) == 0))) {
+		clear_bit(BB_FREEING, &bb_info->flag);
+		put_bb_info(bb_info);
+		pr_debug("IOC_BB_FREE is interrupted. CMG: %u, BB: %u\n", bb_ctl.cmg, bb_ctl.bb);
+		return -EINTR;
+	}
+
+	teardown_bb(bb_info);
+	spin_lock(&pdata->list_lock);
+	list_del_init(&bb_info->node);
+	spin_unlock(&pdata->list_lock);
+
+	/* 1 put for get_bb_info, 1 for alloc_bb_info */
+	put_bb_info(bb_info);
+	put_bb_info(bb_info);
+
+	return 0;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -636,6 +748,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BW_UNASSIGN:
 		ret = ioc_bw_unassign(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_BB_FREE:
+		ret = ioc_bb_free(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index 396029f2bc0d..7a285d8db0a9 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -28,5 +28,7 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	0x01, struct fujitsu_hwb_ioc_bw_ctl)
 #define FUJITSU_HWB_IOC_BW_UNASSIGN _IOW(__FUJITSU_IOCTL_MAGIC, \
 	0x02, struct fujitsu_hwb_ioc_bw_ctl)
+#define FUJITSU_HWB_IOC_BB_FREE _IOW(__FUJITSU_IOCTL_MAGIC, \
+	0x03, struct fujitsu_hwb_ioc_bb_ctl)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 07/10] soc: fujitsu: hwb: Add IOC_GET_PE_INFO ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

This is an infomative ioctl to tell users CMG/PE number of currently
running PE.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 18 ++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |  7 +++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 2535942cc0d7..1132cb74b13b 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -733,6 +733,21 @@ static int ioc_bb_free(struct file *filp,  void __user *argp)
 	return 0;
 }
 
+static int ioc_get_pe_info(struct file *filp, void __user *argp)
+{
+	struct fujitsu_hwb_ioc_pe_info pe_info = {0};
+	int cpu = smp_processor_id();
+
+	pe_info.cmg = _hwinfo.core_map[cpu].cmg;
+	pe_info.ppe = _hwinfo.core_map[cpu].ppe;
+
+	if (copy_to_user((struct fujitsu_hwb_ioc_pe_info __user *)argp, &pe_info,
+						sizeof(struct fujitsu_hwb_ioc_pe_info)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -751,6 +766,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BB_FREE:
 		ret = ioc_bb_free(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_GET_PE_INFO:
+		ret = ioc_get_pe_info(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index 7a285d8db0a9..1226014d97c4 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -22,6 +22,11 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	__s8 window;
 };
 
+struct fujitsu_hwb_ioc_pe_info {
+	__u8 cmg;
+	__u8 ppe;
+};
+
 #define FUJITSU_HWB_IOC_BB_ALLOC _IOWR(__FUJITSU_IOCTL_MAGIC, \
 	0x00, struct fujitsu_hwb_ioc_bb_ctl)
 #define FUJITSU_HWB_IOC_BW_ASSIGN _IOWR(__FUJITSU_IOCTL_MAGIC, \
@@ -30,5 +35,7 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	0x02, struct fujitsu_hwb_ioc_bw_ctl)
 #define FUJITSU_HWB_IOC_BB_FREE _IOW(__FUJITSU_IOCTL_MAGIC, \
 	0x03, struct fujitsu_hwb_ioc_bb_ctl)
+#define FUJITSU_HWB_IOC_GET_PE_INFO _IOR(__FUJITSU_IOCTL_MAGIC, \
+	0x04, struct fujitsu_hwb_ioc_pe_info)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 07/10] soc: fujitsu: hwb: Add IOC_GET_PE_INFO ioctl
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

This is an infomative ioctl to tell users CMG/PE number of currently
running PE.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c      | 18 ++++++++++++++++++
 include/uapi/linux/fujitsu_hpc_ioctl.h |  7 +++++++
 2 files changed, 25 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 2535942cc0d7..1132cb74b13b 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -733,6 +733,21 @@ static int ioc_bb_free(struct file *filp,  void __user *argp)
 	return 0;
 }
 
+static int ioc_get_pe_info(struct file *filp, void __user *argp)
+{
+	struct fujitsu_hwb_ioc_pe_info pe_info = {0};
+	int cpu = smp_processor_id();
+
+	pe_info.cmg = _hwinfo.core_map[cpu].cmg;
+	pe_info.ppe = _hwinfo.core_map[cpu].ppe;
+
+	if (copy_to_user((struct fujitsu_hwb_ioc_pe_info __user *)argp, &pe_info,
+						sizeof(struct fujitsu_hwb_ioc_pe_info)))
+		return -EFAULT;
+
+	return 0;
+}
+
 static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
 	void __user *argp = (void __user *)arg;
@@ -751,6 +766,9 @@ static long fujitsu_hwb_dev_ioctl(struct file *filp, unsigned int cmd, unsigned
 	case FUJITSU_HWB_IOC_BB_FREE:
 		ret = ioc_bb_free(filp, argp);
 		break;
+	case FUJITSU_HWB_IOC_GET_PE_INFO:
+		ret = ioc_get_pe_info(filp, argp);
+		break;
 	default:
 		ret = -ENOTTY;
 		break;
diff --git a/include/uapi/linux/fujitsu_hpc_ioctl.h b/include/uapi/linux/fujitsu_hpc_ioctl.h
index 7a285d8db0a9..1226014d97c4 100644
--- a/include/uapi/linux/fujitsu_hpc_ioctl.h
+++ b/include/uapi/linux/fujitsu_hpc_ioctl.h
@@ -22,6 +22,11 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	__s8 window;
 };
 
+struct fujitsu_hwb_ioc_pe_info {
+	__u8 cmg;
+	__u8 ppe;
+};
+
 #define FUJITSU_HWB_IOC_BB_ALLOC _IOWR(__FUJITSU_IOCTL_MAGIC, \
 	0x00, struct fujitsu_hwb_ioc_bb_ctl)
 #define FUJITSU_HWB_IOC_BW_ASSIGN _IOWR(__FUJITSU_IOCTL_MAGIC, \
@@ -30,5 +35,7 @@ struct fujitsu_hwb_ioc_bw_ctl {
 	0x02, struct fujitsu_hwb_ioc_bw_ctl)
 #define FUJITSU_HWB_IOC_BB_FREE _IOW(__FUJITSU_IOCTL_MAGIC, \
 	0x03, struct fujitsu_hwb_ioc_bb_ctl)
+#define FUJITSU_HWB_IOC_GET_PE_INFO _IOR(__FUJITSU_IOCTL_MAGIC, \
+	0x04, struct fujitsu_hwb_ioc_pe_info)
 
 #endif /* _UAPI_LINUX_FUJITSU_HPC_IOC_H */
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 08/10] soc: fujitsu: hwb: Add release operation
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

Upon release, we cleanup remaining resources/registers if necessary.
This happens when user does not call IOC_BB_FREE properly and the
function will do effectively the same operation as IOC_BB_FREE.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 1132cb74b13b..46f1f244f93a 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -796,9 +796,35 @@ static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+static int fujitsu_hwb_dev_release(struct inode *inode, struct file *filp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct bb_info *bb_info, *tmp;
+
+	/*
+	 * Cleanup BB if IOC_BB_FREE is not called properly.
+	 * No lock for pdata->bb_list is needed cause there is no one else
+	 */
+	if (!list_empty(&pdata->bb_list)) {
+		pr_warn("free operation is not called properly\n");
+
+		list_for_each_entry_safe(bb_info, tmp, &pdata->bb_list, node) {
+			teardown_bb(bb_info);
+			list_del_init(&bb_info->node);
+			/* 1 put for alloc_bb_info */
+			put_bb_info(bb_info);
+		}
+	}
+
+	kfree(pdata);
+
+	return 0;
+}
+
 static const struct file_operations fujitsu_hwb_dev_fops = {
 	.owner          = THIS_MODULE,
 	.open           = fujitsu_hwb_dev_open,
+	.release        = fujitsu_hwb_dev_release,
 	.unlocked_ioctl = fujitsu_hwb_dev_ioctl,
 };
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 08/10] soc: fujitsu: hwb: Add release operation
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

Upon release, we cleanup remaining resources/registers if necessary.
This happens when user does not call IOC_BB_FREE properly and the
function will do effectively the same operation as IOC_BB_FREE.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 1132cb74b13b..46f1f244f93a 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -796,9 +796,35 @@ static int fujitsu_hwb_dev_open(struct inode *inode, struct file *filp)
 	return 0;
 }
 
+static int fujitsu_hwb_dev_release(struct inode *inode, struct file *filp)
+{
+	struct hwb_private_data *pdata = (struct hwb_private_data *)filp->private_data;
+	struct bb_info *bb_info, *tmp;
+
+	/*
+	 * Cleanup BB if IOC_BB_FREE is not called properly.
+	 * No lock for pdata->bb_list is needed cause there is no one else
+	 */
+	if (!list_empty(&pdata->bb_list)) {
+		pr_warn("free operation is not called properly\n");
+
+		list_for_each_entry_safe(bb_info, tmp, &pdata->bb_list, node) {
+			teardown_bb(bb_info);
+			list_del_init(&bb_info->node);
+			/* 1 put for alloc_bb_info */
+			put_bb_info(bb_info);
+		}
+	}
+
+	kfree(pdata);
+
+	return 0;
+}
+
 static const struct file_operations fujitsu_hwb_dev_fops = {
 	.owner          = THIS_MODULE,
 	.open           = fujitsu_hwb_dev_open,
+	.release        = fujitsu_hwb_dev_release,
 	.unlocked_ioctl = fujitsu_hwb_dev_ioctl,
 };
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 09/10] soc: fujitsu: hwb: Add sysfs entry
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

This adds sysfs entry per CMG to show running barrier driver status
for debugging user application. The following entries will be created:

/sys/class/misc/fujitsu_hwb
 |- hwinfo ... number of CMG/BB/BW/pe_per_cmg on running system
 |- CMG0
     |- core_map      ... cpuid belonging to this CMG
     |- used_bb_bmap  ... bitmap of currently allocated BB
     |- used_bw_bmap  ... bitmap of currently allocated BW
     |- init_sync_bb0 ... current value of INIT_SYNC register 0
     |- init_sync_bb1 ... current value of INIT_SYNC register 1
     ...
 |- CMG1
  ...

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 258 ++++++++++++++++++++++++++++++
 1 file changed, 258 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 46f1f244f93a..a3a0e314f63a 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -32,6 +32,7 @@
 #include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/kernel.h>
+#include <linux/kobject.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
@@ -931,6 +932,254 @@ static int hwb_cpu_online(unsigned int cpu)
 	return 0;
 }
 
+static void read_init_sync_reg(void *args)
+{
+	struct init_sync_args *sync_args = (struct init_sync_args *)args;
+	u64 val = 0;
+
+	switch (sync_args->bb) {
+	case 0:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB0_EL1);
+		break;
+	case 1:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB1_EL1);
+		break;
+	case 2:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB2_EL1);
+		break;
+	case 3:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB3_EL1);
+		break;
+	case 4:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB4_EL1);
+		break;
+	case 5:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB5_EL1);
+		break;
+	}
+
+	sync_args->val = val;
+}
+
+struct hwb_attr {
+	struct kobj_attribute attr;
+	u8 bb;
+};
+static struct hwb_attr *battr;
+
+/* kobject for each CMG */
+static struct kobject **cmg_kobj;
+
+/* Get CMG number based on index value of cmg_kobj */
+static int get_cmg_from_kobj(struct kobject *kobj)
+{
+	int i;
+
+	for (i = 0; i < _hwinfo.num_cmg; i++) {
+		if (cmg_kobj[i] == kobj)
+			return i;
+	}
+	/* should not happen */
+	WARN_ON_ONCE("cmg_kobj not found\n");
+	return 0;
+}
+
+static ssize_t hwb_init_sync_bb_show(struct kobject *kobj,
+					 struct kobj_attribute *attr, char *buf)
+{
+	struct hwb_attr *battr = container_of(attr, struct hwb_attr, attr);
+	struct init_sync_args args = {0};
+	ssize_t written = 0;
+	int cpu;
+	int cmg;
+	u64 mask;
+	u64 bst;
+
+	/* Find online cpu in target cmg */
+	cmg = get_cmg_from_kobj(kobj);
+	for_each_online_cpu(cpu) {
+		if (_hwinfo.core_map[cpu].cmg == cmg)
+			break;
+	}
+	if (cpu >= nr_cpu_ids)
+		return 0;
+
+	/* Send IPI to read INIT_SYNC register */
+	args.bb = battr->bb;
+	on_each_cpu_mask(cpumask_of(cpu), read_init_sync_reg, &args, 1);
+
+	mask = FIELD_GET(FHWB_INIT_SYNC_BB_EL1_MASK_FIELD, args.val);
+	bst = FIELD_GET(FHWB_INIT_SYNC_BB_EL1_BST_FIELD, args.val);
+
+	written += scnprintf(buf, PAGE_SIZE, "%04llx\n", mask);
+	written += scnprintf(buf + written, PAGE_SIZE - written, "%04llx\n", bst);
+
+	return written;
+}
+
+#define BARRIER_ATTR(name) \
+static struct kobj_attribute hwb_##name##_attribute = \
+	__ATTR(name, 0444, hwb_##name##_show, NULL)
+
+static ssize_t hwb_hwinfo_show(struct kobject *kobj,
+				   struct kobj_attribute *attr, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d %d %d %d\n",
+				_hwinfo.num_cmg, _hwinfo.num_bb,
+				_hwinfo.num_bw, _hwinfo.max_pe_per_cmg);
+}
+BARRIER_ATTR(hwinfo);
+
+static ssize_t hwb_used_bb_bmap_show(struct kobject *kobj,
+					 struct kobj_attribute *attr, char *buf)
+{
+	int cmg;
+
+	cmg = get_cmg_from_kobj(kobj);
+
+	return scnprintf(buf, PAGE_SIZE, "%04lx\n", _hwinfo.used_bb_bmap[cmg]);
+}
+BARRIER_ATTR(used_bb_bmap);
+
+static ssize_t hwb_used_bw_bmap_show(struct kobject *kobj,
+					 struct kobj_attribute *attr, char *buf)
+{
+	ssize_t written = 0;
+	int cmg;
+	int cpu;
+
+	cmg = get_cmg_from_kobj(kobj);
+	for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
+		if (_hwinfo.core_map[cpu].cmg == cmg)
+			written += scnprintf(buf + written, PAGE_SIZE - written, "%d %04lx\n",
+						 cpu, _hwinfo.used_bw_bmap[cpu]);
+	}
+
+	return written;
+}
+BARRIER_ATTR(used_bw_bmap);
+
+static ssize_t hwb_core_map_show(struct kobject *kobj,
+				     struct kobj_attribute *attr, char *buf)
+{
+	ssize_t written = 0;
+	int cmg;
+	int cpu;
+
+	cmg = get_cmg_from_kobj(kobj);
+	for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
+		if (_hwinfo.core_map[cpu].cmg == cmg)
+			written += scnprintf(buf + written, PAGE_SIZE - written, "%d %d\n",
+				cpu, _hwinfo.core_map[cpu].ppe);
+	}
+
+	return written;
+}
+BARRIER_ATTR(core_map);
+
+static struct attribute *hwb_attrs[] = {
+	&hwb_used_bb_bmap_attribute.attr,
+	&hwb_used_bw_bmap_attribute.attr,
+	&hwb_core_map_attribute.attr,
+	NULL,
+};
+
+static const struct attribute_group hwb_attribute = {
+	.attrs = hwb_attrs,
+};
+
+static void destroy_sysfs(void)
+{
+	int cmg;
+	int bb;
+	int i;
+
+	sysfs_remove_file(&bar_miscdev.this_device->kobj, &hwb_hwinfo_attribute.attr);
+
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		for (bb = 0; bb < _hwinfo.num_bb; bb++) {
+			i = (cmg * _hwinfo.num_bb) + bb;
+			if (battr[i].attr.attr.name)
+				sysfs_remove_file(cmg_kobj[cmg], &battr[i].attr.attr);
+		}
+	}
+	kfree(battr);
+
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		if (cmg_kobj[cmg]) {
+			sysfs_remove_group(cmg_kobj[cmg], &hwb_attribute);
+			kobject_put(cmg_kobj[cmg]);
+		}
+	}
+	kfree(cmg_kobj);
+}
+
+/* Create sysfs file under /sys/class/misc/fujitsu_hwb */
+#define NAME_LEN 16
+static int __init init_sysfs(void)
+{
+	char name[NAME_LEN];
+	int ret;
+	int cmg;
+	int bb;
+	int i;
+
+	/* Create file to show number of CMG/BB/BW/pe_per_cmg */
+	ret = sysfs_create_file(&bar_miscdev.this_device->kobj, &hwb_hwinfo_attribute.attr);
+	if (ret)
+		return ret;
+
+	cmg_kobj = kcalloc(_hwinfo.num_cmg, sizeof(struct kobject *), GFP_KERNEL);
+	battr = kcalloc(_hwinfo.num_cmg * _hwinfo.num_bb, sizeof(struct hwb_attr), GFP_KERNEL);
+	if (!cmg_kobj || !battr) {
+		kfree(cmg_kobj);
+		kfree(battr);
+		return -ENOMEM;
+	}
+
+	/* Create folder for each CMG and create core_map/bitmap file */
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		scnprintf(name, NAME_LEN, "CMG%d", cmg);
+		cmg_kobj[cmg] = kobject_create_and_add(name, &bar_miscdev.this_device->kobj);
+		if (!cmg_kobj[cmg]) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+
+		ret = sysfs_create_group(cmg_kobj[cmg], &hwb_attribute);
+		if (ret)
+			goto fail;
+	}
+
+	/* Create files for INIT_SYNC register */
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		for (bb = 0; bb < _hwinfo.num_bb; bb++) {
+			i = (cmg * _hwinfo.num_bb) + bb;
+
+			scnprintf(name, NAME_LEN, "init_sync_bb%d", bb);
+			battr[i].bb = bb;
+			battr[i].attr.attr.name = kstrdup(name, GFP_KERNEL);
+			if (!battr[i].attr.attr.name) {
+				ret = -ENOMEM;
+				goto fail;
+			}
+			battr[i].attr.attr.mode = 0400; /* root only */
+			battr[i].attr.show = hwb_init_sync_bb_show;
+
+			sysfs_attr_init(&battr[i].attr.attr);
+			ret = sysfs_create_file(cmg_kobj[cmg], &battr[i].attr.attr);
+			if (ret < 0)
+				goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	destroy_sysfs();
+	return ret;
+}
+
 static int __init hwb_init(void)
 {
 	int ret;
@@ -967,8 +1216,16 @@ static int __init hwb_init(void)
 		goto out3;
 	}
 
+	ret = init_sysfs();
+	if (ret < 0) {
+		pr_err("sysfs creation failed: %d\n", ret);
+		goto out4;
+	}
+
 	return 0;
 
+out4:
+	misc_deregister(&bar_miscdev);
 out3:
 	cpuhp_remove_state(_hp_state);
 out2:
@@ -981,6 +1238,7 @@ static int __init hwb_init(void)
 
 static void __exit hwb_exit(void)
 {
+	destroy_sysfs();
 	misc_deregister(&bar_miscdev);
 	cpuhp_remove_state(_hp_state);
 	destroy_bb_info_cachep();
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 09/10] soc: fujitsu: hwb: Add sysfs entry
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

This adds sysfs entry per CMG to show running barrier driver status
for debugging user application. The following entries will be created:

/sys/class/misc/fujitsu_hwb
 |- hwinfo ... number of CMG/BB/BW/pe_per_cmg on running system
 |- CMG0
     |- core_map      ... cpuid belonging to this CMG
     |- used_bb_bmap  ... bitmap of currently allocated BB
     |- used_bw_bmap  ... bitmap of currently allocated BW
     |- init_sync_bb0 ... current value of INIT_SYNC register 0
     |- init_sync_bb1 ... current value of INIT_SYNC register 1
     ...
 |- CMG1
  ...

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 drivers/soc/fujitsu/fujitsu_hwb.c | 258 ++++++++++++++++++++++++++++++
 1 file changed, 258 insertions(+)

diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
index 46f1f244f93a..a3a0e314f63a 100644
--- a/drivers/soc/fujitsu/fujitsu_hwb.c
+++ b/drivers/soc/fujitsu/fujitsu_hwb.c
@@ -32,6 +32,7 @@
 #include <linux/cpu.h>
 #include <linux/cpumask.h>
 #include <linux/kernel.h>
+#include <linux/kobject.h>
 #include <linux/miscdevice.h>
 #include <linux/module.h>
 #include <linux/spinlock.h>
@@ -931,6 +932,254 @@ static int hwb_cpu_online(unsigned int cpu)
 	return 0;
 }
 
+static void read_init_sync_reg(void *args)
+{
+	struct init_sync_args *sync_args = (struct init_sync_args *)args;
+	u64 val = 0;
+
+	switch (sync_args->bb) {
+	case 0:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB0_EL1);
+		break;
+	case 1:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB1_EL1);
+		break;
+	case 2:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB2_EL1);
+		break;
+	case 3:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB3_EL1);
+		break;
+	case 4:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB4_EL1);
+		break;
+	case 5:
+		val = read_sysreg_s(FHWB_INIT_SYNC_BB5_EL1);
+		break;
+	}
+
+	sync_args->val = val;
+}
+
+struct hwb_attr {
+	struct kobj_attribute attr;
+	u8 bb;
+};
+static struct hwb_attr *battr;
+
+/* kobject for each CMG */
+static struct kobject **cmg_kobj;
+
+/* Get CMG number based on index value of cmg_kobj */
+static int get_cmg_from_kobj(struct kobject *kobj)
+{
+	int i;
+
+	for (i = 0; i < _hwinfo.num_cmg; i++) {
+		if (cmg_kobj[i] == kobj)
+			return i;
+	}
+	/* should not happen */
+	WARN_ON_ONCE("cmg_kobj not found\n");
+	return 0;
+}
+
+static ssize_t hwb_init_sync_bb_show(struct kobject *kobj,
+					 struct kobj_attribute *attr, char *buf)
+{
+	struct hwb_attr *battr = container_of(attr, struct hwb_attr, attr);
+	struct init_sync_args args = {0};
+	ssize_t written = 0;
+	int cpu;
+	int cmg;
+	u64 mask;
+	u64 bst;
+
+	/* Find online cpu in target cmg */
+	cmg = get_cmg_from_kobj(kobj);
+	for_each_online_cpu(cpu) {
+		if (_hwinfo.core_map[cpu].cmg == cmg)
+			break;
+	}
+	if (cpu >= nr_cpu_ids)
+		return 0;
+
+	/* Send IPI to read INIT_SYNC register */
+	args.bb = battr->bb;
+	on_each_cpu_mask(cpumask_of(cpu), read_init_sync_reg, &args, 1);
+
+	mask = FIELD_GET(FHWB_INIT_SYNC_BB_EL1_MASK_FIELD, args.val);
+	bst = FIELD_GET(FHWB_INIT_SYNC_BB_EL1_BST_FIELD, args.val);
+
+	written += scnprintf(buf, PAGE_SIZE, "%04llx\n", mask);
+	written += scnprintf(buf + written, PAGE_SIZE - written, "%04llx\n", bst);
+
+	return written;
+}
+
+#define BARRIER_ATTR(name) \
+static struct kobj_attribute hwb_##name##_attribute = \
+	__ATTR(name, 0444, hwb_##name##_show, NULL)
+
+static ssize_t hwb_hwinfo_show(struct kobject *kobj,
+				   struct kobj_attribute *attr, char *buf)
+{
+	return scnprintf(buf, PAGE_SIZE, "%d %d %d %d\n",
+				_hwinfo.num_cmg, _hwinfo.num_bb,
+				_hwinfo.num_bw, _hwinfo.max_pe_per_cmg);
+}
+BARRIER_ATTR(hwinfo);
+
+static ssize_t hwb_used_bb_bmap_show(struct kobject *kobj,
+					 struct kobj_attribute *attr, char *buf)
+{
+	int cmg;
+
+	cmg = get_cmg_from_kobj(kobj);
+
+	return scnprintf(buf, PAGE_SIZE, "%04lx\n", _hwinfo.used_bb_bmap[cmg]);
+}
+BARRIER_ATTR(used_bb_bmap);
+
+static ssize_t hwb_used_bw_bmap_show(struct kobject *kobj,
+					 struct kobj_attribute *attr, char *buf)
+{
+	ssize_t written = 0;
+	int cmg;
+	int cpu;
+
+	cmg = get_cmg_from_kobj(kobj);
+	for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
+		if (_hwinfo.core_map[cpu].cmg == cmg)
+			written += scnprintf(buf + written, PAGE_SIZE - written, "%d %04lx\n",
+						 cpu, _hwinfo.used_bw_bmap[cpu]);
+	}
+
+	return written;
+}
+BARRIER_ATTR(used_bw_bmap);
+
+static ssize_t hwb_core_map_show(struct kobject *kobj,
+				     struct kobj_attribute *attr, char *buf)
+{
+	ssize_t written = 0;
+	int cmg;
+	int cpu;
+
+	cmg = get_cmg_from_kobj(kobj);
+	for (cpu = 0; cpu < num_possible_cpus(); cpu++) {
+		if (_hwinfo.core_map[cpu].cmg == cmg)
+			written += scnprintf(buf + written, PAGE_SIZE - written, "%d %d\n",
+				cpu, _hwinfo.core_map[cpu].ppe);
+	}
+
+	return written;
+}
+BARRIER_ATTR(core_map);
+
+static struct attribute *hwb_attrs[] = {
+	&hwb_used_bb_bmap_attribute.attr,
+	&hwb_used_bw_bmap_attribute.attr,
+	&hwb_core_map_attribute.attr,
+	NULL,
+};
+
+static const struct attribute_group hwb_attribute = {
+	.attrs = hwb_attrs,
+};
+
+static void destroy_sysfs(void)
+{
+	int cmg;
+	int bb;
+	int i;
+
+	sysfs_remove_file(&bar_miscdev.this_device->kobj, &hwb_hwinfo_attribute.attr);
+
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		for (bb = 0; bb < _hwinfo.num_bb; bb++) {
+			i = (cmg * _hwinfo.num_bb) + bb;
+			if (battr[i].attr.attr.name)
+				sysfs_remove_file(cmg_kobj[cmg], &battr[i].attr.attr);
+		}
+	}
+	kfree(battr);
+
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		if (cmg_kobj[cmg]) {
+			sysfs_remove_group(cmg_kobj[cmg], &hwb_attribute);
+			kobject_put(cmg_kobj[cmg]);
+		}
+	}
+	kfree(cmg_kobj);
+}
+
+/* Create sysfs file under /sys/class/misc/fujitsu_hwb */
+#define NAME_LEN 16
+static int __init init_sysfs(void)
+{
+	char name[NAME_LEN];
+	int ret;
+	int cmg;
+	int bb;
+	int i;
+
+	/* Create file to show number of CMG/BB/BW/pe_per_cmg */
+	ret = sysfs_create_file(&bar_miscdev.this_device->kobj, &hwb_hwinfo_attribute.attr);
+	if (ret)
+		return ret;
+
+	cmg_kobj = kcalloc(_hwinfo.num_cmg, sizeof(struct kobject *), GFP_KERNEL);
+	battr = kcalloc(_hwinfo.num_cmg * _hwinfo.num_bb, sizeof(struct hwb_attr), GFP_KERNEL);
+	if (!cmg_kobj || !battr) {
+		kfree(cmg_kobj);
+		kfree(battr);
+		return -ENOMEM;
+	}
+
+	/* Create folder for each CMG and create core_map/bitmap file */
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		scnprintf(name, NAME_LEN, "CMG%d", cmg);
+		cmg_kobj[cmg] = kobject_create_and_add(name, &bar_miscdev.this_device->kobj);
+		if (!cmg_kobj[cmg]) {
+			ret = -ENOMEM;
+			goto fail;
+		}
+
+		ret = sysfs_create_group(cmg_kobj[cmg], &hwb_attribute);
+		if (ret)
+			goto fail;
+	}
+
+	/* Create files for INIT_SYNC register */
+	for (cmg = 0; cmg < _hwinfo.num_cmg; cmg++) {
+		for (bb = 0; bb < _hwinfo.num_bb; bb++) {
+			i = (cmg * _hwinfo.num_bb) + bb;
+
+			scnprintf(name, NAME_LEN, "init_sync_bb%d", bb);
+			battr[i].bb = bb;
+			battr[i].attr.attr.name = kstrdup(name, GFP_KERNEL);
+			if (!battr[i].attr.attr.name) {
+				ret = -ENOMEM;
+				goto fail;
+			}
+			battr[i].attr.attr.mode = 0400; /* root only */
+			battr[i].attr.show = hwb_init_sync_bb_show;
+
+			sysfs_attr_init(&battr[i].attr.attr);
+			ret = sysfs_create_file(cmg_kobj[cmg], &battr[i].attr.attr);
+			if (ret < 0)
+				goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	destroy_sysfs();
+	return ret;
+}
+
 static int __init hwb_init(void)
 {
 	int ret;
@@ -967,8 +1216,16 @@ static int __init hwb_init(void)
 		goto out3;
 	}
 
+	ret = init_sysfs();
+	if (ret < 0) {
+		pr_err("sysfs creation failed: %d\n", ret);
+		goto out4;
+	}
+
 	return 0;
 
+out4:
+	misc_deregister(&bar_miscdev);
 out3:
 	cpuhp_remove_state(_hp_state);
 out2:
@@ -981,6 +1238,7 @@ static int __init hwb_init(void)
 
 static void __exit hwb_exit(void)
 {
+	destroy_sysfs();
 	misc_deregister(&bar_miscdev);
 	cpuhp_remove_state(_hp_state);
 	destroy_bb_info_cachep();
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 10/10] soc: fujitsu: hwb: Add Kconfig/Makefile to build fujitsu_hwb driver
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: will, catalin.marinas, arnd, olof, misono.tomohiro

This adds kconfig/Makefile to build fujitsu hardware barrier driver
(fujitsu_hwb.ko when built as module).

Note that this is the first time to add A64FX specific driver,
this also adds A64FX entry in Kconfig.platforms of arm64 Kconfig.
Also add MAINTAINERS entry for ARM/A64FX accordingly.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 MAINTAINERS                  |  7 +++++++
 arch/arm64/Kconfig.platforms |  5 +++++
 drivers/soc/Kconfig          |  1 +
 drivers/soc/Makefile         |  1 +
 drivers/soc/fujitsu/Kconfig  | 24 ++++++++++++++++++++++++
 drivers/soc/fujitsu/Makefile |  2 ++
 6 files changed, 40 insertions(+)
 create mode 100644 drivers/soc/fujitsu/Kconfig
 create mode 100644 drivers/soc/fujitsu/Makefile

diff --git a/MAINTAINERS b/MAINTAINERS
index 6eff4f720c72..d57ec44ceaed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1508,6 +1508,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
 F:	arch/arm/mach-*/
 F:	arch/arm/plat-*/
 
+ARM/A64FX SOC SUPPORT
+M:	Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/soc/fujitsu/
+F:	include/uapi/linux/fujitsu_hpc_ioctl.h
+
 ARM/ACTIONS SEMI ARCHITECTURE
 M:	Andreas Färber <afaerber@suse.de>
 M:	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 6eecdef538bd..41fb214adaff 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -1,6 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only
 menu "Platform selection"
 
+config ARCH_A64FX
+	bool "Fujitsu A64FX Platforms"
+	help
+	  This enables support for Fujitsu A64FX SoC family.
+
 config ARCH_ACTIONS
 	bool "Actions Semi Platforms"
 	select OWL_TIMER
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index d097d070f579..7a52b5dc4c96 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -7,6 +7,7 @@ source "drivers/soc/aspeed/Kconfig"
 source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/fsl/Kconfig"
+source "drivers/soc/fujitsu/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
 source "drivers/soc/litex/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 699b758d28e4..57c0dddc4d23 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -10,6 +10,7 @@ obj-y				+= bcm/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-y				+= fujitsu/
 obj-$(CONFIG_ARCH_GEMINI)	+= gemini/
 obj-y				+= imx/
 obj-$(CONFIG_ARCH_IXP4XX)	+= ixp4xx/
diff --git a/drivers/soc/fujitsu/Kconfig b/drivers/soc/fujitsu/Kconfig
new file mode 100644
index 000000000000..cbba0c939e62
--- /dev/null
+++ b/drivers/soc/fujitsu/Kconfig
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# FUJITSU SoC drivers
+#
+menuconfig SOC_FUJITSU
+	bool "FUJITSU SoC drivers"
+	depends on ARCH_A64FX || COMPILE_TEST
+
+if SOC_FUJITSU
+
+config FUJITSU_HARDWARE_BARRIER
+	tristate "FUJITSU HPC Hardware Barrier Driver"
+	depends on ARM64_VHE || COMPILE_TEST
+	help
+	  FUJITSU HPC Hardware Barrier Driver
+
+	  This driver offers hardware barrier functions for A64FX system
+	  which realizes synchronization by PEs in the same CMG (L3 cache
+	  domain) by using implementation defined registers. As control
+	  registers can only be accessed from EL2 on reset, this driver
+	  needs support of VHE.
+	  When built as a module, this will be called as "fujitsu_hwb".
+
+endif # SOC_FUJITSU
diff --git a/drivers/soc/fujitsu/Makefile b/drivers/soc/fujitsu/Makefile
new file mode 100644
index 000000000000..1b8e4c947f7f
--- /dev/null
+++ b/drivers/soc/fujitsu/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_FUJITSU_HARDWARE_BARRIER) +=	fujitsu_hwb.o
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 32+ messages in thread

* [PATCH 10/10] soc: fujitsu: hwb: Add Kconfig/Makefile to build fujitsu_hwb driver
@ 2021-01-08 10:32   ` Misono Tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: Misono Tomohiro @ 2021-01-08 10:32 UTC (permalink / raw)
  To: linux-arm-kernel, soc; +Cc: olof, catalin.marinas, will, misono.tomohiro, arnd

This adds kconfig/Makefile to build fujitsu hardware barrier driver
(fujitsu_hwb.ko when built as module).

Note that this is the first time to add A64FX specific driver,
this also adds A64FX entry in Kconfig.platforms of arm64 Kconfig.
Also add MAINTAINERS entry for ARM/A64FX accordingly.

Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
---
 MAINTAINERS                  |  7 +++++++
 arch/arm64/Kconfig.platforms |  5 +++++
 drivers/soc/Kconfig          |  1 +
 drivers/soc/Makefile         |  1 +
 drivers/soc/fujitsu/Kconfig  | 24 ++++++++++++++++++++++++
 drivers/soc/fujitsu/Makefile |  2 ++
 6 files changed, 40 insertions(+)
 create mode 100644 drivers/soc/fujitsu/Kconfig
 create mode 100644 drivers/soc/fujitsu/Makefile

diff --git a/MAINTAINERS b/MAINTAINERS
index 6eff4f720c72..d57ec44ceaed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1508,6 +1508,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git
 F:	arch/arm/mach-*/
 F:	arch/arm/plat-*/
 
+ARM/A64FX SOC SUPPORT
+M:	Tomohiro Misono <misono.tomohiro@jp.fujitsu.com>
+L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
+S:	Maintained
+F:	drivers/soc/fujitsu/
+F:	include/uapi/linux/fujitsu_hpc_ioctl.h
+
 ARM/ACTIONS SEMI ARCHITECTURE
 M:	Andreas Färber <afaerber@suse.de>
 M:	Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
index 6eecdef538bd..41fb214adaff 100644
--- a/arch/arm64/Kconfig.platforms
+++ b/arch/arm64/Kconfig.platforms
@@ -1,6 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0-only
 menu "Platform selection"
 
+config ARCH_A64FX
+	bool "Fujitsu A64FX Platforms"
+	help
+	  This enables support for Fujitsu A64FX SoC family.
+
 config ARCH_ACTIONS
 	bool "Actions Semi Platforms"
 	select OWL_TIMER
diff --git a/drivers/soc/Kconfig b/drivers/soc/Kconfig
index d097d070f579..7a52b5dc4c96 100644
--- a/drivers/soc/Kconfig
+++ b/drivers/soc/Kconfig
@@ -7,6 +7,7 @@ source "drivers/soc/aspeed/Kconfig"
 source "drivers/soc/atmel/Kconfig"
 source "drivers/soc/bcm/Kconfig"
 source "drivers/soc/fsl/Kconfig"
+source "drivers/soc/fujitsu/Kconfig"
 source "drivers/soc/imx/Kconfig"
 source "drivers/soc/ixp4xx/Kconfig"
 source "drivers/soc/litex/Kconfig"
diff --git a/drivers/soc/Makefile b/drivers/soc/Makefile
index 699b758d28e4..57c0dddc4d23 100644
--- a/drivers/soc/Makefile
+++ b/drivers/soc/Makefile
@@ -10,6 +10,7 @@ obj-y				+= bcm/
 obj-$(CONFIG_ARCH_DOVE)		+= dove/
 obj-$(CONFIG_MACH_DOVE)		+= dove/
 obj-y				+= fsl/
+obj-y				+= fujitsu/
 obj-$(CONFIG_ARCH_GEMINI)	+= gemini/
 obj-y				+= imx/
 obj-$(CONFIG_ARCH_IXP4XX)	+= ixp4xx/
diff --git a/drivers/soc/fujitsu/Kconfig b/drivers/soc/fujitsu/Kconfig
new file mode 100644
index 000000000000..cbba0c939e62
--- /dev/null
+++ b/drivers/soc/fujitsu/Kconfig
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# FUJITSU SoC drivers
+#
+menuconfig SOC_FUJITSU
+	bool "FUJITSU SoC drivers"
+	depends on ARCH_A64FX || COMPILE_TEST
+
+if SOC_FUJITSU
+
+config FUJITSU_HARDWARE_BARRIER
+	tristate "FUJITSU HPC Hardware Barrier Driver"
+	depends on ARM64_VHE || COMPILE_TEST
+	help
+	  FUJITSU HPC Hardware Barrier Driver
+
+	  This driver offers hardware barrier functions for A64FX system
+	  which realizes synchronization by PEs in the same CMG (L3 cache
+	  domain) by using implementation defined registers. As control
+	  registers can only be accessed from EL2 on reset, this driver
+	  needs support of VHE.
+	  When built as a module, this will be called as "fujitsu_hwb".
+
+endif # SOC_FUJITSU
diff --git a/drivers/soc/fujitsu/Makefile b/drivers/soc/fujitsu/Makefile
new file mode 100644
index 000000000000..1b8e4c947f7f
--- /dev/null
+++ b/drivers/soc/fujitsu/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_FUJITSU_HARDWARE_BARRIER) +=	fujitsu_hwb.o
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 32+ messages in thread

* Re: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-08 11:58     ` Jonathan Cameron
  0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2021-01-08 11:58 UTC (permalink / raw)
  To: Misono Tomohiro; +Cc: linux-arm-kernel, soc, olof, catalin.marinas, will, arnd

On Fri, 8 Jan 2021 19:32:18 +0900
Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> wrote:

> This adds hardware barrier driver's struct definitions and
> module init/exit code. We use miscdeice for barrier driver ioctl

device

> and /dev/fujitsu_hwb will be created upon module load.
> Following commits will add each ioctl definition.
> 
> Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Hi Misono,

I was taking a look out of curiosity so the following is definitely not
a full review but a few general comments inline.

Thanks,

Jonathan

> ---
>  drivers/soc/fujitsu/fujitsu_hwb.c | 313 ++++++++++++++++++++++++++++++
>  1 file changed, 313 insertions(+)
>  create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c
> 
> diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
> new file mode 100644
> index 000000000000..44c32c1683df
> --- /dev/null
> +++ b/drivers/soc/fujitsu/fujitsu_hwb.c
> @@ -0,0 +1,313 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 FUJITSU LIMITED
> + *
> + * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
> + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> + * On A64FX, CMG is the same as L3 cache domain.
> + *
> + * The main purpose of the driver is setting up registers which cannot be accessed
> + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
> + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> + *
> + * Simplified barrier operation flow of user application is as follows:
> + *  (one PE)
> + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> + *       This specifies which PEs join synchronization
> + *  (on each PE joining synchronization)
> + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> + *    3. Barrier main logic (all logic runs in EL0)
> + *      a) Write 1 to BST_SYNC register
> + *      b) Read LBSY_SYNC register
> + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> + *  (one PE)
> + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> + */
> +
> +#include <asm/cputype.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +
> +#ifdef pr_fmt
> +#undef pr_fmt
> +#endif
> +#define pr_fmt(fmt) "[%s:%s:%d] " fmt, KBUILD_MODNAME, __func__, __LINE__
> +
> +/* Since miscdevice is used, /dev/fujitsu_hwb will be created when module is loaded */
> +#define FHWB_DEV_NAME "fujitsu_hwb"
> +
> +/* Implementation defined registers for barrier shared in CMG */
> +#define FHWB_INIT_SYNC_BB0_EL1  sys_reg(3, 0, 15, 13, 0)
> +#define FHWB_INIT_SYNC_BB1_EL1  sys_reg(3, 0, 15, 13, 1)
> +#define FHWB_INIT_SYNC_BB2_EL1  sys_reg(3, 0, 15, 13, 2)
> +#define FHWB_INIT_SYNC_BB3_EL1  sys_reg(3, 0, 15, 13, 3)
> +#define FHWB_INIT_SYNC_BB4_EL1  sys_reg(3, 0, 15, 13, 4)
> +#define FHWB_INIT_SYNC_BB5_EL1  sys_reg(3, 0, 15, 13, 5)
> +
> +/* Implementation defined registers for barrier per PE */
> +#define FHWB_CTRL_EL1           sys_reg(3, 0, 11, 12, 0)
> +#define FHWB_BST_BIT_EL1        sys_reg(3, 0, 11, 12, 4)
> +#define FHWB_ASSIGN_SYNC_W0_EL1 sys_reg(3, 0, 15, 15, 0)
> +#define FHWB_ASSIGN_SYNC_W1_EL1 sys_reg(3, 0, 15, 15, 1)
> +#define FHWB_ASSIGN_SYNC_W2_EL1 sys_reg(3, 0, 15, 15, 2)
> +#define FHWB_ASSIGN_SYNC_W3_EL1 sys_reg(3, 0, 15, 15, 3)
> +
> +/* Field definitions for above registers */
> +#define FHWB_INIT_SYNC_BB_EL1_MASK_FIELD  GENMASK_ULL(44, 32)
> +#define FHWB_INIT_SYNC_BB_EL1_BST_FIELD   GENMASK_ULL(12, 0)
> +#define FHWB_CTRL_EL1_EL1AE               BIT_ULL(63)
> +#define FHWB_CTRL_EL1_EL0AE               BIT_ULL(62)
> +#define FHWB_BST_BIT_EL1_CMG_FILED        GENMASK_ULL(5, 4)
> +#define FHWB_BST_BIT_EL1_PE_FILED         GENMASK_ULL(3, 0)
> +#define FHWB_ASSIGN_SYNC_W_EL1_VALID      BIT_ULL(63)
> +
> +static enum cpuhp_state _hp_state;
> +
> +/*
> + * Each PE has its own CMG and Physical PE number (determined by BST_BIT_EL1 register).
> + * Barrier operation can be performed by PEs which belong to the same CMG.
> + */
> +struct pe_info {
> +	/* CMG number of this PE */
> +	u8 cmg;
> +	/* Physical PE number of this PE */
> +	u8 ppe;
> +};
> +
> +/* Hardware information of running system */
> +struct hwb_hwinfo {
> +	/* CPU type (part number) */
> +	unsigned int type;
> +	/* Number of CMG */
> +	u8 num_cmg;
> +	/* Number of barrier blade(BB) per CMG */
> +	u8 num_bb;
> +	/* Number of barrier window(BW) per PE */
> +	u8 num_bw;
> +	/*
> +	 * Maximum number of PE per CMG.
> +	 * Depending on BIOS configuration, each CMG has up to max_pe_per_cmg PEs
> +	 * and each PE has unique physical PE number between 0 ~ (max_pe_per_cmg-1)
> +	 */
> +	u8 max_pe_per_cmg;
> +
> +	/* Bitmap for currently allocated BB per CMG */
> +	unsigned long *used_bb_bmap;
> +	/* Bitmap for currently allocated BW per PE */
> +	unsigned long *used_bw_bmap;
> +	/* Mapping table of cpuid -> CMG/PE number */
> +	struct pe_info *core_map;
> +};
> +static struct hwb_hwinfo _hwinfo;
> +
> +/* List for barrier blade currently used per FD */
> +struct hwb_private_data {
> +	struct list_head bb_list;
> +	spinlock_t list_lock;
> +};
> +
> +/* Each barrier blade info */
> +#define BB_FREEING 1
> +struct bb_info {
> +	/* cpumask for PEs which participate synchronization */
> +	cpumask_var_t pemask;
> +	/* cpumask for PEs which currently assigned BW for this BB */
> +	cpumask_var_t assigned_pemask;
> +	/* Added to hwb_private_data::bb_list */
> +	struct list_head node;
> +	/* For indicating if this bb is currently being freed or not */
> +	unsigned long flag;
> +	/* For waiting ongoing assign/unassign operation to finish before freeing BB */
> +	wait_queue_head_t wq;
> +	/* Track ongoing assign/unassign operation count */
> +	atomic_t ongoing_assign_count;
> +	/* CMG  number of this blade */

nitpick: Double space currently after CMG that looks inconsistent.

> +	u8 cmg;
> +	/* BB number of this blade */
> +	u8 bb;
> +	/* Hold assigned window number of each PE corresponding to @assigned_pemask */
> +	u8 *bw;
> +	/* Track usage count as IOC_BB_FREE and IOC_BW_[UN]ASSIGN might be run in parallel */
> +	struct kref kref;
> +};
> +static struct kmem_cache *bb_info_cachep;
> +
> +static const struct file_operations fujitsu_hwb_dev_fops = {
> +	.owner          = THIS_MODULE,
> +};
> +
> +static struct miscdevice bar_miscdev = {
> +	.fops  = &fujitsu_hwb_dev_fops,
> +	.minor = MISC_DYNAMIC_MINOR,
> +	.mode  = 0666,
> +	.name  = FHWB_DEV_NAME,
> +};
> +
> +static void destroy_bb_info_cachep(void)
> +{
> +	kmem_cache_destroy(bb_info_cachep);
> +}
> +
> +static int __init init_bb_info_cachep(void)
> +{
> +	/*
> +	 * Since cpumask value will be copied from userspace to the beginning of
> +	 * struct bb_info, use kmem_cache_create_usercopy to mark that region.
> +	 * Otherwise CONFIG_HARDENED_USERCOPY gives user_copy_warn.
> +	 */
> +	bb_info_cachep = kmem_cache_create_usercopy("bb_info_cache", sizeof(struct bb_info),
> +			0, SLAB_HWCACHE_ALIGN, 0, sizeof(cpumask_var_t), NULL);
> +	if (bb_info_cachep == NULL)

inconsistent on ! vs == NULL checks.  I personally don't care which you use, but better to chose
one style and use if everywhere in a given driver.

> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void free_map(void)
> +{
> +	kfree(_hwinfo.used_bw_bmap);
> +	kfree(_hwinfo.used_bb_bmap);
> +	kfree(_hwinfo.core_map);
> +}
> +
> +static int __init alloc_map(void)

For generic sounding function names, it is better to prefix with something driver specific.
perhaps hwb_alloc_map() or similar?  Both avoids possible clashes of naming in future and
leads to more readable code as people then know the function is local.


> +{
> +	_hwinfo.core_map = kcalloc(num_possible_cpus(), sizeof(struct pe_info), GFP_KERNEL);

preferred to use sizeof(*_hwinfo.core_map) as saves reviewer checking the types.  Note
applies in other places as well.
Whilst it's nice to make these flexible in size, the separate allocations do add overheads.
Given num_possible_cpus is probably constrained, perhaps better to just make these fixed
size and big enough for all plausible usecases?


> +	_hwinfo.used_bb_bmap = kcalloc(_hwinfo.num_cmg, sizeof(unsigned long), GFP_KERNEL);
> +	_hwinfo.used_bw_bmap = kcalloc(num_possible_cpus(), sizeof(unsigned long), GFP_KERNEL);
> +	if (!_hwinfo.core_map || !_hwinfo.used_bb_bmap || !_hwinfo.used_bw_bmap)
> +		goto fail;

I'd prefer you check these individually and handle the frees explicitly.  Makes for easier reviewing
as we can match each fail against the clean up.

> +
> +	/* 0 is valid number for both CMG/PE. Set all bits to 1 to represents uninitialized state */
> +	memset(_hwinfo.core_map, 0xFF, sizeof(struct pe_info) * num_possible_cpus());
> +
> +	return 0;
> +
> +fail:
> +	free_map();
> +	return -ENOMEM;
> +}
> +
> +/* Get this system's CPU type (part number). If it is not fujitsu CPU, return -1 */
> +static int __init get_cpu_type(void)
> +{
> +	if (read_cpuid_implementor() != ARM_CPU_IMP_FUJITSU)
> +		return -1;

Better to return a meaningful error code from here, then pass it on at the caller.

> +
> +	return read_cpuid_part_number();
> +}
> +
> +static int __init setup_hwinfo(void)
> +{
> +	int type;
> +
> +	type = get_cpu_type();
> +	if (type < 0)

As above, I'd expect to see return type; here.

> +		return -ENODEV;
> +
> +	_hwinfo.type = type;
> +	switch (type) {
> +	case FUJITSU_CPU_PART_A64FX:
> +		_hwinfo.num_cmg = 4;
> +		_hwinfo.num_bb = 6;
> +		_hwinfo.num_bw = 4;
> +		_hwinfo.max_pe_per_cmg = 13;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hwb_cpu_online(unsigned int cpu)
> +{
> +	u64 val;
> +	int i;
> +
> +	/* Setup core_map by reading BST_BIT_EL1 register of each PE */
> +	val = read_sysreg_s(FHWB_BST_BIT_EL1);
> +	_hwinfo.core_map[cpu].cmg = FIELD_GET(FHWB_BST_BIT_EL1_CMG_FILED, val);
> +	_hwinfo.core_map[cpu].ppe = FIELD_GET(FHWB_BST_BIT_EL1_PE_FILED, val);
> +
> +	/* Since these registers' values are UNKNOWN on reset, explicitly clear all */
> +	for (i = 0; i < _hwinfo.num_bw; i++)
> +		write_bw_reg(i, 0);
> +
> +	write_sysreg_s(0, FHWB_CTRL_EL1);
> +
> +	return 0;
> +}
> +
> +static int __init hwb_init(void)
> +{
> +	int ret;
> +
> +	ret = setup_hwinfo();
> +	if (ret < 0) {

As it's not obvious from the function name that the only thing it is doing is
checking the cpu type, I'd move this error print into that function call or
rename setup_hwinfo()


> +		pr_err("Unsupported CPU type\n");
> +		return ret;
> +	}
> +
> +	ret = alloc_map();
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = init_bb_info_cachep();
> +	if (ret < 0)
> +		goto out1;
> +
> +	/*
> +	 * Setup cpuhp callback to ensure each PE's resource will be initialized
> +	 * even if some PEs are offline at this point
> +	 */
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/fujitsu_hwb:online",
> +		hwb_cpu_online, NULL);
> +	if (ret < 0) {
> +		pr_err("cpuhp setup failed: %d\n", ret);
> +		goto out2;
> +	}
> +	_hp_state = ret;
> +
> +	ret = misc_register(&bar_miscdev);
> +	if (ret < 0) {
> +		pr_err("misc_register failed: %d\n", ret);
> +		goto out3;
> +	}
> +
> +	return 0;
> +
> +out3:
> +	cpuhp_remove_state(_hp_state);
> +out2:
> +	destroy_bb_info_cachep();
> +out1:
> +	free_map();
> +
> +	return ret;
> +}
> +
> +static void __exit hwb_exit(void)
> +{
> +	misc_deregister(&bar_miscdev);
> +	cpuhp_remove_state(_hp_state);
> +	destroy_bb_info_cachep();
> +	free_map();
> +}
> +
> +module_init(hwb_init);
> +module_exit(hwb_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("FUJITSU LIMITED");
> +MODULE_DESCRIPTION("FUJITSU HPC Hardware Barrier Driver");


^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-08 11:58     ` Jonathan Cameron
  0 siblings, 0 replies; 32+ messages in thread
From: Jonathan Cameron @ 2021-01-08 11:58 UTC (permalink / raw)
  To: Misono Tomohiro; +Cc: arnd, catalin.marinas, soc, olof, will, linux-arm-kernel

On Fri, 8 Jan 2021 19:32:18 +0900
Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> wrote:

> This adds hardware barrier driver's struct definitions and
> module init/exit code. We use miscdeice for barrier driver ioctl

device

> and /dev/fujitsu_hwb will be created upon module load.
> Following commits will add each ioctl definition.
> 
> Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>

Hi Misono,

I was taking a look out of curiosity so the following is definitely not
a full review but a few general comments inline.

Thanks,

Jonathan

> ---
>  drivers/soc/fujitsu/fujitsu_hwb.c | 313 ++++++++++++++++++++++++++++++
>  1 file changed, 313 insertions(+)
>  create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c
> 
> diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c b/drivers/soc/fujitsu/fujitsu_hwb.c
> new file mode 100644
> index 000000000000..44c32c1683df
> --- /dev/null
> +++ b/drivers/soc/fujitsu/fujitsu_hwb.c
> @@ -0,0 +1,313 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright 2020 FUJITSU LIMITED
> + *
> + * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
> + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> + * On A64FX, CMG is the same as L3 cache domain.
> + *
> + * The main purpose of the driver is setting up registers which cannot be accessed
> + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
> + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> + *
> + * Simplified barrier operation flow of user application is as follows:
> + *  (one PE)
> + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> + *       This specifies which PEs join synchronization
> + *  (on each PE joining synchronization)
> + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> + *    3. Barrier main logic (all logic runs in EL0)
> + *      a) Write 1 to BST_SYNC register
> + *      b) Read LBSY_SYNC register
> + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> + *  (one PE)
> + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> + */
> +
> +#include <asm/cputype.h>
> +#include <linux/bitfield.h>
> +#include <linux/bitops.h>
> +#include <linux/cpu.h>
> +#include <linux/cpumask.h>
> +#include <linux/kernel.h>
> +#include <linux/miscdevice.h>
> +#include <linux/module.h>
> +#include <linux/spinlock.h>
> +#include <linux/slab.h>
> +#include <linux/wait.h>
> +
> +#ifdef pr_fmt
> +#undef pr_fmt
> +#endif
> +#define pr_fmt(fmt) "[%s:%s:%d] " fmt, KBUILD_MODNAME, __func__, __LINE__
> +
> +/* Since miscdevice is used, /dev/fujitsu_hwb will be created when module is loaded */
> +#define FHWB_DEV_NAME "fujitsu_hwb"
> +
> +/* Implementation defined registers for barrier shared in CMG */
> +#define FHWB_INIT_SYNC_BB0_EL1  sys_reg(3, 0, 15, 13, 0)
> +#define FHWB_INIT_SYNC_BB1_EL1  sys_reg(3, 0, 15, 13, 1)
> +#define FHWB_INIT_SYNC_BB2_EL1  sys_reg(3, 0, 15, 13, 2)
> +#define FHWB_INIT_SYNC_BB3_EL1  sys_reg(3, 0, 15, 13, 3)
> +#define FHWB_INIT_SYNC_BB4_EL1  sys_reg(3, 0, 15, 13, 4)
> +#define FHWB_INIT_SYNC_BB5_EL1  sys_reg(3, 0, 15, 13, 5)
> +
> +/* Implementation defined registers for barrier per PE */
> +#define FHWB_CTRL_EL1           sys_reg(3, 0, 11, 12, 0)
> +#define FHWB_BST_BIT_EL1        sys_reg(3, 0, 11, 12, 4)
> +#define FHWB_ASSIGN_SYNC_W0_EL1 sys_reg(3, 0, 15, 15, 0)
> +#define FHWB_ASSIGN_SYNC_W1_EL1 sys_reg(3, 0, 15, 15, 1)
> +#define FHWB_ASSIGN_SYNC_W2_EL1 sys_reg(3, 0, 15, 15, 2)
> +#define FHWB_ASSIGN_SYNC_W3_EL1 sys_reg(3, 0, 15, 15, 3)
> +
> +/* Field definitions for above registers */
> +#define FHWB_INIT_SYNC_BB_EL1_MASK_FIELD  GENMASK_ULL(44, 32)
> +#define FHWB_INIT_SYNC_BB_EL1_BST_FIELD   GENMASK_ULL(12, 0)
> +#define FHWB_CTRL_EL1_EL1AE               BIT_ULL(63)
> +#define FHWB_CTRL_EL1_EL0AE               BIT_ULL(62)
> +#define FHWB_BST_BIT_EL1_CMG_FILED        GENMASK_ULL(5, 4)
> +#define FHWB_BST_BIT_EL1_PE_FILED         GENMASK_ULL(3, 0)
> +#define FHWB_ASSIGN_SYNC_W_EL1_VALID      BIT_ULL(63)
> +
> +static enum cpuhp_state _hp_state;
> +
> +/*
> + * Each PE has its own CMG and Physical PE number (determined by BST_BIT_EL1 register).
> + * Barrier operation can be performed by PEs which belong to the same CMG.
> + */
> +struct pe_info {
> +	/* CMG number of this PE */
> +	u8 cmg;
> +	/* Physical PE number of this PE */
> +	u8 ppe;
> +};
> +
> +/* Hardware information of running system */
> +struct hwb_hwinfo {
> +	/* CPU type (part number) */
> +	unsigned int type;
> +	/* Number of CMG */
> +	u8 num_cmg;
> +	/* Number of barrier blade(BB) per CMG */
> +	u8 num_bb;
> +	/* Number of barrier window(BW) per PE */
> +	u8 num_bw;
> +	/*
> +	 * Maximum number of PE per CMG.
> +	 * Depending on BIOS configuration, each CMG has up to max_pe_per_cmg PEs
> +	 * and each PE has unique physical PE number between 0 ~ (max_pe_per_cmg-1)
> +	 */
> +	u8 max_pe_per_cmg;
> +
> +	/* Bitmap for currently allocated BB per CMG */
> +	unsigned long *used_bb_bmap;
> +	/* Bitmap for currently allocated BW per PE */
> +	unsigned long *used_bw_bmap;
> +	/* Mapping table of cpuid -> CMG/PE number */
> +	struct pe_info *core_map;
> +};
> +static struct hwb_hwinfo _hwinfo;
> +
> +/* List for barrier blade currently used per FD */
> +struct hwb_private_data {
> +	struct list_head bb_list;
> +	spinlock_t list_lock;
> +};
> +
> +/* Each barrier blade info */
> +#define BB_FREEING 1
> +struct bb_info {
> +	/* cpumask for PEs which participate synchronization */
> +	cpumask_var_t pemask;
> +	/* cpumask for PEs which currently assigned BW for this BB */
> +	cpumask_var_t assigned_pemask;
> +	/* Added to hwb_private_data::bb_list */
> +	struct list_head node;
> +	/* For indicating if this bb is currently being freed or not */
> +	unsigned long flag;
> +	/* For waiting ongoing assign/unassign operation to finish before freeing BB */
> +	wait_queue_head_t wq;
> +	/* Track ongoing assign/unassign operation count */
> +	atomic_t ongoing_assign_count;
> +	/* CMG  number of this blade */

nitpick: Double space currently after CMG that looks inconsistent.

> +	u8 cmg;
> +	/* BB number of this blade */
> +	u8 bb;
> +	/* Hold assigned window number of each PE corresponding to @assigned_pemask */
> +	u8 *bw;
> +	/* Track usage count as IOC_BB_FREE and IOC_BW_[UN]ASSIGN might be run in parallel */
> +	struct kref kref;
> +};
> +static struct kmem_cache *bb_info_cachep;
> +
> +static const struct file_operations fujitsu_hwb_dev_fops = {
> +	.owner          = THIS_MODULE,
> +};
> +
> +static struct miscdevice bar_miscdev = {
> +	.fops  = &fujitsu_hwb_dev_fops,
> +	.minor = MISC_DYNAMIC_MINOR,
> +	.mode  = 0666,
> +	.name  = FHWB_DEV_NAME,
> +};
> +
> +static void destroy_bb_info_cachep(void)
> +{
> +	kmem_cache_destroy(bb_info_cachep);
> +}
> +
> +static int __init init_bb_info_cachep(void)
> +{
> +	/*
> +	 * Since cpumask value will be copied from userspace to the beginning of
> +	 * struct bb_info, use kmem_cache_create_usercopy to mark that region.
> +	 * Otherwise CONFIG_HARDENED_USERCOPY gives user_copy_warn.
> +	 */
> +	bb_info_cachep = kmem_cache_create_usercopy("bb_info_cache", sizeof(struct bb_info),
> +			0, SLAB_HWCACHE_ALIGN, 0, sizeof(cpumask_var_t), NULL);
> +	if (bb_info_cachep == NULL)

inconsistent on ! vs == NULL checks.  I personally don't care which you use, but better to chose
one style and use if everywhere in a given driver.

> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void free_map(void)
> +{
> +	kfree(_hwinfo.used_bw_bmap);
> +	kfree(_hwinfo.used_bb_bmap);
> +	kfree(_hwinfo.core_map);
> +}
> +
> +static int __init alloc_map(void)

For generic sounding function names, it is better to prefix with something driver specific.
perhaps hwb_alloc_map() or similar?  Both avoids possible clashes of naming in future and
leads to more readable code as people then know the function is local.


> +{
> +	_hwinfo.core_map = kcalloc(num_possible_cpus(), sizeof(struct pe_info), GFP_KERNEL);

preferred to use sizeof(*_hwinfo.core_map) as saves reviewer checking the types.  Note
applies in other places as well.
Whilst it's nice to make these flexible in size, the separate allocations do add overheads.
Given num_possible_cpus is probably constrained, perhaps better to just make these fixed
size and big enough for all plausible usecases?


> +	_hwinfo.used_bb_bmap = kcalloc(_hwinfo.num_cmg, sizeof(unsigned long), GFP_KERNEL);
> +	_hwinfo.used_bw_bmap = kcalloc(num_possible_cpus(), sizeof(unsigned long), GFP_KERNEL);
> +	if (!_hwinfo.core_map || !_hwinfo.used_bb_bmap || !_hwinfo.used_bw_bmap)
> +		goto fail;

I'd prefer you check these individually and handle the frees explicitly.  Makes for easier reviewing
as we can match each fail against the clean up.

> +
> +	/* 0 is valid number for both CMG/PE. Set all bits to 1 to represents uninitialized state */
> +	memset(_hwinfo.core_map, 0xFF, sizeof(struct pe_info) * num_possible_cpus());
> +
> +	return 0;
> +
> +fail:
> +	free_map();
> +	return -ENOMEM;
> +}
> +
> +/* Get this system's CPU type (part number). If it is not fujitsu CPU, return -1 */
> +static int __init get_cpu_type(void)
> +{
> +	if (read_cpuid_implementor() != ARM_CPU_IMP_FUJITSU)
> +		return -1;

Better to return a meaningful error code from here, then pass it on at the caller.

> +
> +	return read_cpuid_part_number();
> +}
> +
> +static int __init setup_hwinfo(void)
> +{
> +	int type;
> +
> +	type = get_cpu_type();
> +	if (type < 0)

As above, I'd expect to see return type; here.

> +		return -ENODEV;
> +
> +	_hwinfo.type = type;
> +	switch (type) {
> +	case FUJITSU_CPU_PART_A64FX:
> +		_hwinfo.num_cmg = 4;
> +		_hwinfo.num_bb = 6;
> +		_hwinfo.num_bw = 4;
> +		_hwinfo.max_pe_per_cmg = 13;
> +		break;
> +	default:
> +		return -ENODEV;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hwb_cpu_online(unsigned int cpu)
> +{
> +	u64 val;
> +	int i;
> +
> +	/* Setup core_map by reading BST_BIT_EL1 register of each PE */
> +	val = read_sysreg_s(FHWB_BST_BIT_EL1);
> +	_hwinfo.core_map[cpu].cmg = FIELD_GET(FHWB_BST_BIT_EL1_CMG_FILED, val);
> +	_hwinfo.core_map[cpu].ppe = FIELD_GET(FHWB_BST_BIT_EL1_PE_FILED, val);
> +
> +	/* Since these registers' values are UNKNOWN on reset, explicitly clear all */
> +	for (i = 0; i < _hwinfo.num_bw; i++)
> +		write_bw_reg(i, 0);
> +
> +	write_sysreg_s(0, FHWB_CTRL_EL1);
> +
> +	return 0;
> +}
> +
> +static int __init hwb_init(void)
> +{
> +	int ret;
> +
> +	ret = setup_hwinfo();
> +	if (ret < 0) {

As it's not obvious from the function name that the only thing it is doing is
checking the cpu type, I'd move this error print into that function call or
rename setup_hwinfo()


> +		pr_err("Unsupported CPU type\n");
> +		return ret;
> +	}
> +
> +	ret = alloc_map();
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = init_bb_info_cachep();
> +	if (ret < 0)
> +		goto out1;
> +
> +	/*
> +	 * Setup cpuhp callback to ensure each PE's resource will be initialized
> +	 * even if some PEs are offline at this point
> +	 */
> +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/fujitsu_hwb:online",
> +		hwb_cpu_online, NULL);
> +	if (ret < 0) {
> +		pr_err("cpuhp setup failed: %d\n", ret);
> +		goto out2;
> +	}
> +	_hp_state = ret;
> +
> +	ret = misc_register(&bar_miscdev);
> +	if (ret < 0) {
> +		pr_err("misc_register failed: %d\n", ret);
> +		goto out3;
> +	}
> +
> +	return 0;
> +
> +out3:
> +	cpuhp_remove_state(_hp_state);
> +out2:
> +	destroy_bb_info_cachep();
> +out1:
> +	free_map();
> +
> +	return ret;
> +}
> +
> +static void __exit hwb_exit(void)
> +{
> +	misc_deregister(&bar_miscdev);
> +	cpuhp_remove_state(_hp_state);
> +	destroy_bb_info_cachep();
> +	free_map();
> +}
> +
> +module_init(hwb_init);
> +module_exit(hwb_exit);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_AUTHOR("FUJITSU LIMITED");
> +MODULE_DESCRIPTION("FUJITSU HPC Hardware Barrier Driver");


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re:
@ 2021-01-08 12:30   ` Arnd Bergmann
  0 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2021-01-08 12:30 UTC (permalink / raw)
  To: Misono Tomohiro
  Cc: Linux ARM, SoC Team, Will Deacon, Catalin Marinas, Arnd Bergmann,
	Olof Johansson

On Fri, Jan 8, 2021 at 11:32 AM Misono Tomohiro
<misono.tomohiro@jp.fujitsu.com> wrote:
> Subject: [RFC PATCH 00/10] Add Fujitsu A64FX soc entry/hardware barrier driver
> [RFC]
>  This is the first time we upstream drivers for our chip and I want to
>  confirm driver location and patch submission process.
>
>  Based on my observation it seems drivers/soc folder is right place to put
>  this driver, so I added Kconfig entry for arm64 platform config, created
>  soc/fujitsu folder and updated MAINTAINER entry accordingly (last patch).
>  Is it right?

This looks good as a start. It may be possible that during review, we
come up with a different location or a different user interface that may
change the code, but if it stays in drivers/soc/fujitsu, then the other
steps are absolutely right.

>  Also for final submission I think I need to 1) create some public git
>  tree to push driver code (github or something), 2) make pull request to
>  SOC team (soc@kernel.org). Is it a correct procedure?

Yes. I would prefer something other than github, e.g. an account
on a fujitsu.com host, on kernel.org, or on git.linaro.org, but github
works if none of the alternatives are easy for you.

When you send a pull request, make sure you sign the tag with
a gpg key, ideally after getting it on the kernel.org keyring [1].

       Arnd

[1] https://korg.docs.kernel.org/pgpkeys.html

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re:
@ 2021-01-08 12:30   ` Arnd Bergmann
  0 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2021-01-08 12:30 UTC (permalink / raw)
  To: Misono Tomohiro
  Cc: Arnd Bergmann, Catalin Marinas, SoC Team, Olof Johansson,
	Will Deacon, Linux ARM

On Fri, Jan 8, 2021 at 11:32 AM Misono Tomohiro
<misono.tomohiro@jp.fujitsu.com> wrote:
> Subject: [RFC PATCH 00/10] Add Fujitsu A64FX soc entry/hardware barrier driver
> [RFC]
>  This is the first time we upstream drivers for our chip and I want to
>  confirm driver location and patch submission process.
>
>  Based on my observation it seems drivers/soc folder is right place to put
>  this driver, so I added Kconfig entry for arm64 platform config, created
>  soc/fujitsu folder and updated MAINTAINER entry accordingly (last patch).
>  Is it right?

This looks good as a start. It may be possible that during review, we
come up with a different location or a different user interface that may
change the code, but if it stays in drivers/soc/fujitsu, then the other
steps are absolutely right.

>  Also for final submission I think I need to 1) create some public git
>  tree to push driver code (github or something), 2) make pull request to
>  SOC team (soc@kernel.org). Is it a correct procedure?

Yes. I would prefer something other than github, e.g. an account
on a fujitsu.com host, on kernel.org, or on git.linaro.org, but github
works if none of the alternatives are easy for you.

When you send a pull request, make sure you sign the tag with
a gpg key, ideally after getting it on the kernel.org keyring [1].

       Arnd

[1] https://korg.docs.kernel.org/pgpkeys.html

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-08 12:41     ` Arnd Bergmann
  0 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2021-01-08 12:41 UTC (permalink / raw)
  To: Misono Tomohiro
  Cc: Linux ARM, SoC Team, Will Deacon, Catalin Marinas, Arnd Bergmann,
	Olof Johansson

On Fri, Jan 8, 2021 at 11:32 AM Misono Tomohiro
<misono.tomohiro@jp.fujitsu.com> wrote:
> + *
> + * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
> + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> + * On A64FX, CMG is the same as L3 cache domain.
> + *
> + * The main purpose of the driver is setting up registers which cannot be accessed
> + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
> + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> + *
> + * Simplified barrier operation flow of user application is as follows:
> + *  (one PE)
> + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> + *       This specifies which PEs join synchronization
> + *  (on each PE joining synchronization)
> + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> + *    3. Barrier main logic (all logic runs in EL0)
> + *      a) Write 1 to BST_SYNC register
> + *      b) Read LBSY_SYNC register
> + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> + *  (one PE)
> + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> + */

On a very general note, I would like to see some background about how
specific this functionality is to the specific design of A64fx. If there are
other processors with a similar requirement, then it would be best to
define a more abstract user API that can work for any such product.

> +static int __init hwb_init(void)
> +{
> +       int ret;
> +
> +       ret = setup_hwinfo();
> +       if (ret < 0) {
> +               pr_err("Unsupported CPU type\n");
> +               return ret;
> +       }

Loading the module on a different machine should not print a warning:
In general, we want it to be possible to have all hardware specific
drivers built into the kernel, but not print any irritating messages
when they are simply not used on the hardware.

One way to avoid this would be to use a platform_driver() that
only gets loaded when a corresponding hardware device of some
sort is found, or ignored otherwise.

      Arnd

^ permalink raw reply	[flat|nested] 32+ messages in thread

* Re: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-08 12:41     ` Arnd Bergmann
  0 siblings, 0 replies; 32+ messages in thread
From: Arnd Bergmann @ 2021-01-08 12:41 UTC (permalink / raw)
  To: Misono Tomohiro
  Cc: Arnd Bergmann, Catalin Marinas, SoC Team, Olof Johansson,
	Will Deacon, Linux ARM

On Fri, Jan 8, 2021 at 11:32 AM Misono Tomohiro
<misono.tomohiro@jp.fujitsu.com> wrote:
> + *
> + * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
> + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> + * On A64FX, CMG is the same as L3 cache domain.
> + *
> + * The main purpose of the driver is setting up registers which cannot be accessed
> + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
> + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> + *
> + * Simplified barrier operation flow of user application is as follows:
> + *  (one PE)
> + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> + *       This specifies which PEs join synchronization
> + *  (on each PE joining synchronization)
> + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> + *    3. Barrier main logic (all logic runs in EL0)
> + *      a) Write 1 to BST_SYNC register
> + *      b) Read LBSY_SYNC register
> + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> + *  (one PE)
> + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> + */

On a very general note, I would like to see some background about how
specific this functionality is to the specific design of A64fx. If there are
other processors with a similar requirement, then it would be best to
define a more abstract user API that can work for any such product.

> +static int __init hwb_init(void)
> +{
> +       int ret;
> +
> +       ret = setup_hwinfo();
> +       if (ret < 0) {
> +               pr_err("Unsupported CPU type\n");
> +               return ret;
> +       }

Loading the module on a different machine should not print a warning:
In general, we want it to be possible to have all hardware specific
drivers built into the kernel, but not print any irritating messages
when they are simply not used on the hardware.

One way to avoid this would be to use a platform_driver() that
only gets loaded when a corresponding hardware device of some
sort is found, or ignored otherwise.

      Arnd

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-12 10:35       ` misono.tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: misono.tomohiro @ 2021-01-12 10:35 UTC (permalink / raw)
  To: 'Jonathan Cameron'
  Cc: linux-arm-kernel, soc, olof, catalin.marinas, will, arnd

> On Fri, 8 Jan 2021 19:32:18 +0900
> Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> wrote:
> 
> > This adds hardware barrier driver's struct definitions and module
> > init/exit code. We use miscdeice for barrier driver ioctl
> 
> device
> 
> > and /dev/fujitsu_hwb will be created upon module load.
> > Following commits will add each ioctl definition.
> >
> > Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> 
> Hi Misono,
> 
> I was taking a look out of curiosity so the following is definitely
> not a full review but a few general comments inline.

Hi Jonathan,
Thanks for all reviews. I will update accordingly if I send v2
(each comments follows below).

> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/soc/fujitsu/fujitsu_hwb.c | 313
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 313 insertions(+)
> >  create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c
> >
> > diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c
> > b/drivers/soc/fujitsu/fujitsu_hwb.c
> > new file mode 100644
> > index 000000000000..44c32c1683df
> > --- /dev/null
> > +++ b/drivers/soc/fujitsu/fujitsu_hwb.c
> > @@ -0,0 +1,313 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2020 FUJITSU LIMITED
> > + *
> > + * This hardware barrier (HWB) driver provides a set of ioctls to
> > +realize synchronization
> > + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> > + * On A64FX, CMG is the same as L3 cache domain.
> > + *
> > + * The main purpose of the driver is setting up registers which
> > +cannot be accessed
> > + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC
> > +registers which is used
> > + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> > + *
> > + * Simplified barrier operation flow of user application is as follows:
> > + *  (one PE)
> > + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> > + *       This specifies which PEs join synchronization
> > + *  (on each PE joining synchronization)
> > + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> > + *    3. Barrier main logic (all logic runs in EL0)
> > + *      a) Write 1 to BST_SYNC register
> > + *      b) Read LBSY_SYNC register
> > + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> > + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> > + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> > + *  (one PE)
> > + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> > + */
> > +
> > +#include <asm/cputype.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/bitops.h>
> > +#include <linux/cpu.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/slab.h>
> > +#include <linux/wait.h>
> > +
> > +#ifdef pr_fmt
> > +#undef pr_fmt
> > +#endif
> > +#define pr_fmt(fmt) "[%s:%s:%d] " fmt, KBUILD_MODNAME, __func__,
> > +__LINE__
> > +
> > +/* Since miscdevice is used, /dev/fujitsu_hwb will be created when
> > +module is loaded */ #define FHWB_DEV_NAME "fujitsu_hwb"
> > +
> > +/* Implementation defined registers for barrier shared in CMG */
> > +#define FHWB_INIT_SYNC_BB0_EL1  sys_reg(3, 0, 15, 13, 0) #define
> > +FHWB_INIT_SYNC_BB1_EL1  sys_reg(3, 0, 15, 13, 1) #define
> > +FHWB_INIT_SYNC_BB2_EL1  sys_reg(3, 0, 15, 13, 2) #define
> > +FHWB_INIT_SYNC_BB3_EL1  sys_reg(3, 0, 15, 13, 3) #define
> > +FHWB_INIT_SYNC_BB4_EL1  sys_reg(3, 0, 15, 13, 4) #define
> > +FHWB_INIT_SYNC_BB5_EL1  sys_reg(3, 0, 15, 13, 5)
> > +
> > +/* Implementation defined registers for barrier per PE */
> > +#define FHWB_CTRL_EL1           sys_reg(3, 0, 11, 12, 0)
> > +#define FHWB_BST_BIT_EL1        sys_reg(3, 0, 11, 12, 4)
> > +#define FHWB_ASSIGN_SYNC_W0_EL1 sys_reg(3, 0, 15, 15, 0) #define
> > +FHWB_ASSIGN_SYNC_W1_EL1 sys_reg(3, 0, 15, 15, 1) #define
> > +FHWB_ASSIGN_SYNC_W2_EL1 sys_reg(3, 0, 15, 15, 2) #define
> > +FHWB_ASSIGN_SYNC_W3_EL1 sys_reg(3, 0, 15, 15, 3)
> > +
> > +/* Field definitions for above registers */ #define
> > +FHWB_INIT_SYNC_BB_EL1_MASK_FIELD  GENMASK_ULL(44, 32)
> > +#define FHWB_INIT_SYNC_BB_EL1_BST_FIELD   GENMASK_ULL(12, 0)
> > +#define FHWB_CTRL_EL1_EL1AE               BIT_ULL(63)
> > +#define FHWB_CTRL_EL1_EL0AE               BIT_ULL(62)
> > +#define FHWB_BST_BIT_EL1_CMG_FILED        GENMASK_ULL(5, 4)
> > +#define FHWB_BST_BIT_EL1_PE_FILED         GENMASK_ULL(3, 0)
> > +#define FHWB_ASSIGN_SYNC_W_EL1_VALID      BIT_ULL(63)
> > +
> > +static enum cpuhp_state _hp_state;
> > +
> > +/*
> > + * Each PE has its own CMG and Physical PE number (determined by BST_BIT_EL1 register).
> > + * Barrier operation can be performed by PEs which belong to the same CMG.
> > + */
> > +struct pe_info {
> > +	/* CMG number of this PE */
> > +	u8 cmg;
> > +	/* Physical PE number of this PE */
> > +	u8 ppe;
> > +};
> > +
> > +/* Hardware information of running system */ struct hwb_hwinfo {
> > +	/* CPU type (part number) */
> > +	unsigned int type;
> > +	/* Number of CMG */
> > +	u8 num_cmg;
> > +	/* Number of barrier blade(BB) per CMG */
> > +	u8 num_bb;
> > +	/* Number of barrier window(BW) per PE */
> > +	u8 num_bw;
> > +	/*
> > +	 * Maximum number of PE per CMG.
> > +	 * Depending on BIOS configuration, each CMG has up to max_pe_per_cmg PEs
> > +	 * and each PE has unique physical PE number between 0 ~ (max_pe_per_cmg-1)
> > +	 */
> > +	u8 max_pe_per_cmg;
> > +
> > +	/* Bitmap for currently allocated BB per CMG */
> > +	unsigned long *used_bb_bmap;
> > +	/* Bitmap for currently allocated BW per PE */
> > +	unsigned long *used_bw_bmap;
> > +	/* Mapping table of cpuid -> CMG/PE number */
> > +	struct pe_info *core_map;
> > +};
> > +static struct hwb_hwinfo _hwinfo;
> > +
> > +/* List for barrier blade currently used per FD */ struct
> > +hwb_private_data {
> > +	struct list_head bb_list;
> > +	spinlock_t list_lock;
> > +};
> > +
> > +/* Each barrier blade info */
> > +#define BB_FREEING 1
> > +struct bb_info {
> > +	/* cpumask for PEs which participate synchronization */
> > +	cpumask_var_t pemask;
> > +	/* cpumask for PEs which currently assigned BW for this BB */
> > +	cpumask_var_t assigned_pemask;
> > +	/* Added to hwb_private_data::bb_list */
> > +	struct list_head node;
> > +	/* For indicating if this bb is currently being freed or not */
> > +	unsigned long flag;
> > +	/* For waiting ongoing assign/unassign operation to finish before freeing BB */
> > +	wait_queue_head_t wq;
> > +	/* Track ongoing assign/unassign operation count */
> > +	atomic_t ongoing_assign_count;
> > +	/* CMG  number of this blade */
> 
> nitpick: Double space currently after CMG that looks inconsistent.

Right. I will fix it.

> 
> > +	u8 cmg;
> > +	/* BB number of this blade */
> > +	u8 bb;
> > +	/* Hold assigned window number of each PE corresponding to @assigned_pemask */
> > +	u8 *bw;
> > +	/* Track usage count as IOC_BB_FREE and IOC_BW_[UN]ASSIGN might be run in parallel */
> > +	struct kref kref;
> > +};
> > +static struct kmem_cache *bb_info_cachep;
> > +
> > +static const struct file_operations fujitsu_hwb_dev_fops = {
> > +	.owner          = THIS_MODULE,
> > +};
> > +
> > +static struct miscdevice bar_miscdev = {
> > +	.fops  = &fujitsu_hwb_dev_fops,
> > +	.minor = MISC_DYNAMIC_MINOR,
> > +	.mode  = 0666,
> > +	.name  = FHWB_DEV_NAME,
> > +};
> > +
> > +static void destroy_bb_info_cachep(void) {
> > +	kmem_cache_destroy(bb_info_cachep);
> > +}
> > +
> > +static int __init init_bb_info_cachep(void) {
> > +	/*
> > +	 * Since cpumask value will be copied from userspace to the beginning of
> > +	 * struct bb_info, use kmem_cache_create_usercopy to mark that region.
> > +	 * Otherwise CONFIG_HARDENED_USERCOPY gives user_copy_warn.
> > +	 */
> > +	bb_info_cachep = kmem_cache_create_usercopy("bb_info_cache", sizeof(struct bb_info),
> > +			0, SLAB_HWCACHE_ALIGN, 0, sizeof(cpumask_var_t), NULL);
> > +	if (bb_info_cachep == NULL)
> 
> inconsistent on ! vs == NULL checks.  I personally don't care which you use, but better to chose one style and use if
> everywhere in a given driver.

OK. I will use "if (!var)" style in everywhere.

> 
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> > +
> > +static void free_map(void)
> > +{
> > +	kfree(_hwinfo.used_bw_bmap);
> > +	kfree(_hwinfo.used_bb_bmap);
> > +	kfree(_hwinfo.core_map);
> > +}
> > +
> > +static int __init alloc_map(void)
> 
> For generic sounding function names, it is better to prefix with something driver specific.
> perhaps hwb_alloc_map() or similar?  Both avoids possible clashes of naming in future and leads to more readable code
> as people then know the function is local.

OK. I will unify to use "hwb" prefix.

> 
> > +{
> > +	_hwinfo.core_map = kcalloc(num_possible_cpus(), sizeof(struct
> > +pe_info), GFP_KERNEL);
> 
> preferred to use sizeof(*_hwinfo.core_map) as saves reviewer checking the types.  Note applies in other places as well.
OK. I will fix it.

> Whilst it's nice to make these flexible in size, the separate allocations do add overheads.
> Given num_possible_cpus is probably constrained, perhaps better to just make these fixed size and big enough for all
> plausible usecases?

Well, it might be possible that the number of CPUs may vary in systems (though currently all system has 1 CPU),
I'd rather keep current notation.

> 
> 
> > +	_hwinfo.used_bb_bmap = kcalloc(_hwinfo.num_cmg, sizeof(unsigned long), GFP_KERNEL);
> > +	_hwinfo.used_bw_bmap = kcalloc(num_possible_cpus(), sizeof(unsigned long), GFP_KERNEL);
> > +	if (!_hwinfo.core_map || !_hwinfo.used_bb_bmap || !_hwinfo.used_bw_bmap)
> > +		goto fail;
> 
> I'd prefer you check these individually and handle the frees explicitly.  Makes for easier reviewing as we can match each
> fail against the clean up.

OK, I will separate them.

> 
> > +
> > +	/* 0 is valid number for both CMG/PE. Set all bits to 1 to represents uninitialized state */
> > +	memset(_hwinfo.core_map, 0xFF, sizeof(struct pe_info) *
> > +num_possible_cpus());
> > +
> > +	return 0;
> > +
> > +fail:
> > +	free_map();
> > +	return -ENOMEM;
> > +}
> > +
> > +/* Get this system's CPU type (part number). If it is not fujitsu
> > +CPU, return -1 */ static int __init get_cpu_type(void) {
> > +	if (read_cpuid_implementor() != ARM_CPU_IMP_FUJITSU)
> > +		return -1;
> 
> Better to return a meaningful error code from here, then pass it on at the caller.

OK. I will incorporate this code into hwinfo_setup() and return -ENODEV for error.

> 
> > +
> > +	return read_cpuid_part_number();
> > +}
> > +
> > +static int __init setup_hwinfo(void)
> > +{
> > +	int type;
> > +
> > +	type = get_cpu_type();
> > +	if (type < 0)
> 
> As above, I'd expect to see return type; here.	
> 
> > +		return -ENODEV;
> > +
> > +	_hwinfo.type = type;
> > +	switch (type) {
> > +	case FUJITSU_CPU_PART_A64FX:
> > +		_hwinfo.num_cmg = 4;
> > +		_hwinfo.num_bb = 6;
> > +		_hwinfo.num_bw = 4;
> > +		_hwinfo.max_pe_per_cmg = 13;
> > +		break;
> > +	default:
> > +		return -ENODEV;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int hwb_cpu_online(unsigned int cpu) {
> > +	u64 val;
> > +	int i;
> > +
> > +	/* Setup core_map by reading BST_BIT_EL1 register of each PE */
> > +	val = read_sysreg_s(FHWB_BST_BIT_EL1);
> > +	_hwinfo.core_map[cpu].cmg = FIELD_GET(FHWB_BST_BIT_EL1_CMG_FILED, val);
> > +	_hwinfo.core_map[cpu].ppe = FIELD_GET(FHWB_BST_BIT_EL1_PE_FILED,
> > +val);
> > +
> > +	/* Since these registers' values are UNKNOWN on reset, explicitly clear all */
> > +	for (i = 0; i < _hwinfo.num_bw; i++)
> > +		write_bw_reg(i, 0);
> > +
> > +	write_sysreg_s(0, FHWB_CTRL_EL1);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init hwb_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = setup_hwinfo();
> > +	if (ret < 0) {
> 
> As it's not obvious from the function name that the only thing it is doing is checking the cpu type, I'd move this error print
> into that function call or rename setup_hwinfo()

I will remove this error print as following arnd's comment in a different thread.

Thanks,
Misono

> 
> 
> > +		pr_err("Unsupported CPU type\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = alloc_map();
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = init_bb_info_cachep();
> > +	if (ret < 0)
> > +		goto out1;
> > +
> > +	/*
> > +	 * Setup cpuhp callback to ensure each PE's resource will be initialized
> > +	 * even if some PEs are offline at this point
> > +	 */
> > +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/fujitsu_hwb:online",
> > +		hwb_cpu_online, NULL);
> > +	if (ret < 0) {
> > +		pr_err("cpuhp setup failed: %d\n", ret);
> > +		goto out2;
> > +	}
> > +	_hp_state = ret;
> > +
> > +	ret = misc_register(&bar_miscdev);
> > +	if (ret < 0) {
> > +		pr_err("misc_register failed: %d\n", ret);
> > +		goto out3;
> > +	}
> > +
> > +	return 0;
> > +
> > +out3:
> > +	cpuhp_remove_state(_hp_state);
> > +out2:
> > +	destroy_bb_info_cachep();
> > +out1:
> > +	free_map();
> > +
> > +	return ret;
> > +}
> > +
> > +static void __exit hwb_exit(void)
> > +{
> > +	misc_deregister(&bar_miscdev);
> > +	cpuhp_remove_state(_hp_state);
> > +	destroy_bb_info_cachep();
> > +	free_map();
> > +}
> > +
> > +module_init(hwb_init);
> > +module_exit(hwb_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("FUJITSU LIMITED");
> > +MODULE_DESCRIPTION("FUJITSU HPC Hardware Barrier Driver");


^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-12 10:35       ` misono.tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: misono.tomohiro @ 2021-01-12 10:35 UTC (permalink / raw)
  To: 'Jonathan Cameron'
  Cc: arnd, catalin.marinas, soc, olof, will, linux-arm-kernel

> On Fri, 8 Jan 2021 19:32:18 +0900
> Misono Tomohiro <misono.tomohiro@jp.fujitsu.com> wrote:
> 
> > This adds hardware barrier driver's struct definitions and module
> > init/exit code. We use miscdeice for barrier driver ioctl
> 
> device
> 
> > and /dev/fujitsu_hwb will be created upon module load.
> > Following commits will add each ioctl definition.
> >
> > Signed-off-by: Misono Tomohiro <misono.tomohiro@jp.fujitsu.com>
> 
> Hi Misono,
> 
> I was taking a look out of curiosity so the following is definitely
> not a full review but a few general comments inline.

Hi Jonathan,
Thanks for all reviews. I will update accordingly if I send v2
(each comments follows below).

> 
> Thanks,
> 
> Jonathan
> 
> > ---
> >  drivers/soc/fujitsu/fujitsu_hwb.c | 313
> > ++++++++++++++++++++++++++++++
> >  1 file changed, 313 insertions(+)
> >  create mode 100644 drivers/soc/fujitsu/fujitsu_hwb.c
> >
> > diff --git a/drivers/soc/fujitsu/fujitsu_hwb.c
> > b/drivers/soc/fujitsu/fujitsu_hwb.c
> > new file mode 100644
> > index 000000000000..44c32c1683df
> > --- /dev/null
> > +++ b/drivers/soc/fujitsu/fujitsu_hwb.c
> > @@ -0,0 +1,313 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright 2020 FUJITSU LIMITED
> > + *
> > + * This hardware barrier (HWB) driver provides a set of ioctls to
> > +realize synchronization
> > + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> > + * On A64FX, CMG is the same as L3 cache domain.
> > + *
> > + * The main purpose of the driver is setting up registers which
> > +cannot be accessed
> > + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC
> > +registers which is used
> > + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> > + *
> > + * Simplified barrier operation flow of user application is as follows:
> > + *  (one PE)
> > + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> > + *       This specifies which PEs join synchronization
> > + *  (on each PE joining synchronization)
> > + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> > + *    3. Barrier main logic (all logic runs in EL0)
> > + *      a) Write 1 to BST_SYNC register
> > + *      b) Read LBSY_SYNC register
> > + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> > + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> > + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> > + *  (one PE)
> > + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> > + */
> > +
> > +#include <asm/cputype.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/bitops.h>
> > +#include <linux/cpu.h>
> > +#include <linux/cpumask.h>
> > +#include <linux/kernel.h>
> > +#include <linux/miscdevice.h>
> > +#include <linux/module.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/slab.h>
> > +#include <linux/wait.h>
> > +
> > +#ifdef pr_fmt
> > +#undef pr_fmt
> > +#endif
> > +#define pr_fmt(fmt) "[%s:%s:%d] " fmt, KBUILD_MODNAME, __func__,
> > +__LINE__
> > +
> > +/* Since miscdevice is used, /dev/fujitsu_hwb will be created when
> > +module is loaded */ #define FHWB_DEV_NAME "fujitsu_hwb"
> > +
> > +/* Implementation defined registers for barrier shared in CMG */
> > +#define FHWB_INIT_SYNC_BB0_EL1  sys_reg(3, 0, 15, 13, 0) #define
> > +FHWB_INIT_SYNC_BB1_EL1  sys_reg(3, 0, 15, 13, 1) #define
> > +FHWB_INIT_SYNC_BB2_EL1  sys_reg(3, 0, 15, 13, 2) #define
> > +FHWB_INIT_SYNC_BB3_EL1  sys_reg(3, 0, 15, 13, 3) #define
> > +FHWB_INIT_SYNC_BB4_EL1  sys_reg(3, 0, 15, 13, 4) #define
> > +FHWB_INIT_SYNC_BB5_EL1  sys_reg(3, 0, 15, 13, 5)
> > +
> > +/* Implementation defined registers for barrier per PE */
> > +#define FHWB_CTRL_EL1           sys_reg(3, 0, 11, 12, 0)
> > +#define FHWB_BST_BIT_EL1        sys_reg(3, 0, 11, 12, 4)
> > +#define FHWB_ASSIGN_SYNC_W0_EL1 sys_reg(3, 0, 15, 15, 0) #define
> > +FHWB_ASSIGN_SYNC_W1_EL1 sys_reg(3, 0, 15, 15, 1) #define
> > +FHWB_ASSIGN_SYNC_W2_EL1 sys_reg(3, 0, 15, 15, 2) #define
> > +FHWB_ASSIGN_SYNC_W3_EL1 sys_reg(3, 0, 15, 15, 3)
> > +
> > +/* Field definitions for above registers */ #define
> > +FHWB_INIT_SYNC_BB_EL1_MASK_FIELD  GENMASK_ULL(44, 32)
> > +#define FHWB_INIT_SYNC_BB_EL1_BST_FIELD   GENMASK_ULL(12, 0)
> > +#define FHWB_CTRL_EL1_EL1AE               BIT_ULL(63)
> > +#define FHWB_CTRL_EL1_EL0AE               BIT_ULL(62)
> > +#define FHWB_BST_BIT_EL1_CMG_FILED        GENMASK_ULL(5, 4)
> > +#define FHWB_BST_BIT_EL1_PE_FILED         GENMASK_ULL(3, 0)
> > +#define FHWB_ASSIGN_SYNC_W_EL1_VALID      BIT_ULL(63)
> > +
> > +static enum cpuhp_state _hp_state;
> > +
> > +/*
> > + * Each PE has its own CMG and Physical PE number (determined by BST_BIT_EL1 register).
> > + * Barrier operation can be performed by PEs which belong to the same CMG.
> > + */
> > +struct pe_info {
> > +	/* CMG number of this PE */
> > +	u8 cmg;
> > +	/* Physical PE number of this PE */
> > +	u8 ppe;
> > +};
> > +
> > +/* Hardware information of running system */ struct hwb_hwinfo {
> > +	/* CPU type (part number) */
> > +	unsigned int type;
> > +	/* Number of CMG */
> > +	u8 num_cmg;
> > +	/* Number of barrier blade(BB) per CMG */
> > +	u8 num_bb;
> > +	/* Number of barrier window(BW) per PE */
> > +	u8 num_bw;
> > +	/*
> > +	 * Maximum number of PE per CMG.
> > +	 * Depending on BIOS configuration, each CMG has up to max_pe_per_cmg PEs
> > +	 * and each PE has unique physical PE number between 0 ~ (max_pe_per_cmg-1)
> > +	 */
> > +	u8 max_pe_per_cmg;
> > +
> > +	/* Bitmap for currently allocated BB per CMG */
> > +	unsigned long *used_bb_bmap;
> > +	/* Bitmap for currently allocated BW per PE */
> > +	unsigned long *used_bw_bmap;
> > +	/* Mapping table of cpuid -> CMG/PE number */
> > +	struct pe_info *core_map;
> > +};
> > +static struct hwb_hwinfo _hwinfo;
> > +
> > +/* List for barrier blade currently used per FD */ struct
> > +hwb_private_data {
> > +	struct list_head bb_list;
> > +	spinlock_t list_lock;
> > +};
> > +
> > +/* Each barrier blade info */
> > +#define BB_FREEING 1
> > +struct bb_info {
> > +	/* cpumask for PEs which participate synchronization */
> > +	cpumask_var_t pemask;
> > +	/* cpumask for PEs which currently assigned BW for this BB */
> > +	cpumask_var_t assigned_pemask;
> > +	/* Added to hwb_private_data::bb_list */
> > +	struct list_head node;
> > +	/* For indicating if this bb is currently being freed or not */
> > +	unsigned long flag;
> > +	/* For waiting ongoing assign/unassign operation to finish before freeing BB */
> > +	wait_queue_head_t wq;
> > +	/* Track ongoing assign/unassign operation count */
> > +	atomic_t ongoing_assign_count;
> > +	/* CMG  number of this blade */
> 
> nitpick: Double space currently after CMG that looks inconsistent.

Right. I will fix it.

> 
> > +	u8 cmg;
> > +	/* BB number of this blade */
> > +	u8 bb;
> > +	/* Hold assigned window number of each PE corresponding to @assigned_pemask */
> > +	u8 *bw;
> > +	/* Track usage count as IOC_BB_FREE and IOC_BW_[UN]ASSIGN might be run in parallel */
> > +	struct kref kref;
> > +};
> > +static struct kmem_cache *bb_info_cachep;
> > +
> > +static const struct file_operations fujitsu_hwb_dev_fops = {
> > +	.owner          = THIS_MODULE,
> > +};
> > +
> > +static struct miscdevice bar_miscdev = {
> > +	.fops  = &fujitsu_hwb_dev_fops,
> > +	.minor = MISC_DYNAMIC_MINOR,
> > +	.mode  = 0666,
> > +	.name  = FHWB_DEV_NAME,
> > +};
> > +
> > +static void destroy_bb_info_cachep(void) {
> > +	kmem_cache_destroy(bb_info_cachep);
> > +}
> > +
> > +static int __init init_bb_info_cachep(void) {
> > +	/*
> > +	 * Since cpumask value will be copied from userspace to the beginning of
> > +	 * struct bb_info, use kmem_cache_create_usercopy to mark that region.
> > +	 * Otherwise CONFIG_HARDENED_USERCOPY gives user_copy_warn.
> > +	 */
> > +	bb_info_cachep = kmem_cache_create_usercopy("bb_info_cache", sizeof(struct bb_info),
> > +			0, SLAB_HWCACHE_ALIGN, 0, sizeof(cpumask_var_t), NULL);
> > +	if (bb_info_cachep == NULL)
> 
> inconsistent on ! vs == NULL checks.  I personally don't care which you use, but better to chose one style and use if
> everywhere in a given driver.

OK. I will use "if (!var)" style in everywhere.

> 
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> > +
> > +static void free_map(void)
> > +{
> > +	kfree(_hwinfo.used_bw_bmap);
> > +	kfree(_hwinfo.used_bb_bmap);
> > +	kfree(_hwinfo.core_map);
> > +}
> > +
> > +static int __init alloc_map(void)
> 
> For generic sounding function names, it is better to prefix with something driver specific.
> perhaps hwb_alloc_map() or similar?  Both avoids possible clashes of naming in future and leads to more readable code
> as people then know the function is local.

OK. I will unify to use "hwb" prefix.

> 
> > +{
> > +	_hwinfo.core_map = kcalloc(num_possible_cpus(), sizeof(struct
> > +pe_info), GFP_KERNEL);
> 
> preferred to use sizeof(*_hwinfo.core_map) as saves reviewer checking the types.  Note applies in other places as well.
OK. I will fix it.

> Whilst it's nice to make these flexible in size, the separate allocations do add overheads.
> Given num_possible_cpus is probably constrained, perhaps better to just make these fixed size and big enough for all
> plausible usecases?

Well, it might be possible that the number of CPUs may vary in systems (though currently all system has 1 CPU),
I'd rather keep current notation.

> 
> 
> > +	_hwinfo.used_bb_bmap = kcalloc(_hwinfo.num_cmg, sizeof(unsigned long), GFP_KERNEL);
> > +	_hwinfo.used_bw_bmap = kcalloc(num_possible_cpus(), sizeof(unsigned long), GFP_KERNEL);
> > +	if (!_hwinfo.core_map || !_hwinfo.used_bb_bmap || !_hwinfo.used_bw_bmap)
> > +		goto fail;
> 
> I'd prefer you check these individually and handle the frees explicitly.  Makes for easier reviewing as we can match each
> fail against the clean up.

OK, I will separate them.

> 
> > +
> > +	/* 0 is valid number for both CMG/PE. Set all bits to 1 to represents uninitialized state */
> > +	memset(_hwinfo.core_map, 0xFF, sizeof(struct pe_info) *
> > +num_possible_cpus());
> > +
> > +	return 0;
> > +
> > +fail:
> > +	free_map();
> > +	return -ENOMEM;
> > +}
> > +
> > +/* Get this system's CPU type (part number). If it is not fujitsu
> > +CPU, return -1 */ static int __init get_cpu_type(void) {
> > +	if (read_cpuid_implementor() != ARM_CPU_IMP_FUJITSU)
> > +		return -1;
> 
> Better to return a meaningful error code from here, then pass it on at the caller.

OK. I will incorporate this code into hwinfo_setup() and return -ENODEV for error.

> 
> > +
> > +	return read_cpuid_part_number();
> > +}
> > +
> > +static int __init setup_hwinfo(void)
> > +{
> > +	int type;
> > +
> > +	type = get_cpu_type();
> > +	if (type < 0)
> 
> As above, I'd expect to see return type; here.	
> 
> > +		return -ENODEV;
> > +
> > +	_hwinfo.type = type;
> > +	switch (type) {
> > +	case FUJITSU_CPU_PART_A64FX:
> > +		_hwinfo.num_cmg = 4;
> > +		_hwinfo.num_bb = 6;
> > +		_hwinfo.num_bw = 4;
> > +		_hwinfo.max_pe_per_cmg = 13;
> > +		break;
> > +	default:
> > +		return -ENODEV;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int hwb_cpu_online(unsigned int cpu) {
> > +	u64 val;
> > +	int i;
> > +
> > +	/* Setup core_map by reading BST_BIT_EL1 register of each PE */
> > +	val = read_sysreg_s(FHWB_BST_BIT_EL1);
> > +	_hwinfo.core_map[cpu].cmg = FIELD_GET(FHWB_BST_BIT_EL1_CMG_FILED, val);
> > +	_hwinfo.core_map[cpu].ppe = FIELD_GET(FHWB_BST_BIT_EL1_PE_FILED,
> > +val);
> > +
> > +	/* Since these registers' values are UNKNOWN on reset, explicitly clear all */
> > +	for (i = 0; i < _hwinfo.num_bw; i++)
> > +		write_bw_reg(i, 0);
> > +
> > +	write_sysreg_s(0, FHWB_CTRL_EL1);
> > +
> > +	return 0;
> > +}
> > +
> > +static int __init hwb_init(void)
> > +{
> > +	int ret;
> > +
> > +	ret = setup_hwinfo();
> > +	if (ret < 0) {
> 
> As it's not obvious from the function name that the only thing it is doing is checking the cpu type, I'd move this error print
> into that function call or rename setup_hwinfo()

I will remove this error print as following arnd's comment in a different thread.

Thanks,
Misono

> 
> 
> > +		pr_err("Unsupported CPU type\n");
> > +		return ret;
> > +	}
> > +
> > +	ret = alloc_map();
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	ret = init_bb_info_cachep();
> > +	if (ret < 0)
> > +		goto out1;
> > +
> > +	/*
> > +	 * Setup cpuhp callback to ensure each PE's resource will be initialized
> > +	 * even if some PEs are offline at this point
> > +	 */
> > +	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "soc/fujitsu_hwb:online",
> > +		hwb_cpu_online, NULL);
> > +	if (ret < 0) {
> > +		pr_err("cpuhp setup failed: %d\n", ret);
> > +		goto out2;
> > +	}
> > +	_hp_state = ret;
> > +
> > +	ret = misc_register(&bar_miscdev);
> > +	if (ret < 0) {
> > +		pr_err("misc_register failed: %d\n", ret);
> > +		goto out3;
> > +	}
> > +
> > +	return 0;
> > +
> > +out3:
> > +	cpuhp_remove_state(_hp_state);
> > +out2:
> > +	destroy_bb_info_cachep();
> > +out1:
> > +	free_map();
> > +
> > +	return ret;
> > +}
> > +
> > +static void __exit hwb_exit(void)
> > +{
> > +	misc_deregister(&bar_miscdev);
> > +	cpuhp_remove_state(_hp_state);
> > +	destroy_bb_info_cachep();
> > +	free_map();
> > +}
> > +
> > +module_init(hwb_init);
> > +module_exit(hwb_exit);
> > +
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_AUTHOR("FUJITSU LIMITED");
> > +MODULE_DESCRIPTION("FUJITSU HPC Hardware Barrier Driver");


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-12 10:49       ` misono.tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: misono.tomohiro @ 2021-01-12 10:49 UTC (permalink / raw)
  To: 'Arnd Bergmann'
  Cc: Linux ARM, SoC Team, Will Deacon, Catalin Marinas, Arnd Bergmann,
	Olof Johansson

> On Fri, Jan 8, 2021 at 11:32 AM Misono Tomohiro
> <misono.tomohiro@jp.fujitsu.com> wrote:
> > + *
> > + * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
> > + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> > + * On A64FX, CMG is the same as L3 cache domain.
> > + *
> > + * The main purpose of the driver is setting up registers which cannot be accessed
> > + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
> > + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> > + *
> > + * Simplified barrier operation flow of user application is as follows:
> > + *  (one PE)
> > + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> > + *       This specifies which PEs join synchronization
> > + *  (on each PE joining synchronization)
> > + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> > + *    3. Barrier main logic (all logic runs in EL0)
> > + *      a) Write 1 to BST_SYNC register
> > + *      b) Read LBSY_SYNC register
> > + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> > + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> > + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> > + *  (one PE)
> > + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> > + */
> 
> On a very general note, I would like to see some background about how
> specific this functionality is to the specific design of A64fx. If there are
> other processors with a similar requirement, then it would be best to
> define a more abstract user API that can work for any such product.

As I said in reply to cover letter, I don't know any other product having similar features.
 (reply: https://lore.kernel.org/linux-arm-kernel/20210108105241.1757799-1-misono.tomohiro@jp.fujitsu.com/T/#mdb9dd8f8cb8fd0b1ce24983efb8b616a38f4b8e8)

> > +static int __init hwb_init(void)
> > +{
> > +       int ret;
> > +
> > +       ret = setup_hwinfo();
> > +       if (ret < 0) {
> > +               pr_err("Unsupported CPU type\n");
> > +               return ret;
> > +       }
> 
> Loading the module on a different machine should not print a warning:
> In general, we want it to be possible to have all hardware specific
> drivers built into the kernel, but not print any irritating messages
> when they are simply not used on the hardware.
> 
> One way to avoid this would be to use a platform_driver() that
> only gets loaded when a corresponding hardware device of some
> sort is found, or ignored otherwise.

As far as I understand, to use platform_driver() the system needs to provide device tree or
ACPI entry for the driver. Target system uses ACPI but there is no corresponding ACPI HID
for this hardware barrier feature as that is an extension feature to the processor.
So, I thought it is not applicable to use platform_driver().

At least, I will remove this pr_err() If I send updated patch.

Regards,
Tomohiro

^ permalink raw reply	[flat|nested] 32+ messages in thread

* RE: [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code
@ 2021-01-12 10:49       ` misono.tomohiro
  0 siblings, 0 replies; 32+ messages in thread
From: misono.tomohiro @ 2021-01-12 10:49 UTC (permalink / raw)
  To: 'Arnd Bergmann'
  Cc: Arnd Bergmann, Catalin Marinas, SoC Team, Olof Johansson,
	Will Deacon, Linux ARM

> On Fri, Jan 8, 2021 at 11:32 AM Misono Tomohiro
> <misono.tomohiro@jp.fujitsu.com> wrote:
> > + *
> > + * This hardware barrier (HWB) driver provides a set of ioctls to realize synchronization
> > + * by PEs in the same Come Memory Group (CMG) by using implementation defined registers.
> > + * On A64FX, CMG is the same as L3 cache domain.
> > + *
> > + * The main purpose of the driver is setting up registers which cannot be accessed
> > + * from EL0. However, after initialization, BST_SYNC/LBSY_SYNC registers which is used
> > + * in synchronization main logic can be accessed from EL0 (therefore it is fast).
> > + *
> > + * Simplified barrier operation flow of user application is as follows:
> > + *  (one PE)
> > + *    1. Call IOC_BB_ALLOC to setup INIT_SYNC register which is shared in a CMG.
> > + *       This specifies which PEs join synchronization
> > + *  (on each PE joining synchronization)
> > + *    2. Call IOC_BW_ASSIGN to setup ASSIGN_SYNC register per PE
> > + *    3. Barrier main logic (all logic runs in EL0)
> > + *      a) Write 1 to BST_SYNC register
> > + *      b) Read LBSY_SYNC register
> > + *      c) If LBSY_SYNC value is 1, sync is finished, otherwise go back to b
> > + *         (If all PEs joining synchronization write 1 to BST_SYNC, LBSY_SYNC becomes 1)
> > + *    4. Call IOC_BW_UNASSIGN to reset ASSIGN_SYNC register
> > + *  (one PE)
> > + *    5. Call IOC_BB_FREE to reset INIT_SYNC register
> > + */
> 
> On a very general note, I would like to see some background about how
> specific this functionality is to the specific design of A64fx. If there are
> other processors with a similar requirement, then it would be best to
> define a more abstract user API that can work for any such product.

As I said in reply to cover letter, I don't know any other product having similar features.
 (reply: https://lore.kernel.org/linux-arm-kernel/20210108105241.1757799-1-misono.tomohiro@jp.fujitsu.com/T/#mdb9dd8f8cb8fd0b1ce24983efb8b616a38f4b8e8)

> > +static int __init hwb_init(void)
> > +{
> > +       int ret;
> > +
> > +       ret = setup_hwinfo();
> > +       if (ret < 0) {
> > +               pr_err("Unsupported CPU type\n");
> > +               return ret;
> > +       }
> 
> Loading the module on a different machine should not print a warning:
> In general, we want it to be possible to have all hardware specific
> drivers built into the kernel, but not print any irritating messages
> when they are simply not used on the hardware.
> 
> One way to avoid this would be to use a platform_driver() that
> only gets loaded when a corresponding hardware device of some
> sort is found, or ignored otherwise.

As far as I understand, to use platform_driver() the system needs to provide device tree or
ACPI entry for the driver. Target system uses ACPI but there is no corresponding ACPI HID
for this hardware barrier feature as that is an extension feature to the processor.
So, I thought it is not applicable to use platform_driver().

At least, I will remove this pr_err() If I send updated patch.

Regards,
Tomohiro
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 32+ messages in thread

end of thread, other threads:[~2021-01-12 10:52 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-08 10:32 Misono Tomohiro
2021-01-08 10:32 ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 01/10] soc: fujitsu: hwb: Add hardware barrier driver init/exit code Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 11:58   ` Jonathan Cameron
2021-01-08 11:58     ` Jonathan Cameron
2021-01-12 10:35     ` misono.tomohiro
2021-01-12 10:35       ` misono.tomohiro
2021-01-08 12:41   ` Arnd Bergmann
2021-01-08 12:41     ` Arnd Bergmann
2021-01-12 10:49     ` misono.tomohiro
2021-01-12 10:49       ` misono.tomohiro
2021-01-08 10:32 ` [PATCH 02/10] soc: fujtisu: hwb: Add open operation Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 03/10] soc: fujitsu: hwb: Add IOC_BB_ALLOC ioctl Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 04/10] soc: fujitsu: hwb: Add IOC_BW_ASSIGN ioctl Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 05/10] soc: fujitsu: hwb: Add IOC_BW_UNASSIGN ioctl Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 06/10] soc: fujitsu: hwb: Add IOC_BB_FREE ioctl Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 07/10] soc: fujitsu: hwb: Add IOC_GET_PE_INFO ioctl Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 08/10] soc: fujitsu: hwb: Add release operation Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 09/10] soc: fujitsu: hwb: Add sysfs entry Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 10:32 ` [PATCH 10/10] soc: fujitsu: hwb: Add Kconfig/Makefile to build fujitsu_hwb driver Misono Tomohiro
2021-01-08 10:32   ` Misono Tomohiro
2021-01-08 12:30 ` Arnd Bergmann
2021-01-08 12:30   ` Re: Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.