|
From: | John Snow |
Subject: | Re: [Qemu-devel] [PATCH v2 6/6] q35/ahci: Pick up -cdrom and -hda options |
Date: | Tue, 30 Sep 2014 13:32:47 -0400 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.1.0 |
On 09/30/2014 03:54 AM, Markus Armbruster wrote:
John Snow <address@hidden> writes:This patch implements the backend for the Q35 board for us to be able to pick up and use drives defined by the -cdrom, -hda, or -drive if=ide shorthand options. Signed-off-by: John Snow <address@hidden> --- hw/i386/pc_q35.c | 4 ++++ hw/ide/ahci.c | 15 +++++++++++++++ hw/ide/ahci.h | 2 ++ 3 files changed, 21 insertions(+) diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c index b28ddbb..bb0dc8e 100644 --- a/hw/i386/pc_q35.c +++ b/hw/i386/pc_q35.c @@ -86,6 +86,7 @@ static void pc_q35_init(MachineState *machine) DeviceState *icc_bridge; PcGuestInfo *guest_info; ram_addr_t lowmem; + DriveInfo *hd[MAX_SATA_PORTS]; /* Check whether RAM fits below 4G (leaving 1/2 GByte for IO memory * and 256 Mbytes for PCI Express Enhanced Configuration Access Mapping @@ -253,6 +254,9 @@ static void pc_q35_init(MachineState *machine) true, "ich9-ahci"); idebus[0] = qdev_get_child_bus(&ahci->qdev, "ide.0"); idebus[1] = qdev_get_child_bus(&ahci->qdev, "ide.1"); + g_assert_cmpint(MAX_SATA_PORTS, ==, ICH_AHCI(ahci)->ahci.ports); + ide_drive_get(hd, ICH_AHCI(ahci)->ahci.ports); + ahci_ide_create_devs(ahci, hd);The assertion is new since v1, and a bit more interesting than it looks on first glance. It protects the two calls following it, by ensuring the array has space for the ports. MAX_SATA_PORTS is defined to 6 in this file. ICH_AHCI(ahci)->ahci.ports is initialized by ahci_init() to its ports argument. pci_ich9_ahci_init() passes literal 6. Oookay. The assertion is more restrictive than required for correctness: >= would do. I don't mind.
It's more of a warning that these two values are, for now, considered equivalent. If that should change in the future for some unthinkable reason, this assertion will help remind whoever changes it.
For now, Q35 uses ICH9. ICH9's AHCI controller has 6 ports. This should always be the case.
I could, I suppose, make the HD array in the heap, and ask the ICH9 how many ports it has, etc... The assertion was a quick, equally authoritative way that is not likely to need changing.
It's tempting to do ide_drive_get(hd, ARRAY_SIZE(hd)); for more obvious correctness, except that'll screw with the detection of extra drives if ahci.ports ever becomes < MAX_SATA_PORTS. Good.if (usb_enabled(false)) { /* Should we create 6 UHCI according to ich9 spec? */ diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 8978643..79abb6a 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1419,3 +1419,18 @@ static void sysbus_ahci_register_types(void) } type_init(sysbus_ahci_register_types) + +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **tab)Elsewhere, we call it DriveInfo **hd. I'd stick to the common name.+{ + AHCIPCIState *d = ICH_AHCI(dev); + AHCIState *ahci = &d->ahci; + int i; + + for (i = 0; i < ahci->ports; i++) { + if (tab[i] == NULL) { + continue; + } + ide_create_drive(&ahci->dev[i].port, 0, tab[i]); + } + +} diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h index 1543df7..b6dc64e 100644 --- a/hw/ide/ahci.h +++ b/hw/ide/ahci.h @@ -332,4 +332,6 @@ void ahci_uninit(AHCIState *s); void ahci_reset(AHCIState *s); +void ahci_ide_create_devs(PCIDevice *dev, DriveInfo **tab); + #endif /* HW_IDE_AHCI_H */
[Prev in Thread] | Current Thread | [Next in Thread] |