Realtek 8139 problem on 486.

Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hello all,

I'm observing a problem with Realtek 8139 cards on a couple of 486 
boxes. The respective driver is 8139too. It starts operation 
successfully, obtains an ip address via dhcp, replies to pings steadily, 
but some subsequent communication fails apparently. At least, nfsroot is 
unusable (it gets stuck in "... not responding, still trying" forever), 
and also iperf3 -c xxx when run against a neighbour box on a lan prints 
2-3 lines with some reasonable 7Mbit/s rate, then just prints 0s and 
subsequently throws a panic about output queue full or some such.

My kernel is 4.14.221 at the moment, but I can compile another if 
necessary.
I've already tried the "#define RTL8139_DEBUG 3" and "8139TOO_PIO=y" and 
"#define RX_DMA_BURST 4" and "#define TX_DMA_BURST 4" (in case there is 
a PCI burst issue, as mentioned somewhere) and nothing changed whatsoever.

Some additional notes:
- the problem is 100% reproducable;
- replacing this 8139 card with some entirely different 8139-based card 
changes nothing;
- if I replace this 8139 with a (just random) intel Pro/1000 card, 
everything seem to work fine;
- if I insert this 8139 into some other 486 motherboard (with a 
different chipset), everything seem to work fine again;
- etherboot and pxelinux work fine.

I'm willing to do some debugging but unfortunately I'm not anywhere 
familiar with this driver and network controllers in general, therefore 
I'm asking for some hints/advice first.


Thank you,

Regards,
Nikolai

Re: Realtek 8139 problem on 486.

[Heiner Kallweit]

On 29.05.2021 16:08, Nikolai Zhubr wrote:
> Hello all,
> 
> I'm observing a problem with Realtek 8139 cards on a couple of 486 boxes. The respective driver is 8139too. It starts operation successfully, obtains an ip address via dhcp, replies to pings steadily, but some subsequent communication fails apparently. At least, nfsroot is unusable (it gets stuck in "... not responding, still trying" forever), and also iperf3 -c xxx when run against a neighbour box on a lan prints 2-3 lines with some reasonable 7Mbit/s rate, then just prints 0s and subsequently throws a panic about output queue full or some such.
> 
> My kernel is 4.14.221 at the moment, but I can compile another if necessary.
> I've already tried the "#define RTL8139_DEBUG 3" and "8139TOO_PIO=y" and "#define RX_DMA_BURST 4" and "#define TX_DMA_BURST 4" (in case there is a PCI burst issue, as mentioned somewhere) and nothing changed whatsoever.
> 
> Some additional notes:
> - the problem is 100% reproducable;
> - replacing this 8139 card with some entirely different 8139-based card changes nothing;
> - if I replace this 8139 with a (just random) intel Pro/1000 card, everything seem to work fine;
> - if I insert this 8139 into some other 486 motherboard (with a different chipset), everything seem to work fine again;
> - etherboot and pxelinux work fine.
> 
> I'm willing to do some debugging but unfortunately I'm not anywhere familiar with this driver and network controllers in general, therefore I'm asking for some hints/advice first.
> 
This driver hasn't seen functional changes for ages. Any previous kernel
version that works fine so that you could bisect? It will be hard to
find any developer who has test hw, especially as your issue seems to be
system-dependent.
Please provide a full dmesg log, maybe it provides a hint.

You write: "reasonable 7Mbit/s rate"
So you operate the systems with a 10Mbit hub? Or any other reason why
you consider 7Mbps reasonable?

> 
> Thank you,
> 
> Regards,
> Nikolai
Heiner

Re: Realtek 8139 problem on 486.

[tedheadster]

> > I'm willing to do some debugging but unfortunately I'm not anywhere familiar with this driver and network controllers in general, therefore I'm asking for some hints/advice first.
> >
> This driver hasn't seen functional changes for ages. Any previous kernel
> version that works fine so that you could bisect? It will be hard to
> find any developer who has test hw, especially as your issue seems to be
> system-dependent.
> Please provide a full dmesg log, maybe it provides a hint.

I have a few active 80486 test systems (the 5.12.7 kernel works fine
on 80486) that I might be able to help test with too.

- Matthew

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

30.05.2021 0:44, tedheadster:
>> This driver hasn't seen functional changes for ages. Any previous kernel
>> version that works fine so that you could bisect? It will be hard to
>> find any developer who has test hw, especially as your issue seems to be
>> system-dependent.
>> Please provide a full dmesg log, maybe it provides a hint.
>
> I have a few active 80486 test systems (the 5.12.7 kernel works fine
> on 80486) that I might be able to help test with too.

Unfortunately I don't have any proven-to-work-well previous kernel for 
this specific hardware config, and dmesg does not show anything even 
slightly usefull.

However, I've just installed Debian Woody (kernel 2.2.20, gcc 2.95) and 
got something interesting. There are two interfaces shown in ifconfig:

eth0      Link encap:Ethernet  HWaddr 00:11:6B:32:85:74
           inet addr:192.168.0.3  Bcast:192.168.0.255  Mask:255.255.255.0
           ......
           Interrupt:9 Base address:0x6000

eth1      Link encap:Ethernet  HWaddr 00:11:6B:32:85:74
           inet addr:192.168.0.4  Bcast:192.168.0.255  Mask:255.255.255.0
           ......
           Interrupt:9 Base address:0x8000

whereas actually only one physical network card present. One can notice 
HWaddr and Irq is the same, although base addr is not.

lspci says:
00:0d.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139 
(rev 10)

Now as soon as ip configuration is done preperly, I can ifup any of them 
successfully.
And, eth0 shows some familiar misbehaviour: I can do ssh, but scp large 
file fails with subsequent communication breakdown. On the other hand, 
eth1 works fine, scp and iperf3 (both as a client and as a server) show 
quite regular operation. I'm puzzled a bit. But at least it looks like a 
possible starting point.


Thank you,

Regards,
Nikolai

>
> - Matthew
>

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

30.05.2021 3:49, Nikolai Zhubr:
[...]
> eth0 Link encap:Ethernet HWaddr 00:11:6B:32:85:74
> inet addr:192.168.0.3 Bcast:192.168.0.255 Mask:255.255.255.0
> ......
> Interrupt:9 Base address:0x6000
>
> eth1 Link encap:Ethernet HWaddr 00:11:6B:32:85:74
> inet addr:192.168.0.4 Bcast:192.168.0.255 Mask:255.255.255.0
> ......
> Interrupt:9 Base address:0x8000
>
> whereas actually only one physical network card present. One can notice
> HWaddr and Irq is the same, although base addr is not.
>
> lspci says:
> 00:0d.0 Ethernet controller: Realtek Semiconductor Co., Ltd. RTL-8139
> (rev 10)

Ok, sorted out this.
eth0 == rtl8139.c:v1.07 5/6/99
eth1 == 8139too Fast Ethernet driver 0.9.18-pre4

So, 8139too 0.9.18-pre4 as of kernel 2.2.20 is the one that apparently 
works correctly here. I feel the bisect is going to be massive.


Thank you,

Regards,
Nikolai

> Now as soon as ip configuration is done preperly, I can ifup any of them
> successfully.
> And, eth0 shows some familiar misbehaviour: I can do ssh, but scp large
> file fails with subsequent communication breakdown. On the other hand,
> eth1 works fine, scp and iperf3 (both as a client and as a server) show
> quite regular operation. I'm puzzled a bit. But at least it looks like a
> possible starting point.
>
>
> Thank you,
>
> Regards,
> Nikolai
>
>>
>> - Matthew
>>
>

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

30.05.2021 13:36, Nikolai Zhubr:
> So, 8139too 0.9.18-pre4 as of kernel 2.2.20 is the one that apparently
> works correctly here. I feel the bisect is going to be massive.

It appears the problem arrived between kernel 2.6.2 and 2.6.3, that is 
respectively 8139too versions 0.9.26 and 0.9.27.
Unmodified kernel 2.6.2 works fine, unmodified kernel 2.6.3 shows 
reproducable connectivity issues.

The diff is not small, I'm not sure I can dig through.
Any hints/ideas greatly appreciated.


Thank you,

Regards,
Nikolai


>
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Sun, May 30, 2021 at 7:19 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> Hi all,
>
> 30.05.2021 13:36, Nikolai Zhubr:
> > So, 8139too 0.9.18-pre4 as of kernel 2.2.20 is the one that apparently
> > works correctly here. I feel the bisect is going to be massive.
>
> It appears the problem arrived between kernel 2.6.2 and 2.6.3, that is
> respectively 8139too versions 0.9.26 and 0.9.27.
> Unmodified kernel 2.6.2 works fine, unmodified kernel 2.6.3 shows
> reproducable connectivity issues.
>
> The diff is not small, I'm not sure I can dig through.
> Any hints/ideas greatly appreciated.

This is apparently when NAPI was introduced into the driver.

One thing I noticed here was the handling for shared IRQs changing in
the process. Do you happen to have shared IRQs, and if so, can you
change it so this card has an IRQ that is not shared with any other
device?

        Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

30.05.2021 23:54, Arnd Bergmann:
[...]
>> Unmodified kernel 2.6.2 works fine, unmodified kernel 2.6.3 shows
>> reproducable connectivity issues.
>>
>> The diff is not small, I'm not sure I can dig through.
>> Any hints/ideas greatly appreciated.
>
> This is apparently when NAPI was introduced into the driver.
>
> One thing I noticed here was the handling for shared IRQs changing in
> the process. Do you happen to have shared IRQs, and if so, can you

I think this IRQ is not shared:

# cat /proc/interrupts
            CPU0
   0:     322570          XT-PIC  timer
   1:          8          XT-PIC  i8042
   2:          0          XT-PIC  cascade
   9:        896          XT-PIC  eth0
  14:      18000          XT-PIC  ide0
NMI:          0
ERR:          0

# dmesg | grep -i irq
eth0: RealTek RTL8139 at 0xc4800000, 00:11:6b:32:85:74, IRQ 9
ide0 at 0x1f0-0x1f7,0x3f6 on irq 14
serio: i8042 AUX port at 0x60,0x64 irq 12
serio: i8042 KBD port at 0x60,0x64 irq 1

However indeed, it seems a problem was introduced with a rework of 
interrupt handling (rtl8139_interrupt) in 2.6.3, because I have already 
pushed all other differences from 2.6.3 to 2.6.2 and it still keeps 
working fine.
My resulting minimized diff is still ~300 lines, it is too big and 
complicated to be usefull to post here as is.


Thank you,

Regards,
Nikolai


> change it so this card has an IRQ that is not shared with any other
> device?
>
>          Arnd
>

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

31.05.2021 2:17, Nikolai Zhubr:
[...]
> However indeed, it seems a problem was introduced with a rework of
> interrupt handling (rtl8139_interrupt) in 2.6.3, because I have already
> pushed all other differences from 2.6.3 to 2.6.2 and it still keeps
> working fine.
> My resulting minimized diff is still ~300 lines, it is too big and
> complicated to be usefull to post here as is.

Some more input.

I was able to minimize the problematic diff to basically one screenfull, 
it is quite comprehencable now, and I'm including it below. It is the 
change in status/event handling due to a switch to NAPI that intruduced 
the problem.
Now, in some more detailed tests, I observe that _receiving_ still works 
fine. It is _sending_ that suffers, and apparently, only when trying to 
send a lot at a time. In such case I see these warnings:

NETDEV WATCHDOG: eth0: transmit timed out
eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1

It looks like the queue of tx frames somehow gets messed up.

The essential diff fragment:
================================================
  	dev->open = rtl8139_open;
  	dev->hard_start_xmit = rtl8139_start_xmit;
+	dev->poll = rtl8139_poll;
  	dev->weight = 64;
  	dev->stop = rtl8139_close;
  	dev->get_stats = rtl8139_get_stats;
@@ -2015,7 +2010,7 @@
  			tp->stats.rx_bytes += pkt_size;
  			tp->stats.rx_packets++;

-			netif_rx (skb);
+			netif_receive_skb (skb);
  		} else {
  			if (net_ratelimit())
  				printk (KERN_WARNING
@@ -2138,10 +2133,8 @@
  	u16 status, ackstat;
  	int link_changed = 0; /* avoid bogus "uninit" warning */
  	int handled = 0;
-	int boguscnt = max_interrupt_work;

  	spin_lock (&tp->lock);
-	do {
  	status = RTL_R16 (IntrStatus);

  	/* shared irq? */
@@ -2169,8 +2162,14 @@
  	if (ackstat)
  		RTL_W16 (IntrStatus, ackstat);

-	if (netif_running (dev) && (status & RxAckBits))
-		rtl8139_rx (dev, tp, 1000000000);
+	/* Receive packets are processed by poll routine.
+	   If not running start it now. */
+	if (status & RxAckBits){
+		if (netif_rx_schedule_prep(dev)) {
+			RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
+			__netif_rx_schedule (dev);
+		}
+	}

  	/* Check uncommon events with one test. */
  	if (unlikely(status & (PCIErr | PCSTimeout | RxUnderrun | RxErr)))
@@ -2182,16 +2181,6 @@
  		if (status & TxErr)
  			RTL_W16 (IntrStatus, TxErr);
  	}
-	boguscnt--;
-	} while (boguscnt > 0);
-
================================================


Thank you,

Regards,
Nikolai



>
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Mon, May 31, 2021 at 7:35 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
> @@ -2169,8 +2162,14 @@
>         if (ackstat)
>                 RTL_W16 (IntrStatus, ackstat);
>
> -       if (netif_running (dev) && (status & RxAckBits))
> -               rtl8139_rx (dev, tp, 1000000000);
> +       /* Receive packets are processed by poll routine.
> +          If not running start it now. */
> +       if (status & RxAckBits){
> +               if (netif_rx_schedule_prep(dev)) {
> +                       RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
> +                       __netif_rx_schedule (dev);
> +               }
> +       }

Ok, so what you observe is that you get fewer interrupts because of NAPI,
and that TX times out, which points to a missing TX interrupt.

I looked at how the irq status and mask registers are handled, and did not
spot an obvious mistake there that would e.g. lead to the TX interrupts
being disabled or not acked while RX interrupts are disabled. (it does
seem odd that it both disables and defers the ack of the RX interrupts,
but that is probably harmless).

One possible issue is that the "RTL_W16 (IntrStatus, TxErr)" can
leak out of the spinlock unless it is changed to RTL_W16_F(), but
I don't see how that would cause your problem. This is probably
not the issue here, but it can't hurt to change that. Similarly,
the "RTL_W16 (IntrStatus, ackstat)" would need the same _F
to ensure that a  normal TX-only interrupt gets acked before the
spinlock.

Another observation I have is that the loop used to be around
"RTL_R16(IntrStatus); rtl8139_rx(); rtl8139_tx_interrupt()", so
removing the loop also means that the tx handler is only called
once when it used to be called for every loop iteration.
If this is what triggers the problem, you should be able to break
it the same way by moving the rtl8139_tx_interrupt() ahead of the
loop, and adjusting the RTL_W16 (IntrStatus, ackstat) accordingly
so you only Ack the TX before calling rtl8139_tx_interrupt().

       Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

Some more results follow. I'll report on all suggestions here in one go 
for brevity.

> One possible issue is that the "RTL_W16 (IntrStatus, TxErr)" can
> leak out of the spinlock unless it is changed to RTL_W16_F(), but
> I don't see how that would cause your problem. This is probably
> not the issue here, but it can't hurt to change that. Similarly,
> the "RTL_W16 (IntrStatus, ackstat)" would need the same _F
> to ensure that a  normal TX-only interrupt gets acked before the
> spinlock.

Just tested with "_F" added to all of them, did not help.

> Another observation I have is that the loop used to be around
> "RTL_R16(IntrStatus); rtl8139_rx(); rtl8139_tx_interrupt()", so
> removing the loop also means that the tx handler is only called
> once when it used to be called for every loop iteration.
> If this is what triggers the problem, you should be able to break
> it the same way by moving the rtl8139_tx_interrupt() ahead of the
> loop, and adjusting the RTL_W16 (IntrStatus, ackstat) accordingly
> so you only Ack the TX before calling rtl8139_tx_interrupt().

I get the idea in general, but not sure how exactly you proposed to move 
rtl8139_tx_interrupt() and adjust the RTL_W16 (IntrStatus, ackstat).
But meanwhile, I tried a dumb thing instead, and it worked!
I've put back The Loop:
---------------------------
+       int boguscnt = 20;

         spin_lock (&tp->lock);
+       do {
         status = RTL_R16 (IntrStatus);

         /* shared irq? */
@@ -2181,6 +2183,8 @@
                 if (status & TxErr)
                         RTL_W16 (IntrStatus, TxErr);
         }
+       boguscnt--;
+       } while (boguscnt > 0);
   out:
---------------------------
With this added, connection works fine again. Of course it is silly, but 
hopefully it gives a path for a real fix.

> What's your qdisc? Recently there was a bug related to the lockless
> pfifo_fast qdisc

If I understand correctly this means packet scheduler type. In more 
recent kernels I typically have CONFIG_DEFAULT_NET_SCH="fq_codel", now 
in 2.6.3 no explicite scheduler is enabled, so it must be some fast 
fifo. But as the sympthoms were basically identical in e.g. 2.6.3 and 
4.14, I suppose it is unlikely to be the cause.

> Issue could be related to rx and tx processing now potentially running in parallel.
> I only have access to the current 8139too source code, hopefully the following
> works on the old version:
>
> In the end of rtl8139_start_xmit() there's
> if ((tp->cur_tx - NUM_TX_DESC) == tp->dirty_tx)
> 		netif_stop_queue (dev);
>
> Try changing this to

Ok, the changes compiled fine, but unfortunately made no noticable 
difference.


Thank you,

Regards,
Nikolai

Re: Realtek 8139 problem on 486.

[Heiner Kallweit]

On 01.06.2021 00:18, Nikolai Zhubr wrote:
> Hi all,
> 
> Some more results follow. I'll report on all suggestions here in one go for brevity.
> 
>> One possible issue is that the "RTL_W16 (IntrStatus, TxErr)" can
>> leak out of the spinlock unless it is changed to RTL_W16_F(), but
>> I don't see how that would cause your problem. This is probably
>> not the issue here, but it can't hurt to change that. Similarly,
>> the "RTL_W16 (IntrStatus, ackstat)" would need the same _F
>> to ensure that a  normal TX-only interrupt gets acked before the
>> spinlock.
> 
> Just tested with "_F" added to all of them, did not help.
> 
>> Another observation I have is that the loop used to be around
>> "RTL_R16(IntrStatus); rtl8139_rx(); rtl8139_tx_interrupt()", so
>> removing the loop also means that the tx handler is only called
>> once when it used to be called for every loop iteration.
>> If this is what triggers the problem, you should be able to break
>> it the same way by moving the rtl8139_tx_interrupt() ahead of the
>> loop, and adjusting the RTL_W16 (IntrStatus, ackstat) accordingly
>> so you only Ack the TX before calling rtl8139_tx_interrupt().
> 
> I get the idea in general, but not sure how exactly you proposed to move rtl8139_tx_interrupt() and adjust the RTL_W16 (IntrStatus, ackstat).
> But meanwhile, I tried a dumb thing instead, and it worked!
> I've put back The Loop:
> ---------------------------
> +       int boguscnt = 20;
> 
>         spin_lock (&tp->lock);
> +       do {
>         status = RTL_R16 (IntrStatus);
> 
>         /* shared irq? */
> @@ -2181,6 +2183,8 @@
>                 if (status & TxErr)
>                         RTL_W16 (IntrStatus, TxErr);
>         }
> +       boguscnt--;
> +       } while (boguscnt > 0);
>   out:
> ---------------------------
> With this added, connection works fine again. Of course it is silly, but hopefully it gives a path for a real fix.
> 

What was discussed here 16 yrs ago should sound familiar to you.
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg92234.html
"It was an option in my BIOS PCI level/edge settings as I posted."
You could check whether you have same/similar option in your BIOS
and play with it.


>> What's your qdisc? Recently there was a bug related to the lockless
>> pfifo_fast qdisc
> 
> If I understand correctly this means packet scheduler type. In more recent kernels I typically have CONFIG_DEFAULT_NET_SCH="fq_codel", now in 2.6.3 no explicite scheduler is enabled, so it must be some fast fifo. But as the sympthoms were basically identical in e.g. 2.6.3 and 4.14, I suppose it is unlikely to be the cause.
> 
>> Issue could be related to rx and tx processing now potentially running in parallel.
>> I only have access to the current 8139too source code, hopefully the following
>> works on the old version:
>>
>> In the end of rtl8139_start_xmit() there's
>> if ((tp->cur_tx - NUM_TX_DESC) == tp->dirty_tx)
>>         netif_stop_queue (dev);
>>
>> Try changing this to
> 
> Ok, the changes compiled fine, but unfortunately made no noticable difference.
> 
> 
> Thank you,
> 
> Regards,
> Nikolai
> 
> 

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Tue, Jun 1, 2021 at 12:31 AM Heiner Kallweit <hkallweit1@gmail.com> wrote:
> On 01.06.2021 00:18, Nikolai Zhubr wrote:
> > But meanwhile, I tried a dumb thing instead, and it worked!
> > I've put back The Loop:
> > ---------------------------
> > +       int boguscnt = 20;
> >
> >         spin_lock (&tp->lock);
> > +       do {
> >         status = RTL_R16 (IntrStatus);
> >
> >         /* shared irq? */
> > @@ -2181,6 +2183,8 @@
> >                 if (status & TxErr)
> >                         RTL_W16 (IntrStatus, TxErr);
> >         }
> > +       boguscnt--;
> > +       } while (boguscnt > 0);
> >   out:
> > ---------------------------
> > With this added, connection works fine again. Of course it is silly, but hopefully it gives a path for a real fix.
> >
>
> What was discussed here 16 yrs ago should sound familiar to you.
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg92234.html
> "It was an option in my BIOS PCI level/edge settings as I posted."
> You could check whether you have same/similar option in your BIOS
> and play with it.

So it appears that the interrupt is lost if new TX events come in after the
status register is read, and that checking it again manages to make that
race harder to hit, but maybe not reliably.

The best idea I have for a proper fix would be to move the TX processing
into the poll function as well, making sure that by the end of that function
the driver is either still in napi polling mode, or both RX and TX interrupts
are enabled and acked.

         Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

01.06.2021 10:20, Arnd Bergmann:
[...]
>> What was discussed here 16 yrs ago should sound familiar to you.
>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg92234.html
>> "It was an option in my BIOS PCI level/edge settings as I posted."
>> You could check whether you have same/similar option in your BIOS
>> and play with it.

Yes indeed, this motherboard does have such an option, and it defaulted 
to "Edge", which apparently is not what PCI device normally expects.
Changing it to "Level" made unmodified kernel 2.6.4 work fine.
And 8259A.pl comfirms this, too.

Before:
# ./8259A.pl
irq 0: 00, edge
irq 1: 00, edge
irq 2: 00, edge
irq 3: 00, edge
irq 4: 00, edge
irq 5: 00, edge
irq 6: 00, edge
irq 7: 00, edge
irq 8: 00, edge
irq 9: 00, edge
irq 10: 00, edge
irq 11: 00, edge
irq 12: 00, edge
irq 13: 00, edge
irq 14: 00, edge
irq 15: 00, edge

After:
# ./8259A.pl
irq 0: 00, edge
irq 1: 00, edge
irq 2: 00, edge
irq 3: 00, edge
irq 4: 00, edge
irq 5: 00, edge
irq 6: 00, edge
irq 7: 00, edge
irq 8: 06, edge
irq 9: 06, level
irq 10: 06, level
irq 11: 06, edge
irq 12: 06, edge
irq 13: 06, edge
irq 14: 06, edge
irq 15: 06, edge

> So it appears that the interrupt is lost if new TX events come in after the
> status register is read, and that checking it again manages to make that
> race harder to hit, but maybe not reliably.

It looks like incorrect IRQ triggering mode makes 2 or more IRQs merge 
into one, kind of. However, if I understand this 8139 operation logic 
correctly, the possible max number of signaled events in one go is 
limited by the number of tx/rx descriptors and can not grow beyond it 
while inside the interrupt handler in any case. If so, using the loop 
would seem not that bad, and the limit would be certainly not 20 but 
max(NUM_TX_DESC, CONFIG_8139_RXBUF_IDX) == 4.

> The best idea I have for a proper fix would be to move the TX processing
> into the poll function as well, making sure that by the end of that function
> the driver is either still in napi polling mode, or both RX and TX interrupts
> are enabled and acked.

This one is too complicated for me to implement myself, so I'll have to 
wait if someone does this.

Alternatively, maybe it is possible to explicitely request level mode 
from 8259 at the driver startup?


Thank you,

Regards,
Nikolai

>
>           Arnd
>

Re: Realtek 8139 problem on 486.

[Heiner Kallweit]

On 01.06.2021 12:53, Nikolai Zhubr wrote:
> Hi all,
> 
> 01.06.2021 10:20, Arnd Bergmann:
> [...]
>>> What was discussed here 16 yrs ago should sound familiar to you.
>>> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg92234.html
>>> "It was an option in my BIOS PCI level/edge settings as I posted."
>>> You could check whether you have same/similar option in your BIOS
>>> and play with it.
> 
> Yes indeed, this motherboard does have such an option, and it defaulted to "Edge", which apparently is not what PCI device normally expects.
> Changing it to "Level" made unmodified kernel 2.6.4 work fine.

Great, so you have a solution for your problem, as i would expect that this
setting also makes 8139too in newer kernels usable for you.

> And 8259A.pl comfirms this, too.
> 
> Before:
> # ./8259A.pl
> irq 0: 00, edge
> irq 1: 00, edge
> irq 2: 00, edge
> irq 3: 00, edge
> irq 4: 00, edge
> irq 5: 00, edge
> irq 6: 00, edge
> irq 7: 00, edge
> irq 8: 00, edge
> irq 9: 00, edge
> irq 10: 00, edge
> irq 11: 00, edge
> irq 12: 00, edge
> irq 13: 00, edge
> irq 14: 00, edge
> irq 15: 00, edge
> 
> After:
> # ./8259A.pl
> irq 0: 00, edge
> irq 1: 00, edge
> irq 2: 00, edge
> irq 3: 00, edge
> irq 4: 00, edge
> irq 5: 00, edge
> irq 6: 00, edge
> irq 7: 00, edge
> irq 8: 06, edge
> irq 9: 06, level
> irq 10: 06, level
> irq 11: 06, edge
> irq 12: 06, edge
> irq 13: 06, edge
> irq 14: 06, edge
> irq 15: 06, edge
> 
>> So it appears that the interrupt is lost if new TX events come in after the
>> status register is read, and that checking it again manages to make that
>> race harder to hit, but maybe not reliably.
> 
> It looks like incorrect IRQ triggering mode makes 2 or more IRQs merge into one, kind of. However, if I understand this 8139 operation logic correctly, the possible max number of signaled events in one go is limited by the number of tx/rx descriptors and can not grow beyond it while inside the interrupt handler in any case. If so, using the loop would seem not that bad, and the limit would be certainly not 20 but max(NUM_TX_DESC, CONFIG_8139_RXBUF_IDX) == 4.
> 
>> The best idea I have for a proper fix would be to move the TX processing
>> into the poll function as well, making sure that by the end of that function
>> the driver is either still in napi polling mode, or both RX and TX interrupts
>> are enabled and acked.
> 
> This one is too complicated for me to implement myself, so I'll have to wait if someone does this.
> 
This hardware is so old that it's not very likely someone spends effort on this.

> Alternatively, maybe it is possible to explicitely request level mode from 8259 at the driver startup?
> 
I have doubts that it is possible to overrule what the BIOS/ACPI communicate
to the kernel.

> 
> Thank you,
> 
> Regards,
> Nikolai
> 
>>
>>           Arnd
>>
> 

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

01.06.2021 14:42, Heiner Kallweit:
[...]
> Great, so you have a solution for your problem, as i would expect that this
> setting also makes 8139too in newer kernels usable for you.

Well, not exactly, although I'm definitely getting close to some sort of 
satisfactory solution, and I appreciate all the helpfull replies.

Relying on this BIOS setting is still not good, because yet another 
motherboard I've just tested today does not have it (or at least in no 
evident form anyway).

> This hardware is so old that it's not very likely someone spends effort on this.

Now I'd like to ask, is quality reliable fix still wanted in mainline or 
rather not? Because I'll personally do my best to create/find a good fix 
anyway, but if it is of zero interest for mainline, I'll probably not 
invest much time into communicating it. My understanding was that 
default rule is "if broken go fix it" but due to the age of both code 
and hardware, maybe it is considered frozen or some such (I'm just not 
aware really).


Thank you,

Regards,
Nikolai

Re: Realtek 8139 problem on 486.

[Heiner Kallweit]

