| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <abe3b3f3-0c9d-4ac2-af1f-59aa186c723c@roeck-us.net>
Date: Thu, 10 Apr 2025 05:10:50 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Heiko Carstens <hca@...ux.ibm.com>,
Wim Van Sebroeck <wim@...ux-watchdog.org>
Cc: Alexander Gordeev <agordeev@...ux.ibm.com>,
Vasily Gorbik <gor@...ux.ibm.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>, linux-watchdog@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-s390@...r.kernel.org
Subject: Re: [PATCH] watchdog: diag288_wdt: Implement module autoload
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@...ux.ibm.com>
> Tested-by: Mete Durlu <meted@...ux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@...ux.ibm.com>
Acked-by: Guenter Roeck <linux@...ck-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
>
> 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);
Powered by blists - more mailing lists