qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: Markus Armbruster <armbru@redhat.com>, Michael Tokarev <mjt@tls.msk.ru>
Cc: "Kevin Wolf" <kwolf@redhat.com>,
	agraf@suse.de, qemu-devel <qemu-devel@nongnu.org>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Andreas Färber" <afaerber@suse.de>
Subject: [Qemu-devel] IF_AHCI RFC (Was Re: Are -cdrom/-hda (or -drive if=ide) supposed to work in q35?)
Date: Thu, 14 Aug 2014 16:09:51 -0400	[thread overview]
Message-ID: <53ED178F.1060504@redhat.com> (raw)
In-Reply-To: <87y4x2xzx3.fsf@blackfin.pond.sub.org>



On 06/12/2014 05:03 AM, Markus Armbruster wrote:
>     I've always argued for SATA, because for me if=ide does *not* imply a
>     specific kind of HBA any more than if=scsi does, and the "natural"
>     HBA for Q35 is AHCI in SATA mode.
>
>     Kevin (cc'ed) has argued for a way to make it connect in legacy PATA
>     mode.  I'd be fine with that, as long as it's off by default.
>
>     Patches welcome.
>

I have a design question for fixing this issue.

I wired up the Q35 board to search for and add drives for bus=0 to 
bus=MAX_SATA_PORTS with unit=0 as seen in the blockdev.c dinfo list, as 
queried by using drive_get or drive_get_by_index.

This part works just fine, and -hda and -cdrom work well.

However, from the frontend, -hd[a-d] and -cdrom assume a particular 
index number which is later converted into a bus,unit pairing under 
drive_new via the functions drive_index_to_bus_id and 
drive_index_to_unit_id. The math performed here is a function of the IF 
type we use which is by default IF_IDE. IF_IDE specifies there are two 
units available per bus.

The math for IF_IDE doesn't work for AHCI connected devices, though, 
which do not allow for master/slave configurations -- meaning that we 
actually may wind up only picking up -hda, -hdc and -cdrom, and -hdb and 
-hdd would go unnoticed because we don't have or look for slaves.

I have two immediate thoughts for how to correct this:

(1) The disgusting way that will make you hate me:
Let drive_new do its bad math, but in the q35 initialization, look for 
drives with a unit != 0 and "fix" them in the drive info entries by 
adjusting their bus and unit numbers to be compliant with an AHCI setup. 
This has the least impact, but it looks gross, and could cause problems 
if anybody ever tries to do any other kind of initialization between 
initial option parsing and when we do our chipset initialization.

(2) Add IF_SATA or IF_AHCI where we can specify that the number of units 
per bus is 1, and adjust the code accordingly so things get entered into 
the drive list properly from the start, and picking them up from the Q35 
board initialization is trivial.

The option I want is #2, but I am not familiar enough with the depth or 
breadth of the code to confidently say this is the right choice, so I am 
requesting some feedback on how people feel about this decision.

As an added bonus for #2, this does give us another avenue to 
distinguish between legacy and AHCI interfaces on the Q35 board, and it 
would allow us to specify for Q35 that the default interface is 
IF_AHCI/IF_SATA to make the -hd[a-d], -cdrom shorthands more explicit 
about what they're going to try to do on our board.

      parent reply	other threads:[~2014-08-14 20:10 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-06-10  6:30 [Qemu-devel] Are -cdrom/-hda (or -drive if=ide) supposed to work in q35? Michael Tokarev
2014-06-10  6:34 ` Paolo Bonzini
2014-06-10  8:10   ` Michael Tokarev
2014-06-10  8:14     ` Paolo Bonzini
2014-06-12  9:03     ` Markus Armbruster
2014-08-01 20:10       ` John Snow
2014-08-01 20:17         ` Michael Tokarev
2014-08-05  8:30         ` Kevin Wolf
2014-08-05  9:13           ` Markus Armbruster
2014-08-05  9:30             ` Kevin Wolf
2014-08-05  9:32               ` Michael Tokarev
2014-08-05 11:05                 ` Markus Armbruster
2014-08-05 11:01               ` Markus Armbruster
2014-08-05 21:14           ` John Snow
2014-08-06  9:07             ` Kevin Wolf
2014-08-14 20:09       ` John Snow [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53ED178F.1060504@redhat.com \
    --to=jsnow@redhat.com \
    --cc=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=armbru@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mjt@tls.msk.ru \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).