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

Hi!

This patch series brings Nested Virtualization to Xen.
This is the third patch series. Improvements to the
previous patch submission:

- Incorporated feedback discussion with Keir, Tim and Eddie.

There might still be some nuances to fiddle with to make it
fit for VMX. Feedback from Intel is appreciated, therefore.

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.
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).


-- 
---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


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

The fundamental argument of whether we should convert vendor specific code into
vendor neutral code is not solved yet.I guess I don;t need to review rest of the
code due to this :)
I strongly suggest we remove those unnecessary wrapper for readibilty,
flexibility and performance.

BTW, VMX has a policy to access VMCS field only when it is a must.

Thanks, Eddie



Christoph Egger wrote:
> Hi!
> 
> This patch series brings Nested Virtualization to Xen.
> This is the third patch series. Improvements to the
> previous patch submission:
> 
> - Incorporated feedback discussion with Keir, Tim and Eddie.
> 
> There might still be some nuances to fiddle with to make it
> fit for VMX. Feedback from Intel is appreciated, therefore.
> 
> 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.
> 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). 

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

On Friday 03 September 2010 10:12:08 Dong, Eddie wrote:
> The fundamental argument of whether we should convert vendor specific code
> into vendor neutral code is not solved yet.
> I guess I don;t need to review rest of the code due to this :) I strongly
> suggest we remove those unnecessary wrapper for readibilty, flexibility and
> performance. 
I am sort of surprised that you raise the same things again that have been
discussed in the last round. Please correct me if I am wrong: This sounds to
me that the statements/explanations from Keir, Tim and me are not clear
to you. Please feel free to ask whatever is unclear / questionable to you.
> BTW, VMX has a policy to access VMCS field only when it is a must.
SVM, too. AFAICS, the difference is the method *how* to access fields.

Christoph
> Thanks, Eddie
>
> Christoph Egger wrote:
> > Hi!
> >
> > This patch series brings Nested Virtualization to Xen.
> > This is the third patch series. Improvements to the
> > previous patch submission:
> >
> > - Incorporated feedback discussion with Keir, Tim and Eddie.
> >
> > There might still be some nuances to fiddle with to make it
> > fit for VMX. Feedback from Intel is appreciated, therefore.
> >
> > 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.
> > 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).


-- 
---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


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

Christoph Egger wrote:
> On Friday 03 September 2010 10:12:08 Dong, Eddie wrote:
>> The fundamental argument of whether we should convert vendor
>> specific code into vendor neutral code is not solved yet.
>> I guess I don;t need to review rest of the code due to this :) I
>> strongly suggest we remove those unnecessary wrapper for readibilty,
>> flexibility and performance.
> 
> I am sort of surprised that you raise the same things again that have
> been discussed in the last round. Please correct me if I am wrong:
> This sounds to me that the statements/explanations from Keir, Tim and
> me are not clear 
> to you. Please feel free to ask whatever is unclear / questionable to
> you. 
So you are actually not asking for my review comments, though you explicitly
called it.

I am not convinced a wrapper for wrapping, which is just make it like C style as
you said, should be taken just because somebody have called it out.

Freely (at any time) preload/post-store of VMCS fields is very hard because VMX
only provide access to current VMCS (because it is CPU register), while SVM may
be able to access all VMCBs given that it is in memory. I can''t say it
is impossible (one can solve it with further limitation/complexity), however
enforcing those conversion simply put trickies to VMX code to limiting the time
of pre-load/post-load, and the additional cost of VMCS access.



Another portion of so called common code are actually SVM code only. Here are
part of them:



+
+static enum nestedhvm_vmexits
+nestedhvm_vmexit_msr(unsigned long *msr_bitmap, uint32_t msr, bool_t write)
+{
+	bool_t enabled;
+	unsigned long *msr_bit = NULL;
+
+	/*
+	 * See AMD64 Programmers Manual, Vol 2, Section 15.10
+	 * (MSR-Bitmap Address).
+	 */
+	if ( msr <= 0x1fff )
+		msr_bit = msr_bitmap + 0x0000 / BYTES_PER_LONG;
+	else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
+		msr_bit = msr_bitmap + 0x0800 / BYTES_PER_LONG;
+	else if ( (msr >= 0xc0010000) && (msr <= 0xc0011fff) )
+		msr_bit = msr_bitmap + 0x1000 / BYTES_PER_LONG;



+/* Virtual GIF */
+int
+nestedsvm_vcpu_clgi(struct vcpu *v)
+{
+	if (!nestedhvm_enabled(v->domain)) {
+		hvm_inject_exception(TRAP_invalid_op, 0, 0);
+		return -1;
+	}
+
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+		return 0;
+
+	/* clear gif flag */
+	vcpu_nestedhvm(v).nh_gif = 0;
+	local_event_delivery_disable(); /* mask events for PV drivers */
+	return 0;
+}
+
+int
+nestedsvm_vcpu_stgi(struct vcpu *v)
+{
+	if (!nestedhvm_enabled(v->domain)) {
+		hvm_inject_exception(TRAP_invalid_op, 0, 0);
+		return -1;
+	}
+
+	/* Always set the GIF to make hvm_interrupt_blocked work. */
+	vcpu_nestedhvm(v).nh_gif = 1;
+
+	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
+		return 0;
+
+	local_event_delivery_enable(); /* unmask events for PV drivers */
+	return 0;
+}
+

> 
>> BTW, VMX has a policy to access VMCS field only when it is a must.
> 
> SVM, too. AFAICS, the difference is the method *how* to access fields.
> 
> Christoph
> 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Saturday 04 September 2010 03:30:56 Dong, Eddie
wrote:
> Christoph Egger wrote:
> > On Friday 03 September 2010 10:12:08 Dong, Eddie wrote:
> >> The fundamental argument of whether we should convert vendor
> >> specific code into vendor neutral code is not solved yet.
> >> I guess I don;t need to review rest of the code due to this :) I
> >> strongly suggest we remove those unnecessary wrapper for
readibilty,
> >> flexibility and performance.
> >
> > I am sort of surprised that you raise the same things again that have
> > been discussed in the last round. Please correct me if I am wrong:
> > This sounds to me that the statements/explanations from Keir, Tim and
> > me are not clear
> > to you. Please feel free to ask whatever is unclear / questionable to
> > you.
>
> So you are actually not asking for my review comments, though you
> explicitly called it.
Well, when you say so :)
> I am not convinced a wrapper for wrapping, which is just make it like C
> style as you said, should be taken just because somebody have called it
> out.
I wouldn''t call it a wrapper. I call as the implementation of an
abstraction.

