Re: [XenPPC] performance profiling current and future steps

[Christian Ehrhardt <ehrhardt@xxxxxxxxxxxxxxxxxx>]

Jimi Xenidis wrote:
> On Mar 23, 2007, at 9:07 AM, Hollis Blanchard wrote:
>
> > On Fri, 2007-03-23 at 08:52 +0100, Christian Ehrhardt wrote:
> > > Jimi Xenidis wrote:
> > > ...
> > > > > Ensure MMCR0[FCH] for this first step:
> > > > > -(ensure) set MMCR[FCH] always in xen when entering xen space. This
> > > > > should prevent a domain
> > > > >  messing up MMCR0[FCH]
> > > > > ->EXCEPTION_HEAD in exception.S set MMCR0[FCH] always
> > > > You need at least the following instructions:
> > > >     mfspr r0, SPRN_MMCR0
> > > >     ori r0, r0, 1 /* MMRC0_FCH */
> > > >     mtspr 795, SPRN_MMCR0
> > > >
> > > > Unfortuantely, there is not enough room in EXCEPTION_HEAD for that and
> > > > you will get:
> > > >
> > > >   exceptions.S: Assembler messages:
> > > >   exceptions.S:246: Error: attempt to .org/.space backwards? (-4)
> > > >   exceptions.S:253: Error: attempt to .org/.space backwards? (-4)
> > > >   exceptions.S:260: Error: attempt to .org/.space backwards? (-4)
> > > >   exceptions.S:267: Error: attempt to .org/.space backwards? (-4)
> > > >   exceptions.o: Bad value
> > > >   exceptions.S:626: FATAL: Can't write exceptions.o: Bad value
> > > >
> > > >
> > > > I suggest we start a new macro PMU_SAVE_STATE(save,scratch), which
> > > > does the above (for now, using only scratch) and sprinkling it in all
> > > > the code that EXCEPTION_HEAD branches to.
> > > Yep, I had those already:
> > >          mfspr r0,SPRN_MMCR0             /* ensure MMCR0[FCH]=1 */
> > >          ori r0,r0,MMCR0_FCH
> > >          mtspr SPRN_MMCR0, r0
> > >
> > > Do you mean to insert a branch to PMU_SAVE_STATE from EXCEPTION_HEAD or
> > > to put a PMU_SAVE_STATE branch to every section calling 
> > > EXCEPTION_HEAD ?
> > > Can someone please point at a code piece that does a similar macro
> > > definition and branch already.
> > We have 0x80 bytes for the "top half" of our exception handlers. This is
> > dictated by hardware: there are vectors at 0x300 and 0x380, so our code
> > must fit in that much space. If you do the math, that means we have room
> > for 32 instructions, including a branch to the rest of the handler
> > (named "ex_*_continued"). Right now, we use 30 instructions. So without
> > doing major exception handler surgery, your three new instructions will
> > need to go after the branch.
> >
> > Side note: I think it's ridiculous that our architecture allows the
> > guest to set MMCR0:FCH, since that bit controls the CPU while in
> > hypervisor mode. Anyways, disabling that bit a few dozen instructions
> > later shouldn't make a difference, and the domain never should have set
> > it in the first place.
> >
> > > Alternatively we could setup an "assist" session with screen -x
> > > developing this part together if there is a volunteer.
> > >
> > > I created this one blindly now as a start
> > >
> > >      .macro PMU_SAVE_STATE save scratch
> > >          mfspr \scratch,SPRN_MMCR0             /* ensure MMCR0[FCH]=1,
> > > not yet saving anything */
> > >          ori \scratch,\scratch,MMCR0_FCH
> > >          mtspr SPRN_MMCR0, \scratch
> > >      .endm
> > >
> > > and added the following to EXCEPTION_HEAD, but I guess it it wrong that
> > > way ;)
> > >
> > >      PMU_SAVE_STATE r0 r0
> > Right, that won't fit in EXCEPTION_HEAD (you will get the assembler
> > error messages Jimi pasted above).
> Yeah,
> So EXCEPTION_HEAD branches to a passed in label. Find all those labels 
> and insert PMU_SAVE_STATE there.
> -JX
I need a register to clobber with my "ori". (H_)EXCEPTION_SAVE_STATE 
clobbers r0 anyway and it resides in all continue labels that are passed 
to EXCEPTION_HEAD. So PMU_SAVE_STATE can follow in its wake and clobber 
r0 too like EXCEPTION_SAVE_STATE.
branches to EXCEPTION_HEAD:
       xenppc-unstable_step1/xen# cat 
arch/powerpc/powerpc64/exceptions.S | grep "EXCEPTION_HEAD" | sort | uniq
           EXCEPTION_HEAD r13 ex_dec_continued
           EXCEPTION_HEAD r13 ex_external_continued
           EXCEPTION_HEAD r13 ex_hcall_continued
           EXCEPTION_HEAD r13 ex_hdec_continued
           EXCEPTION_HEAD r13 ex_machcheck_continued
           EXCEPTION_HEAD r13 ex_program_continued



One of those calls as example "diff -Naur":
@@ -384,6 +391,7 @@
    mr r14, r0
    EXCEPTION_SAVE_STATE r1
+    PMU_SAVE_STATE r0,r0
    mr r4, r14
    LOADADDR r12, program_exception
    mr r3, r1                           /* pass pointer to cpu_user_regs */

Please correct me If my assumption is wrong that I'm allowed to clobber 
r0 there.
--

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
+49 7031/16-3385
Ehrhardt@xxxxxxxxxxxxxxxxxxx
Ehrhardt@xxxxxxxxxx

IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Johann Weihen 
Geschäftsführung: Herbert Kircher 
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294


_______________________________________________
Xen-ppc-devel mailing list
Xen-ppc-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ppc-devel