On 01.06.2021 18:09, Nikolai Zhubr wrote:
> Hi all,
> 
> 01.06.2021 14:42, Heiner Kallweit:
> [...]
>> Great, so you have a solution for your problem, as i would expect that this
>> setting also makes 8139too in newer kernels usable for you.
> 
> Well, not exactly, although I'm definitely getting close to some sort of satisfactory solution, and I appreciate all the helpfull replies.
> 
> Relying on this BIOS setting is still not good, because yet another motherboard I've just tested today does not have it (or at least in no evident form anyway).
> 
>> This hardware is so old that it's not very likely someone spends effort on this.
> 
> Now I'd like to ask, is quality reliable fix still wanted in mainline or rather not? Because I'll personally do my best to create/find a good fix anyway, but if it is of zero interest for mainline, I'll probably not invest much time into communicating it. My understanding was that default rule is "if broken go fix it" but due to the age of both code and hardware, maybe it is considered frozen or some such (I'm just not aware really).
> 
Driver 8139too has no maintainer. And you refer to "mainline" like to a number of developers
who are paid by somebody to maintain all drivers in the kernel. That's not the case in general.
You provided valuable input, and if you'd contribute to improving 8139too and submit patches for
fixing the issue you're facing, this would be much appreciated.
> 
> Thank you,
> 
> Regards,
> Nikolai

Heiner

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

02.06.2021 0:48, Heiner Kallweit:
> Driver 8139too has no maintainer.

Ups, indeed. I just looked at the header and supposed it has. Sorry.
(I do not touch kernel development much, usually)

> who are paid by somebody to maintain all drivers in the kernel. That's not the case in general.
> You provided valuable input, and if you'd contribute to improving 8139too and submit patches for
> fixing the issue you're facing, this would be much appreciated.

Ok, it is a bit more clear now.
I'll do more testing/searching/reading and probably come up with 
something then.


Thank you,

Regards,
Nikolai

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Wed, Jun 2, 2021 at 1:27 AM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
> 02.06.2021 0:48, Heiner Kallweit:
> > Driver 8139too has no maintainer.
>
> Ups, indeed. I just looked at the header and supposed it has. Sorry.
> (I do not touch kernel development much, usually)

It was apparently maintained by Jeff Garzik until 2007, but he already
considered
this driver outdated back then, and later stopped maintaining drivers
altogether.

> > who are paid by somebody to maintain all drivers in the kernel. That's not the case in general.
> > You provided valuable input, and if you'd contribute to improving 8139too and submit patches for
> > fixing the issue you're facing, this would be much appreciated.
>
> Ok, it is a bit more clear now.
> I'll do more testing/searching/reading and probably come up with
> something then.

I thought about the issue a bit more. The idea of the level interrupt
handler is that it
gets called again as long as any of the bits in 'IntrStatus' are set,
so the handler
can just read the register, write ack or mask the bits it sees and then process
the events it got. If another tx-complete even comes in before the end
of the handler,
it gets entered again and everything works.

If the irqchip is set to edge mode, this breaks down, and the loop can
work around
it by making it much more likely that all bits are cleared or masked at the
end of the handler, but there is no guarantee for this: If any
interrupt comes in
between reading the IntrStatus register and finishing the handler, you never get
an interrupt again.

I think the easiest workaround to address this reliably would be to move all
the irq processing into the poll function. This way the interrupt is completely
masked in the device until the poll handler finishes, and unmasking it
while there
are pending events would reliably trigger a new irq regardless of level or edge
mode. Something like the untested change at https://pastebin.com/MhBJDt6Z .
I don't know of other drivers that do it like this though, so I'm not
sure if this
causes a different set of problems.

       Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi Arnd,

02.06.2021 12:12, Arnd Bergmann:
[...]
> I think the easiest workaround to address this reliably would be to move all
> the irq processing into the poll function. This way the interrupt is completely
> masked in the device until the poll handler finishes, and unmasking it
> while there
> are pending events would reliably trigger a new irq regardless of level or edge
> mode. Something like the untested change at https://pastebin.com/MhBJDt6Z .
> I don't know of other drivers that do it like this though, so I'm not
> sure if this causes a different set of problems.

I started applying your patch (trying to morph it a little bit so as to 
shove in a minimally invasive manner into 4.14) and then noticed that it 
probably won't work as intended. If I'm not mistaken this rx poll thing 
is only active within kind of "rx bursts", so it is not guaranteed to be 
continually running all the time when there is no or little rx input. 
I'd suppose some new additional work/thread would have to be introduced 
in order for such approach to be reliably implemented.

Meanwhile, beside the lost tx irq issue, I've apparently identified rx 
overrun issue. According to tinymembench, the raw RAM performance of 
this system is roughly around 15-30 Mbytes/s at best, so it is close to 
100Mbit wire speed. Tracing NFS over UDP operation (client side) I've 
found that of 2 full-sized incoming NFS/UDP packets the second one will 
always be lost, approved by rapid increase of iface err counter. More 
specifically, I've found that a couple of packets sized 1500+700 can 
still be successfully accepted, but no way 1500+1500. Apparently 8139 
has very little ram builtin so it needs that packets can go into main 
ram fast enough. It appeared though that just adding rsize=1024 allows 
NFS work quite well, with only ocasional small pauses. Also, apparenly 
TCP/IP somehow recovers/autotunes iteself automatically, so it just 
works fine. I suppose this overrun problem can not be fixed in a general 
form (other than forcing a downgrade of link speed to 10 Mbit), as AFAIK 
there are no provisions in ethernet to request e.g. extra delays between 
packets. What might be usefull though is dropping some line to dmesg 
suggesting to somehow limit the incoming flow. Such hint in dmesg would 
have saved me quite some time.

Anyway, for now I got it working quite well (with a re-added busy loop 
and rsize=1024). I'm going to look at the elcr_set_level_irq approach 
later, but it looks quite complicated. If there is something else I can 
test while at it, please let me know.


Thank you,

Regards,
Nikolai


>
>         Arnd
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Tue, Jun 8, 2021 at 1:07 AM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
> 02.06.2021 12:12, Arnd Bergmann:
> [...]
> > I think the easiest workaround to address this reliably would be to move all
> > the irq processing into the poll function. This way the interrupt is completely
> > masked in the device until the poll handler finishes, and unmasking it
> > while there
> > are pending events would reliably trigger a new irq regardless of level or edge
> > mode. Something like the untested change at https://pastebin.com/MhBJDt6Z .
> > I don't know of other drivers that do it like this though, so I'm not
> > sure if this causes a different set of problems.
>
> I started applying your patch (trying to morph it a little bit so as to
> shove in a minimally invasive manner into 4.14) and then noticed that it
> probably won't work as intended. If I'm not mistaken this rx poll thing
> is only active within kind of "rx bursts", so it is not guaranteed to be
> continually running all the time when there is no or little rx input.
> I'd suppose some new additional work/thread would have to be introduced
> in order for such approach to be reliably implemented.

The basic idea of the napi poll function is to do batch processing as much
as possible and avoid excessive interrupts when the system is already
busy processing the received data.

However, it should not lead to any missed interrupts with my patch:
at any point in time, you have either all hardware interrupts enabled,
or you are in napi polling mode and are guaranteed to call the poll
function within a relatively short timespan. If you have no pending
rx events, processing should be pretty much instantaneous, it just
gets pushed from the irq handler to immediately following the irq
handler. If there is a constant stream of incoming data, it gets
moved into softirqd context, which may be delayed when there is
another thread running.

        Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

08.06.2021 10:44, Arnd Bergmann:
[...]
> However, it should not lead to any missed interrupts with my patch:
> at any point in time, you have either all hardware interrupts enabled,
> or you are in napi polling mode and are guaranteed to call the poll

For this to work, napi_complete should likely be called with some 
different condition instead?
E.g.:

