From: Peter Lieven <pl@kamp.de>
To: Stefan Hajnoczi <stefanha@gmail.com>
Cc: kwolf@redhat.com, pbonzini@redhat.com, jsnow@redhat.com,
qemu-devel@nongnu.org, qemu-block@nongnu.org
Subject: Re: [Qemu-devel] [PATCH 2/2] ide/atapi: partially avoid deadlock if the storage backend is dead
Date: Mon, 07 Sep 2015 16:05:59 +0200 [thread overview]
Message-ID: <55ED99C7.1070402@kamp.de> (raw)
In-Reply-To: <20150907134326.GI9593@stefanha-thinkpad.redhat.com>
Am 07.09.2015 um 15:43 schrieb Stefan Hajnoczi:
> On Sun, Sep 06, 2015 at 11:24:10AM +0200, Peter Lieven wrote:
>>> Taking a step back, what are the semantics of writing !(val &
>>> BM_CMD_START)? Is the device guaranteed to cancel/complete requests
>>> during the register write?
>> I have to check that. John, do you have an idea?
>>
>> Stefan, or do you have a better approach for getting rid of the bdrv_drain_all
>> in this piece of code?
> What's needed is an asynchronous approach that honors the semantics of
> the Start/Stop bit.
>
> From "Programming Interface for Bus Master IDE Controller", Revision 1.0:
>
> "Start/Stop Bus Master: Writing a '1' to this bit enables bus master
> operation of the controller. Bus master operation begins when this bit
> is detected changing from a zero to a one. The controller will transfer
> data between the IDE device and memory only when this bit is set.
> Master operation can be halted by writing a '0' to this bit. All state
> information is lost when a '0' is written; Master mode operation cannot
> be stopped and then resumed. If this bit is reset while bus master
> operation is still active (i.e., the Bus Master IDE Active bit of the
> Bus Master IDE Status register for that IDE channel is set) and the
> drive has not yet finished its data transfer (The Interupt bit in the
> Bus Master IDE Status register for that IDE channel is not set), the bus
> master command is said to be aborted and data transfered from the drive
> may be discarded before being written to system memory. This bit is
> intended to be reset after the data transfer is completed, as indicated
> by either the Bus Master IDE Active bit or the Interrupt bit of the Bus
> Master IDE Status register for that IDE channel being set, or both."
>
> bdrv_aio_cancel() can be used to nudge the request to cancel. The
> completion function is still called at some later point - and it could
> take an arbitrary amount of time before the request finishes.
>
> From the IDE emulation perspective, there are two requirements:
>
> 1. After the Start bit has been cleared, no IDE state changes should
> occur due to the pending request. No interrupts should be raised and
> no registers should change when the request completes
> (success/error/cancelled). Also, the guest should be able to set the
> bit again and submit a new request.
I think my approach should fullfil this requirement already The callback
into the IDE layer is set to NULL.
>
> 2. No guest memory should be touched by a running request after the bit
> is cleared. QEMU needs to go through bounce buffers in all cases
> instead of doing zero copy (mapping the PRDT).
>
> Using bounce buffers should be doable but you need to check the IDE
> code paths (PIO, DMA, and ATAPI) reachable from hw/ide/pci.c.
> There's also a denial-of-service scenario where the guest repeatedly
> submits an I/O requests and then clears the Start/Stop bit. That
> would cause QEMU to build up many pending I/O requests and bounce
> buffers. That needs to be handled, for example by punishing the
> guest by failing news I/O if there are more than 16 cancelled
> requests pending.
This sounds doable. I will look into this.
Peter
prev parent reply other threads:[~2015-09-07 14:06 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-20 8:14 [Qemu-devel] [PATCH 0/2] ide/atapi: partially avoid deadlock if the storage backend is dead Peter Lieven
2015-08-20 8:14 ` [Qemu-devel] [PATCH 1/2] block/io: allow AIOCB without callback Peter Lieven
2015-08-21 6:12 ` Eric Blake
2015-08-31 8:38 ` Peter Lieven
2015-08-20 8:14 ` [Qemu-devel] [PATCH 2/2] ide/atapi: partially avoid deadlock if the storage backend is dead Peter Lieven
2015-08-21 6:13 ` Eric Blake
2015-09-03 16:59 ` Stefan Hajnoczi
2015-09-06 9:24 ` Peter Lieven
2015-09-07 13:43 ` Stefan Hajnoczi
2015-09-07 14:05 ` Peter Lieven [this message]
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=55ED99C7.1070402@kamp.de \
--to=pl@kamp.de \
--cc=jsnow@redhat.com \
--cc=kwolf@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@gmail.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.