[PATCH 00/10] migration/rdma: Fixes or drops

[PATCH 00/10] migration/rdma: Fixes or drops

[Peter Xu]

This series includes some RDMA changes, either to drop some dead code, or
to harden some checks for the incoming data stream, or both.

Please have a look, this should close all RDMA recent AI reports (none of
them are real security issues), thanks.

Peter Xu (10):
  migration/rdma: Introduce RDMA_CONTROL_NUM
  migration/rdma: Remove unregister code
  migration/rdma: Stick with rdma_ prefix for all tracepoints
  migration/rdma: Drop RDMALocalBlock.is_ram_block
  migration/rdma: Drop RDMALocalBlock.unregister_bitmap
  migration/rdma: Drop RDMARegister.key.chunk
  migration/rdma: Sanity check RDMA_CONTROL_REGISTER_REQUEST on buflen
  migration/rdma: Sanity check RDMA_CONTROL_REGISTER_REQUEST chunks
  migration/rdma: Sanity check upper bound of register MR address
  migration/rdma: Sanity check compress request ranges

 migration/rdma.c       | 344 +++++++++++++----------------------------
 migration/trace-events |  83 +++++-----
 2 files changed, 144 insertions(+), 283 deletions(-)

-- 
2.54.0

[PATCH 01/10] migration/rdma: Introduce RDMA_CONTROL_NUM

[Peter Xu]

Introduce this to say how many control messages we have.  We did it because
we're going to remove the last one (RDMA_CONTROL_UNREGISTER_FINISHED) soon.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 438419d189..62a509b236 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -144,6 +144,7 @@ enum {
     RDMA_CONTROL_REGISTER_FINISHED,   /* current iteration finished */
     RDMA_CONTROL_UNREGISTER_REQUEST,  /* dynamic UN-registration */
     RDMA_CONTROL_UNREGISTER_FINISHED, /* unpinning finished */
+    RDMA_CONTROL_NUM,
 };
 
 
@@ -235,7 +236,7 @@ static const char *control_desc(unsigned int rdma_control)
         [RDMA_CONTROL_UNREGISTER_FINISHED] = "UNREGISTER FINISHED",
     };
 
-    if (rdma_control > RDMA_CONTROL_UNREGISTER_FINISHED) {
+    if (rdma_control >= RDMA_CONTROL_NUM) {
         return "??BAD CONTROL VALUE??";
     }
 
-- 
2.54.0

[PATCH 02/10] migration/rdma: Remove unregister code

[Peter Xu]

The unregister code was there since the first commit RDMA migration was
merged, but it was never functioning.  Remove the dead code.

Since the two control messages are the last ones, we don't even need to
worry about compatibility of legacy RDMA control commands, we can directly
remove the messages too.

As a side effect, this patch closes a report by removing the code
completely.

Reported-by: Tristan (@TristanInSec)
Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4003
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c       | 133 -----------------------------------------
 migration/trace-events |   7 ---
 2 files changed, 140 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 62a509b236..eae4afd6df 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -142,8 +142,6 @@ enum {
     RDMA_CONTROL_REGISTER_REQUEST,    /* dynamic page registration */
     RDMA_CONTROL_REGISTER_RESULT,     /* key to use after registration */
     RDMA_CONTROL_REGISTER_FINISHED,   /* current iteration finished */
-    RDMA_CONTROL_UNREGISTER_REQUEST,  /* dynamic UN-registration */
-    RDMA_CONTROL_UNREGISTER_FINISHED, /* unpinning finished */
     RDMA_CONTROL_NUM,
 };
 
@@ -232,8 +230,6 @@ static const char *control_desc(unsigned int rdma_control)
         [RDMA_CONTROL_REGISTER_REQUEST] = "REGISTER REQUEST",
         [RDMA_CONTROL_REGISTER_RESULT] = "REGISTER RESULT",
         [RDMA_CONTROL_REGISTER_FINISHED] = "REGISTER FINISHED",
-        [RDMA_CONTROL_UNREGISTER_REQUEST] = "UNREGISTER REQUEST",
-        [RDMA_CONTROL_UNREGISTER_FINISHED] = "UNREGISTER FINISHED",
     };
 
     if (rdma_control >= RDMA_CONTROL_NUM) {
@@ -370,9 +366,6 @@ typedef struct RDMAContext {
     int total_registrations;
     int total_writes;
 
-    int unregister_current, unregister_next;
-    uint64_t unregistrations[RDMA_SIGNALED_SEND_MAX];
-
     GHashTable *blockmap;
 
     /* the RDMAContext for return path */
@@ -1186,91 +1179,6 @@ static int qemu_rdma_reg_control(RDMAContext *rdma, int idx)
     return -1;
 }
 
-/*
- * Perform a non-optimized memory unregistration after every transfer
- * for demonstration purposes, only if pin-all is not requested.
- *
- * Potential optimizations:
- * 1. Start a new thread to run this function continuously
-        - for bit clearing
-        - and for receipt of unregister messages
- * 2. Use an LRU.
- * 3. Use workload hints.
- */
-static int qemu_rdma_unregister_waiting(RDMAContext *rdma)
-{
-    Error *err = NULL;
-
-    while (rdma->unregistrations[rdma->unregister_current]) {
-        int ret;
-        uint64_t wr_id = rdma->unregistrations[rdma->unregister_current];
-        uint64_t chunk =
-            (wr_id & RDMA_WRID_CHUNK_MASK) >> RDMA_WRID_CHUNK_SHIFT;
-        uint64_t index =
-            (wr_id & RDMA_WRID_BLOCK_MASK) >> RDMA_WRID_BLOCK_SHIFT;
-        RDMALocalBlock *block =
-            &(rdma->local_ram_blocks.block[index]);
-        RDMARegister reg = { .current_index = index };
-        RDMAControlHeader resp = { .type = RDMA_CONTROL_UNREGISTER_FINISHED,
-                                 };
-        RDMAControlHeader head = { .len = sizeof(RDMARegister),
-                                   .type = RDMA_CONTROL_UNREGISTER_REQUEST,
-                                   .repeat = 1,
-                                 };
-
-        trace_qemu_rdma_unregister_waiting_proc(chunk,
-                                                rdma->unregister_current);
-
-        rdma->unregistrations[rdma->unregister_current] = 0;
-        rdma->unregister_current++;
-
-        if (rdma->unregister_current == RDMA_SIGNALED_SEND_MAX) {
-            rdma->unregister_current = 0;
-        }
-
-
-        /*
-         * Unregistration is speculative (because migration is single-threaded
-         * and we cannot break the protocol's inifinband message ordering).
-         * Thus, if the memory is currently being used for transmission,
-         * then abort the attempt to unregister and try again
-         * later the next time a completion is received for this memory.
-         */
-        clear_bit(chunk, block->unregister_bitmap);
-
-        if (test_bit(chunk, block->transit_bitmap)) {
-            trace_qemu_rdma_unregister_waiting_inflight(chunk);
-            continue;
-        }
-
-        trace_qemu_rdma_unregister_waiting_send(chunk);
-
-        ret = ibv_dereg_mr(block->pmr[chunk]);
-        block->pmr[chunk] = NULL;
-        block->remote_keys[chunk] = 0;
-
-        if (ret != 0) {
-            error_report("unregistration chunk failed: %s",
-                         strerror(ret));
-            return -1;
-        }
-        rdma->total_registrations--;
-
-        reg.key.chunk = chunk;
-        register_to_network(rdma, &reg);
-        ret = qemu_rdma_exchange_send(rdma, &head, (uint8_t *) &reg,
-                                      &resp, NULL, NULL, &err);
-        if (ret < 0) {
-            error_report_err(err);
-            return -1;
-        }
-
-        trace_qemu_rdma_unregister_waiting_complete(chunk);
-    }
-
-    return 0;
-}
-
 static uint64_t qemu_rdma_make_wrid(uint64_t wr_id, uint64_t index,
                                          uint64_t chunk)
 {
@@ -2757,8 +2665,6 @@ static int qemu_rdma_drain_cq(RDMAContext *rdma)
         }
     }
 
