From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Date: Mon, 01 Sep 2008 12:26:32 +0000 Subject: Re: [PATCH 3/6] kvmppc: rewrite guest code - sprg0-3 Message-Id: <48BBDF78.2050400@linux.vnet.ibm.com> List-Id: References: <1219142205-12062-4-git-send-email-ehrhardt@linux.vnet.ibm.com> In-Reply-To: <1219142205-12062-4-git-send-email-ehrhardt@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable To: kvm-ppc@vger.kernel.org Liu Yu wrote: >> -----Original Message----- >> From: kvm-ppc-owner@vger.kernel.org=20 >> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of=20 >> ehrhardt@linux.vnet.ibm.com >> Sent: Tuesday, August 19, 2008 6:37 PM >> To: kvm-ppc@vger.kernel.org >> Cc: hollisb@us.ibm.com; ehrhardt@linux.vnet.ibm.com >> Subject: [PATCH 3/6] kvmppc: rewrite guest code - sprg0-3 >> >> From: Christian Ehrhardt >> >> This patch adds the functionality of rewriting guest code=20 >> using the magic page >> mechanism. If a paravirtual guest memory (magic page) is=20 >> registered the host >> rewrites trapping emulations instead of emulating them. This=20 >> only works with >> instructions that can be rewritten with one instruction. >> On registration of the paravirtual memory the values have to=20 >> be synced so that >> the old emulated values can be found in the magic page now=20 >> and are ready to be >> read by the guest. >> The rewriting of guest code is guest visible (the guest=20 >> already agrees on this >> by registering a magic page) and is driven by the program=20 >> interrupts usually >> used to emulate these instructions in the hypervisor. >> This patch replaces m[ft]spr SPRG[0-3] instructions with stw=20 >> (write) and lwz >> (read) instructions. >> >> Signed-off-by: Christian Ehrhardt >> --- >> >> [diffstat] >> arch/powerpc/kvm/booke_guest.c | 17 ++++ >> arch/powerpc/kvm/emulate.c | 158=20 >> +++++++++++++++++++++++++++++++++++++++++ >> include/asm-powerpc/kvm_para.h | 27 ++++++- >> 3 files changed, 200 insertions(+), 2 deletions(-) >> >> [diff] >> diff --git a/arch/powerpc/kvm/booke_guest.c=20 >> b/arch/powerpc/kvm/booke_guest.c >> --- a/arch/powerpc/kvm/booke_guest.c >> +++ b/arch/powerpc/kvm/booke_guest.c >> @@ -339,7 +339,7 @@ >> gfn_t gfn; >> =20 >> =20 >> - if (vcpu->arch.pvmem && kvmppc_is_pvmem(vcpu, eaddr)) { >> + if (kvmppc_is_pvmem(vcpu, eaddr)) { >> kvmppc_mmu_map(vcpu, eaddr, >> vcpu->arch.pvmem_gpaddr >>=20 >> KVM_PPCPV_MAGIC_PAGE_SHIFT, >> 0, KVM_PPCPV_MAGIC_PAGE_FLAGS); >> @@ -536,6 +536,13 @@ >> for (i =3D 0; i < ARRAY_SIZE(regs->gpr); i++) >> regs->gpr[i] =3D vcpu->arch.gpr[i]; >> =20 >> + if (vcpu->arch.pvmem) { >> + regs->sprg0 =3D kvmppc_get_pvreg(vcpu,=20 >> KVM_PPCPV_OFFSET_SPRG0); >> + regs->sprg1 =3D kvmppc_get_pvreg(vcpu,=20 >> KVM_PPCPV_OFFSET_SPRG1); >> + regs->sprg2 =3D kvmppc_get_pvreg(vcpu,=20 >> KVM_PPCPV_OFFSET_SPRG2); >> + regs->sprg3 =3D kvmppc_get_pvreg(vcpu,=20 >> KVM_PPCPV_OFFSET_SPRG3); >> + } >> + >> return 0; >> } >> =20 >> @@ -561,6 +568,14 @@ >> =20 >> for (i =3D 0; i < ARRAY_SIZE(vcpu->arch.gpr); i++) >> vcpu->arch.gpr[i] =3D regs->gpr[i]; >> + >> + if (vcpu->arch.pvmem) { >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG0,=20 >> regs->sprg0); >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG1,=20 >> regs->sprg1); >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG2,=20 >> regs->sprg2); >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG3,=20 >> regs->sprg3); >> + } >> + >> =20 >> return 0; >> } >> diff --git a/arch/powerpc/kvm/emulate.c b/arch/powerpc/kvm/emulate.c >> --- a/arch/powerpc/kvm/emulate.c >> +++ b/arch/powerpc/kvm/emulate.c >> @@ -29,6 +29,7 @@ >> #include >> #include >> #include >> +#include >> #include >> =20 >> #include "44x_tlb.h" >> @@ -87,6 +88,37 @@ >> static inline unsigned int get_d(u32 inst) >> { >> return inst & 0xffff; >> +} >> + >> +static inline void set_op(u32 op, u32 *inst) >> +{ >> + *inst |=3D ((op & 0x3f) << 26); >> + return; >> +} >> + >> +static inline void set_ra(u32 ra, u32 *inst) >> +{ >> + *inst |=3D ((ra & 0x1f) << 16); >> + return; >> +} >> + >> +static inline void set_rs(u32 rs, u32 *inst) >> +{ >> + *inst |=3D ((rs & 0x1f) << 21); >> + return; >> +} >> + >> +static inline void set_rt(u32 rt, u32 *inst) >> +{ >> + *inst |=3D ((rt & 0x1f) << 21); >> + return; >> +} >> + >> +static inline void set_d(s16 d, u32 *inst) >> +{ >> + /* mask in last 16 signed bits */ >> + *inst |=3D ((u32)d & 0xFFFF); >> + return; >> } >> =20 >> static int tlbe_is_host_safe(const struct kvm_vcpu *vcpu, >> @@ -212,6 +244,8 @@ >> =20 >> switch (vcpu->arch.gpr[0]) { >> case KVM_HCALL_RESERVE_MAGICPAGE: >> + /* FIXME assert : we don't support this on smp guests */ >> + >> vcpu->arch.pvmem_gvaddr =3D vcpu->arch.gpr[3]; >> vcpu->arch.pvmem_gpaddr =3D vcpu->arch.gpr[4]; >> down_read(¤t->mm->mmap_sem); >> @@ -219,6 +253,18 @@ >> vcpu->arch.pvmem_gpaddr >>=20 >> KVM_PPCPV_MAGIC_PAGE_SHIFT); >> up_read(¤t->mm->mmap_sem); >> vcpu->arch.pvmem =3D kmap(pvmem_page); >> + >> + /* on enablement of pvmem support we need to=20 >> sync all values >> + * into the pvmem area to be able so correcty=20 >> fulfill read >> + * acesses that might occur prior to writes */ >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG0, >> + vcpu->arch.sprg0); >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG1, >> + vcpu->arch.sprg1); >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG2, >> + vcpu->arch.sprg2); >> + kvmppc_set_pvreg(vcpu, KVM_PPCPV_OFFSET_SPRG3, >> + vcpu->arch.sprg3); >> break; >> default: >> printk(KERN_ERR "unknown hypercall %d\n",=20 >> vcpu->arch.gpr[0]); >> @@ -232,6 +278,115 @@ >> return ret; >> } >> =20 >> +int kvmppc_pvmem_rewrite_instruction(struct kvm_vcpu *vcpu) >> +{ >> + int err =3D 0; >> + u32 inst =3D vcpu->arch.last_inst; >> + int sprn; >> + int rw =3D -1; >> + >> + int offset =3D -1; >> + >> + switch (get_op(inst)) { >> + case 31: >> + switch (get_xop(inst)) { >> + case 339: =20 >> /* mfspr */ >> + sprn =3D get_sprn(inst); >> + rw =3D KVM_PPCPV_PVMEM_READ; >> + switch (sprn) { >> + case SPRN_SPRG0: >> + offset =3D KVM_PPCPV_OFFSET_SPRG0; >> + break; >> + case SPRN_SPRG1: >> + offset =3D KVM_PPCPV_OFFSET_SPRG1; >> + break; >> + case SPRN_SPRG2: >> + offset =3D KVM_PPCPV_OFFSET_SPRG2; >> + break; >> + case SPRN_SPRG3: >> + offset =3D KVM_PPCPV_OFFSET_SPRG3; >> + break; >> + default: >> + err =3D -EFAULT; >> + } >> + break; >> + case 467: =20 >> /* mtspr */ >> + sprn =3D get_sprn(inst); >> + rw =3D KVM_PPCPV_PVMEM_WRITE; >> + switch (sprn) { >> + case SPRN_SPRG0: >> + offset =3D KVM_PPCPV_OFFSET_SPRG0; >> + break; >> + case SPRN_SPRG1: >> + offset =3D KVM_PPCPV_OFFSET_SPRG1; >> + break; >> + case SPRN_SPRG2: >> + offset =3D KVM_PPCPV_OFFSET_SPRG2; >> + break; >> + case SPRN_SPRG3: >> + offset =3D KVM_PPCPV_OFFSET_SPRG3; >> + break; >> + default: >> + err =3D -EFAULT; >> + } >> + break; >> + default: >> + err =3D -EFAULT; >> + } >> + break; >> + default: >> + err =3D -EFAULT; >> + } >> + >> + if (!err) { >> + u32 newinst =3D 0; >> + struct page *pcpage; >> + void *pcmem; >> + unsigned long pcaddr; >> + s16 displacement =3D 0; >> + gpa_t guest_pcaddr; >> + struct tlbe *gtlbe; >> + >> + BUG_ON(offset =3D -1); >> + /* calculate displacement of gvaddr+offset from 0 */ >> + displacement +=3D (vcpu->arch.pvmem_gvaddr & 0xFFFF); >> + displacement +=3D offset; >> + >> + BUG_ON(rw =3D -1); >> + if (rw =3D KVM_PPCPV_PVMEM_READ) { >> + set_op(32, &newinst); /* lwz */ >> + set_rt(get_rt(inst), &newinst); /* set=20 >> original rt */ >> + } else { >> + set_op(36, &newinst); /* stw */ >> + set_rs(get_rs(inst), &newinst); /* set=20 >> original rs */ >> + } >> + set_ra(0, &newinst); /* ra =3D 0 -> base addr 0=20 >> (no reg needed) */ >> + set_d(displacement, &newinst); /* set=20 >> displacement from 0 */ >> + >> + gtlbe =3D kvmppc_44x_itlb_search(vcpu, vcpu->arch.pc); >> + guest_pcaddr =3D tlb_xlate(gtlbe, vcpu->arch.pc); >> + if (!gtlbe) >> + return -EFAULT; >> + >> + down_read(¤t->mm->mmap_sem); >> + pcpage =3D gfn_to_page(vcpu->kvm, >> + guest_pcaddr >>=20 >> KVM_PPCPV_MAGIC_PAGE_SHIFT); >> + up_read(¤t->mm->mmap_sem); >> + if (pcpage =3D bad_page) >> + return -EFAULT; >> + >> + pcmem =3D kmap_atomic(pcpage, KM_USER0); >> + if (!pcmem) >> + return -EFAULT; >> + pcaddr =3D ((unsigned long)pcmem >> + | (vcpu->arch.pc &=20 >> KVM_PPCPV_MAGIC_PAGE_MASK)); >> + BUG_ON(inst !=3D *((u32 *)pcaddr)); >> + create_instruction(pcaddr, newinst); >> + kunmap_atomic(pcpage, KM_USER0); >> + } >> + >> + return err; >> +} >> =20 >> /* XXX to do: >> * lhax >> @@ -260,6 +415,9 @@ >> int dcrn; >> enum emulation_result emulated =3D EMULATE_DONE; >> int advance =3D 1; >> + >> + if (kvmppc_has_pvmem(vcpu) &&=20 >> !kvmppc_pvmem_rewrite_instruction(vcpu)) >> + return EMULATE_DONE; >> =20 >> switch (get_op(inst)) { >> case 0: >> diff --git a/include/asm-powerpc/kvm_para.h=20 >> b/include/asm-powerpc/kvm_para.h >> --- a/include/asm-powerpc/kvm_para.h >> +++ b/include/asm-powerpc/kvm_para.h >> @@ -34,7 +34,16 @@ >> */ >> #define KVM_PPCPV_MAGIC_PAGE_SIZE 4096 >> #define KVM_PPCPV_MAGIC_PAGE_SHIFT 12 >> +#define KVM_PPCPV_MAGIC_PAGE_MASK 0xFFF >> #define KVM_PPCPV_MAGIC_PAGE_FLAGS 0x3f >> + >> +#define KVM_PPCPV_PVMEM_READ 1 >> +#define KVM_PPCPV_PVMEM_WRITE 2 >> + >> +#define KVM_PPCPV_OFFSET_SPRG0 0x00 >> +#define KVM_PPCPV_OFFSET_SPRG1 0x04 >> +#define KVM_PPCPV_OFFSET_SPRG2 0x08 >> +#define KVM_PPCPV_OFFSET_SPRG3 0x0C >> =20 > > Maintaining copies is painful. > Is there any method to use the sole storage? > > For exampe, allocate a page beforehand, and initialize kvm_arch_vcpu on > the page. > After guest request para support, then map this page to guest. > =20 We discussed that in the past and came out that guest allocation of that=20 memory is really preferable. Allocating an additional piece just for that thing that is only=20 optionally used creates a lot of pain coding it. You would also need to split the vcpu.arch structure for that to only=20 map the accessible part to keep security e.g. for effective msr, ... There are just a lot of little issues and corner cases if we would share=20 that memory, which I believe in the end might be more complicated in=20 code&runtime complexity than just keeping a few pv copies. For example once changed sprg0-3 is only held in pvmem and not=20 interesting to the host in any way. And esr, dear, ... are only changed=20 and synced to pvmem if we are on the irq delivery path anyway. Feel free to catch us up on #kvm on freenode (You, I & Hollis) if you=20 want to discuss it in detail (nick =3D paelzer). >> =20 >> static inline int kvm_para_available(void) >> { >> @@ -55,7 +64,23 @@ >> =20 >> static inline int kvmppc_has_pvmem(struct kvm_vcpu *vcpu) >> { >> - return vcpu->arch.pvmem; >> + return !!vcpu->arch.pvmem; >> +} >> + >> +static inline u32 kvmppc_get_pvreg(struct kvm_vcpu *vcpu, int offset) >> +{ >> + u32 *pvreg; >> + >> + pvreg =3D vcpu->arch.pvmem + offset; >> + return *pvreg; >> +} >> + >> +static inline void kvmppc_set_pvreg(struct kvm_vcpu *vcpu,=20 >> int offset, u32 val) >> +{ >> + u32 *pvreg; >> + >> + pvreg =3D vcpu->arch.pvmem + offset; >> + *pvreg =3D val; >> } >> =20 >> #endif /* __KERNEL__ */ >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> =20 --=20 Gr=FCsse / regards,=20 Christian Ehrhardt IBM Linux Technology Center, Open Virtualization