[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 09/17] hw/sd/sdcard: Special case the -ENOMEDIUM error
From: |
Philippe Mathieu-Daudé |
Subject: |
Re: [PATCH v7 09/17] hw/sd/sdcard: Special case the -ENOMEDIUM error |
Date: |
Sat, 4 Jul 2020 01:42:37 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 |
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);
Re: [PATCH v7 09/17] hw/sd/sdcard: Special case the -ENOMEDIUM error, Markus Armbruster, 2020/07/06