From 1b00618fd96179dad90c9d3be6d8e01427ce64ba Mon Sep 17 00:00:00 2001 From: Liav A Date: Fri, 25 Aug 2023 19:48:46 +0300 Subject: [PATCH] Kernel+Userland: Replace the beep syscall with the new /dev/beep device There's no need to have separate syscall for this kind of functionality, as we can just have a device node in /dev, called "beep", that allows writing tone generation packets to emulate the same behavior. In addition to that, we remove LibC sysbeep function, as this function was never being used by any C program nor it was standardized in any way. Instead, we move the userspace implementation to LibCore. --- Kernel/API/BeepInstruction.h | 13 ++++ Kernel/API/Syscall.h | 1 - Kernel/Arch/init.cpp | 3 + Kernel/CMakeLists.txt | 2 +- Kernel/Devices/Generic/PCSpeakerDevice.cpp | 63 +++++++++++++++++++ Kernel/Devices/Generic/PCSpeakerDevice.h | 36 +++++++++++ Kernel/Syscalls/beep.cpp | 34 ---------- .../DevTools/UserspaceEmulator/Emulator.h | 1 - .../UserspaceEmulator/Emulator_syscalls.cpp | 7 --- Userland/Libraries/LibC/unistd.cpp | 6 -- Userland/Libraries/LibC/unistd.h | 1 - Userland/Libraries/LibCore/System.cpp | 12 ++-- Userland/Libraries/LibCore/System.h | 2 +- Userland/Libraries/LibVT/TerminalWidget.cpp | 2 +- .../Services/DeviceMapper/DeviceEventLoop.cpp | 1 + Userland/Utilities/beep.cpp | 2 +- 16 files changed, 128 insertions(+), 58 deletions(-) create mode 100644 Kernel/API/BeepInstruction.h create mode 100644 Kernel/Devices/Generic/PCSpeakerDevice.cpp create mode 100644 Kernel/Devices/Generic/PCSpeakerDevice.h delete mode 100644 Kernel/Syscalls/beep.cpp diff --git a/Kernel/API/BeepInstruction.h b/Kernel/API/BeepInstruction.h new file mode 100644 index 0000000000..29ea3c21b2 --- /dev/null +++ b/Kernel/API/BeepInstruction.h @@ -0,0 +1,13 @@ +/* + * Copyright (c) 2023, Liav A. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +struct BeepInstruction { + u16 tone; +}; diff --git a/Kernel/API/Syscall.h b/Kernel/API/Syscall.h index 33eafe3545..15547b7a08 100644 --- a/Kernel/API/Syscall.h +++ b/Kernel/API/Syscall.h @@ -51,7 +51,6 @@ enum class NeedsBigProcessLock { S(allocate_tls, NeedsBigProcessLock::Yes) \ S(anon_create, NeedsBigProcessLock::No) \ S(annotate_mapping, NeedsBigProcessLock::No) \ - S(beep, NeedsBigProcessLock::No) \ S(bind, NeedsBigProcessLock::No) \ S(bindmount, NeedsBigProcessLock::No) \ S(chdir, NeedsBigProcessLock::No) \ diff --git a/Kernel/Arch/init.cpp b/Kernel/Arch/init.cpp index 8454479862..df3e98a8a3 100644 --- a/Kernel/Arch/init.cpp +++ b/Kernel/Arch/init.cpp @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -380,6 +381,8 @@ void init_stage2(void*) (void)MUST(RPi::MiniUART::create()).leak_ref(); #endif + (void)PCSpeakerDevice::must_create().leak_ref(); + #if ARCH(X86_64) VMWareBackdoor::the(); // don't wait until first mouse packet #endif diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index 635a90d43f..2ef545929c 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -80,6 +80,7 @@ set(KERNEL_SOURCES Devices/Generic/FullDevice.cpp Devices/Generic/MemoryDevice.cpp Devices/Generic/NullDevice.cpp + Devices/Generic/PCSpeakerDevice.cpp Devices/Generic/RandomDevice.cpp Devices/Generic/SelfTTYDevice.cpp Devices/Generic/ZeroDevice.cpp @@ -281,7 +282,6 @@ set(KERNEL_SOURCES Security/UBSanitizer.cpp Syscalls/anon_create.cpp Syscalls/alarm.cpp - Syscalls/beep.cpp Syscalls/chdir.cpp Syscalls/chmod.cpp Syscalls/chown.cpp diff --git a/Kernel/Devices/Generic/PCSpeakerDevice.cpp b/Kernel/Devices/Generic/PCSpeakerDevice.cpp new file mode 100644 index 0000000000..d17431923c --- /dev/null +++ b/Kernel/Devices/Generic/PCSpeakerDevice.cpp @@ -0,0 +1,63 @@ +/* + * Copyright (c) 2023, Liav A. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#if ARCH(X86_64) +# include +#endif +#include +#include +#include +#include + +namespace Kernel { + +UNMAP_AFTER_INIT NonnullRefPtr PCSpeakerDevice::must_create() +{ + auto device = MUST(DeviceManagement::try_create_device()); + return *device; +} + +UNMAP_AFTER_INIT PCSpeakerDevice::PCSpeakerDevice() + : CharacterDevice(1, 10) +{ +} + +UNMAP_AFTER_INIT PCSpeakerDevice::~PCSpeakerDevice() = default; + +bool PCSpeakerDevice::can_read(OpenFileDescription const&, u64) const +{ + return true; +} + +ErrorOr PCSpeakerDevice::read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) +{ + return Error::from_errno(ENOTIMPL); +} + +ErrorOr PCSpeakerDevice::write(OpenFileDescription&, u64, UserOrKernelBuffer const& buffer, size_t buffer_size) +{ + if (!kernel_command_line().is_pc_speaker_enabled()) + return Error::from_errno(ENOTSUP); + if (buffer_size % sizeof(BeepInstruction) != 0) + return Error::from_errno(EINVAL); + BeepInstruction instruction {}; + TRY(buffer.read(&instruction, sizeof(BeepInstruction))); + if (instruction.tone < 20 || instruction.tone > 20000) + return Error::from_errno(EINVAL); +#if ARCH(X86_64) + PCSpeaker::tone_on(instruction.tone); + auto result = Thread::current()->sleep(Duration::from_nanoseconds(200'000'000)); + PCSpeaker::tone_off(); + if (result.was_interrupted()) + return Error::from_errno(EINTR); + return sizeof(BeepInstruction); +#else + return Error::from_errno(ENOTIMPL); +#endif +} + +} diff --git a/Kernel/Devices/Generic/PCSpeakerDevice.h b/Kernel/Devices/Generic/PCSpeakerDevice.h new file mode 100644 index 0000000000..663bac8683 --- /dev/null +++ b/Kernel/Devices/Generic/PCSpeakerDevice.h @@ -0,0 +1,36 @@ +/* + * Copyright (c) 2023, Liav A. + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include + +namespace Kernel { + +class PCSpeakerDevice final : public CharacterDevice { + friend class DeviceManagement; + +public: + virtual ~PCSpeakerDevice() override; + + static NonnullRefPtr must_create(); + +private: + PCSpeakerDevice(); + + // ^Device + virtual bool is_openable_by_jailed_processes() const override { return true; } + + // ^CharacterDevice + virtual ErrorOr read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; + virtual ErrorOr write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; + virtual bool can_write(OpenFileDescription const&, u64) const override { return true; } + virtual bool can_read(OpenFileDescription const&, u64) const override; + virtual StringView class_name() const override { return "PCSpeakerDevice"sv; } + virtual bool is_seekable() const override { return true; } +}; + +} diff --git a/Kernel/Syscalls/beep.cpp b/Kernel/Syscalls/beep.cpp deleted file mode 100644 index a93fe8d9be..0000000000 --- a/Kernel/Syscalls/beep.cpp +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright (c) 2018-2020, Andreas Kling - * - * SPDX-License-Identifier: BSD-2-Clause - */ - -#include -#if ARCH(X86_64) -# include -#endif -#include - -namespace Kernel { - -ErrorOr Process::sys$beep(int tone) -{ - VERIFY_NO_PROCESS_BIG_LOCK(this); - if (!kernel_command_line().is_pc_speaker_enabled()) - return ENODEV; - if (tone < 20 || tone > 20000) - return EINVAL; -#if ARCH(X86_64) - PCSpeaker::tone_on(tone); - auto result = Thread::current()->sleep(Duration::from_nanoseconds(200'000'000)); - PCSpeaker::tone_off(); - if (result.was_interrupted()) - return EINTR; - return 0; -#else - return ENOTIMPL; -#endif -} - -} diff --git a/Userland/DevTools/UserspaceEmulator/Emulator.h b/Userland/DevTools/UserspaceEmulator/Emulator.h index df9f4003ea..20424fa3a3 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator.h +++ b/Userland/DevTools/UserspaceEmulator/Emulator.h @@ -145,7 +145,6 @@ private: int virt$accept4(FlatPtr); u32 virt$allocate_tls(FlatPtr, size_t); int virt$anon_create(size_t, int); - int virt$beep(); int virt$bind(int sockfd, FlatPtr address, socklen_t address_length); u32 virt$bindmount(u32 params_addr); int virt$chdir(FlatPtr, size_t); diff --git a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp index 82e469ce44..07cf3abc7e 100644 --- a/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp +++ b/Userland/DevTools/UserspaceEmulator/Emulator_syscalls.cpp @@ -47,8 +47,6 @@ u32 Emulator::virt_syscall(u32 function, u32 arg1, u32 arg2, u32 arg3) return virt$anon_create(arg1, arg2); case SC_annotate_mapping: return virt$annotate_mapping(arg1); - case SC_beep: - return virt$beep(); case SC_bind: return virt$bind(arg1, arg2, arg3); case SC_bindmount: @@ -1606,11 +1604,6 @@ u32 Emulator::virt$allocate_tls(FlatPtr initial_data, size_t size) return tls_base; } -int Emulator::virt$beep() -{ - return syscall(SC_beep); -} - u32 Emulator::virt$sysconf(u32 name) { return syscall(SC_sysconf, name); diff --git a/Userland/Libraries/LibC/unistd.cpp b/Userland/Libraries/LibC/unistd.cpp index 028776c181..c187b795d3 100644 --- a/Userland/Libraries/LibC/unistd.cpp +++ b/Userland/Libraries/LibC/unistd.cpp @@ -925,12 +925,6 @@ int gettid() return cached_tid; } -int sysbeep(int tone) -{ - int rc = syscall(SC_beep, tone); - __RETURN_WITH_ERRNO(rc, rc, -1); -} - // https://pubs.opengroup.org/onlinepubs/9699919799/functions/fsync.html int fsync(int fd) { diff --git a/Userland/Libraries/LibC/unistd.h b/Userland/Libraries/LibC/unistd.h index d935f2e314..75b67f90a7 100644 --- a/Userland/Libraries/LibC/unistd.h +++ b/Userland/Libraries/LibC/unistd.h @@ -36,7 +36,6 @@ int get_process_name(char* buffer, int buffer_size); int set_process_name(char const* name, size_t name_length); void dump_backtrace(void); int fsync(int fd); -int sysbeep(int tone); int gettid(void); int getpagesize(void); pid_t fork(void); diff --git a/Userland/Libraries/LibCore/System.cpp b/Userland/Libraries/LibCore/System.cpp index 364bb77667..eb44deb85f 100644 --- a/Userland/Libraries/LibCore/System.cpp +++ b/Userland/Libraries/LibCore/System.cpp @@ -9,10 +9,12 @@ #include #include +#include #include #include #include #include +#include #include #include #include @@ -147,11 +149,13 @@ namespace Core::System { #ifdef AK_OS_SERENITY -ErrorOr beep(Optional tone) +ErrorOr beep(u16 tone) { - auto rc = ::sysbeep(tone.value_or(440)); - if (rc < 0) - return Error::from_syscall("beep"sv, -errno); + static Optional beep_fd; + if (!beep_fd.has_value()) + beep_fd = TRY(Core::System::open("/dev/beep"sv, O_RDWR)); + BeepInstruction instruction { tone }; + TRY(Core::System::write(beep_fd.value(), Span(&instruction, sizeof(BeepInstruction)))); return {}; } diff --git a/Userland/Libraries/LibCore/System.h b/Userland/Libraries/LibCore/System.h index 1774be6de7..b035faaa92 100644 --- a/Userland/Libraries/LibCore/System.h +++ b/Userland/Libraries/LibCore/System.h @@ -51,7 +51,7 @@ namespace Core::System { #ifdef AK_OS_SERENITY -ErrorOr beep(Optional tone); +ErrorOr beep(u16 tone = 440); ErrorOr pledge(StringView promises, StringView execpromises = {}); ErrorOr unveil(StringView path, StringView permissions); ErrorOr unveil_after_exec(StringView path, StringView permissions); diff --git a/Userland/Libraries/LibVT/TerminalWidget.cpp b/Userland/Libraries/LibVT/TerminalWidget.cpp index b1cbbfcc98..bfa1cbb805 100644 --- a/Userland/Libraries/LibVT/TerminalWidget.cpp +++ b/Userland/Libraries/LibVT/TerminalWidget.cpp @@ -1069,7 +1069,7 @@ void TerminalWidget::beep() return; } if (m_bell_mode == BellMode::AudibleBeep) { - sysbeep(440); + [[maybe_unused]] auto ret_val = Core::System::beep(); return; } m_visual_beep_timer->restart(200); diff --git a/Userland/Services/DeviceMapper/DeviceEventLoop.cpp b/Userland/Services/DeviceMapper/DeviceEventLoop.cpp index 8eabbf9569..fbbdc11fc6 100644 --- a/Userland/Services/DeviceMapper/DeviceEventLoop.cpp +++ b/Userland/Services/DeviceMapper/DeviceEventLoop.cpp @@ -194,6 +194,7 @@ struct PluggableOnceCharacterDeviceNodeMatch { }; static constexpr PluggableOnceCharacterDeviceNodeMatch s_simple_matchers[] = { + { "/dev/beep"sv, 0666, 1, 10 }, { "/dev/kcov"sv, 0666, 30, 0 }, }; diff --git a/Userland/Utilities/beep.cpp b/Userland/Utilities/beep.cpp index eb765876cf..9ea32ad92e 100644 --- a/Userland/Utilities/beep.cpp +++ b/Userland/Utilities/beep.cpp @@ -14,6 +14,6 @@ ErrorOr serenity_main(Main::Arguments arguments) Core::ArgsParser args_parser; args_parser.add_option(tone, "Beep tone", "beep-tone", 'f', "Beep tone (frequency in Hz)"); args_parser.parse(arguments); - TRY(Core::System::beep(tone)); + TRY(Core::System::beep(tone.value_or(440))); return 0; }