From 8b65e841215436f5dbcd3bef04fb731c698d8431 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 19 Jun 2022 11:34:53 -0700
Subject: [PATCH 1/4] DSPHLE: Make all uCode implementations final classes

(Apart from AXUCode, which is inherited by AXWiiUCode.)
---
 Source/Core/Core/HW/DSPHLE/UCodes/AX.h    | 2 +-
 Source/Core/Core/HW/DSPHLE/UCodes/AXWii.h | 2 +-
 Source/Core/Core/HW/DSPHLE/UCodes/CARD.h  | 2 +-
 Source/Core/Core/HW/DSPHLE/UCodes/GBA.h   | 3 ++-
 Source/Core/Core/HW/DSPHLE/UCodes/INIT.h  | 2 +-
 Source/Core/Core/HW/DSPHLE/UCodes/ROM.h   | 2 +-
 Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h | 2 +-
 7 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/AX.h b/Source/Core/Core/HW/DSPHLE/UCodes/AX.h
index d95d2db27f..c51cdb34b9 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/AX.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/AX.h
@@ -62,7 +62,7 @@ enum AXMixControl
   // clang-format on
 };
 
-class AXUCode : public UCodeInterface
+class AXUCode /* not final: subclassed by AXWiiUCode */ : public UCodeInterface
 {
 public:
   AXUCode(DSPHLE* dsphle, u32 crc);
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/AXWii.h b/Source/Core/Core/HW/DSPHLE/UCodes/AXWii.h
index 27d79ba1ae..e024ee87ad 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/AXWii.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/AXWii.h
@@ -11,7 +11,7 @@ namespace DSP::HLE
 struct AXPBWii;
 class DSPHLE;
 
-class AXWiiUCode : public AXUCode
+class AXWiiUCode final : public AXUCode
 {
 public:
   AXWiiUCode(DSPHLE* dsphle, u32 crc);
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h b/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h
index 55b1e7f95d..4a071c4e29 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h
@@ -10,7 +10,7 @@ namespace DSP::HLE
 {
 class DSPHLE;
 
-class CARDUCode : public UCodeInterface
+class CARDUCode final : public UCodeInterface
 {
 public:
   CARDUCode(DSPHLE* dsphle, u32 crc);
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
index 22083d127d..37af8a5d52 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
@@ -15,8 +15,9 @@ class DSPHLE;
 // written back to RAM at the dest address provided in the crypto parameters.
 void ProcessGBACrypto(u32 address);
 
-struct GBAUCode : public UCodeInterface
+class GBAUCode final : public UCodeInterface
 {
+public:
   GBAUCode(DSPHLE* dsphle, u32 crc);
 
   void Initialize() override;
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h b/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h
index 53cbe01428..9ea8a4174e 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h
@@ -10,7 +10,7 @@ namespace DSP::HLE
 {
 class DSPHLE;
 
-class INITUCode : public UCodeInterface
+class INITUCode final : public UCodeInterface
 {
 public:
   INITUCode(DSPHLE* dsphle, u32 crc);
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/ROM.h b/Source/Core/Core/HW/DSPHLE/UCodes/ROM.h
index 431c9ed052..68e1fc0f7f 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/ROM.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/ROM.h
@@ -10,7 +10,7 @@ namespace DSP::HLE
 {
 class DSPHLE;
 
-class ROMUCode : public UCodeInterface
+class ROMUCode final : public UCodeInterface
 {
 public:
   ROMUCode(DSPHLE* dsphle, u32 crc);
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h
index 6051faf853..33963a8dd6 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h
@@ -187,7 +187,7 @@ private:
   u32 m_reverb_pb_base_addr = 0;
 };
 
-class ZeldaUCode : public UCodeInterface
+class ZeldaUCode final : public UCodeInterface
 {
 public:
   ZeldaUCode(DSPHLE* dsphle, u32 crc);

From 8d66c29f3303f564d039015ec7ea0f95a570b564 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Sun, 19 Jun 2022 12:09:32 -0700
Subject: [PATCH 2/4] DSPHLE: Eliminate global state in GBA uCode + accuracy
 improvements

The accuracy improvements are:

* The request mail must be 0xabba0000 exactly; both the low and high parts are checked
* The address is masked with 0x0fffffff
* Before, the global state meant that after the GBA uCode had been used once, it would accept 0xcdd1 commands immediately. Now, it only accepts them after execution has finished.
---
 Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp | 39 +++++++++++++++--------
 Source/Core/Core/HW/DSPHLE/UCodes/GBA.h   | 12 +++++++
 2 files changed, 38 insertions(+), 13 deletions(-)

diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp
index f6eae73088..767aeda2ba 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp
@@ -89,28 +89,44 @@ void GBAUCode::Update()
 
 void GBAUCode::HandleMail(u32 mail)
 {
-  static bool nextmail_is_mramaddr = false;
-  static bool calc_done = false;
-
   if (m_upload_setup_in_progress)
   {
     PrepareBootUCode(mail);
+    // The GBA ucode ignores the first 3 mails (mram_dest_addr, mram_size, mram_dram_addr)
+    // but we currently don't handle that (they're read when they shoudln't be, but DSP HLE doesn't
+    // implement them so it's fine).
+    return;
   }
-  else if ((mail >> 16 == 0xabba) && !nextmail_is_mramaddr)
+
+  switch (m_mail_state)
   {
-    nextmail_is_mramaddr = true;
+  case MailState::WaitingForRequest:
+  {
+    if (mail == REQUEST_MAIL)
+    {
+      INFO_LOG_FMT(DSPHLE, "GBAUCode - Recieved request mail");
+      m_mail_state = MailState::WaitingForAddress;
+    }
+    else
+    {
+      WARN_LOG_FMT(DSPHLE, "GBAUCode - Expected request mail but got {:08x}", mail);
+    }
+    break;
   }
-  else if (nextmail_is_mramaddr)
+  case MailState::WaitingForAddress:
   {
-    nextmail_is_mramaddr = false;
+    const u32 address = mail & 0x0fff'ffff;
 
-    ProcessGBACrypto(mail);
+    ProcessGBACrypto(address);
 
-    calc_done = true;
     m_mail_handler.PushMail(DSP_DONE);
+    m_mail_state = MailState::WaitingForNextTask;
+    break;
   }
-  else if (((mail & TASK_MAIL_MASK) == TASK_MAIL_TO_DSP) && calc_done)
+  case MailState::WaitingForNextTask:
   {
+    // The GBA uCode checks that the high word is cdd1, so we compare the full mail with
+    // MAIL_NEW_UCODE/MAIL_RESET without doing masking
     switch (mail)
     {
     case MAIL_NEW_UCODE:
@@ -124,9 +140,6 @@ void GBAUCode::HandleMail(u32 mail)
       break;
     }
   }
-  else
-  {
-    WARN_LOG_FMT(DSPHLE, "GBAUCode - unknown command: {:08x}", mail);
   }
 }
 }  // namespace DSP::HLE
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
index 37af8a5d52..842797ad72 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
@@ -23,5 +23,17 @@ public:
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+
+private:
+  static constexpr u32 REQUEST_MAIL = 0xabba0000;
+
+  enum class MailState
+  {
+    WaitingForRequest,
+    WaitingForAddress,
+    WaitingForNextTask,
+  };
+
+  MailState m_mail_state = MailState::WaitingForRequest;
 };
 }  // namespace DSP::HLE

From f2e833b5c474c8f5e42d413c8994174bc6fccd16 Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Wed, 22 Jun 2022 13:20:00 -0700
Subject: [PATCH 3/4] DSPHLE: Eliminate global state in AX uCode

This also increases accuracy as to when specific mail is allowed, and correctly handles masking of the 0xCDD1 mails.
---
 Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp | 105 +++++++++++++----------
 Source/Core/Core/HW/DSPHLE/UCodes/AX.h   |   9 ++
 2 files changed, 71 insertions(+), 43 deletions(-)

diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp
index c53d7a8e8e..7a7d9e5d39 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/AX.cpp
@@ -653,53 +653,72 @@ void AXUCode::SendAUXAndMix(u32 main_auxa_up, u32 auxb_s_up, u32 main_l_dl, u32
 
 void AXUCode::HandleMail(u32 mail)
 {
-  // Indicates if the next message is a command list address.
-  static bool next_is_cmdlist = false;
-  static u16 cmdlist_size = 0;
-
-  bool set_next_is_cmdlist = false;
-
-  if (next_is_cmdlist)
+  if (m_upload_setup_in_progress)
   {
-    CopyCmdList(mail, cmdlist_size);
+    PrepareBootUCode(mail);
+    return;
+  }
+
+  switch (m_mail_state)
+  {
+  case MailState::WaitingForCmdListSize:
+    if ((mail & MAIL_CMDLIST_MASK) == MAIL_CMDLIST)
+    {
+      // A command list address is going to be sent next.
+      m_cmdlist_size = static_cast<u16>(mail & ~MAIL_CMDLIST_MASK);
+      m_mail_state = MailState::WaitingForCmdListAddress;
+    }
+    else
+    {
+      ERROR_LOG_FMT(DSPHLE, "Unknown mail sent to AX::HandleMail; expected command list: {:08x}",
+                    mail);
+    }
+    break;
+
+  case MailState::WaitingForCmdListAddress:
+    CopyCmdList(mail, m_cmdlist_size);
     HandleCommandList();
     m_cmdlist_size = 0;
     SignalWorkEnd();
-  }
-  else if (m_upload_setup_in_progress)
-  {
-    PrepareBootUCode(mail);
-  }
-  else if (mail == MAIL_RESUME)
-  {
-    // Acknowledge the resume request
-    m_mail_handler.PushMail(DSP_RESUME, true);
-  }
-  else if (mail == MAIL_NEW_UCODE)
-  {
-    m_upload_setup_in_progress = true;
-  }
-  else if (mail == MAIL_RESET)
-  {
-    m_dsphle->SetUCode(UCODE_ROM);
-  }
-  else if (mail == MAIL_CONTINUE)
-  {
-    // We don't have to do anything here - the CPU does not wait for a ACK
-    // and sends a cmdlist mail just after.
-  }
-  else if ((mail & MAIL_CMDLIST_MASK) == MAIL_CMDLIST)
-  {
-    // A command list address is going to be sent next.
-    set_next_is_cmdlist = true;
-    cmdlist_size = (u16)(mail & ~MAIL_CMDLIST_MASK);
-  }
-  else
-  {
-    ERROR_LOG_FMT(DSPHLE, "Unknown mail sent to AX::HandleMail: {:08x}", mail);
-  }
+    m_mail_state = MailState::WaitingForNextTask;
+    break;
 
-  next_is_cmdlist = set_next_is_cmdlist;
+  case MailState::WaitingForNextTask:
+    if ((mail & TASK_MAIL_MASK) != TASK_MAIL_TO_DSP)
+    {
+      WARN_LOG_FMT(DSPHLE, "Rendering task without prefix CDD1: {:08x}", mail);
+      mail = TASK_MAIL_TO_DSP | (mail & ~TASK_MAIL_MASK);
+      // The actual uCode does not check for the CDD1 prefix.
+    }
+
+    switch (mail)
+    {
+    case MAIL_RESUME:
+      // Acknowledge the resume request
+      m_mail_handler.PushMail(DSP_RESUME, true);
+      m_mail_state = MailState::WaitingForCmdListSize;
+      break;
+
+    case MAIL_NEW_UCODE:
+      m_upload_setup_in_progress = true;
+      break;
+
+    case MAIL_RESET:
+      m_dsphle->SetUCode(UCODE_ROM);
+      break;
+
+    case MAIL_CONTINUE:
+      // We don't have to do anything here - the CPU does not wait for a ACK
+      // and sends a cmdlist mail just after.
+      m_mail_state = MailState::WaitingForCmdListSize;
+      break;
+
+    default:
+      WARN_LOG_FMT(DSPHLE, "Unknown task mail: {:08x}", mail);
+      break;
+    }
+    break;
+  }
 }
 
 void AXUCode::CopyCmdList(u32 addr, u16 size)
@@ -712,7 +731,6 @@ void AXUCode::CopyCmdList(u32 addr, u16 size)
 
   for (u32 i = 0; i < size; ++i, addr += 2)
     m_cmdlist[i] = HLEMemory_Read_U16(addr);
-  m_cmdlist_size = size;
 }
 
 void AXUCode::Update()
@@ -728,6 +746,7 @@ void AXUCode::DoAXState(PointerWrap& p)
 {
   p.Do(m_cmdlist);
   p.Do(m_cmdlist_size);
+  p.Do(m_mail_state);
 
   p.Do(m_samples_main_left);
   p.Do(m_samples_main_right);
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/AX.h b/Source/Core/Core/HW/DSPHLE/UCodes/AX.h
index c51cdb34b9..d63659a995 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/AX.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/AX.h
@@ -200,5 +200,14 @@ private:
     CMD_COMPRESSOR = 0x12,
     CMD_SEND_AUX_AND_MIX = 0x13,
   };
+
+  enum class MailState
+  {
+    WaitingForCmdListSize,
+    WaitingForCmdListAddress,
+    WaitingForNextTask,
+  };
+
+  MailState m_mail_state = MailState::WaitingForCmdListSize;
 };
 }  // namespace DSP::HLE

From bf7002672891adb3712a028472e28511028c569b Mon Sep 17 00:00:00 2001
From: Pokechu22 <Pokechu022@gmail.com>
Date: Mon, 20 Jun 2022 12:37:58 -0700
Subject: [PATCH 4/4] DSPHLE: Require implementing DoState

CARDUCode, GBAUCode, and INITUCode previously didn't have an implementation of it. In practice it's unlikely that this caused an issue, since these uCodes are only active for a few frames at most, but now that GBAUCode doesn't have global state, we can implement it there. I also implemented it for CARDUCode, although our CARDUCode implementation does not have all states handled yet - this is simply future-proofing so that when the card uCode is properly implemented, the save state version does not need to be bumped. INITUCode does not have any state to save, though.
---
 Source/Core/Core/HW/DSPHLE/UCodes/CARD.cpp |  7 +++++++
 Source/Core/Core/HW/DSPHLE/UCodes/CARD.h   | 12 ++++++++++++
 Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp  |  7 +++++++
 Source/Core/Core/HW/DSPHLE/UCodes/GBA.h    |  1 +
 Source/Core/Core/HW/DSPHLE/UCodes/INIT.cpp |  5 +++++
 Source/Core/Core/HW/DSPHLE/UCodes/INIT.h   |  1 +
 Source/Core/Core/HW/DSPHLE/UCodes/UCodes.h |  2 +-
 Source/Core/Core/State.cpp                 |  2 +-
 8 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/CARD.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/CARD.cpp
index 4ad47757f8..43d145f75e 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/CARD.cpp
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/CARD.cpp
@@ -3,6 +3,7 @@
 
 #include "Core/HW/DSPHLE/UCodes/CARD.h"
 
+#include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
 #include "Core/HW/DSP.h"
@@ -44,4 +45,10 @@ void CARDUCode::HandleMail(u32 mail)
   m_mail_handler.PushMail(DSP_DONE);
   m_dsphle->SetUCode(UCODE_ROM);
 }
+
+void CARDUCode::DoState(PointerWrap& p)
+{
+  DoStateShared(p);
+  p.Do(m_state);
+}
 }  // namespace DSP::HLE
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h b/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h
index 4a071c4e29..f1790b98d4 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/CARD.h
@@ -18,5 +18,17 @@ public:
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+  void DoState(PointerWrap& p) override;
+
+private:
+  enum class State
+  {
+    WaitingForRequest,
+    WaitingForAddress,
+    WaitingForNextTask,
+  };
+
+  // Currently unused, will be used in a later version
+  State m_state = State::WaitingForRequest;
 };
 }  // namespace DSP::HLE
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp
index 767aeda2ba..6c1a46cadc 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.cpp
@@ -4,6 +4,7 @@
 #include "Core/HW/DSPHLE/UCodes/GBA.h"
 
 #include "Common/Align.h"
+#include "Common/ChunkFile.h"
 #include "Common/CommonTypes.h"
 #include "Common/Logging/Log.h"
 #include "Core/HW/DSP.h"
@@ -142,4 +143,10 @@ void GBAUCode::HandleMail(u32 mail)
   }
   }
 }
+
+void GBAUCode::DoState(PointerWrap& p)
+{
+  DoStateShared(p);
+  p.Do(m_mail_state);
+}
 }  // namespace DSP::HLE
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
index 842797ad72..a2cf96869a 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/GBA.h
@@ -23,6 +23,7 @@ public:
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+  void DoState(PointerWrap& p) override;
 
 private:
   static constexpr u32 REQUEST_MAIL = 0xabba0000;
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/INIT.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/INIT.cpp
index 7869460bc1..cf5204df1c 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/INIT.cpp
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/INIT.cpp
@@ -28,4 +28,9 @@ void INITUCode::Update()
 void INITUCode::HandleMail(u32 mail)
 {
 }
+
+void INITUCode::DoState(PointerWrap& p)
+{
+  // We don't need to call DoStateShared() as the init uCode doesn't support launching new uCode
+}
 }  // namespace DSP::HLE
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h b/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h
index 9ea8a4174e..fb1bd7860d 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/INIT.h
@@ -18,5 +18,6 @@ public:
   void Initialize() override;
   void HandleMail(u32 mail) override;
   void Update() override;
