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

VSH: Filesystem Mounting Enhancement #12917

Merged
merged 3 commits into from Nov 19, 2022
Merged

VSH: Filesystem Mounting Enhancement #12917

merged 3 commits into from Nov 19, 2022

Conversation

brian218
Copy link
Contributor

@brian218 brian218 commented Nov 2, 2022

Made the VSH willing to access /dev_usbXXX, which wasn't deemed mounted by VSH when the device wasn't in the list provided by sys_fs_get_mount_info().

For example, PS3UPDAT.PUP in /dev_usb000/PS3/UPDATE/ now can be detected by the VSH with this PR.
screenshot_2022_11_02_23_48_54

@Darkhost1999
Copy link
Contributor

Darkhost1999 commented Nov 3, 2022

I noticed a few things
dev_usb000/MUSIC
dev_usb000/PICTURE
dev_usb000/PS3/THEME
dev_usb000/VIDEO
While working on my PS3 doesn't work here. The usb doesn't even show up on xmb as connected.

The only path detected was dev_usb000/PS3/UPDATE and it appropriately returned the error that the pup version couldn't be read.

And while you're doing this PR. What about these other USBs? Why leave these out
dev_usb // (SYS_DEV_USB)
dev_usb000 // (CELL_FS_IOS:USB_MASS_STORAGE000)
dev_usb001 // (CELL_FS_IOS:USB_MASS_STORAGE001)
dev_usb002 // (CELL_FS_IOS:USB_MASS_STORAGE002)
dev_usb003 // (CELL_FS_IOS:USB_MASS_STORAGE003)
dev_usb004 // (CELL_FS_IOS:USB_MASS_STORAGE004)
dev_usb005 // (CELL_FS_IOS:USB_MASS_STORAGE005)

At the least usb mass storage 1-5 should be the same as 0 right? Can we add those because that's how it should be? Have we found out anything about sys_dev_usb?

@brian218
Copy link
Contributor Author

brian218 commented Nov 3, 2022

I noticed a few things dev_usb000/MUSIC dev_usb000/PICTURE dev_usb000/PS3/THEME dev_usb000/VIDEO While working on my PS3 doesn't work here. The usb doesn't even show up on xmb as connected.

The only path detected was dev_usb000/PS3/UPDATE and it appropriately returned the error that the pup version couldn't be read.

And while you're doing this PR. What about these other USBs? Why leave these out dev_usb // (SYS_DEV_USB) dev_usb000 // (CELL_FS_IOS:USB_MASS_STORAGE000) dev_usb001 // (CELL_FS_IOS:USB_MASS_STORAGE001) dev_usb002 // (CELL_FS_IOS:USB_MASS_STORAGE002) dev_usb003 // (CELL_FS_IOS:USB_MASS_STORAGE003) dev_usb004 // (CELL_FS_IOS:USB_MASS_STORAGE004) dev_usb005 // (CELL_FS_IOS:USB_MASS_STORAGE005)

At the least usb mass storage 1-5 should be the same as 0 right? Can we add those because that's how it should be? Have we found out anything about sys_dev_usb?

I can add the other dev_usb to the mount_info list of course, though I think we should eventually implement dynamic mount_info list reflecting what is actually mounted or unmounted by vfs::mount or vfs::unmount() in real time.

@brian218 brian218 changed the title VSH: Add /dev_usb000 to sys_fs_get_mount_info() VSH: Add /dev_usb000 ~ /dev_usb005 to sys_fs_get_mount_info() Nov 3, 2022
@brian218
Copy link
Contributor Author

brian218 commented Nov 3, 2022

@Darkhost1999 I've added the temporary workaround for /dev_usb000 to /dev_usb005.

@brian218
Copy link
Contributor Author

brian218 commented Nov 3, 2022

Managed to further implement sys_fs_get_mount_info() stuff so that it can reflect the actually mounted devices in real time.

@Darkhost1999
Copy link
Contributor

Your example for dev_usb/PS3/UPDATE is still valid
image
But dev_usb000/MUSIC
dev_usb000/PICTURE
dev_usb000/PS3/THEME
dev_usb000/VIDEO
aren't detected.
Here is for themes
image
Here is for Music/xmb
image
Why doesn't it show like
image
Is that achievable here or a future PR?

@Darkhost1999
Copy link
Contributor

There is a theme inside dev_flash/vsh/resource/theme you can use inside the dev_usb000/PS3/Theme path for testing if you don't have a .p3t

@brian218
Copy link
Contributor Author

brian218 commented Nov 3, 2022

Up to 8 USBs (from dev_usb000 to dev_usb007) are supported now.

@brian218 brian218 changed the title VSH: Add /dev_usb000 ~ /dev_usb005 to sys_fs_get_mount_info() VSH: Filesystem Mounting Enhancement Nov 3, 2022
@brian218
Copy link
Contributor Author

brian218 commented Nov 3, 2022

