[Skiboot] [PATCH v2] libflash/file: Only use 64bit MTD erase ioctl() when needed
Alistair Popple
alistair at popple.id.au
Thu Apr 20 15:52:54 AEST 2017
This looks reasonable to me. I'm guessing there is no workaround for
doing a 64-bit MTD erase on kernels that don't support it so just
returning an error seems ok to me.
Acked-By: Alistair Popple <alistair at popple.id.au>
On Thu, 20 Apr 2017 03:39:44 PM Cyril Bur wrote:
> We recently made MTD 64 bit safe in e5720d3fe94 which now requires the
> 64 bit MTD erase ioctl. Unfortunately this ioctl is not present in
> older kernels used by some BMC vendors that use pflash.
>
> This patch addresses this by only using the 64bit version of the erase
> ioctl() if the parameters exceed 32bit in size.
>
> If an erase requires the 64bit ioctl() on a kernel which does not
> support it, the code will still attempt it. There is no way of knowing
> beforehand if the kernel supports it. The ioctl() will fail and an error
> will be returned from from the function.
>
> Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> ---
> V2: Reword this commit message
> Dropped the test, on reflection it isn't a great test. In that it
> doesn't really test much.
>
> libflash/file.c | 44 ++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/libflash/file.c b/libflash/file.c
> index 5f074cf2..8d1ed02a 100644
> --- a/libflash/file.c
> +++ b/libflash/file.c
> @@ -15,6 +15,7 @@
> */
> #define _GNU_SOURCE
> #include <errno.h>
> +#include <inttypes.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> @@ -130,13 +131,44 @@ static int file_erase(struct blocklevel_device *bl, uint64_t dst, uint64_t len)
> static int mtd_erase(struct blocklevel_device *bl, uint64_t dst, uint64_t len)
> {
> struct file_data *file_data = container_of(bl, struct file_data, bl);
> - struct erase_info_user64 erase_info = {
> - .start = dst,
> - .length = len
> - };
> + int err;
>
> - if (ioctl(file_data->fd, MEMERASE64, &erase_info) == -1)
> - return FLASH_ERR_PARM_ERROR;
> + FL_DBG("%s: dst: 0x%" PRIx64 ", len: 0x%" PRIx64 "\n", __func__, dst, len);
> +
> + /*
> + * Some kernels that pflash supports do not know about the 64bit
> + * version of the ioctl() therefore we'll just use the 32bit (which
> + * should always be supported...) unless we MUST use the 64bit and
> + * then lets just hope the kernel knows how to deal with it. If it
> + * is unsupported the ioctl() will fail and we'll report that -
> + * there is no other option.
> + */
> + if (dst > UINT_MAX || len > UINT_MAX) {
> + struct erase_info_user64 erase_info = {
> + .start = dst,
> + .length = len
> + };
> +
> + if (ioctl(file_data->fd, MEMERASE64, &erase_info) == -1) {
> + err = errno;
> + if (err == 25) /* Kernel doesn't do 64bit MTD erase ioctl() */
> + FL_DBG("Attempted a 64bit erase on a kernel which doesn't support it\n");
> + FL_ERR("%s: IOCTL to kernel failed! %s\n", __func__, strerror(err));
> + errno = err;
> + return FLASH_ERR_PARM_ERROR;
> + }
> + } else {
> + struct erase_info_user erase_info = {
> + .start = dst,
> + .length = len
> + };
> + if (ioctl(file_data->fd, MEMERASE, &erase_info) == -1) {
> + err = errno;
> + FL_ERR("%s: IOCTL to kernel failed! %s\n", __func__, strerror(err));
> + errno = err;
> + return FLASH_ERR_PARM_ERROR;
> + }
> + }
>
> return 0;
> }
>
More information about the Skiboot
mailing list