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" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
