Kernel: Cleanup AML code and fix bugs

I can enter ACPI mode on my own laptop!

kernel/include/kernel/ACPI/AML/Expression.h

@@ -18,11 +18,51 @@ namespace Kernel::ACPI::AML
 				// unary
 				case AML::Byte::IncrementOp:
 				case AML::Byte::DecrementOp:
+				{
+					auto opcode = (static_cast<AML::Byte>(context.aml_data[0]) == AML::Byte::IncrementOp) ? AML::Byte::AddOp : AML::Byte::SubtractOp;
+					context.aml_data = context.aml_data.slice(1);
+
+					auto source_result = AML::parse_object(context);
+					if (!source_result.success())
+						return ParseResult::Failure;
+					auto source_node = source_result.node() ? source_result.node()->evaluate() : BAN::RefPtr<AML::Node>();
+					if (!source_node || source_node->type != AML::Node::Type::Integer)
+					{
+						AML_ERROR("UnaryOp source not integer");
+						return ParseResult::Failure;
+					}
+
+					auto source_integer = static_cast<AML::Integer*>(source_node.ptr());
+					if (source_integer->constant)
+					{
+						AML_ERROR("UnaryOp source is constant");
+						return ParseResult::Failure;
+					}
+
+					source_integer->value += (opcode == AML::Byte::AddOp) ? 1 : -1;
+					return ParseResult(source_integer);
+				}
 				case AML::Byte::NotOp:
-				case AML::Byte::LNotOp:
+					AML_TODO("NotOp", context.aml_data[0]);
-					AML_TODO("Expression {2H}", context.aml_data[0]);
 					return ParseResult::Failure;
-				// binary
+				case AML::Byte::LNotOp:
+				{
+					context.aml_data = context.aml_data.slice(1);
+
+					auto node_result = AML::parse_object(context);
+					if (!node_result.success())
+						return ParseResult::Failure;
+
+					auto value = node_result.node() ? node_result.node()->as_integer() : BAN::Optional<uint64_t>();
+					if (!value.has_value())
+					{
+						AML_ERROR("Logical NotOp source is not integer");
+						return ParseResult::Failure;
+					}
+
+					auto result = value.value() ? Integer::Constants::Zero : Integer::Constants::Ones;
+					return ParseResult(result);
+				}
 				case AML::Byte::AddOp:
 				case AML::Byte::AndOp:
 				case AML::Byte::ModOp:
@@ -34,52 +74,15 @@ namespace Kernel::ACPI::AML
 				case AML::Byte::ShiftRightOp:
 				case AML::Byte::SubtractOp:
 				case AML::Byte::XorOp:
+					return parse_binary_op(context);
 				case AML::Byte::LAndOp:
 				case AML::Byte::LEqualOp:
 				case AML::Byte::LGreaterOp:
 				case AML::Byte::LLessOp:
 				case AML::Byte::LOrOp:
-				{
+					return parse_logical_binary_op(context);
-					auto opcode = static_cast<AML::Byte>(context.aml_data[0]);
-					context.aml_data = context.aml_data.slice(1);
-
-					auto lhs_result = AML::parse_object(context);
-					if (!lhs_result.success())
-						return ParseResult::Failure;
-					auto lhs_node = lhs_result.node();
-					if (!lhs_node)
-					{
-						AML_ERROR("LHS object is null");
-						return ParseResult::Failure;
-					}
-					auto lhs = lhs_node->evaluate();
-					if (!lhs)
-					{
-						AML_ERROR("Failed to evaluate LHS object");
-						return ParseResult::Failure;
-					}
-
-					auto rhs_result = AML::parse_object(context);
-					if (!rhs_result.success())
-						return ParseResult::Failure;
-					auto rhs_node = rhs_result.node();
-					if (!rhs_node)
-					{
-						AML_ERROR("RHS object is null");
-						return ParseResult::Failure;
-					}
-					auto rhs = rhs_node->evaluate();
-					if (!rhs)
-					{
-						AML_ERROR("Failed to evaluate RHS object");
-						return ParseResult::Failure;
-					}
-
-					return parse_binary_op(context, opcode, lhs, rhs);
-				}
-				// trinary
 				case AML::Byte::DivideOp:
