From 74940ed33c57b29f80bb04751dc1e03f3f8b304b Mon Sep 17 00:00:00 2001 From: Bananymous Date: Fri, 12 Apr 2024 16:03:14 +0300 Subject: [PATCH] Kernel: Cleanup AML code and fix bugs I can enter ACPI mode on my own laptop! --- kernel/include/kernel/ACPI/AML/Expression.h | 216 ++++++++++++------- kernel/include/kernel/ACPI/AML/Integer.h | 54 ++++- kernel/include/kernel/ACPI/AML/Method.h | 8 +- kernel/include/kernel/ACPI/AML/Mutex.h | 4 +- kernel/include/kernel/ACPI/AML/NamedObject.h | 10 +- kernel/include/kernel/ACPI/AML/Node.h | 6 +- kernel/include/kernel/ACPI/AML/Reference.h | 2 +- kernel/include/kernel/ACPI/AML/Register.h | 12 +- kernel/include/kernel/ACPI/AML/Store.h | 14 +- kernel/kernel/ACPI/AML.cpp | 2 +- kernel/kernel/ACPI/AML/Namespace.cpp | 9 + kernel/kernel/ACPI/AML/Node.cpp | 4 + kernel/kernel/ACPI/AML/Scope.cpp | 17 +- 13 files changed, 240 insertions(+), 118 deletions(-) diff --git a/kernel/include/kernel/ACPI/AML/Expression.h b/kernel/include/kernel/ACPI/AML/Expression.h index eaae2aad51..5a6275794e 100644 --- a/kernel/include/kernel/ACPI/AML/Expression.h +++ b/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(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(); + if (!source_node || source_node->type != AML::Node::Type::Integer) + { + AML_ERROR("UnaryOp source not integer"); + return ParseResult::Failure; + } + + auto source_integer = static_cast(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("Expression {2H}", context.aml_data[0]); + AML_TODO("NotOp", 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(); + 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: - { - auto opcode = static_cast(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 + return parse_logical_binary_op(context); 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 lhs_node, BAN::RefPtr rhs_node) + static ParseResult parse_binary_op(ParseContext& context) { - if (lhs_node->type != AML::Node::Type::Integer) - { - AML_TODO("LHS object is not an integer, type {}", static_cast(lhs_node->type)); + auto opcode = static_cast(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; - } - if (rhs_node->type != AML::Node::Type::Integer) + auto lhs_value = lhs_result.node() ? lhs_result.node()->as_integer() : BAN::Optional(); + if (!lhs_value.has_value()) { - AML_TODO("RHS object is not an integer, type {}", static_cast(rhs_node->type)); + AML_ERROR("BinaryOP {2H} LHS not an integer", static_cast(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(); + if (!rhs_value.has_value()) + { + AML_ERROR("BinaryOP {2H} RHS not an integer", static_cast(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(opcode)); + return ParseResult::Failure; + } + BAN::RefPtr 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(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(lhs_node.ptr())->value; - uint64_t rhs = static_cast(rhs_node.ptr())->value; - uint64_t result = func(lhs, rhs); - + uint64_t result = func(lhs_value.value(), rhs_value.value()); auto result_node = MUST(BAN::RefPtr::create(result)); - if (!logical) + if (target_node && !target_node->store(result_node)) { - if (context.aml_data.size() < 1) - 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; - } - } + AML_ERROR("BinaryOp {2H} failed to store result", static_cast(opcode)); + return ParseResult::Failure; } return ParseResult(result_node); } + + static ParseResult parse_logical_binary_op(ParseContext& context) + { + auto opcode = static_cast(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(); + if (!lhs_value.has_value()) + { + AML_TODO("Logical BinaryOP {2H} LHS not integer", static_cast(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(); + if (!rhs_value.has_value()) + { + AML_TODO("Logical BinaryOP {2H} RHS not integer", static_cast(opcode)); + return ParseResult::Failure; + } + + BAN::RefPtr (*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())); + } }; } diff --git a/kernel/include/kernel/ACPI/AML/Integer.h b/kernel/include/kernel/ACPI/AML/Integer.h index 9ec5f6dbba..2fe80d4ef5 100644 --- a/kernel/include/kernel/ACPI/AML/Integer.h +++ b/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; - const uint64_t value; + struct Constants + { + // Initialized in Namespace::create_root_namespace + static BAN::RefPtr Zero; + static BAN::RefPtr One; + static BAN::RefPtr 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 evaluate() override @@ -26,19 +35,36 @@ namespace Kernel::ACPI::AML return this; } + bool store(BAN::RefPtr 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_data[0])) { case AML::Byte::ZeroOp: aml_data = aml_data.slice(1); - return ParseResult(MUST(BAN::RefPtr::create(0))); + return ParseResult(Constants::Zero); case AML::Byte::OneOp: aml_data = aml_data.slice(1); - return ParseResult(MUST(BAN::RefPtr::create(1))); + return ParseResult(Constants::One); case AML::Byte::OnesOp: aml_data = aml_data.slice(1); - return ParseResult(MUST(BAN::RefPtr::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) - AML_DEBUG_PRINT("Ones"); - else + if (!constant) 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(); + } } }; diff --git a/kernel/include/kernel/ACPI/AML/Method.h b/kernel/include/kernel/ACPI/AML/Method.h index 1aeadc0bd7..edc058d17f 100644 --- a/kernel/include/kernel/ACPI/AML/Method.h +++ b/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> return_value; +#if AML_DEBUG_LEVEL >= 2 + AML_DEBUG_PRINTLN("Evaluating {}", scope); +#endif + + + BAN::Optional> return_value = BAN::RefPtr(); 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; } } diff --git a/kernel/include/kernel/ACPI/AML/Mutex.h b/kernel/include/kernel/ACPI/AML/Mutex.h index 100cd2408f..f29dfa65b6 100644 --- a/kernel/include/kernel/ACPI/AML/Mutex.h +++ b/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::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::create(0))); + return ParseResult(Integer::Constants::Zero); } static ParseResult parse_release(ParseContext& context) diff --git a/kernel/include/kernel/ACPI/AML/NamedObject.h b/kernel/include/kernel/ACPI/AML/NamedObject.h index a5de7e2f9f..cc943f7201 100644 --- a/kernel/include/kernel/ACPI/AML/NamedObject.h +++ b/kernel/include/kernel/ACPI/AML/NamedObject.h @@ -24,11 +24,17 @@ namespace Kernel::ACPI::AML BAN::RefPtr evaluate() override { - if (!object) - return {}; + ASSERT(object); return object->evaluate(); } + bool store(BAN::RefPtr node) override + { + ASSERT(object); + object = node; + return true; + } + static ParseResult parse(ParseContext& context); virtual void debug_print(int indent) const override; }; diff --git a/kernel/include/kernel/ACPI/AML/Node.h b/kernel/include/kernel/ACPI/AML/Node.h index 1a6bfc5733..6377f89d97 100644 --- a/kernel/include/kernel/ACPI/AML/Node.h +++ b/kernel/include/kernel/ACPI/AML/Node.h @@ -11,6 +11,8 @@ namespace Kernel::ACPI::AML struct Node : public BAN::RefCounted { + 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) {} - virtual ~Node() = default; + Node(Type type) : type(type) { total_node_count++; } + virtual ~Node() { total_node_count--; } virtual bool is_scope() const { return false; } diff --git a/kernel/include/kernel/ACPI/AML/Reference.h b/kernel/include/kernel/ACPI/AML/Reference.h index b068543cdc..50112b156c 100644 --- a/kernel/include/kernel/ACPI/AML/Reference.h +++ b/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::create(object ? Integer::Ones : 0)); + auto return_value = object ? Integer::Constants::Ones : Integer::Constants::Zero; return AML::ParseResult(return_value); } diff --git a/kernel/include/kernel/ACPI/AML/Register.h b/kernel/include/kernel/ACPI/AML/Register.h index 9c89afe2e0..d2fa396cf7 100644 --- a/kernel/include/kernel/ACPI/AML/Register.h +++ b/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) - value->debug_print(indent + 1); + if (!value) + AML_DEBUG_PRINT("Register { No value }"); else { - AML_DEBUG_PRINT_INDENT(indent + 1); - AML_DEBUG_PRINT("No value\n"); + AML_DEBUG_PRINTLN("Register { "); + value->debug_print(indent + 1); + AML_DEBUG_PRINTLN(""); + AML_DEBUG_PRINT_INDENT(indent); + AML_DEBUG_PRINT(" }"); } } }; diff --git a/kernel/include/kernel/ACPI/AML/Store.h b/kernel/include/kernel/ACPI/AML/Store.h index 886940d9e7..59a6d45b73 100644 --- a/kernel/include/kernel/ACPI/AML/Store.h +++ b/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(); 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; diff --git a/kernel/kernel/ACPI/AML.cpp b/kernel/kernel/ACPI/AML.cpp index 6637c35981..351203e367 100644 --- a/kernel/kernel/ACPI/AML.cpp +++ b/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; } diff --git a/kernel/kernel/ACPI/AML/Namespace.cpp b/kernel/kernel/ACPI/AML/Namespace.cpp index 3d702e77eb..dcec78a166 100644 --- a/kernel/kernel/ACPI/AML/Namespace.cpp +++ b/kernel/kernel/ACPI/AML/Namespace.cpp @@ -1,3 +1,4 @@ +#include #include #include #include @@ -9,6 +10,10 @@ namespace Kernel::ACPI static BAN::RefPtr s_root_namespace; static BAN::Vector s_osi_aml_data; + BAN::RefPtr AML::Integer::Constants::Zero; + BAN::RefPtr AML::Integer::Constants::One; + BAN::RefPtr AML::Integer::Constants::Ones; + BAN::RefPtr 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::create(NameSeg("\\"sv))); + Integer::Constants::Zero = MUST(BAN::RefPtr::create(0, true)); + Integer::Constants::One = MUST(BAN::RefPtr::create(1, true)); + Integer::Constants::Ones = MUST(BAN::RefPtr::create(0xFFFFFFFFFFFFFFFF, true)); + AML::ParseContext context; context.scope = AML::NameString("\\"sv); diff --git a/kernel/kernel/ACPI/AML/Node.cpp b/kernel/kernel/ACPI/AML/Node.cpp index b5677b9242..632283ddd4 100644 --- a/kernel/kernel/ACPI/AML/Node.cpp +++ b/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 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); diff --git a/kernel/kernel/ACPI/AML/Scope.cpp b/kernel/kernel/ACPI/AML/Scope.cpp index a4a1328f81..903e85ea01 100644 --- a/kernel/kernel/ACPI/AML/Scope.cpp +++ b/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); - return false; + AML_ERROR("Failed to evaluate {}._STA, ignoring device", scope->scope); + return true; } - if (!result.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()) + auto result_value = result.has_value() ? result.value()->as_integer() : BAN::Optional(); + if (!result_value.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); - init_children = run_ini || (result_val.value() & 0x02); + run_ini = (result_value.value() & 0x01); + init_children = run_ini || (result_value.value() & 0x02); } if (run_ini)