From 3800d5420e90070a7ddfd2df87dac1419c9d8f03 Mon Sep 17 00:00:00 2001
From: Bananymous <oskari.alaranta@bananymous.com>
Date: Tue, 1 Apr 2025 22:48:22 +0300
Subject: [PATCH] Kernel: Collect created AML nodes in add_{named,alias}...
This makes it harder to miss adding scoped objects. Before I was not
deleting all types of nodes on method return
---
kernel/include/kernel/ACPI/AML/Namespace.h | 4 +-
kernel/kernel/ACPI/AML/Namespace.cpp | 28 ++++++++-----
kernel/kernel/ACPI/AML/Node.cpp | 49 +++++++++++++++-------
kernel/kernel/ACPI/AML/OpRegion.cpp | 12 +++---
4 files changed, 59 insertions(+), 34 deletions(-)
diff --git a/kernel/include/kernel/ACPI/AML/Namespace.h b/kernel/include/kernel/ACPI/AML/Namespace.h
index 139513c6..d7e46f7b 100644
--- a/kernel/include/kernel/ACPI/AML/Namespace.h
+++ b/kernel/include/kernel/ACPI/AML/Namespace.h
@@ -31,8 +31,8 @@ namespace Kernel::ACPI::AML
BAN::ErrorOr<Node> evaluate(const Scope& scope, BAN::StringView);
// returns empty scope if object already exited
- BAN::ErrorOr<Scope> add_named_object(const Scope& scope, const NameString& name_string, Node&& node);
- BAN::ErrorOr<Scope> add_alias(const Scope& scope, const NameString& name_string, Reference* reference);
+ BAN::ErrorOr<Scope> add_named_object(ParseContext& context, const NameString& name_string, Node&& node);
+ BAN::ErrorOr<Scope> add_alias(ParseContext& scope, const NameString& name_string, Reference* reference);
BAN::ErrorOr<void> remove_named_object(const Scope& absolute_path);
diff --git a/kernel/kernel/ACPI/AML/Namespace.cpp b/kernel/kernel/ACPI/AML/Namespace.cpp
index 661f0778..58ca02e3 100644
--- a/kernel/kernel/ACPI/AML/Namespace.cpp
+++ b/kernel/kernel/ACPI/AML/Namespace.cpp
@@ -53,9 +53,11 @@ namespace Kernel::ACPI::AML
const auto add_predefined_root_namespace =
[](const char* name) -> BAN::ErrorOr<void>
{
+ ParseContext dummy_context {};
+
Node predefined {};
predefined.type = Node::Type::PredefinedScope;
- TRY(s_root_namespace.add_named_object({}, TRY(NameString::from_string(name)), BAN::move(predefined)));
+ TRY(s_root_namespace.add_named_object(dummy_context, TRY(NameString::from_string(name)), BAN::move(predefined)));
return {};
};
@@ -66,11 +68,13 @@ namespace Kernel::ACPI::AML
TRY(add_predefined_root_namespace("\\_SI_"));
TRY(add_predefined_root_namespace("\\_TZ_"));
+ ParseContext dummy_context {};
+
{
Node revision;
revision.type = Node::Type::Integer;
revision.as.integer.value = 2;
- TRY(s_root_namespace.add_named_object({}, TRY(NameString::from_string("_REV")), BAN::move(revision)));
+ TRY(s_root_namespace.add_named_object(dummy_context, TRY(NameString::from_string("_REV")), BAN::move(revision)));
}
{
@@ -107,7 +111,7 @@ namespace Kernel::ACPI::AML
return result;
};
- TRY(s_root_namespace.add_named_object({}, osi_string, BAN::move(method)));
+ TRY(s_root_namespace.add_named_object(dummy_context, osi_string, BAN::move(method)));
}
{
@@ -120,7 +124,7 @@ namespace Kernel::ACPI::AML
mutex.as.mutex->sync_level = 0;
mutex.as.mutex->global_lock = true;
- TRY(s_root_namespace.add_named_object({}, gl_string, BAN::move(mutex)));
+ TRY(s_root_namespace.add_named_object(dummy_context, gl_string, BAN::move(mutex)));
}
return {};
@@ -318,11 +322,11 @@ namespace Kernel::ACPI::AML
return resolved_path;
}
- BAN::ErrorOr<Scope> Namespace::add_named_object(const Scope& scope, const NameString& name_string, Node&& node)
+ BAN::ErrorOr<Scope> Namespace::add_named_object(ParseContext& context, const NameString& name_string, Node&& node)
{
- dprintln_if(AML_DUMP_FUNCTION_CALLS, "add_named_object('{}', '{}', {})", scope, name_string, node);
+ dprintln_if(AML_DUMP_FUNCTION_CALLS, "add_named_object('{}', '{}', {})", context.scope, name_string, node);
- auto resolved_path = TRY(resolve_path(scope, name_string));
+ auto resolved_path = TRY(resolve_path(context.scope, name_string));
if (m_named_objects.contains(resolved_path))
return Scope();
@@ -337,14 +341,16 @@ namespace Kernel::ACPI::AML
if (m_has_initialized_namespace && reference->node.type == Node::Type::OpRegion)
(void)opregion_call_reg(resolved_path, reference->node);
+ TRY(context.created_nodes.push_back(TRY(resolved_path.copy())));
+
return resolved_path;
}
- BAN::ErrorOr<Scope> Namespace::add_alias(const Scope& scope, const NameString& name_string, Reference* reference)
+ BAN::ErrorOr<Scope> Namespace::add_alias(ParseContext& context, const NameString& name_string, Reference* reference)
{
- dprintln_if(AML_DUMP_FUNCTION_CALLS, "add_alias('{}', '{}', {})", scope, name_string, reference->node);
+ dprintln_if(AML_DUMP_FUNCTION_CALLS, "add_alias('{}', '{}', {})", context.scope, name_string, reference->node);
- auto resolved_path = TRY(resolve_path(scope, name_string));
+ auto resolved_path = TRY(resolve_path(context.scope, name_string));
if (m_named_objects.contains(resolved_path))
return Scope();
@@ -354,6 +360,8 @@ namespace Kernel::ACPI::AML
TRY(m_named_objects.insert(TRY(resolved_path.copy()), reference));
TRY(m_aliases.insert(TRY(resolved_path.copy())));
+ TRY(context.created_nodes.push_back(TRY(resolved_path.copy())));
+
return resolved_path;
}
diff --git a/kernel/kernel/ACPI/AML/Node.cpp b/kernel/kernel/ACPI/AML/Node.cpp
index 17982061..04094260 100644
--- a/kernel/kernel/ACPI/AML/Node.cpp
+++ b/kernel/kernel/ACPI/AML/Node.cpp
@@ -1201,7 +1201,12 @@ namespace Kernel::ACPI::AML
buffer_field.as.buffer_field.bit_count = bit_count;
buffer_field.as.buffer_field.bit_offset = index_in_bits ? index_node.as.integer.value : index_node.as.integer.value * 8;
- TRY(Namespace::root_namespace().add_named_object(context.scope, field_name_string, BAN::move(buffer_field)));
+ auto absolte_path = TRY(Namespace::root_namespace().add_named_object(context, field_name_string, BAN::move(buffer_field)));
+ if (absolte_path.parts.empty())
+ {
+ dwarnln("Could not add Buffer Field '{}'.'{}' to namespace", context.scope, field_name_string);
+ return {};
+ }
return {};
}
@@ -1695,7 +1700,7 @@ namespace Kernel::ACPI::AML
return {};
}
- TRY(Namespace::root_namespace().add_alias(context.scope, object_name_string, source_ref));
+ TRY(Namespace::root_namespace().add_alias(context, object_name_string, source_ref));
return {};
}
@@ -1711,9 +1716,12 @@ namespace Kernel::ACPI::AML
auto name_string = TRY(parse_name_string(context.aml_data));
auto object = TRY(parse_node(context));
- auto path = TRY(Namespace::root_namespace().add_named_object(context.scope, name_string, BAN::move(object)));
- if (!path.parts.empty())
- TRY(context.created_nodes.push_back(BAN::move(path)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, name_string, BAN::move(object)));
+ if (absolute_path.parts.empty())
+ {
+ dwarnln("Could not add Name '{}'.'{}' to namespace", context.scope, name_string);
+ return {};
+ }
return {};
}
@@ -1892,10 +1900,10 @@ namespace Kernel::ACPI::AML
event_node.type = Node::Type::Event;
event_node.as.event.signal_count = 0;
- auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context.scope, event_name, BAN::move(event_node)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, event_name, BAN::move(event_node)));
if (absolute_path.parts.empty())
{
- dwarnln("Could not add Device '{}'.'{}' to namespace", context.scope, event_name);
+ dwarnln("Could not add Event '{}'.'{}' to namespace", context.scope, event_name);
return {};
}
@@ -2045,7 +2053,7 @@ namespace Kernel::ACPI::AML
Node device_node {};
device_node.type = Node::Type::Device;
- auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context.scope, device_name, BAN::move(device_node)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, device_name, BAN::move(device_node)));
if (absolute_path.parts.empty())
{
dwarnln("Could not add Device '{}'.'{}' to namespace", context.scope, device_name);
@@ -2076,7 +2084,7 @@ namespace Kernel::ACPI::AML
Node processor_node {};
processor_node.type = Node::Type::Processor;
- auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context.scope, processor_name, BAN::move(processor_node)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, processor_name, BAN::move(processor_node)));
if (absolute_path.parts.empty())
{
dwarnln("Could not add Processor '{}'.'{}' to namespace", context.scope, processor_name);
@@ -2107,10 +2115,10 @@ namespace Kernel::ACPI::AML
Node power_resource_node {};
power_resource_node.type = Node::Type::PowerResource;
- auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context.scope, power_resource_name, BAN::move(power_resource_node)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, power_resource_name, BAN::move(power_resource_node)));
if (absolute_path.parts.empty())
{
- dwarnln("Could not add Processor '{}'.'{}' to namespace", context.scope, power_resource_name);
+ dwarnln("Could not add Power Resource '{}'.'{}' to namespace", context.scope, power_resource_name);
return {};
}
@@ -2133,7 +2141,7 @@ namespace Kernel::ACPI::AML
Node thermal_zone_node {};
thermal_zone_node.type = Node::Type::ThermalZone;
- auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context.scope, thermal_zone_name, BAN::move(thermal_zone_node)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, thermal_zone_name, BAN::move(thermal_zone_node)));
if (absolute_path.parts.empty())
{
dwarnln("Could not add Thermal Zone '{}'.'{}' to namespace", context.scope, thermal_zone_name);
@@ -2173,7 +2181,13 @@ namespace Kernel::ACPI::AML
mutex.as.mutex->sync_level = mutex_flags;
mutex.as.mutex->global_lock = false;
- TRY(Namespace::root_namespace().add_named_object(context.scope, mutex_name, BAN::move(mutex)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, mutex_name, BAN::move(mutex)));
+ if (absolute_path.parts.empty())
+ {
+ dwarnln("Could not add Mutex '{}'.'{}' to namespace", context.scope, mutex);
+ return {};
+ }
+
return {};
}
@@ -2321,9 +2335,12 @@ namespace Kernel::ACPI::AML
method_node.as.method.mutex->ref_count = 1;
}
- auto path = TRY(Namespace::root_namespace().add_named_object(context.scope, method_name, BAN::move(method_node)));
- if (!path.parts.empty())
- TRY(context.created_nodes.push_back(BAN::move(path)));
+ auto absolute_path = TRY(Namespace::root_namespace().add_named_object(context, method_name, BAN::move(method_node)));
+ if (absolute_path.parts.empty())
+ {
+ dwarnln("Could not add Method '{}'.'{}' to namespace", context.scope, method_name);
+ return {};
+ }
return {};
}
diff --git a/kernel/kernel/ACPI/AML/OpRegion.cpp b/kernel/kernel/ACPI/AML/OpRegion.cpp
index 7e6db814..c488d4fe 100644
--- a/kernel/kernel/ACPI/AML/OpRegion.cpp
+++ b/kernel/kernel/ACPI/AML/OpRegion.cpp
@@ -92,13 +92,13 @@ namespace Kernel::ACPI::AML
opregion.as.opregion.offset = region_offset.as.integer.value;
opregion.as.opregion.length = region_length.as.integer.value;
- TRY(Namespace::root_namespace().add_named_object(context.scope, region_name, BAN::move(opregion)));
+ TRY(Namespace::root_namespace().add_named_object(context, region_name, BAN::move(opregion)));
return {};
}
template<typename F>
- static BAN::ErrorOr<void> parse_field_list(const Scope& scope, BAN::ConstByteSpan field_list_pkg, const F& create_element, uint8_t field_flags)
+ static BAN::ErrorOr<void> parse_field_list(ParseContext& context, BAN::ConstByteSpan field_list_pkg, const F& create_element, uint8_t field_flags)
{
uint64_t offset = 0;
while (!field_list_pkg.empty())
@@ -144,7 +144,7 @@ namespace Kernel::ACPI::AML
Node field_node = create_element(offset, field_length, field_flags);
- TRY(Namespace::root_namespace().add_named_object(scope, field_name, BAN::move(field_node)));
+ TRY(Namespace::root_namespace().add_named_object(context, field_name, BAN::move(field_node)));
offset += field_length;
@@ -197,7 +197,7 @@ namespace Kernel::ACPI::AML
return field_node;
};
- TRY(parse_field_list(context.scope, field_pkg, create_element, default_flags));
+ TRY(parse_field_list(context, field_pkg, create_element, default_flags));
return {};
}
@@ -260,7 +260,7 @@ namespace Kernel::ACPI::AML
return field_node;
};
- TRY(parse_field_list(context.scope, field_pkg, create_element, default_flags));
+ TRY(parse_field_list(context, field_pkg, create_element, default_flags));
return {};
}
@@ -331,7 +331,7 @@ namespace Kernel::ACPI::AML
return field_node;
};
- TRY(parse_field_list(context.scope, field_pkg, create_element, default_flags));
+ TRY(parse_field_list(context, field_pkg, create_element, default_flags));
return {};
}