On Sun, Nov 19, 2023 at 11:51:02PM +0100, Philippe Mathieu-Daudé wrote:
> 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 <cyruscy...@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é <phi...@linaro.org>
Reviewed-by: Francisco Iglesias <francisco.igles...@amd.com>
> ---
> 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
>