-					AML_TODO("Expression {2H}", context.aml_data[0]);
+					AML_TODO("DivideOp");
 					return ParseResult::Failure;
 				default:
 					ASSERT_NOT_REACHED();
@@ -87,20 +90,56 @@ namespace Kernel::ACPI::AML
 		}
 
 	private:
-		static ParseResult parse_binary_op(ParseContext& context, AML::Byte opcode, BAN::RefPtr<AML::Node> lhs_node, BAN::RefPtr<AML::Node> rhs_node)
+		static ParseResult parse_binary_op(ParseContext& context)
 		{
-			if (lhs_node->type != AML::Node::Type::Integer)
+			auto opcode = static_cast<AML::Byte>(context.aml_data[0]);
-			{
+			context.aml_data = context.aml_data.slice(1);
-				AML_TODO("LHS object is not an integer, type {}", static_cast<uint8_t>(lhs_node->type));
+
+			auto lhs_result = AML::parse_object(context);
+			if (!lhs_result.success())
 				return ParseResult::Failure;
-			}
+			auto lhs_value = lhs_result.node() ? lhs_result.node()->as_integer() : BAN::Optional<uint64_t>();
-			if (rhs_node->type != AML::Node::Type::Integer)
+			if (!lhs_value.has_value())
 			{
-				AML_TODO("RHS object is not an integer, type {}", static_cast<uint8_t>(rhs_node->type));
+				AML_ERROR("BinaryOP {2H} LHS not an integer", static_cast<uint8_t>(opcode));
+				if (lhs_result.node())
+					lhs_result.node()->debug_print(1);
 				return ParseResult::Failure;
 			}
 
-			bool logical = false;
+			auto rhs_result = AML::parse_object(context);
+			if (!rhs_result.success())
+				return ParseResult::Failure;
+			auto rhs_value = lhs_result.node() ? rhs_result.node()->as_integer() : BAN::Optional<uint64_t>();
+			if (!rhs_value.has_value())
+			{
+				AML_ERROR("BinaryOP {2H} RHS not an integer", static_cast<uint8_t>(opcode));
+				if (rhs_result.node())
+					rhs_result.node()->debug_print(1);
+				return ParseResult::Failure;
+			}
+
+			if (context.aml_data.size() < 1)
+			{
+				AML_ERROR("BinaryOP {2H} missing target", static_cast<uint8_t>(opcode));
+				return ParseResult::Failure;
+			}
+			BAN::RefPtr<AML::Node> target_node;
+			if (context.aml_data[0] == 0x00)
+				context.aml_data = context.aml_data.slice(1);
+			else
+			{
+				auto target_result = AML::parse_object(context);
+				if (!target_result.success())
+					return ParseResult::Failure;
+				target_node = target_result.node();
+				if (!target_node)
+				{
+					AML_ERROR("BinaryOP {2H} target invalid", static_cast<uint8_t>(opcode));
+					return ParseResult::Failure;
+				}
+			}
+
 			uint64_t (*func)(uint64_t, uint64_t) = nullptr;
 			switch (opcode)
 			{
@@ -115,48 +154,61 @@ namespace Kernel::ACPI::AML
 				case AML::Byte::ShiftRightOp:	func = [](uint64_t a, uint64_t b) { return a >> b; }; break;
 				case AML::Byte::SubtractOp:		func = [](uint64_t a, uint64_t b) { return a - b; }; break;
 				case AML::Byte::XorOp:			func = [](uint64_t a, uint64_t b) { return a ^ b; }; break;
-				case AML::Byte::LAndOp:			func = [](uint64_t a, uint64_t b) { return a && b ? Integer::Ones : 0; }; logical = true; break;
-				case AML::Byte::LEqualOp:		func = [](uint64_t a, uint64_t b) { return a == b ? Integer::Ones : 0; }; logical = true; break;
-				case AML::Byte::LGreaterOp:		func = [](uint64_t a, uint64_t b) { return a > b  ? Integer::Ones : 0; }; logical = true; break;
-				case AML::Byte::LLessOp:		func = [](uint64_t a, uint64_t b) { return a < b  ? Integer::Ones : 0; }; logical = true; break;
-				case AML::Byte::LOrOp:			func = [](uint64_t a, uint64_t b) { return a || b ? Integer::Ones : 0; }; logical = true; break;
 				default:
 					ASSERT_NOT_REACHED();
 			}
 
-			uint64_t lhs = static_cast<AML::Integer*>(lhs_node.ptr())->value;
+			uint64_t result = func(lhs_value.value(), rhs_value.value());
-			uint64_t rhs = static_cast<AML::Integer*>(rhs_node.ptr())->value;
-			uint64_t result = func(lhs, rhs);
-
 			auto result_node = MUST(BAN::RefPtr<AML::Integer>::create(result));
 
-			if (!logical)
+			if (target_node && !target_node->store(result_node))
 			{
-				if (context.aml_data.size() < 1)
+				AML_ERROR("BinaryOp {2H} failed to store result", static_cast<uint8_t>(opcode));
-					return ParseResult::Failure;
+				return ParseResult::Failure;
-				if (context.aml_data[0] == 0x00)
-					context.aml_data = context.aml_data.slice(1);
-				else
-				{
-					auto target_result = AML::parse_object(context);
-					if (!target_result.success())
-						return ParseResult::Failure;
-					auto target = target_result.node();
-					if (!target)
-					{
-						AML_ERROR("Target object is null");
-						return ParseResult::Failure;
-					}
-					if (!target->store(result_node))
-					{
-						AML_ERROR("Failed to store result");
-						return ParseResult::Failure;
-					}
-				}
 			}
 
 			return ParseResult(result_node);
 		}
