Code cleanup for remote access, prevent fault loop

Author: fwsGonzo

lib/tinykvm/common.hpp

@@ -47,7 +47,7 @@ namespace tinykvm
 		// Print profiling results. Side effect: *sorts vectors*
 		// when user_defined is non-empty, it will use that label
 		// instead of "UserDefined"
-		void print(const char* user_defined = "");
+		void print(const char* user_defined = "") const;
 		// Clear all profiling samples
 		void reset() {
 			for (auto& vec : times)

lib/tinykvm/machine.hpp

@@ -149,8 +149,6 @@ struct Machine
 	template <typename T> T* get_userdata() { return static_cast<T*> (m_userdata); }
 
 	std::string_view memory_at(uint64_t a, size_t s) const;
-	template <typename T = char>
-	T* rw_memory_at(uint64_t a, size_t s);
 	bool memory_safe_at(uint64_t a, size_t s) const;
 	char* unsafe_memory_at(uint64_t a, size_t s) { return memory.at(a, s); }
 	uint64_t translate(uint64_t virt) const;

lib/tinykvm/machine_inline.hpp

@@ -215,11 +215,6 @@ inline std::string_view Machine::memory_at(uint64_t a, size_t s) const
 {
 	return memory.view(a, s);
 }
-template <typename T>
-inline T* Machine::rw_memory_at(uint64_t a, size_t s)
-{
-	return (T*) memory.safely_at(a, s);
-}
 inline bool Machine::memory_safe_at(uint64_t a, size_t s) const
 {
 	return memory.safely_within(a, s);

lib/tinykvm/machine_utils.cpp

@@ -637,7 +637,7 @@ std::string Machine::memcstring(address_t src, size_t maxlen) const
 	return result;
 }
 
-void MachineProfiling::print(const char* user_defined) {
+void MachineProfiling::print(const char* user_defined) const {
 	std::array<std::string, 7> locnames = {
 		"vCPU Run",
 		"Reset",
@@ -651,7 +651,7 @@ void MachineProfiling::print(const char* user_defined) {
 		locnames[UserDefined] = user_defined;
 	}
 	for (size_t i = 0; i < locnames.size(); i++) {
-		auto& vec = this->times[i];
+		auto vec = this->times[i];
 		if (vec.empty()) continue;
 		// Sort the times to find min/max and median
 		std::sort(vec.begin(), vec.end());

lib/tinykvm/memory.cpp

@@ -261,8 +261,7 @@ uint64_t* vMemory::page_at(uint64_t addr) const
 		}
 	}
 	/* Remote machine always last resort */
-	if (machine.has_remote())
-	{
+	if (machine.has_remote()) {
 		return machine.remote().main_memory().page_at(addr);
 	}
 	memory_exception("Memory::page_at() invalid region", addr, 4096);
@@ -278,21 +277,12 @@ char* vMemory::safely_at(uint64_t addr, size_t asize)
 
 	if (safely_within(addr, asize))
 		return &ptr[addr - physbase];
+
 	/* Remote machine always last resort */
-	if (machine.has_remote())
-	{
+	if (machine.has_remote()) {
 		return machine.remote().main_memory().safely_at(addr, asize);
 	}
 
-	/* Slow-path page walk */
-	const auto pagebase = addr & ~PageMask();
-	const auto offset   = addr & PageMask();
-	if (offset + asize <= vMemory::PageSize())
-	{
-		auto* page = this->get_writable_page(pagebase, expectedUsermodeFlags(), false, false);
-		return &page[offset];
-	}
-
 	memory_exception("Memory::safely_at() invalid region", addr, asize);
 }
 const char* vMemory::safely_at(uint64_t addr, size_t asize) const
@@ -306,20 +296,9 @@ const char* vMemory::safely_at(uint64_t addr, size_t asize) const
 		}
 	}
 	/* Remote machine always last resort */
-	if (machine.has_remote())
-	{
+	if (machine.has_remote()) {
 		return machine.remote().main_memory().safely_at(addr, asize);
 	}
-
-	/* Slow-path page walk */
-	const auto pagebase = addr & ~PageMask();
-	const auto offset   = addr & PageMask();
-	if (offset + asize <= vMemory::PageSize())
-	{
-		auto* page = this->get_userpage_at(pagebase);
-		return &page[offset];
-	}
-
 	memory_exception("Memory::safely_at() invalid region", addr, asize);
 }
 std::string_view vMemory::view(uint64_t addr, size_t asize) const {

lib/tinykvm/vcpu.hpp

@@ -56,6 +56,7 @@ namespace tinykvm
 		void* timer_id = nullptr;
 		uint64_t remote_return_address = 0;
 		uint64_t remote_original_tls_base = 0;
+		uint64_t remote_fault_address = 0;
 		std::mutex* remote_serializer = nullptr;
 
 	private:

lib/tinykvm/vcpu_run.cpp

@@ -293,6 +293,13 @@ long vCPU::run_once()
 							WritablePageOptions zero_opts;
 							zero_opts.zeroes = false;
 							(void)writable_page_at(machine().remote().memory, addr, PDE64_USER | PDE64_RW, zero_opts);
+							// Remember that this address caused a fault, so that we don't loop infinitely
+							if (this->remote_fault_address == addr) {
+								// This address already caused a fault
+								this->handle_exception(intr);
+								Machine::machine_exception("Remote VM page fault repeat on same address", intr);
+							}
+							this->remote_fault_address = addr;
 							regs.rax = 0; /* Indicate that it was local */
 							this->set_registers(regs);
 							return KVM_EXIT_IO;