ntrteam / flashcart_core

A hopefully reusable component for dealing with flashcart specific behavior.
GNU General Public License v3.0
128 stars 32 forks source link

r4isdhc: Fix non 3ds platform #94

Closed d3m3vilurr closed 6 years ago

d3m3vilurr commented 6 years ago
  1. DSi system requires bypass cartridge reset and BF initialize

    • platform not support fully cartridge reset
    • BF value already ready into flashcart
  2. Very first step not return 0xFFFFFFFF

angelsl commented 6 years ago

I don't think BYPASS_CART_INIT is accurate enough; the real issue is that flashcart_core gains control when the cart is already in KEY2, and that should be what is reflected to the flashcart class. The result, of course, may be that it skips trying RAW initialisation, etc.

The platform shouldn't be concerned about whether to override the flags sent to it from flashcart_core either — the role of the platform class is to glue the (N/3)DS-like interface provided in ntrcard in flashcart_core to whatever interface the platform provides, whether it be an NDS, DSi or 3DS, so flashcart_core should just send the correct flags.

d3m3vilurr commented 6 years ago

actually, it depends on the system. and your approach also has unclear sections.

I guess you want to generate state-machine into the platform namespace and specific cart class. right? but this approach has many cons cases;

  1. my patch set just setting up on/off flags or ignore. and next device also easily inject control; because, the case is very clear. 1) need cart and blowfish initialize 2) cart going out secure section
  2. other hands, your patchset requires more controls related mode and system flags.1
  3. increase complex always makes hazard 2
  4. actually i don't know understand HAS_HW_KEY2 flags, system always require this. but you forget, some system(like dsi) requires already set SEC flags, and bf loaded. we need remove and change flag something, not need split device type. for example, how about ak2i, can you make right flag for the dsi flasher?
angelsl commented 6 years ago

actually, it depends on the system.

Yes. That is exactly why we are adding more platform flags.

and your approach also has unclear sections.

What exactly are they?

\1. my patch set just setting up on/off flags or ignore

Your patch is breaking the (attempt at) encapsulation we made when we separated the platform-specific parts from the actual NTR cart protocol.

... I don't want this to become a case of your approach vs my approach, but it seems ultimately it comes down to that.

1) need cart and blowfish initialize 2) cart going out secure section

My branch does, at the moment, account for more cases than are necessary.

\2. other hands, your patchset requires more controls related mode and system flags.

My branch requires only two additional const bools in the flasher. The rest are changes in flashcart_core.

\3. increase complex always makes hazard

As I said, my branch does, at the moment, account for more cases than are necessary. Also, that code you linked to was written in a hurry. The duplicated parts will be cleaned up.

\4. actually i don't know understand HAS_HW_KEY2 flags

This flag sets whether the platform has hardware support for KEY2. If it is off, flashcart_core will do KEY2 in software (as yet unimplemented). It should be true on all DS and 3DS platforms. It has nothing to do with this issue at all.

My bad for not documenting the flag properly.

how about ak2i, can you make right flag for the dsi flasher?

Yes. By tracking the encryption state in the huge state global in ntrcard, we know whether we should apply the KEY2 flags or not.

d3m3vilurr commented 6 years ago

Yes. That is exactly why we are adding more platform flags.

I'm disagree. And I'm not blame you. I just say, we need discussion about decision.

Your patch is breaking the (attempt at) encapsulation we made when we separated the platform-specific parts from the actual NTR cart protocol.

it's c++'s bad side. sometimes transparency system cannot encapsulation, and if looks not much benefit in the change, we should check other point.

... I don't want this to become a case of your approach vs my approach, but it seems ultimately it comes down to that.

I don't want to that, neither. and I'm sorry. But probably one pr will be drop.

My branch requires only two additional const bools in the flasher. The rest are changes in flashcart_core.

I don't think so. Your change will impact both side.

This flag sets whether the platform has hardware support for KEY2. If it is off, flashcart_core will do KEY2 in software (as yet unimplemented). It should be true on all DS and 3DS platforms. It has nothing to do with this issue at all.

I can't judge this flag's usefulness, but in my case that flags is useless. also I think, initKey2Seed should be not weak.

but it's outside of this PR.

My bad for not documenting the flag properly.

NP, I was just uncareful.

Yes. By tracking the encryption state in the huge state global in ntrcard, we know whether we should apply the KEY2 flags or not.

That is reason of why I said cannot encapsulation, this jail has risk at future process to be more complecate and sensitive.

kitlith commented 6 years ago

oh right, a summary was never made here. uhhhhhh... yeah that isn't going to happen right now.

so we're going to be using @angelsl's solution. @d3m3vilurr still has a few reservations (safety concerns?), but doesn't have any major complaints against their solution.