From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v3 07/32] xen/x86: fix arch_set_info_guest for HVM guests Date: Thu, 23 Jul 2015 12:41:58 +0100 Message-ID: <1437651718.19412.92.camel@citrix.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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZIEt1-0008FU-5x for xen-devel@lists.xenproject.org; Thu, 23 Jul 2015 11:42:03 +0000 In-Reply-To: <55B0EC520200007800094842@prv-mh.provo.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich , Roger Pau =?ISO-8859-1?Q?Monn=E9?= Cc: Andrew Cooper , xen-devel@lists.xenproject.org List-Id: xen-devel@lists.xenproject.org On Thu, 2015-07-23 at 05:29 -0600, Jan Beulich wrote: > > > > > > 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. On ARM we deliberately arranged that the vcpu_guest_context was the same for both arm32 and arm64. Moving to PVH seems like an ideal opportunity to do something similar for x86, if not by standardising on the x86_64 layout then by declaring a new struct which can encompass both and declaring the old ones PV legacy. Ian.