Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow plugins with heaps of 5 MiB or less to use PRIVATE memory instead of SHARED #2079

Closed
wants to merge 7 commits into from

Conversation

LittlestCube
Copy link
Contributor

@LittlestCube LittlestCube commented Sep 13, 2024

Temporary fix to plugins giving them a PRIVATE heap for use with svcCreateMemoryBlock and other calls that expect MemState to be PRIVATE. This opens the door for many plugin projects that are blocked since they cannot use these svc calls.

This fixes #1777 and #2078.

To be clear, I do not necessarily expect this to be merged anytime soon (if at all) but I thought it would be nice to show that the fix really does not require that many changes, even if it's a few more changes than ideal.

@PabloMK7
Copy link
Collaborator

Hello, this is not backwards compatible and will break every single software that uses map process memory ex (many plugins and other apps). A backwards compatible solution should be implemented.

About the memory state, a lot of tests will be needed to ensure it doesn't break anything.

@LittlestCube
Copy link
Contributor Author

Ah, thank you for letting me know other apps use that svc. I was hoping I wouldn't have to implement a new svc since I thought that would be messier, but perhaps that would be more worth doing.

I knew that the MK7 Deluxe plugin still worked (which is a 10 MiB plugin, so that was reassuring) but I'll be testing a lot with this to make sure it doesn't break anything else. If you could, can you give me an example of an application that uses svcMapProcessMemoryEx? That would help me test this further.

@LittlestCube
Copy link
Contributor Author

Well I made a new svc for it instead, which will solve the applications from breaking in the way you described. In terms of testing to make sure changing MemState doesn't break anything, fortunately me and some other people will be testing this a whole lot. So if any issues come up they will most certainly be fixed, or at least outlined here if fixing is not possible.

@PabloMK7
Copy link
Collaborator

@TuxSH what do you think of this? Backwards compatibility should be kept, but at the same time having a new SVC seems overkill. I think the same SVC could be kept, but add a magic number to a register to signal the kernel it's the new version.

@LittlestCube
Copy link
Contributor Author

I agree, if a new SVC can be avoided it probably should be. I can attempt the magic number idea.

@PabloMK7
Copy link
Collaborator

Also, do not add a boolean parameter, instead create a flags parameter and make an enum. That way it's more future proof.

@TuxSH
Copy link
Member

TuxSH commented Sep 17, 2024

Backwards compatibility should be kept

Yes, though only until I rewrite k11 (in a year or possibly more)

Ideally a backwards compatible options should be done, but it needs to not be (more) difficult to refactor when I eventually need to, either.

Agree it would be nice to avoid using up extra svc numbers


Result res = 0;

res = svcUnmapProcessMemoryEx(target, 0x07000000, header->exeSize);
res = svcMapProcessMemoryEx(target, header->heapVA, CUR_PROCESS_HANDLE, (u32) memblock->memblock + header->exeSize, header->heapSize, true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the logic behind this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/LittlestCube/Luma3DS/blob/48cf6596b16a62aa6fe09d4008d18fe1624a1dee/sysmodules/rosalina/source/plugin/memoryblock.c#L246-L249

Probably doesn't matter now, but two lines under you can see both original Unmaps were kept. The idea was to make sure it was still mapped the normal way with 0x5806 just in case Unmap would somehow break on PRIVATE memory.

Admittedly, perhaps a call to svcControlProcessMemory would have been more prudent, but I'm not sure.

@PabloMK7
Copy link
Collaborator

Superseeded by #2086 with the remaining changes.

@PabloMK7 PabloMK7 closed this Sep 27, 2024
@TuxSH
Copy link
Member

TuxSH commented Sep 27, 2024

@LittlestCube To add context, I am the one who was pushing to make a release. You will be credited regardless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[QUESTION] Networking in Rosalina plugins?
3 participants