build break in xl.c in python

build break in xl.c in python

[Kay, Allen M]

I'm getting a build break in python xl code.  Looks like the following changeset did not make the corresponding parameter change from the caller site in python:

-static int do_pci_remove(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev)
+static int do_pci_remove(libxl__gc *gc, uint32_t domid,
+                         libxl_device_pci *pcidev, int force)


Changeset:

31 hours ago: xl: Implement PCI passthrough force removal 
changeset 22229: 1385b15e168f 
tag: tip 
author: Gianni Tedesco <gianni.tedesco@citrix.com> 
date: Wed Oct 06 17:38:15 2010 +0100 
files: tools/libxl/libxl.h tools/libxl/libxl_pci.c tools/libxl/xl_cmdimpl.c  

Configuration of nestedhvm

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 4104 bytes --]

Keir:
       Despite of the negotioatiom of nested virtualization wrapper, here comes with the global configuration parameter and the CR4 layout handling between SVM & VMX. These are wrapper neutral IMO.

Thx, Eddie



===Patch1

Nested virtualization usage model is emerging, however we must guarantee it won't impact the performance of simple virtualization.
This patch add an boot parameter for nested virtualization, which is disabled by default for now.

Signed-off-by: Eddie Dong <eddie.dong@intel.com>


diff -r 1385b15e168f xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/arch/x86/hvm/hvm.c	Fri Oct 08 10:29:38 2010 +0800
@@ -66,6 +66,9 @@ unsigned int opt_hvm_debug_level __read_
 unsigned int opt_hvm_debug_level __read_mostly;
 integer_param("hvm_debug", opt_hvm_debug_level);
 
+unsigned int enable_nested_hvm __read_mostly;
+integer_param("nested_hvm", enable_nested_hvm);
+
 struct hvm_function_table hvm_funcs __read_mostly;
 
 /* I/O permission bitmap is globally shared by all HVM guests. */
