diff mbox series

watchdog: diag288_wdt: Implement module autoload

Message ID 20250410095036.1525057-1-hca@linux.ibm.com
State New
Headers show
Series watchdog: diag288_wdt: Implement module autoload | expand

Commit Message

Heiko Carstens April 10, 2025, 9:50 a.m. UTC
The s390 specific diag288_wdt watchdog driver makes use of the virtual
watchdog timer, which is available in most machine configurations.
If executing the diagnose instruction with subcode 0x288 results in an
exception the watchdog timer is not available, otherwise it is available.

In order to allow module autoload of the diag288_wdt module, move the
detection of the virtual watchdog timer to early boot code, and provide
its availability as a cpu feature.

This allows to make use of module_cpu_feature_match() to automatically load
the module iff the virtual watchdog timer is available.

Suggested-by: Marc Hartmayer <mhartmay@linux.ibm.com>
Tested-by: Mete Durlu <meted@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/boot/startup.c           | 17 ++++++++++
 arch/s390/include/asm/cpufeature.h |  1 +
 arch/s390/include/asm/diag288.h    | 41 +++++++++++++++++++++++
 arch/s390/include/asm/machine.h    |  1 +
 arch/s390/kernel/cpufeature.c      |  5 +++
 drivers/watchdog/diag288_wdt.c     | 53 ++----------------------------
 6 files changed, 68 insertions(+), 50 deletions(-)
 create mode 100644 arch/s390/include/asm/diag288.h

Comments

Heiko Carstens April 15, 2025, 4:24 p.m. UTC | #1
On Thu, Apr 10, 2025 at 05:10:50AM -0700, Guenter Roeck wrote:
> On 4/10/25 02:50, Heiko Carstens wrote:
> > The s390 specific diag288_wdt watchdog driver makes use of the virtual
> > watchdog timer, which is available in most machine configurations.
> > If executing the diagnose instruction with subcode 0x288 results in an
> > exception the watchdog timer is not available, otherwise it is available.
> > 
> > In order to allow module autoload of the diag288_wdt module, move the
> > detection of the virtual watchdog timer to early boot code, and provide
> > its availability as a cpu feature.
> > 
> > This allows to make use of module_cpu_feature_match() to automatically load
> > the module iff the virtual watchdog timer is available.
> > 
> > Suggested-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> > Tested-by: Mete Durlu <meted@linux.ibm.com>
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> 
> Acked-by: Guenter Roeck <linux@roeck-us.net>
> 
> > ---
> >   arch/s390/boot/startup.c           | 17 ++++++++++
> >   arch/s390/include/asm/cpufeature.h |  1 +
> >   arch/s390/include/asm/diag288.h    | 41 +++++++++++++++++++++++
> >   arch/s390/include/asm/machine.h    |  1 +
> >   arch/s390/kernel/cpufeature.c      |  5 +++
> >   drivers/watchdog/diag288_wdt.c     | 53 ++----------------------------
> >   6 files changed, 68 insertions(+), 50 deletions(-)
> >   create mode 100644 arch/s390/include/asm/diag288.h

Guenter, Wim, I assume this can/should go upstream via the s390 tree?
Guenter Roeck April 15, 2025, 4:51 p.m. UTC | #2
On 4/15/25 09:24, Heiko Carstens wrote:
> On Thu, Apr 10, 2025 at 05:10:50AM -0700, Guenter Roeck wrote:
>> On 4/10/25 02:50, Heiko Carstens wrote:
>>> The s390 specific diag288_wdt watchdog driver makes use of the virtual
>>> watchdog timer, which is available in most machine configurations.
>>> If executing the diagnose instruction with subcode 0x288 results in an
>>> exception the watchdog timer is not available, otherwise it is available.
>>>
>>> In order to allow module autoload of the diag288_wdt module, move the
>>> detection of the virtual watchdog timer to early boot code, and provide
>>> its availability as a cpu feature.
>>>
>>> This allows to make use of module_cpu_feature_match() to automatically load
>>> the module iff the virtual watchdog timer is available.
>>>
>>> Suggested-by: Marc Hartmayer <mhartmay@linux.ibm.com>
>>> Tested-by: Mete Durlu <meted@linux.ibm.com>
>>> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
>>
>> Acked-by: Guenter Roeck <linux@roeck-us.net>
>>
>>> ---
>>>    arch/s390/boot/startup.c           | 17 ++++++++++
>>>    arch/s390/include/asm/cpufeature.h |  1 +
>>>    arch/s390/include/asm/diag288.h    | 41 +++++++++++++++++++++++
>>>    arch/s390/include/asm/machine.h    |  1 +
>>>    arch/s390/kernel/cpufeature.c      |  5 +++
>>>    drivers/watchdog/diag288_wdt.c     | 53 ++----------------------------
>>>    6 files changed, 68 insertions(+), 50 deletions(-)
>>>    create mode 100644 arch/s390/include/asm/diag288.h
> 
> Guenter, Wim, I assume this can/should go upstream via the s390 tree?

It touches s390 core code, and you did not suggest otherwise when submitting
the patch, so that was my assumption.

