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
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; 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.Tue, Apr 20, 7:23 PM
This revision was automatically updated to reflect the committed changes.