libflash/mbox-flash: Fix bad WRITE_FLUSH parameter

Submitted by Cyril Bur on March 21, 2017, 3:03 a.m.

Details

Message ID 20170321030320.8062-1-cyril.bur@au1.ibm.com
State New
Headers show

Commit Message

Cyril Bur March 21, 2017, 3:03 a.m.
The position to flush from should be relative to the window not to the
entire flash.

"Args 0-1: Where within window as number of blocks"

This is actually quite nice as skiboot will always want to flush the
entire window, so from zero to size.

Fixes: 23fe769115c41e8859ce3d23dc75953bfb290f45
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
---
 libflash/mbox-flash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Cyril Bur March 24, 2017, 12:34 a.m.
I think Stewart was informed out of band but I'll say it here.

This patch should not be merged. This bug is known to exist by all
current implementations and changing the behaviour would break more
than it fixes.

Cyril

On Tue, 2017-03-21 at 14:03 +1100, Cyril Bur wrote:
> The position to flush from should be relative to the window not to the
> entire flash.
> 
> "Args 0-1: Where within window as number of blocks"
> 
> This is actually quite nice as skiboot will always want to flush the
> entire window, so from zero to size.
> 
> Fixes: 23fe769115c41e8859ce3d23dc75953bfb290f45
> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
> ---
>  libflash/mbox-flash.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c
> index 7bf731d0..61de4ff8 100644
> --- a/libflash/mbox-flash.c
> +++ b/libflash/mbox-flash.c
> @@ -342,7 +342,7 @@ static int mbox_flash_write(struct blocklevel_device *bl, uint64_t pos,
>  		 * without flushing entitles the BMC to throw away the
>  		 * data
>  		 */
> -		rc = mbox_flash_flush(mbox_flash, pos, size);
> +		rc = mbox_flash_flush(mbox_flash, 0, size);
>  		if (rc)
>  			return rc;
>
Oliver O'Halloran March 24, 2017, 12:40 a.m.
On Fri, Mar 24, 2017 at 11:34 AM, Cyril Bur <cyrilbur@gmail.com> wrote:
> I think Stewart was informed out of band but I'll say it here.
>
> This patch should not be merged. This bug is known to exist by all
> current implementations and changing the behaviour would break more
> than it fixes.
>
> Cyril
>
> On Tue, 2017-03-21 at 14:03 +1100, Cyril Bur wrote:
>> The position to flush from should be relative to the window not to the
>> entire flash.
>>
>> "Args 0-1: Where within window as number of blocks"
>>
>> This is actually quite nice as skiboot will always want to flush the
>> entire window, so from zero to size.

IS THIS V3???????????

>>
>> Fixes: 23fe769115c41e8859ce3d23dc75953bfb290f45
>> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
>> ---
>>  libflash/mbox-flash.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c
>> index 7bf731d0..61de4ff8 100644
>> --- a/libflash/mbox-flash.c
>> +++ b/libflash/mbox-flash.c
>> @@ -342,7 +342,7 @@ static int mbox_flash_write(struct blocklevel_device *bl, uint64_t pos,
>>                * without flushing entitles the BMC to throw away the
>>                * data
>>                */
>> -             rc = mbox_flash_flush(mbox_flash, pos, size);
>> +             rc = mbox_flash_flush(mbox_flash, 0, size);
>>               if (rc)
>>                       return rc;
>>
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot

Patch hide | download patch | download mbox

diff --git a/libflash/mbox-flash.c b/libflash/mbox-flash.c
index 7bf731d0..61de4ff8 100644
--- a/libflash/mbox-flash.c
+++ b/libflash/mbox-flash.c
@@ -342,7 +342,7 @@  static int mbox_flash_write(struct blocklevel_device *bl, uint64_t pos,
 		 * without flushing entitles the BMC to throw away the
 		 * data
 		 */
-		rc = mbox_flash_flush(mbox_flash, pos, size);
+		rc = mbox_flash_flush(mbox_flash, 0, size);
 		if (rc)
 			return rc;