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

Reply via email to