Opened 2 years ago

Last modified 14 months ago

#13620 new bug

Strange/wrong channel attachment by generic ATA with more than one IDE/ATA controller

Reported by: Alexco Owned by: nobody
Priority: normal Milestone: Unscheduled
Component: Drivers/Disk/ATA Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

My BeOS/Haiku test system features 2 dedicated IDE controllers, each with 2 separate channels. One is the chipset one (Intel BX440), the other is a Highpoint 366. While investigating why the Highpoint is not supported, I found that the channel assignment in device tree is messed up (tested with hrev51219). If you look at the distdev output you can see that the Intel channel is attached to the Highpoint PCI (vendor 0x1103) device while the device attached to the Highpoint is not seen as all.

Looking at the dmesg output of FreeBSD confirms that all addresses are identified correctly.

Attachments (2)

listdev_haiku.txt (19.0 KB) - added by Alexco 2 years ago.
dmesg_fbsd.txt (10.8 KB) - added by Alexco 2 years ago.

Download all attachments as: .zip

Change History (12)

Changed 2 years ago by Alexco

Attachment: listdev_haiku.txt added

comment:1 Changed 2 years ago by Alexco

Has a Patch: set

Changed 2 years ago by Alexco

Attachment: dmesg_fbsd.txt added

comment:2 Changed 2 years ago by Alexco

I would have searched deeper but I couldn't find out the connection between Device Manager and ATA stack. Which one is started first, which component is created/started by whom, how is channel detection startet, etc..

comment:3 Changed 2 years ago by Alexco

The reason for this behaviour lies in the implementation of ata_adapter.cpp and generic_ide_pci.cpp. Every IDE controller which is handled by generic_ide_pci is configured with "compatibility" mode set to true:

generic_ide_pci.cpp:

static status_t
probe_controller(device_node *parent)
{
	return sATAAdapter->probe_controller(parent,
		GENERIC_IDE_PCI_CONTROLLER_MODULE_NAME, "generic_ide_pci",
		"Generic IDE PCI Controller",
		GENERIC_IDE_PCI_CHANNEL_MODULE_NAME,
		true,
		true,					// assume that command queuing works
		1,						// assume 16 bit alignment is enough
		0xffff,					// boundary is on 64k according to spec
		0x10000,				// up to 64k per S/G block according to spec
		true);					// by default, compatibility mode is used
}

This forces fixed addresses for IDE channels:

ata_adapter.cpp:

	if (supports_compatibility_mode
		&& channel_index == 0 && (api & PCI_ide_primary_native) == 0) {
		command_block_base = 0x1f0;
		control_block_base = 0x3f6;
		intnum = 14;
		TRACE("PCI-ATA: Controller in legacy mode: cmd %#x, ctrl %#x, irq %d\n",
			  command_block_base, control_block_base, intnum);
	} else if (supports_compatibility_mode
		&& channel_index == 1 && (api & PCI_ide_secondary_native) == 0) {
		command_block_base = 0x170;
		control_block_base = 0x376;
		intnum = 15;
		TRACE("PCI-ATA: Controller in legacy mode: cmd %#x, ctrl %#x, irq %d\n",
			  command_block_base, control_block_base, intnum);
	}

If a system is equipped with additional IDE controllers (compared to the chipset one, which is configured by BIOS), these are of course then configured to use different command & control block bases and interrupts. What happens now is that during driver/PCI device enumeration the additional controller is enumerated first, with the chipset controller beeing second. The additional controller then creates the IDE channels and configures these in compatibility mode with the command & control block bases and interrupts of the chipset controller. This results in

  1. strange/wrong channel attachments (this ticket)
  2. not working DMA, since the wrong controller is used (#12292)

Since not much systems feature more than one IDE controller I would propose not to change the current implementation but to provide additional IDE controller drivers on a case-by-case basis (in my case I wrote a Highpoint driver which initialises all channels correctly, so I was able to verify the dependency of non-working DMA).

But I have no idea if a similar issue exists for modern SATA controllers.

comment:4 Changed 2 years ago by pulkomandy

Modern SATA controllers will have the same issue if set to "legacy" non-AHCI mode, where they are looking like IDE controllers to the OS. Can't we make sure that only the first controller found is set to compatibility mode? Or do we need a more clever way to detect which one is the chipset one, and which are extensions?

comment:5 Changed 2 years ago by Alexco

Currently the order of enumeration of PCI devices is unclear for me. On my test system the enumeration starts with the additional controller which results in the observed mismatch. I don't know how other systems do that, but looking at FreeBSD I see a lot of different chipset drivers which cover all that. For me that sounds logically. At the moment I can't figure out a way of distinguishing between chipset controller and additional controller so I wrote a separate highpoint driver.

comment:6 Changed 2 years ago by pulkomandy

Has a Patch: unset

comment:7 Changed 14 months ago by waddlesplash

Is this fixed following the merge of #13819?

comment:8 Changed 14 months ago by Alexco

Only for the combination of Intel IDE (generic ide driver) and Hightpoint IDE, or any other combination of generic ide and specific ide driver. It will still happen if the generic ide driver needs to attach to more than one device. Since this is seldom nowadays (SATA, anyone?) this is not a big issue, I guess. But the design is still flawed and the IDE/SATA stack needs a big redesign, IMHO.

comment:9 Changed 14 months ago by pulkomandy

I'm still unhappy with having to write "specific" drivers (they are just a copypaste of the IDE driver with a filter on specific devices PCI IDs. This makes no sense, there should be a generic driver and it should handle all controllers.

It should indeed detect the main controller and put this one (and only this one) in "compatibility" mode. We are certainly not going to add specific drivers for each extended IDE controller out there, especially if there are no actual code changes...

comment:10 Changed 14 months ago by Alexco

No, sorry. They are not only copy&paste, at least not for some of them, since they have different PCI register mapping, or a special init procedure. Integrate this all in generic would make this unmaintainable. A nice solution is done by FreeBSD where all those chipset properties is separated in different files. And as I said before, this thing needs a redesign. For example, I still fail to see why there is such thing as a channel object...

Note: See TracTickets for help on using tickets.