Kernel: AML tries to initialize processors when entering ACPI mode

I had forgotten that Processors used to be a different definition
in AML.

I also implemented reads/writes for FieldElement/IndexFieldElement
that fit in 64 bits. Reads and writes to buffer are still a TODO.

kernel/CMakeLists.txt

@@ -12,6 +12,7 @@ set(KERNEL_SOURCES
 	font/prefs.psf.o
 	kernel/ACPI/ACPI.cpp
 	kernel/ACPI/AML.cpp
+	kernel/ACPI/AML/Device.cpp
 	kernel/ACPI/AML/Field.cpp
 	kernel/ACPI/AML/NamedObject.cpp
 	kernel/ACPI/AML/Namespace.cpp

kernel/include/kernel/ACPI/AML/Device.h

@@ -36,71 +36,6 @@ namespace Kernel::ACPI::AML
 			return device->enter_context_and_parse_term_list(context, name_string.value(), device_pkg.value());
 		}
 
-		void initialize()
-		{
-			bool run_ini = true;
-			bool init_children = true;
-
-			auto _sta = Namespace::root_namespace()->find_object(scope, NameString("_STA"sv));
-			if (_sta && _sta->type == Node::Type::Method)
-			{
-				auto* method = static_cast<Method*>(_sta.ptr());
-				if (method->arg_count != 0)
-				{
-					AML_ERROR("Method {}._STA has {} arguments, expected 0", scope, method->arg_count);
-					return;
-				}
-				auto result = method->evaluate({});
-				if (!result.has_value())
-				{
-					AML_ERROR("Failed to evaluate {}._STA", scope);
-					return;
-				}
-				if (!result.value())
-				{
-					AML_ERROR("Failed to evaluate {}._STA, return value is null", scope);
-					return;
-				}
-				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);
-					AML_ERROR("  Return value: ");
-					result.value()->debug_print(0);
-					return;
-				}
-				run_ini = (result_val.value() & 0x01);
-				init_children = run_ini || (result_val.value() & 0x02);
-			}
-
-			if (run_ini)
-			{
-				auto _ini = Namespace::root_namespace()->find_object(scope, NameString("_INI"sv));
-				if (_ini && _ini->type == Node::Type::Method)
-				{
-					auto* method = static_cast<Method*>(_ini.ptr());
-					if (method->arg_count != 0)
-					{
-						AML_ERROR("Method {}._INI has {} arguments, expected 0", scope, method->arg_count);
-						return;
-					}
-					method->evaluate({});
-				}
-			}
-
-			if (init_children)
-			{
-				for (auto& [_, child] : objects)
-				{
-					if (child->type == Node::Type::Device)
-					{
-						auto* device = static_cast<Device*>(child.ptr());
-						device->initialize();
-					}
-				}
-			}
-		}
-
 		virtual void debug_print(int indent) const override
 		{
 			AML_DEBUG_PRINT_INDENT(indent);
@@ -117,4 +52,6 @@ namespace Kernel::ACPI::AML
 		}
 	};
 
+	bool initialize_device(BAN::RefPtr<NamedObject> device);
+
 }

kernel/include/kernel/ACPI/AML/Field.h

@@ -40,13 +40,13 @@ namespace Kernel::ACPI::AML
 	struct FieldElement : public NamedObject
 	{
 		uint64_t bit_offset;
-		uint32_t bit_count;
+		uint64_t bit_count;
 
 		FieldRules access_rules;
 
-		BAN::RefPtr<OpRegion> op_region = nullptr;
+		BAN::RefPtr<OpRegion> op_region;
 
-		FieldElement(NameSeg name, uint64_t bit_offset, uint32_t bit_count, FieldRules access_rules)
+		FieldElement(NameSeg name, uint64_t bit_offset, uint64_t bit_count, FieldRules access_rules)
 			: NamedObject(Node::Type::FieldElement, name)
 			, bit_offset(bit_offset)
 			, bit_count(bit_count)
@@ -59,16 +59,10 @@ namespace Kernel::ACPI::AML
 		void debug_print(int indent) const override;
 
 	private:
-		struct AccessType
+		BAN::Optional<uint64_t> evaluate_internal();
-		{
+		bool store_internal(uint64_t value);
-			uint64_t offset;
+
-			uint64_t mask;
+		friend struct IndexFieldElement;
-			uint32_t access_size;
-			uint32_t shift;
-		};
-		BAN::Optional<AccessType> determine_access_type() const;
-		BAN::Optional<uint64_t> read_field(uint64_t offset, uint32_t access_size) const;
-		bool write_field(uint64_t offset, uint32_t access_size, uint64_t value) const;
 	};
 
 	struct Field
