Skip to content

Add portio emulation handling for the in{b,w,l} instructions #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
brendank310 wants to merge 1 commit into from
Closed

Add portio emulation handling for the in{b,w,l} instructions #52

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions vmm/src/x64/amd/dispatch_vmexit_io.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,31 @@

namespace microv
{
/// <!-- description -->
/// @brief Handle emulation of port IO read/write accesses
///
/// <!-- input/outputs -->
/// @param mut_sys the bf_syscall_t to use
/// @param pmut_mut_exit_io mv_exit_io_t pointer to use
/// @param data_mask bitmask to apply to either the read or
/// write
/// @return void
///
[[nodiscard]] constexpr auto
handle_io(
syscall::bf_syscall_t &mut_sys,
hypercall::mv_exit_io_t *pmut_mut_exit_io,
bsl::safe_u64 const &data_mask) noexcept
{
auto const rax{mut_sys.bf_tls_rax()};
if (hypercall::MV_EXIT_IO_OUT == pmut_mut_exit_io->type) {
pmut_mut_exit_io->data = (data_mask & rax).get();
}
else {
mut_sys.bf_tls_set_rax((data_mask & pmut_mut_exit_io->data));
}
}

/// <!-- description -->
/// @brief Dispatches IO VMExits.
///
Expand Down Expand Up @@ -128,14 +153,13 @@ namespace microv
mut_exit_io->type = hypercall::MV_EXIT_IO_OUT.get();
}
else {
bsl::error() << "MV_EXIT_IO_IN not implemented\n" << bsl::here();
return bsl::errc_failure;
mut_exit_io->type = hypercall::MV_EXIT_IO_IN.get();
}

