[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [microblaze-uclinux] [patch] msrset/msrclr support
Hi - I see how the FSL saves development time, but how does it execute
faster than an instruction, when both are made of HW logic on the same
chip?
On Tue, 2004-05-18 at 01:53, Goran Bilski wrote:
> Hi,
>
> It's much better to find the C-function that consumes most time in the
> kernel and put the whole function on a FSL.
> The FSL interface exists today and you can build your own function and
> attach to it.
> It will give more you more speed up than user defined instructions.
>
> Göran
>
> Matthew Rubenstein wrote:
>
> > Has anyone profiled the code to find the most common instruction
> >sequences, which might be similarly replaced by atomic instructions? Is
> >there anything preventing rolling our own instruction patches, rather
> >than waiting for Xilinx? Another win for the platform over hardcoded
> >CPU. Perhaps a new tool might be added to the toolchain which would
> >profile the code, report candidates for porting to instructions, and
> >recompile once the new instruction dependencies were satisfied in
> >Verilog.
> >
> >
> >On Mon, 2004-05-17 at 22:30, John Williams wrote:
> >
> >
> >>Hi folks,
> >>
> >>The latest version of the microblaze core in EDK6.2 SP1
> >>(microblaze_v2_10_a) has optional support for a new pair of instructions:
> >>
> >>msrset rd, IMM and msrclr rd, IMM
> >>
> >>this pair of instructions are basically an atomic test-and-set on the
> >>microblaze status register. We lobbied Xilinx to add these on the
> >>grounds of the code size reduction:
> >>
> >>mfs rd, rmsr
> >>andi rd, rd, ~2
> >>mts rmsr, rd
> >>
> >>becomes
> >>
> >>msrclr r0, ~2
> >>
> >>to clear interrupts, and similarly for enable interrupts (and caches
> >>too). Also you no longer need a scratch register when doing these
> >>manipulations on the MSR.
> >>
> >>Even better (well, in fact by design!), the msrclr instruction maps
> >>directly onto the very common save_flags_cli() macro, that is sprinkled
> >>liberally throughout the kernel and drivers.
> >>
> >>The attached patch (David or Greg would you please apply?) adds optional
> >>support for these new instructions. It is selectable from the
> >>configuration menu (processor options, in kernel config).
> >>
> >>Using this option saves roughly 15kbytes of code in a typical kernel
> >>image, or about 1.5 percent. I expect there's a small performance
> >>improvement as well, but haven't tried to measure it yet.
> >>
> >>The only catch is that a gremlin in EDK6.2 (and also SP1 alas) means
> >>that you have to twiddle your microblaze.mpd file to allow you to set
> >>the C_USE_MSR_INSTR parameter. It's trivial:
> >>
> >>edit
> >>/edk6.2/hw/XilinxProcessorIPLib/pcores/microblaze_v2_10_a/data/microblaze_v2_1_0.mpd
> >>
> >>add a new line (somewhere around line 50, among the other PARAMETER
> >>declarations, doesn't matter):
> >>
> >>PARAMETER C_USE_MSR_INSTR = 0, DT = integer
> >>
> >>Finally, edit the system.mhs file of your favourite mbvanilla target,
> >>and add the following in the microblaze_0 block:
> >>
> >>PARAMETER C_USE_MSR_INSTR = 1
> >>
> >>Enable support in the kernel as described above, make dep etc, and off
> >>you go!
> >>
> >>Congratulations if you are still reading! Any feedback welcome.
> >>
> >>Cheers,
> >>
> >>John
> >>
> >>______________________________________________________________________
> >>Index: include/asm-microblaze/system.h
> >>===================================================================
> >>RCS file: /var/cvs/uClinux-2.4.x/include/asm-microblaze/system.h,v
> >>retrieving revision 1.5
> >>diff -u -b -B -w -p -r1.5 system.h
> >>--- include/asm-microblaze/system.h 10 Dec 2003 02:39:03 -0000 1.5
> >>+++ include/asm-microblaze/system.h 18 May 2004 02:14:38 -0000
> >>@@ -37,10 +37,46 @@ extern void *switch_thread (struct threa
> >> } \
> >> } while (0)
> >>
> >>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
> >>+/* the msrset/msrclr instructions are available, let's use them! */
> >>
> >> /* Enable/disable interrupts. */
> >> #define __sti() \
> >> { \
> >>+ __asm__ __volatile__ ("msrset r0, 0x2;" \
> >>+ : \
> >>+ : \
> >>+ : "memory"); \
> >>+}
> >>+
> >>+#define __cli() \
> >>+{ \
> >>+ __asm__ __volatile__ ("msrclr r0, 0x2;" \
> >>+ : \
> >>+ : \
> >>+ : "memory"); \
> >>+}
> >>+
> >>+#define __save_flags_cli(flags) \
> >>+{ \
> >>+ __asm__ __volatile__ ("msrclr %0, 0x2;" \
> >>+ : "=r" (flags) \
> >>+ : \
> >>+ : "memory"); \
> >>+}
> >>+
> >>+#define __save_flags_sti(flags) \
> >>+{ \
> >>+ __asm__ __volatile__ ("msrset %0, 0x2;" \
> >>+ : "=r" (flags) \
> >>+ : \
> >>+ : "memory"); \
> >>+}
> >>+
> >>+#else /* MICROBLAZE_CONFIG_MSRSETCLR not set */
> >>+/* Enable/disable interrupts. */
> >>+#define __sti() \
> >>+{ \
> >> register unsigned tmp; \
> >> __asm__ __volatile__ (" \
> >> mfs %0, rmsr; \
> >>@@ -63,11 +99,6 @@ extern void *switch_thread (struct threa
> >> : "memory"); \
> >> }
> >>
> >>-#define __save_flags(flags) \
> >>- __asm__ __volatile__ ("mfs %0, rmsr" : "=r" (flags))
> >>-#define __restore_flags(flags) \
> >>- __asm__ __volatile__ ("mts rmsr, %0" :: "r" (flags))
> >>-
> >> #define __save_flags_cli(flags) \
> >> { \
> >> register unsigned tmp; \
> >>@@ -91,6 +122,13 @@ extern void *switch_thread (struct threa
> >> : \
> >> : "memory"); \
> >> }
> >>+
> >>+#endif
> >>+
> >>+#define __save_flags(flags) \
> >>+ __asm__ __volatile__ ("mfs %0, rmsr" : "=r" (flags))
> >>+#define __restore_flags(flags) \
> >>+ __asm__ __volatile__ ("mts rmsr, %0" :: "r" (flags))
> >>
> >> /* For spinlocks etc */
> >> #define local_irq_save(flags) __save_flags_cli (flags)
> >>Index: include/asm-microblaze/cache.h
> >>===================================================================
> >>RCS file: /var/cvs/uClinux-2.4.x/include/asm-microblaze/cache.h,v
> >>retrieving revision 1.3
> >>diff -u -b -B -w -p -r1.3 cache.h
> >>--- include/asm-microblaze/cache.h 22 Sep 2003 04:34:25 -0000 1.3
> >>+++ include/asm-microblaze/cache.h 18 May 2004 02:14:41 -0000
> >>@@ -24,7 +24,23 @@
> >> #define ICACHE_MSR_BIT (1 << 5)
> >> #define DCACHE_MSR_BIT (1 << 7)
> >>
> >>-#ifdef CONFIG_MICROBLAZE_ICACHE
> >>+#ifdef CONFIG_MICROBLAZE_ICACHE /* Cache support? */
> >>+
> >>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
> >>+#define __enable_icache() \
> >>+ __asm__ __volatile__ (" msrset r0, %0;" \
> >>+ : \
> >>+ : "i" (ICACHE_MSR_BIT) \
> >>+ : "memory")
> >>+
> >>+#define __disable_icache() \
> >>+ __asm__ __volatile__ (" msrclr r0, %0;" \
> >>+ : \
> >>+ : "i" (ICACHE_MSR_BIT) \
> >>+ : "memory")
> >>+
> >>+
> >>+#else /* !CONFIG_MICROBLAZE_MSRSETCLR */
> >> #define __enable_icache() \
> >> __asm__ __volatile__ (" \
> >> mfs r12, rmsr; \
> >>@@ -44,6 +60,8 @@
> >> : "memory", "r12")
> >>
> >>
> >>+#endif /* CONFIG_MICROBLAZE_MSRSETCLR */
> >>+
> >> #define __invalidate_icache(addr) \
> >> __asm__ __volatile__ (" \
> >> wic %0, r0" \
> >>@@ -56,6 +74,21 @@
> >> #endif
> >>
> >> #ifdef CONFIG_MICROBLAZE_DCACHE
> >>+
> >>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
> >>+#define __enable_dcache() \
> >>+ __asm__ __volatile__ (" msrset r0, %0;" \
> >>+ : \
> >>+ : "i" (DCACHE_MSR_BIT) \
> >>+ : "memory")
> >>+
> >>+#define __disable_dcache() \
> >>+ __asm__ __volatile__ (" msrclr r0, %0;" \
> >>+ : \
> >>+ : "i" (DCACHE_MSR_BIT) \
> >>+ : "memory")
> >>+
> >>+#else /* !CONFIG_MICROBLAZE_MSRSETCLR */
> >> #define __enable_dcache() \
> >> __asm__ __volatile__ (" \
> >> mfs r12, rmsr; \
> >>@@ -73,6 +106,8 @@
> >> : \
> >> : "i" (DCACHE_MSR_BIT) \
> >> : "memory", "r12")
> >>+
> >>+#endif /* CONFIG_MICROBLAZE_MSRSETCLR */
> >>
> >> #define __invalidate_dcache(addr) \
> >> __asm__ __volatile__ (" \
> >>Index: arch/microblaze/kernel/entry.S
> >>===================================================================
> >>RCS file: /var/cvs/uClinux-2.4.x/arch/microblaze/kernel/entry.S,v
> >>retrieving revision 1.7
> >>diff -u -b -B -w -p -r1.7 entry.S
> >>--- arch/microblaze/kernel/entry.S 4 Mar 2004 03:47:35 -0000 1.7
> >>+++ arch/microblaze/kernel/entry.S 18 May 2004 02:14:51 -0000
> >>@@ -31,6 +31,25 @@
> >> /* The offset of the struct pt_regs in a `state save frame' on the stack. */
> >> #define PTO STATE_SAVE_PT_OFFSET
> >>
> >>+/* Standard way of enabling and disabling interrupts in entry.S */
> >>+#ifdef CONFIG_MICROBLAZE_MSRSETCLR
> >>+#define ENTRY_CLI(scratch_reg) \
> >>+ msrclr scratch_reg, 0x2;
> >>+
> >>+#define ENTRY_EI(scratch_reg) \
> >>+ msrset scratch_reg, 0x2;
> >>+#else
> >>+#define ENTRY_CLI(scratch_reg) \
> >>+ mfs scratch_reg, rmsr; \
> >>+ andi scratch_reg, scratch_reg, ~2; \
> >>+ mts rmsr, scratch_reg;
> >>+
> >>+#define ENTRY_EI(scratch_reg) \
> >>+ mfs scratch_reg, rmsr; \
> >>+ ori scratch_reg, scratch_reg, 2; \
> >>+ mts rmsr, scratch_reg;
> >>+#endif
> >>+
> >> /* Standard macros to save and restore regs in the pt_regs struct pointed
> >> to by the stack pointer (r1) */
> >> #define SAVE_REG(reg_num) \
> >>@@ -469,9 +488,7 @@
> >> the C compiler are restored (that is, R1(sp), R2(gp), R15(lp), and
> >> anything restored by EXTRA_STATE_RESTORER). */
> >> #define RETURN(type) \
> >>- mfs r11, rmsr; /* Disable interrupts */ \
> >>- andi r11, r11, ~2; \
> >>- mts rmsr, r11; \
> >>+ ENTRY_CLI(r11); \
> >> lwi r11, r1, PTO+PT_KERNEL_MODE; \
> >> bnei r11, 2f; /* See if returning to kernel mode, */\
> >> /* ... if so, skip resched &c. */ \
> >>@@ -511,9 +528,7 @@
> >> 3: SAVE_EXTRA_STATE_FOR_FUNCALL(type); /* Prepare for funcall. */ \
> >> bralid r15, CSYM(schedule); /* Call scheduler */ \
> >> nop; /* delay slot */ \
> >>- mfs r11, rmsr; /* The scheduler enables interrupts */ \
> >>- andi r11, r11, ~2; \
> >>- mts rmsr, r11; \
> >>+ ENTRY_CLI(r11); /* The scheduler enables interrupts */ \
> >> RESTORE_EXTRA_STATE_FOR_FUNCALL(type); \
> >> brid 5b; /* Back to continue return processing */ \
> >> nop; \
> >>@@ -532,9 +547,7 @@
> >> add r6, r0, r0; /* Arg 2: sigset_t *oldset */ \
> >> bralid r15, CSYM(do_signal); /* Handle any signals */ \
> >> nop; \
> >>- mfs r11, rmsr; /* Sig handling enables interrupts */ \
> >>- andi r11, r11, ~2; \
> >>- mts rmsr, r11; \
> >>+ ENTRY_CLI(r11); /* Sig handling enables interrupts */ \
> >> RESTORE_EXTRA_STATE(type); /* Restore extra regs. */ \
> >> brid 1b; \
> >> nop;
> >>@@ -584,16 +597,12 @@ CSYM(_shift_temp_loc):
> >> * Return value in r3
> >> */
> >> G_ENTRY(trap):
> >>- mfs r11, rmsr; // disable interrupts
> >>- andi r11, r11, ~2;
> >>- mts rmsr, r11;
> >>+ ENTRY_CLI(r11);
> >> swi r1, r0, ENTRY_SP; // save stack (emulate v850)
> >> SAVE_STATE (TRAP, r12, ENTRY_SP) // Save registers.
> >> // No need to enable intrs here because microblaze
> >> // traps just implemented as a branch, not a hardware trap
> >>- mfs r11, rmsr; // Enable interrupts
> >>- ori r11, r11, 2;
> >>- mts rmsr, r11;
> >>+ ENTRY_EI(r11);
> >> la r15, r0, ret_from_trap-8// where the trap should return
> >> // need -8 to adjust for rtsd r15, 8
> >>
> >>@@ -813,10 +822,7 @@ END(nmi0)
> >> G_ENTRY(illegal_instruction):
> >> swi r1, r0, ENTRY_SP; // Save stack (emulate v850)
> >> SAVE_STATE (IRQ, r0, ENTRY_SP) // Save registers.
> >>- mfs r5, rmsr; // Enable interrupts
> >>- ori r5, r5, 2;
> >>- mts rmsr, r5;
> >>-
> >>+ ENTRY_EI(r11);
> >> addi r5, r0, SIGILL; // Arg 0: signal number
> >> RETRIEVE_CURRENT_TASK(r6); // Arg 1: task
> >> la r15, r0, ret_from_irq-8 // where the handler should return
> >>@@ -836,14 +842,10 @@ G_ENTRY(dbtrap):
> >> rtbd r16, dbtrap_o;
> >> nop;
> >> dbtrap_o:
> >>- mfs r11, rmsr; // disable interrupts
> >>- andi r11, r11, ~2;
> >>- mts rmsr, r11;
> >>+ ENTRY_CLI(r11);
> >> swi r1, r0, ENTRY_SP; // Save stack (emulate v850)
> >> SAVE_STATE (DBTRAP, r0, ENTRY_SP)// Save registers.
> >>- mfs r11, rmsr; // Enable interrupts
> >>- ori r11, r11, 2;
> >>- mts rmsr, r11;
> >>+ ENTRY_EI(r11);
> >>
> >> // Must insert code to detect illegal traps etc
> >>
> >>Index: arch/microblaze/config.in
> >>===================================================================
> >>RCS file: /var/cvs/uClinux-2.4.x/arch/microblaze/config.in,v
> >>retrieving revision 1.10
> >>diff -u -b -B -w -p -r1.10 config.in
> >>--- arch/microblaze/config.in 12 Mar 2004 05:27:26 -0000 1.10
> >>+++ arch/microblaze/config.in 18 May 2004 02:14:52 -0000
> >>@@ -51,6 +51,7 @@ mainmenu_option next_comment
> >> #### Microblaze processor-specific config
> >>
> >> #bool 'Reset Guard' CONFIG_RESET_GUARD
> >>+ bool 'Use msrset/msrclr insns' CONFIG_MICROBLAZE_MSRSETCLR
> >> bool 'Hardware Multiplier' CONFIG_MICROBLAZE_HARD_MULT
> >> bool 'Hardware Divider' CONFIG_MICROBLAZE_HARD_DIV
> >> bool 'Hardware Barrel Shift' CONFIG_MICROBLAZE_HARD_BARREL
> >>
> >>
>
>
> ___________________________
> microblaze-uclinux mailing list
> microblaze-uclinux@itee.uq.edu.au
> Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
> Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/
--
(C) Matthew Rubenstein
___________________________
microblaze-uclinux mailing list
microblaze-uclinux@itee.uq.edu.au
Project Home Page : http://www.itee.uq.edu.au/~jwilliams/mblaze-uclinux
Mailing List Archive : http://www.itee.uq.edu.au/~listarch/microblaze-uclinux/