* [PATCH v3 0/2] fsi: sbefifo: Add userspace timeout control @ 2022-01-21 5:38 Joel Stanley 2022-01-21 5:38 ` [PATCH v3 1/2] fsi: sbefifo: Use specified value of start of response timeout Joel Stanley 2022-01-21 5:38 ` [PATCH v3 2/2] fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl Joel Stanley 0 siblings, 2 replies; 5+ messages in thread From: Joel Stanley @ 2022-01-21 5:38 UTC (permalink / raw) To: Jeremy Kerr, Alistar Popple, Eddie James, Amitay Isaacs Cc: linux-fsi, linux-kernel Certain SBE operations (e.g. collecting trace information from processors) can take long time (> 10 seconds) to finish before SBE can respond. Such operations will currently timeout due to the default response timeout of 10 seconds. This patchset allows users to set a longer timeout using ioctl on the sbefifo device fd, before issuing SBE operations that are likely to take longer. The userspace that interacts with the sbefifo character device is libpdbg. An example use of the ioctl is in this branch: https://github.com/amitay/pdbg/commits/ioctl v3: Take over from Amitay Clarify use case in commit message Link to userspace implementation in cover letter Amitay Isaacs (2): fsi: sbefifo: Use specified value of start of response timeout fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl drivers/fsi/fsi-sbefifo.c | 53 ++++++++++++++++++++++++++++++++++++++- include/uapi/linux/fsi.h | 14 +++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/2] fsi: sbefifo: Use specified value of start of response timeout 2022-01-21 5:38 [PATCH v3 0/2] fsi: sbefifo: Add userspace timeout control Joel Stanley @ 2022-01-21 5:38 ` Joel Stanley 2022-01-24 14:17 ` Eddie James 2022-01-21 5:38 ` [PATCH v3 2/2] fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl Joel Stanley 1 sibling, 1 reply; 5+ messages in thread From: Joel Stanley @ 2022-01-21 5:38 UTC (permalink / raw) To: Jeremy Kerr, Alistar Popple, Eddie James, Amitay Isaacs Cc: linux-fsi, linux-kernel From: Amitay Isaacs <amitay@ozlabs.org> For some of the chip-ops where sbe needs to collect trace information, sbe can take a long time (>30s) to respond. Currently these chip-ops will timeout as the start of response timeout defaults to 10s. Instead of default value, use specified value. The require timeout value will be set using ioctl. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/fsi/fsi-sbefifo.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c index 52328adef643..1e9b326e8f67 100644 --- a/drivers/fsi/fsi-sbefifo.c +++ b/drivers/fsi/fsi-sbefifo.c @@ -125,6 +125,7 @@ struct sbefifo { bool dead; bool async_ffdc; bool timed_out; + u32 timeout_start_rsp_ms; }; struct sbefifo_user { @@ -549,7 +550,7 @@ static int sbefifo_read_response(struct sbefifo *sbefifo, struct iov_iter *respo dev_vdbg(dev, "reading response, buflen = %zd\n", iov_iter_count(response)); - timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_START_RSP); + timeout = msecs_to_jiffies(sbefifo->timeout_start_rsp_ms); for (;;) { /* Grab FIFO status (this will handle parity errors) */ rc = sbefifo_wait(sbefifo, false, &status, timeout); @@ -972,6 +973,7 @@ static int sbefifo_probe(struct device *dev) sbefifo->fsi_dev = fsi_dev; dev_set_drvdata(dev, sbefifo); mutex_init(&sbefifo->lock); + sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP; /* * Try cleaning up the FIFO. If this fails, we still register the -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/2] fsi: sbefifo: Use specified value of start of response timeout 2022-01-21 5:38 ` [PATCH v3 1/2] fsi: sbefifo: Use specified value of start of response timeout Joel Stanley @ 2022-01-24 14:17 ` Eddie James 0 siblings, 0 replies; 5+ messages in thread From: Eddie James @ 2022-01-24 14:17 UTC (permalink / raw) To: Joel Stanley, Jeremy Kerr, Alistar Popple, Amitay Isaacs Cc: linux-fsi, linux-kernel On 1/20/22 23:38, Joel Stanley wrote: > From: Amitay Isaacs <amitay@ozlabs.org> > > For some of the chip-ops where sbe needs to collect trace information, > sbe can take a long time (>30s) to respond. Currently these chip-ops > will timeout as the start of response timeout defaults to 10s. > > Instead of default value, use specified value. The require timeout > value will be set using ioctl. Reviewed-by: Eddie James <eajames@linux.ibm.com> > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/fsi/fsi-sbefifo.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c > index 52328adef643..1e9b326e8f67 100644 > --- a/drivers/fsi/fsi-sbefifo.c > +++ b/drivers/fsi/fsi-sbefifo.c > @@ -125,6 +125,7 @@ struct sbefifo { > bool dead; > bool async_ffdc; > bool timed_out; > + u32 timeout_start_rsp_ms; > }; > > struct sbefifo_user { > @@ -549,7 +550,7 @@ static int sbefifo_read_response(struct sbefifo *sbefifo, struct iov_iter *respo > > dev_vdbg(dev, "reading response, buflen = %zd\n", iov_iter_count(response)); > > - timeout = msecs_to_jiffies(SBEFIFO_TIMEOUT_START_RSP); > + timeout = msecs_to_jiffies(sbefifo->timeout_start_rsp_ms); > for (;;) { > /* Grab FIFO status (this will handle parity errors) */ > rc = sbefifo_wait(sbefifo, false, &status, timeout); > @@ -972,6 +973,7 @@ static int sbefifo_probe(struct device *dev) > sbefifo->fsi_dev = fsi_dev; > dev_set_drvdata(dev, sbefifo); > mutex_init(&sbefifo->lock); > + sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP; > > /* > * Try cleaning up the FIFO. If this fails, we still register the ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 2/2] fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl 2022-01-21 5:38 [PATCH v3 0/2] fsi: sbefifo: Add userspace timeout control Joel Stanley 2022-01-21 5:38 ` [PATCH v3 1/2] fsi: sbefifo: Use specified value of start of response timeout Joel Stanley @ 2022-01-21 5:38 ` Joel Stanley 2022-01-24 14:17 ` Eddie James 1 sibling, 1 reply; 5+ messages in thread From: Joel Stanley @ 2022-01-21 5:38 UTC (permalink / raw) To: Jeremy Kerr, Alistar Popple, Eddie James, Amitay Isaacs Cc: linux-fsi, linux-kernel From: Amitay Isaacs <amitay@ozlabs.org> FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl sets the read timeout (in seconds) for the response received by sbefifo device from sbe. The timeout affects only the read operation on current sbefifo device fd. Certain SBE operations can take long time to complete and the default timeout of 10 seconds might not be sufficient to start receiving response from SBE. In such cases, allow the timeout to be set to the maximum of 120 seconds. The kernel does not contain the definition of the various SBE operations, so we must expose an interface to userspace to set the timeout for the given operation. Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> Signed-off-by: Joel Stanley <joel@jms.id.au> --- v3: - Clarify why this is an ioctl and not hardcoded in the kernel - Add seconds to the name --- drivers/fsi/fsi-sbefifo.c | 49 +++++++++++++++++++++++++++++++++++++++ include/uapi/linux/fsi.h | 14 +++++++++++ 2 files changed, 63 insertions(+) diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c index 1e9b326e8f67..f52a912cdf16 100644 --- a/drivers/fsi/fsi-sbefifo.c +++ b/drivers/fsi/fsi-sbefifo.c @@ -32,6 +32,8 @@ #include <linux/vmalloc.h> #include <linux/mm.h> +#include <uapi/linux/fsi.h> + /* * The SBEFIFO is a pipe-like FSI device for communicating with * the self boot engine on POWER processors. @@ -134,6 +136,7 @@ struct sbefifo_user { void *cmd_page; void *pending_cmd; size_t pending_len; + u32 read_timeout_ms; }; static DEFINE_MUTEX(sbefifo_ffdc_mutex); @@ -796,6 +799,7 @@ static int sbefifo_user_open(struct inode *inode, struct file *file) return -ENOMEM; } mutex_init(&user->file_lock); + user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP; return 0; } @@ -838,7 +842,9 @@ static ssize_t sbefifo_user_read(struct file *file, char __user *buf, rc = mutex_lock_interruptible(&sbefifo->lock); if (rc) goto bail; + sbefifo->timeout_start_rsp_ms = user->read_timeout_ms; rc = __sbefifo_submit(sbefifo, user->pending_cmd, cmd_len, &resp_iter); + sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP; mutex_unlock(&sbefifo->lock); if (rc < 0) goto bail; @@ -928,12 +934,55 @@ static int sbefifo_user_release(struct inode *inode, struct file *file) return 0; } +static int sbefifo_read_timeout(struct sbefifo_user *user, void __user *argp) +{ + struct device *dev = &user->sbefifo->dev; + u32 timeout; + + if (get_user(timeout, (__u32 __user *)argp)) + return -EFAULT; + + if (timeout == 0) { + user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP; + dev_dbg(dev, "Timeout reset to %d\n", user->read_timeout_ms); + return 0; + } + + if (timeout < 10 || timeout > 120) + return -EINVAL; + + user->read_timeout_ms = timeout * 1000; /* user timeout is in sec */ + + dev_dbg(dev, "Timeout set to %d\n", user->read_timeout_ms); + + return 0; +} + +static long sbefifo_user_ioctl(struct file *file, unsigned int cmd, unsigned long arg) +{ + struct sbefifo_user *user = file->private_data; + int rc = -ENOTTY; + + if (!user) + return -EINVAL; + + mutex_lock(&user->file_lock); + switch (cmd) { + case FSI_SBEFIFO_READ_TIMEOUT_SECONDS: + rc = sbefifo_read_timeout(user, (void __user *)arg); + break; + } + mutex_unlock(&user->file_lock); + return rc; +} + static const struct file_operations sbefifo_fops = { .owner = THIS_MODULE, .open = sbefifo_user_open, .read = sbefifo_user_read, .write = sbefifo_user_write, .release = sbefifo_user_release, + .unlocked_ioctl = sbefifo_user_ioctl, }; static void sbefifo_free(struct device *dev) diff --git a/include/uapi/linux/fsi.h b/include/uapi/linux/fsi.h index da577ecd90e7..b2f1977378c7 100644 --- a/include/uapi/linux/fsi.h +++ b/include/uapi/linux/fsi.h @@ -55,4 +55,18 @@ struct scom_access { #define FSI_SCOM_WRITE _IOWR('s', 0x02, struct scom_access) #define FSI_SCOM_RESET _IOW('s', 0x03, __u32) +/* + * /dev/sbefifo* ioctl interface + */ + +/** + * FSI_SBEFIFO_READ_TIMEOUT sets the read timeout for response from SBE. + * + * The read timeout is specified in seconds. The minimum value of read + * timeout is 10 seconds (default) and the maximum value of read timeout is + * 120 seconds. A read timeout of 0 will reset the value to the default of + * (10 seconds). + */ +#define FSI_SBEFIFO_READ_TIMEOUT_SECONDS _IOW('s', 0x00, __u32) + #endif /* _UAPI_LINUX_FSI_H */ -- 2.34.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 2/2] fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl 2022-01-21 5:38 ` [PATCH v3 2/2] fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl Joel Stanley @ 2022-01-24 14:17 ` Eddie James 0 siblings, 0 replies; 5+ messages in thread From: Eddie James @ 2022-01-24 14:17 UTC (permalink / raw) To: Joel Stanley, Jeremy Kerr, Alistar Popple, Amitay Isaacs Cc: linux-fsi, linux-kernel On 1/20/22 23:38, Joel Stanley wrote: > From: Amitay Isaacs <amitay@ozlabs.org> > > FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl sets the read timeout (in > seconds) for the response received by sbefifo device from sbe. The > timeout affects only the read operation on current sbefifo device fd. > > Certain SBE operations can take long time to complete and the default > timeout of 10 seconds might not be sufficient to start receiving > response from SBE. In such cases, allow the timeout to be set to the > maximum of 120 seconds. > > The kernel does not contain the definition of the various SBE > operations, so we must expose an interface to userspace to set the > timeout for the given operation. Reviewed-by: Eddie James <eajames@linux.ibm.com> > > Signed-off-by: Amitay Isaacs <amitay@ozlabs.org> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > v3: > - Clarify why this is an ioctl and not hardcoded in the kernel > - Add seconds to the name > --- > drivers/fsi/fsi-sbefifo.c | 49 +++++++++++++++++++++++++++++++++++++++ > include/uapi/linux/fsi.h | 14 +++++++++++ > 2 files changed, 63 insertions(+) > > diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c > index 1e9b326e8f67..f52a912cdf16 100644 > --- a/drivers/fsi/fsi-sbefifo.c > +++ b/drivers/fsi/fsi-sbefifo.c > @@ -32,6 +32,8 @@ > #include <linux/vmalloc.h> > #include <linux/mm.h> > > +#include <uapi/linux/fsi.h> > + > /* > * The SBEFIFO is a pipe-like FSI device for communicating with > * the self boot engine on POWER processors. > @@ -134,6 +136,7 @@ struct sbefifo_user { > void *cmd_page; > void *pending_cmd; > size_t pending_len; > + u32 read_timeout_ms; > }; > > static DEFINE_MUTEX(sbefifo_ffdc_mutex); > @@ -796,6 +799,7 @@ static int sbefifo_user_open(struct inode *inode, struct file *file) > return -ENOMEM; > } > mutex_init(&user->file_lock); > + user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP; > > return 0; > } > @@ -838,7 +842,9 @@ static ssize_t sbefifo_user_read(struct file *file, char __user *buf, > rc = mutex_lock_interruptible(&sbefifo->lock); > if (rc) > goto bail; > + sbefifo->timeout_start_rsp_ms = user->read_timeout_ms; > rc = __sbefifo_submit(sbefifo, user->pending_cmd, cmd_len, &resp_iter); > + sbefifo->timeout_start_rsp_ms = SBEFIFO_TIMEOUT_START_RSP; > mutex_unlock(&sbefifo->lock); > if (rc < 0) > goto bail; > @@ -928,12 +934,55 @@ static int sbefifo_user_release(struct inode *inode, struct file *file) > return 0; > } > > +static int sbefifo_read_timeout(struct sbefifo_user *user, void __user *argp) > +{ > + struct device *dev = &user->sbefifo->dev; > + u32 timeout; > + > + if (get_user(timeout, (__u32 __user *)argp)) > + return -EFAULT; > + > + if (timeout == 0) { > + user->read_timeout_ms = SBEFIFO_TIMEOUT_START_RSP; > + dev_dbg(dev, "Timeout reset to %d\n", user->read_timeout_ms); > + return 0; > + } > + > + if (timeout < 10 || timeout > 120) > + return -EINVAL; > + > + user->read_timeout_ms = timeout * 1000; /* user timeout is in sec */ > + > + dev_dbg(dev, "Timeout set to %d\n", user->read_timeout_ms); > + > + return 0; > +} > + > +static long sbefifo_user_ioctl(struct file *file, unsigned int cmd, unsigned long arg) > +{ > + struct sbefifo_user *user = file->private_data; > + int rc = -ENOTTY; > + > + if (!user) > + return -EINVAL; > + > + mutex_lock(&user->file_lock); > + switch (cmd) { > + case FSI_SBEFIFO_READ_TIMEOUT_SECONDS: > + rc = sbefifo_read_timeout(user, (void __user *)arg); > + break; > + } > + mutex_unlock(&user->file_lock); > + return rc; > +} > + > static const struct file_operations sbefifo_fops = { > .owner = THIS_MODULE, > .open = sbefifo_user_open, > .read = sbefifo_user_read, > .write = sbefifo_user_write, > .release = sbefifo_user_release, > + .unlocked_ioctl = sbefifo_user_ioctl, > }; > > static void sbefifo_free(struct device *dev) > diff --git a/include/uapi/linux/fsi.h b/include/uapi/linux/fsi.h > index da577ecd90e7..b2f1977378c7 100644 > --- a/include/uapi/linux/fsi.h > +++ b/include/uapi/linux/fsi.h > @@ -55,4 +55,18 @@ struct scom_access { > #define FSI_SCOM_WRITE _IOWR('s', 0x02, struct scom_access) > #define FSI_SCOM_RESET _IOW('s', 0x03, __u32) > > +/* > + * /dev/sbefifo* ioctl interface > + */ > + > +/** > + * FSI_SBEFIFO_READ_TIMEOUT sets the read timeout for response from SBE. > + * > + * The read timeout is specified in seconds. The minimum value of read > + * timeout is 10 seconds (default) and the maximum value of read timeout is > + * 120 seconds. A read timeout of 0 will reset the value to the default of > + * (10 seconds). > + */ > +#define FSI_SBEFIFO_READ_TIMEOUT_SECONDS _IOW('s', 0x00, __u32) > + > #endif /* _UAPI_LINUX_FSI_H */ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-24 14:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-21 5:38 [PATCH v3 0/2] fsi: sbefifo: Add userspace timeout control Joel Stanley 2022-01-21 5:38 ` [PATCH v3 1/2] fsi: sbefifo: Use specified value of start of response timeout Joel Stanley 2022-01-24 14:17 ` Eddie James 2022-01-21 5:38 ` [PATCH v3 2/2] fsi: sbefifo: Implement FSI_SBEFIFO_READ_TIMEOUT_SECONDS ioctl Joel Stanley 2022-01-24 14:17 ` Eddie James
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.