From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:34725) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STnsb-0005fJ-WA for qemu-devel@nongnu.org; Mon, 14 May 2012 01:31:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1STnsZ-0006Oz-QZ for qemu-devel@nongnu.org; Mon, 14 May 2012 01:31:33 -0400 Received: from mx1.redhat.com ([209.132.183.28]:50097) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1STnsZ-0006Og-Il for qemu-devel@nongnu.org; Mon, 14 May 2012 01:31:31 -0400 Message-ID: <4FB098AA.3060809@redhat.com> Date: Mon, 14 May 2012 13:31:22 +0800 From: Jason Wang MIME-Version: 1.0 References: <20120510083717.45992.7495.stgit@amd-6168-8-1.englab.nay.redhat.com> <20120510085453.GB8461@redhat.com> <4FAB8B0C.9070606@redhat.com> <20120510110532.GB9488@redhat.com> In-Reply-To: <20120510110532.GB9488@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode" List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: aliguori@us.ibm.com, qemu-devel@nongnu.org, avi@redhat.com On 05/10/2012 07:05 PM, Michael S. Tsirkin wrote: > On Thu, May 10, 2012 at 05:31:56PM +0800, Jason Wang wrote: >> On 05/10/2012 04:54 PM, Michael S. Tsirkin wrote: >>> On Thu, May 10, 2012 at 04:37:22PM +0800, Jason Wang wrote: >>>> This reverts commit ff71f2e8cacefae99179993204172bc65e4303df. This is because >>>> the linux 8139cp driver would leave the card in "Config Register Write Enable" >>>> mode after the eeprom were read or write ( which is unexpected in the spec >>>> ). >>> Could you show where this happens please? >> Hi Michael: >> >> According to the spec: >> """ >> Normal(0x00): RTL8139C(L)+ network/host communication mode. >> ... >> Config. Register Write Enable(0x11): Before writing to the CONFIG0, >> 1, 3, 4 registers, and bits 13, 12, 8 of BMCR (offset 62h-63h), the >> RTL8139C(L)+ must be placed in this mode. This will protect the >> RTL8139C(L)+ configuration from accidental change. >> """ >> So If I am reading it correctly, guest should place the card in >> "Normal mode" during transmission and reception. But linux driver >> would reset the mode to 11 after each read or write to the eeprom, >> see eeprom_cmd_end() in 8139cp.c. > Which version? The one I see upstream just clears chip select ... It use writeb(), so in the meantime it also set the high two bits of 9346. >>>> Also a physical 8139 card can still DMA into host memory in modes other than >>>> Normal mode, so we need revert this commit to align with the behavior of >>>> physical card. >>>> >>>> The issue of 8139cp driver should be fixed in linux seperately. >>>> >>>> Signed-off-by: Jason Wang >>> It's admittedly a bit of a hack but I thought the point is >>> to work with unmodified drivers? >> Yes, and as the physical 8139 card would still doing transmission >> and reception when it's not in normal mode, so we need revert this >> patch to let unmodified driver work. > But it *won't* work - the reason we applied this hack is > because guest memory got corrupted because of > the guest bug. eeprom writes are rare enough so this > seems like the lesser evil. > >>> What do windows drivers do? Can you check pls? >> Windows guest would let the card in normal mode after eeprom access. > Does it have the rx ring programming bug too? It does not have such a bug according to debug log. >>>> --- >>>> hw/rtl8139.c | 9 --------- >>>> 1 files changed, 0 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/hw/rtl8139.c b/hw/rtl8139.c >>>> index eb22d04..2413bc3 100644 >>>> --- a/hw/rtl8139.c >>>> +++ b/hw/rtl8139.c >>>> @@ -791,9 +791,6 @@ static int rtl8139_can_receive(VLANClientState *nc) >>>> return 1; >>>> if (!rtl8139_receiver_enabled(s)) >>>> return 1; >>>> - /* network/host communication happens only in normal mode */ >>>> - if ((s->Cfg9346& Chip9346_op_mask) != Cfg9346_Normal) >>>> - return 0; >>>> >>>> if (rtl8139_cp_receiver_enabled(s)) { >>>> /* ??? Flow control not implemented in c+ mode. >>>> @@ -836,12 +833,6 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_ >>>> return -1; >>>> } >>>> >>>> - /* check whether we are in normal mode */ >>>> - if ((s->Cfg9346& Chip9346_op_mask) != Cfg9346_Normal) { >>>> - DPRINTF("not in normal op mode\n"); >>>> - return -1; >>>> - } >>>> - >>>> /* XXX: check this */ >>>> if (s->RxConfig& AcceptAllPhys) { >>>> /* promiscuous: receive all */