Skip to content

Conversation

@sktt
Copy link
Contributor

@sktt sktt commented Jul 21, 2023

No description provided.

@sktt sktt requested a review from a team as a code owner July 21, 2023 08:45
@sktt sktt force-pushed the bug/syscall_return branch from 7a0249b to 7be5644 Compare July 21, 2023 09:09
The previous method would get optimized out by compiler. Using asm
volatile instead.

Signed-off-by: Johannes Wikner <[email protected]>
@sktt sktt force-pushed the bug/syscall_return branch from 7be5644 to 32ae36d Compare July 21, 2023 09:13
@wipawel wipawel enabled auto-merge (rebase) July 21, 2023 09:14
@wipawel wipawel merged commit c693e0e into KernelTestFramework:mainline Jul 21, 2023
* GRUB2 bootloader tools - `grub2-common` package (e.g. `apt install grub2-common`)
* ISO generation tools - `xorriso` package (e.g. `apt install xorriso`)
* ISO generation tools - `xorriso` and `mtools` package (e.g. `apt install xorriso mtools`)

Copy link
Contributor

Choose a reason for hiding this comment

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

mtools is needed for creating the ISO via grub-mkrescue? If so, you should mention that in the commit log, at least. However, it should already had been installed via the grub2-common package, as that depends on grub-common which "suggests" mtools. But yeah, it's not a strong dependency, so we may want to make it explicit.

);
/* clang-format on */
return return_code;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

WTF is this?! Why isn't this a simple return return_code?.... Oh, I see. syscall_handler(), what a bummer! It does the right thing only by chance. We'd better rewrite that think with a ASM thunk instead of this fragile register pinning code (gcc is free to reuse these registers, you know ;)).

Copy link
Contributor

Choose a reason for hiding this comment

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

You are right of course, the current implementation of syscall handling is indeed quite fragile. I plan to improve it all at some point.

@minipli-oss
Copy link
Contributor

Oh, heh, I took too long to read and comment, Pawel already merged it :D
Feel free to ignore my mumbling in this case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants