Hello Jason,
+-- On Tue, 23 Feb 2016, Jason Wang wrote --+
| I mean with your patch, driver will only be allowed to set EN0_STOPPG
| before EN0_STARTPG. So if a driver want to set STARTPG first, the check
|
| + if (v < NE2000_PMEM_END && v < s->stop) {
|
| will prevent the driver from working correctly since s->stop is zero here.
Before drivers could start using NIC, it'll be initialised from its ROM,
right? Which would set the PSTART & PSTOP registers to the default values.
With '-net nic,model=ne2k_pci,vlan=0' I see,
s->start = 19456, s->stop = 32768
| > I think any attempts to define the ring buffer limits should reset
| > 'boundary' and 'curpag' registers to s->start(STARTPG). I wonder if a
| > driver should be allowed to fiddle with the ring buffers location inside
| > contorller's memory. It does not seem right.
|
| Well, I think we could not assume the behavior of a driver, especially
| consider it may be malicious.
Yes; That's why it'll help to keep drivers from fiddling with the ring
buffer dimensions. IIUC, there is an upper limit to where PSTOP could
point[1],
"In 8 bit mode the PSTOP register should not exceed to 0x60,
in 16 bit mode the PSTOP register should not exceed to 0x80"
[1] http://www.ethernut.de/pdf/8019asds.pdf
Kernel drivers too seem to have it fixed
->
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/8390/ne.c#n398
->
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/net/ethernet/8390/ne2k-pci.c#n342
| > Check if (s->start == s->stop) at each receive call?
| Or in ne2000_buffer_full()?
ne2000_buffer_full() too assumes that 's->stop > s->start'
...
avail = (s->stop - s->start) - (index - boundary);
Is there a case wherein drivers need to adjust ring buffer pointers? If not, I
think it's better to convert EN0_STARTPG:, EN0_STOPPG:, EN0_BOUNDARY: and
EN1_CURPAG: cases into no-ops.
--
- P J P
47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F