-        if (work_done < budget) {
+        if ((work_done < budget) && !status) {

Otherwise polling would possibly be shut down before all non-rx events 
are cleared?
For some reason such 'corrected' version does not work here though 
(Communication fails completely). Probably I'm still missing something.


Thank you,

Regards,
Nikolai

> function within a relatively short timespan. If you have no pending
> rx events, processing should be pretty much instantaneous, it just
> gets pushed from the irq handler to immediately following the irq
> handler. If there is a constant stream of incoming data, it gets
> moved into softirqd context, which may be delayed when there is
> another thread running.
>
>          Arnd
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Tue, Jun 8, 2021 at 10:32 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> 08.06.2021 10:44, Arnd Bergmann:
> [...]
> > However, it should not lead to any missed interrupts with my patch:
> > at any point in time, you have either all hardware interrupts enabled,
> > or you are in napi polling mode and are guaranteed to call the poll
>
> For this to work, napi_complete should likely be called with some
> different condition instead?
> E.g.:
>
> -        if (work_done < budget) {
> +        if ((work_done < budget) && !status) {
>
> Otherwise polling would possibly be shut down before all non-rx events
> are cleared?
> For some reason such 'corrected' version does not work here though
> (Communication fails completely). Probably I'm still missing something.

The idea was that all non-rx events that were pending at the start of the
function have been Acked at this point, by writing to the IntrMask
register before processing the particular event. If the same kind of event
arrives after the Ack, then opening in the mask should immediately trigger
the interrupt handler, which reactivates the poll function.

If you instead want to re-arm the poll function every time that 'status'
was non-zero, you would have to also return 'budget' to the caller, like

+      if (status)
+             work_done = budget; /* pretend RX is still busy */
        if (work_done < budget) {

I think that should also work, but it seems more expensive since it would
always go back into the poll function after a non-RX event, rather than
only going back into the irq handler if a new event has just arrived.

        Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

08.06.2021 23:45, Arnd Bergmann:
> The idea was that all non-rx events that were pending at the start of the
> function have been Acked at this point, by writing to the IntrMask
> register before processing the particular event. If the same kind of event
> arrives after the Ack, then opening in the mask should immediately trigger
> the interrupt handler, which reactivates the poll function.

Ok, it works, indeed. The overall bitrate seems lower somewhat.
I'll re-test and benchmark some few variants (e.g. with and without busy 
loop) and report my findings.


Thank you,

Regards,
Nikolai

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Wed, Jun 9, 2021 at 12:07 AM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> 08.06.2021 23:45, Arnd Bergmann:
> > The idea was that all non-rx events that were pending at the start of the
> > function have been Acked at this point, by writing to the IntrMask
> > register before processing the particular event. If the same kind of event
> > arrives after the Ack, then opening in the mask should immediately trigger
> > the interrupt handler, which reactivates the poll function.
>
> Ok, it works, indeed. The overall bitrate seems lower somewhat.
> I'll re-test and benchmark some few variants (e.g. with and without busy
> loop) and report my findings.

If it's only a bit slower, that is not surprising, I'd expect it to
use fewer CPU
cycles though, as it avoids the expensive polling.

There are a couple of things you could do to make it faster without reducing
reliability, but I wouldn't recommend major surgery on this driver, I was just
going for the simplest change that would make it work right with broken
IRQ settings.

You could play around a little with the order in which you process events:
doing RX first would help free up buffer space in the card earlier, possibly
alternating between TX and RX one buffer at a time, or processing both
in a loop until the budget runs out would also help.

         Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi Arnd,

09.06.2021 10:09, Arnd Bergmann:
[...]
> If it's only a bit slower, that is not surprising, I'd expect it to
> use fewer CPU
> cycles though, as it avoids the expensive polling.
>
> There are a couple of things you could do to make it faster without reducing
> reliability, but I wouldn't recommend major surgery on this driver, I was just
> going for the simplest change that would make it work right with broken
> IRQ settings.
>
> You could play around a little with the order in which you process events:
> doing RX first would help free up buffer space in the card earlier, possibly
> alternating between TX and RX one buffer at a time, or processing both
> in a loop until the budget runs out would also help.

I've modified your patch so as to quickly test several approaches within 
a single file by just switching some conditional defines.
My diff against 4.14 is here:
https://pastebin.com/mgpLPciE

The tests were performed using a simple shell script:
https://pastebin.com/Vfr8JC3X

Each cell in the resulting table shows:
- tcp sender/receiver (Mbit/s) as reported by iperf3 (total)
- udp sender/receiver (Mbit/s) as reported by iperf3 (total)
- accumulated cpu utilization during tcp+upd test.

The first line in the table essentially corresponds to a standard 
unmodified kernel. The second line corresponds to your initially 
proposed approach.

All tests run with the same physical instance of 8139D card against the 
same server.

(The table best viewed in monospace font)
+-------------------+-------------+-----------+-----------+
| #Defines          ; i486dx2/66  ; Pentium3/ ; PentiumE/ |
|                   ; (Edge IRQ)  ;  1200     ; Dual 2600 |
+-------------------+-------------+-----------+-----------+
| TX_WORK_IN_IRQ 1  ;             ; tcp 86/86 ; tcp 94/94 |
| TX_WORK_IN_POLL 0 ;  (fails)    ; udp 96/96 ; udp 96/96 |
| LOOP_IN_IRQ 0     ;             ; cpu 59%   ; cpu 15%   |
| LOOP_IN_POLL 0    ;             ;           ;           |
+-------------------+-------------+-----------+-----------+
| TX_WORK_IN_IRQ 0  ; tcp 9.4/9.1 ; tcp 88/88 ; tcp 95/94 |
| TX_WORK_IN_POLL 1 ; udp 5.5/5.5 ; udp 96/96 ; udp 96/96 |
| LOOP_IN_IRQ 0     ; cpu 98%     ; cpu 55%   ; cpu 19%   |
| LOOP_IN_POLL 0    ;             ;           ;           |
+-------------------+-------------+-----------+-----------+
| TX_WORK_IN_IRQ 0  ; tcp 9.0/8.7 ; tcp 87/87 ; tcp 95/94 |
| TX_WORK_IN_POLL 1 ; udp 5.8/5.8 ; udp 96/96 ; udp 96/96 |
| LOOP_IN_IRQ 0     ; cpu 98%     ; cpu 58%   ; cpu 20%   |
| LOOP_IN_POLL 1    ;             ;           ;           |
+-------------------+-------------+-----------+-----------+
| TX_WORK_IN_IRQ 1  ; tcp 7.3/7.3 ; tcp 87/86 ; tcp 94/94 |
| TX_WORK_IN_POLL 0 ; udp 6.2/6.2 ; udp 96/96 ; udp 96/96 |
| LOOP_IN_IRQ 1     ; cpu 99%     ; cpu 57%   ; cpu 17%   |
| LOOP_IN_POLL 0    ;             ;           ;           |
+-------------------+-------------+-----------+-----------+
| TX_WORK_IN_IRQ 1  ; tcp 6.5/6.5 ; tcp 88/88 ; tcp 94/94 |
| TX_WORK_IN_POLL 1 ; udp 6.1/6.1 ; udp 96/96 ; udp 96/96 |
| LOOP_IN_IRQ 1     ; cpu 99%     ; cpu 55%   ; cpu 16%   |
| LOOP_IN_POLL 1    ;             ;           ;           |
+-------------------+-------------+-----------+-----------+
| TX_WORK_IN_IRQ 1  ; tcp 5.7/5.7 ; tcp 87/87 ; tcp 95/94 |
| TX_WORK_IN_POLL 1 ; udp 6.1/6.1 ; udp 96/96 ; udp 96/96 |
| LOOP_IN_IRQ 1     ; cpu 98%     ; cpu 56%   ; cpu 15%   |
| LOOP_IN_POLL 0    ;             ;           ;           |
+-------------------+-------------+-----------+-----------+

Hopefully this helps to choose the most benefical approach.


Thank you,

Regards,
Nikolai


>
>           Arnd
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Sat, Jun 12, 2021 at 7:40 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
> 09.06.2021 10:09, Arnd Bergmann:
> [...]
> > If it's only a bit slower, that is not surprising, I'd expect it to
> > use fewer CPU
> > cycles though, as it avoids the expensive polling.
> >
> > There are a couple of things you could do to make it faster without reducing
> > reliability, but I wouldn't recommend major surgery on this driver, I was just
> > going for the simplest change that would make it work right with broken
> > IRQ settings.
> >
> > You could play around a little with the order in which you process events:
> > doing RX first would help free up buffer space in the card earlier, possibly
> > alternating between TX and RX one buffer at a time, or processing both
> > in a loop until the budget runs out would also help.
>
> I've modified your patch so as to quickly test several approaches within
> a single file by just switching some conditional defines.
> My diff against 4.14 is here:
> https://pastebin.com/mgpLPciE
>
> The tests were performed using a simple shell script:
> https://pastebin.com/Vfr8JC3X
>
> Each cell in the resulting table shows:
> - tcp sender/receiver (Mbit/s) as reported by iperf3 (total)
> - udp sender/receiver (Mbit/s) as reported by iperf3 (total)
> - accumulated cpu utilization during tcp+upd test.
>
> The first line in the table essentially corresponds to a standard
> unmodified kernel. The second line corresponds to your initially
> proposed approach.
>
> All tests run with the same physical instance of 8139D card against the
> same server.
>
> (The table best viewed in monospace font)
> +-------------------+-------------+-----------+-----------+
> | #Defines          ; i486dx2/66  ; Pentium3/ ; PentiumE/ |
> |                   ; (Edge IRQ)  ;  1200     ; Dual 2600 |
> +-------------------+-------------+-----------+-----------+
> | TX_WORK_IN_IRQ 1  ;             ; tcp 86/86 ; tcp 94/94 |
> | TX_WORK_IN_POLL 0 ;  (fails)    ; udp 96/96 ; udp 96/96 |
> | LOOP_IN_IRQ 0     ;             ; cpu 59%   ; cpu 15%   |
> | LOOP_IN_POLL 0    ;             ;           ;           |
> +-------------------+-------------+-----------+-----------+
> | TX_WORK_IN_IRQ 0  ; tcp 9.4/9.1 ; tcp 88/88 ; tcp 95/94 |
> | TX_WORK_IN_POLL 1 ; udp 5.5/5.5 ; udp 96/96 ; udp 96/96 |
> | LOOP_IN_IRQ 0     ; cpu 98%     ; cpu 55%   ; cpu 19%   |
> | LOOP_IN_POLL 0    ;             ;           ;           |
> +-------------------+-------------+-----------+-----------+
> | TX_WORK_IN_IRQ 0  ; tcp 9.0/8.7 ; tcp 87/87 ; tcp 95/94 |
> | TX_WORK_IN_POLL 1 ; udp 5.8/5.8 ; udp 96/96 ; udp 96/96 |
> | LOOP_IN_IRQ 0     ; cpu 98%     ; cpu 58%   ; cpu 20%   |
> | LOOP_IN_POLL 1    ;             ;           ;           |
> +-------------------+-------------+-----------+-----------+
> | TX_WORK_IN_IRQ 1  ; tcp 7.3/7.3 ; tcp 87/86 ; tcp 94/94 |
> | TX_WORK_IN_POLL 0 ; udp 6.2/6.2 ; udp 96/96 ; udp 96/96 |
> | LOOP_IN_IRQ 1     ; cpu 99%     ; cpu 57%   ; cpu 17%   |
> | LOOP_IN_POLL 0    ;             ;           ;           |
> +-------------------+-------------+-----------+-----------+
> | TX_WORK_IN_IRQ 1  ; tcp 6.5/6.5 ; tcp 88/88 ; tcp 94/94 |
> | TX_WORK_IN_POLL 1 ; udp 6.1/6.1 ; udp 96/96 ; udp 96/96 |
> | LOOP_IN_IRQ 1     ; cpu 99%     ; cpu 55%   ; cpu 16%   |
> | LOOP_IN_POLL 1    ;             ;           ;           |
> +-------------------+-------------+-----------+-----------+
> | TX_WORK_IN_IRQ 1  ; tcp 5.7/5.7 ; tcp 87/87 ; tcp 95/94 |
> | TX_WORK_IN_POLL 1 ; udp 6.1/6.1 ; udp 96/96 ; udp 96/96 |
> | LOOP_IN_IRQ 1     ; cpu 98%     ; cpu 56%   ; cpu 15%   |
> | LOOP_IN_POLL 0    ;             ;           ;           |
> +-------------------+-------------+-----------+-----------+
>
> Hopefully this helps to choose the most benefical approach.

I think several variants can just be eliminated without looking
at the numbers:

- doing the TX work in the irq handler (with the loop) but not in
  the poll function is incorrect with the edge interupts, as it has
  the same race as before, you just make it much harder to hit

- doing the tx work in both the irq handler and the poll function
  is probably not helpful, you just do extra work

- calling the tx cleanup loop in a second loop is not helpful
  if you don't do anything interesting after finding that all
  TX frames are done.

For best performance I would suggest restructuring the poll
function from your current

  while (boguscnt--) {
       handle_rare_events();
       while (tx_pending())
             handle_one_tx();
  }
  while (rx_pending && work_done < budged)
         work_done += handle_one_rx();

to something like

   handle_rare_events();
   do {
      if (rx_pending())
          work_done += handle_one_rx();
      if (tx_pending())
          work_done += handle_one_tx();
   } while ((tx_pending || rx_pending) && work_done < budget)

This way, you can catch the most events in one poll function
if new work comes in while you are processing the pending
events.

Or, to keep the change simpler, keep the inner loop in the tx
and rx processing, doing all rx events before moving on
to processing all tx events, but then looping back to try both
again, until either the budget runs out or no further events
are pending.

      Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

13.06.2021 1:41, Arnd Bergmann:
> Or, to keep the change simpler, keep the inner loop in the tx
> and rx processing, doing all rx events before moving on
> to processing all tx events, but then looping back to try both
> again, until either the budget runs out or no further events
> are pending.

Ok, made a new version: https://pastebin.com/3FUUrg7C
It is much simpler and is very close to your patch now.

All previous conditional defines are eliminated along with unnecessary 
code fragments, and here is TUNE8139_BIG_LOOP to introduce a top-level 
loop in poll function as you suggested above. But apparently it works 
well both with and without this loop. At least my testing did not show 
any substantial difference in performance. Therefore I think it could be 
completely removed for the sake of simplicity.

One problem though is the kernel now always throws a traceback shortly 
after communication start:
https://pastebin.com/VhwQ8wsU
According to system.map it likely points to __local_bh_endble_ip() and 
there is one WARN_ON_ONCE() in it indeed, but I have no idea what it is 
and how to fix it.

Yet another thing is that tp->rx_lock and tp->lock are now used within 
poll function in a way that possibly suggests one of them could be 
eliminated.


Thank you,

Regards,
Nikolai


>
>        Arnd
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Sun, Jun 13, 2021 at 4:00 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> 13.06.2021 1:41, Arnd Bergmann:
> > Or, to keep the change simpler, keep the inner loop in the tx
> > and rx processing, doing all rx events before moving on
> > to processing all tx events, but then looping back to try both
> > again, until either the budget runs out or no further events
> > are pending.
>
> Ok, made a new version: https://pastebin.com/3FUUrg7C
> It is much simpler and is very close to your patch now.
>
> All previous conditional defines are eliminated along with unnecessary
> code fragments, and here is TUNE8139_BIG_LOOP to introduce a top-level
> loop in poll function as you suggested above. But apparently it works
> well both with and without this loop. At least my testing did not show
> any substantial difference in performance. Therefore I think it could be
> completely removed for the sake of simplicity.

Ok, simpler is better in that case.

> One problem though is the kernel now always throws a traceback shortly
> after communication start:
> https://pastebin.com/VhwQ8wsU
> According to system.map it likely points to __local_bh_endble_ip() and
> there is one WARN_ON_ONCE() in it indeed, but I have no idea what it is
> and how to fix it.

There must be some call to spin_unlock_bh() or local_bh_enabled() or similar,
which is not allowed when interrupts are disabled with spin_lock_irqsave().

I don't see where exactly happens, but since nothing interesting is now
executed in hardirq context, I assume you can change tp->lock from
being called with _irq or _irqsave to using the _bh version that just
blocks the poll and start_xmit functions from happening, not the
hardirq.

> Yet another thing is that tp->rx_lock and tp->lock are now used within
> poll function in a way that possibly suggests one of them could be
> eliminated.

Agreed. I didn't want to change too much in my proposal, but I'm
sure these can be merged into a single lock, or possibly even
eliminated entirely.

        Arnd

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Tue, 1 Jun 2021, Heiner Kallweit wrote:

> > Now I'd like to ask, is quality reliable fix still wanted in mainline or rather not? Because I'll personally do my best to create/find a good fix anyway, but if it is of zero interest for mainline, I'll probably not invest much time into communicating it. My understanding was that default rule is "if broken go fix it" but due to the age of both code and hardware, maybe it is considered frozen or some such (I'm just not aware really).
> > 
> Driver 8139too has no maintainer. And you refer to "mainline" like to a number of developers
> who are paid by somebody to maintain all drivers in the kernel. That's not the case in general.
> You provided valuable input, and if you'd contribute to improving 8139too and submit patches for
> fixing the issue you're facing, this would be much appreciated.

 It's an issue in x86 platform code, not the 8139too driver.  Any option 
card wired to PCI in this system somehow could suffer from it.  Depending 
on how you look at it you may or may not qualify it as a bug though, and 
any solution can be considered a workaround (for a BIOS misfeature) rather 
than a bug fix.

 The question is IMHO legitimate, and I can't speak for x86 platform 
maintainers.  If I were one, I'd accept a reasonable workaround and it 
does not appear to me it would be a very complex one to address this case: 
basically a PCI quirk to set "this southbridge has ELCR at the usual 
location" (if indeed it does; you don't want to blindly poke at random 
port I/O locations in generic code), and then a tweak to `pirq_enable_irq' 
or `pcibios_lookup_irq' as Arnd has suggested.

  Maciej

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Thu, Jun 3, 2021 at 8:32 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> On Tue, 1 Jun 2021, Heiner Kallweit wrote:
>
> > > Now I'd like to ask, is quality reliable fix still wanted in mainline or rather not?
> > > Because I'll personally do my best to create/find a good fix anyway, but if it is
> > > of zero interest for mainline, I'll probably not invest much time into communicating
> > > it. My understanding was that default rule is "if broken go fix it" but due to the
> > > age of both code and hardware, maybe it is considered frozen or some such
> > > (I'm just not aware really).
> > >
> > Driver 8139too has no maintainer. And you refer to "mainline" like to a number of developers
> > who are paid by somebody to maintain all drivers in the kernel. That's not the case in general.
> > You provided valuable input, and if you'd contribute to improving 8139too and submit patches for
> > fixing the issue you're facing, this would be much appreciated.
>
>  It's an issue in x86 platform code, not the 8139too driver.  Any option
> card wired to PCI in this system somehow could suffer from it.  Depending
> on how you look at it you may or may not qualify it as a bug though, and
> any solution can be considered a workaround (for a BIOS misfeature) rather
> than a bug fix.

I think it would be good though to reinstate the driver workaround in some way,
regardless of whether the x86 platform code gets changed or not.

From the old linux-2.6.2 code it appears that someone had intentionally
added the loop as a hack to make it work on a broken or misconfigured BIOS.
It's hard to know if that was indeed the intention, but it's clear that the
driver change in 2.6.3 broke something that worked (most of the time)
without fixing it in a better way.

>  The question is IMHO legitimate, and I can't speak for x86 platform
> maintainers.  If I were one, I'd accept a reasonable workaround and it
> does not appear to me it would be a very complex one to address this case:
> basically a PCI quirk to set "this southbridge has ELCR at the usual
> location" (if indeed it does; you don't want to blindly poke at random
> port I/O locations in generic code), and then a tweak to `pirq_enable_irq'
> or `pcibios_lookup_irq' as Arnd has suggested.

(adding the x86 maintainers to Cc, the thread is archived at
 https://lore.kernel.org/netdev/60B24AC2.9050505@gmail.com/)

Changing the x86 platform code as well would clearly help avoid similar
issues with other PCI cards on these broken platforms, but doing it
correctly  seems hard for a couple of reasons.

It sounds like it would have been a good idea 20 years ago when the
broken i486 platforms were still fairly common, but now we don't even
know whether the code was intentional or not. I don't remember a lot of
the specifics of pre-APIC x86, but I do remember IRQ configuration
as a common problem without a single good solution.

There is a realistic chance that other combinations of broken hardware
and drivers rely on the x86 PCI code doing exactly what it does today.
If overriding the BIOS setting breaks something that works today, nothing
is gained, because the next person running into an i486 PCI specific bug
is unlikely to be as persistent and competent as Nikolai in tracking down
the root cause.

Doing a narrower change to a specific chipset, motherboard or BIOS
would be less risky, but would likely miss similar cases. Any x86
specific change would clearly also miss similar problems that may
or may not exist on other architectures.

     Arnd

Re: Realtek 8139 problem on 486.

[Thomas Gleixner]

On Fri, Jun 04 2021 at 09:36, Arnd Bergmann wrote:
> On Thu, Jun 3, 2021 at 8:32 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>>  It's an issue in x86 platform code, not the 8139too driver.  Any option
>> card wired to PCI in this system somehow could suffer from it.  Depending
>> on how you look at it you may or may not qualify it as a bug though, and
>> any solution can be considered a workaround (for a BIOS misfeature) rather
>> than a bug fix.
>
> I think it would be good though to reinstate the driver workaround in some way,
> regardless of whether the x86 platform code gets changed or not.

Why?

> From the old linux-2.6.2 code it appears that someone had intentionally
> added the loop as a hack to make it work on a broken or misconfigured
> BIOS.

The usual fix the symptom not the root cause approach. So, no.

> It's hard to know if that was indeed the intention, but it's clear that the
> driver change in 2.6.3 broke something that worked (most of the time)
> without fixing it in a better way.
>
>>  The question is IMHO legitimate, and I can't speak for x86 platform
>> maintainers.  If I were one, I'd accept a reasonable workaround and it
>> does not appear to me it would be a very complex one to address this case:
>> basically a PCI quirk to set "this southbridge has ELCR at the usual
>> location" (if indeed it does; you don't want to blindly poke at random
>> port I/O locations in generic code), and then a tweak to `pirq_enable_irq'
>> or `pcibios_lookup_irq' as Arnd has suggested.

Correct. Any legacy PCI interrupt #A..#D must be level triggered. Edge
is simply wrong for those. But of course that's wishful thinking. See
below.

Vs. ELCR: it's documented to be 0x4d0, 0x4d1 and the kernel has it hard
coded. So that's really the least of my worries.

> (adding the x86 maintainers to Cc, the thread is archived at
>  https://lore.kernel.org/netdev/60B24AC2.9050505@gmail.com/)
>
> Changing the x86 platform code as well would clearly help avoid similar
> issues with other PCI cards on these broken platforms, but doing it
> correctly  seems hard for a couple of reasons.
>
> It sounds like it would have been a good idea 20 years ago when the
> broken i486 platforms were still fairly common, but now we don't even
> know whether the code was intentional or not. I don't remember a lot of
> the specifics of pre-APIC x86, but I do remember IRQ configuration
> as a common problem without a single good solution.

Yes. It still is because even today BIOS tinkerers get it wrong.

> There is a realistic chance that other combinations of broken hardware
> and drivers rely on the x86 PCI code doing exactly what it does today.

Legacy PCI interrupts are specified as being level triggered because
legacy PCI requires to share interrupts and you cannot share edge
triggered interrupts reliably. PCI got a lot of things wrong, but this
part is completely correct.

From experience I know that 8139 depends on that and I debugged that
myself many years ago on an ARM system which had the trigger type
configuration wrong.

Let me clarify why this breaks first. The interesting things to look at
are:

  - the device internal interrupt condition which in this case are
    package received or transmitted and status change

  - the interrupt line

Let's look at the timing diagram of simple RX events:

           ___________________               _____...
RX     ___|                   |_____________|

            ___________________               ____...
INTA   ____|                   |_____________|

              _ INT handler _____ RETI _        __...
Kernel ______|                          |______|

Trivial case which works with both edge and level. Now let me add TX
complete for the level triggered case:

           ___________________              
RX     ___|                   |_____________

                             _______________
TX     _____________________|                   

            ________________________________
INTA   ____|                

              _ INT handle RX ___ RETI_   __ INT handle TX
Kernel ______|                         |_|

Works as expected. Now the same with edge mode on the i8259 side:

           ___________________              
RX     ___|                   |_____________

                             _______________
TX     _____________________|                   

            ________________________________
INTA   ____|                

              _ INT handle RX ___ RETI_   
Kernel ______|                         |_____ <- FAIL


Edge mode loses the TX interrupt when it arrives before the RX condition
is cleared because INTA will not go low.

The driver cannot do anything about it reliably because all of this is
asynchronous. The loop approach is a bandaid and kinda works, but can it
work reliably? No. Aside of that it's curing the symptom which is the
wrong approach.

The only reasonable solution to this is to enforce level even if that
BIOS switch says otherwise.

> If overriding the BIOS setting breaks something that works today, nothing
> is gained, because the next person running into an i486 PCI specific bug
> is unlikely to be as persistent and competent as Nikolai in tracking down
> the root cause.

Yes, sigh. The reason why this BIOS switch exists, which should have
never existed, is that during the transition from EISA which was edge
triggered to PCI some card manufacturers just changed the bus interface
of their cards but completely missed the edge -> level change in
hardware either by stupidity or intentionally so they did not have to
make any changes to the rest of the hardware and to drivers.

The predominant OS'es at that time of course did not have an easy way to
add a quirk to fix this, so the way out was to add the chicken bit
option to the BIOS which then either tells the OS the trigger mode for
INTA..INTD and just sets up ELCR before booting the OS. I have faint
memories that I had to use such a BIOS switch long time ago to get some
add-on PCI card working.

So the right thing to do is to leave 8139 as is and add a PCI quirk
which enforces level trigger type for 8139 in legacy PCI interrupt mode.

Alternatively we can just emit a noisy warning when a legacy PCI
interrupt is configured as edge by the BIOS and tell people to toggle
that switch if stuff does not work. Though that might be futile because
not all BIOSes have these toggle options.

Thanks,

        tglx

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Sun, Jun 20, 2021 at 2:37 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, Jun 04 2021 at 09:36, Arnd Bergmann wrote:
> > There is a realistic chance that other combinations of broken hardware
> > and drivers rely on the x86 PCI code doing exactly what it does today.
>
> Legacy PCI interrupts are specified as being level triggered because
> legacy PCI requires to share interrupts and you cannot share edge
> triggered interrupts reliably. PCI got a lot of things wrong, but this
> part is completely correct.
>
> From experience I know that 8139 depends on that and I debugged that
> myself many years ago on an ARM system which had the trigger type
> configuration wrong.
>
> Let me clarify why this breaks first. The interesting things to look at
> are:
>
>   - the device internal interrupt condition which in this case are
>     package received or transmitted and status change
>
>   - the interrupt line
>
> Let's look at the timing diagram of simple RX events:
>
>            ___________________               _____...
> RX     ___|                   |_____________|
>
>             ___________________               ____...
> INTA   ____|                   |_____________|
>
>               _ INT handler _____ RETI _        __...
> Kernel ______|                          |______|
>
> Trivial case which works with both edge and level. Now let me add TX
> complete for the level triggered case:
>
>            ___________________
> RX     ___|                   |_____________
>
>                              _______________
> TX     _____________________|
>
>             ________________________________
> INTA   ____|
>
>               _ INT handle RX ___ RETI_   __ INT handle TX
> Kernel ______|                         |_|
>
> Works as expected. Now the same with edge mode on the i8259 side:
>
>            ___________________
> RX     ___|                   |_____________
>
>                              _______________
> TX     _____________________|
>
>             ________________________________
> INTA   ____|
>
>               _ INT handle RX ___ RETI_
> Kernel ______|                         |_____ <- FAIL
>
>
> Edge mode loses the TX interrupt when it arrives before the RX condition
> is cleared because INTA will not go low.

Right, that is what I concluded as well.

> The driver cannot do anything about it reliably because all of this is
> asynchronous. The loop approach is a bandaid and kinda works, but can it
> work reliably? No. Aside of that it's curing the symptom which is the
> wrong approach.
>
> The only reasonable solution to this is to enforce level even if that
> BIOS switch says otherwise.

I think the hack that Nikolai implemented based on my suggestion
should work around this reliably though: All events (RX, TX, and others)
are now masked in the hardirq handler but processed and acked inside
the napi poll function. After it has processed all events, the device unmasks
all events, so if a new event is already pending at this time, the IRQ line
gets raised and will correctly be processed in both edge and level
mode.

Am I missing something here?

Obviously it's just a hack and it's still going to fail if the edge interrupt
is actually shared with another device, but I don't see any downside
in changing the driver that way.

> > If overriding the BIOS setting breaks something that works today, nothing
> > is gained, because the next person running into an i486 PCI specific bug
> > is unlikely to be as persistent and competent as Nikolai in tracking down
> > the root cause.
>
> Yes, sigh. The reason why this BIOS switch exists, which should have
> never existed, is that during the transition from EISA which was edge
> triggered to PCI some card manufacturers just changed the bus interface
> of their cards but completely missed the edge -> level change in
> hardware either by stupidity or intentionally so they did not have to
> make any changes to the rest of the hardware and to drivers.
>
> The predominant OS'es at that time of course did not have an easy way to
> add a quirk to fix this, so the way out was to add the chicken bit
> option to the BIOS which then either tells the OS the trigger mode for
> INTA..INTD and just sets up ELCR before booting the OS. I have faint
> memories that I had to use such a BIOS switch long time ago to get some
> add-on PCI card working.
>
> So the right thing to do is to leave 8139 as is and add a PCI quirk
> which enforces level trigger type for 8139 in legacy PCI interrupt mode.

So you would prefer a quirk specific to this PCI device on misconfigured
machines? That would at least completely avoid the risk of breaking setups
that rely on an intentional misconfiguration, since it only changes setups
that are definitely not working.

On the other hand, it wouldn't help with non-x86 machines that might
have the same issue, or on x86 machines with a different PCI card
in a misconfigured slot, unless there we maintain a list of PCI
IDs that are known to not work with a broken BIOS configuration.

> Alternatively we can just emit a noisy warning when a legacy PCI
> interrupt is configured as edge by the BIOS and tell people to toggle
> that switch if stuff does not work. Though that might be futile because
> not all BIOSes have these toggle options.

That sounds like a good idea regardless of any other outcome.

       Arnd

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Sun, 20 Jun 2021, Thomas Gleixner wrote:

> > If overriding the BIOS setting breaks something that works today, nothing
> > is gained, because the next person running into an i486 PCI specific bug
> > is unlikely to be as persistent and competent as Nikolai in tracking down
> > the root cause.
> 
> Yes, sigh. The reason why this BIOS switch exists, which should have
> never existed, is that during the transition from EISA which was edge
> triggered to PCI some card manufacturers just changed the bus interface
> of their cards but completely missed the edge -> level change in
> hardware either by stupidity or intentionally so they did not have to
> make any changes to the rest of the hardware and to drivers.

 Umm, I have been fairly sure it is actually EISA that has introduced 
switching between edge- and level-triggered interrupts on a line-by-line 
basis for ISA option compatibility (previously ISA and MCA were globally 
edge-triggered and level-triggered respectively via a bit in ICW1 as per 
classic 8259A configuration modes) and at least Intel documentation[1][2] 
confirms that ELCR was already present in plain EISA chipsets, so that you 
could set IRQ lines used by ISA and EISA cards for edge-triggering and 
level-triggering respectively.

 One peculiarity here is that level-triggered interrupts are necessarily 
active-low to permit sharing, but ISA edge-triggered interrupts are 
active-high.  Otherwise due to the oddity of how edge-triggered 8259A 
interrupts work we could actually safely reprogram all but the 8254 timer 
for level triggering and things would still work just fine (pretty much 
what MCA did with a hardware hack for the timer).

 So while there may have been odd cases of ISA circuitry repurposed for 
PCI (and then in a broken manner), they must have been exceedingly rare in 
the first place, and likely hardly any have survived, as their usability 
must have been marginal.

 For EISA I reckon such repurposing wouldn't be an issue as the 
manufacturer would simply require the interrupt line to be programmed by 
the EISA BIOS for the edge-triggered mode via the accompanying ECU 
configuration file for the piece of hardware in question.  In fact some 
genuine ISA hardware, such as the 3Com 3c509b Ethernet card, did support 
configuring as EISA hardware.

> The predominant OS'es at that time of course did not have an easy way to
> add a quirk to fix this, so the way out was to add the chicken bit
> option to the BIOS which then either tells the OS the trigger mode for
> INTA..INTD and just sets up ELCR before booting the OS. I have faint
> memories that I had to use such a BIOS switch long time ago to get some
> add-on PCI card working.
> 
> So the right thing to do is to leave 8139 as is and add a PCI quirk
> which enforces level trigger type for 8139 in legacy PCI interrupt mode.

 I think it makes little sense really to add a quirk based on option card 
hardware ID, as one could plug any PCI card into a slot whose interrupt 
line to be used has been incorrectly configured in the BIOS.  So all PCI 
cards would have to have such a quirk defined.

 As I noted in a previous message I think the only sensible approach is to 
have the quirk based on the southbridge hardware ID, or in the absence of 
one -- the northbridge ID (I have now double-checked and it's the Aries 
chipset that does not make its southbridge component, the 82426EX ISA 
Bridge (IB), visible in the PCI configuration space).

 If this system uses the IB or the 82378ZB SIO southbridge, then we can 
handle it with full confidence however, as they have been documented, and 
they both have the ELCR register[3][4][5].  Note that the earlier version 
of the SIO southbridge, the 82378IB doesn't have the ELCR register[6][7], 
and it could only handle PCI interrupts with board-specific external logic 
(probably a design erratum/oversight, hence a later correction[8]).

 The northbridge ID for Aries is 8086:0486 and the southbridge ID for SIO 
is 8086:0484.  The 82378IB and 82378ZB SIO revisions can be told apart by 
the configuration-space Revision Identification Register, bits 3:0: 0x0 
for the 82378IB w/o ELCR, 0x3 for the 82378ZB with ELCR[9].

> Alternatively we can just emit a noisy warning when a legacy PCI
> interrupt is configured as edge by the BIOS and tell people to toggle
> that switch if stuff does not work. Though that might be futile because
> not all BIOSes have these toggle options.

 A warning surely won't hurt, but TBH I'd just reprogram any incorrectly 
configured PCI interrupt line unconditionally where the ELCR is available.  
The chance someone uses a broken PCI card that drives its interrupt line 
as edge-triggered and is actually handled by a Linux driver both at a time 
is IMO nil.  Such a card requires special provisions hardly any system 
provides and has anyone here seen a report of such a beast?

 NB copies of documents referred available upon request, except for 
290473-004, which I only have in the hardcopy form (though see 
<https://archive.org/details/bitsavers_intelpentirocessorsandRelatedComponents_64170750>).

References:

[1] "82357 Integrated System Peripheral (ISP)", Intel Corporation, Order 
    Number: 290253-006, March, 1994, Section 5.14.7 "Edge and Level
    Triggered Modes", p. 44

[2] "Multiprocessor Specification", Version 1.1, Intel Corporation, Order 
    Number: 242016-003, September 1994, Section 5.3.2 "Level-triggered 
    Interrupt Support", p. 5-7

[3] "82420EX PCIset Data Sheet, 82425EX PCI System Controller (PSC)
    and 82426EX ISA Bridge (IB)", Intel Corporation, Order Number: 
    290488-004, December 1995, Section 4.11.2 "Edge and Level Interrupt
    Triggered Mode", p. 111

[4] "82378 System I/O (SIO)", Intel Corporation, Order Number: 290473-004, 
    December 1994, Section 5.8.1 "Edge and Level Triggered Modes"

[5] "82378ZB System I/O (SIO) and 82379AB System I/O APIC (SIO.A)", Intel 
    Corporation, Order Number: 290571-001, March 1996, Section 4.8.1. 
    "Edge and Level Triggered Modes", p. 101

[6] "82378IB System I/O (SIO)", Intel Corporation, Order Number: 
    290473-002, April 1993, Section 5.8.7.7 "Edge and Level Triggered 
    Modes"

[7] "82378 System I/O (SIO)", Intel Corporation, Order Number: 290473-004, 
    December 1994, Section 4.4.1 "ICW1--Initialization Command Word 1 
    Register"

[8] "82378IB to 82378ZB Errata Fix and Feature Enhancement Conversion 
    FOL933002-01", 
    <https://web.archive.org/web/19990421045433/http://support.intel.com/support/chipsets/420/8511.htm>

[9] "82378 System I/O (SIO)", Intel Corporation, Order Number: 290473-004,
    December 1994, Section 4.1.5 "RID--Revision Identification Register"

  Maciej

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Mon, Jun 21, 2021 at 6:10 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> On Sun, 20 Jun 2021, Thomas Gleixner wrote:
>
> > Alternatively we can just emit a noisy warning when a legacy PCI
> > interrupt is configured as edge by the BIOS and tell people to toggle
> > that switch if stuff does not work. Though that might be futile because
> > not all BIOSes have these toggle options.
>
>  A warning surely won't hurt, but TBH I'd just reprogram any incorrectly
> configured PCI interrupt line unconditionally where the ELCR is available.
> The chance someone uses a broken PCI card that drives its interrupt line
> as edge-triggered and is actually handled by a Linux driver both at a time
> is IMO nil.  Such a card requires special provisions hardly any system
> provides and has anyone here seen a report of such a beast?

I looked some more through the git history and found at least one time
that the per-chipset ELCR fixup came up for discussion[1], and this
appears to have resulted in generalizing an ALI specific fixup into
common code into common code[2], so we should already be doing
exactly this in many cases. If Nikolai can boot the system with debugging
enabled for arch/x86/pci/irq.c, we should be able to see exactly
which code path is his in his case, and why it doesn't go through
setting that register at the moment.

I also found an slightly more recent discussion, from where it seems
that the authoritative decision when it came up in the past was that edge
triggered interrupts are supposed to work as long as they are not
shared [3][4].

       Arnd

[1] https://lore.kernel.org/lkml/Pine.LNX.4.10.10011230901580.7992-100000@penguin.transmeta.com/
[2] https://repo.or.cz/davej-history.git/commitdiff/e3576079d9e2#patch63
[3] https://yarchive.net/comp/linux/edge_triggered_interrupts.html
[4] https://lore.kernel.org/lkml/Pine.LNX.4.64.0604241156340.3701@g5.osdl.org/

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Mon, 21 Jun 2021, Arnd Bergmann wrote:

> >  A warning surely won't hurt, but TBH I'd just reprogram any incorrectly
> > configured PCI interrupt line unconditionally where the ELCR is available.
> > The chance someone uses a broken PCI card that drives its interrupt line
> > as edge-triggered and is actually handled by a Linux driver both at a time
> > is IMO nil.  Such a card requires special provisions hardly any system
> > provides and has anyone here seen a report of such a beast?
> 
> I looked some more through the git history and found at least one time
> that the per-chipset ELCR fixup came up for discussion[1], and this
> appears to have resulted in generalizing an ALI specific fixup into
> common code into common code[2], so we should already be doing
> exactly this in many cases. If Nikolai can boot the system with debugging
> enabled for arch/x86/pci/irq.c, we should be able to see exactly
> which code path is his in his case, and why it doesn't go through
> setting that register at the moment.
> 
> I also found an slightly more recent discussion, from where it seems
> that the authoritative decision when it came up in the past was that edge
> triggered interrupts are supposed to work as long as they are not
> shared [3][4].

 Sadly Linus's rule applies both ways: if a device has been designed with 
level-triggered interrupts in mind, there may be no race-free way to 
ensure an active-to-inactive-to-active transition has happened on its IRQ 
line as the driver acknowledges handling in the relevant device's CSR.  

 The rule of thumb is to acknowledge early in the handler, and to work 
around broken configurations it may be desirable to also briefly mask all 
the interrupt sources with the device so as to make sure it deasserts its 
IRQ line even if another interrupt has already been queued.  OTOH if IRQ
sharing is to be supported a device absolutely has to have an interrupt 
mask register, as the system cannot rely on masking at the interrupt 
controller if multiple devices are to be handled with a single line.  I 
suspect many of our drivers do not do such precautionary masking though.

 Is there a mask register with the 8139?

 NB I find it amazing how people can break such relatively simple 
concepts, like not getting PCI IRQ steering right in early designs or 
messing up edge and level triggering.  I mean wired interrupts are not 
rocket science.  Sigh...

  Maciej

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Mon, Jun 21, 2021 at 4:42 PM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Mon, 21 Jun 2021, Arnd Bergmann wrote:
>
> > I also found an slightly more recent discussion, from where it seems
> > that the authoritative decision when it came up in the past was that edge
> > triggered interrupts are supposed to work as long as they are not
> > shared [3][4].
>
>  Sadly Linus's rule applies both ways: if a device has been designed with
> level-triggered interrupts in mind, there may be no race-free way to
> ensure an active-to-inactive-to-active transition has happened on its IRQ
> line as the driver acknowledges handling in the relevant device's CSR.
>
>  The rule of thumb is to acknowledge early in the handler, and to work
> around broken configurations it may be desirable to also briefly mask all
> the interrupt sources with the device so as to make sure it deasserts its
> IRQ line even if another interrupt has already been queued.  OTOH if IRQ
> sharing is to be supported a device absolutely has to have an interrupt
> mask register, as the system cannot rely on masking at the interrupt
> controller if multiple devices are to be handled with a single line.  I
> suspect many of our drivers do not do such precautionary masking though.
>
>  Is there a mask register with the 8139?

Ah, it seems that the Cc list got dropped in a different sub-thread, see
https://pastebin.com/3FUUrg7C for a version of my patch that Nikolai
tested successfully. This one goes further by completely masking
all interrupts between the hardirq handler and the subsequent
napi_complete_done(). This may not be the most efficient way of
doing it, but it seems good enough according to his measurements,
and it is a relatively safe and straightforward change.

      Arnd

RE: Realtek 8139 problem on 486.

[David Laight]

From: Maciej W. Rozycki
> Sent: 21 June 2021 15:42
...
>  The rule of thumb is to acknowledge early in the handler, and to work
> around broken configurations it may be desirable to also briefly mask all
> the interrupt sources with the device so as to make sure it deasserts its
> IRQ line even if another interrupt has already been queued.  OTOH if IRQ
> sharing is to be supported a device absolutely has to have an interrupt
> mask register, as the system cannot rely on masking at the interrupt
> controller if multiple devices are to be handled with a single line.  I
> suspect many of our drivers do not do such precautionary masking though.

Typically you need to:
1) stop the chip driving IRQ low.
2) process all the completed RX and TX entries.
3) clear the chip's interrupt pending bits (often write to clear).
4) check for completed RX/TX entries, back to 2 if found.
5) enable driving IRQ.

