Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{lib,arch,plat}: Redo syscall ctx's and swapgs logic #1346

Merged

Conversation

mogasergiu
Copy link
Member

@mogasergiu mogasergiu commented Mar 4, 2024

Prerequisite checklist

  • Read the contribution guidelines regarding submitting new changes to the project;
  • Tested your changes against relevant architectures and platforms;
  • Ran the checkpatch.uk on your commit series before opening this PR;
  • Updated relevant documentation.

Base target

  • Architecture(s): [e.g. x86_64 or N/A]
  • Platform(s): [e.g. kvm, xen or N/A]
  • Application(s): [e.g. app-python3 or N/A]

Additional configuration

Description of changes

To make git bisecting and rebasing significantly easier and avoid
builds breaking across commits, this whole set of changes shall be
introduced under one single all encompassing commit.

Following the introduction of the concept of auxiliary stack pointers,
swapgs, struct uk_syscall_ctx and struct ukarch_sysregs, a number
of things have emerged:

  • the aforemenetioned structs are very generic so they should be moved
    under libcontext (arch/)
  • swapgs introduces a significant inconsistency between ARM64 and x86_64
    as we never know during an exception the state of
    MSR_GS_BASE/MSR_KERNEL_GS_BASE
  • auxiliary stack pointers have increased flexibility as every thread
    and LCPU can have one and have private data stored in there than may
    be accessed anytime, dependency free

Thus, this commit does the following:

  1. Move/rename aforementioned structured to libcontext and document them
  • lib/syscall_shim/arch/x86_64/sysregs.c -> arch/x86/sysctx.c
  • lib/syscall_shim/arch/x86_64include/arch/sysregs.h -> arch/x86/x86_64include/uk/asm/sysctx.h
  • s/struct ukarch_sysregs/struct ukarch_sysctx/ (and all related defs)
  • struct uk_syscall_ctx from lib/syscall_shim/include/uk/syscall.h to
    include/uk/arch/ctx.h as struct ukarch_execenv
  • s/struct uk_syscall_ctx/struct ukarch_execenv/ (and all related defs)
  • actually comment these functions
  • re-adjust all places that make use of such definitions
  1. Get rid of the swapgs, architecture specific holdback by exploiting
    the flexibility of auxiliary stacks through the introduction of a new
    always existing contrl block at their top end:
  • introduce struct ukarch_auxspcb under libcontext
  • add Unikraft system context as field to it so that we always have and
    know Unikraft TLS (and LCPU in case ox x86_64) in a dependency free
    and assumption free manner
  • add a current frame pointer field: since the auxspscb will be part of
    the auxiliary stack, we need to know the safe place where we can start
    using the auxiliary stack area as a stack (this is also helpful in cases
    where we need to nest on the auxstack)
    -for the aforementione fields/structs, init/getter/setter functions have
    been added and documented
  • now the swapgs pair will only be done very early during system call
    entry (and only there, not on clone child exit anymore either) just
    enough so that we, first things first, switch to auxstack and push auxsp
    so that on entry to C handler we will know that we must do a call to
    ukarch_sysctx_load on the Unikraft sysctx we can get from the pushed
    auxsp (another benefit of this is we get rid of MSR read/writes)

