[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helpe
From: |
Jason Baron |
Subject: |
Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper |
Date: |
Fri, 21 Sep 2012 15:37:36 -0400 |
User-agent: |
Mutt/1.5.20 (2009-12-10) |
On Fri, Sep 21, 2012 at 04:05:14PM +0200, Markus Armbruster wrote:
> Jason Baron <address@hidden> writes:
>
> > From: Isaku Yamahata <address@hidden>
> >
> > Introduce a helper function which initializes the ahci port with ide
> > devices.
> > It will be used by q35 support.
> >
> > Cc: Alexander Graf <address@hidden>
> > Signed-off-by: Isaku Yamahata <address@hidden>
> > Signed-off-by: Jason Baron <address@hidden>
> > ---
> > 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?
Right, so now that we have ide disks that can be attached to either the
legacy ide controller or to ahci, I think we need to differentiate which
controller we mean. That is, as proposed q35 is treating -drive if=ide
as an ide attached to the ahci controller. I think that is broken
behavior b/c we need a way to differentiate between the controllers.
As Alexander Graf has proposed before, I think we need a -drive if=ahci
introduced. In that case, I think we reject unit > 0, as you've
suggested.
In terms of the current q35 patch series, I think the first step would
be to introduce the ahci interface type, and have hda-hdd be added with
the default type for q35 of ahci. Then, we can simply fetch ahci drives
of index 0-3, and populate the controller, without any of this skipping
odd numbers stuff.
The next step would then to add if=ahci interface to -drive.
Thanks,
-Jason
- [Qemu-devel] [PATCH 08/25] pci_ids: add intel 82801BA pci-to-pci bridge id and PCI_CLASS_SERIAL_SMBUS, (continued)
- [Qemu-devel] [PATCH 08/25] pci_ids: add intel 82801BA pci-to-pci bridge id and PCI_CLASS_SERIAL_SMBUS, Jason Baron, 2012/09/13
- [Qemu-devel] [PATCH 07/25] pc/piix_pci: factor out smram/pam logic, Jason Baron, 2012/09/13
- [Qemu-devel] [PATCH 10/25] pcie: Convert PCIExpressHost to use the QOM., Jason Baron, 2012/09/13
- [Qemu-devel] [PATCH 06/25] pc: Move ioapic_init() from pc_piix.c to pc.c, Jason Baron, 2012/09/13
- [Qemu-devel] [PATCH 13/25] q35: Suppress SMM BIOS initialization under KVM, Jason Baron, 2012/09/13
- [Qemu-devel] [PATCH 03/25] pci: introduce pci_swizzle_map_irq_fn() for standardized interrupt pin swizzle, Jason Baron, 2012/09/13
- [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Jason Baron, 2012/09/13
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Markus Armbruster, 2012/09/21
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper,
Jason Baron <=
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Markus Armbruster, 2012/09/24
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Jason Baron, 2012/09/24
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Markus Armbruster, 2012/09/26
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Kevin Wolf, 2012/09/26
- Re: [Qemu-devel] [PATCH 04/25] ahci: add ide device initialization helper, Jason Baron, 2012/09/27
[Qemu-devel] [PATCH 15/25] q35: smbus: Remove PCI_STATUS_SIG_SYSTEM_ERROR and PCI_STATUS_DETECTED_PARITY from w1cmask, Jason Baron, 2012/09/13
[Qemu-devel] [PATCH 09/25] pcie: pass pcie window size to pcie_host_mmcfg_update(), Jason Baron, 2012/09/13
[Qemu-devel] [PATCH 17/25] q35: Add kvmclock support, Jason Baron, 2012/09/13
[Qemu-devel] [PATCH 14/25] q35: Fix non-PCI IRQ processing in ich9_lpc_update_apic, Jason Baron, 2012/09/13
[Qemu-devel] [PATCH 20/25] pcie: drop version_id field for live migration, Jason Baron, 2012/09/13