Changeset 71279 in vbox for trunk/src/VBox/VMM/VMMR3

Timestamp:

Mar 8, 2018 7:56:47 PM (7 years ago)

Author:

vboxsync

svn:sync-xref-src-repo-rev:

121208

Message:

NEM: working on the @page docs for windows. bugref:9044

File:

• trunk/src/VBox/VMM/VMMR3/NEMR3.cpp (modified) (7 diffs)

trunk/src/VBox/VMM/VMMR3/NEMR3.cpp

@@ -24 +24 @@
  *
  * On Windows the Hyper-V root partition (dom0 in zen terminology) does not have
- * nested VT-x or AMD-V capabilities.  For a while raw-mode worked in it,
- * however now we \#GP when modifying CR4.  So, when Hyper-V is active on
- * Windows we have little choice but to use Hyper-V to run our VMs.
+ * nested VT-x or AMD-V capabilities.  For a while raw-mode worked inside it,
+ * but for a while now we've been getting \#GP when trying to modify CR4 in the
+ * world switcher.  So, when Hyper-V is active on Windows we have little choice
+ * but to use Hyper-V to run our VMs.
+ *
  *
  * @subsection subsec_nem_win_whv   The WinHvPlatform API
@@ -34 +36 @@
  * This interface is a wrapper around the undocumented Virtualization
  * Infrastructure Driver (VID) API - VID.DLL and VID.SYS.  The wrapper is
- * written in C++, namespaced and early version (at least) was using standard
+ * written in C++, namespaced, early versions (at least) was using standard C++
  * container templates in several places.
  *
@@ -73 +75 @@
  * Running guest code is done thru the WHvRunVirtualProcessor function.  It
  * asynchronously starts or resumes hyper-V CPU execution and then waits for an
- * VMEXIT message.   Other threads can interrupt the execution by using
- * WHvCancelVirtualProcessor, which which case the thread in
- * WHvRunVirtualProcessor is woken up via a dummy QueueUserAPC and will call
- * VidStopVirtualProcessor to asynchronously end execution.  The stop CPU call
- * not immediately succeed if the CPU encountered a VMEXIT before the stop was
- * processed, in which case the VMEXIT needs to be processed first, and the
- * pending stop will be processed in a subsequent call to
- * WHvRunVirtualProcessor.
- *
- * {something about registers}
+ * VMEXIT message.  Hyper-V / VID.SYS will return information about the message
+ * in the message buffer mapping, and WHvRunVirtualProcessor will convert that
+ * into it's own WHV_RUN_VP_EXIT_CONTEXT format.
+ *
+ * Other threads can interrupt the execution by using WHvCancelVirtualProcessor,
+ * which which case the thread in WHvRunVirtualProcessor is woken up via a dummy
+ * QueueUserAPC and will call VidStopVirtualProcessor to asynchronously end
+ * execution.  The stop CPU call not immediately succeed if the CPU encountered
+ * a VMEXIT before the stop was processed, in which case the VMEXIT needs to be
+ * processed first, and the pending stop will be processed in a subsequent call
+ * to WHvRunVirtualProcessor.
+ *
+ * Registers are retrieved and set via WHvGetVirtualProcessorRegisters and
+ * WHvSetVirtualProcessorRegisters.  In addition, several VMEXITs include
+ * essential register state in the exit context information, potentially making
+ * it possible to emulate the instruction causing the exit without involving
+ * WHvGetVirtualProcessorRegisters.
+ *
  *
  * @subsubsection subsubsec_nem_win_whv_cons    Issues / Disadvantages
@@ -92 +102 @@
  *   the VidMessageSlotHandleAndGetNext call.
  *
- *   IIRC this will make the kernel schedule the callback thru
+ *   IIRC this will make the kernel schedule the specified callback thru
  *   NTDLL!KiUserApcDispatcher by modifying the thread context and quite
  *   possibly the userland thread stack.  When the APC callback returns to
  *   KiUserApcDispatcher, it will call NtContinue to restore the old thread
- *   context and resume execution from there.  Upshot this is a bit expensive.
+ *   context and resume execution from there.  This naturally adds up to some
+ *   CPU cycles, ring transitions aren't for free, especially after Spectre &
+ *   Meltdown mitigations.
  *
  *   Using NtAltertThread call could do the same without the thread context
