From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Jan Beulich" Subject: Re: [PATCH v3 07/32] xen/x86: fix arch_set_info_guest for HVM guests Date: Fri, 24 Jul 2015 04:46:01 -0600 Message-ID: <55B233890200007800095124@prv-mh.provo.novell.com> References: <1435923310-9019-1-git-send-email-roger.pau@citrix.com> <1435923310-9019-8-git-send-email-roger.pau@citrix.com> <55A3E0F102000078000903B5@mail.emea.novell.com> <55B0C100.6000308@citrix.com> <55B0EC520200007800094842@prv-mh.provo.novell.com> <1437651718.19412.92.camel@citrix.com> <55B103DD.4030901@citrix.com> <55B125440200007800094BD2@prv-mh.provo.novell.com> <55B10CDB.5010708@citrix.com> <1437667259.24746.12.camel@citrix.com> <55B11316.2000201@citrix.com> <55B130420200007800094CB8@prv-mh.provo.novell.com> <55B11CBE.8050704@citrix.com> <55B1208C.9090207@citrix.com> <55B211D70200007800094FCF@prv-mh.provo.novell.com> <55B20C9D.5070708@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZIaUQ-0006iX-QW for xen-devel@lists.xenproject.org; Fri, 24 Jul 2015 10:46:06 +0000 In-Reply-To: <55B20C9D.5070708@citrix.com> Content-Disposition: inline List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: =?UTF-8?Q?Roger=20Pau=20Monn=C3=A9?= Cc: Ian Campbell , Andrew Cooper , David Vrabel , xen-devel@lists.xenproject.org, Boris Ostrovsky List-Id: xen-devel@lists.xenproject.org >>> On 24.07.15 at 11:59, wrote: > Just to recap and to make sure I got all the points: > > - vCPU initialization from the toolstack (BSP initialization) is going > to be done using XEN_DOMCTL_sethvmcontext. > - XEN_DOMCTL_{get/set}vcpucontext is going to return EOPNOTSUPP for > HVM guests. > - A new vcpu_guest_contatext (vcpu_hvm_context?) is going to be > introduced together with a a new VCPUOP_initialize hypercall > (VCPUOP_hvm_initialize?). Since the current one is called VCPUOP_initialise, why not just VCPUOP_initialize? Having "hvm" in the name when it's meant to also be used for PVH may end up being confusing. > The following is a layout proposal for the new vcpu_context: > > struct vcpu_hvm_context { > /* 32bit fields of the structure will be used. */ > #define _VCPUHVM_MODE_32B 0 > #define VCPUHVM_MODE_32B (1<<_VCPUHVM_MODE_32B) > /* 64bit fields of the structure will be used. */ > #define _VCPUHVM_MODE_64B 1 > #define VCPUHVM_MODE_64B (1<<_VCPUHVM_MODE_64B) 16-bit mode? > #define _VCPUHVM_online 2 > #define VCPUHVM_online (1<<_VCPUHVM_online ) What is this needed for? > uint32_t flags; /* VCPUHVM_* flags. */ > struct cpu_hvm_regs user_regs; /* CPU registers. */ > }; > > #if defined(__GNUC__) && !defined(__STRICT_ANSI__) > /* Anonymous union includes both 32- and 64-bit names (e.g., ebp/rbp). */ > # define __DECL_REG(n64, n32) union { \ > uint64_t n64; \ > uint32_t n32; \ > } > #else > /* Non-gcc sources must always use the proper 64-bit name (e.g., rbp). */ > #define __DECL_REG(n64, n32) uint64_t n64 > #endif > > #define __DECL_GP_REG(n) __DECL_REG(r##n, e##n) > > struct cpu_hvm_regs { > /* General purpose registers. */ > __DECL_GP_REG(bx); > __DECL_GP_REG(cx); > __DECL_GP_REG(dx); > __DECL_GP_REG(si); > __DECL_GP_REG(di); > __DECL_GP_REG(bp); > __DECL_GP_REG(ax); > __DECL_GP_REG(ip); > __DECL_GP_REG(sp); Please put GPRs in their natural order - no need to mimic the order in any existing structure. > __DECL_GP_REG(flags); r8-r11, selector and descriptor registers, pseudo descriptor registers. Or else for all of them their default state would need to be spelled out. > /* Control registers. */ > uint64_t cr[8]; > /* Valid on amd64 only. */ Fields valid/useful in one mode only should probably be put in union-ized sub-structures. Jan > uint64_t efer; > > /* Debug registers. */ > uint64_t db[8]; > }; > > #undef __DECL_GP_REG > #undef __DECL_REG > > Of course the APs will be allowed to start in any mode they wish, > regardless of the mode the guest is currently running on. > > Roger.