@@ -79,20 +73,23 @@ namespace Kernel::ACPI::AML
 	struct IndexFieldElement : public NamedObject
 	{
 		uint64_t bit_offset;
-		uint32_t bit_count;
+		uint64_t bit_count;
 
 		FieldRules access_rules;
 
-		BAN::RefPtr<FieldElement> index_element = nullptr;
+		BAN::RefPtr<FieldElement> index_element;
-		BAN::RefPtr<FieldElement> data_element = nullptr;
+		BAN::RefPtr<FieldElement> data_element;
 
-		IndexFieldElement(NameSeg name, uint64_t bit_offset, uint32_t bit_count, FieldRules access_rules)
+		IndexFieldElement(NameSeg name, uint64_t bit_offset, uint64_t bit_count, FieldRules access_rules)
 			: NamedObject(Node::Type::IndexFieldElement, name)
 			, bit_offset(bit_offset)
 			, bit_count(bit_count)
 			, access_rules(access_rules)
 		{}
 
+		BAN::RefPtr<Node> evaluate() override;
+		bool store(BAN::RefPtr<Node> source) override;
+
 		void debug_print(int indent) const override;
 	};
 

kernel/kernel/ACPI/ACPI.cpp

@@ -406,13 +406,8 @@ namespace Kernel::ACPI
 		{
 			auto* scope = static_cast<AML::Scope*>(_sb.ptr());
 			for (auto& [name, object] : scope->objects)
-			{
+				if (object->type == AML::Node::Type::Device || object->type == AML::Node::Type::Processor)
-				if (object->type == AML::Node::Type::Device)
+					AML::initialize_device(object);
-				{
-					auto* device = static_cast<AML::Device*>(object.ptr());
-					device->initialize();
-				}
-			}
 		}
 
 		// Evaluate \\_PIC (mode)

kernel/kernel/ACPI/AML/Device.cpp

@@ -0,0 +1,71 @@
+#include <kernel/ACPI/AML/Device.h>
+
+namespace Kernel::ACPI
+{
+
+	bool AML::initialize_device(BAN::RefPtr<AML::NamedObject> device)
+	{
+		bool run_ini = true;
+		bool init_children = true;
+
+		ASSERT(device->type == Node::Type::Device || device->type == Node::Type::Processor);
+		ASSERT(device->is_scope());
+		auto* scope = static_cast<Scope*>(device.ptr());
+
+		auto it = scope->objects.find(NameSeg("_STA"sv));
+		if (it != scope->objects.end() && it->value->type == Node::Type::Method)
+		{
+			auto* method = static_cast<Method*>(it->value.ptr());
+			if (method->arg_count != 0)
+			{
+				AML_ERROR("Method {}._STA has {} arguments, expected 0", scope, method->arg_count);
+				return false;
+			}
+			auto result = method->evaluate({});
+			if (!result.has_value())
+			{
+				AML_ERROR("Failed to evaluate {}._STA", scope);
+				return false;
+			}
+			if (!result.value())
+			{
+				AML_ERROR("Failed to evaluate {}._STA, return value is null", 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);
+				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);
+		}
+
+		if (run_ini)
+		{
+			auto it = scope->objects.find(NameSeg("_STA"sv));
+			if (it != scope->objects.end() && it->value->type == Node::Type::Method)
+			{
+				auto* method = static_cast<Method*>(it->value.ptr());
+				if (method->arg_count != 0)
+				{
+					AML_ERROR("Method {}._INI has {} arguments, expected 0", scope, method->arg_count);
+					return false;
+				}
+				method->evaluate({});
+			}
+		}
+
+		bool success = true;
+		if (init_children)
+			for (auto& [_, child] : scope->objects)
+				if (child->type == Node::Type::Device || child->type == Node::Type::Processor)
+					if (!initialize_device(child))
+						success = false;
+		return success;
+	}
+
+}

