On 7/4/20 1:42 AM, Philippe Mathieu-Daudé wrote:
> On 7/3/20 5:16 PM, Philippe Mathieu-Daudé wrote:
>> On 7/3/20 3:23 PM, Peter Maydell wrote:
>>> On Tue, 30 Jun 2020 at 14:39, Philippe Mathieu-Daudé <f4...@amsat.org>
>>> wrote:
>>>>
>>>> As we have no interest in the underlying block geometry,
>>>> directly call blk_getlength(). We have to care about machines
>>>> creating SD card with not drive attached (probably incorrect
>>>> API use). Simply emit a warning when such Frankenstein cards
>>>> of zero size are reset.
>>>
>>> Which machines create SD cards without a backing block device?
>>
>> The Aspeed machines:
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg718116.html
Also all boards using:
hw/sd/milkymist-memcard.c:278: /* FIXME use a qdev drive property
instead of drive_get_next() */
hw/sd/pl181.c:506: /* FIXME use a qdev drive property instead of
drive_get_next() */
hw/sd/ssi-sd.c:253: /* FIXME use a qdev drive property instead of
drive_get_next() */
I.e.:
static void pl181_realize(DeviceState *dev, Error **errp)
{
PL181State *s = PL181(dev);
DriveInfo *dinfo;
/* FIXME use a qdev drive property instead of drive_get_next() */
dinfo = drive_get_next(IF_SD);
s->card = sd_init(dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, false);
if (s->card == NULL) {
error_setg(errp, "sd_init failed");
}
}
>>
>>> I have a feeling that also the monitor "change" and "eject"
>>> commands can remove the backing block device from the SD card
>>> object.
>>
>> This is what I wanted to talk about on IRC. This seems wrong to me,
>> we should eject the card and destroy it, and recreate a new card
>> when plugging in another backing block device.
>>
>> Keep the reparenting on the bus layer, not on the card.
>
> I was wrong, the current code is correct:
>
> void sdbus_reparent_card(SDBus *from, SDBus *to)
> {
> SDState *card = get_card(from);
> SDCardClass *sc;
> bool readonly;
>
> /* We directly reparent the card object rather than implementing this
> * as a hotpluggable connection because we don't want to expose SD cards
> * to users as being hotpluggable, and we can get away with it in this
> * limited use case. This could perhaps be implemented more cleanly in
> * future by adding support to the hotplug infrastructure for "device
> * can be hotplugged only via code, not by user".
> */
>
> if (!card) {
> return;
> }
>
> sc = SD_CARD_GET_CLASS(card);
> readonly = sc->get_readonly(card);
>
> sdbus_set_inserted(from, false);
> qdev_set_parent_bus(DEVICE(card), &to->qbus);
> sdbus_set_inserted(to, true);
> sdbus_set_readonly(to, readonly);
> }
>
> What I don't understand is why create a sdcard with no block backend.
>
> Maybe this is old code before the null-co block backend existed? I
> haven't checked the git history yet.
>
> I'll try to restrict sdcard with only block backend and see if
> something break (I doubt) at least it simplifies the code.
> But I need to update the Aspeed machines first.
>
> The problem when not using block backend, is the size is 0,
> so the next patch abort in sd_reset() due to:
>
> static uint64_t sd_addr_to_wpnum(SDState *sd, uint64_t addr)
> {
> assert(addr < sd->size);
>
