| Message ID | 0A54BACCF270364FBD51EAC504A0F0670F8880EA@FRMRSSXCHMBSC2.dc-m.alcatel-lucent.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, Aug 19, 2009 at 11:38:21AM +0200, ZAPPACOSTA, Rolando (Rolando) wrote: > I was able to make VxWorks 5.4.2 load from FTP it's target binary file (I'll now try to figure out why I receive some VxWorks errors once it finishes the download). > > To do so, I had to implement some 8255x features not yet included as well as to change some of the already coded ones. Some of your changes are basically "cosmetic", e.g. replacing a single assert by if+message+assert(0), could you please point out which parts are really necessary? Btw. I already some time ago sent a patch that implements CmdDiagnose (also sets status to 0), which is also necessary for OpenSolaris (which also needs RX_ABORT support in addition). I find it hard to imagine that the reads you implemented are necessary, since they would be unaligned which I for some reason believe aren't allowed and/or in real hardware get split by the PCI implementation or something...
Hi Reimar,
yes, you are correct, some of them could be considered cosmetic (I added them initially for a better tracking of the emulation each time I launched it).
The ones really important are:
1)
@@ -257,6 +266,8 @@
/* Temporary data. */
eepro100_tx_t tx;
uint32_t cb_address;
+ /* used to store the partial values of the pointer before calling eepro100_write_port */
+ uint16_t port_lo_word;
/* Statistical counters. */
eepro100_stats_t statistics;
===========> REASON: because VxWorks access it as two dwords and the emulation fails if this is not implemented.
2)
@@ -782,27 +793,26 @@
TRACE(RXTX, logout
("transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD count %u\n",
tbd_array, tcb_bytes, s->tx.tbd_count));
- assert(!(s->tx.command & COMMAND_NC));
- assert(tcb_bytes <= sizeof(buf));
+ if (s->tx.command & COMMAND_NC) {
+ logout("support for NC=1 is not implemented\n");
+ assert (0);
+ }
+ if (tcb_bytes > sizeof(buf)) {
+ logout("illegal value of the TCB byte count! (cannot be greater than 0x%04x)\n", sizeof(buf));
+ assert (0);
+ }
if (!((tcb_bytes > 0) || (tbd_array != 0xffffffff))) {
logout
("illegal values of TBD array address and TCB byte count!\n");
}
- for (size = 0; size < tcb_bytes; ) {
- uint32_t tx_buffer_address = ldl_le_phys(tbd_address);
- uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4);
- //~ uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6);
- tbd_address += 8;
- TRACE(RXTX, logout
- ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
- tx_buffer_address, tx_buffer_size));
- assert(size + tx_buffer_size <= sizeof(buf));
- cpu_physical_memory_read(tx_buffer_address, &buf[size],
- tx_buffer_size);
- size += tx_buffer_size;
+ if (tcb_bytes > 0) {
+ TRACE(RXTX, logout("TCB byte count>0, adding the data after the TCB to the buffer\n"));
+ cpu_physical_memory_read(s->cb_address + 0x10, &buf[0], tcb_bytes);
+ size = tcb_bytes;
}
if (!(s->tx.command & COMMAND_SF)) {
/* Simplified mode. Was already handled by code above. */
+ TRACE(RXTX, logout("Simplified Mode, no TBDs have to be processed\n"));
if (tbd_array != 0xffffffff) {
UNEXPECTED();
}
===========> REASON: because VxWorks, at least on what I run, doesn't use TBDs at all, just puts the data to be sent out by the emulation in memory (following the TCB). In my humble opinion, this wasn't handled properly by the original code but your comments are, of course, more than welcomed.
3)
@@ -918,6 +931,9 @@
/* Starting with offset 8, the command contains
* 64 dwords microcode which we just ignore here. */
break;
+ case CmdDiagnose:
+ TRACE(OTHER, logout(" Rolando: diagnose\n"));
+ break;
default:
missing("undefined command");
}
===========> REASON: even though diagnose is called by VxWorks, it should work without this (I realized that later).
4)
@@ -1079,9 +1095,9 @@
return val;
}
-static void eepro100_write_eeprom(eeprom_t * eeprom, uint8_t val)
+static void eepro100_write_eeprom(eeprom_t * eeprom, uint32_t val)
{
- TRACE(OTHER, logout("val=0x%02x\n", val));
+ TRACE(OTHER, logout("val=0x%08x\n", val));
/* mask unwriteable bits */
//~ val = SET_MASKED(val, 0x31, eeprom->value);
===========> REASON: I don't remember exactly what part was requiring this but my emulation failed without this change. If interested, I could change this back and tell you what fails.
5)
@@ -1129,7 +1145,7 @@
if (reg < ARRAY_SIZE(mdi_reg_name)) {
p = mdi_reg_name[reg];
} else {
- snprintf(buffer, sizeof(buf), "reg=0x%02x", reg);
+ snprintf(buffer, sizeof(buffer), "reg=0x%02x", reg);
}
return p;
}
===========> REASON: got an overflow each time I tried to trace the run otherwise.
6)
@@ -1484,6 +1504,23 @@
case SCBeeprom:
eepro100_write_eeprom(s->eeprom, val);
break;
+ case SCBPointer:
+ s->pointer = (s->pointer & 0xffff0000) + val;
+ logout(" Rolando: wrote the General Pointer, it's now 0x%08x\n", s->pointer);
+ break;
+ case SCBPointer+2:
+ s->pointer = (s->pointer & 0xffff) + ( val * 0x10000 );
+ logout(" Rolando: wrote the General Pointer, it's now 0x%08x\n", s->pointer);
+ break;
+ case SCBPort:
+ s->port_lo_word = val;
+ logout(" Rolando: stored the low-word of the Port Interface: 0x%04x\n", s->port_lo_word);
+ break;
+ case SCBPort+2:
+ val = s->port_lo_word + val*0x10000;
+ logout(" Rolando: got the hi-word of the Port Interface, calling it. The command is 0x%08x\n", val);
+ eepro100_write_port(s, val);
+ break;
default:
logout("addr=%s val=0x%04x\n", regname(addr), val);
missing("unknown word write");
===========> REASON: same than for 1 above, VxWorks writes them as two words instead of as just one single Dword.
HTH and please don't hesitate to give me your feedback.
Rolando.
I am only looking at this because I hacked this code, not because I have anything to say on whether it gets in... On Sat, Aug 22, 2009 at 06:42:55PM +0200, ZAPPACOSTA, Rolando (Rolando) wrote: > yes, you are correct, some of them could be considered cosmetic (I added them initially for a better tracking of the emulation each time I launched it). > 1) > @@ -257,6 +266,8 @@ > /* Temporary data. */ > eepro100_tx_t tx; > uint32_t cb_address; > + /* used to store the partial values of the pointer before calling eepro100_write_port */ > + uint16_t port_lo_word; > > /* Statistical counters. */ > eepro100_stats_t statistics; > ===========> REASON: because VxWorks access it as two dwords and the emulation fails if this is not implemented. I don't really understand that, the values always already gets stored to and restored from ->mem... > @@ -782,27 +793,26 @@ > TRACE(RXTX, logout > ("transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD count %u\n", > tbd_array, tcb_bytes, s->tx.tbd_count)); > - assert(!(s->tx.command & COMMAND_NC)); > - assert(tcb_bytes <= sizeof(buf)); > + if (s->tx.command & COMMAND_NC) { > + logout("support for NC=1 is not implemented\n"); > + assert (0); > + } > + if (tcb_bytes > sizeof(buf)) { > + logout("illegal value of the TCB byte count! (cannot be greater than 0x%04x)\n", sizeof(buf)); > + assert (0); > + } That part I meant with cosmetics, I think it would simplify things to do it at best in a different patch. Btw. there is a huge amount of trailing whitespace after that } > if (!((tcb_bytes > 0) || (tbd_array != 0xffffffff))) { > logout > ("illegal values of TBD array address and TCB byte count!\n"); > } > - for (size = 0; size < tcb_bytes; ) { > - uint32_t tx_buffer_address = ldl_le_phys(tbd_address); > - uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4); > - //~ uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6); > - tbd_address += 8; > - TRACE(RXTX, logout > - ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n", > - tx_buffer_address, tx_buffer_size)); > - assert(size + tx_buffer_size <= sizeof(buf)); > - cpu_physical_memory_read(tx_buffer_address, &buf[size], > - tx_buffer_size); > - size += tx_buffer_size; > + if (tcb_bytes > 0) { > + TRACE(RXTX, logout("TCB byte count>0, adding the data after the TCB to the buffer\n")); > + cpu_physical_memory_read(s->cb_address + 0x10, &buf[0], tcb_bytes); Lots of trailing whitespace here too. And s->cb_address + 0x10 was already calulated before and stored in tbd_address. Also if extended TCB is enabled I think it must be + 0x20. I admit that the Intel specification says differently but that does not make any sense (I suspect they just did not consider that case)... Apart from that I think this is correct, reading the specification. > @@ -918,6 +931,9 @@ > /* Starting with offset 8, the command contains > * 64 dwords microcode which we just ignore here. */ > break; > + case CmdDiagnose: > + TRACE(OTHER, logout(" Rolando: diagnose\n")); > + break; > default: > missing("undefined command"); > } > ===========> REASON: even though diagnose is called by VxWorks, it should work without this (I realized that later). No, it will crash with an assert due to the missing(...), as said I already sent a patch myself (that sets status to 0). > 4) > @@ -1079,9 +1095,9 @@ > return val; > } > > -static void eepro100_write_eeprom(eeprom_t * eeprom, uint8_t val) > +static void eepro100_write_eeprom(eeprom_t * eeprom, uint32_t val) > { > - TRACE(OTHER, logout("val=0x%02x\n", val)); > + TRACE(OTHER, logout("val=0x%08x\n", val)); > > /* mask unwriteable bits */ > //~ val = SET_MASKED(val, 0x31, eeprom->value); > ===========> REASON: I don't remember exactly what part was requiring this but my emulation failed without this change. If interested, I could change this back and tell you what fails. Except for a compiler bug I can't see how it could make a difference... > 6) > @@ -1484,6 +1504,23 @@ > case SCBeeprom: > eepro100_write_eeprom(s->eeprom, val); > break; > + case SCBPointer: > + s->pointer = (s->pointer & 0xffff0000) + val; > + logout(" Rolando: wrote the General Pointer, it's now 0x%08x\n", s->pointer); > + break; > + case SCBPointer+2: > + s->pointer = (s->pointer & 0xffff) + ( val * 0x10000 ); > + logout(" Rolando: wrote the General Pointer, it's now 0x%08x\n", s->pointer); > + break; I guess reasonable in principle (though it might be simpler to get rid of the pointer variable and just read it whenever necessary form s->mem), except that you need to use le16_to_cpu on val I think. (val << 16) seems better to me than (val * 0x10000).
--- hw/eepro100.c.original 2009-08-13 09:41:52.000000000 +0200 +++ hw/eepro100.c 2009-08-19 11:13:50.000000000 +0200 @@ -25,6 +25,14 @@ * Linux networking e100 driver (mips, mipsel) ok * Linux networking eepro100 driver (mipsel) not ok * Windows networking (Vista) ??? + * VxWorks 5.4.2 (i386) ok, tested with 82557b: succesfuly loads + * the target file when limiting the download rate of + * the FTP server to ~5KB/s. The target used to test it + * then hangs with these errors: + * 0x00000633 + * 0x00000800 + * 0x00000001 + * 0x0781ABFD * * Untested: * non-i386 platforms @@ -39,6 +47,7 @@ * EE100 eepro100_write2 feature is missing in this emulation: unknown word write * EE100 eepro100_read2 addr=General Status/Control+2 val=0x0080 * EE100 eepro100_read2 feature is missing in this emulation: unknown word read + * Rolando: action_command => case CmdDiagnose: should make sure bit F gets reseted */ #include <assert.h> @@ -257,6 +266,8 @@ typedef struct { /* Temporary data. */ eepro100_tx_t tx; uint32_t cb_address; + /* used to store the partial values of the pointer before calling eepro100_write_port */ + uint16_t port_lo_word; /* Statistical counters. */ eepro100_stats_t statistics; @@ -374,8 +385,8 @@ static const char *nic_dump(const uint8_ { static char dump[3 * 16 + 1]; char *p = &dump[0]; - if (size > 16) - size = 16; + if (size > 2600) + size = 2600; while (size-- > 0) { p += sprintf(p, " %02x", *buf++); } @@ -661,7 +672,7 @@ static const char *e100_reg[PCI_IO_SIZE static char *regname(uint32_t addr) { - static char buf[16]; + static char buf[26]; if (addr < PCI_IO_SIZE) { const char *r = e100_reg[addr / 4]; if (r != 0) { @@ -782,27 +793,30 @@ static void tx_command(EEPRO100State *s) TRACE(RXTX, logout ("transmit, TBD array address 0x%08x, TCB byte count 0x%04x, TBD count %u\n", tbd_array, tcb_bytes, s->tx.tbd_count)); - assert(!(s->tx.command & COMMAND_NC)); - assert(tcb_bytes <= sizeof(buf)); + + if (s->tx.command & COMMAND_NC) { + logout("support for NC=1 is not implemented\n"); + assert (0); + } + + if (tcb_bytes > sizeof(buf)) { + logout("illegal value of the TCB byte count! (cannot be greater than 0x%04x)\n", sizeof(buf)); + assert (0); + } + if (!((tcb_bytes > 0) || (tbd_array != 0xffffffff))) { logout ("illegal values of TBD array address and TCB byte count!\n"); } - for (size = 0; size < tcb_bytes; ) { - uint32_t tx_buffer_address = ldl_le_phys(tbd_address); - uint16_t tx_buffer_size = lduw_le_phys(tbd_address + 4); - //~ uint16_t tx_buffer_el = lduw_le_phys(tbd_address + 6); - tbd_address += 8; - TRACE(RXTX, logout - ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n", - tx_buffer_address, tx_buffer_size)); - assert(size + tx_buffer_size <= sizeof(buf)); - cpu_physical_memory_read(tx_buffer_address, &buf[size], - tx_buffer_size); - size += tx_buffer_size; + + if (tcb_bytes > 0) { + TRACE(RXTX, logout("TCB byte count>0, adding the data after the TCB to the buffer\n")); + cpu_physical_memory_read(s->cb_address + 0x10, &buf[0], tcb_bytes); + size = tcb_bytes; } if (!(s->tx.command & COMMAND_SF)) { /* Simplified mode. Was already handled by code above. */ + TRACE(RXTX, logout("Simplified Mode, no TBDs have to be processed\n")); if (tbd_array != 0xffffffff) { UNEXPECTED(); } @@ -896,16 +910,18 @@ static void action_command(EEPRO100State switch (s->tx.command & COMMAND_CMD) { case CmdNOp: /* Do nothing. */ + TRACE(OTHER, logout(" Rolando: NOP\n")); break; case CmdIASetup: cpu_physical_memory_read(s->cb_address + 8, &s->macaddr[0], 6); - TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->macaddr[0], 6))); + TRACE(OTHER, logout("initial addr setup, macaddr: %s\n", nic_dump(&s->macaddr[0], 6))); // !!! missing break; case CmdConfigure: cpu_physical_memory_read(s->cb_address + 8, &s->configuration[0], sizeof(s->configuration)); - TRACE(OTHER, logout("configuration: %s\n", nic_dump(&s->configuration[0], 16))); + TRACE(OTHER, logout("configuration: %s\n", nic_dump(&s->configuration[0], sizeof(s->configuration)))); + TRACE(OTHER, logout(" Rolando: NSAI bit=0x%01x (1=SA comes from memory, 0=SA comes from internal device IA)\n", (s->configuration[10] & 4)/4)); break; case CmdMulticastList: set_multicast_list(s); @@ -918,6 +934,9 @@ static void action_command(EEPRO100State /* Starting with offset 8, the command contains * 64 dwords microcode which we just ignore here. */ break; + case CmdDiagnose: + TRACE(OTHER, logout(" Rolando: diagnose\n")); + break; default: missing("undefined command"); } @@ -1079,9 +1098,9 @@ static uint16_t eepro100_read_eeprom(EEP return val; } -static void eepro100_write_eeprom(eeprom_t * eeprom, uint8_t val) +static void eepro100_write_eeprom(eeprom_t * eeprom, uint32_t val) { - TRACE(OTHER, logout("val=0x%02x\n", val)); + TRACE(OTHER, logout("val=0x%08x\n", val)); /* mask unwriteable bits */ //~ val = SET_MASKED(val, 0x31, eeprom->value); @@ -1129,7 +1148,7 @@ static const char *reg2name(uint8_t reg) if (reg < ARRAY_SIZE(mdi_reg_name)) { p = mdi_reg_name[reg]; } else { - snprintf(buffer, sizeof(buf), "reg=0x%02x", reg); + snprintf(buffer, sizeof(buffer), "reg=0x%02x", reg); } return p; } @@ -1364,12 +1383,16 @@ static uint16_t eepro100_read2(EEPRO100S switch (addr) { case SCBStatus: //~ val = eepro100_read_status(s); - TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val)); + TRACE(OTHER, logout(" Rolando: read the Status word, addr=%s val=0x%04x\n", regname(addr), val)); break; case SCBeeprom: val = eepro100_read_eeprom(s); TRACE(OTHER, logout("addr=%s val=0x%04x\n", regname(addr), val)); break; + case SCBCmd: + // val = eepro100_read_command(s); + TRACE(OTHER, logout(" Rolando: read the Command word, addr=%s val=0x%04x\n", regname(addr), val)); + break; default: logout("addr=%s val=0x%04x\n", regname(addr), val); missing("unknown word read"); @@ -1484,6 +1507,23 @@ static void eepro100_write2(EEPRO100Stat case SCBeeprom: eepro100_write_eeprom(s->eeprom, val); break; + case SCBPointer: + s->pointer = (s->pointer & 0xffff0000) + val; + logout(" Rolando: wrote the General Pointer, it's now 0x%08x\n", s->pointer); + break; + case SCBPointer+2: + s->pointer = (s->pointer & 0xffff) + ( val * 0x10000 ); + logout(" Rolando: wrote the General Pointer, it's now 0x%08x\n", s->pointer); + break; + case SCBPort: + s->port_lo_word = val; + logout(" Rolando: stored the low-word of the Port Interface: 0x%04x\n", s->port_lo_word); + break; + case SCBPort+2: + val = s->port_lo_word + val*0x10000; + logout(" Rolando: got the hi-word of the Port Interface, calling it. The command is 0x%08x\n", val); + eepro100_write_port(s, val); + break; default: logout("addr=%s val=0x%04x\n", regname(addr), val); missing("unknown word write");