[PATCH 05/13] Nested Virtualization: CRn & paged real mode

[PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Christoph Egger]

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


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

-- 
---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_nh05_cr.diff --]
[-- Type: text/x-diff, Size: 1305 bytes --]

# HG changeset patch
# User cegger
# Date 1283345878 -7200
Allow paged real mode during vmrun emulation.
Emulate cr0 and cr4 when guest does not intercept them.

diff -r 0199b689a2d0 -r e0eae5b67977 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -54,6 +54,7 @@
 #include <asm/hvm/support.h>
 #include <asm/hvm/cacheattr.h>
 #include <asm/hvm/trace.h>
+#include <asm/hvm/nestedhvm.h>
 #include <asm/mtrr.h>
 #include <asm/apic.h>
 #include <public/sched.h>
@@ -1109,9 +1110,13 @@ int hvm_set_cr0(unsigned long value)
     /* ET is reserved and should be always be 1. */
     value |= X86_CR0_ET;
 
-    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
+    if ( !nestedhvm_vmentry_emulate(v) &&
+         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
         goto gpf;
 
+    if ( nestedhvm_vcpu_in_guestmode(v) )
+        value &= ~(X86_CR0_CD | X86_CR0_NW);
+
     if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
     {
         if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
@@ -1163,7 +1168,7 @@ int hvm_set_cr0(unsigned long value)
         }
     }
 