Guenter
diff mbox series

Patch

diff --git a/arch/s390/boot/startup.c b/arch/s390/boot/startup.c
index 06316fb8e0fa..da8337e63a3e 100644
--- a/arch/s390/boot/startup.c
+++ b/arch/s390/boot/startup.c
@@ -6,6 +6,7 @@ 
 #include <asm/boot_data.h>
 #include <asm/extmem.h>
 #include <asm/sections.h>
+#include <asm/diag288.h>
 #include <asm/maccess.h>
 #include <asm/machine.h>
 #include <asm/sysinfo.h>
@@ -71,6 +72,20 @@  static void detect_machine_type(void)
 		set_machine_feature(MFEATURE_VM);
 }
 
+static void detect_diag288(void)
+{
+	/* "BEGIN" in EBCDIC character set */
+	static const char cmd[] = "\xc2\xc5\xc7\xc9\xd5";
+	unsigned long action, len;
+
+	action = machine_is_vm() ? (unsigned long)cmd : LPARWDT_RESTART;
+	len = machine_is_vm() ? sizeof(cmd) : 0;
+	if (__diag288(WDT_FUNC_INIT, MIN_INTERVAL, action, len))
+		return;
+	__diag288(WDT_FUNC_CANCEL, 0, 0, 0);
+	set_machine_feature(MFEATURE_DIAG288);
+}
+
 static void detect_diag9c(void)
 {
 	unsigned int cpu;
@@ -519,6 +534,8 @@  void startup_kernel(void)
 	detect_facilities();
 	detect_diag9c();
 	detect_machine_type();
+	/* detect_diag288() needs machine type */
+	detect_diag288();
 	cmma_init();
 	sanitize_prot_virt_host();
 	max_physmem_end = detect_max_physmem_end();
diff --git a/arch/s390/include/asm/cpufeature.h b/arch/s390/include/asm/cpufeature.h
index e08169bd63a5..6c6a99660e78 100644
--- a/arch/s390/include/asm/cpufeature.h
+++ b/arch/s390/include/asm/cpufeature.h
@@ -15,6 +15,7 @@  enum {
 	S390_CPU_FEATURE_MSA,
 	S390_CPU_FEATURE_VXRS,
 	S390_CPU_FEATURE_UV,
+	S390_CPU_FEATURE_D288,
 	MAX_CPU_FEATURES
 };
 
diff --git a/arch/s390/include/asm/diag288.h b/arch/s390/include/asm/diag288.h
new file mode 100644
index 000000000000..5e1b43cea9d6
--- /dev/null
+++ b/arch/s390/include/asm/diag288.h
@@ -0,0 +1,41 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_S390_DIAG288_H
+#define _ASM_S390_DIAG288_H
+
+#include <asm/asm-extable.h>
+#include <asm/types.h>
+
+#define MIN_INTERVAL 15	    /* Minimal time supported by diag288 */
+#define MAX_INTERVAL 3600   /* One hour should be enough - pure estimation */
+
+#define WDT_DEFAULT_TIMEOUT 30
+
+/* Function codes - init, change, cancel */
+#define WDT_FUNC_INIT 0
+#define WDT_FUNC_CHANGE 1
+#define WDT_FUNC_CANCEL 2
+#define WDT_FUNC_CONCEAL 0x80000000
+
+/* Action codes for LPAR watchdog */
+#define LPARWDT_RESTART 0
+
+static inline int __diag288(unsigned int func, unsigned int timeout,
+			    unsigned long action, unsigned int len)
+{
+	union register_pair r1 = { .even = func, .odd = timeout, };
+	union register_pair r3 = { .even = action, .odd = len, };
+	int rc = -EINVAL;
+
+	asm volatile(
+		"	diag	%[r1],%[r3],0x288\n"
+		"0:	lhi	%[rc],0\n"
+		"1:"
+		EX_TABLE(0b, 1b)
+		: [rc] "+d" (rc)
+		: [r1] "d" (r1.pair), [r3] "d" (r3.pair)
+		: "cc", "memory");
+	return rc;
+}
+
+#endif /* _ASM_S390_DIAG288_H */
diff --git a/arch/s390/include/asm/machine.h b/arch/s390/include/asm/machine.h
index 54478caa5237..8abe5afdbfc4 100644
--- a/arch/s390/include/asm/machine.h
+++ b/arch/s390/include/asm/machine.h
@@ -18,6 +18,7 @@ 
 #define MFEATURE_VM		7
 #define MFEATURE_KVM		8
 #define MFEATURE_LPAR		9
+#define MFEATURE_DIAG288	10
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/s390/kernel/cpufeature.c b/arch/s390/kernel/cpufeature.c
index 1b2ae42a0c15..76210f001028 100644
--- a/arch/s390/kernel/cpufeature.c
+++ b/arch/s390/kernel/cpufeature.c
@@ -5,11 +5,13 @@ 
 
 #include <linux/cpufeature.h>
 #include <linux/bug.h>
+#include <asm/machine.h>
 #include <asm/elf.h>
 
 enum {
 	TYPE_HWCAP,
 	TYPE_FACILITY,
+	TYPE_MACHINE,
 };
 
 struct s390_cpu_feature {
@@ -21,6 +23,7 @@  static struct s390_cpu_feature s390_cpu_features[MAX_CPU_FEATURES] = {
 	[S390_CPU_FEATURE_MSA]	= {.type = TYPE_HWCAP, .num = HWCAP_NR_MSA},
 	[S390_CPU_FEATURE_VXRS]	= {.type = TYPE_HWCAP, .num = HWCAP_NR_VXRS},
 	[S390_CPU_FEATURE_UV]	= {.type = TYPE_FACILITY, .num = 158},
+	[S390_CPU_FEATURE_D288]	= {.type = TYPE_MACHINE, .num = MFEATURE_DIAG288},
 };
 
 /*
@@ -38,6 +41,8 @@  int cpu_have_feature(unsigned int num)
 		return !!(elf_hwcap & BIT(feature->num));
 	case TYPE_FACILITY:
 		return test_facility(feature->num);
+	case TYPE_MACHINE:
+		return test_machine_feature(feature->num);
 	default:
 		WARN_ON_ONCE(1);
 		return 0;
diff --git a/drivers/watchdog/diag288_wdt.c b/drivers/watchdog/diag288_wdt.c
index 76dffc89c641..887d5a6c155b 100644
--- a/drivers/watchdog/diag288_wdt.c
+++ b/drivers/watchdog/diag288_wdt.c
@@ -29,26 +29,13 @@ 
 #include <linux/watchdog.h>
 #include <asm/machine.h>
 #include <asm/ebcdic.h>
+#include <asm/diag288.h>
 #include <asm/diag.h>
 #include <linux/io.h>
 
 #define MAX_CMDLEN 240
 #define DEFAULT_CMD "SYSTEM RESTART"
 
-#define MIN_INTERVAL 15     /* Minimal time supported by diag88 */
-#define MAX_INTERVAL 3600   /* One hour should be enough - pure estimation */
-
-#define WDT_DEFAULT_TIMEOUT 30
-
-/* Function codes - init, change, cancel */
-#define WDT_FUNC_INIT 0
-#define WDT_FUNC_CHANGE 1
-#define WDT_FUNC_CANCEL 2
-#define WDT_FUNC_CONCEAL 0x80000000
-
-/* Action codes for LPAR watchdog */
-#define LPARWDT_RESTART 0
-
 static char wdt_cmd[MAX_CMDLEN] = DEFAULT_CMD;
 static bool conceal_on;
 static bool nowayout_info = WATCHDOG_NOWAYOUT;
@@ -75,22 +62,8 @@  static char *cmd_buf;
 static int diag288(unsigned int func, unsigned int timeout,
 		   unsigned long action, unsigned int len)
 {
-	union register_pair r1 = { .even = func, .odd = timeout, };
-	union register_pair r3 = { .even = action, .odd = len, };
-	int err;
-
 	diag_stat_inc(DIAG_STAT_X288);
-
-	err = -EINVAL;
-	asm volatile(
-		"	diag	%[r1],%[r3],0x288\n"
-		"0:	la	%[err],0\n"
-		"1:\n"
-		EX_TABLE(0b, 1b)
-		: [err] "+d" (err)
-		: [r1] "d" (r1.pair), [r3] "d" (r3.pair)
-		: "cc", "memory");
-	return err;
+	return __diag288(func, timeout, action, len);
 }
 
 static int diag288_str(unsigned int func, unsigned int timeout, char *cmd)
@@ -189,8 +162,6 @@  static struct watchdog_device wdt_dev = {
 
 static int __init diag288_init(void)
 {
-	int ret;
-
 	watchdog_set_nowayout(&wdt_dev, nowayout_info);
 
 	if (machine_is_vm()) {
@@ -199,24 +170,6 @@  static int __init diag288_init(void)
 			pr_err("The watchdog cannot be initialized\n");
 			return -ENOMEM;
 		}
-
-		ret = diag288_str(WDT_FUNC_INIT, MIN_INTERVAL, "BEGIN");
-		if (ret != 0) {
-			pr_err("The watchdog cannot be initialized\n");
-			kfree(cmd_buf);
-			return -EINVAL;
-		}
-	} else {
-		if (diag288(WDT_FUNC_INIT, WDT_DEFAULT_TIMEOUT,
-			    LPARWDT_RESTART, 0)) {
-			pr_err("The watchdog cannot be initialized\n");
-			return -EINVAL;
-		}
-	}
-
-	if (diag288(WDT_FUNC_CANCEL, 0, 0, 0)) {
-		pr_err("The watchdog cannot be deactivated\n");
-		return -EINVAL;
 	}
 
 	return watchdog_register_device(&wdt_dev);
@@ -228,5 +181,5 @@  static void __exit diag288_exit(void)
 	kfree(cmd_buf);
 }
 
-module_init(diag288_init);
+module_cpu_feature_match(S390_CPU_FEATURE_D288, diag288_init);
 module_exit(diag288_exit);