Skip to contentSkip to conversation

Nested Virtualization: core

PATCH In review Christoph Egger 2 participants 3 messages Aug 5, 2010 – Aug 10, 2010

lore
Share & tools
lore
#0 lore raw

PATCH 5/14 Nested Virtualization: core

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

# HG changeset patch
# User cegger
# Date 1280925500 -7200
Nested Virtualization core implementation

xen/arch/x86/hvm/Makefile +1 -0
diff -r c19b4e3959c5 -r 917f9eb07fc3 xen/arch/x86/hvm/Makefile
@@ -10,6 +10,7 @@ obj-y += intercept.o
 obj-y += io.o
 obj-y += irq.o
 obj-y += mtrr.o
+obj-y += nestedhvm.o
 obj-y += pmtimer.o
 obj-y += quirks.o
 obj-y += rtc.o
xen/arch/x86/hvm/nestedhvm.c new file +673 -0

673 lines not on this page · raw message

xen/include/asm-x86/hvm/nestedhvm.h new file +139 -0

139 lines not on this page · raw message

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

Hi,

xen/arch/x86/hvm/nestedhvm.c line 155

Quoted 12 lines
xen/arch/x86/hvm/nestedhvm.c new file +12 -0
+int
+nestedhvm_vcpu_destroy(struct vcpu *v)
+{
+	int ret = 0;
+
+	if (!nestedhvm_enabled(v->domain))
+		return 0;
+
+	if (nestedhvm_vcpu_in_guestmode(v)) {
+		nestedhvm_vcpu_exit_guestmode(v);
+		ret = -EBUSY;
+	}

This still bothers me, since we go on to complete the teardown anyway, and this function's only caller does nothing interesting with the result.

xen/arch/x86/hvm/nestedhvm.c line 162

Quoted 6 lines
xen/arch/x86/hvm/nestedhvm.c new file +6 -0
+	/* Enable interrupts or the guest won't see any interrupts
+	 * after nested guest exited.
+	 */
+	nestedsvm_vcpu_stgi(v);
+
+	hvm_nestedhvm_vcpu_destroy(v);

While I'm complaining about unchecked return values, I should point out that your implementation of this returns an integer (which in turn is always 0).

Review

Quoted 2 lines
(quoted diff) +2 -0
+	return ret;
+}

Cheers,

Tim.

Signature · 3 lines
Tim Deegan <[email protected]>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)
#2 lore raw

Hi Tim,

It looks like I have missed this from your first review. Thanks for bringing this up again.

Quoted 18 lines · On Monday 09 August 2010 14:38:48 Tim Deegan wrote:

Hi,

xen/arch/x86/hvm/nestedhvm.c line 155

Quoted 12 lines
xen/arch/x86/hvm/nestedhvm.c new file +12 -0
+int
+nestedhvm_vcpu_destroy(struct vcpu *v)
+{
+	int ret = 0;
+
+	if (!nestedhvm_enabled(v->domain))
+		return 0;
+
+	if (nestedhvm_vcpu_in_guestmode(v)) {
+		nestedhvm_vcpu_exit_guestmode(v);
+		ret = -EBUSY;
+	}

This still bothers me, since we go on to complete the teardown anyway, and this function's only caller does nothing interesting with the result.

I removed "vcpu in guestmode" check completely.

Quoted 4 lines

xen/arch/x86/hvm/nestedhvm.c line 160

Quoted 4 lines
xen/arch/x86/hvm/nestedhvm.c new file +4 -0
+	/* Enable interrupts or the guest won't see any interrupts
+	 * after nested guest exited.
+	 */
+	nestedsvm_vcpu_stgi(v);

This stgi call is gone.

Quoted 6 lines

Review

Quoted 2 lines
(quoted diff) +2 -0
+
+	hvm_nestedhvm_vcpu_destroy(v);

While I'm complaining about unchecked return values, I should point out that your implementation of this returns an integer (which in turn is always 0).

I changed this to 'return hvm_nestedhvm_vcpu_destroy(v);'

Quoted 1 line

Review

Quoted 1 line
(quoted diff) +1 -0
+	return ret;

This return is gone.

Quoted 1 line
Quoted 1 line

+}

Christoph

Signature · 6 lines
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85609 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632