@mogasergiu mogasergiu requested review from a team as code owners March 4, 2024 13:29
@github-actions github-actions bot added arch/arm arch/arm64 arch/x86_64 area/arch Unikraft Architecture area/include Part of include/uk area/lib Internal Unikraft Microlibrary area/plat Unikraft Patform lang/c Issues or PRs to do with C/C++ lib/posix-process Information about system parameters lib/syscall_shim lib/ukboot lib/uksched lib/ukschedcoop plat/common Common to all platforms plat/kvm Unikraft for KVM labels Mar 4, 2024
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Mar 4, 2024
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Mar 5, 2024
mogasergiu added a commit to mogasergiu/unikraft that referenced this pull request Mar 5, 2024
@mogasergiu mogasergiu force-pushed the smoga/#1346/redo_syscall_ctxs branch from c9dddeb to 3d9cd61 Compare March 5, 2024 08:33
@razvand razvand removed the request for review from a team March 17, 2024 18:15
@mogasergiu mogasergiu requested a review from a team as a code owner May 23, 2024 15:32
@github-actions github-actions bot added the plat/xen Unikraft for Xen label May 23, 2024
@mogasergiu mogasergiu requested a review from skuenzer May 23, 2024 15:33
@mogasergiu mogasergiu force-pushed the smoga/#1346/redo_syscall_ctxs branch from 45c9a27 to 4da8c3e Compare May 23, 2024 15:49
@razvand razvand removed the request for review from a team May 24, 2024 07:41
@mogasergiu mogasergiu force-pushed the smoga/#1346/redo_syscall_ctxs branch 2 times, most recently from 22cf957 to 802e89f Compare May 24, 2024 20:12
include/uk/arch/ctx.h Show resolved Hide resolved
include/uk/arch/ctx.h Show resolved Hide resolved
include/uk/arch/ctx.h Outdated Show resolved Hide resolved
lib/posix-process/arch/arm64/clone.c Outdated Show resolved Hide resolved
plat/common/x86/syscall.S Outdated Show resolved Hide resolved
plat/common/x86/syscall.S Outdated Show resolved Hide resolved
plat/common/x86/syscall.S Outdated Show resolved Hide resolved
plat/kvm/arm/exceptions.S Outdated Show resolved Hide resolved
plat/kvm/arm/lcpu.c Show resolved Hide resolved
lib/syscall_shim/include/uk/syscall.h Outdated Show resolved Hide resolved
@mogasergiu mogasergiu force-pushed the smoga/#1346/redo_syscall_ctxs branch 3 times, most recently from a107b9e to e27551c Compare May 26, 2024 11:40
@mogasergiu mogasergiu requested a review from skuenzer May 26, 2024 11:53
@mogasergiu mogasergiu force-pushed the smoga/#1346/redo_syscall_ctxs branch from e27551c to 8c92c37 Compare May 26, 2024 12:48
Copy link
Member

@skuenzer skuenzer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot for your work. As discussed, we will later restructure libcontext and your additions to it with plat rearch and the lcpu library in particular.

Reviewed-by: Simon Kuenzer simon.kuenzer@unikraft.io

@michpappas I am handing over to you.

@michpappas michpappas mentioned this pull request May 27, 2024
4 tasks
To make git bisecting and rebasing significantly easier and avoid
builds breaking across commits, this whole set of changes shall be
introduced under one single all encompassing commit.

Following the introduction of the concept of auxiliary stack pointers,
swapgs, `struct uk_syscall_ctx` and `struct ukarch_sysregs`, a number
of things have emerged:
- the aforemenetioned structs are very generic so they should be moved
under libcontext (arch/)
- swapgs introduces a significant inconsistency between ARM64 and x86_64
as we never know during an exception the state of
MSR_GS_BASE/MSR_KERNEL_GS_BASE
- auxiliary stack pointers  have increased flexibility as every thread
and LCPU can have one and have private data stored in there than may
be accessed anytime, dependency free

Thus, this commit does the following:
1. Move/rename aforementioned structured to libcontext and document them
- lib/syscall_shim/arch/x86_64/sysregs.c -> arch/x86/sysctx.c
- lib/syscall_shim/arch/x86_64include/arch/sysregs.h -> arch/x86/x86_64include/uk/asm/sysctx.h
- s/struct ukarch_sysregs/struct ukarch_sysctx/ (and all related defs)
- struct uk_syscall_ctx from lib/syscall_shim/include/uk/syscall.h to
include/uk/arch/ctx.h as struct ukarch_execenv
- s/struct uk_syscall_ctx/struct ukarch_execenv/ (and all related defs)
- actually comment these functions
- re-adjust all places that make use of such definitions

