Revision 1.136 introduced two features that may cause undefined

behaviour of the hardware: a possibly reserved bit of the receive
descriptor (RFA) `status' field is borrowed to record no-resource
(RNR) events, and the same status field is read and written to at
a time that may clash with the hardware updating this field.

There is no hardware documentation available to determine if these
things are safe to do; the second issue almost certainly isn't, and
the first is only safe if there is documentation saying that this
bit is free to be used by the driver. The PR referenced below
provides extremely convincing evidence that the changes cause random
crashes on some (unusual) hardware.

Since these features are only required by the DEVICE_POLLING case,
this commit makes their use conditional on that option. It does not
change the DEVICE_POLLING case, but at least people with the rare
hardware on which this code causes problems can now avoid the crashes
by not enabling DEVICE_POLLING.

PR:		kern/42260
Reviewed by:	luigi
Problem revision found by: Pawel Malachowski <pawmal@unia.3lo.lublin.pl>
Tested by:	Pawel Malachowski <pawmal@unia.3lo.lublin.pl>
MFC after:	1 week

1 files changed, 14 insertions, 1 deletions

diff --git a/sys/dev/fxp/if_fxp.c b/sys/dev/fxp/if_fxp.c
index f1588c9..63b2b9e 100644
--- a/sys/dev/fxp/if_fxp.c
+++ b/sys/dev/fxp/if_fxp.c
@@ -1288,15 +1288,22 @@ fxp_intr_body(struct fxp_softc *sc, u_int8_t statack, int count)
 	 * Process receiver interrupts. If a no-resource (RNR)
 	 * condition exists, get whatever packets we can and
 	 * re-start the receiver.
+	 */
+
+#ifdef DEVICE_POLLING
+	/*
 	 * When using polling, we do not process the list to completion,
 	 * so when we get an RNR interrupt we must defer the restart
 	 * until we hit the last buffer with the C bit set.
 	 * If we run out of cycles and rfa_headm has the C bit set,
 	 * record the pending RNR in an unused status bit, so that the
 	 * info will be used in the subsequent polling cycle.
+	 *
+	 * XXX there is absolutely no guarantee that this reserved bit
+	 * will be ignored by the hardware!
 	 */
-
 #define	FXP_RFA_RNRMARK		0x4000	/* used to mark a pending RNR intr */
+#endif
 
 	for (;;) {
 		m = sc->rfa_headm;
@@ -1311,8 +1318,10 @@ fxp_intr_body(struct fxp_softc *sc, u_int8_t statack, int count)
 		if ( (rfa->rfa_status & FXP_RFA_STATUS_C) == 0)
 			break;
 
+#ifdef DEVICE_POLLING
 		if (rfa->rfa_status & FXP_RFA_RNRMARK)
 			rnr = 1;
+#endif
 		/*
 		 * Remove first packet from the chain.
 		 */
@@ -1347,15 +1356,19 @@ fxp_intr_body(struct fxp_softc *sc, u_int8_t statack, int count)
 		}
 	}
 	if (rnr) {
+#ifdef DEVICE_POLLING
 		if (rfa->rfa_status & FXP_RFA_STATUS_C)
 			rfa->rfa_status |= FXP_RFA_RNRMARK;
 		else {
+#endif
 			fxp_scb_wait(sc);
 			CSR_WRITE_4(sc, FXP_CSR_SCB_GENERAL,
 			    vtophys(sc->rfa_headm->m_ext.ext_buf) +
 				RFA_ALIGNMENT_FUDGE);
 			fxp_scb_cmd(sc, FXP_SCB_COMMAND_RU_START);
+#ifdef DEVICE_POLLING
 		}
+#endif
 	}
 }