Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add XGD support for Kreon Drives #181
Add XGD support for Kreon Drives #181
Changes from 23 commits
a9c5b63
4dc507a
8713cd4
b16c0c9
95cb887
b250aa4
7bf77e5
554d521
091a376
dbb9caa
2e72289
e3d3ea0
0ac21a7
2b93df0
09999fb
b169b21
174fefd
f6a21fb
3f8177c
51f1363
9361f67
1faf9d1
6e527ae
e65c19a
11971f9
dd81526
5e2f986
ad95833
a3e4c7f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the purpose of clean_ss here, is it just to make sure we refine the same disc or there is more?
Can't ".raw_ss" be used for that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
.raw_ss
is the security sector as received by the drive, this cleaned version is the one that is used for redump submissions as (I might get some details wrong here) my understanding is the security sector contains disc angle data, and if there was a new manufacturing run of a disc with the same data the security sector would have different angles in it. This cleaned version overwrites that angle data in order to make sure that we can check if the rest of the security sector is the same. @Deterous do you happen to know more details about this?For the non-dump runs though we can check the
.raw_ss
if you think that would be better. The goal of doing the cleaned version for the comparison is if you have two of the same disc you would be able to verify that the contents of the disc are the same even if it has different angles in the security sector.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may has misinterpreted this, if you are referring to the variable itself, yes I think we can reuse the
security_sector
variable and don't need to copy to a new vectorThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reason for clean_ss is both for making sure we refine the same disc as you mentioned, and also for redump. The clean SS is the hash that redump puts in comments, and it will match the output of ss_sector_range (dumpers won't have to run that program anymore).
We had to put it in
redumper dump
code because we use it for checking same disc is being refined as you mentioned.The
.raw_ss
contents differ between dumps of the same disc, the cleaning function is meant to remove the parts that differ.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so we have two things:
Specifically, I don't like outputting both files with otherwise identical content when using
redumper dump
command. This command should always store only what was received from the drive in the most RAW formats.Here's what I would like to do here:
(1) should be dumped as .security
(2) should be generated from (1) on refine in memory and compared to the one from disc to ensure we refine the same disc
Hash for the cleaned one should be printed in
redumper info
command from (1) when it detects xbox dump.Optionally, (2) can be generated during
redumper info
using a command line option. We have a similar problem with PSX discs where we want to generate .sbi file for libcrypt games (generated from raw .subcode). I have to think what would be the proper implementation of this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So just to make sure I understand everything correctly:
.security
file and for the current disc to be able to do the comparison (but don't save anything to disc, only exists in memory for the given run)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, basically load .security file to memory, clear angle data, read security sector from the disc, clear it's angle data and compare the two.
That should be the output of
redumper info
command that analyzes dump files and do not access the drive at all. If xbox dump is detected, read .security file, clear angle data in memory, print hash and that's it.That should come as a separate PR after we're done with this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay great, that is pretty straight forward. Looking at the code I have one more question.
Currently only one user has reported this happening (so I think it should be uncommon) but their drive sometimes fails to send the full SS sequence leading to only a partial SS being received, in the current code I save it as the
.security
and just log that it is incomplete but didn't perform the cleanup. Now that we are only outputting the.security
file should I:a) Continue to save it anyways
b) Save it but change the extension to something like
.partial_security
c) Do NOT save it (should we attempt to continue the dump (theoretically should be fine) or force a failure)
note in all of these cases it would log that it was incomplete
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer (a). We can't always troubleshoot drive reliability so
redumper dump
does best effort. Warning about it being incomplete is nice.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for the lack of response the past few months, I got caught up with some stuff and then never revisited this.
I believe I have updated the behavior to address everything so now it will:
.security
file.security
and clean it so we can do a proper compare without worrying about faked anglesI re-tested with an XGD1 and XGD3 disc and both dumped correctly. I deleted some random portion of
.state
for the XGD1 after it completed and did a refine and it completed and the SHA matched (did a manual SHA-1 compute of the ISO after refine)