| Message ID | 20170419041143.12042-2-cyril.bur@au1.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2017-04-19 at 14:11 +1000, Cyril Bur wrote: > Commit e5720d3fe94f5b85c374520b89fb8fbb2143df94 introduced using the > 64bit MTD erase ioctl() after a rework of blocklevel to make it fully > 64bit safe. It turns out that not all kernels on which pflash might run > actually have the 64bit MTD ioctl(), this is probably due to the kernel > being based off 2.6.28.10. > > This patch addresses this by only using the 64bit version of the erase > ioctl() if the parameters exceed 32bit in size. > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > --- > libflash/file.c | 35 +++++++++++++++++++++++++++++------ > 1 file changed, 29 insertions(+), 6 deletions(-) > > diff --git a/libflash/file.c b/libflash/file.c > index 5f074cf2..a4d4fe10 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,35 @@ 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 > - }; > > - 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 (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) > + 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) { > + if (errno == 25) /* Kernel doesn't do 64 MTD 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(errno)); > + return FLASH_ERR_PARM_ERROR; This error should be after the MEMERASE_64 ioctl right, not the 32 bit one? > + } > + } > > return 0; > }
On Wed, 2017-04-19 at 14:34 +1000, Samuel Mendoza-Jonas wrote: > On Wed, 2017-04-19 at 14:11 +1000, Cyril Bur wrote: > > Commit e5720d3fe94f5b85c374520b89fb8fbb2143df94 introduced using the > > 64bit MTD erase ioctl() after a rework of blocklevel to make it fully > > 64bit safe. It turns out that not all kernels on which pflash might run > > actually have the 64bit MTD ioctl(), this is probably due to the kernel > > being based off 2.6.28.10. > > > > This patch addresses this by only using the 64bit version of the erase > > ioctl() if the parameters exceed 32bit in size. > > > > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> > > --- > > libflash/file.c | 35 +++++++++++++++++++++++++++++------ > > 1 file changed, 29 insertions(+), 6 deletions(-) > > > > diff --git a/libflash/file.c b/libflash/file.c > > index 5f074cf2..a4d4fe10 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,35 @@ 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 > > - }; > > > > - 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 (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) > > + 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) { > > + if (errno == 25) /* Kernel doesn't do 64 MTD 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(errno)); > > + return FLASH_ERR_PARM_ERROR; > > This error should be after the MEMERASE_64 ioctl right, not the 32 bit > one? Oops, yes. Thanks! > > > + } > > + } > > > > return 0; > > } > > _______________________________________________ > Skiboot mailing list > Skiboot@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/skiboot
diff --git a/libflash/file.c b/libflash/file.c index 5f074cf2..a4d4fe10 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,35 @@ 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 - }; - 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 (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) + 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) { + if (errno == 25) /* Kernel doesn't do 64 MTD 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(errno)); + return FLASH_ERR_PARM_ERROR; + } + } return 0; }
Commit e5720d3fe94f5b85c374520b89fb8fbb2143df94 introduced using the 64bit MTD erase ioctl() after a rework of blocklevel to make it fully 64bit safe. It turns out that not all kernels on which pflash might run actually have the 64bit MTD ioctl(), this is probably due to the kernel being based off 2.6.28.10. This patch addresses this by only using the 64bit version of the erase ioctl() if the parameters exceed 32bit in size. Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com> --- libflash/file.c | 35 +++++++++++++++++++++++++++++------ 1 file changed, 29 insertions(+), 6 deletions(-)