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: Thu, 23 Jul 2015 05:29:54 -0600 Message-ID: <55B0EC520200007800094842@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> 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 1ZIEhL-0006i1-V6 for xen-devel@lists.xenproject.org; Thu, 23 Jul 2015 11:30:00 +0000 In-Reply-To: <55B0C100.6000308@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: Andrew Cooper , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org >>> On 23.07.15 at 12:25, wrote: > El 13/07/15 a les 16.01, Jan Beulich ha escrit: >>>>> On 03.07.15 at 13:34, wrote: >>> --- a/xen/arch/x86/domain.c >>> +++ b/xen/arch/x86/domain.c >>> @@ -795,6 +795,15 @@ int arch_set_info_guest( >>> c.nat->fs_base || c.nat->gs_base_user)) ) >>> return -EINVAL; >>> } >>> + else if ( is_hvm_domain(d) ) >>> + { >>> + if ( c(ctrlreg[0]) || c(ctrlreg[1]) || c(ctrlreg[2]) || >>> + c(ctrlreg[3]) || c(ctrlreg[4]) || c(ctrlreg[5]) || >>> + c(ctrlreg[6]) || c(ctrlreg[7]) || c(ldt_base) || >>> + c(ldt_ents) || c(kernel_ss) || c(kernel_sp) || >>> + c(gdt_ents) ) >>> + return -EINVAL; >>> + } >> >> In addition to what Andrew said - is the use of c() here really correct >> considering >> >> compat = is_pv_32bit_domain(d); >> >> #define c(fld) (compat ? (c.cmp->fld) : (c.nat->fld)) >> >> near the beginning of the function? > > I've been thinking about this. Since HVM vCPUs are always started in > 32bit mode, I would ideally like to use the vcpu_guest_context_x86_32_t > struct to set the vCPU context. This means that for HVM guests "compat" > should always be true. > > The problem is that inside of the guest there's no notion of > vcpu_guest_context_x86_32_t or vcpu_guest_context_x86_64_t, there's only > vcpu_guest_context, which defaults to the native bitness of the guest. > So if your guest is running in 64bit mode vcpu_guest_context is aliased > to vcpu_guest_context_x86_64_t by default. > > TBH I'm not sure about the best way to solve this. Should > vcpu_guest_context_x86_32_t and vcpu_guest_context_x86_64_t be exposed > to the guest like it's done for the tools? Why? We're in arch_set_info_guest(), which is unreachable for a HVM guest on itself. Or is this in preparation of allowing HVM guests to use VCPUOP_initialise? In that case, exposing might be an option, but remember that the compat mode layout even today is used only for PV guests. So I'd rather avoid exposing both layouts to guests, and do translation instead inside the hypervisor. Jan