2. Get rid of the `swapgs`, architecture specific holdback by exploiting
the flexibility of auxiliary stacks through the introduction of a new
always existing contrl block at their top end:
- introduce `struct ukarch_auxspcb` under libcontext
- add Unikraft system context as field to it so that we always have and
know Unikraft TLS (and LCPU in case ox x86_64) in a dependency free
and assumption free manner
- add a current frame pointer field: since the auxspscb will be part of
the auxiliary stack, we need to know the safe place where we can start
using the auxiliary stack area as a stack (this is also helpful in cases
where we need to nest on the auxstack)
-for the aforementione fields/structs, init/getter/setter functions have
been added and documented
- now the `swapgs` pair will only be done very early during system call
entry (and only there, not on clone child exit anymore either) just
enough so that we, first things first, switch to auxstack and push auxsp
so that on entry to C handler we will know that we must do a call to
`ukarch_sysctx_load` on the Unikraft sysctx we can get from the pushed
auxsp (another benefit of this is we get rid of MSR read/writes)

IMPORTANT NOTE: Additionally, some minor fixes have been made:
- Do not switch stack pointer to execenv pointer (previously
known as uk_syscall_ctx) during execenv loading as this implies that
functions such as `ukarch_ectx_load` or `ukarch_sysctx_load` would reuse
the space after the execenv as stack. While this is safe if the
execenv was passed through the stack, is definetely not safe if it was
passed through something like a heap buffer that may be bounded to the
execenv size by the caller. Instead, use one of the callee-saved
registers
- Set IRQ flag of the pushed flags of the caller during system call
early assembly entry (both native and binary for both architectures)
so that we don't have to explicitly set it during something like clone
child creation. This also reflects the reality better as no syscall
caller will have IRQ's disabled.
- Do not use spsr_el1, esr_el1 and elr_el1 during native system call
assembly prologue (UK_SYSCALL_EXECENV_PROLOGUE_DEFINE) on Arm, as they
are invalid because there is no actual SVC/exception happening. Instead,
try to emulate it by manually building sane values for them on the
created execenv to replicate an actual SVC while benefitting from not
dealing with the performance impacting flow of actually taking a SVC.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Some macros of `essentials.h` can be used in assembly sources as well,
while others not so much. Allow one to safely include and use the former
by adding assembly guards in the header for the macros in question

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Move the Linux ABI defined syscall architecture specific calling
convention definitions to `uk/bits/`.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Include `legacy_syscall.h` header as if it is part of the inclusion
search path, which it is indeed.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
@mogasergiu mogasergiu force-pushed the smoga/#1346/redo_syscall_ctxs branch from 8c92c37 to 0e82448 Compare May 31, 2024 06:58
@michpappas
Copy link
Member

With all @razvand's tests passing,

Approved-by: Michalis Pappas michalis@unikraft.io

@michpappas michpappas added the merge Label to trigger merge action label May 31, 2024
@unikraft-bot unikraft-bot changed the base branch from staging to staging-1346 May 31, 2024 13:45
@unikraft-bot unikraft-bot merged commit a948a1b into unikraft:staging-1346 May 31, 2024
14 of 15 checks passed
unikraft-bot pushed a commit that referenced this pull request May 31, 2024
To make git bisecting and rebasing significantly easier and avoid
builds breaking across commits, this whole set of changes shall be
introduced under one single all encompassing commit.

Following the introduction of the concept of auxiliary stack pointers,
swapgs, `struct uk_syscall_ctx` and `struct ukarch_sysregs`, a number
of things have emerged:
- the aforemenetioned structs are very generic so they should be moved
under libcontext (arch/)
- swapgs introduces a significant inconsistency between ARM64 and x86_64
as we never know during an exception the state of
MSR_GS_BASE/MSR_KERNEL_GS_BASE
- auxiliary stack pointers  have increased flexibility as every thread
and LCPU can have one and have private data stored in there than may
be accessed anytime, dependency free

Thus, this commit does the following:
1. Move/rename aforementioned structured to libcontext and document them
- lib/syscall_shim/arch/x86_64/sysregs.c -> arch/x86/sysctx.c
- lib/syscall_shim/arch/x86_64include/arch/sysregs.h -> arch/x86/x86_64include/uk/asm/sysctx.h
- s/struct ukarch_sysregs/struct ukarch_sysctx/ (and all related defs)
- struct uk_syscall_ctx from lib/syscall_shim/include/uk/syscall.h to
include/uk/arch/ctx.h as struct ukarch_execenv
- s/struct uk_syscall_ctx/struct ukarch_execenv/ (and all related defs)
- actually comment these functions
- re-adjust all places that make use of such definitions

