[ipxe-devel] eepro100 vs qemu

[Michael Brown]

On Tuesday 15 Mar 2011 03:00:34 Alex Williamson wrote:
> Even better, thanks for fixing it!  I'll do some more testing, but I
> think that's the last regression.  The only nit I have is that pcnet32
> doesn't work with more than 2048MB in the guest (maybe a signed int
> somewhere?)

Found it.  It is indeed caused by an erroneous signed int, but it's in qemu 
rather than in iPXE.  The problem is with definitions in hw/pcnet.c such as:

  #define CSR_CRDA(S)      ((S)->csr[28] | ((S)->csr[29] << 16))

"(S)->csr[29]" is a uint16_t, but "(S)->csr[29] << 16" gets promoted to int, 
so the overall CSR_CRDA(s) is a (signed) int rather than a uint32_t.

This then gets assigned to a uint64_t using

  target_phys_addr_t crda = CSR_CRDA(s);

so when (S)->csr[29] has the high bit set, we end up with crda=0xffffffffxxxxxxxx.

Somewhat perniciously, this isn't immediately visible from the debug output in 
pcnet.c, which uses CSR_CRDA(s) directly in a printf() statement, rather than 
printing the value of the supposedly equivalent variable "crda".  The printf() 
therefore shows the expected 32-bit value, because the argument never gets 
promoted to uint64_t.

Attached is a patch to qemu that fixes this problem by casting to uint32_t 
before applying the "<<16".  (It also fixes what looks like a currently-
harmless typo in the definition of PHYSADDR().)  Will you push this patch 
upstream, or do you want me to send it to the qemu list?

Michael
-------------- next part --------------
A non-text attachment was scrubbed...
Name: qemu-pcnet-2gb.patch
Type: text/x-patch
Size: 2388 bytes
Desc: not available
URL: <http://lists.ipxe.org/pipermail/ipxe-devel/attachments/20110315/31718bf2/attachment.bin>