kernel/kernel/ACPI/AML/Field.cpp

@@ -7,6 +7,18 @@
 namespace Kernel::ACPI
 {
 
+	template<typename Func>
+	concept ReadFunc = requires(Func func, uint64_t offset)
+	{
+		requires BAN::is_same_v<decltype(func(offset)), BAN::Optional<uint64_t>>;
+	};
+
+	template<typename Func>
+	concept WriteFunc = requires(Func func, uint64_t offset, uint64_t value)
+	{
+		requires BAN::is_same_v<decltype(func(offset, value)), bool>;
+	};
+
 	template<typename Element>
 	struct ParseFieldElementContext
 	{
@@ -19,6 +31,8 @@ namespace Kernel::ACPI
 	template<typename Element>
 	static bool parse_field_element(ParseFieldElementContext<Element>& context)
 	{
+		// FIXME: Validate elements
+
 		ASSERT(context.field_pkg.size() >= 1);
 		switch (context.field_pkg[0])
 		{
@@ -81,6 +95,248 @@ namespace Kernel::ACPI
 		}
 	}
 
+	static BAN::Optional<uint32_t> determine_access_size(const AML::FieldRules::AccessType& access_type)
+	{
+		switch (access_type)
+		{
+			case AML::FieldRules::AccessType::Any:
+			case AML::FieldRules::AccessType::Byte:
+				return 1;
+			case AML::FieldRules::AccessType::Word:
+				return 2;
+			case AML::FieldRules::AccessType::DWord:
+				return 4;
+			case AML::FieldRules::AccessType::QWord:
+				return 8;
+			case AML::FieldRules::AccessType::Buffer:
+				AML_TODO("FieldElement with access type Buffer");
+				return {};
+		}
+		return {};
+	}
+
+	static BAN::Optional<uint64_t> perform_read(AML::OpRegion::RegionSpace region_space, uint64_t access_offset, uint32_t access_size)
+	{
+		switch (region_space)
+		{
+			case AML::OpRegion::RegionSpace::SystemMemory:
+			{
+				uint64_t result = 0;
+				size_t index_in_page = (access_offset % PAGE_SIZE) / access_size;
+				PageTable::with_fast_page(access_offset & PAGE_ADDR_MASK, [&] {
+					switch (access_size)
+					{
+						case 1: result = PageTable::fast_page_as_sized<uint8_t> (index_in_page); break;
+						case 2: result = PageTable::fast_page_as_sized<uint16_t>(index_in_page); break;
+						case 4: result = PageTable::fast_page_as_sized<uint32_t>(index_in_page); break;
+						case 8: result = PageTable::fast_page_as_sized<uint64_t>(index_in_page); break;
+					}
+				});
+				return result;
+			}
+			case AML::OpRegion::RegionSpace::SystemIO:
+			{
+				uint64_t result = 0;
+				switch (access_size)
+				{
+					case 1: result = IO::inb(access_offset); break;
+					case 2: result = IO::inw(access_offset); break;
+					case 4: result = IO::inl(access_offset); break;
+					default:
+						AML_ERROR("FieldElement read_field (SystemIO) with access size {}", access_size);
+						return {};
+				}
+				return result;
+			}
+			case AML::OpRegion::RegionSpace::PCIConfig:
+			{
+				// https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#address-space-format
+				// PCI configuration space is confined to segment 0, bus 0
+
+				uint16_t device		= (access_offset >> 32) & 0xFFFF;
+				uint16_t function	= (access_offset >> 16) & 0xFFFF;
+				uint16_t offset		= access_offset & 0xFFFF;
+
+				uint64_t result = 0;
+				switch (access_size)
+				{
+					case 1: result = PCI::PCIManager::read_config_byte(0, device, function, offset); break;
+					case 2: result = PCI::PCIManager::read_config_word(0, device, function, offset); break;
+					case 4: result = PCI::PCIManager::read_config_dword(0, device, function, offset); break;
+					default:
+						AML_ERROR("FieldElement read_field (PCIConfig) with access size {}", access_size);
+						return {};
+				}
+				return result;
+			}
+			default:
+				AML_TODO("FieldElement read_field with region space {}", static_cast<uint8_t>(region_space));
+				return {};
+		}
+	}
+
+	static bool perform_write(AML::OpRegion::RegionSpace region_space, uint64_t access_offset, uint32_t access_size, uint64_t value)
+	{
+		switch (region_space)
+		{
+			case AML::OpRegion::RegionSpace::SystemMemory:
+			{
+				size_t index_in_page = (access_offset % PAGE_SIZE) / access_size;
+				PageTable::with_fast_page(access_offset & PAGE_ADDR_MASK, [&] {
+					switch (access_size)
+					{
+						case 1: PageTable::fast_page_as_sized<uint8_t> (index_in_page) = value; break;
+						case 2: PageTable::fast_page_as_sized<uint16_t>(index_in_page) = value; break;
+						case 4: PageTable::fast_page_as_sized<uint32_t>(index_in_page) = value; break;
+						case 8: PageTable::fast_page_as_sized<uint64_t>(index_in_page) = value; break;
+					}
+				});
+				return true;
+			}
+			case AML::OpRegion::RegionSpace::SystemIO:
+			{
+				switch (access_size)
+				{
+					case 1: IO::outb(access_offset, value); break;
+					case 2: IO::outw(access_offset, value); break;
+					case 4: IO::outl(access_offset, value); break;
+					default:
+						AML_ERROR("FieldElement write_field (SystemIO) with access size {}", access_size);
+						return false;
+				}
+				return true;
+			}
+			case AML::OpRegion::RegionSpace::PCIConfig:
+			{
+				// https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#address-space-format
+				// PCI configuration space is confined to segment 0, bus 0
+
+				uint16_t device		= (access_offset >> 32) & 0xFFFF;
+				uint16_t function	= (access_offset >> 16) & 0xFFFF;
+				uint16_t offset		= access_offset & 0xFFFF;
+
+				switch (access_size)
+				{
+					case 1: PCI::PCIManager::write_config_byte(0, device, function, offset, value); break;
+					case 2: PCI::PCIManager::write_config_word(0, device, function, offset, value); break;
+					case 4: PCI::PCIManager::write_config_dword(0, device, function, offset, value); break;
+					default:
+						AML_ERROR("FieldElement write_field (PCIConfig) with access size {}", access_size);
+						return false;
+				}
+				return true;
+			}
+			default:
+				AML_TODO("FieldElement write_field with region space {}", static_cast<uint8_t>(region_space));
+				return false;
+		}
+	}
+
+	template<ReadFunc ReadFunc>
+	static BAN::Optional<uint64_t> perform_read_general(
+		uint64_t base_byte_offset,
+		uint64_t bit_count,
+		uint64_t bit_offset,
+		uint32_t access_size,
+		ReadFunc& read_func
+	)
+	{
+		if (bit_count > 64)
+		{
+			AML_TODO("Field read with bit_count > 64");
+			return {};
+		}
+
+		uint32_t access_bytes = access_size;
+		uint32_t access_bits = access_bytes * 8;
+
+		uint64_t result = 0;
+		uint32_t bits_read = 0;
+		while (bits_read < bit_count)
+		{
+			uint64_t byte_offset = base_byte_offset + ((bit_offset + bits_read) / 8);
+			if (auto rem = byte_offset % access_bytes)
+				byte_offset -= rem;
+
+			auto partial = read_func(byte_offset);
+			if (!partial.has_value())
+				return {};
+
+			uint32_t shift = (bit_offset + bits_read) % access_bits;
+			uint32_t valid_bits = BAN::Math::min<uint32_t>(access_bits - shift, bit_count - bits_read);
+			uint64_t mask = ((uint64_t)1 << valid_bits) - 1;
+
+			result |= ((partial.value() >> shift) & mask) << bits_read;
+			bits_read += valid_bits;
+		}
+
+		return result;
+	}
+
+	template<ReadFunc ReadFunc, WriteFunc WriteFunc>
+	static bool perform_write_general(
+		uint64_t base_byte_offset,
+		uint64_t bit_count,
+		uint64_t bit_offset,
+		uint32_t access_size,
+		uint64_t value,
+		AML::FieldRules::UpdateRule update_rule,
+		ReadFunc& read_func,
+		WriteFunc& write_func
+	)
+	{
+		if (bit_count > 64)
+		{
+			AML_TODO("Field write with bit_count > 64");
+			return false;
+		}
+
+		uint32_t access_bytes = access_size;
+		uint32_t access_bits = access_bytes * 8;
+
+		uint32_t bits_written = 0;
+		while (bits_written < bit_count)
+		{
+			uint64_t byte_offset = base_byte_offset + ((bit_offset + bits_written) / 8);
+			if (auto rem = byte_offset % access_bytes)
+				byte_offset -= rem;
+
+			uint32_t shift = (bit_offset + bits_written) % access_bits;
+			uint32_t valid_bits = BAN::Math::min<uint32_t>(access_bits - shift, bit_count - bits_written);
+			uint64_t mask = ((uint64_t)1 << valid_bits) - 1;
+
+			uint64_t to_write = 0;
+			if (valid_bits != access_bits)
+			{
+				switch (update_rule)
+				{
+					case AML::FieldRules::UpdateRule::Preserve:
+					{
+						auto read_result = read_func(byte_offset);
+						if (!read_result.has_value())
+							return false;
+						to_write = read_result.value() & ~(mask << shift);
+						break;
+					}
+					case AML::FieldRules::UpdateRule::WriteAsOnes:
+						to_write = ~(mask << shift);
+						break;
+					case AML::FieldRules::UpdateRule::WriteAsZeros:
+						to_write = 0;
+						break;
+				}
+			}
+			to_write |= ((value >> bits_written) & mask) << shift;
+
+			if (!write_func(byte_offset, to_write))
+				return false;
+
+			bits_written += valid_bits;
+		}
+
+		return true;
+	}
+
 	AML::ParseResult AML::Field::parse(ParseContext& context)
 	{
 		ASSERT(context.aml_data.size() >= 2);
@@ -137,185 +393,37 @@ namespace Kernel::ACPI
 		return ParseResult::Success;
 	}
 
-
+	BAN::Optional<uint64_t> AML::FieldElement::evaluate_internal()
-	BAN::Optional<AML::FieldElement::AccessType> AML::FieldElement::determine_access_type() const
 	{
-		uint32_t access_size = 0;
+		auto access_size = determine_access_size(access_rules.access_type);
-		switch (access_rules.access_type)
+		if (!access_size.has_value())
-		{
-			case FieldRules::AccessType::Any:
-			case FieldRules::AccessType::Byte:
-				access_size = 1;
-				break;
-			case FieldRules::AccessType::Word:
-				access_size = 2;
-				break;
-			case FieldRules::AccessType::DWord:
-				access_size = 4;
-				break;
-			case FieldRules::AccessType::QWord:
-				access_size = 8;
-				break;
-			case FieldRules::AccessType::Buffer:
-				AML_TODO("FieldElement with access type Buffer");
 			return {};
-		}
+		auto read_func = [&](uint64_t byte_offset) -> BAN::Optional<uint64_t> {
-
+			return perform_read(op_region->region_space, byte_offset, access_size.value());
-		uint64_t byte_offset = op_region->region_offset + (bit_offset / 8);
-		if (auto rem = byte_offset % access_size)
-			byte_offset -= rem;
-
-		if ((bit_offset % access_size) + bit_count > access_size * 8)
-		{
-			AML_ERROR("FieldElement over multiple access sizes");
-			return {};
-		}
-
-		if (byte_offset + access_size > op_region->region_offset + op_region->region_length)
-		{
-			AML_ERROR("FieldElement out of bounds");
-			return {};
-		}
-
-		uint32_t shift = bit_offset % access_size;
-		uint64_t mask = ((uint64_t)1 << bit_count) - 1;
-
-		return AccessType {
-			.offset = byte_offset,
-			.mask = mask,
-			.access_size = access_size,
-			.shift = shift
 		};
+		return perform_read_general(op_region->region_offset, bit_count, bit_offset, access_size.value(), read_func);
 	}
 
-	BAN::Optional<uint64_t> AML::FieldElement::read_field(uint64_t access_offset, uint32_t access_size) const
+	bool AML::FieldElement::store_internal(uint64_t value)
 	{
-		switch (op_region->region_space)
+		auto access_size = determine_access_size(access_rules.access_type);
-		{
+		if (!access_size.has_value())
-			case OpRegion::RegionSpace::SystemMemory:
-			{
-				uint64_t result = 0;
-				PageTable::with_fast_page(access_offset & PAGE_ADDR_MASK, [&] {
-					switch (access_size)
-					{
-						case 1: result = PageTable::fast_page_as_sized<uint8_t> ((access_offset % PAGE_SIZE) / access_size); break;
-						case 2: result = PageTable::fast_page_as_sized<uint16_t>((access_offset % PAGE_SIZE) / access_size); break;
-						case 4: result = PageTable::fast_page_as_sized<uint32_t>((access_offset % PAGE_SIZE) / access_size); break;
-						case 8: result = PageTable::fast_page_as_sized<uint64_t>((access_offset % PAGE_SIZE) / access_size); break;
-					}
-				});
-				return result;
-			}
-			case OpRegion::RegionSpace::SystemIO:
-			{
-				uint64_t result = 0;
-				switch (access_size)
-				{
-					case 1: result = IO::inb(access_offset); break;
-					case 2: result = IO::inw(access_offset); break;
-					case 4: result = IO::inl(access_offset); break;
-					default:
-						AML_ERROR("FieldElement read_field (SystemIO) with access size {}", access_size);
-						return {};
-				}
-				return result;
-			}
-			case OpRegion::RegionSpace::PCIConfig:
-			{
-				// https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#address-space-format
-				// PCI configuration space is confined to segment 0, bus 0
-
-				uint16_t device		= (access_offset >> 32) & 0xFFFF;
-				uint16_t function	= (access_offset >> 16) & 0xFFFF;
-				uint16_t offset		= access_offset & 0xFFFF;
-
-				uint64_t result = 0;
-				switch (access_size)
-				{
-					case 1: result = PCI::PCIManager::read_config_byte(0, device, function, offset); break;
-					case 2: result = PCI::PCIManager::read_config_word(0, device, function, offset); break;
-					case 4: result = PCI::PCIManager::read_config_dword(0, device, function, offset); break;
-					default:
-						AML_ERROR("FieldElement read_field (PCIConfig) with access size {}", access_size);
-						return {};
-				}
-				return result;
-			}
-			default:
-				AML_TODO("FieldElement read_field with region space {}", static_cast<uint8_t>(op_region->region_space));
-				return {};
-		}
-	}
-
-	bool AML::FieldElement::write_field(uint64_t access_offset, uint32_t access_size, uint64_t value) const
-	{
-		switch (op_region->region_space)
-		{
-			case OpRegion::RegionSpace::SystemMemory:
-			{
-				PageTable::with_fast_page(access_offset & PAGE_ADDR_MASK, [&] {
-					switch (access_size)
-					{
-						case 1: PageTable::fast_page_as_sized<uint8_t> ((access_offset % PAGE_SIZE) / access_size) = value; break;
-						case 2: PageTable::fast_page_as_sized<uint16_t>((access_offset % PAGE_SIZE) / access_size) = value; break;
-						case 4: PageTable::fast_page_as_sized<uint32_t>((access_offset % PAGE_SIZE) / access_size) = value; break;
-						case 8: PageTable::fast_page_as_sized<uint64_t>((access_offset % PAGE_SIZE) / access_size) = value; break;
-					}
-				});
-				return true;
-			}
-			case OpRegion::RegionSpace::SystemIO:
-			{
-				switch (access_size)
-				{
-					case 1: IO::outb(access_offset, value); break;
-					case 2: IO::outw(access_offset, value); break;
-					case 4: IO::outl(access_offset, value); break;
-					default:
-						AML_ERROR("FieldElement write_field (SystemIO) with access size {}", access_size);
 			return false;
-				}
+		auto read_func = [&](uint64_t byte_offset) -> BAN::Optional<uint64_t> {
-				return true;
+			return perform_read(op_region->region_space, byte_offset, access_size.value());
-			}
+		};
-			case OpRegion::RegionSpace::PCIConfig:
+		auto write_func = [&](uint64_t byte_offset, uint64_t value) -> bool {
-			{
+			return perform_write(op_region->region_space, byte_offset, access_size.value(), value);
-				// https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#address-space-format
+		};
-				// PCI configuration space is confined to segment 0, bus 0
+		return perform_write_general(op_region->region_offset, bit_count, bit_offset, access_size.value(), value, access_rules.update_rule, read_func, write_func);
-
-				uint16_t device		= (access_offset >> 32) & 0xFFFF;
-				uint16_t function	= (access_offset >> 16) & 0xFFFF;
-				uint16_t offset		= access_offset & 0xFFFF;
-
-				switch (access_size)
-				{
-					case 1: PCI::PCIManager::write_config_byte(0, device, function, offset, value); break;
-					case 2: PCI::PCIManager::write_config_word(0, device, function, offset, value); break;
-					case 4: PCI::PCIManager::write_config_dword(0, device, function, offset, value); break;
-					default:
-						AML_ERROR("FieldElement write_field (PCIConfig) with access size {}", access_size);
-						return false;
-				}
-				return true;
-			}
-			default:
-				AML_TODO("FieldElement write_field with region space {}", static_cast<uint8_t>(op_region->region_space));
-				return false;
-		}
 	}
 
 	BAN::RefPtr<AML::Node> AML::FieldElement::evaluate()
 	{
-		// Field LockRule only applies to modifying the field, not reading it
+		auto result = evaluate_internal();
-
-		auto access_type = determine_access_type();
-		if (!access_type.has_value())
-			return {};
-
-		auto result = read_field(access_type->offset, access_type->access_size);
 		if (!result.has_value())
 			return {};
-
+		return MUST(BAN::RefPtr<Integer>::create(result.value()));
-		return MUST(BAN::RefPtr<Integer>::create((result.value() >> access_type->shift) & access_type->mask));
 	}
 
 	bool AML::FieldElement::store(BAN::RefPtr<AML::Node> source)
@@ -327,10 +435,6 @@ namespace Kernel::ACPI
 			return false;
 		}
 
-		auto access_type = determine_access_type();
-		if (!access_type.has_value())
-			return false;
-
 		if (access_rules.lock_rule == FieldRules::LockRule::Lock)
 			Namespace::root_namespace()->global_lock.lock();
 		BAN::ScopeGuard unlock_guard([&] {
@@ -338,40 +442,18 @@ namespace Kernel::ACPI
 				Namespace::root_namespace()->global_lock.unlock();
 		});
 
-		uint64_t to_write = 0;
+		return store_internal(source_integer.value());
-		switch (access_rules.update_rule)
-		{
-			case FieldRules::UpdateRule::Preserve:
-			{
-				auto read_result = read_field(access_type->offset, access_type->access_size);
-				if (!read_result.has_value())
-					return false;
-				to_write = read_result.value();
-				to_write &= ~(access_type->mask << access_type->shift);
-				to_write |= (source_integer.value() & access_type->mask) << access_type->shift;
-				break;
-			}
-			case FieldRules::UpdateRule::WriteAsOnes:
-				to_write = ~(access_type->mask << access_type->shift);
-				to_write |= (source_integer.value() & access_type->mask) << access_type->shift;
-				break;
-			case FieldRules::UpdateRule::WriteAsZeros:
-				to_write = 0;
-				to_write |= (source_integer.value() & access_type->mask) << access_type->shift;
-				break;
-		}
-
-		return write_field(access_type->offset, access_type->access_size, to_write);
 	}
 
 	void AML::FieldElement::debug_print(int indent) const
 	{
 		AML_DEBUG_PRINT_INDENT(indent);
-		AML_DEBUG_PRINT("FieldElement ");
+		AML_DEBUG_PRINT("FieldElement {} ({}, offset {}, OpRegion {})",
-		name.debug_print();
+			name,
-		AML_DEBUG_PRINT("({}, offset {}, OpRegion ", bit_count, bit_offset);
+			bit_count,
-		op_region->name.debug_print();
+			bit_offset,
-		AML_DEBUG_PRINT(")");
+			op_region->name
+		);
 	}
 
 	AML::ParseResult AML::IndexField::parse(ParseContext& context)
@@ -440,16 +522,88 @@ namespace Kernel::ACPI
 		return AML::ParseResult::Success;
 	}
 
