27.08.2024 15:09, Mark Cave-Ayland wrote:
On 27/08/2024 11:02, Michael Tokarev wrote:
in many cases, <zlib.h> is only included for crc32 function,
and in some of them, there's a comment saying that, but in
a different way. In one place (hw/net/rtl8139.c), there was
another #include added between the comment and <zlib.h> include.
Make all such comments to be on the same line as #include, make
it consistent, and also add a few missing comments, including
hw/nvram/mac_nvram.c which uses adler32 instead.
...
//#define DEBUG_STELLARIS_ENET 1
For the hw/net devices there are separate net_crc32() and net_crc32_le() functions
from net/net.c which are intended for (most) network devices where the "standard"
polynomials are used.
In many hw/net files I touched in this patch, *both*
plain crc32() and qemu's net_crc32() are used.
For now I just marked the #include, nothing more, we
can finish the refactoring later if needs to be.
Speaking of crc32 from zlib, I don't really see a point
in re-implementing it in this context (it is re-implemented
in net/net.c:net_crc32(), with comment "XXX: optimize*).
Implementation from zlib is quite a good one. Not the best
possible but definitely not the worst and is better than
net_crc32().
(See also https://create.stephan-brumme.com/crc32/)
What we definitely *can* optimize is the two cases in
tcg (arm and loong iirc) - they have hardware isns for
crc32, but these operate in fixed 4 or 8-bytes integers,
and there, implementing a function in qemu would be
nice - not much code but significant speedup due to
fixed size of the argument.
I don't see any isse with using crc32 from zlib, since
zlib is used for other things anyway and is mandatory
dependency. In case of qemu-user binaries, even static
link, it is tiny (since only crc32 stuff is linked to),
but there it would be more interesting to have in-qemu
implementation for static-size isns.