[Qemu-devel] [PATCH v1 0/2] Cadence GEM bug fixes

[Qemu-devel] [PATCH v1 0/2] Cadence GEM bug fixes

[Alistair Francis]

Alistair Francis (2):
  cadence_gem: Avoid infinite loops with a misconfigured buffer
  cadence_gem: Set the last bit when wrap is set

 hw/net/cadence_gem.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

-- 
2.7.4

[Qemu-devel] [PATCH v1 1/2] cadence_gem: Avoid infinite loops with a misconfigured buffer

[Alistair Francis]

A guest can write zero to the DMACFG resulting in an infinite loop when
it reaches the while(bytes_to_copy) loop.

To avoid this issue enforce a minimum size for the RX buffer. Hardware
does not have this enforcement and relies on the guest to set a non-zero
value.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reported-by: Li Qiang <liqiang6-s@360.cn>
Reported-by: P J P <ppandit@redhat.com>
---

 hw/net/cadence_gem.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 0346f3e..e5f3c98 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -664,6 +664,13 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
                  GEM_DMACFG_RBUFSZ_S) * GEM_DMACFG_RBUFSZ_MUL;
     bytes_to_copy = size;
 
+    /* Hardware allows a zero value here but warns against it. To avoid QEMU
+     * indefinite loops we enforce a minimum value here
+     */
+    if (rxbufsize < GEM_DMACFG_RBUFSZ_MUL) {
+        rxbufsize = GEM_DMACFG_RBUFSZ_MUL;
+    }
+
     /* Pad to minimum length. Assume FCS field is stripped, logic
      * below will increment it to the real minimum of 64 when
      * not FCS stripping
-- 
2.7.4

[Qemu-devel] [PATCH v1 2/2] cadence_gem: Set the last bit when wrap is set

[Alistair Francis]

The Cadence GEM data sheet says:
"Wrap - marks last descriptor in transmit buffer descriptor list. This
can be set for any buffer within the frame."
which seems to imply that when the wrap bit is set so is the last bit.

Previously if the wrap bit is set, but the last is not then QEMU will
enter an infinite loop.

Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reported-by: Li Qiang <liqiang6-s@360.cn>
Reported-by: P J P <ppandit@redhat.com>
---

 hw/net/cadence_gem.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index e5f3c98..8a4be1e 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -274,6 +274,11 @@ static inline unsigned tx_desc_get_last(unsigned *desc)
     return (desc[1] & DESC_1_TX_LAST) ? 1 : 0;
 }
 
+static inline void tx_desc_set_last(unsigned *desc)
+{
+    desc[1] |= DESC_1_TX_LAST;
+}
+
 static inline unsigned tx_desc_get_length(unsigned *desc)
 {
     return desc[1] & DESC_1_LENGTH;
@@ -939,6 +944,7 @@ static void gem_transmit(CadenceGEMState *s)
 
         /* read next descriptor */
         if (tx_desc_get_wrap(desc)) {
+            tx_desc_set_last(desc);
             packet_desc_addr = s->regs[GEM_TXQBASE];
         } else {
             packet_desc_addr += 8;
-- 
2.7.4

Re: [Qemu-devel] [Qemu-arm] [PATCH v1 0/2] Cadence GEM bug fixes

[Peter Maydell]

On 21 June 2016 at 21:03, Alistair Francis <alistair.francis@xilinx.com> wrote:
>
>
> Alistair Francis (2):
>   cadence_gem: Avoid infinite loops with a misconfigured buffer
>   cadence_gem: Set the last bit when wrap is set
>
>  hw/net/cadence_gem.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)



Applied to target-arm.next, thanks.

-- PMM