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

Some machines lacks rom0:OSDCNF #28

Closed
AKuHAK opened this issue Sep 2, 2019 · 86 comments
Closed

Some machines lacks rom0:OSDCNF #28

AKuHAK opened this issue Sep 2, 2019 · 86 comments
Labels
help wanted Extra attention is needed

Comments

@AKuHAK
Copy link

AKuHAK commented Sep 2, 2019

#define IOP_RESET_ARGS "rom0:UDNL rom0:OSDCNF"

Some PS2 machines lack OSDCNF in their ROM0. For better compatibility, it will better to avoid using this parameter for resetting IOP, or at least use this parameter only on machines that have this inside ROM0.
So it will lead to incompatibility with SCPH-10000, SCPH-15000, DTL-H10000, all ps2 TOOLs, Namco systems and ps2 emulators inside ps3 and ps4.
You can read more about restrictions here: https://assemblergames.com/threads/ps2-pops-stuff-popstarter.45347/page-39#post-771470

To my mind, it will be safe to use NULL for IOP resetting but I am not quite sure. It is better to ask @sp193 about this.

@AKuHAK

This comment has been minimized.

@frno7
Copy link
Owner

frno7 commented Sep 4, 2019

Interesting, thanks! I tried to remove rom0:OSDCNF but then SIF RPC bind failed with error code -6. An investigation will be needed to figure out why that happens, and what a working alternative would look like. Perhaps some important IOP service is missing then. Same goes with the empty string for IOP_RESET_ARGS, which I suppose corresponds to NULL?

It would be great to have SCPH-10000, SCPH-15000, DTL-H10000, TOOL and Namco working properly with Linux as well, although I am unable to test those particular models myself, unfortunately.

@AKuHAK
Copy link
Author

AKuHAK commented Sep 5, 2019

I can refer only to standard practice for resetting IOP:

    SifInitRpc(0);

#ifdef _DTL_T10000
    while (!SifIopReset("rom0:UDNL", 0))
        ;
#else
#ifdef __DECI2_DEBUG
    while (!SifIopRebootBuffer(&deci2_img, size_deci2_img))
        ;
#else
     //Final IOP reset, to fill the IOP with the default modules.
    while (!SifIopReset("", 0))
        ;
#endif
#endif

    dev9Initialized = 0;
    while (!SifIopSync())
        ;

    SifInitRpc(0);
    SifSetCmdBuffer(OplSifCmdbuffer, OPL_SIF_CMD_BUFF_SIZE);

Taken from OPL. You can safely ignore DTL and DECI2 exception but the rest should be correct.

So you should init SIF, then you should wait until SifIopReset is completed, then wait until SifIopSync is completed and only then init SIF again (!) and do the rest.

@AKuHAK
Copy link
Author

AKuHAK commented Sep 5, 2019

Please check this thread for proper ps2 initialize.

@frno7 frno7 added the help wanted Extra attention is needed label Sep 7, 2019
@frno7
Copy link
Owner

frno7 commented Sep 7, 2019

Linux kernel SIF initialisation is currently documented as thirteen steps:

linux/drivers/ps2/sif.c

Lines 740 to 781 in c94ed72

/**
* sif_init - initialise the SIF with a reset
*
* The IOP follows a certain boot protocol, with the following steps:
*
* 1. The kernel allocates a DMA memory buffer that the IOP can use to send
* commands. The kernel advertises this buffer by writing to the MAINADDR
* register.
*
* 2. The kernel reads the provisional SUBADDR register to obtain the
* corresponding command buffer for the IOP.
*
* 3. The kernel clears the %SIF_STATUS_BOOTEND flag in the SMFLAG register.
*
* 4. The kernel issues the %SIF_CMD_RESET_CMD command to the IOP.
*
* 5. The kernel indicates that the SIF and system commands are ready by
* setting the %SIF_STATUS_SIFINIT and %SIF_STATUS_CMDINIT flags in the
* SMFLAG register.
*
* 6. The kernel waits for the IOP to set the %SIF_STATUS_BOOTEND flag in
* the SMFLAG register.
*
* 7. The kernel indicates that the boot is completed by writing updating
* its MAINADDR and setting the %SIF_STATUS_CMDINIT and %SIF_STATUS_BOOTEND
* flags in the MSFLAG register. The %SIF_UNKNF260 register is set to 0xff.
*
* 8. The kernel reads the final SUBADDR register to obtain the command
* buffer for the IOP.
*
* 9. Register SIF commands to enable remote procedure calls (RPCs).
*
* 10. Reset the SIF0 (sub-to-main) DMA controller.
*
* 11. Service SIF0 RPCs via interrupts.
*
* 12. Enable the IOP to issue SIF commands.
*
* 13. Enable the IOP to issue SIF RPCs.
*
* Return: 0 on success, otherwise a negative error number
*/

It seems OPL SifIopReset corresponds to step 4, and SifIopSync to step 6?

Steps 1 and 2 are repeated in 7 and 8, which I believe corresponds to the double SifInitRpc calls? A SIF command is used to reset the IOP, but actual remote procedure calls (RPCs) are not needed until step 9 and enabled in step 13.

I seems you are right that IOP module loading must be conditional on the machine model, which means that IOP_RESET_ARGS cannot be the same for all, although it works with all models that I have tested that are SCPH-30004, SCPH-30004 R, SCPH-37000 L, SCPH-39004, SCPH-50004, SCPH-70004, SCPH-75004 and SCPH-77004. Those are fairly common models, though.

@AKuHAK
Copy link
Author

AKuHAK commented Sep 7, 2019

You are not using any part of our homebrew ps2sdk for unknown reason. I see your post that there is something strange after you boot kernel from uLaunchELF, this is because each ps2 app (firstly you are building ps2 app) should reset ps2 into proper state, but it seems that you are depending of what uLaunchELF does before and do not clear and set up everything. Also sometimes official Sony recomendations are not the best and it is better to use fixes from homebrew ps2sdk. But anyway if you are not using ps2sdk functions you should clone them and clone them as close as you can:
[SifInitRpc(0);](https://github.com/ps2dev/ps2sdk/blob/master/ee/kernel/src/sifrpc.c#L388-L426
SifIopReset())
It seems that your steps are very close to SifIopReset. But remember that you should repeat SifIopReset until it will return success.
SifIopSync
You should repeat this step again until you return success.

