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

Wrap some SysMemForKernel's nids, fixing #7960 #7965

Closed
wants to merge 9 commits into from

Conversation

sum2012
Copy link
Collaborator

@sum2012 sum2012 commented Sep 13, 2015

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 14, 2015

@hrydgard this commit have problem ?

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 14, 2015

OMG,If I include sceKernelMemory.cpp
1

As SysMemForUser in there
@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 15, 2015

I move SysMemForKernel to sceKernelMemory
As SysMemForUser in there

std::string name;
BlockAllocator alloc;
static int GetStaticIDType() { return SCE_KERNEL_TMID_Fpl; }// wrong
int GetIDType() const override { return SCE_KERNEL_TMID_Fpl; }// wrong
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I take a search but no SCE_KERNEL_TMID_HEAP

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

partitionId, name,flags,name not used
Not sure really need save status these variable

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't use the "XyzInformation" convention in PPSSPP, even if JPCSP does. I really think we should have testing to understand how this differs from sceHeap. Also, testing can identify which TMID is uses - there's a syscall that gives you the type id for a uid, iirc.

I can't remember for sure, but I thought we used the TMID for savestates, though. I don't think it's good if they're wrong.

-[Unknown]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So this pull request is not ready for v1.1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also yes,I copy most of code from sceHeap.cpp

Copy link
Owner

Choose a reason for hiding this comment

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

yeah, it won't make it. we can start adding some kernel stuff after 1.1 but for now I want to keep things the way they are to be safe.

1.1 will be soon though, I'm planning to release next weekend.

Copy link
Collaborator

Choose a reason for hiding this comment

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

See also PPSSPP_KERNEL_TMID_Module and similar for using fake TMID values. For savestates to work correctly, we need unique values right now.

-[Unknown]

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 16, 2015

Unfortunately,the screen is still black after HLE\sceKernelThread.cpp:2851 sceKernelSuspendThread(276): cannot suspend current thread

modify log:
https://gist.github.com/sum2012/04b57eb492018d02ca29

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 17, 2015

In the soft gpu,the menu appear now.
(But in-game crash even turn off fast memory)
1

@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 17, 2015

The crash is from sceKernelDeleteHeap...
I cannot lend code from sceheap as it doesn't use uid
2

As this commit is work
(tested on Tales of Phantasia with Chinese patch)
@sum2012
Copy link
Collaborator Author

sum2012 commented Sep 27, 2015

Just a note that my change is based on jpcsp/jpcsp@3c25717

@@ -2301,3 +2301,115 @@ void Register_SysMemUserForUser()
{
RegisterModule("SysMemUserForUser", ARRAY_SIZE(SysMemUserForUser), SysMemUserForUser);
}


struct HeapInformation : public KernelObject {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should:

  1. Put these functions and structs in sceKernelHeap.cpp.
  2. Keep SysMemForKernel here and export the functions in sceKernelHeap.h (kinda like sceKernelMutex.h, etc.)

-[Unknown]

@hrydgard
Copy link
Owner

hrydgard commented Dec 1, 2016

This will need some rebasing and cleanup work but seems worthwhile if it gets Chinese language patches going, with not too much trouble.

@sum2012 do you intend to improve this some more anytime soon? Otherwise I might clean it up a bit soon for you, but you'll have to test it.

@hrydgard hrydgard added this to the Future milestone Dec 1, 2016
@hrydgard hrydgard changed the title Wrap some SysMemForKernel's nids Wrap some SysMemForKernel's nids, fixing #7960 Dec 1, 2016
@sum2012
Copy link
Collaborator Author

sum2012 commented Dec 1, 2016

@hrydgard I remember this code not more work , maybe I transate something wrong
with jpcsp/jpcsp@3c25717

@hrydgard
Copy link
Owner

hrydgard commented Jun 6, 2017

Hm. Either way it needs a rebase if we should keep it open. Close or not?

@sum2012
Copy link
Collaborator Author

sum2012 commented Jun 7, 2017

I would rebase it in Sunday in Hong Kong time

@hrydgard
Copy link
Owner

hrydgard commented Jun 7, 2017

Oh, there's absolutely no rush. Whenever you have time. 1.5 is still far away..

@sum2012
Copy link
Collaborator Author

sum2012 commented Jun 11, 2017

1
FPL is from sceKernelMemory,I don't know how to solve
kernelObjects.Destroy(heap->uid); not work ether

@hrydgard hrydgard removed this from the Future milestone Mar 3, 2019
@hrydgard
Copy link
Owner

hrydgard commented Aug 8, 2019

Closing this in favor of #12225.

Sorry for the long delay ;)

@hrydgard hrydgard closed this Aug 8, 2019
@hrydgard hrydgard removed this from the v1.9.0 milestone Apr 26, 2020
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.

3 participants