qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <philmd@linaro.org>
To: qemu-devel@nongnu.org
Cc: "Anton Kochkov" <anton.kochkov@proton.me>,
	"Francisco Iglesias" <francisco.iglesias@amd.com>,
	"Vikram Garhwal" <vikram.garhwal@amd.com>,
	"Jason Wang" <jasowang@redhat.com>,
	"Pavel Pisa" <pisa@cmp.felk.cvut.cz>,
	"Vikram Garhwal" <fnu.vikram@xilinx.com>,
	"Philippe Mathieu-Daudé" <philmd@linaro.org>,
	"Qiang Liu" <cyruscyliu@gmail.com>
Subject: [PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO
Date: Sun, 19 Nov 2023 23:51:02 +0100	[thread overview]
Message-ID: <20231119225102.49227-3-philmd@linaro.org> (raw)
In-Reply-To: <20231119225102.49227-1-philmd@linaro.org>

Per https://docs.xilinx.com/r/en-US/ug1085-zynq-ultrascale-trm/Message-Format

  Message Format

  The same message format is used for RXFIFO, TXFIFO, and TXHPB.
  Each message includes four words (16 bytes). Software must read
  and write all four words regardless of the actual number of data
  bytes and valid fields in the message.

There is no mention in this reference manual about what the
hardware does when not all four words are read. To fix the
reported underflow behavior, I choose to fill the 4 frame data
registers when the first register (ID) is accessed, which is how
I expect hardware would do.

Reported-by: Qiang Liu <cyruscyliu@gmail.com>
Fixes: 98e5d7a2b7 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller")
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1427
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/can/xlnx-zynqmp-can.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
index 58938b574e..c63fb4a83c 100644
--- a/hw/net/can/xlnx-zynqmp-can.c
+++ b/hw/net/can/xlnx-zynqmp-can.c
@@ -777,14 +777,18 @@ static void update_rx_fifo(XlnxZynqMPCANState *s, const qemu_can_frame *frame)
     }
 }
 
-static uint64_t can_rxfifo_pre_read(RegisterInfo *reg, uint64_t val)
+static uint64_t can_rxfifo_post_read_id(RegisterInfo *reg, uint64_t val)
 {
     XlnxZynqMPCANState *s = XLNX_ZYNQMP_CAN(reg->opaque);
+    unsigned used = fifo32_num_used(&s->rx_fifo);
 
-    if (!fifo32_is_empty(&s->rx_fifo)) {
-        val = fifo32_pop(&s->rx_fifo);
-    } else {
+    if (used < CAN_FRAME_SIZE) {
         ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXUFLW, 1);
+    } else {
+        val = s->regs[R_RXFIFO_ID] = fifo32_pop(&s->rx_fifo);
+        s->regs[R_RXFIFO_DLC] = fifo32_pop(&s->rx_fifo);
+        s->regs[R_RXFIFO_DATA1] = fifo32_pop(&s->rx_fifo);
+        s->regs[R_RXFIFO_DATA2] = fifo32_pop(&s->rx_fifo);
     }
 
     can_update_irq(s);
@@ -945,14 +949,11 @@ static const RegisterAccessInfo can_regs_info[] = {
         .post_write = can_tx_post_write,
     },{ .name = "RXFIFO_ID",  .addr = A_RXFIFO_ID,
         .ro = 0xffffffff,
-        .post_read = can_rxfifo_pre_read,
+        .post_read = can_rxfifo_post_read_id,
     },{ .name = "RXFIFO_DLC",  .addr = A_RXFIFO_DLC,
         .rsvd = 0xfff0000,
-        .post_read = can_rxfifo_pre_read,
     },{ .name = "RXFIFO_DATA1",  .addr = A_RXFIFO_DATA1,
-        .post_read = can_rxfifo_pre_read,
     },{ .name = "RXFIFO_DATA2",  .addr = A_RXFIFO_DATA2,
-        .post_read = can_rxfifo_pre_read,
     },{ .name = "AFR",  .addr = A_AFR,
         .rsvd = 0xfffffff0,
         .post_write = can_filter_enable_post_write,
-- 
2.41.0



  parent reply	other threads:[~2023-11-19 22:52 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-19 22:51 [PATCH-for-8.2 v2 0/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping FIFOs Philippe Mathieu-Daudé
2023-11-19 22:51 ` [PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs Philippe Mathieu-Daudé
2023-11-22 19:46   ` Francisco Iglesias
2023-11-22 21:55   ` Vikram Garhwal
2023-11-19 22:51 ` Philippe Mathieu-Daudé [this message]
2023-11-22 19:45   ` [PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO Francisco Iglesias
2023-11-22 21:38     ` Vikram Garhwal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20231119225102.49227-3-philmd@linaro.org \
    --to=philmd@linaro.org \
    --cc=anton.kochkov@proton.me \
    --cc=cyruscyliu@gmail.com \
    --cc=fnu.vikram@xilinx.com \
    --cc=francisco.iglesias@amd.com \
    --cc=jasowang@redhat.com \
    --cc=pisa@cmp.felk.cvut.cz \
    --cc=qemu-devel@nongnu.org \
    --cc=vikram.garhwal@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).