On 7/5/20 12:10 AM, Philippe Mathieu-Daudé wrote: > 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"); > } > }
Doh I was pretty sure this series was merged: https://www.mail-archive.com/qemu-devel@nongnu.org/msg514645.html Time to respin I guess, addressing your comment... https://www.mail-archive.com/qemu-devel@nongnu.org/msg515866.html > >>> >>>> 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); >> > >
