thr3ads.net - Xen devel - [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest

If this information is useful, please help other people find it:
Share via:

Roger Pau Monne

2013-Nov-18 18:18 UTC

[PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

The aim of this patch is to define a stable way in which PVH is
going to do AP bringup.

Since we are running inside of a HVM container, PVH should only need
to set flags, cr3 and user_regs in order to bring up a vCPU, the rest
can be set once the vCPU is started using the bare metal methods.
Additionally, the guest can also set cr0 and cr4, and those values
will be appended to the default values set by Xen.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Mukesh Rathor <mukesh.rathor@oracle.com>
Cc: Jan Beulich <JBeulich@suse.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Keir Fraser <keir@xen.org>
---
 xen/arch/x86/domain.c             |   24 ++++++++++++++++--------
 xen/arch/x86/hvm/vmx/vmx.c        |    6 +-----
 xen/include/asm-x86/hvm/hvm.h     |    6 +++---
 xen/include/public/arch-x86/xen.h |    8 ++++----
 4 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a3868f9..51311fc 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -51,6 +51,7 @@
 #include <asm/fixmap.h>
 #include <asm/hvm/hvm.h>
 #include <asm/hvm/support.h>
+#include <asm/hvm/nestedhvm.h>
 #include <asm/debugreg.h>
 #include <asm/msr.h>
 #include <asm/traps.h>
@@ -704,9 +705,13 @@ int arch_set_info_guest(
         /* PVH 32bitfixme */
         ASSERT(!compat);
 
-        if ( c(ctrlreg[1]) || c(ldt_base) || c(ldt_ents) ||
+        if ( (c(ctrlreg[0]) & HVM_CR0_GUEST_RESERVED_BITS) ||
+             (c(ctrlreg[4]) & HVM_CR4_GUEST_RESERVED_BITS(v)) ||
+             c(ctrlreg[1]) || c(ctrlreg[2]) || c(ctrlreg[5]) ||
+             c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) || c(ldt_ents) ||
              c(user_regs.cs) || c(user_regs.ss) || c(user_regs.es) ||
              c(user_regs.ds) || c(user_regs.fs) || c(user_regs.gs) ||
+             c(kernel_ss) || c(kernel_sp) || c.nat->gs_base_kernel ||
              c.nat->gdt_ents || c.nat->fs_base || c.nat->gs_base_user
)
             return -EINVAL;
     }
@@ -745,17 +750,20 @@ int arch_set_info_guest(
 
     if ( has_hvm_container_vcpu(v) )
     {
-        /*
-         * NB: TF_kernel_mode is set unconditionally for HVM guests,
-         * so we always use the gs_base_kernel here. If we change this
-         * function to imitate the PV functionality, we'll need to
-         * make it pay attention to the kernel bit.
-         */
-        hvm_set_info_guest(v, compat ? 0 : c.nat->gs_base_kernel);
 
         if ( is_hvm_vcpu(v) || v->is_initialised )
             goto out;
 
+        if ( c.nat->ctrlreg[0] ) {
+            v->arch.hvm_vcpu.guest_cr[0] |= c.nat->ctrlreg[0];
+            hvm_update_guest_cr(v, 0);
+        }
+
+        if ( c.nat->ctrlreg[4] ) {
+            v->arch.hvm_vcpu.guest_cr[4] |= c.nat->ctrlreg[4];
+            hvm_update_guest_cr(v, 4);
+        }
+
         /* NB: No need to use PV cr3 un-pickling macros */
         cr3_gfn = c(ctrlreg[3]) >> PAGE_SHIFT;
         cr3_page = get_page_from_gfn(d, cr3_gfn, NULL, P2M_ALLOC);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index f0132a4..8d923e7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1467,7 +1467,7 @@ static int vmx_event_pending(struct vcpu *v)
     return intr_info & INTR_INFO_VALID_MASK;
 }
 
-static void vmx_set_info_guest(struct vcpu *v, uint64_t gs_base_kernel)
+static void vmx_set_info_guest(struct vcpu *v)
 {
     unsigned long intr_shadow;
 
@@ -1492,10 +1492,6 @@ static void vmx_set_info_guest(struct vcpu *v, uint64_t
gs_base_kernel)
         __vmwrite(GUEST_INTERRUPTIBILITY_INFO, intr_shadow);
     }
 
-    /* PVH 32bitfixme */
-    if ( is_pvh_vcpu(v) )
-        __vmwrite(GUEST_GS_BASE, gs_base_kernel);
-
     vmx_vmcs_exit(v);
 }
 
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index a8ba06d..ccca5df 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -160,7 +160,7 @@ struct hvm_function_table {
     int (*msr_write_intercept)(unsigned int msr, uint64_t msr_content);
     void (*invlpg_intercept)(unsigned long vaddr);
     void (*handle_cd)(struct vcpu *v, unsigned long value);
-    void (*set_info_guest)(struct vcpu *v, uint64_t gs_base_kernel);
+    void (*set_info_guest)(struct vcpu *v);
     void (*set_rdtsc_exiting)(struct vcpu *v, bool_t);
 
     /* Nested HVM */
@@ -434,10 +434,10 @@ void *hvm_map_guest_frame_rw(unsigned long gfn, bool_t
permanent);
 void *hvm_map_guest_frame_ro(unsigned long gfn, bool_t permanent);
 void hvm_unmap_guest_frame(void *p, bool_t permanent);
 
-static inline void hvm_set_info_guest(struct vcpu *v, uint64_t gs_base_kernel)
+static inline void hvm_set_info_guest(struct vcpu *v)
 {
     if ( hvm_funcs.set_info_guest )
-        return hvm_funcs.set_info_guest(v, gs_base_kernel);
+        return hvm_funcs.set_info_guest(v);
 }
 
 int hvm_debug_op(struct vcpu *v, int32_t op);
diff --git a/xen/include/public/arch-x86/xen.h
b/xen/include/public/arch-x86/xen.h
index 5d220ce..77d1b98 100644
--- a/xen/include/public/arch-x86/xen.h
+++ b/xen/include/public/arch-x86/xen.h
@@ -161,10 +161,10 @@ typedef uint64_t tsc_timestamp_t; /* RDTSC timestamp */
  * - For HVM guests, the structures read include: fpu_ctxt (if
  * VGCT_I387_VALID is set), flags, user_regs, debugreg[*]
  *
- * - PVH guests are the same as HVM guests, but additionally set cr3,
- * and for 64-bit guests, gs_base_kernel.  Additionally, the following
- * entries must be 0: ctrlreg[1], ldt_base, ldt_ents, user_regs.{cs,
- * ss, es, ds, fs, gs), gdt_ents, fs_base, and gs_base_user.
+ * - PVH guests are the same as HVM guests, but additionally use ctrlreg[0] to
+ * set cr0, ctrlreg[3] to set cr3 and ctrlreg[4] to set cr4. The values passed
+ * for cr0 and cr4 will be appended to the default cr0 and cr4 values set by
+ * Xen.
  */
 struct vcpu_guest_context {
     /* FPU registers come first so they can be aligned for FXSAVE/FXRSTOR. */
-- 
1.7.7.5 (Apple Git-26)


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

Mukesh Rathor

2013-Nov-18 22:30 UTC

head link

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

On Mon, 18 Nov 2013 19:18:31 +0100
Roger Pau Monne <roger.pau@citrix.com> wrote:
> The aim of this patch is to define a stable way in which PVH is
> going to do AP bringup.
> 
> Since we are running inside of a HVM container, PVH should only need
> to set flags, cr3 and user_regs in order to bring up a vCPU, the rest
> can be set once the vCPU is started using the bare metal methods.
> Additionally, the guest can also set cr0 and cr4, and those values
> will be appended to the default values set by Xen.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
OK from linux perspective. Tested. If non-maintainers are
allowed to ack:

Acked-by: Mukesh Rathor <mukesh.rathor@oracle.com>

thanks
Mukesh

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

George Dunlap

2013-Nov-19 10:34 UTC

head link

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

On 11/18/2013 06:18 PM, Roger Pau Monne wrote:> The aim of this patch is to define a stable way in which PVH is
> going to do AP bringup.
>
> Since we are running inside of a HVM container, PVH should only need
> to set flags, cr3 and user_regs in order to bring up a vCPU, the rest
> can be set once the vCPU is started using the bare metal methods.
> Additionally, the guest can also set cr0 and cr4, and those values
> will be appended to the default values set by Xen.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: George Dunlap <george.dunlap@eu.citrix.com>
> Cc: Mukesh Rathor <mukesh.rathor@oracle.com>
> Cc: Jan Beulich <JBeulich@suse.com>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Keir Fraser <keir@xen.org>
> ---
>   xen/arch/x86/domain.c             |   24 ++++++++++++++++--------
>   xen/arch/x86/hvm/vmx/vmx.c        |    6 +-----
>   xen/include/asm-x86/hvm/hvm.h     |    6 +++---
>   xen/include/public/arch-x86/xen.h |    8 ++++----
>   4 files changed, 24 insertions(+), 20 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index a3868f9..51311fc 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -51,6 +51,7 @@
>   #include <asm/fixmap.h>
>   #include <asm/hvm/hvm.h>
>   #include <asm/hvm/support.h>
> +#include <asm/hvm/nestedhvm.h>
>   #include <asm/debugreg.h>
>   #include <asm/msr.h>
>   #include <asm/traps.h>
> @@ -704,9 +705,13 @@ int arch_set_info_guest(
>           /* PVH 32bitfixme */
>           ASSERT(!compat);
>
> -        if ( c(ctrlreg[1]) || c(ldt_base) || c(ldt_ents) ||
> +        if ( (c(ctrlreg[0]) & HVM_CR0_GUEST_RESERVED_BITS) ||
> +             (c(ctrlreg[4]) & HVM_CR4_GUEST_RESERVED_BITS(v)) ||
Good idea. :-)
> +             c(ctrlreg[1]) || c(ctrlreg[2]) || c(ctrlreg[5]) ||
> +             c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) || c(ldt_ents)
||
>                c(user_regs.cs) || c(user_regs.ss) || c(user_regs.es) ||
>                c(user_regs.ds) || c(user_regs.fs) || c(user_regs.gs) ||
> +             c(kernel_ss) || c(kernel_sp) || c.nat->gs_base_kernel ||
>                c.nat->gdt_ents || c.nat->fs_base ||
c.nat->gs_base_user )
>               return -EINVAL;
>       }
> @@ -745,17 +750,20 @@ int arch_set_info_guest(
>
>       if ( has_hvm_container_vcpu(v) )
>       {
> -        /*
> -         * NB: TF_kernel_mode is set unconditionally for HVM guests,
> -         * so we always use the gs_base_kernel here. If we change this
> -         * function to imitate the PV functionality, we'll need to
> -         * make it pay attention to the kernel bit.
> -         */
> -        hvm_set_info_guest(v, compat ? 0 : c.nat->gs_base_kernel);
I think we still need to call hvm_set_info_guest() here -- there is 
other stuff being set up, and it was called (without the extra argument 
for gs_base_kernel) before the PVH patch series went in.

(See c/s 35b1e07.)
>
>           if ( is_hvm_vcpu(v) || v->is_initialised )
>               goto out;
>
> +        if ( c.nat->ctrlreg[0] ) {
> +            v->arch.hvm_vcpu.guest_cr[0] |= c.nat->ctrlreg[0];
> +            hvm_update_guest_cr(v, 0);
> +        }
> +
> +        if ( c.nat->ctrlreg[4] ) {
> +            v->arch.hvm_vcpu.guest_cr[4] |= c.nat->ctrlreg[4];
> +            hvm_update_guest_cr(v, 4);
> +        }
I thought the idea was that he would set cr0, cr3, and cr4 for all HVM 
guests, rather than special-casing PVH guests?

  -George


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

Roger Pau Monné

2013-Nov-19 10:42 UTC

head link

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

On 19/11/13 11:34, George Dunlap wrote:> On 11/18/2013 06:18 PM, Roger Pau Monne wrote:
>> The aim of this patch is to define a stable way in which PVH is
>> going to do AP bringup.
>>
>> Since we are running inside of a HVM container, PVH should only need
>> to set flags, cr3 and user_regs in order to bring up a vCPU, the rest
>> can be set once the vCPU is started using the bare metal methods.
>> Additionally, the guest can also set cr0 and cr4, and those values
>> will be appended to the default values set by Xen.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>> Cc: Mukesh Rathor <mukesh.rathor@oracle.com>
>> Cc: Jan Beulich <JBeulich@suse.com>
>> Cc: Tim Deegan <tim@xen.org>
>> Cc: Keir Fraser <keir@xen.org>
>> ---
>>   xen/arch/x86/domain.c             |   24 ++++++++++++++++--------
>>   xen/arch/x86/hvm/vmx/vmx.c        |    6 +-----
>>   xen/include/asm-x86/hvm/hvm.h     |    6 +++---
>>   xen/include/public/arch-x86/xen.h |    8 ++++----
>>   4 files changed, 24 insertions(+), 20 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>> index a3868f9..51311fc 100644
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -51,6 +51,7 @@
>>   #include <asm/fixmap.h>
>>   #include <asm/hvm/hvm.h>
>>   #include <asm/hvm/support.h>
>> +#include <asm/hvm/nestedhvm.h>
>>   #include <asm/debugreg.h>
>>   #include <asm/msr.h>
>>   #include <asm/traps.h>
>> @@ -704,9 +705,13 @@ int arch_set_info_guest(
>>           /* PVH 32bitfixme */
>>           ASSERT(!compat);
>>
>> -        if ( c(ctrlreg[1]) || c(ldt_base) || c(ldt_ents) ||
>> +        if ( (c(ctrlreg[0]) & HVM_CR0_GUEST_RESERVED_BITS) ||
>> +             (c(ctrlreg[4]) & HVM_CR4_GUEST_RESERVED_BITS(v)) ||
> 
> Good idea. :-)
> 
>> +             c(ctrlreg[1]) || c(ctrlreg[2]) || c(ctrlreg[5]) ||
>> +             c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) ||
>> c(ldt_ents) ||
>>                c(user_regs.cs) || c(user_regs.ss) || c(user_regs.es) ||
>>                c(user_regs.ds) || c(user_regs.fs) || c(user_regs.gs) ||
>> +             c(kernel_ss) || c(kernel_sp) || c.nat->gs_base_kernel
||
>>                c.nat->gdt_ents || c.nat->fs_base ||
c.nat->gs_base_user )
>>               return -EINVAL;
>>       }
>> @@ -745,17 +750,20 @@ int arch_set_info_guest(
>>
>>       if ( has_hvm_container_vcpu(v) )
>>       {
>> -        /*
>> -         * NB: TF_kernel_mode is set unconditionally for HVM guests,
>> -         * so we always use the gs_base_kernel here. If we change this
>> -         * function to imitate the PV functionality, we'll need to
>> -         * make it pay attention to the kernel bit.
>> -         */
>> -        hvm_set_info_guest(v, compat ? 0 : c.nat->gs_base_kernel);
> 
> I think we still need to call hvm_set_info_guest() here -- there is
> other stuff being set up, and it was called (without the extra argument
> for gs_base_kernel) before the PVH patch series went in.
> 
> (See c/s 35b1e07.)
I see, I will restore the hvm_set_info_guest call, good catch.
> 
>>
>>           if ( is_hvm_vcpu(v) || v->is_initialised )
>>               goto out;
>>
>> +        if ( c.nat->ctrlreg[0] ) {
>> +            v->arch.hvm_vcpu.guest_cr[0] |= c.nat->ctrlreg[0];
>> +            hvm_update_guest_cr(v, 0);
>> +        }
>> +
>> +        if ( c.nat->ctrlreg[4] ) {
>> +            v->arch.hvm_vcpu.guest_cr[4] |= c.nat->ctrlreg[4];
>> +            hvm_update_guest_cr(v, 4);
>> +        }
> 
> I thought the idea was that he would set cr0, cr3, and cr4 for all HVM
> guests, rather than special-casing PVH guests?
Well, I was just trying to make the PVH AP bringup interface stable,
without messing much with HVM (not that we shouldn't aim to do it for
HVM guests also), but I think the main concern now is releasing 4.4 with
a stable interface for PVH guests.

Thanks, Roger.


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

Roger Pau Monné

2013-Nov-19 10:44 UTC

head link

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

On 19/11/13 11:34, George Dunlap wrote:> On 11/18/2013 06:18 PM, Roger Pau Monne wrote:
>>       if ( has_hvm_container_vcpu(v) )
>>       {
>> -        /*
>> -         * NB: TF_kernel_mode is set unconditionally for HVM guests,
>> -         * so we always use the gs_base_kernel here. If we change this
>> -         * function to imitate the PV functionality, we''ll
need to
>> -         * make it pay attention to the kernel bit.
>> -         */
>> -        hvm_set_info_guest(v, compat ? 0 : c.nat->gs_base_kernel);
> 
> I think we still need to call hvm_set_info_guest() here -- there is
> other stuff being set up, and it was called (without the extra argument
> for gs_base_kernel) before the PVH patch series went in.
> 
> (See c/s 35b1e07.)
I see, I will restore the hvm_set_info_guest call, good catch.
> 
>>
>>           if ( is_hvm_vcpu(v) || v->is_initialised )
>>               goto out;
>>
>> +        if ( c.nat->ctrlreg[0] ) {
>> +            v->arch.hvm_vcpu.guest_cr[0] |= c.nat->ctrlreg[0];
>> +            hvm_update_guest_cr(v, 0);
>> +        }
>> +
>> +        if ( c.nat->ctrlreg[4] ) {
>> +            v->arch.hvm_vcpu.guest_cr[4] |= c.nat->ctrlreg[4];
>> +            hvm_update_guest_cr(v, 4);
>> +        }
> 
> I thought the idea was that he would set cr0, cr3, and cr4 for all HVM
> guests, rather than special-casing PVH guests?
Well, I was just trying to make the PVH AP bringup interface stable,
without messing much with HVM (not that we shouldn''t aim to do it for
HVM guests also), but I think the main concern now is releasing 4.4 with
a stable interface for PVH guests.

Thanks, Roger.

George Dunlap

2013-Nov-19 11:10 UTC

head link

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

On 11/19/2013 10:42 AM, Roger Pau Monné wrote:> On 19/11/13 11:34, George Dunlap wrote:
>> On 11/18/2013 06:18 PM, Roger Pau Monne wrote:
>>> The aim of this patch is to define a stable way in which PVH is
>>> going to do AP bringup.
>>>
>>> Since we are running inside of a HVM container, PVH should only
need
>>> to set flags, cr3 and user_regs in order to bring up a vCPU, the
rest
>>> can be set once the vCPU is started using the bare metal methods.
>>> Additionally, the guest can also set cr0 and cr4, and those values
>>> will be appended to the default values set by Xen.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Cc: George Dunlap <george.dunlap@eu.citrix.com>
>>> Cc: Mukesh Rathor <mukesh.rathor@oracle.com>
>>> Cc: Jan Beulich <JBeulich@suse.com>
>>> Cc: Tim Deegan <tim@xen.org>
>>> Cc: Keir Fraser <keir@xen.org>
>>> ---
>>>    xen/arch/x86/domain.c             |   24
++++++++++++++++--------
>>>    xen/arch/x86/hvm/vmx/vmx.c        |    6 +-----
>>>    xen/include/asm-x86/hvm/hvm.h     |    6 +++---
>>>    xen/include/public/arch-x86/xen.h |    8 ++++----
>>>    4 files changed, 24 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
>>> index a3868f9..51311fc 100644
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -51,6 +51,7 @@
>>>    #include <asm/fixmap.h>
>>>    #include <asm/hvm/hvm.h>
>>>    #include <asm/hvm/support.h>
>>> +#include <asm/hvm/nestedhvm.h>
>>>    #include <asm/debugreg.h>
>>>    #include <asm/msr.h>
>>>    #include <asm/traps.h>
>>> @@ -704,9 +705,13 @@ int arch_set_info_guest(
>>>            /* PVH 32bitfixme */
>>>            ASSERT(!compat);
>>>
>>> -        if ( c(ctrlreg[1]) || c(ldt_base) || c(ldt_ents) ||
>>> +        if ( (c(ctrlreg[0]) & HVM_CR0_GUEST_RESERVED_BITS) ||
>>> +             (c(ctrlreg[4]) & HVM_CR4_GUEST_RESERVED_BITS(v))
||
>>
>> Good idea. :-)
>>
>>> +             c(ctrlreg[1]) || c(ctrlreg[2]) || c(ctrlreg[5]) ||
>>> +             c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) ||
>>> c(ldt_ents) ||
>>>                 c(user_regs.cs) || c(user_regs.ss) ||
c(user_regs.es) ||
>>>                 c(user_regs.ds) || c(user_regs.fs) ||
c(user_regs.gs) ||
>>> +             c(kernel_ss) || c(kernel_sp) ||
c.nat->gs_base_kernel ||
>>>                 c.nat->gdt_ents || c.nat->fs_base ||
c.nat->gs_base_user )
>>>                return -EINVAL;
>>>        }
>>> @@ -745,17 +750,20 @@ int arch_set_info_guest(
>>>
>>>        if ( has_hvm_container_vcpu(v) )
>>>        {
>>> -        /*
>>> -         * NB: TF_kernel_mode is set unconditionally for HVM
guests,
>>> -         * so we always use the gs_base_kernel here. If we change
this
>>> -         * function to imitate the PV functionality, we'll
need to
>>> -         * make it pay attention to the kernel bit.
>>> -         */
>>> -        hvm_set_info_guest(v, compat ? 0 :
c.nat->gs_base_kernel);
>>
>> I think we still need to call hvm_set_info_guest() here -- there is
>> other stuff being set up, and it was called (without the extra argument
>> for gs_base_kernel) before the PVH patch series went in.
>>
>> (See c/s 35b1e07.)
>
> I see, I will restore the hvm_set_info_guest call, good catch.
>
>>
>>>
>>>            if ( is_hvm_vcpu(v) || v->is_initialised )
>>>                goto out;
>>>
>>> +        if ( c.nat->ctrlreg[0] ) {
>>> +            v->arch.hvm_vcpu.guest_cr[0] |=
c.nat->ctrlreg[0];
>>> +            hvm_update_guest_cr(v, 0);
>>> +        }
>>> +
>>> +        if ( c.nat->ctrlreg[4] ) {
>>> +            v->arch.hvm_vcpu.guest_cr[4] |=
c.nat->ctrlreg[4];
>>> +            hvm_update_guest_cr(v, 4);
>>> +        }
>>
>> I thought the idea was that he would set cr0, cr3, and cr4 for all HVM
>> guests, rather than special-casing PVH guests?
>
> Well, I was just trying to make the PVH AP bringup interface stable,
> without messing much with HVM (not that we shouldn't aim to do it for
> HVM guests also), but I think the main concern now is releasing 4.4 with
> a stable interface for PVH guests.
Right -- I guess it's better safe than sorry wrt changing the HVM paths; 
we can make it PVH for now, and make it more generic for 4.5.

  -George

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

George Dunlap

2013-Nov-19 11:11 UTC

head link

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

On 11/18/2013 10:30 PM, Mukesh Rathor wrote:> On Mon, 18 Nov 2013 19:18:31 +0100
> Roger Pau Monne <roger.pau@citrix.com> wrote:
>
>> The aim of this patch is to define a stable way in which PVH is
>> going to do AP bringup.
>>
>> Since we are running inside of a HVM container, PVH should only need
>> to set flags, cr3 and user_regs in order to bring up a vCPU, the rest
>> can be set once the vCPU is started using the bare metal methods.
>> Additionally, the guest can also set cr0 and cr4, and those values
>> will be appended to the default values set by Xen.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>
> OK from linux perspective. Tested. If non-maintainers are
> allowed to ack:
>
> Acked-by: Mukesh Rathor <mukesh.rathor@oracle.com>
I think for this code your opinion definitely matters. :-)

Thanks,
  -George

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

Seemingly Similar Threads

Search for more reasonably related threads

Xen devel - Nov 2013 - [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

[PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Re: [PATCH RFC v2] pvh: clearly specify used parameters in vcpu_guest_context

Seemingly Similar Threads