On Thu, Jul 25, 2013 at 03:54:17PM +0200, Alexander Graf wrote: > > On 11.07.2013, at 13:50, Paul Mackerras wrote: > > > diff --git a/arch/powerpc/kvm/book3s_interrupts.S > > b/arch/powerpc/kvm/book3s_interrupts.S > > index 17cfae5..c935195 100644 > > --- a/arch/powerpc/kvm/book3s_interrupts.S > > +++ b/arch/powerpc/kvm/book3s_interrupts.S > > @@ -26,8 +26,12 @@ > > > > #if defined(CONFIG_PPC_BOOK3S_64) > > #define FUNC(name) GLUE(.,name) > > +#define GET_SHADOW_VCPU(reg) mr reg, r13 > > Is this correct?
Yes, it's just a copy of what's in book3s_segment.S already. For 64-bit the shadow vcpu is in the PACA (and the offsets defined in asm-offsets.c are expressed as offsets from the PACA). > > kvm_start_lightweight: > > + /* Copy registers into shadow vcpu so we can access them in real mode */ > > + GET_SHADOW_VCPU(r3) > > + PPC_LL r5, VCPU_GPR(R0)(r4) > > + PPC_LL r6, VCPU_GPR(R1)(r4) > > + PPC_LL r7, VCPU_GPR(R2)(r4) > > + PPC_LL r8, VCPU_GPR(R3)(r4) > > + PPC_STL r5, SVCPU_R0(r3) > > + PPC_STL r6, SVCPU_R1(r3) > > + PPC_STL r7, SVCPU_R2(r3) > > + PPC_STL r8, SVCPU_R3(r3) > > + PPC_LL r5, VCPU_GPR(R4)(r4) > > + PPC_LL r6, VCPU_GPR(R5)(r4) > > + PPC_LL r7, VCPU_GPR(R6)(r4) > > + PPC_LL r8, VCPU_GPR(R7)(r4) > > + PPC_STL r5, SVCPU_R4(r3) > > + PPC_STL r6, SVCPU_R5(r3) > > + PPC_STL r7, SVCPU_R6(r3) > > + PPC_STL r8, SVCPU_R7(r3) > > + PPC_LL r5, VCPU_GPR(R8)(r4) > > + PPC_LL r6, VCPU_GPR(R9)(r4) > > + PPC_LL r7, VCPU_GPR(R10)(r4) > > + PPC_LL r8, VCPU_GPR(R11)(r4) > > + PPC_STL r5, SVCPU_R8(r3) > > + PPC_STL r6, SVCPU_R9(r3) > > + PPC_STL r7, SVCPU_R10(r3) > > + PPC_STL r8, SVCPU_R11(r3) > > + PPC_LL r5, VCPU_GPR(R12)(r4) > > + PPC_LL r6, VCPU_GPR(R13)(r4) > > + lwz r7, VCPU_CR(r4) > > + PPC_LL r8, VCPU_XER(r4) > > + PPC_STL r5, SVCPU_R12(r3) > > + PPC_STL r6, SVCPU_R13(r3) > > + stw r7, SVCPU_CR(r3) > > + stw r8, SVCPU_XER(r3) > > + PPC_LL r5, VCPU_CTR(r4) > > + PPC_LL r6, VCPU_LR(r4) > > + PPC_LL r7, VCPU_PC(r4) > > + PPC_STL r5, SVCPU_CTR(r3) > > + PPC_STL r6, SVCPU_LR(r3) > > + PPC_STL r7, SVCPU_PC(r3) > > Can we put this and the reverse copy into C functions and just call them from > here and below? It'd definitely improve readability. We could even skip the > whole thing on systems where we're not limited by an RMA. Sure, will do. > Why are we even using a shadow vcpu on book3s_32? We can always just access > the real vcpu, no? Hrm. At this stage the vcpu is still part of the vcpu_book3s struct, which is vmalloc'd, so no we can't access it in real mode. However, I have a patch series which I'm about to post that will make the vcpu be kmalloc'd, and then 32-bit could access it directly in real mode, as could 64-bit bare-metal platforms if that made sense. > I think it makes sense to get rid of 99% of the svcpu logic. Remove all bits > in C code that ever refer to an svcpu. Only use the vcpu as reference for > everything. Until you hit the real mode barrier on book3s_64. There > explicitly copy the few things we need in real mode into the PACA and only > refer to the PACA in rm code. > > It'd be nice if we could speed up that code path on systems that don't need > to jump through hoops to get to their vcpu data (G5s, PowerStation, etc), but > I don't think it's worth the added complexity. We should rather try to make > the code easy to understand :). I agree completely. :) Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html