All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-block@nongnu.org
Cc: kwolf@redhat.com, pbonzini@redhat.com, qemu-devel@nongnu.org,
	stefanha@redhat.com
Subject: Re: [Qemu-devel] [PATCH v2 0/6] ide: fix atapi software reset
Date: Tue, 26 Jan 2016 15:39:14 -0500	[thread overview]
Message-ID: <56A7D972.3050003@redhat.com> (raw)
In-Reply-To: <1453225191-11871-1-git-send-email-jsnow@redhat.com>

Ping

(I know I could just send a PR, but in this case I'd like someone to
look over it. I've gone over pretty terrible detail to convince myself
this won't break anything IDE-wise, so if it looks sane block-wise, I'd
be satisfied with an ACK.)

On 01/19/2016 12:39 PM, John Snow wrote:
> The ATAPI software reset function is implemented somewhat lackadaisically.
> 
> Firstly, it is valid only for ATAPI drives - not HDs. If a HD should
> receive this command while BSY, it should be ignored like any other
> command instead of aborted. A non-BSY HD is free to abort the command
> in the usual fashion to indicate it doesn't understand or doesn't support
> that command.
> 
> Second, for drives that should accept a software reset, they should not
> "forget" about all pending AIO during the reset. Since a software reset
> resets the DRQ and BSY flags, it is possible to 'stack' multiple
> concurrent reads using DMA and alternately chaining software reset and
> DMA reads. We mustn't reset BSY/DRQ until we are confident that we
> have canceled existing AIO.
> 
> Third, the existing software reset routine does not perform a very
> rigorous reset.
> 
> This series corrects this by:
> 
> (1) Correcting ide_exec_cmd to correctly ignore, not abort, software
>     reset commands for ide-hd devices that are busy executing a command.
> 
> (2) Improving the software reset routine to cancel buffered DMA, then
>     fall back to synchronously waiting for any pending DMA to finish
>     before returning, insuring that the reset completes sanely.
> 
> (3) Use existing reset routines to comprehensively reset the device.
> 
> Reported-by: Kevin Wolf <kwolf@redhat.com>
> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> ________________________________________________________________________________
> 
> For convenience, this branch is available at:
> https://github.com/jnsnow/qemu.git branch ide-reset-fix
> https://github.com/jnsnow/qemu/tree/ide-reset-fix
> 
> This version is tagged ide-reset-fix-v2:
> https://github.com/jnsnow/qemu/releases/tag/ide-reset-fix-v2
> 
> John Snow (6):
>   ide: Prohibit RESET on IDE drives
>   ide: code motion
>   ide: move buffered DMA cancel to core
>   ide: replace blk_drain_all by blk_drain
>   ide: Add silent DRQ cancellation
>   ide: fix device_reset to not ignore pending AIO
> 
>  hw/ide/core.c     | 215 ++++++++++++++++++++++++++++++++++++------------------
>  hw/ide/internal.h |   1 +
>  hw/ide/pci.c      |  36 +--------
>  3 files changed, 144 insertions(+), 108 deletions(-)
> 

  parent reply	other threads:[~2016-01-26 20:39 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-19 17:39 [Qemu-devel] [PATCH v2 0/6] ide: fix atapi software reset John Snow
2016-01-19 17:39 ` [Qemu-devel] [PATCH v2 1/6] ide: Prohibit RESET on IDE drives John Snow
2016-01-19 17:39 ` [Qemu-devel] [PATCH v2 2/6] ide: code motion John Snow
2016-01-19 17:39 ` [Qemu-devel] [PATCH v2 3/6] ide: move buffered DMA cancel to core John Snow
2016-01-19 17:39 ` [Qemu-devel] [PATCH v2 4/6] ide: replace blk_drain_all by blk_drain John Snow
2016-01-19 17:39 ` [Qemu-devel] [PATCH v2 5/6] ide: Add silent DRQ cancellation John Snow
2016-02-08 16:09   ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2016-02-08 17:08     ` John Snow
2016-02-09 13:21       ` Stefan Hajnoczi
2016-01-19 17:39 ` [Qemu-devel] [PATCH v2 6/6] ide: fix device_reset to not ignore pending AIO John Snow
2016-01-26 20:39 ` John Snow [this message]
2016-02-08 16:10 ` [Qemu-devel] [Qemu-block] [PATCH v2 0/6] ide: fix atapi software reset Stefan Hajnoczi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A7D972.3050003@redhat.com \
    --to=jsnow@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.