Page MenuHomeFreeBSD

Fix mprutil endianness
ClosedPublic

Authored by afscoelho_gmail.com on Aug 12 2020, 1:39 PM.

Details

Summary

This patch fix endianness issues in the mprutil utility. Except for flash operations (firmware/bios save/update), other query operations do work when functioning with a mpr driver also fixed with proper endianness conversions (https://reviews.freebsd.org/D25785).

Flash operations are not fixed by this patch yet. Tested firmware version ( 16.00.01.00) do not respond to flash operations like for example MPI2_FUNCTION_FW_UPLOAD, giving a timeout.

Test Plan

Use the patched mpr driver version (https://reviews.freebsd.org/D25785).
Compile and execute mprutil, and execute for example:

$ mprutil show iocfacts

   MsgVersion: 2.5
    MsgLength: 17
     Function: 0x3
HeaderVersion: 50,00
    IOCNumber: 0

...

If you try to execute a flash operation, an error like the one bellow should appear :

mprutil flash save firmware firm.bin

mpr0: mpr_user_command: unsupported parameter or unsupported function in request (function = 0x12)
mprutil: Fail to save firmware

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This revision is now accepted and ready to land.Aug 20 2020, 11:45 AM
alfredo added a subscriber: scottl.

Looks good to me. @scottl , would you like to take a look on this patch as well?

@afscoelho_gmail.com, would you mind adding TODO section in usr.sbin/mpsutil/mpsutil.8 regarding flash operations not working on BE (timeout) ?
You can use D28475 as reference. It can be in a different review linked to this one (child revision).

Thank you for doing this. My main suggestion is to fix mps_read_extended_config_page so it returns IOCStatus already byte-swapped, which will eliminate a lot of the other diffs. I also recommend looking at mps_cmd.c and fixing code that is under #ifdef USE_MPT_IOCTLS as well as not

usr.sbin/mpsutil/mps_cmd.c
402

IOCStatus should be byte-swapped here and stored for return to the caller so it doesn't have to be byte swapped by the caller

456

IOCStatus should be byte-swapped here and stored for return to the caller so it doesn't have to be byte swapped by the caller

usr.sbin/mpsutil/mps_show.c
141

This, and many others, shouldn't need to be byte-swapped if mps_read_extended_config_page does the swapping itself.

scottl requested changes to this revision.Feb 9 2021, 2:52 PM
This revision now requires changes to proceed.Feb 9 2021, 2:52 PM

Fixes after review. Make mps_read_extended_config_page return IOCStatus already byte-swapped. Other missing swaps fixed. Updated man page with TODO about flash operations not working on BE.

Also reviewed by Sreekanth Reddy (by e-mail)

@scottl are you ok with latest changes?

This revision was not accepted when it landed; it landed in state Needs Review.Apr 20 2021, 7:23 PM
This revision was automatically updated to reflect the committed changes.