The loop (4) is needed because of the timing window between
(2) and (3).
You can swap (2) and (3) over - but then you get an additional
interrupt if packets arrive during processing - which is common.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

21.06.2021 14:22, Arnd Bergmann:
[...]
> I looked some more through the git history and found at least one time
> that the per-chipset ELCR fixup came up for discussion[1], and this
> appears to have resulted in generalizing an ALI specific fixup into
> common code into common code[2], so we should already be doing
> exactly this in many cases. If Nikolai can boot the system with debugging
> enabled for arch/x86/pci/irq.c, we should be able to see exactly
> which code path is his in his case, and why it doesn't go through
> setting that register at the moment.

Here is my dmesg with debugging (hopefully) added to irq.c:

https://pastebin.com/tnC2rRDM

This is unmodified kernel, except for 8139too.c.
I've now realized maybe some usefull config option(s) related to pci irq 
handling are not enabled because due to size considerations it was 
initially configured as very minimalistic with just some really 
necessary features enabled (although ACPI, PCI_BIOS, PCI, EISA, ISA are 
enabled)

Just rechecked it again with the 8259A.pl script:

# 8259A.pl
irq 0: 00, edge
irq 1: 00, edge
irq 2: 00, edge
irq 3: 00, edge
irq 4: 00, edge
irq 5: 00, edge
irq 6: 00, edge
irq 7: 00, edge
irq 8: 00, edge
irq 9: 00, edge
irq 10: 00, edge
irq 11: 00, edge
irq 12: 00, edge
irq 13: 00, edge
irq 14: 00, edge
irq 15: 00, edge


