Re: [Qemu-devel] Segfault using qemu-system-arm in smc91c111

[Peter Crosthwaite]

On Mon, Sep 7, 2015 at 2:21 AM, Peter Maydell <address@hidden> wrote:
> CCing the net maintainers on this thread seems like it would
> be a good idea...
>
> On 7 September 2015 at 08:47, Richard Purdie
> <address@hidden> wrote:
>> On Sun, 2015-09-06 at 17:48 -0700, Peter Crosthwaite wrote:
>>> This doesn't sound right. There are other network controllers that
>>> rely of can_receive catching all cases properly. Is this a regression?
>>> Looking at logs, I see some refactoring of QEMU net framework around
>>> June timeframe, if you rewind to QEMU 2.3 (or earlier) does the bug go
>>> away?
>>
>> I did find an interesting comment in this commit:
>>
>> http://git.qemu.org/?p=qemu.git;a=commitdiff;h=625de449fc5597f2e1aff9cb586e249e198f03c9
>>
>> """
>> Since commit 6e99c63 "net/socket: Drop net_socket_can_send" and friends,
>> net queues need to be explicitly flushed after qemu_can_send_packet()
>> returns false, because the netdev side will disable the polling of fd.
>> """
>>
>> smc91x111 is calling flush functions when it knows can_receive
>> would/should return false. I believe that is the bug here.
>>
>> I suspect the driver needs:
>>
>> * can_receive to actually return the right value

Agreed, I think we have that figured further up the thread.

>> * the locations of the flush calls to be when there is receive space
>>

So my understanding is the net layer should be able to properly handle
a spurious flush. This avoids every device model having to if
(can_receive) { flush () }, rather than just flush. There are other
enet controllers that do this, e.g. xilinx_axienet and e1000. Has this
API change in the net layer?

Regards,
Peter

>> This could explain what changed to break this and why moving the flush
>> calls works in my patch.
>
> thanks
> -- PMM