From 09432a82412357421b2a1247001e4c919e954777 Mon Sep 17 00:00:00 2001 From: Ben Wiederhake Date: Sat, 23 Oct 2021 17:31:00 +0200 Subject: [PATCH] Kernel: Separate panic behavior from bootmode Bootmode used to control panic behavior and SystemServer. This patch factors panic behavior control into a separate flag. --- .github/workflows/cmake.yml | 2 +- Base/usr/share/man/man7/boot_parameters.md | 2 ++ Documentation/RunningTests.md | 3 ++- Kernel/CommandLine.cpp | 18 +++++++++++++++++- Kernel/CommandLine.h | 6 ++++++ Kernel/Panic.cpp | 8 ++++++-- Meta/Azure/Serenity.yml | 2 +- Meta/serenity.sh | 2 ++ 8 files changed, 37 insertions(+), 6 deletions(-) diff --git a/.github/workflows/cmake.yml b/.github/workflows/cmake.yml index c7f898170c67a5..1a8960be676bfd 100644 --- a/.github/workflows/cmake.yml +++ b/.github/workflows/cmake.yml @@ -183,7 +183,7 @@ jobs: working-directory: ${{ github.workspace }}/Build/${{ matrix.arch }} env: SERENITY_QEMU_CPU: "max,vmx=off" - SERENITY_KERNEL_CMDLINE: "fbdev=off boot_mode=self-test" + SERENITY_KERNEL_CMDLINE: "fbdev=off panic=shutdown boot_mode=self-test" SERENITY_RUN: "ci" run: | echo "::group::ninja run # Qemu output" diff --git a/Base/usr/share/man/man7/boot_parameters.md b/Base/usr/share/man/man7/boot_parameters.md index 74fb84908498f3..20a9ad71d0b736 100644 --- a/Base/usr/share/man/man7/boot_parameters.md +++ b/Base/usr/share/man/man7/boot_parameters.md @@ -51,6 +51,8 @@ List of options: * **`init_args`** - This parameter expects a set of arguments to pass to the **`init`** program. The value should be a set of strings separated by `,` characters. +* **`panic`** - This parameter expects **`halt`** or **`shutdown`**. This is particularly useful in CI contexts. + * **`pci_ecam`** - This parameter expects **`on`** or **`off`**. * **`root`** - This parameter configures the device to use as the root file system. It defaults to **`/dev/hda`** if unspecified. diff --git a/Documentation/RunningTests.md b/Documentation/RunningTests.md index 01d7aa058e4b51..07e8ece69e6512 100644 --- a/Documentation/RunningTests.md +++ b/Documentation/RunningTests.md @@ -110,7 +110,8 @@ the serial debug output to `./debug.log` so that both stdout of the tests and th captured. To run with CI's TestRunner system server entry, SerenityOS needs booted in self-test mode. Running the following shell -lines will boot SerenityOS in self-test mode, run tests, and exit. +lines will boot SerenityOS in self-test mode, run tests, and exit. Note that CI also sets `panic=shutdown` to terminate qemu; +the default value `halt` keeps qemu around, which allows you to inspect the state. ```sh export SERENITY_RUN=ci diff --git a/Kernel/CommandLine.cpp b/Kernel/CommandLine.cpp index 369ffd9027bf72..e35bd9b815d34c 100644 --- a/Kernel/CommandLine.cpp +++ b/Kernel/CommandLine.cpp @@ -38,8 +38,9 @@ UNMAP_AFTER_INIT void CommandLine::initialize() VERIFY(!s_the); s_the = new CommandLine(s_cmd_line); dmesgln("Kernel Commandline: {}", kernel_command_line().string()); - // Validate the boot mode the user passed in. + // Validate the modes the user passed in. (void)s_the->boot_mode(Validate::Yes); + (void)s_the->panic_mode(Validate::Yes); } UNMAP_AFTER_INIT void CommandLine::build_commandline(const String& cmdline_from_bootloader) @@ -214,6 +215,21 @@ BootMode CommandLine::boot_mode(Validate should_validate) const return BootMode::Unknown; } +PanicMode CommandLine::panic_mode(Validate should_validate) const +{ + const auto panic_mode = lookup("panic"sv).value_or("halt"sv); + if (panic_mode == "halt"sv) { + return PanicMode::Halt; + } else if (panic_mode == "shutdown"sv) { + return PanicMode::Shutdown; + } + + if (should_validate == Validate::Yes) + PANIC("Unknown PanicMode: {}", panic_mode); + + return PanicMode::Halt; +} + UNMAP_AFTER_INIT bool CommandLine::are_framebuffer_devices_enabled() const { return lookup("fbdev"sv).value_or("on"sv) == "on"sv; diff --git a/Kernel/CommandLine.h b/Kernel/CommandLine.h index 27afe7958878b7..34d4f64e53897e 100644 --- a/Kernel/CommandLine.h +++ b/Kernel/CommandLine.h @@ -22,6 +22,11 @@ enum class BootMode { Unknown, }; +enum class PanicMode { + Halt, + Shutdown, +}; + enum class HPETMode { Periodic, NonPeriodic @@ -70,6 +75,7 @@ class CommandLine { [[nodiscard]] bool is_force_pio() const; [[nodiscard]] AcpiFeatureLevel acpi_feature_level() const; [[nodiscard]] BootMode boot_mode(Validate should_validate = Validate::No) const; + [[nodiscard]] PanicMode panic_mode(Validate should_validate = Validate::No) const; [[nodiscard]] HPETMode hpet_mode() const; [[nodiscard]] bool disable_physical_storage() const; [[nodiscard]] bool disable_ps2_controller() const; diff --git a/Kernel/Panic.cpp b/Kernel/Panic.cpp index b4fe667425fd22..c6a51221442bbf 100644 --- a/Kernel/Panic.cpp +++ b/Kernel/Panic.cpp @@ -33,9 +33,13 @@ void __panic(const char* file, unsigned int line, const char* function) critical_dmesgln("at {}:{} in {}", file, line, function); dump_backtrace(PrintToScreen::Yes); - if (kernel_command_line().boot_mode() == BootMode::SelfTest) + switch (kernel_command_line().panic_mode()) { + case PanicMode::Shutdown: __shutdown(); - else + case PanicMode::Halt: + [[fallthrough]]; + default: Processor::halt(); + } } } diff --git a/Meta/Azure/Serenity.yml b/Meta/Azure/Serenity.yml index a115f3e3dc7fce..e2bc903a5b02bf 100644 --- a/Meta/Azure/Serenity.yml +++ b/Meta/Azure/Serenity.yml @@ -80,7 +80,7 @@ jobs: timeoutInMinutes: 60 env: SERENITY_QEMU_CPU: 'max,vmx=off' - SERENITY_KERNEL_CMDLINE: 'fbdev=off boot_mode=self-test' + SERENITY_KERNEL_CMDLINE: 'fbdev=off panic=shutdown boot_mode=self-test' SERENITY_RUN: 'ci' - script: | diff --git a/Meta/serenity.sh b/Meta/serenity.sh index 46bd9f54d90319..a5218b343c211a 100755 --- a/Meta/serenity.sh +++ b/Meta/serenity.sh @@ -358,6 +358,8 @@ if [[ "$CMD" =~ ^(build|install|image|copy-src|run|gdb|test|rebuild|recreate|kad else build_target install build_target image + # In contrast to CI, we don't set 'panic=shutdown' here, + # in case the user wants to inspect qemu some more. export SERENITY_KERNEL_CMDLINE="fbdev=off boot_mode=self-test" export SERENITY_RUN="ci" build_target run