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

2020-07-06 Thread Peter Maydell
On Mon, 6 Jul 2020 at 06:52, Markus Armbruster  wrote:
> This is TYPE_SD_CARD's ("sd-card").  What exactly does that device
> model?

It is I think supposed to be an SD card. The modelling
may well be odd -- it's a qomification of a pre-existing
abstraction layer that predates QOM and qbus.

> If it's the sd-card, then the modelling is odd.  A physical SD card gets
> plugged and unplugged as a whole.  This model can't.  Instead, you
> change "media".  Isn't the SD card the medium?

I suspect this is because the requirement to change SD
card contents existed and was implemented via changing
the block backend before the concept of a hot-pluggable
QOM object even existed. So now we have something we
wish to maintain back-compat for (in terms of monitor
commands to change/eject doing what they've always done),
some SD controllers which haven't been entirely QOMified
(I see Philippe just posted a series to do that cleanup,
which is great), and some parts of the code which have been
updated but by people (ie me) who understand the QOM parts
of things but don't have any understanding of the operations
the block layer provides and so converted the device/SD code's
API to the rest of QEMU but left its interaction with the block
layer using the same APIs that the pre-QOM code used.

> The other device models with removable media (IDE & SCSI CD drives,
> floppy drives) model the receptacle for media.  On media change, the
> drive stays put, and only the medium changes, both in the physical world
> and in our virtual world.
>
> Our "sd-card" seems to be an "SD card drive".  But then I wonder what
> the thing at the other end of TYPE_SD_BUS ("sd-bus") actually models.
> Any ideas?

The thing at the other end of the TYPE_SD_BUS is the SD controller
(ie the bit of hardware which presents a registers-and-interrupts
interface to the system on one end and has an SD card slot on the
other end). The link between them has operations like "do command",
"read data", "write data", "get readonly status", which are abstractions
of the h/w operations on the pins between an SD controller
and the SD card. It also has an operation for "tell the controller
I'm actually an inserted card". So the "sd-card" device implements
the logic that in real h/w really is inside the microcontroller
on the SD card (and so is identical for all SD cards regardless
of machine type), and the sd controller device implements the logic
that's in the sd controller chip in the machine proper (which
can vary a lot between machines, from very-simple "software
does all the work and the controller just waves the wires up and
down" to much more sophisticated setups).

thanks
-- PMM



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

2020-07-05 Thread Markus Armbruster
Peter Maydell  writes:

> On Tue, 30 Jun 2020 at 14:39, Philippe Mathieu-Daudé  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?
>
> I have a feeling that also the monitor "change" and "eject"
> commands can remove the backing block device from the SD card
> object.

Correct:

static const BlockDevOps sd_block_ops = {
.change_media_cb = sd_cardchange,
};

This is TYPE_SD_CARD's ("sd-card").  What exactly does that device
model?

If it's the sd-card, then the modelling is odd.  A physical SD card gets
plugged and unplugged as a whole.  This model can't.  Instead, you
change "media".  Isn't the SD card the medium?

The other device models with removable media (IDE & SCSI CD drives,
floppy drives) model the receptacle for media.  On media change, the
drive stays put, and only the medium changes, both in the physical world
and in our virtual world.

Our "sd-card" seems to be an "SD card drive".  But then I wonder what
the thing at the other end of TYPE_SD_BUS ("sd-bus") actually models.
Any ideas?




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

2020-07-05 Thread Philippe Mathieu-Daudé
On 7/5/20 12:26 AM, Philippe Mathieu-Daudé wrote:
> On 7/5/20 12:18 AM, Philippe Mathieu-Daudé wrote:
>> 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é  
>> 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
> 
> Not straight forward at first glance, so probably too late for soft
> freeze.

I looked in backups of my previous computer and found the branch with
your comment addressed... Not sure why I never send the series, but
this explains why I was sure it was already fixed.

> 
> Meanwhile patches 1-8 are reviewed and sufficient to fix
> CVE-2020-13253. The problematic patch is #9, your "Check address is
> in range" suggestion. Patches 11-14 are also reviewed and can go in.
> 
> Peter, can you consider taking them via your ARM queue? If you prefer
> I can prepare a pull request.
> 
> I'll keep working on this during the next dev window.
> 
> Thanks,
> 
> Phil.
> 
>>
>>>
>
>> 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), >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

2020-07-04 Thread Philippe Mathieu-Daudé
On 7/5/20 12:18 AM, Philippe Mathieu-Daudé wrote:
> 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é  
> 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

Not straight forward at first glance, so probably too late for soft
freeze.

Meanwhile patches 1-8 are reviewed and sufficient to fix
CVE-2020-13253. The problematic patch is #9, your "Check address is
in range" suggestion. Patches 11-14 are also reviewed and can go in.

Peter, can you consider taking them via your ARM queue? If you prefer
I can prepare a pull request.

I'll keep working on this during the next dev window.

Thanks,

Phil.

> 
>>

> 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), >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

2020-07-04 Thread Philippe Mathieu-Daudé
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é  
 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), >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

2020-07-04 Thread Philippe Mathieu-Daudé
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é  
>>> 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), >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

2020-07-03 Thread 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é  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), >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

2020-07-03 Thread Philippe Mathieu-Daudé
On 7/3/20 3:23 PM, Peter Maydell wrote:
> On Tue, 30 Jun 2020 at 14:39, Philippe Mathieu-Daudé  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.



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

2020-07-03 Thread Peter Maydell
On Tue, 30 Jun 2020 at 14:39, Philippe Mathieu-Daudé  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?

I have a feeling that also the monitor "change" and "eject"
commands can remove the backing block device from the SD card
object.

thanks
-- PMM



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

2020-06-30 Thread Philippe Mathieu-Daudé
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.

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sd/sd.c | 28 
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e5adcc8055..548745614e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -545,18 +545,30 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
 static void sd_reset(DeviceState *dev)
 {
 SDState *sd = SD_CARD(dev);
-uint64_t size;
-uint64_t sect;
 
 trace_sdcard_reset();
 if (sd->blk) {
-blk_get_geometry(sd->blk, );
-} else {
-sect = 0;
-}
-size = sect << 9;
+int64_t size = blk_getlength(sd->blk);
+
+if (size == -ENOMEDIUM) {
+/*
+ * FIXME blk should be set once per device in sd_realize(),
+ * and we shouldn't be checking it in sed_reset(). But this
+ * is how the reparent currently works.
+ */
+char *id = object_get_canonical_path_component(OBJECT(dev));
+
+warn_report("sd-card '%s' created with no drive.",
+id ? id : "unknown");
+g_free(id);
+size = 0;
+}
+assert(size >= 0);
+sd->size = size;
+} else {
+sd->size = 0;
+}
 
-sd->size = size;
 sd->state = sd_idle_state;
 sd->rca = 0x;
 sd_set_ocr(sd);
-- 
2.21.3