From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:48331) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQEaD-00035T-R2 for qemu-devel@nongnu.org; Mon, 22 Oct 2012 05:46:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TQEa9-0004Sk-7u for qemu-devel@nongnu.org; Mon, 22 Oct 2012 05:46:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15075) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TQEa8-0004Sg-Vp for qemu-devel@nongnu.org; Mon, 22 Oct 2012 05:46:01 -0400 Date: Mon, 22 Oct 2012 12:48:06 +0200 From: "Michael S. Tsirkin" Message-ID: <20121022104806.GB28828@redhat.com> References: <44ade6cc409ff55e4623d0db05e1a79b5f00cab2.1350677361.git.jbaron@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <44ade6cc409ff55e4623d0db05e1a79b5f00cab2.1350677361.git.jbaron@redhat.com> Subject: Re: [Qemu-devel] [PATCH v3 02/26] blockdev: Introduce IF_AHCI List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Baron Cc: kwolf@redhat.com, agraf@suse.de, aliguori@us.ibm.com, juzhang@redhat.com, jan.kiszka@siemens.com, qemu-devel@nongnu.org, armbru@redhat.com, blauwirbel@gmail.com, yamahata@valinux.co.jp, alex.williamson@redhat.com, kevin@koconnor.net, avi@redhat.com, mkletzan@redhat.com, pbonzini@redhat.com, lcapitulino@redhat.com, afaerber@suse.de, kraxel@redhat.com On Fri, Oct 19, 2012 at 04:43:27PM -0400, Jason Baron wrote: > From: Jason Baron > > Introduce IF_AHCI so that q35 can differentiate between ide and ahci disks. > This allows q35 to specify its default disk type. It also allows q35 to > differentiate between ahci and ide disks, such that -drive if=ide does not > result in the creating of an ahci disk. This is important, since we don't want > to have the meaning of if=ide changing once q35 is introduced. Thus, its > important for this to be applied before we introduce q35. > > This patch also adds: > > pci_ahci_create_devs(PCIDevice *pci_dev, DriveInfo **hd_table) > > Which provides a convient way of attaching ahci drives to an > ahci controller. > > Reviewed-by: Paolo Bonzini > Signed-off-by: Jason Baron > --- Kevin, could you review/ack this patch pls? > blockdev.c | 13 ++++++++++++- > blockdev.h | 2 ++ > hw/ide.h | 6 ++++++ > hw/ide/ahci.c | 18 ++++++++++++++++++ > hw/ide/core.c | 23 ++++++++++++++++++----- > 5 files changed, 56 insertions(+), 6 deletions(-) > > diff --git a/blockdev.c b/blockdev.c > index c9a49c8..b684348 100644 > --- a/blockdev.c > +++ b/blockdev.c > @@ -33,6 +33,7 @@ static const char *const if_name[IF_COUNT] = { > [IF_SD] = "sd", > [IF_VIRTIO] = "virtio", > [IF_XEN] = "xen", > + [IF_AHCI] = "ahci", > }; > > static const int if_max_devs[IF_COUNT] = { > @@ -52,8 +53,17 @@ static const int if_max_devs[IF_COUNT] = { > */ > [IF_IDE] = 2, > [IF_SCSI] = 7, > + [IF_AHCI] = 6, > }; > > +int get_if_max_devs(BlockInterfaceType if_type) > +{ > + assert(if_type < IF_COUNT); > + assert(if_type >= IF_DEFAULT); > + > + return if_max_devs[if_type]; > +} > + > /* > * We automatically delete the drive when a device using it gets > * unplugged. Questionable feature, but we can't just drop it. > @@ -518,7 +528,7 @@ DriveInfo *drive_init(QemuOpts *opts, int mach_if) > } else { > /* no id supplied -> create one */ > dinfo->id = g_malloc0(32); > - if (type == IF_IDE || type == IF_SCSI) > + if (type == IF_IDE || type == IF_SCSI || type == IF_AHCI) > mediastr = (media == MEDIA_CDROM) ? "-cd" : "-hd"; > if (max_devs) > snprintf(dinfo->id, 32, "%s%i%s%i", > @@ -550,6 +560,7 @@ DriveInfo *drive_init(QemuOpts *opts, int mach_if) > > switch(type) { > case IF_IDE: > + case IF_AHCI: > case IF_SCSI: > case IF_XEN: > case IF_NONE: > diff --git a/blockdev.h b/blockdev.h > index 8b126ad..bbd1017 100644 > --- a/blockdev.h > +++ b/blockdev.h > @@ -21,6 +21,7 @@ typedef enum { > IF_DEFAULT = -1, /* for use with drive_add() only */ > IF_NONE, > IF_IDE, IF_SCSI, IF_FLOPPY, IF_PFLASH, IF_MTD, IF_SD, IF_VIRTIO, IF_XEN, > + IF_AHCI, > IF_COUNT > } BlockInterfaceType; > > @@ -56,6 +57,7 @@ static inline int get_mach_if(int mach_if) > return mach_if; > } > > +int get_if_max_devs(BlockInterfaceType if_type); > DriveInfo *drive_get(BlockInterfaceType type, int bus, int unit); > DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); > int drive_get_max_bus(BlockInterfaceType type); > diff --git a/hw/ide.h b/hw/ide.h > index 2db4079..0b7e000 100644 > --- a/hw/ide.h > +++ b/hw/ide.h > @@ -4,6 +4,7 @@ > #include "isa.h" > #include "pci.h" > #include "memory.h" > +#include "blockdev.h" > > #define MAX_IDE_DEVS 2 > > @@ -34,6 +35,11 @@ int ide_get_geometry(BusState *bus, int unit, > int ide_get_bios_chs_trans(BusState *bus, int unit); > > /* ide/core.c */ > +void ata_drive_get(DriveInfo **hd, int max_bus, BlockInterfaceType type); > void ide_drive_get(DriveInfo **hd, int max_bus); > +void ahci_drive_get(DriveInfo **hd, int max_bus); > + > +/* ide/ahci.c */ > +void pci_ahci_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 68671bc..824b86f 100644 > --- a/hw/ide/ahci.c > +++ b/hw/ide/ahci.c > @@ -26,6 +26,7 @@ > #include > #include > #include > +#include > > #include "monitor.h" > #include "dma.h" > @@ -1260,3 +1261,20 @@ static void sysbus_ahci_register_types(void) > } > > type_init(sysbus_ahci_register_types) > + > +void pci_ahci_create_devs(PCIDevice *pci_dev, DriveInfo **hd_table) > +{ > + struct AHCIPCIState *dev = DO_UPCAST(struct AHCIPCIState, card, pci_dev); > + int i; > + DriveInfo *drive; > + > + for (i = 0; i < dev->ahci.ports; i++) { > + if (hd_table[i] == NULL) { > + continue; > + } > + drive = hd_table[i]; > + assert(drive->type == IF_AHCI); > + ide_create_drive(&dev->ahci.dev[i].port, 0, > + hd_table[i]); > + } > +} > diff --git a/hw/ide/core.c b/hw/ide/core.c > index d683a8c..044da3c 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -2341,16 +2341,29 @@ const VMStateDescription vmstate_ide_bus = { > } > }; > > -void ide_drive_get(DriveInfo **hd, int max_bus) > +void ata_drive_get(DriveInfo **hd, int max_bus, BlockInterfaceType type) > { > int i; > + int max_devs; > + > + assert((type == IF_IDE) || type == IF_AHCI); > > - if (drive_get_max_bus(IF_IDE) >= max_bus) { > + if (drive_get_max_bus(type) >= max_bus) { > fprintf(stderr, "qemu: too many IDE bus: %d\n", max_bus); > exit(1); > } > - > - for(i = 0; i < max_bus * MAX_IDE_DEVS; i++) { > - hd[i] = drive_get(IF_IDE, i / MAX_IDE_DEVS, i % MAX_IDE_DEVS); > + max_devs = get_if_max_devs(type); > + for (i = 0; i < max_bus * max_devs; i++) { > + hd[i] = drive_get(type, i / max_devs, i % max_devs); > } > } > + > +void ide_drive_get(DriveInfo **hd, int max_bus) > +{ > + ata_drive_get(hd, max_bus, IF_IDE); > +} > + > +void ahci_drive_get(DriveInfo **hd, int max_bus) > +{ > + ata_drive_get(hd, max_bus, IF_AHCI); > +} > -- > 1.7.1