@sp193
Copy link

sp193 commented Sep 8, 2019

@AKuHAK I believe the PS2Linux folks have traditionally not used the homebrew PS2SDK as they have the RTE library. The Linux kernel also replaces the EE kernel.

The proper way to reboot the IOP without any IOPRP image, is to pass an empty string. I used to pass a NULL, until I realize that it was only possible with the homebrew SDK's SifIopReset() function.
Rebooting the IOP without any arguments specified, will cause the default IOP kernel modules to be loaded. This set of modules are the same across all PlayStation 2 consoles, regardless of model or type.

If you use OSDCNF, it will only work with consoles that have OSDCNF. If your software stops working after not using OSDCNF, then you shaped your work to be dependent on the board-specific modules (X-modules, OSDSND etc).
It's likely that you are expecting XMCMAN, XPADMAN etc to be loaded, which are usually automatically loaded with OSDCNF.

OPL's method of rebooting the IOP is more complicated because I also designed it to have a custom reboot procedure to load in DECI2 modules (which are missing from the CEX/DEX ROM) for debugging.

You really only need to initialize the SIF, issue the reset command, wait for completion, and then finally bring up the SIF interface.

Linux kernel SIF initialisation is currently documented as thirteen steps:

linux/drivers/ps2/sif.c

Lines 740 to 781 in c94ed72

/**
* sif_init - initialise the SIF with a reset
*
* The IOP follows a certain boot protocol, with the following steps:
*
* 1. The kernel allocates a DMA memory buffer that the IOP can use to send
* commands. The kernel advertises this buffer by writing to the MAINADDR
* register.
*
* 2. The kernel reads the provisional SUBADDR register to obtain the
* corresponding command buffer for the IOP.
*
* 3. The kernel clears the %SIF_STATUS_BOOTEND flag in the SMFLAG register.
*
* 4. The kernel issues the %SIF_CMD_RESET_CMD command to the IOP.
*
* 5. The kernel indicates that the SIF and system commands are ready by
* setting the %SIF_STATUS_SIFINIT and %SIF_STATUS_CMDINIT flags in the
* SMFLAG register.
*
* 6. The kernel waits for the IOP to set the %SIF_STATUS_BOOTEND flag in
* the SMFLAG register.
*
* 7. The kernel indicates that the boot is completed by writing updating
* its MAINADDR and setting the %SIF_STATUS_CMDINIT and %SIF_STATUS_BOOTEND
* flags in the MSFLAG register. The %SIF_UNKNF260 register is set to 0xff.
*
* 8. The kernel reads the final SUBADDR register to obtain the command
* buffer for the IOP.
*
* 9. Register SIF commands to enable remote procedure calls (RPCs).
*
* 10. Reset the SIF0 (sub-to-main) DMA controller.
*
* 11. Service SIF0 RPCs via interrupts.
*
* 12. Enable the IOP to issue SIF commands.
*
* 13. Enable the IOP to issue SIF RPCs.
*
* Return: 0 on success, otherwise a negative error number
*/

It seems OPL SifIopReset corresponds to step 4, and SifIopSync to step 6?

Steps 1 and 2 are repeated in 7 and 8, which I believe corresponds to the double SifInitRpc calls? A SIF command is used to reset the IOP, but actual remote procedure calls (RPCs) are not needed until step 9 and enabled in step 13.

(1) and (2) are done during initialization of SIFCMD.
(3) to (5) are done by SifIopReset(), while (6) is done by SifIopSync().
(7) is done by both the IOP and EE by reading each others SIF registers, although this description seems to be written from the IOP's perspective (hence MSFLAG, rather than SMFLAG).
(8) to (13) seem to be for initialization of the SIFCMD (and SIFRPC) implementation.

SifInitRpc() has to be called before the reboot. This will do a few things. It will internally call SifInitCmd(), to initialize SIF communications with the existing IOP kernel. It will also initialize the IOP side with the new SIFCMD receive buffer address. So if you don't do this, it could corrupt EE memory when you reset the IOP.
I know Sony documented calling SifInitCmd(), but we might be just using a really old SDK design (the default IOP modules were based on SDK release v1.3.4). Or the way we do things, is just different.

I seems you are right that IOP module loading must be conditional on the machine model, which means that IOP_RESET_ARGS cannot be the same for all, although it works with all models that I have tested that are SCPH-30004, SCPH-30004 R, SCPH-37000 L, SCPH-39004, SCPH-50004, SCPH-70004, SCPH-75004 and SCPH-77004. Those are fairly common models, though.

Although this is true, I think you should not be using board-specific modules. This has been a wrong practice that has been going on for too long.

I also find

