summary refs log tree commit diff stats
path: root/hw/scsi/scsi-bus.c (follow)
Commit message (Collapse)AuthorAgeFilesLines
* system/runstate: add VM state change cb with return valueHaoqian He2025-05-141-1/+1
| | | | | | | | | | | | | | | | | | | | | This patch adds the new VM state change cb type `VMChangeStateHandlerWithRet`, which has return value for `VMChangeStateEntry`. Thus, we can register a new VM state change cb with return value for device. Note that `VMChangeStateHandler` and `VMChangeStateHandlerWithRet` are mutually exclusive and cannot be provided at the same time. This patch is the pre patch for 'vhost-user: return failure if backend crashes when live migration', which makes the live migration aware of the loss of connection with the vhost-user backend and aborts the live migration. Virtio device will use VMChangeStateHandlerWithRet. Signed-off-by: Haoqian He <haoqian.he@smartx.com> Message-Id: <20250416024729.3289157-2-haoqian.he@smartx.com> Tested-by: Lei Yang <leiyang@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
* qom: Make InterfaceInfo[] uses constPhilippe Mathieu-Daudé2025-04-251-1/+1
| | | | | | | | | | | Mechanical change using: $ sed -i -E 's/\(InterfaceInfo.?\[/\(const InterfaceInfo\[/g' \ $(git grep -lE '\(InterfaceInfo.?\[\]\)') Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-Id: <20250424194905.82506-7-philmd@linaro.org>
* qom: Have class_init() take a const data argumentPhilippe Mathieu-Daudé2025-04-251-2/+2
| | | | | | | | | | Mechanical change using gsed, then style manually adapted to pass checkpatch.pl script. Suggested-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20250424194905.82506-4-philmd@linaro.org>
* scsi: introduce requests_lockStefan Hajnoczi2025-03-131-35/+85
| | | | | | | | | | | | | SCSIDevice keeps track of in-flight requests for device reset and Task Management Functions (TMFs). The request list requires protection so that multi-threaded SCSI emulation can be implemented in commits that follow. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20250311132616.1049687-5-stefanha@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* scsi: track per-SCSIRequest AioContextStefan Hajnoczi2025-03-131-0/+1
| | | | | | | | | | | | Until now, a SCSIDevice's I/O requests have run in a single AioContext. In order to support multiple IOThreads it will be necessary to move to the concept of a per-SCSIRequest AioContext. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20250311132616.1049687-4-stefanha@redhat.com> Tested-by: Peter Krempa <pkrempa@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* Merge tag 'exec-20241220' of https://github.com/philmd/qemu into stagingStefan Hajnoczi2024-12-211-5/+5
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Accel & Exec patch queue - Ignore writes to CNTP_CTL_EL0 on HVF ARM (Alexander) - Add '-d invalid_mem' logging option (Zoltan) - Create QOM containers explicitly (Peter) - Rename sysemu/ -> system/ (Philippe) - Re-orderning of include/exec/ headers (Philippe) Move a lot of declarations from these legacy mixed bag headers: . "exec/cpu-all.h" . "exec/cpu-common.h" . "exec/cpu-defs.h" . "exec/exec-all.h" . "exec/translate-all" to these more specific ones: . "exec/page-protection.h" . "exec/translation-block.h" . "user/cpu_loop.h" . "user/guest-host.h" . "user/page-protection.h" # -----BEGIN PGP SIGNATURE----- # # iQIzBAABCAAdFiEE+qvnXhKRciHc/Wuy4+MsLN6twN4FAmdlnyAACgkQ4+MsLN6t # wN6mBw//QFWi7CrU+bb8KMM53kOU9C507tjn99LLGFb5or73/umDsw6eo/b8DHBt # KIwGLgATel42oojKfNKavtAzLK5rOrywpboPDpa3SNeF1onW+99NGJ52LQUqIX6K # A6bS0fPdGG9ZzEuPpbjDXlp++0yhDcdSgZsS42fEsT7Dyj5gzJYlqpqhiXGqpsn8 # 4Y0UMxSL21K3HEexlzw2hsoOBFA3tUm2ujNDhNkt8QASr85yQVLCypABJnuoe/// # 5Ojl5wTBeDwhANET0rhwHK8eIYaNboiM9fHopJYhvyw1bz6yAu9jQwzF/MrL3s/r # xa4OBHBy5mq2hQV9Shcl3UfCQdk/vDaYaWpgzJGX8stgMGYfnfej1SIl8haJIfcl # VMX8/jEFdYbjhO4AeGRYcBzWjEJymkDJZoiSWp2NuEDi6jqIW+7yW1q0Rnlg9lay # ShAqLK5Pv4zUw3t0Jy3qv9KSW8sbs6PQxtzXjk8p97rTf76BJ2pF8sv1tVzmsidP # 9L92Hv5O34IqzBu2oATOUZYJk89YGmTIUSLkpT7asJZpBLwNM2qLp5jO00WVU0Sd # +kAn324guYPkko/TVnjC/AY7CMu55EOtD9NU35k3mUAnxXT9oDUeL4NlYtfgrJx6 # x1Nzr2FkS68+wlPAFKNSSU5lTjsjNaFM0bIJ4LCNtenJVP+SnRo= # =cjz8 # -----END PGP SIGNATURE----- # gpg: Signature made Fri 20 Dec 2024 11:45:20 EST # gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE # gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [unknown] # gpg: WARNING: This key is not certified with a trusted signature! # gpg: There is no indication that the signature belongs to the owner. # Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE * tag 'exec-20241220' of https://github.com/philmd/qemu: (59 commits) util/qemu-timer: fix indentation meson: Do not define CONFIG_DEVICES on user emulation system/accel-ops: Remove unnecessary 'exec/cpu-common.h' header system/numa: Remove unnecessary 'exec/cpu-common.h' header hw/xen: Remove unnecessary 'exec/cpu-common.h' header target/mips: Drop left-over comment about Jazz machine target/mips: Remove tswap() calls in semihosting uhi_fstat_cb() target/xtensa: Remove tswap() calls in semihosting simcall() helper accel/tcg: Un-inline translator_is_same_page() accel/tcg: Include missing 'exec/translation-block.h' header accel/tcg: Move tcg_cflags_has/set() to 'exec/translation-block.h' accel/tcg: Restrict curr_cflags() declaration to 'internal-common.h' qemu/coroutine: Include missing 'qemu/atomic.h' header exec/translation-block: Include missing 'qemu/atomic.h' header accel/tcg: Declare cpu_loop_exit_requested() in 'exec/cpu-common.h' exec/cpu-all: Include 'cpu.h' earlier so MMU_USER_IDX is always defined target/sparc: Move sparc_restore_state_to_opc() to cpu.c target/sparc: Uninline cpu_get_tb_cpu_state() target/loongarch: Declare loongarch_cpu_dump_state() locally user: Move various declarations out of 'exec/exec-all.h' ... Conflicts: hw/char/riscv_htif.c hw/intc/riscv_aplic.c target/s390x/cpu.c Apply sysemu header path changes to not in the pull request. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
| * include: Rename sysemu/ -> system/Philippe Mathieu-Daudé2024-12-201-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | Headers in include/sysemu/ are not only related to system *emulation*, they are also used by virtualization. Rename as system/ which is clearer. Files renamed manually then mechanical change using sed tool. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Lei Yang <leiyang@redhat.com> Message-Id: <20241203172445.28576-1-philmd@linaro.org>
* | include/hw/qdev-properties: Remove DEFINE_PROP_END_OF_LISTRichard Henderson2024-12-191-1/+0
|/ | | | | | | | | | | | | | Now that all of the Property arrays are counted, we can remove the terminator object from each array. Update the assertions in device_class_set_props to match. With struct Property being 88 bytes, this was a rather large form of terminator. Saves 30k from qemu-system-aarch64. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Tested-by: Lei Yang <leiyang@redhat.com> Link: https://lore.kernel.org/r/20241218134251.4724-21-richard.henderson@linaro.org Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* hw/scsi: Constify all PropertyRichard Henderson2024-12-151-1/+1
| | | | | Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
* scsi: fix regression and honor bootindex again for legacy drivesFiona Ebner2024-07-161-0/+9
| | | | | | | | | | | | | | | | | | | | | | | | Commit 3089637461 ("scsi: Don't ignore most usb-storage properties") removed the call to object_property_set_int() and thus the 'set' method for the bootindex property was also not called anymore. Here that method is device_set_bootindex() (as configured by scsi_dev_instance_init() -> device_add_bootindex_property()) which as a side effect registers the device via add_boot_device_path(). As reported by a downstream user [0], the bootindex property did not have the desired effect anymore for legacy drives. Fix the regression by explicitly calling the add_boot_device_path() function after checking that the bootindex is not yet used (to avoid add_boot_device_path() calling exit()). [0]: https://forum.proxmox.com/threads/149772/post-679433 Cc: qemu-stable@nongnu.org Fixes: 3089637461 ("scsi: Don't ignore most usb-storage properties") Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Link: https://lore.kernel.org/r/20240710152529.1737407-1-f.ebner@proxmox.com Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: Don't ignore most usb-storage propertiesKevin Wolf2024-02-071-20/+13
| | | | | | | | | | | | | | | | | | | usb-storage is for the most part just a wrapper around an internally created scsi-disk device. It uses DEFINE_BLOCK_PROPERTIES() to offer all of the usual block device properties to the user, but then only forwards a few select properties to the internal device while the rest is silently ignored. This changes scsi_bus_legacy_add_drive() to accept a whole BlockConf instead of some individual values inside of it so that usb-storage can now pass the whole configuration to the internal scsi-disk. This enables the remaining block device properties, e.g. logical/physical_block_size or discard_granularity. Buglink: https://issues.redhat.com/browse/RHEL-22375 Signed-off-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <20240131130607.24117-1-kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* scsi: Await request purgingHanna Czenczek2024-02-071-9/+21
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | scsi_device_for_each_req_async() currently does not provide any way to be awaited. One of its callers is scsi_device_purge_requests(), which therefore currently does not guarantee that all requests are fully settled when it returns. We want all requests to be settled, because scsi_device_purge_requests() is called through the unrealize path, including the one invoked by virtio_scsi_hotunplug() through qdev_simple_device_unplug_cb(), which most likely assumes that all SCSI requests are done then. In fact, scsi_device_purge_requests() already contains a blk_drain(), but this will not fully await scsi_device_for_each_req_async(), only the I/O requests it potentially cancels (not the non-I/O requests). However, we can have scsi_device_for_each_req_async() increment the BB in-flight counter, and have scsi_device_for_each_req_async_bh() decrement it when it is done. This way, the blk_drain() will fully await all SCSI requests to be purged. This also removes the need for scsi_device_for_each_req_async_bh() to double-check the current context and potentially re-schedule itself, should it now differ from the BB's context: Changing a BB's AioContext with a root node is done through bdrv_try_change_aio_context(), which creates a drained section. With this patch, we keep the BB in-flight counter elevated throughout, so we know the BB's context cannot change. Signed-off-by: Hanna Czenczek <hreitz@redhat.com> Message-ID: <20240202144755.671354-3-hreitz@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* hw/scsi: Constify VMStateRichard Henderson2023-12-301-3/+3
| | | | | Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Message-Id: <20231221031652.119827-52-richard.henderson@linaro.org>
* scsi: remove AioContext lockingStefan Hajnoczi2023-12-211-2/+0
| | | | | | | | | | The AioContext lock no longer has any effect. Remove it. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-ID: <20231205182011.1976568-9-stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* scsi: only access SCSIDevice->requests from one threadStefan Hajnoczi2023-12-211-56/+125
| | | | | | | | | | | | | | | | | | | | | Stop depending on the AioContext lock and instead access SCSIDevice->requests from only one thread at a time: - When the VM is running only the BlockBackend's AioContext may access the requests list. - When the VM is stopped only the main loop may access the requests list. These constraints protect the requests list without the need for locking in the I/O code path. Note that multiple IOThreads are not supported yet because the code assumes all SCSIRequests are executed from a single AioContext. Leave that as future work. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-ID: <20231204164259.1515217-2-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* scsi: clear unit attention only for REPORT LUNS commandsStefano Garzarella2023-07-141-23/+11
| | | | | | | | | | | | | | scsi_clear_unit_attention() now only handles REPORTED LUNS DATA HAS CHANGED. This only happens when we handle REPORT LUNS commands, so let's rename the function in scsi_clear_reported_luns_changed() and call it only in scsi_target_emulate_report_luns(). Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-ID: <20230712134352.118655-4-sgarzare@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: cleanup scsi_clear_unit_attention()Stefano Garzarella2023-07-141-22/+6
| | | | | | | | | | | | The previous commit moved the unit attention clearing when we create the request. So now we can clean scsi_clear_unit_attention() to handle only the case of the REPORT LUNS command: this is the only case in which a UNIT ATTENTION is cleared without having been reported. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-ID: <20230712134352.118655-3-sgarzare@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: fetch unit attention when creating the requestStefano Garzarella2023-07-141-3/+33
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 1880ad4f4e ("virtio-scsi: Batched prepare for cmd reqs") split calls to scsi_req_new() and scsi_req_enqueue() in the virtio-scsi device. No ill effects were observed until commit 8cc5583abe ("virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events") added a unit attention that was easy to trigger with device hotplug and hot-unplug. Because the two calls were separated, all requests in the batch were prepared calling scsi_req_new() to report a sense. The first one submitted would report the right sense and reset it to NO_SENSE, while the others reported CHECK_CONDITION with no sense data. This caused SCSI errors in Linux. To solve this issue, let's fetch the unit attention as early as possible when we prepare the request, so that only the first request in the batch will use the unit attention SCSIReqOps and the others will not report CHECK CONDITION. Fixes: 1880ad4f4e ("virtio-scsi: Batched prepare for cmd reqs") Fixes: 8cc5583abe ("virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug events") Reported-by: Thomas Huth <thuth@redhat.com> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=2176702 Co-developed-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Message-ID: <20230712134352.118655-2-sgarzare@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* virtio-scsi: implement BlockDevOps->drained_begin()Stefan Hajnoczi2023-05-301-0/+40
| | | | | | | | | | | | | | | | | | | | | | | | The virtio-scsi Host Bus Adapter provides access to devices on a SCSI bus. Those SCSI devices typically have a BlockBackend. When the BlockBackend enters a drained section, the SCSI device must temporarily stop submitting new I/O requests. Implement this behavior by temporarily stopping virtio-scsi virtqueue processing when one of the SCSI devices enters a drained section. The new scsi_device_drained_begin() API allows scsi-disk to message the virtio-scsi HBA. scsi_device_drained_begin() uses a drain counter so that multiple SCSI devices can have overlapping drained sections. The HBA only sees one pair of .drained_begin/end() calls. After this commit, virtio-scsi no longer depends on hw/virtio's ioeventfd aio_set_event_notifier(is_external=true). This commit is a step towards removing the aio_disable_external() API. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20230516190238.8401-19-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* virtio-scsi: avoid race between unplug and transport eventStefan Hajnoczi2023-05-301-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Only report a transport reset event to the guest after the SCSIDevice has been unrealized by qdev_simple_device_unplug_cb(). qdev_simple_device_unplug_cb() sets the SCSIDevice's qdev.realized field to false so that scsi_device_find/get() no longer see it. scsi_target_emulate_report_luns() also needs to be updated to filter out SCSIDevices that are unrealized. Change virtio_scsi_push_event() to take event information as an argument instead of the SCSIDevice. This allows virtio_scsi_hotunplug() to emit a VIRTIO_SCSI_T_TRANSPORT_RESET event after the SCSIDevice has already been unrealized. These changes ensure that the guest driver does not see the SCSIDevice that's being unplugged if it responds very quickly to the transport reset event. Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Daniil Tatianin <d-tatianin@yandex-team.ru> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230516190238.8401-4-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* hw/qdev: introduce qdev_is_realized() helperStefan Hajnoczi2023-05-301-2/+1
| | | | | | | | | | | | | | Add a helper function to check whether the device is realized without requiring the Big QEMU Lock. The next patch adds a second caller. The goal is to avoid spreading DeviceState field accesses throughout the code. Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-Id: <20230516190238.8401-3-stefanha@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
* hw: replace most qemu_bh_new calls with qemu_bh_new_guardedAlexander Bulekov2023-04-281-1/+2
| | | | | | | | | | | | | | This protects devices from bh->mmio reentrancy issues. Thanks: Thomas Huth <thuth@redhat.com> for diagnosing OS X test failure. Signed-off-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Reviewed-by: Paul Durrant <paul@xen.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-Id: <20230427211013.2994127-5-alxndr@bu.edu> Signed-off-by: Thomas Huth <thuth@redhat.com>
* replace TABs with spacesYeqi Fu2023-03-201-2/+2
| | | | | | | | | | | Bring the files in line with the QEMU coding style, with spaces for indentation. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/378 Signed-off-by: Yeqi Fu <fufuyqqqqqq@gmail.com> Message-Id: <20230315032649.57568-1-fufuyqqqqqq@gmail.com> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Thomas Huth <thuth@redhat.com>
* virtio-scsi: Send "REPORTED LUNS CHANGED" sense data upon disk hotplug eventsVenu Busireddy2022-10-131-0/+18
| | | | | | | | | | | | | | | | | | | Section 5.6.6.3 of VirtIO specification states, "Events will also be reported via sense codes..." However, no sense data is sent when VIRTIO_SCSI_EVT_RESET_RESCAN or VIRTIO_SCSI_EVT_RESET_REMOVED events are reported (when disk hotplug/hotunplug events occur). SCSI layer on Solaris depends on this sense data, and hence does not handle disk hotplug/hotunplug events. When the disk inventory changes, use the bus unit attention mechanism to return a CHECK_CONDITION status with sense data of 0x06/0x3F/0x0E (sense code REPORTED_LUNS_CHANGED). The first device on the bus to execute a command successfully will report and consume the unit attention status. Signed-off-by: Venu Busireddy <venu.busireddy@oracle.com> Message-Id: <20221006194946.24134-1-venu.busireddy@oracle.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: Reject commands if the CDB length exceeds buf_lenJohn Millikin2022-09-011-1/+7
| | | | | | | | | | | | | | | | | | | | | | | | In scsi_req_parse_cdb(), if the CDB length implied by the command type exceeds the initialized portion of the command buffer, reject the request. Rejected requests are recorded by the `scsi_req_parse_bad` trace event. On example of a bug detected by this check is SunOS's use of interleaved DMA and non-DMA commands. This guest behavior currently causes QEMU to parse uninitialized memory as a SCSI command, with unpredictable outcomes. With the new check in place: * QEMU consistently creates a trace event and rejects the request. * SunOS retries the request(s) and is able to successfully boot from disk. Signed-off-by: John Millikin <john@john-millikin.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1127 Message-Id: <20220817053458.698416-2-john@john-millikin.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: Add buf_len parameter to scsi_req_new()John Millikin2022-09-011-8/+13
| | | | | | | | | | | | | | | | When a SCSI command is received from the guest, the CDB length implied by the first byte might exceed the number of bytes the guest sent. In this case scsi_req_new() will read uninitialized data, causing unpredictable behavior. Adds the buf_len parameter to scsi_req_new() and plumbs it through the call stack. Signed-off-by: John Millikin <john@john-millikin.com> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1127 Message-Id: <20220817053458.698416-1-john@john-millikin.com> [Fill in correct length for adapters other than ESP. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* include: Move hardware version declarations to new qemu/hw-version.hPeter Maydell2022-02-211-0/+1
| | | | | | | | | | | | The "hardware version" machinery (qemu_set_hw_version(), qemu_hw_version(), and the QEMU_HW_VERSION define) is used by fewer than 10 files. Move it out from osdep.h into a new qemu/hw-version.h. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Message-id: 20220208200856.3558249-6-peter.maydell@linaro.org
* hw/dma: Let dma_buf_read() / dma_buf_write() propagate MemTxResultPhilippe Mathieu-Daudé2022-01-181-2/+4
| | | | | | | | | | | | | | | | | | | | | | Since commit 292e13142d2, dma_buf_rw() returns a MemTxResult type. Do not discard it, return it to the caller. Pass the previously returned value (the QEMUSGList residual size, which was rarely used) as an optional argument. With this new API, SCSIRequest::residual might now be accessed via a pointer. Since the size_t type does not have the same size on 32 and 64-bit host architectures, convert it to a uint64_t, which is big enough to hold the residual size, and the type is constant on both 32/64-bit hosts. Update the few dma_buf_read() / dma_buf_write() callers to the new API. Reviewed-by: Klaus Jensen <k.jensen@samsung.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Acked-by: Peter Xu <peterx@redhat.com> Message-Id: <20220117125130.131828-1-f4bug@amsat.org>
* hw/scsi: Rename SCSIRequest::resid as 'residual'Philippe Mathieu-Daudé2022-01-181-5/+5
| | | | | | | | | | | | The 'resid' field is slightly confusing and could be interpreted as some ID. Rename it as 'residual' which is clearer to review. No logical change. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: David Hildenbrand <david@redhat.com> Message-Id: <20220111184309.28637-8-f4bug@amsat.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
* dma: Let dma_buf_read() take MemTxAttrs argumentPhilippe Mathieu-Daudé2021-12-311-1/+1
| | | | | | | | | | | Let devices specify transaction attributes when calling dma_buf_read(). Keep the default MEMTXATTRS_UNSPECIFIED in the few callers. Reviewed-by: Klaus Jensen <k.jensen@samsung.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20211223115554.3155328-13-philmd@redhat.com>
* dma: Let dma_buf_write() take MemTxAttrs argumentPhilippe Mathieu-Daudé2021-12-311-1/+1
| | | | | | | | | | | Let devices specify transaction attributes when calling dma_buf_write(). Keep the default MEMTXATTRS_UNSPECIFIED in the few callers. Reviewed-by: Klaus Jensen <k.jensen@samsung.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20211223115554.3155328-12-philmd@redhat.com>
* qbus: Rename qbus_create_inplace() to qbus_init()Peter Maydell2021-09-301-1/+1
| | | | | | | | | | | Rename qbus_create_inplace() to qbus_init(); this is more in line with our usual naming convention for functions that in-place initialize objects. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Message-id: 20210923121153.23754-5-peter.maydell@linaro.org
* scsi: Replace scsi_bus_new() with scsi_bus_init(), scsi_bus_init_named()Peter Maydell2021-09-301-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The function scsi_bus_new() creates a new SCSI bus; callers can either pass in a name argument to specify the name of the new bus, or they can pass in NULL to allow the bus to be given an automatically generated unique name. Almost all callers want to use the autogenerated name; the only exception is the virtio-scsi device. Taking a name argument that should almost always be NULL is an easy-to-misuse API design -- it encourages callers to think perhaps they should pass in some standard name like "scsi" or "scsi-bus". We don't do this anywhere for SCSI, but we do (incorrectly) do it for other bus types such as i2c. The function name also implies that it will return a newly allocated object, when it in fact does in-place allocation. We more commonly name such functions foo_init(), with foo_new() being the allocate-and-return variant. Replace all the scsi_bus_new() callsites with either: * scsi_bus_init() for the usual case where the caller wants an autogenerated bus name * scsi_bus_init_named() for the rare case where the caller needs to specify the bus name and document that for the _named() version it's then the caller's responsibility to think about uniqueness of bus names. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Message-id: 20210923121153.23754-2-peter.maydell@linaro.org
* sysemu: Let VMChangeStateHandler take boolean 'running' argumentPhilippe Mathieu-Daudé2021-03-091-1/+1
| | | | | | | | | | | The 'running' argument from VMChangeStateHandler does not require other value than 0 / 1. Make it a plain boolean. Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Acked-by: David Gibson <david@gibson.dropbear.id.au> Message-Id: <20210111152020.1422021-3-philmd@redhat.com> Signed-off-by: Laurent Vivier <laurent@vivier.eu>
* scsi: move host_status handling into SCSI driversHannes Reinecke2021-03-061-2/+31
| | | | | | | | | | | Some SCSI drivers like virtio have an internal mapping for the host_status. This patch moves the host_status translation into the SCSI drivers to allow those drivers to set up the correct values. Signed-off-by: Hannes Reinecke <hare@suse.de>. [Added default handling to avoid touching all drivers. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: drop 'result' argument from command_complete callbackHannes Reinecke2021-02-251-1/+1
| | | | | | | | | | The command complete callback has a SCSIRequest as the first argument, and the status field of that structure is identical to the 'status' argument. So drop the argument from the callback. Signed-off-by: Hannes Reinecke <hare@suse.de> Message-Id: <20201116184041.60465-3-hare@suse.de> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* migration: Replace migration's JSON writer by the general oneMarkus Armbruster2020-12-191-1/+1
| | | | | | | | | | | | | | | | | | | Commit 8118f0950f "migration: Append JSON description of migration stream" needs a JSON writer. The existing qobject_to_json() wasn't a good fit, because it requires building a QObject to convert. Instead, migration got its very own JSON writer, in commit 190c882ce2 "QJSON: Add JSON writer". It tacitly limits numbers to int64_t, and strings contents to characters that don't need escaping, unlike qobject_to_json(). The previous commit factored the JSON writer out of qobject_to_json(). Replace migration's JSON writer by it. Cc: Juan Quintela <quintela@redhat.com> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20201211171152.146877-17-armbru@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
* scsi: fix device removal race vs IO restart callback on resumeMaxim Levitsky2020-12-151-0/+4
| | | | | | | | | | | | | | | | | | There is (mostly theoretical) race between removal of a scsi device and scsi_dma_restart_bh. It used to be easier to hit this race prior to my / Paulo's patch series that added rcu to scsi bus device handling code, but IMHO this race should still be possible to hit, at least in theory. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1854811 Fix it anyway with a patch that was proposed by Paulo in the above bugzilla. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Message-Id: <20201210125929.1136390-2-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi/scsi_bus: fix races in REPORT LUNSMaxim Levitsky2020-10-121-35/+33
| | | | | | | | | | | | | | | | | | Currently scsi_target_emulate_report_luns iterates over the child device list twice, and there is no guarantee that this list is the same in both iterations. The reason for iterating twice is that the first iteration calculates how much memory to allocate. However if we use a dynamic array we can avoid iterating twice, and therefore we avoid this race. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1866707 Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-10-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-14-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi/scsi_bus: Add scsi_device_getMaxim Levitsky2020-10-121-0/+11
| | | | | | | | | | | | Add scsi_device_get which finds the scsi device and takes a reference to it. Suggested-by: Stefan Hajnoczi <stefanha@gmail.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Message-Id: <20200913160259.32145-8-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-12-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi/scsi-bus: scsi_device_find: don't return unrealized devicesPaolo Bonzini2020-10-121-30/+53
| | | | | | | | | | | | | | | | The device core first places a device on the bus and then realizes it. Make scsi_device_find avoid returing such devices to avoid races in drivers that use an iothread (currently virtio-scsi) Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1812399 Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-7-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-11-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi: switch to bus->check_addressPaolo Bonzini2020-10-121-47/+75
| | | | | | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-6-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* device-core: use RCU for list of children of a busMaxim Levitsky2020-10-121-3/+9
| | | | | | | | | | | | | | | | | | | | | This fixes the race between device emulation code that tries to find a child device to dispatch the request to (e.g a scsi disk), and hotplug of a new device to that bus. Note that this doesn't convert all the readers of the list but only these that might go over that list without BQL held. This is a very small first step to make this code thread safe. Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-5-mlevitsk@redhat.com> [Use RCU_READ_LOCK_GUARD in more places, adjust testcase now that the delay in DEVICE_DELETED due to RCU is more consistent. - Paolo] Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Message-Id: <20201006123904.610658-9-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* scsi/scsi_bus: switch search direction in scsi_device_findMaxim Levitsky2020-10-121-2/+10
| | | | | | | | | | This change will allow us to convert the bus children list to RCU, while not changing the logic of this function Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Message-Id: <20200913160259.32145-2-mlevitsk@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
* qom: simplify object_find_property / object_class_find_propertyDaniel P. Berrangé2020-09-221-2/+2
| | | | | | | | | | | | | | | | | | | | | | | When debugging QEMU it is often useful to put a breakpoint on the error_setg_internal method impl. Unfortunately the object_property_add / object_class_property_add methods call object_property_find / object_class_property_find methods to check if a property exists already before adding the new property. As a result there are a huge number of calls to error_setg_internal on startup of most QEMU commands, making it very painful to set a breakpoint on this method. Most callers of object_find_property and object_class_find_property, however, pass in a NULL for the Error parameter. This simplifies the methods to remove the Error parameter entirely, and then adds some new wrapper methods that are able to raise an Error when needed. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Message-Id: <20200914135617.1493072-1-berrange@redhat.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
* error: Eliminate error_propagate() with Coccinelle, part 1Markus Armbruster2020-07-101-7/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When all we do with an Error we receive into a local variable is propagating to somewhere else, we can just as well receive it there right away. Convert if (!foo(..., &err)) { ... error_propagate(errp, err); ... return ... } to if (!foo(..., errp)) { ... ... return ... } where nothing else needs @err. Coccinelle script: @rule1 forall@ identifier fun, err, errp, lbl; expression list args, args2; binary operator op; constant c1, c2; symbol false; @@ if ( ( - fun(args, &err, args2) + fun(args, errp, args2) | - !fun(args, &err, args2) + !fun(args, errp, args2) | - fun(args, &err, args2) op c1 + fun(args, errp, args2) op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; ) } @rule2 forall@ identifier fun, err, errp, lbl; expression list args, args2; expression var; binary operator op; constant c1, c2; symbol false; @@ - var = fun(args, &err, args2); + var = fun(args, errp, args2); ... when != err if ( ( var | !var | var op c1 ) ) { ... when != err when != lbl: when strict - error_propagate(errp, err); ... when != err ( return; | return c2; | return false; | return var; ) } @depends on rule1 || rule2@ identifier err; @@ - Error *err = NULL; ... when != err Not exactly elegant, I'm afraid. The "when != lbl:" is necessary to avoid transforming if (fun(args, &err)) { goto out } ... out: error_propagate(errp, err); even though other paths to label out still need the error_propagate(). For an actual example, see sclp_realize(). Without the "when strict", Coccinelle transforms vfio_msix_setup(), incorrectly. I don't know what exactly "when strict" does, only that it helps here. The match of return is narrower than what I want, but I can't figure out how to express "return where the operand doesn't use @err". For an example where it's too narrow, see vfio_intx_enable(). Silently fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. Line breaks tidied up manually. One nested declaration of @local_err deleted manually. Preexisting unwanted blank line dropped in hw/riscv/sifive_e.c. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-35-armbru@redhat.com>
* qdev: Use returned bool to check for failure, Coccinelle partMarkus Armbruster2020-07-101-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous commit enables conversion of qdev_prop_set_drive_err(..., &err); if (err) { ... } to if (!qdev_prop_set_drive_err(..., errp)) { ... } Coccinelle script: @@ identifier fun = qdev_prop_set_drive_err; expression list args; typedef Error; Error *err; @@ - fun(args, &err); - if (err) + if (!fun(args, &err)) { ... } One line break tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-Id: <20200707160613.848843-33-armbru@redhat.com>
* qom: Use returned bool to check for failure, Coccinelle partMarkus Armbruster2020-07-101-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The previous commit enables conversion of foo(..., &err); if (err) { ... } to if (!foo(..., errp)) { ... } for QOM functions that now return true / false on success / error. Coccinelle script: @@ identifier fun = { object_apply_global_props, object_initialize_child_with_props, object_initialize_child_with_propsv, object_property_get, object_property_get_bool, object_property_parse, object_property_set, object_property_set_bool, object_property_set_int, object_property_set_link, object_property_set_qobject, object_property_set_str, object_property_set_uint, object_set_props, object_set_propv, user_creatable_add_dict, user_creatable_complete, user_creatable_del }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-29-armbru@redhat.com>
* qom: Put name parameter before value / visitor parameterMarkus Armbruster2020-07-101-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The object_property_set_FOO() setters take property name and value in an unusual order: void object_property_set_FOO(Object *obj, FOO_TYPE value, const char *name, Error **errp) Having to pass value before name feels grating. Swap them. Same for object_property_set(), object_property_get(), and object_property_parse(). Convert callers with this Coccinelle script: @@ identifier fun = { object_property_get, object_property_parse, object_property_set_str, object_property_set_link, object_property_set_bool, object_property_set_int, object_property_set_uint, object_property_set, object_property_set_qobject }; expression obj, v, name, errp; @@ - fun(obj, v, name, errp) + fun(obj, name, v, errp) Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Convert that one manually. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Convert manually. Fails to convert hw/rx/rx-gdbsim.c, because Coccinelle gets confused by RXCPU being used both as typedef and function-like macro there. Convert manually. The other files using RXCPU that way don't need conversion. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Message-Id: <20200707160613.848843-27-armbru@redhat.com> [Straightforwad conflict with commit 2336172d9b "audio: set default value for pcspk.iobase property" resolved]
* qdev: Use returned bool to check for qdev_realize() etc. failureMarkus Armbruster2020-07-101-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Convert foo(..., &err); if (err) { ... } to if (!foo(..., &err)) { ... } for qdev_realize(), qdev_realize_and_unref(), qbus_realize() and their wrappers isa_realize_and_unref(), pci_realize_and_unref(), sysbus_realize(), sysbus_realize_and_unref(), usb_realize_and_unref(). Coccinelle script: @@ identifier fun = { isa_realize_and_unref, pci_realize_and_unref, qbus_realize, qdev_realize, qdev_realize_and_unref, sysbus_realize, sysbus_realize_and_unref, usb_realize_and_unref }; expression list args, args2; typedef Error; Error *err; @@ - fun(args, &err, args2); - if (err) + if (!fun(args, &err, args2)) { ... } Chokes on hw/arm/musicpal.c's lcd_refresh() with the unhelpful error message "no position information". Nothing to convert there; skipped. Fails to convert hw/arm/armsse.c, because Coccinelle gets confused by ARMSSE being used both as typedef and function-like macro there. Converted manually. A few line breaks tidied up manually. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Greg Kurz <groug@kaod.org> Message-Id: <20200707160613.848843-5-armbru@redhat.com>