[PATCH-for-8.2 v2 0/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping FIFOs

[PATCH-for-8.2 v2 0/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping FIFOs

[Philippe Mathieu-Daudé]

Fix a pair of fuzzed bugs.

Patch #1 is reviewed, #2 is new.

Tested with the CAN tests from 'make check-qtest-aarch64'.

Regards,

Phil.

Philippe Mathieu-Daudé (2):
  hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs
  hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO

 hw/net/can/xlnx-zynqmp-can.c | 66 ++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 11 deletions(-)

-- 
2.41.0

[PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs

[Philippe Mathieu-Daudé]

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 written. To fix the
reported underflow behavior when DATA2 register is written,
I choose to fill the data with the previous content of the
ID / DLC / DATA1 registers, which is how I expect hardware
would do.

Note there is no hardware flag raised under such condition.

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/1425
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Francisco Iglesias <francisco.iglesias@amd.com>
---
 hw/net/can/xlnx-zynqmp-can.c | 49 +++++++++++++++++++++++++++++++++---
 1 file changed, 46 insertions(+), 3 deletions(-)

diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
index e93e6c5e19..58938b574e 100644
--- a/hw/net/can/xlnx-zynqmp-can.c
+++ b/hw/net/can/xlnx-zynqmp-can.c
@@ -434,6 +434,51 @@ static bool tx_ready_check(XlnxZynqMPCANState *s)
     return true;
 }
 
+static void read_tx_frame(XlnxZynqMPCANState *s, Fifo32 *fifo, uint32_t *data)
+{
+    unsigned used = fifo32_num_used(fifo);
+    bool is_txhpb = fifo == &s->txhpb_fifo;
+
+    assert(used > 0);
+    used %= CAN_FRAME_SIZE;
+
+    /*
+     * Frame Message Format
+     *
+     * Each frame 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.
+     * If software misbehave (not writting all four words), we use the previous
+     * registers content to initialize each missing word.
+     */
+    if (used > 0) {
+        /* ID, DLC, DATA1 missing */
+        data[0] = s->regs[is_txhpb ? R_TXHPB_ID : R_TXFIFO_ID];
+    } else {
+        data[0] = fifo32_pop(fifo);
+    }
+    if (used == 1 || used == 2) {
+        /* DLC, DATA1 missing */
+        data[1] = s->regs[is_txhpb ? R_TXHPB_DLC : R_TXFIFO_DLC];
+    } else {
+        data[1] = fifo32_pop(fifo);
+    }
+    if (used == 1) {
+        /* DATA1 missing */
+        data[2] = s->regs[is_txhpb ? R_TXHPB_DATA1 : R_TXFIFO_DATA1];
+    } else {
+        data[2] = fifo32_pop(fifo);
+    }
+    /* DATA2 triggered the transfer thus is always available */
+    data[3] = fifo32_pop(fifo);
+
+    if (used) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: Incomplete CAN frame (only %u/%u slots used)\n",
+                      TYPE_XLNX_ZYNQMP_CAN, used, CAN_FRAME_SIZE);
+    }
+}
+
 static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
 {
     qemu_can_frame frame;
@@ -451,9 +496,7 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
     }
 
     while (!fifo32_is_empty(fifo)) {
-        for (i = 0; i < CAN_FRAME_SIZE; i++) {
-            data[i] = fifo32_pop(fifo);
-        }
+        read_tx_frame(s, fifo, data);
 
         if (ARRAY_FIELD_EX32(s->regs, STATUS_REGISTER, LBACK)) {
             /*
-- 
2.41.0

Re: [PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs

[Francisco Iglesias]

On Sun, Nov 19, 2023 at 11:51:01PM +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 written. To fix the
> reported underflow behavior when DATA2 register is written,
> I choose to fill the data with the previous content of the
> ID / DLC / DATA1 registers, which is how I expect hardware
> would do.
> 
> Note there is no hardware flag raised under such condition.
> 
> 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/1425
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Francisco Iglesias <francisco.iglesias@amd.com>

-above line
+below line

Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>


> ---
>  hw/net/can/xlnx-zynqmp-can.c | 49 +++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> index e93e6c5e19..58938b574e 100644
> --- a/hw/net/can/xlnx-zynqmp-can.c
> +++ b/hw/net/can/xlnx-zynqmp-can.c
> @@ -434,6 +434,51 @@ static bool tx_ready_check(XlnxZynqMPCANState *s)
>      return true;
>  }
>  
> +static void read_tx_frame(XlnxZynqMPCANState *s, Fifo32 *fifo, uint32_t *data)
> +{
> +    unsigned used = fifo32_num_used(fifo);
> +    bool is_txhpb = fifo == &s->txhpb_fifo;
> +
> +    assert(used > 0);
> +    used %= CAN_FRAME_SIZE;
> +
> +    /*
> +     * Frame Message Format
> +     *
> +     * Each frame 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.
> +     * If software misbehave (not writting all four words), we use the previous
> +     * registers content to initialize each missing word.
> +     */
> +    if (used > 0) {
> +        /* ID, DLC, DATA1 missing */
> +        data[0] = s->regs[is_txhpb ? R_TXHPB_ID : R_TXFIFO_ID];
> +    } else {
> +        data[0] = fifo32_pop(fifo);
> +    }
> +    if (used == 1 || used == 2) {
> +        /* DLC, DATA1 missing */
> +        data[1] = s->regs[is_txhpb ? R_TXHPB_DLC : R_TXFIFO_DLC];
> +    } else {
> +        data[1] = fifo32_pop(fifo);
> +    }
> +    if (used == 1) {
> +        /* DATA1 missing */
> +        data[2] = s->regs[is_txhpb ? R_TXHPB_DATA1 : R_TXFIFO_DATA1];
> +    } else {
> +        data[2] = fifo32_pop(fifo);
> +    }
> +    /* DATA2 triggered the transfer thus is always available */
> +    data[3] = fifo32_pop(fifo);
> +
> +    if (used) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Incomplete CAN frame (only %u/%u slots used)\n",
> +                      TYPE_XLNX_ZYNQMP_CAN, used, CAN_FRAME_SIZE);
> +    }
> +}
> +
>  static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
>  {
>      qemu_can_frame frame;
> @@ -451,9 +496,7 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
>      }
>  
>      while (!fifo32_is_empty(fifo)) {
> -        for (i = 0; i < CAN_FRAME_SIZE; i++) {
> -            data[i] = fifo32_pop(fifo);
> -        }
> +        read_tx_frame(s, fifo, data);
>  
>          if (ARRAY_FIELD_EX32(s->regs, STATUS_REGISTER, LBACK)) {
>              /*
> -- 
> 2.41.0
> 

Re: [PATCH-for-8.2 v2 1/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping TX FIFOs

[Vikram Garhwal]

Hi,
On Sun, Nov 19, 2023 at 11:51:01PM +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 written. To fix the
> reported underflow behavior when DATA2 register is written,
> I choose to fill the data with the previous content of the
> ID / DLC / DATA1 registers, which is how I expect hardware
> would do.
> 
> Note there is no hardware flag raised under such condition.
> 
> 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/1425
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Francisco Iglesias <francisco.iglesias@amd.com>
> ---
>  hw/net/can/xlnx-zynqmp-can.c | 49 +++++++++++++++++++++++++++++++++---
>  1 file changed, 46 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c
> index e93e6c5e19..58938b574e 100644
> --- a/hw/net/can/xlnx-zynqmp-can.c
> +++ b/hw/net/can/xlnx-zynqmp-can.c
> @@ -434,6 +434,51 @@ static bool tx_ready_check(XlnxZynqMPCANState *s)
>      return true;
>  }
>  
> +static void read_tx_frame(XlnxZynqMPCANState *s, Fifo32 *fifo, uint32_t *data)
> +{
> +    unsigned used = fifo32_num_used(fifo);
> +    bool is_txhpb = fifo == &s->txhpb_fifo;
> +
> +    assert(used > 0);
> +    used %= CAN_FRAME_SIZE;
> +
> +    /*
> +     * Frame Message Format
> +     *
> +     * Each frame 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.
> +     * If software misbehave (not writting all four words), we use the previous
%s/writting/writing
> +     * registers content to initialize each missing word.
> +     */
> +    if (used > 0) {
> +        /* ID, DLC, DATA1 missing */
Code is correct but i feel This comment is confusing. ID is missing for sure
if user > 0 but same is not the case for DLC and DATA1.

> +        data[0] = s->regs[is_txhpb ? R_TXHPB_ID : R_TXFIFO_ID];
> +    } else {
> +        data[0] = fifo32_pop(fifo);
> +    }
> +    if (used == 1 || used == 2) {
> +        /* DLC, DATA1 missing */
Same here DLC is missing for sure but DATA1 is not for used == 2.
> +        data[1] = s->regs[is_txhpb ? R_TXHPB_DLC : R_TXFIFO_DLC];
> +    } else {
> +        data[1] = fifo32_pop(fifo);
> +    }
> +    if (used == 1) {
> +        /* DATA1 missing */
May be we remove all these individual comments and write a common comment before
the first check(if(used > 0)). Something like this:
/*
 * If used is 1 then ID, DLC and DATA1 are missing.
 *
 * if used is 2 then ID and DLC are missing.
 *
 * if used is 3 then only ID is missing.
 */

Code looks correct to me. So, With above minor changes in code comments:

Reviewed-by: Vikram Garhwal <vikram.garhwal@amd.com>

> +        data[2] = s->regs[is_txhpb ? R_TXHPB_DATA1 : R_TXFIFO_DATA1];
> +    } else {
> +        data[2] = fifo32_pop(fifo);
> +    }
> +    /* DATA2 triggered the transfer thus is always available */
> +    data[3] = fifo32_pop(fifo);
> +
> +    if (used) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: Incomplete CAN frame (only %u/%u slots used)\n",
> +                      TYPE_XLNX_ZYNQMP_CAN, used, CAN_FRAME_SIZE);
> +    }
> +}
> +
>  static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
>  {
>      qemu_can_frame frame;
> @@ -451,9 +496,7 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo)
>      }
>  
>      while (!fifo32_is_empty(fifo)) {
> -        for (i = 0; i < CAN_FRAME_SIZE; i++) {
> -            data[i] = fifo32_pop(fifo);
> -        }
> +        read_tx_frame(s, fifo, data);
>  
>          if (ARRAY_FIELD_EX32(s->regs, STATUS_REGISTER, LBACK)) {
>              /*
> -- 
> 2.41.0
> 

[PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO

[Philippe Mathieu-Daudé]

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

Re: [PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO

[Francisco Iglesias]

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 <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>

Reviewed-by: Francisco Iglesias <francisco.iglesias@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
> 

Re: [PATCH-for-8.2 v2 2/2] hw/net/can/xlnx-zynqmp: Avoid underflow while popping RX FIFO

[Vikram Garhwal]

On Wed, Nov 22, 2023 at 08:45:56PM +0100, Francisco Iglesias wrote:
> 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 <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>
> 
> Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
Reviewed-by: Vikram Garhwal <vikram.garhwal@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
> >