From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH v3 07/32] xen/x86: fix arch_set_info_guest for HVM guests Date: Wed, 5 Aug 2015 16:39:08 +0100 Message-ID: <55C22E1C.5020703@citrix.com> References: <1435923310-9019-1-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> <55B233890200007800095124@prv-mh.provo.novell.com> <55B22B5A.8020304@citrix.com> <55B24F4A02000078000952CC@prv-mh.provo.novell.com> <55B25941.9@citrix.com> <55B27AAB02000078000954A0@prv-mh.provo.novell.com> <55B26DB1.4020302@citrix.com> <55BF9CE6.1030904@citrix.com> <55BFA569.7050102@citrix.com> <55C0FF93.70501@citrix.com> <55C1DD1C.3060108@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1ZN0ml-0001iU-9o for xen-devel@lists.xenproject.org; Wed, 05 Aug 2015 15:39:19 +0000 In-Reply-To: <55C1DD1C.3060108@citrix.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: =?windows-1252?Q?Roger_Pau_Monn=E9?= , Jan Beulich Cc: xen-devel@lists.xenproject.org, Boris Ostrovsky , David Vrabel , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 05/08/15 10:53, Roger Pau Monn=E9 wrote: > El 04/08/15 a les 20.08, Andrew Cooper ha escrit: >> On 03/08/15 18:31, Roger Pau Monn=E9 wrote: >>>> Therefore, I am leaning slightly towards the specify the internals side >>>> of things, which removes some complexity from the Xen side of the hype= rcall. >>> I've updated the proposed structure, so now it looks like: >>> >>> (I still need to figure out how to order the bits in the access rights = >>> (_ar) fields.) >> All struct's need a flags register. > I was unsure about adding the {e/r}flags register, I will add it in new > versions. For first-gen VT-x, CR0.PE in unable to be cleared. HVMLoader has to do a dance with vm86 mode, which is where HVM_PARAM_IDENT_PT gets used. I don't realistically expect this to be a problem for DMlite guests. > >>> struct vcpu_hvm_x86_16 { >>> uint16_t ax; >>> uint16_t cx; >>> uint16_t dx; >>> uint16_t bx; >>> uint16_t sp; >>> uint16_t bp; >>> uint16_t si; >>> uint16_t di; >>> uint16_t ip; >>> >>> uint32_t cr[8]; >> Definitely no need for anything other than cr0 and 4 in 16 bit mode. > Yes. Would you rather prefer to spell out the exact control registers > that are going to be used instead of using an array? I would suggest so. CRs 1,5-7 are unconditionally #UD to attempt to use. CR2 is useless until you enter the #PF handler. CR8 may (hardware dependent) have the TPR in it, which is useless until the IDT is set up and interrupts have been enabled. > > For 16bit mode this is going to be CR0/CR4, for 32bit or long mode mode > CR0/CR3/CR4. Agreed. > >>> uint32_t cs_base; >>> uint32_t ds_base; >>> uint32_t ss_base; >>> uint32_t cs_limit; >>> uint32_t ds_limit; >>> uint32_t ss_limit; >>> uint16_t cs_ar; >>> uint16_t ds_ar; >>> uint16_t ss_ar; >>> }; >>> >>> struct vcpu_hvm_x86_32 { >>> uint32_t eax; >>> uint32_t ecx; >>> uint32_t edx; >>> uint32_t ebx; >>> uint32_t esp; >>> uint32_t ebp; >>> uint32_t esi; >>> uint32_t edi; >>> uint32_t eip; >>> >>> uint32_t cr[8]; >> Don't need cr's 5-8. >> >>> uint64_t efer; >>> >>> uint32_t cs_base; >>> uint32_t ds_base; >>> uint32_t ss_base; >>> uint32_t cs_limit; >>> uint32_t ds_limit; >>> uint32_t ss_limit; >>> uint16_t cs_ar; >>> uint16_t ds_ar; >>> uint16_t ss_ar; >> You need selector entries for each segment as well. > Really? What's the point in having the selector if we don't have a GDT, > and we allow loading the cached part, which is the relevant one. push %cs; push 1f; lret At all points when segment details are updated, it is the responsibility of software to ensure that the details match with the GDT/LDT entry. = See for example the Intel and AMD manuals for syscall/sysenter where similar "updating segment details behind the scenes" occurs. > >>> }; >>> >>> struct vcpu_hvm_x86_64 { >>> uint64_t rax; >>> uint64_t rcx; >>> uint64_t rdx; >>> uint64_t rbx; >>> uint64_t rsp; >>> uint64_t rbp; >>> uint64_t rsi; >>> uint64_t rdi; >>> uint64_t r8; >>> uint64_t r9; >>> uint64_t r10; >>> uint64_t r11; >>> uint64_t r12; >>> uint64_t r13; >>> uint64_t r14; >>> uint64_t r15; >>> uint64_t rip; >>> >>> uint64_t cr[8]; >>> uint64_t efer; >> What has happened to the segment information? It cannot be omitted >> completely, even in 64bit. > I had in mind to set them to the values required for long mode: > > base =3D 0 > limit =3D 0xffff > > and the attributes field for CS to: > > ar =3D 0x29b (L bit set) > > And for SS/DS: > > ar =3D 0x093 > > But maybe it makes sense to allow setting them in case someone wants to > start in compatibility mode. Agreed. ~Andrew