+
+		static ParseResult parse_logical_binary_op(ParseContext& context)
+		{
+			auto opcode = static_cast<AML::Byte>(context.aml_data[0]);
+			context.aml_data = context.aml_data.slice(1);
+
+			auto lhs_result = AML::parse_object(context);
+			if (!lhs_result.success())
+				return ParseResult::Failure;
+			auto lhs_value = lhs_result.node() ? lhs_result.node()->as_integer() : BAN::Optional<uint64_t>();
+			if (!lhs_value.has_value())
+			{
+				AML_TODO("Logical BinaryOP {2H} LHS not integer", static_cast<uint8_t>(opcode));
+				return ParseResult::Failure;
+			}
+
+			auto rhs_result = AML::parse_object(context);
+			if (!rhs_result.success())
+				return ParseResult::Failure;
+			auto rhs_value = rhs_result.node() ? rhs_result.node()->as_integer() : BAN::Optional<uint64_t>();
+			if (!rhs_value.has_value())
+			{
+				AML_TODO("Logical BinaryOP {2H} RHS not integer", static_cast<uint8_t>(opcode));
+				return ParseResult::Failure;
+			}
+
+			BAN::RefPtr<AML::Integer> (*func)(uint64_t, uint64_t) = nullptr;
+			switch (opcode)
+			{
+				case AML::Byte::LAndOp:			func = [](uint64_t a, uint64_t b) { return a && b ? Integer::Constants::Ones : Integer::Constants::Zero; }; break;
+				case AML::Byte::LEqualOp:		func = [](uint64_t a, uint64_t b) { return a == b ? Integer::Constants::Ones : Integer::Constants::Zero; }; break;
+				case AML::Byte::LGreaterOp:		func = [](uint64_t a, uint64_t b) { return a > b  ? Integer::Constants::Ones : Integer::Constants::Zero; }; break;
+				case AML::Byte::LLessOp:		func = [](uint64_t a, uint64_t b) { return a < b  ? Integer::Constants::Ones : Integer::Constants::Zero; }; break;
+				case AML::Byte::LOrOp:			func = [](uint64_t a, uint64_t b) { return a || b ? Integer::Constants::Ones : Integer::Constants::Zero; }; break;
+				default:
+					ASSERT_NOT_REACHED();
+			}
+
+			return ParseResult(func(lhs_value.value(), rhs_value.value()));
+		}
 	};
 
 }

kernel/include/kernel/ACPI/AML/Integer.h

