Clean up AHCI: volatile, and sata_reset

src/kernel/ahci/hba.cpp

@@ -64,8 +64,9 @@ hba::hba(pci_device *device)
 	uint32_t bar5 = device->get_bar(5);
 	log::debug(logs::driver, "HBA raw BAR5 is %08lx", bar5);
 
-	m_data = reinterpret_cast<hba_data *>(bar5 & ~0xfffull);
-	pm->map_offset_pointer(reinterpret_cast<void **>(&m_data), 0x2000);
+	void *data = reinterpret_cast<void *>(bar5 & ~0xfffull);
+	pm->map_offset_pointer(&data, 0x2000);
+	m_data = reinterpret_cast<hba_data volatile *>(data);
 
 	if (! bitfield_has(m_data->cap, hba_cap::ahci_only))
 		m_data->host_control |= 0x80000000; // Enable AHCI mode
@@ -78,7 +79,7 @@ hba::hba(pci_device *device)
 	log::debug(logs::driver, "  %d ports", ports);
 	log::debug(logs::driver, "  %d command slots", slots);
 
-	port_data *pd = reinterpret_cast<port_data *>(
+	auto *pd = reinterpret_cast<port_data volatile *>(
 			kutil::offset_pointer(m_data, 0x100));
 
 	bool needs_interrupt = false;
@@ -99,9 +100,11 @@ hba::hba(pci_device *device)
 		if (!p.active()) continue;
 
 		if (p.get_type() == sata_signature::sata_drive) {
-			p.identify_async();
+			p.sata_reconnect();
+			/*
 			if (fs::partition::load(&p) == 0)
 				dm.register_block_device(&p);
+			*/
 		}
 	}
 }
@@ -129,7 +132,7 @@ hba::dump()
 	};
 
 	cons->printf("HBA Registers:\n");
-	uint32_t *data = reinterpret_cast<uint32_t *>(m_data);
+	auto *data = reinterpret_cast<uint32_t volatile *>(m_data);
 	for (int i = 0; i < 11; ++i) {
 		cons->printf("  %s: %08x\n", regs[i], data[i]);
 	}

src/kernel/ahci/hba.h

@@ -30,7 +30,7 @@ public:
 
 private:
 	pci_device *m_device;
-	hba_data *m_data;
+	hba_data volatile *m_data;
 	kutil::vector<port> m_ports;
 };
 

src/kernel/ahci/port.cpp

@@ -15,6 +15,7 @@ namespace ahci {
 }
 
 IS_BITFIELD(ahci::port_cmd);
+IS_BITFIELD(volatile ahci::port_cmd);
 IS_BITFIELD(ahci::cmd_list_flags);
 
 namespace ahci {
@@ -117,7 +118,7 @@ struct port_data
 } __attribute__ ((packed));
 
 
-port::port(hba *device, uint8_t index, port_data *data, bool impl) :
+port::port(hba *device, uint8_t index, port_data volatile *data, bool impl) :
 	m_index(index),
 	m_type(sata_signature::none),
 	m_state(state::unimpl),
@@ -351,6 +352,14 @@ port::identify_async()
 		return -1;
 }
 
+void
+port::sata_reconnect()
+{
+	m_data->serial_control |= 1;
+	io_wait(1000); // About 1ms
+	m_data->serial_control &= ~1;
+}
+
 bool
 port::issue_command(int slot)
 {
@@ -378,8 +387,18 @@ port::handle_interrupt()
 
 	// TODO: handle other states in interrupt_status
 
-	if (m_data->interrupt_status & 0x40000000) {
+	uint32_t is = m_data->interrupt_status;
+
+	if (is & 0x00000040) {
+		// Port connect status change: For now clear the "exchange"
+		// bit in SERR, this should probably kick off diagnostics.
+		m_data->serial_error = 0x04000000;
+		identify_async();
+	}
+
+	if (is & 0x40000000) {
 		log::error(logs::driver, "AHCI task file error");
+		dump();
 		kassert(0, "Task file error");
 	}
 
@@ -589,7 +608,7 @@ port::dump()
 	};
 
 	cons->printf("Port Registers:\n");
-	uint32_t *data = reinterpret_cast<uint32_t *>(m_data);
+	auto *data = reinterpret_cast<volatile uint32_t *>(m_data);
 	for (int i = 0; i < 18; ++i) {
 		if (regs[i]) cons->printf("  %s: %08x\n", regs[i], data[i]);
 	}

src/kernel/ahci/port.h

@@ -27,7 +27,7 @@ public:
 	/// \arg index  Index of the port on its HBA
 	/// \arg data   Pointer to the device's registers for this port
 	/// \arg impl   Whether this port is marked as implemented in the HBA
-	port(hba *device, uint8_t index, port_data *data, bool impl);
+	port(hba *device, uint8_t index, port_data volatile *data, bool impl);
 
 	/// Destructor
 	~port();
@@ -80,6 +80,10 @@ public:
 	/// \returns     A handle to the read operation, or -1 on error
 	int identify_async();
 
+	/// Tell the HBA to reconnect to the SATA device. A successful
+	/// reconnect will kick off an identify command.
+	void sata_reconnect();
+
 	/// Handle an incoming interrupt
 	void handle_interrupt();
 
@@ -123,7 +127,7 @@ private:
 	state m_state;
 
 	hba *m_hba;
-	port_data *m_data;
+	port_data volatile *m_data;
 	void *m_fis;
 	cmd_list_entry *m_cmd_list;
 	cmd_table *m_cmd_table;

src/kernel/io.cpp

@@ -31,7 +31,8 @@ wrmsr(uint64_t addr, uint64_t value)
 }
 
 void
