Skip to content

Commit bc5f072

Browse files
committed
Fix issues with remote resume, add forking test
1 parent aac5072 commit bc5f072

File tree

5 files changed

+79
-8
lines changed

5 files changed

+79
-8
lines changed

lib/tinykvm/memory.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ vMemory::vMemory(Machine& m, const MachineOptions& options, const vMemory& other
5757
this->executable_heap = other.executable_heap;
5858
this->mmap_physical_begin = other.mmap_physical_begin;
5959
this->mmap_physical = other.mmap_physical;
60+
this->remote_end = other.remote_end;
6061
banks.init_from(other.banks);
6162
}
6263
vMemory::~vMemory()

lib/tinykvm/memory_bank.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@ void MemoryBanks::init_from(const MemoryBanks& other)
2828
{
2929
this->m_arena_begin = other.m_arena_begin;
3030
this->m_arena_next = other.m_arena_next;
31+
// Verify that there aren't any existing banks allocated
32+
if (!this->m_mem.empty()) {
33+
throw MemoryException("Cannot init_from() when banks are already allocated (arena_begin will be wrong)",
34+
this->m_arena_begin, m_mem[0].addr);
35+
}
3136
}
3237
void MemoryBanks::set_max_pages(size_t new_max, size_t new_hugepages)
3338
{

lib/tinykvm/remote.cpp

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ void Machine::remote_connect(Machine& remote, bool connect_now)
4040
this->delete_memory(1);
4141
this->memory.delete_foreign_mmap_ranges();
4242
this->memory.delete_foreign_banks();
43+
this->m_remote_cow_counter = ~0u;
4344
}
4445
// Install the remote memory in this machine
4546
this->install_memory(1, remote_vmem, false);
@@ -115,6 +116,29 @@ void Machine::ipre_remote_resume_now(bool save_all, std::function<void(Machine&)
115116
// 2. Connect to remote now
116117
const auto remote_fsbase = this->remote_activate_now();
117118

119+
const auto remote_cow_counter = remote().memory.cow_written_pages.size();
120+
bool do_prepare = false;
121+
if (this->m_remote_cow_counter != remote_cow_counter) {
122+
this->m_remote_cow_counter = remote_cow_counter;
123+
// New working memory pages have been created in the remote,
124+
// so we need to make sure we see the latest changes.
125+
for (auto& bank : remote().memory.banks)
126+
{
127+
const VirtualMem vmem = bank.to_vmem();
128+
if (this->memory.remote_bank_break < bank.addr) {
129+
this->memory.remote_bank_break = bank.addr;
130+
if constexpr (VERBOSE_REMOTE) {
131+
fprintf(stderr, "IPRE remote: mapped bank %u at 0x%lX-0x%lX\n",
132+
bank.idx, bank.addr, bank.addr + bank.size());
133+
}
134+
const unsigned new_idx = memory.allocate_region_idx();
135+
this->install_memory(new_idx, vmem, false);
136+
memory.foreign_banks.push_back(new_idx);
137+
do_prepare = true;
138+
}
139+
}
140+
}
141+
118142
// 3. Copy remote registers into current state
119143
tinykvm::Machine& remote_vm = remote();
120144
copy_callee_saved_registers(save_all, this->registers(), remote_vm.registers());
@@ -130,6 +154,8 @@ void Machine::ipre_remote_resume_now(bool save_all, std::function<void(Machine&)
130154
// 4. Resume execution
131155
// Set RDI to our FSBASE for the remote VM
132156
this->registers().rdi = remote_fsbase;
157+
if (do_prepare)
158+
this->prepare_vmresume(remote_fsbase, true);
133159
this->run(0.0f);
134160
} catch (const std::exception& e) {
135161
// If an exception occurred, disconnect and restore FSBASE

lib/tinykvm/vcpu_run.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,13 +283,18 @@ long vCPU::run_once()
283283
} else if (machine().is_foreign_address(addr)) {
284284
/* Check that the error code is instruction fetch failed */
285285
const uint32_t errcode = regs.rax;
286+
if constexpr (VERBOSE_REMOTE) {
287+
printf("Remote VM page fault at 0x%lX, errcode=0x%X\n", addr, errcode);
288+
}
286289
if ((errcode & 0x10) == 0) {
287290
if (machine().remote().is_remote_connected()) {
288291
// Not an instruction fetch, but a memory read or write
289292
// Since it's foreign memory, we try to handle it in the remote VM
290293
WritablePageOptions zero_opts;
291294
zero_opts.zeroes = false;
292295
(void)writable_page_at(machine().remote().memory, addr, PDE64_USER | PDE64_RW, zero_opts);
296+
regs.rax = 0; /* Indicate that it was local */
297+
this->set_registers(regs);
293298
return KVM_EXIT_IO;
294299
} else {
295300
// Not connected, so we can't handle it
@@ -326,7 +331,17 @@ long vCPU::run_once()
326331

327332
WritablePageOptions zero_opts;
328333
zero_opts.zeroes = false;
329-
(void)writable_page_at(memory, addr, PDE64_USER | PDE64_RW, zero_opts);
334+
auto result = writable_page_at(memory, addr, PDE64_USER | PDE64_RW, zero_opts);
335+
if constexpr (false) {
336+
char buffer[256];
337+
PRINTER(machine().m_printer, buffer,
338+
"Page fault on 0x%lX handled, mapped to host page %p\n", addr, result.page);
339+
PRINTER(machine().m_printer, buffer,
340+
" Entry value 0x%lX\n", result.entry);
341+
PRINTER(machine().m_printer, buffer,
342+
" Our bank arena: begin=0x%lX\n",
343+
memory.banks.arena_begin());
344+
}
330345
return KVM_EXIT_IO;
331346
}
332347
else if (intr == 1) /* Debug trap */

tests/unit/remote.cpp

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,7 @@ int main() {
342342
while (1) {
343343
void* p = NULL;
344344
size_t len = storage_wait_paused(&p);
345+
memset(p, 0, len);
345346
strcpy((char*)p, "Data from storage");
346347
}
347348
return 1234;
@@ -369,7 +370,7 @@ extern long remote_resume(void* data, size_t len);
369370
long test_remote() {
370371
for (int i = 0; i < 100; i++) {
371372
char buffer[8192];
372-
remote_resume(buffer, sizeof(buffer));
373+
remote_resume(buffer, 8192);
373374
if (strcmp(buffer, "Data from storage") == 0) {
374375
continue;
375376
}
@@ -380,8 +381,7 @@ long test_remote() {
380381
int main() {
381382
return test_remote();
382383
}
383-
)M",
384-
"-Wl,--just-symbols=storage.syms");
384+
)M", "-Wl,--just-symbols=storage.syms");
385385

386386
static bool is_waiting = false;
387387
tinykvm::Machine::install_unhandled_syscall_handler(
@@ -438,8 +438,32 @@ int main() {
438438
REQUIRE(fork.has_remote());
439439

440440
// Test remote resume
441-
fork.vmcall("test_remote");
442-
REQUIRE(fork.return_value() == 2345);
443-
REQUIRE(!fork.is_remote_connected());
444-
REQUIRE(fork.remote_connection_count() == 100);
441+
for (int i = 0; i < 100; i++) {
442+
is_waiting = false;
443+
fork.vmcall("test_remote");
444+
REQUIRE(fork.return_value() == 2345);
445+
REQUIRE(!fork.is_remote_connected());
446+
REQUIRE(fork.remote_connection_count() == (i + 1) * 100);
447+
REQUIRE(is_waiting);
448+
}
449+
450+
// Test remote resume against a forked storage VM
451+
storage.prepare_copy_on_write();
452+
tinykvm::Machine storage_fork(storage, {
453+
.max_mem = 16ULL << 20, // MB
454+
.max_cow_mem = MAX_COWMEM,
455+
.split_hugepages = true
456+
});
457+
fork.remote_connect(storage_fork);
458+
459+
// Test remote resume against the forked storage VM
460+
printf("Testing remote resume against forked storage VM\n");
461+
for (int i = 0; i < 100; i++) {
462+
is_waiting = false;
463+
fork.vmcall("test_remote");
464+
REQUIRE(fork.return_value() == 2345);
465+
REQUIRE(!fork.is_remote_connected());
466+
REQUIRE(fork.remote_connection_count() == 10000 + (i + 1) * 100);
467+
REQUIRE(is_waiting);
468+
}
445469
}

0 commit comments

Comments
 (0)