There are two things to fix here:
The first one is subtle: the PxSACT register in the AHCI HBA has different
semantics from the field it is shadowing, the ACT field in the
Set Device Bits FIS.
In the HBA register, PxSACT acts as a bitfield indicating outstanding
NCQ commands where a set bit indicates a pending NCQ operation. The FIS
field however operates as an RWC register update to PxSACT, where a set
bit indicates a *successfully* completed command.
Correct the FIS semantics. At the same time, move the "clear finished"
action to the SDB FIS generation instead of the register read to mimick
how the other shadow registers work, which always just report the last
reported value from a FIS, and not the most current values which may
not have been reported by a FIS yet.
Lastly and more simply, SATA 3.2 section 13.6.4.2 (and later sections)
all specify that the Interrupt bit for the SDB FIS should always be set
to one for NCQ commands. That's currently the only time we generate this
FIS, so set it on all the time.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-16-git-send-email-jsnow@redhat.com
The Register D2H FIS should copy the current values of
the registers instead of just parroting back the same
values the guest sent back to it.
In this case, the SECTOR COUNT variables are actually
not generally meaningful in terms of standard commands
(See ATA8-AC3 Section 9.2 Normal Outputs), so it actually
probably doesn't matter what we put in here.
Meanwhile, we do need to use the Register update FIS from
the NCQ pathways (in error cases), so getting rid of
references to cur_cmd here is a win for AHCI concurrency.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-14-git-send-email-jsnow@redhat.com
Migrate the NCQ queue. This is solely for the benefit of halted commands,
since anything else should have completed and had any relevant status
flushed to the HBA registers already.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-13-git-send-email-jsnow@redhat.com
cur_cmd is an internal bookmark that points to the
current AHCI Command Header being processed by the
AHCI state machine. With NCQ needing to occasionally
rely on some of the same AHCI helpers, we cannot use
cur_cmd and will need to grab explicit pointers instead.
In an attempt to begin relying on the cur_cmd pointer
less, add a helper to let us specifically get the pointer
to the command header of particular interest.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-12-git-send-email-jsnow@redhat.com
While the rest of the AHCI device can rely on a single bookmarked
pointer for the AHCI Command Header currently being processed, NCQ
is asynchronous and may have many commands in flight simultaneously.
Add a cmdh pointer to the ncq_tfs object and make the sglist prepare
function take an AHCICmdHeader pointer so we can be explicit about
where we'd like to build SGlists from.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-11-git-send-email-jsnow@redhat.com
uint16_t isn't enough to hold the real sector count, since a value of
zero implies a full 64K sectors, so we need a uint32_t here.
We *could* cheat and pretend that this value is 0-based and fit it in
a uint16_t, but I'd rather waste 2 bytes instead of a future dev's
10 minutes when they forget to +1/-1 accordingly somewhere.
See SATA 3.2, section 13.6.4.1 "READ FPDMA QUEUED".
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-9-git-send-email-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
When we add werror=stop or rerror=stop support to NCQ,
we'll want to take a codepath where we don't actually
complete the command, so factor that out into a new routine.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-6-git-send-email-jsnow@redhat.com
Split off execute_ncq_command so that we can call
it separately later if we desire.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-5-git-send-email-jsnow@redhat.com
We already checked this in the handle_cmd phase, so just
change this to an assertion and simplify the error logic.
(Also, fix the switch indent, because checkpatch.pl yelled.)
((Sorry for churn.))
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-4-git-send-email-jsnow@redhat.com
For migration and werror=stop/rerror=stop resume purposes,
it will be convenient to have the command handy inside of
ncq_tfs.
Eventually, we'd like to avoid reading from the FIS entirely
after the initial read, so this is a byte (hah!) sized step
in that direction.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435767578-32743-3-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
Test the NCQ pathways for a simple IO RW test.
Also, test that libqos doesn't explode when
running NCQ commands :)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-16-git-send-email-jsnow@redhat.com
NCQ commands are LBA48 by definition.
See SATA 3.2 13.6.4.1 "READ FPDMA QUEUED", or
SATA 3.2 13.6.5.1 "WRITE FPDMA QUEUED."
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-15-git-send-email-jsnow@redhat.com
NCQ commands have the concept of a "TAG" that they need to set,
but in the AHCI world, it is mandated that the TAG always match
the command slot that you executed the NCQ from.
See AHCI 9.3.1.1.5.2 "Native Queued Commands".
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-14-git-send-email-jsnow@redhat.com
NCQ commands will expect the SDBS interrupt,
and in the normative case, do not expect to see
a D2H Register FIS unless something went wrong.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-13-git-send-email-jsnow@redhat.com
The wait command should check to make sure SACT is clear as well
as the Command Issue register.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-12-git-send-email-jsnow@redhat.com
NCQ frames are generated a little differently than
their non-NCQ cousins. Add support for them.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-11-git-send-email-jsnow@redhat.com
NCQ commands should not / do not update the byte count
in the command header post command, so this field is
meaningless for NCQ tests.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-10-git-send-email-jsnow@redhat.com
If you try to execute an NCQ command before trying to engage with the
device by issuing an IDENTIFY command, the error bits that are part of
the signature will fool the test suite into thinking there was a failure.
Issue IDENTIFY first on "boot", which will clear the signature out of
the registers for us.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-9-git-send-email-jsnow@redhat.com
This value should not be size-corrected, 0 sectors does not imply
1 sector(s). This is just debug information, but it's misleading!
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-8-git-send-email-jsnow@redhat.com
Most of the time, these bits can be safely ignored. For the purposes
of debugging however, it's nice to know that they're not being used.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-7-git-send-email-jsnow@redhat.com
There's no real reason to have it bundled together, and this way
is a little nicer to follow if you have the AHCI spec pulled up.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-6-git-send-email-jsnow@redhat.com
Don't attempt the NCQ transfer if the PRDT we were given is not big
enough to perform the entire transfer.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-5-git-send-email-jsnow@redhat.com
Set some appropriate error bits for NCQ for us.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-4-git-send-email-jsnow@redhat.com
Trivial cleanup that I didn't want to tack-on to anything else.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-3-git-send-email-jsnow@redhat.com
Several fields of the NCQFIS structure are ambiguously named. This patch
clarifies the intended (if unsupported) usage of the NCQ fields to aid
in creating more meaningful debug messages through the NCQ codepaths.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: 1435016308-6150-2-git-send-email-jsnow@redhat.com
Test that we can survive a couple of cycles of running a basic identify
test, some IO, and resetting the HBA. Ensures that we can bring the HBA
back to compliant spec during the lifecycle of the VM.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1434470575-21625-5-git-send-email-jsnow@redhat.com
There's a handful of trivial bugs in the libqos/ahci functions,
squish them together.
- Zero cached pointers after freeing them
- The Command List Buffer is an array of 32x 32 byte structures, not
32x 8 byte pointers -- it's 1MiB, not 256 bytes. Zero it ALL.
- Free the correct command in ahci_pick_cmd.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1434470575-21625-4-git-send-email-jsnow@redhat.com
Test that the FIS delivered after a nondata command has appropriately
updated registers, just as we'd expect a data command to do.
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1434470575-21625-3-git-send-email-jsnow@redhat.com
The only guidance the AHCI specification gives on memory access is:
"Register accesses shall have a maximum size of 64-bits; 64-bit access
must not cross an 8-byte alignment boundary."
I interpret this to mean that aligned or unaligned 1, 2 and 4 byte
accesses should work, as well as aligned 8 byte accesses.
In practice, a real Q35/ICH9 responds to 1, 2, 4 and 8 byte reads
regardless of alignment. Windows 7 can be observed making 1 byte
reads to the middle of 32 bit registers to fetch error codes.
Introduce a wrapper to support unaligned accesses to AHCI.
This wrapper will support aligned 8 byte reads, but will make
no effort to support unaligned 8 byte reads, which although they
will work on real hardware, are not guaranteed to work and do
not appear to be used by either Windows or Linux.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1434470575-21625-2-git-send-email-jsnow@redhat.com
Add display and head properties for input routing to
virtio-input devices, update multiseat documentation.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The section footer changes commit f68945d42b ("Add a protective
section footer") and commit 37fb569c01 ("Disable section footers
on older machine types") broke migration for any non-versioned
machines.
This pinpoints a problem of s390-ccw machines: it needs to
be versioned to be compatible with future changes in common
code data structures such as section footers.
Let's introduce a version scheme for s390-ccw-virtio machines.
We will use the old s390-ccw-virtio name as alias to the latest
version as all existing libvirt XML for the ccw type were expanded
by libvirt to that name.
The only downside of this patch is, that the old alias s390-ccw
will no longer be available as machines can have only one alias,
but it should not really matter.
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Cc: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com>
Cc: Jason J. Herne <jjherne@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Message-Id: <1435742217-62246-1-git-send-email-borntraeger@de.ibm.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Whenever we touch the access control registers, we have to make sure that
the values will make it into kvm. Otherwise the change will simply be lost.
When synchronizing qemu and kvm, a normal KVM_PUT_RUNTIME_STATE does not take
care of these registers. Let's simply trigger a KVM_PUT_FULL_STATE sync,
so the values will directly be written to kvm. The performance overhead can
be ignored and this is much cleaner than manually writing these registers to kvm
via our two supported ways.
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
commit fa92e218df ("s390x/ipl: avoid sign extension") introduced
a regression:
qemu-system-s390x -drive file=image.qcow,format=qcow2
does not boot, the bios states
"No virtio-blk device found!"
adding bootindex=1 does boot.
The reason is that the uint32_t as return value will not do the right
thing for the return -1 (default without bootindex).
The bios itself, will interpret a 64bit -1 as autodetect (but it will
interpret 32bit -1 as ccw device address ff.ff.ffff)
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: qemu-stable@nongnu.org # v2.3.0
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
We need to migrate the revision field as well. No compatibility
concerns as we already introduced migration of ->config_vector in
this release.
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Support the new CCW_CMD_SET_VQ format for virtio-1 devices.
While we're at it, refactor the code a bit and enforce big endian
fields (which had always been required, even for legacy).
Reviewed-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Handle the virtio-ccw revision according to what the guest sets.
When revision 1 is selected, we have a virtio-1 standard device
with byteswapping for the virtio rings.
When a channel gets disabled, we have to revert to the legacy behavior
in case the next user of the device does not negotiate the revision 1
anymore (e.g. the boot firmware uses revision 1, but the operating
system only uses the legacy mode).
Note that revisions > 0 are still disabled.
[CH: assure memory accesses are always BE]
Signed-off-by: Thomas Huth <thuth@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Version: GnuPG v1
iQEcBAABAgAGBQJVlQAPAAoJEJykq7OBq3PIV7EH/iijbZ6q/i1UdinVsF09Dls3
f++CLEb2VUx25KqReDwUDjVsimBeb9jntLUyqHuHAYlCwrrJGcbTPQqw3d2k+MBB
pbWcaFyjGyHb7LOd1JOVRHK0z4QfjVNPu9IjGMsUXP3/JXtqkbIcLwg1UF6p/bzj
NXSaCv5NOSEpJdxCN/wYXCi78G9WfsSF0X1qCOLsJ/+sm8hung21T/o9CTph0Yua
6aiZodnB6vvk/YLPz1/RS37LabiMmgLeUdgbjxYhjBZBUQckmpOExm9gKRzujT/h
wZCuBeySdwqEDF0sLWOB9gDZRoTujStsnwpK6MmD/o0lXiWGbeYBtjakx93EY0c=
=HgNT
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging
# gpg: Signature made Thu Jul 2 10:10:39 2015 BST using RSA key ID 81AB73C8
# gpg: Good signature from "Stefan Hajnoczi <stefanha@redhat.com>"
# gpg: aka "Stefan Hajnoczi <stefanha@gmail.com>"
* remotes/stefanha/tags/block-pull-request:
block: remove redundant check before g_slist_find()
block/nfs: limit maximum readahead size to 1MB
block/iscsi: restore compatiblity with libiscsi 1.9.0
iotests: Use event_wait in wait_ready
qemu-iotests: Add test case for mirror with unmap
qemu-iotests: Make block job methods common
block: Remove bdrv_reset_dirty
block: Fix dirty bitmap in bdrv_co_discard
mirror: Do zero write on target if sectors not allocated
qmp: Add optional bool "unmap" to drive-mirror
block: Add bdrv_get_block_status_above
timer: Use a single definition of NSEC_PER_SEC for the whole codebase
timer: Move NANOSECONDS_PER_SECONDS to timer.h
blockdev: no need to drain+flush in hmp_drive_del
qapi: Rename 'dirty-bitmap' mode to 'incremental'
qcow2: Handle EAGAIN returned from update_refcount
block/iscsi: add support for request timeouts
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
An empty GSList is represented by a NULL pointer, therefore it's a
perfectly valid argument for g_slist_find() and there's no need to
make any additional check.
Signed-off-by: Alberto Garcia <berto@igalia.com>
Message-id: 1435583533-5758-1-git-send-email-berto@igalia.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
a malicious caller could otherwise specify a very
large value via the URI and force libnfs to allocate
a large amount of memory for the readahead buffer.
Cc: qemu-stable@nongnu.org
Signed-off-by: Peter Lieven <pl@kamp.de>
Message-id: 1435317241-25585-1-git-send-email-pl@kamp.de
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
RHEL7 and others are stuck with libiscsi 1.9.0 since there
unfortunately was an ABI breakage after that release.
Signed-off-by: Peter Lieven <pl@kamp.de>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-id: 1435313881-19366-1-git-send-email-pl@kamp.de
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Only poll the specific type of event we are interested in, to avoid
stealing events that should be consumed by someone else.
Suggested-by: John Snow <jsnow@redhat.com>
Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
