qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 6/6] q35/ahci: Pick up -cdrom and -hda option


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v2 6/6] q35/ahci: Pick up -cdrom and -hda options
Date: Tue, 30 Sep 2014 09:54:31 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux)

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 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 */



reply via email to

[Prev in Thread] Current Thread [Next in Thread]