From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54106) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UuUnf-0006V0-VP for qemu-devel@nongnu.org; Wed, 03 Jul 2013 17:41:21 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UuUne-0001op-OS for qemu-devel@nongnu.org; Wed, 03 Jul 2013 17:41:19 -0400 Received: from mx1.redhat.com ([209.132.183.28]:25215) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UuUne-0001og-Fh for qemu-devel@nongnu.org; Wed, 03 Jul 2013 17:41:18 -0400 Message-ID: <1372887676.2883.216.camel@ul30vt.home> From: Alex Williamson Date: Wed, 03 Jul 2013 15:41:16 -0600 In-Reply-To: <1372065035-19601-13-git-send-email-stefanha@redhat.com> References: <1372065035-19601-1-git-send-email-stefanha@redhat.com> <1372065035-19601-13-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 12/23] ide: Convert FLUSH CACHE to ide_cmd_table handler List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: Kevin Wolf , Anthony Liguori , qemu-devel@nongnu.org On Mon, 2013-06-24 at 11:10 +0200, Stefan Hajnoczi wrote: > From: Kevin Wolf > > Signed-off-by: Kevin Wolf > Signed-off-by: Stefan Hajnoczi > --- > hw/ide/core.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 8789758..83e86aa 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -1184,6 +1184,12 @@ static bool cmd_write_dma(IDEState *s, uint8_t cmd) > return false; > } > > +static bool cmd_flush_cache(IDEState *s, uint8_t cmd) > +{ > + ide_flush_cache(s); > + return false; > +} > + > static bool cmd_read_native_max(IDEState *s, uint8_t cmd) > { > bool lba48 = (cmd == WIN_READ_NATIVE_MAX_EXT); > @@ -1345,8 +1351,8 @@ static const struct { > [WIN_SETIDLE1] = { cmd_nop, ALL_OK }, > [WIN_CHECKPOWERMODE1] = { cmd_check_power_mode, ALL_OK | SET_DSC }, > [WIN_SLEEPNOW1] = { cmd_nop, ALL_OK }, > - [WIN_FLUSH_CACHE] = { NULL, ALL_OK }, > - [WIN_FLUSH_CACHE_EXT] = { NULL, HD_CFA_OK }, > + [WIN_FLUSH_CACHE] = { cmd_flush_cache, ALL_OK }, > + [WIN_FLUSH_CACHE_EXT] = { cmd_flush_cache, HD_CFA_OK }, > [WIN_IDENTIFY] = { cmd_identify, ALL_OK }, > [WIN_SETFEATURES] = { cmd_set_features, ALL_OK | SET_DSC }, > [IBM_SENSE_CONDITION] = { NULL, CFA_OK }, > @@ -1403,10 +1409,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > } > > switch(val) { > - case WIN_FLUSH_CACHE: > - case WIN_FLUSH_CACHE_EXT: > - ide_flush_cache(s); > - break; > case WIN_SEEK: > /* XXX: Check that seek is within bounds */ > s->status = READY_STAT | SEEK_STAT; This also breaks win7 x64 q35 IDE. Note that while this change looks like a no-op, filling in a handler now means that we do: s->status = READY_STAT | BUSY_STAT; before calling the handler and don't clear it on the way out since the function statically returns false. This then introduces the same bug as f68ec837. Thanks, Alex