These are defined twice, just use one set consistently.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
It is now called only after the set_inactive callback. Put the two together.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Similar to the case removed in commit 69c38b8 (ide/core: Remove explicit
setting of BM_STATUS_INT, 2011-05-19), the only remaining use of
add_status(..., BM_STATUS_INT) is for short PRDs. The flag should
not be raised in this case.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Drop the unused return value and make the callback optional.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Drop the unused return value.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Drop the unused return value and make the callback optional.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Drop the unused return value and make the callback optional.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
In map_page() in hw/ide/ahci.c, replace cpu_physical_memory_map() and
cpu_physical_memory_unmap() with dma_memory_map() and dma_memory_unmap(),
because ahci devices should not access memory directly but via their address
space. Add an AddressSpace parameter to map_page(). In order to call
map_page(), we should pass the AHCIState.as as the AddressSpace argument.
Signed-off-by: Le Tan <tamlokveer@gmail.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
The data byte count(DBC) read from the description information is defined for
bits 21:00. Bits 30:22 are reserved and bit 31 is the Interrupt on Completion
(I) flag.
Completion interrupts are triggered after every transaction instead of on
I-flag in QEMU. tbl_entry_size is a signed integer and improperly reading the
DBC leads to a negative offset that causes sglist allocation to fail.
Signed-off-by: Reza Jelveh <reza.jelveh@tuhh.de>
Reviewed-by: Alexander Graf <agraf@suse.de>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
After previous Peter patch, they are redundant. This way we don't
assign them except when needed. Once there, there were lots of case
where the ".fields" indentation was wrong:
.fields = (VMStateField []) {
and
.fields = (VMStateField []) {
Change all the combinations to:
.fields = (VMStateField[]){
The biggest problem (appart from aesthetics) was that checkpatch complained
when we copy&pasted the code from one place to another.
Signed-off-by: Juan Quintela <quintela@redhat.com>
Acked-by: Alexey Kardashevskiy <aik@ozlabs.ru>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
CVE-2013-4526
Within hw/ide/ahci.c, VARRAY refers to ports which is also loaded. So
we use the old version of ports to read the array but then allow any
value for ports. This can cause the code to overflow.
There's no reason to migrate ports - it never changes.
So just make sure it matches.
Reported-by: Anthony Liguori <anthony@codemonkey.ws>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Add U suffix to avoid shifting left into the sign bit, which
is undefined behaviour.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
Non-PCI AHCI support is broken due to assertion failures when trying
to convert AHCIState to a PCIDevice pointer as AHCIState can have
different container structs. Fix this by using the non-asserting object
cast and checking the returned pointer is not NULL.
The AddressSpace pointer is also being initialized to NULL and causing
dma_memory_map call to fail. Fix this by initializing to
address_space_memory for sysbus instances.
Also correct AHCI_VMSTATE to use the correct container SysbusAHCIState
for sysbus instances.
Signed-off-by: Rob Herring <rob.herring@linaro.org>
Message-id: 1392073373-3295-1-git-send-email-robherring2@gmail.com
[PMM: added linebreaks to fix overlong lines]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
When AHCI executes an asynchronous IDE command, it checked DRDY without
checking either DRQ or BSY. This sometimes caused interrupt to be sent
before command is actually completed.
This resulted in a race condition: if guest then managed to access the
device before command has completed, it would hang waiting for an
interrupt.
This was observed with windows 7 guests.
To fix, check for DRQ or BSY in additiona to DRDY, if set,
the command is asynchronous so delay the interrupt until
asynchronous done callback is invoked.
Reported-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Tested-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
ATM we set AHCI mode on 1st GHC write.
Spec says we should set it on reset.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The category will be used to sort the devices displayed in
the command line help.
Signed-off-by: Marcel Apfelbaum <marcel.a@redhat.com>
Message-id: 1375107465-25767-4-git-send-email-marcel.a@redhat.com
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Introduce type constant and avoid DO_UPCAST().
Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
[AF: Renamed parent field]
Signed-off-by: Andreas Färber <afaerber@suse.de>
Replace direct uses of AHCIPCIState::card with QOM casts and rename it
to parent_obj.
Acked-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Signed-off-by: Andreas Färber <afaerber@suse.de>
AHCI couldn't cope with asynchronous commands that aren't doing DMA, it
simply wouldn't complete them. Due to the bug fixed in commit f68ec837,
FLUSH commands would seem to have completed immediately even if they
were still running on the host. After the commit, they would simply hang
and never unset the BSY bit, rendering AHCI unusable on any OS sending
flushes.
This patch adds another callback for the completion of asynchronous
commands. This is what AHCI really wants to use for its command
completion logic rather than an DMA completion callback.
Cc: qemu-stable@nongnu.org
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
The DMAContext is a simple pointer to an AddressSpace that is now always
already available. Make everyone hold the address space directly,
and clean up the DMA API to use the AddressSpace directly.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
An IDE bus provided by AHCI can only take a single IDE drive. If you add
a drive as slave, qemu used to accept the command line but the device
wouldn't be actually usable. Catch the situation instead and error out.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Headers in include/exec/ are for the deepest innards of QEMU,
they should almost never be included directly.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Many of these should be cleaned up with proper qdev-/QOM-ification.
Right now there are many catch-all headers in include/hw/ARCH depending
on cpu.h, and this makes it necessary to compile these files per-target.
However, fixing this does not belong in these patches.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Jason tested these patches by migrating Windows 7 and Fedora 17 guests
(while under I/O) on both piix with ahci attached and on q35 (which has
a built-in AHCI controller).
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The size of an int depends on the host, so in order to be able to
migrate these fields, make them either int32_t or bool, depending on the
use.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
'dma_status' and 'dma_cb' are written to, but never read.
Remove these fields in preparation for AHCI migration bits.
Signed-off-by: Jason Baron <jbaron@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Since 39bffca203 (qdev: register all
types natively through QEMU Object Model), TypeInfo as used in
the common, non-iterative pattern is no longer amended with information
and should therefore be const.
Fix the documented QOM examples:
sed -i 's/static TypeInfo/static const TypeInfo/g' include/qom/object.h
Since frequently the wrong examples are being copied by contributors of
new devices, fix all types in the tree:
sed -i 's/^static TypeInfo/static const TypeInfo/g' */*.c
sed -i 's/^static TypeInfo/static const TypeInfo/g' */*/*.c
This also avoids to piggy-back these changes onto real functional
changes or other refactorings.
Signed-off-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
target_phys_addr_t is unwieldly, violates the C standard (_t suffixes are
reserved) and its purpose doesn't match the name (most target_phys_addr_t
addresses are not target specific). Replace it with a finger-friendly,
standards conformant hwaddr.
Outstanding patchsets can be fixed up with the command
git rebase -i --exec 'find -name "*.[ch]"
| xargs s/target_phys_addr_t/hwaddr/g' origin
Signed-off-by: Avi Kivity <avi@redhat.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
While testing q35, I found that windows 7 (specifically, windows 7 ultimate
with sp1 x64), wouldn't install because it can't find the cdrom or disk drive.
The failure message is: 'A required cd/dvd device driver is missing. If you
have a driver floppy disk, CD, DVD, or USB flash drive, please insert it now.'
This can also be reproduced on piix by adding an ahci controller, and
observing that windows 7 does not see any devices behind it.
The problem is that when windows issues a HBA reset, qemu does not reset the
individual ports' PxCMD register. Windows 7 then reads back the PxCMD register
and presumably assumes that the ahci controller has already been initialized.
Windows then never sets up the PxIE register to enable interrupts, and thus it
never gets irqs back when it sends ata device inquiry commands.
This change brings qemu into ahci 1.3 specification compliance.
Section 10.4.3 HBA Reset:
"
When GHC.HR is set to '1', GHC.AE, GHC.IE, the IS register, and all port
register fields (except PxFB/PxFBU/PxCLB/PxCLBU) that are not HwInit in the
HBA's register memory space are reset.
"
I've also re-tested Fedora 16 and 17 to verify that they continue to work with
this change.
Signed-off-by: Jason Baron <jbaron@redhat.com>
Acked-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
I noticed that in hw/ide/ahci:ahci_dma_rw_buf() we do not free the sglist. Thus,
I've added a call to qemu_sglist_destroy() to fix this memory leak.
In addition, I've adeed a call in qemu_sglist_destroy() to 0 all of the sglist
fields, in case there is some other codepath that tries to free the sglist.
Signed-off-by: Jason Baron <jbaron@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
While testing q35, which has its cdrom attached to the ahci controller, I found
that the Fedora 17 install would panic on boot. The panic occurs while
squashfs is trying to read from the cdrom. The errors are:
[ 8.622711] SQUASHFS error: xz_dec_run error, data probably corrupt
[ 8.625180] SQUASHFS error: squashfs_read_data failed to read block
0x20be48a
I was also able to produce corrupt data reads using an installed piix based
qemu machine, using 'dd'. I found that the corruptions were only occuring when
then read size was greater than 128k. For example, the following command
results in corrupted reads:
dd if=/dev/sr0 of=/tmp/blah bs=256k iflag=direct
The > 128k size reads exercise a different code path than 128k and below. In
ide_atapi_cmd_read_dma_cb() s->io_buffer_size is capped at 128k. Thus,
ide_atapi_cmd_read_dma_cb() is called a second time when the read is > 128k.
However, ahci_dma_rw_buf() restart the read from offset 0, instead of at 128k.
Thus, resulting in a corrupted read.
To fix this, I've introduced 'io_buffer_offset' field in IDEState to keep
track of the offset. I've also modified ahci_populate_sglist() to take a new
3rd offset argument, so that the sglist is property initialized.
I've tested this patch using 'dd' testing, and Fedora 17 now correctly boots
and installs on q35 with the cdrom ahci controller.
Signed-off-by: Jason Baron <jbaron@redhat.com>
Tested-by: Andreas Färber <afaerber@suse.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The AHCI device can provide both PCI and SysBus AHCI device
emulations. For this reason, it wasn't previously converted to use
the pci_dma_*() helper functions. Now that we have universal DMA
helper functions, this converts AHCI to use them.
The DMAContext is obtained from pci_dma_context() in the PCI case and
set to NULL in the SysBus case (i.e. we assume for now that a SysBus
AHCI has no IOMMU translation).
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
dma-helpers.c contains a number of helper functions for doing
scatter/gather DMA, and various block device related DMA. Currently,
these directly access guest memory using cpu_physical_memory_*(),
assuming no IOMMU translation.
This patch updates this code to use the new universal DMA helper
functions. qemu_sglist_init() now takes a DMAContext * to describe
the DMA address space in which the scatter/gather will take place.
We minimally update the callers qemu_sglist_init() to pass NULL
(i.e. no translation, same as current behaviour). Some of those
callers should pass something else in some cases to allow proper IOMMU
translation in future, but that will be fixed in later patches.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Properly register reset functions via the device class.
CC: Alexander Graf <agraf@suse.de>
Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
As in the SATA and AHCI specifications, a FIS is 5 Dwords of 4 bytes
each, which comes to 20 bytes (decimal), not 0x20.
Signed-off-by: Daniel Verkamp <daniel@drv.nu>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Most MemoryRegionOps already had the const attribute.
This patch adds it to the remaining ones.
Signed-off-by: Stefan Weil <sw@weilnetz.de>
Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
* kwolf/for-anthony:
AHCI: Masking of IRQs actually masks them
sheepdog: fix co_recv coroutine context
AHCI: Fix port reset race
rewrite QEMU_BUILD_BUG_ON
qcow2: Keep unknown header extension when rewriting header
qcow2: Update whole header at once
vpc: Round up image size during fixed image creation
vpc: Add support for Fixed Disk type
iSCSI: add configuration variables for iSCSI
qemu-io: add write -z option for bdrv_co_write_zeroes
qed: add .bdrv_co_write_zeroes() support
qed: replace is_write with flags field
block: perform zero-detection during copy-on-read
block: add .bdrv_co_write_zeroes() interface
cutils: extract buffer_is_zero() from qemu-img.c
Replace device_init() with generalized type_init().
While at it, unify naming convention: type_init([$prefix_]register_types)
Also, type_init() is a function, so add preceding blank line where
necessary and don't put a semicolon after the closing brace.
Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Anthony Liguori <anthony@codemonkey.ws>
Cc: malc <av1474@comtv.ru>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
When masking IRQ lines, we should actually mask them out and not declare
them active anymore. Once we mask them in again, they are allowed to trigger
again.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
bdrv_aio_cancel() can trigger bdrv_aio_flush() which makes all aio
that is currently in flight finish. So what we do is:
port reset
detect ncq in flight
cancel ncq
delete ncq sg list
at which point we have double freed the sg list. Instead, with this
patch we do:
port reset
detect ncq in flight
cancel ncq
check if we are really still in flight
delete ncq sg list
which makes things work and gets rid of the race.
Signed-off-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
