[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1353531637.26346.179.camel@shinybook.infradead.org>
Date: Wed, 21 Nov 2012 21:00:37 +0000
From: David Woodhouse <dwmw2@...radead.org>
To: Jeff Garzik <jgarzik@...ox.com>
Cc: Ben Hutchings <bhutchings@...arflare.com>,
Jason Wang <jasowang@...hat.com>,
"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: 8139cp: set ring address before enabling receiver
On Wed, 2012-11-21 at 15:40 -0500, Jeff Garzik wrote:
> A larger point is that the commit was created to avoid imagined
> disaster on simulated hardware...
In their defence, I suspect that qemu/kvm is probably now the most
common type of RTL8139 on Linux deployments :)
And since KVM is now capable of supporting an IOMMU, which most *real*
boxes with RTL8139 won't have, it was probably a *real* problem rather
than just an imagined one.
> ...and wound up creating behavior that is (a) contra to the data
> sheet and (b) breaks real hardware.
And again in their defence... the data sheet does appear to be
suggesting something completely stupid. The patch I just submitted
doesn't do what the data sheet says *either*, although I did at least
test it on real hardware. How many versions of the 8139C+ are there?
Should I be looking for more testing on different revisions?
I had a quick play with the Cfg9346 register. I note that when you set
it to 0x80 it *does* disable both network and bus mastering... and we
set it to the 'Write Enable' value 0xC0 while we're configuring
everything. I wondered if that might perhaps be the thing that made the
original behaviour, and the recommendation in the data sheet, sane.
But it doesn't disable operation when it's in the "Unlock" mode. I tried
setting the driver's value of Cfg9346_Lock to 0xC0 (i.e. leave it
write-enabled at all times), hoping that it would then fail to do any
DMA and prove that the original code was actually safe after all. But
the driver is working fine.
--
dwmw2
Download attachment "smime.p7s" of type "application/x-pkcs7-signature" (6171 bytes)
Powered by blists - more mailing lists