diff -r 1385b15e168f xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/include/asm-x86/hvm/hvm.h	Fri Oct 08 10:35:03 2010 +0800
@@ -250,6 +250,9 @@ hvm_set_segment_register(struct vcpu *v,
 #define is_viridian_domain(_d)                                             \
  (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
 
+/* TODO: handle per domain configuration */
+#define is_nestedhvm(_d)     (enable_nested_hvm && is_hvm_domain(_d))
+
 void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
                                    unsigned int *ecx, unsigned int *edx);
 void hvm_migrate_timers(struct vcpu *v);


===Patch2

This patch solves the CR4 bit (VMXE) format difference between AMD & Intel processor.

Signed-off-by: Qing He <qing.he@intel.com>
Signed-off-by: Eddie Dong <eddie.dong@intel.com>

diff -r 1385b15e168f xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/arch/x86/hvm/hvm.c	Fri Oct 08 10:46:55 2010 +0800
@@ -630,6 +630,11 @@ static int hvm_load_cpu_ctxt(struct doma
     struct hvm_hw_cpu ctxt;
     struct segment_register seg;
     struct vcpu_guest_context *vc;
+    unsigned long rsv_bits = HVM_CR4_GUEST_RESERVED_BITS;
+
+    if ( !is_nestedhvm(d) ||
+          boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+        rsv_bits |= X86_CR4_VMXE;
 
     /* Which vcpu is this? */
     vcpuid = hvm_load_instance(h);
@@ -662,7 +667,7 @@ static int hvm_load_cpu_ctxt(struct doma
         return -EINVAL;
     }
 
-    if ( ctxt.cr4 & HVM_CR4_GUEST_RESERVED_BITS )
+    if ( ctxt.cr4 & rsv_bits )
     {
         gdprintk(XENLOG_ERR, "HVM restore: bad CR4 0x%"PRIx64"\n",
                  ctxt.cr4);
@@ -1243,8 +1248,12 @@ int hvm_set_cr4(unsigned long value)
 {
     struct vcpu *v = current;
     unsigned long old_cr;
-
-    if ( value & HVM_CR4_GUEST_RESERVED_BITS )
+    unsigned long rsv_bits = HVM_CR4_GUEST_RESERVED_BITS;
+
+    if ( !is_nestedhvm(v->domain) ||
+         boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+        rsv_bits |= X86_CR4_VMXE;
+    if ( value & rsv_bits )
     {
         HVM_DBG_LOG(DBG_LEVEL_1,
                     "Guest attempts to set reserved bit in CR4: %lx",
diff -r 1385b15e168f xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/include/asm-x86/hvm/hvm.h	Fri Oct 08 10:46:35 2010 +0800
@@ -291,7 +291,8 @@ static inline int hvm_do_pmu_interrupt(s
         X86_CR4_DE  | X86_CR4_PSE | X86_CR4_PAE |       \
         X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE |       \
         X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT |           \
-        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))))
+        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))) |       \
+        X86_CR4_VMXE)
 
 /* These exceptions must always be intercepted. */
 #define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op))

[-- Attachment #2: patch1 --]
[-- Type: application/octet-stream, Size: 1540 bytes --]



Nested virtualization usage model is emerging, however we must guarantee it won't impact the performance of simple virtualization.
This patch add an boot parameter for nested virtualization, which is disabled by default for now.

Signed-off-by: Eddie Dong <eddie.dong@intel.com>


diff -r 1385b15e168f xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/arch/x86/hvm/hvm.c	Fri Oct 08 10:29:38 2010 +0800
@@ -66,6 +66,9 @@ unsigned int opt_hvm_debug_level __read_
 unsigned int opt_hvm_debug_level __read_mostly;
 integer_param("hvm_debug", opt_hvm_debug_level);
 
+unsigned int enable_nested_hvm __read_mostly;
+integer_param("nested_hvm", enable_nested_hvm);
+
 struct hvm_function_table hvm_funcs __read_mostly;
 
 /* I/O permission bitmap is globally shared by all HVM guests. */
diff -r 1385b15e168f xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/include/asm-x86/hvm/hvm.h	Fri Oct 08 10:35:03 2010 +0800
@@ -250,6 +250,9 @@ hvm_set_segment_register(struct vcpu *v,
 #define is_viridian_domain(_d)                                             \
  (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
 
+/* TODO: handle per domain configuration */
+#define is_nestedhvm(_d)     (enable_nested_hvm && is_hvm_domain(_d))
+
 void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
                                    unsigned int *ecx, unsigned int *edx);
 void hvm_migrate_timers(struct vcpu *v);

[-- Attachment #3: patch2 --]
[-- Type: application/octet-stream, Size: 2231 bytes --]


This patch solves the CR4 bit (VMXE) format difference between AMD & Intel processor.

Signed-off-by: Qing He <qing.he@intel.com>
Signed-off-by: Eddie Dong <eddie.dong@intel.com>

diff -r 1385b15e168f xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/arch/x86/hvm/hvm.c	Fri Oct 08 10:46:55 2010 +0800
@@ -630,6 +630,11 @@ static int hvm_load_cpu_ctxt(struct doma
     struct hvm_hw_cpu ctxt;
     struct segment_register seg;
     struct vcpu_guest_context *vc;
+    unsigned long rsv_bits = HVM_CR4_GUEST_RESERVED_BITS;
+
+    if ( !is_nestedhvm(d) ||
+          boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+        rsv_bits |= X86_CR4_VMXE;
 
     /* Which vcpu is this? */
     vcpuid = hvm_load_instance(h);
@@ -662,7 +667,7 @@ static int hvm_load_cpu_ctxt(struct doma
         return -EINVAL;
     }
 
-    if ( ctxt.cr4 & HVM_CR4_GUEST_RESERVED_BITS )
+    if ( ctxt.cr4 & rsv_bits )
     {
         gdprintk(XENLOG_ERR, "HVM restore: bad CR4 0x%"PRIx64"\n",
                  ctxt.cr4);
@@ -1243,8 +1248,12 @@ int hvm_set_cr4(unsigned long value)
 {
     struct vcpu *v = current;
     unsigned long old_cr;
-
-    if ( value & HVM_CR4_GUEST_RESERVED_BITS )
+    unsigned long rsv_bits = HVM_CR4_GUEST_RESERVED_BITS;
+
+    if ( !is_nestedhvm(v->domain) ||
+         boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
+        rsv_bits |= X86_CR4_VMXE;
+    if ( value & rsv_bits )
     {
         HVM_DBG_LOG(DBG_LEVEL_1,
                     "Guest attempts to set reserved bit in CR4: %lx",
diff -r 1385b15e168f xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h	Wed Oct 06 17:38:15 2010 +0100
+++ b/xen/include/asm-x86/hvm/hvm.h	Fri Oct 08 10:46:35 2010 +0800
@@ -291,7 +291,8 @@ static inline int hvm_do_pmu_interrupt(s
         X86_CR4_DE  | X86_CR4_PSE | X86_CR4_PAE |       \
         X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE |       \
         X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT |           \
-        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))))
+        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))) |       \
+        X86_CR4_VMXE)
 
 /* These exceptions must always be intercepted. */
 #define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op))

[-- Attachment #4: Type: text/plain, Size: 138 bytes --]

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

Re: Configuration of nestedhvm

[Keir Fraser]

On 08/10/2010 05:34, "Dong, Eddie" <eddie.dong@intel.com> wrote:

> Nested virtualization usage model is emerging, however we must guarantee it
> won't impact the performance of simple virtualization.
> This patch add an boot parameter for nested virtualization, which is disabled
> by default for now.

What's the point when a per-domain config option is going to be implemented?
You can then simply not configure nestedhvm for a domain you want to test
without that capability? I suppose it makes your second patch make a bit
more sense than it would in total isolation.

I think patch#2 probably makes sense, but it should wait for the patch that
actually implements the per-domain config option, and properly implements
is_nestedhvm(), before going in.

 -- Keir

RE: Configuration of nestedhvm

[Dong, Eddie]

Keir Fraser wrote:
> On 08/10/2010 05:34, "Dong, Eddie" <eddie.dong@intel.com> wrote:
> 
>> Nested virtualization usage model is emerging, however we must
>> guarantee it won't impact the performance of simple virtualization.
>> This patch add an boot parameter for nested virtualization, which is
>> disabled by default for now.
> 
> What's the point when a per-domain config option is going to be
> implemented? You can then simply not configure nestedhvm for a domain
> you want to test without that capability? I suppose it makes your
> second patch make a bit more sense than it would in total isolation.

I want double-lock (AND) like other components such as IOMMU. 
If the global switch is off, even per domain configuration is turned on, the final effect is "OFF". 

The point here is to avoid manual mistake when the nested code is built in as formal release but targeting for pilot. Relying on HVM guest configuration only may cause the host crash or performance impact if the code has a bug and a guest enables nested virtualization feature.  

This switch is mainly for developer only at least for now.

> 
> I think patch#2 probably makes sense, but it should wait for the
> patch that actually implements the per-domain config option, and
> properly implements is_nestedhvm(), before going in.

Yes, I am waiting for the take of that patch from Chris as well, but for some reason it is not in yet :(

Either patch is taken first is fine. I can do the simple rebase.

> 
>  -- Keir

Thx, Eddie

Re: Configuration of nestedhvm

[Keir Fraser]

On 08/10/2010 08:56, "Dong, Eddie" <eddie.dong@intel.com> wrote:

>> I think patch#2 probably makes sense, but it should wait for the
>> patch that actually implements the per-domain config option, and
>> properly implements is_nestedhvm(), before going in.
> 
> Yes, I am waiting for the take of that patch from Chris as well, but for some
> reason it is not in yet :(
> 
> Either patch is taken first is fine. I can do the simple rebase.

The patch isn't in my queue. I assume it is largely tools-side and waiting
for a tools maintainer to do something with it, and/or Tim to Ack it.

 -- Keir

Re: Configuration of nestedhvm

[Keir Fraser]

On 08/10/2010 08:56, "Dong, Eddie" <eddie.dong@intel.com> wrote:

>> What's the point when a per-domain config option is going to be
>> implemented? You can then simply not configure nestedhvm for a domain
>> you want to test without that capability? I suppose it makes your
>> second patch make a bit more sense than it would in total isolation.
> 
> I want double-lock (AND) like other components such as IOMMU.
> If the global switch is off, even per domain configuration is turned on, the
> final effect is "OFF".
> 
> The point here is to avoid manual mistake when the nested code is built in as
> formal release but targeting for pilot. Relying on HVM guest configuration
> only may cause the host crash or performance impact if the code has a bug and
> a guest enables nested virtualization feature.
> 
> This switch is mainly for developer only at least for now.

Well, at least it should only be disallowing toolstack to set the per-domain
config option. Then it won't need to be accessed on every use of
is_nestedhvm(). So again it depends on that, mainly tool-side, patch.

 -- Keir

RE: Configuration of nestedhvm

[Dong, Eddie]

Keir Fraser wrote:
> On 08/10/2010 08:56, "Dong, Eddie" <eddie.dong@intel.com> wrote:
> 
>>> What's the point when a per-domain config option is going to be
>>> implemented? You can then simply not configure nestedhvm for a
>>> domain you want to test without that capability? I suppose it makes
>>> your second patch make a bit more sense than it would in total
>>> isolation. 
>> 
>> I want double-lock (AND) like other components such as IOMMU.
>> If the global switch is off, even per domain configuration is turned
>> on, the final effect is "OFF". 
>> 
>> The point here is to avoid manual mistake when the nested code is
>> built in as formal release but targeting for pilot. Relying on HVM
>> guest configuration only may cause the host crash or performance
>> impact if the code has a bug and a guest enables nested
>> virtualization feature. 
>> 
>> This switch is mainly for developer only at least for now.
> 
> Well, at least it should only be disallowing toolstack to set the
> per-domain config option. Then it won't need to be accessed on every

The tools side patch will hypercall to try to set, but it is hypervisor's decision to allow the setting or not.
The userland CPUID emulation will rely on the VMM returned setting.

> use of is_nestedhvm(). So again it depends on that, mainly tool-side,
> patch. 

It is chicken and egg then :) emulation of per domain setting hypercall relies on the global setting. is_nestedhvm relies on the per domain setting :)

But it is fine too, as if you want the global control, and Chris may merge the patch into his if he needs to repost..

> 
>  -- Keir

So can Ian have a look at the previous Chris patch again? What is the reason to block that patch?

Thx, Eddie

Re: Configuration of nestedhvm

[Keir Fraser]

On 08/10/2010 09:47, "Dong, Eddie" <eddie.dong@intel.com> wrote:

>> use of is_nestedhvm(). So again it depends on that, mainly tool-side,
>> patch. 
> 
> It is chicken and egg then :) emulation of per domain setting hypercall relies
> on the global setting. is_nestedhvm relies on the per domain setting :)
> 
> But it is fine too, as if you want the global control, and Chris may merge the
> patch into his if he needs to repost..
> 
>> 
>>  -- Keir
> 
> So can Ian have a look at the previous Chris patch again? What is the reason
> to block that patch?

Best ask him. Cc'ing him on this email.

 -- Keir

Re: Configuration of nestedhvm

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 4683 bytes --]


Eddie,

you can simplify the second patch with attached patch merged
and is correct for AMD side.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

Christoph


On Friday 08 October 2010 06:34:14 Dong, Eddie wrote:
> Keir:
>        Despite of the negotioatiom of nested virtualization wrapper, here
> comes with the global configuration parameter and the CR4 layout handling
> between SVM & VMX. These are wrapper neutral IMO.
>
> Thx, Eddie
>
>
>
> ===Patch1
>
> Nested virtualization usage model is emerging, however we must guarantee it
> won't impact the performance of simple virtualization. This patch add an
> boot parameter for nested virtualization, which is disabled by default for
> now.
>
> Signed-off-by: Eddie Dong <eddie.dong@intel.com>
>
>
> diff -r 1385b15e168f xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c	Wed Oct 06 17:38:15 2010 +0100
> +++ b/xen/arch/x86/hvm/hvm.c	Fri Oct 08 10:29:38 2010 +0800
> @@ -66,6 +66,9 @@ unsigned int opt_hvm_debug_level __read_
>  unsigned int opt_hvm_debug_level __read_mostly;
>  integer_param("hvm_debug", opt_hvm_debug_level);
>
> +unsigned int enable_nested_hvm __read_mostly;
> +integer_param("nested_hvm", enable_nested_hvm);
> +
>  struct hvm_function_table hvm_funcs __read_mostly;
>
>  /* I/O permission bitmap is globally shared by all HVM guests. */
> diff -r 1385b15e168f xen/include/asm-x86/hvm/hvm.h
> --- a/xen/include/asm-x86/hvm/hvm.h	Wed Oct 06 17:38:15 2010 +0100
> +++ b/xen/include/asm-x86/hvm/hvm.h	Fri Oct 08 10:35:03 2010 +0800
> @@ -250,6 +250,9 @@ hvm_set_segment_register(struct vcpu *v,
>  #define is_viridian_domain(_d)                                            
> \ (is_hvm_domain(_d) && ((_d)->arch.hvm_domain.params[HVM_PARAM_VIRIDIAN]))
>
> +/* TODO: handle per domain configuration */
> +#define is_nestedhvm(_d)     (enable_nested_hvm && is_hvm_domain(_d))
> +
>  void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>                                     unsigned int *ecx, unsigned int *edx);
>  void hvm_migrate_timers(struct vcpu *v);
>
>
> ===Patch2
>
> This patch solves the CR4 bit (VMXE) format difference between AMD & Intel
> processor.
>
> Signed-off-by: Qing He <qing.he@intel.com>
> Signed-off-by: Eddie Dong <eddie.dong@intel.com>
>
> diff -r 1385b15e168f xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c	Wed Oct 06 17:38:15 2010 +0100
> +++ b/xen/arch/x86/hvm/hvm.c	Fri Oct 08 10:46:55 2010 +0800
> @@ -630,6 +630,11 @@ static int hvm_load_cpu_ctxt(struct doma
>      struct hvm_hw_cpu ctxt;
>      struct segment_register seg;
>      struct vcpu_guest_context *vc;
> +    unsigned long rsv_bits = HVM_CR4_GUEST_RESERVED_BITS;
> +
> +    if ( !is_nestedhvm(d) ||
> +          boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
> +        rsv_bits |= X86_CR4_VMXE;
>
>      /* Which vcpu is this? */
>      vcpuid = hvm_load_instance(h);
> @@ -662,7 +667,7 @@ static int hvm_load_cpu_ctxt(struct doma
>          return -EINVAL;
>      }
>
> -    if ( ctxt.cr4 & HVM_CR4_GUEST_RESERVED_BITS )
> +    if ( ctxt.cr4 & rsv_bits )
>      {
>          gdprintk(XENLOG_ERR, "HVM restore: bad CR4 0x%"PRIx64"\n",
>                   ctxt.cr4);
> @@ -1243,8 +1248,12 @@ int hvm_set_cr4(unsigned long value)
>  {
>      struct vcpu *v = current;
>      unsigned long old_cr;
> -
> -    if ( value & HVM_CR4_GUEST_RESERVED_BITS )
> +    unsigned long rsv_bits = HVM_CR4_GUEST_RESERVED_BITS;
> +
> +    if ( !is_nestedhvm(v->domain) ||
> +         boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
> +        rsv_bits |= X86_CR4_VMXE;
> +    if ( value & rsv_bits )
>      {
>          HVM_DBG_LOG(DBG_LEVEL_1,
>                      "Guest attempts to set reserved bit in CR4: %lx",
> diff -r 1385b15e168f xen/include/asm-x86/hvm/hvm.h
> --- a/xen/include/asm-x86/hvm/hvm.h	Wed Oct 06 17:38:15 2010 +0100
> +++ b/xen/include/asm-x86/hvm/hvm.h	Fri Oct 08 10:46:35 2010 +0800
> @@ -291,7 +291,8 @@ static inline int hvm_do_pmu_interrupt(s
>          X86_CR4_DE  | X86_CR4_PSE | X86_CR4_PAE |       \
>          X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE |       \
>          X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT |           \
> -        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))))
> +        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))) |       \
> +        X86_CR4_VMXE)
>
>  /* These exceptions must always be intercepted. */
>  #define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U <<
> TRAP_invalid_op))



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

