* [Qemu-devel] [PATCH 0/2] hw/sd: Fix medium change
@ 2016-01-07 21:03 Max Reitz
2016-01-07 21:03 ` [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status Max Reitz
2016-01-07 21:03 ` [Qemu-devel] [PATCH 2/2] hw/sd: Implement is_tray_open() BlockDevOp Max Reitz
0 siblings, 2 replies; 6+ messages in thread
From: Max Reitz @ 2016-01-07 21:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Markus Armbruster, qemu-stable, Stefan Hajnoczi,
Max Reitz
Commit de2c6c0536c5c5ebb6e0ce7dcfd8fa9476edab52 broke changing mediums
in SD drives; this series fixes that, and the fact that
blockdev-open-tray did not do anything on SD drives so far.
Max Reitz (2):
hw/sd: Add medium insertion status
hw/sd: Implement is_tray_open() BlockDevOp
hw/sd/sd.c | 23 +++++++++++++++++------
1 file changed, 17 insertions(+), 6 deletions(-)
--
2.6.4
^ permalink raw reply [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status
2016-01-07 21:03 [Qemu-devel] [PATCH 0/2] hw/sd: Fix medium change Max Reitz
@ 2016-01-07 21:03 ` Max Reitz
2016-01-07 21:38 ` John Snow
2016-01-07 21:45 ` Peter Maydell
2016-01-07 21:03 ` [Qemu-devel] [PATCH 2/2] hw/sd: Implement is_tray_open() BlockDevOp Max Reitz
1 sibling, 2 replies; 6+ messages in thread
From: Max Reitz @ 2016-01-07 21:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Markus Armbruster, qemu-stable, Stefan Hajnoczi,
Max Reitz
Right now, the change_media_cb (sd_cardchange()) completely ignores its
@load parameter. This means that issuing a blockdev-open-tray command
will actually not have any effect.
Fix this by keeping track of the medium insertion status in the SDState
and updating it in sd_init() and sd_cardchange().
Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/sd/sd.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1a9935c..0751ba2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -114,6 +114,7 @@ struct SDState {
uint8_t *buf;
bool enable;
+ bool medium_inserted;
};
static void sd_set_mode(SDState *sd)
@@ -429,8 +430,10 @@ static void sd_cardchange(void *opaque, bool load)
{
SDState *sd = opaque;
- qemu_set_irq(sd->inserted_cb, blk_is_inserted(sd->blk));
- if (blk_is_inserted(sd->blk)) {
+ sd->medium_inserted = load && blk_is_inserted(sd->blk);
+
+ qemu_set_irq(sd->inserted_cb, sd->medium_inserted);
+ if (sd->medium_inserted) {
sd_reset(sd);
qemu_set_irq(sd->readonly_cb, sd->wp_switch);
}
@@ -497,6 +500,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
/* FIXME ignoring blk_attach_dev() failure is dangerously brittle */
blk_attach_dev(sd->blk, sd);
blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
+ sd->medium_inserted = blk_is_inserted(sd->blk);
}
vmstate_register(NULL, -1, &sd_vmstate, sd);
return sd;
@@ -507,7 +511,7 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
sd->readonly_cb = readonly;
sd->inserted_cb = insert;
qemu_set_irq(readonly, sd->blk ? blk_is_read_only(sd->blk) : 0);
- qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
+ qemu_set_irq(insert, sd->blk ? sd->medium_inserted : 0);
}
static void sd_erase(SDState *sd)
@@ -1349,7 +1353,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
sd_rsp_type_t rtype;
int rsplen;
- if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable) {
+ if (!sd->blk || !sd->medium_inserted || !sd->enable) {
return 0;
}
@@ -1517,7 +1521,7 @@ void sd_write_data(SDState *sd, uint8_t value)
{
int i;
- if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
+ if (!sd->blk || !sd->medium_inserted || !sd->enable)
return;
if (sd->state != sd_receivingdata_state) {
@@ -1643,7 +1647,7 @@ uint8_t sd_read_data(SDState *sd)
uint8_t ret;
int io_len;
- if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
+ if (!sd->blk || !sd->medium_inserted || !sd->enable)
return 0x00;
if (sd->state != sd_sendingdata_state) {
--
2.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [Qemu-devel] [PATCH 2/2] hw/sd: Implement is_tray_open() BlockDevOp
2016-01-07 21:03 [Qemu-devel] [PATCH 0/2] hw/sd: Fix medium change Max Reitz
2016-01-07 21:03 ` [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status Max Reitz
@ 2016-01-07 21:03 ` Max Reitz
1 sibling, 0 replies; 6+ messages in thread
From: Max Reitz @ 2016-01-07 21:03 UTC (permalink / raw)
To: qemu-devel
Cc: Peter Maydell, Markus Armbruster, qemu-stable, Stefan Hajnoczi,
Max Reitz
This makes the change HMP/QMP command on SD devices work again, after it
has been broken in commit de2c6c0536c5c5ebb6e0ce7dcfd8fa9476edab52.
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-stable <qemu-stable@nongnu.org>
Signed-off-by: Max Reitz <mreitz@redhat.com>
---
hw/sd/sd.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0751ba2..7d1e1cb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -439,8 +439,15 @@ static void sd_cardchange(void *opaque, bool load)
}
}
+static bool sd_is_tray_open(void *opaque)
+{
+ SDState *sd = opaque;
+ return !sd->medium_inserted;
+}
+
static const BlockDevOps sd_block_ops = {
.change_media_cb = sd_cardchange,
+ .is_tray_open = sd_is_tray_open,
};
static const VMStateDescription sd_vmstate = {
--
2.6.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status
2016-01-07 21:03 ` [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status Max Reitz
@ 2016-01-07 21:38 ` John Snow
2016-01-07 21:45 ` Peter Maydell
1 sibling, 0 replies; 6+ messages in thread
From: John Snow @ 2016-01-07 21:38 UTC (permalink / raw)
To: Max Reitz, qemu-devel
Cc: Peter Maydell, Markus Armbruster, Stefan Hajnoczi, qemu-stable
On 01/07/2016 04:03 PM, Max Reitz wrote:
> Right now, the change_media_cb (sd_cardchange()) completely ignores its
> @load parameter. This means that issuing a blockdev-open-tray command
> will actually not have any effect.
>
> Fix this by keeping track of the medium insertion status in the SDState
> and updating it in sd_init() and sd_cardchange().
>
> Cc: qemu-stable <qemu-stable@nongnu.org>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> hw/sd/sd.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1a9935c..0751ba2 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -114,6 +114,7 @@ struct SDState {
> uint8_t *buf;
>
> bool enable;
> + bool medium_inserted;
> };
>
> static void sd_set_mode(SDState *sd)
> @@ -429,8 +430,10 @@ static void sd_cardchange(void *opaque, bool load)
> {
> SDState *sd = opaque;
>
> - qemu_set_irq(sd->inserted_cb, blk_is_inserted(sd->blk));
> - if (blk_is_inserted(sd->blk)) {
> + sd->medium_inserted = load && blk_is_inserted(sd->blk);
> +
> + qemu_set_irq(sd->inserted_cb, sd->medium_inserted);
> + if (sd->medium_inserted) {
> sd_reset(sd);
> qemu_set_irq(sd->readonly_cb, sd->wp_switch);
> }
> @@ -497,6 +500,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
> /* FIXME ignoring blk_attach_dev() failure is dangerously brittle */
> blk_attach_dev(sd->blk, sd);
> blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
> + sd->medium_inserted = blk_is_inserted(sd->blk);
> }
> vmstate_register(NULL, -1, &sd_vmstate, sd);
> return sd;
> @@ -507,7 +511,7 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
> sd->readonly_cb = readonly;
> sd->inserted_cb = insert;
> qemu_set_irq(readonly, sd->blk ? blk_is_read_only(sd->blk) : 0);
> - qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
> + qemu_set_irq(insert, sd->blk ? sd->medium_inserted : 0);
> }
>
> static void sd_erase(SDState *sd)
> @@ -1349,7 +1353,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
> sd_rsp_type_t rtype;
> int rsplen;
>
> - if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable) {
> + if (!sd->blk || !sd->medium_inserted || !sd->enable) {
> return 0;
> }
>
> @@ -1517,7 +1521,7 @@ void sd_write_data(SDState *sd, uint8_t value)
> {
> int i;
>
> - if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
> + if (!sd->blk || !sd->medium_inserted || !sd->enable)
> return;
>
> if (sd->state != sd_receivingdata_state) {
> @@ -1643,7 +1647,7 @@ uint8_t sd_read_data(SDState *sd)
> uint8_t ret;
> int io_len;
>
> - if (!sd->blk || !blk_is_inserted(sd->blk) || !sd->enable)
> + if (!sd->blk || !sd->medium_inserted || !sd->enable)
> return 0x00;
>
> if (sd->state != sd_sendingdata_state) {
>
Same thing you did to the floppy. Looks OK.
Reviewed-by: John Snow <jsnow@redhat.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status
2016-01-07 21:03 ` [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status Max Reitz
2016-01-07 21:38 ` John Snow
@ 2016-01-07 21:45 ` Peter Maydell
2016-01-07 22:00 ` Max Reitz
1 sibling, 1 reply; 6+ messages in thread
From: Peter Maydell @ 2016-01-07 21:45 UTC (permalink / raw)
To: Max Reitz
Cc: Markus Armbruster, QEMU Developers, Stefan Hajnoczi, qemu-stable
On 7 January 2016 at 21:03, Max Reitz <mreitz@redhat.com> wrote:
> Right now, the change_media_cb (sd_cardchange()) completely ignores its
> @load parameter. This means that issuing a blockdev-open-tray command
> will actually not have any effect.
>
> Fix this by keeping track of the medium insertion status in the SDState
> and updating it in sd_init() and sd_cardchange().
>
> Cc: qemu-stable <qemu-stable@nongnu.org>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> hw/sd/sd.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1a9935c..0751ba2 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -114,6 +114,7 @@ struct SDState {
> uint8_t *buf;
>
> bool enable;
> + bool medium_inserted;
When would this be different from blk_is_inserted(sd->blk) ?
I don't see why we need a separate flag here rather
than querying the block layer.
If we do need a flag, why doesn't it need to be migrated?
thanks
-- PMM
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status
2016-01-07 21:45 ` Peter Maydell
@ 2016-01-07 22:00 ` Max Reitz
0 siblings, 0 replies; 6+ messages in thread
From: Max Reitz @ 2016-01-07 22:00 UTC (permalink / raw)
To: Peter Maydell
Cc: Markus Armbruster, QEMU Developers, Stefan Hajnoczi, qemu-stable
[-- Attachment #1: Type: text/plain, Size: 1770 bytes --]
On 07.01.2016 22:45, Peter Maydell wrote:
> On 7 January 2016 at 21:03, Max Reitz <mreitz@redhat.com> wrote:
>> Right now, the change_media_cb (sd_cardchange()) completely ignores its
>> @load parameter. This means that issuing a blockdev-open-tray command
>> will actually not have any effect.
>>
>> Fix this by keeping track of the medium insertion status in the SDState
>> and updating it in sd_init() and sd_cardchange().
>>
>> Cc: qemu-stable <qemu-stable@nongnu.org>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>> hw/sd/sd.c | 16 ++++++++++------
>> 1 file changed, 10 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 1a9935c..0751ba2 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -114,6 +114,7 @@ struct SDState {
>> uint8_t *buf;
>>
>> bool enable;
>> + bool medium_inserted;
>
> When would this be different from blk_is_inserted(sd->blk) ?
First, whenever you use a host CD-ROM drive for an SD card. But I highly
doubt this worked before, because the device model is actually never
notified if the medium in the host drive is exchanged.
For any other medium, blk_is_inserted() should generally be always true.
Second, whenever the change_media_cb is called with @load set to false.
This means that the medium is to be unloaded, which was completely
ignored by sd.c so far (see commit message). It just interpreted a call
to sd_cardchange() as a notification that the medium might have changed,
but this is wrong.
> I don't see why we need a separate flag here rather
> than querying the block layer.
Because of the second case.
> If we do need a flag, why doesn't it need to be migrated?
Oh, yes, you're right, it probably should.
Max
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-07 22:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-07 21:03 [Qemu-devel] [PATCH 0/2] hw/sd: Fix medium change Max Reitz
2016-01-07 21:03 ` [Qemu-devel] [PATCH 1/2] hw/sd: Add medium insertion status Max Reitz
2016-01-07 21:38 ` John Snow
2016-01-07 21:45 ` Peter Maydell
2016-01-07 22:00 ` Max Reitz
2016-01-07 21:03 ` [Qemu-devel] [PATCH 2/2] hw/sd: Implement is_tray_open() BlockDevOp Max Reitz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.