Address feedback

app/src/main/cpp/skyline/common/address_space.h

@@ -10,23 +10,28 @@ namespace skyline {
     template<typename VaType, size_t AddressSpaceBits>
     concept AddressSpaceValid = std::is_unsigned_v<VaType> && sizeof(VaType) * 8 >= AddressSpaceBits;
 
+    struct EmptyStruct {};
+
     /**
      * @brief FlatAddressSpaceMap provides a generic VA->PA mapping implementation using a sorted vector
      */
-    template<typename VaType, VaType UnmappedVa, typename PaType, PaType UnmappedPa, bool PaContigSplit, size_t AddressSpaceBits> requires AddressSpaceValid<VaType, AddressSpaceBits>
+    template<typename VaType, VaType UnmappedVa, typename PaType, PaType UnmappedPa, bool PaContigSplit, size_t AddressSpaceBits, typename ExtraBlockInfo = EmptyStruct> requires AddressSpaceValid<VaType, AddressSpaceBits>
-    extern class FlatAddressSpaceMap {
+    class FlatAddressSpaceMap {
       private:
+        std::function<void(VaType, VaType)> unmapCallback{}; //!< Callback called when the mappings in an region have changed
+
+      protected:
         /**
          * @brief Represents a block of memory in the AS, the physical mapping is contiguous until another block with a different phys address is hit
          */
         struct Block {
             VaType virt{UnmappedVa}; //!< VA of the block
             PaType phys{UnmappedPa}; //!< PA of the block, will increase 1-1 with VA until a new block is encountered
-            bool flag{}; //!< General purpose flag for use by derived classes
+            [[no_unique_address]] ExtraBlockInfo extraInfo;
 
             Block() = default;
 
-            Block(VaType virt, PaType phys, bool flag) : virt(virt), phys(phys), flag(flag) {}
+            Block(VaType virt, PaType phys, ExtraBlockInfo extraInfo) : virt(virt), phys(phys), extraInfo(extraInfo) {}
 
             constexpr bool Valid() {
                 return virt != UnmappedVa;
@@ -45,15 +50,14 @@ namespace skyline {
             }
         };
 
-      protected:
         std::mutex blockMutex;
         std::vector<Block> blocks{Block{}};
 
         /**
-         * @brief Maps a PA range into the given AS region, optionally setting the flag
+         * @brief Maps a PA range into the given AS region
          * @note blockMutex MUST be locked when calling this
          */
-        void MapLocked(VaType virt, PaType phys, VaType size, bool flag = {});
+        void MapLocked(VaType virt, PaType phys, VaType size, ExtraBlockInfo extraInfo);
 
         /**
          * @brief Unmaps the given range and merges it with other unmapped regions
@@ -66,13 +70,13 @@ namespace skyline {
 
         VaType vaLimit{VaMaximum}; //!< A soft limit on the maximum VA of the AS
 
-        FlatAddressSpaceMap(VaType pVaLimit);
+        FlatAddressSpaceMap(VaType vaLimit, std::function<void(VaType, VaType)> unmapCallback = {});
 
         FlatAddressSpaceMap() = default;
 
-        void Map(VaType virt, PaType phys, VaType size, bool flag = {}) {
+        void Map(VaType virt, PaType phys, VaType size, ExtraBlockInfo extraInfo = {}) {
             std::scoped_lock lock(blockMutex);
-            MapLocked(virt, phys, size, flag);
+            MapLocked(virt, phys, size, extraInfo);
         }
 
         void Unmap(VaType virt, VaType size) {
@@ -82,11 +86,26 @@ namespace skyline {
     };
 
     /**
-     * @brief FlatMemoryManager specialises FlatAddressSpaceMap to focus on pointers as PAs, adding read/write functions
+     * @brief Hold memory manager specific block info
+     */
+    struct MemoryManagerBlockInfo {
+        bool sparseMapped;
+    };
+
+    /**
+     * @brief FlatMemoryManager specialises FlatAddressSpaceMap to focus on pointers as PAs, adding read/write functions and sparse mapping support
      */
     template<typename VaType, VaType UnmappedVa, size_t AddressSpaceBits> requires AddressSpaceValid<VaType, AddressSpaceBits>
-    class FlatMemoryManager : public FlatAddressSpaceMap<VaType, UnmappedVa, u8 *, nullptr, true, AddressSpaceBits> {
+    class FlatMemoryManager : public FlatAddressSpaceMap<VaType, UnmappedVa, u8 *, nullptr, true, AddressSpaceBits, MemoryManagerBlockInfo> {
+      private:
+        static constexpr u64 SparseMapSize{0x400000000}; //!< 16GiB pool size for sparse mappings returned by TranslateRange, this number is arbritary and should be large enough to fit the largest sparse mapping in the AS
+        u8 *sparseMap; //!< Pointer to a zero filled memory region that is returned by TranslateRange for sparse mappings
+
       public:
+        FlatMemoryManager();
+
+        ~FlatMemoryManager();
+
         /**
          * @return A placeholder address for sparse mapped regions, this means nothing
          */
@@ -94,6 +113,11 @@ namespace skyline {
             return reinterpret_cast<u8 *>(0xCAFEBABE);
         }
 
+        /**
+         * @brief Returns a vector of all physical ranges inside of the given virtual range
+         */
+        std::vector<span<u8>> TranslateRange(VaType virt, VaType size);
+
         void Read(u8 *destination, VaType virt, VaType size);
 
         template<typename T>

app/src/main/cpp/skyline/common/address_space.inc

@@ -5,19 +5,21 @@
 #include <kernel/types/KProcess.h>
 #include "address_space.h"
 
-#define MAP_MEMBER(returnType) template<typename VaType, VaType UnmappedVa, typename PaType, PaType UnmappedPa, bool PaContigSplit, size_t AddressSpaceBits> requires AddressSpaceValid<VaType, AddressSpaceBits> returnType FlatAddressSpaceMap<VaType, UnmappedVa, PaType, UnmappedPa, PaContigSplit, AddressSpaceBits>
+#define MAP_MEMBER(returnType) template<typename VaType, VaType UnmappedVa, typename PaType, PaType UnmappedPa, bool PaContigSplit, size_t AddressSpaceBits, typename ExtraBlockInfo> requires AddressSpaceValid<VaType, AddressSpaceBits> returnType FlatAddressSpaceMap<VaType, UnmappedVa, PaType, UnmappedPa, PaContigSplit, AddressSpaceBits, ExtraBlockInfo>
 
 #define MM_MEMBER(returnType) template<typename VaType, VaType UnmappedVa, size_t AddressSpaceBits> requires AddressSpaceValid<VaType, AddressSpaceBits> returnType FlatMemoryManager<VaType, UnmappedVa, AddressSpaceBits>
 
 #define ALLOC_MEMBER(returnType) template<typename VaType, VaType UnmappedVa, size_t AddressSpaceBits> requires AddressSpaceValid<VaType, AddressSpaceBits> returnType FlatAllocator<VaType, UnmappedVa, AddressSpaceBits>
 
 namespace skyline {
-    MAP_MEMBER()::FlatAddressSpaceMap(VaType pVaLimit) : vaLimit(pVaLimit) {
+    MAP_MEMBER()::FlatAddressSpaceMap(VaType vaLimit, std::function<void(VaType, VaType)> unmapCallback) :
-        if (pVaLimit > VaMaximum)
+        vaLimit(vaLimit),
+        unmapCallback(std::move(unmapCallback)) {
+        if (vaLimit > VaMaximum)
             throw exception("Invalid VA limit!");
     }
 
-    MAP_MEMBER(void)::MapLocked(VaType virt, PaType phys, VaType size, bool flag) {
+    MAP_MEMBER(void)::MapLocked(VaType virt, PaType phys, VaType size, ExtraBlockInfo extraInfo) {
         TRACE_EVENT("containers", "FlatAddressSpaceMap::Map");
 
         VaType virtEnd{virt + size};
@@ -45,10 +47,13 @@ namespace skyline {
                     // If this block's start would be overlapped by the map then reuse it as a tail block
                     blockEndPredecessor->virt = virtEnd;
                     blockEndPredecessor->phys = tailPhys;
-                    blockEndPredecessor->flag = blockEndPredecessor->flag;
+                    blockEndPredecessor->extraInfo = blockEndPredecessor->extraInfo;
                 } else {
                     // Else insert a new one and we're done
-                    blocks.insert(blockEndSuccessor, {Block(virt, phys, flag), Block(virtEnd, tailPhys, blockEndPredecessor->flag)});
+                    blocks.insert(blockEndSuccessor, {Block(virt, phys, extraInfo), Block(virtEnd, tailPhys, blockEndPredecessor->extraInfo)});
+                    if (unmapCallback)
+                        unmapCallback(virt, size);
+
                     return;
                 }
             }
@@ -59,7 +64,10 @@ namespace skyline {
                 blockEndPredecessor->virt = virtEnd;
             } else {
                 // Else insert a new one and we're done
-                blocks.insert(blockEndSuccessor, {Block(virt, phys, flag), Block(virtEnd, UnmappedPa, false)});
+                blocks.insert(blockEndSuccessor, {Block(virt, phys, extraInfo), Block(virtEnd, UnmappedPa, {})});
+                if (unmapCallback)
+                    unmapCallback(virt, size);
+
                 return;
             }
         }
@@ -75,13 +83,12 @@ namespace skyline {
             throw exception("Unsorted block in AS map: virt: 0x{:X}", blockStartSuccessor->virt);
         } else if (blockStartSuccessor->virt == virtEnd) {
             // We need to create a new block as there are none spare that we would overwrite
-            blocks.insert(blockStartSuccessor, Block(virt, phys, flag));
+            blocks.insert(blockStartSuccessor, Block(virt, phys, extraInfo));
-            return;
         } else {
             // Reuse a block that would otherwise be overwritten as a start block
             blockStartSuccessor->virt = virt;
             blockStartSuccessor->phys = phys;
-            blockStartSuccessor->flag = flag;
+            blockStartSuccessor->extraInfo = extraInfo;
 
             // Erase overwritten blocks
             if (auto eraseStart{std::next(blockStartSuccessor)}; blockStartSuccessor != blockEndPredecessor) {
@@ -91,6 +98,9 @@ namespace skyline {
                 blocks.erase(eraseStart, blockEndPredecessor);
             }
         }
+
+        if (unmapCallback)
+            unmapCallback(virt, size);
     }
 
     MAP_MEMBER(void)::UnmapLocked(VaType virt, VaType size) {
@@ -141,9 +151,16 @@ namespace skyline {
             if (blockEndPredecessor->virt > virt)
                 eraseBlocksWithEndUnmapped(blockEndPredecessor);
 
+            if (unmapCallback)
+                unmapCallback(virt, size);
+
             return; // The region is unmapped, bail out early
         } else if (blockEndSuccessor->virt == virtEnd && blockEndSuccessor->Unmapped()) {
             eraseBlocksWithEndUnmapped(blockEndSuccessor);
+
+            if (unmapCallback)
+                unmapCallback(virt, size);
+
             return; // The region is unmapped here and doesn't need splitting, bail out early
         } else if (blockEndSuccessor == blocks.end()) {
             // This should never happen as the end should always follow an unmapped block
@@ -164,8 +181,11 @@ namespace skyline {
                 blockEndPredecessor->virt = virtEnd;
                 blockEndPredecessor->phys = tailPhys;
             } else {
-                blocks.insert(blockEndSuccessor, {Block(virt, UnmappedPa, false), Block(virtEnd, tailPhys, blockEndPredecessor->flag)});
+                blocks.insert(blockEndSuccessor, {Block(virt, UnmappedPa, {}), Block(virtEnd, tailPhys, blockEndPredecessor->extraInfo)});
-                return; // The previous block is mapped and ends bef
+                if (unmapCallback)
+                    unmapCallback(virt, size);
+
+                return; // The previous block is mapped and ends before
             }
         }
 
@@ -180,7 +200,7 @@ namespace skyline {
 
             // The previous block is may be unmapped, if so we don't need to insert any unmaps after it
             if (blockStartPredecessor->Mapped())
-                blocks.insert(blockStartSuccessor, Block(virt, UnmappedPa, false));
+                blocks.insert(blockStartSuccessor, Block(virt, UnmappedPa, {}));
         } else if (blockStartPredecessor->Unmapped()) {
             // If the previous block is unmapped
             blocks.erase(blockStartSuccessor, blockEndPredecessor);
@@ -197,13 +217,67 @@ namespace skyline {
                 blocks.erase(eraseStart, blockEndPredecessor);
             }
         }
+
+        if (unmapCallback)
+            unmapCallback(virt, size);
+    }
+
+    MM_MEMBER()::FlatMemoryManager() {
+        sparseMap = static_cast<u8 *>(mmap(0, SparseMapSize, PROT_READ, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0));
+        if (!sparseMap)
+            throw exception("Failed to mmap sparse map!");
+    }
+
+    MM_MEMBER()::~FlatMemoryManager() {
+        munmap(sparseMap, SparseMapSize);
+    }
+
+    MM_MEMBER(std::vector<span<u8>>)::TranslateRange(VaType virt, VaType size) {
+        TRACE_EVENT("containers", "FlatMemoryManager::TranslateRange");
+
+        std::scoped_lock lock(this->blockMutex);
+
+        VaType virtEnd{virt + size};
+
+        auto successor{std::upper_bound(this->blocks.begin(), this->blocks.end(), virt, [] (auto virt, const auto &block) {
+            return virt < block.virt;
+        })};
+
+        auto predecessor{std::prev(successor)};
+
+        u8 *blockPhys{predecessor->phys + (virt - predecessor->virt)};
+        VaType blockSize{std::min(successor->virt - virt, size)};
+
+        std::vector<span<u8>> ranges;
+
+        while (size) {
+            // Return a zeroed out map to emulate sparse mappings
+            if (predecessor->extraInfo.sparseMapped) {
+                if (blockSize > SparseMapSize)
+                    throw exception("Size of the sparse map is too small to fit block of size: 0x{:X}", blockSize);
+
+                blockPhys = sparseMap;
+            }
+
+            ranges.push_back(span(blockPhys, blockSize));
+
+            size -= blockSize;
+
+            if (size) {
+                predecessor = successor++;
+                blockPhys = predecessor->phys;
+                blockSize = std::min(successor->virt - predecessor->virt, size);
+            }
+        }
+
+        return ranges;
     }
 
     MM_MEMBER(void)::Read(u8 *destination, VaType virt, VaType size) {
-        std::scoped_lock lock(this->blockMutex);
-
         TRACE_EVENT("containers", "FlatMemoryManager::Read");
 
+        std::scoped_lock lock(this->blockMutex);
+
         VaType virtEnd{virt + size};
 
         auto successor{std::upper_bound(this->blocks.begin(), this->blocks.end(), virt, [] (auto virt, const auto &block) {
@@ -218,12 +292,12 @@ namespace skyline {
         // Reads may span across multiple individual blocks
         while (size) {
             if (predecessor->phys == nullptr) {
-                if (predecessor->flag) // Sparse mapping
+                throw exception("Page fault at 0x{:X}", predecessor->virt);
+            } else {
+                if (predecessor->extraInfo.sparseMapped) // Sparse mappings read all zeroes
                     std::memset(destination, 0, blockReadSize);
                 else
-                    throw exception("Page fault at 0x{:X}", predecessor->virt);
+                    std::memcpy(destination, blockPhys, blockReadSize);
-            } else {
-                std::memcpy(destination, blockPhys, blockReadSize);
             }
 
             destination += blockReadSize;
@@ -238,10 +312,10 @@ namespace skyline {
     }
 
     MM_MEMBER(void)::Write(VaType virt, u8 *source, VaType size) {
-        std::scoped_lock lock(this->blockMutex);
-
         TRACE_EVENT("containers", "FlatMemoryManager::Write");
 
+        std::scoped_lock lock(this->blockMutex);
+
         VaType virtEnd{virt + size};
 
         auto successor{std::upper_bound(this->blocks.begin(), this->blocks.end(), virt, [] (auto virt, const auto &block) {
@@ -256,10 +330,10 @@ namespace skyline {
         // Writes may span across multiple individual blocks
         while (size) {
             if (predecessor->phys == nullptr) {
-                if (!predecessor->flag) // Sparse mappings allow unmapped writes
+                throw exception("Page fault at 0x{:X}", predecessor->virt);
-                    throw exception("Page fault at 0x{:X}", predecessor->virt);
             } else {
-                std::memcpy(blockPhys, source, blockWriteSize);
+                if (!predecessor->extraInfo.sparseMapped) // Sparse mappings ignore writes
+                    std::memcpy(blockPhys, source, blockWriteSize);
             }
 
             source += blockWriteSize;
@@ -277,10 +351,10 @@ namespace skyline {
     ALLOC_MEMBER()::FlatAllocator(VaType vaStart, VaType vaLimit) : Base(vaLimit), vaStart(vaStart), currentLinearAllocEnd(vaStart) {}
 
     ALLOC_MEMBER(VaType)::Allocate(VaType size) {
-        std::scoped_lock lock(this->blockMutex);
-
         TRACE_EVENT("containers", "FlatAllocator::Allocate");
 
+        std::scoped_lock lock(this->blockMutex);
+
         VaType allocStart{UnmappedVa};
         VaType allocEnd{currentLinearAllocEnd + size};
 
@@ -335,7 +409,7 @@ namespace skyline {
         }
 
 
-        this->MapLocked(allocStart, true, size);
+        this->MapLocked(allocStart, true, size, {});
         return allocStart;
     }
 

app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp

@@ -13,7 +13,7 @@ namespace skyline::kernel::type {
         if (fd < 0)
             throw exception("An error occurred while creating shared memory: {}", fd);
 
-        host.ptr = reinterpret_cast<u8 *>(mmap(nullptr, size, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED, fd, 0));
+        host.ptr = static_cast<u8 *>(mmap(nullptr, size, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED, fd, 0));
         if (host.ptr == MAP_FAILED)
             throw exception("An occurred while mapping shared memory: {}", strerror(errno));
 
@@ -28,7 +28,7 @@ namespace skyline::kernel::type {
         if (guest.Valid())
             throw exception("Mapping KSharedMemory multiple times on guest is not supported: Requested Mapping: 0x{:X} - 0x{:X} (0x{:X}), Current Mapping: 0x{:X} - 0x{:X} (0x{:X})", ptr, ptr + size, size, guest.ptr, guest.ptr + guest.size, guest.size);
 
-        guest.ptr = reinterpret_cast<u8 *>(mmap(ptr, size, permission.Get(), MAP_SHARED | (ptr ? MAP_FIXED : 0), fd, 0));
+        guest.ptr = static_cast<u8 *>(mmap(ptr, size, permission.Get(), MAP_SHARED | (ptr ? MAP_FIXED : 0), fd, 0));
         if (guest.ptr == MAP_FAILED)
             throw exception("An error occurred while mapping shared memory in guest: {}", strerror(errno));
         guest.size = size;

app/src/main/cpp/skyline/services/nvdrv/core/nvmap.cpp

@@ -108,10 +108,10 @@ namespace skyline::service::nvdrv::core {
 
             if (internalSession) {
                 if (--handleDesc->internalDupes < 0)
-                    state.logger->Warn("Internal duplicate count inbalance detected!");
+                    state.logger->Warn("Internal duplicate count imbalance detected!");
             } else {
                 if (--handleDesc->dupes < 0) {
-                    state.logger->Warn("User duplicate count inbalance detected!");
+                    state.logger->Warn("User duplicate count imbalance detected!");
                 } else if (handleDesc->dupes == 0) {
                     // TODO: unpin
                 }

app/src/main/cpp/skyline/services/nvdrv/devices/nvhost/as_gpu.cpp

@@ -53,7 +53,7 @@ namespace skyline::service::nvdrv::device::nvhost {
         u64 size{static_cast<u64>(pages) * pageSize};
 
         if (flags.sparse)
-            state.soc->gm20b.gmmu.Map(offset, GMMU::SparsePlaceholderAddress(), size, true);
+            state.soc->gm20b.gmmu.Map(offset, GMMU::SparsePlaceholderAddress(), size, {true});
 
         allocationMap[offset] = {
             .size = size,
@@ -77,7 +77,7 @@ namespace skyline::service::nvdrv::device::nvhost {
         // Sparse mappings shouldn't be fully unmapped, just returned to their sparse state
         // Only FreeSpace can unmap them fully
         if (mapping->sparseAlloc)
-            state.soc->gm20b.gmmu.Map(offset, GMMU::SparsePlaceholderAddress(), mapping->size, true);
+            state.soc->gm20b.gmmu.Map(offset, GMMU::SparsePlaceholderAddress(), mapping->size, {true});
         else
             state.soc->gm20b.gmmu.Unmap(offset, mapping->size);
 
@@ -138,7 +138,7 @@ namespace skyline::service::nvdrv::device::nvhost {
             // Sparse mappings shouldn't be fully unmapped, just returned to their sparse state
             // Only FreeSpace can unmap them fully
             if (mapping->sparseAlloc)
-                state.soc->gm20b.gmmu.Map(offset, GMMU::SparsePlaceholderAddress(), mapping->size, true);
+                state.soc->gm20b.gmmu.Map(offset, GMMU::SparsePlaceholderAddress(), mapping->size, {true});
             else
                 state.soc->gm20b.gmmu.Unmap(offset, mapping->size);
 
@@ -320,7 +320,7 @@ namespace skyline::service::nvdrv::device::nvhost {
             }
 
             if (!entry.handle) {
-                state.soc->gm20b.gmmu.Map(virtAddr, soc::gm20b::GM20B::GMMU::SparsePlaceholderAddress(), size, true);
+                state.soc->gm20b.gmmu.Map(virtAddr, soc::gm20b::GM20B::GMMU::SparsePlaceholderAddress(), size, {true});
             } else {
                 auto h{core.nvMap.GetHandle(entry.handle)};
                 if (!h)

app/src/main/cpp/skyline/services/nvdrv/types.h

@@ -43,7 +43,7 @@ namespace skyline::service::nvdrv {
     };
 
     /**
-     * @brief An bitfield struct that unpacks an ioctl number, used as an alternative to Linux's macros
+     * @brief A bitfield struct that unpacks an ioctl number, used as an alternative to Linux's macros
      */
     union IoctlDescriptor {
         struct {

app/src/main/cpp/skyline/soc/gm20b/engines/maxwell_3d.h

@@ -17,7 +17,7 @@ namespace skyline::soc::gm20b::engine::maxwell3d {
         std::array<size_t, 0x80> macroPositions{}; //!< The positions of each individual macro in macro memory, there can be a maximum of 0x80 macros at any one time
 
         struct {
-            i32 index;
+            i32 index{-1};
             std::vector<u32> arguments;
         } macroInvocation{}; //!< Data for a macro that is pending execution