@@ -13,12 +13,21 @@ namespace Kernel::ACPI::AML
 
 	struct Integer : public Node
 	{
-		static constexpr uint64_t Ones = -1;
+		struct Constants
-		const uint64_t value;
+		{
+			// Initialized in Namespace::create_root_namespace
+			static BAN::RefPtr<Integer> Zero;
+			static BAN::RefPtr<Integer> One;
+			static BAN::RefPtr<Integer> Ones;
+		};
 
-		Integer(uint64_t value)
+		const bool constant;
+		uint64_t value;
+
+		Integer(uint64_t value, bool constant = false)
 			: Node(Node::Type::Integer)
 			, value(value)
+			, constant(constant)
 		{}
 
 		BAN::RefPtr<AML::Node> evaluate() override
@@ -26,19 +35,36 @@ namespace Kernel::ACPI::AML
 			return this;
 		}
 
+		bool store(BAN::RefPtr<AML::Node> store_node) override
+		{
+			if (constant)
+			{
+				AML_ERROR("Cannot store to constant integer");
+				return false;
+			}
+			auto store_value = store_node->as_integer();
+			if (!store_value.has_value())
+			{
+				AML_ERROR("Cannot store non-integer to integer");
+				return false;
+			}
+			value = store_value.value();
+			return true;
+		}
+
 		static ParseResult parse(BAN::ConstByteSpan& aml_data)
 		{
 			switch (static_cast<AML::Byte>(aml_data[0]))
 			{
 				case AML::Byte::ZeroOp:
 					aml_data = aml_data.slice(1);
-					return ParseResult(MUST(BAN::RefPtr<Integer>::create(0)));
+					return ParseResult(Constants::Zero);
 				case AML::Byte::OneOp:
 					aml_data = aml_data.slice(1);
-					return ParseResult(MUST(BAN::RefPtr<Integer>::create(1)));
+					return ParseResult(Constants::One);
 				case AML::Byte::OnesOp:
 					aml_data = aml_data.slice(1);
-					return ParseResult(MUST(BAN::RefPtr<Integer>::create(Ones)));
+					return ParseResult(Constants::Ones);
 				case AML::Byte::BytePrefix:
 				{
 					if (aml_data.size() < 2)
@@ -85,10 +111,20 @@ namespace Kernel::ACPI::AML
 		void debug_print(int indent) const override
 		{
 			AML_DEBUG_PRINT_INDENT(indent);
-			if (value == Ones)
+			if (!constant)
-				AML_DEBUG_PRINT("Ones");
-			else
 				AML_DEBUG_PRINT("0x{H}", value);
+			else
+			{
+				AML_DEBUG_PRINT("Const ");
+				if (value == Constants::Zero->value)
+					AML_DEBUG_PRINT("Zero");
+				else if (value == Constants::One->value)
+					AML_DEBUG_PRINT("One");
+				else if (value == Constants::Ones->value)
+					AML_DEBUG_PRINT("Ones");
+				else
+					ASSERT_NOT_REACHED();
+			}
 		}
 	};
 

kernel/include/kernel/ACPI/AML/Method.h

@@ -91,7 +91,12 @@ namespace Kernel::ACPI::AML
 				MUST(context.sync_stack.push_back(sync_level));
 			}
 
-			BAN::Optional<BAN::RefPtr<AML::Node>> return_value;
+#if AML_DEBUG_LEVEL >= 2
+			AML_DEBUG_PRINTLN("Evaluating {}", scope);
+#endif
+
+
+			BAN::Optional<BAN::RefPtr<AML::Node>> return_value = BAN::RefPtr<AML::Node>();
 			while (context.aml_data.size() > 0)
 			{
 				auto parse_result = AML::parse_object(context);
@@ -103,6 +108,7 @@ namespace Kernel::ACPI::AML
 				if (!parse_result.success())
 				{
 					AML_ERROR("Method {} evaluate failed", scope);
+					return_value = {};
 					break;
 				}
 			}

kernel/include/kernel/ACPI/AML/Mutex.h

@@ -119,13 +119,13 @@ namespace Kernel::ACPI::AML
 				while (!mutex->mutex.try_lock())
 				{
 					if (SystemTimer::get().ms_since_boot() >= wake_time)
-						return ParseResult(MUST(BAN::RefPtr<AML::Integer>::create(AML::Integer::Ones)));
+						return ParseResult(Integer::Constants::Ones);
 					SystemTimer::get().sleep(1);
 				}
 			}
 
 			MUST(context.sync_stack.push_back(mutex->sync_level));
