Callers of dma_blk_io have no way to pass extra data to the DMAIOFunc,
because the original callback and opaque are gone by the time DMAIOFunc
is called. On the other hand, the BlockBackend is usually derived
from those extra data that you could pass to the DMAIOFunc (in the
next patch, that would be the SCSIRequest).
So change DMAIOFunc's prototype, decoupling it from blk_aio_readv
and blk_aio_writev's. The new prototype loses the BlockBackend
and gains an extra opaque value which, in the case of dma_blk_readv
and dma_blk_writev, is of course used for the BlockBackend.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Sector-based blk_aio_readv() and blk_aio_writev() should die; switch
to byte-based blk_aio_preadv() and blk_aio_pwritev() instead.
The patch had to touch multiple files at once, because dma_blk_io()
takes pointers to the functions, and ide_issue_trim() piggybacks on
the same interface (while ignoring offset under the hood).
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Restart of ATAPI DMA used to be unreachable, because the request to do
so wasn't indicated in bus->error_status due to the lack of spare bits, and
ide_restart_bh() would return early doing nothing.
This patch makes use of the observation that not all bit combinations were
possible in ->error_status. In particular, IDE_RETRY_READ only made sense
together with IDE_RETRY_DMA or IDE_RETRY_PIO. This allows to re-use
IDE_RETRY_READ alone as an indicator of ATAPI DMA restart request.
To makes things more uniform, ATAPI DMA gets its own value for ->dma_cmd.
As a means against confusion, macros are added to test the state of
->error_status.
The patch fixes the restart of both in-flight and pending ATAPI DMA,
following the scheme similar to that of IDE DMA.
[Including a fixup patch:
Message-id: 1460465594-15777-1-git-send-email-pbutsykin@virtuozzo.com
--js]
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1459924806-306-4-git-send-email-den@openvz.org
Signed-off-by: John Snow <jsnow@redhat.com>
If the migration occurs after the IDE DMA has been set up but before it
has been initiated, the state gets lost upon save/restore. Specifically,
->dma_cb callback gets cleared, so, when the guest eventually starts bus
mastering, the DMA never completes, causing the guest to time out the
operation.
OTOH all the infrastructure is already in place to restart the DMA if
the migration happens while the DMA is in progress.
So reuse that infrastructure, by setting bus->error_status based on
->dma_cmd in pre_save if ->dma_cb callback is already set but DMAING is
clear. This will indicate the need for restart and make sure ->dma_cb
is restored in ide_restart_bh(); howeover since DMAING is clear the state
upon restore will be exactly "ready for DMA" as before the save.
Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
Reviewed-by: Roman Kagan <rkagan@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1459924806-306-2-git-send-email-den@openvz.org
Signed-off-by: John Snow <jsnow@redhat.com>
Move declarations out of qemu-common.h for functions declared in
utils/ files: e.g. include/qemu/path.h for utils/path.c.
Move inline functions out of qemu-common.h and into new files (e.g.
include/qemu/bcd.h)
Signed-off-by: Veronia Bahaa <veroniabahaa@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
This patch replaces get_ticks_per_sec() calls with the macro
NANOSECONDS_PER_SECOND. Also, as there are no callers, get_ticks_per_sec()
is then removed. This replacement improves the readability and
understandability of code.
For example,
timer_mod(fdctrl->result_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (get_ticks_per_sec() / 50));
NANOSECONDS_PER_SECOND makes it obvious that qemu_clock_get_ns
matches the unit of the expression on the right side of the plus.
Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1453225191-11871-7-git-send-email-jsnow@redhat.com
Split apart the ide_transfer_stop function into two versions: one that
interrupts and one that doesn't. The one that doesn't can be used to
halt any PIO transfers that are in the DRQ phase. It will not halt
any PIO transfers that are currently in the process of buffering data
for the guest to read.
Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[Renamed 'etf' to 'end_transfer_func' --js]
Message-id: 1453225191-11871-6-git-send-email-jsnow@redhat.com
Target the drain for just one device.
Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1453225191-11871-5-git-send-email-jsnow@redhat.com
Buffered DMA cancellation was added to ATAPI devices and implemented
for the BMDMA HBA. Move the code over to common IDE code and allow
it to be used for any HBA.
Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1453225191-11871-4-git-send-email-jsnow@redhat.com
Shuffle the reset function upwards.
Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1453225191-11871-3-git-send-email-jsnow@redhat.com
This command is meant for ATAPI devices only, prohibit acknowledging it with
a command aborted response when an IDE device is busy.
Signed-off-by: John Snow <jsnow@redhat.com>
Reported-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1453225191-11871-2-git-send-email-jsnow@redhat.com
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1453832250-766-17-git-send-email-peter.maydell@linaro.org
this patch adds a new aio readv compatible function which copies
all data through a bounce buffer. These buffered requests can be
flagged as orphaned which means that their original callback has
already been invoked and the request has just not been completed
by the backend storage. The bounce buffer guarantees that guest
memory corruption is avoided when such a orphaned request is
completed by the backend at a later stage.
This trick only works for read requests as a write request completed
at a later stage might corrupt data as there is no way to control
if and what data has already been written to the storage.
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Fam Zheng <famz@redhat.com>
Message-id: 1447345846-15624-4-git-send-email-pl@kamp.de
Signed-off-by: John Snow <jsnow@redhat.com>
This makes the purpose of the function clearer: it is not about the
version of QEMU that's running, but the version string exposed in the
emulated hardware.
Cc: Andrzej Zaborowski <balrogg@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: John Snow <jsnow@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Message-Id: <1446233769-7892-3-git-send-email-ehabkost@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We're supposed to abort on transfers like this, unless we fill
Word 125 of our IDENTIFY data with a default transfer size, which
we don't currently do.
This is an ATA error, not a SCSI/ATAPI one.
See ATA8-ACS3 sections 7.17.6.49 or 7.21.5.
If we don't do this, QEMU will loop forever trying to transfer
zero bytes, which isn't particularly useful.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1442253685-23349-2-git-send-email-jsnow@redhat.com
We're a little too lenient with what we'll let an ATAPI drive handle.
Clamp down on the IDE command execution table to remove CD_OK permissions
from commands that are not and have never been ATAPI commands.
For ATAPI command validity, please see:
- ATA4 Section 6.5 ("PACKET Command feature set")
- ATA8/ACS Section 4.3 ("The PACKET feature set")
- ACS3 Section 4.3 ("The PACKET feature set")
ACS3 has a historical command validity table in Table B.4
("Historical Command Assignments") that can be referenced to find when
a command was introduced, deprecated, obsoleted, etc.
The only reference for ATAPI command validity is by checking that
version's PACKET feature set section.
ATAPI was introduced by T13 into ATA4, all commands retired prior to ATA4
therefore are assumed to have never been ATAPI commands.
Mandatory commands, as listed in ATA8-ACS3, are:
- DEVICE RESET
- EXECUTE DEVICE DIAGNOSTIC
- IDENTIFY DEVICE
- IDENTIFY PACKET DEVICE
- NOP
- PACKET
- READ SECTOR(S)
- SET FEATURES
Optional commands as listed in ATA8-ACS3, are:
- FLUSH CACHE
- READ LOG DMA EXT
- READ LOG EXT
- WRITE LOG DMA EXT
- WRITE LOG EXT
All other commands are illegal to send to an ATAPI device and should
be rejected by the device.
CD_OK removal justifications:
0x06 WIN_DSM Defined in ACS2. Not valid for ATAPI.
0x21 WIN_READ_ONCE Retired in ATA5. Not ATAPI in ATA4.
0x94 WIN_STANDBYNOW2 Retired in ATA4. Did not coexist with ATAPI.
0x95 WIN_IDLEIMMEDIATE2 Retired in ATA4. Did not coexist with ATAPI.
0x96 WIN_STANDBY2 Retired in ATA4. Did not coexist with ATAPI.
0x97 WIN_SETIDLE2 Retired in ATA4. Did not coexist with ATAPI.
0x98 WIN_CHECKPOWERMODE2 Retired in ATA4. Did not coexist with ATAPI.
0x99 WIN_SLEEPNOW2 Retired in ATA4. Did not coexist with ATAPI.
0xE0 WIN_STANDBYNOW1 Not part of ATAPI in ATA4, ACS or ACS3.
0xE1 WIN_IDLEIMMDIATE Not part of ATAPI in ATA4, ACS or ACS3.
0xE2 WIN_STANDBY Not part of ATAPI in ATA4, ACS or ACS3.
0xE3 WIN_SETIDLE1 Not part of ATAPI in ATA4, ACS or ACS3.
0xE4 WIN_CHECKPOWERMODE1 Not part of ATAPI in ATA4, ACS or ACS3.
0xE5 WIN_SLEEPNOW1 Not part of ATAPI in ATA4, ACS or ACS3.
0xF8 WIN_READ_NATIVE_MAX Obsoleted in ACS3. Not ATAPI in ATA4 or ACS.
This patch fixes a divide by zero fault that can be caused by sending
the WIN_READ_NATIVE_MAX command to an ATAPI drive, which causes it to
attempt to use zeroed CHS values to perform sector arithmetic.
Reported-by: Qinghao Tang <luodalongde@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 1441816082-21031-1-git-send-email-jsnow@redhat.com
CC: qemu-stable@nongnu.org
IDEState's io_buffer_offset was originally added to keep track of offsets
in AHCI rather exclusively, but it was added to IDEState instead of an
AHCI-specific structure.
AHCI fakes all PIO transfers using DMA and a scatter-gather list. When
the core or atapi layers invoke HBA-specific mechanisms for transfers,
they do not always know that it is being backed by DMA or a sglist, so
this offset is not always updated by the HBA code everywhere.
If we modify it in dma_buf_commit, however, any HBA that needs to use
this offset to manage operating on only part of a sglist will have
access to it.
This will fix ATAPI PIO transfers performed through the AHCI HBA,
which were previously not modifying this value appropriately.
This will fix ATAPI PIO transfers larger than one sector.
Reported-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>
Message-id: 1440546331-29087-2-git-send-email-jsnow@redhat.com
CC: qemu-stable@nongnu.org
This is additional hardening against an end_transfer_func that fails to
clear the DRQ status bit. The bit must be unset as soon as the PIO
transfer has completed, so it's better to do this in a central place
instead of duplicating the code in all commands (and forgetting it in
some).
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
If the end_transfer_func of a command is called because enough data has
been read or written for the current PIO transfer, and it fails to
correctly call the command completion functions, the DRQ bit in the
status register and s->end_transfer_func may remain set. This allows the
guest to access further bytes in s->io_buffer beyond s->data_end, and
eventually overflowing the io_buffer.
One case where this currently happens is emulation of the ATAPI command
START STOP UNIT.
This patch fixes the problem by adding explicit array bounds checks
before accessing the buffer instead of relying on end_transfer_func to
function correctly.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Handle NCQ failures for cases where we want to halt the VM on IO errors.
Upon a VM state change, retry the halted NCQ commands.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-7-git-send-email-jsnow@redhat.com
prepare_buf should not always grab as many descriptors
as it can, sometimes it should self-limit.
For example, an NCQ transfer of 1 sector with a PRDT that
describes 4GiB of data should not copy 4GiB of data, it
should just transfer that first 512 bytes.
PIO is not affected, because the dma_buf_rw dma helpers
already have a byte limit built-in to them, but DMA/NCQ
will exhaust the entire list regardless of requested size.
AHCI 1.3 specifies in section 6.1.6 Command List Underflow that
NCQ is not required to detect underflow conditions. Non-NCQ
pathways signal underflow by writing to the PRDBC field, which
will already occur by writing the actual transferred byte count
to the PRDBC, signaling the underflow.
Our NCQ pathways aren't required to detect underflow, but since our DMA
backend uses the size of the PRDT to determine the size of the transer,
if our PRDT is bigger than the transaction (the underflow condition) it
doesn't cost us anything to detect it and truncate the PRDT.
This is a recoverable error and is not signaled to the guest, in either
NCQ or normal DMA cases.
For BMDMA, the existing pathways should see no guest-visible difference,
but any bytes described in the overage will no longer be transferred
before indicating to the guest that there was an underflow.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-2-git-send-email-jsnow@redhat.com
We create optional sections with this patch. But we already have
optional subsections. Instead of having two mechanism that do the
same, we can just generalize it.
For subsections we just change:
- Add a needed function to VMStateDescription
- Remove VMStateSubsection (after removal of the needed function
it is just a VMStateDescription)
- Adjust the whole tree, moving the needed function to the corresponding
VMStateDescription
Signed-off-by: Juan Quintela <quintela@redhat.com>
IDE PIO data must be written, for example, at 0x1f0. You cannot
do word or dword writes to 0x1f1..0x1f3 to access the data register.
Adjust the ide_portio_list accordingly.
Cc: John Snow <jsnow@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Similar to the cmd_write_pio fix, update the nsector count and
ide sector before we invoke ide_transfer_start.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Andreas Färber <afaerber@suse.de>
Message-id: 1426811056-2202-3-git-send-email-jsnow@redhat.com
We need to adjust the sector being written to
prior to calling ide_transfer_start, otherwise
we'll write to the same sector again.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Tested-by: Andreas Färber <afaerber@suse.de>
Message-id: 1426811056-2202-2-git-send-email-jsnow@redhat.com
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-13-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Resetting the io_buffer_index to 0 is commonized,
with the exception of the case within ide_atapi_cmd_reply,
where we need to reset this index to 0 prior to the
ide_atapi_cmd_reply_end call.
Note that not all calls to ide_atapi_cmd_reply_end
expect the index to be 0, so setting it there is
not appropriate.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-12-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This only breaks backwards migration compatibility if the bus is in
an error state. It is in principle possible to avoid this by making
two subsections (one for version 1, and one for version 2, but with
the same name) with different "_needed" callbacks. The v1 callback would
return true if error_status != 0 and the bus is PATA; the v2 callback
would return true if error_status != 0 and the bus is AHCI.
Forward migration keeps working.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-11-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This moves more common restarting logic to the core IDE code.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-10-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Start moving the initial state of the current request to IDEBus, so that
AHCI can use it. The set_unit callback is not used anymore once this is
done.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-9-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
With restarts now handled by ide_restart_cb and
the IDEDMAOps.restart_dma() member, remove the old
restart_cb callback.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-8-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
With BMDMA specific excised from the restart functions,
create a HBA-agnostic restart callback to be shared
between the different HBAs.
Change the callback registered with the vmstate_change
handler to always point to ide_restart_cb instead of
relying on the IDEDMAOps.restart_cb() member.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-7-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Pass the containing IDEBus to the restart_cb instead
of the more specific BMDMAState child.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-6-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
A helper is added that registers the IDEDMAOp .restart_cb()
via qemu_add_vm_change_state_handler instead of requiring
each HBA to register the callback themselves.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1424708286-16483-4-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
If a migration happens just after the guest has kicked
off an ATAPI command and kicked off DMA, we lose the atapi_dma
flag, and the destination tries to complete the command as PIO
rather than DMA. This upsets Linux; modern libata based kernels
stumble and recover OK, older kernels end up passing bad data
to userspace.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The other callers to blk_set_enable_write_cache() in this file
already check for s->blk == NULL.
Signed-off-by: Don Slutz <dslutz@verizon.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1416259239-13281-1-git-send-email-dslutz@verizon.com
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This impacts both BMDMA and AHCI HBA interfaces for IDE.
Currently, we confuse the difference between a PRDT having
"0 bytes" and a PRDT having "0 complete sectors."
When we receive an incomplete sector, inconsistent error checking
leads to an infinite loop wherein the call succeeds, but it
didn't give us enough bytes -- leading us to re-call the
DMA chain over and over again. This leads to, in the BMDMA case,
leaked memory for short PRDTs, and infinite loops and resource
usage in the AHCI case.
The .prepare_buf() callback is reworked to return the number of
bytes that it successfully prepared. 0 is a valid, non-error
answer that means the table was empty and described no bytes.
-1 indicates an error.
Our current implementation uses the io_buffer in IDEState to
ultimately describe the size of a prepared scatter-gather list.
Even though the AHCI PRDT/SGList can be as large as 256GiB, the
AHCI command header limits transactions to just 4GiB. ATA8-ACS3,
however, defines the largest transaction to be an LBA48 command
that transfers 65,536 sectors. With a 512 byte sector size, this
is just 32MiB.
Since our current state structures use the int type to describe
the size of the buffer, and this state is migrated as int32, we
are limited to describing 2GiB buffer sizes unless we change the
migration protocol.
For this reason, this patch begins to unify the assertions in the
IDE pathways that the scatter-gather list provided by either the
AHCI PRDT or the PCI BMDMA PRDs can only describe, at a maximum,
2GiB. This should be resilient enough unless we need a sector
size that exceeds 32KiB.
Further, the likelihood of any guest operating system actually
attempting to transfer this much data in a single operation is
very slim.
To this end, the IDEState variables have been updated to more
explicitly clarify our maximum supported size. Callers to the
prepare_buf callback have been reworked to understand the new
return code, and all versions of the prepare_buf callback have
been adjusted accordingly.
Lastly, the ahci_populate_sglist helper, relied upon by the
AHCI implementation of .prepare_buf() as well as the PCI
implementation of the callback have had overflow assertions
added to help make clear the reasonings behind the various
type changes.
[Added %d -> %"PRId64" fix John sent because off_pos changed from int to
int64_t.
--Stefan]
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1414785819-26209-4-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Currently, for emulated PIO transfers through the AHCI device,
any attempt made to request more than a single sector's worth
of data will result in the same sector being transferred over
and over.
For example, if we request 8 sectors via PIO READ SECTORS, the
AHCI device will give us the same sector eight times.
This patch adds offset tracking into the PIO pathways so that
we can fulfill these requests appropriately.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1414785819-26209-2-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Currently, DMA read/write operations neglect to update
the byte count after a successful transfer like ATAPI
DMA read or PIO read/write operations do.
We correct this oversight by adding another callback into
the IDEDMAOps structure. The commit callback is called
whenever we are cleaning up a scatter-gather list.
AHCI can register this callback in order to update post-
transfer information such as byte count updates.
We use this callback in AHCI to consolidate where we delete
the SGlist as generated from the PRDT, as well as update the
byte count after the transfer is complete.
The QEMUSGList structure has an init flag added to it in order
to make qemu_sglist_destroy a nop if it is called when
there is no sglist, which simplifies cleanup and error paths.
This patch fixes several AHCI problems, notably Non-NCQ modes
of operation for Windows 7 as well as Hibernate support for Windows 7.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 1412204151-18117-3-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Add a BlockBackend member to TrimAIOCB, so ide_issue_trim_cb() can use
blk_aio_discard() instead of bdrv_aio_discard().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Device models should access their block backends only through the
block-backend.h API. Convert them, and drop direct includes of
inappropriate headers.
Just four uses of BlockDriverState are left:
* The Xen paravirtual block device backend (xen_disk.c) opens images
itself when set up via xenbus, bypassing blockdev.c. I figure it
should go through qmp_blockdev_add() instead.
* Device model "usb-storage" prompts for keys. No other device model
does, and this one probably shouldn't do it, either.
* ide_issue_trim_cb() uses bdrv_aio_discard() instead of
blk_aio_discard() because it fishes its backend out of a BlockAIOCB,
which has only the BlockDriverState.
* PC87312State has an unused BlockDriverState[] member.
The next two commits take care of the latter two.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
I'll use it with block backends shortly, and the name is going to fit
badly there. It's a block layer thing anyway, not just a block driver
thing.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
I'll use BlockDriverAIOCB with block backends shortly, and the name is
going to fit badly there. It's a block layer thing anyway, not just a
block driver thing.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Instead of duplicating the logic for the if_ide
(bus,unit) mappings, rely on the blockdev layer
for managing those mappings for us, and use the
drive_get_by_index call instead.
This allows ide_drive_get to work for AHCI HBAs
as well, and can be used in the Q35 initialization.
Lastly, change the nature of the argument to
ide_drive_get so that represents the number of
total drives we can support, and not the total
number of buses. This will prevent array overflows
if the units-per-default-bus property ever needs
to be adjusted for compatibility reasons.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 1412187569-23452-5-git-send-email-jsnow@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