+	BAN::RefPtr<AML::Node> AML::IndexFieldElement::evaluate()
+	{
+		auto access_size = determine_access_size(access_rules.access_type);
+		if (!access_size.has_value())
+			return {};
+		if (access_size.value() > data_element->bit_count)
+		{
+			AML_ERROR("IndexFieldElement read_field with access size {} > data element bit count {}", access_size.value(), data_element->bit_count);
+			return {};
+		}
+		auto read_func = [&](uint64_t byte_offset) -> BAN::Optional<uint64_t> {
+			if (!index_element->store_internal(byte_offset))
+				return {};
+			return data_element->evaluate_internal();
+		};
+
+		if (access_rules.lock_rule == FieldRules::LockRule::Lock)
+			Namespace::root_namespace()->global_lock.lock();
+		BAN::ScopeGuard unlock_guard([&] {
+			if (access_rules.lock_rule == FieldRules::LockRule::Lock)
+				Namespace::root_namespace()->global_lock.unlock();
+		});
+
+		auto result = perform_read_general(0, bit_count, bit_offset, access_size.value(), read_func);
+		if (!result.has_value())
+			return {};
+		return MUST(BAN::RefPtr<Integer>::create(result.value()));
+	}
+
+	bool AML::IndexFieldElement::store(BAN::RefPtr<Node> source)
+	{
+		auto source_integer = source->as_integer();
+		if (!source_integer.has_value())
+		{
+			AML_TODO("IndexFieldElement store with non-integer source, type {}", static_cast<uint8_t>(source->type));
+			return false;
+		}
+
+		auto access_size = determine_access_size(access_rules.access_type);
+		if (!access_size.has_value())
+			return false;
+
+		if (access_size.value() > data_element->bit_count)
+		{
+			AML_ERROR("IndexFieldElement write_field with access size {} > data element bit count {}", access_size.value(), data_element->bit_count);
+			return false;
+		}
+
+		auto read_func = [&](uint64_t byte_offset) -> BAN::Optional<uint64_t> {
+			if (!index_element->store_internal(byte_offset))
+				return {};
+			return data_element->evaluate_internal();
+		};
+		auto write_func = [&](uint64_t byte_offset, uint64_t value) -> bool {
+			if (!index_element->store_internal(byte_offset))
+				return false;
+			return data_element->store_internal(value);
+		};
+
+		if (access_rules.lock_rule == FieldRules::LockRule::Lock)
+			Namespace::root_namespace()->global_lock.lock();
+		BAN::ScopeGuard unlock_guard([&] {
+			if (access_rules.lock_rule == FieldRules::LockRule::Lock)
+				Namespace::root_namespace()->global_lock.unlock();
+		});
+
+		if (!perform_write_general(0, bit_count, bit_offset, access_size.value(), source_integer.value(), access_rules.update_rule, read_func, write_func))
+			return false;
+
+		return true;
+	}
+
 	void AML::IndexFieldElement::debug_print(int indent) const
 	{
 		AML_DEBUG_PRINT_INDENT(indent);
-		AML_DEBUG_PRINT("IndexFieldElement ");
+		AML_DEBUG_PRINT("IndexFieldElement {} ({}, offset {}, IndexName {}, DataName {})",
-		name.debug_print();
+			name,
-		AML_DEBUG_PRINT("({}, offset {}, IndexName ", bit_count, bit_offset);
+			bit_count,
-		index_element->name.debug_print();
+			bit_offset,
-		AML_DEBUG_PRINT(", DataName ");
+			index_element->name,
-		data_element->name.debug_print();
+			data_element->name
-		AML_DEBUG_PRINT(")");
+		);
 	}
 
 }