On Thu, 2014-11-27 at 10:56 -0600, Segher Boessenkool wrote: > On Thu, Nov 27, 2014 at 05:48:40PM +0530, Madhavan Srinivasan wrote: > > Here is the design of this patch. Since local_* operations > > are only need to be atomic to interrupts (IIUC), patch uses > > one of the Condition Register (CR) fields as a flag variable. When > > entering the local_*, specific bit in the CR5 field is set > > and on exit, bit is cleared. CR bit checking is done in the > > interrupt return path. If CR5[EQ] bit set and if we return > > to kernel, we reset to start of local_* operation. > > Have you tested this with (upcoming) GCC 5.0? GCC now uses CR5, > and it likes to use it very much, it might be more convenient to > use e.g. CR1 (which is allocated almost last, only before CR0).
We use CR1 all over the place in your asm code. Any other suggestion ? What's the damage of -ffixed-cr5 on gcc5 ? won't it just use CR4 or 6 instead ? > > --- a/arch/powerpc/include/asm/exception-64s.h > > +++ b/arch/powerpc/include/asm/exception-64s.h > > @@ -306,7 +306,26 @@ do_kvm_##n: > > \ > > std r10,0(r1); /* make stack chain pointer */ \ > > std r0,GPR0(r1); /* save r0 in stackframe */ \ > > std r10,GPR1(r1); /* save r1 in stackframe */ \ > > - beq 4f; /* if from kernel mode */ \ > > +BEGIN_FTR_SECTION; \ > > + lis r9,4096; /* Create a mask with HV and PR */ \ > > + rldicr r9,r9,32,31; /* bits, AND with the MSR */ \ > > + mr r10,r9; /* to check for Hyp state */ \ > > + ori r9,r9,16384; \ > > + and r9,r12,r9; \ > > + cmpd cr3,r10,r9; > > \ > > + beq cr3,66f; /* Jump if we come from Hyp mode*/ \ > > + mtcrf 0x04,r10; /* Clear CR5 if coming from usr */ \ > > Wow, such nastiness, only to avoid using dot insns (since you need to keep > the current CR0 value for the following beq 4f). And CR0 already holds the > PR bit, so you need only to check the HV bit anyway? Some restructuring > would make this a lot simpler and clearer. > > > + /* > > + * Now that we are about to exit from interrupt, lets check for > > + * cr5 eq bit. If it is set, then we may be in the middle of > > + * local_t update. In this case, we should rewind the NIP > > + * accordingly. > > + */ > > + mfcr r3 > > + andi. r4,r3,0x200 > > + beq 63f > > This is just bne cr5,63f isn't it? > > > index 72e783e..edb75a9 100644 > > --- a/arch/powerpc/kernel/exceptions-64s.S > > +++ b/arch/powerpc/kernel/exceptions-64s.S > > @@ -637,7 +637,7 @@ masked_##_H##interrupt: > > \ > > rldicl r10,r10,48,1; /* clear MSR_EE */ \ > > rotldi r10,r10,16; \ > > mtspr SPRN_##_H##SRR1,r10; \ > > -2: mtcrf 0x80,r9; \ > > +2: mtcrf 0x90,r9; \ > > ld r9,PACA_EXGEN+EX_R9(r13); \ > > ld r10,PACA_EXGEN+EX_R10(r13); \ > > ld r11,PACA_EXGEN+EX_R11(r13); \ > > What does this do? > > > Segher > _______________________________________________ > Linuxppc-dev mailing list > Linuxppc-dev@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/linuxppc-dev _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
