[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property |
Date: |
Mon, 23 Mar 2015 13:05:39 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 |
On 23/03/2015 10:10, Markus Armbruster wrote:
> Ugh. sdhci-pci is incorrectly qdevified: it uses drive_get_next() in
> its realize() method. Is it the only device model with this issue?
I think SD devices are the only ones, but all of them have the issue.
Ironically, the qdevified ones call drive_get_next(), while the others
get a BlockBackend from the outside.
> I dislike drive_get_next(), because it makes the unit number implicit in
> the order of calls. Explicit would be easier to understand, and make
> breaking ABI harder. But as long as we do it elsewhere, you get to do
> it here.
On the other hand, drive_get_next() is exactly what the code used to do
and *also* makes breaking ABI harder... as long as we're in hard freeze.
> From 30,000ft, this looks a bit like the floppy case: BB's dev points to
> the block device, and BB's dev_opaque points within the device.
>
> If you descend a bit, it looks a lot more like the usb-storage hack that
> has caused us nothing but grief: two separate device models attaching to
> the same BlockBackend.
>
> What is the usb-storage hack? Device model usb-storage pretends to be a
> block device, but really is a SCSI controller that can serve just one
> SCSI device, which it creates automatically, in its realize() method.
> Since the automatically created device isn't accessible at -device /
> device_add level, we need to stick the drive property for it into
> usb-storage. Before the realize() method creates the SCSI device, it
> carefully detaches the usb-storage device:
>
> /*
> * Hack alert: this pretends to be a block device, but it's really
> * a SCSI bus that can serve only a single device, which it
> * creates automatically. But first it needs to detach from its
> * blockdev, or else scsi_bus_legacy_add_drive() dies when it
> * attaches again.
> *
> * The hack is probably a bad idea.
> */
> blk_detach_dev(blk, &s->dev.qdev);
> s->conf.blk = NULL;
>
> Bad idea, but ABI.
>
> Before we make another bad idea ABI, let's stop and think.
>
> I believe the proper solution for your problem is qdevifying the SD
> card.
The question is whether there is a use for qdevifying the SD card.
Each SD/MMC controller will have exactly zero or one SD cards, but the
hw/sd/sd.c interface already treats "BlockBackend ejected" as "zero SD
cards":
if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable) {
return 0;
}
Unlike SCSI, the SD card code:
1) doesn't need multiplexing (a la scsi-hd/scsi-cd/scsi-generic)
2) doesn't have a bus to talk on (real-world SD cards are just connected
with GPIO pins; hw/sd/sd.c abstracts the bitbanging protocol but still
there is only one device to talk to)
So in the end I think it's easier to treat hw/sd/sd.c as the common code
for all hw/sd/* devices, like e.g. hw/display/vga.c.
> If we can't do that for 2.3, and we absolutely need *something* for 2.3
> (do we?), we should still consider whether that something will get in
> the way of the proper solution.
If you want me to fix the sd.c identity crisis for 2.3, and remove
blk_attach_dev I can do it. It will be a series of patches much like
this one, so this one in particular doesn't get in the way.
The only alternative for 2.3 is reverting the patch for sdhci-pci. I
certainly don't want "-drive if=sd -device sdhci-pci" to become ABI!
Paolo
- [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Paolo Bonzini, 2015/03/21
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Markus Armbruster, 2015/03/23
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property,
Paolo Bonzini <=
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Markus Armbruster, 2015/03/23
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Paolo Bonzini, 2015/03/23
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Peter Crosthwaite, 2015/03/23
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Peter Maydell, 2015/03/23
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Paolo Bonzini, 2015/03/23
- Re: [Qemu-devel] [PATCH for-2.3] sdhci: add "drive" property, Markus Armbruster, 2015/03/24