Jason Baron <jba...@redhat.com> writes:
> From: Isaku Yamahata <yamah...@valinux.co.jp>
>
> Introduce a helper function which initializes the ahci port with ide devices.
> It will be used by q35 support.
>
> Cc: Alexander Graf <ag...@suse.de>
> Signed-off-by: Isaku Yamahata <yamah...@valinux.co.jp>
> Signed-off-by: Jason Baron <jba...@redhat.com>
> ---
> hw/ide.h | 3 +++
> hw/ide/ahci.c | 16 ++++++++++++++++
> 2 files changed, 19 insertions(+), 0 deletions(-)
>
> diff --git a/hw/ide.h b/hw/ide.h
> index 2db4079..8df872e 100644
> --- a/hw/ide.h
> +++ b/hw/ide.h
> @@ -36,4 +36,7 @@ int ide_get_bios_chs_trans(BusState *bus, int unit);
> /* ide/core.c */
> void ide_drive_get(DriveInfo **hd, int max_bus);
>
> +/* ide/ahci.c */
> +void pci_ahci_ide_create_devs(PCIDevice *pci_dev, DriveInfo **hd_table);
> +
> #endif /* HW_IDE_H */
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 5ea3cad..9561210 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -1260,3 +1260,19 @@ static void sysbus_ahci_register_types(void)
> }
>
> type_init(sysbus_ahci_register_types)
> +
> +void pci_ahci_ide_create_devs(PCIDevice *pci_dev, DriveInfo **hd_table)
> +{
> + struct AHCIPCIState *dev = DO_UPCAST(struct AHCIPCIState, card, pci_dev);
> + int i;
> +
> + for (i = 0; i < dev->ahci.ports; i++) {
> + /* master device only, ignore slaves */
> + if (hd_table[i * MAX_IDE_DEVS] == NULL) {
> + continue;
> + }
> + ide_create_drive(&dev->ahci.dev[i].port, 0,
> + hd_table[i * MAX_IDE_DEVS]);
> + }
> +}
> +
Ignores odd entries in hd_table[] (MAX_IDE_DEVS is 2). Here's my
attempt at explaining why.
-drive has parameters bus, unit, and index. index and (bus, unit) are
related in a well-known way that depends on parameter if. For if=ide,
index = bus * 2 + unit. This relationship is ABI, i.e. it cannot be
changed.
"bus * 2 + unit" makes sense for IDE, because each IDE bus can connect
two IDE devices, "master" and "slave".
Boards implementing IDE reject drives with (bus, unit) that make no
sense for the board's IDE controller(s). A typical board has a single
controller with two buses, which means bus > 1 get rejected.
q35 implements AHCI instead of IDE. It connects if=ide drives to AHCI,
because that's felt to be convenient.
An AHCI port can connect a single AHCI device, unlike an IDE bus. This
patch identifies maps -drive's bus to AHCI port number.
PATCH 11/25 sets up argument hd_table[] as follows:
ide_drive_get(hd, MAX_SATA_PORTS);
This rejects bus > MAX_SATA_PORTS. It doesn't reject unit == 1. I
believe these get silently ignored. Bug or feature?
Should we reject unit == 1 instead?
Should we map -drive's index to AHCI port number instead?
