Xen project Mailing List

Re: [Xen-devel] [PATCH 1/2] pvh: clearly specify used parameters in vcpu_guest_context

From: Roger Pau MonnÃ <roger.pau@xxxxxxxxxx>

Date: Wed, 20 Nov 2013 10:18:12 +0100

Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Keir Fraser <keir@xxxxxxx>, Tim Deegan <tim@xxxxxxx>

Delivery-date: Wed, 20 Nov 2013 09:18:29 +0000

List-id: Xen developer discussion <xen-devel.lists.xen.org>

On 19/11/13 17:53, Jan Beulich wrote: >>>> On 19.11.13 at 17:42, Roger Pau MonnÃ<roger.pau@xxxxxxxxxx> wrote: >> On 19/11/13 16:34, Jan Beulich wrote: >>>>>> On 19.11.13 at 16:04, Roger Pau MonnÃ<roger.pau@xxxxxxxxxx> wrote: >>>> On 19/11/13 14:32, Jan Beulich wrote: >>>>> Also, by now honoring CR0 and CR4 settings, you again move >>>>> towards the hybrid model (some fields honored, some refused) >>>>> that was (I think by you) previously described as unacceptable. >>>> >>>> From a strict POV we should just set cr3, flags and user_regs, but then >>>> Tim mentioned also honouring cr0/cr4, >>> >>> I understood his response to mean all fields, or none of them. >> >> Trying to make all those fields functional on PVH (or HVM) is quite >> useless IMHO, it's going to add more code that I doubt anyone is going >> to use when you can instead use the bare metal functions to set all >> those things (and from an OS point of view it's also more comfortable >> because you need less Xen specific stuff). > > That last part I certainly agree to, but that would apply to CR0 > and CR4 just as much. I've removed the usage of anything that's not strictly necessary in order to do AP bringup, so I've removed the setting of debugregs: --- From 1fa84464ca8b65860a21e4e3d9ac9646bfe5591b Mon Sep 17 00:00:00 2001 From: Roger Pau Monne <roger.pau@xxxxxxxxxx> Date: Thu, 14 Nov 2013 18:07:51 +0100 Subject: [PATCH v2] pvh: clearly specify used parameters in vcpu_guest_context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. Signed-off-by: Roger Pau MonnÃ <roger.pau@xxxxxxxxxx> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx> Cc: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> Cc: Jan Beulich <JBeulich@xxxxxxxx> Cc: Tim Deegan <tim@xxxxxxx> Cc: Keir Fraser <keir@xxxxxxx> --- 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, 18 insertions(+), 26 deletions(-) diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index a3868f9..aa043a8 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -704,10 +704,16 @@ int arch_set_info_guest( /* PVH 32bitfixme */ ASSERT(!compat); - if ( c(ctrlreg[1]) || 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.nat->gdt_ents || c.nat->fs_base || c.nat->gs_base_user ) + if ( c(ctrlreg[0]) || c(ctrlreg[1]) || c(ctrlreg[2]) || + c(ctrlreg[4]) || c(ctrlreg[5]) || c(ctrlreg[6]) || + c(ctrlreg[7]) || c(debugreg[0]) || c(debugreg[1]) || + c(debugreg[2]) || c(debugreg[3]) || c(debugreg[4]) || + c(debugreg[5]) || c(debugreg[6]) || c(debugreg[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; } @@ -740,18 +746,10 @@ int arch_set_info_guest( XLAT_trap_info(v->arch.pv_vcpu.trap_ctxt + i, c.cmp->trap_ctxt + i); } - for ( i = 0; i < ARRAY_SIZE(v->arch.debugreg); ++i ) - v->arch.debugreg[i] = c(debugreg[i]); 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); + hvm_set_info_guest(v); if ( is_hvm_vcpu(v) || v->is_initialised ) goto out; 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..8c92308 100644 --- a/xen/include/public/arch-x86/xen.h +++ b/xen/include/public/arch-x86/xen.h @@ -159,12 +159,10 @@ typedef uint64_t tsc_timestamp_t; /* RDTSC timestamp */ * for HVM and PVH guests, not all information in this structure is updated: * * - For HVM guests, the structures read include: fpu_ctxt (if - * VGCT_I387_VALID is set), flags, user_regs, debugreg[*] + * VGCT_I387_VALID is set), flags and user_regs. * - * - 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[3] to + * set cr3. All other fields not used should be set to 0. */ 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@xxxxxxxxxxxxx http://lists.xen.org/xen-devel

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.