Message ID | c67f4ad53054761e0ae51eb85bd32e880c614288.camel@kernel.crashing.org |
---|---|
State | Not Applicable, archived |
Headers | show |
Series | [v3] fsi: sbefifo: Bump max command length | expand |
-----Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: ----- > To: openbmc@lists.ozlabs.org > From: Benjamin Herrenschmidt <benh@kernel.crashing.org> > Date: 2018/08/08 13:01 > Cc: Eddie James <eajames@linux.vnet.ibm.com>, Andrew Jeffery > <andrewrj@au1.ibm.com>, Joel Stanley <joel@jms.id.au> > Subject: [PATCH v3] fsi: sbefifo: Bump max command length > > Otherwise cronus putmem fails istep and BML fails to upload skiboot > > To do that, we still use our one-page command buffer for small commands > for speed, and for anything bigger, with a limit of 1MB plus a page, > we vmalloc a temporary buffer. > > The limit was chosen because Cronus will break up any data transfer > into 1M chunks (the extra page is for the command header). > > Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> Reviewed-by: Andrew Jeffery <andrew@aj.id.au> > --- > v3. Factor the buffer cleanup code into a helper function > > v2. Reduce max to 1MB + PAGE_SIZE which is enough for Cronus as > it will break up transfers in 1MB chunks and we can make > sure pdbg does the same. > > drivers/fsi/fsi-sbefifo.c | 39 ++++++++++++++++++++++++++++++++------- > 1 file changed, 32 insertions(+), 7 deletions(-) > > diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c > index f97ec3e4ddea..9e778aac9ae7 100644 > --- a/drivers/fsi/fsi-sbefifo.c > +++ b/drivers/fsi/fsi-sbefifo.c > @@ -30,6 +30,7 @@ > #include <linux/delay.h> > #include <linux/uio.h> > #include <linux/vmalloc.h> > +#include <linux/mm.h> > > /* > * The SBEFIFO is a pipe-like FSI device for communicating with > @@ -110,7 +111,7 @@ enum sbe_state > #define SBEFIFO_TIMEOUT_IN_RSP 1000 > > /* Other constants */ > -#define SBEFIFO_MAX_CMD_LEN PAGE_SIZE > +#define SBEFIFO_MAX_USER_CMD_LEN (0x100000 + PAGE_SIZE) > #define SBEFIFO_RESET_MAGIC 0x52534554 /* "RSET" */ > #define SBEFIFO_BENCH_MAGIC 0x424E4348 /* "BNCH" */ > > @@ -129,6 +130,7 @@ struct sbefifo { > struct sbefifo_user { > struct sbefifo *sbefifo; > struct mutex file_lock; > + void *cmd_page; > void *pending_cmd; > size_t pending_len; > }; > @@ -724,7 +726,7 @@ int sbefifo_submit(struct device *dev, const __be32 > *command, size_t cmd_len, > return -ENODEV; > if (WARN_ON_ONCE(sbefifo->magic != SBEFIFO_MAGIC)) > return -ENODEV; > - if (!resp_len || !command || !response || cmd_len > SBEFIFO_MAX_CMD_LEN) > + if (!resp_len || !command || !response) > return -EINVAL; > > /* Prepare iov iterator */ > @@ -749,6 +751,15 @@ EXPORT_SYMBOL_GPL(sbefifo_submit); > /* > * Char device interface > */ > + > +static void sbefifo_release_command(struct sbefifo_user *user) > +{ > + if (is_vmalloc_addr(user->pending_cmd)) > + vfree(user->pending_cmd); > + user->pending_cmd = NULL; > + user->pending_len = 0; > +} > + > static int sbefifo_user_open(struct inode *inode, struct file *file) > { > struct sbefifo *sbefifo = container_of(inode->i_cdev, struct sbefifo, cdev); > @@ -760,8 +771,8 @@ static int sbefifo_user_open(struct inode *inode, struct > file *file) > > file->private_data = user; > user->sbefifo = sbefifo; > - user->pending_cmd = (void *)__get_free_page(GFP_KERNEL); > - if (!user->pending_cmd) { > + user->cmd_page = (void *)__get_free_page(GFP_KERNEL); > + if (!user->cmd_page) { > kfree(user); > return -ENOMEM; > } > @@ -814,7 +825,7 @@ static ssize_t sbefifo_user_read(struct file *file, char > __user *buf, > /* Extract the response length */ > rc = len - iov_iter_count(&resp_iter); > bail: > - user->pending_len = 0; > + sbefifo_release_command(user); > mutex_unlock(&user->file_lock); > return rc; > } > @@ -859,13 +870,23 @@ static ssize_t sbefifo_user_write(struct file *file, > const char __user *buf, > if (!user) > return -EINVAL; > sbefifo = user->sbefifo; > - if (len > SBEFIFO_MAX_CMD_LEN) > + if (len > SBEFIFO_MAX_USER_CMD_LEN) > return -EINVAL; > if (len & 3) > return -EINVAL; > > mutex_lock(&user->file_lock); > > + /* Can we use the pre-allocate buffer ? If not, allocate */ > + if (len <= PAGE_SIZE) > + user->pending_cmd = user->cmd_page; > + else > + user->pending_cmd = vmalloc(len); > + if (!user->pending_cmd) { > + rc = -ENOMEM; > + goto bail; > + } > + > /* Copy the command into the staging buffer */ > if (copy_from_user(user->pending_cmd, buf, len)) { > rc = -EFAULT; > @@ -906,6 +927,9 @@ static ssize_t sbefifo_user_write(struct file *file, const > char __user *buf, > /* Update the staging buffer size */ > user->pending_len = len; > bail: > + if (!user->pending_len) > + sbefifo_release_command(user); > + > mutex_unlock(&user->file_lock); > > /* And that's it, we'll issue the command on a read */ > @@ -919,7 +943,8 @@ static int sbefifo_user_release(struct inode *inode, > struct file *file) > if (!user) > return -EINVAL; > > - free_page((unsigned long)user->pending_cmd); > + sbefifo_release_command(user); > + free_page((unsigned long)user->cmd_page); > kfree(user); > > return 0; > > >
diff --git a/drivers/fsi/fsi-sbefifo.c b/drivers/fsi/fsi-sbefifo.c index f97ec3e4ddea..9e778aac9ae7 100644 --- a/drivers/fsi/fsi-sbefifo.c +++ b/drivers/fsi/fsi-sbefifo.c @@ -30,6 +30,7 @@ #include <linux/delay.h> #include <linux/uio.h> #include <linux/vmalloc.h> +#include <linux/mm.h> /* * The SBEFIFO is a pipe-like FSI device for communicating with @@ -110,7 +111,7 @@ enum sbe_state #define SBEFIFO_TIMEOUT_IN_RSP 1000 /* Other constants */ -#define SBEFIFO_MAX_CMD_LEN PAGE_SIZE +#define SBEFIFO_MAX_USER_CMD_LEN (0x100000 + PAGE_SIZE) #define SBEFIFO_RESET_MAGIC 0x52534554 /* "RSET" */ #define SBEFIFO_BENCH_MAGIC 0x424E4348 /* "BNCH" */ @@ -129,6 +130,7 @@ struct sbefifo { struct sbefifo_user { struct sbefifo *sbefifo; struct mutex file_lock; + void *cmd_page; void *pending_cmd; size_t pending_len; }; @@ -724,7 +726,7 @@ int sbefifo_submit(struct device *dev, const __be32 *command, size_t cmd_len, return -ENODEV; if (WARN_ON_ONCE(sbefifo->magic != SBEFIFO_MAGIC)) return -ENODEV; - if (!resp_len || !command || !response || cmd_len > SBEFIFO_MAX_CMD_LEN) + if (!resp_len || !command || !response) return -EINVAL; /* Prepare iov iterator */ @@ -749,6 +751,15 @@ EXPORT_SYMBOL_GPL(sbefifo_submit); /* * Char device interface */ + +static void sbefifo_release_command(struct sbefifo_user *user) +{ + if (is_vmalloc_addr(user->pending_cmd)) + vfree(user->pending_cmd); + user->pending_cmd = NULL; + user->pending_len = 0; +} + static int sbefifo_user_open(struct inode *inode, struct file *file) { struct sbefifo *sbefifo = container_of(inode->i_cdev, struct sbefifo, cdev); @@ -760,8 +771,8 @@ static int sbefifo_user_open(struct inode *inode, struct file *file) file->private_data = user; user->sbefifo = sbefifo; - user->pending_cmd = (void *)__get_free_page(GFP_KERNEL); - if (!user->pending_cmd) { + user->cmd_page = (void *)__get_free_page(GFP_KERNEL); + if (!user->cmd_page) { kfree(user); return -ENOMEM; } @@ -814,7 +825,7 @@ static ssize_t sbefifo_user_read(struct file *file, char __user *buf, /* Extract the response length */ rc = len - iov_iter_count(&resp_iter); bail: - user->pending_len = 0; + sbefifo_release_command(user); mutex_unlock(&user->file_lock); return rc; } @@ -859,13 +870,23 @@ static ssize_t sbefifo_user_write(struct file *file, const char __user *buf, if (!user) return -EINVAL; sbefifo = user->sbefifo; - if (len > SBEFIFO_MAX_CMD_LEN) + if (len > SBEFIFO_MAX_USER_CMD_LEN) return -EINVAL; if (len & 3) return -EINVAL; mutex_lock(&user->file_lock); + /* Can we use the pre-allocate buffer ? If not, allocate */ + if (len <= PAGE_SIZE) + user->pending_cmd = user->cmd_page; + else + user->pending_cmd = vmalloc(len); + if (!user->pending_cmd) { + rc = -ENOMEM; + goto bail; + } + /* Copy the command into the staging buffer */ if (copy_from_user(user->pending_cmd, buf, len)) { rc = -EFAULT; @@ -906,6 +927,9 @@ static ssize_t sbefifo_user_write(struct file *file, const char __user *buf, /* Update the staging buffer size */ user->pending_len = len; bail: + if (!user->pending_len) + sbefifo_release_command(user); + mutex_unlock(&user->file_lock); /* And that's it, we'll issue the command on a read */ @@ -919,7 +943,8 @@ static int sbefifo_user_release(struct inode *inode, struct file *file) if (!user) return -EINVAL; - free_page((unsigned long)user->pending_cmd); + sbefifo_release_command(user); + free_page((unsigned long)user->cmd_page); kfree(user); return 0;
Otherwise cronus putmem fails istep and BML fails to upload skiboot To do that, we still use our one-page command buffer for small commands for speed, and for anything bigger, with a limit of 1MB plus a page, we vmalloc a temporary buffer. The limit was chosen because Cronus will break up any data transfer into 1M chunks (the extra page is for the command header). Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org> --- v3. Factor the buffer cleanup code into a helper function v2. Reduce max to 1MB + PAGE_SIZE which is enough for Cronus as it will break up transfers in 1MB chunks and we can make sure pdbg does the same. drivers/fsi/fsi-sbefifo.c | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-)