-    qemu_rdma_unregister_waiting(rdma);
-
     return 0;
 }
 
@@ -3336,10 +3242,6 @@ int rdma_registration_handle(QEMUFile *f)
                                .type = RDMA_CONTROL_REGISTER_RESULT,
                                .repeat = 0,
                              };
-    RDMAControlHeader unreg_resp = { .len = 0,
-                               .type = RDMA_CONTROL_UNREGISTER_FINISHED,
-                               .repeat = 0,
-                             };
     RDMAControlHeader blocks = { .type = RDMA_CONTROL_RAM_BLOCKS_RESULT,
                                  .repeat = 1 };
     QIOChannelRDMA *rioc;
@@ -3552,41 +3454,6 @@ int rdma_registration_handle(QEMUFile *f)
             ret = qemu_rdma_post_send_control(rdma,
                             (uint8_t *) results, &reg_resp, &err);
 
-            if (ret < 0) {
-                error_report_err(err);
-                goto err;
-            }
-            break;
-        case RDMA_CONTROL_UNREGISTER_REQUEST:
-            trace_rdma_registration_handle_unregister(head.repeat);
-            unreg_resp.repeat = head.repeat;
-            registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
-
-            for (int count = 0; count < head.repeat; count++) {
-                reg = &registers[count];
-                network_to_register(reg);
-
-                trace_rdma_registration_handle_unregister_loop(count,
-                           reg->current_index, reg->key.chunk);
-
-                block = &(rdma->local_ram_blocks.block[reg->current_index]);
-
-                ret = ibv_dereg_mr(block->pmr[reg->key.chunk]);
-                block->pmr[reg->key.chunk] = NULL;
-
-                if (ret != 0) {
-                    error_report("rdma unregistration chunk failed: %s",
-                                 strerror(errno));
-                    goto err;
-                }
-
-                rdma->total_registrations--;
-
-                trace_rdma_registration_handle_unregister_success(reg->key.chunk);
-            }
-
-            ret = qemu_rdma_post_send_control(rdma, NULL, &unreg_resp, &err);
-
             if (ret < 0) {
                 error_report_err(err);
                 goto err;
diff --git a/migration/trace-events b/migration/trace-events
index af0e784535..d1fe8f1382 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -244,10 +244,6 @@ qemu_rdma_advise_mr(const char *name, uint32_t len, uint64_t addr, const char *r
 qemu_rdma_resolve_host_trying(const char *host, const char *ip) "Trying %s => %s"
 qemu_rdma_signal_unregister_append(uint64_t chunk, int pos) "Appending unregister chunk %" PRIu64 " at position %d"
 qemu_rdma_signal_unregister_already(uint64_t chunk) "Unregister chunk %" PRIu64 " already in queue"
-qemu_rdma_unregister_waiting_inflight(uint64_t chunk) "Cannot unregister inflight chunk: %" PRIu64
-qemu_rdma_unregister_waiting_proc(uint64_t chunk, int pos) "Processing unregister for chunk: %" PRIu64 " at position %d"
-qemu_rdma_unregister_waiting_send(uint64_t chunk) "Sending unregister for chunk: %" PRIu64
-qemu_rdma_unregister_waiting_complete(uint64_t chunk) "Unregister for chunk: %" PRIu64 " complete."
 qemu_rdma_write_flush(int sent) "sent total: %d"
 qemu_rdma_write_one_block(int count, int block, uint64_t chunk, uint64_t current, uint64_t len, int nb_sent, int nb_chunks) "(%d) Not clobbering: block: %d chunk %" PRIu64 " current %" PRIu64 " len %" PRIu64 " %d %d"
 qemu_rdma_write_one_post(uint64_t chunk, long addr, long remote, uint32_t len) "Posting chunk: %" PRIu64 ", addr: 0x%lx remote: 0x%lx, bytes %" PRIu32
@@ -266,9 +262,6 @@ rdma_registration_handle_ram_blocks_loop(const char *name, uint64_t offset, uint
 rdma_registration_handle_register(int requests) "%d requests"
 rdma_registration_handle_register_loop(int req, int index, uint64_t addr, uint64_t chunks) "Registration request (%d): index %d, current_addr %" PRIu64 " chunks: %" PRIu64
 rdma_registration_handle_register_rkey(int rkey) "0x%x"
-rdma_registration_handle_unregister(int requests) "%d requests"
-rdma_registration_handle_unregister_loop(int count, int index, uint64_t chunk) "Unregistration request (%d): index %d, chunk %" PRIu64
-rdma_registration_handle_unregister_success(uint64_t chunk) "%" PRIu64
 rdma_registration_handle_wait(void) ""
 rdma_registration_start(uint64_t flags) "%" PRIu64
 rdma_registration_stop(uint64_t flags) "%" PRIu64
-- 
2.54.0

[PATCH 03/10] migration/rdma: Stick with rdma_ prefix for all tracepoints

[Peter Xu]

There're a bunch of RDMA tracepoints that are prefixed with "rdma_*", most
of the rest with "qemu_rdma_*".  Align them up.  The "qemu_" prefix isn't
helping.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c       | 76 +++++++++++++++++++++---------------------
 migration/trace-events | 76 +++++++++++++++++++++---------------------
 2 files changed, 76 insertions(+), 76 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index eae4afd6df..ffae0a887f 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -631,7 +631,7 @@ static void qemu_rdma_init_ram_blocks(RDMAContext *rdma)
     memset(local, 0, sizeof *local);
     ret = foreach_not_ignored_block(qemu_rdma_init_one_block, rdma);
     assert(!ret);
-    trace_qemu_rdma_init_ram_blocks(local->nb_blocks);
+    trace_rdma_init_ram_blocks(local->nb_blocks);
     rdma->dest_blocks = g_new0(RDMADestBlock,
                                rdma->local_ram_blocks.nb_blocks);
     local->init = true;
@@ -734,11 +734,11 @@ static void qemu_rdma_dump_id(const char *who, struct ibv_context *verbs)
     struct ibv_port_attr port;
 
     if (ibv_query_port(verbs, 1, &port)) {
-        trace_qemu_rdma_dump_id_failed(who);
+        trace_rdma_dump_id_failed(who);
         return;
     }
 
-    trace_qemu_rdma_dump_id(who,
+    trace_rdma_dump_id(who,
                 verbs->device->name,
                 verbs->device->dev_name,
                 verbs->device->dev_path,
@@ -759,7 +759,7 @@ static void qemu_rdma_dump_gid(const char *who, struct rdma_cm_id *id)
     char dgid[33];
     inet_ntop(AF_INET6, &id->route.addr.addr.ibaddr.sgid, sgid, sizeof sgid);
     inet_ntop(AF_INET6, &id->route.addr.addr.ibaddr.dgid, dgid, sizeof dgid);
-    trace_qemu_rdma_dump_gid(who, sgid, dgid);
+    trace_rdma_dump_gid(who, sgid, dgid);
 }
 
 /*
@@ -809,7 +809,7 @@ static int qemu_rdma_resolve_host(RDMAContext *rdma, Error **errp)
 
         inet_ntop(e->ai_family,
             &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
-        trace_qemu_rdma_resolve_host_trying(rdma->host, ip);
+        trace_rdma_resolve_host_trying(rdma->host, ip);
 
         ret = rdma_resolve_addr(rdma->cm_id, NULL, e->ai_dst_addr,
                 RDMA_RESOLVE_TIMEOUT_MS);
@@ -998,7 +998,7 @@ static void qemu_rdma_advise_prefetch_mr(struct ibv_pd *pd, uint64_t addr,
     ret = ibv_advise_mr(pd, advice,
                         IBV_ADVISE_MR_FLAG_FLUSH, &sg_list, 1);
     /* ignore the error */
-    trace_qemu_rdma_advise_mr(name, len, addr, strerror(ret));
+    trace_rdma_advise_mr(name, len, addr, strerror(ret));
 #endif
 }
 
@@ -1029,7 +1029,7 @@ static int qemu_rdma_reg_whole_ram_blocks(RDMAContext *rdma, Error **errp)
                     ibv_reg_mr(rdma->pd,
                                local->block[i].local_host_addr,
                                local->block[i].length, access);
-                trace_qemu_rdma_register_odp_mr(local->block[i].block_name);
+                trace_rdma_register_odp_mr(local->block[i].block_name);
 
                 if (local->block[i].mr) {
                     qemu_rdma_advise_prefetch_mr(rdma->pd,
@@ -1125,7 +1125,7 @@ static int qemu_rdma_register_and_get_keys(RDMAContext *rdma,
         int access = rkey ? IBV_ACCESS_LOCAL_WRITE | IBV_ACCESS_REMOTE_WRITE :
                      0;
 
-        trace_qemu_rdma_register_and_get_keys(len, chunk_start);
+        trace_rdma_register_and_get_keys(len, chunk_start);
 
         block->pmr[chunk] = ibv_reg_mr(rdma->pd, chunk_start, len, access);
         /*
@@ -1139,7 +1139,7 @@ static int qemu_rdma_register_and_get_keys(RDMAContext *rdma,
             access |= IBV_ACCESS_ON_DEMAND;
             /* register ODP mr */
             block->pmr[chunk] = ibv_reg_mr(rdma->pd, chunk_start, len, access);
-            trace_qemu_rdma_register_odp_mr(block->block_name);
+            trace_rdma_register_odp_mr(block->block_name);
 
             if (block->pmr[chunk]) {
                 qemu_rdma_advise_prefetch_mr(rdma->pd, (uintptr_t)chunk_start,
@@ -1221,7 +1221,7 @@ static int qemu_rdma_poll(RDMAContext *rdma, struct ibv_cq *cq,
 
     if (rdma->control_ready_expected &&
         (wr_id >= RDMA_WRID_RECV_CONTROL)) {
-        trace_qemu_rdma_poll_recv(wr_id - RDMA_WRID_RECV_CONTROL, wr_id,
+        trace_rdma_poll_recv(wr_id - RDMA_WRID_RECV_CONTROL, wr_id,
                                   rdma->nb_sent);
         rdma->control_ready_expected = 0;
     }
@@ -1233,7 +1233,7 @@ static int qemu_rdma_poll(RDMAContext *rdma, struct ibv_cq *cq,
             (wc.wr_id & RDMA_WRID_BLOCK_MASK) >> RDMA_WRID_BLOCK_SHIFT;
         RDMALocalBlock *block = &(rdma->local_ram_blocks.block[index]);
 
-        trace_qemu_rdma_poll_write(wr_id, rdma->nb_sent,
+        trace_rdma_poll_write(wr_id, rdma->nb_sent,
                                    index, chunk, block->local_host_addr,
                                    (void *)(uintptr_t)block->remote_host_addr);
 
@@ -1243,7 +1243,7 @@ static int qemu_rdma_poll(RDMAContext *rdma, struct ibv_cq *cq,
             rdma->nb_sent--;
         }
     } else {
-        trace_qemu_rdma_poll_other(wr_id, rdma->nb_sent);
+        trace_rdma_poll_other(wr_id, rdma->nb_sent);
     }
 
     *wr_id_out = wc.wr_id;
@@ -1378,7 +1378,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
             break;
         }
         if (wr_id != wrid_requested) {
-            trace_qemu_rdma_block_for_wrid_miss(wrid_requested, wr_id);
+            trace_rdma_block_for_wrid_miss(wrid_requested, wr_id);
         }
     }
 
@@ -1415,7 +1415,7 @@ static int qemu_rdma_block_for_wrid(RDMAContext *rdma,
                 break;
             }
             if (wr_id != wrid_requested) {
-                trace_qemu_rdma_block_for_wrid_miss(wrid_requested, wr_id);
+                trace_rdma_block_for_wrid_miss(wrid_requested, wr_id);
             }
         }
 
@@ -1463,7 +1463,7 @@ static int qemu_rdma_post_send_control(RDMAContext *rdma, uint8_t *buf,
                                    .num_sge = 1,
                                 };
 
-    trace_qemu_rdma_post_send_control(control_desc(head->type));
+    trace_rdma_post_send_control(control_desc(head->type));
 
     /*
      * We don't actually need to do a memcpy() in here if we used
@@ -1546,10 +1546,10 @@ static int qemu_rdma_exchange_get_response(RDMAContext *rdma,
     network_to_control((void *) rdma->wr_data[idx].control);
     memcpy(head, rdma->wr_data[idx].control, sizeof(RDMAControlHeader));
 
-    trace_qemu_rdma_exchange_get_response_start(control_desc(expecting));
+    trace_rdma_exchange_get_response_start(control_desc(expecting));
 
     if (expecting == RDMA_CONTROL_NONE) {
-        trace_qemu_rdma_exchange_get_response_none(control_desc(head->type),
+        trace_rdma_exchange_get_response_none(control_desc(head->type),
                                              head->type);
     } else if (head->type != expecting || head->type == RDMA_CONTROL_ERROR) {
         error_setg(errp, "Was expecting a %s (%d) control message"
@@ -1659,14 +1659,14 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, RDMAControlHeader *head,
      */
     if (resp) {
         if (callback) {
-            trace_qemu_rdma_exchange_send_issue_callback();
+            trace_rdma_exchange_send_issue_callback();
             ret = callback(rdma, errp);
             if (ret < 0) {
                 return -1;
             }
         }
 
-        trace_qemu_rdma_exchange_send_waiting(control_desc(resp->type));
+        trace_rdma_exchange_send_waiting(control_desc(resp->type));
         ret = qemu_rdma_exchange_get_response(rdma, resp,
                                               resp->type, RDMA_WRID_DATA,
                                               errp);
@@ -1679,7 +1679,7 @@ static int qemu_rdma_exchange_send(RDMAContext *rdma, RDMAControlHeader *head,
         if (resp_idx) {
             *resp_idx = RDMA_WRID_DATA;
         }
-        trace_qemu_rdma_exchange_send_received(control_desc(resp->type));
+        trace_rdma_exchange_send_received(control_desc(resp->type));
     }
 
     rdma->control_ready_expected = 1;
@@ -1782,7 +1782,7 @@ retry:
         }
     }
 
-    trace_qemu_rdma_write_one_top(chunks + 1,
+    trace_rdma_write_one_top(chunks + 1,
                                   (chunks + 1) * chunk_size / 1024 / 1024);
 
     chunk_end = ram_chunk_end(block, chunk + chunks);
@@ -1790,7 +1790,7 @@ retry:
 
     while (test_bit(chunk, block->transit_bitmap)) {
         (void)count;
-        trace_qemu_rdma_write_one_block(count++, current_index, chunk,
+        trace_rdma_write_one_block(count++, current_index, chunk,
                 sge.addr, length, rdma->nb_sent, block->nb_chunks);
 
         ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
@@ -1823,7 +1823,7 @@ retry:
                 head.len = sizeof(comp);
                 head.type = RDMA_CONTROL_COMPRESS;
 
-                trace_qemu_rdma_write_one_zero(chunk, sge.length,
+                trace_rdma_write_one_zero(chunk, sge.length,
                                                current_index, current_addr);
 
                 compress_to_network(rdma, &comp);
@@ -1860,7 +1860,7 @@ retry:
             }
             reg.chunks = chunks;
 
-            trace_qemu_rdma_write_one_sendreg(chunk, sge.length, current_index,
+            trace_rdma_write_one_sendreg(chunk, sge.length, current_index,
                                               current_addr);
 
             register_to_network(rdma, &reg);
@@ -1883,7 +1883,7 @@ retry:
 
             network_to_result(reg_result);
 
-            trace_qemu_rdma_write_one_recvregres(block->remote_keys[chunk],
+            trace_rdma_write_one_recvregres(block->remote_keys[chunk],
                                                  reg_result->rkey, chunk);
 
             block->remote_keys[chunk] = reg_result->rkey;
@@ -1926,7 +1926,7 @@ retry:
     send_wr.wr.rdma.remote_addr = block->remote_host_addr +
                                 (current_addr - block->offset);
 
-    trace_qemu_rdma_write_one_post(chunk, sge.addr, send_wr.wr.rdma.remote_addr,
+    trace_rdma_write_one_post(chunk, sge.addr, send_wr.wr.rdma.remote_addr,
                                    sge.length);
 
     /*
@@ -1936,7 +1936,7 @@ retry:
     ret = ibv_post_send(rdma->qp, &send_wr, &bad_wr);
 
     if (ret == ENOMEM) {
-        trace_qemu_rdma_write_one_queue_full();
+        trace_rdma_write_one_queue_full();
         ret = qemu_rdma_block_for_wrid(rdma, RDMA_WRID_RDMA_WRITE, NULL);
         if (ret < 0) {
             error_setg(errp, "rdma migration: failed to make "
@@ -1993,7 +1993,7 @@ static int qemu_rdma_write_flush(RDMAContext *rdma, Error **errp)
 
     if (ret == 0) {
         rdma->nb_sent++;
-        trace_qemu_rdma_write_flush(rdma->nb_sent);
+        trace_rdma_write_flush(rdma->nb_sent);
     }
 
     rdma->current_length = 0;
@@ -2109,7 +2109,7 @@ static void qemu_rdma_cleanup(RDMAContext *rdma)
         }
 
         rdma_disconnect(rdma->cm_id);
-        trace_qemu_rdma_cleanup_disconnect();
+        trace_rdma_cleanup_disconnect();
         rdma->connected = false;
     }
 
@@ -2292,7 +2292,7 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool return_path,
      * on the source first requested the capability.
      */
     if (rdma->pin_all) {
-        trace_qemu_rdma_connect_pin_all_requested();
+        trace_rdma_connect_pin_all_requested();
         cap.flags |= RDMA_CAPABILITY_PIN_ALL;
     }
 
@@ -2343,7 +2343,7 @@ static int qemu_rdma_connect(RDMAContext *rdma, bool return_path,
         rdma->pin_all = false;
     }
 
-    trace_qemu_rdma_connect_pin_all_outcome(rdma->pin_all);
+    trace_rdma_connect_pin_all_outcome(rdma->pin_all);
 
     rdma_ack_cm_event(cm_event);
 
@@ -2412,7 +2412,7 @@ static int qemu_rdma_dest_init(RDMAContext *rdma, Error **errp)
 
         inet_ntop(e->ai_family,
             &((struct sockaddr_in *) e->ai_dst_addr)->sin_addr, ip, sizeof ip);
-        trace_qemu_rdma_dest_init_trying(rdma->host, ip);
+        trace_rdma_dest_init_trying(rdma->host, ip);
         ret = rdma_bind_addr(listen_id, e->ai_dst_addr);
         if (ret < 0) {
             continue;
@@ -2547,7 +2547,7 @@ static size_t qemu_rdma_fill(RDMAContext *rdma, uint8_t *buf,
     size_t len = 0;
 
     if (rdma->wr_data[idx].control_len) {
-        trace_qemu_rdma_fill(rdma->wr_data[idx].control_len, size);
+        trace_rdma_fill(rdma->wr_data[idx].control_len, size);
 
         len = MIN(size, rdma->wr_data[idx].control_len);
         memcpy(buf, rdma->wr_data[idx].control_curr, len);
@@ -2859,7 +2859,7 @@ static int qio_channel_rdma_close(QIOChannel *ioc,
     RDMAContext *rdmain, *rdmaout;
     struct rdma_close_rcu *rcu = g_new(struct rdma_close_rcu, 1);
 
-    trace_qemu_rdma_close();
+    trace_rdma_close();
 
     rdmain = rioc->rdmain;
     if (rdmain) {
@@ -3132,11 +3132,11 @@ static int qemu_rdma_accept(RDMAContext *rdma)
 
     rdma_ack_cm_event(cm_event);
 
-    trace_qemu_rdma_accept_pin_state(rdma->pin_all);
+    trace_rdma_accept_pin_state(rdma->pin_all);
 
     caps_to_network(&cap);
 
-    trace_qemu_rdma_accept_pin_verbsc(verbs);
+    trace_rdma_accept_pin_verbsc(verbs);
 
     if (!rdma->verbs) {
         rdma->verbs = verbs;
@@ -3727,13 +3727,13 @@ static void rdma_accept_incoming_migration(void *opaque)
     RDMAContext *rdma = opaque;
     QIOChannel *ioc;
 
-    trace_qemu_rdma_accept_incoming_migration();
+    trace_rdma_accept_incoming_migration();
     if (qemu_rdma_accept(rdma) < 0) {
         error_report("RDMA ERROR: Migration initialization failed");
         return;
     }
 
-    trace_qemu_rdma_accept_incoming_migration_accepted();
+    trace_rdma_accept_incoming_migration_accepted();
 
     if (rdma->is_return_path) {
         return;
diff --git a/migration/trace-events b/migration/trace-events
index d1fe8f1382..d0ad736c8c 100644
--- a/migration/trace-events
+++ b/migration/trace-events
@@ -214,44 +214,44 @@ migrate_global_state_post_load(const char *state) "loaded state: %s"
 migrate_global_state_pre_save(const char *state) "saved state: %s"
 
 # rdma.c
-qemu_rdma_accept_incoming_migration(void) ""
-qemu_rdma_accept_incoming_migration_accepted(void) ""
-qemu_rdma_accept_pin_state(bool pin) "%d"
-qemu_rdma_accept_pin_verbsc(void *verbs) "Verbs context after listen: %p"
-qemu_rdma_block_for_wrid_miss(uint64_t wcomp, uint64_t req) "A Wanted wrid %" PRIu64 " but got %" PRIu64
-qemu_rdma_cleanup_disconnect(void) ""
-qemu_rdma_close(void) ""
-qemu_rdma_connect_pin_all_requested(void) ""
-qemu_rdma_connect_pin_all_outcome(bool pin) "%d"
-qemu_rdma_dest_init_trying(const char *host, const char *ip) "%s => %s"
-qemu_rdma_dump_id_failed(const char *who) "%s RDMA Device opened, but can't query port information"
-qemu_rdma_dump_id(const char *who, const char *name, const char *dev_name, const char *dev_path, const char *ibdev_path, int transport, const char *transport_name) "%s RDMA Device opened: kernel name %s uverbs device name %s, infiniband_verbs class device path %s, infiniband class device path %s, transport: (%d) %s"
-qemu_rdma_dump_gid(const char *who, const char *src, const char *dst) "%s Source GID: %s, Dest GID: %s"
-qemu_rdma_exchange_get_response_start(const char *desc) "CONTROL: %s receiving..."
-qemu_rdma_exchange_get_response_none(const char *desc, int type) "Surprise: got %s (%d)"
-qemu_rdma_exchange_send_issue_callback(void) ""
-qemu_rdma_exchange_send_waiting(const char *desc) "Waiting for response %s"
-qemu_rdma_exchange_send_received(const char *desc) "Response %s received."
-qemu_rdma_fill(size_t control_len, size_t size) "RDMA %zd of %zd bytes already in buffer"
-qemu_rdma_init_ram_blocks(int blocks) "Allocated %d local ram block structures"
-qemu_rdma_poll_recv(uint64_t comp, int64_t id, int sent) "completion %" PRIu64 " received (%" PRId64 ") left %d"
-qemu_rdma_poll_write(uint64_t comp, int left, uint64_t block, uint64_t chunk, void *local, void *remote) "completions %" PRIu64 " left %d, block %" PRIu64 ", chunk: %" PRIu64 " %p %p"
-qemu_rdma_poll_other(uint64_t comp, int left) "other completion %" PRIu64 " received left %d"
-qemu_rdma_post_send_control(const char *desc) "CONTROL: sending %s.."
-qemu_rdma_register_and_get_keys(uint64_t len, void *start) "Registering %" PRIu64 " bytes @ %p"
-qemu_rdma_register_odp_mr(const char *name) "Try to register On-Demand Paging memory region: %s"
-qemu_rdma_advise_mr(const char *name, uint32_t len, uint64_t addr, const char *res) "Try to advise block %s prefetch at %" PRIu32 "@0x%" PRIx64 ": %s"
-qemu_rdma_resolve_host_trying(const char *host, const char *ip) "Trying %s => %s"
-qemu_rdma_signal_unregister_append(uint64_t chunk, int pos) "Appending unregister chunk %" PRIu64 " at position %d"
-qemu_rdma_signal_unregister_already(uint64_t chunk) "Unregister chunk %" PRIu64 " already in queue"
-qemu_rdma_write_flush(int sent) "sent total: %d"
-qemu_rdma_write_one_block(int count, int block, uint64_t chunk, uint64_t current, uint64_t len, int nb_sent, int nb_chunks) "(%d) Not clobbering: block: %d chunk %" PRIu64 " current %" PRIu64 " len %" PRIu64 " %d %d"
-qemu_rdma_write_one_post(uint64_t chunk, long addr, long remote, uint32_t len) "Posting chunk: %" PRIu64 ", addr: 0x%lx remote: 0x%lx, bytes %" PRIu32
-qemu_rdma_write_one_queue_full(void) ""
-qemu_rdma_write_one_recvregres(int mykey, int theirkey, uint64_t chunk) "Received registration result: my key: 0x%x their key 0x%x, chunk %" PRIu64
-qemu_rdma_write_one_sendreg(uint64_t chunk, int len, int index, int64_t offset) "Sending registration request chunk %" PRIu64 " for %d bytes, index: %d, offset: %" PRId64
-qemu_rdma_write_one_top(uint64_t chunks, uint64_t size) "Writing %" PRIu64 " chunks, (%" PRIu64 " MB)"
-qemu_rdma_write_one_zero(uint64_t chunk, int len, int index, int64_t offset) "Entire chunk is zero, sending compress: %" PRIu64 " for %d bytes, index: %d, offset: %" PRId64
+rdma_accept_incoming_migration(void) ""
+rdma_accept_incoming_migration_accepted(void) ""
+rdma_accept_pin_state(bool pin) "%d"
+rdma_accept_pin_verbsc(void *verbs) "Verbs context after listen: %p"
+rdma_block_for_wrid_miss(uint64_t wcomp, uint64_t req) "A Wanted wrid %" PRIu64 " but got %" PRIu64
+rdma_cleanup_disconnect(void) ""
+rdma_close(void) ""
+rdma_connect_pin_all_requested(void) ""
+rdma_connect_pin_all_outcome(bool pin) "%d"
+rdma_dest_init_trying(const char *host, const char *ip) "%s => %s"
+rdma_dump_id_failed(const char *who) "%s RDMA Device opened, but can't query port information"
+rdma_dump_id(const char *who, const char *name, const char *dev_name, const char *dev_path, const char *ibdev_path, int transport, const char *transport_name) "%s RDMA Device opened: kernel name %s uverbs device name %s, infiniband_verbs class device path %s, infiniband class device path %s, transport: (%d) %s"
+rdma_dump_gid(const char *who, const char *src, const char *dst) "%s Source GID: %s, Dest GID: %s"
+rdma_exchange_get_response_start(const char *desc) "CONTROL: %s receiving..."
+rdma_exchange_get_response_none(const char *desc, int type) "Surprise: got %s (%d)"
+rdma_exchange_send_issue_callback(void) ""
+rdma_exchange_send_waiting(const char *desc) "Waiting for response %s"
+rdma_exchange_send_received(const char *desc) "Response %s received."
+rdma_fill(size_t control_len, size_t size) "RDMA %zd of %zd bytes already in buffer"
+rdma_init_ram_blocks(int blocks) "Allocated %d local ram block structures"
+rdma_poll_recv(uint64_t comp, int64_t id, int sent) "completion %" PRIu64 " received (%" PRId64 ") left %d"
+rdma_poll_write(uint64_t comp, int left, uint64_t block, uint64_t chunk, void *local, void *remote) "completions %" PRIu64 " left %d, block %" PRIu64 ", chunk: %" PRIu64 " %p %p"
+rdma_poll_other(uint64_t comp, int left) "other completion %" PRIu64 " received left %d"
+rdma_post_send_control(const char *desc) "CONTROL: sending %s.."
+rdma_register_and_get_keys(uint64_t len, void *start) "Registering %" PRIu64 " bytes @ %p"
+rdma_register_odp_mr(const char *name) "Try to register On-Demand Paging memory region: %s"
+rdma_advise_mr(const char *name, uint32_t len, uint64_t addr, const char *res) "Try to advise block %s prefetch at %" PRIu32 "@0x%" PRIx64 ": %s"
+rdma_resolve_host_trying(const char *host, const char *ip) "Trying %s => %s"
+rdma_signal_unregister_append(uint64_t chunk, int pos) "Appending unregister chunk %" PRIu64 " at position %d"
+rdma_signal_unregister_already(uint64_t chunk) "Unregister chunk %" PRIu64 " already in queue"
+rdma_write_flush(int sent) "sent total: %d"
+rdma_write_one_block(int count, int block, uint64_t chunk, uint64_t current, uint64_t len, int nb_sent, int nb_chunks) "(%d) Not clobbering: block: %d chunk %" PRIu64 " current %" PRIu64 " len %" PRIu64 " %d %d"
+rdma_write_one_post(uint64_t chunk, long addr, long remote, uint32_t len) "Posting chunk: %" PRIu64 ", addr: 0x%lx remote: 0x%lx, bytes %" PRIu32
+rdma_write_one_queue_full(void) ""
+rdma_write_one_recvregres(int mykey, int theirkey, uint64_t chunk) "Received registration result: my key: 0x%x their key 0x%x, chunk %" PRIu64
+rdma_write_one_sendreg(uint64_t chunk, int len, int index, int64_t offset) "Sending registration request chunk %" PRIu64 " for %d bytes, index: %d, offset: %" PRId64
+rdma_write_one_top(uint64_t chunks, uint64_t size) "Writing %" PRIu64 " chunks, (%" PRIu64 " MB)"
+rdma_write_one_zero(uint64_t chunk, int len, int index, int64_t offset) "Entire chunk is zero, sending compress: %" PRIu64 " for %d bytes, index: %d, offset: %" PRId64
 rdma_add_block(const char *block_name, int block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Added Block: '%s':%d, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d"
 rdma_block_notification_handle(const char *name, int index) "%s at %d"
 rdma_delete_block(void *block, uint64_t addr, uint64_t offset, uint64_t len, uint64_t end, uint64_t bits, int chunks) "Deleted Block: %p, addr: %" PRIu64 ", offset: %" PRIu64 " length: %" PRIu64 " end: %" PRIu64 " bits %" PRIu64 " chunks %d"
-- 
2.54.0

[PATCH 04/10] migration/rdma: Drop RDMALocalBlock.is_ram_block

[Peter Xu]

This is guaranteed to be true.  There seems to have support for some
dynamically allocated buffers but it was never really supported.  Remove
dead code.

As a side effect, this patch closes a report by removing the buggy code
completely.

Reported-by: Tristan (@TristanInSec)
Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4006
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 73 ++++++++++++++----------------------------------
 1 file changed, 21 insertions(+), 52 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index ffae0a887f..354935433c 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -196,7 +196,6 @@ typedef struct RDMALocalBlock {
     uint32_t       remote_rkey;     /* rkeys for non-chunk-level registration */
     int            index;           /* which block are we */
     unsigned int   src_index;       /* (Only used on dest) */
-    bool           is_ram_block;
     int            nb_chunks;
     unsigned long *transit_bitmap;
     unsigned long *unregister_bitmap;
@@ -441,14 +440,12 @@ static void register_to_network(RDMAContext *rdma, RDMARegister *reg)
     RDMALocalBlock *local_block;
     local_block  = &rdma->local_ram_blocks.block[reg->current_index];
 
-    if (local_block->is_ram_block) {
-        /*
-         * current_addr as passed in is an address in the local ram_addr_t
-         * space, we need to translate this for the destination
-         */
-        reg->key.current_addr -= local_block->offset;
-        reg->key.current_addr += rdma->dest_blocks[reg->current_index].offset;
-    }
+    /*
+     * current_addr as passed in is an address in the local ram_addr_t
+     * space, we need to translate this for the destination
+     */
+    reg->key.current_addr -= local_block->offset;
+    reg->key.current_addr += rdma->dest_blocks[reg->current_index].offset;
     reg->key.current_addr = htonll(reg->key.current_addr);
     reg->current_index = htonl(reg->current_index);
     reg->chunks = htonll(reg->chunks);
@@ -585,8 +582,6 @@ static void rdma_add_block(RDMAContext *rdma, const char *block_name,
     bitmap_clear(block->unregister_bitmap, 0, block->nb_chunks);
     block->remote_keys = g_new0(uint32_t, block->nb_chunks);
 
-    block->is_ram_block = local->init ? false : true;
-
     if (rdma->blockmap) {
         g_hash_table_insert(rdma->blockmap, (void *)(uintptr_t)block_offset, block);
     }
@@ -1767,19 +1762,10 @@ retry:
     chunk = ram_chunk_index(block->local_host_addr,
                             (uint8_t *)(uintptr_t)sge.addr);
     chunk_start = ram_chunk_start(block, chunk);
+    chunks = length / chunk_size;
 
-    if (block->is_ram_block) {
-        chunks = length / chunk_size;
-
-        if (chunks && ((length % chunk_size) == 0)) {
-            chunks--;
-        }
-    } else {
-        chunks = block->length / chunk_size;
-
-        if (chunks && ((block->length % chunk_size) == 0)) {
-            chunks--;
-        }
+    if (chunks && ((length % chunk_size) == 0)) {
+        chunks--;
     }
 
     trace_rdma_write_one_top(chunks + 1,
@@ -1804,7 +1790,7 @@ retry:
         }
     }
 
-    if (!rdma->pin_all || !block->is_ram_block) {
+    if (!rdma->pin_all) {
         if (!block->remote_keys[chunk]) {
             /*
              * This chunk has not yet been registered, so first check to see
@@ -1853,11 +1839,7 @@ retry:
              * Otherwise, tell other side to register.
              */
             reg.current_index = current_index;
-            if (block->is_ram_block) {
-                reg.key.current_addr = current_addr;
-            } else {
-                reg.key.chunk = chunk;
-            }
+            reg.key.current_addr = current_addr;
             reg.chunks = chunks;
 
             trace_rdma_write_one_sendreg(chunk, sge.length, current_index,
@@ -3408,30 +3390,17 @@ int rdma_registration_handle(QEMUFile *f)
                     goto err;
                 }
                 block = &(rdma->local_ram_blocks.block[reg->current_index]);
-                if (block->is_ram_block) {
-                    if (block->offset > reg->key.current_addr) {
-                        error_report("rdma: bad register address for block %s"
-                            " offset: %" PRIx64 " current_addr: %" PRIx64,
-                            block->block_name, block->offset,
-                            reg->key.current_addr);
-                        goto err;
-                    }
-                    host_addr = (block->local_host_addr +
-                                (reg->key.current_addr - block->offset));
-                    chunk = ram_chunk_index(block->local_host_addr,
-                                            (uint8_t *) host_addr);
-                } else {
-                    chunk = reg->key.chunk;
-                    host_addr = block->local_host_addr +
-                        (reg->key.chunk * migrate_rdma_chunk_size());
-                    /* Check for particularly bad chunk value */
-                    if (host_addr < (void *)block->local_host_addr) {
-                        error_report("rdma: bad chunk for block %s"
-                            " chunk: %" PRIx64,
-                            block->block_name, reg->key.chunk);
-                        goto err;
-                    }
+                if (block->offset > reg->key.current_addr) {
+                    error_report("rdma: bad register address for block %s"
+                        " offset: %" PRIx64 " current_addr: %" PRIx64,
+                        block->block_name, block->offset,
+                        reg->key.current_addr);
+                    goto err;
                 }
+                host_addr = (block->local_host_addr +
+                            (reg->key.current_addr - block->offset));
+                chunk = ram_chunk_index(block->local_host_addr,
+                                        (uint8_t *) host_addr);
                 chunk_start = ram_chunk_start(block, chunk);
                 chunk_end = ram_chunk_end(block, chunk + reg->chunks);
                 /* avoid "-Waddress-of-packed-member" warning */
-- 
2.54.0

[PATCH 05/10] migration/rdma: Drop RDMALocalBlock.unregister_bitmap

[Peter Xu]

Not used anymore.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 354935433c..c06395a51a 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -198,7 +198,6 @@ typedef struct RDMALocalBlock {
     unsigned int   src_index;       /* (Only used on dest) */
     int            nb_chunks;
     unsigned long *transit_bitmap;
-    unsigned long *unregister_bitmap;
 } RDMALocalBlock;
 
 /*
@@ -578,8 +577,6 @@ static void rdma_add_block(RDMAContext *rdma, const char *block_name,
     block->nb_chunks = ram_chunk_index(host_addr, host_addr + length) + 1UL;
     block->transit_bitmap = bitmap_new(block->nb_chunks);
     bitmap_clear(block->transit_bitmap, 0, block->nb_chunks);
-    block->unregister_bitmap = bitmap_new(block->nb_chunks);
-    bitmap_clear(block->unregister_bitmap, 0, block->nb_chunks);
     block->remote_keys = g_new0(uint32_t, block->nb_chunks);
 
     if (rdma->blockmap) {
@@ -665,9 +662,6 @@ static void rdma_delete_block(RDMAContext *rdma, RDMALocalBlock *block)
     g_free(block->transit_bitmap);
     block->transit_bitmap = NULL;
 
-    g_free(block->unregister_bitmap);
-    block->unregister_bitmap = NULL;
-
     g_free(block->remote_keys);
     block->remote_keys = NULL;
 
-- 
2.54.0

[PATCH 06/10] migration/rdma: Drop RDMARegister.key.chunk

[Peter Xu]

Not used anymore.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index c06395a51a..f64bcee418 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -415,10 +415,7 @@ static void network_to_control(RDMAControlHeader *control)
  * the actual RDMA operation.
  */
 typedef struct QEMU_PACKED {
-    union QEMU_PACKED {
-        uint64_t current_addr;  /* offset into the ram_addr_t space */
-        uint64_t chunk;         /* chunk to lookup if unregistering */
-    } key;
+    uint64_t current_addr;  /* offset into the ram_addr_t space */
     uint32_t current_index; /* which ramblock the chunk belongs to */
     uint32_t padding;
     uint64_t chunks;            /* how many sequential chunks to register */
@@ -443,16 +440,16 @@ static void register_to_network(RDMAContext *rdma, RDMARegister *reg)
      * current_addr as passed in is an address in the local ram_addr_t
      * space, we need to translate this for the destination
      */
-    reg->key.current_addr -= local_block->offset;
-    reg->key.current_addr += rdma->dest_blocks[reg->current_index].offset;
-    reg->key.current_addr = htonll(reg->key.current_addr);
+    reg->current_addr -= local_block->offset;
+    reg->current_addr += rdma->dest_blocks[reg->current_index].offset;
+    reg->current_addr = htonll(reg->current_addr);
     reg->current_index = htonl(reg->current_index);
     reg->chunks = htonll(reg->chunks);
 }
 
 static void network_to_register(RDMARegister *reg)
 {
-    reg->key.current_addr = ntohll(reg->key.current_addr);
+    reg->current_addr = ntohll(reg->current_addr);
     reg->current_index = ntohl(reg->current_index);
     reg->chunks = ntohll(reg->chunks);
 }
@@ -1833,7 +1830,7 @@ retry:
              * Otherwise, tell other side to register.
              */
             reg.current_index = current_index;
-            reg.key.current_addr = current_addr;
+            reg.current_addr = current_addr;
             reg.chunks = chunks;
 
             trace_rdma_write_one_sendreg(chunk, sge.length, current_index,
@@ -3375,7 +3372,7 @@ int rdma_registration_handle(QEMUFile *f)
                 reg_result = &results[count];
 
                 trace_rdma_registration_handle_register_loop(count,
-                         reg->current_index, reg->key.current_addr, reg->chunks);
+                         reg->current_index, reg->current_addr, reg->chunks);
 
                 if (reg->current_index >= rdma->local_ram_blocks.nb_blocks) {
                     error_report("rdma: 'register' bad block index %u (vs %d)",
@@ -3384,15 +3381,15 @@ int rdma_registration_handle(QEMUFile *f)
                     goto err;
                 }
                 block = &(rdma->local_ram_blocks.block[reg->current_index]);
-                if (block->offset > reg->key.current_addr) {
+                if (block->offset > reg->current_addr) {
                     error_report("rdma: bad register address for block %s"
                         " offset: %" PRIx64 " current_addr: %" PRIx64,
                         block->block_name, block->offset,
-                        reg->key.current_addr);
+                        reg->current_addr);
                     goto err;
                 }
                 host_addr = (block->local_host_addr +
-                            (reg->key.current_addr - block->offset));
+                            (reg->current_addr - block->offset));
                 chunk = ram_chunk_index(block->local_host_addr,
                                         (uint8_t *) host_addr);
                 chunk_start = ram_chunk_start(block, chunk);
-- 
2.54.0

[PATCH 07/10] migration/rdma: Sanity check RDMA_CONTROL_REGISTER_REQUEST on buflen

[Peter Xu]

RDMA header supports head.repeat on RDMA_CONTROL_REGISTER_REQUEST, which
can include >1 memory registrations.  The current code did check over
head.repeat to guard against RDMA_CONTROL_MAX_COMMANDS_PER_MESSAGE, however
it didn't further check the buffer size (head.len) to make sure the
received data is large enough to include the repeated entries.  Check it.

This is almost only to harden this piece of code, in reality on source side
QEMU never uses repeat>1..  However since it's a protocol, still keep it.
Check the buffer size instead.

In case it's not obvious to new RDMA readers: head.len should be the size
RDMA has last received, as qemu_rdma_exchange_get_response() checked on it
against byte_len (which was further fetched from ibv_wc.byte_len in
qemu_rdma_poll()).

Reported-by: Tristan (@TristanInSec)
Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4028
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index f64bcee418..5ce8b06818 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3362,6 +3362,14 @@ int rdma_registration_handle(QEMUFile *f)
             reg_resp.repeat = head.repeat;
             registers = (RDMARegister *) rdma->wr_data[idx].control_curr;
 
+            /* Making sure the register buffers to read are valid */
+            if (head.len != head.repeat * sizeof(RDMARegister)) {
+                error_report("%s: Invalid RDMA_CONTROL_REGISTER_REQUEST "
+                             "(head.repeat=%"PRIu32", head.len=%"PRIu32")",
+                             __func__, head.repeat, head.len);
+                goto err;
+            }
+
             for (int count = 0; count < head.repeat; count++) {
                 uint64_t chunk;
                 uint8_t *chunk_start, *chunk_end;
-- 
2.54.0

[PATCH 08/10] migration/rdma: Sanity check RDMA_CONTROL_REGISTER_REQUEST chunks

[Peter Xu]

The value received on wire for head.chunks when registering new RDMA
regions is not correctly checked.  Logically the value can still make
ram_chunk_start() (of ram_chunk_end()) to overflow, having a result pointer
very small, smaller than RDMALocalBlock.local_host_addr.

Add the sanity check.

Reported-by: Tristan (@TristanInSec)
Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4011
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/migration/rdma.c b/migration/rdma.c
index 5ce8b06818..bbbc40ea3b 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3401,6 +3401,13 @@ int rdma_registration_handle(QEMUFile *f)
                 chunk = ram_chunk_index(block->local_host_addr,
                                         (uint8_t *) host_addr);
                 chunk_start = ram_chunk_start(block, chunk);
+                if (chunk + reg->chunks > block->nb_chunks) {
+                    error_report("%s: head.chunks contains illegal value"
+                                 " (chunk=%"PRIu64", chunks=%"PRIu64", "
+                                 "nb_chunks=%d)", __func__, chunk,
+                                 reg->chunks, block->nb_chunks);
+                    goto err;
+                }
                 chunk_end = ram_chunk_end(block, chunk + reg->chunks);
                 /* avoid "-Waddress-of-packed-member" warning */
                 uint32_t tmp_rkey = 0;
-- 
2.54.0

[PATCH 09/10] migration/rdma: Sanity check upper bound of register MR address

[Peter Xu]

Before registration of RDMA ranges, QEMU destination checks for validity of
current_addr no less than the target ramblock's start address.  But it
didn't check for the upper bound to make sure the address is within the
size of the ramblock.  Add it.

Reported-by: Tristan (@TristanInSec)
Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4007
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index bbbc40ea3b..13dd60c11b 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3389,7 +3389,8 @@ int rdma_registration_handle(QEMUFile *f)
                     goto err;
                 }
                 block = &(rdma->local_ram_blocks.block[reg->current_index]);
-                if (block->offset > reg->current_addr) {
+                if (block->offset > reg->current_addr ||
+                    block->offset + block->length <= reg->current_addr) {
                     error_report("rdma: bad register address for block %s"
                         " offset: %" PRIx64 " current_addr: %" PRIx64,
                         block->block_name, block->offset,
-- 
2.54.0

[PATCH 10/10] migration/rdma: Sanity check compress request ranges

[Peter Xu]

The offset/length ranges are not sanity checked in RDMA requests, add the
checks.

Reported-by: Tristan (@TristanInSec)
Closes: https://gitlab.com/qemu-project/qemu/-/work_items/4005
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/rdma.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index 13dd60c11b..699aaa9903 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3200,6 +3200,28 @@ static int dest_ram_sort_func(const void *a, const void *b)
     return (a_index < b_index) ? -1 : (a_index != b_index);
 }
 
+static bool rdma_compress_range_check(RDMALocalBlock *block,
+                                      RDMACompress *comp)
+{
+    uint64_t block_end = block->offset + block->length;
+    uint64_t comp_end;
+
+    if (uadd64_overflow(comp->offset, comp->length, &comp_end)) {
+        goto fail;
+    }
+
+    if (comp->offset < block->offset || comp_end > block_end) {
+        goto fail;
+    }
+
+    return true;
+fail:
+    error_report("%s: compress request range outside range"
+                 " (block=%s, offset=%"PRIu64", length=%"PRIu64")",
+                 __func__, block->block_name, comp->offset, comp->length);
+    return false;
+}
+
 /*
  * During each iteration of the migration, we listen for instructions
  * by the source VM to perform dynamic page registrations before they
@@ -3279,7 +3301,9 @@ int rdma_registration_handle(QEMUFile *f)
                 goto err;
             }
             block = &(rdma->local_ram_blocks.block[comp->block_idx]);
-
+            if (!rdma_compress_range_check(block, comp)) {
+                goto err;
+            }
             host_addr = block->local_host_addr +
                             (comp->offset - block->offset);
             if (comp->value) {
-- 
2.54.0

Re: [PATCH 02/10] migration/rdma: Remove unregister code

[Yanfei Xu]

Hi Peter,

No objection to removing the dead code — it clearly never worked

I do have one question about the direction, though. The removed logic
was the only in-tree attempt at MR unregistration for the non-pin-all
path. Without it, registered MRs grow monotonically over a migration,
and with large, widely-spread dirty memory over chunks the accumulated
MR metadata (user + kernel) can cost more than pin-all and even perform
worse — which rather defeats the purpose of not pinning everything.

do we still intend to keep and improve the non-pin-all path going
forward? If so, some form of dynamic MR unregistration will eventually
be needed and it might be worth keeping this code，or at least leaving
a TODO to mark the gap?

Thanks,
Yanfei

On 2026/8/18 04:24, Peter Xu wrote:
> The unregister code was there since the first commit RDMA migration was
> merged, but it was never functioning.  Remove the dead code.
>
> Since the two control messages are the last ones, we don't even need to
> worry about compatibility of legacy RDMA control commands, we can directly
> remove the messages too.
>
> As a side effect, this patch closes a report by removing the code
> completely.
>
> Reported-by: Tristan (@TristanInSec)
> Closes:https://gitlab.com/qemu-project/qemu/-/work_items/4003
> Signed-off-by: Peter Xu<peterx@redhat.com>

Re: [PATCH 02/10] migration/rdma: Remove unregister code

[Peter Xu]

On Tue, Aug 18, 2026 at 07:57:06PM +0800, Yanfei Xu wrote:
> Hi Peter,

Hi, Yanfei,

> 
> No objection to removing the dead code — it clearly never worked
> 
> I do have one question about the direction, though. The removed logic
> was the only in-tree attempt at MR unregistration for the non-pin-all
> path. Without it, registered MRs grow monotonically over a migration,
> and with large, widely-spread dirty memory over chunks the accumulated
> MR metadata (user + kernel) can cost more than pin-all and even perform
> worse — which rather defeats the purpose of not pinning everything.
> 
> do we still intend to keep and improve the non-pin-all path going
> forward? If so, some form of dynamic MR unregistration will eventually
> be needed and it might be worth keeping this code，or at least leaving
> a TODO to mark the gap?

Thanks for taking a look.  This is a valid question to ask.

Though it was there for 13 years without being "enhanced", it means the
possibility we leverage it in the next couple of years is low.

You also discussed the other side of things: I am not a frequent RDMA user,
but my understanding is frequent MR reg operations already slow down
migration quite a bit.  It means dynamic management including unregisters
will be even worse.  AFAICT, it'll be a challenging task if we want to keep
the performance in bar and add a hard throttle to pinned memory.

Obviously, RDMA migration users care a bunch on performance.

So I see no good reason to not drop it.  Even if someone will work out that
problem, it's still not much code to add, and it'll likely be easier we
design it from scratch with the demand, and without worry of breaking
anyone.

When that happens (if it ever will..), someone should also send an update
to MAINTAINER file on RDMA migration to change it out of Odd Fixes stage..

Thanks,

-- 
Peter Xu

Re: [PATCH 02/10] migration/rdma: Remove unregister code

[Yanfei Xu]

On 2026/8/18 21:01, Peter Xu wrote:
> On Tue, Aug 18, 2026 at 07:57:06PM +0800, Yanfei Xu wrote:
>> Hi Peter,
> Hi, Yanfei,
>
>> No objection to removing the dead code — it clearly never worked
>>
>> I do have one question about the direction, though. The removed logic
>> was the only in-tree attempt at MR unregistration for the non-pin-all
>> path. Without it, registered MRs grow monotonically over a migration,
>> and with large, widely-spread dirty memory over chunks the accumulated
>> MR metadata (user + kernel) can cost more than pin-all and even perform
>> worse — which rather defeats the purpose of not pinning everything.
>>
>> do we still intend to keep and improve the non-pin-all path going
>> forward? If so, some form of dynamic MR unregistration will eventually
>> be needed and it might be worth keeping this code，or at least leaving
>> a TODO to mark the gap?
> Thanks for taking a look.  This is a valid question to ask.
>
> Though it was there for 13 years without being "enhanced", it means the
> possibility we leverage it in the next couple of years is low.
>
> You also discussed the other side of things: I am not a frequent RDMA user,
> but my understanding is frequent MR reg operations already slow down
> migration quite a bit.  It means dynamic management including unregisters
> will be even worse.  AFAICT, it'll be a challenging task if we want to keep
> the performance in bar and add a hard throttle to pinned memory.

One advantage of non-pin-all is that it neither sends the all-zero chunk
nor registers the corresponding MRs. For guests with a low dirty-page
workload and a large number of zero pages, this lets it migrate faster
than pin-all and pin less guest memory during the migration.

>
> Obviously, RDMA migration users care a bunch on performance.
>
> So I see no good reason to not drop it.  Even if someone will work out that
> problem, it's still not much code to add, and it'll likely be easier we
> design it from scratch with the demand, and without worry of breaking
> anyone.

Got it, thanks for the explanation.

Regards,
Yanfei

>
> When that happens (if it ever will..), someone should also send an update
> to MAINTAINER file on RDMA migration to change it out of Odd Fixes stage..
>
> Thanks,
>

Re: [PATCH 02/10] migration/rdma: Remove unregister code

[Peter Xu]

On Tue, Aug 18, 2026 at 09:57:12PM +0800, Yanfei Xu wrote:
> 
> On 2026/8/18 21:01, Peter Xu wrote:
> > On Tue, Aug 18, 2026 at 07:57:06PM +0800, Yanfei Xu wrote:
> > > Hi Peter,
> > Hi, Yanfei,
> > 
> > > No objection to removing the dead code — it clearly never worked
> > > 
> > > I do have one question about the direction, though. The removed logic
> > > was the only in-tree attempt at MR unregistration for the non-pin-all
> > > path. Without it, registered MRs grow monotonically over a migration,
> > > and with large, widely-spread dirty memory over chunks the accumulated
> > > MR metadata (user + kernel) can cost more than pin-all and even perform
> > > worse — which rather defeats the purpose of not pinning everything.
> > > 
> > > do we still intend to keep and improve the non-pin-all path going
> > > forward? If so, some form of dynamic MR unregistration will eventually
> > > be needed and it might be worth keeping this code，or at least leaving
> > > a TODO to mark the gap?
> > Thanks for taking a look.  This is a valid question to ask.
> > 
> > Though it was there for 13 years without being "enhanced", it means the
> > possibility we leverage it in the next couple of years is low.
> > 
> > You also discussed the other side of things: I am not a frequent RDMA user,
> > but my understanding is frequent MR reg operations already slow down
> > migration quite a bit.  It means dynamic management including unregisters
> > will be even worse.  AFAICT, it'll be a challenging task if we want to keep
> > the performance in bar and add a hard throttle to pinned memory.
> 
> One advantage of non-pin-all is that it neither sends the all-zero chunk
> nor registers the corresponding MRs. For guests with a low dirty-page
> workload and a large number of zero pages, this lets it migrate faster
> than pin-all and pin less guest memory during the migration.

I actually don't know why RDMA_CONTROL_COMPRESS is only used in !pin_all,
do you know?

I can only guess RDMA WRITEs were fast and need no round-robin chats, so
it's faster than RDMA_CONTROL_COMPRESS, but you seem to say it's not true.

Meanwhile, I would expect pin-all=off ultimately should meet the same perf
over pin-all=on.. so I don't really know how needs pin-all=on...  maybe
it's useful when one is looking for minimum total migration time when VMs
are required to be evicted from one host?

Feel free to share if you have more data points; I'm almost speaking from
reading the code, so it could be wrong.

Thanks,

-- 
Peter Xu

Re: [PATCH 02/10] migration/rdma: Remove unregister code

[Yanfei Xu]

On 2026/8/18 22:43, Peter Xu wrote:
> I actually don't know why RDMA_CONTROL_COMPRESS is only used in !pin_all,
> do you know?
my understanding is that with RDMA_CONTROL_COMPRESS, the corresponding MR
isn't registered, so the memset(0) in destination side can be guaranteed to
complete before the next RDMA write to the same chunk. Or it needs an
extra ack from destionation for the memset(0) complation.
>
> I can only guess RDMA WRITEs were fast and need no round-robin chats, so
> it's faster than RDMA_CONTROL_COMPRESS, but you seem to say it's not true.
if most of guest's pages are not being allocated yet or are zero page,
RDMA_CONTROL_COMPRESS works more efficient.
>
> Meanwhile, I would expect pin-all=off ultimately should meet the same perf
> over pin-all=on.. so I don't really know how needs pin-all=on...  maybe
> it's useful when one is looking for minimum total migration time when VMs
> are required to be evicted from one host?
>
> Feel free to share if you have more data points; I'm almost speaking from
> reading the code, so it could be wrong.

Hah, I am new to RDMA live migration, glad to discuss ^_^

Yanfei