[Qemu-devel] [PATCH] eepro100: fix simplified mode

[Qemu-devel] [PATCH] eepro100: fix simplified mode

[initcrash]

From: Christian Schilling <initcrash@gmail.com>

A driver using simplified mode that works on real hardware
did not work in qemu.

Signed-off-by: Christian Schilling <initcrash@gmail.com>
---
 hw/eepro100.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 6279ae3..4a48372 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -774,6 +774,13 @@ static void tx_command(EEPRO100State *s)
 #if 0
         uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
 #endif
+        if (tbd_array == 0xffffffff) {
+            /* In simpliyfied mode there is no tbd_array. Instead the packet data
+             * starts right after the tcb_bytes field, and the packet size is
+             * equal to tcb_bytes */
+            tx_buffer_size = tcb_bytes;
+            tx_buffer_address = tbd_address;
+        }
         tbd_address += 8;
         TRACE(RXTX, logout
             ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
-- 
1.7.7.6

Re: [Qemu-devel] [PATCH] eepro100: fix simplified mode

[Stefan Weil]

Am 23.07.2012 11:25, schrieb initcrash@gmail.com:
> From: Christian Schilling <initcrash@gmail.com>
>
> A driver using simplified mode that works on real hardware
> did not work in qemu.
>
> Signed-off-by: Christian Schilling <initcrash@gmail.com>
> ---
>   hw/eepro100.c |    7 +++++++
>   1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 6279ae3..4a48372 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -774,6 +774,13 @@ static void tx_command(EEPRO100State *s)
>   #if 0
>           uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>   #endif
> +        if (tbd_array == 0xffffffff) {
> +            /* In simpliyfied mode there is no tbd_array. Instead the packet data
> +             * starts right after the tcb_bytes field, and the packet size is
> +             * equal to tcb_bytes */
> +            tx_buffer_size = tcb_bytes;
> +            tx_buffer_address = tbd_address;
> +        }
>           tbd_address += 8;
>           TRACE(RXTX, logout
>               ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",

Do you really think that's a trivial patch?

I have a different fix for simplified mode in my QEMU tree:

http://repo.or.cz/w/qemu/ar7.git/blob/HEAD:/hw/eepro100.c

That version is implemented according to the Intel specifications
and avoids hacks for specific guest drivers.

Maybe you can give it a try.

If it works for you, I can put the changes needed for simplified mode
in a patch for QEMU git master.

Regards,

Stefan Weil

Re: [Qemu-devel] [PATCH] eepro100: fix simplified mode

[christian schilling]

On Mon, Jul 23, 2012 at 6:28 PM, Stefan Weil <sw@weilnetz.de> wrote:
> Am 23.07.2012 11:25, schrieb initcrash@gmail.com:
>>
>> A driver using simplified mode that works on real hardware
>> did not work in qemu.
>>
>> Signed-off-by: Christian Schilling <initcrash@gmail.com>
>> ---
>>   hw/eepro100.c |    7 +++++++
>>   1 files changed, 7 insertions(+), 0 deletions(-)
>>
>
>
> Do you really think that's a trivial patch?
It's only three lines plus comments, but ok small != trivial.

>
> I have a different fix for simplified mode in my QEMU tree:
>
> http://repo.or.cz/w/qemu/ar7.git/blob/HEAD:/hw/eepro100.c
>
> That version is implemented according to the Intel specifications
> and avoids hacks for specific guest drivers.
My fix isn't a hack for a specific guest driver, but is also in
accordance with the
intel specs.

>
> Maybe you can give it a try.
I have, it does work.
Overall the code look better to me, but one thing irritates me:
The comment on line 821 contradicts the trace on 830 and 833.
in fact i don't understand the code from 820 to 340. It seems to me it should
handle extended flexible TCBs, but if it does what does the code
following line 855 do?

Re: [Qemu-devel] [PATCH] eepro100: fix simplified mode

[Paolo Bonzini]

Il 24/07/2012 09:49, christian schilling ha scritto:
> On Mon, Jul 23, 2012 at 6:28 PM, Stefan Weil <sw@weilnetz.de> wrote:
>> Am 23.07.2012 11:25, schrieb initcrash@gmail.com:
>>>
>>> A driver using simplified mode that works on real hardware
>>> did not work in qemu.
>>>
>>> Signed-off-by: Christian Schilling <initcrash@gmail.com>
>>> ---
>>>   hw/eepro100.c |    7 +++++++
>>>   1 files changed, 7 insertions(+), 0 deletions(-)
>>>
>>
>>
>> Do you really think that's a trivial patch?
> It's only three lines plus comments, but ok small != trivial.
> 
>>
>> I have a different fix for simplified mode in my QEMU tree:
>>
>> http://repo.or.cz/w/qemu/ar7.git/blob/HEAD:/hw/eepro100.c
>>
>> That version is implemented according to the Intel specifications
>> and avoids hacks for specific guest drivers.
> My fix isn't a hack for a specific guest driver, but is also in
> accordance with the
> intel specs.
> 
>>
>> Maybe you can give it a try.
> I have, it does work.
> Overall the code look better to me, but one thing irritates me:
> The comment on line 821 contradicts the trace on 830 and 833.
> in fact i don't understand the code from 820 to 340. It seems to me it should
> handle extended flexible TCBs, but if it does what does the code
> following line 855 do?

Anything new about this one-year-old patch?

Paolo

Re: [Qemu-devel] [PATCH] eepro100: fix simplified mode

[Stefan Weil]

Am 27.09.2013 12:02, schrieb Paolo Bonzini:
> Il 24/07/2012 09:49, christian schilling ha scritto:
>> On Mon, Jul 23, 2012 at 6:28 PM, Stefan Weil <sw@weilnetz.de> wrote:
>>> Am 23.07.2012 11:25, schrieb initcrash@gmail.com:
>>>> A driver using simplified mode that works on real hardware
>>>> did not work in qemu.
>>>>
>>>> Signed-off-by: Christian Schilling <initcrash@gmail.com>
>>>> ---
>>>>   hw/eepro100.c |    7 +++++++
>>>>   1 files changed, 7 insertions(+), 0 deletions(-)
>>>>
>>>
>>> Do you really think that's a trivial patch?
>> It's only three lines plus comments, but ok small != trivial.
>>
>>> I have a different fix for simplified mode in my QEMU tree:
>>>
>>> http://repo.or.cz/w/qemu/ar7.git/blob/HEAD:/hw/eepro100.c
>>>
>>> That version is implemented according to the Intel specifications
>>> and avoids hacks for specific guest drivers.
>> My fix isn't a hack for a specific guest driver, but is also in
>> accordance with the
>> intel specs.
>>
>>> Maybe you can give it a try.
>> I have, it does work.
>> Overall the code look better to me, but one thing irritates me:
>> The comment on line 821 contradicts the trace on 830 and 833.
>> in fact i don't understand the code from 820 to 340. It seems to me it should
>> handle extended flexible TCBs, but if it does what does the code
>> following line 855 do?
> Anything new about this one-year-old patch?
>
> Paolo

No, there isn't, sorry. I still have to send a patch which merges my
modifications into official QEMU.
And I also still wish a good set of test cases which can be used for all
nic emulations.

My version of eepro100.c is in http://repo.or.cz/w/qemu/ar7.git.

Regards,
Stefan