* [Qemu-devel] [PATCH] i.MX: Fix FEC/ENET receive funtions
@ 2018-01-10 20:38 Jean-Christophe Dubois
2018-01-12 17:08 ` Peter Maydell
0 siblings, 1 reply; 3+ messages in thread
From: Jean-Christophe Dubois @ 2018-01-10 20:38 UTC (permalink / raw)
To: peter.maydell
Cc: Jean-Christophe Dubois, qemu-devel, fyleo45, qemu-arm,
Peter Chubb, Jason Wang
The actual imx_eth_enable_rx() function is buggy.
It updates s->regs[ENET_RDAR] after calling qemu_flush_queued_packets().
qemu_flush_queued_packets() is going to call imx_XXX_receive() which itself
is going to call imx_eth_enable_rx().
By updating s->regs[ENET_RDAR] after calling qemu_flush_queued_packets()
we end up updating the register with an outdated value which might
lead to disabling the receive function in the i.MX FEC/ENET device.
This patch change the place where the register update is done so that the
register value stays up to date and the receive function can keep
running.
Reported-by: Fyleo <fyleo45@gmail.com>
Tested-by: Fyleo <fyleo45@gmail.com>
Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
---
hw/net/imx_fec.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/hw/net/imx_fec.c b/hw/net/imx_fec.c
index 90e6ee35ba..04a5cf12f1 100644
--- a/hw/net/imx_fec.c
+++ b/hw/net/imx_fec.c
@@ -536,19 +536,16 @@ static void imx_eth_do_tx(IMXFECState *s)
static void imx_eth_enable_rx(IMXFECState *s)
{
IMXFECBufDesc bd;
- bool tmp;
imx_fec_read_bd(&bd, s->rx_descriptor);
- tmp = ((bd.flags & ENET_BD_E) != 0);
+ s->regs[ENET_RDAR] = (bd.flags & ENET_BD_E) ? ENET_RDAR_RDAR : 0;
- if (!tmp) {
+ if (!s->regs[ENET_RDAR]) {
FEC_PRINTF("RX buffer full\n");
- } else if (!s->regs[ENET_RDAR]) {
+ } else {
qemu_flush_queued_packets(qemu_get_queue(s->nic));
}
-
- s->regs[ENET_RDAR] = tmp ? ENET_RDAR_RDAR : 0;
}
static void imx_eth_reset(DeviceState *d)
@@ -806,7 +803,6 @@ static void imx_eth_write(void *opaque, hwaddr offset, uint64_t value,
case ENET_RDAR:
if (s->regs[ENET_ECR] & ENET_ECR_ETHEREN) {
if (!s->regs[index]) {
- s->regs[index] = ENET_RDAR_RDAR;
imx_eth_enable_rx(s);
}
} else {
--
2.14.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] i.MX: Fix FEC/ENET receive funtions
2018-01-10 20:38 [Qemu-devel] [PATCH] i.MX: Fix FEC/ENET receive funtions Jean-Christophe Dubois
@ 2018-01-12 17:08 ` Peter Maydell
2018-01-12 17:24 ` Jean-Christophe Dubois
0 siblings, 1 reply; 3+ messages in thread
From: Peter Maydell @ 2018-01-12 17:08 UTC (permalink / raw)
To: Jean-Christophe Dubois
Cc: QEMU Developers, fyleo45, qemu-arm, Peter Chubb, Jason Wang
On 10 January 2018 at 20:38, Jean-Christophe Dubois <jcd@tribudubois.net> wrote:
> The actual imx_eth_enable_rx() function is buggy.
>
> It updates s->regs[ENET_RDAR] after calling qemu_flush_queued_packets().
>
> qemu_flush_queued_packets() is going to call imx_XXX_receive() which itself
> is going to call imx_eth_enable_rx().
>
> By updating s->regs[ENET_RDAR] after calling qemu_flush_queued_packets()
> we end up updating the register with an outdated value which might
> lead to disabling the receive function in the i.MX FEC/ENET device.
>
> This patch change the place where the register update is done so that the
> register value stays up to date and the receive function can keep
> running.
>
> Reported-by: Fyleo <fyleo45@gmail.com>
> Tested-by: Fyleo <fyleo45@gmail.com>
> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
Could you have a look at current QEMU master, please? I think that
commit b2b012afdd9c has probably fixed this bug. (At any rate it
has changed that code so that your patch won't apply.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Qemu-devel] [PATCH] i.MX: Fix FEC/ENET receive funtions
2018-01-12 17:08 ` Peter Maydell
@ 2018-01-12 17:24 ` Jean-Christophe Dubois
0 siblings, 0 replies; 3+ messages in thread
From: Jean-Christophe Dubois @ 2018-01-12 17:24 UTC (permalink / raw)
To: Peter Maydell; +Cc: QEMU Developers, fyleo45, qemu-arm, Peter Chubb, Jason Wang
Le 2018-01-12 18:08, Peter Maydell a écrit :
> On 10 January 2018 at 20:38, Jean-Christophe Dubois
> <jcd@tribudubois.net> wrote:
>> The actual imx_eth_enable_rx() function is buggy.
>>
>> It updates s->regs[ENET_RDAR] after calling
>> qemu_flush_queued_packets().
>>
>> qemu_flush_queued_packets() is going to call imx_XXX_receive() which
>> itself
>> is going to call imx_eth_enable_rx().
>>
>> By updating s->regs[ENET_RDAR] after calling
>> qemu_flush_queued_packets()
>> we end up updating the register with an outdated value which might
>> lead to disabling the receive function in the i.MX FEC/ENET device.
>>
>> This patch change the place where the register update is done so that
>> the
>> register value stays up to date and the receive function can keep
>> running.
>>
>> Reported-by: Fyleo <fyleo45@gmail.com>
>> Tested-by: Fyleo <fyleo45@gmail.com>
>> Signed-off-by: Jean-Christophe Dubois <jcd@tribudubois.net>
>
> Could you have a look at current QEMU master, please? I think that
> commit b2b012afdd9c has probably fixed this bug. (At any rate it
> has changed that code so that your patch won't apply.)
It seems the patch (imx_fec: Refactor imx_eth_enable_rx()) only renamed
a variable (from tmp to rx_ring_full) without changing the logic. So I
don't expect the bug to be fixed in mainline.
I'll rebase and resubmit my patch.
JC
>
> thanks
> -- PMM
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-01-12 17:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-10 20:38 [Qemu-devel] [PATCH] i.MX: Fix FEC/ENET receive funtions Jean-Christophe Dubois
2018-01-12 17:08 ` Peter Maydell
2018-01-12 17:24 ` Jean-Christophe Dubois
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.