Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"

[Jason Wang <jasowang@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<jasowang@redhat.com>
>>> 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 */