[-- Attachment #2: xen_vmx_enable.diff --]
[-- Type: text/x-diff, Size: 988 bytes --]

diff -r b1ace8d2143c -r 325392ae335f xen/include/asm-x86/cpufeature.h
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -201,6 +201,8 @@
 
 #define cpu_has_svm		boot_cpu_has(X86_FEATURE_SVME)
 
+#define cpu_has_vmx		boot_cpu_has(X86_FEATURE_VMXE)
+
 #endif /* __ASM_I386_CPUFEATURE_H */
 
 /* 
diff -r b1ace8d2143c -r 325392ae335f xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -315,7 +315,8 @@ static inline int hvm_do_pmu_interrupt(s
         X86_CR4_DE  | X86_CR4_PSE | X86_CR4_PAE |       \
         X86_CR4_MCE | X86_CR4_PGE | X86_CR4_PCE |       \
         X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT |           \
-        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0))))
+        (cpu_has_xsave ? X86_CR4_OSXSAVE : 0)   |       \
+        (cpu_has_vmx ? X86_CR4_VMXE : 0))))
 
 /* These exceptions must always be intercepted. */
 #define HVM_TRAP_MASK ((1U << TRAP_machine_check) | (1U << TRAP_invalid_op))

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

RE: Configuration of nestedhvm

[Dong, Eddie]

Christoph Egger wrote:
> Eddie,
> 
> you can simplify the second patch with attached patch merged
> and is correct for AMD side.
> 

cpu_has_vmx is not enough. We need to check the configuration (global and per domain), not HW feature availability.

Eddie

Re: Configuration of nestedhvm

[Christoph Egger]

On Friday 08 October 2010 10:54:58 Dong, Eddie wrote:
> Christoph Egger wrote:
> > Eddie,
> >
> > you can simplify the second patch with attached patch merged
> > and is correct for AMD side.
>
> cpu_has_vmx is not enough. We need to check the configuration (global and
> per domain), not HW feature availability.

Can you *merge* that patch into yours, please?

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

RE: Configuration of nestedhvm

[Dong, Eddie]

Christoph Egger wrote:
> On Friday 08 October 2010 10:54:58 Dong, Eddie wrote:
>> Christoph Egger wrote:
>>> Eddie,
>>> 
>>> you can simplify the second patch with attached patch merged
>>> and is correct for AMD side.
>> 
>> cpu_has_vmx is not enough. We need to check the configuration
>> (global and per domain), not HW feature availability.
> 
> Can you *merge* that patch into yours, please?
> 
What do you mean? Do you want me to re-post your tools patch w/ this one merged?

If you are talking about the attached part of the patch, my patch 2 is exactly the replacement.

Thx, Eddie

Re: Configuration of nestedhvm

[Christoph Egger]

On Friday 08 October 2010 15:33:53 Dong, Eddie wrote:
> Christoph Egger wrote:
> > On Friday 08 October 2010 10:54:58 Dong, Eddie wrote:
> >> Christoph Egger wrote:
> >>> Eddie,
> >>>
> >>> you can simplify the second patch with attached patch merged
> >>> and is correct for AMD side.
> >>
> >> cpu_has_vmx is not enough. We need to check the configuration
> >> (global and per domain), not HW feature availability.
> >
> > Can you *merge* that patch into yours, please?
>
> What do you mean? Do you want me to re-post your tools patch w/ this one
> merged?
>
> If you are talking about the attached part of the patch, my patch 2 is
> exactly the replacement.

Oh, sorry. I thought, it was to allow the guest to set the VMX bit in CR4
- analogous to my 'efer' patch which allows the guest to set the SVME bit in 
EFER.

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

add missing VMCS definition

[Dong, Eddie]

[-- Attachment #1: Type: text/plain, Size: 2038 bytes --]

This patch add back some missing VMCS defintions as preparation for nested VMX.

Signed-off-by: Qing He <qing.he@intel.com>
Signed-off-by: Eddie Dong <eddie.dong@intel.com>


diff -r e7440ba79e38 xen/include/asm-x86/hvm/vmx/vmcs.h
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h	Wed Sep 29 19:48:47 2010 +0800
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h	Wed Sep 29 19:49:23 2010 +0800
@@ -163,18 +163,23 @@
 #define PIN_BASED_EXT_INTR_MASK         0x00000001
 #define PIN_BASED_NMI_EXITING           0x00000008
 #define PIN_BASED_VIRTUAL_NMIS          0x00000020
+#define PIN_BASED_PREEMPT_TIMER         0x00000040
 extern u32 vmx_pin_based_exec_control;
 
 #define VM_EXIT_IA32E_MODE              0x00000200
 #define VM_EXIT_ACK_INTR_ON_EXIT        0x00008000
 #define VM_EXIT_SAVE_GUEST_PAT          0x00040000
 #define VM_EXIT_LOAD_HOST_PAT           0x00080000
+#define VM_EXIT_SAVE_GUEST_EFER         0x00100000
+#define VM_EXIT_LOAD_HOST_EFER          0x00200000
+#define VM_EXIT_SAVE_PREEMPT_TIMER      0x00400000
 extern u32 vmx_vmexit_control;
 
 #define VM_ENTRY_IA32E_MODE             0x00000200
 #define VM_ENTRY_SMM                    0x00000400
 #define VM_ENTRY_DEACT_DUAL_MONITOR     0x00000800
 #define VM_ENTRY_LOAD_GUEST_PAT         0x00004000
+#define VM_ENTRY_LOAD_GUEST_EFER        0x00008000
 extern u32 vmx_vmentry_control;
 
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
diff -r 70956e25dc07 xen/include/asm-x86/msr-index.h
--- a/xen/include/asm-x86/msr-index.h	Fri Oct 08 13:20:19 2010 +0800
+++ b/xen/include/asm-x86/msr-index.h	Fri Oct 08 13:21:27 2010 +0800
@@ -172,6 +172,7 @@
 #define MSR_IA32_VMX_CR0_FIXED1                 0x487
 #define MSR_IA32_VMX_CR4_FIXED0                 0x488
 #define MSR_IA32_VMX_CR4_FIXED1                 0x489
+#define MSR_IA32_VMX_VMCS_ENUM                  0x48a
 #define MSR_IA32_VMX_PROCBASED_CTLS2            0x48b
 #define MSR_IA32_VMX_EPT_VPID_CAP               0x48c
 #define MSR_IA32_VMX_TRUE_PINBASED_CTLS         0x48d

[-- Attachment #2: head.patch --]
[-- Type: application/octet-stream, Size: 1970 bytes --]

This patch add back some missing VMCS defintions.

Signed-off-by: Qing He <qing.he@intel.com>
Signed-off-by: Eddie Dong <eddie.dong@intel.com>


diff -r e7440ba79e38 xen/include/asm-x86/hvm/vmx/vmcs.h
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h	Wed Sep 29 19:48:47 2010 +0800
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h	Wed Sep 29 19:49:23 2010 +0800
@@ -163,18 +163,23 @@
 #define PIN_BASED_EXT_INTR_MASK         0x00000001
 #define PIN_BASED_NMI_EXITING           0x00000008
 #define PIN_BASED_VIRTUAL_NMIS          0x00000020
+#define PIN_BASED_PREEMPT_TIMER         0x00000040
 extern u32 vmx_pin_based_exec_control;
 
 #define VM_EXIT_IA32E_MODE              0x00000200
 #define VM_EXIT_ACK_INTR_ON_EXIT        0x00008000
 #define VM_EXIT_SAVE_GUEST_PAT          0x00040000
 #define VM_EXIT_LOAD_HOST_PAT           0x00080000
+#define VM_EXIT_SAVE_GUEST_EFER         0x00100000
+#define VM_EXIT_LOAD_HOST_EFER          0x00200000
+#define VM_EXIT_SAVE_PREEMPT_TIMER      0x00400000
 extern u32 vmx_vmexit_control;
 
 #define VM_ENTRY_IA32E_MODE             0x00000200
 #define VM_ENTRY_SMM                    0x00000400
 #define VM_ENTRY_DEACT_DUAL_MONITOR     0x00000800
 #define VM_ENTRY_LOAD_GUEST_PAT         0x00004000
+#define VM_ENTRY_LOAD_GUEST_EFER        0x00008000
 extern u32 vmx_vmentry_control;
 
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
diff -r 70956e25dc07 xen/include/asm-x86/msr-index.h
--- a/xen/include/asm-x86/msr-index.h	Fri Oct 08 13:20:19 2010 +0800
+++ b/xen/include/asm-x86/msr-index.h	Fri Oct 08 13:21:27 2010 +0800
@@ -172,6 +172,7 @@
 #define MSR_IA32_VMX_CR0_FIXED1                 0x487
 #define MSR_IA32_VMX_CR4_FIXED0                 0x488
 #define MSR_IA32_VMX_CR4_FIXED1                 0x489
+#define MSR_IA32_VMX_VMCS_ENUM                  0x48a
 #define MSR_IA32_VMX_PROCBASED_CTLS2            0x48b
 #define MSR_IA32_VMX_EPT_VPID_CAP               0x48c
 #define MSR_IA32_VMX_TRUE_PINBASED_CTLS         0x48d

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

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

Re: build break in xl.c in python

[Gianni Tedesco]

On Fri, 2010-10-08 at 01:28 +0100, Kay, Allen M wrote:
> I'm getting a build break in python xl code.  Looks like the following changeset did not make the corresponding parameter change from the caller site in python:
> 
> -static int do_pci_remove(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev)
> +static int do_pci_remove(libxl__gc *gc, uint32_t domid,
> +                         libxl_device_pci *pcidev, int force)
> 
> 
> Changeset:
> 
> 31 hours ago: xl: Implement PCI passthrough force removal 
> changeset 22229: 1385b15e168f 
> tag: tip 
> author: Gianni Tedesco <gianni.tedesco@citrix.com> 
> date: Wed Oct 06 17:38:15 2010 +0100 
> files: tools/libxl/libxl.h tools/libxl/libxl_pci.c tools/libxl/xl_cmdimpl.c  

You are right, I should have realised! It also breaks the ocaml binding.

Vincent, I have patched this so that it builds but perhaps you want to
expose the new PCI force removal in the ocaml binding? I just set
'force' parameter to always be zero for now to keep it building and
running same as before.

---
xl: Fix build in python binding since API change in 22229:1385b15e168f

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 1385b15e168f tools/ocaml/libs/xl/xl_stubs.c
--- a/tools/ocaml/libs/xl/xl_stubs.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/tools/ocaml/libs/xl/xl_stubs.c	Fri Oct 08 09:25:22 2010 +0100
@@ -638,7 +638,7 @@ value stub_xl_pci_remove(value info, val
 	device_pci_val(&gc, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_pci_remove(&ctx, Int_val(domid), &c_info);
+	ret = libxl_device_pci_remove(&ctx, Int_val(domid), &c_info, 0);
 	if (ret != 0)
 		failwith_xl("pci_remove", &lg);
 	FREE_CTX();
diff -r 1385b15e168f tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/tools/python/xen/lowlevel/xl/xl.c	Fri Oct 08 09:25:22 2010 +0100
@@ -441,15 +441,16 @@ static PyObject *pyxl_pci_del(XlObject *
 {
     Py_device_pci *pci;
     PyObject *obj;
-    int domid;
-    if ( !PyArg_ParseTuple(args, "iO", &domid, &obj) )
+    int domid, force = 0;
+
+    if ( !PyArg_ParseTuple(args, "iO|i", &domid, &obj) )
         return NULL;
     if ( !Pydevice_pci_Check(obj) ) {
         PyErr_SetString(PyExc_TypeError, "Xxpected xl.device_pci");
         return NULL;
     }
     pci = (Py_device_pci *)obj;
-    if ( libxl_device_pci_remove(&self->ctx, domid, &pci->obj) ) {
+    if ( libxl_device_pci_remove(&self->ctx, domid, &pci->obj, force) ) {
         PyErr_SetString(xl_error_obj, "cannot remove pci device");
         return NULL;
     }

Re: Re: build break in xl.c in python

[Gianni Tedesco]

On Fri, 2010-10-08 at 09:28 +0100, Gianni Tedesco wrote:
> On Fri, 2010-10-08 at 01:28 +0100, Kay, Allen M wrote:
> > I'm getting a build break in python xl code.  Looks like the following changeset did not make the corresponding parameter change from the caller site in python:
> > 
> > -static int do_pci_remove(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev)
> > +static int do_pci_remove(libxl__gc *gc, uint32_t domid,
> > +                         libxl_device_pci *pcidev, int force)
> > 
> > 
> > Changeset:
> > 
> > 31 hours ago: xl: Implement PCI passthrough force removal 
> > changeset 22229: 1385b15e168f 
> > tag: tip 
> > author: Gianni Tedesco <gianni.tedesco@citrix.com> 
> > date: Wed Oct 06 17:38:15 2010 +0100 
> > files: tools/libxl/libxl.h tools/libxl/libxl_pci.c tools/libxl/xl_cmdimpl.c  
> 
> You are right, I should have realised! It also breaks the ocaml binding.
> 
> Vincent, I have patched this so that it builds but perhaps you want to
> expose the new PCI force removal in the ocaml binding? I just set
> 'force' parameter to always be zero for now to keep it building and
> running same as before.

Actually, ignore the deliberate mistake in python part of that patch :)

Correct patch is below:

---
xl: Fix build in python binding since API change in 22229:1385b15e168f

Signed-off-by: Gianni Tedesco <gianni.tedesco@citrix.com>

diff -r 1385b15e168f tools/ocaml/libs/xl/xl_stubs.c
--- a/tools/ocaml/libs/xl/xl_stubs.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/tools/ocaml/libs/xl/xl_stubs.c	Fri Oct 08 09:33:27 2010 +0100
@@ -638,7 +638,7 @@ value stub_xl_pci_remove(value info, val
 	device_pci_val(&gc, &c_info, info);
 
 	INIT_CTX();
-	ret = libxl_device_pci_remove(&ctx, Int_val(domid), &c_info);
+	ret = libxl_device_pci_remove(&ctx, Int_val(domid), &c_info, 0);
 	if (ret != 0)
 		failwith_xl("pci_remove", &lg);
 	FREE_CTX();
diff -r 1385b15e168f tools/python/xen/lowlevel/xl/xl.c
--- a/tools/python/xen/lowlevel/xl/xl.c	Wed Oct 06 17:38:15 2010 +0100
+++ b/tools/python/xen/lowlevel/xl/xl.c	Fri Oct 08 09:33:27 2010 +0100
@@ -441,15 +441,16 @@ static PyObject *pyxl_pci_del(XlObject *
 {
     Py_device_pci *pci;
     PyObject *obj;
-    int domid;
-    if ( !PyArg_ParseTuple(args, "iO", &domid, &obj) )
+    int domid, force = 0;
+
+    if ( !PyArg_ParseTuple(args, "iO|i", &domid, &obj, &force) )
         return NULL;
     if ( !Pydevice_pci_Check(obj) ) {
         PyErr_SetString(PyExc_TypeError, "Xxpected xl.device_pci");
         return NULL;
     }
     pci = (Py_device_pci *)obj;
-    if ( libxl_device_pci_remove(&self->ctx, domid, &pci->obj) ) {
+    if ( libxl_device_pci_remove(&self->ctx, domid, &pci->obj, force) ) {
         PyErr_SetString(xl_error_obj, "cannot remove pci device");
         return NULL;
     }

Re: Re: build break in xl.c in python

[Stefano Stabellini]

On Fri, 8 Oct 2010, Gianni Tedesco wrote:
> > You are right, I should have realised! It also breaks the ocaml binding.
> > 

yeah, I should have realized as well...

> > Vincent, I have patched this so that it builds but perhaps you want to
> > expose the new PCI force removal in the ocaml binding? I just set
> > 'force' parameter to always be zero for now to keep it building and
> > running same as before.
> 
> Actually, ignore the deliberate mistake in python part of that patch :)
> 
> Correct patch is below:
> 

applied