-    if ( has_arch_mmios(v->domain) )
+    if ( !nestedhvm_vmentry_emulate(v) && has_arch_mmios(v->domain) )
     {
         if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
         {

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

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

Re: [PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Tim Deegan]

Hi, 

> diff -r 0199b689a2d0 -r e0eae5b67977 xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -54,6 +54,7 @@
>  #include <asm/hvm/support.h>
>  #include <asm/hvm/cacheattr.h>
>  #include <asm/hvm/trace.h>
> +#include <asm/hvm/nestedhvm.h>
>  #include <asm/mtrr.h>
>  #include <asm/apic.h>
>  #include <public/sched.h>
> @@ -1109,9 +1110,13 @@ int hvm_set_cr0(unsigned long value)
>      /* ET is reserved and should be always be 1. */
>      value |= X86_CR0_ET;
>  
> -    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> +    if ( !nestedhvm_vmentry_emulate(v) &&
> +         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
>          goto gpf;

The change above makes perfect sense: nested SVM guests should be
allowed to enter paged real mode. 

But I don't understand either of the changes below.  Can you explain why
the cache control bits get special treatment?

Tim.

> +    if ( nestedhvm_vcpu_in_guestmode(v) )
> +        value &= ~(X86_CR0_CD | X86_CR0_NW);
> +
>      if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
>      {
>          if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
> @@ -1163,7 +1168,7 @@ int hvm_set_cr0(unsigned long value)
>          }
>      }
>  
> -    if ( has_arch_mmios(v->domain) )
> +    if ( !nestedhvm_vmentry_emulate(v) && has_arch_mmios(v->domain) )
>      {
>          if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
>          {


-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Christoph Egger]

On Wednesday 08 September 2010 17:11:33 Tim Deegan wrote:
> Hi,
>
> > diff -r 0199b689a2d0 -r e0eae5b67977 xen/arch/x86/hvm/hvm.c
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -54,6 +54,7 @@
> >  #include <asm/hvm/support.h>
> >  #include <asm/hvm/cacheattr.h>
> >  #include <asm/hvm/trace.h>
> > +#include <asm/hvm/nestedhvm.h>
> >  #include <asm/mtrr.h>
> >  #include <asm/apic.h>
> >  #include <public/sched.h>
> > @@ -1109,9 +1110,13 @@ int hvm_set_cr0(unsigned long value)
> >      /* ET is reserved and should be always be 1. */
> >      value |= X86_CR0_ET;
> >
> > -    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> > +    if ( !nestedhvm_vmentry_emulate(v) &&
> > +         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> >          goto gpf;
>
> The change above makes perfect sense: nested SVM guests should be
> allowed to enter paged real mode.
>
> But I don't understand either of the changes below.  Can you explain why
> the cache control bits get special treatment?

Your question confuses me related to this mail:
http://lists.xensource.com/archives/html/xen-devel/2010-08/msg00432.html

Please explain what you want to know.

Christoph


> Tim.
>
> > +    if ( nestedhvm_vcpu_in_guestmode(v) )
> > +        value &= ~(X86_CR0_CD | X86_CR0_NW);
> > +
> >      if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
> >      {
> >          if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
> > @@ -1163,7 +1168,7 @@ int hvm_set_cr0(unsigned long value)
> >          }
> >      }
> >
> > -    if ( has_arch_mmios(v->domain) )
> > +    if ( !nestedhvm_vmentry_emulate(v) && has_arch_mmios(v->domain) )
> >      {
> >          if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
> >          {



-- 
---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: [PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Tim Deegan]

At 16:42 +0100 on 08 Sep (1283964177), Christoph Egger wrote:
> On Wednesday 08 September 2010 17:11:33 Tim Deegan wrote:
> > Hi,
> >
> > > diff -r 0199b689a2d0 -r e0eae5b67977 xen/arch/x86/hvm/hvm.c
> > > --- a/xen/arch/x86/hvm/hvm.c
> > > +++ b/xen/arch/x86/hvm/hvm.c
> > > @@ -54,6 +54,7 @@
> > >  #include <asm/hvm/support.h>
> > >  #include <asm/hvm/cacheattr.h>
> > >  #include <asm/hvm/trace.h>
> > > +#include <asm/hvm/nestedhvm.h>
> > >  #include <asm/mtrr.h>
> > >  #include <asm/apic.h>
> > >  #include <public/sched.h>
> > > @@ -1109,9 +1110,13 @@ int hvm_set_cr0(unsigned long value)
> > >      /* ET is reserved and should be always be 1. */
> > >      value |= X86_CR0_ET;
> > >
> > > -    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> > > +    if ( !nestedhvm_vmentry_emulate(v) &&
> > > +         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> > >          goto gpf;
> >
> > The change above makes perfect sense: nested SVM guests should be
> > allowed to enter paged real mode.
> >
> > But I don't understand either of the changes below.  Can you explain why
> > the cache control bits get special treatment?
> 
> Your question confuses me related to this mail:
> http://lists.xensource.com/archives/html/xen-devel/2010-08/msg00432.html
> 
> Please explain what you want to know.

Sorry, I misread the patch last time. 

| I understand the paged-real-mode case, but why do you also allow CR0.CD
| and CR0.NW for nested VMs?

because the patch seemed to do two things: 
- allow nested VMs to ask for paged real mode; and
- honour CR0.CD and CR0.NW for nested VMs

and I didn't understand the second one.  I see now that it actually
explicitly _doesn't_ honour CR0.CD and CR0.NW for nested VMs, which
makes more sense.  Sorry for the confusing reply.

I don't think that needs to happen -- it's up to the L1 VMM to decide
whether the L2 VMM should be allowed to set cache control bits in CR0,
so only the first change in this patch is needed.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Christoph Egger]

On Tuesday 16 November 2010 14:19:14 Tim Deegan wrote:
> At 18:42 +0000 on 12 Nov (1289587334), Christoph Egger wrote:
> > @@ -1096,7 +1097,8 @@ int hvm_set_cr0(unsigned long value)
> >      /* ET is reserved and should be always be 1. */
> >      value |= X86_CR0_ET;
> >
> > -    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> > +    if ( !nestedhvm_vmswitch_in_progress(v) &&
> > +         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> >          goto gpf;
> >
> >      if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
> > @@ -1150,7 +1152,7 @@ int hvm_set_cr0(unsigned long value)
> >          }
> >      }
> >
> > -    if ( has_arch_mmios(v->domain) )
> > +    if ( !nestedhvm_vmswitch_in_progress(v) && has_arch_mmios(v->domain)
> > ) {
> >          if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
> >          {
>
> Like I said last time:
>
> I don't think that needs to happen -- it's up to the L1 VMM to decide
> whether the L2 VMM should be allowed to set cache control bits in CR0.

I think, I misunderstood you then. Fixed in local tree.

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: [PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Tim Deegan]

At 18:42 +0000 on 12 Nov (1289587334), Christoph Egger wrote:
> @@ -1096,7 +1097,8 @@ int hvm_set_cr0(unsigned long value)
>      /* ET is reserved and should be always be 1. */
>      value |= X86_CR0_ET;
>  
> -    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
> +    if ( !nestedhvm_vmswitch_in_progress(v) &&
> +         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
>          goto gpf;
>  
>      if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
> @@ -1150,7 +1152,7 @@ int hvm_set_cr0(unsigned long value)
>          }
>      }
>  
> -    if ( has_arch_mmios(v->domain) )
> +    if ( !nestedhvm_vmswitch_in_progress(v) && has_arch_mmios(v->domain) )
>      {
>          if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
>          {

Like I said last time:

I don't think that needs to happen -- it's up to the L1 VMM to decide
whether the L2 VMM should be allowed to set cache control bits in CR0.

Cheers,

Tim.


-- 
Tim Deegan <Tim.Deegan@citrix.com>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Christoph Egger]

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


-- 
---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_nh05_cr.diff --]
[-- Type: text/x-diff, Size: 1219 bytes --]

# HG changeset patch
# User cegger
# Date 1289574161 -3600
Allow paged real mode during vmrun emulation.
Emulate cr0 and cr4 when guest does not intercept them.

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

diff -r 5ea6bde78dc6 -r 7edc8a5266f9 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -54,6 +54,7 @@
 #include <asm/hvm/support.h>
 #include <asm/hvm/cacheattr.h>
 #include <asm/hvm/trace.h>
+#include <asm/hvm/nestedhvm.h>
 #include <asm/mtrr.h>
 #include <asm/apic.h>
 #include <public/sched.h>
@@ -1096,7 +1097,8 @@ int hvm_set_cr0(unsigned long value)
     /* ET is reserved and should be always be 1. */
     value |= X86_CR0_ET;
 
-    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
+    if ( !nestedhvm_vmswitch_in_progress(v) &&
+         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
         goto gpf;
 
     if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
@@ -1150,7 +1152,7 @@ int hvm_set_cr0(unsigned long value)
         }
     }
 
-    if ( has_arch_mmios(v->domain) )
+    if ( !nestedhvm_vmswitch_in_progress(v) && has_arch_mmios(v->domain) )
     {
         if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
         {

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

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

[PATCH 05/13] Nested Virtualization: CRn & paged real mode

[Christoph Egger]

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


-- 
---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_nh05_cr.diff --]
[-- Type: text/x-diff, Size: 1209 bytes --]

# HG changeset patch
# User cegger
# Date 1287134068 -7200
Allow paged real mode during vmrun emulation.
Emulate cr0 and cr4 when guest does not intercept them.

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

diff -r a1355c180672 -r 37f3c02d20c4 xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -54,6 +54,7 @@
 #include <asm/hvm/support.h>
 #include <asm/hvm/cacheattr.h>
 #include <asm/hvm/trace.h>
+#include <asm/hvm/nestedhvm.h>
 #include <asm/mtrr.h>
 #include <asm/apic.h>
 #include <public/sched.h>
@@ -1109,7 +1110,8 @@ int hvm_set_cr0(unsigned long value)
     /* ET is reserved and should be always be 1. */
     value |= X86_CR0_ET;
 
-    if ( (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
+    if ( !nestedhvm_vmentry_emulate(v) &&
+         (value & (X86_CR0_PE | X86_CR0_PG)) == X86_CR0_PG )
         goto gpf;
 
     if ( (value & X86_CR0_PG) && !(old_value & X86_CR0_PG) )
@@ -1163,7 +1165,7 @@ int hvm_set_cr0(unsigned long value)
         }
     }
 
-    if ( has_arch_mmios(v->domain) )
+    if ( !nestedhvm_vmentry_emulate(v) && has_arch_mmios(v->domain) )
     {
         if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) )
         {

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

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