Re: [PATCH v7 09/17] hw/sd/sdcard: Special case the -ENOMEDIUM error

[Philippe Mathieu-Daudé]

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é <f4bug@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
> 
>> 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);