All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
@ 2012-05-10  8:37 Jason Wang
  2012-05-10  8:54 ` Michael S. Tsirkin
  2012-06-04  5:48 ` Anthony Liguori
  0 siblings, 2 replies; 10+ messages in thread
From: Jason Wang @ 2012-05-10  8:37 UTC (permalink / raw)
  To: aliguori, qemu-devel, avi; +Cc: mst

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
). 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>
---
 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 */

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-10  8:37 [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode" Jason Wang
@ 2012-05-10  8:54 ` Michael S. Tsirkin
  2012-05-10  9:31   ` Jason Wang
  2012-06-04  5:48 ` Anthony Liguori
  1 sibling, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-05-10  8:54 UTC (permalink / raw)
  To: Jason Wang; +Cc: aliguori, qemu-devel, avi

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?

> 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?
What do windows drivers do? Can you check pls?

> ---
>  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 */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-10  8:54 ` Michael S. Tsirkin
@ 2012-05-10  9:31   ` Jason Wang
  2012-05-10 11:05     ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2012-05-10  9:31 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, avi

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.

>> 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.
> What do windows drivers do? Can you check pls?

Windows guest would let the card in normal mode after eeprom access.
>
>> ---
>>   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 */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-10  9:31   ` Jason Wang
@ 2012-05-10 11:05     ` Michael S. Tsirkin
  2012-05-14  5:31       ` Jason Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-05-10 11:05 UTC (permalink / raw)
  To: Jason Wang; +Cc: aliguori, qemu-devel, avi

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 ...

> >>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?

> >
> >>---
> >>  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 */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-10 11:05     ` Michael S. Tsirkin
@ 2012-05-14  5:31       ` Jason Wang
  2012-05-14  6:20         ` Michael S. Tsirkin
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2012-05-14  5:31 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, avi

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 */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-14  5:31       ` Jason Wang
@ 2012-05-14  6:20         ` Michael S. Tsirkin
  2012-05-14  7:10           ` Jason Wang
  0 siblings, 1 reply; 10+ messages in thread
From: Michael S. Tsirkin @ 2012-05-14  6:20 UTC (permalink / raw)
  To: Jason Wang; +Cc: aliguori, qemu-devel, avi

On Mon, May 14, 2012 at 01:31:22PM +0800, Jason Wang wrote:
> 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.

OK, so that's the bug, right?
And it's later cleared during normal init, but reading
eeprom will set it?

> >>>>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 */

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-14  6:20         ` Michael S. Tsirkin
@ 2012-05-14  7:10           ` Jason Wang
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Wang @ 2012-05-14  7:10 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: aliguori, qemu-devel, avi

On 05/14/2012 02:20 PM, Michael S. Tsirkin wrote:
> On Mon, May 14, 2012 at 01:31:22PM +0800, Jason Wang wrote:
>> >  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.
> OK, so that's the bug, right?
> And it's later cleared during normal init, but reading
> eeprom will set it?
>

Right, that's exactly what I mean.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-05-10  8:37 [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode" Jason Wang
  2012-05-10  8:54 ` Michael S. Tsirkin
@ 2012-06-04  5:48 ` Anthony Liguori
  2012-06-04  5:58   ` Jason Wang
  1 sibling, 1 reply; 10+ messages in thread
From: Anthony Liguori @ 2012-06-04  5:48 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, qemu-devel, avi

On 05/10/2012 04:37 PM, 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
> ). 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>

Applied.  Thanks.

Regards,

Anthony Liguori

> ---
>   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 */
>
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-06-04  5:48 ` Anthony Liguori
@ 2012-06-04  5:58   ` Jason Wang
  2012-06-04 12:28     ` Anthony Liguori
  0 siblings, 1 reply; 10+ messages in thread
From: Jason Wang @ 2012-06-04  5:58 UTC (permalink / raw)
  To: Anthony Liguori; +Cc: mst, qemu-devel, avi

On 06/04/2012 01:48 PM, Anthony Liguori wrote:
> On 05/10/2012 04:37 PM, 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
>> ). 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>
>
> Applied.  Thanks.
>
> Regards,
>
> Anthony Liguori

Hi Anthony:

Michael has some concern on this patch, so I post another patch of 
validating the rx buf instead of checking the opmode, please see 
http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02385.html. So 
maybe we can apply that or I need to send another patch on top of this 
reverting?

Thanks

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode"
  2012-06-04  5:58   ` Jason Wang
@ 2012-06-04 12:28     ` Anthony Liguori
  0 siblings, 0 replies; 10+ messages in thread
From: Anthony Liguori @ 2012-06-04 12:28 UTC (permalink / raw)
  To: Jason Wang; +Cc: mst, qemu-devel, avi

On 06/04/2012 01:58 PM, Jason Wang wrote:
> On 06/04/2012 01:48 PM, Anthony Liguori wrote:
>> On 05/10/2012 04:37 PM, 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
>>> ). 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>
>>
>> Applied. Thanks.
>>
>> Regards,
>>
>> Anthony Liguori
>
> Hi Anthony:
>
> Michael has some concern on this patch, so I post another patch of validating
> the rx buf instead of checking the opmode, please see
> http://lists.gnu.org/archive/html/qemu-devel/2012-05/msg02385.html. So maybe we
> can apply that or I need to send another patch on top of this reverting?

I'll work it all out.

Regards,

Anthony Liguori

>
> Thanks
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2012-06-04 12:29 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-10  8:37 [Qemu-devel] [PATCH] Revert "rtl8139: do the network/host communication only in normal operating mode" Jason Wang
2012-05-10  8:54 ` Michael S. Tsirkin
2012-05-10  9:31   ` Jason Wang
2012-05-10 11:05     ` Michael S. Tsirkin
2012-05-14  5:31       ` Jason Wang
2012-05-14  6:20         ` Michael S. Tsirkin
2012-05-14  7:10           ` Jason Wang
2012-06-04  5:48 ` Anthony Liguori
2012-06-04  5:58   ` Jason Wang
2012-06-04 12:28     ` Anthony Liguori

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.