| Message ID | 20230214061140.36696-9-jasowang@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PULL,01/10] net: Move the code to collect available NIC models to a separate function | expand |
Adding CAN bus maintainers. On 14/2/23 07:11, Jason Wang wrote: > From: Qiang Liu <cyruscyliu@gmail.com> > > Check fifos before poping data from and pushing data into it. > > Fixes: 98e5d7a2b726 ("hw/net/can: Introduce Xilinx ZynqMP CAN controller") > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1425 > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1427 > Reported-by: Qiang Liu <cyruscyliu@gmail.com> > Signed-off-by: Qiang Liu <cyruscyliu@gmail.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > hw/net/can/xlnx-zynqmp-can.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c > index e93e6c5..55d3221 100644 > --- a/hw/net/can/xlnx-zynqmp-can.c > +++ b/hw/net/can/xlnx-zynqmp-can.c > @@ -451,6 +451,12 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo) > } > > while (!fifo32_is_empty(fifo)) { > + if (fifo32_num_used(fifo) < (4 * CAN_FRAME_SIZE)) { > + g_autofree char *path = object_get_canonical_path(OBJECT(s)); > + qemu_log_mask(LOG_GUEST_ERROR, "%s: data left in the fifo is not" > + " enough for transfer.\n", path); > + break; This change looks dubious... Shouldn't this rejected earlier? Shouldn't we assert(fifo32_num_used(fifo)) >= CAN_FRAME_SIZE here? Is this really how this works on the hardware? > for (i = 0; i < CAN_FRAME_SIZE; i++) { > data[i] = fifo32_pop(fifo); > }
diff --git a/hw/net/can/xlnx-zynqmp-can.c b/hw/net/can/xlnx-zynqmp-can.c index e93e6c5..55d3221 100644 --- a/hw/net/can/xlnx-zynqmp-can.c +++ b/hw/net/can/xlnx-zynqmp-can.c @@ -451,6 +451,12 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo) } while (!fifo32_is_empty(fifo)) { + if (fifo32_num_used(fifo) < (4 * CAN_FRAME_SIZE)) { + g_autofree char *path = object_get_canonical_path(OBJECT(s)); + qemu_log_mask(LOG_GUEST_ERROR, "%s: data left in the fifo is not" + " enough for transfer.\n", path); + break; + } for (i = 0; i < CAN_FRAME_SIZE; i++) { data[i] = fifo32_pop(fifo); } @@ -463,7 +469,8 @@ static void transfer_fifo(XlnxZynqMPCANState *s, Fifo32 *fifo) * acknowledged. The XlnxZynqMPCAN core receives any message * that it transmits. */ - if (fifo32_is_full(&s->rx_fifo)) { + if (fifo32_is_full(&s->rx_fifo) || + (fifo32_num_free(&s->rx_fifo) < (4 * CAN_FRAME_SIZE))) { ARRAY_FIELD_DP32(s->regs, INTERRUPT_STATUS_REGISTER, RXOFLW, 1); } else { for (i = 0; i < CAN_FRAME_SIZE; i++) {