Thank you,

Regards,
Nikolai

> I also found an slightly more recent discussion, from where it seems
> that the authoritative decision when it came up in the past was that edge
> triggered interrupts are supposed to work as long as they are not
> shared [3][4].
>
>         Arnd
>
> [1] https://lore.kernel.org/lkml/Pine.LNX.4.10.10011230901580.7992-100000@penguin.transmeta.com/
> [2] https://repo.or.cz/davej-history.git/commitdiff/e3576079d9e2#patch63
> [3] https://yarchive.net/comp/linux/edge_triggered_interrupts.html
> [4] https://lore.kernel.org/lkml/Pine.LNX.4.64.0604241156340.3701@g5.osdl.org/
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Tue, Jun 22, 2021 at 2:32 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> 21.06.2021 14:22, Arnd Bergmann:
> [...]
> > I looked some more through the git history and found at least one time
> > that the per-chipset ELCR fixup came up for discussion[1], and this
> > appears to have resulted in generalizing an ALI specific fixup into
> > common code into common code[2], so we should already be doing
> > exactly this in many cases. If Nikolai can boot the system with debugging
> > enabled for arch/x86/pci/irq.c, we should be able to see exactly
> > which code path is his in his case, and why it doesn't go through
> > setting that register at the moment.
>
> Here is my dmesg with debugging (hopefully) added to irq.c:
>
> https://pastebin.com/tnC2rRDM

Ok, so it's getting roughly through the right code path:

        dev_dbg(&dev->dev, "PCI INT %c -> PIRQ %02x, mask %04x, excl %04x",
                'A' + pin - 1, pirq, mask, pirq_table->exclusive_irqs);
...
        dev_dbg(&dev->dev, "PCI INT %c -> newirq %d", 'A' + pin - 1, newirq);

        /* Check if it is hardcoded */
        if ((pirq & 0xf0) == 0xf0) {
                irq = pirq & 0xf;
                msg = "hardcoded";
        } else if (r->get && (irq = r->get(pirq_router_dev, dev, pirq)) && \
        ((!(pci_probe & PCI_USE_PIRQ_MASK)) || ((1 << irq) & mask))) {
                msg = "found";
                elcr_set_level_irq(irq);
        } else if (newirq && r->set &&
                (dev->class >> 8) != PCI_CLASS_DISPLAY_VGA) {
                if (r->set(pirq_router_dev, dev, pirq, newirq)) {
                        elcr_set_level_irq(newirq);
                        msg = "assigned";
                        irq = newirq;
                }
        }

        if (!irq) {
                if (newirq && mask == (1 << newirq)) {
                        msg = "guessed";
                        irq = newirq;
                } else {
                        dev_dbg(&dev->dev, "can't route interrupt\n");
                        return 0;
                }
        }
        dev_info(&dev->dev, "%s PCI INT %c -> IRQ %d\n", msg, 'A' +
pin - 1, irq);

with the corresponding output from that one being

[    0.761546] 8139too 0000:00:0d.0: runtime IRQ mapping not provided by arch
[    0.761546] 8139too: 8139too Fast Ethernet driver 0.9.28
[    0.761546] 8139too 0000:00:0d.0: PCI INT A -> PIRQ 02, mask 1eb8, excl 0001
[    0.765817] 8139too 0000:00:0d.0: PCI INT A -> newirq 9
[    0.765817] 8139too 0000:00:0d.0: can't route interrupt
[    0.777546] 8139too 0000:00:0d.0 eth0: RealTek RTL8139 at
0xc4804000, 00:11:6b:32:85:74, IRQ 9

From what I can tell, this means that there is no chipset specific
get/set function,
the irq is not hardcoded but the 'mask' value in the irq_info table (0x1eb8)
lists a number of options.

In this case, the function gives up with the  "can't route interrupt\n"
output and does not call elcr_set_level_irq(newirq). Adding the call
to elcr_set_level_irq() in that code path as well probably makes it
set the irq to level mode:

--- a/arch/x86/pci/irq.c
+++ b/arch/x86/pci/irq.c
@@ -984,6 +984,7 @@ static int pcibios_lookup_irq(struct pci_dev *dev,
int assign)
                        irq = newirq;
                } else {
                        dev_dbg(&dev->dev, "can't route interrupt\n");
+                       elcr_set_level_irq(newirq);
                        return 0;
                }
        }

No idea if doing this is a good idea though, in particular I have no clue
about whether this is a common scenario or not.

        Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

22.06.2021 16:22, Arnd Bergmann:
[...]
> In this case, the function gives up with the  "can't route interrupt\n"
> output and does not call elcr_set_level_irq(newirq). Adding the call
> to elcr_set_level_irq() in that code path as well probably makes it
> set the irq to level mode:
>
> --- a/arch/x86/pci/irq.c
> +++ b/arch/x86/pci/irq.c
> @@ -984,6 +984,7 @@ static int pcibios_lookup_irq(struct pci_dev *dev,
> int assign)
>                          irq = newirq;
>                  } else {
>                          dev_dbg(&dev->dev, "can't route interrupt\n");
> +                       elcr_set_level_irq(newirq);
>                          return 0;
>                  }
>          }

Yes, it does indeed:

[    0.765886] 8139too 0000:00:0d.0: can't route interrupt
[    0.765886] PCI: setting IRQ 9 as level-triggered
[    0.781734] 8139too 0000:00:0d.0 eth0: RealTek RTL8139 at 0xc4804000, 
00:11:6b:32:85:74, IRQ 9

And also here:

# 8259A.pl
irq 0: 00, edge
irq 1: 00, edge
irq 2: 00, edge
irq 3: 00, edge
irq 4: 00, edge
irq 5: 00, edge
irq 6: 00, edge
irq 7: 00, edge
irq 8: 02, edge
irq 9: 02, level
irq 10: 02, edge
irq 11: 02, edge
irq 12: 02, edge
irq 13: 02, edge
irq 14: 02, edge
irq 15: 02, edge

Now connection also works fine with unmodified 8139too driver.
The percentage of low-level errors stays very small:

RX packets:13953 errors:1 dropped:2 overruns:1 frame:0
TX packets:37346 errors:0 dropped:0 overruns:13 carrier:0

This fix looks really nice. Maybe it is right thing to do.


Thank you,

Regards,
Nikolai

>
> No idea if doing this is a good idea though, in particular I have no clue
> about whether this is a common scenario or not.
>
>          Arnd
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Tue, Jun 22, 2021 at 8:32 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
> 22.06.2021 16:22, Arnd Bergmann:

> irq 8: 02, edge
> irq 9: 02, level
> irq 10: 02, edge
> irq 11: 02, edge
> irq 12: 02, edge
> irq 13: 02, edge
> irq 14: 02, edge
> irq 15: 02, edge
>
> Now connection also works fine with unmodified 8139too driver.
> The percentage of low-level errors stays very small:
>
> RX packets:13953 errors:1 dropped:2 overruns:1 frame:0
> TX packets:37346 errors:0 dropped:0 overruns:13 carrier:0