@@ -122 +134 @@
  *   Since MMIO is currently realized as unmapped GPA, this will slow down all
  *   MMIO accesses a tiny little bit as WHvRunVirtualProcessor looks up the
- *   guest physical address the checks if it's a pending lazy mapping.
+ *   guest physical address to check if it is a pending lazy mapping.
+ *
+ *   The lazy mapping feature makes no sense to us.  We as API user have all the
+ *   information and can do lazy mapping ourselves if we want/have to (see next
+ *   point).
  *
  *
  * - There is no API for modifying protection of a page within a GPA range.
  *
- *   We're left with having to unmap the range and then remap it with the new
- *   protection.  For instance we're actively using this to track dirty VRAM
- *   pages, which means there are occational readonly->writable transitions at
- *   run time followed by bulk reversal to readonly when the display is
- *   refreshed.
- *
- *   Now to work around the issue, we do page sized GPA ranges.  In addition to
- *   add a lot of tracking overhead to WinHvPlatform and VID.SYS, it also causes
- *   us to exceed our quota before we've even mapped a default sized VRAM
- *   page-by-page.  So, to work around this quota issue we have to lazily map
- *   pages and actively restrict the number of mappings.
- *
- *   Out best workaround thus far is bypassing WinHvPlatform and VID when in
- *   comes to memory and instead us the hypercalls to do it (HvCallMapGpaPages,
- *   HvCallUnmapGpaPages).  (This also maps a whole lot better into our own
- *   guest page management infrastructure.)
- *
- *
- * - Observed problems doing WHvUnmapGpaRange followed by WHvMapGpaRange.
+ *   From what we can tell, the only way to modify the protection (like readonly
+ *   -> writable, or vice versa) is to first unmap the range and then remap it
+ *   with the new protection.
+ *
+ *   We are for instance doing this quite a bit in order to track dirty VRAM
+ *   pages.  VRAM pages starts out as readonly, when the guest writes to a page
+ *   we take an exit, notes down which page it is, makes it writable and restart
+ *   the instruction.  After refreshing the display, we reset all the writable
+ *   pages to readonly again, bulk fashion.
+ *
+ *   Now to work around this issue, we do page sized GPA ranges.  In addition to
+ *   add a lot of tracking overhead to WinHvPlatform and VID.SYS, this also
+ *   causes us to exceed our quota before we've even mapped a default sized
+ *   (128MB) VRAM page-by-page.  So, to work around this quota issue we have to
+ *   lazily map pages and actively restrict the number of mappings.
+ *
+ *   Our best workaround thus far is bypassing WinHvPlatform and VID entirely
+ *   when in comes to guest memory management and instead use the underlying
+ *   hypercalls (HvCallMapGpaPages, HvCallUnmapGpaPages) to do it ourselves.
+ *   (This also maps a whole lot better into our own guest page management
+ *   infrastructure.)
+ *
+ *
+ * - Observed problems doing WHvUnmapGpaRange immediately followed by
+ *   WHvMapGpaRange.
  *
  *   As mentioned above, we've been forced to use this sequence when modifying
- *   page protection.   However, when upgrading from readonly to writable, we've
- *   ended up looping forever with the same write to readonly memory exit.
+ *   page protection.   However, when transitioning from readonly to writable,
+ *   we've ended up looping forever with the same write to readonly memory
+ *   VMEXIT.  We're wondering if this issue might be related to the lazy mapping
+ *   logic in WinHvPlatform.
  *
  *   Workaround: Insert a WHvRunVirtualProcessor call and make sure to get a GPA
- *   unmapped exit between the two calls.  Terrible for performance and code
- *   sanity.
- *
- *
- * - WHVRunVirtualProcessor wastes time converting VID/Hyper-V messages to it's
- *   own defined format.
+ *   unmapped exit between the two calls.  Not entirely great performance wise
+ *   (or the santity of our code).
+ *
+ *
+ * - WHVRunVirtualProcessor wastes time converting VID/Hyper-V messages to its
+ *   own format (WHV_RUN_VP_EXIT_CONTEXT).
  *
  *   We understand this might be because Microsoft wishes to remain free to
  *   modify the VID/Hyper-V messages, but it's still rather silly and does slow
- *   things down.
+ *   things down a little.  We'd much rather just process the messages directly.
  *
  *
  * - WHVRunVirtualProcessor would've benefited from using a callback interface:
+ *
  *      - The potential size changes of the exit context structure wouldn't be
  *        an issue, since the function could manage that itself.
- *      - State handling could be optimized simplified (esp. cancellation).
+ *
+ *      - State handling could probably be simplified (like cancelation).
  *
  *
@@ -173 +199 @@
  *   internally converts register names, probably using temporary heap buffers.
  *
- *   From the looks of things, it's converting from WHV_REGISTER_NAME to
- *   HV_REGISTER_NAME that's documented in the "Virtual Processor Register
- *   Names" section of "Hypervisor Top-Level Functional Specification".  This
- *   feels like an awful waste of time.  We simply cannot understand why it
- *   wouldn't have sufficed to use HV_REGISTER_NAME here and simply checked the
- *   input values if restrictions were desired.
+ *   From the looks of things, they are converting from WHV_REGISTER_NAME to
+ *   HV_REGISTER_NAME from in the "Virtual Processor Register Names" section in
+ *   the "Hypervisor Top-Level Functional Specification" document.  This feels
+ *   like an awful waste of time.
+ *
+ *   We simply cannot understand why HV_REGISTER_NAME isn't used directly here,
+ *   or at least the same values, making any conversion reduntant.  Restricting
+ *   access to certain registers could easily be implement by scanning the
+ *   inputs.
  *
  *   To avoid the heap + conversion overhead, we're currently using the
@@ -184 +213 @@
  *
  *
+ * - The YMM and XCR0 registers are not yet named (17083).  This probably
+ *   wouldn't be a problem if HV_REGISTER_NAME was used, see previous point.
+ *
+ *
  * - Why does WINHVR.SYS (or VID.SYS) only query/set 32 registers at the time
  *   thru the HvCallGetVpRegisters and HvCallSetVpRegisters hypercalls?
  *
  *   We've not trouble getting/setting all the registers defined by
- *   WHV_REGISTER_NAME in one hypercall...
- *
- *
- * - .
+ *   WHV_REGISTER_NAME in one hypercall (around 80)...
+ *
+ *
+ * - The I/O port exit context information seems to be missing the address size
+ *   information needed for correct string I/O emulation.
+ *
+ *   VT-x provides this information in bits 7:9 in the instruction information
+ *   field on newer CPUs.  AMD-V in bits 7:9 in the EXITINFO1 field in the VMCB.
+ *
+ *   We can probably work around this by scanning the instruction bytes for
+ *   address size prefixes.  Haven't investigated it any further yet.
+ *
+ *
+ * - The WHvGetCapability function has a weird design:
+ *      - The CapabilityCode parameter is pointlessly duplicated in the output
+ *        structure (WHV_CAPABILITY).
+ *
+ *      - API takes void pointer, but everyone will probably be using
+ *        WHV_CAPABILITY due to WHV_CAPABILITY::CapabilityCode making it
+ *        impractical to use anything else.
+ *
+ *      - No output size.
+ *
+ *      - See GetFileAttributesEx, GetFileInformationByHandleEx,
+ *        FindFirstFileEx, and others for typical pattern for generic
+ *        information getters.
+ *
+ *
+ * - The WHvGetPartitionProperty function uses the same weird design as
+ *   WHvGetCapability, see above.
+ *
+ *
+ * - The WHvSetPartitionProperty function has a totally weird design too:
+ *      - In contrast to its partner WHvGetPartitionProperty, the property code
+ *        is not a separate input parameter here but part of the input
+ *        structure.
+ *
+ *      - The input structure is a void pointer rather than a pointer to
+ *        WHV_PARTITION_PROPERTY which everyone probably will be using because
+ *        of the WHV_PARTITION_PROPERTY::PropertyCode field.
+ *
+ *      - Really, why use PVOID for the input when the function isn't accepting
+ *        minimal sizes.  E.g. WHVPartitionPropertyCodeProcessorClFlushSize only
+ *        requires a 9 byte input, but the function insists on 16 bytes (17083).
+ *
+ *      - See GetFileAttributesEx, SetFileInformationByHandle, FindFirstFileEx,
+ *        and others for typical pattern for generic information setters and
+ *        getters.
+ *
+ *
+ * @subsection subsec_nem_win_impl   Our implementation.
+ *
+ * Tomorrow...
+ *
  *
  */