We were accidentally casting the pointer to m_ttl from an u8* to an int*
which resulted in copying of 3 extra unrelated bytes (which turned out
to be padding in this case).
We create a base class called GenericFramebufferDevice, which defines
all the virtual functions that must be implemented by a
FramebufferDevice. Then, we make the VirtIO FramebufferDevice and other
FramebufferDevice implementations inherit from it.
The most important consequence of rearranging the classes is that we now
have one IOCTL method, so all drivers should be committed to not
override the IOCTL method or make their own IOCTLs of FramebufferDevice.
All graphical IOCTLs are known to all FramebufferDevices, and it's up to
the specific implementation whether to support them or discard them (so
we require extensive usage of KResult and KResultOr, together with
virtual characteristic functions).
As a result, the interface is much cleaner and understandable to read.
As suggested by @ccapitalK, it makes the interface more neat and clean.
The proper order is to get ScanoutID first, then ResourceID and after it
everything else that is needed to complete the operation successfully.
A VirtIO graphics adapter is really the VirtIO GPU, so the virtualized
hardware has no distinction between both components so there's no
need to put such distinction in software.
We might need to split things in the future, but when we do so, we must
take proper care to ensure that the interface between the components
is correct and use the usual codebase patterns.
We never used that type method except in initialization in
GraphicsManagement, and we used it there to query whether the device is
VGA compatible or not.
'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.
Bootmode used to control framebuffers, panic behavior, and SystemServer.
This patch factors framebuffer control into a separate flag.
Note that the combination 'bootmode=self-test fbdev=on' leads to
unexpected behavior, which can only be fixed in a later commit.
Some ports (like `bc` with history enabled) sensibly set the termios
character size to 8 bits.
Previously, we left the character size value (given by the bitmask
CSIZE) as zero by default (meaning 5 bits per character), and returned
ENOTIMPL whenever someone modified it. This was dumb.
Bit 3 is set here:
c5b2f55981/hw/input/ps2.c (L736)
Spurious mouse packets can be received without this bit set, for
example when double-clicking and keeping the mouse button depressed
instead of releasing it the second time (i.e. mousedown > mouseup >
mousedown). We should not process such packets.
This makes interaction with our buttons much smoother!
Fixes#5881.
Instead of detecting which flag was set in the status register, we can
use the instrument type passed to us. This works because the mouse and
keyboard use different IRQs.
The System V ABI requires that the stack is 16-byte aligned on function
call. Confusingly, however, they mean that the stack must be aligned
this way **before** the `CALL` instruction is executed. That instruction
pushes the return value onto the stack, so the callee will actually see
the stack pointer as a value `sizeof(FlatPtr)` smaller.
The signal trampoline was written with this in mind, but `setup_stack`
aligned the entire stack, *including the return address* to a 16-byte
boundary. Because of this, the trampoline subtracted too much from the
stack pointer, thus misaligning it.
This was not a problem on i686 because we didn't execute any
instructions from signal handlers that would require memory operands to
be aligned to more than 4 bytes. This is not the case, however, on
x86_64, where SSE instructions are enabled by default and they require
16-byte aligned operands. Running such instructions raised a GP fault,
immediately killing the offending program with a SIGSEGV signal.
This issue caused TestKernelAlarm to fail in LibC when ran locally, and
at one point, the zsh port was affected too.
Fixes#9291
Instead, just ensure we pick the m_access_lock and then m_scan_lock when
doing a scan/re-scan of the PCI configuration space so we know nobody
can actually access the PCI configuration space during the scan.
The m_scan_lock is now a Spinlock, to ensure we cannot yield to other
process while we do the PCI configuration space scanning.
This small change simplifies the function a bit but also fixes a problem
with it.
Let's take an example to see this:
Let's say we have a reserved range between 0xe0000 to 0xfffff (EBDA),
then we want to map from the memory device (/dev/mem) the entire
EBDA to a program. If a program tries to map more than 131072 bytes,
the current logic will work - the start address is 0xe0000, and ofcourse
it's below the limit, hence it passes the first two restrictions.
Then, the third if statement will fail if we try to mmap more than
the said allowed bytes.
However, let's take another scenario, where we try to mmap from
0xf0000 - but we try to mmap less than 131072 - but more than 65536.
In such case, we again pass the first two if statements, but the third
one is passed two, because it doesn't take into account the offseted
address from the start of the reserved range (0xe0000). In such case,
a user can easily mmap 65535 bytes above 0x100000. This might
seem negligible. However, it's still a severe bug that can theoretically
be exploited into a info leak or tampering with important kernel
structures.
Storing assigning a string literal to a String object just to pass it to
a function expecting a StringView is wasteful. Let's just not do that.
For consistency's sake, this commit changes all of the other invocations
to use StringView literals, too.
Forcing the formatting to go through `Formatter<FormatString>` is
completely unnecessary, increases code size, performs a String
allocation and prevents us from using the formatting options available
on that type.
This commit also removes explicit formatters from
`BlockBasedFileSystem::BlockIndex` and `Kernel::InodeIndex`, as those
are already covered by the blanket implementation for all
`DistinctNumeric` types.
This change allows the Kernel to actually construct other interfaces
besides the E1000 type.
This solves a breakage that was introduced recently because of move
semantics.
A couple of points on this patch:
1. In current situation, we can waste time to create a KString and throw
it for nothing. This patch ensures we only create it near construction
point so we know we actually need it.
2. It's very likely to assume that non-x86 machines will expose network
device with a device tree (or with ACPI). The raspberry pi machine is a
good example of that. Therefore, each driver should explicitly ask the
correct interface name generation method, and this patch simplifies this
pattern greatly, especially in a case where the same network device can
appear as a PCI device or as device in another bus type on the same
platform target. For example, the (in)famous ne2000 device can be used
either as a PCI device or as an ISA device, depending on the model.
3. In my opinion, it seems much more readable to construct the name near
calling point of the object constructor than to just pass it with move
semantics.
By enabling LTO for the kernel_heap object too, we open the door for
optimization opportunities that come from (partially) inlining `::new`
or kmalloc. Every software spends a non-trivial amount of its run time
on allocating memory, so hopefully this change will make LTO builds even
faster.
This commit updates the Clang toolchain's version to 13.0.0, which comes
with better C++20 support and improved handling of new features by
clang-format. Due to the newly enabled `-Bsymbolic-functions` flag, our
Clang binaries will only be 2-4% slower than if we dynamically linked
them, but we save hundreds of megabytes of disk space.
The `BuildClang.sh` script has been reworked to build the entire
toolchain in just three steps: one for the compiler, one for GNU
binutils, and one for the runtime libraries. This reduces the complexity
of the build script, and will allow us to modify the CI configuration to
only rebuild the libraries when our libc headers change.
Most of the compile flags have been moved out to a separate CMake cache
file, similarly to how the Android and Fuchsia toolchains are
implemented within the LLVM repo. This provides a nicer interface than
the heaps of command-line arguments.
We no longer build separate toolchains for each architecture, as the
same Clang binary can compile code for multiple targets.
The horrible mess that `SERENITY_CLANG_ARCH` was, has been removed in
this commit. Clang happily accepts an `i686-pc-serenity` target triple,
which matches what our GCC toolchain accepts.
LLD fails to define the _GLOBAL_OFFSET_TABLE_ symbol if all inputs to it
are LLVM bitcode files (i.e. those used for LTO). To allow the kernel to
be built with ThinLTO, the workaround suggested in the original LLVM bug
report (<https://bugs.llvm.org/show_bug.cgi?id=39634>) is added in this
commit.
ProcFSGlobalInode now calls `write_bytes()`, `truncate()` and
`set_mtime()` on its associated component. This allows us to write 0 or
1 to a ProcFSSystemBoolean component to toggle a boolean value.
Spinlocks are tied to the platform they are built for, this is why they
have been moved into the Arch folder. They are still available via
"Locking/Spinlock.h"
An Aarch64 stub has been created
A new RegisterState header includes the platform specific RegisterState
header based on the platform being compiled.
The Aarch64 RegisterState header contains stubs for Debug
When booting on RPI3 firmware puts CPU in EL2 mode which is
different from QEMU's default EL3.
I've added logic to discover initial mode at boot
and then act accordingly. This results in Serenity corectly
switching to EL1 on target hardware now.
The platform independent Processor.h file includes the shared processor
code and includes the specific platform header file.
All references to the Arch/x86/Processor.h file have been replaced with
a reference to Arch/Processor.h.
The OpenFileDescription class already offers the necessary functionlity,
so implementing this was only a matter of following the structure for
`read` while handling the additional `offset` argument.
Having these bits of code factored out not only prevents duplication
now, but will also allow us to implement pread without repeating
ourselves (too much).
Values in `ioctl` are given through a pointer, but ioctl's FIONBIO
implementation was interpreting this pointer as an integer directly.
This meant that programs using `ioctl` to set a file descriptor in
blocking mode met with incorrect behavior: they passed a non-null
pointer pointing to a value of 0, but the kernel interpreted the pointer
as a non-zero integer, thus making the file non-blocking.
This commit fixes this behavior by reading the value from the userspace
pointer and using that to set the non-blocking flag on the file
descriptor.
This bug was found while trying to run the openssl tool on serenity,
which used `ioctl` to ensure newly-created sockets are in blocking mode.
Normally, trying to truncate a SysFSInode should result in EPERM error.
However, as suggested by Ali (@alimpfard), we can allow the PowerState
node to be "truncated" so one can open that file with O_TRUNC option.
Likewise, we also need to provide a way to set modified time on SysFS
inodes. For most inodes, we should return ENOTIMPL error, but for the
power state switch, we ignore the modified time setting and just return
KSuccess.
These fixes allow to do "echo -n 1 > /sys/firmware/power_state" in Shell
after gaining root permissions, to switch the power state.
There's basically no real difference in software between a SATA harddisk
and IDE harddisk. The difference in the implementation is for the host
bus adapter protocol and registers layout.
Therefore, there's no point in putting a distinction in software to
these devices.
This change also greatly simplifies and removes stale APIs and removes
unnecessary parameters in constructor calls, which tighten things
further everywhere.
While I was working on LibWeb, I got a page fault at 0xe0e0e0e4.
This indicates a destroyed RefPtr if compiled with SANITIZE_PTRS
defined. However, the page fault handler didn't print out this
indication.
This makes the page fault handler print out a note if the faulting
address looks like a recently destroyed RefPtr, OwnPtr, NonnullRefPtr,
NonnullOwnPtr, ThreadSafeRefPtr or ThreadSafeNonnullRefPtr. It will
only do this if SANITIZE_PTRS is defined, as smart pointers don't get
scrubbed without it being defined.
This exposes a small subset of the information exposed by the Linux
equivalent, and will be used to optimize applications that would like
to know the current CPU usage statistics, but don't want to read all of
the unrelated information in /proc/all
Some time ago, automatic locking was added to the AK smart pointers to
paper over various race conditions in the kernel. Until we've actually
solved the issues in the kernel, we're stuck with the locking.
However, we don't need to punish single-threaded userspace programs with
the high cost of locking. This patch moves the thread-safe variants of
RefPtr, NonnullRefPtr, WeakPtr and RefCounted into Kernel/Library/.
This change is another minor step towards removing `AK::String` from
the Kernel. Instead of dynamically allocating the storage_name we can
instead allocate it via a KString in the factory for each device, and
then push the device name down into the StorageDevice base class.
We don't have a way of doing `AK::String::formatted(..)` with a KString
at the moment, so cleaning that up will be left for a later day.
Previously there was a mix of returning plain strings and returning
explicit string views using `operator ""sv`. This change switches them
all to standardized on `operator ""sv` as it avoids a call to strlen.
Previously there was a mix of returning plain strings and returning
explicit string views using `operator ""sv`. This change switches them
all to standardized on `operator ""sv` as it avoids a call to strlen.
For now, this can only query microseconds since boot.
Use this to print a timestamp every second. This busy-loops
until a second has passed. This might be a good first use of
interrupts soon.
qemu used to not implement this timer at some point, but
it seems to work fine even in qemu now (qemu v 5.2.0).
SonarCloud flagged this "Code Smell", where we are accessing these
static methods as if they are instance methods. While it is technically
possible, it is very confusing to read when you realize they are static
functions.
SonarCloud flagged this "Code Smell", where we are accessing these
static methods as if they are instance methods. While it is technically
possible, it is very confusing to read when you realize they are static
functions.
When a process with a large heap crashes (e.g WebContent), it gets very
cumbersome to dump out a huge amount of memory.
In the vast majority of cases, we're only interested in generating a
nice backtrace from the coredump, so let's have the kernel skip over
userspace heap regions when dumping memory for now.
This is not ideal, and almost a little bit ugly, but it does make
investigating 500 MiB WebContent crashes significantly easier for now.
After building and running
objcopy -O binary Build/aarch64/Kernel/Prekernel/Prekernel \
/media/sdcard/kernel8.img
things start booting on an actual RPi4 :^)
(Assuming the sdcard contains RPi firmware, an empty config.txt,
and no other kernel*.img files).
This allows us to remove the PCI::get_interrupt_line API function. As a
result, this removes a bunch of not so great patterns that we used to
cache PCI interrupt line in many IRQHandler derived classes instead of
just using interrupt_number method of IRQHandler class.
Instead of blindly forcing BGR format on the bochs-display device, let's
ensure we do that only on QEMU bochs-display and not on VirtualBox
graphics adapter too.
- .text now starts at 0x80000, where an actual (non-qemu) RPi expects
- use magic section name ".text.first" to make sure the linker script
puts the kernel entry point at the start of the .text section
- remove a few things from the x86 linker script that aren't needed
for aarch64 (yet?)
This moves Kernel/Prekernel/linker.ld unchanged to
Kernel/Prekernel/Arch/aarch64 and Kernel/Prekernel/Arch/x86.
The aarch64 will change in a future commit.
No behavior change.
Looking at how these two constants are commonly used in other systems,
we should be able to mimic their behavior using our PT_PEEK constant.
For example, see:
https://man.netbsd.org/NetBSD-6.0.1/i386/ptrace.2
This ensures we dont try to hold the PCI Access mutex under IRQ when
printing VirtIO debug logs (which is not allowed and results in an
assertion). This is also relatively free, as it requires no allocations
(we're just storing a pointer to the rodata section).
As a demo, query the firmware version. `Meta/serenity.sh gdb aarch64`
can be used to observe that qemu puts 0x548E1 in x0 in response
to this mailbox message.
This fixes a Kernel Panic where the lazy allocation triggers inside an
ISR and grabs a mutex, which isn't allowed when interrupts are
disabled. This also fixes a bug where the mapping for VirtIO device
BARs is never allocated. #9876
Currently, writing anything to `/dev/mouse0` or `/dev/keyboard0` causes
the Kernel to panic. The reason for this is that
`[Mouse,Keyboard]Device::write` always returns 0, which is explicitly
prohibited by `VERIFY` macro in `Process::sys$write`. The fix seems
trivial; `write` should return EINVAL instead (as is the case with, for
example, `KCOVDevice`).
When testing the RTL8168 driver, it seems we can't allocate super pages
anymore. Either we expand the super pages range, or find a solution to
dynamically expand the range (or let drivers utilize other ranges).
This singleton simplifies many aspects that we struggled with before:
1. There's no need to make derived classes of Device expose the
constructor as public anymore. The singleton is a friend of them, so he
can call the constructor. This solves the issue with try_create_device
helper neatly, hopefully for good.
2. Getting a reference of the NullDevice is now being done from this
singleton, which means that NullDevice no longer needs to use its own
singleton, and we can apply the try_create_device helper on it too :)
3. We can now defer registration completely after the Device constructor
which means the Device constructor is merely assigning the major and
minor numbers of the Device, and the try_create_device helper ensures it
calls the after_inserting method immediately after construction. This
creates a great opportunity to make registration more OOM-safe.
Previously, attempting to call sys$waitid on non-child processes
returned ECHILD.
That prevented debugging non-child processes by attaching to them during
runtime (as opposed to forking and debugging the child, which is what
was previously supported).
We now allow calling sys$waitid on a any process that is being traced
by us, even if it's not our child.
Because we were holding a strong ref to the OpenFileDescription in
LocalSocket and a strong ref to the LocalSocket in Inode, we were
creating a reference cycle in the event of the socket being cleaned up
after the file description did (i.e. unlinking the file before closing
the socket), because the file description never got destructed.
The TimeWait state is intended to prevent another socket from taking the
address tuple in case any packets are still in transit after the final
close. Since this state never delivers packets to userspace, it doesn't
make sense to keep the receive buffer around.
Replace the old logic where we would start with a host build, and swap
all the CMake compiler and target variables underneath it to trick
CMake into building for Serenity after we configured and built the Lagom
code generators.
The SuperBuild creates two ExternalProjects, one for Lagom and one for
Serenity. The Serenity project depends on the install stage for the
Lagom build. The SuperBuild also generates a CMakeToolchain file for the
Serenity build to use that replaces the old toolchain file that was only
used for Ports.
To ensure that code generators are rebuilt when core libraries such as
AK and LibCore are modified, developers will need to direct their manual
`ninja` invocations to the SuperBuild's binary directory instead of the
Serenity binary directory.
This commit includes warning coalescing and option style cleanup for the
affected CMakeLists in the Kernel, top level, and runtime support
libraries. A large part of the cleanup is replacing USE_CLANG_TOOLCHAIN
with the proper CMAKE_CXX_COMPILER_ID variable, which will no longer be
confused by a host clang compiler.
There are a few violations with signal handling that I won't be able to
fix it until later this week. So lets put lock rank enforcement under a
debug option for now so other folks don't hit these crashes until rank
enforcement is more fleshed out.
The better fix is to have a linker script. We'll need this to set
the entry point to 0x80000 for bare-metal builds anyways. But I'd
like to get some UART output in qemu before I add this (otherwise
I can't check if the bare-metal version does anything), so put
in this temporary kludge for now.
Needed for functions that have local variables.
In time we need to share this between aarch64 and intel, but while
we figure out what exactly the aarch64 Prekernel should do, let's
duplicate this.
Else, function-local statics create calls to
__cxa_guard_acquire / __cxa_guard_release on aarch64, which we don't
(yet?) implement. Since Prekernel is single-threaded, just sidestep
that for now.
PVS-Studio flagged these as uninitialized. While there is no bug here,
it is our policy to always initialize members to avoid potential bugs
in the future.
This change removes the halt and reboot syscalls, and create a new
mechanism to change the power state of the machine.
Instead of how power state was changed until now, put a SysFS node as
writable only for the superuser, that with a defined value, can result
in either reboot or poweroff.
In the future, a power group can be assigned to this node (which will be
the GroupID responsible for power management).
This opens an opportunity to permit to shutdown/reboot without superuser
permissions, so in the future, a userspace daemon can take control of
this node to perform power management operations without superuser
permissions, if we enforce different UserID/GroupID on that node.
Both should reside in the SysFS firmware directory which is normally
located in /sys/firmware.
Also, apply some OOM-safety patterns when creating the BIOS and ACPI
directories.
This will somwhat help unify them also under the same SysFS directory in
the commit.
Also, it feels much more like this change reflects the reality that both
ACPI and the BIOS are part of the firmware on x86 computers.
These interfaces are broken for about 9 months, maybe longer than that.
At this point, this is just a dead code nobody tests or tries to use, so
let's remove it instead of keeping a stale code just for the sake of
keeping it and hoping someone will fix it.
To better justify this, I read that OpenBSD removed loadable kernel
modules in 5.7 release (2014), mainly for the same reason we do -
nobody used it so they had no good reason to maintain it.
Still, OpenBSD had LKMs being effectively working, which is not the
current state in our project for a long time.
An arguably better approach to minimize the Kernel image size is to
allow dropping drivers and features while compiling a new image.
I forgot that we need to also initialize SerialDevice and also to ensure
it creates a sysfs node properly. Although I had a better fix for this,
it keeps the CI happy, so for now it's more than enough :)
Instead of doing so in the constructor, let's do immediately after the
constructor, so we can safely pass a reference of a Device, so the
SysFSDeviceComponent constructor can use that object to identify whether
it's a block device or a character device.
This allows to us to not hold a device in SysFSDeviceComponent with a
RefPtr.
Also, we also call the before_removing method in both SlavePTY::unref
and File::unref, so because Device has that method being overrided, it
can ensure the device is removed always cleanly.
This function was checking 1 byte after the provided range, which caused
it to reject valid userspace ranges that happened to end exactly at the
top of the user address space.
This fixes a long-standing issue with mysterious Optional errors in
Coredump::write_regions(). (It happened when trying to add a memory
region at the very top of the address space to a coredump.)
Let's remove the DynamicParser class, as it really did nothing yet in
the Kernel. Instead, when we add support for AML parsing, we can figure
out how to do it properly without the need of a derived class that just
complicates everything for no good reason.
This variant of dbgputstr does not lock the global log lock, as it is
called before the current or any other processor was initialized,
meaning that:
A) The $gs base was not setup yet, so we cannot enter into critical
sections, and as a result we cannot use SpinLocks
B) No other processors may try to print at the same time anyway
This fixes a triple fault that occurs when compiling serenity with
the i686 clang toolchain. (The underlying issue is that the old inline
assembly did not specify that it clobbered the eax/ecx/edx registers
and as such the compiler assumed they were not changed and used their
values across it)
Co-authored-by: Brian Gianforcaro <bgianf@serenityos.org>
