PS2: Fix booting from internal HDD

[mlevogiannis]

Description

Fixes booting from the internal HDD which has been broken for quite some time:

• Loads the missing dev9 module.
• Fixes construction of the partition name.
• Updates "path is absolute" check to take device names into account.
• Prevents double slashes from appearing in user configuration paths.
• Normalizes the CWD to work around a reported bug in PS2SDK.

Also see the commits' descriptions for more details and the rationale behind each change.

Related Issues

Fixes: #16010

Reviewers

@fjtrujy @AKuHAK

(Referencing users with recent activity, edit if required)

[AKuHAK]

@mlevogiannis your PR looks ok in terms of hdd support, but can you also test if it will still support cdfs? Cause cdfs has weird slashes and this PR may broke it.

[AKuHAK]

@mlevogiannis can you add into description that it
fixes #16010 , #15865 ,
so those issues will automatically close when PR is merged

[fjtrujy]

I would like to verify this, because so far the HDD support has been working fine.
The dev9 is not needed to be loaded, as it was loaded automatically by the own HDD driver.

Then ps2: normalize CWD to work around PS2SDK bug, this is a separate topic, I didn't want to change the PS2SDK because at that time

[fjtrujy]

Here you have how the dependencies are loaded automatically.
https://github.com/fjtrujy/ps2_drivers/blob/main/src/ps2_hdd_driver.c#L120-L138
It is true that the RA toolchain is outdated. I will try to update the toolchain and check if the issues are solved.

[AKuHAK]

I would like to verify this, because so far the HDD support has been working fine.

HDD is broken in RA for a long time, you can check if toolchain update will fix it, but I doubt that it helps.

[fjtrujy]

I would like to verify this, because so far the HDD support has been working fine.

HDD is broken in RA for a long time, you can check if toolchain update will fix it, but I doubt that it helps.

It must fix it, as I'm using ps2_drivers in other projects, and I tested it last week, NJEMU launching it from HDD and it worked out of the box... the only different thing that RA has, it is the path to load the ROM in the second parameter

[mlevogiannis]

@mlevogiannis your PR looks ok in terms of hdd support, but can you also test if it will still support cdfs? Cause cdfs has weird slashes and this PR may broke it.

@AKuHAK Good catch. I updated the workaround to be applied only if the CWD is an HDD path.

[mlevogiannis]

@mlevogiannis can you add into description that it fixes #16010 , #15865 , so those issues will automatically close when PR is merged

@AKuHAK Issue #16010 is already there.

The original description of issue #15865 is about external USB HDD with FAT32 partitions, not internal HDD with APA partitions. So this PR does not apply to that case.

[mlevogiannis]

I would like to verify this, because so far the HDD support has been working fine. The dev9 is not needed to be loaded, as it was loaded automatically by the own HDD driver.

Then ps2: normalize CWD to work around PS2SDK bug, this is a separate topic, I didn't want to change the PS2SDK because at that time

Here you have how the dependencies are loaded automatically. https://github.com/fjtrujy/ps2_drivers/blob/main/src/ps2_hdd_driver.c#L120-L138 It is true that the RA toolchain is outdated. I will try to update the toolchain and check if the issues are solved.

@fjtrujy Although the code to automatically load the dev9 dependency is there in ps2_drivers, init_drivers calls ps2_drivers' init_hdd_driver with init_dependencies = false, as seen here. Respectively for ps2_drivers' deinit_hdd_driver and deinit_dependencies = false as seen here.

As described in the commit message, it is preferable to initialize and deinitialize dev9 manually. If we used the automatic functionality offered by ps2_drivers, fileXio (and bdm in later versions of ps2_drivers) would have been deinitialized too early. It won't make a functional difference the way deinit_drivers is currently implemented (i.e. no fileXio functionality is used between the call to deinit_hdd_driver and the call to deinit_fileXio_driver), but semantically it is not correct (e.g. someone writing code for RetroArch without knowledge of the internals of ps2_drivers will think that fileXio is still available until deinit_fileXio_driver is called).

Consequently, using the latest version of ps2_drivers would not have helped in this case, as automatic loading of the dev9 dependency is disabled by the caller.

[fjtrujy]

I would like to verify this, because so far the HDD support has been working fine. The dev9 is not needed to be loaded, as it was loaded automatically by the own HDD driver.
Then ps2: normalize CWD to work around PS2SDK bug, this is a separate topic, I didn't want to change the PS2SDK because at that time

Here you have how the dependencies are loaded automatically. https://github.com/fjtrujy/ps2_drivers/blob/main/src/ps2_hdd_driver.c#L120-L138 It is true that the RA toolchain is outdated. I will try to update the toolchain and check if the issues are solved.

@fjtrujy Although the code to automatically load the dev9 dependency is there in ps2_drivers, init_drivers calls ps2_drivers' init_hdd_driver with init_dependencies = false, as seen here. Respectively for ps2_drivers' deinit_hdd_driver and deinit_dependencies = false as seen here.

As described in the commit message, it is preferable to initialize and deinitialize dev9 manually. If we used the automatic functionality offered by ps2_drivers, fileXio (and bdm in later versions of ps2_drivers) would have been deinitialized too early. It won't make a functional difference the way deinit_drivers is currently implemented (i.e. no fileXio functionality is used between the call to deinit_hdd_driver and the call to deinit_fileXio_driver), but semantically it is not correct (e.g. someone writing code for RetroArch without knowledge of the internals of ps2_drivers will think that fileXio is still available until deinit_fileXio_driver is called).

Consequently, using the latest version of ps2_drivers would not have helped in this case, as automatic loading of the dev9 dependency is disabled by the caller.

You're totally right!
I have upgraded the toolchain, and your branch now has conflicts; either solve them or we can merge mine (as you wish).
I think that changes for libretro-common/file/file_path.c aren't needed at all, but maybe it's not hurting either.

Cheers

[mlevogiannis]

I would like to verify this, because so far the HDD support has been working fine. The dev9 is not needed to be loaded, as it was loaded automatically by the own HDD driver.
Then ps2: normalize CWD to work around PS2SDK bug, this is a separate topic, I didn't want to change the PS2SDK because at that time

Here you have how the dependencies are loaded automatically. https://github.com/fjtrujy/ps2_drivers/blob/main/src/ps2_hdd_driver.c#L120-L138 It is true that the RA toolchain is outdated. I will try to update the toolchain and check if the issues are solved.

@fjtrujy Although the code to automatically load the dev9 dependency is there in ps2_drivers, init_drivers calls ps2_drivers' init_hdd_driver with init_dependencies = false, as seen here. Respectively for ps2_drivers' deinit_hdd_driver and deinit_dependencies = false as seen here.
As described in the commit message, it is preferable to initialize and deinitialize dev9 manually. If we used the automatic functionality offered by ps2_drivers, fileXio (and bdm in later versions of ps2_drivers) would have been deinitialized too early. It won't make a functional difference the way deinit_drivers is currently implemented (i.e. no fileXio functionality is used between the call to deinit_hdd_driver and the call to deinit_fileXio_driver), but semantically it is not correct (e.g. someone writing code for RetroArch without knowledge of the internals of ps2_drivers will think that fileXio is still available until deinit_fileXio_driver is called).
Consequently, using the latest version of ps2_drivers would not have helped in this case, as automatic loading of the dev9 dependency is disabled by the caller.

You're totally right! I have upgraded the toolchain, and your branch now has conflicts; either solve them or we can merge mine (as you wish). I think that changes for libretro-common/file/file_path.c aren't needed at all, but maybe it's not hurting either.

Cheers

I will rebase this branch to the current master to resolve the conflicts.

You are correct about libretro-common/file/file_path.c, it is not strictly required.

Furthermore, regarding the toolchain upgrade and commit 0f62d73 that has already been merged, deinit_dev9_driver has to be called after deinit_hdd_driver is called, as the HDD modules depend on DEV9 module. I will include fix for that, too.

[mlevogiannis]

@fjtrujy The conflicts have been resolved and the PR is now ready to be merged.

I tested the changes again and internal HDD support seems to be working fine.

However, I noticed something probably unrelated to this PR: When booting from a memory card, it fails to save the configuration file, unless the retroarch (user_path) directory already exists. When booting from the HDD, the directory is auto-created. Before the toolchain upgrade, it was auto-created for the memory card, too. In my opinion, we should move on with this PR and tackle this potential issue separately.

[mlevogiannis]

@fjtrujy Your latest comments have been resolved.