if (((exitinfo1 & sz32_mask) >> sz32_shft).is_pos()) {
constexpr auto data_mask{0x00000000FFFFFFFF_u64};
mut_exit_io->size = hypercall::mv_bit_size_t::mv_bit_size_t_32;
mut_exit_io->data = (data_mask & rax).get();
handle_io(mut_sys, mut_exit_io.get(), data_mask);
}
else {
bsl::touch();
Expand All @@ -144,7 +168,7 @@ namespace microv
if (((exitinfo1 & sz16_mask) >> sz16_shft).is_pos()) {
constexpr auto data_mask{0x000000000000FFFF_u64};
mut_exit_io->size = hypercall::mv_bit_size_t::mv_bit_size_t_16;
mut_exit_io->data = (data_mask & rax).get();
handle_io(mut_sys, mut_exit_io.get(), data_mask);
}
else {
bsl::touch();
Expand All @@ -153,7 +177,7 @@ namespace microv
if (((exitinfo1 & sz08_mask) >> sz08_shft).is_pos()) {
constexpr auto data_mask{0x00000000000000FF_u64};
mut_exit_io->size = hypercall::mv_bit_size_t::mv_bit_size_t_8;
mut_exit_io->data = (data_mask & rax).get();
handle_io(mut_sys, mut_exit_io.get(), data_mask);
}
else {
bsl::touch();
Expand Down
35 changes: 29 additions & 6 deletions vmm/src/x64/intel/dispatch_vmexit_io.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,31 @@

namespace microv
{
/// <!-- description -->
/// @brief Handle emulation of port IO read/write accesses
///
/// <!-- input/outputs -->
/// @param mut_sys the bf_syscall_t to use
/// @param pmut_mut_exit_io mv_exit_io_t pointer to use
/// @param data_mask bitmask to apply to either the read or
/// write
/// @return void
///
[[nodiscard]] constexpr auto
handle_io(
syscall::bf_syscall_t &mut_sys,
hypercall::mv_exit_io_t *pmut_mut_exit_io,
bsl::safe_u64 const &data_mask) noexcept
{
auto const rax{mut_sys.bf_tls_rax()};
if (hypercall::MV_EXIT_IO_OUT == pmut_mut_exit_io->type) {
pmut_mut_exit_io->data = (data_mask & rax).get();
}
else {
mut_sys.bf_tls_set_rax((data_mask & pmut_mut_exit_io->data));
Copy link
Member

@rianquinn rianquinn Oct 12, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pmut_mut_exit_io is the shared page, so this is not going to work. I would make sure that any PRs that you put include an integration test to prove that your emulation is doing what you think it is. In this case, I would create a new 16bit VM and have that VM not only write to a port, but also read from a port, and then have the integration test modify the value in some way so that the test and the VM are passing information back and forth.

The reason this will not work is that you got a VMExit from the guest. So, an out instruction works like this

guest: out dx, ax
microv (guest context):  grab dx, ax and instruction info
microv (root context): checkout shared page, fill in with dx, ax and instruction info, return to root
root: save info to emulated port, perform some action, whatever, run guest

When you see an in instruction, this process doesn't change much:

guest: in ax, dx
microv (guest context):  grab dx and instruction info
microv (root context): checkout shared page, fill in with dx and instruction info, return to root
root: read data from emulated port, do whatever, __set guest ax with port info__, run guest

The code that you have here is reading information from the shared page, but this is a message you "send" to the root so that QEMU can handle it, which means that it this code will always return 0 to ax.

The only complication with QEMU on this code might be on the return. Specifically, in KVM_RUN, there are some registers at the bottom that allow QEMU to pass register information back when KVM_RUN is executed. We have to sort out how that works because instead of using KVM_SET_ONE_REG or KVM_SET_REGS to set ax with the port value, QEMU might just do something like kvm_run->rax = port value, and then execute KVM_RUN.

If you notice in https://github.com/Bareflank/MicroV/blob/master/docs/MicroV%20Hypercall%20Specification.md#2159-mv_vs_op_run-op0x6-idx0x8

We have a reg_t that you can set. This code is not there yet, but it is intended to do something similar. Allowing you to set a register when you return.

So IMO, this PR should:

  • be modified to not set rax. This should be done by QEMU, or in your case, and integration test (as these pretend to be QEMU)
  • A new 16bit VM should be created that performs every kind of IO that is possible. Both reads and writes. These should be done in a way that an integration test can verify both the IN and the OUT. This should include the rep prefix, etc... The only thing I would skip for now is string instructions.
  • An integration test should be added like mv_emulated_io. This should provide a means to execute the new VM, and provide the proper emulation and verification that is needed for this new VM, with the goal to ensure that everything for IO is working as intended.

The MMIO stuff should be done the same. The existing shim integration test should be modified to perform delete and modify as well as different memory regions with different flags. A new VM should be created that touches these memory regions, causing in some cases the integration test to be notified of a read or a write and in others, no notifications. At a certain point the integration test should change the layout of memory, and the guest should continue to touch memory and you see a different set of traps. What used to trap no longer does, and what never trapped before now traps, proving that you modified the flags properly and that the TLB was flushed properly. This would be it's own PR of course.

}
}

/// <!-- description -->
/// @brief Dispatches IO VMExits.
///
Expand Down Expand Up @@ -91,7 +116,6 @@ namespace microv
constexpr auto exitqual_idx{syscall::bf_reg_t::bf_reg_t_exit_qualification};
auto const exitqual{mut_sys.bf_vs_op_read(vsid, exitqual_idx)};

auto const rax{mut_sys.bf_tls_rax()};
auto const rcx{mut_sys.bf_tls_rcx()};
auto const rdx{mut_sys.bf_tls_rdx()};

Expand Down Expand Up @@ -131,8 +155,7 @@ namespace microv
mut_exit_io->type = hypercall::MV_EXIT_IO_OUT.get();
}
else {
bsl::error() << "MV_EXIT_IO_IN not implemented\n" << bsl::here();
return bsl::errc_failure;
mut_exit_io->type = hypercall::MV_EXIT_IO_IN.get();
}

constexpr auto bytes1{0_u64};
Expand All @@ -142,7 +165,7 @@ namespace microv
if (bytes4 == ((exitqual & size_mask) >> size_shft)) {
constexpr auto data_mask{0x00000000FFFFFFFF_u64};
mut_exit_io->size = hypercall::mv_bit_size_t::mv_bit_size_t_32;
mut_exit_io->data = (data_mask & rax).get();
handle_io(mut_sys, mut_exit_io.get(), data_mask);
}
else {
bsl::touch();
Expand All @@ -151,7 +174,7 @@ namespace microv
if (bytes2 == ((exitqual & size_mask) >> size_shft)) {
constexpr auto data_mask{0x000000000000FFFF_u64};
mut_exit_io->size = hypercall::mv_bit_size_t::mv_bit_size_t_16;
mut_exit_io->data = (data_mask & rax).get();
handle_io(mut_sys, mut_exit_io.get(), data_mask);
}
else {
bsl::touch();
Expand All @@ -160,7 +183,7 @@ namespace microv
if (bytes1 == ((exitqual & size_mask) >> size_shft)) {
constexpr auto data_mask{0x00000000000000FF_u64};
mut_exit_io->size = hypercall::mv_bit_size_t::mv_bit_size_t_8;
mut_exit_io->data = (data_mask & rax).get();
handle_io(mut_sys, mut_exit_io.get(), data_mask);
}
else {
bsl::touch();
Expand Down