Skip to content

Commit

Permalink
A small fix and addition for a TEAC CD-ROM driver (#105)
Browse files Browse the repository at this point in the history
This fixes the CD-ROM hardware by making sure the `drive_ready` flag is
zero after an `ATAPI_DEVICE_RESET` command. As far as I know, no other
Guest needed this fix except for the TEAC-CDI driver referred to below.

This also adds an assumption fix so that the TEAC_CDI driver will work.
The TEAC_CDI driver relies on the fact that the TEAC drive it is
designed for, sets the `interrupt reason` register (`sector count`
register) after the `ATAPI_IDENTIFY` command is issued. It uses this
`i_o` bit of this register to determine the direction of transfer for
the 0xA1 command, even though this is a read-only direction command.

Through other tests, I see no other Guests effected by this addition.

I also added a few minor syntax modifications, tab spacing, and
comments. The DEBUG out string will display the ATAPI command as well as
the 0xA0 command.
  • Loading branch information
fysnet authored Oct 22, 2023
1 parent ab1e94b commit bfba293
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 17 deletions.
45 changes: 36 additions & 9 deletions bochs/iodev/harddrv.cc
Original file line number Diff line number Diff line change
Expand Up @@ -809,12 +809,17 @@ Bit32u bx_hard_drive_c::read(Bit32u address, unsigned io_len)
switch (port) {
case 0x00: // hard disk data (16bit) 0x1f0
if (controller->status.drq == 0) {
BX_ERROR(("IO read(0x%04x) with drq == 0: last command was %02xh",
address, (unsigned) controller->current_command));
return(0);
BX_ERROR(("IO read(0x%04x) with drq == 0: last command was %02xh",
address, (unsigned) controller->current_command));
return(0);
}
BX_DEBUG(("IO read(0x%04x): current command is %02xh",
if (controller->current_command == 0xa0) {
BX_DEBUG(("IO read(0x%04x): current command is a0h/%02xh",
address, (unsigned) BX_SELECTED_DRIVE(channel).atapi.command));
} else {
BX_DEBUG(("IO read(0x%04x): current command is %02xh",
address, (unsigned) controller->current_command));
}
switch (controller->current_command) {
case 0x20: // READ SECTORS, with retries
case 0x21: // READ SECTORS, without retries
Expand Down Expand Up @@ -1913,8 +1918,13 @@ void bx_hard_drive_c::write(Bit32u address, Bit32u value, unsigned io_len)
break;

default:
BX_PANIC(("IO write(0x%04x): current command is %02xh", address,
(unsigned) controller->current_command));
if (controller->current_command == 0xa0) {
BX_PANIC(("IO read(0x%04x): current command is a0h/%02xh",
address, (unsigned) BX_SELECTED_DRIVE(channel).atapi.command));
} else {
BX_PANIC(("IO write(0x%04x): current command is %02xh",
address, (unsigned) controller->current_command));
}
}
break;

Expand Down Expand Up @@ -1957,7 +1967,8 @@ void bx_hard_drive_c::write(Bit32u address, Bit32u value, unsigned io_len)
{
if ((value & 0xa0) != 0xa0) // 1x1xxxxx
BX_DEBUG(("IO write 0x%04x (%02x): not 1x1xxxxxb", address, (unsigned) value));
Bit32u drvsel = BX_HD_THIS channels[channel].drive_select = (value >> 4) & 0x01;
Bit32u drvsel =
BX_HD_THIS channels[channel].drive_select = (value >> 4) & 1;
WRITE_HEAD_NO(channel,value & 0xf);
if (!controller->lba_mode && ((value >> 6) & 1) == 1)
BX_DEBUG(("enabling LBA mode"));
Expand Down Expand Up @@ -2333,6 +2344,14 @@ void bx_hard_drive_c::write(Bit32u address, Bit32u value, unsigned io_len)
controller->status.write_fault = 0;
controller->status.drq = 1;

// The TEAC-CDI driver relies on the TEAC drive to set its
// 'interrupt_reason' register, so the driver can determine
// the direction of the transfer even though a CD-ROM is readonly.
// atapi-4/5: 'Sector count' register after this command is N/A
controller->interrupt_reason.i_o = 1;
controller->interrupt_reason.c_d = 0;
controller->interrupt_reason.rel = 0;

controller->status.seek_complete = 1;
controller->status.corrected_data = 0;

Expand All @@ -2358,7 +2377,12 @@ void bx_hard_drive_c::write(Bit32u address, Bit32u value, unsigned io_len)
if (BX_SELECTED_IS_CD(channel)) {
set_signature(channel, BX_SLAVE_SELECTED(channel));

controller->status.busy = 1;
// specs say the busy bit needs to be set, though
// we don't have any mechanism to clear it after the
// command, so we clear it just a few lines below.
// (also, the drive_ready must be zero)
//controller->status.busy = 1;
controller->status.drive_ready = 0;

This comment has been minimized.

Copy link
@vruppert

vruppert Nov 24, 2023

Contributor

The DOS cdrom driver for OTI-910 fails to detect CD-ROM drive after these changes. When I disable the last line of the changes (drive_ready = 0), the detection works again.

This comment has been minimized.

Copy link
@Vort

Vort Nov 24, 2023

Contributor

It is better to attach file right away in such cases: NEC_IDE.ZIP.
I also confirm such behaviour of the driver.

This comment has been minimized.

Copy link
@vruppert

vruppert Nov 24, 2023

Contributor

After turning on debug messages when loading the driver I noticed that the 'seek complete' bit is still set after ATAPI device reset. After clearing it both CD-ROM drivers work as expected.

This comment has been minimized.

Copy link
@Vort

Vort Nov 24, 2023

Contributor

I confirm that fix from e9cd258 works both for NEC_IDE.SYS and TEAC_CDI.SYS.

controller->error_register &= ~(1 << 7);

controller->status.write_fault = 0;
Expand Down Expand Up @@ -3291,7 +3315,10 @@ void bx_hard_drive_c::command_aborted(Bit8u channel, unsigned value)
{
controller_t *controller = &BX_SELECTED_CONTROLLER(channel);

BX_DEBUG(("aborting on command 0x%02x {%s}", value, BX_SELECTED_TYPE_STRING(channel)));
if (value == 0xa0)
BX_DEBUG(("aborting on command 0xa0/0x%02x {%s}", BX_SELECTED_DRIVE(channel).atapi.command, BX_SELECTED_TYPE_STRING(channel)));
else
BX_DEBUG(("aborting on command 0x%02x {%s}", value, BX_SELECTED_TYPE_STRING(channel)));
controller->current_command = 0;
controller->status.busy = 0;
controller->status.drive_ready = 1;
Expand Down
16 changes: 8 additions & 8 deletions bochs/iodev/harddrv.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,15 @@ class cdrom_base_c;

typedef struct {
struct {
bool busy;
bool drive_ready;
bool write_fault;
bool seek_complete;
bool drq;
bool corrected_data;
bool index_pulse;
bool busy; // bit 7
bool drive_ready; // bit 6
bool write_fault; // bit 5
bool seek_complete; // bit 4
bool drq; // bit 3
bool corrected_data; // bit 2
bool index_pulse; // bit 1
bool err; // bit 0
unsigned index_pulse_count;
bool err;
} status;
Bit8u error_register;
Bit8u head_no;
Expand Down

0 comments on commit bfba293

Please sign in to comment.