On 9 May 2018 at 04:46, Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > Hi, > > This series emerged after last Coverity scan and Peter suggestion in: > http://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg05046.html > > (3) "proper" implementation of CRC, so that an sd controller > can either (a) mark the SDRequest as "no CRC" and have > sd_req_crc_validate() always pass, or (b) mark the SDRequest > as having a CRC and have sd_req_crc_validate() actually > do the check which it currently stubs out with "return 0" > > - Coverity issues fixed > - new functions documented > - qtests added > > This series would be much smaller without qtests (less refactor and code > movement), but I feel more confident having them passing :)
This series isn't going in the direction I expected. We have two cases: (1) we model an SD controller which in hardware calculates its own checksums. Most of our SD controllers are like that. Since in QEMU there is no chance of data corruption between the controller and the card, there's no point in calculating a checksum by calling a function, and then checking that we get the same result a few function calls later when we call the exact same function in the SD card model. So we should just unconditionally say "no checksum provided" in the SDRequest struct the controller fills in, and then skip the check in the card model. (2) we model an SD controller which leaves the checksum calculation to guest software. The only one of these we have that I know of is milkymist-memcard. In this case, the checksum is calculated by guest software, and so we do want to check it in the SD card model. So the controller should fill in the CRC field in SDRequest, and set the flag in SDRequest to say "checksum provided". We don't need to provide a property on the device or the card objects to control this behaviour, for either case. I'm not clear why this patchset has removed the SDRequest struct in favour of a raw buffer, either: that makes it harder to just say "this request doesn't have a checksum you need to check". thanks -- PMM