- 29 May, 2019 4 commits
-
-
Andre Przywara authored
clang and GCC9 refuse to compile virtio/blk.c with the following message: virtio/blk.c:161:37: error: taking address of packed member 'geometry' of class or structure 'virtio_blk_config' may result in an unaligned pointer value [-Werror,-Waddress-of-packed-member] struct virtio_blk_geometry *geo = &conf->geometry; Since struct virtio_blk_geometry is in a kernel header, we can't do much about the packed attribute, but as Peter pointed out, the solution is rather simple: just get rid of the convenience variable and use the original struct member directly. Reviewed-by:Jean-Philippe Brucker <jean-philippe.brucker@arm.com> Suggested-by:
Peter Maydell <peter.maydell@linaro.org> Signed-off-by:
Andre Przywara <andre.przywara@arm.com> Signed-off-by:
Will Deacon <will.deacon@arm.com>
-
Andre Przywara authored
struct vfio_irq_set from the kernel headers contains a variable sized array to hold a payload. The vfio_irq_eventfd struct puts the "fd" member right after this, hoping it to automatically fit in the payload slot. But having a variable sized type not at the end of a struct is a GNU C extension, so clang will refuse to compile this. Solve this by somewhat doing the compiler's job and place the payload manually at the end of the structure. Reviewed-by:
-
Andre Przywara authored
clang complained that the comparison of an u8 variable against 256 is somewhat pointless. Just remove the check, as the condition will never hit. Signed-off-by:
-
Andre Przywara authored
As clang rightfully pointed out, the ampersand in front of this member looks wrong. Remove it so we actually really compare against the count being 0. Signed-off-by:
-
- 26 Apr, 2019 15 commits
-
-
Jean-Philippe Brucker authored
Ensure that all requests are complete when resetting a virtqueue, by draining the AIO queue after stopping the submission thread. Signed-off-by:
-
Jean-Philippe Brucker authored
Add a call into the disk layer to synchronize the AIO queue. Wait for all pending requests to complete. This will be necessary when resetting a virtqueue. The wait() operation isn't the same as flush(). A VIRTIO_BLK_T_FLUSH request ensures that any write request *that completed before the FLUSH is sent* is committed to permanent storage (e.g. written back from a write cache). But it doesn't do anything for requests that are still pending when the FLUSH is sent. Avoid introducing a mutex on the io_submit() and io_getevents() paths, because it can lead to 30% throughput drop on heavy FIO jobs. Instead manage an inflight counter using compare-and-swap operations, which is simple enough as the caller doesn't submit new requests while it waits for the AIO queue to drain. The __sync_fetch_and_* operations are a bit rough since they use full barriers, but that didn't seem to introduce a performance regression. Signed-off-by: Jean-Philippe Brucker <jean-philippe.brucker@arm....
-
Jean-Philippe Brucker authored
If the AIO thread is still calling io_getevents() while the exit path calls io_destroy(), it will segfault. Wait for the thread to finish before destroying the context. Reviewed-by:
-
Jean-Philippe Brucker authored
Currently when the kernel completes a batch of AIO requests and signals it via eventfd, we retrieve at most AIO_MAX events (256), and ignore the rest. Call io_getevents() again in case more events are pending. Reviewed-by:
-
Jean-Philippe Brucker authored
Add an 'async' attribute to disk_image_operations, that describes if they can submit async I/O or not. disk_image->async is now set iff CONFIG_HAS_AIO and the ops do use AIO. This fixes qcow1, which used to set async = 1 even though the qcow operations don't use AIO. The disk core would perform the read/write operation without pushing the completion onto the virtio queue, and the guest would be stuck waiting. Reviewed-by:
-
Jean-Philippe Brucker authored
Move all AIO code to a separate file, disk/aio.c, to remove as much #ifdefs as possible. Split the raw read/write disk ops into async and sync, and choose which ones to use depending on CONFIG_HAS_AIO. Note that we fix raw_image__close() which incorrectly checked CONFIG_HAS_VIRTIO instead of CONFIG_HAS_AIO, and closed an unitialized disk->evt. A subsequent commit will complete this refactoring by fixing use of the 'async' disk attribute. Reviewed-by:
-
Jean-Philippe Brucker authored
sync() should be called before reboot(RB_AUTOBOOT), otherwise data written to disks might be lost. Signed-off-by:
-
Jean-Philippe Brucker authored
Since we don't currently tell the guest when the disk backend is read-only, it will report any inconsistent read after write as an error. An image may be read-only either because user requested it on the command-line, or because write support isn't implemented. Pass the read-only attribute using the VIRTIO_BLK_F_RO feature. Reviewed-by:
-
Jean-Philippe Brucker authored
Even though qcow1 doesn't use the refcount table, the cleanup path still attempts to iterate over its LRU list. Initialize the list to avoid a segfault on exit. Reviewed-by:
-
Julien Thierry authored
Build breaks when using KVM_BRLOCK_DEBUG because the header was seamingly conceived to be included in a single .c file... Fix this by moving the definition of the read/write lock into the kvm struct. Reviewed-by:
Julien Thierry <julien.thierry@arm.com> Signed-off-by:
-
Julien Thierry authored
The kvm argument is not passed to br_read_lock/unlock, this works for the barrier implementation because the argument is not used. This ever breaks if another lock implementation is used. Reviewed-by:
-
Julien Thierry authored
The vesa framebuffer is only used by architectures that explicitly require it (i.e. x86). Compile it out for architectures not using it, as its current implementation might not work for them. Reviewed-by:
-
Leo Yan authored
Since PCI forbids enabling INTx, MSI or MSIX at the same time, it's by default to disable INTx mode when enable MSI/MSIX mode; but this logic is easily broken if the guest PCI driver detects the MSI/MSIX cannot work as expected and tries to rollback to use INTx mode. In this case, the INTx mode has been disabled and has no chance to re-enable it, thus both INTx mode and MSI/MSIX mode cannot work in vfio. Below shows the detailed flow for introducing this issue: vfio_pci_configure_dev_irqs() `-> vfio_pci_enable_intx() vfio_pci_enable_msis() `-> vfio_pci_disable_intx() vfio_pci_disable_msis() => Guest PCI driver disables MSI To fix this issue, when disable MSI/MSIX we need to check if INTx mode is available for this device or not; if the device can support INTx then re-enable it so that the device can fallback to use it. Since vfio_pci_disable_intx() / vfio_pci_enable_intx() pair functions may be called for multiple times, this patch uses 'intx_fd == -1' to denote the INTx is disabled, the pair functions can directly bail out when detect INTx has been disabled and enabled respectively. Suggested-by:Leo Yan <leo.yan@linaro.org> Reviewed-by:
-
Leo Yan authored
To support INTx enabling for multiple times, we need firstly to extract one-time initialisation and move the related code into a new function vfio_pci_init_intx(); if later disable and re-enable the INTx, we can skip these one-time operations. This patch move below three main operations for INTx one-time initialisation from function vfio_pci_enable_intx() into function vfio_pci_init_intx(): - Reserve 2 FDs for INTx; - Sanity check with ioctl VFIO_DEVICE_GET_IRQ_INFO; - Setup pdev->intx_gsi. Suggested-by:
-
Leo Yan authored
The PCI device INTx uses event fd 'unmask_fd' to signal the deassertion of the line from guest to host; but this eventfd isn't released properly when disable INTx. This patch firstly adds field 'unmask_fd' in struct vfio_pci_device for storing unmask eventfd and close it when disable INTx. Signed-off-by:
-
- 11 Feb, 2019 1 commit
-
-
Andre Przywara authored
At the moment kvmtool always tries to instantiate a virtual GICv2 interrupt controller for the guest, and fails with some scary error message if that doesn't work. The user has then to manually specify "--irqchip=gicv3", which is not really obvious. With the advent of more GICv3-only machines, let's try to be more clever and implement some auto-detection of the GIC type needed: We try gicv3-its, gicv3, gicv2m and gicv2, in that order. The first one succeeding wins. For GICv2 machines the first two will always fail. On GICv3 machines offering GICv2 compatibility we used to prefer a virtual GICv2 in the guest, but these days the GICv3 support both in guests and in KVM is equally mature and wide-spread, so we should use the GICv3 emulation for the guest as well. This algorithm is in effect is there is no explicit --irqchip parameter on the command line. We still allow the GIC type to be set explicitly. Signed-off-by:
-
- 08 Feb, 2019 8 commits
-
-
Andre Przywara authored
The code for copying an empty IP address into the DHCP opt buffer used strncpy, however used the source length as the size argument. GCC 8.x complains about it. Since the source string is actually fixed, just revert to the old strcpy, which gives us actually the same level of security in this case, but makes the compiler happy. Signed-off-by:
-
Andre Przywara authored
GCC 8.x complains about improper usage of strncpy in virtio/net.c and virtio/scsi.c: In function 'virtio_scsi_init_one', inlined from 'virtio_scsi_init' at virtio/scsi.c:285:7: virtio/scsi.c:247:2: error: 'strncpy' specified bound 224 equals destination size [-Werror=stringop-truncation] strncpy((char *)&sdev->target.vhost_wwpn, disk->wwpn, sizeof(sdev->target.vhost_wwpn)); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Fix this and the other occurences in virtio/ by using strlcpy instead of strncpy. Signed-off-by: -
Andre Przywara authored
There are two uses of strncpy in builtin-run.c, where we don't make proper use of strncpy, so that GCC 8.x complains and aborts compilation. Replace those two calls with strlcpy(), which does the right thing in our case. Signed-off-by:
-
Andre Przywara authored
"make -s" suppresses normal output, just shows warnings and errors. But since we explicitly override the make output with our fancy concise version, we miss out on this feature. Do as the kernel does and explicitly suppress every normal output when -s is given. This helps to spot warnings that scroll out of the terminal window too quickly. Signed-off-by:
-
Andre Przywara authored
The DT spec describes the stdout-path property in the /chosen node to contain the DT path for a default device usable for outputting characters. The Linux kernel uses this for earlycon (without further parameters), other DT users might rely on this as well. Add a stdout-path property pointing to the "serial0" alias, then add an aliases node at the end of the FDT, containing the actual path. This allows the FDT generation code in hw/serial.c to set this string. Even when we use the virtio console, the serial console is still there and works, so we can expose this unconditionally. Putting the virtio console path in there will not work anyway. Signed-off-by:
-
Anisse Astier authored
GCC 8.2 gives this warning: virtio/9p.c: In function ‘virtio_p9_create’: virtio/9p.c:335:21: error: passing argument 1 to restrict-qualified parameter aliases with argument 4 [-Werror=restrict] ret = snprintf(dfid->path, size, "%s/%s", dfid->path, name); ~~~~^~~~~~ ~~~~~~~~~~ Fix it by allocating a temporary string with dfid->path content instead of overwriting it in-place, which is limited in glibc snprintf with the __restrict qualifier. Reviewed-by:Anisse Astier <aastier@freebox.fr> Signed-off-by:
-
Anisse Astier authored
GCC 8.2 gives this warning: virtio/net.c: In function ‘virtio_net__tap_init’: virtio/net.c:336:47: error: argument to ‘sizeof’ in ‘strncpy’ call is the same expression as the source; did you mean to use the size of the destination? [-Werror=sizeof-pointer-memaccess] strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name)); ^ virtio/net.c:348:47: error: argument to ‘sizeof’ in ‘strncpy’ call is the same expression as the source; did you mean to use the size of the destination? [-Werror=sizeof-pointer-memaccess] strncpy(ifr.ifr_name, ndev->tap_name, sizeof(ndev->tap_name)); ^ Fix it by using sizeof of destination instead, even if they're the same size in this case. Reviewed-by: -
Anisse Astier authored
GCC 8.2 gives this warning: builtin-run.c: In function ‘kvm_run_write_sandbox_cmd.isra.1’: builtin-run.c:417:28: error: ‘%s’ directive output may be truncated writing up to 4095 bytes into a region of size 4091 [-Werror=format-truncation=] snprintf(dst, len, "/host%s", resolved_path); ^~ ~~~~~~~~~~~~~ It's because it understands that len is PATH_MAX, the same as resolved_path's size. This patch handles the case where the string is truncated, and fixes the warning. Reviewed-by:
-
- 01 Feb, 2019 3 commits
-
-
Dmitry Monakhov authored
It is not good idea to pass empty 'source' argument to mount(2) because libmount complains about incorrect /proc/self/mountinfo structure. This affects many applications such as findmnt, umount and etc. Let's add fake source argument to sysfs mount command as we do with all other filesystems. Reviewed-by:
Dmitry Monakhov <dmtrmonakhov@yandex-team.ru> Signed-off-by:
-
Andre Przywara authored
When loading a firmware instead of a kernel, we can still pass on any *user-provided* command line, as /chosen/bootargs is a generic device tree feature. We just need to make sure to not pass our mangled-for-Linux version. This allows to run "firmware" images which make use of a command line, still are not Linux kernels, like kvm-unit-tests. Signed-off-by:
-
Andre Przywara authored
On every build we report the kvmtool "version" number, which isn't meaningful at all anymore. Remove the line from the KVMTOOLS-VERSION-GEN script to drop a pointless message. Signed-off-by:
-
- 30 Jan, 2019 2 commits
-
-
Andre Przywara authored
The KVM ioctls mostly just return -1 in the error case, leaving the actual error code in errno. Change the output of the PMU error message to actually print this error code instead of the generic -1. Signed-off-by:
-
Andre Przywara authored
For whatever reason on ARM/arm64 machines kvmtool greets us with quite some elaborate messages: Info: Loaded kernel to 0x80080000 (18704896 bytes) Info: Placing fdt at 0x8fe00000 - 0x8fffffff Info: virtio-mmio.devices=0x200@0x10000:36 Info: virtio-mmio.devices=0x200@0x10200:37 Info: virtio-mmio.devices=0x200@0x10400:38 This is not really useful information for the casual user, so change those lines to use pr_debug(). This also fixes the long standing line ending issue for the mmio output. Signed-off-by:
-
- 22 Jan, 2019 7 commits
-
-
Jean-Philippe Brucker authored
The virtio-console reset cancels all running jobs. Unfortunately we don't have a good way to prevent the term polling thread from getting in the way, read invalid data during reset and cause a segfault. To prevent this, move all handling of the Rx queue in the threadpool job. Signed-off-by:
-
Jean-Philippe Brucker authored
The p9 reset cancels all running jobs and closes any open fid. Signed-off-by:
-
Jean-Philippe Brucker authored
When resetting a virtqueue, it is often necessary to make sure that the associated threadpool job isn't running anymore. Add a function to cancel a job. A threadpool job has three states: idle, queued and running. A job is queued when it is in the job list. It is running when it is out the list, but its signal count is greater than zero. It is idle when it is both out of the list and its signal count is zero. The cancel() function simply waits for the job to be idle. It is up to the caller to make sure that the job isn't queued concurrently. Signed-off-by:
-
Jean-Philippe Brucker authored
Move pthread creation to init_vq, and kill the thread in exit_vq. Initialize the virtqueue states at runtime. All in-flight I/O is canceled with the virtqueue pthreads, except for AIO threads, but after reading the code I'm not sure if AIO has ever worked anyway. Signed-off-by:
-
Jean-Philippe Brucker authored
On exit_vq(), clean all resources allocated for the queue. When the device is reset, clean the backend. Signed-off-by:
-
Jean-Philippe Brucker authored
Currently the virtqueue state is mixed with the netdev state. Move it to a separate structure. Signed-off-by:
-
Jean-Philippe Brucker authored
When resetting the virtio-net queues, the UIP state needs to be reset as well. Stop all threads (one per TCP stream and UDP connection) and free memory. Signed-off-by:
-
