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.
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.
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.
- 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.
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 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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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.
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).
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.
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.
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.
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.
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.
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.
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.
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.
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.
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
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.
This adds the missing address range checking that the service functions
do before attempting to map or unmap memory. Given that both service
functions perform the same set of checks in the same order, we can wrap
these into a function and just call it from both functions, which
deduplicates a little bit of code.
There's no real need to use a shared pointer in these cases, and only
makes object management more fragile in terms of how easy it would be to
introduce cycles. Instead, just do the simple thing of using a regular
pointer. Much of this is just a hold-over from citra anyways.
It also doesn't make sense from a behavioral point of view for a
process' thread to prolong the lifetime of the process itself (the
process is supposed to own the thread, not the other way around).
When loading NROs, svcBreak is called to signal to the debugger that a new "module" is loaded. As no debugger is technically attached we shouldn't be killing the programs execution.
Many of the member variables of the thread class aren't even used
outside of the class itself, so there's no need to make those variables
public. This change follows in the steps of the previous changes that
made other kernel types' members private.
The main motivation behind this is that the Thread class will likely
change in the future as emulation becomes more accurate, and letting
random bits of the emulator access data members of the Thread class
directly makes it a pain to shuffle around and/or modify internals.
Having all data members public like this also makes it difficult to
reason about certain bits of behavior without first verifying what parts
of the core actually use them.
Everything being public also generally follows the tendency for changes
to be introduced in completely different translation units that would
otherwise be better introduced as an addition to the Thread class'
public interface.
Now that we have all of the rearranging and proper structure sizes in
place, it's fairly trivial to implement svcGetThreadContext(). In the
64-bit case we can more or less just write out the context as is, minus
some minor value sanitizing. In the 32-bit case we'll need to clear out
the registers that wouldn't normally be accessible from a 32-bit
AArch32 exectuable (or process).
Makes the public interface consistent in terms of how accesses are done
on a process object. It also makes it slightly nicer to reason about the
logic of the process class, as we don't want to expose everything to
external code.
The locations of these can actually vary depending on the address space
layout, so we shouldn't be using these when determining where to map
memory or be using them as offsets for calculations. This keeps all the
memory ranges flexible and malleable based off of the virtual memory
manager instance state.
Previously, these were reporting hardcoded values, but given the regions
can change depending on the requested address spaces, these need to
report the values that the memory manager contains.
The kernel does the equivalent of the following check before proceeding:
if (address + 0x8000000000 < 0x7FFFE00000) {
return ERR_INVALID_MEMORY_STATE;
}
which is essentially what our IsKernelVirtualAddress() function does. So
we should also be checking for this.
The kernel also checks if the given input addresses are 4-byte aligned,
however our Mutex::TryAcquire() and Mutex::Release() functions already
handle this, so we don't need to add code for this case.
The kernel caps the size limit of shared memory to 8589930496 bytes (or
(1GB - 512 bytes) * 8), so approximately 8GB, where every GB has a 512
byte sector taken off of it.
It also ensures the shared memory is created with either read or
read/write permissions for both permission types passed in, allowing the
remote permissions to also be set as "don't care".
Part of the checking done by the kernel is to check if the given
address and size are 4KB aligned, as well as checking if the size isn't
zero. It also only allows mapping shared memory as readable or
read/write, but nothing else, and so we shouldn't allow mapping as
anything else either.
While unlikely, it does avoid constructing a std::string and
unnecessarily calling into the memory code if a game or executable
decides to be really silly about their logging.
Now that we have a class representing the kernel in some capacity, we
now have a place to put the named port map, so we move it over and get
rid of another piece of global state within the core.
The follow-up to e2457418da, which
replaces most of the includes in the core header with forward declarations.
This makes it so that if any of the headers the core header was
previously including change, then no one will need to rebuild the bulk
of the core, due to core.h being quite a prevalent inclusion.
This should make turnaround for changes much faster for developers.
As means to pave the way for getting rid of global state within core,
This eliminates kernel global state by removing all globals. Instead
this introduces a KernelCore class which acts as a kernel instance. This
instance lives in the System class, which keeps its lifetime contained
to the lifetime of the System class.
This also forces the kernel types to actually interact with the main
kernel instance itself instead of having transient kernel state placed
all over several translation units, keeping everything together. It also
has a nice consequence of making dependencies much more explicit.
This also makes our initialization a tad bit more correct. Previously we
were creating a kernel process before the actual kernel was initialized,
which doesn't really make much sense.
The KernelCore class itself follows the PImpl idiom, which allows
keeping all the implementation details sealed away from everything else,
which forces the use of the exposed API and allows us to avoid any
unnecessary inclusions within the main kernel header.
We already have the variable itself set up to perform this task, so we
can just return its value from the currently executing process instead
of always stubbing it to zero.
The current core may have nothing to do with the core where the new thread was scheduled to run. In case it's the same core, then the following PrepareReshedule call will take care of that.
Exit from AddMutexWaiter early if the thread is already waiting for a mutex owned by the owner thread.
This accounts for the possibility of a thread that is waiting on a condition variable being awakened twice in a row.
Also added more validation asserts.
This should fix one of the random crashes in Breath Of The Wild.
The reason this would never be true is that ideal_processor is a u8 and
THREADPROCESSORID_DEFAULT is an s32. In this case, it boils down to how
arithmetic conversions are performed before performing the comparison.
If an unsigned value has a lesser conversion rank (aka smaller size)
than the signed type being compared, then the unsigned value is promoted
to the signed value (i.e. u8 -> s32 happens before the comparison). No
sign-extension occurs here either.
An alternative phrasing:
Say we have a variable named core and it's given a value of -2.
u8 core = -2;
This becomes 254 due to the lack of sign. During integral promotion to
the signed type, this still remains as 254, and therefore the condition
will always be true, because no matter what value the u8 is given it
will never be -2 in terms of 32 bits.
Now, if one type was a s32 and one was a u32, this would be entirely
different, since they have the same bit width (and the signed type would
be converted to unsigned instead of the other way around) but would
still have its representation preserved in terms of bits, allowing the
comparison to be false in some cases, as opposed to being true all the
time.
---
We also get rid of two signed/unsigned comparison warnings while we're
at it.
Fernando Sahmkow
bunnei
Lioncash
Michael Scire
Zach Hilman
Sebastian Valle
Sebastian Valle
David Marcec
Jens Schmer
Luke Street
V.Kalyuzhny
Tobias
David
Frederic Laing
Frederic Laing
tech4me
Valentin Vanelslande
James Rowe
