From e87936ea299d8204e496b5ff19ffdca46c21610e Mon Sep 17 00:00:00 2001 From: Cindy Lu Date: Fri, 25 Sep 2020 23:13:33 +0800 Subject: [PATCH 01/17] virtio-net: Set mac address to hardware if the peer is vdpa If the peer's type is vdpa, we need to set the mac address to hardware in virtio_net_device_realize, Signed-off-by: Cindy Lu Signed-off-by: Jason Wang --- hw/net/virtio-net.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c index 277289d56e..9179013ac4 100644 --- a/hw/net/virtio-net.c +++ b/hw/net/virtio-net.c @@ -3395,6 +3395,12 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp) nc = qemu_get_queue(n->nic); nc->rxfilter_notify_enabled = 1; + if (nc->peer && nc->peer->info->type == NET_CLIENT_DRIVER_VHOST_VDPA) { + struct virtio_net_config netcfg = {}; + memcpy(&netcfg.mac, &n->nic_conf.macaddr, ETH_ALEN); + vhost_net_set_config(get_vhost_net(nc->peer), + (uint8_t *)&netcfg, 0, ETH_ALEN, VHOST_SET_CONFIG_TYPE_MASTER); + } QTAILQ_INIT(&n->rsc_chains); n->qdev = dev; From b492a4b8cad9977334fa4c80983e686184d6bb30 Mon Sep 17 00:00:00 2001 From: Pan Nengyuan Date: Fri, 16 Oct 2020 13:51:59 +0800 Subject: [PATCH 02/17] net/filter-rewriter: destroy g_hash_table in colo_rewriter_cleanup s->connection_track_table forgot to destroy in colo_rewriter_cleanup. Fix it. Reported-by: Euler Robot Signed-off-by: Pan Nengyuan Signed-off-by: Zhang Chen Reviewed-by: Zhang Chen Reviewed-by: Li Qiang Signed-off-by: Jason Wang --- net/filter-rewriter.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/filter-rewriter.c b/net/filter-rewriter.c index dc3c27a489..e063a818b7 100644 --- a/net/filter-rewriter.c +++ b/net/filter-rewriter.c @@ -381,6 +381,8 @@ static void colo_rewriter_cleanup(NetFilterState *nf) filter_rewriter_flush(nf); g_free(s->incoming_queue); } + + g_hash_table_destroy(s->connection_track_table); } static void colo_rewriter_setup(NetFilterState *nf, Error **errp) From 33609e95b206788681263b76d6649a556d064e4d Mon Sep 17 00:00:00 2001 From: "Rao, Lei" Date: Fri, 16 Oct 2020 13:52:00 +0800 Subject: [PATCH 03/17] Optimize seq_sorter function for colo-compare The seq of tcp has been filled in fill_pkt_tcp_info, it can be used directly here. Signed-off-by: Lei Rao Signed-off-by: Zhang Chen Reviewed-by: Li Zhijian Reviewed-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 3a45d64175..a35c10fb59 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -194,13 +194,10 @@ static void colo_compare_inconsistency_notify(CompareState *s) } } +/* Use restricted to colo_insert_packet() */ static gint seq_sorter(Packet *a, Packet *b, gpointer data) { - struct tcp_hdr *atcp, *btcp; - - atcp = (struct tcp_hdr *)(a->transport_header); - btcp = (struct tcp_hdr *)(b->transport_header); - return ntohl(atcp->th_seq) - ntohl(btcp->th_seq); + return a->tcp_seq - b->tcp_seq; } static void fill_pkt_tcp_info(void *data, uint32_t *max_ack) From b70cb3b4854dc4d65c89a8f6704c0f1e9d900ac3 Mon Sep 17 00:00:00 2001 From: "Rao, Lei" Date: Fri, 16 Oct 2020 13:52:01 +0800 Subject: [PATCH 04/17] Reduce the time of checkpoint for COLO we should set ram_bulk_stage to false after ram_state_init, otherwise the bitmap will be unused in migration_bitmap_find_dirty. all pages in ram cache will be flushed to the ram of secondary guest for each checkpoint. Signed-off-by: Lei Rao Signed-off-by: Derek Su Signed-off-by: Zhang Chen Reviewed-by: Li Zhijian Reviewed-by: Zhang Chen Signed-off-by: Jason Wang --- migration/ram.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/migration/ram.c b/migration/ram.c index 2da2b622ab..add5396a62 100644 --- a/migration/ram.c +++ b/migration/ram.c @@ -3011,6 +3011,18 @@ static void decompress_data_with_multi_threads(QEMUFile *f, qemu_mutex_unlock(&decomp_done_lock); } + /* + * we must set ram_bulk_stage to false, otherwise in + * migation_bitmap_find_dirty the bitmap will be unused and + * all the pages in ram cache wil be flushed to the ram of + * secondary VM. + */ +static void colo_init_ram_state(void) +{ + ram_state_init(&ram_state); + ram_state->ram_bulk_stage = false; +} + /* * colo cache: this is for secondary VM, we cache the whole * memory of the secondary VM, it is need to hold the global lock @@ -3054,7 +3066,7 @@ int colo_init_ram_cache(void) } } - ram_state_init(&ram_state); + colo_init_ram_state(); return 0; } From 5647051f432b7c9b57525470b0a79a31339062d2 Mon Sep 17 00:00:00 2001 From: "Rao, Lei" Date: Fri, 16 Oct 2020 13:52:02 +0800 Subject: [PATCH 05/17] Fix the qemu crash when guest shutdown in COLO mode In COLO mode, if the startup parameters of QEMU include "no-shutdown", QEMU will crash when the guest shutdown. The root cause is when the guest shutdown, the state of VM will switch COLO to SHUTDOWN. When do checkpoint again, the state will be changed to COLO. But the state switch is undefined in runstate_transitions_def, we should add it. This patch fixes the following: qemu-system-x86_64: invalid runstate transition: 'shutdown' -> 'colo' Aborted Signed-off-by: Lei Rao Signed-off-by: Zhang Chen Reviewed-by: Zhang Chen Signed-off-by: Jason Wang --- softmmu/vl.c | 1 + 1 file changed, 1 insertion(+) diff --git a/softmmu/vl.c b/softmmu/vl.c index a71164494e..e32fd48f14 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -632,6 +632,7 @@ static const RunStateTransition runstate_transitions_def[] = { { RUN_STATE_SHUTDOWN, RUN_STATE_PAUSED }, { RUN_STATE_SHUTDOWN, RUN_STATE_FINISH_MIGRATE }, { RUN_STATE_SHUTDOWN, RUN_STATE_PRELAUNCH }, + { RUN_STATE_SHUTDOWN, RUN_STATE_COLO }, { RUN_STATE_DEBUG, RUN_STATE_SUSPENDED }, { RUN_STATE_RUNNING, RUN_STATE_SUSPENDED }, From 862ee1e07e9d4dc97263fc919cb76364a2b6d193 Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Fri, 16 Oct 2020 13:52:03 +0800 Subject: [PATCH 06/17] colo-compare: fix missing compare_seq initialization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: f449c9e549c ("colo: compare the packet based on the tcp sequence number") Signed-off-by: Li Zhijian Signed-off-by: Zhang Chen Reviewed-by: Zhang Chen Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- net/colo.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/net/colo.c b/net/colo.c index a6c66d829a..ef00609848 100644 --- a/net/colo.c +++ b/net/colo.c @@ -133,14 +133,11 @@ void reverse_connection_key(ConnectionKey *key) Connection *connection_new(ConnectionKey *key) { - Connection *conn = g_slice_new(Connection); + Connection *conn = g_slice_new0(Connection); conn->ip_proto = key->ip_proto; conn->processing = false; - conn->offset = 0; conn->tcp_state = TCPS_CLOSED; - conn->pack = 0; - conn->sack = 0; g_queue_init(&conn->primary_list); g_queue_init(&conn->secondary_list); From 45b9e8c33a844c80d6067a3271652af5654ba7bd Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Fri, 16 Oct 2020 13:52:04 +0800 Subject: [PATCH 07/17] colo-compare: check mark in mutual exclusion Signed-off-by: Li Zhijian Signed-off-by: Zhang Chen Reviewed-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index a35c10fb59..8d476bbd99 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -477,13 +477,11 @@ sec: colo_release_primary_pkt(s, ppkt); g_queue_push_head(&conn->secondary_list, spkt); goto pri; - } - if (mark == COLO_COMPARE_FREE_SECONDARY) { + } else if (mark == COLO_COMPARE_FREE_SECONDARY) { conn->compare_seq = spkt->seq_end; packet_destroy(spkt, NULL); goto sec; - } - if (mark == (COLO_COMPARE_FREE_PRIMARY | COLO_COMPARE_FREE_SECONDARY)) { + } else if (mark == (COLO_COMPARE_FREE_PRIMARY | COLO_COMPARE_FREE_SECONDARY)) { conn->compare_seq = ppkt->seq_end; colo_release_primary_pkt(s, ppkt); packet_destroy(spkt, NULL); From 0c4266ef2690312512512ad6f4e44b5ac1d44c0c Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Fri, 16 Oct 2020 13:52:05 +0800 Subject: [PATCH 08/17] net/colo-compare.c: Fix compare_timeout format issue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This parameter need compare with the return of qemu_clock_get_ms(), it is uint64_t. So we need fix this issue here. Fixes: 9cc43c94b31 ("net/colo-compare.c: Expose "compare_timeout" to users") Reported-by: Derek Su Signed-off-by: Zhang Chen Reviewed-by: Li Zhijian Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- net/colo-compare.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 8d476bbd99..76b83a9ca0 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -120,7 +120,7 @@ struct CompareState { SendCo out_sendco; SendCo notify_sendco; bool vnet_hdr; - uint32_t compare_timeout; + uint64_t compare_timeout; uint32_t expired_scan_cycle; /* @@ -1076,9 +1076,9 @@ static void compare_get_timeout(Object *obj, Visitor *v, Error **errp) { CompareState *s = COLO_COMPARE(obj); - uint32_t value = s->compare_timeout; + uint64_t value = s->compare_timeout; - visit_type_uint32(v, name, &value, errp); + visit_type_uint64(v, name, &value, errp); } static void compare_set_timeout(Object *obj, Visitor *v, @@ -1141,9 +1141,9 @@ static void set_max_queue_size(Object *obj, Visitor *v, Error **errp) { Error *local_err = NULL; - uint32_t value; + uint64_t value; - visit_type_uint32(v, name, &value, &local_err); + visit_type_uint64(v, name, &value, &local_err); if (local_err) { goto out; } @@ -1391,7 +1391,7 @@ static void colo_compare_init(Object *obj) object_property_add_str(obj, "notify_dev", compare_get_notify_dev, compare_set_notify_dev); - object_property_add(obj, "compare_timeout", "uint32", + object_property_add(obj, "compare_timeout", "uint64", compare_get_timeout, compare_set_timeout, NULL, NULL); From ec081984f4ae7017e30f58599be54271e1b66d29 Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Fri, 16 Oct 2020 13:52:06 +0800 Subject: [PATCH 09/17] net/colo-compare.c: Change the timer clock type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The virtual clock only runs during the emulation. It stops when the virtual machine is stopped. The host clock should be used for device models that emulate accurate real time sources. It will continue to run when the virtual machine is suspended. COLO need to know the host time here. Fixes: dd321ecfc2e ("colo-compare: Use IOThread to Check old packet regularly and Process packets of the primary") Reported-by: Derek Su Signed-off-by: Zhang Chen Reviewed-by: Li Zhijian Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Jason Wang --- net/colo-compare.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 76b83a9ca0..1263203e7f 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -900,7 +900,7 @@ static void check_old_packet_regular(void *opaque) /* if have old packet we will notify checkpoint */ colo_old_packet_check(s); - timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + + timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) + s->expired_scan_cycle); } @@ -934,10 +934,10 @@ static void colo_compare_timer_init(CompareState *s) { AioContext *ctx = iothread_get_aio_context(s->iothread); - s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_VIRTUAL, + s->packet_check_timer = aio_timer_new(ctx, QEMU_CLOCK_HOST, SCALE_MS, check_old_packet_regular, s); - timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) + + timer_mod(s->packet_check_timer, qemu_clock_get_ms(QEMU_CLOCK_HOST) + s->expired_scan_cycle); } From 17475df2c1f5dc5b9a4fecd5997765897782832e Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Fri, 16 Oct 2020 13:52:07 +0800 Subject: [PATCH 10/17] net/colo-compare.c: Add secondary old packet detection Detect queued secondary packet to sync VM state in time. Signed-off-by: Zhang Chen Reviewed-by: Li Zhijian Signed-off-by: Jason Wang --- net/colo-compare.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 1263203e7f..0c87fd9e33 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -636,19 +636,26 @@ void colo_compare_unregister_notifier(Notifier *notify) static int colo_old_packet_check_one_conn(Connection *conn, CompareState *s) { - GList *result = NULL; + if (!g_queue_is_empty(&conn->primary_list)) { + if (g_queue_find_custom(&conn->primary_list, + &s->compare_timeout, + (GCompareFunc)colo_old_packet_check_one)) + goto out; + } - result = g_queue_find_custom(&conn->primary_list, - &s->compare_timeout, - (GCompareFunc)colo_old_packet_check_one); - - if (result) { - /* Do checkpoint will flush old packet */ - colo_compare_inconsistency_notify(s); - return 0; + if (!g_queue_is_empty(&conn->secondary_list)) { + if (g_queue_find_custom(&conn->secondary_list, + &s->compare_timeout, + (GCompareFunc)colo_old_packet_check_one)) + goto out; } return 1; + +out: + /* Do checkpoint will flush old packet */ + colo_compare_inconsistency_notify(s); + return 0; } /* From 2f2fcff323349b6d4ffe00e897f8efb507f071b9 Mon Sep 17 00:00:00 2001 From: Zhang Chen Date: Fri, 16 Oct 2020 13:52:08 +0800 Subject: [PATCH 11/17] net/colo-compare.c: Increase default queued packet scan frequency In my test, use this default parameter looks better. Signed-off-by: Zhang Chen Signed-off-by: Jason Wang --- net/colo-compare.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/colo-compare.c b/net/colo-compare.c index 0c87fd9e33..337025b44f 100644 --- a/net/colo-compare.c +++ b/net/colo-compare.c @@ -52,7 +52,7 @@ static NotifierList colo_compare_notifiers = #define COLO_COMPARE_FREE_PRIMARY 0x01 #define COLO_COMPARE_FREE_SECONDARY 0x02 -#define REGULAR_PACKET_CHECK_MS 3000 +#define REGULAR_PACKET_CHECK_MS 1000 #define DEFAULT_TIME_OUT_MS 3000 /* #define DEBUG_COLO_PACKETS */ From 7564bf7701f00214cdc8a678a9f7df765244def1 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 21 Oct 2020 11:35:50 +0530 Subject: [PATCH 12/17] net: remove an assert call in eth_get_gso_type eth_get_gso_type() routine returns segmentation offload type based on L3 protocol type. It calls g_assert_not_reached if L3 protocol is unknown, making the following return statement unreachable. Remove the g_assert call, it maybe triggered by a guest user. Reported-by: Gaoning Pan Signed-off-by: Prasad J Pandit Signed-off-by: Jason Wang --- net/eth.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/eth.c b/net/eth.c index 0c1d413ee2..1e0821c5f8 100644 --- a/net/eth.c +++ b/net/eth.c @@ -16,6 +16,7 @@ */ #include "qemu/osdep.h" +#include "qemu/log.h" #include "net/eth.h" #include "net/checksum.h" #include "net/tap.h" @@ -71,9 +72,8 @@ eth_get_gso_type(uint16_t l3_proto, uint8_t *l3_hdr, uint8_t l4proto) return VIRTIO_NET_HDR_GSO_TCPV6 | ecn_state; } } - - /* Unsupported offload */ - g_assert_not_reached(); + qemu_log_mask(LOG_UNIMP, "%s: probably not GSO frame, " + "unknown L3 protocol: 0x%04"PRIx16"\n", __func__, l3_proto); return VIRTIO_NET_HDR_GSO_NONE | ecn_state; } From d949fe64b074af7adca1076556aaebbcfdf6932e Mon Sep 17 00:00:00 2001 From: AlexChen Date: Fri, 30 Oct 2020 10:46:55 +0800 Subject: [PATCH 13/17] net/l2tpv3: Remove redundant check in net_init_l2tpv3() The result has been checked to be NULL before, it cannot be NULL here, so the check is redundant. Remove it. Reported-by: Euler Robot Signed-off-by: AlexChen Signed-off-by: Jason Wang --- net/l2tpv3.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/net/l2tpv3.c b/net/l2tpv3.c index 55fea17c0f..e4d4218db6 100644 --- a/net/l2tpv3.c +++ b/net/l2tpv3.c @@ -655,9 +655,8 @@ int net_init_l2tpv3(const Netdev *netdev, error_setg(errp, "could not bind socket err=%i", errno); goto outerr; } - if (result) { - freeaddrinfo(result); - } + + freeaddrinfo(result); memset(&hints, 0, sizeof(hints)); @@ -686,9 +685,7 @@ int net_init_l2tpv3(const Netdev *netdev, memcpy(s->dgram_dst, result->ai_addr, result->ai_addrlen); s->dst_size = result->ai_addrlen; - if (result) { - freeaddrinfo(result); - } + freeaddrinfo(result); if (l2tpv3->has_counter && l2tpv3->counter) { s->has_counter = true; From 5e73953a276106f8e2be475cca3299748bfd1201 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 10 Nov 2020 22:52:47 +0100 Subject: [PATCH 14/17] hw/net/can/ctucan: Don't allow guest to write off end of tx_buffer The ctucan device has 4 CAN bus cores, each of which has a set of 20 32-bit registers for writing the transmitted data. The registers are however not contiguous; each core's buffers is 0x100 bytes after the last. We got the checks on the address wrong in the ctucan_mem_write() function: * the first "is addr in range at all" check allowed addr == CTUCAN_CORE_MEM_SIZE, which is actually the first byte off the end of the range * the decode of addresses into core-number plus offset in the tx buffer for that core failed to check that the offset was in range, so the guest could write off the end of the tx_buffer[] array NB: currently the values of CTUCAN_CORE_MEM_SIZE, CTUCAN_CORE_TXBUF_NUM, etc, make "buff_num >= CTUCAN_CORE_TXBUF_NUM" impossible, but we retain this as a runtime check rather than an assertion to permit those values to be changed in future (in hardware they are configurable synthesis parameters). Fix the top level check, and check the offset is within the buffer. Fixes: Coverity CID 1432874 Signed-off-by: Peter Maydell Signed-off-by: Pavel Pisa Tested-by: Pavel Pisa Signed-off-by: Jason Wang --- hw/net/can/ctucan_core.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c index d20835cd7e..8486f429d7 100644 --- a/hw/net/can/ctucan_core.c +++ b/hw/net/can/ctucan_core.c @@ -303,7 +303,7 @@ void ctucan_mem_write(CtuCanCoreState *s, hwaddr addr, uint64_t val, DPRINTF("write 0x%02llx addr 0x%02x\n", (unsigned long long)val, (unsigned int)addr); - if (addr > CTUCAN_CORE_MEM_SIZE) { + if (addr >= CTUCAN_CORE_MEM_SIZE) { return; } @@ -312,7 +312,9 @@ void ctucan_mem_write(CtuCanCoreState *s, hwaddr addr, uint64_t val, addr -= CTU_CAN_FD_TXTB1_DATA_1; buff_num = addr / CTUCAN_CORE_TXBUFF_SPAN; addr %= CTUCAN_CORE_TXBUFF_SPAN; - if (buff_num < CTUCAN_CORE_TXBUF_NUM) { + addr &= ~3; + if ((buff_num < CTUCAN_CORE_TXBUF_NUM) && + (addr < sizeof(s->tx_buffer[buff_num].data))) { uint32_t *bufp = (uint32_t *)(s->tx_buffer[buff_num].data + addr); *bufp = cpu_to_le32(val); } From e0784d8375962da584fa92be8457845f433e2ae2 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 10 Nov 2020 22:52:48 +0100 Subject: [PATCH 15/17] hw/net/can/ctucan: Avoid unused value in ctucan_send_ready_buffers() Coverity points out that in ctucan_send_ready_buffers() we set buff_st_mask = 0xf << (i * 4) inside the loop, but then we never use it before overwriting it later. The only thing we use the mask for is as part of the code that is inserting the new buff_st field into tx_status. That is more comprehensibly written using deposit32(), so do that and drop the mask variable entirely. We also update the buff_st local variable at multiple points during this function, but nothing can ever see these intermediate values, so just drop those, write the final TXT_TOK as a fixed constant value, and collapse the only remaining set/use of buff_st down into an extract32(). Fixes: Coverity CID 1432869 Signed-off-by: Peter Maydell Acked-by: Pavel Pisa Tested-by: Pavel Pisa Signed-off-by: Jason Wang --- hw/net/can/ctucan_core.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c index 8486f429d7..f49c76261c 100644 --- a/hw/net/can/ctucan_core.c +++ b/hw/net/can/ctucan_core.c @@ -240,8 +240,6 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s) uint8_t *pf; int buff2tx_idx; uint32_t tx_prio_max; - unsigned int buff_st; - uint32_t buff_st_mask; if (!s->mode_settings.s.ena) { return; @@ -256,10 +254,7 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s) for (i = 0; i < CTUCAN_CORE_TXBUF_NUM; i++) { uint32_t prio; - buff_st_mask = 0xf << (i * 4); - buff_st = (s->tx_status.u32 >> (i * 4)) & 0xf; - - if (buff_st != TXT_RDY) { + if (extract32(s->tx_status.u32, i * 4, 4) != TXT_RDY) { continue; } prio = (s->tx_priority.u32 >> (i * 4)) & 0x7; @@ -271,10 +266,7 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s) if (buff2tx_idx == -1) { break; } - buff_st_mask = 0xf << (buff2tx_idx * 4); - buff_st = (s->tx_status.u32 >> (buff2tx_idx * 4)) & 0xf; int_stat.u32 = 0; - buff_st = TXT_RDY; pf = s->tx_buffer[buff2tx_idx].data; ctucan_buff2frame(pf, &frame); s->status.s.idle = 0; @@ -283,12 +275,11 @@ static void ctucan_send_ready_buffers(CtuCanCoreState *s) s->status.s.idle = 1; s->status.s.txs = 0; s->tx_fr_ctr.s.tx_fr_ctr_val++; - buff_st = TXT_TOK; int_stat.s.txi = 1; int_stat.s.txbhci = 1; s->int_stat.u32 |= int_stat.u32 & ~s->int_mask.u32; - s->tx_status.u32 = (s->tx_status.u32 & ~buff_st_mask) | - (buff_st << (buff2tx_idx * 4)); + s->tx_status.u32 = deposit32(s->tx_status.u32, + buff2tx_idx * 4, 4, TXT_TOK); } while (1); } From 676ea985c0d13c9d39b9ead4c60005abb9ea4218 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 10 Nov 2020 22:52:49 +0100 Subject: [PATCH 16/17] hw/net/can/ctucan_core: Handle big-endian hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ctucan driver defines types for its registers which are a union of a uint32_t with a struct with bitfields for the individual fields within that register. This is a bad idea, because bitfields aren't portable. The ctu_can_fd_regs.h header works around the most glaring of the portability issues by defining the fields in two different orders depending on the setting of the __LITTLE_ENDIAN_BITFIELD define. However, in ctucan_core.h this is unconditionally set to 1, which is wrong for big-endian hosts. Set it only if HOST_WORDS_BIGENDIAN is not set. There is no need for a "have we defined it already" guard, because the only place that should set it is ctucan_core.h, which has the usual double-inclusion guard. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Acked-by: Pavel Pisa Tested-by: Pavel Pisa Signed-off-by: Jason Wang --- hw/net/can/ctucan_core.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/hw/net/can/ctucan_core.h b/hw/net/can/ctucan_core.h index f21cb1c5ec..bbc09ae067 100644 --- a/hw/net/can/ctucan_core.h +++ b/hw/net/can/ctucan_core.h @@ -31,8 +31,7 @@ #include "exec/hwaddr.h" #include "net/can_emu.h" - -#ifndef __LITTLE_ENDIAN_BITFIELD +#ifndef HOST_WORDS_BIGENDIAN #define __LITTLE_ENDIAN_BITFIELD 1 #endif From 71182187ddae5d5b17bd48464f719798321484ed Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Tue, 10 Nov 2020 22:52:50 +0100 Subject: [PATCH 17/17] hw/net/can/ctucan_core: Use stl_le_p to write to tx_buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Instead of casting an address within a uint8_t array to a uint32_t*, use stl_le_p(). This handles possibly misaligned addresses which would otherwise crash on some hosts. Signed-off-by: Peter Maydell Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Pavel Pisa Tested-by: Pavel Pisa Signed-off-by: Jason Wang --- hw/net/can/ctucan_core.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/hw/net/can/ctucan_core.c b/hw/net/can/ctucan_core.c index f49c76261c..d171c372e0 100644 --- a/hw/net/can/ctucan_core.c +++ b/hw/net/can/ctucan_core.c @@ -303,11 +303,9 @@ void ctucan_mem_write(CtuCanCoreState *s, hwaddr addr, uint64_t val, addr -= CTU_CAN_FD_TXTB1_DATA_1; buff_num = addr / CTUCAN_CORE_TXBUFF_SPAN; addr %= CTUCAN_CORE_TXBUFF_SPAN; - addr &= ~3; if ((buff_num < CTUCAN_CORE_TXBUF_NUM) && - (addr < sizeof(s->tx_buffer[buff_num].data))) { - uint32_t *bufp = (uint32_t *)(s->tx_buffer[buff_num].data + addr); - *bufp = cpu_to_le32(val); + ((addr + size) <= sizeof(s->tx_buffer[buff_num].data))) { + stn_le_p(s->tx_buffer[buff_num].data + addr, size, val); } } else { switch (addr & ~3) {