Ok, nice!

> This fix looks really nice. Maybe it is right thing to do.

I'll leave that up to Thomas and Maciej to decide, they should have the
best idea of why the x86 pci-irq code looks the way it does today and
what the possible risk with my patch is.

As I said before, I still think we should also merge the 8139 driver patch,
probably without that loop. It's not great, but I'm much more confident
I understand what that does and that the patched version is better than
the current code.

      Arnd

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Tue, 22 Jun 2021, Arnd Bergmann wrote:

> > This fix looks really nice. Maybe it is right thing to do.
> 
> I'll leave that up to Thomas and Maciej to decide, they should have the
> best idea of why the x86 pci-irq code looks the way it does today and
> what the possible risk with my patch is.

 Ah, so this is the SiS 85C496/497 chipset; another one that does not have 
its southbridge visible in the PCI configuration space, perhaps because it 
doesn't put the southbridge on PCI in the first place, and instead it maps 
its configuration registers in the upper half of the northbridge's space.  
Oh, the joys of early attempts!

 It does PCI interrupt steering, it has the ELCR, but we don't have a PIRQ 
router implemented for it.  I have a datasheet, so this should be fairly 
trivial to do, and hopefully things will then work automagically, no need 
for hacks.

 It's very late tonight here, so let me come back with something tomorrow.

  Maciej

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi Maciej,

23.06.2021 4:04, Maciej W. Rozycki:
[...]
>   Ah, so this is the SiS 85C496/497 chipset; another one that does not have
> its southbridge visible in the PCI configuration space, perhaps because it
> doesn't put the southbridge on PCI in the first place, and instead it maps
> its configuration registers in the upper half of the northbridge's space.
> Oh, the joys of early attempts!

I see your patches for this chipset have now arrived and I'll test them 
as soon as I get to that box.
Meanwhile I've captured a similar log from another (and actually this 
one is my main concern because it can not be replaced easily):

https://pastebin.com/NVfRcMww

Something is clearly different here, 8259A.pl reports all irqs are edge 
no matter what. BIOS setup only offers some strange "PCI IDE IRQ mode" 
(Edge/Level) and apparently it has no effect anyway.
(A modified version of 8139too works fine though)


Thank you,

Regards,
Nikolai

>   It does PCI interrupt steering, it has the ELCR, but we don't have a PIRQ
> router implemented for it.  I have a datasheet, so this should be fairly
> trivial to do, and hopefully things will then work automagically, no need
> for hacks.
>
>   It's very late tonight here, so let me come back with something tomorrow.
>
>    Maciej
>

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

Hi Nikolai,

> Meanwhile I've captured a similar log from another (and actually this one is
> my main concern because it can not be replaced easily):
> 
> https://pastebin.com/NVfRcMww
> 
> Something is clearly different here, 8259A.pl reports all irqs are edge no
> matter what. BIOS setup only offers some strange "PCI IDE IRQ mode"
> (Edge/Level) and apparently it has no effect anyway.

 Such an option was common in BIOS configuration of the time, due to odd 
arrangements with early PCI ATA interfaces, such as being equipped with an 
ISA stub board with a connecting cable to route IRQ14/15 from an ISA slot.

 Anyway, this the ALi M1489/M1487 chipset and no datasheet is easily 
available (perhaps Nvidia, which bought the chipset part of ALi back in 
2006, has a copy, but I wouldn't bet on it).  A product brief is available 
that says the chipset supports PCI steering, but we can only guess how it 
works.

 Please try the debug patch I sent with the previous message and maybe 
it'll dump the PIRQ router, which may or may not help.  Choosing the 
CONFIG_PCI_GOBIOS option may also reveal something along with said patch.

  Maciej

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Thu, 24 Jun 2021, Maciej W. Rozycki wrote:

>  Anyway, this the ALi M1489/M1487 chipset and no datasheet is easily 
> available (perhaps Nvidia, which bought the chipset part of ALi back in 
> 2006, has a copy, but I wouldn't bet on it).  A product brief is available 
> that says the chipset supports PCI steering, but we can only guess how it 
> works.

 I have actually tracked a datasheet down now, so I'll see what I can do.  
The southbridge has a separate edge/level trigger mode control register 
for PIRQ lines in addition to the usual ELCR register, so it will require 
extra handling and the wording is not as clear as with Intel documentation 
(understandably), so it may require further experimentation.

  Maciej

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hello Maciej,

15.07.2021 2:32, Maciej W. Rozycki:
>>   Anyway, this the ALi M1489/M1487 chipset and no datasheet is easily
>> available (perhaps Nvidia, which bought the chipset part of ALi back in
>> 2006, has a copy, but I wouldn't bet on it).  A product brief is available
>> that says the chipset supports PCI steering, but we can only guess how it
>> works.
>
>   I have actually tracked a datasheet down now, so I'll see what I can do.
> The southbridge has a separate edge/level trigger mode control register
> for PIRQ lines in addition to the usual ELCR register, so it will require
> extra handling and the wording is not as clear as with Intel documentation
> (understandably), so it may require further experimentation.

Thank you very much for investigating this! Tomorrow I'll likely be able 
to pull this motherboard out and arrange it for whatever testing might 
be necessary. However I'll be in a trip for a couple of weeks soon so 
please excuse some possible delay in my replies. I'm definitely willing 
to make all efforts to get it supported.


Regards,
Nikolai


>
>    Maciej
>

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

Hello Nikolai,

> >   I have actually tracked a datasheet down now, so I'll see what I can do.
> > The southbridge has a separate edge/level trigger mode control register
> > for PIRQ lines in addition to the usual ELCR register, so it will require
> > extra handling and the wording is not as clear as with Intel documentation
> > (understandably), so it may require further experimentation.
> 
> Thank you very much for investigating this! Tomorrow I'll likely be able 
> to pull this motherboard out and arrange it for whatever testing might 
> be necessary. However I'll be in a trip for a couple of weeks soon so 
> please excuse some possible delay in my replies. I'm definitely willing 
> to make all efforts to get it supported.

 I have preliminary code ready for submission, so I only need to wrap it 
up for posting.  However it requires some additional infrastructure shared 
with other parts of the kernel, so it will be a small patch series again, 
and due to code dependencies I'll include support for another PIRQ router 
as well as some clean-ups, because they all need to be applied in order.  
With that in place we should have complete coverage for Intel chipsets.

 Moreover based on the ALi M1533 datasheet I have also tracked down I have 
noticed we have bugs in the handling for the other ALi chipsets, which I 
seem unable to satisfactorily resolve without a copy of the ALi M1563 
datasheet I don't have.  I've noticed Nvidia does continue having at least 
some ALi-related stuff online, so chances are they have retained the ALi 
legacy internally.  I have therefore requested a copy of the M1563 
datasheet and will likely extend the patch series accordingly, whether I 
do or do not receive the document.

 Stay tuned.

  Maciej

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

22.06.2021 22:26, Arnd Bergmann:
[...]
> As I said before, I still think we should also merge the 8139 driver patch,
> probably without that loop. It's not great, but I'm much more confident
> I understand what that does and that the patched version is better than
> the current code.

Yes, the 'poll' approach apparently works stable and does not cause any 
measurable performance decrease. But it would need some carefull 
cleanup/review, especially WRT locking. Now that all real event handling 
work is happening in the poll function, it still has to be protected 
against the (potentially also long-running) reset function which in 
current design can be called e.g. from a different thread due to tx 
timeout, and this does not look good, but it is a bit beyond my 
capability to arrange it better. Besides, the idea was to keep the fix 
simple and avoid a massive rework...


Thank you,

Regards,
Nikolai


>
>        Arnd
>

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Wed, 23 Jun 2021, Nikolai Zhubr wrote:

> > As I said before, I still think we should also merge the 8139 driver patch,
> > probably without that loop. It's not great, but I'm much more confident
> > I understand what that does and that the patched version is better than
> > the current code.
> 
> Yes, the 'poll' approach apparently works stable and does not cause any
> measurable performance decrease. But it would need some carefull
> cleanup/review, especially WRT locking. Now that all real event handling work
> is happening in the poll function, it still has to be protected against the
> (potentially also long-running) reset function which in current design can be
> called e.g. from a different thread due to tx timeout, and this does not look
> good, but it is a bit beyond my capability to arrange it better. Besides, the
> idea was to keep the fix simple and avoid a massive rework...

 The simplest fix is not always the right one.

 I've now posted a small patch series that adds a PCI IRQ router for the 
SiS85C497 device.  Would you please try it with your system along with the 
debug patch included below and send me the resulting bootstap log?

  Maciej
---
 arch/x86/include/asm/pci_x86.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-macro-ide/arch/x86/include/asm/pci_x86.h
===================================================================
--- linux-macro-ide.orig/arch/x86/include/asm/pci_x86.h
+++ linux-macro-ide/arch/x86/include/asm/pci_x86.h
@@ -7,7 +7,7 @@
 
 #include <linux/ioport.h>
 
-#undef DEBUG
+#define DEBUG 1
 
 #ifdef DEBUG
 #define DBG(fmt, ...) printk(fmt, ##__VA_ARGS__)

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Thu, Jun 24, 2021 at 1:39 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Wed, 23 Jun 2021, Nikolai Zhubr wrote:
>
> > > As I said before, I still think we should also merge the 8139 driver patch,
> > > probably without that loop. It's not great, but I'm much more confident
> > > I understand what that does and that the patched version is better than
> > > the current code.
> >
> > Yes, the 'poll' approach apparently works stable and does not cause any
> > measurable performance decrease. But it would need some carefull
> > cleanup/review, especially WRT locking.

Right, I forgot you saw that one WARN_ON trigger. If you enable KALLSYMS
and BUGVERBOSE, it should become obvious what that one is about.

> >  Now that all real event handling work
> > is happening in the poll function, it still has to be protected against the
> > (potentially also long-running) reset function which in current design can be
> > called e.g. from a different thread due to tx timeout, and this does not look
> > good, but it is a bit beyond my capability to arrange it better. Besides, the
> > idea was to keep the fix simple and avoid a massive rework...

Since the calls are just moved from hardirq to softirq context, it should
be possible to do this without changing the locking state of any bit, by
just making sure that irqs are disabled during the code that was part of
the hardirq handler.

If you remove the loop, you can move the spin_lock_irqsave(&tp->lock)
back down  after the rx handler.

Anything on top of that can be done as a cleanup or simplification.

>  The simplest fix is not always the right one.

Sure, but in this case, a fairly simple change can help make this driver
work on any machine that for whatever reason treats the IRQ as
edge triggered.

>  I've now posted a small patch series that adds a PCI IRQ router for the
> SiS85C497 device.

Thanks a lot, that should help avoid the same problem on
this chipset with other drivers.

        Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

24.06.2021 11:28, Arnd Bergmann:
> Right, I forgot you saw that one WARN_ON trigger. If you enable KALLSYMS
> and BUGVERBOSE, it should become obvious what that one is about.

Here is new log with a better backtrace:

https://pastebin.com/DP3dSE4w

The respective diff against regular 8139too.c is here:

https://pastebin.com/v8kA7ZmX

(Basically the same as you proposed initially, just slight difference 
might be as a remainder of my various previous testing)


Thank you,

Regards,
Nikolai

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Fri, Jul 2, 2021 at 8:53 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> 24.06.2021 11:28, Arnd Bergmann:
> > Right, I forgot you saw that one WARN_ON trigger. If you enable KALLSYMS
> > and BUGVERBOSE, it should become obvious what that one is about.
>
> Here is new log with a better backtrace:
>
> https://pastebin.com/DP3dSE4w

Ok, I think the generic code changed a bit, but it does confirm that the
problem is doing the RX handing with IRQs disabled.

> The respective diff against regular 8139too.c is here:
>
> https://pastebin.com/v8kA7ZmX
>
> (Basically the same as you proposed initially, just slight difference
> might be as a remainder of my various previous testing)

The simplest workaround would be to just move the
"spin_lock_irqsave(&tp->lock, flags);" a few lines down, below the rx
processing. This keeps the locking rules exactly how they were before
your patch, and anything beyond that could be done as a follow-up
cleanup, if someone wants to think this through more.

        Arnd

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hello Arnd,

03.07.2021 12:10, Arnd Bergmann:
> The simplest workaround would be to just move the
> "spin_lock_irqsave(&tp->lock, flags);" a few lines down, below the rx
> processing. This keeps the locking rules exactly how they were before

Indeed, moving spin_lock_irqsave below rtl8139_rx eliminated the warn_on 
message apparently, here is a new log:

https://pastebin.com/dVFNVEu4

and here is my resulting diff:

https://pastebin.com/CzNzsUPu

My usual tests run fine. However I still see 2 issues:

1. I do not understand all this locking thing enough to do a good 
cleanup myself, and it looks like it needs one;
2. It looks like in case of incorrect (edge) triggering mode, the "poll 
approach" with no loop added in the poll function would still allow a 
race window, as explained in following outline (from some previous mails):

22.06.2021 14:12, David Laight:
 > Typically you need to:
 > 1) stop the chip driving IRQ low.
 > 2) process all the completed RX and TX entries.
 > 3) clear the chip's interrupt pending bits (often write to clear).
 > 4) check for completed RX/TX entries, back to 2 if found.
 > 5) enable driving IRQ.
 >
 > The loop (4) is needed because of the timing window between
 > (2) and (3).
 > You can swap (2) and (3) over - but then you get an additional
 > interrupt if packets arrive during processing - which is common.

So in terms of such outline, the "poll approach" now implements 1, 2, 3, 
5 but still misses 4, and my understanding is that it is therefore still 
not a complete solution for the broken triggering case (Although 
practically, the time window might be too small for the race effect to 
be ever observable) From my previous testing I know that such a loop 
does not affect the perfomance too much anyway, so it seems quite safe 
to add it. Maybe I've missunderstood something though.


Thank you,

Regards,
Nikolai


> your patch, and anything beyond that could be done as a follow-up
> cleanup, if someone wants to think this through more.
>
>          Arnd
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Thu, Jul 8, 2021 at 9:21 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> Hello Arnd,
>
> 03.07.2021 12:10, Arnd Bergmann:
> > The simplest workaround would be to just move the
> > "spin_lock_irqsave(&tp->lock, flags);" a few lines down, below the rx
> > processing. This keeps the locking rules exactly how they were before
>
> Indeed, moving spin_lock_irqsave below rtl8139_rx eliminated the warn_on
> message apparently, here is a new log:
>
> https://pastebin.com/dVFNVEu4
>
> and here is my resulting diff:
>
> https://pastebin.com/CzNzsUPu