-			return ParseResult(MUST(BAN::RefPtr<AML::Integer>::create(0)));
+			return ParseResult(Integer::Constants::Zero);
 		}
 
 		static ParseResult parse_release(ParseContext& context)

kernel/include/kernel/ACPI/AML/NamedObject.h

@@ -24,11 +24,17 @@ namespace Kernel::ACPI::AML
 
 		BAN::RefPtr<AML::Node> evaluate() override
 		{
-			if (!object)
+			ASSERT(object);
-				return {};
 			return object->evaluate();
 		}
 
+		bool store(BAN::RefPtr<AML::Node> node) override
+		{
+			ASSERT(object);
+			object = node;
+			return true;
+		}
+
 		static ParseResult parse(ParseContext& context);
 		virtual void debug_print(int indent) const override;
 	};

kernel/include/kernel/ACPI/AML/Node.h

@@ -11,6 +11,8 @@ namespace Kernel::ACPI::AML
 
 	struct Node : public BAN::RefCounted<Node>
 	{
+		static uint64_t total_node_count;
+
 		enum class Type
 		{
 			BankFieldElement,
@@ -35,8 +37,8 @@ namespace Kernel::ACPI::AML
 		};
 		const Type type;
 
-		Node(Type type) : type(type) {}
+		Node(Type type) : type(type) { total_node_count++; }
-		virtual ~Node() = default;
+		virtual ~Node() { total_node_count--; }
 
 		virtual bool is_scope() const { return false; }
 

kernel/include/kernel/ACPI/AML/Reference.h

@@ -102,7 +102,7 @@ namespace Kernel::ACPI::AML
 			AML_DEBUG_PRINTLN("");
 #endif
 
-			auto return_value = MUST(BAN::RefPtr<Integer>::create(object ? Integer::Ones : 0));
+			auto return_value = object ? Integer::Constants::Ones : Integer::Constants::Zero;
 			return AML::ParseResult(return_value);
 		}
 

kernel/include/kernel/ACPI/AML/Register.h

@@ -39,13 +39,15 @@ namespace Kernel::ACPI::AML
 		void debug_print(int indent) const override
 		{
 			AML_DEBUG_PRINT_INDENT(indent);
-			AML_DEBUG_PRINT("Register\n");
+			if (!value)
-			if (value)
+				AML_DEBUG_PRINT("Register { No value }");
-				value->debug_print(indent + 1);
 			else
 			{
-				AML_DEBUG_PRINT_INDENT(indent + 1);
+				AML_DEBUG_PRINTLN("Register { ");
-				AML_DEBUG_PRINT("No value\n");
+				value->debug_print(indent + 1);
+				AML_DEBUG_PRINTLN("");
+				AML_DEBUG_PRINT_INDENT(indent);
+				AML_DEBUG_PRINT(" }");
 			}
 		}
 	};

kernel/include/kernel/ACPI/AML/Store.h

@@ -17,10 +17,10 @@ namespace Kernel::ACPI::AML
 			auto source_result = AML::parse_object(context);
 			if (!source_result.success())
 				return ParseResult::Failure;
-			auto source = source_result.node();
+			auto source = source_result.node() ? source_result.node()->evaluate() : BAN::RefPtr<AML::Node>();
 			if (!source)
 			{
-				AML_ERROR("Store source is null");
+				AML_ERROR("Store source cannot be evaluated");
 				return ParseResult::Failure;
 			}
 
@@ -34,6 +34,16 @@ namespace Kernel::ACPI::AML
 				return ParseResult::Failure;
 			}
 
+#if AML_DEBUG_LEVEL >= 2
+			AML_DEBUG_PRINTLN("Storing {");
+			source->debug_print(1);
+			AML_DEBUG_PRINTLN("");
+			AML_DEBUG_PRINTLN("} to {");
+			destination->debug_print(1);
+			AML_DEBUG_PRINTLN("");
+			AML_DEBUG_PRINTLN("}");
+#endif
+
 			if (!destination->store(source))
 				return ParseResult::Failure;
 			return ParseResult::Success;