+  void DoState(PointerWrap& p) override;
 };
 }  // namespace DSP::HLE
diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.h b/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.h
index 408c1e1239..029713ca60 100644
--- a/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.h
+++ b/Source/Core/Core/HW/DSPHLE/UCodes/UCodes.h
@@ -46,7 +46,7 @@ public:
   virtual void HandleMail(u32 mail) = 0;
   virtual void Update() = 0;
 
-  virtual void DoState(PointerWrap& p) { DoStateShared(p); }
+  virtual void DoState(PointerWrap& p) = 0;
   static u32 GetCRC(UCodeInterface* ucode) { return ucode ? ucode->m_crc : UCODE_NULL; }
 
 protected:
diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp
index c7c2aee8dd..d4abf52bcd 100644
--- a/Source/Core/Core/State.cpp
+++ b/Source/Core/Core/State.cpp
@@ -74,7 +74,7 @@ static std::recursive_mutex g_save_thread_mutex;
 static std::thread g_save_thread;
 
 // Don't forget to increase this after doing changes on the savestate system
-constexpr u32 STATE_VERSION = 147;  // Last changed in PR 10935
+constexpr u32 STATE_VERSION = 148;  // Last changed in PR 10768
 
 // Maps savestate versions to Dolphin versions.
 // Versions after 42 don't need to be added to this list,