int err = rom_read_file(rom0_dir, "OSDSYS",

PS2 TOOL doesn't have OSDSYS inside rom0 so this command will fail on it. I realized that it doesn't have OSDSYS only in WS moe which is abnormal and it is better to ignore it.

Inside ps2 part of ps3 and ps4 there is no ROMVER unfortunately. However, I am not quite sure that this will affect cause PS3 and PS4 use some kind of emulator (can be treated as not 'official') so this is not so important. But I own ps3 with built-in PS2 board inside (CECH-A) so I can make tests if you need it. Im sorry wrong information, ROMVER is presented inside all models.

The WS mode ROM version is 0100 - so it will be treated as a "SCPH-10000". Sony would print an "Unknown" if the model cannot be determined, which is something this code is doing differently.

The TOOL used to not have OSDSYS in its ROMs because it used to not boot discs directly. This function was added later on, for people to test their master discs without needing to write some code.

Who knows why Sony would want to try to read the model number from OSDSYS from a late SCPH-10000/SCPH-15000/DTL-H10000. At the end of the day, we haven't found one that doesn't isn't labelled as a "SCPH-10000" by OSDSYS. But this was how Sony did it for the v1.01 ROMs.

@frno7
Copy link
Owner

frno7 commented Sep 8, 2019

You are not using any part of our homebrew ps2sdk for unknown reason.

That is true, and as @sp193 notes, Linux replaces the EE kernel. Linux is an operating system, after all. One of the goals is to run entirely on the hardware. Another goal is to use fully modern and up-to-date versions of tools such as GCC, which is particularly important to compile modern Linux applications.

The older Linux 2.6 kernel relies on the BIOS for certain services, perhaps most notably handling of the SIF, but the latest Linux kernel discards the BIOS and handles the hardware directly instead. Linux has several advanced sub-systems, for example related to DMA, and hopefully these can be put into effective use with the capabilities of the PS2 hardware.

The great common goal for both the PS2SDK and Linux for the PS2 is, as I see it, to learn about, explore and have fun with the hardware. Having two, or more, independent implementations is a strength, I think, as projects can learn techniques from the others that do things differently. 😄

This Linux project does not attempt to mimic Sony application interfaces as much as the PS2SDK. However, when handling the IOP in particular, some level of SIF protocol compatibility is necessary since the IOP is operated from ROM routines.

Linux is concered with MIPS o32 and n32 ABI compatibility. The FPU, for example, is emulated in software for standard MIPS ELF executables due to FPU hardware anomalies, as described in #3.

I see your post that there is something strange after you boot kernel from uLaunchELF, this is because each ps2 app (firstly you are building ps2 app) should reset ps2 into proper state, but it seems that you are depending of what uLaunchELF does before and do not clear and set up everything.

Yes, as described in #7, the Linux kernel currently uses uLaunchELF as a boot loader. This works well, except for the CP0.Status anomaly. Perhaps a lower-level loader, as described in #4, could be used as a complement or a replacement in the future.

Also sometimes official Sony recomendations are not the best and it is better to use fixes from homebrew ps2sdk. But anyway if you are not using ps2sdk functions you should clone them and clone them as close as you can:

The Academic Free License version 2 licence used for the PS2SDK is considered to be incompatible with the GPL for Linux. This is unfortunate, and means that we cannot copy code between the projects unless it is dual-licenced with for example MIT or some of the other compatible ones.

But remember that you should repeat SifIopReset until it will return success.

This is very interesting. Why is repetition required? I have not observed that IOP reset is unreliable.

@sp193
Copy link

sp193 commented Sep 8, 2019

This Linux project does not attempt to mimic Sony application interfaces as much as the PS2SDK. However, when handling the IOP in particular, some level of SIF protocol compatibility is necessary since the IOP is operated from ROM routines.

All IOP kernel modules are replaceable through the IOP reboots (by supplying an IOPRP image to UDNL), but you'll be defining your own standard if you deviate from how Sony designed it.
The only time when the IOP runs directly from ROM, is during bootup when it accesses the reset vector and runs IOPBOOT. Or if it is in PlayStation mode.

But remember that you should repeat SifIopReset until it will return success.

This is very interesting. Why is repetition required? I have not observed that IOP reset is unreliable.

The IOP reboot involves sending a SIFCMD message. Within (sce)SifSendCmd() is a call to SifSetDma(), which can fail if the queue within the kernel is full. Although we all know that it's not supposed to be full at that point.

Perhaps, it's just a good practice to ensure that your code will never fail, by handling all return values. Whether to do this or not, is up to you.

@frno7
Copy link
Owner

frno7 commented Sep 8, 2019

All IOP kernel modules are replaceable through the IOP reboots (by supplying an IOPRP image to UDNL), but you'll be defining your own standard if you deviate from how Sony designed it.
The only time when the IOP runs directly from ROM, is during bootup when it accesses the reset vector and runs IOPBOOT. Or if it is in PlayStation mode.

Thanks, this is valuable information! Sony compatible IOP modules are sometimes of limited use, since Linux already has sub-systems for USB, file systems, networking, and various other devices and protocols.

It can make sense to write new IOP modules that work effectively with these Linux sub-systems, at suitable interface and protocol levels. The low-level SIF DMA command and RPC protocol can probably remain the same though, as I’m not aware of any particular reason to do that differently.

The iopmod project is a starting-point for IOP modules for Linux. An IRQ relay module is currently implemented, as it is needed by the current USB OHCI device driver.

Handling of the IOP is a bit loose at present, so things may change. 😄

The IOP reboot involves sending a SIFCMD message. Within (sce)SifSendCmd() is a call to SifSetDma(), which can fail if the queue within the kernel is full. Although we all know that it's not supposed to be full at that point.

Yes, it seems reasonable for the Linux kernel, when invoked by a boot loader, to expect that the IOP is idle and not in the middle of some transactions.

Perhaps, it's just a good practice to ensure that your code will never fail, by handling all return values. Whether to do this or not, is up to you.

All return codes that I’m aware of are checked by the Linux kernel. However, it doesn’t make multiple attempts at resetting the IOP. I will have to think about this.

@AKuHAK
Copy link
Author

AKuHAK commented Sep 9, 2019

Interesting, thanks! I tried to remove rom0:OSDCNF but then SIF RPC bind failed with error code -6. An investigation will be needed to figure out why that happens, and what a working alternative would look like. Perhaps some important IOP service is missing then. Same goes with the empty string for IOP_RESET_ARGS, which I suppose corresponds to NULL?

If you use OSDCNF, it will only work with consoles that have OSDCNF. If your software stops working after not using OSDCNF, then you shaped your work to be dependent on the board-specific modules (X-modules, OSDSND etc).
It's likely that you are expecting XMCMAN, XPADMAN etc to be loaded, which are usually automatically loaded with OSDCNF.

But there is still a problem that you have an error when trying to remove OSDCNF, I personally don't think that this is because board-specific modules are loaded. To my mind its because IOP is not resetting with standards that cause problems with some arguments.

The great common goal for both the PS2SDK and Linux for the PS2 is, as I see it, to learn about, explore and have fun with the hardware. Having two, or more, independent implementations is a strength, I think, as projects can learn techniques from the others that do things differently.

One of the goals is to run entirely on the hardware.

Yes, as described in #7, the Linux kernel currently uses uLaunchELF as a boot loader. This works well, except for the CP0.Status anomaly. Perhaps a lower-level loader, as described in #4, could be used as a complement or a replacement in the future.

I understand your point of view, but to my mind, it is overkill and reinventing the wheel. You still are building a PS2 app (in the first place) and only secondly Linux system. You are trying to make as much as possible from the Linux side which can cause anomalies if you are trying to make all from scratch.
As about license - I don't mean to copy code from ps2sdk line-per-line - just investigate how exactly IOP should be reset in the proper way.

@sp193
Copy link

sp193 commented Sep 11, 2019

SIF RPC bind will fail if the IOP RPC server module does not exist - like if you are using libpad without loading the right PADMAN version.

@frno7
Copy link
Owner

frno7 commented Sep 11, 2019

If you use OSDCNF, it will only work with consoles that have OSDCNF. If your software stops working after not using OSDCNF, then you shaped your work to be dependent on the board-specific modules (X-modules, OSDSND etc). It's likely that you are expecting XMCMAN, XPADMAN etc to be loaded, which are usually automatically loaded with OSDCNF.

The IOP library dependencies are intrman, loadcore, sifcmd, sifman and thbase. In addition, the IOP heap memory RPC service is expected to be available (to allocate a USB OHCI buffer).

More specifically, the printk IOP module, that prints to the kernel log, has the following dependencies:

iopmod name		printk
import library	0x0102	intrman
import		23	intrman_in_irq QueryIntrContext
import library	0x0101	loadcore
import		 6	loadcore_register_library	
import		 7	loadcore_release_library	
import library	0x0101	sifcmd
import		12	sifcmd_send_cmd
import		13	sifcmd_send_cmd_irq isceSifSendCmd
import library	0x0101	sifman
import		 8	__sifman_dma_stat sceSifDmaStat

Note: When it says for example intrman_in_irq QueryIntrContext it means the two symbols are aliased, and refer to the same function, namely the one with id 23 for intrman in this case. Typically, a Linux module would refer to intrman_in_irq but a PS2SDK module would refer to QueryIntrContext. It’s just a matter of using a particular naming convention.

Similarly, the irqrelay module, that forwards IOP interrupts to the EE, has the following dependencies:

iopmod name		irqrelay
import library	0x0102	intrman
import		 4	intrman_request_irq RegisterIntrHandler
import		 5	intrman_release_irq ReleaseIntrHandler
import		 6	intrman_enable_irq EnableIntr
import		 7	intrman_disable_irq
import		17	intrman_cpu_suspend_irq CpuSuspendIntr
import		18	intrman_cpu_resume_irq CpuResumeIntr
import		23	intrman_in_irq QueryIntrContext
import library	0x0101	sifcmd
import		12	sifcmd_send_cmd
import		13	sifcmd_send_cmd_irq isceSifSendCmd
import		17	sifcmd_register_rpc sceSifRegisterRpc
import		19	sifcmd_set_rpc_queue
import		22	sifcmd_rpc_loop
import		24	sifcmd_remove_rpc sceSifRemoveRpc
import		25	sifcmd_remove_rpc_queue
import library	0x0101	thbase
import		 4	thbase_create
import		 5	thbase_delet
import		 6	thbase_star
import library	0x0101	sifman
import 		 8	__sifman_dma_stat sceSifDmaStat
import		24	sifman_set_sm_flag sceSifSetSMFlag
import		28	sifman_intr_main

So, some of these dependencies are not initialised by the IOP unless rom0:UDNL rom0:OSDCNF is given as a reset argument.

Looking at the contents of rom0:OSDCNF, I agree that it appears excessive with a lot of modules that are not needed:

RESEROMDIRHPEXTINFOXIOPBTCON?' D20030227-193049,osd.conf,osdconf.bin,hana@rom-server/~/g/app/rom' @800
SYSMEM
LOADCORE
EXCEPMAN
INTRMANP
INTRMANI
SSBUSC
DMACMAN
TIMEMANP
TIMEMANI
SYSCLIB
HEAPLIB
EECONF
THREADMAN
VBLANK
IOMAN
MODLOAD
ROMDRV
ADDDRV
STDIO
SIFMAN
IGREETING
XSIFCMD
REBOOT
XLOADFILE
XCDVDMAN
XCDVDFSV
SIFINIT
XFILEIO
SECRMAN
EESYNC
RMRESET
CLEARSPU
XSIO2MAN
XMTAPMAN
XMCMAN
XMCSERV
XPADMAN
XRMMAN2
OSDSND

You really only need to initialize the SIF, issue the reset command, wait for completion, and then finally bring up the SIF interface.

Yes, that sounds reasonable!

Although this is true, I think you should not be using board-specific modules. This has been a wrong practice that has been going on for too long.

What modules would you recommend to resolve the dependencies for printk and irqrelay above?

@sp193
Copy link

sp193 commented Sep 12, 2019

Among the imports you have listed, I cannot see any that are likely to be only available with the board-specific modules.

Previously, you wrote that it was a problem with binding with a SIF RPC. Have you determined that it was either of those two IOP modules that could not be loaded, due to a dependency problem? In other words, (Sif)LoadModule returned the exact error code which indicates that.
Which SIF RPC was it?

@AKuHAK
Copy link
Author

AKuHAK commented Sep 12, 2019

@frno7 maybe you can provide full kernel log? and the difference between 2 versions IOP_RESET_ARGS

@frno7
Copy link
Owner

frno7 commented Sep 12, 2019

The two IOP modules printk and irqrelay appear to load successfully. However, when trying to bind the irqrelay RPC service from the EE side, the returned server pointer is NULL:

return client->server ? 0 : -ENXIO;

Somehow the irqrelay module fails to set its RPC server up. The server NULL pointer comes from here:

linux/drivers/ps2/sif.c

Lines 530 to 533 in 1a3ad68

case SIF_CMD_RPC_BIND:
client->server = packet->server;
client->server_buffer = packet->server_buffer;
break;

For reference, this is the corresponding code in PS2SDK:

https://github.com/ps2dev/ps2sdk/blob/e2afe928fcf97babfa12917e54cfb48db261e914/ee/kernel/src/sifrpc.c#L287-L291

Perhaps the modules fail to initialise themselves, somehow. The first part to start to executing in any IOP module is this (in two variants, conditional on SYMTAB_EXPORT):

https://github.com/frno7/iopmod/blob/13732bffa01dd29f3289a5a340a2ff08401557d3/include/iopmod/mod.h#L26-L51

printk has SYMTAB_EXPORT defined, because it exports the printk function, and so it calls loadcore_register_library (RegisterLibraryEntries) to register it. That might fail, of course, and perhaps an error code ought to be returned here:

https://github.com/frno7/iopmod/blob/13732bffa01dd29f3289a5a340a2ff08401557d3/include/iopmod/mod.h#L34

But since loading irqrelay is (or appears to be) successful, we might assume that its dependency printk did load successfully too. irqrelay does not define SYMTAB_EXPORT, because it does not export any library functions, hence the module init function is called here:

https://github.com/frno7/iopmod/blob/13732bffa01dd29f3289a5a340a2ff08401557d3/include/iopmod/mod.h#L48

And we ought to end up here:

https://github.com/frno7/iopmod/blob/13732bffa01dd29f3289a5a340a2ff08401557d3/module/irqrelay.c#L264-L305

This is where threads and RPC services are set up for irqrelay. However, none of the print functions appear work as they are supposed to, which seem to indicate that something about the state of the IOP is dysfunctional when rom0:UDNL rom0:OSDCNF isn’t given as a reset argument.

Hmm... Any ideas?

@sp193
Copy link

sp193 commented Sep 12, 2019

If your iop_reset_arg() function is meant to issue the reboot command to the IOP, it does not look like it supports a NULL parameter. You should pass an empty string.

Are those modules loaded from a buffer in EE memory? If so, then I would also like to ask whether you actually have an implementation like mrbrown's SBV LoadModuleBuffer() patch. Sony does something like that too when rebooting the IOP for the HDD Browser with an embeeded IOPRP image, since the v1.3.4 kernel has no support for SifLoadModuleBuffer(), even though the IOP function is there. Without it, LoadModuleBuffer() from the EE does not work because the switch-case block on the IOP doesn't call the function. This would also explain what you have observed because OSDCNF (and EELOADCNF) does include XLOADFILE instead of LOADFILE, which has a working implementation of this RPC function.

@frno7
Copy link
Owner

frno7 commented Sep 12, 2019

If your iop_reset_arg() function is meant to issue the reboot command to the IOP, it does not look like it supports a NULL parameter. You should pass an empty string.

The empty string is passed (as opposed to previous "rom0:UDNL rom0:OSDCNF"). NULL for arg has never been supported for the iop_reset_arg function, as can be seen here:

linux/drivers/ps2/sif.c

Lines 581 to 607 in 1a3ad68

static int iop_reset_arg(const char *arg)
{
const size_t arglen = strlen(arg) + 1;
struct {
u32 arglen;
u32 mode;
char arg[79 + 1]; /* Including NUL */
} reset_pkt = {
.arglen = arglen,
.mode = 0
};
int err;
if (arglen > sizeof(reset_pkt.arg))
return -EINVAL;
memcpy(reset_pkt.arg, arg, arglen);
sif_write_smflag(SIF_STATUS_BOOTEND);
err = sif_cmd(SIF_CMD_RESET_CMD, &reset_pkt, sizeof(reset_pkt));
if (err)
return err;
sif_write_smflag(SIF_STATUS_SIFINIT | SIF_STATUS_CMDINIT);
return completed(sif_smflag_bootend) ? 0 : -EIO;
}

Are those modules loaded from a buffer in EE memory?

Yes, always.

If so, then I would also like to ask whether you actually have an implementation like mrbrown's SBV LoadModuleBuffer() patch.

No, I wasn’t aware of that patch. Is it this one?

https://github.com/ps2dev/ps2sdk/blob/master/ee/sbv/src/patch_enable_lmb.c

Sony does something like that too when rebooting the IOP for the HDD Browser with an embeeded IOPRP image, since the v1.3.4 kernel has no support for SifLoadModuleBuffer(), even though the IOP function is there. Without it, LoadModuleBuffer() from the EE does not work because the switch-case block on the IOP doesn't call the function. This would also explain what you have observed because OSDCNF (and EELOADCNF) does include XLOADFILE instead of LOADFILE, which has a working implementation of this RPC function.

Oh, many thanks!

@sp193
Copy link

sp193 commented Sep 12, 2019

Yes, that is the patch. You need to apply something like this, before you attempt to call LoadModuleBuffer() from the EE. It requires the SIF, LOADFILE and IOP heap services.

frno7 pushed a commit that referenced this issue Sep 22, 2019
commit 3dd550a upstream.

The syzbot fuzzer provoked a slab-out-of-bounds error in the USB core:

BUG: KASAN: slab-out-of-bounds in memcmp+0xa6/0xb0 lib/string.c:904
Read of size 1 at addr ffff8881d175bed6 by task kworker/0:3/2746

CPU: 0 PID: 2746 Comm: kworker/0:3 Not tainted 5.3.0-rc5+ #28
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Workqueue: usb_hub_wq hub_event
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0xca/0x13e lib/dump_stack.c:113
  print_address_description+0x6a/0x32c mm/kasan/report.c:351
  __kasan_report.cold+0x1a/0x33 mm/kasan/report.c:482
  kasan_report+0xe/0x12 mm/kasan/common.c:612
  memcmp+0xa6/0xb0 lib/string.c:904
  memcmp include/linux/string.h:400 [inline]
  descriptors_changed drivers/usb/core/hub.c:5579 [inline]
  usb_reset_and_verify_device+0x564/0x1300 drivers/usb/core/hub.c:5729
  usb_reset_device+0x4c1/0x920 drivers/usb/core/hub.c:5898
  rt2x00usb_probe+0x53/0x7af
drivers/net/wireless/ralink/rt2x00/rt2x00usb.c:806

The error occurs when the descriptors_changed() routine (called during
a device reset) attempts to compare the old and new BOS and capability
descriptors.  The length it uses for the comparison is the
wTotalLength value stored in BOS descriptor, but this value is not
necessarily the same as the length actually allocated for the
descriptors.  If it is larger the routine will call memcmp() with a
length that is too big, thus reading beyond the end of the allocated
region and leading to this fault.

The kernel reads the BOS descriptor twice: first to get the total
length of all the capability descriptors, and second to read it along
with all those other descriptors.  A malicious (or very faulty) device
may send different values for the BOS descriptor fields each time.
The memory area will be allocated using the wTotalLength value read
the first time, but stored within it will be the value read the second
time.

To prevent this possibility from causing any errors, this patch
modifies the BOS descriptor after it has been read the second time:
It sets the wTotalLength field to the actual length of the descriptors
that were read in and validated.  Then the memcpy() call, or any other
code using these descriptors, will be able to rely on wTotalLength
being valid.

Reported-and-tested-by: syzbot+35f4d916c623118d576e@syzkaller.appspotmail.com
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
CC: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/Pine.LNX.4.44L0.1909041154260.1722-100000@iolanthe.rowland.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
@AKuHAK
Copy link
Author

AKuHAK commented Mar 18, 2020

@frno7 I see you created wiki pages. Just one note - you created the page with compatible models.
I just would like to mention that until this issue #28 is resolved you cannot state that ps2linux is compatible with all models where wLaunchELF can be run.
So, for now, it is incompatible with:
SCPH-10000
SCPH-15000
SCPH-18000
DTL-H10000
Namco System 246/256 (all arcade machines)
PS2 TOOL (all modes and revisions)
PS3 Backward Compatible with PS2
PS3 Semi-Backward Compatible with PS2
PS3 PS2 Classics
PS4 PS2 Classics

@frno7
Copy link
Owner

frno7 commented Mar 18, 2020

Thanks, @AKuHAK! I’m happy you noticed that the wiki is being updated only some days and hours ago! I’m getting some help too: Денис Визница supplied a detailed guide to building an R5900 cross compiler from sources, for example.

Maybe we can be even more precise about compatibility? Some of the models that are expected to work have not actually been tested (yet) with a recent Linux kernel, as far as I know. Some of the models that you list may eventually be made compatible, while others are likely to remain incompatible (mostly those incompatible with uLaunchELF).

Would you like to contribute by editing the wiki yourself?

@AKuHAK
Copy link
Author

AKuHAK commented Mar 18, 2020

@frno7 Oh I cannot say that your ps2 Linux implementation will definitely work on all other models :) I just mentioned all models where your implementation will DEFINETYLY (100%) freeze on start. At least for now., until this issue is resolved. There can be other incompatibilities which I didnt catch. I am still waiting for precompiled binaries. I will try to build the system again when I have more spare time.