And regarding to what Tim said:
http://lists.xensource.com/archives/html/xen-devel/2010-08/msg01044.html

Is there a particular reason not to do so apart from the LOC as you brought
up in http://lists.xensource.com/archives/html/xen-devel/2010-08/msg01255.html

(As Tim said in
http://lists.xensource.com/archives/html/xen-devel/2010-08/msg01256.html
it''s the duplication of logic that counts.)
> Freely (at any time) preload/post-store of VMCS fields is very hard because
> VMX only provide access to current VMCS (because it is CPU register), while
> SVM may be able to access all VMCBs given that it is in memory. I
can''t say
> it is impossible (one can solve it with further limitation/complexity),
> however enforcing those conversion simply put trickies to VMX code to
> limiting the time of pre-load/post-load, and the additional cost of VMCS
> access.
>
When the VCPU switches between host and guest mode then you need to
save the l1 guest state, restore the l2 guest state and vice versa.

This requires a lot of accesses from/to the vmcb/vmcs  unless you have
a lazy switching technique, do you ?

>
> Another portion of so called common code are actually SVM code only. Here
> are part of them:
>
>
>
> +
> +static enum nestedhvm_vmexits
> +nestedhvm_vmexit_msr(unsigned long *msr_bitmap, uint32_t msr, bool_t
> write) +{
> +	bool_t enabled;
> +	unsigned long *msr_bit = NULL;
> +
> +	/*
> +	 * See AMD64 Programmers Manual, Vol 2, Section 15.10
> +	 * (MSR-Bitmap Address).
> +	 */
> +	if ( msr <= 0x1fff )
> +		msr_bit = msr_bitmap + 0x0000 / BYTES_PER_LONG;
> +	else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
> +		msr_bit = msr_bitmap + 0x0800 / BYTES_PER_LONG;
> +	else if ( (msr >= 0xc0010000) && (msr <= 0xc0011fff) )
> +		msr_bit = msr_bitmap + 0x1000 / BYTES_PER_LONG;
>
Why does above code snippet not work on Intel CPUs ?

>
> +/* Virtual GIF */
> +int
> +nestedsvm_vcpu_clgi(struct vcpu *v)
> +{
> +	if (!nestedhvm_enabled(v->domain)) {
> +		hvm_inject_exception(TRAP_invalid_op, 0, 0);
> +		return -1;
> +	}
> +
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +		return 0;
> +
> +	/* clear gif flag */
> +	vcpu_nestedhvm(v).nh_gif = 0;
> +	local_event_delivery_disable(); /* mask events for PV drivers */
> +	return 0;
> +}
> +
> +int
> +nestedsvm_vcpu_stgi(struct vcpu *v)
> +{
> +	if (!nestedhvm_enabled(v->domain)) {
> +		hvm_inject_exception(TRAP_invalid_op, 0, 0);
> +		return -1;
> +	}
> +
> +	/* Always set the GIF to make hvm_interrupt_blocked work. */
> +	vcpu_nestedhvm(v).nh_gif = 1;
> +
> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> +		return 0;
> +
> +	local_event_delivery_enable(); /* unmask events for PV drivers */
> +	return 0;
> +}
> +
The reason to leave this in the generic code is what Keir stated out as
feedback to the second patch series:
http://lists.xensource.com/archives/html/xen-devel/2010-06/msg00280.html
(What was patch #10 there is  patch #8 in latest patch series).

Christoph


-- 
---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


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

>> I am not convinced a wrapper for wrapping, which is just make it
>> like C style as you said, should be taken just because somebody have
>> called it out.
> 
> I wouldn''t call it a wrapper. I call as the implementation of an
> abstraction. 

While, that is terminology difference, abstract for abstract is as same bad as
wrapper for wrapper. One good example is SVM on VMX that you abstracted months
ago.
Extending SVM knowledge to VMX and limiting the flexbility of VMX implementation
(in case you won''t implement VMX code) is not a practical solution IMO.
Nested VMX will touch existing code more comprehensively.
>> Freely (at any time) preload/post-store of VMCS fields is very hard
>> because VMX only provide access to current VMCS (because it is CPU
>> register), while SVM may be able to access all VMCBs given that it
>> is in memory. I can''t say it is impossible (one can solve it
with
>> further limitation/complexity), however enforcing those conversion
>> simply put trickies to VMX code to limiting the time of
>> pre-load/post-load, and the additional cost of VMCS access. 
>> 
> 
> When the VCPU switches between host and guest mode then you need to
> save the l1 guest state, restore the l2 guest state and vice versa.
No. Switching from L1 to L2 guest, it is a simple restore of L2 guest state
(VMCLEAR/VMPTRLD). VMX doesn''t save L1 state, while SVM does require
the save of L1 state per my understanding. Intel process can hold VMCS in
processor for performance.

Switching from L2 to L1, we need to convert (or store) some VMCS information
from physical to virtual VMCS. But it is limited and only covers the "guest
state" and exit information. Load of L1 state may be as simple as VMPTRLD
(of course it may modify some VMCS field upon different situation).

This is another good example how risky it is to extend SVM knowledge to VMX.
> 
> This requires a lot of accesses from/to the vmcb/vmcs  unless you have
> a lazy switching technique, do you ?
Yes, the Intel hardware already did lazy switching thru VMPTRLD.
And the access of VMCS is depending on the L1 guest modification, only dirty
VMCS fields needs to be updated.

In the majority case, the VM exit from L2 guest will be handled by root VMM
directly. One example is external interrupt, which doesn''t need to
access rest of VMCS fields except the reason, but the wrapper makes the access a
must, which I can''t agree.
> 
> 
>> 
>> Another portion of so called common code are actually SVM code only.
>> Here are part of them: 
>> 
>> 
>> 
>> +
>> +static enum nestedhvm_vmexits
>> +nestedhvm_vmexit_msr(unsigned long *msr_bitmap, uint32_t msr,
>> bool_t write) +{ +	bool_t enabled;
>> +	unsigned long *msr_bit = NULL;
>> +
>> +	/*
>> +	 * See AMD64 Programmers Manual, Vol 2, Section 15.10 +	 *
>> (MSR-Bitmap Address). +	 */
>> +	if ( msr <= 0x1fff )
>> +		msr_bit = msr_bitmap + 0x0000 / BYTES_PER_LONG;
>> +	else if ( (msr >= 0xc0000000) && (msr <= 0xc0001fff) )
>> +		msr_bit = msr_bitmap + 0x0800 / BYTES_PER_LONG;
>> +	else if ( (msr >= 0xc0010000) && (msr <= 0xc0011fff) )
>> +		msr_bit = msr_bitmap + 0x1000 / BYTES_PER_LONG;
>> 
> 
> Why does above code snippet not work on Intel CPUs ?
It is said even "Intel processor" is following AMD64 manual, isn;t it?
msr_bitmap in Intel doesn''t have a fixed bitmap position, rather it
scan the entire table to decide which MSR to automatically save/restore for
performance given that we only put single digital MSRs for that purpose.
BTW, Intel does implement MSRs large than 0x1fff such as 0x107D0. Preassuming
their usage model for now is risky.
> 
> 
>> 
>> +/* Virtual GIF */
>> +int
>> +nestedsvm_vcpu_clgi(struct vcpu *v)
>> +{
>> +	if (!nestedhvm_enabled(v->domain)) {
>> +		hvm_inject_exception(TRAP_invalid_op, 0, 0);
>> +		return -1;
>> +	}
>> +
>> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
>> +		return 0;
>> +
>> +	/* clear gif flag */
>> +	vcpu_nestedhvm(v).nh_gif = 0;
>> +	local_event_delivery_disable(); /* mask events for PV drivers */
>> +	return 0; +}
>> +
>> +int
>> +nestedsvm_vcpu_stgi(struct vcpu *v)
>> +{
>> +	if (!nestedhvm_enabled(v->domain)) {
>> +		hvm_inject_exception(TRAP_invalid_op, 0, 0);
>> +		return -1;
>> +	}
>> +
>> +	/* Always set the GIF to make hvm_interrupt_blocked work. */
>> +	vcpu_nestedhvm(v).nh_gif = 1;
>> +
>> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
>> +		return 0;
>> +
>> +	local_event_delivery_enable(); /* unmask events for PV drivers */
>> +	return 0; +}
>> +
> 
> The reason to leave this in the generic code is what Keir stated out
> as feedback to the second patch series:
> http://lists.xensource.com/archives/html/xen-devel/2010-06/msg00280.html
> (What was patch #10 there is  patch #8 in latest patch series).
> 
While, I didn;t read in this way. Even the function itself is wrapped, they
should go to SVM tree if not go together with the caller.

Anyway, to me given that nested SVM & VMX is still on the very beginning of
development, I can only say yes to those wrappers that have clear understanding
to both side. I would rather leave those uncertain wrappers to future, once the
basic shape of nested virtualization is good and stable enough, i.e. lightweight
wrapper.  We have plenty of performance work ahead such as virtual VTd support,
enhanced PV driver for nested etc. Excessive wrapper is simple a burden to
nested VMX developers for those future feaftures.

Qing will post his patch today or tomorrow for your reference if you want.

C++ abstracts better than C, but C++ never replaces C in places like Linux. 

BTW, can you add SR-IOV test into your patch test system to avoid regression?

Thx, Eddie

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

Thank you very much for giving me the valuable information I am asking for, 
finally.
> >> Freely (at any time) preload/post-store of VMCS fields is very
hard
> >> because VMX only provide access to current VMCS (because it is CPU
> >> register), while SVM may be able to access all VMCBs given that it
> >> is in memory. I can''t say it is impossible (one can solve
it with
> >> further limitation/complexity), however enforcing those conversion
> >> simply put trickies to VMX code to limiting the time of
> >> pre-load/post-load, and the additional cost of VMCS access.
> >
> > When the VCPU switches between host and guest mode then you need to
> > save the l1 guest state, restore the l2 guest state and vice versa.
>
> No. Switching from L1 to L2 guest, it is a simple restore of L2 guest state
> (VMCLEAR/VMPTRLD). VMX doesn''t save L1 state,
So the function hooks prepare4vmentry is pretty cheap and the
''hostsave''
function hook is empty.
> while SVM does require the save of L1 state per my understanding.
SVM does not per architecture. It is the implementation I have taken over from
KVM. It will be replaced with an implementation that only requires a VMSAVE
to save the L1 state. But that won''t change the patch series
fundamentally as
this change touches SVM code only.
> Intel process can hold VMCS in processor for performance.
>
> Switching from L2 to L1, we need to convert (or store) some VMCS
> information from physical to virtual VMCS.
That is what can be put into the prepare4vmexit function hook.
> But it is limited and only covers the "guest state" and exit
information.
> Load of L1 state may be as simple as VMPTRLD (of course it may modify some
> VMCS field upon different situation).
That is what can be put into the ''hostrestore'' function hook.
> > This requires a lot of accesses from/to the vmcb/vmcs  unless you have
> > a lazy switching technique, do you ?
>
> Yes, the Intel hardware already did lazy switching thru VMPTRLD.
> And the access of VMCS is depending on the L1 guest modification, only
> dirty VMCS fields needs to be updated.
Sounds like you need a ''shadow vmcs'' that holds the l2 guest
state.
> In the majority case, the VM exit from L2 guest will be handled by root VMM
> directly.
Same on SVM. root VMM handles everything L1 guest does not intercept plus some
special intercepts such as interrupts, nmi''s, page fault/nested page
faults.
> One example is external interrupt, which doesn''t need to access 
> rest of VMCS fields except the reason, but the wrapper makes the access a
> must, which I can''t agree.
Which conversion function do you mean by ''wrapper'' here ? Why
does it require
additional VMCS field accesses ?

Can you explain this in detail, please ?

> >> Another portion of so called common code are actually SVM code
only.
> >> Here are part of them:
> >>
> >>
> >>
> >> +
> >> +static enum nestedhvm_vmexits
> >> +nestedhvm_vmexit_msr(unsigned long *msr_bitmap, uint32_t msr,
> >> bool_t write) +{ +	bool_t enabled;
> >> +	unsigned long *msr_bit = NULL;
> >> +
> >> +	/*
> >> +	 * See AMD64 Programmers Manual, Vol 2, Section 15.10 +	 *
> >> (MSR-Bitmap Address). +	 */
> >> +	if ( msr <= 0x1fff )
> >> +		msr_bit = msr_bitmap + 0x0000 / BYTES_PER_LONG;
> >> +	else if ( (msr >= 0xc0000000) && (msr <=
0xc0001fff) )
> >> +		msr_bit = msr_bitmap + 0x0800 / BYTES_PER_LONG;
> >> +	else if ( (msr >= 0xc0010000) && (msr <=
0xc0011fff) )
> >> +		msr_bit = msr_bitmap + 0x1000 / BYTES_PER_LONG;
> >
> > Why does above code snippet not work on Intel CPUs ?
>
> It is said even "Intel processor" is following AMD64 manual,
isn;t it?
> msr_bitmap in Intel doesn''t have a fixed bitmap position, rather
it scan
> the entire table to decide which MSR to automatically save/restore for
> performance given that we only put single digital MSRs for that purpose.
> BTW, Intel does implement MSRs large than 0x1fff such as 0x107D0.
> Preassuming their usage model for now is risky.
Ok, I will think about this.
> >> +/* Virtual GIF */
> >> +int
> >> +nestedsvm_vcpu_clgi(struct vcpu *v)
> >> +{
> >> +	if (!nestedhvm_enabled(v->domain)) {
> >> +		hvm_inject_exception(TRAP_invalid_op, 0, 0);
> >> +		return -1;
> >> +	}
> >> +
> >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> >> +		return 0;
> >> +
> >> +	/* clear gif flag */
> >> +	vcpu_nestedhvm(v).nh_gif = 0;
> >> +	local_event_delivery_disable(); /* mask events for PV drivers */
> >> +	return 0; +}
> >> +
> >> +int
> >> +nestedsvm_vcpu_stgi(struct vcpu *v)
> >> +{
> >> +	if (!nestedhvm_enabled(v->domain)) {
> >> +		hvm_inject_exception(TRAP_invalid_op, 0, 0);
> >> +		return -1;
> >> +	}
> >> +
> >> +	/* Always set the GIF to make hvm_interrupt_blocked work. */
> >> +	vcpu_nestedhvm(v).nh_gif = 1;
> >> +
> >> +	if (boot_cpu_data.x86_vendor != X86_VENDOR_AMD)
> >> +		return 0;
> >> +
> >> +	local_event_delivery_enable(); /* unmask events for PV drivers
*/
> >> +	return 0; +}
> >> +
> >
> > The reason to leave this in the generic code is what Keir stated out
> > as feedback to the second patch series:
> >
http://lists.xensource.com/archives/html/xen-devel/2010-06/msg00280.html
> > (What was patch #10 there is  patch #8 in latest patch series).
>
> While, I didn;t read in this way. Even the function itself is wrapped, they
> should go to SVM tree if not go together with the caller.
>
> Anyway, to me given that nested SVM & VMX is still on the very
beginning of
> development, I can only say yes to those wrappers that have clear
> understanding to both side.
Good to know. See my offer below.
> I would rather leave those uncertain wrappers to future, once the basic
> shape of nested virtualization is good and stable enough, i.e. lightweight
> wrapper.  We have plenty of performance work ahead such as virtual VTd
> support, enhanced PV driver for nested etc. Excessive wrapper is simple a
> burden to nested VMX developers for those future features.
>
> Qing will post his patch today or tomorrow for your reference if you want.
Thanks. May I take code from there and add into my patch series ?
> C++ abstracts better than C, but C++ never replaces C in places like Linux.
It''s not necessary. When you truely understand the concept behind
interface
and implementation you can do that in any language you know.
> BTW, can you add SR-IOV test into your patch test system to avoid
> regression?
Yes.
> Thx, Eddie


-- 
---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


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

>>> When the VCPU switches between host and guest mode then you need to
>>> save the l1 guest state, restore the l2 guest state and vice versa.
>> 
>> No. Switching from L1 to L2 guest, it is a simple restore of L2
>> guest state (VMCLEAR/VMPTRLD). VMX doesn''t save L1 state,
> 
> So the function hooks prepare4vmentry is pretty cheap and the
> ''hostsave'' function hook is empty.
I don;t know how you conclude here, extending SVM knowledge to VMX again?
Prepare4vmentry has to load several VMCS fields, which is expansive in VMX.
>From SVM code, it loads 11 VMCB fields, which may be 11 VMCS field access in
VMX.
nsvm_vmcb_prepare4vmexit does reverse thing which touches at least 6 VMCS field
again.

Those 17 VMCS access already far exceeds the average VMCS access # in Xen for a
typical VM exit, and even may be more expansive than an entire SW VM exit
handler in some old platforms (The modern processor may be pretty fast though).
In many cases (even with nested virtualization), we can complete the VM exit
handling in 2-3 VMCS field access.
> 
>> while SVM does require the save of L1 state per my understanding.
> 
> SVM does not per architecture. It is the implementation I have taken
> over from KVM. It will be replaced with an implementation that only
> requires a VMSAVE to save the L1 state. But that won''t change the
> patch series fundamentally as this change touches SVM code only.
> 
>> Intel process can hold VMCS in processor for performance.
>> 
>> Switching from L2 to L1, we need to convert (or store) some VMCS
>> information from physical to virtual VMCS.
> 
> That is what can be put into the prepare4vmexit function hook.
No, all those are conditionally and accessible at current VMCS context only.
It is not guranteed at the time of prepare4vmexit. It is particular risky if we
change the nested VMX code algorithm.
> 
>> But it is limited and only covers the "guest state" and exit
>> information. Load of L1 state may be as simple as VMPTRLD (of course
>> it may modify some VMCS field upon different situation).
> 
> That is what can be put into the ''hostrestore'' function
hook.
> 
>>> This requires a lot of accesses from/to the vmcb/vmcs  unless you
>>> have a lazy switching technique, do you ?
>> 
>> Yes, the Intel hardware already did lazy switching thru VMPTRLD.
>> And the access of VMCS is depending on the L1 guest modification,
>> only dirty VMCS fields needs to be updated.
> 
> Sounds like you need a ''shadow vmcs'' that holds the l2
guest state.
> 
>> In the majority case, the VM exit from L2 guest will be handled by
>> root VMM directly.
> 
> Same on SVM. root VMM handles everything L1 guest does not intercept
> plus some special intercepts such as interrupts, nmi''s, page
> fault/nested page faults. 
That is the key reason that I strongly against the wrapper of VM exit and VM
entry. W/o the wrapper, we can complete the handler in 2-3 VMCS field access in
most case, but w/ the wrapper, we have to spend 17+ VMCS field access.
> 
>> One example is external interrupt, which doesn''t need to
access
>> rest of VMCS fields except the reason, but the wrapper makes the
>> access a must, which I can''t agree.
> 
> Which conversion function do you mean by ''wrapper'' here ?
Why does it
> require additional VMCS field accesses ?
> 
> Can you explain this in detail, please ?
> 
see before. nsvm_vmcb_prepare4vmexit and Prepare4vmentry.

>>> The reason to leave this in the generic code is what Keir stated
out
>>> as feedback to the second patch series:
>>>
http://lists.xensource.com/archives/html/xen-devel/2010-06/msg00280.html
>>> (What was patch #10 there is  patch #8 in latest patch series).
>> 
>> While, I didn;t read in this way. Even the function itself is
>> wrapped, they should go to SVM tree if not go together with the
>> caller. 
>> 
>> Anyway, to me given that nested SVM & VMX is still on the very
>> beginning of development, I can only say yes to those wrappers that
>> have clear understanding to both side.
> 
> Good to know. See my offer below.
I am not sure if you want to efficiently start with lighweight wrapper first
(those wrapper with consense and individual solution for those not), or you want
to keep spinning on the heavy wrapper.  For me, if you go with lighweight
wrapper like single layer VMX/SVM virtualization does, I can ack soon. But for
the heavy weight wrapper, it is not necessary to me and will block future
development of nested VMX, so I can''t ack. Even current code is a lot
of hard to understand given that I need to understand the semantics of those new
(vendor-neutral) fields, however the semantics of VMX fields are clear to all
VMX developers. Sorry for that, although I know Tim is more considerative to the
effort you have called it out. I appreciate your effort as well, but that reason
is not strong enough yet to impact nested VMX development.
> 
>> I would rather leave those uncertain wrappers to future, once the
>> basic shape of nested virtualization is good and stable enough, i.e.
>> lightweight wrapper.  We have plenty of performance work ahead such
>> as virtual VTd support, enhanced PV driver for nested etc. Excessive
>> wrapper is simple a burden to nested VMX developers for those future
>> features. 
>> 
>> Qing will post his patch today or tomorrow for your reference if you
>> want. 
> 
> Thanks. May I take code from there and add into my patch series ?
Sure. But you won;t assume this is the logic we won''t change. The more
performance tunning happens, the more we may change, and it is possible some new
hardware features may come some time later.

Thx, Eddie
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Hi, 

Christoph, I''ve posted separate comments on patches 5, 12 and 13.
Otherwise this series is looking OK to me -- provided that you and Eddie
can agree on how the common code is going to work.

Eddie - I don''t think having a common vmexit interface _requires_ you
to
do 17 VMREADs on every VMEXIT (and if it does, that needs to be
changed).  The SVM implementation may pull all the information out at
once but the VMX one could do it lazily.

Christoph, I asked before about whether we could lose the extra
namespace for exit reasons.  Looking at the way they''re used,
couldn''t
we just pass around the native numbers and maybe some flags for which of
the ancillary fields are valid?

Cheers,

Tim.


At 15:53 +0100 on 01 Sep (1283356406), Christoph Egger
wrote:
> 
> Hi!
> 
> This patch series brings Nested Virtualization to Xen.
> This is the third patch series. Improvements to the
> previous patch submission:
> 
> - Incorporated feedback discussion with Keir, Tim and Eddie.
> 
> There might still be some nuances to fiddle with to make it
> fit for VMX. Feedback from Intel is appreciated, therefore.
> 
> 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.
> 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).
> 
> 
> -- 
> ---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
> 
-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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

On Wednesday 08 September 2010 10:58:52 Dong, Eddie
wrote:
> >>> When the VCPU switches between host and guest mode then you
need to
> >>> save the l1 guest state, restore the l2 guest state and vice
versa.
> >>
> >> No. Switching from L1 to L2 guest, it is a simple restore of L2
> >> guest state (VMCLEAR/VMPTRLD). VMX doesn''t save L1 state,
> >
> > So the function hooks prepare4vmentry is pretty cheap and the
> > ''hostsave'' function hook is empty.
>
> I don;t know how you conclude here, extending SVM knowledge to VMX again?
> Prepare4vmentry has to load several VMCS fields, which is expansive in VMX.
>
> From SVM code, it loads 11 VMCB fields, which may be 11 VMCS field access
> in VMX. nsvm_vmcb_prepare4vmexit does reverse thing which touches at least
> 6 VMCS field again.
I better start with explaining the basic thing for software design: Interface
and Implementation.

Think on cars. All cars are different but they all have three pedals and a 
steering wheel. The three pedals and the steering wheel have the same
function in all cars. To drive a car you need to know what they do and how you
use them. But you do not have to know how they work.

Speaking in software design, this is called an interface.

How the pedals and the steering wheel work is up to the car manufacturers how
they make them work to function as intended. They build different motors
with different numbers of cylinders, one needs benzine an other needs diesel.
By concept the car manufacturer can even build in hamster wheels as a motor
(Getting the hamster running faster/slower is up to the car manufacturer
when using the pedals).

Speaking in software design, this is called an implementation.


Now translated to nested virtualization, the interface
''prepare4vmentry''
does prepare the l2 guest state ready to run. The way you think you have to
implement it is a possible way to make it work but the assumption you have
to follow the SVM implementation is wrong.

If I were you I would take the function ''virtual_vmentry'' from
http://lists.xensource.com/archives/html/xen-devel/2010-04/msg01166.html
and implement ''prepare4vmrun'' this way:

static void virtual_vmentry(struct vcpu *v, struct cpu_user_regs *regs)
{
    struct nestedhvm *hvm = &vcpu_nestedhvm(v);
    struct vmx_nest_struct *nest = hvm->nh_arch;
    struct vmcs_struct *svmcs = hvm->nh_vm;
    struct vmcs_struct *host_vmcs = v->arch.hvm_vmx.vmcs;

#ifdef __x86_64__
    unsigned long lm_l1, lm_l2;
#endif

    vmx_vmcs_switch_current(v, host_vmcs, svmcs);

    /* generic code switches vcpu into guest mode */

    nest->vmresume_pending = 0;

    /* generic code indicates that we are in vmentry emulation by
     * nh_hostflags.fields.vmentry flag.
     */

#ifdef __x86_64__
    /*
     * EFER handling:
     * hvm_set_efer won''t work if CR0.PG = 1, so we change the value
     * directly to make hvm_long_mode_enabled(v) work in L2.
     * An additional update_paging_modes is also needed is
     * there is 32/64 switch. v->arch.hvm_vcpu.guest_efer doesn''t
     * need to be saved, since its value on vmexit is determined by
     * L1 exit_controls
     */
    lm_l1 = !!hvm_long_mode_enabled(v);
    lm_l2 = !!(__get_vvmcs(nest->vvmcs, VM_ENTRY_CONTROLS) &
                           VM_ENTRY_IA32E_MODE);

    if ( lm_l2 )
        v->arch.hvm_vcpu.guest_efer |= EFER_LMA | EFER_LME;
    else
        v->arch.hvm_vcpu.guest_efer &= ~(EFER_LMA | EFER_LME);
#endif

    load_l2_control(nest);
    load_vvmcs_guest_state(nest);

#ifdef __x86_64__
    if ( lm_l1 != lm_l2 )
    {
        paging_update_paging_modes(v);
    }
#endif

    regs->rip = __get_vvmcs(nest->vvmcs, GUEST_RIP);
    regs->rsp = __get_vvmcs(nest->vvmcs, GUEST_RSP);
    regs->rflags = __get_vvmcs(nest->vvmcs, GUEST_RFLAGS);

    /* TODO: EPT_POINTER */
}


Analogous to this I would take ''virtual_vmexit'' and implement
it
for ''prepare4vmexit''.

Will this work for you ?
Do you still see utterly much VMCS field accesses when you go this way?

> >> Anyway, to me given that nested SVM & VMX is still on the very
> >> beginning of development, I can only say yes to those wrappers
that
> >> have clear understanding to both side.
> >
> > Good to know. See my offer below.
>
> I am not sure if you want to efficiently start with lighweight wrapper
> first (those wrapper with consense and individual solution for those not),
> or you want to keep spinning on the heavy wrapper.  For me, if you go with
> lighweight wrapper like single layer VMX/SVM virtualization does, I can ack
> soon. But for the heavy weight wrapper, it is not necessary to me and will
> block future development of nested VMX, so I can''t ack. Even
current code
> is a lot of hard to understand given that I need to understand the
> semantics of those new (vendor-neutral) fields, however the semantics of
> VMX fields are clear to all VMX developers. Sorry for that, although I know
> Tim is more considerative to the effort you have called it out. I
> appreciate your effort as well, but that reason is not strong enough yet to
> impact nested VMX development.
>
> >> I would rather leave those uncertain wrappers to future, once the
> >> basic shape of nested virtualization is good and stable enough,
i.e.
> >> lightweight wrapper.  We have plenty of performance work ahead
such
> >> as virtual VTd support, enhanced PV driver for nested etc.
Excessive
> >> wrapper is simple a burden to nested VMX developers for those
future
> >> features.
> >>
> >> Qing will post his patch today or tomorrow for your reference if
you
> >> want.
> >
> > Thanks. May I take code from there and add into my patch series ?
>
> Sure.
Thanks.
> But you won;t assume this is the logic we won''t change. 
Of course not.

Christoph


-- 
---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


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

Well, so you decide to cycle here, though it obviously not only hurt Intel but
also AMD''s progress.

Let me ask several questions first:
1: Does this heavy weight wrapper a must for nested virtualization?
2: What is the gain and lose?
3: How single layer virtualization does? Are they abstracting with heavy weight
wrapper or light weight and why? Particularly do they abstract the 1st level VM
exit/entry event or do they invent 3rd name space for VMCS/VMCB field?

The only reason I can see so far is that you called out the "abstract"
in nested virtualization side, that is for some reason we have to appreciate but
not enough for us to swallow the side effect of its impact to future nested VMX
development.


Taking your "car" abstract as example, 
> All cars are different but they all have three pedals and a steering wheel.
Well, automatic cars have 2 pedals only (but I know most German cars are
mechanical car for green fuel reason which does have 3 pedals.
Steering wheel is current implementation, but there are researches inventing
automatic controlled cars that is fully controlled by computer, and I am not
sure if trolley car is a car? If it is, it can work without steering wheel
though it may have today.


Above comments doesn;t mean I against your abstract of "car with 3 pedals
and a steering wheeling" even it may be not accurate enough. But it does
reflect the trade off between innovation and industry standardization. W/o
standaradization, there are many variants coming in to compete in different
area, but once a standardization comes, the innovation is deprioritized, rather
other competition comes such as price. Hardly we can see if we need innovation
more (no standardization) or cost saving more. But at very beginning of a new
product/feature, I think innovation is more important to allow different
variants to compete.

Taking software or nested virtualization as example, defining those wrappers or
abstract to force each vendor to follow does deprioritize innovation, but they
may be easier for followers to understand easily if they doesn''t
understand SVM/VMX itself very well. However, as I said, nested virtualization
is still at its very beginning and we need innovation more to find what is the
best solution especially from performance point of view. Otherwise re-abstract
does require double effort for VMX developers to consider SVM support as well
(and even more than double effort because normally Intel VMX developers
doesn''t know SVM architecture very well and we don''t have test
machine for SVM).

Anyway, the ball is in your side.

Thx, Eddie

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

On Thursday 09 September 2010 03:45:55 Dong, Eddie
wrote:
> Well, so you decide to cycle here, though it obviously not only hurt Intel
> but also AMD''s progress.
>
> Let me ask several questions first:
> 1: Does this heavy weight wrapper a must for nested virtualization?
It''s not a question of a must. It''s a question of design.
> 2: What is the gain and lose?
in what respect ?
> 3: How single layer virtualization does? Are they abstracting with heavy
> weight wrapper or light weight and why?
Maybe you should explain what you mean by ''heavy weight'' and
''light weight''
wrapper.

By software concept, a wrapper is a function that implements one interface by
calling an other interface. The purpose is to provide/emulate yet another API.
Example: In NetBSD, the Linux syscalls are implemented by calling the
native sycalls. This allows to run Linux binaries on NetBSD.

In that sense, the answer to your question is "Neither nor".
It just puts common logic into one place that has been found over time.
> Particularly do they abstract the 1st level VM exit/entry event or do they
> invent 3rd name space for VMCS/VMCB field?
Yes, there is the guest_cr array for use in generic code, for example.

> The only reason I can see so far is that you called out the
"abstract" in
> nested virtualization side, that is for some reason we have to appreciate
> but not enough for us to swallow the side effect of its impact to future
> nested VMX development.
You are worried about that the interface is fixed, right?

Note that this interface is an internal one. In opposition to an external
interface (xen/include/public/*) you do not need to care about backward
compatibility.

Common to an internal and an external interface are:
- care about not breaking what has worked before
- think first before doing a change what for, if it is really needed
   and that it makes really sense to avoid make it go wild

One example for a wild gone interface are the hypercalls
to get/set C- and P-States in Xen. When I want to start implementing
the feature in a Dom0 from scratch, I do not know which ones I should use.

Tim said in
http://lists.xensource.com/archives/html/xen-devel/2010-08/msg01044.html
"I''ve no strong feelings about the details of the interface
between the
common and the arch-specific code, but it seems like with a bit of
flexibility on both sides it could be made suitable for everyone."

Your point about standardization from below comes in here, too.

> Taking your "car" abstract as example,
>
> > All cars are different but they all have three pedals and a steering
> > wheel.
>
> Well, automatic cars have 2 pedals only (but I know most German cars are
> mechanical car for green fuel reason which does have 3 pedals.
> Steering wheel is current implementation, but there are researches
inventing
> automatic controlled cars that is fully controlled by computer, and I am
> not sure if trolley car is a car? If it is, it can work without steering
> wheel though it may have today.
Having 2 or 3 pedals or a steering wheel or not, here by concept
the question is who *uses* the interface.

Switching the gears and the capability to turn left/right is still there, 
because the implementation of the same interface still exists.

The difference is that who can see and use which interface.
Who switches the gears and who turns left/right
- is it the human or a computer ?


You also mentioned research: You know that doing research means to develop
prototypes ?  Prototypes may be but must not be production stable.
The purpose for writing prototypes is figure out how things work and
what problems may come up.

In software engineering, prototypes are something that get thrown away to
implement the production version from scratch where you know the
problems ahead.

Below you say, nested virtualization is at the very beginning. That''s
right
for the code, the idea is old (Goldberg''s paper).

If you want to call the patch series a prototype then that means that
all of our code needs to be thrown away and we both must start from
scratch. Do you really want that?
>
> Above comments doesn;t mean I against your abstract of "car with 3
pedals
> and a steering wheeling" even it may be not accurate enough. But it
does
> reflect the trade off between innovation and industry standardization. W/o
> standaradization, there are many variants coming in to compete in different
> area, but once a standardization comes, the innovation is deprioritized,
> rather other competition comes such as price. Hardly we can see if we need
> innovation more (no standardization) or cost saving more. But at very
> beginning of a new product/feature, I think innovation is more important to
> allow different variants to compete.
I disagree in that standardization and innovation are exclusive on each other.
A community consisting full of software engineers show again and again
that there are synergy effects:

Example: In NetBSD, the access to  a bus are standardized through an interface 
(http://netbsd.gw.com/cgi-bin/man-cgi?bus_space++NetBSD-current).
The drivers have a frontend and a backend. The frontend attaches
the driver onto the pci, isa, whatever bus. The backend contains all the
code using the interface, there''s no code specific to pci, isa, etc.

Now when you come up with a new and innovative bus for the PC,
then you can add support for it by implementing the interface.
Instead of writing/porting all drivers to the new bus you go the other
way: You use the existing drivers to test if your bus implementation
works. And if you find a bug in the driver then the fix is not only good
for the new innovative bus.
> Taking software or nested virtualization as example, defining those
> wrappers or abstract to force each vendor to follow does deprioritize
> innovation, but they may be easier for followers to understand easily if
> they doesn''t understand SVM/VMX itself very well.
I do not say you are wrong here. The sad thing is that it doesn''t
work in reality.

You said in
http://lists.xensource.com/archives/html/xen-devel/2010-08/msg01007.html

"Drawing a picture which doesn''t have a real usage with massive
common code
change is a kind of too much load for us now. Xen hvm_function table is a good 
example. Intel enabled VMX at very beginning of Xen HVM support, and SVM comes 
later on with a lot of code reuse from VMX side. Then the community and both 
side work together to make an API wrapper to reuse common code better and 
accomodate both architecture. I think we have to go similar path to make it 
work first."

I looked through the changesets where xen/arch/x86/hvm/hvm.c
was involved (hg log -v xen/arch/x86/hvm/hvm.c). I did not find a
changeset from Intel where code from SVM and VMX has moved into
hvm.c. So "both side work together" does not apply here.
Factoring code out from VMX was a very hard time for AMD developers
and that time is still not over.

Tim said in
http://lists.xensource.com/archives/html/xen-devel/2010-04/msg00812.html

"I would like to see some reasonable discussion of exactly how much of
the nested virtualization logic can be shared.  With HVM it has turned
out to be quite a lot, but it''s taken years of reshuffling to pull code
out into common HVM routines (and we''re not there yet)."

I do not want to repeat that again with nested virtualization.

Later when the nested virtualization becomes major where
the ''standardization'' matters according to you, I doubt that
Intel developers will take account for it for two reasons:

a) Almost every patch from Intel that touches a file under
xen/include/public/* breaks backward-compatibility.
b) The fact of rare if not missing Intel patches where code from SVM and VMX
moves into hvm.c w/o breaking anything.

That all speaks a lot for pulling as much logic as possible
into common code.
> However, as I said, nested virtualization is still at its very beginning
and
> we need innovation more to find what is the best solution especially from
> performance point of view.
Well, that is the research aspect with prototypes I mentioned above.
> Otherwise re-abstract does require double effort for VMX developers 
> to consider SVM support as well
At least in thinking, but not necessarily in coding. The result is higher code
quality that makes this effort worth. Keir will more often apply patches
w/o asking you to fix/re-work things first.
> (and even more than double effort because normally Intel VMX developers
> doesn''t know SVM architecture very well
Not knowing something does not mean being stupid. There are manuals
to learn from.
> and we don''t have test machine for SVM). 
You surely can afford some AMD machines given the much higher revenue
Intel has over AMD.

Christoph


-- 
---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


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