mirror of
				https://github.com/RGBCube/serenity
				synced 2025-10-31 11:12:45 +00:00 
			
		
		
		
	Kernel: Refactor AHCIController to propagate more errors
Before, the mapping of our HBA region would be done in the constructor. Since this can fail, I moved it into initialize(). Additionally, we now use the TypedMapping helper for mapping the HBA instead of doing it manually. This actually uncovered a bug where we would ignore any possible offset into the page we were mapping, which caused us to miss the mapped registers entirely.
This commit is contained in:
		
							parent
							
								
									5541dfd9f8
								
							
						
					
					
						commit
						f31a9e9374
					
				
					 2 changed files with 10 additions and 8 deletions
				
			
		|  | @ -106,18 +106,16 @@ volatile AHCI::PortRegisters& AHCIController::port(size_t port_number) const | |||
| 
 | ||||
| volatile AHCI::HBA& AHCIController::hba() const | ||||
| { | ||||
|     return static_cast<volatile AHCI::HBA&>(*(volatile AHCI::HBA*)(m_hba_region->vaddr().as_ptr())); | ||||
|     return const_cast<AHCI::HBA&>(*m_hba_mapping); | ||||
| } | ||||
| 
 | ||||
| UNMAP_AFTER_INIT AHCIController::AHCIController(PCI::DeviceIdentifier const& pci_device_identifier) | ||||
|     : ATAController() | ||||
|     , PCI::Device(const_cast<PCI::DeviceIdentifier&>(pci_device_identifier)) | ||||
|     , m_hba_region(default_hba_region()) | ||||
|     , m_hba_capabilities(capabilities()) | ||||
| { | ||||
| } | ||||
| 
 | ||||
| AHCI::HBADefinedCapabilities AHCIController::capabilities() const | ||||
| UNMAP_AFTER_INIT AHCI::HBADefinedCapabilities AHCIController::capabilities() const | ||||
| { | ||||
|     u32 capabilities = hba().control_regs.cap; | ||||
|     u32 extended_capabilities = hba().control_regs.cap2; | ||||
|  | @ -154,15 +152,18 @@ AHCI::HBADefinedCapabilities AHCIController::capabilities() const | |||
|     }; | ||||
| } | ||||
| 
 | ||||
| UNMAP_AFTER_INIT NonnullOwnPtr<Memory::Region> AHCIController::default_hba_region() const | ||||
| UNMAP_AFTER_INIT ErrorOr<Memory::TypedMapping<AHCI::HBA volatile>> AHCIController::map_default_hba_region(PCI::DeviceIdentifier const& pci_device_identifier) | ||||
| { | ||||
|     return MM.allocate_kernel_region(PhysicalAddress(PCI::get_BAR5(device_identifier())).page_base(), Memory::page_round_up(sizeof(AHCI::HBA)).release_value_but_fixme_should_propagate_errors(), "AHCI HBA"sv, Memory::Region::Access::ReadWrite).release_value(); | ||||
|     return Memory::map_typed_writable<AHCI::HBA volatile>(PhysicalAddress(PCI::get_BAR5(pci_device_identifier))); | ||||
| } | ||||
| 
 | ||||
| AHCIController::~AHCIController() = default; | ||||
| 
 | ||||
| UNMAP_AFTER_INIT ErrorOr<void> AHCIController::initialize_hba(PCI::DeviceIdentifier const& pci_device_identifier) | ||||
| { | ||||
|     m_hba_mapping = TRY(map_default_hba_region(pci_device_identifier)); | ||||
|     m_hba_capabilities = capabilities(); | ||||
| 
 | ||||
|     u32 version = hba().control_regs.version; | ||||
| 
 | ||||
|     hba().control_regs.ghc = 0x80000000; // Ensure that HBA knows we are AHCI aware.
 | ||||
|  |  | |||
|  | @ -9,6 +9,7 @@ | |||
| #include <AK/OwnPtr.h> | ||||
| #include <AK/Types.h> | ||||
| #include <Kernel/Library/LockRefPtr.h> | ||||
| #include <Kernel/Memory/TypedMapping.h> | ||||
| #include <Kernel/Sections.h> | ||||
| #include <Kernel/Storage/ATA/AHCI/Definitions.h> | ||||
| #include <Kernel/Storage/ATA/ATAController.h> | ||||
|  | @ -49,11 +50,11 @@ private: | |||
|     LockRefPtr<StorageDevice> device_by_port(u32 index) const; | ||||
| 
 | ||||
|     volatile AHCI::PortRegisters& port(size_t port_number) const; | ||||
|     NonnullOwnPtr<Memory::Region> default_hba_region() const; | ||||
|     ErrorOr<Memory::TypedMapping<AHCI::HBA volatile>> map_default_hba_region(PCI::DeviceIdentifier const&); | ||||
|     volatile AHCI::HBA& hba() const; | ||||
| 
 | ||||
|     Array<LockRefPtr<AHCIPort>, 32> m_ports; | ||||
|     NonnullOwnPtr<Memory::Region> m_hba_region; | ||||
|     Memory::TypedMapping<AHCI::HBA volatile> m_hba_mapping; | ||||
|     AHCI::HBADefinedCapabilities m_hba_capabilities; | ||||
| 
 | ||||
|     // FIXME: There could be multiple IRQ (MSI) handlers for AHCI. Find a way to use all of them.
 | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue
	
	 Julian Offenhäuser
						Julian Offenhäuser