* [PATCH v3 1/3] s390/diag204: Introduce diag204 rework
2026-10-22 15:48 [PATCH v3 0/3] s390: Diag204 Rework Mete Durlu
@ 2026-10-22 15:48 ` Mete Durlu
2026-10-22 16:08 ` sashiko-bot
2026-10-22 15:48 ` [PATCH v3 2/3] s390/diag204: Replace diag204 users with new implementation Mete Durlu
2026-10-22 15:48 ` [PATCH v3 3/3] s390/diag: Add diag204 IOCTL to diag misc device Mete Durlu
2 siblings, 1 reply; 7+ messages in thread
From: Mete Durlu @ 2026-10-22 15:48 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sumanth Korikkar
Cc: Ilya Leoshkevich, linux-s390, Mete Durlu
diag204 is an expensive call, limit its invocation to once per
second at most for all users. Use a caching mechanism to provide diag204
data at demand. Cache buffer gets freed by a delayed work function if
diag204 data is not fetched after a certain amount of time.
Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
arch/s390/include/asm/diag.h | 7 +
arch/s390/kernel/diag/diag204.c | 293 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 300 insertions(+)
diff --git a/arch/s390/include/asm/diag.h b/arch/s390/include/asm/diag.h
index 1416ee1ad323..8669cfa039d3 100644
--- a/arch/s390/include/asm/diag.h
+++ b/arch/s390/include/asm/diag.h
@@ -336,6 +336,13 @@ static inline bool diag204_has_bif(void)
return sclp.has_diag204_bif;
}
+int diag204_store(void *buf, int allow_busy, int allow_busy_wait);
+enum diag204_format diag204_get_info_type(void);
+enum diag204_sc diag204_get_subcode(void);
+void *diag204_allocate_buffer(void);
+void diag204_free_buffer(void *buf);
+bool diag204_is_available(void);
+int diag204_get_buf_size(void);
int diag204(unsigned long subcode, unsigned long size, void *addr);
int diag224(void *ptr);
int diag26c(void *req, void *resp, enum diag26c_sc subcode);
diff --git a/arch/s390/kernel/diag/diag204.c b/arch/s390/kernel/diag/diag204.c
new file mode 100644
index 000000000000..f447bfaf756b
--- /dev/null
+++ b/arch/s390/kernel/diag/diag204.c
@@ -0,0 +1,293 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Request CPU utilization information from underlying diag 0x204
+ * facility. Diag 0x204 provides a variety of utilization values.
+ *
+ * Copyright IBM Corp. 2026
+ */
+
+#define pr_fmt(fmt) "diag204: " fmt
+
+#include <linux/jiffies.h>
+#include <linux/kernel.h>
+#include <linux/timer.h>
+#include <linux/vmalloc.h>
+
+#include <asm/diag.h>
+
+struct diag204_info {
+ int pages; /* number of pages for diag204 data */
+ enum diag204_format type; /* used diag204 data format */
+ enum diag204_sc subcode; /* used diag204 subcode for store */
+ bool has_bif;
+};
+
+struct diag204_cache {
+ ktime_t timestamp;
+ void *buf; /* page aligned diag204 buffer */
+ bool updating; /* ongoing cache update indicator */
+};
+
+static struct diag204_cache g_cache;
+static struct diag204_info g_info;
+static DEFINE_MUTEX(diag204_mutex);
+
+#define DIAG204_CACHE_EXPIRY (NSEC_PER_SEC)
+#define DIAG204_WORK_DELAY (HZ * 10)
+#define DIAG204_BUSY_WAIT (HZ / 10)
+
+static void diag204_work_fn(struct work_struct *work);
+static DECLARE_DELAYED_WORK(diag204_work, diag204_work_fn);
+
+bool diag204_is_available(void)
+{
+ return g_info.pages > 0;
+}
+
+enum diag204_format diag204_get_info_type(void)
+{
+ return g_info.type;
+}
+
+enum diag204_sc diag204_get_subcode(void)
+{
+ return g_info.subcode;
+}
+
+int diag204_get_buf_size(void)
+{
+ return g_info.pages * PAGE_SIZE;
+}
+
+void diag204_free_buffer(void *buf)
+{
+ if (!buf)
+ return;
+ vfree(buf);
+}
+
+/*
+ * If we use subcode 6 or 7 with extended data format, we can (and should) use
+ * vmalloc, since we need a lot of memory in that case. Currently
+ * up to 423 pages!
+ */
+static void *__diag204_allocate_buffer(int pages)
+{
+ void *buf;
+
+ buf = __vmalloc_node(array_size(pages, PAGE_SIZE), PAGE_SIZE,
+ GFP_KERNEL | __GFP_ZERO, NUMA_NO_NODE,
+ __builtin_return_address(0));
+ return buf;
+}
+
+void *diag204_allocate_buffer(void)
+{
+ return __diag204_allocate_buffer(g_info.pages);
+}
+
+static void diag204_schedule_work(void)
+{
+ mod_delayed_work(system_dfl_wq, &diag204_work, DIAG204_WORK_DELAY);
+}
+
+static int diag204_is_cache_valid(unsigned long duration_ns)
+{
+ ktime_t expire;
+
+ lockdep_assert_held(&diag204_mutex);
+ if (!READ_ONCE(g_cache.buf))
+ return 0;
+ expire = ktime_add_ns(READ_ONCE(g_cache.timestamp), duration_ns);
+ if (ktime_after(ktime_get(), expire))
+ return 0;
+ return 1;
+}
+
+static void diag204_set_cache(void *buf)
+{
+ diag204_free_buffer(READ_ONCE(g_cache.buf));
+ WRITE_ONCE(g_cache.buf, buf);
+ if (!buf)
+ WRITE_ONCE(g_cache.timestamp, 0);
+ else
+ WRITE_ONCE(g_cache.timestamp, ktime_get());
+}
+
+static void diag204_work_fn(struct work_struct *work)
+{
+ mutex_lock(&diag204_mutex);
+ if (!READ_ONCE(g_cache.buf))
+ goto unlock;
+ if (diag204_is_cache_valid(DIAG204_CACHE_EXPIRY))
+ goto rearm;
+ if (READ_ONCE(g_cache.updating))
+ goto rearm;
+ diag204_set_cache(NULL);
+ goto unlock;
+rearm:
+ diag204_schedule_work();
+unlock:
+ mutex_unlock(&diag204_mutex);
+}
+
+static int diag204_update_cache(int allow_busy, int allow_busy_wait)
+{
+ unsigned long subcode;
+ void *buf;
+ int rc;
+
+ lockdep_assert_held(&diag204_mutex);
+ buf = diag204_allocate_buffer();
+ if (!buf)
+ return -ENOMEM;
+ subcode = g_info.type | g_info.subcode;
+ if (g_info.has_bif && allow_busy)
+ subcode |= DIAG204_BIF_BIT;
+ WRITE_ONCE(g_cache.updating, true);
+ mutex_unlock(&diag204_mutex);
+ do {
+ rc = diag204(subcode, g_info.pages, buf);
+ if (rc != -EBUSY || !allow_busy_wait)
+ break;
+ if (signal_pending(current)) {
+ rc = -ERESTARTSYS;
+ break;
+ }
+ schedule_timeout_interruptible(DIAG204_BUSY_WAIT);
+ } while (allow_busy_wait);
+ mutex_lock(&diag204_mutex);
+ if (rc >= 0) {
+ diag204_set_cache(buf);
+ diag204_schedule_work();
+ } else {
+ diag204_free_buffer(buf);
+ }
+ WRITE_ONCE(g_cache.updating, false);
+ return rc;
+}
+
+/*
+ * diag204_store() provides users data retrieved from the diag204 facility.
+ * Since diag204 is an expensive call to make, it is limited to a call per
+ * second for now. To circumvent this limitation a caching mechanism is used.
+ *
+ * diag204_store() first checks if diag204 cache is present and valid. If the
+ * cache is valid, it copies the contents to the users buffer.
+ * If the cache is expired, it first updates the cache and then proceeds with
+ * the same copy operation.
+ *
+ * @buf: Users have to manage their own buffers by using
+ * diag204_allocate_buffer() and diag204_free_buffer().
+ *
+ * @allow_busy: If diag204 supports busy indication facility(bif), users can
+ * specify diag204 to utilize it and return BUSY instead of blocking until
+ * data becomes ready.
+ *
+ * @allow_busy_wait: If users allow diag204 to use bif, they would also get
+ * the option to specify an interruptible wait behavior by allowing busy wait.
+ * This is useful since it gives users to interrupt diag204 waiting period
+ * via signals.
+ */
+int diag204_store(void *buf, int allow_busy, int allow_busy_wait)
+{
+ int rc;
+
+ if (!diag204_is_available())
+ return -EOPNOTSUPP;
+ if (!allow_busy && allow_busy_wait)
+ return -EOPNOTSUPP;
+ if (allow_busy && !g_info.has_bif)
+ return -EINVAL;
+ if (!buf)
+ return -ENOMEM;
+ rc = 0;
+ do {
+ mutex_lock(&diag204_mutex);
+ /* Exit loop with lock taken if there is no concurrent update */
+ if (!READ_ONCE(g_cache.updating))
+ break;
+ mutex_unlock(&diag204_mutex);
+ if (allow_busy && !allow_busy_wait)
+ return -EBUSY;
+ if (signal_pending(current))
+ return -ERESTARTSYS;
+ schedule_timeout_interruptible(DIAG204_BUSY_WAIT);
+ } while (1);
+ if (!diag204_is_cache_valid(DIAG204_CACHE_EXPIRY))
+ rc = diag204_update_cache(allow_busy, allow_busy_wait);
+ if (rc >= 0)
+ memcpy(buf, READ_ONCE(g_cache.buf), diag204_get_buf_size());
+ mutex_unlock(&diag204_mutex);
+ return rc;
+}
+
+/*
+ * diag204_probe() has to find out, which type of diagnose 204 implementation
+ * we have on our machine. Currently there are three possible scanarios:
+ * - subcode 4 + simple data format (only one page)
+ * - subcode 4-6 + extended data format
+ * - subcode 4-7 + extended data format
+ *
+ * Subcode 5 is used to retrieve the size of the data, provided by subcodes
+ * 6 and 7. Subcode 7 basically has the same function as subcode 6. In addition
+ * to subcode 6 it provides also information about secondary cpus.
+ * In order to get as much information as possible, we first try
+ * subcode 7, then 6 and if both fail, we use subcode 4.
+ */
+static int diag204_probe(void)
+{
+ unsigned long subcode, type;
+ int pages, rc;
+ void *buf;
+
+ pages = diag204((unsigned long)DIAG204_INFO_EXT |
+ (unsigned long)DIAG204_SUBC_RSI, 0, NULL);
+ if (pages > 0) {
+ buf = __diag204_allocate_buffer(pages);
+ if (!buf) {
+ rc = -ENOMEM;
+ goto fail_alloc;
+ }
+ type = DIAG204_INFO_EXT;
+ if (diag204(DIAG204_SUBC_STIB7 | type, pages, buf) >= 0) {
+ subcode = DIAG204_SUBC_STIB7;
+ goto out;
+ }
+ if (diag204(DIAG204_SUBC_STIB6 | type, pages, buf) >= 0) {
+ subcode = DIAG204_SUBC_STIB6;
+ goto out;
+ }
+ vfree(buf);
+ }
+ /* extended data format is not available, fallback to simple */
+ pages = 1;
+ buf = __diag204_allocate_buffer(pages);
+ if (!buf) {
+ rc = -ENOMEM;
+ goto fail_alloc;
+ }
+ type = DIAG204_INFO_SIMPLE;
+ if (diag204(DIAG204_SUBC_STIB4 | type, pages, buf) >= 0) {
+ subcode = DIAG204_SUBC_STIB4;
+ goto out;
+ }
+ rc = -EOPNOTSUPP;
+ goto fail_store;
+out:
+ rc = 0;
+ g_info.pages = pages;
+ g_info.subcode = subcode;
+ g_info.type = type;
+ g_info.has_bif = diag204_has_bif();
+fail_store:
+ vfree(buf);
+fail_alloc:
+ return rc;
+}
+
+static int __init diag204_init(void)
+{
+ return diag204_probe();
+}
+device_initcall(diag204_init);
--
2.56.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 1/3] s390/diag204: Introduce diag204 rework
2026-10-22 15:48 ` [PATCH v3 1/3] s390/diag204: Introduce diag204 rework Mete Durlu
@ 2026-10-22 16:08 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-22 16:08 UTC (permalink / raw)
To: Mete Durlu
Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] s390/diag204: diag204_store() rejects allow_busy when BIF is unsupported
- [Medium] s390/diag204: diag204_store() returns -ERESTARTSYS when allow_busy_wait is 0
--
commit 4b4fb3ec71dc3de5bb7d6e208cc41238367d2b59
Author: Mete Durlu <meted@linux.ibm.com>
s390/diag204: Introduce diag204 rework
This commit introduces a caching layer for diagnose 204 calls in
arch/s390/kernel/diag/diag204.c to limit hardware invocations to at most
once per second, freeing the cache buffer via delayed work when idle.
> diff --git a/arch/s390/kernel/diag/diag204.c b/arch/s390/kernel/diag/diag204.c
> new file mode 100644
> index 0000000000000..f447bfaf756b4
> --- /dev/null
> +++ b/arch/s390/kernel/diag/diag204.c
> @@ -0,0 +1,293 @@
[ ... ]
> +static int diag204_update_cache(int allow_busy, int allow_busy_wait)
> +{
> + unsigned long subcode;
> + void *buf;
> + int rc;
> +
> + lockdep_assert_held(&diag204_mutex);
> + buf = diag204_allocate_buffer();
> + if (!buf)
> + return -ENOMEM;
> + subcode = g_info.type | g_info.subcode;
> + if (g_info.has_bif && allow_busy)
> + subcode |= DIAG204_BIF_BIT;
[ ... ]
> +int diag204_store(void *buf, int allow_busy, int allow_busy_wait)
> +{
> + int rc;
> +
> + if (!diag204_is_available())
> + return -EOPNOTSUPP;
> + if (!allow_busy && allow_busy_wait)
> + return -EOPNOTSUPP;
> + if (allow_busy && !g_info.has_bif)
> + return -EINVAL;
[Severity: High]
Should diag204_store() in arch/s390/kernel/diag/diag204.c return -EINVAL when
allow_busy is set and g_info.has_bif is false, or should it fall back to a
regular blocking diagnose 204 call?
In diag204_update_cache(), the code checks both g_info.has_bif and allow_busy
before setting DIAG204_BIF_BIT:
subcode = g_info.type | g_info.subcode;
if (g_info.has_bif && allow_busy)
subcode |= DIAG204_BIF_BIT;
Because diag204_update_cache() is only called from diag204_store(), the early
return of -EINVAL in diag204_store() when allow_busy && !g_info.has_bif makes
the g_info.has_bif check in diag204_update_cache() unreachable when
!g_info.has_bif.
When the follow-up change ("s390/diag204: Replace diag204 users with new
implementation") converts existing callers:
- dbfs_d204_create() in arch/s390/hypfs/hypfs_diag.c and
hypfs_diag_create_files() in arch/s390/hypfs/hypfs_diag_fs.c both call
diag204_store(..., 1, 1) unconditionally.
- diag204_get_data() in arch/s390/kernel/sthyi.c calls
diag204_store(diag204_buf, diag204_allow_busy, 0), where
diag204_allow_busy is true on every cache refresh after the initial fill.
On systems where !sclp.has_diag204_bif, will this cause hypfs initialization,
debugfs diag_204 reads, and sthyi cache updates to fail with -EINVAL instead
of falling back to a non-busy-indication diag204 call?
> + if (!buf)
> + return -ENOMEM;
> + rc = 0;
> + do {
> + mutex_lock(&diag204_mutex);
> + /* Exit loop with lock taken if there is no concurrent update */
> + if (!READ_ONCE(g_cache.updating))
> + break;
> + mutex_unlock(&diag204_mutex);
> + if (allow_busy && !allow_busy_wait)
> + return -EBUSY;
> + if (signal_pending(current))
> + return -ERESTARTSYS;
> + schedule_timeout_interruptible(DIAG204_BUSY_WAIT);
> + } while (1);
[Severity: Medium]
Can this wait loop in diag204_store() in arch/s390/kernel/diag/diag204.c
unexpectedly sleep interruptibly and return -ERESTARTSYS when the caller
passes allow_busy = 0 and allow_busy_wait = 0?
If one thread is inside diag204_update_cache() with g_cache.updating set to
true while dropping diag204_mutex, a second thread calling
diag204_store(buf, 0, 0) will see READ_ONCE(g_cache.updating) as true:
if (allow_busy && !allow_busy_wait)
return -EBUSY;
if (signal_pending(current))
return -ERESTARTSYS;
schedule_timeout_interruptible(DIAG204_BUSY_WAIT);
Because allow_busy is 0, the first condition is false and the loop checks
signal_pending(current) and returns -ERESTARTSYS even though allow_busy_wait
was not requested.
When the follow-up commits ("s390/diag204: Replace diag204 users with new
implementation" and "s390/diag: Add diag204 IOCTL to diag misc device") are
applied, callers such as diag204_buf() or the guest sthyi path:
handle_sthyi()
-> sthyi_fill()
-> sthyi_update_cache()
-> fill_dst()
-> diag204_get_data(false)
-> diag204_store(diag204_buf, 0, 0)
can hit this path when a signal or TIF_NOTIFY_SIGNAL is pending during a
concurrent cache update. In handle_sthyi(), an unexpected -ERESTARTSYS return
from sthyi_fill() when the sthyi cache is not yet populated causes
handle_sthyi() to return before filling the guest buffer or setting the guest
condition code after the guest instruction pointer has already been advanced.
Should the concurrent update wait in diag204_store() avoid interruptible
sleep and -ERESTARTSYS when !allow_busy_wait?
> + if (!diag204_is_cache_valid(DIAG204_CACHE_EXPIRY))
> + rc = diag204_update_cache(allow_busy, allow_busy_wait);
> + if (rc >= 0)
> + memcpy(buf, READ_ONCE(g_cache.buf), diag204_get_buf_size());
> + mutex_unlock(&diag204_mutex);
> + return rc;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-diag204ioctl-v3-0-c25e7147736e@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] s390/diag204: Replace diag204 users with new implementation
2026-10-22 15:48 [PATCH v3 0/3] s390: Diag204 Rework Mete Durlu
2026-10-22 15:48 ` [PATCH v3 1/3] s390/diag204: Introduce diag204 rework Mete Durlu
@ 2026-10-22 15:48 ` Mete Durlu
2026-10-22 16:01 ` sashiko-bot
2026-10-22 15:48 ` [PATCH v3 3/3] s390/diag: Add diag204 IOCTL to diag misc device Mete Durlu
2 siblings, 1 reply; 7+ messages in thread
From: Mete Durlu @ 2026-10-22 15:48 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sumanth Korikkar
Cc: Ilya Leoshkevich, linux-s390, Mete Durlu
Replace the existing diag204 users in kernelspace with the new diag204
interface.
Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
arch/s390/hypfs/hypfs_diag.c | 140 ++++------------------------------------
arch/s390/hypfs/hypfs_diag.h | 4 +-
arch/s390/hypfs/hypfs_diag_fs.c | 9 ++-
arch/s390/include/asm/diag.h | 1 -
arch/s390/kernel/diag/Makefile | 2 +-
arch/s390/kernel/sthyi.c | 30 +++------
6 files changed, 29 insertions(+), 157 deletions(-)
diff --git a/arch/s390/hypfs/hypfs_diag.c b/arch/s390/hypfs/hypfs_diag.c
index 08777f57bd24..c3eebe4667fa 100644
--- a/arch/s390/hypfs/hypfs_diag.c
+++ b/arch/s390/hypfs/hypfs_diag.c
@@ -22,139 +22,24 @@
#define DBFS_D204_HDR_VERSION 0
-static enum diag204_sc diag204_store_sc; /* used subcode for store */
-static enum diag204_format diag204_info_type; /* used diag 204 data format */
-
static void *diag204_buf; /* 4K aligned buffer for diag204 data */
-static int diag204_buf_pages; /* number of pages for diag204 data */
-
-enum diag204_format diag204_get_info_type(void)
-{
- return diag204_info_type;
-}
-
-static void diag204_set_info_type(enum diag204_format type)
-{
- diag204_info_type = type;
-}
-
-/* Diagnose 204 functions */
-/*
- * For the old diag subcode 4 with simple data format we have to use real
- * memory. If we use subcode 6 or 7 with extended data format, we can (and
- * should) use vmalloc, since we need a lot of memory in that case. Currently
- * up to 93 pages!
- */
-static void diag204_free_buffer(void)
+static void hypfs_diag204_free_buffer(void)
{
- vfree(diag204_buf);
+ diag204_free_buffer(diag204_buf);
diag204_buf = NULL;
}
-void *diag204_get_buffer(enum diag204_format fmt, int *pages)
+void *hypfs_diag204_get_buffer(void)
{
- if (diag204_buf) {
- *pages = diag204_buf_pages;
+ if (diag204_buf)
return diag204_buf;
- }
- if (fmt == DIAG204_INFO_SIMPLE) {
- *pages = 1;
- } else {/* DIAG204_INFO_EXT */
- *pages = diag204((unsigned long)DIAG204_SUBC_RSI |
- (unsigned long)DIAG204_INFO_EXT, 0, NULL);
- if (*pages <= 0)
- return ERR_PTR(-EOPNOTSUPP);
- }
- diag204_buf = __vmalloc_node(array_size(*pages, PAGE_SIZE),
- PAGE_SIZE, GFP_KERNEL, NUMA_NO_NODE,
- __builtin_return_address(0));
+ diag204_buf = diag204_allocate_buffer();
if (!diag204_buf)
return ERR_PTR(-ENOMEM);
- diag204_buf_pages = *pages;
return diag204_buf;
}
-/*
- * diag204_probe() has to find out, which type of diagnose 204 implementation
- * we have on our machine. Currently there are three possible scanarios:
- * - subcode 4 + simple data format (only one page)
- * - subcode 4-6 + extended data format
- * - subcode 4-7 + extended data format
- *
- * Subcode 5 is used to retrieve the size of the data, provided by subcodes
- * 6 and 7. Subcode 7 basically has the same function as subcode 6. In addition
- * to subcode 6 it provides also information about secondary cpus.
- * In order to get as much information as possible, we first try
- * subcode 7, then 6 and if both fail, we use subcode 4.
- */
-
-static int diag204_probe(void)
-{
- void *buf;
- int pages, rc;
-
- buf = diag204_get_buffer(DIAG204_INFO_EXT, &pages);
- if (!IS_ERR(buf)) {
- if (diag204((unsigned long)DIAG204_SUBC_STIB7 |
- (unsigned long)DIAG204_INFO_EXT, pages, buf) >= 0) {
- diag204_store_sc = DIAG204_SUBC_STIB7;
- diag204_set_info_type(DIAG204_INFO_EXT);
- goto out;
- }
- if (diag204((unsigned long)DIAG204_SUBC_STIB6 |
- (unsigned long)DIAG204_INFO_EXT, pages, buf) >= 0) {
- diag204_store_sc = DIAG204_SUBC_STIB6;
- diag204_set_info_type(DIAG204_INFO_EXT);
- goto out;
- }
- diag204_free_buffer();
- }
-
- /* subcodes 6 and 7 failed, now try subcode 4 */
-
- buf = diag204_get_buffer(DIAG204_INFO_SIMPLE, &pages);
- if (IS_ERR(buf)) {
- rc = PTR_ERR(buf);
- goto fail_alloc;
- }
- if (diag204((unsigned long)DIAG204_SUBC_STIB4 |
- (unsigned long)DIAG204_INFO_SIMPLE, pages, buf) >= 0) {
- diag204_store_sc = DIAG204_SUBC_STIB4;
- diag204_set_info_type(DIAG204_INFO_SIMPLE);
- goto out;
- } else {
- rc = -EOPNOTSUPP;
- goto fail_store;
- }
-out:
- rc = 0;
-fail_store:
- diag204_free_buffer();
-fail_alloc:
- return rc;
-}
-
-int diag204_store(void *buf, int pages)
-{
- unsigned long subcode;
- int rc;
-
- subcode = diag204_get_info_type();
- subcode |= diag204_store_sc;
- if (diag204_has_bif())
- subcode |= DIAG204_BIF_BIT;
- while (1) {
- rc = diag204(subcode, pages, buf);
- if (rc != -EBUSY)
- break;
- if (signal_pending(current))
- return -ERESTARTSYS;
- schedule_timeout_interruptible(DIAG204_BUSY_WAIT);
- }
- return rc < 0 ? rc : 0;
-}
-
struct dbfs_d204_hdr {
u64 len; /* Length of d204 buffer without header */
u16 version; /* Version of header */
@@ -173,19 +58,20 @@ static int dbfs_d204_create(void **data, void **data_free_ptr, size_t *size)
int rc, buf_size;
void *base;
- buf_size = PAGE_SIZE * (diag204_buf_pages + 1) + sizeof(d204->hdr);
+ buf_size = diag204_get_buf_size();
+ buf_size += PAGE_SIZE + sizeof(d204->hdr);
base = vzalloc(buf_size);
if (!base)
return -ENOMEM;
d204 = PTR_ALIGN(base + sizeof(d204->hdr), PAGE_SIZE) - sizeof(d204->hdr);
- rc = diag204_store(d204->buf, diag204_buf_pages);
- if (rc) {
+ rc = diag204_store(d204->buf, 1, 1);
+ if (rc < 0) {
vfree(base);
return rc;
}
d204->hdr.version = DBFS_D204_HDR_VERSION;
- d204->hdr.len = PAGE_SIZE * diag204_buf_pages;
- d204->hdr.sc = diag204_store_sc;
+ d204->hdr.len = diag204_get_buf_size();
+ d204->hdr.sc = diag204_get_subcode();
*data = d204;
*data_free_ptr = base;
*size = d204->hdr.len + sizeof(struct dbfs_d204_hdr);
@@ -202,7 +88,7 @@ __init int hypfs_diag_init(void)
{
int rc;
- if (diag204_probe()) {
+ if (!diag204_is_available()) {
pr_info("The hardware system does not support hypfs\n");
return -ENODATA;
}
@@ -219,6 +105,6 @@ __init int hypfs_diag_init(void)
void hypfs_diag_exit(void)
{
hypfs_diag_fs_exit();
- diag204_free_buffer();
+ hypfs_diag204_free_buffer();
hypfs_dbfs_remove_file(&dbfs_file_d204);
}
diff --git a/arch/s390/hypfs/hypfs_diag.h b/arch/s390/hypfs/hypfs_diag.h
index b5218135b8fe..6b5bea39b6f9 100644
--- a/arch/s390/hypfs/hypfs_diag.h
+++ b/arch/s390/hypfs/hypfs_diag.h
@@ -12,9 +12,7 @@
#include <asm/diag.h>
-enum diag204_format diag204_get_info_type(void);
-void *diag204_get_buffer(enum diag204_format fmt, int *pages);
-int diag204_store(void *buf, int pages);
+void *hypfs_diag204_get_buffer(void);
int __hypfs_diag_fs_init(void);
void __hypfs_diag_fs_exit(void);
diff --git a/arch/s390/hypfs/hypfs_diag_fs.c b/arch/s390/hypfs/hypfs_diag_fs.c
index 39621c326010..2ce9119abe99 100644
--- a/arch/s390/hypfs/hypfs_diag_fs.c
+++ b/arch/s390/hypfs/hypfs_diag_fs.c
@@ -303,15 +303,14 @@ int hypfs_diag_create_files(struct dentry *root)
struct dentry *systems_dir, *hyp_dir;
void *time_hdr, *part_hdr;
void *buffer, *ptr;
- int i, rc, pages;
+ int i, rc;
- buffer = diag204_get_buffer(diag204_get_info_type(), &pages);
+ buffer = hypfs_diag204_get_buffer();
if (IS_ERR(buffer))
return PTR_ERR(buffer);
- rc = diag204_store(buffer, pages);
- if (rc)
+ rc = diag204_store(buffer, 1, 1);
+ if (rc < 0)
return rc;
-
systems_dir = hypfs_mkdir(root, "systems");
if (IS_ERR(systems_dir))
return PTR_ERR(systems_dir);
diff --git a/arch/s390/include/asm/diag.h b/arch/s390/include/asm/diag.h
index 8669cfa039d3..ca78ca45a8a2 100644
--- a/arch/s390/include/asm/diag.h
+++ b/arch/s390/include/asm/diag.h
@@ -121,7 +121,6 @@ enum diag204_sc {
#define DIAG204_SUBCODE_MASK 0xffff
#define DIAG204_BIF_BIT 0x80000000
-#define DIAG204_BUSY_WAIT (HZ / 10)
/* The two available diag 204 data formats */
enum diag204_format {
diff --git a/arch/s390/kernel/diag/Makefile b/arch/s390/kernel/diag/Makefile
index 956aee6c4090..6e255b66be44 100644
--- a/arch/s390/kernel/diag/Makefile
+++ b/arch/s390/kernel/diag/Makefile
@@ -1 +1 @@
-obj-y := diag_misc.o diag324.o diag.o diag310.o
+obj-y := diag_misc.o diag324.o diag.o diag310.o diag204.o
diff --git a/arch/s390/kernel/sthyi.c b/arch/s390/kernel/sthyi.c
index 5eae2e25997a..9adb26588d5b 100644
--- a/arch/s390/kernel/sthyi.c
+++ b/arch/s390/kernel/sthyi.c
@@ -305,29 +305,19 @@ static struct diag204_x_part_block *lpar_cpu_inf(struct lpar_cpu_inf *part_inf,
static void *diag204_get_data(bool diag204_allow_busy)
{
- unsigned long subcode;
void *diag204_buf;
- int pages, rc;
-
- subcode = DIAG204_SUBC_RSI;
- subcode |= DIAG204_INFO_EXT;
- pages = diag204(subcode, 0, NULL);
- if (pages < 0)
- return ERR_PTR(pages);
- if (pages == 0)
- return ERR_PTR(-ENODATA);
- diag204_buf = __vmalloc_node(array_size(pages, PAGE_SIZE),
- PAGE_SIZE, GFP_KERNEL, NUMA_NO_NODE,
- __builtin_return_address(0));
+ int rc;
+
+ if (!diag204_is_available())
+ return ERR_PTR(-EOPNOTSUPP);
+ if (diag204_get_subcode() != DIAG204_SUBC_STIB7)
+ return ERR_PTR(-EOPNOTSUPP);
+ diag204_buf = diag204_allocate_buffer();
if (!diag204_buf)
return ERR_PTR(-ENOMEM);
- subcode = DIAG204_SUBC_STIB7;
- subcode |= DIAG204_INFO_EXT;
- if (diag204_has_bif() && diag204_allow_busy)
- subcode |= DIAG204_BIF_BIT;
- rc = diag204(subcode, pages, diag204_buf);
+ rc = diag204_store(diag204_buf, diag204_allow_busy, 0);
if (rc < 0) {
- vfree(diag204_buf);
+ diag204_free_buffer(diag204_buf);
return ERR_PTR(rc);
}
return diag204_buf;
@@ -462,7 +452,7 @@ static int fill_dst(void *dst, u64 *rc)
fill_hdr(sctns);
fill_stsi(sctns);
fill_diag(sctns, diag204_buf);
- vfree(diag204_buf);
+ diag204_free_buffer(diag204_buf);
return 0;
}
--
2.56.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 2/3] s390/diag204: Replace diag204 users with new implementation
2026-10-22 15:48 ` [PATCH v3 2/3] s390/diag204: Replace diag204 users with new implementation Mete Durlu
@ 2026-10-22 16:01 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-22 16:01 UTC (permalink / raw)
To: Mete Durlu
Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] s390/diag204: diag204_store() calls fail with -EINVAL without BIF support
--
commit e6a66aa1f5ad19fef893eed0590d4b6f880aaa42
Author: Mete Durlu <meted@linux.ibm.com>
s390/diag204: Replace diag204 users with new implementation
This commit converts existing kernelspace diagnose 204 users in hypfs and
sthyi to use the new centralized diag204 interface.
> diff --git a/arch/s390/hypfs/hypfs_diag.c b/arch/s390/hypfs/hypfs_diag.c
> index 08777f57bd244..c3eebe4667faf 100644
> --- a/arch/s390/hypfs/hypfs_diag.c
> +++ b/arch/s390/hypfs/hypfs_diag.c
> @@ -22,139 +22,24 @@
[ ... ]
> -int diag204_store(void *buf, int pages)
> -{
> - unsigned long subcode;
> - int rc;
> -
> - subcode = diag204_get_info_type();
> - subcode |= diag204_store_sc;
> - if (diag204_has_bif())
> - subcode |= DIAG204_BIF_BIT;
> - while (1) {
> - rc = diag204(subcode, pages, buf);
> - if (rc != -EBUSY)
> - break;
> - if (signal_pending(current))
> - return -ERESTARTSYS;
> - schedule_timeout_interruptible(DIAG204_BUSY_WAIT);
> - }
> - return rc < 0 ? rc : 0;
> -}
> -
> struct dbfs_d204_hdr {
> u64 len; /* Length of d204 buffer without header */
> u16 version; /* Version of header */
> @@ -173,19 +58,20 @@ static int dbfs_d204_create(void **data, void **data_free_ptr, size_t *size)
> int rc, buf_size;
> void *base;
>
> - buf_size = PAGE_SIZE * (diag204_buf_pages + 1) + sizeof(d204->hdr);
> + buf_size = diag204_get_buf_size();
> + buf_size += PAGE_SIZE + sizeof(d204->hdr);
> base = vzalloc(buf_size);
> if (!base)
> return -ENOMEM;
> d204 = PTR_ALIGN(base + sizeof(d204->hdr), PAGE_SIZE) - sizeof(d204->hdr);
> - rc = diag204_store(d204->buf, diag204_buf_pages);
> - if (rc) {
> + rc = diag204_store(d204->buf, 1, 1);
> + if (rc < 0) {
> vfree(base);
> return rc;
> }
[ ... ]
> diff --git a/arch/s390/hypfs/hypfs_diag_fs.c b/arch/s390/hypfs/hypfs_diag_fs.c
> index 39621c3260103..2ce9119abe990 100644
> --- a/arch/s390/hypfs/hypfs_diag_fs.c
> +++ b/arch/s390/hypfs/hypfs_diag_fs.c
> @@ -303,15 +303,14 @@ int hypfs_diag_create_files(struct dentry *root)
> struct dentry *systems_dir, *hyp_dir;
> void *time_hdr, *part_hdr;
> void *buffer, *ptr;
> - int i, rc, pages;
> + int i, rc;
>
> - buffer = diag204_get_buffer(diag204_get_info_type(), &pages);
> + buffer = hypfs_diag204_get_buffer();
> if (IS_ERR(buffer))
> return PTR_ERR(buffer);
> - rc = diag204_store(buffer, pages);
> - if (rc)
> + rc = diag204_store(buffer, 1, 1);
> + if (rc < 0)
> return rc;
> -
> systems_dir = hypfs_mkdir(root, "systems");
> if (IS_ERR(systems_dir))
> return PTR_ERR(systems_dir);
[ ... ]
> diff --git a/arch/s390/kernel/sthyi.c b/arch/s390/kernel/sthyi.c
> index 5eae2e25997ae..9adb26588d5b5 100644
> --- a/arch/s390/kernel/sthyi.c
> +++ b/arch/s390/kernel/sthyi.c
> @@ -305,29 +305,19 @@ static struct diag204_x_part_block *lpar_cpu_inf(struct lpar_cpu_inf *part_inf,
>
> static void *diag204_get_data(bool diag204_allow_busy)
> {
> - unsigned long subcode;
> void *diag204_buf;
> - int pages, rc;
> -
> - subcode = DIAG204_SUBC_RSI;
> - subcode |= DIAG204_INFO_EXT;
> - pages = diag204(subcode, 0, NULL);
> - if (pages < 0)
> - return ERR_PTR(pages);
> - if (pages == 0)
> - return ERR_PTR(-ENODATA);
> - diag204_buf = __vmalloc_node(array_size(pages, PAGE_SIZE),
> - PAGE_SIZE, GFP_KERNEL, NUMA_NO_NODE,
> - __builtin_return_address(0));
> + int rc;
> +
> + if (!diag204_is_available())
> + return ERR_PTR(-EOPNOTSUPP);
> + if (diag204_get_subcode() != DIAG204_SUBC_STIB7)
> + return ERR_PTR(-EOPNOTSUPP);
> + diag204_buf = diag204_allocate_buffer();
> if (!diag204_buf)
> return ERR_PTR(-ENOMEM);
> - subcode = DIAG204_SUBC_STIB7;
> - subcode |= DIAG204_INFO_EXT;
> - if (diag204_has_bif() && diag204_allow_busy)
> - subcode |= DIAG204_BIF_BIT;
> - rc = diag204(subcode, pages, diag204_buf);
> + rc = diag204_store(diag204_buf, diag204_allow_busy, 0);
> if (rc < 0) {
> - vfree(diag204_buf);
> + diag204_free_buffer(diag204_buf);
> return ERR_PTR(rc);
> }
> return diag204_buf;
[Severity: High]
Does this cause diag204_store() calls in hypfs and sthyi to fail with
-EINVAL on systems without the Diagnose 204 Busy Indication Facility
(BIF)?
In arch/s390/kernel/diag/diag204.c, diag204_store() rejects any call
where allow_busy is set if g_info.has_bif is false:
if (!allow_busy && allow_busy_wait)
return -EOPNOTSUPP;
if (allow_busy && !g_info.has_bif)
return -EINVAL;
Previously, diag204_store() in arch/s390/hypfs/hypfs_diag.c checked
diag204_has_bif() before setting DIAG204_BIF_BIT, and diag204_get_data()
in arch/s390/kernel/sthyi.c checked both diag204_has_bif() and
diag204_allow_busy. When BIF was not supported, both callers issued
diagnose 204 without DIAG204_BIF_BIT.
With this change, dbfs_d204_create() in arch/s390/hypfs/hypfs_diag.c and
hypfs_diag_create_files() in arch/s390/hypfs/hypfs_diag_fs.c both pass
allow_busy = 1 and allow_busy_wait = 1 unconditionally:
rc = diag204_store(d204->buf, 1, 1);
and:
rc = diag204_store(buffer, 1, 1);
When g_info.has_bif is false, reading the debugfs diag_204 file or
mounting and updating s390_hypfs will immediately return -EINVAL.
Similarly, in arch/s390/kernel/sthyi.c, diag204_get_data() now passes
diag204_allow_busy directly to diag204_store():
sthyi_fill()
-> sthyi_update_cache()
-> fill_dst()
-> diag204_get_data(is_diag204_cached(sctns))
-> diag204_store(diag204_buf, diag204_allow_busy, 0)
On systems without hardware STHYI (!test_facility(74)) and without BIF,
the first sthyi_fill() call succeeds because sthyi_cache.info is zeroed
and is_diag204_cached(sctns) is false. Once fill_diag() populates the
validity bits in sthyi_cache.info and the cache expires after
CACHE_VALID_JIFFIES, subsequent calls to sthyi_fill() pass
diag204_allow_busy = true and fail with -EINVAL because
sthyi_update_cache() only handles -EBUSY.
Should these callers gate allow_busy and allow_busy_wait on BIF support,
or should diag204_store() fall back to a non-BIF store when
!g_info.has_bif?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-diag204ioctl-v3-0-c25e7147736e@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] s390/diag: Add diag204 IOCTL to diag misc device
2026-10-22 15:48 [PATCH v3 0/3] s390: Diag204 Rework Mete Durlu
2026-10-22 15:48 ` [PATCH v3 1/3] s390/diag204: Introduce diag204 rework Mete Durlu
2026-10-22 15:48 ` [PATCH v3 2/3] s390/diag204: Replace diag204 users with new implementation Mete Durlu
@ 2026-10-22 15:48 ` Mete Durlu
2026-10-22 16:08 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Mete Durlu @ 2026-10-22 15:48 UTC (permalink / raw)
To: Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Sumanth Korikkar
Cc: Ilya Leoshkevich, linux-s390, Mete Durlu
Add IOCTL implementation necessary to fetch diag204 data from the new
diag misc device under /dev.
Signed-off-by: Mete Durlu <meted@linux.ibm.com>
---
arch/s390/include/uapi/asm/diag.h | 9 +++++++
arch/s390/kernel/diag/diag204.c | 50 ++++++++++++++++++++++++++++++++++++++
arch/s390/kernel/diag/diag_ioctl.h | 4 +++
arch/s390/kernel/diag/diag_misc.c | 9 +++++++
4 files changed, 72 insertions(+)
diff --git a/arch/s390/include/uapi/asm/diag.h b/arch/s390/include/uapi/asm/diag.h
index b7e6ccb4ff6e..e3d09a450a70 100644
--- a/arch/s390/include/uapi/asm/diag.h
+++ b/arch/s390/include/uapi/asm/diag.h
@@ -22,11 +22,20 @@ struct diag310_memtop {
__u64 nesting_lvl;
};
+struct diag204_request {
+ __u64 address;
+ __u32 allow_busy;
+ __u32 allow_busy_wait;
+};
+
/* Diag ioctl definitions */
#define DIAG324_GET_PIBBUF _IOWR(DIAG_MAGIC_STR, 0x77, struct diag324_pib)
#define DIAG324_GET_PIBLEN _IOR(DIAG_MAGIC_STR, 0x78, size_t)
#define DIAG310_GET_STRIDE _IOR(DIAG_MAGIC_STR, 0x79, size_t)
#define DIAG310_GET_MEMTOPLEN _IOWR(DIAG_MAGIC_STR, 0x7a, size_t)
#define DIAG310_GET_MEMTOPBUF _IOWR(DIAG_MAGIC_STR, 0x7b, struct diag310_memtop)
+#define DIAG204_GET_SUBCODE _IOR(DIAG_MAGIC_STR, 0x7c, size_t)
+#define DIAG204_GET_LEN _IOR(DIAG_MAGIC_STR, 0x7d, size_t)
+#define DIAG204_GET_BUF _IOR(DIAG_MAGIC_STR, 0x7e, struct diag204_request)
#endif /* __S390_UAPI_ASM_DIAG_H */
diff --git a/arch/s390/kernel/diag/diag204.c b/arch/s390/kernel/diag/diag204.c
index f447bfaf756b..237303aa44d6 100644
--- a/arch/s390/kernel/diag/diag204.c
+++ b/arch/s390/kernel/diag/diag204.c
@@ -14,6 +14,8 @@
#include <linux/vmalloc.h>
#include <asm/diag.h>
+#include <uapi/asm/diag.h>
+#include "diag_ioctl.h"
struct diag204_info {
int pages; /* number of pages for diag204 data */
@@ -222,6 +224,54 @@ int diag204_store(void *buf, int allow_busy, int allow_busy_wait)
return rc;
}
+long diag204_subcode(unsigned long arg)
+{
+ if (!diag204_is_available())
+ return -EOPNOTSUPP;
+ if (put_user(diag204_get_subcode(), (size_t __user *)arg))
+ return -EFAULT;
+ return 0;
+}
+
+long diag204_len(unsigned long arg)
+{
+ if (!diag204_is_available())
+ return -EOPNOTSUPP;
+ if (put_user(diag204_get_buf_size(), (size_t __user *)arg))
+ return -EFAULT;
+ return 0;
+}
+
+long diag204_buf(unsigned long arg)
+{
+ struct diag204_request __user *udata;
+ int rc, allow_busy, allow_busy_wait;
+ u64 address;
+ void *buf;
+
+ if (!diag204_is_available())
+ return -EOPNOTSUPP;
+ udata = (struct diag204_request __user *)arg;
+ if (get_user(address, &udata->address))
+ return -EFAULT;
+ if (get_user(allow_busy, &udata->allow_busy))
+ return -EFAULT;
+ if (get_user(allow_busy_wait, &udata->allow_busy_wait))
+ return -EFAULT;
+ buf = diag204_allocate_buffer();
+ if (!buf)
+ return -ENOMEM;
+ rc = diag204_store(buf, allow_busy, allow_busy_wait);
+ if (rc < 0) {
+ diag204_free_buffer(buf);
+ return rc;
+ }
+ rc = copy_to_user((void __user *)address, buf,
+ diag204_get_buf_size());
+ diag204_free_buffer(buf);
+ return rc ? -EFAULT : 0;
+}
+
/*
* diag204_probe() has to find out, which type of diagnose 204 implementation
* we have on our machine. Currently there are three possible scanarios:
diff --git a/arch/s390/kernel/diag/diag_ioctl.h b/arch/s390/kernel/diag/diag_ioctl.h
index 7080be946785..68241c67451a 100644
--- a/arch/s390/kernel/diag/diag_ioctl.h
+++ b/arch/s390/kernel/diag/diag_ioctl.h
@@ -11,4 +11,8 @@ long diag310_memtop_stride(unsigned long arg);
long diag310_memtop_len(unsigned long arg);
long diag310_memtop_buf(unsigned long arg);
+long diag204_subcode(unsigned long arg);
+long diag204_len(unsigned long arg);
+long diag204_buf(unsigned long arg);
+
#endif /* _DIAG_IOCTL_H */
diff --git a/arch/s390/kernel/diag/diag_misc.c b/arch/s390/kernel/diag/diag_misc.c
index efffe02ea02e..ea7ee9ed805a 100644
--- a/arch/s390/kernel/diag/diag_misc.c
+++ b/arch/s390/kernel/diag/diag_misc.c
@@ -35,6 +35,15 @@ static long diag_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
case DIAG310_GET_MEMTOPBUF:
rc = diag310_memtop_buf(arg);
break;
+ case DIAG204_GET_SUBCODE:
+ rc = diag204_subcode(arg);
+ break;
+ case DIAG204_GET_LEN:
+ rc = diag204_len(arg);
+ break;
+ case DIAG204_GET_BUF:
+ rc = diag204_buf(arg);
+ break;
default:
rc = -ENOIOCTLCMD;
break;
--
2.56.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 3/3] s390/diag: Add diag204 IOCTL to diag misc device
2026-10-22 15:48 ` [PATCH v3 3/3] s390/diag: Add diag204 IOCTL to diag misc device Mete Durlu
@ 2026-10-22 16:08 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-22 16:08 UTC (permalink / raw)
To: Mete Durlu
Cc: linux-s390, Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] s390/diag: diag204_store() fails with -EINVAL when BIF is unavailable
--
commit 24db640b46c74f692d9871eb2b56c29382e3bc2f
Author: Mete Durlu <meted@linux.ibm.com>
s390/diag: Add diag204 IOCTL to diag misc device
This commit adds the DIAG204_GET_SUBCODE, DIAG204_GET_LEN, and
DIAG204_GET_BUF ioctls to the /dev/diag misc device to allow userspace to
retrieve diagnose 204 data.
> diff --git a/arch/s390/kernel/diag/diag204.c b/arch/s390/kernel/diag/diag204.c
> index f447bfaf756b4..237303aa44d6f 100644
> --- a/arch/s390/kernel/diag/diag204.c
> +++ b/arch/s390/kernel/diag/diag204.c
[ ... ]
> @@ -222,6 +224,54 @@ int diag204_store(void *buf, int allow_busy, int allow_busy_wait)
> return rc;
> }
>
[ ... ]
> +long diag204_buf(unsigned long arg)
> +{
> + struct diag204_request __user *udata;
> + int rc, allow_busy, allow_busy_wait;
> + u64 address;
> + void *buf;
> +
> + if (!diag204_is_available())
> + return -EOPNOTSUPP;
> + udata = (struct diag204_request __user *)arg;
> + if (get_user(address, &udata->address))
> + return -EFAULT;
> + if (get_user(allow_busy, &udata->allow_busy))
> + return -EFAULT;
> + if (get_user(allow_busy_wait, &udata->allow_busy_wait))
> + return -EFAULT;
> + buf = diag204_allocate_buffer();
> + if (!buf)
> + return -ENOMEM;
> + rc = diag204_store(buf, allow_busy, allow_busy_wait);
[Severity: Medium]
What happens on systems where diagnose 204 is available, but the busy
indication facility (g_info.has_bif) is not supported, when a caller passes a
non-zero allow_busy value?
In arch/s390/kernel/diag/diag204.c, diag204_update_cache() is written to fall
back to a regular diagnose 204 call when g_info.has_bif is false:
subcode = g_info.type | g_info.subcode;
if (g_info.has_bif && allow_busy)
subcode |= DIAG204_BIF_BIT;
However, earlier in the series, diag204_store() in
arch/s390/kernel/diag/diag204.c rejects any call with allow_busy set when
g_info.has_bif is false:
if (allow_busy && !g_info.has_bif)
return -EINVAL;
Because arch/s390/include/uapi/asm/diag.h does not provide an ioctl for
userspace to query whether BIF is supported, a userspace caller invoking
ioctl(fd, DIAG204_GET_BUF, &req) -> diag_ioctl() -> diag204_buf() ->
diag204_store() with allow_busy set has no way to know upfront that it must
pass allow_busy = 0 on machines without BIF.
In addition, the in-kernel callers converted earlier in the series pass
allow_busy = 1 unconditionally when BIF is not supported:
- arch/s390/hypfs/hypfs_diag.c: dbfs_d204_create() calls
diag204_store(d204->buf, 1, 1)
- arch/s390/hypfs/hypfs_diag_fs.c: hypfs_diag_create_files() calls
diag204_store(buffer, 1, 1)
- arch/s390/kernel/sthyi.c: diag204_get_data() calls
diag204_store(diag204_buf, diag204_allow_busy, 0) where
diag204_allow_busy = is_diag204_cached(sctns) is true after the first cache
fill
Should diag204_store() allow allow_busy when !g_info.has_bif and let
diag204_update_cache() ignore it (or clamp allow_busy and allow_busy_wait to 0
when !g_info.has_bif) instead of returning -EINVAL?
> + if (rc < 0) {
> + diag204_free_buffer(buf);
> + return rc;
> + }
> + rc = copy_to_user((void __user *)address, buf,
> + diag204_get_buf_size());
> + diag204_free_buffer(buf);
> + return rc ? -EFAULT : 0;
> +}
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-diag204ioctl-v3-0-c25e7147736e@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 7+ messages in thread