From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:59504) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QAgnP-0006yl-Up for qemu-devel@nongnu.org; Fri, 15 Apr 2011 07:02:41 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QAgnO-0005fD-Bm for qemu-devel@nongnu.org; Fri, 15 Apr 2011 07:02:39 -0400 Received: from mx1.redhat.com ([209.132.183.28]:36260) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QAgnO-0005ex-11 for qemu-devel@nongnu.org; Fri, 15 Apr 2011 07:02:38 -0400 Message-ID: <4DA8265C.5050007@redhat.com> Date: Fri, 15 Apr 2011 13:05:00 +0200 From: Kevin Wolf MIME-Version: 1.0 References: In-Reply-To: Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] atapi: Indicate 'medium not ready' to 'medium ready' transition via error List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amit Shah Cc: Stefan Hajnoczi , Jes Sorensen , Juan Quintela , qemu list , Markus Armbruster Am 15.04.2011 11:33, schrieb Amit Shah: > MMC-5 Table F.1 lists errors that can be thrown for the TEST_UNIT_READY > command. Going from medium not ready to medium ready states is > communicated by throwing an error. > > This adds the missing 'tray opened' event that we fail to report to > guests. After doing this, older Linux guests properly revalidate a disc > on the change command. HSM violation errors, which caused Linux guests > to do a soft-reset of the link, also go away: > > ata2.00: exception Emask 0x0 SAct 0x0 SErr 0x0 action 0x6 > sr 1:0:0:0: CDB: Test Unit Ready: 00 00 00 00 00 00 > ata2.00: cmd a0/00:00:00:00:00/00:00:00:00:00/a0 tag 0 > res 01/60:00:00:00:00/00:00:00:00:00/a0 Emask 0x3 (HSM violation) > ata2.00: status: { ERR } > ata2: soft resetting link > ata2.00: configured for MWDMA2 > ata2: EH complete > > Signed-off-by: Amit Shah > --- > hw/ide/core.c | 32 ++++++++++++++++++++++++++------ > 1 files changed, 26 insertions(+), 6 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index f028ddb..5a72420 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -1253,9 +1253,16 @@ static void ide_atapi_cmd(IDEState *s) > if (bdrv_is_inserted(s->bs) && !s->cdrom_changed) { > ide_atapi_cmd_ok(s); > } else { > - s->cdrom_changed = 0; > - ide_atapi_cmd_error(s, SENSE_NOT_READY, > - ASC_MEDIUM_NOT_PRESENT); > + int sense, asc; > + > + sense = SENSE_NOT_READY; > + asc = ASC_MEDIUM_NOT_PRESENT; > + if (bdrv_is_inserted(s->bs) && s->cdrom_changed) { > + s->cdrom_changed = 0; > + sense = SENSE_UNIT_ATTENTION; > + asc = ASC_MEDIUM_MAY_HAVE_CHANGED; > + } > + ide_atapi_cmd_error(s, sense, asc); > } > break; > case GPCMD_MODE_SENSE_6: > @@ -1734,11 +1741,24 @@ static void cdrom_change_cb(void *opaque, int reason) > bdrv_get_geometry(s->bs, &nb_sectors); > s->nb_sectors = nb_sectors; > > - s->sense_key = SENSE_UNIT_ATTENTION; > - s->asc = ASC_MEDIUM_MAY_HAVE_CHANGED; > + /* > + * This function combines three actions a physical cdrom would do to > + * change a disc in a drive: > + * > + * 1. eject the tray, > + * 2. change the disc, > + * 3. close the tray. > + * > + * Guests expect responses from us in the same order. So first, > + * just mark the disc changed, but provide an 'ejected' event to > + * the guest. Later, when the guest invokes a TEST_UNIT_READY > + * command, we will provide with a disc change (UNIT_ATTENTION) > + * event. > + */ > s->cdrom_changed = 1; > s->events.new_media = true; > - ide_set_irq(s->bus); > + > + ide_atapi_cmd_error(s, SENSE_NOT_READY, ASC_MEDIUM_NOT_PRESENT); > } ide_atapi_cmd_error expands to: s->error = sense_key << 4; s->status = READY_STAT | ERR_STAT; s->nsector = (s->nsector & ~7) | ATAPI_INT_REASON_IO | ATAPI_INT_REASON_CD; s->sense_key = sense_key; s->asc = asc; ide_set_irq(s->bus); Are you sure that you can modify the ATA error register here, even though we're not responding to a guest initiated command? I think possibly we're even in the middle of processing an independent command, which would falsely appear to have failed. I'm not even sure that raising an IRQ is correct in cdrom_change_cb. Kevin