From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40692) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TF3rC-0007pT-Os for qemu-devel@nongnu.org; Fri, 21 Sep 2012 10:05:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TF3r7-0006Fq-AW for qemu-devel@nongnu.org; Fri, 21 Sep 2012 10:05:26 -0400 Received: from mx1.redhat.com ([209.132.183.28]:12759) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TF3r6-0006Db-Oh for qemu-devel@nongnu.org; Fri, 21 Sep 2012 10:05:21 -0400 From: Markus Armbruster References: <8a374796284b9bcb9685833809f6c6932f6430c6.1347561356.git.jbaron@redhat.com> Date: Fri, 21 Sep 2012 16:05:14 +0200 In-Reply-To: <8a374796284b9bcb9685833809f6c6932f6430c6.1347561356.git.jbaron@redhat.com> (Jason Baron's message of "Thu, 13 Sep 2012 16:12:38 -0400") Message-ID: <8762778o9x.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Baron Cc: aliguori@us.ibm.com, juzhang@redhat.com, mst@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, agraf@suse.de, yamahata@valinux.co.jp, alex.williamson@redhat.com, kevin@koconnor.net, avi@redhat.com, mkletzan@redhat.com, lcapitulino@redhat.com, afaerber@suse.de Jason Baron writes: > From: Isaku Yamahata > > Introduce a helper function which initializes the ahci port with ide devices. > It will be used by q35 support. > > Cc: Alexander Graf > Signed-off-by: Isaku Yamahata > Signed-off-by: Jason Baron > --- > 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?