filesystem-littlefs Greentea test failed after CRC revision #11559 merged

[jamesbeyond]

Description of defect

following filesystem-littlefs Greentea test start to failing after CRC revision #11559 merged into master

features-storage-filesystem-littlefs-tests-filesystem-dirs
features-storage-filesystem-littlefs-tests-filesystem-files
features-storage-filesystem-littlefs-tests-filesystem-interspersed
features-storage-filesystem-littlefs-tests-filesystem-seek
features-storage-filesystem-littlefs-tests-filesystem_retarget-dirs
features-storage-filesystem-littlefs-tests-filesystem_integration-format
features-storage-filesystem-littlefs-tests-filesystem_retarget-interspersed
features-storage-filesystem-littlefs-tests-filesystem_retarget-files

@kjbracey-arm @0xc0170

Target(s) affected by this defect ?

filesystem-littlefs test rather generic , no specific to any target. but can be reproduced on K64F

Toolchain(s) (name and version) displaying this defect ?

GCC_ARM

What version of Mbed-os are you using (tag or sha) ?

539779f (passing)
fd22997 (failed)

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

mbed-cli

How is this defect reproduced ?

You need to K64F with SD card inserted
run :

mbed test -t GCC_ARM -m K64F -n features-storage-filesystem-littlefs-tests-filesystem-seek -DMBED_TEST_BLOCKDEVICE=SDBlockDevice '-DMBED_TEST_BLOCKDEVICE_DECL=SDBlockDevice bd(MBSPI_MISO, MBED_CONF_SD_SPI_CLK, MBED_CONF_SD_SPI_CS)'

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2355

[kjbracey]

Ta for report - will investigate.

[kjbracey]

Ick.

LittleFS is making a chain of start/compute_partial/stop;start/compute_partial/stop calls, feeding final CRC result from each stop into the initial value of the next start.

It uses the conventional reversed CRC-32, so asks MbedCRC for reflected output. But that reflected output is the wrong way round for the next start. MbedCRC (as is conventional) always expects a non-reflected init value, regardless of the other reflect settings. Whereas LFS's fallback software lfs_crc expects a reflected init value, so it's fine.

There's also a test coverage hole here too - no software or HAL CRC test checks reflection handling of the init value - all tests with reflection use all-0 or all-1 inits.

Simplistic fix would be to have uint32_t initial_xor = __RBIT(*crc) in lfs_crc, although it's a bit dopey as it ends up just assembling two RBITs in a row.

Alternatively could have a constructor/template parameter to indicate "reversed init", but don't really fancy adding more complexity to MbedCRC.h.

[0xc0170]

Is this affecting current master ? As we already have two new PRs dependent on the fix for this one.

I can't find these in nightly.

[kjbracey]

Is this affecting current master ?

Yes - so there's no LittleFS testing on PRs, and none on master?

[kjbracey]

I'll do the simplistic fix of adding an "RBIT" in LittleFS - LittleFS is already super clunky because it's not using the compute_partial call elegantly, so adding one RBIT doesn't make much difference.

Mind you, unlike GCC, ARMC6 at can spot and eliminate the double reversal - adding an "RBIT" in LittleFS actually cancels out one in MbedCRC and reduces the code size.

I'll need to polish up a bit to make that portable for unit tests, and also I want to add some init reversal tests to CRC/HAL greentea. I'm not 100% confident hardware CRC is matching software from the current coverage.

(Although LittleFS explicitly requests software CRC anyway, because its 1-byte-at-a-time CRC would be super inefficient for hardware acceleration).