As about wiki - you can just insert a reference to this issue. I think this will be enough.

@frno7
Copy link
Owner

frno7 commented Mar 18, 2020

@AKuHAK, please review the updated model compatibility. I’m aiming for a comprehensive, structured, and easy to understand wiki. Issues such as this one are good for discussions, but I think that the wiki ought to have the conclusions.

Has anybody heard from @kernle32dll lately? He had interesting ideas in #34 (comment).

@mirh
Copy link

mirh commented Mar 18, 2020

Doesn't 9000x also support everything "normally" since the release of PS2 Fortuna?

@AKuHAK
Copy link
Author

AKuHAK commented Mar 19, 2020

@frno7 Due to a recently discovered method of booting wLaunchELF by using a new exploit called Fortuna you can add SCPH-900xx to the expected to be compatible list.

More information regarding new exploit can be found here or here.

@frno7
Copy link
Owner

frno7 commented Mar 19, 2020

Great, thanks, @mirh & @AKuHAK! I have updated model compatibility.

By the way, I linked uLaunchELF to your repo. Is it an authoritative source?

@AKuHAK
Copy link
Author

AKuHAK commented Mar 19, 2020

Yes, uLaunchELF that I host is the official wLaunchELF repository.

BTW you can also add Sony Bravia KDL-22PX300 into expected to be compatible list :)

