eepro100: fix simplified mode

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(-)

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

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?

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

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