Re: [Qemu-devel] [RFC 0/4] Adding -cdrom, -hd[abcd] and -drive file=... to Q35

[Markus Armbruster]

John Snow <address@hidden> writes:

> Currently, the drive definitions created by drive_new() when using
> the -drive file=...[,if=ide] or -cdrom or -hd[abcd] options are not
> picked up by the Q35 initialization routine.
>
> To fix this, we have to add hooks to search for these drives using
> something like pc_piix's ide_drive_get and then add them using
> something like pci_ide_create_devs.

ide_drive_get() isn't pc_piix's, it's a helper function in the IDE core
which boards (not just pc_piix) use to find the if=ide drives.  It fills
in an array of DriveInfo.

pci_ide_create_devs() is a helper function in the IDE PCI code which PCI
IDE controllers (not just piix3-ide) use to create IDE devices for an
array of DriveInfo.

> Where it gets slightly wonky is the fact that if=ide is specified
> to use two devices per bus, whereas AHCI does not utilize that
> same master/slave mechanic. Therefore, many places inside of
> blockdev.c where we add and define new drives use incorrect math
> for AHCI devices and try to place them on impossible buses.
> Notably -hdb and -hdd would become inaccessible.

Yes.

> To remedy this, I added a new interface type, IF_AHCI. Corresponding
> to this change, I modified the default machine properties for Q35
> to use this interface as a default.
>
> The changes appear to work well, but where I'd like some feedback
> is what should happen if people do something like:
>
> qemu -M q35 -drive if=ide,file=fedora.qcow2
>
> The code as presented here is not going to look for or attempt to
> connect IDE devices, because it is now looking for /AHCI/ devices.
>
> At worst, this may break a few existing scripts, but I actually think
> that since the if=ide,file=... shorthand never worked to begin with,
> the impact might actually be minimal.
>
> But since the legacy IDE interface of the ICH9 is as of yet unemulated,
> the if=ide drives don't have a reasonable place to go yet. I am also
> not sure what a reasonable way to handle people specifying BOTH
> if=ide and if=ahci drives would be.

We've been through IF_AHCI before, more than once, but that was before
you got involved :)

The problem at hand is that "-drive if=ide" and its sugared forms -hda,
-hdb, -cdrom, ... don't work with Q35.

You provide a solution for the sugared forms, you leave the problem
unsolved for the unsugared form, and you add a new problem: -drive
if=ahci doesn't work with i440FX.  Progress, sort of.

Let's take a step back, and recap previous discussion.  There are two
defensible point of views, in my opinion.

One is that IDE and AHCI should be separate interface types, just like
IDE and SCSI are.

Attempts to define an if=X drive with a board that doesn't provide a
controller for X should fail[*].  Only onboard controllers matter,
add-ons plugged in with -device don't.  An i440FX board provides only
IDE.  A Q35 board provides only AHCI, not IDE.  If we implement an
ich9-ahci legacy mode, and switch it on, then it provides only IDE, not
AHCI.  Or maybe both, depending on how we do it.

The other point of view is that IDE and AHCI are no more different than
the different kinds of SCSI HBAs.  This is certainly true from a qdev
point of view: just like SCSI devices can connect to any SCSI qbus,
regardless of the HBA providing it, so can IDE devices connect to any
IDE qbus, regardless of the controller providing it.

There's a wrinkle: the mapping between index to (bus, unit).  This
mapping is ABI.  The current mapping makes sense for the first
generation of controllers: PATA (two devices per bus, thus
if_max_devs[IF_IDE] = 2), and 8-bit SCSI (seven per bus, thus
if_max_devs[IF_SCSI = 7).

The mapping is silly for newer SCSI HBAs.  Commit 622b520f tried to make
it less silly, but had to be reverted in 27d6bf4 because the silliness
was ABI.

The mapping is also silly for ich9-ahci.  You side-step that silliness
only, by adding a new interface type for it.  But shouldn't we add a
number of SCSI interface types then, too?  Where does that end?

Can we do better?  I think we can, by making this part of the ABI
board-specific.  The general form of the mapping remains

    (bus, unit) = (index / N, index % N)

but N now depends on board and interface type, not just the latter.

If the board connects if=scsi to an lsi53c895a, then N = 7.

If the board connects if=ide to an piix3-ide, then N = 2.

If the board connects if=ide to an ich9-ahci, then N = 1.

I trust you get the idea :)


[*] Currently, they're silently ignored with most boards for most X, but
I regard that as implementation defect.