@frno7
Copy link
Owner

frno7 commented Mar 21, 2020

Done, thanks, @AKuHAK!

@frno7
Copy link
Owner

frno7 commented Apr 19, 2022

Alright then) Oo so many nasty hid_errors...

Oh, we should take a closer look at those, and try to fix them. Is it on real hardware?

Yes, like I said, I am always seeing "initrd not found" while it is actually presented and is working. That's a lier message)

Actually (the old) INITRD and (the new) INITRAMFS are different things, confer the article on what is initramfs?, and especially the section all this differs from the old initrd in several ways. My apologies, I previously failed to recall that when INITRD bails out it’s perfectly natural because INITRAMFS is the one we’ve installed anyway.

In pcsx2 it stucks on that first screen. Even if it is working, the next screen is not a continuation of the first screen - that next screen is other screen, so I think pcsx2 just can not switch to it.

Cool, I’ve never attempted to boot Linux with PCSX2. :-)

@AKuHAK
Copy link
Author

AKuHAK commented Apr 19, 2022

Oh, we should take a closer look at those, and try to fix them. Is it on real hardware?

Yes it is on my SCPH-70003 that can be seen in this "picture"

@AKuHAK
Copy link
Author

AKuHAK commented Apr 19, 2022

maybe it is because I compiled busybox with default options... don't know
here is the kernel, anyone can test it on real HW
https://github.com/AKuHAK/linux/actions/runs/2188258161

@AKuHAK
Copy link
Author

AKuHAK commented Apr 19, 2022

when this pr will be merged, such things can be downloaded directly here

@frno7
Copy link
Owner

frno7 commented Apr 19, 2022

Yes it is on my SCPH-70003 that can be seen in this "picture"

As a simple exercise I propose that you try dmesg >/mnt/usb/dmesg.txt, assuming you’ve mounted a writable USB filesystem on /mnt/usb, and post this text. :-)

@frno7
Copy link
Owner

frno7 commented Apr 19, 2022

@AKuHAK, regarding the previously mentioned memory regions and /proc/iomem, see also issue #13. I hope to map as many address spaces as possible, and make them available via /dev/mem and similar devices.

@Arch91
Copy link

Arch91 commented Apr 20, 2022

anyone can test it on real HW

Tested. I saw the same hid_errors. While I do not have them in the compiled kernel of mine. Maybe some module to activate in bootscript - like ohci-ps2, usbhid or hid-generic - should solve them.
Also some message about that tty is not ready... well, that's also fixable with an additions in initramfs bootscript. Those are not kernel bugs, just the linux needs.

Once the line with "modprobe gs" is processed, the screen should flash once, the resolution will be changed to the only available for the current gs-drm driver

I propose that you try dmesg >/mnt/usb/dmesg.txt

I was wrong saying that there is the only resolution for gs-drm. There are lines in dmesg output txt file:
drm: gs_mode_valid name "resolutions"

Launching @AKuHAK's kernel I saw a high resolutioned screen... Hint me, please, how to change it - need I add something in "modprobe gs ..." ?

Actually (the old) INITRD and (the new) INITRAMFS are different things

If to be seriously busy into that trifle problem, that should be fixed once and for all, so the people will never be confused about it. Maybe to add some INITRAMFS/INITRD presence checks? If/else, "initramfs detected"...

@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

Tested. I saw the same hid_errors. While I do not have them in the compiled kernel of mine. Maybe some module to activate in bootscript - like ohci-ps2, usbhid or hid-generic - should solve them.

Hmm... interesting. X86-64 is uncommon in this household, and on the decline. I’ll see what I can find for use with the Docker packages. :-)

Also some message about that tty is not ready... well, that's also fixable with an additions in initramfs bootscript. Those are not kernel bugs, just the linux needs.

I highly recommend setting up a network interface via USB, until issue #19 completes. I personally use wifi, but Ethernet is simpler, if one has such a USB device. I’ve compiled Dropbear for remote SSH login, remote scripting, and so on. It could be a consideration for INITRAMFS inclusion, eventually. For development and testing purposes, TCP/IP access is much faster and convenient than using a USB keyboard/filesystem directly attached to the PlayStation 2 hardware.

I was wrong saying that there is the only resolution for gs-drm. There are lines in dmesg output txt file: drm: gs_mode_valid name "resolutions" Launching @AKuHAK's kernel I saw a high resolutioned screen... Hint me, please, how to change it - need I add something in "modprobe gs ..." ?

I think it’s best to avoid the DRM prototype driver, because it’s inferior to the fbdev driver in all respects. I consider dropping it, now that fbdev is actively maintained again, and the DRM no longer is a requirement for issue #1. Also, the DRM subsystem has a lot of known problems with vintage video equipment.

Actually (the old) INITRD and (the new) INITRAMFS are different things

If to be seriously busy into that trifle problem, that should be fixed once and for all, so the people will never be confused about it. Maybe to add some INITRAMFS/INITRD presence checks? If/else, "initramfs detected"...

Agreed. The confusing INITRD print statement comes from

printk(KERN_INFO "Initrd not found or empty");

which is part of the MIPS architecture kernel setup, but otherwise unrelated to the PlayStation 2 parts. Anyone could post the MIPS maintainers a patch to clear that up.

@AKuHAK
Copy link
Author

AKuHAK commented Apr 20, 2022

Maybe some module to activate in bootscript

with the Docker packages

I used init scripts from the wiki. These scripts are populated almost as-is into the Docker container.

@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

@Arch91 thanks a lot! I just should wait a bit longer. Everything is working now :)

@AKuHAK how long does it take? I’m clocking approximately 13 seconds from the first

zimage at:     00803BE0 00BDDE8C
Uncompressing Linux at load address 80010000
Now, booting the kernel...

to the shell prompt. It’s apparently possible to boot the Linux kernel in less than a second, which would be marvellous. :-)

@AKuHAK
Copy link
Author

AKuHAK commented Apr 20, 2022

how long does it take?

It takes more cause my TV loose resolution and I should wait for about 20 seconds until the screen will become alive again

@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

It takes more cause my TV loose resolution and I should wait for about 20 seconds until the screen will become alive again

We could make the ps2fb driver pick up the video seamlessly, without any flickering at all. I had it myself a few years ago, when the driver wasn’t fully developed. However, since GS registers cannot be read, confer #68 (comment), the driver will have to assume that the video resolution matches. Alternatively, perhaps we can come up with some kind of protocol to negotiate what the video resolution is from wLaunchELF. If it doesn’t match the expected, it’ll all be garbled output. Likewise with the currently hardcoded 1920×1080p boot console. We could make it inherit the preexisting video resolution from wLaunchELF, for a completely flicker-free video boot.

@frno7 frno7 mentioned this issue Apr 20, 2022
6 tasks
@Arch91
Copy link

Arch91 commented Apr 20, 2022

Actually (the old) INITRD and (the new) INITRAMFS are different things

some INITRAMFS/INITRD presence checks?

This is what I get with initrd disabled

During the work day I was thinking something about it. We have two facts - @AKuHAK launched the system with initrd disabled; initrd and initramfs are different things and do not depends one from another. Then just simple to disable initrd in the kernelconfig ps2_defconfig file, and that message will not bother anyone anymore) Moreover, that will enlight the result vmlinuz.elf kernel a bit.

It’s apparently possible to boot the Linux kernel in less than a second

If you ask me, I would not recommend to achieve that. There are manually reasonably written "sleep" lines in the init scripts. If to use init scripts without "sleep" lines, there might be unordered occasions happens. For example, mount a partition with further file accessing in the exact path - the example occasion is "no such file or directory" output, and that's because the system was not in time to mount the partition before the file access from it.

@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

Then just simple to disable initrd in the kernelconfig ps2_defconfig file, and that message will not bother anyone anymore

How? INITRD and INITRAMFS seem to use the same kernel config knob:

linux/init/Kconfig

Lines 1213 to 1214 in 59a11ab

config BLK_DEV_INITRD
bool "Initial RAM filesystem and RAM disk (initramfs/initrd) support"

@frno7
Copy link
Owner

frno7 commented Apr 20, 2022

It’s apparently possible to boot the Linux kernel in less than a second

If you ask me, I would not recommend to achieve that. There are manually reasonably written "sleep" lines in the init scripts. If to use init scripts without "sleep" lines, there might be unordered occasions happens.

With proper dependencies no sleeping is necessary at all, and we'd have maximum speed. :-)

For example, mount a partition with further file accessing in the exact path - the example occasion is "no such file or directory" output, and that's because the system was not in time to mount the partition before the file access from it.

I don't think the mount command will complete until the filesystem has been made available (or the command failed to mount it), so for instance mount /mnt && cat /mnt/foo is robust and works as expected with no timing issues.

@AKuHAK
Copy link
Author

AKuHAK commented Apr 20, 2022

#define IOP_RESET_ARGS ""

TBH this particular issue can be closed, cause now IOP is rebooted with a null string. So hardware compatibility list is not actual anymore.

There still can be incompatibilities. For example, you are mapping rom1 into memory, but many consoles just doesnt have rom1 at all (like protokernels, desr, TEST consoles). But I dont think that this will prevent kernel from booting on those consoles, so this issue technically can be closed.

@frno7
Copy link
Owner

frno7 commented Apr 21, 2022

Thanks, @AKuHAK! I believe other technical difficulties remain, such as the OHCI driver directly accessing IOP memory space from the R5900:

err = iopheap_alloc_dma_buffer(pdev, DMA_BUFFER_SIZE);

However, I think it’d be best to keep things simple and complete issue #1, before complicating drivers to accommodate rare hardware (and software emulated) models.

What’s the best approach to update the list of incompatible models? Remove the link to this issue?

@AKuHAK
Copy link
Author

AKuHAK commented Apr 21, 2022

You can remove the whole section: Incompatible models as currently all models should becompatible

@frno7
Copy link
Owner

frno7 commented Apr 21, 2022

Closing issue per #28 (comment). Thanks again!

@frno7 frno7 closed this as completed Apr 21, 2022
@sp193
Copy link

sp193 commented Apr 22, 2022

If you are not already doing it, perhaps you might want to consider probing whether the ROM exists before mapping it, much like Sony does for ADDDRV and friends? ADDDRV will first map the DVD ROM (DEV1, if I remember right) to memory with the SSBUSC service, before checking whether there is a valid ROM image there.
The SSBUSC module controls the memory mapping and access timings to devices connected to the SSBUS (the IOP's peripheral bus). If you don't actually care about knowing the real sizes of the DVD ROM, you could refer to the window size and address configured for DEV1 in SSBUSC. Now to think about it, I don't suppose a bus error occurs if there is actually nothing connected for the associated memory range, since ADDDRV can probe the region for a connected ROM chip.

This can be seen as a problem of using the outdated idea that rom0, rom1, rom2, erom etc, belong to separate ROMs, when they belong to the same DVD ROM chip. Most retail PS2s just have them because the DVD player is built into them.
The size and location of rom1, rom2 and erom depend on the ROM's version. rom1 begins from the start of the address that the DVD ROM, while rom2 (if it exists) and erom exist after it.

@frno7
Copy link
Owner

frno7 commented Apr 22, 2022

Thank you, @sp193. I’ve registered issue #72 to improve this.

@Arch91
Copy link

Arch91 commented Apr 22, 2022

Well, that's the issue offtopic, a background barking while big men talking :D But since that was mentioned...

to disable initrd in the kernelconfig ps2_defconfig file, and that message will not bother anyone anymore

How? INITRD and INITRAMFS seem to use the same kernel config knob

Yeah, looks like there is no way to influence to that kernel message by changing something in kernelconfig; initramfs can not be "separated" from initrd in the needed way.

to add some INITRAMFS/INITRD presence checks? If/else, "initramfs detected"...

I gone that way. The theory:
If we are building with initramfs then the kernelconfig parameter CONFIG_INITRAMFS_SOURCE will be certainly set manually. By default it is
CONFIG_INITRAMFS_SOURCE=""
If it was absent in the kernelconfig or written as #CONFIG_INITRAMFS_SOURCE is not set, it will be changed to that default value of two underscores (not to the empty value between those underscores) after the make oldconfig passed. So, we will be checking whenever there was something written to that kernelconfig parameter and comparing it with the default two underscores string value. A comparing of the two strings.

The practice:
file arch/mips/kernel/setup.c :
from line 245:

 static void __init finalize_initrd(void)
 {
+#define VAL(str) #str
+#define TOSTRING(str) VAL(str)
 	unsigned long size = initrd_end - initrd_start;
+	char *initramfs = TOSTRING (CONFIG_INITRAMFS_SOURCE);
+	char default_null_initramfs[] = "\"\"";
+	int nullcheck = strcmp(initramfs,default_null_initramfs);

a bit lower, from line 266:

 	return;
 disable:
 	printk(KERN_CONT " - disabling initrd\n");
+   if ( nullcheck != 0 ) {
+	printk(KERN_INFO "initramfs presence detected - it will be tried to be loaded instead of the absent initrd\n");
+   }
 	initrd_start = 0;
 	initrd_end = 0;

Tested, working as expected.

Anyone could post the MIPS maintainers a patch to clear that up.

However, I see you have a tendention to avoid anything kernel maintainers unacceptable and a things not related to PS2 devices parts coding) @frno7, should I keep it away from your source codes again?)) Means in not your repository.

@frno7
Copy link
Owner

frno7 commented Apr 22, 2022

Anyone could post the MIPS maintainers a patch to clear that up.

However, I see you have a tendention to avoid anything kernel maintainers unacceptable and a things not related to PS2 devices parts coding) @frno7, should I keep it away from your source codes again?)) Means in not your repository.

@Arch91, since this is an independent improvement, to the benefit of the MIPS architecture, I suggest posting it to linux-mips at vger.kernel.org with a CC to me (noring at nocrew org), explaining the confusion caused by the previous behaviour. No need to wait for issue #1 completion. Hopefully a MIPS maintainer will review your post, discuss its technical details, and eventually accept your Linux kernel patch contribution. Review the procedures on posting Linux kernel patches, should you decide to proceed.

I’ve submitted several independent patches. Check USB: Fix incorrect DMA allocations for local memory pool drivers, for instance.

@AKuHAK
Copy link
Author

AKuHAK commented May 1, 2022

is it possible to change the resolution by typing some command into the terminal?

@frno7
Copy link
Owner

frno7 commented May 1, 2022

is it possible to change the resolution by typing some command into the terminal?

Yes, there are three main methods to configure video modes:

  1. The current video mode can be shown with cat /sys/class/graphics/fb0/mode. Applying a preset video mode is very easy. One can for instance do echo S:720x576p-50 >/sys/class/graphics/fb0/mode, from the list of modes shown with cat /sys/class/graphics/fb0/modes.

  2. The fbset command can be used to configure advanced video modes. See also Trim Busybox, supply other essential commands gentoo-mipsr5900el#5 (comment).

  3. In addition, the video hardware registers can be examined and manipulated for ultimate video freedom.

I just updated the video modes section on the wiki about this topic. Thanks for asking, @AKuHAK. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

7 participants