-io_wait()
+io_wait(unsigned times)
 {
-	outb(0x80, 0);
+	for (unsigned i = 0; i < times; ++i)
+		outb(0x80, 0);
 }

src/kernel/io.h

@@ -25,7 +25,8 @@ uint64_t rdmsr(uint64_t addr);
 void wrmsr(uint64_t addr, uint64_t value);
 
 /// Pause briefly by doing IO to port 0x80
-void io_wait();
+/// \arg times  Number of times to delay by writing
+void io_wait(unsigned times = 1);
 
 }
 

src/modules/kutil/enum_bitfields.h

@@ -8,27 +8,27 @@ struct is_enum_bitfield { static constexpr bool value = false; };
 #define IS_BITFIELD(name) \
 	template<> struct ::is_enum_bitfield<name> {static constexpr bool value=true;}
 
-template <typename E>
+template <typename E, typename F>
 typename std::enable_if<is_enum_bitfield<E>::value,E>::type
-operator & (E lhs, E rhs)
+operator & (E lhs, F rhs)
 {
 	return static_cast<E> (
 		static_cast<typename std::underlying_type<E>::type>(lhs) &
 		static_cast<typename std::underlying_type<E>::type>(rhs));
 }
 
-template <typename E>
+template <typename E, typename F>
 typename std::enable_if<is_enum_bitfield<E>::value,E>::type
-operator | (E lhs, E rhs)
+operator | (E lhs, F rhs)
 {
 	return static_cast<E> (
 		static_cast<typename std::underlying_type<E>::type>(lhs) |
 		static_cast<typename std::underlying_type<E>::type>(rhs));
 }
 
-template <typename E>
+template <typename E, typename F>
 typename std::enable_if<is_enum_bitfield<E>::value,E>::type
-operator ^ (E lhs, E rhs)
+operator ^ (E lhs, F rhs)
 {
 	return static_cast<E> (
 		static_cast<typename std::underlying_type<E>::type>(lhs) ^
@@ -42,9 +42,9 @@ operator ~ (E rhs)
 	return static_cast<E>(~static_cast<typename std::underlying_type<E>::type>(rhs));
 }
 
-template <typename E>
+template <typename E, typename F>
 typename std::enable_if<is_enum_bitfield<E>::value,E>::type&
-operator |= (E &lhs, E rhs)
+operator |= (E &lhs, F rhs)
 {
 	lhs = static_cast<E>(
 		static_cast<typename std::underlying_type<E>::type>(lhs) |
@@ -53,9 +53,9 @@ operator |= (E &lhs, E rhs)
 	return lhs;
 }
 
-template <typename E>
+template <typename E, typename F>
 typename std::enable_if<is_enum_bitfield<E>::value,E>::type&
-operator &= (E &lhs, E rhs)
+operator &= (E &lhs, F rhs)
 {
 	lhs = static_cast<E>(
 		static_cast<typename std::underlying_type<E>::type>(lhs) &
@@ -64,9 +64,9 @@ operator &= (E &lhs, E rhs)
 	return lhs;
 }
 
-template <typename E>
+template <typename E, typename F>
 typename std::enable_if<is_enum_bitfield<E>::value,E>::type&
-operator ^= (E &lhs, E rhs)
+operator ^= (E &lhs, F rhs)
 {
 	lhs = static_cast<E>(
 		static_cast<typename std::underlying_type<E>::type>(lhs) ^