Edward Pilatowicz wrote:
On Thu, Dec 17, 2009 at 09:39:34AM -0700, Jerry Jelinek wrote:
Edward Pilatowicz wrote:
- CALC_TABLE_ADDR() a little clunky. (it has seperate 32 and 64
versions, it assumes the syscall number is in eax/rax, and it has a
side effect of munging the syscall number.) how about defining just one
version of CALC_TABLE_ADDR() as:
---8<---
#define CALC_TABLE_ADDR(sysnum, rv)
\
GET_P_BRAND_DATA(%rsp, 1, rv) /* get p_brand_data ptr */;
\
mov SPD_HANDLER(rv), rv /* get p_brand_data->spd_handler ptr
*/;\
shl $4, sysnum /* syscall_num * 16 */;
\
add sysnum, result /* calc JMP table address */;
\
shr $4, sysnum /* syscall_num / 16 */
---8<---
Ed,
I reworked the macros and I think its a lot cleaner now.
Let me know what you think. The new webrev is at:
http://cr.opensolaris.org/~gjelinek/webrev.6768950/
this looks great. things are much simpler and the callbacks are more
similar. :)
i only have one nit. i think the following comment is incorrect:
* To 'return' to our user-space handler, we just need to place its
* address into the user's %ss.
it should read:
* To 'return' to our user-space handler we need to update the
* user's %eip pointer in the saved interrupt state. The
* interrupt state was pushed onto our stack automatically when
* the interrupt occured, see the comments above.
actually, now that i look at it some more...
i think you could make the int91 callback simpler by making it almost
the same as the like the 32-bit syscall callback. after all, the stack
content basically is the same in both call paths...
specifically, you could change the following:
---8<---
/*
* The saved stack pointer points at the state saved when we took
* the interrupt:
...
*/
ENTRY(sn1_brand_int91_callback)
...
movq %r15, %rax /* save new ret addr in %rax */
GET_V(%rsp, 1, V_SSP, %r15) /* Get saved %esp */
xchgq (%r15), %rax /* swap %rax and return addr */
---8<---
to be:
---8<---
/*
* The saved stack pointer (V_SSP) points to the interrupt specific
* state, which is saved directly above the stack contents common to all
* callbacks.
...
*/
#define V_U_SS (V_END + (CLONGSIZE * 4))
#define V_U_ESP (V_END + (CLONGSIZE * 3))
#define V_EFLAGS (V_END + (CLONGSIZE * 2))
#define V_U_CS (V_END + (CLONGSIZE * 1))
#define V_U_EIP (V_END + (CLONGSIZE * 0))
ENTRY(sn1_brand_int91_callback)
...
SET_V(%rsp, 1, V_U_EIP, %r15) /* set user %eip to JMP table addr */
GET_V(%rsp, 1, V_URET_ADDR, %rax) /* save orig return addr in %eax */
---8<---
Ed,
Thanks for the correction on the comment. I also updated the code as
you suggested. I'm not sure if what I have now is better than before
but its the same number of instructions and its more similar to the
the 32-bit code path (although it can't be identical). I posted a new
webrev at:
http://cr.opensolaris.org/~gjelinek/webrev.6768950/
Let me know if you have any other comments.
Thanks,
Jerry
_______________________________________________
zones-discuss mailing list
zones-discuss@opensolaris.org
