[Skiboot] [PATCH v2 1/5] libflash/ipmi-hiomap: Fix blocks count issue

[Andrew Jeffery]

On Mon, 8 Apr 2019, at 15:35, Vasant Hegde wrote:
> We convert data size to block count and pass block count to BMC.
> If data size is not block aligned then we endup sending block count
> less than actual data. BMC will write partial data to flash memory.
> 
> Sample log :
> [  594.388458416,7] HIOMAP: Marked flash dirty at 0x42010 for 8
> [  594.398756487,7] HIOMAP: Flushed writes
> [  594.409596439,7] HIOMAP: Marked flash dirty at 0x42018 for 3970
> [  594.419897507,7] HIOMAP: Flushed writes
> 
> In this case HIOMAP sent data with block count=0 and hence BMC didn't
> flush data to flash.
> 
> Lets fix this issue by adjusting block count before sending it to BMC.
> 
> Cc: Andrew Jeffery <andrew at aj.id.au>
> Cc: skiboot-stable at lists.ozlabs.org
> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>

Reviewed-by: Andrew Jeffery <andrew at aj.id.au>

Thanks for cleaning up my mess :)

> ---
>  libflash/ipmi-hiomap.c | 21 ++++++++++++++++++---
>  1 file changed, 18 insertions(+), 3 deletions(-)
> 
> diff --git a/libflash/ipmi-hiomap.c b/libflash/ipmi-hiomap.c
> index 56492fa87..9174eabd2 100644
> --- a/libflash/ipmi-hiomap.c
> +++ b/libflash/ipmi-hiomap.c
> @@ -49,6 +49,21 @@ static inline uint16_t bytes_to_blocks(struct 
> ipmi_hiomap *ctx, uint32_t bytes)
>  	return bytes >> ctx->block_size_shift;
>  }
>  
> +static inline uint16_t bytes_to_blocks_align_up(struct ipmi_hiomap 
> *ctx,
> +						uint32_t pos, uint32_t len)
> +{
> +	uint32_t block_size = 1 << ctx->block_size_shift;
> +	uint32_t delta = pos & (block_size - 1);
> +	uint32_t aligned = ALIGN_UP((len + delta), block_size);
> +	uint32_t blocks = aligned >> ctx->block_size_shift;
> +	/* Our protocol can handle block count < sizeof(u16) */
> +	uint32_t mask = ((1 << 16) - 1);
> +
> +	assert(!(blocks & ~mask));
> +
> +	return blocks & mask;
> +}
> +
>  /* Call under ctx->lock */
>  static int hiomap_protocol_ready(struct ipmi_hiomap *ctx)
>  {
> @@ -321,7 +336,7 @@ static int hiomap_window_move(struct ipmi_hiomap 
> *ctx, uint8_t command,
>  
>  	range = (struct hiomap_v2_range *)&req[2];
>  	range->offset = cpu_to_le16(bytes_to_blocks(ctx, pos));
> -	range->size = cpu_to_le16(bytes_to_blocks(ctx, len));
> +	range->size = cpu_to_le16(bytes_to_blocks_align_up(ctx, pos, len));
>  
>  	msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
>  		         bmc_platform->sw->ipmi_oem_hiomap_cmd,
> @@ -384,7 +399,7 @@ static int hiomap_mark_dirty(struct ipmi_hiomap 
> *ctx, uint64_t offset,
>  	pos = offset - ctx->current.cur_pos;
>  	range = (struct hiomap_v2_range *)&req[2];
>  	range->offset = cpu_to_le16(bytes_to_blocks(ctx, pos));
> -	range->size = cpu_to_le16(bytes_to_blocks(ctx, size));
> +	range->size = cpu_to_le16(bytes_to_blocks_align_up(ctx, pos, size));
>  
>  	msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
>  		         bmc_platform->sw->ipmi_oem_hiomap_cmd,
> @@ -494,7 +509,7 @@ static int hiomap_erase(struct ipmi_hiomap *ctx, 
> uint64_t offset,
>  	pos = offset - ctx->current.cur_pos;
>  	range = (struct hiomap_v2_range *)&req[2];
>  	range->offset = cpu_to_le16(bytes_to_blocks(ctx, pos));
> -	range->size = cpu_to_le16(bytes_to_blocks(ctx, size));
> +	range->size = cpu_to_le16(bytes_to_blocks_align_up(ctx, pos, size));
>  
>  	msg = ipmi_mkmsg(IPMI_DEFAULT_INTERFACE,
>  		         bmc_platform->sw->ipmi_oem_hiomap_cmd,
> -- 
> 2.14.3
> 
>