[Skiboot] [PATCH v2 1/5] libflash/file: Only use 64bit MTD erase ioctl() when needed

[Cyril Bur]

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 at 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 at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot