RE: [Xen-devel] [PATCH 00/13] Nested Virtualization: Overview

["Dong, Eddie" <eddie.dong@xxxxxxxxx>]

Christoph Egger wrote:
> Hi!
> 
> This patch series brings Nested Virtualization to Xen.
> This is the sixth patch series. Improvements to the
> previous patch submission:
> 
> - Move GIF definition into SVM
> - Move VMEXIT emulation into SVM
> - Introduce hooks for getting host/guest cr3 for use with hap-on-hap
>    per proposal from Eddie Dong
> - Moved fields from struct nestedhvm into SVM
> - Renamed struct nestedhvm to struct nestedvcpu
> - Reworked VMRUN and VMEXIT emulation. It uses a defered emulation
>    mechanism that makes interrupt handling more efficient and is
>    closer to what VMX is doing
> - VMCB is peristent mapped. Only remap the VMCB when l1 guest
>    changes the address.
> 
> 
> The patch series:
> 
> patch 01: add nestedhvm guest config option to the tools.
>                   This is the only one patch touching the tools
> patch 02: Add data structures for nested virtualization.
> patch 03: add nestedhvm function hooks.
> patch 04: The heart of nested virtualization.
> patch 05: Allow switch to paged real mode during vmrun emulation.
>                   Emulate cr0 and cr4 when guest does not intercept
>                   them (i.e. Hyper-V/Windows7, KVM)
> patch 06: When injecting an exception into nested guest, inject
>                   #VMEXIT into the guest if intercepted.
> patch 07: Allow guest to enable SVM in EFER only on AMD.
> patch 08: Handle interrupts (generic part).
> patch 09: SVM specific implementation for nested virtualization.
> patch 10: Handle interrupts (SVM specific).
> patch 11: The piece of code that effectively turns on nested
> virtualization. patch 12: Move dirty_vram from struct hvm_domain to
>                   struct p2m_domain. This change is the first part
>                   from a larger not-yet-ready change where the vram
>                   and log_dirty tracking is teached to work on per
> p2m. 
> patch 13: Handle nested pagefault to enable hap-on-hap and handle
>                   nested guest page-table-walks to emulate
>                   instructions the guest does not intercept (i.e.
> WBINVD with Windows 7). 

Thanks for the progress! 
I am fine with the patch series in general .

Some minor comments here:

+enum nestedhvm_intercepts {
+       /* exitinfo1 and exitinfo2 undefined */
+       NESTEDHVM_INTERCEPT_INVALID = 0, /* INVALID vmcb/vmcs */
+       NESTEDHVM_INTERCEPT_SHUTDOWN = 1, /* kill guest */
+       NESTEDHVM_INTERCEPT_MCE = 2, /* machine check exception */
+       NESTEDHVM_INTERCEPT_VMMCALL = 3, /* VMMCALL/VMCALL */
+
+       /* exitinfo1 is hvm_intsrc_*, exitinfo2 is the vector */
+       NESTEDHVM_INTERCEPT_INTR = 4, /* interrupt exit code */
+       NESTEDHVM_INTERCEPT_NMI = 5, /* NMI exit code */
+
+       /* exitinfo1 is PF error code, exitinfo2 is PF fault address */
+       NESTEDHVM_INTERCEPT_NPF = 6, /* nested page fault */
+       NESTEDHVM_INTERCEPT_PF = 7, /* page fault */
+
+       /* exceptions: exitinfo1 and exitinfo2 are undefined */
+       NESTEDHVM_INTERCEPT_NM = 8, /* device-not-available */
+
+       /* end mark */
+       NESTEDHVM_INTERCEPT_LAST,
+};

I didn't see where it is used. Did I miss something?


+    union {
+        uint32_t bytes;
+        struct {
+            uint32_t vmentry_pending: 1;
+            uint32_t vmexit_pending: 1;
+            uint32_t vmswitch_in_progress: 1; /* true during vmentry/vmexit 
emulation */
+            uint32_t reserved : 29;
+        } fields;
+    } nv_hostflags;

I feel it is unnecessary to use tricky bit definition (slow and readers may 
think it has some relationship w/ hardware field). Using a simple byte variable 
for each field seems more simple to me.


Both nhvm & nestedhvm is used in API names, what is the policy?

Thx, Eddie


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel