[PATCH] CD-ROM improvements
[PATCH] CD-ROM improvements
The patch is attached. Here's what it does:
- Improves IDE loading - makes CD-ROM always the 1st slave, except for when no hard disks are present in which case it disables the CD-ROM so some BIOS'es don't freak out about it, and makes sure the hard disks fill the slots in order, which makes sure there's no slave without master;
- Changes the entire ATAPI implementation to the one from PCem-X which adds several ATAPI commands previously not emulated, as well as better disc change handling (code partially written by SA1988 too);
- IOCTL changes to accomodate the above;
- Ability to load ISO images (most of the code written by RichardG867).
- Improves IDE loading - makes CD-ROM always the 1st slave, except for when no hard disks are present in which case it disables the CD-ROM so some BIOS'es don't freak out about it, and makes sure the hard disks fill the slots in order, which makes sure there's no slave without master;
- Changes the entire ATAPI implementation to the one from PCem-X which adds several ATAPI commands previously not emulated, as well as better disc change handling (code partially written by SA1988 too);
- IOCTL changes to accomodate the above;
- Ability to load ISO images (most of the code written by RichardG867).
- Attachments
-
- pcem_cdrom.rar
- (18.11 KiB) Downloaded 486 times
Re: [PATCH] CD-ROM improvements
New version attached. Differs only in that I removed the modification (added -g switch) to makefile.mingw.
- Attachments
-
- pcem_cdrom.rar
- (18.12 KiB) Downloaded 506 times
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
.ISO support committed to rev 414.
Not too sure about the other changes though. I don't see the need for the hard drive loading changes, frankly if users want to load slave drives without masters and get the same issues they'd get on real hardware with the same setup, that's up to them. Forcing the CD-ROM to primary slave prevents the use of 2 hard drives plus CD-ROM on older BIOSes, which is a valid setup on real hardware (with a dual-channel IDE controller or a sound card with an IDE port) that is supported by OAKCDROM.SYS and probably some other drivers. I also don't see the need to replace the entire ATAPI implementation - new commands are nice, but you've changed a lot of the existing code for what seems like no good reason, without improving it.
Not too sure about the other changes though. I don't see the need for the hard drive loading changes, frankly if users want to load slave drives without masters and get the same issues they'd get on real hardware with the same setup, that's up to them. Forcing the CD-ROM to primary slave prevents the use of 2 hard drives plus CD-ROM on older BIOSes, which is a valid setup on real hardware (with a dual-channel IDE controller or a sound card with an IDE port) that is supported by OAKCDROM.SYS and probably some other drivers. I also don't see the need to replace the entire ATAPI implementation - new commands are nice, but you've changed a lot of the existing code for what seems like no good reason, without improving it.
Re: [PATCH] CD-ROM improvements
Without improving it? So disc changes being handled correctly and VIDE-CDD.SYS actually detecting the CD-ROM drive is not an improvement? And yes, the loading change is also needed because VIDE-CDD.SYS seems to fail to detect a CD-ROM drive if it's master. Maybe a better way would be to allow the user to specify what IDE channel to set the CD-ROM to instead of forcing a specific one. That way both configurations can be accomodated.
Also the only changes *I* did to the API part was redoing the disc change detection since the old one was bad enough that NT 3.x and VIDE-CDD.SYS both failed to detect disc change and kept causing old contents to be listed. SA1988 did everything else. I have no idea why he didn't just add some commands without changing other things, but I don't see the harm, either. In my opinion, the emulator would benefit more from accepting the changes than from not accepting them.
Edit: I also changed makefile.mingw64 to add several files into it that you forgot to add to it.
Edit #2: Your cdrom_ioctl.c is designed for outdated 32-bit MinGW/GCC, because already since 4.9.x., the path to the DDK CD-ROM include has been same in both 64-bit and 32-bit (and certainly not the same as in 32-bit 4.7.x/4.8.x), and they also added the TOC definition there. This is the only thing preventing compiling mainline PCem on GCC 4.9.x and later. Maybe a GCC version check would be in order there.
Also the only changes *I* did to the API part was redoing the disc change detection since the old one was bad enough that NT 3.x and VIDE-CDD.SYS both failed to detect disc change and kept causing old contents to be listed. SA1988 did everything else. I have no idea why he didn't just add some commands without changing other things, but I don't see the harm, either. In my opinion, the emulator would benefit more from accepting the changes than from not accepting them.
Edit: I also changed makefile.mingw64 to add several files into it that you forgot to add to it.
Edit #2: Your cdrom_ioctl.c is designed for outdated 32-bit MinGW/GCC, because already since 4.9.x., the path to the DDK CD-ROM include has been same in both 64-bit and 32-bit (and certainly not the same as in 32-bit 4.7.x/4.8.x), and they also added the TOC definition there. This is the only thing preventing compiling mainline PCem on GCC 4.9.x and later. Maybe a GCC version check would be in order there.
Re: [PATCH] CD-ROM improvements
New ATAPI patch is attached. It does this:
- Rewrites the disc changed / not ready handler because the old one misbehaved with several DOS CD-ROM drivers, as well as with NT 3.x;
- Makes the CD-ROM IDE channel fully customizable, and defaulting to 2 (secondary master);
- Adds several new ATAPI commands.
I tried my best to keep changes to those that were necessary. Hopefully this one will be OK.
- Rewrites the disc changed / not ready handler because the old one misbehaved with several DOS CD-ROM drivers, as well as with NT 3.x;
- Makes the CD-ROM IDE channel fully customizable, and defaulting to 2 (secondary master);
- Adds several new ATAPI commands.
I tried my best to keep changes to those that were necessary. Hopefully this one will be OK.
- Attachments
-
- pcem_atapi.patch
- (60.71 KiB) Downloaded 480 times
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
Yes, that's an improvement, but changing all the #defines and playing with the indentation is not improving it, it's just changing code for the sake of it. It also makes it very difficult to see what actual improvements have been made as it shows that _everything_ has changed.Battler wrote:Without improving it? So disc changes being handled correctly and VIDE-CDD.SYS actually detecting the CD-ROM drive is not an improvement?
Also, if you don't want me to assume they're all your changes then say who's responsible! I'm not psychic, and if you post patches without saying who made the changes (and you didn't originally say specifically what changes SA1988 made) then I'm going to assume you did.
Re: [PATCH] CD-ROM improvements
Going to upload a new version of the ATAPI patch soon, as I have since found out the above's handling of disc changes is a bit awkward, so I have redone it.
Re: [PATCH] CD-ROM improvements
I really hate my cdrom code I'm using in my basilisk port, are these patches gpl able? I have no issues giving attribution, I'd love to just try them.
Re: [PATCH] CD-ROM improvements
- neozeed: They are intended for PCem which is GPL, so they are obviously GPL themselves.
Re: [PATCH] CD-ROM improvements
cool, just wanted to make sure
Re: [PATCH] CD-ROM improvements
New patach attached. Adds various improvements as well as bugfixes over the previous (and the disc change detection now actually *WORKS*, verified with several pieces of software). Also made IDE set signature on Reset ATAPI device and Drive diagnostics commands, per the spec, and writes to IDE data buffer are now discarded after packet command (A0) is issued and the 12 bytes (6 words) are written, unless a command specifically requests a write. This fixes problems with software that issues excess writes after issuing packet command. Also added the audio-related senses and made PLAY AUDIO correctly fail if either an ISO is inserted or the command was issued on a non-audio track.
- Attachments
-
- cdrom_patch_latest.patch
- (98.6 KiB) Downloaded 443 times
Re: [PATCH] CD-ROM improvements
And yet another version of the patch, with MODE SELECT-related fixes. Page 0x0E (CDROM AUDIO page) is now actually changeable (though the changes have no practical effect - but otherwise, LBA 2 hangs at the volume menu when attempting to change CD audio volume) and fixed some of the atastat and secount values used by MODE SELECT so it actually starts writing.
- Attachments
-
- cdrom_patch_latest_updated.patch
- (102.89 KiB) Downloaded 463 times
Re: [PATCH] CD-ROM improvements
Updated again. Now disc changes within a host drive while the drive remains mounted inside PCem are visible to the emulated machine.
Edit #2: Removed two pclog lines from the patch that I forgot to remove before.
Edit #2: Removed two pclog lines from the patch that I forgot to remove before.
- Attachments
-
- cdrom_patch_latest_updated_again.patch
- (106.02 KiB) Downloaded 439 times
Re: [PATCH] CD-ROM improvements
Another update. The ATAPI CDROM AUDIO mode page volume settings now actually affect CD Audio volume. Makes the CD Audio volume bar in LBA 2 work.
- Attachments
-
- cdrom_patch_latest_updated_again_2.patch
- (107.66 KiB) Downloaded 431 times
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
I haven't forgotten these changes, I just haven't had much chance lately to look at them. I've attached the current state - mostly I've changed code formatting (tabs are 8 characters wide!) and tried to neaten it up a bit.
I noticed that changing CD in Windows 95 is broken, that will have to be fixed before this can be committed.
I noticed that changing CD in Windows 95 is broken, that will have to be fixed before this can be committed.
Re: [PATCH] CD-ROM improvements
- TomWalker: The last update of my patch broke CD changing in Windows 95, but I have discovered that only after I posted it here. I'm still investigating why it's broken.
Re: [PATCH] CD-ROM improvements
Fixed the Windows 95 disc change problem. New patch attached.
- Attachments
-
- cdrom_patch_win95fixed.patch
- (108.61 KiB) Downloaded 408 times
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
Finally started getting this committed - CD-ROM channel selection committed at rev 449.
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
Quick question -
events and class are calculated but never used. Is this just dead code or did something go wrong when generating the patch?
Code: Select all
case GPCMD_GET_EVENT_STATUS_NOTIFICATION: /*0x4a*/
if (idebufferb[1] != 0)
{
ide->atastat = READY_STAT | ERR_STAT;
ide->error = (SENSE_ILLEGAL_REQUEST << 4) | ABRT_ERR;
if (atapi_sense.sensekey == SENSE_UNIT_ATTENTION)
{
ide->error |= MCR_ERR;
}
atapi_sense.asc=ASC_INV_FIELD_IN_CMD_PACKET;
ide->packetstatus = ATAPI_STATUS_ERROR;
idecallback[ide_board]=50*IDE_TIME;
break;
}
uint8_t events, class;
events = 1 << GESN_MEDIA;
class = 0;
if (idebufferb[4] & (1 << GESN_MEDIA))
{
class |= GESN_MEDIA;
len = atapi_event_status(ide, idebufferb);
}
else
{
class = 0x80;
len = idebufferb[4];
}
len=(idebufferb[7]<<8)|idebufferb[8];
ide->cylinder=len;
ide->packetstatus = ATAPI_STATUS_DATA;
ide->secount=2;
ide->pos=0;
idecallback[ide_board]=50*IDE_TIME;
ide->packlen=len;
break;
Re: [PATCH] CD-ROM improvements
I tried porting it from QEMU, but yeah, it's still primitive about that command.
Re: [PATCH] CD-ROM improvements
- TomWalker: That's a part I asked SA1988 to port from QEMU but evidently, as per usual, he only ported like half of it and claimed he was done. I am going to have a look at it later and see what he did and what's missing.
Re: [PATCH] CD-ROM improvements
Correct implementation of the GPCMD_GET_EVENT_STATUS_NOTIFICATION command:
Code: Select all
case GPCMD_GET_EVENT_STATUS_NOTIFICATION: /*0x4a*/
temp_command = idebufferb[0];
alloc_length = len;
const uint8_t *packet = idebufferb;
struct {
uint8_t opcode;
uint8_t polled;
uint8_t reserved2[2];
uint8_t class;
uint8_t reserved3[2];
uint16_t len;
uint8_t control;
} *gesn_cdb;
struct {
uint16_t len;
uint8_t notification_class;
uint8_t supported_events;
} *gesn_event_header;
unsigned int max_len, used_len;
gesn_cdb = (void *)packet;
gesn_event_header = (void *)idebufferb;
max_len = gesn_cdb->len;
/* It is fine by the MMC spec to not support async mode operations */
if (!(gesn_cdb->polled & 0x01))
{ /* asynchronous mode */
/* Only pollign is supported, asynchronous mode is not. */
ide->error = (SENSE_ILLEGAL_REQUEST << 4) | ABRT_ERR;
idecallback[ide_board]=50*IDE_TIME;
atapi_cmd_error(ide, atapi_sense.sensekey, atapi_sense.asc);
ide->atastat = 0x53;
ide->packetstatus=0x80;
atapi_sense.sensekey = SENSE_ILLEGAL_REQUEST;
atapi_sense.asc = ASC_INV_FIELD_IN_CMD_PACKET;
atapi_sense.ascq = 0;
return;
}
/* polling mode operation */
/*
* These are the supported events.
*
* We currently only support requests of the 'media' type.
* Notification class requests and supported event classes are bitmasks,
* but they are built from the same values as the "notification class"
* field.
*/
gesn_event_header->supported_events = 1 << GESN_MEDIA;
/*
* We use |= below to set the class field; other bits in this byte
* are reserved now but this is useful to do if we have to use the
* reserved fields later.
*/
gesn_event_header->notification_class = 0;
/*
* Responses to requests are to be based on request priority. The
* notification_class_request_type enum above specifies the
* priority: upper elements are higher prio than lower ones.
*/
if (gesn_cdb->class & (1 << GESN_MEDIA))
{
gesn_event_header->notification_class |= GESN_MEDIA;
used_len = atapi_event_status(ide, idebufferb);
}
else
{
gesn_event_header->notification_class = 0x80; /* No event available */
used_len = sizeof(*gesn_event_header);
}
gesn_event_header->len = used_len - sizeof(*gesn_event_header);
atapi_command_send_init(ide, temp_command, len, alloc_length);
atapi_command_ready(ide_board, len);
break;
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
How much of this code has been taken from QEMU?
Re: [PATCH] CD-ROM improvements
This that I posted tonight? Ported from QEMU in its entirety.
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
And in the main patch?
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
I admit, that last change has me slightly dubious. GPCMD_GET_EVENT_STATUS_NOTIFICATION was clearly never tested, as otherwise you'd have noticed the implementation was essentially broken. How much of the rest of the code is like that?
Re: [PATCH] CD-ROM improvements
- TomWalker: Only that part wasn't tested, and that's simply because I don't have a single piece of software that used it. The rest was not only tested, but for a lot of it I even went through each command in an ATAPI reference I found to make sure it's implemented correctly.
Edit: And in the main patch, there is not much from QEMU, especially since most if not all of the new commands added other than GET EVENT STATUS NOTIFICATION were not even present in the QEMU code, and the disc change detection was originally ported from QEMU, but then I redid it completely after I found out just how inadequate the QEMU implementation of it was.
Edit: And in the main patch, there is not much from QEMU, especially since most if not all of the new commands added other than GET EVENT STATUS NOTIFICATION were not even present in the QEMU code, and the disc change detection was originally ported from QEMU, but then I redid it completely after I found out just how inadequate the QEMU implementation of it was.
- SarahWalker
- Site Admin
- Posts: 2054
- Joined: Thu 24 Apr, 2014 4:18 pm
Re: [PATCH] CD-ROM improvements
Committed at rev 450.
Re: [PATCH] CD-ROM improvements
Please forgive my ignorance, but does this solve the infamous "DOS drivers (GSCDROM.SYS) stalling then not detecting the drive when the drive is empty" bug?
Re: [PATCH] CD-ROM improvements
- FredPJ: It should, I have tested it with VIDE-CDD.SYS version 2.15 which had the same problem and it works now.