Skip to content

Conversation

@brendank310
Copy link
Member

Signed-off-by: Brendan Kerrigan [email protected]

@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #52 (f544fe0) into master (db7587d) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##            master       #52   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          205       205           
  Lines         5493      5493           
  Branches       107       107           
=========================================
  Hits          5493      5493           

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

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.

@brendank310
Copy link
Member Author

Superceded by: #73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants