| Message ID | 20220718071825.22113-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/wait: Improvements | expand |
On 18.07.2022 09:18, Andrew Cooper wrote: > @@ -199,9 +211,18 @@ void check_wakeup_from_wait(void) > } > > /* > - * Hand-rolled longjmp(). Returns to __prepare_to_wait(), and lands on a > - * `rep movs` instruction. All other GPRs are restored from the stack, so > - * are available for use here. > + * Hand-rolled longjmp(). > + * > + * check_wakeup_from_wait() is always called with a shallow stack, > + * immediately after the vCPU has been rescheduled. > + * > + * Adjust %rsp to be the correct depth for the (deeper) stack we want to > + * restore, then prepare %rsi, %rdi and %rcx such that when we intercept > + * the rep movs in __prepare_to_wait(), it copies from wqv->stack over the > + * active stack. I'm struggling with the use of "intercept" here, but I guess that's just because I'm not a native speaker. > + * All other GPRs are available for use; they're either restored from > + * wqv->stack or explicitly clobbered. You talking of "other GPRs" - there aren't any which are explicitly clobbered. It's only the previously named ones which are. Hence I'd like to ask that the respective parts of the sentence be dropped. Then Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 18/07/2022 11:00, Jan Beulich wrote: > On 18.07.2022 09:18, Andrew Cooper wrote: >> @@ -199,9 +211,18 @@ void check_wakeup_from_wait(void) >> } >> >> /* >> - * Hand-rolled longjmp(). Returns to __prepare_to_wait(), and lands on a >> - * `rep movs` instruction. All other GPRs are restored from the stack, so >> - * are available for use here. >> + * Hand-rolled longjmp(). >> + * >> + * check_wakeup_from_wait() is always called with a shallow stack, >> + * immediately after the vCPU has been rescheduled. >> + * >> + * Adjust %rsp to be the correct depth for the (deeper) stack we want to >> + * restore, then prepare %rsi, %rdi and %rcx such that when we intercept >> + * the rep movs in __prepare_to_wait(), it copies from wqv->stack over the >> + * active stack. > I'm struggling with the use of "intercept" here, but I guess that's just > because I'm not a native speaker. "intercept" is the same terminology used in the middle of __prepare_to_wait()'s block. It's because we have a weird setup where this is (now) a noreturn function merging into the middle of a function which already executed once. I'm happy to change it if it's unclear, but I can't think of a better description. >> + * All other GPRs are available for use; they're either restored from >> + * wqv->stack or explicitly clobbered. > You talking of "other GPRs" - there aren't any which are explicitly > clobbered. It's only the previously named ones which are. Hence I'd like > to ask that the respective parts of the sentence be dropped. Then > Reviewed-by: Jan Beulich <jbeulich@suse.com> It becomes true in the next patch. I'll try and shuffle things. ~Andrew
On 18.07.2022 12:11, Andrew Cooper wrote: > On 18/07/2022 11:00, Jan Beulich wrote: >> On 18.07.2022 09:18, Andrew Cooper wrote: >>> @@ -199,9 +211,18 @@ void check_wakeup_from_wait(void) >>> } >>> >>> /* >>> - * Hand-rolled longjmp(). Returns to __prepare_to_wait(), and lands on a >>> - * `rep movs` instruction. All other GPRs are restored from the stack, so >>> - * are available for use here. >>> + * Hand-rolled longjmp(). >>> + * >>> + * check_wakeup_from_wait() is always called with a shallow stack, >>> + * immediately after the vCPU has been rescheduled. >>> + * >>> + * Adjust %rsp to be the correct depth for the (deeper) stack we want to >>> + * restore, then prepare %rsi, %rdi and %rcx such that when we intercept >>> + * the rep movs in __prepare_to_wait(), it copies from wqv->stack over the >>> + * active stack. >> I'm struggling with the use of "intercept" here, but I guess that's just >> because I'm not a native speaker. > > "intercept" is the same terminology used in the middle of > __prepare_to_wait()'s block. > > It's because we have a weird setup where this is (now) a noreturn > function merging into the middle of a function which already executed once. > > I'm happy to change it if it's unclear, but I can't think of a better > description. "... when we go back to ..."? (But I'm not meaning to insist on a wording change.) Jan
diff --git a/xen/common/wait.c b/xen/common/wait.c index 3ebb884fe738..4dcfa17a8a3f 100644 --- a/xen/common/wait.c +++ b/xen/common/wait.c @@ -137,7 +137,19 @@ static void __prepare_to_wait(struct waitqueue_vcpu *wqv) do_softirq(); } - /* Hand-rolled setjmp(). */ + /* + * Hand-rolled setjmp(). + * + * __prepare_to_wait() is the leaf of a deep calltree. Preserve the GPRs, + * bounds check what we want to stash in wqv->stack, copy the active stack + * (up to cpu_info) into wqv->stack, then return normally. Our caller + * will shortly schedule() and discard the current context. + * + * The copy out is performed with a rep movsb. When + * check_wakeup_from_wait() longjmp()'s back into us, %rsp is pre-adjusted + * to be suitable and %rsi/%rdi are swapped, so the rep movsb instead + * copies in from wqv->stack over the active stack. + */ asm volatile ( "push %%rax; push %%rbx; push %%rdx; push %%rbp;" "push %%r8; push %%r9; push %%r10; push %%r11;" @@ -199,9 +211,18 @@ void check_wakeup_from_wait(void) } /* - * Hand-rolled longjmp(). Returns to __prepare_to_wait(), and lands on a - * `rep movs` instruction. All other GPRs are restored from the stack, so - * are available for use here. + * Hand-rolled longjmp(). + * + * check_wakeup_from_wait() is always called with a shallow stack, + * immediately after the vCPU has been rescheduled. + * + * Adjust %rsp to be the correct depth for the (deeper) stack we want to + * restore, then prepare %rsi, %rdi and %rcx such that when we intercept + * the rep movs in __prepare_to_wait(), it copies from wqv->stack over the + * active stack. + * + * All other GPRs are available for use; they're either restored from + * wqv->stack or explicitly clobbered. */ asm volatile ( "mov %1,%%"__OP"sp; jmp .L_wq_resume;"
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> --- xen/common/wait.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)