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

[George Dunlap <george.dunlap@eu.citrix.com>]

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