Your example for dev_usb/PS3/UPDATE is still valid image But dev_usb000/MUSIC dev_usb000/PICTURE dev_usb000/PS3/THEME dev_usb000/VIDEO aren't detected. Here is for themes image Here is for Music/xmb image Why doesn't it show like image Is that achievable here or a future PR?

Interesting. We can look into these in another PR in the future.
I think currently this PR can be reviewed and merged since it does fix something that didn't work at all (such as firmware check from USB).

rpcs3/Emu/Cell/lv2/sys_fs.h Outdated Show resolved Hide resolved
rpcs3/Emu/VFS.cpp Outdated Show resolved Hide resolved
rpcs3/Emu/VFS.cpp Outdated Show resolved Hide resolved
rpcs3/Emu/Cell/lv2/sys_fs.cpp Outdated Show resolved Hide resolved
rpcs3/Emu/Cell/lv2/sys_fs.cpp Outdated Show resolved Hide resolved
@brian218
Copy link
Contributor Author

brian218 commented Nov 5, 2022

After this PR, the arcade XMB can be fully accessible and operable.
screenshot_2022_11_04_22_49_25

@brian218
Copy link
Contributor Author

brian218 commented Nov 6, 2022

"Install Package Files" from USB also works smoothly.
screenshot_2022_11_04_23_09_37

@Darkhost1999
Copy link
Contributor

I thought that was confirmed working before this PR. Several months ago

@brian218
Copy link
Contributor Author

brian218 commented Nov 6, 2022

I thought that was confirmed working before this PR. Several months ago

I don't think so. You'll probably get error 80029567 and 80029564.

@brian218 brian218 requested review from elad335 and removed request for Megamouse November 9, 2022 10:33
Copy link
Contributor

@elad335 elad335 left a comment

Choose a reason for hiding this comment

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

I think it's safer to not use a member to tell if mounted, just vfs::get or some other indirect way to extract data from vfs. It's because it has issues when relainching emulation.

@elad335
Copy link
Contributor

elad335 commented Nov 9, 2022

or actually reset is_mounted on FXO destruction using a custom destructor in a struct, similar to how raw_spu_cleanup struct behaves.

@brian218
Copy link
Contributor Author

brian218 commented Nov 9, 2022

I think it's safer to not use a member to tell if mounted, just vfs::get or some other indirect way to extract data from vfs. It's because it has issues when relainching emulation.

I get you.
I need some time to come up with an implementation to fix it up.

@brian218
Copy link
Contributor Author

or actually reset is_mounted on FXO destruction using a custom destructor in a struct, similar to how raw_spu_cleanup struct behaves.

Fixed.

@brian218
Copy link
Contributor Author

Wait, I have some ideas.

@brian218 brian218 marked this pull request as ready for review November 10, 2022 11:35
@brian218
Copy link
Contributor Author

I think it's safer to not use a member to tell if mounted, just vfs::get or some other indirect way to extract data from vfs. It's because it has issues when relainching emulation.

Okay, it no longer relies on u8 is_mounted member to determine the mount status.

@brian218 brian218 requested review from elad335 and Megamouse and removed request for elad335 November 12, 2022 13:13
@brian218
Copy link
Contributor Author

What else is still keeping this PR from being merged?

@elad335
Copy link
Contributor

elad335 commented Nov 15, 2022

mount_point_reset needs to be ordered after vfs_manager, maybe use SAVESTATE_INIT_POS fir it.

@brian218
Copy link
Contributor Author

mount_point_reset needs to be ordered after vfs_manager, maybe use SAVESTATE_INIT_POS fir it.

You mean I should move mount_point_reset to VFS.cpp?

@elad335
Copy link
Contributor

elad335 commented Nov 15, 2022

No I mean you need to ensure the destructor is called before vfs_manager destructor.

@brian218
Copy link
Contributor Author

No I mean you need to ensure the destructor is called before vfs_manager destructor.

How about doing mount_point_reset's work (unmount all mount point) in vfs_manager destructor?

@elad335
Copy link
Contributor

elad335 commented Nov 15, 2022

You just need to add 2 lines, in vfs_manager and moint_point_resend. See other usages if this macro for example.

@brian218
Copy link
Contributor Author

You just need to add 2 lines, in vfs_manager and moint_point_resend. See other usages if this macro for example.

Could you please advise what value of x I should use for SAVESTATE_INIT_POS(x)?

@brian218
Copy link
Contributor Author

brian218 commented Nov 18, 2022

I didn't see any new problem in the log with current implementation.
mount_point_reset always works as expected.

@elad335
Copy link
Contributor

elad335 commented Nov 18, 2022

It's because the bug depends on which gets compiled first, sys_fs.cpp or VFS.cpp.

@elad335
Copy link
Contributor

elad335 commented Nov 18, 2022

You can search values used on other SAVESTATE_INIT_POS and come up with new ones, mount_point_reset value must be higher than vfs_manager struct

@brian218
Copy link
Contributor Author

You can search values used on other SAVESTATE_INIT_POS and come up with new ones, mount_point_reset value must be higher than vfs_manager struct

@elad335 How does it look now?

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

Successfully merging this pull request may close these issues.

None yet

4 participants