On 28/02/16(Sun) 17:34, Martin Pieuchot wrote: > On 28/02/16(Sun) 17:02, Mike Belopuhov wrote: > > On 28 February 2016 at 14:38, Martin Pieuchot <[email protected]> wrote: > > > In order to dynamically instrument kernel functions, I plan to add > > > breakpoints where a probe needs to be executed. Trap handlers will > > > be modified to check if the address of the trapping instruction > > > correspond to a registered probe, and if that's the case, the kernel > > > will execute the associated code. > > > > > > While implementing such mechanism for amd64 and i386, I discovered > > > some differences in the existing code that only history seems to justify. > > > > > > So the diff below get rids of the BPTTRAP() and other small differences > > > around 'calltrap' for these architectures. In particular interrupts > > > are now enabled unconditionally on i386 before entering trap(). Any > > > counter indication for doing so? > > > > > > > That 'sti' that you're adding might have side effects, but I can't think of > > any > > bad ones. You'll get some funny 'sti', 'sti' sequences though since some > > of the callers do 'sti' themselves (e.g resume_iret). > > > > > I wonder if I should split i386's locore.s to match amd64's vector.S... > > > > > > > Technically i386 has apicvec.s which is what vectror.S on amd64 > > since the arch is specified to always have an APIC. Splitting it into > > trap.S is another possibility, but I'm not sure it's worth it. Possibly > > moving actual APIC vector stubs to C is a better plan and then > > vector.s will be up for the grabs. > > > > > Comments, ok? > > > > > > > Honestly, I'd like the alltraps/sti bit to be handled separately even > > if it's just for the purpose of sync'ing up two implementations. > > Something like that first?
Any ok so we can start with the "interesting" part? Or are you guys hesitating because you want a trap.S first? > Index: amd64/amd64/vector.S > =================================================================== > RCS file: /cvs/src/sys/arch/amd64/amd64/vector.S,v > retrieving revision 1.44 > diff -u -p -r1.44 vector.S > --- amd64/amd64/vector.S 8 Dec 2015 19:45:55 -0000 1.44 > +++ amd64/amd64/vector.S 28 Feb 2016 13:12:56 -0000 > @@ -96,24 +96,22 @@ > * (possibly the next clock tick). Thus, we disable interrupt before > checking, > * and only enable them again on the final `iret' or before calling the AST > * handler. > - */ > + */ > > > /*****************************************************************************/ > > #define TRAP(a) pushq $(a) ; jmp _C_LABEL(alltraps) > #define ZTRAP(a) pushq $0 ; TRAP(a) > > -#define BPTTRAP(a) ZTRAP(a) > - > .text > IDTVEC(trap00) > ZTRAP(T_DIVIDE) > IDTVEC(trap01) > - BPTTRAP(T_TRCTRAP) > + ZTRAP(T_TRCTRAP) > IDTVEC(trap02) > ZTRAP(T_NMI) > IDTVEC(trap03) > - BPTTRAP(T_BPTFLT) > + ZTRAP(T_BPTFLT) > IDTVEC(trap04) > ZTRAP(T_OFLOW) > IDTVEC(trap05) > @@ -245,7 +243,6 @@ NENTRY(resume_iret) > NENTRY(alltraps) > INTRENTRY > sti > - .globl calltrap > calltrap: > cld > #ifdef DIAGNOSTIC > Index: i386/i386/locore.s > =================================================================== > RCS file: /cvs/src/sys/arch/i386/i386/locore.s,v > retrieving revision 1.163 > diff -u -p -r1.163 locore.s > --- i386/i386/locore.s 26 Feb 2016 02:25:09 -0000 1.163 > +++ i386/i386/locore.s 28 Feb 2016 16:25:34 -0000 > @@ -1308,14 +1308,14 @@ ENTRY(savectx) > * handler. > * > * XXX - debugger traps are now interrupt gates so at least bdb doesn't lose > - * control. The sti's give the standard losing behaviour for ddb and kgdb. > + * control. STI give the standard losing behaviour for ddb and kgdb. > */ > #define IDTVEC(name) ALIGN_TEXT; .globl X##name; X##name: > > #define TRAP(a) pushl $(a) ; jmp _C_LABEL(alltraps) > #define ZTRAP(a) pushl $0 ; TRAP(a) > -#define BPTTRAP(a) testb $(PSL_I>>8),13(%esp) ; jz 1f ; sti ; 1: ; > \ > - TRAP(a) > +#define STI testb $(PSL_I>>8),13(%esp) ; jz 1f ; sti ; 1: ; > + > > .text > IDTVEC(div) > @@ -1328,12 +1328,13 @@ IDTVEC(dbg) > andb $~0xf,%al > movl %eax,%dr6 > popl %eax > - BPTTRAP(T_TRCTRAP) > + STI > + TRAP(T_TRCTRAP) > IDTVEC(nmi) > ZTRAP(T_NMI) > IDTVEC(bpt) > - pushl $0 > - BPTTRAP(T_BPTFLT) > + STI > + ZTRAP(T_BPTFLT) > IDTVEC(ofl) > ZTRAP(T_OFLOW) > IDTVEC(bnd) > @@ -1447,6 +1448,10 @@ NENTRY(resume_pop_fs) > sti > jmp calltrap > > +/* > + * All traps go through here. Call the generic trap handler, and > + * check for ASTs afterwards. > + */ > NENTRY(alltraps) > INTRENTRY > calltrap: >
