Skip to content

Commit

Permalink
Kernel+SystemServer: Change bootmode to system_mode
Browse files Browse the repository at this point in the history
'bootmode' now only controls which set of services are started by
SystemServer, so it is more appropriate to rename it to system_mode, and
no longer validate it in the Kernel.
  • Loading branch information
BenWiederhake authored and awesomekling committed Oct 25, 2021
1 parent 09432a8 commit 8d13f6d
Show file tree
Hide file tree
Showing 11 changed files with 64 additions and 51 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/cmake.yml
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ jobs:
working-directory: ${{ github.workspace }}/Build/${{ matrix.arch }}
env:
SERENITY_QEMU_CPU: "max,vmx=off"
SERENITY_KERNEL_CMDLINE: "fbdev=off panic=shutdown boot_mode=self-test"
SERENITY_KERNEL_CMDLINE: "fbdev=off panic=shutdown system_mode=self-test"
SERENITY_RUN: "ci"
run: |
echo "::group::ninja run # Qemu output"
Expand Down
2 changes: 1 addition & 1 deletion Base/usr/share/man/man5/SystemServer.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describing how to launch and manage this service.
* `SocketPermissions` - comma-separated list of (octal) file system permissions for the socket file. The default permissions are 0600. If the number of socket permissions defined is less than the number of sockets defined, then the last defined permission will be used for the remainder of the items in `Socket`.
* `User` - a name of the user to run the service as. This impacts what UID, GID (and extra GIDs) the service processes have. By default, services are run as root.
* `WorkingDirectory` - the working directory in which the service is spawned. By default, services are spawned in the root (`"/"`) directory.
* `BootModes` - a comma-separated list of boot modes the service should be enabled in. By default, services are only enabled in the "graphical" mode. The current boot mode is read from the kernel command line, and is assumed to be "graphical" if not specified there.
* `BootModes` - a comma-separated list of boot modes the service should be enabled in. By default, services are only enabled in the "graphical" mode. The current system mode is read from the [kernel command line](../man7/boot_parameters.md#options), and is assumed to be "graphical" if not specified there.
* `Environment` - a space-separated list of "variable=value" pairs to set in the environment for the service.
* `MultiInstance` - whether multiple instances of the service can be running simultaneously.
* `AcceptSocketConnections` - whether SystemServer should accept connections on the socket, and spawn an instance of the service for each client connection.
Expand Down
8 changes: 5 additions & 3 deletions Base/usr/share/man/man7/boot_parameters.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,6 @@ List of options:
* **`ahci_reset_mode`** - This parameter expects one of the following values. **`controller`** - Reset just the AHCI controller on boot.
**`none`** - Don't perform any AHCI reset. **`complete`** - Reset the AHCI controller, and all AHCI ports on boot.

* **`boot_mode`** - This parameter expects one of the following values: **`text`** - Boots the system in text only mode.
**`self-test`** - Boots the system in self-test, validation mode. **`graphical`** - Boots the system in the normal graphical mode.

* **`boot_prof`** - If present on the command line, global system profiling will be enabled
as soon as possible during the boot sequence. Allowing you to profile startup of all applications.

Expand Down Expand Up @@ -62,6 +59,11 @@ List of options:
for handling interrupts instead of [PIC](https://en.wikipedia.org/wiki/Programmable_interrupt_controller) mode.
This parameter defaults to **`off`**.

* **`system_mode`** - This parameter is not interpreted by the Kernel, and is made available at `/proc/system_mode`. SystemServer uses it to select the set of services that should be started. Common values are:
- **`graphical`** (default) - Boots the system in the normal graphical mode.
- **`self-test`** - Boots the system in self-test, validation mode.
- **`text`** - Boots the system in text only mode. (You may need to also set **`fbdev=off`**.)

* **`time`** - This parameter expects one of the following values. **`modern`** - This configures the system to attempt
to use High Precision Event Timer (HPET) on boot. **`legacy`** - Configures the system to use the legacy programmable interrupt
time for managing system team.
Expand Down
2 changes: 1 addition & 1 deletion Documentation/RunningTests.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,6 @@ the default value `halt` keeps qemu around, which allows you to inspect the stat

```sh
export SERENITY_RUN=ci
export SERENITY_KERNEL_CMDLINE="fbdev=off boot_mode=self-test"
export SERENITY_KERNEL_CMDLINE="fbdev=off system_mode=self-test"
ninja run
```
22 changes: 8 additions & 14 deletions Kernel/CommandLine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,13 @@ UNMAP_AFTER_INIT void CommandLine::initialize()
s_the = new CommandLine(s_cmd_line);
dmesgln("Kernel Commandline: {}", kernel_command_line().string());
// Validate the modes the user passed in.
(void)s_the->boot_mode(Validate::Yes);
(void)s_the->panic_mode(Validate::Yes);
if (s_the->contains("boot_mode"sv)) {
// I know, we don't do legacy, but even though I eliminated 'boot_mode' from the codebase, there
// is a good chance that someone's still using it. Let's be nice and tell them where to look.
// TODO: Remove this in 2022.
PANIC("'boot_mode' is now split into panic=[halt|shutdown], fbdev=[on|off], and system_mode=[graphical|text|selftest].");
}
}

UNMAP_AFTER_INIT void CommandLine::build_commandline(const String& cmdline_from_bootloader)
Expand Down Expand Up @@ -199,20 +204,9 @@ UNMAP_AFTER_INIT AHCIResetMode CommandLine::ahci_reset_mode() const
PANIC("Unknown AHCIResetMode: {}", ahci_reset_mode);
}