kernel/kernel/ACPI/AML.cpp

@@ -77,7 +77,7 @@ namespace Kernel::ACPI
 		AML_DEBUG_PRINTLN("");
 #endif
 
-		dprintln("Parsed ACPI namespace");
+		dprintln("Parsed ACPI namespace, total of {} nodes created", AML::Node::total_node_count);
 
 		return ns;
 	}

kernel/kernel/ACPI/AML/Namespace.cpp

@@ -1,3 +1,4 @@
+#include <kernel/ACPI/AML/Integer.h>
 #include <kernel/ACPI/AML/Method.h>
 #include <kernel/ACPI/AML/Namespace.h>
 #include <kernel/ACPI/AML/ParseContext.h>
@@ -9,6 +10,10 @@ namespace Kernel::ACPI
 	static BAN::RefPtr<AML::Namespace> s_root_namespace;
 	static BAN::Vector<uint8_t> s_osi_aml_data;
 
+	BAN::RefPtr<AML::Integer> AML::Integer::Constants::Zero;
+	BAN::RefPtr<AML::Integer> AML::Integer::Constants::One;
+	BAN::RefPtr<AML::Integer> AML::Integer::Constants::Ones;
+
 	BAN::RefPtr<AML::Namespace> AML::Namespace::root_namespace()
 	{
 		ASSERT(s_root_namespace);
@@ -198,6 +203,10 @@ namespace Kernel::ACPI
 		ASSERT(!s_root_namespace);
 		s_root_namespace = MUST(BAN::RefPtr<Namespace>::create(NameSeg("\\"sv)));
 
+		Integer::Constants::Zero = MUST(BAN::RefPtr<Integer>::create(0, true));
+		Integer::Constants::One = MUST(BAN::RefPtr<Integer>::create(1, true));
+		Integer::Constants::Ones = MUST(BAN::RefPtr<Integer>::create(0xFFFFFFFFFFFFFFFF, true));
+
 		AML::ParseContext context;
 		context.scope = AML::NameString("\\"sv);
 

kernel/kernel/ACPI/AML/Node.cpp

@@ -27,6 +27,8 @@ namespace Kernel::ACPI
 	AML::ParseResult AML::ParseResult::Failure = AML::ParseResult(AML::ParseResult::Result::Failure);
 	AML::ParseResult AML::ParseResult::Success = AML::ParseResult(AML::ParseResult::Result::Success);
 
+	uint64_t AML::Node::total_node_count = 0;
+
 	BAN::Optional<uint64_t> AML::Node::as_integer()
 	{
 		if (type == Type::Integer)
@@ -214,6 +216,8 @@ namespace Kernel::ACPI
 					AML_ERROR("Failed to evaluate {}", name_string.value());
 					return ParseResult::Failure;
 				}
+				if (!result.value())
+					return ParseResult::Success;
 				return ParseResult(result.value());
 			}
 			return ParseResult(aml_object);

kernel/kernel/ACPI/AML/Scope.cpp

@@ -101,24 +101,19 @@ namespace Kernel::ACPI
 			auto result = method->evaluate({}, sync_stack);
 			if (!result.has_value())
 			{
-				AML_ERROR("Failed to evaluate {}._STA", scope->scope);
+				AML_ERROR("Failed to evaluate {}._STA, ignoring device", scope->scope);
-				return false;
+				return true;
 			}
-			if (!result.value())
+			auto result_value = result.has_value() ? result.value()->as_integer() : BAN::Optional<uint64_t>();
-			{
+			if (!result_value.has_value())
-				AML_ERROR("Failed to evaluate {}._STA, return value is null", scope->scope);
-				return false;
-			}
-			auto result_val = result.value()->as_integer();
-			if (!result_val.has_value())
 			{
 				AML_ERROR("Failed to evaluate {}._STA, return value could not be resolved to integer", scope->scope);
 				AML_ERROR("  Return value: ");
 				result.value()->debug_print(0);
 				return false;
 			}
-			run_ini = (result_val.value() & 0x01);
+			run_ini = (result_value.value() & 0x01);
-			init_children = run_ini || (result_val.value() & 0x02);
+			init_children = run_ini || (result_value.value() & 0x02);
 		}
 
 		if (run_ini)