feat(uhyve): use v2 interface

On x86_64, the data address will be stored in the rdi register
to support 64-bit addresses.

This also removes the usage of ExitArgs, as it is a single integer.

We additionally introduce improved, more POSIX-compliant-ish error
handling together, more debugging messages and replaced the if..else
with matches for readability reasons.

Co-authored-by: Ellen Εμιλία Άννα Zscheile <fogti+devel@ytrizja.de>
Helped-by: Jonathan Klimt <jonathan.klimt@eonerc.rwth-aachen.de>

Replace register, improve error handling, assert UhyveStdout/UhyveStderr return value, use matches

fix abs bug

Author: n0toose

Cargo.lock

@@ -141,7 +141,7 @@ thiserror = { version = "2", default-features = false }
 time = { version = "0.3", default-features = false }
 volatile = "0.6"
 zerocopy = { version = "0.8", default-features = false }
-uhyve-interface = "0.1.4"
+uhyve-interface = "0.2.0"
 
 [dependencies.smoltcp]
 version = "0.12"
@@ -210,6 +210,7 @@ exclude = [
 ]
 
 [patch.crates-io]
+uhyve-interface = { git = "https://github.com/n0toose/uhyve", branch = "uhyve-if-v2_io-plus-rcx" }
 safe-mmio = { git = "https://github.com/hermit-os/safe-mmio", branch = "be" }
 
 [profile.profiling]

Cargo.toml

@@ -4,9 +4,12 @@ use core::task::Poll;
 
 use async_trait::async_trait;
 use embedded_io::{Read, ReadReady, Write};
-use uhyve_interface::parameters::WriteParams;
-use uhyve_interface::{GuestVirtAddr, Hypercall};
+use memory_addresses::VirtAddr;
+use uhyve_interface::GuestPhysAddr;
+use uhyve_interface::v2::Hypercall;
+use uhyve_interface::v2::parameters::WriteParams;
 