BootMode CommandLine::boot_mode(Validate should_validate) const
StringView CommandLine::system_mode() const
{
const auto boot_mode = lookup("boot_mode"sv).value_or("graphical"sv);
if (boot_mode == "no-fbdev"sv) {
return BootMode::NoFramebufferDevices;
} else if (boot_mode == "self-test"sv) {
return BootMode::SelfTest;
} else if (boot_mode == "graphical"sv) {
return BootMode::Graphical;
}

if (should_validate == Validate::Yes)
PANIC("Unknown BootMode: {}", boot_mode);
return BootMode::Unknown;
return lookup("system_mode"sv).value_or("graphical"sv);
}

PanicMode CommandLine::panic_mode(Validate should_validate) const
Expand Down
9 changes: 1 addition & 8 deletions Kernel/CommandLine.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,6 @@

namespace Kernel {

enum class BootMode {
NoFramebufferDevices,
SelfTest,
Graphical,
Unknown,
};

enum class PanicMode {
Halt,
Shutdown,
Expand Down Expand Up @@ -74,7 +67,7 @@ class CommandLine {
[[nodiscard]] bool are_framebuffer_devices_enabled() const;
[[nodiscard]] bool is_force_pio() const;
[[nodiscard]] AcpiFeatureLevel acpi_feature_level() const;
[[nodiscard]] BootMode boot_mode(Validate should_validate = Validate::No) const;
[[nodiscard]] StringView system_mode() const;
[[nodiscard]] PanicMode panic_mode(Validate should_validate = Validate::No) const;
[[nodiscard]] HPETMode hpet_mode() const;
[[nodiscard]] bool disable_physical_storage() const;
Expand Down
22 changes: 22 additions & 0 deletions Kernel/GlobalProcessExposed.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -706,6 +706,19 @@ class ProcFSCommandLine final : public ProcFSGlobalInformation {
return KSuccess;
}
};
class ProcFSSystemMode final : public ProcFSGlobalInformation {
public:
static NonnullRefPtr<ProcFSSystemMode> must_create();

private:
ProcFSSystemMode();
virtual KResult try_generate(KBufferBuilder& builder) override
{
TRY(builder.append(kernel_command_line().system_mode()));
TRY(builder.append('\n'));
return KSuccess;
}
};

class ProcFSProfile final : public ProcFSGlobalInformation {
public:
Expand Down Expand Up @@ -793,6 +806,10 @@ UNMAP_AFTER_INIT NonnullRefPtr<ProcFSCommandLine> ProcFSCommandLine::must_create
{
return adopt_ref_if_nonnull(new (nothrow) ProcFSCommandLine).release_nonnull();
}
UNMAP_AFTER_INIT NonnullRefPtr<ProcFSSystemMode> ProcFSSystemMode::must_create()
{
return adopt_ref_if_nonnull(new (nothrow) ProcFSSystemMode).release_nonnull();
}
UNMAP_AFTER_INIT NonnullRefPtr<ProcFSProfile> ProcFSProfile::must_create()
{
return adopt_ref_if_nonnull(new (nothrow) ProcFSProfile).release_nonnull();
Expand Down Expand Up @@ -855,6 +872,10 @@ UNMAP_AFTER_INIT ProcFSCommandLine::ProcFSCommandLine()
: ProcFSGlobalInformation("cmdline"sv)
{
}
UNMAP_AFTER_INIT ProcFSSystemMode::ProcFSSystemMode()
: ProcFSGlobalInformation("system_mode"sv)
{
}
UNMAP_AFTER_INIT ProcFSProfile::ProcFSProfile()
: ProcFSGlobalInformation("profile"sv)
{
Expand Down Expand Up @@ -895,6 +916,7 @@ UNMAP_AFTER_INIT NonnullRefPtr<ProcFSRootDirectory> ProcFSRootDirectory::must_cr
directory->m_components.append(ProcFSDevices::must_create());
directory->m_components.append(ProcFSUptime::must_create());
directory->m_components.append(ProcFSCommandLine::must_create());
directory->m_components.append(ProcFSSystemMode::must_create());
directory->m_components.append(ProcFSProfile::must_create());
directory->m_components.append(ProcFSKernelBase::must_create());

Expand Down
2 changes: 1 addition & 1 deletion Meta/Azure/Serenity.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ jobs:
timeoutInMinutes: 60
env:
SERENITY_QEMU_CPU: 'max,vmx=off'
SERENITY_KERNEL_CMDLINE: 'fbdev=off panic=shutdown boot_mode=self-test'
SERENITY_KERNEL_CMDLINE: 'fbdev=off panic=shutdown system_mode=self-test'
SERENITY_RUN: 'ci'
- script: |
Expand Down
4 changes: 2 additions & 2 deletions Meta/serenity.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ Usage: $NAME COMMAND [TARGET] [TOOLCHAIN] [ARGS...]
is specified tests matching it.
All other TARGETs: $NAME test [TARGET]
Runs the built image in QEMU in self-test mode, by passing
boot_mode=self-test to the Kernel
system_mode=self-test to the Kernel
delete: Removes the build environment for TARGET
recreate: Deletes and re-creates the build environment for TARGET
rebuild: Deletes and re-creates the build environment, and compiles for TARGET
Expand Down Expand Up @@ -360,7 +360,7 @@ if [[ "$CMD" =~ ^(build|install|image|copy-src|run|gdb|test|rebuild|recreate|kad
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_KERNEL_CMDLINE="fbdev=off system_mode=self-test"
export SERENITY_RUN="ci"
build_target run
fi
Expand Down
4 changes: 2 additions & 2 deletions Userland/Services/SystemServer/Service.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -405,6 +405,6 @@ void Service::save_to(JsonObject& json)

bool Service::is_enabled() const
{
extern String g_boot_mode;
return m_boot_modes.contains_slow(g_boot_mode);
extern String g_system_mode;
return m_boot_modes.contains_slow(g_system_mode);
}
38 changes: 20 additions & 18 deletions Userland/Services/SystemServer/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
#include <sys/wait.h>
#include <unistd.h>

String g_boot_mode = "graphical";
String g_system_mode = "graphical";

static void sigchld_handler(int)
{
Expand All @@ -51,31 +51,33 @@ static void sigchld_handler(int)
}
}

static void parse_boot_mode()
static void determine_system_mode()
{
auto f = Core::File::construct("/proc/cmdline");
auto f = Core::File::construct("/proc/system_mode");
if (!f->open(Core::OpenMode::ReadOnly)) {
dbgln("Failed to read command line: {}", f->error_string());
dbgln("Failed to read system_mode: {}", f->error_string());
// Continue to assume "graphical".
return;
}
const String cmdline = String::copy(f->read_all(), Chomp);
dbgln("Read command line: {}", cmdline);
const String system_mode = String::copy(f->read_all(), Chomp);
if (f->error()) {
dbgln("Failed to read system_mode: {}", f->error_string());
// Continue to assume "graphical".
return;
}

g_system_mode = system_mode;
dbgln("Read system_mode: {}", g_system_mode);

// FIXME: Support more than one framebuffer detection
struct stat file_state;
int rc = lstat("/dev/fb0", &file_state);
if (rc < 0) {
for (auto& part : cmdline.split_view(' ')) {
auto pair = part.split_view('=', 2);
if (pair.size() == 2 && pair[0] == "boot_mode")
g_boot_mode = pair[1];
}
// We could boot into self-test which is not graphical too.
if (g_boot_mode == "self-test")
return;
g_boot_mode = "text";
if (rc < 0 && g_system_mode == "graphical") {
g_system_mode = "text";
} else if (rc == 0 && g_system_mode == "text") {
dbgln("WARNING: Text mode with framebuffers won't work as expected! Consider using 'fbdev=off'.");
}
dbgln("Booting in {} mode", g_boot_mode);
dbgln("System in {} mode", g_system_mode);
}

static void chown_wrapper(const char* path, uid_t uid, gid_t gid)
Expand Down Expand Up @@ -477,7 +479,7 @@ int main(int argc, char** argv)

if (!user) {
create_tmp_coredump_directory();
parse_boot_mode();
determine_system_mode();
}

Core::EventLoop event_loop;
Expand Down

0 comments on commit 8d13f6d

Please sign in to comment.