Re: [Intel-wired-lan] [PATCH] e1000e: prevent division by zero if TIMINCA is zero

From: Rustad, Mark D
Date: Fri May 06 2016 - 19:43:23 EST

Denys Vlasenko <dvlasenk@xxxxxxxxxx> wrote:

Users report that under VMWare, er32(TIMINCA) returns zero.
This causes division by zero at init time as follows:

==> incvalue = er32(TIMINCA) & E1000_TIMINCA_INCVALUE_MASK;
for (i = 0; i < E1000_MAX_82574_SYSTIM_REREADS; i++) {
/* latch SYSTIMH on read of SYSTIML */
systim_next = (cycle_t)er32(SYSTIML);
systim_next |= (cycle_t)er32(SYSTIMH) << 32;

time_delta = systim_next - systim;
temp = time_delta;
====> rem = do_div(temp, incvalue);

This change makes kernel survive this, and users report that
NIC does work after this change.

Since on real hardware incvalue is never zero, this should not affect
real hardware use case.

Signed-off-by: Denys Vlasenko <dvlasenk@xxxxxxxxxx>
CC: Jeff Kirsher <jeffrey.t.kirsher@xxxxxxxxx>
CC: "Ruinskiy, Dima" <dima.ruinskiy@xxxxxxxxx>
CC: intel-wired-lan@xxxxxxxxxxxxxxxx
CC: netdev@xxxxxxxxxxxxxxx
CC: LKML <linux-kernel@xxxxxxxxxxxxxxx>
---
drivers/net/ethernet/intel/e1000e/netdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 269087c..0626935 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -4315,7 +4315,8 @@ static cycle_t e1000e_cyclecounter_read(const struct cyclecounter *cc)

time_delta = systim_next - systim;
temp = time_delta;
- rem = do_div(temp, incvalue);
+ /* VMWare users have seen incvalue of zero, don't div / 0 */
+ rem = incvalue ? do_div(temp, incvalue) : (time_delta != 0);

systim = systim_next;


I seem to recall that this was rejected before because it really is VMWare's bug and, if they fix it, any existing VMs that use this will just work. Changing the driver will only fix it for vms that install a new driver. I don't object to doing it, it just seems like not the most effective place to address the issue.

--
Mark Rustad, Networking Division, Intel Corporation

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail