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

Support ATAPI raw sector reading with READ CD command #3622

Merged
merged 2 commits into from
Jul 10, 2022

Conversation

schellingb
Copy link
Contributor

This PR contains 2 fixes for the ATAPI IDE emulation regarding the READ CD command.

What issue(s) does this PR address?

Maybe #585? It was the only one that came up with my searches.

Together with the PR #3623 it partially addresses #211.

Does this PR introduce new feature(s)?

  1. Fix the ATAPI READ CD (0xBE) command by adding a missing break statement. Without it, it would accidentally fall through into the next case which would then overwrite TransferLength with an invalid value which then can cause memory corruption in the following call to on_atapi_busy_time by calling ReadSectorsHost with a TransferLength that fills the 64k sector buffer past its length.

  2. Add support for reading raw and CDDA sectors of size 2352 bytes. It does so by passing true for the raw argument of the ReadSectorsHost function which is already equipped to read 2352 bytes that way. Implementations for other transfer types and lengths are not part of this PR.

Does this PR introduce any breaking change(s)?

No, it only adds new functionality where otherwise "Unsupported sector type" warnings would have been logged.

Additional information

To fully comply with the READ CD command described in the ATAPI standard, ide.cpp would need to know more about the track layout of the CD. For example information if it's a MODE2 track or track boundaries to refuse when requested to read across them.

Maybe it would make more sense to change ReadSectorsHost to something like ReadSectorsATAPI which gets called with all the information (TransferSectorType, TransferSectorSize, TransferLength) and have the CDROM_Interface implementation comply with the request in a manner that matches the standard.

Wengier and others added 2 commits July 10, 2022 14:20
Add missing break statement and add support for reading raw sectors.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants