Simplifies and removes some casts. In all cases, these were generally
widening from a 32-bit unsigned type to a 64-bit unsigned type, so no
information would be lost from the conversion.
`PhysicalCore`'s move assignment operator was declared as `= default`,
but was implicitly deleted because `PhysicalCore` has fields
of reference type. Switch to explicitly deleting it to avoid a Clang
warning.
The move *constructor* is still defaulted, and is required to exist due
to the use of `std::vector<PhysicalCore>`.
Resolves numerous deprecation warnings throughout the codebase due to
inclusion of this header. Now building core should be significantly less
noisy (and also relying on less global state).
This also uncovered quite a few modules that were relying on indirect
includes, which have also been fixed.
Unicorn long-since lost most of its use, due to dynarmic gaining support
for handling most instructions. At this point any further issues
encountered should be used to make dynarmic better.
This also allows us to remove our dependency on Python.
Locks on GetCurrentHostThreadID were causing performance issues
according to Visual Studio's profiler. It was consuming twice the time
as arm_interface.Run(). The cost was not in the function itself but in
the lockinig it required.
Reimplement these functions using atomics and static storage instead of
an unordered_map. This is a side effect to avoid locking and using linked
lists for reads.
Replace unordered_map with a linear search.
Makes our error coverage a little more consistent across the board by
applying it to Linux side of things as well. This also makes it more
consistent with the warning settings in other libraries in the project.
This also updates httplib to 0.7.9, as there are several warning
cleanups made that allow us to enable several warnings as errors.
As reported by tsan, SelectThreads could write to
is_context_switch_pending holding a mutex while SwitchToCurrent reads it
without holding any.
It is assumed that the author didn't want an atomic here, so the code is
reordered so that whenever is_context_switch_pending is read inside
SwitchToContext, the mutex is locked.
As reported by tsan, host_thread_ids could be read while
any of the RegisterHostThread variants were called.
To fix this, lock the register mutex when yuzu is running in multicore
mode and GetCurrentHostThreadID is called.
Allows the compiler to warn about cases where the constructor is used
but then immediately discarded, which is a potential cause of
locking/unlocking bugs.
* ipc: Allow all trivially copyable objects to be passed directly into WriteBuffer
With the support of C++20, we can use concepts to deduce if a type is an STL container or not.
* More agressive concept for stl containers
* Add -fconcepts
* Move to common namespace
* Add Common::IsBaseOf
Makes the interface future-proofed for supporting other platforms in the event we ever support platforms with differing pointer sizes. This way, we have a type in place that is always guaranteed to be able to represent a pointer exactly.
These aren't directly important or commonly used within the process, so
we can move these to the bottom to allow everything else to be more
likely to be within a cache line.
Profiling shows that this is a highly contested mutex, causing dimishing
results compared to a OS lock. std::mutex implementations can spin for a
while before falling back to an OS lock.
This avoids wasting precious CPU cycles in a no-op.
* Switch game settings to use a pointer
In order to add full per-game settings, we need to be able to tell yuzu to switch
to using either the global or game configuration. Using a pointer makes it easier
to switch.
* configuration: add new UI without changing existing funcitonality
The new UI also adds General, System, Graphics, Advanced Graphics,
and Audio tabs, but as yet they do nothing. This commit keeps yuzu
to the same functionality as originally branched.
* configuration: Rename files
These weren't included in the last commit. Now they are.
* configuration: setup global configuration checkbox
Global config checkbox now enables/disables the appropriate tabs in the game
properties dialog. The use global configuration setting is now saved to the
config, defaulting to true. This also addresses some changes requested in the PR.
* configuration: swap to per-game config memory for properties dialog
Does not set memory going in-game. Swaps to game values when opening the
properties dialog, then swaps back when closing it. Uses a `memcpy` to swap.
Also implements saving config files, limited to certain groups of configurations
so as to not risk setting unsafe configurations.
* configuration: change config interfaces to use config-specific pointers
When a game is booted, we need to be able to open the configuration dialogs
without changing the settings pointer in the game's emualtion. A new pointer
specific to just the configuration dialogs can be used to separate changes
to just those config dialogs without affecting the emulation.
* configuration: boot a game using per-game settings
Swaps values where needed to boot a game.
* configuration: user correct config during emulation
Creates a new pointer specifically for modifying the configuration while
emulation is in progress. Both the regular configuration dialog and the game
properties dialog now use the pointer Settings::config_values to focus edits to
the correct struct.
* settings: split Settings::values into two different structs
By splitting the settings into two mutually exclusive structs, it becomes easier,
as a developer, to determine how to use the Settings structs after per-game
configurations is merged. Other benefits include only duplicating the required
settings in memory.
* settings: move use_docked_mode to Controls group
`use_docked_mode` is set in the input settings and cannot be accessed from the
system settings. Grouping it with system settings causes it to be saved with
per-game settings, which may make transferring configs more difficult later on,
especially since docked mode cannot be set from within the game properties
dialog.
* configuration: Fix the other yuzu executables and a regression
In main.cpp, we have to get the title ID before the ROM is loaded, else the
renderer will reflect only the global settings and now the user's game specific
settings.
* settings: use a template to duplicate memory for each setting
Replaces the type of each variable in the Settings::Values struct with a new
class that allows basic data reading and writing. The new struct
Settings::Setting duplicates the data in memory and can manage global overrides
per each setting.
* configuration: correct add-ons config and swap settings when apropriate
Any add-ons interaction happens directly through the global values struct.
Swapping bewteen structs now also includes copying the necessary global configs
that cannot be changed nor saved in per-game settings. General and System config
menus now update based on whether it is viewing the global or per-game settings.
* settings: restore old values struct
No longer needed with the Settings::Setting class template.
* configuration: implement hierarchical game properties dialog
This sets the apropriate global or local data in each setting.
* clang format
* clang format take 2
can the docker container save this?
* address comments and style issues
* config: read and write settings with global awareness
Adds new functions to read and write settings while keeping the global state in
focus. Files now generated per-game are much smaller since often they only need
address the global state.
* settings: restore global state when necessary
Upon closing a game or the game properties dialog, we need to restore all global
settings to the original global state so that we can properly open the
configuration dialog or boot a different game.
* configuration: guard setting values incorrectly
This disables setting values while a game is running if the setting is
overwritten by a per game setting.
* config: don't write local settings in the global config
Simple guards to prevent writing the wrong settings in the wrong files.
* configuration: add comments, assume less, and clang format
No longer assumes that a disabled UI element means the global state is turned
off, instead opting to directly answer that question. Still however assumes a
game is running if it is in that state.
* configuration: fix a logic error
Should not be negated
* restore settings' global state regardless of accept/cancel
Fixes loading a properties dialog and causing the global config dialog to show
local settings.
* fix more logic errors
Fixed the frame limit would set the global setting from the game properties
dialog. Also strengthened the Settings::Setting member variables and simplified
the logic in config reading (ReadSettingGlobal).
* fix another logic error
In my efforts to guard RestoreGlobalState, I accidentally negated the IsPowered
condition.
* configure_audio: set toggle_stretched_audio to tristate
* fixed custom rtc and rng seed overwriting the global value
* clang format
* rebased
* clang format take 4
* address my own review
Basically revert unintended changes
* settings: literal instead of casting
"No need to cast, use 1U instead"
Thanks, Morph!
Co-authored-by: Morph <39850852+Morph1984@users.noreply.github.com>
* Revert "settings: literal instead of casting
"
This reverts commit 95e992a87c898f3e882ffdb415bb0ef9f80f613f.
* main: fix status buttons reporting wrong settings after stop emulation
* settings: Log UseDockedMode in the Controls group
This should have happened when use_docked_mode was moved over to the controls group
internally. This just reflects this in the log.
* main: load settings if the file has a title id
In other words, don't exit if the loader has trouble getting a title id.
* use a zero
* settings: initalize resolution factor with constructor instead of casting
* Revert "settings: initalize resolution factor with constructor instead of casting"
This reverts commit 54c35ecb46a29953842614620f9b7de1aa9d5dc8.
* configure_graphics: guard device selector when Vulkan is global
Prevents the user from editing the device selector if Vulkan is the global
renderer backend. Also resets the vulkan_device variable when the users
switches back-and-forth between global and Vulkan.
* address reviewer concerns
Changes function variables to const wherever they don't need to be changed. Sets Settings::Setting to final as it should not be inherited from. Sets ConfigurationShared::use_global_text to static.
Co-Authored-By: VolcaEM <volcaem@users.noreply.github.com>
* main: load per-game settings after LoadROM
This prevents `Restart Emulation` from restoring the global settings *after* the per-game settings were applied. Thanks to BSoDGamingYT for finding this bug.
* Revert "main: load per-game settings after LoadROM"
This reverts commit 9d0d48c52d2dcf3bfb1806cc8fa7d5a271a8a804.
* main: only restore global settings when necessary
Loading the per-game settings cannot happen after the ROM is loaded, so we have to specify when to restore the global state. Again thanks to BSoD for finding the bug.
* configuration_shared: address reviewer concerns except operator overrides
Dropping operator override usage in next commit.
Co-Authored-By: LC <lioncash@users.noreply.github.com>
* settings: Drop operator overrides from Setting template
Requires using GetValue and SetValue explicitly. Also reverts a change that broke title ID formatting in the game properties dialog.
* complete rebase
* configuration_shared: translate "Use global configuration"
Uses ConfigurePerGame to do so, since its usage, at least as of now, corresponds with ConfigurationShared.
* configure_per_game: address reviewer concern
As far as I understand, it prevents the program from unnecessarily copying strings.
Co-Authored-By: LC <lioncash@users.noreply.github.com>
Co-authored-by: Morph <39850852+Morph1984@users.noreply.github.com>
Co-authored-by: VolcaEM <volcaem@users.noreply.github.com>
Co-authored-by: LC <lioncash@users.noreply.github.com>
This commit: Implements CPU Interrupts, Replaces Cycle Timing for Host
Timing, Reworks the Kernel's Scheduler, Introduce Idle State and
Suspended State, Recreates the bootmanager, Initializes Multicore
system.
Previously if applications would send faulty buffers(example homebrew) it would lead to us returning uninitalized data. Switching from ASSERT_MSG to ASSERT_OR_EXECUTE_MSG allows us to have a fail safe to prevent crashes but also continue execution without introducing undefined behavior
GetTotalPhysicalMemoryAvailableWithoutSystemResource & GetTotalPhysicalMemoryUsedWithoutSystemResource seem to subtract the resource size from the usage.
While èis generally representable in some language encodings, in some
it isn't and will result in compilation warnings occurring. To remain
friendly with other language's codepages on Windows, we normalize it to
an ASCII e.
We can also allow unicorn to be constructed in 32-bit mode or 64-bit
mode to satisfy the need for both interpreter instances.
Allows this code to compile successfully of non x86-64 architectures.
Any time the lambda function is called, the permission being used in the
capture would be passed in as an argument to the lambda, so the capture
is unnecessary.
These were added in the change that enabled -Wextra on linux builds so
as not to introduce interface changes in the same change as a
build-system flag addition.
Now that the flags are enabled, we can freely change the interface to
make these unnecessary.
Allows reporting more cases where logic errors may exist, such as
implicit fallthrough cases, etc.
We currently ignore unused parameters, since we currently have many
cases where this is intentional (virtual interfaces).
While we're at it, we can also tidy up any existing code that causes
warnings. This also uncovered a few bugs as well.
This can result in silent logic bugs within code, and given the amount
of times these kind of warnings are caused, they should be flagged at
compile-time so no new code is submitted with them.
shared_ptr was used in 2d1984c20c due to a
misunderstanding of how the language generates move constructors and
move assignment operators.
If a destructor is user-provided, then the compiler won't generate the
move constructor and move assignment operators by default--they must be
explicitly opted into.
The reason for the compilation errors is due to the fact that the
language will fall back to attempting to use the copy constructor/copy
assignment operators if the respective move constructor or move
assignment operator is unavailable.
Given that we explicitly opt into them now, the the move constructor and
move assignment operators will be generated as expected.
This isn't used within the class, so it can be removed to simplify the
overall interface.
While we're in the same area, we can simplify a unique_ptr reset() call.
module._memory was already moved over to a new shared_ptr.
So code_memory_size was not increased at all.
This lowers the heap space and so saves a bit of memory, usually between 50 to 100 MB.
This fixes a regression of c0a01f3adc
* Kernel: Correct behavior of Address Arbiter threads.
This corrects arbitration threads to behave just like in Horizon OS.
They are added into a container and released according to what priority
they had when added. Horizon OS does not reorder them if their priority
changes.
* Kernel: Address Feedback.
Over the course of the changes to the kernel code, a few includes are no
longer necessary, particularly with the change over to std::shared_ptr
from Boost's intrusive_ptr.
These are fairly trivial to implement, we can just do nothing. This also
provides a spot for us to potentially dump out any relevant info in the
future (e.g. for debugging purposes with homebrew, etc).
While we're at it, we can also correct the names of both of these
supervisor calls.
This commit corrects an error in which a Core could remain with an
exclusive state after running, leaving space for possible race
conditions between changing cores.
Now that literally every other API function is converted over to the
Memory class, we can just move the file-local page table into the Memory
implementation class, finally getting rid of global state within the
memory code.
The Write functions are used slightly less than the Read functions,
which make these a bit nicer to move over.
The only adjustments we really need to make here are to Dynarmic's
exclusive monitor instance. We need to keep a reference to the currently
active memory instance to perform exclusive read/write operations.
With all of the trivial parts of the memory interface moved over, we can
get right into moving over the bits that are used.
Note that this does require the use of GetInstance from the global
system instance to be used within hle_ipc.cpp and the gdbstub. This is
fine for the time being, as they both already rely on the global system
instance in other functions. These will be removed in a change directed
at both of these respectively.
For now, it's sufficient, as it still accomplishes the goal of
de-globalizing the memory code.
Amends a few interfaces to be able to handle the migration over to the
new Memory class by passing the class by reference as a function
parameter where necessary.
Notably, within the filesystem services, this eliminates two ReadBlock()
calls by using the helper functions of HLERequestContext to do that for
us.
A fairly straightforward migration. These member functions can just be
mostly moved verbatim with minor changes. We already have the necessary
plumbing in places that they're used.
IsKernelVirtualAddress() can remain a non-member function, since it
doesn't rely on class state in any form.
Migrates all of the direct mapping facilities over to the new memory
class. In the process, this also obsoletes the need for memory_setup.h,
so we can remove it entirely from the project.
* core_timing: Use better reference tracking for EventType.
- Moves ownership of the event to the caller, ensuring we don't fire events for destroyed objects.
- Removes need for unique names - we won't be using this for save states anyways.
This commit ensures cond var threads act exactly as they do in the real
console. The original implementation uses an RBTree and the behavior of
cond var threads is that at the same priority level they act like a
FIFO.
This commit corrects the behavior of cancel synchronization when the
thread is running/ready and ensures the next wait is cancelled as it's
suppose to.
Uncovered a bug within Thread's SetCoreAndAffinityMask() where an
unsigned variable (ideal_core) was being compared against "< 0", which
would always be a false condition.
We can also get rid of an unused function (GetNextProcessorId) which contained a sign
mismatch warning.
- This does not actually seem to exist in the real kernel - games reset these automatically.
# Conflicts:
# src/core/hle/service/am/applets/applets.cpp
# src/core/hle/service/filesystem/fsp_srv.cpp
In case of redundant yields, the scheduler will now idle the core for
it's timeslice, in order to avoid continuously yielding the same thing
over and over.
This only encourages the use of the global system instance (which will
be phased out long-term). Instead, we use the direct system function
call directly to remove the appealing but discouraged short-hand.
If an unmapping operation fails, we shouldn't be decrementing the amount
of memory mapped and returning that the operation was successful. We
should actually be returning the error code in this case.
Avoids potentially expensive (depending on the size of the memory block)
allocations by reserving the necessary memory before performing both
insertions. This avoids scenarios where the second insert may cause a
reallocation to occur.
Avoids needing to read the same long sequence of code in both code
paths. Also makes it slightly nicer to read and debug, as the locals
will be able to be shown in the debugger.
This commit ensures that all backing memory allocated for the Guest CPU
is aligned to 256 bytes. This due to how gpu memory works and the heavy
constraints it has in the alignment of physical memory.
This was initially necessary when AArch64 JIT emulation was in its
infancy and all memory-related instructions weren't implemented.
Given the JIT now has all of these facilities implemented, we can remove
these functions from the CPU interface.
Prior to PR, Yuzu did not restore memory to RW-
on unmap of mirrored memory or unloading of NRO.
(In fact, in the NRO case, the memory was unmapped
instead of reprotected to --- on Load, so it was
actually lost entirely...)
This PR addresses that, and restores memory to RW-
as it should.
This fixes a crash in Super Smash Bros when creating
a World of Light save for the first time, and possibly
other games/circumstances.
This sets the DeviceMapped attribute for GPU-mapped memory blocks,
and prevents merging device mapped blocks. This prevents memory
mapped from the gpu from having its backing address changed by
block coalesce.
This implements svcMapPhysicalMemory/svcUnmapPhysicalMemory for Yuzu,
which can be used to map memory at a desired address by games since
3.0.0.
It also properly parses SystemResourceSize from NPDM, and makes
information available via svcGetInfo.
This is needed for games like Super Smash Bros. and Diablo 3 -- this
PR's implementation does not run into the "ASCII reads" issue mentioned
in the comments of #2626, which was caused by the following bugs in
Yuzu's memory management that this PR also addresses:
* Yuzu's memory coalescing does not properly merge blocks. This results
in a polluted address space/svcQueryMemory results that would be
impossible to replicate on hardware, which can lead to game code making
the wrong assumptions about memory layout.
* This implements better merging for AllocatedMemoryBlocks.
* Yuzu's implementation of svcMirrorMemory unprotected the entire
virtual memory range containing the range being mirrored. This could
lead to games attempting to map data at that unprotected
range/attempting to access that range after yuzu improperly unmapped
it.
* This PR fixes it by simply calling ReprotectRange instead of
Reprotect.
Prior to execution within a process beginning, the process establishes
its own TLS region for uses (as far as I can tell) related to exception
handling.
Now that TLS creation was decoupled from threads themselves, we can add
this behavior to our Process class. This is also good, as it allows us
to remove a stub within svcGetInfo, namely querying the address of that
region.
Provides a more accurate name for the memory region and also
disambiguates between the map and new map regions of memory, making it
easier to understand.
Handles the placement of the stack a little nicer compared to the
previous code, which was off in a few ways. e.g.
The stack (new map) region, shouldn't be the width of the entire address
space if the size of the region calculation ends up being zero. It
should be placed at the same location as the TLS IO region and also have
the same size.
In the event the TLS IO region contains a size of zero, we should also
be doing the same thing. This fixes our memory layout a little bit and
also resolves some cases where assertions can trigger due to the memory
layout being incorrect.
Extracts out all of the thread local storage management from thread
instances themselves and makes the owning process handle the management
of the memory. This brings the memory management slightly more in line
with how the kernel handles these allocations.
Furthermore, this also makes the TLS page management a little more
readable compared to the lingering implementation that was carried over
from Citra.
This will be necessary for making our TLS slot management slightly more
straightforward. This can also be utilized for other purposes in the
future.
We can implement the existing simpler overload in terms of this one
anyways, we just pass the beginning and end of the ASLR region as the
boundaries.
The old implementation had faulty Threadsafe methods where events could
be missing. This implementation unifies unsafe/safe methods and makes
core timing thread safe overall.
This is performing more work than would otherwise be necessary during
VMManager's destruction. All we actually want to occur in this scenario
is for any allocated memory to be freed, which will happen automatically
as the VMManager instance goes out of scope.
Anything else being done is simply unnecessary work.
Given we don't currently implement the personal heap yet, the existing
memory querying functions are essentially doing what the memory querying
types introduced in 6.0.0 do.
So, we can build the necessary machinery over the top of those and just
use them as part of info types.
These are only used from within this translation unit, so they don't
need to have external linkage. They were intended to be marked with this
anyways to be consistent with the other service functions.
Renames the members to more accurately indicate what they signify.
"OneShot" and "Sticky" are kind of ambiguous identifiers for the reset
types, and can be kind of misleading. Automatic and Manual communicate
the kind of reset type in a clearer manner. Either the event is
automatically reset, or it isn't and must be manually cleared.
The "OneShot" and "Sticky" terminology is just a hold-over from Citra
where the kernel had a third type of event reset type known as "Pulse".
Given the Switch kernel only has two forms of event reset types, we
don't need to keep the old terminology around anymore.
This reduces the boilerplate that services have to write out the current thread explicitly. Using current thread instead of client thread is also semantically incorrect, and will be a problem when we implement multicore (at which time there will be multiple current threads)
These are actually quite important indicators of thread lifetimes, so
they should be going into the debug log, rather than being treated as
misc info and delegated to the trace log.
Makes the code much nicer to follow in terms of behavior and control
flow. It also fixes a few bugs in the implementation.
Notably, the thread's owner process shouldn't be accessed in order to
retrieve the core mask or ideal core. This should be done through the
current running process. The only reason this bug wasn't encountered yet
is because we currently only support running one process, and thus every
owner process will be the current process.
We also weren't checking against the process' CPU core mask to see if an
allowed core is specified or not.
With this out of the way, it'll be less noisy to implement proper
handling of the affinity flags internally within the kernel thread
instances.
This is a holdover from Citra, where the 3DS has both
WaitSynchronization1 and WaitSynchronizationN. The switch only has one
form of wait synchronizing (literally WaitSynchonization). This allows
us to throw out code that doesn't apply at all to the Switch kernel.
Because of this unnecessary dichotomy within the wait synchronization
utilities, we were also neglecting to properly handle waiting on
multiple objects.
While we're at it, we can also scrub out any lingering references to
WaitSynchronization1/WaitSynchronizationN in comments, and change them
to WaitSynchronization (or remove them if the mention no longer
applies).
The actual behavior of this function is slightly more complex than what
we're currently doing within the supervisor call. To avoid dumping most
of this behavior in the supervisor call itself, we can migrate this to
another function.
This member variable is entirely unused. It was only set but never
actually utilized. Given that, we can remove it to get rid of noise in
the thread interface.
Essentially performs the inverse of svcMapProcessCodeMemory. This unmaps
the aliasing region first, then restores the general traits of the
aliased memory.
What this entails, is:
- Restoring Read/Write permissions to the VMA.
- Restoring its memory state to reflect it as a general heap memory region.
- Clearing the memory attributes on the region.
This gives us significantly more control over where in the
initialization process we start execution of the main process.
Previously we were running the main process before the CPU or GPU
threads were initialized (not good). This amends execution to start
after all of our threads are properly set up.
Initially required due to the split codepath with how the initial main
process instance was initialized. We used to initialize the process
like:
Init() {
main_process = Process::Create(...);
kernel.MakeCurrentProcess(main_process.get());
}
Load() {
const auto load_result = loader.Load(*kernel.GetCurrentProcess());
if (load_result != Loader::ResultStatus::Success) {
// Handle error here.
}
...
}
which presented a problem.
Setting a created process as the main process would set the page table
for that process as the main page table. This is fine... until we get to
the part that the page table can have its size changed in the Load()
function via NPDM metadata, which can dictate either a 32-bit, 36-bit,
or 39-bit usable address space.
Now that we have full control over the process' creation in load, we can
simply set the initial process as the main process after all the loading
is done, reflecting the potential page table changes without any
special-casing behavior.
We can also remove the cache flushing within LoadModule(), as execution
wouldn't have even begun yet during all usages of this function, now
that we have the initialization order cleaned up.
Our initialization process is a little wonky than one would expect when
it comes to code flow. We initialize the CPU last, as opposed to
hardware, where the CPU obviously needs to be first, otherwise nothing
else would work, and we have code that adds checks to get around this.
For example, in the page table setting code, we check to see if the
system is turned on before we even notify the CPU instances of a page
table switch. This results in dead code (at the moment), because the
only time a page table switch will occur is when the system is *not*
running, preventing the emulated CPU instances from being notified of a
page table switch in a convenient manner (technically the code path
could be taken, but we don't emulate the process creation svc handlers
yet).
This moves the threads creation into its own member function of the core
manager and restores a little order (and predictability) to our
initialization process.
Previously, in the multi-threaded cases, we'd kick off several threads
before even the main kernel process was created and ready to execute (gross!).
Now the initialization process is like so:
Initialization:
1. Timers
2. CPU
3. Kernel
4. Filesystem stuff (kind of gross, but can be amended trivially)
5. Applet stuff (ditto in terms of being kind of gross)
6. Main process (will be moved into the loading step in a following
change)
7. Telemetry (this should be initialized last in the future).
8. Services (4 and 5 should ideally be alongside this).
9. GDB (gross. Uses namespace scope state. Needs to be refactored into a
class or booted altogether).
10. Renderer
11. GPU (will also have its threads created in a separate step in a
following change).
Which... isn't *ideal* per-se, however getting rid of the wonky
intertwining of CPU state initialization out of this mix gets rid of
most of the footguns when it comes to our initialization process.
Some objects declare their handle type as const, while others declare it
as constexpr. This makes the const ones constexpr for consistency, and
prevent unexpected compilation errors if these happen to be attempted to be
used within a constexpr context.
We need to ensure dynarmic gets a valid pointer if the page table is
resized (the relevant pointers would be invalidated in this scenario).
In this scenario, the page table can be resized depending on what kind
of address space is specified within the NPDM metadata (if it's
present).
Adjusts the interface of the wrappers to take a system reference, which
allows accessing a system instance without using the global accessors.
This also allows getting rid of all global accessors within the
supervisor call handling code. While this does make the wrappers
themselves slightly more noisy, this will be further cleaned up in a
follow-up. This eliminates the global system accessors in the current
code while preserving the existing interface.
Keeps the return type consistent with the function name. While we're at
it, we can also reduce the amount of boilerplate involved with handling
these by using structured bindings.
We need to be checking whether or not the given address is within the
kernel address space or if the given address isn't word-aligned and bail
in these scenarios instead of trashing any kernel state.
Given server sessions can be given a name, we should allow retrieving
it instead of using the default implementation of GetName(), which would
just return "[UNKNOWN KERNEL OBJECT]".
The AddressArbiter type isn't actually used, given the arbiter itself
isn't a direct kernel object (or object that implements the wait object
facilities).
Given this, we can remove the enum entry entirely.
Similarly like svcGetProcessList, this retrieves the list of threads
from the current process. In the kernel itself, a process instance
maintains a list of threads, which are used within this function.
Threads are registered to a process' thread list at thread
initialization, and unregistered from the list upon thread destruction
(if said thread has a non-null owning process).
We assert on the debug event case, as we currently don't implement
kernel debug objects.
Now that ShouldWait() is a const qualified member function, this one can
be made const qualified as well, since it can handle passing a const
qualified this pointer to ShouldWait().
Previously this was performing a u64 + int sign conversion. When dealing
with addresses, we should generally be keeping the arithmetic in the
same signedness type.
This also gets rid of the static lifetime of the constant, as there's no
need to make a trivial type like this potentially live for the entire
duration of the program.
This doesn't really provide any benefit to the resource limit interface.
There's no way for callers to any of the service functions for resource
limits to provide a custom name, so all created instances of resource
limits other than the system resource limit would have a name of
"Unknown".
The system resource limit itself is already trivially identifiable from
its limit values, so there's no real need to take up space in the object to
identify one object meaningfully out of N total objects.
Since C++17, the introduction of deduction guides for locking facilities
means that we no longer need to hardcode the mutex type into the locks
themselves, making it easier to switch mutex types, should it ever be
necessary in the future.
Since C++17, we no longer need to explicitly specify the type of the
mutex within the lock_guard. The type system can now deduce these with
deduction guides.
The kernel makes sure that the given size to unmap is always the same
size as the entire region managed by the shared memory instance,
otherwise it returns an error code signifying an invalid size.
This is similarly done for transfer memory (which we already check for).
Reports the (mostly) correct size through svcGetInfo now for queries to
total used physical memory. This still doesn't correctly handle memory
allocated via svcMapPhysicalMemory, however, we don't currently handle
that case anyways.
This will make operating with the process-related SVC commands much
nicer in the future (the parameter representing the stack size in
svcStartProcess is a 64-bit value).
In some cases, our callbacks were using s64 as a parameter, and in other
cases, they were using an int, which is inconsistent.
To make all callbacks consistent, we can just use an s64 as the type for
late cycles, given it gets rid of the need to cast internally.
While we're at it, also resolve some signed/unsigned conversions that
were occurring related to the callback registration.
One behavior that we weren't handling properly in our heap allocation
process was the ability for the heap to be shrunk down in size if a
larger size was previously requested.
This adds the basic behavior to do so and also gets rid of HeapFree, as
it's no longer necessary now that we have allocations and deallocations
going through the same API function.
While we're at it, fully document the behavior that this function
performs.
Makes it more obvious that this function is intending to stand in for
the actual supervisor call itself, and not acting as a general heap
allocation function.
Also the following change will merge the freeing behavior of HeapFree
into this function, so leaving it as HeapAllocate would be misleading.
In cases where HeapAllocate is called with the same size of the current
heap, we can simply do nothing and return successfully.
This avoids doing work where we otherwise don't have to. This is also
what the kernel itself does in this scenario.
Another holdover from citra that can be tossed out is the notion of the
heap needing to be allocated in different addresses. On the switch, the
base address of the heap will always be managed by the memory allocator
in the kernel, so this doesn't need to be specified in the function's
interface itself.
The heap on the switch is always allocated with read/write permissions,
so we don't need to add specifying the memory permissions as part of the
heap allocation itself either.
This also corrects the error code returned from within the function.
If the size of the heap is larger than the entire heap region, then the
kernel will report an out of memory condition.
The use of a shared_ptr is an implementation detail of the VMManager
itself when mapping memory. Because of that, we shouldn't require all
users of the CodeSet to have to allocate the shared_ptr ahead of time.
It's intended that CodeSet simply pass in the required direct data, and
that the memory manager takes care of it from that point on.
This means we just do the shared pointer allocation in a single place,
when loading modules, as opposed to in each loader.
Makes it more evident that one is for actual code and one is for actual
data. Mutable and static are less than ideal terms here, because
read-only data is technically not mutable, but we were mapping it with
that label.
Given this is utilized by the loaders, this allows avoiding inclusion of
the kernel process definitions where avoidable.
This also keeps the loading format for all executable data separate from
the kernel objects.
Rather than make a global accessor for this sort of thing. We can make
it a part of the thread interface itself. This allows getting rid of a
hidden global accessor in the kernel code.
This condition was checking against the nominal thread priority, whereas
the kernel itself checks against the current priority instead. We were
also assigning the nominal priority, when we should be assigning
current_priority, which takes priority inheritance into account.
This can lead to the incorrect priority being assigned to a thread.
Given we recursively update the relevant threads, we don't need to go
through the whole mutex waiter list. This matches what the kernel does
as well (only accessing the first entry within the waiting list).
Makes it an instantiable class like it is in the actual kernel. This
will also allow removing reliance on global accessors in a following
change, now that we can encapsulate a reference to the system instance
in the class.
Within the kernel, shared memory and transfer memory facilities exist as
completely different kernel objects. They also have different validity
checking as well. Therefore, we shouldn't be treating the two as the
same kind of memory.
They also differ in terms of their behavioral aspect as well. Shared
memory is intended for sharing memory between processes, while transfer
memory is intended to be for transferring memory to other processes.
This breaks out the handling for transfer memory into its own class and
treats it as its own kernel object. This is also important when we
consider resource limits as well. Particularly because transfer memory
is limited by the resource limit value set for it.
While we currently don't handle resource limit testing against objects
yet (but we do allow setting them), this will make implementing that
behavior much easier in the future, as we don't need to distinguish
between shared memory and transfer memory allocations in the same place.
With this, all kernel objects finally have all of their data members
behind an interface, making it nicer to reason about interactions with
other code (as external code no longer has the freedom to totally alter
internals and potentially messing up invariants).
There's no real need to use a shared lifetime here, since we don't
actually expose them to anything else. This is also kind of an
unnecessary use of the heap given the objects themselves are so small;
small enough, in fact that changing over to optionals actually reduces
the overall size of the HLERequestContext struct (818 bytes to 808
bytes).
Now that we have the address arbiter extracted to its own class, we can
fix an innaccuracy with the kernel. Said inaccuracy being that there
isn't only one address arbiter. Each process instance contains its own
AddressArbiter instance in the actual kernel.
This fixes that and gets rid of another long-standing issue that could
arise when attempting to create more than one process.
Similar to how WaitForAddress was isolated to its own function, we can
also move the necessary conditional checking into the address arbiter
class itself, allowing us to hide the implementation details of it from
public use.
Rather than let the service call itself work out which function is the
proper one to call, we can make that a behavior of the arbiter itself,
so we don't need to directly expose those implementation details.
Places all of the functions for address arbiter operation into a class.
This will be necessary for future deglobalizing efforts related to both
the memory and system itself.
Removes a few inclusion dependencies from the headers or replaces
existing ones with ones that don't indirectly include the required
headers.
This allows removing an inclusion of core/memory.h, meaning that if the
memory header is ever changed in the future, it won't result in
rebuilding the entirety of the HLE services (as the IPC headers are used
quite ubiquitously throughout the HLE service implementations).
Avoids directly relying on the global system instance and instead makes
an arbitrary system instance an explicit dependency on construction.
This also allows removing dependencies on some global accessor functions
as well.
Given we already pass in a reference to the kernel that the shared
memory instance is created under, we can just use that to check the
current process, rather than using the global accessor functions.
This allows removing direct dependency on the system instance entirely.
The kernel allows restricting the total size of the handle table through
the process capability descriptors. Until now, this functionality wasn't
hooked up. With this, the process handle tables become properly restricted.
In the case of metadata-less executables, the handle table will assume
the maximum size is requested, preserving the behavior that existed
before these changes.
A fairly trivial change. Other sections of the codebase use nested
namespaces instead of separate namespaces here. This one must have just
been overlooked.
Gets rid of the largest set of mutable global state within the core.
This also paves a way for eliminating usages of GetInstance() on the
System class as a follow-up.
Note that no behavioral changes have been made, and this simply extracts
the functionality into a class. This also has the benefit of making
dependencies on the core timing functionality explicit within the
relevant interfaces.
Places all of the timing-related functionality under the existing Core
namespace to keep things consistent, rather than having the timing
utilities sitting in its own completely separate namespace.
A holdover from citra, the Horizon kernel on the switch has no
prominent kernel object that functions as a timer. At least not
to the degree of sophistication that this class provided.
As such, this can be removed entirely. This class also wasn't used at
all in any meaningful way within the core, so this was just code sitting
around doing nothing. This also allows removing a few things from the
main KernelCore class that allows it to use slightly less resources
overall (though very minor and not anything really noticeable).
No inheritors of the WaitObject class actually make use of their own
implementations of these functions, so they can be made non-virtual.
It's also kind of sketchy to allow overriding how the threads get added
to the list anyways, given the kernel itself on the actual hardware
doesn't seem to customize based off this.
Looking into the implementation of the C++ standard facilities that seem
to be within all modules, it appears that they use 7 as a break reason
to indicate an uncaught C++ exception.
This was primarily found via the third last function called within
Horizon's equivalent of libcxxabi's demangling_terminate_handler(),
which passes the value 0x80000007 to svcBreak.
This is a bounds check to ensure that the thread priority is within the
valid range of 0-64. If it exceeds 64, that doesn't necessarily mean
that an actual priority of 64 was expected (it actually means whoever
called the function screwed up their math).
Instead clarify the message to indicate the allowed range of thread
priorities.
Now that we handle the kernel capability descriptors we can correct
CreateThread to properly check against the core and priority masks
like the actual kernel does.
This makes the naming more closely match its meaning. It's just a
preferred core, not a required default core. This also makes the usages
of this term consistent across the thread and process implementations.
This function isn't a general purpose function that should be exposed to
everything, given it's specific to initializing the main thread for a
Process instance.
Given that, it's a tad bit more sensible to place this within
process.cpp, which keeps it visible only to the code that actually needs
it.
In all cases that these functions are needed, the VMManager can just be
retrieved and used instead of providing the same functions in Process'
interface.
This also makes it a little nicer dependency-wise, since it gets rid of
cases where the VMManager interface was being used, and then switched
over to using the interface for a Process instance. Instead, it makes
all accesses uniform and uses the VMManager instance for all necessary
tasks.
All the basic memory mapping functions did was forward to the Process'
VMManager instance anyways.
Similar to the service capability flags, however, we currently don't
emulate the GIC, so this currently handles all interrupts as being valid
for the time being.
Handles the priority mask and core mask flags to allow building up the
masks to determine the usable thread priorities and cores for a kernel
process instance.
We've had the old kernel capability parser from Citra, however, this is
unused code and doesn't actually map to how the kernel on the Switch
does it. This introduces the basic functional skeleton for parsing
process capabilities.
If a thread handle is passed to svcGetProcessId, the kernel attempts to
access the process ID via the thread's instance's owning process.
Technically, this function should also be handling the kernel debug
objects as well, however we currently don't handle those kernel objects
yet, so I've left a note via a comment about it to remind myself when
implementing it in the future.
Starts the process ID counter off at 81, which is what the kernel itself
checks against internally when creating processes. It's actually
supposed to panic if the PID is less than 81 for a userland process.
Adds the barebones enumeration constants and functions in place to
handle memory attributes, while also essentially leaving the attribute
itself non-functional.
In the previous change, the memory writing was moved into the service
function itself, however it still had a problem, in that the entire
MemoryInfo structure wasn't being written out, only the first 32 bytes
of it were being written out. We still need to write out the trailing
two reference count members and zero out the padding bits.
Not doing this can result in wrong behavior in userland code in the following
scenario:
MemoryInfo info; // Put on the stack, not quaranteed to be zeroed out.
svcQueryMemory(&info, ...);
if (info.device_refcount == ...) // Whoops, uninitialized read.
This can also cause the wrong thing to happen if the user code uses
std::memcmp to compare the struct, with another one (questionable, but
allowed), as the padding bits are not guaranteed to be a deterministic
value. Note that the kernel itself also fully zeroes out the structure
before writing it out including the padding bits.
Moves the memory writes directly into QueryProcessMemory instead of
letting the wrapper function do it. It would be inaccurate to allow the
handler to do it because there's cases where memory shouldn't even be
written to. For example, if the given process handle is invalid.
HOWEVER, if the memory writing is within the wrapper, then we have no
control over if these memory writes occur, meaning in an error case, 68
bytes of memory randomly get trashed with zeroes, 64 of those being
written to wherever the memory info address points to, and the remaining
4 being written wherever the page info address points to.
One solution in this case would be to just conditionally check within
the handler itself, but this is kind of smelly, given the handler
shouldn't be performing conditional behavior itself, it's a behavior of
the managed function. In other words, if you remove the handler from the
equation entirely, does the function still retain its proper behavior?
In this case, no.
Now, we don't potentially trash memory from this function if an invalid
query is performed.
This would result in svcSetMemoryAttribute getting the wrong value for
its third parameter. This is currently fine, given the service function
is stubbed, however this will be unstubbed in a future change, so this
needs to change.
The kernel returns a memory info instance with the base address set to
the end of the address space, and the size of said block as
0 - address_space_end, it doesn't set both of said members to zero.
Gets the two structures out of an unrelated header and places them with
the rest of the memory management code.
This also corrects the structures. PageInfo appears to only contain a
32-bit flags member, and the extra padding word in MemoryInfo isn't
necessary.
Amends the MemoryState enum to use the same values like the actual
kernel does. Also provides the necessary operators to operate on them.
This will be necessary in the future for implementing
svcSetMemoryAttribute, as memory block state is checked before applying
the attribute.
The Process object kept itself alive indefinitely because its handle_table
contains a SharedMemory object which owns a reference to the same Process object,
creating a circular ownership scenario.
Break that up by storing only a non-owning pointer in the SharedMemory object.
This was only ever public so that code could check whether or not a
handle was valid or not. Instead of exposing the object directly and
allowing external code to potentially mess with the map contents, we
just provide a member function that allows checking whether or not a
handle is valid.
This makes all member variables of the VMManager class private except
for the page table.
While partially correct, this service call allows the retrieved event to
be null, as it also uses the same handle to check if it was referring to
a Process instance. The previous two changes put the necessary machinery
in place to allow for this, so we can simply call those member functions
here and be done with it.
Process instances can be waited upon for state changes. This is also
utilized by svcResetSignal, which will be modified in an upcoming
change. This simply puts all of the WaitObject related machinery in
place.
svcResetSignal relies on the event instance to have already been
signaled before attempting to reset it. If this isn't the case, then an
error code has to be returned.
This function simply does a handle table lookup for a writable event
instance identified by the given handle value. If a writable event
cannot be found for the given handle, then an invalid handle error is
returned. If a writable event is found, then it simply signals the
event, as one would expect.
svcCreateEvent operates by creating both a readable and writable event
and then attempts to add both to the current process' handle table.
If adding either of the events to the handle table fails, then the
relevant error from the handle table is returned.
If adding the readable event after the writable event to the table
fails, then the writable event is removed from the handle table and the
relevant error from the handle table is returned.
Note that since we do not currently test resource limits, we don't check
the resource limit table yet.
Two kernel object should absolutely never have the same handle ID type.
This can cause incorrect behavior when it comes to retrieving object
types from the handle table. In this case it allows converting a
WritableEvent into a ReadableEvent and vice-versa, which is undefined
behavior, since the object types are not the same.
This also corrects ClearEvent() to check both kernel types like the
kernel itself does.
The kernel uses the handle table of the current process to retrieve the
process that should be used to retrieve certain information. To someone
not familiar with the kernel, this might raise the question of "Ok,
sounds nice, but doesn't this make it impossible to retrieve information
about the current process?".
No, it doesn't, because HandleTable instances in the kernel have the
notion of a "pseudo-handle", where certain values allow the kernel to
lookup objects outside of a given handle table. Currently, there's only
a pseudo-handle for the current process (0xFFFF8001) and a pseudo-handle
for the current thread (0xFFFF8000), so to retrieve the current process,
one would just pass 0xFFFF8001 into svcGetInfo.
The lookup itself in the handle table would be something like:
template <typename T>
T* Lookup(Handle handle) {
if (handle == PSEUDO_HANDLE_CURRENT_PROCESS) {
return CurrentProcess();
}
if (handle == PSUEDO_HANDLE_CURRENT_THREAD) {
return CurrentThread();
}
return static_cast<T*>(&objects[handle]);
}
which, as is shown, allows accessing the current process or current
thread, even if those two objects aren't actually within the HandleTable
instance.
Our implementation of svcGetInfo was slightly incorrect in that we
weren't doing proper error checking everywhere. Instead, reorganize it
to be similar to how the kernel seems to do it.
More hardware accurate. On the actual system, there is a differentiation between the signaler and signalee, they form a client/server relationship much like ServerPort and ClientPort.
The opposite of the getter functions, this function sets the limit value
for a particular ResourceLimit resource category, with the restriction
that the new limit value must be equal to or greater than the current
resource value. If this is violated, then ERR_INVALID_STATE is returned.
e.g.
Assume:
current[Events] = 10;
limit[Events] = 20;
a call to this service function lowering the limit value to 10 would be
fine, however, attempting to lower it to 9 in this case would cause an
invalid state error.
This kernel service function is essentially the exact same as
svcGetResourceLimitLimitValue(), with the only difference being that it
retrieves the current value for a given resource category using the
provided resource limit handle, rather than retrieving the limiting
value of that resource limit instance.
Given these are exactly the same and only differ on returned values, we
can extract the existing code for svcGetResourceLimitLimitValue() to
handle both values.
This kernel service function retrieves the maximum allowable value for
a provided resource category for a given resource limit instance. Given
we already have the functionality added to the resource limit instance
itself, it's sufficient to just hook it up.
The error scenarios for this are:
1. If an invalid resource category type is provided, then ERR_INVALID_ENUM is returned.
2. If an invalid handle is provided, then ERR_INVALID_HANDLE is returned (bad thing goes in, bad thing goes out, as one would expect).
If neither of the above error cases occur, then the out parameter is
provided with the maximum limit value for the given category and success
is returned.
This function simply creates a ResourceLimit instance and attempts to
create a handle for it within the current process' handle table. If the
kernal fails to either create the ResourceLimit instance or create a
handle for the ResourceLimit instance, it returns a failure code
(OUT_OF_RESOURCE, and HANDLE_TABLE_FULL respectively). Finally, it exits
by providing the output parameter with the handle value for the
ResourceLimit instance and returning that it was successful.
Note: We do not return OUT_OF_RESOURCE because, if yuzu runs out of
available memory, then new will currently throw. We *could* allocate the
kernel instance with std::nothrow, however this would be inconsistent
with how all other kernel objects are currently allocated.
<random> isn't necesary directly within the header and can be placed in
the cpp file where its needed. Avoids propagating random generation
utilities via a header file.
Cleans out the citra/3DS-specific implementation details that don't
apply to the Switch. Sets the stage for implementing ResourceLimit
instances properly.
While we're at it, remove the erroneous checks within CreateThread() and
SetThreadPriority(). While these are indeed checked in some capacity,
they are not checked via a ResourceLimit instance.
In the process of moving out Citra-specifics, this also replaces the
system ResourceLimit instance's values with ones from the Switch.
Both member functions assume the passed in target process will not be
null. Instead of making this assumption implicit, we can change the
functions to be references and enforce this at the type-system level.
Makes the interface nicer to use in terms of 64-bit code, as it makes it
less likely for one to get truncation warnings (and also makes sense in
the context of the rest of the interface where 64-bit types are used for
sizes and offsets
Similar to PR 1706, which cleans up the error codes for the filesystem
code, but done for the kernel error codes. This removes the ErrCodes
namespace and specifies the errors directly. This also fixes up any
straggling lines of code that weren't using the named error codes where
applicable.
* svcBreak now dumps information from the debug buffer passed
info1 and info2 seem to somtimes hold an address to a buffer, this is usually 4 bytes or the size of the int and contains an error code. There's other circumstances where it can be something different so we hexdump these to examine them at a later date.
* Addressed comments
* get rid of boost::optional
* Remove optional references
* Use std::reference_wrapper for optional references
* Fix clang format
* Fix clang format part 2
* Adressed feedback
* Fix clang format and MacOS build
Now that we've gotten the innaccurate error codes out of the way, we can
finally toss away a bunch of these, trimming down the error codes to
ones that are actually used and knocking out two TODO comments.
All priority checks are supposed to occur before checking the validity
of the thread handle, we're also not supposed to return
ERR_NOT_AUTHORIZED here.
In the kernel, there isn't a singular handle table that everything gets
tossed into or used, rather, each process gets its own handle table that
it uses. This currently isn't an issue for us, since we only execute one
process at the moment, but we may as well get this out of the way so
it's not a headache later on.
This should be comparing against the queried process' vma_map, not the
current process'. The only reason this hasn't become an issue yet is we
currently only handle one process being active at any time.
Now that the changes clarifying the address spaces has been merged, we
can wrap the checks that the kernel performs when mapping shared memory
(and other forms of memory) into its own helper function and then use
those within MapSharedMemory and UnmapSharedMemory to complete the
sanitizing checks that are supposed to be done.
So, one thing that's puzzled me is why the kernel seemed to *not* use
the direct code address ranges in some cases for some service functions.
For example, in svcMapMemory, the full address space width is compared
against for validity, but for svcMapSharedMemory, it compares against
0xFFE00000, 0xFF8000000, and 0x7FF8000000 as upper bounds, and uses
either 0x200000 or 0x8000000 as the lower-bounds as the beginning of the
compared range. Coincidentally, these exact same values are also used in
svcGetInfo, and also when initializing the user address space, so this
is actually retrieving the ASLR extents, not the extents of the address
space in general.
This should help diagnose crashes easier and prevent many users thinking that a game is still running when in fact it's just an audio thread still running(this is typically not killed when svcBreak is hit since the game expects us to do this)
A fairly basic service function, which only appears to currently support
retrieving the process state. This also alters the ProcessStatus enum to
contain all of the values that a kernel process seems to be able of
reporting with regards to state.
These only exist to ferry data into a Process instance and end up going
out of scope quite early. Because of this, we can just make it a plain
struct for holding things and just std::move it into the relevant
function. There's no need to make this inherit from the kernel's Object
type.
Regular value initialization is adequate here for zeroing out data. It
also has the benefit of not invoking undefined behavior if a non-trivial
type is ever added to the struct for whatever reason.