2. Get rid of the `swapgs`, architecture specific holdback by exploiting
the flexibility of auxiliary stacks through the introduction of a new
always existing contrl block at their top end:
- introduce `struct ukarch_auxspcb` under libcontext
- add Unikraft system context as field to it so that we always have and
know Unikraft TLS (and LCPU in case ox x86_64) in a dependency free
and assumption free manner
- add a current frame pointer field: since the auxspscb will be part of
the auxiliary stack, we need to know the safe place where we can start
using the auxiliary stack area as a stack (this is also helpful in cases
where we need to nest on the auxstack)
-for the aforementione fields/structs, init/getter/setter functions have
been added and documented
- now the `swapgs` pair will only be done very early during system call
entry (and only there, not on clone child exit anymore either) just
enough so that we, first things first, switch to auxstack and push auxsp
so that on entry to C handler we will know that we must do a call to
`ukarch_sysctx_load` on the Unikraft sysctx we can get from the pushed
auxsp (another benefit of this is we get rid of MSR read/writes)

IMPORTANT NOTE: Additionally, some minor fixes have been made:
- Do not switch stack pointer to execenv pointer (previously
known as uk_syscall_ctx) during execenv loading as this implies that
functions such as `ukarch_ectx_load` or `ukarch_sysctx_load` would reuse
the space after the execenv as stack. While this is safe if the
execenv was passed through the stack, is definetely not safe if it was
passed through something like a heap buffer that may be bounded to the
execenv size by the caller. Instead, use one of the callee-saved
registers
- Set IRQ flag of the pushed flags of the caller during system call
early assembly entry (both native and binary for both architectures)
so that we don't have to explicitly set it during something like clone
child creation. This also reflects the reality better as no syscall
caller will have IRQ's disabled.
- Do not use spsr_el1, esr_el1 and elr_el1 during native system call
assembly prologue (UK_SYSCALL_EXECENV_PROLOGUE_DEFINE) on Arm, as they
are invalid because there is no actual SVC/exception happening. Instead,
try to emulate it by manually building sane values for them on the
created execenv to replicate an actual SVC while benefitting from not
dealing with the performance impacting flow of actually taking a SVC.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Simon Kuenzer <simon.kuenzer@unikraft.io>
GitHub-Closes: #1346
unikraft-bot pushed a commit that referenced this pull request May 31, 2024
Some macros of `essentials.h` can be used in assembly sources as well,
while others not so much. Allow one to safely include and use the former
by adding assembly guards in the header for the macros in question

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Simon Kuenzer <simon.kuenzer@unikraft.io>
GitHub-Closes: #1346
unikraft-bot pushed a commit that referenced this pull request May 31, 2024
Move the Linux ABI defined syscall architecture specific calling
convention definitions to `uk/bits/`.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Simon Kuenzer <simon.kuenzer@unikraft.io>
GitHub-Closes: #1346
unikraft-bot pushed a commit that referenced this pull request May 31, 2024
Include `legacy_syscall.h` header as if it is part of the inclusion
search path, which it is indeed.

Signed-off-by: Sergiu Moga <sergiu@unikraft.io>
Approved-by: Michalis Pappas <michalis@unikraft.io>
Reviewed-by: Simon Kuenzer <simon.kuenzer@unikraft.io>
GitHub-Closes: #1346
@unikraft-bot unikraft-bot added ci/merged Merged by CI and removed merge Label to trigger merge action labels May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch/arm arch/arm64 arch/x86_64 area/arch Unikraft Architecture area/include Part of include/uk area/lib Internal Unikraft Microlibrary area/plat Unikraft Patform area/support Support scripts, tools, services. ci/merged Merged by CI lang/c Issues or PRs to do with C/C++ lib/posix-process Information about system parameters lib/syscall_shim lib/ukboot lib/uksched lib/ukschedcoop plat/common Common to all platforms plat/kvm Unikraft for KVM plat/xen Unikraft for Xen
Projects
Status: Done
Status: Done!
Development

Successfully merging this pull request may close these issues.

5 participants