+use crate::arch::mm::paging;
 use crate::console::{CONSOLE, CONSOLE_WAKER};
 use crate::fd::{
 	AccessPermission, FileAttr, ObjectInterface, PollEvent, STDERR_FILENO, STDOUT_FILENO,
@@ -159,14 +162,20 @@ impl ObjectInterface for UhyveStdout {
 	}
 
 	async fn write(&self, buf: &[u8]) -> io::Result<usize> {
-		let write_params = WriteParams {
+		let mut write_params = WriteParams {
 			fd: STDOUT_FILENO,
-			buf: GuestVirtAddr::from_ptr(buf.as_ptr()),
-			len: buf.len(),
+			buf: GuestPhysAddr::new(
+				paging::virtual_to_physical(VirtAddr::from_ptr(buf.as_ptr()))
+					.unwrap()
+					.as_u64(),
+			),
+			len: buf.len().try_into().unwrap(),
+			ret: 0i64,
 		};
-		uhyve_hypercall(Hypercall::FileWrite(&write_params));
-
-		Ok(write_params.len)
+		uhyve_hypercall(Hypercall::FileWrite(&mut write_params));
+		// Assumption: Uhyve will write to stdout and manually override the ret.
+		debug_assert_eq!(write_params.len, write_params.ret as u64);
+		Ok(write_params.ret.try_into().unwrap())
 	}
 
 	async fn isatty(&self) -> io::Result<bool> {
@@ -198,14 +207,20 @@ impl ObjectInterface for UhyveStderr {
 	}
 
 	async fn write(&self, buf: &[u8]) -> io::Result<usize> {
-		let write_params = WriteParams {
+		let mut write_params = WriteParams {
 			fd: STDERR_FILENO,
-			buf: GuestVirtAddr::from_ptr(buf.as_ptr()),
-			len: buf.len(),
+			buf: GuestPhysAddr::new(
+				paging::virtual_to_physical(VirtAddr::from_ptr(buf.as_ptr()))
+					.unwrap()
+					.as_u64(),
+			),
+			len: buf.len().try_into().unwrap(),
+			ret: 0i64,
 		};
-		uhyve_hypercall(Hypercall::FileWrite(&write_params));
-
-		Ok(write_params.len)
+		uhyve_hypercall(Hypercall::FileWrite(&mut write_params));
+		// Assumption: Uhyve will write to stdout and manually override the ret.
+		debug_assert_eq!(write_params.len, write_params.ret as u64);
+		Ok(write_params.ret.try_into().unwrap())
 	}
 
 	async fn isatty(&self) -> io::Result<bool> {

src/fd/stdio.rs

@@ -9,10 +9,11 @@ use async_lock::Mutex;
 use async_trait::async_trait;
 use embedded_io::{ErrorType, Read, Write};
 use memory_addresses::VirtAddr;
-use uhyve_interface::parameters::{
+use uhyve_interface::GuestPhysAddr;
+use uhyve_interface::v2::Hypercall;
+use uhyve_interface::v2::parameters::{
 	CloseParams, LseekParams, OpenParams, ReadParams, UnlinkParams, WriteParams,
 };
-use uhyve_interface::{GuestPhysAddr, GuestVirtAddr, Hypercall};
 
 use crate::arch::mm::paging;
 use crate::env::fdt;
@@ -35,15 +36,19 @@ impl UhyveFileHandleInner {
 	fn lseek(&self, offset: isize, whence: SeekWhence) -> io::Result<isize> {
 		let mut lseek_params = LseekParams {
 			fd: self.0,
-			offset,
+			offset: offset.try_into().unwrap(),
 			whence: u8::from(whence).into(),
 		};
 		uhyve_hypercall(Hypercall::FileLseek(&mut lseek_params));
-
-		if lseek_params.offset >= 0 {
-			Ok(lseek_params.offset)
-		} else {
-			Err(Errno::Inval)
+		// TODO: Although we can generally assume that what Uhyve delivers should be
+		// correct for now, it might make sense to build in checks (or at least debug_assert's)
+		match lseek_params.offset {
+			offset if offset >= 0 => Ok(offset.try_into().unwrap()),
+			errno if errno < 0 => Err((errno as i32).abs().try_into().unwrap()),
+			_ => {
+				debug!("Uhyve lseek hypercall yielded a zero.");
+				Err(Errno::Inval)
+			}
 		}
 	}
 }
@@ -56,30 +61,47 @@ impl Read for UhyveFileHandleInner {
 	fn read(&mut self, buf: &mut [u8]) -> Result<usize, Self::Error> {
 		let mut read_params = ReadParams {
 			fd: self.0,
-			buf: GuestVirtAddr::from_ptr(buf.as_mut_ptr()),
-			len: buf.len(),
-			ret: 0,
+			buf: GuestPhysAddr::new(
+				paging::virtual_to_physical(VirtAddr::from_ptr(buf.as_mut_ptr()))
+					.unwrap()
+					.as_u64(),
+			),
+			len: buf.len().try_into().unwrap(),
+			ret: 0i64,
 		};
 		uhyve_hypercall(Hypercall::FileRead(&mut read_params));
-
-		if read_params.ret >= 0 {
-			Ok(read_params.ret.try_into().unwrap())
-		} else {
-			Err(Errno::Io)
+		match read_params.ret {
+			ret if ret >= 0 => Ok(ret.try_into().unwrap()),
+			_ => Err((read_params.ret as i32).abs().try_into().unwrap()),
 		}
 	}
 }
 
 impl Write for UhyveFileHandleInner {
 	fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-		let write_params = WriteParams {
+		let mut write_params = WriteParams {
 			fd: self.0,
-			buf: GuestVirtAddr::from_ptr(buf.as_ptr()),
-			len: buf.len(),
+			buf: GuestPhysAddr::new(
+				paging::virtual_to_physical(VirtAddr::from_ptr(buf.as_ptr()))
+					.unwrap()
+					.as_u64(),
+			),
+			len: buf.len().try_into().unwrap(),
+			ret: 0i64,
 		};
-		uhyve_hypercall(Hypercall::FileWrite(&write_params));
-
-		Ok(write_params.len)
+		// fd refers to a regular file
+		uhyve_hypercall(Hypercall::FileWrite(&mut write_params));
+		match write_params.ret {
+			// Assumption: fd is a regular file, a zero is only valid if the len
+			// (aka. "count") is also zero. Otherwise, however, we assume that something
+			// is wrong in Hermit<>Uhyve communication.
+			ret if ret > 0 || (ret == 0 && write_params.len == 0) => Ok(ret.try_into().unwrap()),
+			errno if errno < 0 => Err((errno as i32).abs().try_into().unwrap()),
+			_ => {
+				debug!("Uhyve write hypercall yielded a zero.");
+				Err(Errno::Inval)
+			}
+		}
 	}
 
 	fn flush(&mut self) -> Result<(), Self::Error> {
@@ -186,13 +208,11 @@ impl VfsNode for UhyveDirectory {
 			ret: -1,
 		};
 		uhyve_hypercall(Hypercall::FileOpen(&mut open_params));
-
-		if open_params.ret > 0 {
-			Ok(Arc::new(async_lock::RwLock::new(UhyveFileHandle::new(
-				open_params.ret,
-			))))
-		} else {
-			Err(Errno::Io)
+		let ret = open_params.ret; // circumvent packed field access
+		match ret {
+			// Assumption: Uhyve will never return a standard stream.
+			ret if ret >= 0 => Ok(Arc::new(async_lock::RwLock::new(UhyveFileHandle::new(ret)))),
+			_ => Err(ret.abs().try_into().unwrap()),
 		}
 	}
 
@@ -208,11 +228,10 @@ impl VfsNode for UhyveDirectory {
 			ret: -1,
 		};
 		uhyve_hypercall(Hypercall::FileUnlink(&mut unlink_params));
-
-		if unlink_params.ret == 0 {
-			Ok(())
-		} else {
-			Err(Errno::Io)
+		let ret = unlink_params.ret; // circumvent packed field access
+		match ret {
+			0 => Ok(()),
+			_ => Err(unlink_params.ret.abs().try_into().unwrap()),
 		}
 	}
 

src/fs/uhyve.rs

@@ -1,23 +1,26 @@
 use core::ptr;
 
 use memory_addresses::VirtAddr;
-use uhyve_interface::parameters::{ExitParams, SerialWriteBufferParams};
-use uhyve_interface::{Hypercall, HypercallAddress};
+use uhyve_interface::GuestPhysAddr;
+use uhyve_interface::v2::parameters::SerialWriteBufferParams;
+use uhyve_interface::v2::{Hypercall, HypercallAddress};
 
 use crate::arch;
 use crate::arch::mm::paging::{self, virtual_to_physical};
 use crate::syscalls::interfaces::SyscallInterface;
 
-/// perform a SerialWriteBuffer hypercall with `buf` as payload.
+/// perform a SerialWriteBuffer hypercall with `buf` as payload
 #[inline]
 #[cfg_attr(target_arch = "riscv64", expect(dead_code))]
 pub(crate) fn serial_buf_hypercall(buf: &[u8]) {
-	let len = buf.len();
-	let buf = virtual_to_physical(VirtAddr::from_ptr(core::ptr::from_ref::<[u8]>(buf)))
-		.unwrap()
-		.as_u64()
-		.into();
-	let p = SerialWriteBufferParams { buf, len };
+	let p = SerialWriteBufferParams {
+		buf: GuestPhysAddr::new(
+			virtual_to_physical(VirtAddr::from_ptr(core::ptr::from_ref::<[u8]>(buf)))
+				.unwrap()
+				.as_u64(),
+		),
+		len: buf.len() as u64,
+	};
 	uhyve_hypercall(Hypercall::SerialWriteBuffer(&p));
 }
 
@@ -33,9 +36,11 @@ fn data_addr<T>(data: &T) -> u64 {
 #[inline]
 fn hypercall_data(hypercall: &Hypercall<'_>) -> u64 {
 	match hypercall {
-		Hypercall::Cmdsize(data) => data_addr(*data),
-		Hypercall::Cmdval(data) => data_addr(*data),
-		Hypercall::Exit(data) => data_addr(*data),
+		// As we are encoding an exit code (max 32 bits) into "an
+		// address", and memory_addresses complains if an address
+		// has any bits above the 48th one set to 1, we encode
+		// potential negative numbers into a u32, then a u64.
+		Hypercall::Exit(exit_code) => u64::from((*exit_code) as u32),
 		Hypercall::FileClose(data) => data_addr(*data),
 		Hypercall::FileLseek(data) => data_addr(*data),
 		Hypercall::FileOpen(data) => data_addr(*data),
@@ -56,20 +61,21 @@ pub(crate) fn uhyve_hypercall(hypercall: Hypercall<'_>) {
 	let data = hypercall_data(&hypercall);
 
 	#[cfg(target_arch = "x86_64")]
-	unsafe {
-		use x86_64::instructions::port::Port;
-
-		let data =
-			u32::try_from(data).expect("Hypercall data must lie in the first 4GiB of memory");
-		Port::new(ptr).write(data);
+	{
+		unsafe {
+			use core::arch::asm;
+			asm!(
+				"out dx, eax", in("dx") ptr, in("eax") 0x1234u32, in("rdi") data, options(nostack, preserves_flags)
+			);
+		}
 	}
 
 	#[cfg(target_arch = "aarch64")]
 	unsafe {
 		use core::arch::asm;
 		asm!(
 			"str x8, [{ptr}]",
-			ptr = in(reg) u64::from(ptr),
+			ptr = in(reg) ptr,
 			in("x8") data,
 			options(nostack),
 		);
@@ -83,8 +89,7 @@ pub struct Uhyve;
 
 impl SyscallInterface for Uhyve {
 	fn shutdown(&self, error_code: i32) -> ! {
-		let sysexit = ExitParams { arg: error_code };
-		uhyve_hypercall(Hypercall::Exit(&sysexit));
+		uhyve_hypercall(Hypercall::Exit(error_code));
 
 		loop {
 			arch::processor::halt();