Ok, great, the patch looks good to me, and I think we should just
merge that, in addition to Maciej's bugfix for the platform.

> My usual tests run fine. However I still see 2 issues:
>
> 1. I do not understand all this locking thing enough to do a good
> cleanup myself, and it looks like it needs one;

A lot of drivers need one. With your latest patch, I'm confident enough
that it's not getting worse here and given the age of this device I don't
think the cleanup is required. It's probably possible to squeeze out a
little more performance from this device on SMP systems by
improving it, but this would still be hopeless to compete against a
$5 gigabit ethernet card.

> 2. It looks like in case of incorrect (edge) triggering mode, the "poll
> approach" with no loop added in the poll function would still allow a
> race window, as explained in following outline (from some previous mails):
>
> 22.06.2021 14:12, David Laight:
>  > Typically you need to:
>  > 1) stop the chip driving IRQ low.
>  > 2) process all the completed RX and TX entries.
>  > 3) clear the chip's interrupt pending bits (often write to clear).
>  > 4) check for completed RX/TX entries, back to 2 if found.
>  > 5) enable driving IRQ.
>  >
>  > The loop (4) is needed because of the timing window between
>  > (2) and (3).
>  > You can swap (2) and (3) over - but then you get an additional
>  > interrupt if packets arrive during processing - which is common.
>
> So in terms of such outline, the "poll approach" now implements 1, 2, 3,
> 5 but still misses 4, and my understanding is that it is therefore still
> not a complete solution for the broken triggering case (Although
> practically, the time window might be too small for the race effect to
> be ever observable) From my previous testing I know that such a loop
> does not affect the perfomance too much anyway, so it seems quite safe
> to add it. Maybe I've missunderstood something though.

The latest version of your patch already does what David explained as
the alternative: the 'ack'  (step 3) happens before processing the interrupts
(2), so you don't need step 4 for correctness. You had that in the previous
version of the patch that had the loop, and since you have experimentally
shown that it makes no significant difference to performance, I'd rather
leave it out for simplicity.

If another event becomes pending after the Ack but before the
napi_complete_done(), then we get an interrupt and call napi_schedule()
again.

For some reason, the TxErr bit is only cleared after tx processing, so
we could miss an error event, but that seems fine as the tx errors are
not handled in any way other than counting them, which is already
unreliable if multiple transmits fail before the interrupt comes.

       Arnd

RE: Realtek 8139 problem on 486.

[David Laight]

From: Nikolai Zhubr
> Sent: 08 July 2021 20:22
...
> 22.06.2021 14:12, David Laight:
>  > Typically you need to:
>  > 1) stop the chip driving IRQ low.
>  > 2) process all the completed RX and TX entries.
>  > 3) clear the chip's interrupt pending bits (often write to clear).
>  > 4) check for completed RX/TX entries, back to 2 if found.
>  > 5) enable driving IRQ.
>  >
>  > The loop (4) is needed because of the timing window between
>  > (2) and (3).
>  > You can swap (2) and (3) over - but then you get an additional
>  > interrupt if packets arrive during processing - which is common.
> 
> So in terms of such outline, the "poll approach" now implements 1, 2, 3,
> 5 but still misses 4, and my understanding is that it is therefore still
> not a complete solution for the broken triggering case (Although
> practically, the time window might be too small for the race effect to
> be ever observable) From my previous testing I know that such a loop
> does not affect the perfomance too much anyway, so it seems quite safe
> to add it. Maybe I've missunderstood something though.

The timing window (4) happens.
The next receive packet will usually clear it - but that might require a timeout.
I'm sure I got a trip to Sweden out of it.

I also think Linux requires the tx-reap be done in a timely manner.
If that is only done in response to an end of tx interrupt the delay
could be substantial.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: Realtek 8139 problem on 486.

[Maciej W. Rozycki]

On Tue, 1 Jun 2021, Heiner Kallweit wrote:

> > Alternatively, maybe it is possible to explicitely request level mode from 8259 at the driver startup?
> > 
> I have doubts that it is possible to overrule what the BIOS/ACPI communicate
> to the kernel.

 Umm, any 486 system surely predates ACPI by several years; indeed I have 
a dual Pentium MMX system from 1997 that does not have ACPI either and 
uses an MP Table to communicate interrupt line configuration.  I am fairly
sure an old UP 486 system communicates nothing.

 Since the inception of EISA the usual way for x86 systems to switch 
between classic edge-triggered and level-triggered interrupts on a line by 
line basis has been the ELCR, a 16-bit mainboard register at 0x4d0 in the 
port I/O space.  We do have support for that register, however used in 
certain circumstances only, clearly not yours.  Classic 8259A chips could 
only switch trigger modes globally across all interrupt lines.

 You might be able to add a quirk based on your chipset's vendor/device ID 
though, which would call `elcr_set_level_irq' for interrupt lines claimed 
by PCI devices.  You'd have to match on the southbridge's ID I imagine, if 
any (ISTR at least one Intel chipset did not have a southbridge visible on 
PCI), as it's where the 8259A cores along with any ELCR reside.

 It would be the right thing to do IMO; those early PCI systems often got 
these things wrong, and the selection for the trigger mode shouldn't have 
been there in the BIOS setup in the first place (the manufacturer couldn't 
obviously figure it out how to do this correctly by just scanning PCI, so 
they shifted the burden onto the end user; though I have to admit odd hw 
used to be made too, e.g. I remember seeing a PCI PATA option card with an 
extra cable and a tiny PCB stub to be plugged into the upper part of a 
spare ISA slot to get IRQ 14/15 routed from there).

 HTH,

  Maciej

Re: Realtek 8139 problem on 486.

[Nikolai Zhubr]

Hi all,

01.06.2021 20:44, Maciej W. Rozycki:
[...]
>   You might be able to add a quirk based on your chipset's vendor/device ID
> though, which would call `elcr_set_level_irq' for interrupt lines claimed
> by PCI devices.  You'd have to match on the southbridge's ID I imagine, if
> any (ISTR at least one Intel chipset did not have a southbridge visible on
> PCI), as it's where the 8259A cores along with any ELCR reside.

I'm looking at this comment in arch/x86/kernel/acpi/boot.c:

	/*
	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
	 */

Doesn't it target exactly the case in question? If so, why it does not 
actually work?

By legacy they likely mean non-ACPI IRQs, so for 486 it's just all of 
them. So I'd suppose, if the kernel readily knows a particular IRQ is 
assigned to PCI bus (I'm almost sure it does) shouldn't it already take 
care of proper triggering mode automatically? Because then there would 
be no need to add workarounds to individual drivers.


Thank you,

Regards,
Nikolai

>   It would be the right thing to do IMO; those early PCI systems often got
> these things wrong, and the selection for the trigger mode shouldn't have
> been there in the BIOS setup in the first place (the manufacturer couldn't
> obviously figure it out how to do this correctly by just scanning PCI, so
> they shifted the burden onto the end user; though I have to admit odd hw
> used to be made too, e.g. I remember seeing a PCI PATA option card with an
> extra cable and a tiny PCB stub to be plugged into the upper part of a
> spare ISA slot to get IRQ 14/15 routed from there).
>
>   HTH,
>
>    Maciej
>

Re: Realtek 8139 problem on 486.

[Arnd Bergmann]

On Wed, Jun 2, 2021 at 5:14 PM Nikolai Zhubr <zhubr.2@gmail.com> wrote:
>
> Hi all,
>
> 01.06.2021 20:44, Maciej W. Rozycki:
> [...]
> >   You might be able to add a quirk based on your chipset's vendor/device ID
> > though, which would call `elcr_set_level_irq' for interrupt lines claimed
> > by PCI devices.  You'd have to match on the southbridge's ID I imagine, if
> > any (ISTR at least one Intel chipset did not have a southbridge visible on
> > PCI), as it's where the 8259A cores along with any ELCR reside.
>
> I'm looking at this comment in arch/x86/kernel/acpi/boot.c:
>
>         /*
>          * Make sure all (legacy) PCI IRQs are set as level-triggered.
>          */
>
> Doesn't it target exactly the case in question? If so, why it does not
> actually work?
>
> By legacy they likely mean non-ACPI IRQs, so for 486 it's just all of
> them.

I think this means non-MSI interrupts

> So I'd suppose, if the kernel readily knows a particular IRQ is
> assigned to PCI bus (I'm almost sure it does) shouldn't it already take
> care of proper triggering mode automatically? Because then there would
> be no need to add workarounds to individual drivers.

Without ACPI, this code is never called, instead of acpi_pci_irq_enable(),
you use pirq_enable_irq()/pcibios_lookup_irq(). There are already
some hardware specific quirks in there, it's possible this could be
changed to address your case as well, but it already looks a bit fragile.

       Arnd

Re: Realtek 8139 problem on 486.

[Heiner Kallweit]

On 31.05.2021 18:53, Nikolai Zhubr wrote:
> Hi all,
> 
> 31.05.2021 2:17, Nikolai Zhubr:
> [...]
>> However indeed, it seems a problem was introduced with a rework of
>> interrupt handling (rtl8139_interrupt) in 2.6.3, because I have already
>> pushed all other differences from 2.6.3 to 2.6.2 and it still keeps
>> working fine.
>> My resulting minimized diff is still ~300 lines, it is too big and
>> complicated to be usefull to post here as is.
> 
> Some more input.
> 
> I was able to minimize the problematic diff to basically one screenfull, it is quite comprehencable now, and I'm including it below. It is the change in status/event handling due to a switch to NAPI that intruduced the problem.
> Now, in some more detailed tests, I observe that _receiving_ still works fine. It is _sending_ that suffers, and apparently, only when trying to send a lot at a time. In such case I see these warnings:
> 
> NETDEV WATCHDOG: eth0: transmit timed out
> eth0: link up, 100Mbps, full-duplex, lpa 0xC5E1
> 
> It looks like the queue of tx frames somehow gets messed up.
> 
> The essential diff fragment:
> ================================================
>      dev->open = rtl8139_open;
>      dev->hard_start_xmit = rtl8139_start_xmit;
> +    dev->poll = rtl8139_poll;
>      dev->weight = 64;
>      dev->stop = rtl8139_close;
>      dev->get_stats = rtl8139_get_stats;
> @@ -2015,7 +2010,7 @@
>              tp->stats.rx_bytes += pkt_size;
>              tp->stats.rx_packets++;
> 
> -            netif_rx (skb);
> +            netif_receive_skb (skb);
>          } else {
>              if (net_ratelimit())
>                  printk (KERN_WARNING
> @@ -2138,10 +2133,8 @@
>      u16 status, ackstat;
>      int link_changed = 0; /* avoid bogus "uninit" warning */
>      int handled = 0;
> -    int boguscnt = max_interrupt_work;
> 
>      spin_lock (&tp->lock);
> -    do {
>      status = RTL_R16 (IntrStatus);
> 
>      /* shared irq? */
> @@ -2169,8 +2162,14 @@
>      if (ackstat)
>          RTL_W16 (IntrStatus, ackstat);
> 
> -    if (netif_running (dev) && (status & RxAckBits))
> -        rtl8139_rx (dev, tp, 1000000000);
> +    /* Receive packets are processed by poll routine.
> +       If not running start it now. */
> +    if (status & RxAckBits){
> +        if (netif_rx_schedule_prep(dev)) {
> +            RTL_W16_F (IntrMask, rtl8139_norx_intr_mask);
> +            __netif_rx_schedule (dev);
> +        }
> +    }
> 
>      /* Check uncommon events with one test. */
>      if (unlikely(status & (PCIErr | PCSTimeout | RxUnderrun | RxErr)))
> @@ -2182,16 +2181,6 @@
>          if (status & TxErr)
>              RTL_W16 (IntrStatus, TxErr);
>      }
> -    boguscnt--;
> -    } while (boguscnt > 0);
> -
> ================================================
> 
> 
> Thank you,
> 
> Regards,
> Nikolai
> 
> 
> 
>>
>>
Issue could be related to rx and tx processing now potentially running in parallel.
I only have access to the current 8139too source code, hopefully the following
works on the old version:

In the end of rtl8139_start_xmit() there's
if ((tp->cur_tx - NUM_TX_DESC) == tp->dirty_tx)
		netif_stop_queue (dev);

Try changing this to

if (tp->cur_tx - NUM_TX_DESC == tp->dirty_tx) {
	smp_wmb();
	netif_stop_queue (dev);
	smp_mb__after_atomic();       /* if this doesn't exist yet, use mb() */
	if (tp->cur_tx - NUM_TX_DESC != tp->dirty_tx)
		netif_start_queue(dev);
}


And at the end of rtl8139_tx_interrupt() change

	if (tp->dirty_tx != dirty_tx) {
		tp->dirty_tx = dirty_tx;
		mb();
		netif_wake_queue (dev);
	}

to

	if (tp->dirty_tx != dirty_tx) {
		tp->dirty_tx = dirty_tx;
		mb();
		if (netif_queue_stopped(dev) && tp->cur_tx - NUM_TX_DESC != tp->dirty_tx)
			netif_wake_queue (dev);
	}

Re: Realtek 8139 problem on 486.

[Denis Kirjanov]

On 5/29/21, Nikolai Zhubr <zhubr.2@gmail.com> wrote:
> Hello all,
>
> I'm observing a problem with Realtek 8139 cards on a couple of 486
> boxes. The respective driver is 8139too. It starts operation
> successfully, obtains an ip address via dhcp, replies to pings steadily,
> but some subsequent communication fails apparently. At least, nfsroot is
> unusable (it gets stuck in "... not responding, still trying" forever),

What's your qdisc? Recently there was a bug related to the lockless
pfifo_fast qdisc

> and also iperf3 -c xxx when run against a neighbour box on a lan prints
> 2-3 lines with some reasonable 7Mbit/s rate, then just prints 0s and
> subsequently throws a panic about output queue full or some such.
>
> My kernel is 4.14.221 at the moment, but I can compile another if
> necessary.
> I've already tried the "#define RTL8139_DEBUG 3" and "8139TOO_PIO=y" and
> "#define RX_DMA_BURST 4" and "#define TX_DMA_BURST 4" (in case there is
> a PCI burst issue, as mentioned somewhere) and nothing changed whatsoever.
>
> Some additional notes:
> - the problem is 100% reproducable;
> - replacing this 8139 card with some entirely different 8139-based card
> changes nothing;
> - if I replace this 8139 with a (just random) intel Pro/1000 card,
> everything seem to work fine;
> - if I insert this 8139 into some other 486 motherboard (with a
> different chipset), everything seem to work fine again;
> - etherboot and pxelinux work fine.
>
> I'm willing to do some debugging but unfortunately I'm not anywhere
> familiar with this driver and network controllers in general, therefore
> I'm asking for some hints/advice first.
>
>
> Thank you,
>
> Regards,
> Nikolai
>